* Re: [for-stable-4.19 PATCH 1/2] vmlinux.lds.h: Create section for protection against instrumentation
From: Alexandre Chartre @ 2021-03-19 11:20 UTC (permalink / raw)
To: Greg Kroah-Hartman, Nicolas Boichat
Cc: Sasha Levin, linux-arch, Michal Marek, Arnd Bergmann,
linux-kbuild, Peter Zijlstra, Christopher Li, linux-kernel,
stable, Masahiro Yamada, linux-sparse, Paul Mackerras,
Nicholas Piggin, Thomas Gleixner, linuxppc-dev, Naveen N. Rao,
Daniel Axtens
In-Reply-To: <YFR/fQIePjDQcO5W@kroah.com>
On 3/19/21 11:39 AM, Greg Kroah-Hartman wrote:
> On Fri, Mar 19, 2021 at 07:54:15AM +0800, Nicolas Boichat wrote:
>> From: Thomas Gleixner <tglx@linutronix.de>
>>
>> commit 6553896666433e7efec589838b400a2a652b3ffa upstream.
>>
>> Some code pathes, especially the low level entry code, must be protected
>> against instrumentation for various reasons:
>>
>> - Low level entry code can be a fragile beast, especially on x86.
>>
>> - With NO_HZ_FULL RCU state needs to be established before using it.
>>
>> Having a dedicated section for such code allows to validate with tooling
>> that no unsafe functions are invoked.
>>
>> Add the .noinstr.text section and the noinstr attribute to mark
>> functions. noinstr implies notrace. Kprobes will gain a section check
>> later.
>>
>> Provide also a set of markers: instrumentation_begin()/end()
>>
>> These are used to mark code inside a noinstr function which calls
>> into regular instrumentable text section as safe.
>>
>> The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is
>> enabled as the end marker emits a NOP to prevent the compiler from merging
>> the annotation points. This means the objtool verification requires a
>> kernel compiled with this option.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>
>> Acked-by: Peter Zijlstra <peterz@infradead.org>
>> Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de
>>
>> [Nicolas: context conflicts in:
>> arch/powerpc/kernel/vmlinux.lds.S
>> include/asm-generic/vmlinux.lds.h
>> include/linux/compiler.h
>> include/linux/compiler_types.h]
>> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
>
> Did you build this on x86?
>
> I get the following build error:
>
> ld:./arch/x86/kernel/vmlinux.lds:20: syntax error
>
> And that line looks like:
>
> . = ALIGN(8); *(.text.hot .text.hot.*) *(.text .text.fixup) *(.text.unlikely .text.unlikely.*) *(.text.unknown .text.unknown.*) . = ALIGN(8); __noinstr_text_start = .; *(.__attribute__((noinline)) __attribute__((no_instrument_function)) __attribute((__section__(".noinstr.text"))).text) __noinstr_text_end = .; *(.text..refcount) *(.ref.text) *(.meminit.text*) *(.memexit.text*)
>
In the NOINSTR_TEXT macro, noinstr is expanded with the value of the noinstr
macro from linux/compiler_types.h while it shouldn't.
The problem is possibly that the noinstr macro is defined for assembly. Make
sure that the macro is not defined for assembly e.g.:
#ifndef __ASSEMBLY__
/* Section for code which can't be instrumented at all */
#define noinstr \
noinline notrace __attribute((__section__(".noinstr.text")))
#endif
alex.
^ permalink raw reply
* Re: [PATCH] powerpc: epapr: A typo fix
From: Randy Dunlap @ 2021-03-20 22:41 UTC (permalink / raw)
To: Bhaskar Chowdhury; +Cc: paulus, linuxppc-dev, linux-kernel
In-Reply-To: <20210320213932.22697-1-unixbhaskar@gmail.com>
On Sun, 21 Mar 2021, Bhaskar Chowdhury wrote:
>
> s/parmeters/parameters/
>
> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> arch/powerpc/include/asm/epapr_hcalls.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/epapr_hcalls.h b/arch/powerpc/include/asm/epapr_hcalls.h
> index c99ba08a408d..cdf3c6df5123 100644
> --- a/arch/powerpc/include/asm/epapr_hcalls.h
> +++ b/arch/powerpc/include/asm/epapr_hcalls.h
> @@ -65,7 +65,7 @@
> * but the gcc inline assembly syntax does not allow us to specify registers
> * on the clobber list that are also on the input/output list. Therefore,
> * the lists of clobbered registers depends on the number of register
> - * parmeters ("+r" and "=r") passed to the hypercall.
> + * parameters ("+r" and "=r") passed to the hypercall.
> *
> * Each assembly block should use one of the HCALL_CLOBBERSx macros. As a
> * general rule, 'x' is the number of parameters passed to the assembly
> --
> 2.26.2
>
>
^ permalink raw reply
* [GIT PULL] Please pull powerpc/linux.git powerpc-5.12-4 tag
From: Michael Ellerman @ 2021-03-21 5:15 UTC (permalink / raw)
To: Linus Torvalds; +Cc: tyreld, linuxppc-dev, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hi Linus,
Please pull some more powerpc fixes for 5.12:
The following changes since commit 0b736881c8f1a6cd912f7a9162b9e097b28c1c30:
powerpc/traps: unrecoverable_exception() is not an interrupt handler (2021-03-12 11:02:12 +1100)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.12-4
for you to fetch changes up to cc7a0bb058b85ea03db87169c60c7cfdd5d34678:
PCI: rpadlpar: Fix potential drc_name corruption in store functions (2021-03-17 13:48:07 +1100)
- ------------------------------------------------------------------
powerpc fixes for 5.12 #4
Fix a possible stack corruption and subsequent DLPAR failure in the rpadlpar_io
PCI hotplug driver.
Two build fixes for uncommon configurations.
Thanks to Christophe Leroy, Tyrel Datwyler.
- ------------------------------------------------------------------
Christophe Leroy (2):
powerpc/vdso32: Add missing _restgpr_31_x to fix build failure
powerpc: Force inlining of cpu_has_feature() to avoid build failure
Tyrel Datwyler (1):
PCI: rpadlpar: Fix potential drc_name corruption in store functions
arch/powerpc/include/asm/cpu_has_feature.h | 4 ++--
arch/powerpc/kernel/vdso32/gettimeofday.S | 11 +++++++++++
drivers/pci/hotplug/rpadlpar_sysfs.c | 14 ++++++--------
3 files changed, 19 insertions(+), 10 deletions(-)
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEEJFGtCPCthwEv2Y/bUevqPMjhpYAFAmBW1cQACgkQUevqPMjh
pYAhog/+KVrIL4frKx1m7bmM+FJ83heS20+F7i5yQWRovR2FdgYwwbIrtxNUKzRU
sNShwQbt9H6GVEGc12kusH5M+t6wGrHrYQ47kdkD6qhDTPfQzKXsoi5eWbgMkQnx
Hd3njqjzfGFEicmfp8l+1WmoWeYpL2MLR7E/KTtS/MYMq5Gsz2lZSMEsmXlAY5Cs
5lT8dBHdH0AX9krRJk9BzKrRqB0qjrK60/sg9oLJQ4sixeWaa+dSORnJq26jYV+n
MPyaNCWF3EUrs7BNyxfZOeFwAZ0FzRSBZrgIt6MNPreT/FaL6lo2McHHyQ9Ls5Qb
yPUmioNKLhXdtcU6AZ0+QOlQvyiGJ7xb3UTAbFFq8TifYHVS0v9srBkgfcvFHycc
yjDtdeTk18WB6NSDC8zVSc+Ut5q7WZa6RLKubiCPgd5DqDkpAKcYcTQSZZUgTb6Q
IyT7bwCBqW6Z5bNTsHhSZ+Ub06L6RLTQ1IfD4GXtqe16F1eeFlMOPCP2YzGUB6s9
IB7GQilliVtRZaFCGwYyVdZCHftHVdK5k7DV+3aImzPRYOKpL4YWwrBnDoqkccYs
Mhbw2YtO7oXSZ2yXGrsu/WF56QCDf6PFO5r28dvNFEi6qZWbfLEQQLZmv/0efFtC
Je1VKXKXCfuLD2VH02F8URZpzbg90dC8YRsTUpgSLCHgiGjEOcE=
=zfoH
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: remove the legacy ide driver
From: John Paul Adrian Glaubitz @ 2021-03-21 9:20 UTC (permalink / raw)
To: Christoph Hellwig, David S. Miller, Jens Axboe,
Geert Uytterhoeven
Cc: Thomas Bogendoerfer, linux-doc, Bartlomiej Zolnierkiewicz,
Russell King, linux-kernel, linux-ide, linux-m68k,
Ivan Kokshaysky, linux-alpha, Matt Turner, linux-mips,
linuxppc-dev, linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-1-hch@lst.de>
Hello Christoph!
On 3/18/21 5:56 AM, Christoph Hellwig wrote:
> libata mostly covers all hardware supported by the legacy ide driver.
> There are three mips drivers that are not supported, but the linux-mips
> list could not identify any users of those. There also are two m68k
> drivers that do not have libata equivalents, which might or might not
> have users, so we'll need some input and possibly help from the m68k
> community here.
I think those drivers were the Q60 driver and the MacIDE driver, weren't they?
Either way, I have so far been unsuccessful in obtaining access to these machines
but I assume once we gain access to such machines, Bartlomiej could convert the
drivers the same way he already converted the falcon, gayle and buddha drivers,
for example.
One could also just convert the drivers to libata and include them untested, the
conversion itself seems pretty little work for someone experienced with libata.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: [PATCH next v1 2/3] printk: remove safe buffers
From: Sergey Senozhatsky @ 2021-03-21 5:26 UTC (permalink / raw)
To: John Ogness
Cc: Rafael Aquini, Peter Zijlstra, Paul Mackerras, Tiezhu Yang,
Alexey Kardashevskiy, Yue Hu, Jordan Niethe, Petr Mladek,
Kees Cook, Paul E. McKenney, Alistair Popple,
Guilherme G. Piccoli, Nicholas Piggin, Steven Rostedt,
Thomas Gleixner, kexec, linux-kernel, Eric Biederman,
Andrew Morton, linuxppc-dev, Cédric Le Goater
In-Reply-To: <20210316233326.10778-3-john.ogness@linutronix.de>
On (21/03/17 00:33), John Ogness wrote:
[..]
> void printk_nmi_direct_enter(void)
> {
> @@ -324,27 +44,8 @@ void printk_nmi_direct_exit(void)
> this_cpu_and(printk_context, ~PRINTK_NMI_DIRECT_CONTEXT_MASK);
> }
>
> -#else
> -
> -static __printf(1, 0) int vprintk_nmi(const char *fmt, va_list args)
> -{
> - return 0;
> -}
> -
> #endif /* CONFIG_PRINTK_NMI */
>
> -/*
> - * Lock-less printk(), to avoid deadlocks should the printk() recurse
> - * into itself. It uses a per-CPU buffer to store the message, just like
> - * NMI.
> - */
> -static __printf(1, 0) int vprintk_safe(const char *fmt, va_list args)
> -{
> - struct printk_safe_seq_buf *s = this_cpu_ptr(&safe_print_seq);
> -
> - return printk_safe_log_store(s, fmt, args);
> -}
> -
> /* Can be preempted by NMI. */
> void __printk_safe_enter(void)
> {
> @@ -369,7 +70,10 @@ __printf(1, 0) int vprintk_func(const char *fmt, va_list args)
> * Use the main logbuf even in NMI. But avoid calling console
> * drivers that might have their own locks.
> */
> - if ((this_cpu_read(printk_context) & PRINTK_NMI_DIRECT_CONTEXT_MASK)) {
> + if (this_cpu_read(printk_context) &
> + (PRINTK_NMI_DIRECT_CONTEXT_MASK |
> + PRINTK_NMI_CONTEXT_MASK |
> + PRINTK_SAFE_CONTEXT_MASK)) {
Do we need printk_nmi_direct_enter/exit() and PRINTK_NMI_DIRECT_CONTEXT_MASK?
Seems like all printk_safe() paths are now DIRECT - we store messages to the
prb, but don't call console drivers.
-ss
^ permalink raw reply
* [PATCH] crypto: nx: fix incorrect kernel-doc comment syntax in files
From: Aditya Srivastava @ 2021-03-21 12:30 UTC (permalink / raw)
To: linux-kernel
Cc: nayna, herbert, corbet, rdunlap, linuxppc-dev, yashsri421,
pfsmorigo, linux-crypto, leitao, lukas.bulwahn,
linux-kernel-mentees, davem, paulus
The opening comment mark '/**' is used for highlighting the beginning of
kernel-doc comments.
There are certain files in drivers/crypto/nx, which follow this syntax,
but the content inside does not comply with kernel-doc.
Such lines were probably not meant for kernel-doc parsing, but are parsed
due to the presence of kernel-doc like comment syntax(i.e, '/**'), which
causes unexpected warnings from kernel-doc.
E.g., presence of kernel-doc like comment in the header lines for
drivers/crypto/nx/nx-sha256.c at header causes these warnings:
"warning: Function parameter or member 'tfm' not described in 'nx_crypto_ctx_sha256_init'"
"warning: expecting prototype for SHA(). Prototype was for nx_crypto_ctx_sha256_init() instead"
Similarly for other files too.
Provide a simple fix by replacing such occurrences with general comment
format, i.e. '/*', to prevent kernel-doc from parsing it.
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
* Applies perfectly on next-20210319
drivers/crypto/nx/nx-aes-cbc.c | 2 +-
drivers/crypto/nx/nx-aes-ccm.c | 2 +-
drivers/crypto/nx/nx-aes-ctr.c | 2 +-
drivers/crypto/nx/nx-aes-ecb.c | 2 +-
drivers/crypto/nx/nx-aes-gcm.c | 2 +-
drivers/crypto/nx/nx-aes-xcbc.c | 2 +-
drivers/crypto/nx/nx-sha256.c | 2 +-
drivers/crypto/nx/nx-sha512.c | 2 +-
drivers/crypto/nx/nx.c | 2 +-
drivers/crypto/nx/nx_debugfs.c | 2 +-
10 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index 92e921eceed7..d6314ea9ae89 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* AES CBC routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index 4c9362eebefd..e7384d107573 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* AES CCM routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c
index 6d5ce1a66f1e..13f518802343 100644
--- a/drivers/crypto/nx/nx-aes-ctr.c
+++ b/drivers/crypto/nx/nx-aes-ctr.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* AES CTR routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx-aes-ecb.c b/drivers/crypto/nx/nx-aes-ecb.c
index 77e338dc33f1..7a729dc2bc17 100644
--- a/drivers/crypto/nx/nx-aes-ecb.c
+++ b/drivers/crypto/nx/nx-aes-ecb.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* AES ECB routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index 19c6ed5baea4..fc9baca13920 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* AES GCM routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c
index 48dc1c98ca52..eb5c8f689360 100644
--- a/drivers/crypto/nx/nx-aes-xcbc.c
+++ b/drivers/crypto/nx/nx-aes-xcbc.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* AES XCBC routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c
index 90d9a37a57f6..b0ad665e4bda 100644
--- a/drivers/crypto/nx/nx-sha256.c
+++ b/drivers/crypto/nx/nx-sha256.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* SHA-256 routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx-sha512.c b/drivers/crypto/nx/nx-sha512.c
index eb8627a0f317..c29103a1a0b6 100644
--- a/drivers/crypto/nx/nx-sha512.c
+++ b/drivers/crypto/nx/nx-sha512.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* SHA-512 routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index 1d0e8a1ba160..13bb705ba6a4 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* Routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
diff --git a/drivers/crypto/nx/nx_debugfs.c b/drivers/crypto/nx/nx_debugfs.c
index 1975bcbee997..ee7cd88bb10a 100644
--- a/drivers/crypto/nx/nx_debugfs.c
+++ b/drivers/crypto/nx/nx_debugfs.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/**
+/*
* debugfs routines supporting the Power 7+ Nest Accelerators driver
*
* Copyright (C) 2011-2012 International Business Machines Inc.
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] crypto: nx: fix incorrect kernel-doc comment syntax in files
From: Randy Dunlap @ 2021-03-21 17:34 UTC (permalink / raw)
To: Aditya Srivastava
Cc: linuxppc-dev, herbert, corbet, nayna, linux-kernel, pfsmorigo,
linux-crypto, leitao, lukas.bulwahn, linux-kernel-mentees, davem,
paulus
In-Reply-To: <20210321123007.15505-1-yashsri421@gmail.com>
Looks good. Thanks.
Acked-by: Randy Dunlap <rdunlap@infradead.org>
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.12-4 tag
From: pr-tracker-bot @ 2021-03-21 18:45 UTC (permalink / raw)
To: Michael Ellerman; +Cc: tyreld, Linus Torvalds, linuxppc-dev, linux-kernel
In-Reply-To: <875z1lhytk.fsf@mpe.ellerman.id.au>
The pull request you sent on Sun, 21 Mar 2021 16:15:35 +1100:
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.12-4
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b35660a7cebdf438e01bba05075ae2bcc0125650
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v11 1/6] kasan: allow an architecture to disable inline instrumentation
From: Daniel Axtens @ 2021-03-22 0:29 UTC (permalink / raw)
To: Balbir Singh
Cc: aneesh.kumar, linux-kernel, linux-mm, kasan-dev, linuxppc-dev
In-Reply-To: <20210320014606.GB77072@balbir-desktop>
Balbir Singh <bsingharora@gmail.com> writes:
> On Sat, Mar 20, 2021 at 01:40:53AM +1100, Daniel Axtens wrote:
>> For annoying architectural reasons, it's very difficult to support inline
>> instrumentation on powerpc64.
>
> I think we can expand here and talk about how in hash mode, the vmalloc
> address space is in a region of memory different than where kernel virtual
> addresses are mapped. Did I recollect the reason correctly?
I think that's _a_ reason, but for radix mode (which is all I support at
the moment), the reason is a bit simpler. We call into generic code like
the DT parser and printk when we have translations off. The shadow
region lives at c00e.... which is not part of the linear mapping, so if
you try to access the shadow while in real mode you will access unmapped
memory and (at least on PowerNV) take a machine check.
>>
>> Add a Kconfig flag to allow an arch to disable inline. (It's a bit
>> annoying to be 'backwards', but I'm not aware of any way to have
>> an arch force a symbol to be 'n', rather than 'y'.)
>>
>> We also disable stack instrumentation in this case as it does things that
>> are functionally equivalent to inline instrumentation, namely adding
>> code that touches the shadow directly without going through a C helper.
>>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> ---
>> lib/Kconfig.kasan | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
>> index cffc2ebbf185..7e237dbb6df3 100644
>> --- a/lib/Kconfig.kasan
>> +++ b/lib/Kconfig.kasan
>> @@ -12,6 +12,9 @@ config HAVE_ARCH_KASAN_HW_TAGS
>> config HAVE_ARCH_KASAN_VMALLOC
>> bool
>>
>> +config ARCH_DISABLE_KASAN_INLINE
>> + def_bool n
>> +
>
> Some comments on what arch's want to disable kasan inline would
> be helpful and why.
Sure, added.
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH v11 6/6] powerpc: Book3S 64-bit outline-only KASAN support
From: Daniel Axtens @ 2021-03-22 0:55 UTC (permalink / raw)
To: Balbir Singh
Cc: aneesh.kumar, linux-kernel, linux-mm, kasan-dev, linuxppc-dev
In-Reply-To: <20210320060259.GF77072@balbir-desktop>
Hi Balbir,
> Could you highlight the changes from
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20170729140901.5887-1-bsingharora@gmail.com/?
>
> Feel free to use my signed-off-by if you need to and add/update copyright
> headers if appropriate.
There's not really anything in common any more:
- ppc32 KASAN landed, so there was already a kasan.h for powerpc, the
explicit memcpy changes, the support for non-instrumented files,
prom_check.sh, etc. all already landed.
- I locate the shadow region differently and don't resize any virtual
memory areas.
- The ARCH_DEFINES_KASAN_ZERO_PTE handling changed upstream and our
handling for that is now handled more by patch 3.
- The outline hook is now an inline function rather than a #define.
- The init function has been totally rewritten as it's gone from
supporting real mode to not supporting real mode and back.
- The list of non-instrumented files has grown a lot.
- There's new stuff: stack walking is now safe, KASAN vmalloc support
means modules are better supported now, ptdump works, and there's
documentation.
It's been a while now, but I don't think when I started this process 2
years ago that I directly reused much of your code. So I'm not sure that
a signed-off-by makes sense here? Would a different tag (Originally-by?)
make more sense?
>> + * The shadow ends before the highest accessible address
>> + * because we don't need a shadow for the shadow. Instead:
>> + * c00e000000000000 << 3 + a80e 0000 0000 0000 000 = c00fc00000000000
>
> The comment has one extra 0 in a80e.., I did the math and had to use
> the data from the defines :)
3 extra 0s, even! Fixed.
>> +void __init kasan_init(void)
>> +{
>> + /*
>> + * We want to do the following things:
>> + * 1) Map real memory into the shadow for all physical memblocks
>> + * This takes us from c000... to c008...
>> + * 2) Leave a hole over the shadow of vmalloc space. KASAN_VMALLOC
>> + * will manage this for us.
>> + * This takes us from c008... to c00a...
>> + * 3) Map the 'early shadow'/zero page over iomap and vmemmap space.
>> + * This takes us up to where we start at c00e...
>> + */
>> +
>
> assuming we have
> #define VMEMMAP_END R_VMEMMAP_END
> and ditto for hash we probably need
>
> BUILD_BUG_ON(VMEMMAP_END + KASAN_SHADOW_OFFSET != KASAN_SHADOW_END);
Sorry, I'm not sure what this is supposed to be testing? In what
situation would this trigger?
Kind regards,
Daniel
>
> Looks good otherwise, I've not been able to test it yet
>
> Balbir Singh.
^ permalink raw reply
* Re: [PATCH v3 15/41] KVM: PPC: Book3S 64: Minimise hcall handler calling convention differences
From: Alexey Kardashevskiy @ 2021-03-22 2:09 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210305150638.2675513-16-npiggin@gmail.com>
On 06/03/2021 02:06, Nicholas Piggin wrote:
> This sets up the same calling convention from interrupt entry to
> KVM interrupt handler for system calls as exists for other interrupt
> types.
>
> This is a better API, it uses a save area rather than SPR, and it has
> more registers free to use. Using a single common API helps maintain
> it, and it becomes easier to use in C in a later patch.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/kernel/exceptions-64s.S | 16 +++++++++++++++-
> arch/powerpc/kvm/book3s_64_entry.S | 22 +++-------------------
> 2 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index b4eab5084964..ce6f5f863d3d 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -1892,8 +1892,22 @@ EXC_VIRT_END(system_call, 0x4c00, 0x100)
>
> #ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> TRAMP_REAL_BEGIN(kvm_hcall)
> + std r9,PACA_EXGEN+EX_R9(r13)
> + std r11,PACA_EXGEN+EX_R11(r13)
> + std r12,PACA_EXGEN+EX_R12(r13)
> + mfcr r9
> mfctr r10
> - SET_SCRATCH0(r10) /* Save r13 in SCRATCH0 */
> + std r10,PACA_EXGEN+EX_R13(r13)
> + li r10,0
> + std r10,PACA_EXGEN+EX_CFAR(r13)
> + std r10,PACA_EXGEN+EX_CTR(r13)
> +BEGIN_FTR_SECTION
> + mfspr r10,SPRN_PPR
> + std r10,PACA_EXGEN+EX_PPR(r13)
> +END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> +
> + HMT_MEDIUM
> +
> #ifdef CONFIG_RELOCATABLE
> /*
> * Requires __LOAD_FAR_HANDLER beause kvmppc_hcall lives
> diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
> index 7a6b060ceed8..129d3f81800e 100644
> --- a/arch/powerpc/kvm/book3s_64_entry.S
> +++ b/arch/powerpc/kvm/book3s_64_entry.S
> @@ -14,24 +14,9 @@
> .global kvmppc_hcall
> .balign IFETCH_ALIGN_BYTES
> kvmppc_hcall:
> - /*
> - * This is a hcall, so register convention is as
> - * Documentation/powerpc/papr_hcalls.rst, with these additions:
> - * R13 = PACA
> - * guest R13 saved in SPRN_SCRATCH0
> - * R10 = free
> - */
> -BEGIN_FTR_SECTION
> - mfspr r10,SPRN_PPR
> - std r10,HSTATE_PPR(r13)
> -END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> - HMT_MEDIUM
> - mfcr r10
> - std r12,HSTATE_SCRATCH0(r13)
> - sldi r12,r10,32
> - ori r12,r12,0xc00
> - ld r10,PACA_EXGEN+EX_R10(r13)
> - b do_kvm_interrupt
> + ld r10,PACA_EXGEN+EX_R13(r13)
> + SET_SCRATCH0(r10)
> + li r10,0xc00
>
> .global kvmppc_interrupt
> .balign IFETCH_ALIGN_BYTES
> @@ -62,7 +47,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> ld r10,EX_R10(r11)
> ld r11,EX_R11(r11)
>
> -do_kvm_interrupt:
> /*
> * Hcalls and other interrupts come here after normalising register
> * contents and save locations:
>
--
Alexey
^ permalink raw reply
* [PATCH] cxl: Fix couple of spellings
From: Bhaskar Chowdhury @ 2021-03-22 2:33 UTC (permalink / raw)
To: fbarrat, ajd, arnd, gregkh, linuxppc-dev, linux-kernel
Cc: rdunlap, Bhaskar Chowdhury
s/filesytem/filesystem/
s/symantics/semantics/
Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
---
drivers/misc/cxl/context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index fb2eff69e449..e627b4056623 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -52,7 +52,7 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
* can always access it when dereferenced from IDR. For the same
* reason, the segment table is only destroyed after the context is
* removed from the IDR. Access to this in the IOCTL is protected by
- * Linux filesytem symantics (can't IOCTL until open is complete).
+ * Linux filesystem semantics (can't IOCTL until open is complete).
*/
i = cxl_alloc_sst(ctx);
if (i)
--
2.31.0
^ permalink raw reply related
* Re: [PATCH 5/6] powerpc/mm/64s/hash: Add real-mode change_memory_range() for hash LPAR
From: Nicholas Piggin @ 2021-03-22 2:56 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman; +Cc: aneesh.kumar
In-Reply-To: <878s6iht88.fsf@mpe.ellerman.id.au>
Excerpts from Michael Ellerman's message of March 20, 2021 11:04 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Excerpts from Michael Ellerman's message of February 11, 2021 11:51 pm:
> ...
>>> diff --git a/arch/powerpc/mm/book3s64/hash_pgtable.c b/arch/powerpc/mm/book3s64/hash_pgtable.c
>>> index 3663d3cdffac..01de985df2c4 100644
>>> --- a/arch/powerpc/mm/book3s64/hash_pgtable.c
>>> +++ b/arch/powerpc/mm/book3s64/hash_pgtable.c
>>> @@ -414,6 +428,73 @@ static void change_memory_range(unsigned long start, unsigned long end,
>>> mmu_kernel_ssize);
>>> }
>>>
>>> +static int notrace chmem_secondary_loop(struct change_memory_parms *parms)
>>> +{
>>> + unsigned long msr, tmp, flags;
>>> + int *p;
>>> +
>>> + p = &parms->cpu_counter.counter;
>>> +
>>> + local_irq_save(flags);
>>> + __hard_EE_RI_disable();
>>> +
>>> + asm volatile (
>>> + // Switch to real mode and leave interrupts off
>>> + "mfmsr %[msr] ;"
>>> + "li %[tmp], %[MSR_IR_DR] ;"
>>> + "andc %[tmp], %[msr], %[tmp] ;"
>>> + "mtmsrd %[tmp] ;"
>>> +
>>> + // Tell the master we are in real mode
>>> + "1: "
>>> + "lwarx %[tmp], 0, %[p] ;"
>>> + "addic %[tmp], %[tmp], -1 ;"
>>> + "stwcx. %[tmp], 0, %[p] ;"
>>> + "bne- 1b ;"
>>> +
>>> + // Spin until the counter goes to zero
>>> + "2: ;"
>>> + "lwz %[tmp], 0(%[p]) ;"
>>> + "cmpwi %[tmp], 0 ;"
>>> + "bne- 2b ;"
>>> +
>>> + // Switch back to virtual mode
>>> + "mtmsrd %[msr] ;"
>>> +
>>> + : // outputs
>>> + [msr] "=&r" (msr), [tmp] "=&b" (tmp), "+m" (*p)
>>> + : // inputs
>>> + [p] "b" (p), [MSR_IR_DR] "i" (MSR_IR | MSR_DR)
>>> + : // clobbers
>>> + "cc", "xer"
>>> + );
>>> +
>>> + local_irq_restore(flags);
>>
>> Hmm. __hard_EE_RI_disable won't get restored by this because it doesn't
>> set the HARD_DIS flag. Also we don't want RI disabled here because
>> tracing will get called first (which might take SLB or HPTE fault).
>
> Thanks for noticing. I originally wrote hard_irq_disable() but then
> thought disabling RI also would be good.
>
>> But it's also slightly rude to ever enable EE under an irq soft mask,
>> because you don't know if it had been disabled by the masked interrupt
>> handler. It's not strictly a problem AFAIK because the interrupt would
>> just get masked again, but if we try to maintain a good pattern would
>> be good. Hmm that means we should add a check for irqs soft masked in
>> __hard_irq_enable(), I'm not sure if all existing users would follow
>> this rule.
>>
>> Might be better to call hard_irq_disable(); after the local_irq_save();
>> and then clear and reset RI inside that region (could just do it at the
>> same time as disabling MMU).
>
> Thinking about it more, there's no real reason to disable RI.
>
> We should be able to return from an interrupt in there, it's just that
> if we do take one we'll probably die before we get a chance to return
> because the mapping of text will be missing.
Yeah it probably will because the pseries hash machine check handler has
some hacks in it that require turning the MMU on. We might never fix
that if we're moving to radix, but if we did then in theory we'd be able
to take a MCE here and recover.
> So disabling RI doesn't really gain us anything I don't think.
Yeah I probably agree. So local_irq_save(flags); hard_irq_disable();
should do the trick.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v11 6/6] powerpc: Book3S 64-bit outline-only KASAN support
From: Balbir Singh @ 2021-03-22 2:59 UTC (permalink / raw)
To: Daniel Axtens
Cc: aneesh.kumar, linux-kernel, linux-mm, kasan-dev, linuxppc-dev
In-Reply-To: <87o8fcatxv.fsf@dja-thinkpad.axtens.net>
On Mon, Mar 22, 2021 at 11:55:08AM +1100, Daniel Axtens wrote:
> Hi Balbir,
>
> > Could you highlight the changes from
> > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20170729140901.5887-1-bsingharora@gmail.com/?
> >
> > Feel free to use my signed-off-by if you need to and add/update copyright
> > headers if appropriate.
>
> There's not really anything in common any more:
>
> - ppc32 KASAN landed, so there was already a kasan.h for powerpc, the
> explicit memcpy changes, the support for non-instrumented files,
> prom_check.sh, etc. all already landed.
>
> - I locate the shadow region differently and don't resize any virtual
> memory areas.
>
> - The ARCH_DEFINES_KASAN_ZERO_PTE handling changed upstream and our
> handling for that is now handled more by patch 3.
>
> - The outline hook is now an inline function rather than a #define.
>
> - The init function has been totally rewritten as it's gone from
> supporting real mode to not supporting real mode and back.
>
> - The list of non-instrumented files has grown a lot.
>
> - There's new stuff: stack walking is now safe, KASAN vmalloc support
> means modules are better supported now, ptdump works, and there's
> documentation.
>
> It's been a while now, but I don't think when I started this process 2
> years ago that I directly reused much of your code. So I'm not sure that
> a signed-off-by makes sense here? Would a different tag (Originally-by?)
> make more sense?
>
Sure
> >> + * The shadow ends before the highest accessible address
> >> + * because we don't need a shadow for the shadow. Instead:
> >> + * c00e000000000000 << 3 + a80e 0000 0000 0000 000 = c00fc00000000000
> >
> > The comment has one extra 0 in a80e.., I did the math and had to use
> > the data from the defines :)
>
> 3 extra 0s, even! Fixed.
>
> >> +void __init kasan_init(void)
> >> +{
> >> + /*
> >> + * We want to do the following things:
> >> + * 1) Map real memory into the shadow for all physical memblocks
> >> + * This takes us from c000... to c008...
> >> + * 2) Leave a hole over the shadow of vmalloc space. KASAN_VMALLOC
> >> + * will manage this for us.
> >> + * This takes us from c008... to c00a...
> >> + * 3) Map the 'early shadow'/zero page over iomap and vmemmap space.
> >> + * This takes us up to where we start at c00e...
> >> + */
> >> +
> >
> > assuming we have
> > #define VMEMMAP_END R_VMEMMAP_END
> > and ditto for hash we probably need
> >
> > BUILD_BUG_ON(VMEMMAP_END + KASAN_SHADOW_OFFSET != KASAN_SHADOW_END);
>
> Sorry, I'm not sure what this is supposed to be testing? In what
> situation would this trigger?
>
I am bit concerned that we have hard coded (IIR) 0xa80e... in the
config, any changes to VMEMMAP_END, KASAN_SHADOW_OFFSET/END
should be guarded.
Balbir Singh.
^ permalink raw reply
* Re: [PATCH] cxl: Fix couple of spellings
From: Randy Dunlap @ 2021-03-22 3:02 UTC (permalink / raw)
To: Bhaskar Chowdhury, fbarrat, ajd, arnd, gregkh, linuxppc-dev,
linux-kernel
In-Reply-To: <20210322023307.168754-1-unixbhaskar@gmail.com>
On 3/21/21 7:33 PM, Bhaskar Chowdhury wrote:
>
> s/filesytem/filesystem/
> s/symantics/semantics/
>
> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> drivers/misc/cxl/context.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
> index fb2eff69e449..e627b4056623 100644
> --- a/drivers/misc/cxl/context.c
> +++ b/drivers/misc/cxl/context.c
> @@ -52,7 +52,7 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master)
> * can always access it when dereferenced from IDR. For the same
> * reason, the segment table is only destroyed after the context is
> * removed from the IDR. Access to this in the IOCTL is protected by
> - * Linux filesytem symantics (can't IOCTL until open is complete).
> + * Linux filesystem semantics (can't IOCTL until open is complete).
> */
> i = cxl_alloc_sst(ctx);
> if (i)
> --
--
~Randy
^ permalink raw reply
* Re: [PATCH 5/6] powerpc/mm/64s/hash: Add real-mode change_memory_range() for hash LPAR
From: Nicholas Piggin @ 2021-03-22 3:09 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman; +Cc: aneesh.kumar
In-Reply-To: <87czvz4n47.fsf@mpe.ellerman.id.au>
Excerpts from Michael Ellerman's message of March 16, 2021 4:40 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Excerpts from Michael Ellerman's message of February 11, 2021 11:51 pm:
>>> When we enabled STRICT_KERNEL_RWX we received some reports of boot
>>> failures when using the Hash MMU and running under phyp. The crashes
>>> are intermittent, and often exhibit as a completely unresponsive
>>> system, or possibly an oops.
> ...
>>>
>>> diff --git a/arch/powerpc/mm/book3s64/hash_pgtable.c b/arch/powerpc/mm/book3s64/hash_pgtable.c
>>> index 3663d3cdffac..01de985df2c4 100644
>>> --- a/arch/powerpc/mm/book3s64/hash_pgtable.c
>>> +++ b/arch/powerpc/mm/book3s64/hash_pgtable.c
>>> @@ -414,6 +428,73 @@ static void change_memory_range(unsigned long start, unsigned long end,
>>> mmu_kernel_ssize);
>>> }
>>>
>>> +static int notrace chmem_secondary_loop(struct change_memory_parms *parms)
>>> +{
>>> + unsigned long msr, tmp, flags;
>>> + int *p;
>>> +
>>> + p = &parms->cpu_counter.counter;
>>> +
>>> + local_irq_save(flags);
>>> + __hard_EE_RI_disable();
>>> +
>>> + asm volatile (
>>> + // Switch to real mode and leave interrupts off
>>> + "mfmsr %[msr] ;"
>>> + "li %[tmp], %[MSR_IR_DR] ;"
>>> + "andc %[tmp], %[msr], %[tmp] ;"
>>> + "mtmsrd %[tmp] ;"
>>> +
>>> + // Tell the master we are in real mode
>>> + "1: "
>>> + "lwarx %[tmp], 0, %[p] ;"
>>> + "addic %[tmp], %[tmp], -1 ;"
>>> + "stwcx. %[tmp], 0, %[p] ;"
>>> + "bne- 1b ;"
>>> +
>>> + // Spin until the counter goes to zero
>>> + "2: ;"
>>> + "lwz %[tmp], 0(%[p]) ;"
>>> + "cmpwi %[tmp], 0 ;"
>>> + "bne- 2b ;"
>>> +
>>> + // Switch back to virtual mode
>>> + "mtmsrd %[msr] ;"
>>
>> Pity we don't have something that can switch to emergency stack and
>> so we can write this stuff in C.
>>
>> How's something like this suit you?
>
> It looks like it would be really good for writing exploits :)
Hmm. In that case maybe the callee function could be inlined into it
like the interrupt wrappers, and the asm real-mode entry/exit gets
added around it rather than have this little exploit stub. So similar to
yours but with a stack switch as well so you can come back up in real
mode.
> I think at the very least we would want the asm part to load the SP
> from the paca itself, rather than taking it as a parameter.
>
> But I'm not sure writing these type of things in C is a big win, because
> you have to be so careful about what you call anyway. It's almost better
> in asm because it's so restrictive.
>
> Obviously having said that, my first attempt got the IRQ save/restore
> wrong, so maybe we should at least have some macros to help with it.
>
> Did you have another user for this in mind? The only one that I can
> think of at the moment is the subcore stuff.
Possibly rtas entry/exit (although that has other issues). But I guess
it's not a huge amount of asm compared with what I'm dealing with.
I'm okay if you just put your thing in at the moment, we might or might
not get keen and c-ify it later.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 14/41] KVM: PPC: Book3S 64: move bad_host_intr check to HV handler
From: Nicholas Piggin @ 2021-03-22 3:18 UTC (permalink / raw)
To: Alexey Kardashevskiy, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <1f68b37c-7167-30d7-ee19-f6ebc69bd4a6@ozlabs.ru>
Excerpts from Alexey Kardashevskiy's message of March 20, 2021 7:07 pm:
>
>
> On 06/03/2021 02:06, Nicholas Piggin wrote:
>> This is not used by PR KVM.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> a small tote - it probably makes sense to move this before 09/41 as this
> one removes what 09/41 added to book3s_64_entry.S. Thanks,
Thanks.
I do realise there's a bit of shuffling around in this part of the
series, I'm trying to see if that can be improved a bit. But 9/41
is just moving the code without change which I prefer to do first.
This one changes the calling convention for PR which I think is
better to do after we have the entry point in a common file.
Thanks,
Nick
>
>
>> ---
>> arch/powerpc/kvm/book3s_64_entry.S | 3 ---
>> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 +++-
>> arch/powerpc/kvm/book3s_segment.S | 7 +++++++
>> 3 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
>> index d06e81842368..7a6b060ceed8 100644
>> --- a/arch/powerpc/kvm/book3s_64_entry.S
>> +++ b/arch/powerpc/kvm/book3s_64_entry.S
>> @@ -78,11 +78,8 @@ do_kvm_interrupt:
>> beq- .Lmaybe_skip
>> .Lno_skip:
>> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>> - cmpwi r9,KVM_GUEST_MODE_HOST_HV
>> - beq kvmppc_bad_host_intr
>> #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
>> cmpwi r9,KVM_GUEST_MODE_GUEST
>> - ld r9,HSTATE_SCRATCH2(r13)
>> beq kvmppc_interrupt_pr
>> #endif
>> b kvmppc_interrupt_hv
>> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> index f976efb7e4a9..75405ef53238 100644
>> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
>> @@ -1265,6 +1265,7 @@ hdec_soon:
>> kvmppc_interrupt_hv:
>> /*
>> * Register contents:
>> + * R9 = HSTATE_IN_GUEST
>> * R12 = (guest CR << 32) | interrupt vector
>> * R13 = PACA
>> * guest R12 saved in shadow VCPU SCRATCH0
>> @@ -1272,6 +1273,8 @@ kvmppc_interrupt_hv:
>> * guest R9 saved in HSTATE_SCRATCH2
>> */
>> /* We're now back in the host but in guest MMU context */
>> + cmpwi r9,KVM_GUEST_MODE_HOST_HV
>> + beq kvmppc_bad_host_intr
>> li r9, KVM_GUEST_MODE_HOST_HV
>> stb r9, HSTATE_IN_GUEST(r13)
>>
>> @@ -3272,7 +3275,6 @@ END_FTR_SECTION_IFCLR(CPU_FTR_P9_TM_HV_ASSIST)
>> * cfar is saved in HSTATE_CFAR(r13)
>> * ppr is saved in HSTATE_PPR(r13)
>> */
>> -.global kvmppc_bad_host_intr
>> kvmppc_bad_host_intr:
>> /*
>> * Switch to the emergency stack, but start half-way down in
>> diff --git a/arch/powerpc/kvm/book3s_segment.S b/arch/powerpc/kvm/book3s_segment.S
>> index 1f492aa4c8d6..ef1d88b869bf 100644
>> --- a/arch/powerpc/kvm/book3s_segment.S
>> +++ b/arch/powerpc/kvm/book3s_segment.S
>> @@ -167,8 +167,15 @@ kvmppc_interrupt_pr:
>> * R12 = (guest CR << 32) | exit handler id
>> * R13 = PACA
>> * HSTATE.SCRATCH0 = guest R12
>> + *
>> + * If HV is possible, additionally:
>> + * R9 = HSTATE_IN_GUEST
>> + * HSTATE.SCRATCH2 = guest R9
>> */
>> #ifdef CONFIG_PPC64
>> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>> + ld r9,HSTATE_SCRATCH2(r13)
>> +#endif
>> /* Match 32-bit entry */
>> rotldi r12, r12, 32 /* Flip R12 halves for stw */
>> stw r12, HSTATE_SCRATCH1(r13) /* CR is now in the low half */
>>
>
> --
> Alexey
>
^ permalink raw reply
* Re: [PATCH v3 34/41] KVM: PPC: Book3S HV: Remove support for dependent threads mode on P9
From: Nicholas Piggin @ 2021-03-22 3:27 UTC (permalink / raw)
To: Aneesh Kumar K.V, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87ft0tzug8.fsf@linux.ibm.com>
Excerpts from Aneesh Kumar K.V's message of March 18, 2021 1:11 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> Radix guest support will be removed from the P7/8 path, so disallow
>> dependent threads mode on P9.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/kvm_host.h | 1 -
>> arch/powerpc/kvm/book3s_hv.c | 27 +++++----------------------
>> 2 files changed, 5 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> index 05fb00d37609..dd017dfa4e65 100644
>> --- a/arch/powerpc/include/asm/kvm_host.h
>> +++ b/arch/powerpc/include/asm/kvm_host.h
>> @@ -304,7 +304,6 @@ struct kvm_arch {
>> u8 fwnmi_enabled;
>> u8 secure_guest;
>> u8 svm_enabled;
>> - bool threads_indep;
>> bool nested_enable;
>> bool dawr1_enabled;
>> pgd_t *pgtable;
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index cb428e2f7140..928ed8180d9d 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -103,13 +103,9 @@ static int target_smt_mode;
>> module_param(target_smt_mode, int, 0644);
>> MODULE_PARM_DESC(target_smt_mode, "Target threads per core (0 = max)");
>>
>> -static bool indep_threads_mode = true;
>> -module_param(indep_threads_mode, bool, S_IRUGO | S_IWUSR);
>> -MODULE_PARM_DESC(indep_threads_mode, "Independent-threads mode (only on POWER9)");
>> -
>> static bool one_vm_per_core;
>> module_param(one_vm_per_core, bool, S_IRUGO | S_IWUSR);
>> -MODULE_PARM_DESC(one_vm_per_core, "Only run vCPUs from the same VM on a core (requires indep_threads_mode=N)");
>> +MODULE_PARM_DESC(one_vm_per_core, "Only run vCPUs from the same VM on a core (requires POWER8 or older)");
>
> Isn't this also a security feature, where there was an ask to make sure
> threads/vCPU from other VM won't run on this core? In that context isn't
> this applicable also for P9?
I'm not sure about an ask, but it is a possible security feature that
would be relevant to all SMT CPUs running KVM guests.
It doesn't make much sense to plumb P9 support all through the P8 path
just for that though, in my opinion? Is it tested? Who uses it? It's
lacking features of the P9 path.
It would be better added to KVM/QEMU in general (or until that is
available, disable SMT, or use CPU pinning and isolcpus to prevent host
code running on secondaries, and isolating VMs from one another, etc).
I think it's quite possible to rendezvous threads in kernel, move them
onto the threads of a core, and then have them all running in KVM code
before they enter the guest, without disabling SMT in the host.
You could do it with kernel threads on the secondaries even, but I
wouldn't like to have to plumb the vcore concept entirely through
everywhere so I would actually prefer to see QEMU grow an understanding
of it so it would know it has to call the ioctl on every guest SMT
thread.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Revert "powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc"
From: Aneesh Kumar K.V @ 2021-03-22 4:00 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <87lfak54lx.fsf@mpe.ellerman.id.au>
On 3/19/21 6:42 AM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> This reverts commit 675bceb097e6 ("powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc")
>>
>> All the related issues are fixed by the series
>> https://lore.kernel.org/linux-mm/20200902114222.181353-1-aneesh.kumar@linux.ibm.com
>
> Was that series merged?
>
> If so this seems like this could be tagged as a Fix for the last commit
> in that series.
commit f14312e1ed1e ("mm/debug_vm_pgtable: avoid doing memory allocation
with pgtable_t mapped.")
>
> cheers
>
>> Hence enable it back
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> Documentation/features/debug/debug-vm-pgtable/arch-support.txt | 2 +-
>> arch/powerpc/Kconfig | 1 +
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/features/debug/debug-vm-pgtable/arch-support.txt b/Documentation/features/debug/debug-vm-pgtable/arch-support.txt
>> index 7aff505af706..fa83403b4aec 100644
>> --- a/Documentation/features/debug/debug-vm-pgtable/arch-support.txt
>> +++ b/Documentation/features/debug/debug-vm-pgtable/arch-support.txt
>> @@ -21,7 +21,7 @@
>> | nios2: | TODO |
>> | openrisc: | TODO |
>> | parisc: | TODO |
>> - | powerpc: | TODO |
>> + | powerpc: | ok |
>> | riscv: | ok |
>> | s390: | ok |
>> | sh: | TODO |
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 386ae12d8523..982c87d5c051 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -119,6 +119,7 @@ config PPC
>> #
>> select ARCH_32BIT_OFF_T if PPC32
>> select ARCH_HAS_DEBUG_VIRTUAL
>> + select ARCH_HAS_DEBUG_VM_PGTABLE
>> select ARCH_HAS_DEVMEM_IS_ALLOWED
>> select ARCH_HAS_ELF_RANDOMIZE
>> select ARCH_HAS_FORTIFY_SOURCE
>> --
>> 2.30.2
^ permalink raw reply
* Re: [PATCH v3 15/41] KVM: PPC: Book3S 64: Minimise hcall handler calling convention differences
From: Nicholas Piggin @ 2021-03-22 4:06 UTC (permalink / raw)
To: kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev
In-Reply-To: <20210305150638.2675513-16-npiggin@gmail.com>
Excerpts from Nicholas Piggin's message of March 6, 2021 1:06 am:
> This sets up the same calling convention from interrupt entry to
> KVM interrupt handler for system calls as exists for other interrupt
> types.
>
> This is a better API, it uses a save area rather than SPR, and it has
> more registers free to use. Using a single common API helps maintain
> it, and it becomes easier to use in C in a later patch.
On second look I'm happy enough with this.
It does add some hcall setup code back into exception-64s.S and removes
most of the "fixup" code that was previously moved into
book3s_64_entry.S in patch 12. But if you take patch 12 and 13 and other
earlier patches together they are moving most KVM interrupt knowledge
into KVM which is a good change.
Once that is done, this final one then gets hcall into better shape for
the C code. If anything this patch could go together with patch 12 but
I guess I ended up writing it for the C code whereas the previous ones
were cleanups so the ordering didn't come out that way. It won't be
trivial to move now so I don't think I'd bother.
Thanks,
Nick
^ permalink raw reply
* Re: Advice needed on SMP regression after cpu_core_mask change
From: David Gibson @ 2021-03-22 3:16 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: aneesh.kumar, Srikar Dronamraju, Greg Kurz, Cédric Le Goater,
linuxppc-dev
In-Reply-To: <04bf6d12-a806-d417-3d95-b6d315c44b58@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7169 bytes --]
On Wed, Mar 17, 2021 at 01:05:21PM -0300, Daniel Henrique Barboza wrote:
>
>
> On 3/17/21 12:30 PM, Cédric Le Goater wrote:
> > On 3/17/21 2:00 PM, Daniel Henrique Barboza wrote:
> > > Hello,
> > >
> > > Patch 4bce545903fa ("powerpc/topology: Update topology_core_cpumask") introduced
> > > a regression in both upstream and RHEL downstream kernels [1]. The assumption made
> > > in the commit:
> > >
> > > "Further analysis shows that cpu_core_mask and cpu_cpu_mask for any CPU would be
> > > equal on Power"
> > >
> > > Doesn't seem to be true. After this commit, QEMU is now unable to set single NUMA
> > > node SMP topologies such as:
> > >
> > > -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
> > >
> > > lscpu will give the following output in this case:
> > >
> > > # lscpu
> > > Architecture: ppc64le
> > > Byte Order: Little Endian
> > > CPU(s): 8
> > > On-line CPU(s) list: 0-7
> > > Thread(s) per core: 2
> > > Core(s) per socket: 4
> > > Socket(s): 1
> > > NUMA node(s): 1
> > > Model: 2.2 (pvr 004e 1202)
> > > Model name: POWER9 (architected), altivec supported
> > > Hypervisor vendor: KVM
> > > Virtualization type: para
> > > L1d cache: 32K
> > > L1i cache: 32K
> > > NUMA node0 CPU(s): 0-7
> > >
> > >
> > > This is happening because the macro cpu_cpu_mask(cpu) expands to
> > > cpumask_of_node(cpu_to_node(cpu)), which in turn expands to node_to_cpumask_map[node].
> > > node_to_cpumask_map is a NUMA array that maps CPUs to NUMA nodes (Aneesh is on CC to
> > > correct me if I'm wrong). We're now associating sockets to NUMA nodes directly.
> > >
> > > If I add a second NUMA node then I can get the intended smp topology:
> > >
> > > -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
> > > -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
> > > -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
> > >
> > > # lscpu
> > > Architecture: ppc64le
> > > Byte Order: Little Endian
> > > CPU(s): 8
> > > On-line CPU(s) list: 0-7
> > > Thread(s) per core: 2
> > > Core(s) per socket: 2
> > > Socket(s): 2
> > > NUMA node(s): 2
> > > Model: 2.2 (pvr 004e 1202)
> > > Model name: POWER9 (architected), altivec supported
> > > Hypervisor vendor: KVM
> > > Virtualization type: para
> > > L1d cache: 32K
> > > L1i cache: 32K
> > > NUMA node0 CPU(s): 0-3
> > > NUMA node1 CPU(s): 4-7
> > >
> > >
> > > However, if I try a single socket with multiple NUMA nodes topology, which is the case
> > > of Power10, e.g.:
> > >
> > >
> > > -smp 8,maxcpus=8,cores=4,threads=2,sockets=1
> > > -numa node,memdev=mem0,cpus=0-3,nodeid=0 \
> > > -numa node,memdev=mem1,cpus=4-7,nodeid=1 \
> > >
> > >
> > > This is the result:
> > >
> > > # lscpu
> > > Architecture: ppc64le
> > > Byte Order: Little Endian
> > > CPU(s): 8
> > > On-line CPU(s) list: 0-7
> > > Thread(s) per core: 2
> > > Core(s) per socket: 2
> > > Socket(s): 2
> > > NUMA node(s): 2
> > > Model: 2.2 (pvr 004e 1202)
> > > Model name: POWER9 (architected), altivec supported
> > > Hypervisor vendor: KVM
> > > Virtualization type: para
> > > L1d cache: 32K
> > > L1i cache: 32K
> > > NUMA node0 CPU(s): 0-3
> > > NUMA node1 CPU(s): 4-7
> > >
> > >
> > > This confirms my suspicions that, at this moment, we're making sockets == NUMA nodes.
> >
> > Yes. I don't think we can do better on PAPR and the above examples
> > seem to confirm that the "sockets" definition is simply ignored.
> > > Cedric, the reason I'm CCing you is because this is related to ibm,chip-id. The commit
> > > after the one that caused the regression, 4ca234a9cbd7c3a65 ("powerpc/smp: Stop updating
> > > cpu_core_mask"), is erasing the code that calculated cpu_core_mask. cpu_core_mask, despite
> > > its shortcomings that caused its removal, was giving a precise SMP topology. And it was
> > > using physical_package_id/'ibm,chip-id' for that.
> >
> > ibm,chip-id is a no-no on pSeries. I guess this is inherent to PAPR which
> > is hiding a lot of the underlying HW and topology. May be we are trying
> > to reconcile two orthogonal views of machine virtualization ...
> >
> > > Checking in QEMU I can say that the ibm,chip-id calculation is the only place in the code
> > > that cares about cores per socket information. The kernel is now ignoring that, starting
> > > on 4bce545903fa, and now QEMU is unable to provide this info to the guest.
> > >
> > > If we're not going to use ibm,chip-id any longer, which seems sensible given that PAPR does
> > > not declare it, we need another way of letting the guest know how much cores per socket
> > > we want.
> > The RTAS call "ibm,get-system-parameter" with token "Processor Module
> > Information" returns that kind of information :
> >
> > 2 byte binary number (N) of module types followed by N module specifiers of the form:
> > 2 byte binary number (M) of sockets of this module type
> > 2 byte binary number (L) of chips per this module type
> > 2 byte binary number (K) of cores per chip in this module type.
> >
> > See the values in these sysfs files :
> >
> > cat /sys/devices/hv_24x7/interface/{sockets,chipspersocket,coresperchip}
> >
> > But I am afraid these are host level information and not guest/LPAR.
>
>
> I believe there might be some sort of reasoning behind not having this on
> PAPR, but I'll say in advance that the virtual machine should act as the
> real hardware, as close as possible. This is the kind of hcall that could
> be used in this situation.
In the case of POWER, that's pretty much a lost battle. The
virtualization features of the CPU don't really permit full hardware
virtualization - it has to be a paravirtualized environment. Once
that's the case, the value of keeping secondary things the same
between the bare metal and paravirt environments isn't that compelling
any more.
> > I didn't find any LPAR level properties or hcalls in the PAPR document.
> > They need to be specified.
> >
> > or
> >
> > We can add extra properties like ibm,chip-id but making sure it's only
> > used under the KVM hypervisor. My understanding is that's something we
> > are trying to avoid.
>
> We can change PAPR to add ibm,chip-id. Problem is that ibm,chip-id today, with
> the current kernel codebase, does not fix the issue because the code is
> ignoring it hehehe
>
>
> If we're going to change PAPR - and I believe we should, there's a clear
> lack of proper support for SMP topologies - we're better make sure that whatever
> attribute/hcall we add there fixes it in a robust way for the long term.
>
>
> Thanks,
>
>
> DHB
>
>
> >
> > C.
> >
>
--
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 v3 16/41] KVM: PPC: Book3S HV P9: Move radix MMU switching instructions together
From: Alexey Kardashevskiy @ 2021-03-22 4:24 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210305150638.2675513-17-npiggin@gmail.com>
On 06/03/2021 02:06, Nicholas Piggin wrote:
> Switching the MMU from radix<->radix mode is tricky particularly as the
> MMU can remain enabled and requires a certain sequence of SPR updates.
> Move these together into their own functions.
>
> This also includes the radix TLB check / flush because it's tied in to
> MMU switching due to tlbiel getting LPID from LPIDR.
>
> (XXX: isync / hwsync synchronisation TBD)
Looks alright but what is this comment about? Is something missing or
just sub optimal?
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 55 +++++++++++++++++++++---------------
> 1 file changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index f1230f9d98ba..b9cae42b9cd5 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3449,12 +3449,38 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
> trace_kvmppc_run_core(vc, 1);
> }
>
> +static void switch_mmu_to_guest_radix(struct kvm *kvm, struct kvm_vcpu *vcpu, u64 lpcr)
> +{
> + struct kvmppc_vcore *vc = vcpu->arch.vcore;
> + struct kvm_nested_guest *nested = vcpu->arch.nested;
> + u32 lpid;
> +
> + lpid = nested ? nested->shadow_lpid : kvm->arch.lpid;
> +
> + mtspr(SPRN_LPID, lpid);
> + mtspr(SPRN_LPCR, lpcr);
> + mtspr(SPRN_PID, vcpu->arch.pid);
> + isync();
> +
> + /* TLBIEL must have LPIDR set, so set guest LPID before flushing. */
> + kvmppc_check_need_tlb_flush(kvm, vc->pcpu, nested);
> +}
> +
> +static void switch_mmu_to_host_radix(struct kvm *kvm, u32 pid)
> +{
> + mtspr(SPRN_PID, pid);
> + mtspr(SPRN_LPID, kvm->arch.host_lpid);
> + mtspr(SPRN_LPCR, kvm->arch.host_lpcr);
> + isync();
> +}
> +
> /*
> * Load up hypervisor-mode registers on P9.
> */
> static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> unsigned long lpcr)
> {
> + struct kvm *kvm = vcpu->kvm;
> struct kvmppc_vcore *vc = vcpu->arch.vcore;
> s64 hdec;
> u64 tb, purr, spurr;
> @@ -3477,12 +3503,12 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> * P8 and P9 suppress the HDEC exception when LPCR[HDICE] = 0,
> * so set HDICE before writing HDEC.
> */
> - mtspr(SPRN_LPCR, vcpu->kvm->arch.host_lpcr | LPCR_HDICE);
> + mtspr(SPRN_LPCR, kvm->arch.host_lpcr | LPCR_HDICE);
> isync();
>
> hdec = time_limit - mftb();
> if (hdec < 0) {
> - mtspr(SPRN_LPCR, vcpu->kvm->arch.host_lpcr);
> + mtspr(SPRN_LPCR, kvm->arch.host_lpcr);
> isync();
> return BOOK3S_INTERRUPT_HV_DECREMENTER;
> }
> @@ -3517,7 +3543,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> }
> mtspr(SPRN_CIABR, vcpu->arch.ciabr);
> mtspr(SPRN_IC, vcpu->arch.ic);
> - mtspr(SPRN_PID, vcpu->arch.pid);
>
> mtspr(SPRN_PSSCR, vcpu->arch.psscr | PSSCR_EC |
> (local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
> @@ -3531,8 +3556,7 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>
> mtspr(SPRN_AMOR, ~0UL);
>
> - mtspr(SPRN_LPCR, lpcr);
> - isync();
> + switch_mmu_to_guest_radix(kvm, vcpu, lpcr);
>
> kvmppc_xive_push_vcpu(vcpu);
>
> @@ -3571,7 +3595,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> mtspr(SPRN_DAWR1, host_dawr1);
> mtspr(SPRN_DAWRX1, host_dawrx1);
> }
> - mtspr(SPRN_PID, host_pidr);
>
> /*
> * Since this is radix, do a eieio; tlbsync; ptesync sequence in
> @@ -3586,9 +3609,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> if (cpu_has_feature(CPU_FTR_ARCH_31))
> asm volatile(PPC_CP_ABORT);
>
> - mtspr(SPRN_LPID, vcpu->kvm->arch.host_lpid); /* restore host LPID */
> - isync();
> -
> vc->dpdes = mfspr(SPRN_DPDES);
> vc->vtb = mfspr(SPRN_VTB);
> mtspr(SPRN_DPDES, 0);
> @@ -3605,7 +3625,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> }
>
> mtspr(SPRN_HDEC, 0x7fffffff);
> - mtspr(SPRN_LPCR, vcpu->kvm->arch.host_lpcr);
> +
> + switch_mmu_to_host_radix(kvm, host_pidr);
>
> return trap;
> }
> @@ -4138,7 +4159,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
> {
> struct kvm_run *run = vcpu->run;
> int trap, r, pcpu;
> - int srcu_idx, lpid;
> + int srcu_idx;
> struct kvmppc_vcore *vc;
> struct kvm *kvm = vcpu->kvm;
> struct kvm_nested_guest *nested = vcpu->arch.nested;
> @@ -4212,13 +4233,6 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
> vc->vcore_state = VCORE_RUNNING;
> trace_kvmppc_run_core(vc, 0);
>
> - if (cpu_has_feature(CPU_FTR_HVMODE)) {
The new location of mtspr(SPRN_LPID, lpid) does not check for
CPU_FTR_HVMODE anymore, is this going to work with HV KVM on pseries?
Thanks,
> - lpid = nested ? nested->shadow_lpid : kvm->arch.lpid;
> - mtspr(SPRN_LPID, lpid);
> - isync();
> - kvmppc_check_need_tlb_flush(kvm, pcpu, nested);
> - }
> -
> guest_enter_irqoff();
>
> srcu_idx = srcu_read_lock(&kvm->srcu);
> @@ -4237,11 +4251,6 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>
> srcu_read_unlock(&kvm->srcu, srcu_idx);
>
> - if (cpu_has_feature(CPU_FTR_HVMODE)) {
> - mtspr(SPRN_LPID, kvm->arch.host_lpid);
> - isync();
> - }
> -
> set_irq_happened(trap);
>
> kvmppc_set_host_core(pcpu);
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v3 17/41] KVM: PPC: Book3S HV P9: implement kvmppc_xive_pull_vcpu in C
From: Alexey Kardashevskiy @ 2021-03-22 5:05 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210305150638.2675513-18-npiggin@gmail.com>
On 06/03/2021 02:06, Nicholas Piggin wrote:
> This is more symmetric with kvmppc_xive_push_vcpu. The extra test in
> the asm will go away in a later change.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/kvm_ppc.h | 2 ++
> arch/powerpc/kvm/book3s_hv.c | 2 ++
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 5 ++++
> arch/powerpc/kvm/book3s_xive.c | 31 +++++++++++++++++++++++++
> 4 files changed, 40 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index 9531b1c1b190..73b1ca5a6471 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -672,6 +672,7 @@ extern int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval);
> extern int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
> int level, bool line_status);
> extern void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu);
> +extern void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu);
>
> static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
> {
> @@ -712,6 +713,7 @@ static inline int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval) { retur
> static inline int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
> int level, bool line_status) { return -ENODEV; }
> static inline void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu) { }
> +static inline void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu) { }
>
> static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
> { return 0; }
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index b9cae42b9cd5..b265522fc467 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3565,6 +3565,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>
> trap = __kvmhv_vcpu_entry_p9(vcpu);
>
> + kvmppc_xive_pull_vcpu(vcpu);
> +
> /* Advance host PURR/SPURR by the amount used by guest */
> purr = mfspr(SPRN_PURR);
> spurr = mfspr(SPRN_SPURR);
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 75405ef53238..c11597f815e4 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1442,6 +1442,11 @@ guest_exit_cont: /* r9 = vcpu, r12 = trap, r13 = paca */
> bl kvmhv_accumulate_time
> #endif
> #ifdef CONFIG_KVM_XICS
> + /* If we came in through the P9 short path, xive pull is done in C */
> + lwz r0, STACK_SLOT_SHORT_PATH(r1)
> + cmpwi r0, 0
> + bne 1f
> +
> /* We are exiting, pull the VP from the XIVE */
> lbz r0, VCPU_XIVE_PUSHED(r9)
> cmpwi cr0, r0, 0
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index e7219b6f5f9a..52cdb9e2660a 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -127,6 +127,37 @@ void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu)
> }
> EXPORT_SYMBOL_GPL(kvmppc_xive_push_vcpu);
>
> +/*
> + * Pull a vcpu's context from the XIVE on guest exit.
> + * This assumes we are in virtual mode (MMU on)
> + */
> +void kvmppc_xive_pull_vcpu(struct kvm_vcpu *vcpu)
> +{
> + void __iomem *tima = local_paca->kvm_hstate.xive_tima_virt;
> +
> + if (!vcpu->arch.xive_pushed)
> + return;
> +
> + /*
> + * Sould not have been pushed if there is no tima
s/Sould/Should/
Otherwise good
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> + */
> + if (WARN_ON(!tima))
> + return;
> +
> + eieio();
> + /* First load to pull the context, we ignore the value */
> + __raw_readl(tima + TM_SPC_PULL_OS_CTX);
> + /* Second load to recover the context state (Words 0 and 1) */
> + vcpu->arch.xive_saved_state.w01 = __raw_readq(tima + TM_QW1_OS);
> +
> + /* Fixup some of the state for the next load */
> + vcpu->arch.xive_saved_state.lsmfb = 0;
> + vcpu->arch.xive_saved_state.ack = 0xff;
> + vcpu->arch.xive_pushed = 0;
> + eieio();
> +}
> +EXPORT_SYMBOL_GPL(kvmppc_xive_pull_vcpu);
> +
> /*
> * This is a simple trigger for a generic XIVE IRQ. This must
> * only be called for interrupts that support a trigger page
>
--
Alexey
^ permalink raw reply
* Re: [PATCH] powerpc/numa: Fix topology_physical_package_id() on pSeries
From: David Gibson @ 2021-03-22 5:19 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Nathan Lynch, Srikar Dronamraju, Daniel Henrique Barboza,
Greg Kurz, Vasant Hegde, linuxppc-dev
In-Reply-To: <20210316122437.3571843-1-clg@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]
On Tue, Mar 16, 2021 at 01:24:37PM +0100, Cédric Le Goater wrote:
> The topology-id of a CPU in a pSeries machine can be queried from
> sysfs but under PowerVM the value is always -1 even if NUMA nodes are
> defined. This is because the topology_physical_package_id() routine is
> using the "ibm,chip-id" property which is not specified in PAPR.
>
> Under QEMU/KVM, things are different because QEMU populates the CPU DT
> node with "ibm,chip-id" property. However, its value can be incorrect
> for uncommon SMT configuration and expose a bogus topology-id value in
> sysfs.
Incorrect in what sense? It's still indicating the (admittedly
arbitrary) qemu socket number, isn't it? And isn't that what it
should be?
> The use of cpu_to_node() guarantees to have a correct NUMA node id
> under both environments QEMU/KVM and PowerVM. This introduces a slight
> change for the QEMU/KVM guest, as the topology-id now matches the NUMA
> node and not the socket-id as before. Since QEMU also needs to remove
> "ibm,chip-id" property for the DT to follow the PAPR specs, both
> hypervisor environments will be in sync.
>
> On the PowerNV side, the NUMA node id returned by cpu_to_node() is
> computed from the "ibm,associativity" property of the CPU. Its value
> is built from the OPAL chip id and is equivalent to "ibm,chip-id".
Like mpe, I'm not convinced this is the right approach. "physical
packate" and NUMA node are not the same thing, except sometimes by
accident.
>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/include/asm/topology.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> index 3beeb030cd78..887c42a4e43d 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -123,7 +123,7 @@ static inline int cpu_to_coregroup_id(int cpu)
> #ifdef CONFIG_PPC64
> #include <asm/smp.h>
>
> -#define topology_physical_package_id(cpu) (cpu_to_chip_id(cpu))
> +#define topology_physical_package_id(cpu) (cpu_to_node(cpu))
>
> #define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))
> #define topology_core_cpumask(cpu) (cpu_cpu_mask(cpu))
--
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 v3 16/41] KVM: PPC: Book3S HV P9: Move radix MMU switching instructions together
From: Nicholas Piggin @ 2021-03-22 5:25 UTC (permalink / raw)
To: Alexey Kardashevskiy, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <47284fdd-51ef-5ba7-487b-dfb46ec2816e@ozlabs.ru>
Excerpts from Alexey Kardashevskiy's message of March 22, 2021 2:24 pm:
>
>
> On 06/03/2021 02:06, Nicholas Piggin wrote:
>> Switching the MMU from radix<->radix mode is tricky particularly as the
>> MMU can remain enabled and requires a certain sequence of SPR updates.
>> Move these together into their own functions.
>>
>> This also includes the radix TLB check / flush because it's tied in to
>> MMU switching due to tlbiel getting LPID from LPIDR.
>>
>> (XXX: isync / hwsync synchronisation TBD)
>
>
> Looks alright but what is this comment about? Is something missing or
> just sub optimal?
Ah, yeah the architecture says for example a CSI is required before +
after each, but the fine print is that you only need those to separate
between previous or subsequent accesses that may use those contexts
being switched from/to.
Then there is the question of CSI between the instructions so e.g., you
don't get the TLB prefetch bug if the mtPIDR could go out of order ahead
of the mtLPIDR, but those instructions are serialized so they wouldn't.
There's possibly a few clarifications coming to the architecture around
this as well.
I think things are relatively okay but probably need a bit more
commenting to justify where the isyncs() aren't. It's possible we might
be able to even remove the isyncs that are there.
Making a like-for-like conversion is a bit tricky because there are
possible context synchronising instructions between them already.
Maybe for the first series, I'll just put an isync between all of them,
and then a later patch can replace some of them with comments.
>
>
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
>
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 55 +++++++++++++++++++++---------------
>> 1 file changed, 32 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index f1230f9d98ba..b9cae42b9cd5 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -3449,12 +3449,38 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
>> trace_kvmppc_run_core(vc, 1);
>> }
>>
>> +static void switch_mmu_to_guest_radix(struct kvm *kvm, struct kvm_vcpu *vcpu, u64 lpcr)
>> +{
>> + struct kvmppc_vcore *vc = vcpu->arch.vcore;
>> + struct kvm_nested_guest *nested = vcpu->arch.nested;
>> + u32 lpid;
>> +
>> + lpid = nested ? nested->shadow_lpid : kvm->arch.lpid;
>> +
>> + mtspr(SPRN_LPID, lpid);
>> + mtspr(SPRN_LPCR, lpcr);
>> + mtspr(SPRN_PID, vcpu->arch.pid);
>> + isync();
>> +
>> + /* TLBIEL must have LPIDR set, so set guest LPID before flushing. */
>> + kvmppc_check_need_tlb_flush(kvm, vc->pcpu, nested);
>> +}
>> +
>> +static void switch_mmu_to_host_radix(struct kvm *kvm, u32 pid)
>> +{
>> + mtspr(SPRN_PID, pid);
>> + mtspr(SPRN_LPID, kvm->arch.host_lpid);
>> + mtspr(SPRN_LPCR, kvm->arch.host_lpcr);
>> + isync();
>> +}
>> +
>> /*
>> * Load up hypervisor-mode registers on P9.
>> */
>> static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>> unsigned long lpcr)
>> {
>> + struct kvm *kvm = vcpu->kvm;
>> struct kvmppc_vcore *vc = vcpu->arch.vcore;
>> s64 hdec;
>> u64 tb, purr, spurr;
>> @@ -3477,12 +3503,12 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>> * P8 and P9 suppress the HDEC exception when LPCR[HDICE] = 0,
>> * so set HDICE before writing HDEC.
>> */
>> - mtspr(SPRN_LPCR, vcpu->kvm->arch.host_lpcr | LPCR_HDICE);
>> + mtspr(SPRN_LPCR, kvm->arch.host_lpcr | LPCR_HDICE);
>> isync();
>>
>> hdec = time_limit - mftb();
>> if (hdec < 0) {
>> - mtspr(SPRN_LPCR, vcpu->kvm->arch.host_lpcr);
>> + mtspr(SPRN_LPCR, kvm->arch.host_lpcr);
>> isync();
>> return BOOK3S_INTERRUPT_HV_DECREMENTER;
>> }
>> @@ -3517,7 +3543,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>> }
>> mtspr(SPRN_CIABR, vcpu->arch.ciabr);
>> mtspr(SPRN_IC, vcpu->arch.ic);
>> - mtspr(SPRN_PID, vcpu->arch.pid);
>>
>> mtspr(SPRN_PSSCR, vcpu->arch.psscr | PSSCR_EC |
>> (local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
>> @@ -3531,8 +3556,7 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>>
>> mtspr(SPRN_AMOR, ~0UL);
>>
>> - mtspr(SPRN_LPCR, lpcr);
>> - isync();
>> + switch_mmu_to_guest_radix(kvm, vcpu, lpcr);
>>
>> kvmppc_xive_push_vcpu(vcpu);
>>
>> @@ -3571,7 +3595,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>> mtspr(SPRN_DAWR1, host_dawr1);
>> mtspr(SPRN_DAWRX1, host_dawrx1);
>> }
>> - mtspr(SPRN_PID, host_pidr);
>>
>> /*
>> * Since this is radix, do a eieio; tlbsync; ptesync sequence in
>> @@ -3586,9 +3609,6 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>> if (cpu_has_feature(CPU_FTR_ARCH_31))
>> asm volatile(PPC_CP_ABORT);
>>
>> - mtspr(SPRN_LPID, vcpu->kvm->arch.host_lpid); /* restore host LPID */
>> - isync();
>> -
>> vc->dpdes = mfspr(SPRN_DPDES);
>> vc->vtb = mfspr(SPRN_VTB);
>> mtspr(SPRN_DPDES, 0);
>> @@ -3605,7 +3625,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
>> }
>>
>> mtspr(SPRN_HDEC, 0x7fffffff);
>> - mtspr(SPRN_LPCR, vcpu->kvm->arch.host_lpcr);
>> +
>> + switch_mmu_to_host_radix(kvm, host_pidr);
>>
>> return trap;
>> }
>> @@ -4138,7 +4159,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>> {
>> struct kvm_run *run = vcpu->run;
>> int trap, r, pcpu;
>> - int srcu_idx, lpid;
>> + int srcu_idx;
>> struct kvmppc_vcore *vc;
>> struct kvm *kvm = vcpu->kvm;
>> struct kvm_nested_guest *nested = vcpu->arch.nested;
>> @@ -4212,13 +4233,6 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
>> vc->vcore_state = VCORE_RUNNING;
>> trace_kvmppc_run_core(vc, 0);
>>
>> - if (cpu_has_feature(CPU_FTR_HVMODE)) {
>
>
> The new location of mtspr(SPRN_LPID, lpid) does not check for
> CPU_FTR_HVMODE anymore, is this going to work with HV KVM on pseries?
Yes, these are moved to HVMODE specific code now.
Thanks,
Nick
^ 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