Re: [Xen-devel] [PATCH] passthrough/PCI: replace improper uses of pci_find_next_cap()

["Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx>]

Acked-by: Xiantao Zhang <xiantao.zhang@xxxxxxxxx>

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Friday, November 16, 2012 9:14 PM
> To: xen-devel
> Cc: Wei Huang; Wei Wang; Zhang, Xiantao; Zhang, Yang Z
> Subject: [PATCH] passthrough/PCI: replace improper uses of
> pci_find_next_cap()
> 
> Using pci_find_next_cap() without prior pci_find_cap_offset() is bogus (and
> possibly wrong, given that the latter doesn't check the
> PCI_STATUS_CAP_LIST flag, which so far was checked in an open-coded way
> only for the non-bridge case).
> 
> Once at it, fold the two calls into one, as we need its result in any case.
> 
> Question is whether, without any caller left, pci_find_next_cap() should be
> purged as well.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -589,16 +589,13 @@ void pci_release_devices(struct domain *
> 
>  int pdev_type(u16 seg, u8 bus, u8 devfn)  {
> -    u16 class_device;
> -    u16 status, creg;
> -    int pos;
> +    u16 class_device, creg;
>      u8 d = PCI_SLOT(devfn), f = PCI_FUNC(devfn);
> +    int pos = pci_find_cap_offset(seg, bus, d, f, PCI_CAP_ID_EXP);
> 
>      class_device = pci_conf_read16(seg, bus, d, f, PCI_CLASS_DEVICE);
>      if ( class_device == PCI_CLASS_BRIDGE_PCI )
>      {
> -        pos = pci_find_next_cap(seg, bus, devfn,
> -                                PCI_CAPABILITY_LIST, PCI_CAP_ID_EXP);
>          if ( !pos )
>              return DEV_TYPE_LEGACY_PCI_BRIDGE;
>          creg = pci_conf_read16(seg, bus, d, f, pos + PCI_EXP_FLAGS); @@ -
> 606,15 +603,7 @@ int pdev_type(u16 seg, u8 bus, u8 devfn)
>              DEV_TYPE_PCIe2PCI_BRIDGE : DEV_TYPE_PCIe_BRIDGE;
>      }
> 
> -    status = pci_conf_read16(seg, bus, d, f, PCI_STATUS);
> -    if ( !(status & PCI_STATUS_CAP_LIST) )
> -        return DEV_TYPE_PCI;
> -
> -    if ( pci_find_next_cap(seg, bus, devfn, PCI_CAPABILITY_LIST,
> -                           PCI_CAP_ID_EXP) )
> -        return DEV_TYPE_PCIe_ENDPOINT;
> -
> -    return DEV_TYPE_PCI;
> +    return pos ? DEV_TYPE_PCIe_ENDPOINT : DEV_TYPE_PCI;
>  }
> 
>  /*
> 
> 


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