On 06/21/2016 05:16 AM, Julien Grall wrote:
Hello Shanker,
On 19/06/16 00:45, Shanker Donthineni wrote:
The redistributor address can be specified either as part of GICC or
GICR subtable depending on the power domain. The current driver
doesn't support parsing redistributor entry that is defined in GICC
subtable. The GIC CPU subtable entry holds the associated Redistributor
base address if it is not on always-on power domain.
This patch adds necessary code to handle both types of Redistributors
base addresses.
Signed-off-by: Shanker Donthineni <shank...@codeaurora.org>
---
xen/arch/arm/gic-v3.c | 97
++++++++++++++++++++++++++++++++-------
xen/include/asm-arm/gic.h | 2 +
xen/include/asm-arm/gic_v3_defs.h | 1 +
3 files changed, 83 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index af12ebc..42cf848 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -659,6 +659,10 @@ static int __init gicv3_populate_rdist(void)
smp_processor_id(), i, ptr);
return 0;
}
+
+ if ( gicv3.rdist_regions[i].single_rdist )
+ break;
+
if ( gicv3.rdist_stride )
ptr += gicv3.rdist_stride;
else
@@ -1282,6 +1286,11 @@ static int gicv3_iomem_deny_access(const struct
domain *d)
}
#ifdef CONFIG_ACPI
+static bool gic_dist_supports_dvis(void)
static inline and please use bool_t here.
Still learning XEN coding style, I'll fix it.
+{
+ return !!(readl_relaxed(GICD + GICD_TYPER) & GICD_TYPER_DVIS);
+}
+
static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
{
struct acpi_subtable_header *header;
@@ -1393,18 +1402,39 @@ gic_acpi_parse_madt_redistributor(struct
acpi_subtable_header *header,
const unsigned long end)
{
struct acpi_madt_generic_redistributor *rdist;
+ struct acpi_madt_generic_interrupt *processor;
struct rdist_region *region;
region = gicv3.rdist_regions + gicv3.rdist_count;
- rdist = (struct acpi_madt_generic_redistributor *)header;
- if ( BAD_MADT_ENTRY(rdist, end) )
- return -EINVAL;
+ if ( header->type == ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR )
+ {
+ rdist = (struct acpi_madt_generic_redistributor *)header;
+ if ( BAD_MADT_ENTRY(rdist, end) )
+ return -EINVAL;
- if ( !rdist->base_address || !rdist->length )
- return -EINVAL;
+ if ( !rdist->base_address || !rdist->length )
+ return -EINVAL;
+
+ region->base = rdist->base_address;
+ region->size = rdist->length;
+ }
+ else if ( header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT )
+ {
Parsing the GICC and the redistributor is quite different. I would
much prefer a function for parsing each table and an helper to add a
new redistributor.
I'll do.
+ processor = (struct acpi_madt_generic_interrupt *)header;
+ if ( BAD_MADT_ENTRY(processor, end) )
+ return -EINVAL;
+
+ if ( !(processor->flags & ACPI_MADT_ENABLED) )
+ return 0;
+
+ if ( !processor->gicr_base_address )
+ return -EINVAL;
+
+ region->base = processor->gicr_base_address;
+ region->size = gic_dist_supports_dvis() ? SZ_256K : SZ_128K;
Please explain in the commit message how you find the size. I would
also prefer if you use (4 x SZ_64K) and (2 * SZ_64K) as we do in
populate_rdist.
+ region->single_rdist = true;
The indentation looks wrong.
+ }
- region->base = rdist->base_address;
- region->size = rdist->length;
region->map_base = ioremap_nocache(region->base, region->size);
if ( !region->map_base )
@@ -1412,6 +1442,7 @@ gic_acpi_parse_madt_redistributor(struct
acpi_subtable_header *header,
printk("Unable to map GICR registers\n");
return -ENOMEM;
}
+
Spurious change.
gicv3.rdist_count++;
return 0;
@@ -1421,9 +1452,22 @@ static int __init
gic_acpi_get_madt_redistributor_num(struct acpi_subtable_header
*header,
const unsigned long end)
{
- /* Nothing to do here since it only wants to get the number of GIC
- * redistributors.
- */
+ struct acpi_madt_generic_redistributor *rdist;
+ struct acpi_madt_generic_interrupt *cpuif;
+
+ if ( header->type == ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR )
+ {
+ rdist = (struct acpi_madt_generic_redistributor *)header;
+ if ( BAD_MADT_ENTRY(rdist, end) || !rdist->base_address )
+ return -EINVAL;
+ }
+ else if ( header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT )
+ {
+ cpuif = (struct acpi_madt_generic_interrupt *)header;
+ if ( BAD_MADT_ENTRY(cpuif, end) || !cpuif->gicr_base_address )
+ return -EINVAL;
+ }
+
Ditto for the parsing.
return 0;
}
[...]
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 44b9ef6..7f9ad86 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -101,6 +101,8 @@
#define GICD_TYPE_CPUS_SHIFT 5
#define GICD_TYPE_CPUS 0x0e0
#define GICD_TYPE_SEC 0x400
+#define GICD_TYPER_LPIS (1U << 17)
This is not used by this patch. So please drop it.
I'll drop it.
+#define GICD_TYPER_DVIS (1U << 18)
#define GICC_CTL_ENABLE 0x1
#define GICC_CTL_EOI (0x1 << 9)
diff --git a/xen/include/asm-arm/gic_v3_defs.h
b/xen/include/asm-arm/gic_v3_defs.h
index 6d98491..6bd25a5 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -141,6 +141,7 @@ struct rdist_region {
paddr_t base;
paddr_t size;
void __iomem *map_base;
+ bool single_rdist;
};
#endif /* __ASM_ARM_GIC_V3_DEFS_H__ */
Regards,
--
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
