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

["Abner Chang" <abner.chang@hpe.com>]

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, April 19, 2022 9:15 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
> 
> 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.
Ok.
> 
> > > > 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.
Thanks
Abner
> 
> take care,
>   Gerd