| Message ID | 1329498837-11717-17-git-send-email-agraf@suse.de |
|---|---|
| State | New, archived |
| Headers | show |
On 02/17/2012 11:13 AM, Alexander Graf wrote: > When one vcpu wants to kick another, it can issue a special IPI instruction > called msgsnd. This patch emulates this instruction, its clearing counterpart > and the infrastructure required to actually trigger that interrupt inside > a guest vcpu. > > With this patch, SMP guests on e500mc work. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > arch/powerpc/kvm/booke.c | 6 +++ > arch/powerpc/kvm/e500_emulate.c | 68 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 74 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index 3dd200d..ce1599d 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -326,6 +326,9 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, > int_class = INT_CLASS_NONCRIT; > break; > case BOOKE_IRQPRIO_CRITICAL: > +#ifdef CONFIG_KVM_E500MC > + case BOOKE_IRQPRIO_DBELL_CRIT: > +#endif > allowed = vcpu->arch.shared->msr & MSR_CE; > allowed = allowed && !crit; > msr_mask = MSR_GS | MSR_ME; > @@ -342,6 +345,9 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, > keep_irq = true; > /* fall through */ > case BOOKE_IRQPRIO_EXTERNAL: > +#ifdef CONFIG_KVM_E500MC > + case BOOKE_IRQPRIO_DBELL: > +#endif This isn't e500mc specific -- it's in the ISA as "Embedded.Processor Control". Any harm in just removing the ifdef (similar to tlbilx)? > allowed = vcpu->arch.shared->msr & MSR_EE; > allowed = allowed && !crit; > msr_mask = MSR_GS | MSR_CE | MSR_ME | MSR_DE; > diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c > index 98b6c1c..29d5604 100644 > --- a/arch/powerpc/kvm/e500_emulate.c > +++ b/arch/powerpc/kvm/e500_emulate.c > @@ -14,16 +14,74 @@ > > #include <asm/kvm_ppc.h> > #include <asm/disassemble.h> > +#include <asm/dbell.h> > > #include "booke.h" > #include "e500.h" > > +#define XOP_MSGSND 206 > +#define XOP_MSGCLR 238 > #define XOP_TLBIVAX 786 > #define XOP_TLBSX 914 > #define XOP_TLBRE 946 > #define XOP_TLBWE 978 > #define XOP_TLBILX 18 > > +#ifdef CONFIG_KVM_E500MC > +static int dbell2prio(ulong param) > +{ > + int msg = param & PPC_DBELL_TYPE(-1); Maybe introduce PPC_DBELL_TYPE_MASK or GET_PPC_DBELL_TYPE? > + int prio = -1; > + > + switch (msg) { > + case PPC_DBELL_TYPE(PPC_DBELL): > + prio = BOOKE_IRQPRIO_DBELL; > + break; > + case PPC_DBELL_TYPE(PPC_DBELL_CRIT): > + prio = BOOKE_IRQPRIO_DBELL_CRIT; > + break; > + default: > + break; > + } > + > + return prio; > +} > + > +static int kvmppc_e500_emul_msgclr(struct kvm_vcpu *vcpu, int rb) > +{ > + ulong param = vcpu->arch.gpr[rb]; > + int prio = dbell2prio(param); > + > + if (prio < 0) > + return EMULATE_FAIL; > + > + clear_bit(prio, &vcpu->arch.pending_exceptions); > + return EMULATE_DONE; > +} > + > +static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu, int rb) > +{ > + ulong param = vcpu->arch.gpr[rb]; > + int prio = dbell2prio(rb); > + int pir = param & 0x3fff; Introduce PPC_DBELL_PIR_MASK or GET_PPC_DBELL_PIR? > + int i; > + struct kvm_vcpu *cvcpu; > + > + if (prio < 0) > + return EMULATE_FAIL; > + > + kvm_for_each_vcpu(i, cvcpu, vcpu->kvm) { > + int cpir = cvcpu->arch.shared->pir; > + if ((param & PPC_DBELL_MSG_BRDCAST) || (cpir == pir)) { > + set_bit(prio, &cvcpu->arch.pending_exceptions); > + kvm_vcpu_kick(cvcpu); > + } > + } Should this be a kvm_make_request instead (with a separate pending_doorbell bool in vcpu that msgclr can act on), considering earlier discussion of phasing out atomics on pending_exceptions, in favor of requests? -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/17/2012 03:55 PM, Scott Wood wrote: > Should this be a kvm_make_request instead (with a separate > pending_doorbell bool in vcpu that msgclr can act on), considering > earlier discussion of phasing out atomics on pending_exceptions, in > favor of requests? Ignore the bit about msgclr -- it acts on the local vcpu, so no need for pending_doorbell. It can just be a plain request. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17.02.2012, at 22:55, Scott Wood wrote: > On 02/17/2012 11:13 AM, Alexander Graf wrote: >> When one vcpu wants to kick another, it can issue a special IPI instruction >> called msgsnd. This patch emulates this instruction, its clearing counterpart >> and the infrastructure required to actually trigger that interrupt inside >> a guest vcpu. >> >> With this patch, SMP guests on e500mc work. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> arch/powerpc/kvm/booke.c | 6 +++ >> arch/powerpc/kvm/e500_emulate.c | 68 +++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 74 insertions(+), 0 deletions(-) >> >> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c >> index 3dd200d..ce1599d 100644 >> --- a/arch/powerpc/kvm/booke.c >> +++ b/arch/powerpc/kvm/booke.c >> @@ -326,6 +326,9 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, >> int_class = INT_CLASS_NONCRIT; >> break; >> case BOOKE_IRQPRIO_CRITICAL: >> +#ifdef CONFIG_KVM_E500MC >> + case BOOKE_IRQPRIO_DBELL_CRIT: >> +#endif >> allowed = vcpu->arch.shared->msr & MSR_CE; >> allowed = allowed && !crit; >> msr_mask = MSR_GS | MSR_ME; >> @@ -342,6 +345,9 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, >> keep_irq = true; >> /* fall through */ >> case BOOKE_IRQPRIO_EXTERNAL: >> +#ifdef CONFIG_KVM_E500MC >> + case BOOKE_IRQPRIO_DBELL: >> +#endif > > This isn't e500mc specific -- it's in the ISA as "Embedded.Processor > Control". > > Any harm in just removing the ifdef (similar to tlbilx)? Well, to me this is more of an indication "this should become a runtime check one day" in case we want to combine the two targets. On e500v2, we don't know what a doorbell interrupt is, so we really shouldn't be delivering one either. > >> allowed = vcpu->arch.shared->msr & MSR_EE; >> allowed = allowed && !crit; >> msr_mask = MSR_GS | MSR_CE | MSR_ME | MSR_DE; >> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c >> index 98b6c1c..29d5604 100644 >> --- a/arch/powerpc/kvm/e500_emulate.c >> +++ b/arch/powerpc/kvm/e500_emulate.c >> @@ -14,16 +14,74 @@ >> >> #include <asm/kvm_ppc.h> >> #include <asm/disassemble.h> >> +#include <asm/dbell.h> >> >> #include "booke.h" >> #include "e500.h" >> >> +#define XOP_MSGSND 206 >> +#define XOP_MSGCLR 238 >> #define XOP_TLBIVAX 786 >> #define XOP_TLBSX 914 >> #define XOP_TLBRE 946 >> #define XOP_TLBWE 978 >> #define XOP_TLBILX 18 >> >> +#ifdef CONFIG_KVM_E500MC >> +static int dbell2prio(ulong param) >> +{ >> + int msg = param & PPC_DBELL_TYPE(-1); > > Maybe introduce PPC_DBELL_TYPE_MASK or GET_PPC_DBELL_TYPE? TYPE_MASK I'd say. > >> + int prio = -1; >> + >> + switch (msg) { >> + case PPC_DBELL_TYPE(PPC_DBELL): >> + prio = BOOKE_IRQPRIO_DBELL; >> + break; >> + case PPC_DBELL_TYPE(PPC_DBELL_CRIT): >> + prio = BOOKE_IRQPRIO_DBELL_CRIT; >> + break; >> + default: >> + break; >> + } >> + >> + return prio; >> +} >> + >> +static int kvmppc_e500_emul_msgclr(struct kvm_vcpu *vcpu, int rb) >> +{ >> + ulong param = vcpu->arch.gpr[rb]; >> + int prio = dbell2prio(param); >> + >> + if (prio < 0) >> + return EMULATE_FAIL; >> + >> + clear_bit(prio, &vcpu->arch.pending_exceptions); >> + return EMULATE_DONE; >> +} >> + >> +static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu, int rb) >> +{ >> + ulong param = vcpu->arch.gpr[rb]; >> + int prio = dbell2prio(rb); >> + int pir = param & 0x3fff; > > Introduce PPC_DBELL_PIR_MASK or GET_PPC_DBELL_PIR? Good point :) > >> + int i; >> + struct kvm_vcpu *cvcpu; >> + >> + if (prio < 0) >> + return EMULATE_FAIL; >> + >> + kvm_for_each_vcpu(i, cvcpu, vcpu->kvm) { >> + int cpir = cvcpu->arch.shared->pir; >> + if ((param & PPC_DBELL_MSG_BRDCAST) || (cpir == pir)) { >> + set_bit(prio, &cvcpu->arch.pending_exceptions); >> + kvm_vcpu_kick(cvcpu); >> + } >> + } > > Should this be a kvm_make_request instead (with a separate > pending_doorbell bool in vcpu that msgclr can act on), considering > earlier discussion of phasing out atomics on pending_exceptions, in > favor of requests? Yeah, I was thinking about that too, but right now we already have some direct use of pending_exceptions in different places around the code. So today, that is our public interface. I'd rather go in and clean up the whole thing to make pending_exceptions private in a separate cleanup round, rather than having it part of e500mc support. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 20, 2012 at 12:49:46PM +0100, Alexander Graf wrote: > > On 17.02.2012, at 22:55, Scott Wood wrote: > > > On 02/17/2012 11:13 AM, Alexander Graf wrote: > >> case BOOKE_IRQPRIO_EXTERNAL: > >> +#ifdef CONFIG_KVM_E500MC > >> + case BOOKE_IRQPRIO_DBELL: > >> +#endif > > > > This isn't e500mc specific -- it's in the ISA as "Embedded.Processor > > Control". > > > > Any harm in just removing the ifdef (similar to tlbilx)? > > Well, to me this is more of an indication "this should become a runtime > check one day" in case we want to combine the two targets. On e500v2, > we don't know what a doorbell interrupt is, so we really shouldn't be > delivering one either. Should at least have a comment saying that eventually the decision should be based on ISA category support rather than e500mc. > > Should this be a kvm_make_request instead (with a separate > > pending_doorbell bool in vcpu that msgclr can act on), considering > > earlier discussion of phasing out atomics on pending_exceptions, in > > favor of requests? > > Yeah, I was thinking about that too, but right now we already have some > direct use of pending_exceptions in different places around the code. > So today, that is our public interface. > > I'd rather go in and clean up the whole thing to make > pending_exceptions private in a separate cleanup round, rather than > having it part of e500mc support. We already use requests for timers, and it seems simple enough to add one for doorbells now rather than come back and clean it up later (it's not tied to what we'd have to do for external IRQs), but if you'd rather do it later that's fine with me. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20.02.2012, at 16:39, Scott Wood wrote: > On Mon, Feb 20, 2012 at 12:49:46PM +0100, Alexander Graf wrote: >> >> On 17.02.2012, at 22:55, Scott Wood wrote: >> >>> On 02/17/2012 11:13 AM, Alexander Graf wrote: >>>> case BOOKE_IRQPRIO_EXTERNAL: >>>> +#ifdef CONFIG_KVM_E500MC >>>> + case BOOKE_IRQPRIO_DBELL: >>>> +#endif >>> >>> This isn't e500mc specific -- it's in the ISA as "Embedded.Processor >>> Control". >>> >>> Any harm in just removing the ifdef (similar to tlbilx)? >> >> Well, to me this is more of an indication "this should become a runtime >> check one day" in case we want to combine the two targets. On e500v2, >> we don't know what a doorbell interrupt is, so we really shouldn't be >> delivering one either. > > Should at least have a comment saying that eventually the decision should > be based on ISA category support rather than e500mc. Hrm. How about a new #define that is a bit more verbose? #ifdef CONFIG_KVM_E500MC #define KVM_CAT_DOORBELL #endif > >>> Should this be a kvm_make_request instead (with a separate >>> pending_doorbell bool in vcpu that msgclr can act on), considering >>> earlier discussion of phasing out atomics on pending_exceptions, in >>> favor of requests? >> >> Yeah, I was thinking about that too, but right now we already have some >> direct use of pending_exceptions in different places around the code. >> So today, that is our public interface. >> >> I'd rather go in and clean up the whole thing to make >> pending_exceptions private in a separate cleanup round, rather than >> having it part of e500mc support. > > We already use requests for timers, and it seems simple enough to add one > for doorbells now rather than come back and clean it up later (it's not > tied to what we'd have to do for external IRQs), but if you'd rather do > it later that's fine with me. Yeah, because I want the external interface to include the kick, which we don't model today. So instead of touching basically every line of code again, I'd rather do it in one roll. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 3dd200d..ce1599d 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -326,6 +326,9 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, int_class = INT_CLASS_NONCRIT; break; case BOOKE_IRQPRIO_CRITICAL: +#ifdef CONFIG_KVM_E500MC + case BOOKE_IRQPRIO_DBELL_CRIT: +#endif allowed = vcpu->arch.shared->msr & MSR_CE; allowed = allowed && !crit; msr_mask = MSR_GS | MSR_ME; @@ -342,6 +345,9 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, keep_irq = true; /* fall through */ case BOOKE_IRQPRIO_EXTERNAL: +#ifdef CONFIG_KVM_E500MC + case BOOKE_IRQPRIO_DBELL: +#endif allowed = vcpu->arch.shared->msr & MSR_EE; allowed = allowed && !crit; msr_mask = MSR_GS | MSR_CE | MSR_ME | MSR_DE; diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c index 98b6c1c..29d5604 100644 --- a/arch/powerpc/kvm/e500_emulate.c +++ b/arch/powerpc/kvm/e500_emulate.c @@ -14,16 +14,74 @@ #include <asm/kvm_ppc.h> #include <asm/disassemble.h> +#include <asm/dbell.h> #include "booke.h" #include "e500.h" +#define XOP_MSGSND 206 +#define XOP_MSGCLR 238 #define XOP_TLBIVAX 786 #define XOP_TLBSX 914 #define XOP_TLBRE 946 #define XOP_TLBWE 978 #define XOP_TLBILX 18 +#ifdef CONFIG_KVM_E500MC +static int dbell2prio(ulong param) +{ + int msg = param & PPC_DBELL_TYPE(-1); + int prio = -1; + + switch (msg) { + case PPC_DBELL_TYPE(PPC_DBELL): + prio = BOOKE_IRQPRIO_DBELL; + break; + case PPC_DBELL_TYPE(PPC_DBELL_CRIT): + prio = BOOKE_IRQPRIO_DBELL_CRIT; + break; + default: + break; + } + + return prio; +} + +static int kvmppc_e500_emul_msgclr(struct kvm_vcpu *vcpu, int rb) +{ + ulong param = vcpu->arch.gpr[rb]; + int prio = dbell2prio(param); + + if (prio < 0) + return EMULATE_FAIL; + + clear_bit(prio, &vcpu->arch.pending_exceptions); + return EMULATE_DONE; +} + +static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu, int rb) +{ + ulong param = vcpu->arch.gpr[rb]; + int prio = dbell2prio(rb); + int pir = param & 0x3fff; + int i; + struct kvm_vcpu *cvcpu; + + if (prio < 0) + return EMULATE_FAIL; + + kvm_for_each_vcpu(i, cvcpu, vcpu->kvm) { + int cpir = cvcpu->arch.shared->pir; + if ((param & PPC_DBELL_MSG_BRDCAST) || (cpir == pir)) { + set_bit(prio, &cvcpu->arch.pending_exceptions); + kvm_vcpu_kick(cvcpu); + } + } + + return EMULATE_DONE; +} +#endif + int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, unsigned int inst, int *advance) { @@ -36,6 +94,16 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, case 31: switch (get_xop(inst)) { +#ifdef CONFIG_KVM_E500MC + case XOP_MSGSND: + emulated = kvmppc_e500_emul_msgsnd(vcpu, get_rb(inst)); + break; + + case XOP_MSGCLR: + emulated = kvmppc_e500_emul_msgclr(vcpu, get_rb(inst)); + break; +#endif + case XOP_TLBRE: emulated = kvmppc_e500_emul_tlbre(vcpu); break;
When one vcpu wants to kick another, it can issue a special IPI instruction called msgsnd. This patch emulates this instruction, its clearing counterpart and the infrastructure required to actually trigger that interrupt inside a guest vcpu. With this patch, SMP guests on e500mc work. Signed-off-by: Alexander Graf <agraf@suse.de> --- arch/powerpc/kvm/booke.c | 6 +++ arch/powerpc/kvm/e500_emulate.c | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 0 deletions(-)