Re: [PATCH 1/2] powerpc/pseries: define driver for Platform KeyStore
Hi, Nayna.
Some comments below.
On 7/12/22 21:59, Nayna Jain wrote:
PowerVM provides an isolated Platform Keystore(PKS) storage allocation
for each LPAR with individually managed access controls to store
sensitive information securely. It provides a new set of hypervisor
calls for Linux kernel to access PKS storage.
I think it would be nice to have some consistency as to spaces before
acronymous, e.g.: "Keystore(PKS)" vs. "Keystore (PKS)". The same could
be applied to "Keystore" vs. "KeyStore" vs. "Key Storage".
Define PLPKS driver using H_CALL interface to access PKS storage.
Signed-off-by: Nayna Jain
---
arch/powerpc/include/asm/hvcall.h | 9 +
arch/powerpc/include/asm/plpks.h | 90
arch/powerpc/platforms/pseries/Kconfig| 13 +
arch/powerpc/platforms/pseries/Makefile | 2 +
arch/powerpc/platforms/pseries/plpks/Makefile | 7 +
arch/powerpc/platforms/pseries/plpks/plpks.c | 509 ++
6 files changed, 630 insertions(+)
create mode 100644 arch/powerpc/include/asm/plpks.h
create mode 100644 arch/powerpc/platforms/pseries/plpks/Makefile
create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks.c
diff --git a/arch/powerpc/include/asm/hvcall.h
b/arch/powerpc/include/asm/hvcall.h
index d92a20a85395..24b661b0717c 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -97,6 +97,7 @@
#define H_OP_MODE -73
#define H_COP_HW -74
#define H_STATE -75
+#define H_IN_USE -77
#define H_UNSUPPORTED_FLAG_START -256
#define H_UNSUPPORTED_FLAG_END-511
#define H_MULTI_THREADS_ACTIVE-9005
@@ -321,6 +322,14 @@
#define H_SCM_UNBIND_ALL0x3FC
#define H_SCM_HEALTH0x400
#define H_SCM_PERFORMANCE_STATS 0x418
+#define H_PKS_GET_CONFIG 0x41C
+#define H_PKS_SET_PASSWORD 0x420
+#define H_PKS_GEN_PASSWORD 0x424
+#define H_PKS_WRITE_OBJECT 0x42C
+#define H_PKS_GEN_KEY 0x430
+#define H_PKS_READ_OBJECT 0x434
+#define H_PKS_REMOVE_OBJECT0x438
+#define H_PKS_CONFIRM_OBJECT_FLUSHED 0x43C
#define H_RPT_INVALIDATE 0x448
#define H_SCM_FLUSH 0x44C
#define H_GET_ENERGY_SCALE_INFO 0x450
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
new file mode 100644
index ..cf60e53e1f15
--- /dev/null
+++ b/arch/powerpc/include/asm/plpks.h
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain
+ *
+ * Platform keystore for pseries LPAR(PLPKS).
+ */
+
+#ifndef _PSERIES_PLPKS_H
+#define _PSERIES_PLPKS_H
+
+#include
+#include
+
+#define OSSECBOOTAUDIT 0x4000
+#define OSSECBOOTENFORCE 0x2000
+#define WORLDREADABLE 0x0800
+#define SIGNEDUPDATE 0x0100
+
+#define PLPKS_VAR_LINUX0x01
+#define PLPKS_VAR_COMMON 0x04
+
+struct plpks_var {
+ char *component;
+ u8 os;
+ u8 *name;
+ u16 namelen;
+ u32 policy;
+ u16 datalen;
+ u8 *data;
+};
+
+struct plpks_var_name {
+ u16 namelen;
+ u8 *name;
+};
+
+struct plpks_var_name_list {
+ u32 varcount;
+ struct plpks_var_name varlist[];
+};
+
+struct plpks_config {
+ u8 version;
+ u8 flags;
+ u32 rsvd0;
+ u16 maxpwsize;
+ u16 maxobjlabelsize;
+ u16 maxobjsize;
+ u32 totalsize;
+ u32 usedspace;
+ u32 supportedpolicies;
+ u64 rsvd1;
+} __packed;
+
+/**
+ * Successful return from this API implies PKS is available.
+ * This is used to initialize kernel driver and user interfaces.
+ */
+struct plpks_config *plpks_get_config(void);
+
+/**
+ * Writes the specified var and its data to PKS.
+ * Any caller of PKS driver should present a valid component type for
+ * their variable.
+ */
+int plpks_write_var(struct plpks_var var);
+
+/**
+ * Removes the specified var and its data from PKS.
+ */
+int plpks_remove_var(char *component, u8 varos,
+struct plpks_var_name vname);
+
+/**
+ * Returns the data for the specified os variable.
+ */
+int plpks_read_os_var(struct plpks_var *var);
+
+/**
+ * Returns the data for the specified firmware variable.
+ */
+int plpks_read_fw_var(struct plpks_var *var);
+
+/**
+ * Returns the data for the specified bootloader variable.
+ */
+int plpks_read_bootloader_var(struct plpks_var *var);
+
+#endif
diff --git a/arch/powerpc/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index f7fd91d153a4..de6efe5d18c2 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -142,6 +142,19 @@ config IBMEBUS
help
Bus device driver for GX bus based adapters.
+config PSERIES_PLPKS
+ depends on PPC_PSERIES
+ tristate "Support for the Platform Key Storage"
+ help
+ PowerVM provides an isolated Platform Keystore(PKS) storage
+ allocation for each LPA
Re: [PATCH] KVM: PPC: Align pt_regs in kvm_vcpu_arch structure
Hi, Fabiano.
On 5/25/22 09:49, Fabiano Rosas wrote:
The H_ENTER_NESTED hypercall receives as second parameter the address
of a region of memory containing the values for the nested guest
privileged registers. We currently use the pt_regs structure contained
within kvm_vcpu_arch for that end.
Most hypercalls that receive a memory address expect that region to
not cross a 4k page boundary. We would want H_ENTER_NESTED to follow
the same pattern so this patch ensures the pt_regs structure sits
within a page.
Signed-off-by: Fabiano Rosas
Is it necessary to explain in the commit message that even though the second
parameter needs to be 4k-aligned, we're aligning pt_regs to 512 bytes so it can
be placed within a 4k boundary because its size is below 512 bytes?
The natural thinking would be aligning it to 4k bytes, which would punch a huge
hole in kvm_vcpu_arch. I think having the explanation of why 512 vs. 4k is
worthwhile mentioning.
---
arch/powerpc/include/asm/kvm_host.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/kvm_host.h
b/arch/powerpc/include/asm/kvm_host.h
index faf301d0dec0..87eba60f2920 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -519,7 +519,11 @@ struct kvm_vcpu_arch {
struct kvmppc_book3s_shadow_vcpu *shadow_vcpu;
#endif
- struct pt_regs regs;
+ /*
+* This is passed along to the HV via H_ENTER_NESTED. Align to
+* prevent it crossing a real 4K page.
+*/
+ struct pt_regs regs __aligned(512);
struct thread_fp_state fp;
--
Murilo
Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
On 4/6/22 04:08, Joel Stanley wrote:
On Thu, 31 Mar 2022 at 23:46, Segher Boessenkool
wrote:
On Thu, Mar 31, 2022 at 12:19:52PM -0300, Murilo Opsfelder Araújo wrote:
My understanding is that the default cpu type for -mcpu=powerpc64 can
change.
Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
etc.) have different default CPUs. It also can be set at configure time
for most subtargets.
Linux can be built with compilers not targetting *-linux*, so it would
be best to specify a specific CPU always.
I did a little test using my buildroot compiler which has
with-cpu=power10. I used the presence of PCREL relocations as evidence
that it was build for power10.
$ powerpc64le-buildroot-linux-gnu-gcc -mcpu=power10 -c test.c
$ readelf -r test.o |grep -c PCREL
24
It respected the -mcpu=power10 you provided.
Of course.
And that's my concern. We're relying on the compiler default cpu type.
That is not the compiler default. It is the default from who built the
compiler. It can vary wildly and unpredictably.
The actual compiler default will not change so easily at all, basically
only when its subtarget drops support for an older CPU.
If gcc defaults -mcpu=powerpc64le to power10, you're going to have
the same problem again.
That will not happen before power10 is the minimum supported CPU.
Anything else is madness.
Murilo, does Segher's explanation address your concerns?
The comment:
"Different subtargets (Linux, AIX, Darwin, the various BSDs, bare ELF,
etc.) have different default CPUs. It also can be set at configure time
for most subtargets.
Linux can be built with compilers not targetting *-linux*, so it would
be best to specify a specific CPU always."
made me think that it's better to specify -mcpu=power8 instead of
-mcpu=powerpc64le
because of such compilers not targetting *-linux*.
Did I understand Segher's comment correctly? To be honest, I don't know
how much concerned we should be about this scenario.
Just for the sake of consistency, if we decide to go with -mcpu=powerpc64le,
then I think we should also change arch/powerpc/Makefile CFLAGS.
Otherwise, we could follow what we already have in the tree and use
-mcpu=power8 in BOOTCLAGS, too.
Practically speaking, either way works for us. In any case:
Reviewed-by: Murilo Opsfelder Araujo
I believe the patch I sent fixes the problem that you're worried
about. It should be compatible into the future too.
Cheers,
Joel
It happens that the power8 default cpu type
is compatible to your system by coincidence.
No, power8 is (and always was) the minimum supported CPU type for
powerpc64le-linux.
In gcc/config/rs6000/rs6000-cpus.def, they are set to different processors:
254 RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, MASK_PPC_GFXOPT |
MASK_POWERPC64)
255 RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 |
ISA_2_7_MASKS_SERVER
Those can and will change though, over time. But -mcpu=powerpc64 (etc.)
always will mean what the current documentation says it does:
'-mcpu=powerpc', '-mcpu=powerpc64', and '-mcpu=powerpc64le' specify
pure 32-bit PowerPC (either endian), 64-bit big endian PowerPC and
64-bit little endian PowerPC architecture machine types, with an
appropriate, generic processor model assumed for scheduling
purposes.
My suggestion was to explicitly set -mcpu=power8 instead of
-mcpu=powerpc64le.
That is implied anyway, it is the minimum supported for
powerpc64le-linux. Using -mcpu=powerpc64le might schedule better for
newer CPUs, in the future (but the code will always work on all still
supported CPUs).
Segher
--
Murilo
Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
Hi, Joel. On 3/31/22 02:01, Joel Stanley wrote: On Thu, 31 Mar 2022 at 02:05, Murilo Opsfelder Araújo wrote: Hi, Joel. On 3/30/22 08:24, Joel Stanley wrote: Currently the boot wrapper lacks a -mcpu option, so it will be built for the toolchain's default cpu. This is a problem if the toolchain defaults to a cpu with newer instructions. We could wire in TARGET_CPU but instead use the oldest supported option so the wrapper runs anywhere. The GCC documentation stays that -mcpu=powerpc64le will give us a generic 64 bit powerpc machine: -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure 32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit little endian PowerPC architecture machine types, with an appropriate, generic processor model assumed for scheduling purposes. So do that for each of the three machines. This bug was found when building the kernel with a toolchain that defaulted to powre10, resulting in a pcrel enabled wrapper which fails to link: arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base': (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc save/adjust stub) (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust stub) powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value Even with tha bug worked around the resulting kernel would crash on a power9 box: $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel arch/powerpc/boot/zImage.epapr -serial mon:stdio [7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 25694 bytes [7.130374661,3] *** [7.131072886,3] Fatal Exception 0xe40 at 200101e4MSR 9001 [7.131290613,3] CFAR : 2001027c MSR : 9001 [7.131433759,3] SRR0 : 20010050 SRR1 : 9001 [7.13155,3] HSRR0: 200101e4 HSRR1: 9001 [7.131733687,3] DSISR: DAR : [7.131905162,3] LR : 20010280 CTR : [7.132068356,3] CR : 44002004 XER : Link: https://github.com/linuxppc/issues/issues/400 Signed-off-by: Joel Stanley --- Tested: - ppc64le_defconfig - pseries and powernv qemu, for power8, power9, power10 cpus - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1) - RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9) All decompressed and made it into the kernel ok. ppc64_defconfig did not work, as we've got a regression when the wrapper is built for big endian. It hasn't worked for zImage.pseries for a long time (at least v4.14), and broke some time between v5.4 and v5.17 for zImage.epapr. arch/powerpc/boot/Makefile | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile index 9993c6256ad2..1f5cc401bfc0 100644 --- a/arch/powerpc/boot/Makefile +++ b/arch/powerpc/boot/Makefile @@ -38,9 +38,13 @@ BOOTCFLAGS:= -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ $(LINUXINCLUDE) ifdef CONFIG_PPC64_BOOT_WRAPPER -BOOTCFLAGS += -m64 +ifdef CONFIG_CPU_LITTLE_ENDIAN +BOOTCFLAGS += -m64 -mcpu=powerpc64le else -BOOTCFLAGS += -m32 +BOOTCFLAGS += -m64 -mcpu=powerpc64 +endif +else +BOOTCFLAGS += -m32 -mcpu=powerpc endif BOOTCFLAGS += -isystem $(shell $(BOOTCC) -print-file-name=include) I think it was a fortunate coincidence that the default cpu type of your gcc is compatible with your system. If the distro gcc moves its default to a newer cpu type than your system, this bug would happen again. Perhaps I needed to be clear in my commit message: that's the exact bug I'm looking to avoid. I have a buildroot toolchain that was built for -mcpu=power10. I think you're suggesting the -mcpu=powerpc64 option will change it 's behavior depending on the default. From my reading of the man page, I don't think that's true. Looking at GCC source code (gcc/config/rs6000/rs6000.h:287), -mcpu=powerpc64 seems to select "rs64" as the default cpu type. 284 /* Define generic processor types based upon current deployment. */ 285 #define PROCESSOR_COMMONPROCESSOR_PPC601 286 #define PROCESSOR_POWERPC PROCESSOR_PPC604 287 #define PROCESSOR_POWERPC64 PROCESSOR_RS64A Then in gcc/config/rs6000/linux64.h:77 it sets the 64-bit default with power8. 74 #undef PROCESSOR_DEFAULT 75 #define PROCESSOR_DEFAULT PROCESSOR_POWER7 76 #undef PROCESSOR_DEFAULT64 77 #define PROCESSOR_DEFAULT64 PROCESSOR_POWER8 My understanding is that the default cpu type for -mcpu=powerpc64 can change. If that change is unlikely to happen, that's a separate discussion. I did a little test using my buildroot compiler which has with-cpu=power1
Re: [PATCH] powerpc/boot: Build wrapper for an appropriate CPU
Hi, Joel. On 3/30/22 08:24, Joel Stanley wrote: Currently the boot wrapper lacks a -mcpu option, so it will be built for the toolchain's default cpu. This is a problem if the toolchain defaults to a cpu with newer instructions. We could wire in TARGET_CPU but instead use the oldest supported option so the wrapper runs anywhere. The GCC documentation stays that -mcpu=powerpc64le will give us a generic 64 bit powerpc machine: -mcpu=powerpc, -mcpu=powerpc64, and -mcpu=powerpc64le specify pure 32-bit PowerPC (either endian), 64-bit big endian PowerPC and 64-bit little endian PowerPC architecture machine types, with an appropriate, generic processor model assumed for scheduling purposes. So do that for each of the three machines. This bug was found when building the kernel with a toolchain that defaulted to powre10, resulting in a pcrel enabled wrapper which fails to link: arch/powerpc/boot/wrapper.a(crt0.o): in function `p_base': (.text+0x150): call to `platform_init' lacks nop, can't restore toc; (toc save/adjust stub) (.text+0x154): call to `start' lacks nop, can't restore toc; (toc save/adjust stub) powerpc64le-buildroot-linux-gnu-ld: final link failed: bad value Even with tha bug worked around the resulting kernel would crash on a power9 box: $ qemu-system-ppc64 -nographic -nodefaults -M powernv9 -kernel arch/powerpc/boot/zImage.epapr -serial mon:stdio [7.069331356,5] INIT: Starting kernel at 0x20010020, fdt at 0x3068c628 25694 bytes [7.130374661,3] *** [7.131072886,3] Fatal Exception 0xe40 at 200101e4MSR 9001 [7.131290613,3] CFAR : 2001027c MSR : 9001 [7.131433759,3] SRR0 : 20010050 SRR1 : 9001 [7.13155,3] HSRR0: 200101e4 HSRR1: 9001 [7.131733687,3] DSISR: DAR : [7.131905162,3] LR : 20010280 CTR : [7.132068356,3] CR : 44002004 XER : Link: https://github.com/linuxppc/issues/issues/400 Signed-off-by: Joel Stanley --- Tested: - ppc64le_defconfig - pseries and powernv qemu, for power8, power9, power10 cpus - buildroot compiler that defaults to -mcpu=power10 (gcc 10.3.0, ld 2.36.1) - RHEL9 cross compilers (gcc 11.2.1-1, ld 2.35.2-17.el9) All decompressed and made it into the kernel ok. ppc64_defconfig did not work, as we've got a regression when the wrapper is built for big endian. It hasn't worked for zImage.pseries for a long time (at least v4.14), and broke some time between v5.4 and v5.17 for zImage.epapr. arch/powerpc/boot/Makefile | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile index 9993c6256ad2..1f5cc401bfc0 100644 --- a/arch/powerpc/boot/Makefile +++ b/arch/powerpc/boot/Makefile @@ -38,9 +38,13 @@ BOOTCFLAGS:= -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ $(LINUXINCLUDE) ifdef CONFIG_PPC64_BOOT_WRAPPER -BOOTCFLAGS += -m64 +ifdef CONFIG_CPU_LITTLE_ENDIAN +BOOTCFLAGS += -m64 -mcpu=powerpc64le else -BOOTCFLAGS += -m32 +BOOTCFLAGS += -m64 -mcpu=powerpc64 +endif +else +BOOTCFLAGS += -m32 -mcpu=powerpc endif BOOTCFLAGS += -isystem $(shell $(BOOTCC) -print-file-name=include) I think it was a fortunate coincidence that the default cpu type of your gcc is compatible with your system. If the distro gcc moves its default to a newer cpu type than your system, this bug would happen again. The command "gcc -v |& grep with-cpu" will show you the default cpu type for 32 and 64-bit that gcc was configured. Considering the CONFIG_TARGET_CPU for BOOTCFLAGS would bring some level of consistency between CFLAGS and BOOTCFLAGS regarding -mcpu value. We could mimic the behaviour from arch/powerpc/Makefile: 166 ifdef config_ppc_book3s_64 167 ifdef config_cpu_little_endian 168 cflags-$(config_generic_cpu) += -mcpu=power8 169 cflags-$(config_generic_cpu) += $(call cc-option,-mtune=power9,-mtune=power8) 170 else 171 cflags-$(config_generic_cpu) += $(call cc-option,-mtune=power7,$(call cc-option,-mtune=power5)) 172 cflags-$(config_generic_cpu) += $(call cc-option,-mcpu=power5,-mcpu=power4) 173 endif 174 else ifdef config_ppc_book3e_64 175 cflags-$(config_generic_cpu) += -mcpu=powerpc64 176 endif ... 185 CFLAGS-$(CONFIG_TARGET_CPU_BOOL) += $(call cc-option,-mcpu=$(CONFIG_TARGET_CPU)) Cheers! -- Murilo
Re: [PATCH 1/2] powerpc: Reject probes on instructions that can't be single stepped
Hi, Naveen.
Some comments below.
On 3/23/22 08:51, Naveen N. Rao wrote:
Per the ISA, a Trace interrupt is not generated for:
- [h|u]rfi[d]
- rfscv
- sc, scv, and Trap instructions that trap
- Power-Saving Mode instructions
- other instructions that cause interrupts (other than Trace interrupts)
- the first instructions of any interrupt handler (applies to Branch and Single
Step tracing;
CIABR matches may still occur)
- instructions that are emulated by software
Add a helper to check for instructions belonging to the first four
categories above and to reject kprobes, uprobes and xmon breakpoints on
such instructions. We reject probing on instructions belonging to these
categories across all ISA versions and across both BookS and BookE.
For trap instructions, we can't know in advance if they can cause a
trap, and there is no good reason to allow probing on those. Also,
uprobes already refuses to probe trap instructions and kprobes does not
allow probes on trap instructions used for kernel warnings and bugs. As
such, stop allowing any type of probes/breakpoints on trap instruction
across uprobes, kprobes and xmon.
For some of the fp/altivec instructions that can generate an interrupt
and which we emulate in the kernel (altivec assist, for example), we
check and turn off single stepping in emulate_single_step().
Instructions generating a DSI are restarted and single stepping normally
completes once the instruction is completed.
In uprobes, if a single stepped instruction results in a non-fatal
signal to be delivered to the task, such signals are "delayed" until
after the instruction completes. For fatal signals, single stepping is
cancelled and the instruction restarted in-place so that core dump
captures proper addresses.
In kprobes, we do not allow probes on instructions having an extable
entry and we also do not allow probing interrupt vectors.
Signed-off-by: Naveen N. Rao
---
arch/powerpc/include/asm/probes.h | 55 +++
arch/powerpc/kernel/kprobes.c | 4 +--
arch/powerpc/kernel/uprobes.c | 5 +++
arch/powerpc/xmon/xmon.c | 11 +++
4 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/probes.h
b/arch/powerpc/include/asm/probes.h
index c5d984700d241a..21ab5b6256b590 100644
--- a/arch/powerpc/include/asm/probes.h
+++ b/arch/powerpc/include/asm/probes.h
@@ -31,6 +31,61 @@ typedef u32 ppc_opcode_t;
#define MSR_SINGLESTEP(MSR_SE)
#endif
+static inline bool can_single_step(u32 inst)
+{
+ switch (inst >> 26) {
Can't ppc_inst_primary_opcode() be used instead?
+ case 2: /* tdi */
+ return false;
+ case 3: /* twi */
+ return false;
+ case 17:/* sc and scv */
+ return false;
+ case 19:
+ switch ((inst >> 1) & 0x3ff) {
+ case 18:/* rfid */
+ return false;
+ case 38:/* rfmci */
+ return false;
+ case 39:/* rfdi */
+ return false;
+ case 50:/* rfi */
+ return false;
+ case 51:/* rfci */
+ return false;
+ case 82:/* rfscv */
+ return false;
+ case 274: /* hrfid */
+ return false;
+ case 306: /* urfid */
+ return false;
+ case 370: /* stop */
+ return false;
+ case 402: /* doze */
+ return false;
+ case 434: /* nap */
+ return false;
+ case 466: /* sleep */
+ return false;
+ case 498: /* rvwinkle */
+ return false;
+ }
+ break;
+ case 31:
+ switch ((inst >> 1) & 0x3ff) {
+ case 4: /* tw */
+ return false;
+ case 68:/* td */
+ return false;
+ case 146: /* mtmsr */
+ return false;
+ case 178: /* mtmsrd */
+ return false;
+ }
+ break;
+ }
+ return true;
+}
+
Can't OP_* definitions from ppc-opcode.h be used for all of these switch-case
statements?
/* Enable single stepping for the current task */
static inline void enable_single_step(struct pt_regs *regs)
{
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 9a492fdec1dfbe..0936a6c8c256b9 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -129,8 +129,8 @@ int arch_prepare_kprobe(struct kprobe *p)
if ((unsigned long)p->addr & 0x03) {
printk("Attempt to
Re: [PATCH] powerpc/powernv: Get more flushing requirements from device-tree
Hi, Russell.
I think this patch could have been split in half with their corresponding
Fixes: tag.
This may sound nitpicking but doing this would certainly help distros doing
their backports.
More comments below.
On 3/22/22 04:47, Russell Currey wrote:
The device-tree properties no-need-l1d-flush-msr-pr-1-to-0,
no-need-l1d-flush-kernel-on-user-access and
no-need-store-drain-on-priv-state-switch are the equivalents of
H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY, H_CPU_BEHAV_NO_L1D_FLUSH_UACCESS
and H_CPU_BEHAV_NO_STF_BARRIER from the H_GET_CPU_CHARACTERISTICS
hcall on pseries, respectively.
Since commit 84ed26fd00c5 ("powerpc/security: Add a security feature for
STF barrier") powernv systems with this device-tree property have been
enabling the STF barrier when they have no need for it. This patch
fixes this by clearing the STF barrier feature on those systems.
In commit d02fa40d759f ("powerpc/powernv: Remove POWER9 PVR version
check for entry and uaccess flushes") the condition for disabling the
L1D flush on kernel entry and user access was changed from any non-P9
CPU to only checking P7 and P8. Without the appropriate device-tree
checks for newer processors on powernv, these flushes are unnecessarily
enabled on those systems. This patch fixes that too.
Reported-by: Joel Stanley
Signed-off-by: Russell Currey
---
arch/powerpc/platforms/powernv/setup.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/setup.c
b/arch/powerpc/platforms/powernv/setup.c
index 105d889abd51..824c3ad7a0fa 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -96,6 +96,15 @@ static void __init init_fw_feat_flags(struct device_node *np)
if (fw_feature_is("disabled", "needs-spec-barrier-for-bound-checks", np))
security_ftr_clear(SEC_FTR_BNDS_CHK_SPEC_BAR);
+
+ if (fw_feature_is("enabled", "no-need-l1d-flush-msr-pr-1-to-0", np))
+ security_ftr_clear(SEC_FTR_L1D_FLUSH_ENTRY);
+
+ if (fw_feature_is("enabled", "no-need-l1d-flush-kernel-on-user-access",
np))
+ security_ftr_clear(SEC_FTR_L1D_FLUSH_UACCESS);
+
This first diff in one patch with:
Fixes: d02fa40d759f (powerpc/powernv: Remove POWER9 PVR version check for entry
and uaccess flushes)
+ if (fw_feature_is("enabled",
"no-need-store-drain-on-priv-state-switch", np))
+ security_ftr_clear(SEC_FTR_STF_BARRIER);
And this second diff in another one with:
Fixes: 84ed26fd00c5 (powerpc/security: Add a security feature for STF barrier)
And commit messages could be updated for both commits accordingly.
}
static void __init pnv_setup_security_mitigations(void)
Cheers!
--
Murilo
Re: [PATCH v2] powerpc/mm: Fix set_memory_*() against concurrent accesses
On 8/18/21 9:05 AM, Michael Ellerman wrote:
Laurent reported that STRICT_MODULE_RWX was causing intermittent crashes
on one of his systems:
kernel tried to execute exec-protected page (c00804073278) - exploit
attempt? (uid: 0)
BUG: Unable to handle kernel instruction fetch
Faulting instruction address: 0xc00804073278
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: drm virtio_console fuse drm_panel_orientation_quirks ...
CPU: 3 PID: 44 Comm: kworker/3:1 Not tainted 5.14.0-rc4+ #12
Workqueue: events control_work_handler [virtio_console]
NIP: c00804073278 LR: c00804073278 CTR: c01e9de0
REGS: c0002e4ef7e0 TRAP: 0400 Not tainted (5.14.0-rc4+)
MSR: 80004280b033 CR: 24002822 XER:
200400cf
...
NIP fill_queue+0xf0/0x210 [virtio_console]
LR fill_queue+0xf0/0x210 [virtio_console]
Call Trace:
fill_queue+0xb4/0x210 [virtio_console] (unreliable)
add_port+0x1a8/0x470 [virtio_console]
control_work_handler+0xbc/0x1e8 [virtio_console]
process_one_work+0x290/0x590
worker_thread+0x88/0x620
kthread+0x194/0x1a0
ret_from_kernel_thread+0x5c/0x64
Jordan, Fabiano & Murilo were able to reproduce and identify that the
problem is caused by the call to module_enable_ro() in do_init_module(),
which happens after the module's init function has already been called.
Our current implementation of change_page_attr() is not safe against
concurrent accesses, because it invalidates the PTE before flushing the
TLB and then installing the new PTE. That leaves a window in time where
there is no valid PTE for the page, if another CPU tries to access the
page at that time we see something like the fault above.
We can't simply switch to set_pte_at()/flush TLB, because our hash MMU
code doesn't handle a set_pte_at() of a valid PTE. See [1].
But we do have pte_update(), which replaces the old PTE with the new,
meaning there's no window where the PTE is invalid. And the hash MMU
version hash__pte_update() deals with synchronising the hash page table
correctly.
[1]: https://lore.kernel.org/linuxppc-dev/87y318wp9r@linux.ibm.com/
Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines")
Reported-by: Laurent Vivier
Signed-off-by: Fabiano Rosas
Signed-off-by: Michael Ellerman
Reviewed-by: Murilo Opsfelder Araújo
---
arch/powerpc/mm/pageattr.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
v2: Use pte_update(..., ~0, pte_val(pte), ...) as suggested by Fabiano,
and ptep_get() as suggested by Christophe.
diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index 0876216ceee6..edea388e9d3f 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -18,16 +18,12 @@
/*
* Updates the attributes of a page in three steps:
*
- * 1. invalidate the page table entry
- * 2. flush the TLB
- * 3. install the new entry with the updated attributes
- *
- * Invalidating the pte means there are situations where this will not work
- * when in theory it should.
- * For example:
- * - removing write from page whilst it is being executed
- * - setting a page read-only whilst it is being read by another CPU
+ * 1. take the page_table_lock
+ * 2. install the new entry with the updated attributes
+ * 3. flush the TLB
*
+ * This sequence is safe against concurrent updates, and also allows updating
the
+ * attributes of a page currently being executed or accessed.
*/
static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
{
@@ -36,9 +32,7 @@ static int change_page_attr(pte_t *ptep, unsigned long addr,
void *data)
spin_lock(&init_mm.page_table_lock);
- /* invalidate the PTE so it's safe to modify */
- pte = ptep_get_and_clear(&init_mm, addr, ptep);
- flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+ pte = ptep_get(ptep);
/* modify the PTE bits as desired, then apply */
switch (action) {
@@ -59,11 +53,14 @@ static int change_page_attr(pte_t *ptep, unsigned long
addr, void *data)
break;
}
- set_pte_at(&init_mm, addr, ptep, pte);
+ pte_update(&init_mm, addr, ptep, ~0UL, pte_val(pte), 0);
/* See ptesync comment in radix__set_pte_at() */
if (radix_enabled())
asm volatile("ptesync": : :"memory");
+
+ flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+
spin_unlock(&init_mm.page_table_lock);
return 0;
base-commit: cbc06f051c524dcfe52ef0d1f30647828e226d30
--
Murilo
Re: [PATCH] powerpc/mm/book3s64: Use the correct storage key value when calling H_PROTECT
On Friday, March 26, 2021 4:07:55 AM -03 Aneesh Kumar K.V wrote:
> H_PROTECT expect the flag value to include
> flags: AVPN, pp0, pp1, pp2, key0-key4, Noexec, CMO Option flags
>
> This patch updates hpte_updatepp() to fetch the storage key value from the
> linux page table and use the same in H_PROTECT hcall.
>
> native_hpte_updatepp() is not updated because the kernel doesn't clear the
> existing storage key value there. The kernel also doesn't use
> hpte_updatepp() callback for updating storage keys.
>
> This fixes the below kernel crash observed with KUAP enabled.
>
> BUG: Unable to handle kernel data access on write at 0xc009fc44
> Faulting instruction address: 0xc00b7030
> Key fault AMR: 0xfcff IAMR: 0xc077bc498100
> Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ff1194
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> .
> CFAR: c0010100 DAR: c009fc44 DSISR: 0220 IRQMASK: 0
> ..
> NIP [c00b7030] memset+0x68/0x104
> LR [c03ef00c] pcpu_alloc+0x54c/0xb50
> Call Trace:
> [c0001c7534f0] [c03ef01c] pcpu_alloc+0x55c/0xb50 (unreliable)
> [c0001c753600] [c08bb214] blk_stat_alloc_callback+0x94/0x150
> [c0001c753650] [c08b7a04]
> blk_mq_init_allocated_queue+0x64/0x560 [c0001c7536b0]
> [c08b8024] blk_mq_init_queue+0x54/0xb0 [c0001c7536e0]
> [c0b87650] scsi_mq_alloc_queue+0x30/0xa0 [c0001c753710]
> [c0b88b2c] scsi_alloc_sdev+0x1cc/0x300 [c0001c7537b0]
> [c0b897b0] scsi_probe_and_add_lun+0xb50/0x1020 [c0001c753950]
> [c0b8a35c] __scsi_scan_target+0x17c/0x790 [c0001c753a80]
> [c0b8ab90] scsi_scan_channel+0x90/0xe0 [c0001c753ad0]
> [c0b8ae48] scsi_scan_host_selected+0x148/0x1f0 [c0001c753b60]
> [c0b8b31c] do_scan_async+0x2c/0x2a0
> [c0001c753be0] [c0187a18] async_run_entry_fn+0x78/0x220
> [c0001c753c70] [c0176a74] process_one_work+0x264/0x540
> [c0001c753d10] [c0177338] worker_thread+0xa8/0x600
> [c0001c753da0] [c01807b0] kthread+0x190/0x1a0
> [c0001c753e10] [c000d8f0] ret_from_kernel_thread+0x5c/0x6c
>
> With KUAP enabled the kernel uses storage key 3 for all its translations.
> But as shown by the debug print, in this specific case we have the hash
> page table entry created with key value 0.
>
> [2.249497] Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ff1194
>
> and DSISR indicates a key fault.
>
> This can happen due to parallel fault on the same EA by different CPUs
>
> CPU 0 CPU 1
> fault on X
>
> H_PAGE_BUSY set
> fault on X
>
> finish fault handling and
> clear H_PAGE_BUSY
> check for H_PAGE_BUSY
> continue with fault
handling.
>
> This implies CPU1 will end up calling hpte_updatepp for address X
> and the kernel updated the hash pte entry with key 0
>
> Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping
> with hash translation")
>
> Debugged-by: Michael Ellerman
> Reported-by: Murilo Opsfelder Araujo
> Signed-off-by: Aneesh Kumar K.V
> ---
I've tested this on top of commit db24726bfefa68c606947a86132591568a06bfb4
("Merge tag 'integrity-v5.12-fix' of git://git.kernel.org/pub/scm/linux/
kernel/git/zohar/linux-integrity"),
and the reported issue did not manifest.
Thank you, Michael and Aneesh, for the help.
Tested-by: Murilo Opsfelder Araujo
--
Murilo
Re: [PATCH v3 0/7] Base support for POWER10
On Tue, Jun 09, 2020 at 03:28:31PM +1000, Michael Ellerman wrote: > On Thu, 21 May 2020 11:43:34 +1000, Alistair Popple wrote: > > This series brings together several previously posted patches required for > > POWER10 support and introduces a new patch enabling POWER10 architected > > mode to enable booting as a POWER10 pseries guest. > > > > It includes support for enabling facilities related to MMA and prefix > > instructions. > > > > [...] > > Patches 1-3 and 5-7 applied to powerpc/next. > > [1/7] powerpc: Add new HWCAP bits > > https://git.kernel.org/powerpc/c/ee988c11acf6f9464b7b44e9a091bf6afb3b3a49 > [2/7] powerpc: Add support for ISA v3.1 > > https://git.kernel.org/powerpc/c/3fd5836ee801ab9ac5b314c26550e209bafa5eaa > [3/7] powerpc/dt_cpu_ftrs: Advertise support for ISA v3.1 if selected > > https://git.kernel.org/powerpc/c/43d0d37acbe40a9a93d9891ca670638cd22116b1 Just out of curiosity, why do we define ISA_V3_0B and ISA_V3_1 macros and don't use them anywhere else in the code? Can't they be used in cpufeatures_setup_start() instead of 3000 and 3100 literals? > [5/7] powerpc/dt_cpu_ftrs: Enable Prefixed Instructions > > https://git.kernel.org/powerpc/c/c63d688c3dabca973c5a7da73d17422ad13f3737 > [6/7] powerpc/dt_cpu_ftrs: Add MMA feature > > https://git.kernel.org/powerpc/c/87939d50e5888bd78478d9aa9455f56b919df658 > [7/7] powerpc: Add POWER10 architected mode > > https://git.kernel.org/powerpc/c/a3ea40d5c7365e7e5c7c85b6f30b15142b397571 > > cheers -- Murilo
Re: WARNING in ext4_da_update_reserve_space
On Thursday, April 2, 2020 8:02:11 AM -03 syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:1a147b74 Merge branch 'DSA-mtu'
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=14237713e0
> kernel config: https://syzkaller.appspot.com/x/.config?x=46ee14d4915944bc
> dashboard link: https://syzkaller.appspot.com/bug?extid=67e4f16db666b1c8253c
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12237713e0
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10ec7c97e0
>
> The bug was bisected to:
>
> commit 658b0f92bc7003bc734471f61bf7cd56339eb8c3
> Author: Murilo Opsfelder Araujo
> Date: Wed Aug 1 21:33:15 2018 +
>
> powerpc/traps: Print unhandled signals in a separate function
This commit is specific to powerpc and the crash is from an x86_64 system.
There is a bunch of scp errors in the logs:
scp: ./syz-executor998635077: No space left on device
Is it possible that these errors might be misleading the syzbot?
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=15979f5be0
> final crash:https://syzkaller.appspot.com/x/report.txt?x=17979f5be0
> console output: https://syzkaller.appspot.com/x/log.txt?x=13979f5be0
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+67e4f16db666b1c82...@syzkaller.appspotmail.com
> Fixes: 658b0f92bc70 ("powerpc/traps: Print unhandled signals in a separate
> function")
>
> EXT4-fs warning (device sda1): ext4_da_update_reserve_space:344:
> ext4_da_update_reserve_space: ino 15722, used 1 with only 0 reserved data
> blocks [ cut here ]
> WARNING: CPU: 1 PID: 359 at fs/ext4/inode.c:348
> ext4_da_update_reserve_space+0x622/0x7d0 fs/ext4/inode.c:344 Kernel panic -
> not syncing: panic_on_warn set ...
> CPU: 1 PID: 359 Comm: kworker/u4:5 Not tainted 5.6.0-rc7-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011 Workqueue: writeback wb_workfn (flush-8:0)
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x188/0x20d lib/dump_stack.c:118
> panic+0x2e3/0x75c kernel/panic.c:221
> __warn.cold+0x2f/0x35 kernel/panic.c:582
> report_bug+0x27b/0x2f0 lib/bug.c:195
> fixup_bug arch/x86/kernel/traps.c:174 [inline]
> fixup_bug arch/x86/kernel/traps.c:169 [inline]
> do_error_trap+0x12b/0x220 arch/x86/kernel/traps.c:267
> do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
> invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
> RIP: 0010:ext4_da_update_reserve_space+0x622/0x7d0 fs/ext4/inode.c:348
> Code: 02 00 0f 85 94 01 00 00 48 8b 7d 28 49 c7 c0 20 72 3c 88 41 56 48 c7
> c1 80 60 3c 88 53 ba 58 01 00 00 4c 89 c6 e8 1e 6d 0d 00 <0f> 0b 48 b8 00
> 00 00 00 00 fc ff df 4c 89 ea 48 c1 ea 03 0f b6 04 RSP:
> 0018:c90002197288 EFLAGS: 00010296
> RAX: RBX: 0001 RCX:
> RDX: RSI: 820bf066 RDI: f52000432e21
> RBP: 888086b744c8 R08: 0091 R09: ed1015ce6659
> R10: ed1015ce6658 R11: 8880ae7332c7 R12: 0001
> R13: 888086b74990 R14: R15: 888086b74a40
> ext4_ext_map_blocks+0x24aa/0x37d0 fs/ext4/extents.c:4500
> ext4_map_blocks+0x4cb/0x1650 fs/ext4/inode.c:622
> mpage_map_one_extent fs/ext4/inode.c:2365 [inline]
> mpage_map_and_submit_extent fs/ext4/inode.c:2418 [inline]
> ext4_writepages+0x19eb/0x3080 fs/ext4/inode.c:2772
> do_writepages+0xfa/0x2a0 mm/page-writeback.c:2344
> __writeback_single_inode+0x12a/0x1410 fs/fs-writeback.c:1452
> writeback_sb_inodes+0x515/0xdd0 fs/fs-writeback.c:1716
> wb_writeback+0x2a5/0xd90 fs/fs-writeback.c:1892
> wb_do_writeback fs/fs-writeback.c:2037 [inline]
> wb_workfn+0x339/0x11c0 fs/fs-writeback.c:2078
> process_one_work+0x94b/0x1690 kernel/workqueue.c:2266
> worker_thread+0x96/0xe20 kernel/workqueue.c:2412
> kthread+0x357/0x430 kernel/kthread.c:255
> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkal...@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
--
Murilo
Re: [PATCH v3 6/9] KVM: PPC: Ultravisor: Restrict flush of the partition tlb cache
Claudio Carvalho writes:
> From: Ram Pai
>
> Ultravisor is responsible for flushing the tlb cache, since it manages
> the PATE entries. Hence skip tlb flush, if the ultravisor firmware is
> available.
>
> Signed-off-by: Ram Pai
> Signed-off-by: Claudio Carvalho
> ---
> arch/powerpc/mm/book3s64/pgtable.c | 33 +-
> 1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c
> b/arch/powerpc/mm/book3s64/pgtable.c
> index 40a9fc8b139f..1eeb5fe87023 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -224,6 +224,23 @@ void __init mmu_partition_table_init(void)
> powernv_set_nmmu_ptcr(ptcr);
> }
>
> +static void flush_partition(unsigned int lpid, unsigned long dw0)
> +{
> + if (dw0 & PATB_HR) {
> + asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 1) : :
> + "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> + asm volatile(PPC_TLBIE_5(%0, %1, 2, 1, 1) : :
> + "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> + trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 1);
> + } else {
> + asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 0) : :
> + "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> + trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 0);
> + }
> + /* do we need fixup here ?*/
> + asm volatile("eieio; tlbsync; ptesync" : : : "memory");
> +}
> +
checkpatch.pl seems to complain:
ERROR: need consistent spacing around '%' (ctx:WxV)
#125: FILE: arch/powerpc/mm/book3s64/pgtable.c:230:
+ asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 1) : :
^
ERROR: need consistent spacing around '%' (ctx:WxV)
#127: FILE: arch/powerpc/mm/book3s64/pgtable.c:232:
+ asm volatile(PPC_TLBIE_5(%0, %1, 2, 1, 1) : :
^
ERROR: need consistent spacing around '%' (ctx:WxV)
#131: FILE: arch/powerpc/mm/book3s64/pgtable.c:236:
+ asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 0) : :
^
> static void __mmu_partition_table_set_entry(unsigned int lpid,
> unsigned long dw0,
> unsigned long dw1)
> @@ -238,20 +255,8 @@ static void __mmu_partition_table_set_entry(unsigned int
> lpid,
>* The type of flush (hash or radix) depends on what the previous
>* use of this partition ID was, not the new use.
>*/
> - asm volatile("ptesync" : : : "memory");
> - if (old & PATB_HR) {
> - asm volatile(PPC_TLBIE_5(%0,%1,2,0,1) : :
> - "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> - asm volatile(PPC_TLBIE_5(%0,%1,2,1,1) : :
> - "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> - trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 1);
> - } else {
> - asm volatile(PPC_TLBIE_5(%0,%1,2,0,0) : :
> - "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> - trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 0);
> - }
> - /* do we need fixup here ?*/
> - asm volatile("eieio; tlbsync; ptesync" : : : "memory");
> + if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
> + flush_partition(lpid, old);
> }
>
> void mmu_partition_table_set_entry(unsigned int lpid, unsigned long dw0,
> --
> 2.20.1
Re: [PATCH v2] include/linux/vfio.h: Guard powerpc-specific functions with CONFIG_VFIO_SPAPR_EEH
On 07/26/2017 04:37 PM, Alex Williamson wrote: [...] > Applied to my for-linus branch with David and Alexey's R-b for v4.13. > Thanks, > > Alex Thank you all! -- Murilo
Re: [PATCH v2] include/linux/vfio.h: Guard powerpc-specific functions with CONFIG_VFIO_SPAPR_EEH
On 07/18/2017 02:22 PM, Murilo Opsfelder Araujo wrote: > When CONFIG_EEH=y and CONFIG_VFIO_SPAPR_EEH=n, build fails with the > following: > > drivers/vfio/pci/vfio_pci.o: In function `.vfio_pci_release': > vfio_pci.c:(.text+0xa98): undefined reference to > `.vfio_spapr_pci_eeh_release' > drivers/vfio/pci/vfio_pci.o: In function `.vfio_pci_open': > vfio_pci.c:(.text+0x1420): undefined reference to > `.vfio_spapr_pci_eeh_open' > > In this case, vfio_pci.c should use the empty definitions of > vfio_spapr_pci_eeh_open and vfio_spapr_pci_eeh_release functions. > > This patch fixes it by guarding these function definitions with > CONFIG_VFIO_SPAPR_EEH, the symbol that controls whether vfio_spapr_eeh.c is > built, which is where the non-empty versions of these functions are. We need > to > make use of IS_ENABLED() macro because CONFIG_VFIO_SPAPR_EEH is a tristate > option. > > This issue was found during a randconfig build. Logs are here: > > http://kisskb.ellerman.id.au/kisskb/buildresult/12982362/ > > Signed-off-by: Murilo Opsfelder Araujo > --- > > Changes from v1: > - Rebased on top of next-20170718. Hi, Alex. Are you applying this? Thanks! -- Murilo
Re: [PATCH] include/linux/vfio.h: Guard powerpc-specific functions with CONFIG_VFIO_SPAPR_EEH
On 06/08/2017 10:10 AM, Alexey Kardashevskiy wrote: [...] > The config you attached in the first mail has CONFIG_VFIO_SPAPR_EEH=m, here > is my confusion. The config from the link below does not have KVM_BOOK3S_64 > which selects SPAPR_TCE_IOMMU and which in turn selects VFIO_IOMMU_SPAPR_TCE. > > So > https://github.com/0day-ci/linux/commit/36ed1ddb05e132aa3cfbb610f0f8402a0774da12 > looks correct. It wasn't me that attached the .config.gz, it was this 0dayci robot. When CONFIG_VFIO_SPAPR_EEH=m, there is no definition of it in autoconf.h, only CONFIG_VFIO_SPAPR_EEH_MODULE is defined: $ grep 'VFIO_SPAPR_EEH' ./include/generated/autoconf.h #define CONFIG_VFIO_SPAPR_EEH_MODULE 1 In this case, `#ifdef CONFIG_VFIO_SPAPR_EEH` will be false. That's why my v1 patch failed with the 0dayci .config and robot reported back. This was addressed in my v2 patch using the IS_ENABLED() macro, which checks for both CONFIG_ and CONFIG__MODULE definitions. -- Murilo
Re: [PATCH] include/linux/vfio.h: Guard powerpc-specific functions with CONFIG_VFIO_SPAPR_EEH
On 06/08/2017 08:41 AM, Michael Ellerman wrote: > Alexey Kardashevskiy writes: > >> Hi, >> >> How did you manage to have CONFIG_EEH=y and CONFIG_VFIO_SPAPR_EEH=n? "make >> oldconfig" fixes this to CONFIG_VFIO_SPAPR_EEH=y. > > Hmm, Murilo did you confirm the bug still happens on upstream with that > rand config? Yes, it's still happening with next-20170607. For me, `make oldconfig` hasn't changed it to CONFIG_VFIO_SPAPR_EEH=y. See: $ git clean -dfxq $ git reset --hard origin/master HEAD is now at 8d1b80c Add linux-next specific files for 20170607 $ curl http://kisskb.ellerman.id.au/kisskb/buildresult/12982362/config/ -o .config $ grep -E 'EEH|SPAPR' .config CONFIG_EEH=y # CONFIG_SPAPR_TCE_IOMMU is not set $ yes '' | make oldconfig $ grep -E 'EEH|SPAPR' .config CONFIG_EEH=y # CONFIG_SPAPR_TCE_IOMMU is not set $ make -j 160 ARCH=powerpc ... drivers/vfio/pci/vfio_pci.o: In function `.vfio_pci_release': vfio_pci.c:(.text+0xa98): undefined reference to `.vfio_spapr_pci_eeh_release' drivers/vfio/pci/vfio_pci.o: In function `.vfio_pci_open': vfio_pci.c:(.text+0x1420): undefined reference to `.vfio_spapr_pci_eeh_open' make: *** [vmlinux] Error 1 -- Murilo
