Re: [PATCH v4 23/42] tests/acpi: allow CEDT table addition

From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Ben Widawsky" <ben.widawsky@intel.com>,
	"Samarth Saxena" <samarths@cadence.com>,
	"Chris Browy" <cbrowy@avery-design.com>,
	qemu-devel@nongnu.org, linux-cxl@vger.kernel.org,
	linuxarm@huawei.com, "Shreyas Shah" <shreyas.shah@elastics.cloud>,
	"Saransh Gupta1" <saransh@ibm.com>,
	"Shameerali Kolothum Thodi"
	<shameerali.kolothum.thodi@huawei.com>,
	"Marcel Apfelbaum" <marcel@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: [PATCH v4 23/42] tests/acpi: allow CEDT table addition
Date: Wed, 9 Feb 2022 14:09:26 -0500	[thread overview]
Message-ID: <20220209140746-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20220209181801.00000e4d@huawei.com>

On Wed, Feb 09, 2022 at 06:18:01PM +0000, Jonathan Cameron wrote:
> On Mon, 24 Jan 2022 17:16:46 +0000
> Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> 
> > From: Ben Widawsky <ben.widawsky@intel.com>
> > 
> > Following patches will add a new ACPI table, the
> > CXL Early Discovery Table (CEDT).
> > 
> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  tests/data/acpi/pc/CEDT                     | 0
> >  tests/data/acpi/q35/CEDT                    | 0
> >  tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
> >  3 files changed, 2 insertions(+)
> > 
> > diff --git a/tests/data/acpi/pc/CEDT b/tests/data/acpi/pc/CEDT
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/data/acpi/q35/CEDT b/tests/data/acpi/q35/CEDT
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> > index dfb8523c8b..9b07f1e1ff 100644
> > --- a/tests/qtest/bios-tables-test-allowed-diff.h
> > +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> > @@ -1 +1,3 @@
> >  /* List of comma-separated changed AML files to ignore */
> > +"tests/data/acpi/pc/CEDT",
> > +"tests/data/acpi/q35/CEDT",
> 
> Whilst reordering the series to allow partial sets to be picked up
> it occurred to me that there is reason why we should now generate a CEDT
> table unless cxl=on is set for the relevant machine.

Absolutely. But this is just an example, it's an instance of
a bigger issue: skipping new features when they are off
reduces the chance of legacy guest breakage.

> Adding the relevant conditions means we can avoid this dance
> to add effectively empty CEDT tables.  I have added a proper
> test for to bios-test-tables but that can be introduced once
> everything is in place, rather than as we go along (whereas
> for qtests/cxl-test.c I am doing a series of updates as the 
> functionality is built up.).
> 
> Makes for a simpler patch series to update.
> 
> Jonathan