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

Re: [PATCH v3] vpci/msix: fix PBA accesses


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 7 Mar 2022 17:17:59 +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=+4XvI9Cxx+nhR5CYFbw6cyhfcUZCOR3VuOE9EzBy+cE=; b=f6tulXgUJmuf8+B8Ye7D2Yd3CT+l4pyXzzAhxgC7Wknh8R1pSRcaaH15s85CNyxWj7P6YbYTZeIJPL+L4tT4GMLkp0zk9hOduuVm/jStpcIDT5h4rYAG0ei0bIWIYxLP3zs7qR59UDfBykOilsa8tjd0b1e6zGv3Dn5tH+bfKpDUxoEPGVWVg/STpBWBLAQG89ndQnsrVtsZcwZeMahGMyC36Iuq0y0tY60q38cyPITFKauDT55F6pb+wr5ps4ouXALzLUaQPyOOrL2oXvox7bbfiZEzEvSFj88CH9YyczNRmstk3XUey8lnFHwFAxBcIq6v3gHBpd6Zoiex4/8YNA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gLjdFXBJ4fRxNgJ1L5vC53jRtyvwt1bhhNd/Ti7rsyV66zM1sKSh3b2WkXbdIIrK+ILpg2IHa+eVDZSLSYiTU6lA/blMfn16Qg/NNrfFQA5ruD7BCeS7fOhHtVm6IVL/USUCaS8mdhZvGJzkZYnNNyVtE1HYWelchiLAUr8bCyxEqIBBHR1zVnPflEdjMKxIk+uSdBw/abyx1M8UGTCp7IzdGR3OZJcUod9X98ycu//HnFQCz39bfbkuNTZ428VNo7MBGokg0Se4q2tNcovBJ3Wy3hLJ1wnSwm37xr1YbJmyHjwytfWVynz696s0xnL0dM6CX//f92bIwYDPRWZnLg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Alex Olson <this.is.a0lson@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 07 Mar 2022 16:18:03 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 07.03.2022 17:15, Roger Pau Monné wrote:
> On Mon, Mar 07, 2022 at 05:11:34PM +0100, Jan Beulich wrote:
>> On 07.03.2022 17:06, Roger Pau Monné wrote:
>>> On Mon, Mar 07, 2022 at 03:12:32PM +0100, Jan Beulich wrote:
>>>> On 07.03.2022 13:53, Roger Pau Monne wrote:
>>>>> --- a/xen/drivers/vpci/msix.c
>>>>> +++ b/xen/drivers/vpci/msix.c
>>>>> @@ -182,6 +182,33 @@ static struct vpci_msix_entry *get_entry(struct 
>>>>> vpci_msix *msix,
>>>>>      return &msix->entries[(addr - start) / PCI_MSIX_ENTRY_SIZE];
>>>>>  }
>>>>>  
>>>>> +static void __iomem *get_pba(struct vpci *vpci)
>>>>> +{
>>>>> +    struct vpci_msix *msix = vpci->msix;
>>>>> +    void __iomem *pba;
>>>>> +
>>>>> +    /*
>>>>> +     * PBA will only be unmapped when the device is deassigned, so 
>>>>> access it
>>>>> +     * without holding the vpci lock.
>>>>> +     */
>>>>> +    if ( likely(msix->pba) )
>>>>> +        return msix->pba;
>>>>> +
>>>>> +    pba = ioremap(vmsix_table_addr(vpci, VPCI_MSIX_PBA),
>>>>> +                  vmsix_table_size(vpci, VPCI_MSIX_PBA));
>>>>> +    if ( !pba )
>>>>> +        return msix->pba;
>>>>
>>>> For this particular purpose may want to consider using ACCESS_ONCE() for
>>>> all accesses to this field.
>>>
>>> Hm, I think I've asked before, but we do assume that ACCESS_ONCE will
>>> generate a single instruction, or else we would have to use
>>> read_atomic.
>>
>> Yeah, that looks to be the assumption. It has become my understanding
>> that ACCESS_ONCE() is generally favored over {read,write}_atomic().
>> Personally I prefer the latter when the goal is to have single insns.
> 
> Oh, OK, so I should use 'ACCESS_ONCE(msix->pba) = pba;' rather than
> write_atomic then.

To please others, perhaps. As said, I'd be fine with you using
{read,write}_atomic().

Jan




 


Rackspace

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