[libvirt] [PATCH 1/2] libxl: add acpi slic table support

[Marek Marczykowski-Górecki]

On Wed, Sep 11, 2019 at 01:31:34PM +0000, Jim Fehlig wrote:
> On 9/11/19 5:43 AM, Marek Marczykowski-Górecki  wrote:
> > On Wed, Sep 11, 2019 at 02:34:57AM +0000, Jim Fehlig wrote:
> >> On 9/10/19 5:24 PM, Marek Marczykowski-Górecki  wrote:
> >>> On Tue, Sep 10, 2019 at 10:54:15PM +0000, Jim Fehlig wrote:
> >>>> On 9/6/19 8:31 PM, Marek Marczykowski-Górecki  wrote:
> >>>>> From: Ivan Kardykov <kardykov at tabit.pro>
> >>>>>
> >>>>> Libxl driver did not support setup additional acpi firmware to xen
> >>>>> guest. It is necessary to activate OEM Windows installs. This patch
> >>>>> allow to define in OS section acpi table param (which supported domain
> >>>>> common schema).
> >>>>>
> >>>>> Signed-off-by: Ivan Kardykov <kardykov at tabit.pro>
> >>>>> [added info to docs/formatdomain.html.in]
> >>>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek at invisiblethingslab.com>
> >>>>> ---
> >>>>>     docs/formatdomain.html.in | 3 ++-
> >>>>>     src/libxl/libxl_conf.c    | 5 +++++
> >>>>>     2 files changed, 7 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> >>>>> index fcb7c59c00..de612ae870 100644
> >>>>> --- a/docs/formatdomain.html.in
> >>>>> +++ b/docs/formatdomain.html.in
> >>>>> @@ -363,7 +363,8 @@
> >>>>>           <dd>The <code>table</code> element contains a fully-qualified path
> >>>>>             to the ACPI table. The <code>type</code> attribute contains the
> >>>>>             ACPI table type (currently only <code>slic</code> is supported)
> >>>>> -        <span class="since">Since 1.3.5 (QEMU only)</span></dd>
> >>>>> +        <span class="since">Since 1.3.5 (QEM)</span>
> >>>>
> >>>> You removed one too many characters :-). s/QEM/QEMU/
> >>>>
> >>>>> +        <span class="since">Since 5.8.0 (Xen)</span></dd>
> >>>>>         </dl>
> >>>>>     
> >>>>>         <h4><a id="elementsOSContainer">Container boot</a></h4>
> >>>>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> >>>>> index 766a726ebc..c1e248d98c 100644
> >>>>> --- a/src/libxl/libxl_conf.c
> >>>>> +++ b/src/libxl/libxl_conf.c
> >>>>> @@ -506,6 +506,11 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> >>>>>                               def->features[VIR_DOMAIN_FEATURE_ACPI] ==
> >>>>>                               VIR_TRISTATE_SWITCH_ON);
> >>>>>     
> >>>>> +        /* copy SLIC table path to acpi_firmware */
> >>>>> +        if (def->os.slic_table &&
> >>>>> +                VIR_STRDUP(b_info->u.hvm.acpi_firmware, def->os.slic_table) < 0)
> >>>>> +            return -1;
> >>>>> +
> >>>>
> >>>> Is 'acpi_firmware=' the xl.cfg equivalent setting? If so we'll want it added to
> >>>> the domXML<->xl.cfg converter (which now lives in the src/libxl/ directory).
> >>>
> >>> Functionally yes. But acpi_firmware= is about generic ACPI table, not
> >>> only SLIC. This means xl.cfg acpi_firmware= converted to domXML may be
> >>> misleading. Is it a problem?
> >>
> >> I don't think it's a problem. But let me ask another way: How would you specify
> >> the SLIC in xl.cfg? I.e., what would a comparable xl.cfg snippet look like?
> > 
> > acpi_firmware="/sys/firmware/acpi/tables/SLIC"
> > 
> > (for those brave enough ;) )
> 
> :-)
> 
> Ok, so there is no setting to specify the type of ACPI firmware.
> 
> > My concern (maybe not important), is that
> > acpi_firmware="/path/to/non-SLIC/table" will be converted to SLIC entry
> > in libvirt xml.
> 
> Are there other tables in use? Or was this added to libxl primarily for the 
> license table? We are probably fine to imply SLIC if that is all there is ATM. 
> We can add support for other tables as those become prevalent.

I'm not sure what is the primary purpose of this option, I've never
needed it for anything else. But it can point a file with multiple ACPI
tables concatenated. From man page:

    acpi_firmware="STRING"
        Specify a path to a file that contains extra ACPI firmware tables to
        pass in to a guest. The file can contain several tables in their binary
        AML form concatenated together. Each table self describes its length so
        no additional information is needed. These tables will be added to the
        ACPI table set in the guest. Note that existing tables cannot be
        overridden by this feature. For example this cannot be used to override
        tables like DSDT, FADT, etc.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190911/3df24ece/attachment-0001.sig>