[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Me
From: |
Alexey Kardashevskiy |
Subject: |
Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability |
Date: |
Mon, 11 Dec 2017 20:20:30 +1100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 11/12/17 18:08, David Gibson wrote:
> This adds an spapr capability bit for Hardware Transactional Memory. By
> default it is enabled for all machine type versions with POWER8 and later
> CPUs.
>
> This means that "qemu -machine pseries,accel=tcg =cpu POWER8" will now
> fail to start, however that configuration was already broken in that it
> would provide a nonstandard environment to the guest, which could break
> migration.
>
> Signed-off-by: David Gibson <address@hidden>
>
> # Conflicts:
> # hw/ppc/spapr_caps.c
> ---
> hw/ppc/spapr.c | 13 +++++++------
> hw/ppc/spapr_caps.c | 41 ++++++++++++++++++++++++++++++++++++-----
> include/hw/ppc/spapr.h | 3 +++
> 3 files changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a921feeb03..0487d67894 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -253,7 +253,8 @@ static int spapr_fixup_cpu_numa_dt(void *fdt, int offset,
> PowerPCCPU *cpu)
> }
>
> /* Populate the "ibm,pa-features" property */
> -static void spapr_populate_pa_features(PowerPCCPU *cpu, void *fdt, int
> offset,
> +static void spapr_populate_pa_features(sPAPRMachineState *spapr, PowerPCCPU
> *cpu,
> + void *fdt, int offset,
> bool legacy_guest)
> {
> CPUPPCState *env = &cpu->env;
> @@ -318,7 +319,7 @@ static void spapr_populate_pa_features(PowerPCCPU *cpu,
> void *fdt, int offset,
> */
> pa_features[3] |= 0x20;
> }
> - if (kvmppc_has_cap_htm() && pa_size > 24) {
> + if (spapr_has_cap(spapr, SPAPR_CAP_HTM) && pa_size > 24) {
> pa_features[24] |= 0x80; /* Transactional memory support */
> }
> if (legacy_guest && pa_size > 40) {
> @@ -384,8 +385,8 @@ static int spapr_fixup_cpu_dt(void *fdt,
> sPAPRMachineState *spapr)
> return ret;
> }
>
> - spapr_populate_pa_features(cpu, fdt, offset,
> - spapr->cas_legacy_guest_workaround);
> + spapr_populate_pa_features(spapr, cpu, fdt, offset,
> + spapr->cas_legacy_guest_workaround);
> }
> return ret;
> }
> @@ -579,7 +580,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void
> *fdt, int offset,
> page_sizes_prop, page_sizes_prop_size)));
> }
>
> - spapr_populate_pa_features(cpu, fdt, offset, false);
> + spapr_populate_pa_features(spapr, cpu, fdt, offset, false);
>
> _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
> cs->cpu_index / vcpus_per_socket)));
> @@ -3823,7 +3824,7 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
> */
> mc->numa_mem_align_shift = 28;
>
> - smc->default_caps = spapr_caps(0);
> + smc->default_caps = spapr_caps(SPAPR_CAP_HTM);
> spapr_capability_properties(smc, &error_abort);
> }
>
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index f1721cc68f..5afb4d9d5f 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -24,6 +24,10 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "qapi/visitor.h"
> +#include "sysemu/hw_accel.h"
> +#include "target/ppc/cpu.h"
> +#include "cpu-models.h"
> +#include "kvm_ppc.h"
>
> #include "hw/ppc/spapr.h"
>
> @@ -34,30 +38,57 @@ typedef struct sPAPRCapabilityInfo {
> } sPAPRCapabilityInfo;
>
> static sPAPRCapabilityInfo capability_table[] = {
> + {
> + .name = "htm",
> + .description = "Allow Hardware Transactional Memory (HTM)",
> + .bit = SPAPR_CAP_HTM,
> + },
> };
>
> static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState *spapr,
> CPUState *cs)
> {
> sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> + PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> sPAPRCapabilities caps;
>
> caps = smc->default_caps;
>
> - /* TODO: clamp according to cpu model */
> + if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_2_07,
> + 0, spapr->max_compat_pvr)) {
> + caps.mask &= ~SPAPR_CAP_HTM;
> + }
>
> return caps;
> }
>
> static void spapr_allow_caps(sPAPRMachineState *spapr, Error **errp)
> {
> - /* TODO: make sure all requested caps are allowed with the current
> - * accelerator, cpu etc. */
> + Error *err = NULL;
> +
> + if (spapr_has_cap(spapr, SPAPR_CAP_HTM)) {
> + if (tcg_enabled()) {
> + error_setg(&err,
> +"No Transactional Memory support in TCG, try cap-htm=off"
> + );
> + goto out;
> + } else if (kvm_enabled() && !kvmppc_has_cap_htm()) {
> + error_setg(&err,
> +"KVM implementation does not support Transactional Memory, try cap-htm=off"
> + );
> + }
> + }
> +
> +out:
> + if (err) {
> + error_propagate(errp, err);
> + }
> }
>
> static void spapr_enforce_caps(sPAPRMachineState *spapr, Error **errp)
> {
> - /* TODO: to the extent possible, prevent the guest from accessing
> - * features controlled by disabled caps */
> + if (!spapr_has_cap(spapr, SPAPR_CAP_HTM)) {
> + /* TODO: Tell KVM not to allow HTM instructions */
Are the capabilities expected to be enabled by default in the KVM? There is
no such "TODO" in spapr_allow_caps while it should probably be.
I would rather implement spapr_get_supported_caps() which would tell full
list of what the accelerator can do for the spapr machine; and
spapr_set_caps() which would compare supported caps what we got from
defaults and the command line and then enable requested or disable
forbidden caps, explicitly. Right now both spapr_allow_caps() and
spapr_enforce_caps() both have to check for spapr_has_cap() and (at least
what TODO suggests) for kvm_enabled() and do something to KVM in this regard.
> + }
> }
>
> void spapr_caps_reset(sPAPRMachineState *spapr)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index fffe10ee72..4215b113f4 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -54,6 +54,9 @@ typedef enum {
> * Capabilities
> */
>
> +/* Hardware Transactional Memory */
> +#define SPAPR_CAP_HTM 0x0000000000000001ULL
> +
> typedef struct sPAPRCapabilities sPAPRCapabilities;
> struct sPAPRCapabilities {
> uint64_t mask;
>
--
Alexey
- [Qemu-ppc] [RFC for-2.12 0/8] spapr: Add optional capabilities, David Gibson, 2017/12/11
- [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, David Gibson, 2017/12/11
- Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability,
Alexey Kardashevskiy <=
- Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, David Gibson, 2017/12/11
- Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, Alexey Kardashevskiy, 2017/12/11
- Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, David Gibson, 2017/12/11
- Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, Alexey Kardashevskiy, 2017/12/11
- Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, David Gibson, 2017/12/12
- Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, Alexey Kardashevskiy, 2017/12/12
- Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, David Gibson, 2017/12/12
Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, Daniel Henrique Barboza, 2017/12/11
Re: [Qemu-ppc] [RFC for-2.12 3/8] spapr: Treat Hardware Transactional Memory (HTM) as an optional capability, Michael Roth, 2017/12/11