RE: [PATCH v2 1/4] KVM: PPC: e500mc: Revert "add load inst fixup"
> From: Alexander Graf > Sent: Friday, May 2, 2014 12:24 PM > To: Caraman Mihai Claudiu-B02008 > Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; > linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH v2 1/4] KVM: PPC: e500mc: Revert "add load inst fixup" > > On 05/01/2014 02:45 AM, Mihai Caraman wrote: > > The commit 1d628af7 "add load inst fixup" made an attempt to handle > > failures generated by reading the guest current instruction. The fixup > > code that was added works by chance hiding the real issue. > > > > Load external pid (lwepx) instruction, used by KVM to read guest > > instructions, is executed in a subsituted guest translation context > > (EPLC[EGS] = 1). In consequence lwepx's TLB error and data storage > > interrupts need to be handled by KVM, even though these interrupts > > are generated from host context (MSR[GS] = 0). > > > > Currently, KVM hooks only interrupts generated from guest context > > (MSR[GS] = 1), doing minimal checks on the fast path to avoid host > > performance degradation. As a result, the host kernel handles lwepx > > faults searching the faulting guest data address (loaded in DEAR) in > > its own Logical Partition ID (LPID) 0 context. In case a host translation > > is found the execution returns to the lwepx instruction instead of the > > fixup, the host ending up in an infinite loop. > > > > Revert the commit "add load inst fixup". lwepx issue will be addressed > > in a subsequent patch without needing fixup code. > > > > Signed-off-by: Mihai Caraman > > Just a random idea: Could we just switch IVOR2 during the critical lwepx > phase? In fact, we could even do that later when we're already in C code > and try to recover the last instruction. The code IVOR2 would point to > would simply set the register we're trying to read to as LAST_INST_FAIL > and rfi. This was the first idea that sprang to my mind inspired from how DO_KVM is hooked on PR. I actually did a simple POC for e500mc/e5500, but this will not work on e6500 which has shared IVORs between HW threads. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2 1/4] KVM: PPC: e500mc: Revert "add load inst fixup"
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Sunday, May 04, 2014 1:14 AM > To: Caraman Mihai Claudiu-B02008 > Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH v2 1/4] KVM: PPC: e500mc: Revert "add load inst > fixup" > > > > Am 03.05.2014 um 01:14 schrieb "mihai.cara...@freescale.com" > : > > >> From: Alexander Graf > >> Sent: Friday, May 2, 2014 12:24 PM > > This was the first idea that sprang to my mind inspired from how DO_KVM > > is hooked on PR. I actually did a simple POC for e500mc/e5500, but this > will > > not work on e6500 which has shared IVORs between HW threads. > > What if we combine the ideas? On read we flip the IVOR to a separate > handler that checks for a field in the PACA. Only if that field is set, > we treat the fault as kvm fault, otherwise we jump into the normal > handler. > > I suppose we'd have to also take a lock to make sure we don't race with > the other thread when it wants to also read a guest instruction, but you > get the idea. This might be a solution for TLB eviction but not for execute-but-not-read entries which requires access from host context. > > I have no idea whether this would be any faster, it's more of a > brainstorming thing really. But regardless this patch set would be a move > into the right direction. > > Btw, do we have any guarantees that we don't get scheduled away before we > run kvmppc_get_last_inst()? If we run on a different core we can't read > the inst anymore. Hrm. It was your suggestion to move the logic from kvmppc_handle_exit() irq disabled area to kvmppc_get_last_inst(): http://git.freescale.com/git/cgit.cgi/ppc/sdk/linux.git/tree/arch/powerpc/kvm/booke.c Still, what is wrong if we get scheduled on another core? We will emulate again and the guest will populate the TLB on the new core. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2 3/4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Friday, May 02, 2014 12:55 PM
> To: Caraman Mihai Claudiu-B02008
> Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org
> Subject: Re: [PATCH v2 3/4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
>
> On 05/01/2014 02:45 AM, Mihai Caraman wrote:
...
> > diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> > index 4096f16..6e7c358 100644
> > --- a/arch/powerpc/include/asm/kvm_ppc.h
> > +++ b/arch/powerpc/include/asm/kvm_ppc.h
> > @@ -72,6 +72,8 @@ extern int kvmppc_sanity_check(struct kvm_vcpu
> *vcpu);
> > extern int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu);
> > extern void kvmppc_subarch_vcpu_uninit(struct kvm_vcpu *vcpu);
> >
> > +extern int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, u32 *inst);
>
> Phew. Moving this into a separate function sure has some performance
> implications. Was there no way to keep it in a header?
>
> You could just move it into its own .h file which we include after
> kvm_ppc.h. That way everything's available. That would also help me a
> lot with the little endian port where I'm also struggling with header
> file inclusion order and kvmppc_need_byteswap().
Great, I will do this.
> > diff --git a/arch/powerpc/kvm/book3s_pr.c
> b/arch/powerpc/kvm/book3s_pr.c
> > index c5c052a..b7fffd1 100644
> > --- a/arch/powerpc/kvm/book3s_pr.c
> > +++ b/arch/powerpc/kvm/book3s_pr.c
> > @@ -608,12 +608,9 @@ void kvmppc_giveup_ext(struct kvm_vcpu *vcpu,
> ulong msr)
> >
> > static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
> > {
> > - ulong srr0 = kvmppc_get_pc(vcpu);
> > - u32 last_inst = kvmppc_get_last_inst(vcpu);
> > - int ret;
> > + u32 last_inst;
> >
> > - ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> > - if (ret == -ENOENT) {
> > + if (kvmppc_get_last_inst(vcpu, &last_inst) == -ENOENT) {
>
> ENOENT?
You have to tell us :) Why does kvmppc_ld() mix emulation_result
enumeration with generic errors? Do you want to change that and
use EMULATE_FAIL instead?
>
> > ulong msr = vcpu->arch.shared->msr;
> >
> > msr = kvmppc_set_field(msr, 33, 33, 1);
> > @@ -867,15 +864,18 @@ int kvmppc_handle_exit_pr(struct kvm_run *run,
> struct kvm_vcpu *vcpu,
> > {
> > enum emulation_result er;
> > ulong flags;
> > + u32 last_inst;
> >
> > program_interrupt:
> > flags = vcpu->arch.shadow_srr1 & 0x1full;
> > + kvmppc_get_last_inst(vcpu, &last_inst);
>
> No check for the return value?
Should we queue a program exception and resume guest?
>
> >
> > if (vcpu->arch.shared->msr & MSR_PR) {
> > #ifdef EXIT_DEBUG
> > - printk(KERN_INFO "Userspace triggered 0x700 exception
> at 0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
> > + pr_info("Userspace triggered 0x700 exception at\n"
> > + "0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), last_inst);
> > #endif
> > - if ((kvmppc_get_last_inst(vcpu) & 0xff0007ff) !=
> > + if ((last_inst & 0xff0007ff) !=
> > (INS_DCBZ & 0xfff7)) {
> > kvmppc_core_queue_program(vcpu, flags);
> > r = RESUME_GUEST;
> > @@ -894,7 +894,7 @@ program_interrupt:
> > break;
> > case EMULATE_FAIL:
> > printk(KERN_CRIT "%s: emulation at %lx failed
> (%08x)\n",
> > - __func__, kvmppc_get_pc(vcpu),
> kvmppc_get_last_inst(vcpu));
> > + __func__, kvmppc_get_pc(vcpu), last_inst);
> > kvmppc_core_queue_program(vcpu, flags);
> > r = RESUME_GUEST;
> > break;
> > @@ -911,8 +911,12 @@ program_interrupt:
> > break;
> > }
> > case BOOK3S_INTERRUPT_SYSCALL:
> > + {
> > + u32 last_sc;
> > +
> > + kvmppc_get_last_sc(vcpu, &last_sc);
>
> No check for the return value?
The existing code does not handle KVM_INST_FETCH_FAILED.
How should we continue if papr is enabled and last_sc fails?
>
> > if (vcpu->arch.papr_enabled &&
> > - (kvmppc_get_last_sc(vcpu) == 0x4422) &&
> > + (last_sc == 0x4422) &&
> > !(vcpu->arch.shared->msr & MSR_PR)) {
> > /* SC 1 papr hypercalls */
> > ulong cmd = kvmppc_get_gpr(vcpu, 3);
> > @@ -957,6 +961,7 @@ program_interrupt:
> > r = RESUME_GUEST;
> > }
> > break;
> > + }
> > case BOOK3S_INTERRUPT_FP_UNAVAIL:
> > case BOOK3S_INTERRUPT_ALTIVEC:
> > case BOOK3S_INTERRUPT_VSX:
> > @@ -985,15 +990,20 @@ program_interrupt:
> > break;
> > }
> > case BOOK3S_INTERRUPT_ALIGNMENT:
> > + {
> > + u32 last_inst;
> > +
>
RE: [PATCH 2/2] powerpc/e6500: hw tablewalk: fix recursive tlb lock on cpu 0
> -Original Message- > From: Linuxppc-dev [mailto:linuxppc-dev- > bounces+mihai.caraman=freescale@lists.ozlabs.org] On Behalf Of Scott > Wood > Sent: Friday, May 23, 2014 12:45 AM > To: linuxppc-dev@lists.ozlabs.org > Cc: Wood Scott-B07421 > Subject: [PATCH 2/2] powerpc/e6500: hw tablewalk: fix recursive tlb lock > on cpu 0 > > Commit 82d86de25b9c99db546e17c6f7ebf9a691da557e "TLB lock recursive" > introduced a bug whereby cpu 0 uses the same value for "lock held" as > is used to indicate that the lock is free. Isn't his what spin lock implementation solves by combines paca_index with lock_token? Can't we have a common approach? > Add one to the CPU value to ensure we do not use zero as a "lock held" > value. The CPU value is loaded in r10 from tlb_miss_common_e6500. "TLB lock recursive" commit also introduced this misleading comment: We are entered with: r10 = cpu number -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Thursday, June 12, 2014 8:05 PM > To: Caraman Mihai Claudiu-B02008 > Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org; Wood Scott-B07421 > Subject: Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition > on vcpu schedule > > On 06/12/2014 04:00 PM, Mihai Caraman wrote: > > On vcpu schedule, the condition checked for tlb pollution is too tight. > > The tlb entries of one vcpu are polluted when a different vcpu from the > > same partition runs in-between. Relax the current tlb invalidation > > condition taking into account the lpid. > > > > Signed-off-by: Mihai Caraman freescale.com> > > Your mailer is broken? :) > This really should be an @. > > I think this should work. Scott, please ack. Alex, you were right. I screwed up the patch description by inverting relax and tight terms :) It should have been more like this: KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule On vcpu schedule, the condition checked for tlb pollution is too loose. The tlb entries of a vcpu are polluted (vs stale) only when a different vcpu within the same logical partition runs in-between. Optimize the tlb invalidation condition taking into account the lpid. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Tuesday, June 17, 2014 12:09 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] KVM: PPC: e500mc: Relax tlb invalidation condition > on vcpu schedule > > > On 13.06.14 21:42, Scott Wood wrote: > > On Fri, 2014-06-13 at 16:55 +0200, Alexander Graf wrote: > >> On 13.06.14 16:43, mihai.cara...@freescale.com wrote: > >>>> -Original Message- > >>>> From: Alexander Graf [mailto:ag...@suse.de] > >>>> Sent: Thursday, June 12, 2014 8:05 PM > >>>> To: Caraman Mihai Claudiu-B02008 > >>>> Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- > >>>> d...@lists.ozlabs.org; Wood Scott-B07421 > >>>> Subject: Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation > condition > >>>> on vcpu schedule > >>>> > >>>> On 06/12/2014 04:00 PM, Mihai Caraman wrote: > >>>>> On vcpu schedule, the condition checked for tlb pollution is too > tight. > >>>>> The tlb entries of one vcpu are polluted when a different vcpu from > the > >>>>> same partition runs in-between. Relax the current tlb invalidation > >>>>> condition taking into account the lpid. > > Can you quantify the performance improvement from this? We've had bugs > > in this area before, so let's make sure it's worth it before making > this > > more complicated. > > > >>>>> Signed-off-by: Mihai Caraman freescale.com> > >>>> Your mailer is broken? :) > >>>> This really should be an @. > >>>> > >>>> I think this should work. Scott, please ack. > >>> Alex, you were right. I screwed up the patch description by inverting > relax > >>> and tight terms :) It should have been more like this: > >>> > >>> KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule > >>> > >>> On vcpu schedule, the condition checked for tlb pollution is too > loose. > >>> The tlb entries of a vcpu are polluted (vs stale) only when a > different vcpu > >>> within the same logical partition runs in-between. Optimize the tlb > invalidation > >>> condition taking into account the lpid. > >> Can't we give every vcpu its own lpid? Or don't we trap on global > >> invalidates? > > That would significantly increase the odds of exhausting LPIDs, > > especially on large chips like t4240 with similarly large VMs. If we > > were to do that, the LPIDs would need to be dynamically assigned (like > > PIDs), and should probably be a separate numberspace per physical core. > > True, I didn't realize we only have so few of them. It would however > save us from most flushing as long as we have spare LPIDs available :). Yes, we had this proposal on the table for e6500 multithreaded core. This core lacks tlb write conditional instruction, so an OS needs to use locks to protect itself against concurrent tlb writes executed from sibling threads. When we expose hw treads as single-threaded vcpus (useful when the user opt not to pin vcpus), the guest can't no longer protect itself optimally (it can protect tlb writes across all threads but this is not acceptable). So instead, we found a solution at hypervisor level by assigning different logical partition ids to guest's vcpus running simultaneous on sibling hw threads. Currently in FSL SDK we allocate two lpids to each guest. I am also a proponent for using all LPID space (63 values) per (multi-threaded) physical core, which will lead to fewer invalidates on vcpu schedule and will accommodate the solution described above. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
> -Original Message- > From: Wood Scott-B07421 > Sent: Tuesday, June 17, 2014 6:36 PM > To: Caraman Mihai Claudiu-B02008 > Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH v2] KVM: PPC: e500mc: Enhance tlb invalidation > condition on vcpu schedule > > On Tue, 2014-06-17 at 12:02 +0300, Mihai Caraman wrote: > > On vcpu schedule, the condition checked for tlb pollution is too loose. > > The tlb entries of a vcpu become polluted (vs stale) only when a > different > > vcpu within the same logical partition runs in-between. Optimize the > tlb > > invalidation condition taking into account the logical partition id. > > > > With the new invalidation condition, a guest shows 4% performance > improvement > > on P5020DS while running a memory stress application with the cpu > oversubscribed, > > the other guest running a cpu intensive workload. > > See > https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-June/118547.html Thanks. The original code needs just a simple adjustment to benefit from this optimization, please review v3. - Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
> > -static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu); > > +static DEFINE_PER_CPU(struct kvm_vcpu * [KVMPPC_NR_LPIDS], > last_vcpu_on_cpu); > > Hmm, I didn't know you could express types like that. Is this special > syntax that only works for typeof? Yes, AFAIK. > No space after * Checkpatch complains about the missing space ;) > > Name should be adjusted to match, something like last_vcpu_of_lpid (with > the _on_cpu being implied by the fact that it's PER_CPU). I was thinking to the long name but it was not appealing, I will change it to last_vcpu_of_lpid. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
> -Original Message- > From: Wood Scott-B07421 > Sent: Tuesday, June 17, 2014 10:48 PM > To: Caraman Mihai Claudiu-B02008 > Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation > condition on vcpu schedule > > On Tue, 2014-06-17 at 14:42 -0500, Caraman Mihai Claudiu-B02008 wrote: > > > > -static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu); > > > > +static DEFINE_PER_CPU(struct kvm_vcpu * [KVMPPC_NR_LPIDS], > > > last_vcpu_on_cpu); > > > > > > Hmm, I didn't know you could express types like that. Is this > special > > > syntax that only works for typeof? > > > > Yes, AFAIK. > > > > > No space after * > > > > Checkpatch complains about the missing space ;) > > Checkpatch is wrong, which isn't surprising given that this is unusual > syntax. We don't normally put a space after * when used to represent a > pointer. This is not something new. See [PATCH 04/10] percpu: cleanup percpu array definitions: https://lkml.org/lkml/2009/6/24/26 -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
> -Original Message- > From: Wood Scott-B07421 > Sent: Tuesday, June 17, 2014 11:05 PM > To: Caraman Mihai Claudiu-B02008 > Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation > condition on vcpu schedule > > On Tue, 2014-06-17 at 15:02 -0500, Caraman Mihai Claudiu-B02008 wrote: > > > -Original Message- > > > From: Wood Scott-B07421 > > > Sent: Tuesday, June 17, 2014 10:48 PM > > > To: Caraman Mihai Claudiu-B02008 > > > Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc- > > > d...@lists.ozlabs.org > > > Subject: Re: [PATCH v3] KVM: PPC: e500mc: Enhance tlb invalidation > > > condition on vcpu schedule > > > > > > On Tue, 2014-06-17 at 14:42 -0500, Caraman Mihai Claudiu-B02008 > wrote: > > > > > > -static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu); > > > > > > +static DEFINE_PER_CPU(struct kvm_vcpu * [KVMPPC_NR_LPIDS], > > > > > last_vcpu_on_cpu); > > > > > > > > > > Hmm, I didn't know you could express types like that. Is this > > > special > > > > > syntax that only works for typeof? > > > > > > > > Yes, AFAIK. > > > > > > > > > No space after * > > > > > > > > Checkpatch complains about the missing space ;) > > > > > > Checkpatch is wrong, which isn't surprising given that this is > unusual > > > syntax. We don't normally put a space after * when used to represent > a > > > pointer. > > > > This is not something new. See [PATCH 04/10] percpu: cleanup percpu > array > > definitions: > > > > https://lkml.org/lkml/2009/6/24/26 > > I didn't say it was new, just unusual, and checkpatch doesn't recognize > it. Checkpatch shouldn't be blindly and silently obeyed when it says > something strange. I agree with you about the syntax and I know other cases where checkpatch is a moron. For similar corner cases checkpatch maintainers did not wanted (or found it difficult) to make an exception. I would also like to see Alex opinion on this. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Thursday, July 03, 2014 3:21 PM > To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org > Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for > SPE/FP/AltiVec int numbers > > > On 30.06.14 17:34, Mihai Caraman wrote: > > Use common BOOKE_IRQPRIO and BOOKE_INTERRUPT defines for SPE/FP/AltiVec > > which share the same interrupt numbers. > > > > Signed-off-by: Mihai Caraman > > --- > > v2: > > - remove outdated definitions > > > > arch/powerpc/include/asm/kvm_asm.h| 8 > > arch/powerpc/kvm/booke.c | 17 + > > arch/powerpc/kvm/booke.h | 4 ++-- > > arch/powerpc/kvm/booke_interrupts.S | 9 + > > arch/powerpc/kvm/bookehv_interrupts.S | 4 ++-- > > arch/powerpc/kvm/e500.c | 10 ++ > > arch/powerpc/kvm/e500_emulate.c | 10 ++ > > 7 files changed, 30 insertions(+), 32 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/kvm_asm.h > b/arch/powerpc/include/asm/kvm_asm.h > > index 9601741..c94fd33 100644 > > --- a/arch/powerpc/include/asm/kvm_asm.h > > +++ b/arch/powerpc/include/asm/kvm_asm.h > > @@ -56,14 +56,6 @@ > > /* E500 */ > > #define BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL 32 > > #define BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST 33 > > -/* > > - * 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 > > I think I'd prefer to keep them separate. What is the reason from changing your mind from ver 1? Do you want to have different defines with same values (we specifically mapped them to the hardware interrupt numbers). We already upstreamed the necessary changes in the kernel. Scott, please share your opinion here. > > > #define BOOKE_INTERRUPT_SPE_FP_ROUND 34 > > #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35 > > #define BOOKE_INTERRUPT_DOORBELL 36 > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > > index ab62109..3c86d9b 100644 > > --- a/arch/powerpc/kvm/booke.c > > +++ b/arch/powerpc/kvm/booke.c > > @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct > kvm_vcpu *vcpu, > > case BOOKE_IRQPRIO_ITLB_MISS: > > case BOOKE_IRQPRIO_SYSCALL: > > case BOOKE_IRQPRIO_FP_UNAVAIL: > > - case BOOKE_IRQPRIO_SPE_UNAVAIL: > > - case BOOKE_IRQPRIO_SPE_FP_DATA: > > + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL: > > + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST: > > #ifdef CONFIG_KVM_E500V2 >case ...SPE: > #else >case ..ALTIVEC: > #endif -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Thursday, July 03, 2014 3:29 PM > To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org > Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness > > > On 30.06.14 17:34, 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. > > How do you handle context switching with this patch applied? During most > of the guest's lifetime we never exit kvmppc_vcpu_run(), so when the > guest gets switched out all FPU state gets lost? No, we had this discussion in ver 1. The FP/VMX/VSX is implemented lazy in the kernel i.e. the unit state is not saved/restored until another thread that once claimed the unit is sched in. Since FP/VMX/VSX can be activated by the guest independent of the host, the vcpu thread is always using the unit (even if it did not claimed it once). Now, this patch optimize the sched in flow. Instead of checking on each vcpu sched in if the kernel unloaded unit's guest state for another competing host process we do this when we enter the guest. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Thursday, July 03, 2014 6:31 PM > To: Caraman Mihai Claudiu-B02008; Wood Scott-B07421; kvm- > p...@vger.kernel.org > Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for > SPE/FP/AltiVec int numbers > > > On 03.07.14 17:25, mihai.cara...@freescale.com wrote: > >> -Original Message- > >> From: Alexander Graf [mailto:ag...@suse.de] > >> Sent: Thursday, July 03, 2014 3:21 PM > >> To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org > >> Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > >> Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for > >> SPE/FP/AltiVec int numbers > >> > >> > >> On 30.06.14 17:34, Mihai Caraman wrote: > >>> Use common BOOKE_IRQPRIO and BOOKE_INTERRUPT defines for > SPE/FP/AltiVec > >>> which share the same interrupt numbers. > >>> > >>> Signed-off-by: Mihai Caraman > >>> --- > >>> v2: > >>>- remove outdated definitions > >>> > >>>arch/powerpc/include/asm/kvm_asm.h| 8 > >>>arch/powerpc/kvm/booke.c | 17 + > >>>arch/powerpc/kvm/booke.h | 4 ++-- > >>>arch/powerpc/kvm/booke_interrupts.S | 9 + > >>>arch/powerpc/kvm/bookehv_interrupts.S | 4 ++-- > >>>arch/powerpc/kvm/e500.c | 10 ++ > >>>arch/powerpc/kvm/e500_emulate.c | 10 ++ > >>>7 files changed, 30 insertions(+), 32 deletions(-) > >>> > >>> diff --git a/arch/powerpc/include/asm/kvm_asm.h > >> b/arch/powerpc/include/asm/kvm_asm.h > >>> index 9601741..c94fd33 100644 > >>> --- a/arch/powerpc/include/asm/kvm_asm.h > >>> +++ b/arch/powerpc/include/asm/kvm_asm.h > >>> @@ -56,14 +56,6 @@ > >>>/* E500 */ > >>>#define BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL 32 > >>>#define BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST 33 > >>> -/* > >>> - * 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 > >> I think I'd prefer to keep them separate. > > What is the reason from changing your mind from ver 1? Do you want to > have > > Uh, mind to point me to an email where I said I like the approach? :) You tacitly approved it in this thread ... I did not say you like it :) https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-July/108501.html -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Thursday, July 03, 2014 3:32 PM > To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org > Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support > > > On 30.06.14 17:34, Mihai Caraman wrote: > > Add KVM Book3E AltiVec support. KVM Book3E FPU support gracefully reuse > host > > infrastructure so follow the same approach for AltiVec. > > > > Signed-off-by: Mihai Caraman > > Same comment here - I fail to see how we refetch Altivec state after a > context switch. See previous comment. I also run my usual Altivec stress test consisting in a guest and host process running affine to a physical core an competing for the same unit's resources using different data sets. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 5/6 v2] KVM: PPC: Book3E: Add ONE_REG AltiVec support
> -Original Message- > From: Alexander Graf [mailto:ag...@suse.de] > Sent: Thursday, July 03, 2014 3:34 PM > To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org > Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 5/6 v2] KVM: PPC: Book3E: Add ONE_REG AltiVec support > > > On 30.06.14 17:34, Mihai Caraman wrote: > > Add ONE_REG support for AltiVec on Book3E. > > > > Signed-off-by: Mihai Caraman > > Any chance we can handle these in generic code? I expected this request :) Can we let this for a second phase to have e6500 enabled first? Can you share with us a Book3S setup so I can validate the requested changes? I already fell anxious touching strange hardware specific Book3S code without running it. -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() function
> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Thursday, July 03, 2014 4:37 PM
> To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org
> Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst()
> function
>
>
> On 28.06.14 00:49, Mihai Caraman wrote:
> > In the context of replacing kvmppc_ld() function calls with a version
> of
> > kvmppc_get_last_inst() which allow to fail, Alex Graf suggested this:
> >
> > "If we get EMULATE_AGAIN, we just have to make sure we go back into the
> guest.
> > No need to inject an ISI into the guest - it'll do that all by itself.
> > With an error returning kvmppc_get_last_inst we can just use completely
> > get rid of kvmppc_read_inst() and only use kvmppc_get_last_inst()
> instead."
> >
> > As a intermediate step get rid of kvmppc_read_inst() and only use
> kvmppc_ld()
> > instead.
> >
> > Signed-off-by: Mihai Caraman
> > ---
> > v4:
> > - new patch
> >
> > arch/powerpc/kvm/book3s_pr.c | 85 ++-
> -
> > 1 file changed, 35 insertions(+), 50 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/book3s_pr.c
> b/arch/powerpc/kvm/book3s_pr.c
> > index 15fd6c2..d247d88 100644
> > --- a/arch/powerpc/kvm/book3s_pr.c
> > +++ b/arch/powerpc/kvm/book3s_pr.c
> > @@ -665,42 +665,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu
> *vcpu, ulong fac)
> > #endif
> > }
> >
> > -static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
> > -{
> > - ulong srr0 = kvmppc_get_pc(vcpu);
> > - u32 last_inst = kvmppc_get_last_inst(vcpu);
> > - int ret;
> > -
> > - ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> > - if (ret == -ENOENT) {
> > - ulong msr = kvmppc_get_msr(vcpu);
> > -
> > - msr = kvmppc_set_field(msr, 33, 33, 1);
> > - msr = kvmppc_set_field(msr, 34, 36, 0);
> > - msr = kvmppc_set_field(msr, 42, 47, 0);
> > - kvmppc_set_msr_fast(vcpu, msr);
> > - kvmppc_book3s_queue_irqprio(vcpu,
> BOOK3S_INTERRUPT_INST_STORAGE);
> > - return EMULATE_AGAIN;
> > - }
> > -
> > - return EMULATE_DONE;
> > -}
> > -
> > -static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int
> exit_nr)
> > -{
> > -
> > - /* Need to do paired single emulation? */
> > - if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE))
> > - return EMULATE_DONE;
> > -
> > - /* Read out the instruction */
> > - if (kvmppc_read_inst(vcpu) == EMULATE_DONE)
> > - /* Need to emulate */
> > - return EMULATE_FAIL;
> > -
> > - return EMULATE_AGAIN;
> > -}
> > -
> > /* Handle external providers (FPU, Altivec, VSX) */
> > static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int
> exit_nr,
> > ulong msr)
> > @@ -1101,31 +1065,51 @@ program_interrupt:
> > case BOOK3S_INTERRUPT_VSX:
> > {
> > int ext_msr = 0;
> > + int emul;
> > + ulong pc;
> > + u32 last_inst;
> >
> > - switch (exit_nr) {
> > - case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr = MSR_FP; break;
> > - case BOOK3S_INTERRUPT_ALTIVEC:ext_msr = MSR_VEC; break;
> > - case BOOK3S_INTERRUPT_VSX:ext_msr = MSR_VSX; break;
> > - }
> > + if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE)) {
>
> Please make paired single emulation the unusual, if()'ed case, not the
> normal exit path :).
Huh ... do you have more Book3s specific requests, it will be strange if
it will still work after all this blind changes :)
-Mike
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Thursday, July 17, 2014 5:21 PM
> To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org
> Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
>
>
> On 17.07.14 13:22, Mihai Caraman wrote:
> > On book3e, guest last instruction is read on the exit path using load
> > external pid (lwepx) dedicated instruction. This load operation may
> fail
> > due to TLB eviction and execute-but-not-read entries.
> >
> > This patch lay down the path for an alternative solution to read the
> guest
> > last instruction, by allowing kvmppc_get_lat_inst() function to fail.
> > Architecture specific implmentations of kvmppc_load_last_inst() may
> read
> > last guest instruction and instruct the emulation layer to re-execute
> the
> > guest in case of failure.
> >
> > Make kvmppc_get_last_inst() definition common between architectures.
> >
> > Signed-off-by: Mihai Caraman
> > ---
...
> > diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> > index e2fd5a1..7f9c634 100644
> > --- a/arch/powerpc/include/asm/kvm_ppc.h
> > +++ b/arch/powerpc/include/asm/kvm_ppc.h
> > @@ -47,6 +47,11 @@ enum emulation_result {
> > EMULATE_EXIT_USER,/* emulation requires exit to user-space */
> > };
> >
> > +enum instruction_type {
> > + INST_GENERIC,
> > + INST_SC,/* system call */
> > +};
> > +
> > extern int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu
> *vcpu);
> > extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu
> *vcpu);
> > extern void kvmppc_handler_highmem(void);
> > @@ -62,6 +67,9 @@ extern int kvmppc_handle_store(struct kvm_run *run,
> struct kvm_vcpu *vcpu,
> >u64 val, unsigned int bytes,
> >int is_default_endian);
> >
> > +extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
> > +enum instruction_type type, u32 *inst);
> > +
> > extern int kvmppc_emulate_instruction(struct kvm_run *run,
> > struct kvm_vcpu *vcpu);
> > extern int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu
> *vcpu);
> > @@ -234,6 +242,23 @@ struct kvmppc_ops {
> > extern struct kvmppc_ops *kvmppc_hv_ops;
> > extern struct kvmppc_ops *kvmppc_pr_ops;
> >
> > +static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
> > + enum instruction_type type, u32 *inst)
> > +{
> > + int ret = EMULATE_DONE;
> > +
> > + /* Load the instruction manually if it failed to do so in the
> > +* exit path */
> > + if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
> > + ret = kvmppc_load_last_inst(vcpu, type, &vcpu-
> >arch.last_inst);
> > +
> > +
> > + *inst = (ret == EMULATE_DONE && kvmppc_need_byteswap(vcpu)) ?
> > + swab32(vcpu->arch.last_inst) : vcpu->arch.last_inst;
>
> This makes even less sense than the previous version. Either you treat
> inst as "definitely overwritten" or as "preserves previous data on
> failure".
Both v4 and v5 versions treat inst as "definitely overwritten".
>
> So either you unconditionally swap like you did before
If we make abstraction of its symmetry, KVM_INST_FETCH_FAILED is operated
in host endianness, so it doesn't need byte swap.
I agree with your reasoning if last_inst is initialized and compared with
data in guest endianess, which is not the case yet for KVM_INST_FETCH_FAILED.
> or you do
>
> if (ret == EMULATE_DONE)
> *inst = kvmppc_need_byteswap(vcpu) ? swab32(vcpu->arch.last_inst) :
> vcpu->arch.last_inst;
-Mike
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
> -Original Message-
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+mihai.caraman=freescale@lists.ozlabs.org] On Behalf Of
> mihai.cara...@freescale.com
> Sent: Friday, July 18, 2014 12:06 PM
> To: Alexander Graf; kvm-...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org; k...@vger.kernel.org
> Subject: RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
>
> > -Original Message-
> > From: Alexander Graf [mailto:ag...@suse.de]
> > Sent: Thursday, July 17, 2014 5:21 PM
> > To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org
> > Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
> fail
> >
> >
> > On 17.07.14 13:22, Mihai Caraman wrote:
> > > On book3e, guest last instruction is read on the exit path using load
> > > external pid (lwepx) dedicated instruction. This load operation may
> > fail
> > > due to TLB eviction and execute-but-not-read entries.
> > >
> > > This patch lay down the path for an alternative solution to read the
> > guest
> > > last instruction, by allowing kvmppc_get_lat_inst() function to fail.
> > > Architecture specific implmentations of kvmppc_load_last_inst() may
> > read
> > > last guest instruction and instruct the emulation layer to re-execute
> > the
> > > guest in case of failure.
> > >
> > > Make kvmppc_get_last_inst() definition common between architectures.
> > >
> > > Signed-off-by: Mihai Caraman
> > > ---
>
> ...
>
> > > diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> > b/arch/powerpc/include/asm/kvm_ppc.h
> > > index e2fd5a1..7f9c634 100644
> > > --- a/arch/powerpc/include/asm/kvm_ppc.h
> > > +++ b/arch/powerpc/include/asm/kvm_ppc.h
> > > @@ -47,6 +47,11 @@ enum emulation_result {
> > > EMULATE_EXIT_USER,/* emulation requires exit to user-
> space */
> > > };
> > >
> > > +enum instruction_type {
> > > + INST_GENERIC,
> > > + INST_SC,/* system call */
> > > +};
> > > +
> > > extern int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu
> > *vcpu);
> > > extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct
> kvm_vcpu
> > *vcpu);
> > > extern void kvmppc_handler_highmem(void);
> > > @@ -62,6 +67,9 @@ extern int kvmppc_handle_store(struct kvm_run *run,
> > struct kvm_vcpu *vcpu,
> > > u64 val, unsigned int bytes,
> > > int is_default_endian);
> > >
> > > +extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
> > > + enum instruction_type type, u32 *inst);
> > > +
> > > extern int kvmppc_emulate_instruction(struct kvm_run *run,
> > > struct kvm_vcpu *vcpu);
> > > extern int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu
> > *vcpu);
> > > @@ -234,6 +242,23 @@ struct kvmppc_ops {
> > > extern struct kvmppc_ops *kvmppc_hv_ops;
> > > extern struct kvmppc_ops *kvmppc_pr_ops;
> > >
> > > +static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
> > > + enum instruction_type type, u32 *inst)
> > > +{
> > > + int ret = EMULATE_DONE;
> > > +
> > > + /* Load the instruction manually if it failed to do so in the
> > > + * exit path */
> > > + if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
> > > + ret = kvmppc_load_last_inst(vcpu, type, &vcpu-
> > >arch.last_inst);
> > > +
> > > +
> > > + *inst = (ret == EMULATE_DONE && kvmppc_need_byteswap(vcpu)) ?
> > > + swab32(vcpu->arch.last_inst) : vcpu->arch.last_inst;
> >
> > This makes even less sense than the previous version. Either you treat
> > inst as "definitely overwritten" or as "preserves previous data on
> > failure".
>
> Both v4 and v5 versions treat inst as "definitely overwritten".
>
> >
> > So either you unconditionally swap like you did before
>
> If we make abstraction of its symmetry, KVM_INST_FETCH_FAILED is operated
> in host endianness, so it doesn't need byte swap.
>
> I agree with your reasoning if last_inst is initialized and compared with
> data in guest endianess, which is not the case yet for
> KVM_INST_FETCH_FAILED.
Alex, are you relying on the fact that KVM_INST_FETCH_FAILED value is
symmetrical?
With a non symmetrical value like 0xDEADBEEF, and considering a little-endian
guest
on a big-endian host, we need to fix kvm logic to initialize and compare
last_inst
with 0xEFBEADDE swaped value.
Your suggestion to unconditionally swap makes sense only with the above fix,
otherwise
inst may end up with 0xEFBEADDE swaped value with is wrong.
-Mike
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Thursday, July 03, 2014 3:21 PM
> To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org
> Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for
> SPE/FP/AltiVec int numbers
>
>
> On 30.06.14 17:34, Mihai Caraman wrote:
> > Use common BOOKE_IRQPRIO and BOOKE_INTERRUPT defines for SPE/FP/AltiVec
> > which share the same interrupt numbers.
> >
> > Signed-off-by: Mihai Caraman
> > ---
> > v2:
> > - remove outdated definitions
> >
> > arch/powerpc/include/asm/kvm_asm.h| 8
> > arch/powerpc/kvm/booke.c | 17 +
> > arch/powerpc/kvm/booke.h | 4 ++--
> > arch/powerpc/kvm/booke_interrupts.S | 9 +
> > arch/powerpc/kvm/bookehv_interrupts.S | 4 ++--
> > arch/powerpc/kvm/e500.c | 10 ++
> > arch/powerpc/kvm/e500_emulate.c | 10 ++
> > 7 files changed, 30 insertions(+), 32 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_asm.h
> b/arch/powerpc/include/asm/kvm_asm.h
> > index 9601741..c94fd33 100644
> > --- a/arch/powerpc/include/asm/kvm_asm.h
> > +++ b/arch/powerpc/include/asm/kvm_asm.h
> > @@ -56,14 +56,6 @@
> > /* E500 */
> > #define BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL 32
> > #define BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST 33
> > -/*
> > - * 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
>
> I think I'd prefer to keep them separate.
>
> > #define BOOKE_INTERRUPT_SPE_FP_ROUND 34
> > #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35
> > #define BOOKE_INTERRUPT_DOORBELL 36
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index ab62109..3c86d9b 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct
> kvm_vcpu *vcpu,
> > case BOOKE_IRQPRIO_ITLB_MISS:
> > case BOOKE_IRQPRIO_SYSCALL:
> > case BOOKE_IRQPRIO_FP_UNAVAIL:
> > - case BOOKE_IRQPRIO_SPE_UNAVAIL:
> > - case BOOKE_IRQPRIO_SPE_FP_DATA:
> > + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL:
> > + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST:
>
> #ifdef CONFIG_KVM_E500V2
>case ...SPE:
> #else
>case ..ALTIVEC:
> #endif
>
> > case BOOKE_IRQPRIO_SPE_FP_ROUND:
> > case BOOKE_IRQPRIO_AP_UNAVAIL:
> > allowed = 1;
> > @@ -977,18 +977,19 @@ int kvmppc_handle_exit(struct kvm_run *run,
> struct kvm_vcpu *vcpu,
> > break;
> >
> > #ifdef CONFIG_SPE
> > - case BOOKE_INTERRUPT_SPE_UNAVAIL: {
> > + 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_UNAVAIL);
> > + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL);
> > r = RESUME_GUEST;
> > break;
> > }
> >
> > - case BOOKE_INTERRUPT_SPE_FP_DATA:
> > - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_DATA);
> > + case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
> > + kvmppc_booke_queue_irqprio(vcpu,
> > + BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST);
> > r = RESUME_GUEST;
> > break;
> >
> > @@ -997,7 +998,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
> > r = RESUME_GUEST;
> > break;
> > #else
> > - case BOOKE_INTERRUPT_SPE_UNAVAIL:
> > + case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL:
> > /*
> > * Guest wants SPE, but host kernel doesn't support it. Send
> > * an "unimplemented operation" program check to the guest.
> > @@ -1010,7 +1011,7 @@ int kvmppc_handle_exit(struct kvm_run *run,
> struct kvm_vcpu *vcpu,
> > * These really should never happen without CONFIG_SPE,
> > * as we should never enable the real MSR[SPE] in the guest.
> > */
> > - case BOOKE_INTERRUPT_SPE_FP_DATA:
> > + case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
> > case BOOKE_INTERRUPT_SPE_FP_ROUND:
> > printk(KERN_CRIT "%s: unexpected SPE interrupt %u at
> %08lx\n",
> >__func__, exit_nr, vcpu->arch.pc);
> > diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h
> > index b632cd3..f182b32 100644
> > --- a/arch/powerpc/kvm/booke.h
> > +++ b/arch/powerpc/kvm/booke.h
> > @@ -32,8 +32,8 @@
>
RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-
> ow...@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Wednesday, July 23, 2014 12:21 AM
> To: Caraman Mihai Claudiu-B02008
> Cc: kvm-...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> k...@vger.kernel.org
> Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
>
>
> On 21.07.14 11:59, mihai.cara...@freescale.com wrote:
> >> -Original Message-
> >> From: Linuxppc-dev [mailto:linuxppc-dev-
> >> bounces+mihai.caraman=freescale@lists.ozlabs.org] On Behalf Of
> >> mihai.cara...@freescale.com
> >> Sent: Friday, July 18, 2014 12:06 PM
> >> To: Alexander Graf; kvm-...@vger.kernel.org
> >> Cc: linuxppc-dev@lists.ozlabs.org; k...@vger.kernel.org
> >> Subject: RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
> fail
> >>
> >>> -Original Message-
> >>> From: Alexander Graf [mailto:ag...@suse.de]
> >>> Sent: Thursday, July 17, 2014 5:21 PM
> >>> To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org
> >>> Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> >>> Subject: Re: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to
> >> fail
> >>>
> >>> On 17.07.14 13:22, Mihai Caraman wrote:
> >>>> On book3e, guest last instruction is read on the exit path using
> load
> >>>> external pid (lwepx) dedicated instruction. This load operation may
> >>> fail
> >>>> due to TLB eviction and execute-but-not-read entries.
> >>>>
> >>>> This patch lay down the path for an alternative solution to read the
> >>> guest
> >>>> last instruction, by allowing kvmppc_get_lat_inst() function to
> fail.
> >>>> Architecture specific implmentations of kvmppc_load_last_inst() may
> >>> read
> >>>> last guest instruction and instruct the emulation layer to re-
> execute
> >>> the
> >>>> guest in case of failure.
> >>>>
> >>>> Make kvmppc_get_last_inst() definition common between architectures.
> >>>>
> >>>> Signed-off-by: Mihai Caraman
> >>>> ---
> >> ...
> >>
> >>>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> >>> b/arch/powerpc/include/asm/kvm_ppc.h
> >>>> index e2fd5a1..7f9c634 100644
> >>>> --- a/arch/powerpc/include/asm/kvm_ppc.h
> >>>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> >>>> @@ -47,6 +47,11 @@ enum emulation_result {
> >>>> EMULATE_EXIT_USER,/* emulation requires exit to user-
> >> space */
> >>>>};
> >>>>
> >>>> +enum instruction_type {
> >>>> +INST_GENERIC,
> >>>> +INST_SC,/* system call */
> >>>> +};
> >>>> +
> >>>>extern int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct
> kvm_vcpu
> >>> *vcpu);
> >>>>extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct
> >> kvm_vcpu
> >>> *vcpu);
> >>>>extern void kvmppc_handler_highmem(void);
> >>>> @@ -62,6 +67,9 @@ extern int kvmppc_handle_store(struct kvm_run
> *run,
> >>> struct kvm_vcpu *vcpu,
> >>>> u64 val, unsigned int bytes,
> >>>> int is_default_endian);
> >>>>
> >>>> +extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
> >>>> + enum instruction_type type, u32 *inst);
> >>>> +
> >>>>extern int kvmppc_emulate_instruction(struct kvm_run *run,
> >>>> struct kvm_vcpu *vcpu);
> >>>>extern int kvmppc_emulate_mmio(struct kvm_run *run, struct
> kvm_vcpu
> >>> *vcpu);
> >>>> @@ -234,6 +242,23 @@ struct kvmppc_ops {
> >>>>extern struct kvmppc_ops *kvmppc_hv_ops;
> >>>>extern struct kvmppc_ops *kvmppc_pr_ops;
> >>>>
> >>>> +static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
> >>>> +enum instruction_type type, u32
> *inst)
> >>>> +{
> >>>> +int ret = EMULATE_DONE;
> >>>> +
> >>>> +/* Load the instruction manually if it failed t
RE: [PATCH v5 4/5] KVM: PPC: Alow kvmppc_get_last_inst() to fail
> > Right. With this do you acknowledge that v5 (definitely overwritten > approach) > > is ok? > > I think I'm starting to understand your logic of v5. You write > fetch_failed into *inst unswapped if the fetch failed. "v5 - don't swap when load fails" :) > > I think that's ok, but I definitely do not like the code flow - it's too > hard to understand at a glimpse. Just rewrite it to swab at local > variable level, preferably with if()s and comments what this is about and > have a single unconditional *inst = fetched_inst; at the end of the > function. I will incorporate these change requests into v6. Thanks, -Mike ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-
> ow...@vger.kernel.org] On Behalf Of mihai.cara...@freescale.com
> Sent: Monday, July 21, 2014 4:23 PM
> To: Alexander Graf; Wood Scott-B07421
> Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org
> Subject: RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for
> SPE/FP/AltiVec int numbers
>
> > -Original Message-
> > From: Alexander Graf [mailto:ag...@suse.de]
> > Sent: Thursday, July 03, 2014 3:21 PM
> > To: Caraman Mihai Claudiu-B02008; kvm-...@vger.kernel.org
> > Cc: k...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for
> > SPE/FP/AltiVec int numbers
> >
> >
> > On 30.06.14 17:34, Mihai Caraman wrote:
> > > Use common BOOKE_IRQPRIO and BOOKE_INTERRUPT defines for
> SPE/FP/AltiVec
> > > which share the same interrupt numbers.
> > >
> > > Signed-off-by: Mihai Caraman
> > > ---
> > > v2:
> > > - remove outdated definitions
> > >
> > > arch/powerpc/include/asm/kvm_asm.h| 8
> > > arch/powerpc/kvm/booke.c | 17 +
> > > arch/powerpc/kvm/booke.h | 4 ++--
> > > arch/powerpc/kvm/booke_interrupts.S | 9 +
> > > arch/powerpc/kvm/bookehv_interrupts.S | 4 ++--
> > > arch/powerpc/kvm/e500.c | 10 ++
> > > arch/powerpc/kvm/e500_emulate.c | 10 ++
> > > 7 files changed, 30 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/arch/powerpc/include/asm/kvm_asm.h
> > b/arch/powerpc/include/asm/kvm_asm.h
> > > index 9601741..c94fd33 100644
> > > --- a/arch/powerpc/include/asm/kvm_asm.h
> > > +++ b/arch/powerpc/include/asm/kvm_asm.h
> > > @@ -56,14 +56,6 @@
> > > /* E500 */
> > > #define BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL 32
> > > #define BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST 33
> > > -/*
> > > - * 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
> >
> > I think I'd prefer to keep them separate.
> >
> > > #define BOOKE_INTERRUPT_SPE_FP_ROUND 34
> > > #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35
> > > #define BOOKE_INTERRUPT_DOORBELL 36
> > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > > index ab62109..3c86d9b 100644
> > > --- a/arch/powerpc/kvm/booke.c
> > > +++ b/arch/powerpc/kvm/booke.c
> > > @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct
> > kvm_vcpu *vcpu,
> > > case BOOKE_IRQPRIO_ITLB_MISS:
> > > case BOOKE_IRQPRIO_SYSCALL:
> > > case BOOKE_IRQPRIO_FP_UNAVAIL:
> > > - case BOOKE_IRQPRIO_SPE_UNAVAIL:
> > > - case BOOKE_IRQPRIO_SPE_FP_DATA:
> > > + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL:
> > > + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST:
> >
> > #ifdef CONFIG_KVM_E500V2
> >case ...SPE:
> > #else
> >case ..ALTIVEC:
> > #endif
> >
> > > case BOOKE_IRQPRIO_SPE_FP_ROUND:
> > > case BOOKE_IRQPRIO_AP_UNAVAIL:
> > > allowed = 1;
> > > @@ -977,18 +977,19 @@ int kvmppc_handle_exit(struct kvm_run *run,
> > struct kvm_vcpu *vcpu,
> > > break;
> > >
> > > #ifdef CONFIG_SPE
> > > - case BOOKE_INTERRUPT_SPE_UNAVAIL: {
> > > + 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_UNAVAIL);
> > > + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL);
> > > r = RESUME_GUEST;
> > > break;
> > > }
> > >
> >
RE: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
> -Original Message-
> From: Wood Scott-B07421
> Sent: Saturday, July 26, 2014 3:11 AM
> To: Caraman Mihai Claudiu-B02008
> Cc: Alexander Graf; kvm-...@vger.kernel.org; k...@vger.kernel.org;
> linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for
> SPE/FP/AltiVec int numbers
>
> On Thu, 2014-07-24 at 04:16 -0500, Caraman Mihai Claudiu-B02008 wrote:
> > Scott, Alex's request to define SPE handlers only for e500v2 implies
> changes
> > in 32-bit FSL kernel to have exclusive configurations for e200/e500v2
> and
> > e500mc/e5500. We would probably need something like this, what's your
> take on it?
>
> That is already a compile-time decision.
Yes, but is not fully implemented.
>
> > diff --git a/arch/powerpc/kernel/head_fsl_booke.S
> b/arch/powerpc/kernel/head_fsl_booke.S
> > index b497188..9d41015 100644
> > --- a/arch/powerpc/kernel/head_fsl_booke.S
> > +++ b/arch/powerpc/kernel/head_fsl_booke.S
> > @@ -613,6 +613,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> > mfspr r10, SPRN_SPRG_RSCRATCH0
> > b InstructionStorage
> >
> > +/* Define SPE handlers only for e500v2 and e200 */
> > +#ifndef CONFIG_PPC_E500MC
> > #ifdef CONFIG_SPE
> > /* SPE Unavailable */
> > START_EXCEPTION(SPEUnavailable)
> > @@ -626,7 +628,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
> > EXCEPTION(0x2020, SPE_ALTIVEC_UNAVAIL, SPEUnavailable, \
> > unknown_exception, EXC_XFER_EE)
> > #endif /* CONFIG_SPE */
> > +#endif
>
> This is redundant:
>
> config SPE
> bool "SPE Support"
> depends on E200 || (E500 && !PPC_E500MC)
> default y
I see what you mean, but it's not redundant. Alex was looking to remove SPE
handlers for e500mc+ and the proposal handled !SPE case. In the new
light I find this variant more readable:
+/* Define SPE handlers for e200 and e500v2 */
#ifdef CONFIG_SPE
/* SPE Unavailable */
START_EXCEPTION(SPEUnavailable)
@@ -622,11 +623,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
b fast_exception_return
1: addir3,r1,STACK_FRAME_OVERHEAD
EXC_XFER_EE_LITE(0x2010, KernelSPE)
-#else
+#elif defined(CONFIG_E200) || \
+ (defined(CONFIG_E500) && !defined(CONFIG_PPC_E500MC))
EXCEPTION(0x2020, SPE_ALTIVEC_UNAVAIL, SPEUnavailable, \
unknown_exception, EXC_XFER_EE)
#endif /* CONFIG_SPE */
>
> > diff --git a/arch/powerpc/kernel/cputable.c
> b/arch/powerpc/kernel/cputable.c
> > index c1faade..3ab65c2 100644
> > --- a/arch/powerpc/kernel/cputable.c
> > +++ b/arch/powerpc/kernel/cputable.c
> > @@ -2030,6 +2030,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
> > #endif /* CONFIG_PPC32 */
> > #ifdef CONFIG_E500
> > #ifdef CONFIG_PPC32
> > +#ifndef CONFIG_PPC_E500MC
> > { /* e500 */
> > .pvr_mask = 0x,
> > .pvr_value = 0x8020,
> > @@ -2069,6 +2070,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
> > .machine_check = machine_check_e500,
> > .platform = "ppc8548",
> > },
> > +#endif /* !CONFIG_PPC_E500MC */
> > { /* e500mc */
> > .pvr_mask = 0x,
> > .pvr_value = 0x8023,
> >
>
> This looks a bit strange -- e500v2 gets excluded if PPC_E500MC, but
> e500mc gets included in !PPC_E500MC?
Right, my main purpose was to get rid of __setup_e500_ivors on PPC_E500MC
which refers SPEUnavailable. I will add an #else to exclude e500mc.
The "generic E500 PPC" default cpu advertises PPC_FEATURE_HAS_SPE_COMP.
Do we want to excluded it if PPC_E500MC? Is this cpu useful without cpu_setup()
and if so why don't we also have a default for 64-bit?
-Mike
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] KVM: PPC: Add devname:kvm aliases for modules
> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-
> ow...@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Monday, December 09, 2013 5:02 PM
> To: "; " "@suse.de
> Cc: k...@vger.kernel.org mailing list
> Subject: [PATCH] KVM: PPC: Add devname:kvm aliases for modules
>
> Systems that support automatic loading of kernel modules through
> device aliases should try and automatically load kvm when /dev/kvm
> gets opened.
>
> Add code to support that magic for all PPC kvm targets, even the
> ones that don't support modules yet.
>
> Signed-off-by: Alexander Graf
...
> --- a/arch/powerpc/kvm/e500mc.c
> +++ b/arch/powerpc/kvm/e500mc.c
> @@ -391,3 +391,6 @@ static void __exit kvmppc_e500mc_exit(void)
>
> module_init(kvmppc_e500mc_init);
> module_exit(kvmppc_e500mc_exit);
> +#include
> +MODULE_ALIAS_MISCDEV(KVM_MINOR);
> +MODULE_ALIAS("devname:kvm");
> --
This patch breaks the build on KMV Book3E, you need to include
too.
-Mike
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 05/10] powerpc/booke64: Use SPRG7 for VDSO
> -Original Message-
> From: Wood Scott-B07421
> Sent: Friday, March 14, 2014 2:01 AM
> To: Benjamin Herrenschmidt
> Cc: linuxppc-dev@lists.ozlabs.org; Kumar Gala; Tiejun Chen; Wood Scott-
> B07421; Caraman Mihai Claudiu-B02008; Anton Blanchard; Paul Mackerras;
> kvm-...@vger.kernel.org
> Subject: [PATCH 05/10] powerpc/booke64: Use SPRG7 for VDSO
>
> Previously SPRG3 was marked for use by both VDSO and critical
> interrupts (though critical interrupts were not fully implemented).
>
> In commit 8b64a9dfb091f1eca8b7e58da82f1e7d1d5fe0ad ("powerpc/booke64:
> Use SPRG0/3 scratch for bolted TLB miss & crit int"), Mihai Caraman
> made an attempt to resolve this conflict by restoring the VDSO value
> early in the critical interrupt, but this has some issues:
>
> - It forces critical exceptions to be a special case handled
>differently from even machine check and debug level exceptions.
Yes, this was ugly.
> Since we cannot use SPRG4-7 for scratch without corrupting the state of
> a KVM guest, move VDSO to SPRG7 on book3e.
At that time I thought that information exposed through SPRN_USPRG3 is
part of the ABI which can't be changed, and this lead to complicated
gymnastic. But since VSDO's __kernel_getcpu() function hides SPRN_USPRGx
access, I think your unified approach is the right way.
-Mike
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
