Re: [PATCH v2 2/8] x86/mtrr: support setting MTRR state for software defined MTRRs

[Juergen Gross]

On 13.02.23 16:03, Borislav Petkov wrote:
> On Mon, Feb 13, 2023 at 03:07:07PM +0100, Juergen Gross wrote:
> > Fixed in the sense of static.
>
> Well, you can't use "fixed" to say "static" when former means something
> very specific already in MTRR land.
>
> > Wouldn't !cpu_feature_enabled(X86_FEATURE_HYPERVISOR) be enough?
> >
> > I'm not sure we won't need that for TDX guests, too.
>
> See, that's the problem. I wanna have it simple too. Lemme check with
> dhansen.
>
> > Yes, it is only relevant for PV dom0.
>
> Right, I was asking whether "PV dom0" == X86_FEATURE_XENPV?

No, you can have PV guests not being dom0.

> :)
>
> > The number of fixed MTRRs is not dynamic AFAIK.
>
> But nothing guarantees that the caller would pass an array "mtrr_type
> *fixed" of size MTRR_NUM_FIXED_RANGES, right?

Right.

In the end I wouldn't mind dropping the fixed MTRRs from the interface, as
they are currently not needed at all.

> > A single interface makes it easier to avoid multiple calls.
> >
> > In the end I'm fine with either way.
>
> Yeah, I know. Question is, how much of this functionality will be
> needed/used so that we can go all out on the interface design or we can
> do a single one and forget about it...

I'd say we go with what is needed right now. And having a single interface
makes all the sanity checking you are asking for easier.

> > > Can Xen use x86_hyper_type() too?
> >
> > It does.
>
> Then pls add a x86_hyper_type check too to make sure a potential move of
> this call is caught in the future.

What are you especially asking for?

With my current patches Xen PV dom0 will call mtrr_overwrite_state() before
x86_hyper_type is set, while a Hyper-V SEV-SNP guest will make the call after
it has been set. Both calls happen before cache_bp_init().

So I could move the mtrr_overwrite_state() call for Xen PV dom0 into its
init_platform() callback and check in mtrr_overwrite_state() x86_hyper_type
to be set, or I could reject a call of mtrr_overwrite_state() after the call
of cache_bp_init() has happened, or I could do both.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature