On 08.11.21 13:10, Jan Beulich wrote:
> On 05.11.2021 07:56, Oleksandr Andrushchenko wrote:
>> --- a/xen/arch/arm/vpci.c
>> +++ b/xen/arch/arm/vpci.c
>> @@ -41,6 +41,15 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t
>> *info,
>> /* data is needed to prevent a pointer cast on 32bit */
>> unsigned long data;
>>
>> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
>> + /*
>> + * For the passed through devices we need to map their virtual SBDF
>> + * to the physical PCI device being passed through.
>> + */
>> + if ( !bridge && !vpci_translate_virtual_device(v->domain, &sbdf) )
>> + return 1;
> Nit: Indentation.
Ouch, sure
>
>> @@ -59,6 +68,15 @@ static int vpci_mmio_write(struct vcpu *v, mmio_info_t
>> *info,
>> struct pci_host_bridge *bridge = p;
>> pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
>>
>> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
>> + /*
>> + * For the passed through devices we need to map their virtual SBDF
>> + * to the physical PCI device being passed through.
>> + */
>> + if ( !bridge && !vpci_translate_virtual_device(v->domain, &sbdf) )
>> + return 1;
> Again.
Will fix
>
>> @@ -172,10 +175,37 @@ REGISTER_VPCI_INIT(vpci_add_virtual_device,
>> VPCI_PRIORITY_MIDDLE);
>> static void vpci_remove_virtual_device(struct domain *d,
>> const struct pci_dev *pdev)
>> {
>> + ASSERT(pcidevs_locked());
>> +
>> clear_bit(pdev->vpci->guest_sbdf.dev, &d->vpci_dev_assigned_map);
>> pdev->vpci->guest_sbdf.sbdf = ~0;
>> }
>>
>> +/*
>> + * Find the physical device which is mapped to the virtual device
>> + * and translate virtual SBDF to the physical one.
>> + */
>> +bool vpci_translate_virtual_device(struct domain *d, pci_sbdf_t *sbdf)
> const struct domain *d ?
Will change
>
>> +{
>> + const struct pci_dev *pdev;
>> + bool found = false;
>> +
>> + pcidevs_lock();
>> + for_each_pdev( d, pdev )
>> + {
>> + if ( pdev->vpci->guest_sbdf.sbdf == sbdf->sbdf )
>> + {
>> + /* Replace virtual SBDF with the physical one. */
>> + *sbdf = pdev->sbdf;
>> + found = true;
>> + break;
>> + }
>> + }
>> + pcidevs_unlock();
> I think the description wants to at least mention that in principle
> this is too coarse grained a lock, providing justification for why
> it is deemed good enough nevertheless. (Personally, as expressed
> before, I don't think the lock should be used here, but as long as
> Roger agrees with you, you're fine.)
Yes, makes sense
>
> Jan
>
Thank you,
Oleksandr