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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 25.02.22 15:12, Jason A. Donenfeld wrote:
Hi Alex,

On Fri, Feb 25, 2022 at 02:57:38PM +0100, Alexander Graf wrote:
+static const struct acpi_device_id vmgenid_ids[] = {
+       { "VMGENID", 0 },
+       { "QEMUVGID", 0 },

According to the VMGenID spec[1], you can only rely on _CID and _DDN for
matching. They both contain "VM_Gen_Counter". The list above contains
_HID values which are not an official identifier for the VMGenID device.

IIRC the ACPI device match logic does match _CID in addition to _HID.
However, it is limited to 8 characters. Let me paste an experimental
hack I did back then to do the _CID matching instead.

[1]
https://download.microsoft.com/download/3/1/C/31CFC307-98CA-4CA5-914C-D9772691E214/VirtualMachineGenerationID.docx


Alex

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 1682f8b454a2..452443d79d87 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -748,7 +748,7 @@ static bool __acpi_match_device(struct acpi_device
*device,
           /* First, check the ACPI/PNP IDs provided by the caller. */
           if (acpi_ids) {
               for (id = acpi_ids; id->id[0] || id->cls; id++) {
-                if (id->id[0] && !strcmp((char *)id->id, hwid->id))
+                if (id->id[0] && !strncmp((char *)id->id, hwid->id,
ACPI_ID_LEN - 1))
                       goto out_acpi_match;
                   if (id->cls && __acpi_match_device_cls(id, hwid))
                       goto out_acpi_match;
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index 75a787da8aad..0bfa422cf094 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -356,7 +356,8 @@ static void vmgenid_acpi_notify(struct acpi_device
*device, u32 event)
   }

   static const struct acpi_device_id vmgenid_ids[] = {
-    {"QEMUVGID", 0},
+    /* This really is VM_Gen_Counter, but we can only match 8 characters */
+    {"VM_GEN_C", 0},
       {"", 0},
   };
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.


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






[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux