Regards, Ray
>-----Original Message----- >From: Laszlo Ersek [mailto:[email protected]] >Sent: Wednesday, February 24, 2016 5:40 PM >To: Ni, Ruiyu <[email protected]>; Marcel Apfelbaum <[email protected]> >Cc: Justen, Jordan L <[email protected]>; [email protected]; >Tian, Feng <[email protected]>; Fan, Jeff <[email protected]>; Marcel >Apfelbaum (GMail address) <[email protected]> >Subject: Re: [edk2] [Patch V4 4/4] MdeModulePkg: Add generic >PciHostBridgeDxe driver. > >On 02/24/16 03:46, Ni, Ruiyu wrote: >> Laszlo, >> >> PciBus summarize all the resource requirements for a root bridge, >> and submit the resource requirement summary to PciHostBridge driver. >> So the allocations occur for each root bridge, but not for each BAR. >> >> >> For the Add[Io/Memory]Space operations, I see two options: >> 1. A flag to tell PciHostBridge the resources have been already added >> by platform. PciHostBridge only adds the resources when the flag is >> not set. >> >> 2. PciHostBridge guarantees the resources reported by each root >> bridge have been added. Part of the resources may have been added. >> PciHostBridge adds the remaining part. >> >> I prefer option 2 because it doesn't need an additional flag - >> additional flag means additional interface. >> >> You strongly prefer option 1? > >No, my preference for option 1 is not strong. Option 2 can be certainly >accommodated in OvmfPkg, so if option 2 is your preference, I suggest we >go with that. > >> For the Allocate[Io/Memory]Space operation, I think we might be aligned: >> >> For each root bridge (not each BAR, but I think BAR or root bridge >> doesn't bother you), > >Actually, that difference is crucial. See below. > >> PciHostBridge calls Allocate[Io/Memory]Space to >> allocate the required resources. If allocation fails, >> PciHostBridgeResourceConflict() is called to inform platform. >> >> But we do have some small differences for the allocation: >> >> The current allocation algorithm uses EfiGcdAllocateAddress. You could >> check PciHostBridge.c: >> >> AllocateResource() function for the detailed implementation. It's >> different from what you suggested: >> >> EfiGcdAllocateMaxAddressSearchTopDown. Do you think it matters? > >EfiGcdAllocateAddress is correct if the driver allocates memory space >(or IO space) for the *entire root bridge aperture*. > >My suggestion with EfiGcdAllocateMaxAddressSearchTopDown was for the >allocation of an *individual BAR*. > >So, if the driver is supposed to allocate the *entire bridge aperture* >in one fell swoop, then EfiGcdAllocateAddress is correct. > >*However*, this only takes us back to the original question of >overlapping apertures, between separate bridges! > >Consider the following example. You have two root bridges, and the >32-bit MMIO aperture for each is [3.0GB, 3.5GB). > >(i) According to option #2 above, the *first* gDS->AddMemorySpace() >operation will ensure that the [3.0GB, 3.5GB) range *exists* as MMIO in >the GCD memory space map. Fine. > >(ii) Then the logic in PciHostBridgeDxe will determine, for the *second* >root bridge, according to option #2, that the [3.0GB, 3.5GB) range >*already* exists as MMIO in the GCD memory space map, without gaps. So >there will be no need for another gDS->AddMemorySpace() call. This is >fine as well. > >(iii) Then the driver *allocates* the [3.0GB, 3.5GB) MMIO range for the >*first* root bridge, with the gDS->AllocateMemorySpace() call. It works >perfectly. No. The driver does *NOT* allocate [3G, 3.5G) for root bridge #0. It just allocates the resource that is really required by root bridge #0. for example, [3G, 3.3G). > >(iv) Then the driver tries to allocate the same [3.0GB, 3.5GB) MMIO >range for the *second* root bridge, with another >gDS->AllocateMemorySpace() call. This fails, because that range is no >longer free! Hence the driver calls PciHostBridgeResourceConflict(). > When the real requirement for root bridge #1 is <= 0.2G, the allocation still succeeds. Though the driver uses EfiGcdAllocateAddress type, it tries different base address to allocate. So it ultimately searches to 3.3G as a valid base address for allocation. When the real requirement for root bridge #1 is > 0.2G, the allocation fails. That's a fatal error which will call PciHostBridgeResourceConflict(). >Unfortunately, the allocation failure in (iv) would be a bug in the >driver -- the two root bridges want to *share* the [3.0GB, 3.5GB) MMIO >range between them, and the driver should permit it. It is not a >resource conflict between the root bridges, but a valid configuration on >QEMU. > >Thanks >Laszlo > > > >> >> >> >> Regards, >> >> Ray >> >> >> >> *From:*Laszlo Ersek [mailto:[email protected]] >> *Sent:* Tuesday, February 23, 2016 4:49 PM >> *To:* Ni, Ruiyu <[email protected]>; Marcel Apfelbaum ><[email protected]> >> *Cc:* Justen, Jordan L <[email protected]>; >> [email protected]; Tian, Feng <[email protected]>; Fan, Jeff >> <[email protected]>; Marcel Apfelbaum (GMail address) >> <[email protected]> >> *Subject:* Re: [edk2] [Patch V4 4/4] MdeModulePkg: Add generic >> PciHostBridgeDxe driver. >> >> >> >> (Adding Marcel's gmail address -- I think that's the one he is >> subscribed to edk2-devel with.) >> >> On 02/23/16 06:13, Ni, Ruiyu wrote: >>> Marcel, >>> I see two requirements from your mail: >>> 1. non-continuously resources: root bridge #1 uses [2G, 2.4G) and [2.8G, >3G) while >>> root bridge #2 uses [2.4G, 2.8G) >>> 2. sharable resources among root bridges: All root bridges in same pci >segment >>> can share one common range of resources. >>> >>> Requirement #1 is not supported by MdeModulePkg/PciBus driver and I >guess >>> it's not the urgent requirement and doesn't block OVMF PciHostBridge >porting. >> >> That's correct. >> >>> Requirement #2 can be interpreted as it's valid when the resources claimed >by >>> different root bridges overlap. No matter which segment they belong to. >>> >>> The overlap can be like root bridge #1 claims [2G, 2.4G) while root bridge >#2 >>> claims [2.2G, 2.6G) -- [2.2G, 2.4G) is shared by both root bridges. >>> In such case, PCI devices under root bridge #1 can only use resources [2G, >2.4G) >>> and root bridge #2 can only use [2.2G, 2.6G). GCD services can guarantee >there >>> is no resource conflict -- if [2.2G, 2.3G) is used by one device under root >bridge #1, >>> it won't be used by device under root bridge #2. >> >> Correct as well. >> >>> >>> An extreme case is both root bridges claim [2G, 3G) which is the OVMF >case. >> >> Right. >> >>> So the change to PciHostBridgeDxe can be: >>> 1. Checks whether the resources claimed by the root bridges are already >added, >>> and call AddMemorySpace/AddIoSpace for those resource ranges which >haven't >>> been added. >> >> Do you mean that PciHostBridgeDxe would determine "gaps" in the memory >> space map / IO space map, by calling GetMemorySpaceMap() and >> GetIoSpaceMap(), and taking the intersection of each map with the >> bridge's corresponding aperture(s)? >> >> I can imagine this, but I don't readily understand how this is useful. >> Probably as a basis for the next step: >> >>> 2. Call AllocateMemorySpace/AllocateIoSpace to occupy these resources >in GCD. >>> The Allocation shouldn't fail, otherwise it's a fatal error and >PciHostBridgeDxe driver >>> will assert and exit. >> >> I understand how step (1) is the basis for step (2) -- one can only >> allocate from the memory & IO space maps what has been added to them >> earlier --, but I don't understand how step (2) is supposed to help with >> overlapping apertures. Assuming the allocation is made with >> EfiGcdAllocateAddress, then a conflict for the shared range [2.2G, 2.4G) >> -- using your example at the top -- will be reported. >> >> Furthermore, I'm not sure why the entire aperture for a root bridge >> should be allocated up-front. I thought the allocations occur only for >> specific BARs -- is that incorrect? >> >> For OVMF's purposes, the following would be ideal: >> >> (a) PciHostBridgeDxe should let the platform populate the memory and IO >> space maps as appropriate. This could happen in several ways, for >> example: (i) creation of resource descriptor HOBs during PEI, from which >> the DXE core would prime the memory and IO space maps, or (ii) in the >> constructor of the PciHostBridgeLib instance that the platform links >> into PciHostBridgeDxe. >> >> (b) no explicit overlap checks whatsoever between root bridge apertures >> >> (c) when a BAR is being allocated, allocate the right type of resource >> with GcdAllocateType = EfiGcdAllocateMaxAddressSearchTopDown. >> >> Start the top-down search from the highest address in the bridge's >> aperture. If the allocation fails completely (i.e., no free room from >> the aperture's top, down to zero), report failure. >> >> If the allocation succeeds, but the allocation address is below the base >> address of the bridge's aperture, *undo* the allocation, and report >> failure again. >> >> Otherwise, the BAR allocation succeeds. >> >> >> The above strategy would give full flexibility to the platform, and it >> would ensure that no BAR is allocated outside the respective bridge >> aperture. It would handle overlaps between apertures automatically. >> >> It would also handle the case transparently when a bridge aperture is a >> *proper superset* of what is available in the GCD memory space map. For >> example, assume the GCD memory space map only has MMIO added at >[3.0G, >> 3.5G), and "nothing" at [2.0G, 3.0G) -- a gap. Then a bridge aperture of >> [2.0G, 3.5G) would "just work", without changing the GCD memory space >> map. GCD would satisfy BAR allocations from [3.0G, 3.5G), and any >> addresses allocated from that range would *also* satisfy the bridge >> aperture, automatically. >> >> Do you see any problems with the idea in (a) through (c)? >> >> Thank you >> Laszlo >> _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
