Re: [PATCH 43/86] hppa: drop RAM size fixup

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 1/2/20 4:35 PM, Igor Mammedov wrote:
> On Thu, 2 Jan 2020 15:45:55 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> On 1/2/20 3:41 PM, Igor Mammedov wrote:
>>> On Thu, 2 Jan 2020 12:31:58 +0100
>>> Helge Deller <deller@gmx.de> wrote:
>>>    
>>>> On 31.12.19 16:44, Philippe Mathieu-Daudé wrote:
>>>>> On 12/31/19 2:03 PM, Igor Mammedov wrote:
>>>>>> If user provided non-sense RAM size, board will complain and
>>>>>> continue running with max RAM size supported.
>>>>>> Also RAM is going to be allocated by generic code, so it won't be
>>>>>> possible for board to fix things up for user.
>>>>>>
>>>>>> Make it error message and exit to force user fix CLI,
>>>>>> instead of accepting non-sense CLI values.
>>>>>>
>>>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>>>>> ---
>>>>>>     hw/hppa/machine.c | 3 ++-
>>>>>>     1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
>>>>>> index 5d0de26..25f5afc 100644
>>>>>> --- a/hw/hppa/machine.c
>>>>>> +++ b/hw/hppa/machine.c
>>>>>> @@ -92,7 +92,8 @@ static void machine_hppa_init(MachineState *machine)
>>>>>>           /* Limit main memory. */
>>>>>>         if (ram_size > FIRMWARE_START) {
>>>>>> -        machine->ram_size = ram_size = FIRMWARE_START;
>>>>>> +        error_report("RAM size more than %d is not supported", FIRMWARE_START);
>>>>>> +        exit(EXIT_FAILURE);
>>>>>
>>>>> $ qemu-system-hppa -m 3841m
>>>>> qemu-system-hppa: invalid accelerator kvm
>>>>> qemu-system-hppa: falling back to tcg
>>>>> qemu-system-hppa: RAM size more than -268435456 is not supported
>>>>>
>>>>> Instead of using qemu_strtosz_MiB on FIRMWARE_START or unsigned format, we can simply use "RAM size more than 3840m is not supported". Is that OK with you?
>>>>
>>>> I don't really like that change.
>>>>
>>>> We currently only emulate a 32-bit system, and for those 4GB is the maximum.
>>>> So, if I start my machine with "qemu-system-hppa -m 4G", the current code
>>>> then automatically uses the maximum possible of 3841MB (which is limited by
>>>> firmware start address).
>>>> I don't expect users to know the excact 3841MB number.
>>> It's annoying to see a error where it used to work before
>>> with no matter what -m value user have used.
>>>
>>> But error message tells exact max size one could use,
>>> so user doesn't have to know max, just fix CLI with provided value.
>>>    
>>>> Even on a phyiscal machine you can only add DIMMs of sizes 2GB, 3GB or 4GB,
>>>> but not "3841MB".
>>>>
>>>> So, I think that patch is - although it's more correct - not a
>>>> benefit for the end user.
>>> Sure thing that users dislike when we do breaking changes (removing legacy CLI
>>> options, fixups or adding error checks that weren't there before).
>>> But I'd choose correctness (and consistent codebase) vs convenience.
>>> (it's no like we are hiding max from user)
>>
>> Going from 3841MB to 4GB is not a breaking change, and 4GB is correct I
> under breaking change, I've meant exit with error when it used to work with
> any values.
> 
> But the point in removing fixups to make user aware of valid input and
> make clear what's going on when someone reads the code.
> 
> QEMU code is already too complex to read, so I'd avoid supporting values
> that are not really supported by current impl. just for the sake of convenience.
> (a wrapper script could do that RAM size clamping if it is really necessary)
> 
>> think. The Raspberry Pi machines do the same (allocate more RAM than the
>> CPU is able to access, and DMA is able to use this extra RAM).
> In current impl. raspi accepts RAM upto 1Gb maps all of it and reports
> that size to the guest. QEMU doesn't mask or waste anything.
> If we where to rise limit upto 4Gb, then there should be the code that
> would inform and allow guest utilize that.

For the raspi2/3, 16MB are lost and we don't complain neither change the 
ram_size, see the bcm2835-peripherals block overmapping the ram:

$ qemu-system-aarch64 -M raspi3 -monitor stdio -S
QEMU 3.1.1 monitor - type 'help' for more information
(qemu) info mtree
address-space: memory
   0000000000000000-ffffffffffffffff (prio 0, i/o): system
     0000000000000000-000000003fffffff (prio 0, ram): ram
     000000003f000000-000000003fffffff (prio 1, i/o): bcm2835-peripherals
       000000003f007000-000000003f007fff (prio 0, i/o): bcm2835-dma
       000000003f00b200-000000003f00b3ff (prio 0, i/o): bcm2835-ic
       000000003f00b800-000000003f00bbff (prio 0, i/o): bcm2835-mbox
       000000003f104000-000000003f10400f (prio 0, i/o): bcm2835-rng
       000000003f200000-000000003f200fff (prio 0, i/o): bcm2835_gpio
       000000003f201000-000000003f201fff (prio 0, i/o): pl011
       000000003f202000-000000003f202fff (prio 0, i/o): bcm2835-sdhost
       000000003f215000-000000003f2150ff (prio 0, i/o): bcm2835-aux
       000000003f300000-000000003f3000ff (prio 0, i/o): sdhci
       000000003fe05000-000000003fe050ff (prio 0, i/o): bcm2835-dma-chan15
     0000000040000000-00000000400000ff (prio 0, i/o): bcm2836-control