Messages in this thread | | | Date | Thu, 20 Apr 2023 14:15:51 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v5 11/15] x86/mtrr: construct a memory map with cache modes |
| |
On Sat, Apr 01, 2023 at 08:36:48AM +0200, Juergen Gross wrote: > +static void rm_map_entry_at(int idx) > +{ > + cache_map_n--;
Let's not call memmove() when cache_map_n == idx.
Below too.
Especially since cache_map + idx + 1 is not valid and I wouldn't want it getting prefetched from %rsi in the hw when there's no reason for it and also the RET even from a function which doesn't do anything, costs.
> + memmove(cache_map + idx, cache_map + idx + 1, > + sizeof(*cache_map) * (cache_map_n - idx)); > +}
Ok, first weird issue I encountered while playing with my carved out program to exercise this cache_map handling thing. I can share it if you want it - it is ugly but it works.
So while rebuilding the map, I have these current regions in the map, at one point in time:
Current map: 0: start: 0x0000000000000000, end: 0x0000000000100000, type: 0x0 1: start: 0x0000000100000000, end: 0x0000000820000000, type: 0x6 2: start: 0x000002f10000c000, end: 0x000003bf0000c000, type: 0x2 3: start: 0x000003bf0000c000, end: 0x00000d4b0000c000, type: 0x1 4: start: 0x00000d4b0000c000, end: 0x00019fc000001000, type: 0x0 5: start: 0x00019fc000001000, end: 0x0001df2d00001000, type: 0x2
note entry #3.
Now the next one it inserts is:
add_map_entry_at: start: 0x3bf0000c000, end: 0xd4b0000c000, type: 0x0, idx: 3 merge_prev 0: prev->fixed: 0, prev->end: 0x3bf0000c000, prev->type: 0x2 merge_next: 1, next->fixed: 0, next->start: 0xd4b0000c000, next->type: 0x0 add_map_entry_at: ret: 1
Note how it is the same as entry number #3 - just a different type.
What it ends up doing is, it simply overwrites the previous one and merges it with the next:
Current map: 0: start: 0x0000000000000000, end: 0x0000000000100000, type: 0x0 1: start: 0x0000000100000000, end: 0x0000000820000000, type: 0x6 2: start: 0x000002f10000c000, end: 0x000003bf0000c000, type: 0x2 3: start: 0x000003bf0000c000, end: 0x00019fc000001000, type: 0x0 4: start: 0x00019fc000001000, end: 0x0001df2d00001000, type: 0x2
Now I think right about now we should've screamed loudly.
I know I know, this should never happen, right? And firmware programming those MTRRs doesn't make mistakes...
However, we should be loud here and scream when a MTRR range disappears like that.
Right?
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |