On 24-10-2026 04:34, Matt Roper wrote:
Fine, I will keep them here only.On Fri, Oct 23, 2026 at 04:32:48AM +0200, Raag Jadav wrote:On Thu, Oct 22, 2026 at 10:24:45PM +0530, Nilawar, Badal wrote:On 22-10-2026 19:10, Nilawar, Badal wrote:On 14-10-2026 16:00, Raag Jadav wrote:+static const struct xe_platform_id_entry xe_platform_ids[] = { + /* 0x674C platform/8086:674c */ + { 0x674C, GUID_INIT(0x9046afe5, 0x9041, 0x5124, + 0x86, 0x14, 0x92, 0x55, 0x0d, 0x9e, 0x9d, 0xa6) }, + /* 0x674D platform/8086:674d */ + { 0x674D, GUID_INIT(0xf95911b8, 0x6564, 0x5905, + 0xa6, 0xa7, 0xb9, 0xbf, 0x54, 0xab, 0x6c, 0x27) }, + /* 0x674E platform/8086:674e */ + { 0x674E, GUID_INIT(0xb8d24106, 0x0108, 0x59f1, + 0x8f, 0xfc, 0x9f, 0xc4, 0xa9, 0xc9, 0xb7, 0xb0) }, + /* 0x674F platform/8086:674f */ + { 0x674F, GUID_INIT(0xdd8dd88e, 0xef2f, 0x59b1, + 0x99, 0x15, 0xdb, 0x6f, 0x1d, 0xc9, 0xb4, 0x5d) }, + /* 0x6750 platform/8086:6750 */ + { 0x6750, GUID_INIT(0x1dd66c44, 0x87d5, 0x524a, + 0xad, 0x6e, 0xfc, 0x18, 0x9d, 0x42, 0x6f, 0x1a) }, + /* 0x6751 platform/8086:6751 */ + { 0x6751, GUID_INIT(0x17d0dd04, 0x2fad, 0x5400, + 0x99, 0xc8, 0x9b, 0x45, 0x51, 0x3b, 0x67, 0xe3) }, +};This is unrelated to cper and shouldn't be here. I'll rely on Matt's input on how to deal with these.Agreed, I don't think this is the ideal place either. For now, I'll keep it here and, based on Matt's input, consider moving it to xe_pci.c, which maintains the PCI ID list for the platforms supported by Xe.One reason I placed the table here is that these aren’t general-purpose platform GUIDs; they’re UUIDv5 values generated specifically for CPER’s platform_id field, using platform/8086:<device-id> as the name. Since CPER is their only consumer, keeping the mapping with the CPER code seems reasonable to me. I’m happy to follow Matt’s input if there’s a preferred place for this kind of mapping.My only concern is that we usually don't have device ids lying around in random components, but that I think is upto the maintainers.Yeah, I'm not familiar with these structures. If they're CPER-specific, then keeping them in the CPER file seems natural to me.But it's also true that it could be easy for these to get out of sync with our general PCI ID list if PCI IDs are added/removed. If a new PCI ID shows up, how do we know what the values should be here? Are they documented in the bspec somewhere?No these are generated as using uuidgen utility as UUIDv5.
$ INTEL_NS=$(uuidgen --sha1 -n @dns -N "intel.com")
uuidgen --sha1 -n "$INTEL_NS" -N "platform/8086:674c"
9046afe5-9041-5124-8614-92550d9e9da6
How do these values wind up getting used?This UUID is populated in CPER record to identify the platform/SKU associated with it.
Are there uapi / ABI implications of these values?No uapi implications. As they are part of CPER record should remain stable for given platform/PCI ID.
I am considering generating the platform UUID dynamically using UUIDv5, with the PCI vendor/device ID as the name.
This keeps the UUID deterministic for a given PCI ID and avoids maintaining a separate mapping table when PCI IDs are added or removed.
+/* Namespace: uuidgen --sha1 -n @dns -N "intel.com" */
+#define INTEL_CPER_NAMESPACE \
+ UUID_INIT(0x3e54f727, 0x549c, 0x5ab3, \
+ 0x90, 0x73, 0x52, 0x2a, 0x81, 0x15, 0x20, 0xe7)
+
+/*
+ * Platform GUID is UUIDv5 (RFC 9562) of "platform/8086:<devid>" in the
+ * Intel CPER namespace, stored in the mixed-endian guid_t layout used by CPER.
+ */
+static void cper_platform_id(const struct pci_dev *pdev, guid_t *guid)
+{
+ static const uuid_t ns = INTEL_CPER_NAMESPACE;
+ u8 digest[SHA1_DIGEST_SIZE];
+ struct sha1_ctx ctx;
+ char name[sizeof("platform/8086:ffff")];
+ int len;
+
+ len = scnprintf(name, sizeof(name), "platform/%04x:%04x",
+ pdev->vendor, pdev->device);
+
+ sha1_init(&ctx);
+ sha1_update(&ctx, ns.b, UUID_SIZE);
+ sha1_update(&ctx, name, len);
+ sha1_final(&ctx, digest);
+
+ digest[6] = (digest[6] & 0x0f) | 0x50;
+ digest[8] = (digest[8] & 0x3f) | 0x80;
+
+ *guid = GUID_INIT(get_unaligned_be32(&digest[0]),
+ get_unaligned_be16(&digest[4]),
+ get_unaligned_be16(&digest[6]),
+ digest[8], digest[9], digest[10], digest[11],
+ digest[12], digest[13], digest[14], digest[15]);
}
If we want to go this way, IMO this logic should be placed in lib/uuid.c and not in Xe. But ...
Aside from everything else, should we also add some kind of kunit test that makes sure we each PCI ID has an entry in this table and vice-versa to make sure we don't forget to update this table when platform PCI IDs are added/removed?Agree, Kunit can be added as follow up.
... any reason the Kunit isn't enough with a static table? If we add it immediately instead of later, we'd be sure to get a CI failure if a new PCI ID is added without the matching GUID. Compared to the dynamic generation, we wouldn't get the GUID for free if we get a new PCI ID, but overall it should be less work IMO.
Daniele
Thanks,
BadalMattRaag