lkml.org 
[lkml]   [2022]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] hvc/xen: prevent concurrent accesses to the shared ring
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.

\
 
 \ /
  Last update: 2022-12-02 12:40    [W:2.093 / U:0.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site