On 02/28/2018 01:10 PM, Cornelia Huck wrote:
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?
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;
