[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH V2 04/10] arm/gic-v3: Parse per-cpu redistributor entry in GICC subtable
On 06/27/2016 06:47 AM, Julien Grall wrote:
Hi Shanker,
On 26/06/16 18:48, 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.
The per CPU Redistributor size is not defined in ACPI specification.
Set it's size to SZ_256K if the GIC hardware is capable of Direct
s/it's/its/, although I would use "the".
Virtual LPI Injection feature otherwise SZ_128K.
"Set the size to SZ_256K if the GIC hardware is supporting Direct
Virtual LPI injection, SZ_128K otherwise".
This patch adds necessary code to handle both types of Redistributors
base addresses.
Signed-off-by: Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>
---
Changes since v1:
Edited commit text and fixed white spaces.
Added a new function for parsing per CPU Redistributor entry.
xen/arch/arm/gic-v3.c | 84
++++++++++++++++++++++++++++++++++-----
xen/include/asm-arm/gic.h | 1 +
xen/include/asm-arm/gic_v3_defs.h | 1 +
3 files changed, 77 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 0471fea..3977244 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,14 +1286,21 @@ static int gicv3_iomem_deny_access(const struct
domain *d)
}
#ifdef CONFIG_ACPI
-static void __init gic_acpi_add_rdist_region(u64 base_addr, u32 size)
+static void __init
+gic_acpi_add_rdist_region(u64 base_addr, u32 size, bool single_rdist)
{
unsigned int idx = gicv3.rdist_count++;
+ gicv3.rdist_regions[idx].single_rdist = single_rdist;
gicv3.rdist_regions[idx].base = base_addr;
gicv3.rdist_regions[idx].size = size;
}
+static inline bool gic_dist_supports_dvis(void)
+{
+ 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;
@@ -1397,6 +1408,42 @@ gic_acpi_parse_madt_distributor(struct
acpi_subtable_header *header,
}
static int __init
+gic_acpi_parse_cpu_redistributor(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+ struct acpi_madt_generic_interrupt *processor;
+ u32 size;
+
+ processor = (struct acpi_madt_generic_interrupt *)header;
+ if ( BAD_MADT_ENTRY(processor, end) )
+ return -EINVAL;
+
+ if ( !processor->gicr_base_address )
+ return -EINVAL;
You already check it in gic_acpi_get_madt_cpu_num, so there is no
reason to do it again.
Other function just finds the number of valid cpu interfaces. I would
prefer to keep the validation check here.
+
+ if ( processor->flags & ACPI_MADT_ENABLED )
+ {
+ size = gic_dist_supports_dvis() ? 4 * SZ_64K : 2 * SZ_64K;
+ gic_acpi_add_rdist_region(processor->gicr_base_address, size,
true);
+ }
I would revert the condition to avoid one level of indentation. I.e
I'll do.
if ( !(processor->flags & ACPI_MADT_ENABLED) )
return 0;
size = ....
gic_acpi_add...
return 0;
However, it looks like that the other function that parses GICC within
gic-v3.c (see gic_acpi_parse_madt_cpu) does not check if the CPU is
usable.
Disabled GICC entries should be skipped because its Redistributor region
is not always-on power domain. Please look at my review comment to your
KVM-ACPI patch http://www.gossamer-threads.com/lists/linux/kernel/2413670.
I think we need to have the same parsing behavior on every function.
+
+ return 0;
+}
+
+static int __init
+gic_acpi_get_madt_cpu_num(struct acpi_subtable_header *header,
+ const unsigned long end)
+{
+ struct acpi_madt_generic_interrupt *cpuif;
+
+ cpuif = (struct acpi_madt_generic_interrupt *)header;
+ if ( BAD_MADT_ENTRY(cpuif, end) || !cpuif->gicr_base_address )
+ return -EINVAL;
+
+ return 0;
+}
+
+static int __init
gic_acpi_parse_madt_redistributor(struct acpi_subtable_header *header,
const unsigned long end)
{
@@ -1409,7 +1456,7 @@ gic_acpi_parse_madt_redistributor(struct
acpi_subtable_header *header,
if ( !rdist->base_address || !rdist->length )
return -EINVAL;
- gic_acpi_add_rdist_region(rdist->base_address, rdist->length);
+ gic_acpi_add_rdist_region(rdist->base_address, rdist->length,
false);
return 0;
}
@@ -1428,6 +1475,7 @@ static void __init gicv3_acpi_init(void)
{
struct acpi_table_header *table;
struct rdist_region *rdist_regs;
+ bool gicr_table = true;
acpi_status status;
int count;
@@ -1457,8 +1505,18 @@ static void __init gicv3_acpi_init(void)
count = acpi_parse_entries(ACPI_SIG_MADT, sizeof(struct
acpi_table_madt),
gic_acpi_get_madt_redistributor_num,
table,
ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
0);
- if ( count <= 0 )
- panic("GICv3: No valid GICR entries exists");
+
+ /* Count the total number of CPU interface entries */
+ if (count <= 0) {
Coding style:
if ( count <= 0 )
{
+ count = acpi_parse_entries(ACPI_SIG_MADT,
+ sizeof(struct acpi_table_madt),
+ gic_acpi_get_madt_cpu_num,
+ table,
ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
Please use acpi_table_parse_madt.
+ if (count <= 0)
Coding style:
if ( count <= 0 )
+ panic("GICv3: No valid GICR entries exists");
+
+ gicr_table = false;
+ }
if ( count > MAX_RDIST_COUNT )
panic("GICv3: Number of redistributor regions is more than"
@@ -1470,11 +1528,19 @@ static void __init gicv3_acpi_init(void)
gicv3.rdist_regions = rdist_regs;
- /* Parse always-on power domain Re-distributor entries */
- count = acpi_parse_entries(ACPI_SIG_MADT,
- sizeof(struct acpi_table_madt),
- gic_acpi_parse_madt_redistributor,
table,
- ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
count);
+ if ( gicr_table )
+ /* Parse always-on power domain Re-distributor entries */
+ count = acpi_parse_entries(ACPI_SIG_MADT,
+ sizeof(struct acpi_table_madt),
+ gic_acpi_parse_madt_redistributor,
table,
+
ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR, count);
Please use acpi_table_parse_madt.
+ else
+ /* Parse Re-distributor entries described in CPU interface
table */
+ count = acpi_parse_entries(ACPI_SIG_MADT,
+ sizeof(struct acpi_table_madt),
+ gic_acpi_parse_cpu_redistributor,
table,
+ ACPI_MADT_TYPE_GENERIC_INTERRUPT,
count);
Ditto.
+
if ( count <= 0 )
panic("GICv3: Can't get Redistributor entry");
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 44b9ef6..fedf1fa 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -101,6 +101,7 @@
#define GICD_TYPE_CPUS_SHIFT 5
#define GICD_TYPE_CPUS 0x0e0
#define GICD_TYPE_SEC 0x400
+#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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
