On Wed, 28 Feb 2018 11:43:37 -0500
Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> 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;
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
