[v10,15/17] xen/arm: account IO handlers for emulated PCI MSI-X

Commit Message

Volodymyr Babchuk Oct. 12, 2023, 10:09 p.m. UTC

From: Oleksandr Andrushchenko <[e-mail]>

At the moment, we always allocate an extra 16 slots for IO handlers
(see MAX_IO_HANDLER). So while adding IO trap handlers for the emulated
MSI-X registers we need to explicitly tell that we have additional IO
handlers, so those are accounted.

Signed-off-by: Oleksandr Andrushchenko <[e-mail]>
Acked-by: Julien Grall <[e-mail]>
---
Cc: Julien Grall <[e-mail]>
Cc: Stefano Stabellini <[e-mail]>
---
This actually moved here from the part 2 of the prep work for PCI
passthrough on Arm as it seems to be the proper place for it.

Since v5:
- optimize with IS_ENABLED(CONFIG_HAS_PCI_MSI) since VPCI_MAX_VIRT_DEV is
  defined unconditionally
New in v5
---
 xen/arch/arm/vpci.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Julien Grall Oct. 13, 2023, 8:34 a.m. UTC | #1

Hi Volodymyr,

On 12/10/2023 23:09, Volodymyr Babchuk wrote:
> From: Oleksandr Andrushchenko <[e-mail]>
> 
> At the moment, we always allocate an extra 16 slots for IO handlers
> (see MAX_IO_HANDLER). So while adding IO trap handlers for the emulated
> MSI-X registers we need to explicitly tell that we have additional IO
> handlers, so those are accounted.
> 
> Signed-off-by: Oleksandr Andrushchenko <[e-mail]>

Some process remark. All the patches you send (even if they are 
unmodified) should also contain your signed-off-by. This is to comply 
with point (b) in the DCO certificate:

https://cert-manager.io/docs/contributing/sign-off/

Please check the other patches in this series.

> Acked-by: Julien Grall <[e-mail]>

Is this patch depends on the rest of the series? If not we can merge it 
in the for-4.19 branch Stefano created. This will reduce the number of 
patches you need to resend.

Cheers,

Volodymyr Babchuk Oct. 13, 2023, 1:06 p.m. UTC | #2

Hi Julien,

Julien Grall <[e-mail]> writes:

> Hi Volodymyr,
>
> On 12/10/2023 23:09, Volodymyr Babchuk wrote:
>> From: Oleksandr Andrushchenko <[e-mail]>
>> At the moment, we always allocate an extra 16 slots for IO handlers
>> (see MAX_IO_HANDLER). So while adding IO trap handlers for the emulated
>> MSI-X registers we need to explicitly tell that we have additional IO
>> handlers, so those are accounted.
>> Signed-off-by: Oleksandr Andrushchenko
>> <[e-mail]>
>
> Some process remark. All the patches you send (even if they are
> unmodified) should also contain your signed-off-by. This is to comply
> with point (b) in the DCO certificate:

Oh, sorry. I assumed that it is enough to have signed-off-by tag of the
original author. I'll add my tags in the next version.

> https://urldefense.com/v3/__https://cert-manager.io/docs/contributing/sign-off/__;!!GF_29dbcQIUBPA!0mzdEfHOZMm2OmzFc6TZukGgMYRHxDWLdEQvbhUlDmOg3tZNeDbWb8vHz38zLzcYv8GUZeHLn-5sWTYCkvkb$ [cert-manager[.]io]
>
> Please check the other patches in this series.
>
>> Acked-by: Julien Grall <[e-mail]>
>
> Is this patch depends on the rest of the series? If not we can merge
> it in the for-4.19 branch Stefano created. This will reduce the number
> of patches you need to resend.

It uses VPCI_MAX_VIRT_DEV constant which was introduced in ("vpci: add
initial support for virtual PCI bus topology").

Julien Grall Oct. 13, 2023, 4:46 p.m. UTC | #3

Hi,

On 13/10/2023 14:06, Volodymyr Babchuk wrote:
> 
> Hi Julien,
> 
> Julien Grall <[e-mail]> writes:
> 
>> Hi Volodymyr,
>>
>> On 12/10/2023 23:09, Volodymyr Babchuk wrote:
>>> From: Oleksandr Andrushchenko <[e-mail]>
>>> At the moment, we always allocate an extra 16 slots for IO handlers
>>> (see MAX_IO_HANDLER). So while adding IO trap handlers for the emulated
>>> MSI-X registers we need to explicitly tell that we have additional IO
>>> handlers, so those are accounted.
>>> Signed-off-by: Oleksandr Andrushchenko
>>> <[e-mail]>
>>
>> Some process remark. All the patches you send (even if they are
>> unmodified) should also contain your signed-off-by. This is to comply
>> with point (b) in the DCO certificate:
> 
> Oh, sorry. I assumed that it is enough to have signed-off-by tag of the
> original author. I'll add my tags in the next version.

Thanks!

> 
>> https://urldefense.com/v3/__https://cert-manager.io/docs/contributing/sign-off/__;!!GF_29dbcQIUBPA!0mzdEfHOZMm2OmzFc6TZukGgMYRHxDWLdEQvbhUlDmOg3tZNeDbWb8vHz38zLzcYv8GUZeHLn-5sWTYCkvkb$ [cert-manager[.]io]
>>
>> Please check the other patches in this series.
>>
>>> Acked-by: Julien Grall <[e-mail]>
>>
>> Is this patch depends on the rest of the series? If not we can merge
>> it in the for-4.19 branch Stefano created. This will reduce the number
>> of patches you need to resend.
> 
> It uses VPCI_MAX_VIRT_DEV constant which was introduced in ("vpci: add
> initial support for virtual PCI bus topology").

Ok. I will wait before committing. Please let me know if there are any 
Arm patches that can be already committed (or could potentially be 
reviewed independently).

Cheers,

Volodymyr Babchuk Oct. 13, 2023, 5:17 p.m. UTC | #4

Julien,

Julien Grall <[e-mail]> writes:

> Hi,
>
> On 13/10/2023 14:06, Volodymyr Babchuk wrote:
>> Hi Julien,
>> Julien Grall <[e-mail]> writes:
>> 
>>> Hi Volodymyr,
>>>
>>> On 12/10/2023 23:09, Volodymyr Babchuk wrote:
>>>> From: Oleksandr Andrushchenko <[e-mail]>
>>>> At the moment, we always allocate an extra 16 slots for IO handlers
>>>> (see MAX_IO_HANDLER). So while adding IO trap handlers for the emulated
>>>> MSI-X registers we need to explicitly tell that we have additional IO
>>>> handlers, so those are accounted.
>>>> Signed-off-by: Oleksandr Andrushchenko
>>>> <[e-mail]>
>>>
>>> Some process remark. All the patches you send (even if they are
>>> unmodified) should also contain your signed-off-by. This is to comply
>>> with point (b) in the DCO certificate:
>> Oh, sorry. I assumed that it is enough to have signed-off-by tag of
>> the
>> original author. I'll add my tags in the next version.
>
> Thanks!
>
>> 
>>> https://urldefense.com/v3/__https://cert-manager.io/docs/contributing/sign-off/__;!!GF_29dbcQIUBPA!0mzdEfHOZMm2OmzFc6TZukGgMYRHxDWLdEQvbhUlDmOg3tZNeDbWb8vHz38zLzcYv8GUZeHLn-5sWTYCkvkb$ [cert-manager[.]io]
>>>
>>> Please check the other patches in this series.
>>>
>>>> Acked-by: Julien Grall <[e-mail]>
>>>
>>> Is this patch depends on the rest of the series? If not we can merge
>>> it in the for-4.19 branch Stefano created. This will reduce the number
>>> of patches you need to resend.
>> It uses VPCI_MAX_VIRT_DEV constant which was introduced in ("vpci:
>> add
>> initial support for virtual PCI bus topology").
>
> Ok. I will wait before committing. Please let me know if there are any
> Arm patches that can be already committed (or could potentially be
> reviewed independently).

Well, there is the ("arm/vpci: honor access size when returning an
error") which can be applied partially to the current staging branch.

Maybe it is a good idea to publish it separately.

Patch

diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 58e2a20135..01b50d435e 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -134,6 +134,8 @@  static int vpci_get_num_handlers_cb(struct domain *d,
 
 unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
 {
+    unsigned int count;
+
     if ( !has_vpci(d) )
         return 0;
 
@@ -154,7 +156,17 @@  unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
      * For guests each host bridge requires one region to cover the
      * configuration space. At the moment, we only expose a single host bridge.
      */
-    return 1;
+    count = 1;
+
+    /*
+     * There's a single MSI-X MMIO handler that deals with both PBA
+     * and MSI-X tables per each PCI device being passed through.
+     * Maximum number of emulated virtual devices is VPCI_MAX_VIRT_DEV.
+     */
+    if ( IS_ENABLED(CONFIG_HAS_PCI_MSI) )
+        count += VPCI_MAX_VIRT_DEV;
+
+    return count;
 }
 
 /*