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 > -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html