[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 13/17] x86/hvm: remove HVMIO_dispatched I/O state
> -----Original Message----- > From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel- > bounces@xxxxxxxxxxxxx] On Behalf Of Jan Beulich > Sent: 24 June 2015 17:12 > To: Paul Durrant > Cc: Andrew Cooper; Keir (Xen.org); xen-devel@xxxxxxxxxxxxxxxxxxxx > Subject: Re: [Xen-devel] [PATCH v4 13/17] x86/hvm: remove > HVMIO_dispatched I/O state > > >>> On 24.06.15 at 18:00, <Paul.Durrant@xxxxxxxxxx> wrote: > >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > >> Sent: 24 June 2015 16:53 > >> >>> On 24.06.15 at 13:24, <paul.durrant@xxxxxxxxxx> wrote: > >> > --- a/xen/arch/x86/hvm/emulate.c > >> > +++ b/xen/arch/x86/hvm/emulate.c > >> > @@ -138,20 +138,14 @@ static int hvmemul_do_io( > >> > if ( data_is_addr || dir == IOREQ_WRITE ) > >> > return X86EMUL_UNHANDLEABLE; > >> > goto finish_access; > >> > - case HVMIO_dispatched: > >> > - /* May have to wait for previous cycle of a multi-write to > >> > complete. > */ > >> > - if ( is_mmio && !data_is_addr && (dir == IOREQ_WRITE) && > >> > - (addr == (vio->mmio_large_write_pa + > >> > - vio->mmio_large_write_bytes)) ) > >> > - return X86EMUL_RETRY; > >> > - /* fallthrough */ > >> > >> The description is plausible, and the rest of the patch looks plausible > >> too, but I can't seem to follow why the above can go away without > >> any replacement (or what the replacement would be). Could you > >> clarify that, please? > >> > > > > The reason is that the dispatched state was only ever used for writes (or > > data_is_addr, which is disallowed in this case), and the code below this > > forces writes to be completed with X86EMUL_OKAY meaning that > emulation will > > never be retried (because it's not retried directly from hvm_io_assist() > > unless state was HVMIO_mmio_awaiting_completion). Thus I believe this > was > > either always dead code, or it's been dead for a long time. > > Looks like you're right, albeit I can't see how what the comment says > would be achieved elsewhere. > > > Do you want me to stick words to that effect in the comment? > > That would be helpful I think, namely in case we later learn there > was some use for that code despite it looking to be dead. (Did you > btw do some basic verification that it's dead, by sticking a printk() > or ASSERT() in there?) > While I was testing I stuck a BUG_ON() in there to prove to myself it really really was never hit. I never managed to provoke it (using a variety of Windows OS and both QEMUs). Paul > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |