Re: [Xen-devel] [PATCH] tools/pvh: set coherent MTRR state for all vCPUs

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Oct 04, 2018 at 01:22:12PM +0100, Andrew Cooper wrote:
> On 02/10/18 17:36, Roger Pau Monne wrote:
> > Instead of just doing it for the BSP. This requires storing the
> > maximum number of possible vCPUs in xc_dom_image.
> >
> > This has been a latent bug so far because PVH doesn't yet support
> > pci-passthrough, so the effective memory cache attribute is forced to
> > WB by the hypervisor. Note also that even without this in place vCPU#0
> > is preferred in certain scenarios in order to calculate the memory
> > cache attributes.
> >
> > Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> This is all moderately nasty, but that is entirely the fault of get/set
> context API.  Seeing as the interface is going to be fully rewritten
> eventually, this will do.
> 
> > ---
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > ---
> >  tools/libxc/include/xc_dom.h |  3 ++
> >  tools/libxc/xc_dom_x86.c     | 69 ++++++++++++++++++++++++------------
> >  tools/libxl/libxl_dom.c      |  2 ++
> >  3 files changed, 52 insertions(+), 22 deletions(-)
> >
> > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> > index 0b5a632d3c..bc125dbd3a 100644
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -230,6 +230,9 @@ struct xc_dom_image {
> >  #endif
> >  
> >      xen_pfn_t vuart_gfn;
> > +
> > +    /* Number of vCPUs */
> > +    unsigned int nr_vcpus;
> 
> I'd be tempted to name this max_vcpus so it can't be confused with CPU
> hotplug cases.

The naming was chosen to match existing fields in xc_dom_image that
already start with nr_. I don't mind changing it to match the
hypervisor side that uses max_vcpus.

> Somewhere early in the domain builder also might want to sanity check
> that it isn't 0.
> 
> If you're in the mood for some cleanup, build_hvm_info() probably wants
> an adjustment in light of this field now existing.
> 
> As for the eventual code, LGTM.

I can take a look tomorrow and send a new version.

Thanks, Roger.

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