On 31/07/2018 16:48, Cornelia Huck wrote:
On Tue, 31 Jul 2018 16:18:03 +0200
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
On 07/31/2018 03:40 PM, Cornelia Huck wrote:
On Tue, 31 Jul 2018 15:12:59 +0200
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
On 07/31/2018 02:24 PM, Cornelia Huck wrote:
+static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev, bool force)
+{
+ int ret;
+ int rc = 0;
+ unsigned long apid, apqi;
+ struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+ if (!matrix_mdev->activated) {
+ pr_err("%s: reset failed for mdev %s, not activated",
+ VFIO_AP_MODULE_NAME, matrix_mdev->name);
+
+ return -EPERM;
+ }
Hm... if we stick with the activation approach, what happens if we:
- open the mdev
- activate the matrix
- deactivate the matrix
I guess this step will fail and the matrix remains activated.
Have a look at vfio_ap_mdev_deactivate()
Hm, I don't see it...
static int vfio_ap_mdev_deactivate(struct ap_matrix_mdev *matrix_mdev)
{
int ret = 0;
if (!matrix_mdev->activated)
return 0;
if (matrix_mdev->kvm) {
pr_warn("%s: %s: deactivate failed, mdev %s is in use by guest %s\n",
VFIO_AP_MODULE_NAME, __func__, matrix_mdev->name,
matrix_mdev->kvm->arch.dbf->name);
return -EBUSY; <===================================== Return -EBUSY and don't deactivate.
Yes, but what forces ->kvm to be !NULL? I did not see this in the code,
and I'm not sure what I'm missing. (If ->kvm is guaranteed to be !NULL,
why do we check for it during open?)
The matrix_mdev->kvm pointer life cycle is, currently:
1) initialized to 0
2) Set during vfio device open by the callback of the
VFIO_GROUP_NOTIFY_SET_KVM notification
3) Cleared during vfio device close.
Since VFIO-AP only works with KVM, I think that you are right and that
we could
forget the check on open.
However the callback is called again when the guest is stopping,
we need to handle this too in the next version.
Thanks for the comment,
Regards,
Pierre
}
matrix_mdev->activated = false;
return ret;
}
- release the mdev, triggering this function
It seems we won't call PAPQ(ZAPQ) in that case -- is that how it is
supposed to work?
So basically the device remains active until the mdev release is
called if I'm correct.
So, why do we need the activate/deactivate approach, then? Why can't we
do the verification etc. during open?
We ca do the verification during open. If there was no activate before
the open we actually try to do one.
With activate however the admin can claim the resources before the
open (that is the guest start). We have a mechanism for both 'let's
overcommit and fail when we hit the wall' and 'let's make sure everything
is reserved before we give it to the guest'. The admin is free to
decide on his policy.
OK, let's discuss that in the other thread, then. I am not sure that
departing from the general mdev model is worth it.
(I have not yet gotten to replying there.)
Please take note that activate also has an effect on the availability
of certain operations (e.g. assign/unassign).
Regards,
Halil
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
