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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.05.18 at 10:35, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, May 15, 2018 at 01:52:43AM -0600, Jan Beulich wrote:
>> >>> On 14.05.18 at 18:33, <roger.pau@xxxxxxxxxx> wrote:
>> > On Mon, May 14, 2018 at 08:26:30AM -0600, Jan Beulich wrote:
>> >> >>> On 10.05.18 at 19:15, <roger.pau@xxxxxxxxxx> 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@xxxxxxxxxx>
>> >> > ---
>> >> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
>> >> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> >> > ---
>> >> >  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.

"The guest" being Dom0 here, don't forget. I simply don't see how you
would properly mimic the behavior without there actually being fixed
range MTRRs. Plus it contradicts the patch description.

Jan

> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel