KVM: arm/arm64: Clean up some obsolete code
v2 cover KVM: arm/arm64: Clean up some obsolete code
Current KVM code has lots of old redundancies, which can be cleaned up. This patchset is actually a better alternative to
http://www.spinics.net/lists/arm-kernel/msg430726.html, which allows to keep piggy-backed LRs. The idea is based on the fact that our code also maintains LR state in elrsr, and this information is enough to track LR usage.
In case of problems this series can be applied partially, each patch is a complete refactoring step on its own.
Thanks to Andre Przywara for pinpointing some 4.3+ specifics.
This version has been tested on SMDK5410 development board (Exynos5410 SoC).
v1 => v2:
- Rebased to kvmarm/next of 23.10.2015.
- Do not use vgic_retire_lr() for initializing ELRSR bitmask, because now it also handles pushback of PENDING state, use direct initialization instead (copied from Andre's patchset).
- Took more care about vgic_retire_lr(), which has deserved own patch.
Pavel Fedin (3):
KVM: arm/arm64: Optimize away redundant LR tracking
KVM: arm/arm64: Merge vgic_set_lr() and vgic_sync_lr_elrsr()
KVM: arm/arm64: Refactor vgic_retire_lr()
include/kvm/arm_vgic.h | 7 ---- virt/kvm/arm/vgic-v2.c | 6 +-- virt/kvm/arm/vgic-v3.c | 6 +-- virt/kvm/arm/vgic.c | 106 ++++++++++++++----------------------------------- 4 files changed, 31 insertions(+), 94 deletions(-)
Signature · 1 line
2.4.4
PATCH v2 1/3 KVM: arm/arm64: Optimize away redundant LR tracking
Currently we use vgic_irq_lr_map in order to track which LRs hold which IRQs, and lr_used bitmap in order to track which LRs are used or free.
vgic_irq_lr_map is actually used only for piggy-back optimization, and can be easily replaced by iteration over lr_used. This is good because in future, when LPI support is introduced, number of IRQs will grow up to at least 16384, while numbers from 1024 to 8192 are never going to be used. This would be a huge memory waste.
In its turn, lr_used is also completely redundant since ae705930fca6322600690df9dc1c7d0516145a93 ("arm/arm64: KVM: Keep elrsr/aisr in sync with software model"), because together with lr_used we also update elrsr. This allows to easily replace lr_used with elrsr, inverting all conditions (because in elrsr '1' means 'free').
--- include/kvm/arm_vgic.h | 6 ------ virt/kvm/arm/vgic-v2.c | 1 + virt/kvm/arm/vgic-v3.c | 1 + virt/kvm/arm/vgic.c | 55 ++++++++++++++++---------------------------------- 4 files changed, 19 insertions(+), 44 deletions(-)
include/kvm/arm_vgic.h +0 -6
@@ -295,9 +295,6 @@ struct vgic_v3_cpu_if { };struct vgic_cpu { -/* per IRQ to LR mapping */ -u8 *vgic_irq_lr_map; -/* Pending/active/both interrupts on this VCPU */ DECLARE_BITMAP(pending_percpu, VGIC_NR_PRIVATE_IRQS); DECLARE_BITMAP(active_percpu, VGIC_NR_PRIVATE_IRQS);
@@ -308,9 +305,6 @@ struct vgic_cpu {unsigned long *active_shared;unsigned long *pend_act_shared; -/* Bitmap of used/free list registers */ -DECLARE_BITMAP (lr_used,VGIC_V2_MAX_LRS ); -/* Number of list registers on this CPU */ int nr_lr;
virt/kvm/arm/vgic-v2.c +1 -0
@@ -158,6 +158,7 @@ static void vgic_v2_enable(struct kvm_vcpu *vcpu)* anyway. */ vcpu->arch.vgic_cpu.vgic_v2.vgic_vmcr =0 ; + vcpu->arch.vgic_cpu.vgic_v2.vgic_elrsr = ~0 ;/* Get the show on the road... */ vcpu->arch.vgic_cpu.vgic_v2.vgic_hcr =GICH_HCR_EN ;
virt/kvm/arm/vgic-v3.c +1 -0
@@ -193,6 +193,7 @@ static void vgic_v3_enable(struct kvm_vcpu *vcpu)* anyway. */ vgic_v3->vgic_vmcr =0 ; + vgic_v3->vgic_elrsr = ~0 ;/* * If we are emulating a GICv3, we do it in an non-GICv2-compatible
virt/kvm/arm/vgic.c +17 -38
158 lines not on this page · raw message
Signature · 1 line
2.4.4
PATCH v2 2/3 KVM: arm/arm64: Merge vgic_set_lr() and vgic_sync_lr_elrsr()
In reply to Pavel FedinNow we see that vgic_set_lr() and vgic_sync_lr_elrsr() are always used together. Merge them into one function, saving from second vgic_ops dereferencing every time.
Additionally, remove unnecessary vgic_set_lr() and LR_STATE_PENDING check in vgic_unqueue_irqs(), because all these things are now done by the following vgic_retire_lr().
--- include/kvm/arm_vgic.h | 1 - virt/kvm/arm/vgic-v2.c | 5 ----- virt/kvm/arm/vgic-v3.c | 5 ----- virt/kvm/arm/vgic.c | 33 ++++----------------------------- 4 files changed, 4 insertions(+), 40 deletions(-)
include/kvm/arm_vgic.h +0 -1
@@ -112,7 +112,6 @@ struct vgic_vmcr {struct vgic_ops {struct vgic_lr (*get_lr)(const struct kvm_vcpu *,int );void (*set_lr)(struct kvm_vcpu *,int ,struct vgic_lr ); -void (*sync_lr_elrsr)(struct kvm_vcpu *,int ,struct vgic_lr );u64 (*get_elrsr)(const struct kvm_vcpu *vcpu);u64 (*get_eisr)(const struct kvm_vcpu *vcpu);void (*clear_eisr)(struct kvm_vcpu *vcpu);
virt/kvm/arm/vgic-v2.c +0 -5
@@ -79,11 +79,7 @@ static void vgic_v2_set_lr(struct kvm_vcpu *vcpu, int lr, lr_val |= (lr_desc.source <<GICH_LR_PHYSID_CPUID_SHIFT ); vcpu->arch.vgic_cpu.vgic_v2.vgic_lr[lr] = lr_val; -} -static void vgic_v2_sync_lr_elrsr (struct kvm_vcpu *vcpu,int lr, -struct vgic_lr lr_desc) -{if (!(lr_desc.state &LR_STATE_MASK )) vcpu->arch.vgic_cpu.vgic_v2.vgic_elrsr |= (1ULL << lr);else
@@ -167,7 +163,6 @@ static void vgic_v2_enable(struct kvm_vcpu *vcpu)static const struct vgic_ops vgic_v2_ops = { .get_lr = vgic_v2_get_lr, .set_lr = vgic_v2_set_lr, - .sync_lr_elrsr = vgic_v2_sync_lr_elrsr, .get_elrsr = vgic_v2_get_elrsr, .get_eisr = vgic_v2_get_eisr, .clear_eisr = vgic_v2_clear_eisr,
virt/kvm/arm/vgic-v3.c +0 -5
@@ -112,11 +112,7 @@ static void vgic_v3_set_lr(struct kvm_vcpu *vcpu, int lr, } vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[LR_INDEX (lr)] = lr_val; -} -static void vgic_v3_sync_lr_elrsr (struct kvm_vcpu *vcpu,int lr, -struct vgic_lr lr_desc) -{if (!(lr_desc.state &LR_STATE_MASK )) vcpu->arch.vgic_cpu.vgic_v3.vgic_elrsr |= (1U << lr);else
@@ -212,7 +208,6 @@ static void vgic_v3_enable(struct kvm_vcpu *vcpu)static const struct vgic_ops vgic_v3_ops = { .get_lr = vgic_v3_get_lr, .set_lr = vgic_v3_set_lr, - .sync_lr_elrsr = vgic_v3_sync_lr_elrsr, .get_elrsr = vgic_v3_get_elrsr, .get_eisr = vgic_v3_get_eisr, .clear_eisr = vgic_v3_clear_eisr,
virt/kvm/arm/vgic.c +4 -29
@@ -717,28 +717,13 @@ void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)* interrupt then move the active state to the * distributor tracking bit. */ -if (lr.state &LR_STATE_ACTIVE ) { +if (lr.state &LR_STATE_ACTIVE )vgic_irq_set_active (vcpu, lr.irq); - lr.state &= ~LR_STATE_ACTIVE ; - }/* * Reestablish the pending state on the distributor and the -* CPU interface. It may have already been pending, but that -* is fine, then we are only setting a few bits that were -* already set. -*/ -if (lr.state &LR_STATE_PENDING ) { -vgic_dist_irq_set_pending (vcpu, lr.irq); - lr.state &= ~LR_STATE_PENDING ; - } - -vgic_set_lr (vcpu, i, lr); - -/* -* Mark the LR as free for other use. +* CPU interface and mark the LR as free for other use. */ -BUG_ON (lr.state &LR_STATE_MASK );vgic_retire_lr (i, lr.irq, vcpu);vgic_irq_clear_queued (vcpu, lr.irq);
@@ -1048,12 +1033,6 @@ static void vgic_set_lr(struct kvm_vcpu *vcpu, int lr, vgic_ops->set_lr (vcpu, lr, vlr); } -static void vgic_sync_lr_elrsr (struct kvm_vcpu *vcpu,int lr, -struct vgic_lr vlr) -{ - vgic_ops->sync_lr_elrsr (vcpu, lr, vlr); -} -static inline u64 vgic_get_elrsr (struct kvm_vcpu *vcpu) {return vgic_ops->get_elrsr (vcpu);
@@ -1114,7 +1093,6 @@ static void vgic_retire_lr(int lr_nr, int irq, struct kvm_vcpu *vcpu) vlr.state =0 ;vgic_set_lr (vcpu, lr_nr, vlr); -vgic_sync_lr_elrsr (vcpu, lr_nr, vlr); }/*
@@ -1179,7 +1157,6 @@ static void vgic_queue_irq_to_lr(struct kvm_vcpu *vcpu, int irq, }vgic_set_lr (vcpu, lr_nr, vlr); -vgic_sync_lr_elrsr (vcpu, lr_nr, vlr); }/*
@@ -1359,8 +1336,6 @@ static int process_queued_irq(struct kvm_vcpu *vcpu, vlr.hwirq =0 ;vgic_set_lr (vcpu, lr, vlr); -vgic_sync_lr_elrsr (vcpu, lr, vlr); -return pending; }
@@ -1461,8 +1436,6 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)bool level_pending; level_pending =vgic_process_maintenance (vcpu); - elrsr =vgic_get_elrsr (vcpu); - elrsr_ptr =u64_to_bitmask (&elrsr);/* Deal with HW interrupts, and clear mappings for empty LRs */ for (lr =0 ; lr < vgic->nr_lr; lr++) {
@@ -1473,6 +1446,8 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu) }/* Check if we still have something up our sleeve... */ + elrsr =vgic_get_elrsr (vcpu); + elrsr_ptr =u64_to_bitmask (&elrsr); pending =find_first_zero_bit (elrsr_ptr, vgic->nr_lr);if (level_pending || pending < vgic->nr_lr)set_bit (vcpu->vcpu_id, dist->irq_pending_on_cpu);
Signature · 1 line
2.4.4
PATCH v2 3/3 KVM: arm/arm64: Refactor vgic_retire_lr()
In reply to Pavel Fedin1. Remove unnecessary 'irq' argument, because irq number can be retrieved from the LR.
2. vgic_retire_lr() is always accompanied by vgic_irq_clear_queued(). Since it already does more than just clearing the LR, move vgic_irq_clear_queued() inside of it.
--- virt/kvm/arm/vgic.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
virt/kvm/arm/vgic.c +8 -10
@@ -105,7 +105,7 @@#include "vgic.h" static void vgic_retire_disabled_irqs (struct kvm_vcpu *vcpu); -static void vgic_retire_lr (int lr_nr,int irq,struct kvm_vcpu *vcpu); +static void vgic_retire_lr (int lr_nr,struct kvm_vcpu *vcpu);static struct vgic_lr vgic_get_lr (const struct kvm_vcpu *vcpu,int lr);static void vgic_set_lr (struct kvm_vcpu *vcpu,int lr,struct vgic_lr lr_desc);static u64 vgic_get_elrsr (struct kvm_vcpu *vcpu);
@@ -724,8 +724,7 @@ void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)* Reestablish the pending state on the distributor and the * CPU interface and mark the LR as free for other use. */ -vgic_retire_lr (i, lr.irq, vcpu); -vgic_irq_clear_queued (vcpu, lr.irq); +vgic_retire_lr (i, vcpu);/* Finally update the VGIC state. */ vgic_update_state (vcpu->kvm);
@@ -1078,16 +1077,18 @@ static inline void vgic_enable(struct kvm_vcpu *vcpu) vgic_ops->enable (vcpu); } -static void vgic_retire_lr (int lr_nr,int irq,struct kvm_vcpu *vcpu) +static void vgic_retire_lr (int lr_nr,struct kvm_vcpu *vcpu) {struct vgic_lr vlr =vgic_get_lr (vcpu, lr_nr); +vgic_irq_clear_queued (vcpu, vlr.irq); +/* * We must transfer the pending state back to the distributor before * retiring the LR, otherwise we may loose edge-triggered interrupts. */ if (vlr.state &LR_STATE_PENDING ) { -vgic_dist_irq_set_pending (vcpu, irq); +vgic_dist_irq_set_pending (vcpu, vlr.irq); vlr.hwirq =0 ; }
@@ -1113,11 +1114,8 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)for_each_clear_bit (lr, elrsr_ptr, vgic->nr_lr) {struct vgic_lr vlr =vgic_get_lr (vcpu, lr); -if (!vgic_irq_is_enabled (vcpu, vlr.irq)) { -vgic_retire_lr (lr, vlr.irq, vcpu); -if (vgic_irq_is_queued (vcpu, vlr.irq)) -vgic_irq_clear_queued (vcpu, vlr.irq); - } +if (!vgic_irq_is_enabled (vcpu, vlr.irq)) +vgic_retire_lr (lr, vcpu); } }
Signature · 1 line
2.4.4