Messages in this thread | | | Date | Fri, 31 Mar 2023 15:23:13 +0200 | From | Juergen Gross <> | Subject | Re: [PATCH v4 09/12] x86/mtrr: construct a memory map with cache modes |
| |
On 31.03.23 14:55, Borislav Petkov wrote: > On Wed, Mar 29, 2023 at 03:39:35PM +0200, Juergen Gross wrote: >> No. :-) > > Because?
In general the critical case is add_map_entry_at() returning 2 (in the case it is returning 1, the index can be set to -1, but there is always the "continue" statement right after that, which would execute the "i++" of the "for" statement).
add_map_entry_at() can return 2 only, if it detects "merge_prev" and "merge_next". "merge_prev" can be set only if the current index was > 0, which makes it impossible to return 2 if the index was 0.
>> The final form of the code is the result of an iterative process. :-) > > I have a similar iterative process: until it hasn't been reviewed and > explained properly, this is not going anywhere.
Of course.
> So however you wanna do it, fine by me. > >> I've reused the wording from cleanup.c (just above amd_special_default_mtrr()). > > That got added with K8. K8 is ancient history so nothing magic about > that anymore. It is basically a bit in the SYSCFG MSR which says that > > [4G ... TOP_MEM2] > > is WB.
How should it be named? AMD TOP_MEM2 MSR?
>>> Why not in mtrr_bp_init()? That is the first CPU. >> >> Yeah, but generic_set_mtrr() can be called after boot, too. > > That function sets a single MTRR register so you'd have to merge the > ranges, AFAICT. Not rebuild the whole map...
The problem isn't an added MTRR register, but a possibly replaced or removed one. Handling that is much more complicated, so I've chosen to do it the simple way.
In the end I'd expect setting of MTRRs to be a rare event, so there shouldn't be a performance issue with that approach.
> >> Umm, not really. I want to do the copy even in the Xen PV case. > > How about some comments? Or you're expecting me to be able to read your > mind?!
Okay, I'll add some more comments.
OTOH, what was hard to write should be hard to read (just kidding).
Juergen [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |