Re: [PATCH v4 6/7] mtrr, x86: Clean up mtrr_type_lookup()

[Borislav Petkov]

On Wed, May 06, 2015 at 10:00:30AM -0600, Toshi Kani wrote:
> Ingo asked me to describe this info here in his review...

Ok.

> mtrr_type_lookup_fixed() checks the above conditions at entry, and
> returns immediately with TYPE_INVALID.  I think it is safer to have such
> checks in mtrr_type_lookup_fixed() in case there will be multiple
> callers.

This is not what I mean - I mean to call mtrr_type_lookup_fixed() based
on @start and not unconditionally, like you do.

And there most likely won't be multiple callers because we're phasing
out MTRR use.

And even if there are, they better look at how this function is being
called before calling it. Which I seriously doubt - it is a static
function which you *just* came up with.

> Right, and there is more.  As the original code had comment "Just return
> the type as per start", which I noticed that I had accidentally removed,
> the code only returns the type of the start address.  The fixed ranges
> have multiple entries with different types.  Hence, a given range may
> overlap with multiple fixed entries.  I will restore the comment in the
> function header to clarify this limitation.

Ok, let's cleanup this function first and then consider fixing other
possible bugs which haven't been fixed since forever. Again, we might
not even need to address them because we won't be using MTRRs once we
switch to PAT completely, which is what Luis is working on.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
--
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/