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

Re: [Xen-devel] [PATCH v2] xen/arm: gicv2: Export GICv2m register frames to domain0 by device tree



Hello Wei,

On 25/04/2016 10:39, Wei Chen wrote:
This patch adds v2m extension support in GIC-v2 driver. The GICv2 driver
detects the MSI frames from device tree and creates corresponding device
tree nodes in Domain0's DTB. It also provides one hw_ops callback to map

NIT: s/Domain0/dom0/

v2m MMIO regions to domain0 and route v2m SPIs to domain0.

Ditto.


With this GICv2m extension support, the domain0 kernel can do GICv2m

Ditto

frame setup and initialization.

This patch is based on the GICv2m patch of Suravee Suthikulpanit:
[PATCH 2/2] xen/arm: gicv2: Adding support for GICv2m in Dom0
http://lists.xen.org/archives/html/xen-devel/2015-04/msg02613.html

Signed-off-by: Wei Chen <Wei.Chen@xxxxxxxxxx>

[...]

+static int gicv2_map_hwdown_extra_mappings(struct domain *d)
+{
+    const struct v2m_data *v2m_data;
+
+    /* For the moment, we'll assign all v2m frames to the hardware domain. */
+    list_for_each_entry( v2m_data, &gicv2m_info, entry )
+    {
+        int ret;
+        u32 spi;
+
+        printk("GICv2: Mapping v2m frame to d%d: addr=0x%lx size=0x%lx spi_base=%u 
num_spis=%u\n",
+               d->domain_id, v2m_data->addr, v2m_data->size,
+               v2m_data->spi_start, v2m_data->nr_spis);
+
+        ret = map_mmio_regions(d, paddr_to_pfn(v2m_data->addr),
+                            DIV_ROUND_UP(v2m_data->size, PAGE_SIZE),
+                            paddr_to_pfn(v2m_data->addr));
+        if ( ret )
+        {
+            printk(XENLOG_ERR "GICv2: Map v2m frame to s%d failed.\n",

s/s%d/d%d/

+                   d->domain_id);
+            return ret;
+        }
+
+        /*
+         * Map all SPIs that are allocated to MSIs for the frame to the
+         * domain.
+         */
+        for ( spi = v2m_data->spi_start;
+              spi < (v2m_data->spi_start + v2m_data->nr_spis); spi++ )
+        {
+            /*
+             * MSIs are always edge-triggered. Configure the associated SPIs
+             * to be edge-rising.

How did you find that SPIs should be configured edge-rising?

+             */
+            ret = irq_set_spi_type(spi, IRQ_TYPE_EDGE_RISING);
+            if ( ret )
+            {
+                printk(XENLOG_ERR
+                       "GICv2: Failed to set v2m MSI SPI[%d] type.\n", spi);
+                return ret;
+            }
+
+            /* Route a SPI that is allocated to MSI to the domain. */
+            ret = route_irq_to_guest(d, spi, spi, "v2m");
+            if ( ret )
+            {
+                printk(XENLOG_ERR
+                       "GICv2: Failed to route v2m MSI SPI[%d] to Dom%d.\n",
+                        spi, d->domain_id);
+                return ret;
+            }
+
+            /* Reserve a SPI that is allocated to MSI for the domain. */
+            if ( !vgic_reserve_virq(d, spi) )
+            {
+                printk(XENLOG_ERR
+                       "GICv2: Failed to reserve v2m MSI SPI[%d] for Dom%d.\n",
+                        spi, d->domain_id);
+                return -EINVAL;
+            }
+        }
+    }
+
+    return 0;
+}
+
+/*
+ * Set up gic v2m DT sub-node.
+ * Please refer to the binding document:
+ * 
https://www.kernel.org/doc/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
+ */
+static int gicv2m_make_dt_node(const struct domain *d,
+                            const struct dt_device_node *gic,
+                            void *fdt)

The indentation is still wrong here.

The start of the second and third lines should be aligned to "const..." on the first line. I.e

foo(const
____const
____void

Where _ is the empty space.

[...]

+static void gicv2_add_v2m_frame_to_list(paddr_t addr, paddr_t size,
+                                        u32 spi_start, u32 nr_spis,
+                                        const struct dt_device_node *v2m)
+{
+    struct v2m_data *v2m_data;
+
+    v2m_data = xzalloc_bytes(sizeof(struct v2m_data));
+    if ( !v2m_data )
+        panic("GICv2: Cannot allocate memory for v2m frame");
+
+    /* Initialize a new entry to record new frame infomation. */

s/infomation/information/

+    INIT_LIST_HEAD(&v2m_data->entry);
+    v2m_data->addr = addr;
+    v2m_data->size = size;
+    v2m_data->spi_start = spi_start;
+    v2m_data->nr_spis = nr_spis;
+    v2m_data->dt_node = v2m;
+
+    printk("GICv2m extension register frame:\n"
+           "        gic_v2m_addr=%"PRIpaddr"\n"
+           "        gic_v2m_size=%"PRIpaddr"\n"
+           "        gic_v2m_spi_base=%u\n"
+           "        gic_v2m_num_spis=%u\n",
+           v2m_data->addr, v2m_data->size,
+           v2m_data->spi_start, v2m_data->nr_spis);
+
+    list_add_tail(&v2m_data->entry, &gicv2m_info);
+}
+
+static void gicv2_extension_dt_init(const struct dt_device_node *node)
+{
+    int res;
+    u32 spi_start, nr_spis;
+    paddr_t addr, size;
+    const struct dt_device_node *v2m = NULL;
+
+    /*
+     * Check whether this GIC implements the v2m extension. If so,
+     * add v2m register frames to gicv2m_info.
+     */
+    dt_for_each_child_node(node, v2m)
+    {
+        if ( !dt_device_is_compatible(v2m, "arm,gic-v2m-frame") )
+            continue;
+
+        /*
+         * Check whether DT uses msi-base-spi and msi-num-spis properties to
+         * override the hardware setting.
+         */
+        if ( !dt_property_read_u32(v2m, "arm,msi-base-spi", &spi_start) ||
+             !dt_property_read_u32(v2m, "arm,msi-num-spis", &nr_spis) )
+        {
+            u32 msi_typer;
+            void __iomem *base;
+
+            /*
+            * DT does not override the hardware setting, so we have to read
+            * base_spi and num_spis from hardware registers to reserve irqs.
+            */

The indentation of the comments if wrong.

It should be:

/*
 * FOo
 * Bar
 */

+            res = dt_device_get_address(v2m, 0, &addr, &size);
+            if ( res )
+                panic("GICv2: Cannot find a valid v2m frame address");
+
+            base = ioremap_nocache(addr, size);
+            if ( !base )
+                panic("GICv2: Cannot remap v2m register frame");
+
+            msi_typer = readl_relaxed(base + V2M_MSI_TYPER);
+            spi_start = V2M_MSI_TYPER_BASE_SPI(msi_typer);
+            nr_spis = V2M_MSI_TYPER_NUM_SPI(msi_typer);

The ACPI code will likely need to read those registers. So this could go in the new function you have added.

You could pass pointers to your function to know whether the values are overridden by the firmware.

+
+            iounmap(base);
+        }
+
+        /* Add this v2m frame information to list. */
+        gicv2_add_v2m_frame_to_list(addr, size, spi_start, nr_spis, v2m);
+    }
+}
+

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

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