Messages in this thread | | | Date | Fri, 31 Mar 2023 15:35:07 +0200 | From | Juergen Gross <> | Subject | Re: [PATCH v4 09/12] x86/mtrr: construct a memory map with cache modes |
| |
On 31.03.23 14:57, Borislav Petkov wrote: > On Mon, Mar 06, 2023 at 05:34:22PM +0100, Juergen Gross wrote: >> +/* Build the cache_map containing the cache modes per memory range. */ >> +void mtrr_build_map(void) >> +{ >> + unsigned int i; >> + u64 start, end, size; >> + u8 type; >> + >> + if (!mtrr_state.enabled) > ^^^^^^^^^^^^^^^^^^^ > >> + return; >> + >> + /* Add fixed MTRRs, optimize for adjacent entries with same type. */ >> + if (mtrr_state.enabled & MTRR_STATE_MTRR_FIXED_ENABLED) { > ^^^^^^^^^^^^^^^^^^ > > First check can go.
Hmm, this makes it much harder to see that the code really does nothing if enabled isn't set.
> >> + start = 0; >> + end = size = 0x10000; >> + type = mtrr_state.fixed_ranges[0]; >> + >> + for (i = 1; i < MTRR_NUM_FIXED_RANGES; i++) { > > Loops starts at 1. Comment explaining why pls.
Okay.
> >> + if (i == 8 || i == 24) > > Magic numbers.
I'll add more comments.
> > Ok, I'll stop here. > > Please send a new version with the review comments incorporated. I need > to look more at that map thing more.
Will do.
Thanks,
Juergen
[unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |