On 09/20/17 13:16, Marcel Apfelbaum wrote: > On 20/09/2017 14:01, Laszlo Ersek wrote: >> On 09/20/17 09:42, Marcel Apfelbaum wrote:
>>> The implementation issue might be that Guest Firmware would >>> take the alignment as default value for an empty root port. >>> (maybe take it as a bug and "solve" it?) >> >> Hm, I don't get this. What do you mean? >> > > Empty PCI Express Root Port with io_reserve = 0 should end with > no IO ports range, right? Yes. > But there is some code in SeaBIOS that "aligns" the IO range to 4K, > so we may end up with a 4K range even if io_reserve=0. Ah. OK, if this makes a difference, please report your findings :) >>>> (d) pci_bridge_qemu_reserve_cap_init() should be updated accordingly >>>> (i.e., a conflict exists if both mem_pref_32_reserve and >>>> mem_pref_64_reserve are *positive*), >>>> >>> >>> I thought we have this check already. >> >> We have a check like this, but it doesn't use the exact condition that >> I'm suggesting above (emphasis on *positive*). We now have >> >> if (mem_pref_32_reserve != (uint32_t)-1 && >> mem_pref_64_reserve != (uint64_t)-1) { >> error_setg(errp, >> >> but after introducing the zero value, this is going to be too strict. >> Consider all the possible combinations: >> >> * (-1; -1): default fw behavior, check is OK >> >> * (-1; 0): fw sticks with the default for the first component, picks >> "no reservation" for the second component, check is OK >> >> * (-1; >0): fw can treat this identically to ( 0; >0) -- see below. This >> is justified because, while the first component says "use >> the default", the entire situation of having such a >> capability is new, so the firmware can introduce new ways >> for handling it. The check passes, which is good. >> >> * ( 0; 0): check reports error, but the firmware can handle this case >> fine (no reservation for either component) >> >> * ( 0; >0): check reports error, but the fw can handle this (no >> reservation for the first component, specific reservation >> for the second) >> >> * (>0; >0): conflict, check is OK >> >> (I'm skipping the description of the inverted orderings, such as (0;-1), >> they behave similarly.) >> >> So we need to accept 0 as "valid" for either component: >> >> if (mem_pref_32_reserve > 0 && >> mem_pref_32_reserve < (uint32_t)-1 && >> mem_pref_64_reserve > 0 && >> mem_pref_64_reserve < (uint64_t)-1) { >> error_setg(errp, >> > > Got it, you are right. How did we end up with something > complicated again :( ? > (complicated = needs documentation and not straight forward) Haha, are you serious? :) *Nothing* is simple in QEMU; nothing at all. :) In general, software is complex because of feature requests, and QEMU caters to a million different (and conflicting) use cases, so you get complexity. (I know this is trivial, but it sure felt good to write down! :) ) >>>> Second, when determining the reservations in OVMF, I would like to look >>>> only at the capability fields, and not do a read-write-read-write >>>> quadruplet to the IO base/limit registers. Do you agree? >>>> >>> >>> Well, as stated before, the semantics for the hint is >>> max(hint, <sum of all IO/MEM ranges for devices behind the bridge>). >>> If you can follow it, that would be OK. >> >> In OVMF there are two separate questions: >> - how to report the requested reservations, >> - how to act upon the reported values. >> >> The first question pertains to "platform code", i.e., what I'm going to >> implement under OvmfPkg. The second question pertains to "universal >> code" (under "MdeModulePkg/Bus/Pci/PciBusDxe"), which is a lot harder to >> modify, because it is shipped on a wide range of physical platforms too. >> >> Again, my understanding is that PciBusDxe implements the "max" semantics >> that you describe (pertaining to the 2nd question). >> >> My interest is in figuring out the 1st question now, that is, where I >> should take the information from that goes into the "reservation >> description" that PciBusDxe understands. My preference would be to look >> only at the PCIE port in question (i.e. no other PCI devices at all), >> and only at said capability in the config space of the PCIE port. >> > > Sounds OK. Thank you! Looks like I can start writing code for OVMF when I find the time. Thanks! Laszlo