On 2/22/19 9:09 AM, Laszlo Ersek wrote: > On 02/21/19 21:07, Alex Bennée wrote: >> Laszlo Ersek <[email protected]> writes: >> >>> On 02/21/19 19:48, Alex Bennée wrote: >>>> It looks like there was going to be code to check we had some sort of >>>> alignment so lets replace it with an actual check. This is a bit more >>>> useful than the enigmatic "failed to read the initial flash content" >>>> when we attempt to read the number of bytes the device should have. >>>> >>>> This is a potential confusing stumbling block when you move from using >>>> -bios to using -drive if=pflash,file=blob,format=raw,readonly for >>>> loading your firmware code. To mitigate that we automatically pad in >>>> the read-only case. >>>> >>>> Signed-off-by: Alex Bennée <[email protected]> >>>> >>>> --- >>>> v3 >>>> - tweak commit title/commentary >>>> - use total_len instead of device_len for checks >>>> - if the device is read-only do the padding for them >>>> - accept baking_len > total_len (how to warn_report with NULL *errp?) >>>> --- >>>> hw/block/pflash_cfi01.c | 28 +++++++++++++++++++++------- [...]>>>> + total_len = backing_len; >>>> + } else { >>>> + error_setg(errp, "device(s) needs %" PRIu64 " bytes, " >>> >>> (4) not too important, I'm just curious: why the optional plural? >> >> I discovered the difference between device_len and total_len and found >> (for some reason) the efivars came out as multiple devices. > > It is true that the executable code is in one chip, and the UEFI > varstore in another (in the most common & most recommended setup > anyway); however, the varstore itself doesn't need multiple chips, and > more importantly, I think the realize function of any given single chip > should only report errors about that one chip. (Unless we have some
I agree with László. > higher level invariant binding the chips together, but I'm unaware of > any such in this case.) > > If I understand correctly, when we set the error here, QEMU will produce > an error report that is tied to the specific pflash chip / command line > option that triggered the error. That looks like the right thing to me. > > Again I don't really insist on plural vs. singular here, I just wanted > to understand your train of thought. > > Thanks! > Laszlo
