[Xen-devel,1/8] xen/arm: acpi: Handle correctly detection of GICv2 on GICv3

Message ID 1486149538-20432-2-git-send-email-julien.grall@arm.com
State New
Headers show
Series
  • xen/arm: Fix and clean-up for ACPI and EFI
Related show

Commit Message

Julien Grall Feb. 3, 2017, 7:18 p.m.
When the GICv3 is not GICv2 compatible, the associated field in the MADT
will be zeroed. However, the rest of the code expects the variable to
be set to INVALID_PADDR.

This will result to false detection of GICv2 and give I/O access to page
0 for the hardware domain.

Thankfully, it will fail because the size of GICV has not been set.

Fix the detection by converting 0 to INVALID_PADDR for the GICC and
GICV base. At the same time only set the size of each region when the
base address is not 0.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/gic-v3.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Comments

Stefano Stabellini Feb. 16, 2017, 1:36 a.m. | #1
On Fri, 3 Feb 2017, Julien Grall wrote:
> When the GICv3 is not GICv2 compatible, the associated field in the MADT
> will be zeroed. However, the rest of the code expects the variable to
> be set to INVALID_PADDR.
> 
> This will result to false detection of GICv2 and give I/O access to page
> 0 for the hardware domain.
> 
> Thankfully, it will fail because the size of GICV has not been set.
> 
> Fix the detection by converting 0 to INVALID_PADDR for the GICC and
> GICV base. At the same time only set the size of each region when the
> base address is not 0.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/gic-v3.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 955591b..bb1861e 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1356,7 +1356,6 @@ gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>      if ( !cpu_base_assigned )
>      {
>          cbase = processor->base_address;
> -        csize = SZ_8K;
>          vbase = processor->gicv_base_address;
>          gicv3_info.maintenance_irq = processor->vgic_interrupt;
>  
> @@ -1505,6 +1504,25 @@ static void __init gicv3_acpi_init(void)
>          panic("GICv3: No valid GICC entries exists");
>  
>      gicv3.rdist_stride = 0;
> +
> +    /*
> +     * In ACPI, 0 is considered as the invalid address. However the rest
> +     * of the initialization rely on the invalid address to be
> +     * INVALID_ADDR.
> +     *
> +     * Also set the size of the GICC and GICV when there base address
> +     * is not invalid as those values are not present in ACPI.
> +     */
> +    if ( !cbase )
> +        cbase = INVALID_PADDR;
> +    else
> +        csize = SZ_8K;
> +
> +    if ( !vbase )
> +        vbase = INVALID_PADDR;
> +    else
> +        vsize = GUEST_GICC_SIZE;
> +
>  }
>  #else
>  static void __init gicv3_acpi_init(void) { }
> -- 
> 1.9.1
>

Patch

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 955591b..bb1861e 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1356,7 +1356,6 @@  gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
     if ( !cpu_base_assigned )
     {
         cbase = processor->base_address;
-        csize = SZ_8K;
         vbase = processor->gicv_base_address;
         gicv3_info.maintenance_irq = processor->vgic_interrupt;
 
@@ -1505,6 +1504,25 @@  static void __init gicv3_acpi_init(void)
         panic("GICv3: No valid GICC entries exists");
 
     gicv3.rdist_stride = 0;
+
+    /*
+     * In ACPI, 0 is considered as the invalid address. However the rest
+     * of the initialization rely on the invalid address to be
+     * INVALID_ADDR.
+     *
+     * Also set the size of the GICC and GICV when there base address
+     * is not invalid as those values are not present in ACPI.
+     */
+    if ( !cbase )
+        cbase = INVALID_PADDR;
+    else
+        csize = SZ_8K;
+
+    if ( !vbase )
+        vbase = INVALID_PADDR;
+    else
+        vsize = GUEST_GICC_SIZE;
+
 }
 #else
 static void __init gicv3_acpi_init(void) { }