1. Patchew
  2. QEMU
  3. [PATCH v5 00/79] refactor main RAM allocation to use hostmem backend
  4. View patch

[PATCH v5 17/79] arm/integratorcp: use memdev for RAM

Igor Mammedov posted 79 patches 5 years, 11 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, KONRAD Frederic <frederic.konrad@adacore.com>, Jan Kiszka <jan.kiszka@web.de>, Paolo Bonzini <pbonzini@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Cornelia Huck <cohuck@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paul Burton <pburton@wavecomp.com>, David Hildenbrand <david@redhat.com>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Fabien Chouteau <chouteau@adacore.com>, Andrew Baumann <Andrew.Baumann@microsoft.com>, Michael Walle <michael@walle.cc>, Peter Chubb <peter.chubb@nicta.com.au>, Richard Henderson <rth@twiddle.net>, Laurent Vivier <laurent@vivier.eu>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>, Sergio Lopez <slp@redhat.com>, Thomas Huth <huth@tuxfamily.org>, Beniamino Galvani <b.galvani@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Helge Deller <deller@gmx.de>, Igor Mammedov <imammedo@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, Alistair Francis <alistair@alistair23.me>, Christian Borntraeger <borntraeger@de.ibm.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Thomas Huth <thuth@redhat.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Andrey Smirnov <andrew.smirnov@gmail.com>, Halil Pasic <pasic@linux.ibm.com>, BALATON Zoltan <balaton@eik.bme.hu>, Antony Pavlov <antonynpavlov@gmail.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, Rob Herring <robh@kernel.org>, Peter Maydell <peter.maydell@linaro.org>, "Cédric Le Goater" <clg@kaod.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Andrzej Zaborowski <balrogg@gmail.com>, Leif Lindholm <leif@nuviainc.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Artyom Tarasenko <atar4qemu@gmail.com>
There is a newer version of this series
[PATCH v5 17/79] arm/integratorcp: use memdev for RAM
Posted by Igor Mammedov 5 years, 11 months ago 
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
  MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
CC: peter.chubb@nicta.com.au
---
 hw/arm/integratorcp.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 0cd94d9f09..cc845b8534 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -585,7 +585,6 @@ static void integratorcp_init(MachineState *machine)
     Object *cpuobj;
     ARMCPU *cpu;
     MemoryRegion *address_space_mem = get_system_memory();
-    MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
     qemu_irq pic[32];
     DeviceState *dev, *sic, *icp;
@@ -605,14 +604,13 @@ static void integratorcp_init(MachineState *machine)
 
     cpu = ARM_CPU(cpuobj);
 
-    memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
-                                         ram_size);
     /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash.  */
     /* ??? RAM should repeat to fill physical memory space.  */
     /* SDRAM at address zero*/
-    memory_region_add_subregion(address_space_mem, 0, ram);
+    memory_region_add_subregion(address_space_mem, 0, machine->ram);
     /* And again at address 0x80000000 */
-    memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
+    memory_region_init_alias(ram_alias, NULL, "ram.alias", machine->ram,
+                             0, ram_size);
     memory_region_add_subregion(address_space_mem, 0x80000000, ram_alias);
 
     dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
@@ -660,6 +658,7 @@ static void integratorcp_machine_init(MachineClass *mc)
     mc->init = integratorcp_init;
     mc->ignore_memory_transaction_failures = true;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
+    mc->default_ram_id = "integrator.ram";
 }
 
 DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
-- 
2.18.1
Re: [PATCH v5 17/79] arm/integratorcp: use memdev for RAM
Posted by Richard Henderson 5 years, 11 months ago 
On 2/17/20 9:33 AM, Igor Mammedov wrote:
> memory_region_allocate_system_memory() API is going away, so
> replace it with memdev allocated MemoryRegion. The later is
> initialized by generic code, so board only needs to opt in
> to memdev scheme by providing
>   MachineClass::default_ram_id
> and using MachineState::ram instead of manually initializing
> RAM memory region.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> ---
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Re: [PATCH v5 17/79] arm/integratorcp: use memdev for RAM
Posted by Philippe Mathieu-Daudé 5 years, 11 months ago 
On 2/17/20 6:33 PM, Igor Mammedov wrote:
> memory_region_allocate_system_memory() API is going away, so
> replace it with memdev allocated MemoryRegion. The later is
> initialized by generic code, so board only needs to opt in
> to memdev scheme by providing
>    MachineClass::default_ram_id
> and using MachineState::ram instead of manually initializing
> RAM memory region.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> ---
> CC: peter.chubb@nicta.com.au
> ---
>   hw/arm/integratorcp.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> index 0cd94d9f09..cc845b8534 100644
> --- a/hw/arm/integratorcp.c
> +++ b/hw/arm/integratorcp.c
> @@ -585,7 +585,6 @@ static void integratorcp_init(MachineState *machine)
>       Object *cpuobj;
>       ARMCPU *cpu;
>       MemoryRegion *address_space_mem = get_system_memory();
> -    MemoryRegion *ram = g_new(MemoryRegion, 1);
>       MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
>       qemu_irq pic[32];
>       DeviceState *dev, *sic, *icp;
> @@ -605,14 +604,13 @@ static void integratorcp_init(MachineState *machine)
>   
>       cpu = ARM_CPU(cpuobj);
>   
> -    memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
> -                                         ram_size);
>       /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash.  */
>       /* ??? RAM should repeat to fill physical memory space.  */
>       /* SDRAM at address zero*/
> -    memory_region_add_subregion(address_space_mem, 0, ram);
> +    memory_region_add_subregion(address_space_mem, 0, machine->ram);
>       /* And again at address 0x80000000 */
> -    memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
> +    memory_region_init_alias(ram_alias, NULL, "ram.alias", machine->ram,
> +                             0, ram_size);
>       memory_region_add_subregion(address_space_mem, 0x80000000, ram_alias);
>   
>       dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
> @@ -660,6 +658,7 @@ static void integratorcp_machine_init(MachineClass *mc)
>       mc->init = integratorcp_init;
>       mc->ignore_memory_transaction_failures = true;
>       mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
> +    mc->default_ram_id = "integrator.ram";
>   }
>   
>   DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
> 

Looking at integratorcm_realize() this machine seems to handle at most 
512MiB.
Re: [PATCH v5 17/79] arm/integratorcp: use memdev for RAM
Posted by Igor Mammedov 5 years, 11 months ago 
On Tue, 18 Feb 2020 07:55:14 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 2/17/20 6:33 PM, Igor Mammedov wrote:
> > memory_region_allocate_system_memory() API is going away, so
> > replace it with memdev allocated MemoryRegion. The later is
> > initialized by generic code, so board only needs to opt in
> > to memdev scheme by providing
> >    MachineClass::default_ram_id
> > and using MachineState::ram instead of manually initializing
> > RAM memory region.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > ---
> > CC: peter.chubb@nicta.com.au
> > ---
> >   hw/arm/integratorcp.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> > index 0cd94d9f09..cc845b8534 100644
> > --- a/hw/arm/integratorcp.c
> > +++ b/hw/arm/integratorcp.c
> > @@ -585,7 +585,6 @@ static void integratorcp_init(MachineState *machine)
> >       Object *cpuobj;
> >       ARMCPU *cpu;
> >       MemoryRegion *address_space_mem = get_system_memory();
> > -    MemoryRegion *ram = g_new(MemoryRegion, 1);
> >       MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
> >       qemu_irq pic[32];
> >       DeviceState *dev, *sic, *icp;
> > @@ -605,14 +604,13 @@ static void integratorcp_init(MachineState *machine)
> >   
> >       cpu = ARM_CPU(cpuobj);
> >   
> > -    memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
> > -                                         ram_size);
> >       /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash.  */
> >       /* ??? RAM should repeat to fill physical memory space.  */
> >       /* SDRAM at address zero*/
> > -    memory_region_add_subregion(address_space_mem, 0, ram);
> > +    memory_region_add_subregion(address_space_mem, 0, machine->ram);
> >       /* And again at address 0x80000000 */
> > -    memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
> > +    memory_region_init_alias(ram_alias, NULL, "ram.alias", machine->ram,
> > +                             0, ram_size);
> >       memory_region_add_subregion(address_space_mem, 0x80000000, ram_alias);
> >   
> >       dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
> > @@ -660,6 +658,7 @@ static void integratorcp_machine_init(MachineClass *mc)
> >       mc->init = integratorcp_init;
> >       mc->ignore_memory_transaction_failures = true;
> >       mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
> > +    mc->default_ram_id = "integrator.ram";
> >   }
> >   
> >   DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
> >   
> 
> Looking at integratorcm_realize() this machine seems to handle at most 
> 512MiB.

According to 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0159b/Cegeadbj.html

it supports "optionally, 16 to 256MB of SDRAM plugged into the DIMM socket"
so I'm not sure that realize is valid reference here.
(well I don't know anything about arm boards, but it probably should be
double checked by maintainer).

PS:
It should not hold this series (as check wasn't there to begin with),
I'll post a patch on top to add check once we decide to what limit it
should be set.
Re: [PATCH v5 17/79] arm/integratorcp: use memdev for RAM
Posted by Philippe Mathieu-Daudé 5 years, 11 months ago 
On 2/18/20 5:41 PM, Igor Mammedov wrote:
> On Tue, 18 Feb 2020 07:55:14 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> On 2/17/20 6:33 PM, Igor Mammedov wrote:
>>> memory_region_allocate_system_memory() API is going away, so
>>> replace it with memdev allocated MemoryRegion. The later is
>>> initialized by generic code, so board only needs to opt in
>>> to memdev scheme by providing
>>>     MachineClass::default_ram_id
>>> and using MachineState::ram instead of manually initializing
>>> RAM memory region.
>>>
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>>> ---
>>> CC: peter.chubb@nicta.com.au
>>> ---
>>>    hw/arm/integratorcp.c | 9 ++++-----
>>>    1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
>>> index 0cd94d9f09..cc845b8534 100644
>>> --- a/hw/arm/integratorcp.c
>>> +++ b/hw/arm/integratorcp.c
>>> @@ -585,7 +585,6 @@ static void integratorcp_init(MachineState *machine)
>>>        Object *cpuobj;
>>>        ARMCPU *cpu;
>>>        MemoryRegion *address_space_mem = get_system_memory();
>>> -    MemoryRegion *ram = g_new(MemoryRegion, 1);
>>>        MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
>>>        qemu_irq pic[32];
>>>        DeviceState *dev, *sic, *icp;
>>> @@ -605,14 +604,13 @@ static void integratorcp_init(MachineState *machine)
>>>    
>>>        cpu = ARM_CPU(cpuobj);
>>>    
>>> -    memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
>>> -                                         ram_size);
>>>        /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash.  */
>>>        /* ??? RAM should repeat to fill physical memory space.  */
>>>        /* SDRAM at address zero*/
>>> -    memory_region_add_subregion(address_space_mem, 0, ram);
>>> +    memory_region_add_subregion(address_space_mem, 0, machine->ram);
>>>        /* And again at address 0x80000000 */
>>> -    memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
>>> +    memory_region_init_alias(ram_alias, NULL, "ram.alias", machine->ram,
>>> +                             0, ram_size);
>>>        memory_region_add_subregion(address_space_mem, 0x80000000, ram_alias);
>>>    
>>>        dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
>>> @@ -660,6 +658,7 @@ static void integratorcp_machine_init(MachineClass *mc)
>>>        mc->init = integratorcp_init;
>>>        mc->ignore_memory_transaction_failures = true;
>>>        mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
>>> +    mc->default_ram_id = "integrator.ram";
>>>    }
>>>    
>>>    DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
>>>    
>>
>> Looking at integratorcm_realize() this machine seems to handle at most
>> 512MiB.
> 
> According to
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0159b/Cegeadbj.html
> 
> it supports "optionally, 16 to 256MB of SDRAM plugged into the DIMM socket"
> so I'm not sure that realize is valid reference here.
> (well I don't know anything about arm boards, but it probably should be
> double checked by maintainer).
> 
> PS:
> It should not hold this series (as check wasn't there to begin with),
> I'll post a patch on top to add check once we decide to what limit it
> should be set.

This is certainly not a blocking comment (neither am I waiting for you 
to fix this, I was just making a comment while reviewing the whole). 
Sorry if this was not clear.
Re: [PATCH v5 17/79] arm/integratorcp: use memdev for RAM
Posted by Igor Mammedov 5 years, 11 months ago 
On Tue, 18 Feb 2020 07:55:14 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 2/17/20 6:33 PM, Igor Mammedov wrote:
> > memory_region_allocate_system_memory() API is going away, so
> > replace it with memdev allocated MemoryRegion. The later is
> > initialized by generic code, so board only needs to opt in
> > to memdev scheme by providing
> >    MachineClass::default_ram_id
> > and using MachineState::ram instead of manually initializing
> > RAM memory region.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > ---
> > CC: peter.chubb@nicta.com.au
> > ---
> >   hw/arm/integratorcp.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> > index 0cd94d9f09..cc845b8534 100644
> > --- a/hw/arm/integratorcp.c
> > +++ b/hw/arm/integratorcp.c
> > @@ -585,7 +585,6 @@ static void integratorcp_init(MachineState *machine)
> >       Object *cpuobj;
> >       ARMCPU *cpu;
> >       MemoryRegion *address_space_mem = get_system_memory();
> > -    MemoryRegion *ram = g_new(MemoryRegion, 1);
> >       MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
> >       qemu_irq pic[32];
> >       DeviceState *dev, *sic, *icp;
> > @@ -605,14 +604,13 @@ static void integratorcp_init(MachineState *machine)
> >   
> >       cpu = ARM_CPU(cpuobj);
> >   
> > -    memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
> > -                                         ram_size);
> >       /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash.  */
> >       /* ??? RAM should repeat to fill physical memory space.  */
> >       /* SDRAM at address zero*/
> > -    memory_region_add_subregion(address_space_mem, 0, ram);
> > +    memory_region_add_subregion(address_space_mem, 0, machine->ram);
> >       /* And again at address 0x80000000 */
> > -    memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
> > +    memory_region_init_alias(ram_alias, NULL, "ram.alias", machine->ram,
> > +                             0, ram_size);
> >       memory_region_add_subregion(address_space_mem, 0x80000000, ram_alias);
> >   
> >       dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
> > @@ -660,6 +658,7 @@ static void integratorcp_machine_init(MachineClass *mc)
> >       mc->init = integratorcp_init;
> >       mc->ignore_memory_transaction_failures = true;
> >       mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
> > +    mc->default_ram_id = "integrator.ram";
> >   }
> >   
> >   DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
> >   
> 
> Looking at integratorcm_realize() this machine seems to handle at most 
> 512MiB.
> 
I'll add a patch on top.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.