| Message ID | 1680766393-46220-1-git-send-email-lirongqing@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] x86/kvm: Don't check vCPU preempted if vCPU has dedicated pCPU and non-trap HLT | expand |
On Thu, Apr 06, 2023 at 03:33:13PM +0800, lirongqing@baidu.com wrote: > From: Li RongQing <lirongqing@baidu.com> > > Check whether vCPU is preempted or not only when HLT is trapped or > there is not realtime hint. In other words, it is unnecessary to check > preemption when vCPU has realtime hint (which means vCPU has dedicated > pCP) and has not PV_UNHALT (which means unintercepted HLT), because > vCPU should not to be marked as preempted in this setup. To what benefit? > Signed-off-by: Li RongQing <lirongqing@baidu.com> > --- > diff with v1: rewrite changelog and indentation > > arch/x86/kernel/kvm.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c > index 1cceac5..25398d2 100644 > --- a/arch/x86/kernel/kvm.c > +++ b/arch/x86/kernel/kvm.c > @@ -820,8 +820,10 @@ static void __init kvm_guest_init(void) > has_steal_clock = 1; > static_call_update(pv_steal_clock, kvm_steal_clock); > > - pv_ops.lock.vcpu_is_preempted = > - PV_CALLEE_SAVE(__kvm_vcpu_is_preempted); > + if (kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) || > + !kvm_para_has_hint(KVM_HINTS_REALTIME)) This is atrocious coding style, please align on the (. > + pv_ops.lock.vcpu_is_preempted = > + PV_CALLEE_SAVE(__kvm_vcpu_is_preempted); > } > > if (kvm_para_has_feature(KVM_FEATURE_PV_EOI)) > -- > 2.9.4 >
On Thu, Apr 06, 2023, Peter Zijlstra wrote: > On Thu, Apr 06, 2023 at 03:33:13PM +0800, lirongqing@baidu.com wrote: > > From: Li RongQing <lirongqing@baidu.com> > > > > Check whether vCPU is preempted or not only when HLT is trapped or > > there is not realtime hint. In other words, it is unnecessary to check > > preemption when vCPU has realtime hint (which means vCPU has dedicated > > pCP) and has not PV_UNHALT (which means unintercepted HLT), because > > vCPU should not to be marked as preempted in this setup. > > To what benefit? > > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > > --- > > diff with v1: rewrite changelog and indentation This also fails to mention my objection to querying PV_UNHALT[*]. When I said "this needs Paolo's attention no matter what", I did not mean "post a v2 and hope Paolo applies it", I meant we need Paolo (and others) to weigh in on the ongoing discussion. [*] https://lore.kernel.org/all/ZBEOK6ws9wGqof3O@google.com
On 4/6/23 18:08, Sean Christopherson wrote: >> Signed-off-by: Li RongQing<lirongqing@baidu.com> >> --- >> diff with v1: rewrite changelog and indentation > > This also fails to mention my objection to querying PV_UNHALT[*]. When I said > "this needs Paolo's attention no matter what", I did not mean "post a v2 and hope > Paolo applies it", I meant we need Paolo (and others) to weigh in on the ongoing > discussion. Quoting v1: >> + if (kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) || > > Rather than have the guest rely on host KVM behavior clearing PV_UNHALT when HLT > is passed through), would it make sense to add something like KVM_HINTS_HLT_PASSTHROUGH > to more explicitly tell the guest that HLT isn't intercepted? Yes, I agree with adding KVM_HINTS_HLT_PASSTHROUGH or KVM_HINTS_GUEST_CSTATE (i.e. host can remain in guest mode even when running in C1 aka hlt or possibly deeper states). Lack of PV_UNHALT does not indicate anything about whether HLT will be handled in host or guest. In fact the same KVM_HINTS_* value could also be used to disable PV_UNHALT. Paolo
diff with v1: rewrite changelog and indentation arch/x86/kernel/kvm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c index 1cceac5..25398d2 100644 --- a/arch/x86/kernel/kvm.c +++ b/arch/x86/kernel/kvm.c @@ -820,8 +820,10 @@ static void __init kvm_guest_init(void) has_steal_clock = 1; static_call_update(pv_steal_clock, kvm_steal_clock); - pv_ops.lock.vcpu_is_preempted = - PV_CALLEE_SAVE(__kvm_vcpu_is_preempted); + if (kvm_para_has_feature(KVM_FEATURE_PV_UNHALT) || + !kvm_para_has_hint(KVM_HINTS_REALTIME)) + pv_ops.lock.vcpu_is_preempted = + PV_CALLEE_SAVE(__kvm_vcpu_is_preempted); } if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))