> -----Original Message----- > From: kvm-ppc-owner@xxxxxxxxxxxxxxx [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On > Behalf Of Alexander Graf > Sent: Monday, July 09, 2012 8:07 PM > To: Wood Scott-B07421 > Cc: Bhushan Bharat-R65777; Wood Scott-B07421; kvm-ppc@xxxxxxxxxxxxxxx; > kvm@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 2/2] KVM: PPC: booke: Add watchdog emulation > > > On 09.07.2012, at 16:28, Scott Wood wrote: > > > On 07/09/2012 12:13 AM, Bhushan Bharat-R65777 wrote: > >> > >> > >>> -----Original Message----- > >>> From: Alexander Graf [mailto:agraf@xxxxxxx] > >>> Sent: Saturday, July 07, 2012 1:21 PM > >>> To: Wood Scott-B07421 > >>> Cc: Bhushan Bharat-R65777; kvm-ppc@xxxxxxxxxxxxxxx; > >>> kvm@xxxxxxxxxxxxxxx; Bhushan > >>> Bharat-R65777 > >>> Subject: Re: [PATCH 2/2] KVM: PPC: booke: Add watchdog emulation > >>> > >>> > >>> Ah, being 2 bits wide, the above code suddenly makes more sense :). > >>> How about > >>> > >>> /* WRC is a 2-bit field that is supposed to preserve its value once > >>> written to be non-zero */ spr_val &= ~TCR_WRC_MASK; spr_val |= > >>> vcpu->arch.tcr & TCR_WRC_MASK; kvmppc_set_tcr(vcpu, spr_val); > >> > >> I think you mean: > >> > >> if (TCR_WRC_MASK & vcpu->arch.tcr) { > >> spr_val &= ~TCR_WRC_MASK; > >> spr_val |= vcpu->arch.tcr & TCR_WRC_MASK; } kvmppc_set_tcr(vcpu, > >> spr_val); > > > > Actually I think he means: > > > > if (vcpu->arch.tcr & TCR_WRC_MASK) { > > spr_val &= ~TCR_WRC_MASK; > > spr_val |= vcpu->arch.tcr & TCR_WRC_MASK; } > > > > kvmppc_set_tcr(vcpu, spr_val); > > Plus the comment, yes :). I really don't like (mask & val) constructs. About as > much as I dislike if (0 == x) ones. I think I did in v2 :) > > > Alex > > -- > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body > of a message to majordomo@xxxxxxxxxxxxxxx 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
