* [PATCH] irqchip/gicv3-its: Avoid cache flush beyond ITS_BASERn memory size
@ 2016-02-16 3:41 Shanker Donthineni
2016-02-16 8:35 ` Marc Zyngier
0 siblings, 1 reply; 5+ messages in thread
From: Shanker Donthineni @ 2016-02-16 3:41 UTC (permalink / raw)
To: linux-arm-kernel
Function its_alloc_tables() maintains two local variables, "order" and
and "alloc_size", to hold memory size that has been allocated to
ITS_BASEn. We don't always refresh the variable alloc_size whenever
value of the variable order changes, causing the following two problems.
- Cache flush operation with size more than required.
- Information reported by pr_info is not correct.
Use a helper macro that converts page order to size in bytes instead of
variable "alloc_size" to fix both the problems.
Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
drivers/irqchip/irq-gic-v3-its.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 0a73632..6f717f7 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -78,6 +78,9 @@ struct its_node {
#define ITS_ITT_ALIGN SZ_256
+/* Convert page order to size in bytes */
+#define PAGE_ORDER_TO_BYTES(o) (PAGE_SIZE << (o))
+
struct event_lpi_map {
unsigned long *lpi_map;
u16 *col_map;
@@ -846,7 +849,6 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
u64 type = GITS_BASER_TYPE(val);
u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
int order = get_order(psz);
- int alloc_size;
int alloc_pages;
u64 tmp;
void *base;
@@ -878,9 +880,8 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
}
}
- alloc_size = (1 << order) * PAGE_SIZE;
retry_alloc_baser:
- alloc_pages = (alloc_size / psz);
+ alloc_pages = (PAGE_ORDER_TO_BYTES(order) / psz);
if (alloc_pages > GITS_BASER_PAGES_MAX) {
alloc_pages = GITS_BASER_PAGES_MAX;
order = get_order(GITS_BASER_PAGES_MAX * psz);
@@ -933,7 +934,8 @@ retry_baser:
shr = tmp & GITS_BASER_SHAREABILITY_MASK;
if (!shr) {
cache = GITS_BASER_nC;
- __flush_dcache_area(base, alloc_size);
+ __flush_dcache_area(base,
+ PAGE_ORDER_TO_BYTES(order));
}
goto retry_baser;
}
@@ -966,7 +968,7 @@ retry_baser:
}
pr_info("ITS: allocated %d %s @%lx (psz %dK, shr %d)\n",
- (int)(alloc_size / entry_size),
+ (int)(PAGE_ORDER_TO_BYTES(order) / entry_size),
its_base_type_string[type],
(unsigned long)virt_to_phys(base),
psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
--
Qualcomm Technologies, Inc. on behalf
of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc.
is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] irqchip/gicv3-its: Avoid cache flush beyond ITS_BASERn memory size
2016-02-16 3:41 [PATCH] irqchip/gicv3-its: Avoid cache flush beyond ITS_BASERn memory size Shanker Donthineni
@ 2016-02-16 8:35 ` Marc Zyngier
2016-02-16 23:03 ` Shanker Donthineni
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Marc Zyngier @ 2016-02-16 8:35 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 15 Feb 2016 21:41:52 -0600
Shanker Donthineni <shankerd@codeaurora.org> wrote:
> Function its_alloc_tables() maintains two local variables, "order" and
> and "alloc_size", to hold memory size that has been allocated to
> ITS_BASEn. We don't always refresh the variable alloc_size whenever
> value of the variable order changes, causing the following two problems.
>
> - Cache flush operation with size more than required.
> - Information reported by pr_info is not correct.
>
> Use a helper macro that converts page order to size in bytes instead of
> variable "alloc_size" to fix both the problems.
>
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> ---
> drivers/irqchip/irq-gic-v3-its.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 0a73632..6f717f7 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -78,6 +78,9 @@ struct its_node {
>
> #define ITS_ITT_ALIGN SZ_256
>
> +/* Convert page order to size in bytes */
> +#define PAGE_ORDER_TO_BYTES(o) (PAGE_SIZE << (o))
> +
nit: PAGE_ORDER_TO_size() seems more appropriate.
> struct event_lpi_map {
> unsigned long *lpi_map;
> u16 *col_map;
> @@ -846,7 +849,6 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
> u64 type = GITS_BASER_TYPE(val);
> u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
> int order = get_order(psz);
> - int alloc_size;
> int alloc_pages;
> u64 tmp;
> void *base;
> @@ -878,9 +880,8 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
> }
> }
>
> - alloc_size = (1 << order) * PAGE_SIZE;
> retry_alloc_baser:
> - alloc_pages = (alloc_size / psz);
> + alloc_pages = (PAGE_ORDER_TO_BYTES(order) / psz);
> if (alloc_pages > GITS_BASER_PAGES_MAX) {
> alloc_pages = GITS_BASER_PAGES_MAX;
> order = get_order(GITS_BASER_PAGES_MAX * psz);
> @@ -933,7 +934,8 @@ retry_baser:
> shr = tmp & GITS_BASER_SHAREABILITY_MASK;
> if (!shr) {
> cache = GITS_BASER_nC;
> - __flush_dcache_area(base, alloc_size);
> + __flush_dcache_area(base,
> + PAGE_ORDER_TO_BYTES(order));
Please keep it on the same line. It helps my random grepping...
> }
> goto retry_baser;
> }
> @@ -966,7 +968,7 @@ retry_baser:
> }
>
> pr_info("ITS: allocated %d %s @%lx (psz %dK, shr %d)\n",
> - (int)(alloc_size / entry_size),
> + (int)(PAGE_ORDER_TO_BYTES(order) / entry_size),
> its_base_type_string[type],
> (unsigned long)virt_to_phys(base),
> psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
Otherwise looks good.
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] irqchip/gicv3-its: Avoid cache flush beyond ITS_BASERn memory size
2016-02-16 8:35 ` Marc Zyngier
@ 2016-02-16 23:03 ` Shanker Donthineni
2016-02-16 23:08 ` Shanker Donthineni
2016-02-16 23:27 ` Shanker Donthineni
2 siblings, 0 replies; 5+ messages in thread
From: Shanker Donthineni @ 2016-02-16 23:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi Marc,
On 02/16/2016 02:35 AM, Marc Zyngier wrote:
> On Mon, 15 Feb 2016 21:41:52 -0600
> Shanker Donthineni <shankerd@codeaurora.org> wrote:
>
>> Function its_alloc_tables() maintains two local variables, "order" and
>> and "alloc_size", to hold memory size that has been allocated to
>> ITS_BASEn. We don't always refresh the variable alloc_size whenever
>> value of the variable order changes, causing the following two problems.
>>
>> - Cache flush operation with size more than required.
>> - Information reported by pr_info is not correct.
>>
>> Use a helper macro that converts page order to size in bytes instead of
>> variable "alloc_size" to fix both the problems.
>>
>> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 0a73632..6f717f7 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -78,6 +78,9 @@ struct its_node {
>>
>> #define ITS_ITT_ALIGN SZ_256
>>
>> +/* Convert page order to size in bytes */
>> +#define PAGE_ORDER_TO_BYTES(o) (PAGE_SIZE << (o))
>> +
> nit: PAGE_ORDER_TO_size() seems more appropriate.
>
I will change to PAGE_ORDER_TO_SIZE in v2 patch.
>> struct event_lpi_map {
>> unsigned long *lpi_map;
>> u16 *col_map;
>> @@ -846,7 +849,6 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>> u64 type = GITS_BASER_TYPE(val);
>> u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
>> int order = get_order(psz);
>> - int alloc_size;
>> int alloc_pages;
>> u64 tmp;
>> void *base;
>> @@ -878,9 +880,8 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>> }
>> }
>>
>> - alloc_size = (1 << order) * PAGE_SIZE;
>> retry_alloc_baser:
>> - alloc_pages = (alloc_size / psz);
>> + alloc_pages = (PAGE_ORDER_TO_BYTES(order) / psz);
>> if (alloc_pages > GITS_BASER_PAGES_MAX) {
>> alloc_pages = GITS_BASER_PAGES_MAX;
>> order = get_order(GITS_BASER_PAGES_MAX * psz);
>> @@ -933,7 +934,8 @@ retry_baser:
>> shr = tmp & GITS_BASER_SHAREABILITY_MASK;
>> if (!shr) {
>> cache = GITS_BASER_nC;
>> - __flush_dcache_area(base, alloc_size);
>> + __flush_dcache_area(base,
>> + PAGE_ORDER_TO_BYTES(order));
> Please keep it on the same line. It helps my random grepping...
Sure, I will keep it on the same line in v2 patch.
>> }
>> goto retry_baser;
>> }
>> @@ -966,7 +968,7 @@ retry_baser:
>> }
>>
>> pr_info("ITS: allocated %d %s @%lx (psz %dK, shr %d)\n",
>> - (int)(alloc_size / entry_size),
>> + (int)(PAGE_ORDER_TO_BYTES(order) / entry_size),
>> its_base_type_string[type],
>> (unsigned long)virt_to_phys(base),
>> psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
>
> Otherwise looks good.
>
> Thanks,
>
> M.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] irqchip/gicv3-its: Avoid cache flush beyond ITS_BASERn memory size
2016-02-16 8:35 ` Marc Zyngier
2016-02-16 23:03 ` Shanker Donthineni
@ 2016-02-16 23:08 ` Shanker Donthineni
2016-02-16 23:27 ` Shanker Donthineni
2 siblings, 0 replies; 5+ messages in thread
From: Shanker Donthineni @ 2016-02-16 23:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi Marc,
On 02/16/2016 02:35 AM, Marc Zyngier wrote:
> On Mon, 15 Feb 2016 21:41:52 -0600
> Shanker Donthineni <shankerd@codeaurora.org> wrote:
>
>> Function its_alloc_tables() maintains two local variables, "order" and
>> and "alloc_size", to hold memory size that has been allocated to
>> ITS_BASEn. We don't always refresh the variable alloc_size whenever
>> value of the variable order changes, causing the following two problems.
>>
>> - Cache flush operation with size more than required.
>> - Information reported by pr_info is not correct.
>>
>> Use a helper macro that converts page order to size in bytes instead of
>> variable "alloc_size" to fix both the problems.
>>
>> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 0a73632..6f717f7 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -78,6 +78,9 @@ struct its_node {
>>
>> #define ITS_ITT_ALIGN SZ_256
>>
>> +/* Convert page order to size in bytes */
>> +#define PAGE_ORDER_TO_BYTES(o) (PAGE_SIZE << (o))
>> +
> nit: PAGE_ORDER_TO_size() seems more appropriate.
I will change to PAGE_ORDER_TO_SIZE in v2 patch.
>> struct event_lpi_map {
>> unsigned long *lpi_map;
>> u16 *col_map;
>> @@ -846,7 +849,6 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>> u64 type = GITS_BASER_TYPE(val);
>> u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
>> int order = get_order(psz);
>> - int alloc_size;
>> int alloc_pages;
>> u64 tmp;
>> void *base;
>> @@ -878,9 +880,8 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>> }
>> }
>>
>> - alloc_size = (1 << order) * PAGE_SIZE;
>> retry_alloc_baser:
>> - alloc_pages = (alloc_size / psz);
>> + alloc_pages = (PAGE_ORDER_TO_BYTES(order) / psz);
>> if (alloc_pages > GITS_BASER_PAGES_MAX) {
>> alloc_pages = GITS_BASER_PAGES_MAX;
>> order = get_order(GITS_BASER_PAGES_MAX * psz);
>> @@ -933,7 +934,8 @@ retry_baser:
>> shr = tmp & GITS_BASER_SHAREABILITY_MASK;
>> if (!shr) {
>> cache = GITS_BASER_nC;
>> - __flush_dcache_area(base, alloc_size);
>> + __flush_dcache_area(base,
>> + PAGE_ORDER_TO_BYTES(order));
> Please keep it on the same line. It helps my random grepping...
Sure, I will keep it on the same line in v2 patch.
>> }
>> goto retry_baser;
>> }
>> @@ -966,7 +968,7 @@ retry_baser:
>> }
>>
>> pr_info("ITS: allocated %d %s @%lx (psz %dK, shr %d)\n",
>> - (int)(alloc_size / entry_size),
>> + (int)(PAGE_ORDER_TO_BYTES(order) / entry_size),
>> its_base_type_string[type],
>> (unsigned long)virt_to_phys(base),
>> psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
>
> Otherwise looks good.
>
> Thanks,
>
> M.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] irqchip/gicv3-its: Avoid cache flush beyond ITS_BASERn memory size
2016-02-16 8:35 ` Marc Zyngier
2016-02-16 23:03 ` Shanker Donthineni
2016-02-16 23:08 ` Shanker Donthineni
@ 2016-02-16 23:27 ` Shanker Donthineni
2 siblings, 0 replies; 5+ messages in thread
From: Shanker Donthineni @ 2016-02-16 23:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Marc,
On 02/16/2016 02:35 AM, Marc Zyngier wrote:
> On Mon, 15 Feb 2016 21:41:52 -0600
> Shanker Donthineni <shankerd@codeaurora.org> wrote:
>
>> Function its_alloc_tables() maintains two local variables, "order" and
>> and "alloc_size", to hold memory size that has been allocated to
>> ITS_BASEn. We don't always refresh the variable alloc_size whenever
>> value of the variable order changes, causing the following two problems.
>>
>> - Cache flush operation with size more than required.
>> - Information reported by pr_info is not correct.
>>
>> Use a helper macro that converts page order to size in bytes instead of
>> variable "alloc_size" to fix both the problems.
>>
>> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
>> ---
>> drivers/irqchip/irq-gic-v3-its.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 0a73632..6f717f7 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -78,6 +78,9 @@ struct its_node {
>>
>> #define ITS_ITT_ALIGN SZ_256
>>
>> +/* Convert page order to size in bytes */
>> +#define PAGE_ORDER_TO_BYTES(o) (PAGE_SIZE << (o))
>> +
> nit: PAGE_ORDER_TO_size() seems more appropriate.
I will change to PAGE_ORDER_TO_SIZE in v2 patch.
>> struct event_lpi_map {
>> unsigned long *lpi_map;
>> u16 *col_map;
>> @@ -846,7 +849,6 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>> u64 type = GITS_BASER_TYPE(val);
>> u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
>> int order = get_order(psz);
>> - int alloc_size;
>> int alloc_pages;
>> u64 tmp;
>> void *base;
>> @@ -878,9 +880,8 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>> }
>> }
>>
>> - alloc_size = (1 << order) * PAGE_SIZE;
>> retry_alloc_baser:
>> - alloc_pages = (alloc_size / psz);
>> + alloc_pages = (PAGE_ORDER_TO_BYTES(order) / psz);
>> if (alloc_pages > GITS_BASER_PAGES_MAX) {
>> alloc_pages = GITS_BASER_PAGES_MAX;
>> order = get_order(GITS_BASER_PAGES_MAX * psz);
>> @@ -933,7 +934,8 @@ retry_baser:
>> shr = tmp & GITS_BASER_SHAREABILITY_MASK;
>> if (!shr) {
>> cache = GITS_BASER_nC;
>> - __flush_dcache_area(base, alloc_size);
>> + __flush_dcache_area(base,
>> + PAGE_ORDER_TO_BYTES(order));
> Please keep it on the same line. It helps my random grepping...
Sure, I will keep it on the same line in v2 patch.
>> }
>> goto retry_baser;
>> }
>> @@ -966,7 +968,7 @@ retry_baser:
>> }
>>
>> pr_info("ITS: allocated %d %s @%lx (psz %dK, shr %d)\n",
>> - (int)(alloc_size / entry_size),
>> + (int)(PAGE_ORDER_TO_BYTES(order) / entry_size),
>> its_base_type_string[type],
>> (unsigned long)virt_to_phys(base),
>> psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
>
> Otherwise looks good.
>
> Thanks,
>
> M.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-16 23:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-16 3:41 [PATCH] irqchip/gicv3-its: Avoid cache flush beyond ITS_BASERn memory size Shanker Donthineni
2016-02-16 8:35 ` Marc Zyngier
2016-02-16 23:03 ` Shanker Donthineni
2016-02-16 23:08 ` Shanker Donthineni
2016-02-16 23:27 ` Shanker Donthineni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).