diff mbox

[Xen-devel,RFC,03/35] xen: arm64: ACPI: Add basic ACPI initialization

Message ID 1423058539-26403-4-git-send-email-parth.dixit@linaro.org
State New
Headers show

Commit Message

Parth Dixit Feb. 4, 2015, 2:01 p.m. UTC 
From: Naresh Bhat <naresh.bhat@linaro.org>

This patch introduce arm-core.c and its related header file

- asm/acpi.h for arch specific variables and functions needed by
  ACPI driver core;
- arm-core.c for ARM64 related ACPI implementation for ACPI driver
  core;

acpi_boot_table_init() will be called in setup_arch()
to get the RSDP and all the table pointers. with this patch,
we can get ACPI boot-time tables from firmware on ARM64.

Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
---
 xen/arch/arm/arm64/Makefile        |  1 +
 xen/arch/arm/arm64/acpi/Makefile   |  1 +
 xen/arch/arm/arm64/acpi/arm-core.c | 97 ++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/setup.c               | 14 +++++-
 xen/include/xen/acpi.h             | 11 +++++
 5 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/arm/arm64/acpi/Makefile
 create mode 100644 xen/arch/arm/arm64/acpi/arm-core.c

Comments

Julien Grall Feb. 4, 2015, 9 p.m. UTC | #1 
Hi Parth,

On 04/02/2015 14:01, parth.dixit@linaro.org wrote:
> From: Naresh Bhat <naresh.bhat@linaro.org>
>
> This patch introduce arm-core.c and its related header file
>
> - asm/acpi.h for arch specific variables and functions needed by
>    ACPI driver core;
> - arm-core.c for ARM64 related ACPI implementation for ACPI driver
>    core;
>
> acpi_boot_table_init() will be called in setup_arch()
> to get the RSDP and all the table pointers. with this patch,
> we can get ACPI boot-time tables from firmware on ARM64.
>
> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
> ---
>   xen/arch/arm/arm64/Makefile        |  1 +
>   xen/arch/arm/arm64/acpi/Makefile   |  1 +
>   xen/arch/arm/arm64/acpi/arm-core.c | 97 ++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/setup.c               | 14 +++++-
>   xen/include/xen/acpi.h             | 11 +++++
>   5 files changed, 123 insertions(+), 1 deletion(-)
>   create mode 100644 xen/arch/arm/arm64/acpi/Makefile
>   create mode 100644 xen/arch/arm/arm64/acpi/arm-core.c
>
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index c7243f5..49d135f 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -1,4 +1,5 @@
>   subdir-y += lib
> +subdir-y += acpi

It would be more logic to include this directory only when HAS_ACPI is 
defined.

It would be smth like:

subdir-$(HAS_ACPI) += acpi

[...]

> diff --git a/xen/arch/arm/arm64/acpi/arm-core.c b/xen/arch/arm/arm64/acpi/arm-core.c

I think "arm" is pointless in the filename. Maybe core.c would be a 
better name?

> new file mode 100644
> index 0000000..50a83d6
> --- /dev/null
> +++ b/xen/arch/arm/arm64/acpi/arm-core.c
> @@ -0,0 +1,97 @@

[..]

> +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI)

Compiling the directory only when HAS_ACPI is enabled would avoid a 
pointless #ifdef here.

> +#include <xen/init.h>
> +#include <xen/acpi.h>
> +
> +#include <asm/acpi.h>

You should be consistent here, the acpi.h headers is in asm so generic 
to all ARM architecture but the ACPI code actually reside in an ARM64 
directory.

Even though ACPI is only ARM64, this code doesn't seem ARM64 specific. 
So I would move the directory in arch/arm/

> +
> +/*
> + * We never plan to use RSDT on arm/arm64 as its deprecated in spec but this
> + * variable is still required by the ACPI core
> + */
> +u32 acpi_rsdt_forced;

I didn't find any usage for this in Xen.

> +int acpi_noirq;                        /* skip ACPI IRQ initialization */

This is set but never used.

> +int acpi_strict;

I didn't find any usage for this in Xen.

> +int acpi_disabled;
> +EXPORT_SYMBOL(acpi_disabled);
> +
> +int acpi_pci_disabled;         /* skip ACPI PCI scan and IRQ initialization */
> +EXPORT_SYMBOL(acpi_pci_disabled);

This is set but never used.

> +enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM;

Why do you define acpi_irq_model to ACPI_IRQ_MODEL_PLATFORM here and in 
patch #15, change to ACPI_IRQ_MODEL_PLATFORM?

Furthermore, you set it, but never use it.

> +
> +struct acpi_arm_root acpi_arm_rsdp_info;     /* info about RSDP from FDT */

I didn't find any usage in Xen.

