Re: [Xen-devel] [PATCH v6 3/8] x86/PCI: read maximum MSI vector count early

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.09.2019 16:22, Roger Pau Monné  wrote:
> On Thu, Sep 19, 2019 at 03:22:54PM +0200, Jan Beulich wrote:
>> Rather than doing this every time we set up interrupts for a device
>> anew (and then in several places) fill this invariant field right after
>> allocating struct pci_dev.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> LGTM:
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

> Just one nit below.
> 
>> @@ -711,7 +710,7 @@ static int msi_capability_init(struct pc
>>  
>>          /* All MSIs are unmasked by default, Mask them all */
>>          maskbits = pci_conf_read32(dev->sbdf, mpos);
>> -        maskbits |= ~(u32)0 >> (32 - maxvec);
>> +        maskbits |= ~(u32)0 >> (32 - dev->msi_maxvec);
> 
> GENMASK would be slightly easier to parse IMO (here and below).

Besides this being an unrelated change, I'm afraid I'm going to
object to use of a macro where it's unclear what its parameters
mean: Even the example in xen/bitops.h is so confusing that I
can't tell whether "h" is meant to be exclusive or inclusive
(looks like the latter is intended). To me the two parameters
also look reversed - I'd expect "low" first and "high" second.
(ISTR having voiced reservations against its introduction, and
I'm happy to see that it's used in Arm code only.)

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel