Re: [PATCH v1 0/4] virt: vmgenid: Add devicetree bindings support

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3624 bytes --]

On Fri, 2024-03-22 at 06:40 +0100, Krzysztof Kozlowski wrote:
> On 21/03/2024 18:39, Landge, Sudan wrote:
> > 
> > 
> > On 21/03/2024 13:32, Rob Herring wrote:
> > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> > > 
> > > 
> > > 
> > > On Wed, Mar 20, 2024 at 04:55:45PM +0000, David Woodhouse wrote:
> > > > On Wed, 2024-03-20 at 11:15 -0500, Rob Herring wrote:
> > > > > On Wed, Mar 20, 2024 at 01:50:43PM +0000, David Woodhouse wrote:
> > > > > > On Tue, 2024-03-19 at 16:24 +0100, Krzysztof Kozlowski wrote:
> > > > > > > On 19/03/2024 15:32, Sudan Landge wrote:
> > > > > > > > This small series of patches aims to add devicetree bindings support for
> > > > > > > > the Virtual Machine Generation ID (vmgenid) driver.
> > > > > > > > 
> > > > > > > > Virtual Machine Generation ID driver was introduced in commit af6b54e2b5ba
> > > > > > > > ("virt: vmgenid: notify RNG of VM fork and supply generation ID") as an
> > > > > > > > ACPI only device.
> > > > > > > > We would like to extend vmgenid to support devicetree bindings because:
> > > > > > > > 1. A device should not be defined as an ACPI or DT only device.
> > > > > 
> > > > > This (and the binding patch) tells me nothing about what "Virtual
> > > > > Machine Generation ID driver" is and isn't really justification for
> > > > > "why".
> > > > 
> > > > It's a reference to a memory area which the OS can use to tell whether
> > > > it's been snapshotted and restored (or 'forked'). A future submission
> > > > should have a reference to something like
> > > > https://www.qemu.org/docs/master/specs/vmgenid.html or the Microsoft
> > > > doc which is linked from there.
> > > 
> > > That doc mentions fw_cfg for which we already have a binding. Why can't
> > > it be used/extended here?
> > QEMU has support for vmgenid but even they do not pass vmgenid directly 
> > to the guest kernel using fw_cfg. QEMU passes the vmgenid/UUID via 
> > fw_cfg to an intermediate UEFI firmware. This UEFI firmware, running as 
> > a guest in QEMU, reads the UUID from fw_cfg and creates ACPI tables for 
> > it. The UEFI firmware then passes the UUID information to the guest
> > kernel via ACPI.
> > This approach requires yet another intermediary which is UEFI firmware 
> > and adds more complexity than ACPI for minimalist hypervisors that do 
> > not have an intermediate bootloader today.
> 
> What stops you from passing fw_cfg not to UEFI FW? BTW, no actual VM
> name was used in your posting, but now suddenly it is a talk about QEMU.

That would be possible. But not ideal. Just as exposing it via PCI
would be possible, but not ideal. Or forcing ACPI onto the guests in
question, and various other less efficient options.

If what we're really looking at here is a hostile takeover of the DT
bindings repository, with a blanket "No, DT is dead. Go use something
else, preferably ACPI", than all those other options are possible. We
*never* have to add a new binding to DT ever again. Let's just set the
existing bindings in stone and move on.

But hopefully that isn't the case. DT is the simplest and most
effective way to provide discovery, especially for embedded and microVM
systems. It isn't just a *workaround* for broken hardware which *can't*
do a slower and more complex form of discovery. 

And it's absolutely the right thing to do for exposing the equivalent
of the ACPI vmgenid device in a system which isn't afflicted by ACPI
and doesn't *want* to be.


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]