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

[Philippe Mathieu-Daudé]

On 1/2/20 3:12 PM, Igor Mammedov wrote:
> On Thu, 2 Jan 2020 13:06:33 +0100
> Philippe Mathieu-Daudé <address@hidden> wrote:
>
> > On 1/2/20 12:31 PM, Helge Deller 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 <address@hidden>
> > > > > ---
> > > > >     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.
> > > Even on a phyiscal machine you can only add DIMMs of sizes 2GB, 3GB or 4GB,
> > > but not "3841MB".
> >
> > Thanks for the explanation. This deserves a comment in the source file
> > IMHO (and displaying a warning to the user that the behavior is changed).
> >
> > I understand the CPU can't access this DRAM area because the ROM is
> > mapped there. What about other devices, can they do DMA access to it?
> >
> > Igor: If this complicates your series too much, I think we can directly
> > allocate up-to 4GiB and not worry about the 256MiB lost.
>
> Do you mean
> s/"RAM size more than %d is not supported"/"RAM size more than 4Gb is not 
> supported"/

Works for me! You can keep my R-b with this change, thanks.

> > > So, I think that patch is - although it's more correct - not a
> > > benefit for the end user.
> > >
> > > Helge
> > >