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

Re: [libvirt] [PATCH] virpci:fix Secondary Bus Reset bug




At 2019-08-16 23:10:31, "Laine Stump" <laine laine org> wrote: >The fact that you are modifying this code implies that you are using it, >and that implies that you are still using legacy KVM device assignment >(i.e. the pcistub driver) instead of VFIO device assignment. (I say this >because the function that calls this function you've patched, >virPCIDeviceReset(), has a check very early on that short-circuits the >entire function if the device is bound to vfio-pci (and if the device is >bound to a host driver, then you shouldn't be resetting it anyway, so >the only reason it would be executed is if you're using legacy KVM >device assignment or are calling virsh nodedev-reset when you shouldn't). > >Legacy PCI device assignment was deprecated over 5 years ago, and was >removed from the kernel sometime after that. We are seriously >considering removing all vestigal support for legacy KVM device >assignment from libvirt, since any kernel that is less than 5 years old >have VFIO, and most don't even support legacy KVM device assignment at all. > >So are you actually using legacy KVM device assignment, or did you just >find this bug by manual code inspection? If the latter, then you need to >seriously look into using VFIO instead. If the latter, then this patch >can safely be pushed, but it's not going to actually do anything (and >the code it's in will likely no longer exist within the next 6 months). > >On 8/15/19 5:44 AM, hexin900110 163 com wrote: >> From: hexin <hexin15 baidu com> >> >> The parent bridge configuration of the current device >> should be read and reset, instead of reading the current >> device configuration. >> >> Signed-off-by: He Xin <hexin15 baidu com> >> Signed-off-by: Liu Qi <liuqi16 baidu com> >> Signed-off-by: Zhang Yu <zhangyu31 baidu com> >> --- >> src/util/virpci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/util/virpci.c b/src/util/virpci.c >> index 61a6b359e5..483de2cb16 100644 >> --- a/src/util/virpci.c >> +++ b/src/util/virpci.c >> @@ -821,7 +821,7 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevicePtr dev, >> /* Read the control register, set the reset flag, wait 200ms, >> * unset the reset flag and wait 200ms. >> */ >> - ctl = virPCIDeviceRead16(dev, cfgfd, PCI_BRIDGE_CONTROL); >> + ctl = virPCIDeviceRead16(dev, parentfd, PCI_BRIDGE_CONTROL); >> >> virPCIDeviceWrite16(parent, parentfd, PCI_BRIDGE_CONTROL, >> ctl | PCI_BRIDGE_CTL_RESET);
>

Thanks for attention. I finded this bug by manual code inspection.

Thanks,
HeXin


 


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