[Qemu-devel] [PATCH 0/9] tests: apci: consolidate and cleanup ACPI test code

Igor Mammedov posted 9 patches 5 years, 4 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
tests/acpi-utils.h       |  51 ++--------
tests/acpi-utils.c       |  33 ++++--
tests/bios-tables-test.c | 257 ++++++++++++-----------------------------------
tests/vmgenid-test.c     |  63 ++++--------
4 files changed, 116 insertions(+), 288 deletions(-)
[Qemu-devel] [PATCH 0/9] tests: apci: consolidate and cleanup ACPI test code
Posted by Igor Mammedov 5 years, 4 months ago
While working on adding tests for virt/arm board (uefi/XSDT/64-bit table pointers),
I found it's rather difficult to deal with mixed ACPI testing code that we've
collected so far. So instead of just adding a pile of XSDT hacks on top, here
goes small refactoring series:
   * that removes dead code
   * replaces reading tables with a fetch per table everywhere instead of
     mix of field by field and whole table
   * consolidates the way tables are read (reduces code duplication)
   * test no longer depends on ACPI structures from QEMU (i.e. doesn't affected
     by mistakes there) 
   * fixiex FACS not beint compared against reference tables
Overall test is reduced on ~170LOC and hopefully it makes easier to add more
stuff on top.

PS:
Series depends on '[PATCH v3 0/8] hw: acpi: RSDP fixes and refactoring'
to avoid nontrivial conflicts

PS2:
arm/virt test patches fill follow up a separate series on top of this one
for not to mix things up


CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Thomas Huth <thuth@redhat.com>
CC: Laurent Vivier <lvivier@redhat.com>
CC: Samuel Ortiz <sameo@linux.intel.com>


Igor Mammedov (9):
  tests: acpi: remove not used ACPI_READ_GENERIC_ADDRESS macro
  tests: acpi: use AcpiSdtTable::aml in consistent way
  tests: acpi: make sure FADT is fetched only once
  tests: acpi: simplify rsdt handling
  tests: acpi: reuse fetch_table() for fetching FACS and DSDT
  tests: acpi: reuse fetch_table() in vmgenid-test
  tests: smbios: fetch whole table in one step instead of reading it
    step by step
  tests: acpi: squash sanitize_fadt_ptrs() into test_acpi_fadt_table()
  tests: acpi: use AcpiSdtTable::aml instead of
    AcpiSdtTable::header::signature

 tests/acpi-utils.h       |  51 ++--------
 tests/acpi-utils.c       |  33 ++++--
 tests/bios-tables-test.c | 257 ++++++++++++-----------------------------------
 tests/vmgenid-test.c     |  63 ++++--------
 4 files changed, 116 insertions(+), 288 deletions(-)

-- 
2.7.4


Re: [Qemu-devel] [PATCH 0/9] tests: apci: consolidate and cleanup ACPI test code
Posted by Michael S. Tsirkin 5 years, 4 months ago
On Mon, Dec 10, 2018 at 07:10:06PM +0100, Igor Mammedov wrote:
> While working on adding tests for virt/arm board (uefi/XSDT/64-bit table pointers),
> I found it's rather difficult to deal with mixed ACPI testing code that we've
> collected so far. So instead of just adding a pile of XSDT hacks on top, here
> goes small refactoring series:
>    * that removes dead code
>    * replaces reading tables with a fetch per table everywhere instead of
>      mix of field by field and whole table
>    * consolidates the way tables are read (reduces code duplication)
>    * test no longer depends on ACPI structures from QEMU (i.e. doesn't affected
>      by mistakes there) 
>    * fixiex FACS not beint compared against reference tables
> Overall test is reduced on ~170LOC and hopefully it makes easier to add more
> stuff on top.

So this was posted outside the merge window - do you still want
it merged? If yes pls repost.

Another idea I had for a while is to compare against expected
files before iasl. Only disassemble on comparison failure.

This will
- speed the good path up
- allow testing on systems without iasl (though debugging will be
  harder)

