Re: [Xen-devel] [PATCH v2 for-next v2 5/8] x86/mm: split PV guest supporting code to pv/mm.c

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

> -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.

> @@ -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.

Jan


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