* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Russell Currey @ 2019-02-07 6:28 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <875ztwkyqu.fsf@concordia.ellerman.id.au>
On Thu, 2019-02-07 at 15:29 +1100, Michael Ellerman wrote:
> Russell Currey <ruscur@russell.cc> writes:
> >
> > Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of
> > user
> > space")
>
> Don't word wrap the fixes line please.
My bad, will teach my editor :)
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > ---
> > arch/powerpc/include/asm/cpuidle.h | 1 +
> > arch/powerpc/kernel/asm-offsets.c | 1 +
> > arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
> > 3 files changed, 22 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/cpuidle.h
> > b/arch/powerpc/include/asm/cpuidle.h
> > index 43e5f31fe64d..ad67dbe59498 100644
> > --- a/arch/powerpc/include/asm/cpuidle.h
> > +++ b/arch/powerpc/include/asm/cpuidle.h
> > @@ -77,6 +77,7 @@ struct stop_sprs {
> > u64 mmcr1;
> > u64 mmcr2;
> > u64 mmcra;
> > + u64 iamr;
> > };
>
> We don't actually need to put this in the paca anymore.
>
> > diff --git a/arch/powerpc/kernel/idle_book3s.S
> > b/arch/powerpc/kernel/idle_book3s.S
> > index 7f5ac2e8581b..bb4f552f6c7e 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -200,6 +200,12 @@ pnv_powersave_common:
> > /* Continue saving state */
> > SAVE_GPR(2, r1)
> > SAVE_NVGPRS(r1)
> > +
> > +BEGIN_FTR_SECTION
> > + mfspr r5, SPRN_IAMR
> > + std r5, STOP_IAMR(r13)
> > +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
>
> We have space for a full pt_regs on the stack, and we're not using it
> all.
>
> We don't have a specific slot for the IAMR (we may want to in
> future),
> but for now you could follow the time-honoured tradition of (ab)using
> the _DAR slot, with an appropriate comment.
I read this, then did it, and when writing the comment I thought I was
clever using "(ab)use". I then reread this and realised I just
subconsciously stole it.
Thanks for the review.
> cheers
^ permalink raw reply
* Re: [PATCH v16 16/21] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
From: Christophe Leroy @ 2019-02-07 6:07 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev, Michael Ellerman
In-Reply-To: <1549515139.mlgyosrnpx.astroid@bobo.none>
Le 07/02/2019 à 05:53, Nicholas Piggin a écrit :
> Michael Ellerman's on February 5, 2019 9:32 pm:
>> From: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> This patch activates CONFIG_THREAD_INFO_IN_TASK which
>> moves the thread_info into task_struct.
>>
>> Moving thread_info into task_struct has the following advantages:
>> - It protects thread_info from corruption in the case of stack
>> overflows.
>> - Its address is harder to determine if stack addresses are leaked,
>> making a number of attacks more difficult.
>>
>> This has the following consequences:
>> - thread_info is now located at the beginning of task_struct.
>> - The 'cpu' field is now in task_struct, and only exists when
>> CONFIG_SMP is active.
>> - thread_info doesn't have anymore the 'task' field.
>>
>> This patch:
>> - Removes all recopy of thread_info struct when the stack changes.
>> - Changes the CURRENT_THREAD_INFO() macro to point to current.
>> - Selects CONFIG_THREAD_INFO_IN_TASK.
>> - Modifies raw_smp_processor_id() to get ->cpu from current without
>> including linux/sched.h to avoid circular inclusion and without
>> including asm/asm-offsets.h to avoid symbol names duplication
>> between ASM constants and C constants.
>
> Come to think of it, can this patch be split out entirely and moved
> earlier as a 32-bit patch? 64-bit does not require that change or the
> additional build step AFAIKS?
Euh ... we may do that but the change in smp.h cannot go as is until
thread_info is moved into current. So it would mean only having the
Makefile change and the GENERATING_ASM_OFFSETS define in asm-offsets.c,
and eventually an intermediate version of raw_smp_processor_id() in
smp.h, that would have to get modified in the activation patch anyway.
So I'm not sure this is really worth it.
Christophe
^ permalink raw reply
* Re: [PATCH] powerpc/44x: Force PCI on for CURRITUCK
From: Christoph Hellwig @ 2019-02-07 5:37 UTC (permalink / raw)
To: Michael Ellerman; +Cc: bhelgaas, linuxppc-dev, rdunlap, geert, hch
In-Reply-To: <20190207024935.30456-1-mpe@ellerman.id.au>
Thanks, this looks good to me:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* [PATCH 2/2] powerpc: Enable kcov
From: Andrew Donnellan @ 2019-02-07 5:33 UTC (permalink / raw)
To: linuxppc-dev; +Cc: syzkaller, dvyukov
In-Reply-To: <20190207053324.17541-1-andrew.donnellan@au1.ibm.com>
kcov provides kernel coverage data that's useful for fuzzing tools like
syzkaller.
Wire up kcov support on powerpc. Disable kcov instrumentation on the same
files where we currently disable gcov and UBSan instrumentation.
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Acked-by: Dmitry Vyukov <dvyukov@google.com>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/Makefile | 7 ++++++-
arch/powerpc/kernel/trace/Makefile | 3 ++-
arch/powerpc/kernel/vdso32/Makefile | 1 +
arch/powerpc/kernel/vdso64/Makefile | 1 +
arch/powerpc/xmon/Makefile | 1 +
6 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2890d36eb531..d3698dae0e60 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -134,6 +134,7 @@ config PPC
select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_FORTIFY_SOURCE
select ARCH_HAS_GCOV_PROFILE_ALL
+ select ARCH_HAS_KCOV
select ARCH_HAS_PHYS_TO_DMA
select ARCH_HAS_PMEM_API if PPC64
select ARCH_HAS_PTE_SPECIAL
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index cb7f0bb9ee71..961f44eabb65 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -142,16 +142,21 @@ endif
obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o
obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
-# Disable GCOV & sanitizers in odd or sensitive code
+# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_prom_init.o := n
+KCOV_INSTRUMENT_prom_init.o := n
UBSAN_SANITIZE_prom_init.o := n
GCOV_PROFILE_machine_kexec_64.o := n
+KCOV_INSTRUMENT_machine_kexec_64.o := n
UBSAN_SANITIZE_machine_kexec_64.o := n
GCOV_PROFILE_machine_kexec_32.o := n
+KCOV_INSTRUMENT_machine_kexec_32.o := n
UBSAN_SANITIZE_machine_kexec_32.o := n
GCOV_PROFILE_kprobes.o := n
+KCOV_INSTRUMENT_kprobes.o := n
UBSAN_SANITIZE_kprobes.o := n
GCOV_PROFILE_kprobes-ftrace.o := n
+KCOV_INSTRUMENT_kprobes-ftrace.o := n
UBSAN_SANITIZE_kprobes-ftrace.o := n
UBSAN_SANITIZE_vdso.o := n
diff --git a/arch/powerpc/kernel/trace/Makefile b/arch/powerpc/kernel/trace/Makefile
index b1725ad3e13d..858503775c58 100644
--- a/arch/powerpc/kernel/trace/Makefile
+++ b/arch/powerpc/kernel/trace/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_TRACING) += trace_clock.o
obj-$(CONFIG_PPC64) += $(obj64-y)
obj-$(CONFIG_PPC32) += $(obj32-y)
-# Disable GCOV & sanitizers in odd or sensitive code
+# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_ftrace.o := n
+KCOV_INSTRUMENT_ftrace.o := n
UBSAN_SANITIZE_ftrace.o := n
diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
index 50112d4473bb..ce199f6e4256 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -23,6 +23,7 @@ targets := $(obj-vdso32) vdso32.so vdso32.so.dbg
obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
GCOV_PROFILE := n
+KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
ccflags-y := -shared -fno-common -fno-builtin
diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
index 69cecb346269..28e7d112aa2f 100644
--- a/arch/powerpc/kernel/vdso64/Makefile
+++ b/arch/powerpc/kernel/vdso64/Makefile
@@ -9,6 +9,7 @@ targets := $(obj-vdso64) vdso64.so vdso64.so.dbg
obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
GCOV_PROFILE := n
+KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
ccflags-y := -shared -fno-common -fno-builtin
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index 878f9c1d3615..3050f9323254 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -5,6 +5,7 @@
subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
GCOV_PROFILE := n
+KCOV_INSTRUMENT := n
UBSAN_SANITIZE := n
# Disable ftrace for the entire directory
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2] powerpc/64s: Work around spurious warning on old gccs with -fsanitize-coverage
From: Andrew Donnellan @ 2019-02-07 5:33 UTC (permalink / raw)
To: linuxppc-dev; +Cc: syzkaller, dvyukov
Some older gccs (<GCC 7), when invoked with -fsanitize-coverage=trace-pc,
cause a spurious uninitialised variable warning in dt_cpu_ftrs.c:
arch/powerpc/kernel/dt_cpu_ftrs.c: In function ‘cpufeatures_process_feature’:
arch/powerpc/kernel/dt_cpu_ftrs.c:686:7: warning: ‘m’ may be used uninitialized in this function [-Wmaybe-uninitialized]
if (m->cpu_ftr_bit_mask)
An upcoming patch will enable support for kcov, which requires
-fsanitize-coverage=trace-pc.
Work around this by explicitly initialising m to NULL.
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
arch/powerpc/kernel/dt_cpu_ftrs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 8be3721d9302..2192b2114513 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -658,7 +658,7 @@ static void __init cpufeatures_setup_start(u32 isa)
static bool __init cpufeatures_process_feature(struct dt_cpu_feature *f)
{
- const struct dt_cpu_feature_match *m;
+ const struct dt_cpu_feature_match *m = NULL;
bool known = false;
int i;
--
2.11.0
^ permalink raw reply related
* [PATCH 2/2] powerpc: Fix defconfig choice logic when cross compiling
From: Michael Ellerman @ 2019-02-07 5:16 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20190207051652.11380-1-mpe@ellerman.id.au>
Our logic for choosing defconfig doesn't work well in some situations.
For example if you're on a ppc64le machine but you specify a non-empty
CROSS_COMPILE, in order to use a non-default toolchain, then defconfig
will give you ppc64_defconfig (big endian):
$ make CROSS_COMPILE=~/toolchains/gcc-8/bin/powerpc-linux- defconfig
*** Default configuration is based on 'ppc64_defconfig'
This is because we assume that CROSS_COMPILE being set means we
can't be on a ppc machine and rather than checking we just default to
ppc64_defconfig.
We should just ignore CROSS_COMPILE, instead check the machine with
uname and if it's one of ppc, ppc64 or ppc64le then use that
defconfig. If it's none of those then we fall back to ppc64_defconfig.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/Makefile | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 70e6e8119aeb..81563986a30e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -34,11 +34,10 @@ ifdef CONFIG_PPC_BOOK3S_32
KBUILD_CFLAGS += -mcpu=powerpc
endif
-ifeq ($(CROSS_COMPILE),)
-KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
-else
-KBUILD_DEFCONFIG := ppc64_defconfig
-endif
+# If we're on a ppc/ppc64/ppc64le machine use that defconfig, otherwise just use
+# ppc64_defconfig because we have nothing better to go on.
+uname := $(shell uname -m)
+KBUILD_DEFCONFIG := $(if $(filter ppc%,$(uname)),$(uname),ppc64)_defconfig
ifdef CONFIG_PPC64
new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' > /dev/null; then echo y; else echo n; fi)
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] powerpc/32: Add ppc_defconfig
From: Michael Ellerman @ 2019-02-07 5:16 UTC (permalink / raw)
To: linuxppc-dev
Add a generic 32-bit defconfig called ppc_defconfig. This means we'll
have a defconfig matching "uname -m" for all cases.
This config is mostly intended for build testing but if someone wants
to tweak it to get it booting on something that would be fine too.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index ac033341ed55..70e6e8119aeb 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -367,6 +367,10 @@ PHONY += ppc32_allmodconfig
$(Q)$(MAKE) KCONFIG_ALLCONFIG=$(srctree)/arch/powerpc/configs/book3s_32.config \
-f $(srctree)/Makefile allmodconfig
+PHONY += ppc_defconfig
+ppc_defconfig:
+ $(call merge_into_defconfig,book3s_32.config,)
+
PHONY += ppc64le_allmodconfig
ppc64le_allmodconfig:
$(Q)$(MAKE) KCONFIG_ALLCONFIG=$(srctree)/arch/powerpc/configs/le.config \
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/mm: move a KERN_WARNING message to pr_debug()
From: David Gibson @ 2019-02-07 3:03 UTC (permalink / raw)
To: Laurent Vivier; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20190205202133.5048-1-lvivier@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2199 bytes --]
On Tue, Feb 05, 2019 at 09:21:33PM +0100, Laurent Vivier wrote:
> resize_hpt_for_hotplug() reports a warning when it cannot
> increase the hash page table ("Unable to resize hash page
> table to target order") but this is not blocking and
> can make user thinks something has not worked properly.
> As we move the message to the debug area, report again the
> ENODEV error.
>
> If the operation cannot be done the real error message
> will be reported by arch_add_memory() if create_section_mapping()
> fails.
>
> Fixes: 7339390d772dd
> powerpc/pseries: Don't give a warning when HPT resizing isn't available
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Sorry, I'm pretty dubious about this. It's true that in the case for
which this bug was filed this is a harmless situation which deserves a
pr_debug() at most.
But that's not necessarily true in all paths leading to this message.
It will also trip if we fail to reshrink the HPT after genuinely
hotunplugging a bunch of memory, in which case failing to release
expected resources does deserve a warning.
> ---
>
> Notes:
> v2:
> - use pr_debug instead of printk(KERN_DEBUG
> - remove check for ENODEV
>
> arch/powerpc/mm/hash_utils_64.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index 0cc7fbc3bd1c..6a0cc4eb2c83 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -777,10 +777,9 @@ void resize_hpt_for_hotplug(unsigned long new_mem_size)
> int rc;
>
> rc = mmu_hash_ops.resize_hpt(target_hpt_shift);
> - if (rc && (rc != -ENODEV))
> - printk(KERN_WARNING
> - "Unable to resize hash page table to target order %d: %d\n",
> - target_hpt_shift, rc);
> + if (rc)
> + pr_debug("Unable to resize hash page table to target order %d: %d\n",
> + target_hpt_shift, rc);
> }
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v16 05/21] powerpc: prep stack walkers for THREAD_INFO_IN_TASK
From: Nicholas Piggin @ 2019-02-07 4:05 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20190205113219.17903-6-mpe@ellerman.id.au>
Michael Ellerman's on February 5, 2019 9:32 pm:
> From: Christophe Leroy <christophe.leroy@c-s.fr>
>
> [text copied from commit 9bbd4c56b0b6
> ("arm64: prep stack walkers for THREAD_INFO_IN_TASK")]
>
> When CONFIG_THREAD_INFO_IN_TASK is selected, task stacks may be freed
> before a task is destroyed. To account for this, the stacks are
> refcounted, and when manipulating the stack of another task, it is
> necessary to get/put the stack to ensure it isn't freed and/or re-used
> while we do so.
>
> This patch reworks the powerpc stack walking code to account for this.
> When CONFIG_THREAD_INFO_IN_TASK is not selected these perform no
> refcounting, and this should only be a structural change that does not
> affect behaviour.
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Seems to do what is advertised and asked for by the CONFIG option. I
don't see any other code that uses non-current stack.
Acked-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Nicholas Piggin @ 2019-02-07 5:08 UTC (permalink / raw)
To: linuxppc-dev, Russell Currey
In-Reply-To: <20190206062837.26917-1-ruscur@russell.cc>
Russell Currey's on February 6, 2019 4:28 pm:
> Without restoring the IAMR after idle, execution prevention on POWER9
> with Radix MMU is overwritten and the kernel can freely execute userspace without
> faulting.
>
> This is necessary when returning from any stop state that modifies user
> state, as well as hypervisor state.
>
> To test how this fails without this patch, load the lkdtm driver and
> do the following:
>
> echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
>
> which won't fault, then boot the kernel with powersave=off, where it
> will fault. Applying this patch will fix this.
>
> Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
> space")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
Good catch and debugging. This really should be a quirk, we don't want
to have to restore this thing on a thread switch.
Can we put it under a CONFIG option if we're not using IAMR?
> ---
> arch/powerpc/include/asm/cpuidle.h | 1 +
> arch/powerpc/kernel/asm-offsets.c | 1 +
> arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 43e5f31fe64d..ad67dbe59498 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -77,6 +77,7 @@ struct stop_sprs {
> u64 mmcr1;
> u64 mmcr2;
> u64 mmcra;
> + u64 iamr;
> };
>
> #define PNV_IDLE_NAME_LEN 16
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index 9ffc72ded73a..10e0314c2b0d 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -774,6 +774,7 @@ int main(void)
> STOP_SPR(STOP_MMCR1, mmcr1);
> STOP_SPR(STOP_MMCR2, mmcr2);
> STOP_SPR(STOP_MMCRA, mmcra);
> + STOP_SPR(STOP_IAMR, iamr);
> #endif
>
> DEFINE(PPC_DBELL_SERVER, PPC_DBELL_SERVER);
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 7f5ac2e8581b..bb4f552f6c7e 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -200,6 +200,12 @@ pnv_powersave_common:
> /* Continue saving state */
> SAVE_GPR(2, r1)
> SAVE_NVGPRS(r1)
> +
> +BEGIN_FTR_SECTION
> + mfspr r5, SPRN_IAMR
> + std r5, STOP_IAMR(r13)
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> +
> mfcr r5
> std r5,_CCR(r1)
> std r1,PACAR1(r13)
> @@ -924,6 +930,13 @@ BEGIN_FTR_SECTION
> END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> REST_NVGPRS(r1)
> REST_GPR(2, r1)
> +
> +BEGIN_FTR_SECTION
> + ld r4, STOP_IAMR(r13)
> + mtspr SPRN_IAMR, r4
> + isync
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
Sigh, good old isync. Suspect you'll get away without it, mtmsrd L=0
just below is architecturally guaranteeing a CSI, so just add a comment
there, might save a flush.
> +
> ld r4,PACAKMSR(r13)
> ld r5,_LINK(r1)
> ld r6,_CCR(r1)
> @@ -946,6 +959,13 @@ pnv_wakeup_noloss:
> BEGIN_FTR_SECTION
> CHECK_HMI_INTERRUPT
> END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> +
> +BEGIN_FTR_SECTION
> + ld r4, STOP_IAMR(r13)
> + mtspr SPRN_IAMR, r4
> + isync
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
For the noloss part, it should mean really nothing lost including GPRs,
so I think IAMR *should* be okay here.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 1/2] mm: add probe_user_read()
From: Michael Ellerman @ 2019-02-07 5:04 UTC (permalink / raw)
To: Murilo Opsfelder Araujo, Christophe Leroy
Cc: Kees Cook, linux-kernel, Mike Rapoport, linux-mm, Paul Mackerras,
Andrew Morton, linuxppc-dev
In-Reply-To: <20190205174242.GA24427@kermit.br.ibm.com>
Murilo Opsfelder Araujo <muriloo@linux.ibm.com> writes:
>> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
>> index 37b226e8df13..ef99edd63da3 100644
>> --- a/include/linux/uaccess.h
>> +++ b/include/linux/uaccess.h
>> @@ -263,6 +263,40 @@ extern long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count);
>> #define probe_kernel_address(addr, retval) \
>> probe_kernel_read(&retval, addr, sizeof(retval))
>>
>> +/**
>> + * probe_user_read(): safely attempt to read from a user location
>> + * @dst: pointer to the buffer that shall take the data
>> + * @src: address to read from
>> + * @size: size of the data chunk
>> + *
>> + * Safely read from address @src to the buffer at @dst. If a kernel fault
>> + * happens, handle that and return -EFAULT.
>> + *
>> + * We ensure that the copy_from_user is executed in atomic context so that
>> + * do_page_fault() doesn't attempt to take mmap_sem. This makes
>> + * probe_user_read() suitable for use within regions where the caller
>> + * already holds mmap_sem, or other locks which nest inside mmap_sem.
>> + *
>> + * Returns: 0 on success, -EFAULT on error.
>> + */
>> +
>> +#ifndef probe_user_read
>> +static __always_inline long probe_user_read(void *dst, const void __user *src,
>> + size_t size)
>> +{
>> + long ret;
>> +
>> + if (!access_ok(src, size))
>> + return -EFAULT;
>
> Hopefully, there is still time for a minor comment.
>
> Do we need to differentiate the returned error here, e.g.: return
> -EACCES?
>
> I wonder if there will be situations where callers need to know why
> probe_user_read() failed.
It's pretty standard to return EFAULT when an access_ok() check fails,
so I think using EFAULT here is the safest option.
If we used EACCES we'd need to be more careful about converting code to
use this helper, as doing so would potentially cause the error value to
change, which in some cases is not OK.
cheers
^ permalink raw reply
* [PATCH] KVM: PPC: Book3S HV: Optimise mmio emulation for devices on FAST_MMIO_BUS
From: Suraj Jitindar Singh @ 2019-02-07 4:56 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Suraj Jitindar Singh, kvm
Devices on the KVM_FAST_MMIO_BUS by definition have length zero and are
thus used for notification purposes rather than data transfer. For
example eventfd for virtio devices.
This means that when emulating mmio instructions which target devices on
this bus we can immediately handle them and return without needing to load
the instruction from guest memory.
For now we restrict this to stores as this is the only use case at
present.
For a normal guest the effect is negligible, however for a nested guest
we save on the order of 5us per access.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index bd2dcfbf00cd..be7bc070eae5 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -442,6 +442,24 @@ int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
u32 last_inst;
/*
+ * Fast path - check if the guest physical address corresponds to a
+ * device on the FAST_MMIO_BUS, if so we can avoid loading the
+ * instruction all together, then we can just handle it and return.
+ */
+ if (is_store) {
+ int idx, ret;
+
+ idx = srcu_read_lock(&vcpu->kvm->srcu);
+ ret = kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, (gpa_t) gpa, 0,
+ NULL);
+ srcu_read_unlock(&vcpu->kvm->srcu, idx);
+ if (!ret) {
+ kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
+ return RESUME_GUEST;
+ }
+ }
+
+ /*
* If we fail, we just return to the guest and try executing it again.
*/
if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
--
2.13.6
^ permalink raw reply related
* Re: [PATCH v16 16/21] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
From: Nicholas Piggin @ 2019-02-07 4:53 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20190205113219.17903-17-mpe@ellerman.id.au>
Michael Ellerman's on February 5, 2019 9:32 pm:
> From: Christophe Leroy <christophe.leroy@c-s.fr>
>
> This patch activates CONFIG_THREAD_INFO_IN_TASK which
> moves the thread_info into task_struct.
>
> Moving thread_info into task_struct has the following advantages:
> - It protects thread_info from corruption in the case of stack
> overflows.
> - Its address is harder to determine if stack addresses are leaked,
> making a number of attacks more difficult.
>
> This has the following consequences:
> - thread_info is now located at the beginning of task_struct.
> - The 'cpu' field is now in task_struct, and only exists when
> CONFIG_SMP is active.
> - thread_info doesn't have anymore the 'task' field.
>
> This patch:
> - Removes all recopy of thread_info struct when the stack changes.
> - Changes the CURRENT_THREAD_INFO() macro to point to current.
> - Selects CONFIG_THREAD_INFO_IN_TASK.
> - Modifies raw_smp_processor_id() to get ->cpu from current without
> including linux/sched.h to avoid circular inclusion and without
> including asm/asm-offsets.h to avoid symbol names duplication
> between ASM constants and C constants.
Come to think of it, can this patch be split out entirely and moved
earlier as a 32-bit patch? 64-bit does not require that change or the
additional build step AFAIKS?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v03] powerpc/numa: Perform full re-add of CPU for PRRN/VPHN topology update
From: kbuild test robot @ 2019-02-07 4:48 UTC (permalink / raw)
To: Michael Bringmann
Cc: Rob Herring, Thomas Falcon, Srikar Dronamraju, Corentin Labbe,
linux-kernel, Nicholas Piggin, Al Viro, Michael Bringmann,
Juliet Kim, Oliver O'Halloran, kbuild-all, Nathan Lynch,
Tyrel Datwyler, linuxppc-dev, Guenter Roeck, Kees Cook
In-Reply-To: <305ed693-ea85-8a70-1d3c-ae405aebc0ad@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 6147 bytes --]
Hi Michael,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.0-rc4 next-20190206]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Michael-Bringmann/powerpc-numa-Perform-full-re-add-of-CPU-for-PRRN-VPHN-topology-update/20190207-101545
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
arch/powerpc/mm/numa.c: In function 'numa_update_cpu_topology':
>> arch/powerpc/mm/numa.c:1361:4: error: implicit declaration of function 'dlpar_cpu_readd'; did you mean 'raw_cpu_read'? [-Werror=implicit-function-declaration]
dlpar_cpu_readd(cpu);
^~~~~~~~~~~~~~~
raw_cpu_read
cc1: some warnings being treated as errors
vim +1361 arch/powerpc/mm/numa.c
1298
1299 /*
1300 * Update the node maps and sysfs entries for each cpu whose home node
1301 * has changed. Returns 1 when the topology has changed, and 0 otherwise.
1302 *
1303 * readd_cpus: Also readd any CPUs that have changed affinity
1304 */
1305 static int numa_update_cpu_topology(bool readd_cpus)
1306 {
1307 unsigned int cpu, sibling, changed = 0;
1308 struct topology_update_data *updates, *ud;
1309 cpumask_t updated_cpus;
1310 struct device *dev;
1311 int weight, new_nid, i = 0;
1312
1313 if ((!prrn_enabled && !vphn_enabled && topology_inited) ||
1314 topology_update_in_progress)
1315 return 0;
1316
1317 weight = cpumask_weight(&cpu_associativity_changes_mask);
1318 if (!weight)
1319 return 0;
1320
1321 updates = kcalloc(weight, sizeof(*updates), GFP_KERNEL);
1322 if (!updates)
1323 return 0;
1324
1325 topology_update_in_progress = 1;
1326
1327 cpumask_clear(&updated_cpus);
1328
1329 for_each_cpu(cpu, &cpu_associativity_changes_mask) {
1330 /*
1331 * If siblings aren't flagged for changes, updates list
1332 * will be too short. Skip on this update and set for next
1333 * update.
1334 */
1335 if (!cpumask_subset(cpu_sibling_mask(cpu),
1336 &cpu_associativity_changes_mask)) {
1337 pr_info("Sibling bits not set for associativity "
1338 "change, cpu%d\n", cpu);
1339 cpumask_or(&cpu_associativity_changes_mask,
1340 &cpu_associativity_changes_mask,
1341 cpu_sibling_mask(cpu));
1342 cpu = cpu_last_thread_sibling(cpu);
1343 continue;
1344 }
1345
1346 new_nid = find_and_online_cpu_nid(cpu);
1347
1348 if ((new_nid == numa_cpu_lookup_table[cpu]) ||
1349 !cpu_present(cpu)) {
1350 cpumask_andnot(&cpu_associativity_changes_mask,
1351 &cpu_associativity_changes_mask,
1352 cpu_sibling_mask(cpu));
1353 if (cpu_present(cpu))
1354 dbg("Assoc chg gives same node %d for cpu%d\n",
1355 new_nid, cpu);
1356 cpu = cpu_last_thread_sibling(cpu);
1357 continue;
1358 }
1359
1360 if (readd_cpus)
> 1361 dlpar_cpu_readd(cpu);
1362
1363 for_each_cpu(sibling, cpu_sibling_mask(cpu)) {
1364 ud = &updates[i++];
1365 ud->next = &updates[i];
1366 ud->cpu = sibling;
1367 ud->new_nid = new_nid;
1368 ud->old_nid = numa_cpu_lookup_table[sibling];
1369 cpumask_set_cpu(sibling, &updated_cpus);
1370 }
1371 cpu = cpu_last_thread_sibling(cpu);
1372 }
1373
1374 /*
1375 * Prevent processing of 'updates' from overflowing array
1376 * where last entry filled in a 'next' pointer.
1377 */
1378 if (i)
1379 updates[i-1].next = NULL;
1380
1381 pr_debug("Topology update for the following CPUs:\n");
1382 if (cpumask_weight(&updated_cpus)) {
1383 for (ud = &updates[0]; ud; ud = ud->next) {
1384 pr_debug("cpu %d moving from node %d "
1385 "to %d\n", ud->cpu,
1386 ud->old_nid, ud->new_nid);
1387 }
1388 }
1389
1390 /*
1391 * In cases where we have nothing to update (because the updates list
1392 * is too short or because the new topology is same as the old one),
1393 * skip invoking update_cpu_topology() via stop-machine(). This is
1394 * necessary (and not just a fast-path optimization) since stop-machine
1395 * can end up electing a random CPU to run update_cpu_topology(), and
1396 * thus trick us into setting up incorrect cpu-node mappings (since
1397 * 'updates' is kzalloc()'ed).
1398 *
1399 * And for the similar reason, we will skip all the following updating.
1400 */
1401 if (!cpumask_weight(&updated_cpus))
1402 goto out;
1403
1404 stop_machine(update_cpu_topology, &updates[0], &updated_cpus);
1405
1406 /*
1407 * Update the numa-cpu lookup table with the new mappings, even for
1408 * offline CPUs. It is best to perform this update from the stop-
1409 * machine context.
1410 */
1411 stop_machine(update_lookup_table, &updates[0],
1412 cpumask_of(raw_smp_processor_id()));
1413
1414 for (ud = &updates[0]; ud; ud = ud->next) {
1415 unregister_cpu_under_node(ud->cpu, ud->old_nid);
1416 register_cpu_under_node(ud->cpu, ud->new_nid);
1417
1418 dev = get_cpu_device(ud->cpu);
1419 if (dev)
1420 kobject_uevent(&dev->kobj, KOBJ_CHANGE);
1421 cpumask_clear_cpu(ud->cpu, &cpu_associativity_changes_mask);
1422 changed = 1;
1423 }
1424
1425 out:
1426 topology_changed = changed;
1427 topology_update_in_progress = 0;
1428 kfree(updates);
1429 return changed;
1430 }
1431
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59726 bytes --]
^ permalink raw reply
* Re: [PATCH v16 16/21] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
From: Nicholas Piggin @ 2019-02-07 4:44 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20190205113219.17903-17-mpe@ellerman.id.au>
Michael Ellerman's on February 5, 2019 9:32 pm:
> From: Christophe Leroy <christophe.leroy@c-s.fr>
>
> This patch activates CONFIG_THREAD_INFO_IN_TASK which
> moves the thread_info into task_struct.
>
> Moving thread_info into task_struct has the following advantages:
> - It protects thread_info from corruption in the case of stack
> overflows.
> - Its address is harder to determine if stack addresses are leaked,
> making a number of attacks more difficult.
>
> This has the following consequences:
> - thread_info is now located at the beginning of task_struct.
> - The 'cpu' field is now in task_struct, and only exists when
> CONFIG_SMP is active.
> - thread_info doesn't have anymore the 'task' field.
>
> This patch:
> - Removes all recopy of thread_info struct when the stack changes.
> - Changes the CURRENT_THREAD_INFO() macro to point to current.
> - Selects CONFIG_THREAD_INFO_IN_TASK.
> - Modifies raw_smp_processor_id() to get ->cpu from current without
> including linux/sched.h to avoid circular inclusion and without
> including asm/asm-offsets.h to avoid symbol names duplication
> between ASM constants and C constants.
> - Modifies klp_init_thread_info() to take a task_struct pointer
> argument.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> [mpe: Add task_stack.h to livepatch.h to fix build fails]
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
Nothing stands out here since my last review that I can see.
raw_smp_processor_id change is for 32-bit only, of course.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v16 19/21] powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU
From: Nicholas Piggin @ 2019-02-07 4:41 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <1549513559.eo06g2mjtn.astroid@bobo.none>
Nicholas Piggin's on February 7, 2019 2:27 pm:
> Michael Ellerman's on February 5, 2019 9:32 pm:
>> From: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> Now that thread_info is similar to task_struct, its address is in r2
>> so CURRENT_THREAD_INFO() macro is useless. This patch removes it.
>
> You wouldn't want to keep CURRENT_THREAD_INFO(r2) to mean get the thread
> info out of current, as just a no-op for documentation purpose? I don't
> know /32 code at all so it's up to you.
I see that I said roughly the opposite last year, if that helps to
determine the importance of my opinion.
Thanks,
Nick
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-02-07 4:34 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <20190206151655.GA31172@lst.de>
Hi Christoph,
I also didn’t notice the 32-bit DMA mask in your patch. I have to read your patches and descriptions carefully in the future. I will test your new patch at the weekend.
Thanks,
Christian
Sent from my iPhone
> On 6. Feb 2019, at 16:16, Christoph Hellwig <hch@lst.de> wrote:
>
>> On Wed, Feb 06, 2019 at 04:15:05PM +0100, Christoph Hellwig wrote:
>> The last good one was 29e7e2287e196f48fe5d2a6e017617723ea979bf
>> ("dma-direct: we might need GFP_DMA for 32-bit dma masks"), if I
>> remember correctly. powerpc/dma: use the dma_direct mapping routines
>> was the one that you said makes the pasemi ethernet stop working.
>>
>> Can you post the dmesg from the failing runs?
>
> But I just noticed I sent you a wrong patch - the pasemi ethernet
> should set a 64-bit DMA mask, not 32-bit. Updated version below,
> 32-bit would just keep the previous status quo.
>
> commit 6c8f88045dee35933337b9ce2ea5371eee37073a
> Author: Christoph Hellwig <hch@lst.de>
> Date: Mon Feb 4 13:38:22 2019 +0100
>
> pasemi WIP
>
> diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
> index 8a31a02c9f47..2d7d1589490a 100644
> --- a/drivers/net/ethernet/pasemi/pasemi_mac.c
> +++ b/drivers/net/ethernet/pasemi/pasemi_mac.c
> @@ -1716,6 +1716,7 @@ pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> err = -ENODEV;
> goto out;
> }
> + dma_set_mask(&mac->dma_pdev->dev, DMA_BIT_MASK(64));
>
> mac->iob_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa001, NULL);
> if (!mac->iob_pdev) {
^ permalink raw reply
* Re: [PATCH v2] powerpc/mm: move a KERN_WARNING message to pr_debug()
From: Michael Ellerman @ 2019-02-07 4:33 UTC (permalink / raw)
To: Laurent Vivier, linux-kernel; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <20190205202133.5048-1-lvivier@redhat.com>
Hi Laurent,
I'm not sure I'm convinced about this one. It seems like we're just
throwing away the warning because it's annoying.
Laurent Vivier <lvivier@redhat.com> writes:
> resize_hpt_for_hotplug() reports a warning when it cannot
> increase the hash page table ("Unable to resize hash page
> table to target order") but this is not blocking and
> can make user thinks something has not worked properly.
Something did not work properly, the resize didn't work properly. Right?
> As we move the message to the debug area, report again the
> ENODEV error.
>
> If the operation cannot be done the real error message
> will be reported by arch_add_memory() if create_section_mapping()
> fails.
Can you explain that more. Isn't the fact that the resize failed "the
real error message"?
> Fixes: 7339390d772dd
> powerpc/pseries: Don't give a warning when HPT resizing isn't available
This should all be on one line, and formatted as:
Fixes: 7339390d772d ("powerpc/pseries: Don't give a warning when HPT resizing isn't available")
See Documentation/process/submitting-patches.rst for more info and how
to configure git to do it automatically for you.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/idle: Restore IAMR after idle
From: Michael Ellerman @ 2019-02-07 4:29 UTC (permalink / raw)
To: Russell Currey, linuxppc-dev
In-Reply-To: <20190206062837.26917-1-ruscur@russell.cc>
Russell Currey <ruscur@russell.cc> writes:
> Without restoring the IAMR after idle, execution prevention on POWER9
> with Radix MMU is overwritten and the kernel can freely execute userspace without
> faulting.
>
> This is necessary when returning from any stop state that modifies user
> state, as well as hypervisor state.
>
> To test how this fails without this patch, load the lkdtm driver and
> do the following:
>
> echo EXEC_USERSPACE > /sys/kernel/debug/provoke-crash/DIRECT
>
> which won't fault, then boot the kernel with powersave=off, where it
> will fault. Applying this patch will fix this.
>
> Fixes: 3b10d0095a1e ("powerpc/mm/radix: Prevent kernel execution of user
> space")
Don't word wrap the fixes line please.
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
> arch/powerpc/include/asm/cpuidle.h | 1 +
> arch/powerpc/kernel/asm-offsets.c | 1 +
> arch/powerpc/kernel/idle_book3s.S | 20 ++++++++++++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 43e5f31fe64d..ad67dbe59498 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -77,6 +77,7 @@ struct stop_sprs {
> u64 mmcr1;
> u64 mmcr2;
> u64 mmcra;
> + u64 iamr;
> };
We don't actually need to put this in the paca anymore.
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 7f5ac2e8581b..bb4f552f6c7e 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -200,6 +200,12 @@ pnv_powersave_common:
> /* Continue saving state */
> SAVE_GPR(2, r1)
> SAVE_NVGPRS(r1)
> +
> +BEGIN_FTR_SECTION
> + mfspr r5, SPRN_IAMR
> + std r5, STOP_IAMR(r13)
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
We have space for a full pt_regs on the stack, and we're not using it
all.
We don't have a specific slot for the IAMR (we may want to in future),
but for now you could follow the time-honoured tradition of (ab)using
the _DAR slot, with an appropriate comment.
cheers
^ permalink raw reply
* Re: [PATCH v16 19/21] powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU
From: Nicholas Piggin @ 2019-02-07 4:27 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20190205113219.17903-20-mpe@ellerman.id.au>
Michael Ellerman's on February 5, 2019 9:32 pm:
> From: Christophe Leroy <christophe.leroy@c-s.fr>
>
> Now that thread_info is similar to task_struct, its address is in r2
> so CURRENT_THREAD_INFO() macro is useless. This patch removes it.
You wouldn't want to keep CURRENT_THREAD_INFO(r2) to mean get the thread
info out of current, as just a no-op for documentation purpose? I don't
know /32 code at all so it's up to you.
> This patch also moves the 'tovirt(r2, r2)' down just before the
> reactivation of MMU translation, so that we keep the physical address
> of 'current' in r2 until then. It avoids a few calls to tophys().
>
> At the same time, as the 'cpu' field is not anymore in thread_info,
> TI_CPU is renamed TASK_CPU by this patch.
>
> It also allows to get rid of a couple of
> '#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE' as ACCOUNT_CPU_USER_ENTRY()
> and ACCOUNT_CPU_USER_EXIT() are empty when
> CONFIG_VIRT_CPU_ACCOUNTING_NATIVE is not defined.
May want to split these a little more?
^ permalink raw reply
* Re: [PATCH v16 20/21] powerpc/64: Replace CURRENT_THREAD_INFO with PACA_CURRENT_TI
From: Nicholas Piggin @ 2019-02-07 4:24 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20190205113219.17903-21-mpe@ellerman.id.au>
Michael Ellerman's on February 5, 2019 9:32 pm:
> From: Christophe Leroy <christophe.leroy@c-s.fr>
>
> Now that current_thread_info is located at the beginning of 'current'
> task struct, CURRENT_THREAD_INFO macro is not really needed any more.
>
> This patch replaces it by loads of the value at PACA_CURRENT_TI(r13).
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> [mpe: Add PACA_CURRENT_TI rather than using PACACURRENT]
Okay. Does it need to be CURRENT_TI not just TI (or THREAD_INFO may be
more descriptive)? TI/THREAD_INFO implies the current one I think.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply
* Re: [PATCH v16 14/21] powerpc: Use task_stack_page() in current_pt_regs()
From: Nicholas Piggin @ 2019-02-07 4:13 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20190205113219.17903-15-mpe@ellerman.id.au>
Michael Ellerman's on February 5, 2019 9:32 pm:
> From: Christophe Leroy <christophe.leroy@c-s.fr>
>
> Change current_pt_regs() to use task_stack_page() rather than
> current_thread_info() so that it keeps working once we enable
> THREAD_INFO_IN_TASK.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> [mpe: Split out of large patch]
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply
* Re: [PATCH v16 01/21] powerpc/irq: use memblock functions returning virtual address
From: Nicholas Piggin @ 2019-02-07 3:54 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20190205113219.17903-2-mpe@ellerman.id.au>
Michael Ellerman's on February 5, 2019 9:31 pm:
> From: Christophe Leroy <christophe.leroy@c-s.fr>
>
> Since only the virtual address of allocated blocks is used,
> lets use functions returning directly virtual address.
>
> Those functions have the advantage of also zeroing the block.
>
> Suggested-by: Mike Rapoport <rppt@linux.ibm.com>
> Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/kernel/irq.c | 5 -----
> arch/powerpc/kernel/setup_32.c | 26 ++++++++++++++++----------
> arch/powerpc/kernel/setup_64.c | 19 +++++++------------
> 3 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index bb299613a462..4a5dd8800946 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -725,18 +725,15 @@ void exc_lvl_ctx_init(void)
> #endif
> #endif
>
> - memset((void *)critirq_ctx[cpu_nr], 0, THREAD_SIZE);
> tp = critirq_ctx[cpu_nr];
> tp->cpu = cpu_nr;
> tp->preempt_count = 0;
>
> #ifdef CONFIG_BOOKE
> - memset((void *)dbgirq_ctx[cpu_nr], 0, THREAD_SIZE);
> tp = dbgirq_ctx[cpu_nr];
> tp->cpu = cpu_nr;
> tp->preempt_count = 0;
>
> - memset((void *)mcheckirq_ctx[cpu_nr], 0, THREAD_SIZE);
> tp = mcheckirq_ctx[cpu_nr];
> tp->cpu = cpu_nr;
> tp->preempt_count = HARDIRQ_OFFSET;
> @@ -754,12 +751,10 @@ void irq_ctx_init(void)
> int i;
>
> for_each_possible_cpu(i) {
> - memset((void *)softirq_ctx[i], 0, THREAD_SIZE);
> tp = softirq_ctx[i];
> tp->cpu = i;
> klp_init_thread_info(tp);
>
> - memset((void *)hardirq_ctx[i], 0, THREAD_SIZE);
> tp = hardirq_ctx[i];
> tp->cpu = i;
> klp_init_thread_info(tp);
> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> index 947f904688b0..1f0b7629c1a6 100644
> --- a/arch/powerpc/kernel/setup_32.c
> +++ b/arch/powerpc/kernel/setup_32.c
> @@ -196,6 +196,17 @@ static int __init ppc_init(void)
> }
> arch_initcall(ppc_init);
>
> +static void *__init alloc_stack(void)
> +{
> + void *ptr = memblock_alloc(THREAD_SIZE, THREAD_SIZE);
> +
> + if (!ptr)
> + panic("cannot allocate %d bytes for stack at %pS\n",
> + THREAD_SIZE, (void *)_RET_IP_);
> +
> + return ptr;
> +}
> +
> void __init irqstack_early_init(void)
> {
> unsigned int i;
> @@ -203,10 +214,8 @@ void __init irqstack_early_init(void)
> /* interrupt stacks must be in lowmem, we get that for free on ppc32
> * as the memblock is limited to lowmem by default */
> for_each_possible_cpu(i) {
> - softirq_ctx[i] = (struct thread_info *)
> - __va(memblock_phys_alloc(THREAD_SIZE, THREAD_SIZE));
> - hardirq_ctx[i] = (struct thread_info *)
> - __va(memblock_phys_alloc(THREAD_SIZE, THREAD_SIZE));
> + softirq_ctx[i] = alloc_stack();
> + hardirq_ctx[i] = alloc_stack();
> }
> }
>
> @@ -224,13 +233,10 @@ void __init exc_lvl_early_init(void)
> hw_cpu = 0;
> #endif
>
> - critirq_ctx[hw_cpu] = (struct thread_info *)
> - __va(memblock_phys_alloc(THREAD_SIZE, THREAD_SIZE));
> + critirq_ctx[hw_cpu] = alloc_stack();
> #ifdef CONFIG_BOOKE
> - dbgirq_ctx[hw_cpu] = (struct thread_info *)
> - __va(memblock_phys_alloc(THREAD_SIZE, THREAD_SIZE));
> - mcheckirq_ctx[hw_cpu] = (struct thread_info *)
> - __va(memblock_phys_alloc(THREAD_SIZE, THREAD_SIZE));
> + dbgirq_ctx[hw_cpu] = alloc_stack();
> + mcheckirq_ctx[hw_cpu] = alloc_stack();
> #endif
> }
> }
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 236c1151a3a7..080dd515d587 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -634,19 +634,17 @@ __init u64 ppc64_bolted_size(void)
>
> static void *__init alloc_stack(unsigned long limit, int cpu)
> {
> - unsigned long pa;
> + void *ptr;
>
> BUILD_BUG_ON(STACK_INT_FRAME_SIZE % 16);
>
> - pa = memblock_alloc_base_nid(THREAD_SIZE, THREAD_SIZE, limit,
> - early_cpu_to_node(cpu), MEMBLOCK_NONE);
> - if (!pa) {
> - pa = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
> - if (!pa)
> - panic("cannot allocate stacks");
> - }
> + ptr = memblock_alloc_try_nid(THREAD_SIZE, THREAD_SIZE,
> + MEMBLOCK_LOW_LIMIT, limit,
> + early_cpu_to_node(cpu));
This is much nicer. Looks like removing MEMBLOCK_LOW_LIMIT would be a
cleanup for generic kernel code. Actually some of these calls are used
only by powerpc (memblock_alloc_base_nid) which probably could get
tidied up. Something for a rainy day or a beginner.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply
* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: David Gibson @ 2019-02-07 2:51 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev
In-Reply-To: <bf3725b6-9881-74a0-ec4f-43ac409cb496@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]
On Wed, Feb 06, 2019 at 08:35:24AM +0100, Cédric Le Goater wrote:
> On 2/6/19 2:18 AM, David Gibson wrote:
> > On Wed, Feb 06, 2019 at 09:13:15AM +1100, Paul Mackerras wrote:
> >> On Tue, Feb 05, 2019 at 12:31:28PM +0100, Cédric Le Goater wrote:
> >>>>>> As for nesting, I suggest for the foreseeable future we stick to XICS
> >>>>>> emulation in nested guests.
> >>>>>
> >>>>> ok. so no kernel_irqchip at all. hmm.
> >>>
> >>> I was confused with what Paul calls 'XICS emulation'. It's not the QEMU
> >>> XICS emulated device but the XICS-over-XIVE KVM device, the KVM XICS
> >>> device KVM uses when under a P9 processor.
> >>
> >> Actually there are two separate implementations of XICS emulation in
> >> KVM. The first (older) one is almost entirely a software emulation
> >> but does have some cases where it accesses an underlying XICS device
> >> in order to make some things faster (IPIs and pass-through of a device
> >> interrupt to a guest). The other, newer one is the XICS-on-XIVE
> >> emulation that Ben wrote, which uses the XIVE hardware pretty heavily.
> >> My patch was about making the the older code work when there is no
> >> XICS available to the host.
> >
> > Ah, right. To clarify my earlier statements in light of this:
> >
> > * We definitely want some sort of kernel-XICS available in a nested
> > guest. AIUI, this is now accomplished, so, Yay!
> >
> > * Implementing the L2 XICS in terms of L1's PAPR-XIVE would be a
> > bonus, but it's a much lower priority.
>
> Yes. In this case, the L1 KVM-HV should not advertise KVM_CAP_PPC_IRQ_XIVE
> to QEMU which will restrict CAS to the XICS only interrupt mode.
Uh... no... we shouldn't change what's available to the guest based on
host configuration only. We should just stop advertising the CAP
saying that *KVM implemented* is available so that qemu will fall back
to userspace XIVE emulation.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 06/19] KVM: PPC: Book3S HV: add a GET_ESB_FD control to the XIVE native device
From: David Gibson @ 2019-02-07 2:49 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <1745dd9f-2927-cae6-e8da-c350b0bd0a66@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 3588 bytes --]
On Wed, Feb 06, 2019 at 08:21:10AM +0100, Cédric Le Goater wrote:
> On 2/6/19 2:23 AM, David Gibson wrote:
> > On Tue, Feb 05, 2019 at 01:55:40PM +0100, Cédric Le Goater wrote:
> >> On 2/5/19 6:28 AM, David Gibson wrote:
> >>> On Mon, Feb 04, 2019 at 12:30:39PM +0100, Cédric Le Goater wrote:
> >>>> On 2/4/19 5:45 AM, David Gibson wrote:
> >>>>> On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
> >>>>>> This will let the guest create a memory mapping to expose the ESB MMIO
> >>>>>> regions used to control the interrupt sources, to trigger events, to
> >>>>>> EOI or to turn off the sources.
> >>>>>>
> >>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>>>> ---
> >>>>>> arch/powerpc/include/uapi/asm/kvm.h | 4 ++
> >>>>>> arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
> >>>>>> 2 files changed, 101 insertions(+)
> >>>>>>
> >>>>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>> index 8c876c166ef2..6bb61ba141c2 100644
> >>>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> >>>>>> @@ -675,4 +675,8 @@ struct kvm_ppc_cpu_char {
> >>>>>> #define KVM_XICS_PRESENTED (1ULL << 43)
> >>>>>> #define KVM_XICS_QUEUED (1ULL << 44)
> >>>>>>
> >>>>>> +/* POWER9 XIVE Native Interrupt Controller */
> >>>>>> +#define KVM_DEV_XIVE_GRP_CTRL 1
> >>>>>> +#define KVM_DEV_XIVE_GET_ESB_FD 1
> >>>>>
> >>>>> Introducing a new FD for ESB and TIMA seems overkill. Can't you get
> >>>>> to both with an mmap() directly on the xive device fd? Using the
> >>>>> offset to distinguish which one to map, obviously.
> >>>>
> >>>> The page offset would define some sort of user API. It seems feasible.
> >>>> But I am not sure this would be practical in the future if we need to
> >>>> tune the length.
> >>>
> >>> Um.. why not? I mean, yes the XIVE supports rather a lot of
> >>> interrupts, but we have 64-bits of offset we can play with - we can
> >>> leave room for billions of ESB slots and still have room for billions
> >>> of VPs.
> >>
> >> So the first 4 pages could be the TIMA pages and then would come
> >> the pages for the interrupt ESBs. I think that we can have different
> >> vm_fault handler for each mapping.
> >
> > Um.. no, I'm saying you don't need to tightly pack them. So you could
> > have the ESB pages at 0, the TIMA at, say offset 2^60.
>
> Well, we know that the TIMA is 4 pages wide and is "directly" related
> with the KVM interrupt device. So being at offset 0 seems a good idea.
> While the ESB segment is of a variable size depending on the number
> of IRQs and it can come after I think.
>
> >> I wonder how this will work out with pass-through. As Paul said in
> >> a previous email, it would be better to let QEMU request a new
> >> mapping to handle the ESB pages of the device being passed through.
> >> I guess this is not a special case, just another offset and length.
> >
> > Right, if we need multiple "chunks" of ESB pages we can given them
> > each their own terabyte or several. No need to be stingy with address
> > space.
>
> You can not put them anywhere. They should map the same interrupt range
> of ESB pages, overlapping with the underlying segment of IPI ESB pages.
I don't really follow what you're saying here.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
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