On 07/02/2015 02:01 AM, Ashwin Chaugule wrote:
static int __init
>-gic_v3_acpi_init(struct acpi_table_header *table) >+gic_acpi_add_gicc_redist(phys_addr_t phys_base, u64 mpidr) > { >- int count, i, err = 0; >+ struct acpi_gicc_redist *redist; >+ >+ redist = kzalloc(sizeof(*redist), GFP_KERNEL); >+ if (!redist) >+ return -ENOMEM; >+ >+ redist->mpidr = mpidr; >+ redist->phys_base = phys_base; >+ >+ if (acpi_gic_version() == ACPI_MADT_GIC_VERSION_V3) >+ /* RD_base + SGI_base */ >+ redist->redist_base = ioremap(phys_base, 2 * SZ_64K); >+ else >+ /* >+ * RD_base + SGI_base + VLPI_base, >+ * we don't map reserved page as it's buggy to access it >+ */ >+ redist->redist_base = ioremap(phys_base, 3 * SZ_64K);
This will break things in the gic_populate_rdist() loop for the case where there is only one rdist entry per CPU and each entry is not delimited by the LAST bit in the GICR_TYPER. This is possible when the hardware only sets the LAST bit for the last rdist entry. In such a case, the __iomem *ptr in the loop will increment endlessly until it triggers some kind of illegal memory read.
Need to think some more, but 2 options come to mind:
- Figure a way to allocate contiguous memory for all rdist_regions.
You already do that in GICR case: gic_acpi_register_redist().
- Find a way to break the loop in gic_populate_rdist() if the hardware
doesn't have the LAST bit set. If we know number of entries in each rdist region this could be possible.
I'm not using the normal dist population mechanism, I'm use a list to link all the GICR base in GICC and match it with the MPIDR.
For the gicV3 init using GIC redistributor regions, will be the same as you said, hope I didn't misunderstand your question.
Ah. I see. So does each redist_base only point to one entry? IIUC,
yes.
Marc's code doesn't assume that. He loops until he finds the LAST bit.
And yes :)
Honestly I dont understand why we can't have all the redist regions listed in one table. That would make things much simpler but maybe I'm missing some details though.
I already handled the GICR regions just as DT does in another GICv3 patch [1], there are GICR regions entries in MADT and we can reuse Marc's code.
[1]: http://lkml.iu.edu/hypermail/linux/kernel/1506.2/03033.html
Thanks Hanjun