Re: [PATCH] KVM: PPC: e500: Expose MMU registers via ONE_REG

[Scott Wood <scottwood@freescale.com>]

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.

> +	case KVM_REG_PPC_TLB0CFG:
> +	case KVM_REG_PPC_TLB1CFG:
> +	case KVM_REG_PPC_TLB2CFG:
> +	case KVM_REG_PPC_TLB3CFG: {
> +		u32 tlbncfg = set_reg_val(id,  
> *val);				
> +		u32 geometry_mask = TLBnCFG_N_ENTRY | TLBnCFG_ASSOC;
> +		i = id - KVM_REG_PPC_TLB0CFG;
> +
> +		/* MMU geometry (way/size) can be set only using SW_TLB  
> */
> +		if ((vcpu->arch.tlbcfg[i] & geometry_mask) !> +		    (tlbncfg & geometry_mask))
> +			r = -EINVAL;
> +
> +		vcpu->arch.tlbcfg[i] = set_reg_val(id, *val);
> +		break;
> +	}

Likewise -- just because QEMU sets a bit here doesn't mean KVM can  
support it.

I thought the initial plan for setting these config registers was to  
accept it if it exactly matches what KVM already has, and give an error  
otherwise -- thus allowing for the possibliity of accepting certain  
specific updates in the future.

-Scott