From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

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")

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

---
New in v7
---
 xen/arch/arm/vpci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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. */
+    return 1;
 }
 
 /*
-- 
2.25.1

Hi Oleksandr,

On 24/11/2021 07:59, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> 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@epam.com>
 >
> 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@amazon.com>

Cheers,

-- 
Julien Grall

Hi, Julien!

On 08.12.21 18:53, Julien Grall wrote:
> Hi Oleksandr,
>
> On 24/11/2021 07:59, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> 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.
I am planning to resend the whole series, so I can move this to the bottom,
but it is indeed doesn't matter at the moment
>
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> >
>> 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.
> "
>
Ok, will change
> With that (or a similar comment):
>
> Acked-by: Julien Grall <jgrall@amazon.com>
Thank you,
Oleksandr
>
> Cheers,
>

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.
> I am planning to resend the whole series, so I can move this to the bottom,
> but it is indeed doesn't matter at the moment
I tried to re-order, but this patch already depends on the previous in the series.
It needs to be fixed in a different way then which will change the patches above,
so I leave this where it is.

Thank you,
Oleksandr