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.
> @@ -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.
> @@ -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 ?
> +{
> + 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.)
Jan
