[RFC PATCH] libxl/arm: Insert "xen,dev-domid" property to virtio-mmio device node

[RFC PATCH] libxl/arm: Insert "xen,dev-domid" property to virtio-mmio device node

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

This is needed for grant table based DMA ops layer (CONFIG_XEN_VIRTIO)
at the guest side to retrieve the ID of Xen domain where the corresponding
backend resides (it is used as an argument to the grant table APIs).

This is a part of restricted memory access under Xen feature.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
!!! This patch is based on non upstreamed yet “Virtio support for toolstack
on Arm” series which is on review now:
https://lore.kernel.org/xen-devel/1649442065-8332-1-git-send-email-olekstysh@gmail.com/

All details are at:
https://lore.kernel.org/xen-devel/1649963973-22879-1-git-send-email-olekstysh@gmail.com/
---
 tools/libs/light/libxl_arm.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 8132a47..d9b26fc 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -875,7 +875,8 @@ static int make_vpci_node(libxl__gc *gc, void *fdt,
 
 
 static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
-                                 uint64_t base, uint32_t irq)
+                                 uint64_t base, uint32_t irq,
+                                 uint32_t backend_domid)
 {
     int res;
     gic_interrupt intr;
@@ -900,6 +901,14 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
     res = fdt_property(fdt, "dma-coherent", NULL, 0);
     if (res) return res;
 
+    if (backend_domid != LIBXL_TOOLSTACK_DOMID) {
+        uint32_t domid[1];
+
+        domid[0] = cpu_to_fdt32(backend_domid);
+        res = fdt_property(fdt, "xen,dev-domid", domid, sizeof(domid));
+        if (res) return res;
+    }
+
     res = fdt_end_node(fdt);
     if (res) return res;
 
@@ -1218,7 +1227,8 @@ next_resize:
             libxl_device_disk *disk = &d_config->disks[i];
 
             if (disk->protocol == LIBXL_DISK_PROTOCOL_VIRTIO_MMIO)
-                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) );
+                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
+                                           disk->backend_domid) );
         }
 
         if (pfdt)
-- 
2.7.4

Re: [RFC PATCH] libxl/arm: Insert "xen,dev-domid" property to virtio-mmio device node

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]

On Thu, 14 Apr 2022, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> This is needed for grant table based DMA ops layer (CONFIG_XEN_VIRTIO)
> at the guest side to retrieve the ID of Xen domain where the corresponding
> backend resides (it is used as an argument to the grant table APIs).
> 
> This is a part of restricted memory access under Xen feature.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

I think this looks good overall. Instead of mentioning details of the
Linux implementation, we should mention the device tree binding instead,
including a link to it. The device tree binding is the relevant spec in
this case.


> ---
> !!! This patch is based on non upstreamed yet “Virtio support for toolstack
> on Arm” series which is on review now:
> https://lore.kernel.org/xen-devel/1649442065-8332-1-git-send-email-olekstysh@gmail.com/
> 
> All details are at:
> https://lore.kernel.org/xen-devel/1649963973-22879-1-git-send-email-olekstysh@gmail.com/
> ---
>  tools/libs/light/libxl_arm.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 8132a47..d9b26fc 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -875,7 +875,8 @@ static int make_vpci_node(libxl__gc *gc, void *fdt,
>  
>  
>  static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
> -                                 uint64_t base, uint32_t irq)
> +                                 uint64_t base, uint32_t irq,
> +                                 uint32_t backend_domid)
>  {
>      int res;
>      gic_interrupt intr;
> @@ -900,6 +901,14 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
>      res = fdt_property(fdt, "dma-coherent", NULL, 0);
>      if (res) return res;
>  
> +    if (backend_domid != LIBXL_TOOLSTACK_DOMID) {
> +        uint32_t domid[1];
> +
> +        domid[0] = cpu_to_fdt32(backend_domid);
> +        res = fdt_property(fdt, "xen,dev-domid", domid, sizeof(domid));
> +        if (res) return res;
> +    }
> +
>      res = fdt_end_node(fdt);
>      if (res) return res;
>  
> @@ -1218,7 +1227,8 @@ next_resize:
>              libxl_device_disk *disk = &d_config->disks[i];
>  
>              if (disk->protocol == LIBXL_DISK_PROTOCOL_VIRTIO_MMIO)
> -                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) );
> +                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
> +                                           disk->backend_domid) );
>          }
>  
>          if (pfdt)
> -- 
> 2.7.4
> 

Re: [RFC PATCH] libxl/arm: Insert "xen,dev-domid" property to virtio-mmio device node

[Oleksandr]

On 19.04.22 00:47, Stefano Stabellini wrote:


Hello Stefano

> On Thu, 14 Apr 2022, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> This is needed for grant table based DMA ops layer (CONFIG_XEN_VIRTIO)
>> at the guest side to retrieve the ID of Xen domain where the corresponding
>> backend resides (it is used as an argument to the grant table APIs).
>>
>> This is a part of restricted memory access under Xen feature.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> I think this looks good overall.

thank you


>   Instead of mentioning details of the
> Linux implementation, we should mention the device tree binding instead,
> including a link to it. The device tree binding is the relevant spec in
> this case.

I got it, while new device tree binding is not approved yet, I should 
have inserted a link to the corresponding Linux commit.

https://lore.kernel.org/xen-devel/1649963973-22879-4-git-send-email-olekstysh@gmail.com/


>
>
>> ---
>> !!! This patch is based on non upstreamed yet “Virtio support for toolstack
>> on Arm” series which is on review now:
>> https://lore.kernel.org/xen-devel/1649442065-8332-1-git-send-email-olekstysh@gmail.com/
>>
>> All details are at:
>> https://lore.kernel.org/xen-devel/1649963973-22879-1-git-send-email-olekstysh@gmail.com/
>> ---
>>   tools/libs/light/libxl_arm.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
>> index 8132a47..d9b26fc 100644
>> --- a/tools/libs/light/libxl_arm.c
>> +++ b/tools/libs/light/libxl_arm.c
>> @@ -875,7 +875,8 @@ static int make_vpci_node(libxl__gc *gc, void *fdt,
>>   
>>   
>>   static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
>> -                                 uint64_t base, uint32_t irq)
>> +                                 uint64_t base, uint32_t irq,
>> +                                 uint32_t backend_domid)
>>   {
>>       int res;
>>       gic_interrupt intr;
>> @@ -900,6 +901,14 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
>>       res = fdt_property(fdt, "dma-coherent", NULL, 0);
>>       if (res) return res;
>>   
>> +    if (backend_domid != LIBXL_TOOLSTACK_DOMID) {
>> +        uint32_t domid[1];
>> +
>> +        domid[0] = cpu_to_fdt32(backend_domid);
>> +        res = fdt_property(fdt, "xen,dev-domid", domid, sizeof(domid));
>> +        if (res) return res;
>> +    }
>> +
>>       res = fdt_end_node(fdt);
>>       if (res) return res;
>>   
>> @@ -1218,7 +1227,8 @@ next_resize:
>>               libxl_device_disk *disk = &d_config->disks[i];
>>   
>>               if (disk->protocol == LIBXL_DISK_PROTOCOL_VIRTIO_MMIO)
>> -                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) );
>> +                FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
>> +                                           disk->backend_domid) );
>>           }
>>   
>>           if (pfdt)
>> -- 
>> 2.7.4

-- 
Regards,

Oleksandr Tyshchenko