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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.05.18 at 11:16, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, May 15, 2018 at 02:48:16AM -0600, Jan Beulich wrote:
>> >>> 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.
> 
> Please bear with me.
> 
> The reason of this patchset is to provide PVH Dom0 with a sane initial
> MTRR state, not to allow a PVH Dom0 to set the host MTRR state
> directly.
> 
> So the fact that the underlying hardware doesn't have support for
> fixed MTRR ranges shouldn't affect Xen's capability to provide such
> feature to Dom0.
> 
> I see no reason to allow Dom0 to directly control the host MTRR
> values. A PVH Dom0 has it's own physical memory map and can set
> whatever cache attributes it wishes without affecting the host MTRR
> types.

Oh, right, I've been confused by the mix of copying of host state and
and leaving untouched of virtual capabilities. I'm sorry for the noise.

Jan


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