[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: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 12 Jan 2022 16:52:51 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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=Bwfi8e4CEqUxsEeBbHGscrvKhlOZGR44hx3/ktbTFqk=; b=Qd8e3sWkKqGV1u7PxNV6GwmfyWK7KRUmhj9eUx2TJMXbXVvtdUDHE0E91iO8oByYMYIjH4fQoPzNjW/Hl8GDkDTY9++cJK5f7pUy8XjAIfOn54cA6+0ugNwowx3f4RbeFr1gX34LIhgcGbeCT/9sVepzOxfAQAOuCAn5ZbVhh8JlI96ziTOxiL/D8EAItZmuXkNRNAlPMUo18PDyje5e94yTlBKcVrGBQz+SigMbwNQM/vmPO2H2WZHvwpxClRoLOlflC9E406dsz5kvJfr5VFfzejCIx6Ljd+WdbbTxRFrnaQT6P+ALEgUg2w0pc9XAjFjkZ+grpcO9IU6jiN9J6w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ctij/2DWLaNQfpo2xGBmATkU5C74eSAtox6gkAYc8Dlcd5LkEdEpKn6fUyBQaSG6O7DdpTo++vrXT8GCKSlOMNtlDy98UohBwtVjXPcTmTjV8lGx5KAqBT6p97TO7tF6wN/iFGepBRQbXWZe8wNRdtpYGtwHFjxLKHZ23lYOyET6K56pUUJLSC6xOlelDg0rONcS5MrQ3zY55z1Q1nL4ocohQR5f0twpmXJamedGuFtCyphCHWKFXLYlv4sGJdbroTzr9dv5oErUVH5kyNfXuh6zvoBnKC95ml2QVGfLujnrOEwlHsezndoO1hctkHnz9T1v8vV/QcpftCplH9p4Og==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.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:53:06 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 12.01.2022 16:42, Roger Pau Monné wrote:
> 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}.

IOW you consider it acceptable for a guest (really: Dom0) read racing
a write to read back only part of what was written (so far)? I would
think individual multi-byte reads and writes should appear atomic to
the guest.

Jan




 


Rackspace

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