diff mbox series

[RFC,XEN,1/6] x86/pvh: report ACPI VFCT table to dom0 if present

Message ID 20230312075455.450187-2-ray.huang@amd.com (mailing list archive)
State New, archived
Headers show
Series Introduce VirtIO GPU and Passthrough GPU support on Xen PVH dom0 | expand

Commit Message

Huang Rui March 12, 2023, 7:54 a.m. UTC
From: Roger Pau Monne <roger.pau@citrix.com>

The VFCT ACPI table is used by AMD GPUs to expose the vbios ROM image
from the firmware instead of doing it on the PCI ROM on the physical
device.

As such, this needs to be available for PVH dom0 to access, or else
the GPU won't work.

Reported-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-and-Tested-by: Huang Rui <ray.huang@amd.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 xen/arch/x86/hvm/dom0_build.c | 1 +
 xen/include/acpi/actbl3.h     | 1 +
 2 files changed, 2 insertions(+)

Comments

Andrew Cooper March 13, 2023, 11:55 a.m. UTC | #1
On 12/03/2023 7:54 am, Huang Rui wrote:
> From: Roger Pau Monne <roger.pau@citrix.com>
>
> The VFCT ACPI table is used by AMD GPUs to expose the vbios ROM image
> from the firmware instead of doing it on the PCI ROM on the physical
> device.
>
> As such, this needs to be available for PVH dom0 to access, or else
> the GPU won't work.
>
> Reported-by: Huang Rui <ray.huang@amd.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Acked-and-Tested-by: Huang Rui <ray.huang@amd.com>
> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>

Huh...  Despite the release ack, this didn't get committed for 4.17.

Sorry for the oversight.  I've queued this now.

~Andrew
Roger Pau Monné March 13, 2023, 12:21 p.m. UTC | #2
On Mon, Mar 13, 2023 at 11:55:56AM +0000, Andrew Cooper wrote:
> On 12/03/2023 7:54 am, Huang Rui wrote:
> > From: Roger Pau Monne <roger.pau@citrix.com>
> >
> > The VFCT ACPI table is used by AMD GPUs to expose the vbios ROM image
> > from the firmware instead of doing it on the PCI ROM on the physical
> > device.
> >
> > As such, this needs to be available for PVH dom0 to access, or else
> > the GPU won't work.
> >
> > Reported-by: Huang Rui <ray.huang@amd.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Acked-and-Tested-by: Huang Rui <ray.huang@amd.com>
> > Release-acked-by: Henry Wang <Henry.Wang@arm.com>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> 
> Huh...  Despite the release ack, this didn't get committed for 4.17.

There was a pending query from Jan as to where was this table
signature documented or at least registered, as it's not in the ACPI
spec or any related files.

I don't oppose to the change, as it's already used by Linux, so I
think it's impossible for the table signature to be reused, even if
not properly documented (it would cause havoc).

It's however not ideal to set this kind of precedents.

Thanks, Roger.
Andrew Cooper March 13, 2023, 12:27 p.m. UTC | #3
On 13/03/2023 12:21 pm, Roger Pau Monné wrote:
> On Mon, Mar 13, 2023 at 11:55:56AM +0000, Andrew Cooper wrote:
>> On 12/03/2023 7:54 am, Huang Rui wrote:
>>> From: Roger Pau Monne <roger.pau@citrix.com>
>>>
>>> The VFCT ACPI table is used by AMD GPUs to expose the vbios ROM image
>>> from the firmware instead of doing it on the PCI ROM on the physical
>>> device.
>>>
>>> As such, this needs to be available for PVH dom0 to access, or else
>>> the GPU won't work.
>>>
>>> Reported-by: Huang Rui <ray.huang@amd.com>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> Acked-and-Tested-by: Huang Rui <ray.huang@amd.com>
>>> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
>>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>> Huh...  Despite the release ack, this didn't get committed for 4.17.
> There was a pending query from Jan as to where was this table
> signature documented or at least registered, as it's not in the ACPI
> spec or any related files.
>
> I don't oppose to the change, as it's already used by Linux, so I
> think it's impossible for the table signature to be reused, even if
> not properly documented (it would cause havoc).
>
> It's however not ideal to set this kind of precedents.

It's not great, but this exists in real systems, for several generations
it seems.

Making things work for users trumps any idealistic beliefs about
firmware actually conforming to spec.

~Andrew
Huang Rui March 21, 2023, 6:26 a.m. UTC | #4
On Mon, Mar 13, 2023 at 08:27:02PM +0800, Andrew Cooper wrote:
> On 13/03/2023 12:21 pm, Roger Pau Monné wrote:
> > On Mon, Mar 13, 2023 at 11:55:56AM +0000, Andrew Cooper wrote:
> >> On 12/03/2023 7:54 am, Huang Rui wrote:
> >>> From: Roger Pau Monne <roger.pau@citrix.com>
> >>>
> >>> The VFCT ACPI table is used by AMD GPUs to expose the vbios ROM image
> >>> from the firmware instead of doing it on the PCI ROM on the physical
> >>> device.
> >>>
> >>> As such, this needs to be available for PVH dom0 to access, or else
> >>> the GPU won't work.
> >>>
> >>> Reported-by: Huang Rui <ray.huang@amd.com>
> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >>> Acked-and-Tested-by: Huang Rui <ray.huang@amd.com>
> >>> Release-acked-by: Henry Wang <Henry.Wang@arm.com>
> >>> Signed-off-by: Huang Rui <ray.huang@amd.com>
> >> Huh...  Despite the release ack, this didn't get committed for 4.17.
> > There was a pending query from Jan as to where was this table
> > signature documented or at least registered, as it's not in the ACPI
> > spec or any related files.
> >
> > I don't oppose to the change, as it's already used by Linux, so I
> > think it's impossible for the table signature to be reused, even if
> > not properly documented (it would cause havoc).
> >
> > It's however not ideal to set this kind of precedents.
> 
> It's not great, but this exists in real systems, for several generations
> it seems.
> 
> Making things work for users trumps any idealistic beliefs about
> firmware actually conforming to spec.
> 

Thanks Andrew for understanding! These tables have been there for more than
10+ years on all AMD GPU platforms.

Thanks,
Ray
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index 3ac6b7b423..d44de7f2b2 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -892,6 +892,7 @@  static bool __init pvh_acpi_table_allowed(const char *sig,
         ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_FACS, ACPI_SIG_PSDT,
         ACPI_SIG_SSDT, ACPI_SIG_SBST, ACPI_SIG_MCFG, ACPI_SIG_SLIC,
         ACPI_SIG_MSDM, ACPI_SIG_WDAT, ACPI_SIG_FPDT, ACPI_SIG_S3PT,
+        ACPI_SIG_VFCT,
     };
     unsigned int i;
 
diff --git a/xen/include/acpi/actbl3.h b/xen/include/acpi/actbl3.h
index 0a6778421f..6858d3e60f 100644
--- a/xen/include/acpi/actbl3.h
+++ b/xen/include/acpi/actbl3.h
@@ -79,6 +79,7 @@ 
 #define ACPI_SIG_MATR           "MATR"	/* Memory Address Translation Table */
 #define ACPI_SIG_MSDM           "MSDM"	/* Microsoft Data Management Table */
 #define ACPI_SIG_WPBT           "WPBT"	/* Windows Platform Binary Table */
+#define ACPI_SIG_VFCT           "VFCT"	/* AMD Video BIOS */
 
 /*
  * All tables must be byte-packed to match the ACPI specification, since