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

[Brandon Perez <a0225893@xxxxxx>]

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