[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [RFC for-2.12 7/8] spapr: Handle VMX/VSX presence as an s
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [RFC for-2.12 7/8] spapr: Handle VMX/VSX presence as an spapr capability flag |
Date: |
Tue, 12 Dec 2017 13:54:36 +0100 |
On Mon, 11 Dec 2017 18:08:07 +1100
David Gibson <address@hidden> wrote:
> We currently have some conditionals in the spapr device tree code to decide
> whether or not to advertise the availability of the VMX (aka Altivec) and
> VSX vector extensions to the guest, based on whether the guest cpu has
> those features.
>
> This can lead to confusion and subtle failures on migration, since it makes
> a guest visible change based only on host capabilities. We now have a
> better mechanism for this, in spapr capabilities flags, which explicitly
> depend on user options rather than host capabilities.
>
> Rework the advertisement of VSX and VMX based on a new VSX capability. We
> no longer bother with a conditional for VMX support, because every CPU
> that's ever been supported by the pseries machine type supports VMX.
>
> NOTE: Some userspace distributions (e.g. RHEL7.4) already rely on
> availability of VSX in libc, so using cap-vsx=off may lead to a fatal
> SIGILL in init.
>
> Signed-off-by: David Gibson <address@hidden>
> ---
Just one nit, see below, but anyway:
Reviewed-by: Greg Kurz <address@hidden>
> hw/ppc/spapr.c | 18 ++++++++++--------
> hw/ppc/spapr_caps.c | 24 ++++++++++++++++++++++++
> include/hw/ppc/spapr.h | 3 +++
> 3 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e206825ed9..6b37511e8f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -556,14 +556,16 @@ static void spapr_populate_cpu_dt(CPUState *cs, void
> *fdt, int offset,
> segs, sizeof(segs))));
> }
>
> - /* Advertise VMX/VSX (vector extensions) if available
> - * 0 / no property == no vector extensions
> + /* Advertise VSX (vector extensions) if available
> * 1 == VMX / Altivec available
> - * 2 == VSX available */
> - if (env->insns_flags & PPC_ALTIVEC) {
> - uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
> -
> - _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx)));
> + * 2 == VSX available
> + *
> + * Only CPUs for which we create core types in spapr_cpu_core.c
> + * are possible, and all of those have VMX */
> + if (spapr_has_cap(spapr, SPAPR_CAP_VSX)) {
> + _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", 2)));
Maybe a good opportunity to drop the extra parens since the _FDT() macro
already adds them around its argument.
#define _FDT(exp) \
do { \
int ret = (exp); \
> + } else {
> + _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", 1)));
> }
>
> /* Advertise DFP (Decimal Floating Point) if available
> @@ -3829,7 +3831,7 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
> */
> mc->numa_mem_align_shift = 28;
>
> - smc->default_caps = spapr_caps(SPAPR_CAP_HTM);
> + smc->default_caps = spapr_caps(SPAPR_CAP_HTM | SPAPR_CAP_VSX);
> spapr_capability_properties(smc, &error_abort);
> }
>
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 6bba04d60e..b19a9f4417 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -44,6 +44,11 @@ static sPAPRCapabilityInfo capability_table[] = {
> .description = "Allow Hardware Transactional Memory (HTM)",
> .bit = SPAPR_CAP_HTM,
> },
> + {
> + .name = "vsx",
> + .description = "Allow Vector Scalar Extensions (VSX)",
> + .bit = SPAPR_CAP_VSX,
> + },
> };
>
> static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
> CPUState *cs)
> @@ -59,6 +64,11 @@ static sPAPRCapabilities
> default_caps_with_cpu(sPAPRMachineState *spapr, CPUStat
> caps.mask &= ~SPAPR_CAP_HTM;
> }
>
> + if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_06,
> + 0, spapr->max_compat_pvr)) {
> + caps.mask &= ~SPAPR_CAP_VSX;
> + }
> +
> return caps;
> }
>
> @@ -149,6 +159,8 @@ const VMStateDescription vmstate_spapr_caps = {
>
> static void spapr_allow_caps(sPAPRMachineState *spapr, Error **errp)
> {
> + PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> + CPUPPCState *env = &cpu->env;
> Error *err = NULL;
>
> if (spapr_has_cap(spapr, SPAPR_CAP_HTM)) {
> @@ -164,6 +176,14 @@ static void spapr_allow_caps(sPAPRMachineState *spapr,
> Error **errp)
> }
> }
>
> + g_assert(env->insns_flags & PPC_ALTIVEC);
> + if (spapr_has_cap(spapr, SPAPR_CAP_VSX)) {
> + if (!(env->insns_flags2 & PPC2_VSX)) {
> + error_setg(&err, "VSX support not available, try cap-vsx=off");
> + goto out;
> + }
> + }
> +
> out:
> if (err) {
> error_propagate(errp, err);
> @@ -175,6 +195,10 @@ static void spapr_enforce_caps(sPAPRMachineState *spapr,
> Error **errp)
> if (!spapr_has_cap(spapr, SPAPR_CAP_HTM)) {
> /* TODO: Tell KVM not to allow HTM instructions */
> }
> +
> + if (!spapr_has_cap(spapr, SPAPR_CAP_VSX)) {
> + /* TODO: Any way to disable VSX? */
> + }
> }
>
> void spapr_caps_reset(sPAPRMachineState *spapr)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 9b80fe72ed..4c173cb40c 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -59,6 +59,9 @@ typedef enum {
> /* Hardware Transactional Memory */
> #define SPAPR_CAP_HTM 0x0000000000000001ULL
>
> +/* Vector Scalar Extensions */
> +#define SPAPR_CAP_VSX 0x0000000000000002ULL
> +
> typedef struct sPAPRCapabilities sPAPRCapabilities;
> struct sPAPRCapabilities {
> uint64_t mask;
- Re: [Qemu-ppc] [RFC for-2.12 2/8] spapr: Capabilities infrastructure, (continued)
[Qemu-ppc] [RFC for-2.12 6/8] target/ppc: Clean up probing of VMX, VSX and DFP availability on KVM, David Gibson, 2017/12/11
[Qemu-ppc] [RFC for-2.12 4/8] spapr: Validate capabilities on migration, David Gibson, 2017/12/11