Re: [PATCH v3 6/7] acpi/ghes: update comments to point to newer ACPI specs

From: Mauro Carvalho Chehab
Date: Wed Jul 31 2024 - 02:06:04 EST


Em Tue, 30 Jul 2024 07:36:32 -0400
"Michael S. Tsirkin" <mst@xxxxxxxxxx> escreveu:

> On Tue, Jul 30, 2024 at 01:24:30PM +0200, Igor Mammedov wrote:
> > On Mon, 22 Jul 2024 08:45:58 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:
> >
> > > There is one reference to ACPI 4.0 and several references
> > > to ACPI 6.x versions.
> > >
> > > Update them to point to ACPI 6.5 whenever possible.
> >
> > when it comes to APCI doc comments, they should point to
> > the 1st (earliest) revision that provides given feature/value/field/table.
>
> Yes. And the motivation is twofold.
> First, guests are built against
> old acpi versions. knowing in which version things appeared
> helps us know which guests support a feature.

Good point, but IMO, a comment like "since: ACPI 4.0" would
be better, as the comment may not reflect the first version
supporting such features, but, instead, when someone added
support to a particular feature set.

> Second, acpi guys keep churning out new versions.
> It makes no sense to try and update to latest one,
> it will soon get out of date again.

True, but having it updated helps people adding new code to
get things right.

Anyway, I got your point, I'll drop this patch.

> > > void acpi_build_hest(GArray *table_data, BIOSLinker *linker,
> > > const char *oem_id, const char *oem_table_id)
> > > {
> > > - AcpiTable table = { .sig = "HEST", .rev = 1,
> > > + AcpiTable table = { .sig = "HEST",
> > > + .rev = 1, /* ACPI 4.0 to 6.4 */
> > > .oem_id = oem_id, .oem_table_id = oem_table_id };
> > >
> > > acpi_table_begin(&table, table_data);

This hunk might still make sense, though. When double-checking the links
against ACPI 6.5, I noticed that HEST now requires .rev = 2.

There are some future incompatibilities, but the current
implementation of acpi/ghes satisfies both rev 1 and ref 2 of HEST.

Also, this is not relevant on Linux, as the revision is not checked
there.

So, currently this is not a problem.

Thanks,
Mauro