* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
@ 2021-08-20 15:47 ` David Hildenbrand
0 siblings, 0 replies; 23+ 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] 23+ 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
-1 siblings, 1 reply; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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,
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] 23+ messages in thread* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
@ 2021-08-20 16:13 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 23+ 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] 23+ 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 16:03 ` Igor Mammedov
-1 siblings, 0 replies; 23+ 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, 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] 23+ messages in thread* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
@ 2021-08-20 16:03 ` Igor Mammedov
0 siblings, 0 replies; 23+ 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] 23+ 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
-1 siblings, 1 reply; 23+ 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] 23+ 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; 23+ 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,
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] 23+ messages in thread* Re: xilinx-zynq-a9: cannot set up guest memory 'zynq.ext_ram'
@ 2021-08-20 16:36 ` Igor Mammedov
0 siblings, 0 replies; 23+ 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] 23+ messages in thread