[libvirt] [PATCH 7/8] qemu: Implement the HTM pSeries feature

Andrea Bolognani posted 8 patches 7 years, 2 months ago
[libvirt] [PATCH 7/8] qemu: Implement the HTM pSeries feature
Posted by Andrea Bolognani 7 years, 2 months ago
This is the first in a list of pSeries-specific optional features
that have recently been introduced in QEMU. Along with the feature
proper, some generic code that will make it easier to implement
subsequent features is introduced as well.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 docs/formatdomain.html.in                          |   7 +
 docs/schemas/domaincommon.rng                      |   5 +
 src/conf/domain_conf.c                             |  19 +++
 src/conf/domain_conf.h                             |   1 +
 src/qemu/qemu_command.c                            | 149 +++++++++++++++++++++
 src/qemu/qemu_domain.c                             |  13 ++
 .../pseries-features-invalid-machine.xml           |   1 +
 tests/qemuxml2argvdata/pseries-features.args       |   2 +-
 tests/qemuxml2argvdata/pseries-features.xml        |   1 +
 tests/qemuxml2argvtest.c                           |   1 +
 tests/qemuxml2xmltest.c                            |   1 +
 11 files changed, 199 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6fd2189cd2..51400afa49 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2057,6 +2057,13 @@
           the attribute is not defined, the hypervisor default will be used.
           <span class="since">Since 3.10.0</span> (QEMU/KVM only)
       </dd>
+      <dt><code>htm</code></dt>
+      <dd>Configure HTM (Hardware Transational Memory) availability for
+          pSeries guests. Possible values for the <code>state</code> attribute
+          are <code>on</code> and <code>off</code>. If the attribute is not
+          defined, the hypervisor default will be used.
+          <span class="since">Since 4.2.0</span> (QEMU/KVM only)
+      </dd>
       <dt><code>vmcoreinfo</code></dt>
       <dd>Enable QEMU vmcoreinfo device to let the guest kernel save debug
           details. <span class="since">Since 3.10.0</span> (QEMU only)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8165e699d6..b4143f5bc3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4797,6 +4797,11 @@
           <optional>
             <ref name="hpt"/>
           </optional>
+          <optional>
+            <element name="htm">
+              <ref name="featurestate"/>
+            </element>
+          </optional>
           <optional>
             <ref name="vmcoreinfo"/>
           </optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 70b19311b4..98897d3a63 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -152,6 +152,7 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
               "ioapic",
               "hpt",
               "vmcoreinfo",
+              "htm",
 );
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
@@ -19297,6 +19298,22 @@ virDomainDefParseXML(xmlDocPtr xml,
             }
             break;
 
+        case VIR_DOMAIN_FEATURE_HTM:
+            if (!(tmp = virXMLPropString(nodes[i], "state"))) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("missing state attribute '%s' of feature '%s'"),
+                               tmp, virDomainFeatureTypeToString(val));
+                goto error;
+            }
+            if ((def->features[val] = virTristateSwitchTypeFromString(tmp)) < 0) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("unknown state attribute '%s' of feature '%s'"),
+                               tmp, virDomainFeatureTypeToString(val));
+                goto error;
+            }
+            VIR_FREE(tmp);
+            break;
+
         /* coverity[dead_error_begin] */
         case VIR_DOMAIN_FEATURE_LAST:
             break;
@@ -21384,6 +21401,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
         case VIR_DOMAIN_FEATURE_VMPORT:
         case VIR_DOMAIN_FEATURE_SMM:
         case VIR_DOMAIN_FEATURE_VMCOREINFO:
+        case VIR_DOMAIN_FEATURE_HTM:
             if (src->features[i] != dst->features[i]) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("State of feature '%s' differs: "
@@ -26845,6 +26863,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
             case VIR_DOMAIN_FEATURE_PVSPINLOCK:
             case VIR_DOMAIN_FEATURE_VMPORT:
             case VIR_DOMAIN_FEATURE_SMM:
+            case VIR_DOMAIN_FEATURE_HTM:
                 switch ((virTristateSwitch) def->features[i]) {
                 case VIR_TRISTATE_SWITCH_LAST:
                 case VIR_TRISTATE_SWITCH_ABSENT:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5859c8f4b1..b87a9d9de2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1746,6 +1746,7 @@ typedef enum {
     VIR_DOMAIN_FEATURE_IOAPIC,
     VIR_DOMAIN_FEATURE_HPT,
     VIR_DOMAIN_FEATURE_VMCOREINFO,
+    VIR_DOMAIN_FEATURE_HTM,
 
     VIR_DOMAIN_FEATURE_LAST
 } virDomainFeature;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fa0aa5d5c3..d58211c4c6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7117,6 +7117,113 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
     return 0;
 }
 
+static int
+virDomainFeatureToQEMUCaps(int feature)
+{
+    switch ((virDomainFeature) feature) {
+    case VIR_DOMAIN_FEATURE_HTM:
+        return QEMU_CAPS_MACHINE_PSERIES_CAP_HTM;
+    case VIR_DOMAIN_FEATURE_ACPI:
+    case VIR_DOMAIN_FEATURE_APIC:
+    case VIR_DOMAIN_FEATURE_PAE:
+    case VIR_DOMAIN_FEATURE_HAP:
+    case VIR_DOMAIN_FEATURE_VIRIDIAN:
+    case VIR_DOMAIN_FEATURE_PRIVNET:
+    case VIR_DOMAIN_FEATURE_HYPERV:
+    case VIR_DOMAIN_FEATURE_KVM:
+    case VIR_DOMAIN_FEATURE_PVSPINLOCK:
+    case VIR_DOMAIN_FEATURE_CAPABILITIES:
+    case VIR_DOMAIN_FEATURE_PMU:
+    case VIR_DOMAIN_FEATURE_VMPORT:
+    case VIR_DOMAIN_FEATURE_GIC:
+    case VIR_DOMAIN_FEATURE_SMM:
+    case VIR_DOMAIN_FEATURE_IOAPIC:
+    case VIR_DOMAIN_FEATURE_HPT:
+    case VIR_DOMAIN_FEATURE_VMCOREINFO:
+    case VIR_DOMAIN_FEATURE_LAST:
+    default:
+        return -1;
+    }
+
+    return -1;
+}
+
+static const char*
+virDomainFeatureToMachineOption(int feature)
+{
+    switch ((virDomainFeature) feature) {
+    case VIR_DOMAIN_FEATURE_HTM:
+        return "cap-htm";
+    case VIR_DOMAIN_FEATURE_ACPI:
+    case VIR_DOMAIN_FEATURE_APIC:
+    case VIR_DOMAIN_FEATURE_PAE:
+    case VIR_DOMAIN_FEATURE_HAP:
+    case VIR_DOMAIN_FEATURE_VIRIDIAN:
+    case VIR_DOMAIN_FEATURE_PRIVNET:
+    case VIR_DOMAIN_FEATURE_HYPERV:
+    case VIR_DOMAIN_FEATURE_KVM:
+    case VIR_DOMAIN_FEATURE_PVSPINLOCK:
+    case VIR_DOMAIN_FEATURE_CAPABILITIES:
+    case VIR_DOMAIN_FEATURE_PMU:
+    case VIR_DOMAIN_FEATURE_VMPORT:
+    case VIR_DOMAIN_FEATURE_GIC:
+    case VIR_DOMAIN_FEATURE_SMM:
+    case VIR_DOMAIN_FEATURE_IOAPIC:
+    case VIR_DOMAIN_FEATURE_HPT:
+    case VIR_DOMAIN_FEATURE_VMCOREINFO:
+    case VIR_DOMAIN_FEATURE_LAST:
+    default:
+        return NULL;
+    }
+
+    return NULL;
+}
+
+static int
+qemuBuildMachineCommandLineFeature(virBufferPtr buf,
+                                   virDomainFeature feature,
+                                   const char *value,
+                                   virQEMUCapsPtr qemuCaps)
+{
+    const char *name;
+    const char *option;
+    int cap;
+    int ret = -1;
+
+    if (!(name = virDomainFeatureTypeToString(feature))) {
+        virReportEnumRangeError(virDomainFeature, feature);
+        goto cleanup;
+    }
+
+    if (!(option = virDomainFeatureToMachineOption(feature))) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Unknown QEMU option for '%s' feature"),
+                       name);
+        goto cleanup;
+    }
+
+    if ((cap = virDomainFeatureToQEMUCaps(feature)) < 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Unknown QEMU capability for '%s' feature"),
+                       name);
+        goto cleanup;
+    }
+
+    if (cap > 0 && !virQEMUCapsGet(qemuCaps, cap)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("'%s' feature not supported by this QEMU binary"),
+                       name);
+        goto cleanup;
+    }
+
+    virBufferAsprintf(buf, ",%s=%s", option, value);
+
+    ret = 0;
+
+ cleanup:
+    return ret;
+}
+
 static int
 qemuBuildMachineCommandLine(virCommandPtr cmd,
                             virQEMUDriverConfigPtr cfg,
@@ -7343,6 +7450,48 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
             virBufferAsprintf(&buf, ",resize-hpt=%s", str);
         }
 
+        for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
+            const char *value;
+
+            switch ((virDomainFeature) i) {
+            case VIR_DOMAIN_FEATURE_HTM:
+                if (def->features[i] == VIR_TRISTATE_SWITCH_ABSENT)
+                    break;
+
+                if (!(value = virTristateSwitchTypeToString(def->features[i]))) {
+                    virReportEnumRangeError(virTristateSwitch, def->features[i]);
+                    goto cleanup;
+                }
+
+                if (qemuBuildMachineCommandLineFeature(&buf, i, value, qemuCaps) < 0)
+                    goto cleanup;
+                break;
+
+            case VIR_DOMAIN_FEATURE_ACPI:
+            case VIR_DOMAIN_FEATURE_APIC:
+            case VIR_DOMAIN_FEATURE_PAE:
+            case VIR_DOMAIN_FEATURE_HAP:
+            case VIR_DOMAIN_FEATURE_VIRIDIAN:
+            case VIR_DOMAIN_FEATURE_PRIVNET:
+            case VIR_DOMAIN_FEATURE_HYPERV:
+            case VIR_DOMAIN_FEATURE_KVM:
+            case VIR_DOMAIN_FEATURE_PVSPINLOCK:
+            case VIR_DOMAIN_FEATURE_CAPABILITIES:
+            case VIR_DOMAIN_FEATURE_PMU:
+            case VIR_DOMAIN_FEATURE_VMPORT:
+            case VIR_DOMAIN_FEATURE_GIC:
+            case VIR_DOMAIN_FEATURE_SMM:
+            case VIR_DOMAIN_FEATURE_IOAPIC:
+            case VIR_DOMAIN_FEATURE_HPT:
+            case VIR_DOMAIN_FEATURE_VMCOREINFO:
+                break;
+
+            case VIR_DOMAIN_FEATURE_LAST:
+            default:
+                break;
+            }
+        }
+
         if (cpu && cpu->model &&
             cpu->mode == VIR_CPU_MODE_HOST_MODEL &&
             qemuDomainIsPSeries(def) &&
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b55013de6a..35d84adae3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3377,6 +3377,19 @@ qemuDomainDefValidateFeatures(const virDomainDef *def)
             }
             break;
 
