Joerg Roedel
2020-Mar-04 13:37 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
Hi Michael, On Tue, Mar 03, 2020 at 11:09:41AM -0500, Michael S. Tsirkin wrote:> No. It's coded into the hardware. Which might even be practical > for bare-metal (e.g. on-board flash), but is very practical > when the device is part of a hypervisor.If its that way on PPC, than fine for them. But since this is enablement for x86, it should follow the x86 platform best practices, and that means describing hardware through ACPI.> This "hardware" is actually part of hypervisor so there's no > reason it can't be completely self-descriptive. It's specified > by the same entity as the "firmware".That is just an implementation detail. Yes, QEMU emulates the hardware and builds the ACPI tables. But it could also be implemented in a way where the ACPI tables are build by guest firmware.> I don't see why it would be much faster. The interface isn't that > different from command queues of VTD.VirtIO IOMMU doesn't need to build page-tables that the hypervisor then has to shadow, which makes things much faster. If you emulate one of the other IOMMUs (VT-d or AMD-Vi) the code has to shadow the full page-table at once when device passthrough is used. VirtIO-IOMMU doesn't need that, and that makes it much faster and efficient.> Making ACPI meet the goals of embedded projects such as kata containers > would be a gigantic task with huge stability implications. By > comparison this 400-line parser is well contained and does the job. I > didn't yet see compelling reasons not to merge this, but I'll be > interested to see some more specific concerns.An ACPI table parse wouldn't need more lines of code. For embedded systems there is still the DT way of describing things. Regards, Joerg
Jean-Philippe Brucker
2020-Mar-04 15:38 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
On Wed, Mar 04, 2020 at 02:37:08PM +0100, Joerg Roedel wrote:> Hi Michael, > > On Tue, Mar 03, 2020 at 11:09:41AM -0500, Michael S. Tsirkin wrote: > > No. It's coded into the hardware. Which might even be practical > > for bare-metal (e.g. on-board flash), but is very practical > > when the device is part of a hypervisor. > > If its that way on PPC, than fine for them. But since this is enablement > for x86, it should follow the x86 platform best practices, and that > means describing hardware through ACPI.I agree with this. The problem is I don't know how to get a new ACPI table or change an existing one. It needs to go through the UEFI forum in order to be accepted, and I don't have any weight there. I've been trying to get the tiny change into IORT for ages. I haven't been given any convincing reason against it or offered any alternative, it's just stalled. The topology description introduced here wasn't my first choice either but unless someone can help finding another way into ACPI, I don't have a better idea. Thanks, Jean
Joerg Roedel
2020-Mar-04 17:40 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
On Wed, Mar 04, 2020 at 04:38:21PM +0100, Jean-Philippe Brucker wrote:> I agree with this. The problem is I don't know how to get a new ACPI table > or change an existing one. It needs to go through the UEFI forum in order > to be accepted, and I don't have any weight there. I've been trying to get > the tiny change into IORT for ages. I haven't been given any convincing > reason against it or offered any alternative, it's just stalled. The > topology description introduced here wasn't my first choice either but > unless someone can help finding another way into ACPI, I don't have a > better idea.A quote from the ACPI Specification (Version 6.3, Section 5.2.6, Page 119): Table signatures will be reserved by the ACPI promoters and posted independently of this specification in ACPI errata and clarification documents on the ACPI web site. Requests to reserve a 4-byte alphanumeric table signature should be sent to the email address info at acpi.info and should include the purpose of the table and reference URL to a document that describes the table format. Tables defined outside of the ACPI specification may define data value encodings in either little endian or big endian format. For the purpose of clarity, external table definition documents should include the endian-ness of their data value encodings. So it sounds like you need to specifiy the table format and send a request to info at acpi.info to get a table signature for it. Regards, Joerg
Michael S. Tsirkin
2020-Mar-04 19:34 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
On Wed, Mar 04, 2020 at 02:37:08PM +0100, Joerg Roedel wrote:> Hi Michael, > > On Tue, Mar 03, 2020 at 11:09:41AM -0500, Michael S. Tsirkin wrote: > > No. It's coded into the hardware. Which might even be practical > > for bare-metal (e.g. on-board flash), but is very practical > > when the device is part of a hypervisor. > > If its that way on PPC, than fine for them. But since this is enablement > for x86, it should follow the x86 platform best practices, and that > means describing hardware through ACPI.For hardware, sure. Hypervisors aren't hardware though and a bunch of hypervisors don't use ACPI.> > This "hardware" is actually part of hypervisor so there's no > > reason it can't be completely self-descriptive. It's specified > > by the same entity as the "firmware". > > That is just an implementation detail. Yes, QEMU emulates the hardware > and builds the ACPI tables. But it could also be implemented in a way > where the ACPI tables are build by guest firmware.All these extra levels of indirection is one of the reasons hypervisors such as kata try to avoid ACPI.> > I don't see why it would be much faster. The interface isn't that > > different from command queues of VTD. > > VirtIO IOMMU doesn't need to build page-tables that the hypervisor then > has to shadow, which makes things much faster. If you emulate one of the > other IOMMUs (VT-d or AMD-Vi) the code has to shadow the full page-table > at once when device passthrough is used. VirtIO-IOMMU doesn't need that, > and that makes it much faster and efficient.IIUC VT-d at least supports range invalidations.> > > Making ACPI meet the goals of embedded projects such as kata containers > > would be a gigantic task with huge stability implications. By > > comparison this 400-line parser is well contained and does the job. I > > didn't yet see compelling reasons not to merge this, but I'll be > > interested to see some more specific concerns. > > An ACPI table parse wouldn't need more lines of code.It realies on ACPI OSPM itself to handle ACPI bytecode, which is huge.> For embedded > systems there is still the DT way of describing things.For some embedded systems.> Regards, > > Joerg
Joerg Roedel
2020-Mar-04 21:50 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
On Wed, Mar 04, 2020 at 02:34:33PM -0500, Michael S. Tsirkin wrote:> All these extra levels of indirection is one of the reasons > hypervisors such as kata try to avoid ACPI.Platforms that don't use ACPI need another hardware detection mechanism, which can also be supported. But the first step here is to enable the general case, and for x86 platforms this means ACPI. Regards, Joerg
Joerg Roedel
2020-Mar-04 21:54 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
On Wed, Mar 04, 2020 at 01:37:44PM -0800, Jacob Pan (Jun) wrote:> + Mike and Erik who work closely on UEFI and ACPICA. > > Copy paste Erik's initial response below on how to get a new table, > seems to confirm with the process you stated above. > > "Fairly easy. You reserve a 4-letter symbol by sending a message > requesting to reserve the signature to Mike or the ASWG mailing list or > info at acpi.infoGreat! I think this is going to be the path forward here. Regards, Joerg> > There is also another option. You can have ASWG own this new table so > that not one entity or company owns the new table."
Michael S. Tsirkin
2020-Mar-05 15:42 UTC
[PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space
On Wed, Mar 04, 2020 at 10:54:49PM +0100, Joerg Roedel wrote:> On Wed, Mar 04, 2020 at 01:37:44PM -0800, Jacob Pan (Jun) wrote: > > + Mike and Erik who work closely on UEFI and ACPICA. > > > > Copy paste Erik's initial response below on how to get a new table, > > seems to confirm with the process you stated above. > > > > "Fairly easy. You reserve a 4-letter symbol by sending a message > > requesting to reserve the signature to Mike or the ASWG mailing list or > > info at acpi.info > > Great! I think this is going to be the path forward here. > > Regards, > > JoergI don't, I don't see why we should bother with ACPI. This is a PV device anyway, we can just make it self-describing. The need for extra firmware on some platforms is a bug not a feature. No point in emulating that.> > > > There is also another option. You can have ASWG own this new table so > > that not one entity or company owns the new table."
Apparently Analagous 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
- [PATCH v2 1/3] iommu/virtio: Add topology description to virtio-iommu config space