| Message ID | 1468848357-2331-8-git-send-email-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2016-07-18 13:25+0000, Eric Auger: > If the ITS modality is not available, let's simply support MSI > injection by transforming the MSI.data into an SPI ID. > > This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI > routing for arm too. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > > --- > > v6 -> v7: > - move vgic_v2m_inject_msi into vgic-irqfd > > v4 -> v5: > - on vgic_v2m_inject_msi check the msi->data is within the SPI range > - move KVM_HAVE_MSI in the KVM section (to be symetrical with ARM64) > > v2 -> v3: > - reword the commit message > - add sanity check about devid provision > > v1 -> v2: > - introduce vgic_v2m_inject_msi in vgic-v2-emul.c following Andre's > advice > --- > diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c > +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi) > +{ > + if (msi->flags & KVM_MSI_VALID_DEVID) > + return -EINVAL; > + if (!vgic_valid_spi(kvm, msi->data)) > + return -EINVAL; > + > + return kvm_vgic_inject_irq(kvm, 0, msi->data, 1); Hm, this isn't very MSI related ... arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with KVM_ARM_IRQ_TYPE_SPI that does kvm_vgic_inject_irq(kvm, 0, irq_num, level) Is that interface lacking? Thanks. > +} > + > +/** > * kvm_set_msi: inject the MSI corresponding to the > * MSI routing entry > * > @@ -96,7 +113,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, > msi.devid = e->msi_devid; > > if (!vgic_has_its(kvm)) > - return -ENODEV; > + return vgic_v2m_inject_msi(kvm, &msi); > > return vgic_its_inject_msi(kvm, &msi); > } > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Radim, On 21/07/2016 18:33, Radim Krčmář wrote: > 2016-07-18 13:25+0000, Eric Auger: >> If the ITS modality is not available, let's simply support MSI >> injection by transforming the MSI.data into an SPI ID. >> >> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI >> routing for arm too. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> >> --- >> >> v6 -> v7: >> - move vgic_v2m_inject_msi into vgic-irqfd >> >> v4 -> v5: >> - on vgic_v2m_inject_msi check the msi->data is within the SPI range >> - move KVM_HAVE_MSI in the KVM section (to be symetrical with ARM64) >> >> v2 -> v3: >> - reword the commit message >> - add sanity check about devid provision >> >> v1 -> v2: >> - introduce vgic_v2m_inject_msi in vgic-v2-emul.c following Andre's >> advice >> --- >> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c >> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi) >> +{ >> + if (msi->flags & KVM_MSI_VALID_DEVID) >> + return -EINVAL; >> + if (!vgic_valid_spi(kvm, msi->data)) >> + return -EINVAL; >> + >> + return kvm_vgic_inject_irq(kvm, 0, msi->data, 1); > > Hm, this isn't very MSI related ... > > arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with > KVM_ARM_IRQ_TYPE_SPI that does > kvm_vgic_inject_irq(kvm, 0, irq_num, level) > > Is that interface lacking? You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't. For kvm-tools I guess, Andre manages without. My first feeling was it is part of the KVM API and we can implement it easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to do what QEMU implements as "kvm_gsi_direct_mapping" and manage the translation into the semantic of the ARM GSI. But Well, if you prefer we do not implement it for GICv2M, since considered as far fetched I can remove this patch. Thank you for your time Best Regards Eric > > Thanks. > >> +} >> + >> +/** >> * kvm_set_msi: inject the MSI corresponding to the >> * MSI routing entry >> * >> @@ -96,7 +113,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, >> msi.devid = e->msi_devid; >> >> if (!vgic_has_its(kvm)) >> - return -ENODEV; >> + return vgic_v2m_inject_msi(kvm, &msi); >> >> return vgic_its_inject_msi(kvm, &msi); >> } >> -- >> 1.9.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-07-21 23:10+0200, Auger Eric: > On 21/07/2016 18:33, Radim Krčmář wrote: >> 2016-07-18 13:25+0000, Eric Auger: >>> If the ITS modality is not available, let's simply support MSI >>> injection by transforming the MSI.data into an SPI ID. >>> >>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI >>> routing for arm too. >>> >>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>> >>> --- >>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c >>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi) >>> +{ >>> + if (msi->flags & KVM_MSI_VALID_DEVID) >>> + return -EINVAL; >>> + if (!vgic_valid_spi(kvm, msi->data)) >>> + return -EINVAL; >>> + >>> + return kvm_vgic_inject_irq(kvm, 0, msi->data, 1); >> >> Hm, this isn't very MSI related ... >> >> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with >> KVM_ARM_IRQ_TYPE_SPI that does >> kvm_vgic_inject_irq(kvm, 0, irq_num, level) >> >> Is that interface lacking? > > You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't. No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today. Or isn't it? > For kvm-tools I guess, Andre manages without. > > My first feeling was it is part of the KVM API and we can implement it > easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to > do what QEMU implements as "kvm_gsi_direct_mapping" and manage the > translation into the semantic of the ARM GSI. I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is unfortunate. SPI only uses msi.data, which makes remaining fields in the msi struct arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two route types now do the same, but only sometimes (without ITS), which makes the situation even less understandable ... Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a new interface to two different meanings for KVM_SIGNAL_MSI: KVM_SIGNAL_MSI was created because we didn't have anything that could inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING and we are still missing a generic interface to do that. > But Well, if you prefer we do not implement it for GICv2M, since > considered as far fetched I can remove this patch. I do, thanks. Documentation in [6/7] was ahead and needs changing then. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Radim, On 22/07/2016 15:39, Radim Krčmář wrote: > 2016-07-21 23:10+0200, Auger Eric: >> On 21/07/2016 18:33, Radim Krčmář wrote: >>> 2016-07-18 13:25+0000, Eric Auger: >>>> If the ITS modality is not available, let's simply support MSI >>>> injection by transforming the MSI.data into an SPI ID. >>>> >>>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI >>>> routing for arm too. >>>> >>>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>>> >>>> --- >>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c >>>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi) >>>> +{ >>>> + if (msi->flags & KVM_MSI_VALID_DEVID) >>>> + return -EINVAL; >>>> + if (!vgic_valid_spi(kvm, msi->data)) >>>> + return -EINVAL; >>>> + >>>> + return kvm_vgic_inject_irq(kvm, 0, msi->data, 1); >>> >>> Hm, this isn't very MSI related ... >>> >>> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with >>> KVM_ARM_IRQ_TYPE_SPI that does >>> kvm_vgic_inject_irq(kvm, 0, irq_num, level) >>> >>> Is that interface lacking? >> >> You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't. > > No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today. > Or isn't it? > >> For kvm-tools I guess, Andre manages without. >> >> My first feeling was it is part of the KVM API and we can implement it >> easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to >> do what QEMU implements as "kvm_gsi_direct_mapping" and manage the >> translation into the semantic of the ARM GSI. > > I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is > unfortunate. > > SPI only uses msi.data, which makes remaining fields in the msi struct > arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two > route types now do the same, but only sometimes (without ITS), which > makes the situation even less understandable ... > > Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we > wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a > new interface to two different meanings for KVM_SIGNAL_MSI: > KVM_SIGNAL_MSI was created because we didn't have anything that could > inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING > and we are still missing a generic interface to do that. > >> But Well, if you prefer we do not implement it for GICv2M, since >> considered as far fetched I can remove this patch. > > I do, thanks. Documentation in [6/7] was ahead and needs changing then. Argh just saw your reply after sending v8. Will respin immediatly. Sorry for the confusion Thanks Eric > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-07-22 15:52+0200, Auger Eric: > On 22/07/2016 15:39, Radim Krčmář wrote: >> 2016-07-21 23:10+0200, Auger Eric: >>> On 21/07/2016 18:33, Radim Krčmář wrote: >>>> 2016-07-18 13:25+0000, Eric Auger: >>>>> If the ITS modality is not available, let's simply support MSI >>>>> injection by transforming the MSI.data into an SPI ID. >>>>> >>>>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI >>>>> routing for arm too. >>>>> >>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>>>> >>>>> --- >>>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c >>>>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi) >>>>> +{ >>>>> + if (msi->flags & KVM_MSI_VALID_DEVID) >>>>> + return -EINVAL; >>>>> + if (!vgic_valid_spi(kvm, msi->data)) >>>>> + return -EINVAL; >>>>> + >>>>> + return kvm_vgic_inject_irq(kvm, 0, msi->data, 1); >>>> >>>> Hm, this isn't very MSI related ... >>>> >>>> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with >>>> KVM_ARM_IRQ_TYPE_SPI that does >>>> kvm_vgic_inject_irq(kvm, 0, irq_num, level) >>>> >>>> Is that interface lacking? >>> >>> You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't. >> >> No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today. >> Or isn't it? >> >>> For kvm-tools I guess, Andre manages without. >>> >>> My first feeling was it is part of the KVM API and we can implement it >>> easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to >>> do what QEMU implements as "kvm_gsi_direct_mapping" and manage the >>> translation into the semantic of the ARM GSI. >> >> I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is >> unfortunate. >> >> SPI only uses msi.data, which makes remaining fields in the msi struct >> arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two >> route types now do the same, but only sometimes (without ITS), which >> makes the situation even less understandable ... >> >> Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we >> wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a >> new interface to two different meanings for KVM_SIGNAL_MSI: >> KVM_SIGNAL_MSI was created because we didn't have anything that could >> inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING >> and we are still missing a generic interface to do that. >> >>> But Well, if you prefer we do not implement it for GICv2M, since >>> considered as far fetched I can remove this patch. >> >> I do, thanks. Documentation in [6/7] was ahead and needs changing then. > > Argh just saw your reply after sending v8. Will respin immediatly. > > Sorry for the confusion No problem. Give me half an hour for a review, please. :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22/07/2016 15:56, Radim Krčmář wrote: > 2016-07-22 15:52+0200, Auger Eric: >> On 22/07/2016 15:39, Radim Krčmář wrote: >>> 2016-07-21 23:10+0200, Auger Eric: >>>> On 21/07/2016 18:33, Radim Krčmář wrote: >>>>> 2016-07-18 13:25+0000, Eric Auger: >>>>>> If the ITS modality is not available, let's simply support MSI >>>>>> injection by transforming the MSI.data into an SPI ID. >>>>>> >>>>>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI >>>>>> routing for arm too. >>>>>> >>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>>>>> >>>>>> --- >>>>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c >>>>>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi) >>>>>> +{ >>>>>> + if (msi->flags & KVM_MSI_VALID_DEVID) >>>>>> + return -EINVAL; >>>>>> + if (!vgic_valid_spi(kvm, msi->data)) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + return kvm_vgic_inject_irq(kvm, 0, msi->data, 1); >>>>> >>>>> Hm, this isn't very MSI related ... >>>>> >>>>> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with >>>>> KVM_ARM_IRQ_TYPE_SPI that does >>>>> kvm_vgic_inject_irq(kvm, 0, irq_num, level) >>>>> >>>>> Is that interface lacking? >>>> >>>> You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't. >>> >>> No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today. >>> Or isn't it? >>> >>>> For kvm-tools I guess, Andre manages without. >>>> >>>> My first feeling was it is part of the KVM API and we can implement it >>>> easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to >>>> do what QEMU implements as "kvm_gsi_direct_mapping" and manage the >>>> translation into the semantic of the ARM GSI. >>> >>> I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is >>> unfortunate. >>> >>> SPI only uses msi.data, which makes remaining fields in the msi struct >>> arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two >>> route types now do the same, but only sometimes (without ITS), which >>> makes the situation even less understandable ... >>> >>> Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we >>> wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a >>> new interface to two different meanings for KVM_SIGNAL_MSI: >>> KVM_SIGNAL_MSI was created because we didn't have anything that could >>> inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING >>> and we are still missing a generic interface to do that. >>> >>>> But Well, if you prefer we do not implement it for GICv2M, since >>>> considered as far fetched I can remove this patch. >>> >>> I do, thanks. Documentation in [6/7] was ahead and needs changing then. >> >> Argh just saw your reply after sending v8. Will respin immediatly. >> >> Sorry for the confusion > > No problem. Give me half an hour for a review, please. :) Sure Eric > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig index 3e1cd04..90d0176 100644 --- a/arch/arm/kvm/Kconfig +++ b/arch/arm/kvm/Kconfig @@ -34,6 +34,7 @@ config KVM select HAVE_KVM_IRQFD select HAVE_KVM_IRQCHIP select HAVE_KVM_IRQ_ROUTING + select HAVE_KVM_MSI depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER ---help--- Support hosting virtualized guest machines. diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c index 4f0d7eb..28c96ad 100644 --- a/virt/kvm/arm/vgic/vgic-irqfd.c +++ b/virt/kvm/arm/vgic/vgic-irqfd.c @@ -77,6 +77,23 @@ out: } /** + * vgic_v2m_inject_msi: emulates GICv2M MSI injection by injecting + * the SPI ID matching the msi data + * + * @kvm: pointer to the kvm struct + * @msi: the msi struct handle + */ +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi) +{ + if (msi->flags & KVM_MSI_VALID_DEVID) + return -EINVAL; + if (!vgic_valid_spi(kvm, msi->data)) + return -EINVAL; + + return kvm_vgic_inject_irq(kvm, 0, msi->data, 1); +} + +/** * kvm_set_msi: inject the MSI corresponding to the * MSI routing entry * @@ -96,7 +113,7 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, msi.devid = e->msi_devid; if (!vgic_has_its(kvm)) - return -ENODEV; + return vgic_v2m_inject_msi(kvm, &msi); return vgic_its_inject_msi(kvm, &msi); }
If the ITS modality is not available, let's simply support MSI injection by transforming the MSI.data into an SPI ID. This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI routing for arm too. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- v6 -> v7: - move vgic_v2m_inject_msi into vgic-irqfd v4 -> v5: - on vgic_v2m_inject_msi check the msi->data is within the SPI range - move KVM_HAVE_MSI in the KVM section (to be symetrical with ARM64) v2 -> v3: - reword the commit message - add sanity check about devid provision v1 -> v2: - introduce vgic_v2m_inject_msi in vgic-v2-emul.c following Andre's advice --- arch/arm/kvm/Kconfig | 1 + virt/kvm/arm/vgic/vgic-irqfd.c | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-)