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_andrushche...@epam.com>
>
> 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....@citrix.com>
> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
>
> ... the first S-o-b, as these are expected to be in chronological
> order.
>
> > Acked-by: Roger Pau Monné <roger....@citrix.com>
>
> 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 <jbeul...@suse.com>
>
> 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.
