hw/vfio/pci.c | 25 +++++++++++++++++++++++++ hw/vfio/pci.h | 1 + hw/vfio/trace-events | 1 + 3 files changed, 27 insertions(+)
Introduce x-pci-class-code option to allow users to override PCI class
code of a device, similar to the existing x-pci-vendor-id option. Only
the lower 24 bits of this option are used, though a uint32 is used here
for determining whether the value is valid and set by user.
Additionally, to prevent exposing VGA ranges on non-VGA devices, the
x-vga=on option requires x-pci-class-code is either unset or set to
VGA controller class.
This is mainly intended for IGD devices that expose themselves either
as VGA controller (primary display) or Display controller (non-primary
display). The UEFI GOP driver depends on the device reporting a VGA
controller class code (0x030000).
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
v2:
* Add vdev class code check in vfio_populate_vga().
* Fix type in trace-events.
Link: https://lore.kernel.org/all/20250524153102.19747-1-tomitamoeko@gmail.com/
hw/vfio/pci.c | 25 +++++++++++++++++++++++++
hw/vfio/pci.h | 1 +
hw/vfio/trace-events | 1 +
3 files changed, 27 insertions(+)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b1250d85bf..d57cb7356e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2726,6 +2726,14 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
return false;
}
+ /* vdev class should be either unmodified (PCI_ANY_ID), or VGA controller */
+ if ((vdev->class_code != PCI_ANY_ID) &&
+ (vdev->class_code != (PCI_CLASS_DISPLAY_VGA << 8)) &&
+ (vdev->class_code != (PCI_CLASS_NOT_DEFINED_VGA << 8))) {
+ error_setg(errp, "vdev is not a VGA device");
+ return false;
+ }
+
if (!(reg_info->flags & VFIO_REGION_INFO_FLAG_READ) ||
!(reg_info->flags & VFIO_REGION_INFO_FLAG_WRITE) ||
reg_info->size < 0xbffff + 1) {
@@ -3092,6 +3100,21 @@ static bool vfio_pci_config_setup(VFIOPCIDevice *vdev, Error **errp)
vdev->sub_device_id);
}
+ /*
+ * Class code is a 24-bit value at config space 0x09. Allow overriding it
+ * with any 24-bit value.
+ */
+ if (vdev->class_code != PCI_ANY_ID) {
+ if (vdev->class_code > 0xffffff) {
+ error_setg(errp, "invalid PCI class code provided");
+ return false;
+ }
+ /* Higher 24 bits of PCI_CLASS_REVISION are class code */
+ vfio_add_emulated_long(vdev, PCI_CLASS_REVISION,
+ vdev->class_code << 8, ~0xff);
+ trace_vfio_pci_emulated_class_code(vbasedev->name, vdev->class_code);
+ }
+
/* QEMU can change multi-function devices to single function, or reverse */
vdev->emulated_config_bits[PCI_HEADER_TYPE] =
PCI_HEADER_TYPE_MULTI_FUNCTION;
@@ -3489,6 +3512,8 @@ static const Property vfio_pci_dev_properties[] = {
sub_vendor_id, PCI_ANY_ID),
DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
sub_device_id, PCI_ANY_ID),
+ DEFINE_PROP_UINT32("x-pci-class-code", VFIOPCIDevice,
+ class_code, PCI_ANY_ID),
DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
DEFINE_PROP_UNSIGNED_NODEFAULT("x-nv-gpudirect-clique", VFIOPCIDevice,
nv_gpudirect_clique,
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 5ce0fb916f..587eb8cc9a 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -156,6 +156,7 @@ struct VFIOPCIDevice {
uint32_t device_id;
uint32_t sub_vendor_id;
uint32_t sub_device_id;
+ uint32_t class_code;
uint32_t features;
#define VFIO_FEATURE_ENABLE_VGA_BIT 0
#define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT)
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index e90ec9bff8..e8d585b49a 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -46,6 +46,7 @@ vfio_pci_emulated_vendor_id(const char *name, uint16_t val) "%s 0x%04x"
vfio_pci_emulated_device_id(const char *name, uint16_t val) "%s 0x%04x"
vfio_pci_emulated_sub_vendor_id(const char *name, uint16_t val) "%s 0x%04x"
vfio_pci_emulated_sub_device_id(const char *name, uint16_t val) "%s 0x%04x"
+vfio_pci_emulated_class_code(const char *name, uint32_t val) "%s 0x%06x"
# pci-quirks.c
vfio_quirk_rom_in_denylist(const char *name, uint16_t vid, uint16_t did) "%s %04x:%04x"
--
2.47.2On Wed, 28 May 2025 23:55:48 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:
> Introduce x-pci-class-code option to allow users to override PCI class
> code of a device, similar to the existing x-pci-vendor-id option. Only
> the lower 24 bits of this option are used, though a uint32 is used here
> for determining whether the value is valid and set by user.
>
> Additionally, to prevent exposing VGA ranges on non-VGA devices, the
> x-vga=on option requires x-pci-class-code is either unset or set to
> VGA controller class.
>
> This is mainly intended for IGD devices that expose themselves either
> as VGA controller (primary display) or Display controller (non-primary
> display). The UEFI GOP driver depends on the device reporting a VGA
> controller class code (0x030000).
>
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
> v2:
> * Add vdev class code check in vfio_populate_vga().
> * Fix type in trace-events.
> Link: https://lore.kernel.org/all/20250524153102.19747-1-tomitamoeko@gmail.com/
>
> hw/vfio/pci.c | 25 +++++++++++++++++++++++++
> hw/vfio/pci.h | 1 +
> hw/vfio/trace-events | 1 +
> 3 files changed, 27 insertions(+)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index b1250d85bf..d57cb7356e 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2726,6 +2726,14 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
> return false;
> }
>
> + /* vdev class should be either unmodified (PCI_ANY_ID), or VGA controller */
> + if ((vdev->class_code != PCI_ANY_ID) &&
> + (vdev->class_code != (PCI_CLASS_DISPLAY_VGA << 8)) &&
> + (vdev->class_code != (PCI_CLASS_NOT_DEFINED_VGA << 8))) {
> + error_setg(errp, "vdev is not a VGA device");
> + return false;
> + }
I think we should follow the scheme used for vendor_id and device_id to
populate the struct field when not specified. That let's us use it
more easily and consistently for things like this.
I think we can ignore PCI_CLASS_NOT_DEFINED_VGA. The PCI Local Bus
Specification 2.1, dated June 1, 1995 (earliest I can find on the SIG)
includes the VGA class code and specifies base class 0x00 is reserved
for compatibility with devices built before the base class field was
defined, so at least before 1995. Also, neither the kernel or QEMU
is_vga helpers account for this, so they'd not have a VGA region or be
properly detected elsewhere. Thanks,
Alex
> +
> if (!(reg_info->flags & VFIO_REGION_INFO_FLAG_READ) ||
> !(reg_info->flags & VFIO_REGION_INFO_FLAG_WRITE) ||
> reg_info->size < 0xbffff + 1) {
> @@ -3092,6 +3100,21 @@ static bool vfio_pci_config_setup(VFIOPCIDevice *vdev, Error **errp)
> vdev->sub_device_id);
> }
>
> + /*
> + * Class code is a 24-bit value at config space 0x09. Allow overriding it
> + * with any 24-bit value.
> + */
> + if (vdev->class_code != PCI_ANY_ID) {
> + if (vdev->class_code > 0xffffff) {
> + error_setg(errp, "invalid PCI class code provided");
> + return false;
> + }
> + /* Higher 24 bits of PCI_CLASS_REVISION are class code */
> + vfio_add_emulated_long(vdev, PCI_CLASS_REVISION,
> + vdev->class_code << 8, ~0xff);
> + trace_vfio_pci_emulated_class_code(vbasedev->name, vdev->class_code);
> + }
> +
> /* QEMU can change multi-function devices to single function, or reverse */
> vdev->emulated_config_bits[PCI_HEADER_TYPE] =
> PCI_HEADER_TYPE_MULTI_FUNCTION;
> @@ -3489,6 +3512,8 @@ static const Property vfio_pci_dev_properties[] = {
> sub_vendor_id, PCI_ANY_ID),
> DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
> sub_device_id, PCI_ANY_ID),
> + DEFINE_PROP_UINT32("x-pci-class-code", VFIOPCIDevice,
> + class_code, PCI_ANY_ID),
> DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
> DEFINE_PROP_UNSIGNED_NODEFAULT("x-nv-gpudirect-clique", VFIOPCIDevice,
> nv_gpudirect_clique,
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 5ce0fb916f..587eb8cc9a 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -156,6 +156,7 @@ struct VFIOPCIDevice {
> uint32_t device_id;
> uint32_t sub_vendor_id;
> uint32_t sub_device_id;
> + uint32_t class_code;
> uint32_t features;
> #define VFIO_FEATURE_ENABLE_VGA_BIT 0
> #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT)
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index e90ec9bff8..e8d585b49a 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -46,6 +46,7 @@ vfio_pci_emulated_vendor_id(const char *name, uint16_t val) "%s 0x%04x"
> vfio_pci_emulated_device_id(const char *name, uint16_t val) "%s 0x%04x"
> vfio_pci_emulated_sub_vendor_id(const char *name, uint16_t val) "%s 0x%04x"
> vfio_pci_emulated_sub_device_id(const char *name, uint16_t val) "%s 0x%04x"
> +vfio_pci_emulated_class_code(const char *name, uint32_t val) "%s 0x%06x"
>
> # pci-quirks.c
> vfio_quirk_rom_in_denylist(const char *name, uint16_t vid, uint16_t did) "%s %04x:%04x"
On 2025/5/29 2:30, Alex Williamson wrote:
> On Wed, 28 May 2025 23:55:48 +0800
> Tomita Moeko <tomitamoeko@gmail.com> wrote:
>
>> Introduce x-pci-class-code option to allow users to override PCI class
>> code of a device, similar to the existing x-pci-vendor-id option. Only
>> the lower 24 bits of this option are used, though a uint32 is used here
>> for determining whether the value is valid and set by user.
>>
>> Additionally, to prevent exposing VGA ranges on non-VGA devices, the
>> x-vga=on option requires x-pci-class-code is either unset or set to
>> VGA controller class.
>>
>> This is mainly intended for IGD devices that expose themselves either
>> as VGA controller (primary display) or Display controller (non-primary
>> display). The UEFI GOP driver depends on the device reporting a VGA
>> controller class code (0x030000).
>>
>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
>> ---
>> v2:
>> * Add vdev class code check in vfio_populate_vga().
>> * Fix type in trace-events.
>> Link: https://lore.kernel.org/all/20250524153102.19747-1-tomitamoeko@gmail.com/
>>
>> hw/vfio/pci.c | 25 +++++++++++++++++++++++++
>> hw/vfio/pci.h | 1 +
>> hw/vfio/trace-events | 1 +
>> 3 files changed, 27 insertions(+)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index b1250d85bf..d57cb7356e 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -2726,6 +2726,14 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
>> return false;
>> }
>>
>> + /* vdev class should be either unmodified (PCI_ANY_ID), or VGA controller */
>> + if ((vdev->class_code != PCI_ANY_ID) &&
>> + (vdev->class_code != (PCI_CLASS_DISPLAY_VGA << 8)) &&
>> + (vdev->class_code != (PCI_CLASS_NOT_DEFINED_VGA << 8))) {
>> + error_setg(errp, "vdev is not a VGA device");
>> + return false;
>> + }
>
> I think we should follow the scheme used for vendor_id and device_id to
> populate the struct field when not specified. That let's us use it
> more easily and consistently for things like this.
Hi, Alex
The class code override takes place in vfio_pci_config_setup(), where
is after vfio_populate_vga() is called in vfio_populate_device(). So
I have to check if it equals to default or VGA class code here, and
not initializing the sturct field with device value. If we decide to
initialize it for other purpose, I personally think we should also
save the subvendor/subdevice value as well.
> I think we can ignore PCI_CLASS_NOT_DEFINED_VGA. The PCI Local Bus
> Specification 2.1, dated June 1, 1995 (earliest I can find on the SIG)
> includes the VGA class code and specifies base class 0x00 is reserved
> for compatibility with devices built before the base class field was
> defined, so at least before 1995. Also, neither the kernel or QEMU
> is_vga helpers account for this, so they'd not have a VGA region or be
> properly detected elsewhere. Thanks,
>
> Alex
Got it, will remove this in v3.
Thanks,
Moeko
>> +
>> if (!(reg_info->flags & VFIO_REGION_INFO_FLAG_READ) ||
>> !(reg_info->flags & VFIO_REGION_INFO_FLAG_WRITE) ||
>> reg_info->size < 0xbffff + 1) {
>> @@ -3092,6 +3100,21 @@ static bool vfio_pci_config_setup(VFIOPCIDevice *vdev, Error **errp)
>> vdev->sub_device_id);
>> }
>>
>> + /*
>> + * Class code is a 24-bit value at config space 0x09. Allow overriding it
>> + * with any 24-bit value.
>> + */
>> + if (vdev->class_code != PCI_ANY_ID) {
>> + if (vdev->class_code > 0xffffff) {
>> + error_setg(errp, "invalid PCI class code provided");
>> + return false;
>> + }
>> + /* Higher 24 bits of PCI_CLASS_REVISION are class code */
>> + vfio_add_emulated_long(vdev, PCI_CLASS_REVISION,
>> + vdev->class_code << 8, ~0xff);
>> + trace_vfio_pci_emulated_class_code(vbasedev->name, vdev->class_code);
>> + }
>> +
>> /* QEMU can change multi-function devices to single function, or reverse */
>> vdev->emulated_config_bits[PCI_HEADER_TYPE] =
>> PCI_HEADER_TYPE_MULTI_FUNCTION;
>> @@ -3489,6 +3512,8 @@ static const Property vfio_pci_dev_properties[] = {
>> sub_vendor_id, PCI_ANY_ID),
>> DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
>> sub_device_id, PCI_ANY_ID),
>> + DEFINE_PROP_UINT32("x-pci-class-code", VFIOPCIDevice,
>> + class_code, PCI_ANY_ID),
>> DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
>> DEFINE_PROP_UNSIGNED_NODEFAULT("x-nv-gpudirect-clique", VFIOPCIDevice,
>> nv_gpudirect_clique,
>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>> index 5ce0fb916f..587eb8cc9a 100644
>> --- a/hw/vfio/pci.h
>> +++ b/hw/vfio/pci.h
>> @@ -156,6 +156,7 @@ struct VFIOPCIDevice {
>> uint32_t device_id;
>> uint32_t sub_vendor_id;
>> uint32_t sub_device_id;
>> + uint32_t class_code;
>> uint32_t features;
>> #define VFIO_FEATURE_ENABLE_VGA_BIT 0
>> #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT)
>> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
>> index e90ec9bff8..e8d585b49a 100644
>> --- a/hw/vfio/trace-events
>> +++ b/hw/vfio/trace-events
>> @@ -46,6 +46,7 @@ vfio_pci_emulated_vendor_id(const char *name, uint16_t val) "%s 0x%04x"
>> vfio_pci_emulated_device_id(const char *name, uint16_t val) "%s 0x%04x"
>> vfio_pci_emulated_sub_vendor_id(const char *name, uint16_t val) "%s 0x%04x"
>> vfio_pci_emulated_sub_device_id(const char *name, uint16_t val) "%s 0x%04x"
>> +vfio_pci_emulated_class_code(const char *name, uint32_t val) "%s 0x%06x"
>>
>> # pci-quirks.c
>> vfio_quirk_rom_in_denylist(const char *name, uint16_t vid, uint16_t did) "%s %04x:%04x"
>
On 2025/5/29 18:41, Tomita Moeko wrote:
> On 2025/5/29 2:30, Alex Williamson wrote:
>> On Wed, 28 May 2025 23:55:48 +0800
>> Tomita Moeko <tomitamoeko@gmail.com> wrote:
>>
>>> Introduce x-pci-class-code option to allow users to override PCI class
>>> code of a device, similar to the existing x-pci-vendor-id option. Only
>>> the lower 24 bits of this option are used, though a uint32 is used here
>>> for determining whether the value is valid and set by user.
>>>
>>> Additionally, to prevent exposing VGA ranges on non-VGA devices, the
>>> x-vga=on option requires x-pci-class-code is either unset or set to
>>> VGA controller class.
>>>
>>> This is mainly intended for IGD devices that expose themselves either
>>> as VGA controller (primary display) or Display controller (non-primary
>>> display). The UEFI GOP driver depends on the device reporting a VGA
>>> controller class code (0x030000).
>>>
>>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
>>> ---
>>> v2:
>>> * Add vdev class code check in vfio_populate_vga().
>>> * Fix type in trace-events.
>>> Link: https://lore.kernel.org/all/20250524153102.19747-1-tomitamoeko@gmail.com/
>>>
>>> hw/vfio/pci.c | 25 +++++++++++++++++++++++++
>>> hw/vfio/pci.h | 1 +
>>> hw/vfio/trace-events | 1 +
>>> 3 files changed, 27 insertions(+)
>>>
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index b1250d85bf..d57cb7356e 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -2726,6 +2726,14 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
>>> return false;
>>> }
>>>
>>> + /* vdev class should be either unmodified (PCI_ANY_ID), or VGA controller */
>>> + if ((vdev->class_code != PCI_ANY_ID) &&
>>> + (vdev->class_code != (PCI_CLASS_DISPLAY_VGA << 8)) &&
>>> + (vdev->class_code != (PCI_CLASS_NOT_DEFINED_VGA << 8))) {
>>> + error_setg(errp, "vdev is not a VGA device");
>>> + return false;
>>> + }
>>
>> I think we should follow the scheme used for vendor_id and device_id to
>> populate the struct field when not specified. That let's us use it
>> more easily and consistently for things like this.
>
> Hi, Alex
>
> The class code override takes place in vfio_pci_config_setup(), where
> is after vfio_populate_vga() is called in vfio_populate_device(). So
> I have to check if it equals to default or VGA class code here, and
> not initializing the sturct field with device value. If we decide to
> initialize it for other purpose, I personally think we should also
> save the subvendor/subdevice value as well.
It have been several weeks, wondering if there is further comments.
Thanks,
Moeko
>> I think we can ignore PCI_CLASS_NOT_DEFINED_VGA. The PCI Local Bus
>> Specification 2.1, dated June 1, 1995 (earliest I can find on the SIG)
>> includes the VGA class code and specifies base class 0x00 is reserved
>> for compatibility with devices built before the base class field was
>> defined, so at least before 1995. Also, neither the kernel or QEMU
>> is_vga helpers account for this, so they'd not have a VGA region or be
>> properly detected elsewhere. Thanks,
>>
>> Alex
>
> Got it, will remove this in v3.
>
> Thanks,
> Moeko
>
>>> +
>>> if (!(reg_info->flags & VFIO_REGION_INFO_FLAG_READ) ||
>>> !(reg_info->flags & VFIO_REGION_INFO_FLAG_WRITE) ||
>>> reg_info->size < 0xbffff + 1) {
>>> @@ -3092,6 +3100,21 @@ static bool vfio_pci_config_setup(VFIOPCIDevice *vdev, Error **errp)
>>> vdev->sub_device_id);
>>> }
>>>
>>> + /*
>>> + * Class code is a 24-bit value at config space 0x09. Allow overriding it
>>> + * with any 24-bit value.
>>> + */
>>> + if (vdev->class_code != PCI_ANY_ID) {
>>> + if (vdev->class_code > 0xffffff) {
>>> + error_setg(errp, "invalid PCI class code provided");
>>> + return false;
>>> + }
>>> + /* Higher 24 bits of PCI_CLASS_REVISION are class code */
>>> + vfio_add_emulated_long(vdev, PCI_CLASS_REVISION,
>>> + vdev->class_code << 8, ~0xff);
>>> + trace_vfio_pci_emulated_class_code(vbasedev->name, vdev->class_code);
>>> + }
>>> +
>>> /* QEMU can change multi-function devices to single function, or reverse */
>>> vdev->emulated_config_bits[PCI_HEADER_TYPE] =
>>> PCI_HEADER_TYPE_MULTI_FUNCTION;
>>> @@ -3489,6 +3512,8 @@ static const Property vfio_pci_dev_properties[] = {
>>> sub_vendor_id, PCI_ANY_ID),
>>> DEFINE_PROP_UINT32("x-pci-sub-device-id", VFIOPCIDevice,
>>> sub_device_id, PCI_ANY_ID),
>>> + DEFINE_PROP_UINT32("x-pci-class-code", VFIOPCIDevice,
>>> + class_code, PCI_ANY_ID),
>>> DEFINE_PROP_UINT32("x-igd-gms", VFIOPCIDevice, igd_gms, 0),
>>> DEFINE_PROP_UNSIGNED_NODEFAULT("x-nv-gpudirect-clique", VFIOPCIDevice,
>>> nv_gpudirect_clique,
>>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>>> index 5ce0fb916f..587eb8cc9a 100644
>>> --- a/hw/vfio/pci.h
>>> +++ b/hw/vfio/pci.h
>>> @@ -156,6 +156,7 @@ struct VFIOPCIDevice {
>>> uint32_t device_id;
>>> uint32_t sub_vendor_id;
>>> uint32_t sub_device_id;
>>> + uint32_t class_code;
>>> uint32_t features;
>>> #define VFIO_FEATURE_ENABLE_VGA_BIT 0
>>> #define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT)
>>> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
>>> index e90ec9bff8..e8d585b49a 100644
>>> --- a/hw/vfio/trace-events
>>> +++ b/hw/vfio/trace-events
>>> @@ -46,6 +46,7 @@ vfio_pci_emulated_vendor_id(const char *name, uint16_t val) "%s 0x%04x"
>>> vfio_pci_emulated_device_id(const char *name, uint16_t val) "%s 0x%04x"
>>> vfio_pci_emulated_sub_vendor_id(const char *name, uint16_t val) "%s 0x%04x"
>>> vfio_pci_emulated_sub_device_id(const char *name, uint16_t val) "%s 0x%04x"
>>> +vfio_pci_emulated_class_code(const char *name, uint32_t val) "%s 0x%06x"
>>>
>>> # pci-quirks.c
>>> vfio_quirk_rom_in_denylist(const char *name, uint16_t vid, uint16_t did) "%s %04x:%04x"
>>
>
On Tue, 17 Jun 2025 08:56:41 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:
> On 2025/5/29 18:41, Tomita Moeko wrote:
> > On 2025/5/29 2:30, Alex Williamson wrote:
> >> On Wed, 28 May 2025 23:55:48 +0800
> >> Tomita Moeko <tomitamoeko@gmail.com> wrote:
> >>
> >>> Introduce x-pci-class-code option to allow users to override PCI class
> >>> code of a device, similar to the existing x-pci-vendor-id option. Only
> >>> the lower 24 bits of this option are used, though a uint32 is used here
> >>> for determining whether the value is valid and set by user.
> >>>
> >>> Additionally, to prevent exposing VGA ranges on non-VGA devices, the
> >>> x-vga=on option requires x-pci-class-code is either unset or set to
> >>> VGA controller class.
> >>>
> >>> This is mainly intended for IGD devices that expose themselves either
> >>> as VGA controller (primary display) or Display controller (non-primary
> >>> display). The UEFI GOP driver depends on the device reporting a VGA
> >>> controller class code (0x030000).
> >>>
> >>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> >>> ---
> >>> v2:
> >>> * Add vdev class code check in vfio_populate_vga().
> >>> * Fix type in trace-events.
> >>> Link: https://lore.kernel.org/all/20250524153102.19747-1-tomitamoeko@gmail.com/
> >>>
> >>> hw/vfio/pci.c | 25 +++++++++++++++++++++++++
> >>> hw/vfio/pci.h | 1 +
> >>> hw/vfio/trace-events | 1 +
> >>> 3 files changed, 27 insertions(+)
> >>>
> >>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> >>> index b1250d85bf..d57cb7356e 100644
> >>> --- a/hw/vfio/pci.c
> >>> +++ b/hw/vfio/pci.c
> >>> @@ -2726,6 +2726,14 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
> >>> return false;
> >>> }
> >>>
> >>> + /* vdev class should be either unmodified (PCI_ANY_ID), or VGA controller */
> >>> + if ((vdev->class_code != PCI_ANY_ID) &&
> >>> + (vdev->class_code != (PCI_CLASS_DISPLAY_VGA << 8)) &&
> >>> + (vdev->class_code != (PCI_CLASS_NOT_DEFINED_VGA << 8))) {
> >>> + error_setg(errp, "vdev is not a VGA device");
> >>> + return false;
> >>> + }
> >>
> >> I think we should follow the scheme used for vendor_id and device_id to
> >> populate the struct field when not specified. That let's us use it
> >> more easily and consistently for things like this.
> >
> > Hi, Alex
> >
> > The class code override takes place in vfio_pci_config_setup(), where
> > is after vfio_populate_vga() is called in vfio_populate_device(). So
> > I have to check if it equals to default or VGA class code here, and
> > not initializing the sturct field with device value. If we decide to
> > initialize it for other purpose, I personally think we should also
> > save the subvendor/subdevice value as well.
>
> It have been several weeks, wondering if there is further comments.
Hi Moeko,
Thank you for your patience. I'm still a little confused how this
works. AIUI you have an GPU reported as a display controller, but you
want to change the class code to VGA, despite there being no VGA region
provided by vfio, right? So the GOP driver only depends on the legacy
VGA class code and not the VGA regions.
vfio_populate_vga() initializes the vfio VGA region and calls
pci_register_vga(). Therefore with the ability to modify the class
code we only want to register the device as VGA if the class code is
unmodified, where the VGA vfio regions will only exist if the device is
VGA, or if the class override is VGA, where the VGA vfio regions again
only exist if the device is actually VGA and therefore the override is
a no-op. But we only get to vfio_populate_vga() if the user specifies
x-vga=on. So I think we're really just trying to prevent a physical VGA
device overridden as a non-VGA class code, but still specified with
x-vga=on, from calling pci_register_vga(), right?
What if we just separate pci_register_vga() out of vfio_populate_vga()?
Then we can continue the semantics of making class_code valid
regardless of whether it's been specified and around the point where we
call vfio_bars_register() we can also call pci_register_vga() if and
only if the class code is VGA and vdev->vga is configured. Does that
make sense? Thanks,
Alex
On 6/27/25 02:26, Alex Williamson wrote:
> On Tue, 17 Jun 2025 08:56:41 +0800
> Tomita Moeko <tomitamoeko@gmail.com> wrote:
>
>> On 2025/5/29 18:41, Tomita Moeko wrote:
>>> On 2025/5/29 2:30, Alex Williamson wrote:
>>>> On Wed, 28 May 2025 23:55:48 +0800
>>>> Tomita Moeko <tomitamoeko@gmail.com> wrote:
>>>>
>>>>> Introduce x-pci-class-code option to allow users to override PCI class
>>>>> code of a device, similar to the existing x-pci-vendor-id option. Only
>>>>> the lower 24 bits of this option are used, though a uint32 is used here
>>>>> for determining whether the value is valid and set by user.
>>>>>
>>>>> Additionally, to prevent exposing VGA ranges on non-VGA devices, the
>>>>> x-vga=on option requires x-pci-class-code is either unset or set to
>>>>> VGA controller class.
>>>>>
>>>>> This is mainly intended for IGD devices that expose themselves either
>>>>> as VGA controller (primary display) or Display controller (non-primary
>>>>> display). The UEFI GOP driver depends on the device reporting a VGA
>>>>> controller class code (0x030000).
>>>>>
>>>>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
>>>>> ---
>>>>> v2:
>>>>> * Add vdev class code check in vfio_populate_vga().
>>>>> * Fix type in trace-events.
>>>>> Link: https://lore.kernel.org/all/20250524153102.19747-1-tomitamoeko@gmail.com/
>>>>>
>>>>> hw/vfio/pci.c | 25 +++++++++++++++++++++++++
>>>>> hw/vfio/pci.h | 1 +
>>>>> hw/vfio/trace-events | 1 +
>>>>> 3 files changed, 27 insertions(+)
>>>>>
>>>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>>>> index b1250d85bf..d57cb7356e 100644
>>>>> --- a/hw/vfio/pci.c
>>>>> +++ b/hw/vfio/pci.c
>>>>> @@ -2726,6 +2726,14 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
>>>>> return false;
>>>>> }
>>>>>
>>>>> + /* vdev class should be either unmodified (PCI_ANY_ID), or VGA controller */
>>>>> + if ((vdev->class_code != PCI_ANY_ID) &&
>>>>> + (vdev->class_code != (PCI_CLASS_DISPLAY_VGA << 8)) &&
>>>>> + (vdev->class_code != (PCI_CLASS_NOT_DEFINED_VGA << 8))) {
>>>>> + error_setg(errp, "vdev is not a VGA device");
>>>>> + return false;
>>>>> + }
>>>>
>>>> I think we should follow the scheme used for vendor_id and device_id to
>>>> populate the struct field when not specified. That let's us use it
>>>> more easily and consistently for things like this.
>>>
>>> Hi, Alex
>>>
>>> The class code override takes place in vfio_pci_config_setup(), where
>>> is after vfio_populate_vga() is called in vfio_populate_device(). So
>>> I have to check if it equals to default or VGA class code here, and
>>> not initializing the sturct field with device value. If we decide to
>>> initialize it for other purpose, I personally think we should also
>>> save the subvendor/subdevice value as well.
>>
>> It have been several weeks, wondering if there is further comments.
>
> Hi Moeko,
>
> Thank you for your patience. I'm still a little confused how this
> works. AIUI you have an GPU reported as a display controller, but you
> want to change the class code to VGA, despite there being no VGA region
> provided by vfio, right? So the GOP driver only depends on the legacy
> VGA class code and not the VGA regions.
Hi, Alex
Sorry for my late reply, I was been on a travel on past 2 weeks.
Yes, the GOP driver only depends on VGA class code. Though I doesn't
have access to its source code, I tried with not exposing VGA regions,
the GOP driver works as expected. It seems Intel uses class code to tell
if IGD is set to primary (VGA controller) or secondary (display cntl)
and only enable GOP output when it is primary.
> vfio_populate_vga() initializes the vfio VGA region and calls
> pci_register_vga(). Therefore with the ability to modify the class
> code we only want to register the device as VGA if the class code is
> unmodified, where the VGA vfio regions will only exist if the device is
> VGA, or if the class override is VGA, where the VGA vfio regions again
> only exist if the device is actually VGA and therefore the override is
> a no-op. But we only get to vfio_populate_vga() if the user specifies
> x-vga=on. So I think we're really just trying to prevent a physical VGA
> device overridden as a non-VGA class code, but still specified with
> x-vga=on, from calling pci_register_vga(), right?
>
> What if we just separate pci_register_vga() out of vfio_populate_vga()?
> Then we can continue the semantics of making class_code valid
> regardless of whether it's been specified and around the point where we
> call vfio_bars_register() we can also call pci_register_vga() if and
> only if the class code is VGA and vdev->vga is configured. Does that
> make sense? Thanks,
>
> Alex
Thanks for pointing that out. I hadn't considered the case that x-vga=on
when class code is overridden to non-VGA should be forbidden. Moving the
pci_register_vga() call into vfio_bars_register() after overriding the
class code absolutely makes sense. I'll proceed with this option in v3.
Thanks,
Moeko
© 2016 - 2025 Red Hat, Inc.