Re: [PATCH v7 4/7] xen/arm: account IO handler for emulated PCI host bridge

[Julien Grall <julien@xxxxxxx>]

Hi Oleksandr,

On 24/11/2021 07:59, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>
> At the moment, we always allocate an extra 16 slots for IO handlers
> (see MAX_IO_HANDLER). So while adding an IO trap handler for the emulated
> PCI host bridge we are not breaking anything, but we have a latent bug
> as the maximum number of IOs may be exceeded.
> Fix this by explicitly telling that we have an additional IO handler, so it is
> accounted.
>
> Fixes: d59168dc05a5 ("xen/arm: Enable the existing x86 virtual PCI support for 
> ARM")
In general, it is better to have the fixes at the beginning of a series. 
So they are relying on less rework and easier to backport (if needed).
In this case, PCI passthrough is still a technical preview so it doesn't 
matter too much.
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>
> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
> index ccd998d8dba2..8e801f275879 100644
> --- a/xen/arch/arm/vpci.c
> +++ b/xen/arch/arm/vpci.c
> @@ -126,7 +126,8 @@ unsigned int domain_vpci_get_num_mmio_handlers(struct 
> domain *d)
>           return ret < 0 ? 0 : ret;
>       }
>   
> -    return 0;
> +    /* For a single emulated host bridge's configuration space. */
This comment is lacking some context. I would suggest to reword to 
something like:
"For the guests, each host bridge requires one region to cover the 
configuration space. At the moment, we only expose a single host bridge.
"

With that (or a similar comment):

Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall