Re: [PATCH RFC 2/5] libxl: describe host cpu features based on hwcaps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2016-07-08 at 17:16 +0100, Joao Martins wrote:
> Parse libxl_hwcaps accounting for versions for Xen 4.4.
> libxl_hwcaps is a set of cpuid leaves output that is described in [0] or
> [1] in Xen 4.7. This is a collection of CPUID leaves that are we version
> in libvirt whenever leaves are reordered or added. Thus we keep the
> common ones in one struct and others for each version. Since
> libxl_hwcaps doesn't appear to have a stable format across all supported
> versions thus we need to keep track of changes as a compromise whenever
> these extracting host featuresets are exported by libxl. Also we don't
> fail in initializing the driver in case parsing of hwcaps failed.
> 
> xen/include/asm-x86/cpufeature.h
> include/public/arch-x86/cpufeatureset.h
> 
> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
> ---
> Probably to make it easier to extend hwcaps handling in the future may
> be it's better to introduce an enum for the version and restructure this
> in another way?
> ---
>  src/libxl/libxl_capabilities.c | 106 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 103 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
> index f71fd3b..c0857e5 100644
> --- a/src/libxl/libxl_capabilities.c
> +++ b/src/libxl/libxl_capabilities.c
> @@ -36,6 +36,8 @@
>  #include "domain_capabilities.h"
>  #include "vircommand.h"
>  #include "libxl_capabilities.h"
> +#include "cpu/cpu_x86.h"
> +#include "cpu/cpu_x86_data.h"
>  
>  
>  #define VIR_FROM_THIS VIR_FROM_LIBXL
> @@ -43,7 +45,8 @@
>  VIR_LOG_INIT("libxl.libxl_capabilities");
>  
>  /* see xen-unstable.hg/xen/include/asm-x86/cpufeature.h */
> -#define LIBXL_X86_FEATURE_PAE_MASK 0x40
> +#define LIBXL_X86_FEATURE_PAE_MASK (1 << 6)
> +#define LIBXL_X86_FEATURE_LM_MASK  (1 << 29)

+1 for the notation change: makes it much easier to associate to the
original bits definitions.

>  
>  struct guest_arch {
> @@ -57,17 +60,94 @@ struct guest_arch {
>  
>  #define XEN_CAP_REGEX "(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(aarch64|armv7l|x86_32|x86_64|ia64|powerpc64)(p|be)?"
>  
> +static int
> +libxlCapsAddCPUID(virCPUx86Data *data, virCPUx86CPUID *cpuid, ssize_t ncaps)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < ncaps; i++) {
> +        virCPUx86CPUID *c = &cpuid[i];
> +
> +        if (virCPUx86DataAddCPUID(data, c) < 0) {
> +            VIR_DEBUG("Failed to add CPUID(%x,%x)", c->eax_in, c->ecx_in);
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +/*
> + * The words represented in physinfo.hw_cap are host CPUID (sub) leafs.
> + * Position of these hasn't changed much up until Xen 4.7 with a rework
> + * on how CPUID is handled internally. As a side-effect it got normalized
> + * and also added more feature words. Although cannot be relied upon as
> + * stable interface, and hence we version changes in position of the features
> + * across all supported versions of the libxl driver until libxl exposes a
> + * stable representation of these capabilities. Fortunately not a lot of
> + * variation happened so it's still possible to keep track of these leafs
> + * to describe host CPU in libvirt capabilities. v0 is for Xen 4.4 up to 4.6,
> + * while v1 is meant for Xen 4.7, as depicted in the table below:
> + *
> + *              | v0 (Xen 4.4 - 4.6) |   v1 (Xen >= 4.7)    |
> + *              ---------------------------------------------
> + *       word 0 | CPUID.00000001.EDX | CPUID.00000001.EDX   |
> + *       word 1 | CPUID.80000001.EDX | CPUID.00000001.ECX   |
> + *       word 2 | CPUID.80860001     | CPUID.80000001.EDX   |
> + *       word 3 | -     Linux    -   | CPUID.80000001.ECX   |
> + *       word 4 | CPUID.00000001.ECX | CPUID.0000000D:1.EAX |
> + *       word 5 | CPUID.C0000001     | CPUID.00000007:0.EBX |
> + *       word 6 | CPUID.80000001.ECX | CPUID.00000007:0.ECX |
> + *       word 7 | CPUID.00000007.EBX | CPUID.80000007.EDX   |
> + *       word 8 | - Non existent -   | CPUID.80000008.EBX   |
> + *
> + */
> +static virCPUDataPtr
> +libxlCapsNodeData(virCPUDefPtr cpu, libxl_hwcap hwcap, unsigned version)
> +{
> +    ssize_t ncaps;
> +    virCPUx86Data data = VIR_CPU_X86_DATA_INIT;
> +    virCPUx86CPUID cpuid[] = {
> +        { .eax_in = 0x00000001, .edx = hwcap[0] },
> +        { .eax_in = 0x00000001, .ecx = (version > 0 ? hwcap[1] : hwcap[4]) },

Shouldn't constants be used rather than magic numbers for the versions?
How about something like LIBXL_HWCAP_V0?

Other than that, the patch looks good to me.

--
Cedric

> +        { .eax_in = 0x80000001, .edx = (version > 0 ? hwcap[2] : hwcap[1]) },
> +        { .eax_in = 0x80000001, .ecx = (version > 0 ? hwcap[3] : hwcap[6]) },
> +        { .eax_in = 0x00000007, .ebx = (version > 0 ? hwcap[5] : hwcap[7]) }
> +    };
> +    virCPUx86CPUID cpuid_ver1[] = {
> +        { .eax_in = 0x0000000D, .ecx_in = 1U, .eax = hwcap[4] },
> +        { .eax_in = 0x00000007, .ecx_in = 0U, .ecx = hwcap[6] },
> +        { .eax_in = 0x80000007, .ecx_in = 0U, .edx = hwcap[7] },
> +    };
> +
> +    ncaps = ARRAY_CARDINALITY(cpuid);
> +    if (libxlCapsAddCPUID(&data, cpuid, ncaps) < 0)
> +        return NULL;
> +
> +    ncaps = ARRAY_CARDINALITY(cpuid_ver1);
> +    if (version > 0 && libxlCapsAddCPUID(&data, cpuid_ver1, ncaps) < 0)
> +        return NULL;
> +
> +    return virCPUx86MakeData(cpu->arch, &data);
> +}
> +
>  /* hw_caps is an array of 32-bit words whose meaning is listed in
>   * xen-unstable.hg/xen/include/asm-x86/cpufeature.h.  Each feature
>   * is defined in the form X*32+Y, corresponding to the Y'th bit in
>   * the X'th 32-bit word of hw_cap.
>   */
>  static int
> -libxlCapsInitCPU(virCapsPtr caps, libxl_physinfo *phy_info)
> +libxlCapsInitCPU(virCapsPtr caps, libxl_physinfo *phy_info, unsigned version)
>  {
>      virCPUDefPtr cpu = NULL;
> +    virCPUDataPtr data = NULL;
>      int ret = -1;
>      int host_pae;
> +    int host_lm;
> +
> +    /* parse hw_cap only for x86 */
> +    if (!phy_info->hw_cap[0])
> +        return 0;
>  
>      if (VIR_ALLOC(cpu) < 0)
>          goto error;
> @@ -77,15 +157,26 @@ libxlCapsInitCPU(virCapsPtr caps, libxl_physinfo *phy_info)
>          virCapabilitiesAddHostFeature(caps, "pae") < 0)
>          goto error;
>  
> +    host_lm = phy_info->hw_cap[version > 0 ? 2 : 1] & LIBXL_X86_FEATURE_LM_MASK;
> +    if (host_lm)
> +        cpu->arch = VIR_ARCH_X86_64;
> +    else
> +        cpu->arch = VIR_ARCH_I686;
> +
>      cpu->type = VIR_CPU_TYPE_HOST;
>      cpu->cores = phy_info->cores_per_socket;
>      cpu->threads = phy_info->threads_per_core;
>      cpu->sockets = phy_info->nr_cpus / (cpu->cores * cpu->threads);
>      caps->host.cpu = cpu;
>  
> +    if (!(data = libxlCapsNodeData(cpu, phy_info->hw_cap, version))
> +        || cpuDecode(cpu, data, NULL, 0, NULL) < 0)
> +        goto cleanup;
> +
>      ret = 0;
>  
>   cleanup:
> +    cpuDataFree(data);
>  
>      return ret;
>  
> @@ -97,7 +188,9 @@ libxlCapsInitCPU(virCapsPtr caps, libxl_physinfo *phy_info)
>  static int
>  libxlCapsInitHost(libxl_ctx *ctx, virCapsPtr caps)
>  {
> +    const libxl_version_info *ver_info;
>      libxl_physinfo phy_info;
> +    unsigned version;
>  
>      if (libxl_get_physinfo(ctx, &phy_info) != 0) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -105,8 +198,15 @@ libxlCapsInitHost(libxl_ctx *ctx, virCapsPtr caps)
>          return -1;
>      }
>  
> -    if (libxlCapsInitCPU(caps, &phy_info) < 0)
> +    if ((ver_info = libxl_get_version_info(ctx)) == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Failed to get version info from libxenlight"));
>          return -1;
> +    }
> +
> +    version = (ver_info->xen_version_minor >= 7);
> +    if (libxlCapsInitCPU(caps, &phy_info, version) < 0)
> +        VIR_WARN("Failed to initialize host cpu data");
>  
>      if (virCapabilitiesSetNetPrefix(caps, LIBXL_GENERATED_PREFIX_XEN) < 0)
>          return -1;

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]