> +
> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
> +{
> +    *irq = -1;
> +    return 0;
> +}
> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);

This function is never used. I found a prototype defined in the headers 
(xen/acpi.h) few years ago but not implemented on x86.

I suspect that we forgot to drop the prototype at some point. Can you 
send a patch to remove it?

> +
> +/*
> + * success: return IRQ number (>0)
> + * failure: return =< 0
> + */
> +//int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
> +unsigned int acpi_register_gsi (u32 gsi, int edge_level, int active_high_low)
> +{
> +    return -1;
> +}
> +EXPORT_SYMBOL_GPL(acpi_register_gsi);
> +
> +void acpi_unregister_gsi(u32 gsi)
> +{
> +}
> +EXPORT_SYMBOL_GPL(acpi_unregister_gsi);

Ditto for these 2 functions.

> +/*
> + * acpi_boot_table_init() called from setup_arch(), always.
> + *      1. find RSDP and get its address, and then find XSDT
> + *      2. extract all tables and checksums them all
> + *
> + * We can parse ACPI boot-time tables such as FADT, MADT after
> + * this function is called.
> + */
> +int __init acpi_boot_table_init(void)
> +{
> +    int error;
> +    /* If acpi_disabled, bail out */
> +    if (acpi_disabled)
> +        return 1;
> +
> +    /* Initialize the ACPI boot-time table parser. */
> +    error = acpi_table_init();
> +    if (error)
> +    {
> +        disable_acpi();
> +        return error;
> +    }
> +
> +    return 0;
> +}
> +#endif
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 3991d64..7ae126b 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -45,6 +45,8 @@
>   #include <asm/procinfo.h>
>   #include <asm/setup.h>
>   #include <xsm/xsm.h>
> +#include <xen/acpi.h>
> +#include <asm/acpi.h>
>
>   struct bootinfo __initdata bootinfo;
>
> @@ -737,7 +739,18 @@ void __init start_xen(unsigned long boot_phys_offset,
>
>       setup_mm(fdt_paddr, fdt_size);
>
> +    system_state = SYS_STATE_boot;
> +

Hmmm why? Is it because of the memory allocator in ACPI? If so, you 
should justify a such change and drop the other system_state = 
SYS_STATE_boot within this function.

>       vm_init();
> +
> +/*
> + * Parse the ACPI tables for possible boot-time configuration
> + */
> +
> +#if defined(CONFIG_ACPI) && defined(CONFIG_ARM_64)

No need to check CONFIG_ARM_64 here, CONFIG_ACPI is only defined for ARM64.

Although, a stub exist when CONFIG_ACPI_BOOT (implicitly enabled with 
CONFIG_ACPI) is not enabled. So maybe we should just drop the #ifdef.

> +    acpi_boot_table_init();

Should not we return try to use the device tree when ACPI return an 
error. What about the Linux boot protocol? I.e prefer DT when it's present?

> +#endif
> +
>       dt_unflatten_host_device_tree();

>       dt_irq_xlate = gic_irq_xlate;

This 2 lines should not be necessary when ACPI is used. Please make sure 
that we never used the device tree in this case. This would help us to 
catch possible error later.

>
> @@ -802,7 +815,6 @@ void __init start_xen(unsigned long boot_phys_offset,
>                   printk("Failed to bring up CPU %u (error %d)\n", i, ret);
>           }
>       }
> -

Spurious change

>       printk("Brought up %ld CPUs\n", (long)num_online_cpus());
>       /* TODO: smp_cpus_done(); */
>
> diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
> index 3aeba4a..ff96336 100644
> --- a/xen/include/xen/acpi.h
> +++ b/xen/include/xen/acpi.h
> @@ -42,6 +42,17 @@
>
>   #ifdef CONFIG_ACPI_BOOT
>
> +enum acpi_irq_model_id {
> +        ACPI_IRQ_MODEL_PIC = 0,
> +        ACPI_IRQ_MODEL_IOAPIC,
> +        ACPI_IRQ_MODEL_IOSAPIC,
> +        ACPI_IRQ_MODEL_PLATFORM,
> +        ACPI_IRQ_MODEL_GIC,
> +        ACPI_IRQ_MODEL_COUNT
> +};
> +
> +extern enum acpi_irq_model_id   acpi_irq_model;
> +

If we still need it (see my comment on the declaration above). Then this 
should be moved in asm/acpi.h.

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index c7243f5..49d135f 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -1,4 +1,5 @@ 
 subdir-y += lib
+subdir-y += acpi
 
 obj-y += entry.o
 
