On 10/10/2010 09:29 AM, Gleb Natapov wrote:
quoted text > On Sat, Oct 09, 2010 at 08:30:18PM +0200, Avi Kivity wrote:
> > On 10/07/2010 07:47 PM, Gleb Natapov wrote:
> > >On Thu, Oct 07, 2010 at 11:50:08AM +0200, Avi Kivity wrote:
> > >> On 10/04/2010 05:56 PM, Gleb Natapov wrote:
> > >> >If a guest accesses swapped out memory do not swap it in from vcpu thread
> > >> >context. Schedule work to do swapping and put vcpu into halted state
> > >> >instead.
> > >> >
> > >> >Interrupts will still be delivered to the guest and if interrupt will
> > >> >cause reschedule guest will continue to run another task.
> > >> >
> > >> >
> > >> >+
> > >> >+static bool can_do_async_pf(struct kvm_vcpu *vcpu)
> > >> >+{
> > >> >+ if (unlikely(!irqchip_in_kernel(vcpu->kvm) ||
> > >> >+ kvm_event_needs_reinjection(vcpu)))
> > >> >+ return false;
> > >> >+
> > >> >+ return kvm_x86_ops->interrupt_allowed(vcpu);
> > >> >+}
> > >>
> > >> Strictly speaking, if the cpu can handle NMIs it can take an apf?
> > >>
> > >We can always do apf, but if vcpu can't do anything hwy bother. For NMI
> > >watchdog yes, may be it is worth to allow apf if nmi is allowed.
> >
> > Actually it's very dangerous - the IRET from APF will re-enable
> > NMIs. So without the guest enabling apf-in-nmi we shouldn't allow
> > it.
> >
> Good point.
>
> > Not worth the complexity IMO.
> >
> > >> >@@ -5112,6 +5122,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > >> > if (unlikely(r))
> > >> > goto out;
> > >> >
> > >> >+ kvm_check_async_pf_completion(vcpu);
> > >> >+ if (vcpu->arch.mp_state == KVM_MP_STATE_HALTED) {
> > >> >+ /* Page is swapped out. Do synthetic halt */
> > >> >+ r = 1;
> > >> >+ goto out;
> > >> >+ }
> > >> >+
> > >>
> > >> Why do it here in the fast path? Can't you halt the cpu when
> > >> starting the page fault?
> > >Page fault may complete before guest re-entry. We do not want to halt vcpu
> > >in this case.
> >
> > So unhalt on completion.
> >
> I want to avoid touching vcpu state from work if possible. Work code does
> not contain arch dependent code right now and mp_state is x86 thing
>
Use a KVM_REQ.
quoted text > > >>
> > >> I guess the apf threads can't touch mp_state, but they can have a
> > >> KVM_REQ to trigger the check.
> > >This will require KVM_REQ check on fast path, so what's the difference
> > >performance wise.
> >
> > We already have a KVM_REQ check (if (vcpu->requests)) so it doesn't
> > cost anything extra.
> if (vcpu->requests) does not clear req bit, so what will have to be added
> is: if (kvm_check_request(KVM_REQ_APF_HLT, vcpu)) which is even more
> expensive then my check (but not so expensive to worry about).
It's only expensive when it happens. Most entries will have the bit clear.
quoted text > >
> > >> >
> > >> >@@ -6040,6 +6064,7 @@ void kvm_arch_flush_shadow(struct kvm *kvm)
> > >> > int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
> > >> > {
> > >> > return vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE
> > >> >+ || !list_empty_careful(&vcpu->async_pf.done)
> > >> > || vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED
> > >> > || vcpu->arch.nmi_pending ||
> > >> > (kvm_arch_interrupt_allowed(vcpu)&&
> > >>
> > >> Unrelated, shouldn't kvm_arch_vcpu_runnable() look at
> > >> vcpu->requests? Specifically KVM_REQ_EVENT?
> > >I think KVM_REQ_EVENT is covered by checking nmi and interrupt queue
> > >here.
> >
> > No, the nmi and interrupt queues are only filled when the lapic is
> > polled via KVM_REQ_EVENT. I'll prepare a patch.
> I don't think you are correct. nmi_pending is filled before setting
> KVM_REQ_EVENT and kvm_cpu_has_interrupt() checks directly in apic/pic.
Right.
quoted text > >
> > >> >+
> > >> >+TRACE_EVENT(
> > >> >+ kvm_async_pf_not_present,
> > >> >+ TP_PROTO(u64 gva),
> > >> >+ TP_ARGS(gva),
> > >>
> > >> Do you actually have a gva with tdp? With nested virtualization,
> > >> how do you interpret this gva?
> > >With tdp it is gpa just like tdp_page_fault gets gpa where shadow page
> > >version gets gva. Nested virtualization is too complex to interpret.
> >
> > It's not good to have a tracepoint that depends on cpu mode (without
> > recording that mode). I think we have the same issue in
> > trace_kvm_page_fault though.
> We have mmu_is_nested(). I'll just disable apf while vcpu is in nested
> mode for now.
What if we get the apf in non-nested mode and it completes in nested mode?
quoted text > >
> > >> >+
> > >> >+ /* do alloc nowait since if we are going to sleep anyway we
> > >> >+ may as well sleep faulting in page */
> > >> /*
> > >> * multi
> > >> * line
> > >> * comment
> > >> */
> > >>
> > >> (but a good one, this is subtle)
> > >>
> > >> I missed where you halt the vcpu. Can you point me at the function?
> > >>
> > >> Note this is a synthetic halt and must not be visible to live
> > >> migration, or we risk live migrating a halted state which doesn't
> > >> really exist.
> > >>
> > >> Might be simplest to drain the apf queue on any of the save/restore ioctls.
> > >>
> > >So that "info cpu" will interfere with apf? Migration should work
> > >in regular way. apf state should not be migrated since it has no meaning
> > >on the destination. I'll make sure synthetic halt state will not
> > >interfere with migration.
> >
> > If you deliver an apf, the guest expects a completion.
> >
> There is special completion that tells guest to wake all sleeping tasks
> on vcpu. It is delivered after migration on the destination.
>
Yes, I saw.
What if you can't deliver it? is it possible that some other vcpu will
start receiving apfs that alias the old ones? Or is the broadcast global?
--
error compiling committee.c: too many arguments to function
--
unsubscribe notice To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
majordomo@vger.kernel.org
More majordomo info at
http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
http://www.tux.org/lkml/ Messages in current thread:
Re: [PATCH v6 02/12] Halt vcpu if page it tries to access ... , Avi Kivity , (Sun Oct 10, 8:55 am)
