Re: [PATCH V7 02/11] vpci: add hooks for PCI device assign/de-assign

[Oleksandr <olekstysh@gmail.com>]

On 27.07.22 17:35, Jan Beulich wrote:


Hello Jan

> On 27.07.2022 16:01, Oleksandr wrote:
>> On 27.07.22 13:03, Jan Beulich wrote:
>>> On 19.07.2022 19:42, Oleksandr Tyshchenko wrote:
>>>> @@ -1603,6 +1610,10 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
>>>>                            pci_to_dev(pdev), flag);
>>>>        }
>>>>    
>>>> +    rc = vpci_assign_device(pdev);
>>>> +    if ( rc && deassign_device(d, seg, bus, old_devfn) )
>>> ... use pdev->devfn here.
>>
>> Thanks, good point, will drop old_devfn and use pdev->devfn. I am
>> wondering whether the printk after "done:" label (and other possible
>> printk-s down the code) should really use pdev->devfn instead of devfn
>> in PCI_SBDF construct?
> Yes, that's intended: If assigning a phantom function fails, this
> should be distinguishable from failure to assign the real device.


Thank you for the clarification.

Hmm, so before this patch the assigning of phantom functions was the 
last action before the "done:" label. So I will probably need to check 
for "rc" before calling vpci_assign_device().

Something like that:


[snip]

@@ -1602,6 +1606,17 @@ static int assign_device(struct domain *d, u16 
seg, u8 bus, u8 devfn, u32 flag)
          rc = iommu_call(hd->platform_ops, assign_device, d, devfn,
                          pci_to_dev(pdev), flag);
      }
+    if ( rc )
+        goto done;
+
+    devfn = pdev->devfn;
+    rc = vpci_assign_device(pdev);
+    if ( rc && deassign_device(d, seg, bus, devfn) )
+    {
+        printk(XENLOG_ERR "%pd: %pp was left partially assigned\n",
+               d, &PCI_SBDF(seg, bus, devfn));
+        domain_crash(d);
+    }

   done:
      if ( rc )

[snip]


>
> Jan

-- 
Regards,

Oleksandr Tyshchenko