[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 03/14] vpci: move lock outside of struct vpci
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 12 Jan 2022 16:42:58 +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=v+0TNkJOwTARJYSgBPRrPJTm2zaozFgdnEd37LZcyqQ=; b=lB01GtiOnhFI6K956rNTQbBeM1Z+hDaA5ULDHjitaByhnI8+bMv/YcPuq1vtEuxEAv7sy0AuADGIAmhDXT0HjdFnI2ld5e/csS99WucfZeJgEGMRkOmuew6opajO0QDKvyd6MLmeuRoMD6vyAmqP0lh3M16xLqfwOgmy8+8P5lA3dQQadSjkbHXgwMAXuMvEs2KLDe+QAPYkuEUMtusBO/2eM0QDTmTkGk3WDqs8AehMZzHM9LHIjm1kKYb222mn7H5hZtzKaLdsZm/rBnIfSJ13zEUeNYIxekMYjVTQJ5If/XovkUSfLQRqbp5eKPEA8+P5NOhszPCH70tgJAlC+g==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ncl5OEZmKrJKrJVS9zkOihr5Y3BodJrIBlq21BJ4z5nS+J6x+fq0QB7Db66yJ13Y/onYIbUmK4Q3oTolyN6aixKw4MnrhkgagDcNEz8f+wO7klsNfq4gOJQczOEY/elcBAEJSbJT5FlCux9IYHr+6FnHIF4hLp86z+7HehsVsabysuU/qPwrcIrUsXSEetO3Z1lqyBqZ5z6l7srWb1NrWxQtihjVRHTCXP4LFr1JmAN4+H+eDG3aRAUU1daWvw9ROeoKRhYWX5tjuFv1DKDi3FowwJOGhC7/6jIujZxM8+mq5JMFh0CSf3Lq0bVjo/GKzLN9GDN4iA1StzwfQOHAwg==
- Authentication-results: esa1.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
- Cc: Oleksandr Andrushchenko <andr2000@xxxxxxxxx>, <julien@xxxxxxx>, <sstabellini@xxxxxxxxxx>, <oleksandr_tyshchenko@xxxxxxxx>, <volodymyr_babchuk@xxxxxxxx>, <Artem_Mygaiev@xxxxxxxx>, <andrew.cooper3@xxxxxxxxxx>, <george.dunlap@xxxxxxxxxx>, <paul@xxxxxxx>, <bertrand.marquis@xxxxxxx>, <rahul.singh@xxxxxxx>, Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Wed, 12 Jan 2022 15:43:59 +0000
- Ironport-data: A9a23:zYGkBalZLE0ZrK+djs519oTo5gxBIERdPkR7XQ2eYbSJt1+Wr1Gzt xJOC2uEPfnfMDCncowib461oEoGuJLSzt5qTQtkrSw9QiMWpZLJC+rCIxarNUt+DCFioGGLT Sk6QoOdRCzhZiaE/n9BClVlxJVF/fngqoDUUYYoAQgsA180IMsdoUg7wbRh29cy2YLR7z6l4 rseneWOYDdJ5BYsWo4kw/rrRMRH5amaVJsw5zTSVNgT1LPsvyB94KE3fMldG0DQUIhMdtNWc s6YpF2PEsE1yD92Yj+tuu6TnkTn2dc+NyDW4pZdc/DKbhSvOkXee0v0XRYRQR4/ttmHozx+4 PFVscShFiYFBKLvmMRCAyZUKD5UI4QTrdcrIVDn2SCS50jPcn+qyPRyFkAme4Yf/46bA0kXq 6ZecmpUKEne2aTmm9pXScE17ignBNPsM44F/Glp0BnSDOo8QICFSKLPjTNd9Glq3JwTTK6PD yYfQSp1RRfPRBhKAF0oC4kaoM70omT9UjIN/Tp5ooJoujOOnWSdyoPFOtfPZsaDQ8kTm0+Cv 3/H5EzwGBRcP9uaoRKd+2+orv/Cm2X8Qo16PJSi6vNvt3iCyWUSBQM+WEOypL+yjUvWc8JSL QkY9zQjqYA29Ve3VZ/tUhugunmGsxUAHd1KHIUS7wWAybHd5QqDMWECQiRcc9wttMIwRjsC2 0eAmpXiAjkHmKecSW+ZsKyVqzyyESEPKCkJYipsZQkY59jupqkjgxSJScxseIa8iNHvQ2mom xiFqSE/g/MYistj/6ex8E3DgjmsjoPUVQNz7QLSNkq76Qd+aJ+gdpaf41HR5vZdL66UVlCE+ nMDnqC26+QDDoqEkiCXd/kcB7Gi5/uDMzr0jEZmGt8q8DHF02W4YYla7TV6JUFoGsUJYznkZ AnUoww52XNIFCL0N+ktOdv3Upl0i/i7fTj4ahzKRv5WZsBgKy6rxi5NZW7Pj1/hq1cGj4hqb P93bv2QJXodDK1myh+/SOEczaIny0gC+I/DeXzo50/5iOTDPRZ5XZ9AaQLTNb5hsMtotS2Iq 443Ciec9/lIvAQSiAHz+JVbE10FJGNT6Xve+50OLb7rzuaL9Qgc5x7tLVEJJ90Nc0d9zL6gE pSBtqlwkguXaZrvc1TiV5ybQOmzNauTVFpiVcDWAX6m2mI4faGk57oFep08cNEPrbI/lK8uE adVI5rbU5yjrwgrHRxHPPERS6Q4JXyWaf+mZXL5MFDTgbY9L+A2xjMUVlS2r3RfZsZGncA/v 6ehxmvmrWkrHGxf4DLtQKv3lTuZ5CFF8MorBhegCoQNJC3ErdY7QwSs3q5fC5xdcn3rm2rFv zt69D9F/4EhVadvromQ7U1Fxq/0e9ZD8r1yRDiEve3oZHiDrgJOA+ZoCY61QNwUb0utkI2Kb uRJ1fDsdvoBmVdBqY1nFLh3i6k54rPSS3Vyk2yIxV3HMAamDK1OOH6D0ZUdv6FB3OYB6wC3R liO6p9RPrDQYJHpF1sYJQwEaOWf1K5LxmmOvKpteEiqtjVq+LenUFlJO0XegiJqM7YoYpgux v0suZBK5lXn2AYqKNuPkgtd63+Ici4bS6wiu5xDWN3rhwMnx0tse5vZDiOqspiDZ88VahshI yOOhbqEjLNZnxKQf302HHnL/OxcmZVR50wakA5cfwyEw4OXiOU20Rtd9SUMYj5UlhgXgfhuP mVLNlFuIfnc9Tlfm8UeDXunHBtMBUPF9xWpmUcJjmDQU2KhSnfJcD8mIe+I8U0UrzBcczxc8 O3KwWrpS2+3LsT43y90Uk95sf3zC9d281SaysygGs2EGbg8YCbk3fDyNTZZ9UO/DJNjnlDDq Mlr4P10OP/yOiMnqqEmD5WXiOYLQxeeKW0eGfxs8cvlx40HlO1eDdRWF32MRw==
- Ironport-hdrordr: A9a23:HWtfq6j7Fbx0JNfeiwxYzlmVUXBQXz513DAbv31ZSRFFG/FwyP rBoB1L73DJYWgqNE3I+erhBEGBKUmsk6KdxbNhQItKPTOWwldASbsC0WKM+UyEJ8STzJ846U 4kSdkDNDSSNykKsS+Z2njBLz9I+rDum8rE9ISurUuFDzsaEJ2Ihz0JdDpzeXcGPTWua6BJc6 Z1saF81kWdkDksH4+GL0hAe9KGi8zAlZrgbxJDLxk76DOWhTftzLLhCRCX0joXTjsKmN4ZgC X4uj28wp/mn+Cwyxfa2WOWx5NKmOH5wt8GIMCXkMAaJhjllw7tToV8XL+puiwzvYiUmRwXue iJhy1lE9V46nvXcG3wiRzx2zP42DJr0HPmwU/wuwqUneXJABYBT+ZRj4NQdRXUr2A6ustn7a 5N12WF87JKEBLphk3Glpb1fiAvsnDxjWspkOYVgXAae5AZcqVtoYsW+14QOIscHRj99JssHI BVfY7hDc5tABOnhk3izypSKITGZAVwIv7GeDlPhiWt6UkWoJgjpHFogfD2nR87heUAotd/lq D5259T5cJzp/ktHNZA7dc6MLuK41P2MGDx2UKpUB3a/fI8SjrwQ6Ce2sRB2AjtQu1O8KcP
- Ironport-sdr: DltGclRys/UFQJoJbT/WKvMRO90PC/jMFcnwDr1PWR/u8t8F06hJ0lq7vYUUSIMYhWKA6E+aDd GdJ4FKpiriN5xrRHZYzOpfDm0OYIyaYZXIKRB5R1Aih7hy7K/7fEPE26uPejHPCUqvLxoHFzT2 har8EjLk9S+fc1ObJtstr1X9R9PIlg9UZI4DIBAayFTHB65WSGLqLhGabtU/BtDS1dCntBFpva 5HliEph1GUD0PlEgCCzOSu/zm+FBteU5YFRNzL8RsN4jAxFeBJiY92ox9yyltwqbQ2PUbIF+Fq ic2eHlDNpEFDn3PEpEyOaVHc
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Wed, Jan 12, 2022 at 03:57:36PM +0100, Jan Beulich wrote:
> On 25.11.2021 12:02, Oleksandr Andrushchenko wrote:
> > @@ -379,7 +396,6 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg,
> > unsigned int size)
> >
> > data = merge_result(data, tmp_data, size - data_offset,
> > data_offset);
> > }
> > - spin_unlock(&pdev->vpci->lock);
> >
> > return data & (0xffffffff >> (32 - 8 * size));
> > }
>
> Here and ...
>
> > @@ -475,13 +498,12 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg,
> > unsigned int size,
> > break;
> > ASSERT(data_offset < size);
> > }
> > + spin_unlock(&pdev->vpci_lock);
> >
> > if ( data_offset < size )
> > /* Tailing gap, write the remaining. */
> > vpci_write_hw(sbdf, reg + data_offset, size - data_offset,
> > data >> (data_offset * 8));
> > -
> > - spin_unlock(&pdev->vpci->lock);
> > }
>
> ... even more so here I'm not sure of the correctness of the moving
> you do: While pdev->vpci indeed doesn't get accessed, I wonder
> whether there wasn't an intention to avoid racing calls to
> vpci_{read,write}_hw() this way. In any event I think such movement
> would need justification in the description.
I agree about the need for justification in the commit message, or
even better this being split into a pre-patch, as it's not related to
the lock switching done here.
I do think this is fine however, as racing calls to
vpci_{read,write}_hw() shouldn't be a problem. Those are just wrappers
around pci_conf_{read,write} functions, and the required locking (in
case of using the IO ports) is already taken care in
pci_conf_{read,write}.
Thanks, Roger.
|