Mailing List Archive: [RFC PATCH V1 09/12] libxl: Handle virtio-mmio irq in more correct way

[RFC PATCH V1 09/12] libxl: Handle virtio-mmio irq in more correct way

Aug 3, 2020, 11:21 AM

Post #1 of 3 (57 views)

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

This patch makes possible to use device passthrough again.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
tools/libxl/libxl_arm.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 620b499..4f748e3 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -9,6 +9,10 @@
#include <assert.h>
#include <xen/device_tree_defs.h>

+#define GUEST_VIRTIO_MMIO_BASE xen_mk_ullong(0x02000000)
+#define GUEST_VIRTIO_MMIO_SIZE xen_mk_ullong(0x200)
+#define GUEST_VIRTIO_MMIO_SPI 33
+
static const char *gicv_to_string(libxl_gic_version gic_version)
{
switch (gic_version) {
@@ -27,8 +31,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
{
uint32_t nr_spis = 0;
unsigned int i;
- uint32_t vuart_irq;
- bool vuart_enabled = false;
+ uint32_t vuart_irq, virtio_irq;
+ bool vuart_enabled = false, virtio_enabled = false;

/*
* If pl011 vuart is enabled then increment the nr_spis to allow allocation
@@ -40,6 +44,17 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
vuart_enabled = true;
}

+ /*
+ * XXX: Handle properly virtio
+ * A proper solution would be the toolstack to allocate the interrupts
+ * used by each virtio backend and let the backend now which one is used
+ */
+ if (libxl_defbool_val(d_config->b_info.arch_arm.virtio)) {
+ nr_spis += (GUEST_VIRTIO_MMIO_SPI - 32) + 1;
+ virtio_irq = GUEST_VIRTIO_MMIO_SPI;
+ virtio_enabled = true;
+ }
+
for (i = 0; i < d_config->b_info.num_irqs; i++) {
uint32_t irq = d_config->b_info.irqs[i];
uint32_t spi;
@@ -59,6 +74,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
return ERROR_FAIL;
}

+ /* The same check as for vpl011 */
+ if (virtio_enabled && irq == virtio_irq) {
+ LOG(ERROR, "Physical IRQ %u conflicting with virtio SPI\n", irq);
+ return ERROR_FAIL;
+ }
+
if (irq < 32)
continue;

@@ -68,10 +89,6 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
nr_spis = spi + 1;
}

-
- /* XXX: Handle properly virtio */
- nr_spis = 1;
-
LOG(DEBUG, "Configure the domain");

config->arch.nr_spis = nr_spis;
@@ -663,10 +680,6 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
return 0;
}

-#define GUEST_VIRTIO_MMIO_BASE xen_mk_ullong(0x02000000)
-#define GUEST_VIRTIO_MMIO_SIZE xen_mk_ullong(0x200)
-#define GUEST_VIRTIO_MMIO_SPI 33
-
static int make_virtio_mmio_node(libxl__gc *gc, void *fdt)
{
int res;
--
2.7.4

Re: [RFC PATCH V1 09/12] libxl: Handle virtio-mmio irq in more correct way [ In reply to ]

sstabellini at kernel

Aug 4, 2020, 4:22 PM

Post #2 of 3 (56 views)

Permalink

On Mon, 3 Aug 2020, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> This patch makes possible to use device passthrough again.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
> tools/libxl/libxl_arm.c | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 620b499..4f748e3 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -9,6 +9,10 @@
> #include <assert.h>
> #include <xen/device_tree_defs.h>
>
> +#define GUEST_VIRTIO_MMIO_BASE xen_mk_ullong(0x02000000)
> +#define GUEST_VIRTIO_MMIO_SIZE xen_mk_ullong(0x200)
> +#define GUEST_VIRTIO_MMIO_SPI 33

They should be in xen/include/public/arch-arm.h

Is one interrupt enough if there are multiple virtio devices? Is it one
interrupt for all virtio devices, or one for each device?

Of course this patch should be folded in the patch to add virtio support
to libxl.

> static const char *gicv_to_string(libxl_gic_version gic_version)
> {
> switch (gic_version) {
> @@ -27,8 +31,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> {
> uint32_t nr_spis = 0;
> unsigned int i;
> - uint32_t vuart_irq;
> - bool vuart_enabled = false;
> + uint32_t vuart_irq, virtio_irq;
> + bool vuart_enabled = false, virtio_enabled = false;
>
> /*
> * If pl011 vuart is enabled then increment the nr_spis to allow allocation
> @@ -40,6 +44,17 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> vuart_enabled = true;
> }
>
> + /*
> + * XXX: Handle properly virtio
> + * A proper solution would be the toolstack to allocate the interrupts
> + * used by each virtio backend and let the backend now which one is used
> + */
> + if (libxl_defbool_val(d_config->b_info.arch_arm.virtio)) {
> + nr_spis += (GUEST_VIRTIO_MMIO_SPI - 32) + 1;
> + virtio_irq = GUEST_VIRTIO_MMIO_SPI;
> + virtio_enabled = true;
> + }
> +
> for (i = 0; i < d_config->b_info.num_irqs; i++) {
> uint32_t irq = d_config->b_info.irqs[i];
> uint32_t spi;
> @@ -59,6 +74,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> return ERROR_FAIL;
> }
>
> + /* The same check as for vpl011 */
> + if (virtio_enabled && irq == virtio_irq) {
> + LOG(ERROR, "Physical IRQ %u conflicting with virtio SPI\n", irq);
> + return ERROR_FAIL;
> + }
> +
> if (irq < 32)
> continue;
>
> @@ -68,10 +89,6 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> nr_spis = spi + 1;
> }
>
> -
> - /* XXX: Handle properly virtio */
> - nr_spis = 1;
> -
> LOG(DEBUG, "Configure the domain");
>
> config->arch.nr_spis = nr_spis;
> @@ -663,10 +680,6 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
> return 0;
> }
>
> -#define GUEST_VIRTIO_MMIO_BASE xen_mk_ullong(0x02000000)
> -#define GUEST_VIRTIO_MMIO_SIZE xen_mk_ullong(0x200)
> -#define GUEST_VIRTIO_MMIO_SPI 33
> -
> static int make_virtio_mmio_node(libxl__gc *gc, void *fdt)
> {
> int res;
> --
> 2.7.4
>

Re: [RFC PATCH V1 09/12] libxl: Handle virtio-mmio irq in more correct way [ In reply to ]

olekstysh at gmail

Aug 5, 2020, 1:51 PM

Post #3 of 3 (56 views)

Permalink

On 05.08.20 02:22, Stefano Stabellini wrote:

Hi Stefano

> On Mon, 3 Aug 2020, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> This patch makes possible to use device passthrough again.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> ---
>> tools/libxl/libxl_arm.c | 33 +++++++++++++++++++++++----------
>> 1 file changed, 23 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index 620b499..4f748e3 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -9,6 +9,10 @@
>> #include <assert.h>
>> #include <xen/device_tree_defs.h>
>>
>> +#define GUEST_VIRTIO_MMIO_BASE xen_mk_ullong(0x02000000)
>> +#define GUEST_VIRTIO_MMIO_SIZE xen_mk_ullong(0x200)
>> +#define GUEST_VIRTIO_MMIO_SPI 33
> They should be in xen/include/public/arch-arm.h

ok

>
> Is one interrupt enough if there are multiple virtio devices? Is it one
> interrupt for all virtio devices, or one for each device?

One interrupt for each virtio device. I experimented with current
series and assigned 4 disk partitions to the guest. This resulted in 4
separate device-tree nodes, and each node had individual SPI and MMIO range.

>
> Of course this patch should be folded in the patch to add virtio support
> to libxl.

ok

--
Regards,

Oleksandr Tyshchenko