> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, March 27, 2013 12:43 AM
> To: Caraman Mihai Claudiu-B02008
> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; agraf@xxxxxxx
> Subject: Re: [PATCH 1/7 v2] KVM: PPC: e500: Expose MMU registers via
> ONE_REG
>
> 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.
I also found the existing code difficult to read. If we stick to reg and
val varible names what about val_to_reg/reg_to_val.
-Mike
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
