[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] x86/vMSI-X emulation issue


  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
  • Date: Thu, 24 Mar 2016 09:53:57 +0000
  • Accept-language: en-GB, en-US
  • Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 24 Mar 2016 10:00:57 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>
  • Thread-index: AQHRhSswWjeEL9v6CUa53Q6giYkK759oKNuAgAAl6lD///cQAIAAESxQ///yAQCAABEoEA==
  • Thread-topic: [Xen-devel] x86/vMSI-X emulation issue

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 24 March 2016 09:47
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel
> Subject: RE: [Xen-devel] x86/vMSI-X emulation issue
> 
> >>> On 24.03.16 at 10:39, <Paul.Durrant@xxxxxxxxxx> wrote:
> >>  -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: 24 March 2016 09:35
> >> To: Paul Durrant
> >> Cc: Andrew Cooper; xen-devel
> >> Subject: RE: [Xen-devel] x86/vMSI-X emulation issue
> >>
> >> >>> On 24.03.16 at 10:09, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> >> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf
> Of
> >> Jan
> >> >> Beulich
> >> >> Sent: 24 March 2016 07:52
> >> >> > 2) Do aforementioned chopping automatically on seeing
> >> >> >     X86EMUL_UNHANDLEABLE, on the basis that the .check
> >> >> >     handler had indicated that the full range was acceptable. That
> >> >> >     would at once cover other similarly undesirable cases like the
> >> >> >     vLAPIC code returning this error. However, any stdvga like
> >> >> >     emulated device would clearly not want such to happen, and
> >> >> >     would instead prefer the entire batch to get forwarded in one
> >> >> >     go (stdvga itself sits on a different path). Otoh, with the
> >> >> >     devices we have currently, this would seem to be the least
> >> >> >     intrusive solution.
> >> >>
> >> >> Having thought about it more over night, I think this indeed is
> >> >> the most reasonable route, not just because it's least intrusive:
> >> >> For non-buffered internally handled I/O requests, no good can
> >> >> come from forwarding full batches to qemu, when the respective
> >> >> range checking function has indicated that this is an acceptable
> >> >> request. And in fact neither vHPET not vIO-APIC code generate
> >> >> X86EMUL_UNHANDLEABLE. And vLAPIC code doing so is also
> >> >> just apparently so - I'll submit a patch to make this obvious once
> >> >> tested.
> >> >>
> >> >> Otoh stdvga_intercept_pio() uses X86EMUL_UNHANDLEABLE in
> >> >> a manner similar to the vMSI-X code - for internal caching and
> >> >> then forwarding to qemu. Clearly that is also broken for
> >> >> REP OUTS, and hence a similar rep count reduction is going to
> >> >> be needed for the port I/O case.
> >> >
> >> > It suggests that such cache-and/or-forward models should probably sit
> >> > somewhere else in the flow, possibly being invoked from
> >> hvm_send_ioreq()
> >> > since there should indeed be a selected ioreq server for these cases.
> >>
> >> I don't really think so. As I have gone through and carried out
> >> what I had described above, I think I managed to address at
> >> least one more issue with not properly handled rep counts, and
> >> hence I think doing it that way is correct. I'll have to test the
> >> thing before I can send it out, for you to take a look.
> >>
> >
> > Ok. I never particularly liked using X86EMUL_UNHANDLEABLE to invoke the
> > forwarding behaviour though as it's only legitimate to do it on the first
> > rep.
> 
> Well, that's explicitly one of the wrong assumptions that patch
> addresses: It is perfectly fine for an individual handler to return
> this on other than the first iteration. It's only the generic
> infrastructure which doesn't currently permit this (for no
> apparent reason).
> 

Well, I guess the reason was that something returning X86EMUL_UNHANDLEABLE on a 
rep used to be used in the case of a page fault to bail out of the rep cycle, 
page in the memory and have it be restarted. I got rid of that in favour of 
pre-slicing the reps and making sure the memory was paged in before attempting 
the I/O. Thus there needed to be some special way of indicating an I/O that 
needed to be forwarded to QEMU vs. a page fault somewhere in the middle.

> > I always had the feeling there had to be a nicer way of doing it.
> > Possibly just too intrusive a change at this point though.
> 
> I'm of course up for alternatives, if you're willing to work on such.

I'll have a look at your code but, if I have the time I may look to re-factor 
things once 4.7 is out the door.

  Paul

> Yet I think backporting would become even more of a problem when
> going such an alternative route.
> 
> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.