Re: [PATCH v4 3/3] drivers: Make ioapic_sbdf and hpet_sbdf contain pci_sbdf_t

[Andriy Sultanov <sultanovandriy@xxxxxxxxx>]

On 4/15/25 8:05 AM, dmkhn@xxxxxxxxx wrote:

> > --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> > @@ -707,8 +707,7 @@ static int __init cf_check parse_ivrs_ioapic(const char 
> > *str)
> >           }
> >       }
> >
> > -    ioapic_sbdf[idx].bdf = PCI_BDF(bus, dev, func);
> > -    ioapic_sbdf[idx].seg = seg;
> > +    ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) );
> PCI_SBDF() is a variadic macro, so, IMO, the line above can be simplified as:
>
>         ioapic_sbdf[idx].sbdf = PCI_SBDF( seg, bus, dev, func );
>
> Ex: pdev_type() in xen/drivers/passthrough/pci.c
>
> Can you please double check in the modified code?
>
> >       ioapic_sbdf[idx].id = id;
> >       ioapic_sbdf[idx].cmdline = true;
> >
> > @@ -734,8 +733,7 @@ static int __init cf_check parse_ivrs_hpet(const char *str)
> >           return -EINVAL;
> >
> >       hpet_sbdf.id = id;
> > -    hpet_sbdf.bdf = PCI_BDF(bus, dev, func);
> > -    hpet_sbdf.seg = seg;
> > +    hpet_sbdf.sbdf = PCI_SBDF( seg, PCI_BDF(bus, dev, func) );
>                          ^^
>                          e.g. here it can be simplified too.
You are right, PCI_SBDF(sef, bus, dev, func) works here and above. Will 
resend.
> > @@ -1139,9 +1135,9 @@ static int __init cf_check parse_ivrs_table(struct 
> > acpi_table_header *table)
> >                   return -ENXIO;
> >           }
> >
> > -        if ( !ioapic_sbdf[idx].seg &&
> > +        if ( !ioapic_sbdf[idx].sbdf.seg &&
> >                /* SB IO-APIC is always on this device in AMD systems. */
> > -             ioapic_sbdf[idx].bdf == PCI_BDF(0, 0x14, 0) )
> > +             ioapic_sbdf[idx].sbdf.bdf == PCI_BDF(0, 0x14, 0) )
> Looks like something like the following should work:
>
>            if ( ioapic_sbdf[idx].sbdf.sbdf == PCI_SBDF(0, 0, 0x14, 0).sbdf )
>
> What do you think?
Will replace this one as well.

Thank you!