Re: [Xen-devel] [PATCH v4 4/5] amd/iommu: assign iommu devices to Xen

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.11.18 at 12:57, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -993,6 +993,16 @@ static void * __init allocate_ppr_log(struct amd_iommu 
> *iommu)
>  
>  static int __init amd_iommu_init_one(struct amd_iommu *iommu)
>  {
> +    struct pci_dev *pdev;
> +
> +    pcidevs_lock();
> +    pdev = pci_get_pdev(iommu->seg, PCI_BUS(iommu->bdf),
> +                        PCI_DEVFN2(iommu->bdf));
> +    if ( pdev )
> +        /* Assign the IOMMU PCI device to Xen  */
> +        pdev->domain = dom_xen;
> +    pcidevs_unlock();

Why do you kind of open-code pci_hide_device()? It would need
extending to cope with a non-zero segment number, but I'd much
prefer if there could be one central place where the logic lives.
That way list addition would also not be omitted, like you do.

As to the hiding in general, also considering Andrew's objection:
Are these devices representing the IOMMU and nothing else? As
mentioned by Andrew something similar would be needed on the
VT-d side, but iirc there's less clear of a relationship there in any
event (which causes me to wonder about the situation on the
AMD side). I'm asking not the least because iirc at the time
pci_hide_device() was introduced I think it was considered to
hide the AMD IOMMU devices; I don't recall why we didn't in the
end, though.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel