Re: [PATCH v3 3/5] mtrr, x86: Fix MTRR state checks in mtrr_type_lookup()

[Ingo Molnar]

* Toshi Kani <toshi.kani@xxxxxx> wrote:

> 'mtrr_state.enabled' contains FE (fixed MTRRs enabled) and
> E (MTRRs enabled) flags in MSR_MTRRdefType.  Intel SDM,
> section 11.11.2.1, defines these flags as follows:
>  - All MTRRs are disabled when the E flag is clear.
>    The FE flag has no affect when the E flag is clear.
>  - The default type is enabled when the E flag is set.
>  - MTRR variable ranges are enabled when the E flag is set.
>  - MTRR fixed ranges are enabled when both E and FE flags
>    are set.
> 
> MTRR state checks in __mtrr_type_lookup() do not follow the
> SDM definitions.  Therefore, this patch fixes the MTRR state
> checks according to the SDM.  This patch defines the flags
> in mtrr_state.enabled as follows.  print_mtrr_state() is also
> updated.
>  - FE flag: MTRR_STATE_MTRR_FIXED_ENABLED
>  - E  flag: MTRR_STATE_MTRR_ENABLED
> 
> Lastly, this patch fixes the 'else if (start < 0x1000000)',
> which checks a fixed range but has an extra-zero in the
> address, to 'else' with no condition.

Firstly, this does multiple bug fixes in a single patch, which is a 
no-no: please split it up into separate patches.

Secondly, please also outline the differences between the old code and 
the new code - don't just list the SDM logic and state that we are 
updating to it.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/