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. (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(). 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
