[PATCH] ocxl: Fix access to the AFU Descriptor Data

[PATCH] ocxl: Fix access to the AFU Descriptor Data

[Christophe Lombard]

From: Christophe Lombard <christophe_lombard@fr.ibm.cm>

The AFU Information DVSEC capability is a means to extract common,
general information about all of the AFUs associated with a Function
independent of the specific functionality that each AFU provides.

This patch fixes the access to the AFU Descriptor Data indexed by the
AFU Info Index field.

Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")

Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
 drivers/misc/ocxl/config.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 2e30de9..de01623 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
 	u32 val;
 	int rc, templ_major, templ_minor, len;
 
-	pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
+	pci_write_config_word(dev,
+			      fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
+			      afu_idx);
 	rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
 	if (rc)
 		return rc;
-- 
2.7.4

Re: [PATCH] ocxl: Fix access to the AFU Descriptor Data

[Andrew Donnellan]

On 13/08/18 19:01, Christophe Lombard wrote:
> From: Christophe Lombard <christophe_lombard@fr.ibm.cm>

Your git committer email should probably match your sign-off email.

> 
> The AFU Information DVSEC capability is a means to extract common,
> general information about all of the AFUs associated with a Function
> independent of the specific functionality that each AFU provides.
> 
> This patch fixes the access to the AFU Descriptor Data indexed by the
> AFU Info Index field.
> 
> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
> 
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>

This looks like it should go to stable? I assume the reason we haven't 
noticed this previously is because we have not been testing with 
multi-AFU cards.

> ---
>   drivers/misc/ocxl/config.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index 2e30de9..de01623 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
>   	u32 val;
>   	int rc, templ_major, templ_minor, len;
>   
> -	pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
> +	pci_write_config_word(dev,
> +			      fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
> +			      afu_idx);

pci_write_config_byte() would be more appropriate here (see 
ocxl_config_read_afu() at line 454)

>   	rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
>   	if (rc)
>   		return rc;
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH] ocxl: Fix access to the AFU Descriptor Data

[Frederic Barrat]

Le 13/08/2018 à 11:48, Andrew Donnellan a écrit :
> On 13/08/18 19:01, Christophe Lombard wrote:
>> From: Christophe Lombard <christophe_lombard@fr.ibm.cm>
> 
> Your git committer email should probably match your sign-off email.
> 
>>
>> The AFU Information DVSEC capability is a means to extract common,
>> general information about all of the AFUs associated with a Function
>> independent of the specific functionality that each AFU provides.
>>
>> This patch fixes the access to the AFU Descriptor Data indexed by the
>> AFU Info Index field.
>>
>> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
>>
>> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> 
> This looks like it should go to stable? I assume the reason we haven't 
> noticed this previously is because we have not been testing with 
> multi-AFU cards.

Yes, it's hidden until we have more than one AFU per function. Also, the 
field we overwrite at the wrong offset is read-only, so it goes undetected.

   Fred




> 
>> ---
>>   drivers/misc/ocxl/config.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
>> index 2e30de9..de01623 100644
>> --- a/drivers/misc/ocxl/config.c
>> +++ b/drivers/misc/ocxl/config.c
>> @@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
>>       u32 val;
>>       int rc, templ_major, templ_minor, len;
>> -    pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
>> +    pci_write_config_word(dev,
>> +                  fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
>> +                  afu_idx);
> 
> pci_write_config_byte() would be more appropriate here (see 
> ocxl_config_read_afu() at line 454)
> 
>>       rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
>>       if (rc)
>>           return rc;
>>
> 

Re: [PATCH] ocxl: Fix access to the AFU Descriptor Data

[christophe lombard]

Le 13/08/2018 à 11:48, Andrew Donnellan a écrit :
> On 13/08/18 19:01, Christophe Lombard wrote:
>> From: Christophe Lombard <christophe_lombard@fr.ibm.cm>
> 
> Your git committer email should probably match your sign-off email.
> 
>>
>> The AFU Information DVSEC capability is a means to extract common,
>> general information about all of the AFUs associated with a Function
>> independent of the specific functionality that each AFU provides.
>>
>> This patch fixes the access to the AFU Descriptor Data indexed by the
>> AFU Info Index field.
>>
>> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
>>
>> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> 
> This looks like it should go to stable? I assume the reason we haven't 
> noticed this previously is because we have not been testing with 
> multi-AFU cards.
> 
>> ---
>>   drivers/misc/ocxl/config.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
>> index 2e30de9..de01623 100644
>> --- a/drivers/misc/ocxl/config.c
>> +++ b/drivers/misc/ocxl/config.c
>> @@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
>>       u32 val;
>>       int rc, templ_major, templ_minor, len;
>> -    pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
>> +    pci_write_config_word(dev,
>> +                  fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
>> +                  afu_idx);
> 
> pci_write_config_byte() would be more appropriate here (see 
> ocxl_config_read_afu() at line 454)
> 
>>       rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
>>       if (rc)
>>           return rc;
>>
> 

right. Thanks for the review.