Re: [PATCH 3/5] hvm/mtrr: copy hardware state for Dom0

["Roger Pau Monné" <roger.pau@citrix.com>]

On Tue, May 15, 2018 at 01:52:43AM -0600, Jan Beulich wrote:
> >>> On 14.05.18 at 18:33, <roger.pau@citrix.com> wrote:
> > On Mon, May 14, 2018 at 08:26:30AM -0600, Jan Beulich wrote:
> >> >>> On 10.05.18 at 19:15, <roger.pau@citrix.com> wrote:
> >> > Copy the state found on the hardware when creating a PVH Dom0. Since
> >> > the memory map provided to a PVH Dom0 is based on the native one using
> >> > the same set of MTRR ranges should provide Dom0 with a sane MTRR state
> >> > without having to manually build it in Xen.
> >> > 
> >> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> > ---
> >> > Cc: Jan Beulich <jbeulich@suse.com>
> >> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >> > ---
> >> >  xen/arch/x86/hvm/mtrr.c | 23 +++++++++++++++++++++++
> >> >  1 file changed, 23 insertions(+)
> >> > 
> >> > diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
> >> > index 95a3deabea..1cb000388a 100644
> >> > --- a/xen/arch/x86/hvm/mtrr.c
> >> > +++ b/xen/arch/x86/hvm/mtrr.c
> >> > @@ -176,6 +176,29 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v)
> >> >          ((uint64_t)PAT_TYPE_UC_MINUS << 48) |       /* PAT6: UC- */
> >> >          ((uint64_t)PAT_TYPE_UNCACHABLE << 56);      /* PAT7: UC */
> >> >  
> >> > +    if ( is_hardware_domain(v->domain) )
> >> > +    {
> >> > +        /* Copy values from the host. */
> >> > +        struct domain *d = v->domain;
> >> > +        unsigned int i;
> >> > +
> >> > +        if ( mtrr_state.have_fixed )
> >> > +            for ( i = 0; i < NUM_FIXED_MSR; i++ )
> >> > +                mtrr_fix_range_msr_set(d, m, i,
> >> > +                                      ((uint64_t *)mtrr_state.fixed_ranges)[i]);
> >> 
> >> The presence/absence of fixed range MTRRs needs to be reflected in the
> >> capabilities MSR. Strictly speaking in their absence MSR access attempts to
> >> the fixed range MSRs should also cause #GP, as should any attempt to
> >> enable them in defType.
> > 
> > My intention was to always provide the fixed range MTRR capability,
> > regardless of whether the underlying hardware has it or not. Then of
> > course fixed ranges won't be enabled by default in the deftype MSR if
> > the underlying hardware also hasn't got them enabled.
> 
> What would the result be of the OS writing to any of these MSRs, or
> setting the respective enable bit?

Likely the cache attributes for the guest will change if it sets some
fixed ranges and enables the FE bit. But I'm not sure why is that a
problem.

If a guest plays with the MTRR ranges it must know what it's doing
anyway, and the fact that the underlying hardware has fixed range
support or not doesn't affect the changes that the guest might be
trying to perform to it's virtual MTRR registers/state.

Likewise the guest could change the variable ranges and mess up with
the types, but that's just going to affect the guest cache attributes,
not the host (Xen) ones.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel