Re: [PATCH v7 09/22] s390: vfio-ap: register matrix device with VFIO mdev framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 30/07/2018 13:28, Cornelia Huck wrote:
On Mon, 30 Jul 2018 13:24:24 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:

On 30/07/2018 11:49, Cornelia Huck wrote:
On Fri, 27 Jul 2018 16:17:47 +0200
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
On 07/26/2018 09:54 PM, Christian Borntraeger wrote:
@@ -65,6 +66,21 @@ static int vfio_ap_matrix_dev_create(void)
    {
    	int ret;
+ mutex_init(&matrix_dev.lock);
+	INIT_LIST_HEAD(&matrix_dev.mdev_list);
+
+	/* Test if PQAP(QCI) instruction is available */
+	if (test_facility(12)) {
+		ret = ap_qci(&matrix_dev.info);
+		if (ret && (ret != -EOPNOTSUPP)) {
After Connie's curiosity was piqued I gave this another look. If
I read the ap_qci() documentation and code correctly, it can return
either 0 or -EOPNOTSUPP, but nothing else. So basically this
is a dead branch.
Can it return -EOPNOTSUPP if facility 12 is present?
I do not think it is reasonable to continue if
we stated that facility 12 is present but the ap_qci function failed.

So I propose that we return an error and break the insmod.
So, would that be a bug? If yes, returning an error makes sense.

(I can't check the documentation to find out about the interaction
between facility 12 and PQAP(QCI)...)


The comment is right:

Facility 12 means that PQAP(QCI) is available... so it better be there!


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux