Re: [PATCH v4 4/4] vfio: pci: Using a device region to retrieve zPCI information
On 9/20/19 10:26 AM, Cornelia Huck wrote: > On Thu, 19 Sep 2019 16:57:10 -0400 > Matthew Rosato wrote: > >> On 9/19/19 11:25 AM, Cornelia Huck wrote: >>> On Fri, 6 Sep 2019 20:13:51 -0400 >>> Matthew Rosato wrote: >>> From: Pierre Morel We define a new configuration entry for VFIO/PCI, VFIO_PCI_ZDEV When the VFIO_PCI_ZDEV feature is configured we initialize a new device region, VFIO_REGION_SUBTYPE_ZDEV_CLP, to hold the information from the ZPCI device the use Signed-off-by: Pierre Morel Signed-off-by: Matthew Rosato --- drivers/vfio/pci/Kconfig| 7 +++ drivers/vfio/pci/Makefile | 1 + drivers/vfio/pci/vfio_pci.c | 9 drivers/vfio/pci/vfio_pci_private.h | 10 + drivers/vfio/pci/vfio_pci_zdev.c| 85 + 5 files changed, 112 insertions(+) create mode 100644 drivers/vfio/pci/vfio_pci_zdev.c diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index ac3c1dd..d4562a8 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -45,3 +45,10 @@ config VFIO_PCI_NVLINK2 depends on VFIO_PCI && PPC_POWERNV help VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs + +config VFIO_PCI_ZDEV + bool "VFIO PCI Generic for ZPCI devices" + depends on VFIO_PCI && S390 + default y + help +VFIO PCI support for S390 Z-PCI devices >>> >>> >From that description, I'd have no idea whether I'd want that or not. >>> Is there any downside to enabling it? >>> >> >> :) Not really, you're just getting information from the hardware vs >> using hard-coded defaults. The only reason I could think of to turn it >> off would be if you wanted/needed to restore this hard-coded behavior. > > I'm not really sure whether that's worth adding a Kconfig switch for. > Won't older versions simply ignore the new region anyway? > Yes, you have a point here... This switch showed up in v3 of this series when Pierre changed to using a region to pass this info and I haven't yet found a 'why' he decided to add the Kconfig switch. If I can't convince myself of a reason to keep it, I'll just remove it from the next version. > Also, I don't think we have any migration compatibility issues, as > vfio-pci devices are not (yet) migrateable anyway. > >> >> bool "VFIO PCI support for generic ZPCI devices" ? > > "Support zPCI-specific configuration for VFIO PCI" ? > >> >> "Support for sharing ZPCI hardware device information between the host >> and guests." ? > > "Enabling this options exposes a region containing hardware > configuration for zPCI devices. This enables userspace (e.g. QEMU) to > supply proper configuration values instead of hard-coded defaults for > zPCI devices passed through via VFIO on s390. > > Say Y here." > > ? > Your descriptions are much better - thanks for the feedback!
Re: [PATCH v4 4/4] vfio: pci: Using a device region to retrieve zPCI information
On 9/19/19 6:57 PM, Alex Williamson wrote:
> On Fri, 6 Sep 2019 20:13:51 -0400
> Matthew Rosato wrote:
>
>> From: Pierre Morel
>>
>> We define a new configuration entry for VFIO/PCI, VFIO_PCI_ZDEV
>>
>> When the VFIO_PCI_ZDEV feature is configured we initialize
>> a new device region, VFIO_REGION_SUBTYPE_ZDEV_CLP, to hold
>> the information from the ZPCI device the use
>>
>> Signed-off-by: Pierre Morel
>> Signed-off-by: Matthew Rosato
>> ---
>> drivers/vfio/pci/Kconfig| 7 +++
>> drivers/vfio/pci/Makefile | 1 +
>> drivers/vfio/pci/vfio_pci.c | 9
>> drivers/vfio/pci/vfio_pci_private.h | 10 +
>> drivers/vfio/pci/vfio_pci_zdev.c| 85
>> +
>> 5 files changed, 112 insertions(+)
>> create mode 100644 drivers/vfio/pci/vfio_pci_zdev.c
>>
>> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
>> index ac3c1dd..d4562a8 100644
>> --- a/drivers/vfio/pci/Kconfig
>> +++ b/drivers/vfio/pci/Kconfig
>> @@ -45,3 +45,10 @@ config VFIO_PCI_NVLINK2
>> depends on VFIO_PCI && PPC_POWERNV
>> help
>>VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs
>> +
>> +config VFIO_PCI_ZDEV
>> +bool "VFIO PCI Generic for ZPCI devices"
>> +depends on VFIO_PCI && S390
>> +default y
>> +help
>> + VFIO PCI support for S390 Z-PCI devices
>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
>> index f027f8a..781e080 100644
>> --- a/drivers/vfio/pci/Makefile
>> +++ b/drivers/vfio/pci/Makefile
>> @@ -3,5 +3,6 @@
>> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
>> vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
>> vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
>> +vfio-pci-$(CONFIG_VFIO_PCI_ZDEV) += vfio_pci_zdev.o
>>
>> obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>> index 703948c..b40544a 100644
>> --- a/drivers/vfio/pci/vfio_pci.c
>> +++ b/drivers/vfio/pci/vfio_pci.c
>> @@ -356,6 +356,15 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>> }
>> }
>>
>> +if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV)) {
>> +ret = vfio_pci_zdev_init(vdev);
>> +if (ret) {
>> +dev_warn(&vdev->pdev->dev,
>> + "Failed to setup ZDEV regions\n");
>> +goto disable_exit;
>> +}
>> +}
>> +
>> vfio_pci_probe_mmaps(vdev);
>>
>> return 0;
>> diff --git a/drivers/vfio/pci/vfio_pci_private.h
>> b/drivers/vfio/pci/vfio_pci_private.h
>> index ee6ee91..08e02f5 100644
>> --- a/drivers/vfio/pci/vfio_pci_private.h
>> +++ b/drivers/vfio/pci/vfio_pci_private.h
>> @@ -186,4 +186,14 @@ static inline int vfio_pci_ibm_npu2_init(struct
>> vfio_pci_device *vdev)
>> return -ENODEV;
>> }
>> #endif
>> +
>> +#ifdef CONFIG_VFIO_PCI_ZDEV
>> +extern int vfio_pci_zdev_init(struct vfio_pci_device *vdev);
>> +#else
>> +static inline int vfio_pci_zdev_init(struct vfio_pci_device *vdev)
>> +{
>> +return -ENODEV;
>> +}
>> +#endif
>> +
>> #endif /* VFIO_PCI_PRIVATE_H */
>> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c
>> b/drivers/vfio/pci/vfio_pci_zdev.c
>> new file mode 100644
>> index 000..22e2b60
>> --- /dev/null
>> +++ b/drivers/vfio/pci/vfio_pci_zdev.c
>> @@ -0,0 +1,85 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * VFIO ZPCI devices support
>> + *
>> + * Copyright (C) IBM Corp. 2019. All rights reserved.
>> + * Author: Pierre Morel
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "vfio_pci_private.h"
>> +
>> +static size_t vfio_pci_zdev_rw(struct vfio_pci_device *vdev,
>> + char __user *buf, size_t count, loff_t *ppos,
>> + bool iswrite)
>> +{
>> +struct vfio_region_zpci_info *region;
>> +struct zpci_dev *zdev;
>> +unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
>> +loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> +
>> +if (!vdev->pdev->bus)
>> +return -ENODEV;
>> +
>> +zdev = vdev->pdev->bus->sysdata;
>> +if (!zdev)
>> +return -ENODEV;
>> +
>> +if (pos >= sizeof(*region) || iswrite)
>> +return -EINVAL;
>> +
>> +region = vdev->region[index - VFIO_PCI_NUM_REGIONS].data;
>> +region->dasm = zdev->dma_mask;
>> +region->start_dma = zdev->start_dma;
>> +region->end_dma = zdev->end_dma;
>> +region->msi_addr = zdev->msi_addr;
>> +region->flags = VFIO_PCI_ZDEV_FLAGS_REFRESH;
>
> Even more curious what this means, why do we need a flag that's always
> set? Maybe NOREFRESH if it were ever to exist.>
This flag also has a hardware structure counterpart
Re: [PATCH v4 4/4] vfio: pci: Using a device region to retrieve zPCI information
On Thu, 19 Sep 2019 16:57:10 -0400 Matthew Rosato wrote: > On 9/19/19 11:25 AM, Cornelia Huck wrote: > > On Fri, 6 Sep 2019 20:13:51 -0400 > > Matthew Rosato wrote: > > > >> From: Pierre Morel > >> > >> We define a new configuration entry for VFIO/PCI, VFIO_PCI_ZDEV > >> > >> When the VFIO_PCI_ZDEV feature is configured we initialize > >> a new device region, VFIO_REGION_SUBTYPE_ZDEV_CLP, to hold > >> the information from the ZPCI device the use > >> > >> Signed-off-by: Pierre Morel > >> Signed-off-by: Matthew Rosato > >> --- > >> drivers/vfio/pci/Kconfig| 7 +++ > >> drivers/vfio/pci/Makefile | 1 + > >> drivers/vfio/pci/vfio_pci.c | 9 > >> drivers/vfio/pci/vfio_pci_private.h | 10 + > >> drivers/vfio/pci/vfio_pci_zdev.c| 85 > >> + > >> 5 files changed, 112 insertions(+) > >> create mode 100644 drivers/vfio/pci/vfio_pci_zdev.c > >> > >> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig > >> index ac3c1dd..d4562a8 100644 > >> --- a/drivers/vfio/pci/Kconfig > >> +++ b/drivers/vfio/pci/Kconfig > >> @@ -45,3 +45,10 @@ config VFIO_PCI_NVLINK2 > >>depends on VFIO_PCI && PPC_POWERNV > >>help > >> VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs > >> + > >> +config VFIO_PCI_ZDEV > >> + bool "VFIO PCI Generic for ZPCI devices" > >> + depends on VFIO_PCI && S390 > >> + default y > >> + help > >> +VFIO PCI support for S390 Z-PCI devices > > > >>From that description, I'd have no idea whether I'd want that or not. > > Is there any downside to enabling it? > > > > :) Not really, you're just getting information from the hardware vs > using hard-coded defaults. The only reason I could think of to turn it > off would be if you wanted/needed to restore this hard-coded behavior. I'm not really sure whether that's worth adding a Kconfig switch for. Won't older versions simply ignore the new region anyway? Also, I don't think we have any migration compatibility issues, as vfio-pci devices are not (yet) migrateable anyway. > > bool "VFIO PCI support for generic ZPCI devices" ? "Support zPCI-specific configuration for VFIO PCI" ? > > "Support for sharing ZPCI hardware device information between the host > and guests." ? "Enabling this options exposes a region containing hardware configuration for zPCI devices. This enables userspace (e.g. QEMU) to supply proper configuration values instead of hard-coded defaults for zPCI devices passed through via VFIO on s390. Say Y here." ? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 4/4] vfio: pci: Using a device region to retrieve zPCI information
On Fri, 6 Sep 2019 20:13:51 -0400
Matthew Rosato wrote:
> From: Pierre Morel
>
> We define a new configuration entry for VFIO/PCI, VFIO_PCI_ZDEV
>
> When the VFIO_PCI_ZDEV feature is configured we initialize
> a new device region, VFIO_REGION_SUBTYPE_ZDEV_CLP, to hold
> the information from the ZPCI device the use
>
> Signed-off-by: Pierre Morel
> Signed-off-by: Matthew Rosato
> ---
> drivers/vfio/pci/Kconfig| 7 +++
> drivers/vfio/pci/Makefile | 1 +
> drivers/vfio/pci/vfio_pci.c | 9
> drivers/vfio/pci/vfio_pci_private.h | 10 +
> drivers/vfio/pci/vfio_pci_zdev.c| 85
> +
> 5 files changed, 112 insertions(+)
> create mode 100644 drivers/vfio/pci/vfio_pci_zdev.c
>
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index ac3c1dd..d4562a8 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -45,3 +45,10 @@ config VFIO_PCI_NVLINK2
> depends on VFIO_PCI && PPC_POWERNV
> help
> VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs
> +
> +config VFIO_PCI_ZDEV
> + bool "VFIO PCI Generic for ZPCI devices"
> + depends on VFIO_PCI && S390
> + default y
> + help
> + VFIO PCI support for S390 Z-PCI devices
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index f027f8a..781e080 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -3,5 +3,6 @@
> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
> vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
> vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
> +vfio-pci-$(CONFIG_VFIO_PCI_ZDEV) += vfio_pci_zdev.o
>
> obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 703948c..b40544a 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -356,6 +356,15 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
> }
> }
>
> + if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV)) {
> + ret = vfio_pci_zdev_init(vdev);
> + if (ret) {
> + dev_warn(&vdev->pdev->dev,
> + "Failed to setup ZDEV regions\n");
> + goto disable_exit;
> + }
> + }
> +
> vfio_pci_probe_mmaps(vdev);
>
> return 0;
> diff --git a/drivers/vfio/pci/vfio_pci_private.h
> b/drivers/vfio/pci/vfio_pci_private.h
> index ee6ee91..08e02f5 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -186,4 +186,14 @@ static inline int vfio_pci_ibm_npu2_init(struct
> vfio_pci_device *vdev)
> return -ENODEV;
> }
> #endif
> +
> +#ifdef CONFIG_VFIO_PCI_ZDEV
> +extern int vfio_pci_zdev_init(struct vfio_pci_device *vdev);
> +#else
> +static inline int vfio_pci_zdev_init(struct vfio_pci_device *vdev)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> #endif /* VFIO_PCI_PRIVATE_H */
> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c
> b/drivers/vfio/pci/vfio_pci_zdev.c
> new file mode 100644
> index 000..22e2b60
> --- /dev/null
> +++ b/drivers/vfio/pci/vfio_pci_zdev.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * VFIO ZPCI devices support
> + *
> + * Copyright (C) IBM Corp. 2019. All rights reserved.
> + * Author: Pierre Morel
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "vfio_pci_private.h"
> +
> +static size_t vfio_pci_zdev_rw(struct vfio_pci_device *vdev,
> +char __user *buf, size_t count, loff_t *ppos,
> +bool iswrite)
> +{
> + struct vfio_region_zpci_info *region;
> + struct zpci_dev *zdev;
> + unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
> + loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> +
> + if (!vdev->pdev->bus)
> + return -ENODEV;
> +
> + zdev = vdev->pdev->bus->sysdata;
> + if (!zdev)
> + return -ENODEV;
> +
> + if (pos >= sizeof(*region) || iswrite)
> + return -EINVAL;
> +
> + region = vdev->region[index - VFIO_PCI_NUM_REGIONS].data;
> + region->dasm = zdev->dma_mask;
> + region->start_dma = zdev->start_dma;
> + region->end_dma = zdev->end_dma;
> + region->msi_addr = zdev->msi_addr;
> + region->flags = VFIO_PCI_ZDEV_FLAGS_REFRESH;
Even more curious what this means, why do we need a flag that's always
set? Maybe NOREFRESH if it were ever to exist.
> + region->gid = zdev->pfgid;
> + region->mui = zdev->fmb_update;
> + region->noi = zdev->max_msi;
> + memcpy(region->util_str, zdev->util_str, CLP_UTIL_STR_LEN);
Just checking, I ass
Re: [PATCH v4 4/4] vfio: pci: Using a device region to retrieve zPCI information
On 9/19/19 11:25 AM, Cornelia Huck wrote:
> On Fri, 6 Sep 2019 20:13:51 -0400
> Matthew Rosato wrote:
>
>> From: Pierre Morel
>>
>> We define a new configuration entry for VFIO/PCI, VFIO_PCI_ZDEV
>>
>> When the VFIO_PCI_ZDEV feature is configured we initialize
>> a new device region, VFIO_REGION_SUBTYPE_ZDEV_CLP, to hold
>> the information from the ZPCI device the use
>>
>> Signed-off-by: Pierre Morel
>> Signed-off-by: Matthew Rosato
>> ---
>> drivers/vfio/pci/Kconfig| 7 +++
>> drivers/vfio/pci/Makefile | 1 +
>> drivers/vfio/pci/vfio_pci.c | 9
>> drivers/vfio/pci/vfio_pci_private.h | 10 +
>> drivers/vfio/pci/vfio_pci_zdev.c| 85
>> +
>> 5 files changed, 112 insertions(+)
>> create mode 100644 drivers/vfio/pci/vfio_pci_zdev.c
>>
>> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
>> index ac3c1dd..d4562a8 100644
>> --- a/drivers/vfio/pci/Kconfig
>> +++ b/drivers/vfio/pci/Kconfig
>> @@ -45,3 +45,10 @@ config VFIO_PCI_NVLINK2
>> depends on VFIO_PCI && PPC_POWERNV
>> help
>>VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs
>> +
>> +config VFIO_PCI_ZDEV
>> +bool "VFIO PCI Generic for ZPCI devices"
>> +depends on VFIO_PCI && S390
>> +default y
>> +help
>> + VFIO PCI support for S390 Z-PCI devices
>
>>From that description, I'd have no idea whether I'd want that or not.
> Is there any downside to enabling it?
>
:) Not really, you're just getting information from the hardware vs
using hard-coded defaults. The only reason I could think of to turn it
off would be if you wanted/needed to restore this hard-coded behavior.
bool "VFIO PCI support for generic ZPCI devices" ?
"Support for sharing ZPCI hardware device information between the host
and guests." ?
>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
>> index f027f8a..781e080 100644
>> --- a/drivers/vfio/pci/Makefile
>> +++ b/drivers/vfio/pci/Makefile
>> @@ -3,5 +3,6 @@
>> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
>> vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
>> vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
>> +vfio-pci-$(CONFIG_VFIO_PCI_ZDEV) += vfio_pci_zdev.o
>>
>> obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
>> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>> index 703948c..b40544a 100644
>> --- a/drivers/vfio/pci/vfio_pci.c
>> +++ b/drivers/vfio/pci/vfio_pci.c
>> @@ -356,6 +356,15 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>> }
>> }
>>
>> +if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV)) {
>> +ret = vfio_pci_zdev_init(vdev);
>> +if (ret) {
>> +dev_warn(&vdev->pdev->dev,
>> + "Failed to setup ZDEV regions\n");
>> +goto disable_exit;
>> +}
>> +}
>> +
>> vfio_pci_probe_mmaps(vdev);
>>
>> return 0;
>> diff --git a/drivers/vfio/pci/vfio_pci_private.h
>> b/drivers/vfio/pci/vfio_pci_private.h
>> index ee6ee91..08e02f5 100644
>> --- a/drivers/vfio/pci/vfio_pci_private.h
>> +++ b/drivers/vfio/pci/vfio_pci_private.h
>> @@ -186,4 +186,14 @@ static inline int vfio_pci_ibm_npu2_init(struct
>> vfio_pci_device *vdev)
>> return -ENODEV;
>> }
>> #endif
>> +
>> +#ifdef CONFIG_VFIO_PCI_ZDEV
>> +extern int vfio_pci_zdev_init(struct vfio_pci_device *vdev);
>> +#else
>> +static inline int vfio_pci_zdev_init(struct vfio_pci_device *vdev)
>> +{
>> +return -ENODEV;
>
> If you really want to have this configurable, why not just return 0
> here and skip the IS_ENABLED check above?
>
I agree that it functionally has the same result, but in this case I
think Pierre was repeating the same thing the other init() functions
here (IGD, etc) are doing. Though I guess the other cases have at least
1 other condition they care about besides IS_ENABLED... OK, I can make
this change.
>> +}
>> +#endif
>> +
>> #endif /* VFIO_PCI_PRIVATE_H */
>
> (...)
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 4/4] vfio: pci: Using a device region to retrieve zPCI information
On Fri, 6 Sep 2019 20:13:51 -0400
Matthew Rosato wrote:
> From: Pierre Morel
>
> We define a new configuration entry for VFIO/PCI, VFIO_PCI_ZDEV
>
> When the VFIO_PCI_ZDEV feature is configured we initialize
> a new device region, VFIO_REGION_SUBTYPE_ZDEV_CLP, to hold
> the information from the ZPCI device the use
>
> Signed-off-by: Pierre Morel
> Signed-off-by: Matthew Rosato
> ---
> drivers/vfio/pci/Kconfig| 7 +++
> drivers/vfio/pci/Makefile | 1 +
> drivers/vfio/pci/vfio_pci.c | 9
> drivers/vfio/pci/vfio_pci_private.h | 10 +
> drivers/vfio/pci/vfio_pci_zdev.c| 85
> +
> 5 files changed, 112 insertions(+)
> create mode 100644 drivers/vfio/pci/vfio_pci_zdev.c
>
> diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
> index ac3c1dd..d4562a8 100644
> --- a/drivers/vfio/pci/Kconfig
> +++ b/drivers/vfio/pci/Kconfig
> @@ -45,3 +45,10 @@ config VFIO_PCI_NVLINK2
> depends on VFIO_PCI && PPC_POWERNV
> help
> VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs
> +
> +config VFIO_PCI_ZDEV
> + bool "VFIO PCI Generic for ZPCI devices"
> + depends on VFIO_PCI && S390
> + default y
> + help
> + VFIO PCI support for S390 Z-PCI devices
>From that description, I'd have no idea whether I'd want that or not.
Is there any downside to enabling it?
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index f027f8a..781e080 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -3,5 +3,6 @@
> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
> vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
> vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
> +vfio-pci-$(CONFIG_VFIO_PCI_ZDEV) += vfio_pci_zdev.o
>
> obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 703948c..b40544a 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -356,6 +356,15 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
> }
> }
>
> + if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV)) {
> + ret = vfio_pci_zdev_init(vdev);
> + if (ret) {
> + dev_warn(&vdev->pdev->dev,
> + "Failed to setup ZDEV regions\n");
> + goto disable_exit;
> + }
> + }
> +
> vfio_pci_probe_mmaps(vdev);
>
> return 0;
> diff --git a/drivers/vfio/pci/vfio_pci_private.h
> b/drivers/vfio/pci/vfio_pci_private.h
> index ee6ee91..08e02f5 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -186,4 +186,14 @@ static inline int vfio_pci_ibm_npu2_init(struct
> vfio_pci_device *vdev)
> return -ENODEV;
> }
> #endif
> +
> +#ifdef CONFIG_VFIO_PCI_ZDEV
> +extern int vfio_pci_zdev_init(struct vfio_pci_device *vdev);
> +#else
> +static inline int vfio_pci_zdev_init(struct vfio_pci_device *vdev)
> +{
> + return -ENODEV;
If you really want to have this configurable, why not just return 0
here and skip the IS_ENABLED check above?
> +}
> +#endif
> +
> #endif /* VFIO_PCI_PRIVATE_H */
(...)
[PATCH v4 4/4] vfio: pci: Using a device region to retrieve zPCI information
From: Pierre Morel
We define a new configuration entry for VFIO/PCI, VFIO_PCI_ZDEV
When the VFIO_PCI_ZDEV feature is configured we initialize
a new device region, VFIO_REGION_SUBTYPE_ZDEV_CLP, to hold
the information from the ZPCI device the use
Signed-off-by: Pierre Morel
Signed-off-by: Matthew Rosato
---
drivers/vfio/pci/Kconfig| 7 +++
drivers/vfio/pci/Makefile | 1 +
drivers/vfio/pci/vfio_pci.c | 9
drivers/vfio/pci/vfio_pci_private.h | 10 +
drivers/vfio/pci/vfio_pci_zdev.c| 85 +
5 files changed, 112 insertions(+)
create mode 100644 drivers/vfio/pci/vfio_pci_zdev.c
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index ac3c1dd..d4562a8 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -45,3 +45,10 @@ config VFIO_PCI_NVLINK2
depends on VFIO_PCI && PPC_POWERNV
help
VFIO PCI support for P9 Witherspoon machine with NVIDIA V100 GPUs
+
+config VFIO_PCI_ZDEV
+ bool "VFIO PCI Generic for ZPCI devices"
+ depends on VFIO_PCI && S390
+ default y
+ help
+ VFIO PCI support for S390 Z-PCI devices
diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
index f027f8a..781e080 100644
--- a/drivers/vfio/pci/Makefile
+++ b/drivers/vfio/pci/Makefile
@@ -3,5 +3,6 @@
vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o
+vfio-pci-$(CONFIG_VFIO_PCI_ZDEV) += vfio_pci_zdev.o
obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 703948c..b40544a 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -356,6 +356,15 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
}
}
+ if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV)) {
+ ret = vfio_pci_zdev_init(vdev);
+ if (ret) {
+ dev_warn(&vdev->pdev->dev,
+"Failed to setup ZDEV regions\n");
+ goto disable_exit;
+ }
+ }
+
vfio_pci_probe_mmaps(vdev);
return 0;
diff --git a/drivers/vfio/pci/vfio_pci_private.h
b/drivers/vfio/pci/vfio_pci_private.h
index ee6ee91..08e02f5 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -186,4 +186,14 @@ static inline int vfio_pci_ibm_npu2_init(struct
vfio_pci_device *vdev)
return -ENODEV;
}
#endif
+
+#ifdef CONFIG_VFIO_PCI_ZDEV
+extern int vfio_pci_zdev_init(struct vfio_pci_device *vdev);
+#else
+static inline int vfio_pci_zdev_init(struct vfio_pci_device *vdev)
+{
+ return -ENODEV;
+}
+#endif
+
#endif /* VFIO_PCI_PRIVATE_H */
diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
new file mode 100644
index 000..22e2b60
--- /dev/null
+++ b/drivers/vfio/pci/vfio_pci_zdev.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * VFIO ZPCI devices support
+ *
+ * Copyright (C) IBM Corp. 2019. All rights reserved.
+ * Author: Pierre Morel
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+
+#include "vfio_pci_private.h"
+
+static size_t vfio_pci_zdev_rw(struct vfio_pci_device *vdev,
+ char __user *buf, size_t count, loff_t *ppos,
+ bool iswrite)
+{
+ struct vfio_region_zpci_info *region;
+ struct zpci_dev *zdev;
+ unsigned int index = VFIO_PCI_OFFSET_TO_INDEX(*ppos);
+ loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
+
+ if (!vdev->pdev->bus)
+ return -ENODEV;
+
+ zdev = vdev->pdev->bus->sysdata;
+ if (!zdev)
+ return -ENODEV;
+
+ if (pos >= sizeof(*region) || iswrite)
+ return -EINVAL;
+
+ region = vdev->region[index - VFIO_PCI_NUM_REGIONS].data;
+ region->dasm = zdev->dma_mask;
+ region->start_dma = zdev->start_dma;
+ region->end_dma = zdev->end_dma;
+ region->msi_addr = zdev->msi_addr;
+ region->flags = VFIO_PCI_ZDEV_FLAGS_REFRESH;
+ region->gid = zdev->pfgid;
+ region->mui = zdev->fmb_update;
+ region->noi = zdev->max_msi;
+ memcpy(region->util_str, zdev->util_str, CLP_UTIL_STR_LEN);
+
+ count = min(count, (size_t)(sizeof(*region) - pos));
+ if (copy_to_user(buf, region, count))
+ return -EFAULT;
+
+ return count;
+}
+
+static void vfio_pci_zdev_release(struct vfio_pci_device *vdev,
+ struct vfio_pci_region *region)
+{
+ kfree(region->data);
+}
+
+static const struct vfio_pci_regops vfio_pci_zdev_regops =
