* [PATCH V2 5/6] mm: Drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK
From: Anshuman Khandual @ 2021-04-01 6:44 UTC (permalink / raw)
To: linux-mm, akpm
Cc: linux-s390, Rich Felker, Yoshinori Sato, Vasily Gorbik,
Anshuman Khandual, linux-sh, Heiko Carstens, x86, H. Peter Anvin,
linux-kernel, Christian Borntraeger, Ingo Molnar, Paul Mackerras,
Catalin Marinas, linux-ia64, Thomas Gleixner, linuxppc-dev,
Will Deacon, linux-arm-kernel
In-Reply-To: <1617259448-22529-1-git-send-email-anshuman.khandual@arm.com>
ARCH_ENABLE_SPLIT_PMD_PTLOCKS has duplicate definitions on platforms that
subscribe it. Drop these redundant definitions and instead just select it
on applicable platforms.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Catalin Marinas <catalin.marinas@arm.com> (arm64)
Acked-by: Heiko Carstens <hca@linux.ibm.com> (s390)
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/arm64/Kconfig | 4 +---
arch/powerpc/platforms/Kconfig.cputype | 5 +----
arch/s390/Kconfig | 4 +---
arch/x86/Kconfig | 5 +----
4 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index cd012af0a4b7..c1d505cad6d5 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -14,6 +14,7 @@ config ARM64
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_MEMORY_HOTPLUG
select ARCH_ENABLE_MEMORY_HOTREMOVE
+ select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_DEBUG_VIRTUAL
@@ -1062,9 +1063,6 @@ config HW_PERF_EVENTS
def_bool y
depends on ARM_PMU
-config ARCH_ENABLE_SPLIT_PMD_PTLOCK
- def_bool y if PGTABLE_LEVELS > 2
-
# Supported by clang >= 7.0
config CC_HAVE_SHADOW_CALL_STACK
def_bool $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18)
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 4465b71b2bff..be0e29f18dd4 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -97,6 +97,7 @@ config PPC_BOOK3S_64
select PPC_HAVE_PMU_SUPPORT
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
+ select ARCH_ENABLE_PMD_SPLIT_PTLOCK
select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
select ARCH_SUPPORTS_HUGETLBFS
select ARCH_SUPPORTS_NUMA_BALANCING
@@ -356,10 +357,6 @@ config SPE
If in doubt, say Y here.
-config ARCH_ENABLE_SPLIT_PMD_PTLOCK
- def_bool y
- depends on PPC_BOOK3S_64
-
config PPC_RADIX_MMU
bool "Radix MMU Support"
depends on PPC_BOOK3S_64
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index f8b356550daa..d72989591223 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -62,6 +62,7 @@ config S390
select ARCH_BINFMT_ELF_STATE
select ARCH_ENABLE_MEMORY_HOTPLUG if SPARSEMEM
select ARCH_ENABLE_MEMORY_HOTREMOVE
+ select ARCH_ENABLE_SPLIT_PMD_PTLOCK
select ARCH_HAS_DEBUG_VM_PGTABLE
select ARCH_HAS_DEBUG_WX
select ARCH_HAS_DEVMEM_IS_ALLOWED
@@ -628,9 +629,6 @@ config ARCH_SPARSEMEM_ENABLE
config ARCH_SPARSEMEM_DEFAULT
def_bool y
-config ARCH_ENABLE_SPLIT_PMD_PTLOCK
- def_bool y
-
config MAX_PHYSMEM_BITS
int "Maximum size of supported physical memory in bits (42-53)"
range 42 53
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 10702ef1eb57..5dd70c798167 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -63,6 +63,7 @@ config X86
select ARCH_ENABLE_HUGEPAGE_MIGRATION if x86_64 && HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_MEMORY_HOTPLUG if X86_64 || (X86_32 && HIGHMEM)
select ARCH_ENABLE_MEMORY_HOTREMOVE if MEMORY_HOTPLUG
+ select ARCH_ENABLE_SPLIT_PMD_PTLOCK if X86_64 || X86_PAE
select ARCH_ENABLE_THP_MIGRATION if x86_64 && TRANSPARENT_HUGEPAGE
select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
select ARCH_HAS_CACHE_LINE_SIZE
@@ -2431,10 +2432,6 @@ config USE_PERCPU_NUMA_NODE_ID
def_bool y
depends on NUMA
-config ARCH_ENABLE_SPLIT_PMD_PTLOCK
- def_bool y
- depends on X86_64 || X86_PAE
-
menu "Power management and ACPI options"
config ARCH_HIBERNATION_HEADER
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] powerpc/64s: power4 nap fixup in C
From: Nicholas Piggin @ 2021-04-01 7:36 UTC (permalink / raw)
To: Michael Ellerman, Andreas Schwab; +Cc: linuxppc-dev
In-Reply-To: <87r1jyj5e1.fsf@igel.home>
Excerpts from Andreas Schwab's message of March 30, 2021 2:23 am:
> On Mär 29 2021, Andreas Schwab wrote:
>
>> On Mär 29 2021, Michael Ellerman wrote:
>>
>>> Nicholas Piggin <npiggin@gmail.com> writes:
>>>> There is no need for this to be in asm, use the new intrrupt entry wrapper.
>>>>
>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>>> ---
>>>> Hopefully this works on a real G5 now, but I couldn't reproduce the
>>>> problem with QEMU.
>>>
>>> It still prevents my G5 from booting.
>>
>> I see differing failures. What's common is that there is a pause of
>> about 60 seconds before the crash occurs. It looks like the crash
>> occurs in power4_idle_nap+0x30/0x34. Unfortuately, the BootX console is
>> too small to see enough.
>
> I was now able to see the messages on the VGA console, and the problem
> is actually that the cpus are starting to stall.
This is strange, I haven't been able to figure out what is wrong.
I've been looking at QEMU code and now I'll have to try find a POWER4/5
or PPC970 manual to see what exactly this MSR[POW] thing does and make
sure QEMU matches it.
Progress might be slow. Worst case I guess we revert if it can't
be fixed before next merge window.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2] powerpc/traps: Enhance readability for trap types
From: Nicholas Piggin @ 2021-04-01 8:01 UTC (permalink / raw)
To: Michael Ellerman, Segher Boessenkool
Cc: ravi.bangoria, aik, peterx, oleg, paulus, kan.liang, leobras.c,
mikey, maddy, aneesh.kumar, haren, peterz, kjain, msuchanek,
pmladek, john.ogness, alistair, kvm-ppc, jniethe5, atrajeev,
Xiongwei Song, Xiongwei Song, linux-kernel, akpm, linuxppc-dev,
rppt
In-Reply-To: <87im5620f3.fsf@mpe.ellerman.id.au>
Excerpts from Michael Ellerman's message of April 1, 2021 12:39 pm:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
>> On Wed, Mar 31, 2021 at 08:58:17PM +1100, Michael Ellerman wrote:
>>> So perhaps:
>>>
>>> EXC_SYSTEM_RESET
>>> EXC_MACHINE_CHECK
>>> EXC_DATA_STORAGE
>>> EXC_DATA_SEGMENT
>>> EXC_INST_STORAGE
>>> EXC_INST_SEGMENT
>>> EXC_EXTERNAL_INTERRUPT
>>> EXC_ALIGNMENT
>>> EXC_PROGRAM_CHECK
>>> EXC_FP_UNAVAILABLE
>>> EXC_DECREMENTER
>>> EXC_HV_DECREMENTER
>>> EXC_SYSTEM_CALL
>>> EXC_HV_DATA_STORAGE
>>> EXC_PERF_MONITOR
>>
>> These are interrupt (vectors), not exceptions. It doesn't matter all
>> that much, but confusing things more isn't useful either! There can be
>> multiple exceptions that all can trigger the same interrupt.
>
> Yeah I know, but I think that ship has already sailed as far as the
> naming we have in the kernel.
It has, but there are also several other ships also sailing in different
directions. It could be worse though, at least they are not sideways in
the Suez.
> We have over 250 uses of "exc", and several files called "exception"
> something.
>
> Using "interrupt" can also be confusing because Linux uses that to mean
> "external interrupt".
>
> But I dunno, maybe INT or VEC is clearer? .. or TRAP :)
We actually already have defines that follow Segher's suggestion, it's
just that they're hidden away in a KVM header.
#define BOOK3S_INTERRUPT_SYSTEM_RESET 0x100
#define BOOK3S_INTERRUPT_MACHINE_CHECK 0x200
#define BOOK3S_INTERRUPT_DATA_STORAGE 0x300
#define BOOK3S_INTERRUPT_DATA_SEGMENT 0x380
#define BOOK3S_INTERRUPT_INST_STORAGE 0x400
#define BOOK3S_INTERRUPT_INST_SEGMENT 0x480
#define BOOK3S_INTERRUPT_EXTERNAL 0x500
#define BOOK3S_INTERRUPT_EXTERNAL_HV 0x502
#define BOOK3S_INTERRUPT_ALIGNMENT 0x600
It would take just a small amount of work to move these to general
powerpc header, add #ifdefs for Book E/S where the numbers differ,
and remove the BOOK3S_ prefix.
I don't mind INTERRUPT_ but INT_ would be okay too. VEC_ actually
doesn't match what Book E does (which is some weirdness to map some
of them to match Book S but not all, arguably we should clean that
up too and just use vector numbers consistently, but the INTERRUPT_
prefix would still be valid if we did that).
BookE KVM entry will still continue to use a different convention
there so I would leave all those KVM defines in place for now, we
might do another pass on them later.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 0/9] powerpc/xive: Map one IPI interrupt per node
From: Greg Kurz @ 2021-04-01 8:04 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <20210331144514.892250-1-clg@kaod.org>
On Wed, 31 Mar 2021 16:45:05 +0200
Cédric Le Goater <clg@kaod.org> wrote:
>
> Hello,
>
> ipistorm [*] can be used to benchmark the raw interrupt rate of an
> interrupt controller by measuring the number of IPIs a system can
> sustain. When applied to the XIVE interrupt controller of POWER9 and
> POWER10 systems, a significant drop of the interrupt rate can be
> observed when crossing the second node boundary.
>
> This is due to the fact that a single IPI interrupt is used for all
> CPUs of the system. The structure is shared and the cache line updates
> impact greatly the traffic between nodes and the overall IPI
> performance.
>
> As a workaround, the impact can be reduced by deactivating the IRQ
> lockup detector ("noirqdebug") which does a lot of accounting in the
> Linux IRQ descriptor structure and is responsible for most of the
> performance penalty.
>
> As a fix, this proposal allocates an IPI interrupt per node, to be
> shared by all CPUs of that node. It solves the scaling issue, the IRQ
> lockup detector still has an impact but the XIVE interrupt rate scales
> linearly. It also improves the "noirqdebug" case as showed in the
> tables below.
>
As explained by David and others, NUMA nodes happen to match sockets
with current POWER CPUs but these are really different concepts. NUMA
is about CPU memory accesses latency, while in the case of XIVE you
really need to identify a XIVE chip localized in a given socket.
PAPR doesn't know about sockets, only cores. In other words, a PAPR
compliant guest sees all vCPUs like they all sit in a single socket.
Same for the XIVE. Trying to introduce a concept of socket, either
by hijacking OPAL's ibm,chip-id or NUMA node ids, is a kind of
spec violation in this context. If the user cares for locality of
the vCPUs and XIVE on the same socket, then it should bind vCPU
threads to host CPUs from the same socket in the first place.
Isn't this enough to solve the performance issues this series
want to fix, without the need for virtual socket ids ?
> * P9 DD2.2 - 2s * 64 threads
>
> "noirqdebug"
> Mint/s Mint/s
> chips cpus IPI/sys IPI/chip IPI/chip IPI/sys
> --------------------------------------------------------------
> 1 0-15 4.984023 4.875405 4.996536 5.048892
> 0-31 10.879164 10.544040 10.757632 11.037859
> 0-47 15.345301 14.688764 14.926520 15.310053
> 0-63 17.064907 17.066812 17.613416 17.874511
> 2 0-79 11.768764 21.650749 22.689120 22.566508
> 0-95 10.616812 26.878789 28.434703 28.320324
> 0-111 10.151693 31.397803 31.771773 32.388122
> 0-127 9.948502 33.139336 34.875716 35.224548
>
>
> * P10 DD1 - 4s (not homogeneous) 352 threads
>
> "noirqdebug"
> Mint/s Mint/s
> chips cpus IPI/sys IPI/chip IPI/chip IPI/sys
> --------------------------------------------------------------
> 1 0-15 2.409402 2.364108 2.383303 2.395091
> 0-31 6.028325 6.046075 6.089999 6.073750
> 0-47 8.655178 8.644531 8.712830 8.724702
> 0-63 11.629652 11.735953 12.088203 12.055979
> 0-79 14.392321 14.729959 14.986701 14.973073
> 0-95 12.604158 13.004034 17.528748 17.568095
> 2 0-111 9.767753 13.719831 19.968606 20.024218
> 0-127 6.744566 16.418854 22.898066 22.995110
> 0-143 6.005699 19.174421 25.425622 25.417541
> 0-159 5.649719 21.938836 27.952662 28.059603
> 0-175 5.441410 24.109484 31.133915 31.127996
> 3 0-191 5.318341 24.405322 33.999221 33.775354
> 0-207 5.191382 26.449769 36.050161 35.867307
> 0-223 5.102790 29.356943 39.544135 39.508169
> 0-239 5.035295 31.933051 42.135075 42.071975
> 0-255 4.969209 34.477367 44.655395 44.757074
> 4 0-271 4.907652 35.887016 47.080545 47.318537
> 0-287 4.839581 38.076137 50.464307 50.636219
> 0-303 4.786031 40.881319 53.478684 53.310759
> 0-319 4.743750 43.448424 56.388102 55.973969
> 0-335 4.709936 45.623532 59.400930 58.926857
> 0-351 4.681413 45.646151 62.035804 61.830057
>
> [*] https://github.com/antonblanchard/ipistorm
>
> Thanks,
>
> C.
>
> Changes in v3:
>
> - improved commit log for the misuse of "ibm,chip-id"
> - better error handling of xive_request_ipi()
> - use of a fwnode_handle to name the new domain
> - increased IPI name length
> - use of early_cpu_to_node() for hotplugged CPUs
> - filter CPU-less nodes
>
> Changes in v2:
>
> - extra simplification on xmon
> - fixes on issues reported by the kernel test robot
>
> Cédric Le Goater (9):
> powerpc/xive: Use cpu_to_node() instead of "ibm,chip-id" property
> powerpc/xive: Introduce an IPI interrupt domain
> powerpc/xive: Remove useless check on XIVE_IPI_HW_IRQ
> powerpc/xive: Simplify xive_core_debug_show()
> powerpc/xive: Drop check on irq_data in xive_core_debug_show()
> powerpc/xive: Simplify the dump of XIVE interrupts under xmon
> powerpc/xive: Fix xmon command "dxi"
> powerpc/xive: Map one IPI interrupt per node
> powerpc/xive: Modernize XIVE-IPI domain with an 'alloc' handler
>
> arch/powerpc/include/asm/xive.h | 1 +
> arch/powerpc/sysdev/xive/xive-internal.h | 2 -
> arch/powerpc/sysdev/xive/common.c | 211 +++++++++++++++--------
> arch/powerpc/xmon/xmon.c | 28 +--
> 4 files changed, 139 insertions(+), 103 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v3 0/9] powerpc/xive: Map one IPI interrupt per node
From: Cédric Le Goater @ 2021-04-01 8:42 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Greg Kurz, David Gibson
In-Reply-To: <20210331144514.892250-1-clg@kaod.org>
Hello,
On 3/31/21 4:45 PM, Cédric Le Goater wrote:
>
> Hello,
>
> ipistorm [*] can be used to benchmark the raw interrupt rate of an
> interrupt controller by measuring the number of IPIs a system can
> sustain. When applied to the XIVE interrupt controller of POWER9 and
> POWER10 systems, a significant drop of the interrupt rate can be
> observed when crossing the second node boundary.
>
> This is due to the fact that a single IPI interrupt is used for all
> CPUs of the system. The structure is shared and the cache line updates
> impact greatly the traffic between nodes and the overall IPI
> performance.
>
> As a workaround, the impact can be reduced by deactivating the IRQ
> lockup detector ("noirqdebug") which does a lot of accounting in the
> Linux IRQ descriptor structure and is responsible for most of the
> performance penalty.
>
> As a fix, this proposal allocates an IPI interrupt per node, to be
> shared by all CPUs of that node. It solves the scaling issue, the IRQ
> lockup detector still has an impact but the XIVE interrupt rate scales
> linearly. It also improves the "noirqdebug" case as showed in the
> tables below. Hello,
From the comments, I received on different email threads. It seems
I am doing some wrong assumption on the code and concepts. We canpostpone this patchset. It's an optimization and there are some
more cleanups that can be done before.
Thanks for the time and the shared expertise,
C.
>
> * P9 DD2.2 - 2s * 64 threads
>
> "noirqdebug"
> Mint/s Mint/s
> chips cpus IPI/sys IPI/chip IPI/chip IPI/sys
> --------------------------------------------------------------
> 1 0-15 4.984023 4.875405 4.996536 5.048892
> 0-31 10.879164 10.544040 10.757632 11.037859
> 0-47 15.345301 14.688764 14.926520 15.310053
> 0-63 17.064907 17.066812 17.613416 17.874511
> 2 0-79 11.768764 21.650749 22.689120 22.566508
> 0-95 10.616812 26.878789 28.434703 28.320324
> 0-111 10.151693 31.397803 31.771773 32.388122
> 0-127 9.948502 33.139336 34.875716 35.224548
>
>
> * P10 DD1 - 4s (not homogeneous) 352 threads
>
> "noirqdebug"
> Mint/s Mint/s
> chips cpus IPI/sys IPI/chip IPI/chip IPI/sys
> --------------------------------------------------------------
> 1 0-15 2.409402 2.364108 2.383303 2.395091
> 0-31 6.028325 6.046075 6.089999 6.073750
> 0-47 8.655178 8.644531 8.712830 8.724702
> 0-63 11.629652 11.735953 12.088203 12.055979
> 0-79 14.392321 14.729959 14.986701 14.973073
> 0-95 12.604158 13.004034 17.528748 17.568095
> 2 0-111 9.767753 13.719831 19.968606 20.024218
> 0-127 6.744566 16.418854 22.898066 22.995110
> 0-143 6.005699 19.174421 25.425622 25.417541
> 0-159 5.649719 21.938836 27.952662 28.059603
> 0-175 5.441410 24.109484 31.133915 31.127996
> 3 0-191 5.318341 24.405322 33.999221 33.775354
> 0-207 5.191382 26.449769 36.050161 35.867307
> 0-223 5.102790 29.356943 39.544135 39.508169
> 0-239 5.035295 31.933051 42.135075 42.071975
> 0-255 4.969209 34.477367 44.655395 44.757074
> 4 0-271 4.907652 35.887016 47.080545 47.318537
> 0-287 4.839581 38.076137 50.464307 50.636219
> 0-303 4.786031 40.881319 53.478684 53.310759
> 0-319 4.743750 43.448424 56.388102 55.973969
> 0-335 4.709936 45.623532 59.400930 58.926857
> 0-351 4.681413 45.646151 62.035804 61.830057
>
> [*] https://github.com/antonblanchard/ipistorm
>
> Thanks,
>
> C.
>
> Changes in v3:
>
> - improved commit log for the misuse of "ibm,chip-id"
> - better error handling of xive_request_ipi()
> - use of a fwnode_handle to name the new domain
> - increased IPI name length
> - use of early_cpu_to_node() for hotplugged CPUs
> - filter CPU-less nodes
>
> Changes in v2:
>
> - extra simplification on xmon
> - fixes on issues reported by the kernel test robot
>
> Cédric Le Goater (9):
> powerpc/xive: Use cpu_to_node() instead of "ibm,chip-id" property
> powerpc/xive: Introduce an IPI interrupt domain
> powerpc/xive: Remove useless check on XIVE_IPI_HW_IRQ
> powerpc/xive: Simplify xive_core_debug_show()
> powerpc/xive: Drop check on irq_data in xive_core_debug_show()
> powerpc/xive: Simplify the dump of XIVE interrupts under xmon
> powerpc/xive: Fix xmon command "dxi"
> powerpc/xive: Map one IPI interrupt per node
> powerpc/xive: Modernize XIVE-IPI domain with an 'alloc' handler
>
> arch/powerpc/include/asm/xive.h | 1 +
> arch/powerpc/sysdev/xive/xive-internal.h | 2 -
> arch/powerpc/sysdev/xive/common.c | 211 +++++++++++++++--------
> arch/powerpc/xmon/xmon.c | 28 +--
> 4 files changed, 139 insertions(+), 103 deletions(-)
>
^ permalink raw reply
* [PATCH V2 RESEND] mm/memtest: Add ARCH_USE_MEMTEST
From: Anshuman Khandual @ 2021-04-01 9:26 UTC (permalink / raw)
To: linux-mm
Cc: Chris Zankel, Thomas Bogendoerfer, Anshuman Khandual,
linux-xtensa, linuxppc-dev, linux-kernel, Russell King,
linux-mips, Max Filippov, Ingo Molnar, Paul Mackerras,
Catalin Marinas, Thomas Gleixner, Will Deacon, linux-arm-kernel
early_memtest() does not get called from all architectures. Hence enabling
CONFIG_MEMTEST and providing a valid memtest=[1..N] kernel command line
option might not trigger the memory pattern tests as would be expected in
normal circumstances. This situation is misleading.
The change here prevents the above mentioned problem after introducing a
new config option ARCH_USE_MEMTEST that should be subscribed on platforms
that call early_memtest(), in order to enable the config CONFIG_MEMTEST.
Conversely CONFIG_MEMTEST cannot be enabled on platforms where it would
not be tested anyway.
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Catalin Marinas <catalin.marinas@arm.com> (arm64)
Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This patch applies on v5.12-rc5 and has been tested on arm64 platform.
But it has been just build tested on all other platforms.
Changes in V2:
https://patchwork.kernel.org/project/linux-mm/patch/1614573126-7740-1-git-send-email-anshuman.khandual@arm.com/
- Added ARCH_USE_MEMTEST in the sorted alphabetical order on platforms
Changes in V1:
https://patchwork.kernel.org/project/linux-mm/patch/1612498242-31579-1-git-send-email-anshuman.khandual@arm.com/
arch/arm/Kconfig | 1 +
arch/arm64/Kconfig | 1 +
arch/mips/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/x86/Kconfig | 1 +
arch/xtensa/Kconfig | 1 +
lib/Kconfig.debug | 9 ++++++++-
7 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5da96f5df48f..49878877df88 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -33,6 +33,7 @@ config ARM
select ARCH_SUPPORTS_ATOMIC_RMW
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF
+ select ARCH_USE_MEMTEST
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
select ARCH_WANT_IPC_PARSE_VERSION
select ARCH_WANT_LD_ORPHAN_WARN
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index e4e1b6550115..63c380587a77 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -67,6 +67,7 @@ config ARM64
select ARCH_KEEP_MEMBLOCK
select ARCH_USE_CMPXCHG_LOCKREF
select ARCH_USE_GNU_PROPERTY
+ select ARCH_USE_MEMTEST
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_USE_SYM_ANNOTATIONS
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index d89efba3d8a4..93a4f502f962 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -14,6 +14,7 @@ config MIPS
select ARCH_SUPPORTS_UPROBES
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
+ select ARCH_USE_MEMTEST
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 386ae12d8523..3778ad17f56a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -149,6 +149,7 @@ config PPC
select ARCH_SUPPORTS_DEBUG_PAGEALLOC if PPC32 || PPC_BOOK3S_64
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
+ select ARCH_USE_MEMTEST
select ARCH_USE_QUEUED_RWLOCKS if PPC_QUEUED_SPINLOCKS
select ARCH_USE_QUEUED_SPINLOCKS if PPC_QUEUED_SPINLOCKS
select ARCH_WANT_IPC_PARSE_VERSION
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2792879d398e..2cb76fd5258e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -100,6 +100,7 @@ config X86
select ARCH_SUPPORTS_LTO_CLANG if X86_64
select ARCH_SUPPORTS_LTO_CLANG_THIN if X86_64
select ARCH_USE_BUILTIN_BSWAP
+ select ARCH_USE_MEMTEST
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_USE_SYM_ANNOTATIONS
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 9ad6b7b82707..524413aabbc4 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -7,6 +7,7 @@ config XTENSA
select ARCH_HAS_SYNC_DMA_FOR_CPU if MMU
select ARCH_HAS_SYNC_DMA_FOR_DEVICE if MMU
select ARCH_HAS_DMA_SET_UNCACHED if MMU
+ select ARCH_USE_MEMTEST
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_WANT_FRAME_POINTERS
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2779c29d9981..a3fd69e6f6af 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2515,11 +2515,18 @@ config TEST_FPU
endif # RUNTIME_TESTING_MENU
+config ARCH_USE_MEMTEST
+ bool
+ help
+ An architecture should select this when it uses early_memtest()
+ during boot process.
+
config MEMTEST
bool "Memtest"
+ depends on ARCH_USE_MEMTEST
help
This option adds a kernel parameter 'memtest', which allows memtest
- to be set.
+ to be set and executed.
memtest=0, mean disabled; -- default
memtest=1, mean do 1 test pattern;
...
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 05/18] iommu/fsl_pamu: remove support for multiple windows
From: Christoph Hellwig @ 2021-04-01 9:29 UTC (permalink / raw)
To: Will Deacon
Cc: freedreno, kvm, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, linux-arm-msm, virtualization, David Woodhouse,
Christoph Hellwig, linux-arm-kernel, Lu Baolu
In-Reply-To: <20210330122234.GE5908@willie-the-truck>
On Tue, Mar 30, 2021 at 01:22:34PM +0100, Will Deacon wrote:
> > * win_arr contains information of the configured
> > * windows for a domain. This is allocated only
> > * when the number of windows for the domain are
> > * set.
> > */
>
> The last part of this comment is now stale ^^
I've updated it, although the comment will go away entirely a little
bit later anyway.
^ permalink raw reply
* Re: [PATCH 06/18] iommu/fsl_pamu: remove ->domain_window_enable
From: Christoph Hellwig @ 2021-04-01 9:32 UTC (permalink / raw)
To: Will Deacon
Cc: freedreno, kvm, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, linux-arm-msm, virtualization, David Woodhouse,
Christoph Hellwig, linux-arm-kernel, Lu Baolu
In-Reply-To: <20210330124009.GF5908@willie-the-truck>
On Tue, Mar 30, 2021 at 01:40:09PM +0100, Will Deacon wrote:
> > + ret = pamu_config_ppaace(liodn, geom->aperture_start,
> > + geom->aperture_end - 1, ~(u32)0,
>
> You're passing 'geom->aperture_end - 1' as the size here, but the old code
> seemed to _add_ 1:
>
> > - win_size = (domain->geometry.aperture_end + 1) >> ilog2(1);
>
> here ^^ when calculating the exclusive upper bound. In other words, I think
> '1ULL << 36' used to get passed to pamu_config_ppaace(). Is that an
> intentional change?
No, I've fixed it up.
^ permalink raw reply
* Re: [PATCH v4 02/46] KVM: PPC: Book3S HV: Add a function to filter guest LPCR bits
From: Nicholas Piggin @ 2021-04-01 9:32 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <YGP1uXH5q72auwP7@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of March 31, 2021 2:08 pm:
> On Tue, Mar 23, 2021 at 11:02:21AM +1000, Nicholas Piggin wrote:
>> Guest LPCR depends on hardware type, and future changes will add
>> restrictions based on errata and guest MMU mode. Move this logic
>> to a common function and use it for the cases where the guest
>> wants to update its LPCR (or the LPCR of a nested guest).
>
> [snip]
>
>> @@ -4641,8 +4662,9 @@ void kvmppc_update_lpcr(struct kvm *kvm, unsigned long lpcr, unsigned long mask)
>> struct kvmppc_vcore *vc = kvm->arch.vcores[i];
>> if (!vc)
>> continue;
>> +
>> spin_lock(&vc->lock);
>> - vc->lpcr = (vc->lpcr & ~mask) | lpcr;
>> + vc->lpcr = kvmppc_filter_lpcr_hv(vc, (vc->lpcr & ~mask) | lpcr);
>
> This change seems unnecessary, since kvmppc_update_lpcr is called only
> to update MMU configuration bits, not as a result of any action by
> userspace or a nested hypervisor. It's also beyond the scope of what
> was mentioned in the commit message.
I didn't think it was outside the spirit of the patch, but yes
only the guest update LPCR case was enumerated. Would it be more
consistent to add it to the changelog and leave it in here or would
you prefer it left out until there is a real use?
The intention is a single location to add some of these things
(handwaving: say tlbie doesn't work on some chip and we want to
emulate it for old guests we could clear GTSE).
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 08/18] iommu/fsl_pamu: merge pamu_set_liodn and map_liodn
From: Christoph Hellwig @ 2021-04-01 9:34 UTC (permalink / raw)
To: Will Deacon
Cc: freedreno, kvm, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, linux-arm-msm, virtualization, David Woodhouse,
Christoph Hellwig, linux-arm-kernel, Lu Baolu
In-Reply-To: <20210330124651.GH5908@willie-the-truck>
On Tue, Mar 30, 2021 at 01:46:51PM +0100, Will Deacon wrote:
> > + ret = pamu_config_ppaace(liodn, geom->aperture_start,
> > + geom->aperture_end - 1, ~(u32)0,
> > + 0, dma_domain->snoop_id, dma_domain->stash_id,
> > + PAACE_AP_PERMS_QUERY | PAACE_AP_PERMS_UPDATE);
>
> There's more '+1' / '-1' confusion here with aperture_end which I'm not
> managing to follow. What am I missing?
You did not missing anything, I messed this up. Fixed.
^ permalink raw reply
* Re: [PATCH v3 0/9] powerpc/xive: Map one IPI interrupt per node
From: Cédric Le Goater @ 2021-04-01 9:18 UTC (permalink / raw)
To: Greg Kurz; +Cc: linuxppc-dev
In-Reply-To: <20210401100415.28eb9967@bahia.lan>
Hello,
On 4/1/21 10:04 AM, Greg Kurz wrote:
> On Wed, 31 Mar 2021 16:45:05 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
>
>>
>> Hello,
>>
>> ipistorm [*] can be used to benchmark the raw interrupt rate of an
>> interrupt controller by measuring the number of IPIs a system can
>> sustain. When applied to the XIVE interrupt controller of POWER9 and
>> POWER10 systems, a significant drop of the interrupt rate can be
>> observed when crossing the second node boundary.
>>
>> This is due to the fact that a single IPI interrupt is used for all
>> CPUs of the system. The structure is shared and the cache line updates
>> impact greatly the traffic between nodes and the overall IPI
>> performance.
>>
>> As a workaround, the impact can be reduced by deactivating the IRQ
>> lockup detector ("noirqdebug") which does a lot of accounting in the
>> Linux IRQ descriptor structure and is responsible for most of the
>> performance penalty.
>>
>> As a fix, this proposal allocates an IPI interrupt per node, to be
>> shared by all CPUs of that node. It solves the scaling issue, the IRQ
>> lockup detector still has an impact but the XIVE interrupt rate scales
>> linearly. It also improves the "noirqdebug" case as showed in the
>> tables below.
>>
>
> As explained by David and others, NUMA nodes happen to match sockets
> with current POWER CPUs but these are really different concepts. NUMA
> is about CPU memory accesses latency,
This is exactly our problem. we have cache issues because hw threads
on different chips are trying to access the same structure in memory.
It happens on virtual platforms and baremetal platforms. This is not
restricted to pseries.
> while in the case of XIVE you
> really need to identify a XIVE chip localized in a given socket.
>
> PAPR doesn't know about sockets, only cores. In other words, a PAPR
> compliant guest sees all vCPUs like they all sit in a single socket.
There are also NUMA nodes on PAPR.
> Same for the XIVE. Trying to introduce a concept of socket, either
> by hijacking OPAL's ibm,chip-id or NUMA node ids, is a kind of
> spec violation in this context. If the user cares for locality of
> the vCPUs and XIVE on the same socket, then it should bind vCPU
> threads to host CPUs from the same socket in the first place.
Yes. that's a must have of course. You need to reflect the real HW
topology in the guest or LPAR if you are after performance, or
restrict the virtual machine to be on a single socket/chip/node.
And this is not only a XIVE problem. XICS has the same problem with
a shared single IPI interrupt descriptor but XICS doesn't scale well
by design, so it doesn't show.
> Isn't this enough to solve the performance issues this series
> want to fix, without the need for virtual socket ids ?
what are virtual socket ids ? A new concept ?
Thanks,
C.
>
>> * P9 DD2.2 - 2s * 64 threads
>>
>> "noirqdebug"
>> Mint/s Mint/s
>> chips cpus IPI/sys IPI/chip IPI/chip IPI/sys
>> --------------------------------------------------------------
>> 1 0-15 4.984023 4.875405 4.996536 5.048892
>> 0-31 10.879164 10.544040 10.757632 11.037859
>> 0-47 15.345301 14.688764 14.926520 15.310053
>> 0-63 17.064907 17.066812 17.613416 17.874511
>> 2 0-79 11.768764 21.650749 22.689120 22.566508
>> 0-95 10.616812 26.878789 28.434703 28.320324
>> 0-111 10.151693 31.397803 31.771773 32.388122
>> 0-127 9.948502 33.139336 34.875716 35.224548
>>
>>
>> * P10 DD1 - 4s (not homogeneous) 352 threads
>>
>> "noirqdebug"
>> Mint/s Mint/s
>> chips cpus IPI/sys IPI/chip IPI/chip IPI/sys
>> --------------------------------------------------------------
>> 1 0-15 2.409402 2.364108 2.383303 2.395091
>> 0-31 6.028325 6.046075 6.089999 6.073750
>> 0-47 8.655178 8.644531 8.712830 8.724702
>> 0-63 11.629652 11.735953 12.088203 12.055979
>> 0-79 14.392321 14.729959 14.986701 14.973073
>> 0-95 12.604158 13.004034 17.528748 17.568095
>> 2 0-111 9.767753 13.719831 19.968606 20.024218
>> 0-127 6.744566 16.418854 22.898066 22.995110
>> 0-143 6.005699 19.174421 25.425622 25.417541
>> 0-159 5.649719 21.938836 27.952662 28.059603
>> 0-175 5.441410 24.109484 31.133915 31.127996
>> 3 0-191 5.318341 24.405322 33.999221 33.775354
>> 0-207 5.191382 26.449769 36.050161 35.867307
>> 0-223 5.102790 29.356943 39.544135 39.508169
>> 0-239 5.035295 31.933051 42.135075 42.071975
>> 0-255 4.969209 34.477367 44.655395 44.757074
>> 4 0-271 4.907652 35.887016 47.080545 47.318537
>> 0-287 4.839581 38.076137 50.464307 50.636219
>> 0-303 4.786031 40.881319 53.478684 53.310759
>> 0-319 4.743750 43.448424 56.388102 55.973969
>> 0-335 4.709936 45.623532 59.400930 58.926857
>> 0-351 4.681413 45.646151 62.035804 61.830057
>>
>> [*] https://github.com/antonblanchard/ipistorm
>>
>> Thanks,
>>
>> C.
>>
>> Changes in v3:
>>
>> - improved commit log for the misuse of "ibm,chip-id"
>> - better error handling of xive_request_ipi()
>> - use of a fwnode_handle to name the new domain
>> - increased IPI name length
>> - use of early_cpu_to_node() for hotplugged CPUs
>> - filter CPU-less nodes
>>
>> Changes in v2:
>>
>> - extra simplification on xmon
>> - fixes on issues reported by the kernel test robot
>>
>> Cédric Le Goater (9):
>> powerpc/xive: Use cpu_to_node() instead of "ibm,chip-id" property
>> powerpc/xive: Introduce an IPI interrupt domain
>> powerpc/xive: Remove useless check on XIVE_IPI_HW_IRQ
>> powerpc/xive: Simplify xive_core_debug_show()
>> powerpc/xive: Drop check on irq_data in xive_core_debug_show()
>> powerpc/xive: Simplify the dump of XIVE interrupts under xmon
>> powerpc/xive: Fix xmon command "dxi"
>> powerpc/xive: Map one IPI interrupt per node
>> powerpc/xive: Modernize XIVE-IPI domain with an 'alloc' handler
>>
>> arch/powerpc/include/asm/xive.h | 1 +
>> arch/powerpc/sysdev/xive/xive-internal.h | 2 -
>> arch/powerpc/sysdev/xive/common.c | 211 +++++++++++++++--------
>> arch/powerpc/xmon/xmon.c | 28 +--
>> 4 files changed, 139 insertions(+), 103 deletions(-)
>>
>
^ permalink raw reply
* Re: [PATCH 11/18] iommu/fsl_pamu: remove the snoop_id field
From: Christoph Hellwig @ 2021-04-01 9:36 UTC (permalink / raw)
To: Will Deacon
Cc: freedreno, kvm, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, linux-arm-msm, virtualization, David Woodhouse,
Christoph Hellwig, linux-arm-kernel, Lu Baolu
In-Reply-To: <20210330125816.GK5908@willie-the-truck>
On Tue, Mar 30, 2021 at 01:58:17PM +0100, Will Deacon wrote:
> pamu_config_ppaace() takes quite a few useless parameters at this stage,
> but anyway:
I'll see it it makes sense to throw in another patch at the end to cut
it down a bit more.
> Acked-by: Will Deacon <will@kernel.org>
>
> Do you know if this driver is actually useful? Once the complexity has been
> stripped back, the stubs and default values really stand out.
Yeah. No idea what the usefulness of this driver is. Bascially all it
seems to do is to setup a few registers to allow access to the whole
physical memory. But maybe that is required on this hardware to allow
for any DMA access?
^ permalink raw reply
* Re: [PATCH v4 04/46] KVM: PPC: Book3S HV: Prevent radix guests from setting LPCR[TC]
From: Nicholas Piggin @ 2021-04-01 9:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <YGP74QVmx5yyE8Rc@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of March 31, 2021 2:34 pm:
> On Tue, Mar 23, 2021 at 11:02:23AM +1000, Nicholas Piggin wrote:
>> This bit only applies to hash partitions.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 6 ++++++
>> arch/powerpc/kvm/book3s_hv_nested.c | 3 +--
>> 2 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index c5de7e3f22b6..1ffb0902e779 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -1645,6 +1645,12 @@ static int kvm_arch_vcpu_ioctl_set_sregs_hv(struct kvm_vcpu *vcpu,
>> */
>> unsigned long kvmppc_filter_lpcr_hv(struct kvmppc_vcore *vc, unsigned long lpcr)
>> {
>> + struct kvm *kvm = vc->kvm;
>> +
>> + /* LPCR_TC only applies to HPT guests */
>> + if (kvm_is_radix(kvm))
>> + lpcr &= ~LPCR_TC;
>
> I'm not sure I see any benefit from this, and it is a little extra
> complexity.
Principle of allowing a guest to mess with as little HV state as
possible (but not littler), which I think is a good one to follow.
>
>> /* On POWER8 and above, userspace can modify AIL */
>> if (!cpu_has_feature(CPU_FTR_ARCH_207S))
>> lpcr &= ~LPCR_AIL;
>> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
>> index f7b441b3eb17..851e3f527eb2 100644
>> --- a/arch/powerpc/kvm/book3s_hv_nested.c
>> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
>> @@ -140,8 +140,7 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
>> /*
>> * Don't let L1 change LPCR bits for the L2 except these:
>> */
>> - mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
>> - LPCR_LPES | LPCR_MER;
>> + mask = LPCR_DPFD | LPCR_ILE | LPCR_AIL | LPCR_LD | LPCR_LPES | LPCR_MER;
>
> Doesn't this make it completely impossible to set TC for any guest?
Argh, yes Fabiano pointed this out in an earlier rev and I didn't
fix this hunk after adding the filter bit. Thanks.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v4 09/46] powerpc/64s: remove KVM SKIP test from instruction breakpoint handler
From: Nicholas Piggin @ 2021-04-01 9:47 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Fabiano Rosas, linuxppc-dev, kvm-ppc, Daniel Axtens
In-Reply-To: <YGQAfCOAR5Hfj13u@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of March 31, 2021 2:54 pm:
> On Tue, Mar 23, 2021 at 11:02:28AM +1000, Nicholas Piggin wrote:
>> The code being executed in KVM_GUEST_MODE_SKIP is hypervisor code with
>> MSR[IR]=0, so the faults of concern are the d-side ones caused by access
>> to guest context by the hypervisor.
>>
>> Instruction breakpoint interrupts are not a concern here. It's unlikely
>> any good would come of causing breaks in this code, but skipping the
>> instruction that caused it won't help matters (e.g., skip the mtmsr that
>> sets MSR[DR]=0 or clears KVM_GUEST_MODE_SKIP).
>>
>> Reviewed-by: Daniel Axtens <dja@axtens.net>
>> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
> It might be worth noting in the commit message that the 0x1300
> interrupt was dropped from the architecture a long time ago and is not
> generated by P7, P8, P9 or P10.
Good background, I'll add that.
Thanks,
Nick
>
> Acked-by: Paul Mackerras <paulus@ozlabs.org>
>
^ permalink raw reply
* Re: [PATCH v4 11/46] KVM: PPC: Book3S HV: Ensure MSR[HV] is always clear in guest MSR
From: Nicholas Piggin @ 2021-04-01 9:49 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <YGQBdVntWnG/ewtj@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of March 31, 2021 2:58 pm:
> On Tue, Mar 23, 2021 at 11:02:30AM +1000, Nicholas Piggin wrote:
>> Rather than clear the HV bit from the MSR at guest entry, make it clear
>> that the hypervisor does not allow the guest to set the bit.
>>
>> The HV clear is kept in guest entry for now, but a future patch will
>> warn if it's not present.
>
> Will warn if it *is* present, surely?
Just making sure you were awake, definitely wasn't a copy-paste bug...
Thanks,
Nick
>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
> Acked-by: Paul Mackerras <paulus@ozlabs.org>
>
^ permalink raw reply
* Re: [PATCH v3 1/9] powerpc/xive: Use cpu_to_node() instead of "ibm,chip-id" property
From: Cédric Le Goater @ 2021-04-01 9:10 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Greg Kurz
In-Reply-To: <YGU0u4v2GVWBCrWw@yekko.fritz.box>
On 4/1/21 4:49 AM, David Gibson wrote:
> On Wed, Mar 31, 2021 at 04:45:06PM +0200, Cédric Le Goater wrote:
>> The 'chip_id' field of the XIVE CPU structure is used to choose a
>> target for a source located on the same chip when possible. The XIVE
>> driver queries the chip id value from the "ibm,chip-id" DT property
>> but this property is not available on all platforms. It was first
>> introduced on the PowerNV platform and later, under QEMU for pseries.
>> However, the property does not exist under PowerVM since it is not
>> specified in PAPR.
>>
>> cpu_to_node() is a better alternative. On the PowerNV platform, the
>> node id is computed from the "ibm,associativity" property of the CPU.
>> Its value is built in the OPAL firmware from the physical chip id and
>> is equivalent to "ibm,chip-id".
>
> Hrm... I mean, for powernv this is certainly correct, but seems to be
> relying on pretty specific specifics of the OPAL / chip behaviour,
> namely that the NUMA id == chip ID.
Yes. It seems so.
>> On pSeries, the hcall H_HOME_NODE_ASSOCIATIVITY returns the node id.
>
> AFAICT, the chip_id field is never actually used in the PAPR version
> of XIVE. The only access to the field outside native.c is in
> xive_pick_irq_target(), and it only looks at chip_id if src_chip is
> valid.
Yes.
> But src_chip is initialized to XIVE_INVALID_CHIP_ID in papr.c
Yes. The H_INT hcalls do no return any information on the source
location.
> So it would make more sense to me to also initialize chip_id to
> XIVE_INVALID_CHIP_ID for PAPR to make it clearer that it's not
> relevant.
yes. That would clarify that chip_id is only relevant on PowerVM/KVM.
We can drop this patch, it's not a requirement for patches 2-9, simply
a cleanup. I will move the chip_id assignment to a platform handler
in a other patch.
>> Also to be noted that under QEMU/KVM "ibm,chip-id" is badly calculated
>> with unusual SMT configuration. This leads to a bogus chip id value
>> being returned by of_get_ibm_chip_id().
>
> I *still* don't clearly understand what you think is bogus about the
> chip id value that qemu generates. It's clearly not a problem for
> XIVE, since PAPR XIVE never uses it.
I am getting confused by socket/node/chip concepts under PPC.
However, when looking at PHB and MSI, there is definitely a "node"
concept being used in the core IRQ layer for allocation and affinity.
We will need to clarify that when we introduce MSI domains.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH v4 13/46] KVM: PPC: Book3S 64: Move GUEST_MODE_SKIP test into KVM
From: Nicholas Piggin @ 2021-04-01 9:53 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Fabiano Rosas, linuxppc-dev, kvm-ppc, Daniel Axtens
In-Reply-To: <YGVbApPydgwAU8cP@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of April 1, 2021 3:32 pm:
> On Tue, Mar 23, 2021 at 11:02:32AM +1000, Nicholas Piggin wrote:
>> Move the GUEST_MODE_SKIP logic into KVM code. This is quite a KVM
>> internal detail that has no real need to be in common handlers.
>>
>> Also add a comment explaining why this thing exists.
>
> [snip]
>
>> diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
>> index 7a039ea78f15..a5412e24cc05 100644
>> --- a/arch/powerpc/kvm/book3s_64_entry.S
>> +++ b/arch/powerpc/kvm/book3s_64_entry.S
>> @@ -1,6 +1,7 @@
>> /* SPDX-License-Identifier: GPL-2.0-only */
>> #include <asm/asm-offsets.h>
>> #include <asm/cache.h>
>> +#include <asm/exception-64s.h>
>> #include <asm/kvm_asm.h>
>> #include <asm/kvm_book3s_asm.h>
>> #include <asm/ppc_asm.h>
>> @@ -20,9 +21,12 @@ kvmppc_interrupt:
>> * guest R12 saved in shadow VCPU SCRATCH0
>> * guest R13 saved in SPRN_SCRATCH0
>> */
>> -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>> std r9,HSTATE_SCRATCH2(r13)
>> lbz r9,HSTATE_IN_GUEST(r13)
>> + cmpwi r9,KVM_GUEST_MODE_SKIP
>> + 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
>> @@ -34,3 +38,48 @@ kvmppc_interrupt:
>> #else
>> b kvmppc_interrupt_pr
>> #endif
>
> It's a bit hard to see without more context, but I think that in the
> PR-only case (CONFIG_KVM_BOOK3S_HV_POSSIBLE undefined), this will
> corrupt R9. You need to restore R9 before the unconditional branch to
> kvmppc_interrupt_pr. (I realize this code gets modified further, but
> I'd rather not break bisection.)
Very good catch, thanks.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Christoph Hellwig @ 2021-04-01 9:59 UTC (permalink / raw)
To: Robin Murphy
Cc: kvm, Will Deacon, linux-arm-msm, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, Christoph Hellwig, virtualization, freedreno,
David Woodhouse, linux-arm-kernel
In-Reply-To: <81dd27fe-28ee-c800-fe5d-aaa64cb93513@arm.com>
For now I'll just pass the iommu_domain to iommu_get_dma_strict,
so that we can check for it. We can do additional cleanups on top
of that later.
^ permalink raw reply
* Re: [PATCH v4 15/46] KVM: PPC: Book3S 64: Move hcall early register setup to KVM
From: Nicholas Piggin @ 2021-04-01 10:03 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <YGVdFrsEtD88oB90@thinks.paulus.ozlabs.org>
Excerpts from Paul Mackerras's message of April 1, 2021 3:41 pm:
> On Tue, Mar 23, 2021 at 11:02:34AM +1000, Nicholas Piggin wrote:
>> System calls / hcalls have a different calling convention than
>> other interrupts, so there is code in the KVMTEST to massage these
>> into the same form as other interrupt handlers.
>>
>> Move this work into the KVM hcall handler. This means teaching KVM
>> a little more about the low level interrupt handler setup, PACA save
>> areas, etc., although that's not obviously worse than the current
>> approach of coming up with an entirely different interrupt register
>> / save convention.
>
> [snip]
>
>> @@ -1964,29 +1948,8 @@ EXC_VIRT_END(system_call, 0x4c00, 0x100)
>>
>> #ifdef CONFIG_KVM_BOOK3S_64_HANDLER
>> TRAMP_REAL_BEGIN(system_call_kvm)
>> - /*
>> - * This is a hcall, so register convention is as above, with these
>> - * differences:
>
> I haven't checked all the code changes in detail yet, but this comment
> at least is slightly misleading, since under PR KVM, system calls (to
> the guest kernel) and hypercalls both come through this path.
Yeah good point, I'll update the comment at its destination.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v4 24/46] KVM: PPC: Book3S HV P9: Use large decrementer for HDEC
From: Nicholas Piggin @ 2021-04-01 10:14 UTC (permalink / raw)
To: Alexey Kardashevskiy, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <11ba0a43-a64a-ca06-581c-e8b7dc97b1d7@ozlabs.ru>
Excerpts from Alexey Kardashevskiy's message of March 26, 2021 12:05 pm:
>
>
> On 23/03/2021 12:02, Nicholas Piggin wrote:
>> On processors that don't suppress the HDEC exceptions when LPCR[HDICE]=0,
>> this could help reduce needless guest exits due to leftover exceptions on
>> entering the guest.
>>
>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>
>
> ERROR: modpost: "decrementer_max" [arch/powerpc/kvm/kvm-hv.ko] undefined!
>
>
> need this:
>
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -89,6 +89,7 @@ static struct clocksource clocksource_timebase = {
>
> #define DECREMENTER_DEFAULT_MAX 0x7FFFFFFF
> u64 decrementer_max = DECREMENTER_DEFAULT_MAX;
> +EXPORT_SYMBOL_GPL(decrementer_max);
Added.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] soc/fsl: qbman: fix conflicting alignment attributes
From: Arnd Bergmann @ 2021-04-01 10:23 UTC (permalink / raw)
To: Roy Pledge, Claudiu Manoil, Scott Wood, Arnd Bergmann, Li Yang
Cc: Arnd Bergmann, linuxppc-dev, Sebastian Andrzej Siewior,
YueHaibing, linux-kernel, soc, Jakub Kicinski, Lee Jones,
linux-arm-kernel
In-Reply-To: <20210323131530.2619900-1-arnd@kernel.org>
From: Arnd Bergmann <arnd@arndb.de>
On Tue, 23 Mar 2021 14:15:23 +0100, Arnd Bergmann wrote:
> When building with W=1, gcc points out that the __packed attribute
> on struct qm_eqcr_entry conflicts with the 8-byte alignment
> attribute on struct qm_fd inside it:
>
> drivers/soc/fsl/qbman/qman.c:189:1: error: alignment 1 of 'struct qm_eqcr_entry' is less than 8 [-Werror=packed-not-aligned]
>
> I assume that the alignment attribute is the correct one, and
> that qm_eqcr_entry cannot actually be unaligned in memory,
> so add the same alignment on the outer struct.
Applied to arm/fixes
[1/1] soc/fsl: qbman: fix conflicting alignment attributes
commit: b9ab2619047edc1c6c07dc68963d84a5979f0abe
Arnd
^ permalink raw reply
* Re: [PATCH v4 29/46] KVM: PPC: Book3S HV P9: Implement the rest of the P9 path in C
From: Nicholas Piggin @ 2021-04-01 10:35 UTC (permalink / raw)
To: Alexey Kardashevskiy, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <56dc4f3f-789d-0bbe-1b1f-508dbdfae487@ozlabs.ru>
Excerpts from Alexey Kardashevskiy's message of April 1, 2021 3:30 pm:
>
>
> On 3/23/21 12:02 PM, Nicholas Piggin wrote:
>> Almost all logic is moved to C, by introducing a new in_guest mode that
>> selects and branches very early in the interrupt handler to the P9 exit
>> code.
[...]
>> +/*
>> + * kvmppc_p9_exit_hcall and kvmppc_p9_exit_interrupt are branched to from
>> + * above if the interrupt was taken for a guest that was entered via
>> + * kvmppc_p9_enter_guest().
>> + *
>> + * This code recovers the host stack and vcpu pointer, saves all GPRs and
>> + * CR, LR, CTR, XER as well as guest MSR and NIA into the VCPU, then re-
>> + * establishes the host stack and registers to return from the
>> + * kvmppc_p9_enter_guest() function.
>
> What does "this code" refer to? If it is the asm below, then it does not
> save CTR, it is in the c code. Otherwise it is confusing (to me) :)
Yes you're right, CTR is saved in C.
>> + */
>> +.balign IFETCH_ALIGN_BYTES
>> +kvmppc_p9_exit_hcall:
>> + mfspr r11,SPRN_SRR0
>> + mfspr r12,SPRN_SRR1
>> + li r10,0xc00
>> + std r10,HSTATE_SCRATCH0(r13)
>> +
>> +.balign IFETCH_ALIGN_BYTES
>> +kvmppc_p9_exit_interrupt:
[...]
>> +static inline void slb_invalidate(unsigned int ih)
>> +{
>> + asm volatile("slbia %0" :: "i"(ih));
>> +}
>
> This one is not used.
It gets used in a later patch, I guess I should move it there.
[...]
>> +int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu)
>> +{
>> + u64 *exsave;
>> + unsigned long msr = mfmsr();
>> + int trap;
>> +
>> + start_timing(vcpu, &vcpu->arch.rm_entry);
>> +
>> + vcpu->arch.ceded = 0;
>> +
>> + WARN_ON_ONCE(vcpu->arch.shregs.msr & MSR_HV);
>> + WARN_ON_ONCE(!(vcpu->arch.shregs.msr & MSR_ME));
>> +
>> + mtspr(SPRN_HSRR0, vcpu->arch.regs.nip);
>> + mtspr(SPRN_HSRR1, (vcpu->arch.shregs.msr & ~MSR_HV) | MSR_ME);
>> +
>> + /*
>> + * On POWER9 DD2.1 and below, sometimes on a Hypervisor Data Storage
>> + * Interrupt (HDSI) the HDSISR is not be updated at all.
>> + *
>> + * To work around this we put a canary value into the HDSISR before
>> + * returning to a guest and then check for this canary when we take a
>> + * HDSI. If we find the canary on a HDSI, we know the hardware didn't
>> + * update the HDSISR. In this case we return to the guest to retake the
>> + * HDSI which should correctly update the HDSISR the second time HDSI
>> + * entry.
>> + *
>> + * Just do this on all p9 processors for now.
>> + */
>> + mtspr(SPRN_HDSISR, HDSISR_CANARY);
>> +
>> + accumulate_time(vcpu, &vcpu->arch.guest_time);
>> +
>> + local_paca->kvm_hstate.in_guest = KVM_GUEST_MODE_GUEST_HV_FAST;
>> + kvmppc_p9_enter_guest(vcpu);
>> + // Radix host and guest means host never runs with guest MMU state
>> + local_paca->kvm_hstate.in_guest = KVM_GUEST_MODE_NONE;
>> +
>> + accumulate_time(vcpu, &vcpu->arch.rm_intr);
>> +
>> + /* Get these from r11/12 and paca exsave */
>> + vcpu->arch.shregs.srr0 = mfspr(SPRN_SRR0);
>> + vcpu->arch.shregs.srr1 = mfspr(SPRN_SRR1);
>> + vcpu->arch.shregs.dar = mfspr(SPRN_DAR);
>> + vcpu->arch.shregs.dsisr = mfspr(SPRN_DSISR);
>> +
>> + /* 0x2 bit for HSRR is only used by PR and P7/8 HV paths, clear it */
>> + trap = local_paca->kvm_hstate.scratch0 & ~0x2;
>> + if (likely(trap > BOOK3S_INTERRUPT_MACHINE_CHECK)) {
>> + exsave = local_paca->exgen;
>> + } else if (trap == BOOK3S_INTERRUPT_SYSTEM_RESET) {
>> + exsave = local_paca->exnmi;
>> + } else { /* trap == 0x200 */
>> + exsave = local_paca->exmc;
>> + }
>> +
>> + vcpu->arch.regs.gpr[1] = local_paca->kvm_hstate.scratch1;
>> + vcpu->arch.regs.gpr[3] = local_paca->kvm_hstate.scratch2;
>> + vcpu->arch.regs.gpr[9] = exsave[EX_R9/sizeof(u64)];
>> + vcpu->arch.regs.gpr[10] = exsave[EX_R10/sizeof(u64)];
>> + vcpu->arch.regs.gpr[11] = exsave[EX_R11/sizeof(u64)];
>> + vcpu->arch.regs.gpr[12] = exsave[EX_R12/sizeof(u64)];
>> + vcpu->arch.regs.gpr[13] = exsave[EX_R13/sizeof(u64)];
>> + vcpu->arch.ppr = exsave[EX_PPR/sizeof(u64)];
>> + vcpu->arch.cfar = exsave[EX_CFAR/sizeof(u64)];
>> + vcpu->arch.regs.ctr = exsave[EX_CTR/sizeof(u64)];
>> +
>> + vcpu->arch.last_inst = KVM_INST_FETCH_FAILED;
>> +
>> + if (unlikely(trap == BOOK3S_INTERRUPT_MACHINE_CHECK)) {
>> + vcpu->arch.fault_dar = exsave[EX_DAR/sizeof(u64)];
>> + vcpu->arch.fault_dsisr = exsave[EX_DSISR/sizeof(u64)];
>> + kvmppc_realmode_machine_check(vcpu);
>> +
>> + } else if (unlikely(trap == BOOK3S_INTERRUPT_HMI)) {
>> + kvmppc_realmode_hmi_handler();
>> +
>> + } else if (trap == BOOK3S_INTERRUPT_H_EMUL_ASSIST) {
>> + vcpu->arch.emul_inst = mfspr(SPRN_HEIR);
>> +
>> + } else if (trap == BOOK3S_INTERRUPT_H_DATA_STORAGE) {
>> + vcpu->arch.fault_dar = exsave[EX_DAR/sizeof(u64)];
>> + vcpu->arch.fault_dsisr = exsave[EX_DSISR/sizeof(u64)];
>> + vcpu->arch.fault_gpa = mfspr(SPRN_ASDR);
>> +
>> + } else if (trap == BOOK3S_INTERRUPT_H_INST_STORAGE) {
>> + vcpu->arch.fault_gpa = mfspr(SPRN_ASDR);
>> +
>> + } else if (trap == BOOK3S_INTERRUPT_H_FAC_UNAVAIL) {
>> + vcpu->arch.hfscr = mfspr(SPRN_HFSCR);
>> +
>> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>> + /*
>> + * Softpatch interrupt for transactional memory emulation cases
>> + * on POWER9 DD2.2. This is early in the guest exit path - we
>> + * haven't saved registers or done a treclaim yet.
>> + */
>> + } else if (trap == BOOK3S_INTERRUPT_HV_SOFTPATCH) {
>> + vcpu->arch.emul_inst = mfspr(SPRN_HEIR);
>> +
>> + /*
>> + * The cases we want to handle here are those where the guest
>> + * is in real suspend mode and is trying to transition to
>> + * transactional mode.
>> + */
>> + if (local_paca->kvm_hstate.fake_suspend &&
>> + (vcpu->arch.shregs.msr & MSR_TS_S)) {
>> + if (kvmhv_p9_tm_emulation_early(vcpu)) {
>> + /* Prevent it being handled again. */
>> + trap = 0;
>> + }
>> + }
>> +#endif
>> + }
>> +
>> + radix_clear_slb();
>> +
>> + __mtmsrd(msr, 0);
>
>
> The asm code only sets RI but this potentially sets more bits including
> MSR_EE, is it expected to be 0 when __kvmhv_vcpu_entry_p9() is called?
Yes.
>> + mtspr(SPRN_CTRLT, 1);
>
> What is this for? ISA does not shed much light:
> ===
> 63 RUN This bit controls an external I/O pin.
> ===
I don't think it even does that these days. It interacts with the PMU.
I was looking whether it's feasible to move it into PMU code entirely,
but apparently some tool or something might sample it. I'm a bit
suspicious about that because an untrusted guest could be running and
claim not to so I don't know what said tool really achieves, but I'll
go through that fight another day.
But KVM has to set it to 1 at exit because Linux host has it set to 1
except in CPU idle.
>
>
>> +
>> + accumulate_time(vcpu, &vcpu->arch.rm_exit);
>
> This should not compile without CONFIG_KVM_BOOK3S_HV_EXIT_TIMING.
It has an ifdef wrapper so it should work (it does on my local tree
which is slightly newer than what you have but I don't think I fixed
anything around this recently).
>> +
>> + end_timing(vcpu);
>> +
>> + return trap;
>
>
> The asm does "For hash guest, read the guest SLB and save it away", this
> code does not. Is this new fast-path-in-c only for radix-on-radix or
> hash VMs are supported too?
That asm code does not run for "guest_exit_short_path" case (aka the
p9 path aka the fast path).
Upstream code only supports radix host and radix guest in this path.
The old path supports hash and radix. That's unchanged with this patch.
After the series, the new path supports all P9 modes (hash/hash,
radix/radix, and radix/hash), and the old path supports P7 and P8 only.
Thanks,
Nick
^ permalink raw reply
* [RFC 2/2] sched: Use H_IDLE_HINT hcall to find if a vCPU can be wakeup target
From: Parth Shah @ 2021-04-01 11:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ego, mikey, srikar, npiggin, paulus, svaidy
In-Reply-To: <20210401115922.1524705-1-parth@linux.ibm.com>
During checking for an available_idle_cpu, if the vCPU is yielded then
check if it will be scheduled instantly by hypervisor or not. From guestOS,
use H_IDLE_HINT hcall to ask for this hint from the hypverisor, and
consider the yielded vCPU as target for wakeups iff it is hinted to be
scheduled instantly.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
arch/powerpc/include/asm/paravirt.h | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
index edc08f04aef7..c7dd0368e1a4 100644
--- a/arch/powerpc/include/asm/paravirt.h
+++ b/arch/powerpc/include/asm/paravirt.h
@@ -41,6 +41,15 @@ static inline void yield_to_any(void)
{
plpar_hcall_norets(H_CONFER, -1, 0);
}
+
+/* Find if the previous physical CPU of this vcpu is available_idle or not */
+static inline void pcpu_available_instantly(int vcpu, unsigned long *is_idle)
+{
+ unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
+
+ if (plpar_hcall(H_IDLE_HINT, retbuf, vcpu) == H_SUCCESS)
+ *is_idle = retbuf[0];
+}
#else
static inline bool is_shared_processor(void)
{
@@ -75,6 +84,8 @@ static inline void prod_cpu(int cpu)
#define vcpu_is_preempted vcpu_is_preempted
static inline bool vcpu_is_preempted(int cpu)
{
+ unsigned long is_idle = 0;
+
if (!is_shared_processor())
return false;
@@ -92,8 +103,14 @@ static inline bool vcpu_is_preempted(int cpu)
}
#endif
- if (yield_count_of(cpu) & 1)
- return true;
+ if (yield_count_of(cpu) & 1) {
+#ifdef CONFIG_PPC_SPLPAR
+ pcpu_available_instantly(cpu, &is_idle);
+#endif
+
+ if (!is_idle)
+ return true;
+ }
return false;
}
--
2.26.2
^ permalink raw reply related
* [RFC 1/2] KVM:PPC: Add new hcall to provide hint if a vcpu task will be scheduled instantly.
From: Parth Shah @ 2021-04-01 11:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ego, mikey, srikar, npiggin, paulus, svaidy
In-Reply-To: <20210401115922.1524705-1-parth@linux.ibm.com>
H_IDLE_HINT is a new hcall introduced to provide a hint to the guestOS
indicating if a given vCPU can be scheduled instantly or not.
The task scheduler generally prefers previous cpu of a task if it is
available_idle. So if a prev_cpu of the corresponding vCPU task_struct is
found to be available_idle or sched_idle then hint guestOS that the given
vCPU can be scheduled instantly by the hypervisor.
Signed-off-by: Parth Shah <parth@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 3 ++-
arch/powerpc/kvm/book3s_hv.c | 13 +++++++++++++
arch/powerpc/kvm/trace_hv.h | 1 +
include/linux/kvm_host.h | 1 +
include/linux/sched.h | 1 +
kernel/sched/core.c | 13 +++++++++++++
kernel/sched/fair.c | 12 ++++++++++++
kernel/sched/sched.h | 1 +
virt/kvm/kvm_main.c | 17 +++++++++++++++++
9 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index c98f5141e3fc..c91e27840c03 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -315,7 +315,8 @@
#define H_SCM_HEALTH 0x400
#define H_SCM_PERFORMANCE_STATS 0x418
#define H_RPT_INVALIDATE 0x448
-#define MAX_HCALL_OPCODE H_RPT_INVALIDATE
+#define H_IDLE_HINT 0x44C
+#define MAX_HCALL_OPCODE H_IDLE_HINT
/* Scope args for H_SCM_UNBIND_ALL */
#define H_UNBIND_SCOPE_ALL (0x1)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 6f612d240392..0472b8a1302f 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -931,6 +931,17 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
if (tvcpu->arch.ceded)
kvmppc_fast_vcpu_kick_hv(tvcpu);
break;
+ case H_IDLE_HINT:
+ target = kvmppc_get_gpr(vcpu, 4);
+ tvcpu = kvmppc_find_vcpu(vcpu->kvm, target);
+ if (!tvcpu) {
+ ret = H_PARAMETER;
+ break;
+ }
+ ret = kvm_vcpu_provide_idle_hint(tvcpu);
+ kvmppc_set_gpr(vcpu, 4, ret);
+ ret = H_SUCCESS;
+ break;
case H_CONFER:
target = kvmppc_get_gpr(vcpu, 4);
if (target == -1)
@@ -1145,6 +1156,7 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
case H_CEDE:
case H_PROD:
case H_CONFER:
+ case H_IDLE_HINT:
case H_REGISTER_VPA:
case H_SET_MODE:
case H_LOGICAL_CI_LOAD:
@@ -5359,6 +5371,7 @@ static unsigned int default_hcall_list[] = {
H_PROD,
H_CONFER,
H_REGISTER_VPA,
+ H_IDLE_HINT,
#ifdef CONFIG_KVM_XICS
H_EOI,
H_CPPR,
diff --git a/arch/powerpc/kvm/trace_hv.h b/arch/powerpc/kvm/trace_hv.h
index 830a126e095d..d0302a917eaf 100644
--- a/arch/powerpc/kvm/trace_hv.h
+++ b/arch/powerpc/kvm/trace_hv.h
@@ -46,6 +46,7 @@
{H_CEDE, "H_CEDE"}, \
{H_CONFER, "H_CONFER"}, \
{H_PROD, "H_PROD"}, \
+ {H_IDLE_HINT, "H_IDLE_HINT"}, \
{H_GET_PPP, "H_GET_PPP"}, \
{H_SET_PPP, "H_SET_PPP"}, \
{H_PURR, "H_PURR"}, \
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f3b1013fb22c..78fb0465cd65 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -843,6 +843,7 @@ bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu);
void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
int kvm_vcpu_yield_to(struct kvm_vcpu *target);
void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu, bool usermode_vcpu_not_eligible);
+unsigned long kvm_vcpu_provide_idle_hint(struct kvm_vcpu *target);
void kvm_flush_remote_tlbs(struct kvm *kvm);
void kvm_reload_remote_mmus(struct kvm *kvm);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6e3a5eeec509..3dea2a4ff58d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1688,6 +1688,7 @@ static inline int set_cpus_allowed_ptr(struct task_struct *p, const struct cpuma
extern int yield_to(struct task_struct *p, bool preempt);
extern void set_user_nice(struct task_struct *p, long nice);
extern int task_prio(const struct task_struct *p);
+extern unsigned long get_idle_hint(struct task_struct *p);
/**
* task_nice - return the nice value of a given task.
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ff74fca39ed2..2962bf97ab13 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6812,6 +6812,19 @@ int __sched yield_to(struct task_struct *p, bool preempt)
}
EXPORT_SYMBOL_GPL(yield_to);
+/*
+ * Provide hint to the VM indicating if the previous vCPU can be scheduled
+ * instantly or not.
+ */
+unsigned long __sched get_idle_hint(struct task_struct *p)
+{
+ unsigned long ret = 0;
+ if (p->sched_class->get_idle_hint)
+ ret = p->sched_class->get_idle_hint(p);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(get_idle_hint);
+
int io_schedule_prepare(void)
{
int old_iowait = current->in_iowait;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 04a3ce20da67..16701a3da5dc 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7236,6 +7236,16 @@ static bool yield_to_task_fair(struct rq *rq, struct task_struct *p)
return true;
}
+static unsigned long get_idle_hint_fair(struct task_struct *p)
+{
+ unsigned int prev_cpu = task_cpu(p);
+
+ if (available_idle_cpu(prev_cpu) || sched_idle_cpu(prev_cpu))
+ return 1;
+
+ return 0;
+}
+
#ifdef CONFIG_SMP
/**************************************************
* Fair scheduling class load-balancing methods.
@@ -11264,6 +11274,8 @@ DEFINE_SCHED_CLASS(fair) = {
.task_change_group = task_change_group_fair,
#endif
+ .get_idle_hint = get_idle_hint_fair,
+
#ifdef CONFIG_UCLAMP_TASK
.uclamp_enabled = 1,
#endif
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index bb09988451a0..09b1e35d8331 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1871,6 +1871,7 @@ struct sched_class {
#ifdef CONFIG_FAIR_GROUP_SCHED
void (*task_change_group)(struct task_struct *p, int type);
#endif
+ unsigned long (*get_idle_hint)(struct task_struct *p);
};
static inline void put_prev_task(struct rq *rq, struct task_struct *prev)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8367d88ce39b..5d750ae2fe0a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2907,6 +2907,23 @@ int kvm_vcpu_yield_to(struct kvm_vcpu *target)
}
EXPORT_SYMBOL_GPL(kvm_vcpu_yield_to);
+unsigned long kvm_vcpu_provide_idle_hint(struct kvm_vcpu *target)
+{
+ struct pid *pid;
+ struct task_struct *task = NULL;
+
+ rcu_read_lock();
+ pid = rcu_dereference(target->pid);
+ if (pid)
+ task = get_pid_task(pid, PIDTYPE_PID);
+ rcu_read_unlock();
+ if (!task)
+ return 0;
+
+ return get_idle_hint(task);
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_provide_idle_hint);
+
/*
* Helper that checks whether a VCPU is eligible for directed yield.
* Most eligible candidate to yield is decided by following heuristics:
--
2.26.2
^ permalink raw reply related
* [RFC 0/2] Define a new apporach to determine if an idle vCPU will be scheduled instantly or not
From: Parth Shah @ 2021-04-01 11:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ego, mikey, srikar, npiggin, paulus, svaidy
Abstract:
=========
The Linux task scheduler tries to find an idle cpu for a wakee task
thereby lowering the wakeup latency as much as possible. The process
of determining if a cpu is idle or not has evolved over time.
Currently, a cpu is considered idle if
- there are no task running or enqueued to the runqueue of the cpu and
- in while running inside a guest, a cpu is not yielded (determined
via available_idle_cpu())
While inside the guest, there is no way to deterministically predict
if a vCPU that has been yielded/ceded to the hypervisor can be gotten
back. Hence currently the scheduler considers such CEDEd vCPU as not
"available" idle and would instead pick other busy CPUs for waking up
the wakee task.
In this patch-set we try to further classify idle cpus as instantly
available or not. This is achieved by taking hint from the hypervisor
by quering if the vCPU will be scheduled instantly or not. In most
cases, scheduler prefers prev_cpu of a waking task unless it is busy.
In this patchset, the hypervisor uses this information to figure out
if the prev_cpu used by the task (of the corresponding vCPU) is idle
or not, and passes this information to the guest.
Interface:
===========
This patchset introduces a new HCALL named H_IDLE_HINT for the guest
to query if a vCPU can be dispatched quickly or not. This is
currently a crude interface to demonstrate the efficacy of this
method. We are looking for feedback on any other mechanisms of
obtainining a hint from the hypervisor where the hint is still relevant.
But this patch series tries to emphasis the possible optimization for
task wakeup-latency and is open to accept any interface/architecture.
Internal working:
========
The code-flow of the current implementation is as follow:
- GuestOS scheduler searches for idle cpu using `avaialable_idle_cpu()`
which also looks if a vcpu_is_preempted() to see if vCPU is yielded or
not.
- If vCPU is yielded, then the GuestOS will additionally make hcall
H_IDLE_HINT to find if a vCPU can be scheduled instantly or not.
- The hypervisor services hcall by first finding the corresponding
task-p of the vCPU and returns 1 if the task_cpu(p) is
avaialable_idle_cpu or running SCHED_IDLE task. Else returns 0.
- GuestOS takes up this hint and considers the vCPU as idle if the hint
from hypervisor has value == 1.
The patch-set is based on v5.11 kernel.
Results:
========
- Baseline kernel = v5.11
- Patched kernel = v5.11 + this patch-set
Setup:
All the results are taken on IBM POWER9 baremetal system running patched
kernel. This system consists of 2 NUMA nodes, 22 cores per socket with
SMT-4 mode.
2 KVM guests are created sharing same set of physical CPUs, and each KVM
has identical CPU topology of 40 CPUs, 10 cores with SMT-4 support.
Scenarios:
----------
1. Under-commit case: Only one KVM is active at a time.
- Baseline (v5.11):
$> schbench -m 20 -t 2 -r 30
Latency percentiles (usec)
50.0000th: 67
75.0000th: 83
90.0000th: 115
95.0000th: 352
*99.0000th: 2260 <-----
99.5000th: 3580
99.9000th: 7128
min=0, max=9927
- With patch (v5.11 + patch):
$> schbench -m 20 -t 2 -r 30
Latency percentiles (usec)
50.0000th: 100
75.0000th: 113
90.0000th: 328
95.0000th: 360
*99.0000th: 434 (-80%) <----
99.5000th: 489
99.9000th: 2324
min=0, max=6054
We see a significant reduction in the tail latencies due to being able
to schedule on an yielded/ceded idle CPU with the patchset instead of
waking up the task on a busy CPU.
2. Over-commit case: Both KVMs sharing same set of CPUs. One KVM is
creating noise using `schbench -m 10 -t 2 -r 3000` while only the other
KVM is benchmarked.
- Baseline:
$> schbench -m 20 -t 2 -r 30
Latency percentiles (usec)
50.0000th: 73
75.0000th: 89
90.0000th: 115
95.0000th: 166
*99.0000th: 3084
99.5000th: 4044
99.9000th: 7656
min=0, max=18448
- With patch:
$> schbench -m 20 -t 2 -r 30
Latency percentiles (usec)
50.0000th: 114
75.0000th: 137
90.0000th: 170
95.0000th: 237
*99.0000th: 2828
99.5000th: 4168
99.9000th: 7528
min=0, max=15387
The results demonstrates that the proposed method of getting idle-hint
from the hypervisor to better find an idle cpu in the guestOS is very
helpful in under-commmit cases due to higher chance of finding the
previously used physical cpu as idle.
The results also confirms that there is no regression in the over-commit
case where the proposed methodlogy does not affect much.
Additionally, more tests were carried out with different combinations of
schbench threads and different numbers of KVM guest. The results for
these tests further confirmed that there is no major regression on the
workload performance.
Parth Shah (2):
KVM:PPC: Add new hcall to provide hint if a vcpu task will be
scheduled instantly.
sched: Use H_IDLE_HINT hcall to find if a vCPU can be wakeup target
arch/powerpc/include/asm/hvcall.h | 3 ++-
arch/powerpc/include/asm/paravirt.h | 21 +++++++++++++++++++--
arch/powerpc/kvm/book3s_hv.c | 13 +++++++++++++
arch/powerpc/kvm/trace_hv.h | 1 +
include/linux/kvm_host.h | 1 +
include/linux/sched.h | 1 +
kernel/sched/core.c | 13 +++++++++++++
kernel/sched/fair.c | 12 ++++++++++++
kernel/sched/sched.h | 1 +
virt/kvm/kvm_main.c | 17 +++++++++++++++++
10 files changed, 80 insertions(+), 3 deletions(-)
--
2.26.2
^ 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