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

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

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

Xen-devel mailing list



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