Re: [edk2-devel] [PATCH v3 1/6] MdeModulePkg/PciHostBridge: io range is not mandatory

["Gerd Hoffmann" <kraxel@redhat.com>]

On Wed, Apr 13, 2022 at 01:44:55PM +0000, Abner Chang wrote:
> 
> 
> > -----Original Message-----
> > From: Gerd Hoffmann <kraxel@redhat.com>
> > Sent: Wednesday, April 13, 2022 4:12 PM
> > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> > <abner.chang@hpe.com>
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>; Jordan Justen <jordan.l.justen@intel.com>;
> > Jian J Wang <jian.j.wang@intel.com>; Ray Ni <ray.ni@intel.com>; Hao A Wu
> > <hao.a.wu@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Pawel Polawski <ppolawsk@redhat.com>;
> > Oliver Steffen <osteffen@redhat.com>; Ard Biesheuvel <ardb@kernel.org>
> > Subject: Re: [edk2-devel] [PATCH v3 1/6] MdeModulePkg/PciHostBridge: io
> > range is not mandatory
> > 
> >   Hi,
> > 
> > > > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > > > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > > > index b20bcd310ad5..51a3b987967f 100644
> > > > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > > > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > > > @@ -1085,6 +1085,9 @@ NotifyPhase (
> > > >                RootBridge->ResAllocNode[Index].Base   = BaseAddress;
> > > >                RootBridge->ResAllocNode[Index].Status = ResAllocated;
> > > >                DEBUG ((DEBUG_INFO, "Success\n"));
> > > > +            } else if (Index == TypeIo) {
> > > How do we tell the BaseAddress is set to UINT64 because "(BaseAddress <
> > Limit)" or "gDS->AllocateMemorySpace()" returns error in AllocateResource()
> > for TypeIo ?
> > 
> > Does the reason matter?
> > 
> > io resources are optional, so if there is no io address space available
> > it should not be a fatal error, no matter what the root cause is.
> If the device requires I/O resource however the io address space is not available, shouldn't this an error? 

How do you figure that?

The PCIe spec requires devices being fully functional without io address
space resources, so in theory this case should not exist.  In practice
things are not that simple unfortunately.  But the pure presence of an
io bar doesn't imply it is actually required.

I think we can't do much about that at this point.  An actual driver for
the device which has more knowledge about the device would be in a
better position to figure whenever not having io resources is a fatal
error or not.

> > > Is "else if (Index == TypeIo  && RootBridge->Io.Base == MAX_UINT64)"
> > more reliable? Or I missed the code logic here?
> > 
> > I think it is not needed, but if you think it is better that way I can
> > change it.
> This is more clear to reader. That means that we don't have to treat it as an error on the TypeIo if the base address for it is MAX_UINT64.

Ok, I'll change it.

take care,
  Gerd