RE: [PATCH 1/2] powerpc/booke64: Add LRAT error exception handler
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, July 03, 2013 11:18 PM To: Caraman Mihai Claudiu-B02008 Cc: linuxppc-dev@lists.ozlabs.org; kvm-...@vger.kernel.org; k...@vger.kernel.org; Caraman Mihai Claudiu-B02008 Subject: Re: [PATCH 1/2] powerpc/booke64: Add LRAT error exception handler On 07/03/2013 11:56:05 AM, Mihai Caraman wrote: @@ -1410,6 +1423,7 @@ _GLOBAL(setup_doorbell_ivors) _GLOBAL(setup_ehv_ivors) SET_IVOR(40, 0x300) /* Embedded Hypervisor System Call */ SET_IVOR(41, 0x320) /* Embedded Hypervisor Privilege */ + SET_IVOR(42, 0x340) /* LRAT Error */ What happens if we write to IVOR42 on e5500? If the answer is no-op, is that behavior guaranteed on any CPU with E.HV but not LRAT? Oops. I would rather do it __setup_cpu_e6500 in the same way we deal with AltiVec. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] KVM: PPC: Book3E: Add LRAT error exception handler
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, July 03, 2013 11:17 PM To: Caraman Mihai Claudiu-B02008 Cc: linuxppc-dev@lists.ozlabs.org; kvm-...@vger.kernel.org; k...@vger.kernel.org; Caraman Mihai Claudiu-B02008 Subject: Re: [PATCH 2/2] KVM: PPC: Book3E: Add LRAT error exception handler On 07/03/2013 11:56:06 AM, Mihai Caraman wrote: With LRAT (Logical to Real Address Translation) error exception handler in kernel KVM needs to add the counterpart otherwise will break the build. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/bookehv_interrupts.S |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) Please combine these two patches to avoid breaking bisectability. -Scott This is a solid reason. Ben it's ok for you to apply the combined patch? If so I will respin it. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/6] KVM: PPC: Book3E: Increase FPU laziness
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Wednesday, July 03, 2013 9:40 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 3/6] KVM: PPC: Book3E: Increase FPU laziness On 03.07.2013, at 20:37, Scott Wood wrote: On 07/03/2013 07:42:36 AM, Mihai Caraman wrote: Increase FPU laziness by calling kvmppc_load_guest_fp() just before returning to guest instead of each sched in. Without this improvement an interrupt may also claim floting point corrupting guest state. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/booke.c |1 + arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 1 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 113961f..3cae2e3 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -1204,6 +1204,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, r = (s 2) | RESUME_HOST | (r RESUME_FLAG_NV); } else { kvmppc_lazy_ee_enable(); + kvmppc_load_guest_fp(vcpu); } } diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index 19c8379..09da1ac 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -143,8 +143,6 @@ void kvmppc_core_vcpu_load(struct kvm_vcpu *vcpu, int cpu) kvmppc_e500_tlbil_all(vcpu_e500); __get_cpu_var(last_vcpu_on_cpu) = vcpu; } - - kvmppc_load_guest_fp(vcpu); } void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu) Can we now remove vcpu-fpu_active, and the comment that says Kernel usage of FP (via enable_kernel_fp()) in this thread must not occur while vcpu- fpu_active is set.? I think so, yes. Yes, as I already did this for AltiVec. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 5/6] KVM: PPC: Book3E: Add ONE_REG AltiVec support
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, June 05, 2013 1:40 AM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; Caraman Mihai Claudiu-B02008 Subject: Re: [RFC PATCH 5/6] KVM: PPC: Book3E: Add ONE_REG AltiVec support On 06/03/2013 03:54:27 PM, Mihai Caraman wrote: Add ONE_REG support for AltiVec on Book3E. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/booke.c | 32 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 01eb635..019496d 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -1570,6 +1570,22 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, struct kvm_one_reg *reg) case KVM_REG_PPC_DEBUG_INST: val = get_reg_val(reg-id, KVMPPC_INST_EHPRIV); break; +#ifdef CONFIG_ALTIVEC + case KVM_REG_PPC_VR0 ... KVM_REG_PPC_VR31: + if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { + r = -ENXIO; + break; + } + val.vval = vcpu-arch.vr[reg-id - KVM_REG_PPC_VR0]; + break; + case KVM_REG_PPC_VSCR: + if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { + r = -ENXIO; + break; + } + val = get_reg_val(reg-id, vcpu-arch.vscr.u[3]); + break; Why u[3]? AltiVec PEM manual says: The VSCR has two defined bits, the AltiVec non-Java mode (NJ) bit (VSCR[15]) and the AltiVec saturation (SAT) bit (VSCR[31]); the remaining bits are reserved. I think this is the reason Paul M. exposed KVM_REG_PPC_VSCR width as 32-bit. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/2] powerpc/booke64: Use common defines for AltiVec interrupts numbers
So we can remove this hack in kvm_asm.h: Not yet, this comment was added in the context of AltiVec RFC patches which intended to remove a similar dependency. /* * TODO: Unify 32-bit and 64-bit kernel exception handlers to use same defines */ #define BOOKE_INTERRUPT_SPE_UNAVAIL BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL #define BOOKE_INTERRUPT_SPE_FP_DATA BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST #define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL #define BOOKE_INTERRUPT_ALTIVEC_ASSIST \ BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST It was added as a compilation fix, and it was less intrusive to temporarily fix it this way. I am curious why the above code wasn't removed at the end of this patchset. :-) Before removing it we also need to apply at least the first patch from the Altivec set that I will send today. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/6] KVM: PPC: Book3E: Refactor SPE/FP exit handling
-#ifdef CONFIG_SPE case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL: { - if (vcpu-arch.shared-msr MSR_SPE) - kvmppc_vcpu_enable_spe(vcpu); - else - kvmppc_booke_queue_irqprio(vcpu, - BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL); + if (kvmppc_supports_spe()) { + bool enabled = false; + +#ifndef CONFIG_KVM_BOOKE_HV + if (vcpu-arch.shared-msr MSR_SPE) { + kvmppc_vcpu_enable_spe(vcpu); + enabled = true; + } +#endif Why the #ifdef? On HV capable systems kvmppc_supports_spe() will just always return false. AltiVec and SPE unavailable exceptions follows the same path. While kvmppc_supports_spe() will always return false kvmppc_supports_altivec() may not. And I don't really understand why HV would be special in the first place here. Is it because we're accessing shared-msr? You are right on HV case MSP[SPV] should be always zero when an unavailabe exception take place. The distrinction was made because on non HV the guest doesn't have direct access to MSR[SPE]. The name of the bit (not the position) was changed on HV cores. + if (!enabled) + kvmppc_booke_queue_irqprio(vcpu, + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL); + } else { + /* +* Guest wants SPE, but host kernel doesn't support it. host kernel or hardware Ok. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/6] KVM: PPC: Book3E: Increase FPU laziness
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, July 03, 2013 4:45 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 3/6] KVM: PPC: Book3E: Increase FPU laziness On 03.07.2013, at 14:42, Mihai Caraman wrote: Increase FPU laziness by calling kvmppc_load_guest_fp() just before returning to guest instead of each sched in. Without this improvement an interrupt may also claim floting point corrupting guest state. Not sure I follow. Could you please describe exactly what's happening? This was already discussed on the list, I will forward you the thread. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/6] KVM: PPC: Book3E: Increase FPU laziness
Increase FPU laziness by calling kvmppc_load_guest_fp() just before returning to guest instead of each sched in. Without this improvement an interrupt may also claim floting point corrupting guest state. Not sure I follow. Could you please describe exactly what's happening? This was already discussed on the list, I will forward you the thread. The only thing I've seen in that thread was some pathetic theoretical case where an interrupt handler would enable fp and clobber state carelessly. That's not something I'm worried about. Neither me though I don't find it pathetic. Please refer it to Scott. I really don't see where this patch improves anything tbh. It certainly makes the code flow more awkward. I was pointing you to this: The idea of FPU/AltiVec laziness that the kernel is struggling to achieve is to reduce the number of store/restore operations. Without this improvement we restore the unit each time we are sched it. If an other process take the ownership of the unit (on SMP it's even worse but don't bother with this) the kernel store the unit state to qemu task. This can happen multiple times during handle_exit(). Do you see it now? -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/6] KVM: PPC: Book3E: Add AltiVec support
+ * Simulate AltiVec unavailable fault to load guest state + * from thread to AltiVec unit. + * It requires to be called with preemption disabled. + */ +static inline void kvmppc_load_guest_altivec(struct kvm_vcpu *vcpu) +{ + if (kvmppc_supports_altivec()) { + if (!(current-thread.regs-msr MSR_VEC)) { + load_up_altivec(NULL); + current-thread.regs-msr |= MSR_VEC; Does this ensure that the kernel saves / restores all altivec state on task switch? Does it load it again when it schedules us in? Would definitely be worth a comment. These units are _LAZY_ !!! Only SMP kernel save the state when it schedules out. + } + } +} + +/* * Helper function for full MSR writes. No need to call this if only * EE/CE/ME/DE/RI are changing. */ @@ -678,6 +706,12 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) u64 fpr[32]; #endif +#ifdef CONFIG_ALTIVEC + vector128 vr[32]; + vector128 vscr; + int used_vr = 0; bool Why don't you ask first to change the type of used_vr member in /include/asm/processor.h? +#endif + if (!vcpu-arch.sane) { kvm_run-exit_reason = KVM_EXIT_INTERNAL_ERROR; return -EINVAL; @@ -716,6 +750,22 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) kvmppc_load_guest_fp(vcpu); #endif +#ifdef CONFIG_ALTIVEC /* Switch from user space Altivec to guest Altivec state */ + if (cpu_has_feature(CPU_FTR_ALTIVEC)) { Why not use your kvmppc_supports_altivec() helper here? Give it a try ... because Linus guarded this members with CONFIG_ALTIVEC :) + /* Save userspace VEC state in stack */ + enable_kernel_altivec(); Can't you hide this in the load function? We only want to enable kernel altivec for a short time while we shuffle the registers around. + memcpy(vr, current-thread.vr, sizeof(current-thread.vr)); vr = current-thread.vr; Are you kidding, be more careful with arrays :) + vscr = current-thread.vscr; + used_vr = current-thread.used_vr; + + /* Restore guest VEC state to thread */ + memcpy(current-thread.vr, vcpu-arch.vr, sizeof(vcpu- arch.vr)); current-thread.vr = vcpu-arch.vr; + current-thread.vscr = vcpu-arch.vscr; + + kvmppc_load_guest_altivec(vcpu); + } Do we need to do this even when the guest doesn't use Altivec? Can't we just load it on demand then once we fault? This code path really should only be a prefetch enable when MSR_VEC is already set in guest context. No we can't, read 6/6. +#endif + ret = __kvmppc_vcpu_run(kvm_run, vcpu); /* No need for kvm_guest_exit. It's done in handle_exit. @@ -736,6 +786,23 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) current-thread.fpexc_mode = fpexc_mode; #endif +#ifdef CONFIG_ALTIVEC /* Switch from guest Altivec to user space Altivec state */ + if (cpu_has_feature(CPU_FTR_ALTIVEC)) { + /* Save AltiVec state to thread */ + if (current-thread.regs-msr MSR_VEC) + giveup_altivec(current); + + /* Save guest state */ + memcpy(vcpu-arch.vr, current-thread.vr, sizeof(vcpu- arch.vr)); + vcpu-arch.vscr = current-thread.vscr; + + /* Restore userspace state */ + memcpy(current-thread.vr, vr, sizeof(current-thread.vr)); + current-thread.vscr = vscr; + current-thread.used_vr = used_vr; + } Same comments here. If the guest never touched Altivec state, don't bother restoring it, as it's still good. LOL, the mighty guest kernel does whatever he wants with AltiVec and doesn't inform us. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/6] KVM: PPC: Book3E: Add AltiVec support
+ if (!vcpu-arch.sane) { kvm_run-exit_reason = KVM_EXIT_INTERNAL_ERROR; return -EINVAL; @@ -716,6 +750,22 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu) kvmppc_load_guest_fp(vcpu); #endif +#ifdef CONFIG_ALTIVEC /* Switch from user space Altivec to guest Altivec state */ + if (cpu_has_feature(CPU_FTR_ALTIVEC)) { Why not use your kvmppc_supports_altivec() helper here? Give it a try ... because Linus guarded this members with CONFIG_ALTIVEC :) Huh? You already are in an #ifdef CONFIG_ALTIVEC here. I think it's a good idea to be consistent in helper usage. And the name you gave to the helper (kvmppc_supports_altivec) is actually quite nice and tells us exactly what we're asking for. I thought you asking to replace #ifdef CONFIG_ALTIVEC. Do we need to do this even when the guest doesn't use Altivec? Can't we just load it on demand then once we fault? This code path really should only be a prefetch enable when MSR_VEC is already set in guest context. No we can't, read 6/6. So we have to make sure we're completely unlazy when it comes to a KVM guest. Are we? Yes, because MSR[SPV] is under its control. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 0/6] KVM: PPC: Book3E: AltiVec support
This looks like a bit much for 3.10 (certainly, subject lines like refactor and enhance and add support aren't going to make Linus happy given that we're past rc4) so I think we should apply http://patchwork.ozlabs.org/patch/242896/ for 3.10. Then for 3.11, revert it after applying this patchset. Why not 1/6 plus e6500 removal? 1/6 is not a bugfix. Not sure I get it. Isn't this a better fix for AltiVec build breakage: -#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL 42 -#define BOOKE_INTERRUPT_ALTIVEC_ASSIST 43 +#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL 32 +#define BOOKE_INTERRUPT_ALTIVEC_ASSIST 33 This removes the need for additional kvm_handlers. Obvious this doesn't make AltiVec to work so we still need to disable e6500. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 0/6] KVM: PPC: Book3E: AltiVec support
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, June 05, 2013 12:39 AM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; Alexander Graf Subject: Re: [RFC PATCH 0/6] KVM: PPC: Book3E: AltiVec support On 06/03/2013 03:54:22 PM, Mihai Caraman wrote: Mihai Caraman (6): KVM: PPC: Book3E: Fix AltiVec interrupt numbers and build breakage KVM: PPC: Book3E: Refactor SPE_FP exit handling KVM: PPC: Book3E: Rename IRQPRIO names to accommodate ALTIVEC KVM: PPC: Book3E: Add AltiVec support KVM: PPC: Book3E: Add ONE_REG AltiVec support KVM: PPC: Book3E: Enhance FPU laziness arch/powerpc/include/asm/kvm_asm.h| 16 ++- arch/powerpc/kvm/booke.c | 189 arch/powerpc/kvm/booke.h |4 +- arch/powerpc/kvm/bookehv_interrupts.S |8 +- arch/powerpc/kvm/e500.c | 10 +- arch/powerpc/kvm/e500_emulate.c |8 +- arch/powerpc/kvm/e500mc.c | 10 ++- 7 files changed, 199 insertions(+), 46 deletions(-) This looks like a bit much for 3.10 (certainly, subject lines like refactor and enhance and add support aren't going to make Linus happy given that we're past rc4) so I think we should apply http://patchwork.ozlabs.org/patch/242896/ for 3.10. Then for 3.11, revert it after applying this patchset. Why not 1/6 plus e6500 removal? -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 2/6] KVM: PPC: Book3E: Refactor SPE_FP exit handling
+ /* +* The interrupt is shared, KVM support for the featured unit +* is detected at run-time. +*/ This is a decent comment for the changelog, but for the code itself it seems fairly obvious if you look at the definition of kvmppc_supports_spe(). I will move it to change log. + bool handled = false; + + if (kvmppc_supports_spe()) { +#ifdef CONFIG_SPE + if (cpu_has_feature(CPU_FTR_SPE)) Didn't you already check this using kvmppc_supports_spe()? It makes sense with the next patch. It handles the improbable case of having CONFIG_ALTIVEC and CONFIG_SPE defined: if (kvmppc_supports_altivec() || kvmppc_supports_spe()). case BOOKE_INTERRUPT_SPE_FP_ROUND: +#ifdef CONFIG_SPE kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_ROUND); r = RESUME_GUEST; break; Why not use kvmppc_supports_spe() here, for consistency? I added cpu_has_feature(CPU_FTR_SPE) for the case specified above, but here SPE_FP_ROUND is not shared with ALTIVEC. CONFIG_SPE is used in other places in KVM without this check, shouldn't be all or nothing? -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 3/6] KVM: PPC: Book3E: Rename IRQPRIO names to accommodate ALTIVEC
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, June 05, 2013 1:28 AM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; Caraman Mihai Claudiu-B02008 Subject: Re: [RFC PATCH 3/6] KVM: PPC: Book3E: Rename IRQPRIO names to accommodate ALTIVEC On 06/03/2013 03:54:25 PM, Mihai Caraman wrote: Rename BOOKE_IRQPRIO_SPE_UNAVAIL and BOOKE_IRQPRIO_SPE_FP_DATA names to accommodate ALTIVEC. Replace BOOKE_INTERRUPT_SPE_UNAVAIL and BOOKE_INTERRUPT_SPE_FP_DATA with the common version. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/booke.c | 12 ++-- arch/powerpc/kvm/booke.h |4 ++-- arch/powerpc/kvm/bookehv_interrupts.S |8 arch/powerpc/kvm/e500.c | 10 ++ arch/powerpc/kvm/e500_emulate.c |8 5 files changed, 22 insertions(+), 20 deletions(-) Can you remove the TODO separate definitions from 1/6 now? And/or combine 1/6 with this patch? TODO separate definitions are for kernel code, they require a new patch depending on comments. I sent 1/6 to fix the build brake generated by this /* altivec */ #define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL 42 #define BOOKE_INTERRUPT_ALTIVEC_ASSIST 43 So if we agree that 42/43 numbers are wrong then I would prefer 1/6 (or something similar) instead of adding new kvm_handlers (which legitimates the above commit) just to remove them later. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 6/6] KVM: PPC: Book3E: Enhance FPU laziness
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, June 05, 2013 1:54 AM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; Caraman Mihai Claudiu-B02008 Subject: Re: [RFC PATCH 6/6] KVM: PPC: Book3E: Enhance FPU laziness On 06/03/2013 03:54:28 PM, Mihai Caraman wrote: Adopt AltiVec approach to increase laziness by calling kvmppc_load_guest_fp() just before returning to guest instaed of each sched in. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com If you did this *before* adding Altivec it would have saved a question in an earlier patch. :-) I kept asking myself about the order and in the end I decided that this is an improvement originated from AltiVec work. FPU may be further cleaned up (get rid of active state, etc). --- arch/powerpc/kvm/booke.c |1 + arch/powerpc/kvm/e500mc.c |2 -- 2 files changed, 1 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 019496d..5382238 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -1258,6 +1258,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, } else { kvmppc_lazy_ee_enable(); kvmppc_load_guest_altivec(vcpu); + kvmppc_load_guest_fp(vcpu); } } You should probably do these before kvmppc_lazy_ee_enable(). Why? I wanted to look like part of lightweight_exit. Actually, I don't think this is a good idea at all. As I understand it, you're not supposed to take kernel ownersship of floating point in non-atomic context, because an interrupt could itself call enable_kernel_fp(). So lightweight_exit isn't executed in atomic context? Will be lazyee fixes including kvmppc_fix_ee_before_entry() in 3.10? 64-bit Book3E KVM is unreliable without them. Should we disable e5500 too for 3.10? Do you have benchmarks showing it's even worthwhile? No yet but isn't this the whole idea of FPU/AltiVec laziness that the kernel is struggling to achieve? Without it when returning to host (if another process got unit ownership in handle_exit) we restore and save the unit state for nothing. This multiplies when the ownership goes back and forth between handle_exit and other processes. Do you have in mid a specific AltiVec benchmark? I have a stress application that do consecutive vector assignments which proved to be very good at finding state corruptions. If I combine this application on the host with a guest that stays longer on handle_exit (running a tlb or emulation intensive application) I might have good data to support my case. -Mike -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 4/6] KVM: PPC: Book3E: Add AltiVec support
+ * Simulate AltiVec unavailable fault to load guest state + * from thread to AltiVec unit. + * It requires to be called with preemption disabled. + */ +static inline void kvmppc_load_guest_altivec(struct kvm_vcpu *vcpu) +{ +#ifdef CONFIG_ALTIVEC + if (cpu_has_feature(CPU_FTR_ALTIVEC)) { + if (!(current-thread.regs-msr MSR_VEC)) { + load_up_altivec(NULL); + current-thread.regs-msr |= MSR_VEC; + } + } +#endif Why not use kvmppc_supports_altivec()? In fact, there's nothing KVM-specific about these functions... I will do so, I had this code before kvmppc_supports_altivec() :) static inline bool kvmppc_supports_spe(void) { #ifdef CONFIG_SPE @@ -947,7 +1016,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, */ bool handled = false; - if (kvmppc_supports_spe()) { + if (kvmppc_supports_altivec() || kvmppc_supports_spe()) { #ifdef CONFIG_SPE if (cpu_has_feature(CPU_FTR_SPE)) if (vcpu-arch.shared-msr MSR_SPE) { The distinction between how you're handling SPE and Altivec here doesn't really have anything to do with SPE versus Altivec -- it's PR-mode versus HV-mode. I was mislead by MSR_SPE bit, we should rename it as MSR_SPV. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [v1][KVM][PATCH 1/1] kvm:ppc:booehv: direct ISI exception to Guest
VF stands for virtualization fault see MAS8[VF] and we may use it for virtualized Looks KVM PPC have no this mechanism currently since I don't find MAS8_VF is used in kernel, right? Yes but 'we may use it' in the feature, I have a functional POC with VF. Now we capture virtualized MMIO accesses as TLB misses (we don't write into HW TLB guest translation outside visible memslots). -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [v1][KVM][PATCH 1/1] kvm:ppc:booehv: direct ISI exception to Guest
-Original Message- From: tiejun.chen [mailto:tiejun.c...@windriver.com] Sent: Thursday, May 09, 2013 2:40 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; ag...@suse.de; kvm- p...@vger.kernel.org; k...@vger.kernel.org Subject: Re: [v1][KVM][PATCH 1/1] kvm:ppc:booehv: direct ISI exception to Guest On 05/09/2013 07:34 PM, Caraman Mihai Claudiu-B02008 wrote: VF stands for virtualization fault see MAS8[VF] and we may use it for virtualized Looks KVM PPC have no this mechanism currently since I don't find MAS8_VF is used in kernel, right? Yes but 'we may use it' in the feature, I have a functional POC with VF. Any IO performance to be improved with this POC? VF approach puts more stress on HW TLB so I did not advance with performance measurements though it may worth to do it. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [v1][KVM][PATCH 1/1] kvm:ppc:booehv: direct ISI exception to Guest
-Original Message- From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On Behalf Of tiejun.chen Sent: Wednesday, May 08, 2013 4:54 AM To: Wood Scott-B07421 Cc: ag...@suse.de; kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org Subject: Re: [v1][KVM][PATCH 1/1] kvm:ppc:booehv: direct ISI exception to Guest On 05/08/2013 07:40 AM, Scott Wood wrote: On 05/07/2013 06:06:30 AM, Tiejun Chen wrote: We also can direct ISI exception to Guest like DSI. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/booke_emulate.c |3 +++ arch/powerpc/kvm/e500mc.c|3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) Are you seeing a real performance improvement from this? This will interfere No. But after we reduce the exit to host, shouldn't this improve performance? We lose some flexibility for this so it make sense only if we gain measurable improvements. somewhat with using the VF bit, if we were to ever do so, since VF only affects Sorry, what is the VF you said? VF stands for virtualization fault see MAS8[VF] and we may use it for virtualized MMIO. The hypervisor should deny execute access on pages marked with VF. Accordingly in this case guest ISI exceptions should be handled by the hypervisor. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC][KVM][PATCH 1/1] kvm:ppc:booke-64: soft-disable interrupts
This only disable soft interrupt for kvmppc_restart_interrupt() that restarts interrupts if they were meant for the host: a. SOFT_DISABLE_INTS() only for BOOKE_INTERRUPT_EXTERNAL | BOOKE_INTERRUPT_DECREMENTER | BOOKE_INTERRUPT_DOORBELL Those aren't the only exceptions that can end up going to the host. We could get a TLB miss that results in a heavyweight MMIO exit, etc. And shouldn't we handle kvmppc_restart_interrupt() like the original HOST flow? #define MASKABLE_EXCEPTION(trapnum, intnum, label, hdlr, ack) \ START_EXCEPTION(label); \ NORMAL_EXCEPTION_PROLOG(trapnum, intnum, PROLOG_ADDITION_MASKABLE)\ EXCEPTION_COMMON(trapnum, PACA_EXGEN, *INTS_DISABLE*) \ ... Could you elaborate on what you mean? I think Tiejun was saying that host has flags and replays only EE/DEC/DBELL interrupts. There is special macro masked_interrupt_book3e in those exception handlers that sets paca-irq_happened. The list of replied interrupts is limited to asynchronous noncritical interrupts which can be masked by MSR[EE] (therefore no TLB miss). Now on KVM book3e we don't want to put them in the irq_happened lazy state but rather to execute them directly, so there is no reason for exception handling symmetry between host and guest. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] kvm/ppc/booke64: Hard disable interrupts when entering the guest
-Original Message- From: Wood Scott-B07421 Sent: Saturday, May 04, 2013 2:45 AM To: Alexander Graf Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; Wood Scott-B07421; Caraman Mihai Claudiu-B02008 Subject: [PATCH] kvm/ppc/booke64: Hard disable interrupts when entering the guest kvmppc_lazy_ee_enable() was causing interrupts to be soft-enabled (albeit hard-disabled) in kvmppc_restart_interrupt(). This led to warnings, and possibly breakage if the interrupt state was later saved and then restored (leading to interrupts being hard-and-soft enabled when they should be at least soft-disabled). Simply removing kvmppc_lazy_ee_enable() leaves interrupts only soft-disabled when we enter the guest, but they will be hard-disabled when we exit the guest -- without PACA_IRQ_HARD_DIS ever being set, so the local_irq_enable() fails to hard-enable. Just to mention one special case. may_hard_irq_enable() called from do_IRQ() and timer_interrupt() clears PACA_IRQ_HARD_DIS but it either hard-enable or let PACA_IRQ_EE set which is enough for local_irq_enable() to hard-enable. While we could just set PACA_IRQ_HARD_DIS after an exit to compensate, instead hard-disable interrupts before entering the guest. This way, we won't have to worry about interactions if we take an interrupt during the guest entry code. While I don't see any obvious interactions, it could change in the future (e.g. it would be bad if the non-hv code were used on 64-bit or if 32-bit guest lazy interrupt disabling, since the non-hv code changes IVPR among other things). Signed-off-by: Scott Wood scottw...@freescale.com Cc: Mihai Caraman mihai.cara...@freescale.com Please add my signed-off, it builds on the same principle of interrupts soft-disabled to fix warnings and irq_happened flags to force interrupts hard-enabled ... and parts of the code ;) -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 03, 2013 9:05 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; Caraman Mihai Claudiu-B02008 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs The unresponsiveness has to do with the fact that arch_local_irq_restore() does not guarantees to hard enable interrupts. Could you elaborate? If the saved IRQ state was enabled, why wouldn't arch_local_irq_restore() hard-enable IRQs? The last thing it does is __hard_irq_enable(). if (!irq_happened) return; Where is the arch_local_irq_restore() instance you're talking about? ./arch/power/kernel/irq.c To do so replace exception function calls like timer_interrupt() with irq_happened flags. The local_irq_enable() call takes care of replaying them and lets the interrupts hard enabled. Not sure what you mean by lets the interrupts hard enabled... Do you mean the EE bit in regs-msr, as opposed to the EE bit in the current MSR? If irq_happened the last thing it does is __hard_irq_enable(). @@ -789,16 +788,16 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu, switch (exit_nr) { case BOOKE_INTERRUPT_EXTERNAL: kvmppc_fill_pt_regs(regs); - do_IRQ(regs); + local_paca-irq_happened |= PACA_IRQ_EE; break; Aren't you breaking 32-bit here? I had eyes only for 64-bit hangs :) -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
-Original Message- From: Wood Scott-B07421 Sent: Friday, May 03, 2013 11:15 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs The unresponsiveness has to do with the fact that arch_local_irq_restore() does not guarantees to hard enable interrupts. Could you elaborate? If the saved IRQ state was enabled, why wouldn't arch_local_irq_restore() hard-enable IRQs? The last thing it does is __hard_irq_enable(). if (!irq_happened) return; OK, so the problem is that we're not setting PACA_IRQ_HARD_DIS when we hard-disable interrupts? We enter guest with local_irq_disable() which means soft disabled, when do we hard-disable interrupts? If we follow host exception handlers model they set PACA_IRQ_EE/DEC/DBELL but not PACA_IRQ_HARD_DIS. Can you give it a try to see how KVM behaves with PACA_IRQ_HARD_DIS? I can't do it right now. Where is the arch_local_irq_restore() instance you're talking about? ./arch/power/kernel/irq.c I meant the caller. :-P ./arch/powerpc/include/asm/hw_irq.h 55static inline unsigned long arch_local_irq_disable(void) 56{ 57unsigned long flags, zero; 58 59asm volatile( 60li %1,0; lbz %0,%2(13); stb %1,%2(13) 61: =r (flags), =r (zero) 62: i (offsetof(struct paca_struct, soft_enabled)) 63: memory); 64 65return flags; 66} 67 68extern void arch_local_irq_restore(unsigned long); 69 70static inline void arch_local_irq_enable(void) 71{ 72arch_local_irq_restore(1); 73} -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
-Original Message- From: Wood Scott-B07421 Sent: Saturday, May 04, 2013 1:07 AM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs I replaced the two calls to kvmppc_lazy_ee_enable() with calls to hard_irq_disable(), and it seems to be working fine. Please take a look on 'KVM: PPC64: booke: Hard disable interrupts when entering guest' RFC thread and see if your solution addresses Ben's comments. Where is the arch_local_irq_restore() instance you're talking about? ./arch/power/kernel/irq.c I meant the caller. :-P ./arch/powerpc/include/asm/hw_irq.h 55static inline unsigned long arch_local_irq_disable(void) 56{ 57unsigned long flags, zero; 58 59asm volatile( 60li %1,0; lbz %0,%2(13); stb %1,%2(13) 61: =r (flags), =r (zero) 62: i (offsetof(struct paca_struct, soft_enabled)) 63: memory); 64 65return flags; 66} 67 68extern void arch_local_irq_restore(unsigned long); 69 70static inline void arch_local_irq_enable(void) 71{ 72arch_local_irq_restore(1); 73} Sigh. I meant the real caller, who's calling local_irq_restore(). I'm not sure what you mean, arch_local_irq_restore() is called indirectly by local_irq_enable() in our case from handle_exit(). -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/1] kvm:book3e: Fix a build error
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Tiejun Chen Sent: Thursday, April 25, 2013 2:46 PM To: ga...@kernel.crashing.org Cc: linuxppc-dev@lists.ozlabs.org; kvm-...@vger.kernel.org; k...@vger.kernel.org Subject: [PATCH 1/1] kvm:book3e: Fix a build error Commit cd66cc2e, powerpc/85xx: Add AltiVec support for e6500, adds support for AltiVec on a Book-E class processor, but while compiling in the CONFIG_PPC_BOOK3E_64 and CONFIG_VIRTUALIZATION case, this introduce the following error: arch/powerpc/kernel/exceptions-64e.S:402: undefined reference to `kvmppc_handler_42_0x01B' arch/powerpc/kernel/built-in.o: In function `exc_altivec_assist_book3e': arch/powerpc/kernel/exceptions-64e.S:424: undefined reference to `kvmppc_handler_43_0x01B' make: *** [vmlinux] Error 1 Looks we should add these altivec kvm handlers. Signed-off-by: Tiejun Chen tiejun.c...@windriver.com --- arch/powerpc/kvm/bookehv_interrupts.S |5 + 1 file changed, 5 insertions(+) diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index e8ed7d6..fa9c78a 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -319,6 +319,11 @@ kvm_handler BOOKE_INTERRUPT_DEBUG, EX_PARAMS(DBG), \ SPRN_DSRR0, SPRN_DSRR1, 0 kvm_handler BOOKE_INTERRUPT_DEBUG, EX_PARAMS(CRIT), \ SPRN_CSRR0, SPRN_CSRR1, 0 +/* altivec */ +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \ + SPRN_SRR0, SPRN_SRR1, 0 #else /* * For input register values, see arch/powerpc/include/asm/kvm_booke_hv_asm.h -- It seems that you are not using kvm-ppc-queue branch. I already have a patch ready for this (and AltiVec support is work in progress) but we need first to pull e6500 kernel patches from Linux tree into agraf.git. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: e500: Expose MMU registers via ONE_REG
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 21, 2013 12:07 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; k...@vger.kernel.org Subject: Re: [PATCH] KVM: PPC: e500: Expose MMU registers via ONE_REG On 19.03.2013, at 18:26, Scott Wood wrote: On 03/19/2013 12:17:11 PM, Mihai Caraman wrote: diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c index 66b6e31..b77b855 100644 --- a/arch/powerpc/kvm/e500_mmu.c +++ b/arch/powerpc/kvm/e500_mmu.c @@ -596,6 +596,95 @@ int kvmppc_set_sregs_e500_tlb(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) return 0; } +int kvmppc_get_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id, + union kvmppc_one_reg *val) s/500/e500/ +int kvmppc_set_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id, + union kvmppc_one_reg *val) +{ + int r = 0; + long int i; + + switch (id) { + case KVM_REG_PPC_MAS0: + vcpu-arch.shared-mas0 = set_reg_val(id, *val); + break; + case KVM_REG_PPC_MAS1: + vcpu-arch.shared-mas1 = set_reg_val(id, *val); + break; + case KVM_REG_PPC_MAS2: + vcpu-arch.shared-mas2 = set_reg_val(id, *val); + break; + case KVM_REG_PPC_MAS7_3: + vcpu-arch.shared-mas7_3 = set_reg_val(id, *val); + break; + case KVM_REG_PPC_MAS4: + vcpu-arch.shared-mas4 = set_reg_val(id, *val); + break; + case KVM_REG_PPC_MAS6: + vcpu-arch.shared-mas6 = set_reg_val(id, *val); + break; + case KVM_REG_PPC_MMUCFG: { + u32 mmucfg = set_reg_val(id, *val); + vcpu-arch.mmucfg = mmucfg ~MMUCFG_LPIDSIZE; + break; + } Do we really want to allow arbitrary MMUCFG changes? It won't magically make us able to support larger RAs, PIDs, different MAVN, etc. Not magically, some changes e.g TLBnCFG_IND or TLBnPS require just a kvm check other changes e.g. TLBnCFG_MAVN require additional support and we might not implement all of them. Until then this code should do the job: /* MMU registers can be set only to the configuration supported by KVM */ case KVM_REG_PPC_MMUCFG: { if (set_reg_val(id, *val) != vcpu-arch.mmucfg) r = -EINVAL; break; } Only if we update the actual shadow mmu configuration as well. These registers (MMUCFG, EPTCFG, TLBnCFG, TLBnPS) are read-only (and shared between e6500 threads), we can only emulate them. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: e500: Add separate functions for vcpu's MMU configuration
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Thursday, March 21, 2013 12:07 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH] KVM: PPC: e500: Add separate functions for vcpu's MMU configuration On 19.03.2013, at 18:16, Mihai Caraman wrote: Move vcpu's MMU default configuration and geometry update into their own functions. Mind to explain why? You requested a separate function for clearing TLBnCFG_IND bit (E.PT removal) to self-document the code. The existing logic (that TLBnCFG_IND relies on) was buried in a chunk of code and I thought this will add more clarity. If you don't agree I would document the code at least. -Mike Alex Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/e500_mmu.c | 59 +++- --- 1 files changed, 37 insertions(+), 22 deletions(-) diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c index 5c44759..66b6e31 100644 --- a/arch/powerpc/kvm/e500_mmu.c +++ b/arch/powerpc/kvm/e500_mmu.c @@ -596,6 +596,20 @@ int kvmppc_set_sregs_e500_tlb(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) return 0; } +static int vcpu_mmu_geometry_update(struct kvm_vcpu *vcpu, + struct kvm_book3e_206_tlb_params *params) +{ + vcpu-arch.tlbcfg[0] = ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC); + if (params-tlb_sizes[0] = 2048) + vcpu-arch.tlbcfg[0] |= params-tlb_sizes[0]; + vcpu-arch.tlbcfg[0] |= params-tlb_ways[0] TLBnCFG_ASSOC_SHIFT; + + vcpu-arch.tlbcfg[1] = ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC); + vcpu-arch.tlbcfg[1] |= params-tlb_sizes[1]; + vcpu-arch.tlbcfg[1] |= params-tlb_ways[1] TLBnCFG_ASSOC_SHIFT; + return 0; +} + int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu, struct kvm_config_tlb *cfg) { @@ -692,16 +706,8 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu, vcpu_e500-gtlb_offset[0] = 0; vcpu_e500-gtlb_offset[1] = params.tlb_sizes[0]; - vcpu-arch.mmucfg = mfspr(SPRN_MMUCFG) ~MMUCFG_LPIDSIZE; - - vcpu-arch.tlbcfg[0] = ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC); - if (params.tlb_sizes[0] = 2048) - vcpu-arch.tlbcfg[0] |= params.tlb_sizes[0]; - vcpu-arch.tlbcfg[0] |= params.tlb_ways[0] TLBnCFG_ASSOC_SHIFT; - - vcpu-arch.tlbcfg[1] = ~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC); - vcpu-arch.tlbcfg[1] |= params.tlb_sizes[1]; - vcpu-arch.tlbcfg[1] |= params.tlb_ways[1] TLBnCFG_ASSOC_SHIFT; + /* Update vcpu's MMU geometry based on SW_TLB input */ + vcpu_mmu_geometry_update(vcpu, params); vcpu_e500-shared_tlb_pages = pages; vcpu_e500-num_shared_tlb_pages = num_pages; @@ -737,6 +743,26 @@ int kvm_vcpu_ioctl_dirty_tlb(struct kvm_vcpu *vcpu, return 0; } +/* vcpu's MMU default configuration */ +static int vcpu_mmu_init(struct kvm_vcpu *vcpu, + struct kvmppc_e500_tlb_params *params) +{ + /* Initialize RASIZE, PIDSIZE, NTLBS and MAVN fields with host values*/ + vcpu-arch.mmucfg = mfspr(SPRN_MMUCFG) ~MMUCFG_LPIDSIZE; + + /* Initialize IPROT field with host value*/ + vcpu-arch.tlbcfg[0] = mfspr(SPRN_TLB0CFG) +~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC); + vcpu-arch.tlbcfg[0] |= params[0].entries; + vcpu-arch.tlbcfg[0] |= params[0].ways TLBnCFG_ASSOC_SHIFT; + + vcpu-arch.tlbcfg[1] = mfspr(SPRN_TLB1CFG) +~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC); + vcpu-arch.tlbcfg[1] |= params[1].entries; + vcpu-arch.tlbcfg[1] |= params[1].ways TLBnCFG_ASSOC_SHIFT; + return 0; +} + int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500) { struct kvm_vcpu *vcpu = vcpu_e500-vcpu; @@ -781,18 +807,7 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500) if (!vcpu_e500-g2h_tlb1_map) goto err; - /* Init TLB configuration register */ - vcpu-arch.tlbcfg[0] = mfspr(SPRN_TLB0CFG) -~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC); - vcpu-arch.tlbcfg[0] |= vcpu_e500-gtlb_params[0].entries; - vcpu-arch.tlbcfg[0] |= - vcpu_e500-gtlb_params[0].ways TLBnCFG_ASSOC_SHIFT; - - vcpu-arch.tlbcfg[1] = mfspr(SPRN_TLB1CFG) -~(TLBnCFG_N_ENTRY | TLBnCFG_ASSOC); - vcpu-arch.tlbcfg[1] |= vcpu_e500-gtlb_params[1].entries; - vcpu-arch.tlbcfg[1] |= - vcpu_e500-gtlb_params[1].ways TLBnCFG_ASSOC_SHIFT; + vcpu_mmu_init(vcpu, vcpu_e500-gtlb_params); kvmppc_recalc_tlb1map_range(vcpu_e500); return 0; -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo
RE: [PATCH] KVM: PPC: e500: Expose MMU registers via ONE_REG
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 21, 2013 1:07 PM To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; k...@vger.kernel.org Subject: Re: [PATCH] KVM: PPC: e500: Expose MMU registers via ONE_REG On 21.03.2013, at 12:02, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, March 21, 2013 12:07 PM To: Wood Scott-B07421 Cc: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; k...@vger.kernel.org Subject: Re: [PATCH] KVM: PPC: e500: Expose MMU registers via ONE_REG On 19.03.2013, at 18:26, Scott Wood wrote: On 03/19/2013 12:17:11 PM, Mihai Caraman wrote: diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c index 66b6e31..b77b855 100644 --- a/arch/powerpc/kvm/e500_mmu.c +++ b/arch/powerpc/kvm/e500_mmu.c @@ -596,6 +596,95 @@ int kvmppc_set_sregs_e500_tlb(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs) return 0; } +int kvmppc_get_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id, +union kvmppc_one_reg *val) s/500/e500/ +int kvmppc_set_one_reg_500_tlb(struct kvm_vcpu *vcpu, u64 id, + union kvmppc_one_reg *val) +{ +int r = 0; +long int i; + +switch (id) { +case KVM_REG_PPC_MAS0: +vcpu-arch.shared-mas0 = set_reg_val(id, *val); +break; +case KVM_REG_PPC_MAS1: +vcpu-arch.shared-mas1 = set_reg_val(id, *val); +break; +case KVM_REG_PPC_MAS2: +vcpu-arch.shared-mas2 = set_reg_val(id, *val); +break; +case KVM_REG_PPC_MAS7_3: +vcpu-arch.shared-mas7_3 = set_reg_val(id, *val); +break; +case KVM_REG_PPC_MAS4: +vcpu-arch.shared-mas4 = set_reg_val(id, *val); +break; +case KVM_REG_PPC_MAS6: +vcpu-arch.shared-mas6 = set_reg_val(id, *val); +break; +case KVM_REG_PPC_MMUCFG: { +u32 mmucfg = set_reg_val(id, *val); +vcpu-arch.mmucfg = mmucfg ~MMUCFG_LPIDSIZE; +break; +} Do we really want to allow arbitrary MMUCFG changes? It won't magically make us able to support larger RAs, PIDs, different MAVN, etc. Not magically, some changes e.g TLBnCFG_IND or TLBnPS require just a kvm check other changes e.g. TLBnCFG_MAVN require additional support and we might not implement all of them. Until then this code should do the job: /* MMU registers can be set only to the configuration supported by KVM */ case KVM_REG_PPC_MMUCFG: { if (set_reg_val(id, *val) != vcpu-arch.mmucfg) r = -EINVAL; break; } Yes :). Only if we update the actual shadow mmu configuration as well. These registers (MMUCFG, EPTCFG, TLBnCFG, TLBnPS) are read-only (and shared between e6500 threads), we can only emulate them. We need to change the behavior of the shadow mmu as well. It's not about the registers, but the actually exposed TLBs. If you configure 4 TLBs, and you announce to the guest that you can do 4 TLBs, you better emulate 4 TLBs :). Right, like the rest of configs I was talking above:) -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/5] KVM: PPC: e500: Move VCPU's MMUCFG register initialization earlier
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, January 31, 2013 3:21 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 1/5] KVM: PPC: e500: Move VCPU's MMUCFG register initialization earlier On 30.01.2013, at 14:29, Mihai Caraman wrote: VCPU's MMUCFG register initialization should not depend on KVM_CAP_SW_TLB ioctl call. Move it earlier into tlb initalization phase. Quite the contrary. The fact that there is an mfspr() in e500_mmu.c already tells us that the code is broken. The TLB guest code should only depend on input from the SW_TLB configuration. It's completely orthogonal to the host capabilities. Then we have the same issue for TLBnCFG registers which need to be configured via SW_TLB ioctl. What is the purpose of guest tlb initalization in e500_mmu.c if we rely on SW_TLB? -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/5] KVM: PPC: e500: Emulate EPTCFG register
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, January 31, 2013 3:31 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 4/5] KVM: PPC: e500: Emulate EPTCFG register On 30.01.2013, at 14:29, Mihai Caraman wrote: EPTCFG register defined by E.PT is accessed unconditionally by Linux guests in the presence of MAV 2.0. Emulate EPTCFG register now. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/include/asm/kvm_host.h |1 + arch/powerpc/kvm/e500.h |6 ++ arch/powerpc/kvm/e500_emulate.c |9 + arch/powerpc/kvm/e500_mmu.c |5 + 4 files changed, 21 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 88fcfe6..f480b20 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -503,6 +503,7 @@ struct kvm_vcpu_arch { u32 tlbcfg[4]; u32 tlbps[4]; u32 mmucfg; + u32 eptcfg; This too needs to be settable through SW_TLB. u32 epr; struct kvmppc_booke_debug_reg dbg_reg; #endif diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h index b9f76d8..983eb95 100644 --- a/arch/powerpc/kvm/e500.h +++ b/arch/powerpc/kvm/e500.h @@ -308,4 +308,10 @@ static inline unsigned int has_mmu_v2(const struct kvm_vcpu *vcpu) return ((vcpu-arch.mmucfg MMUCFG_MAVN) == MMUCFG_MAVN_V2); } +static inline unsigned int supports_page_tables(const struct kvm_vcpu *vcpu) bool again. Can we generalize this a bit more? How about a small framework that allows us to differentiate across e.XX features? I thought you will ask for it :) -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/5] KVM: PPC: e500: Move VCPU's MMUCFG register initialization earlier
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, January 31, 2013 4:58 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 1/5] KVM: PPC: e500: Move VCPU's MMUCFG register initialization earlier On 31.01.2013, at 15:56, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, January 31, 2013 3:21 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH 1/5] KVM: PPC: e500: Move VCPU's MMUCFG register initialization earlier On 30.01.2013, at 14:29, Mihai Caraman wrote: VCPU's MMUCFG register initialization should not depend on KVM_CAP_SW_TLB ioctl call. Move it earlier into tlb initalization phase. Quite the contrary. The fact that there is an mfspr() in e500_mmu.c already tells us that the code is broken. The TLB guest code should only depend on input from the SW_TLB configuration. It's completely orthogonal to the host capabilities. Then we have the same issue for TLBnCFG registers which need to be configured via SW_TLB ioctl. What is the purpose of guest tlb initalization in e500_mmu.c if we rely on SW_TLB? It's to provide a fallback to user space that doesn't implement SW_TLB configuration yet. Do we have such a case now or is it just hypothetical? For the fallback we need to initialize the MMUCFG register which I intended to say in the commit message. Alex ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 05/17] KVM: PPC: booke: Extend MAS2 EPN mask for 64-bit
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Monday, October 08, 2012 1:11 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 05/17] KVM: PPC: booke: Extend MAS2 EPN mask for 64-bit On 05.07.2012, at 13:14, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, July 04, 2012 4:50 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 05/17] KVM: PPC: booke: Extend MAS2 EPN mask for 64-bit On 25.06.2012, at 14:26, Mihai Caraman wrote: Extend MAS2 EPN mask for 64-bit hosts, to retain most significant bits. Change get tlb eaddr to use this mask. Please see section 6.11.4.8 in the PowerISA 2.06b: MMU behavior is largely unaffected by whether the thread is in 32-bit computation mode (MSRCM=0) or 64- bit computation mode (MSRCM=1). The only differ- ences occur in the EPN field of the TLB entry and the EPN field of MAS2. The differences are summarized here. * Executing a tlbwe instruction in 32-bit mode will set bits 0:31 of the TLB EPN field to zero unless MAS0ATSEL is set, in which case those bits are not written to zero. * In 32-bit implementations, MAS2U can be used to read or write EPN0:31 of MAS2. So if MSR.CM is not set tlbwe should mask the upper 32 bits out - which can happen regardless of CONFIG_64BIT. MAS2_EPN reflects EPN field of MAS2 aka bits 0:51 (for MAV = 1.0) according to section 6.10.3.10 in the PowerISA 2.06b. MAS2_EPN is not used in tlbwe execution emulation, we have MAS2_VAL define for this case. So tlbe-mas2 is guaranteed to have the upper bits be 0 when MSR.CM=0? We chose to mask out mas2 upper bits on tlbwe emulation so gtlbe-mas2 will respect this but vcpu-arch.shared-mas2 will not. tlb entry selection does not require this treatment since EPN upper bits are not taken into consideration anyway. Also, we need to implement MAS2U, to potentially make the upper 32bits of MAS2 available, right? But that one isn't as important as the first bit. MAS2U is guest privileged why does it need special care? Maybe it's mapped to the upper bits of GMAS2 automatically? GMAS2? Freescale core Manuals and EREF does not mention MAS2U so I think I our case it is not implemented. Please check with a simple mfspr() test on real hw to see if it really isn't implemented. I will try this with SPR number 0x277. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: bookehv: Allow duplicate calls of DO_KVM macro
-Original Message- From: Wood Scott-B07421 Sent: Thursday, September 13, 2012 12:54 AM To: Alexander Graf Cc: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; k...@vger.kernel.org Subject: Re: [PATCH] KVM: PPC: bookehv: Allow duplicate calls of DO_KVM macro On 09/12/2012 04:45 PM, Alexander Graf wrote: On 12.09.2012, at 23:38, Scott Wood scottw...@freescale.com wrote: On 09/12/2012 01:56 PM, Alexander Graf wrote: On 12.09.2012, at 15:18, Mihai Caraman mihai.cara...@freescale.com wrote: The current form of DO_KVM macro restricts its use to one call per input parameter set. This is caused by kvmppc_resume_\intno\()_\srr1 symbol definition. Duplicate calls of DO_KVM are required by distinct implementations of exeption handlers which are delegated at runtime. Not sure I understand what you're trying to achieve here. Please elaborate ;) On 64-bit book3e we compile multiple versions of the TLB miss handlers, and choose from them at runtime. The exception handler patching is active in __early_init_mmu() function powerpc/mm/tlb_nohash.c for quite a few years. For tlb miss exceptions there are three handler versions: standard, HW tablewalk and bolted. I posted a patch to add another variant, for e6500-style hardware tablewalk, which shares the bolted prolog/epilog (besides prolog/epilog performance, e6500 is incompatible with the IBM tablewalk code for various reasons). That caused us to have two DO_KVMs for the same exception type. Sorry, I missed to cc kvm-ppc mailist when I replayed to that discussion thread. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] powerpc/e6500: TLB miss handler with hardware tablewalk support
diff --git a/arch/powerpc/mm/tlb_low_64e.S b/arch/powerpc/mm/tlb_low_64e.S index efe0f33..8e82772 100644 --- a/arch/powerpc/mm/tlb_low_64e.S +++ b/arch/powerpc/mm/tlb_low_64e.S @@ -232,6 +232,173 @@ itlb_miss_fault_bolted: beq tlb_miss_common_bolted b itlb_miss_kernel_bolted +/* + * TLB miss handling for e6500 and derivatives, using hardware tablewalk. + * + * Linear mapping is bolted: no virtual page table or nested TLB misses + * Indirect entries in TLB1, hardware loads resulting direct entries + *into TLB0 + * No HES or NV hint on TLB1, so we need to do software round-robin + * No tlbsrx. so we need a spinlock, and we have to deal + *with MAS-damage caused by tlbsx + * 4K pages only + */ + + START_EXCEPTION(instruction_tlb_miss_e6500) + tlb_prolog_bolted SPRN_SRR0 + + ld r11,PACA_TLB_PER_CORE_PTR(r13) + srdi. r15,r16,60 /* get region */ + ori r16,r16,1 + + TLB_MISS_STATS_SAVE_INFO_BOLTED + bne tlb_miss_kernel_e6500 /* user/kernel test */ + + b tlb_miss_common_e6500 + + START_EXCEPTION(data_tlb_miss_e6500) + tlb_prolog_bolted SPRN_DEAR + + ld r11,PACA_TLB_PER_CORE_PTR(r13) + srdi. r15,r16,60 /* get region */ + rldicr r16,r16,0,62 + + TLB_MISS_STATS_SAVE_INFO_BOLTED + bne tlb_miss_kernel_e6500 /* user vs kernel check */ + This ends up calling DO_KVM macro twice with same parameters which generates the following compile error: arch/powerpc/mm/tlb_low_64e.S:307: Error: symbol `kvmppc_resume_14_0x01B' is already defined arch/powerpc/mm/tlb_low_64e.S:319: Error: symbol `kvmppc_resume_13_0x01B' is already defined We can live with it if we patch DO_KVM like this: diff --git a/arch/powerpc/include/asm/kvm_booke_hv_asm.h b/arch/powerpc/include/asm/kvm_booke_hv_asm.h index 4610fb0..029ecab 100644 --- a/arch/powerpc/include/asm/kvm_booke_hv_asm.h +++ b/arch/powerpc/include/asm/kvm_booke_hv_asm.h @@ -55,9 +55,9 @@ #ifdef CONFIG_KVM_BOOKE_HV BEGIN_FTR_SECTION mtocrf 0x80, r11 /* check MSR[GS] without clobbering reg */ - bf 3, kvmppc_resume_\intno\()_\srr1 + bf 3, 1f b kvmppc_handler_\intno\()_\srr1 -kvmppc_resume_\intno\()_\srr1: +1: END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) #endif .endm -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks
From: Alexander Graf [ag...@suse.de] Sent: Saturday, July 07, 2012 2:11 AM To: Caraman Mihai Claudiu-B02008 Cc: Benjamin Herrenschmidt; kvm-...@vger.kernel.org; KVM list; linuxppc-dev; qemu-...@nongnu.org List Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks On 07.07.2012, at 00:33, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] Sent: Thursday, July 05, 2012 1:26 AM To: Alexander Graf Cc: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org; KVM list; linuxppc-dev; qemu-...@nongnu.org List Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks You can't but in any case I don't see the point of the conditional here, we'll eventually have to load srr1 no ? We can move the load up to here in all cases or can't we ? I like the idea, but there is a problem with addition macros which may clobber r11 and PROLOG_ADDITION_MASKABLE_GEN is such a case. Mike -v please :) Ben suggested something like this: #define EXCEPTION_PROLOG(n, type, addition) \ mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \ mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \ std r10,PACA_EX##type+EX_R10(r13); \ std r11,PACA_EX##type+EX_R11(r13); \ mfcr r10; /* save CR */ \ + mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \ DO_KVM intnum,srr1; \ addition; /* additional code for that exc. */ \ std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \ stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \ - mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \ type##_SET_KSTACK; /* get special stack if necessary */\ andi. r10,r11,MSR_PR; /* save stack pointer */ \ But one of the addition looks like this: #define PROLOG_ADDITION_MASKABLE_GEN(n) \ lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \ cmpwi cr0,r11,0; /* yes - go out of line */ \ beq masked_interrupt_book3e_##n So for maskable gen we end up with: #define EXCEPTION_PROLOG(n, type, addition) \ mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \ mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \ std r10,PACA_EX##type+EX_R10(r13); \ std r11,PACA_EX##type+EX_R11(r13); \ mfcr r10; /* save CR */ \ mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \ DO_KVM intnum,srr1; \ lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \ cmpwi cr0,r11,0; /* yes - go out of line */ \ beq masked_interrupt_book3e_##n \ std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \ stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \ type##_SET_KSTACK; /* get special stack if necessary */\ andi. r10,r11,MSR_PR; /* save stack pointer */ \ This affects the last asm line, we load srr1 into r11 but clobber it in-between. We need a spare register for maskable gen addition. I think we can free r10 sooner and used it in addition like this: #define EXCEPTION_PROLOG(n, type, addition) \ mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \ mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \ std r10,PACA_EX##type+EX_R10(r13); \ std r11,PACA_EX##type+EX_R11(r13); \ + mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \ mfcr r10; /* save CR */ \ + stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \ DO_KVM intnum,srr1; \ - lbz r11,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \ - cmpwi cr0,r11,0; /* yes - go out of line */ \ + lbz r10,PACASOFTIRQEN(r13); /* are irqs soft-disabled ? */ \ + cmpwi cr0,r10,0; /* yes - go out of line */ \ beq masked_interrupt_book3e_##n \ std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \ - stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \ - mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */ \ type##_SET_KSTACK; /* get special stack if necessary */\ andi. r10,r11,MSR_PR; /* save stack pointer */ \ -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks
-Original Message- From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] Sent: Thursday, July 05, 2012 1:26 AM To: Alexander Graf Cc: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org; KVM list; linuxppc-dev; qemu-...@nongnu.org List Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks On Wed, 2012-07-04 at 16:29 +0200, Alexander Graf wrote: +#ifdef CONFIG_KVM_BOOKE_HV +#define KVM_BOOKE_HV_MFSPR(reg, spr) \ + BEGIN_FTR_SECTION \ + mfspr reg, spr; \ + END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) +#else +#define KVM_BOOKE_HV_MFSPR(reg, spr) +#endif Bleks - this is ugly. Do we really need to open-code the #ifdef here? Can't the feature section code determine that the feature is disabled and just always not include the code? You can't but in any case I don't see the point of the conditional here, we'll eventually have to load srr1 no ? We can move the load up to here in all cases or can't we ? I like the idea, but there is a problem with addition macros which may clobber r11 and PROLOG_ADDITION_MASKABLE_GEN is such a case. If really not, we could have it inside DO_KVM and be done with it no ? 32-bit exception prolog loads srr1 unconditionally, as Alex and Scott mentioned earlier, so we will be suboptimal for this case. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 04/17] KVM: PPC64: booke: Add guest computation mode for irq delivery
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, July 04, 2012 4:41 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 04/17] KVM: PPC64: booke: Add guest computation mode for irq delivery On 25.06.2012, at 14:26, Mihai Caraman wrote: Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/booke.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index d15c4b5..93b48e0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -287,6 +287,7 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, bool crit; bool keep_irq = false; enum int_class int_class; + ulong msr_cm = 0; /* Truncate crit indicators in 32 bit mode */ if (!(vcpu-arch.shared-msr MSR_SF)) { @@ -299,6 +300,10 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, /* ... and we're in supervisor mode */ crit = crit !(vcpu-arch.shared-msr MSR_PR); +#ifdef CONFIG_64BIT + msr_cm = vcpu-arch.epcr SPRN_EPCR_ICM ? MSR_CM : 0; +#endif No need for the ifdef, no?. Just mask EPCR_ICM out in the 32-bit host case, then this check is always false on 32-bit hosts. It will break e500v2. epcr field is declared only for CONFIG_KVM_BOOKE_HV, we can limit to this instead of CONFIG_64BIT. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 05/17] KVM: PPC: booke: Extend MAS2 EPN mask for 64-bit
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, July 04, 2012 4:50 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 05/17] KVM: PPC: booke: Extend MAS2 EPN mask for 64-bit On 25.06.2012, at 14:26, Mihai Caraman wrote: Extend MAS2 EPN mask for 64-bit hosts, to retain most significant bits. Change get tlb eaddr to use this mask. Please see section 6.11.4.8 in the PowerISA 2.06b: MMU behavior is largely unaffected by whether the thread is in 32-bit computation mode (MSRCM=0) or 64- bit computation mode (MSRCM=1). The only differ- ences occur in the EPN field of the TLB entry and the EPN field of MAS2. The differences are summarized here. * Executing a tlbwe instruction in 32-bit mode will set bits 0:31 of the TLB EPN field to zero unless MAS0ATSEL is set, in which case those bits are not written to zero. * In 32-bit implementations, MAS2U can be used to read or write EPN0:31 of MAS2. So if MSR.CM is not set tlbwe should mask the upper 32 bits out - which can happen regardless of CONFIG_64BIT. MAS2_EPN reflects EPN field of MAS2 aka bits 0:51 (for MAV = 1.0) according to section 6.10.3.10 in the PowerISA 2.06b. MAS2_EPN is not used in tlbwe execution emulation, we have MAS2_VAL define for this case. Also, we need to implement MAS2U, to potentially make the upper 32bits of MAS2 available, right? But that one isn't as important as the first bit. MAS2U is guest privileged why does it need special care? Freescale core Manuals and EREF does not mention MAS2U so I think I our case it is not implemented. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 06/17] KVM: PPC: e500: Add emulation helper for getting instruction ea
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Wednesday, July 04, 2012 4:56 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [RFC PATCH 06/17] KVM: PPC: e500: Add emulation helper for getting instruction ea On 25.06.2012, at 14:26, Mihai Caraman wrote: Add emulation helper for getting instruction ea and refactor tlb instruction emulation to use it. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/e500.h |6 +++--- arch/powerpc/kvm/e500_emulate.c | 21 ++--- arch/powerpc/kvm/e500_tlb.c | 23 ++- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h index 3e31098..70bfed4 100644 --- a/arch/powerpc/kvm/e500.h +++ b/arch/powerpc/kvm/e500.h @@ -130,9 +130,9 @@ int kvmppc_e500_emul_mt_mmucsr0(struct kvmppc_vcpu_e500 *vcpu_e500, ulong value); int kvmppc_e500_emul_tlbwe(struct kvm_vcpu *vcpu); int kvmppc_e500_emul_tlbre(struct kvm_vcpu *vcpu); -int kvmppc_e500_emul_tlbivax(struct kvm_vcpu *vcpu, int ra, int rb); -int kvmppc_e500_emul_tlbilx(struct kvm_vcpu *vcpu, int rt, int ra, int rb); -int kvmppc_e500_emul_tlbsx(struct kvm_vcpu *vcpu, int rb); +int kvmppc_e500_emul_tlbivax(struct kvm_vcpu *vcpu, gva_t ea); +int kvmppc_e500_emul_tlbilx(struct kvm_vcpu *vcpu, int rt, gva_t ea); +int kvmppc_e500_emul_tlbsx(struct kvm_vcpu *vcpu, gva_t ea); int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500); void kvmppc_e500_tlb_uninit(struct kvmppc_vcpu_e500 *vcpu_e500); diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c index 8b99e07..81288f7 100644 --- a/arch/powerpc/kvm/e500_emulate.c +++ b/arch/powerpc/kvm/e500_emulate.c @@ -82,6 +82,17 @@ static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu, int rb) } #endif +static inline ulong kvmppc_get_ea_indexed(struct kvm_vcpu *vcpu, int ra, int rb) +{ + ulong ea; + + ea = kvmppc_get_gpr(vcpu, rb); + if (ra) + ea += kvmppc_get_gpr(vcpu, ra); + + return ea; +} + Please move this one to arch/powerpc/include/asm/kvm_ppc.h. Yep. This is similar with what I had in my internal version before emulation refactoring took place upstream. The only difference is that I split the embedded and server implementation touching this files: arch/powerpc/include/asm/kvm_booke.h arch/powerpc/include/asm/kvm_book3s.h Which approach do you prefer? int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, unsigned int inst, int *advance) { @@ -89,6 +100,7 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, int ra = get_ra(inst); int rb = get_rb(inst); int rt = get_rt(inst); + gva_t ea; switch (get_op(inst)) { case 31: @@ -113,15 +125,18 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, break; case XOP_TLBSX: - emulated = kvmppc_e500_emul_tlbsx(vcpu,rb); + ea = kvmppc_get_ea_indexed(vcpu, ra, rb); + emulated = kvmppc_e500_emul_tlbsx(vcpu, ea); break; case XOP_TLBILX: - emulated = kvmppc_e500_emul_tlbilx(vcpu, rt, ra, rb); + ea = kvmppc_get_ea_indexed(vcpu, ra, rb); + emulated = kvmppc_e500_emul_tlbilx(vcpu, rt, ea); What's the point in hiding ra+rb, but not rt? I like the idea of hiding the register semantics, but please move rt into a local variable that gets passed as pointer to kvmppc_e500_emul_tlbilx. Why to send it as a pointer? rt which should be rather named t in this case is an [in] value for tlbilx, according to section 6.11.4.9 in the PowerISA 2.06b. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, July 04, 2012 4:34 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs On 25.06.2012, at 14:26, Mihai Caraman wrote: Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs for 64-bit hosts. Please also implement a ONE_REG interface while at it. Over time, I'd like to move towards ONE_REG instead of the messy regs/sregs API. ONE_REG doesn't seem to be implemented at all for book3e, I looked at kvm_vcpu_ioctl_set_one_reg/kvm_vcpu_ioctl_get_one_reg in booke.c file. I can take care of it soon but in a different patch set. It's ok like this? -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Thursday, July 05, 2012 3:13 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs On 07/05/2012 01:49 PM, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, July 04, 2012 4:34 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs On 25.06.2012, at 14:26, Mihai Caraman wrote: Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs for 64-bit hosts. Please also implement a ONE_REG interface while at it. Over time, I'd like to move towards ONE_REG instead of the messy regs/sregs API. ONE_REG doesn't seem to be implemented at all for book3e, I looked at kvm_vcpu_ioctl_set_one_reg/kvm_vcpu_ioctl_get_one_reg in booke.c file. I can take care of it soon but in a different patch set. It's ok like this? Do it in a different patch, but as part of this patch set. Hmm ... then if you don't disagree I will do it as a prerequisite patch since I want to keep this patchset strictly for 64-bit support. I am not familiar with ONE_REG, is qemu tailored to use it? I need a way to test it. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 13/17] PowerPC: booke64: Use SPRG0/3 scratch for bolted TLB miss crit int
-Original Message- From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] Sent: Wednesday, June 27, 2012 1:16 AM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org; Anton Blanchard Subject: Re: [RFC PATCH 13/17] PowerPC: booke64: Use SPRG0/3 scratch for bolted TLB miss crit int On Mon, 2012-06-25 at 15:26 +0300, Mihai Caraman wrote: Embedded.Hypervisor category defines GSPRG0..3 physical registers for guests. Avoid SPRG4-7 usage as scratch in host exception handlers, otherwise guest SPRG4-7 registers will be clobbered. For bolted TLB miss exception handlers, which is the version currently supported by KVM, use SPRN_SPRG_GEN_SCRATCH (aka SPRG0) instead of SPRN_SPRG_TLB_SCRATCH (aka SPRG6) and replace TLB with GEN PACA slots to keep consitency. For critical exception handler use SPRG3 instead of SPRG7. Beware with SPRG3 usage. It's user space visible and we plan to use it for other things (see Anton's patch to stick topology information in there for use by the vdso). If you clobber it, you may want to restore it later. In booke3e case SPRG3 will not be clobbered by the guests which access GSPRG3, but by the host exception handler. This means that we will have to restore SPRG3 even in the absence of KVM. My proposal is to add a PACA slot for r13 and save it in the same way you did with r12 in TLB_MISS_PROLOG. Then we can restore SPRG3 right in the prolog thus also avoiding to deal with it in KVM. The EXCEPTION_PROLOG is a common define for GEN/DBG/CRIT/MC, we use addition defines to specialize just the CRIT case. I think Anton's patch should put the proper value we want in the PACA anyway since we also need to restore it on exit from KVM, so you can still use it as scratch, just restore the value before going to C. I just saw last iteration of Anton's vsdo patch that matches your description. Cheers, -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 02/17] KVM: PPC64: booke: Add EPCR support in mtspr/mfspr emulation
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Wednesday, July 04, 2012 4:22 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [RFC PATCH 02/17] KVM: PPC64: booke: Add EPCR support in mtspr/mfspr emulation On 25.06.2012, at 14:26, Mihai Caraman wrote: Add EPCR support in booke mtspr/mfspr emulation. EPCR register is defined only for 64-bit and HV categories, so it shoud be available only on 64- bit virtual processors. Undefine the support for 32-bit builds. Define a reusable setter function for vcpu's EPCR. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/booke.c | 12 +++- arch/powerpc/kvm/booke.h |6 ++ arch/powerpc/kvm/booke_emulate.c | 13 - 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 72f13f4..f9fa260 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -13,7 +13,7 @@ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * * Copyright IBM Corp. 2007 - * Copyright 2010-2011 Freescale Semiconductor, Inc. + * Copyright 2010-2012 Freescale Semiconductor, Inc. * * Authors: Hollis Blanchard holl...@us.ibm.com * Christian Ehrhardt ehrha...@linux.vnet.ibm.com @@ -1243,6 +1243,16 @@ void kvmppc_core_commit_memory_region(struct kvm *kvm, { } +#ifdef CONFIG_64BIT +void kvmppc_set_epcr(struct kvm_vcpu *vcpu, u32 new_epcr) +{ + vcpu-arch.epcr = new_epcr; + vcpu-arch.shadow_epcr = ~SPRN_EPCR_GICM; + if (vcpu-arch.epcr SPRN_EPCR_ICM) + vcpu-arch.shadow_epcr |= SPRN_EPCR_GICM; Why would the setter be #ifdef CONFIG_64BIT? EPCR exists on e500mc too, no? Please only #ifdef the GICM bits out. kvmppc_set_epcr deals with guest EPCR and EPCR does not exist on a virtual e500mc as detailed in patch's comment. All callers are also guarded by #ifdef CONFIG_64BIT, my assumption was that we will not support a virtual core with 64-bit category on a 32-bit host. Alex -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks
-Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, July 04, 2012 5:30 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; KVM list; linuxppc-dev; qemu- p...@nongnu.org List; Benjamin Herrenschmidt Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks On 25.06.2012, at 14:26, Mihai Caraman wrote: Hook DO_KVM macro to 64-bit booke in a optimal way similar to 32-bit booke see head_fsl_booke.S file. Extend interrupt handlers' parameter list with interrupt vector numbers to accomodate the macro. Rework Guest Doorbell handler to use the proper GSRRx save/restore registers. Only the bolted version of tlb miss handers is addressed now. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kernel/exceptions-64e.S | 114 --- --- arch/powerpc/mm/tlb_low_64e.S| 14 +++- 2 files changed, 92 insertions(+), 36 deletions(-) diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S index 06f7aec..a60f81f 100644 --- a/arch/powerpc/kernel/exceptions-64e.S +++ b/arch/powerpc/kernel/exceptions-64e.S @@ -25,6 +25,8 @@ #include asm/ppc-opcode.h #include asm/mmu.h #include asm/hw_irq.h +#include asm/kvm_asm.h +#include asm/kvm_booke_hv_asm.h /* XXX This will ultimately add space for a special exception save * structure used to save things like SRR0/SRR1, SPRGs, MAS, etc... @@ -34,13 +36,24 @@ */ #define SPECIAL_EXC_FRAME_SIZE INT_FRAME_SIZE +#ifdef CONFIG_KVM_BOOKE_HV +#define KVM_BOOKE_HV_MFSPR(reg, spr) \ + BEGIN_FTR_SECTION \ + mfspr reg, spr; \ + END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) +#else +#define KVM_BOOKE_HV_MFSPR(reg, spr) +#endif Bleks - this is ugly. I agree :) But I opted to keep the optimizations done for 32-bit. Do we really need to open-code the #ifdef here? 32-bit implementation fortunately use asm macros, we can't nest defines. Can't the feature section code determine that the feature is disabled and just always not include the code? CPU_FTR_EMB_HV is set even if KVM is not configured. + /* Exception prolog code for all exceptions */ -#define EXCEPTION_PROLOG(n, type, srr0, srr1, addition) \ +#define EXCEPTION_PROLOG(n, intnum, type, srr0, srr1, addition) \ mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \ mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \ std r10,PACA_EX##type+EX_R10(r13); \ std r11,PACA_EX##type+EX_R11(r13); \ mfcrr10;/* save CR */ \ + KVM_BOOKE_HV_MFSPR(r11,srr1); \ + DO_KVM intnum,srr1;\ So if DO_KVM already knows srr1, why explicitly do something with it the line above, and not in DO_KVM itself? srr1 is used to expand the interrupt handler symbol name while r11 is used for the actual MSR[GS] optimal check: mtocrf 0x80, r11 -/* Guest Doorbell */ - MASKABLE_EXCEPTION(0x2c0, guest_doorbell, .unknown_exception, ACK_NONE) +/* + * Guest doorbell interrupt + * This general exception use GSRRx save/restore registers + */ + START_EXCEPTION(guest_doorbell); + EXCEPTION_PROLOG(0x2c0, BOOKE_INTERRUPT_GUEST_DBELL, GEN, +SPRN_GSRR0, SPRN_GSRR1, PROLOG_ADDITION_NONE) + EXCEPTION_COMMON(0x2c0, PACA_EXGEN, INTS_KEEP) + addir3,r1,STACK_FRAME_OVERHEAD + bl .save_nvgprs + INTS_RESTORE_HARD + bl .unknown_exception + b .ret_from_except This is independent of DO_KVM, right? Yes, just kvm_handler definitions in bookehv_interrupts.S depends on this. /* Guest Doorbell critical Interrupt */ START_EXCEPTION(guest_doorbell_crit); - CRIT_EXCEPTION_PROLOG(0x2e0, PROLOG_ADDITION_NONE) + CRIT_EXCEPTION_PROLOG(0x2e0, BOOKE_INTERRUPT_GUEST_DBELL_CRIT, + PROLOG_ADDITION_NONE) Shouldn't this one also use GSRR? No, this is a critical exception. -.macro tlb_prolog_bolted addr +.macro tlb_prolog_bolted intnum addr mtspr SPRN_SPRG_TLB_SCRATCH,r13 mfspr r13,SPRN_SPRG_PACA std r10,PACA_EXTLB+EX_TLB_R10(r13) mfcrr10 std r11,PACA_EXTLB+EX_TLB_R11(r13) +#ifdef CONFIG_KVM_BOOKE_HV +BEGIN_FTR_SECTION + mfspr r11, SPRN_SRR1 +END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) +#endif This thing really should vanish behind DO_KVM :) Then let's do it first for 32-bit ;) -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 15/17] KVM: PPC64: bookehv: Add support for interrupt handling
-Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Wednesday, July 04, 2012 6:14 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 15/17] KVM: PPC64: bookehv: Add support for interrupt handling On 25.06.2012, at 14:26, Mihai Caraman wrote: Add bookehv interrupt handling support for 64-bit hosts. Change common stack layout to refer PPC_LR_STKOFF kernel constant. Dispatch the 64-bit execution flow to the existing kvm_handler_common asm macro. Update input register values documentation. Only the bolted version of TLB miss exception handlers is supported now. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/include/asm/kvm_booke_hv_asm.h | 12 +++- arch/powerpc/kvm/bookehv_interrupts.S | 120 +-- 2 files changed, 122 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/include/asm/kvm_booke_hv_asm.h b/arch/powerpc/include/asm/kvm_booke_hv_asm.h index 30a600f..8be6f87 100644 --- a/arch/powerpc/include/asm/kvm_booke_hv_asm.h +++ b/arch/powerpc/include/asm/kvm_booke_hv_asm.h @@ -1,5 +1,5 @@ /* - * Copyright 2010-2011 Freescale Semiconductor, Inc. + * Copyright 2010-2012 Freescale Semiconductor, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License, version 2, as @@ -17,6 +17,7 @@ * there are no exceptions for which we fall through directly to * the normal host handler. * + * 32-bit host * Expected inputs (normal exceptions): * SCRATCH0 = saved r10 * r10 = thread struct @@ -33,6 +34,15 @@ * *(r8 + GPR9) = saved r9 * *(r8 + GPR10) = saved r10 (r10 not yet clobbered) * *(r8 + GPR11) = saved r11 + * + * 64-bit host + * Expected inputs (exception types GEN/DBG/CRIT/MC): + * r13 = PACA_POINTER + * r10 = saved CR + * SPRN_SPRG_##type##_SCRATCH = saved r13 + * *(r13 + PACA_EX##type + EX_R10) = saved r10 + * *(r13 + PACA_EX##type + EX_R11) = saved r11 + * Only the bolted version of TLB miss exception handlers is supported now. */ .macro DO_KVM intno srr1 #ifdef CONFIG_KVM_BOOKE_HV diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S index dff8ed4..04097de 100644 --- a/arch/powerpc/kvm/bookehv_interrupts.S +++ b/arch/powerpc/kvm/bookehv_interrupts.S @@ -12,10 +12,11 @@ * along with this program; if not, write to the Free Software * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. + * Copyright (C) 2010-2012 Freescale Semiconductor, Inc. * * Author: Varun Sethi varun.se...@freescale.com * Author: Scott Wood scotw...@freescale.com + * Author: Mihai Caraman mihai.cara...@freescale.com * * This file is derived from arch/powerpc/kvm/booke_interrupts.S */ @@ -30,7 +31,11 @@ #include asm/bitsperlong.h #include asm/thread_info.h +#ifdef CONFIG_64BIT +#include asm/exception-64e.h +#else #include ../kernel/head_booke.h /* for THREAD_NORMSAVE() */ +#endif +#ifdef CONFIG_64BIT +/* + * For input register values, see arch/powerpc/include/asm/kvm_booke_hv_asm.h + */ +.macro kvm_handler intno scratch, paca_ex, ex_r10, ex_r11, srr0, srr1, flags + _GLOBAL(kvmppc_handler_\intno\()_\srr1) Is this code so vastly different from the 32bit variant that they can't be the same with a few simple ifdef's here and there? As you can see from input register values things are quite different. I strived to keep the code common, the only divergence is in the kvm_handler definitions. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks
From: Alexander Graf [ag...@suse.de] Sent: Wednesday, July 04, 2012 6:45 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; KVM list; linuxppc-dev; qemu-...@nongnu.org List; Benjamin Herrenschmidt Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks On 04.07.2012, at 17:27, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: Alexander Graf [mailto:ag...@suse.de] Sent: Wednesday, July 04, 2012 5:30 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; KVM list; linuxppc-dev; qemu- p...@nongnu.org List; Benjamin Herrenschmidt Subject: Re: [Qemu-ppc] [RFC PATCH 12/17] PowerPC: booke64: Add DO_KVM kernel hooks On 25.06.2012, at 14:26, Mihai Caraman wrote: Hook DO_KVM macro to 64-bit booke in a optimal way similar to 32-bit booke see head_fsl_booke.S file. Extend interrupt handlers' parameter list with interrupt vector numbers to accomodate the macro. Rework Guest Doorbell handler to use the proper GSRRx save/restore registers. Only the bolted version of tlb miss handers is addressed now. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kernel/exceptions-64e.S | 114 --- --- arch/powerpc/mm/tlb_low_64e.S| 14 +++- 2 files changed, 92 insertions(+), 36 deletions(-) diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S index 06f7aec..a60f81f 100644 --- a/arch/powerpc/kernel/exceptions-64e.S +++ b/arch/powerpc/kernel/exceptions-64e.S @@ -25,6 +25,8 @@ #include asm/ppc-opcode.h #include asm/mmu.h #include asm/hw_irq.h +#include asm/kvm_asm.h +#include asm/kvm_booke_hv_asm.h /* XXX This will ultimately add space for a special exception save * structure used to save things like SRR0/SRR1, SPRGs, MAS, etc... @@ -34,13 +36,24 @@ */ #define SPECIAL_EXC_FRAME_SIZE INT_FRAME_SIZE +#ifdef CONFIG_KVM_BOOKE_HV +#define KVM_BOOKE_HV_MFSPR(reg, spr) \ + BEGIN_FTR_SECTION \ + mfspr reg, spr; \ + END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) +#else +#define KVM_BOOKE_HV_MFSPR(reg, spr) +#endif Bleks - this is ugly. I agree :) But I opted to keep the optimizations done for 32-bit. Do we really need to open-code the #ifdef here? 32-bit implementation fortunately use asm macros, we can't nest defines. Can't the feature section code determine that the feature is disabled and just always not include the code? CPU_FTR_EMB_HV is set even if KVM is not configured. I don't get the point then. Why not have the whole DO_KVM masked under FTR_SECTION_IFSET(CPU_FTR_EMB_HV)? Are there book3s_64 implementations without HV? I guess you refer to book3e_64. I don't know all implementations but Embedded.HV category is optional. Can't we just mfspr unconditionally in DO_KVM? I think Scott should better answer this question, I don't know why he opted for the other approach. -/* Guest Doorbell */ - MASKABLE_EXCEPTION(0x2c0, guest_doorbell, .unknown_exception, ACK_NONE) +/* + * Guest doorbell interrupt + * This general exception use GSRRx save/restore registers + */ + START_EXCEPTION(guest_doorbell); + EXCEPTION_PROLOG(0x2c0, BOOKE_INTERRUPT_GUEST_DBELL, GEN, +SPRN_GSRR0, SPRN_GSRR1, PROLOG_ADDITION_NONE) + EXCEPTION_COMMON(0x2c0, PACA_EXGEN, INTS_KEEP) + addir3,r1,STACK_FRAME_OVERHEAD + bl .save_nvgprs + INTS_RESTORE_HARD + bl .unknown_exception + b .ret_from_except This is independent of DO_KVM, right? Yes, just kvm_handler definitions in bookehv_interrupts.S depends on this. Then please split it out into a separate patch. Can you be more precise, are you referring to guest_doorbell exception handler? -.macro tlb_prolog_bolted addr +.macro tlb_prolog_bolted intnum addr mtspr SPRN_SPRG_TLB_SCRATCH,r13 mfspr r13,SPRN_SPRG_PACA std r10,PACA_EXTLB+EX_TLB_R10(r13) mfcrr10 std r11,PACA_EXTLB+EX_TLB_R11(r13) +#ifdef CONFIG_KVM_BOOKE_HV +BEGIN_FTR_SECTION + mfspr r11, SPRN_SRR1 +END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) +#endif This thing really should vanish behind DO_KVM :) Then let's do it first for 32-bit ;) You could #ifdef it in DO_KVM for 64-bit for now. IIRC it's not done on 32-bit because the register value is used even beyond DO_KVM there. Nope, 32-bit code is also guarded by CONFIG_KVM_BOOKE_HV. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [Qemu-ppc] [RFC PATCH 15/17] KVM: PPC64: bookehv: Add support for interrupt handling
On 04.07.2012, at 17:37, Caraman Mihai Claudiu-B02008 wrote: -Original Message- From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc- ow...@vger.kernel.org] On Behalf Of Alexander Graf Sent: Wednesday, July 04, 2012 6:14 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [Qemu-ppc] [RFC PATCH 15/17] KVM: PPC64: bookehv: Add support for interrupt handling Is this code so vastly different from the 32bit variant that they can't be the same with a few simple ifdef's here and there? As you can see from input register values things are quite different. I strived to keep the code common, the only divergence is in the kvm_handler definitions. What a shame :(. A lot of it looks very very similar. The Devil is in the details ;) -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
-Original Message- From: Wood Scott-B07421 Sent: Wednesday, June 27, 2012 1:35 AM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs On 06/25/2012 07:26 AM, Mihai Caraman wrote: Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs for 64-bit hosts. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/booke.c | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index f9fa260..d15c4b5 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -1052,6 +1052,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, u64 tb = get_tb(); sregs-u.e.features |= KVM_SREGS_E_BASE; +#ifdef CONFIG_64BIT + sregs-u.e.features |= KVM_SREGS_E_64;0 #endif sregs-u.e.csrr0 = vcpu-arch.csrr0; sregs-u.e.csrr1 = vcpu-arch.csrr1; @@ -1063,6 +1066,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, sregs-u.e.dec = kvmppc_get_dec(vcpu, tb); sregs-u.e.tb = tb; sregs-u.e.vrsave = vcpu-arch.vrsave; +#ifdef CONFIG_64BIT + sregs-u.e.epcr = vcpu-arch.epcr; +#endif } static int set_sregs_base(struct kvm_vcpu *vcpu, @@ -1071,6 +1077,11 @@ static int set_sregs_base(struct kvm_vcpu *vcpu, if (!(sregs-u.e.features KVM_SREGS_E_BASE)) return 0; +#ifdef CONFIG_64BIT + if (!(sregs-u.e.features KVM_SREGS_E_64)) + return 0; +#endif This means that a QEMU targeting a 32-bit guest won't be able to set any special registers, if it sets feature bits manually rather than getting them from GET_SREGS. I had some concerns about his. I only check qemu ppc code which uses get/set approach and I followed the BASE model. Now I see that qemu x86 set them manually :( Why do we care if the caller set or not BASE? -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 10/17] PowerPC: booke64: Refactor exception prolog for save/restore regs
-Original Message- From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] Sent: Wednesday, June 27, 2012 1:13 AM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [RFC PATCH 10/17] PowerPC: booke64: Refactor exception prolog for save/restore regs On Mon, 2012-06-25 at 15:26 +0300, Mihai Caraman wrote: Refactor exception prolog to allow save/restore register parameters. Add addition none definition for exception prolog usage. This is needed for exceptions like Guest Doorbell that use GSRRx regsiters which do not map on exception type. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kernel/exceptions-64e.S | 23 --- 1 files changed, 8 insertions(+), 15 deletions(-) diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S index 7215cc2..52aa96b 100644 --- a/arch/powerpc/kernel/exceptions-64e.S +++ b/arch/powerpc/kernel/exceptions-64e.S @@ -35,7 +35,7 @@ #defineSPECIAL_EXC_FRAME_SIZE INT_FRAME_SIZE /* Exception prolog code for all exceptions */ -#define EXCEPTION_PROLOG(n, type, addition) \ +#define EXCEPTION_PROLOG(n, type, srr0, srr1, addition) \ mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \ mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \ std r10,PACA_EX##type+EX_R10(r13); \ @@ -44,54 +44,47 @@ addition; /* additional code for that exc. */ \ std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \ stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \ - mfspr r11,SPRN_##type##_SRR1;/* what are we coming from */\ + mfspr r11,srr1;/* what are we coming from */ \ type##_SET_KSTACK; /* get special stack if necessary */\ andi. r10,r11,MSR_PR; /* save stack pointer */\ beq 1f; /* branch around if supervisor */ \ ld r1,PACAKSAVE(r13); /* get kernel stack coming from usr */\ 1: cmpdi cr1,r1,0; /* check if SP makes sense */ \ bge-cr1,exc_##n##_bad_stack;/* bad stack (TODO: out of line) */ \ - mfspr r10,SPRN_##type##_SRR0; /* read SRR0 before touching stack */ + mfspr r10,srr0; /* read SRR0 before touching stack */ No, use the existing macro, use a ##type## specific to guest doorbells, with appropriate definitions of the corresponding SPRN_ macros. I assume that specific PACA_EX, SCRATCH and SET_KSTACK definitions will fallback to GEN. Cheers, Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
-Original Message- From: Avi Kivity [mailto:a...@redhat.com] Sent: Monday, June 25, 2012 4:00 PM To: Caraman Mihai Claudiu-B02008 Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- d...@lists.ozlabs.org; qemu-...@nongnu.org Subject: Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs On 06/25/2012 03:26 PM, Mihai Caraman wrote: Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs for 64-bit hosts. Signed-off-by: Mihai Caraman mihai.cara...@freescale.com --- arch/powerpc/kvm/booke.c | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index f9fa260..d15c4b5 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -1052,6 +1052,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, u64 tb = get_tb(); sregs-u.e.features |= KVM_SREGS_E_BASE; +#ifdef CONFIG_64BIT + sregs-u.e.features |= KVM_SREGS_E_64; #endif This is an ABI, but I see no trace of it in Documentation. The ppc sregs documentation in api.txt redirects to arch/powerpc/include/asm/kvm.h. Isn't this enough? Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev