[PATCH 03/15] cxl/aer/pci: Refactor AER driver's existing interfaces to support CXL PCIe ports

Terry Bowman posted 15 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 03/15] cxl/aer/pci: Refactor AER driver's existing interfaces to support CXL PCIe ports
Posted by Terry Bowman 1 month, 2 weeks ago
The AER service driver already includes support for CXL restricted host
(RCH) downstream port error handling. The current implementation is based
CXl1.1 using a root complex event collector.

Update the function interfaces and parameters where necessary to add
virtual hierarchy (VH) mode CXL PCIe port error handling alongside the RCH
handling. The CXL PCIe port error handling will be added in a future patch.

Limit changes to refactoring variable and function names. No
functional changes are added.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
---
 drivers/pci/pcie/aer.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 1e72829a249f..dc8b17999001 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1030,7 +1030,7 @@ static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
 	return 0;
 }
 
-static void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
+static void cxl_handle_error(struct pci_dev *dev, struct aer_err_info *info)
 {
 	/*
 	 * Internal errors of an RCEC indicate an AER error in an
@@ -1053,30 +1053,30 @@ static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
 	return *handles_cxl;
 }
 
-static bool handles_cxl_errors(struct pci_dev *rcec)
+static bool handles_cxl_errors(struct pci_dev *dev)
 {
 	bool handles_cxl = false;
 
-	if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
-	    pcie_aer_is_native(rcec))
-		pcie_walk_rcec(rcec, handles_cxl_error_iter, &handles_cxl);
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC &&
+	    pcie_aer_is_native(dev))
+		pcie_walk_rcec(dev, handles_cxl_error_iter, &handles_cxl);
 
 	return handles_cxl;
 }
 
-static void cxl_rch_enable_rcec(struct pci_dev *rcec)
+static void cxl_enable_internal_errors(struct pci_dev *dev)
 {
-	if (!handles_cxl_errors(rcec))
+	if (!handles_cxl_errors(dev))
 		return;
 
-	pci_aer_unmask_internal_errors(rcec);
-	pci_info(rcec, "CXL: Internal errors unmasked");
+	pci_aer_unmask_internal_errors(dev);
+	pci_info(dev, "CXL: Internal errors unmasked");
 }
 
 #else
-static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
-static inline void cxl_rch_handle_error(struct pci_dev *dev,
-					struct aer_err_info *info) { }
+static inline void cxl_enable_internal_errors(struct pci_dev *dev) { }
+static inline void cxl_handle_error(struct pci_dev *dev,
+				    struct aer_err_info *info) { }
 #endif
 
 void register_cxl_port_hndlrs(struct cxl_port_err_hndlrs *_cxl_port_hndlrs)
@@ -1134,7 +1134,7 @@ static void pci_aer_handle_error(struct pci_dev *dev, struct aer_err_info *info)
 
 static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
 {
-	cxl_rch_handle_error(dev, info);
+	cxl_handle_error(dev, info);
 	pci_aer_handle_error(dev, info);
 	pci_dev_put(dev);
 }
@@ -1512,7 +1512,7 @@ static int aer_probe(struct pcie_device *dev)
 		return status;
 	}
 
-	cxl_rch_enable_rcec(port);
+	cxl_enable_internal_errors(port);
 	aer_enable_rootport(rpc);
 	pci_info(port, "enabled with IRQ %d\n", dev->irq);
 	return 0;
-- 
2.34.1
Re: [PATCH 03/15] cxl/aer/pci: Refactor AER driver's existing interfaces to support CXL PCIe ports
Posted by Bjorn Helgaas 1 month, 2 weeks ago
I would describe this more as "renaming" than "refactoring".

On Tue, Oct 08, 2024 at 05:16:45PM -0500, Terry Bowman wrote:
> The AER service driver already includes support for CXL restricted host
> (RCH) downstream port error handling. The current implementation is based
> CXl1.1 using a root complex event collector.
> 
> Update the function interfaces and parameters where necessary to add
> virtual hierarchy (VH) mode CXL PCIe port error handling alongside the RCH
> handling. The CXL PCIe port error handling will be added in a future patch.

"Virtual Hierarchy mode" sounds like something defined by the spec.
If so, add a citation and capitalize it the same way it's used in the
spec.

Same for "restricted host", at least in terms of styling.  That
support was added previously, so a citation probably isn't necessary
here, but since this is part of *adding* VH support, hints about VH
will be more helpful.

> Limit changes to refactoring variable and function names. No
> functional changes are added.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> ---
>  drivers/pci/pcie/aer.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 1e72829a249f..dc8b17999001 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1030,7 +1030,7 @@ static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
>  	return 0;
>  }
>  
> -static void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
> +static void cxl_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>  {
>  	/*
>  	 * Internal errors of an RCEC indicate an AER error in an
> @@ -1053,30 +1053,30 @@ static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
>  	return *handles_cxl;
>  }
>  
> -static bool handles_cxl_errors(struct pci_dev *rcec)
> +static bool handles_cxl_errors(struct pci_dev *dev)
>  {
>  	bool handles_cxl = false;
>  
> -	if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
> -	    pcie_aer_is_native(rcec))
> -		pcie_walk_rcec(rcec, handles_cxl_error_iter, &handles_cxl);
> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC &&
> +	    pcie_aer_is_native(dev))
> +		pcie_walk_rcec(dev, handles_cxl_error_iter, &handles_cxl);
>  
>  	return handles_cxl;
>  }
>  
> -static void cxl_rch_enable_rcec(struct pci_dev *rcec)
> +static void cxl_enable_internal_errors(struct pci_dev *dev)
>  {
> -	if (!handles_cxl_errors(rcec))
> +	if (!handles_cxl_errors(dev))
>  		return;
>  
> -	pci_aer_unmask_internal_errors(rcec);
> -	pci_info(rcec, "CXL: Internal errors unmasked");
> +	pci_aer_unmask_internal_errors(dev);
> +	pci_info(dev, "CXL: Internal errors unmasked");
>  }
>  
>  #else
> -static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
> -static inline void cxl_rch_handle_error(struct pci_dev *dev,
> -					struct aer_err_info *info) { }
> +static inline void cxl_enable_internal_errors(struct pci_dev *dev) { }
> +static inline void cxl_handle_error(struct pci_dev *dev,
> +				    struct aer_err_info *info) { }
>  #endif
>  
>  void register_cxl_port_hndlrs(struct cxl_port_err_hndlrs *_cxl_port_hndlrs)
> @@ -1134,7 +1134,7 @@ static void pci_aer_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>  
>  static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
>  {
> -	cxl_rch_handle_error(dev, info);
> +	cxl_handle_error(dev, info);
>  	pci_aer_handle_error(dev, info);
>  	pci_dev_put(dev);
>  }
> @@ -1512,7 +1512,7 @@ static int aer_probe(struct pcie_device *dev)
>  		return status;
>  	}
>  
> -	cxl_rch_enable_rcec(port);
> +	cxl_enable_internal_errors(port);
>  	aer_enable_rootport(rpc);
>  	pci_info(port, "enabled with IRQ %d\n", dev->irq);
>  	return 0;
> -- 
> 2.34.1
>
Re: [PATCH 03/15] cxl/aer/pci: Refactor AER driver's existing interfaces to support CXL PCIe ports
Posted by Terry Bowman 1 month, 1 week ago
Hi Bjorn,


On 10/10/24 14:11, Bjorn Helgaas wrote:
> I would describe this more as "renaming" than "refactoring".
> 

Good point. Renaming is more correct. Thanks. 

> On Tue, Oct 08, 2024 at 05:16:45PM -0500, Terry Bowman wrote:
>> The AER service driver already includes support for CXL restricted host
>> (RCH) downstream port error handling. The current implementation is based
>> CXl1.1 using a root complex event collector.
>>
>> Update the function interfaces and parameters where necessary to add
>> virtual hierarchy (VH) mode CXL PCIe port error handling alongside the RCH
>> handling. The CXL PCIe port error handling will be added in a future patch.
> 
> "Virtual Hierarchy mode" sounds like something defined by the spec.
> If so, add a citation and capitalize it the same way it's used in the
> spec.
> 
> Same for "restricted host", at least in terms of styling.  That
> support was added previously, so a citation probably isn't necessary
> here, but since this is part of *adding* VH support, hints about VH
> will be more helpful.
> 

Ok.

Regards,
Terry

>> Limit changes to refactoring variable and function names. No
>> functional changes are added.
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> ---
>>  drivers/pci/pcie/aer.c | 28 ++++++++++++++--------------
>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
>> index 1e72829a249f..dc8b17999001 100644
>> --- a/drivers/pci/pcie/aer.c
>> +++ b/drivers/pci/pcie/aer.c
>> @@ -1030,7 +1030,7 @@ static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
>>  	return 0;
>>  }
>>  
>> -static void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>> +static void cxl_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>>  {
>>  	/*
>>  	 * Internal errors of an RCEC indicate an AER error in an
>> @@ -1053,30 +1053,30 @@ static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
>>  	return *handles_cxl;
>>  }
>>  
>> -static bool handles_cxl_errors(struct pci_dev *rcec)
>> +static bool handles_cxl_errors(struct pci_dev *dev)
>>  {
>>  	bool handles_cxl = false;
>>  
>> -	if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
>> -	    pcie_aer_is_native(rcec))
>> -		pcie_walk_rcec(rcec, handles_cxl_error_iter, &handles_cxl);
>> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC &&
>> +	    pcie_aer_is_native(dev))
>> +		pcie_walk_rcec(dev, handles_cxl_error_iter, &handles_cxl);
>>  
>>  	return handles_cxl;
>>  }
>>  
>> -static void cxl_rch_enable_rcec(struct pci_dev *rcec)
>> +static void cxl_enable_internal_errors(struct pci_dev *dev)
>>  {
>> -	if (!handles_cxl_errors(rcec))
>> +	if (!handles_cxl_errors(dev))
>>  		return;
>>  
>> -	pci_aer_unmask_internal_errors(rcec);
>> -	pci_info(rcec, "CXL: Internal errors unmasked");
>> +	pci_aer_unmask_internal_errors(dev);
>> +	pci_info(dev, "CXL: Internal errors unmasked");
>>  }
>>  
>>  #else
>> -static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
>> -static inline void cxl_rch_handle_error(struct pci_dev *dev,
>> -					struct aer_err_info *info) { }
>> +static inline void cxl_enable_internal_errors(struct pci_dev *dev) { }
>> +static inline void cxl_handle_error(struct pci_dev *dev,
>> +				    struct aer_err_info *info) { }
>>  #endif
>>  
>>  void register_cxl_port_hndlrs(struct cxl_port_err_hndlrs *_cxl_port_hndlrs)
>> @@ -1134,7 +1134,7 @@ static void pci_aer_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>>  
>>  static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
>>  {
>> -	cxl_rch_handle_error(dev, info);
>> +	cxl_handle_error(dev, info);
>>  	pci_aer_handle_error(dev, info);
>>  	pci_dev_put(dev);
>>  }
>> @@ -1512,7 +1512,7 @@ static int aer_probe(struct pcie_device *dev)
>>  		return status;
>>  	}
>>  
>> -	cxl_rch_enable_rcec(port);
>> +	cxl_enable_internal_errors(port);
>>  	aer_enable_rootport(rpc);
>>  	pci_info(port, "enabled with IRQ %d\n", dev->irq);
>>  	return 0;
>> -- 
>> 2.34.1
>>