Re: [RFC WIP PATCH] powerpc/32: system call implement entry/exit logic in C
Le 03/04/2020 à 09:33, Nicholas Piggin a écrit : Christophe Leroy's on April 1, 2020 9:48 pm: Le 31/03/2020 à 17:22, Christophe Leroy a écrit : That's first try to port PPC64 syscall entry/exit logic in C to PPC32. I've do the minimum to get it work. I have not reworked calls to sys_fork() and friends for instance. For the time being, it seems to work more or less but: - ping reports EINVAL on recvfrom - strace shows NULL instead of strings in call like open() for instance. For the two above problems, that's because system_call_exception() doesn't set orig_gpr3 whereas DoSycall() does in entry_32.S . Is that only done on PPC32 ? With the following line at the begining of system_call_exception(), it works perfectly: regs->orig_gpr3 = r3; Oh great, nice work. We should be able to make some simple helpers or move some things a bit to reduce the amount of ifdefs in the C code. It doesn't look too bad though. I will now focus on performance to see if we can do something about it. What's the performance difference between current asm code just with always saving NVGPRS vs C? Done new measurement and sent a series. lower values now because the time accounting was done twice as it was still in the ASM part. Before the series, 311 cycles for a null_syscall If adding SAVE_NVGPRS to the entry macro, 335 cycles First patch: 353 cycles ie +13,5% After a few changes, including conditional saving of non volatile registers, I get 325 cycles that is only +4,5%. I thing that's acceptable. Do you see a problem with still saving non volatile registers only when required ? Christophe
Re: [RFC WIP PATCH] powerpc/32: system call implement entry/exit logic in C
Christophe Leroy's on April 1, 2020 9:48 pm: > > > Le 31/03/2020 à 17:22, Christophe Leroy a écrit : >> That's first try to port PPC64 syscall entry/exit logic in C to PPC32. >> I've do the minimum to get it work. I have not reworked calls >> to sys_fork() and friends for instance. >> >> For the time being, it seems to work more or less but: >> - ping reports EINVAL on recvfrom >> - strace shows NULL instead of strings in call like open() for instance. > > For the two above problems, that's because system_call_exception() > doesn't set orig_gpr3 whereas DoSycall() does in entry_32.S . Is that > only done on PPC32 ? > > With the following line at the begining of system_call_exception(), it > works perfectly: > > regs->orig_gpr3 = r3; Oh great, nice work. We should be able to make some simple helpers or move some things a bit to reduce the amount of ifdefs in the C code. It doesn't look too bad though. > I will now focus on performance to see if we can do something about it. What's the performance difference between current asm code just with always saving NVGPRS vs C? Thanks, Nick
Re: [RFC WIP PATCH] powerpc/32: system call implement entry/exit logic in C
Le 31/03/2020 à 17:22, Christophe Leroy a écrit :
That's first try to port PPC64 syscall entry/exit logic in C to PPC32.
I've do the minimum to get it work. I have not reworked calls
to sys_fork() and friends for instance.
For the time being, it seems to work more or less but:
- ping reports EINVAL on recvfrom
- strace shows NULL instead of strings in call like open() for instance.
For the two above problems, that's because system_call_exception()
doesn't set orig_gpr3 whereas DoSycall() does in entry_32.S . Is that
only done on PPC32 ?
With the following line at the begining of system_call_exception(), it
works perfectly:
regs->orig_gpr3 = r3;
I will now focus on performance to see if we can do something about it.
Christophe
- the performance is definitively bad
On an 8xx, null_syscall test is about 30% slower after this patch:
- Without the patch: 284 cycles
- With the patch: 371 cycles
@nick and others, any suggestion to fix and improve ?
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/kup.h | 21 ++
.../powerpc/include/asm/book3s/64/kup-radix.h | 12 +-
arch/powerpc/include/asm/hw_irq.h | 15 +
arch/powerpc/include/asm/kup.h| 2 +
arch/powerpc/include/asm/nohash/32/kup-8xx.h | 13 +
arch/powerpc/kernel/Makefile | 5 +-
arch/powerpc/kernel/entry_32.S| 259 ++
arch/powerpc/kernel/head_32.h | 3 +-
.../kernel/{syscall_64.c => syscall.c}| 25 +-
9 files changed, 102 insertions(+), 253 deletions(-)
rename arch/powerpc/kernel/{syscall_64.c => syscall.c} (97%)
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h
b/arch/powerpc/include/asm/book3s/32/kup.h
index 3c0ba22dc360..c85bc5b56366 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -102,6 +102,27 @@ static inline void kuap_update_sr(u32 sr, u32 addr, u32
end)
isync();/* Context sync required after mtsrin() */
}
+static inline void kuap_restore(struct pt_regs *regs)
+{
+ u32 kuap = current->thread.kuap;
+ u32 addr = kuap & 0xf000;
+ u32 end = kuap << 28;
+
+ if (unlikely(!kuap))
+ return;
+
+ current->thread.kuap = 0;
+ kuap_update_sr(mfsrin(addr) & ~SR_KS, addr, end); /* Clear Ks */
+}
+
+static inline void kuap_check(void)
+{
+ if (!IS_ENABLED(CONFIG_PPC_KUAP_DEBUG))
+ return;
+
+ WARN_ON_ONCE(current->thread.kuap != 0);
+}
+
static __always_inline void allow_user_access(void __user *to, const void
__user *from,
u32 size, unsigned long dir)
{
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h
b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index 3bcef989a35d..1f2716a0dcd8 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
@@ -60,13 +60,13 @@
#include
#include
-static inline void kuap_restore_amr(struct pt_regs *regs)
+static inline void kuap_restore(struct pt_regs *regs)
{
if (mmu_has_feature(MMU_FTR_RADIX_KUAP))
mtspr(SPRN_AMR, regs->kuap);
}
-static inline void kuap_check_amr(void)
+static inline void kuap_check(void)
{
if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) &&
mmu_has_feature(MMU_FTR_RADIX_KUAP))
WARN_ON_ONCE(mfspr(SPRN_AMR) != AMR_KUAP_BLOCKED);
@@ -141,14 +141,6 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long
address, bool is_write)
(regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE :
AMR_KUAP_BLOCK_READ)),
"Bug: %s fault blocked by AMR!", is_write ? "Write" :
"Read");
}
-#else /* CONFIG_PPC_KUAP */
-static inline void kuap_restore_amr(struct pt_regs *regs)
-{
-}
-
-static inline void kuap_check_amr(void)
-{
-}
#endif /* CONFIG_PPC_KUAP */
#endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_irq.h
b/arch/powerpc/include/asm/hw_irq.h
index e0e71777961f..6ccf07de6665 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -321,6 +321,16 @@ static inline void arch_local_irq_disable(void)
mtmsr(mfmsr() & ~MSR_EE);
}
+static inline void arch_local_recovery_disable(void)
+{
+ if (IS_ENABLED(CONFIG_BOOKE))
+ wrtee(0);
+ else if (IS_ENABLED(CONFIG_PPC_8xx))
+ wrtspr(SPRN_NRI);
+ else
+ mtmsr(mfmsr() & ~(MSR_EE | MSR_RI));
+}
+
static inline void arch_local_irq_enable(void)
{
if (IS_ENABLED(CONFIG_BOOKE))
@@ -343,6 +353,11 @@ static inline bool arch_irqs_disabled(void)
#define hard_irq_disable() arch_local_irq_disable()
+#define __hard_irq_enable() arch_local_irq_enable()
+#define __hard_irq_disable() arch_local_irq_disable()
+#define __hard_EE_RI_disable() arch_local_recovery_disable()
+#define __hard_RI_e
[RFC WIP PATCH] powerpc/32: system call implement entry/exit logic in C
That's first try to port PPC64 syscall entry/exit logic in C to PPC32.
I've do the minimum to get it work. I have not reworked calls
to sys_fork() and friends for instance.
For the time being, it seems to work more or less but:
- ping reports EINVAL on recvfrom
- strace shows NULL instead of strings in call like open() for instance.
- the performance is definitively bad
On an 8xx, null_syscall test is about 30% slower after this patch:
- Without the patch: 284 cycles
- With the patch: 371 cycles
@nick and others, any suggestion to fix and improve ?
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/kup.h | 21 ++
.../powerpc/include/asm/book3s/64/kup-radix.h | 12 +-
arch/powerpc/include/asm/hw_irq.h | 15 +
arch/powerpc/include/asm/kup.h| 2 +
arch/powerpc/include/asm/nohash/32/kup-8xx.h | 13 +
arch/powerpc/kernel/Makefile | 5 +-
arch/powerpc/kernel/entry_32.S| 259 ++
arch/powerpc/kernel/head_32.h | 3 +-
.../kernel/{syscall_64.c => syscall.c}| 25 +-
9 files changed, 102 insertions(+), 253 deletions(-)
rename arch/powerpc/kernel/{syscall_64.c => syscall.c} (97%)
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h
b/arch/powerpc/include/asm/book3s/32/kup.h
index 3c0ba22dc360..c85bc5b56366 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -102,6 +102,27 @@ static inline void kuap_update_sr(u32 sr, u32 addr, u32
end)
isync();/* Context sync required after mtsrin() */
}
+static inline void kuap_restore(struct pt_regs *regs)
+{
+ u32 kuap = current->thread.kuap;
+ u32 addr = kuap & 0xf000;
+ u32 end = kuap << 28;
+
+ if (unlikely(!kuap))
+ return;
+
+ current->thread.kuap = 0;
+ kuap_update_sr(mfsrin(addr) & ~SR_KS, addr, end); /* Clear Ks */
+}
+
+static inline void kuap_check(void)
+{
+ if (!IS_ENABLED(CONFIG_PPC_KUAP_DEBUG))
+ return;
+
+ WARN_ON_ONCE(current->thread.kuap != 0);
+}
+
static __always_inline void allow_user_access(void __user *to, const void
__user *from,
u32 size, unsigned long dir)
{
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h
b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index 3bcef989a35d..1f2716a0dcd8 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
@@ -60,13 +60,13 @@
#include
#include
-static inline void kuap_restore_amr(struct pt_regs *regs)
+static inline void kuap_restore(struct pt_regs *regs)
{
if (mmu_has_feature(MMU_FTR_RADIX_KUAP))
mtspr(SPRN_AMR, regs->kuap);
}
-static inline void kuap_check_amr(void)
+static inline void kuap_check(void)
{
if (IS_ENABLED(CONFIG_PPC_KUAP_DEBUG) &&
mmu_has_feature(MMU_FTR_RADIX_KUAP))
WARN_ON_ONCE(mfspr(SPRN_AMR) != AMR_KUAP_BLOCKED);
@@ -141,14 +141,6 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long
address, bool is_write)
(regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE :
AMR_KUAP_BLOCK_READ)),
"Bug: %s fault blocked by AMR!", is_write ? "Write" :
"Read");
}
-#else /* CONFIG_PPC_KUAP */
-static inline void kuap_restore_amr(struct pt_regs *regs)
-{
-}
-
-static inline void kuap_check_amr(void)
-{
-}
#endif /* CONFIG_PPC_KUAP */
#endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_irq.h
b/arch/powerpc/include/asm/hw_irq.h
index e0e71777961f..6ccf07de6665 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -321,6 +321,16 @@ static inline void arch_local_irq_disable(void)
mtmsr(mfmsr() & ~MSR_EE);
}
+static inline void arch_local_recovery_disable(void)
+{
+ if (IS_ENABLED(CONFIG_BOOKE))
+ wrtee(0);
+ else if (IS_ENABLED(CONFIG_PPC_8xx))
+ wrtspr(SPRN_NRI);
+ else
+ mtmsr(mfmsr() & ~(MSR_EE | MSR_RI));
+}
+
static inline void arch_local_irq_enable(void)
{
if (IS_ENABLED(CONFIG_BOOKE))
@@ -343,6 +353,11 @@ static inline bool arch_irqs_disabled(void)
#define hard_irq_disable() arch_local_irq_disable()
+#define __hard_irq_enable()arch_local_irq_enable()
+#define __hard_irq_disable() arch_local_irq_disable()
+#define __hard_EE_RI_disable() arch_local_recovery_disable()
+#define __hard_RI_enable() arch_local_irq_disable()
+
static inline bool arch_irq_disabled_regs(struct pt_regs *regs)
{
return !(regs->msr & MSR_EE);
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index 92bcd1a26d73..1100c13b6d9e 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -62,6 +62,8 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address,
bool is_write)
{
