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.