| Message ID | cover.1731130093.git.nicolinc@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | vfio: Allow userspace to specify the address for each MSI vector | expand |
On 2024-11-09 5:48 am, Nicolin Chen wrote: > On ARM GIC systems and others, the target address of the MSI is translated > by the IOMMU. For GIC, the MSI address page is called "ITS" page. When the > IOMMU is disabled, the MSI address is programmed to the physical location > of the GIC ITS page (e.g. 0x20200000). When the IOMMU is enabled, the ITS > page is behind the IOMMU, so the MSI address is programmed to an allocated > IO virtual address (a.k.a IOVA), e.g. 0xFFFF0000, which must be mapped to > the physical ITS page: IOVA (0xFFFF0000) ===> PA (0x20200000). > When a 2-stage translation is enabled, IOVA will be still used to program > the MSI address, though the mappings will be in two stages: > IOVA (0xFFFF0000) ===> IPA (e.g. 0x80900000) ===> 0x20200000 > (IPA stands for Intermediate Physical Address). > > If the device that generates MSI is attached to an IOMMU_DOMAIN_DMA, the > IOVA is dynamically allocated from the top of the IOVA space. If attached > to an IOMMU_DOMAIN_UNMANAGED (e.g. a VFIO passthrough device), the IOVA is > fixed to an MSI window reported by the IOMMU driver via IOMMU_RESV_SW_MSI, > which is hardwired to MSI_IOVA_BASE (IOVA==0x8000000) for ARM IOMMUs. > > So far, this IOMMU_RESV_SW_MSI works well as kernel is entirely in charge > of the IOMMU translation (1-stage translation), since the IOVA for the ITS > page is fixed and known by kernel. However, with virtual machine enabling > a nested IOMMU translation (2-stage), a guest kernel directly controls the > stage-1 translation with an IOMMU_DOMAIN_DMA, mapping a vITS page (at an > IPA 0x80900000) onto its own IOVA space (e.g. 0xEEEE0000). Then, the host > kernel can't know that guest-level IOVA to program the MSI address. > > To solve this problem the VMM should capture the MSI IOVA allocated by the > guest kernel and relay it to the GIC driver in the host kernel, to program > the correct MSI IOVA. And this requires a new ioctl via VFIO. Once VFIO has that information from userspace, though, do we really need the whole complicated dance to push it right down into the irqchip layer just so it can be passed back up again? AFAICS vfio_msi_set_vector_signal() via VFIO_DEVICE_SET_IRQS already explicitly rewrites MSI-X vectors, so it seems like it should be pretty straightforward to override the message address in general at that level, without the lower layers having to be aware at all, no? Thanks, Robin. > Extend the VFIO path to allow an MSI target IOVA to be forwarded into the > kernel and pushed down to the GIC driver. > > Add VFIO ioctl VFIO_IRQ_SET_ACTION_PREPARE with VFIO_IRQ_SET_DATA_MSI_IOVA > to carry the data. > > The downstream calltrace is quite long from the VFIO to the ITS driver. So > in order to carry the MSI IOVA from the top to its_irq_domain_alloc(), add > patches in a leaf-to-root order: > > vfio_pci_core_ioctl: > vfio_pci_set_irqs_ioctl: > vfio_pci_set_msi_prepare: // PATCH-7 > pci_alloc_irq_vectors_iovas: // PATCH-6 > __pci_alloc_irq_vectors: // PATCH-5 > __pci_enable_msi/msix_range: // PATCH-4 > msi/msix_capability_init: // PATCH-3 > msi/msix_setup_msi_descs: > msi_insert_msi_desc(); // PATCH-1 > pci_msi_setup_msi_irqs: > msi_domain_alloc_irqs_all_locked: > __msi_domain_alloc_locked: > __msi_domain_alloc_irqs: > __irq_domain_alloc_irqs: > irq_domain_alloc_irqs_locked: > irq_domain_alloc_irqs_hierarchy: > msi_domain_alloc: > irq_domain_alloc_irqs_parent: > its_irq_domain_alloc(); // PATCH-2 > > Note that this series solves half the problem, since it only allows kernel > to set the physical PCI MSI/MSI-X on the device with the correct head IOVA > of a 2-stage translation, where the guest kernel does the stage-1 mapping > that MSI IOVA (0xEEEE0000) to its own vITS page (0x80900000) while missing > the stage-2 mapping from that IPA to the physical ITS page: > 0xEEEE0000 ===> 0x80900000 =x=> 0x20200000 > A followup series should fill that gap, doing the stage-2 mapping from the > vITS page 0x80900000 to the physical ITS page (0x20200000), likely via new > IOMMUFD ioctl. Once VMM sets up this stage-2 mapping, VM will act the same > as bare metal relying on a running kernel to handle the stage-1 mapping: > 0xEEEE0000 ===> 0x80900000 ===> 0x20200000 > > This series (prototype) is on Github: > https://github.com/nicolinc/iommufd/commits/vfio_msi_giova-rfcv1/ > It's tested by hacking the host kernel to hard-code a stage-2 mapping. > > Thanks! > Nicolin > > Nicolin Chen (7): > genirq/msi: Allow preset IOVA in struct msi_desc for MSI doorbell > address > irqchip/gic-v3-its: Bypass iommu_cookie if desc->msi_iova is preset > PCI/MSI: Pass in msi_iova to msi_domain_insert_msi_desc > PCI/MSI: Allow __pci_enable_msi_range to pass in iova > PCI/MSI: Extract a common __pci_alloc_irq_vectors function > PCI/MSI: Add pci_alloc_irq_vectors_iovas helper > vfio/pci: Allow preset MSI IOVAs via VFIO_IRQ_SET_ACTION_PREPARE > > drivers/pci/msi/msi.h | 3 +- > include/linux/msi.h | 11 +++ > include/linux/pci.h | 18 ++++ > include/linux/vfio_pci_core.h | 1 + > include/uapi/linux/vfio.h | 8 +- > drivers/irqchip/irq-gic-v3-its.c | 21 ++++- > drivers/pci/msi/api.c | 136 ++++++++++++++++++++---------- > drivers/pci/msi/msi.c | 20 +++-- > drivers/vfio/pci/vfio_pci_intrs.c | 41 ++++++++- > drivers/vfio/vfio_main.c | 3 + > kernel/irq/msi.c | 6 ++ > 11 files changed, 212 insertions(+), 56 deletions(-) >
On Mon, 11 Nov 2024 13:09:20 +0000, Robin Murphy <robin.murphy@arm.com> wrote: > > On 2024-11-09 5:48 am, Nicolin Chen wrote: > > On ARM GIC systems and others, the target address of the MSI is translated > > by the IOMMU. For GIC, the MSI address page is called "ITS" page. When the > > IOMMU is disabled, the MSI address is programmed to the physical location > > of the GIC ITS page (e.g. 0x20200000). When the IOMMU is enabled, the ITS > > page is behind the IOMMU, so the MSI address is programmed to an allocated > > IO virtual address (a.k.a IOVA), e.g. 0xFFFF0000, which must be mapped to > > the physical ITS page: IOVA (0xFFFF0000) ===> PA (0x20200000). > > When a 2-stage translation is enabled, IOVA will be still used to program > > the MSI address, though the mappings will be in two stages: > > IOVA (0xFFFF0000) ===> IPA (e.g. 0x80900000) ===> 0x20200000 > > (IPA stands for Intermediate Physical Address). > > > > If the device that generates MSI is attached to an IOMMU_DOMAIN_DMA, the > > IOVA is dynamically allocated from the top of the IOVA space. If attached > > to an IOMMU_DOMAIN_UNMANAGED (e.g. a VFIO passthrough device), the IOVA is > > fixed to an MSI window reported by the IOMMU driver via IOMMU_RESV_SW_MSI, > > which is hardwired to MSI_IOVA_BASE (IOVA==0x8000000) for ARM IOMMUs. > > > > So far, this IOMMU_RESV_SW_MSI works well as kernel is entirely in charge > > of the IOMMU translation (1-stage translation), since the IOVA for the ITS > > page is fixed and known by kernel. However, with virtual machine enabling > > a nested IOMMU translation (2-stage), a guest kernel directly controls the > > stage-1 translation with an IOMMU_DOMAIN_DMA, mapping a vITS page (at an > > IPA 0x80900000) onto its own IOVA space (e.g. 0xEEEE0000). Then, the host > > kernel can't know that guest-level IOVA to program the MSI address. > > > > To solve this problem the VMM should capture the MSI IOVA allocated by the > > guest kernel and relay it to the GIC driver in the host kernel, to program > > the correct MSI IOVA. And this requires a new ioctl via VFIO. > > Once VFIO has that information from userspace, though, do we really > need the whole complicated dance to push it right down into the > irqchip layer just so it can be passed back up again? AFAICS > vfio_msi_set_vector_signal() via VFIO_DEVICE_SET_IRQS already > explicitly rewrites MSI-X vectors, so it seems like it should be > pretty straightforward to override the message address in general at > that level, without the lower layers having to be aware at all, no? +1. I would like to avoid polluting each and every interrupt controller with usage-specific knowledge (they usually are brain-damaged enough). We already have an indirection into the IOMMU subsystem and it shouldn't be a big deal to intercept the message for all implementations at this level. I also wonder how to handle the case of braindead^Wwonderful platforms where ITS transactions are not translated by the SMMU. Somehow, VFIO should be made aware of this situation. Thanks, M.
On Mon, Nov 11, 2024 at 01:09:20PM +0000, Robin Murphy wrote: > On 2024-11-09 5:48 am, Nicolin Chen wrote: > > To solve this problem the VMM should capture the MSI IOVA allocated by the > > guest kernel and relay it to the GIC driver in the host kernel, to program > > the correct MSI IOVA. And this requires a new ioctl via VFIO. > > Once VFIO has that information from userspace, though, do we really need > the whole complicated dance to push it right down into the irqchip layer > just so it can be passed back up again? AFAICS > vfio_msi_set_vector_signal() via VFIO_DEVICE_SET_IRQS already explicitly > rewrites MSI-X vectors, so it seems like it should be pretty > straightforward to override the message address in general at that > level, without the lower layers having to be aware at all, no? Didn't see that clearly!! It works with a simple following override: -------------------------------------------------------------------- @@ -497,6 +497,10 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev, struct msi_msg msg; get_cached_msi_msg(irq, &msg); + if (vdev->msi_iovas) { + msg.address_lo = lower_32_bits(vdev->msi_iovas[vector]); + msg.address_hi = upper_32_bits(vdev->msi_iovas[vector]); + } pci_write_msi_msg(irq, &msg); } -------------------------------------------------------------------- With that, I think we only need one VFIO change for this part :) Thanks! Nicolin
On Mon, Nov 11, 2024 at 02:14:15PM +0000, Marc Zyngier wrote: > On Mon, 11 Nov 2024 13:09:20 +0000, > Robin Murphy <robin.murphy@arm.com> wrote: > > On 2024-11-09 5:48 am, Nicolin Chen wrote: > > > To solve this problem the VMM should capture the MSI IOVA allocated by the > > > guest kernel and relay it to the GIC driver in the host kernel, to program > > > the correct MSI IOVA. And this requires a new ioctl via VFIO. > > > > Once VFIO has that information from userspace, though, do we really > > need the whole complicated dance to push it right down into the > > irqchip layer just so it can be passed back up again? AFAICS > > vfio_msi_set_vector_signal() via VFIO_DEVICE_SET_IRQS already > > explicitly rewrites MSI-X vectors, so it seems like it should be > > pretty straightforward to override the message address in general at > > that level, without the lower layers having to be aware at all, no? > > +1. > > I would like to avoid polluting each and every interrupt controller > with usage-specific knowledge (they usually are brain-damaged enough). > We already have an indirection into the IOMMU subsystem and it > shouldn't be a big deal to intercept the message for all > implementations at this level. > > I also wonder how to handle the case of braindead^Wwonderful platforms > where ITS transactions are not translated by the SMMU. Somehow, VFIO > should be made aware of this situation. Perhaps we should do iommu_get_domain_for_dev(&vdev->pdev->dev) and check the returned domain->type: * if (domain->type & __IOMMU_DOMAIN_PAGING): 1-stage translation * if (domain->type == IOMMU_DOMAIN_NESTED): 2-stage translation And for this particular topic/series, we should do something like: if (vdev->msi_iovas && domain->type == IOMMU_DOMAIN_NESTED) { msg.address_lo = lower_32_bits(vdev->msi_iovas[vector]); msg.address_hi = upper_32_bits(vdev->msi_iovas[vector]); } ? Thanks Nicolin
On Tue, Nov 12, 2024 at 01:54:58PM -0800, Nicolin Chen wrote: > On Mon, Nov 11, 2024 at 01:09:20PM +0000, Robin Murphy wrote: > > On 2024-11-09 5:48 am, Nicolin Chen wrote: > > > To solve this problem the VMM should capture the MSI IOVA allocated by the > > > guest kernel and relay it to the GIC driver in the host kernel, to program > > > the correct MSI IOVA. And this requires a new ioctl via VFIO. > > > > Once VFIO has that information from userspace, though, do we really need > > the whole complicated dance to push it right down into the irqchip layer > > just so it can be passed back up again? AFAICS > > vfio_msi_set_vector_signal() via VFIO_DEVICE_SET_IRQS already explicitly > > rewrites MSI-X vectors, so it seems like it should be pretty > > straightforward to override the message address in general at that > > level, without the lower layers having to be aware at all, no? > > Didn't see that clearly!! It works with a simple following override: > -------------------------------------------------------------------- > @@ -497,6 +497,10 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev, > struct msi_msg msg; > > get_cached_msi_msg(irq, &msg); > + if (vdev->msi_iovas) { > + msg.address_lo = lower_32_bits(vdev->msi_iovas[vector]); > + msg.address_hi = upper_32_bits(vdev->msi_iovas[vector]); > + } > pci_write_msi_msg(irq, &msg); > } > > -------------------------------------------------------------------- > > With that, I think we only need one VFIO change for this part :) Wow, is that really OK from a layering perspective? The comment is pretty clear on the intention that this is to resync the irq layer view of the device with the physical HW. Editing the msi_msg while doing that resync smells bad. Also, this is only doing MSI-X, we should include normal MSI as well. (it probably should have a resync too?) I'd want Thomas/Marc/Alex to agree.. (please read the cover letter for context) I think there are many options here we just need to get a clearer understanding what best fits the architecture of the interrupt subsystem. Jason
On Tue, 12 Nov 2024 21:34:30 -0400 Jason Gunthorpe <jgg@nvidia.com> wrote: > On Tue, Nov 12, 2024 at 01:54:58PM -0800, Nicolin Chen wrote: > > On Mon, Nov 11, 2024 at 01:09:20PM +0000, Robin Murphy wrote: > > > On 2024-11-09 5:48 am, Nicolin Chen wrote: > > > > To solve this problem the VMM should capture the MSI IOVA allocated by the > > > > guest kernel and relay it to the GIC driver in the host kernel, to program > > > > the correct MSI IOVA. And this requires a new ioctl via VFIO. > > > > > > Once VFIO has that information from userspace, though, do we really need > > > the whole complicated dance to push it right down into the irqchip layer > > > just so it can be passed back up again? AFAICS > > > vfio_msi_set_vector_signal() via VFIO_DEVICE_SET_IRQS already explicitly > > > rewrites MSI-X vectors, so it seems like it should be pretty > > > straightforward to override the message address in general at that > > > level, without the lower layers having to be aware at all, no? > > > > Didn't see that clearly!! It works with a simple following override: > > -------------------------------------------------------------------- > > @@ -497,6 +497,10 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev, > > struct msi_msg msg; > > > > get_cached_msi_msg(irq, &msg); > > + if (vdev->msi_iovas) { > > + msg.address_lo = lower_32_bits(vdev->msi_iovas[vector]); > > + msg.address_hi = upper_32_bits(vdev->msi_iovas[vector]); > > + } > > pci_write_msi_msg(irq, &msg); > > } > > > > -------------------------------------------------------------------- > > > > With that, I think we only need one VFIO change for this part :) > > Wow, is that really OK from a layering perspective? The comment is > pretty clear on the intention that this is to resync the irq layer > view of the device with the physical HW. > > Editing the msi_msg while doing that resync smells bad. > > Also, this is only doing MSI-X, we should include normal MSI as > well. (it probably should have a resync too?) This was added for a specific IBM HBA that clears the vector table during a built-in self test, so it's possible the MSI table being in config space never had the same issue, or we just haven't encountered it. I don't expect anything else actually requires this. > I'd want Thomas/Marc/Alex to agree.. (please read the cover letter for > context) It seems suspect to me too. In a sense it is still just synchronizing the MSI address, but to a different address space. Is it possible to do this with the existing write_msi_msg callback on the msi descriptor? For instance we could simply translate the msg address and call pci_write_msi_msg() (while avoiding an infinite recursion). Or maybe there should be an xlate_msi_msg callback we can register. Or I suppose there might be a way to insert an irqchip that does the translation on write. Thanks, Alex
On ARM GIC systems and others, the target address of the MSI is translated by the IOMMU. For GIC, the MSI address page is called "ITS" page. When the IOMMU is disabled, the MSI address is programmed to the physical location of the GIC ITS page (e.g. 0x20200000). When the IOMMU is enabled, the ITS page is behind the IOMMU, so the MSI address is programmed to an allocated IO virtual address (a.k.a IOVA), e.g. 0xFFFF0000, which must be mapped to the physical ITS page: IOVA (0xFFFF0000) ===> PA (0x20200000). When a 2-stage translation is enabled, IOVA will be still used to program the MSI address, though the mappings will be in two stages: IOVA (0xFFFF0000) ===> IPA (e.g. 0x80900000) ===> 0x20200000 (IPA stands for Intermediate Physical Address). If the device that generates MSI is attached to an IOMMU_DOMAIN_DMA, the IOVA is dynamically allocated from the top of the IOVA space. If attached to an IOMMU_DOMAIN_UNMANAGED (e.g. a VFIO passthrough device), the IOVA is fixed to an MSI window reported by the IOMMU driver via IOMMU_RESV_SW_MSI, which is hardwired to MSI_IOVA_BASE (IOVA==0x8000000) for ARM IOMMUs. So far, this IOMMU_RESV_SW_MSI works well as kernel is entirely in charge of the IOMMU translation (1-stage translation), since the IOVA for the ITS page is fixed and known by kernel. However, with virtual machine enabling a nested IOMMU translation (2-stage), a guest kernel directly controls the stage-1 translation with an IOMMU_DOMAIN_DMA, mapping a vITS page (at an IPA 0x80900000) onto its own IOVA space (e.g. 0xEEEE0000). Then, the host kernel can't know that guest-level IOVA to program the MSI address. To solve this problem the VMM should capture the MSI IOVA allocated by the guest kernel and relay it to the GIC driver in the host kernel, to program the correct MSI IOVA. And this requires a new ioctl via VFIO. Extend the VFIO path to allow an MSI target IOVA to be forwarded into the kernel and pushed down to the GIC driver. Add VFIO ioctl VFIO_IRQ_SET_ACTION_PREPARE with VFIO_IRQ_SET_DATA_MSI_IOVA to carry the data. The downstream calltrace is quite long from the VFIO to the ITS driver. So in order to carry the MSI IOVA from the top to its_irq_domain_alloc(), add patches in a leaf-to-root order: vfio_pci_core_ioctl: vfio_pci_set_irqs_ioctl: vfio_pci_set_msi_prepare: // PATCH-7 pci_alloc_irq_vectors_iovas: // PATCH-6 __pci_alloc_irq_vectors: // PATCH-5 __pci_enable_msi/msix_range: // PATCH-4 msi/msix_capability_init: // PATCH-3 msi/msix_setup_msi_descs: msi_insert_msi_desc(); // PATCH-1 pci_msi_setup_msi_irqs: msi_domain_alloc_irqs_all_locked: __msi_domain_alloc_locked: __msi_domain_alloc_irqs: __irq_domain_alloc_irqs: irq_domain_alloc_irqs_locked: irq_domain_alloc_irqs_hierarchy: msi_domain_alloc: irq_domain_alloc_irqs_parent: its_irq_domain_alloc(); // PATCH-2 Note that this series solves half the problem, since it only allows kernel to set the physical PCI MSI/MSI-X on the device with the correct head IOVA of a 2-stage translation, where the guest kernel does the stage-1 mapping that MSI IOVA (0xEEEE0000) to its own vITS page (0x80900000) while missing the stage-2 mapping from that IPA to the physical ITS page: 0xEEEE0000 ===> 0x80900000 =x=> 0x20200000 A followup series should fill that gap, doing the stage-2 mapping from the vITS page 0x80900000 to the physical ITS page (0x20200000), likely via new IOMMUFD ioctl. Once VMM sets up this stage-2 mapping, VM will act the same as bare metal relying on a running kernel to handle the stage-1 mapping: 0xEEEE0000 ===> 0x80900000 ===> 0x20200000 This series (prototype) is on Github: https://github.com/nicolinc/iommufd/commits/vfio_msi_giova-rfcv1/ It's tested by hacking the host kernel to hard-code a stage-2 mapping. Thanks! Nicolin Nicolin Chen (7): genirq/msi: Allow preset IOVA in struct msi_desc for MSI doorbell address irqchip/gic-v3-its: Bypass iommu_cookie if desc->msi_iova is preset PCI/MSI: Pass in msi_iova to msi_domain_insert_msi_desc PCI/MSI: Allow __pci_enable_msi_range to pass in iova PCI/MSI: Extract a common __pci_alloc_irq_vectors function PCI/MSI: Add pci_alloc_irq_vectors_iovas helper vfio/pci: Allow preset MSI IOVAs via VFIO_IRQ_SET_ACTION_PREPARE drivers/pci/msi/msi.h | 3 +- include/linux/msi.h | 11 +++ include/linux/pci.h | 18 ++++ include/linux/vfio_pci_core.h | 1 + include/uapi/linux/vfio.h | 8 +- drivers/irqchip/irq-gic-v3-its.c | 21 ++++- drivers/pci/msi/api.c | 136 ++++++++++++++++++++---------- drivers/pci/msi/msi.c | 20 +++-- drivers/vfio/pci/vfio_pci_intrs.c | 41 ++++++++- drivers/vfio/vfio_main.c | 3 + kernel/irq/msi.c | 6 ++ 11 files changed, 212 insertions(+), 56 deletions(-)