* Re: [PATCH v10 01/10] powerpc/mm: Implement set_memory() routines
From: Aneesh Kumar K.V @ 2021-04-01 4:37 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: ajd, Jordan Niethe, npiggin, cmr, naveen.n.rao, dja
In-Reply-To: <20210330045132.722243-2-jniethe5@gmail.com>
Jordan Niethe <jniethe5@gmail.com> writes:
> From: Russell Currey <ruscur@russell.cc>
>
> The set_memory_{ro/rw/nx/x}() functions are required for STRICT_MODULE_RWX,
> and are generally useful primitives to have. This implementation is
> designed to be completely generic across powerpc's many MMUs.
>
> It's possible that this could be optimised to be faster for specific
> MMUs, but the focus is on having a generic and safe implementation for
> now.
>
> This implementation does not handle cases where the caller is attempting
> to change the mapping of the page it is executing from, or if another
> CPU is concurrently using the page being altered. These cases likely
> shouldn't happen, but a more complex implementation with MMU-specific code
> could safely handle them, so that is left as a TODO for now.
>
> On hash the linear mapping is not kept in the linux pagetable, so this
> will not change the protection if used on that range. Currently these
> functions are not used on the linear map so just WARN for now.
>
> These functions do nothing if STRICT_KERNEL_RWX is not enabled.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> [jpn: -rebase on next plus "powerpc/mm/64s: Allow STRICT_KERNEL_RWX again"
> - WARN on hash linear map]
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v10: WARN if trying to change the hash linear map
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/set_memory.h | 32 ++++++++++
> arch/powerpc/mm/Makefile | 2 +-
> arch/powerpc/mm/pageattr.c | 88 +++++++++++++++++++++++++++
> 4 files changed, 122 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/include/asm/set_memory.h
> create mode 100644 arch/powerpc/mm/pageattr.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index fc7f5c5933e6..4498a27ac9db 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -135,6 +135,7 @@ config PPC
> select ARCH_HAS_MEMBARRIER_CALLBACKS
> select ARCH_HAS_MEMBARRIER_SYNC_CORE
> select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
> + select ARCH_HAS_SET_MEMORY
> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HAS_UACCESS_FLUSHCACHE
> diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
> new file mode 100644
> index 000000000000..64011ea444b4
> --- /dev/null
> +++ b/arch/powerpc/include/asm/set_memory.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_POWERPC_SET_MEMORY_H
> +#define _ASM_POWERPC_SET_MEMORY_H
> +
> +#define SET_MEMORY_RO 0
> +#define SET_MEMORY_RW 1
> +#define SET_MEMORY_NX 2
> +#define SET_MEMORY_X 3
> +
> +int change_memory_attr(unsigned long addr, int numpages, long action);
> +
> +static inline int set_memory_ro(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_RO);
> +}
> +
> +static inline int set_memory_rw(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_RW);
> +}
> +
> +static inline int set_memory_nx(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_NX);
> +}
> +
> +static inline int set_memory_x(unsigned long addr, int numpages)
> +{
> + return change_memory_attr(addr, numpages, SET_MEMORY_X);
> +}
> +
> +#endif
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index 3b4e9e4e25ea..d8a08abde1ae 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -5,7 +5,7 @@
>
> ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>
> -obj-y := fault.o mem.o pgtable.o mmap.o maccess.o \
> +obj-y := fault.o mem.o pgtable.o mmap.o maccess.o pageattr.o \
> init_$(BITS).o pgtable_$(BITS).o \
> pgtable-frag.o ioremap.o ioremap_$(BITS).o \
> init-common.o mmu_context.o drmem.o
> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> new file mode 100644
> index 000000000000..9efcb01088da
> --- /dev/null
> +++ b/arch/powerpc/mm/pageattr.c
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * MMU-generic set_memory implementation for powerpc
> + *
> + * Copyright 2019, IBM Corporation.
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/set_memory.h>
> +
> +#include <asm/mmu.h>
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +
> +
> +/*
> + * Updates the attributes of a page in three steps:
> + *
> + * 1. invalidate the page table entry
> + * 2. flush the TLB
> + * 3. install the new entry with the updated attributes
> + *
> + * This is unsafe if the caller is attempting to change the mapping of the
> + * page it is executing from, or if another CPU is concurrently using the
> + * page being altered.
> + *
> + * TODO make the implementation resistant to this.
> + *
> + * NOTE: can be dangerous to call without STRICT_KERNEL_RWX
> + */
> +static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
> +{
> + long action = (long)data;
> + pte_t pte;
> +
> + spin_lock(&init_mm.page_table_lock);
> +
> + /* invalidate the PTE so it's safe to modify */
> + pte = ptep_get_and_clear(&init_mm, addr, ptep);
> + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> +
> + /* modify the PTE bits as desired, then apply */
> + switch (action) {
> + case SET_MEMORY_RO:
> + pte = pte_wrprotect(pte);
> + break;
> + case SET_MEMORY_RW:
> + pte = pte_mkwrite(pte);
> + break;
> + case SET_MEMORY_NX:
> + pte = pte_exprotect(pte);
> + break;
> + case SET_MEMORY_X:
> + pte = pte_mkexec(pte);
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + break;
> + }
> +
> + set_pte_at(&init_mm, addr, ptep, pte);
> + spin_unlock(&init_mm.page_table_lock);
> +
> + return 0;
> +}
> +
> +int change_memory_attr(unsigned long addr, int numpages, long action)
> +{
> + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> + unsigned long sz = numpages * PAGE_SIZE;
> +
> + if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> + return 0;
What restrictions imposed by that config are we dependent on here?
> +
> + if (numpages <= 0)
> + return 0;
> +
> +#ifdef CONFIG_PPC_BOOK3S_64
> + if (WARN_ON_ONCE(!radix_enabled() &&
> + get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
> + return -1;
> + }
> +#endif
What about VMEMMAP_REGIOND_ID
> +
> + return apply_to_existing_page_range(&init_mm, start, sz,
> + change_page_attr, (void *)action);
That handles on 64K mapping. What about linear map? Also there is a
patchset implementing hugepage for vmalloc mapping.
> +}
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v4 29/46] KVM: PPC: Book3S HV P9: Implement the rest of the P9 path in C
From: Alexey Kardashevskiy @ 2021-04-01 5:30 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210323010305.1045293-30-npiggin@gmail.com>
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.
>
> The remaining assembly is only about 160 lines of low level stack setup,
> with VCPU vs host register save and restore, plus a small shim to the
> legacy paths in the interrupt handler.
>
> There are two motivations for this, the first is just make the code more
> maintainable being in C. The second is to reduce the amount of code
> running in a special KVM mode, "realmode". I put that in quotes because
> with radix it is no longer necessarily real-mode in the MMU, but it
> still has to be treated specially because it may be in real-mode, and
> has various important registers like PID, DEC, TB, etc set to guest.
> This is hostile to the rest of Linux and can't use arbitrary kernel
> functionality or be instrumented well.
>
> This initial patch is a reasonably faithful conversion of the asm code.
> It does lack any loop to return quickly back into the guest without
> switching out of realmode in the case of unimportant or easily handled
> interrupts, as explained in the previous change, handling HV interrupts
> in real mode is not so important for P9.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/asm-prototypes.h | 3 +-
> arch/powerpc/include/asm/kvm_asm.h | 3 +-
> arch/powerpc/include/asm/kvm_book3s_64.h | 8 +
> arch/powerpc/kernel/security.c | 5 +-
> arch/powerpc/kvm/Makefile | 3 +
> arch/powerpc/kvm/book3s_64_entry.S | 246 ++++++++++++++++++++++
> arch/powerpc/kvm/book3s_hv.c | 9 +-
> arch/powerpc/kvm/book3s_hv_interrupt.c | 223 ++++++++++++++++++++
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 123 +----------
> 9 files changed, 500 insertions(+), 123 deletions(-)
> create mode 100644 arch/powerpc/kvm/book3s_hv_interrupt.c
>
> diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
> index 939f3c94c8f3..7c74c80ed994 100644
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -122,6 +122,7 @@ extern s32 patch__call_flush_branch_caches3;
> extern s32 patch__flush_count_cache_return;
> extern s32 patch__flush_link_stack_return;
> extern s32 patch__call_kvm_flush_link_stack;
> +extern s32 patch__call_kvm_flush_link_stack_p9;
> extern s32 patch__memset_nocache, patch__memcpy_nocache;
>
> extern long flush_branch_caches;
> @@ -142,7 +143,7 @@ void kvmhv_load_host_pmu(void);
> void kvmhv_save_guest_pmu(struct kvm_vcpu *vcpu, bool pmu_in_use);
> void kvmhv_load_guest_pmu(struct kvm_vcpu *vcpu);
>
> -int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu);
> +void kvmppc_p9_enter_guest(struct kvm_vcpu *vcpu);
>
> long kvmppc_h_set_dabr(struct kvm_vcpu *vcpu, unsigned long dabr);
> long kvmppc_h_set_xdabr(struct kvm_vcpu *vcpu, unsigned long dabr,
> diff --git a/arch/powerpc/include/asm/kvm_asm.h b/arch/powerpc/include/asm/kvm_asm.h
> index a3633560493b..b4f9996bd331 100644
> --- a/arch/powerpc/include/asm/kvm_asm.h
> +++ b/arch/powerpc/include/asm/kvm_asm.h
> @@ -146,7 +146,8 @@
> #define KVM_GUEST_MODE_GUEST 1
> #define KVM_GUEST_MODE_SKIP 2
> #define KVM_GUEST_MODE_GUEST_HV 3
> -#define KVM_GUEST_MODE_HOST_HV 4
> +#define KVM_GUEST_MODE_GUEST_HV_FAST 4 /* ISA v3.0 with host radix mode */
> +#define KVM_GUEST_MODE_HOST_HV 5
>
> #define KVM_INST_FETCH_FAILED -1
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index 9bb9bb370b53..c214bcffb441 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -153,9 +153,17 @@ static inline bool kvmhv_vcpu_is_radix(struct kvm_vcpu *vcpu)
> return radix;
> }
>
> +int __kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu);
> +
> #define KVM_DEFAULT_HPT_ORDER 24 /* 16MB HPT by default */
> #endif
>
> +/*
> + * Invalid HDSISR value which is used to indicate when HW has not set the reg.
> + * Used to work around an errata.
> + */
> +#define HDSISR_CANARY 0x7fff
> +
> /*
> * We use a lock bit in HPTE dword 0 to synchronize updates and
> * accesses to each HPTE, and another bit to indicate non-present
> diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
> index e4e1a94ccf6a..3a607c11f20f 100644
> --- a/arch/powerpc/kernel/security.c
> +++ b/arch/powerpc/kernel/security.c
> @@ -430,16 +430,19 @@ device_initcall(stf_barrier_debugfs_init);
>
> static void update_branch_cache_flush(void)
> {
> - u32 *site;
> + u32 *site, __maybe_unused *site2;
>
> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> site = &patch__call_kvm_flush_link_stack;
> + site2 = &patch__call_kvm_flush_link_stack_p9;
> // This controls the branch from guest_exit_cont to kvm_flush_link_stack
> if (link_stack_flush_type == BRANCH_CACHE_FLUSH_NONE) {
> patch_instruction_site(site, ppc_inst(PPC_INST_NOP));
> + patch_instruction_site(site2, ppc_inst(PPC_INST_NOP));
> } else {
> // Could use HW flush, but that could also flush count cache
> patch_branch_site(site, (u64)&kvm_flush_link_stack, BRANCH_SET_LINK);
> + patch_branch_site(site2, (u64)&kvm_flush_link_stack, BRANCH_SET_LINK);
> }
> #endif
>
> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
> index cdd119028f64..40a55a5ba4ff 100644
> --- a/arch/powerpc/kvm/Makefile
> +++ b/arch/powerpc/kvm/Makefile
> @@ -43,6 +43,9 @@ kvm-objs-$(CONFIG_KVM_E500MC) := $(kvm-e500mc-objs)
> kvm-book3s_64-builtin-objs-$(CONFIG_SPAPR_TCE_IOMMU) := \
> book3s_64_vio_hv.o
>
> +kvm-book3s_64-builtin-objs-y += \
> + book3s_hv_interrupt.o
> +
> kvm-pr-y := \
> fpu.o \
> emulate.o \
> diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
> index de81ab69555b..845df5fefdbd 100644
> --- a/arch/powerpc/kvm/book3s_64_entry.S
> +++ b/arch/powerpc/kvm/book3s_64_entry.S
> @@ -1,11 +1,16 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> #include <asm/asm-offsets.h>
> #include <asm/cache.h>
> +#include <asm/code-patching-asm.h>
> #include <asm/exception-64s.h>
> +#include <asm/export.h>
> #include <asm/kvm_asm.h>
> #include <asm/kvm_book3s_asm.h>
> +#include <asm/mmu.h>
> #include <asm/ppc_asm.h>
> +#include <asm/ptrace.h>
> #include <asm/reg.h>
> +#include <asm/ultravisor-api.h>
>
> /*
> * These are branched to from interrupt handlers in exception-64s.S which set
> @@ -22,10 +27,15 @@
> .global kvmppc_hcall
> .balign IFETCH_ALIGN_BYTES
> kvmppc_hcall:
> + lbz r10,HSTATE_IN_GUEST(r13)
> + cmpwi r10,KVM_GUEST_MODE_GUEST_HV_FAST
> + beq kvmppc_p9_exit_hcall
> ld r10,PACA_EXGEN+EX_R13(r13)
> SET_SCRATCH0(r10)
> li r10,0xc00
> /* Now we look like kvmppc_interrupt */
> + li r11,PACA_EXGEN
> + b 1f
>
> /*
> * KVM interrupt entry occurs after GEN_INT_ENTRY runs, and follows that
> @@ -46,6 +56,12 @@ kvmppc_hcall:
> .global kvmppc_interrupt
> .balign IFETCH_ALIGN_BYTES
> kvmppc_interrupt:
> + std r10,HSTATE_SCRATCH0(r13)
> + lbz r10,HSTATE_IN_GUEST(r13)
> + cmpwi r10,KVM_GUEST_MODE_GUEST_HV_FAST
> + beq kvmppc_p9_exit_interrupt
> + ld r10,HSTATE_SCRATCH0(r13)
> + lbz r11,HSTATE_IN_GUEST(r13)
> li r11,PACA_EXGEN
> cmpdi r10,0x200
> bgt+ 1f
> @@ -140,3 +156,233 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> GET_SCRATCH0(r13)
> HRFI_TO_KERNEL
> #endif
> +
> +/* Stack frame offsets for kvmppc_hv_entry */
> +#define SFS (144 + STACK_FRAME_MIN_SIZE)
> +#define STACK_SLOT_NVGPRS (SFS - 144) /* 18 gprs */
> +
> +/*
> + * void kvmppc_p9_enter_guest(struct vcpu *vcpu);
> + *
> + * Enter the guest on a ISAv3.0 or later system where we have exactly
> + * one vcpu per vcore, and both the host and guest are radix, and threads
> + * are set to "indepdent mode".
> + */
> +.balign IFETCH_ALIGN_BYTES
> +_GLOBAL(kvmppc_p9_enter_guest)
> +EXPORT_SYMBOL_GPL(kvmppc_p9_enter_guest)
> + mflr r0
> + std r0,PPC_LR_STKOFF(r1)
> + stdu r1,-SFS(r1)
> +
> + std r1,HSTATE_HOST_R1(r13)
> +
> + mfcr r4
> + stw r4,SFS+8(r1)
> +
> + reg = 14
> + .rept 18
> + std reg,STACK_SLOT_NVGPRS + ((reg - 14) * 8)(r1)
> + reg = reg + 1
> + .endr
> +
> + ld r4,VCPU_LR(r3)
> + mtlr r4
> + ld r4,VCPU_CTR(r3)
> + mtctr r4
> + ld r4,VCPU_XER(r3)
> + mtspr SPRN_XER,r4
> +
> + ld r1,VCPU_CR(r3)
> +
> +BEGIN_FTR_SECTION
> + ld r4,VCPU_CFAR(r3)
> + mtspr SPRN_CFAR,r4
> +END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
> +BEGIN_FTR_SECTION
> + ld r4,VCPU_PPR(r3)
> + mtspr SPRN_PPR,r4
> +END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> +
> + reg = 4
> + .rept 28
> + ld reg,__VCPU_GPR(reg)(r3)
> + reg = reg + 1
> + .endr
> +
> + ld r4,VCPU_KVM(r3)
> + lbz r4,KVM_SECURE_GUEST(r4)
> + cmpdi r4,0
> + ld r4,VCPU_GPR(R4)(r3)
> + bne .Lret_to_ultra
> +
> + mtcr r1
> +
> + ld r0,VCPU_GPR(R0)(r3)
> + ld r1,VCPU_GPR(R1)(r3)
> + ld r2,VCPU_GPR(R2)(r3)
> + ld r3,VCPU_GPR(R3)(r3)
> +
> + HRFI_TO_GUEST
> + b .
> +
> + /*
> + * Use UV_RETURN ultracall to return control back to the Ultravisor
> + * after processing an hypercall or interrupt that was forwarded
> + * (a.k.a. reflected) to the Hypervisor.
> + *
> + * All registers have already been reloaded except the ucall requires:
> + * R0 = hcall result
> + * R2 = SRR1, so UV can detect a synthesized interrupt (if any)
> + * R3 = UV_RETURN
> + */
> +.Lret_to_ultra:
> + mtcr r1
> + ld r1,VCPU_GPR(R1)(r3)
> +
> + ld r0,VCPU_GPR(R3)(r3)
> + mfspr r2,SPRN_SRR1
> + LOAD_REG_IMMEDIATE(r3, UV_RETURN)
> + sc 2
> +
> +/*
> + * 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) :)
> + */
> +.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:
> + /*
> + * If set to KVM_GUEST_MODE_GUEST_HV_FAST but we're still in the
> + * hypervisor, that means we can't return from the entry stack.
> + */
> + rldicl. r10,r12,64-MSR_HV_LG,63
> + bne- kvmppc_p9_bad_interrupt
> +
> + std r1,HSTATE_SCRATCH1(r13)
> + std r3,HSTATE_SCRATCH2(r13)
> + ld r1,HSTATE_HOST_R1(r13)
> + ld r3,HSTATE_KVM_VCPU(r13)
> +
> + std r9,VCPU_CR(r3)
> +
> +1:
> + std r11,VCPU_PC(r3)
> + std r12,VCPU_MSR(r3)
> +
> + reg = 14
> + .rept 18
> + std reg,__VCPU_GPR(reg)(r3)
> + reg = reg + 1
> + .endr
> +
> + /* r1, r3, r9-r13 are saved to vcpu by C code */
> + std r0,VCPU_GPR(R0)(r3)
> + std r2,VCPU_GPR(R2)(r3)
> + reg = 4
> + .rept 5
> + std reg,__VCPU_GPR(reg)(r3)
> + reg = reg + 1
> + .endr
> +
> + ld r2,PACATOC(r13)
> +
> + mflr r4
> + std r4,VCPU_LR(r3)
> + mfspr r4,SPRN_XER
> + std r4,VCPU_XER(r3)
> +
> + reg = 14
> + .rept 18
> + ld reg,STACK_SLOT_NVGPRS + ((reg - 14) * 8)(r1)
> + reg = reg + 1
> + .endr
> +
> + lwz r4,SFS+8(r1)
> + mtcr r4
> +
> + /*
> + * Flush the link stack here, before executing the first blr on the
> + * way out of the guest.
> + *
> + * The link stack won't match coming out of the guest anyway so the
> + * only cost is the flush itself. The call clobbers r0.
> + */
> +1: nop
> + patch_site 1b patch__call_kvm_flush_link_stack_p9
> +
> + addi r1,r1,SFS
> + ld r0,PPC_LR_STKOFF(r1)
> + mtlr r0
> + blr
> +
> +/*
> + * Took an interrupt somewhere right before HRFID to guest, so registers are
> + * in a bad way. Return things hopefully enough to run host virtual code and
> + * run the Linux interrupt handler (SRESET or MCE) to print something useful.
> + *
> + * We could be really clever and save all host registers in known locations
> + * before setting HSTATE_IN_GUEST, then restoring them all here, and setting
> + * return address to a fixup that sets them up again. But that's a lot of
> + * effort for a small bit of code. Lots of other things to do first.
> + */
> +kvmppc_p9_bad_interrupt:
> + /*
> + * Set GUEST_MODE_NONE so the handler won't branch to KVM, and clear
> + * MSR_RI in r12 ([H]SRR1) so the handler won't try to return.
> + */
> + li r10,KVM_GUEST_MODE_NONE
> + stb r10,HSTATE_IN_GUEST(r13)
> + li r10,MSR_RI
> + andc r12,r12,r10
> +
> + /*
> + * Clean up guest registers to give host a chance to run.
> + */
> + li r10,0
> + mtspr SPRN_AMR,r10
> + mtspr SPRN_IAMR,r10
> + mtspr SPRN_CIABR,r10
> + mtspr SPRN_DAWRX0,r10
> +BEGIN_FTR_SECTION
> + mtspr SPRN_DAWRX1,r10
> +END_FTR_SECTION_IFSET(CPU_FTR_DAWR1)
> + mtspr SPRN_PID,r10
> +
> + /*
> + * Switch to host MMU mode
> + */
> + ld r10, HSTATE_KVM_VCPU(r13)
> + ld r10, VCPU_KVM(r10)
> + lwz r10, KVM_HOST_LPID(r10)
> + mtspr SPRN_LPID,r10
> +
> + ld r10, HSTATE_KVM_VCPU(r13)
> + ld r10, VCPU_KVM(r10)
> + ld r10, KVM_HOST_LPCR(r10)
> + mtspr SPRN_LPCR,r10
> +
> + /*
> + * Go back to interrupt handler
> + */
> + ld r10,HSTATE_SCRATCH0(r13)
> + cmpwi r10,BOOK3S_INTERRUPT_MACHINE_CHECK
> + beq machine_check_common
> +
> + ld r10,HSTATE_SCRATCH0(r13)
> + cmpwi r10,BOOK3S_INTERRUPT_SYSTEM_RESET
> + beq system_reset_common
> +
> + b .
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 989a1ff5ad11..6dca639ed997 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1431,6 +1431,8 @@ static int kvmppc_handle_exit_hv(struct kvm_vcpu *vcpu,
> */
> case BOOK3S_INTERRUPT_H_DATA_STORAGE:
> r = RESUME_PAGE_FAULT;
> + if (vcpu->arch.fault_dsisr == HDSISR_CANARY)
> + r = RESUME_GUEST; /* Just retry if it's the canary */
> break;
> case BOOK3S_INTERRUPT_H_INST_STORAGE:
> vcpu->arch.fault_dar = kvmppc_get_pc(vcpu);
> @@ -3690,6 +3692,8 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> u64 tb, next_timer;
> int trap, save_pmu;
>
> + WARN_ON_ONCE(vcpu->arch.ceded);
> +
> tb = mftb();
> next_timer = timer_get_next_tb();
> if (tb >= next_timer)
> @@ -3698,8 +3702,6 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> if (next_timer < time_limit)
> time_limit = next_timer;
>
> - vcpu->arch.ceded = 0;
> -
> kvmhv_save_host_pmu(); /* saves it to PACA kvm_hstate */
>
> kvmppc_subcore_enter_guest();
> @@ -3826,9 +3828,10 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
> }
> }
> kvmppc_xive_pull_vcpu(vcpu);
> +
> + vcpu->arch.slb_max = 0;
> }
>
> - vcpu->arch.slb_max = 0;
> dec = mfspr(SPRN_DEC);
> if (!(lpcr & LPCR_LD)) /* Sign extend if not using large decrementer */
> dec = (s32) dec;
> diff --git a/arch/powerpc/kvm/book3s_hv_interrupt.c b/arch/powerpc/kvm/book3s_hv_interrupt.c
> new file mode 100644
> index 000000000000..3151b3d62c01
> --- /dev/null
> +++ b/arch/powerpc/kvm/book3s_hv_interrupt.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/kernel.h>
> +#include <linux/kvm_host.h>
> +#include <asm/asm-prototypes.h>
> +#include <asm/dbell.h>
> +#include <asm/kvm_ppc.h>
> +
> +#ifdef CONFIG_KVM_BOOK3S_HV_EXIT_TIMING
> +static void __start_timing(struct kvm_vcpu *vcpu, struct kvmhv_tb_accumulator *next)
> +{
> + struct kvmppc_vcore *vc = vcpu->arch.vcore;
> + u64 tb = mftb() - vc->tb_offset_applied;
> +
> + vcpu->arch.cur_activity = next;
> + vcpu->arch.cur_tb_start = tb;
> +}
> +
> +static void __accumulate_time(struct kvm_vcpu *vcpu, struct kvmhv_tb_accumulator *next)
> +{
> + struct kvmppc_vcore *vc = vcpu->arch.vcore;
> + struct kvmhv_tb_accumulator *curr;
> + u64 tb = mftb() - vc->tb_offset_applied;
> + u64 prev_tb;
> + u64 delta;
> + u64 seq;
> +
> + curr = vcpu->arch.cur_activity;
> + vcpu->arch.cur_activity = next;
> + prev_tb = vcpu->arch.cur_tb_start;
> + vcpu->arch.cur_tb_start = tb;
> +
> + if (!curr)
> + return;
> +
> + delta = tb - prev_tb;
> +
> + seq = curr->seqcount;
> + curr->seqcount = seq + 1;
> + smp_wmb();
> + curr->tb_total += delta;
> + if (seq == 0 || delta < curr->tb_min)
> + curr->tb_min = delta;
> + if (delta > curr->tb_max)
> + curr->tb_max = delta;
> + smp_wmb();
> + curr->seqcount = seq + 2;
> +}
> +
> +#define start_timing(vcpu, next) __start_timing(vcpu, next)
> +#define end_timing(vcpu) __start_timing(vcpu, NULL)
> +#define accumulate_time(vcpu, next) __accumulate_time(vcpu, next)
> +#else
> +#define start_timing(vcpu, next) do {} while (0)
> +#define end_timing(vcpu) do {} while (0)
> +#define accumulate_time(vcpu, next) do {} while (0)
> +#endif
> +
> +static inline void mfslb(unsigned int idx, u64 *slbee, u64 *slbev)
> +{
> + asm volatile("slbmfev %0,%1" : "=r" (*slbev) : "r" (idx));
> + asm volatile("slbmfee %0,%1" : "=r" (*slbee) : "r" (idx));
> +}
> +
> +static inline void mtslb(unsigned int idx, u64 slbee, u64 slbev)
> +{
> + BUG_ON((slbee & 0xfff) != idx);
> +
> + asm volatile("slbmte %0,%1" :: "r" (slbev), "r" (slbee));
> +}
> +
> +static inline void slb_invalidate(unsigned int ih)
> +{
> + asm volatile("slbia %0" :: "i"(ih));
> +}
This one is not used.
> +
> +/*
> + * Malicious or buggy radix guests may have inserted SLB entries
> + * (only 0..3 because radix always runs with UPRT=1), so these must
> + * be cleared here to avoid side-channels. slbmte is used rather
> + * than slbia, as it won't clear cached translations.
> + */
> +static void radix_clear_slb(void)
> +{
> + u64 slbee, slbev;
> + int i;
> +
> + for (i = 0; i < 4; i++) {
> + mfslb(i, &slbee, &slbev);
> + if (unlikely(slbee || slbev)) {
> + slbee = i;
> + slbev = 0;
> + mtslb(i, slbee, slbev);
> + }
> + }
> +}
> +
> +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?
> + mtspr(SPRN_CTRLT, 1);
What is this for? ISA does not shed much light:
===
63 RUN This bit controls an external I/O pin.
===
> +
> + accumulate_time(vcpu, &vcpu->arch.rm_exit);
This should not compile without CONFIG_KVM_BOOK3S_HV_EXIT_TIMING.
> +
> + 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?
> +}
> +EXPORT_SYMBOL_GPL(__kvmhv_vcpu_entry_p9);
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 2d0d14ed1d92..6118e8a97ddd 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -44,9 +44,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
> #define NAPPING_UNSPLIT 3
>
> /* Stack frame offsets for kvmppc_hv_entry */
> -#define SFS 208
> +#define SFS 160
> #define STACK_SLOT_TRAP (SFS-4)
> -#define STACK_SLOT_SHORT_PATH (SFS-8)
> #define STACK_SLOT_TID (SFS-16)
> #define STACK_SLOT_PSSCR (SFS-24)
> #define STACK_SLOT_PID (SFS-32)
> @@ -59,8 +58,6 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
> #define STACK_SLOT_UAMOR (SFS-88)
> #define STACK_SLOT_DAWR1 (SFS-96)
> #define STACK_SLOT_DAWRX1 (SFS-104)
> -/* the following is used by the P9 short path */
> -#define STACK_SLOT_NVGPRS (SFS-152) /* 18 gprs */
>
> /*
> * Call kvmppc_hv_entry in real mode.
> @@ -1008,9 +1005,6 @@ END_MMU_FTR_SECTION_IFCLR(MMU_FTR_TYPE_RADIX)
> no_xive:
> #endif /* CONFIG_KVM_XICS */
>
> - li r0, 0
> - stw r0, STACK_SLOT_SHORT_PATH(r1)
> -
> deliver_guest_interrupt: /* r4 = vcpu, r13 = paca */
> /* Check if we can deliver an external or decrementer interrupt now */
> ld r0, VCPU_PENDING_EXC(r4)
> @@ -1030,7 +1024,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> mtspr SPRN_SRR0, r6
> mtspr SPRN_SRR1, r7
>
> -fast_guest_entry_c:
> ld r10, VCPU_PC(r4)
> ld r11, VCPU_MSR(r4)
> /* r11 = vcpu->arch.msr & ~MSR_HV */
> @@ -1135,97 +1128,6 @@ ret_to_ultra:
> ld r4, VCPU_GPR(R4)(r4)
> sc 2
>
> -/*
> - * Enter the guest on a P9 or later system where we have exactly
> - * one vcpu per vcore and we don't need to go to real mode
> - * (which implies that host and guest are both using radix MMU mode).
> - * r3 = vcpu pointer
> - * Most SPRs and all the VSRs have been loaded already.
> - */
> -_GLOBAL(__kvmhv_vcpu_entry_p9)
> -EXPORT_SYMBOL_GPL(__kvmhv_vcpu_entry_p9)
> - mflr r0
> - std r0, PPC_LR_STKOFF(r1)
> - stdu r1, -SFS(r1)
> -
> - li r0, 1
> - stw r0, STACK_SLOT_SHORT_PATH(r1)
> -
> - std r3, HSTATE_KVM_VCPU(r13)
> - mfcr r4
> - stw r4, SFS+8(r1)
> -
> - std r1, HSTATE_HOST_R1(r13)
> -
> - reg = 14
> - .rept 18
> - std reg, STACK_SLOT_NVGPRS + ((reg - 14) * 8)(r1)
> - reg = reg + 1
> - .endr
> -
> - reg = 14
> - .rept 18
> - ld reg, __VCPU_GPR(reg)(r3)
> - reg = reg + 1
> - .endr
> -
> - mfmsr r10
> - std r10, HSTATE_HOST_MSR(r13)
> -
> - mr r4, r3
> - b fast_guest_entry_c
> -guest_exit_short_path:
> - /*
> - * Malicious or buggy radix guests may have inserted SLB entries
> - * (only 0..3 because radix always runs with UPRT=1), so these must
> - * be cleared here to avoid side-channels. slbmte is used rather
> - * than slbia, as it won't clear cached translations.
> - */
> - li r0,0
> - slbmte r0,r0
> - li r4,1
> - slbmte r0,r4
> - li r4,2
> - slbmte r0,r4
> - li r4,3
> - slbmte r0,r4
> -
> - li r0, KVM_GUEST_MODE_NONE
> - stb r0, HSTATE_IN_GUEST(r13)
> -
> - reg = 14
> - .rept 18
> - std reg, __VCPU_GPR(reg)(r9)
> - reg = reg + 1
> - .endr
> -
> - reg = 14
> - .rept 18
> - ld reg, STACK_SLOT_NVGPRS + ((reg - 14) * 8)(r1)
> - reg = reg + 1
> - .endr
> -
> - lwz r4, SFS+8(r1)
> - mtcr r4
> -
> - mr r3, r12 /* trap number */
> -
> - addi r1, r1, SFS
> - ld r0, PPC_LR_STKOFF(r1)
> - mtlr r0
> -
> - /* If we are in real mode, do a rfid to get back to the caller */
> - mfmsr r4
> - andi. r5, r4, MSR_IR
> - bnelr
> - rldicl r5, r4, 64 - MSR_TS_S_LG, 62 /* extract TS field */
> - mtspr SPRN_SRR0, r0
> - ld r10, HSTATE_HOST_MSR(r13)
> - rldimi r10, r5, MSR_TS_S_LG, 63 - MSR_TS_T_LG
> - mtspr SPRN_SRR1, r10
> - RFI_TO_KERNEL
> - b .
> -
> secondary_too_late:
> li r12, 0
> stw r12, STACK_SLOT_TRAP(r1)
> @@ -1397,14 +1299,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> mr r4,r9
> bge fast_guest_return
> 2:
> - /* If we came in through the P9 short path, no real mode hcalls */
> - lwz r0, STACK_SLOT_SHORT_PATH(r1)
> - cmpwi r0, 0
> - bne no_try_real
> /* See if this is an hcall we can handle in real mode */
> cmpwi r12,BOOK3S_INTERRUPT_SYSCALL
> beq hcall_try_real_mode
> -no_try_real:
>
> /* Hypervisor doorbell - exit only if host IPI flag set */
> cmpwi r12, BOOK3S_INTERRUPT_H_DOORBELL
> @@ -1447,11 +1344,6 @@ 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
> @@ -1496,11 +1388,6 @@ guest_exit_cont: /* r9 = vcpu, r12 = trap, r13 = paca */
> 1: nop
> patch_site 1b patch__call_kvm_flush_link_stack
>
> - /* If we came in through the P9 short path, go back out to C now */
> - lwz r0, STACK_SLOT_SHORT_PATH(r1)
> - cmpwi r0, 0
> - bne guest_exit_short_path
> -
> /* For hash guest, read the guest SLB and save it away */
> ld r5, VCPU_KVM(r9)
> lbz r0, KVM_RADIX(r5)
> @@ -1548,8 +1435,10 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_RADIX)
> b guest_bypass
>
> 0: /*
> - * Sanitise radix guest SLB, see guest_exit_short_path comment.
> - * We clear vcpu->arch.slb_max to match earlier behaviour.
> + * Malicious or buggy radix guests may have inserted SLB entries
> + * (only 0..3 because radix always runs with UPRT=1), so these must
> + * be cleared here to avoid side-channels. slbmte is used rather
> + * than slbia, as it won't clear cached translations.
> */
> li r0,0
> stw r0,VCPU_SLB_MAX(r9)
> @@ -3362,7 +3251,7 @@ BEGIN_FTR_SECTION
> mtspr SPRN_DAWRX1, r0
> END_FTR_SECTION_IFSET(CPU_FTR_DAWR1)
>
> - /* Clear hash and radix guest SLB, see guest_exit_short_path comment. */
> + /* Clear hash and radix guest SLB. */
> slbmte r0, r0
> PPC_SLBIA(6)
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v4 15/46] KVM: PPC: Book3S 64: Move hcall early register setup to KVM
From: Paul Mackerras @ 2021-04-01 5:41 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <20210323010305.1045293-16-npiggin@gmail.com>
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.
Paul.
^ permalink raw reply
* Re: [PATCH v4 12/46] KVM: PPC: Book3S 64: move KVM interrupt entry to a common entry point
From: Paul Mackerras @ 2021-04-01 4:29 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Fabiano Rosas, linuxppc-dev, kvm-ppc, Daniel Axtens
In-Reply-To: <20210323010305.1045293-13-npiggin@gmail.com>
On Tue, Mar 23, 2021 at 11:02:31AM +1000, Nicholas Piggin wrote:
> Rather than bifurcate the call depending on whether or not HV is
> possible, and have the HV entry test for PR, just make a single
> common point which does the demultiplexing. This makes it simpler
> to add another type of exit handler.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Acked-by: Paul Mackerras <paulus@ozlabs.org>
^ permalink raw reply
* Re: [PATCH v4 13/46] KVM: PPC: Book3S 64: Move GUEST_MODE_SKIP test into KVM
From: Paul Mackerras @ 2021-04-01 5:32 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Fabiano Rosas, linuxppc-dev, kvm-ppc, Daniel Axtens
In-Reply-To: <20210323010305.1045293-14-npiggin@gmail.com>
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.)
Paul.
^ permalink raw reply
* [PATCH V2 2/6] mm: Generalize SYS_SUPPORTS_HUGETLBFS (rename as ARCH_SUPPORTS_HUGETLBFS)
From: Anshuman Khandual @ 2021-04-01 6:44 UTC (permalink / raw)
To: linux-mm, akpm
Cc: Rich Felker, linux-sh, linux-mips, James E.J. Bottomley,
Paul Mackerras, linux-riscv, Will Deacon, Yoshinori Sato,
Helge Deller, Russell King, Catalin Marinas, Albert Ou,
Anshuman Khandual, Alexander Viro, Paul Walmsley,
linux-arm-kernel, Thomas Bogendoerfer, linux-parisc, linux-kernel,
Palmer Dabbelt, linux-fsdevel, linuxppc-dev
In-Reply-To: <1617259448-22529-1-git-send-email-anshuman.khandual@arm.com>
SYS_SUPPORTS_HUGETLBFS config has duplicate definitions on platforms that
subscribe it. Instead, just make it a generic option which can be selected
on applicable platforms. Also rename it as ARCH_SUPPORTS_HUGETLBFS instead.
This reduces code duplication and makes it cleaner.
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: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-sh@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Catalin Marinas <catalin.marinas@arm.com> (arm64)
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com> (riscv)
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/arm/Kconfig | 5 +----
arch/arm64/Kconfig | 4 +---
arch/mips/Kconfig | 6 +-----
arch/parisc/Kconfig | 5 +----
arch/powerpc/Kconfig | 3 ---
arch/powerpc/platforms/Kconfig.cputype | 6 +++---
arch/riscv/Kconfig | 5 +----
arch/sh/Kconfig | 5 +----
fs/Kconfig | 5 ++++-
9 files changed, 13 insertions(+), 31 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5da96f5df48f..e12d9d1f4892 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -31,6 +31,7 @@ config ARM
select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7
select ARCH_SUPPORTS_ATOMIC_RMW
+ select ARCH_SUPPORTS_HUGETLBFS if ARM_LPAE
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
@@ -1504,10 +1505,6 @@ config HW_PERF_EVENTS
def_bool y
depends on ARM_PMU
-config SYS_SUPPORTS_HUGETLBFS
- def_bool y
- depends on ARM_LPAE
-
config HAVE_ARCH_TRANSPARENT_HUGEPAGE
def_bool y
depends on ARM_LPAE
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 75a1af6cef9e..c86b28ef6ac0 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -72,6 +72,7 @@ config ARM64
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_USE_SYM_ANNOTATIONS
select ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ select ARCH_SUPPORTS_HUGETLBFS
select ARCH_SUPPORTS_MEMORY_FAILURE
select ARCH_SUPPORTS_SHADOW_CALL_STACK if CC_HAVE_SHADOW_CALL_STACK
select ARCH_SUPPORTS_LTO_CLANG if CPU_LITTLE_ENDIAN
@@ -1063,9 +1064,6 @@ config HW_PERF_EVENTS
def_bool y
depends on ARM_PMU
-config SYS_SUPPORTS_HUGETLBFS
- def_bool y
-
config ARCH_ENABLE_SPLIT_PMD_PTLOCK
def_bool y if PGTABLE_LEVELS > 2
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index d89efba3d8a4..73ea9b7558c1 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -16,6 +16,7 @@ config MIPS
select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
+ select ARCH_SUPPORTS_HUGETLBFS if CPU_SUPPORTS_HUGEPAGES
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
select ARCH_WANT_IPC_PARSE_VERSION
select ARCH_WANT_LD_ORPHAN_WARN
@@ -1281,11 +1282,6 @@ config SYS_SUPPORTS_BIG_ENDIAN
config SYS_SUPPORTS_LITTLE_ENDIAN
bool
-config SYS_SUPPORTS_HUGETLBFS
- bool
- depends on CPU_SUPPORTS_HUGEPAGES
- default y
-
config MIPS_HUGE_TLB_SUPPORT
def_bool HUGETLB_PAGE || TRANSPARENT_HUGEPAGE
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index afc3b8d03572..bde9907bc5b2 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -12,6 +12,7 @@ config PARISC
select ARCH_HAS_STRICT_KERNEL_RWX
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARCH_NO_SG_CHAIN
+ select ARCH_SUPPORTS_HUGETLBFS if PA20
select ARCH_SUPPORTS_MEMORY_FAILURE
select DMA_OPS
select RTC_CLASS
@@ -138,10 +139,6 @@ config PGTABLE_LEVELS
default 3 if 64BIT && PARISC_PAGE_SIZE_4KB
default 2
-config SYS_SUPPORTS_HUGETLBFS
- def_bool y if PA20
-
-
menu "Processor type and features"
choice
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 386ae12d8523..a74c211e55b1 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -700,9 +700,6 @@ config ARCH_SPARSEMEM_DEFAULT
def_bool y
depends on PPC_BOOK3S_64
-config SYS_SUPPORTS_HUGETLBFS
- bool
-
config ILLEGAL_POINTER_VALUE
hex
# This is roughly half way between the top of user space and the bottom
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 3ce907523b1e..cec1017813f8 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -40,8 +40,8 @@ config PPC_85xx
config PPC_8xx
bool "Freescale 8xx"
+ select ARCH_SUPPORTS_HUGETLBFS
select FSL_SOC
- select SYS_SUPPORTS_HUGETLBFS
select PPC_HAVE_KUEP
select PPC_HAVE_KUAP
select HAVE_ARCH_VMAP_STACK
@@ -95,9 +95,9 @@ config PPC_BOOK3S_64
bool "Server processors"
select PPC_FPU
select PPC_HAVE_PMU_SUPPORT
- select SYS_SUPPORTS_HUGETLBFS
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
+ select ARCH_SUPPORTS_HUGETLBFS
select ARCH_SUPPORTS_NUMA_BALANCING
select IRQ_WORK
select PPC_MM_SLICES
@@ -278,9 +278,9 @@ config FSL_BOOKE
# this is for common code between PPC32 & PPC64 FSL BOOKE
config PPC_FSL_BOOK3E
bool
+ select ARCH_SUPPORTS_HUGETLBFS if PHYS_64BIT || PPC64
select FSL_EMB_PERFMON
select PPC_SMP_MUXED_IPI
- select SYS_SUPPORTS_HUGETLBFS if PHYS_64BIT || PPC64
select PPC_DOORBELL
default y if FSL_BOOKE
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 87d7b52f278f..440b75adeb1c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -30,6 +30,7 @@ config RISCV
select ARCH_HAS_STRICT_KERNEL_RWX if MMU
select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT
+ select ARCH_SUPPORTS_HUGETLBFS if MMU
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
select ARCH_WANT_FRAME_POINTERS
select ARCH_WANT_HUGE_PMD_SHARE if 64BIT
@@ -165,10 +166,6 @@ config ARCH_WANT_GENERAL_HUGETLB
config ARCH_SUPPORTS_UPROBES
def_bool y
-config SYS_SUPPORTS_HUGETLBFS
- depends on MMU
- def_bool y
-
config STACKTRACE_SUPPORT
def_bool y
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index e798e55915c2..a54b0c5de37b 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -101,9 +101,6 @@ config SYS_SUPPORTS_APM_EMULATION
bool
select ARCH_SUSPEND_POSSIBLE
-config SYS_SUPPORTS_HUGETLBFS
- bool
-
config SYS_SUPPORTS_SMP
bool
@@ -175,12 +172,12 @@ config CPU_SH3
config CPU_SH4
bool
+ select ARCH_SUPPORTS_HUGETLBFS if MMU
select CPU_HAS_INTEVT
select CPU_HAS_SR_RB
select CPU_HAS_FPU if !CPU_SH4AL_DSP
select SH_INTC
select SYS_SUPPORTS_SH_TMU
- select SYS_SUPPORTS_HUGETLBFS if MMU
config CPU_SH4A
bool
diff --git a/fs/Kconfig b/fs/Kconfig
index a55bda4233bb..366a603ad947 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -222,10 +222,13 @@ config TMPFS_INODE64
If unsure, say N.
+config ARCH_SUPPORTS_HUGETLBFS
+ def_bool n
+
config HUGETLBFS
bool "HugeTLB file system support"
depends on X86 || IA64 || SPARC64 || (S390 && 64BIT) || \
- SYS_SUPPORTS_HUGETLBFS || BROKEN
+ ARCH_SUPPORTS_HUGETLBFS || BROKEN
help
hugetlbfs is a filesystem backing for HugeTLB pages, based on
ramfs. For architectures that support it, say Y here and read
--
2.20.1
^ permalink raw reply related
* [PATCH V2 3/6] mm: Generalize ARCH_ENABLE_MEMORY_[HOTPLUG|HOTREMOVE]
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_MEMORY_[HOTPLUG|HOTREMOVE] configs have duplicate definitions
on platforms that subscribe them. Instead, just make them generic options
which can be selected 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 | 8 ++------
arch/ia64/Kconfig | 8 ++------
arch/powerpc/Kconfig | 8 ++------
arch/s390/Kconfig | 8 ++------
arch/sh/Kconfig | 2 ++
arch/sh/mm/Kconfig | 8 --------
arch/x86/Kconfig | 10 ++--------
mm/Kconfig | 6 ++++++
8 files changed, 18 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c86b28ef6ac0..48355c5519c3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -11,6 +11,8 @@ config ARM64
select ACPI_PPTT if ACPI
select ARCH_HAS_DEBUG_WX
select ARCH_BINFMT_ELF_STATE
+ select ARCH_ENABLE_MEMORY_HOTPLUG
+ select ARCH_ENABLE_MEMORY_HOTREMOVE
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEBUG_VM_PGTABLE
@@ -305,12 +307,6 @@ config ZONE_DMA32
bool "Support DMA32 zone" if EXPERT
default y
-config ARCH_ENABLE_MEMORY_HOTPLUG
- def_bool y
-
-config ARCH_ENABLE_MEMORY_HOTREMOVE
- def_bool y
-
config SMP
def_bool y
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 2ad7a8d29fcc..96ce53ad5c9d 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -13,6 +13,8 @@ config IA64
select ARCH_MIGHT_HAVE_PC_SERIO
select ACPI
select ACPI_NUMA if NUMA
+ select ARCH_ENABLE_MEMORY_HOTPLUG
+ select ARCH_ENABLE_MEMORY_HOTREMOVE
select ARCH_SUPPORTS_ACPI
select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI
@@ -250,12 +252,6 @@ config HOTPLUG_CPU
can be controlled through /sys/devices/system/cpu/cpu#.
Say N if you want to disable CPU hotplug.
-config ARCH_ENABLE_MEMORY_HOTPLUG
- def_bool y
-
-config ARCH_ENABLE_MEMORY_HOTREMOVE
- def_bool y
-
config SCHED_SMT
bool "SMT scheduler support"
depends on SMP
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a74c211e55b1..02a05a24659d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -118,6 +118,8 @@ config PPC
# Please keep this list sorted alphabetically.
#
select ARCH_32BIT_OFF_T if PPC32
+ select ARCH_ENABLE_MEMORY_HOTPLUG
+ select ARCH_ENABLE_MEMORY_HOTREMOVE
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEVMEM_IS_ALLOWED
select ARCH_HAS_ELF_RANDOMIZE
@@ -515,12 +517,6 @@ config ARCH_CPU_PROBE_RELEASE
def_bool y
depends on HOTPLUG_CPU
-config ARCH_ENABLE_MEMORY_HOTPLUG
- def_bool y
-
-config ARCH_ENABLE_MEMORY_HOTREMOVE
- def_bool y
-
config PPC64_SUPPORTS_MEMORY_FAILURE
bool "Add support for memory hwpoison"
depends on PPC_BOOK3S_64
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index c1ff874e6c2e..f8b356550daa 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -60,6 +60,8 @@ config S390
imply IMA_SECURE_AND_OR_TRUSTED_BOOT
select ARCH_32BIT_USTAT_F_TINODE
select ARCH_BINFMT_ELF_STATE
+ select ARCH_ENABLE_MEMORY_HOTPLUG if SPARSEMEM
+ select ARCH_ENABLE_MEMORY_HOTREMOVE
select ARCH_HAS_DEBUG_VM_PGTABLE
select ARCH_HAS_DEBUG_WX
select ARCH_HAS_DEVMEM_IS_ALLOWED
@@ -626,12 +628,6 @@ config ARCH_SPARSEMEM_ENABLE
config ARCH_SPARSEMEM_DEFAULT
def_bool y
-config ARCH_ENABLE_MEMORY_HOTPLUG
- def_bool y if SPARSEMEM
-
-config ARCH_ENABLE_MEMORY_HOTREMOVE
- def_bool y
-
config ARCH_ENABLE_SPLIT_PMD_PTLOCK
def_bool y
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index a54b0c5de37b..68129537e350 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -2,6 +2,8 @@
config SUPERH
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_ENABLE_MEMORY_HOTPLUG if SPARSEMEM && MMU
+ select ARCH_ENABLE_MEMORY_HOTREMOVE if SPARSEMEM && MMU
select ARCH_HAVE_CUSTOM_GPIO_H
select ARCH_HAVE_NMI_SAFE_CMPXCHG if (GUSA_RB || CPU_SH4A)
select ARCH_HAS_BINFMT_FLAT if !MMU
diff --git a/arch/sh/mm/Kconfig b/arch/sh/mm/Kconfig
index 77aa2f802d8d..d551a9cac41e 100644
--- a/arch/sh/mm/Kconfig
+++ b/arch/sh/mm/Kconfig
@@ -136,14 +136,6 @@ config ARCH_SPARSEMEM_DEFAULT
config ARCH_SELECT_MEMORY_MODEL
def_bool y
-config ARCH_ENABLE_MEMORY_HOTPLUG
- def_bool y
- depends on SPARSEMEM && MMU
-
-config ARCH_ENABLE_MEMORY_HOTREMOVE
- def_bool y
- depends on SPARSEMEM && MMU
-
config ARCH_MEMORY_PROBE
def_bool y
depends on MEMORY_HOTPLUG
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 51d171abb57a..503d8b2e8676 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -60,6 +60,8 @@ config X86
select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
select ARCH_32BIT_OFF_T if X86_32
select ARCH_CLOCKSOURCE_INIT
+ select ARCH_ENABLE_MEMORY_HOTPLUG if X86_64 || (X86_32 && HIGHMEM)
+ select ARCH_ENABLE_MEMORY_HOTREMOVE if MEMORY_HOTPLUG
select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_DEBUG_VIRTUAL
@@ -2423,14 +2425,6 @@ config ARCH_HAS_ADD_PAGES
def_bool y
depends on X86_64 && ARCH_ENABLE_MEMORY_HOTPLUG
-config ARCH_ENABLE_MEMORY_HOTPLUG
- def_bool y
- depends on X86_64 || (X86_32 && HIGHMEM)
-
-config ARCH_ENABLE_MEMORY_HOTREMOVE
- def_bool y
- depends on MEMORY_HOTPLUG
-
config USE_PERCPU_NUMA_NODE_ID
def_bool y
depends on NUMA
diff --git a/mm/Kconfig b/mm/Kconfig
index 1c9a37fc651a..9b58fa08847d 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -149,6 +149,9 @@ config MEMORY_ISOLATION
config HAVE_BOOTMEM_INFO_NODE
def_bool n
+config ARCH_ENABLE_MEMORY_HOTPLUG
+ bool
+
# eventually, we can have this option just 'select SPARSEMEM'
config MEMORY_HOTPLUG
bool "Allow for memory hot-add"
@@ -177,6 +180,9 @@ config MEMORY_HOTPLUG_DEFAULT_ONLINE
Say N here if you want the default policy to keep all hot-plugged
memory blocks in 'offline' state.
+config ARCH_ENABLE_MEMORY_HOTREMOVE
+ bool
+
config MEMORY_HOTREMOVE
bool "Allow for memory hot remove"
select HAVE_BOOTMEM_INFO_NODE if (X86_64 || PPC64)
--
2.20.1
^ permalink raw reply related
* [PATCH V2 4/6] mm: Drop redundant ARCH_ENABLE_[HUGEPAGE|THP]_MIGRATION
From: Anshuman Khandual @ 2021-04-01 6:44 UTC (permalink / raw)
To: linux-mm, akpm
Cc: x86, Anshuman Khandual, H. Peter Anvin, linux-kernel,
linuxppc-dev, Ingo Molnar, Paul Mackerras, Catalin Marinas,
Thomas Gleixner, Will Deacon, linux-arm-kernel
In-Reply-To: <1617259448-22529-1-git-send-email-anshuman.khandual@arm.com>
ARCH_ENABLE_[HUGEPAGE|THP]_MIGRATION configs have duplicate definitions on
platforms that subscribe them. Drop these reduntant definitions and instead
just select them appropriately.
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: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: x86@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Catalin Marinas <catalin.marinas@arm.com> (arm64)
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/arm64/Kconfig | 10 ++--------
arch/powerpc/platforms/Kconfig.cputype | 5 +----
arch/x86/Kconfig | 10 ++--------
3 files changed, 5 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 48355c5519c3..cd012af0a4b7 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -11,8 +11,10 @@ config ARM64
select ACPI_PPTT if ACPI
select ARCH_HAS_DEBUG_WX
select ARCH_BINFMT_ELF_STATE
+ select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_MEMORY_HOTPLUG
select ARCH_ENABLE_MEMORY_HOTREMOVE
+ select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEBUG_VM_PGTABLE
@@ -1905,14 +1907,6 @@ config SYSVIPC_COMPAT
def_bool y
depends on COMPAT && SYSVIPC
-config ARCH_ENABLE_HUGEPAGE_MIGRATION
- def_bool y
- depends on HUGETLB_PAGE && MIGRATION
-
-config ARCH_ENABLE_THP_MIGRATION
- def_bool y
- depends on TRANSPARENT_HUGEPAGE
-
menu "Power management options"
source "kernel/power/Kconfig"
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index cec1017813f8..4465b71b2bff 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -96,6 +96,7 @@ config PPC_BOOK3S_64
select PPC_FPU
select PPC_HAVE_PMU_SUPPORT
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
+ select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
select ARCH_SUPPORTS_HUGETLBFS
select ARCH_SUPPORTS_NUMA_BALANCING
@@ -420,10 +421,6 @@ config PPC_PKEY
depends on PPC_BOOK3S_64
depends on PPC_MEM_KEYS || PPC_KUAP || PPC_KUEP
-config ARCH_ENABLE_HUGEPAGE_MIGRATION
- def_bool y
- depends on PPC_BOOK3S_64 && HUGETLB_PAGE && MIGRATION
-
config PPC_MMU_NOHASH
def_bool y
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 503d8b2e8676..10702ef1eb57 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -60,8 +60,10 @@ config X86
select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
select ARCH_32BIT_OFF_T if X86_32
select ARCH_CLOCKSOURCE_INIT
+ 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_THP_MIGRATION if x86_64 && TRANSPARENT_HUGEPAGE
select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
select ARCH_HAS_CACHE_LINE_SIZE
select ARCH_HAS_DEBUG_VIRTUAL
@@ -2433,14 +2435,6 @@ config ARCH_ENABLE_SPLIT_PMD_PTLOCK
def_bool y
depends on X86_64 || X86_PAE
-config ARCH_ENABLE_HUGEPAGE_MIGRATION
- def_bool y
- depends on X86_64 && HUGETLB_PAGE && MIGRATION
-
-config ARCH_ENABLE_THP_MIGRATION
- def_bool y
- depends on X86_64 && TRANSPARENT_HUGEPAGE
-
menu "Power management and ACPI options"
config ARCH_HIBERNATION_HEADER
--
2.20.1
^ permalink raw reply related
* [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
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