diff mbox series

pci-ids.rst: Add Red Hat pci-id for AMD IOMMU device

Message ID 20250304183747.639382-1-suravee.suthikulpanit@amd.com (mailing list archive)
State New, archived
Headers show
Series pci-ids.rst: Add Red Hat pci-id for AMD IOMMU device | expand

Commit Message

Suthikulpanit, Suravee March 4, 2025, 6:37 p.m. UTC
The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD I/O
Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
device is platform-specific.

Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
undefined device id.

Therefore, change the vendor id to Red Hat and request a new QEMU-specific
device id.

[1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 docs/specs/pci-ids.rst | 2 ++
 hw/i386/amd_iommu.c    | 3 ++-
 include/hw/pci/pci.h   | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé March 4, 2025, 6:46 p.m. UTC | #1
On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD I/O
> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> device is platform-specific.
> 
> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> undefined device id.
> 
> Therefore, change the vendor id to Red Hat and request a new QEMU-specific
> device id.
> 
> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  docs/specs/pci-ids.rst | 2 ++
>  hw/i386/amd_iommu.c    | 3 ++-
>  include/hw/pci/pci.h   | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/pci-ids.rst b/docs/specs/pci-ids.rst
> index 261b0f359f..2416a70a2d 100644
> --- a/docs/specs/pci-ids.rst
> +++ b/docs/specs/pci-ids.rst
> @@ -100,6 +100,8 @@ PCI devices (other than virtio):
>    PCI UFS device (``-device ufs``)
>  1b36:0014
>    PCI RISC-V IOMMU device
> +1b36:0015
> +  PCI AMD IOMMU device (``-device amd-iommu``)
>  
>  All these devices are documented in :doc:`index`.
>  
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index dda1a5781f..4d8564249c 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1766,7 +1766,8 @@ static void amdvi_pci_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -    k->vendor_id = PCI_VENDOR_ID_AMD;
> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> +    k->device_id = PCI_DEVICE_ID_REDHAT_AMD_IOMMU;

Ordinarily this change would need to have versioned machine type
back compat logic.

The AMD IOMMU has a migration blocker currently, so at least we
are guaranteed to not hit a problem for running guests.

There's still risk that an existing installed guest is upset by a
change of PCI vendor/device ID across cold-boots.

We can add in the fact that we couldn't guarantee a stable PCI
device slot either until the split to allow AMDVI-PCI to be
independantly created.

With all this in mind, I think it is reasonable to say the current
device is incapable of providing a stable ABI in general, and thus
it is justified to omit the versioned machine type back compat
logic this time.

This patch should probably be bundled with the other two outstanding
amd iommu patches for migration support.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

>      k->class_id = 0x0806;
>      k->realize = amdvi_pci_realize;
>  
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4002bbeebd..da44e6673d 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -117,6 +117,7 @@ extern bool pci_available;
>  #define PCI_DEVICE_ID_REDHAT_ACPI_ERST   0x0012
>  #define PCI_DEVICE_ID_REDHAT_UFS         0x0013
>  #define PCI_DEVICE_ID_REDHAT_RISCV_IOMMU 0x0014
> +#define PCI_DEVICE_ID_REDHAT_AMD_IOMMU   0x0015
>  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
>  
>  #define FMT_PCIBUS                      PRIx64
> -- 
> 2.34.1
> 

With regards,
Daniel
Donald Dutile March 4, 2025, 11:02 p.m. UTC | #2
Hi Suravee!

Not your issue, but wondering if others know:

Why isn't this an issue for Intel-vtd-iommu & ARM-SMMUV3 ?
Are they instantiated as non-PCI-id (platform) devices, but AMD puts their IOMMU in PCI space?

Adv. thanks for the info.

- Don


On 3/4/25 1:37 PM, Suravee Suthikulpanit wrote:
> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD I/O
> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> device is platform-specific.
> 
> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> undefined device id.
> 
> Therefore, change the vendor id to Red Hat and request a new QEMU-specific
> device id.
> 
> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>   docs/specs/pci-ids.rst | 2 ++
>   hw/i386/amd_iommu.c    | 3 ++-
>   include/hw/pci/pci.h   | 1 +
>   3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/pci-ids.rst b/docs/specs/pci-ids.rst
> index 261b0f359f..2416a70a2d 100644
> --- a/docs/specs/pci-ids.rst
> +++ b/docs/specs/pci-ids.rst
> @@ -100,6 +100,8 @@ PCI devices (other than virtio):
>     PCI UFS device (``-device ufs``)
>   1b36:0014
>     PCI RISC-V IOMMU device
> +1b36:0015
> +  PCI AMD IOMMU device (``-device amd-iommu``)
>   
>   All these devices are documented in :doc:`index`.
>   
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index dda1a5781f..4d8564249c 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1766,7 +1766,8 @@ static void amdvi_pci_class_init(ObjectClass *klass, void *data)
>       DeviceClass *dc = DEVICE_CLASS(klass);
>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>   
> -    k->vendor_id = PCI_VENDOR_ID_AMD;
> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> +    k->device_id = PCI_DEVICE_ID_REDHAT_AMD_IOMMU;
>       k->class_id = 0x0806;
>       k->realize = amdvi_pci_realize;
>   
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4002bbeebd..da44e6673d 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -117,6 +117,7 @@ extern bool pci_available;
>   #define PCI_DEVICE_ID_REDHAT_ACPI_ERST   0x0012
>   #define PCI_DEVICE_ID_REDHAT_UFS         0x0013
>   #define PCI_DEVICE_ID_REDHAT_RISCV_IOMMU 0x0014
> +#define PCI_DEVICE_ID_REDHAT_AMD_IOMMU   0x0015
>   #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
>   
>   #define FMT_PCIBUS                      PRIx64
Suthikulpanit, Suravee March 5, 2025, 2:56 a.m. UTC | #3
On 3/5/2025 6:02 AM, Donald Dutile wrote:
> Hi Suravee!
> 
> Not your issue, but wondering if others know:
> 
> Why isn't this an issue for Intel-vtd-iommu & ARM-SMMUV3 ?
> Are they instantiated as non-PCI-id (platform) devices, but AMD puts 
> their IOMMU in PCI space?
> 
> Adv. thanks for the info.

Unlike AMD IOMMU, Intel VT-d IOMMU and ARM SMMUV3 are not enumerated as 
a PCI device in the system.

Thanks,
Suravee
Michael S. Tsirkin March 5, 2025, 6:52 a.m. UTC | #4
On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD I/O
> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> device is platform-specific.
> 
> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> undefined device id.

undefined?

> Therefore, change the vendor id to Red Hat and request a new QEMU-specific
> device id.

Won't the drivers fail to load then?


> 
> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf

what is this link teaching us? It's a 300 page document. Where to look
in there?

> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  docs/specs/pci-ids.rst | 2 ++
>  hw/i386/amd_iommu.c    | 3 ++-
>  include/hw/pci/pci.h   | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/pci-ids.rst b/docs/specs/pci-ids.rst
> index 261b0f359f..2416a70a2d 100644
> --- a/docs/specs/pci-ids.rst
> +++ b/docs/specs/pci-ids.rst
> @@ -100,6 +100,8 @@ PCI devices (other than virtio):
>    PCI UFS device (``-device ufs``)
>  1b36:0014
>    PCI RISC-V IOMMU device
> +1b36:0015
> +  PCI AMD IOMMU device (``-device amd-iommu``)
>  
>  All these devices are documented in :doc:`index`.
>  
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index dda1a5781f..4d8564249c 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1766,7 +1766,8 @@ static void amdvi_pci_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -    k->vendor_id = PCI_VENDOR_ID_AMD;
> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> +    k->device_id = PCI_DEVICE_ID_REDHAT_AMD_IOMMU;
>      k->class_id = 0x0806;
>      k->realize = amdvi_pci_realize;
>  
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4002bbeebd..da44e6673d 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -117,6 +117,7 @@ extern bool pci_available;
>  #define PCI_DEVICE_ID_REDHAT_ACPI_ERST   0x0012
>  #define PCI_DEVICE_ID_REDHAT_UFS         0x0013
>  #define PCI_DEVICE_ID_REDHAT_RISCV_IOMMU 0x0014
> +#define PCI_DEVICE_ID_REDHAT_AMD_IOMMU   0x0015
>  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
>  
>  #define FMT_PCIBUS                      PRIx64
> -- 
> 2.34.1
Donald Dutile March 5, 2025, 8:28 p.m. UTC | #5
On 3/4/25 9:56 PM, Suthikulpanit, Suravee wrote:
> 
> 
> On 3/5/2025 6:02 AM, Donald Dutile wrote:
>> Hi Suravee!
>>
>> Not your issue, but wondering if others know:
>>
>> Why isn't this an issue for Intel-vtd-iommu & ARM-SMMUV3 ?
>> Are they instantiated as non-PCI-id (platform) devices, but AMD puts their IOMMU in PCI space?
>>
>> Adv. thanks for the info.
> 
> Unlike AMD IOMMU, Intel VT-d IOMMU and ARM SMMUV3 are not enumerated as a PCI device in the system.
> 
> Thanks,
> Suravee
> 
ok. thanks for confirmation.
--dd
Yan Vugenfirer March 6, 2025, 7:11 a.m. UTC | #6
On Wed, Mar 5, 2025 at 8:54 AM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
> > The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD
> I/O
> > Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> > device is platform-specific.
> >
> > Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> > undefined device id.
>
> undefined?
>
> > Therefore, change the vendor id to Red Hat and request a new
> QEMU-specific
> > device id.
>
> Won't the drivers fail to load then?
>

Windows will not identify the device (it is a dummy device, without driver)
and SVVP certifications will fail as a result.
I suggest using ID that is already present in Windows machine.inf:
VEN_1002&DEV_5A23
VEN_1022&DEV_1419


>
> >
> > [1]
> https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
>
> what is this link teaching us? It's a 300 page document. Where to look
> in there?
>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > ---
> >  docs/specs/pci-ids.rst | 2 ++
> >  hw/i386/amd_iommu.c    | 3 ++-
> >  include/hw/pci/pci.h   | 1 +
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/specs/pci-ids.rst b/docs/specs/pci-ids.rst
> > index 261b0f359f..2416a70a2d 100644
> > --- a/docs/specs/pci-ids.rst
> > +++ b/docs/specs/pci-ids.rst
> > @@ -100,6 +100,8 @@ PCI devices (other than virtio):
> >    PCI UFS device (``-device ufs``)
> >  1b36:0014
> >    PCI RISC-V IOMMU device
> > +1b36:0015
> > +  PCI AMD IOMMU device (``-device amd-iommu``)
> >
> >  All these devices are documented in :doc:`index`.
> >
> > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> > index dda1a5781f..4d8564249c 100644
> > --- a/hw/i386/amd_iommu.c
> > +++ b/hw/i386/amd_iommu.c
> > @@ -1766,7 +1766,8 @@ static void amdvi_pci_class_init(ObjectClass
> *klass, void *data)
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> >
> > -    k->vendor_id = PCI_VENDOR_ID_AMD;
> > +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> > +    k->device_id = PCI_DEVICE_ID_REDHAT_AMD_IOMMU;
> >      k->class_id = 0x0806;
> >      k->realize = amdvi_pci_realize;
> >
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 4002bbeebd..da44e6673d 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -117,6 +117,7 @@ extern bool pci_available;
> >  #define PCI_DEVICE_ID_REDHAT_ACPI_ERST   0x0012
> >  #define PCI_DEVICE_ID_REDHAT_UFS         0x0013
> >  #define PCI_DEVICE_ID_REDHAT_RISCV_IOMMU 0x0014
> > +#define PCI_DEVICE_ID_REDHAT_AMD_IOMMU   0x0015
> >  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
> >
> >  #define FMT_PCIBUS                      PRIx64
> > --
> > 2.34.1
>
>
>
Daniel P. Berrangé March 6, 2025, 8:58 a.m. UTC | #7
On Thu, Mar 06, 2025 at 09:11:53AM +0200, Yan Vugenfirer wrote:
> On Wed, Mar 5, 2025 at 8:54 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> > On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
> > > The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD
> > I/O
> > > Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> > > device is platform-specific.
> > >
> > > Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> > > undefined device id.
> >
> > undefined?
> >
> > > Therefore, change the vendor id to Red Hat and request a new
> > QEMU-specific
> > > device id.
> >
> > Won't the drivers fail to load then?
> >
> 
> Windows will not identify the device (it is a dummy device, without driver)
> and SVVP certifications will fail as a result.
> I suggest using ID that is already present in Windows machine.inf:
> VEN_1002&DEV_5A23

 Ven:  Advanced Micro Devices, Inc. [AMD/ATI]
 Dev: RD890S/RD990 I/O Memory Management Unit (IOMMU) 

> VEN_1022&DEV_1419

 Vendor:  Advanced Micro Devices, Inc. [AMD]
 Dev: Family 15h (Models 10h-1fh) I/O Memory Management Unit

Is our implementation semantically a match for the functionality
in either of those real hardware devices ?

We shouldn't use an existing hardware dev ID unless we intend to
emulate its functionality as a precise match.


With regards,
Daniel
Suthikulpanit, Suravee March 7, 2025, 2:44 a.m. UTC | #8
On 3/6/2025 3:58 PM, Daniel P. Berrangé wrote:
> On Thu, Mar 06, 2025 at 09:11:53AM +0200, Yan Vugenfirer wrote:
>> On Wed, Mar 5, 2025 at 8:54 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>>> On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
>>>> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD
>>> I/O
>>>> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
>>>> device is platform-specific.
>>>>
>>>> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
>>>> undefined device id.
>>>
>>> undefined?

Currently, QEMU is using vendor ID "1022" and device ID "0".

>>>
>>>> Therefore, change the vendor id to Red Hat and request a new
>>> QEMU-specific
>>>> device id.
>>>
>>> Won't the drivers fail to load then?
>>>
>>
>> Windows will not identify the device (it is a dummy device, without driver)
>> and SVVP certifications will fail as a result.
>> I suggest using ID that is already present in Windows machine.inf:
>> VEN_1002&DEV_5A23
> 
>   Ven:  Advanced Micro Devices, Inc. [AMD/ATI]
>   Dev: RD890S/RD990 I/O Memory Management Unit (IOMMU)
> 
>> VEN_1022&DEV_1419
> 
>   Vendor:  Advanced Micro Devices, Inc. [AMD]
>   Dev: Family 15h (Models 10h-1fh) I/O Memory Management Unit
> 
> Is our implementation semantically a match for the functionality
> in either of those real hardware devices ?
> 
> We shouldn't use an existing hardware dev ID unless we intend to
> emulate its functionality as a precise match.

I agree. We should not use the "fake" ID unless we know for sure that 
the QEMU device is a match from the list of supported features and 
behavior (which might be implementation-specific on different 
platforms). Currently, this is not an exact match.

Linux does not care much about the PCI vendor/device ids since it uses 
the information in the IVRS table to probe for the device. Features are 
determined using the information in:
   * MMIO Offset 0030h IOMMU Extended Feature Register
   * MMIO Offset 01A0h IOMMU Extended Feature 2 Register

However, we can't guarantee how other OSes might be using these 
information for probing / loading drivers.

Thanks,
Suravee
Suthikulpanit, Suravee March 7, 2025, 2:53 a.m. UTC | #9
On 3/5/2025 1:52 PM, Michael S. Tsirkin wrote:
> On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
>> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD I/O
>> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
>> device is platform-specific.
>>
>> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
>> undefined device id.
> undefined?
> 
>> Therefore, change the vendor id to Red Hat and request a new QEMU-specific
>> device id.
> Won't the drivers fail to load then?
> 
> 
>> [1]https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/ 
>> specifications/48882_IOMMU.pdf
> what is this link teaching us? It's a 300 page document. Where to look
> in there?

What I am trying to say is that all AMD IOMMU implementations must 
adhere to this specification including the QEMU-emulated AMD IOMMU. 
Unlike cpu emulation, we don't need to exactly mimic a particular 
hardware implementation.

QEMU-emulated AMD IOMMU device can have its own set of features that it 
supports. Going forward, we might even have QEMU-specific feature / 
behavior. So, we should not be using an existing PCI ID.

Thanks,
Suravee
Yan Vugenfirer March 9, 2025, 6:47 a.m. UTC | #10
On Fri, Mar 7, 2025 at 4:55 AM Suthikulpanit, Suravee <
suravee.suthikulpanit@amd.com> wrote:

>
>
> On 3/5/2025 1:52 PM, Michael S. Tsirkin wrote:
> > On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
> >> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD
> I/O
> >> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> >> device is platform-specific.
> >>
> >> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> >> undefined device id.
> > undefined?
> >
> >> Therefore, change the vendor id to Red Hat and request a new
> QEMU-specific
> >> device id.
> > Won't the drivers fail to load then?
> >
> >
> >> [1]
> https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/
> >> specifications/48882_IOMMU.pdf
> > what is this link teaching us? It's a 300 page document. Where to look
> > in there?
>
> What I am trying to say is that all AMD IOMMU implementations must
> adhere to this specification including the QEMU-emulated AMD IOMMU.
> Unlike cpu emulation, we don't need to exactly mimic a particular
> hardware implementation.
>
> QEMU-emulated AMD IOMMU device can have its own set of features that it
> supports. Going forward, we might even have QEMU-specific feature /
> behavior. So, we should not be using an existing PCI ID.
>
> And in this case how "other" (namely Windows) OSes will know how to use
the device?


> Thanks,
> Suravee
>
>
Michael S. Tsirkin March 9, 2025, 1:44 p.m. UTC | #11
On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD I/O
> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> device is platform-specific.
> 
> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> undefined device id.
> 
> Therefore, change the vendor id to Red Hat and request a new QEMU-specific
> device id.
> 
> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Will the existing drivers bind with the device then?


> ---
>  docs/specs/pci-ids.rst | 2 ++
>  hw/i386/amd_iommu.c    | 3 ++-
>  include/hw/pci/pci.h   | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/pci-ids.rst b/docs/specs/pci-ids.rst
> index 261b0f359f..2416a70a2d 100644
> --- a/docs/specs/pci-ids.rst
> +++ b/docs/specs/pci-ids.rst
> @@ -100,6 +100,8 @@ PCI devices (other than virtio):
>    PCI UFS device (``-device ufs``)
>  1b36:0014
>    PCI RISC-V IOMMU device
> +1b36:0015
> +  PCI AMD IOMMU device (``-device amd-iommu``)
>  
>  All these devices are documented in :doc:`index`.
>  
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index dda1a5781f..4d8564249c 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1766,7 +1766,8 @@ static void amdvi_pci_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -    k->vendor_id = PCI_VENDOR_ID_AMD;
> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> +    k->device_id = PCI_DEVICE_ID_REDHAT_AMD_IOMMU;
>      k->class_id = 0x0806;
>      k->realize = amdvi_pci_realize;
>  
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4002bbeebd..da44e6673d 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -117,6 +117,7 @@ extern bool pci_available;
>  #define PCI_DEVICE_ID_REDHAT_ACPI_ERST   0x0012
>  #define PCI_DEVICE_ID_REDHAT_UFS         0x0013
>  #define PCI_DEVICE_ID_REDHAT_RISCV_IOMMU 0x0014
> +#define PCI_DEVICE_ID_REDHAT_AMD_IOMMU   0x0015
>  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
>  
>  #define FMT_PCIBUS                      PRIx64
> -- 
> 2.34.1
Jonathan Cameron March 10, 2025, 1:57 p.m. UTC | #12
On Tue, 4 Mar 2025 18:37:47 +0000
Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> wrote:

> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD I/O
> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> device is platform-specific.
> 
> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> undefined device id.
> 
> Therefore, change the vendor id to Red Hat and request a new QEMU-specific
> device id.
> 
> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

As a heads up, I believe we have a similar problem with a few of the CXL IDs.
The root port and type 3 device both use Intel IDs that were not reserved
for this purpose.  VID=0x8086, DID=0x7075 and DID=0x0d93

Switch ports and switch-cci are using valid Hisilicon IDs that are for
emulation of these device only and are registered in our tracker
for these IDs so won't get 'reused'.

In both those cases the driver binds on class code in Linux so an ID
change to resolve this would be fine for Linux - I can't speak for
other OS.

Jonathan




> ---
>  docs/specs/pci-ids.rst | 2 ++
>  hw/i386/amd_iommu.c    | 3 ++-
>  include/hw/pci/pci.h   | 1 +
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/pci-ids.rst b/docs/specs/pci-ids.rst
> index 261b0f359f..2416a70a2d 100644
> --- a/docs/specs/pci-ids.rst
> +++ b/docs/specs/pci-ids.rst
> @@ -100,6 +100,8 @@ PCI devices (other than virtio):
>    PCI UFS device (``-device ufs``)
>  1b36:0014
>    PCI RISC-V IOMMU device
> +1b36:0015
> +  PCI AMD IOMMU device (``-device amd-iommu``)
>  
>  All these devices are documented in :doc:`index`.
>  
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index dda1a5781f..4d8564249c 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1766,7 +1766,8 @@ static void amdvi_pci_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -    k->vendor_id = PCI_VENDOR_ID_AMD;
> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> +    k->device_id = PCI_DEVICE_ID_REDHAT_AMD_IOMMU;
>      k->class_id = 0x0806;
>      k->realize = amdvi_pci_realize;
>  
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4002bbeebd..da44e6673d 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -117,6 +117,7 @@ extern bool pci_available;
>  #define PCI_DEVICE_ID_REDHAT_ACPI_ERST   0x0012
>  #define PCI_DEVICE_ID_REDHAT_UFS         0x0013
>  #define PCI_DEVICE_ID_REDHAT_RISCV_IOMMU 0x0014
> +#define PCI_DEVICE_ID_REDHAT_AMD_IOMMU   0x0015
>  #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
>  
>  #define FMT_PCIBUS                      PRIx64
Suthikulpanit, Suravee March 11, 2025, 1:55 a.m. UTC | #13
On 3/9/2025 8:44 PM, Michael S. Tsirkin wrote:
> On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
>> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD I/O
>> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
>> device is platform-specific.
>>
>> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
>> undefined device id.
>>
>> Therefore, change the vendor id to Red Hat and request a new QEMU-specific
>> device id.
>>
>> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> Will the existing drivers bind with the device then?

Existing Windows would not recognize the device ID.

Actually, Linux and Windows does not depend on the PCI vendor / device 
ids to probe devices and initialize AMD IOMMU. Instead, it depends on 
the ACPI IVRS table.

Checking on a real system w/ AMD IOMMU enabled booting Windows Server 
2022, there is no AMD IOMMU device showing in the Device Manger.

In this case, I believe Windows is not fully initializing the 
QEMU-emulated AMD IOMMU. So Windows would not remove the IOMMU PCIe from 
the list of OS visible devices and therefore expose the PNPID to the 
device manager. And since the device ID is zero, it appears as an "Other 
devices->PCI Device (with warning sign).

Therefore, it we have two options:

1. Fake the device ID to 0x1419, which is current appear in the 
machine.inf as an entry in section [AMD_SYS.NTamd64]:

%IommuDevice_Desc% = NO_DRV,PCI\VEN_1022&DEV_1419

2. Figure out why Windows does not recognize the device.

Anyhow, we should still assign some PCI ID value (instead of zero).

Thanks,
Suravee
Gerd Hoffmann March 11, 2025, 8:08 a.m. UTC | #14
Hi,

> As a heads up, I believe we have a similar problem with a few of the CXL IDs.
> The root port and type 3 device both use Intel IDs that were not reserved
> for this purpose.  VID=0x8086, DID=0x7075 and DID=0x0d93

Essentially we have two kinds of PCI devices in qemu.

 * The ones which try to mimic existing hardware, they usually have the
   PCI ID of the device they are emulating (and use the qemu subsystem ID).
   The classic example is the cirrus vga.  There are also many intel
   chipset devices for piix4 ('pc' machine type) and ich9 ('q35' machine
   type) with intel IDs.

 * The ones which are PCI class implementations and do not need a
   specific ID for drivers to accept them.  Most of them have a
   PCI device ID from the 1b36 vendor ID range.

The former tend to be older devices (before hardware standardization was
a thing, also before we got a range from 1b36 for qemu), and the latter
tend to be newer devices.  There are also a bunch of exceptions for
historical reasons.  The ahci emulation has a ich9 id.  xhci even has
two variants (one mimicking a NEC host adapter, one with qemu device id).

So, in short, using the intel IDs is not necessarily a problem.  Depends
a bit on what kind of device we are talking about.  For PCI class
devices it usually is more useful to have a qemu ID though.

> Switch ports and switch-cci are using valid Hisilicon IDs that are for
> emulation of these device only and are registered in our tracker
> for these IDs so won't get 'reused'.

That is perfectly fine.  There is no need to change IDs, although it
makes sense to document that fact in docs/specs/pci-ids.rst

Moving them to qemu pci id range is an option too if you prefer that.
Your choice.

take care,
  Gerd
Yan Vugenfirer March 12, 2025, 12:43 p.m. UTC | #15
On Tue, Mar 11, 2025 at 4:02 AM Suthikulpanit, Suravee <
suravee.suthikulpanit@amd.com> wrote:

>
>
> On 3/9/2025 8:44 PM, Michael S. Tsirkin wrote:
> > On Tue, Mar 04, 2025 at 06:37:47PM +0000, Suravee Suthikulpanit wrote:
> >> The QEMU-emulated AMD IOMMU PCI device is implemented based on the AMD
> I/O
> >> Virtualization Technology (IOMMU) Specification [1]. The PCI id for this
> >> device is platform-specific.
> >>
> >> Currently, the QEMU-emulated AMD IOMMU device is using AMD vendor id and
> >> undefined device id.
> >>
> >> Therefore, change the vendor id to Red Hat and request a new
> QEMU-specific
> >> device id.
> >>
> >> [1]
> https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
> >>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >
> > Will the existing drivers bind with the device then?
>
> Existing Windows would not recognize the device ID.
>
> Actually, Linux and Windows does not depend on the PCI vendor / device
> ids to probe devices and initialize AMD IOMMU. Instead, it depends on
> the ACPI IVRS table.
>
> Checking on a real system w/ AMD IOMMU enabled booting Windows Server
> 2022, there is no AMD IOMMU device showing in the Device Manger.
>
> In this case, I believe Windows is not fully initializing the
> QEMU-emulated AMD IOMMU. So Windows would not remove the IOMMU PCIe from
> the list of OS visible devices and therefore expose the PNPID to the
> device manager. And since the device ID is zero, it appears as an "Other
> devices->PCI Device (with warning sign).
>
> Therefore, it we have two options:
>
> 1. Fake the device ID to 0x1419, which is current appear in the
> machine.inf as an entry in section [AMD_SYS.NTamd64]:
>
> %IommuDevice_Desc% = NO_DRV,PCI\VEN_1022&DEV_1419
>
Considering that this is a "null driver" (no actual driver is loaded for
the PCIe endpoint according to machine.inf), I recommend using this PNP ID.


>
> 2. Figure out why Windows does not recognize the device.
>
The answer is simple: the PCIe endpoint's PNP ID is unknown to Windows. So
technically device is recognized (it is shown in Device Manager after all),
but there are no compatible drivers for it. And in anycase, machine.inf
specifies "null driver" for AMD PCIe endpoint IOMMU device. The device will
get a friendly name in Device Manager and considered to be "installed" by
Windows, by there is no actual driver associated with the device.

Best regards,
Yan.


>
> Anyhow, we should still assign some PCI ID value (instead of zero).
>
> Thanks,
> Suravee
>
>
>
>
Jonathan Cameron March 12, 2025, 6:18 p.m. UTC | #16
On Tue, 11 Mar 2025 09:08:06 +0100
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > As a heads up, I believe we have a similar problem with a few of the CXL IDs.
> > The root port and type 3 device both use Intel IDs that were not reserved
> > for this purpose.  VID=0x8086, DID=0x7075 and DID=0x0d93  
> 
> Essentially we have two kinds of PCI devices in qemu.
> 
>  * The ones which try to mimic existing hardware, they usually have the
>    PCI ID of the device they are emulating (and use the qemu subsystem ID).
>    The classic example is the cirrus vga.  There are also many intel
>    chipset devices for piix4 ('pc' machine type) and ich9 ('q35' machine
>    type) with intel IDs.
> 
>  * The ones which are PCI class implementations and do not need a
>    specific ID for drivers to accept them.  Most of them have a
>    PCI device ID from the 1b36 vendor ID range.
> 
> The former tend to be older devices (before hardware standardization was
> a thing, also before we got a range from 1b36 for qemu), and the latter
> tend to be newer devices.  There are also a bunch of exceptions for
> historical reasons.  The ahci emulation has a ich9 id.  xhci even has
> two variants (one mimicking a NEC host adapter, one with qemu device id).
> 
> So, in short, using the intel IDs is not necessarily a problem.  Depends
> a bit on what kind of device we are talking about.  For PCI class
> devices it usually is more useful to have a qemu ID though.

They are not valid IDs, so they may get used in future for real
hardware of an entirely different type. We don't want these to
correspond to real hardware either because the aim is to test
out corners of the spec, so we may well implement completely different
sets of features to any real implementation.

> 
> > Switch ports and switch-cci are using valid Hisilicon IDs that are for
> > emulation of these device only and are registered in our tracker
> > for these IDs so won't get 'reused'.  
> 
> That is perfectly fine.  There is no need to change IDs, although it
> makes sense to document that fact in docs/specs/pci-ids.rst

Ah. I wasn't aware of the ID space reserved for QEMU or that doc.

We can leave the HiSilicon ones alone.  I'll sort a docs patch for
the 3 of those soon.

For the Intel ones can I have a pair for the root port and the CXL
type 3 device (so 2 IDs)?  We will probably need one shortly for
the type2 emulation test device as well.

Thanks,

Jonathan


> 
> Moving them to qemu pci id range is an option too if you prefer that.
> Your choice.
> 
> take care,
>   Gerd
> 
>
diff mbox series

Patch

diff --git a/docs/specs/pci-ids.rst b/docs/specs/pci-ids.rst
index 261b0f359f..2416a70a2d 100644
--- a/docs/specs/pci-ids.rst
+++ b/docs/specs/pci-ids.rst
@@ -100,6 +100,8 @@  PCI devices (other than virtio):
   PCI UFS device (``-device ufs``)
 1b36:0014
   PCI RISC-V IOMMU device
+1b36:0015
+  PCI AMD IOMMU device (``-device amd-iommu``)
 
 All these devices are documented in :doc:`index`.
 
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index dda1a5781f..4d8564249c 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1766,7 +1766,8 @@  static void amdvi_pci_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->vendor_id = PCI_VENDOR_ID_AMD;
+    k->vendor_id = PCI_VENDOR_ID_REDHAT;
+    k->device_id = PCI_DEVICE_ID_REDHAT_AMD_IOMMU;
     k->class_id = 0x0806;
     k->realize = amdvi_pci_realize;
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 4002bbeebd..da44e6673d 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -117,6 +117,7 @@  extern bool pci_available;
 #define PCI_DEVICE_ID_REDHAT_ACPI_ERST   0x0012
 #define PCI_DEVICE_ID_REDHAT_UFS         0x0013
 #define PCI_DEVICE_ID_REDHAT_RISCV_IOMMU 0x0014
+#define PCI_DEVICE_ID_REDHAT_AMD_IOMMU   0x0015
 #define PCI_DEVICE_ID_REDHAT_QXL         0x0100
 
 #define FMT_PCIBUS                      PRIx64