+        case VIR_DOMAIN_FEATURE_HTM:
+            if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT &&
+                !qemuDomainIsPSeries(def)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("The '%s' feature is not supported for "
+                                 "architecture '%s' or machine type '%s'"),
+                               featureName,
+                               virArchToString(def->os.arch),
+                               def->os.machine);
+                return -1;
+            }
+            break;
+
         case VIR_DOMAIN_FEATURE_ACPI:
         case VIR_DOMAIN_FEATURE_APIC:
         case VIR_DOMAIN_FEATURE_PAE:
diff --git a/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml b/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
index 5a6bb02d5b..76cbf0737f 100644
--- a/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
+++ b/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
@@ -9,6 +9,7 @@
   <features>
     <!-- pSeries features can't be enabled for non-pSeries guests -->
     <hpt resizing='enabled'/>
+    <htm state='on'/>
   </features>
   <devices>
     <emulator>/usr/bin/qemu-system-x86_64</emulator>
diff --git a/tests/qemuxml2argvdata/pseries-features.args b/tests/qemuxml2argvdata/pseries-features.args
index 8cdb329651..0517ca8237 100644
--- a/tests/qemuxml2argvdata/pseries-features.args
+++ b/tests/qemuxml2argvdata/pseries-features.args
@@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
 /usr/bin/qemu-system-ppc64 \
 -name guest \
 -S \
--machine pseries,accel=tcg,resize-hpt=required \
+-machine pseries,accel=tcg,resize-hpt=required,cap-htm=on \
 -m 512 \
 -smp 1,sockets=1,cores=1,threads=1 \
 -uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \
diff --git a/tests/qemuxml2argvdata/pseries-features.xml b/tests/qemuxml2argvdata/pseries-features.xml
index 5dd0dbd0be..a0e98db8b2 100644
--- a/tests/qemuxml2argvdata/pseries-features.xml
+++ b/tests/qemuxml2argvdata/pseries-features.xml
@@ -10,6 +10,7 @@
   </os>
   <features>
     <hpt resizing='required'/>
