On Wed, Apr 13, 2022 at 01:48:14PM +0000, Bertrand Marquis wrote: > Hi Jan, > > > On 11 Apr 2022, at 10:40, Jan Beulich <jbeul...@suse.com> wrote: > > > > There's no good reason to use these when we already have a pci_sbdf_t > > type object available. This extends to the use of PCI_BUS() in > > pci_ecam_map_bus() as well. > > > > No change to generated code (with gcc11 at least, and I have to admit > > that I didn't expect compilers to necessarily be able to spot the > > optimization potential on the original code). > > > > Signed-off-by: Jan Beulich <jbeul...@suse.com> > > --- > > Note that the Arm changes are "blind": I haven't been able to spot a way > > to at least compile test the changes there; the code looks to be > > entirely dead. > > > > --- a/xen/arch/arm/pci/ecam.c > > +++ b/xen/arch/arm/pci/ecam.c > > @@ -28,8 +28,7 @@ void __iomem *pci_ecam_map_bus(struct pc > > container_of(bridge->ops, const struct pci_ecam_ops, pci_ops); > > unsigned int devfn_shift = ops->bus_shift - 8; > > void __iomem *base; > > - > > - unsigned int busn = PCI_BUS(sbdf.bdf); > > + unsigned int busn = sbdf.bus; > > > > if ( busn < cfg->busn_start || busn > cfg->busn_end ) > > return NULL; > > @@ -37,7 +36,7 @@ void __iomem *pci_ecam_map_bus(struct pc > > busn -= cfg->busn_start; > > base = cfg->win + (busn << ops->bus_shift); > > > > - return base + (PCI_DEVFN2(sbdf.bdf) << devfn_shift) + where; > > + return base + (sbdf.df << devfn_shift) + where; > > I think this should be sbdf.bdf instead (typo you removed the b).
I don't think so, notice PCI_DEVFN2(sbdf.bdf) which is extracting the devfn from sbdf.bdf. That's not needed, as you can just get the devfn directly from sbdf.df. Or else the original code is wrong, and the PCI_DEVFN2 shouldn't be there. Thanks, Roger.
