On 31.01.2014, at 12:40, Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com> wrote:
> Alexander Graf <ag...@suse.de> writes: > >> On 01/28/2014 05:44 PM, Aneesh Kumar K.V wrote: >>> At this point we allow all the supported facilities except EBB. So >>> forward the interrupt to guest as illegal instruction. >>> >>> Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com> >>> --- >>> arch/powerpc/include/asm/kvm_asm.h | 4 +++- >>> arch/powerpc/kvm/book3s.c | 4 ++++ >>> arch/powerpc/kvm/book3s_emulate.c | 18 ++++++++++++++++++ >>> arch/powerpc/kvm/book3s_pr.c | 17 +++++++++++++++++ >>> 4 files changed, 42 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/powerpc/include/asm/kvm_asm.h >>> b/arch/powerpc/include/asm/kvm_asm.h >>> index 1bd92fd43cfb..799244face51 100644 >>> --- a/arch/powerpc/include/asm/kvm_asm.h >>> +++ b/arch/powerpc/include/asm/kvm_asm.h >>> @@ -99,6 +99,7 @@ >>> #define BOOK3S_INTERRUPT_PERFMON 0xf00 >>> #define BOOK3S_INTERRUPT_ALTIVEC 0xf20 >>> #define BOOK3S_INTERRUPT_VSX 0xf40 >>> +#define BOOK3S_INTERRUPT_FAC_UNAVAIL 0xf60 >>> >>> #define BOOK3S_IRQPRIO_SYSTEM_RESET 0 >>> #define BOOK3S_IRQPRIO_DATA_SEGMENT 1 >>> @@ -117,7 +118,8 @@ >>> #define BOOK3S_IRQPRIO_DECREMENTER 14 >>> #define BOOK3S_IRQPRIO_PERFORMANCE_MONITOR 15 >>> #define BOOK3S_IRQPRIO_EXTERNAL_LEVEL 16 >>> -#define BOOK3S_IRQPRIO_MAX 17 >>> +#define BOOK3S_IRQPRIO_FAC_UNAVAIL 17 >>> +#define BOOK3S_IRQPRIO_MAX 18 >>> >>> #define BOOK3S_HFLAG_DCBZ32 0x1 >>> #define BOOK3S_HFLAG_SLB 0x2 >>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c >>> index 8912608b7e1b..a9aea28c2677 100644 >>> --- a/arch/powerpc/kvm/book3s.c >>> +++ b/arch/powerpc/kvm/book3s.c >>> @@ -143,6 +143,7 @@ static int kvmppc_book3s_vec2irqprio(unsigned int vec) >>> case 0xd00: prio = BOOK3S_IRQPRIO_DEBUG; break; >>> case 0xf20: prio = BOOK3S_IRQPRIO_ALTIVEC; break; >>> case 0xf40: prio = BOOK3S_IRQPRIO_VSX; break; >>> + case 0xf60: prio = BOOK3S_IRQPRIO_FAC_UNAVAIL; break; >>> default: prio = BOOK3S_IRQPRIO_MAX; break; >>> } >>> >>> @@ -273,6 +274,9 @@ int kvmppc_book3s_irqprio_deliver(struct kvm_vcpu >>> *vcpu, unsigned int priority) >>> case BOOK3S_IRQPRIO_PERFORMANCE_MONITOR: >>> vec = BOOK3S_INTERRUPT_PERFMON; >>> break; >>> + case BOOK3S_IRQPRIO_FAC_UNAVAIL: >>> + vec = BOOK3S_INTERRUPT_FAC_UNAVAIL; >>> + break; >>> default: >>> deliver = 0; >>> printk(KERN_ERR "KVM: Unknown interrupt: 0x%x\n", priority); >>> diff --git a/arch/powerpc/kvm/book3s_emulate.c >>> b/arch/powerpc/kvm/book3s_emulate.c >>> index 60d0b6b745e7..bf6b11021250 100644 >>> --- a/arch/powerpc/kvm/book3s_emulate.c >>> +++ b/arch/powerpc/kvm/book3s_emulate.c >>> @@ -481,6 +481,15 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu >>> *vcpu, int sprn, ulong spr_val) >>> vcpu->arch.shadow_fscr = vcpu->arch.fscr & host_fscr; >>> break; >>> } >>> + case SPRN_EBBHR: >>> + vcpu->arch.ebbhr = spr_val; >>> + break; >>> + case SPRN_EBBRR: >>> + vcpu->arch.ebbrr = spr_val; >>> + break; >>> + case SPRN_BESCR: >>> + vcpu->arch.bescr = spr_val; >>> + break; >>> unprivileged: >>> default: >>> printk(KERN_INFO "KVM: invalid SPR write: %d\n", sprn); >>> @@ -607,6 +616,15 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu >>> *vcpu, int sprn, ulong *spr_val >>> case SPRN_FSCR: >>> *spr_val = vcpu->arch.fscr; >>> break; >>> + case SPRN_EBBHR: >>> + *spr_val = vcpu->arch.ebbhr; >>> + break; >>> + case SPRN_EBBRR: >>> + *spr_val = vcpu->arch.ebbrr; >>> + break; >>> + case SPRN_BESCR: >>> + *spr_val = vcpu->arch.bescr; >>> + break; >>> default: >>> unprivileged: >>> printk(KERN_INFO "KVM: invalid SPR read: %d\n", sprn); >>> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c >>> index 51d469f8c9fd..828056ec208f 100644 >>> --- a/arch/powerpc/kvm/book3s_pr.c >>> +++ b/arch/powerpc/kvm/book3s_pr.c >>> @@ -900,6 +900,23 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct >>> kvm_vcpu *vcpu, >>> case BOOK3S_INTERRUPT_PERFMON: >>> r = RESUME_GUEST; >>> break; >>> + case BOOK3S_INTERRUPT_FAC_UNAVAIL: >>> + { >>> + /* >>> + * Check for the facility that need to be emulated >>> + */ >>> + ulong fscr_ic = vcpu->arch.shadow_fscr >> 56; >>> + if (fscr_ic != FSCR_EBB_LG) { >>> + /* >>> + * We only disable EBB facility. >>> + * So only emulate that. >> >> I don't understand the comment. We emulate nothing at all here. We either >> - hit an EBB unavailable in which case we send the guest an illegal >> instruction interrupt or we >> - hit another facility interrupt in which case we forward the >> interrupt to the guest, but not the interrupt cause (fscr_ic). >> > > What i wanted to achive was, enable both TAR and DSCR and disable > EBB. The reason to disable EBB was, we are still not clear how to handle > PMU details in PR. Now with FSCR carrying that value, we would get > facility unavailable interrupt when we try to mfspr/mtspr few EBB > related registers. The PR guest kernel do that on context switch > (_switch). Now what we do here is to fallthrough and handle that via > emulate mtspr/mfspr. > > If we get facility unavailable interrupt due to any other reason, that > means PR guest has explicitly disabled that facility. Hence we forward > that as facility unavailable interrupt to guest allowing PR guest to > handle that. Please adjust the comment accordingly. From the code flow that is very unclear. "Disable" means we don't allow the guest to access EBB. You do want to allow the guest to use a fake version of EBB by emulating the facility unavailable interrupt. if (fscr_ic == FSCR_EBB_LG) { /* * We filtered EBB out of FSCR so that we get traps whenever the guest is trying to * access EBB registers. Thanks to that we can now emulate these instructions and * expose a virtual (no-op) ebb facility to the guest */ <call instruction emulation> } else { /* forward interrupt to the guest */ } Alex > > >> I think the EBB case should be explicit: >> >> /* We don't allow EBB inside the guest, so something must have gone >> terribly wrong */ >> if (fscr_ic == FSCR_EBB_LG) >> BUG(); >> > > Instead of BUG, we do handle few mfspr/mtspr via emulate which we are > mostly ignoring. For event based branch instruction, the emulation will > fail and we will send 0x700(interrupt program) to PR guest right ? > > >> vcpu->arch.fscr &= ~FSCR_IC_MASK; >> vcpu->arch.fscr |= vcpu->arch.shadow_fscr & FSCR_IC_MASK; >> kvmppc_book3s_queue_irqprio(vcpu, exit_nr); >> r = RESUME_GUEST; >> break; >> > > -aneesh > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html