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

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

[Bin Meng]

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

Any ideas?

Regards,
Bin

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

[Philippe Mathieu-Daudé]

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
> 
> Any ideas?

Richard hit that recently too.

Can you provide:

cat /proc/sys/vm/overcommit_kbytes
cat /proc/sys/vm/overcommit_memory
cat /proc/sys/vm/overcommit_ratio

and

cat /proc/meminfo

(CommitLimit, Committed_AS)

and OOM messages.

Per David, 'you can trick QEMU in trying to work around
that issue, specifying a memory-backend-ram with "reserve=off"
as guest RAM.'

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

[Bin Meng]

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
> >
> > Any ideas?
>
> Richard hit that recently too.

I hit this when in the VM on Azure pipelines, but I was able to
reproduce this issue on my local machine.

>
> Can you provide:
>
> cat /proc/sys/vm/overcommit_kbytes
> cat /proc/sys/vm/overcommit_memory
> cat /proc/sys/vm/overcommit_ratio

$ cat /proc/sys/vm/overcommit_kbytes
0
$ cat /proc/sys/vm/overcommit_memory
0
$ cat /proc/sys/vm/overcommit_ratio
50

>
> and
>
> cat /proc/meminfo
>
> (CommitLimit, Committed_AS)

$ cat /proc/meminfo

CommitLimit:    12388820 kB
Committed_AS:    5019088 kB

> and OOM messages.

I did not see any OOM messages.

>
> Per David, 'you can trick QEMU in trying to work around
> that issue, specifying a memory-backend-ram with "reserve=off"
> as guest RAM.'

Regards,
Bin

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

[David Hildenbrand]

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
>>>
>>> Any ideas?
>>
>> Richard hit that recently too.
> 
> I hit this when in the VM on Azure pipelines, but I was able to
> reproduce this issue on my local machine.
> 
>>
>> Can you provide:
>>
>> cat /proc/sys/vm/overcommit_kbytes
>> cat /proc/sys/vm/overcommit_memory
>> cat /proc/sys/vm/overcommit_ratio
> 
> $ cat /proc/sys/vm/overcommit_kbytes
> 0
> $ cat /proc/sys/vm/overcommit_memory
> 0
> $ cat /proc/sys/vm/overcommit_ratio
> 50
> 
>>
>> and
>>
>> cat /proc/meminfo
>>
>> (CommitLimit, Committed_AS)
> 
> $ cat /proc/meminfo
> 
> CommitLimit:    12388820 kB
> Committed_AS:    5019088 kB
> 
>> and OOM messages.
> 
> I did not see any OOM messages.

-m 40000000

corresponds to 38 TB if I am not wrong. Is that really what you want?

How much main memory does your system have?

-- 
Thanks,

David / dhildenb

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

[Peter Maydell]

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...

-- PMM

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

[Philippe Mathieu-Daudé]

On 8/20/21 4:39 PM, Peter Maydell 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...

Ouch... I remember having tested -M raspi2 -m 8G etc... to verify
the error messages, but didn't noticed the memory was allocated.

static void qemu_init_board(void)
{
    MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);

    if (machine_class->default_ram_id && current_machine->ram_size &&
        numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
        create_default_memdev(current_machine, mem_path); // <- alloc
    }

    /* process plugin before CPUs are created ... */
    qemu_plugin_load_list(&plugin_list, &error_fatal);

    /* From here on we enter MACHINE_PHASE_INITIALIZED.  */
    machine_run_board_init(current_machine); // <- Machine::init()
                                             //    checks RAM size

    ...

$ qemu-system-x86_64 -m 1T
qemu-system-x86_64: cannot set up guest memory 'pc.ram': Cannot allocate
memory

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

[Igor Mammedov]

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).

However if user creates backend explicitly, there aren't any
clue about machine limits. We basically don't know what
backend is created for at the time it's initialized
(which includes RAM allocation, it might be created for VM's
RAM or ram/storage for some other device).

> 
> -- PMM
> 

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

[David Hildenbrand]

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?

-- 
Thanks,

David / dhildenb

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

[Philippe Mathieu-Daudé]

On 8/20/21 5:47 PM, David Hildenbrand 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?

Or maybe more explicit adding a MachineClass::check_ram_size() handler?

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

[Igor Mammedov]

On Fri, 20 Aug 2021 17:53:41 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 8/20/21 5:47 PM, David Hildenbrand 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?  
> 
> Or maybe more explicit adding a MachineClass::check_ram_size() handler?

On the first glance, just max_size field should be sufficient
with checking code being generic, which should remove code duplication
such checks introduce across tree. Is there a specific board for
which call back is 'must to have'?

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

[Philippe Mathieu-Daudé]

On 8/20/21 6:08 PM, Igor Mammedov wrote:
> On Fri, 20 Aug 2021 17:53:41 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> On 8/20/21 5:47 PM, David Hildenbrand 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?  
>>
>> Or maybe more explicit adding a MachineClass::check_ram_size() handler?
> 
> On the first glance, just max_size field should be sufficient
> with checking code being generic, which should remove code duplication
> such checks introduce across tree. Is there a specific board for
> which call back is 'must to have'?

Some boards have minimum or set of possible values (i.e. 2
or 4 SIMMs, each a pow2 between 8M-64M).

We could have few generic helpers and reuse them in each
machine, instead of open-coding each machine:
  machine_check_max_ram_size(),
  machine_check_ram_size_in_range(),
  ...

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

[Igor Mammedov]

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 (fixups were one of items that were in
the way of splitting guest RAM into backend/frontend model)

 

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

[Philippe Mathieu-Daudé]

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 ;)

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

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)
> >   
>