* [PATCH v1 5/7] KVM: PPC: Book3S 64: Move interrupt early register setup to KVM
From: Nicholas Piggin @ 2021-04-12 7:51 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210412075103.1533302-1-npiggin@gmail.com>
Like the earlier patch for hcalls, KVM interrupt entry requires a
different calling convention than the Linux interrupt handlers
set up. Move the code that converts from one to the other into KVM.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 131 +++++----------------------
arch/powerpc/kvm/book3s_64_entry.S | 50 +++++++++-
2 files changed, 71 insertions(+), 110 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 1bfd0d7af09e..cd1731642b12 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -187,7 +187,6 @@ do_define_int n
.endif
.endm
-#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
/*
* All interrupts which set HSRR registers, as well as SRESET and MCE and
* syscall when invoked with "sc 1" switch to MSR[HV]=1 (HVMODE) to be taken,
@@ -220,54 +219,25 @@ do_define_int n
* to KVM to handle.
*/
-.macro KVMTEST name
+.macro KVMTEST name handler
+#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
lbz r10,HSTATE_IN_GUEST(r13)
cmpwi r10,0
- bne \name\()_kvm
-.endm
-
-.macro GEN_KVM name
- .balign IFETCH_ALIGN_BYTES
-\name\()_kvm:
-
-BEGIN_FTR_SECTION
- ld r10,IAREA+EX_CFAR(r13)
- std r10,HSTATE_CFAR(r13)
-END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
-
- ld r10,IAREA+EX_CTR(r13)
- mtctr r10
-BEGIN_FTR_SECTION
- ld r10,IAREA+EX_PPR(r13)
- std r10,HSTATE_PPR(r13)
-END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
- ld r11,IAREA+EX_R11(r13)
- ld r12,IAREA+EX_R12(r13)
- std r12,HSTATE_SCRATCH0(r13)
- sldi r12,r9,32
- ld r9,IAREA+EX_R9(r13)
- ld r10,IAREA+EX_R10(r13)
/* HSRR variants have the 0x2 bit added to their trap number */
.if IHSRR_IF_HVMODE
BEGIN_FTR_SECTION
- ori r12,r12,(IVEC + 0x2)
+ li r10,(IVEC + 0x2)
FTR_SECTION_ELSE
- ori r12,r12,(IVEC)
+ li r10,(IVEC)
ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
.elseif IHSRR
- ori r12,r12,(IVEC+ 0x2)
+ li r10,(IVEC + 0x2)
.else
- ori r12,r12,(IVEC)
+ li r10,(IVEC)
.endif
- b kvmppc_interrupt
-.endm
-
-#else
-.macro KVMTEST name
-.endm
-.macro GEN_KVM name
-.endm
+ bne \handler
#endif
+.endm
/*
* This is the BOOK3S interrupt entry code macro.
@@ -409,7 +379,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
DEFINE_FIXED_SYMBOL(\name\()_common_real)
\name\()_common_real:
.if IKVM_REAL
- KVMTEST \name
+ KVMTEST \name kvm_interrupt
.endif
ld r10,PACAKMSR(r13) /* get MSR value for kernel */
@@ -432,7 +402,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real)
DEFINE_FIXED_SYMBOL(\name\()_common_virt)
\name\()_common_virt:
.if IKVM_VIRT
- KVMTEST \name
+ KVMTEST \name kvm_interrupt
1:
.endif
.endif /* IVIRT */
@@ -446,7 +416,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_virt)
DEFINE_FIXED_SYMBOL(\name\()_common_real)
\name\()_common_real:
.if IKVM_REAL
- KVMTEST \name
+ KVMTEST \name kvm_interrupt
.endif
.endm
@@ -967,8 +937,6 @@ EXC_COMMON_BEGIN(system_reset_common)
EXCEPTION_RESTORE_REGS
RFI_TO_USER_OR_KERNEL
- GEN_KVM system_reset
-
/**
* Interrupt 0x200 - Machine Check Interrupt (MCE).
@@ -1132,7 +1100,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
/*
* Check if we are coming from guest. If yes, then run the normal
* exception handler which will take the
- * machine_check_kvm->kvmppc_interrupt branch to deliver the MC event
+ * machine_check_kvm->kvm_interrupt branch to deliver the MC event
* to guest.
*/
lbz r11,HSTATE_IN_GUEST(r13)
@@ -1203,8 +1171,6 @@ EXC_COMMON_BEGIN(machine_check_common)
bl machine_check_exception
b interrupt_return
- GEN_KVM machine_check
-
#ifdef CONFIG_PPC_P7_NAP
/*
@@ -1339,8 +1305,6 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
REST_NVGPRS(r1)
b interrupt_return
- GEN_KVM data_access
-
/**
* Interrupt 0x380 - Data Segment Interrupt (DSLB).
@@ -1390,8 +1354,6 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
bl do_bad_slb_fault
b interrupt_return
- GEN_KVM data_access_slb
-
/**
* Interrupt 0x400 - Instruction Storage Interrupt (ISI).
@@ -1428,8 +1390,6 @@ MMU_FTR_SECTION_ELSE
ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
b interrupt_return
- GEN_KVM instruction_access
-
/**
* Interrupt 0x480 - Instruction Segment Interrupt (ISLB).
@@ -1474,8 +1434,6 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
bl do_bad_slb_fault
b interrupt_return
- GEN_KVM instruction_access_slb
-
/**
* Interrupt 0x500 - External Interrupt.
@@ -1521,8 +1479,6 @@ EXC_COMMON_BEGIN(hardware_interrupt_common)
bl do_IRQ
b interrupt_return
- GEN_KVM hardware_interrupt
-
/**
* Interrupt 0x600 - Alignment Interrupt
@@ -1550,8 +1506,6 @@ EXC_COMMON_BEGIN(alignment_common)
REST_NVGPRS(r1) /* instruction emulation may change GPRs */
b interrupt_return
- GEN_KVM alignment
-
/**
* Interrupt 0x700 - Program Interrupt (program check).
@@ -1659,8 +1613,6 @@ EXC_COMMON_BEGIN(program_check_common)
REST_NVGPRS(r1) /* instruction emulation may change GPRs */
b interrupt_return
- GEN_KVM program_check
-
/*
* Interrupt 0x800 - Floating-Point Unavailable Interrupt.
@@ -1710,8 +1662,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_TM)
b interrupt_return
#endif
- GEN_KVM fp_unavailable
-
/**
* Interrupt 0x900 - Decrementer Interrupt.
@@ -1751,8 +1701,6 @@ EXC_COMMON_BEGIN(decrementer_common)
bl timer_interrupt
b interrupt_return
- GEN_KVM decrementer
-
/**
* Interrupt 0x980 - Hypervisor Decrementer Interrupt.
@@ -1798,8 +1746,6 @@ EXC_COMMON_BEGIN(hdecrementer_common)
ld r13,PACA_EXGEN+EX_R13(r13)
HRFI_TO_KERNEL
- GEN_KVM hdecrementer
-
/**
* Interrupt 0xa00 - Directed Privileged Doorbell Interrupt.
@@ -1840,8 +1786,6 @@ EXC_COMMON_BEGIN(doorbell_super_common)
#endif
b interrupt_return
- GEN_KVM doorbell_super
-
EXC_REAL_NONE(0xb00, 0x100)
EXC_VIRT_NONE(0x4b00, 0x100)
@@ -1891,7 +1835,7 @@ INT_DEFINE_END(system_call)
GET_PACA(r13)
std r10,PACA_EXGEN+EX_R10(r13)
INTERRUPT_TO_KERNEL
- KVMTEST system_call /* uses r10, branch to system_call_kvm */
+ KVMTEST system_call kvm_hcall /* uses r10, branch to kvm_hcall */
mfctr r9
#else
mr r9,r13
@@ -1947,7 +1891,7 @@ EXC_VIRT_BEGIN(system_call, 0x4c00, 0x100)
EXC_VIRT_END(system_call, 0x4c00, 0x100)
#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
-TRAMP_REAL_BEGIN(system_call_kvm)
+TRAMP_REAL_BEGIN(kvm_hcall)
mfctr r10
SET_SCRATCH0(r10) /* Save r13 in SCRATCH0 */
#ifdef CONFIG_RELOCATABLE
@@ -1987,8 +1931,6 @@ EXC_COMMON_BEGIN(single_step_common)
bl single_step_exception
b interrupt_return
- GEN_KVM single_step
-
/**
* Interrupt 0xe00 - Hypervisor Data Storage Interrupt (HDSI).
@@ -2027,8 +1969,6 @@ MMU_FTR_SECTION_ELSE
ALT_MMU_FTR_SECTION_END_IFSET(MMU_FTR_TYPE_RADIX)
b interrupt_return
- GEN_KVM h_data_storage
-
/**
* Interrupt 0xe20 - Hypervisor Instruction Storage Interrupt (HISI).
@@ -2054,8 +1994,6 @@ EXC_COMMON_BEGIN(h_instr_storage_common)
bl unknown_exception
b interrupt_return
- GEN_KVM h_instr_storage
-
/**
* Interrupt 0xe40 - Hypervisor Emulation Assistance Interrupt.
@@ -2080,8 +2018,6 @@ EXC_COMMON_BEGIN(emulation_assist_common)
REST_NVGPRS(r1) /* instruction emulation may change GPRs */
b interrupt_return
- GEN_KVM emulation_assist
-
/**
* Interrupt 0xe60 - Hypervisor Maintenance Interrupt (HMI).
@@ -2153,8 +2089,6 @@ EXC_COMMON_BEGIN(hmi_exception_early_common)
EXCEPTION_RESTORE_REGS hsrr=1
GEN_INT_ENTRY hmi_exception, virt=0
- GEN_KVM hmi_exception_early
-
EXC_COMMON_BEGIN(hmi_exception_common)
GEN_COMMON hmi_exception
FINISH_NAP
@@ -2162,8 +2096,6 @@ EXC_COMMON_BEGIN(hmi_exception_common)
bl handle_hmi_exception
b interrupt_return
- GEN_KVM hmi_exception
-
/**
* Interrupt 0xe80 - Directed Hypervisor Doorbell Interrupt.
@@ -2195,8 +2127,6 @@ EXC_COMMON_BEGIN(h_doorbell_common)
#endif
b interrupt_return
- GEN_KVM h_doorbell
-
/**
* Interrupt 0xea0 - Hypervisor Virtualization Interrupt.
@@ -2224,8 +2154,6 @@ EXC_COMMON_BEGIN(h_virt_irq_common)
bl do_IRQ
b interrupt_return
- GEN_KVM h_virt_irq
-
EXC_REAL_NONE(0xec0, 0x20)
EXC_VIRT_NONE(0x4ec0, 0x20)
@@ -2270,8 +2198,6 @@ EXC_COMMON_BEGIN(performance_monitor_common)
bl performance_monitor_exception
b interrupt_return
- GEN_KVM performance_monitor
-
/**
* Interrupt 0xf20 - Vector Unavailable Interrupt.
@@ -2321,8 +2247,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
bl altivec_unavailable_exception
b interrupt_return
- GEN_KVM altivec_unavailable
-
/**
* Interrupt 0xf40 - VSX Unavailable Interrupt.
@@ -2371,8 +2295,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
bl vsx_unavailable_exception
b interrupt_return
- GEN_KVM vsx_unavailable
-
/**
* Interrupt 0xf60 - Facility Unavailable Interrupt.
@@ -2401,8 +2323,6 @@ EXC_COMMON_BEGIN(facility_unavailable_common)
REST_NVGPRS(r1) /* instruction emulation may change GPRs */
b interrupt_return
- GEN_KVM facility_unavailable
-
/**
* Interrupt 0xf60 - Hypervisor Facility Unavailable Interrupt.
@@ -2431,8 +2351,6 @@ EXC_COMMON_BEGIN(h_facility_unavailable_common)
REST_NVGPRS(r1) /* XXX Shouldn't be necessary in practice */
b interrupt_return
- GEN_KVM h_facility_unavailable
-
EXC_REAL_NONE(0xfa0, 0x20)
EXC_VIRT_NONE(0x4fa0, 0x20)
@@ -2462,8 +2380,6 @@ EXC_COMMON_BEGIN(cbe_system_error_common)
bl cbe_system_error_exception
b interrupt_return
- GEN_KVM cbe_system_error
-
#else /* CONFIG_CBE_RAS */
EXC_REAL_NONE(0x1200, 0x100)
EXC_VIRT_NONE(0x5200, 0x100)
@@ -2495,8 +2411,6 @@ EXC_COMMON_BEGIN(instruction_breakpoint_common)
bl instruction_breakpoint_exception
b interrupt_return
- GEN_KVM instruction_breakpoint
-
EXC_REAL_NONE(0x1400, 0x100)
EXC_VIRT_NONE(0x5400, 0x100)
@@ -2617,8 +2531,6 @@ EXC_COMMON_BEGIN(denorm_exception_common)
bl unknown_exception
b interrupt_return
- GEN_KVM denorm_exception
-
#ifdef CONFIG_CBE_RAS
INT_DEFINE_BEGIN(cbe_maintenance)
@@ -2636,8 +2548,6 @@ EXC_COMMON_BEGIN(cbe_maintenance_common)
bl cbe_maintenance_exception
b interrupt_return
- GEN_KVM cbe_maintenance
-
#else /* CONFIG_CBE_RAS */
EXC_REAL_NONE(0x1600, 0x100)
EXC_VIRT_NONE(0x5600, 0x100)
@@ -2668,8 +2578,6 @@ EXC_COMMON_BEGIN(altivec_assist_common)
#endif
b interrupt_return
- GEN_KVM altivec_assist
-
#ifdef CONFIG_CBE_RAS
INT_DEFINE_BEGIN(cbe_thermal)
@@ -2687,8 +2595,6 @@ EXC_COMMON_BEGIN(cbe_thermal_common)
bl cbe_thermal_exception
b interrupt_return
- GEN_KVM cbe_thermal
-
#else /* CONFIG_CBE_RAS */
EXC_REAL_NONE(0x1800, 0x100)
EXC_VIRT_NONE(0x5800, 0x100)
@@ -2941,6 +2847,15 @@ TRAMP_REAL_BEGIN(rfscv_flush_fallback)
USE_TEXT_SECTION()
+#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
+kvm_interrupt:
+ /*
+ * The conditional branch in KVMTEST can't reach all the way,
+ * make a stub.
+ */
+ b kvmppc_interrupt
+#endif
+
_GLOBAL(do_uaccess_flush)
UACCESS_FLUSH_FIXUP_SECTION
nop
diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
index f527e16707db..2c9d106145e8 100644
--- a/arch/powerpc/kvm/book3s_64_entry.S
+++ b/arch/powerpc/kvm/book3s_64_entry.S
@@ -44,15 +44,61 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
sldi r12,r10,32
ori r12,r12,0xc00
ld r10,PACA_EXGEN+EX_R10(r13)
+ b do_kvm_interrupt
+/*
+ * KVM interrupt entry occurs after GEN_INT_ENTRY runs, and follows that
+ * call convention:
+ *
+ * guest R9-R13, CTR, CFAR, PPR saved in PACA EX_xxx save area
+ * guest (H)DAR, (H)DSISR are also in the save area for relevant interrupts
+ * guest R13 also saved in SCRATCH0
+ * R13 = PACA
+ * R11 = (H)SRR0
+ * R12 = (H)SRR1
+ * R9 = guest CR
+ * PPR is set to medium
+ *
+ * With the addition for KVM:
+ * R10 = trap vector
+ */
.global kvmppc_interrupt
.balign IFETCH_ALIGN_BYTES
kvmppc_interrupt:
+ li r11,PACA_EXGEN
+ cmpdi r10,0x200
+ bgt+ 1f
+ li r11,PACA_EXMC
+ beq 1f
+ li r11,PACA_EXNMI
+1: add r11,r11,r13
+
+BEGIN_FTR_SECTION
+ ld r12,EX_CFAR(r11)
+ std r12,HSTATE_CFAR(r13)
+END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
+ ld r12,EX_CTR(r11)
+ mtctr r12
+BEGIN_FTR_SECTION
+ ld r12,EX_PPR(r11)
+ std r12,HSTATE_PPR(r13)
+END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
+ ld r12,EX_R12(r11)
+ std r12,HSTATE_SCRATCH0(r13)
+ sldi r12,r9,32
+ or r12,r12,r10
+ ld r9,EX_R9(r11)
+ ld r10,EX_R10(r11)
+ ld r11,EX_R11(r11)
+
+do_kvm_interrupt:
/*
- * Register contents:
+ * Hcalls and other interrupts come here after normalising register
+ * contents and save locations:
+ *
* R12 = (guest CR << 32) | interrupt vector
* R13 = PACA
- * guest R12 saved in shadow VCPU SCRATCH0
+ * guest R12 saved in shadow HSTATE_SCRATCH0
* guest R13 saved in SPRN_SCRATCH0
*/
std r9,HSTATE_SCRATCH2(r13)
--
2.23.0
^ permalink raw reply related
* [PATCH v1 4/7] KVM: PPC: Book3S 64: Move hcall early register setup to KVM
From: Nicholas Piggin @ 2021-04-12 7:51 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210412075103.1533302-1-npiggin@gmail.com>
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.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/exception-64s.h | 13 ++++++++
arch/powerpc/kernel/exceptions-64s.S | 42 +-----------------------
arch/powerpc/kvm/book3s_64_entry.S | 30 +++++++++++++++++
3 files changed, 44 insertions(+), 41 deletions(-)
diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index c1a8aac01cf9..bb6f78fcf981 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -35,6 +35,19 @@
/* PACA save area size in u64 units (exgen, exmc, etc) */
#define EX_SIZE 10
+/* PACA save area offsets */
+#define EX_R9 0
+#define EX_R10 8
+#define EX_R11 16
+#define EX_R12 24
+#define EX_R13 32
+#define EX_DAR 40
+#define EX_DSISR 48
+#define EX_CCR 52
+#define EX_CFAR 56
+#define EX_PPR 64
+#define EX_CTR 72
+
/*
* maximum recursive depth of MCE exceptions
*/
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 9467fd1038f9..1bfd0d7af09e 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -21,22 +21,6 @@
#include <asm/feature-fixups.h>
#include <asm/kup.h>
-/* PACA save area offsets (exgen, exmc, etc) */
-#define EX_R9 0
-#define EX_R10 8
-#define EX_R11 16
-#define EX_R12 24
-#define EX_R13 32
-#define EX_DAR 40
-#define EX_DSISR 48
-#define EX_CCR 52
-#define EX_CFAR 56
-#define EX_PPR 64
-#define EX_CTR 72
-.if EX_SIZE != 10
- .error "EX_SIZE is wrong"
-.endif
-
/*
* Following are fixed section helper macros.
*
@@ -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:
- * r13 = PACA
- * ctr = orig r13
- * orig r10 saved in PACA
- */
- /*
- * Save the PPR (on systems that support it) before changing to
- * HMT_MEDIUM. That allows the KVM code to save that value into the
- * guest state (it is the guest's PPR value).
- */
-BEGIN_FTR_SECTION
- mfspr r10,SPRN_PPR
- std r10,HSTATE_PPR(r13)
-END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
- HMT_MEDIUM
mfctr r10
- SET_SCRATCH0(r10)
- mfcr r10
- std r12,HSTATE_SCRATCH0(r13)
- sldi r12,r10,32
- ori r12,r12,0xc00
+ SET_SCRATCH0(r10) /* Save r13 in SCRATCH0 */
#ifdef CONFIG_RELOCATABLE
/*
* Requires __LOAD_FAR_HANDLER beause kvmppc_hcall lives
@@ -1994,15 +1957,12 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
*/
__LOAD_FAR_HANDLER(r10, kvmppc_hcall)
mtctr r10
- ld r10,PACA_EXGEN+EX_R10(r13)
bctr
#else
- ld r10,PACA_EXGEN+EX_R10(r13)
b kvmppc_hcall
#endif
#endif
-
/**
* Interrupt 0xd00 - Trace Interrupt.
* This is a synchronous interrupt in response to instruction step or
diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
index c21fa64059ef..f527e16707db 100644
--- a/arch/powerpc/kvm/book3s_64_entry.S
+++ b/arch/powerpc/kvm/book3s_64_entry.S
@@ -14,6 +14,36 @@
.global kvmppc_hcall
.balign IFETCH_ALIGN_BYTES
kvmppc_hcall:
+ /*
+ * This is a hcall, so register convention is as
+ * Documentation/powerpc/papr_hcalls.rst, with these additions:
+ * R13 = PACA
+ * guest R13 saved in SPRN_SCRATCH0
+ * R10 = free
+ * guest r10 saved in PACA_EXGEN
+ *
+ * This may also be a syscall from PR-KVM userspace that is to be
+ * reflected to the PR guest kernel, so registers may be set up for
+ * a system call rather than hcall. We don't currently clobber
+ * anything here, but the 0xc00 handler has already clobbered CTR
+ * and CR0, so PR-KVM can not support a guest kernel that preserves
+ * those registers across its system calls.
+ */
+ /*
+ * Save the PPR (on systems that support it) before changing to
+ * HMT_MEDIUM. That allows the KVM code to save that value into the
+ * guest state (it is the guest's PPR value).
+ */
+BEGIN_FTR_SECTION
+ mfspr r10,SPRN_PPR
+ std r10,HSTATE_PPR(r13)
+END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
+ HMT_MEDIUM
+ mfcr r10
+ std r12,HSTATE_SCRATCH0(r13)
+ sldi r12,r10,32
+ ori r12,r12,0xc00
+ ld r10,PACA_EXGEN+EX_R10(r13)
.global kvmppc_interrupt
.balign IFETCH_ALIGN_BYTES
--
2.23.0
^ permalink raw reply related
* [PATCH v1 3/7] KVM: PPC: Book3S 64: add hcall interrupt handler
From: Nicholas Piggin @ 2021-04-12 7:50 UTC (permalink / raw)
To: kvm-ppc; +Cc: Fabiano Rosas, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <20210412075103.1533302-1-npiggin@gmail.com>
Add a separate hcall entry point. This can be used to deal with the
different calling convention.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 6 +++---
arch/powerpc/kvm/book3s_64_entry.S | 6 +++++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 5842cc3ebbbb..9467fd1038f9 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1989,16 +1989,16 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
ori r12,r12,0xc00
#ifdef CONFIG_RELOCATABLE
/*
- * Requires __LOAD_FAR_HANDLER beause kvmppc_interrupt lives
+ * Requires __LOAD_FAR_HANDLER beause kvmppc_hcall lives
* outside the head section.
*/
- __LOAD_FAR_HANDLER(r10, kvmppc_interrupt)
+ __LOAD_FAR_HANDLER(r10, kvmppc_hcall)
mtctr r10
ld r10,PACA_EXGEN+EX_R10(r13)
bctr
#else
ld r10,PACA_EXGEN+EX_R10(r13)
- b kvmppc_interrupt
+ b kvmppc_hcall
#endif
#endif
diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
index bf927e7a06af..c21fa64059ef 100644
--- a/arch/powerpc/kvm/book3s_64_entry.S
+++ b/arch/powerpc/kvm/book3s_64_entry.S
@@ -8,9 +8,13 @@
#include <asm/reg.h>
/*
- * This is branched to from interrupt handlers in exception-64s.S which set
+ * These are branched to from interrupt handlers in exception-64s.S which set
* IKVM_REAL or IKVM_VIRT, if HSTATE_IN_GUEST was found to be non-zero.
*/
+.global kvmppc_hcall
+.balign IFETCH_ALIGN_BYTES
+kvmppc_hcall:
+
.global kvmppc_interrupt
.balign IFETCH_ALIGN_BYTES
kvmppc_interrupt:
--
2.23.0
^ permalink raw reply related
* [PATCH v1 2/7] KVM: PPC: Book3S 64: Move GUEST_MODE_SKIP test into KVM
From: Nicholas Piggin @ 2021-04-12 7:50 UTC (permalink / raw)
To: kvm-ppc; +Cc: Fabiano Rosas, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <20210412075103.1533302-1-npiggin@gmail.com>
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.
Add a comment explaining the what and why of KVM "skip" interrupts.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 60 ----------------------------
arch/powerpc/kvm/book3s_64_entry.S | 59 ++++++++++++++++++++++++++-
2 files changed, 58 insertions(+), 61 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index bdec40bd92a8..5842cc3ebbbb 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -133,7 +133,6 @@ name:
#define IBRANCH_TO_COMMON .L_IBRANCH_TO_COMMON_\name\() /* ENTRY branch to common */
#define IREALMODE_COMMON .L_IREALMODE_COMMON_\name\() /* Common runs in realmode */
#define IMASK .L_IMASK_\name\() /* IRQ soft-mask bit */
-#define IKVM_SKIP .L_IKVM_SKIP_\name\() /* Generate KVM skip handler */
#define IKVM_REAL .L_IKVM_REAL_\name\() /* Real entry tests KVM */
#define __IKVM_REAL(name) .L_IKVM_REAL_ ## name
#define IKVM_VIRT .L_IKVM_VIRT_\name\() /* Virt entry tests KVM */
@@ -190,9 +189,6 @@ do_define_int n
.ifndef IMASK
IMASK=0
.endif
- .ifndef IKVM_SKIP
- IKVM_SKIP=0
- .endif
.ifndef IKVM_REAL
IKVM_REAL=0
.endif
@@ -250,15 +246,10 @@ do_define_int n
.balign IFETCH_ALIGN_BYTES
\name\()_kvm:
- .if IKVM_SKIP
- cmpwi r10,KVM_GUEST_MODE_SKIP
- beq 89f
- .else
BEGIN_FTR_SECTION
ld r10,IAREA+EX_CFAR(r13)
std r10,HSTATE_CFAR(r13)
END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
- .endif
ld r10,IAREA+EX_CTR(r13)
mtctr r10
@@ -285,27 +276,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
ori r12,r12,(IVEC)
.endif
b kvmppc_interrupt
-
- .if IKVM_SKIP
-89: mtocrf 0x80,r9
- ld r10,IAREA+EX_CTR(r13)
- mtctr r10
- ld r9,IAREA+EX_R9(r13)
- ld r10,IAREA+EX_R10(r13)
- ld r11,IAREA+EX_R11(r13)
- ld r12,IAREA+EX_R12(r13)
- .if IHSRR_IF_HVMODE
- BEGIN_FTR_SECTION
- b kvmppc_skip_Hinterrupt
- FTR_SECTION_ELSE
- b kvmppc_skip_interrupt
- ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE | CPU_FTR_ARCH_206)
- .elseif IHSRR
- b kvmppc_skip_Hinterrupt
- .else
- b kvmppc_skip_interrupt
- .endif
- .endif
.endm
#else
@@ -1083,7 +1053,6 @@ INT_DEFINE_BEGIN(machine_check)
ISET_RI=0
IDAR=1
IDSISR=1
- IKVM_SKIP=1
IKVM_REAL=1
INT_DEFINE_END(machine_check)
@@ -1356,7 +1325,6 @@ INT_DEFINE_BEGIN(data_access)
IVEC=0x300
IDAR=1
IDSISR=1
- IKVM_SKIP=1
IKVM_REAL=1
INT_DEFINE_END(data_access)
@@ -1410,7 +1378,6 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
INT_DEFINE_BEGIN(data_access_slb)
IVEC=0x380
IDAR=1
- IKVM_SKIP=1
IKVM_REAL=1
INT_DEFINE_END(data_access_slb)
@@ -2080,7 +2047,6 @@ INT_DEFINE_BEGIN(h_data_storage)
IHSRR=1
IDAR=1
IDSISR=1
- IKVM_SKIP=1
IKVM_REAL=1
IKVM_VIRT=1
INT_DEFINE_END(h_data_storage)
@@ -3030,32 +2996,6 @@ EXPORT_SYMBOL(do_uaccess_flush)
MASKED_INTERRUPT
MASKED_INTERRUPT hsrr=1
-#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
-kvmppc_skip_interrupt:
- /*
- * Here all GPRs are unchanged from when the interrupt happened
- * except for r13, which is saved in SPRG_SCRATCH0.
- */
- mfspr r13, SPRN_SRR0
- addi r13, r13, 4
- mtspr SPRN_SRR0, r13
- GET_SCRATCH0(r13)
- RFI_TO_KERNEL
- b .
-
-kvmppc_skip_Hinterrupt:
- /*
- * Here all GPRs are unchanged from when the interrupt happened
- * except for r13, which is saved in SPRG_SCRATCH0.
- */
- mfspr r13, SPRN_HSRR0
- addi r13, r13, 4
- mtspr SPRN_HSRR0, r13
- GET_SCRATCH0(r13)
- HRFI_TO_KERNEL
- b .
-#endif
-
/*
* Relocation-on interrupts: A subset of the interrupts can be delivered
* with IR=1/DR=1, if AIL==2 and MSR.HV won't be changed by delivering
diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
index 7a039ea78f15..bf927e7a06af 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
@@ -32,5 +36,58 @@ kvmppc_interrupt:
#endif
b kvmppc_interrupt_hv
#else
+ ld r9,HSTATE_SCRATCH2(r13)
b kvmppc_interrupt_pr
#endif
+
+/*
+ * "Skip" interrupts are part of a trick KVM uses a with hash guests to load
+ * the faulting instruction in guest memory from the the hypervisor without
+ * walking page tables.
+ *
+ * When the guest takes a fault that requires the hypervisor to load the
+ * instruction (e.g., MMIO emulation), KVM is running in real-mode with HV=1
+ * and the guest MMU context loaded. It sets KVM_GUEST_MODE_SKIP, and sets
+ * MSR[DR]=1 while leaving MSR[IR]=0, so it continues to fetch HV instructions
+ * but loads and stores will access the guest context. This is used to load
+ * the faulting instruction using the faulting guest effective address.
+ *
+ * However the guest context may not be able to translate, or it may cause a
+ * machine check or other issue, which results in a fault in the host
+ * (even with KVM-HV).
+ *
+ * These faults come here because KVM_GUEST_MODE_SKIP was set, so if they
+ * are (or are likely) caused by that load, the instruction is skipped by
+ * just returning with the PC advanced +4, where it is noticed the load did
+ * not execute and it goes to the slow path which walks the page tables to
+ * read guest memory.
+ */
+.Lmaybe_skip:
+ cmpwi r12,BOOK3S_INTERRUPT_MACHINE_CHECK
+ beq 1f
+ cmpwi r12,BOOK3S_INTERRUPT_DATA_STORAGE
+ beq 1f
+ cmpwi r12,BOOK3S_INTERRUPT_DATA_SEGMENT
+ beq 1f
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+ /* HSRR interrupts get 2 added to interrupt number */
+ cmpwi r12,BOOK3S_INTERRUPT_H_DATA_STORAGE | 0x2
+ beq 2f
+#endif
+ b .Lno_skip
+1: mfspr r9,SPRN_SRR0
+ addi r9,r9,4
+ mtspr SPRN_SRR0,r9
+ ld r12,HSTATE_SCRATCH0(r13)
+ ld r9,HSTATE_SCRATCH2(r13)
+ GET_SCRATCH0(r13)
+ RFI_TO_KERNEL
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+2: mfspr r9,SPRN_HSRR0
+ addi r9,r9,4
+ mtspr SPRN_HSRR0,r9
+ ld r12,HSTATE_SCRATCH0(r13)
+ ld r9,HSTATE_SCRATCH2(r13)
+ GET_SCRATCH0(r13)
+ HRFI_TO_KERNEL
+#endif
--
2.23.0
^ permalink raw reply related
* [PATCH v1 1/7] KVM: PPC: Book3S 64: move KVM interrupt entry to a common entry point
From: Nicholas Piggin @ 2021-04-12 7:50 UTC (permalink / raw)
To: kvm-ppc; +Cc: Fabiano Rosas, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <20210412075103.1533302-1-npiggin@gmail.com>
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.
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 8 +-----
arch/powerpc/kvm/Makefile | 3 +++
arch/powerpc/kvm/book3s_64_entry.S | 36 +++++++++++++++++++++++++
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 11 ++------
4 files changed, 42 insertions(+), 16 deletions(-)
create mode 100644 arch/powerpc/kvm/book3s_64_entry.S
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 358cd4b0c08e..bdec40bd92a8 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -208,7 +208,6 @@ do_define_int n
.endm
#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
-#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
/*
* All interrupts which set HSRR registers, as well as SRESET and MCE and
* syscall when invoked with "sc 1" switch to MSR[HV]=1 (HVMODE) to be taken,
@@ -238,13 +237,8 @@ do_define_int n
/*
* If an interrupt is taken while a guest is running, it is immediately routed
- * to KVM to handle. If both HV and PR KVM arepossible, KVM interrupts go first
- * to kvmppc_interrupt_hv, which handles the PR guest case.
+ * to KVM to handle.
*/
-#define kvmppc_interrupt kvmppc_interrupt_hv
-#else
-#define kvmppc_interrupt kvmppc_interrupt_pr
-#endif
.macro KVMTEST name
lbz r10,HSTATE_IN_GUEST(r13)
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 2bfeaa13befb..cdd119028f64 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -59,6 +59,9 @@ kvm-pr-y := \
kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
tm.o
+kvm-book3s_64-builtin-objs-y += \
+ book3s_64_entry.o
+
ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
book3s_rmhandlers.o
diff --git a/arch/powerpc/kvm/book3s_64_entry.S b/arch/powerpc/kvm/book3s_64_entry.S
new file mode 100644
index 000000000000..7a039ea78f15
--- /dev/null
+++ b/arch/powerpc/kvm/book3s_64_entry.S
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <asm/asm-offsets.h>
+#include <asm/cache.h>
+#include <asm/kvm_asm.h>
+#include <asm/kvm_book3s_asm.h>
+#include <asm/ppc_asm.h>
+#include <asm/reg.h>
+
+/*
+ * This is branched to from interrupt handlers in exception-64s.S which set
+ * IKVM_REAL or IKVM_VIRT, if HSTATE_IN_GUEST was found to be non-zero.
+ */
+.global kvmppc_interrupt
+.balign IFETCH_ALIGN_BYTES
+kvmppc_interrupt:
+ /*
+ * Register contents:
+ * R12 = (guest CR << 32) | interrupt vector
+ * R13 = PACA
+ * 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_HOST_HV
+ beq kvmppc_bad_host_intr
+#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
+ cmpwi r9,KVM_GUEST_MODE_GUEST
+ ld r9,HSTATE_SCRATCH2(r13)
+ beq kvmppc_interrupt_pr
+#endif
+ b kvmppc_interrupt_hv
+#else
+ b kvmppc_interrupt_pr
+#endif
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 5e634db4809b..f976efb7e4a9 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1269,16 +1269,8 @@ kvmppc_interrupt_hv:
* R13 = PACA
* guest R12 saved in shadow VCPU SCRATCH0
* guest R13 saved in SPRN_SCRATCH0
+ * guest R9 saved in HSTATE_SCRATCH2
*/
- std r9, HSTATE_SCRATCH2(r13)
- lbz r9, HSTATE_IN_GUEST(r13)
- cmpwi r9, KVM_GUEST_MODE_HOST_HV
- beq kvmppc_bad_host_intr
-#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
- cmpwi r9, KVM_GUEST_MODE_GUEST
- ld r9, HSTATE_SCRATCH2(r13)
- beq kvmppc_interrupt_pr
-#endif
/* We're now back in the host but in guest MMU context */
li r9, KVM_GUEST_MODE_HOST_HV
stb r9, HSTATE_IN_GUEST(r13)
@@ -3280,6 +3272,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_P9_TM_HV_ASSIST)
* cfar is saved in HSTATE_CFAR(r13)
* ppr is saved in HSTATE_PPR(r13)
*/
+.global kvmppc_bad_host_intr
kvmppc_bad_host_intr:
/*
* Switch to the emergency stack, but start half-way down in
--
2.23.0
^ permalink raw reply related
* [PATCH v1 0/7] KVM / 64s interrupt handling changes
From: Nicholas Piggin @ 2021-04-12 7:50 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
This is the second batch of patches split from the big KVM in C
series.
This implements all the changes to exception-64s.S required for the
subsequent C conversion. I think they stand on their own as good
patches.
The main things done here are to make the code more amenable to
adding different KVM interrupt handlers (rather than just PR and
HV), and moving a lot of KVM specific code out of exceptions-64s.S
and moving it to arch/powerpc/kvm/book3s_64_entry.S. Calling
convention between those files is changed to mostly match the
exception-64s.S "GEN_INT_ENTRY" convention, so that's a change but
now you only have to remember that one for both cases (either
branching to KVM handler or continuing to GEN_COMMON handler).
This is tested with HV KVM, nested HV under radix L0+L1, and nested
PR KVM under HPT guest. All seems to be working okay.
Thanks,
Nick
Nicholas Piggin (7):
KVM: PPC: Book3S 64: move KVM interrupt entry to a common entry point
KVM: PPC: Book3S 64: Move GUEST_MODE_SKIP test into KVM
KVM: PPC: Book3S 64: add hcall interrupt handler
KVM: PPC: Book3S 64: Move hcall early register setup to KVM
KVM: PPC: Book3S 64: Move interrupt early register setup to KVM
KVM: PPC: Book3S 64: move bad_host_intr check to HV handler
KVM: PPC: Book3S 64: Minimise hcall handler calling convention
differences
arch/powerpc/include/asm/exception-64s.h | 13 ++
arch/powerpc/kernel/exceptions-64s.S | 250 ++++-------------------
arch/powerpc/kvm/Makefile | 3 +
arch/powerpc/kvm/book3s_64_entry.S | 158 ++++++++++++++
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 13 +-
arch/powerpc/kvm/book3s_segment.S | 3 +
6 files changed, 220 insertions(+), 220 deletions(-)
create mode 100644 arch/powerpc/kvm/book3s_64_entry.S
--
2.23.0
^ permalink raw reply
* Re: [PATCH] powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting an overflown PMC
From: Athira Rajeev @ 2021-04-12 7:19 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: nasastry@in.ibm.com, Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <1618195598.pijmcbmr3o.astroid@bobo.none>
[-- Attachment #1: Type: text/html, Size: 29315 bytes --]
^ permalink raw reply
* Re: [PATCH v2 5/5] powerpc/rtas: rename RTAS_RMOBUF_MAX to RTAS_USER_REGION_SIZE
From: Andrew Donnellan @ 2021-04-12 7:45 UTC (permalink / raw)
To: Nathan Lynch, linuxppc-dev; +Cc: tyreld, aik, aneesh.kumar, npiggin, brking
In-Reply-To: <20210408140630.205502-6-nathanl@linux.ibm.com>
On 9/4/21 12:06 am, Nathan Lynch wrote:
> RTAS_RMOBUF_MAX doesn't actually describe a "maximum" value in any
> sense. It represents the size of an area of memory set aside for user
> space to use as work areas for certain RTAS calls.
>
> Rename it to RTAS_USER_REGION_SIZE.
>
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
This appears to correctly rename all users of the macro.
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [RFC/PATCH] powerpc/smp: Add SD_SHARE_PKG_RESOURCES flag to MC sched-domain
From: Srikar Dronamraju @ 2021-04-12 6:24 UTC (permalink / raw)
To: Gautham R. Shenoy
Cc: Parth Shah, Michael Neuling, Vaidyanathan Srinivasan,
Vincent Guittot, Rik van Riel, linuxppc-dev, LKML,
Nicholas Piggin, Dietmar Eggemann, Mel Gorman, Valentin Schneider
In-Reply-To: <1617341874-1205-1-git-send-email-ego@linux.vnet.ibm.com>
* Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2021-04-02 11:07:54]:
>
> To remedy this, this patch proposes that the LLC be moved to the MC
> level which is a group of cores in one half of the chip.
>
> SMT (SMT4) --> MC (Hemisphere)[LLC] --> DIE
>
I think marking Hemisphere as a LLC in a P10 scenario is a good idea.
> While there is no cache being shared at this level, this is still the
> level where some amount of cache-snooping takes place and it is
> relatively faster to access the data from the caches of the cores
> within this domain. With this change, we no longer see regressions on
> P10 for applications which require single threaded performance.
Peter, Valentin, Vincent, Mel, etal
On architectures where we have multiple levels of cache access latencies
within a DIE, (For example: one within the current LLC or SMT core and the
other at MC or Hemisphere, and finally across hemispheres), do you have any
suggestions on how we could handle the same in the core scheduler?
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v2 2/4] powerpc/selftests/perf-hwbreak: Coalesce event creation code
From: Ravi Bangoria @ 2021-04-12 5:03 UTC (permalink / raw)
To: Daniel Axtens, mpe
Cc: linuxppc-dev, mikey, shuah, linux-kselftest, Ravi Bangoria
In-Reply-To: <87k0pbgc18.fsf@linkitivity.dja.id.au>
On 4/9/21 12:49 PM, Daniel Axtens wrote:
> Hi Ravi,
>
>> perf-hwbreak selftest opens hw-breakpoint event at multiple places for
>> which it has same code repeated. Coalesce that code into a function.
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> ---
>> .../selftests/powerpc/ptrace/perf-hwbreak.c | 78 +++++++++----------
>
> This doesn't simplify things very much for the code as it stands now,
> but I think your next patch adds a bunch of calls to these functions, so
> I agree that it makes sense to consolidate them now.
Right. This helps in next patch.
>> 1 file changed, 38 insertions(+), 40 deletions(-)
>>
>> diff --git a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
>> index c1f324afdbf3..bde475341c8a 100644
>> --- a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
>> +++ b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
>> @@ -34,28 +34,46 @@
>>
>> #define DAWR_LENGTH_MAX ((0x3f + 1) * 8)
>>
>> -static inline int sys_perf_event_open(struct perf_event_attr *attr, pid_t pid,
>> - int cpu, int group_fd,
>> - unsigned long flags)
>> +static void perf_event_attr_set(struct perf_event_attr *attr,
>> + __u32 type, __u64 addr, __u64 len,
>> + bool exclude_user)
>> {
>> - attr->size = sizeof(*attr);
>> - return syscall(__NR_perf_event_open, attr, pid, cpu, group_fd, flags);
>> + memset(attr, 0, sizeof(struct perf_event_attr));
>> + attr->type = PERF_TYPE_BREAKPOINT;
>> + attr->size = sizeof(struct perf_event_attr);
>> + attr->bp_type = type;
>> + attr->bp_addr = addr;
>> + attr->bp_len = len;
>> + attr->exclude_kernel = 1;
>> + attr->exclude_hv = 1;
>> + attr->exclude_guest = 1;
>
> Only 1 of the calls to perf sets exclude_{kernel,hv,guest} - I assume
> there's no issue with setting them always but I wanted to check.
True. there is no issue in setting them as this test does all
load/stores in userspace. So all events will be recorded
irrespective of settings of exclude_{kernel,hv,guest}.
>
>> + attr->exclude_user = exclude_user;
>> + attr->disabled = 1;
>> }
>>
>> - /* setup counters */
>> - memset(&attr, 0, sizeof(attr));
>> - attr.disabled = 1;
>> - attr.type = PERF_TYPE_BREAKPOINT;
>> - attr.bp_type = readwriteflag;
>> - attr.bp_addr = (__u64)ptr;
>> - attr.bp_len = sizeof(int);
>> - if (arraytest)
>> - attr.bp_len = DAWR_LENGTH_MAX;
>> - attr.exclude_user = exclude_user;
>> - break_fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
>> + break_fd = perf_process_event_open_exclude_user(readwriteflag, (__u64)ptr,
>> + arraytest ? DAWR_LENGTH_MAX : sizeof(int),
>> + exclude_user);
>
> checkpatch doesn't like this very much:
>
> CHECK: Alignment should match open parenthesis
> #103: FILE: tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c:115:
> + break_fd = perf_process_event_open_exclude_user(readwriteflag, (__u64)ptr,
> + arraytest ? DAWR_LENGTH_MAX : sizeof(int),
Sure will fix it.
>
> Apart from that, this seems good but I haven't checked in super fine
> detail just yet :)
Thanks for the review.
-Ravi
^ permalink raw reply
* Re: [PATCH v2 1/4] powerpc/selftests/ptrace-hwbreak: Add testcases for 2nd DAWR
From: Ravi Bangoria @ 2021-04-12 4:57 UTC (permalink / raw)
To: Daniel Axtens, mpe
Cc: linuxppc-dev, mikey, shuah, linux-kselftest, Ravi Bangoria
In-Reply-To: <87mtu8eyqz.fsf@linkitivity.dja.id.au>
On 4/9/21 12:22 PM, Daniel Axtens wrote:
> Hi Ravi,
>
>> Add selftests to test multiple active DAWRs with ptrace interface.
>
> It would be good if somewhere (maybe in the cover letter) you explain
> what DAWR stands for and where to find more information about it. I
> found the Power ISA v3.1 Book 3 Chapter 9 very helpful.
Sure. Will add the details in v2 cover letter.
> Apart from that, I don't have any specific comments about this patch. It
> looks good to me, it seems to do what it says, and there are no comments
> from checkpatch. It is a bit sparse in terms of comments but it is
> consistent with the rest of the file so I can't really complain there :)
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
Thanks
Ravi
^ permalink raw reply
* Re: [PATCH] powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting an overflown PMC
From: Nicholas Piggin @ 2021-04-12 3:08 UTC (permalink / raw)
To: Athira Rajeev; +Cc: nasastry@in.ibm.com, Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <6F7D0CD6-EA13-4D6F-9592-98CCC4537133@linux.vnet.ibm.com>
Excerpts from Athira Rajeev's message of April 9, 2021 10:53 pm:
>
>
>> On 09-Apr-2021, at 6:38 AM, Nicholas Piggin <npiggin@gmail.com> wrote:
>>
> Hi Nick,
>
> Thanks for checking the patch and sharing review comments.
>
>> I was going to nitpick "overflown" here as something birds do, but some
>> sources says overflown is okay for past tense.
>>
>> You could use "overflowed" for that, but I understand the issue with the
>> word: you are talking about counters that are currently in an "overflow"
>> state, but the overflow occurred in the past and is not still happening
>> so you "overflowing" doesn't exactly fit either.
>>
>> overflown kind of works for some reason you can kind of use it for
>> present tense!
>
> Ok sure, Yes counter is currently in an “overflow” state.
>
>>
>> Excerpts from Athira Rajeev's message of April 7, 2021 12:47 am:
>>> Running perf fuzzer showed below in dmesg logs:
>>> "Can't find PMC that caused IRQ"
>>>
>>> This means a PMU exception happened, but none of the PMC's (Performance
>>> Monitor Counter) were found to be overflown. There are some corner cases
>>> that clears the PMCs after PMI gets masked. In such cases, the perf
>>> interrupt handler will not find the active PMC values that had caused
>>> the overflow and thus leads to this message while replaying.
>>>
>>> Case 1: PMU Interrupt happens during replay of other interrupts and
>>> counter values gets cleared by PMU callbacks before replay:
>>>
>>> During replay of interrupts like timer, __do_irq and doorbell exception, we
>>> conditionally enable interrupts via may_hard_irq_enable(). This could
>>> potentially create a window to generate a PMI. Since irq soft mask is set
>>> to ALL_DISABLED, the PMI will get masked here.
>>
>> I wonder if may_hard_irq_enable shouldn't enable if PMI is soft
>> disabled. And also maybe replay should not set ALL_DISABLED if
>> there are no PMI interrupts pending.
>>
>> Still, I think those are a bit more tricky and might take a while
>> to get right or just not be worth while, so I think your patch is
>> fine.
>
> Ok Nick.
>>
>>> We could get IPIs run before
>>> perf interrupt is replayed and the PMU events could deleted or stopped.
>>> This will change the PMU SPR values and resets the counters. Snippet of
>>> ftrace log showing PMU callbacks invoked in "__do_irq":
>>>
>>> <idle>-0 [051] dns. 132025441306354: __do_irq <-call_do_irq
>>> <idle>-0 [051] dns. 132025441306430: irq_enter <-__do_irq
>>> <idle>-0 [051] dns. 132025441306503: irq_enter_rcu <-__do_irq
>>> <idle>-0 [051] dnH. 132025441306599: xive_get_irq <-__do_irq
>>> <<>>
>>> <idle>-0 [051] dnH. 132025441307770: generic_smp_call_function_single_interrupt <-smp_ipi_demux_relaxed
>>> <idle>-0 [051] dnH. 132025441307839: flush_smp_call_function_queue <-smp_ipi_demux_relaxed
>>> <idle>-0 [051] dnH. 132025441308057: _raw_spin_lock <-event_function
>>> <idle>-0 [051] dnH. 132025441308206: power_pmu_disable <-perf_pmu_disable
>>> <idle>-0 [051] dnH. 132025441308337: power_pmu_del <-event_sched_out
>>> <idle>-0 [051] dnH. 132025441308407: power_pmu_read <-power_pmu_del
>>> <idle>-0 [051] dnH. 132025441308477: read_pmc <-power_pmu_read
>>> <idle>-0 [051] dnH. 132025441308590: isa207_disable_pmc <-power_pmu_del
>>> <idle>-0 [051] dnH. 132025441308663: write_pmc <-power_pmu_del
>>> <idle>-0 [051] dnH. 132025441308787: power_pmu_event_idx <-perf_event_update_userpage
>>> <idle>-0 [051] dnH. 132025441308859: rcu_read_unlock_strict <-perf_event_update_userpage
>>> <idle>-0 [051] dnH. 132025441308975: power_pmu_enable <-perf_pmu_enable
>>> <<>>
>>> <idle>-0 [051] dnH. 132025441311108: irq_exit <-__do_irq
>>> <idle>-0 [051] dns. 132025441311319: performance_monitor_exception <-replay_soft_interrupts
>>>
>>> Case 2: PMI's masked during local_* operations, example local_add.
>>> If the local_add operation happens within a local_irq_save, replay of
>>> PMI will be during local_irq_restore. Similar to case 1, this could
>>> also create a window before replay where PMU events gets deleted or
>>> stopped.
>>
>> Here as well perhaps PMIs should be replayed if they are unmasked
>> even if other interrupts are still masked. Again that might be more
>> complexity than it's worth.
> Ok..
>
>>
>>>
>>> Patch adds a fix to update the PMU callback functions (del,stop,enable) to
>>> check for pending perf interrupt. If there is an overflown PMC and pending
>>> perf interrupt indicated in Paca, clear the PMI bit in paca to drop that
>>> sample. In case of power_pmu_del, also clear the MMCR0 PMAO bit which
>>> otherwise could lead to spurious interrupts in some corner cases. Example,
>>> a timer after power_pmu_del which will re-enable interrupts since PMI is
>>> cleared and triggers a PMI again since PMAO bit is still set.
>>>
>>> We can't just replay PMI any time. Hence this approach is preferred rather
>>> than replaying PMI before resetting overflown PMC. Patch also documents
>>> core-book3s on a race condition which can trigger these PMC messages during
>>> idle path in PowerNV.
>>>
>>> Fixes: f442d004806e ("powerpc/64s: Add support to mask perf interrupts and replay them")
>>> Reported-by: Nageswara R Sastry <nasastry@in.ibm.com>
>>> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
>>> Suggested-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/include/asm/pmc.h | 11 +++++++++
>>> arch/powerpc/perf/core-book3s.c | 55 +++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 66 insertions(+)
>>>
>>> diff --git a/arch/powerpc/include/asm/pmc.h b/arch/powerpc/include/asm/pmc.h
>>> index c6bbe9778d3c..97b4bd8de25b 100644
>>> --- a/arch/powerpc/include/asm/pmc.h
>>> +++ b/arch/powerpc/include/asm/pmc.h
>>> @@ -34,11 +34,22 @@ static inline void ppc_set_pmu_inuse(int inuse)
>>> #endif
>>> }
>>>
>>> +static inline int clear_paca_irq_pmi(void)
>>> +{
>>> + if (get_paca()->irq_happened & PACA_IRQ_PMI) {
>>> + WARN_ON_ONCE(mfmsr() & MSR_EE);
>>> + get_paca()->irq_happened &= ~PACA_IRQ_PMI;
>>> + return 1;
>>> + }
>>> + return 0;
>>> +}
>>
>> Could you put this in arch/powerpc/include/asm/hw_irq.h and
>> rather than paca_irq, call it irq_pending perhaps
>>
>> clear_pmi_irq_pending()
>>
>> get_clear_pmi_irq_pending() if you're also testing it.
>
> Sure, I will use “get_clear_pmi_irq_pending()” and try with moving this to arch/powerpc/include/asm/hw_irq.h
>
>>
>> Could you add a little comment about the corner cases above it too?
>> The root cause seem to be interrupt replay while a masked PMI is
>> pending can result in other interrupts arriving which clear the PMU
>> overflow so the pending PMI must be cleared.
>
> Ok, I will add comment and fix this in next version.
>
>>
>>> +
>>> extern void power4_enable_pmcs(void);
>>>
>>> #else /* CONFIG_PPC64 */
>>>
>>> static inline void ppc_set_pmu_inuse(int inuse) { }
>>> +static inline int clear_paca_irq_pmi(void) { return 0; }
>>>
>>> #endif
>>>
>>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>>> index 766f064f00fb..18ca3c90f866 100644
>>> --- a/arch/powerpc/perf/core-book3s.c
>>> +++ b/arch/powerpc/perf/core-book3s.c
>>> @@ -847,6 +847,20 @@ static void write_pmc(int idx, unsigned long val)
>>> }
>>> }
>>>
>>> +static int pmc_overflown(int idx)
>>> +{
>>> + unsigned long val[8];
>>> + int i;
>>> +
>>> + for (i = 0; i < ppmu->n_counter; i++)
>>> + val[i] = read_pmc(i + 1);
>>> +
>>> + if ((int)val[idx-1] < 0)
>>> + return 1;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> /* Called from sysrq_handle_showregs() */
>>> void perf_event_print_debug(void)
>>> {
>>> @@ -1438,6 +1452,15 @@ static void power_pmu_enable(struct pmu *pmu)
>>> event = cpuhw->event[i];
>>> if (event->hw.idx && event->hw.idx != hwc_index[i] + 1) {
>>> power_pmu_read(event);
>>> + /*
>>> + * if the PMC corresponding to event->hw.idx is
>>> + * overflown, check if there is any pending perf
>>> + * interrupt set in paca. If so, disable the interrupt
>>> + * by clearing the paca bit for PMI since we are going
>>> + * to reset the PMC.
>>> + */
>>> + if (pmc_overflown(event->hw.idx))
>>> + clear_paca_irq_pmi();
>>
>> If the pmc is not overflown, could there still be a PMI pending?
>
> I didn’t hit that scenario where PMI is pending without an overflown PMC.
> Also I believe if such a case happens, we will need an investigation there. It could be a different case to be handled.
Okay, so a PMI will not occur without an overflown PMC, and the
overflown PMC will only be cleared in places where you also clear a
possible pending PMI?
>
> I actually considered below two points for adding this PMC check instead of just clearing the PMI.
>
> 1. Make sure we are not masking any bug here by just clearing PACA_IRQ_PMI.
> Ideally if PMI is set in irq_happened, it means there was a counter overflow.
> 2. If there is more than one PMU event, say two events. Make sure we are clearing PMI only for the
> event whose counter is overflown.
Those are good points. Would you consider also adding a warning for the
case of no PMCs overflown but PMI is pending? That way you might have more
information about such a problem if it ever happens.
We try to add a good deal of warnings around the soft-mask code because
it's very tricky to change without causing more bugs, so even for future
changes to the code this would probably be useful.
>>> @@ -1636,6 +1664,22 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
>>> --cpuhw->n_events;
>>> ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);
>>> if (event->hw.idx) {
>>> + /*
>>> + * if the PMC corresponding to event->hw.idx is
>>> + * overflown, check if there is any pending perf
>>> + * interrupt set in paca. If so, disable the interrupt
>>> + * and clear the MMCR0 PMAO bit since we are going
>>> + * to reset the PMC and delete the event.
>>> + */
>>> + if (pmc_overflown(event->hw.idx)) {
>>> + if (clear_paca_irq_pmi()) {
>>> + val_mmcr0 = mfspr(SPRN_MMCR0);
>>> + val_mmcr0 &= ~MMCR0_PMAO;
>>> + write_mmcr0(cpuhw, val_mmcr0);
>>> + mb();
>>> + isync();
>>
>> I don't know the perf subsystem, but just out of curiosity why does
>> MMCR0 need to be cleared only in this case?
>
> I got a corner case in power_pmu_del, with only clearing PACA_IRQ_PMI and without resetting MMCR0 PMAO bit.
> Here is the flow:
>
> 1. We clear the PMI bit Paca, but MMCR0 has the PMAO bit still set. PMAO bit indicates a PMI has occurred.
> 2. A timer interrupt is replayed after power_pmu_del which does a “may_hard_irq_enable”.
> This will re-enable interrupts and triggers a PMI again since PMAO bit is still set.
>
> So clear PMAO bit to avoid such spurious interrupts.
> Ftrace logs showing the same with some debug trace_printks :
>
> <idle>-0 [134] d.h. 327287888478: power_pmu_del <-event_sched_out.isra.126
> <<>> Here we cleared the PMI
> <idle>-0 [134] d.h. 327287889272: write_pmc <-power_pmu_del
> <idle>-0 [134] d.h. 327287889346: rcu_read_unlock_strict <-perf_event_update_userpage
> <idle>-0 [134] d.h. 327287889711: power_pmu_del: In power_pmu_del MMCR0 is 82004090, local_paca->irq_happened is 9
> <idle>-0 [134] d.h. 327287889811: power_pmu_enable <-perf_pmu_enable
> <idle>-0 [134] d.h. 327287889982: irq_exit <-doorbell_exception
> <idle>-0 [134] d... 327287890053: idle_cpu <-irq_exit
> <idle>-0 [134] d... 327287890158: tick_nohz_irq_exit <-irq_exit
> <idle>-0 [134] d... 327287890219: ktime_get <-tick_nohz_irq_exit
> <idle>-0 [134] d... 327287890328: replay_soft_interrupts <-interrupt_exit_kernel_prepare
> <idle>-0 [134] d... 327287890399: irq_enter <-timer_interrupt
> <<>>
> <idle>-0 [134] d.h. 327287891163: timer_interrupt: Before may_hard_irq_enable MMCR0 is 82004090, local_paca->irq_happened is 1
> <<>>
> <idle>-0 [134] d.h. 327287894310: timer_interrupt: After may_hard_irq_enable MMCR0 is 82004090, local_paca->irq_happened is 21
>
> In case of other callbacks like pmu enable, we are programming MMCR0. But in case of event getting deleted, there is no
> way we clear PMAO unless an event gets scheduled again in that cpu. Hence added this check only in pmu_del callback.
>
>
>> What if we disabled MSR[EE]
>> right before a perf interrupt came in, so we don't get a pending PMI
>> but the condition is still close to the same.
>
> Nick, I didn’t get this question exactly. Can you please help explain a bit ?
> From my understanding, consider that we disabled MSR[EE] before perf interrupt came in.
> So once the interrupts are re-enabled:
>
> 1. If soft mask is set to IRQS_DISABLED, perf interrupt will be triggered as NMI.
> 2. In case of ALL_DISABLED, it will be masked for replay. If PMU callbacks are invoked before replay,
> our present patch will take care of clearing PMI in corner cases.
Well I'm wondering about the same PMAO bug. Above you said:
1. We clear the PMI bit Paca, but MMCR0 has the PMAO bit still set. PMAO bit indicates a PMI has occurred.
2. A timer interrupt is replayed after power_pmu_del which does a “may_hard_irq_enable”.
This will re-enable interrupts and triggers a PMI again since PMAO bit is still set.
So in this situation, what if we had disabled interrupts and that had
caused MSR[EE] to be cleared (let's say due to a PCI interrupt
arriving), and then a PMC overflows and causes PMAO to be set.
Then you run this code:
+ /*
+ * if the PMC corresponding to event->hw.idx is
+ * overflown, check if there is any pending perf
+ * interrupt set in paca. If so, disable the interrupt
+ * and clear the MMCR0 PMAO bit since we are going
+ * to reset the PMC and delete the event.
+ */
+ if (pmc_overflown(event->hw.idx)) {
+ if (clear_paca_irq_pmi()) {
+ val_mmcr0 = mfspr(SPRN_MMCR0);
+ val_mmcr0 &= ~MMCR0_PMAO;
+ write_mmcr0(cpuhw, val_mmcr0);
+ mb();
+ isync();
And this does not clear PMAO because we had no pending PMI, but we still
have the pending PMAO exception.
The only difference was that MSR[EE] happened to be disabled when the
PMC overflowed so no pending PMI was recorded, but otherwise everything
is the same so I wonder why it's not subject to the same problem?
Thanks,
Nick
^ permalink raw reply
* [PATCH v1 12/12] KVM: PPC: Book3S HV: Ensure MSR[HV] is always clear in guest MSR
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
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 is set.
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv_builtin.c | 4 ++--
arch/powerpc/kvm/book3s_hv_nested.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
index 41cb03d0bde4..7a0e33a9c980 100644
--- a/arch/powerpc/kvm/book3s_hv_builtin.c
+++ b/arch/powerpc/kvm/book3s_hv_builtin.c
@@ -662,8 +662,8 @@ static void kvmppc_end_cede(struct kvm_vcpu *vcpu)
void kvmppc_set_msr_hv(struct kvm_vcpu *vcpu, u64 msr)
{
- /* Guest must always run with ME enabled. */
- msr = msr | MSR_ME;
+ /* Guest must always run with ME enabled, HV disabled. */
+ msr = (msr | MSR_ME) & ~MSR_HV;
/*
* Check for illegal transactional state bit combination
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index fb03085c902b..60724f674421 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -344,8 +344,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
vcpu->arch.nested_vcpu_id = l2_hv.vcpu_token;
vcpu->arch.regs = l2_regs;
- /* Guest must always run with ME enabled. */
- vcpu->arch.shregs.msr = vcpu->arch.regs.msr | MSR_ME;
+ /* Guest must always run with ME enabled, HV disabled. */
+ vcpu->arch.shregs.msr = (vcpu->arch.regs.msr | MSR_ME) & ~MSR_HV;
sanitise_hv_regs(vcpu, &l2_hv);
restore_hv_regs(vcpu, &l2_hv);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 11/12] KVM: PPC: Book3S HV: Ensure MSR[ME] is always set in guest MSR
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: Fabiano Rosas, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
Rather than add the ME bit to the MSR at guest entry, make it clear
that the hypervisor does not allow the guest to clear the bit.
The ME set is kept in guest entry for now, but a future patch will
warn if it's not present.
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv_builtin.c | 3 +++
arch/powerpc/kvm/book3s_hv_nested.c | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
index 158d309b42a3..41cb03d0bde4 100644
--- a/arch/powerpc/kvm/book3s_hv_builtin.c
+++ b/arch/powerpc/kvm/book3s_hv_builtin.c
@@ -662,6 +662,9 @@ static void kvmppc_end_cede(struct kvm_vcpu *vcpu)
void kvmppc_set_msr_hv(struct kvm_vcpu *vcpu, u64 msr)
{
+ /* Guest must always run with ME enabled. */
+ msr = msr | MSR_ME;
+
/*
* Check for illegal transactional state bit combination
* and if we find it, force the TS field to a safe state.
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index d14fe32f167b..fb03085c902b 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -343,7 +343,9 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
vcpu->arch.nested = l2;
vcpu->arch.nested_vcpu_id = l2_hv.vcpu_token;
vcpu->arch.regs = l2_regs;
- vcpu->arch.shregs.msr = vcpu->arch.regs.msr;
+
+ /* Guest must always run with ME enabled. */
+ vcpu->arch.shregs.msr = vcpu->arch.regs.msr | MSR_ME;
sanitise_hv_regs(vcpu, &l2_hv);
restore_hv_regs(vcpu, &l2_hv);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 10/12] powerpc/64s: remove KVM SKIP test from instruction breakpoint handler
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: Fabiano Rosas, linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
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).
[Paul notes: "the 0x1300 interrupt was dropped from the architecture a
long time ago and is not generated by P7, P8, P9 or P10." So add a
comment about this in the handler code while we're here. ]
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index a0515cb829c2..358cd4b0c08e 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -2549,11 +2549,16 @@ EXC_REAL_NONE(0x1200, 0x100)
EXC_VIRT_NONE(0x5200, 0x100)
#endif
-
+/**
+ * Interrupt 0x1300 - Instruction Address Breakpoint Interrupt.
+ * This has been removed from the ISA before 2.01, which is the earliest
+ * 64-bit BookS ISA supported, however the G5 / 970 implements this
+ * interrupt with a non-architected feature available through the support
+ * processor interface.
+ */
INT_DEFINE_BEGIN(instruction_breakpoint)
IVEC=0x1300
#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
- IKVM_SKIP=1
IKVM_REAL=1
#endif
INT_DEFINE_END(instruction_breakpoint)
--
2.23.0
^ permalink raw reply related
* [PATCH v1 09/12] powerpc/64s: Remove KVM handler support from CBE_RAS interrupts
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
Cell does not support KVM.
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 6 ------
1 file changed, 6 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 8082b690e874..a0515cb829c2 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -2530,8 +2530,6 @@ EXC_VIRT_NONE(0x5100, 0x100)
INT_DEFINE_BEGIN(cbe_system_error)
IVEC=0x1200
IHSRR=1
- IKVM_SKIP=1
- IKVM_REAL=1
INT_DEFINE_END(cbe_system_error)
EXC_REAL_BEGIN(cbe_system_error, 0x1200, 0x100)
@@ -2701,8 +2699,6 @@ EXC_COMMON_BEGIN(denorm_exception_common)
INT_DEFINE_BEGIN(cbe_maintenance)
IVEC=0x1600
IHSRR=1
- IKVM_SKIP=1
- IKVM_REAL=1
INT_DEFINE_END(cbe_maintenance)
EXC_REAL_BEGIN(cbe_maintenance, 0x1600, 0x100)
@@ -2754,8 +2750,6 @@ EXC_COMMON_BEGIN(altivec_assist_common)
INT_DEFINE_BEGIN(cbe_thermal)
IVEC=0x1800
IHSRR=1
- IKVM_SKIP=1
- IKVM_REAL=1
INT_DEFINE_END(cbe_thermal)
EXC_REAL_BEGIN(cbe_thermal, 0x1800, 0x100)
--
2.23.0
^ permalink raw reply related
* [PATCH v1 08/12] KVM: PPC: Book3S HV: Fix CONFIG_SPAPR_TCE_IOMMU=n default hcalls
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
This config option causes the warning in init_default_hcalls to fire
because the TCE handlers are in the default hcall list but not
implemented.
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index b88df175aa76..4a532410e128 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5412,8 +5412,10 @@ static unsigned int default_hcall_list[] = {
H_READ,
H_PROTECT,
H_BULK_REMOVE,
+#ifdef CONFIG_SPAPR_TCE_IOMMU
H_GET_TCE,
H_PUT_TCE,
+#endif
H_SET_DABR,
H_SET_XDABR,
H_CEDE,
--
2.23.0
^ permalink raw reply related
* [PATCH v1 07/12] KVM: PPC: Book3S HV: remove unused kvmppc_h_protect argument
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Daniel Axtens
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
The va argument is not used in the function or set by its asm caller,
so remove it to be safe.
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/kvm_ppc.h | 3 +--
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 8aacd76bb702..9531b1c1b190 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -767,8 +767,7 @@ long kvmppc_h_remove(struct kvm_vcpu *vcpu, unsigned long flags,
unsigned long pte_index, unsigned long avpn);
long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu);
long kvmppc_h_protect(struct kvm_vcpu *vcpu, unsigned long flags,
- unsigned long pte_index, unsigned long avpn,
- unsigned long va);
+ unsigned long pte_index, unsigned long avpn);
long kvmppc_h_read(struct kvm_vcpu *vcpu, unsigned long flags,
unsigned long pte_index);
long kvmppc_h_clear_ref(struct kvm_vcpu *vcpu, unsigned long flags,
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 88da2764c1bb..7af7c70f1468 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -673,8 +673,7 @@ long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu)
}
long kvmppc_h_protect(struct kvm_vcpu *vcpu, unsigned long flags,
- unsigned long pte_index, unsigned long avpn,
- unsigned long va)
+ unsigned long pte_index, unsigned long avpn)
{
struct kvm *kvm = vcpu->kvm;
__be64 *hpte;
--
2.23.0
^ permalink raw reply related
* [PATCH v1 06/12] KVM: PPC: Book3S HV: Remove redundant mtspr PSPB
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: Daniel Axtens, linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
This SPR is set to 0 twice when exiting the guest.
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 70c6e9c27eb7..b88df175aa76 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3790,7 +3790,6 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
mtspr(SPRN_DSCR, host_dscr);
mtspr(SPRN_TIDR, host_tidr);
mtspr(SPRN_IAMR, host_iamr);
- mtspr(SPRN_PSPB, 0);
if (host_amr != vcpu->arch.amr)
mtspr(SPRN_AMR, host_amr);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 05/12] KVM: PPC: Book3S HV: Prevent radix guests setting LPCR[TC]
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
Prevent radix guests setting LPCR[TC]. This bit only applies to hash
partitions.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 3de8a1f89a7d..70c6e9c27eb7 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1645,6 +1645,10 @@ static int kvm_arch_vcpu_ioctl_set_sregs_hv(struct kvm_vcpu *vcpu,
*/
unsigned long kvmppc_filter_lpcr_hv(struct kvm *kvm, unsigned long lpcr)
{
+ /* LPCR_TC only applies to HPT guests */
+ if (kvm_is_radix(kvm))
+ lpcr &= ~LPCR_TC;
+
/* On POWER8 and above, userspace can modify AIL */
if (!cpu_has_feature(CPU_FTR_ARCH_207S))
lpcr &= ~LPCR_AIL;
--
2.23.0
^ permalink raw reply related
* [PATCH v1 04/12] KVM: PPC: Book3S HV: Disallow LPCR[AIL] to be set to 1 or 2
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
These are already disallowed by H_SET_MODE from the guest, also disallow
these by updating LPCR directly.
AIL modes can affect the host interrupt behaviour while the guest LPCR
value is set, so filter it here too.
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 268e31c7e49c..3de8a1f89a7d 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -803,7 +803,10 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
vcpu->arch.dawrx1 = value2;
return H_SUCCESS;
case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
- /* KVM does not support mflags=2 (AIL=2) */
+ /*
+ * KVM does not support mflags=2 (AIL=2) and AIL=1 is reserved.
+ * Keep this in synch with kvmppc_filter_guest_lpcr_hv.
+ */
if (mflags != 0 && mflags != 3)
return H_UNSUPPORTED_FLAG_START;
return H_TOO_HARD;
@@ -1645,6 +1648,8 @@ unsigned long kvmppc_filter_lpcr_hv(struct kvm *kvm, unsigned long lpcr)
/* On POWER8 and above, userspace can modify AIL */
if (!cpu_has_feature(CPU_FTR_ARCH_207S))
lpcr &= ~LPCR_AIL;
+ if ((lpcr & LPCR_AIL) != LPCR_AIL_3)
+ lpcr &= ~LPCR_AIL; /* LPCR[AIL]=1/2 is disallowed */
/*
* On POWER9, allow userspace to enable large decrementer for the
--
2.23.0
^ permalink raw reply related
* [PATCH v1 03/12] KVM: PPC: Book3S HV: Add a function to filter guest LPCR bits
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
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).
This also adds a warning in other places that set or update LPCR
if we try to set something that would have been disallowed by
the filter, as a sanity check.
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/kvm_book3s.h | 2 +
arch/powerpc/kvm/book3s_hv.c | 68 ++++++++++++++++++++-------
arch/powerpc/kvm/book3s_hv_nested.c | 8 +++-
3 files changed, 59 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index 2f5f919f6cd3..c58121508157 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -258,6 +258,8 @@ extern long kvmppc_hv_get_dirty_log_hpt(struct kvm *kvm,
extern void kvmppc_harvest_vpa_dirty(struct kvmppc_vpa *vpa,
struct kvm_memory_slot *memslot,
unsigned long *map);
+extern unsigned long kvmppc_filter_lpcr_hv(struct kvm *kvm,
+ unsigned long lpcr);
extern void kvmppc_update_lpcr(struct kvm *kvm, unsigned long lpcr,
unsigned long mask);
extern void kvmppc_set_fscr(struct kvm_vcpu *vcpu, u64 fscr);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 208a053c9adf..268e31c7e49c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1635,6 +1635,35 @@ static int kvm_arch_vcpu_ioctl_set_sregs_hv(struct kvm_vcpu *vcpu,
return 0;
}
+/*
+ * Enforce limits on guest LPCR values based on hardware availability,
+ * guest configuration, and possibly hypervisor support and security
+ * concerns.
+ */
+unsigned long kvmppc_filter_lpcr_hv(struct kvm *kvm, unsigned long lpcr)
+{
+ /* On POWER8 and above, userspace can modify AIL */
+ if (!cpu_has_feature(CPU_FTR_ARCH_207S))
+ lpcr &= ~LPCR_AIL;
+
+ /*
+ * On POWER9, allow userspace to enable large decrementer for the
+ * guest, whether or not the host has it enabled.
+ */
+ if (!cpu_has_feature(CPU_FTR_ARCH_300))
+ lpcr &= ~LPCR_LD;
+
+ return lpcr;
+}
+
+static void verify_lpcr(struct kvm *kvm, unsigned long lpcr)
+{
+ if (lpcr != kvmppc_filter_lpcr_hv(kvm, lpcr)) {
+ WARN_ONCE(1, "lpcr 0x%lx differs from filtered 0x%lx\n",
+ lpcr, kvmppc_filter_lpcr_hv(kvm, lpcr));
+ }
+}
+
static void kvmppc_set_lpcr(struct kvm_vcpu *vcpu, u64 new_lpcr,
bool preserve_top32)
{
@@ -1643,6 +1672,23 @@ static void kvmppc_set_lpcr(struct kvm_vcpu *vcpu, u64 new_lpcr,
u64 mask;
spin_lock(&vc->lock);
+
+ /*
+ * Userspace can only modify
+ * DPFD (default prefetch depth), ILE (interrupt little-endian),
+ * TC (translation control), AIL (alternate interrupt location),
+ * LD (large decrementer).
+ * These are subject to restrictions from kvmppc_filter_lcpr_hv().
+ */
+ mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD;
+
+ /* Broken 32-bit version of LPCR must not clear top bits */
+ if (preserve_top32)
+ mask &= 0xFFFFFFFF;
+
+ new_lpcr = kvmppc_filter_lpcr_hv(kvm,
+ (vc->lpcr & ~mask) | (new_lpcr & mask));
+
/*
* If ILE (interrupt little-endian) has changed, update the
* MSR_LE bit in the intr_msr for each vcpu in this vcore.
@@ -1661,25 +1707,8 @@ static void kvmppc_set_lpcr(struct kvm_vcpu *vcpu, u64 new_lpcr,
}
}
- /*
- * Userspace can only modify DPFD (default prefetch depth),
- * ILE (interrupt little-endian) and TC (translation control).
- * On POWER8 and POWER9 userspace can also modify AIL (alt. interrupt loc.).
- */
- mask = LPCR_DPFD | LPCR_ILE | LPCR_TC;
- if (cpu_has_feature(CPU_FTR_ARCH_207S))
- mask |= LPCR_AIL;
- /*
- * On POWER9, allow userspace to enable large decrementer for the
- * guest, whether or not the host has it enabled.
- */
- if (cpu_has_feature(CPU_FTR_ARCH_300))
- mask |= LPCR_LD;
+ vc->lpcr = new_lpcr;
- /* Broken 32-bit version of LPCR must not clear top bits */
- if (preserve_top32)
- mask &= 0xFFFFFFFF;
- vc->lpcr = (vc->lpcr & ~mask) | (new_lpcr & mask);
spin_unlock(&vc->lock);
}
@@ -4644,8 +4673,10 @@ 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;
+ verify_lpcr(kvm, vc->lpcr);
spin_unlock(&vc->lock);
if (++cores_done >= kvm->arch.online_vcores)
break;
@@ -4973,6 +5004,7 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
kvmppc_setup_partition_table(kvm);
}
+ verify_lpcr(kvm, lpcr);
kvm->arch.lpcr = lpcr;
/* Initialization for future HPT resizes */
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 3060e5deffc8..d14fe32f167b 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -151,7 +151,13 @@ static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
*/
mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
LPCR_LPES | LPCR_MER;
- hr->lpcr = (vc->lpcr & ~mask) | (hr->lpcr & mask);
+
+ /*
+ * Additional filtering is required depending on hardware
+ * and configuration.
+ */
+ hr->lpcr = kvmppc_filter_lpcr_hv(vcpu->kvm,
+ (vc->lpcr & ~mask) | (hr->lpcr & mask));
/*
* Don't let L1 enable features for L2 which we've disabled for L1,
--
2.23.0
^ permalink raw reply related
* [PATCH v1 02/12] KVM: PPC: Book3S HV: Nested move LPCR sanitising to sanitise_hv_regs
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
This will get a bit more complicated in future patches. Move it
into the helper function.
This change allows the L1 hypervisor to determine some of the LPCR
bits that the L0 is using to run it, which could be a privilege
violation (LPCR is HV-privileged), although the same problem exists
now for HFSCR for example. Discussion of the HV privilege issue is
ongoing and can be resolved with a later change.
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv_nested.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 0cd0e7aad588..3060e5deffc8 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -132,8 +132,27 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
}
}
+/*
+ * This can result in some L0 HV register state being leaked to an L1
+ * hypervisor when the hv_guest_state is copied back to the guest after
+ * being modified here.
+ *
+ * There is no known problem with such a leak, and in many cases these
+ * register settings could be derived by the guest by observing behaviour
+ * and timing, interrupts, etc., but it is an issue to consider.
+ */
static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
{
+ struct kvmppc_vcore *vc = vcpu->arch.vcore;
+ u64 mask;
+
+ /*
+ * 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;
+ hr->lpcr = (vc->lpcr & ~mask) | (hr->lpcr & mask);
+
/*
* Don't let L1 enable features for L2 which we've disabled for L1,
* but preserve the interrupt cause field.
@@ -271,8 +290,6 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
u64 hv_ptr, regs_ptr;
u64 hdec_exp;
s64 delta_purr, delta_spurr, delta_ic, delta_vtb;
- u64 mask;
- unsigned long lpcr;
if (vcpu->kvm->arch.l1_ptcr == 0)
return H_NOT_AVAILABLE;
@@ -321,9 +338,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
vcpu->arch.nested_vcpu_id = l2_hv.vcpu_token;
vcpu->arch.regs = l2_regs;
vcpu->arch.shregs.msr = vcpu->arch.regs.msr;
- mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
- LPCR_LPES | LPCR_MER;
- lpcr = (vc->lpcr & ~mask) | (l2_hv.lpcr & mask);
+
sanitise_hv_regs(vcpu, &l2_hv);
restore_hv_regs(vcpu, &l2_hv);
@@ -335,7 +350,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
r = RESUME_HOST;
break;
}
- r = kvmhv_run_single_vcpu(vcpu, hdec_exp, lpcr);
+ r = kvmhv_run_single_vcpu(vcpu, hdec_exp, l2_hv.lpcr);
} while (is_kvmppc_resume_guest(r));
/* save L2 state for return */
--
2.23.0
^ permalink raw reply related
* [PATCH v1 00/12] minor KVM fixes and cleanups
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
Here is the first batch of patches are extracted from the patches of the
KVM C conversion series, plus one new fix (host CTRL not restored) since
v6 was posted.
Please consider for merging.
Thanks,
Nick
Nicholas Piggin (12):
KVM: PPC: Book3S HV P9: Restore host CTRL SPR after guest exit
KVM: PPC: Book3S HV: Nested move LPCR sanitising to sanitise_hv_regs
KVM: PPC: Book3S HV: Add a function to filter guest LPCR bits
KVM: PPC: Book3S HV: Disallow LPCR[AIL] to be set to 1 or 2
KVM: PPC: Book3S HV: Prevent radix guests setting LPCR[TC]
KVM: PPC: Book3S HV: Remove redundant mtspr PSPB
KVM: PPC: Book3S HV: remove unused kvmppc_h_protect argument
KVM: PPC: Book3S HV: Fix CONFIG_SPAPR_TCE_IOMMU=n default hcalls
powerpc/64s: Remove KVM handler support from CBE_RAS interrupts
powerpc/64s: remove KVM SKIP test from instruction breakpoint handler
KVM: PPC: Book3S HV: Ensure MSR[ME] is always set in guest MSR
KVM: PPC: Book3S HV: Ensure MSR[HV] is always clear in guest MSR
arch/powerpc/include/asm/kvm_book3s.h | 2 +
arch/powerpc/include/asm/kvm_ppc.h | 3 +-
arch/powerpc/kernel/exceptions-64s.S | 15 +++--
arch/powerpc/kvm/book3s_hv.c | 85 ++++++++++++++++++++-------
arch/powerpc/kvm/book3s_hv_builtin.c | 3 +
arch/powerpc/kvm/book3s_hv_nested.c | 37 +++++++++---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 3 +-
7 files changed, 109 insertions(+), 39 deletions(-)
--
2.23.0
^ permalink raw reply
* [PATCH v1 01/12] KVM: PPC: Book3S HV P9: Restore host CTRL SPR after guest exit
From: Nicholas Piggin @ 2021-04-12 1:48 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210412014845.1517916-1-npiggin@gmail.com>
The host CTRL (runlatch) value is not restored after guest exit. The
host CTRL should always be 1 except in CPU idle code, so this can result
in the host running with runlatch clear, and potentially switching to
a different vCPU which then runs with runlatch clear as well.
This has little effect on P9 machines, CTRL is only responsible for some
PMU counter logic in the host and so other than corner cases of software
relying on that, or explicitly reading the runlatch value (Linux does
not appear to be affected but it's possible non-Linux guests could be),
there should be no execution correctness problem, though it could be
used as a covert channel between guests.
There may be microcontrollers, firmware or monitoring tools that sample
the runlatch value out-of-band, however since the register is writable
by guests, these values would (should) not be relied upon for correct
operation of the host, so suboptimal performance or incorrect reporting
should be the worst problem.
Fixes: 95a6432ce9038 ("KVM: PPC: Book3S HV: Streamlined guest entry/exit path on P9 for radix guests")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 13bad6bf4c95..208a053c9adf 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3728,7 +3728,10 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
vcpu->arch.dec_expires = dec + tb;
vcpu->cpu = -1;
vcpu->arch.thread_cpu = -1;
+ /* Save guest CTRL register, set runlatch to 1 */
vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
+ if (!(vcpu->arch.ctrl & 1))
+ mtspr(SPRN_CTRLT, vcpu->arch.ctrl | 1);
vcpu->arch.iamr = mfspr(SPRN_IAMR);
vcpu->arch.pspb = mfspr(SPRN_PSPB);
--
2.23.0
^ permalink raw reply related
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