[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 for-next v2 5/8] x86/mm: split PV guest supporting code to pv/mm.c
On Wed, Apr 19, 2017 at 09:31:12AM -0600, Jan Beulich wrote: > >>> On 03.04.17 at 13:22, <wei.liu2@xxxxxxxxxx> wrote: > > -static s8 __read_mostly opt_mmio_relax; > > +s8 __read_mostly opt_mmio_relax; > > static void __init parse_mmio_relax(const char *s) > > The variable has exactly one use site outside of the parsing function, > which means both variable and parsing function should be moved too. > However, this sole user is get_page_from_l1e(), and that in turn is > also being used from shadow code. It's not immediately clear to me > whether moving it under pv/ (and hence eventually compiling it out > of Xen when PV is de-selected) is the right thing. > Yes, you're right. I read the shadow code but couldn't immediately figure out whether the call site of get_page_from_l1e is PV specific. I plan to leave get_page_from_l1e in mm.c and make adjustments to the series where necessary. > > -static int get_page_from_pagenr(unsigned long page_nr, struct domain *d) > > +int get_page_from_pagenr(unsigned long page_nr, struct domain *d) > > { > > I don't think this should be made non-static without first correcting its > return type - either int (and returning -E... values) or bool. > Another patch it is. > > @@ -834,3414 +524,489 @@ static int update_xen_mappings(unsigned long mfn, > > unsigned int cacheattr) > > return err; > > } > > > > -#ifndef NDEBUG > > -struct mmio_emul_range_ctxt { > > - const struct domain *d; > > - unsigned long mfn; > > -}; > > - > > -static int print_mmio_emul_range(unsigned long s, unsigned long e, void > > *arg) > > +bool_t fill_ro_mpt(unsigned long mfn) > > { > > - const struct mmio_emul_range_ctxt *ctxt = arg; > > - > > - if ( ctxt->mfn > e ) > > - return 0; > > + l4_pgentry_t *l4tab = map_domain_page(_mfn(mfn)); > > + bool_t ret = 0; > > From here on the patch becomes basically unreviewable, because > plain removals (which I'd expect is all that is happening here) aren't > represented as such anymore. Please see whether you can > arrange for the diff to become more legible. If you can't talk git > into doing so, maybe an option would be to split this patch by > temporarily #include-ing pv/mm.c here until the move is complete. > I will see what I can do here. I will probably end up splitting it to more patches. Wei. > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |