From: Matthias Brugger <[email protected]>
Some hardware does not implement two-level page tables so that
the amount of contigious memory needed by the baser is bigger
then the zone order. This is a known problem on Cavium Thunderx
with 4K page size.
We fix this by adding an errata which allocates the memory early
in the boot cycle, using the memblock allocator.
Signed-off-by: Matthias Brugger <[email protected]>
---
arch/arm64/Kconfig | 12 ++++++++
arch/arm64/include/asm/cpucaps.h | 3 +-
arch/arm64/kernel/cpu_errata.c | 33 +++++++++++++++++++++
drivers/irqchip/irq-gic-v3-its.c | 50 ++++++++++++++++++++------------
4 files changed, 79 insertions(+), 19 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1b1a0e95c751..dfd9fe08f0b2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -597,6 +597,18 @@ config QCOM_FALKOR_ERRATUM_E1041
If unsure, say Y.
+config CAVIUM_ALLOC_ITS_TABLE_EARLY
+ bool "Cavium Thunderx: Allocate the its table early"
+ default y
+ depends on ARM64_4K_PAGES && FORCE_MAX_ZONEORDER < 13
+ depends on ARM_GIC_V3_ITS
+ help
+ Cavium Thunderx needs to allocate 16MB of ITS translation table.
+ This can be bigger as MAX_ZONE_ORDER and need therefore be done
+ via the memblock allocator.
+
+ If unsure, say Y.
+
endmenu
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index ae1f70450fb2..c98be4809b7f 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -51,7 +51,8 @@
#define ARM64_SSBD 30
#define ARM64_MISMATCHED_CACHE_TYPE 31
#define ARM64_HAS_STAGE2_FWB 32
+#define ARM64_WORKAROUND_CAVIUM_ITS_TABLE 33
-#define ARM64_NCAPS 33
+#define ARM64_NCAPS 34
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index dec10898d688..7908f8fa3ba8 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -411,6 +411,29 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
}
#endif /* CONFIG_ARM64_SSBD */
+#ifdef CONFIG_CAVIUM_ALLOC_ITS_TABLE_EARLY
+#include <linux/bootmem.h>
+extern void *its_base;
+
+/*
+ * Hardware that doesn't use two-level page table and exceedes
+ * the maximum order of pages that can be allocated by the buddy
+ * allocator. Try to use the memblock allocator instead.
+ * This has been observed on Cavium Thunderx machines with 4K
+ * page size.
+ */
+static bool __init its_early_alloc(const struct arm64_cpu_capabilities *cap,
+ int scope)
+{
+ /* We need to allocate the table only once */
+ if (scope & ARM64_CPUCAP_SCOPE_BOOT_CPU && !its_base)
+ its_base = (void *)memblock_virt_alloc_nopanic(16 * SZ_1M,
+ 64 * SZ_1K);
+
+ return true;
+}
+#endif /* CONFIG_CAVIUM_ALLOC_ITS_TABLE_EARLY */
+
#define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max) \
.matches = is_affected_midr_range, \
.midr_range = MIDR_RANGE(model, v_min, r_min, v_max, r_max)
@@ -679,6 +702,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
.matches = has_ssbd_mitigation,
},
+#endif
+#ifdef CONFIG_CAVIUM_ALLOC_ITS_TABLE_EARLY
+ {
+ /* Cavium ThunderX, pass 1.x - 2.1 */
+ .desc = "Cavium alloc ITS table early",
+ .capability = ARM64_WORKAROUND_CAVIUM_ITS_TABLE,
+ .type = ARM64_CPUCAP_SCOPE_BOOT_CPU,
+ .matches = its_early_alloc,
+ .midr_range = MIDR_RANGE(MIDR_THUNDERX, 0, 0, 1, 1),
+ },
#endif
{
}
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c2df341ff6fa..b78546740a0d 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -87,6 +87,8 @@ struct its_baser {
u32 psz;
};
+void *its_base;
+
struct its_device;
/*
@@ -1666,7 +1668,7 @@ static void its_write_baser(struct its_node *its, struct its_baser *baser,
baser->val = its_read_baser(its, baser);
}
-static int its_setup_baser(struct its_node *its, struct its_baser *baser,
+static int __init its_setup_baser(struct its_node *its, struct its_baser *baser,
u64 cache, u64 shr, u32 psz, u32 order,
bool indirect)
{
@@ -1675,7 +1677,6 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
u64 type = GITS_BASER_TYPE(val);
u64 baser_phys, tmp;
u32 alloc_pages;
- void *base;
retry_alloc_baser:
alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz);
@@ -1687,11 +1688,22 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
order = get_order(GITS_BASER_PAGES_MAX * psz);
}
- base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
- if (!base)
- return -ENOMEM;
+ if (cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_ITS_TABLE)) {
+ if (!its_base) {
+ pr_warn("ITS@%pa: %s Allocation using memblock failed %pS\n",
+ &its->phys_base, its_base_type_string[type],
+ its_base);
+ return -ENOMEM;
+ }
+
+ } else {
+ its_base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+ order);
+ if (!its_base)
+ return -ENOMEM;
+ }
- baser_phys = virt_to_phys(base);
+ baser_phys = virt_to_phys(its_base);
/* Check if the physical address of the memory is above 48bits */
if (IS_ENABLED(CONFIG_ARM64_64K_PAGES) && (baser_phys >> 48)) {
@@ -1699,7 +1711,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
/* 52bit PA is supported only when PageSize=64K */
if (psz != SZ_64K) {
pr_err("ITS: no 52bit PA support when psz=%d\n", psz);
- free_pages((unsigned long)base, order);
+ free_pages((unsigned long)its_base, order);
return -ENXIO;
}
@@ -1744,7 +1756,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
shr = tmp & GITS_BASER_SHAREABILITY_MASK;
if (!shr) {
cache = GITS_BASER_nC;
- gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order));
+ gic_flush_dcache_to_poc(its_base, PAGE_ORDER_TO_SIZE(order));
}
goto retry_baser;
}
@@ -1755,7 +1767,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
* size and retry. If we reach 4K, then
* something is horribly wrong...
*/
- free_pages((unsigned long)base, order);
+ free_pages((unsigned long)its_base, order);
baser->base = NULL;
switch (psz) {
@@ -1772,19 +1784,19 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
pr_err("ITS@%pa: %s doesn't stick: %llx %llx\n",
&its->phys_base, its_base_type_string[type],
val, tmp);
- free_pages((unsigned long)base, order);
+ free_pages((unsigned long)its_base, order);
return -ENXIO;
}
baser->order = order;
- baser->base = base;
+ baser->base = its_base;
baser->psz = psz;
tmp = indirect ? GITS_LVL1_ENTRY_SIZE : esz;
pr_info("ITS@%pa: allocated %d %s @%lx (%s, esz %d, psz %dK, shr %d)\n",
&its->phys_base, (int)(PAGE_ORDER_TO_SIZE(order) / (int)tmp),
its_base_type_string[type],
- (unsigned long)virt_to_phys(base),
+ (unsigned long)virt_to_phys(its_base),
indirect ? "indirect" : "flat", (int)esz,
psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
@@ -1832,12 +1844,14 @@ static bool its_parse_indirect_baser(struct its_node *its,
* feature is not supported by hardware.
*/
new_order = max_t(u32, get_order(esz << ids), new_order);
- if (new_order >= MAX_ORDER) {
- new_order = MAX_ORDER - 1;
- ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
- pr_warn("ITS@%pa: %s Table too large, reduce ids %u->%u\n",
- &its->phys_base, its_base_type_string[type],
- its->device_ids, ids);
+ if (!cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_ITS_TABLE)) {
+ if (new_order >= MAX_ORDER) {
+ new_order = MAX_ORDER - 1;
+ ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
+ pr_warn("ITS@%pa: %s Table too large, reduce ids %u->%u\n",
+ &its->phys_base, its_base_type_string[type],
+ its->device_ids, ids);
+ }
}
*order = new_order;
--
2.18.0
Friendly reminder, if anyone has any comment on the patch :)
On 9/12/18 11:52 AM, [email protected] wrote:
> From: Matthias Brugger <[email protected]>
>
> Some hardware does not implement two-level page tables so that
> the amount of contigious memory needed by the baser is bigger
> then the zone order. This is a known problem on Cavium Thunderx
> with 4K page size.
>
> We fix this by adding an errata which allocates the memory early
> in the boot cycle, using the memblock allocator.
>
> Signed-off-by: Matthias Brugger <[email protected]>
> ---
> arch/arm64/Kconfig | 12 ++++++++
> arch/arm64/include/asm/cpucaps.h | 3 +-
> arch/arm64/kernel/cpu_errata.c | 33 +++++++++++++++++++++
> drivers/irqchip/irq-gic-v3-its.c | 50 ++++++++++++++++++++------------
> 4 files changed, 79 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1b1a0e95c751..dfd9fe08f0b2 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -597,6 +597,18 @@ config QCOM_FALKOR_ERRATUM_E1041
>
> If unsure, say Y.
>
> +config CAVIUM_ALLOC_ITS_TABLE_EARLY
> + bool "Cavium Thunderx: Allocate the its table early"
> + default y
> + depends on ARM64_4K_PAGES && FORCE_MAX_ZONEORDER < 13
> + depends on ARM_GIC_V3_ITS
> + help
> + Cavium Thunderx needs to allocate 16MB of ITS translation table.
> + This can be bigger as MAX_ZONE_ORDER and need therefore be done
> + via the memblock allocator.
> +
> + If unsure, say Y.
> +
> endmenu
>
>
> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> index ae1f70450fb2..c98be4809b7f 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -51,7 +51,8 @@
> #define ARM64_SSBD 30
> #define ARM64_MISMATCHED_CACHE_TYPE 31
> #define ARM64_HAS_STAGE2_FWB 32
> +#define ARM64_WORKAROUND_CAVIUM_ITS_TABLE 33
>
> -#define ARM64_NCAPS 33
> +#define ARM64_NCAPS 34
>
> #endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index dec10898d688..7908f8fa3ba8 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -411,6 +411,29 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
> }
> #endif /* CONFIG_ARM64_SSBD */
>
> +#ifdef CONFIG_CAVIUM_ALLOC_ITS_TABLE_EARLY
> +#include <linux/bootmem.h>
> +extern void *its_base;
> +
> +/*
> + * Hardware that doesn't use two-level page table and exceedes
> + * the maximum order of pages that can be allocated by the buddy
> + * allocator. Try to use the memblock allocator instead.
> + * This has been observed on Cavium Thunderx machines with 4K
> + * page size.
> + */
> +static bool __init its_early_alloc(const struct arm64_cpu_capabilities *cap,
> + int scope)
> +{
> + /* We need to allocate the table only once */
> + if (scope & ARM64_CPUCAP_SCOPE_BOOT_CPU && !its_base)
> + its_base = (void *)memblock_virt_alloc_nopanic(16 * SZ_1M,
> + 64 * SZ_1K);
> +
> + return true;
> +}
> +#endif /* CONFIG_CAVIUM_ALLOC_ITS_TABLE_EARLY */
> +
> #define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max) \
> .matches = is_affected_midr_range, \
> .midr_range = MIDR_RANGE(model, v_min, r_min, v_max, r_max)
> @@ -679,6 +702,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> .type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
> .matches = has_ssbd_mitigation,
> },
> +#endif
> +#ifdef CONFIG_CAVIUM_ALLOC_ITS_TABLE_EARLY
> + {
> + /* Cavium ThunderX, pass 1.x - 2.1 */
> + .desc = "Cavium alloc ITS table early",
> + .capability = ARM64_WORKAROUND_CAVIUM_ITS_TABLE,
> + .type = ARM64_CPUCAP_SCOPE_BOOT_CPU,
> + .matches = its_early_alloc,
> + .midr_range = MIDR_RANGE(MIDR_THUNDERX, 0, 0, 1, 1),
> + },
> #endif
> {
> }
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index c2df341ff6fa..b78546740a0d 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -87,6 +87,8 @@ struct its_baser {
> u32 psz;
> };
>
> +void *its_base;
> +
> struct its_device;
>
> /*
> @@ -1666,7 +1668,7 @@ static void its_write_baser(struct its_node *its, struct its_baser *baser,
> baser->val = its_read_baser(its, baser);
> }
>
> -static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> +static int __init its_setup_baser(struct its_node *its, struct its_baser *baser,
> u64 cache, u64 shr, u32 psz, u32 order,
> bool indirect)
> {
> @@ -1675,7 +1677,6 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> u64 type = GITS_BASER_TYPE(val);
> u64 baser_phys, tmp;
> u32 alloc_pages;
> - void *base;
>
> retry_alloc_baser:
> alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz);
> @@ -1687,11 +1688,22 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> order = get_order(GITS_BASER_PAGES_MAX * psz);
> }
>
> - base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
> - if (!base)
> - return -ENOMEM;
> + if (cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_ITS_TABLE)) {
> + if (!its_base) {
> + pr_warn("ITS@%pa: %s Allocation using memblock failed %pS\n",
> + &its->phys_base, its_base_type_string[type],
> + its_base);
> + return -ENOMEM;
> + }
> +
> + } else {
> + its_base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
> + order);
> + if (!its_base)
> + return -ENOMEM;
> + }
>
> - baser_phys = virt_to_phys(base);
> + baser_phys = virt_to_phys(its_base);
>
> /* Check if the physical address of the memory is above 48bits */
> if (IS_ENABLED(CONFIG_ARM64_64K_PAGES) && (baser_phys >> 48)) {
> @@ -1699,7 +1711,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> /* 52bit PA is supported only when PageSize=64K */
> if (psz != SZ_64K) {
> pr_err("ITS: no 52bit PA support when psz=%d\n", psz);
> - free_pages((unsigned long)base, order);
> + free_pages((unsigned long)its_base, order);
> return -ENXIO;
> }
>
> @@ -1744,7 +1756,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> shr = tmp & GITS_BASER_SHAREABILITY_MASK;
> if (!shr) {
> cache = GITS_BASER_nC;
> - gic_flush_dcache_to_poc(base, PAGE_ORDER_TO_SIZE(order));
> + gic_flush_dcache_to_poc(its_base, PAGE_ORDER_TO_SIZE(order));
> }
> goto retry_baser;
> }
> @@ -1755,7 +1767,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> * size and retry. If we reach 4K, then
> * something is horribly wrong...
> */
> - free_pages((unsigned long)base, order);
> + free_pages((unsigned long)its_base, order);
> baser->base = NULL;
>
> switch (psz) {
> @@ -1772,19 +1784,19 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> pr_err("ITS@%pa: %s doesn't stick: %llx %llx\n",
> &its->phys_base, its_base_type_string[type],
> val, tmp);
> - free_pages((unsigned long)base, order);
> + free_pages((unsigned long)its_base, order);
> return -ENXIO;
> }
>
> baser->order = order;
> - baser->base = base;
> + baser->base = its_base;
> baser->psz = psz;
> tmp = indirect ? GITS_LVL1_ENTRY_SIZE : esz;
>
> pr_info("ITS@%pa: allocated %d %s @%lx (%s, esz %d, psz %dK, shr %d)\n",
> &its->phys_base, (int)(PAGE_ORDER_TO_SIZE(order) / (int)tmp),
> its_base_type_string[type],
> - (unsigned long)virt_to_phys(base),
> + (unsigned long)virt_to_phys(its_base),
> indirect ? "indirect" : "flat", (int)esz,
> psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
>
> @@ -1832,12 +1844,14 @@ static bool its_parse_indirect_baser(struct its_node *its,
> * feature is not supported by hardware.
> */
> new_order = max_t(u32, get_order(esz << ids), new_order);
> - if (new_order >= MAX_ORDER) {
> - new_order = MAX_ORDER - 1;
> - ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
> - pr_warn("ITS@%pa: %s Table too large, reduce ids %u->%u\n",
> - &its->phys_base, its_base_type_string[type],
> - its->device_ids, ids);
> + if (!cpus_have_const_cap(ARM64_WORKAROUND_CAVIUM_ITS_TABLE)) {
> + if (new_order >= MAX_ORDER) {
> + new_order = MAX_ORDER - 1;
> + ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
> + pr_warn("ITS@%pa: %s Table too large, reduce ids %u->%u\n",
> + &its->phys_base, its_base_type_string[type],
> + its->device_ids, ids);
> + }
> }
>
> *order = new_order;
>
Hi Matthias,
On 04/10/18 23:11, Matthias Brugger wrote:
> Friendly reminder, if anyone has any comment on the patch :)
>
> On 9/12/18 11:52 AM, [email protected] wrote:
>> From: Matthias Brugger <[email protected]>
>>
>> Some hardware does not implement two-level page tables so that
>> the amount of contigious memory needed by the baser is bigger
>> then the zone order. This is a known problem on Cavium Thunderx
>> with 4K page size.
>>
>> We fix this by adding an errata which allocates the memory early
>> in the boot cycle, using the memblock allocator.
>>
>> Signed-off-by: Matthias Brugger <[email protected]>
>> ---
>> arch/arm64/Kconfig | 12 ++++++++
>> arch/arm64/include/asm/cpucaps.h | 3 +-
>> arch/arm64/kernel/cpu_errata.c | 33 +++++++++++++++++++++
>> drivers/irqchip/irq-gic-v3-its.c | 50 ++++++++++++++++++++------------
>> 4 files changed, 79 insertions(+), 19 deletions(-)
My only comment would be to state how much I dislike both the HW and the
patch... ;-) The idea that we have some erratum that depends on the page
size doesn't feel good at all.
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 1b1a0e95c751..dfd9fe08f0b2 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -597,6 +597,18 @@ config QCOM_FALKOR_ERRATUM_E1041
>>
>> If unsure, say Y.
>>
>> +config CAVIUM_ALLOC_ITS_TABLE_EARLY
>> + bool "Cavium Thunderx: Allocate the its table early"
>> + default y
>> + depends on ARM64_4K_PAGES && FORCE_MAX_ZONEORDER < 13
Here's a though: Why don't we ensure that FORCE_MAX_ZONEORDER is such as
we could always allocate the same amount of memory, no matter what the
page size is? That, or bump FORCE_MAX_ZONEORDER to 13 if the kernel
includes support for TX1.
Any of this of course requires buy-in from the arm64 maintainers, as
this is quite a departure from the way things work so far.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 05/10/2018 12:55, Marc Zyngier wrote:
> Hi Matthias,
>
> On 04/10/18 23:11, Matthias Brugger wrote:
>> Friendly reminder, if anyone has any comment on the patch :)
>>
>> On 9/12/18 11:52 AM, [email protected] wrote:
>>> From: Matthias Brugger <[email protected]>
>>>
>>> Some hardware does not implement two-level page tables so that
>>> the amount of contigious memory needed by the baser is bigger
>>> then the zone order. This is a known problem on Cavium Thunderx
>>> with 4K page size.
>>>
>>> We fix this by adding an errata which allocates the memory early
>>> in the boot cycle, using the memblock allocator.
>>>
>>> Signed-off-by: Matthias Brugger <[email protected]>
>>> ---
>>> arch/arm64/Kconfig | 12 ++++++++
>>> arch/arm64/include/asm/cpucaps.h | 3 +-
>>> arch/arm64/kernel/cpu_errata.c | 33 +++++++++++++++++++++
>>> drivers/irqchip/irq-gic-v3-its.c | 50 ++++++++++++++++++++------------
>>> 4 files changed, 79 insertions(+), 19 deletions(-)
>
> My only comment would be to state how much I dislike both the HW and the
> patch... ;-) The idea that we have some erratum that depends on the page size
> doesn't feel good at all.
>
Well ugly HW needs ugly patches ;-)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 1b1a0e95c751..dfd9fe08f0b2 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -597,6 +597,18 @@ config QCOM_FALKOR_ERRATUM_E1041
>>> If unsure, say Y.
>>> +config CAVIUM_ALLOC_ITS_TABLE_EARLY
>>> + bool "Cavium Thunderx: Allocate the its table early"
>>> + default y
>>> + depends on ARM64_4K_PAGES && FORCE_MAX_ZONEORDER < 13
>
> Here's a though: Why don't we ensure that FORCE_MAX_ZONEORDER is such as we
> could always allocate the same amount of memory, no matter what the page size
> is? That, or bump FORCE_MAX_ZONEORDER to 13 if the kernel includes support for TX1.
>
Bumping FORCE_MAX_ZONEORDER when TX1 is supported was proposed here:
https://patchwork.kernel.org/patch/6322281/
To bring in some more history, the CMA approach ended with this discussion:
https://patchwork.kernel.org/patch/9888041/
> Any of this of course requires buy-in from the arm64 maintainers, as this is
> quite a departure from the way things work so far.
>
With my distribution head on, I would prefer a solution that does not change
FORCE_MAX_ZONEORDER. That's how I came to the idea providing a third solution to
the same problem :)
Regards,
Matthias
On 05/10/18 13:33, Matthias Brugger wrote:
>
>
> On 05/10/2018 12:55, Marc Zyngier wrote:
>> Hi Matthias,
>>
>> On 04/10/18 23:11, Matthias Brugger wrote:
>>> Friendly reminder, if anyone has any comment on the patch :)
>>>
>>> On 9/12/18 11:52 AM, [email protected] wrote:
>>>> From: Matthias Brugger <[email protected]>
>>>>
>>>> Some hardware does not implement two-level page tables so that
>>>> the amount of contigious memory needed by the baser is bigger
>>>> then the zone order. This is a known problem on Cavium Thunderx
>>>> with 4K page size.
>>>>
>>>> We fix this by adding an errata which allocates the memory early
>>>> in the boot cycle, using the memblock allocator.
>>>>
>>>> Signed-off-by: Matthias Brugger <[email protected]>
>>>> ---
>>>> arch/arm64/Kconfig | 12 ++++++++
>>>> arch/arm64/include/asm/cpucaps.h | 3 +-
>>>> arch/arm64/kernel/cpu_errata.c | 33 +++++++++++++++++++++
>>>> drivers/irqchip/irq-gic-v3-its.c | 50 ++++++++++++++++++++------------
>>>> 4 files changed, 79 insertions(+), 19 deletions(-)
>>
>> My only comment would be to state how much I dislike both the HW and the
>> patch... ;-) The idea that we have some erratum that depends on the page size
>> doesn't feel good at all.
>>
>
> Well ugly HW needs ugly patches ;-)
>
>>>>
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 1b1a0e95c751..dfd9fe08f0b2 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -597,6 +597,18 @@ config QCOM_FALKOR_ERRATUM_E1041
>>>> If unsure, say Y.
>>>> +config CAVIUM_ALLOC_ITS_TABLE_EARLY
>>>> + bool "Cavium Thunderx: Allocate the its table early"
>>>> + default y
>>>> + depends on ARM64_4K_PAGES && FORCE_MAX_ZONEORDER < 13
>>
>> Here's a though: Why don't we ensure that FORCE_MAX_ZONEORDER is such as we
>> could always allocate the same amount of memory, no matter what the page size
>> is? That, or bump FORCE_MAX_ZONEORDER to 13 if the kernel includes support for TX1.
>>
>
> Bumping FORCE_MAX_ZONEORDER when TX1 is supported was proposed here:
> https://patchwork.kernel.org/patch/6322281/
>
> To bring in some more history, the CMA approach ended with this discussion:
> https://patchwork.kernel.org/patch/9888041/
>
>> Any of this of course requires buy-in from the arm64 maintainers, as this is
>> quite a departure from the way things work so far.
>>
>
> With my distribution head on, I would prefer a solution that does not change
> FORCE_MAX_ZONEORDER. That's how I came to the idea providing a third solution to
> the same problem :)
Why is that a problem? What impact does this have on your favourite distro?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 05/10/2018 15:42, Marc Zyngier wrote:
> On 05/10/18 13:33, Matthias Brugger wrote:
>>
>>
>> On 05/10/2018 12:55, Marc Zyngier wrote:
>>> Hi Matthias,
>>>
>>> On 04/10/18 23:11, Matthias Brugger wrote:
>>>> Friendly reminder, if anyone has any comment on the patch :)
>>>>
>>>> On 9/12/18 11:52 AM, [email protected] wrote:
>>>>> From: Matthias Brugger <[email protected]>
>>>>>
>>>>> Some hardware does not implement two-level page tables so that
>>>>> the amount of contigious memory needed by the baser is bigger
>>>>> then the zone order. This is a known problem on Cavium Thunderx
>>>>> with 4K page size.
>>>>>
>>>>> We fix this by adding an errata which allocates the memory early
>>>>> in the boot cycle, using the memblock allocator.
>>>>>
>>>>> Signed-off-by: Matthias Brugger <[email protected]>
>>>>> ---
>>>>> arch/arm64/Kconfig | 12 ++++++++
>>>>> arch/arm64/include/asm/cpucaps.h | 3 +-
>>>>> arch/arm64/kernel/cpu_errata.c | 33 +++++++++++++++++++++
>>>>> drivers/irqchip/irq-gic-v3-its.c | 50 ++++++++++++++++++++------------
>>>>> 4 files changed, 79 insertions(+), 19 deletions(-)
>>>
>>> My only comment would be to state how much I dislike both the HW and the
>>> patch... ;-) The idea that we have some erratum that depends on the page size
>>> doesn't feel good at all.
>>>
>>
>> Well ugly HW needs ugly patches ;-)
>>
>>>>>
>>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>>> index 1b1a0e95c751..dfd9fe08f0b2 100644
>>>>> --- a/arch/arm64/Kconfig
>>>>> +++ b/arch/arm64/Kconfig
>>>>> @@ -597,6 +597,18 @@ config QCOM_FALKOR_ERRATUM_E1041
>>>>> If unsure, say Y.
>>>>> +config CAVIUM_ALLOC_ITS_TABLE_EARLY
>>>>> + bool "Cavium Thunderx: Allocate the its table early"
>>>>> + default y
>>>>> + depends on ARM64_4K_PAGES && FORCE_MAX_ZONEORDER < 13
>>>
>>> Here's a though: Why don't we ensure that FORCE_MAX_ZONEORDER is such as we
>>> could always allocate the same amount of memory, no matter what the page size
>>> is? That, or bump FORCE_MAX_ZONEORDER to 13 if the kernel includes support
>>> for TX1.
>>>
>>
>> Bumping FORCE_MAX_ZONEORDER when TX1 is supported was proposed here:
>> https://patchwork.kernel.org/patch/6322281/
>>
>> To bring in some more history, the CMA approach ended with this discussion:
>> https://patchwork.kernel.org/patch/9888041/
>>
>>> Any of this of course requires buy-in from the arm64 maintainers, as this is
>>> quite a departure from the way things work so far.
>>>
>>
>> With my distribution head on, I would prefer a solution that does not change
>> FORCE_MAX_ZONEORDER. That's how I came to the idea providing a third solution to
>> the same problem :)
>
> Why is that a problem? What impact does this have on your favourite distro?
>
The impact is on changing FORCE_MAX_ZONEORDER on an already released kernel will
break Kernel ABI and with that all external modules. I know that's nothing
upstream cares too much about, but the distros do :)
On 05.10.18 16:13:48, Matthias Brugger wrote:
> On 05/10/2018 15:42, Marc Zyngier wrote:
> > On 05/10/18 13:33, Matthias Brugger wrote:
> >> With my distribution head on, I would prefer a solution that does not change
> >> FORCE_MAX_ZONEORDER. That's how I came to the idea providing a third solution to
> >> the same problem :)
> >
> > Why is that a problem? What impact does this have on your favourite distro?
> >
>
> The impact is on changing FORCE_MAX_ZONEORDER on an already released kernel will
> break Kernel ABI and with that all external modules. I know that's nothing
> upstream cares too much about, but the distros do :)
Wrt upstream, there is no way to do this change without patching the
kernel. Thus, it is impossible to run an unpatched 4k page size kernel
on ThunderX.
-Robert
On Fri, 05 Oct 2018 15:13:48 +0100,
Matthias Brugger <[email protected]> wrote:
>
>
>
> On 05/10/2018 15:42, Marc Zyngier wrote:
> > On 05/10/18 13:33, Matthias Brugger wrote:
> >>
> >>
> >> On 05/10/2018 12:55, Marc Zyngier wrote:
> >>> Hi Matthias,
> >>>
> >>> On 04/10/18 23:11, Matthias Brugger wrote:
> >>>> Friendly reminder, if anyone has any comment on the patch :)
> >>>>
> >>>> On 9/12/18 11:52 AM, [email protected] wrote:
> >>>>> From: Matthias Brugger <[email protected]>
> >>>>>
> >>>>> Some hardware does not implement two-level page tables so that
> >>>>> the amount of contigious memory needed by the baser is bigger
> >>>>> then the zone order. This is a known problem on Cavium Thunderx
> >>>>> with 4K page size.
> >>>>>
> >>>>> We fix this by adding an errata which allocates the memory early
> >>>>> in the boot cycle, using the memblock allocator.
> >>>>>
> >>>>> Signed-off-by: Matthias Brugger <[email protected]>
> >>>>> ---
> >>>>> arch/arm64/Kconfig | 12 ++++++++
> >>>>> arch/arm64/include/asm/cpucaps.h | 3 +-
> >>>>> arch/arm64/kernel/cpu_errata.c | 33 +++++++++++++++++++++
> >>>>> drivers/irqchip/irq-gic-v3-its.c | 50 ++++++++++++++++++++------------
> >>>>> 4 files changed, 79 insertions(+), 19 deletions(-)
> >>>
> >>> My only comment would be to state how much I dislike both the HW and the
> >>> patch... ;-) The idea that we have some erratum that depends on the page size
> >>> doesn't feel good at all.
> >>>
> >>
> >> Well ugly HW needs ugly patches ;-)
> >>
> >>>>>
> >>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >>>>> index 1b1a0e95c751..dfd9fe08f0b2 100644
> >>>>> --- a/arch/arm64/Kconfig
> >>>>> +++ b/arch/arm64/Kconfig
> >>>>> @@ -597,6 +597,18 @@ config QCOM_FALKOR_ERRATUM_E1041
> >>>>> If unsure, say Y.
> >>>>> +config CAVIUM_ALLOC_ITS_TABLE_EARLY
> >>>>> + bool "Cavium Thunderx: Allocate the its table early"
> >>>>> + default y
> >>>>> + depends on ARM64_4K_PAGES && FORCE_MAX_ZONEORDER < 13
> >>>
> >>> Here's a though: Why don't we ensure that FORCE_MAX_ZONEORDER is such as we
> >>> could always allocate the same amount of memory, no matter what the page size
> >>> is? That, or bump FORCE_MAX_ZONEORDER to 13 if the kernel includes support
> >>> for TX1.
> >>>
> >>
> >> Bumping FORCE_MAX_ZONEORDER when TX1 is supported was proposed here:
> >> https://patchwork.kernel.org/patch/6322281/
> >>
> >> To bring in some more history, the CMA approach ended with this discussion:
> >> https://patchwork.kernel.org/patch/9888041/
> >>
> >>> Any of this of course requires buy-in from the arm64 maintainers, as this is
> >>> quite a departure from the way things work so far.
> >>>
> >>
> >> With my distribution head on, I would prefer a solution that does not change
> >> FORCE_MAX_ZONEORDER. That's how I came to the idea providing a third solution to
> >> the same problem :)
> >
> > Why is that a problem? What impact does this have on your favourite distro?
> >
>
> The impact is on changing FORCE_MAX_ZONEORDER on an already released
> kernel will break Kernel ABI and with that all external modules. I
> know that's nothing upstream cares too much about, but the distros
> do :)
Unfortunately, that's something you're bringing upon yourself, and I'm
afraid I can't really take this into account. You could always bump
that ABI if you really want to support this platform as, at the end of
the day, this is something you're in control of.
But I'd really like to hear what Catalin or Will think of this (Will
wasn't massively impressed by this 3 years ago, and I wonder if his
approach has changed since).
Thanks,
M.
--
Jazz is not dead, it just smell funny.
On Fri, Oct 05, 2018 at 04:17:30PM +0100, Marc Zyngier wrote:
> On Fri, 05 Oct 2018 15:13:48 +0100,
> Matthias Brugger <[email protected]> wrote:
> >
> >
> >
> > On 05/10/2018 15:42, Marc Zyngier wrote:
> > > On 05/10/18 13:33, Matthias Brugger wrote:
> > >>
> > >>
> > >> On 05/10/2018 12:55, Marc Zyngier wrote:
> > >>> Hi Matthias,
> > >>>
> > >>> On 04/10/18 23:11, Matthias Brugger wrote:
> > >>>> Friendly reminder, if anyone has any comment on the patch :)
> > >>>>
> > >>>> On 9/12/18 11:52 AM, [email protected] wrote:
> > >>>>> From: Matthias Brugger <[email protected]>
> > >>>>>
> > >>>>> Some hardware does not implement two-level page tables so that
> > >>>>> the amount of contigious memory needed by the baser is bigger
> > >>>>> then the zone order. This is a known problem on Cavium Thunderx
> > >>>>> with 4K page size.
> > >>>>>
> > >>>>> We fix this by adding an errata which allocates the memory early
> > >>>>> in the boot cycle, using the memblock allocator.
> > >>>>>
> > >>>>> Signed-off-by: Matthias Brugger <[email protected]>
> > >>>>> ---
> > >>>>> ?? arch/arm64/Kconfig?????????????? | 12 ++++++++
> > >>>>> ?? arch/arm64/include/asm/cpucaps.h |? 3 +-
> > >>>>> ?? arch/arm64/kernel/cpu_errata.c?? | 33 +++++++++++++++++++++
> > >>>>> ?? drivers/irqchip/irq-gic-v3-its.c | 50 ++++++++++++++++++++------------
> > >>>>> ?? 4 files changed, 79 insertions(+), 19 deletions(-)
> > >>>
> > >>> My only comment would be to state how much I dislike both the HW and the
> > >>> patch... ;-) The idea that we have some erratum that depends on the page size
> > >>> doesn't feel good at all.
> > >>>
> > >>
> > >> Well ugly HW needs ugly patches ;-)
> > >>
> > >>>>>
> > >>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > >>>>> index 1b1a0e95c751..dfd9fe08f0b2 100644
> > >>>>> --- a/arch/arm64/Kconfig
> > >>>>> +++ b/arch/arm64/Kconfig
> > >>>>> @@ -597,6 +597,18 @@ config QCOM_FALKOR_ERRATUM_E1041
> > >>>>> ?? ??????? If unsure, say Y.
> > >>>>> ?? +config CAVIUM_ALLOC_ITS_TABLE_EARLY
> > >>>>> +??? bool "Cavium Thunderx: Allocate the its table early"
> > >>>>> +??? default y
> > >>>>> +??? depends on ARM64_4K_PAGES && FORCE_MAX_ZONEORDER < 13
> > >>>
> > >>> Here's a though: Why don't we ensure that FORCE_MAX_ZONEORDER is such as we
> > >>> could always allocate the same amount of memory, no matter what the page size
> > >>> is? That, or bump FORCE_MAX_ZONEORDER to 13 if the kernel includes support
> > >>> for TX1.
> > >>>
> > >>
> > >> Bumping FORCE_MAX_ZONEORDER when TX1 is supported was proposed here:
> > >> https://patchwork.kernel.org/patch/6322281/
> > >>
> > >> To bring in some more history, the CMA approach ended with this discussion:
> > >> https://patchwork.kernel.org/patch/9888041/
> > >>
> > >>> Any of this of course requires buy-in from the arm64 maintainers, as this is
> > >>> quite a departure from the way things work so far.
> > >>>
> > >>
> > >> With my distribution head on, I would prefer a solution that does not change
> > >> FORCE_MAX_ZONEORDER. That's how I came to the idea providing a third solution to
> > >> the same problem :)
> > >
> > > Why is that a problem? What impact does this have on your favourite distro?
> > >
> >
> > The impact is on changing FORCE_MAX_ZONEORDER on an already released
> > kernel will break Kernel ABI and with that all external modules. I
> > know that's nothing upstream cares too much about, but the distros
> > do :)
>
> Unfortunately, that's something you're bringing upon yourself, and I'm
> afraid I can't really take this into account. You could always bump
> that ABI if you really want to support this platform as, at the end of
> the day, this is something you're in control of.
>
> But I'd really like to hear what Catalin or Will think of this (Will
> wasn't massively impressed by this 3 years ago, and I wonder if his
> approach has changed since).
I don't see anything that changes my opinion here, and the reality is
that bumping FORCE_MAX_ZONEORDER doesn't guarantee anything about the
allocation succeeding. I'm also hesitant to punish other platforms
(including TX2!) because of this TX1 "feature".
One thing I'm unsure about is why the CMA approach failed; the link
above is a complain about the use of subsys_initcall() afaict.
Will