[PATCH v4 10/13] ARM: KVM: VGIC control interface world switch

[Marc Zyngier]

On 03/12/12 13:31, Will Deacon wrote:
> On Sat, Nov 10, 2012 at 03:45:25PM +0000, Christoffer Dall wrote:
>> From: Marc Zyngier <marc.zyngier at arm.com>
>>
>> Enable the VGIC control interface to be save-restored on world switch.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier at arm.com>
>> Signed-off-by: Christoffer Dall <c.dall at virtualopensystems.com>
>> ---
>>  arch/arm/include/asm/kvm_arm.h |   12 +++++++
>>  arch/arm/kernel/asm-offsets.c  |   12 +++++++
>>  arch/arm/kvm/interrupts_head.S |   68 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 92 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
>> index 246afd7..8f5dd22 100644
>> --- a/arch/arm/include/asm/kvm_arm.h
>> +++ b/arch/arm/include/asm/kvm_arm.h
>> @@ -192,4 +192,16 @@
>>  #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_EISR0 	0x20
>> +#define GICH_EISR1 	0x24
>> +#define GICH_ELRSR0 	0x30
>> +#define GICH_ELRSR1 	0x34
>> +#define GICH_APR	0xf0
>> +#define GICH_LR0	0x100
> 
> Similar thing to the other new gic defines -- they're probably better off
> in gic.h

Agreed.

>> +
>>  #endif /* __ARM_KVM_ARM_H__ */
>> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
>> index 95cab37..39b6221 100644
>> --- a/arch/arm/kernel/asm-offsets.c
>> +++ b/arch/arm/kernel/asm-offsets.c
>> @@ -167,6 +167,18 @@ int main(void)
>>    DEFINE(VCPU_HxFAR,		offsetof(struct kvm_vcpu, arch.hxfar));
>>    DEFINE(VCPU_HPFAR,		offsetof(struct kvm_vcpu, arch.hpfar));
>>    DEFINE(VCPU_HYP_PC,		offsetof(struct kvm_vcpu, arch.hyp_pc));
>> +#ifdef CONFIG_KVM_ARM_VGIC
>> +  DEFINE(VCPU_VGIC_CPU,		offsetof(struct kvm_vcpu, arch.vgic_cpu));
>> +  DEFINE(VGIC_CPU_HCR,		offsetof(struct vgic_cpu, vgic_hcr));
>> +  DEFINE(VGIC_CPU_VMCR,		offsetof(struct vgic_cpu, vgic_vmcr));
>> +  DEFINE(VGIC_CPU_MISR,		offsetof(struct vgic_cpu, vgic_misr));
>> +  DEFINE(VGIC_CPU_EISR,		offsetof(struct vgic_cpu, vgic_eisr));
>> +  DEFINE(VGIC_CPU_ELRSR,	offsetof(struct vgic_cpu, vgic_elrsr));
>> +  DEFINE(VGIC_CPU_APR,		offsetof(struct vgic_cpu, vgic_apr));
>> +  DEFINE(VGIC_CPU_LR,		offsetof(struct vgic_cpu, vgic_lr));
>> +  DEFINE(VGIC_CPU_NR_LR,	offsetof(struct 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_head.S b/arch/arm/kvm/interrupts_head.S
>> index 2ac8b4a..c2423d8 100644
>> --- a/arch/arm/kvm/interrupts_head.S
>> +++ b/arch/arm/kvm/interrupts_head.S
>> @@ -341,6 +341,45 @@
>>   * @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
>> +
>> +	/* Compute the address of struct vgic_cpu */
>> +	add	r11, \vcpup, #VCPU_VGIC_CPU
> 
> Given that we're dealing with constants, it would be more efficient to
> express this addition as part of the immediate offset and let gas spit
> out the final computed address for the stores below.

We had that for a while, and with the kvm_vcpu structure growing, we
ended up having fields out of the reach of an immediate offset. Is
kvm_vcpu too big? Yes.

>> +
>> +	/* Save all interesting registers */
>> +	ldr	r3, [r2, #GICH_HCR]
>> +	ldr	r4, [r2, #GICH_VMCR]
>> +	ldr	r5, [r2, #GICH_MISR]
>> +	ldr	r6, [r2, #GICH_EISR0]
>> +	ldr	r7, [r2, #GICH_EISR1]
>> +	ldr	r8, [r2, #GICH_ELRSR0]
>> +	ldr	r9, [r2, #GICH_ELRSR1]
>> +	ldr	r10, [r2, #GICH_APR]
>> +
>> +	str	r3, [r11, #VGIC_CPU_HCR]
>> +	str	r4, [r11, #VGIC_CPU_VMCR]
>> +	str	r5, [r11, #VGIC_CPU_MISR]
>> +	str	r6, [r11, #VGIC_CPU_EISR]
>> +	str	r7, [r11, #(VGIC_CPU_EISR + 4)]
>> +	str	r8, [r11, #VGIC_CPU_ELRSR]
>> +	str	r9, [r11, #(VGIC_CPU_ELRSR + 4)]
>> +	str	r10, [r11, #VGIC_CPU_APR]
>> +
>> +	/* Save list registers */
>> +	add	r2, r2, #GICH_LR0
>> +	add	r3, r11, #VGIC_CPU_LR
>> +	ldr	r4, [r11, #VGIC_CPU_NR_LR]
>> +1:	ldr	r6, [r2], #4
>> +	str	r6, [r3], #4
>> +	subs	r4, r4, #1
>> +	bne	1b
>> +2:
>> +#endif
>>  .endm
> 
> Will
> 


-- 
Jazz is not dead. It just smells funny...