Re: [PATCH v11 08/16] KVM: x86/pmu: Refactor code to support guest Arch LBR

[Yang, Weijiang]

On 5/6/2022 11:03 PM, Liang, Kan wrote:
> On 5/5/2022 11:32 PM, Yang Weijiang wrote:
>
>    bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu)
> @@ -199,12 +203,20 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
>    		return ret;
>    	}
>    
> -	ret = (index == MSR_LBR_SELECT) || (index == MSR_LBR_TOS) ||
> -		(index >= records->from && index < records->from + records->nr) ||
> -		(index >= records->to && index < records->to + records->nr);
> +	if (!guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR))
> +		ret = (index == MSR_LBR_SELECT) || (index == MSR_LBR_TOS);
> +
> Shouldn't we return immediately if (ret == true)?
> Keep checking if (!ret) looks uncommon.
>
> Actually we probably don't need the ret in this function.
>
> 	if (!guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR) &&
> 	    ((index == MSR_LBR_SELECT) || (index == MSR_LBR_TOS)))
> 		return true;
>
> > +	if (!ret) {
> > +		ret = (index >= records->from &&
> > +		       index < records->from + records->nr) ||
> > +		      (index >= records->to &&
> > +		       index < records->to + records->nr);
> > +	}
> 	if ((index >= records->from &&
> 	    index < records->from + records->nr) ||
> 	    (index >= records->to &&
> 	    index < records->to + records->nr))
> 		return true;
>
> >    
> > -	if (!ret && records->info)
> > -		ret = (index >= records->info && index < records->info + records->nr);
> > +	if (!ret && records->info) {
> > +		ret = (index >= records->info &&
> > +		       index < records->info + records->nr);
> > +	}
> 	if (records->info &&
> 	    (index >= records->info && index < records->info + records->nr)
> 		return true;
>
> 	return false;
> Sorry, I didn't notice it in the previous review.

Thanks Kan, so I'll modify this function as below (keeping other part 
unchanged):