Messages in this thread | | | From | "Huang, Kai" <> | Subject | Re: [PATCH v3 1/2] x86/cpu/intel: Fix MTRR verification for TME enabled platforms | Date | Fri, 29 Sep 2023 09:14:00 +0000 |
| |
On Thu, 2023-09-28 at 15:30 -0700, Compostella, Jeremy wrote: > On TME enabled platform, BIOS publishes MTRR taking into account Total > Memory Encryption (TME) reserved bits. > > generic_get_mtrr() performs a sanity check of the MTRRs relying on the > `phys_hi_rsvd' variable which is set using the cpuinfo_x86 structure > `x86_phys_bits' field. But at the time the generic_get_mtrr() > function is ran the `x86_phys_bits' has not been updated by > detect_tme() when TME is enabled. > > Since the x86_phys_bits does not reflect yet the real maximal physical > address size yet generic_get_mtrr() complains by logging the following > messages. > > mtrr: your BIOS has configured an incorrect mask, fixing it. > mtrr: your BIOS has configured an incorrect mask, fixing it. > [...] > > In such a situation, generic_get_mtrr() returns an incorrect size but > no side effect were observed during our testing. > > For `x86_phys_bits' to be updated before generic_get_mtrr() runs, > move the detect_tme() call from init_intel() to early_init_intel().
Hi,
This move looks good to me, but +Kirill who is the author of detect_tme() for further comments.
Also I am not sure whether it's worth to consider to move this to get_cpu_address_sizes(), which calculates the virtual/physical address sizes. Thus it seems anything that can impact physical address size could be put there.
Perhaps can be done in the future if ever needed.
> > Change for v3: > Take review commit message comments into account. > > Change for v2: > Add a comment in the code explaining why detect_tme() needs to called > in early_init_intel().
You can put version history to
<Your SoB> --- version history --- ...
then it will be stripped away when the patch is applied.
Also, for related multiple patches please use --cover-letter --thread to make it a series. You can also use --base=auto to include the base commit id to the patch.
| |