Re: [PATCH v5 04/15] x86/mtrr: support setting MTRR state for software defined MTRRs

["Huang, Kai" <kai.huang@intel.com>]

> > >   /**
> > >    * mtrr_type_lookup - look up memory type in MTRR
> > >    *
> > > diff --git a/arch/x86/kernel/cpu/mtrr/mtrr.c b/arch/x86/kernel/cpu/mtrr/mtrr.c
> > > index 1beb38f7a7a3..1c19d67ddab3 100644
> > > --- a/arch/x86/kernel/cpu/mtrr/mtrr.c
> > > +++ b/arch/x86/kernel/cpu/mtrr/mtrr.c
> > > @@ -666,6 +666,15 @@ void __init mtrr_bp_init(void)
> > >   	const char *why = "(not available)";
> > >   	unsigned int phys_addr;
> > >   
> > > +	if (!generic_mtrrs && mtrr_state.enabled) {
> > > +		/* Software overwrite of MTRR state, only for generic case. */
> > 							      ^
> > 							      !generic case?
> 
> No. This test just verifies that the (visible) MTRR feature is switched off,
> as there are no ways to modify any MTRR registers in the overwrite case.
> 
> I can make this more obvious in a comment.

Should the comment say something like (because it applies to the code inside the
check):


	If we have a static (synthetic) MTRR already established for special 
	VMs, we still need to calculate the physical address bits using
generic 
	way, because the hardware to run those special VMs indeed has MTRR.

That explains why 'true' is passed to mtrr_calc_physbits().

?

> 
> 
> Juergen