Joerg Roedel
2020-Mar-02 16:16 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
On Fri, Feb 28, 2020 at 06:25:36PM +0100, Jean-Philippe Brucker wrote:> This solution isn't elegant nor foolproof, but is the best we can do at > the moment and works with existing virtio-iommu implementations. It also > enables an IOMMU for lightweight hypervisors that do not rely on > firmware methods for booting.I appreciate the enablement on x86, but putting the conmfiguration into mmio-space isn't really something I want to see upstream. What is the problem with defining an ACPI table instead? This would also make things work on AARCH64 UEFI machines. Regards, Joerg
Auger Eric
2020-Mar-03 10:19 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
Hi Joerg, On 3/2/20 5:16 PM, Joerg Roedel wrote:> On Fri, Feb 28, 2020 at 06:25:36PM +0100, Jean-Philippe Brucker wrote: >> This solution isn't elegant nor foolproof, but is the best we can do at >> the moment and works with existing virtio-iommu implementations. It also >> enables an IOMMU for lightweight hypervisors that do not rely on >> firmware methods for booting. > > I appreciate the enablement on x86, but putting the conmfiguration into > mmio-space isn't really something I want to see upstream. What is the > problem with defining an ACPI table instead? This would also make things > work on AARCH64 UEFI machines.Michael has pushed this solution (putting the "configuration in the PCI config space"), I think for those main reasons: - ACPI may not be supported on some archs/hyps - the virtio-iommu is a PCIe device so binding should not need ACPI description Another issue with ACPI integration is we have different flavours of tables that exist: IORT, DMAR, IVRS x86 ACPI integration was suggested with IORT. But it seems ARM is reluctant to extend IORT to support para-virtualized IOMMU. So the VIOT was proposed as a different atternative in "[RFC 00/13] virtio-iommu on non-devicetree platforms" (https://patchwork.kernel.org/cover/11257727/). Proposing a table that may be used by different vendors seems to be a challenging issue here. So even if the above solution does not look perfect, it looked a sensible compromise given the above arguments. Please could you explain what are the most compelling arguments against it? Thanks Eric> > Regards, > > Joerg >
Joerg Roedel
2020-Mar-03 13:01 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
Hi Eric, On Tue, Mar 03, 2020 at 11:19:20AM +0100, Auger Eric wrote:> Michael has pushed this solution (putting the "configuration in the PCI > config space"), I think for those main reasons: > - ACPI may not be supported on some archs/hypsBut on those there is device-tree, right?> - the virtio-iommu is a PCIe device so binding should not need ACPI > descriptionThe other x86 IOMMUs are PCI devices too and they definitly need a ACPI table to be configured.> Another issue with ACPI integration is we have different flavours of > tables that exist: IORT, DMAR, IVRSAn integration with IORT might be the best, but if it is not possible ther can be a new table-type for Virtio-iommu. That would still follow platform best practices.> x86 ACPI integration was suggested with IORT. But it seems ARM is > reluctant to extend IORT to support para-virtualized IOMMU. So the VIOT > was proposed as a different atternative in "[RFC 00/13] virtio-iommu on > non-devicetree platforms" > (https://patchwork.kernel.org/cover/11257727/). Proposing a table that > may be used by different vendors seems to be a challenging issue here.Yeah, if I am reading that right this proposes a one-fits-all solution. That is not needed as the other x86 IOMMUs already have their tables defined and implemented. There is no need to change anything there.> So even if the above solution does not look perfect, it looked a > sensible compromise given the above arguments. Please could you explain > what are the most compelling arguments against it?It is a hack and should be avoided if at all possible. And defining an own ACPI table type seems very much possible. Regards, Joerg
Michael S. Tsirkin
2020-Mar-03 14:02 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
On Mon, Mar 02, 2020 at 05:16:12PM +0100, Joerg Roedel wrote:> On Fri, Feb 28, 2020 at 06:25:36PM +0100, Jean-Philippe Brucker wrote: > > This solution isn't elegant nor foolproof, but is the best we can do at > > the moment and works with existing virtio-iommu implementations. It also > > enables an IOMMU for lightweight hypervisors that do not rely on > > firmware methods for booting. > > I appreciate the enablement on x86, but putting the conmfiguration into > mmio-space isn't really something I want to see upstream.It's in virtio config space, not in mmio-space. With a PCI virtio-IOMMU device this will be in memory.> What is the > problem with defining an ACPI table instead? This would also make things > work on AARCH64 UEFI machines. > > Regards, > > Joerg
Seemingly Similar Threads
- [PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
- [PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
- [PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
- [PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
- [RFC 00/13] virtio-iommu on non-devicetree platforms