[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/4] vpci: shrink critical section in vpci_{read/write}
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. > 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. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |