mbox series

[0/6] x86/mtrr: fix handling with PAT but without MTRR

Message ID 20230207072902.5528-1-jgross@suse.com
Headers
Series x86/mtrr: fix handling with PAT but without MTRR |

Message

Juergen Gross Feb. 7, 2023, 7:28 a.m. UTC 
  This series tries to fix the rather special case of PAT being available
without having MTRRs (either due to CONFIG_MTRR being not set, or
because the feature has been disabled e.g. by a hypervisor).

The main use cases are Xen PV guests and SEV-SNP guests running under
Hyper-V.

Patch 2 seems to be a little hacky, as it special cases only
memtype_reserve() and memtype_free(), but OTOH this doesn't seem to
be worse than in previous days, where PAT was disabled when MTRRs
haven't been available.

My tests with Xen didn't show any problems, but I'm rather sure I
couldn't cover all corner cases.

The only cleaner solution I could think of would be to introduce MTRR
read-only access. It would theoretically be possible to get the actual
MTRR contents for the variable MTRRs from Xen, but I'm not sure this
is really the way to go.

For the SEV-SNP case with Hyper-V I guess such a read-only mode could
be rather simple, but I'm really not sure this would cover all needed
corner cases (I'd basically say always "WB" in that case).

I have added more cleanup which has been discussed when looking into
the most recent failures.

Juergen Gross (6):
  x86/mtrr: make mtrr_enabled() non-static
  x86/pat: check for MTRRs enabled in memtype_reserve()
  x86/mtrr: revert commit 90b926e68f50
  x86/mtrr: don't let mtrr_type_lookup() return MTRR_TYPE_INVALID
  x86/mm: only check uniform after calling mtrr_type_lookup()
  x86/mtrr: drop sanity check in mtrr_type_lookup_fixed()

 arch/x86/include/asm/mtrr.h        | 13 +++++++++++--
 arch/x86/include/uapi/asm/mtrr.h   |  6 +++---
 arch/x86/kernel/cpu/mtrr/generic.c | 10 +++-------
 arch/x86/kernel/cpu/mtrr/mtrr.c    |  2 +-
 arch/x86/mm/pat/memtype.c          | 13 ++++++++-----
 arch/x86/mm/pgtable.c              |  6 ++----
 6 files changed, 28 insertions(+), 22 deletions(-)

Comments

Michael Kelley (LINUX) Feb. 8, 2023, 3:08 p.m. UTC | #1 
From: Juergen Gross <jgross@suse.com> Sent: Monday, February 6, 2023 11:29 PM
> 
> This series tries to fix the rather special case of PAT being available
> without having MTRRs (either due to CONFIG_MTRR being not set, or
> because the feature has been disabled e.g. by a hypervisor).
> 
> The main use cases are Xen PV guests and SEV-SNP guests running under
> Hyper-V.
> 
> Patch 2 seems to be a little hacky, as it special cases only
> memtype_reserve() and memtype_free(), but OTOH this doesn't seem to
> be worse than in previous days, where PAT was disabled when MTRRs
> haven't been available.
> 
> My tests with Xen didn't show any problems, but I'm rather sure I
> couldn't cover all corner cases.

I tested this patch set with Hyper-V SEV-SNP guests, and ioremap_cache()
is correctly mapping as WB.

As an observation, with commit 90b926e68f50 it was nice to have
the memtype entries created.  I could check for any unexpected
mappings in /sys/kernel/debug/x86/pat_memtype_list.  With this patch
set, we're back to not creating those entries.  

Michael

> 
> The only cleaner solution I could think of would be to introduce MTRR
> read-only access. It would theoretically be possible to get the actual
> MTRR contents for the variable MTRRs from Xen, but I'm not sure this
> is really the way to go.
> 
> For the SEV-SNP case with Hyper-V I guess such a read-only mode could
> be rather simple, but I'm really not sure this would cover all needed
> corner cases (I'd basically say always "WB" in that case).
> 
> I have added more cleanup which has been discussed when looking into
> the most recent failures.
> 
> Juergen Gross (6):
>   x86/mtrr: make mtrr_enabled() non-static
>   x86/pat: check for MTRRs enabled in memtype_reserve()
>   x86/mtrr: revert commit 90b926e68f50
>   x86/mtrr: don't let mtrr_type_lookup() return MTRR_TYPE_INVALID
>   x86/mm: only check uniform after calling mtrr_type_lookup()
>   x86/mtrr: drop sanity check in mtrr_type_lookup_fixed()
> 
>  arch/x86/include/asm/mtrr.h        | 13 +++++++++++--
>  arch/x86/include/uapi/asm/mtrr.h   |  6 +++---
>  arch/x86/kernel/cpu/mtrr/generic.c | 10 +++-------
>  arch/x86/kernel/cpu/mtrr/mtrr.c    |  2 +-
>  arch/x86/mm/pat/memtype.c          | 13 ++++++++-----
>  arch/x86/mm/pgtable.c              |  6 ++----
>  6 files changed, 28 insertions(+), 22 deletions(-)
> 
> --
> 2.35.3
Juergen Gross Feb. 8, 2023, 3:19 p.m. UTC | #2 
On 08.02.23 16:08, Michael Kelley (LINUX) wrote:
> From: Juergen Gross <jgross@suse.com> Sent: Monday, February 6, 2023 11:29 PM
>>
>> This series tries to fix the rather special case of PAT being available
>> without having MTRRs (either due to CONFIG_MTRR being not set, or
>> because the feature has been disabled e.g. by a hypervisor).
>>
>> The main use cases are Xen PV guests and SEV-SNP guests running under
>> Hyper-V.
>>
>> Patch 2 seems to be a little hacky, as it special cases only
>> memtype_reserve() and memtype_free(), but OTOH this doesn't seem to
>> be worse than in previous days, where PAT was disabled when MTRRs
>> haven't been available.
>>
>> My tests with Xen didn't show any problems, but I'm rather sure I
>> couldn't cover all corner cases.
> 
> I tested this patch set with Hyper-V SEV-SNP guests, and ioremap_cache()
> is correctly mapping as WB.
> 
> As an observation, with commit 90b926e68f50 it was nice to have
> the memtype entries created.  I could check for any unexpected
> mappings in /sys/kernel/debug/x86/pat_memtype_list.  With this patch
> set, we're back to not creating those entries.

I'm currently looking into the solution mentioned below. This might turn
out to be much cleaner.

> 
> Michael
> 
>>
>> The only cleaner solution I could think of would be to introduce MTRR
>> read-only access. It would theoretically be possible to get the actual
>> MTRR contents for the variable MTRRs from Xen, but I'm not sure this
>> is really the way to go.
>>
>> For the SEV-SNP case with Hyper-V I guess such a read-only mode could
>> be rather simple, but I'm really not sure this would cover all needed
>> corner cases (I'd basically say always "WB" in that case).


Juergen