Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-09 Thread Olof Johansson
On Wed, Dec 04, 2013 at 03:50:17PM +, Marc Zyngier wrote:
> On 04/12/13 15:32, Hanjun Guo wrote:
> > On 2013年12月04日 01:26, Marc Zyngier wrote:
> >> Hi Hanjun,
> >>
> >> On 03/12/13 16:39, Hanjun Guo wrote:
> >>> In MADT table, there are GIC cpu interface base address and
> >>> GIC distributor base address, use them to convert GIC to ACPI.
> >>>
> >>> Signed-off-by: Hanjun Guo 
> >>> ---
> >>>   arch/arm64/kernel/irq.c  |5 
> >>>   drivers/acpi/plat/arm-core.c |   66 
> >>> --
> >>>   include/linux/acpi.h |6 
> >>>   3 files changed, 68 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> >>> index 473e5db..a9e68bf 100644
> >>> --- a/arch/arm64/kernel/irq.c
> >>> +++ b/arch/arm64/kernel/irq.c
> >>> @@ -25,6 +25,7 @@
> >>>   #include 
> >>>   #include 
> >>>   #include 
> >>> +#include 
> >>>   #include 
> >>>   #include 
> >>>   #include 
> >>> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
> >>> pt_regs *))
> >>>   void __init init_IRQ(void)
> >>>   {
> >>>   irqchip_init();
> >>> +
> >>> + if (!handle_arch_irq)
> >>> + acpi_gic_init();
> >>> +
> >> Why is the GIC hardcoded?
> > 
> > Very good question, thanks. I considered GIC only in my patch set.
> > I have no idea how to handle the GIC hardcoded problem here for
> > now, but I will figure it out later.
> > 
> > If any suggestion, I will appreciate a lot.
> > 
> >> How are you going to support other interrupt
> >> controllers?
> > 
> > ACPI 5.0 supports GICv2 only for now, if we want to
> > support other interrupt controller, we should introduce
> > some OEM table and parsing it, and it will not covered
> > by this patch set.
> > 
> >>>   if (!handle_arch_irq)
> >>>   panic("No interrupt controller found.");
> >>>   }
> >>> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
> >>> index 17c99e1..509b847 100644
> >>> --- a/drivers/acpi/plat/arm-core.c
> >>> +++ b/drivers/acpi/plat/arm-core.c
> >>> @@ -29,6 +29,7 @@
> >>>   #include 
> >>>   #include 
> >>>   #include 
> >>> +#include 
> >>>   #include 
> >>>   #include 
> >>>   #include 
> >>> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, 
> >>> const unsigned long end)
> >>>   return 0;
> >>>   }
> >>>   
> >>> +#ifdef CONFIG_ARM_GIC
> >>> +/*
> >>> + * Hard code here, we can not get memory size from MADT (but FDT does),
> >>> + * this size is described in ARMv8 foudation model's User Guide
> >>> + */
> >>> +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
> >>> +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
> >> Aside from the incorrect sizes, how do you plan to address the other
> >> regions that the GICv2 specification describes?
> > 
> > Did these regions have the same base address? I mean the same
> > as GIC distributor base address and GIC cpu interface base address.
> > 
> > if yes, since the base address is stored in gic_init(), it can be for 
> > furture
> > use. if I misunderstood your question, please let me know.
> 
> Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own
> probing of the additional regions used for virtualization.
> 
> The GIC and VGIC code are completely separate, and you'll need to find
> an acceptable solution for that too.
> 
> >>>   static int __init
> >>>   acpi_parse_gic_distributor(struct acpi_subtable_header *header,
> >>>   const unsigned long end)
> >>>   {
> >>>   struct acpi_madt_generic_distributor *distributor = NULL;
> >>> + void __iomem *dist_base = NULL;
> >>> + void __iomem *cpu_base = NULL;
> >>>   
> >>>   distributor = (struct acpi_madt_generic_distributor *)header;
> >>>   
> >>> @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct 
> >>> acpi_subtable_header *header,
> >>>   
> >>>   acpi_table_print_madt_entry(header);
> >>>   
> >>> + /* GIC is initialised after page_init(), no need for early_ioremap */
> >>> + dist_base = ioremap(distributor->base_address,
> >>> + GIC_CPU_INTERFACE_MEMORY_SIZE);
> >>> + if (!dist_base) {
> >>> + pr_warn(PREFIX "unable to map gic dist registers\n");
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> + /*
> >>> +  * acpi_lapic_addr is stored in acpi_parse_madt(),
> >>> +  * so we can use it here for GIC init
> >>> +  */
> >>> + if (acpi_lapic_addr) {
> >>> + iounmap(dist_base);
> >>> + pr_warn(PREFIX "Invalid GIC cpu interface base address\n");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
> >>> + if (!cpu_base) {
> >>> + iounmap(dist_base);
> >>> + pr_warn(PREFIX "unable to map gic cpu registers\n");
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> + gic_init(distributor->gic_id, -1, dist_base, cpu_base);
> >>> +
> >>>   return 0;
> >>>   

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-09 Thread Olof Johansson
On Wed, Dec 04, 2013 at 03:50:17PM +, Marc Zyngier wrote:
 On 04/12/13 15:32, Hanjun Guo wrote:
  On 2013年12月04日 01:26, Marc Zyngier wrote:
  Hi Hanjun,
 
  On 03/12/13 16:39, Hanjun Guo wrote:
  In MADT table, there are GIC cpu interface base address and
  GIC distributor base address, use them to convert GIC to ACPI.
 
  Signed-off-by: Hanjun Guo hanjun@linaro.org
  ---
arch/arm64/kernel/irq.c  |5 
drivers/acpi/plat/arm-core.c |   66 
  --
include/linux/acpi.h |6 
3 files changed, 68 insertions(+), 9 deletions(-)
 
  diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
  index 473e5db..a9e68bf 100644
  --- a/arch/arm64/kernel/irq.c
  +++ b/arch/arm64/kernel/irq.c
  @@ -25,6 +25,7 @@
#include linux/irq.h
#include linux/smp.h
#include linux/init.h
  +#include linux/acpi.h
#include linux/irqchip.h
#include linux/seq_file.h
#include linux/ratelimit.h
  @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
  pt_regs *))
void __init init_IRQ(void)
{
irqchip_init();
  +
  + if (!handle_arch_irq)
  + acpi_gic_init();
  +
  Why is the GIC hardcoded?
  
  Very good question, thanks. I considered GIC only in my patch set.
  I have no idea how to handle the GIC hardcoded problem here for
  now, but I will figure it out later.
  
  If any suggestion, I will appreciate a lot.
  
  How are you going to support other interrupt
  controllers?
  
  ACPI 5.0 supports GICv2 only for now, if we want to
  support other interrupt controller, we should introduce
  some OEM table and parsing it, and it will not covered
  by this patch set.
  
if (!handle_arch_irq)
panic(No interrupt controller found.);
}
  diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
  index 17c99e1..509b847 100644
  --- a/drivers/acpi/plat/arm-core.c
  +++ b/drivers/acpi/plat/arm-core.c
  @@ -29,6 +29,7 @@
#include linux/module.h
#include linux/irq.h
#include linux/irqdomain.h
  +#include linux/irqchip/arm-gic.h
#include linux/slab.h
#include linux/bootmem.h
#include linux/ioport.h
  @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, 
  const unsigned long end)
return 0;
}

  +#ifdef CONFIG_ARM_GIC
  +/*
  + * Hard code here, we can not get memory size from MADT (but FDT does),
  + * this size is described in ARMv8 foudation model's User Guide
  + */
  +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
  +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
  Aside from the incorrect sizes, how do you plan to address the other
  regions that the GICv2 specification describes?
  
  Did these regions have the same base address? I mean the same
  as GIC distributor base address and GIC cpu interface base address.
  
  if yes, since the base address is stored in gic_init(), it can be for 
  furture
  use. if I misunderstood your question, please let me know.
 
 Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own
 probing of the additional regions used for virtualization.
 
 The GIC and VGIC code are completely separate, and you'll need to find
 an acceptable solution for that too.
 
static int __init
acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)
{
struct acpi_madt_generic_distributor *distributor = NULL;
  + void __iomem *dist_base = NULL;
  + void __iomem *cpu_base = NULL;

distributor = (struct acpi_madt_generic_distributor *)header;

  @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct 
  acpi_subtable_header *header,

acpi_table_print_madt_entry(header);

  + /* GIC is initialised after page_init(), no need for early_ioremap */
  + dist_base = ioremap(distributor-base_address,
  + GIC_CPU_INTERFACE_MEMORY_SIZE);
  + if (!dist_base) {
  + pr_warn(PREFIX unable to map gic dist registers\n);
  + return -ENOMEM;
  + }
  +
  + /*
  +  * acpi_lapic_addr is stored in acpi_parse_madt(),
  +  * so we can use it here for GIC init
  +  */
  + if (acpi_lapic_addr) {
  + iounmap(dist_base);
  + pr_warn(PREFIX Invalid GIC cpu interface base address\n);
  + return -EINVAL;
  + }
  +
  + cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
  + if (!cpu_base) {
  + iounmap(dist_base);
  + pr_warn(PREFIX unable to map gic cpu registers\n);
  + return -ENOMEM;
  + }
  +
  + gic_init(distributor-gic_id, -1, dist_base, cpu_base);
  +
return 0;
}
  +#else
  +static int __init
  +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
  + const unsigned long end)
  +{
  + return -ENODEV;
  +}
  +#endif /* CONFIG_ARM_GIC */

/*
 * Parse GIC cpu interface related entries in 

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-05 Thread Hanjun Guo

On 2013年12月04日 23:50, Marc Zyngier wrote:

On 04/12/13 15:32, Hanjun Guo wrote:

On 2013年12月04日 01:26, Marc Zyngier wrote:

Hi Hanjun,

On 03/12/13 16:39, Hanjun Guo wrote:

In MADT table, there are GIC cpu interface base address and
GIC distributor base address, use them to convert GIC to ACPI.

Signed-off-by: Hanjun Guo 
---
   arch/arm64/kernel/irq.c  |5 
   drivers/acpi/plat/arm-core.c |   66 
--
   include/linux/acpi.h |6 
   3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 473e5db..a9e68bf 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -25,6 +25,7 @@
   #include 
   #include 
   #include 
+#include 
   #include 
   #include 
   #include 
@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs 
*))
   void __init init_IRQ(void)
   {
irqchip_init();
+
+   if (!handle_arch_irq)
+   acpi_gic_init();
+

Why is the GIC hardcoded?

Very good question, thanks. I considered GIC only in my patch set.
I have no idea how to handle the GIC hardcoded problem here for
now, but I will figure it out later.

If any suggestion, I will appreciate a lot.


How are you going to support other interrupt
controllers?

ACPI 5.0 supports GICv2 only for now, if we want to
support other interrupt controller, we should introduce
some OEM table and parsing it, and it will not covered
by this patch set.


if (!handle_arch_irq)
panic("No interrupt controller found.");
   }
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
index 17c99e1..509b847 100644
--- a/drivers/acpi/plat/arm-core.c
+++ b/drivers/acpi/plat/arm-core.c
@@ -29,6 +29,7 @@
   #include 
   #include 
   #include 
+#include 
   #include 
   #include 
   #include 
@@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const 
unsigned long end)
return 0;
   }
   
+#ifdef CONFIG_ARM_GIC

+/*
+ * Hard code here, we can not get memory size from MADT (but FDT does),
+ * this size is described in ARMv8 foudation model's User Guide
+ */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
+#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

Aside from the incorrect sizes, how do you plan to address the other
regions that the GICv2 specification describes?

Did these regions have the same base address? I mean the same
as GIC distributor base address and GIC cpu interface base address.

if yes, since the base address is stored in gic_init(), it can be for
furture
use. if I misunderstood your question, please let me know.

Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own
probing of the additional regions used for virtualization.

The GIC and VGIC code are completely separate, and you'll need to find
an acceptable solution for that too.


Ok, will review the VGIC code for KVM, thanks for the guidance.


   static int __init
   acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)
   {
struct acpi_madt_generic_distributor *distributor = NULL;
+   void __iomem *dist_base = NULL;
+   void __iomem *cpu_base = NULL;
   
   	distributor = (struct acpi_madt_generic_distributor *)header;
   
@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header,
   
   	acpi_table_print_madt_entry(header);
   
+	/* GIC is initialised after page_init(), no need for early_ioremap */

+   dist_base = ioremap(distributor->base_address,
+   GIC_CPU_INTERFACE_MEMORY_SIZE);
+   if (!dist_base) {
+   pr_warn(PREFIX "unable to map gic dist registers\n");
+   return -ENOMEM;
+   }
+
+   /*
+* acpi_lapic_addr is stored in acpi_parse_madt(),
+* so we can use it here for GIC init
+*/
+   if (acpi_lapic_addr) {
+   iounmap(dist_base);
+   pr_warn(PREFIX "Invalid GIC cpu interface base address\n");
+   return -EINVAL;
+   }
+
+   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
+   if (!cpu_base) {
+   iounmap(dist_base);
+   pr_warn(PREFIX "unable to map gic cpu registers\n");
+   return -ENOMEM;
+   }
+
+   gic_init(distributor->gic_id, -1, dist_base, cpu_base);
+
return 0;
   }
+#else
+static int __init
+acpi_parse_gic_distributor(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   return -ENODEV;
+}
+#endif /* CONFIG_ARM_GIC */
   
   /*

* Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
*header,
   static int __init acpi_parse_madt_gic_entries(void)
   {
int count;
-
+
/*
 * do a partial walk of MADT to determine how many CPUs
 * we have 

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-05 Thread Hanjun Guo

On 2013年12月04日 23:50, Marc Zyngier wrote:

On 04/12/13 15:32, Hanjun Guo wrote:

On 2013年12月04日 01:26, Marc Zyngier wrote:

Hi Hanjun,

On 03/12/13 16:39, Hanjun Guo wrote:

In MADT table, there are GIC cpu interface base address and
GIC distributor base address, use them to convert GIC to ACPI.

Signed-off-by: Hanjun Guo hanjun@linaro.org
---
   arch/arm64/kernel/irq.c  |5 
   drivers/acpi/plat/arm-core.c |   66 
--
   include/linux/acpi.h |6 
   3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 473e5db..a9e68bf 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -25,6 +25,7 @@
   #include linux/irq.h
   #include linux/smp.h
   #include linux/init.h
+#include linux/acpi.h
   #include linux/irqchip.h
   #include linux/seq_file.h
   #include linux/ratelimit.h
@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs 
*))
   void __init init_IRQ(void)
   {
irqchip_init();
+
+   if (!handle_arch_irq)
+   acpi_gic_init();
+

Why is the GIC hardcoded?

Very good question, thanks. I considered GIC only in my patch set.
I have no idea how to handle the GIC hardcoded problem here for
now, but I will figure it out later.

If any suggestion, I will appreciate a lot.


How are you going to support other interrupt
controllers?

ACPI 5.0 supports GICv2 only for now, if we want to
support other interrupt controller, we should introduce
some OEM table and parsing it, and it will not covered
by this patch set.


if (!handle_arch_irq)
panic(No interrupt controller found.);
   }
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
index 17c99e1..509b847 100644
--- a/drivers/acpi/plat/arm-core.c
+++ b/drivers/acpi/plat/arm-core.c
@@ -29,6 +29,7 @@
   #include linux/module.h
   #include linux/irq.h
   #include linux/irqdomain.h
+#include linux/irqchip/arm-gic.h
   #include linux/slab.h
   #include linux/bootmem.h
   #include linux/ioport.h
@@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const 
unsigned long end)
return 0;
   }
   
+#ifdef CONFIG_ARM_GIC

+/*
+ * Hard code here, we can not get memory size from MADT (but FDT does),
+ * this size is described in ARMv8 foudation model's User Guide
+ */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
+#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

Aside from the incorrect sizes, how do you plan to address the other
regions that the GICv2 specification describes?

Did these regions have the same base address? I mean the same
as GIC distributor base address and GIC cpu interface base address.

if yes, since the base address is stored in gic_init(), it can be for
furture
use. if I misunderstood your question, please let me know.

Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own
probing of the additional regions used for virtualization.

The GIC and VGIC code are completely separate, and you'll need to find
an acceptable solution for that too.


Ok, will review the VGIC code for KVM, thanks for the guidance.


   static int __init
   acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)
   {
struct acpi_madt_generic_distributor *distributor = NULL;
+   void __iomem *dist_base = NULL;
+   void __iomem *cpu_base = NULL;
   
   	distributor = (struct acpi_madt_generic_distributor *)header;
   
@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header,
   
   	acpi_table_print_madt_entry(header);
   
+	/* GIC is initialised after page_init(), no need for early_ioremap */

+   dist_base = ioremap(distributor-base_address,
+   GIC_CPU_INTERFACE_MEMORY_SIZE);
+   if (!dist_base) {
+   pr_warn(PREFIX unable to map gic dist registers\n);
+   return -ENOMEM;
+   }
+
+   /*
+* acpi_lapic_addr is stored in acpi_parse_madt(),
+* so we can use it here for GIC init
+*/
+   if (acpi_lapic_addr) {
+   iounmap(dist_base);
+   pr_warn(PREFIX Invalid GIC cpu interface base address\n);
+   return -EINVAL;
+   }
+
+   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
+   if (!cpu_base) {
+   iounmap(dist_base);
+   pr_warn(PREFIX unable to map gic cpu registers\n);
+   return -ENOMEM;
+   }
+
+   gic_init(distributor-gic_id, -1, dist_base, cpu_base);
+
return 0;
   }
+#else
+static int __init
+acpi_parse_gic_distributor(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   return -ENODEV;
+}
+#endif /* CONFIG_ARM_GIC */
   
   /*

* Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct 

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-04 Thread Marc Zyngier
On 04/12/13 15:32, Hanjun Guo wrote:
> On 2013年12月04日 01:26, Marc Zyngier wrote:
>> Hi Hanjun,
>>
>> On 03/12/13 16:39, Hanjun Guo wrote:
>>> In MADT table, there are GIC cpu interface base address and
>>> GIC distributor base address, use them to convert GIC to ACPI.
>>>
>>> Signed-off-by: Hanjun Guo 
>>> ---
>>>   arch/arm64/kernel/irq.c  |5 
>>>   drivers/acpi/plat/arm-core.c |   66 
>>> --
>>>   include/linux/acpi.h |6 
>>>   3 files changed, 68 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
>>> index 473e5db..a9e68bf 100644
>>> --- a/arch/arm64/kernel/irq.c
>>> +++ b/arch/arm64/kernel/irq.c
>>> @@ -25,6 +25,7 @@
>>>   #include 
>>>   #include 
>>>   #include 
>>> +#include 
>>>   #include 
>>>   #include 
>>>   #include 
>>> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
>>> pt_regs *))
>>>   void __init init_IRQ(void)
>>>   {
>>> irqchip_init();
>>> +
>>> +   if (!handle_arch_irq)
>>> +   acpi_gic_init();
>>> +
>> Why is the GIC hardcoded?
> 
> Very good question, thanks. I considered GIC only in my patch set.
> I have no idea how to handle the GIC hardcoded problem here for
> now, but I will figure it out later.
> 
> If any suggestion, I will appreciate a lot.
> 
>> How are you going to support other interrupt
>> controllers?
> 
> ACPI 5.0 supports GICv2 only for now, if we want to
> support other interrupt controller, we should introduce
> some OEM table and parsing it, and it will not covered
> by this patch set.
> 
>>> if (!handle_arch_irq)
>>> panic("No interrupt controller found.");
>>>   }
>>> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
>>> index 17c99e1..509b847 100644
>>> --- a/drivers/acpi/plat/arm-core.c
>>> +++ b/drivers/acpi/plat/arm-core.c
>>> @@ -29,6 +29,7 @@
>>>   #include 
>>>   #include 
>>>   #include 
>>> +#include 
>>>   #include 
>>>   #include 
>>>   #include 
>>> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, 
>>> const unsigned long end)
>>> return 0;
>>>   }
>>>   
>>> +#ifdef CONFIG_ARM_GIC
>>> +/*
>>> + * Hard code here, we can not get memory size from MADT (but FDT does),
>>> + * this size is described in ARMv8 foudation model's User Guide
>>> + */
>>> +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
>>> +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
>> Aside from the incorrect sizes, how do you plan to address the other
>> regions that the GICv2 specification describes?
> 
> Did these regions have the same base address? I mean the same
> as GIC distributor base address and GIC cpu interface base address.
> 
> if yes, since the base address is stored in gic_init(), it can be for 
> furture
> use. if I misunderstood your question, please let me know.

Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own
probing of the additional regions used for virtualization.

The GIC and VGIC code are completely separate, and you'll need to find
an acceptable solution for that too.

>>>   static int __init
>>>   acpi_parse_gic_distributor(struct acpi_subtable_header *header,
>>> const unsigned long end)
>>>   {
>>> struct acpi_madt_generic_distributor *distributor = NULL;
>>> +   void __iomem *dist_base = NULL;
>>> +   void __iomem *cpu_base = NULL;
>>>   
>>> distributor = (struct acpi_madt_generic_distributor *)header;
>>>   
>>> @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
>>> *header,
>>>   
>>> acpi_table_print_madt_entry(header);
>>>   
>>> +   /* GIC is initialised after page_init(), no need for early_ioremap */
>>> +   dist_base = ioremap(distributor->base_address,
>>> +   GIC_CPU_INTERFACE_MEMORY_SIZE);
>>> +   if (!dist_base) {
>>> +   pr_warn(PREFIX "unable to map gic dist registers\n");
>>> +   return -ENOMEM;
>>> +   }
>>> +
>>> +   /*
>>> +* acpi_lapic_addr is stored in acpi_parse_madt(),
>>> +* so we can use it here for GIC init
>>> +*/
>>> +   if (acpi_lapic_addr) {
>>> +   iounmap(dist_base);
>>> +   pr_warn(PREFIX "Invalid GIC cpu interface base address\n");
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
>>> +   if (!cpu_base) {
>>> +   iounmap(dist_base);
>>> +   pr_warn(PREFIX "unable to map gic cpu registers\n");
>>> +   return -ENOMEM;
>>> +   }
>>> +
>>> +   gic_init(distributor->gic_id, -1, dist_base, cpu_base);
>>> +
>>> return 0;
>>>   }
>>> +#else
>>> +static int __init
>>> +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
>>> +   const unsigned long end)
>>> +{
>>> +   return -ENODEV;
>>> +}
>>> +#endif /* CONFIG_ARM_GIC */
>>>   
>>>   /*
>>>* Parse GIC cpu interface related entries in MADT
>>> @@ -234,7 +280,7 @@ 

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-04 Thread Hanjun Guo

On 2013年12月04日 01:26, Marc Zyngier wrote:

Hi Hanjun,

On 03/12/13 16:39, Hanjun Guo wrote:

In MADT table, there are GIC cpu interface base address and
GIC distributor base address, use them to convert GIC to ACPI.

Signed-off-by: Hanjun Guo 
---
  arch/arm64/kernel/irq.c  |5 
  drivers/acpi/plat/arm-core.c |   66 --
  include/linux/acpi.h |6 
  3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 473e5db..a9e68bf 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -25,6 +25,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs 
*))
  void __init init_IRQ(void)
  {
irqchip_init();
+
+   if (!handle_arch_irq)
+   acpi_gic_init();
+

Why is the GIC hardcoded?


Very good question, thanks. I considered GIC only in my patch set.
I have no idea how to handle the GIC hardcoded problem here for
now, but I will figure it out later.

If any suggestion, I will appreciate a lot.


How are you going to support other interrupt
controllers?


ACPI 5.0 supports GICv2 only for now, if we want to
support other interrupt controller, we should introduce
some OEM table and parsing it, and it will not covered
by this patch set.


if (!handle_arch_irq)
panic("No interrupt controller found.");
  }
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
index 17c99e1..509b847 100644
--- a/drivers/acpi/plat/arm-core.c
+++ b/drivers/acpi/plat/arm-core.c
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const 
unsigned long end)
return 0;
  }
  
+#ifdef CONFIG_ARM_GIC

+/*
+ * Hard code here, we can not get memory size from MADT (but FDT does),
+ * this size is described in ARMv8 foudation model's User Guide
+ */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
+#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

Aside from the incorrect sizes, how do you plan to address the other
regions that the GICv2 specification describes?


Did these regions have the same base address? I mean the same
as GIC distributor base address and GIC cpu interface base address.

if yes, since the base address is stored in gic_init(), it can be for 
furture

use. if I misunderstood your question, please let me know.


  static int __init
  acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)
  {
struct acpi_madt_generic_distributor *distributor = NULL;
+   void __iomem *dist_base = NULL;
+   void __iomem *cpu_base = NULL;
  
  	distributor = (struct acpi_madt_generic_distributor *)header;
  
@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header,
  
  	acpi_table_print_madt_entry(header);
  
+	/* GIC is initialised after page_init(), no need for early_ioremap */

+   dist_base = ioremap(distributor->base_address,
+   GIC_CPU_INTERFACE_MEMORY_SIZE);
+   if (!dist_base) {
+   pr_warn(PREFIX "unable to map gic dist registers\n");
+   return -ENOMEM;
+   }
+
+   /*
+* acpi_lapic_addr is stored in acpi_parse_madt(),
+* so we can use it here for GIC init
+*/
+   if (acpi_lapic_addr) {
+   iounmap(dist_base);
+   pr_warn(PREFIX "Invalid GIC cpu interface base address\n");
+   return -EINVAL;
+   }
+
+   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
+   if (!cpu_base) {
+   iounmap(dist_base);
+   pr_warn(PREFIX "unable to map gic cpu registers\n");
+   return -ENOMEM;
+   }
+
+   gic_init(distributor->gic_id, -1, dist_base, cpu_base);
+
return 0;
  }
+#else
+static int __init
+acpi_parse_gic_distributor(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   return -ENODEV;
+}
+#endif /* CONFIG_ARM_GIC */
  
  /*

   * Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
*header,
  static int __init acpi_parse_madt_gic_entries(void)
  {
int count;
-
+
/*
 * do a partial walk of MADT to determine how many CPUs
 * we have including disabled CPUs
@@ -468,19 +514,21 @@ static void __init acpi_process_madt(void)
 * Parse MADT GIC cpu interface entries
 */
error = acpi_parse_madt_gic_entries();
-   if (!error) {
-   /*
-* Parse MADT GIC distributor entries
-*/
-   

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-04 Thread Hanjun Guo

On 2013年12月04日 01:09, Rob Herring wrote:

On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo  wrote:

In MADT table, there are GIC cpu interface base address and
GIC distributor base address, use them to convert GIC to ACPI.

Signed-off-by: Hanjun Guo 
---
  arch/arm64/kernel/irq.c  |5 
  drivers/acpi/plat/arm-core.c |   66 --
  include/linux/acpi.h |6 
  3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 473e5db..a9e68bf 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -25,6 +25,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs 
*))
  void __init init_IRQ(void)
  {
 irqchip_init();
+
+   if (!handle_arch_irq)
+   acpi_gic_init();
+
 if (!handle_arch_irq)
 panic("No interrupt controller found.");
  }
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
index 17c99e1..509b847 100644
--- a/drivers/acpi/plat/arm-core.c
+++ b/drivers/acpi/plat/arm-core.c
@@ -29,6 +29,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const 
unsigned long end)
 return 0;
  }

+#ifdef CONFIG_ARM_GIC

Perhaps this should go in the GIC code? This is more of a general
question of where init/probing code goes. For DT, this as been with
the driver code.


I'm ok with your suggestion, how about move the code to
drivers/irqchip/irq-gic.c ? is this make sense to you?


+/*
+ * Hard code here, we can not get memory size from MADT (but FDT does),
+ * this size is described in ARMv8 foudation model's User Guide
+ */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
+#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

You have the sizes swapped. The cpu interface has the DIR register at 0x1000.


I will figure out the right size in next version.


+
  static int __init
  acpi_parse_gic_distributor(struct acpi_subtable_header *header,
 const unsigned long end)
  {
 struct acpi_madt_generic_distributor *distributor = NULL;
+   void __iomem *dist_base = NULL;
+   void __iomem *cpu_base = NULL;

Initialization here is unnecessary.


ok, will update in next version.


 distributor = (struct acpi_madt_generic_distributor *)header;

@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
*header,

 acpi_table_print_madt_entry(header);

+   /* GIC is initialised after page_init(), no need for early_ioremap */
+   dist_base = ioremap(distributor->base_address,
+   GIC_CPU_INTERFACE_MEMORY_SIZE);

Should be GIC_DISTRIBUTOR_MEMORY_SIZE.


Good catch


+   if (!dist_base) {
+   pr_warn(PREFIX "unable to map gic dist registers\n");
+   return -ENOMEM;
+   }
+
+   /*
+* acpi_lapic_addr is stored in acpi_parse_madt(),
+* so we can use it here for GIC init
+*/
+   if (acpi_lapic_addr) {

Checking this first would be cleaner.


Agreed, thank you for the advice, will update it in next version.


+   iounmap(dist_base);
+   pr_warn(PREFIX "Invalid GIC cpu interface base address\n");
+   return -EINVAL;
+   }
+
+   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);

How are gic's with different cpu address per core going to be handled?


do you mean some GIC without banked registers?
if yes, ACPI can handle that, in the GIC (GIC cpu interface) structure, there is
"Physical Base Address" per core, we can use it to handle gic's with different
cpu address per core.

This part of code is not implemented yet, if needed, will send out in next 
version.



+   if (!cpu_base) {
+   iounmap(dist_base);
+   pr_warn(PREFIX "unable to map gic cpu registers\n");

All the printks are a bit verbose for my tastes. I think a single
error print would suffice.


do you mean if meet some error, then got to a single error printk?


+   return -ENOMEM;
+   }
+
+   gic_init(distributor->gic_id, -1, dist_base, cpu_base);
+
 return 0;
  }
+#else
+static int __init
+acpi_parse_gic_distributor(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   return -ENODEV;
+}
+#endif /* CONFIG_ARM_GIC */

A "if (!IS_ENABLED(CONFIG_ARM_GIC)) return;" in the above function
would eliminate this ifdef.


