* [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables @ 2015-04-07 7:47 Zhen Lei 2015-04-07 7:47 ` [PATCH 2/3] irqchip/gicv3-its: remove GITS_BASER_TYPE_CPU base on latest spec Zhen Lei ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Zhen Lei @ 2015-04-07 7:47 UTC (permalink / raw) To: Thomas Gleixner, Jason Cooper, Marc Zyngier, linux-kernel Cc: Zefan Li, Xinwei Hu, Tianhong Ding, Kefeng Wang, Yun Wu, Zhen Lei For the old version, there maybe some faults: 1. For each Interrupt Collection table, 4K size is enough. But now the default is 64K(if "Page Size" field is not read-only). 2. Suppose two read-only "Page Size" table exist, and software found 4K first, then 16K. For the first time, the value of local variable "psz" will drop to 4K, but we have not reinitialize it again in the loop. So, further process 16K read-only "Page Size" will report error. In this patch, detect all the read-only fields first. So that, no need to try over and over again. Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- drivers/irqchip/irq-gic-v3-its.c | 105 +++++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 43 deletions(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 9687f8a..2577f06 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -800,20 +800,21 @@ static int its_alloc_tables(struct its_node *its) { int err; int i; - int psz = SZ_64K; - u64 shr = GITS_BASER_InnerShareable; - u64 cache = GITS_BASER_WaWb; + int psz; + u64 shr; + u64 cache; for (i = 0; i < GITS_BASER_NR_REGS; i++) { u64 val = readq_relaxed(its->base + GITS_BASER + i * 8); u64 type = GITS_BASER_TYPE(val); u64 entry_size = GITS_BASER_ENTRY_SIZE(val); - int order = get_order(psz); + int order; int alloc_size; u64 tmp; void *base; - if (type == GITS_BASER_TYPE_NONE) + switch (type) { + case GITS_BASER_TYPE_NONE: continue; /* @@ -824,16 +825,65 @@ static int its_alloc_tables(struct its_node *its) * * For other tables, only allocate a single page. */ - if (type == GITS_BASER_TYPE_DEVICE) { + case GITS_BASER_TYPE_DEVICE: { u64 typer = readq_relaxed(its->base + GITS_TYPER); u32 ids = GITS_TYPER_DEVBITS(typer); - order = get_order((1UL << ids) * entry_size); - if (order >= MAX_ORDER) { - order = MAX_ORDER - 1; - pr_warn("%s: Device Table too large, reduce its page order to %u\n", - its->msi_chip.of_node->full_name, order); + alloc_size = (1UL << ids) * entry_size; + break; } + + default: + alloc_size = PAGE_SIZE; + break; + } + + psz = PAGE_SIZE; + shr = GITS_BASER_InnerShareable; + cache = GITS_BASER_WaWb; + + /* Test the fields that maybe read-only */ + tmp = shr | (~val & GITS_BASER_PAGE_SIZE_MASK); + writeq_relaxed(tmp, its->base + GITS_BASER + i * 8); + tmp = readq_relaxed(its->base + GITS_BASER + i * 8); + + /* + * Shareability didn't stick. Just use + * whatever the read reported, which is likely + * to be the only thing this redistributor + * supports. If that's zero, make it + * non-cacheable as well. + */ + shr = tmp & GITS_BASER_SHAREABILITY_MASK; + if (!shr) + cache = GITS_BASER_nC; + + /* + * "Page Size" field is read-only. Should ensure the table + * start address and size align to it. + */ + if (!((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK)) + switch (val & GITS_BASER_PAGE_SIZE_MASK) { + case GITS_BASER_PAGE_SIZE_4K: + psz = SZ_4K; + break; + + case GITS_BASER_PAGE_SIZE_16K: + psz = SZ_16K; + break; + + default: + psz = SZ_64K; + break; + } + + alloc_size = ALIGN(alloc_size, psz); + + order = get_order(alloc_size); + if (order >= MAX_ORDER) { + order = MAX_ORDER - 1; + pr_warn("%s: Device Table too large, reduce its page order to %u\n", + its->msi_chip.of_node->full_name, order); } alloc_size = (1 << order) * PAGE_SIZE; @@ -845,7 +895,6 @@ static int its_alloc_tables(struct its_node *its) its->tables[i] = base; -retry_baser: val = (virt_to_phys(base) | (type << GITS_BASER_TYPE_SHIFT) | ((entry_size - 1) << GITS_BASER_ENTRY_SIZE_SHIFT) | @@ -870,40 +919,10 @@ retry_baser: writeq_relaxed(val, its->base + GITS_BASER + i * 8); tmp = readq_relaxed(its->base + GITS_BASER + i * 8); - if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) { - /* - * Shareability didn't stick. Just use - * whatever the read reported, which is likely - * to be the only thing this redistributor - * supports. If that's zero, make it - * non-cacheable as well. - */ - shr = tmp & GITS_BASER_SHAREABILITY_MASK; - if (!shr) - cache = GITS_BASER_nC; - goto retry_baser; - } - - if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) { - /* - * Page size didn't stick. Let's try a smaller - * size and retry. If we reach 4K, then - * something is horribly wrong... - */ - switch (psz) { - case SZ_16K: - psz = SZ_4K; - goto retry_baser; - case SZ_64K: - psz = SZ_16K; - goto retry_baser; - } - } - if (val != tmp) { pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n", its->msi_chip.of_node->full_name, i, - (unsigned long) val, (unsigned long) tmp); + (unsigned long)val, (unsigned long)tmp); err = -ENXIO; goto out_free; } -- 1.8.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] irqchip/gicv3-its: remove GITS_BASER_TYPE_CPU base on latest spec 2015-04-07 7:47 [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables Zhen Lei @ 2015-04-07 7:47 ` Zhen Lei 2015-04-07 7:47 ` [PATCH 3/3] irqchip/gicv3-its: treat type reserved as 0x0 Zhen Lei 2015-04-07 9:33 ` [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables Marc Zyngier 2 siblings, 0 replies; 7+ messages in thread From: Zhen Lei @ 2015-04-07 7:47 UTC (permalink / raw) To: Thomas Gleixner, Jason Cooper, Marc Zyngier, linux-kernel Cc: Zefan Li, Xinwei Hu, Tianhong Ding, Kefeng Wang, Yun Wu, Zhen Lei In the latest specification(version 24.0), clause 5.12.13 GITS_BASERn. The meaning of value=0x3 in "Type" field was revised to reserved. As below: 0x3. Reserved. In the early versions(like 19.0), it defined as below: 0x3. Physical Processors. Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- drivers/irqchip/irq-gic-v3-its.c | 2 +- include/linux/irqchip/arm-gic-v3.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 2577f06..bbf9504 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -777,7 +777,7 @@ static int __init its_alloc_lpi_tables(void) static const char *its_base_type_string[] = { [GITS_BASER_TYPE_DEVICE] = "Devices", [GITS_BASER_TYPE_VCPU] = "Virtual CPUs", - [GITS_BASER_TYPE_CPU] = "Physical CPUs", + [GITS_BASER_TYPE_RESERVED3] = "Reserved (3)", [GITS_BASER_TYPE_COLLECTION] = "Interrupt Collections", [GITS_BASER_TYPE_RESERVED5] = "Reserved (5)", [GITS_BASER_TYPE_RESERVED6] = "Reserved (6)", diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h index ffbc034..67f5779 100644 --- a/include/linux/irqchip/arm-gic-v3.h +++ b/include/linux/irqchip/arm-gic-v3.h @@ -233,7 +233,7 @@ #define GITS_BASER_TYPE_NONE 0 #define GITS_BASER_TYPE_DEVICE 1 #define GITS_BASER_TYPE_VCPU 2 -#define GITS_BASER_TYPE_CPU 3 +#define GITS_BASER_TYPE_RESERVED3 3 #define GITS_BASER_TYPE_COLLECTION 4 #define GITS_BASER_TYPE_RESERVED5 5 #define GITS_BASER_TYPE_RESERVED6 6 -- 1.8.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] irqchip/gicv3-its: treat type reserved as 0x0 2015-04-07 7:47 [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables Zhen Lei 2015-04-07 7:47 ` [PATCH 2/3] irqchip/gicv3-its: remove GITS_BASER_TYPE_CPU base on latest spec Zhen Lei @ 2015-04-07 7:47 ` Zhen Lei 2015-04-07 9:33 ` [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables Marc Zyngier 2 siblings, 0 replies; 7+ messages in thread From: Zhen Lei @ 2015-04-07 7:47 UTC (permalink / raw) To: Thomas Gleixner, Jason Cooper, Marc Zyngier, linux-kernel Cc: Zefan Li, Xinwei Hu, Tianhong Ding, Kefeng Wang, Yun Wu, Zhen Lei 1. We don't known how many memory needed by type reserved. It's good to take no care about it. 2. Remove unused macro definition and name of type reserved. Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- drivers/irqchip/irq-gic-v3-its.c | 19 +++++++++++-------- include/linux/irqchip/arm-gic-v3.h | 4 ---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index bbf9504..d649910 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -777,11 +777,7 @@ static int __init its_alloc_lpi_tables(void) static const char *its_base_type_string[] = { [GITS_BASER_TYPE_DEVICE] = "Devices", [GITS_BASER_TYPE_VCPU] = "Virtual CPUs", - [GITS_BASER_TYPE_RESERVED3] = "Reserved (3)", [GITS_BASER_TYPE_COLLECTION] = "Interrupt Collections", - [GITS_BASER_TYPE_RESERVED5] = "Reserved (5)", - [GITS_BASER_TYPE_RESERVED6] = "Reserved (6)", - [GITS_BASER_TYPE_RESERVED7] = "Reserved (7)", }; static void its_free_tables(struct its_node *its) @@ -814,9 +810,6 @@ static int its_alloc_tables(struct its_node *its) void *base; switch (type) { - case GITS_BASER_TYPE_NONE: - continue; - /* * Allocate as many entries as required to fit the * range of device IDs that the ITS can grok... The ID @@ -833,9 +826,19 @@ static int its_alloc_tables(struct its_node *its) break; } - default: + case GITS_BASER_TYPE_VCPU: + case GITS_BASER_TYPE_COLLECTION: alloc_size = PAGE_SIZE; break; + + /* + * Here treat type Reserved as 0x0(NONE). If type reserved need + * memory, it should be allocated by BIOS/UEFI, OS take no care + * about it. + */ + case GITS_BASER_TYPE_NONE: + default: + continue; } psz = PAGE_SIZE; diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h index 67f5779..212df88 100644 --- a/include/linux/irqchip/arm-gic-v3.h +++ b/include/linux/irqchip/arm-gic-v3.h @@ -233,11 +233,7 @@ #define GITS_BASER_TYPE_NONE 0 #define GITS_BASER_TYPE_DEVICE 1 #define GITS_BASER_TYPE_VCPU 2 -#define GITS_BASER_TYPE_RESERVED3 3 #define GITS_BASER_TYPE_COLLECTION 4 -#define GITS_BASER_TYPE_RESERVED5 5 -#define GITS_BASER_TYPE_RESERVED6 6 -#define GITS_BASER_TYPE_RESERVED7 7 /* * ITS commands -- 1.8.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables 2015-04-07 7:47 [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables Zhen Lei 2015-04-07 7:47 ` [PATCH 2/3] irqchip/gicv3-its: remove GITS_BASER_TYPE_CPU base on latest spec Zhen Lei 2015-04-07 7:47 ` [PATCH 3/3] irqchip/gicv3-its: treat type reserved as 0x0 Zhen Lei @ 2015-04-07 9:33 ` Marc Zyngier 2015-04-07 12:32 ` leizhen 2 siblings, 1 reply; 7+ messages in thread From: Marc Zyngier @ 2015-04-07 9:33 UTC (permalink / raw) To: Zhen Lei Cc: Thomas Gleixner, Jason Cooper, linux-kernel, Zefan Li, huxinwei, Tianhong Ding, Kefeng Wang, Yun Wu On Tue, 7 Apr 2015 08:47:32 +0100 Zhen Lei <thunder.leizhen@huawei.com> wrote: > For the old version, there maybe some faults: > 1. For each Interrupt Collection table, 4K size is enough. But now the > default is 64K(if "Page Size" field is not read-only). Why is that a problem? > 2. Suppose two read-only "Page Size" table exist, and software found 4K > first, then 16K. For the first time, the value of local variable "psz" > will drop to 4K, but we have not reinitialize it again in the loop. > So, further process 16K read-only "Page Size" will report error. > > In this patch, detect all the read-only fields first. So that, no need > to try over and over again. Frankly, if such HW has been actually designed, the implementor needs to be £$%^&%$"£$% pretty heavily, because this doesn't make much sense, from a HW or SW point of view: Either you support multiple page sizes, or you don't - you don't do it partially. Now, do you know of any HW in the wild that is that crazy? Because I'm not eager to have that kind of approach just because someone *could* implement something silly. From previous discussions with Abel, I gathered that the Huawei ITS is hardwired to 16k, which lead to commit 790b57aed156d. Thanks, M. > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > --- > drivers/irqchip/irq-gic-v3-its.c | 105 +++++++++++++++++++++++---------------- > 1 file changed, 62 insertions(+), 43 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 9687f8a..2577f06 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -800,20 +800,21 @@ static int its_alloc_tables(struct its_node *its) > { > int err; > int i; > - int psz = SZ_64K; > - u64 shr = GITS_BASER_InnerShareable; > - u64 cache = GITS_BASER_WaWb; > + int psz; > + u64 shr; > + u64 cache; > > for (i = 0; i < GITS_BASER_NR_REGS; i++) { > u64 val = readq_relaxed(its->base + GITS_BASER + i * 8); > u64 type = GITS_BASER_TYPE(val); > u64 entry_size = GITS_BASER_ENTRY_SIZE(val); > - int order = get_order(psz); > + int order; > int alloc_size; > u64 tmp; > void *base; > > - if (type == GITS_BASER_TYPE_NONE) > + switch (type) { > + case GITS_BASER_TYPE_NONE: > continue; > > /* > @@ -824,16 +825,65 @@ static int its_alloc_tables(struct its_node *its) > * > * For other tables, only allocate a single page. > */ > - if (type == GITS_BASER_TYPE_DEVICE) { > + case GITS_BASER_TYPE_DEVICE: { > u64 typer = readq_relaxed(its->base + GITS_TYPER); > u32 ids = GITS_TYPER_DEVBITS(typer); > > - order = get_order((1UL << ids) * entry_size); > - if (order >= MAX_ORDER) { > - order = MAX_ORDER - 1; > - pr_warn("%s: Device Table too large, reduce its page order to %u\n", > - its->msi_chip.of_node->full_name, order); > + alloc_size = (1UL << ids) * entry_size; > + break; > } > + > + default: > + alloc_size = PAGE_SIZE; > + break; > + } > + > + psz = PAGE_SIZE; > + shr = GITS_BASER_InnerShareable; > + cache = GITS_BASER_WaWb; > + > + /* Test the fields that maybe read-only */ > + tmp = shr | (~val & GITS_BASER_PAGE_SIZE_MASK); > + writeq_relaxed(tmp, its->base + GITS_BASER + i * 8); > + tmp = readq_relaxed(its->base + GITS_BASER + i * 8); > + > + /* > + * Shareability didn't stick. Just use > + * whatever the read reported, which is likely > + * to be the only thing this redistributor > + * supports. If that's zero, make it > + * non-cacheable as well. > + */ > + shr = tmp & GITS_BASER_SHAREABILITY_MASK; > + if (!shr) > + cache = GITS_BASER_nC; > + > + /* > + * "Page Size" field is read-only. Should ensure the table > + * start address and size align to it. > + */ > + if (!((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK)) > + switch (val & GITS_BASER_PAGE_SIZE_MASK) { > + case GITS_BASER_PAGE_SIZE_4K: > + psz = SZ_4K; > + break; > + > + case GITS_BASER_PAGE_SIZE_16K: > + psz = SZ_16K; > + break; > + > + default: > + psz = SZ_64K; > + break; > + } > + > + alloc_size = ALIGN(alloc_size, psz); > + > + order = get_order(alloc_size); > + if (order >= MAX_ORDER) { > + order = MAX_ORDER - 1; > + pr_warn("%s: Device Table too large, reduce its page order to %u\n", > + its->msi_chip.of_node->full_name, order); > } > > alloc_size = (1 << order) * PAGE_SIZE; > @@ -845,7 +895,6 @@ static int its_alloc_tables(struct its_node *its) > > its->tables[i] = base; > > -retry_baser: > val = (virt_to_phys(base) | > (type << GITS_BASER_TYPE_SHIFT) | > ((entry_size - 1) << GITS_BASER_ENTRY_SIZE_SHIFT) | > @@ -870,40 +919,10 @@ retry_baser: > writeq_relaxed(val, its->base + GITS_BASER + i * 8); > tmp = readq_relaxed(its->base + GITS_BASER + i * 8); > > - if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) { > - /* > - * Shareability didn't stick. Just use > - * whatever the read reported, which is likely > - * to be the only thing this redistributor > - * supports. If that's zero, make it > - * non-cacheable as well. > - */ > - shr = tmp & GITS_BASER_SHAREABILITY_MASK; > - if (!shr) > - cache = GITS_BASER_nC; > - goto retry_baser; > - } > - > - if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) { > - /* > - * Page size didn't stick. Let's try a smaller > - * size and retry. If we reach 4K, then > - * something is horribly wrong... > - */ > - switch (psz) { > - case SZ_16K: > - psz = SZ_4K; > - goto retry_baser; > - case SZ_64K: > - psz = SZ_16K; > - goto retry_baser; > - } > - } > - > if (val != tmp) { > pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n", > its->msi_chip.of_node->full_name, i, > - (unsigned long) val, (unsigned long) tmp); > + (unsigned long)val, (unsigned long)tmp); > err = -ENXIO; > goto out_free; > } > -- > 1.8.0 > > -- Jazz is not dead. It just smells funny. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables 2015-04-07 9:33 ` [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables Marc Zyngier @ 2015-04-07 12:32 ` leizhen 2015-04-07 13:02 ` Marc Zyngier 0 siblings, 1 reply; 7+ messages in thread From: leizhen @ 2015-04-07 12:32 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, Jason Cooper, linux-kernel, Zefan Li, huxinwei, Tianhong Ding, Kefeng Wang, Yun Wu On 2015/4/7 17:33, Marc Zyngier wrote: > On Tue, 7 Apr 2015 08:47:32 +0100 > Zhen Lei <thunder.leizhen@huawei.com> wrote: > >> For the old version, there maybe some faults: >> 1. For each Interrupt Collection table, 4K size is enough. But now the >> default is 64K(if "Page Size" field is not read-only). > > Why is that a problem? psz is initialized to SZ_64K, and it can only be changed in below branch: if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) { /* * Page size didn't stick. Let's try a smaller * size and retry. If we reach 4K, then * something is horribly wrong... */ switch (psz) { case SZ_16K: psz = SZ_4K; goto retry_baser; case SZ_64K: psz = SZ_16K; goto retry_baser; } } The condition enter this branch is that "Page Size" field is read-only. If all GITS_BASERn."Page Size" field are writeable, psz is always SZ_64K. That means, all "Virtual CPUs" and "Interrupt Collections" tables will allocte 64K memory. > >> 2. Suppose two read-only "Page Size" table exist, and software found 4K >> first, then 16K. For the first time, the value of local variable "psz" >> will drop to 4K, but we have not reinitialize it again in the loop. >> So, further process 16K read-only "Page Size" will report error. >> >> In this patch, detect all the read-only fields first. So that, no need >> to try over and over again. > > Frankly, if such HW has been actually designed, the implementor needs > to be £$%^&%$"£$% pretty heavily, because this doesn't make much > sense, from a HW or SW point of view: Either you support > multiple page sizes, or you don't - you don't do it partially. > > Now, do you know of any HW in the wild that is that crazy? Because I'm I really don't know. I just found this by code review. If SW doesn't like HW to be implemented like that, we should comment it clearly. Otherwise, the reader maybe confused, like me. As you see. The code is not hard to write, and not hard to understand. If we can easily tolerate all of the possible, why we not do it? > not eager to have that kind of approach just because someone *could* > implement something silly. From previous discussions with Abel, I > gathered that the Huawei ITS is hardwired to 16k, which lead to commit > 790b57aed156d. > > Thanks, > > M. > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >> --- >> drivers/irqchip/irq-gic-v3-its.c | 105 +++++++++++++++++++++++---------------- >> 1 file changed, 62 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >> index 9687f8a..2577f06 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -800,20 +800,21 @@ static int its_alloc_tables(struct its_node *its) >> { >> int err; >> int i; >> - int psz = SZ_64K; >> - u64 shr = GITS_BASER_InnerShareable; >> - u64 cache = GITS_BASER_WaWb; >> + int psz; >> + u64 shr; >> + u64 cache; >> >> for (i = 0; i < GITS_BASER_NR_REGS; i++) { >> u64 val = readq_relaxed(its->base + GITS_BASER + i * 8); >> u64 type = GITS_BASER_TYPE(val); >> u64 entry_size = GITS_BASER_ENTRY_SIZE(val); >> - int order = get_order(psz); >> + int order; >> int alloc_size; >> u64 tmp; >> void *base; >> >> - if (type == GITS_BASER_TYPE_NONE) >> + switch (type) { >> + case GITS_BASER_TYPE_NONE: >> continue; >> >> /* >> @@ -824,16 +825,65 @@ static int its_alloc_tables(struct its_node *its) >> * >> * For other tables, only allocate a single page. >> */ >> - if (type == GITS_BASER_TYPE_DEVICE) { >> + case GITS_BASER_TYPE_DEVICE: { >> u64 typer = readq_relaxed(its->base + GITS_TYPER); >> u32 ids = GITS_TYPER_DEVBITS(typer); >> >> - order = get_order((1UL << ids) * entry_size); >> - if (order >= MAX_ORDER) { >> - order = MAX_ORDER - 1; >> - pr_warn("%s: Device Table too large, reduce its page order to %u\n", >> - its->msi_chip.of_node->full_name, order); >> + alloc_size = (1UL << ids) * entry_size; >> + break; >> } >> + >> + default: >> + alloc_size = PAGE_SIZE; >> + break; >> + } >> + >> + psz = PAGE_SIZE; >> + shr = GITS_BASER_InnerShareable; >> + cache = GITS_BASER_WaWb; >> + >> + /* Test the fields that maybe read-only */ >> + tmp = shr | (~val & GITS_BASER_PAGE_SIZE_MASK); >> + writeq_relaxed(tmp, its->base + GITS_BASER + i * 8); >> + tmp = readq_relaxed(its->base + GITS_BASER + i * 8); >> + >> + /* >> + * Shareability didn't stick. Just use >> + * whatever the read reported, which is likely >> + * to be the only thing this redistributor >> + * supports. If that's zero, make it >> + * non-cacheable as well. >> + */ >> + shr = tmp & GITS_BASER_SHAREABILITY_MASK; >> + if (!shr) >> + cache = GITS_BASER_nC; >> + >> + /* >> + * "Page Size" field is read-only. Should ensure the table >> + * start address and size align to it. >> + */ >> + if (!((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK)) >> + switch (val & GITS_BASER_PAGE_SIZE_MASK) { >> + case GITS_BASER_PAGE_SIZE_4K: >> + psz = SZ_4K; >> + break; >> + >> + case GITS_BASER_PAGE_SIZE_16K: >> + psz = SZ_16K; >> + break; >> + >> + default: >> + psz = SZ_64K; >> + break; >> + } >> + >> + alloc_size = ALIGN(alloc_size, psz); >> + >> + order = get_order(alloc_size); >> + if (order >= MAX_ORDER) { >> + order = MAX_ORDER - 1; >> + pr_warn("%s: Device Table too large, reduce its page order to %u\n", >> + its->msi_chip.of_node->full_name, order); >> } >> >> alloc_size = (1 << order) * PAGE_SIZE; >> @@ -845,7 +895,6 @@ static int its_alloc_tables(struct its_node *its) >> >> its->tables[i] = base; >> >> -retry_baser: >> val = (virt_to_phys(base) | >> (type << GITS_BASER_TYPE_SHIFT) | >> ((entry_size - 1) << GITS_BASER_ENTRY_SIZE_SHIFT) | >> @@ -870,40 +919,10 @@ retry_baser: >> writeq_relaxed(val, its->base + GITS_BASER + i * 8); >> tmp = readq_relaxed(its->base + GITS_BASER + i * 8); >> >> - if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) { >> - /* >> - * Shareability didn't stick. Just use >> - * whatever the read reported, which is likely >> - * to be the only thing this redistributor >> - * supports. If that's zero, make it >> - * non-cacheable as well. >> - */ >> - shr = tmp & GITS_BASER_SHAREABILITY_MASK; >> - if (!shr) >> - cache = GITS_BASER_nC; >> - goto retry_baser; >> - } >> - >> - if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) { >> - /* >> - * Page size didn't stick. Let's try a smaller >> - * size and retry. If we reach 4K, then >> - * something is horribly wrong... >> - */ >> - switch (psz) { >> - case SZ_16K: >> - psz = SZ_4K; >> - goto retry_baser; >> - case SZ_64K: >> - psz = SZ_16K; >> - goto retry_baser; >> - } >> - } >> - >> if (val != tmp) { >> pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n", >> its->msi_chip.of_node->full_name, i, >> - (unsigned long) val, (unsigned long) tmp); >> + (unsigned long)val, (unsigned long)tmp); >> err = -ENXIO; >> goto out_free; >> } >> -- >> 1.8.0 >> >> > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables 2015-04-07 12:32 ` leizhen @ 2015-04-07 13:02 ` Marc Zyngier 2015-04-07 14:54 ` leizhen 0 siblings, 1 reply; 7+ messages in thread From: Marc Zyngier @ 2015-04-07 13:02 UTC (permalink / raw) To: leizhen Cc: Thomas Gleixner, Jason Cooper, linux-kernel, Zefan Li, huxinwei, Tianhong Ding, Kefeng Wang, Yun Wu On 07/04/15 13:32, leizhen wrote: > On 2015/4/7 17:33, Marc Zyngier wrote: >> On Tue, 7 Apr 2015 08:47:32 +0100 >> Zhen Lei <thunder.leizhen@huawei.com> wrote: >> >>> For the old version, there maybe some faults: >>> 1. For each Interrupt Collection table, 4K size is enough. But now the >>> default is 64K(if "Page Size" field is not read-only). >> >> Why is that a problem? > > psz is initialized to SZ_64K, and it can only be changed in below branch: > if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) { > /* > * Page size didn't stick. Let's try a smaller > * size and retry. If we reach 4K, then > * something is horribly wrong... > */ > switch (psz) { > case SZ_16K: > psz = SZ_4K; > goto retry_baser; > case SZ_64K: > psz = SZ_16K; > goto retry_baser; > } > } > > The condition enter this branch is that "Page Size" field is read-only. If > all GITS_BASERn."Page Size" field are writeable, psz is always SZ_64K. That means, > all "Virtual CPUs" and "Interrupt Collections" tables will allocte 64K memory. At that stage, I don't think it matters (if you want to save memory, have a look at how we allocate the device table...). At boot time, getting 64k is cheap, and it doesn't really shows up on a multi-GB machine. >> >>> 2. Suppose two read-only "Page Size" table exist, and software found 4K >>> first, then 16K. For the first time, the value of local variable "psz" >>> will drop to 4K, but we have not reinitialize it again in the loop. >>> So, further process 16K read-only "Page Size" will report error. >>> >>> In this patch, detect all the read-only fields first. So that, no need >>> to try over and over again. >> >> Frankly, if such HW has been actually designed, the implementor needs >> to be £$%^&%$"£$% pretty heavily, because this doesn't make much >> sense, from a HW or SW point of view: Either you support >> multiple page sizes, or you don't - you don't do it partially. >> >> Now, do you know of any HW in the wild that is that crazy? Because I'm > > I really don't know. I just found this by code review. If SW doesn't like > HW to be implemented like that, we should comment it clearly. Otherwise, > the reader maybe confused, like me. > > As you see. The code is not hard to write, and not hard to understand. If we can > easily tolerate all of the possible, why we not do it? Because carrying code paths that don't get exercised is a maintenance burden and a source of bugs. If you want to add a comment that describes precisely the configurations we support, feel free to summit one. But I don't want to add more complexity to support configurations that don't really make sense from a HW perspective until someone actually (1) build such a configuration and (2) tries to get it supported by mainline. I understand where you're coming from, and I appreciate your effort to make the code more feature-complete. But there is a fine line between between supporting the configurations that actually make sense, and those that nobody would ever produce. So NAK for the "multiple page size" part of the patch. The first point you raise is more interesting, and I wouldn't mind a patch addressing that. Thanks, M. >> not eager to have that kind of approach just because someone *could* >> implement something silly. From previous discussions with Abel, I >> gathered that the Huawei ITS is hardwired to 16k, which lead to commit >> 790b57aed156d. >> >> Thanks, >> >> M. >> >>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >>> --- >>> drivers/irqchip/irq-gic-v3-its.c | 105 +++++++++++++++++++++++---------------- >>> 1 file changed, 62 insertions(+), 43 deletions(-) >>> >>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >>> index 9687f8a..2577f06 100644 >>> --- a/drivers/irqchip/irq-gic-v3-its.c >>> +++ b/drivers/irqchip/irq-gic-v3-its.c >>> @@ -800,20 +800,21 @@ static int its_alloc_tables(struct its_node *its) >>> { >>> int err; >>> int i; >>> - int psz = SZ_64K; >>> - u64 shr = GITS_BASER_InnerShareable; >>> - u64 cache = GITS_BASER_WaWb; >>> + int psz; >>> + u64 shr; >>> + u64 cache; >>> >>> for (i = 0; i < GITS_BASER_NR_REGS; i++) { >>> u64 val = readq_relaxed(its->base + GITS_BASER + i * 8); >>> u64 type = GITS_BASER_TYPE(val); >>> u64 entry_size = GITS_BASER_ENTRY_SIZE(val); >>> - int order = get_order(psz); >>> + int order; >>> int alloc_size; >>> u64 tmp; >>> void *base; >>> >>> - if (type == GITS_BASER_TYPE_NONE) >>> + switch (type) { >>> + case GITS_BASER_TYPE_NONE: >>> continue; >>> >>> /* >>> @@ -824,16 +825,65 @@ static int its_alloc_tables(struct its_node *its) >>> * >>> * For other tables, only allocate a single page. >>> */ >>> - if (type == GITS_BASER_TYPE_DEVICE) { >>> + case GITS_BASER_TYPE_DEVICE: { >>> u64 typer = readq_relaxed(its->base + GITS_TYPER); >>> u32 ids = GITS_TYPER_DEVBITS(typer); >>> >>> - order = get_order((1UL << ids) * entry_size); >>> - if (order >= MAX_ORDER) { >>> - order = MAX_ORDER - 1; >>> - pr_warn("%s: Device Table too large, reduce its page order to %u\n", >>> - its->msi_chip.of_node->full_name, order); >>> + alloc_size = (1UL << ids) * entry_size; >>> + break; >>> } >>> + >>> + default: >>> + alloc_size = PAGE_SIZE; >>> + break; >>> + } >>> + >>> + psz = PAGE_SIZE; >>> + shr = GITS_BASER_InnerShareable; >>> + cache = GITS_BASER_WaWb; >>> + >>> + /* Test the fields that maybe read-only */ >>> + tmp = shr | (~val & GITS_BASER_PAGE_SIZE_MASK); >>> + writeq_relaxed(tmp, its->base + GITS_BASER + i * 8); >>> + tmp = readq_relaxed(its->base + GITS_BASER + i * 8); >>> + >>> + /* >>> + * Shareability didn't stick. Just use >>> + * whatever the read reported, which is likely >>> + * to be the only thing this redistributor >>> + * supports. If that's zero, make it >>> + * non-cacheable as well. >>> + */ >>> + shr = tmp & GITS_BASER_SHAREABILITY_MASK; >>> + if (!shr) >>> + cache = GITS_BASER_nC; >>> + >>> + /* >>> + * "Page Size" field is read-only. Should ensure the table >>> + * start address and size align to it. >>> + */ >>> + if (!((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK)) >>> + switch (val & GITS_BASER_PAGE_SIZE_MASK) { >>> + case GITS_BASER_PAGE_SIZE_4K: >>> + psz = SZ_4K; >>> + break; >>> + >>> + case GITS_BASER_PAGE_SIZE_16K: >>> + psz = SZ_16K; >>> + break; >>> + >>> + default: >>> + psz = SZ_64K; >>> + break; >>> + } >>> + >>> + alloc_size = ALIGN(alloc_size, psz); >>> + >>> + order = get_order(alloc_size); >>> + if (order >= MAX_ORDER) { >>> + order = MAX_ORDER - 1; >>> + pr_warn("%s: Device Table too large, reduce its page order to %u\n", >>> + its->msi_chip.of_node->full_name, order); >>> } >>> >>> alloc_size = (1 << order) * PAGE_SIZE; >>> @@ -845,7 +895,6 @@ static int its_alloc_tables(struct its_node *its) >>> >>> its->tables[i] = base; >>> >>> -retry_baser: >>> val = (virt_to_phys(base) | >>> (type << GITS_BASER_TYPE_SHIFT) | >>> ((entry_size - 1) << GITS_BASER_ENTRY_SIZE_SHIFT) | >>> @@ -870,40 +919,10 @@ retry_baser: >>> writeq_relaxed(val, its->base + GITS_BASER + i * 8); >>> tmp = readq_relaxed(its->base + GITS_BASER + i * 8); >>> >>> - if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) { >>> - /* >>> - * Shareability didn't stick. Just use >>> - * whatever the read reported, which is likely >>> - * to be the only thing this redistributor >>> - * supports. If that's zero, make it >>> - * non-cacheable as well. >>> - */ >>> - shr = tmp & GITS_BASER_SHAREABILITY_MASK; >>> - if (!shr) >>> - cache = GITS_BASER_nC; >>> - goto retry_baser; >>> - } >>> - >>> - if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) { >>> - /* >>> - * Page size didn't stick. Let's try a smaller >>> - * size and retry. If we reach 4K, then >>> - * something is horribly wrong... >>> - */ >>> - switch (psz) { >>> - case SZ_16K: >>> - psz = SZ_4K; >>> - goto retry_baser; >>> - case SZ_64K: >>> - psz = SZ_16K; >>> - goto retry_baser; >>> - } >>> - } >>> - >>> if (val != tmp) { >>> pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n", >>> its->msi_chip.of_node->full_name, i, >>> - (unsigned long) val, (unsigned long) tmp); >>> + (unsigned long)val, (unsigned long)tmp); >>> err = -ENXIO; >>> goto out_free; >>> } >>> -- >>> 1.8.0 >>> >>> >> >> >> > > -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables 2015-04-07 13:02 ` Marc Zyngier @ 2015-04-07 14:54 ` leizhen 0 siblings, 0 replies; 7+ messages in thread From: leizhen @ 2015-04-07 14:54 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, Jason Cooper, linux-kernel, Zefan Li, huxinwei, Tianhong Ding, Kefeng Wang, Yun Wu On 2015/4/7 21:02, Marc Zyngier wrote: > On 07/04/15 13:32, leizhen wrote: >> On 2015/4/7 17:33, Marc Zyngier wrote: >>> On Tue, 7 Apr 2015 08:47:32 +0100 >>> Zhen Lei <thunder.leizhen@huawei.com> wrote: >>> >>>> For the old version, there maybe some faults: >>>> 1. For each Interrupt Collection table, 4K size is enough. But now the >>>> default is 64K(if "Page Size" field is not read-only). >>> >>> Why is that a problem? >> >> psz is initialized to SZ_64K, and it can only be changed in below branch: >> if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) { >> /* >> * Page size didn't stick. Let's try a smaller >> * size and retry. If we reach 4K, then >> * something is horribly wrong... >> */ >> switch (psz) { >> case SZ_16K: >> psz = SZ_4K; >> goto retry_baser; >> case SZ_64K: >> psz = SZ_16K; >> goto retry_baser; >> } >> } >> >> The condition enter this branch is that "Page Size" field is read-only. If >> all GITS_BASERn."Page Size" field are writeable, psz is always SZ_64K. That means, >> all "Virtual CPUs" and "Interrupt Collections" tables will allocte 64K memory. > > At that stage, I don't think it matters (if you want to save memory, > have a look at how we allocate the device table...). At boot time, > getting 64k is cheap, and it doesn't really shows up on a multi-GB machine. No, No. Base on "Page Size" field descripton, each table is at least need 4K(4K,16K,64K: we have no other choice). Er, The maximum entry-size is 32(2^5), 4K page memory can at least support 128(minus one) cores. So, 4K is enough now. Yes, if there too many cores(physical or virtual) exist, we should allocate memory like device table. We simply allocate one page now, becuause we also allocate memory for "Virtual CPUs" table. BTW. Do you need to allocate memory for "Virtual CPUs" table? or leave it to Virtualization? We really don't known how many "Virtual CPUs" will be, it's not controlled by host OS. If we have no need to consider "Virtual CPUs" table, we can allocate memory like device table. And the code will become robust. > >>> >>>> 2. Suppose two read-only "Page Size" table exist, and software found 4K >>>> first, then 16K. For the first time, the value of local variable "psz" >>>> will drop to 4K, but we have not reinitialize it again in the loop. >>>> So, further process 16K read-only "Page Size" will report error. >>>> >>>> In this patch, detect all the read-only fields first. So that, no need >>>> to try over and over again. >>> >>> Frankly, if such HW has been actually designed, the implementor needs >>> to be £$%^&%$"£$% pretty heavily, because this doesn't make much >>> sense, from a HW or SW point of view: Either you support >>> multiple page sizes, or you don't - you don't do it partially. >>> >>> Now, do you know of any HW in the wild that is that crazy? Because I'm >> >> I really don't know. I just found this by code review. If SW doesn't like >> HW to be implemented like that, we should comment it clearly. Otherwise, >> the reader maybe confused, like me. >> >> As you see. The code is not hard to write, and not hard to understand. If we can >> easily tolerate all of the possible, why we not do it? > > Because carrying code paths that don't get exercised is a maintenance > burden and a source of bugs. If you want to add a comment that describes > precisely the configurations we support, feel free to summit one. Oh, I give up add the comment. My English is poor, it's horrible to describe a lot of words. And as you mentioned, it may not make sense. > > But I don't want to add more complexity to support configurations that > don't really make sense from a HW perspective until someone actually (1) > build such a configuration and (2) tries to get it supported by mainline. > > I understand where you're coming from, and I appreciate your effort to > make the code more feature-complete. But there is a fine line between > between supporting the configurations that actually make sense, and > those that nobody would ever produce. > > So NAK for the "multiple page size" part of the patch. The first point OK. > you raise is more interesting, and I wouldn't mind a patch addressing that. > > Thanks, > > M. > >>> not eager to have that kind of approach just because someone *could* >>> implement something silly. From previous discussions with Abel, I >>> gathered that the Huawei ITS is hardwired to 16k, which lead to commit >>> 790b57aed156d. >>> >>> Thanks, >>> >>> M. >>> >>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >>>> --- >>>> drivers/irqchip/irq-gic-v3-its.c | 105 +++++++++++++++++++++++---------------- >>>> 1 file changed, 62 insertions(+), 43 deletions(-) >>>> >>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c >>>> index 9687f8a..2577f06 100644 >>>> --- a/drivers/irqchip/irq-gic-v3-its.c >>>> +++ b/drivers/irqchip/irq-gic-v3-its.c >>>> @@ -800,20 +800,21 @@ static int its_alloc_tables(struct its_node *its) >>>> { >>>> int err; >>>> int i; >>>> - int psz = SZ_64K; >>>> - u64 shr = GITS_BASER_InnerShareable; >>>> - u64 cache = GITS_BASER_WaWb; >>>> + int psz; >>>> + u64 shr; >>>> + u64 cache; >>>> >>>> for (i = 0; i < GITS_BASER_NR_REGS; i++) { >>>> u64 val = readq_relaxed(its->base + GITS_BASER + i * 8); >>>> u64 type = GITS_BASER_TYPE(val); >>>> u64 entry_size = GITS_BASER_ENTRY_SIZE(val); >>>> - int order = get_order(psz); >>>> + int order; >>>> int alloc_size; >>>> u64 tmp; >>>> void *base; >>>> >>>> - if (type == GITS_BASER_TYPE_NONE) >>>> + switch (type) { >>>> + case GITS_BASER_TYPE_NONE: >>>> continue; >>>> >>>> /* >>>> @@ -824,16 +825,65 @@ static int its_alloc_tables(struct its_node *its) >>>> * >>>> * For other tables, only allocate a single page. >>>> */ >>>> - if (type == GITS_BASER_TYPE_DEVICE) { >>>> + case GITS_BASER_TYPE_DEVICE: { >>>> u64 typer = readq_relaxed(its->base + GITS_TYPER); >>>> u32 ids = GITS_TYPER_DEVBITS(typer); >>>> >>>> - order = get_order((1UL << ids) * entry_size); >>>> - if (order >= MAX_ORDER) { >>>> - order = MAX_ORDER - 1; >>>> - pr_warn("%s: Device Table too large, reduce its page order to %u\n", >>>> - its->msi_chip.of_node->full_name, order); >>>> + alloc_size = (1UL << ids) * entry_size; >>>> + break; >>>> } >>>> + >>>> + default: >>>> + alloc_size = PAGE_SIZE; >>>> + break; >>>> + } >>>> + >>>> + psz = PAGE_SIZE; >>>> + shr = GITS_BASER_InnerShareable; >>>> + cache = GITS_BASER_WaWb; >>>> + >>>> + /* Test the fields that maybe read-only */ >>>> + tmp = shr | (~val & GITS_BASER_PAGE_SIZE_MASK); >>>> + writeq_relaxed(tmp, its->base + GITS_BASER + i * 8); >>>> + tmp = readq_relaxed(its->base + GITS_BASER + i * 8); >>>> + >>>> + /* >>>> + * Shareability didn't stick. Just use >>>> + * whatever the read reported, which is likely >>>> + * to be the only thing this redistributor >>>> + * supports. If that's zero, make it >>>> + * non-cacheable as well. >>>> + */ >>>> + shr = tmp & GITS_BASER_SHAREABILITY_MASK; >>>> + if (!shr) >>>> + cache = GITS_BASER_nC; >>>> + >>>> + /* >>>> + * "Page Size" field is read-only. Should ensure the table >>>> + * start address and size align to it. >>>> + */ >>>> + if (!((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK)) >>>> + switch (val & GITS_BASER_PAGE_SIZE_MASK) { >>>> + case GITS_BASER_PAGE_SIZE_4K: >>>> + psz = SZ_4K; >>>> + break; >>>> + >>>> + case GITS_BASER_PAGE_SIZE_16K: >>>> + psz = SZ_16K; >>>> + break; >>>> + >>>> + default: >>>> + psz = SZ_64K; >>>> + break; >>>> + } >>>> + >>>> + alloc_size = ALIGN(alloc_size, psz); >>>> + >>>> + order = get_order(alloc_size); >>>> + if (order >= MAX_ORDER) { >>>> + order = MAX_ORDER - 1; >>>> + pr_warn("%s: Device Table too large, reduce its page order to %u\n", >>>> + its->msi_chip.of_node->full_name, order); >>>> } >>>> >>>> alloc_size = (1 << order) * PAGE_SIZE; >>>> @@ -845,7 +895,6 @@ static int its_alloc_tables(struct its_node *its) >>>> >>>> its->tables[i] = base; >>>> >>>> -retry_baser: >>>> val = (virt_to_phys(base) | >>>> (type << GITS_BASER_TYPE_SHIFT) | >>>> ((entry_size - 1) << GITS_BASER_ENTRY_SIZE_SHIFT) | >>>> @@ -870,40 +919,10 @@ retry_baser: >>>> writeq_relaxed(val, its->base + GITS_BASER + i * 8); >>>> tmp = readq_relaxed(its->base + GITS_BASER + i * 8); >>>> >>>> - if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) { >>>> - /* >>>> - * Shareability didn't stick. Just use >>>> - * whatever the read reported, which is likely >>>> - * to be the only thing this redistributor >>>> - * supports. If that's zero, make it >>>> - * non-cacheable as well. >>>> - */ >>>> - shr = tmp & GITS_BASER_SHAREABILITY_MASK; >>>> - if (!shr) >>>> - cache = GITS_BASER_nC; >>>> - goto retry_baser; >>>> - } >>>> - >>>> - if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) { >>>> - /* >>>> - * Page size didn't stick. Let's try a smaller >>>> - * size and retry. If we reach 4K, then >>>> - * something is horribly wrong... >>>> - */ >>>> - switch (psz) { >>>> - case SZ_16K: >>>> - psz = SZ_4K; >>>> - goto retry_baser; >>>> - case SZ_64K: >>>> - psz = SZ_16K; >>>> - goto retry_baser; >>>> - } >>>> - } >>>> - >>>> if (val != tmp) { >>>> pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n", >>>> its->msi_chip.of_node->full_name, i, >>>> - (unsigned long) val, (unsigned long) tmp); >>>> + (unsigned long)val, (unsigned long)tmp); >>>> err = -ENXIO; >>>> goto out_free; >>>> } >>>> -- >>>> 1.8.0 >>>> >>>> >>> >>> >>> >> >> > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-04-07 14:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-07 7:47 [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables Zhen Lei 2015-04-07 7:47 ` [PATCH 2/3] irqchip/gicv3-its: remove GITS_BASER_TYPE_CPU base on latest spec Zhen Lei 2015-04-07 7:47 ` [PATCH 3/3] irqchip/gicv3-its: treat type reserved as 0x0 Zhen Lei 2015-04-07 9:33 ` [PATCH 1/3] irqchip/gicv3-its: Adjust the implementation of its_alloc_tables Marc Zyngier 2015-04-07 12:32 ` leizhen 2015-04-07 13:02 ` Marc Zyngier 2015-04-07 14:54 ` leizhen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.