On Mon, May 14, 2012 at 9:05 AM, Marc Zyngier <marc.zyngier at arm.com> wrote: > Enable the VGIC control interface to be save-restored on world switch. > > Signed-off-by: Marc Zyngier <marc.zyngier at arm.com> > --- > ?arch/arm/include/asm/kvm_arm.h | ? 10 ++++++ > ?arch/arm/kernel/asm-offsets.c ?| ? 10 ++++++ > ?arch/arm/kvm/interrupts.S ? ? ?| ? 62 ++++++++++++++++++++++++++++++++++++++++ > ?3 files changed, 82 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h > index b9e4197..ab13a89 100644 > --- a/arch/arm/include/asm/kvm_arm.h > +++ b/arch/arm/include/asm/kvm_arm.h > @@ -128,4 +128,14 @@ > ?#define HSR_EC_DABT ? ?(0x24) > ?#define HSR_EC_DABT_HYP ? ? ? ?(0x25) > > +/* GICH offsets */ > +#define GICH_HCR ? ? ? 0x0 > +#define GICH_VTR ? ? ? 0x4 > +#define GICH_VMCR ? ? ?0x8 > +#define GICH_MISR ? ? ?0x10 > +#define GICH_ELSR0 ? ? 0x30 > +#define GICH_ELSR1 ? ? 0x34 > +#define GICH_APR ? ? ? 0xf0 > +#define GICH_LR0 ? ? ? 0x100 > + > ?#endif /* __KVM_ARM_H__ */ > diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c > index c8c1b91..d5f4ddf 100644 > --- a/arch/arm/kernel/asm-offsets.c > +++ b/arch/arm/kernel/asm-offsets.c > @@ -176,6 +176,16 @@ int main(void) > ? DEFINE(VCPU_HIFAR, ? ? ? ? ? offsetof(struct kvm_vcpu, arch.hifar)); > ? DEFINE(VCPU_HPFAR, ? ? ? ? ? offsetof(struct kvm_vcpu, arch.hpfar)); > ? DEFINE(VCPU_PC_IPA, ? ? ? ? ?offsetof(struct kvm_vcpu, arch.pc_ipa)); > +#ifdef CONFIG_KVM_ARM_VGIC > + ?DEFINE(VCPU_VGIC_HCR, ? ? ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.vgic_cpu.vgic_hcr)); > + ?DEFINE(VCPU_VGIC_MCR, ? ? ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.vgic_cpu.vgic_mcr)); > + ?DEFINE(VCPU_VGIC_MISR, ? ? ? offsetof(struct kvm_vcpu, arch.vgic_cpu.vgic_misr)); > + ?DEFINE(VCPU_VGIC_ELSR, ? ? ? offsetof(struct kvm_vcpu, arch.vgic_cpu.vgic_elsr)); > + ?DEFINE(VCPU_VGIC_APR, ? ? ? ? ? ? ? ?offsetof(struct kvm_vcpu, arch.vgic_cpu.vgic_apr)); > + ?DEFINE(VCPU_VGIC_LR, ? ? ? ? offsetof(struct kvm_vcpu, arch.vgic_cpu.vgic_lr)); > + ?DEFINE(VCPU_VGIC_NR_LR, ? ? ?offsetof(struct kvm_vcpu, arch.vgic_cpu.nr_lr)); > + ?DEFINE(KVM_VGIC_VCTRL, ? ? ? offsetof(struct kvm, arch.vgic.vctrl_base)); > +#endif > ? DEFINE(KVM_VTTBR, ? ? ? ? ? ?offsetof(struct kvm, arch.vttbr)); > ?#endif > ? return 0; > diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S > index 2e19b34..541532b 100644 > --- a/arch/arm/kvm/interrupts.S > +++ b/arch/arm/kvm/interrupts.S > @@ -241,6 +241,40 @@ ENTRY(__kvm_flush_vm_context) > ?* @vcpup: Register pointing to VCPU struct > ?*/ > ?.macro save_vgic_state vcpup > +#ifdef CONFIG_KVM_ARM_VGIC > + ? ? ? /* Get VGIC VCTRL base into r2 */ > + ? ? ? ldr ? ? r2, [\vcpup, #VCPU_KVM] > + ? ? ? ldr ? ? r2, [r2, #KVM_VGIC_VCTRL] > + ? ? ? cmp ? ? r2, #0 > + ? ? ? beq ? ? 2f > + > + ? ? ? /* Save all interesting registers */ > + ? ? ? ldr ? ? r3, [r2, #GICH_HCR] > + ? ? ? ldr ? ? r4, [r2, #GICH_VMCR] > + ? ? ? ldr ? ? r5, [r2, #GICH_MISR] > + ? ? ? ldr ? ? r6, [r2, #GICH_ELSR0] > + ? ? ? ldr ? ? r7, [r2, #GICH_ELSR1] > + ? ? ? ldr ? ? r8, [r2, #GICH_APR] > + > + ? ? ? str ? ? r3, [\vcpup, #VCPU_VGIC_HCR] > + ? ? ? str ? ? r4, [\vcpup, #VCPU_VGIC_MCR] > + ? ? ? str ? ? r5, [\vcpup, #VCPU_VGIC_MISR] > + ? ? ? str ? ? r6, [\vcpup, #VCPU_VGIC_ELSR] > + ? ? ? str ? ? r7, [\vcpup, #(VCPU_VGIC_ELSR + 4)] > + ? ? ? str ? ? r8, [\vcpup, #VCPU_VGIC_APR] shouldn't we be disabling the virtual interface here? (or restore the host VGICH_HCR)? > + > + ? ? ? /* Save list registers */ > + ? ? ? add ? ? r2, r2, #GICH_LR0 > + ? ? ? add ? ? r3, \vcpup, #VCPU_VGIC_LR > + ? ? ? ldr ? ? r4, [\vcpup, #VCPU_VGIC_NR_LR] > + ? ? ? mov ? ? r5, #0 > +1: ? ? ldr ? ? r6, [r2], #4 > + ? ? ? str ? ? r6, [r3], #4 > + ? ? ? add ? ? r5, r5, #1 > + ? ? ? cmp ? ? r5, r4 see the comment below for saving an instruction > + ? ? ? bne ? ? 1b > +2: > +#endif > ?.endm > > ?/* > @@ -248,6 +282,34 @@ ENTRY(__kvm_flush_vm_context) > ?* @vcpup: Register pointing to VCPU struct > ?*/ > ?.macro restore_vgic_state ? ? ?vcpup > +#ifdef CONFIG_KVM_ARM_VGIC > + ? ? ? /* Get VGIC VCTRL base into r2 */ > + ? ? ? ldr ? ? r2, [\vcpup, #VCPU_KVM] > + ? ? ? ldr ? ? r2, [r2, #KVM_VGIC_VCTRL] > + ? ? ? cmp ? ? r2, #0 > + ? ? ? beq ? ? 2f > + > + ? ? ? /* We only restore a minimal set of registers */ > + ? ? ? ldr ? ? r3, [\vcpup, #VCPU_VGIC_HCR] > + ? ? ? ldr ? ? r4, [\vcpup, #VCPU_VGIC_MCR] > + ? ? ? ldr ? ? r8, [\vcpup, #VCPU_VGIC_APR] > + > + ? ? ? str ? ? r3, [r2, #GICH_HCR] > + ? ? ? str ? ? r4, [r2, #GICH_VMCR] > + ? ? ? str ? ? r8, [r2, #GICH_APR] > + > + ? ? ? /* Restore list registers */ > + ? ? ? add ? ? r2, r2, #GICH_LR0 > + ? ? ? add ? ? r3, \vcpup, #VCPU_VGIC_LR > + ? ? ? ldr ? ? r4, [\vcpup, #VCPU_VGIC_NR_LR] > + ? ? ? mov ? ? r5, #0 > +1: ? ? ldr ? ? r6, [r3], #4 > + ? ? ? str ? ? r6, [r2], #4 > + ? ? ? add ? ? r5, r5, #1 > + ? ? ? cmp ? ? r5, r4 can't you do this instead: + ? ? ? ldr ? ? r4, [\vcpup, #VCPU_VGIC_NR_LR] +1: ? ? ldr ? ? r6, [r3], #4 + ? ? ? str ? ? r6, [r2], #4 + ? ? ? subs ? r4, r4, #1 ? > + ? ? ? bne ? ? 1b > +2: > +#endif > ?.endm > > ?/* Configures the HSTR (Hyp System Trap Register) on entry/return > -- > 1.7.7.1 > Overall looks good, I'm concerned about doing all this MMIO work on every world switch - especially in the case where you have a single guest more or less dedicated to a single physical CPU, so maybe this whole thing could be done lazily by keeping some affinity between vcpus and pcpus, but it's probably premature. -Christoffer
