diff mbox series

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

Message ID 1534169362-9394-1-git-send-email-clombard@linux.vnet.ibm.com (mailing list archive)
State Superseded, archived
Headers show
Series [V2] ocxl: Fix access to the AFU Descriptor Data | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied
snowpatch_ozlabs/checkpatch warning Test checkpatch on branch next
snowpatch_ozlabs/build-ppc64le success Test build-ppc64le on branch next
snowpatch_ozlabs/build-ppc64be success Test build-ppc64be on branch next
snowpatch_ozlabs/build-ppc64e success Test build-ppc64e on branch next
snowpatch_ozlabs/build-ppc32 success Test build-ppc32 on branch next

Commit Message

Christophe Lombard Aug. 13, 2018, 2:09 p.m. UTC
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")
Cc: stable <stable@vger.kernel.org>     # 4.16
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
Changelog[v2]
 - Rebase to latest upstream.
 - Use pci_write_config_byte instead of pci_write_config_word
---
 drivers/misc/ocxl/config.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Frederic Barrat Aug. 13, 2018, 2:50 p.m. UTC | #1
Le 13/08/2018 à 16:09, Christophe Lombard a écrit :
> 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")
> Cc: stable <stable@vger.kernel.org>     # 4.16
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> ---

Thanks!
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


> Changelog[v2]
>   - Rebase to latest upstream.
>   - Use pci_write_config_byte instead of pci_write_config_word
> ---
>   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..57a6bb1 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_byte(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;
>
Andrew Donnellan Aug. 14, 2018, 1:15 a.m. UTC | #2
On 14/08/18 00:09, Christophe Lombard wrote:
> 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")
> Cc: stable <stable@vger.kernel.org>     # 4.16
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>

Thanks

Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

> ---
> Changelog[v2]
>   - Rebase to latest upstream.
>   - Use pci_write_config_byte instead of pci_write_config_word
> ---
>   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..57a6bb1 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_byte(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;
>
Michael Ellerman Aug. 14, 2018, 3:26 a.m. UTC | #3
Hi Christophe,

The patch looks fine, just a nit about the change log:

Christophe Lombard <clombard@linux.vnet.ibm.com> writes:
> 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")
> Cc: stable <stable@vger.kernel.org>     # 4.16
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>

When fixing a bug it's always good to describe how the bug manifests.
ie. in this case we are clearly writing to the wrong location in config
space, but what is the consequence of that? Does it kill the device, or
just fails to initialise something correctly? How could I tell if I'm
hitting this bug currently? How would I tell if the fix is applied
correctly?

cheers

> ---
> Changelog[v2]
>  - Rebase to latest upstream.
>  - Use pci_write_config_byte instead of pci_write_config_word
> ---
>  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..57a6bb1 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_byte(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
Christophe Lombard Aug. 14, 2018, 12:22 p.m. UTC | #4
Le 14/08/2018 à 05:26, Michael Ellerman a écrit :
> Hi Christophe,
> 
> The patch looks fine, just a nit about the change log:
> 
> Christophe Lombard <clombard@linux.vnet.ibm.com> writes:
>> 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")
>> Cc: stable <stable@vger.kernel.org>     # 4.16
>> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> 
> When fixing a bug it's always good to describe how the bug manifests.
> ie. in this case we are clearly writing to the wrong location in config
> space, but what is the consequence of that? Does it kill the device, or
> just fails to initialise something correctly? How could I tell if I'm
> hitting this bug currently? How would I tell if the fix is applied
> correctly?

You are right, let me send a new version.

Thanks

> 
> cheers
> 
>> ---
>> Changelog[v2]
>>   - Rebase to latest upstream.
>>   - Use pci_write_config_byte instead of pci_write_config_word
>> ---
>>   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..57a6bb1 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_byte(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
>
diff mbox series

Patch

diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 2e30de9..57a6bb1 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_byte(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;