Add ACPI support for arm64 on Xen

From: Parth Dixit <[email protected]>

Parse GTDT (Generic Timer Descriptor Table) to initialize timer. Using the information presented by GTDT to initialize the arch timer (not memory-mapped).

Signed-off-by Naresh Bhat <[email protected]>

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/arch/arm/time.c | 73 +++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 11 deletions(-)

xen/arch/arm/time.c +21 -0

index 5ded30c..adbfc2a 100644
@@ -29,6 +29,7 @@
 #include <xen/time.h>
 #include <xen/sched.h>
 #include <xen/event.h>
+#include <xen/acpi.h>
 #include <asm/system.h>
 #include <asm/time.h>
 #include <asm/gic.h>

@@ -65,8 +66,45 @@ unsigned int timer_get_irq(enum timer_ppi ppi)
 
 static __initdata struct dt_device_node *timer;
 
+#ifdef CONFIG_ACPI
+/* Initialize per-processor generic timer */
+static int __init arch_timer_acpi_init(struct acpi_table_header *table)
+{
+    unsigned int irq_type;
+    struct acpi_table_gtdt *gtdt;
+
+    gtdt = container_of(table, struct acpi_table_gtdt, header);
+
+    /* Initialize all the generic timer IRQ variable from GTDT table */
+    irq_type = acpi_get_irq_type(gtdt->non_secure_el1_flags);
+    irq_set_type(gtdt->non_secure_el1_interrupt, irq_type);
+    timer_irq[TIMER_PHYS_NONSECURE_PPI] = gtdt->non_secure_el1_interrupt;
+
+    irq_type = acpi_get_irq_type(gtdt->secure_el1_flags);
+    irq_set_type(gtdt->secure_el1_interrupt, irq_type);
+    timer_irq[TIMER_PHYS_SECURE_PPI] = gtdt->secure_el1_interrupt;
+
+    irq_type = acpi_get_irq_type(gtdt->virtual_timer_flags);
+    irq_set_type(gtdt->virtual_timer_interrupt, irq_type);

xen/arch/arm/time.c +8 -2

+{
+	acpi_table_parse(ACPI_SIG_GTDT, arch_timer_acpi_init);
+}
+#else
+static inline void acpi_preinit_xen_time(void) { }
+#endif
+
 /* Set up the timer on the boot CPU (early init function) */
-void __init preinit_xen_time(void)
+static void __init dt_preinit_xen_time(void)
 {
     static const struct dt_device_match timer_ids[] __initconst =
     {

@@ -74,7 +112,6 @@ void __init preinit_xen_time(void)
         { /* sentinel */ },
     };
     int res;
-    u32 rate;
 
     timer = dt_find_matching_node(NULL, timer_ids);
     if ( !timer )

xen/arch/arm/time.c +6 -0

+}
+
+void __init preinit_xen_time(void)
+{
+    u32 rate;
+

xen/arch/arm/time.c +1 -7

@@ -104,14 +152,17 @@ int __init init_xen_time(void)
     int res;
     unsigned int i;
 
-    /* Retrieve all IRQs for the timer */
-    for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
+    if( acpi_disabled )
     {
-        res = platform_get_irq(timer, i);
-
-        if ( res < 0 )
-            panic("Timer: Unable to retrieve IRQ %u from the device tree", i);
-        timer_irq[i] = res;

xen/arch/arm/time.c +0 -0

     /* Check that this CPU supports the Generic Timer interface */

Signature · 7 lines

2.1.0


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

From: Parth Dixit <[email protected]>

Rename dt-uart.c to arm-uart.c and create new generic uart init function. Move dt_uart_init to arm_uart_init.

Hi Shannon,

Quoted 4 lines · shannon.zhao at linaro.org

From: Parth Dixit <[email protected]>

Rename dt-uart.c to arm-uart.c and create new generic uart init function. Move dt_uart_init to arm_uart_init.

It's very hard to review a patch to move existing code and add new one. Please split it in 2.

Hi Shannon,

Quoted 4 lines · On 17/11/15 09:40, [email protected] wrote:

From: Parth Dixit <[email protected]>

Rename dt-uart.c to arm-uart.c and create new generic uart init function. Move dt_uart_init to arm_uart_init.

It's very hard to review a patch to move existing code and add new one. Please split it in 2.

From: Parth Dixit <[email protected]>

Rename dt-uart.c to arm-uart.c and create new generic uart init function. Move dt_uart_init to arm_uart_init.

From: Shannon Zhao <[email protected]>

Refactor pl011 driver to dt and common initialization parts. This will be useful later when acpi specific uart initialization function is introduced.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

 xen/drivers/char/pl011.c | 64 ++++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 26 deletions(-)

xen/drivers/char/pl011.c +38 -26

index 67e6df5..16abaa5 100644
@@ -226,12 +226,42 @@ static struct uart_driver __read_mostly pl011_driver = {
     .vuart_info   = pl011_vuart,
 };
 
+static int __init pl011_uart_init(int irq, u64 addr, u64 size)
+{
+    struct pl011 *uart;
+
+    uart = &pl011_com;
+    uart->irq       = irq;
+    uart->clock_hz  = 0x16e3600;
+    uart->baud      = BAUD_AUTO;
+    uart->data_bits = 8;
+    uart->parity    = PARITY_NONE;
+    uart->stop_bits = 1;
+
+    uart->regs = ioremap_nocache(addr, size);
+    if ( !uart->regs )
+    {
+        printk("pl011: Unable to map the UART memory\n");
+        return -ENOMEM;
+    }
+
+    uart->vuart.base_addr = addr;
+    uart->vuart.size = size;
+    uart->vuart.data_off = DR;
+    uart->vuart.status_off = FR;
+    uart->vuart.status = 0;
+
+    /* Register with generic serial driver. */
+    serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
+
+    return 0;
+}
+
 /* TODO: Parse UART config from the command line */
-static int __init pl011_uart_init(struct dt_device_node *dev,
-                                  const void *data)
+static int __init dt_pl011_uart_init(struct dt_device_node *dev,
+                                     const void *data)
 {
     const char *config = data;
-    struct pl011 *uart;
     int res;
     u64 addr, size;
 

@@ -240,14 +270,6 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         printk("WARNING: UART configuration is not supported\n");
     }
 
-    uart = &pl011_com;
-
-    uart->clock_hz  = 0x16e3600;
-    uart->baud      = BAUD_AUTO;
-    uart->data_bits = 8;
-    uart->parity    = PARITY_NONE;
-    uart->stop_bits = 1;
-
     res = dt_device_get_address(dev, 0, &addr, &size);
     if ( res )
     {

@@ -262,24 +284,14 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         printk("pl011: Unable to retrieve the IRQ\n");
         return -EINVAL;
     }
-    uart->irq = res;
 
-    uart->regs = ioremap_nocache(addr, size);
-    if ( !uart->regs )
+    res = pl011_uart_init(res, addr, size);
+    if ( res < 0 )
     {
-        printk("pl011: Unable to map the UART memory\n");
-        return -ENOMEM;
+        printk("pl011: Unable to initialize\n");
+        return res;
     }
 
-    uart->vuart.base_addr = addr;
-    uart->vuart.size = size;
-    uart->vuart.data_off = DR;
-    uart->vuart.status_off = FR;
-    uart->vuart.status = 0;
-
-    /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
-
     dt_device_set_used_by(dev, DOMID_XEN);
 
     return 0;

@@ -293,7 +305,7 @@ static const struct dt_device_match pl011_dt_match[] __initconst =
 
 DT_DEVICE_START(pl011, "PL011 UART", DEVICE_SERIAL)
         .dt_match = pl011_dt_match,
-        .init = pl011_uart_init,
+        .init = dt_pl011_uart_init,
 DT_DEVICE_END
 
 /*

Signature · 1 line

2.1.0

From: Shannon Zhao <[email protected]>

Refactor pl011 driver to dt and common initialization parts. This will be useful later when acpi specific uart initialization function is introduced.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

 xen/drivers/char/pl011.c | 64 ++++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 26 deletions(-)

xen/drivers/char/pl011.c +38 -26

index 67e6df5..16abaa5 100644
@@ -226,12 +226,42 @@ static struct uart_driver __read_mostly pl011_driver = {
     .vuart_info   = pl011_vuart,
 };
 
+static int __init pl011_uart_init(int irq, u64 addr, u64 size)
+{
+    struct pl011 *uart;
+
+    uart = &pl011_com;
+    uart->irq       = irq;
+    uart->clock_hz  = 0x16e3600;
+    uart->baud      = BAUD_AUTO;
+    uart->data_bits = 8;
+    uart->parity    = PARITY_NONE;
+    uart->stop_bits = 1;
+
+    uart->regs = ioremap_nocache(addr, size);
+    if ( !uart->regs )
+    {
+        printk("pl011: Unable to map the UART memory\n");
+        return -ENOMEM;
+    }
+
+    uart->vuart.base_addr = addr;
+    uart->vuart.size = size;
+    uart->vuart.data_off = DR;
+    uart->vuart.status_off = FR;
+    uart->vuart.status = 0;
+
+    /* Register with generic serial driver. */
+    serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
+
+    return 0;
+}
+
 /* TODO: Parse UART config from the command line */
-static int __init pl011_uart_init(struct dt_device_node *dev,
-                                  const void *data)
+static int __init dt_pl011_uart_init(struct dt_device_node *dev,
+                                     const void *data)
 {
     const char *config = data;
-    struct pl011 *uart;
     int res;
     u64 addr, size;
 

@@ -240,14 +270,6 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         printk("WARNING: UART configuration is not supported\n");
     }
 
-    uart = &pl011_com;
-
-    uart->clock_hz  = 0x16e3600;
-    uart->baud      = BAUD_AUTO;
-    uart->data_bits = 8;
-    uart->parity    = PARITY_NONE;
-    uart->stop_bits = 1;
-
     res = dt_device_get_address(dev, 0, &addr, &size);
     if ( res )
     {

@@ -262,24 +284,14 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         printk("pl011: Unable to retrieve the IRQ\n");
         return -EINVAL;
     }
-    uart->irq = res;
 
-    uart->regs = ioremap_nocache(addr, size);
-    if ( !uart->regs )
+    res = pl011_uart_init(res, addr, size);
+    if ( res < 0 )
     {
-        printk("pl011: Unable to map the UART memory\n");
-        return -ENOMEM;
+        printk("pl011: Unable to initialize\n");
+        return res;
     }
 
-    uart->vuart.base_addr = addr;
-    uart->vuart.size = size;
-    uart->vuart.data_off = DR;
-    uart->vuart.status_off = FR;
-    uart->vuart.status = 0;
-
-    /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
-
     dt_device_set_used_by(dev, DOMID_XEN);
 
     return 0;

@@ -293,7 +305,7 @@ static const struct dt_device_match pl011_dt_match[] __initconst =
 
 DT_DEVICE_START(pl011, "PL011 UART", DEVICE_SERIAL)
         .dt_match = pl011_dt_match,
-        .init = pl011_uart_init,
+        .init = dt_pl011_uart_init,
 DT_DEVICE_END
 
 /*

Signature · 1 line

2.1.0

From: Shannon Zhao <[email protected]>

Rename SERHND_DTUART to SERHND_UART in order to reuse it when booting with ACPI.

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/arch/arm/vuart.c               | 2 +-
 xen/drivers/char/cadence-uart.c    | 2 +-
 xen/drivers/char/exynos4210-uart.c | 2 +-
 xen/drivers/char/omap-uart.c       | 2 +-
 xen/drivers/char/pl011.c           | 2 +-
 xen/drivers/char/scif-uart.c       | 2 +-
 xen/drivers/char/serial.c          | 2 +-
 xen/include/xen/serial.h           | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

xen/arch/arm/vuart.c +1 -1

index b5c9288..6619b33 100644
@@ -59,7 +59,7 @@ int domain_vuart_init(struct domain *d)
 {
     ASSERT( is_hardware_domain(d) );
 
-    d->arch.vuart.info = serial_vuart_info(SERHND_DTUART);
+    d->arch.vuart.info = serial_vuart_info(SERHND_UART);
     if ( !d->arch.vuart.info )
         return 0;
 

xen/drivers/char/cadence-uart.c +1 -1

index 933672f..ed65004 100644
@@ -196,7 +196,7 @@ static int __init cuart_init(struct dt_device_node *dev, const void *data)
     uart->vuart.status = UART_SR_INTR_TEMPTY;
 
     /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &cuart_driver, uart);
+    serial_register_uart(SERHND_UART, &cuart_driver, uart);
 
     dt_device_set_used_by(dev, DOMID_XEN);
 

xen/drivers/char/exynos4210-uart.c +1 -1

index bac1c2b..43f5313 100644
@@ -345,7 +345,7 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
     uart->vuart.status = UTRSTAT_TXE | UTRSTAT_TXFE;
 
     /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &exynos4210_uart_driver, uart);
+    serial_register_uart(SERHND_UART, &exynos4210_uart_driver, uart);
 
     dt_device_set_used_by(dev, DOMID_XEN);
 

xen/drivers/char/omap-uart.c +1 -1

index d8f64ea..98c3892 100644
@@ -343,7 +343,7 @@ static int __init omap_uart_init(struct dt_device_node *dev,
     uart->vuart.status = UART_LSR_THRE;
 
     /* Register with generic serial driver */
-    serial_register_uart(SERHND_DTUART, &omap_uart_driver, uart);
+    serial_register_uart(SERHND_UART, &omap_uart_driver, uart);
 
     dt_device_set_used_by(dev, DOMID_XEN);
 

xen/drivers/char/pl011.c +1 -1

index 16abaa5..1efb1cf 100644
@@ -252,7 +252,7 @@ static int __init pl011_uart_init(int irq, u64 addr, u64 size)
     uart->vuart.status = 0;
 
     /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
+    serial_register_uart(SERHND_UART, &pl011_driver, uart);
 
     return 0;
 }

xen/drivers/char/scif-uart.c +1 -1

index 51a2233..739317c 100644
@@ -339,7 +339,7 @@ static int __init scif_uart_init(struct dt_device_node *dev,
     uart->vuart.status     = SCFSR_TDFE;
 
     /* Register with generic serial driver */
-    serial_register_uart(SERHND_DTUART, &scif_uart_driver, uart);
+    serial_register_uart(SERHND_UART, &scif_uart_driver, uart);
 
     dt_device_set_used_by(dev, DOMID_XEN);
 

xen/drivers/char/serial.c +1 -1

index c583a48..659e10d 100644
@@ -312,7 +312,7 @@ int __init serial_parse_handle(char *conf)
 
     if ( !strncmp(conf, "dtuart", 5) )
     {
-        handle = SERHND_DTUART;
+        handle = SERHND_UART;
         goto common;
     }
 

xen/include/xen/serial.h +1 -1

index 5e1a536..a6e7c22 100644
@@ -94,7 +94,7 @@ struct uart_driver {
 # define SERHND_COM1    (0<<0)
 # define SERHND_COM2    (1<<0)
 # define SERHND_DBGP    (2<<0)
-# define SERHND_DTUART  (0<<0) /* Steal SERHND_COM1 value */
+# define SERHND_UART    (0<<0) /* Steal SERHND_COM1 value */
 #define SERHND_HI       (1<<2) /* Mux/demux each transferred char by MSB. */
 #define SERHND_LO       (1<<3) /* Ditto, except that the MSB is cleared.  */
 #define SERHND_COOKED   (1<<4) /* Newline/carriage-return translation?    */

Signature · 7 lines

2.1.0


_______________________________________________
Xen-devel mailing list
Xen-devel at lists.xen.org
http://lists.xen.org/xen-devel

From: Shannon Zhao <[email protected]>

Rename SERHND_DTUART to SERHND_UART in order to reuse it when booting with ACPI.

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/arch/arm/vuart.c               | 2 +-
 xen/drivers/char/cadence-uart.c    | 2 +-
 xen/drivers/char/exynos4210-uart.c | 2 +-
 xen/drivers/char/omap-uart.c       | 2 +-
 xen/drivers/char/pl011.c           | 2 +-
 xen/drivers/char/scif-uart.c       | 2 +-
 xen/drivers/char/serial.c          | 2 +-
 xen/include/xen/serial.h           | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

xen/arch/arm/vuart.c +1 -1

index b5c9288..6619b33 100644
@@ -59,7 +59,7 @@ int domain_vuart_init(struct domain *d)
 {
     ASSERT( is_hardware_domain(d) );
 
-    d->arch.vuart.info = serial_vuart_info(SERHND_DTUART);
+    d->arch.vuart.info = serial_vuart_info(SERHND_UART);
     if ( !d->arch.vuart.info )
         return 0;
 

xen/drivers/char/cadence-uart.c +1 -1

index 933672f..ed65004 100644
@@ -196,7 +196,7 @@ static int __init cuart_init(struct dt_device_node *dev, const void *data)
     uart->vuart.status = UART_SR_INTR_TEMPTY;
 
     /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &cuart_driver, uart);
+    serial_register_uart(SERHND_UART, &cuart_driver, uart);
 
     dt_device_set_used_by(dev, DOMID_XEN);
 

xen/drivers/char/exynos4210-uart.c +1 -1

index bac1c2b..43f5313 100644
@@ -345,7 +345,7 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
     uart->vuart.status = UTRSTAT_TXE | UTRSTAT_TXFE;
 
     /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &exynos4210_uart_driver, uart);
+    serial_register_uart(SERHND_UART, &exynos4210_uart_driver, uart);
 
     dt_device_set_used_by(dev, DOMID_XEN);
 

xen/drivers/char/omap-uart.c +1 -1

index d8f64ea..98c3892 100644
@@ -343,7 +343,7 @@ static int __init omap_uart_init(struct dt_device_node *dev,
     uart->vuart.status = UART_LSR_THRE;
 
     /* Register with generic serial driver */
-    serial_register_uart(SERHND_DTUART, &omap_uart_driver, uart);
+    serial_register_uart(SERHND_UART, &omap_uart_driver, uart);
 
     dt_device_set_used_by(dev, DOMID_XEN);
 

xen/drivers/char/pl011.c +1 -1

index 16abaa5..1efb1cf 100644
@@ -252,7 +252,7 @@ static int __init pl011_uart_init(int irq, u64 addr, u64 size)
     uart->vuart.status = 0;
 
     /* Register with generic serial driver. */
-    serial_register_uart(SERHND_DTUART, &pl011_driver, uart);
+    serial_register_uart(SERHND_UART, &pl011_driver, uart);
 
     return 0;
 }

xen/drivers/char/scif-uart.c +1 -1

index 51a2233..739317c 100644
@@ -339,7 +339,7 @@ static int __init scif_uart_init(struct dt_device_node *dev,
     uart->vuart.status     = SCFSR_TDFE;
 
     /* Register with generic serial driver */
-    serial_register_uart(SERHND_DTUART, &scif_uart_driver, uart);
+    serial_register_uart(SERHND_UART, &scif_uart_driver, uart);
 
     dt_device_set_used_by(dev, DOMID_XEN);
 

xen/drivers/char/serial.c +1 -1

index c583a48..659e10d 100644
@@ -312,7 +312,7 @@ int __init serial_parse_handle(char *conf)
 
     if ( !strncmp(conf, "dtuart", 5) )
     {
-        handle = SERHND_DTUART;
+        handle = SERHND_UART;
         goto common;
     }
 

xen/include/xen/serial.h +1 -1

index 5e1a536..a6e7c22 100644
@@ -94,7 +94,7 @@ struct uart_driver {
 # define SERHND_COM1    (0<<0)
 # define SERHND_COM2    (1<<0)
 # define SERHND_DBGP    (2<<0)
-# define SERHND_DTUART  (0<<0) /* Steal SERHND_COM1 value */
+# define SERHND_UART    (0<<0) /* Steal SERHND_COM1 value */
 #define SERHND_HI       (1<<2) /* Mux/demux each transferred char by MSB. */
 #define SERHND_LO       (1<<3) /* Ditto, except that the MSB is cleared.  */
 #define SERHND_COOKED   (1<<4) /* Newline/carriage-return translation?    */

Signature · 7 lines

2.1.0


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

From: Parth Dixit <[email protected]>

Parse ACPI SPCR (Serial Port Console Redirection table) table and initialize the serial port pl011.

Signed-off-by Naresh Bhat <[email protected]>

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/drivers/char/pl011.c  | 37 +++++++++++++++++++++++++++++++++++++
 xen/include/acpi/actbl2.h |  5 +++++
 2 files changed, 42 insertions(+)

xen/drivers/char/pl011.c +1 -0

index 1efb1cf..7106f1c 100644
@@ -27,6 +27,7 @@
 #include <asm/device.h>
 #include <xen/mm.h>
 #include <xen/vmap.h>
+#include <xen/acpi.h>
 #include <asm/pl011-uart.h>
 #include <asm/io.h>
 

xen/drivers/char/pl011.c +31 -0

+    int res;
+
+    status = acpi_get_table(ACPI_SIG_SPCR, 0,
+                            (struct acpi_table_header **)&spcr);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        printk("pl011: Failed to get SPCR table\n");
+        return -EINVAL;
+    }
+
+    /* trigger/polarity information is not available in spcr */
+    irq_set_type(spcr->interrupt, ACPI_IRQ_TYPE_EDGE_BOTH);
+
+    res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address,
+                          PAGE_SIZE);
+    if ( res < 0 )
+    {
+        printk("pl011: Unable to initialize\n");
+        return res;
+    }
+
+    return 0;
+}
+
+ACPI_DEVICE_START(apl011, "PL011 UART", DEVICE_SERIAL)
+        .class_type = ACPI_SPCR_TYPE_PL011,
+        .init = acpi_pl011_uart_init,
+ACPI_DEVICE_END
+#endif
+
 /*
  * Local variables:
  * mode: C

xen/include/acpi/actbl2.h +0 -0

index 87bc6b3..25be429 100644

xen/include/acpi/actbl2.h +0 -0

 /*******************************************************************************
  *
  * SPMI - Server Platform Management Interface table

Signature · 1 line

2.1.0

From: Parth Dixit <[email protected]>

Parse ACPI SPCR (Serial Port Console Redirection table) table and initialize the serial port pl011.

Signed-off-by Naresh Bhat <[email protected]>

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/drivers/char/pl011.c  | 37 +++++++++++++++++++++++++++++++++++++
 xen/include/acpi/actbl2.h |  5 +++++
 2 files changed, 42 insertions(+)

xen/drivers/char/pl011.c +1 -0

index 1efb1cf..7106f1c 100644
@@ -27,6 +27,7 @@
 #include <asm/device.h>
 #include <xen/mm.h>
 #include <xen/vmap.h>
+#include <xen/acpi.h>
 #include <asm/pl011-uart.h>
 #include <asm/io.h>
 

xen/drivers/char/pl011.c +31 -0

+    int res;
+
+    status = acpi_get_table(ACPI_SIG_SPCR, 0,
+                            (struct acpi_table_header **)&spcr);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        printk("pl011: Failed to get SPCR table\n");
+        return -EINVAL;
+    }
+
+    /* trigger/polarity information is not available in spcr */
+    irq_set_type(spcr->interrupt, ACPI_IRQ_TYPE_EDGE_BOTH);
+
+    res = pl011_uart_init(spcr->interrupt, spcr->serial_port.address,
+                          PAGE_SIZE);
+    if ( res < 0 )
+    {
+        printk("pl011: Unable to initialize\n");
+        return res;
+    }
+
+    return 0;
+}
+
+ACPI_DEVICE_START(apl011, "PL011 UART", DEVICE_SERIAL)
+        .class_type = ACPI_SPCR_TYPE_PL011,
+        .init = acpi_pl011_uart_init,
+ACPI_DEVICE_END
+#endif
+
 /*
  * Local variables:
  * mode: C

xen/include/acpi/actbl2.h +0 -0

index 87bc6b3..25be429 100644

xen/include/acpi/actbl2.h +0 -0

 /*******************************************************************************
  *
  * SPMI - Server Platform Management Interface table

Signature · 1 line

2.1.0

From: Shannon Zhao <[email protected]>

It needs to copy and change the contents of some ACPI and EFI tables for Dom0. Here define a enum for those tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/acpi.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

xen/include/asm-arm/acpi.h +1 -0

index 214fc4e..f159d51 100644
@@ -30,6 +30,18 @@
 
 extern bool_t acpi_disabled;
 
+/* Tables marked as reserved in efi table */

