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

Re: [Xen-devel] [PATCH 2/2] x86/pci: Remove unnecessary check in VF value computation



>>> On 13.02.14 at 10:48, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>>>> On 12.02.14 at 22:05, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
>> This test is already performed a couple of lines above.
> 
> Except that it's the wrong code you remove:

No opinion on this alternative at all?

Jan

>> @@ -639,11 +639,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 bus, u8 slot, 
>> u8 
> func, u8 bir, int vf)
>>          if ( vf < 0 || (vf && vf % stride) )
>>              return 0;
>>          if ( stride )
>> -        {
>> -            if ( vf % stride )
>> -                return 0;
>>              vf /= stride;
>> -        }
> 
> Note how this second check carefully avoids a division by zero.
> From what I can tell I think that I simply forgot to remove the
> right side of the earlier || after having converted it to the safer
> variant inside the if(). Hence I think we instead want:
> 
> x86/MSI: don't risk division by zero
> 
> The check in question is redundant with the one in the immediately
> following if(), where dividing by zero gets carefully avoided.
> 
> Spotted-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -635,7 +635,7 @@ static u64 read_pci_mem_bar(u16 seg, u8 
>              return 0;
>          base = pos + PCI_SRIOV_BAR;
>          vf -= PCI_BDF(bus, slot, func) + offset;
> -        if ( vf < 0 || (vf && vf % stride) )
> +        if ( vf < 0 )
>              return 0;
>          if ( stride )
>          {




_______________________________________________
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®.