[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v8 5/8] xen/arm: assign devices to boot domains



Hi Stefano,

A couple of comments below, mostly because I wasn't clear enough on the previous version. I am not sure if it is worth resending the series, maybe just resending this one would be sufficient?

On 03/10/2019 02:35, Stefano Stabellini wrote:
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 84b65b8f25..b90902ad97 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1714,6 +1714,88 @@ static int __init make_vpl011_uart_node(struct 
kernel_info *kinfo)
  }
  #endif
+/*
+ * Scan device tree properties for passthrough specific information.
+ * Returns < 0 on error
+ *         0 on success
+ */
+static int __init handle_passthrough_prop(struct kernel_info *kinfo,
+                                          const struct fdt_property *xen_reg,
+                                          const struct fdt_property *xen_path,
+                                          bool xen_force,
+                                          uint32_t address_cells, uint32_t 
size_cells)
+{
+    const __be32 *cell;
+    unsigned int i, len;
+    struct dt_device_node *node;
+    int res;
+    paddr_t mstart, size, gstart;
+
+    /* xen,reg specifies where to map the MMIO region */
+    cell = (const __be32 *)xen_reg->data;
+    len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
+                                        sizeof(uint32_t));

Apologies for this, I misread you previous code. I am happy with this version or the previous one.

[...]

+    /*
+     * Only handle passthrough properties if both xen,reg and xen,path
+     * are present, or if xen,force-assign-without-iommu is specified.
+     */
+    if ( xen_reg != NULL && (xen_path != NULL || xen_force) )
+    {
+        res = handle_passthrough_prop(kinfo, xen_reg, xen_path, xen_force,
+                                      address_cells, size_cells);
+        if ( res < 0 )
+        {
+            printk(XENLOG_ERR "Failed to assign device to %pd\n", kinfo->d);

This is not the error path I meant.

The one I was referring is the case where xen,path is present but not xen,reg. At the moment you will continue without telling the user. We should at least print a warning and probably return an error as someone specifying "xen,path" may expect to assign the device.

Cheers,


--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.