All of lore.kernel.org
 help / color / mirror / Atom feed
* xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
@ 2021-08-20 14:04 Bin Meng
  2021-08-20 14:09 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 14+ messages in thread
From: Bin Meng @ 2021-08-20 14:04 UTC (permalink / raw)
  To: Edgar E. Iglesias, Alistair Francis, Peter Maydell, qemu-arm,
	qemu-devel@nongnu.org Developers

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 14:04 xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram' Bin Meng
@ 2021-08-20 14:09 ` Philippe Mathieu-Daudé
  2021-08-20 14:22   ` Bin Meng
  0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-20 14:09 UTC (permalink / raw)
  To: Bin Meng, qemu-devel@nongnu.org Developers, David Hildenbrand,
	Richard W.M. Jones
  Cc: Edgar E. Iglesias, qemu-arm, Alistair Francis, Peter Maydell

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 14:09 ` Philippe Mathieu-Daudé
@ 2021-08-20 14:22   ` Bin Meng
  2021-08-20 14:34     ` David Hildenbrand
  0 siblings, 1 reply; 14+ messages in thread
From: Bin Meng @ 2021-08-20 14:22 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, David Hildenbrand, Richard W.M. Jones,
	qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis,
	Edgar E. Iglesias

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 14:22   ` Bin Meng
@ 2021-08-20 14:34     ` David Hildenbrand
  2021-08-20 14:39       ` Peter Maydell
  0 siblings, 1 reply; 14+ messages in thread
From: David Hildenbrand @ 2021-08-20 14:34 UTC (permalink / raw)
  To: Bin Meng, Philippe Mathieu-Daudé
  Cc: Peter Maydell, qemu-devel@nongnu.org Developers,
	Richard W.M. Jones, qemu-arm, Alistair Francis,
	Edgar E. Iglesias

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 14:34     ` David Hildenbrand
@ 2021-08-20 14:39       ` Peter Maydell
  2021-08-20 15:31         ` Philippe Mathieu-Daudé
  2021-08-20 15:44         ` Igor Mammedov
  0 siblings, 2 replies; 14+ messages in thread
From: Peter Maydell @ 2021-08-20 14:39 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: qemu-devel@nongnu.org Developers, Richard W.M. Jones, qemu-arm,
	Alistair Francis, Igor Mammedov, Edgar E. Iglesias, Bin Meng,
	Philippe Mathieu-Daudé

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 14:39       ` Peter Maydell
@ 2021-08-20 15:31         ` Philippe Mathieu-Daudé
  2021-08-20 15:44         ` Igor Mammedov
  1 sibling, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-20 15:31 UTC (permalink / raw)
  To: Peter Maydell, David Hildenbrand
  Cc: qemu-devel@nongnu.org Developers, Richard W.M. Jones, qemu-arm,
	Alistair Francis, Igor Mammedov, Edgar E. Iglesias, Bin Meng

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 14:39       ` Peter Maydell
  2021-08-20 15:31         ` Philippe Mathieu-Daudé
@ 2021-08-20 15:44         ` Igor Mammedov
  2021-08-20 15:47           ` David Hildenbrand
  1 sibling, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2021-08-20 15:44 UTC (permalink / raw)
  To: Peter Maydell
  Cc: David Hildenbrand, Richard W.M. Jones,
	qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis,
	Edgar E. Iglesias, Bin Meng, Philippe Mathieu-Daudé

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
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 15:44         ` Igor Mammedov
@ 2021-08-20 15:47           ` David Hildenbrand
  2021-08-20 15:53             ` Philippe Mathieu-Daudé
  2021-08-20 16:03             ` Igor Mammedov
  0 siblings, 2 replies; 14+ messages in thread
From: David Hildenbrand @ 2021-08-20 15:47 UTC (permalink / raw)
  To: Igor Mammedov, Peter Maydell
  Cc: qemu-devel@nongnu.org Developers, Richard W.M. Jones, qemu-arm,
	Alistair Francis, Edgar E. Iglesias, Bin Meng,
	Philippe Mathieu-Daudé

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 15:47           ` David Hildenbrand
@ 2021-08-20 15:53             ` Philippe Mathieu-Daudé
  2021-08-20 16:08               ` Igor Mammedov
  2021-08-20 16:03             ` Igor Mammedov
  1 sibling, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-20 15:53 UTC (permalink / raw)
  To: David Hildenbrand, Igor Mammedov, Peter Maydell
  Cc: qemu-devel@nongnu.org Developers, Richard W.M. Jones, qemu-arm,
	Alistair Francis, Edgar E. Iglesias, Bin Meng

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?



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 15:47           ` David Hildenbrand
  2021-08-20 15:53             ` Philippe Mathieu-Daudé
@ 2021-08-20 16:03             ` Igor Mammedov
  2021-08-20 16:06               ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2021-08-20 16:03 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Peter Maydell, qemu-devel@nongnu.org Developers,
	Richard W.M. Jones, qemu-arm, Alistair Francis,
	Edgar E. Iglesias, Bin Meng, Philippe Mathieu-Daudé

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)

 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 16:03             ` Igor Mammedov
@ 2021-08-20 16:06               ` Philippe Mathieu-Daudé
  2021-08-20 16:36                 ` Igor Mammedov
  0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-20 16:06 UTC (permalink / raw)
  To: Igor Mammedov, David Hildenbrand
  Cc: Peter Maydell, qemu-devel@nongnu.org Developers,
	Richard W.M. Jones, qemu-arm, Alistair Francis,
	Edgar E. Iglesias, Bin Meng

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 15:53             ` Philippe Mathieu-Daudé
@ 2021-08-20 16:08               ` Igor Mammedov
  2021-08-20 16:13                 ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2021-08-20 16:08 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, David Hildenbrand, Richard W.M. Jones,
	qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis,
	Edgar E. Iglesias, Bin Meng

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'?



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 16:08               ` Igor Mammedov
@ 2021-08-20 16:13                 ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-20 16:13 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Peter Maydell, David Hildenbrand, Richard W.M. Jones,
	qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis,
	Edgar E. Iglesias, Bin Meng

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
  2021-08-20 16:06               ` Philippe Mathieu-Daudé
@ 2021-08-20 16:36                 ` Igor Mammedov
  0 siblings, 0 replies; 14+ messages in thread
From: Igor Mammedov @ 2021-08-20 16:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, David Hildenbrand, Richard W.M. Jones,
	qemu-devel@nongnu.org Developers, qemu-arm, Alistair Francis,
	Edgar E. Iglesias, Bin Meng

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



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-08-20 16:37 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 14:04 xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram' Bin Meng
2021-08-20 14:09 ` Philippe Mathieu-Daudé
2021-08-20 14:22   ` Bin Meng
2021-08-20 14:34     ` David Hildenbrand
2021-08-20 14:39       ` Peter Maydell
2021-08-20 15:31         ` Philippe Mathieu-Daudé
2021-08-20 15:44         ` Igor Mammedov
2021-08-20 15:47           ` David Hildenbrand
2021-08-20 15:53             ` Philippe Mathieu-Daudé
2021-08-20 16:08               ` Igor Mammedov
2021-08-20 16:13                 ` Philippe Mathieu-Daudé
2021-08-20 16:03             ` Igor Mammedov
2021-08-20 16:06               ` Philippe Mathieu-Daudé
2021-08-20 16:36                 ` Igor Mammedov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.