Convert CPU features policy into libxl cpuid policy settings. Use new
("libxl") syntax, which allow to enable/disable specific bits, using
host CPU as a base. For this reason, accept only "host-passthrough"
mode.
Libxl do not have distinction between "force" and "required" policy
(there is only "force") and also between "forbid" and "disable" (there
is only "disable"). So, merge them appropriately. If anything, "require"
and "forbid" should be enforced outside of specific driver.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Changes since v1:
- use new ("libxl") syntax to set only bits explicitly mentioned in
domain XML
---
src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++---
src/libxl/libxl_conf.h | 1 +-
2 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index a0a53c2..0cf51a8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -291,6 +291,44 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
return 0;
}
+static const char *libxlTranslateCPUFeature(const char *virCPUFeatureName)
+{
+ /* libxl use different names for some CPUID bits */
+ if (STREQ(virCPUFeatureName, "cx16"))
+ return "cmpxchg16";
+ if (STREQ(virCPUFeatureName, "cid"))
+ return "cntxid";
+ if (STREQ(virCPUFeatureName, "ds_cpl"))
+ return "dscpl";
+ if (STREQ(virCPUFeatureName, "pclmuldq"))
+ return "pclmulqdq";
+ if (STREQ(virCPUFeatureName, "pni"))
+ return "sse3";
+ if (STREQ(virCPUFeatureName, "ht"))
+ return "htt";
+ if (STREQ(virCPUFeatureName, "pn"))
+ return "psn";
+ if (STREQ(virCPUFeatureName, "clflush"))
+ return "clfsh";
+ if (STREQ(virCPUFeatureName, "sep"))
+ return "sysenter";
+ if (STREQ(virCPUFeatureName, "cx8"))
+ return "cmpxchg8";
+ if (STREQ(virCPUFeatureName, "nodeid_msr"))
+ return "nodeid";
+ if (STREQ(virCPUFeatureName, "cr8legacy"))
+ return "altmovcr8";
+ if (STREQ(virCPUFeatureName, "lahf_lm"))
+ return "lahfsahf";
+ if (STREQ(virCPUFeatureName, "cmp_legacy"))
+ return "cmplegacy";
+ if (STREQ(virCPUFeatureName, "fxsr_opt"))
+ return "ffxsr";
+ if (STREQ(virCPUFeatureName, "pdpe1gb"))
+ return "page1gb";
+ return virCPUFeatureName;
+}
+
static int
libxlMakeDomBuildInfo(virDomainDefPtr def,
libxl_ctx *ctx,
@@ -376,10 +414,18 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
def->features[VIR_DOMAIN_FEATURE_ACPI] ==
VIR_TRISTATE_SWITCH_ON);
- if (caps &&
- def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+ if (caps && def->cpu) {
bool hasHwVirt = false;
bool svm = false, vmx = false;
+ char xlCPU[32];
+
+ if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu mode '%s'"),
+ virCPUModeTypeToString(def->cpu->mode));
+ return -1;
+ }
+
if (ARCH_IS_X86(def->os.arch)) {
vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
@@ -394,20 +440,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
case VIR_CPU_FEATURE_DISABLE:
case VIR_CPU_FEATURE_FORBID:
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=0",
+ libxlTranslateCPUFeature(
+ def->cpu->features[i].name));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature '%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
if ((vmx && STREQ(def->cpu->features[i].name, "vmx")) ||
- (svm && STREQ(def->cpu->features[i].name, "svm")))
+ (svm && STREQ(def->cpu->features[i].name, "svm")))
hasHwVirt = false;
break;
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
+ snprintf(xlCPU,
+ sizeof(xlCPU),
+ "%s=1",
+ libxlTranslateCPUFeature(
+ def->cpu->features[i].name));
+ if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported cpu feature '%s'"),
+ def->cpu->features[i].name);
+ return -1;
+ }
+ break;
case VIR_CPU_FEATURE_OPTIONAL:
case VIR_CPU_FEATURE_LAST:
break;
}
}
+ libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
}
- libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
}
if (def->nsounds > 0) {
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 264df11..8d89ccd 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -60,6 +60,7 @@
# define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
# define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target"
# define LIBXL_BOOTLOADER_PATH "pygrub"
+# define LIBXL_DEFAULT_CPUID_REG_CONFIG "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
typedef struct _libxlDriverPrivate libxlDriverPrivate;
--
git-series 0.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn 07/03/2017 09:03 PM, Marek Marczykowski-Górecki wrote:
> Convert CPU features policy into libxl cpuid policy settings. Use new
> ("libxl") syntax, which allow to enable/disable specific bits, using
> host CPU as a base. For this reason, accept only "hostf-passthrough"
> mode.
> Libxl do not have distinction between "force" and "required" policy
> (there is only "force") and also between "forbid" and "disable" (there
> is only "disable"). So, merge them appropriately. If anything, "require"
> and "forbid" should be enforced outside of specific driver.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> Changes since v1:
> - use new ("libxl") syntax to set only bits explicitly mentioned in
> domain XML
> ---
> src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++---
> src/libxl/libxl_conf.h | 1 +-
> 2 files changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index a0a53c2..0cf51a8 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -291,6 +291,44 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
> return 0;
> }
>
> +static const char *libxlTranslateCPUFeature(const char *virCPUFeatureName)
> +{
> + /* libxl use different names for some CPUID bits */
> + if (STREQ(virCPUFeatureName, "cx16"))
> + return "cmpxchg16";
> + if (STREQ(virCPUFeatureName, "cid"))
> + return "cntxid";
> + if (STREQ(virCPUFeatureName, "ds_cpl"))
> + return "dscpl";
> + if (STREQ(virCPUFeatureName, "pclmuldq"))
> + return "pclmulqdq";
> + if (STREQ(virCPUFeatureName, "pni"))
> + return "sse3";
> + if (STREQ(virCPUFeatureName, "ht"))
> + return "htt";
> + if (STREQ(virCPUFeatureName, "pn"))
> + return "psn";
> + if (STREQ(virCPUFeatureName, "clflush"))
> + return "clfsh";
> + if (STREQ(virCPUFeatureName, "sep"))
> + return "sysenter";
> + if (STREQ(virCPUFeatureName, "cx8"))
> + return "cmpxchg8";
> + if (STREQ(virCPUFeatureName, "nodeid_msr"))
> + return "nodeid";
> + if (STREQ(virCPUFeatureName, "cr8legacy"))
> + return "altmovcr8";
> + if (STREQ(virCPUFeatureName, "lahf_lm"))
> + return "lahfsahf";
> + if (STREQ(virCPUFeatureName, "cmp_legacy"))
> + return "cmplegacy";
> + if (STREQ(virCPUFeatureName, "fxsr_opt"))
> + return "ffxsr";
> + if (STREQ(virCPUFeatureName, "pdpe1gb"))
> + return "page1gb";
> + return virCPUFeatureName;
> +}
> +
I don't have an alternative, but I see a mapping table becoming stale as new CPU
features are added to libxl.
> static int
> libxlMakeDomBuildInfo(virDomainDefPtr def,
> libxl_ctx *ctx,
> @@ -376,10 +414,18 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> def->features[VIR_DOMAIN_FEATURE_ACPI] ==
> VIR_TRISTATE_SWITCH_ON);
>
> - if (caps &&
> - def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
> + if (caps && def->cpu) {
> bool hasHwVirt = false;
> bool svm = false, vmx = false;
> + char xlCPU[32];
> +
> + if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported cpu mode '%s'"),
> + virCPUModeTypeToString(def->cpu->mode));
> + return -1;
> + }
> +
Although trivial, IMO this change should be in a separate patch where it will be
easy to find.
>
> if (ARCH_IS_X86(def->os.arch)) {
> vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
> @@ -394,20 +440,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>
> case VIR_CPU_FEATURE_DISABLE:
> case VIR_CPU_FEATURE_FORBID:
> + snprintf(xlCPU,
> + sizeof(xlCPU),
> + "%s=0",
> + libxlTranslateCPUFeature(
> + def->cpu->features[i].name));
> + if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported cpu feature '%s'"),
> + def->cpu->features[i].name);
> + return -1;
> + }
> if ((vmx && STREQ(def->cpu->features[i].name, "vmx")) ||
> - (svm && STREQ(def->cpu->features[i].name, "svm")))
> + (svm && STREQ(def->cpu->features[i].name, "svm")))
Spurious change.
> hasHwVirt = false;
> break;
>
> case VIR_CPU_FEATURE_FORCE:
> case VIR_CPU_FEATURE_REQUIRE:
> + snprintf(xlCPU,
> + sizeof(xlCPU),
> + "%s=1",
> + libxlTranslateCPUFeature(
> + def->cpu->features[i].name));
> + if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported cpu feature '%s'"),
> + def->cpu->features[i].name);
> + return -1;
> + }
> + break;
> case VIR_CPU_FEATURE_OPTIONAL:
> case VIR_CPU_FEATURE_LAST:
> break;
> }
> }
> + libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
> }
> - libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
This change also seems unrelated. Perhaps it can be combined with the change
forbidding all but host_passthrough CPU model.
> }
>
> if (def->nsounds > 0) {
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 264df11..8d89ccd 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -60,6 +60,7 @@
> # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
> # define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target"
> # define LIBXL_BOOTLOADER_PATH "pygrub"
> +# define LIBXL_DEFAULT_CPUID_REG_CONFIG "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
This doesn't appear to be used anywhere in the patch.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 17, 2017 at 03:57:17PM -0600, Jim Fehlig wrote:
> On 07/03/2017 09:03 PM, Marek Marczykowski-Górecki wrote:
> > Convert CPU features policy into libxl cpuid policy settings. Use new
> > ("libxl") syntax, which allow to enable/disable specific bits, using
> > host CPU as a base. For this reason, accept only "hostf-passthrough"
> > mode.
> > Libxl do not have distinction between "force" and "required" policy
> > (there is only "force") and also between "forbid" and "disable" (there
> > is only "disable"). So, merge them appropriately. If anything, "require"
> > and "forbid" should be enforced outside of specific driver.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> > Changes since v1:
> > - use new ("libxl") syntax to set only bits explicitly mentioned in
> > domain XML
> > ---
> > src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++---
> > src/libxl/libxl_conf.h | 1 +-
> > 2 files changed, 74 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> > index a0a53c2..0cf51a8 100644
> > --- a/src/libxl/libxl_conf.c
> > +++ b/src/libxl/libxl_conf.c
> > @@ -291,6 +291,44 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
> > return 0;
> > }
> > +static const char *libxlTranslateCPUFeature(const char *virCPUFeatureName)
> > +{
> > + /* libxl use different names for some CPUID bits */
> > + if (STREQ(virCPUFeatureName, "cx16"))
> > + return "cmpxchg16";
> > + if (STREQ(virCPUFeatureName, "cid"))
> > + return "cntxid";
> > + if (STREQ(virCPUFeatureName, "ds_cpl"))
> > + return "dscpl";
> > + if (STREQ(virCPUFeatureName, "pclmuldq"))
> > + return "pclmulqdq";
> > + if (STREQ(virCPUFeatureName, "pni"))
> > + return "sse3";
> > + if (STREQ(virCPUFeatureName, "ht"))
> > + return "htt";
> > + if (STREQ(virCPUFeatureName, "pn"))
> > + return "psn";
> > + if (STREQ(virCPUFeatureName, "clflush"))
> > + return "clfsh";
> > + if (STREQ(virCPUFeatureName, "sep"))
> > + return "sysenter";
> > + if (STREQ(virCPUFeatureName, "cx8"))
> > + return "cmpxchg8";
> > + if (STREQ(virCPUFeatureName, "nodeid_msr"))
> > + return "nodeid";
> > + if (STREQ(virCPUFeatureName, "cr8legacy"))
> > + return "altmovcr8";
> > + if (STREQ(virCPUFeatureName, "lahf_lm"))
> > + return "lahfsahf";
> > + if (STREQ(virCPUFeatureName, "cmp_legacy"))
> > + return "cmplegacy";
> > + if (STREQ(virCPUFeatureName, "fxsr_opt"))
> > + return "ffxsr";
> > + if (STREQ(virCPUFeatureName, "pdpe1gb"))
> > + return "page1gb";
> > + return virCPUFeatureName;
> > +}
> > +
>
> I don't have an alternative, but I see a mapping table becoming stale as new
> CPU features are added to libxl.
This is one reason why v1 used name->bit mapping of libvirt. But it
required "old" (xend) config syntax and had a side effect of setting all
bits of specified CPU model (yes, it required CPU model specified in
XML).
> > static int
> > libxlMakeDomBuildInfo(virDomainDefPtr def,
> > libxl_ctx *ctx,
> > @@ -376,10 +414,18 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> > def->features[VIR_DOMAIN_FEATURE_ACPI] ==
> > VIR_TRISTATE_SWITCH_ON);
> > - if (caps &&
> > - def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
> > + if (caps && def->cpu) {
> > bool hasHwVirt = false;
> > bool svm = false, vmx = false;
> > + char xlCPU[32];
> > +
> > + if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("unsupported cpu mode '%s'"),
> > + virCPUModeTypeToString(def->cpu->mode));
> > + return -1;
> > + }
> > +
>
> Although trivial, IMO this change should be in a separate patch where it
> will be easy to find.
Ok.
> > if (ARCH_IS_X86(def->os.arch)) {
> > vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
> > @@ -394,20 +440,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> > case VIR_CPU_FEATURE_DISABLE:
> > case VIR_CPU_FEATURE_FORBID:
> > + snprintf(xlCPU,
> > + sizeof(xlCPU),
> > + "%s=0",
> > + libxlTranslateCPUFeature(
> > + def->cpu->features[i].name));
> > + if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("unsupported cpu feature '%s'"),
> > + def->cpu->features[i].name);
> > + return -1;
> > + }
> > if ((vmx && STREQ(def->cpu->features[i].name, "vmx")) ||
> > - (svm && STREQ(def->cpu->features[i].name, "svm")))
> > + (svm && STREQ(def->cpu->features[i].name, "svm")))
>
> Spurious change.
>
> > hasHwVirt = false;
> > break;
> > case VIR_CPU_FEATURE_FORCE:
> > case VIR_CPU_FEATURE_REQUIRE:
> > + snprintf(xlCPU,
> > + sizeof(xlCPU),
> > + "%s=1",
> > + libxlTranslateCPUFeature(
> > + def->cpu->features[i].name));
> > + if (libxl_cpuid_parse_config(&b_info->cpuid, xlCPU)) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("unsupported cpu feature '%s'"),
> > + def->cpu->features[i].name);
> > + return -1;
> > + }
> > + break;
> > case VIR_CPU_FEATURE_OPTIONAL:
> > case VIR_CPU_FEATURE_LAST:
> > break;
> > }
> > }
> > + libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
> > }
> > - libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
>
> This change also seems unrelated. Perhaps it can be combined with the change
> forbidding all but host_passthrough CPU model.
>
> > }
> > if (def->nsounds > 0) {
> > diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> > index 264df11..8d89ccd 100644
> > --- a/src/libxl/libxl_conf.h
> > +++ b/src/libxl/libxl_conf.h
> > @@ -60,6 +60,7 @@
> > # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
> > # define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target"
> > # define LIBXL_BOOTLOADER_PATH "pygrub"
> > +# define LIBXL_DEFAULT_CPUID_REG_CONFIG "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
>
> This doesn't appear to be used anywhere in the patch.
Ah, indeed, leftover of v1.
--
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?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/17/2017 03:57 PM, Jim Fehlig wrote:
> On 07/03/2017 09:03 PM, Marek Marczykowski-Górecki wrote:
>> Convert CPU features policy into libxl cpuid policy settings. Use new
>> ("libxl") syntax, which allow to enable/disable specific bits, using
>> host CPU as a base. For this reason, accept only "hostf-passthrough"
>> mode.
>> Libxl do not have distinction between "force" and "required" policy
>> (there is only "force") and also between "forbid" and "disable" (there
>> is only "disable"). So, merge them appropriately. If anything, "require"
>> and "forbid" should be enforced outside of specific driver.
>>
>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>> ---
>> Changes since v1:
>> - use new ("libxl") syntax to set only bits explicitly mentioned in
>> domain XML
>> ---
>> src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++---
>> src/libxl/libxl_conf.h | 1 +-
>> 2 files changed, 74 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index a0a53c2..0cf51a8 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -291,6 +291,44 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
>> return 0;
>> }
>> +static const char *libxlTranslateCPUFeature(const char *virCPUFeatureName)
>> +{
>> + /* libxl use different names for some CPUID bits */
>> + if (STREQ(virCPUFeatureName, "cx16"))
>> + return "cmpxchg16";
>> + if (STREQ(virCPUFeatureName, "cid"))
>> + return "cntxid";
>> + if (STREQ(virCPUFeatureName, "ds_cpl"))
>> + return "dscpl";
>> + if (STREQ(virCPUFeatureName, "pclmuldq"))
>> + return "pclmulqdq";
>> + if (STREQ(virCPUFeatureName, "pni"))
>> + return "sse3";
>> + if (STREQ(virCPUFeatureName, "ht"))
>> + return "htt";
>> + if (STREQ(virCPUFeatureName, "pn"))
>> + return "psn";
>> + if (STREQ(virCPUFeatureName, "clflush"))
>> + return "clfsh";
>> + if (STREQ(virCPUFeatureName, "sep"))
>> + return "sysenter";
>> + if (STREQ(virCPUFeatureName, "cx8"))
>> + return "cmpxchg8";
>> + if (STREQ(virCPUFeatureName, "nodeid_msr"))
>> + return "nodeid";
>> + if (STREQ(virCPUFeatureName, "cr8legacy"))
>> + return "altmovcr8";
>> + if (STREQ(virCPUFeatureName, "lahf_lm"))
>> + return "lahfsahf";
>> + if (STREQ(virCPUFeatureName, "cmp_legacy"))
>> + return "cmplegacy";
>> + if (STREQ(virCPUFeatureName, "fxsr_opt"))
>> + return "ffxsr";
>> + if (STREQ(virCPUFeatureName, "pdpe1gb"))
>> + return "page1gb";
>> + return virCPUFeatureName;
>> +}
>> +
>
> I don't have an alternative, but I see a mapping table becoming stale as new CPU
> features are added to libxl.
>
>> static int
>> libxlMakeDomBuildInfo(virDomainDefPtr def,
>> libxl_ctx *ctx,
>> @@ -376,10 +414,18 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>> def->features[VIR_DOMAIN_FEATURE_ACPI] ==
>> VIR_TRISTATE_SWITCH_ON);
>> - if (caps &&
>> - def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
>> + if (caps && def->cpu) {
>> bool hasHwVirt = false;
>> bool svm = false, vmx = false;
>> + char xlCPU[32];
>> +
>> + if (def->cpu->mode != (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unsupported cpu mode '%s'"),
>> + virCPUModeTypeToString(def->cpu->mode));
>> + return -1;
>> + }
>> +
>
> Although trivial, IMO this change should be in a separate patch where it will be
> easy to find.
>
>> if (ARCH_IS_X86(def->os.arch)) {
>> vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu,
>> "vmx");
>> @@ -394,20 +440,43 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>> case VIR_CPU_FEATURE_DISABLE:
>> case VIR_CPU_FEATURE_FORBID:
>> + snprintf(xlCPU,
>> + sizeof(xlCPU),
>> + "%s=0",
>> + libxlTranslateCPUFeature(
>> + def->cpu->features[i].name));
>> + if (libxl_cpuid_parse_config(&b_info->cpuid,
>> xlCPU)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unsupported cpu feature '%s'"),
>> + def->cpu->features[i].name);
>> + return -1;
>> + }
>> if ((vmx && STREQ(def->cpu->features[i].name,
>> "vmx")) ||
>> - (svm && STREQ(def->cpu->features[i].name,
>> "svm")))
>> + (svm && STREQ(def->cpu->features[i].name,
>> "svm")))
>
> Spurious change.
>
>> hasHwVirt = false;
>> break;
>> case VIR_CPU_FEATURE_FORCE:
>> case VIR_CPU_FEATURE_REQUIRE:
>> + snprintf(xlCPU,
>> + sizeof(xlCPU),
>> + "%s=1",
>> + libxlTranslateCPUFeature(
>> + def->cpu->features[i].name));
>> + if (libxl_cpuid_parse_config(&b_info->cpuid,
>> xlCPU)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("unsupported cpu feature '%s'"),
>> + def->cpu->features[i].name);
>> + return -1;
>> + }
>> + break;
>> case VIR_CPU_FEATURE_OPTIONAL:
>> case VIR_CPU_FEATURE_LAST:
>> break;
>> }
>> }
>> + libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
>> }
>> - libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
>
> This change also seems unrelated. Perhaps it can be combined with the change
> forbidding all but host_passthrough CPU model.
>
>> }
>> if (def->nsounds > 0) {
>> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
>> index 264df11..8d89ccd 100644
>> --- a/src/libxl/libxl_conf.h
>> +++ b/src/libxl/libxl_conf.h
>> @@ -60,6 +60,7 @@
>> # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
>> # define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target"
>> # define LIBXL_BOOTLOADER_PATH "pygrub"
>> +# define LIBXL_DEFAULT_CPUID_REG_CONFIG "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
>
> This doesn't appear to be used anywhere in the patch.
Sorry, I forgot to mention that this series should also include a patch for
domXML <-> xl.cfg conversions, plus a xlconfigtest :-). IMO, the libxl CPUID
format is sufficient for now.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.