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

[BALATON Zoltan]

On Mon, 29 Jun 2020, Markus Armbruster wrote:
> BALATON Zoltan <balaton@eik.bme.hu> writes:
> > On Sat, 27 Jun 2020, Markus Armbruster wrote:
> > > Quick reply without having thought through the issues at all: I'm not
> >
> > Does that mean you'll reply later with more detail or this is all you
> > had to say about this? (Just to know if I should wait for another
> > reply.)
>
> Not sure I can find the time before the soft freeze.  Best not to wait
> for me.

OK I'm not sure my mac_oldworld patches can be finished or would be merged 
before the freeze anyway and this was already broken in 5.0 so it's not 
that urgent now but I'll need this in the future so eventually should 
find some way to come to an agreement.

> > > opposed to you doing work to enable additional or even arbitrary memory
> > > sizes where these actually work.  I'm first and foremost opposed to me
> > > wasting time on "improving" code that is not used for anything.  That's
> > > why I dumbed down spd_data_generate().
> >
> > It was used by sam460ex until moving ram allocation to memdev broke it.
> >
> > > Secondly, I'm opposed to QEMU "correcting" user configuration.  I want
> > > QEMU do exactly what it's told, and fail with a clear error message when
> > > that is not possible.  The error message may include hints for the user
> > > on how to correct the configuration.
> >
> > I don't agree with that. It's already hard enough for non-expert users
> > to figure out the needed command line switches, making that even
> > harder by throwing back an error for everything that could work just
> > not exactly specified is needlessly annoying them further. To the
> > point of chasing them away from using QEMU. A lot of people prefer
> > VMWare or VirtualBox for this reason and only try QEMU if there's no
> > other way.
>
> We don't have to agree on everything.  I'm not the QEMU CLI dictator.
> The status quo is pretty clear, though:
>
>    $ qemu-system-ppc64 -help
>    [...]
>    -m [size=]megs[,slots=n,maxmem=size]
>                    configure guest RAM
>                    size: initial amount of guest memory
>    [...]
>
> It says "Initial amount of guest memory", not "Approximate amount of
> guest memory" or something like that.
>
> If we decide we want to change it from "Initial amount of guest memory"
> to some "do what I mean" behavior, then that behavior needs to be
> documented.

This is sufficiently vague that it says "initial" which to me means it's 
not absolute and can change while the VM is running so a change due to fix 
up fits in that in my opinion :-)

> Moreover, if DWIM is appropriate for one machine, it's probably
> appropriate for all of them.  The CLI should be as consistent as we can
> make it across machines.

That's the point. Rummimg e.g. qemu-system-x86_64 -m 1000 does not abort 
but runs the VM with an odd RAM size even though that's not possible on 
real hardware. Other machines should behave the same, within their limits: 
for sam460ex that means we need to truncate memory size to largest valid 
value becuause of SoC limits, for mac_oldworld that means with OpenBIOS it 
will see all RAM and with firmware ROM somewhat less. I've implemented 
that originally both for consistency and user convenience but this was 
"cleaned up" afterwrds and also made impossible to implement again without 
duplicating code in boards or reverting to some previous state and fixing 
the problems in a way that allows my use case as well.

Regards,
BALATON Zoltan