| Message ID | 20170320105818.20481-10-cdall@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 20/03/17 10:58, Christoffer Dall wrote: > There is no need to call any functions to fold LRs when we don't use any > LRs and we don't need to mess with overflow flags, take spinlocks, or > prune the AP list if the AP list is empty. > > Note: list_empty is a single atomic read (uses READ_ONCE) and can > therefore check if a list is empty or not without the need to take the > spinlock protecting the list. > > Signed-off-by: Christoffer Dall <cdall@linaro.org> > --- > virt/kvm/arm/vgic/vgic.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c > index 093873e..8ecb009 100644 > --- a/virt/kvm/arm/vgic/vgic.c > +++ b/virt/kvm/arm/vgic/vgic.c > @@ -639,15 +639,18 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu) > { > struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; > > - if (unlikely(!vgic_initialized(vcpu->kvm))) Could this be folded with the previous patch? It is the same optimisation. > + /* An empty ap_list_head implies used_lrs == 0 */ > + if (list_empty(&vcpu->arch.vgic_cpu.ap_list_head)) > return; > > vgic_clear_uie(vcpu); > - vgic_fold_lr_state(vcpu); > - vgic_prune_ap_list(vcpu); > > - /* Make sure we can fast-path in flush_hwstate */ > - vgic_cpu->used_lrs = 0; > + if (vgic_cpu->used_lrs) { > + vgic_fold_lr_state(vcpu); > + vgic_cpu->used_lrs = 0; This zeroing could also be moved to vgic_fold_lr_state(), though I don't feel strongly about it. > + } > + > + vgic_prune_ap_list(vcpu); > } > > /* Flush our emulation state into the GIC hardware before entering the guest. */ > Otherwise looks good to me. M.
On Tue, Mar 21, 2017 at 01:29:06PM +0000, Marc Zyngier wrote: > On 20/03/17 10:58, Christoffer Dall wrote: > > There is no need to call any functions to fold LRs when we don't use any > > LRs and we don't need to mess with overflow flags, take spinlocks, or > > prune the AP list if the AP list is empty. > > > > Note: list_empty is a single atomic read (uses READ_ONCE) and can > > therefore check if a list is empty or not without the need to take the > > spinlock protecting the list. > > > > Signed-off-by: Christoffer Dall <cdall@linaro.org> > > --- > > virt/kvm/arm/vgic/vgic.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c > > index 093873e..8ecb009 100644 > > --- a/virt/kvm/arm/vgic/vgic.c > > +++ b/virt/kvm/arm/vgic/vgic.c > > @@ -639,15 +639,18 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu) > > { > > struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; > > > > - if (unlikely(!vgic_initialized(vcpu->kvm))) > > Could this be folded with the previous patch? It is the same optimisation. > Sure. > > + /* An empty ap_list_head implies used_lrs == 0 */ > > + if (list_empty(&vcpu->arch.vgic_cpu.ap_list_head)) > > return; > > > > vgic_clear_uie(vcpu); > > - vgic_fold_lr_state(vcpu); > > - vgic_prune_ap_list(vcpu); > > > > - /* Make sure we can fast-path in flush_hwstate */ > > - vgic_cpu->used_lrs = 0; > > + if (vgic_cpu->used_lrs) { > > + vgic_fold_lr_state(vcpu); > > + vgic_cpu->used_lrs = 0; > > This zeroing could also be moved to vgic_fold_lr_state(), though I don't > feel strongly about it. > Hmm, I'll have a look. > > + } > > + > > + vgic_prune_ap_list(vcpu); > > } > > > > /* Flush our emulation state into the GIC hardware before entering the guest. */ > > > > Otherwise looks good to me. > Thanks, -Christoffer
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c index 093873e..8ecb009 100644 --- a/virt/kvm/arm/vgic/vgic.c +++ b/virt/kvm/arm/vgic/vgic.c @@ -639,15 +639,18 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu) { struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; - if (unlikely(!vgic_initialized(vcpu->kvm))) + /* An empty ap_list_head implies used_lrs == 0 */ + if (list_empty(&vcpu->arch.vgic_cpu.ap_list_head)) return; vgic_clear_uie(vcpu); - vgic_fold_lr_state(vcpu); - vgic_prune_ap_list(vcpu); - /* Make sure we can fast-path in flush_hwstate */ - vgic_cpu->used_lrs = 0; + if (vgic_cpu->used_lrs) { + vgic_fold_lr_state(vcpu); + vgic_cpu->used_lrs = 0; + } + + vgic_prune_ap_list(vcpu); } /* Flush our emulation state into the GIC hardware before entering the guest. */
There is no need to call any functions to fold LRs when we don't use any LRs and we don't need to mess with overflow flags, take spinlocks, or prune the AP list if the AP list is empty. Note: list_empty is a single atomic read (uses READ_ONCE) and can therefore check if a list is empty or not without the need to take the spinlock protecting the list. Signed-off-by: Christoffer Dall <cdall@linaro.org> --- virt/kvm/arm/vgic/vgic.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)