diff --git a/xen/arch/arm/arm64/acpi/Makefile b/xen/arch/arm/arm64/acpi/Makefile
new file mode 100644
index 0000000..07e3c44
--- /dev/null
+++ b/xen/arch/arm/arm64/acpi/Makefile
@@ -0,0 +1 @@ 
+obj-y          += arm-core.o
diff --git a/xen/arch/arm/arm64/acpi/arm-core.c b/xen/arch/arm/arm64/acpi/arm-core.c
new file mode 100644
index 0000000..50a83d6
--- /dev/null
+++ b/xen/arch/arm/arm64/acpi/arm-core.c
@@ -0,0 +1,97 @@ 
+/*
+ *  ARM64 Specific Low-Level ACPI Boot Support
+ *
+ *  Copyright (C) 2014, Naresh Bhat <naresh.bhat@linaro.org>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI)
+#include <xen/init.h>
+#include <xen/acpi.h>
+
+#include <asm/acpi.h>
+
+/*
+ * We never plan to use RSDT on arm/arm64 as its deprecated in spec but this
+ * variable is still required by the ACPI core
+ */
+u32 acpi_rsdt_forced;
+
+int acpi_noirq;                        /* skip ACPI IRQ initialization */
+int acpi_strict;
+int acpi_disabled;
+EXPORT_SYMBOL(acpi_disabled);
+
+int acpi_pci_disabled;         /* skip ACPI PCI scan and IRQ initialization */
+EXPORT_SYMBOL(acpi_pci_disabled);
+
+enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM;
+
+struct acpi_arm_root acpi_arm_rsdp_info;     /* info about RSDP from FDT */
+
+int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
+{
+    *irq = -1;
+    return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
+
+/*
+ * success: return IRQ number (>0)
+ * failure: return =< 0
+ */
+//int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
+unsigned int acpi_register_gsi (u32 gsi, int edge_level, int active_high_low)
+{
+    return -1;
+}
+EXPORT_SYMBOL_GPL(acpi_register_gsi);
+
+void acpi_unregister_gsi(u32 gsi)
+{
+}
+EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
+
+/*
+ * acpi_boot_table_init() called from setup_arch(), always.
+ *      1. find RSDP and get its address, and then find XSDT
+ *      2. extract all tables and checksums them all
+ *
+ * We can parse ACPI boot-time tables such as FADT, MADT after
+ * this function is called.
+ */
+int __init acpi_boot_table_init(void)
+{
+    int error;
+    /* If acpi_disabled, bail out */
+    if (acpi_disabled)
+        return 1;
+
+    /* Initialize the ACPI boot-time table parser. */
+    error = acpi_table_init();
+    if (error)
+    {
+        disable_acpi();
+        return error;
+    }
+
+    return 0;
+}
+#endif
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 3991d64..7ae126b 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -45,6 +45,8 @@ 
 #include <asm/procinfo.h>
 #include <asm/setup.h>
 #include <xsm/xsm.h>
+#include <xen/acpi.h>
+#include <asm/acpi.h>
 
 struct bootinfo __initdata bootinfo;
 
@@ -737,7 +739,18 @@  void __init start_xen(unsigned long boot_phys_offset,
 
     setup_mm(fdt_paddr, fdt_size);
 
+    system_state = SYS_STATE_boot;
+
     vm_init();
+
+/*
+ * Parse the ACPI tables for possible boot-time configuration
+ */
+
+#if defined(CONFIG_ACPI) && defined(CONFIG_ARM_64)
+    acpi_boot_table_init();
+#endif
+
     dt_unflatten_host_device_tree();
     dt_irq_xlate = gic_irq_xlate;
 
@@ -802,7 +815,6 @@  void __init start_xen(unsigned long boot_phys_offset,
                 printk("Failed to bring up CPU %u (error %d)\n", i, ret);
         }
     }
-
     printk("Brought up %ld CPUs\n", (long)num_online_cpus());
     /* TODO: smp_cpus_done(); */
 
diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
index 3aeba4a..ff96336 100644
--- a/xen/include/xen/acpi.h
+++ b/xen/include/xen/acpi.h
@@ -42,6 +42,17 @@ 
 
 #ifdef CONFIG_ACPI_BOOT
 
+enum acpi_irq_model_id {
+        ACPI_IRQ_MODEL_PIC = 0,
+        ACPI_IRQ_MODEL_IOAPIC,
+        ACPI_IRQ_MODEL_IOSAPIC,
+        ACPI_IRQ_MODEL_PLATFORM,
+        ACPI_IRQ_MODEL_GIC,
+        ACPI_IRQ_MODEL_COUNT
+};
+
+extern enum acpi_irq_model_id   acpi_irq_model;
+
 enum acpi_interrupt_id {
 	ACPI_INTERRUPT_PMI	= 1,
 	ACPI_INTERRUPT_INIT,