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