xen/include/asm-arm/acpi.h +0 -0

 #ifdef CONFIG_ACPI
 bool_t __init acpi_psci_present(void);
 bool_t __init acpi_psci_hvc_present(void);

Signature · 7 lines

2.1.0


_______________________________________________
Xen-devel mailing list
Xen-devel at lists.xen.org
http://lists.xen.org/xen-devel

Quoted 19 lines · shannon.zhao at linaro.org

From: Shannon Zhao <[email protected]>

It needs to copy and change the contents of some ACPI and EFI tables for Dom0. Here define a enum for those tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/acpi.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

xen/include/asm-arm/acpi.h +0 -0

index 214fc4e..f159d51 100644
@@ -30,6 +30,18 @@
 
 extern bool_t acpi_disabled;
 

xen/include/asm-arm/acpi.h line 43

Quoted 11 lines

xen/include/asm-arm/acpi.h +11 -0

+/* Tables marked as reserved in efi table */
+typedef enum {
+    TBL_FADT,
+    TBL_MADT,
+    TBL_STAO,
+    TBL_XSDT,
+    TBL_RSDP,
+    TBL_EFIT,
+    TBL_MMAP,
+    TBL_MMAX,
+} EFI_MEM_RES;

Given that in later patches you are using the enum as integer, you should initialize each value explicitly.

