On Wed, Nov 30, 2022 at 05:08:06PM -0800, Stefano Stabellini wrote: > On Wed, 30 Nov 2022, Roger Pau Monne wrote: > > The hvc machinery registers both a console and a tty device based on > > the hv ops provided by the specific implementation. Those two > > interfaces however have different locks, and there's no single locks > > that's shared between the tty and the console implementations, hence > > the driver needs to protect itself against concurrent accesses. > > Otherwise concurrent calls using the split interfaces are likely to > > corrupt the ring indexes, leaving the console unusable. > > > > Introduce a lock to xencons_info to serialize accesses to the shared > > ring. This is only required when using the shared memory console, > > concurrent accesses to the hypercall based console implementation are > > not an issue. > > > > Note the conditional logic in domU_read_console() is slightly modified > > so the notify_daemon() call can be done outside of the locked region: > > it's an hypercall and there's no need for it to be done with the lock > > held. > > For domU_read_console: I don't mean to block this patch but we need to > be sure about the semantics of hv_ops.get_chars. Either it is expected > to be already locked, then we definitely shouldn't add another lock to > domU_read_console. Or it is not expected to be already locked, then we > should add the lock. > > My impression is that it is expected to be already locked, but I think > we need Greg or Jiri to confirm one way or the other.
Let me move both to the 'To:' field then. My main concern is the usage of hv_ops.get_chars hook in hvc_poll_get_char(), as it's not obvious to me that callers of tty->poll_get_char hook as returned by tty_find_polling_driver() will always do so with the tty lock held (in fact the only user right now doesn't seem to hold the tty lock). > Aside from that the rest looks fine. Thanks for the review, Roger.
