[PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[Frederic Barrat]

Introduce a new API to read the VPD of the adapter. In bare-metal, a
kernel driver can find out the adapter pci_dev behind the AFU device
and call pci_read_vpd() directly, but it won't work in a (powerVM)
guest.
Current implementation is a stub to allow existing drivers (cxlflash)
to start using it.

Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
---
 drivers/misc/cxl/api.c | 8 ++++++++
 include/misc/cxl.h     | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index ea3eeb7..c73aa3a 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -363,3 +363,11 @@ void cxl_perst_reloads_same_image(struct cxl_afu *afu,
 	afu->adapter->perst_same_image = perst_reloads_same_image;
 }
 EXPORT_SYMBOL_GPL(cxl_perst_reloads_same_image);
+
+ssize_t cxl_read_adapter_vpd(struct pci_dev *afu_dev, void *buf, size_t count)
+{
+	struct device *parent = cxl_get_phys_dev(afu_dev);
+
+	return pci_read_vpd(to_pci_dev(parent), 0, count, buf);
+}
+EXPORT_SYMBOL_GPL(cxl_read_adapter_vpd);
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index f2ffe5b..3f9e84f 100644
--- a/include/misc/cxl.h
+++ b/include/misc/cxl.h
@@ -210,4 +210,9 @@ ssize_t cxl_fd_read(struct file *file, char __user *buf, size_t count,
 void cxl_perst_reloads_same_image(struct cxl_afu *afu,
 				  bool perst_reloads_same_image);
 
+/*
+ * Read the VPD of the adapter where the AFU pci dev resides
+ */
+ssize_t cxl_read_adapter_vpd(struct pci_dev *afu_dev, void *buf, size_t count);
+
 #endif /* _MISC_CXL_H */
-- 
1.9.1

Re: [PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[Andrew Donnellan]

On 20/01/16 04:14, Frederic Barrat wrote:
> Introduce a new API to read the VPD of the adapter. In bare-metal, a
> kernel driver can find out the adapter pci_dev behind the AFU device
> and call pci_read_vpd() directly, but it won't work in a (powerVM)
> guest.
> Current implementation is a stub to allow existing drivers (cxlflash)
> to start using it.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Looks fine to me.

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

-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

Re: [PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[Michael Neuling]

On Tue, 2016-01-19 at 18:14 +0100, Frederic Barrat wrote:
> Introduce a new API to read the VPD of the adapter. In bare-metal, a
> kernel driver can find out the adapter pci_dev behind the AFU device
> and call pci_read_vpd() directly, but it won't work in a (powerVM)
> guest.
> Current implementation is a stub to allow existing drivers (cxlflash)
> to start using it.

This looks good.

The only thing I'm a bit concerned about is are we going to end up
duplicating a lot of the linux PCI API, but I guess we are only going
to do this for things the papr HCALL interface mimics.

Mikey

> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> ---
>  drivers/misc/cxl/api.c | 8 ++++++++
>  include/misc/cxl.h     | 5 +++++
>  2 files changed, 13 insertions(+)
>=20
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index ea3eeb7..c73aa3a 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -363,3 +363,11 @@ void cxl_perst_reloads_same_image(struct cxl_afu
> *afu,
>  	afu->adapter->perst_same_image =3D perst_reloads_same_image;
>  }
>  EXPORT_SYMBOL_GPL(cxl_perst_reloads_same_image);
> +
> +ssize_t cxl_read_adapter_vpd(struct pci_dev *afu_dev, void *buf,
> size_t count)
> +{
> +	struct device *parent =3D cxl_get_phys_dev(afu_dev);
> +
> +	return pci_read_vpd(to_pci_dev(parent), 0, count, buf);
> +}
> +EXPORT_SYMBOL_GPL(cxl_read_adapter_vpd);
> diff --git a/include/misc/cxl.h b/include/misc/cxl.h
> index f2ffe5b..3f9e84f 100644
> --- a/include/misc/cxl.h
> +++ b/include/misc/cxl.h
> @@ -210,4 +210,9 @@ ssize_t cxl_fd_read(struct file *file, char
> __user *buf, size_t count,
>  void cxl_perst_reloads_same_image(struct cxl_afu *afu,
>  				  bool perst_reloads_same_image);
> =20
> +/*
> + * Read the VPD of the adapter where the AFU pci dev resides
> + */
> +ssize_t cxl_read_adapter_vpd(struct pci_dev *afu_dev, void *buf,
> size_t count);
> +
>  #endif /* _MISC_CXL_H */

Re: [PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[Frederic Barrat]

Le 20/01/2016 03:20, Michael Neuling a écrit :
> The only thing I'm a bit concerned about is are we going to end up
> duplicating a lot of the linux PCI API, but I guess we are only going
> to do this for things the papr HCALL interface mimics.

There are actually very few operations we can do on the adapter with 
hcalls. papr defines 'reset', 'read the VPD' and flashing a new image on 
the card. So we'll soon run out of APIs to mimic.

I guess it means the usage of cxl_get_phys_dev() should be discouraged, 
since it's going to lead to different behaviors between bare-metal and 
powerVM guest. Was there another expected use case for a kernel driver 
other than accessing the VPD?

   Fred

Re: [PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[Michael Neuling]

On Thu, 2016-01-21 at 19:48 +0100, Frederic Barrat wrote:
>=20
> Le 20/01/2016 03:20, Michael Neuling a =C3=A9crit :
> > The only thing I'm a bit concerned about is are we going to end up
> > duplicating a lot of the linux PCI API, but I guess we are only going
> > to do this for things the papr HCALL interface mimics.
>=20
> There are actually very few operations we can do on the adapter with=20
> hcalls. papr defines 'reset', 'read the VPD' and flashing a new image on=
=20
> the card. So we'll soon run out of APIs to mimic.
>=20
> I guess it means the usage of cxl_get_phys_dev() should be discouraged,=
=20
> since it's going to lead to different behaviors between bare-metal and=
=20
> powerVM guest. Was there another expected use case for a kernel driver=
=20
> other than accessing the VPD?

It was just for VPD.  I figured it was the easiest way to add it. Maybe
it's worth getting rid of it in favour of VPD only.

If you want to remove it I'd be happy, but you'll need to coordinate
with the cxlflash driver.  You can probably just write the patch for
them and then get their ACK on it.

Thanks again.

Mikey

Re: [PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[Frederic Barrat]

Le 22/01/2016 01:38, Michael Neuling a écrit :
> On Thu, 2016-01-21 at 19:48 +0100, Frederic Barrat wrote:
>>
>> Le 20/01/2016 03:20, Michael Neuling a écrit :
>>> The only thing I'm a bit concerned about is are we going to end up
>>> duplicating a lot of the linux PCI API, but I guess we are only going
>>> to do this for things the papr HCALL interface mimics.
>>
>> There are actually very few operations we can do on the adapter with
>> hcalls. papr defines 'reset', 'read the VPD' and flashing a new image on
>> the card. So we'll soon run out of APIs to mimic.
>>
>> I guess it means the usage of cxl_get_phys_dev() should be discouraged,
>> since it's going to lead to different behaviors between bare-metal and
>> powerVM guest. Was there another expected use case for a kernel driver
>> other than accessing the VPD?
>
> It was just for VPD.  I figured it was the easiest way to add it. Maybe
> it's worth getting rid of it in favour of VPD only.
>
> If you want to remove it I'd be happy, but you'll need to coordinate
> with the cxlflash driver.  You can probably just write the patch for
> them and then get their ACK on it.

Ok, then I would also be tempted to remove it. I'll sync with cxlflash 
to do so.
Thanks!

   Fred

Re: [PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[Ian Munsie]

Looks good :)

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: [PATCH] cxl: Add cxl_read_adapter_vpd() to the kernel API

[Frederic Barrat]

The point of this patch was to break a dependency with cxlflash, so that 
they could drop their powerVM changes independently from cxl.

Since this hasn't hit upstream or next yet, change of plan: we've agreed 
with cxlflash that the cxl powerVM patchset will do the (small) 
modification to cxlflash (and remove the now obsolete cxl_get_phys_dev() 
kernel API, as discussed in this thread with Mikey). So no more 
dependencies.
It will be part of the powerVM patchset v6

   Fred


Le 19/01/2016 18:14, Frederic Barrat a écrit :
> Introduce a new API to read the VPD of the adapter. In bare-metal, a
> kernel driver can find out the adapter pci_dev behind the AFU device
> and call pci_read_vpd() directly, but it won't work in a (powerVM)
> guest.
> Current implementation is a stub to allow existing drivers (cxlflash)
> to start using it.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> ---
>   drivers/misc/cxl/api.c | 8 ++++++++
>   include/misc/cxl.h     | 5 +++++
>   2 files changed, 13 insertions(+)
>
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index ea3eeb7..c73aa3a 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -363,3 +363,11 @@ void cxl_perst_reloads_same_image(struct cxl_afu *afu,
>   	afu->adapter->perst_same_image = perst_reloads_same_image;
>   }
>   EXPORT_SYMBOL_GPL(cxl_perst_reloads_same_image);
> +
> +ssize_t cxl_read_adapter_vpd(struct pci_dev *afu_dev, void *buf, size_t count)
> +{
> +	struct device *parent = cxl_get_phys_dev(afu_dev);
> +
> +	return pci_read_vpd(to_pci_dev(parent), 0, count, buf);
> +}
> +EXPORT_SYMBOL_GPL(cxl_read_adapter_vpd);
> diff --git a/include/misc/cxl.h b/include/misc/cxl.h
> index f2ffe5b..3f9e84f 100644
> --- a/include/misc/cxl.h
> +++ b/include/misc/cxl.h
> @@ -210,4 +210,9 @@ ssize_t cxl_fd_read(struct file *file, char __user *buf, size_t count,
>   void cxl_perst_reloads_same_image(struct cxl_afu *afu,
>   				  bool perst_reloads_same_image);
>
> +/*
> + * Read the VPD of the adapter where the AFU pci dev resides
> + */
> +ssize_t cxl_read_adapter_vpd(struct pci_dev *afu_dev, void *buf, size_t count);
> +
>   #endif /* _MISC_CXL_H */
>