Re: [PATCH 2/4] smbus: Fix spd_data_generate() error API violation

[Markus Armbruster]

Philippe Mathieu-Daudé <address@hidden> writes:

> On 4/24/20 11:45 AM, Markus Armbruster wrote:
>> BALATON Zoltan <address@hidden> writes:
>>> On Tue, 21 Apr 2020, Markus Armbruster wrote:
> [...]
>>>>
>>>> Here's what I actually disagree with:
>>>>
>>>> 1. QEMU warning the user about its choice of memory type, but only
>>>> sometimes.  Why warn?  There is nothing wrong, and there is nothing the
>>>> user can do to avoid the condition that triggers the warning.
>>>
>>> The memory size that triggers the warning is specified by the user so
>>> the user can do someting about it.
>>
>> There is no way to trigger the warning.  If we dropped PATCH 1 instead
>> of fixing it as I did in v2, then the only way to trigger the warning is
>> -M sam460ex -m 64 or -m 32, and the only way to avoid it is to don't do
>> that.
>>
>> Why would a user care whether he gets DDR or DDR2 memory?
>
> To use a different firmware code path!

Let's see how that works out for users.

Assume machine foobar needs a non-default firmware for "small" memory
sizes, such as -m 64.

Alice doesn't know.  She starts QEMU like this:

    $ qemu-system-foo -M foobar -m 64

It hangs early in boot.  Not good.

Except the warning from spd_data_generate() rides to her rescue!

    qemu-system-foo: warning: Memory size is too small for SDRAM type, 
adjusting type

Since Alice is rather sharp, the warning makes her realize immediately
that she needs to use different firmware, like this:

    $ qemu-system-foo -M foobar -m 64 -bios roms/foobar/firmware-small.bin

Okay, I'm done telling my fairy tale.

If you want helpful, make it work out of the box: default to the
firmware that actually works, don't make users guess which one they
need.

But one more time: this is all theoretical.  We're talking about unused,
broken code.  If you want to keep it, please add users, and fix it up.