Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by default for pcie-root-port

[Laszlo Ersek]

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