Re: [PATCH v12 4/6] IOMMU/x86: using a struct pci_dev* instead of SBDF

["Jan Beulich" <JBeulich@suse.com>]

>>> On 24.06.16 at 07:51, <quan.xu@intel.com> wrote:
> --- a/xen/drivers/passthrough/x86/ats.c
> +++ b/xen/drivers/passthrough/x86/ats.c
> @@ -22,26 +22,34 @@ LIST_HEAD(ats_devices);
>  bool_t __read_mostly ats_enabled = 0;
>  boolean_param("ats", ats_enabled);
>  
> -int enable_ats_device(int seg, int bus, int devfn, const void *iommu)
> +int enable_ats_device(const void *iommu, struct pci_dev *pci_dev)

Is there anything preventing the second parameter to become
a pointer to const too? Afaict that would in turn eliminate the
need for some of the changes further up.

>  {
>      struct pci_ats_dev *pdev = NULL;
>      u32 value;
>      int pos;
>  
> -    pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS);
> +    pos = pci_find_ext_capability(pci_dev->seg, pci_dev->bus, pci_dev->devfn,
> +                                  PCI_EXT_CAP_ID_ATS);

Please add local variables seg, bus, and devfn, which will greatly
reduce the number of changes you need to do to this function
(and which likely will also produce better code).

> @@ -98,7 +104,13 @@ void disable_ats_device(int seg, int bus, int devfn)

For symmetry reasons this function would also get converted to
taking const struct pci_dev *.

> @@ -120,7 +132,13 @@ struct pci_ats_dev *get_ats_device(int seg, int bus, int devfn)

And this one then probably too.

Jan


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