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

Re: [PATCH v12.2 01/15] vpci: use per-domain PCI lock to protect vpci structure


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
  • Date: Wed, 24 Jan 2024 00:07:28 -0500
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=suse.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • 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=p7OQkgaHwt2qt3q0A5mrs+QCPeRPZNHKYNn+J+Chwu4=; b=liFhjt3LThU8JbySdiaP8KEWxbzohVUPt7Rvv1HP7aJCygpNaiM1kaCdlLr7IqW1+Q/tFjQSlvkzPmWWVOCGFx4pQ7LJgOVTsoQKrjBxt6TyyEwRmwg1mUvobEz69vdaxZEh4hkiJ67qnu1H0eXD8koVxpyI3pBY7ZjERZpzpbfupGEv4EmAdZQ1K5kfv8sPse3xpXMd0ZySwJ1vcnKg/gS4V+7W9q/bhsmNSuEXDdpiDvbHuUFNl7g5MBOxLimVQK1uU2wEaVbb9wPrhxJupTI4sOvCnUuDQ+UZe9z4eT5DZMvcrRKskkv95WqLV0xSRe5XwsD25Mey5IhVnTyWhQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=L/CsqmI6xQ1u9J+TChrM13XqHI8r2WNuqM9T+HWVmkiljwb5TXB34goX97Bi8XDVPeh2VKM8pnQ5TT8OrppfClleVmIwLrJYsf8VrFt3uCcORljGK0fSgnZDjpyZXyK6kH/mGX74+tuhZvdopkIXhhoEIXbX5MzIEwnNEvIFDOvFvM/xq60gf83Lqn6aiO7YzqVP4V33g2O5u8D0/altoSGSOqGkE63eTfe1kVti4Y16UU0vHz7Tm6tZ5rbdViSWIRYiBFR/8eS5nrcEnoi/sVge2LnE6def9sxeiCS/kkk8JzGtsawTsaRso2Vkfi5Joav1IExkJtLILrTz4EYUqA==
  • Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 24 Jan 2024 05:08:05 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 1/23/24 09:29, Jan Beulich wrote:
> On 15.01.2024 20:43, Stewart Hildebrand wrote:
>> @@ -1043,11 +1043,11 @@ static int __pci_enable_msix(struct pci_dev *pdev, 
>> struct msi_info *msi,
>>  {
>>      struct msi_desc *old_desc;
>>  
>> -    ASSERT(pcidevs_locked());
>> -
>>      if ( !pdev || !pdev->msix )
>>          return -ENODEV;
>>  
>> +    ASSERT(pcidevs_locked() || rw_is_locked(&pdev->domain->pci_lock));
>> +
>>      if ( msi->entry_nr >= pdev->msix->nr_entries )
>>          return -EINVAL;
> 
> Further looking at this - is dereferencing pdev actually safe without holding
> the global lock?

Are you referring to the new placement of the ASSERT, which opens up the 
possibility that pdev could be dereferenced and the function return before the 
ASSERT? If that is what you mean, I see your point. The ASSERT was placed there 
simply because we wanted to check that pdev != NULL first. See prior discussion 
at [1]. Hmm.. How about splitting the pdev-checking condition? E.g.:

    if ( !pdev )
        return -ENODEV;

    ASSERT(pcidevs_locked() || rw_is_locked(&pdev->domain->pci_lock));

    if ( !pdev->msix )
        return -ENODEV;


[1] 
https://lore.kernel.org/xen-devel/85a52f8d-d6db-4478-92b1-2b6305769c96@xxxxxxx/



 


Rackspace

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