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: 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? 
> 
> > 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.
Abner
> 
> > > +              /* optional on PCIe */
> > We should use double back slash for the comment in the function if my
> understanding of the coding standard is correct.
> 
> I'll fix in v4.
> 
> take care,
>   Gerd