[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 24.06.16 at 07:51, <quan.xu@xxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.