Re: [Xen-devel] [PATCH] PCI uart: fix boot hang, and second S3 resume inactive timer list corruption

[Tomasz Wroblewski <tomasz.wroblewski@xxxxxxxxxx>]

On 08/26/2013 05:26 PM, Jan Beulich wrote:
> > > > On 26.08.13 at 17:09, Tomasz Wroblewski<tomasz.wroblewski@xxxxxxxxxx>  wrote:
> > On 08/26/2013 03:52 PM, Jan Beulich wrote:
> > > > > > On 26.08.13 at 15:25, Tomasz Wroblewski<tomasz.wroblewski@xxxxxxxxxx>   wrote:
> > > > > Nevertheless, the approach of your patch in simply giving up
> > > > > the device (even if only termporarily) looks questionable to me
> > > > > We'd rather need to restore full access to it I would think. But
> > > > > yes, this hypervisor and Dom0 playing with the same device is
> > > > > sort of a gray area.
> > > > Restore ioport access at the start of poll routine (if not on) and
> > > > disable it again at the end (if was not on)? I might do that (if you
> > > > really prefer), but intuitively that seems more likely to produce side
> > > > effects in dom0 kernel than skipping a poll in xen
> > > As long as it's guaranteed to only be a poll (or a few of them) being
> > > affected, this is fine. But what if an interrupt is being used?
> > I'm probably missing something so can you elaborate on this? Probably
> > not what you are asking, but ns16550_interrupt function currently
> > doesn't hang when ioports are disabled as a byproduct of the     "while
> > ( !(ns_read_reg(uart, IIR)&  IIR_NOINT) )" test in there, which already
> > causes it to break out on 0xFF regs
> My question was along the lines of "If I/O port access is disabled,
> isn't the whole driver screwed (even if only temporarily)?" And if
> the answer to this is "yes" (I can't see it to be "no"), dealing with
> this likely requires more than the change you proposed.
It could be, I only have empirical evidence of not noticing any serial 
out hiccups during dom0 kernel init. Since this is is small driver and 
it seems to primarily interact with the I/O only in ns16550_interrupt, 
ns16550_poll, ns16550_tx_ready, putc, getc (and in some init functions 
but these will only be called before dom0 boot), I thought that:
* ns16550_interrupt will be fine with IO ports disabled, it'll just exit
* ns16550_poll will be fine with the posted patch, it'll exit
* ns16550_getc looks like it has a potential of producing 0xFF 
characters incorrectly, so maybe would need a port test as well
* ns16550_putc should be fine since write to ioport will just be dropped
* ns16550_tx_ready should be fine, it will return 1 if ioports are 
disabled which is what it needs to be returning to avoid spinning in 
serial.c
So besides possibly the extra check in getc, not really sure what else 
can be done better here
> Jan

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