| Message ID | 1519741693-17440-6-git-send-email-akrowiak@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 27/02/2018 15:28, Tony Krowiak wrote: > Introduces a new AP device driver. This device driver > is built on the VFIO mediated device framework. The framework > provides sysfs interfaces that facilitate passthrough > access by guests to devices installed on the linux host. > > The VFIO AP device driver will serve two purposes: > > 1. Provide the interfaces to reserve AP devices for exclusive > use by KVM guests. This is accomplished by unbinding the > devices to be reserved for guest usage from the default AP > device driver and binding them to the VFIO AP device driver. > > 2. Implements the functions, callbacks and sysfs attribute > interfaces required to create one or more VFIO mediated > devices each of which will be used to configure the AP > matrix for a guest and serve as a file descriptor > for facilitating communication between QEMU and the > VFIO AP device driver. > > When the VFIO AP device driver is initialized: > > * It registers with the AP bus for control of type 10 (CEX4 > and newer) AP queue devices. The probe and remove callbacks > will be provided to support the binding/unbinding of > AP queue devices to/from the VFIO AP device driver. > > * Creates a /sys/devices/vfio-ap/matrix device to hold > the APQNs of the AP devices bound to the VFIO > AP device driver and serves as the parent of the > mediated devices created for each guest. > > Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> > --- > MAINTAINERS | 2 + > arch/s390/Kconfig | 8 ++ > arch/s390/configs/default_defconfig | 3 + > arch/s390/configs/gcov_defconfig | 3 + > arch/s390/configs/performance_defconfig | 3 + > arch/s390/defconfig | 3 + > drivers/s390/crypto/Makefile | 4 + > drivers/s390/crypto/vfio_ap_drv.c | 134 +++++++++++++++++++++++++++++++ > drivers/s390/crypto/vfio_ap_private.h | 20 +++++ > include/uapi/linux/vfio.h | 2 + > 10 files changed, 182 insertions(+), 0 deletions(-) > create mode 100644 drivers/s390/crypto/vfio_ap_drv.c > create mode 100644 drivers/s390/crypto/vfio_ap_private.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4acf7c2..a2f232d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11884,6 +11884,8 @@ W: http://www.ibm.com/developerworks/linux/linux390/ > S: Supported > F: arch/s390/include/asm/kvm/kvm-ap.h > F: arch/s390/kvm/kvm-ap.c > +F: drivers/s390/crypto/vfio_ap_drv.c > +F: drivers/s390/crypto/vfio_ap_private.h > > S390 ZFCP DRIVER > M: Steffen Maier <maier@linux.vnet.ibm.com> > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index cbe1d97..9f23caf 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -771,6 +771,14 @@ config VFIO_CCW > To compile this driver as a module, choose M here: the > module will be called vfio_ccw. > > +config VFIO_AP > + def_tristate m > + prompt "Support for virtual Adjunct Processor device interface" The VFIO AP devices are not virtual. What about "VFIO support for AP devices" > + depends on ZCRYPT && VFIO_MDEV_DEVICE > + help > + driver grants access to Adjunct Processor (AP) devices > + via the VFIO mediated device interface. > + > endmenu > > menu "Dump support" > diff --git a/arch/s390/configs/default_defconfig b/arch/s390/configs/default_defconfig > index 5af8458..40fa3f6 100644 > --- a/arch/s390/configs/default_defconfig > +++ b/arch/s390/configs/default_defconfig Not sure that this file belongs to this patch > @@ -719,3 +719,6 @@ CONFIG_APPLDATA_BASE=y > CONFIG_KVM=m > CONFIG_KVM_S390_UCONTROL=y > CONFIG_VHOST_NET=m > +VFIO_MDEV=m > +VFIO_MDEV_DEVICE=m > +CONFIG_VFIO_AP=m What is your goal when modifying this three files? Could you add a comment in the commit message? > diff --git a/arch/s390/configs/gcov_defconfig b/arch/s390/configs/gcov_defconfig > index d52eafe..377d40f 100644 > --- a/arch/s390/configs/gcov_defconfig > +++ b/arch/s390/configs/gcov_defconfig > @@ -659,3 +659,6 @@ CONFIG_APPLDATA_BASE=y > CONFIG_KVM=m > CONFIG_KVM_S390_UCONTROL=y > CONFIG_VHOST_NET=m > +VFIO_MDEV=m > +VFIO_MDEV_DEVICE=m > +CONFIG_VFIO_AP=m > diff --git a/arch/s390/configs/performance_defconfig b/arch/s390/configs/performance_defconfig > index 20ed149..f0c52df 100644 > --- a/arch/s390/configs/performance_defconfig > +++ b/arch/s390/configs/performance_defconfig > @@ -657,3 +657,6 @@ CONFIG_APPLDATA_BASE=y > CONFIG_KVM=m > CONFIG_KVM_S390_UCONTROL=y > CONFIG_VHOST_NET=m > +VFIO_MDEV=m > +VFIO_MDEV_DEVICE=m > +CONFIG_VFIO_AP=m > diff --git a/arch/s390/defconfig b/arch/s390/defconfig > index 46a3178..0996eb7 100644 > --- a/arch/s390/defconfig > +++ b/arch/s390/defconfig > @@ -239,3 +239,6 @@ CONFIG_CRC7=m > # CONFIG_XZ_DEC_ARMTHUMB is not set > # CONFIG_XZ_DEC_SPARC is not set > CONFIG_CMM=m > +VFIO_MDEV=m > +VFIO_MDEV_DEVICE=m > +CONFIG_VFIO_AP=m > diff --git a/drivers/s390/crypto/Makefile b/drivers/s390/crypto/Makefile > index b59af54..48e466e 100644 > --- a/drivers/s390/crypto/Makefile > +++ b/drivers/s390/crypto/Makefile > @@ -15,3 +15,7 @@ obj-$(CONFIG_ZCRYPT) += zcrypt_pcixcc.o zcrypt_cex2a.o zcrypt_cex4.o > # pkey kernel module > pkey-objs := pkey_api.o > obj-$(CONFIG_PKEY) += pkey.o > + > +# adjunct processor matrix > +vfio_ap-objs := vfio_ap_drv.o > +obj-$(CONFIG_VFIO_AP) += vfio_ap.o > diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c > new file mode 100644 > index 0000000..8bb72af > --- /dev/null > +++ b/drivers/s390/crypto/vfio_ap_drv.c > @@ -0,0 +1,134 @@ > +/* > + * VFIO based AP device driver > + * > + * Copyright IBM Corp. 2017 > + * > + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com> > + */ > + > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/slab.h> > + > +#include "ap_bus.h" Since you do it in a later patch, may be put this include of ap_bus.h in ap_private.h now. > +#include "vfio_ap_private.h" > + > +#define VFIO_AP_ROOT_NAME "vfio_ap" > +#define VFIO_AP_DEV_TYPE_NAME "ap_matrix" > +#define VFIO_AP_DEV_NAME "matrix" > + > +MODULE_AUTHOR("IBM Corporation"); > +MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017"); > +MODULE_LICENSE("GPL v2"); > + > +static struct device *vfio_ap_root_device; > + > +static struct ap_driver vfio_ap_drv; > + > +static struct ap_matrix *ap_matrix; > + > +static struct device_type vfio_ap_dev_type = { > + .name = VFIO_AP_DEV_TYPE_NAME, > +}; > + > +/* Only type 10 adapters (CEX4 and later) are supported > + * by the AP matrix device driver > + */ > +static struct ap_device_id ap_queue_ids[] = { > + { .dev_type = AP_DEVICE_TYPE_CEX4, > + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, > + { .dev_type = AP_DEVICE_TYPE_CEX5, > + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, > + { .dev_type = AP_DEVICE_TYPE_CEX6, > + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, > + { /* end of sibling */ }, > +}; > + > +MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids); > + > +static int vfio_ap_queue_dev_probe(struct ap_device *apdev) > +{ > + return 0; > +} > + > +static void vfio_ap_matrix_dev_release(struct device *dev) > +{ > + struct ap_matrix *ap_matrix = dev_get_drvdata(dev); > + > + kfree(ap_matrix); > +} > + > +static int vfio_ap_matrix_dev_create(void) > +{ > + int ret; > + > + vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME); > + > + ret = PTR_ERR_OR_ZERO(vfio_ap_root_device); IS_ERR() is enough, root_device_register() never return NULL. > + if (ret) > + goto done; > + > + ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL); > + if (!ap_matrix) { > + ret = -ENOMEM; > + goto matrix_alloc_err; > + } > + > + ap_matrix->device.type = &vfio_ap_dev_type; > + dev_set_name(&ap_matrix->device, "%s", VFIO_AP_DEV_NAME); > + ap_matrix->device.parent = vfio_ap_root_device; > + ap_matrix->device.release = vfio_ap_matrix_dev_release; > + ap_matrix->device.driver = &vfio_ap_drv.driver; > + > + ret = device_register(&ap_matrix->device); > + if (ret) > + goto matrix_reg_err; > + > + goto done; > + > +matrix_reg_err: > + put_device(&ap_matrix->device); > + kfree(ap_matrix); > + > +matrix_alloc_err: > + root_device_unregister(vfio_ap_root_device); > + > +done: > + return ret; > +} > + > +static void vfio_ap_matrix_dev_destroy(struct ap_matrix *ap_matrix) > +{ > + device_unregister(&ap_matrix->device); > + root_device_unregister(vfio_ap_root_device); > +} > + > +int __init vfio_ap_init(void) > +{ > + int ret; > + > + ret = vfio_ap_matrix_dev_create(); > + if (ret) > + return ret; > + > + memset(&vfio_ap_drv, 0, sizeof(vfio_ap_drv)); > + vfio_ap_drv.probe = vfio_ap_queue_dev_probe; > + vfio_ap_drv.ids = ap_queue_ids; > + > + ret = ap_driver_register(&vfio_ap_drv, THIS_MODULE, VFIO_AP_DRV_NAME); > + if (ret) { > + vfio_ap_matrix_dev_destroy(ap_matrix); > + return ret; > + } > + > + return 0; > +} > + > +void __exit vfio_ap_exit(void) > +{ > + ap_driver_unregister(&vfio_ap_drv); > + vfio_ap_matrix_dev_destroy(ap_matrix); > +} > + > +module_init(vfio_ap_init); > +module_exit(vfio_ap_exit); > diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h > new file mode 100644 > index 0000000..3505947 > --- /dev/null > +++ b/drivers/s390/crypto/vfio_ap_private.h > @@ -0,0 +1,20 @@ > +/* > + * Private data and functions for adjunct processor VFIO matrix driver. > + * > + * Copyright IBM Corp. 2017 > + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com> > + */ > + > +#ifndef _VFIO_AP_PRIVATE_H_ > +#define _VFIO_AP_PRIVATE_H_ > + > +#include <linux/types.h> > + > +#define VFIO_AP_MODULE_NAME "vfio_ap" > +#define VFIO_AP_DRV_NAME "vfio_ap" > + > +struct ap_matrix { > + struct device device; > +}; > + > +#endif /* _VFIO_AP_PRIVATE_H_ */ > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index e3301db..cf2a5e9 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -200,6 +200,7 @@ struct vfio_device_info { > #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ > #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */ > #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */ > +#define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */ > __u32 num_regions; /* Max region index + 1 */ > __u32 num_irqs; /* Max IRQ index + 1 */ > }; > @@ -215,6 +216,7 @@ struct vfio_device_info { > #define VFIO_DEVICE_API_PLATFORM_STRING "vfio-platform" > #define VFIO_DEVICE_API_AMBA_STRING "vfio-amba" > #define VFIO_DEVICE_API_CCW_STRING "vfio-ccw" > +#define VFIO_DEVICE_API_AP_STRING "vfio-ap" > > /** > * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
On 02/28/2018 10:33 AM, Pierre Morel wrote: > On 27/02/2018 15:28, Tony Krowiak wrote: >> Introduces a new AP device driver. This device driver >> is built on the VFIO mediated device framework. The framework >> provides sysfs interfaces that facilitate passthrough >> access by guests to devices installed on the linux host. >> >> The VFIO AP device driver will serve two purposes: >> >> 1. Provide the interfaces to reserve AP devices for exclusive >> use by KVM guests. This is accomplished by unbinding the >> devices to be reserved for guest usage from the default AP >> device driver and binding them to the VFIO AP device driver. >> >> 2. Implements the functions, callbacks and sysfs attribute >> interfaces required to create one or more VFIO mediated >> devices each of which will be used to configure the AP >> matrix for a guest and serve as a file descriptor >> for facilitating communication between QEMU and the >> VFIO AP device driver. >> >> When the VFIO AP device driver is initialized: >> >> * It registers with the AP bus for control of type 10 (CEX4 >> and newer) AP queue devices. The probe and remove callbacks >> will be provided to support the binding/unbinding of >> AP queue devices to/from the VFIO AP device driver. >> >> * Creates a /sys/devices/vfio-ap/matrix device to hold >> the APQNs of the AP devices bound to the VFIO >> AP device driver and serves as the parent of the >> mediated devices created for each guest. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> >> --- >> MAINTAINERS | 2 + >> arch/s390/Kconfig | 8 ++ >> arch/s390/configs/default_defconfig | 3 + >> arch/s390/configs/gcov_defconfig | 3 + >> arch/s390/configs/performance_defconfig | 3 + >> arch/s390/defconfig | 3 + >> drivers/s390/crypto/Makefile | 4 + >> drivers/s390/crypto/vfio_ap_drv.c | 134 >> +++++++++++++++++++++++++++++++ >> drivers/s390/crypto/vfio_ap_private.h | 20 +++++ >> include/uapi/linux/vfio.h | 2 + >> 10 files changed, 182 insertions(+), 0 deletions(-) >> create mode 100644 drivers/s390/crypto/vfio_ap_drv.c >> create mode 100644 drivers/s390/crypto/vfio_ap_private.h >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 4acf7c2..a2f232d 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -11884,6 +11884,8 @@ W: >> http://www.ibm.com/developerworks/linux/linux390/ >> S: Supported >> F: arch/s390/include/asm/kvm/kvm-ap.h >> F: arch/s390/kvm/kvm-ap.c >> +F: drivers/s390/crypto/vfio_ap_drv.c >> +F: drivers/s390/crypto/vfio_ap_private.h >> >> S390 ZFCP DRIVER >> M: Steffen Maier <maier@linux.vnet.ibm.com> >> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig >> index cbe1d97..9f23caf 100644 >> --- a/arch/s390/Kconfig >> +++ b/arch/s390/Kconfig >> @@ -771,6 +771,14 @@ config VFIO_CCW >> To compile this driver as a module, choose M here: the >> module will be called vfio_ccw. >> >> +config VFIO_AP >> + def_tristate m >> + prompt "Support for virtual Adjunct Processor device interface" > > The VFIO AP devices are not virtual. > What about > "VFIO support for AP devices" Sounds good. > >> + depends on ZCRYPT && VFIO_MDEV_DEVICE >> + help >> + driver grants access to Adjunct Processor (AP) devices >> + via the VFIO mediated device interface. >> + >> endmenu >> >> menu "Dump support" >> diff --git a/arch/s390/configs/default_defconfig >> b/arch/s390/configs/default_defconfig >> index 5af8458..40fa3f6 100644 >> --- a/arch/s390/configs/default_defconfig >> +++ b/arch/s390/configs/default_defconfig > > Not sure that this file belongs to this patch Neither am I, but at the time I inserted this here - well before August of last year - I was using vfio-ccw as a model. If someone can verify this does not belong here, I'd be more than happy to remove it. > > >> @@ -719,3 +719,6 @@ CONFIG_APPLDATA_BASE=y >> CONFIG_KVM=m >> CONFIG_KVM_S390_UCONTROL=y >> CONFIG_VHOST_NET=m >> +VFIO_MDEV=m >> +VFIO_MDEV_DEVICE=m >> +CONFIG_VFIO_AP=m > > What is your goal when modifying this three files? > Could you add a comment in the commit message? As stated above, this was originally based on the vfio-ccw model and has been in the patch series since its inception. I'd be happy to remove it if it is not necessary. > > > >> diff --git a/arch/s390/configs/gcov_defconfig >> b/arch/s390/configs/gcov_defconfig >> index d52eafe..377d40f 100644 >> --- a/arch/s390/configs/gcov_defconfig >> +++ b/arch/s390/configs/gcov_defconfig >> @@ -659,3 +659,6 @@ CONFIG_APPLDATA_BASE=y >> CONFIG_KVM=m >> CONFIG_KVM_S390_UCONTROL=y >> CONFIG_VHOST_NET=m >> +VFIO_MDEV=m >> +VFIO_MDEV_DEVICE=m >> +CONFIG_VFIO_AP=m >> diff --git a/arch/s390/configs/performance_defconfig >> b/arch/s390/configs/performance_defconfig >> index 20ed149..f0c52df 100644 >> --- a/arch/s390/configs/performance_defconfig >> +++ b/arch/s390/configs/performance_defconfig >> @@ -657,3 +657,6 @@ CONFIG_APPLDATA_BASE=y >> CONFIG_KVM=m >> CONFIG_KVM_S390_UCONTROL=y >> CONFIG_VHOST_NET=m >> +VFIO_MDEV=m >> +VFIO_MDEV_DEVICE=m >> +CONFIG_VFIO_AP=m >> diff --git a/arch/s390/defconfig b/arch/s390/defconfig >> index 46a3178..0996eb7 100644 >> --- a/arch/s390/defconfig >> +++ b/arch/s390/defconfig >> @@ -239,3 +239,6 @@ CONFIG_CRC7=m >> # CONFIG_XZ_DEC_ARMTHUMB is not set >> # CONFIG_XZ_DEC_SPARC is not set >> CONFIG_CMM=m >> +VFIO_MDEV=m >> +VFIO_MDEV_DEVICE=m >> +CONFIG_VFIO_AP=m >> diff --git a/drivers/s390/crypto/Makefile b/drivers/s390/crypto/Makefile >> index b59af54..48e466e 100644 >> --- a/drivers/s390/crypto/Makefile >> +++ b/drivers/s390/crypto/Makefile >> @@ -15,3 +15,7 @@ obj-$(CONFIG_ZCRYPT) += zcrypt_pcixcc.o >> zcrypt_cex2a.o zcrypt_cex4.o >> # pkey kernel module >> pkey-objs := pkey_api.o >> obj-$(CONFIG_PKEY) += pkey.o >> + >> +# adjunct processor matrix >> +vfio_ap-objs := vfio_ap_drv.o >> +obj-$(CONFIG_VFIO_AP) += vfio_ap.o >> diff --git a/drivers/s390/crypto/vfio_ap_drv.c >> b/drivers/s390/crypto/vfio_ap_drv.c >> new file mode 100644 >> index 0000000..8bb72af >> --- /dev/null >> +++ b/drivers/s390/crypto/vfio_ap_drv.c >> @@ -0,0 +1,134 @@ >> +/* >> + * VFIO based AP device driver >> + * >> + * Copyright IBM Corp. 2017 >> + * >> + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com> >> + */ >> + >> +#include <linux/module.h> >> +#include <linux/mod_devicetable.h> >> +#include <linux/slab.h> >> + >> +#include "ap_bus.h" > > Since you do it in a later patch, may be put this include of ap_bus.h > in ap_private.h now. Okay > > >> +#include "vfio_ap_private.h" >> + >> +#define VFIO_AP_ROOT_NAME "vfio_ap" >> +#define VFIO_AP_DEV_TYPE_NAME "ap_matrix" >> +#define VFIO_AP_DEV_NAME "matrix" >> + >> +MODULE_AUTHOR("IBM Corporation"); >> +MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017"); >> +MODULE_LICENSE("GPL v2"); >> + >> +static struct device *vfio_ap_root_device; >> + >> +static struct ap_driver vfio_ap_drv; >> + >> +static struct ap_matrix *ap_matrix; >> + >> +static struct device_type vfio_ap_dev_type = { >> + .name = VFIO_AP_DEV_TYPE_NAME, >> +}; >> + >> +/* Only type 10 adapters (CEX4 and later) are supported >> + * by the AP matrix device driver >> + */ >> +static struct ap_device_id ap_queue_ids[] = { >> + { .dev_type = AP_DEVICE_TYPE_CEX4, >> + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, >> + { .dev_type = AP_DEVICE_TYPE_CEX5, >> + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, >> + { .dev_type = AP_DEVICE_TYPE_CEX6, >> + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, >> + { /* end of sibling */ }, >> +}; >> + >> +MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids); >> + >> +static int vfio_ap_queue_dev_probe(struct ap_device *apdev) >> +{ >> + return 0; >> +} >> + >> +static void vfio_ap_matrix_dev_release(struct device *dev) >> +{ >> + struct ap_matrix *ap_matrix = dev_get_drvdata(dev); >> + >> + kfree(ap_matrix); >> +} >> + >> +static int vfio_ap_matrix_dev_create(void) >> +{ >> + int ret; >> + >> + vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME); >> + >> + ret = PTR_ERR_OR_ZERO(vfio_ap_root_device); > > IS_ERR() is enough, root_device_register() never return NULL. I searched the kernel code to look at other places the root_device_register() function is called to see how the return value is handled. I've seen all of the following used: if (IS_ERR()) ret = PTR_ERR() PTR_ERR() PTR_ERR_OR_ZERO() I'm not sure why this is a concern, but I'll use the first option above since PTR_ERR_OR_ZERO() also embeds the first option. >> + if (ret) >> + goto done; >> + >> + ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL); >> + if (!ap_matrix) { >> + ret = -ENOMEM; >> + goto matrix_alloc_err; >> + } >> + >> + ap_matrix->device.type = &vfio_ap_dev_type; >> + dev_set_name(&ap_matrix->device, "%s", VFIO_AP_DEV_NAME); >> + ap_matrix->device.parent = vfio_ap_root_device; >> + ap_matrix->device.release = vfio_ap_matrix_dev_release; >> + ap_matrix->device.driver = &vfio_ap_drv.driver; >> + >> + ret = device_register(&ap_matrix->device); >> + if (ret) >> + goto matrix_reg_err; >> + >> + goto done; >> + >> +matrix_reg_err: >> + put_device(&ap_matrix->device); >> + kfree(ap_matrix); >> + >> +matrix_alloc_err: >> + root_device_unregister(vfio_ap_root_device); >> + >> +done: >> + return ret; >> +} >> + >> +static void vfio_ap_matrix_dev_destroy(struct ap_matrix *ap_matrix) >> +{ >> + device_unregister(&ap_matrix->device); >> + root_device_unregister(vfio_ap_root_device); >> +} >> + >> +int __init vfio_ap_init(void) >> +{ >> + int ret; >> + >> + ret = vfio_ap_matrix_dev_create(); >> + if (ret) >> + return ret; >> + >> + memset(&vfio_ap_drv, 0, sizeof(vfio_ap_drv)); >> + vfio_ap_drv.probe = vfio_ap_queue_dev_probe; >> + vfio_ap_drv.ids = ap_queue_ids; >> + >> + ret = ap_driver_register(&vfio_ap_drv, THIS_MODULE, >> VFIO_AP_DRV_NAME); >> + if (ret) { >> + vfio_ap_matrix_dev_destroy(ap_matrix); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +void __exit vfio_ap_exit(void) >> +{ >> + ap_driver_unregister(&vfio_ap_drv); >> + vfio_ap_matrix_dev_destroy(ap_matrix); >> +} >> + >> +module_init(vfio_ap_init); >> +module_exit(vfio_ap_exit); >> diff --git a/drivers/s390/crypto/vfio_ap_private.h >> b/drivers/s390/crypto/vfio_ap_private.h >> new file mode 100644 >> index 0000000..3505947 >> --- /dev/null >> +++ b/drivers/s390/crypto/vfio_ap_private.h >> @@ -0,0 +1,20 @@ >> +/* >> + * Private data and functions for adjunct processor VFIO matrix driver. >> + * >> + * Copyright IBM Corp. 2017 >> + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com> >> + */ >> + >> +#ifndef _VFIO_AP_PRIVATE_H_ >> +#define _VFIO_AP_PRIVATE_H_ >> + >> +#include <linux/types.h> >> + >> +#define VFIO_AP_MODULE_NAME "vfio_ap" >> +#define VFIO_AP_DRV_NAME "vfio_ap" >> + >> +struct ap_matrix { >> + struct device device; >> +}; >> + >> +#endif /* _VFIO_AP_PRIVATE_H_ */ >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h >> index e3301db..cf2a5e9 100644 >> --- a/include/uapi/linux/vfio.h >> +++ b/include/uapi/linux/vfio.h >> @@ -200,6 +200,7 @@ struct vfio_device_info { >> #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform >> device */ >> #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */ >> #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */ >> +#define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */ >> __u32 num_regions; /* Max region index + 1 */ >> __u32 num_irqs; /* Max IRQ index + 1 */ >> }; >> @@ -215,6 +216,7 @@ struct vfio_device_info { >> #define VFIO_DEVICE_API_PLATFORM_STRING "vfio-platform" >> #define VFIO_DEVICE_API_AMBA_STRING "vfio-amba" >> #define VFIO_DEVICE_API_CCW_STRING "vfio-ccw" >> +#define VFIO_DEVICE_API_AP_STRING "vfio-ap" >> >> /** >> * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8, > >
On Wed, 28 Feb 2018 11:43:37 -0500 Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: > On 02/28/2018 10:33 AM, Pierre Morel wrote: > > On 27/02/2018 15:28, Tony Krowiak wrote: (...) > >> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > >> index cbe1d97..9f23caf 100644 > >> --- a/arch/s390/Kconfig > >> +++ b/arch/s390/Kconfig > >> @@ -771,6 +771,14 @@ config VFIO_CCW > >> To compile this driver as a module, choose M here: the > >> module will be called vfio_ccw. > >> > >> +config VFIO_AP > >> + def_tristate m Any reason you default to m instead of n here? > >> + prompt "Support for virtual Adjunct Processor device interface" > > > > The VFIO AP devices are not virtual. > > What about > > "VFIO support for AP devices" > Sounds good. +1 > > > >> + depends on ZCRYPT && VFIO_MDEV_DEVICE > >> + help > >> + driver grants access to Adjunct Processor (AP) devices s/driver/This driver/ > >> + via the VFIO mediated device interface. You also might want to add "To compile this driver as a module, choose M here: the module will be called..." > >> + > >> endmenu It's a tad confusing to find this in the I/O submenu, but I don't really have a better idea. > >> > >> menu "Dump support" > >> diff --git a/arch/s390/configs/default_defconfig > >> b/arch/s390/configs/default_defconfig > >> index 5af8458..40fa3f6 100644 > >> --- a/arch/s390/configs/default_defconfig > >> +++ b/arch/s390/configs/default_defconfig > > > > Not sure that this file belongs to this patch > Neither am I, but at the time I inserted this here - well before August > of last year - I was using vfio-ccw as a model. > If someone can verify this does not belong here, I'd be more than happy > to remove it. I don't see any entry for VFIO_CCW in there? > > > > > >> @@ -719,3 +719,6 @@ CONFIG_APPLDATA_BASE=y > >> CONFIG_KVM=m > >> CONFIG_KVM_S390_UCONTROL=y > >> CONFIG_VHOST_NET=m > >> +VFIO_MDEV=m > >> +VFIO_MDEV_DEVICE=m > >> +CONFIG_VFIO_AP=m > > > > What is your goal when modifying this three files? > > Could you add a comment in the commit message? > As stated above, this was originally based on the vfio-ccw model and has > been in the > patch series since its inception. I'd be happy to remove it if it is not > necessary. I'd vote for removing it. (...) > >> +static int vfio_ap_matrix_dev_create(void) > >> +{ > >> + int ret; > >> + > >> + vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME); > >> + > >> + ret = PTR_ERR_OR_ZERO(vfio_ap_root_device); > > > > IS_ERR() is enough, root_device_register() never return NULL. > I searched the kernel code to look at other places the > root_device_register() > function is called to see how the return value is handled. I've seen all > of the > following used: > if (IS_ERR()) > ret = PTR_ERR() > PTR_ERR() > PTR_ERR_OR_ZERO() > > I'm not sure why this is a concern, but I'll use the first option above > since PTR_ERR_OR_ZERO() also embeds the first option. PTR_ERR_OR_ZERO() seems like the best choice for the way the return code is processed here. (It's just unfortunate that its name conjures up connotations of NULL-pointer handling.) > >> + if (ret) > >> + goto done;
On 02/28/2018 01:10 PM, Cornelia Huck wrote: > On Wed, 28 Feb 2018 11:43:37 -0500 > Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote: > >> On 02/28/2018 10:33 AM, Pierre Morel wrote: >>> On 27/02/2018 15:28, Tony Krowiak wrote: > (...) > >>>> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig >>>> index cbe1d97..9f23caf 100644 >>>> --- a/arch/s390/Kconfig >>>> +++ b/arch/s390/Kconfig >>>> @@ -771,6 +771,14 @@ config VFIO_CCW >>>> To compile this driver as a module, choose M here: the >>>> module will be called vfio_ccw. >>>> >>>> +config VFIO_AP >>>> + def_tristate m > Any reason you default to m instead of n here? None in particular, is there a good reason to change this to n? > >>>> + prompt "Support for virtual Adjunct Processor device interface" >>> The VFIO AP devices are not virtual. >>> What about >>> "VFIO support for AP devices" >> Sounds good. > +1 > >>> >>>> + depends on ZCRYPT && VFIO_MDEV_DEVICE >>>> + help >>>> + driver grants access to Adjunct Processor (AP) devices > s/driver/This driver/ Okay, I'll make the change > >>>> + via the VFIO mediated device interface. > You also might want to add > > "To compile this driver as a module, choose M here: the > module will be called..." Will do > >>>> + >>>> endmenu > It's a tad confusing to find this in the I/O submenu, but I don't > really have a better idea. I wasn't sure either, but couldn't think of a better location. Anybody else have any ideas? > >>>> menu "Dump support" >>>> diff --git a/arch/s390/configs/default_defconfig >>>> b/arch/s390/configs/default_defconfig >>>> index 5af8458..40fa3f6 100644 >>>> --- a/arch/s390/configs/default_defconfig >>>> +++ b/arch/s390/configs/default_defconfig >>> Not sure that this file belongs to this patch >> Neither am I, but at the time I inserted this here - well before August >> of last year - I was using vfio-ccw as a model. >> If someone can verify this does not belong here, I'd be more than happy >> to remove it. > I don't see any entry for VFIO_CCW in there? There isn't now, but there was at the time I first started working on this. My first pass was a lot of cut-and-paste followed by modification of the vfio_ccw implementation, but that was long ago. This is a remnant of that time. Like I said, I'd be happy to remove this. > >>> >>>> @@ -719,3 +719,6 @@ CONFIG_APPLDATA_BASE=y >>>> CONFIG_KVM=m >>>> CONFIG_KVM_S390_UCONTROL=y >>>> CONFIG_VHOST_NET=m >>>> +VFIO_MDEV=m >>>> +VFIO_MDEV_DEVICE=m >>>> +CONFIG_VFIO_AP=m >>> What is your goal when modifying this three files? >>> Could you add a comment in the commit message? >> As stated above, this was originally based on the vfio-ccw model and has >> been in the >> patch series since its inception. I'd be happy to remove it if it is not >> necessary. > I'd vote for removing it. Consider them gone. > > (...) > >>>> +static int vfio_ap_matrix_dev_create(void) >>>> +{ >>>> + int ret; >>>> + >>>> + vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME); >>>> + >>>> + ret = PTR_ERR_OR_ZERO(vfio_ap_root_device); >>> IS_ERR() is enough, root_device_register() never return NULL. >> I searched the kernel code to look at other places the >> root_device_register() >> function is called to see how the return value is handled. I've seen all >> of the >> following used: >> if (IS_ERR()) >> ret = PTR_ERR() >> PTR_ERR() >> PTR_ERR_OR_ZERO() >> >> I'm not sure why this is a concern, but I'll use the first option above >> since PTR_ERR_OR_ZERO() also embeds the first option. > PTR_ERR_OR_ZERO() seems like the best choice for the way the return > code is processed here. (It's just unfortunate that its name conjures > up connotations of NULL-pointer handling.) I changed it to: ret = IS_ERR(vfio_ap_root_device); if (ret) { ret = PTR_ERR(vfio_ap_root_device); goto done; } Hopefully everybody is happy with this. > >>>> + if (ret) >>>> + goto done;
diff --git a/MAINTAINERS b/MAINTAINERS index 4acf7c2..a2f232d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11884,6 +11884,8 @@ W: http://www.ibm.com/developerworks/linux/linux390/ S: Supported F: arch/s390/include/asm/kvm/kvm-ap.h F: arch/s390/kvm/kvm-ap.c +F: drivers/s390/crypto/vfio_ap_drv.c +F: drivers/s390/crypto/vfio_ap_private.h S390 ZFCP DRIVER M: Steffen Maier <maier@linux.vnet.ibm.com> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index cbe1d97..9f23caf 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -771,6 +771,14 @@ config VFIO_CCW To compile this driver as a module, choose M here: the module will be called vfio_ccw. +config VFIO_AP + def_tristate m + prompt "Support for virtual Adjunct Processor device interface" + depends on ZCRYPT && VFIO_MDEV_DEVICE + help + driver grants access to Adjunct Processor (AP) devices + via the VFIO mediated device interface. + endmenu menu "Dump support" diff --git a/arch/s390/configs/default_defconfig b/arch/s390/configs/default_defconfig index 5af8458..40fa3f6 100644 --- a/arch/s390/configs/default_defconfig +++ b/arch/s390/configs/default_defconfig @@ -719,3 +719,6 @@ CONFIG_APPLDATA_BASE=y CONFIG_KVM=m CONFIG_KVM_S390_UCONTROL=y CONFIG_VHOST_NET=m +VFIO_MDEV=m +VFIO_MDEV_DEVICE=m +CONFIG_VFIO_AP=m diff --git a/arch/s390/configs/gcov_defconfig b/arch/s390/configs/gcov_defconfig index d52eafe..377d40f 100644 --- a/arch/s390/configs/gcov_defconfig +++ b/arch/s390/configs/gcov_defconfig @@ -659,3 +659,6 @@ CONFIG_APPLDATA_BASE=y CONFIG_KVM=m CONFIG_KVM_S390_UCONTROL=y CONFIG_VHOST_NET=m +VFIO_MDEV=m +VFIO_MDEV_DEVICE=m +CONFIG_VFIO_AP=m diff --git a/arch/s390/configs/performance_defconfig b/arch/s390/configs/performance_defconfig index 20ed149..f0c52df 100644 --- a/arch/s390/configs/performance_defconfig +++ b/arch/s390/configs/performance_defconfig @@ -657,3 +657,6 @@ CONFIG_APPLDATA_BASE=y CONFIG_KVM=m CONFIG_KVM_S390_UCONTROL=y CONFIG_VHOST_NET=m +VFIO_MDEV=m +VFIO_MDEV_DEVICE=m +CONFIG_VFIO_AP=m diff --git a/arch/s390/defconfig b/arch/s390/defconfig index 46a3178..0996eb7 100644 --- a/arch/s390/defconfig +++ b/arch/s390/defconfig @@ -239,3 +239,6 @@ CONFIG_CRC7=m # CONFIG_XZ_DEC_ARMTHUMB is not set # CONFIG_XZ_DEC_SPARC is not set CONFIG_CMM=m +VFIO_MDEV=m +VFIO_MDEV_DEVICE=m +CONFIG_VFIO_AP=m diff --git a/drivers/s390/crypto/Makefile b/drivers/s390/crypto/Makefile index b59af54..48e466e 100644 --- a/drivers/s390/crypto/Makefile +++ b/drivers/s390/crypto/Makefile @@ -15,3 +15,7 @@ obj-$(CONFIG_ZCRYPT) += zcrypt_pcixcc.o zcrypt_cex2a.o zcrypt_cex4.o # pkey kernel module pkey-objs := pkey_api.o obj-$(CONFIG_PKEY) += pkey.o + +# adjunct processor matrix +vfio_ap-objs := vfio_ap_drv.o +obj-$(CONFIG_VFIO_AP) += vfio_ap.o diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c new file mode 100644 index 0000000..8bb72af --- /dev/null +++ b/drivers/s390/crypto/vfio_ap_drv.c @@ -0,0 +1,134 @@ +/* + * VFIO based AP device driver + * + * Copyright IBM Corp. 2017 + * + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com> + */ + +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/slab.h> + +#include "ap_bus.h" +#include "vfio_ap_private.h" + +#define VFIO_AP_ROOT_NAME "vfio_ap" +#define VFIO_AP_DEV_TYPE_NAME "ap_matrix" +#define VFIO_AP_DEV_NAME "matrix" + +MODULE_AUTHOR("IBM Corporation"); +MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017"); +MODULE_LICENSE("GPL v2"); + +static struct device *vfio_ap_root_device; + +static struct ap_driver vfio_ap_drv; + +static struct ap_matrix *ap_matrix; + +static struct device_type vfio_ap_dev_type = { + .name = VFIO_AP_DEV_TYPE_NAME, +}; + +/* Only type 10 adapters (CEX4 and later) are supported + * by the AP matrix device driver + */ +static struct ap_device_id ap_queue_ids[] = { + { .dev_type = AP_DEVICE_TYPE_CEX4, + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, + { .dev_type = AP_DEVICE_TYPE_CEX5, + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, + { .dev_type = AP_DEVICE_TYPE_CEX6, + .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE }, + { /* end of sibling */ }, +}; + +MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids); + +static int vfio_ap_queue_dev_probe(struct ap_device *apdev) +{ + return 0; +} + +static void vfio_ap_matrix_dev_release(struct device *dev) +{ + struct ap_matrix *ap_matrix = dev_get_drvdata(dev); + + kfree(ap_matrix); +} + +static int vfio_ap_matrix_dev_create(void) +{ + int ret; + + vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME); + + ret = PTR_ERR_OR_ZERO(vfio_ap_root_device); + if (ret) + goto done; + + ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL); + if (!ap_matrix) { + ret = -ENOMEM; + goto matrix_alloc_err; + } + + ap_matrix->device.type = &vfio_ap_dev_type; + dev_set_name(&ap_matrix->device, "%s", VFIO_AP_DEV_NAME); + ap_matrix->device.parent = vfio_ap_root_device; + ap_matrix->device.release = vfio_ap_matrix_dev_release; + ap_matrix->device.driver = &vfio_ap_drv.driver; + + ret = device_register(&ap_matrix->device); + if (ret) + goto matrix_reg_err; + + goto done; + +matrix_reg_err: + put_device(&ap_matrix->device); + kfree(ap_matrix); + +matrix_alloc_err: + root_device_unregister(vfio_ap_root_device); + +done: + return ret; +} + +static void vfio_ap_matrix_dev_destroy(struct ap_matrix *ap_matrix) +{ + device_unregister(&ap_matrix->device); + root_device_unregister(vfio_ap_root_device); +} + +int __init vfio_ap_init(void) +{ + int ret; + + ret = vfio_ap_matrix_dev_create(); + if (ret) + return ret; + + memset(&vfio_ap_drv, 0, sizeof(vfio_ap_drv)); + vfio_ap_drv.probe = vfio_ap_queue_dev_probe; + vfio_ap_drv.ids = ap_queue_ids; + + ret = ap_driver_register(&vfio_ap_drv, THIS_MODULE, VFIO_AP_DRV_NAME); + if (ret) { + vfio_ap_matrix_dev_destroy(ap_matrix); + return ret; + } + + return 0; +} + +void __exit vfio_ap_exit(void) +{ + ap_driver_unregister(&vfio_ap_drv); + vfio_ap_matrix_dev_destroy(ap_matrix); +} + +module_init(vfio_ap_init); +module_exit(vfio_ap_exit); diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h new file mode 100644 index 0000000..3505947 --- /dev/null +++ b/drivers/s390/crypto/vfio_ap_private.h @@ -0,0 +1,20 @@ +/* + * Private data and functions for adjunct processor VFIO matrix driver. + * + * Copyright IBM Corp. 2017 + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com> + */ + +#ifndef _VFIO_AP_PRIVATE_H_ +#define _VFIO_AP_PRIVATE_H_ + +#include <linux/types.h> + +#define VFIO_AP_MODULE_NAME "vfio_ap" +#define VFIO_AP_DRV_NAME "vfio_ap" + +struct ap_matrix { + struct device device; +}; + +#endif /* _VFIO_AP_PRIVATE_H_ */ diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index e3301db..cf2a5e9 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -200,6 +200,7 @@ struct vfio_device_info { #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ #define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */ #define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */ +#define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */ __u32 num_regions; /* Max region index + 1 */ __u32 num_irqs; /* Max IRQ index + 1 */ }; @@ -215,6 +216,7 @@ struct vfio_device_info { #define VFIO_DEVICE_API_PLATFORM_STRING "vfio-platform" #define VFIO_DEVICE_API_AMBA_STRING "vfio-amba" #define VFIO_DEVICE_API_CCW_STRING "vfio-ccw" +#define VFIO_DEVICE_API_AP_STRING "vfio-ap" /** * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
Introduces a new AP device driver. This device driver is built on the VFIO mediated device framework. The framework provides sysfs interfaces that facilitate passthrough access by guests to devices installed on the linux host. The VFIO AP device driver will serve two purposes: 1. Provide the interfaces to reserve AP devices for exclusive use by KVM guests. This is accomplished by unbinding the devices to be reserved for guest usage from the default AP device driver and binding them to the VFIO AP device driver. 2. Implements the functions, callbacks and sysfs attribute interfaces required to create one or more VFIO mediated devices each of which will be used to configure the AP matrix for a guest and serve as a file descriptor for facilitating communication between QEMU and the VFIO AP device driver. When the VFIO AP device driver is initialized: * It registers with the AP bus for control of type 10 (CEX4 and newer) AP queue devices. The probe and remove callbacks will be provided to support the binding/unbinding of AP queue devices to/from the VFIO AP device driver. * Creates a /sys/devices/vfio-ap/matrix device to hold the APQNs of the AP devices bound to the VFIO AP device driver and serves as the parent of the mediated devices created for each guest. Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> --- MAINTAINERS | 2 + arch/s390/Kconfig | 8 ++ arch/s390/configs/default_defconfig | 3 + arch/s390/configs/gcov_defconfig | 3 + arch/s390/configs/performance_defconfig | 3 + arch/s390/defconfig | 3 + drivers/s390/crypto/Makefile | 4 + drivers/s390/crypto/vfio_ap_drv.c | 134 +++++++++++++++++++++++++++++++ drivers/s390/crypto/vfio_ap_private.h | 20 +++++ include/uapi/linux/vfio.h | 2 + 10 files changed, 182 insertions(+), 0 deletions(-) create mode 100644 drivers/s390/crypto/vfio_ap_drv.c create mode 100644 drivers/s390/crypto/vfio_ap_private.h