+    <htm state='on'/>
   </features>
   <clock offset='utc'/>
   <on_poweroff>destroy</on_poweroff>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ca53912ebe..03f8c429e0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1898,6 +1898,7 @@ mymain(void)
 
     DO_TEST("pseries-features",
             QEMU_CAPS_MACHINE_OPT,
+            QEMU_CAPS_MACHINE_PSERIES_CAP_HTM,
             QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
     DO_TEST_FAILURE("pseries-features",
                     QEMU_CAPS_MACHINE_OPT);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index a363b55446..b9a8bd6f14 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -765,6 +765,7 @@ mymain(void)
 
     DO_TEST("pseries-features",
             QEMU_CAPS_MACHINE_OPT,
+            QEMU_CAPS_MACHINE_PSERIES_CAP_HTM,
             QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
 
     DO_TEST("pseries-serial-native",
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/8] qemu: Implement the HTM pSeries feature
Posted by John Ferlan 7 years, 1 month ago

On 03/09/2018 10:07 AM, Andrea Bolognani wrote:
> This is the first in a list of pSeries-specific optional features
> that have recently been introduced in QEMU. Along with the feature
> proper, some generic code that will make it easier to implement
> subsequent features is introduced as well.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  docs/formatdomain.html.in                          |   7 +
>  docs/schemas/domaincommon.rng                      |   5 +
>  src/conf/domain_conf.c                             |  19 +++
>  src/conf/domain_conf.h                             |   1 +
>  src/qemu/qemu_command.c                            | 149 +++++++++++++++++++++
>  src/qemu/qemu_domain.c                             |  13 ++
>  .../pseries-features-invalid-machine.xml           |   1 +
>  tests/qemuxml2argvdata/pseries-features.args       |   2 +-
>  tests/qemuxml2argvdata/pseries-features.xml        |   1 +
>  tests/qemuxml2argvtest.c                           |   1 +
>  tests/qemuxml2xmltest.c                            |   1 +
>  11 files changed, 199 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6fd2189cd2..51400afa49 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2057,6 +2057,13 @@
>            the attribute is not defined, the hypervisor default will be used.
>            <span class="since">Since 3.10.0</span> (QEMU/KVM only)
>        </dd>
> +      <dt><code>htm</code></dt>
> +      <dd>Configure HTM (Hardware Transational Memory) availability for
> +          pSeries guests. Possible values for the <code>state</code> attribute
> +          are <code>on</code> and <code>off</code>. If the attribute is not
> +          defined, the hypervisor default will be used.
> +          <span class="since">Since 4.2.0</span> (QEMU/KVM only)
> +      </dd>
>        <dt><code>vmcoreinfo</code></dt>
>        <dd>Enable QEMU vmcoreinfo device to let the guest kernel save debug
>            details. <span class="since">Since 3.10.0</span> (QEMU only)
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 8165e699d6..b4143f5bc3 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4797,6 +4797,11 @@
>            <optional>
>              <ref name="hpt"/>
>            </optional>
> +          <optional>
> +            <element name="htm">
> +              <ref name="featurestate"/>
> +            </element>
> +          </optional>
>            <optional>
>              <ref name="vmcoreinfo"/>
>            </optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 70b19311b4..98897d3a63 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -152,6 +152,7 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
>                "ioapic",
>                "hpt",
>                "vmcoreinfo",
> +              "htm",
>  );
>  
>  VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
> @@ -19297,6 +19298,22 @@ virDomainDefParseXML(xmlDocPtr xml,
>              }
>              break;
>  
> +        case VIR_DOMAIN_FEATURE_HTM:
> +            if (!(tmp = virXMLPropString(nodes[i], "state"))) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("missing state attribute '%s' of feature '%s'"),
> +                               tmp, virDomainFeatureTypeToString(val));
> +                goto error;
> +            }
> +            if ((def->features[val] = virTristateSwitchTypeFromString(tmp)) < 0) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("unknown state attribute '%s' of feature '%s'"),
> +                               tmp, virDomainFeatureTypeToString(val));
> +                goto error;
> +            }
> +            VIR_FREE(tmp);
> +            break;
> +
>          /* coverity[dead_error_begin] */
>          case VIR_DOMAIN_FEATURE_LAST:
>              break;
> @@ -21384,6 +21401,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
>          case VIR_DOMAIN_FEATURE_VMPORT:
>          case VIR_DOMAIN_FEATURE_SMM:
>          case VIR_DOMAIN_FEATURE_VMCOREINFO:
> +        case VIR_DOMAIN_FEATURE_HTM:
>              if (src->features[i] != dst->features[i]) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                                 _("State of feature '%s' differs: "
> @@ -26845,6 +26863,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>              case VIR_DOMAIN_FEATURE_PVSPINLOCK:
>              case VIR_DOMAIN_FEATURE_VMPORT:
>              case VIR_DOMAIN_FEATURE_SMM:
> +            case VIR_DOMAIN_FEATURE_HTM:
>                  switch ((virTristateSwitch) def->features[i]) {
>                  case VIR_TRISTATE_SWITCH_LAST:
>                  case VIR_TRISTATE_SWITCH_ABSENT:
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 5859c8f4b1..b87a9d9de2 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1746,6 +1746,7 @@ typedef enum {
>      VIR_DOMAIN_FEATURE_IOAPIC,
>      VIR_DOMAIN_FEATURE_HPT,
>      VIR_DOMAIN_FEATURE_VMCOREINFO,
> +    VIR_DOMAIN_FEATURE_HTM,
>  
>      VIR_DOMAIN_FEATURE_LAST
>  } virDomainFeature;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index fa0aa5d5c3..d58211c4c6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7117,6 +7117,113 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
>      return 0;
>  }
>  

This looks particularly useful for other commands, but...

> +static int
> +virDomainFeatureToQEMUCaps(int feature)
> +{
> +    switch ((virDomainFeature) feature) {
> +    case VIR_DOMAIN_FEATURE_HTM:
> +        return QEMU_CAPS_MACHINE_PSERIES_CAP_HTM;
> +    case VIR_DOMAIN_FEATURE_ACPI:
> +    case VIR_DOMAIN_FEATURE_APIC:
> +    case VIR_DOMAIN_FEATURE_PAE:
> +    case VIR_DOMAIN_FEATURE_HAP:
> +    case VIR_DOMAIN_FEATURE_VIRIDIAN:
> +    case VIR_DOMAIN_FEATURE_PRIVNET:
> +    case VIR_DOMAIN_FEATURE_HYPERV:
> +    case VIR_DOMAIN_FEATURE_KVM:
> +    case VIR_DOMAIN_FEATURE_PVSPINLOCK:
> +    case VIR_DOMAIN_FEATURE_CAPABILITIES:
> +    case VIR_DOMAIN_FEATURE_PMU:
> +    case VIR_DOMAIN_FEATURE_VMPORT:
> +    case VIR_DOMAIN_FEATURE_GIC:
> +    case VIR_DOMAIN_FEATURE_SMM:
> +    case VIR_DOMAIN_FEATURE_IOAPIC:
> +    case VIR_DOMAIN_FEATURE_HPT:
> +    case VIR_DOMAIN_FEATURE_VMCOREINFO:
> +    case VIR_DOMAIN_FEATURE_LAST:
> +    default:
> +        return -1;
> +    }
> +
> +    return -1;
> +}
> +
> +static const char*
> +virDomainFeatureToMachineOption(int feature)
> +{
> +    switch ((virDomainFeature) feature) {
> +    case VIR_DOMAIN_FEATURE_HTM:
> +        return "cap-htm";
> +    case VIR_DOMAIN_FEATURE_ACPI:
> +    case VIR_DOMAIN_FEATURE_APIC:
> +    case VIR_DOMAIN_FEATURE_PAE:
> +    case VIR_DOMAIN_FEATURE_HAP:
> +    case VIR_DOMAIN_FEATURE_VIRIDIAN:
> +    case VIR_DOMAIN_FEATURE_PRIVNET:
> +    case VIR_DOMAIN_FEATURE_HYPERV:
> +    case VIR_DOMAIN_FEATURE_KVM:
> +    case VIR_DOMAIN_FEATURE_PVSPINLOCK:
> +    case VIR_DOMAIN_FEATURE_CAPABILITIES:
> +    case VIR_DOMAIN_FEATURE_PMU:
> +    case VIR_DOMAIN_FEATURE_VMPORT:
> +    case VIR_DOMAIN_FEATURE_GIC:
> +    case VIR_DOMAIN_FEATURE_SMM:
> +    case VIR_DOMAIN_FEATURE_IOAPIC:
> +    case VIR_DOMAIN_FEATURE_HPT:
> +    case VIR_DOMAIN_FEATURE_VMCOREINFO:
> +    case VIR_DOMAIN_FEATURE_LAST:
> +    default:
> +        return NULL;
> +    }
> +
> +    return NULL;
> +}
> +
> +static int
> +qemuBuildMachineCommandLineFeature(virBufferPtr buf,
> +                                   virDomainFeature feature,
> +                                   const char *value,
> +                                   virQEMUCapsPtr qemuCaps)
> +{
> +    const char *name;
> +    const char *option;
> +    int cap;
> +    int ret = -1;
> +
> +    if (!(name = virDomainFeatureTypeToString(feature))) {
> +        virReportEnumRangeError(virDomainFeature, feature);
> +        goto cleanup;
> +    }
> +
> +    if (!(option = virDomainFeatureToMachineOption(feature))) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Unknown QEMU option for '%s' feature"),
> +                       name);
> +        goto cleanup;
> +    }
> +
> +    if ((cap = virDomainFeatureToQEMUCaps(feature)) < 0) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Unknown QEMU capability for '%s' feature"),
> +                       name);
> +        goto cleanup;
> +    }
> +
> +    if (cap > 0 && !virQEMUCapsGet(qemuCaps, cap)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("'%s' feature not supported by this QEMU binary"),
> +                       name);
> +        goto cleanup;
> +    }
> +
> +    virBufferAsprintf(buf, ",%s=%s", option, value);
> +
> +    ret = 0;
> +
> + cleanup:
> +    return ret;
> +}
> +
>  static int
>  qemuBuildMachineCommandLine(virCommandPtr cmd,
>                              virQEMUDriverConfigPtr cfg,
> @@ -7343,6 +7450,48 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>              virBufferAsprintf(&buf, ",resize-hpt=%s", str);
>          }
>  
> +        for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
> +            const char *value;
> +
> +            switch ((virDomainFeature) i) {
> +            case VIR_DOMAIN_FEATURE_HTM:
> +                if (def->features[i] == VIR_TRISTATE_SWITCH_ABSENT)
> +                    break;
> +
> +                if (!(value = virTristateSwitchTypeToString(def->features[i]))) {
> +                    virReportEnumRangeError(virTristateSwitch, def->features[i]);
> +                    goto cleanup;
> +                }
> +
> +                if (qemuBuildMachineCommandLineFeature(&buf, i, value, qemuCaps) < 0)
> +                    goto cleanup;
> +                break;
> +
> +            case VIR_DOMAIN_FEATURE_ACPI:
> +            case VIR_DOMAIN_FEATURE_APIC:
> +            case VIR_DOMAIN_FEATURE_PAE:
> +            case VIR_DOMAIN_FEATURE_HAP:
> +            case VIR_DOMAIN_FEATURE_VIRIDIAN:
> +            case VIR_DOMAIN_FEATURE_PRIVNET:
> +            case VIR_DOMAIN_FEATURE_HYPERV:
> +            case VIR_DOMAIN_FEATURE_KVM:
> +            case VIR_DOMAIN_FEATURE_PVSPINLOCK:
> +            case VIR_DOMAIN_FEATURE_CAPABILITIES:
> +            case VIR_DOMAIN_FEATURE_PMU:
> +            case VIR_DOMAIN_FEATURE_VMPORT:
> +            case VIR_DOMAIN_FEATURE_GIC:
> +            case VIR_DOMAIN_FEATURE_SMM:
> +            case VIR_DOMAIN_FEATURE_IOAPIC:
> +            case VIR_DOMAIN_FEATURE_HPT:
> +            case VIR_DOMAIN_FEATURE_VMCOREINFO:
> +                break;
> +
> +            case VIR_DOMAIN_FEATURE_LAST:
> +            default:
> +                break;
> +            }
> +        }
> +

