On 14.03.2025 10:30, Andrew Cooper wrote:
> On 14/03/2025 8:56 am, Jan Beulich wrote:
>> On 14.03.2025 09:07, Andriy Sultanov wrote:
>>> On Thu, 13 Mar 2025 at 19:59, Jason Andryuk <jason.andr...@amd.com> wrote:
>>>> On 2025-03-13 14:57, 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;
>>>> Are these still needed by the end of this patch?
>>> Yes - otherwise the patch would be larger as bdf and seg would be one
>>> namespace deeper - /iommu->seg/iommu->sbdf.seg/
>> This kind of union is fragile. Hence we want to avoid it, even if this means
>> an overall larger diff.
>
> This is my suggestion, and it's the pattern used in struct pci_dev.
And I'm hoping to eliminate it there, too, at some point. But adding a hidden
dependency on the layout in an entirely different part of the tree just cannot
do us any good.
> pci_sbdf_t is nice for code generation, but it's not great for source
> verbosity.
I agree, yet if anything we'd need a global approach to deal with that
aspect.
Jan
