Re: [RFC PATCH 08/10] KVM: PPC: BOOK3S: PR: Add support for facility unavailable interrupt

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

Alexander Graf <agraf@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.kumar@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. 


> 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