Re: [PATCH v2 1/4] IOMMU/x86: switch to alternatives-call patching in further instances

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.01.2022 10:28, Durrant, Paul wrote:
> On 27/01/2022 14:47, Jan Beulich wrote:
>> @@ -1457,24 +1462,24 @@ static int iommu_get_device_group(
>>       if ( !is_iommu_enabled(d) || !ops->get_device_group_id )
>>           return 0;
>>   
>> -    group_id = ops->get_device_group_id(seg, bus, devfn);
>> +    group_id = iommu_call(ops, get_device_group_id, seg, bus, devfn);
>>   
>>       pcidevs_lock();
>>       for_each_pdev( d, pdev )
>>       {
>> -        if ( (pdev->seg != seg) ||
>> -             ((pdev->bus == bus) && (pdev->devfn == devfn)) )
>> +        unsigned int b = pdev->bus;
>> +        unsigned int df = pdev->devfn;
>> +
>> +        if ( (pdev->seg != seg) || ((b == bus) && (df == devfn)) )
>>               continue;
>>   
>> -        if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (pdev->bus << 8) 
>> | pdev->devfn) )
>> +        if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (b << 8) | df) )
>>               continue;
>>   
>> -        sdev_id = ops->get_device_group_id(seg, pdev->bus, pdev->devfn);
>> +        sdev_id = iommu_call(ops, get_device_group_id, seg, b, df);
>>           if ( (sdev_id == group_id) && (i < max_sdevs) )
>>           {
>> -            bdf = 0;
>> -            bdf |= (pdev->bus & 0xff) << 16;
>> -            bdf |= (pdev->devfn & 0xff) << 8;
>> +            bdf = (b << 16) | (df << 8);
> 
> Don't we have a macro for this now? Probably best to start using it 
> whilst modifying the code.

We don't. And it would feel somewhat misleading to use PCI_BDF2(b, df) << 8
here. The situation is even worse imo: Besides there not being a macro, I
also cannot seem to find any documentation on this non-standard layout (BDF
shifted left by 8). Yet then again I also can't spot any caller of
xc_get_device_group() ...

> Reviewed-by: Paul Durrant <paul@xxxxxxx>

Thanks.

Jan