Re: [PATCH v5 04/15] x86/mtrr: support setting MTRR state for software defined MTRRs

[Juergen Gross]

On 11.04.23 15:20, Borislav Petkov wrote:
> On Sat, Apr 01, 2023 at 08:36:41AM +0200, Juergen Gross wrote:
> > When running virtualized, MTRR access can be reduced (e.g. in Xen PV
> > guests or when running as a SEV-SNP guest under Hyper-V). Typically
> > the hypervisor will reset the MTRR feature in CPUID data, resulting
> > in no MTRR memory type information being available for the kernel.
> >
> > This has turned out to result in problems:
>
> Let's add the links to those problems:
>
> > - Hyper-V SEV-SNP guests using uncached mappings where they shouldn't
>
> I can't find Michael's original report, do you have a link?

DYM 
https://lore.kernel.org/lkml/BYAPR21MB16883ABC186566BD4D2A1451D7FE9@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ 
?

> > - Xen PV dom0 mapping memory as WB which should be UC- instead
>
> Link: https://lore.kernel.org/all/4fe9541e-4d4c-2b2a-f8c8-2d34a7284930@xxxxxxxxxxxxxxx/
>
> > Solve those problems by supporting to set a static MTRR state,
>
> s/by supporting to set a/allowing an MTRR static state override/
>
> > +void mtrr_overwrite_state(struct mtrr_var_range *var, unsigned int num_var,
> > +			  mtrr_type def_type)
> > +{
> > +	unsigned int i;
> > +
> > +	/* Only allowed to be called once before mtrr_bp_init(). */
> > +	if (WARN_ON(mtrr_state_set))
>
> WARN_ON_ONCE() is probably better.

If you like that better (I don't see the real benefit, as something would
be severely broken if this triggers more than once, but I don't really
mind).

> > +		return;
> > +
> > +	/* Only allowed when running virtualized. */
> > +	if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
> > +		return;
> > +
> > +	/*
> > +	 * Only allowed for special virtualization cases:
> > +	 * - when running as SEV-SNP guest
> > +	 * - when running as Hyper-V isolated guest
>
> 	when running as a SEV-SNP guest on a HyperV with vTOM enabled
>
> that's a single condition.
>
> > +	 * - when running as Xen PV guest
> > +	 * - when running as TSX guest
> > +	 */
> > +	if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP) &&
> > +	    !hv_is_isolation_supported() &&
> > +	    !cpu_feature_enabled(X86_FEATURE_XENPV) &&
> > +	    !cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
>
> IOW:
>
> 	if (!(hv_is_isolation_supported() && cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) &&
> 	    !cpu_feature_enabled(X86_FEATURE_XENPV) &&
> 	    !cpu_feature_enabled(X86_FEATURE_TDX_GUEST))

Okay.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature