Re: [PATCH v2 06/15] pci: Use pci_sbdf_t in pci_prepare_msix()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.06.2026 11:31, Andrew Cooper wrote:
> On 18/06/2026 3:50 pm, Teddy Astie wrote:
>> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
>> index 5bbcf3b530..984fb99aa8 100644
>> --- a/xen/arch/x86/physdev.c
>> +++ b/xen/arch/x86/physdev.c
>> @@ -512,15 +512,16 @@ ret_t do_physdev_op(int cmd, 
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>      case PHYSDEVOP_prepare_msix:
>>      case PHYSDEVOP_release_msix: {
>>          struct physdev_pci_device dev;
>> +        pci_sbdf_t sbdf;
>>  
>>          if ( copy_from_guest(&dev, arg, 1) )
>>              ret = -EFAULT;
>>          else
>> -            ret = xsm_resource_setup_pci(XSM_PRIV,
>> -                                         (dev.seg << 16) | (dev.bus << 8) |
>> -                                         dev.devfn) ?:
>> -                  pci_prepare_msix(dev.seg, dev.bus, dev.devfn,
>> -                                   cmd != PHYSDEVOP_prepare_msix);
>> +        {
>> +            sbdf = PCI_SBDF(dev.seg, dev.bus, dev.devfn);
>> +            ret = xsm_resource_setup_pci(XSM_PRIV, sbdf.sbdf) ?:
>> +                  pci_prepare_msix(sbdf, cmd != PHYSDEVOP_prepare_msix);
>> +        }
>>          break;
>>      }
>>  
> 
> You say "clean up", but IMO this is not much better.  I would much
> prefer if it turned into this:
> 
>     case PHYSDEVOP_release_msix: {
>         struct physdev_pci_device dev;
>         pci_sbdf_t sbdf;
> 
>         ret = -EFAULT;
>         if ( copy_from_guest(&dev, arg, 1) )
>             break;
> 
>         sbdf = PCI_SBDF(dev.seg, dev.bus, dev.devfn);
> 
>         ret = xsm_resource_setup_pci(XSM_PRIV, sbdf.sbdf);
>         if ( ret )
>             break;
> 
>         ret = pci_prepare_msix(sbdf, cmd != PHYSDEVOP_prepare_msix);
>         break;
>     }
> 
> 
> It is slightly longer, but the cognitive complexity is far lower.

This is pretty subjective, isn't it? I for one prefer forms with fewer
"break", as long as indentation doesn't get overly deep.

Jan