| Message ID | 20180109092103.18458-4-sjitindarsingh@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Rework spapr_caps | expand |
On 01/09/2018 07:21 AM, Suraj Jitindar Singh wrote: > The new H-Call H_GET_CPU_CHARACTERISTICS is used by the guest to query > behaviours and available characteristics of the cpu. > > Implement the handler for this new H-Call which formulates its response > based on the setting of the new capabilities added in the previous > patch. > > Note: Currently we return H_FUNCTION under TCG which will direct the > guest to fall back to doing a displacement flush > > Discussion: > Is TCG affected? > Is there any point in telling the guest to do these workarounds on TCG > given they're unlikely to translate to host instructions which have the > desired effect? Hi, Suraj. What if this is left to the cover letter? > --- > hw/ppc/spapr_hcall.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 1 + > 2 files changed, 82 insertions(+) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 51eba52e86..b62b47c8d9 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -1654,6 +1654,84 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > return H_SUCCESS; > } > > +#define CPU_CHARACTERISTIC_SPEC_BARRIER (1ULL << (63 - 0)) > +#define CPU_CHARACTERISTIC_BCCTR_SERIAL (1ULL << (63 - 1)) > +#define CPU_CHARACTERISTIC_ORI_L1_CACHE (1ULL << (63 - 2)) > +#define CPU_CHARACTERISTIC_MTTRIG_L1_CACHE (1ULL << (63 - 3)) > +#define CPU_CHARACTERISTIC_L1_CACHE_PRIV (1ULL << (63 - 4)) > +#define CPU_CHARACTERISTIC_BRANCH_HINTS (1ULL << (63 - 5)) > +#define CPU_CHARACTERISTIC_MTTRIG_THR_RECONF (1ULL << (63 - 6)) > +#define CPU_BEHAVIOUR_FAVOUR_SECURITY (1ULL << (63 - 0)) > +#define CPU_BEHAVIOUR_L1_CACHE_FLUSH (1ULL << (63 - 1)) > +#define CPU_BEHAVIOUR_SPEC_BARRIER (1ULL << (63 - 2)) Can PPC_BIT be used here? Cheers Murilo
On Tue, 2018-01-09 at 09:19 -0200, Murilo Opsfelder Araújo wrote: > On 01/09/2018 07:21 AM, Suraj Jitindar Singh wrote: > > The new H-Call H_GET_CPU_CHARACTERISTICS is used by the guest to > > query > > behaviours and available characteristics of the cpu. > > > > Implement the handler for this new H-Call which formulates its > > response > > based on the setting of the new capabilities added in the previous > > patch. > > > > Note: Currently we return H_FUNCTION under TCG which will direct > > the > > guest to fall back to doing a displacement flush > > > > Discussion: > > Is TCG affected? > > Is there any point in telling the guest to do these workarounds on > > TCG > > given they're unlikely to translate to host instructions which have > > the > > desired effect? > > Hi, Suraj. > > What if this is left to the cover letter? Again, only because this is RFC. > > > --- > > hw/ppc/spapr_hcall.c | 81 > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/spapr.h | 1 + > > 2 files changed, 82 insertions(+) > > > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > index 51eba52e86..b62b47c8d9 100644 > > --- a/hw/ppc/spapr_hcall.c > > +++ b/hw/ppc/spapr_hcall.c > > @@ -1654,6 +1654,84 @@ static target_ulong > > h_client_architecture_support(PowerPCCPU *cpu, > > return H_SUCCESS; > > } > > > > +#define CPU_CHARACTERISTIC_SPEC_BARRIER (1ULL << (63 - 0)) > > +#define CPU_CHARACTERISTIC_BCCTR_SERIAL (1ULL << (63 - 1)) > > +#define CPU_CHARACTERISTIC_ORI_L1_CACHE (1ULL << (63 - 2)) > > +#define CPU_CHARACTERISTIC_MTTRIG_L1_CACHE (1ULL << (63 - 3)) > > +#define CPU_CHARACTERISTIC_L1_CACHE_PRIV (1ULL << (63 - 4)) > > +#define CPU_CHARACTERISTIC_BRANCH_HINTS (1ULL << (63 - 5)) > > +#define CPU_CHARACTERISTIC_MTTRIG_THR_RECONF (1ULL << (63 - 6)) > > +#define CPU_BEHAVIOUR_FAVOUR_SECURITY (1ULL << (63 - 0)) > > +#define CPU_BEHAVIOUR_L1_CACHE_FLUSH (1ULL << (63 - 1)) > > +#define CPU_BEHAVIOUR_SPEC_BARRIER (1ULL << (63 - 2)) > > Can PPC_BIT be used here? Yep, will do > > Cheers > Murilo >
On Tue, Jan 09, 2018 at 08:21:03PM +1100, Suraj Jitindar Singh wrote: > The new H-Call H_GET_CPU_CHARACTERISTICS is used by the guest to query > behaviours and available characteristics of the cpu. > > Implement the handler for this new H-Call which formulates its response > based on the setting of the new capabilities added in the previous > patch. > > Note: Currently we return H_FUNCTION under TCG which will direct the > guest to fall back to doing a displacement flush > > Discussion: > Is TCG affected? Very likely :(. > Is there any point in telling the guest to do these workarounds on TCG > given they're unlikely to translate to host instructions which have the > desired effect? Probably not. We might have to just advertise broken on TCG, at least until someone has time to figure out the details. > --- > hw/ppc/spapr_hcall.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 1 + > 2 files changed, 82 insertions(+) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 51eba52e86..b62b47c8d9 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -1654,6 +1654,84 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > return H_SUCCESS; > } > > +#define CPU_CHARACTERISTIC_SPEC_BARRIER (1ULL << (63 - 0)) > +#define CPU_CHARACTERISTIC_BCCTR_SERIAL (1ULL << (63 - 1)) > +#define CPU_CHARACTERISTIC_ORI_L1_CACHE (1ULL << (63 - 2)) > +#define CPU_CHARACTERISTIC_MTTRIG_L1_CACHE (1ULL << (63 - 3)) > +#define CPU_CHARACTERISTIC_L1_CACHE_PRIV (1ULL << (63 - 4)) > +#define CPU_CHARACTERISTIC_BRANCH_HINTS (1ULL << (63 - 5)) > +#define CPU_CHARACTERISTIC_MTTRIG_THR_RECONF (1ULL << (63 - 6)) > +#define CPU_BEHAVIOUR_FAVOUR_SECURITY (1ULL << (63 - 0)) > +#define CPU_BEHAVIOUR_L1_CACHE_FLUSH (1ULL << (63 - 1)) > +#define CPU_BEHAVIOUR_SPEC_BARRIER (1ULL << (63 - 2)) > + > +static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu, > + sPAPRMachineState *spapr, > + target_ulong opcode, > + target_ulong *args) > +{ > + uint64_t characteristics = CPU_CHARACTERISTIC_BRANCH_HINTS; > + uint64_t behaviour = CPU_BEHAVIOUR_FAVOUR_SECURITY; I guess we're going to want another knob for the favour security vs favour performance bit here. > + uint8_t safe_cache = spapr_get_cap(spapr, SPAPR_CAP_CFPC); > + uint8_t safe_bounds_check = spapr_get_cap(spapr, SPAPR_CAP_SBBC); > + uint8_t safe_indirect_branch = spapr_get_cap(spapr, SPAPR_CAP_IBS); > + > + /* TODO: Is TCG vulnerable? */ Good question, but in any case.. > + if (!kvm_enabled()) { > + return H_FUNCTION; > + } ..this should still advertise things based on the caps. The point we apply the caps to the virtual hardware is where we need to consider TCG's vulnerability. > + > + switch (safe_cache) { > + case SPAPR_CAP_WORKAROUND: > + characteristics |= CPU_CHARACTERISTIC_ORI_L1_CACHE; > + characteristics |= CPU_CHARACTERISTIC_MTTRIG_L1_CACHE; > + characteristics |= CPU_CHARACTERISTIC_L1_CACHE_PRIV; > + behaviour |= CPU_BEHAVIOUR_L1_CACHE_FLUSH; > + break; > + case SPAPR_CAP_FIXED: > + break; > + default: /* broken */ > + if (safe_cache != SPAPR_CAP_BROKEN) { > + error_report("Invalid value for KVM_CAP_PPC_SAFE_CACHE (%d), assuming broken", > + safe_cache); > + } > + behaviour |= CPU_BEHAVIOUR_L1_CACHE_FLUSH; > + break; > + } > + > + switch (safe_bounds_check) { > + case SPAPR_CAP_WORKAROUND: > + characteristics |= CPU_CHARACTERISTIC_SPEC_BARRIER; > + behaviour |= CPU_BEHAVIOUR_SPEC_BARRIER; > + break; > + case SPAPR_CAP_FIXED: > + break; > + default: /* broken */ > + if (safe_bounds_check != SPAPR_CAP_BROKEN) { > + error_report("Invalid value for KVM_CAP_PPC_SAFE_BOUNDS_CHECK (%d), assuming broken", > + safe_bounds_check); > + } > + behaviour |= CPU_BEHAVIOUR_SPEC_BARRIER; > + break; > + } > + > + switch (safe_indirect_branch) { > + case SPAPR_CAP_FIXED: > + characteristics |= CPU_CHARACTERISTIC_BCCTR_SERIAL; > + default: /* broken */ > + if (safe_indirect_branch != SPAPR_CAP_BROKEN) { > + error_report("Invalid value for KVM_CAP_PPC_SAFE_INDIRECT_BRANCH (%d), assuming broken", > + safe_indirect_branch); > + } > + break; > + } > + > + args[0] = characteristics; > + args[1] = behaviour; > + > + return H_SUCCESS; > +} > + > static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1]; > static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1]; > > @@ -1733,6 +1811,9 @@ static void hypercall_register_types(void) > spapr_register_hypercall(H_INVALIDATE_PID, h_invalidate_pid); > spapr_register_hypercall(H_REGISTER_PROC_TBL, h_register_process_table); > > + /* hcall-get-cpu-characteristics */ > + spapr_register_hypercall(H_GET_CPU_CHARACTERISTICS, h_get_cpu_characteristics); > + > /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate > * here between the "CI" and the "CACHE" variants, they will use whatever > * mapping attributes qemu is using. When using KVM, the kernel will > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 2db2f3e2e2..5677c38d2a 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -396,6 +396,7 @@ struct sPAPRMachineState { > #define H_GET_HCA_INFO 0x1B8 > #define H_GET_PERF_COUNT 0x1BC > #define H_MANAGE_TRACE 0x1C0 > +#define H_GET_CPU_CHARACTERISTICS 0x1C8 > #define H_FREE_LOGICAL_LAN_BUFFER 0x1D4 > #define H_QUERY_INT_STATE 0x1E4 > #define H_POLL_PENDING 0x1D8
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 51eba52e86..b62b47c8d9 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -1654,6 +1654,84 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, return H_SUCCESS; } +#define CPU_CHARACTERISTIC_SPEC_BARRIER (1ULL << (63 - 0)) +#define CPU_CHARACTERISTIC_BCCTR_SERIAL (1ULL << (63 - 1)) +#define CPU_CHARACTERISTIC_ORI_L1_CACHE (1ULL << (63 - 2)) +#define CPU_CHARACTERISTIC_MTTRIG_L1_CACHE (1ULL << (63 - 3)) +#define CPU_CHARACTERISTIC_L1_CACHE_PRIV (1ULL << (63 - 4)) +#define CPU_CHARACTERISTIC_BRANCH_HINTS (1ULL << (63 - 5)) +#define CPU_CHARACTERISTIC_MTTRIG_THR_RECONF (1ULL << (63 - 6)) +#define CPU_BEHAVIOUR_FAVOUR_SECURITY (1ULL << (63 - 0)) +#define CPU_BEHAVIOUR_L1_CACHE_FLUSH (1ULL << (63 - 1)) +#define CPU_BEHAVIOUR_SPEC_BARRIER (1ULL << (63 - 2)) + +static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu, + sPAPRMachineState *spapr, + target_ulong opcode, + target_ulong *args) +{ + uint64_t characteristics = CPU_CHARACTERISTIC_BRANCH_HINTS; + uint64_t behaviour = CPU_BEHAVIOUR_FAVOUR_SECURITY; + uint8_t safe_cache = spapr_get_cap(spapr, SPAPR_CAP_CFPC); + uint8_t safe_bounds_check = spapr_get_cap(spapr, SPAPR_CAP_SBBC); + uint8_t safe_indirect_branch = spapr_get_cap(spapr, SPAPR_CAP_IBS); + + /* TODO: Is TCG vulnerable? */ + if (!kvm_enabled()) { + return H_FUNCTION; + } + + switch (safe_cache) { + case SPAPR_CAP_WORKAROUND: + characteristics |= CPU_CHARACTERISTIC_ORI_L1_CACHE; + characteristics |= CPU_CHARACTERISTIC_MTTRIG_L1_CACHE; + characteristics |= CPU_CHARACTERISTIC_L1_CACHE_PRIV; + behaviour |= CPU_BEHAVIOUR_L1_CACHE_FLUSH; + break; + case SPAPR_CAP_FIXED: + break; + default: /* broken */ + if (safe_cache != SPAPR_CAP_BROKEN) { + error_report("Invalid value for KVM_CAP_PPC_SAFE_CACHE (%d), assuming broken", + safe_cache); + } + behaviour |= CPU_BEHAVIOUR_L1_CACHE_FLUSH; + break; + } + + switch (safe_bounds_check) { + case SPAPR_CAP_WORKAROUND: + characteristics |= CPU_CHARACTERISTIC_SPEC_BARRIER; + behaviour |= CPU_BEHAVIOUR_SPEC_BARRIER; + break; + case SPAPR_CAP_FIXED: + break; + default: /* broken */ + if (safe_bounds_check != SPAPR_CAP_BROKEN) { + error_report("Invalid value for KVM_CAP_PPC_SAFE_BOUNDS_CHECK (%d), assuming broken", + safe_bounds_check); + } + behaviour |= CPU_BEHAVIOUR_SPEC_BARRIER; + break; + } + + switch (safe_indirect_branch) { + case SPAPR_CAP_FIXED: + characteristics |= CPU_CHARACTERISTIC_BCCTR_SERIAL; + default: /* broken */ + if (safe_indirect_branch != SPAPR_CAP_BROKEN) { + error_report("Invalid value for KVM_CAP_PPC_SAFE_INDIRECT_BRANCH (%d), assuming broken", + safe_indirect_branch); + } + break; + } + + args[0] = characteristics; + args[1] = behaviour; + + return H_SUCCESS; +} + static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1]; static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1]; @@ -1733,6 +1811,9 @@ static void hypercall_register_types(void) spapr_register_hypercall(H_INVALIDATE_PID, h_invalidate_pid); spapr_register_hypercall(H_REGISTER_PROC_TBL, h_register_process_table); + /* hcall-get-cpu-characteristics */ + spapr_register_hypercall(H_GET_CPU_CHARACTERISTICS, h_get_cpu_characteristics); + /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate * here between the "CI" and the "CACHE" variants, they will use whatever * mapping attributes qemu is using. When using KVM, the kernel will diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 2db2f3e2e2..5677c38d2a 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -396,6 +396,7 @@ struct sPAPRMachineState { #define H_GET_HCA_INFO 0x1B8 #define H_GET_PERF_COUNT 0x1BC #define H_MANAGE_TRACE 0x1C0 +#define H_GET_CPU_CHARACTERISTICS 0x1C8 #define H_FREE_LOGICAL_LAN_BUFFER 0x1D4 #define H_QUERY_INT_STATE 0x1E4 #define H_POLL_PENDING 0x1D8
The new H-Call H_GET_CPU_CHARACTERISTICS is used by the guest to query behaviours and available characteristics of the cpu. Implement the handler for this new H-Call which formulates its response based on the setting of the new capabilities added in the previous patch. Note: Currently we return H_FUNCTION under TCG which will direct the guest to fall back to doing a displacement flush Discussion: Is TCG affected? Is there any point in telling the guest to do these workarounds on TCG given they're unlikely to translate to host instructions which have the desired effect? --- hw/ppc/spapr_hcall.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 1 + 2 files changed, 82 insertions(+)