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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

~Andrew