On 9/11/19 10:33 AM, Marek Marczykowski-Górecki wrote: > 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@xxxxxxxxx> >>>>>>> >>>>>>> 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@xxxxxxxxx> >>>>>>> [added info to docs/formatdomain.html.in] >>>>>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx> >>>>>>> --- >>>>>>> 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. I suspect SLIC was the motivation for this option, and I think we are safe to only support it in the converter since the libvirt documentation already states that SLIC is the only supported type. Regards, Jim -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
