Re: [PATCH] Fix the ACS checking in the PCI code.

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

 



Chris Lalancette wrote:
> The code in libvirt that does ACS checking has a pretty
> serious bug that was essentially rendering the check useless.
> When trying to assign a device, we have to check that all
> bridges upstream of this device support ACS.  That means
> that we have to find the parent of the current device,
> check for ACS, then find the parent of that device, check
> for ACS, etc.
> 
> However, the code to find the parent of the device had a
> much too relaxed check.  It would iterate through all PCI
> devices on the system, looking for a device that had a range
> of busses that included the current device's bus.
> 
> That's not correct, though.  Depending on how we iterated
> through the list of PCI devices, we could first find the
> *topmost* bridge in the system; since it necessarily had
> a range of busses including the current device's bus, we
> would only ever check the topmost bridge, and not check
> any of the intermediate bridges.
> 
> This patch is simple in that it looks for the PCI device
> whose secondary device *exactly* equals the bus of the
> device we are looking for.  That means that one, and only one
> bridge will be found, and it will be the correct device.
> 
> Note that this also caused a fairly serious bug in the
> secondary bus reset code, where we could erroneously
> reset the topmost bus instead of the inner bus.
> 
> This patch was tested by me on a 4-port NIC with a
> bridge without ACS (where assignment failed), a 4-port
> NIC with a bridge with ACS (where assignment succeeded),
> and a 2-port NIC with no bridges (where assignment
> succeeded).
> 
> Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
> ---
>  src/util/pci.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/pci.c b/src/util/pci.c
> index 26d55b8..df30e04 100644
> --- a/src/util/pci.c
> +++ b/src/util/pci.c
> @@ -513,7 +513,7 @@ static int
>  pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED)
>  {
>      uint16_t device_class;
> -    uint8_t header_type, secondary, subordinate;
> +    uint8_t header_type, secondary;
>  
>      if (dev->domain != check->domain)
>          return 0;
> @@ -529,12 +529,11 @@ pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED)
>          return 0;
>  
>      secondary   = pciRead8(check, PCI_SECONDARY_BUS);
> -    subordinate = pciRead8(check, PCI_SUBORDINATE_BUS);
>  
>      VIR_DEBUG("%s %s: found parent device %s", dev->id, dev->name, check->name);
>  
>      /* No, it's superman! */
> -    return (dev->bus >= secondary && dev->bus <= subordinate);
> +    return (dev->bus == secondary);
>  }
>  
>  static pciDevice *

Acked-by: Donald Dutile <ddutile@xxxxxxxxxx>

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]