Thanks for the suggestion, will do it


  /*
   * Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
*header,
  static int __init acpi_parse_madt_gic_entries(void)
  {
 int count;
-
+

Unnecessary whitespace change.


will update it :)


Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-04 Thread Hanjun Guo

On 2013年12月04日 01:09, Rob Herring wrote:

On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun@linaro.org wrote:

In MADT table, there are GIC cpu interface base address and
GIC distributor base address, use them to convert GIC to ACPI.

Signed-off-by: Hanjun Guo hanjun@linaro.org
---
  arch/arm64/kernel/irq.c  |5 
  drivers/acpi/plat/arm-core.c |   66 --
  include/linux/acpi.h |6 
  3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 473e5db..a9e68bf 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -25,6 +25,7 @@
  #include linux/irq.h
  #include linux/smp.h
  #include linux/init.h
+#include linux/acpi.h
  #include linux/irqchip.h
  #include linux/seq_file.h
  #include linux/ratelimit.h
@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs 
*))
  void __init init_IRQ(void)
  {
 irqchip_init();
+
+   if (!handle_arch_irq)
+   acpi_gic_init();
+
 if (!handle_arch_irq)
 panic(No interrupt controller found.);
  }
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
index 17c99e1..509b847 100644
--- a/drivers/acpi/plat/arm-core.c
+++ b/drivers/acpi/plat/arm-core.c
@@ -29,6 +29,7 @@
  #include linux/module.h
  #include linux/irq.h
  #include linux/irqdomain.h
+#include linux/irqchip/arm-gic.h
  #include linux/slab.h
  #include linux/bootmem.h
  #include linux/ioport.h
@@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const 
unsigned long end)
 return 0;
  }

+#ifdef CONFIG_ARM_GIC

Perhaps this should go in the GIC code? This is more of a general
question of where init/probing code goes. For DT, this as been with
the driver code.


I'm ok with your suggestion, how about move the code to
drivers/irqchip/irq-gic.c ? is this make sense to you?


+/*
+ * Hard code here, we can not get memory size from MADT (but FDT does),
+ * this size is described in ARMv8 foudation model's User Guide
+ */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
+#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

You have the sizes swapped. The cpu interface has the DIR register at 0x1000.


I will figure out the right size in next version.


+
  static int __init
  acpi_parse_gic_distributor(struct acpi_subtable_header *header,
 const unsigned long end)
  {
 struct acpi_madt_generic_distributor *distributor = NULL;
+   void __iomem *dist_base = NULL;
+   void __iomem *cpu_base = NULL;

Initialization here is unnecessary.


ok, will update in next version.


 distributor = (struct acpi_madt_generic_distributor *)header;

@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
*header,

 acpi_table_print_madt_entry(header);

+   /* GIC is initialised after page_init(), no need for early_ioremap */
+   dist_base = ioremap(distributor-base_address,
+   GIC_CPU_INTERFACE_MEMORY_SIZE);

Should be GIC_DISTRIBUTOR_MEMORY_SIZE.


Good catch


+   if (!dist_base) {
+   pr_warn(PREFIX unable to map gic dist registers\n);
+   return -ENOMEM;
+   }
+
+   /*
+* acpi_lapic_addr is stored in acpi_parse_madt(),
+* so we can use it here for GIC init
+*/
+   if (acpi_lapic_addr) {

Checking this first would be cleaner.


Agreed, thank you for the advice, will update it in next version.


+   iounmap(dist_base);
+   pr_warn(PREFIX Invalid GIC cpu interface base address\n);
+   return -EINVAL;
+   }
+
+   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);

How are gic's with different cpu address per core going to be handled?


do you mean some GIC without banked registers?
if yes, ACPI can handle that, in the GIC (GIC cpu interface) structure, there is
Physical Base Address per core, we can use it to handle gic's with different
cpu address per core.

This part of code is not implemented yet, if needed, will send out in next 
version.



+   if (!cpu_base) {
+   iounmap(dist_base);
+   pr_warn(PREFIX unable to map gic cpu registers\n);

All the printks are a bit verbose for my tastes. I think a single
error print would suffice.


do you mean if meet some error, then got to a single error printk?


+   return -ENOMEM;
+   }
+
+   gic_init(distributor-gic_id, -1, dist_base, cpu_base);
+
 return 0;
  }
+#else
+static int __init
+acpi_parse_gic_distributor(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   return -ENODEV;
+}
+#endif /* CONFIG_ARM_GIC */

A if (!IS_ENABLED(CONFIG_ARM_GIC)) return; in the above function
would eliminate this ifdef.


Thanks for the suggestion, will do it


  /*
   * Parse GIC cpu interface related entries 

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-04 Thread Hanjun Guo

On 2013年12月04日 01:26, Marc Zyngier wrote:

Hi Hanjun,

On 03/12/13 16:39, Hanjun Guo wrote:

In MADT table, there are GIC cpu interface base address and
GIC distributor base address, use them to convert GIC to ACPI.

Signed-off-by: Hanjun Guo hanjun@linaro.org
---
  arch/arm64/kernel/irq.c  |5 
  drivers/acpi/plat/arm-core.c |   66 --
  include/linux/acpi.h |6 
  3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 473e5db..a9e68bf 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -25,6 +25,7 @@
  #include linux/irq.h
  #include linux/smp.h
  #include linux/init.h
+#include linux/acpi.h
  #include linux/irqchip.h
  #include linux/seq_file.h
  #include linux/ratelimit.h
@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs 
*))
  void __init init_IRQ(void)
  {
irqchip_init();
+
+   if (!handle_arch_irq)
+   acpi_gic_init();
+

Why is the GIC hardcoded?


Very good question, thanks. I considered GIC only in my patch set.
I have no idea how to handle the GIC hardcoded problem here for
now, but I will figure it out later.

If any suggestion, I will appreciate a lot.


How are you going to support other interrupt
controllers?


ACPI 5.0 supports GICv2 only for now, if we want to
support other interrupt controller, we should introduce
some OEM table and parsing it, and it will not covered
by this patch set.


if (!handle_arch_irq)
panic(No interrupt controller found.);
  }
diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
index 17c99e1..509b847 100644
--- a/drivers/acpi/plat/arm-core.c
+++ b/drivers/acpi/plat/arm-core.c
@@ -29,6 +29,7 @@
  #include linux/module.h
  #include linux/irq.h
  #include linux/irqdomain.h
+#include linux/irqchip/arm-gic.h
  #include linux/slab.h
  #include linux/bootmem.h
  #include linux/ioport.h
@@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, const 
unsigned long end)
return 0;
  }
  
+#ifdef CONFIG_ARM_GIC

+/*
+ * Hard code here, we can not get memory size from MADT (but FDT does),
+ * this size is described in ARMv8 foudation model's User Guide
+ */
+#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
+#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

Aside from the incorrect sizes, how do you plan to address the other
regions that the GICv2 specification describes?


Did these regions have the same base address? I mean the same
as GIC distributor base address and GIC cpu interface base address.

if yes, since the base address is stored in gic_init(), it can be for 
furture

use. if I misunderstood your question, please let me know.


  static int __init
  acpi_parse_gic_distributor(struct acpi_subtable_header *header,
const unsigned long end)
  {
struct acpi_madt_generic_distributor *distributor = NULL;
+   void __iomem *dist_base = NULL;
+   void __iomem *cpu_base = NULL;
  
  	distributor = (struct acpi_madt_generic_distributor *)header;
  
@@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header *header,
  
  	acpi_table_print_madt_entry(header);
  
+	/* GIC is initialised after page_init(), no need for early_ioremap */

+   dist_base = ioremap(distributor-base_address,
+   GIC_CPU_INTERFACE_MEMORY_SIZE);
+   if (!dist_base) {
+   pr_warn(PREFIX unable to map gic dist registers\n);
+   return -ENOMEM;
+   }
+
+   /*
+* acpi_lapic_addr is stored in acpi_parse_madt(),
+* so we can use it here for GIC init
+*/
+   if (acpi_lapic_addr) {
+   iounmap(dist_base);
+   pr_warn(PREFIX Invalid GIC cpu interface base address\n);
+   return -EINVAL;
+   }
+
+   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
+   if (!cpu_base) {
+   iounmap(dist_base);
+   pr_warn(PREFIX unable to map gic cpu registers\n);
+   return -ENOMEM;
+   }
+
+   gic_init(distributor-gic_id, -1, dist_base, cpu_base);
+
return 0;
  }
+#else
+static int __init
+acpi_parse_gic_distributor(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   return -ENODEV;
+}
+#endif /* CONFIG_ARM_GIC */
  
  /*

   * Parse GIC cpu interface related entries in MADT
@@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
*header,
  static int __init acpi_parse_madt_gic_entries(void)
  {
int count;
-
+
/*
 * do a partial walk of MADT to determine how many CPUs
 * we have including disabled CPUs
@@ -468,19 +514,21 @@ static void __init acpi_process_madt(void)
 * Parse MADT GIC cpu interface entries
 */
