HV KVM fixes, reposted
Subsystems (4)
cover HV KVM fixes, reposted
This is a repost of 10 patches out of a series of 12 that I posted more than three weeks ago that have had no comments but have not yet been applied. They have been rediffed against Alex Graf's current kvm-ppc-next branch.
This series contains various fixes collected during the process of getting reboot of Book3S HV guests to work correctly, plus some needed for Ben H's forthcoming series to implement in-kernel XICS (interrupt controller) emulation. As part of getting reboot to work, we have relaxed the previous policy where, on POWER7, a virtual core would only run when all of the vcpus in it were ready to run (or were idle). Now a virtual core will run as soon as any one of its vcpus are ready. This avoids the problem where the guest wouldn't run after reboot because userspace (qemu) had stopped all except cpu 0.
Please apply.
Paul.
PATCH 1/10 KVM: PPC: Book3S HV: Provide a way for userspace to get/set per-vCPU areas
The PAPR paravirtualization interface lets guests register three different types of per-vCPU buffer areas in its memory for communication with the hypervisor. These are called virtual processor areas (VPAs). Currently the hypercalls to register and unregister VPAs are handled by KVM in the kernel, and userspace has no way to know about or save and restore these registrations across a migration.
This adds get and set ioctls to allow userspace to see what addresses have been registered, and to register or unregister them. This will be needed for guest hibernation and migration, and is also needed so that userspace can unregister them on reset (otherwise we corrupt guest memory after reboot by writing to the VPAs registered by the previous kernel). We also add a capability to indicate that the ioctls are supported.
This also fixes a bug where we were calling init_vpa unconditionally, leading to an oops when unregistering the VPA.
--- Documentation/virtual/kvm/api.txt | 32 +++++++++++++++++++++ arch/powerpc/include/asm/kvm_ppc.h | 3 ++ arch/powerpc/kvm/book3s_hv.c | 54 +++++++++++++++++++++++++++++++++++- arch/powerpc/kvm/powerpc.c | 26 +++++++++++++++++ include/linux/kvm.h | 11 ++++++++ 5 files changed, 125 insertions(+), 1 deletion(-)
Documentation/virtual/kvm/api.txt +32 -0
@@ -1992,6 +1992,38 @@ return the hash table order in the parameter. (If the guest is using the virtualized real-mode area (VRMA) facility, the kernel will re-create the VMRA HPTEs on the next KVM_RUN of any vcpu.) +4.77 KVM_PPC_GET_VPA_INFO + +Capability: KVM_CAP_PPC_VPA +Architectures: powerpc +Type: vcpu ioctl +Parameters: Pointer to struct kvm_ppc_vpa (out) +Returns: 0 on success, -1 on error + +This populates and returns a structure containing the guest physical +addresses and sizes of the three per-virtual-processor areas that the +guest can register with the hypervisor under the PAPR +paravirtualization interface, namely the Virtual Processor Area, the +SLB (Segment Lookaside Buffer) Shadow Area, and the Dispatch Trace +Log. + +4.78 KVM_PPC_SET_VPA_INFO + +Capability: KVM_CAP_PPC_VPA +Architectures: powerpc +Type: vcpu ioctl +Parameters: Pointer to struct kvm_ppc_vpa (in) +Returns: 0 on success, -1 on error + +This sets the guest physical addresses and sizes of the three +per-virtual-processor areas that the guest can register with the +hypervisor under the PAPR paravirtualization interface, namely the +Virtual Processor Area, the SLB (Segment Lookaside Buffer) Shadow +Area, and the Dispatch Trace Log. Providing an address of zero for +any of these areas causes the kernel to unregister any previously +registered area; a non-zero address replaces any previously registered +area. + 5. The kvm_run structure ------------------------
arch/powerpc/include/asm/kvm_ppc.h +3 -0
@@ -205,6 +205,9 @@ int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs);int kvm_vcpu_ioctl_get_one_reg (struct kvm_vcpu *vcpu,struct kvm_one_reg *reg);int kvm_vcpu_ioctl_set_one_reg (struct kvm_vcpu *vcpu,struct kvm_one_reg *reg); +int kvm_vcpu_get_vpa_info (struct kvm_vcpu *vcpu,struct kvm_ppc_vpa *vpa); +int kvm_vcpu_set_vpa_info (struct kvm_vcpu *vcpu,struct kvm_ppc_vpa *vpa); +void kvmppc_set_pid (struct kvm_vcpu *vcpu,u32 pid);#ifdef CONFIG_KVM_BOOK3S_64_HV
arch/powerpc/kvm/book3s_hv.c +53 -1
66 lines not on this page · raw message
arch/powerpc/kvm/powerpc.c +26 -0
@@ -349,6 +349,12 @@ int kvm_dev_ioctl_check_extension(long ext) r =1 ;#else r =0 ; +break ; +#endif +#ifdef CONFIG_KVM_BOOK3S_64_HV +case KVM_CAP_PPC_VPA : + r =1 ; +break ;#endif break ;case KVM_CAP_NR_VCPUS :
@@ -826,6 +832,26 @@ long kvm_arch_vcpu_ioctl(struct file *filp, break; }#endif +#ifdef CONFIG_KVM_BOOK3S_64_HV +case KVM_PPC_GET_VPA_INFO : { +struct kvm_ppc_vpa vpa; + r =kvm_vcpu_get_vpa_info (vcpu, &vpa); +if (r) +break ; + r = -EFAULT ; +if (copy_to_user (argp, &vpa,sizeof (vpa))) +break ; + r =0 ; + } +case KVM_PPC_SET_VPA_INFO : { +struct kvm_ppc_vpa vpa; + r = -EFAULT ; +if (copy_from_user (&vpa, argp,sizeof (vpa))) +break ; + r =kvm_vcpu_set_vpa_info (vcpu, &vpa); +break ; + } +#endif default : r = -EINVAL ; }
include/linux/kvm.h +11 -0
@@ -629,6 +629,7 @@ struct kvm_ppc_smmu_info {#define KVM_CAP_READONLY_MEM 81#endif #define KVM_CAP_PPC_BOOKE_WATCHDOG 82 +#define KVM_CAP_PPC_VPA 83#ifdef KVM_CAP_IRQ_ROUTING
@@ -915,6 +916,8 @@ struct kvm_s390_ucas_mapping {#define KVM_SET_ONE_REG _IOW(KVMIO, 0xac, struct kvm_one_reg)/* VM is being stopped by host */ #define KVM_KVMCLOCK_CTRL _IO(KVMIO, 0xad) +#define KVM_PPC_GET_VPA_INFO _IOR(KVMIO, 0xae, struct kvm_ppc_vpa) +#define KVM_PPC_SET_VPA_INFO _IOW(KVMIO, 0xaf, struct kvm_ppc_vpa)#define KVM_DEV_ASSIGN_ENABLE_IOMMU (1 << 0)#define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
@@ -966,4 +969,12 @@ struct kvm_assigned_msix_entry { __u16 padding[3]; }; +struct kvm_ppc_vpa { +__u64 vpa_addr; +__u64 slb_shadow_addr; +__u64 dtl_addr; +__u32 slb_shadow_size; +__u32 dtl_size; +}; +#endif /* __LINUX_KVM_H */
Signature · 1 line
1.7.10
Re: [PATCH 01/10] KVM: PPC: Book3S HV: Provide a way for userspace to get/set per-vCPU areas
Quoted 15 lines · Paul Mackerras
The PAPR paravirtualization interface lets guests register three different types of per-vCPU buffer areas in its memory for communication with the hypervisor. These are called virtual processor areas (VPAs). Currently the hypercalls to register and unregister VPAs are handled by KVM in the kernel, and userspace has no way to know about or save and restore these registrations across a migration.
This adds get and set ioctls to allow userspace to see what addresses have been registered, and to register or unregister them. This will be needed for guest hibernation and migration, and is also needed so that userspace can unregister them on reset (otherwise we corrupt guest memory after reboot by writing to the VPAs registered by the previous kernel). We also add a capability to indicate that the ioctls are supported.
Quoted 4 lines
This also fixes a bug where we were calling
init_vpaunconditionally, leading to an oops when unregistering the VPA.Signed-off-by Paul Mackerras <[email protected]>
Do you think it'd be possible to map these onto ONE_REG as well? I'm slightly wary to add an interface that is restricted to only a limited amount of entries. What if some future PAPR spec wants to add another VPA to the game? We'd have to do a completely new ioctl for that one then.
However, if instead we could have 3 REGs
64-bit VPA_ADDR
128-bit VPA_SLB
128-bit VPA_DTL
where you'd have to set VPA_ADDR first, then the other two. It gives us nice extensibility for the future too, right? We could just add another REG and everyone's happy.
Alex
PATCH 2/10 KVM: PPC: Book3S HV: Allow KVM guests to stop secondary threads coming online
In reply to Paul MackerrasWhen a Book3S HV KVM guest is running, we need the host to be in single-thread mode, that is, all of the cores (or at least all of the cores where the KVM guest could run) to be running only one active hardware thread. This is because of the hardware restriction in POWER processors that all of the hardware threads in the core must be in the same logical partition. Complying with this restriction is much easier if, from the host kernel's point of view, only one hardware thread is active.
This adds two hooks in the SMP hotplug code to allow the KVM code to make sure that secondary threads (i.e. hardware threads other than thread 0) cannot come online while any KVM guest exists. The KVM code still has to check that any core where it runs a guest has the secondary threads offline, but having done that check it can now be sure that they will not come online while the guest is running.
--- arch/powerpc/include/asm/smp.h | 8 +++++++ arch/powerpc/kernel/smp.c | 46 ++++++++++++++++++++++++++++++++++++++++ arch/powerpc/kvm/book3s_hv.c | 12 +++++++++-- 3 files changed, 64 insertions(+), 2 deletions(-)
arch/powerpc/include/asm/smp.h +8 -0
@@ -66,6 +66,14 @@ void generic_cpu_die(unsigned int cpu);void generic_mach_cpu_die (void );void generic_set_cpu_dead (unsigned int cpu);int generic_check_cpu_restart (unsigned int cpu); + +extern void inhibit_secondary_onlining (void ); +extern void uninhibit_secondary_onlining (void ); + +#else /* HOTPLUG_CPU */ +static inline void inhibit_secondary_onlining (void ) {} +static inline void uninhibit_secondary_onlining (void ) {} +#endif #ifdef CONFIG_PPC64
arch/powerpc/kernel/smp.c +46 -0
58 lines not on this page · raw message
arch/powerpc/kvm/book3s_hv.c +10 -2
@@ -47,6 +47,7 @@#include <asm/page.h> #include <asm/hvcall.h> #include <asm/switch_to.h> +#include <asm/smp.h> #include <linux/gfp.h> #include <linux/vmalloc.h> #include <linux/highmem.h>
@@ -918,8 +919,6 @@ static int kvmppc_run_core(struct kvmppc_vcore *vc)/* * Make sure we are running on thread 0, and that * secondary threads are offline. -* XXX we should also block attempts to bring any -* secondary threads online. */ if (threads_per_core >1 && !on_primary_thread ()) {list_for_each_entry (vcpu, &vc->runnable_threads, arch.run_list)
@@ -1632,11 +1631,20 @@ int kvmppc_core_init_vm(struct kvm *kvm) kvm->arch.using_mmu_notifiers = !!cpu_has_feature (CPU_FTR_ARCH_206 );spin_lock_init (&kvm->arch.slot_phys_lock); + +/* +* Don't allow secondary CPU threads to come online +* while any KVM VMs exist. +*/ +inhibit_secondary_onlining (); +return 0 ; }void kvmppc_core_destroy_vm (struct kvm *kvm) { +uninhibit_secondary_onlining (); +if (kvm->arch.rma) {kvm_release_rma (kvm->arch.rma); kvm->arch.rma =NULL ;
Signature · 1 line
1.7.10
Re: [PATCH 02/10] KVM: PPC: Book3S HV: Allow KVM guests to stop secondary threads coming online
Quoted 9 lines · Paul Mackerras
When a Book3S HV KVM guest is running, we need the host to be in single-thread mode, that is, all of the cores (or at least all of the cores where the KVM guest could run) to be running only one active hardware thread. This is because of the hardware restriction in POWER processors that all of the hardware threads in the core must be in the same logical partition. Complying with this restriction is much easier if, from the host kernel's point of view, only one hardware thread is active.
Quoted 8 lines
This adds two hooks in the SMP hotplug code to allow the KVM code to make sure that secondary threads (i.e. hardware threads other than thread 0) cannot come online while any KVM guest exists. The KVM code still has to check that any core where it runs a guest has the secondary threads offline, but having done that check it can now be sure that they will not come online while the guest is running.
Signed-off-by Paul Mackerras <[email protected]>
Ben, since this touches generic ppc code, could you please ack?
Alex
Quoted 134 lines
---
arch/powerpc/include/asm/smp.h | 8 +++++++ arch/powerpc/kernel/smp.c | 46 ++++++++++++++++++++++++++++++++++++++++ arch/powerpc/kvm/book3s_hv.c | 12 +++++++++-- 3 files changed, 64 insertions(+), 2 deletions(-)arch/powerpc/include/asm/smp.h +8
-0@@ -66,6 +66,14 @@ void generic_cpu_die(unsigned int cpu);void generic_mach_cpu_die (void );void generic_set_cpu_dead (unsigned int cpu);int generic_check_cpu_restart (unsigned int cpu); + +extern void inhibit_secondary_onlining (void ); +extern void uninhibit_secondary_onlining (void ); + +#else /* HOTPLUG_CPU */ +static inline void inhibit_secondary_onlining (void ) {} +static inline void uninhibit_secondary_onlining (void ) {} +#endif #ifdef CONFIG_PPC64 arch/powerpc/kernel/smp.c +46
-0@@ -410,6 +410,45 @@ int generic_check_cpu_restart(unsigned int cpu) {return per_cpu (cpu_state, cpu) ==CPU_UP_PREPARE ; } + +static atomic_t secondary_inhibit_count; + +/* +* Don't allow secondary CPU threads to come online +*/ +void inhibit_secondary_onlining (void ) +{ +/* +* This makes secondary_inhibit_count stable during cpu +* online/offline operations. +*/ +get_online_cpus (); + +atomic_inc (&secondary_inhibit_count); +put_online_cpus (); +} +EXPORT_SYMBOL_GPL (inhibit_secondary_onlining); + +/* +* Allow secondary CPU threads to come online again +*/ +void uninhibit_secondary_onlining (void ) +{ +get_online_cpus (); +atomic_dec (&secondary_inhibit_count); +put_online_cpus (); +} +EXPORT_SYMBOL_GPL (uninhibit_secondary_onlining); + +static int secondaries_inhibited (void ) +{ +return atomic_read (&secondary_inhibit_count); +} + +#else /* HOTPLUG_CPU */ + +#define secondaries_inhibited () 0 +#endif static void cpu_idle_thread_init (unsigned int cpu,struct task_struct *idle)@@ -428,6 +467,13 @@ int __cpuinit __cpu_up(unsigned int cpu, struct task_struct *tidle) {int rc, c; +/* +* Don't allow secondary threads to come online if inhibited +*/ +if (threads_per_core >1 &&secondaries_inhibited () && + cpu % threads_per_core !=0 ) +return -EBUSY ; +if (smp_ops ==NULL || (smp_ops->cpu_bootable && !smp_ops->cpu_bootable (cpu)))return -EINVAL ;arch/powerpc/kvm/book3s_hv.c +10
-2@@ -47,6 +47,7 @@#include <asm/page.h> #include <asm/hvcall.h> #include <asm/switch_to.h> +#include <asm/smp.h> #include <linux/gfp.h> #include <linux/vmalloc.h> #include <linux/highmem.h> @@ -918,8 +919,6 @@ static int kvmppc_run_core(struct kvmppc_vcore *vc)/* * Make sure we are running on thread 0, and that * secondary threads are offline. -* XXX we should also block attempts to bring any -* secondary threads online. */ if (threads_per_core >1 && !on_primary_thread ()) {list_for_each_entry (vcpu, &vc->runnable_threads, arch.run_list)@@ -1632,11 +1631,20 @@ int kvmppc_core_init_vm(struct kvm *kvm) kvm->arch.using_mmu_notifiers = !!cpu_has_feature (CPU_FTR_ARCH_206 );spin_lock_init (&kvm->arch.slot_phys_lock); + +/* +* Don't allow secondary CPU threads to come online +* while any KVM VMs exist. +*/ +inhibit_secondary_onlining (); +return 0 ; }void kvmppc_core_destroy_vm (struct kvm *kvm) { +uninhibit_secondary_onlining (); +if (kvm->arch.rma) {kvm_release_rma (kvm->arch.rma); kvm->arch.rma =NULL ;Signature · 1 line
1.7.10
Re: [PATCH 02/10] KVM: PPC: Book3S HV: Allow KVM guests to stop secondary threads coming online
In reply to Paul MackerrasQuoted 9 lines · Paul Mackerras
When a Book3S HV KVM guest is running, we need the host to be in single-thread mode, that is, all of the cores (or at least all of the cores where the KVM guest could run) to be running only one active hardware thread. This is because of the hardware restriction in POWER processors that all of the hardware threads in the core must be in the same logical partition. Complying with this restriction is much easier if, from the host kernel's point of view, only one hardware thread is active.
Quoted 8 lines
This adds two hooks in the SMP hotplug code to allow the KVM code to make sure that secondary threads (i.e. hardware threads other than thread 0) cannot come online while any KVM guest exists. The KVM code still has to check that any core where it runs a guest has the secondary threads offline, but having done that check it can now be sure that they will not come online while the guest is running.
Signed-off-by Paul Mackerras <[email protected]>
Quoted 131 lines
--- arch/powerpc/include/asm/smp.h | 8 +++++++ arch/powerpc/kernel/smp.c | 46 ++++++++++++++++++++++++++++++++++++++++ arch/powerpc/kvm/book3s_hv.c | 12 +++++++++-- 3 files changed, 64 insertions(+), 2 deletions(-)arch/powerpc/include/asm/smp.h +8
-0@@ -66,6 +66,14 @@ void generic_cpu_die(unsigned int cpu);void generic_mach_cpu_die (void );void generic_set_cpu_dead (unsigned int cpu);int generic_check_cpu_restart (unsigned int cpu); + +extern void inhibit_secondary_onlining (void ); +extern void uninhibit_secondary_onlining (void ); + +#else /* HOTPLUG_CPU */ +static inline void inhibit_secondary_onlining (void ) {} +static inline void uninhibit_secondary_onlining (void ) {} +#endif #ifdef CONFIG_PPC64 arch/powerpc/kernel/smp.c +46
-0@@ -410,6 +410,45 @@ int generic_check_cpu_restart(unsigned int cpu) {return per_cpu (cpu_state, cpu) =CPU_UP_PREPARE ; } + +static atomic_t secondary_inhibit_count; + +/* +* Don't allow secondary CPU threads to come online +*/ +void inhibit_secondary_onlining (void ) +{ +/* +* This makes secondary_inhibit_count stable during cpu +* online/offline operations. +*/ +get_online_cpus (); + +atomic_inc (&secondary_inhibit_count); +put_online_cpus (); +} +EXPORT_SYMBOL_GPL (inhibit_secondary_onlining); + +/* +* Allow secondary CPU threads to come online again +*/ +void uninhibit_secondary_onlining (void ) +{ +get_online_cpus (); +atomic_dec (&secondary_inhibit_count); +put_online_cpus (); +} +EXPORT_SYMBOL_GPL (uninhibit_secondary_onlining); + +static int secondaries_inhibited (void ) +{ +return atomic_read (&secondary_inhibit_count); +} + +#else /* HOTPLUG_CPU */ + +#define secondaries_inhibited () 0 +#endif static void cpu_idle_thread_init (unsigned int cpu,struct task_struct *idle)@@ -428,6 +467,13 @@ int __cpuinit __cpu_up(unsigned int cpu, struct task_struct *tidle) {int rc, c; +/* +* Don't allow secondary threads to come online if inhibited +*/ +if (threads_per_core >1 &&secondaries_inhibited () && + cpu % threads_per_core !=0 ) +return -EBUSY ; +if (smp_ops =NULL || (smp_ops->cpu_bootable && !smp_ops->cpu_bootable (cpu)))return -EINVAL ;arch/powerpc/kvm/book3s_hv.c +10
-2@@ -47,6 +47,7 @@#include <asm/page.h> #include <asm/hvcall.h> #include <asm/switch_to.h> +#include <asm/smp.h> #include <linux/gfp.h> #include <linux/vmalloc.h> #include <linux/highmem.h> @@ -918,8 +919,6 @@ static int kvmppc_run_core(struct kvmppc_vcore *vc)/* * Make sure we are running on thread 0, and that * secondary threads are offline. -* XXX we should also block attempts to bring any -* secondary threads online. */ if (threads_per_core >1 && !on_primary_thread ()) {list_for_each_entry (vcpu, &vc->runnable_threads, arch.run_list)@@ -1632,11 +1631,20 @@ int kvmppc_core_init_vm(struct kvm *kvm) kvm->arch.using_mmu_notifiers = !!cpu_has_feature (CPU_FTR_ARCH_206 );spin_lock_init (&kvm->arch.slot_phys_lock); + +/* +* Don't allow secondary CPU threads to come online +* while any KVM VMs exist. +*/ +inhibit_secondary_onlining (); +return 0 ; }void kvmppc_core_destroy_vm (struct kvm *kvm) { +uninhibit_secondary_onlining (); +if (kvm->arch.rma) {kvm_release_rma (kvm->arch.rma); kvm->arch.rma =NULL ;
PATCH 3/10 KVM: PPC: Book3S HV: Fix updates of vcpu->cpu
In reply to Paul MackerrasThis removes the powerpc "generic" updates of vcpu->cpu in load and put, and moves them to the various backends.
The reason is that "HV" KVM does its own sauce with that field and the generic updates might corrupt it. The field contains the CPU# of the -first- HW CPU of the core always for all the VCPU threads of a core (the one that's online from a host Linux perspective).
However, the preempt notifiers are going to be called on the threads VCPUs when they are running (due to them sleeping on our private waitqueue) causing unload to be called, potentially clobbering the value.
--- arch/powerpc/kvm/book3s_pr.c | 3 ++- arch/powerpc/kvm/booke.c | 2 ++ arch/powerpc/kvm/powerpc.c | 2 -- 3 files changed, 4 insertions(+), 3 deletions(-)
arch/powerpc/kvm/book3s_pr.c +2 -1
@@ -64,7 +64,7 @@ void kvmppc_core_vcpu_load(struct kvm_vcpu *vcpu, int cpu) svcpu->slb_max =to_book3s (vcpu)->slb_shadow_max;svcpu_put (svcpu);#endif - + vcpu->cpu =smp_processor_id ();#ifdef CONFIG_PPC_BOOK3S_32 current->thread.kvm_shadow_vcpu =to_book3s (vcpu)->shadow_vcpu;#endif
@@ -84,6 +84,7 @@ void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu)kvmppc_giveup_ext (vcpu,MSR_FP );kvmppc_giveup_ext (vcpu,MSR_VEC );kvmppc_giveup_ext (vcpu,MSR_VSX ); + vcpu->cpu =-1 ; }int kvmppc_core_check_requests (struct kvm_vcpu *vcpu)
arch/powerpc/kvm/booke.c +2 -0
@@ -1509,12 +1509,14 @@ void kvmppc_decrementer_func(unsigned long data)void kvmppc_booke_vcpu_load (struct kvm_vcpu *vcpu,int cpu) { + vcpu->cpu =smp_processor_id (); current->thread.kvm_vcpu = vcpu; }void kvmppc_booke_vcpu_put (struct kvm_vcpu *vcpu) { current->thread.kvm_vcpu =NULL ; + vcpu->cpu =-1 ; }int __init kvmppc_booke_init (void )
arch/powerpc/kvm/powerpc.c +0 -2
@@ -510,7 +510,6 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)mtspr (SPRN_VRSAVE , vcpu->arch.vrsave);#endif kvmppc_core_vcpu_load (vcpu, cpu); - vcpu->cpu =smp_processor_id (); }void kvm_arch_vcpu_put (struct kvm_vcpu *vcpu)
@@ -519,7 +518,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)#ifdef CONFIG_BOOKE vcpu->arch.vrsave =mfspr (SPRN_VRSAVE );#endif - vcpu->cpu =-1 ; } int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
Signature · 1 line
1.7.10
Alexander Graf
AppliedRe: [PATCH 03/10] KVM: PPC: Book3S HV: Fix updates of vcpu->cpu
Quoted 9 lines · Paul Mackerras
This removes the powerpc "generic" updates of
vcpu->cpuin load and put, and moves them to the various backends.The reason is that "HV" KVM does its own sauce with that field and the generic updates might corrupt it. The field contains the CPU# of the -first- HW CPU of the core always for all the VCPU threads of a core (the one that's online from a host Linux perspective).
Quoted 7 lines
However, the preempt notifiers are going to be called on the threads VCPUs when they are running (due to them sleeping on our private waitqueue) causing unload to be called, potentially clobbering the value.
Signed-off-by Benjamin Herrenschmidt <[email protected]>Signed-off-by Paul Mackerras <[email protected]>
Thanks, applied to kvm-ppc-next.
Alex
PATCH 4/10 KVM: PPC: Book3S HV: Remove bogus update of physical thread IDs
In reply to Paul MackerrasWhen making a vcpu non-runnable we incorrectly changed the thread IDs of all other threads on the core, just remove that code.
--- arch/powerpc/kvm/book3s_hv.c | 6 ------ 1 file changed, 6 deletions(-)
arch/powerpc/kvm/book3s_hv.c +0 -6
@@ -759,17 +759,11 @@ extern void xics_wake_cpu(int cpu);static void kvmppc_remove_runnable (struct kvmppc_vcore *vc,struct kvm_vcpu *vcpu) { -struct kvm_vcpu *v; -if (vcpu->arch.state !=KVMPPC_VCPU_RUNNABLE )return ; vcpu->arch.state =KVMPPC_VCPU_BUSY_IN_HOST ; --vc->n_runnable; ++vc->n_busy; -/* decrement the physical thread id of each following vcpu */ - v = vcpu; -list_for_each_entry_continue (v, &vc->runnable_threads, arch.run_list) - --v->arch.ptid;list_del (&vcpu->arch.run_list); }
Signature · 1 line
1.7.10
Alexander Graf
AppliedRe: [PATCH 04/10] KVM: PPC: Book3S HV: Remove bogus update of physical thread IDs
Quoted 6 lines · Paul Mackerras
When making a vcpu non-runnable we incorrectly changed the thread IDs of all other threads on the core, just remove that code.
Signed-off-by Benjamin Herrenschmidt <[email protected]>Signed-off-by Paul Mackerras <[email protected]>
Thanks, applied to kvm-ppc-next.
Alex
PATCH 5/10 KVM: PPC: Book3S HV: Fix some races in starting secondary threads
In reply to Paul MackerrasSubsequent patches implementing in-kernel XICS emulation will make it possible for IPIs to arrive at secondary threads at arbitrary times. This fixes some races in how we start the secondary threads, which if not fixed could lead to occasional crashes of the host kernel.
This makes sure that (a) we have grabbed all the secondary threads, and verified that they are no longer in the kernel, before we start any thread, (b) that the secondary thread loads its vcpu pointer after clearing the IPI that woke it up (so we don't miss a wakeup), and (c) that the secondary thread clears its vcpu pointer before incrementing the nap count. It also removes unnecessary setting of the vcpu and vcore pointers in the paca in kvmppc_core_vcpu_load.
--- arch/powerpc/kvm/book3s_hv.c | 41 ++++++++++++++++++------------- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 11 ++++++--- 2 files changed, 32 insertions(+), 20 deletions(-)
arch/powerpc/kvm/book3s_hv.c +24 -17
@@ -64,8 +64,6 @@ void kvmppc_core_vcpu_load(struct kvm_vcpu *vcpu, int cpu) {struct kvmppc_vcore *vc = vcpu->arch.vcore; - local_paca->kvm_hstate.kvm_vcpu = vcpu; - local_paca->kvm_hstate.kvm_vcore = vc;if (vc->runner = vcpu && vc->vcore_state !=VCORE_INACTIVE ) vc->stolen_tb +=mftb () - vc->preempt_tb; }
@@ -776,6 +774,7 @@ static int kvmppc_grab_hwthread(int cpu)/* Ensure the thread won't go into the kernel if it wakes */ tpaca->kvm_hstate.hwthread_req =1 ; + tpaca->kvm_hstate.kvm_vcpu =NULL ;/* * If the thread is already executing in the kernel (e.g. handling
@@ -825,7 +824,6 @@ static void kvmppc_start_thread(struct kvm_vcpu *vcpu)smp_wmb ();#if defined(CONFIG_PPC_ICP_NATIVE ) && defined(CONFIG_SMP )if (vcpu->arch.ptid) { -kvmppc_grab_hwthread (cpu);xics_wake_cpu (cpu); ++vc->n_woken; }
@@ -851,7 +849,8 @@ static void kvmppc_wait_for_nap(struct kvmppc_vcore *vc)/* * Check that we are on thread 0 and that any other threads in -* this core are off-line. +* this core are off-line. Then grab the threads so they can't +* enter the kernel. */ static int on_primary_thread (void ) {
@@ -863,6 +862,17 @@ static int on_primary_thread(void)while (++thr < threads_per_core)if (cpu_online (cpu + thr))return 0 ; + +/* Grab all hw threads so they can't go into the kernel */ +for (thr =1 ; thr < threads_per_core; ++thr) { +if (kvmppc_grab_hwthread (cpu + thr)) { +/* Couldn't grab one; let the others go */ +do { +kvmppc_release_hwthread (cpu + thr); + }while (--thr >0 ); +return 0 ; + } + }return 1 ; }
@@ -911,16 +921,6 @@ static int kvmppc_run_core(struct kvmppc_vcore *vc) }/* -* Make sure we are running on thread 0, and that -* secondary threads are offline. -*/ -if (threads_per_core >1 && !on_primary_thread ()) { -list_for_each_entry (vcpu, &vc->runnable_threads, arch.run_list) - vcpu->arch.ret = -EBUSY ; - goto out; - } - -/* * Assign physical thread IDs, first to non-ceded vcpus * and then to ceded ones. */
@@ -939,15 +939,22 @@ static int kvmppc_run_core(struct kvmppc_vcore *vc)if (vcpu->arch.ceded) vcpu->arch.ptid = ptid++; +/* +* Make sure we are running on thread 0, and that +* secondary threads are offline. +*/ +if (threads_per_core >1 && !on_primary_thread ()) { +list_for_each_entry (vcpu, &vc->runnable_threads, arch.run_list) + vcpu->arch.ret = -EBUSY ; + goto out; + } + vc->stolen_tb +=mftb () - vc->preempt_tb; vc->pcpu =smp_processor_id ();list_for_each_entry (vcpu, &vc->runnable_threads, arch.run_list) {kvmppc_start_thread (vcpu);kvmppc_create_dtl_entry (vcpu, vc); } -/* Grab any remaining hw threads so they can't go into the kernel */ -for (i = ptid; i < threads_per_core; ++i) -kvmppc_grab_hwthread (vc->pcpu + i);preempt_disable ();spin_unlock (&vc->lock);
arch/powerpc/kvm/book3s_hv_rmhandlers.S +8 -3
@@ -134,8 +134,11 @@ kvm_start_guest: 27: /* XXX should handle hypervisor maintenance interrupts etc. here */ + /* reload vcpu pointer after clearing the IPI */ + ld r4,HSTATE_KVM_VCPU(r13) + cmpdi r4,0 /* if we have no vcpu to run, go back to sleep */ - beq cr1,kvm_no_guest + beq kvm_no_guest /* were we napping due to cede? */ lbz r0,HSTATE_NAPPING(r13)
@@ -1587,6 +1590,10 @@ secondary_too_late: .endr secondary_nap: + /* Clear our vcpu pointer so we don't come back in early */ + li r0, 0 + std r0, HSTATE_KVM_VCPU(r13) + lwsync /* Clear any pending IPI - assume we're a secondary thread */ ld r5, HSTATE_XICS_PHYS(r13) li r7, XICS_XIRR
@@ -1612,8 +1619,6 @@ secondary_nap: kvm_no_guest: li r0, KVM_HWTHREAD_IN_NAP stb r0, HSTATE_HWTHREAD_STATE(r13) - li r0, 0 - std r0, HSTATE_KVM_VCPU(r13) li r3, LPCR_PECE0 mfspr r4, SPRN_LPCR
Signature · 1 line
1.7.10
PATCH 6/10 KVM: PPC: Book3s HV: Don't access runnable threads list without vcore lock
In reply to Paul MackerrasThere were a few places where we were traversing the list of runnable threads in a virtual core, i.e. vc->runnable_threads, without holding the vcore spinlock. This extends the places where we hold the vcore spinlock to cover everywhere that we traverse that list.
Since we possibly need to sleep inside kvmppc_book3s_hv_page_fault, this moves the call of it from kvmppc_handle_exit out to kvmppc_vcpu_run, where we don't hold the vcore lock.
In kvmppc_vcore_blocked, we don't actually need to check whether all vcpus are ceded and don't have any pending exceptions, since the caller has already done that. The caller (kvmppc_run_vcpu) wasn't actually checking for pending exceptions, so we add that.
The change of if to while in kvmppc_run_vcpu is to make sure that we never call kvmppc_remove_runnable() when the vcore state is RUNNING or EXITING.
--- arch/powerpc/include/asm/kvm_asm.h | 1 + arch/powerpc/kvm/book3s_hv.c | 64 +++++++++++++++++------------------- 2 files changed, 31 insertions(+), 34 deletions(-)
arch/powerpc/include/asm/kvm_asm.h +1 -0
@@ -123,6 +123,7 @@#define RESUME_GUEST_NV RESUME_FLAG_NV#define RESUME_HOST RESUME_FLAG_HOST#define RESUME_HOST_NV (RESUME_FLAG_HOST|RESUME_FLAG_NV) +#define RESUME_PAGE_FAULT (1<<2)#define KVM_GUEST_MODE_NONE 0#define KVM_GUEST_MODE_GUEST 1
arch/powerpc/kvm/book3s_hv.c +30 -34
159 lines not on this page · raw message
Signature · 1 line
1.7.10
Re: [PATCH 06/10] KVM: PPC: Book3s HV: Don't access runnable threads list without vcore lock
Quoted 29 lines · Paul Mackerras
There were a few places where we were traversing the list of runnable threads in a virtual core, i.e.
vc->runnable_threads, without holding the vcore spinlock. This extends the places where we hold the vcore spinlock to cover everywhere that we traverse that list.Since we possibly need to sleep inside
kvmppc_book3s_hv_page_fault, this moves the call of it fromkvmppc_handle_exitout tokvmppc_vcpu_run, where we don't hold the vcore lock.In
kvmppc_vcore_blocked, we don't actually need to check whether all vcpus are ceded and don't have any pending exceptions, since the caller has already done that. The caller (kvmppc_run_vcpu) wasn't actually checking for pending exceptions, so we add that.The change of if to while in
kvmppc_run_vcpuis to make sure that we never callkvmppc_remove_runnable()when the vcore state is RUNNING or EXITING.Signed-off-by Paul Mackerras <[email protected]>---
arch/powerpc/include/asm/kvm_asm.h| 1 +arch/powerpc/kvm/book3s_hv.c| 64 +++++++++++++++++-------------------2 files changed, 31 insertions(+), 34 deletions(-)arch/powerpc/include/asm/kvm_asm.h +0
-0
arch/powerpc/include/asm/kvm_asm.h line 126
Quoted 4 lines
arch/powerpc/include/asm/kvm_asm.h +1
-0@@ -123,6 +123,7 @@#define RESUME_GUEST_NV RESUME_FLAG_NV#define RESUME_HOST RESUME_FLAG_HOST#define RESUME_HOST_NV (RESUME_FLAG_HOST|RESUME_FLAG_NV) +#define RESUME_PAGE_FAULT (1<<2)
I would actually prefer if you could move this to core specific code. How about
#define RESUME_ARCH1 (1 << 2)
and then in book3s_hv.c:
#define RESUME_PAGE_FAULT (RESUME_GUEST | RESUME_ARCH1)
Alex
PATCH v2 6/10 KVM: PPC: Book3S HV: Don't access runnable threads list without vcore lock
There were a few places where we were traversing the list of runnable threads in a virtual core, i.e. vc->runnable_threads, without holding the vcore spinlock. This extends the places where we hold the vcore spinlock to cover everywhere that we traverse that list.
Since we possibly need to sleep inside kvmppc_book3s_hv_page_fault, this moves the call of it from kvmppc_handle_exit out to kvmppc_vcpu_run, where we don't hold the vcore lock.
In kvmppc_vcore_blocked, we don't actually need to check whether all vcpus are ceded and don't have any pending exceptions, since the caller has already done that. The caller (kvmppc_run_vcpu) wasn't actually checking for pending exceptions, so we add that.
The change of if to while in kvmppc_run_vcpu is to make sure that we never call kvmppc_remove_runnable() when the vcore state is RUNNING or EXITING.
---
v2: move RESUME_PAGE_FAULT defn to book3s_hv.c
arch/powerpc/include/asm/kvm_asm.h | 1 + arch/powerpc/kvm/book3s_hv.c | 67 ++++++++++++++++++------------------ 2 files changed, 34 insertions(+), 34 deletions(-)
arch/powerpc/include/asm/kvm_asm.h +1 -0
@@ -118,6 +118,7 @@#define RESUME_FLAG_NV (1<<0)/* Reload guest nonvolatile state? */ #define RESUME_FLAG_HOST (1<<1)/* Resume host? */ +#define RESUME_FLAG_ARCH1 (1<<2)#define RESUME_GUEST 0#define RESUME_GUEST_NV RESUME_FLAG_NV
arch/powerpc/kvm/book3s_hv.c +33 -34
168 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH 7/10 KVM: PPC: Book3S HV: Fixes for late-joining threads
In reply to Paul MackerrasIf a thread in a virtual core becomes runnable while other threads in the same virtual core are already running in the guest, it is possible for the latecomer to join the others on the core without first pulling them all out of the guest. Currently this only happens rarely, when a vcpu is first started. This fixes some bugs and omissions in the code in this case.
First, we need to check for VPA updates for the latecomer and make a DTL entry for it. Secondly, if it comes along while the master vcpu is doing a VPA update, we don't need to do anything since the master will pick it up in kvmppc_run_core. To handle this correctly we introduce a new vcore state, VCORE_STARTING. Thirdly, there is a race because we currently clear the hardware thread's hwthread_req before waiting to see it get to nap. A latecomer thread could have its hwthread_req cleared before it gets to test it, and therefore never increment the nap_count, leading to messages about wait_for_nap timeouts.
--- arch/powerpc/include/asm/kvm_host.h | 7 ++++--- arch/powerpc/kvm/book3s_hv.c | 14 +++++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-)
arch/powerpc/include/asm/kvm_host.h +4 -3
@@ -289,9 +289,10 @@ struct kvmppc_vcore {/* Values for vcore_state */ #define VCORE_INACTIVE 0 -#define VCORE_RUNNING 1 -#define VCORE_EXITING 2 -#define VCORE_SLEEPING 3 +#define VCORE_SLEEPING 1 +#define VCORE_STARTING 2 +#define VCORE_RUNNING 3 +#define VCORE_EXITING 4/* * Struct used to manage memory for a virtual processor area
arch/powerpc/kvm/book3s_hv.c +11 -3
@@ -368,6 +368,11 @@ static void kvmppc_update_vpa(struct kvm_vcpu *vcpu, struct kvmppc_vpa *vpap)static void kvmppc_update_vpas (struct kvm_vcpu *vcpu) { +if (!(vcpu->arch.vpa.update_pending || + vcpu->arch.slb_shadow.update_pending || + vcpu->arch.dtl.update_pending)) +return ; +spin_lock (&vcpu->arch.vpa_update_lock);if (vcpu->arch.vpa.update_pending) {kvmppc_update_vpa (vcpu, &vcpu->arch.vpa);
@@ -902,7 +907,7 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc) vc->n_woken =0 ; vc->nap_count =0 ; vc->entry_exit_count =0 ; - vc->vcore_state =VCORE_RUNNING ; + vc->vcore_state =VCORE_STARTING ; vc->in_guest =0 ; vc->napping_threads =0 ;
@@ -955,6 +960,7 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)kvmppc_create_dtl_entry (vcpu, vc); } + vc->vcore_state =VCORE_RUNNING ;preempt_disable ();spin_unlock (&vc->lock);
@@ -963,8 +969,6 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc) srcu_idx =srcu_read_lock (&vcpu0->kvm->srcu);__kvmppc_vcore_entry (NULL , vcpu0); -for (i =0 ; i < threads_per_core; ++i) -kvmppc_release_hwthread (vc->pcpu + i);spin_lock (&vc->lock);/* disable sending of IPIs on virtual external irqs */
@@ -973,6 +977,8 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)/* wait for secondary threads to finish writing their state to memory */ if (vc->nap_count < vc->n_woken)kvmppc_wait_for_nap (vc); +for (i =0 ; i < threads_per_core; ++i) +kvmppc_release_hwthread (vc->pcpu + i);/* prevent other vcpu threads from doing kvmppc_start_thread() now */ vc->vcore_state =VCORE_EXITING ;spin_unlock (&vc->lock);
@@ -1063,6 +1069,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) kvm_run->exit_reason =0 ; vcpu->arch.ret =RESUME_GUEST ; vcpu->arch.trap =0 ; +kvmppc_update_vpas (vcpu);/* * Synchronize with other threads in this virtual core
@@ -1086,6 +1093,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)if (vc->vcore_state ==VCORE_RUNNING &&VCORE_EXIT_COUNT (vc) ==0 ) { vcpu->arch.ptid = vc->n_runnable -1 ; +kvmppc_create_dtl_entry (vcpu, vc);kvmppc_start_thread (vcpu); }
Signature · 1 line
1.7.10
PATCH 8/10 KVM: PPC: Book3S HV: Run virtual core whenever any vcpus in it can run
In reply to Paul MackerrasCurrently the Book3S HV code implements a policy on multi-threaded processors (i.e. POWER7) that requires all of the active vcpus in a virtual core to be ready to run before we run the virtual core. However, that causes problems on reset, because reset stops all vcpus except vcpu 0, and can also reduce throughput since all four threads in a virtual core have to wait whenever any one of them hits a hypervisor page fault.
This relaxes the policy, allowing the virtual core to run as soon as any vcpu in it is runnable. With this, the KVMPPC_VCPU_STOPPED state and the KVMPPC_VCPU_BUSY_IN_HOST state have been combined into a single KVMPPC_VCPU_NOTREADY state, since we no longer need to distinguish between them.
--- arch/powerpc/include/asm/kvm_host.h | 5 +-- arch/powerpc/kvm/book3s_hv.c | 74 ++++++++++++++++++----------------- 2 files changed, 40 insertions(+), 39 deletions(-)
arch/powerpc/include/asm/kvm_host.h +2 -3
@@ -563,9 +563,8 @@ struct kvm_vcpu_arch { };/* Values for vcpu->arch.state */ -#define KVMPPC_VCPU_STOPPED 0 -#define KVMPPC_VCPU_BUSY_IN_HOST 1 -#define KVMPPC_VCPU_RUNNABLE 2 +#define KVMPPC_VCPU_NOTREADY 0 +#define KVMPPC_VCPU_RUNNABLE 1/* Values for vcpu->arch.io_gpr */ #define KVM_MMIO_REG_MASK 0x001f
arch/powerpc/kvm/book3s_hv.c +38 -36
129 lines not on this page · raw message
Signature · 1 line
1.7.10
PATCH 9/10 KVM: PPC: Book3S HV: Fix accounting of stolen time
In reply to Paul MackerrasCurrently the code that accounts stolen time tends to overestimate the stolen time, and will sometimes report more stolen time in a DTL (dispatch trace log) entry than has elapsed since the last DTL entry. This can cause guests to underflow the user or system time measured for some tasks, leading to ridiculous CPU percentages and total runtimes being reported by top and other utilities.
In addition, the current code was designed for the previous policy where a vcore would only run when all the vcpus in it were runnable, and so only counted stolen time on a per-vcore basis. Now that a vcore can run while some of the vcpus in it are doing other things in the kernel (e.g. handling a page fault), we need to count the time when a vcpu task is preempted while it is not running as part of a vcore as stolen also.
To do this, we bring back the BUSY_IN_HOST vcpu state and extend the vcpu_load/put functions to count preemption time while the vcpu is in that state. Handling the transitions between the RUNNING and BUSY_IN_HOST states requires checking and updating two variables (accumulated time stolen and time last preempted), so we add a new spinlock, vcpu->arch.tbacct_lock. This protects both the per-vcpu stolen/preempt-time variables, and the per-vcore variables while this vcpu is running the vcore.
Finally, we now don't count time spent in userspace as stolen time. The task could be executing in userspace on behalf of the vcpu, or it could be preempted, or the vcpu could be genuinely stopped. Since we have no way of dividing up the time between these cases, we don't count any of it as stolen.
--- arch/powerpc/include/asm/kvm_host.h | 5 ++ arch/powerpc/kvm/book3s_hv.c | 127 ++++++++++++++++++++++++++++++----- 2 files changed, 117 insertions(+), 15 deletions(-)
arch/powerpc/include/asm/kvm_host.h +5 -0
@@ -559,12 +559,17 @@ struct kvm_vcpu_arch {unsigned long dtl_index;u64 stolen_logged;struct kvmppc_vpa slb_shadow; + +spinlock_t tbacct_lock; +u64 busy_stolen; +u64 busy_preempt;#endif };/* Values for vcpu->arch.state */ #define KVMPPC_VCPU_NOTREADY 0#define KVMPPC_VCPU_RUNNABLE 1 +#define KVMPPC_VCPU_BUSY_IN_HOST 2/* Values for vcpu->arch.io_gpr */ #define KVM_MMIO_REG_MASK 0x001f
arch/powerpc/kvm/book3s_hv.c +112 -15
231 lines not on this page · raw message
Signature · 1 line
1.7.10
PATCH v2 9/10 KVM: PPC: Book3S HV: Fix accounting of stolen time
Currently the code that accounts stolen time tends to overestimate the stolen time, and will sometimes report more stolen time in a DTL (dispatch trace log) entry than has elapsed since the last DTL entry. This can cause guests to underflow the user or system time measured for some tasks, leading to ridiculous CPU percentages and total runtimes being reported by top and other utilities.
In addition, the current code was designed for the previous policy where a vcore would only run when all the vcpus in it were runnable, and so only counted stolen time on a per-vcore basis. Now that a vcore can run while some of the vcpus in it are doing other things in the kernel (e.g. handling a page fault), we need to count the time when a vcpu task is preempted while it is not running as part of a vcore as stolen also.
To do this, we bring back the BUSY_IN_HOST vcpu state and extend the vcpu_load/put functions to count preemption time while the vcpu is in that state. Handling the transitions between the RUNNING and BUSY_IN_HOST states requires checking and updating two variables (accumulated time stolen and time last preempted), so we add a new spinlock, vcpu->arch.tbacct_lock. This protects both the per-vcpu stolen/preempt-time variables, and the per-vcore variables while this vcpu is running the vcore.
Finally, we now don't count time spent in userspace as stolen time. The task could be executing in userspace on behalf of the vcpu, or it could be preempted, or the vcpu could be genuinely stopped. Since we have no way of dividing up the time between these cases, we don't count any of it as stolen.
---
v2: just rediffed for context changes, no real change from previous version.
arch/powerpc/include/asm/kvm_host.h | 5 ++ arch/powerpc/kvm/book3s_hv.c | 127 ++++++++++++++++++++++++++++++----- 2 files changed, 117 insertions(+), 15 deletions(-)
arch/powerpc/include/asm/kvm_host.h +5 -0
@@ -559,12 +559,17 @@ struct kvm_vcpu_arch {unsigned long dtl_index;u64 stolen_logged;struct kvmppc_vpa slb_shadow; + +spinlock_t tbacct_lock; +u64 busy_stolen; +u64 busy_preempt;#endif };/* Values for vcpu->arch.state */ #define KVMPPC_VCPU_NOTREADY 0#define KVMPPC_VCPU_RUNNABLE 1 +#define KVMPPC_VCPU_BUSY_IN_HOST 2/* Values for vcpu->arch.io_gpr */ #define KVM_MMIO_REG_MASK 0x001f
arch/powerpc/kvm/book3s_hv.c +112 -15
231 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH 10/10 KVM: PPC: Book3S HV: Fix calculation of guest phys address for MMIO emulation
In reply to Paul MackerrasIn the case where the host kernel is using a 64kB base page size and the guest uses a 4k HPTE (hashed page table entry) to map an emulated MMIO device, we were calculating the guest physical address wrongly. We were calculating a gfn as the guest physical address shifted right 16 bits (PAGE_SHIFT) but then only adding back in 12 bits from the effective address, since the HPTE had a 4k page size. Thus the gpa reported to userspace was missing 4 bits.
Instead, we now compute the guest physical address from the HPTE without reference to the host page size, and then compute the gfn by shifting the gpa right PAGE_SHIFT bits.
--- arch/powerpc/kvm/book3s_64_mmu_hv.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
arch/powerpc/kvm/book3s_64_mmu_hv.c +4 -5
@@ -571,7 +571,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,struct kvm *kvm = vcpu->kvm;unsigned long *hptep, hpte[3 ], r;unsigned long mmu_seq, psize, pte_size; -unsigned long gfn, hva, pfn; +unsigned long gpa, gfn, hva, pfn;struct kvm_memory_slot *memslot;unsigned long *rmap;struct revmap_entry *rev;
@@ -609,15 +609,14 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,/* Translate the logical address and get the page */ psize =hpte_page_size (hpte[0 ], r); - gfn =hpte_rpn (r, psize); + gpa = (r &HPTE_R_RPN & ~(psize -1 )) | (ea & (psize -1 )); + gfn = gpa >>PAGE_SHIFT ; memslot =gfn_to_memslot (kvm, gfn);/* No memslot means it's an emulated MMIO region */ -if (!memslot || (memslot->flags &KVM_MEMSLOT_INVALID )) { -unsigned long gpa = (gfn <<PAGE_SHIFT ) | (ea & (psize -1 )); +if (!memslot || (memslot->flags &KVM_MEMSLOT_INVALID ))return kvmppc_hv_emulate_mmio (run, vcpu, gpa, ea, dsisr &DSISR_ISSTORE ); - }if (!kvm->arch.using_mmu_notifiers)return -EFAULT ;/* should never get here */
Signature · 1 line
1.7.10
Alexander Graf
AppliedRe: [PATCH 10/10] KVM: PPC: Book3S HV: Fix calculation of guest phys address for MMIO emulation
Quoted 8 lines · Paul Mackerras
In the case where the host kernel is using a 64kB base page size and the guest uses a 4k HPTE (hashed page table entry) to map an emulated MMIO device, we were calculating the guest physical address wrongly. We were calculating a gfn as the guest physical address shifted right 16 bits (
PAGE_SHIFT) but then only adding back in 12 bits from the effective address, since the HPTE had a 4k page size. Thus the gpa reported to userspace was missing 4 bits.
Quoted 6 lines
Instead, we now compute the guest physical address from the HPTE without reference to the host page size, and then compute the gfn by shifting the gpa right
PAGE_SHIFTbits.Reported-by Alexey Kardashevskiy <[email protected]>Signed-off-by Paul Mackerras <[email protected]>
Thanks, applied to kvm-ppc-next.
Alex