On Tue, Sep 11, 2012 at 12:41:28PM +0300, Avi Kivity wrote:
> On 09/11/2012 12:35 PM, Gleb Natapov wrote:
> > On Tue, Sep 11, 2012 at 12:29:06PM +0300, Avi Kivity wrote:
> >> On 09/10/2012 08:05 PM, Gleb Natapov wrote:
> >> > On Mon, Sep 10, 2012 at 07:17:54PM +0300, Gleb Natapov wrote:
> >> >> > > + return 0;
> >> >> > > +}
> >> >> > > +
> >> >> > > +static inline int kvm_apic_set_id(struct kvm_lapic *apic, u8 id)
> >> >> > > +{
> >> >> > > + apic_set_reg(apic, APIC_ID, id << 24);
> >> >> > > + return recalculate_apic_map(apic->vcpu->kvm);
> >> >> > > +}
> >> >> > > +
> >> >> > > +static inline int kvm_apic_set_ldr(struct kvm_lapic *apic, u32 id)
> >> >> > > +{
> >> >> > > + apic_set_reg(apic, APIC_LDR, id);
> >> >> > > + return recalculate_apic_map(apic->vcpu->kvm);
> >> >> > > +}
> >> >> > > +
> >> >> >
> >> >> > return value of these functions seems never checked.
> >> >> >
> >> >> Yes, the problem is that we can do nothing about the failure if failure
> >> >> happens during guest write.
> >>
> >> We can. Return -ENOMEM all the way up to userspace.
> >>
> > There is no userspace to return error to if error happens on guest MMIO
> > write. Unless you mean return it as a return value of ioctl(VM_RUN) in
> > which case it is equivalent of killing the guest.
>
> That is what I meant.
>
> > And this is not fair
> > to a guest who did nothing wrong to suffer from our stupid optimizations :)
> > Actually I am not sure that returning to userspace in the middle of an
> > IO that is handled by a kernel is well defined in KVM ABI.
>
> If you get -ENOMEM when allocating a page without GFP_ATOMIC (or
> GFP_NOIO etc) then the entire host is dead anyway. The same thing can
> happen if the guest (or userspace) touches a yet-unallocated page, or if
> the page fault path fails to allocate mmu pages, or any of a thousand
> other allocations we have all over.
Then it is just simpler to sigkill the guest right away. What's the
point in returning error if you believe that userspace can't handle it
and will likely not run long enough to even get to userspace due to
memory shortage.
>
> >
> >> >>
> >> > Actually I have an idea how to handle the error. Never return one. If
> >> > map cannot be allocated go slow path always. phys_map should be checked
> >> > for NULL during delivery in this case obviously.
> >>
> >> That's better of course (though we have to beware of such tricks, but in
> >> this case the slow path is regularly exercised so it should keep working).
> >>
> > Oh with Windows guests it has work to do for sure.
>
> This reminds me, we could speed up self-ipi for that.
>
The patch does it. Windows sends a lot of all but self IPIs too.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