Quoted 12 lines

xen/include/asm-arm/acpi.h +0 -0

 #ifdef CONFIG_ACPI
 bool_t __init acpi_psci_present(void);
 bool_t __init acpi_psci_hvc_present(void);

Signature · 7 lines

2.1.0


_______________________________________________
Xen-devel mailing list
Xen-devel at lists.xen.org
http://lists.xen.org/xen-devel

Quoted 19 lines · On Tue, 17 Nov 2015, [email protected] wrote:

From: Shannon Zhao <[email protected]>

It needs to copy and change the contents of some ACPI and EFI tables for Dom0. Here define a enum for those tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/acpi.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

xen/include/asm-arm/acpi.h +0 -0

index 214fc4e..f159d51 100644
@@ -30,6 +30,18 @@
 
 extern bool_t acpi_disabled;
 

xen/include/asm-arm/acpi.h line 43

Quoted 11 lines

xen/include/asm-arm/acpi.h +11 -0

+/* Tables marked as reserved in efi table */
+typedef enum {
+    TBL_FADT,
+    TBL_MADT,
+    TBL_STAO,
+    TBL_XSDT,
+    TBL_RSDP,
+    TBL_EFIT,
+    TBL_MMAP,
+    TBL_MMAX,
+} EFI_MEM_RES;

Given that in later patches you are using the enum as integer, you should initialize each value explicitly.

Quoted 12 lines

xen/include/asm-arm/acpi.h +0 -0

 #ifdef CONFIG_ACPI
 bool_t __init acpi_psci_present(void);
 bool_t __init acpi_psci_hvc_present(void);

Signature · 7 lines

2.1.0


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

From: Shannon Zhao <[email protected]>

It needs to copy and change the contents of some ACPI and EFI tables for Dom0. Here define a enum for those tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/acpi.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

xen/include/asm-arm/acpi.h +1 -0

index 214fc4e..f159d51 100644
@@ -30,6 +30,18 @@
 
 extern bool_t acpi_disabled;
 
+/* Tables marked as reserved in efi table */

xen/include/asm-arm/acpi.h +0 -0

 #ifdef CONFIG_ACPI
 bool_t __init acpi_psci_present(void);
 bool_t __init acpi_psci_hvc_present(void);

Signature · 7 lines

2.1.0


_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644

Hi Shannon,

Quoted 20 lines · shannon.zhao at linaro.org

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644
@@ -125,6 +125,11 @@ struct arch_domain
     } vuart;
 

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 6 lines

xen/include/asm-arm/domain.h +5 -0

     unsigned int evtchn_irq;
+#ifdef CONFIG_ACPI
+    void *efi_acpi_table;
+    paddr_t efi_acpi_gpa;
+    paddr_t efi_acpi_len;
+#endif

Why do you need this in the arch_domain? AFAICT those 3 variables should only be used while building DOM0, so a better place would be in struct kernel_info (see xen/arch/arm/kernel.h).

Quoted 23 lines · Julien Grall

Hi Shannon,

Quoted 20 lines · shannon.zhao at linaro.org

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644
@@ -125,6 +125,11 @@ struct arch_domain
     } vuart;
 

Quoted 11 lines

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 6 lines

xen/include/asm-arm/domain.h +5 -0

     unsigned int evtchn_irq;
+#ifdef CONFIG_ACPI
+    void *efi_acpi_table;
+    paddr_t efi_acpi_gpa;
+    paddr_t efi_acpi_len;
+#endif

Why do you need this in the arch_domain? AFAICT those 3 variables should only be used while building DOM0, so a better place would be in struct kernel_info (see xen/arch/arm/kernel.h).

Since I use efi_acpi_table to store the allocated pages pointer which are used to store newly created ACPI and EFI tables.
See [PATCH v3 40/62]:

(quoted diff) +2 -0

+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);

And it free these pages when destroying this domain:

Hi Shannon,

Quoted 44 lines · Shannon Zhao

Quoted 23 lines · Julien Grall

Hi Shannon,

Quoted 20 lines · shannon.zhao at linaro.org

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644
@@ -125,6 +125,11 @@ struct arch_domain
     } vuart;
 

Quoted 11 lines

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 6 lines

xen/include/asm-arm/domain.h +5 -0

     unsigned int evtchn_irq;
+#ifdef CONFIG_ACPI
+    void *efi_acpi_table;
+    paddr_t efi_acpi_gpa;
+    paddr_t efi_acpi_len;
+#endif

Why do you need this in the arch_domain? AFAICT those 3 variables should only be used while building DOM0, so a better place would be in struct kernel_info (see xen/arch/arm/kernel.h).

Since I use efi_acpi_table to store the allocated pages pointer which are used to store newly created ACPI and EFI tables.
See [PATCH v3 40/62]:

(quoted diff) +2 -0

+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 5 lines

And it free these pages when destroying this domain:

xen/include/asm-arm/domain.h +4 -0

+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif

But this is wrong. You are assuming that DOM0 memory is direct mapped which may not be true in the future.

"All above tables will be mapped to Dom0 non-RAM space. Since when booting through ACPI it doesn't need the grant table region(see below section 3), it could use this region to store the tables or use the same way to find one memory region to store them."

Firstly, as Jan suggested, these tables should not be in RAM space, so we drop the previous way that copying these tables to Dom0 RAM. Then I suggested map these tables to the space after the Dom0 RAM space, but this not right because Dom0 RAM region might be at the edge of physical RAM space and there might be device MMIO regions. Then you suggest it could map these tables to the region which is used for grant table(or the region found by the same way) while it's not used when it boots with ACPI. These regions are not used by Xen and will not be used by Dom0 either currently. But as you say, it will be wrong if Dom0 memory is not 1:1 mapped.

So how about below idea:
We still copy these tables to Dom0 RAM space but when we create EFI_MMAP_TABLE, we remove the space occupied by these tables from the EfiConventionalMemory descriptor.

Quoted 5 lines · Shannon Zhao

"All above tables will be mapped to Dom0 non-RAM space. Since when booting through ACPI it doesn't need the grant table region(see below section 3), it could use this region to store the tables or use the same way to find one memory region to store them."

Quoted 10 lines

Firstly, as Jan suggested, these tables should not be in RAM space, so we drop the previous way that copying these tables to Dom0 RAM. Then I suggested map these tables to the space after the Dom0 RAM space, but this not right because Dom0 RAM region might be at the edge of physical RAM space and there might be device MMIO regions. Then you suggest it could map these tables to the region which is used for grant table(or the region found by the same way) while it's not used when it boots with ACPI. These regions are not used by Xen and will not be used by Dom0 either currently. But as you say, it will be wrong if Dom0 memory is not 1:1 mapped.

Will you remember in 6 months why you wrote the code like that?

My point on the previous mail is you don't describe what you did, neither in the code nor in the commit message.

Most of the place in the code are trying to avoid the assumption that DOM0 is using direct mapped. If not, we always have a comment/commit message explaining why we are doing like that and the implication (see the grant table example [1]).

Quoted 4 lines

So how about below idea:
We still copy these tables to Dom0 RAM space but when we create EFI_MMAP_TABLE, we remove the space occupied by these tables from the EfiConventionalMemory descriptor.

As you don't need the grant table region, why don't you re-use it fopr your tables? It may also be possible that we have some space just after for the EFI table.

Quoted 32 lines · Julien Grall

Quoted 5 lines · Shannon Zhao

"All above tables will be mapped to Dom0 non-RAM space. Since when booting through ACPI it doesn't need the grant table region(see below section 3), it could use this region to store the tables or use the same way to find one memory region to store them."

Quoted 10 lines

Firstly, as Jan suggested, these tables should not be in RAM space, so we drop the previous way that copying these tables to Dom0 RAM. Then I suggested map these tables to the space after the Dom0 RAM space, but this not right because Dom0 RAM region might be at the edge of physical RAM space and there might be device MMIO regions. Then you suggest it could map these tables to the region which is used for grant table(or the region found by the same way) while it's not used when it boots with ACPI. These regions are not used by Xen and will not be used by Dom0 either currently. But as you say, it will be wrong if Dom0 memory is not 1:1 mapped.

Will you remember in 6 months why you wrote the code like that?

My point on the previous mail is you don't describe what you did, neither in the code nor in the commit message.

Most of the place in the code are trying to avoid the assumption that DOM0 is using direct mapped. If not, we always have a comment/commit message explaining why we are doing like that and the implication (see the grant table example [1]).

Quoted 4 lines

So how about below idea:
We still copy these tables to Dom0 RAM space but when we create EFI_MMAP_TABLE, we remove the space occupied by these tables from the EfiConventionalMemory descriptor.

Quoted 3 lines

As you don't need the grant table region, why don't you re-use it fopr your tables? It may also be possible that we have some space just after for the EFI table.

I think that the current approach is good, please just extend the in-code comment in patch #40.

Quoted 5 lines · Shannon Zhao

"All above tables will be mapped to Dom0 non-RAM space. Since when booting through ACPI it doesn't need the grant table region(see below section 3), it could use this region to store the tables or use the same way to find one memory region to store them."

Quoted 10 lines

Firstly, as Jan suggested, these tables should not be in RAM space, so we drop the previous way that copying these tables to Dom0 RAM. Then I suggested map these tables to the space after the Dom0 RAM space, but this not right because Dom0 RAM region might be at the edge of physical RAM space and there might be device MMIO regions. Then you suggest it could map these tables to the region which is used for grant table(or the region found by the same way) while it's not used when it boots with ACPI. These regions are not used by Xen and will not be used by Dom0 either currently. But as you say, it will be wrong if Dom0 memory is not 1:1 mapped.

Will you remember in 6 months why you wrote the code like that?

My point on the previous mail is you don't describe what you did, neither in the code nor in the commit message.

Most of the place in the code are trying to avoid the assumption that DOM0 is using direct mapped. If not, we always have a comment/commit message explaining why we are doing like that and the implication (see the grant table example [1]).

Quoted 4 lines

So how about below idea:
We still copy these tables to Dom0 RAM space but when we create EFI_MMAP_TABLE, we remove the space occupied by these tables from the EfiConventionalMemory descriptor.

As you don't need the grant table region, why don't you re-use it fopr your tables? It may also be possible that we have some space just after for the EFI table.

"All above tables will be mapped to Dom0 non-RAM space. Since when booting through ACPI it doesn't need the grant table region(see below section 3), it could use this region to store the tables or use the same way to find one memory region to store them."

Firstly, as Jan suggested, these tables should not be in RAM space, so we drop the previous way that copying these tables to Dom0 RAM. Then I suggested map these tables to the space after the Dom0 RAM space, but this not right because Dom0 RAM region might be at the edge of physical RAM space and there might be device MMIO regions. Then you suggest it could map these tables to the region which is used for grant table(or the region found by the same way) while it's not used when it boots with ACPI. These regions are not used by Xen and will not be used by Dom0 either currently. But as you say, it will be wrong if Dom0 memory is not 1:1 mapped.

So how about below idea:
We still copy these tables to Dom0 RAM space but when we create EFI_MMAP_TABLE, we remove the space occupied by these tables from the EfiConventionalMemory descriptor.

Hi Shannon,

Quoted 44 lines · Shannon Zhao

Quoted 23 lines · Julien Grall

Hi Shannon,

Quoted 20 lines · On 17/11/15 09:40, [email protected] wrote:

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644
@@ -125,6 +125,11 @@ struct arch_domain
     } vuart;
 

Quoted 11 lines

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 6 lines

xen/include/asm-arm/domain.h +5 -0

     unsigned int evtchn_irq;
+#ifdef CONFIG_ACPI
+    void *efi_acpi_table;
+    paddr_t efi_acpi_gpa;
+    paddr_t efi_acpi_len;
+#endif

Why do you need this in the arch_domain? AFAICT those 3 variables should only be used while building DOM0, so a better place would be in struct kernel_info (see xen/arch/arm/kernel.h).

Since I use efi_acpi_table to store the allocated pages pointer which are used to store newly created ACPI and EFI tables.
See [PATCH v3 40/62]:

(quoted diff) +2 -0

+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 5 lines

And it free these pages when destroying this domain:

xen/include/asm-arm/domain.h +4 -0

+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif

But this is wrong. You are assuming that DOM0 memory is direct mapped which may not be true in the future.

Quoted 23 lines · Julien Grall

Hi Shannon,

Quoted 20 lines · On 17/11/15 09:40, [email protected] wrote:

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644
@@ -125,6 +125,11 @@ struct arch_domain
     } vuart;
 

Quoted 11 lines

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 6 lines

xen/include/asm-arm/domain.h +5 -0

     unsigned int evtchn_irq;
+#ifdef CONFIG_ACPI
+    void *efi_acpi_table;
+    paddr_t efi_acpi_gpa;
+    paddr_t efi_acpi_len;
+#endif

Why do you need this in the arch_domain? AFAICT those 3 variables should only be used while building DOM0, so a better place would be in struct kernel_info (see xen/arch/arm/kernel.h).

Since I use efi_acpi_table to store the allocated pages pointer which are used to store newly created ACPI and EFI tables.
See [PATCH v3 40/62]:

(quoted diff) +2 -0

+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);

And it free these pages when destroying this domain:

Quoted 23 lines · Julien Grall

Hi Shannon,

Quoted 20 lines · shannon.zhao at linaro.org

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644
@@ -125,6 +125,11 @@ struct arch_domain
     } vuart;
 

Quoted 11 lines

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 6 lines

xen/include/asm-arm/domain.h +5 -0

     unsigned int evtchn_irq;
+#ifdef CONFIG_ACPI
+    void *efi_acpi_table;
+    paddr_t efi_acpi_gpa;
+    paddr_t efi_acpi_len;
+#endif

Why do you need this in the arch_domain? AFAICT those 3 variables should only be used while building DOM0, so a better place would be in struct kernel_info (see xen/arch/arm/kernel.h).

Since I use efi_acpi_table to store the allocated pages pointer which are used to store newly created ACPI and EFI tables.
See [PATCH v3 40/62]:

(quoted diff) +2 -0

+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);

And it free these pages when destroying this domain:

xen/include/asm-arm/domain.h +4 -0

+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif

efi_acpi_gpa might not need.

Quoted 23 lines · Julien Grall

Hi Shannon,

Quoted 20 lines · On 17/11/15 09:40, [email protected] wrote:

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644
@@ -125,6 +125,11 @@ struct arch_domain
     } vuart;
 

Quoted 11 lines

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 6 lines

xen/include/asm-arm/domain.h +5 -0

     unsigned int evtchn_irq;
+#ifdef CONFIG_ACPI
+    void *efi_acpi_table;
+    paddr_t efi_acpi_gpa;
+    paddr_t efi_acpi_len;
+#endif

Why do you need this in the arch_domain? AFAICT those 3 variables should only be used while building DOM0, so a better place would be in struct kernel_info (see xen/arch/arm/kernel.h).

Since I use efi_acpi_table to store the allocated pages pointer which are used to store newly created ACPI and EFI tables.
See [PATCH v3 40/62]:

(quoted diff) +2 -0

+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);

And it free these pages when destroying this domain:

xen/include/asm-arm/domain.h +4 -0

+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif

efi_acpi_gpa might not need.

Hi Shannon,

Quoted 20 lines · On 17/11/15 09:40, [email protected] wrote:

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644
@@ -125,6 +125,11 @@ struct arch_domain
     } vuart;
 

xen/include/asm-arm/domain.h struct arch_domain · line 132

Quoted 6 lines

xen/include/asm-arm/domain.h +5 -0

     unsigned int evtchn_irq;
+#ifdef CONFIG_ACPI
+    void *efi_acpi_table;
+    paddr_t efi_acpi_gpa;
+    paddr_t efi_acpi_len;
+#endif

Why do you need this in the arch_domain? AFAICT those 3 variables should only be used while building DOM0, so a better place would be in struct kernel_info (see xen/arch/arm/kernel.h).

From: Shannon Zhao <[email protected]>

EFI table, memory description table and some of acpi tables will be placed after DOM0 memory space. Add placeholder for the starting address for loading in DOM0 and the size of new added tables. Also add a placeholder to store the new created tables.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/include/asm-arm/domain.h | 5 +++++
 1 file changed, 5 insertions(+)

xen/include/asm-arm/domain.h +0 -0

index 1e61f30..91272e5 100644

From: Parth Dixit <[email protected]>

ACPI memory is seperate from conventional memory and should be marked as reserved while passing to DOM0. Create a new meminfo structure to store all the acpi tables listed in uefi.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

 xen/arch/arm/efi/efi-boot.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

xen/arch/arm/efi/efi-boot.h +23 -0

index e427e5f..c58caca 100644
@@ -17,6 +17,9 @@ void __flush_dcache_area(const void *vaddr, unsigned long size);
 static struct file __initdata dtbfile;
 static void __initdata *fdt;
 static void __initdata *memmap;
+#ifdef CONFIG_ACPI
+static struct meminfo __initdata acpi_mem;
+#endif
 
 static int __init setup_chosen_node(void *fdt, int *addr_cells, int *size_cells)
 {

@@ -129,6 +132,9 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
 {
     int Index;
     int i = 0;
+#ifdef CONFIG_ACPI
+    int j = 0;
+#endif
     EFI_MEMORY_DESCRIPTOR *desc_ptr = map;
 
     for ( Index = 0; Index < (mmap_size / desc_size); Index++ )

@@ -148,10 +154,27 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
             bootinfo.mem.bank[i].size = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
             ++i;
         }
+#ifdef CONFIG_ACPI
+        else if ( desc_ptr->Type == EfiACPIReclaimMemory )
+        {
+            if ( j >= NR_MEM_BANKS )
+            {
+                PrintStr(L"Error: All " __stringify(NR_MEM_BANKS)
+                          " acpi meminfo mem banks exhausted.\r\n");
+                return EFI_LOAD_ERROR;
+            }
+            acpi_mem.bank[j].start = desc_ptr->PhysicalStart;
+            acpi_mem.bank[j].size  = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
+            ++j;
+        }
+#endif
         desc_ptr = NextMemoryDescriptor(desc_ptr, desc_size);
     }
 
     bootinfo.mem.nr_banks = i;
+#ifdef CONFIG_ACPI
+    acpi_mem.nr_banks = j;
+#endif
     return EFI_SUCCESS;
 }
 

Signature · 1 line

2.1.0

From: Parth Dixit <[email protected]>

ACPI memory is seperate from conventional memory and should be marked as reserved while passing to DOM0. Create a new meminfo structure to store all the acpi tables listed in uefi.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

 xen/arch/arm/efi/efi-boot.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

xen/arch/arm/efi/efi-boot.h +23 -0

index e427e5f..c58caca 100644
@@ -17,6 +17,9 @@ void __flush_dcache_area(const void *vaddr, unsigned long size);
 static struct file __initdata dtbfile;
 static void __initdata *fdt;
 static void __initdata *memmap;
+#ifdef CONFIG_ACPI
+static struct meminfo __initdata acpi_mem;
+#endif
 
 static int __init setup_chosen_node(void *fdt, int *addr_cells, int *size_cells)
 {

@@ -129,6 +132,9 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
 {
     int Index;
     int i = 0;
+#ifdef CONFIG_ACPI
+    int j = 0;
+#endif
     EFI_MEMORY_DESCRIPTOR *desc_ptr = map;
 
     for ( Index = 0; Index < (mmap_size / desc_size); Index++ )

@@ -148,10 +154,27 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
             bootinfo.mem.bank[i].size = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
             ++i;
         }
+#ifdef CONFIG_ACPI
+        else if ( desc_ptr->Type == EfiACPIReclaimMemory )
+        {
+            if ( j >= NR_MEM_BANKS )
+            {
+                PrintStr(L"Error: All " __stringify(NR_MEM_BANKS)
+                          " acpi meminfo mem banks exhausted.\r\n");
+                return EFI_LOAD_ERROR;
+            }
+            acpi_mem.bank[j].start = desc_ptr->PhysicalStart;
+            acpi_mem.bank[j].size  = desc_ptr->NumberOfPages * EFI_PAGE_SIZE;
+            ++j;
+        }
+#endif
         desc_ptr = NextMemoryDescriptor(desc_ptr, desc_size);
     }
 
     bootinfo.mem.nr_banks = i;
+#ifdef CONFIG_ACPI
+    acpi_mem.nr_banks = j;
+#endif
     return EFI_SUCCESS;
 }
 

Signature · 1 line

2.1.0

From: Shannon Zhao <[email protected]>

Estimate the memory required for loading acpi/efi tables in Dom0. Alloc the pages to store the new created EFI and ACPI tables and free these pages when destroying domain.

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/arch/arm/domain.c       |  4 +++
 xen/arch/arm/domain_build.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-
 xen/common/efi/boot.c       | 21 ++++++++++++
 xen/include/asm-arm/setup.h |  2 ++
 4 files changed, 106 insertions(+), 1 deletion(-)

xen/arch/arm/domain.c +4 -0

index 880d0a6..10c58c4 100644
@@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
     domain_vgic_free(d);
     domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif
 }
 
 void arch_domain_shutdown(struct domain *d)

xen/arch/arm/domain_build.c +12 -0

index 0c3441a..b5ed44c 100644
@@ -12,6 +12,8 @@
 #include <xen/libfdt/libfdt.h>
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
+#include <xen/acpi.h>
+#include <acpi/actables.h>
 #include <asm/device.h>
 #include <asm/setup.h>
 #include <asm/platform.h>

@@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
     return -EINVAL;
 }
 
+#ifdef CONFIG_ACPI
+static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
+{
+    u64 efi_size, acpi_size = 0, addr;
+    u32 madt_size;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_header *table = NULL;
+
+    efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+

xen/arch/arm/domain_build.c +24 -0

+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+    acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
+    d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
+
+    return 0;
+}
+
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    int rc = 0;
+    int order;
+

xen/arch/arm/domain_build.c +22 -1

+    }
+    memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+    /* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+     * region. So we use it as the ACPI table mapped address. */
+    d->arch.efi_acpi_gpa = kinfo->gnttab_start;
+
+    return 0;
+}
+#else
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    /* Only booting with ACPI will hit here */
+    BUG_ON(1);
+    return -EINVAL;
+}
+#endif
 static void dtb_load(struct kernel_info *kinfo)
 {
     void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;

@@ -1540,7 +1614,11 @@ int construct_dom0(struct domain *d)
     allocate_memory(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
-    rc = prepare_dtb(d, &kinfo);
+    if ( acpi_disabled )
+        rc = prepare_dtb(d, &kinfo);
+    else
+        rc = prepare_acpi(d, &kinfo);
+
     if ( rc < 0 )
         return rc;
 

xen/common/efi/boot.c +6 -0

index 53c7452..78d8ae9 100644
@@ -13,6 +13,7 @@
 #include <xen/multiboot.h>
 #include <xen/pci_regs.h>
 #include <xen/pfn.h>
+#include <asm/acpi.h>
 #if EFI_PAGE_SIZE != PAGE_SIZE
 # error Cannot use xen/pfn.h here!
 #endif

@@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     for( ; ; ); /* not reached */
 }
 
+#ifdef CONFIG_ACPI
+/* Constant to indicate "Xen" in unicode u16 format */
+static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000};
+
+int __init estimate_efi_size(int mem_nr_banks)

xen/common/efi/boot.c +4 -0

+    return size;
+}
+#endif
+
 #ifndef CONFIG_ARM /* TODO - runtime service support */
 
 static bool_t __initdata efi_rs_enable = 1;

xen/include/asm-arm/setup.h +2 -0

index 30ac53b..b759813 100644
@@ -51,6 +51,8 @@ void arch_init_memory(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
+int estimate_efi_size(int mem_nr_banks);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);

Signature · 1 line

2.1.0

Quoted 5 lines · shannon.zhao at linaro.org

From: Shannon Zhao <[email protected]>

Estimate the memory required for loading acpi/efi tables in Dom0. Alloc the pages to store the new created EFI and ACPI tables and free these pages when destroying domain.

Could you please explain what you are page aligning exactly and why?

Quoted 53 lines

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/arch/arm/domain.c       |  4 +++
 xen/arch/arm/domain_build.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-
 xen/common/efi/boot.c       | 21 ++++++++++++
 xen/include/asm-arm/setup.h |  2 ++
 4 files changed, 106 insertions(+), 1 deletion(-)

xen/arch/arm/domain.c +4 -0

index 880d0a6..10c58c4 100644
@@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
     domain_vgic_free(d);
     domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif
 }
 
 void arch_domain_shutdown(struct domain *d)

xen/arch/arm/domain_build.c +12 -0

index 0c3441a..b5ed44c 100644
@@ -12,6 +12,8 @@
 #include <xen/libfdt/libfdt.h>
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
+#include <xen/acpi.h>
+#include <acpi/actables.h>
 #include <asm/device.h>
 #include <asm/setup.h>
 #include <asm/platform.h>

@@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
     return -EINVAL;
 }
 
+#ifdef CONFIG_ACPI
+static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
+{
+    u64 efi_size, acpi_size = 0, addr;
+    u32 madt_size;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_header *table = NULL;
+
+    efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+

xen/arch/arm/domain_build.c static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) · line 1378

Quoted 10 lines

xen/arch/arm/domain_build.c +10 -0

+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_fadt));
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_stao));
+
+    madt_size = sizeof(struct acpi_table_madt)
+                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
+                + sizeof(struct acpi_madt_generic_distributor);
+    if ( d->arch.vgic.version == GIC_V3 )
+        madt_size += sizeof(struct acpi_madt_generic_redistributor)
+                     * d->arch.vgic.nr_regions;
+    acpi_size += PAGE_ALIGN(madt_size);

are the MADT and FADT tables guaranteed to be on separate pages or is it just an estimate?

Quoted 24 lines

xen/arch/arm/domain_build.c +24 -0

+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+    acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
+    d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
+
+    return 0;
+}
+
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    int rc = 0;
+    int order;
+

xen/arch/arm/domain_build.c static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) · line 1413

Quoted 10 lines

xen/arch/arm/domain_build.c +10 -0

+    rc = estimate_acpi_efi_size(d, kinfo);
+    if ( rc != 0 )
+        return rc;
+
+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+    if ( d->arch.efi_acpi_table == NULL )
+    {
+        printk("unable to allocate memory!\n");
+        return -1;

ENOMEM

Quoted 54 lines

xen/arch/arm/domain_build.c +22 -1

+    }
+    memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+    /* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+     * region. So we use it as the ACPI table mapped address. */
+    d->arch.efi_acpi_gpa = kinfo->gnttab_start;
+
+    return 0;
+}
+#else
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    /* Only booting with ACPI will hit here */
+    BUG_ON(1);
+    return -EINVAL;
+}
+#endif
 static void dtb_load(struct kernel_info *kinfo)
 {
     void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;

@@ -1540,7 +1614,11 @@ int construct_dom0(struct domain *d)
     allocate_memory(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
-    rc = prepare_dtb(d, &kinfo);
+    if ( acpi_disabled )
+        rc = prepare_dtb(d, &kinfo);
+    else
+        rc = prepare_acpi(d, &kinfo);
+
     if ( rc < 0 )
         return rc;
 

xen/common/efi/boot.c +6 -0

index 53c7452..78d8ae9 100644
@@ -13,6 +13,7 @@
 #include <xen/multiboot.h>
 #include <xen/pci_regs.h>
 #include <xen/pfn.h>
+#include <asm/acpi.h>
 #if EFI_PAGE_SIZE != PAGE_SIZE
 # error Cannot use xen/pfn.h here!
 #endif

@@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     for( ; ; ); /* not reached */
 }
 
+#ifdef CONFIG_ACPI
+/* Constant to indicate "Xen" in unicode u16 format */
+static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000};
+
+int __init estimate_efi_size(int mem_nr_banks)

xen/common/efi/boot.c efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) · line 1189

Quoted 10 lines

xen/common/efi/boot.c +10 -0

+{
+    int size;
+    int est_size = sizeof(EFI_SYSTEM_TABLE);
+    int ect_size = sizeof(EFI_CONFIGURATION_TABLE);
+    int emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
+    int fw_vendor_size = sizeof(XEN_EFI_FW_VENDOR);
+
+    size = PAGE_ALIGN(est_size + ect_size + fw_vendor_size)
+           + PAGE_ALIGN(emd_size *
+                        (mem_nr_banks + acpi_mem.nr_banks + TBL_MMAX));

Why are they on two separate pages? Is it mandated by the EFI spec? Why are you adding TBL_MMAX to the multiplicand here?

I don't like that we are passing mem_nr_banks as argument but we are accessing acpi_mem.nr_banks from the global variable. I would prefer if they were both passed as arguments.

Quoted 23 lines

xen/common/efi/boot.c +4 -0

+    return size;
+}
+#endif
+
 #ifndef CONFIG_ARM /* TODO - runtime service support */
 
 static bool_t __initdata efi_rs_enable = 1;

xen/include/asm-arm/setup.h +2 -0

index 30ac53b..b759813 100644
@@ -51,6 +51,8 @@ void arch_init_memory(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
+int estimate_efi_size(int mem_nr_banks);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);

Signature · 1 line

2.1.0

.

Quoted 9 lines · Stefano Stabellini

Quoted 5 lines · shannon.zhao at linaro.org

From: Shannon Zhao <[email protected]>

Estimate the memory required for loading acpi/efi tables in Dom0. Alloc the pages to store the new created EFI and ACPI tables and free these pages when destroying domain.

Could you please explain what you are page aligning exactly and why?

At least it should be 64bit aligned of the table start address. If not, guest will throw out an alignment fault.

ACPI: Using GIC for interrupt routing
Unhandled fault: alignment fault (0x96000021) at 0xffffff800006c19c

Internal error: : 96000021 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0+ #551
Hardware name: (null) (DT)
task: ffffffc008870000 ti: ffffffc00884c000 task.ti: ffffffc00884c000
PC is at acpi_get_phys_id+0x264/0x290
LR is at acpi_get_phys_id+0x178/0x290

Quoted 205 lines

Quoted 53 lines

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/arch/arm/domain.c       |  4 +++
 xen/arch/arm/domain_build.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-
 xen/common/efi/boot.c       | 21 ++++++++++++
 xen/include/asm-arm/setup.h |  2 ++
 4 files changed, 106 insertions(+), 1 deletion(-)

xen/arch/arm/domain.c +4 -0

index 880d0a6..10c58c4 100644
@@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
     domain_vgic_free(d);
     domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif
 }
 
 void arch_domain_shutdown(struct domain *d)

xen/arch/arm/domain_build.c +12 -0

index 0c3441a..b5ed44c 100644
@@ -12,6 +12,8 @@
 #include <xen/libfdt/libfdt.h>
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
+#include <xen/acpi.h>
+#include <acpi/actables.h>
 #include <asm/device.h>
 #include <asm/setup.h>
 #include <asm/platform.h>

@@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
     return -EINVAL;
 }
 
+#ifdef CONFIG_ACPI
+static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
+{
+    u64 efi_size, acpi_size = 0, addr;
+    u32 madt_size;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_header *table = NULL;
+
+    efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+

xen/arch/arm/domain_build.c static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) · line 1378

Quoted 10 lines

xen/arch/arm/domain_build.c +10 -0

+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_fadt));
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_stao));
+
+    madt_size = sizeof(struct acpi_table_madt)
+                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
+                + sizeof(struct acpi_madt_generic_distributor);
+    if ( d->arch.vgic.version == GIC_V3 )
+        madt_size += sizeof(struct acpi_madt_generic_redistributor)
+                     * d->arch.vgic.nr_regions;
+    acpi_size += PAGE_ALIGN(madt_size);

are the MADT and FADT tables guaranteed to be on separate pages or is it just an estimate?

Quoted 24 lines

xen/arch/arm/domain_build.c +24 -0

+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+    acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
+    d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
+
+    return 0;
+}
+
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    int rc = 0;
+    int order;
+

xen/arch/arm/domain_build.c static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) · line 1413

Quoted 10 lines

xen/arch/arm/domain_build.c +10 -0

+    rc = estimate_acpi_efi_size(d, kinfo);
+    if ( rc != 0 )
+        return rc;
+
+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+    if ( d->arch.efi_acpi_table == NULL )
+    {
+        printk("unable to allocate memory!\n");
+        return -1;

ENOMEM

Quoted 54 lines

xen/arch/arm/domain_build.c +22 -1

+    }
+    memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+    /* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+     * region. So we use it as the ACPI table mapped address. */
+    d->arch.efi_acpi_gpa = kinfo->gnttab_start;
+
+    return 0;
+}
+#else
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    /* Only booting with ACPI will hit here */
+    BUG_ON(1);
+    return -EINVAL;
+}
+#endif
 static void dtb_load(struct kernel_info *kinfo)
 {
     void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;

@@ -1540,7 +1614,11 @@ int construct_dom0(struct domain *d)
     allocate_memory(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
-    rc = prepare_dtb(d, &kinfo);
+    if ( acpi_disabled )
+        rc = prepare_dtb(d, &kinfo);
+    else
+        rc = prepare_acpi(d, &kinfo);
+
     if ( rc < 0 )
         return rc;
 

xen/common/efi/boot.c +6 -0

index 53c7452..78d8ae9 100644
@@ -13,6 +13,7 @@
 #include <xen/multiboot.h>
 #include <xen/pci_regs.h>
 #include <xen/pfn.h>
+#include <asm/acpi.h>
 #if EFI_PAGE_SIZE != PAGE_SIZE
 # error Cannot use xen/pfn.h here!
 #endif

@@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     for( ; ; ); /* not reached */
 }
 
+#ifdef CONFIG_ACPI
+/* Constant to indicate "Xen" in unicode u16 format */
+static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000};
+
+int __init estimate_efi_size(int mem_nr_banks)

xen/common/efi/boot.c efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) · line 1189

Quoted 10 lines

xen/common/efi/boot.c +10 -0

+{
+    int size;
+    int est_size = sizeof(EFI_SYSTEM_TABLE);
+    int ect_size = sizeof(EFI_CONFIGURATION_TABLE);
+    int emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
+    int fw_vendor_size = sizeof(XEN_EFI_FW_VENDOR);
+
+    size = PAGE_ALIGN(est_size + ect_size + fw_vendor_size)
+           + PAGE_ALIGN(emd_size *
+                        (mem_nr_banks + acpi_mem.nr_banks + TBL_MMAX));

Why are they on two separate pages? Is it mandated by the EFI spec? Why are you adding TBL_MMAX to the multiplicand here?

I don't like that we are passing mem_nr_banks as argument but we are accessing acpi_mem.nr_banks from the global variable. I would prefer if they were both passed as arguments.

Quoted 23 lines

xen/common/efi/boot.c +4 -0

+    return size;
+}
+#endif
+
 #ifndef CONFIG_ARM /* TODO - runtime service support */
 
 static bool_t __initdata efi_rs_enable = 1;

xen/include/asm-arm/setup.h +2 -0

index 30ac53b..b759813 100644
@@ -51,6 +51,8 @@ void arch_init_memory(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
+int estimate_efi_size(int mem_nr_banks);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);

Signature · 1 line

2.1.0

.

Signature · 1 line

Shannon

Quoted 9 lines · Stefano Stabellini

Quoted 5 lines · On Tue, 17 Nov 2015, [email protected] wrote:

From: Shannon Zhao <[email protected]>

Estimate the memory required for loading acpi/efi tables in Dom0. Alloc the pages to store the new created EFI and ACPI tables and free these pages when destroying domain.

Could you please explain what you are page aligning exactly and why?

At least it should be 64bit aligned of the table start address. If not, guest will throw out an alignment fault.

ACPI: Using GIC for interrupt routing
Unhandled fault: alignment fault (0x96000021) at 0xffffff800006c19c

Internal error: : 96000021 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0+ #551
Hardware name: (null) (DT)
task: ffffffc008870000 ti: ffffffc00884c000 task.ti: ffffffc00884c000
PC is at acpi_get_phys_id+0x264/0x290
LR is at acpi_get_phys_id+0x178/0x290

Quoted 205 lines

Quoted 53 lines

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/arch/arm/domain.c       |  4 +++
 xen/arch/arm/domain_build.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-
 xen/common/efi/boot.c       | 21 ++++++++++++
 xen/include/asm-arm/setup.h |  2 ++
 4 files changed, 106 insertions(+), 1 deletion(-)

xen/arch/arm/domain.c +4 -0

index 880d0a6..10c58c4 100644
@@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
     domain_vgic_free(d);
     domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif
 }
 
 void arch_domain_shutdown(struct domain *d)

xen/arch/arm/domain_build.c +12 -0

index 0c3441a..b5ed44c 100644
@@ -12,6 +12,8 @@
 #include <xen/libfdt/libfdt.h>
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
+#include <xen/acpi.h>
+#include <acpi/actables.h>
 #include <asm/device.h>
 #include <asm/setup.h>
 #include <asm/platform.h>

@@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
     return -EINVAL;
 }
 
+#ifdef CONFIG_ACPI
+static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
+{
+    u64 efi_size, acpi_size = 0, addr;
+    u32 madt_size;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_header *table = NULL;
+
+    efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+

xen/arch/arm/domain_build.c static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) · line 1378

Quoted 10 lines

xen/arch/arm/domain_build.c +10 -0

+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_fadt));
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_stao));
+
+    madt_size = sizeof(struct acpi_table_madt)
+                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
+                + sizeof(struct acpi_madt_generic_distributor);
+    if ( d->arch.vgic.version == GIC_V3 )
+        madt_size += sizeof(struct acpi_madt_generic_redistributor)
+                     * d->arch.vgic.nr_regions;
+    acpi_size += PAGE_ALIGN(madt_size);

are the MADT and FADT tables guaranteed to be on separate pages or is it just an estimate?

Quoted 24 lines

xen/arch/arm/domain_build.c +24 -0

+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+    acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
+    d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
+
+    return 0;
+}
+
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    int rc = 0;
+    int order;
+

xen/arch/arm/domain_build.c static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) · line 1413

Quoted 10 lines

xen/arch/arm/domain_build.c +10 -0

