Re: [Xen-devel] PVH and mtrr/PAT.........

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx
> [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of Jan Beulich
> Sent: Friday, November 22, 2013 6:29 PM
> To: George Dunlap; Dong, Eddie; Nakajima, Jun
> Cc: xen-devel; Tim Deegan
> Subject: Re: [Xen-devel] PVH and mtrr/PAT.........
> 
> >>> On 21.11.13 at 16:47, George Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote:
> > On Wed, Nov 20, 2013 at 10:24 PM, Mukesh Rathor
> > <mukesh.rathor@xxxxxxxxxx> wrote:
> >> On Wed, 20 Nov 2013 18:12:13 +0000
> >> George Dunlap <dunlapg@xxxxxxxxx> wrote:
> >>
> >>> On Wed, Nov 20, 2013 at 8:42 AM, Jan Beulich <JBeulich@xxxxxxxx>
> >>> wrote:
> >>> >>>> On 20.11.13 at 03:11, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> >>> >>>> wrote:
> >>> >> After rebasing my dom0 on latest, it didn't boot. After debugging
> >>> >> couple days, it turned out to be :
> >>> >>
> >>> >> +    if ( is_pvh_domain(d) )
> >>> >> +    {
> >>> >> +        if ( direct_mmio )
> >>> >> +            return MTRR_TYPE_UNCACHABLE;
> >>> >> +        return MTRR_TYPE_WRBACK;
> >>> >> +    }
> >>> >> +
> >>> >>
> >>> >> I had in my patches, missing in epte_get_entry_emt() in latest.
> >>> >>
> >>> >> So, since I don't know much about this, is an HVM guest setting
> >>> >> MTRR range types? Looking for suggestions on best way to do this
> >>> >> for PVH.
> >>> >
> >>> > A HVM guest is permitted to write to (virtual) MTRRs, whereas a PV
> >>> > guest isn't. I'm inclined to prefer PV behavior here to be used for
> >>> > PVH (since, as explained by Dongxiao, MTRRs don't really matter
> >>> > for VMX guests anyway, i.e. the setting of (virtual) MTRRs needs to
> >>> > get translated to EPT memory types anyway, hence a PVH guest
> >>> > ought to be fine ignoring the MTRRs altogether and handling memory
> >>> > types exclusively via PAT mechanisms).
> >>>
> >>> Mukesh,
> >>>
> >>> Do you know why this line is having this effect?  For one, is it a
> >>> matter of direct_mmio pages being given something other than
> >>> UNCACHEABLE, or a matter of non-direct_mmio pages given something
> >>> other than WRBACK?
> >>>
> >>> And is the problem that the guest is *not* setting MTRRs, or that the
> >>> guest *is* setting MTRRs?
> >>>
> >>> I'd prefer to avoid having a special case for PVH in this path if
> >>> possible.
> >>
> >> Without any changes to epte_get_entry_emt(), all types are being returned
> >> as MTRR_TYPE_WRBACK for PVH because of:
> >>
> >>     if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_IDENT_PT] )
> >>             return MTRR_TYPE_WRBACK;
> >>
> >> This is problem for low level non-ram pages being accessed in dom0,
> >> (which interesting gave MCE errors). non-ram IO pages have to be
> >> MTRR_TYPE_UNCACHABLE.
> >
> > Hmm -- that looks like a bug in the logic there.  AFAICT, there's no
> > reason for the lack of IDENT_PT to change the memory type like that.
> >
> > Unfortunately the changeset that introduced this (77283249) has
> > neither comments nor a proper description of what's going on, so it's
> > hard to tell where this came from.
> 
> While a commit without any description at all is clearly bogus (even
> more so in the light that this is the very change that also caused
> XSA-60), in the case here it introduces the function as a whole, and
> hence would not have been likely to comment on why the function
> was written the way it is.
> 
> I take it that this goes alongside the other check immediately previous
> to it: When not set yet, assume WB (for the sake of the tool stack).
> But I think this tool stack requirement should be expressed without
> looking at this HVM param.
> 
> Sadly the person having written that code doesn't appear to be
> working on Xen anymore (and my not be at Intel anymore either),
> so I'm afraid we'll have to sort this out ourselves. Nevertheless -
> Intel folks, can you comment on this please?

Hi Jan,

This is really a piece of old code, and we can't recall why this judgment 
(IDENT_PT) is added in epte_get_entry_emt().

From current view, these two lines are buggy and not necessary, and I will make 
a patch to remove them.

Thanks,
Dongxiao

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel