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

Re: [Xen-devel] [Xen-Devel] Enabling IRQ Crossbar (Secondary Interrupt Controller) Support



On 07/21/2015 05:41 AM, Julien Grall wrote:


On 21/07/2015 00:17, Brandon Perez wrote:
Hello All,

Hi Brandon,

We use to send one mail by patch rather than sending them as an
attachment of a single email. It's easier for reviewing the patches.
You also need to add you Signed-off-by on each patch and CC all the
relevant maintainers. Please see [1] for all the guidelines to submit a
patch to Xen.

A couple of comments I about this series:
     - Patch #2: You are allowing any guest to do smc which, unless you
trust all the guest, is unsecure. There was some discussion about
different solution to handle SMC back in 2013 [2]. So far I didn't see
any more update on it. It may be worth to send a separate thread about
how to handle SMC.
     - Patch #3, I can't find any documentation or implementation of the
property "default-mapping" in Linux. Can you provide a link about it?

I will comment more when you will resend the patches inline.

Regards,

[1] http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches
[2] http://lists.xen.org/archives/html/xen-devel/2013-07/msg02779.html


Hi Julien,

I'm not sure that these patches are quite ready yet to be put into the Xen repo. For one, these patches don't solve the problem of the crossbar as outlined in the thread this came from [1]. Also, I haven't had a chance to clean up these patches yet. I've provided these patches at the request of Ian, so that you guys could see the changes I have made so far, and we could discuss what changes would be needed for the crossbar.

I agree about Patch #2, that makes sense, this was a workaround I've been using for now. Perhaps a check could be added to see if the domain is the privileged domain?

That's correct, the "default-mapping" property is not standard. It's another workaround that I'm working with for now. The interrupts property is going to contain the crossbar input number, not the actual SPI GIC line, so I needed a way to convey this to Xen.

    The patches are inlined below:

Patch #1:

From f2bf190255c8f872d15063d7f8a6382c279e312d Mon Sep 17 00:00:00 2001
From: Brandon Perez <bperez-1@xxxxxx>
Date: Mon, 20 Jul 2015 17:56:49 -0400
Subject: DRA7: Add specific mappings for devices/regions not in the device tree.

The DRA7 chip, which is similar to the OMAP5 chip, also requires specific mappings. These are MMIO mappings which are not explicitly stated in the device tree, so Xen does not know to map them. This patch adds these regions required by the DRA7 to be mapped.

Signed-off-by: Brandon Perez <bperez-1@xxxxxx>

---
 xen/arch/arm/platforms/omap5.c        |   27 +++++++++++++++++++++++++++
 xen/include/asm-arm/platforms/omap5.h |    3 +++
 2 files changed, 30 insertions(+)

diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
index e7bf30d..3c6495a 100644
--- a/xen/arch/arm/platforms/omap5.c
+++ b/xen/arch/arm/platforms/omap5.c
@@ -120,6 +120,32 @@ static int omap5_specific_mapping(struct domain *d)
     return 0;
 }

+/* Additional mappings for dom0 (not in the DTS) */
+static int dra7_specific_mapping(struct domain *d)
+{
+    /* Map the PRM module */
+    map_mmio_regions(d, paddr_to_pfn(OMAP5_PRM_BASE), 2,
+                     paddr_to_pfn(OMAP5_PRM_BASE));
+
+    /* Map the PRM_MPU */
+    map_mmio_regions(d, paddr_to_pfn(OMAP5_PRCM_MPU_BASE), 1,
+                     paddr_to_pfn(OMAP5_PRCM_MPU_BASE));
+
+    /* Map the Wakeup Gen */
+    map_mmio_regions(d, paddr_to_pfn(OMAP5_WKUPGEN_BASE), 1,
+                     paddr_to_pfn(OMAP5_WKUPGEN_BASE));
+
+    /* Map the on-chip SRAM */
+    map_mmio_regions(d, paddr_to_pfn(OMAP5_SRAM_PA), 32,
+                     paddr_to_pfn(OMAP5_SRAM_PA));
+
+    /* Map GPMC address space for NAND flash. */
+    map_mmio_regions(d, paddr_to_pfn(OMAP5_GPMC_PA), 65536,
+                     paddr_to_pfn(OMAP5_GPMC_PA));
+
+    return 0;
+}
+
 static int __init omap5_smp_init(void)
 {
     void __iomem *wugen_base;
@@ -171,6 +197,7 @@ PLATFORM_START(dra7, "TI DRA7")
     .init_time = omap5_init_time,
     .cpu_up = cpu_up_send_sgi,
     .smp_init = omap5_smp_init,
+    .specific_mapping = dra7_specific_mapping,

     .dom0_gnttab_start = 0x4b000000,
     .dom0_gnttab_size = 0x20000,
diff --git a/xen/include/asm-arm/platforms/omap5.h b/xen/include/asm-arm/platforms/omap5.h
index c559c84..d87e7d2 100644
--- a/xen/include/asm-arm/platforms/omap5.h
+++ b/xen/include/asm-arm/platforms/omap5.h
@@ -20,6 +20,9 @@
 #define OMAP_AUX_CORE_BOOT_0_OFFSET             0x800
 #define OMAP_AUX_CORE_BOOT_1_OFFSET             0x804

+#define OMAP5_GPMC_PA                           0x01000000
+#define OMAP5_TILER_PA                          0x60000000
+
 #endif /* __ASM_ARM_PLATFORMS_OMAP5_H */

 /*
--
1.7.9.5

Patch #2:

From e53fdc1ea750dd3143e2d7cd62a5d38eb446afde Mon Sep 17 00:00:00 2001
From: Brandon Perez <bperez-1@xxxxxx>
Date: Mon, 20 Jul 2015 17:58:24 -0400
Subject: Traps: Enable pass-through SMC call support for guest OS's.

Originally, Xen did not allow for guests to make SMC calls. However, on the DRA7 chips, the kernel needs to make several SMC calls to interact with the secure ROM code.

There are two solutions for solving this in the patch. The selected method is to simply allow the kernel to make the call, without going through the hypervisor. The other is to trap and emulate the call in the hypervisor.

Signed-off-by: Brandon Perez <bperez-1@xxxxxx>

---
 xen/arch/arm/traps.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 258d4c5..9b9de7b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -123,8 +123,9 @@ void __cpuinit init_traps(void)
                  CPTR_EL2);

     /* Setup hypervisor traps */
+    // TODO: Choose method
     WRITE_SYSREG(HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
- HCR_TWE|HCR_TWI|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP, HCR_EL2); + HCR_TWE|HCR_TWI/*|HCR_TSC*/|HCR_TAC|HCR_SWIO|HCR_TIDCP, HCR_EL2);
     isb();
 }

@@ -2494,6 +2495,18 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
         GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_smc32);
         inject_undef32_exception(regs);
+// TODO: Choose method
+/*#define omap5_smc(func_id, arg1) \
+        asm volatile ("push {r1-r12, lr}\n\t" \
+                      "mov r12,%0\n\t" \
+                      "mov r0,%1\n\t" \
+                      "smc #1\n\t" \
+                      "pop {r1-r12, lr}" \
+                      : \
+                      : "r" (func_id), "r" (arg1))
+
+        omap5_smc(regs->r12, regs->r0);
+        advance_pc(regs, hsr); */
         break;
     case HSR_EC_HVC32:
         GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
--
1.7.9.5

Patch #3:

From 03f444d9042b1730bf3d8ff9ab3369e744e5e69b Mon Sep 17 00:00:00 2001
From: Brandon Perez <bperez-1@xxxxxx>
Date: Mon, 20 Jul 2015 18:04:36 -0400
Subject: ARM/IRQ-Crossbar: Make Xen aware of devices being statically mapped in the IRQ crossbar.

In general, Xen needs to own very few interrupts to run. Most of these are timer PPIs, which do not involve the crossbar. The notable exception to this is the serial device, which is an SPI. On the DRA7 chips, this involves going through the interrupt crossbar.

As the device tree entry will contain the crossbar input number, and not the IRQ line, Xen must be given the SPI input line number. This is achieved with the "default-mapping" property, which contains the SPI number that the device will correspond to in the mapping setup by the bootloader.

Signed-off-by: Brandon Perez <bperez-1@xxxxxx>

---
 xen/common/device_tree.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 31f169b..fc82eb4 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -1036,10 +1036,13 @@ int dt_device_get_raw_irq(const struct dt_device_node *device,
     dt_dprintk("dt_device_get_raw_irq: dev=%s, index=%u\n",
                device->full_name, index);

-    /* Get the interrupts property */
-    intspec = dt_get_property(device, "interrupts", &intlen);
-    if ( intspec == NULL )
-        return -EINVAL;
+    /* Get the appropiate interrupts property */
+    intspec = dt_get_property(device, "default-mapping", &intlen);
+    if (intspec == NULL) {
+        intspec = dt_get_property(device, "interrupts", &intlen);
+        if (intspec == NULL)
+            return -EINVAL;
+    }
     intlen /= sizeof(*intspec);

     dt_dprintk(" intspec=%d intlen=%d\n", be32_to_cpup(intspec), intlen);
--
1.7.9.5


Brandon Perez

_______________________________________________
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®.