> PS:
> Series depends on '[PATCH v3 0/8] hw: acpi: RSDP fixes and refactoring'
> to avoid nontrivial conflicts
> 
> PS2:
> arm/virt test patches fill follow up a separate series on top of this one
> for not to mix things up
> 
> 
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> CC: Thomas Huth <thuth@redhat.com>
> CC: Laurent Vivier <lvivier@redhat.com>
> CC: Samuel Ortiz <sameo@linux.intel.com>
> 
> 
> Igor Mammedov (9):
>   tests: acpi: remove not used ACPI_READ_GENERIC_ADDRESS macro
>   tests: acpi: use AcpiSdtTable::aml in consistent way
>   tests: acpi: make sure FADT is fetched only once
>   tests: acpi: simplify rsdt handling
>   tests: acpi: reuse fetch_table() for fetching FACS and DSDT
>   tests: acpi: reuse fetch_table() in vmgenid-test
>   tests: smbios: fetch whole table in one step instead of reading it
>     step by step
>   tests: acpi: squash sanitize_fadt_ptrs() into test_acpi_fadt_table()
>   tests: acpi: use AcpiSdtTable::aml instead of
>     AcpiSdtTable::header::signature
> 
>  tests/acpi-utils.h       |  51 ++--------
>  tests/acpi-utils.c       |  33 ++++--
>  tests/bios-tables-test.c | 257 ++++++++++++-----------------------------------
>  tests/vmgenid-test.c     |  63 ++++--------
>  4 files changed, 116 insertions(+), 288 deletions(-)
> 
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH 0/9] tests: apci: consolidate and cleanup ACPI test code
Posted by Daniel P. Berrangé 5 years, 4 months ago
On Wed, Dec 19, 2018 at 11:38:36AM -0500, Michael S. Tsirkin wrote:
> On Mon, Dec 10, 2018 at 07:10:06PM +0100, Igor Mammedov wrote:
> > While working on adding tests for virt/arm board (uefi/XSDT/64-bit table pointers),
> > I found it's rather difficult to deal with mixed ACPI testing code that we've
> > collected so far. So instead of just adding a pile of XSDT hacks on top, here
> > goes small refactoring series:
> >    * that removes dead code
> >    * replaces reading tables with a fetch per table everywhere instead of
> >      mix of field by field and whole table
> >    * consolidates the way tables are read (reduces code duplication)
> >    * test no longer depends on ACPI structures from QEMU (i.e. doesn't affected
> >      by mistakes there) 
> >    * fixiex FACS not beint compared against reference tables
> > Overall test is reduced on ~170LOC and hopefully it makes easier to add more
> > stuff on top.
> 
> So this was posted outside the merge window - do you still want
> it merged? If yes pls repost.

Huh, QEMU development does not have merge windows. Subsystem maintainers
should review patch series at any time & queue it if it is acceptable.
The freeze process only applies to maintainers sending pull requests for
merge to git mater.  Contributors shouldn't be expected to resubmit
patches in this case.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/9] tests: apci: consolidate and cleanup ACPI test code
Posted by Michael S. Tsirkin 5 years, 4 months ago
On Wed, Dec 19, 2018 at 04:45:17PM +0000, Daniel P. Berrangé wrote:
> On Wed, Dec 19, 2018 at 11:38:36AM -0500, Michael S. Tsirkin wrote:
> > On Mon, Dec 10, 2018 at 07:10:06PM +0100, Igor Mammedov wrote:
> > > While working on adding tests for virt/arm board (uefi/XSDT/64-bit table pointers),
> > > I found it's rather difficult to deal with mixed ACPI testing code that we've
> > > collected so far. So instead of just adding a pile of XSDT hacks on top, here
> > > goes small refactoring series:
> > >    * that removes dead code
> > >    * replaces reading tables with a fetch per table everywhere instead of
> > >      mix of field by field and whole table
> > >    * consolidates the way tables are read (reduces code duplication)
> > >    * test no longer depends on ACPI structures from QEMU (i.e. doesn't affected
> > >      by mistakes there) 
> > >    * fixiex FACS not beint compared against reference tables
> > > Overall test is reduced on ~170LOC and hopefully it makes easier to add more
> > > stuff on top.
> > 
> > So this was posted outside the merge window - do you still want
> > it merged? If yes pls repost.
> 
> Huh, QEMU development does not have merge windows. Subsystem maintainers
> should review patch series at any time & queue it if it is acceptable.
> The freeze process only applies to maintainers sending pull requests for
> merge to git mater.  Contributors shouldn't be expected to resubmit
> patches in this case.
> 
> Regards,
> Daniel

Yea, I try to help drive-by contributors by tracking their patches even
during the freeze. Merged some of these in the 1st pull req.  But Igor
is a co-maintainer of the ACPI subsystem. Shouldn't be a problem.

> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/9] tests: apci: consolidate and cleanup ACPI test code
Posted by Igor Mammedov 5 years, 3 months ago
On Wed, 19 Dec 2018 11:38:36 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Dec 10, 2018 at 07:10:06PM +0100, Igor Mammedov wrote:
> > While working on adding tests for virt/arm board (uefi/XSDT/64-bit table pointers),
> > I found it's rather difficult to deal with mixed ACPI testing code that we've
> > collected so far. So instead of just adding a pile of XSDT hacks on top, here
> > goes small refactoring series:
> >    * that removes dead code
> >    * replaces reading tables with a fetch per table everywhere instead of
> >      mix of field by field and whole table
> >    * consolidates the way tables are read (reduces code duplication)
> >    * test no longer depends on ACPI structures from QEMU (i.e. doesn't affected
> >      by mistakes there) 
> >    * fixiex FACS not beint compared against reference tables
> > Overall test is reduced on ~170LOC and hopefully it makes easier to add more
> > stuff on top.  
> 
> So this was posted outside the merge window - do you still want
> it merged? If yes pls repost.
no problem I'll wait for a couple of days for more comments on patches itself
and repost.

> Another idea I had for a while is to compare against expected
> files before iasl. Only disassemble on comparison failure.
I'll add that to my TODO list (it seems easy enough to implement quickly),
but so far starting VM is the largest time consumer and
with UEFI firmware it becomes even worse.

> 
> This will
> - speed the good path up
> - allow testing on systems without iasl (though debugging will be
>   harder)
> 
> > PS:
> > Series depends on '[PATCH v3 0/8] hw: acpi: RSDP fixes and refactoring'
> > to avoid nontrivial conflicts
> > 
> > PS2:
> > arm/virt test patches fill follow up a separate series on top of this one
> > for not to mix things up
> > 
> > 
> > CC: "Michael S. Tsirkin" <mst@redhat.com>
> > CC: Thomas Huth <thuth@redhat.com>
> > CC: Laurent Vivier <lvivier@redhat.com>
> > CC: Samuel Ortiz <sameo@linux.intel.com>
> > 
> > 
> > Igor Mammedov (9):
> >   tests: acpi: remove not used ACPI_READ_GENERIC_ADDRESS macro
> >   tests: acpi: use AcpiSdtTable::aml in consistent way
> >   tests: acpi: make sure FADT is fetched only once
> >   tests: acpi: simplify rsdt handling
> >   tests: acpi: reuse fetch_table() for fetching FACS and DSDT
> >   tests: acpi: reuse fetch_table() in vmgenid-test
> >   tests: smbios: fetch whole table in one step instead of reading it
> >     step by step
> >   tests: acpi: squash sanitize_fadt_ptrs() into test_acpi_fadt_table()
> >   tests: acpi: use AcpiSdtTable::aml instead of
> >     AcpiSdtTable::header::signature
> > 
> >  tests/acpi-utils.h       |  51 ++--------
> >  tests/acpi-utils.c       |  33 ++++--
> >  tests/bios-tables-test.c | 257 ++++++++++++-----------------------------------
> >  tests/vmgenid-test.c     |  63 ++++--------
> >  4 files changed, 116 insertions(+), 288 deletions(-)
> > 
> > -- 
> > 2.7.4