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?)
>
> }
>
> 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
>