+    rc = estimate_acpi_efi_size(d, kinfo);
+    if ( rc != 0 )
+        return rc;
+
+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+    if ( d->arch.efi_acpi_table == NULL )
+    {
+        printk("unable to allocate memory!\n");
+        return -1;

ENOMEM

Quoted 54 lines

xen/arch/arm/domain_build.c +22 -1

+    }
+    memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+    /* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+     * region. So we use it as the ACPI table mapped address. */
+    d->arch.efi_acpi_gpa = kinfo->gnttab_start;
+
+    return 0;
+}
+#else
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    /* Only booting with ACPI will hit here */
+    BUG_ON(1);
+    return -EINVAL;
+}
+#endif
 static void dtb_load(struct kernel_info *kinfo)
 {
     void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;

@@ -1540,7 +1614,11 @@ int construct_dom0(struct domain *d)
     allocate_memory(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
-    rc = prepare_dtb(d, &kinfo);
+    if ( acpi_disabled )
+        rc = prepare_dtb(d, &kinfo);
+    else
+        rc = prepare_acpi(d, &kinfo);
+
     if ( rc < 0 )
         return rc;
 

xen/common/efi/boot.c +6 -0

index 53c7452..78d8ae9 100644
@@ -13,6 +13,7 @@
 #include <xen/multiboot.h>
 #include <xen/pci_regs.h>
 #include <xen/pfn.h>
+#include <asm/acpi.h>
 #if EFI_PAGE_SIZE != PAGE_SIZE
 # error Cannot use xen/pfn.h here!
 #endif

@@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     for( ; ; ); /* not reached */
 }
 
+#ifdef CONFIG_ACPI
+/* Constant to indicate "Xen" in unicode u16 format */
+static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000};
+
+int __init estimate_efi_size(int mem_nr_banks)

xen/common/efi/boot.c efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) · line 1189

Quoted 10 lines

xen/common/efi/boot.c +10 -0

+{
+    int size;
+    int est_size = sizeof(EFI_SYSTEM_TABLE);
+    int ect_size = sizeof(EFI_CONFIGURATION_TABLE);
+    int emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
+    int fw_vendor_size = sizeof(XEN_EFI_FW_VENDOR);
+
+    size = PAGE_ALIGN(est_size + ect_size + fw_vendor_size)
+           + PAGE_ALIGN(emd_size *
+                        (mem_nr_banks + acpi_mem.nr_banks + TBL_MMAX));

Why are they on two separate pages? Is it mandated by the EFI spec? Why are you adding TBL_MMAX to the multiplicand here?

I don't like that we are passing mem_nr_banks as argument but we are accessing acpi_mem.nr_banks from the global variable. I would prefer if they were both passed as arguments.

Quoted 23 lines

xen/common/efi/boot.c +4 -0

+    return size;
+}
+#endif
+
 #ifndef CONFIG_ARM /* TODO - runtime service support */
 
 static bool_t __initdata efi_rs_enable = 1;

xen/include/asm-arm/setup.h +2 -0

index 30ac53b..b759813 100644
@@ -51,6 +51,8 @@ void arch_init_memory(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
+int estimate_efi_size(int mem_nr_banks);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);

Signature · 1 line

2.1.0

.

Signature · 1 line

Shannon

Quoted 5 lines · On Tue, 17 Nov 2015, [email protected] wrote:

From: Shannon Zhao <[email protected]>

Estimate the memory required for loading acpi/efi tables in Dom0. Alloc the pages to store the new created EFI and ACPI tables and free these pages when destroying domain.

Could you please explain what you are page aligning exactly and why?

Quoted 53 lines

Signed-off-by Parth Dixit <[email protected]>

Signed-off-by Shannon Zhao <[email protected]>

---
 xen/arch/arm/domain.c       |  4 +++
 xen/arch/arm/domain_build.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-
 xen/common/efi/boot.c       | 21 ++++++++++++
 xen/include/asm-arm/setup.h |  2 ++
 4 files changed, 106 insertions(+), 1 deletion(-)

xen/arch/arm/domain.c +4 -0

index 880d0a6..10c58c4 100644
@@ -638,6 +638,10 @@ void arch_domain_destroy(struct domain *d)
     domain_vgic_free(d);
     domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif
 }
 
 void arch_domain_shutdown(struct domain *d)

xen/arch/arm/domain_build.c +12 -0

index 0c3441a..b5ed44c 100644
@@ -12,6 +12,8 @@
 #include <xen/libfdt/libfdt.h>
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
+#include <xen/acpi.h>
+#include <acpi/actables.h>
 #include <asm/device.h>
 #include <asm/setup.h>
 #include <asm/platform.h>

@@ -1354,6 +1356,78 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
     return -EINVAL;
 }
 
+#ifdef CONFIG_ACPI
+static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
+{
+    u64 efi_size, acpi_size = 0, addr;
+    u32 madt_size;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_header *table = NULL;
+
+    efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+

xen/arch/arm/domain_build.c static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) · line 1378

Quoted 10 lines

xen/arch/arm/domain_build.c +10 -0

+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_fadt));
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_stao));
+
+    madt_size = sizeof(struct acpi_table_madt)
+                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
+                + sizeof(struct acpi_madt_generic_distributor);
+    if ( d->arch.vgic.version == GIC_V3 )
+        madt_size += sizeof(struct acpi_madt_generic_redistributor)
+                     * d->arch.vgic.nr_regions;
+    acpi_size += PAGE_ALIGN(madt_size);

are the MADT and FADT tables guaranteed to be on separate pages or is it just an estimate?

Quoted 24 lines

xen/arch/arm/domain_build.c +24 -0

+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+    acpi_size += PAGE_ALIGN(table->length + sizeof(u64));
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+    acpi_size += PAGE_ALIGN(sizeof(struct acpi_table_rsdp));
+    d->arch.efi_acpi_len = PAGE_ALIGN(efi_size) + PAGE_ALIGN(acpi_size);
+
+    return 0;
+}
+
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    int rc = 0;
+    int order;
+

xen/arch/arm/domain_build.c static int prepare_dtb(struct domain *d, struct kernel_info *kinfo) · line 1413

Quoted 10 lines

xen/arch/arm/domain_build.c +10 -0

+    rc = estimate_acpi_efi_size(d, kinfo);
+    if ( rc != 0 )
+        return rc;
+
+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+    if ( d->arch.efi_acpi_table == NULL )
+    {
+        printk("unable to allocate memory!\n");
+        return -1;

ENOMEM

Quoted 54 lines

xen/arch/arm/domain_build.c +22 -1

+    }
+    memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+    /* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+     * region. So we use it as the ACPI table mapped address. */
+    d->arch.efi_acpi_gpa = kinfo->gnttab_start;
+
+    return 0;
+}
+#else
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    /* Only booting with ACPI will hit here */
+    BUG_ON(1);
+    return -EINVAL;
+}
+#endif
 static void dtb_load(struct kernel_info *kinfo)
 {
     void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;

@@ -1540,7 +1614,11 @@ int construct_dom0(struct domain *d)
     allocate_memory(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
-    rc = prepare_dtb(d, &kinfo);
+    if ( acpi_disabled )
+        rc = prepare_dtb(d, &kinfo);
+    else
+        rc = prepare_acpi(d, &kinfo);
+
     if ( rc < 0 )
         return rc;
 

xen/common/efi/boot.c +6 -0

index 53c7452..78d8ae9 100644
@@ -13,6 +13,7 @@
 #include <xen/multiboot.h>
 #include <xen/pci_regs.h>
 #include <xen/pfn.h>
+#include <asm/acpi.h>
 #if EFI_PAGE_SIZE != PAGE_SIZE
 # error Cannot use xen/pfn.h here!
 #endif

@@ -1171,6 +1172,26 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     for( ; ; ); /* not reached */
 }
 
+#ifdef CONFIG_ACPI
+/* Constant to indicate "Xen" in unicode u16 format */
+static const u16 XEN_EFI_FW_VENDOR[] ={0x0058,0x0065,0x006E,0x0000};
+
+int __init estimate_efi_size(int mem_nr_banks)

xen/common/efi/boot.c efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) · line 1189

Quoted 10 lines

xen/common/efi/boot.c +10 -0

+{
+    int size;
+    int est_size = sizeof(EFI_SYSTEM_TABLE);
+    int ect_size = sizeof(EFI_CONFIGURATION_TABLE);
+    int emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
+    int fw_vendor_size = sizeof(XEN_EFI_FW_VENDOR);
+
+    size = PAGE_ALIGN(est_size + ect_size + fw_vendor_size)
+           + PAGE_ALIGN(emd_size *
+                        (mem_nr_banks + acpi_mem.nr_banks + TBL_MMAX));

Why are they on two separate pages? Is it mandated by the EFI spec? Why are you adding TBL_MMAX to the multiplicand here?

I don't like that we are passing mem_nr_banks as argument but we are accessing acpi_mem.nr_banks from the global variable. I would prefer if they were both passed as arguments.

Quoted 23 lines

xen/common/efi/boot.c +4 -0

+    return size;
+}
+#endif
+
 #ifndef CONFIG_ARM /* TODO - runtime service support */
 
 static bool_t __initdata efi_rs_enable = 1;

xen/include/asm-arm/setup.h +2 -0

index 30ac53b..b759813 100644
@@ -51,6 +51,8 @@ void arch_init_memory(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
+int estimate_efi_size(int mem_nr_banks);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);

Signature · 1 line

2.1.0

.