What I'd like is reverting f26740c61a57f and fix that some other way so
I don't have to duplicate size check in board code as can be seen in the
patchew link above but could just call spd_data_generate() to do its
job. This was discussed at the time that patch was in review you can
read it here:
http://patchwork.ozlabs.org/project/qemu-devel/patch/20200420132826.8879-3-armbru@redhat.com/
My points were not really considered then, now that I have another use
case maybe it could be revisited and fixed. What I want is to be able to
call spd_data_generate() from board code with whatever siz�© (the
board does not need to know about SPD limits and so cannot pre-check the
size) and the function should return the largest possible size SPD and
some indication if the size was not used completely. If Error cannot be
used for this, return the message or error some other way but let the
board code decide if it wants to abort or it can use the smaller SPD. Do
not assert in the helper function. Maybe the DIMM type fix up can be
dropped and only keep the size fix up so then we don't need to use error
twice, the board could call the function again if a different type is
also acceptable, since only sam460ex would need this I can do that there
for type fixup and call spd_data_generate() again with DDR2 if first one
with DDR could not fit all ram. But at least the asserts should be
dropped for this and the size check brought back. Then adding SPD to
mac_oldworld could also be done by calling spd_data_generate() instead
of duplicating the checks this function does anyway. This board has
three slots so if user says -m 1400 it would call spd_data_generate()
with 1400 first, get back 512 SPD that it adds to first slot then calls
spd_data_generate() again with 888, gets 512 again that it adds to 2nd
slot and calls spd_data_generate() for last slot with 376 which would
give 256 and 120 remaining that it may warn the user about but still
continue because the SPD data is only used by a ROM from real hardware
(that may be used for compatibility with some software) but the default
OpenBIOS disregards SPD data and would still use 1400 so it's not an
error to abort on. Simply if using a firmare ROM then only 1280 MB of
the 1400 will be available due to its limitations but that's not a
reason to force users to change their command line. Printing a warning
is enough to hint they may use different value but aborting without an
error message on an assert which is the current situation is not really
a user friendly way.
I just noticed we have a MachineClass::fixup_ram_size() handler. There
is only one implementation (on s390x) which does a bit the opposite:
If the user asks for -m 1400, it will pad to a valid physical size,
so in your example to 1472 MiB. Then the guest can use only 1400 if it
is happy with it. 72 MiB are wasted, but this is still better than the
576 MiB wasted if we were using 2 GiB instead ;)
See commit 5c30ef937f5:
static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
{
/* same logic as in sclp.c */
int increment_size = 20;
ram_addr_t newsz;
while ((sz >> increment_size) > MAX_STORAGE_INCREMENTS) {
increment_size++;
}
newsz = sz >> increment_size << increment_size;
if (sz != newsz) {
qemu_printf("Ram size %" PRIu64 "MB was fixed up to %" PRIu64
"MB to match machine restrictions. "
"Consider updating "
"the guest definition.\n", (uint64_t) (sz / MiB),
(uint64_t) (newsz / MiB));
}
return newsz;
}