Re: [PATCH v4 16/17] x86/hvm: always re-emulate I/O from a buffer

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 25 June 2015 11:50
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel@lists.xenproject.org; Keir (Xen.org)
> Subject: RE: [PATCH v4 16/17] x86/hvm: always re-emulate I/O from a buffer
> 
> >>> On 25.06.15 at 12:32, <Paul.Durrant@citrix.com> wrote:
> >>  -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: 25 June 2015 10:58
> >> To: Paul Durrant
> >> Cc: Andrew Cooper; xen-devel@lists.xenproject.org; Keir (Xen.org)
> >> Subject: Re: [PATCH v4 16/17] x86/hvm: always re-emulate I/O from a
> buffer
> >>
> >> >>> On 24.06.15 at 13:24, <paul.durrant@citrix.com> wrote:
> >> > If memory mapped I/O is 'chunked' then the I/O must be re-emulated,
> >> > otherwise only the first chunk will be processed. This patch makes
> >> > sure all I/O from a buffer is re-emulated regardless of whether it
> >> > is a read or a write.
> >>
> >> I'm not sure I understand this: Isn't the reason for treating reads
> >> and writes differently due to the fact that MMIO reads may have
> >> side effects, and hence can't be re-issued (whereas writes are
> >> always the last thing an instruction does, and hence can't hold up
> >> retiring of it, and hence don't need retrying)?
> >
> > Read were always re-issued, which is why handle_mmio() is called in
> > hvm_io_assit(). If the underlying MMIO is deferred to QEMU then this is
> the
> > only way for Xen to pick up the result. This patch adds completion for
> > writes.
> > If the I/O has been broken down in the underlying hvmemul_write() and a
> > 'chunk' deferred to QEMU then there is actually need to re-emulate
> otherwise
> > any remaining chunks will not be handled.
> >
> >>
> >> Furthermore, doesn't "only the first chunk" get represented correctly
> >> already by informing the caller that only a single iteration of a
> >> repeated instruction was done, such that further repeats will get
> >> carried out anyway (resulting in another, fresh cycle through the
> >> emulator)?
> >>
> >
> > No, because we're talking about 'chunks' here and not 'reps'. If a single
> > non-rep I/O is broken down into, say, two chunks then we:
> >
> > - Issue the I/O for the first chunk to QEMU
> > - Say we did nothing by returning RETRY
> > - Re-issue the emulation from hvm_io_assist()
> > - Pick up the result of the first chunk from the ioreq, add it to the cache,
> > and issue the second chunk to QEMU
> > - Say we did nothing by returning RETRY
> > - Re-issue the emulation from hvm_io_assist()
> > - Pick up the result of the first chunk from the cache and pick up the result
> > of the second chunk from the ioreq
> > - Say we completed the I/O by returning OKAY
> >
> > I agree it's not nice, and bouncing would have been preferable, but that's
> > the way 'wide I/O' works.
> 
> I see. Which means
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 

Thanks.

  Paul

> Jan