Re: [RFC PATCH 11/16]: PVH xen: some misc changes like mtrr, intr, msi.

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Mon, 14 Jan 2013 12:07:32 +0000
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 12.01.13 at 03:03, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> Doesn't the latter check by itself already do what you want?

Yup. fixed.

> > @@ -606,6 +612,7 @@ static int hvm_save_mtrr_msr(struct doma
> > +
> >      /* save mtrr&pat */
> >      for_each_vcpu(d, v)
> >      {
> 
> Please drop benign changes like this from this already big patch
> series.

Ok. Undone.

> > --- a/xen/arch/x86/msi.c	Fri Jan 11 16:34:17 2013 -0800
> > +++ b/xen/arch/x86/msi.c	Fri Jan 11 16:35:48 2013 -0800
> > @@ -766,10 +766,12 @@ static int msix_capability_init(struct p
> >          WARN_ON(rangeset_overlaps_range(mmio_ro_ranges,
> > dev->msix_pba.first, dev->msix_pba.last));
> >  
> > -        if ( rangeset_add_range(mmio_ro_ranges,
> > dev->msix_table.first, +/* PVH: for now we don't make the mmio
> > range readonly. See xen-devel for thread:
> > + * "[PVH]: Help: msi.c". When linux msi.c is fixed, pvh check can
> > be removed */
> > +        if ( !is_pvh_domain(dev->domain) &&
> > rangeset_add_range(mmio_ro_ranges, dev->msix_table.first,
> > dev->msix_table.last) ) WARN();
> > -        if ( rangeset_add_range(mmio_ro_ranges,
> > dev->msix_pba.first,
> > +        if ( !is_pvh_domain(dev->domain) &&
> > rangeset_add_range(mmio_ro_ranges, dev->msix_pba.first,
> > dev->msix_pba.last) ) WARN();
> 
> I hope there is no plan for this to go in in this shape.


Can I ifdef it and make it go'able? Ifdef saying PVH is experimental?
Not sure who's working on the issue on linux side.


> > --- a/xen/arch/x86/x86_emulate/x86_emulate.c	Fri Jan 11
> > 16:34:17 2013 -0800 +++
> > b/xen/arch/x86/x86_emulate/x86_emulate.c	Fri Jan 11 16:35:48
> > 2013 -0800 @@ -968,6 +968,10 @@ static int
> > ioport_access_check( struct segment_register tr; int rc =
> > X86EMUL_OKAY; 
> > +    /* PVH should not really get here */
> > +    /* fixme: need bunch of headers for this assert. check why no
> > headers. */
> 
> Because the emulator is intended to be (almost) standalone, so
> building the emulator test (as user space app) is also possible.

Got it, thanks.