| Message ID | 20220404174349.58530-17-mjrosato@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: enable zPCI for interpretive execution | expand |
On Mon, Apr 04, 2022 at 01:43:44PM -0400, Matthew Rosato wrote: > At the time a KVM is associated with a vfio group, s390x zPCI devices > must register a special guest indication (GISA designation) to allow > for the use of interpretive execution facilities. This indication is > used to ensure that only the specified KVM can interact with the device. > Similarly, the indication must be removed once the KVM is no longer > associated with the device. > > This patch adds an s390-specific hook to invoke a KVM registration routine > for each device associated with the iommu group; in reality, it will be a > NOP for all but zPCI devices on s390x. > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > --- > virt/kvm/vfio.c | 35 ++++++++++++++++++++++++++++++++++- > 1 file changed, 34 insertions(+), 1 deletion(-) I wonder if this should be done in the vfio_pci side from the existing kvm notifier Jason
On 4/8/22 8:45 AM, Jason Gunthorpe wrote: > On Mon, Apr 04, 2022 at 01:43:44PM -0400, Matthew Rosato wrote: >> At the time a KVM is associated with a vfio group, s390x zPCI devices >> must register a special guest indication (GISA designation) to allow >> for the use of interpretive execution facilities. This indication is >> used to ensure that only the specified KVM can interact with the device. >> Similarly, the indication must be removed once the KVM is no longer >> associated with the device. >> >> This patch adds an s390-specific hook to invoke a KVM registration routine >> for each device associated with the iommu group; in reality, it will be a >> NOP for all but zPCI devices on s390x. >> >> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >> --- >> virt/kvm/vfio.c | 35 ++++++++++++++++++++++++++++++++++- >> 1 file changed, 34 insertions(+), 1 deletion(-) > > I wonder if this should be done in the vfio_pci side from the existing > kvm notifier > So you mean rather than hooking into virt as I do here, drive something out of drivers/vfio/vfio.c:vfio_group_set_kvm? Note, the kvm notifier is handled in vfio, not vfio_pci, so if you want to handle it in vfio_pci I think we'd need to add a new routine to vfio_device_ops and only define it vfio_pci for s390 e.g. static const struct vfio_device_ops vfio_pci_ops = { .name = "vfio-pci", [...] #ifdef CONFIG_S390 .set_kvm = vfio_pci_zdev_set_kvm, #endif }; and something like... void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) { struct vfio_device *vdev; group->kvm = kvm; mutex_lock(&group->device_lock); list_for_each_entry(vdev, &group->device_list, group_next) { if (vdev->ops->set_kvm) it->ops->set_kvm(vdev, kvm); } mutex_unlock(&group->device_lock); blocking_notifier_call_chain(&group->notifier, VFIO_GROUP_NOTIFY_SET_KVM, kvm); }
On Tue, Apr 12, 2022 at 09:39:44AM -0400, Matthew Rosato wrote: > On 4/8/22 8:45 AM, Jason Gunthorpe wrote: > > On Mon, Apr 04, 2022 at 01:43:44PM -0400, Matthew Rosato wrote: > > > At the time a KVM is associated with a vfio group, s390x zPCI devices > > > must register a special guest indication (GISA designation) to allow > > > for the use of interpretive execution facilities. This indication is > > > used to ensure that only the specified KVM can interact with the device. > > > Similarly, the indication must be removed once the KVM is no longer > > > associated with the device. > > > > > > This patch adds an s390-specific hook to invoke a KVM registration routine > > > for each device associated with the iommu group; in reality, it will be a > > > NOP for all but zPCI devices on s390x. > > > > > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > > > virt/kvm/vfio.c | 35 ++++++++++++++++++++++++++++++++++- > > > 1 file changed, 34 insertions(+), 1 deletion(-) > > > > I wonder if this should be done in the vfio_pci side from the existing > > kvm notifier > > > > So you mean rather than hooking into virt as I do here, drive something out > of drivers/vfio/vfio.c:vfio_group_set_kvm? Note, the kvm notifier is > handled in vfio, not vfio_pci, so if you want to handle it in vfio_pci I > think we'd need to add a new routine to vfio_device_ops and only define it > vfio_pci for s390 I've been thinking about doing that anyhow, exactly for reasons like this.. > static const struct vfio_device_ops vfio_pci_ops = { > .name = "vfio-pci", > [...] > #ifdef CONFIG_S390 > .set_kvm = vfio_pci_zdev_set_kvm, > #endif > }; > > and something like... > > void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) > { > struct vfio_device *vdev; > group->kvm = kvm; > > mutex_lock(&group->device_lock); > list_for_each_entry(vdev, &group->device_list, group_next) { > if (vdev->ops->set_kvm) > it->ops->set_kvm(vdev, kvm); > } > mutex_unlock(&group->device_lock); Almost, the device should be open before calling the callback And you have to inject a callback during open if the device is opened after the kvm was set. But I don't think you need to do this, you can just register a notifier in zpci when it hooks open_device like everything else, right? Jason
On 4/12/22 9:55 AM, Jason Gunthorpe wrote: > On Tue, Apr 12, 2022 at 09:39:44AM -0400, Matthew Rosato wrote: >> On 4/8/22 8:45 AM, Jason Gunthorpe wrote: >>> On Mon, Apr 04, 2022 at 01:43:44PM -0400, Matthew Rosato wrote: >>>> At the time a KVM is associated with a vfio group, s390x zPCI devices >>>> must register a special guest indication (GISA designation) to allow >>>> for the use of interpretive execution facilities. This indication is >>>> used to ensure that only the specified KVM can interact with the device. >>>> Similarly, the indication must be removed once the KVM is no longer >>>> associated with the device. >>>> >>>> This patch adds an s390-specific hook to invoke a KVM registration routine >>>> for each device associated with the iommu group; in reality, it will be a >>>> NOP for all but zPCI devices on s390x. >>>> >>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >>>> virt/kvm/vfio.c | 35 ++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 34 insertions(+), 1 deletion(-) >>> >>> I wonder if this should be done in the vfio_pci side from the existing >>> kvm notifier >>> >> >> So you mean rather than hooking into virt as I do here, drive something out >> of drivers/vfio/vfio.c:vfio_group_set_kvm? Note, the kvm notifier is >> handled in vfio, not vfio_pci, so if you want to handle it in vfio_pci I >> think we'd need to add a new routine to vfio_device_ops and only define it >> vfio_pci for s390 > > I've been thinking about doing that anyhow, exactly for reasons like > this.. > >> static const struct vfio_device_ops vfio_pci_ops = { >> .name = "vfio-pci", >> [...] >> #ifdef CONFIG_S390 >> .set_kvm = vfio_pci_zdev_set_kvm, >> #endif >> }; >> >> and something like... >> >> void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) >> { >> struct vfio_device *vdev; >> group->kvm = kvm; >> >> mutex_lock(&group->device_lock); >> list_for_each_entry(vdev, &group->device_list, group_next) { >> if (vdev->ops->set_kvm) >> it->ops->set_kvm(vdev, kvm); >> } >> mutex_unlock(&group->device_lock); > > Almost, the device should be open before calling the callback > > And you have to inject a callback during open if the device is opened > after the kvm was set. > > But I don't think you need to do this, you can just register a > notifier in zpci when it hooks open_device like everything else, > right? Yes, that would also work -- I was registering a notifier for a few prior versions of this series (granted, not from open_device) but got the impression I should avoid registering a notifier from within vfio_pci_zdev. I will go ahead and add register/unregister notifiers hooked from vfio_pci_core_finish_enable/vfio_pci_core_close_device for zpci (e.g. vfio_pci_zdev_{open,close}) and use the notifier events to drive the routines from patch 15.
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c index 8fcbc50221c2..2efe5be5a6ee 100644 --- a/virt/kvm/vfio.c +++ b/virt/kvm/vfio.c @@ -106,7 +106,7 @@ static bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group) return ret > 0; } -#ifdef CONFIG_SPAPR_TCE_IOMMU +#if defined(CONFIG_SPAPR_TCE_IOMMU) || defined(CONFIG_S390) static int kvm_vfio_external_user_iommu_id(struct vfio_group *vfio_group) { int (*fn)(struct vfio_group *); @@ -133,7 +133,9 @@ static struct iommu_group *kvm_vfio_group_get_iommu_group( return iommu_group_get_by_id(group_id); } +#endif +#ifdef CONFIG_SPAPR_TCE_IOMMU static void kvm_spapr_tce_release_vfio_group(struct kvm *kvm, struct vfio_group *vfio_group) { @@ -147,6 +149,24 @@ static void kvm_spapr_tce_release_vfio_group(struct kvm *kvm, } #endif +#ifdef CONFIG_S390 +static int kvm_s390_register_kvm(struct vfio_group *vfio_group, + struct kvm *kvm) +{ + int rc; + + struct iommu_group *grp = kvm_vfio_group_get_iommu_group(vfio_group); + + if (WARN_ON_ONCE(!grp)) + return -EPERM; + + rc = iommu_group_for_each_dev(grp, kvm, kvm_s390_pci_register_kvm); + iommu_group_put(grp); + + return rc; +} +#endif + /* * Groups can use the same or different IOMMU domains. If the same then * adding a new group may change the coherency of groups we've previously @@ -223,6 +243,16 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) return -ENOMEM; } +#ifdef CONFIG_S390 + ret = kvm_s390_register_kvm(vfio_group, dev->kvm); + if (ret) { + kfree(kvg); + mutex_unlock(&kv->lock); + kvm_vfio_group_put_external_user(vfio_group); + return ret; + } +#endif + list_add_tail(&kvg->node, &kv->group_list); kvg->vfio_group = vfio_group; @@ -258,6 +288,9 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) #ifdef CONFIG_SPAPR_TCE_IOMMU kvm_spapr_tce_release_vfio_group(dev->kvm, kvg->vfio_group); +#endif +#ifdef CONFIG_S390 + kvm_s390_register_kvm(vfio_group, NULL); #endif kvm_vfio_group_set_kvm(kvg->vfio_group, NULL); kvm_vfio_group_put_external_user(kvg->vfio_group);
At the time a KVM is associated with a vfio group, s390x zPCI devices must register a special guest indication (GISA designation) to allow for the use of interpretive execution facilities. This indication is used to ensure that only the specified KVM can interact with the device. Similarly, the indication must be removed once the KVM is no longer associated with the device. This patch adds an s390-specific hook to invoke a KVM registration routine for each device associated with the iommu group; in reality, it will be a NOP for all but zPCI devices on s390x. Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> --- virt/kvm/vfio.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)