[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: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>, Jiri Slaby <jirislaby@xxxxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Mon, 12 Dec 2022 13:36:48 +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=bRu9Z56sEmMx/fvaUg5qa1RdF02e6Nvl5C1TgIr1ywE=; b=EuM0ejCUJGIKWxm/sUfyuFwz6dkUST8VwdheCkSUaBJq7p9FemQ7o/+tt/XhTARF/zGZqm/3Zyi09p0w9x1S5Poy4Srf8nAEy+gCvur8S/Zxr983m6s3r667M5Ctw/nXEP92Whd8fP+/SSZCS/V+VzSIscByeDNv6AJW0ZIUHpKmEB43YXub5xxgzDWqlhra07l9K1cdonUWnY/O1RQ4f6N+MaG9sRzW8YH+2MLhaPXCiyuxMzgdr/DbBZ2FhT4SOV3CJell9CPq3rHrk9egEr9AwXheCOwt6EseI4pIAN9YUwWXc2TStLRDD7cWJD6wTR+iNSqAD69U8ohAcJ53aA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mLo6ow+qgVwpnNLrgiHSnv7S7iyPZkORvBk9fHSVLNbZbvQDFZpdrTlrFUjPseWmE1hKeM0u1P/dZrW0gvxGP0crJpkMMt8SIluhdeOB4T7r6+Ip6WDr93qR1T6n+5YWyuxVuiGB5E99A2e6A5R1Vk/q/sHFNU79SlTEum4TxoYdAwlt+Qmv8UCetdkIgbW87sAJsWuYl7qBkdukTvlUrBkkH2FjrZAHaH3Nr2cFpCunGPHmyvGx9eDyPQehCIjPtegQ9eviz0IwLJ3MJSvWQfNvtcBDHN9SY6SmHcH3aCQLnFrm7g6WwOYF94uJ8glLQ0B0s3yV4szvE+gI5FW9FQ==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, 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: Mon, 12 Dec 2022 12:37:04 +0000
- Ironport-data: A9a23:D58RWq2jXl0sfibyY/bD5ahxkn2cJEfYwER7XKvMYLTBsI5bpzUDz 2BNXGyFa/iJMWTwed5yOYW+8RgHu5/dyYQyQFE9pC1hF35El5HIVI+TRqvS04F+DeWYFR46s J9OAjXkBJppJpMJjk71atANlVEliefTAOK5ULSfUsxIbVcMYD87jh5+kPIOjIdtgNyoayuAo tq3qMDEULOf82cc3lk8tuTS9nuDgNyo4GlC5wZnP6gS1LPjvyJ94Kw3dPnZw0TQGuG4LsbiL 87fwbew+H/u/htFIrtJRZ6iLyXm6paLVeS/oiI+t5qK23CulQRrukoPD9IOaF8/ttm8t4sZJ OOhF3CHYVxB0qXkwIzxWvTDes10FfUuFLTveRBTvSEPpqFvnrSFL/hGVSkL0YMkFulfDEdT8 vUDEjs0S0rYpeWvkLbmFthXv5F2RCXrFNt3VnBI6xj8VK5ja7acBqLA6JlfwSs6gd1IEbDGf c0FZDFzbRPGJRpSJlMQD5F4l+Ct7pX9W2QA9BTJ+uxouy6KlF0ZPLvFabI5fvSQQspYhACAr 3/u9GXlGBAKcteYzFJp91r82b+fw36lCer+EpW90uxxuXqR21c9GT09a0a+pMins1axDoc3x 0s8v3BGQbIJ3EOxQ9b+XzW8oXiZrhAbRtxcGvE77wfLzbDbiy6bB3IFVTpIYs0Ot8IsQzEuk FiTkLvBHidzubeYTXac8La8rj6oPyURa2gYakcsSQoD/smmo4wpiB/LZshsHbTzjdDvHzz0h TeQo0AWl7gVyMIGyai/1VTGmC634IjESBYv4QfaVX7j6Rl2DKanaJav8kPz9utbIcCSSVzpl HoOltWa7esOJYqQjyHLS+IIdJmj4OyEK3vbmkJ1GIc69CWF/GSqdoRdpjp5IS9BOcYeeCT2S FTUpBgU/5hXenywYsdfZoO3BMMCwKz8GJLlS5j8ad5JbLB1dR+A+WdoaFL492TsllU8ibk0f JuWb+6oDG0GEuJg3j6/Tfxb1qUkrh3S3kvWTJH/ih6hjryXYSfNTa9faQPfKOck8KmDvQPZt c5FMNeHwAleV+u4ZTTL9YkULhYBKn1T6Y3KlvG7v9WremJOcFzNwdeIqV/9U+SJR5hoq9o=
- Ironport-hdrordr: A9a23:gho7s6ryzCAFvLc0tyJr/pQaV5oleYIsimQD101hICG9E/b1qy nKpp8mPHDP5wr5NEtPpTnjAsm9qALnlKKdiLN5Vd3OYOCMghrKEGgN1/qG/xTQXwH46+5Bxe NBXsFFebnN5IFB/KTH3DU=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
Hello,
Gentle ping regarding the locking question below.
Thanks, Roger.
On Fri, Dec 02, 2022 at 12:40:05PM +0100, Roger Pau Monné wrote:
> 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.
>
|
