| Message ID | 20191001005408.129099-1-liran.alon@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: VMX: Refactor to not compare set PI control bits directly to 1 | expand |
On Mon, Sep 30, 2019 at 5:54 PM Liran Alon <liran.alon@oracle.com> wrote: > > This is a pure code refactoring. > No semantic change is expected. > > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Signed-off-by: Liran Alon <liran.alon@oracle.com> Reviewed-by: Jim Mattson <jmattson@google.com>
On 01/10/19 02:54, Liran Alon wrote: > This is a pure code refactoring. > No semantic change is expected. > > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Signed-off-by: Liran Alon <liran.alon@oracle.com> > --- > arch/x86/kvm/vmx/vmx.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) I'm not sure this is an improvement, for two reasons: 1) the "dy" in vmx_dy_apicv_has_pending_interrupt callback is meant for directed yield, so it's a bit ugly to call it from wakeup_handler. 2) the wakeup_handler is an interrupt handler specific to posted interrupts, so it makes sense to check the posted interrupts descriptor. I wouldn't say ON is a control bit in fact (unlike for example SN or NV), since it is written by the processor or IOMMU rather than the hypervisor. Paolo > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index e31317fc8c95..92eb4910fe9f 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -5302,6 +5302,11 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu) > } > } > > +static bool vmx_dy_apicv_has_pending_interrupt(struct kvm_vcpu *vcpu) > +{ > + return pi_test_on(vcpu_to_pi_desc(vcpu)); > +} > + > /* > * Handler for POSTED_INTERRUPT_WAKEUP_VECTOR. > */ > @@ -5313,9 +5318,7 @@ static void wakeup_handler(void) > spin_lock(&per_cpu(blocked_vcpu_on_cpu_lock, cpu)); > list_for_each_entry(vcpu, &per_cpu(blocked_vcpu_on_cpu, cpu), > blocked_vcpu_list) { > - struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); > - > - if (pi_test_on(pi_desc) == 1) > + if (vmx_dy_apicv_has_pending_interrupt(vcpu)) > kvm_vcpu_kick(vcpu); > } > spin_unlock(&per_cpu(blocked_vcpu_on_cpu_lock, cpu)); > @@ -6168,11 +6171,6 @@ static int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu) > return max_irr; > } > > -static bool vmx_dy_apicv_has_pending_interrupt(struct kvm_vcpu *vcpu) > -{ > - return pi_test_on(vcpu_to_pi_desc(vcpu)); > -} > - > static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap) > { > if (!kvm_vcpu_apicv_active(vcpu)) > @@ -7336,7 +7334,7 @@ static int pi_pre_block(struct kvm_vcpu *vcpu) > do { > old.control = new.control = pi_desc->control; > > - WARN((pi_desc->sn == 1), > + WARN(pi_desc->sn, > "Warning: SN field of posted-interrupts " > "is set before blocking\n"); > > @@ -7361,7 +7359,7 @@ static int pi_pre_block(struct kvm_vcpu *vcpu) > new.control) != old.control); > > /* We should not block the vCPU if an interrupt is posted for it. */ > - if (pi_test_on(pi_desc) == 1) > + if (pi_test_on(pi_desc)) > __pi_post_block(vcpu); > > local_irq_enable(); >
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index e31317fc8c95..92eb4910fe9f 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5302,6 +5302,11 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu) } } +static bool vmx_dy_apicv_has_pending_interrupt(struct kvm_vcpu *vcpu) +{ + return pi_test_on(vcpu_to_pi_desc(vcpu)); +} + /* * Handler for POSTED_INTERRUPT_WAKEUP_VECTOR. */ @@ -5313,9 +5318,7 @@ static void wakeup_handler(void) spin_lock(&per_cpu(blocked_vcpu_on_cpu_lock, cpu)); list_for_each_entry(vcpu, &per_cpu(blocked_vcpu_on_cpu, cpu), blocked_vcpu_list) { - struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); - - if (pi_test_on(pi_desc) == 1) + if (vmx_dy_apicv_has_pending_interrupt(vcpu)) kvm_vcpu_kick(vcpu); } spin_unlock(&per_cpu(blocked_vcpu_on_cpu_lock, cpu)); @@ -6168,11 +6171,6 @@ static int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu) return max_irr; } -static bool vmx_dy_apicv_has_pending_interrupt(struct kvm_vcpu *vcpu) -{ - return pi_test_on(vcpu_to_pi_desc(vcpu)); -} - static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap) { if (!kvm_vcpu_apicv_active(vcpu)) @@ -7336,7 +7334,7 @@ static int pi_pre_block(struct kvm_vcpu *vcpu) do { old.control = new.control = pi_desc->control; - WARN((pi_desc->sn == 1), + WARN(pi_desc->sn, "Warning: SN field of posted-interrupts " "is set before blocking\n"); @@ -7361,7 +7359,7 @@ static int pi_pre_block(struct kvm_vcpu *vcpu) new.control) != old.control); /* We should not block the vCPU if an interrupt is posted for it. */ - if (pi_test_on(pi_desc) == 1) + if (pi_test_on(pi_desc)) __pi_post_block(vcpu); local_irq_enable();