* [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Alexey Kardashevskiy @ 2021-05-13 11:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michal Marek, linux-kbuild, Alexey Kardashevskiy, Masahiro Yamada,
Nick Desaulniers, linux-kernel, Nathan Chancellor,
clang-built-linux
The $(CPP) (do only preprocessing) macro is already defined in Makefile.
However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
in flags duplication. Which is not a big deal by itself except for
the flags which depend on other flags and the compiler checks them
as it parses the command line.
Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
If clang+llvm+sanitizer are enabled, this results in
-emit-llvm-bc -fno-lto -flto -fvisibility=hidden \
-fsanitize=cfi-mfcall -fno-lto ...
in the clang command line and triggers error:
clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed with '-flto'
This removes unnecessary CPP redefinition. Which works fine as in most
place KBUILD_CFLAGS is passed to $CPP except
arch/powerpc/kernel/vdso64/vdso(32|64).lds. To fix vdso, this does:
1. add -m(big|little)-endian to $CPP
2. add target to $KBUILD_CPPFLAGS as otherwise clang ignores -m(big|little)-endian if
the building platform does not support big endian (such as x86).
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* moved vdso cleanup in a separate patch
* only add target to KBUILD_CPPFLAGS for CLANG
v2:
* fix KBUILD_CPPFLAGS
* add CLANG_FLAGS to CPPFLAGS
---
Makefile | 1 +
arch/powerpc/Makefile | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 15b6476d0f89..5b545bef7653 100644
--- a/Makefile
+++ b/Makefile
@@ -576,6 +576,7 @@ CC_VERSION_TEXT = $(subst $(pound),,$(shell $(CC) --version 2>/dev/null | head -
ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
ifneq ($(CROSS_COMPILE),)
CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
+KBUILD_CPPFLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
endif
ifeq ($(LLVM_IAS),1)
CLANG_FLAGS += -integrated-as
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 3212d076ac6a..306bfd2797ad 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -76,6 +76,7 @@ endif
ifdef CONFIG_CPU_LITTLE_ENDIAN
KBUILD_CFLAGS += -mlittle-endian
+KBUILD_CPPFLAGS += -mlittle-endian
KBUILD_LDFLAGS += -EL
LDEMULATION := lppc
GNUTARGET := powerpcle
@@ -83,6 +84,7 @@ MULTIPLEWORD := -mno-multiple
KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
else
KBUILD_CFLAGS += $(call cc-option,-mbig-endian)
+KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
KBUILD_LDFLAGS += -EB
LDEMULATION := ppc
GNUTARGET := powerpc
@@ -208,7 +210,6 @@ KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH) $(asinstr)
KBUILD_AFLAGS += $(AFLAGS-y)
KBUILD_CFLAGS += $(call cc-option,-msoft-float)
KBUILD_CFLAGS += -pipe $(CFLAGS-y)
-CPP = $(CC) -E $(KBUILD_CFLAGS)
CHECKFLAGS += -m$(BITS) -D__powerpc__ -D__powerpc$(BITS)__
ifdef CONFIG_CPU_BIG_ENDIAN
--
2.30.2
^ permalink raw reply related
* [PATCH 0/4] KVM: PPC: Convert P9 HV path to C
From: Nicholas Piggin @ 2021-05-13 12:22 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
This applies on top of these series:
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=238649
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=238941
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=238946
This was broken out from the large Cify series. Since that was last
posted, changes are:
- Rebase, reordering of patches, tweaking changelog and comments.
- Changed P9 radix exist SLB sanitising to use 4x slbmte to clear
rather than 8x slbmfee/slbmfev, which turns out to be faster (and
is what today's asm code does) [from review from Paul].
- Renamed book3s_hv_interrupt.c to book3s_hv_p9_entry.c, which reduces
confusion with book3s_hv_interrupts.S.
- Fixed !HV compile [Alexey].
Nicholas Piggin (4):
KVM: PPC: Book3S HV P9: Move xive vcpu context management into
kvmhv_p9_guest_entry
KVM: PPC: Book3S HV P9: Move radix MMU switching instructions together
KVM: PPC: Book3S HV P9: Stop handling hcalls in real-mode in the P9
path
KVM: PPC: Book3S HV P9: Implement the rest of the P9 path in C
arch/powerpc/include/asm/asm-prototypes.h | 3 +-
arch/powerpc/include/asm/kvm_asm.h | 1 +
arch/powerpc/include/asm/kvm_book3s_64.h | 8 +
arch/powerpc/include/asm/kvm_host.h | 7 +-
arch/powerpc/include/asm/kvm_ppc.h | 6 +
arch/powerpc/kernel/security.c | 5 +-
arch/powerpc/kvm/Makefile | 1 +
arch/powerpc/kvm/book3s.c | 6 +
arch/powerpc/kvm/book3s_64_entry.S | 254 ++++++++++++++++++++++
arch/powerpc/kvm/book3s_hv.c | 155 +++++++++----
arch/powerpc/kvm/book3s_hv_p9_entry.c | 207 ++++++++++++++++++
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 120 +---------
arch/powerpc/kvm/book3s_xive.c | 64 ++++++
13 files changed, 683 insertions(+), 154 deletions(-)
create mode 100644 arch/powerpc/kvm/book3s_hv_p9_entry.c
--
2.23.0
^ permalink raw reply
* [PATCH 1/4] KVM: PPC: Book3S HV P9: Move xive vcpu context management into kvmhv_p9_guest_entry
From: Nicholas Piggin @ 2021-05-13 12:22 UTC (permalink / raw)
To: kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210513122207.1897664-1-npiggin@gmail.com>
Move the xive management up so the low level register switching can be
pushed further down in a later patch. XIVE MMIO CI operations can run in
higher level code with machine checks, tracing, etc., available.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 8a31df067c65..68914b26017b 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3558,15 +3558,11 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
*/
mtspr(SPRN_HDEC, hdec);
- kvmppc_xive_push_vcpu(vcpu);
-
mtspr(SPRN_SRR0, vcpu->arch.shregs.srr0);
mtspr(SPRN_SRR1, vcpu->arch.shregs.srr1);
trap = __kvmhv_vcpu_entry_p9(vcpu);
- kvmppc_xive_pull_vcpu(vcpu);
-
/* Advance host PURR/SPURR by the amount used by guest */
purr = mfspr(SPRN_PURR);
spurr = mfspr(SPRN_SPURR);
@@ -3764,7 +3760,10 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
trap = 0;
}
} else {
+ kvmppc_xive_push_vcpu(vcpu);
trap = kvmhv_load_hv_regs_and_go(vcpu, time_limit, lpcr);
+ kvmppc_xive_pull_vcpu(vcpu);
+
}
vcpu->arch.slb_max = 0;
--
2.23.0
^ permalink raw reply related
* [PATCH 2/4] KVM: PPC: Book3S HV P9: Move radix MMU switching instructions together
From: Nicholas Piggin @ 2021-05-13 12:22 UTC (permalink / raw)
To: kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210513122207.1897664-1-npiggin@gmail.com>
Switching the MMU from radix<->radix mode is tricky particularly as the
MMU can remain enabled and requires a certain sequence of SPR updates.
Move these together into their own functions.
This also includes the radix TLB check / flush because it's tied in to
MMU switching due to tlbiel getting LPID from LPIDR.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 62 ++++++++++++++++++++++++------------
1 file changed, 41 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 68914b26017b..287cd3e1b918 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3478,12 +3478,49 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
trace_kvmppc_run_core(vc, 1);
}
+static void switch_mmu_to_guest_radix(struct kvm *kvm, struct kvm_vcpu *vcpu, u64 lpcr)
+{
+ struct kvmppc_vcore *vc = vcpu->arch.vcore;
+ struct kvm_nested_guest *nested = vcpu->arch.nested;
+ u32 lpid;
+
+ lpid = nested ? nested->shadow_lpid : kvm->arch.lpid;
+
+ /*
+ * All the isync()s are overkill but trivially follow the ISA
+ * requirements. Some can likely be replaced with justification
+ * comment for why they are not needed.
+ */
+ isync();
+ mtspr(SPRN_LPID, lpid);
+ isync();
+ mtspr(SPRN_LPCR, lpcr);
+ isync();
+ mtspr(SPRN_PID, vcpu->arch.pid);
+ isync();
+
+ /* TLBIEL must have LPIDR set, so set guest LPID before flushing. */
+ kvmppc_check_need_tlb_flush(kvm, vc->pcpu, nested);
+}
+
+static void switch_mmu_to_host_radix(struct kvm *kvm, u32 pid)
+{
+ isync();
+ mtspr(SPRN_PID, pid);
+ isync();
+ mtspr(SPRN_LPID, kvm->arch.host_lpid);
+ isync();
+ mtspr(SPRN_LPCR, kvm->arch.host_lpcr);
+ isync();
+}
+
/*
* Load up hypervisor-mode registers on P9.
*/
static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
unsigned long lpcr)
{
+ struct kvm *kvm = vcpu->kvm;
struct kvmppc_vcore *vc = vcpu->arch.vcore;
s64 hdec;
u64 tb, purr, spurr;
@@ -3535,7 +3572,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
}
mtspr(SPRN_CIABR, vcpu->arch.ciabr);
mtspr(SPRN_IC, vcpu->arch.ic);
- mtspr(SPRN_PID, vcpu->arch.pid);
mtspr(SPRN_PSSCR, vcpu->arch.psscr | PSSCR_EC |
(local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
@@ -3549,8 +3585,7 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
mtspr(SPRN_AMOR, ~0UL);
- mtspr(SPRN_LPCR, lpcr);
- isync();
+ switch_mmu_to_guest_radix(kvm, vcpu, lpcr);
/*
* P9 suppresses the HDEC exception when LPCR[HDICE] = 0,
@@ -3593,7 +3628,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
mtspr(SPRN_DAWR1, host_dawr1);
mtspr(SPRN_DAWRX1, host_dawrx1);
}
- mtspr(SPRN_PID, host_pidr);
/*
* Since this is radix, do a eieio; tlbsync; ptesync sequence in
@@ -3608,9 +3642,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
if (cpu_has_feature(CPU_FTR_ARCH_31))
asm volatile(PPC_CP_ABORT);
- mtspr(SPRN_LPID, vcpu->kvm->arch.host_lpid); /* restore host LPID */
- isync();
-
vc->dpdes = mfspr(SPRN_DPDES);
vc->vtb = mfspr(SPRN_VTB);
mtspr(SPRN_DPDES, 0);
@@ -3627,7 +3658,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
}
mtspr(SPRN_HDEC, 0x7fffffff);
- mtspr(SPRN_LPCR, vcpu->kvm->arch.host_lpcr);
+
+ switch_mmu_to_host_radix(kvm, host_pidr);
return trap;
}
@@ -4181,7 +4213,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
{
struct kvm_run *run = vcpu->run;
int trap, r, pcpu;
- int srcu_idx, lpid;
+ int srcu_idx;
struct kvmppc_vcore *vc;
struct kvm *kvm = vcpu->kvm;
struct kvm_nested_guest *nested = vcpu->arch.nested;
@@ -4255,13 +4287,6 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
vc->vcore_state = VCORE_RUNNING;
trace_kvmppc_run_core(vc, 0);
- if (cpu_has_feature(CPU_FTR_HVMODE)) {
- lpid = nested ? nested->shadow_lpid : kvm->arch.lpid;
- mtspr(SPRN_LPID, lpid);
- isync();
- kvmppc_check_need_tlb_flush(kvm, pcpu, nested);
- }
-
guest_enter_irqoff();
srcu_idx = srcu_read_lock(&kvm->srcu);
@@ -4280,11 +4305,6 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
srcu_read_unlock(&kvm->srcu, srcu_idx);
- if (cpu_has_feature(CPU_FTR_HVMODE)) {
- mtspr(SPRN_LPID, kvm->arch.host_lpid);
- isync();
- }
-
set_irq_happened(trap);
kvmppc_set_host_core(pcpu);
--
2.23.0
^ permalink raw reply related
* [PATCH 3/4] KVM: PPC: Book3S HV P9: Stop handling hcalls in real-mode in the P9 path
From: Nicholas Piggin @ 2021-05-13 12:22 UTC (permalink / raw)
To: kvm-ppc
Cc: Alexey Kardashevskiy, linuxppc-dev, Cédric Le Goater,
Nicholas Piggin
In-Reply-To: <20210513122207.1897664-1-npiggin@gmail.com>
In the interest of minimising the amount of code that is run in
"real-mode", don't handle hcalls in real mode in the P9 path. This
requires some new handlers for H_CEDE and xics-on-xive to be added
before xive is pulled or cede logic is checked.
This introduces a change in radix guest behaviour where radix guests
that execute 'sc 1' in userspace now get a privilege fault whereas
previously the 'sc 1' would be reflected as a syscall interrupt to the
guest kernel. That reflection is only required for hash guests that run
PR KVM.
Background:
In POWER8 and earlier processors, it is very expensive to exit from the
HV real mode context of a guest hypervisor interrupt, and switch to host
virtual mode. On those processors, guest->HV interrupts reach the
hypervisor with the MMU off because the MMU is loaded with guest context
(LPCR, SDR1, SLB), and the other threads in the sub-core need to be
pulled out of the guest too. Then the primary must save off guest state,
invalidate SLB and ERAT, and load up host state before the MMU can be
enabled to run in host virtual mode (~= regular Linux mode).
Hash guests also require a lot of hcalls to run due to the nature of the
MMU architecture and paravirtualisation design. The XICS interrupt
controller requires hcalls to run.
So KVM traditionally tries hard to avoid the full exit, by handling
hcalls and other interrupts in real mode as much as possible.
By contrast, POWER9 has independent MMU context per-thread, and in radix
mode the hypervisor is in host virtual memory mode when the HV interrupt
is taken. Radix guests do not require significant hcalls to manage their
translations, and xive guests don't need hcalls to handle interrupts.
So it's much less important from a performance standpoint to handle
hcalls in real mode in P9.
The TCE hcalls are performance critical, introduced for P8 in order to
achieve 10GbE performance. They was not required on P9 (which was able
to drive 40GbE networking with just the virt mode hcalls), but
performance is quite important. After later changes that add hash guest
support to the P9 path, hash hcalls are also performance critical. The
full entry/exit performance can still be improved significanty, but if
this is found to be inadequate then real-mode hcall handlers could be
reintroduced for the P9 path (in C), but this would be a last resort.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/kvm_ppc.h | 6 ++
arch/powerpc/kvm/book3s.c | 6 ++
arch/powerpc/kvm/book3s_hv.c | 79 +++++++++++++++++++++----
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 5 ++
arch/powerpc/kvm/book3s_xive.c | 64 ++++++++++++++++++++
5 files changed, 149 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 8c10c3427166..cb9e3c85c605 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -129,6 +129,7 @@ extern void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu);
extern int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu);
extern int kvmppc_core_pending_dec(struct kvm_vcpu *vcpu);
extern void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong flags);
+extern void kvmppc_core_queue_syscall(struct kvm_vcpu *vcpu);
extern void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags);
extern void kvmppc_core_queue_fpunavail(struct kvm_vcpu *vcpu);
extern void kvmppc_core_queue_vec_unavail(struct kvm_vcpu *vcpu);
@@ -606,6 +607,7 @@ extern void kvmppc_free_pimap(struct kvm *kvm);
extern int kvmppc_xics_rm_complete(struct kvm_vcpu *vcpu, u32 hcall);
extern void kvmppc_xics_free_icp(struct kvm_vcpu *vcpu);
extern int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 cmd);
+extern int kvmppc_xive_xics_hcall(struct kvm_vcpu *vcpu, u32 req);
extern u64 kvmppc_xics_get_icp(struct kvm_vcpu *vcpu);
extern int kvmppc_xics_set_icp(struct kvm_vcpu *vcpu, u64 icpval);
extern int kvmppc_xics_connect_vcpu(struct kvm_device *dev,
@@ -638,6 +640,8 @@ static inline int kvmppc_xics_enabled(struct kvm_vcpu *vcpu)
static inline void kvmppc_xics_free_icp(struct kvm_vcpu *vcpu) { }
static inline int kvmppc_xics_hcall(struct kvm_vcpu *vcpu, u32 cmd)
{ return 0; }
+static inline int kvmppc_xive_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
+ { return 0; }
#endif
#ifdef CONFIG_KVM_XIVE
@@ -672,6 +676,7 @@ extern int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
int level, bool line_status);
extern void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu);
extern void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu);
+extern void kvmppc_xive_rearm_escalation(struct kvm_vcpu *vcpu);
static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
{
@@ -713,6 +718,7 @@ static inline int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 ir
int level, bool line_status) { return -ENODEV; }
static inline void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu) { }
static inline void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu) { }
+static inline void kvmppc_xive_rearm_escalation(struct kvm_vcpu *vcpu) { }
static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
{ return 0; }
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 2b691f4d1f26..d69560d5bf16 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -171,6 +171,12 @@ void kvmppc_core_queue_machine_check(struct kvm_vcpu *vcpu, ulong flags)
}
EXPORT_SYMBOL_GPL(kvmppc_core_queue_machine_check);
+void kvmppc_core_queue_syscall(struct kvm_vcpu *vcpu)
+{
+ kvmppc_inject_interrupt(vcpu, BOOK3S_INTERRUPT_SYSCALL, 0);
+}
+EXPORT_SYMBOL(kvmppc_core_queue_syscall);
+
void kvmppc_core_queue_program(struct kvm_vcpu *vcpu, ulong flags)
{
/* might as well deliver this straight away */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 287cd3e1b918..5815c04a48a3 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -899,6 +899,10 @@ static int kvm_arch_vcpu_yield_to(struct kvm_vcpu *target)
* H_SUCCESS if the source vcore wasn't idle (e.g. if it may
* have useful work to do and should not confer) so we don't
* recheck that here.
+ *
+ * In the case of the P9 single vcpu per vcore case, the real
+ * mode handler is not called but no other threads are in the
+ * source vcore.
*/
spin_lock(&vcore->lock);
@@ -1142,12 +1146,13 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
}
/*
- * Handle H_CEDE in the nested virtualization case where we haven't
- * called the real-mode hcall handlers in book3s_hv_rmhandlers.S.
+ * Handle H_CEDE in the P9 path where we don't call the real-mode hcall
+ * handlers in book3s_hv_rmhandlers.S.
+ *
* This has to be done early, not in kvmppc_pseries_do_hcall(), so
* that the cede logic in kvmppc_run_single_vcpu() works properly.
*/
-static void kvmppc_nested_cede(struct kvm_vcpu *vcpu)
+static void kvmppc_cede(struct kvm_vcpu *vcpu)
{
vcpu->arch.shregs.msr |= MSR_EE;
vcpu->arch.ceded = 1;
@@ -1400,13 +1405,29 @@ static int kvmppc_handle_exit_hv(struct kvm_vcpu *vcpu,
}
case BOOK3S_INTERRUPT_SYSCALL:
{
- /* hcall - punt to userspace */
int i;
- /* hypercall with MSR_PR has already been handled in rmode,
- * and never reaches here.
- */
+ if (unlikely(vcpu->arch.shregs.msr & MSR_PR)) {
+ /*
+ * Guest userspace executed sc 1. This can only be
+ * reached by the P9 path because the old path
+ * handles this case in realmode hcall handlers.
+ *
+ * Radix guests can not run PR KVM or nested HV hash
+ * guests which might run PR KVM, so this is always
+ * a privilege fault. Send a program check to guest
+ * kernel.
+ */
+ kvmppc_core_queue_program(vcpu, SRR1_PROGPRIV);
+ r = RESUME_GUEST;
+ break;
+ }
+ /*
+ * hcall - gather args and set exit_reason. This will next be
+ * handled by kvmppc_pseries_do_hcall which may be able to deal
+ * with it and resume guest, or may punt to userspace.
+ */
run->papr_hcall.nr = kvmppc_get_gpr(vcpu, 3);
for (i = 0; i < 9; ++i)
run->papr_hcall.args[i] = kvmppc_get_gpr(vcpu, 4 + i);
@@ -3664,6 +3685,12 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
return trap;
}
+static inline bool hcall_is_xics(unsigned long req)
+{
+ return req == H_EOI || req == H_CPPR || req == H_IPI ||
+ req == H_IPOLL || req == H_XIRR || req == H_XIRR_X;
+}
+
/*
* Virtual-mode guest entry for POWER9 and later when the host and
* guest are both using the radix MMU. The LPIDR has already been set.
@@ -3787,15 +3814,36 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
/* H_CEDE has to be handled now, not later */
if (trap == BOOK3S_INTERRUPT_SYSCALL && !vcpu->arch.nested &&
kvmppc_get_gpr(vcpu, 3) == H_CEDE) {
- kvmppc_nested_cede(vcpu);
+ kvmppc_cede(vcpu);
kvmppc_set_gpr(vcpu, 3, 0);
trap = 0;
}
} else {
kvmppc_xive_push_vcpu(vcpu);
trap = kvmhv_load_hv_regs_and_go(vcpu, time_limit, lpcr);
+ if (trap == BOOK3S_INTERRUPT_SYSCALL && !vcpu->arch.nested &&
+ !(vcpu->arch.shregs.msr & MSR_PR)) {
+ unsigned long req = kvmppc_get_gpr(vcpu, 3);
+
+ /* H_CEDE has to be handled now, not later */
+ if (req == H_CEDE) {
+ kvmppc_cede(vcpu);
+ kvmppc_xive_rearm_escalation(vcpu); /* may un-cede */
+ kvmppc_set_gpr(vcpu, 3, 0);
+ trap = 0;
+
+ /* XICS hcalls must be handled before xive is pulled */
+ } else if (hcall_is_xics(req)) {
+ int ret;
+
+ ret = kvmppc_xive_xics_hcall(vcpu, req);
+ if (ret != H_TOO_HARD) {
+ kvmppc_set_gpr(vcpu, 3, ret);
+ trap = 0;
+ }
+ }
+ }
kvmppc_xive_pull_vcpu(vcpu);
-
}
vcpu->arch.slb_max = 0;
@@ -4461,8 +4509,17 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
else
r = kvmppc_run_vcpu(vcpu);
- if (run->exit_reason == KVM_EXIT_PAPR_HCALL &&
- !(vcpu->arch.shregs.msr & MSR_PR)) {
+ if (run->exit_reason == KVM_EXIT_PAPR_HCALL) {
+ if (WARN_ON_ONCE(vcpu->arch.shregs.msr & MSR_PR)) {
+ /*
+ * These should have been caught reflected
+ * into the guest by now. Final sanity check:
+ * don't allow userspace to execute hcalls in
+ * the hypervisor.
+ */
+ r = RESUME_GUEST;
+ continue;
+ }
trace_kvm_hcall_enter(vcpu);
r = kvmppc_pseries_do_hcall(vcpu);
trace_kvm_hcall_exit(vcpu, r);
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index c11597f815e4..2d0d14ed1d92 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1397,9 +1397,14 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
mr r4,r9
bge fast_guest_return
2:
+ /* If we came in through the P9 short path, no real mode hcalls */
+ lwz r0, STACK_SLOT_SHORT_PATH(r1)
+ cmpwi r0, 0
+ bne no_try_real
/* See if this is an hcall we can handle in real mode */
cmpwi r12,BOOK3S_INTERRUPT_SYSCALL
beq hcall_try_real_mode
+no_try_real:
/* Hypervisor doorbell - exit only if host IPI flag set */
cmpwi r12, BOOK3S_INTERRUPT_H_DOORBELL
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 741bf1f4387a..24c07094651a 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -158,6 +158,40 @@ void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvmppc_xive_pull_vcpu);
+void kvmppc_xive_rearm_escalation(struct kvm_vcpu *vcpu)
+{
+ void __iomem *esc_vaddr = (void __iomem *)vcpu->arch.xive_esc_vaddr;
+
+ if (!esc_vaddr)
+ return;
+
+ /* we are using XIVE with single escalation */
+
+ if (vcpu->arch.xive_esc_on) {
+ /*
+ * If we still have a pending escalation, abort the cede,
+ * and we must set PQ to 10 rather than 00 so that we don't
+ * potentially end up with two entries for the escalation
+ * interrupt in the XIVE interrupt queue. In that case
+ * we also don't want to set xive_esc_on to 1 here in
+ * case we race with xive_esc_irq().
+ */
+ vcpu->arch.ceded = 0;
+ /*
+ * The escalation interrupts are special as we don't EOI them.
+ * There is no need to use the load-after-store ordering offset
+ * to set PQ to 10 as we won't use StoreEOI.
+ */
+ __raw_readq(esc_vaddr + XIVE_ESB_SET_PQ_10);
+ } else {
+ vcpu->arch.xive_esc_on = true;
+ mb();
+ __raw_readq(esc_vaddr + XIVE_ESB_SET_PQ_00);
+ }
+ mb();
+}
+EXPORT_SYMBOL_GPL(kvmppc_xive_rearm_escalation);
+
/*
* This is a simple trigger for a generic XIVE IRQ. This must
* only be called for interrupts that support a trigger page
@@ -2106,6 +2140,36 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
return 0;
}
+int kvmppc_xive_xics_hcall(struct kvm_vcpu *vcpu, u32 req)
+{
+ struct kvmppc_vcore *vc = vcpu->arch.vcore;
+
+ /* The VM should have configured XICS mode before doing XICS hcalls. */
+ if (!kvmppc_xics_enabled(vcpu))
+ return H_TOO_HARD;
+
+ switch (req) {
+ case H_XIRR:
+ return xive_vm_h_xirr(vcpu);
+ case H_CPPR:
+ return xive_vm_h_cppr(vcpu, kvmppc_get_gpr(vcpu, 4));
+ case H_EOI:
+ return xive_vm_h_eoi(vcpu, kvmppc_get_gpr(vcpu, 4));
+ case H_IPI:
+ return xive_vm_h_ipi(vcpu, kvmppc_get_gpr(vcpu, 4),
+ kvmppc_get_gpr(vcpu, 5));
+ case H_IPOLL:
+ return xive_vm_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4));
+ case H_XIRR_X:
+ xive_vm_h_xirr(vcpu);
+ kvmppc_set_gpr(vcpu, 5, get_tb() + vc->tb_offset);
+ return H_SUCCESS;
+ }
+
+ return H_UNSUPPORTED;
+}
+EXPORT_SYMBOL_GPL(kvmppc_xive_xics_hcall);
+
int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu)
{
struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
--
2.23.0
^ permalink raw reply related
* [PATCH 4/4] KVM: PPC: Book3S HV P9: Implement the rest of the P9 path in C
From: Nicholas Piggin @ 2021-05-13 12:22 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210513122207.1897664-1-npiggin@gmail.com>
Almost all logic is moved to C, by introducing a new in_guest mode for
the P9 path that branches very early in the KVM interrupt handler to P9
exit code.
The main P9 entry and exit assembly is now only about 160 lines of low
level stack setup and register save/restore, plus a bad-interrupt
handler.
There are two motivations for this, the first is just make the code more
maintainable being in C. The second is to reduce the amount of code
running in a special KVM mode, "realmode". In quotes because with radix
it is no longer necessarily real-mode in the MMU, but it still has to be
treated specially because it may be in real-mode, and has various
important registers like PID, DEC, TB, etc set to guest. This is hostile
to the rest of Linux and can't use arbitrary kernel functionality or be
instrumented well.
This initial patch is a reasonably faithful conversion of the asm code,
but it does lack any loop to return quickly back into the guest without
switching out of realmode in the case of unimportant or easily handled
interrupts. As explained in previous changes, handling HV interrupts
very quickly in this low level realmode is not so important for P9
performance, and are important to avoid for security, observability,
debugability reasons.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/asm-prototypes.h | 3 +-
arch/powerpc/include/asm/kvm_asm.h | 1 +
arch/powerpc/include/asm/kvm_book3s_64.h | 8 +
arch/powerpc/include/asm/kvm_host.h | 7 +-
arch/powerpc/kernel/security.c | 5 +-
arch/powerpc/kvm/Makefile | 1 +
arch/powerpc/kvm/book3s_64_entry.S | 254 ++++++++++++++++++++++
arch/powerpc/kvm/book3s_hv.c | 9 +-
arch/powerpc/kvm/book3s_hv_p9_entry.c | 207 ++++++++++++++++++
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 125 +----------
10 files changed, 496 insertions(+), 124 deletions(-)
create mode 100644 arch/powerpc/kvm/book3s_hv_p9_entry.c
diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 1c7b75834e04..02ee6f5ac9fe 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -120,6 +120,7 @@ extern s32 patch__call_flush_branch_caches3;
extern s32 patch__flush_count_cache_return;
extern s32 patch__flush_link_stack_return;
extern s32 patch__call_kvm_flush_link_stack;
+extern s32 patch__call_kvm_flush_link_stack_p9;
extern s32 patch__memset_nocache, patch__memcpy_nocache;
extern long flush_branch_caches;
@@ -140,7 +141,7 @@ void kvmhv_load_host_pmu(void);
void kvmhv_save_guest_pmu(struct kvm_vcpu *vcpu, bool pmu_in_use);
void kvmhv_load_guest_pmu(struct kvm_vcpu *vcpu);
-int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu);
+void kvmppc_p9_enter_guest(struct kvm_vcpu *vcpu);
long kvmppc_h_set_dabr(struct kvm_vcpu *vcpu, unsigned long dabr);
long kvmppc_h_set_xdabr(struct kvm_vcpu *vcpu, unsigned long dabr,
diff --git a/arch/powerpc/include/asm/kvm_asm.h b/arch/powerpc/include/asm/kvm_asm.h
index a3633560493b..43b1788e1f93 100644
--- a/arch/powerpc/include/asm/kvm_asm.h
+++ b/arch/powerpc/include/asm/kvm_asm.h
@@ -147,6 +147,7 @@
#define KVM_GUEST_MODE_SKIP 2
#define KVM_GUEST_MODE_GUEST_HV 3
#define KVM_GUEST_MODE_HOST_HV 4
+#define KVM_GUEST_MODE_HV_FAST 5 /* ISA >= v3.0 host+guest radix, indep thr */
#define KVM_INST_FETCH_FAILED -1
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 9bb9bb370b53..c214bcffb441 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -153,9 +153,17 @@ static inline bool kvmhv_vcpu_is_radix(struct kvm_vcpu *vcpu)
return radix;
}
+int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu);
+
#define KVM_DEFAULT_HPT_ORDER 24 /* 16MB HPT by default */
#endif
+/*
+ * Invalid HDSISR value which is used to indicate when HW has not set the reg.
+ * Used to work around an errata.
+ */
+#define HDSISR_CANARY 0x7fff
+
/*
* We use a lock bit in HPTE dword 0 to synchronize updates and
* accesses to each HPTE, and another bit to indicate non-present
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 1e83359f286b..69add9d662df 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -683,7 +683,12 @@ struct kvm_vcpu_arch {
ulong fault_dar;
u32 fault_dsisr;
unsigned long intr_msr;
- ulong fault_gpa; /* guest real address of page fault (POWER9) */
+ /*
+ * POWER9 and later: fault_gpa contains the guest real address of page
+ * fault for a radix guest, or segment descriptor (equivalent to result
+ * from slbmfev of SLB entry that translated the EA) for hash guests.
+ */
+ ulong fault_gpa;
#endif
#ifdef CONFIG_BOOKE
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 0fdfcdd9d880..c17d1c9362b5 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -432,16 +432,19 @@ device_initcall(stf_barrier_debugfs_init);
static void update_branch_cache_flush(void)
{
- u32 *site;
+ u32 *site, __maybe_unused *site2;
#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
site = &patch__call_kvm_flush_link_stack;
+ site2 = &patch__call_kvm_flush_link_stack_p9;
// This controls the branch from guest_exit_cont to kvm_flush_link_stack
if (link_stack_flush_type == BRANCH_CACHE_FLUSH_NONE) {
patch_instruction_site(site, ppc_inst(PPC_INST_NOP));
+ patch_instruction_site(site2, ppc_inst(PPC_INST_NOP));
} else {
// Could use HW flush, but that could also flush count cache
patch_branch_site(site, (u64)&kvm_flush_link_stack, BRANCH_SET_LINK);
+ patch_branch_site(site2, (u64)&kvm_flush_link_stack, BRANCH_SET_LINK);
}
#endif
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index cdd119028f64..91eb67bb91e1 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -89,6 +89,7 @@ kvm-book3s_64-builtin-tm-objs-$(CONFIG_PPC_TRANSACTIONAL_MEM) += \
ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
book3s_hv_hmi.o \
+ book3s_hv_p9_entry.o \
book3s_hv_rmhandlers.o \
book3s_hv_rm_mmu.o \
book3s_hv_ras.o \
diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
index a01046202eef..177e8fad5c8d 100644
--- a/arch/powerpc/kvm/book3s_64_entry.S
+++ b/arch/powerpc/kvm/book3s_64_entry.S
@@ -1,11 +1,16 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#include <asm/asm-offsets.h>
#include <asm/cache.h>
+#include <asm/code-patching-asm.h>
#include <asm/exception-64s.h>
+#include <asm/export.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_book3s_asm.h>
+#include <asm/mmu.h>
#include <asm/ppc_asm.h>
+#include <asm/ptrace.h>
#include <asm/reg.h>
+#include <asm/ultravisor-api.h>
/*
* These are branched to from interrupt handlers in exception-64s.S which set
@@ -29,6 +34,11 @@
.global kvmppc_hcall
.balign IFETCH_ALIGN_BYTES
kvmppc_hcall:
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+ lbz r10,HSTATE_IN_GUEST(r13)
+ cmpwi r10,KVM_GUEST_MODE_HV_FAST
+ beq kvmppc_p9_exit_hcall
+#endif
ld r10,PACA_EXGEN+EX_R13(r13)
SET_SCRATCH0(r10)
li r10,0xc00
@@ -55,6 +65,13 @@ kvmppc_hcall:
.global kvmppc_interrupt
.balign IFETCH_ALIGN_BYTES
kvmppc_interrupt:
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+ std r10,HSTATE_SCRATCH0(r13)
+ lbz r10,HSTATE_IN_GUEST(r13)
+ cmpwi r10,KVM_GUEST_MODE_HV_FAST
+ beq kvmppc_p9_exit_interrupt
+ ld r10,HSTATE_SCRATCH0(r13)
+#endif
li r11,PACA_EXGEN
cmpdi r10,0x200
bgt+ .Lgot_save_area
@@ -156,3 +173,240 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
GET_SCRATCH0(r13)
HRFI_TO_KERNEL
#endif
+
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+
+/* Stack frame offsets for kvmppc_p9_enter_guest */
+#define SFS (144 + STACK_FRAME_MIN_SIZE)
+#define STACK_SLOT_NVGPRS (SFS - 144) /* 18 gprs */
+
+/*
+ * void kvmppc_p9_enter_guest(struct vcpu *vcpu);
+ *
+ * Enter the guest on a ISAv3.0 or later system where we have exactly
+ * one vcpu per vcore, and both the host and guest are radix, and threads
+ * are set to "indepdent mode".
+ */
+.balign IFETCH_ALIGN_BYTES
+_GLOBAL(kvmppc_p9_enter_guest)
+EXPORT_SYMBOL_GPL(kvmppc_p9_enter_guest)
+ mflr r0
+ std r0,PPC_LR_STKOFF(r1)
+ stdu r1,-SFS(r1)
+
+ std r1,HSTATE_HOST_R1(r13)
+
+ mfcr r4
+ stw r4,SFS+8(r1)
+
+ reg = 14
+ .rept 18
+ std reg,STACK_SLOT_NVGPRS + ((reg - 14) * 8)(r1)
+ reg = reg + 1
+ .endr
+
+ ld r4,VCPU_LR(r3)
+ mtlr r4
+ ld r4,VCPU_CTR(r3)
+ mtctr r4
+ ld r4,VCPU_XER(r3)
+ mtspr SPRN_XER,r4
+
+ ld r1,VCPU_CR(r3)
+
+BEGIN_FTR_SECTION
+ ld r4,VCPU_CFAR(r3)
+ mtspr SPRN_CFAR,r4
+END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
+BEGIN_FTR_SECTION
+ ld r4,VCPU_PPR(r3)
+ mtspr SPRN_PPR,r4
+END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
+
+ reg = 4
+ .rept 28
+ ld reg,__VCPU_GPR(reg)(r3)
+ reg = reg + 1
+ .endr
+
+ ld r4,VCPU_KVM(r3)
+ lbz r4,KVM_SECURE_GUEST(r4)
+ cmpdi r4,0
+ ld r4,VCPU_GPR(R4)(r3)
+ bne .Lret_to_ultra
+
+ mtcr r1
+
+ ld r0,VCPU_GPR(R0)(r3)
+ ld r1,VCPU_GPR(R1)(r3)
+ ld r2,VCPU_GPR(R2)(r3)
+ ld r3,VCPU_GPR(R3)(r3)
+
+ HRFI_TO_GUEST
+ b .
+
+ /*
+ * Use UV_RETURN ultracall to return control back to the Ultravisor
+ * after processing an hypercall or interrupt that was forwarded
+ * (a.k.a. reflected) to the Hypervisor.
+ *
+ * All registers have already been reloaded except the ucall requires:
+ * R0 = hcall result
+ * R2 = SRR1, so UV can detect a synthesized interrupt (if any)
+ * R3 = UV_RETURN
+ */
+.Lret_to_ultra:
+ mtcr r1
+ ld r1,VCPU_GPR(R1)(r3)
+
+ ld r0,VCPU_GPR(R3)(r3)
+ mfspr r2,SPRN_SRR1
+ LOAD_REG_IMMEDIATE(r3, UV_RETURN)
+ sc 2
+
+/*
+ * kvmppc_p9_exit_hcall and kvmppc_p9_exit_interrupt are branched to from
+ * above if the interrupt was taken for a guest that was entered via
+ * kvmppc_p9_enter_guest().
+ *
+ * The exit code recovers the host stack and vcpu pointer, saves all guest GPRs
+ * and CR, LR, XER as well as guest MSR and NIA into the VCPU, then re-
+ * establishes the host stack and registers to return from the
+ * kvmppc_p9_enter_guest() function, which saves CTR and other guest registers
+ * (SPRs and FP, VEC, etc).
+ */
+.balign IFETCH_ALIGN_BYTES
+kvmppc_p9_exit_hcall:
+ mfspr r11,SPRN_SRR0
+ mfspr r12,SPRN_SRR1
+ li r10,0xc00
+ std r10,HSTATE_SCRATCH0(r13)
+
+.balign IFETCH_ALIGN_BYTES
+kvmppc_p9_exit_interrupt:
+ /*
+ * If set to KVM_GUEST_MODE_HV_FAST but we're still in the
+ * hypervisor, that means we can't return from the entry stack.
+ */
+ rldicl. r10,r12,64-MSR_HV_LG,63
+ bne- kvmppc_p9_bad_interrupt
+
+ std r1,HSTATE_SCRATCH1(r13)
+ std r3,HSTATE_SCRATCH2(r13)
+ ld r1,HSTATE_HOST_R1(r13)
+ ld r3,HSTATE_KVM_VCPU(r13)
+
+ std r9,VCPU_CR(r3)
+
+1:
+ std r11,VCPU_PC(r3)
+ std r12,VCPU_MSR(r3)
+
+ reg = 14
+ .rept 18
+ std reg,__VCPU_GPR(reg)(r3)
+ reg = reg + 1
+ .endr
+
+ /* r1, r3, r9-r13 are saved to vcpu by C code */
+ std r0,VCPU_GPR(R0)(r3)
+ std r2,VCPU_GPR(R2)(r3)
+ reg = 4
+ .rept 5
+ std reg,__VCPU_GPR(reg)(r3)
+ reg = reg + 1
+ .endr
+
+ ld r2,PACATOC(r13)
+
+ mflr r4
+ std r4,VCPU_LR(r3)
+ mfspr r4,SPRN_XER
+ std r4,VCPU_XER(r3)
+
+ reg = 14
+ .rept 18
+ ld reg,STACK_SLOT_NVGPRS + ((reg - 14) * 8)(r1)
+ reg = reg + 1
+ .endr
+
+ lwz r4,SFS+8(r1)
+ mtcr r4
+
+ /*
+ * Flush the link stack here, before executing the first blr on the
+ * way out of the guest.
+ *
+ * The link stack won't match coming out of the guest anyway so the
+ * only cost is the flush itself. The call clobbers r0.
+ */
+1: nop
+ patch_site 1b patch__call_kvm_flush_link_stack_p9
+
+ addi r1,r1,SFS
+ ld r0,PPC_LR_STKOFF(r1)
+ mtlr r0
+ blr
+
+/*
+ * Took an interrupt somewhere right before HRFID to guest, so registers are
+ * in a bad way. Return things hopefully enough to run host virtual code and
+ * run the Linux interrupt handler (SRESET or MCE) to print something useful.
+ *
+ * We could be really clever and save all host registers in known locations
+ * before setting HSTATE_IN_GUEST, then restoring them all here, and setting
+ * return address to a fixup that sets them up again. But that's a lot of
+ * effort for a small bit of code. Lots of other things to do first.
+ */
+kvmppc_p9_bad_interrupt:
+ /*
+ * Clean up guest registers to give host a chance to run.
+ */
+ li r10,0
+ mtspr SPRN_AMR,r10
+ mtspr SPRN_IAMR,r10
+ mtspr SPRN_CIABR,r10
+ mtspr SPRN_DAWRX0,r10
+BEGIN_FTR_SECTION
+ mtspr SPRN_DAWRX1,r10
+END_FTR_SECTION_IFSET(CPU_FTR_DAWR1)
+ mtspr SPRN_PID,r10
+
+ /*
+ * Switch to host MMU mode
+ */
+ ld r10, HSTATE_KVM_VCPU(r13)
+ ld r10, VCPU_KVM(r10)
+ lwz r10, KVM_HOST_LPID(r10)
+ mtspr SPRN_LPID,r10
+
+ ld r10, HSTATE_KVM_VCPU(r13)
+ ld r10, VCPU_KVM(r10)
+ ld r10, KVM_HOST_LPCR(r10)
+ mtspr SPRN_LPCR,r10
+
+ /*
+ * Set GUEST_MODE_NONE so the handler won't branch to KVM, and clear
+ * MSR_RI in r12 ([H]SRR1) so the handler won't try to return.
+ */
+ li r10,KVM_GUEST_MODE_NONE
+ stb r10,HSTATE_IN_GUEST(r13)
+ li r10,MSR_RI
+ andc r12,r12,r10
+
+ /*
+ * Go back to interrupt handler. MCE and SRESET have their specific
+ * PACA save area so they should be used directly. They set up their
+ * own stack. The other handlers all use EXGEN. They will use the
+ * guest r1 if it looks like a kernel stack, so just load the
+ * emergency stack and go to program check for all other interrupts.
+ */
+ ld r10,HSTATE_SCRATCH0(r13)
+ cmpwi r10,BOOK3S_INTERRUPT_MACHINE_CHECK
+ beq machine_check_common
+
+ cmpwi r10,BOOK3S_INTERRUPT_SYSTEM_RESET
+ beq system_reset_common
+
+ b .
+#endif
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 5815c04a48a3..e338da9d9312 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1445,6 +1445,8 @@ static int kvmppc_handle_exit_hv(struct kvm_vcpu *vcpu,
*/
case BOOK3S_INTERRUPT_H_DATA_STORAGE:
r = RESUME_PAGE_FAULT;
+ if (vcpu->arch.fault_dsisr == HDSISR_CANARY)
+ r = RESUME_GUEST; /* Just retry if it's the canary */
break;
case BOOK3S_INTERRUPT_H_INST_STORAGE:
vcpu->arch.fault_dar = kvmppc_get_pc(vcpu);
@@ -3708,6 +3710,8 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
u64 tb;
int trap, save_pmu;
+ WARN_ON_ONCE(vcpu->arch.ceded);
+
dec = mfspr(SPRN_DEC);
tb = mftb();
if (dec < 0)
@@ -3716,8 +3720,6 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
if (local_paca->kvm_hstate.dec_expires < time_limit)
time_limit = local_paca->kvm_hstate.dec_expires;
- vcpu->arch.ceded = 0;
-
kvmhv_save_host_pmu(); /* saves it to PACA kvm_hstate */
kvmppc_subcore_enter_guest();
@@ -3844,9 +3846,10 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
}
}
kvmppc_xive_pull_vcpu(vcpu);
+
+ vcpu->arch.slb_max = 0;
}
- vcpu->arch.slb_max = 0;
dec = mfspr(SPRN_DEC);
if (!(lpcr & LPCR_LD)) /* Sign extend if not using large decrementer */
dec = (s32) dec;
diff --git a/arch/powerpc/kvm/book3s_hv_p9_entry.c b/arch/powerpc/kvm/book3s_hv_p9_entry.c
new file mode 100644
index 000000000000..9db0e031a443
--- /dev/null
+++ b/arch/powerpc/kvm/book3s_hv_p9_entry.c
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/kernel.h>
+#include <linux/kvm_host.h>
+#include <asm/asm-prototypes.h>
+#include <asm/dbell.h>
+#include <asm/kvm_ppc.h>
+
+#ifdef CONFIG_KVM_BOOK3S_HV_EXIT_TIMING
+static void __start_timing(struct kvm_vcpu *vcpu, struct kvmhv_tb_accumulator *next)
+{
+ struct kvmppc_vcore *vc = vcpu->arch.vcore;
+ u64 tb = mftb() - vc->tb_offset_applied;
+
+ vcpu->arch.cur_activity = next;
+ vcpu->arch.cur_tb_start = tb;
+}
+
+static void __accumulate_time(struct kvm_vcpu *vcpu, struct kvmhv_tb_accumulator *next)
+{
+ struct kvmppc_vcore *vc = vcpu->arch.vcore;
+ struct kvmhv_tb_accumulator *curr;
+ u64 tb = mftb() - vc->tb_offset_applied;
+ u64 prev_tb;
+ u64 delta;
+ u64 seq;
+
+ curr = vcpu->arch.cur_activity;
+ vcpu->arch.cur_activity = next;
+ prev_tb = vcpu->arch.cur_tb_start;
+ vcpu->arch.cur_tb_start = tb;
+
+ if (!curr)
+ return;
+
+ delta = tb - prev_tb;
+
+ seq = curr->seqcount;
+ curr->seqcount = seq + 1;
+ smp_wmb();
+ curr->tb_total += delta;
+ if (seq == 0 || delta < curr->tb_min)
+ curr->tb_min = delta;
+ if (delta > curr->tb_max)
+ curr->tb_max = delta;
+ smp_wmb();
+ curr->seqcount = seq + 2;
+}
+
+#define start_timing(vcpu, next) __start_timing(vcpu, next)
+#define end_timing(vcpu) __start_timing(vcpu, NULL)
+#define accumulate_time(vcpu, next) __accumulate_time(vcpu, next)
+#else
+#define start_timing(vcpu, next) do {} while (0)
+#define end_timing(vcpu) do {} while (0)
+#define accumulate_time(vcpu, next) do {} while (0)
+#endif
+
+static inline void mtslb(u64 slbee, u64 slbev)
+{
+ asm volatile("slbmte %0,%1" :: "r" (slbev), "r" (slbee));
+}
+
+static inline void clear_slb_entry(unsigned int idx)
+{
+ mtslb(idx, 0);
+}
+
+/*
+ * Malicious or buggy radix guests may have inserted SLB entries
+ * (only 0..3 because radix always runs with UPRT=1), so these must
+ * be cleared here to avoid side-channels. slbmte is used rather
+ * than slbia, as it won't clear cached translations.
+ */
+static void radix_clear_slb(void)
+{
+ int i;
+
+ for (i = 0; i < 4; i++)
+ clear_slb_entry(i);
+}
+
+int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu)
+{
+ u64 *exsave;
+ unsigned long msr = mfmsr();
+ int trap;
+
+ start_timing(vcpu, &vcpu->arch.rm_entry);
+
+ vcpu->arch.ceded = 0;
+
+ WARN_ON_ONCE(vcpu->arch.shregs.msr & MSR_HV);
+ WARN_ON_ONCE(!(vcpu->arch.shregs.msr & MSR_ME));
+
+ mtspr(SPRN_HSRR0, vcpu->arch.regs.nip);
+ mtspr(SPRN_HSRR1, (vcpu->arch.shregs.msr & ~MSR_HV) | MSR_ME);
+
+ /*
+ * On POWER9 DD2.1 and below, sometimes on a Hypervisor Data Storage
+ * Interrupt (HDSI) the HDSISR is not be updated at all.
+ *
+ * To work around this we put a canary value into the HDSISR before
+ * returning to a guest and then check for this canary when we take a
+ * HDSI. If we find the canary on a HDSI, we know the hardware didn't
+ * update the HDSISR. In this case we return to the guest to retake the
+ * HDSI which should correctly update the HDSISR the second time HDSI
+ * entry.
+ *
+ * Just do this on all p9 processors for now.
+ */
+ mtspr(SPRN_HDSISR, HDSISR_CANARY);
+
+ accumulate_time(vcpu, &vcpu->arch.guest_time);
+
+ local_paca->kvm_hstate.in_guest = KVM_GUEST_MODE_HV_FAST;
+ kvmppc_p9_enter_guest(vcpu);
+ // Radix host and guest means host never runs with guest MMU state
+ local_paca->kvm_hstate.in_guest = KVM_GUEST_MODE_NONE;
+
+ accumulate_time(vcpu, &vcpu->arch.rm_intr);
+
+ /* XXX: Could get these from r11/12 and paca exsave instead */
+ vcpu->arch.shregs.srr0 = mfspr(SPRN_SRR0);
+ vcpu->arch.shregs.srr1 = mfspr(SPRN_SRR1);
+ vcpu->arch.shregs.dar = mfspr(SPRN_DAR);
+ vcpu->arch.shregs.dsisr = mfspr(SPRN_DSISR);
+
+ /* 0x2 bit for HSRR is only used by PR and P7/8 HV paths, clear it */
+ trap = local_paca->kvm_hstate.scratch0 & ~0x2;
+ if (likely(trap > BOOK3S_INTERRUPT_MACHINE_CHECK)) {
+ exsave = local_paca->exgen;
+ } else if (trap == BOOK3S_INTERRUPT_SYSTEM_RESET) {
+ exsave = local_paca->exnmi;
+ } else { /* trap == 0x200 */
+ exsave = local_paca->exmc;
+ }
+
+ vcpu->arch.regs.gpr[1] = local_paca->kvm_hstate.scratch1;
+ vcpu->arch.regs.gpr[3] = local_paca->kvm_hstate.scratch2;
+ vcpu->arch.regs.gpr[9] = exsave[EX_R9/sizeof(u64)];
+ vcpu->arch.regs.gpr[10] = exsave[EX_R10/sizeof(u64)];
+ vcpu->arch.regs.gpr[11] = exsave[EX_R11/sizeof(u64)];
+ vcpu->arch.regs.gpr[12] = exsave[EX_R12/sizeof(u64)];
+ vcpu->arch.regs.gpr[13] = exsave[EX_R13/sizeof(u64)];
+ vcpu->arch.ppr = exsave[EX_PPR/sizeof(u64)];
+ vcpu->arch.cfar = exsave[EX_CFAR/sizeof(u64)];
+ vcpu->arch.regs.ctr = exsave[EX_CTR/sizeof(u64)];
+
+ vcpu->arch.last_inst = KVM_INST_FETCH_FAILED;
+
+ if (unlikely(trap == BOOK3S_INTERRUPT_MACHINE_CHECK)) {
+ vcpu->arch.fault_dar = exsave[EX_DAR/sizeof(u64)];
+ vcpu->arch.fault_dsisr = exsave[EX_DSISR/sizeof(u64)];
+ kvmppc_realmode_machine_check(vcpu);
+
+ } else if (unlikely(trap == BOOK3S_INTERRUPT_HMI)) {
+ kvmppc_realmode_hmi_handler();
+
+ } else if (trap == BOOK3S_INTERRUPT_H_EMUL_ASSIST) {
+ vcpu->arch.emul_inst = mfspr(SPRN_HEIR);
+
+ } else if (trap == BOOK3S_INTERRUPT_H_DATA_STORAGE) {
+ vcpu->arch.fault_dar = exsave[EX_DAR/sizeof(u64)];
+ vcpu->arch.fault_dsisr = exsave[EX_DSISR/sizeof(u64)];
+ vcpu->arch.fault_gpa = mfspr(SPRN_ASDR);
+
+ } else if (trap == BOOK3S_INTERRUPT_H_INST_STORAGE) {
+ vcpu->arch.fault_gpa = mfspr(SPRN_ASDR);
+
+ } else if (trap == BOOK3S_INTERRUPT_H_FAC_UNAVAIL) {
+ vcpu->arch.hfscr = mfspr(SPRN_HFSCR);
+
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ /*
+ * Softpatch interrupt for transactional memory emulation cases
+ * on POWER9 DD2.2. This is early in the guest exit path - we
+ * haven't saved registers or done a treclaim yet.
+ */
+ } else if (trap == BOOK3S_INTERRUPT_HV_SOFTPATCH) {
+ vcpu->arch.emul_inst = mfspr(SPRN_HEIR);
+
+ /*
+ * The cases we want to handle here are those where the guest
+ * is in real suspend mode and is trying to transition to
+ * transactional mode.
+ */
+ if (local_paca->kvm_hstate.fake_suspend &&
+ (vcpu->arch.shregs.msr & MSR_TS_S)) {
+ if (kvmhv_p9_tm_emulation_early(vcpu)) {
+ /* Prevent it being handled again. */
+ trap = 0;
+ }
+ }
+#endif
+ }
+
+ radix_clear_slb();
+
+ __mtmsrd(msr, 0);
+
+ accumulate_time(vcpu, &vcpu->arch.rm_exit);
+
+ end_timing(vcpu);
+
+ return trap;
+}
+EXPORT_SYMBOL_GPL(__kvmhv_vcpu_entry_p9);
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 2d0d14ed1d92..85c2595ead8d 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -44,9 +44,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
#define NAPPING_UNSPLIT 3
/* Stack frame offsets for kvmppc_hv_entry */
-#define SFS 208
+#define SFS 160
#define STACK_SLOT_TRAP (SFS-4)
-#define STACK_SLOT_SHORT_PATH (SFS-8)
#define STACK_SLOT_TID (SFS-16)
#define STACK_SLOT_PSSCR (SFS-24)
#define STACK_SLOT_PID (SFS-32)
@@ -59,8 +58,6 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
#define STACK_SLOT_UAMOR (SFS-88)
#define STACK_SLOT_DAWR1 (SFS-96)
#define STACK_SLOT_DAWRX1 (SFS-104)
-/* the following is used by the P9 short path */
-#define STACK_SLOT_NVGPRS (SFS-152) /* 18 gprs */
/*
* Call kvmppc_hv_entry in real mode.
@@ -1008,9 +1005,6 @@ END_MMU_FTR_SECTION_IFCLR(MMU_FTR_TYPE_RADIX)
no_xive:
#endif /* CONFIG_KVM_XICS */
- li r0, 0
- stw r0, STACK_SLOT_SHORT_PATH(r1)
-
deliver_guest_interrupt: /* r4 = vcpu, r13 = paca */
/* Check if we can deliver an external or decrementer interrupt now */
ld r0, VCPU_PENDING_EXC(r4)
@@ -1030,7 +1024,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
mtspr SPRN_SRR0, r6
mtspr SPRN_SRR1, r7
-fast_guest_entry_c:
ld r10, VCPU_PC(r4)
ld r11, VCPU_MSR(r4)
/* r11 = vcpu->arch.msr & ~MSR_HV */
@@ -1135,97 +1128,6 @@ ret_to_ultra:
ld r4, VCPU_GPR(R4)(r4)
sc 2
-/*
- * Enter the guest on a P9 or later system where we have exactly
- * one vcpu per vcore and we don't need to go to real mode
- * (which implies that host and guest are both using radix MMU mode).
- * r3 = vcpu pointer
- * Most SPRs and all the VSRs have been loaded already.
- */
-_GLOBAL(__kvmhv_vcpu_entry_p9)
-EXPORT_SYMBOL_GPL(__kvmhv_vcpu_entry_p9)
- mflr r0
- std r0, PPC_LR_STKOFF(r1)
- stdu r1, -SFS(r1)
-
- li r0, 1
- stw r0, STACK_SLOT_SHORT_PATH(r1)
-
- std r3, HSTATE_KVM_VCPU(r13)
- mfcr r4
- stw r4, SFS+8(r1)
-
- std r1, HSTATE_HOST_R1(r13)
-
- reg = 14
- .rept 18
- std reg, STACK_SLOT_NVGPRS + ((reg - 14) * 8)(r1)
- reg = reg + 1
- .endr
-
- reg = 14
- .rept 18
- ld reg, __VCPU_GPR(reg)(r3)
- reg = reg + 1
- .endr
-
- mfmsr r10
- std r10, HSTATE_HOST_MSR(r13)
-
- mr r4, r3
- b fast_guest_entry_c
-guest_exit_short_path:
- /*
- * Malicious or buggy radix guests may have inserted SLB entries
- * (only 0..3 because radix always runs with UPRT=1), so these must
- * be cleared here to avoid side-channels. slbmte is used rather
- * than slbia, as it won't clear cached translations.
- */
- li r0,0
- slbmte r0,r0
- li r4,1
- slbmte r0,r4
- li r4,2
- slbmte r0,r4
- li r4,3
- slbmte r0,r4
-
- li r0, KVM_GUEST_MODE_NONE
- stb r0, HSTATE_IN_GUEST(r13)
-
- reg = 14
- .rept 18
- std reg, __VCPU_GPR(reg)(r9)
- reg = reg + 1
- .endr
-
- reg = 14
- .rept 18
- ld reg, STACK_SLOT_NVGPRS + ((reg - 14) * 8)(r1)
- reg = reg + 1
- .endr
-
- lwz r4, SFS+8(r1)
- mtcr r4
-
- mr r3, r12 /* trap number */
-
- addi r1, r1, SFS
- ld r0, PPC_LR_STKOFF(r1)
- mtlr r0
-
- /* If we are in real mode, do a rfid to get back to the caller */
- mfmsr r4
- andi. r5, r4, MSR_IR
- bnelr
- rldicl r5, r4, 64 - MSR_TS_S_LG, 62 /* extract TS field */
- mtspr SPRN_SRR0, r0
- ld r10, HSTATE_HOST_MSR(r13)
- rldimi r10, r5, MSR_TS_S_LG, 63 - MSR_TS_T_LG
- mtspr SPRN_SRR1, r10
- RFI_TO_KERNEL
- b .
-
secondary_too_late:
li r12, 0
stw r12, STACK_SLOT_TRAP(r1)
@@ -1397,14 +1299,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
mr r4,r9
bge fast_guest_return
2:
- /* If we came in through the P9 short path, no real mode hcalls */
- lwz r0, STACK_SLOT_SHORT_PATH(r1)
- cmpwi r0, 0
- bne no_try_real
/* See if this is an hcall we can handle in real mode */
cmpwi r12,BOOK3S_INTERRUPT_SYSCALL
beq hcall_try_real_mode
-no_try_real:
/* Hypervisor doorbell - exit only if host IPI flag set */
cmpwi r12, BOOK3S_INTERRUPT_H_DOORBELL
@@ -1447,11 +1344,6 @@ guest_exit_cont: /* r9 = vcpu, r12 = trap, r13 = paca */
bl kvmhv_accumulate_time
#endif
#ifdef CONFIG_KVM_XICS
- /* If we came in through the P9 short path, xive pull is done in C */
- lwz r0, STACK_SLOT_SHORT_PATH(r1)
- cmpwi r0, 0
- bne 1f
-
/* We are exiting, pull the VP from the XIVE */
lbz r0, VCPU_XIVE_PUSHED(r9)
cmpwi cr0, r0, 0
@@ -1491,16 +1383,11 @@ guest_exit_cont: /* r9 = vcpu, r12 = trap, r13 = paca */
/*
* Possibly flush the link stack here, before we do a blr in
- * guest_exit_short_path.
+ * kvmhv_switch_to_host.
*/
1: nop
patch_site 1b patch__call_kvm_flush_link_stack
- /* If we came in through the P9 short path, go back out to C now */
- lwz r0, STACK_SLOT_SHORT_PATH(r1)
- cmpwi r0, 0
- bne guest_exit_short_path
-
/* For hash guest, read the guest SLB and save it away */
ld r5, VCPU_KVM(r9)
lbz r0, KVM_RADIX(r5)
@@ -1548,8 +1435,10 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
b guest_bypass
0: /*
- * Sanitise radix guest SLB, see guest_exit_short_path comment.
- * We clear vcpu->arch.slb_max to match earlier behaviour.
+ * Malicious or buggy radix guests may have inserted SLB entries
+ * (only 0..3 because radix always runs with UPRT=1), so these must
+ * be cleared here to avoid side-channels. slbmte is used rather
+ * than slbia, as it won't clear cached translations.
*/
li r0,0
stw r0,VCPU_SLB_MAX(r9)
@@ -3362,7 +3251,7 @@ BEGIN_FTR_SECTION
mtspr SPRN_DAWRX1, r0
END_FTR_SECTION_IFSET(CPU_FTR_DAWR1)
- /* Clear hash and radix guest SLB, see guest_exit_short_path comment. */
+ /* Clear hash and radix guest SLB. */
slbmte r0, r0
PPC_SLBIA(6)
--
2.23.0
^ permalink raw reply related
* Re: [RFC 1/4] drivers/nvdimm: Add perf interface to expose nvdimm performance stats
From: kajoljain @ 2021-05-13 12:26 UTC (permalink / raw)
To: Peter Zijlstra
Cc: santosh, maddy, ira.weiny, linux-nvdimm, linux-kernel, atrajeev,
aneesh.kumar, vaibhav, dan.j.williams, linuxppc-dev, tglx
In-Reply-To: <YJwP9ByvAcDPixVN@hirez.programming.kicks-ass.net>
On 5/12/21 10:57 PM, Peter Zijlstra wrote:
> On Wed, May 12, 2021 at 10:08:21PM +0530, Kajol Jain wrote:
>> +static void nvdimm_pmu_read(struct perf_event *event)
>> +{
>> + struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
>> +
>> + /* jump to arch/platform specific callbacks if any */
>> + if (nd_pmu && nd_pmu->read)
>> + nd_pmu->read(event, nd_pmu->dev);
>> +}
>> +
>> +static void nvdimm_pmu_del(struct perf_event *event, int flags)
>> +{
>> + struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
>> +
>> + /* jump to arch/platform specific callbacks if any */
>> + if (nd_pmu && nd_pmu->del)
>> + nd_pmu->del(event, flags, nd_pmu->dev);
>> +}
>> +
>> +static int nvdimm_pmu_add(struct perf_event *event, int flags)
>> +{
>> + struct nvdimm_pmu *nd_pmu = to_nvdimm_pmu(event->pmu);
>> +
>> + if (flags & PERF_EF_START)
>> + /* jump to arch/platform specific callbacks if any */
>> + if (nd_pmu && nd_pmu->add)
>> + return nd_pmu->add(event, flags, nd_pmu->dev);
>> + return 0;
>> +}
>
> What's the value add here? Why can't you directly set driver pointers? I
> also don't really believe ->{add,del,read} can be optional and still
> have a sane driver.
>
Hi Peter,
The intend for adding these callbacks is to give flexibility to the
arch/platform specific driver code to use its own routine for getting
counter data or specific checks/operations. Arch/platform driver code
would have different method to get the counter data like IBM pseries
nmem* device which uses a hypervisor call(hcall).
But yes the current read/add/del functions are not adding value. We
could add an arch/platform specific function which could handle the
capturing of the counter data and do the rest of the operation here,
is this approach better?
Thanks,
Kajol Jain
^ permalink raw reply
* Re: [PATCH 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-13 12:49 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: Tyrel Datwyler, linux-pci, Oliver O'Halloran, Bjorn Helgaas,
Paul Mackerras, Kurt Schwemmer, linuxppc-dev
In-Reply-To: <4557364b-76ce-3555-e97d-14f39eda27b8@deltatee.com>
Hi Logan,
[...]
> Thanks, this is a great cleanup. I've reviewed the entire series.
>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Thank you! Appreciate it!
> I agree that the new lines that are missing should be added.
While working on the simple change to add the missing new lines, I've
found that we have slight inconsistency with handling these in one of
the attributes, as per:
# uname -r
5.13.0-rc1-00092-gc06a2ba62fc4
# grep -oE 'pci=resource_alignment.+' /proc/cmdline
pci=resource_alignment=20@00:1f.2
# cat /sys/bus/pci/resource_alignment
20@00:1f.
# echo 20@00:1f.2 > /sys/bus/pci/resource_alignment
# cat /sys/bus/pci/resource_alignment
20@00:1f.2
# echo -n 20@00:1f.2 > /sys/bus/pci/resource_alignment
# cat /sys/bus/pci/resource_alignment
20@00:1f.
I adjusted the code so that we handle the input better in the upcoming
v2 - the change is simple, but since it adds more code, I'll drop your
"Reviewed-by", so that we can make sure that subsequent review will
cover this new change. Sorry for troubles in advance!
Krzysztof
^ permalink raw reply
* [PATCH 2/2] powerpc/64s: Fix stf mitigation patching w/strict RWX & hash
From: Michael Ellerman @ 2021-05-13 14:08 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20210513140800.1391706-1-mpe@ellerman.id.au>
The stf entry barrier fallback is unsafe to execute in a semi-patched
state, which can happen when enabling/disabling the mitigation with
strict kernel RWX enabled and using the hash MMU.
See the previous commit for more details.
Fix it by changing the order in which we patch the instructions.
Note the stf barrier fallback is only used on Power6 or earlier.
Fixes: bd573a81312f ("powerpc/mm/64s: Allow STRICT_KERNEL_RWX again")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/lib/feature-fixups.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index b49bb41e3ec5..71032475aa40 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -150,17 +150,17 @@ static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
-
- if (types & STF_BARRIER_FALLBACK)
+ // See comment in do_entry_flush_fixups() RE order of patching
+ if (types & STF_BARRIER_FALLBACK) {
+ patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
patch_branch((struct ppc_inst *)(dest + 1),
- (unsigned long)&stf_barrier_fallback,
- BRANCH_SET_LINK);
- else
- patch_instruction((struct ppc_inst *)(dest + 1),
- ppc_inst(instrs[1]));
-
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ (unsigned long)&stf_barrier_fallback, BRANCH_SET_LINK);
+ } else {
+ patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
+ patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ }
}
printk(KERN_DEBUG "stf-barrier: patched %d entry locations (%s barrier)\n", i,
--
2.25.1
^ permalink raw reply related
* [PATCH 1/2] powerpc/64s: Fix entry flush patching w/strict RWX & hash
From: Michael Ellerman @ 2021-05-13 14:07 UTC (permalink / raw)
To: linuxppc-dev
The entry flush mitigation can be enabled/disabled at runtime. When this
happens it results in the kernel patching its own instructions to
enable/disable the mitigation sequence.
With strict kernel RWX enabled instruction patching happens via a
secondary mapping of the kernel text, so that we don't have to make the
primary mapping writable. With the hash MMU this leads to a hash fault,
which causes us to execute the exception entry which contains the entry
flush mitigation.
This means we end up executing the entry flush in a semi-patched state,
ie. after we have patched the first instruction but before we patch the
second or third instruction of the sequence.
On machines with updated firmware the entry flush is a series of special
nops, and it's safe to to execute in a semi-patched state.
However when using the fallback flush the sequence is mflr/branch/mtlr,
and so it's not safe to execute if we have patched out the mflr but not
the other two instructions. Doing so leads to us corrputing LR, leading
to an oops, for example:
# echo 0 > /sys/kernel/debug/powerpc/entry_flush
kernel tried to execute exec-protected page (c000000002971000) - exploit attempt? (uid: 0)
BUG: Unable to handle kernel instruction fetch
Faulting instruction address: 0xc000000002971000
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
CPU: 0 PID: 2215 Comm: bash Not tainted 5.13.0-rc1-00010-gda3bb206c9ce #1
NIP: c000000002971000 LR: c000000002971000 CTR: c000000000120c40
REGS: c000000013243840 TRAP: 0400 Not tainted (5.13.0-rc1-00010-gda3bb206c9ce)
MSR: 8000000010009033 <SF,EE,ME,IR,DR,RI,LE> CR: 48428482 XER: 00000000
...
NIP 0xc000000002971000
LR 0xc000000002971000
Call Trace:
do_patch_instruction+0xc4/0x340 (unreliable)
do_entry_flush_fixups+0x100/0x3b0
entry_flush_set+0x50/0xe0
simple_attr_write+0x160/0x1a0
full_proxy_write+0x8c/0x110
vfs_write+0xf0/0x340
ksys_write+0x84/0x140
system_call_exception+0x164/0x2d0
system_call_common+0xec/0x278
The simplest fix is to change the order in which we patch the
instructions, so that the sequence is always safe to execute. For the
non-fallback flushes it doesn't matter what order we patch in.
Fixes: bd573a81312f ("powerpc/mm/64s: Allow STRICT_KERNEL_RWX again")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/lib/feature-fixups.c | 56 ++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 0aefa6a4a259..b49bb41e3ec5 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -325,6 +325,28 @@ static int __do_entry_flush_fixups(void *data)
if (types & L1D_FLUSH_MTTRIG)
instrs[i++] = 0x7c12dba6; /* mtspr TRIG2,r0 (SPR #882) */
+ /*
+ * If we're patching in or out the fallback flush we need to be careful about the
+ * order in which we patch instructions. That's because it's possible we could
+ * take a page fault after patching one instruction, so the sequence of
+ * instructions must be safe even in a half patched state.
+ *
+ * To make that work, when patching in the fallback flush we patch in this order:
+ * - the mflr (dest)
+ * - the mtlr (dest + 2)
+ * - the branch (dest + 1)
+ *
+ * That ensures the sequence is safe to execute at any point. In contrast if we
+ * patch the mtlr last, it's possible we could return from the branch and not
+ * restore LR, leading to a crash later.
+ *
+ * When patching out the fallback flush (either with nops or another flush type),
+ * we patch in this order:
+ * - the branch (dest + 1)
+ * - the mtlr (dest + 2)
+ * - the mflr (dest)
+ */
+
start = PTRRELOC(&__start___entry_flush_fixup);
end = PTRRELOC(&__stop___entry_flush_fixup);
for (i = 0; start < end; start++, i++) {
@@ -332,15 +354,16 @@ static int __do_entry_flush_fixups(void *data)
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
-
- if (types == L1D_FLUSH_FALLBACK)
- patch_branch((struct ppc_inst *)(dest + 1), (unsigned long)&entry_flush_fallback,
- BRANCH_SET_LINK);
- else
+ if (types == L1D_FLUSH_FALLBACK) {
+ patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_branch((struct ppc_inst *)(dest + 1),
+ (unsigned long)&entry_flush_fallback, BRANCH_SET_LINK);
+ } else {
patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
-
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ }
}
start = PTRRELOC(&__start___scv_entry_flush_fixup);
@@ -350,15 +373,16 @@ static int __do_entry_flush_fixups(void *data)
pr_devel("patching dest %lx\n", (unsigned long)dest);
- patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
-
- if (types == L1D_FLUSH_FALLBACK)
- patch_branch((struct ppc_inst *)(dest + 1), (unsigned long)&scv_entry_flush_fallback,
- BRANCH_SET_LINK);
- else
+ if (types == L1D_FLUSH_FALLBACK) {
+ patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_branch((struct ppc_inst *)(dest + 1),
+ (unsigned long)&scv_entry_flush_fallback, BRANCH_SET_LINK);
+ } else {
patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
-
- patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+ patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+ }
}
--
2.25.1
^ permalink raw reply related
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christian Zigotzky @ 2021-05-13 15:19 UTC (permalink / raw)
To: Christophe Leroy
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <c90ba856-1ef2-c010-17ac-a091c5252a5c@csgroup.eu>
On 13 May 2021 at 12:01 pm, Christophe Leroy wrote:
>
>
> Le 13/05/2021 à 08:47, Christian Zigotzky a écrit :
>> Hi Christophe,
>>
>>> On 9. May 2021, at 19:37, Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>
>>> Did I do an error in my analysis ?
>>
>> No, you didn’t. On the contrary you have found the issue. ;-) The
>> issue is somewhere in the new interrupt code.
>
> I'm not convinced, but let's give it a try.
>
>>
>>> ZZ | * | ceff77efa4f8 powerpc/64e/interrupt: Use new interrupt
>>> context tracking scheme
>>
>> Could you please create a patch for reverting the new interrupt code?
>> I would like to confirm your result.
>
> Please fetch https://github.com/chleroy/linux.git and try the branch
> for_christian.
>
> This is a revert of approx a dozen of commits around the changes to
> 64e on top of v5.13-rc1.
>
> If the top of the branch works for you, it would be great if you can
> find out which one of the reverts fixes the problem for real.
>
> Thanks
> Christophe
It's working! Great! I can use the RC1 on my FSL P5040. Thank you! The
issue is definitely somewhere in the interrupt code. Please tell me the
next steps.
- Christian
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christophe Leroy @ 2021-05-13 15:51 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <781a426e-8561-108f-0e5c-9393bc653e86@xenosoft.de>
Le 13/05/2021 à 17:19, Christian Zigotzky a écrit :
> On 13 May 2021 at 12:01 pm, Christophe Leroy wrote:
>>
>>
>> Le 13/05/2021 à 08:47, Christian Zigotzky a écrit :
>>> Hi Christophe,
>>>
>>>> On 9. May 2021, at 19:37, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>
>>>> Did I do an error in my analysis ?
>>>
>>> No, you didn’t. On the contrary you have found the issue. ;-) The issue is somewhere in the new
>>> interrupt code.
>>
>> I'm not convinced, but let's give it a try.
>>
>>>
>>>> ZZ | * | ceff77efa4f8 powerpc/64e/interrupt: Use new interrupt context tracking scheme
>>>
>>> Could you please create a patch for reverting the new interrupt code? I would like to confirm
>>> your result.
>>
>> Please fetch https://github.com/chleroy/linux.git and try the branch for_christian.
>>
>> This is a revert of approx a dozen of commits around the changes to 64e on top of v5.13-rc1.
>>
>> If the top of the branch works for you, it would be great if you can find out which one of the
>> reverts fixes the problem for real.
>>
>> Thanks
>> Christophe
> It's working! Great! I can use the RC1 on my FSL P5040. Thank you! The issue is definitely somewhere
> in the interrupt code. Please tell me the next steps.
>
Can you bisect between 5.13-rc1 and the top of the 'for_christian' branch to find the first 'good'
commit ?
Take care it is an "up side down" bisect, a 'good' is one that does _not_ work and a 'bad' is a
commit that works.
git bisect start
git bisect bad 1c8f441f1485
git bisect good 6efb943b8616
Christophe
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christian Zigotzky @ 2021-05-13 16:35 UTC (permalink / raw)
To: Christophe Leroy
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <56899373-f821-62e2-26ab-b1a98b3fe2e2@csgroup.eu>
On 13 May 2021 at 5:51pm, Christophe Leroy wrote:
>
>
> Le 13/05/2021 à 17:19, Christian Zigotzky a écrit :
>> On 13 May 2021 at 12:01 pm, Christophe Leroy wrote:
>>>
>>>
>>> Le 13/05/2021 à 08:47, Christian Zigotzky a écrit :
>>>> Hi Christophe,
>>>>
>>>>> On 9. May 2021, at 19:37, Christophe Leroy
>>>>> <christophe.leroy@csgroup.eu> wrote:
>>>>>
>>>>> Did I do an error in my analysis ?
>>>>
>>>> No, you didn’t. On the contrary you have found the issue. ;-) The
>>>> issue is somewhere in the new interrupt code.
>>>
>>> I'm not convinced, but let's give it a try.
>>>
>>>>
>>>>> ZZ | * | ceff77efa4f8 powerpc/64e/interrupt: Use new interrupt
>>>>> context tracking scheme
>>>>
>>>> Could you please create a patch for reverting the new interrupt
>>>> code? I would like to confirm your result.
>>>
>>> Please fetch https://github.com/chleroy/linux.git and try the branch
>>> for_christian.
>>>
>>> This is a revert of approx a dozen of commits around the changes to
>>> 64e on top of v5.13-rc1.
>>>
>>> If the top of the branch works for you, it would be great if you can
>>> find out which one of the reverts fixes the problem for real.
>>>
>>> Thanks
>>> Christophe
>> It's working! Great! I can use the RC1 on my FSL P5040. Thank you!
>> The issue is definitely somewhere in the interrupt code. Please tell
>> me the next steps.
>>
>
> Can you bisect between 5.13-rc1 and the top of the 'for_christian'
> branch to find the first 'good' commit ?
>
> Take care it is an "up side down" bisect, a 'good' is one that does
> _not_ work and a 'bad' is a commit that works.
>
> git bisect start
> git bisect bad 1c8f441f1485
> git bisect good 6efb943b8616
>
> Christophe
Hi Christophe,
Yes, I can. Shall I use the branch 'for_christian' or the default linux
git for bisecting? I have tried it already with the branch
'for_christian' but it doesn't compile.
git bisect start
git bisect bad 1c8f441f1485
git bisect good 6efb943b8616
Output: [d66b1d1aab0c1caad11eca417f515b86ec0bebe9] Revert
"powerpc/64e/interrupt: Use new interrupt context tracking scheme"
Result:
arch/powerpc/kernel/interrupt.o: In function `.syscall_exit_prepare':
interrupt.c:(.text+0x278): undefined reference to `.schedule_user'
arch/powerpc/kernel/interrupt.o: In function `.interrupt_exit_user_prepare':
interrupt.c:(.text+0x340): undefined reference to `.schedule_user'
Makefile:1191: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1
----
Thanks,
Christian
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christophe Leroy @ 2021-05-13 17:00 UTC (permalink / raw)
To: Christian Zigotzky
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <4175f794-f8b8-d9fc-620d-408317e27eba@xenosoft.de>
Le 13/05/2021 à 18:35, Christian Zigotzky a écrit :
> On 13 May 2021 at 5:51pm, Christophe Leroy wrote:
>>
>>
>> Le 13/05/2021 à 17:19, Christian Zigotzky a écrit :
>>> On 13 May 2021 at 12:01 pm, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 13/05/2021 à 08:47, Christian Zigotzky a écrit :
>>>>> Hi Christophe,
>>>>>
>>>>>> On 9. May 2021, at 19:37, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>>>>>
>>>>>> Did I do an error in my analysis ?
>>>>>
>>>>> No, you didn’t. On the contrary you have found the issue. ;-) The issue is somewhere in the new
>>>>> interrupt code.
>>>>
>>>> I'm not convinced, but let's give it a try.
>>>>
>>>>>
>>>>>> ZZ | * | ceff77efa4f8 powerpc/64e/interrupt: Use new interrupt context tracking scheme
>>>>>
>>>>> Could you please create a patch for reverting the new interrupt code? I would like to confirm
>>>>> your result.
>>>>
>>>> Please fetch https://github.com/chleroy/linux.git and try the branch for_christian.
>>>>
>>>> This is a revert of approx a dozen of commits around the changes to 64e on top of v5.13-rc1.
>>>>
>>>> If the top of the branch works for you, it would be great if you can find out which one of the
>>>> reverts fixes the problem for real.
>>>>
>>>> Thanks
>>>> Christophe
>>> It's working! Great! I can use the RC1 on my FSL P5040. Thank you! The issue is definitely
>>> somewhere in the interrupt code. Please tell me the next steps.
>>>
>>
>> Can you bisect between 5.13-rc1 and the top of the 'for_christian' branch to find the first 'good'
>> commit ?
>>
>> Take care it is an "up side down" bisect, a 'good' is one that does _not_ work and a 'bad' is a
>> commit that works.
>>
>> git bisect start
>> git bisect bad 1c8f441f1485
>> git bisect good 6efb943b8616
>>
>> Christophe
> Hi Christophe,
>
> Yes, I can. Shall I use the branch 'for_christian' or the default linux git for bisecting? I have
> tried it already with the branch 'for_christian' but it doesn't compile.
>
> git bisect start
> git bisect bad 1c8f441f1485
> git bisect good 6efb943b8616
>
> Output: [d66b1d1aab0c1caad11eca417f515b86ec0bebe9] Revert "powerpc/64e/interrupt: Use new interrupt
> context tracking scheme"
>
> Result:
>
> arch/powerpc/kernel/interrupt.o: In function `.syscall_exit_prepare':
> interrupt.c:(.text+0x278): undefined reference to `.schedule_user'
> arch/powerpc/kernel/interrupt.o: In function `.interrupt_exit_user_prepare':
> interrupt.c:(.text+0x340): undefined reference to `.schedule_user'
> Makefile:1191: recipe for target 'vmlinux' failed
> make: *** [vmlinux] Error 1
>
Ah yes, I remember this problem.
Can you select CONFIG_VIRT_CPU_ACCOUNTING_GEN in your configuration ?
Otherwise, I can try to fix the branch.
Christophe
^ permalink raw reply
* [PATCH] x86: Define only {pud/pmd}_{set/clear}_huge when usefull
From: Christophe Leroy @ 2021-05-13 18:27 UTC (permalink / raw)
To: Andrew Morton, Nicholas Piggin, Mike Kravetz, Mike Rapoport,
rdunlap
Cc: linux-arch, linux-kernel, linux-mm, sparclinux, linuxppc-dev,
linux-arm-kernel
When PUD and/or PMD are folded, those functions are useless
and we have a stub in linux/pgtable.h
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/x86/mm/pgtable.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index d27cf69e811d..1303ff6ef7be 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -682,6 +682,7 @@ int p4d_clear_huge(p4d_t *p4d)
}
#endif
+#if CONFIG_PGTABLE_LEVELS > 3
/**
* pud_set_huge - setup kernel PUD mapping
*
@@ -720,6 +721,23 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
return 1;
}
+/**
+ * pud_clear_huge - clear kernel PUD mapping when it is set
+ *
+ * Returns 1 on success and 0 on failure (no PUD map is found).
+ */
+int pud_clear_huge(pud_t *pud)
+{
+ if (pud_large(*pud)) {
+ pud_clear(pud);
+ return 1;
+ }
+
+ return 0;
+}
+#endif
+
+#if CONFIG_PGTABLE_LEVELS > 2
/**
* pmd_set_huge - setup kernel PMD mapping
*
@@ -750,21 +768,6 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
return 1;
}
-/**
- * pud_clear_huge - clear kernel PUD mapping when it is set
- *
- * Returns 1 on success and 0 on failure (no PUD map is found).
- */
-int pud_clear_huge(pud_t *pud)
-{
- if (pud_large(*pud)) {
- pud_clear(pud);
- return 1;
- }
-
- return 0;
-}
-
/**
* pmd_clear_huge - clear kernel PMD mapping when it is set
*
@@ -779,6 +782,7 @@ int pmd_clear_huge(pmd_t *pmd)
return 0;
}
+#endif
#ifdef CONFIG_X86_64
/**
--
2.25.0
^ permalink raw reply related
* Re: [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Nathan Chancellor @ 2021-05-13 18:59 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, Nick Desaulniers,
linux-kernel, clang-built-linux
In-Reply-To: <20210513115904.519912-1-aik@ozlabs.ru>
On 5/13/2021 4:59 AM, Alexey Kardashevskiy wrote:
> The $(CPP) (do only preprocessing) macro is already defined in Makefile.
> However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
> in flags duplication. Which is not a big deal by itself except for
> the flags which depend on other flags and the compiler checks them
> as it parses the command line.
>
> Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
> If clang+llvm+sanitizer are enabled, this results in
>
> -emit-llvm-bc -fno-lto -flto -fvisibility=hidden \
> -fsanitize=cfi-mfcall -fno-lto ...
>
> in the clang command line and triggers error:
>
> clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed with '-flto'
>
> This removes unnecessary CPP redefinition. Which works fine as in most
> place KBUILD_CFLAGS is passed to $CPP except
> arch/powerpc/kernel/vdso64/vdso(32|64).lds. To fix vdso, this does:
> 1. add -m(big|little)-endian to $CPP
> 2. add target to $KBUILD_CPPFLAGS as otherwise clang ignores -m(big|little)-endian if
> the building platform does not support big endian (such as x86).
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v3:
> * moved vdso cleanup in a separate patch
> * only add target to KBUILD_CPPFLAGS for CLANG
>
> v2:
> * fix KBUILD_CPPFLAGS
> * add CLANG_FLAGS to CPPFLAGS
> ---
> Makefile | 1 +
> arch/powerpc/Makefile | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 15b6476d0f89..5b545bef7653 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -576,6 +576,7 @@ CC_VERSION_TEXT = $(subst $(pound),,$(shell $(CC) --version 2>/dev/null | head -
> ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
> ifneq ($(CROSS_COMPILE),)
> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> +KBUILD_CPPFLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
You can avoid the duplication here by just doing:
KBUILD_CPPFLAGS += $(CLANG_FLAGS)
I am still not super happy about the flag duplication but I am not sure
I can think of a better solution. If KBUILD_CPPFLAGS are always included
when building .o files, maybe we should just add $(CLANG_FLAGS) to
KBUILD_CPPFLAGS instead of KBUILD_CFLAGS?
> endif
> ifeq ($(LLVM_IAS),1)
> CLANG_FLAGS += -integrated-as
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 3212d076ac6a..306bfd2797ad 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -76,6 +76,7 @@ endif
>
> ifdef CONFIG_CPU_LITTLE_ENDIAN
> KBUILD_CFLAGS += -mlittle-endian
> +KBUILD_CPPFLAGS += -mlittle-endian
> KBUILD_LDFLAGS += -EL
> LDEMULATION := lppc
> GNUTARGET := powerpcle
> @@ -83,6 +84,7 @@ MULTIPLEWORD := -mno-multiple
> KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
> else
> KBUILD_CFLAGS += $(call cc-option,-mbig-endian)
> +KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
> KBUILD_LDFLAGS += -EB
> LDEMULATION := ppc
> GNUTARGET := powerpc
> @@ -208,7 +210,6 @@ KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH) $(asinstr)
> KBUILD_AFLAGS += $(AFLAGS-y)
> KBUILD_CFLAGS += $(call cc-option,-msoft-float)
> KBUILD_CFLAGS += -pipe $(CFLAGS-y)
> -CPP = $(CC) -E $(KBUILD_CFLAGS)
>
> CHECKFLAGS += -m$(BITS) -D__powerpc__ -D__powerpc$(BITS)__
> ifdef CONFIG_CPU_BIG_ENDIAN
>
^ permalink raw reply
* Re: [PATCH] x86: Define only {pud/pmd}_{set/clear}_huge when usefull
From: Randy Dunlap @ 2021-05-13 19:56 UTC (permalink / raw)
To: Christophe Leroy, Andrew Morton, Nicholas Piggin, Mike Kravetz,
Mike Rapoport
Cc: linux-arch, linux-kernel, linux-mm, sparclinux, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <7fbf1b6bc3e15c07c24fa45278d57064f14c896b.1620930415.git.christophe.leroy@csgroup.eu>
On 5/13/21 11:27 AM, Christophe Leroy wrote:
> When PUD and/or PMD are folded, those functions are useless
> and we have a stub in linux/pgtable.h
>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/x86/mm/pgtable.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
WorksForMe. Thanks.
Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
--
~Randy
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christian Zigotzky @ 2021-05-13 20:20 UTC (permalink / raw)
To: Christophe Leroy
Cc: Darren Stevens, R.T.Dickinson, mad skateman, linuxppc-dev,
Christian Zigotzky
In-Reply-To: <ea99d32a-b544-e813-8e0f-1719f3600ba9@csgroup.eu>
On 13 May 2021 at 07:00pm, Christophe Leroy wrote:
>
>
> Le 13/05/2021 à 18:35, Christian Zigotzky a écrit :
>> On 13 May 2021 at 5:51pm, Christophe Leroy wrote:
>>>
>>>
>>> Le 13/05/2021 à 17:19, Christian Zigotzky a écrit :
>>>> On 13 May 2021 at 12:01 pm, Christophe Leroy wrote:
>>>>>
>>>>>
>>>>> Le 13/05/2021 à 08:47, Christian Zigotzky a écrit :
>>>>>> Hi Christophe,
>>>>>>
>>>>>>> On 9. May 2021, at 19:37, Christophe Leroy
>>>>>>> <christophe.leroy@csgroup.eu> wrote:
>>>>>>>
>>>>>>> Did I do an error in my analysis ?
>>>>>>
>>>>>> No, you didn’t. On the contrary you have found the issue. ;-) The
>>>>>> issue is somewhere in the new interrupt code.
>>>>>
>>>>> I'm not convinced, but let's give it a try.
>>>>>
>>>>>>
>>>>>>> ZZ | * | ceff77efa4f8 powerpc/64e/interrupt: Use new interrupt
>>>>>>> context tracking scheme
>>>>>>
>>>>>> Could you please create a patch for reverting the new interrupt
>>>>>> code? I would like to confirm your result.
>>>>>
>>>>> Please fetch https://github.com/chleroy/linux.git and try the
>>>>> branch for_christian.
>>>>>
>>>>> This is a revert of approx a dozen of commits around the changes
>>>>> to 64e on top of v5.13-rc1.
>>>>>
>>>>> If the top of the branch works for you, it would be great if you
>>>>> can find out which one of the reverts fixes the problem for real.
>>>>>
>>>>> Thanks
>>>>> Christophe
>>>> It's working! Great! I can use the RC1 on my FSL P5040. Thank you!
>>>> The issue is definitely somewhere in the interrupt code. Please
>>>> tell me the next steps.
>>>>
>>>
>>> Can you bisect between 5.13-rc1 and the top of the 'for_christian'
>>> branch to find the first 'good' commit ?
>>>
>>> Take care it is an "up side down" bisect, a 'good' is one that does
>>> _not_ work and a 'bad' is a commit that works.
>>>
>>> git bisect start
>>> git bisect bad 1c8f441f1485
>>> git bisect good 6efb943b8616
>>>
>>> Christophe
>> Hi Christophe,
>>
>> Yes, I can. Shall I use the branch 'for_christian' or the default
>> linux git for bisecting? I have tried it already with the branch
>> 'for_christian' but it doesn't compile.
>>
>> git bisect start
>> git bisect bad 1c8f441f1485
>> git bisect good 6efb943b8616
>>
>> Output: [d66b1d1aab0c1caad11eca417f515b86ec0bebe9] Revert
>> "powerpc/64e/interrupt: Use new interrupt context tracking scheme"
>>
>> Result:
>>
>> arch/powerpc/kernel/interrupt.o: In function `.syscall_exit_prepare':
>> interrupt.c:(.text+0x278): undefined reference to `.schedule_user'
>> arch/powerpc/kernel/interrupt.o: In function
>> `.interrupt_exit_user_prepare':
>> interrupt.c:(.text+0x340): undefined reference to `.schedule_user'
>> Makefile:1191: recipe for target 'vmlinux' failed
>> make: *** [vmlinux] Error 1
>>
>
> Ah yes, I remember this problem.
>
> Can you select CONFIG_VIRT_CPU_ACCOUNTING_GEN in your configuration ?
>
> Otherwise, I can try to fix the branch.
>
> Christophe
I selected this. After that it compiles.
1. git bisect good - Xorg restarts again and again
Output: [f9aa0ac1e9e82b60401ad567bdabc30598325bc1] Revert
"powerpc/64e/interrupt: use new interrupt return"
2. git bisect good - Xorg restarts again and again
Output: [cd6d259a14704741bf0cd1dcadb84c0de22d7f77] Revert
"powerpc/64e/interrupt: always save nvgprs on interrupt"
3. git bisect bad - Xorg works
Output: [9bfa20ef2ae54d3b9088dfbcde4ef97062cf5ef2] Revert
"powerpc/interrupt: update common interrupt code for"
4. git bisect good - Xorg restarts again and again
Output:
cd6d259a14704741bf0cd1dcadb84c0de22d7f77 is the first bad commit
commit cd6d259a14704741bf0cd1dcadb84c0de22d7f77
Author: Christophe Leroy <christophe.leroy@csgroup.eu>
Date: Thu May 13 09:52:06 2021 +0000
Revert "powerpc/64e/interrupt: always save nvgprs on interrupt"
This reverts commit 4228b2c3d20e9f80b847f809c38e6cf82864fa50.
:040000 040000 156542c857ad72776b69bb67b2f244afeeb7abd3
92ea86ed097fce16238b0c2f2b343473894e4e8e M arch
- Christian
^ permalink raw reply
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Nicholas Piggin @ 2021-05-13 22:58 UTC (permalink / raw)
To: Christophe Leroy, Christian Zigotzky
Cc: Darren Stevens, linuxppc-dev, R.T.Dickinson, mad skateman,
Christian Zigotzky
In-Reply-To: <2f00463c-a87a-6dbc-037c-5a8996790d23@xenosoft.de>
Excerpts from Christian Zigotzky's message of May 14, 2021 6:20 am:
> On 13 May 2021 at 07:00pm, Christophe Leroy wrote:
>>
>> Ah yes, I remember this problem.
>>
>> Can you select CONFIG_VIRT_CPU_ACCOUNTING_GEN in your configuration ?
>>
>> Otherwise, I can try to fix the branch.
>>
>> Christophe
> I selected this. After that it compiles.
>
> 1. git bisect good - Xorg restarts again and again
> Output: [f9aa0ac1e9e82b60401ad567bdabc30598325bc1] Revert
> "powerpc/64e/interrupt: use new interrupt return"
> 2. git bisect good - Xorg restarts again and again
> Output: [cd6d259a14704741bf0cd1dcadb84c0de22d7f77] Revert
> "powerpc/64e/interrupt: always save nvgprs on interrupt"
> 3. git bisect bad - Xorg works
> Output: [9bfa20ef2ae54d3b9088dfbcde4ef97062cf5ef2] Revert
> "powerpc/interrupt: update common interrupt code for"
> 4. git bisect good - Xorg restarts again and again
> Output:
>
> cd6d259a14704741bf0cd1dcadb84c0de22d7f77 is the first bad commit
> commit cd6d259a14704741bf0cd1dcadb84c0de22d7f77
> Author: Christophe Leroy <christophe.leroy@csgroup.eu>
> Date: Thu May 13 09:52:06 2021 +0000
>
> Revert "powerpc/64e/interrupt: always save nvgprs on interrupt"
>
> This reverts commit 4228b2c3d20e9f80b847f809c38e6cf82864fa50.
>
> :040000 040000 156542c857ad72776b69bb67b2f244afeeb7abd3
> 92ea86ed097fce16238b0c2f2b343473894e4e8e M arch
Thank you both very much for chasing this down.
I think I see the problem, it's clobbering r14 and r15 for some
interrupts. Something like this is required, I'll give it more
review and testing though.
Thanks,
Nick
---
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 7c3654b0d0f4..b91ef04f1ce2 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -535,6 +535,10 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
mfspr r14,SPRN_DEAR
mfspr r15,SPRN_ESR
+ std r14,_DAR(r1)
+ std r15,_DSISR(r1)
+ ld r14,PACA_EXGEN+EX_R14(r13)
+ ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x300)
b storage_fault_common
@@ -544,6 +548,10 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
li r15,0
mr r14,r10
+ std r14,_DAR(r1)
+ std r15,_DSISR(r1)
+ ld r14,PACA_EXGEN+EX_R14(r13)
+ ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x400)
b storage_fault_common
@@ -557,6 +565,10 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
mfspr r14,SPRN_DEAR
mfspr r15,SPRN_ESR
+ std r14,_DAR(r1)
+ std r15,_DSISR(r1)
+ ld r14,PACA_EXGEN+EX_R14(r13)
+ ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x600)
b alignment_more /* no room, go out of line */
@@ -565,10 +577,10 @@ __end_interrupts:
NORMAL_EXCEPTION_PROLOG(0x700, BOOKE_INTERRUPT_PROGRAM,
PROLOG_ADDITION_1REG)
mfspr r14,SPRN_ESR
- EXCEPTION_COMMON(0x700)
std r14,_DSISR(r1)
- addi r3,r1,STACK_FRAME_OVERHEAD
ld r14,PACA_EXGEN+EX_R14(r13)
+ EXCEPTION_COMMON(0x700)
+ addi r3,r1,STACK_FRAME_OVERHEAD
bl program_check_exception
REST_NVGPRS(r1)
b interrupt_return
@@ -725,11 +737,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
* normal exception
*/
mfspr r14,SPRN_DBSR
- EXCEPTION_COMMON_CRIT(0xd00)
std r14,_DSISR(r1)
- addi r3,r1,STACK_FRAME_OVERHEAD
ld r14,PACA_EXCRIT+EX_R14(r13)
ld r15,PACA_EXCRIT+EX_R15(r13)
+ EXCEPTION_COMMON_CRIT(0xd00)
+ addi r3,r1,STACK_FRAME_OVERHEAD
bl DebugException
REST_NVGPRS(r1)
b interrupt_return
@@ -796,11 +808,11 @@ kernel_dbg_exc:
* normal exception
*/
mfspr r14,SPRN_DBSR
- EXCEPTION_COMMON_DBG(0xd08)
std r14,_DSISR(r1)
- addi r3,r1,STACK_FRAME_OVERHEAD
ld r14,PACA_EXDBG+EX_R14(r13)
ld r15,PACA_EXDBG+EX_R15(r13)
+ EXCEPTION_COMMON_DBG(0xd08)
+ addi r3,r1,STACK_FRAME_OVERHEAD
bl DebugException
REST_NVGPRS(r1)
b interrupt_return
@@ -931,11 +943,7 @@ masked_interrupt_book3e_0x2c0:
* original values stashed away in the PACA
*/
storage_fault_common:
- std r14,_DAR(r1)
- std r15,_DSISR(r1)
addi r3,r1,STACK_FRAME_OVERHEAD
- ld r14,PACA_EXGEN+EX_R14(r13)
- ld r15,PACA_EXGEN+EX_R15(r13)
bl do_page_fault
b interrupt_return
@@ -944,11 +952,7 @@ storage_fault_common:
* continues here.
*/
alignment_more:
- std r14,_DAR(r1)
- std r15,_DSISR(r1)
addi r3,r1,STACK_FRAME_OVERHEAD
- ld r14,PACA_EXGEN+EX_R14(r13)
- ld r15,PACA_EXGEN+EX_R15(r13)
bl alignment_exception
REST_NVGPRS(r1)
b interrupt_return
^ permalink raw reply related
* Re: [FSL P50x0] Xorg always restarts again and again after the the PowerPC updates 5.13-1
From: Christian Zigotzky @ 2021-05-14 0:19 UTC (permalink / raw)
To: Nicholas Piggin, Christophe Leroy
Cc: Darren Stevens, linuxppc-dev, R.T.Dickinson, mad skateman,
Christian Zigotzky
In-Reply-To: <1620946444.gkflc9lqps.astroid@bobo.none>
On 14 May 2021 at 00:58am, Nicholas Piggin wrote:
> Excerpts from Christian Zigotzky's message of May 14, 2021 6:20 am:
>> On 13 May 2021 at 07:00pm, Christophe Leroy wrote:
>>> Ah yes, I remember this problem.
>>>
>>> Can you select CONFIG_VIRT_CPU_ACCOUNTING_GEN in your configuration ?
>>>
>>> Otherwise, I can try to fix the branch.
>>>
>>> Christophe
>> I selected this. After that it compiles.
>>
>> 1. git bisect good - Xorg restarts again and again
>> Output: [f9aa0ac1e9e82b60401ad567bdabc30598325bc1] Revert
>> "powerpc/64e/interrupt: use new interrupt return"
>> 2. git bisect good - Xorg restarts again and again
>> Output: [cd6d259a14704741bf0cd1dcadb84c0de22d7f77] Revert
>> "powerpc/64e/interrupt: always save nvgprs on interrupt"
>> 3. git bisect bad - Xorg works
>> Output: [9bfa20ef2ae54d3b9088dfbcde4ef97062cf5ef2] Revert
>> "powerpc/interrupt: update common interrupt code for"
>> 4. git bisect good - Xorg restarts again and again
>> Output:
>>
>> cd6d259a14704741bf0cd1dcadb84c0de22d7f77 is the first bad commit
>> commit cd6d259a14704741bf0cd1dcadb84c0de22d7f77
>> Author: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Date: Thu May 13 09:52:06 2021 +0000
>>
>> Revert "powerpc/64e/interrupt: always save nvgprs on interrupt"
>>
>> This reverts commit 4228b2c3d20e9f80b847f809c38e6cf82864fa50.
>>
>> :040000 040000 156542c857ad72776b69bb67b2f244afeeb7abd3
>> 92ea86ed097fce16238b0c2f2b343473894e4e8e M arch
> Thank you both very much for chasing this down.
>
> I think I see the problem, it's clobbering r14 and r15 for some
> interrupts. Something like this is required, I'll give it more
> review and testing though.
>
> Thanks,
> Nick
>
> ---
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index 7c3654b0d0f4..b91ef04f1ce2 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -535,6 +535,10 @@ __end_interrupts:
> PROLOG_ADDITION_2REGS)
> mfspr r14,SPRN_DEAR
> mfspr r15,SPRN_ESR
> + std r14,_DAR(r1)
> + std r15,_DSISR(r1)
> + ld r14,PACA_EXGEN+EX_R14(r13)
> + ld r15,PACA_EXGEN+EX_R15(r13)
> EXCEPTION_COMMON(0x300)
> b storage_fault_common
>
> @@ -544,6 +548,10 @@ __end_interrupts:
> PROLOG_ADDITION_2REGS)
> li r15,0
> mr r14,r10
> + std r14,_DAR(r1)
> + std r15,_DSISR(r1)
> + ld r14,PACA_EXGEN+EX_R14(r13)
> + ld r15,PACA_EXGEN+EX_R15(r13)
> EXCEPTION_COMMON(0x400)
> b storage_fault_common
>
> @@ -557,6 +565,10 @@ __end_interrupts:
> PROLOG_ADDITION_2REGS)
> mfspr r14,SPRN_DEAR
> mfspr r15,SPRN_ESR
> + std r14,_DAR(r1)
> + std r15,_DSISR(r1)
> + ld r14,PACA_EXGEN+EX_R14(r13)
> + ld r15,PACA_EXGEN+EX_R15(r13)
> EXCEPTION_COMMON(0x600)
> b alignment_more /* no room, go out of line */
>
> @@ -565,10 +577,10 @@ __end_interrupts:
> NORMAL_EXCEPTION_PROLOG(0x700, BOOKE_INTERRUPT_PROGRAM,
> PROLOG_ADDITION_1REG)
> mfspr r14,SPRN_ESR
> - EXCEPTION_COMMON(0x700)
> std r14,_DSISR(r1)
> - addi r3,r1,STACK_FRAME_OVERHEAD
> ld r14,PACA_EXGEN+EX_R14(r13)
> + EXCEPTION_COMMON(0x700)
> + addi r3,r1,STACK_FRAME_OVERHEAD
> bl program_check_exception
> REST_NVGPRS(r1)
> b interrupt_return
> @@ -725,11 +737,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> * normal exception
> */
> mfspr r14,SPRN_DBSR
> - EXCEPTION_COMMON_CRIT(0xd00)
> std r14,_DSISR(r1)
> - addi r3,r1,STACK_FRAME_OVERHEAD
> ld r14,PACA_EXCRIT+EX_R14(r13)
> ld r15,PACA_EXCRIT+EX_R15(r13)
> + EXCEPTION_COMMON_CRIT(0xd00)
> + addi r3,r1,STACK_FRAME_OVERHEAD
> bl DebugException
> REST_NVGPRS(r1)
> b interrupt_return
> @@ -796,11 +808,11 @@ kernel_dbg_exc:
> * normal exception
> */
> mfspr r14,SPRN_DBSR
> - EXCEPTION_COMMON_DBG(0xd08)
> std r14,_DSISR(r1)
> - addi r3,r1,STACK_FRAME_OVERHEAD
> ld r14,PACA_EXDBG+EX_R14(r13)
> ld r15,PACA_EXDBG+EX_R15(r13)
> + EXCEPTION_COMMON_DBG(0xd08)
> + addi r3,r1,STACK_FRAME_OVERHEAD
> bl DebugException
> REST_NVGPRS(r1)
> b interrupt_return
> @@ -931,11 +943,7 @@ masked_interrupt_book3e_0x2c0:
> * original values stashed away in the PACA
> */
> storage_fault_common:
> - std r14,_DAR(r1)
> - std r15,_DSISR(r1)
> addi r3,r1,STACK_FRAME_OVERHEAD
> - ld r14,PACA_EXGEN+EX_R14(r13)
> - ld r15,PACA_EXGEN+EX_R15(r13)
> bl do_page_fault
> b interrupt_return
>
> @@ -944,11 +952,7 @@ storage_fault_common:
> * continues here.
> */
> alignment_more:
> - std r14,_DAR(r1)
> - std r15,_DSISR(r1)
> addi r3,r1,STACK_FRAME_OVERHEAD
> - ld r14,PACA_EXGEN+EX_R14(r13)
> - ld r15,PACA_EXGEN+EX_R15(r13)
> bl alignment_exception
> REST_NVGPRS(r1)
> b interrupt_return
>
>
Hi Nicholas,
I compiled the RC1 with your patch today and Xorg works without any
problems.
Many thanks! It was a long way.
Cheers,
Christian
^ permalink raw reply
* Re: [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Alexey Kardashevskiy @ 2021-05-14 1:56 UTC (permalink / raw)
To: Nathan Chancellor, linuxppc-dev
Cc: Michal Marek, linux-kbuild, Masahiro Yamada, Nick Desaulniers,
linux-kernel, clang-built-linux
In-Reply-To: <dedc7262-2956-37b2-ebfd-ae8eb9b56716@kernel.org>
On 14/05/2021 04:59, Nathan Chancellor wrote:
> On 5/13/2021 4:59 AM, Alexey Kardashevskiy wrote:
>> The $(CPP) (do only preprocessing) macro is already defined in Makefile.
>> However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
>> in flags duplication. Which is not a big deal by itself except for
>> the flags which depend on other flags and the compiler checks them
>> as it parses the command line.
>>
>> Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
>> If clang+llvm+sanitizer are enabled, this results in
>>
>> -emit-llvm-bc -fno-lto -flto -fvisibility=hidden \
>> -fsanitize=cfi-mfcall -fno-lto ...
>>
>> in the clang command line and triggers error:
>>
>> clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed
>> with '-flto'
>>
>> This removes unnecessary CPP redefinition. Which works fine as in most
>> place KBUILD_CFLAGS is passed to $CPP except
>> arch/powerpc/kernel/vdso64/vdso(32|64).lds. To fix vdso, this does:
>> 1. add -m(big|little)-endian to $CPP
>> 2. add target to $KBUILD_CPPFLAGS as otherwise clang ignores
>> -m(big|little)-endian if
>> the building platform does not support big endian (such as x86).
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v3:
>> * moved vdso cleanup in a separate patch
>> * only add target to KBUILD_CPPFLAGS for CLANG
>>
>> v2:
>> * fix KBUILD_CPPFLAGS
>> * add CLANG_FLAGS to CPPFLAGS
>> ---
>> Makefile | 1 +
>> arch/powerpc/Makefile | 3 ++-
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 15b6476d0f89..5b545bef7653 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -576,6 +576,7 @@ CC_VERSION_TEXT = $(subst $(pound),,$(shell $(CC)
>> --version 2>/dev/null | head -
>> ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
>> ifneq ($(CROSS_COMPILE),)
>> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
>> +KBUILD_CPPFLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
>
> You can avoid the duplication here by just doing:
>
> KBUILD_CPPFLAGS += $(CLANG_FLAGS)
This has potential of duplicating even more flags which is exactly what
I am trying to avoid here.
> I am still not super happy about the flag duplication but I am not sure
> I can think of a better solution. If KBUILD_CPPFLAGS are always included
> when building .o files,
My understanding is that KBUILD_CPPFLAGS should not be added for .o. Who
does know or decide for sure about what CPPFLAGS are for? :)
> maybe we should just add $(CLANG_FLAGS) to
> KBUILD_CPPFLAGS instead of KBUILD_CFLAGS?
>
>> endif
>> ifeq ($(LLVM_IAS),1)
>> CLANG_FLAGS += -integrated-as
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index 3212d076ac6a..306bfd2797ad 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -76,6 +76,7 @@ endif
>> ifdef CONFIG_CPU_LITTLE_ENDIAN
>> KBUILD_CFLAGS += -mlittle-endian
>> +KBUILD_CPPFLAGS += -mlittle-endian
>> KBUILD_LDFLAGS += -EL
>> LDEMULATION := lppc
>> GNUTARGET := powerpcle
>> @@ -83,6 +84,7 @@ MULTIPLEWORD := -mno-multiple
>> KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
>> else
>> KBUILD_CFLAGS += $(call cc-option,-mbig-endian)
>> +KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
>> KBUILD_LDFLAGS += -EB
>> LDEMULATION := ppc
>> GNUTARGET := powerpc
>> @@ -208,7 +210,6 @@ KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH)
>> $(asinstr)
>> KBUILD_AFLAGS += $(AFLAGS-y)
>> KBUILD_CFLAGS += $(call cc-option,-msoft-float)
>> KBUILD_CFLAGS += -pipe $(CFLAGS-y)
>> -CPP = $(CC) -E $(KBUILD_CFLAGS)
>> CHECKFLAGS += -m$(BITS) -D__powerpc__ -D__powerpc$(BITS)__
>> ifdef CONFIG_CPU_BIG_ENDIAN
>>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Masahiro Yamada @ 2021-05-14 2:42 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Michal Marek, Linux Kbuild mailing list, Alexey Kardashevskiy,
Nick Desaulniers, Linux Kernel Mailing List, clang-built-linux,
linuxppc-dev
In-Reply-To: <dedc7262-2956-37b2-ebfd-ae8eb9b56716@kernel.org>
On Fri, May 14, 2021 at 3:59 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On 5/13/2021 4:59 AM, Alexey Kardashevskiy wrote:
> > The $(CPP) (do only preprocessing) macro is already defined in Makefile.
> > However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
> > in flags duplication. Which is not a big deal by itself except for
> > the flags which depend on other flags and the compiler checks them
> > as it parses the command line.
> >
> > Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
> > If clang+llvm+sanitizer are enabled, this results in
> >
> > -emit-llvm-bc -fno-lto -flto -fvisibility=hidden \
> > -fsanitize=cfi-mfcall -fno-lto ...
> >
> > in the clang command line and triggers error:
I do not know how to reproduce this for powerpc.
Currently, only x86 and arm64 select
ARCH_SUPPORTS_LTO_CLANG.
Is this a fix for a potential issue?
> > clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed with '-flto'
> >
> > This removes unnecessary CPP redefinition. Which works fine as in most
> > place KBUILD_CFLAGS is passed to $CPP except
> > arch/powerpc/kernel/vdso64/vdso(32|64).lds. To fix vdso, this does:
> > 1. add -m(big|little)-endian to $CPP
> > 2. add target to $KBUILD_CPPFLAGS as otherwise clang ignores -m(big|little)-endian if
> > the building platform does not support big endian (such as x86).
> >
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> > Changes:
> > v3:
> > * moved vdso cleanup in a separate patch
> > * only add target to KBUILD_CPPFLAGS for CLANG
> >
> > v2:
> > * fix KBUILD_CPPFLAGS
> > * add CLANG_FLAGS to CPPFLAGS
> > ---
> > Makefile | 1 +
> > arch/powerpc/Makefile | 3 ++-
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 15b6476d0f89..5b545bef7653 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -576,6 +576,7 @@ CC_VERSION_TEXT = $(subst $(pound),,$(shell $(CC) --version 2>/dev/null | head -
> > ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
> > ifneq ($(CROSS_COMPILE),)
> > CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> > +KBUILD_CPPFLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
>
> You can avoid the duplication here by just doing:
>
> KBUILD_CPPFLAGS += $(CLANG_FLAGS)
>
> I am still not super happy about the flag duplication but I am not sure
> I can think of a better solution. If KBUILD_CPPFLAGS are always included
> when building .o files, maybe we should just add $(CLANG_FLAGS) to
> KBUILD_CPPFLAGS instead of KBUILD_CFLAGS?
Hmm, I think including --target=* in CPP flags is sensible,
but not all CLANG_FLAGS are CPP flags.
At least, -(no)-integrated-as is not a CPP flag.
We could introduce a separate CLANG_CPP_FLAGS, but
it would require more code changes...
So, I do not have a strong opinion either way.
BTW, another approach might be to modify the linker script.
In my best guess, the reason why powerpc adding the endian flag to CPP
is this line in arch/powerpc/kernel/vdso64/vdso64.lds.S
#ifdef __LITTLE_ENDIAN__
OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
#else
OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc")
#endif
You can use the CONFIG option to check the endian-ness.
#ifdef CONFIG_CPU_BIG_ENDIAN
OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc")
#else
OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
#endif
All the big endian arches define CONFIG_CPU_BIG_ENDIAN.
(but not all little endian arches define CONFIG_CPU_LITTLE_ENDIAN)
So,
#ifdef CONFIG_CPU_BIG_ENDIAN
< big endian code >
#else
< little endian code >
#endif
works for all architectures.
Only the exception is you cannot replace the one in uapi headers.
arch/powerpc/include/uapi/asm/byteorder.h: #ifdef __LITTLE_ENDIAN__
since it is exported to userspace, where CONFIG options are not available.
BTW, various flags are historically used.
- CONFIG_CPU_BIG_ENDIAN / CONFIG_CPU_LITTLE_ENDIAN
- __BIG_ENDIAN / __LITTLE_ENDIAN
- __LITTLE_ENDIAN__ (powerpc only)
__LITTLE_ENDIAN__ is defined by powerpc gcc and clang.
My experiments...
[1] powerpc-linux-gnu-gcc -> __BIG_ENDIAN__ is defined
masahiro@grover:~$ echo | powerpc-linux-gnu-gcc -E -dM -x c - | grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __BIG_ENDIAN__ 1
#define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define _BIG_ENDIAN 1
#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
#define __VEC_ELEMENT_REG_ORDER__ __ORDER_BIG_ENDIAN__
#define __ORDER_BIG_ENDIAN__ 4321
[2] powerpc-linux-gnu-gcc + -mlittle-endian -> __LITTLE_ENDIAN__ is defined
masahiro@grover:~$ echo | powerpc-linux-gnu-gcc -E -dM -x c -
-mlittle-endian | grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define _LITTLE_ENDIAN 1
#define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define __LITTLE_ENDIAN__ 1
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __VEC_ELEMENT_REG_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_BIG_ENDIAN__ 4321
[3] other arch gcc -> neither of them is defined
masahiro@grover:~$ echo | gcc -E -dM -x c - | grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define __ORDER_BIG_ENDIAN__ 4321
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
masahiro@grover:~$ echo | arm-linux-gnueabihf-gcc -E -dM -x c -
-mlittle-endian | grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_BIG_ENDIAN__ 4321
masahiro@grover:~$ echo | arm-linux-gnueabihf-gcc -E -dM -x c -
-mbig-endian | grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define __ARM_BIG_ENDIAN 1
#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
#define __ORDER_BIG_ENDIAN__ 4321
[4] Clang --target=powerpc-linux-gnu -> __BIG_ENDIAN__ is defined
masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E
--target=powerpc-linux-gnu -dM -x c - | grep ENDIAN
#define _BIG_ENDIAN 1
#define __BIG_ENDIAN__ 1
#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
#define __ORDER_BIG_ENDIAN__ 4321
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __ORDER_PDP_ENDIAN__ 3412
[5] very recent Clang understands --target=powerpcle-linux-gnu -->
__LITTLE_ENDIAN__ is defined
masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E
--target=powerpcle-linux-gnu -dM -x c - | grep ENDIAN
#define _LITTLE_ENDIAN 1
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __LITTLE_ENDIAN__ 1
#define __ORDER_BIG_ENDIAN__ 4321
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __ORDER_PDP_ENDIAN__ 3412
[6] very recent Clang, --target=powerpc-linux-gnu + -mlittle-endian
--> __LITTLE_ENDIAN__ is defined
masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E
--target=powerpc-linux-gnu -dM -x c - -mlittle-endian | grep ENDIAN
#define _LITTLE_ENDIAN 1
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __LITTLE_ENDIAN__ 1
#define __ORDER_BIG_ENDIAN__ 4321
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __ORDER_PDP_ENDIAN__ 3412
[7] Clang, target with little endian only , -mbig-endian is ignored
masahiro@grover:~$ echo | clang -E -dM -x c - | grep ENDIAN
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __LITTLE_ENDIAN__ 1
#define __ORDER_BIG_ENDIAN__ 4321
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __ORDER_PDP_ENDIAN__ 3412
masahiro@grover:~$ echo | clang -E -dM -x c - -mbig-endian | grep ENDIAN
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __LITTLE_ENDIAN__ 1
#define __ORDER_BIG_ENDIAN__ 4321
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __ORDER_PDP_ENDIAN__ 3412
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH kernel v2] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Masahiro Yamada @ 2021-05-14 3:10 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Michal Marek, Linux Kbuild mailing list, Alexey Kardashevskiy,
Nick Desaulniers, Linux Kernel Mailing List, Nicholas Piggin,
Nathan Chancellor, clang-built-linux, linuxppc-dev
In-Reply-To: <20210512102640.GT10366@gate.crashing.org>
On Wed, May 12, 2021 at 7:29 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Wed, May 12, 2021 at 01:48:53PM +1000, Alexey Kardashevskiy wrote:
> > >Oh! I completely missed those few lines. Sorry for that :-(
> >
> > Well, I probably should have made it a separate patch anyway, I'll
> > repost separately.
>
> Thanks.
>
> > >To compensate a bit:
> > >
> > >>It still puzzles me why we need -C
> > >>(preserve comments in the preprocessor output) flag here.
> > >
> > >It is so that a human can look at the output and read it. Comments are
> > >very significant to human readers :-)
> >
> > I seriously doubt anyone ever read those :)
>
> I am pretty sure whoever wrote it did!
Keeping comments in the pre-processed linker scripts
is troublesome.
That is why -C was removed from scripts/Makefile.build
commit 5cb0512c02ecd7e6214e912e4c150f4219ac78e0
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu Nov 2 14:10:37 2017 -0700
Kbuild: don't pass "-C" to preprocessor when processing linker scripts
You can entirely remove
CPPFLAGS_vdso32.lds += -P -C -Upowerpc
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Alexey Kardashevskiy @ 2021-05-14 3:41 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor
Cc: Michal Marek, Linux Kbuild mailing list, Nick Desaulniers,
Linux Kernel Mailing List, clang-built-linux, linuxppc-dev
In-Reply-To: <CAK7LNASFhRE=1EBj9AoTMMEd2YJdu7bCxARAGJfZ7aXcBrMAUw@mail.gmail.com>
On 14/05/2021 12:42, Masahiro Yamada wrote:
> On Fri, May 14, 2021 at 3:59 AM Nathan Chancellor <nathan@kernel.org> wrote:
>>
>> On 5/13/2021 4:59 AM, Alexey Kardashevskiy wrote:
>>> The $(CPP) (do only preprocessing) macro is already defined in Makefile.
>>> However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
>>> in flags duplication. Which is not a big deal by itself except for
>>> the flags which depend on other flags and the compiler checks them
>>> as it parses the command line.
>>>
>>> Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
>>> If clang+llvm+sanitizer are enabled, this results in
>>>
>>> -emit-llvm-bc -fno-lto -flto -fvisibility=hidden \
>>> -fsanitize=cfi-mfcall -fno-lto ...
>>>
>>> in the clang command line and triggers error:
>
> I do not know how to reproduce this for powerpc.
> Currently, only x86 and arm64 select
> ARCH_SUPPORTS_LTO_CLANG.
>
> Is this a fix for a potential issue?
Yeah, it is work in progress to enable LTO_CLANG for PPC64:
https://github.com/aik/linux/commits/lto
>
>
>>> clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed with '-flto'
>>>
>>> This removes unnecessary CPP redefinition. Which works fine as in most
>>> place KBUILD_CFLAGS is passed to $CPP except
>>> arch/powerpc/kernel/vdso64/vdso(32|64).lds. To fix vdso, this does:
>>> 1. add -m(big|little)-endian to $CPP
>>> 2. add target to $KBUILD_CPPFLAGS as otherwise clang ignores -m(big|little)-endian if
>>> the building platform does not support big endian (such as x86).
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> Changes:
>>> v3:
>>> * moved vdso cleanup in a separate patch
>>> * only add target to KBUILD_CPPFLAGS for CLANG
>>>
>>> v2:
>>> * fix KBUILD_CPPFLAGS
>>> * add CLANG_FLAGS to CPPFLAGS
>>> ---
>>> Makefile | 1 +
>>> arch/powerpc/Makefile | 3 ++-
>>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 15b6476d0f89..5b545bef7653 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -576,6 +576,7 @@ CC_VERSION_TEXT = $(subst $(pound),,$(shell $(CC) --version 2>/dev/null | head -
>>> ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
>>> ifneq ($(CROSS_COMPILE),)
>>> CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
>>> +KBUILD_CPPFLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
>>
>> You can avoid the duplication here by just doing:
>>
>> KBUILD_CPPFLAGS += $(CLANG_FLAGS)
>>
>> I am still not super happy about the flag duplication but I am not sure
>> I can think of a better solution. If KBUILD_CPPFLAGS are always included
>> when building .o files, maybe we should just add $(CLANG_FLAGS) to
>> KBUILD_CPPFLAGS instead of KBUILD_CFLAGS?
>
> Hmm, I think including --target=* in CPP flags is sensible,
> but not all CLANG_FLAGS are CPP flags.
> At least, -(no)-integrated-as is not a CPP flag.
>
> We could introduce a separate CLANG_CPP_FLAGS, but
> it would require more code changes...
>
> So, I do not have a strong opinion either way.
>
>
>
> BTW, another approach might be to modify the linker script.
>
>
> In my best guess, the reason why powerpc adding the endian flag to CPP
> is this line in arch/powerpc/kernel/vdso64/vdso64.lds.S
>
> #ifdef __LITTLE_ENDIAN__
> OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
> #else
> OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc")
> #endif
>
>
> You can use the CONFIG option to check the endian-ness.
>
> #ifdef CONFIG_CPU_BIG_ENDIAN
> OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc")
> #else
> OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
> #endif
>
>
> All the big endian arches define CONFIG_CPU_BIG_ENDIAN.
> (but not all little endian arches define CONFIG_CPU_LITTLE_ENDIAN)
This should work with .lds. But missing --target=* might still hit us
somewhere else later, these include 3 header files each and there might
be endianness dependent stuff.
>
>
> So,
> #ifdef CONFIG_CPU_BIG_ENDIAN
> < big endian code >
> #else
> < little endian code >
> #endif
>
> works for all architectures.
>
>
> Only the exception is you cannot replace the one in uapi headers.
> arch/powerpc/include/uapi/asm/byteorder.h: #ifdef __LITTLE_ENDIAN__
> since it is exported to userspace, where CONFIG options are not available.
>
>
>
> BTW, various flags are historically used.
>
> - CONFIG_CPU_BIG_ENDIAN / CONFIG_CPU_LITTLE_ENDIAN
> - __BIG_ENDIAN / __LITTLE_ENDIAN
> - __LITTLE_ENDIAN__ (powerpc only)
>
>
>
> __LITTLE_ENDIAN__ is defined by powerpc gcc and clang.
>
> My experiments...
>
>
> [1] powerpc-linux-gnu-gcc -> __BIG_ENDIAN__ is defined
>
> masahiro@grover:~$ echo | powerpc-linux-gnu-gcc -E -dM -x c - | grep ENDIAN
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __BIG_ENDIAN__ 1
> #define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__
> #define __ORDER_PDP_ENDIAN__ 3412
> #define _BIG_ENDIAN 1
> #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
> #define __VEC_ELEMENT_REG_ORDER__ __ORDER_BIG_ENDIAN__
> #define __ORDER_BIG_ENDIAN__ 4321
>
>
> [2] powerpc-linux-gnu-gcc + -mlittle-endian -> __LITTLE_ENDIAN__ is defined
>
> masahiro@grover:~$ echo | powerpc-linux-gnu-gcc -E -dM -x c -
> -mlittle-endian | grep ENDIAN
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define _LITTLE_ENDIAN 1
> #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __ORDER_PDP_ENDIAN__ 3412
> #define __LITTLE_ENDIAN__ 1
> #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __VEC_ELEMENT_REG_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __ORDER_BIG_ENDIAN__ 4321
>
>
> [3] other arch gcc -> neither of them is defined
>
> masahiro@grover:~$ echo | gcc -E -dM -x c - | grep ENDIAN
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __ORDER_PDP_ENDIAN__ 3412
> #define __ORDER_BIG_ENDIAN__ 4321
> #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
>
> masahiro@grover:~$ echo | arm-linux-gnueabihf-gcc -E -dM -x c -
> -mlittle-endian | grep ENDIAN
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __ORDER_PDP_ENDIAN__ 3412
> #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __ORDER_BIG_ENDIAN__ 4321
>
> masahiro@grover:~$ echo | arm-linux-gnueabihf-gcc -E -dM -x c -
> -mbig-endian | grep ENDIAN
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__
> #define __ORDER_PDP_ENDIAN__ 3412
> #define __ARM_BIG_ENDIAN 1
> #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
> #define __ORDER_BIG_ENDIAN__ 4321
>
>
> [4] Clang --target=powerpc-linux-gnu -> __BIG_ENDIAN__ is defined
>
> masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E
> --target=powerpc-linux-gnu -dM -x c - | grep ENDIAN
> #define _BIG_ENDIAN 1
> #define __BIG_ENDIAN__ 1
> #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
> #define __ORDER_BIG_ENDIAN__ 4321
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __ORDER_PDP_ENDIAN__ 3412
>
>
>
> [5] very recent Clang understands --target=powerpcle-linux-gnu -->
> __LITTLE_ENDIAN__ is defined
>
> masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E
> --target=powerpcle-linux-gnu -dM -x c - | grep ENDIAN
> #define _LITTLE_ENDIAN 1
> #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __LITTLE_ENDIAN__ 1
> #define __ORDER_BIG_ENDIAN__ 4321
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __ORDER_PDP_ENDIAN__ 3412
>
>
> [6] very recent Clang, --target=powerpc-linux-gnu + -mlittle-endian
> --> __LITTLE_ENDIAN__ is defined
>
> masahiro@grover:~$ echo | ~/tools/clang-latest/bin/clang -E
> --target=powerpc-linux-gnu -dM -x c - -mlittle-endian | grep ENDIAN
> #define _LITTLE_ENDIAN 1
> #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __LITTLE_ENDIAN__ 1
> #define __ORDER_BIG_ENDIAN__ 4321
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __ORDER_PDP_ENDIAN__ 3412
>
>
>
>
> [7] Clang, target with little endian only , -mbig-endian is ignored
> masahiro@grover:~$ echo | clang -E -dM -x c - | grep ENDIAN
> #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __LITTLE_ENDIAN__ 1
> #define __ORDER_BIG_ENDIAN__ 4321
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __ORDER_PDP_ENDIAN__ 3412
> masahiro@grover:~$ echo | clang -E -dM -x c - -mbig-endian | grep ENDIAN
> #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
> #define __LITTLE_ENDIAN__ 1
> #define __ORDER_BIG_ENDIAN__ 4321
> #define __ORDER_LITTLE_ENDIAN__ 1234
> #define __ORDER_PDP_ENDIAN__ 3412
Nice research :) Thanks,
>
>
> --
> Best Regards
> Masahiro Yamada
>
--
Alexey
^ permalink raw reply
* [PATCH] powerpc/64e/interrupt: fix nvgprs being clobbered
From: Nicholas Piggin @ 2021-05-14 4:40 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Christian Zigotzky
Some interrupt handlers have an "extra" that saves 1 or 2 registers
(r14, r15) in the paca save area and makes them available to use by the
handler.
The change to always save nvgprs in exception handlers lead to some
interrupt handlers saving those scratch r14 / r15 registers into the
interrupt frame's GPR saves, which get restored on interrupt exit.
Fix this by always reloading those scratch registers from paca before the
EXCEPTION_COMMON that saves nvgprs.
Fixes: 4228b2c3d20e ("powerpc/64e/interrupt: always save nvgprs on interrupt")
Tested-by: Christian Zigotzky <chzigotzky@xenosoft.de>
Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64e.S | 38 ++++++++++++++++++----------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 7c3654b0d0f4..f1ae710274bc 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -340,6 +340,12 @@ ret_from_mc_except:
andi. r10,r10,IRQS_DISABLED; /* yes -> go out of line */ \
bne masked_interrupt_book3e_##n
+/*
+ * Additional regs must be re-loaded from paca before EXCEPTION_COMMON* is
+ * called, because that does SAVE_NVGPRS which must see the original register
+ * values, otherwise the scratch values might be restored when exiting the
+ * interrupt.
+ */
#define PROLOG_ADDITION_2REGS_GEN(n) \
std r14,PACA_EXGEN+EX_R14(r13); \
std r15,PACA_EXGEN+EX_R15(r13)
@@ -535,6 +541,10 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
mfspr r14,SPRN_DEAR
mfspr r15,SPRN_ESR
+ std r14,_DAR(r1)
+ std r15,_DSISR(r1)
+ ld r14,PACA_EXGEN+EX_R14(r13)
+ ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x300)
b storage_fault_common
@@ -544,6 +554,10 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
li r15,0
mr r14,r10
+ std r14,_DAR(r1)
+ std r15,_DSISR(r1)
+ ld r14,PACA_EXGEN+EX_R14(r13)
+ ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x400)
b storage_fault_common
@@ -557,6 +571,10 @@ __end_interrupts:
PROLOG_ADDITION_2REGS)
mfspr r14,SPRN_DEAR
mfspr r15,SPRN_ESR
+ std r14,_DAR(r1)
+ std r15,_DSISR(r1)
+ ld r14,PACA_EXGEN+EX_R14(r13)
+ ld r15,PACA_EXGEN+EX_R15(r13)
EXCEPTION_COMMON(0x600)
b alignment_more /* no room, go out of line */
@@ -565,10 +583,10 @@ __end_interrupts:
NORMAL_EXCEPTION_PROLOG(0x700, BOOKE_INTERRUPT_PROGRAM,
PROLOG_ADDITION_1REG)
mfspr r14,SPRN_ESR
- EXCEPTION_COMMON(0x700)
std r14,_DSISR(r1)
- addi r3,r1,STACK_FRAME_OVERHEAD
ld r14,PACA_EXGEN+EX_R14(r13)
+ EXCEPTION_COMMON(0x700)
+ addi r3,r1,STACK_FRAME_OVERHEAD
bl program_check_exception
REST_NVGPRS(r1)
b interrupt_return
@@ -725,11 +743,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
* normal exception
*/
mfspr r14,SPRN_DBSR
- EXCEPTION_COMMON_CRIT(0xd00)
std r14,_DSISR(r1)
- addi r3,r1,STACK_FRAME_OVERHEAD
ld r14,PACA_EXCRIT+EX_R14(r13)
ld r15,PACA_EXCRIT+EX_R15(r13)
+ EXCEPTION_COMMON_CRIT(0xd00)
+ addi r3,r1,STACK_FRAME_OVERHEAD
bl DebugException
REST_NVGPRS(r1)
b interrupt_return
@@ -796,11 +814,11 @@ kernel_dbg_exc:
* normal exception
*/
mfspr r14,SPRN_DBSR
- EXCEPTION_COMMON_DBG(0xd08)
std r14,_DSISR(r1)
- addi r3,r1,STACK_FRAME_OVERHEAD
ld r14,PACA_EXDBG+EX_R14(r13)
ld r15,PACA_EXDBG+EX_R15(r13)
+ EXCEPTION_COMMON_DBG(0xd08)
+ addi r3,r1,STACK_FRAME_OVERHEAD
bl DebugException
REST_NVGPRS(r1)
b interrupt_return
@@ -931,11 +949,7 @@ masked_interrupt_book3e_0x2c0:
* original values stashed away in the PACA
*/
storage_fault_common:
- std r14,_DAR(r1)
- std r15,_DSISR(r1)
addi r3,r1,STACK_FRAME_OVERHEAD
- ld r14,PACA_EXGEN+EX_R14(r13)
- ld r15,PACA_EXGEN+EX_R15(r13)
bl do_page_fault
b interrupt_return
@@ -944,11 +958,7 @@ storage_fault_common:
* continues here.
*/
alignment_more:
- std r14,_DAR(r1)
- std r15,_DSISR(r1)
addi r3,r1,STACK_FRAME_OVERHEAD
- ld r14,PACA_EXGEN+EX_R14(r13)
- ld r15,PACA_EXGEN+EX_R15(r13)
bl alignment_exception
REST_NVGPRS(r1)
b interrupt_return
--
2.23.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).