Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 1/1] s390/vfio-ap: fix circular lockdep when
 setting/clearing crypto masks
To:     Halil Pasic <pasic@linux.ibm.com>
Cc:     linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org, stable@vger.kernel.org,
        borntraeger@de.ibm.com, cohuck@redhat.com, kwankhede@nvidia.com,
        pbonzini@redhat.com, alex.williamson@redhat.com,
        pasic@linux.vnet.ibm.com
References: <20210302204322.24441-1-akrowiak@linux.ibm.com>
 <20210302204322.24441-2-akrowiak@linux.ibm.com>
 <20210303162332.4d227dbe.pasic@linux.ibm.com>
 <e5cc2a81-7273-2b3e-0d4c-c6c17502bdae@linux.ibm.com>
 <20210303204211.4c021c25.pasic@linux.ibm.com>
From:   Tony Krowiak <akrowiak@linux.ibm.com>
Message-ID: <c623c4d6-4cda-9521-ec5e-e4d6fd978a90@linux.ibm.com>
Date:   Thu, 4 Mar 2021 11:22:28 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
MIME-Version: 1.0
In-Reply-To: <20210303204211.4c021c25.pasic@linux.ibm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


On 3/3/21 2:42 PM, Halil Pasic wrote:
> On Wed, 3 Mar 2021 11:41:22 -0500
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>
>>> How do you exect userspace to react to this -ENODEV?
>> The VFIO_DEVICE_RESET ioctl expects a return code.
>> The vfio_ap_mdev_reset_queues() function can return -EIO or
>> -EBUSY, so I would expect userspace to handle -ENODEV
>> similarly to -EIO or any other non-zero return code. I also
>> looked at all of the VFIO_DEVICE_RESET calls from QEMU to see
>> how the return from the ioctl call is handled:
>>
>> * ap: reports the reset failed along with the rc
> And carries on as if nothing happened. There is not much smart
> userspace can do in such a situation. Therefore the reset really
> should not fail.

Regardless of what we decide to do here, there is the
possibility that the vfio_ap_mdev_reset_queues()
function will return an error, so your point is moot
and maybe should be brought up as a QEMU
implementation issue. I don't think it is encumbent
upon the KVM code to anticipate how userspace
will respond to a non-zero return code. I think the
pertinent question is what return code makes sense.
Having said that, I have other concerns which I
discussed below.

>
> Please note that in this particular case, if the userspace would
> opt for a retry, we would most likely end up in a retry loop.
>
>> * ccw: doesn't check the rc
>> * pci: kind of hard to follow without digging deep, but definitely
>>            handles non-zero rc.
>>
>> I think the caller should be notified whether the queues were
>> successfully reset or not, and why; in this case, the answer is
>> there are no devices to reset.
> That is the wrong answer. The ioctl is supposed to reset the
> ap_matrix_mdev device. The ap_matrix_mdev device still exists. Thus
> returning -ENODEV is bugous.

That makes sense and it begs the question, what does it mean to
reset the mdev? Is resetting the queues an appropriate response
to the VFIO_DEVICE_RESET ioctl call?

The purpose of the mdev is to supply the AP configuration to a KVM
guest. The queues themselves belong to the guest. If the guest enables
interrupts for a queue and vfio_ap does a reset in response to the ioctl
call, then the guest will be sitting there waiting for interrupts which
have been disabled by the reset. It seems that as long as a guest is
using the mdev, then management of its queues (i.e., reset) should be
left to the guest. Unless there is something to reset as far as the
mdev is concerned, maybe the response to the VFIO_RESET_DEVICE
ioctl ought to be a NOP regardless of the value of ->kvm.

>
> Regards,
> Halil