Re: [PATCH v4] virt: vmgenid: introduce driver for reinitializing RNG on VM fork

[Alexander Graf <graf@amazon.com>]

On 25.02.22 16:16, Ard Biesheuvel wrote:
> On Fri, 25 Feb 2022 at 16:12, Alexander Graf <graf@amazon.com> wrote:
>>
>> On 25.02.22 15:33, Jason A. Donenfeld wrote:
>>> On Fri, Feb 25, 2022 at 03:18:43PM +0100, Alexander Graf wrote:
>>>>> I recall this part of the old thread. From what I understood, using
>>>>> "VMGENID" + "QEMUVGID" worked /well enough/, even if that wasn't
>>>>> technically in-spec. Ard noted that relying on _CID like that is
>>>>> technically an ACPI spec notification. So we're between one spec and
>>>>> another, basically, and doing "VMGENID" + "QEMUVGID" requires fewer
>>>>> changes, as mentioned, appears to work fine in my testing.
>>>>>
>>>>> However, with that said, I think supporting this via "VM_Gen_Counter"
>>>>> would be a better eventual thing to do, but will require acks and
>>>>> changes from the ACPI maintainers. Do you think you could prepare your
>>>>> patch proposal above as something on-top of my tree [1]? And if you can
>>>>> convince the ACPI maintainers that that's okay, then I'll happily take
>>>>> the patch.
>>>> Sure, let me send the ACPI patch stand alone. No need to include the
>>>> VMGenID change in there.
>>> That's fine. If the ACPI people take it for 5.18, then we can count on
>>> it being there and adjust the vmgenid driver accordingly also for 5.18.
>>>
>>> I just booted up a Windows VM, and it looks like Hyper-V uses
>>> "Hyper_V_Gen_Counter_V1", which is also quite long, so we can't really
>>> HID match on that either.
>>
>> Yes, due to the same problem. I'd really prefer we sort out the ACPI
>> matching before this goes mainline. Matching on _HID is explicitly
>> discouraged in the VMGenID spec.
>>
> OK, this really sucks. Quoting the ACPI spec:
>
> """
> A _HID object evaluates to either a numeric 32-bit compressed EISA
> type ID or a string. If a string, the format must be an alphanumeric
> PNP or ACPI ID with no asterisk or other leading characters.
> A valid PNP ID must be of the form "AAA####" where A is an uppercase
> letter and # is a hex digit.
> A valid ACPI ID must be of the form "NNNN####" where N is an uppercase
> letter or a digit ('0'-'9') and # is a hex digit. This specification
> reserves the string "ACPI" for use only with devices defined herein.
> It further reserves all strings representing 4 HEX digits for
> exclusive use with PCI-assigned Vendor IDs.
> """
>
> So now we have to implement Microsoft's fork of ACPI to be able to use
> this device, even if we expose it from QEMU instead of Hyper-V? I
> strongly object to that.
>
> Instead, we can match on _HID exposed by QEMU, and cordially invite
> Microsoft to align their spec with the ACPI spec.


Doing that would be a backwards incompatible change for Hyper-V, no? I 
understand that you're upset about their spec, but that doesn't mean we 
can't find a path forward to make it all compatible.

IMHO just matching on the first 9 bytes of the _CID/_HID string is 
perfectly fine. It follows the spec, but still allows for weird 
identifiers like this one to work.

I don't understand the rush here. This had been sitting on the ML for 1 
year - and now suddenly talking the match through properly and getting 
VMGenID spec compatible matching support into the ACPI core is a 
problem? What did I miss? :)


Alex




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879