Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'

[Igor Mammedov]

On Fri, 20 Aug 2021 18:06:30 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 8/20/21 6:03 PM, Igor Mammedov wrote:
> > On Fri, 20 Aug 2021 17:47:01 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >   
> >> On 20.08.21 17:44, Igor Mammedov wrote:  
> >>> On Fri, 20 Aug 2021 15:39:27 +0100
> >>> Peter Maydell <peter.maydell@linaro.org> wrote:
> >>>     
> >>>> On Fri, 20 Aug 2021 at 15:34, David Hildenbrand <david@redhat.com> 
> >>>> wrote:    
> >>>>>
> >>>>> On 20.08.21 16:22, Bin Meng wrote:    
> >>>>>> Hi Philippe,
> >>>>>>
> >>>>>> On Fri, Aug 20, 2021 at 10:10 PM Philippe Mathieu-Daudé
> >>>>>> <philmd@redhat.com> wrote:    
> >>>>>>>
> >>>>>>> Hi Bin,
> >>>>>>>
> >>>>>>> On 8/20/21 4:04 PM, Bin Meng wrote:    
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> The following command used to work on QEMU 4.2.0, but is now broken
> >>>>>>>> with QEMU head.
> >>>>>>>>
> >>>>>>>> $ qemu-system-arm -M xilinx-zynq-a9 -display none -m 40000000
> >>>>>>>> -nographic -serial /dev/null -serial mon:stdio -monitor null -device
> >>>>>>>> loader,file=u-boot-dtb.bin,addr=0x4000000,cpu-num=0
> >>>>>>>> qemu-system-arm: cannot set up guest memory 'zynq.ext_ram': Cannot
> >>>>>>>> allocate memory    
> >>>>    
> >>>>> -m 40000000
> >>>>>
> >>>>> corresponds to 38 TB if I am not wrong. Is that really what you want?   
> >>>>>  
> >>>>
> >>>> Probably not, because the zynq board's init function does:
> >>>>
> >>>>      if (machine->ram_size > 2 * GiB) {
> >>>>          error_report("RAM size more than 2 GiB is not supported");
> >>>>          exit(EXIT_FAILURE);
> >>>>      }
> >>>>
> >>>> It seems a bit daft that we allocate the memory before we do
> >>>> the size check. This didn't use to be this way around...
> >>>>
> >>>> Anyway, I think the cause of this change is commit c9800965c1be6c39
> >>>> from Igor. We used to silently cap the RAM size to 2GB; now we
> >>>> complain. Or at least we would complain if we hadn't already
> >>>> tried to allocate the memory and fallen over...    
> >>>
> >>> That's because RAM (as host resource) is now separated
> >>> from device model (machine limits) and is allocated as
> >>> part of memory backend initialization (in this case
> >>> 'create_default_memdev') before machine_run_board_init()
> >>> is run.
> >>>
> >>> Maybe we can consolidate max limit checks in
> >>> create_default_memdev() by adding MachineClass::max_ram_size
> >>> but that can work only in default usecase (only '-m' is used).    
> >>
> >> We do have a workaround for s390x already: mc->fixup_ram_size
> >>
> >> That should be called before the memory backend is created and seems to 
> >> do just what we want, no?  
> > 
> > it's there for compat sake only if I recall correctly,
> > there should be no fixups ever.
> > If user asks for nonsence, QEMU should error out and force
> > user to correct CLI  
> 
> Agreed, but this would be cheaper to run the checks *before*
> allocating the resources ;)
Agreed,
Only it will work for default usecase only as I described above.

> 
> > (fixups were one of items that were in
> > the way of splitting guest RAM into backend/frontend model)
> >   
>