From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Subject: Re: [PATCH 1/7 v2] KVM: PPC: e500: Expose MMU registers via ONE_REG
Date: Tue, 26 Mar 2013 17:42:33 -0500
Message-ID: <1364337753.469.16@snotra>
References: <1364335512-28426-1-git-send-email-mihai.caraman@freescale.com>
	<1364335512-28426-2-git-send-email-mihai.caraman@freescale.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed
Content-Transfer-Encoding: 8BIT
Cc: <kvm-ppc@vger.kernel.org>, <kvm@vger.kernel.org>, <agraf@suse.de>
To: Mihai Caraman <mihai.caraman@freescale.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from co1ehsobe001.messaging.microsoft.com ([216.32.180.184]:39798
	"EHLO co1outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1754444Ab3CZWns convert rfc822-to-8bit
	(ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 26 Mar 2013 18:43:48 -0400
In-Reply-To: <1364335512-28426-2-git-send-email-mihai.caraman@freescale.com>
	(from mihai.caraman@freescale.com on Tue Mar 26 17:05:06 2013)
Content-Disposition: inline
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 03/26/2013 05:05:06 PM, Mihai Caraman wrote:
> +int kvmppc_set_one_reg_e500_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;
> +	/* Only allow MMU registers to be set to the config supported  
> by KVM */
> +	case KVM_REG_PPC_MMUCFG: {
> +		if (set_reg_val(id, *val) != vcpu->arch.mmucfg)
> +			r = -EINVAL;
> +		break;
> +	}
> +	case KVM_REG_PPC_TLB0CFG:
> +	case KVM_REG_PPC_TLB1CFG:
> +	case KVM_REG_PPC_TLB2CFG:
> +	case KVM_REG_PPC_TLB3CFG: {
> +		/* MMU geometry (N_ENTRY/ASSOC) can be set only using  
> SW_TLB */
> +		i = id - KVM_REG_PPC_TLB0CFG;
> +		if (set_reg_val(id, *val) != vcpu->arch.tlbcfg[i])
> +			r = -EINVAL;
> +		break;
> +	}

Am I the only one that finds the set_reg_val/get_reg_val naming  
confusing?  At first glance it looks like it sets TLBnCFG and then  
later tests whether it should have. :-P

Functions should be named for what they do, not for what context you  
use them in.

-Scott