[PATCH v4 03/13] ARM: KVM: Initial VGIC infrastructure support
From: Will Deacon <hidden>
Date: 2012-11-28 12:49:55
Also in:
kvm
On Sat, Nov 10, 2012 at 03:44:37PM +0000, Christoffer Dall wrote:
From: Marc Zyngier <redacted> Wire the basic framework code for VGIC support. Nothing to enable yet.
Again, not sure how useful this patch is. Might as well merge it with code that actually does something. Couple of comments inline anyway...
Signed-off-by: Marc Zyngier <redacted> Signed-off-by: Christoffer Dall <redacted> --- arch/arm/include/asm/kvm_host.h | 7 ++++ arch/arm/include/asm/kvm_vgic.h | 70 +++++++++++++++++++++++++++++++++++++++ arch/arm/kvm/arm.c | 21 +++++++++++- arch/arm/kvm/interrupts.S | 4 ++ arch/arm/kvm/mmio.c | 3 ++ virt/kvm/kvm_main.c | 5 ++- 6 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 arch/arm/include/asm/kvm_vgic.h
[...]
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 60b119a..426828a 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c@@ -183,6 +183,9 @@ int kvm_dev_ioctl_check_extension(long ext) { int r; switch (ext) { +#ifdef CONFIG_KVM_ARM_VGIC + case KVM_CAP_IRQCHIP: +#endif case KVM_CAP_USER_MEMORY: case KVM_CAP_DESTROY_MEMORY_REGION_WORKS: case KVM_CAP_ONE_REG:@@ -304,6 +307,10 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) { /* Force users to call KVM_ARM_VCPU_INIT */ vcpu->arch.target = -1; + + /* Set up VGIC */ + kvm_vgic_vcpu_init(vcpu); + return 0; }@@ -363,7 +370,7 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu, */ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v) { - return !!v->arch.irq_lines; + return !!v->arch.irq_lines || kvm_vgic_vcpu_pending_irq(v); }
So interrupt injection without the in-kernel GIC updates irq_lines, but the in-kernel GIC has its own separate data structures? Why can't the in-kernel GIC just use irq_lines instead of irq_pending_on_cpu?
quoted hunk ↗ jump to hunk
int kvm_arch_vcpu_in_guest_mode(struct kvm_vcpu *v)@@ -633,6 +640,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) update_vttbr(vcpu->kvm); + kvm_vgic_sync_to_cpu(vcpu); + local_irq_disable(); /*@@ -645,6 +654,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) if (ret <= 0 || need_new_vmid_gen(vcpu->kvm)) { local_irq_enable(); + kvm_vgic_sync_from_cpu(vcpu); continue; }
For VFP, we use different terminology (sync and flush). I don't think they're any clearer than what you have, but the consistency would be nice. Given that both these functions are run with interrupts enabled, why doesn't the second require a lock for updating dist->irq_pending_on_cpu? I notice there's a random smp_mb() over there...
quoted hunk ↗ jump to hunk
@@ -683,6 +693,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) * Back from guest *************************************************************/ + kvm_vgic_sync_from_cpu(vcpu);
Likewise.
quoted hunk ↗ jump to hunk
ret = handle_exit(vcpu, run, ret); }@@ -965,6 +977,13 @@ static int init_hyp_mode(void) } } + /* + * Init HYP view of VGIC + */ + err = kvm_vgic_hyp_init(); + if (err) + goto out_free_mappings; + return 0; out_free_vfp: free_percpu(kvm_host_vfp_state);
[...]
quoted hunk ↗ jump to hunk
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 2fb7319..665af96 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c@@ -1880,12 +1880,13 @@ static long kvm_vcpu_ioctl(struct file *filp, if (vcpu->kvm->mm != current->mm) return -EIO; -#if defined(CONFIG_S390) || defined(CONFIG_PPC) +#if defined(CONFIG_S390) || defined(CONFIG_PPC) || defined(CONFIG_ARM) /* * Special cases: vcpu ioctls that are asynchronous to vcpu execution, * so vcpu_load() would break it. */ - if (ioctl == KVM_S390_INTERRUPT || ioctl == KVM_INTERRUPT) + if (ioctl == KVM_S390_INTERRUPT || ioctl == KVM_INTERRUPT || + ioctl == KVM_IRQ_LINE) return kvm_arch_vcpu_ioctl(filp, ioctl, arg); #endif
Separate patch? Will