Re: [Xen-devel] [PATCH v4 4/9] xen/mm: move modify_identity_mmio to global file and drop __init

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.06.17 at 17:01, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -1465,6 +1465,46 @@ int prepare_ring_for_helper(
>      return 0;
>  }
>  
> +#if defined(CONFIG_X86) || defined(CONFIG_HAS_PCI)

Why both? X86 selects HAS_PCI, and such (reverse) dependencies exist
precisely to avoid such conditionals to become rather complex over
time.

> +int modify_mmio(struct domain *d, gfn_t gfn, mfn_t mfn, unsigned long 
> nr_pages,
> +                const bool map)

Already in the original function I've been puzzled by this const - if
you wanted such, you should put it consistently on all applicable
parameters. But since we don't normally do so elsewhere, the globally
consistent approach would be to simply drop it.

> +{
> +    int rc;
> +
> +    /*
> +     * ATM this function should only be used by the hardware domain
> +     * because it doesn't support preemption/continuation, and as such
> +     * can take a non-trivial amount of time. Note that it periodically calls

non-negligible?

> +     * process_pending_softirqs in order to avoid stalling the system.
> +     */
> +    ASSERT(is_hardware_domain(d));
> +
> +    for ( ; ; )
> +    {
> +        rc = (map ? map_mmio_regions : unmap_mmio_regions)
> +             (d, gfn, nr_pages, mfn);
> +        if ( rc == 0 )
> +            break;
> +        if ( rc < 0 )
> +        {
> +            printk(XENLOG_G_WARNING

As long as this is Dom0 only I'd suggest to drop the _G_ infix, just
like it was in the original.

Jan


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