[libvirt] [PATCH 1/2] libxl: add acpi slic table support
From: Ivan Kardykov <kardykov@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@tabit.pro> [added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek@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> + <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; + if (def->nsounds > 0) { /* * Use first sound device. man xl.cfg(5) describes soundhw as -- 2.21.0
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- .../fullvirt-acpi-slic.json | 54 +++++++++++++++++++ .../fullvirt-acpi-slic.xml | 32 +++++++++++ tests/libxlxml2domconfigtest.c | 2 + 3 files changed, 88 insertions(+) create mode 100644 tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json create mode 100644 tests/libxlxml2domconfigdata/fullvirt-acpi-slic.xml diff --git a/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json b/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json new file mode 100644 index 0000000000..5d85d75af5 --- /dev/null +++ b/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json @@ -0,0 +1,54 @@ +{ + "c_info": { + "type": "hvm", + "name": "XenGuest2", + "uuid": "c7a5fdb2-cdaf-9455-926a-d65c16db1809" + }, + "b_info": { + "max_vcpus": 1, + "avail_vcpus": [ + 0 + ], + "max_memkb": 592896, + "target_memkb": 403456, + "shadow_memkb": 5656, + "sched_params": { + }, + "nested_hvm": "False", + "type.hvm": { + "pae": "True", + "apic": "True", + "acpi": "True", + "acpi_firmware": "/path/to/slic.dat", + "nographic": "True", + "vnc": { + "enable": "False" + }, + "sdl": { + "enable": "False" + }, + "spice": { + + }, + "boot": "c", + "rdm": { + + } + }, + "arch_arm": { + + } + }, + "disks": [ + { + "pdev_path": "/dev/HostVG/XenGuest2", + "vdev": "hda", + "backend": "phy", + "format": "raw", + "removable": 1, + "readwrite": 1 + } + ], + "on_reboot": "restart", + "on_crash": "restart" +} diff --git a/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.xml b/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.xml new file mode 100644 index 0000000000..017fdb5062 --- /dev/null +++ b/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.xml @@ -0,0 +1,32 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>592896</memory> + <currentMemory unit='KiB'>403456</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <acpi> + <table type='slic'>/path/to/slic.dat</table> + </acpi> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + </devices> +</domain> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index 3b3f63403e..120796b110 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -212,6 +212,8 @@ mymain(void) DO_TEST("fullvirt-cpuid-legacy-nest"); # endif + DO_TEST("fullvirt-acpi-slic"); + # ifdef LIBXL_HAVE_BUILDINFO_GRANT_LIMITS DO_TEST("max-gntframes-hvm"); # endif -- 2.21.0
On 9/6/19 8:31 PM, Marek Marczykowski-Górecki wrote:
From: Ivan Kardykov <kardykov@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@tabit.pro> [added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek@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). Regards, Jim
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@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@tabit.pro> [added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek@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? -- 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?
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@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@tabit.pro> [added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek@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? Regards, Jim
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@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@tabit.pro> [added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek@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 ;) ) My concern (maybe not important), is that acpi_firmware="/path/to/non-SLIC/table" will be converted to SLIC entry in libvirt xml. -- 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?
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@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@tabit.pro> [added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek@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. Regards, Jim
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@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@tabit.pro> [added info to docs/formatdomain.html.in] Signed-off-by: Marek Marczykowski-Górecki <marmarek@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?
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@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@tabit.pro> > [added info to docs/formatdomain.html.in] > Signed-off-by: Marek Marczykowski-Górecki <marmarek@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.
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
participants (2)
-
Jim Fehlig -
Marek Marczykowski-Górecki