[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] hvc/xen: prevent concurrent accesses to the shared ring
- To: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>, Jiri Slaby <jirislaby@xxxxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Fri, 2 Dec 2022 12:40:05 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3/TzwSHGpP2p1Yz+lpbrnlAdsXzRUqR00WAaoANJDlk=; b=DdiGo/SYcaCoRG0Wgd6/9gl5CvrQIz5J5jAK0APUb+B+5uUvZXVjN3MbughFwo7HwO0H8x4TB2/DJ8q7X4XazxuX03xbKpMevyatr6UW1V661u56zZegfdkuKgc2+l3eUYIG23M9Vth/bHZBawKjQttTs0tEOqWiWqedMa7e+SrxhT5Iv8oHOY2SWpFIVZZFIpwMvBFsEBuQ4xeoo4ERJbKYD3OLOEj9z5EJBVF6v0PzlDamWOaFLa23aaX39A9G6N6DADLu899oJYML9TgpWEyLQZKVnErHsJW0wBGlRuuGIB2xajcsREjzWiZ0NoxLaeMnZVWUbWMDamEQe9P+Fw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Y31lBoLXLCnef8KGytNNQiWsGEQaZq8cD624qUvKwiC1+CVFYSveUjgBdgnKoEDP306aJgHlxNIh8RnO5oFb3ep3mGgwNzcHo65NPzVdTw4FHqxEdWWaBHlDNi93D184MxrCdqjus+WRvzcEeh0kxr6hy6UAf/deSX3pZw7Ai5V9Jjc+2iFv+6NAe0TOvTaCqmvZPFUh32hK7yx6jk8lK5zYEjdX2JJsIlZLBQtgr1hiPaPuiT1YwPphVz1x5gDb/dmhPE9EoIHRfZfZFeWxSfiPraq3AZdUeBNUSSM00dayudnCRkUfh0gGOi6nFHomxzdJoTeGhjFW+AsBs0JGaQ==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: linux-kernel@xxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, Juergen Gross <jgross@xxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>, Chris Wright <chrisw@xxxxxxxxxxxx>, Ingo Molnar <mingo@xxxxxxx>, Jeremy Fitzhardinge <jeremy@xxxxxxxxxxxxx>, Olof Johansson <olof@xxxxxxxxx>, linuxppc-dev@xxxxxxxxxxxxxxxx
- Delivery-date: Fri, 02 Dec 2022 11:40:36 +0000
- Ironport-data: A9a23:lagO86seT2/DIoYrGqLT2b3KAufnVC5eMUV32f8akzHdYApBsoF/q tZmKTuAPvyCZmPxetB+bNvk8hxS6JTSydFlHgRt/i08EywT+JbJXdiXEBz9bniYRiHhoOCLz O1FM4Wdc5pkJpP4jk3wWlQ0hSAkjclkfpKlVKiffHg0HVU/IMsYoUoLs/YjhYJ1isSODQqIu Nfjy+XSI1bg0DNvWo4uw/vrRChH4bKj5lv0gnRkPaoR5QWGzSFPZH4iDfrZw0XQE9E88tGSH 44v/JnhlkvF8hEkDM+Sk7qTWiXmlZaLYGBiIlIPM0STqkAqSh4ai87XB9JFAatjsB2bnsgZ9 Tl4ncfYpTHFnEH7sL91vxFwS0mSNEDdkVPNCSDXXce7lyUqf5ZwqhnH4Y5f0YAwo45K7W9yG fMwK2sfTEuDrLmMwaulZsxsqJsjPuW2I9ZK0p1g5Wmx4fcOZ7nmGv+PwOACmTA6i4ZJAOrUY NcfZXx3dhPcbhZTO1ARTpUjgOOvgXq5eDpdwL6XjfNvvy6Pk0ovjv6xbrI5efTTLSlRtlyfq W/cuXzwHzkRNcCFyCrD+XWp7gPKtXOlBt9DT+HpnhJsqH2YxH0/NhoxbmWYuMiQlUK6AdMcO nVBr0LCqoB3riRHVOLVRAakqXSJuhodXdt4EOAg7gyJjK3O7G6xCXIJQT1BQNgnstImSzs30 FOAg9LuA3poqrL9YXCc7LaPqTq4KAAWIHUEaCtCShEKi/HnoYcunlfMQ8xlHaqdkNL4A3fzz iqMoSx4gK8c5eYb2qP+8V3ZjjaEopnSUhVz9gjRRnii7A5yeMiifYPAwVrU9/FbN66CU0KM+ nMDnqC27uoDEJWMnyylW/gWEfei4PPtGDfbm190WZ466y6q5WWgbKhX+jh1IEovOcEBERfiY FXSohx5/5hOJz6xaqsxZJi+Y+wvzK7vEvzoUuzZKNpTCrB2cAiJ1CVvdUmVmWbgjCAElaA5J IeDac3pAXsEIaBm1yatAeAP1bYh3WY532y7eHzg5xGu0L7bYWHPT74AaQGKdrphsP3CpxjJ+ dFCMcfM0w9YTOD1fijQ98gUMEwOKn84Q5vxrqS7a9K+H+avI0l5Y9e5/F/rU9UNc3h9/gsQw kyAZw==
- Ironport-hdrordr: A9a23:zQ9lM623xoOi9V+roLibewqjBIwkLtp133Aq2lEZdPU1SKylfq WV98jzuiWYtN9vYgBGpTntAsW9qArnhP1ICOoqXItKPjOW3FdAd7sSj7cKqgeIcxEWndQy6U 4PScdD4Y3LZ2RSvILG/gmkDto6hPOm3cmT9ILj80s=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
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.
|
