Re: [RFC 03/19] s390/zcrypt: new AP matrix bus

[Tony Krowiak]

On 10/16/2017 04:47 AM, Martin Schwidefsky wrote:
> On Fri, 13 Oct 2017 13:38:48 -0400
> Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> wrote:
>
> > Introduces an AP matrix bus. The sysfs location of the
> > AP matrix bus is:
> >
> > /sys/bus/ap_matrix
> >
> > The AP matrix bus will create an AP matrix device that will
> > hold the AP queues reserved for use by KVM guests.
> >
> > Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx>
> > ---
> >   MAINTAINERS                         |    8 +++++
> >   drivers/s390/crypto/Makefile        |    4 +-
> >   drivers/s390/crypto/ap_matrix_bus.c |   52 +++++++++++++++++++++++++++++++++++
> >   drivers/s390/crypto/ap_matrix_bus.h |   15 ++++++++++
> >   4 files changed, 77 insertions(+), 2 deletions(-)
> >   create mode 100644 drivers/s390/crypto/ap_matrix_bus.c
> >   create mode 100644 drivers/s390/crypto/ap_matrix_bus.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ef23cf5..cbd6f7c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11711,6 +11711,14 @@ W:	http://www.ibm.com/developerworks/linux/linux390/
> >   S:	Supported
> >   F:	drivers/s390/crypto/
> >
> > +S390 VFIO AP MATRIX DRIVER
> > +M:	Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx>
> > +L:	linux-s390@xxxxxxxxxxxxxxx
> > +W:	http://www.ibm.com/developerworks/linux/linux390/
> > +S:	Supported
> > +F:	drivers/s390/crypto/ap_matrix_bus.h
> > +F:	drivers/s390/crypto/ap_matrix_bus.c
> > +
> >   S390 ZFCP DRIVER
> >   M:	Steffen Maier <maier@xxxxxxxxxxxxxxxxxx>
> >   M:	Benjamin Block <bblock@xxxxxxxxxxxxxxxxxx>
> > diff --git a/drivers/s390/crypto/Makefile b/drivers/s390/crypto/Makefile
> > index be36f10..87646ca 100644
> > --- a/drivers/s390/crypto/Makefile
> > +++ b/drivers/s390/crypto/Makefile
> > @@ -2,7 +2,7 @@
> >   # S/390 crypto devices
> >   #
> >
> > -ap-objs := ap_bus.o ap_card.o ap_queue.o
> > +ap-objs := ap_bus.o ap_card.o ap_queue.o ap_matrix_bus.o
> >   obj-$(subst m,y,$(CONFIG_ZCRYPT)) += ap.o
> >   # zcrypt_api.o and zcrypt_msgtype*.o depend on ap.o
> >   zcrypt-objs := zcrypt_api.o zcrypt_card.o zcrypt_queue.o
> > @@ -13,4 +13,4 @@ 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
> > +obj-$(CONFIG_PKEY) += pkey.o
> > \ No newline at end of file
> Try to avoid this newline hunk please. It is removed again with patch #5.
Will do.
> > diff --git a/drivers/s390/crypto/ap_matrix_bus.c b/drivers/s390/crypto/ap_matrix_bus.c
> > new file mode 100644
> > index 0000000..fbae175
> > --- /dev/null
> > +++ b/drivers/s390/crypto/ap_matrix_bus.c
> > @@ -0,0 +1,52 @@
> > +/*
> > + * Copyright IBM Corp. 2017
> > + * Author(s): Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx>
> > + *
> > + * Adjunct processor matrix bus.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/device.h>
> > +#include <asm/ap.h>
> > +
> > +#include "ap_matrix_bus.h"
> > +
> > +MODULE_AUTHOR("Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx");
> > +MODULE_DESCRIPTION("AP Matrix, Copyright IBM Corp. 2017");
> > +MODULE_LICENSE("GPL v2");
> > +
> > +#define AP_MATRIX_BUS_NAME "ap_matrix"
> > +
> > +static struct device *ap_matrix_root_device;
> > +
> > +static struct bus_type ap_matrix_bus_type = {
> > +	.name = AP_MATRIX_BUS_NAME,
> > +};
> > +
> > +int __init ap_matrix_init(void)
> > +{
> > +	int ret;
> > +
> > +	ap_matrix_root_device = root_device_register(AP_MATRIX_BUS_NAME);
> > +	ret = PTR_RET(ap_matrix_root_device);
> > +	if (ret)
> > +		goto done;
> > +
> > +	ret = bus_register(&ap_matrix_bus_type);
> > +	if (ret)
> > +		goto bus_reg_err;
> > +
> > +	return 0;
> > +
> > +bus_reg_err:
> > +	root_device_unregister(ap_matrix_root_device);
> > +
> > +done:
> > +	return ret;
> > +}
> > +device_initcall(ap_matrix_init);
> > diff --git a/drivers/s390/crypto/ap_matrix_bus.h b/drivers/s390/crypto/ap_matrix_bus.h
> > new file mode 100644
> > index 0000000..58e116f
> > --- /dev/null
> > +++ b/drivers/s390/crypto/ap_matrix_bus.h
> > @@ -0,0 +1,15 @@
> > +/*
> > + * Adjunct processor matrix bus header file
> > + *
> > + * Copyright IBM Corp. 2017
> > + * Author(s): Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx>
> > + *
> > + * Adjunct processor bus header file.
> > + */
> > +
> > +#ifndef _AP_MATRIX_BUS_H_
> > +#define _AP_MATRIX_BUS_H_
> > +
> > +int ap_matrix_init(void);
> > +
> > +#endif /* _AP_MATRIX_BUS_H_ */
> While I understand the approach to split the patches into meaningful
> parts, there is always a balance between too coarse and too fine granular.
>
> I would merge patch #3, patch #4 and parts of patch #5 that affect
> ap_matrix_bus.c & ap_matrix_bus.h.
Will do.
>