Re: [PATCH v4 06/10] tools/libguest: Make setting MTRR registers unconditional

["Alejandro Vallejo" <alejandro.vallejo@cloud.com>]

On Thu Jun 27, 2024 at 10:42 AM BST, Jan Beulich wrote:
> On 26.06.2024 18:28, Alejandro Vallejo wrote:
> > This greatly simplifies a later patch that makes use of HVM contexts to upload
> > LAPIC data. The idea is to reuse MTRR setting procedure to avoid code
> > duplication. It's currently only used for PVH, but there's no real reason to
> > overcomplicate the toolstack preventing them being set for HVM too when
> > hvmloader will override them anyway.
>
> Yet then - why set them when hvmloader will do so again?

To keep the toolstack complexity tractable, essentially. This way I can send N
hypercalls (for N vCPUs) rather than 2*N and have a single hvmcontext struct
rather than several.

In truth though, I could simply write back the old MTRRs taken from bsp_ctx on
HVM.

> Is it even guaranteed
> to be no change in (guest) behavior to do so?

hvmloader overrides those values, so there is no change by the time BIOS or OVMF
start running. As I mentioned before though, I can actually upload back the old
values in the HVM case.

>
> Plus what about a guest which was configured to have the CPUID bit for MTRRs
> clear?
> I think we ought to document this as not supported for PVH (we may

By "this" do you mean PVH _must_ have MTRR support? I would agree.

> actually choose to refuse building such a guest), but in principle the MTRR
> save/load operations should simply fail for a HVM guest in said configuration.

What use cases does that cover? With the adjustment I mention at the top that
should be sorted. I'm wondering why we allow !mtrr at all.

> Making such a change in Xen now would, afaict, be benign to the tool stack.
> After this adjustment it would result in a perceived regression, when there
> shouldn't be any.

Fair point.

>
> Thinking about it, even for PVH it may make sense to allow CPUID.MTRR=0, as
> long as CPUID.PAT=1, thus forcing it into PAT-only mode. I think we did even
> discuss this possible configuration before.
>
> Jan

Is PAT-only an existing real HW configuration? Can't say I've seen any.

Cheers,
Alejandro