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

Re: [Xen-devel] [PATCH] xen: arm: X-Gene Storm check GIC DIST address for EOI quirk



Hi Stefano,

On 07/04/2015 10:40, Stefano Stabellini wrote:
On Mon, 6 Apr 2015, Julien Grall wrote:
Hi Pranav,

Thank you for the patch.

On 06/04/2015 10:54, Pranavkumar Sawargaonkar wrote:
In old X-Gene Storm firmware and DT, secure mode addresses have been
mentioned in GICv2 node. In this case maintenance interrupt is used
instead of EOI HW method.

This patch checks the GIC Distributor Base Address to enable EOI quirk
for old firmware.

Ref:
http://lists.xen.org/archives/html/xen-devel/2014-07/msg01263.html

Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>
---
   xen/arch/arm/platforms/xgene-storm.c |   37
+++++++++++++++++++++++++++++++++-
   1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/platforms/xgene-storm.c
b/xen/arch/arm/platforms/xgene-storm.c
index eee650e..dd7cbfc 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -22,6 +22,7 @@
   #include <asm/platform.h>
   #include <xen/stdbool.h>
   #include <xen/vmap.h>
+#include <xen/device_tree.h>
   #include <asm/io.h>
   #include <asm/gic.h>

@@ -35,9 +36,41 @@ static u64 reset_addr, reset_size;
   static u32 reset_mask;
   static bool reset_vals_valid = false;

+#define XGENE_SEC_GICV2_DIST_ADDR    0x78010000
+static u32 quirk_guest_pirq_need_eoi;

This variable will mostly be read. So, I would add __read_mostly.

+
+static void xgene_check_pirq_eoi(void)

If I'm not mistaken, this function is only called during Xen initialization.
So, I would add __init.

+{
+    struct dt_device_node *node;
+    int res;
+    paddr_t dbase;
+
+    dt_for_each_device_node( dt_host, node )
+    {

It would be better to create a new callback for platform specific GIC
initialization and use dt_interrupt_controller.

This would avoid to have this loop and rely on there is always only one
interrupt controller in the DT.

That is true, however we do know that on this SoC there is only one GIC,
so it might be acceptable to rely on this knowledge: this is already
very platform specific code.

If we keep the loop, I would add a comment on the above the loop explaining that we rely on there is always only GIC.

Although, I was wondering if we could simply rely on the node path to get the DT node?

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