[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] vpci: shrink critical section in vpci_{read/write}


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 2 Feb 2022 10:49:57 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; 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=GMTs6TmJI+d5slk+yBtZIzCzL4m77upsgD/DgXXlER8=; b=P+gByGj8lc64SpI95TCEdJ+TD6Ee/lFhbe9VaR/N64OiJ+1iveuX8SNudSk5BE4BjvfXH/x2SXAwOB5oEYYAFz9b+318u7OoV6APFuzxUG/Eu0DitSsFD3DeEgakXxQCvRnIf6V/AcfE9jhD1cEdr+fJMiUx8463Xo4hnYRkTYHXd8AbQ463uARpxRz9ZEjunvG7xEU5EKNGQB6dJ+3sSDKNr+1U+ODRlj4jxcGPlnDA16p3MicinKjNuSS4uY5D4O9vMGq0qBOjO+x0xISgQ3NbLK/z1WGeJtmHFY1RoTmMYMJeGsBnbJFB/1/Z5sPUQcQRlB9dJVa9gauoVh1CoA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=j2nsDnJeRS9rDnoHUSUiDP/mBdKW8p7Md2o7TNj3fJhUHwygv/X07HHcGoAgIMFqywsy5CPiNVmWPK6ZoyY3J1uTs/e7aF+MSz41GrEPHn2AXedID6hxeKdg5nM8m9CzJTwkNCanKUaSvB6pG9WKq4xRrlg6AJUAB7Ez66sorC9rzWsb1gcvkRs8MxGmltxcSCGboBmHDKAqPmEessQjY5VBigSnchgQaO53WJn3L5xK9hZK/IjQjp9d+nFUja0xSi0tgxxBwSSlb6QMG0WKYMGsEN9HkIMp+L2xttWziA12T7JgNPau0ckNRpyi3pRmICQywuUwiqnTsKvzhNK3yg==
  • Authentication-results: esa6.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Oleksandr Andrushchenko <andr2000@xxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>, <julien@xxxxxxx>, <sstabellini@xxxxxxxxxx>, <oleksandr_tyshchenko@xxxxxxxx>, <volodymyr_babchuk@xxxxxxxx>, <george.dunlap@xxxxxxxxxx>, <paul@xxxxxxx>, <bertrand.marquis@xxxxxxx>, <rahul.singh@xxxxxxx>, Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
  • Delivery-date: Wed, 02 Feb 2022 09:50:15 +0000
  • Ironport-data: A9a23:9jjWVqLMv/KWqqR2FE+R9ZMlxSXFcZb7ZxGr2PjKsXjdYENS0T0Hm GpLX2nSPvzZZmHwetxzbo23pkIDvMPTxtRnGVRlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokcxIn5BC5C5xZVG/fjgqoHUVaiUakideSc+EH170Ug7wbZg6mJVqYPR7z2l6 IuaT/L3YDdJ6xYsWo7Dw/vewP/HlK2aVAIw5jTSV9gS1LPtvyB94KYkDbOwNxPFrrx8RYZWc QphIIaRpQs19z91Yj+sfy2SnkciGtY+NiDW4pZatjTLbrGvaUXe345iXMfwZ3u7hB2jz5M20 e8QmKaUEyR5MYzxnfkdDjZXRnQW0a1uoNcrIFC6uM2XiUbHb2Ht07NlC0Re0Y8wo7gtRzsUr LpBdW5LPkvra+GemdpXTsF2gcsuNo/zNZ43sXB81zDJS/0hRPgvRo2UvoUBjGpv2aiiG96AZ 5cZcAY1PSj9SDRFOQYNKaIDw7u30yyXnzpw9wvO+PtfD3Lo5BR4zbzFINfTPNuQSq19gEGco W7X+nXjNRsTPtef1Dmt/2qlg6nEmiaTcIgfDqGi//hmxlia3HUOCQY+XEG+5/K+jyaWXNZSK Fcd/CY0mqE0+Fa2Vdn2XxC+o3msswYVXpxbFOhSwBuEyrfQpR2YAGcEZjdbbZots8pebTsu2 1ySltXlHwtzoaaVQnKQ8LSThT6qMC1TJmgHDQcNSwcfuYG7+Kk8ixvOSpBoF6vdpt//FCz0w juKhDMjnLhVhskOv42r8FaCjz+yq5zhSg8u+h6RTm+j9hl+ZoOue8qv81ezxe1bMI+TQ12Fv X4Fs8uT9uYDCdeKjiPlaOcQGLCk4d6VPTuahkRgd6TN7Bz0pSTlJ9oJpmgjegE5aa7oZAMFf mf56UAW/aUOEUGON4wnbKOMDtkqkIfvQIGNuu/vUvJCZZ14dQmi9S5oZFKN022FrHXAgZ3TK r/AL5/yUC9y5bBPiWPvGrxDieNDKjUWmDuLLa0X2ShLxlZ3iJS9bb4eeGWDYekihE9viFWEq o0PXydmJvg2bQEfXsU12dJCRbzpBSJibXwTlyCwXrXbSjeK4El7V5fsLUoJIuSJZZh9mObS5 W2aUURF0lf5jnCvAVzUNis8NOq+DMsm8yJT0ckQ0bGAgCBLXGpSxP1HK8tfkUcPqISPMsKYv 9FaIp7dU5yjuxzM+igHbImVkWCRXE/DuO56BAL8OGJXV8c5H2Tho4a4FiOyqnVmJnfp5KMW/ u3xviuGEMFrb1kzU67rhAeHkgnZUY41wrwiBiMl47B7JS3RzWSdA3eg06Zqc5xQckirK/nz/ 1/+PCr0bNLl+ucd2NLImbqFv8GuFe5/FVBdBG7V8fC9Miyyw4Zp6dUovD+gcW+PWWXq1r+lY OkJnfjwPOdexARBspZmEqYtxqU7voO9q7hfxwViPXPKc1X0Ve8wfijYhZFC5v9X27tUmQqqQ UbTqNNUDqqEZZH+G1kLKQt7MunajaMImiPf5OgeKVnh4HMl56KOVEhfZkHeiCFUILZvHpkix OMt5Jwf5wCl00J4OdealCFEsW+LKyVYAakgs5gbBq7tixYqlQ4eMcCNVHeu7cjWOdtWM0QsL juFv4b4huxRlhjYbn4+NXnRxu4B154Ajw9HkQ0ZLFOTl9ub2vJuhE9N8S46Rxh+xwlc174hI XBiMkB4KPnc/zpsg8QfDWmgFxsYWU+c8031jVAIiHfYXw+jUWmUdD8xPuOE/UY49WNAf2cEo OHEmTi9CTu6Ltvs2iYSWFJ+r62xRNN8wQTOhcS7EpnXBJI9ezfk3vejaGdgR8EL2i/taJkrf dVXwds=
  • Ironport-hdrordr: A9a23:GQf6/KEJoJNAHZB9pLqFcJHXdLJyesId70hD6qkvc3Nom52j+/ xGws536faVslcssHFJo6HmBEClewKnyXcT2/htAV7CZnichILMFu9fBOTZsl/d8kHFh4tgPO JbAtRD4b7LfClHZKTBkXCF+r8bqbHtmsDY5ts2jU0dNT2CA5sQkTuRYTzrdHGeKjM2YabQQ/ Gnl7V6TnebCD8qR/X+IkNAc/nIptXNmp6jSRkaByQ/4A3LqT+z8rb1HzWRwx9bClp0sPof2F mAtza8yrSosvm9xBOZ/2jP765OkN+k7tdYHsSDhuUcNz2poAe1Y4ZKXaGEoVkO0auSwWdvtO OJjwYrPsx15X+UVmapoSH10w2l6zoq42+K8y7vvVLT5ejCAB4qActIgoxUNjHD7VA7gd162K VXm0qEqpt+F3r77WrAzumNcysvulu/oHIkn+JWpWdYS5EiZLhYqpFa1F9JEa0HADnx5OkcYa ZT5fnnlbZrmG6hHjPkVjEF+q3vYp1zJGbLfqE6gL3V79AM90oJinfxx6Qk7wA9HdwGOt15Dt //Q9dVfYd1P7srhJJGdZc8qPSMex7wqDL3QSuvyAfcZek600ykke+D3Fxy3pDsRKA1
  • Ironport-sdr: yU9N1HeSF/bzkFRSZauNJZqa1gdzcbF0TFRe4Enbh65xaAAjriQP9qLfKgqbzCaAr6nxnHJZpl QVS6uaUfCz/Mpah5IhC+6KObarxptJBglqY/rK5uU2pUyX3Bz+rIzOLnK+eOcu+sF1286fcOrt +xkSuuuSmvnw+fx0JXUz8tXyE5pPWhi1h0H+8gLznB+GazEU8RgO2FQiKOjKboN1sUdRI02ZaN rslyUzXyWujcwEydJ1fxZZCXWY16tKCGpx3psQsNbrHNzZeTJs+Rqe17C295N0wXdVHGOTz3SW pC+xeTGGfkiQPkcPQZkC0xIa
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Feb 02, 2022 at 10:05:55AM +0100, Jan Beulich wrote:
> On 02.02.2022 09:44, Roger Pau Monné wrote:
> > On Tue, Feb 01, 2022 at 06:25:07PM +0200, Oleksandr Andrushchenko wrote:
> >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> Oleksandr, can you please clarify authorship here? The rule of thumb is
> that From: matches ...
> 
> >> Shrink critical section in vpci_{read/write} 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}.
> >>
> >> Please note, that we anyways split 64bit writes into two 32bit ones
> >> without taking the lock for the whole duration of the access, so it is
> >> possible to see a partially updated state as a result of a 64bit write:
> >> the PCI(e) specification don't seem to specify whether the ECAM is allowed
> >> to split memory transactions into multiple Configuration Requests and
> >> whether those could then interleave with requests from a different CPU.
> >>
> >> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> ... the first S-o-b, as these are expected to be in chronological
> order.
> 
> > Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> I'll take your unconstrained ack to indicate that you're also fine
> with this going in right away; see my reply to 0/4 as to the earlier
> two patches. Please let me know (soonish) if I shouldn't make this
> implication, but I shall wait with committing for clarification of
> the question further up anyway.

I think both vPCI patches in the series could go in when ready. They
are improvements on their own.

> > Would like to make sure whether Jan still have concerns about
> > splitting accesses though.
> 
> I continue to be a little concerned, but as long as the decision is
> taken consciously (and this is recorded in the description), which
> clearly is the case now, I have no objections. In the end well
> behaved OSes will suitably serialize accesses to config space anyway.
> 
> > Also since I'm the maintainer we need a Reviewed-by from someone else.
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> I'm not sure this is strictly needed though: I'd generally consider
> a 2nd (later) S-o-b as valid stand-in for R-b, at least as long as
> the 2nd author doesn't scope-restrict their tag.
> 
> One further remark though: The resulting asymmetry of the locking
> (covering the "head" hw read but not the "tail" one) looks a little
> odd, but I will admit that I don't see a good way to restore symmetry.

I did realize about such asymmetry also, but I don't think it can be
solved.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.