On 16.03.2025 11:08, Andrii Sultanov wrote:
> --- a/xen/drivers/passthrough/amd/iommu.h
> +++ b/xen/drivers/passthrough/amd/iommu.h
> @@ -77,8 +77,14 @@ struct amd_iommu {
>      struct list_head list;
>      spinlock_t lock; /* protect iommu */
>  
> -    u16 seg;
> -    u16 bdf;
> +    union {
> +        struct {
> +            uint16_t bdf;
> +            uint16_t seg;
> +        };
> +        pci_sbdf_t sbdf;
> +    };

As before: I'm firmly opposed to this kind of secondary aliasing. It's bad
enough that we have something similar in struct pci_dev.

> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -730,7 +730,7 @@ int cf_check amd_iommu_get_reserved_device_memory(
>               * the same alias ID.
>               */
>              if ( bdf != req && ivrs_mappings[req].iommu &&
> -                 func(0, 0, PCI_SBDF(seg, req).sbdf, ctxt) )
> +                 func(0, 0, sbdf.sbdf, ctxt) )
>                  continue;

This looks unrelated to the struct amd_iommu change here? And this is a
change I would ack right away, if sent in isolation or combined with
other, similar changes.

Jan

Reply via email to