[PATCH 0/4] Add support for MSDM ACPI table type

Daniel P. Berrangé posted 4 patches 10 months ago
Failed in applying to current master (apply log)
docs/formatdomain.rst                         |  4 +-
src/conf/domain_conf.c                        | 88 ++++++++++++++-----
src/conf/domain_conf.h                        | 22 ++++-
src/conf/schemas/domaincommon.rng             |  5 +-
src/libvirt_private.syms                      |  2 +
src/libxl/libxl_conf.c                        |  8 +-
src/libxl/libxl_domain.c                      | 21 +++++
src/libxl/xen_xl.c                            | 22 ++++-
src/qemu/qemu_command.c                       | 14 ++-
src/qemu/qemu_validate.c                      | 16 ++++
src/security/security_dac.c                   | 18 ++--
src/security/security_selinux.c               | 16 ++--
src/security/virt-aa-helper.c                 |  5 +-
.../acpi-table-many.x86_64-latest.args        | 34 +++++++
.../acpi-table-many.x86_64-latest.xml         | 39 ++++++++
tests/qemuxmlconfdata/acpi-table-many.xml     | 31 +++++++
tests/qemuxmlconftest.c                       |  1 +
17 files changed, 296 insertions(+), 50 deletions(-)
create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.args
create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/acpi-table-many.xml
[PATCH 0/4] Add support for MSDM ACPI table type
Posted by Daniel P. Berrangé 10 months ago
This was requested by KubeVirt in

  https://gitlab.com/libvirt/libvirt/-/issues/748

I've not functionally tested this, since I lack any suitable guest
windows environment this is looking for MSDM tables, nor does my
machine have MSDM ACPI tables to pass to a guest.

I'm blindly assuming that the QEMU CLI code is identical except for
s/SLIC/MSDM/.

Also I'm pretty unhappy about the situation with the Xen driver
support. This is pre-existing, and IMHO should never have been added
as it exists today, as it allows arbitrary passthrough of *any* set
of ACPI tables, as opposed to a single type of the specific type
listed in the XML.  This should have been handled with a different
XML syntax, but with stuck with this undesirable approach now, so
I've kept it as is.

Daniel P. Berrangé (4):
  conf: introduce support for multiple ACPI tables
  src: validate permitted ACPI table types in libxl/qemu drivers
  conf: support MSDM ACPI table type
  qemu: support MSDM ACPI table type

 docs/formatdomain.rst                         |  4 +-
 src/conf/domain_conf.c                        | 88 ++++++++++++++-----
 src/conf/domain_conf.h                        | 22 ++++-
 src/conf/schemas/domaincommon.rng             |  5 +-
 src/libvirt_private.syms                      |  2 +
 src/libxl/libxl_conf.c                        |  8 +-
 src/libxl/libxl_domain.c                      | 21 +++++
 src/libxl/xen_xl.c                            | 22 ++++-
 src/qemu/qemu_command.c                       | 14 ++-
 src/qemu/qemu_validate.c                      | 16 ++++
 src/security/security_dac.c                   | 18 ++--
 src/security/security_selinux.c               | 16 ++--
 src/security/virt-aa-helper.c                 |  5 +-
 .../acpi-table-many.x86_64-latest.args        | 34 +++++++
 .../acpi-table-many.x86_64-latest.xml         | 39 ++++++++
 tests/qemuxmlconfdata/acpi-table-many.xml     | 31 +++++++
 tests/qemuxmlconftest.c                       |  1 +
 17 files changed, 296 insertions(+), 50 deletions(-)
 create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.args
 create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.xml
 create mode 100644 tests/qemuxmlconfdata/acpi-table-many.xml

-- 
2.47.1
Re: [PATCH 0/4] Add support for MSDM ACPI table type
Posted by Michal Prívozník 10 months ago
On 2/18/25 19:12, Daniel P. Berrangé wrote:
> This was requested by KubeVirt in
> 
>   https://gitlab.com/libvirt/libvirt/-/issues/748
> 
> I've not functionally tested this, since I lack any suitable guest
> windows environment this is looking for MSDM tables, nor does my
> machine have MSDM ACPI tables to pass to a guest.
> 
> I'm blindly assuming that the QEMU CLI code is identical except for
> s/SLIC/MSDM/.
> 
> Also I'm pretty unhappy about the situation with the Xen driver
> support. This is pre-existing, and IMHO should never have been added
> as it exists today, as it allows arbitrary passthrough of *any* set
> of ACPI tables, as opposed to a single type of the specific type
> listed in the XML.  This should have been handled with a different
> XML syntax, but with stuck with this undesirable approach now, so
> I've kept it as is.
> 
> Daniel P. Berrangé (4):
>   conf: introduce support for multiple ACPI tables
>   src: validate permitted ACPI table types in libxl/qemu drivers
>   conf: support MSDM ACPI table type
>   qemu: support MSDM ACPI table type
> 
>  docs/formatdomain.rst                         |  4 +-
>  src/conf/domain_conf.c                        | 88 ++++++++++++++-----
>  src/conf/domain_conf.h                        | 22 ++++-
>  src/conf/schemas/domaincommon.rng             |  5 +-
>  src/libvirt_private.syms                      |  2 +
>  src/libxl/libxl_conf.c                        |  8 +-
>  src/libxl/libxl_domain.c                      | 21 +++++
>  src/libxl/xen_xl.c                            | 22 ++++-
>  src/qemu/qemu_command.c                       | 14 ++-
>  src/qemu/qemu_validate.c                      | 16 ++++
>  src/security/security_dac.c                   | 18 ++--
>  src/security/security_selinux.c               | 16 ++--
>  src/security/virt-aa-helper.c                 |  5 +-
>  .../acpi-table-many.x86_64-latest.args        | 34 +++++++
>  .../acpi-table-many.x86_64-latest.xml         | 39 ++++++++
>  tests/qemuxmlconfdata/acpi-table-many.xml     | 31 +++++++
>  tests/qemuxmlconftest.c                       |  1 +
>  17 files changed, 296 insertions(+), 50 deletions(-)
>  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.args
>  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.xml
>  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.xml
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

I even tested this successfully with the following MSDM table (which was
reconstructed from various posts on stackoverflow and blog posts):

$ hexdump -C msdm.bin
00000000  4d 53 44 4d 55 00 00 00  03 13 48 50 51 4f 45 4d  |MSDMU.....HPQOEM|
00000010  53 4c 49 43 2d 4d 50 43  01 00 00 00 48 50 20 20  |SLIC-MPC....HP  |
00000020  00 00 04 00 01 00 00 00  00 00 00 00 01 00 00 00  |................|
00000030  00 00 00 00 1d 00 00 00  41 42 43 44 45 2d 46 47  |........ABCDE-FG|
00000040  48 49 4a 2d 4b 4c 4d 4e  4f 2d 50 51 52 53 54 2d  |HIJ-KLMNO-PQRST-|
00000050  55 56 57 58 59                                    |UVWXY|
00000055


Here ABCDE-... is the Windows key.

Michal
Re: [PATCH 0/4] Add support for MSDM ACPI table type
Posted by Victor Toso 10 months ago
Hi,

On Tue, Feb 18, 2025 at 06:12:49PM +0000, Daniel P. Berrangé wrote:
> This was requested by KubeVirt in
> 
>   https://gitlab.com/libvirt/libvirt/-/issues/748
> 
> I've not functionally tested this, since I lack any suitable guest
> windows environment this is looking for MSDM tables, nor does my
> machine have MSDM ACPI tables to pass to a guest.
> 
> I'm blindly assuming that the QEMU CLI code is identical except for
> s/SLIC/MSDM/.

I think it is the right assumption from what I see people using
with the qemu:arg from qemu:commandline option in the thread:

    https://gist.github.com/Informatic/49bd034d43e054bd1d8d4fec38c305ec#file-domain-xml

> Also I'm pretty unhappy about the situation with the Xen driver
> support. This is pre-existing, and IMHO should never have been added
> as it exists today, as it allows arbitrary passthrough of *any* set
> of ACPI tables, as opposed to a single type of the specific type
> listed in the XML.  This should have been handled with a different
> XML syntax, but with stuck with this undesirable approach now, so
> I've kept it as is.
> 
> Daniel P. Berrangé (4):
>   conf: introduce support for multiple ACPI tables
>   src: validate permitted ACPI table types in libxl/qemu drivers
>   conf: support MSDM ACPI table type
>   qemu: support MSDM ACPI table type
> 
>  docs/formatdomain.rst                         |  4 +-
>  src/conf/domain_conf.c                        | 88 ++++++++++++++-----
>  src/conf/domain_conf.h                        | 22 ++++-
>  src/conf/schemas/domaincommon.rng             |  5 +-
>  src/libvirt_private.syms                      |  2 +
>  src/libxl/libxl_conf.c                        |  8 +-
>  src/libxl/libxl_domain.c                      | 21 +++++
>  src/libxl/xen_xl.c                            | 22 ++++-
>  src/qemu/qemu_command.c                       | 14 ++-
>  src/qemu/qemu_validate.c                      | 16 ++++
>  src/security/security_dac.c                   | 18 ++--
>  src/security/security_selinux.c               | 16 ++--
>  src/security/virt-aa-helper.c                 |  5 +-
>  .../acpi-table-many.x86_64-latest.args        | 34 +++++++
>  .../acpi-table-many.x86_64-latest.xml         | 39 ++++++++
>  tests/qemuxmlconfdata/acpi-table-many.xml     | 31 +++++++
>  tests/qemuxmlconftest.c                       |  1 +
>  17 files changed, 296 insertions(+), 50 deletions(-)
>  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.args
>  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.xml
>  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.xml

Cheers,
Victor
Re: [PATCH 0/4] Add support for MSDM ACPI table type
Posted by Daniel P. Berrangé 10 months ago
On Tue, Feb 18, 2025 at 07:20:01PM +0100, Victor Toso wrote:
> Hi,
> 
> On Tue, Feb 18, 2025 at 06:12:49PM +0000, Daniel P. Berrangé wrote:
> > This was requested by KubeVirt in
> > 
> >   https://gitlab.com/libvirt/libvirt/-/issues/748
> > 
> > I've not functionally tested this, since I lack any suitable guest
> > windows environment this is looking for MSDM tables, nor does my
> > machine have MSDM ACPI tables to pass to a guest.
> > 
> > I'm blindly assuming that the QEMU CLI code is identical except for
> > s/SLIC/MSDM/.
> 
> I think it is the right assumption from what I see people using
> with the qemu:arg from qemu:commandline option in the thread:
> 
>     https://gist.github.com/Informatic/49bd034d43e054bd1d8d4fec38c305ec#file-domain-xml

Hmm, that's interesting in that they're not passing the 'sig' parameter
to -acpitable at all. Reading the QEMU code it seems 'sig' is indeed
optional, merely used to override the 'sig' that is embedded in the
acpitable content loaded from the file.

This is actually good, as it means we would not need to add XML syupport
for every possible ACPI table type. We can just rely on the built-in
default, which would also make it practical to reconcile the Xen problem
below

> > Also I'm pretty unhappy about the situation with the Xen driver
> > support. This is pre-existing, and IMHO should never have been added
> > as it exists today, as it allows arbitrary passthrough of *any* set
> > of ACPI tables, as opposed to a single type of the specific type
> > listed in the XML.  This should have been handled with a different
> > XML syntax, but with stuck with this undesirable approach now, so
> > I've kept it as is.
> > 
> > Daniel P. Berrangé (4):
> >   conf: introduce support for multiple ACPI tables
> >   src: validate permitted ACPI table types in libxl/qemu drivers
> >   conf: support MSDM ACPI table type
> >   qemu: support MSDM ACPI table type
> > 
> >  docs/formatdomain.rst                         |  4 +-
> >  src/conf/domain_conf.c                        | 88 ++++++++++++++-----
> >  src/conf/domain_conf.h                        | 22 ++++-
> >  src/conf/schemas/domaincommon.rng             |  5 +-
> >  src/libvirt_private.syms                      |  2 +
> >  src/libxl/libxl_conf.c                        |  8 +-
> >  src/libxl/libxl_domain.c                      | 21 +++++
> >  src/libxl/xen_xl.c                            | 22 ++++-
> >  src/qemu/qemu_command.c                       | 14 ++-
> >  src/qemu/qemu_validate.c                      | 16 ++++
> >  src/security/security_dac.c                   | 18 ++--
> >  src/security/security_selinux.c               | 16 ++--
> >  src/security/virt-aa-helper.c                 |  5 +-
> >  .../acpi-table-many.x86_64-latest.args        | 34 +++++++
> >  .../acpi-table-many.x86_64-latest.xml         | 39 ++++++++
> >  tests/qemuxmlconfdata/acpi-table-many.xml     | 31 +++++++
> >  tests/qemuxmlconftest.c                       |  1 +
> >  17 files changed, 296 insertions(+), 50 deletions(-)
> >  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.args
> >  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.x86_64-latest.xml
> >  create mode 100644 tests/qemuxmlconfdata/acpi-table-many.xml
> 
> Cheers,
> Victor



With 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 :|