Re: [Xen-devel] [PATCH v3 3/4] iommu: introduce iommu_groups

[Jan Beulich <JBeulich@xxxxxxxx>]

On 16.07.2019 12:16, Paul Durrant wrote:
> --- a/xen/drivers/passthrough/Makefile
> +++ b/xen/drivers/passthrough/Makefile
> @@ -4,6 +4,7 @@ subdir-$(CONFIG_X86) += x86
>  subdir-$(CONFIG_ARM) += arm
>  
>  obj-y += iommu.o
> +obj-$(CONFIG_HAS_PCI) += groups.o

I assume this dependency on PCI is temporary, as there's nothing
inherently tying grouping of devices to PCI (afaict)?

> +int iommu_group_assign(struct pci_dev *pdev, void *arg)
> +{
> +    const struct iommu_ops *ops = iommu_get_ops();
> +    int id;
> +    struct iommu_group *grp;
> +
> +    if ( !ops->get_device_group_id )
> +        return 0;

With you making groups mandatory (i.e. even solitary devices getting
put in a group), shouldn't this be -EOPNOTSUPP, maybe accompanied by
ASSERT_UNREACHABLE()?

> +    id = ops->get_device_group_id(pdev->seg, pdev->bus, pdev->devfn);
> +    if ( id < 0 )
> +        return -ENODATA;
> +
> +    grp = get_iommu_group(id);
> +    if ( !grp )
> +        return -ENOMEM;
> +
> +    if ( iommu_verbose )
> +        printk(XENLOG_INFO "Assign %04x:%02x:%02x.%u -> IOMMU group %x\n",
> +               pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> +               PCI_FUNC(pdev->devfn), grp->id);

I'm not overly happy about this new logging: On modern systems a
debug level run is already rather verbose about PCI devices,
simply because there are so many. If my hope to not see individual
devices put in groups is not going to be fulfilled, can we at least
try to come to some agreement that certain devices which can't
sensibly be passed through won't be assigned groups (and hence
won't produce output here)? A group-less device then would
automatically be unable to have its owner changed.

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