error = 

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-04 Thread Marc Zyngier
On 04/12/13 15:32, Hanjun Guo wrote:
 On 2013年12月04日 01:26, Marc Zyngier wrote:
 Hi Hanjun,

 On 03/12/13 16:39, Hanjun Guo wrote:
 In MADT table, there are GIC cpu interface base address and
 GIC distributor base address, use them to convert GIC to ACPI.

 Signed-off-by: Hanjun Guo hanjun@linaro.org
 ---
   arch/arm64/kernel/irq.c  |5 
   drivers/acpi/plat/arm-core.c |   66 
 --
   include/linux/acpi.h |6 
   3 files changed, 68 insertions(+), 9 deletions(-)

 diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
 index 473e5db..a9e68bf 100644
 --- a/arch/arm64/kernel/irq.c
 +++ b/arch/arm64/kernel/irq.c
 @@ -25,6 +25,7 @@
   #include linux/irq.h
   #include linux/smp.h
   #include linux/init.h
 +#include linux/acpi.h
   #include linux/irqchip.h
   #include linux/seq_file.h
   #include linux/ratelimit.h
 @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
 pt_regs *))
   void __init init_IRQ(void)
   {
 irqchip_init();
 +
 +   if (!handle_arch_irq)
 +   acpi_gic_init();
 +
 Why is the GIC hardcoded?
 
 Very good question, thanks. I considered GIC only in my patch set.
 I have no idea how to handle the GIC hardcoded problem here for
 now, but I will figure it out later.
 
 If any suggestion, I will appreciate a lot.
 
 How are you going to support other interrupt
 controllers?
 
 ACPI 5.0 supports GICv2 only for now, if we want to
 support other interrupt controller, we should introduce
 some OEM table and parsing it, and it will not covered
 by this patch set.
 
 if (!handle_arch_irq)
 panic(No interrupt controller found.);
   }
 diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
 index 17c99e1..509b847 100644
 --- a/drivers/acpi/plat/arm-core.c
 +++ b/drivers/acpi/plat/arm-core.c
 @@ -29,6 +29,7 @@
   #include linux/module.h
   #include linux/irq.h
   #include linux/irqdomain.h
 +#include linux/irqchip/arm-gic.h
   #include linux/slab.h
   #include linux/bootmem.h
   #include linux/ioport.h
 @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, 
 const unsigned long end)
 return 0;
   }
   
 +#ifdef CONFIG_ARM_GIC
 +/*
 + * Hard code here, we can not get memory size from MADT (but FDT does),
 + * this size is described in ARMv8 foudation model's User Guide
 + */
 +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
 +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)
 Aside from the incorrect sizes, how do you plan to address the other
 regions that the GICv2 specification describes?
 
 Did these regions have the same base address? I mean the same
 as GIC distributor base address and GIC cpu interface base address.
 
 if yes, since the base address is stored in gic_init(), it can be for 
 furture
 use. if I misunderstood your question, please let me know.

Look at the VGIC implementation for KVM in virt/kvm/arm. It does its own
probing of the additional regions used for virtualization.

The GIC and VGIC code are completely separate, and you'll need to find
an acceptable solution for that too.

   static int __init
   acpi_parse_gic_distributor(struct acpi_subtable_header *header,
 const unsigned long end)
   {
 struct acpi_madt_generic_distributor *distributor = NULL;
 +   void __iomem *dist_base = NULL;
 +   void __iomem *cpu_base = NULL;
   
 distributor = (struct acpi_madt_generic_distributor *)header;
   
 @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
 *header,
   
 acpi_table_print_madt_entry(header);
   
 +   /* GIC is initialised after page_init(), no need for early_ioremap */
 +   dist_base = ioremap(distributor-base_address,
 +   GIC_CPU_INTERFACE_MEMORY_SIZE);
 +   if (!dist_base) {
 +   pr_warn(PREFIX unable to map gic dist registers\n);
 +   return -ENOMEM;
 +   }
 +
 +   /*
 +* acpi_lapic_addr is stored in acpi_parse_madt(),
 +* so we can use it here for GIC init
 +*/
 +   if (acpi_lapic_addr) {
 +   iounmap(dist_base);
 +   pr_warn(PREFIX Invalid GIC cpu interface base address\n);
 +   return -EINVAL;
 +   }
 +
 +   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
 +   if (!cpu_base) {
 +   iounmap(dist_base);
 +   pr_warn(PREFIX unable to map gic cpu registers\n);
 +   return -ENOMEM;
 +   }
 +
 +   gic_init(distributor-gic_id, -1, dist_base, cpu_base);
 +
 return 0;
   }
 +#else
 +static int __init
 +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
 +   const unsigned long end)
 +{
 +   return -ENODEV;
 +}
 +#endif /* CONFIG_ARM_GIC */
   
   /*
* Parse GIC cpu interface related entries in MADT
 @@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
 *header,
   static int __init acpi_parse_madt_gic_entries(void)
   {
 int count;
 -
 +
 /*
  * do a partial walk 

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-03 Thread Marc Zyngier
Hi Hanjun,

On 03/12/13 16:39, Hanjun Guo wrote:
> In MADT table, there are GIC cpu interface base address and
> GIC distributor base address, use them to convert GIC to ACPI.
> 
> Signed-off-by: Hanjun Guo 
> ---
>  arch/arm64/kernel/irq.c  |5 
>  drivers/acpi/plat/arm-core.c |   66 
> --
>  include/linux/acpi.h |6 
>  3 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index 473e5db..a9e68bf 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
> pt_regs *))
>  void __init init_IRQ(void)
>  {
>   irqchip_init();
> +
> + if (!handle_arch_irq)
> + acpi_gic_init();
> +

Why is the GIC hardcoded? How are you going to support other interrupt
controllers?

>   if (!handle_arch_irq)
>   panic("No interrupt controller found.");
>  }
> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
> index 17c99e1..509b847 100644
> --- a/drivers/acpi/plat/arm-core.c
> +++ b/drivers/acpi/plat/arm-core.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, 
> const unsigned long end)
>   return 0;
>  }
>  
> +#ifdef CONFIG_ARM_GIC
> +/*
> + * Hard code here, we can not get memory size from MADT (but FDT does),
> + * this size is described in ARMv8 foudation model's User Guide
> + */
> +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
> +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

Aside from the incorrect sizes, how do you plan to address the other
regions that the GICv2 specification describes?

>  static int __init
>  acpi_parse_gic_distributor(struct acpi_subtable_header *header,
>   const unsigned long end)
>  {
>   struct acpi_madt_generic_distributor *distributor = NULL;
> + void __iomem *dist_base = NULL;
> + void __iomem *cpu_base = NULL;
>  
>   distributor = (struct acpi_madt_generic_distributor *)header;
>  
> @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
> *header,
>  
>   acpi_table_print_madt_entry(header);
>  
> + /* GIC is initialised after page_init(), no need for early_ioremap */
> + dist_base = ioremap(distributor->base_address,
> + GIC_CPU_INTERFACE_MEMORY_SIZE);
> + if (!dist_base) {
> + pr_warn(PREFIX "unable to map gic dist registers\n");
> + return -ENOMEM;
> + }
> +
> + /*
> +  * acpi_lapic_addr is stored in acpi_parse_madt(),
> +  * so we can use it here for GIC init
> +  */
> + if (acpi_lapic_addr) {
> + iounmap(dist_base);
> + pr_warn(PREFIX "Invalid GIC cpu interface base address\n");
> + return -EINVAL;
> + }
> +
> + cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
> + if (!cpu_base) {
> + iounmap(dist_base);
> + pr_warn(PREFIX "unable to map gic cpu registers\n");
> + return -ENOMEM;
> + }
> +
> + gic_init(distributor->gic_id, -1, dist_base, cpu_base);
> +
>   return 0;
>  }
> +#else
> +static int __init
> +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + return -ENODEV;
> +}
> +#endif /* CONFIG_ARM_GIC */
>  
>  /*
>   * Parse GIC cpu interface related entries in MADT
> @@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
> *header,
>  static int __init acpi_parse_madt_gic_entries(void)
>  {
>   int count;
> -
> + 
>   /*
>* do a partial walk of MADT to determine how many CPUs
>* we have including disabled CPUs
> @@ -468,19 +514,21 @@ static void __init acpi_process_madt(void)
>* Parse MADT GIC cpu interface entries
>*/
>   error = acpi_parse_madt_gic_entries();
> - if (!error) {
> - /*
> -  * Parse MADT GIC distributor entries
> -  */
> - acpi_parse_madt_gic_distributor_entries();
> - }
> + if (!error)
> + pr_info("Using ACPI for processor (GIC) configuration 
> information\n");
>   }
>  
> - pr_info("Using ACPI for processor (GIC) configuration information\n");
> -
>   return;
>  }
>  
> +int __init acpi_gic_init(void)
> +{
> + /*
> +  * Parse MADT GIC distributor entries
> +  */
> + return acpi_parse_madt_gic_distributor_entries();
> +}
> +

Why can't you do the GIC init in the GIC code? We've tried hard to make
interrupt controllers discoverable and self 

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-03 Thread Rob Herring
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo  wrote:
> In MADT table, there are GIC cpu interface base address and
> GIC distributor base address, use them to convert GIC to ACPI.
>
> Signed-off-by: Hanjun Guo 
> ---
>  arch/arm64/kernel/irq.c  |5 
>  drivers/acpi/plat/arm-core.c |   66 
> --
>  include/linux/acpi.h |6 
>  3 files changed, 68 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index 473e5db..a9e68bf 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
> pt_regs *))
>  void __init init_IRQ(void)
>  {
> irqchip_init();
> +
> +   if (!handle_arch_irq)
> +   acpi_gic_init();
> +
> if (!handle_arch_irq)
> panic("No interrupt controller found.");
>  }
> diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
> index 17c99e1..509b847 100644
> --- a/drivers/acpi/plat/arm-core.c
> +++ b/drivers/acpi/plat/arm-core.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, 
> const unsigned long end)
> return 0;
>  }
>
> +#ifdef CONFIG_ARM_GIC

Perhaps this should go in the GIC code? This is more of a general
question of where init/probing code goes. For DT, this as been with
the driver code.

> +/*
> + * Hard code here, we can not get memory size from MADT (but FDT does),
> + * this size is described in ARMv8 foudation model's User Guide
> + */
> +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
> +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

You have the sizes swapped. The cpu interface has the DIR register at 0x1000.

> +
>  static int __init
>  acpi_parse_gic_distributor(struct acpi_subtable_header *header,
> const unsigned long end)
>  {
> struct acpi_madt_generic_distributor *distributor = NULL;
> +   void __iomem *dist_base = NULL;
> +   void __iomem *cpu_base = NULL;

Initialization here is unnecessary.

>
> distributor = (struct acpi_madt_generic_distributor *)header;
>
> @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
> *header,
>
> acpi_table_print_madt_entry(header);
>
> +   /* GIC is initialised after page_init(), no need for early_ioremap */
> +   dist_base = ioremap(distributor->base_address,
> +   GIC_CPU_INTERFACE_MEMORY_SIZE);

Should be GIC_DISTRIBUTOR_MEMORY_SIZE.

> +   if (!dist_base) {
> +   pr_warn(PREFIX "unable to map gic dist registers\n");
> +   return -ENOMEM;
> +   }
> +
> +   /*
> +* acpi_lapic_addr is stored in acpi_parse_madt(),
> +* so we can use it here for GIC init
> +*/
> +   if (acpi_lapic_addr) {

Checking this first would be cleaner.

> +   iounmap(dist_base);
> +   pr_warn(PREFIX "Invalid GIC cpu interface base address\n");
> +   return -EINVAL;
> +   }
> +
> +   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);

How are gic's with different cpu address per core going to be handled?

> +   if (!cpu_base) {
> +   iounmap(dist_base);
> +   pr_warn(PREFIX "unable to map gic cpu registers\n");

All the printks are a bit verbose for my tastes. I think a single
error print would suffice.

> +   return -ENOMEM;
> +   }
> +
> +   gic_init(distributor->gic_id, -1, dist_base, cpu_base);
> +
> return 0;
>  }
> +#else
> +static int __init
> +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
> +   const unsigned long end)
> +{
> +   return -ENODEV;
> +}
> +#endif /* CONFIG_ARM_GIC */

A "if (!IS_ENABLED(CONFIG_ARM_GIC)) return;" in the above function
would eliminate this ifdef.

>
>  /*
>   * Parse GIC cpu interface related entries in MADT
> @@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
> *header,
>  static int __init acpi_parse_madt_gic_entries(void)
>  {
> int count;
> -
> +

Unnecessary whitespace change.

> /*
>  * do a partial walk of MADT to determine how many CPUs
>  * we have including disabled CPUs
> @@ -468,19 +514,21 @@ static void __init acpi_process_madt(void)
>  * Parse MADT GIC cpu interface entries
>  */
> error = acpi_parse_madt_gic_entries();
> -   if (!error) {
> -   /*
> -* Parse MADT GIC distributor entries
> -*/
> -   acpi_parse_madt_gic_distributor_entries();
> -   }
> +

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-03 Thread Rob Herring
On Tue, Dec 3, 2013 at 10:39 AM, Hanjun Guo hanjun@linaro.org wrote:
 In MADT table, there are GIC cpu interface base address and
 GIC distributor base address, use them to convert GIC to ACPI.

 Signed-off-by: Hanjun Guo hanjun@linaro.org
 ---
  arch/arm64/kernel/irq.c  |5 
  drivers/acpi/plat/arm-core.c |   66 
 --
  include/linux/acpi.h |6 
  3 files changed, 68 insertions(+), 9 deletions(-)

 diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
 index 473e5db..a9e68bf 100644
 --- a/arch/arm64/kernel/irq.c
 +++ b/arch/arm64/kernel/irq.c
 @@ -25,6 +25,7 @@
  #include linux/irq.h
  #include linux/smp.h
  #include linux/init.h
 +#include linux/acpi.h
  #include linux/irqchip.h
  #include linux/seq_file.h
  #include linux/ratelimit.h
 @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
 pt_regs *))
  void __init init_IRQ(void)
  {
 irqchip_init();
 +
 +   if (!handle_arch_irq)
 +   acpi_gic_init();
 +
 if (!handle_arch_irq)
 panic(No interrupt controller found.);
  }
 diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
 index 17c99e1..509b847 100644
 --- a/drivers/acpi/plat/arm-core.c
 +++ b/drivers/acpi/plat/arm-core.c
 @@ -29,6 +29,7 @@
  #include linux/module.h
  #include linux/irq.h
  #include linux/irqdomain.h
 +#include linux/irqchip/arm-gic.h
  #include linux/slab.h
  #include linux/bootmem.h
  #include linux/ioport.h
 @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, 
 const unsigned long end)
 return 0;
  }

 +#ifdef CONFIG_ARM_GIC

Perhaps this should go in the GIC code? This is more of a general
question of where init/probing code goes. For DT, this as been with
the driver code.

 +/*
 + * Hard code here, we can not get memory size from MADT (but FDT does),
 + * this size is described in ARMv8 foudation model's User Guide
 + */
 +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
 +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

You have the sizes swapped. The cpu interface has the DIR register at 0x1000.

 +
  static int __init
  acpi_parse_gic_distributor(struct acpi_subtable_header *header,
 const unsigned long end)
  {
 struct acpi_madt_generic_distributor *distributor = NULL;
 +   void __iomem *dist_base = NULL;
 +   void __iomem *cpu_base = NULL;

Initialization here is unnecessary.


 distributor = (struct acpi_madt_generic_distributor *)header;

 @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
 *header,

 acpi_table_print_madt_entry(header);

 +   /* GIC is initialised after page_init(), no need for early_ioremap */
 +   dist_base = ioremap(distributor-base_address,
 +   GIC_CPU_INTERFACE_MEMORY_SIZE);

Should be GIC_DISTRIBUTOR_MEMORY_SIZE.

 +   if (!dist_base) {
 +   pr_warn(PREFIX unable to map gic dist registers\n);
 +   return -ENOMEM;
 +   }
 +
 +   /*
 +* acpi_lapic_addr is stored in acpi_parse_madt(),
 +* so we can use it here for GIC init
 +*/
 +   if (acpi_lapic_addr) {

Checking this first would be cleaner.

 +   iounmap(dist_base);
 +   pr_warn(PREFIX Invalid GIC cpu interface base address\n);
 +   return -EINVAL;
 +   }
 +
 +   cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);

How are gic's with different cpu address per core going to be handled?

 +   if (!cpu_base) {
 +   iounmap(dist_base);
 +   pr_warn(PREFIX unable to map gic cpu registers\n);

All the printks are a bit verbose for my tastes. I think a single
error print would suffice.

 +   return -ENOMEM;
 +   }
 +
 +   gic_init(distributor-gic_id, -1, dist_base, cpu_base);
 +
 return 0;
  }
 +#else
 +static int __init
 +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
 +   const unsigned long end)
 +{
 +   return -ENODEV;
 +}
 +#endif /* CONFIG_ARM_GIC */

A if (!IS_ENABLED(CONFIG_ARM_GIC)) return; in the above function
would eliminate this ifdef.


  /*
   * Parse GIC cpu interface related entries in MADT
 @@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
 *header,
  static int __init acpi_parse_madt_gic_entries(void)
  {
 int count;
 -
 +

Unnecessary whitespace change.

 /*
  * do a partial walk of MADT to determine how many CPUs
  * we have including disabled CPUs
 @@ -468,19 +514,21 @@ static void __init acpi_process_madt(void)
  * Parse MADT GIC cpu interface entries
  */
 error = acpi_parse_madt_gic_entries();
 -   if (!error) {
 -   /*
 -* Parse MADT GIC distributor entries
 -

Re: [RFC part2 PATCH 9/9] ACPI / GIC: Initialize GIC using the information in MADT

2013-12-03 Thread Marc Zyngier
Hi Hanjun,

On 03/12/13 16:39, Hanjun Guo wrote:
 In MADT table, there are GIC cpu interface base address and
 GIC distributor base address, use them to convert GIC to ACPI.
 
 Signed-off-by: Hanjun Guo hanjun@linaro.org
 ---
  arch/arm64/kernel/irq.c  |5 
  drivers/acpi/plat/arm-core.c |   66 
 --
  include/linux/acpi.h |6 
  3 files changed, 68 insertions(+), 9 deletions(-)
 
 diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
 index 473e5db..a9e68bf 100644
 --- a/arch/arm64/kernel/irq.c
 +++ b/arch/arm64/kernel/irq.c
 @@ -25,6 +25,7 @@
  #include linux/irq.h
  #include linux/smp.h
  #include linux/init.h
 +#include linux/acpi.h
  #include linux/irqchip.h
  #include linux/seq_file.h
  #include linux/ratelimit.h
 @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
 pt_regs *))
  void __init init_IRQ(void)
  {
   irqchip_init();
 +
 + if (!handle_arch_irq)
 + acpi_gic_init();
 +

Why is the GIC hardcoded? How are you going to support other interrupt
controllers?

   if (!handle_arch_irq)
   panic(No interrupt controller found.);
  }
 diff --git a/drivers/acpi/plat/arm-core.c b/drivers/acpi/plat/arm-core.c
 index 17c99e1..509b847 100644
 --- a/drivers/acpi/plat/arm-core.c
 +++ b/drivers/acpi/plat/arm-core.c
 @@ -29,6 +29,7 @@
  #include linux/module.h
  #include linux/irq.h
  #include linux/irqdomain.h
 +#include linux/irqchip/arm-gic.h
  #include linux/slab.h
  #include linux/bootmem.h
  #include linux/ioport.h
 @@ -211,11 +212,21 @@ acpi_parse_gic(struct acpi_subtable_header *header, 
 const unsigned long end)
   return 0;
  }
  
 +#ifdef CONFIG_ARM_GIC
 +/*
 + * Hard code here, we can not get memory size from MADT (but FDT does),
 + * this size is described in ARMv8 foudation model's User Guide
 + */
 +#define GIC_DISTRIBUTOR_MEMORY_SIZE (SZ_8K)
 +#define GIC_CPU_INTERFACE_MEMORY_SIZE (SZ_4K)

Aside from the incorrect sizes, how do you plan to address the other
regions that the GICv2 specification describes?

  static int __init
  acpi_parse_gic_distributor(struct acpi_subtable_header *header,
   const unsigned long end)
  {
   struct acpi_madt_generic_distributor *distributor = NULL;
 + void __iomem *dist_base = NULL;
 + void __iomem *cpu_base = NULL;
  
   distributor = (struct acpi_madt_generic_distributor *)header;
  
 @@ -224,8 +235,43 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
 *header,
  
   acpi_table_print_madt_entry(header);
  
 + /* GIC is initialised after page_init(), no need for early_ioremap */
 + dist_base = ioremap(distributor-base_address,
 + GIC_CPU_INTERFACE_MEMORY_SIZE);
 + if (!dist_base) {
 + pr_warn(PREFIX unable to map gic dist registers\n);
 + return -ENOMEM;
 + }
 +
 + /*
 +  * acpi_lapic_addr is stored in acpi_parse_madt(),
 +  * so we can use it here for GIC init
 +  */
 + if (acpi_lapic_addr) {
 + iounmap(dist_base);
 + pr_warn(PREFIX Invalid GIC cpu interface base address\n);
 + return -EINVAL;
 + }
 +
 + cpu_base = ioremap(acpi_lapic_addr, GIC_CPU_INTERFACE_MEMORY_SIZE);
 + if (!cpu_base) {
 + iounmap(dist_base);
 + pr_warn(PREFIX unable to map gic cpu registers\n);
 + return -ENOMEM;
 + }
 +
 + gic_init(distributor-gic_id, -1, dist_base, cpu_base);
 +
   return 0;
  }
 +#else
 +static int __init
 +acpi_parse_gic_distributor(struct acpi_subtable_header *header,
 + const unsigned long end)
 +{
 + return -ENODEV;
 +}
 +#endif /* CONFIG_ARM_GIC */
  
  /*
   * Parse GIC cpu interface related entries in MADT
 @@ -234,7 +280,7 @@ acpi_parse_gic_distributor(struct acpi_subtable_header 
 *header,
  static int __init acpi_parse_madt_gic_entries(void)
  {
   int count;
 -
 + 
   /*
* do a partial walk of MADT to determine how many CPUs
* we have including disabled CPUs
 @@ -468,19 +514,21 @@ static void __init acpi_process_madt(void)
* Parse MADT GIC cpu interface entries
*/
   error = acpi_parse_madt_gic_entries();
 - if (!error) {
 - /*
 -  * Parse MADT GIC distributor entries
 -  */
 - acpi_parse_madt_gic_distributor_entries();
 - }
 + if (!error)
 + pr_info(Using ACPI for processor (GIC) configuration 
 information\n);
   }
  
 - pr_info(Using ACPI for processor (GIC) configuration information\n);
 -
   return;
  }
  
 +int __init acpi_gic_init(void)
 +{
 + /*
 +  * Parse MADT GIC distributor entries
 +  */
 + return acpi_parse_madt_gic_distributor_entries();
 +}
 +

Why can't you do the GIC init in the GIC code? We've tried hard to