It's only generated/built for one - seems like a lot of work for little
gain unless of course there's a plan to add in all the others.  Without
adding in others, one is left to wonder the "best" way to add things in
the future.

John

>          if (cpu && cpu->model &&
>              cpu->mode == VIR_CPU_MODE_HOST_MODEL &&
>              qemuDomainIsPSeries(def) &&
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index b55013de6a..35d84adae3 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3377,6 +3377,19 @@ qemuDomainDefValidateFeatures(const virDomainDef *def)
>              }
>              break;
>  
> +        case VIR_DOMAIN_FEATURE_HTM:
> +            if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT &&
> +                !qemuDomainIsPSeries(def)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("The '%s' feature is not supported for "
> +                                 "architecture '%s' or machine type '%s'"),
> +                               featureName,
> +                               virArchToString(def->os.arch),
> +                               def->os.machine);
> +                return -1;
> +            }
> +            break;
> +
>          case VIR_DOMAIN_FEATURE_ACPI:
>          case VIR_DOMAIN_FEATURE_APIC:
>          case VIR_DOMAIN_FEATURE_PAE:
> diff --git a/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml b/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
> index 5a6bb02d5b..76cbf0737f 100644
> --- a/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
> +++ b/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
> @@ -9,6 +9,7 @@
>    <features>
>      <!-- pSeries features can't be enabled for non-pSeries guests -->
>      <hpt resizing='enabled'/>
> +    <htm state='on'/>
>    </features>
>    <devices>
>      <emulator>/usr/bin/qemu-system-x86_64</emulator>
> diff --git a/tests/qemuxml2argvdata/pseries-features.args b/tests/qemuxml2argvdata/pseries-features.args
> index 8cdb329651..0517ca8237 100644
> --- a/tests/qemuxml2argvdata/pseries-features.args
> +++ b/tests/qemuxml2argvdata/pseries-features.args
> @@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
>  /usr/bin/qemu-system-ppc64 \
>  -name guest \
>  -S \
> --machine pseries,accel=tcg,resize-hpt=required \
> +-machine pseries,accel=tcg,resize-hpt=required,cap-htm=on \
>  -m 512 \
>  -smp 1,sockets=1,cores=1,threads=1 \
>  -uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \
> diff --git a/tests/qemuxml2argvdata/pseries-features.xml b/tests/qemuxml2argvdata/pseries-features.xml
> index 5dd0dbd0be..a0e98db8b2 100644
> --- a/tests/qemuxml2argvdata/pseries-features.xml
> +++ b/tests/qemuxml2argvdata/pseries-features.xml
> @@ -10,6 +10,7 @@
>    </os>
>    <features>
>      <hpt resizing='required'/>
> +    <htm state='on'/>
>    </features>
>    <clock offset='utc'/>
>    <on_poweroff>destroy</on_poweroff>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index ca53912ebe..03f8c429e0 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1898,6 +1898,7 @@ mymain(void)
>  
>      DO_TEST("pseries-features",
>              QEMU_CAPS_MACHINE_OPT,
> +            QEMU_CAPS_MACHINE_PSERIES_CAP_HTM,
>              QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
>      DO_TEST_FAILURE("pseries-features",
>                      QEMU_CAPS_MACHINE_OPT);
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index a363b55446..b9a8bd6f14 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -765,6 +765,7 @@ mymain(void)
>  
>      DO_TEST("pseries-features",
>              QEMU_CAPS_MACHINE_OPT,
> +            QEMU_CAPS_MACHINE_PSERIES_CAP_HTM,
>              QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
>  
>      DO_TEST("pseries-serial-native",
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/8] qemu: Implement the HTM pSeries feature
Posted by Andrea Bolognani 7 years, 1 month ago
On Fri, 2018-03-16 at 11:33 -0400, John Ferlan wrote:
> This looks particularly useful for other commands, but...
> 
[...]
> 
> It's only generated/built for one - seems like a lot of work for little
> gain unless of course there's a plan to add in all the others.  Without
> adding in others, one is left to wonder the "best" way to add things in
> the future.

The RFC versions of the series added three more features, which made
very good use of the generic code introduced here. I've later decided
to skip those features, at least initially, because QEMU added
mitigated machine type variants that cover 99% of what people need.
I'm still unconvinced there is a need to have more fine-grained knobs
there, so those three features are probably not going to be
introduced any time soon.

Moreover, between the time I posted this series and today, I've
learned that work on a POWER9-compatible version of HTM is
progressing much faster than initially anticipated, so this last
feature too might end up being unnecessary in the medium run. I'm
still not sure about this last part, but there is enough uncertainty
there that I would probably not push this before 4.2.0 is out even
if it were fully R-b'd.

Finally, it was always my intention, assuming the approach taken here
had been considered good, to convert existing features to use it: I
guess at this point the tables have turned and I should *start* with
that instead :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list