* v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX @ 2016-10-21 7:01 Auger Eric 2016-10-21 8:45 ` Marc Zyngier 0 siblings, 1 reply; 19+ messages in thread From: Auger Eric @ 2016-10-21 7:01 UTC (permalink / raw) To: kvm, kvmarm, Marc Zyngier, Christoffer Dall; +Cc: Andrew Jones, daniel.thompson Hi, I am not able to boot 4.9-rc1 as a guest on Cavium ThunderX (dt and acpi mode). Bisecting the guest shows that the problem shows up at 91ef84428a86b75a52e15c6fe4f56b446ba75f93 irqchip/gic-v3: Reset BPR during initialization If I remove the write to the ICC_BPR1_EL1 register on guest, the VM boots. Investigating KVM code ... Thanks Eric ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 7:01 v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX Auger Eric @ 2016-10-21 8:45 ` Marc Zyngier 2016-10-21 9:05 ` Auger Eric 2016-10-21 10:40 ` Daniel Thompson 0 siblings, 2 replies; 19+ messages in thread From: Marc Zyngier @ 2016-10-21 8:45 UTC (permalink / raw) To: Auger Eric, kvm, kvmarm, Christoffer Dall; +Cc: Robert Richter, daniel.thompson +Robert On 21/10/16 08:01, Auger Eric wrote: > Hi, > > I am not able to boot 4.9-rc1 as a guest on Cavium ThunderX (dt and acpi > mode). Bisecting the guest shows that the problem shows up at > > 91ef84428a86b75a52e15c6fe4f56b446ba75f93 > irqchip/gic-v3: Reset BPR during initialization > > If I remove the write to the ICC_BPR1_EL1 register on guest, the VM boots. That's very odd. A ICC_BPR1_EL1 access when HCR_EL2.IMO is set only affects ICH_VMCR_EL2.VBPR1. It is not trapped, since we don't set ICH_HCR_EL2.TALL1. It is a very boring sysreg! So from a pure architectural point of view, I don't see how this can fail. I've just run the same configuration on my Freescale board (GICv3 as well), and can't see any issue at all. > Investigating KVM code ... What is the failure syndrome? Do you see it crashing? Locking up? What is the PC at that stage? Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 8:45 ` Marc Zyngier @ 2016-10-21 9:05 ` Auger Eric 2016-10-21 9:40 ` Marc Zyngier 2016-11-03 18:01 ` Robert Richter 2016-10-21 10:40 ` Daniel Thompson 1 sibling, 2 replies; 19+ messages in thread From: Auger Eric @ 2016-10-21 9:05 UTC (permalink / raw) To: Marc Zyngier, kvm, kvmarm, Christoffer Dall Cc: Andrew Jones, daniel.thompson, Robert Richter Hi Marc, On 21/10/2016 10:45, Marc Zyngier wrote: > +Robert > > On 21/10/16 08:01, Auger Eric wrote: >> Hi, >> >> I am not able to boot 4.9-rc1 as a guest on Cavium ThunderX (dt and acpi >> mode). Bisecting the guest shows that the problem shows up at >> >> 91ef84428a86b75a52e15c6fe4f56b446ba75f93 >> irqchip/gic-v3: Reset BPR during initialization >> >> If I remove the write to the ICC_BPR1_EL1 register on guest, the VM boots. > > That's very odd. A ICC_BPR1_EL1 access when HCR_EL2.IMO is set only > affects ICH_VMCR_EL2.VBPR1. It is not trapped, since we don't set > ICH_HCR_EL2.TALL1. It is a very boring sysreg! > > So from a pure architectural point of view, I don't see how this can > fail. I've just run the same configuration on my Freescale board (GICv3 > as well), and can't see any issue at all. > >> Investigating KVM code ... > > What is the failure syndrome? Do you see it crashing? Locking up? What > is the PC at that stage? No guest crash. the guest just locks up. No traces output. Thanks Eric > > Thanks, > > M. > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 9:05 ` Auger Eric @ 2016-10-21 9:40 ` Marc Zyngier 2016-10-21 9:46 ` Auger Eric 2016-11-03 18:01 ` Robert Richter 1 sibling, 1 reply; 19+ messages in thread From: Marc Zyngier @ 2016-10-21 9:40 UTC (permalink / raw) To: Auger Eric, kvm, kvmarm, Christoffer Dall Cc: Andrew Jones, daniel.thompson, Robert Richter On 21/10/16 10:05, Auger Eric wrote: > Hi Marc, > > On 21/10/2016 10:45, Marc Zyngier wrote: >> +Robert >> >> On 21/10/16 08:01, Auger Eric wrote: >>> Hi, >>> >>> I am not able to boot 4.9-rc1 as a guest on Cavium ThunderX (dt and acpi >>> mode). Bisecting the guest shows that the problem shows up at >>> >>> 91ef84428a86b75a52e15c6fe4f56b446ba75f93 >>> irqchip/gic-v3: Reset BPR during initialization >>> >>> If I remove the write to the ICC_BPR1_EL1 register on guest, the VM boots. >> >> That's very odd. A ICC_BPR1_EL1 access when HCR_EL2.IMO is set only >> affects ICH_VMCR_EL2.VBPR1. It is not trapped, since we don't set >> ICH_HCR_EL2.TALL1. It is a very boring sysreg! >> >> So from a pure architectural point of view, I don't see how this can >> fail. I've just run the same configuration on my Freescale board (GICv3 >> as well), and can't see any issue at all. >> >>> Investigating KVM code ... >> >> What is the failure syndrome? Do you see it crashing? Locking up? What >> is the PC at that stage? > No guest crash. the guest just locks up. No traces output. But you're able to kill the guest, right, and the CPU is not going to lalaland. We should be able to put a breakpoint on this instruction using qemu + GDB, and step it to find out what's happening. Or even execute the instruction in isolation with a bunch of printks in the guest. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 9:40 ` Marc Zyngier @ 2016-10-21 9:46 ` Auger Eric 2016-10-21 11:49 ` Andrew Jones 0 siblings, 1 reply; 19+ messages in thread From: Auger Eric @ 2016-10-21 9:46 UTC (permalink / raw) To: Marc Zyngier, kvm, kvmarm, Christoffer Dall Cc: Robert Richter, daniel.thompson Hi Marc, On 21/10/2016 11:40, Marc Zyngier wrote: > On 21/10/16 10:05, Auger Eric wrote: >> Hi Marc, >> >> On 21/10/2016 10:45, Marc Zyngier wrote: >>> +Robert >>> >>> On 21/10/16 08:01, Auger Eric wrote: >>>> Hi, >>>> >>>> I am not able to boot 4.9-rc1 as a guest on Cavium ThunderX (dt and acpi >>>> mode). Bisecting the guest shows that the problem shows up at >>>> >>>> 91ef84428a86b75a52e15c6fe4f56b446ba75f93 >>>> irqchip/gic-v3: Reset BPR during initialization >>>> >>>> If I remove the write to the ICC_BPR1_EL1 register on guest, the VM boots. >>> >>> That's very odd. A ICC_BPR1_EL1 access when HCR_EL2.IMO is set only >>> affects ICH_VMCR_EL2.VBPR1. It is not trapped, since we don't set >>> ICH_HCR_EL2.TALL1. It is a very boring sysreg! >>> >>> So from a pure architectural point of view, I don't see how this can >>> fail. I've just run the same configuration on my Freescale board (GICv3 >>> as well), and can't see any issue at all. >>> >>>> Investigating KVM code ... >>> >>> What is the failure syndrome? Do you see it crashing? Locking up? What >>> is the PC at that stage? >> No guest crash. the guest just locks up. No traces output. > > But you're able to kill the guest, right, Yes I am and the CPU is not going to > lalaland. We should be able to put a breakpoint on this instruction > using qemu + GDB, and step it to find out what's happening. Or even > execute the instruction in isolation with a bunch of printks in the guest. Yep I will investigate this afternoon. Thanks Eric > > M. > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 9:46 ` Auger Eric @ 2016-10-21 11:49 ` Andrew Jones 2016-10-21 11:57 ` Peter Maydell 0 siblings, 1 reply; 19+ messages in thread From: Andrew Jones @ 2016-10-21 11:49 UTC (permalink / raw) To: Auger Eric Cc: Marc Zyngier, kvm, kvmarm, Christoffer Dall, daniel.thompson, Robert Richter [-- Attachment #1: Type: text/plain, Size: 2570 bytes --] On Fri, Oct 21, 2016 at 11:46:04AM +0200, Auger Eric wrote: > Hi Marc, > On 21/10/2016 11:40, Marc Zyngier wrote: > > On 21/10/16 10:05, Auger Eric wrote: > >> Hi Marc, > >> > >> On 21/10/2016 10:45, Marc Zyngier wrote: > >>> +Robert > >>> > >>> On 21/10/16 08:01, Auger Eric wrote: > >>>> Hi, > >>>> > >>>> I am not able to boot 4.9-rc1 as a guest on Cavium ThunderX (dt and acpi > >>>> mode). Bisecting the guest shows that the problem shows up at > >>>> > >>>> 91ef84428a86b75a52e15c6fe4f56b446ba75f93 > >>>> irqchip/gic-v3: Reset BPR during initialization > >>>> > >>>> If I remove the write to the ICC_BPR1_EL1 register on guest, the VM boots. > >>> > >>> That's very odd. A ICC_BPR1_EL1 access when HCR_EL2.IMO is set only > >>> affects ICH_VMCR_EL2.VBPR1. It is not trapped, since we don't set > >>> ICH_HCR_EL2.TALL1. It is a very boring sysreg! > >>> > >>> So from a pure architectural point of view, I don't see how this can > >>> fail. I've just run the same configuration on my Freescale board (GICv3 > >>> as well), and can't see any issue at all. > >>> > >>>> Investigating KVM code ... > >>> > >>> What is the failure syndrome? Do you see it crashing? Locking up? What > >>> is the PC at that stage? > >> No guest crash. the guest just locks up. No traces output. > > > > But you're able to kill the guest, right, > Yes I am > and the CPU is not going to > > lalaland. We should be able to put a breakpoint on this instruction > > using qemu + GDB, and step it to find out what's happening. Or even > > execute the instruction in isolation with a bunch of printks in the guest. > Yep I will investigate this afternoon. > You might be able to debug faster with kvm-unit-tests. The attached patch applies to the arm/gic branch of my repo, https://github.com/rhdrjones/kvm-unit-tests/commits/arm/gic and reproduces the issue. Before applying the attached patch, running $ arm/run arm/gic.flat passes. PASS: gicv3: ipi: self: Completed in 100 ms After applying the patch it times-out FAIL: gicv3: ipi: self: Timed-out (5s). ACKS: missing=1 extra=0 unexpected=0 Using the monitor and stopping/starting the vcpu to see what it's doing I confirmed that we're just spinning in udelay waiting for the interrupt. So it appears setting this register to zero disables the vcpu's ability to receive interrupts? 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. Thanks, drew [-- Attachment #2: 0001-arm64-gic-write-bpr1.patch --] [-- Type: text/plain, Size: 1401 bytes --] >From cd8989fa11255d0bf21d46051a65c3b073a182e6 Mon Sep 17 00:00:00 2001 From: Andrew Jones <drjones@redhat.com> Date: Fri, 21 Oct 2016 13:22:38 +0200 Subject: [kvm-unit-tests PATCH] arm64: gic: write bpr1 --- lib/arm/gic.c | 1 + lib/arm64/asm/arch_gicv3.h | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/lib/arm/gic.c b/lib/arm/gic.c index bb62407f7286..c44f7614be4d 100644 --- a/lib/arm/gic.c +++ b/lib/arm/gic.c @@ -137,6 +137,7 @@ void gicv3_enable_defaults(void) gicv3_redist_wait_for_rwp(); gicv3_write_pmr(0xf0); + gicv3_write_bpr1(0); gicv3_write_ctlr(ICC_CTLR_EL1_EOImode_drop_dir); gicv3_write_grpen1(1); } diff --git a/lib/arm64/asm/arch_gicv3.h b/lib/arm64/asm/arch_gicv3.h index eff2efdfe2d4..cd9d8c95ef0d 100644 --- a/lib/arm64/asm/arch_gicv3.h +++ b/lib/arm64/asm/arch_gicv3.h @@ -18,6 +18,7 @@ #define ICC_CTLR_EL1 sys_reg(3, 0, 12, 12, 4) #define ICC_SRE_EL1 sys_reg(3, 0, 12, 12, 5) #define ICC_GRPEN1_EL1 sys_reg(3, 0, 12, 12, 7) +#define ICC_BPR1_EL1 sys_reg(3, 0, 12, 12, 3) #define ICC_SRE_EL2 sys_reg(3, 4, 12, 9, 5) @@ -162,6 +163,11 @@ static inline void gicv3_write_sre(u32 val) isb(); } +static inline void gicv3_write_bpr1(u32 val) +{ + asm volatile("msr_s " __stringify(ICC_BPR1_EL1) ", %0" : : "r" (val)); +} + #define gicv3_read_typer(c) readq(c) #define gicv3_write_irouter(v, c) writeq(v, c) -- 2.7.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 11:49 ` Andrew Jones @ 2016-10-21 11:57 ` Peter Maydell 2016-10-21 12:07 ` Andrew Jones 0 siblings, 1 reply; 19+ messages in thread From: Peter Maydell @ 2016-10-21 11:57 UTC (permalink / raw) To: Andrew Jones Cc: Auger Eric, Daniel Thompson, kvm, Marc Zyngier, Robert Richter, kvmarm On 21 October 2016 at 12:49, Andrew Jones <drjones@redhat.com> 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... thanks -- PMM ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 11:57 ` Peter Maydell @ 2016-10-21 12:07 ` Andrew Jones 2016-10-21 12:52 ` Marc Zyngier 0 siblings, 1 reply; 19+ messages in thread From: Andrew Jones @ 2016-10-21 12:07 UTC (permalink / raw) To: Peter Maydell Cc: Auger Eric, Daniel Thompson, kvm, Marc Zyngier, Robert Richter, kvmarm On Fri, Oct 21, 2016 at 12:57:37PM +0100, Peter Maydell wrote: > On 21 October 2016 at 12:49, Andrew Jones <drjones@redhat.com> 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 Thanks, drew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 12:07 ` Andrew Jones @ 2016-10-21 12:52 ` Marc Zyngier 2016-10-21 12:58 ` Peter Maydell ` (3 more replies) 0 siblings, 4 replies; 19+ messages in thread From: Marc Zyngier @ 2016-10-21 12:52 UTC (permalink / raw) To: Andrew Jones, Peter Maydell, Auger Eric Cc: Daniel Thompson, Robert Richter, kvmarm, kvm 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@redhat.com> 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... ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 12:52 ` Marc Zyngier @ 2016-10-21 12:58 ` Peter Maydell 2016-10-21 13:20 ` Marc Zyngier 2016-10-21 13:06 ` Andrew Jones ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Peter Maydell @ 2016-10-21 12:58 UTC (permalink / raw) To: Marc Zyngier; +Cc: kvm, Daniel Thompson, Robert Richter, kvmarm On 21 October 2016 at 13:52, Marc Zyngier <marc.zyngier@arm.com> wrote: > 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); ...a write function that calls read_sysreg() and returns a value? thanks -- PMM ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 12:58 ` Peter Maydell @ 2016-10-21 13:20 ` Marc Zyngier 0 siblings, 0 replies; 19+ messages in thread From: Marc Zyngier @ 2016-10-21 13:20 UTC (permalink / raw) To: Peter Maydell; +Cc: kvm, Daniel Thompson, Robert Richter, kvmarm On Fri, 21 Oct 2016 13:58:47 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On 21 October 2016 at 13:52, Marc Zyngier <marc.zyngier@arm.com> wrote: > > 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); > > > ...a write function that calls read_sysreg() and returns a value? As you can tell, I haven't even compiled the 32bit version... :-/ M. -- Jazz is not dead. It just smells funny. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 12:52 ` Marc Zyngier 2016-10-21 12:58 ` Peter Maydell @ 2016-10-21 13:06 ` Andrew Jones 2016-10-21 13:47 ` Marc Zyngier 2016-10-21 16:34 ` Robert Richter 2016-10-26 7:18 ` Robert Richter 3 siblings, 1 reply; 19+ messages in thread From: Andrew Jones @ 2016-10-21 13:06 UTC (permalink / raw) To: Marc Zyngier Cc: Peter Maydell, Auger Eric, Daniel Thompson, kvm, Robert Richter, kvmarm On Fri, Oct 21, 2016 at 01:52:43PM +0100, Marc Zyngier wrote: > 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@redhat.com> 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) */ > FWIW, the equivalent implementation didn't help kvm-unit-tests. Still timing-out. Even after attempting to set it back to old, which was 3, when I read it again I still get zero, pre-read bpr1=3 post-read bpr1=0 new = 0, old = 3 resetting to old and doing post-post-read... post-post-read bpr1=0 FAIL: gicv3: ipi: self: Timed-out (5s). ACKS: missing=1 extra=0 unexpected=0 drew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 13:06 ` Andrew Jones @ 2016-10-21 13:47 ` Marc Zyngier 0 siblings, 0 replies; 19+ messages in thread From: Marc Zyngier @ 2016-10-21 13:47 UTC (permalink / raw) To: Andrew Jones Cc: Peter Maydell, Auger Eric, Daniel Thompson, kvm, Robert Richter, kvmarm On Fri, 21 Oct 2016 15:06:39 +0200 Andrew Jones <drjones@redhat.com> wrote: > On Fri, Oct 21, 2016 at 01:52:43PM +0100, Marc Zyngier wrote: > > 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@redhat.com> 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) */ > > > > FWIW, the equivalent implementation didn't help kvm-unit-tests. Still > timing-out. Even after attempting to set it back to old, which was 3, > when I read it again I still get zero, > > pre-read bpr1=3 > post-read bpr1=0 > new = 0, old = 3 > resetting to old and doing post-post-read... > post-post-read bpr1=0 > FAIL: gicv3: ipi: self: Timed-out (5s). ACKS: missing=1 extra=0 unexpected=0 fsck... Let's try something else. Obviously, Hardcoded values are bad. Untested. M. diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c index 3947095cc0a1..0d5596f5876c 100644 --- a/virt/kvm/arm/hyp/vgic-v3-sr.c +++ b/virt/kvm/arm/hyp/vgic-v3-sr.c @@ -160,6 +160,8 @@ void __hyp_text __vgic_v3_save_state(struct kvm_vcpu *vcpu) dsb(st); cpu_if->vgic_vmcr = read_gicreg(ICH_VMCR_EL2); + if (unlikely(!((cpu_if->vgic_vmcr >> 18) & 7))) + cpu_if->vgic_vmcr |= 3 << 18; if (vcpu->arch.vgic_cpu.live_lrs) { int i; -- Jazz is not dead. It just smells funny. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 12:52 ` Marc Zyngier 2016-10-21 12:58 ` Peter Maydell 2016-10-21 13:06 ` Andrew Jones @ 2016-10-21 16:34 ` Robert Richter 2016-10-26 7:18 ` Robert Richter 3 siblings, 0 replies; 19+ messages in thread From: Robert Richter @ 2016-10-21 16:34 UTC (permalink / raw) To: Marc Zyngier; +Cc: kvm, Daniel Thompson, kvmarm On 21.10.16 13:52:43, Marc Zyngier wrote: > 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@redhat.com> 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... Marc, we are looking into this. -Robert ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 12:52 ` Marc Zyngier ` (2 preceding siblings ...) 2016-10-21 16:34 ` Robert Richter @ 2016-10-26 7:18 ` Robert Richter 2016-10-26 7:39 ` Marc Zyngier 3 siblings, 1 reply; 19+ messages in thread From: Robert Richter @ 2016-10-26 7:18 UTC (permalink / raw) To: Marc Zyngier; +Cc: kvm, Daniel Thompson, kvmarm On 21.10.16 13:52:43, Marc Zyngier wrote: > 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@redhat.com> 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... "The minimum binary value programmable is BP_MIN. Attempt to set it to a lower value automatically sets it to the minimum value BP_MIN." The min value is 3 in this level, thus it should return 3 for [2:0] of BPR1. I am going to reproduce this. -Robert ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-26 7:18 ` Robert Richter @ 2016-10-26 7:39 ` Marc Zyngier 0 siblings, 0 replies; 19+ messages in thread From: Marc Zyngier @ 2016-10-26 7:39 UTC (permalink / raw) To: Robert Richter; +Cc: kvm, Daniel Thompson, kvmarm On 26/10/16 08:18, Robert Richter wrote: > On 21.10.16 13:52:43, Marc Zyngier wrote: >> 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@redhat.com> 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... > > "The minimum binary value programmable is BP_MIN. Attempt to > set it to a lower value automatically sets it to the minimum value > BP_MIN." > > The min value is 3 in this level, thus it should return 3 for [2:0] of > BPR1. Exactly (though there is a number of other rules such as taking ICC_CTLR_EL1.CBPR into account). And all this should reflect in VMCR_EL2.VBPR1. > I am going to reproduce this. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 9:05 ` Auger Eric 2016-10-21 9:40 ` Marc Zyngier @ 2016-11-03 18:01 ` Robert Richter 2016-11-04 13:30 ` Auger Eric 1 sibling, 1 reply; 19+ messages in thread From: Robert Richter @ 2016-11-03 18:01 UTC (permalink / raw) To: Auger Eric Cc: Marc Zyngier, kvm, kvmarm, Christoffer Dall, Andrew Jones, daniel.thompson, Robert Richter Eric, On 21.10.16 11:05:01, Auger Eric wrote: > On 21/10/2016 10:45, Marc Zyngier wrote: > > What is the failure syndrome? Do you see it crashing? Locking up? What > > is the PC at that stage? > No guest crash. the guest just locks up. No traces output. I am trying to reproduce this but could boot a 4.9-rc3 kernel as guest. Are there any special settings? Here my command line: qemu-system-aarch64 -name qemu.test -enable-kvm -smp 8 -m 16G -cpu host -nographic -kernel /boot/vmlinuz-4.9.0-rc3.0.vanilla10-00002-gcfd8c2cee6ac -initrd /boot/initrd.img-4.9.0-rc3.0.vanilla10-00002-gcfd8c2cee6ac -M virt,gic_version=3 -hdb /dev/sdb -append "root=/dev/vda3" Thanks, -Robert ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-11-03 18:01 ` Robert Richter @ 2016-11-04 13:30 ` Auger Eric 0 siblings, 0 replies; 19+ messages in thread From: Auger Eric @ 2016-11-04 13:30 UTC (permalink / raw) To: Robert Richter Cc: Marc Zyngier, kvm, kvmarm, Christoffer Dall, Andrew Jones, daniel.thompson, Robert Richter Hi Robert, On 03/11/2016 19:01, Robert Richter wrote: > Eric, > > On 21.10.16 11:05:01, Auger Eric wrote: >> On 21/10/2016 10:45, Marc Zyngier wrote: > >>> What is the failure syndrome? Do you see it crashing? Locking up? What >>> is the PC at that stage? >> No guest crash. the guest just locks up. No traces output. > > I am trying to reproduce this but could boot a 4.9-rc3 kernel as > guest. Are there any special settings? Here my command line: > > qemu-system-aarch64 -name qemu.test -enable-kvm -smp 8 -m 16G -cpu host -nographic -kernel /boot/vmlinuz-4.9.0-rc3.0.vanilla10-00002-gcfd8c2cee6ac -initrd /boot/initrd.img-4.9.0-rc3.0.vanilla10-00002-gcfd8c2cee6ac -M virt,gic_version=3 -hdb /dev/sdb -append "root=/dev/vda3" > > Thanks, no, nothing special. I checked again with a 4.9-rc3 host and guest and the guest fails booting except if I comment the gic_write_bpr1(0) call. Thanks Eric > > -Robert > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX 2016-10-21 8:45 ` Marc Zyngier 2016-10-21 9:05 ` Auger Eric @ 2016-10-21 10:40 ` Daniel Thompson 1 sibling, 0 replies; 19+ messages in thread From: Daniel Thompson @ 2016-10-21 10:40 UTC (permalink / raw) To: Marc Zyngier, Auger Eric, kvm, kvmarm, Christoffer Dall Cc: Andrew Jones, Robert Richter On 21/10/16 09:45, Marc Zyngier wrote: > +Robert > > On 21/10/16 08:01, Auger Eric wrote: >> Hi, >> >> I am not able to boot 4.9-rc1 as a guest on Cavium ThunderX (dt and acpi >> mode). Bisecting the guest shows that the problem shows up at >> >> 91ef84428a86b75a52e15c6fe4f56b446ba75f93 >> irqchip/gic-v3: Reset BPR during initialization >> >> If I remove the write to the ICC_BPR1_EL1 register on guest, the VM boots. > > That's very odd. A ICC_BPR1_EL1 access when HCR_EL2.IMO is set only > affects ICH_VMCR_EL2.VBPR1. It is not trapped, since we don't set > ICH_HCR_EL2.TALL1. It is a very boring sysreg! > > So from a pure architectural point of view, I don't see how this can > fail. I've just run the same configuration on my Freescale board (GICv3 > as well), and can't see any issue at all. Wow. Of all the patches I written for arm64 this was not the one I regarded as especially risky [at least when compared to the others ;-)]. Of course it does rely on writing a zero being "an attempt to program the binary point field to a value less than the reset value" and "sets the field to the reset value." and from a silicon validation point of view that might be regarded as a interesting corner case... Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-11-04 13:30 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-21 7:01 v4.9-rc1 fails booting as a guest on ARM64 Cavium ThunderX Auger Eric 2016-10-21 8:45 ` Marc Zyngier 2016-10-21 9:05 ` Auger Eric 2016-10-21 9:40 ` Marc Zyngier 2016-10-21 9:46 ` Auger Eric 2016-10-21 11:49 ` Andrew Jones 2016-10-21 11:57 ` Peter Maydell 2016-10-21 12:07 ` Andrew Jones 2016-10-21 12:52 ` Marc Zyngier 2016-10-21 12:58 ` Peter Maydell 2016-10-21 13:20 ` Marc Zyngier 2016-10-21 13:06 ` Andrew Jones 2016-10-21 13:47 ` Marc Zyngier 2016-10-21 16:34 ` Robert Richter 2016-10-26 7:18 ` Robert Richter 2016-10-26 7:39 ` Marc Zyngier 2016-11-03 18:01 ` Robert Richter 2016-11-04 13:30 ` Auger Eric 2016-10-21 10:40 ` Daniel Thompson
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.