Messages in this thread | | | Date | Tue, 7 Feb 2023 09:49:24 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 2/6] x86/pat: check for MTRRs enabled in memtype_reserve() |
| |
* Juergen Gross <jgross@suse.com> wrote:
> Today memtype_reserve() bails out early if pat_enabled() returns false. > The same can be done in case MTRRs aren't enabled. > > This will reinstate the behavior of memtype_reserve() before commit > 72cbc8f04fe2 ("x86/PAT: Have pat_enabled() properly reflect state when > running on Xen"). There have been reports about that commit breaking > SEV-SNP guests under Hyper-V, which was tried to be resolved by commit > 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled case"), > but that again resulted in problems with Xen PV guests. > > Fixes: 72cbc8f04fe2 ("x86/PAT: Have pat_enabled() properly reflect state when running on Xen") > Fixes: 90b926e68f50 ("x86/pat: Fix pat_x_mtrr_type() for MTRR disabled case") > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > arch/x86/mm/pat/memtype.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c > index fb4b1b5e0dea..18f612b43763 100644 > --- a/arch/x86/mm/pat/memtype.c > +++ b/arch/x86/mm/pat/memtype.c > @@ -557,8 +557,12 @@ int memtype_reserve(u64 start, u64 end, enum page_cache_mode req_type, > return -EINVAL; > } > > - if (!pat_enabled()) { > - /* This is identical to page table setting without PAT */ > + /* > + * PAT disabled or MTRRs disabled don't require any memory type > + * tracking or type adjustments, as there can't be any conflicts > + * between PAT and MTRRs with at least one of both being disabled. > + */ > + if (!pat_enabled() || !mtrr_enabled()) { > if (new_type) > *new_type = req_type;
Doesn't memtype_reserve() also check for overlapping ranges & type compatibility in memtype_check_conflict(), etc., which can occur even in a pure PAT setup? Ie. are we 100% sure that in the !MTRR case it would be a NOP?
But even if it's a functional NOP as you claim, we'd still be better off if the memtype tree was still intact - instead of just turning off the API.
Also, speling nit:
s/one of both /one or both
Thanks,
Ingo
| |