Re: [Xen-devel] [PATCH v5] VT-d: fix VF of RC integrated PF matched to wrong VT-d unit

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.07.17 at 10:13, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Jul 06, 2017 at 03:57:37PM +0800, Chao Gao wrote:
>> --- a/xen/drivers/passthrough/vtd/dmar.c
>> +++ b/xen/drivers/passthrough/vtd/dmar.c
>> @@ -218,8 +218,17 @@ struct acpi_drhd_unit 
>> *acpi_find_matched_drhd_unit(const struct pci_dev *pdev)
>>      }
>>      else if ( pdev->info.is_virtfn )
>>      {
>> +        struct pci_dev *physfn;
>> +
>>          bus = pdev->info.physfn.bus;
>> -        devfn = PCI_SLOT(pdev->info.physfn.devfn) ? 0 : 
>> pdev->info.physfn.devfn;
>> +        /*
>> +         * Use 0 as 'devfn' to search VT-d unit when the physical function
>> +         * is an Extended Function.
>> +         */
>> +        pcidevs_lock();
>> +        physfn = pci_get_pdev(pdev->seg, bus, pdev->info.physfn.devfn);
>> +        devfn = (physfn && physfn->info.is_extfn) ? 0 : 
>> pdev->info.physfn.devfn;
>> +        pcidevs_unlock();
> 
> Just as a note (not that I intend you to fix this), but AFAICT this
> function should be called holding the pcidevs lock, or else there's
> the risk that the pdev argument is freed while poking at it.

As pointed out in discussion on (I think) one of your recent patch
series, it is well known that we don't take that lock in all the places
we should, and we really should ref-count struct pci_dev instances.
I don't think dealing with the issue in individual places would be
very useful - if anything, we'd have to audit the entire code base.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel