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

[何鑫]

At 2019-08-16 23:10:31, "Laine Stump" <laine at 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 at 163.com wrote:
>> From: hexin <hexin15 at 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 at baidu.com>
>> Signed-off-by: Liu Qi <liuqi16 at baidu.com>
>> Signed-off-by: Zhang Yu <zhangyu31 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190820/26bba131/attachment-0001.htm>