Re: [PATCH][next] arm64: KVM: GICv3: move system register access to msr_s/mrs_s
On Thu, Jul 31, 2014 at 05:05:58PM +0100, Will Deacon wrote: > On Thu, Jul 31, 2014 at 02:32:27PM +0100, Christoffer Dall wrote: > > On Thu, Jul 31, 2014 at 02:19:47PM +0100, Will Deacon wrote: > > > On Thu, Jul 31, 2014 at 02:16:39PM +0100, Marc Zyngier wrote: > > > > Commit 72c583951526 (arm64: gicv3: Allow GICv3 compilation with > > > > older binutils) changed the way we express the GICv3 system registers, > > > > but couldn't change the occurences used by KVM as the code wasn't > > > > merged yet. > > > > > > > > Just fix the accessors. > > > > > > > > Cc: Will Deacon > > > > Cc: Catalin Marinas > > > > Cc: Christoffer Dall > > > > Signed-off-by: Marc Zyngier > > > > --- > > > > -next is currently borked. I suggest we take this patch via the kvmarm > > > > tree, > > > > and only send our PR once the arm64 tree has hit Linus' tree. It > > > > contains > > > > the same dependency on the GIC tree anyway. > > > > > > I'm fine with that as long as Paulo doesn't mind waiting for the arm64 > > > stuff > > > to go in first (I have no reason to delay my pull request, so shouldn't be > > > an issue). > > > > > > If it helps: > > > > > > Acked-by: Will Deacon > > > > > > I can't help but point out that we wouldn't be in this mess if you'd got > > > your stuff into -next sooner ;) > > > > > > > Well yes, but it was hard to plan our holidays around the deadlines and > > unfortunately I couldn't find time to verify the kvmarm branch to make > > it ready for -next inclusion before I went on holiday this time. In any > > case, this should be the exception rather than the rule, and I do try to > > get the next branch ready as soon as possible usually. > > Understood, but the communication could have been a lot better. > > Whilst you were away, -next broke due to the non-virt parts of the GICv3 > driver (assumedly going via the irqchip tree?): > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/275410.html > > Catalin fixed that up, and then Jason stated that the intention was for > GICv3 to go via the arm64 tree (see above link). Based on that, we pulled > his tag into our tree and applied our fix on top. > > Now a bunch of stuff has cropped up out of nowhere causing conflicts and > breakages for everybody ~3 days before the merge window opens (the code > defaults to 'y'). We're not mind-readers, so all it would have taken is > a mail to the relevant maintainers before everybody disappeared on holiday > saying (a) what the merge plan is and (b) what to do if everything goes > wrong while you're away. > > Bah, maybe I'm just being grumpy... > Besides from postponing my vacation I'm not really sure what I could have done differently. In this thread (on which you were cc'ed) I really tried to make people aware of what was coming: https://lists.cs.columbia.edu/pipermail/kvmarm/2014-July/010482.html nobody replied to my last mail about something potentially messing stuff up in the queue branch. Maybe my fault was my reluctance to put patches that hadn't been able to verify as working on a v8 platform into a branch that was pulled into -next. If I was being overly cautious in that sense, that's a fair point and I can adjust future behavior for that. Otherwise I think this was just an unfortunate series of events with a lot of new stuff coming in combined with finding some nasty bugs. Hey, it happens. Also, it's not like anyone sent me an e-mail saying URGENT, fix your sh*t, in which case I would have dealt with it. Seriously, if I'm being stupid, please tell me which e-mail I should have sent to whom or what to have done differnently. Thanks, -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][next] arm64: KVM: GICv3: move system register access to msr_s/mrs_s
On Thu, Jul 31, 2014 at 02:32:27PM +0100, Christoffer Dall wrote: > On Thu, Jul 31, 2014 at 02:19:47PM +0100, Will Deacon wrote: > > On Thu, Jul 31, 2014 at 02:16:39PM +0100, Marc Zyngier wrote: > > > Commit 72c583951526 (arm64: gicv3: Allow GICv3 compilation with > > > older binutils) changed the way we express the GICv3 system registers, > > > but couldn't change the occurences used by KVM as the code wasn't > > > merged yet. > > > > > > Just fix the accessors. > > > > > > Cc: Will Deacon > > > Cc: Catalin Marinas > > > Cc: Christoffer Dall > > > Signed-off-by: Marc Zyngier > > > --- > > > -next is currently borked. I suggest we take this patch via the kvmarm > > > tree, > > > and only send our PR once the arm64 tree has hit Linus' tree. It contains > > > the same dependency on the GIC tree anyway. > > > > I'm fine with that as long as Paulo doesn't mind waiting for the arm64 stuff > > to go in first (I have no reason to delay my pull request, so shouldn't be > > an issue). > > > > If it helps: > > > > Acked-by: Will Deacon > > > > I can't help but point out that we wouldn't be in this mess if you'd got > > your stuff into -next sooner ;) > > > > Well yes, but it was hard to plan our holidays around the deadlines and > unfortunately I couldn't find time to verify the kvmarm branch to make > it ready for -next inclusion before I went on holiday this time. In any > case, this should be the exception rather than the rule, and I do try to > get the next branch ready as soon as possible usually. Understood, but the communication could have been a lot better. Whilst you were away, -next broke due to the non-virt parts of the GICv3 driver (assumedly going via the irqchip tree?): http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/275410.html Catalin fixed that up, and then Jason stated that the intention was for GICv3 to go via the arm64 tree (see above link). Based on that, we pulled his tag into our tree and applied our fix on top. Now a bunch of stuff has cropped up out of nowhere causing conflicts and breakages for everybody ~3 days before the merge window opens (the code defaults to 'y'). We're not mind-readers, so all it would have taken is a mail to the relevant maintainers before everybody disappeared on holiday saying (a) what the merge plan is and (b) what to do if everything goes wrong while you're away. Bah, maybe I'm just being grumpy... Will -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][next] arm64: KVM: GICv3: move system register access to msr_s/mrs_s
On Thu, Jul 31, 2014 at 02:19:47PM +0100, Will Deacon wrote: > On Thu, Jul 31, 2014 at 02:16:39PM +0100, Marc Zyngier wrote: > > Commit 72c583951526 (arm64: gicv3: Allow GICv3 compilation with > > older binutils) changed the way we express the GICv3 system registers, > > but couldn't change the occurences used by KVM as the code wasn't > > merged yet. > > > > Just fix the accessors. > > > > Cc: Will Deacon > > Cc: Catalin Marinas > > Cc: Christoffer Dall > > Signed-off-by: Marc Zyngier > > --- > > -next is currently borked. I suggest we take this patch via the kvmarm tree, > > and only send our PR once the arm64 tree has hit Linus' tree. It contains > > the same dependency on the GIC tree anyway. > > I'm fine with that as long as Paulo doesn't mind waiting for the arm64 stuff > to go in first (I have no reason to delay my pull request, so shouldn't be > an issue). > > If it helps: > > Acked-by: Will Deacon > > I can't help but point out that we wouldn't be in this mess if you'd got > your stuff into -next sooner ;) > Well yes, but it was hard to plan our holidays around the deadlines and unfortunately I couldn't find time to verify the kvmarm branch to make it ready for -next inclusion before I went on holiday this time. In any case, this should be the exception rather than the rule, and I do try to get the next branch ready as soon as possible usually. -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][next] arm64: KVM: GICv3: move system register access to msr_s/mrs_s
On Thu, Jul 31, 2014 at 02:16:39PM +0100, Marc Zyngier wrote: > Commit 72c583951526 (arm64: gicv3: Allow GICv3 compilation with > older binutils) changed the way we express the GICv3 system registers, > but couldn't change the occurences used by KVM as the code wasn't > merged yet. > > Just fix the accessors. > > Cc: Will Deacon > Cc: Catalin Marinas > Cc: Christoffer Dall > Signed-off-by: Marc Zyngier > --- > -next is currently borked. I suggest we take this patch via the kvmarm tree, > and only send our PR once the arm64 tree has hit Linus' tree. It contains > the same dependency on the GIC tree anyway. I'm fine with that as long as Paulo doesn't mind waiting for the arm64 stuff to go in first (I have no reason to delay my pull request, so shouldn't be an issue). If it helps: Acked-by: Will Deacon I can't help but point out that we wouldn't be in this mess if you'd got your stuff into -next sooner ;) Will -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][next] arm64: KVM: GICv3: move system register access to msr_s/mrs_s
Commit 72c583951526 (arm64: gicv3: Allow GICv3 compilation with older binutils) changed the way we express the GICv3 system registers, but couldn't change the occurences used by KVM as the code wasn't merged yet. Just fix the accessors. Cc: Will Deacon Cc: Catalin Marinas Cc: Christoffer Dall Signed-off-by: Marc Zyngier --- -next is currently borked. I suggest we take this patch via the kvmarm tree, and only send our PR once the arm64 tree has hit Linus' tree. It contains the same dependency on the GIC tree anyway. arch/arm64/kvm/vgic-v3-switch.S | 130 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/arch/arm64/kvm/vgic-v3-switch.S b/arch/arm64/kvm/vgic-v3-switch.S index 21e68f6..d160469 100644 --- a/arch/arm64/kvm/vgic-v3-switch.S +++ b/arch/arm64/kvm/vgic-v3-switch.S @@ -48,11 +48,11 @@ dsb st // Save all interesting registers - mrs x4, ICH_HCR_EL2 - mrs x5, ICH_VMCR_EL2 - mrs x6, ICH_MISR_EL2 - mrs x7, ICH_EISR_EL2 - mrs x8, ICH_ELSR_EL2 + mrs_s x4, ICH_HCR_EL2 + mrs_s x5, ICH_VMCR_EL2 + mrs_s x6, ICH_MISR_EL2 + mrs_s x7, ICH_EISR_EL2 + mrs_s x8, ICH_ELSR_EL2 str w4, [x3, #VGIC_V3_CPU_HCR] str w5, [x3, #VGIC_V3_CPU_VMCR] @@ -60,9 +60,9 @@ str w7, [x3, #VGIC_V3_CPU_EISR] str w8, [x3, #VGIC_V3_CPU_ELRSR] - msr ICH_HCR_EL2, xzr + msr_s ICH_HCR_EL2, xzr - mrs x21, ICH_VTR_EL2 + mrs_s x21, ICH_VTR_EL2 mvn w22, w21 ubfiz w23, w22, 2, 4 // w23 = (15 - ListRegs) * 4 @@ -71,22 +71,22 @@ br x24 1: - mrs x20, ICH_LR15_EL2 - mrs x19, ICH_LR14_EL2 - mrs x18, ICH_LR13_EL2 - mrs x17, ICH_LR12_EL2 - mrs x16, ICH_LR11_EL2 - mrs x15, ICH_LR10_EL2 - mrs x14, ICH_LR9_EL2 - mrs x13, ICH_LR8_EL2 - mrs x12, ICH_LR7_EL2 - mrs x11, ICH_LR6_EL2 - mrs x10, ICH_LR5_EL2 - mrs x9, ICH_LR4_EL2 - mrs x8, ICH_LR3_EL2 - mrs x7, ICH_LR2_EL2 - mrs x6, ICH_LR1_EL2 - mrs x5, ICH_LR0_EL2 + mrs_s x20, ICH_LR15_EL2 + mrs_s x19, ICH_LR14_EL2 + mrs_s x18, ICH_LR13_EL2 + mrs_s x17, ICH_LR12_EL2 + mrs_s x16, ICH_LR11_EL2 + mrs_s x15, ICH_LR10_EL2 + mrs_s x14, ICH_LR9_EL2 + mrs_s x13, ICH_LR8_EL2 + mrs_s x12, ICH_LR7_EL2 + mrs_s x11, ICH_LR6_EL2 + mrs_s x10, ICH_LR5_EL2 + mrs_s x9, ICH_LR4_EL2 + mrs_s x8, ICH_LR3_EL2 + mrs_s x7, ICH_LR2_EL2 + mrs_s x6, ICH_LR1_EL2 + mrs_s x5, ICH_LR0_EL2 adr x24, 1f add x24, x24, x23 @@ -113,34 +113,34 @@ tbnzw21, #29, 6f// 6 bits tbz w21, #30, 5f// 5 bits // 7 bits - mrs x20, ICH_AP0R3_EL2 + mrs_s x20, ICH_AP0R3_EL2 str w20, [x3, #(VGIC_V3_CPU_AP0R + 3*4)] - mrs x19, ICH_AP0R2_EL2 + mrs_s x19, ICH_AP0R2_EL2 str w19, [x3, #(VGIC_V3_CPU_AP0R + 2*4)] -6: mrs x18, ICH_AP0R1_EL2 +6: mrs_s x18, ICH_AP0R1_EL2 str w18, [x3, #(VGIC_V3_CPU_AP0R + 1*4)] -5: mrs x17, ICH_AP0R0_EL2 +5: mrs_s x17, ICH_AP0R0_EL2 str w17, [x3, #VGIC_V3_CPU_AP0R] tbnzw21, #29, 6f// 6 bits tbz w21, #30, 5f// 5 bits // 7 bits - mrs x20, ICH_AP1R3_EL2 + mrs_s x20, ICH_AP1R3_EL2 str w20, [x3, #(VGIC_V3_CPU_AP1R + 3*4)] - mrs x19, ICH_AP1R2_EL2 + mrs_s x19, ICH_AP1R2_EL2 str w19, [x3, #(VGIC_V3_CPU_AP1R + 2*4)] -6: mrs x18, ICH_AP1R1_EL2 +6: mrs_s x18, ICH_AP1R1_EL2 str w18, [x3, #(VGIC_V3_CPU_AP1R + 1*4)] -5: mrs x17, ICH_AP1R0_EL2 +5: mrs_s x17, ICH_AP1R0_EL2 str w17, [x3, #VGIC_V3_CPU_AP1R] // Restore SRE_EL1 access and re-enable SRE at EL1. - mrs x5, ICC_SRE_EL2 + mrs_s x5, ICC_SRE_EL2 orr x5, x5, #ICC_SRE_EL2_ENABLE - msr ICC_SRE_EL2, x5 + msr_s ICC_SRE_EL2, x5 isb mov x5, #1 - msr ICC_SRE_EL1, x5 + msr_s ICC_SRE_EL1, x5 .endm /* @@ -150,7 +150,7 @@ .macro restore_vgic_v3_state // Disable SRE_EL1 access. Necessary, otherwise // ICH_VMCR_EL2.VFIQEn becomes one, and FIQ happens... - msr ICC_SRE_EL1, xzr + msr_s ICC_SRE_EL1, xzr isb // Compute the address of struct vgic_cpu @@ -160,34 +160,34 @@ ldr w4, [x3, #VGIC_V3_CPU_HCR] ldr w5, [x3, #VGIC_V3_CPU_VMCR] - msr ICH_HCR_EL2, x4 - msr ICH_VMCR_EL2, x5 + msr_s ICH_HCR_EL2, x4 + msr_s ICH_VMCR_EL2, x5 - mrs x21, ICH_VTR_EL