Re: [patch 1/3] KVM: x86: add ioctls to get/set PIO state

[Avi Kivity <avi@redhat.com>]

On 02/09/2010 12:41 AM, Marcelo Tosatti wrote:
> On Thu, Feb 04, 2010 at 11:46:25PM +0200, Avi Kivity wrote:
>    
>> On 02/04/2010 11:36 PM, Marcelo Tosatti wrote:
>>      
>>> On Thu, Feb 04, 2010 at 09:16:47PM +0200, Avi Kivity wrote:
>>>        
>>>> On 01/28/2010 09:03 PM, Marcelo Tosatti wrote:
>>>>          
>>>>> A vcpu can be stopped after handling IO in userspace,
>>>>> but before returning to kernel to finish processing.
>>>>>
>>>>>            
>>>> Is this strictly needed?  If we teach qemu to migrate before
>>>> executing the pio request, I think we'll be all right?  should work
>>>> at least for IN/INS, not sure about OUT/OUTS.
>>>>          
>>> It would be nice (instead of more state to keep track of between
>>> kernel/user) but the drawbacks i see are:
>>>
>>> You'd have to add a limitation so that any IN which was processed
>>> by device emulation has to re-entry kernel to complete it (so it
>>> complicates vcpu stop in userspace).
>>>
>>>        
>> You could fix that by moving the IN emulation to before guest entry.
>> It complicates the vcpu loop a bit, but is backwards compatible and
>> all that.
>>      
> Under such scheme, to avoid a stream of IN's from temporarily blocking
> vcpu stop capability, you'd have to requeue a signal to stop the vcpu
> (so the next IN in the stream is not executed, but complete_pio does).
>
> Or not process the stop signal in the first place (new state for main
> loop, "pending pio/mmio").
>    

Why?  you would handle stops exactly the same way:

vcpu_loop:
      while running:
          process_last_in()
          run_vcpu()
          handle_exit_except_in()

An IN that is stopped would simply be unprocessed, and the next entry, 
if at a new host, will simply re-execute it.

> Or even just copy the result from QEMU device to RAX in userspace, which
> is somewhat nasty since you'd have either userspace or kernel finishing
> the op.
>    

Definitely bad.

> For REP OUTS larger than page size, the current position is held in RCX,
> but complete_pio uses vcpu->arch.pio.cur_count and count to hold the
> position. So you either make it possible to writeback vcpu->arch.pio
> to the kernel, or wait for the operation to finish (with similar
> complications regarding signal processing).
>    

RCX is always consistent, no?  So if we migrate in the middle of REP 
OUTS, the operation will restart at the correct place?

> As i see it, the benefit of backward compatibility is not worthwhile
> compared to the complications introduced to vcpu loop processing (and
> potential for damaging vcpu stop ->  vcpu stopped latency).
>
> Are you certain its worth avoiding the restore ioctl for pio/mmio?
>    

First, let's see if it's feasible or not.  If it's feasible, it's 
probably just a matter of rearranging things to get userspace sane.  A 
small price to pay for backward compatibility.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.