On Wed, Nov 21, 2012 at 1:15 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> The irq_state bitmap describe the input state of our virtual
> distributor. Calling it "pending" in __kvm_vgic_sync_to_cpu()
> does not significantly help understanding the code.
>
> Remove the local "pending" variable and use the generic accessors
> instead.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
> arch/arm/kvm/vgic.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
> index b83de57..e9587f3 100644
> --- a/arch/arm/kvm/vgic.c
> +++ b/arch/arm/kvm/vgic.c
> @@ -764,7 +764,6 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu)
> {
> struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
> - unsigned long *pending;
> int i, c, vcpu_id;
> int overflow = 0;
>
> @@ -783,7 +782,6 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu)
> }
>
> /* SGIs */
> - pending = vgic_bitmap_get_cpu_map(&dist->irq_state, vcpu_id);
> for_each_set_bit(i, vgic_cpu->pending_percpu, 16) {
> unsigned long sources;
>
> @@ -798,7 +796,7 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu)
> }
>
> if (!sources) {
> - clear_bit(i, pending);
> + vgic_bitmap_set_irq_val(&dist->irq_state, vcpu_id, i, 0);
> kvm_vgic_vcpu_clear_pending_irq(vcpu, i);
> }
>
> @@ -812,13 +810,12 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu)
> continue;
> }
>
> - clear_bit(i, pending);
> + vgic_bitmap_set_irq_val(&dist->irq_state, vcpu_id, i, 0);
> kvm_vgic_vcpu_clear_pending_irq(vcpu, i);
> }
>
>
> /* SPIs */
> - pending = vgic_bitmap_get_shared_map(&dist->irq_state);
> for_each_set_bit(i, vgic_cpu->pending_shared, VGIC_NR_SHARED_IRQS) {
> int irq = i + 32;
> if (vgic_bitmap_get_irq_val(&dist->irq_active, 0, irq))
> @@ -831,7 +828,7 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu)
>
> /* Immediate clear on edge, set active on level */
> if (vgic_irq_is_edge(dist, irq)) {
> - clear_bit(i, pending);
> + vgic_bitmap_set_irq_val(&dist->irq_state, 0, irq, 0);
> kvm_vgic_vcpu_clear_pending_irq(vcpu, irq);
> } else {
> vgic_bitmap_set_irq_val(&dist->irq_active, 0, irq, 1);
definitely an improvement, but that function is really hard to read,
so we could probably improve even further by introducing some small
static functions like
static void vgic_set_irq_active(struct kvm_vcpu *vcpu, int irq);
static void vgic_clear_irq_active(struct kvm_vcpu *vcpu, int irq);
or something like that?
I applied this one for now, thanks.
-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
