On Wed, May 14, 2014 at 05:58:04PM +0100, Marc Zyngier wrote: > On 14/05/14 17:34, Christoffer Dall wrote: > > On 14 May 2014 15:33, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > >> On Fri, May 09 2014 at 3:07:23 pm BST, Christoffer Dall <christoffer.dall@xxxxxxxxxx> wrote: > >>> On Wed, Apr 16, 2014 at 02:39:49PM +0100, Marc Zyngier wrote: > >>>> GICv3 requires the hcr_el2 switch to be tightly coupled with some > >>>> of the interrupt controller's register switch. > >>> > >>> can you be more specific, this feels a bit odd, enabling Stage-2 > >>> translation and configuring all traps from within the vgic code... > >> > >> The IMO and FMO bits must be set before restoring the various system > >> registers in GICv3. But I agreee that this looks pretty horrible. > >> > >> The alternative is to split the bits we set in HCR_EL2 into two sets (VM > >> and trap control on one side, interrupt control on the other). This > >> would translate into two accesses to HCR_EL2, but it would look > >> nicer. I'll have a look. > >> > >>>> In order to have similar code paths, start moving the hcr_el2 > >>>> manipulation code to the GICv2 switch code. > >>>> > >>>> Acked-by: Catalin Marinas <catalin.marinas@xxxxxxx> > >>>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> > >>>> --- > >>>> arch/arm64/kvm/hyp.S | 7 ------- > >>>> arch/arm64/kvm/vgic-v2-switch.S | 8 ++++++++ > >>>> 2 files changed, 8 insertions(+), 7 deletions(-) > >>>> > >>>> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S > >>>> index aed72d0..92b9120 100644 > >>>> --- a/arch/arm64/kvm/hyp.S > >>>> +++ b/arch/arm64/kvm/hyp.S > >>>> @@ -335,11 +335,6 @@ > >>>> .endm > >>>> > >>>> .macro activate_traps > >>>> - ldr x2, [x0, #VCPU_IRQ_LINES] > >>>> - ldr x1, [x0, #VCPU_HCR_EL2] > >>>> - orr x2, x2, x1 > >>>> - msr hcr_el2, x2 > >>>> - > >>>> ldr x2, =(CPTR_EL2_TTA) > >>>> msr cptr_el2, x2 > >>>> > >>>> @@ -353,8 +348,6 @@ > >>>> .endm > >>>> > >>>> .macro deactivate_traps > >>>> - mov x2, #HCR_RW > >>>> - msr hcr_el2, x2 > >>>> msr cptr_el2, xzr > >>>> msr hstr_el2, xzr > >>>> > >>>> diff --git a/arch/arm64/kvm/vgic-v2-switch.S b/arch/arm64/kvm/vgic-v2-switch.S > >>>> index c5dc777..d36cd7a 100644 > >>>> --- a/arch/arm64/kvm/vgic-v2-switch.S > >>>> +++ b/arch/arm64/kvm/vgic-v2-switch.S > >>>> @@ -85,6 +85,9 @@ CPU_BE( rev w5, w5 ) > >>>> sub w4, w4, #1 > >>>> cbnz w4, 1b > >>>> 2: > >>>> + mov x2, #HCR_RW > >>>> + msr hcr_el2, x2 > >>>> + isb > >>>> .endm > >>>> > >>>> /* > >>>> @@ -92,6 +95,11 @@ CPU_BE( rev w5, w5 ) > >>>> * x0: Register pointing to VCPU struct > >>>> */ > >>>> .macro restore_vgic_v2_state > >>>> + ldr x2, [x0, #VCPU_IRQ_LINES] > >>> > >>> will this ever have any values on aarch64? Don't we mandate vgic > >>> support and bail out during hyp init if we cannot init a vgic? > >> > >> Yes. But that doesn't mean we don't support the feature either. The case > >> is fairly slim, I agree, but it has been there since Day-1... > >> > > See kvm_vm_ioctl_irq_line() in arch/arm/kvm/arm.c: > > > > case KVM_ARM_IRQ_TYPE_CPU: > > if (irqchip_in_kernel(kvm)) > > return -ENXIO; > > Unfortunately, this only checks if the VM has a vgic instantiated. It is > always possible to create a VM without the in-kernel GIC, and use the > pins to inject IRQs. As I said, unlikely to happen, but nonetheless... > Yeah, you're right, I'm an idiot. Sorry for the noise. Thanks, -Christoffer _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
