On 21/10/16 13:07, Andrew Jones wrote:
> On Fri, Oct 21, 2016 at 12:57:37PM +0100, Peter Maydell wrote:
>> On 21 October 2016 at 12:49, Andrew Jones <drjones@xxxxxxxxxx> wrote:
>>> I also read the register before writing it and saw it was 3. I tried
>>> writing 3 instead of 0 to see what would happen, but the failure
>>> persisted. I did read back the register after writing it to confirm the
>>> change took affect.
>>
>> So what does it read back as after you write 0? The GICv3 spec
>> says it can't read back as zero...
>>
>
> I read back zero
>
> pre-read bpr1=3
> post-read bpr1=0
> FAIL: gicv3: ipi: self: Timed-out (5s). ACKS: missing=1 extra=0 unexpected=0
Gah... I guess we'll have to either roll a dice, or get someone from
Cavium to tell us what's happening here. In the meantime, can you give
the following patch a go? It doesn't fire on my FSL box, but everything
hunky dory on it so far...
Thanks,
M.
diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h
index a808829..5c03171 100644
--- a/arch/arm/include/asm/arch_gicv3.h
+++ b/arch/arm/include/asm/arch_gicv3.h
@@ -222,6 +222,11 @@ static inline void gic_write_bpr1(u32 val)
write_sysreg(val, ICC_BPR1);
}
+static inline u32 gic_write_bpr1(void)
+{
+ return read_sysreg(ICC_BPR1);
+}
+
/*
* Even in 32bit systems that use LPAE, there is no guarantee that the I/O
* interface provides true 64bit atomic accesses, so using strd/ldrd doesn't
diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h
index f8ae6d6..74fe2c9 100644
--- a/arch/arm64/include/asm/arch_gicv3.h
+++ b/arch/arm64/include/asm/arch_gicv3.h
@@ -184,6 +184,13 @@ static inline void gic_write_bpr1(u32 val)
asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val));
}
+static inline u32 gic_read_bpr1(void)
+{
+ u64 val;
+ asm volatile("mrs_s %0, " __stringify(ICC_BPR1_EL1) : "=r" (val));
+ return val;
+}
+
#define gic_read_typer(c) readq_relaxed(c)
#define gic_write_irouter(v, c) writeq_relaxed(v, c)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 9b81bd8..db90286 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -482,6 +482,8 @@ static int gic_populate_rdist(void)
static void gic_cpu_sys_reg_init(void)
{
+ u32 bpr1_old, bpr1_new;
+
/*
* Need to check that the SRE bit has actually been set. If
* not, it means that SRE is disabled at EL2. We're going to
@@ -499,9 +501,19 @@ static void gic_cpu_sys_reg_init(void)
* Some firmwares hand over to the kernel with the BPR changed from
* its reset value (and with a value large enough to prevent
* any pre-emptive interrupts from working at all). Writing a zero
- * to BPR restores is reset value.
+ * to BPR restores is reset value (though there seems to be some
+ * less than compliant implementations around, hence the warning...).
*/
+ bpr1_old = gic_read_bpr1();
gic_write_bpr1(0);
+ isb();
+ bpr1_new = gic_read_bpr1();
+
+ if (bpr1_new == 0) {
+ pr_warn("Failed to reset BPR1 (%d), restoring previous (%d)\n",
+ bpr1_new, bpr1_old);
+ gic_write_bpr1(bpr1_old);
+ }
if (static_key_true(&supports_deactivate)) {
/* EOI drops priority only (mode 1) */
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
