On 09/27/17 12:06, Marcel Apfelbaum wrote: > Hi Laszlo, > > On 20/09/2017 14:01, Laszlo Ersek wrote: >> 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, >> > > SeaBIOS will not allow (0,0) values for pref32 and pref64 fields. > More than that, 0 values are not being taken in consideration. > > We may say is a bug and fix it. But taking a step back, we need the > hints to reserve *more* mem range, I am not sure 0 values are > interesting for mem reservation. Also is also only a hint, > the firmware should decide. > > Do we have a concrete scenario where would we want to use (0, non-zero) > or (non-zero, 0) ? (ask for 32/64 prefetch) > > Our current semantic is: "we give a hint to the Guest Firmware, > we don't decide. However, giving hints to both pref32 and pref64 > is wrong".
After having written the OVMF code for this, my understanding is clearer, and I agree that, for *prefetchable*, distinguishing -1 from 0 is not necessary (-1 means "firmware default (unspecified)", and 0 means "do not reserve"; and for prefetchable, the two things mean the same in OVMF). So the current QEMU code should be fine. > Not related: Using 0 value to disable IO works just fine! > No need for disable-IO-fwd property :) Great! Thanks! Laszlo