Re: [RFC PATCH v2 07/10] KVM: PPC: Ultravisor: Restrict LDBAR access
On 5/21/19 2:24 AM, Madhavan Srinivasan wrote:
>
> On 18/05/19 7:55 PM, Claudio Carvalho wrote:
>> From: Ram Pai When the ultravisor firmware is
>> available, it takes control over the LDBAR register. In this case,
>> thread-imc updates and save/restore operations on the LDBAR register are
>> handled by ultravisor.
> we should remove the core and thread imc nodes in the skiboot
> if the ultravisor is enabled. We dont need imc-pmu.c file changes, imc-pmu.c
> inits only if we have the corresponding nodes. I will post a skiboot patch.
Hi Maddy,
Thanks for the feedback and for taking care of the change needed in skiboot.
Right, if the core and thread imc devtree nodes were not created by
skiboot, then we don't need imc-pmu changes. As a sanity check, should we
have something like the following to make sure that the imc-pmu code will
not be executed even in the situation where ultravisor is enabled, but for
some reason skiboot did not remove those devtree nodes (e.g. your skiboot
patch was not applied)?
-
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -258,6 +258,15 @@ static int opal_imc_counters_probe(struct
platform_device *pdev)
bool core_imc_reg = false, thread_imc_reg = false;
u32 type;
+ /*
+ * When ultravisor is enabled, it is responsible for thread-imc
+ * updates
+ */
+ if (firmware_has_feature(FW_FEATURE_ULTRAVISOR)) {
+ pr_info("IMC Ultravisor enabled\n");
+ return -EACCES;
+ }
+
/*
* Check whether this is kdump kernel. If yes, force the engines to
* stop and return.
Claudio
> Maddy
>
>> Signed-off-by: Ram Pai
>> [Restrict LDBAR access in assembly code and some in C, update the commit
>> message]
>> Signed-off-by: Claudio Carvalho
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 4 +-
>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 +
>> arch/powerpc/perf/imc-pmu.c | 64
>> arch/powerpc/platforms/powernv/idle.c | 6 +-
>> arch/powerpc/platforms/powernv/subcore-asm.S | 4 ++
>> 5 files changed, 52 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 0fab0a201027..81f35f955d16 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -75,6 +75,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "book3s.h"
>>
>> @@ -3117,7 +3118,8 @@ static noinline void kvmppc_run_core(struct
>> kvmppc_vcore *vc)
>> subcore_size = MAX_SMT_THREADS / split;
>> split_info.rpr = mfspr(SPRN_RPR);
>> split_info.pmmar = mfspr(SPRN_PMMAR);
>> - split_info.ldbar = mfspr(SPRN_LDBAR);
>> + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
>> + split_info.ldbar = mfspr(SPRN_LDBAR);
>> split_info.subcore_size = subcore_size;
>> } else {
>> split_info.subcore_size = 1;
>> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> index dd014308f065..938cfa5dceed 100644
>> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> @@ -375,8 +375,10 @@ BEGIN_FTR_SECTION
>> mtspr SPRN_RPR, r0
>> ld r0, KVM_SPLIT_PMMAR(r6)
>> mtspr SPRN_PMMAR, r0
>> +BEGIN_FW_FTR_SECTION_NESTED(70)
>> ld r0, KVM_SPLIT_LDBAR(r6)
>> mtspr SPRN_LDBAR, r0
>> +END_FW_FTR_SECTION_NESTED(FW_FEATURE_ULTRAVISOR, 0, 70)
>> isync
>> FTR_SECTION_ELSE
>> /* On P9 we use the split_info for coordinating LPCR changes */
>> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
>> index 31fa753e2eb2..39c84de74da9 100644
>> --- a/arch/powerpc/perf/imc-pmu.c
>> +++ b/arch/powerpc/perf/imc-pmu.c
>> @@ -17,6 +17,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> /* Nest IMC data structures and variables */
>>
>> @@ -816,6 +817,17 @@ static int core_imc_event_init(struct perf_event
>> *event)
>> return 0;
>> }
>>
>> +static void thread_imc_ldbar_disable(void *dummy)
>> +{
>> + /*
>> + * By Zeroing LDBAR, we disable thread-imc updates. When the
>> ultravisor
>> + * firmware is available, it is responsible for handling thread-imc
>> + * updates, though
>> + */
>> + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
>> + mtspr(SPRN_LDBAR, 0);
>> +}
>> +
>> /*
>> * Allocates a page of memory for each of the online cpus, and load
>> * LDBAR with 0.
>> @@ -856,7 +868,7 @@ static int thread_imc_mem_alloc(int cpu_id, int size)
>> per_cpu(thread_imc_mem, cpu_id) = local_mem;
>> }
>>
>> - mtspr(SPRN_LDBAR, 0);
>> + thread_imc_ldbar_disable(NULL);
>> return 0;
>> }
>>
>> @@ -867,7 +879,7 @@ static int
Re: [RFC PATCH v2 07/10] KVM: PPC: Ultravisor: Restrict LDBAR access
On 18/05/19 7:55 PM, Claudio Carvalho wrote:
From: Ram Pai When the ultravisor firmware is
available, it takes control over the LDBAR register. In this case,
thread-imc updates and save/restore operations on the LDBAR register
are handled by ultravisor.
we should remove the core and thread imc nodes in the skiboot
if the ultravisor is enabled. We dont need imc-pmu.c file changes, imc-pmu.c
inits only if we have the corresponding nodes. I will post a skiboot patch.
Maddy
Signed-off-by: Ram Pai
[Restrict LDBAR access in assembly code and some in C, update the commit
message]
Signed-off-by: Claudio Carvalho
---
arch/powerpc/kvm/book3s_hv.c | 4 +-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 +
arch/powerpc/perf/imc-pmu.c | 64
arch/powerpc/platforms/powernv/idle.c| 6 +-
arch/powerpc/platforms/powernv/subcore-asm.S | 4 ++
5 files changed, 52 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 0fab0a201027..81f35f955d16 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -75,6 +75,7 @@
#include
#include
#include
+#include
#include "book3s.h"
@@ -3117,7 +3118,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore
*vc)
subcore_size = MAX_SMT_THREADS / split;
split_info.rpr = mfspr(SPRN_RPR);
split_info.pmmar = mfspr(SPRN_PMMAR);
- split_info.ldbar = mfspr(SPRN_LDBAR);
+ if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
+ split_info.ldbar = mfspr(SPRN_LDBAR);
split_info.subcore_size = subcore_size;
} else {
split_info.subcore_size = 1;
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index dd014308f065..938cfa5dceed 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -375,8 +375,10 @@ BEGIN_FTR_SECTION
mtspr SPRN_RPR, r0
ld r0, KVM_SPLIT_PMMAR(r6)
mtspr SPRN_PMMAR, r0
+BEGIN_FW_FTR_SECTION_NESTED(70)
ld r0, KVM_SPLIT_LDBAR(r6)
mtspr SPRN_LDBAR, r0
+END_FW_FTR_SECTION_NESTED(FW_FEATURE_ULTRAVISOR, 0, 70)
isync
FTR_SECTION_ELSE
/* On P9 we use the split_info for coordinating LPCR changes */
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 31fa753e2eb2..39c84de74da9 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
/* Nest IMC data structures and variables */
@@ -816,6 +817,17 @@ static int core_imc_event_init(struct perf_event *event)
return 0;
}
+static void thread_imc_ldbar_disable(void *dummy)
+{
+ /*
+* By Zeroing LDBAR, we disable thread-imc updates. When the ultravisor
+* firmware is available, it is responsible for handling thread-imc
+* updates, though
+*/
+ if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
+ mtspr(SPRN_LDBAR, 0);
+}
+
/*
* Allocates a page of memory for each of the online cpus, and load
* LDBAR with 0.
@@ -856,7 +868,7 @@ static int thread_imc_mem_alloc(int cpu_id, int size)
per_cpu(thread_imc_mem, cpu_id) = local_mem;
}
- mtspr(SPRN_LDBAR, 0);
+ thread_imc_ldbar_disable(NULL);
return 0;
}
@@ -867,7 +879,7 @@ static int ppc_thread_imc_cpu_online(unsigned int cpu)
static int ppc_thread_imc_cpu_offline(unsigned int cpu)
{
- mtspr(SPRN_LDBAR, 0);
+ thread_imc_ldbar_disable(NULL);
return 0;
}
@@ -1010,7 +1022,6 @@ static int thread_imc_event_add(struct perf_event *event,
int flags)
{
int core_id;
struct imc_pmu_ref *ref;
- u64 ldbar_value, *local_mem = per_cpu(thread_imc_mem,
smp_processor_id());
if (flags & PERF_EF_START)
imc_event_start(event, flags);
@@ -1019,8 +1030,14 @@ static int thread_imc_event_add(struct perf_event
*event, int flags)
return -EINVAL;
core_id = smp_processor_id() / threads_per_core;
- ldbar_value = ((u64)local_mem & THREAD_IMC_LDBAR_MASK) |
THREAD_IMC_ENABLE;
- mtspr(SPRN_LDBAR, ldbar_value);
+ if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) {
+ u64 ldbar_value, *local_mem;
+
+ local_mem = per_cpu(thread_imc_mem, smp_processor_id());
+ ldbar_value = ((u64)local_mem & THREAD_IMC_LDBAR_MASK) |
+ THREAD_IMC_ENABLE;
+ mtspr(SPRN_LDBAR, ldbar_value);
+ }
/*
* imc pmus are enabled only when it is used.
@@ -1053,7 +1070,7 @@ static void thread_imc_event_del(struct perf_event
*event, int flags)
int core_id;
Re: [RFC PATCH v2 07/10] KVM: PPC: Ultravisor: Restrict LDBAR access
On Sat, May 18, 2019 at 11:25:21AM -0300, Claudio Carvalho wrote: > From: Ram Pai > > When the ultravisor firmware is available, it takes control over the > LDBAR register. In this case, thread-imc updates and save/restore > operations on the LDBAR register are handled by ultravisor. > > Signed-off-by: Ram Pai > [Restrict LDBAR access in assembly code and some in C, update the commit > message] > Signed-off-by: Claudio Carvalho Some of the places that you are patching below are explicitly only executed on POWER8, which can't have an ultravisor, and therefore the change isn't needed: > --- > arch/powerpc/kvm/book3s_hv.c | 4 +- > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 + > arch/powerpc/perf/imc-pmu.c | 64 > arch/powerpc/platforms/powernv/idle.c| 6 +- > arch/powerpc/platforms/powernv/subcore-asm.S | 4 ++ > 5 files changed, 52 insertions(+), 28 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 0fab0a201027..81f35f955d16 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -75,6 +75,7 @@ > #include > #include > #include > +#include > > #include "book3s.h" > > @@ -3117,7 +3118,8 @@ static noinline void kvmppc_run_core(struct > kvmppc_vcore *vc) > subcore_size = MAX_SMT_THREADS / split; > split_info.rpr = mfspr(SPRN_RPR); > split_info.pmmar = mfspr(SPRN_PMMAR); > - split_info.ldbar = mfspr(SPRN_LDBAR); > + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) > + split_info.ldbar = mfspr(SPRN_LDBAR); This is inside an if (is_power8) statement. > diff --git a/arch/powerpc/platforms/powernv/subcore-asm.S > b/arch/powerpc/platforms/powernv/subcore-asm.S > index 39bb24aa8f34..e4383fa5e150 100644 > --- a/arch/powerpc/platforms/powernv/subcore-asm.S > +++ b/arch/powerpc/platforms/powernv/subcore-asm.S > @@ -44,7 +44,9 @@ _GLOBAL(split_core_secondary_loop) > > real_mode: > /* Grab values from unsplit SPRs */ > +BEGIN_FW_FTR_SECTION > mfspr r6, SPRN_LDBAR > +END_FW_FTR_SECTION_IFCLR(FW_FEATURE_ULTRAVISOR) > mfspr r7, SPRN_PMMAR > mfspr r8, SPRN_PMCR > mfspr r9, SPRN_RPR > @@ -77,7 +79,9 @@ real_mode: > mtspr SPRN_HDEC, r4 > > /* Restore SPR values now we are split */ > +BEGIN_FW_FTR_SECTION > mtspr SPRN_LDBAR, r6 > +END_FW_FTR_SECTION_IFCLR(FW_FEATURE_ULTRAVISOR) Only POWER8 supports split-core mode, so we can only get here on POWER8. Paul.
[RFC PATCH v2 07/10] KVM: PPC: Ultravisor: Restrict LDBAR access
From: Ram Pai
When the ultravisor firmware is available, it takes control over the
LDBAR register. In this case, thread-imc updates and save/restore
operations on the LDBAR register are handled by ultravisor.
Signed-off-by: Ram Pai
[Restrict LDBAR access in assembly code and some in C, update the commit
message]
Signed-off-by: Claudio Carvalho
---
arch/powerpc/kvm/book3s_hv.c | 4 +-
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 +
arch/powerpc/perf/imc-pmu.c | 64
arch/powerpc/platforms/powernv/idle.c| 6 +-
arch/powerpc/platforms/powernv/subcore-asm.S | 4 ++
5 files changed, 52 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 0fab0a201027..81f35f955d16 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -75,6 +75,7 @@
#include
#include
#include
+#include
#include "book3s.h"
@@ -3117,7 +3118,8 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore
*vc)
subcore_size = MAX_SMT_THREADS / split;
split_info.rpr = mfspr(SPRN_RPR);
split_info.pmmar = mfspr(SPRN_PMMAR);
- split_info.ldbar = mfspr(SPRN_LDBAR);
+ if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
+ split_info.ldbar = mfspr(SPRN_LDBAR);
split_info.subcore_size = subcore_size;
} else {
split_info.subcore_size = 1;
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index dd014308f065..938cfa5dceed 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -375,8 +375,10 @@ BEGIN_FTR_SECTION
mtspr SPRN_RPR, r0
ld r0, KVM_SPLIT_PMMAR(r6)
mtspr SPRN_PMMAR, r0
+BEGIN_FW_FTR_SECTION_NESTED(70)
ld r0, KVM_SPLIT_LDBAR(r6)
mtspr SPRN_LDBAR, r0
+END_FW_FTR_SECTION_NESTED(FW_FEATURE_ULTRAVISOR, 0, 70)
isync
FTR_SECTION_ELSE
/* On P9 we use the split_info for coordinating LPCR changes */
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 31fa753e2eb2..39c84de74da9 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
/* Nest IMC data structures and variables */
@@ -816,6 +817,17 @@ static int core_imc_event_init(struct perf_event *event)
return 0;
}
+static void thread_imc_ldbar_disable(void *dummy)
+{
+ /*
+* By Zeroing LDBAR, we disable thread-imc updates. When the ultravisor
+* firmware is available, it is responsible for handling thread-imc
+* updates, though
+*/
+ if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
+ mtspr(SPRN_LDBAR, 0);
+}
+
/*
* Allocates a page of memory for each of the online cpus, and load
* LDBAR with 0.
@@ -856,7 +868,7 @@ static int thread_imc_mem_alloc(int cpu_id, int size)
per_cpu(thread_imc_mem, cpu_id) = local_mem;
}
- mtspr(SPRN_LDBAR, 0);
+ thread_imc_ldbar_disable(NULL);
return 0;
}
@@ -867,7 +879,7 @@ static int ppc_thread_imc_cpu_online(unsigned int cpu)
static int ppc_thread_imc_cpu_offline(unsigned int cpu)
{
- mtspr(SPRN_LDBAR, 0);
+ thread_imc_ldbar_disable(NULL);
return 0;
}
@@ -1010,7 +1022,6 @@ static int thread_imc_event_add(struct perf_event *event,
int flags)
{
int core_id;
struct imc_pmu_ref *ref;
- u64 ldbar_value, *local_mem = per_cpu(thread_imc_mem,
smp_processor_id());
if (flags & PERF_EF_START)
imc_event_start(event, flags);
@@ -1019,8 +1030,14 @@ static int thread_imc_event_add(struct perf_event
*event, int flags)
return -EINVAL;
core_id = smp_processor_id() / threads_per_core;
- ldbar_value = ((u64)local_mem & THREAD_IMC_LDBAR_MASK) |
THREAD_IMC_ENABLE;
- mtspr(SPRN_LDBAR, ldbar_value);
+ if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR)) {
+ u64 ldbar_value, *local_mem;
+
+ local_mem = per_cpu(thread_imc_mem, smp_processor_id());
+ ldbar_value = ((u64)local_mem & THREAD_IMC_LDBAR_MASK) |
+ THREAD_IMC_ENABLE;
+ mtspr(SPRN_LDBAR, ldbar_value);
+ }
/*
* imc pmus are enabled only when it is used.
@@ -1053,7 +1070,7 @@ static void thread_imc_event_del(struct perf_event
*event, int flags)
int core_id;
struct imc_pmu_ref *ref;
- mtspr(SPRN_LDBAR, 0);
+ thread_imc_ldbar_disable(NULL);
core_id = smp_processor_id() / threads_per_core;
ref = _imc_refc[core_id];
@@ -1109,7 +1126,7 @@ static int trace_imc_mem_alloc(int cpu_id, int size)
