[00/21] hw: Let the machine be the owner of the system memory
mbox series

Message ID 20191020225650.3671-1-philmd@redhat.com
Headers show
Series
  • hw: Let the machine be the owner of the system memory
Related show

Message

Philippe Mathieu-Daudé Oct. 20, 2019, 10:56 p.m. UTC
Hi,

This series is based on Igor's "eliminate remaining places that
abuse memory_region_allocate_system_memory()":
https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01601.html

It is quite simple, we enforce all machines to be the QOM owner
of the system memory.

This changes the memory tree from:

  (qemu) info mtree -o
  memory-region: pc.ram
    0000000000000000-0000000007ffffff (prio 0, ram): pc.ram parent:{obj path=/machine/unattached}

to:

  (qemu) info mtree -o
  memory-region: pc.ram
    0000000000000000-0000000007ffffff (prio 0, ram): pc.ram owner:{obj path=/machine}

Few patches are required to clean the codebase first, to unify the
creation of the system memory in the board/machine code. Mostly some
old ARM machines (pre-QOM) were affected.

Please review (as a generic codebase cleanup).

Regards,

Phil.

Based-on: <20191008113318.7012-1-imammedo@redhat.com>

Philippe Mathieu-Daudé (21):
  hw/arm/xilinx_zynq: Use the IEC binary prefix definitions
  hw/arm/mps2: Use the IEC binary prefix definitions
  hw/arm/collie: Create the RAM in the board
  hw/arm/omap2: Create the RAM in the board
  hw/arm/omap1: Create the RAM in the board
  hw/arm/digic4: Inline digic4_board_setup_ram() function
  hw: Drop QOM ownership on memory_region_allocate_system_memory() calls
  hw/alpha/dp264: Create the RAM in the board
  hw: Let memory_region_allocate_system_memory take MachineState
    argument
  hw/core: Let the machine be the owner of the system memory
  hw/alpha: Let the machine be the owner of the system memory
  hw/arm: Let the machine be the owner of the system memory
  hw/cris: Let the machine be the owner of the system memory
  hw/hppa: Let the machine be the owner of the system memory
  hw/i386: Let the machine be the owner of the system memory
  hw/lm32: Let the machine be the owner of the system memory
  hw/m68k: Let the machine be the owner of the system memory
  hw/mips: Let the machine be the owner of the system memory
  hw/ppc: Let the machine be the owner of the system memory
  hw/sparc: Let the machine be the owner of the system memory
  hw/core: Assert memory_region_allocate_system_memory has machine owner

 hw/alpha/alpha_sys.h      |  2 +-
 hw/alpha/dp264.c          | 11 ++++++++++-
 hw/alpha/typhoon.c        |  9 +--------
 hw/arm/aspeed.c           |  2 +-
 hw/arm/collie.c           |  8 ++++++--
 hw/arm/cubieboard.c       |  2 +-
 hw/arm/digic_boards.c     | 14 +++++---------
 hw/arm/highbank.c         |  3 ++-
 hw/arm/imx25_pdk.c        |  2 +-
 hw/arm/integratorcp.c     |  2 +-
 hw/arm/kzm.c              |  2 +-
 hw/arm/mcimx6ul-evk.c     |  2 +-
 hw/arm/mcimx7d-sabre.c    |  2 +-
 hw/arm/mps2-tz.c          |  5 +++--
 hw/arm/mps2.c             |  5 +++--
 hw/arm/musicpal.c         |  2 +-
 hw/arm/nseries.c          | 10 +++++++---
 hw/arm/omap1.c            | 12 +++++-------
 hw/arm/omap2.c            | 13 +++++--------
 hw/arm/omap_sx1.c         |  8 ++++++--
 hw/arm/palm.c             |  8 ++++++--
 hw/arm/raspi.c            |  2 +-
 hw/arm/sabrelite.c        |  2 +-
 hw/arm/sbsa-ref.c         |  2 +-
 hw/arm/strongarm.c        |  7 +------
 hw/arm/strongarm.h        |  4 +---
 hw/arm/versatilepb.c      |  2 +-
 hw/arm/vexpress.c         |  4 ++--
 hw/arm/virt.c             |  2 +-
 hw/arm/xilinx_zynq.c      |  5 +++--
 hw/arm/xlnx-versal-virt.c |  2 +-
 hw/arm/xlnx-zcu102.c      |  2 +-
 hw/core/null-machine.c    |  2 +-
 hw/core/numa.c            |  9 +++++----
 hw/cris/axis_dev88.c      |  2 +-
 hw/hppa/machine.c         |  2 +-
 hw/i386/pc.c              |  2 +-
 hw/lm32/lm32_boards.c     |  4 ++--
 hw/lm32/milkymist.c       |  2 +-
 hw/m68k/an5206.c          |  2 +-
 hw/m68k/mcf5208.c         |  2 +-
 hw/m68k/next-cube.c       |  2 +-
 hw/mips/boston.c          |  2 +-
 hw/mips/mips_fulong2e.c   |  3 ++-
 hw/mips/mips_jazz.c       |  2 +-
 hw/mips/mips_malta.c      |  2 +-
 hw/mips/mips_mipssim.c    |  2 +-
 hw/mips/mips_r4k.c        |  3 ++-
 hw/ppc/e500.c             |  3 ++-
 hw/ppc/mac_newworld.c     |  3 ++-
 hw/ppc/mac_oldworld.c     |  2 +-
 hw/ppc/pnv.c              |  2 +-
 hw/ppc/ppc405_boards.c    |  6 +++---
 hw/ppc/prep.c             |  3 ++-
 hw/ppc/spapr.c            |  2 +-
 hw/ppc/virtex_ml507.c     |  2 +-
 hw/sparc/leon3.c          |  2 +-
 hw/sparc/sun4m.c          |  2 +-
 include/hw/arm/omap.h     | 10 +++-------
 include/hw/boards.h       |  6 ++++--
 60 files changed, 127 insertions(+), 117 deletions(-)

Comments

no-reply@patchew.org Oct. 21, 2019, 12:03 a.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20191020225650.3671-1-philmd@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH 00/21] hw: Let the machine be the owner of the system memory
Type: series
Message-id: 20191020225650.3671-1-philmd@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
23b4e9a hw/core: Assert memory_region_allocate_system_memory has machine owner
67caaf0 hw/sparc: Let the machine be the owner of the system memory
9610d69 hw/ppc: Let the machine be the owner of the system memory
3cf0c55 hw/mips: Let the machine be the owner of the system memory
c6cd486 hw/m68k: Let the machine be the owner of the system memory
dd8e9a6 hw/lm32: Let the machine be the owner of the system memory
c7b9ccd hw/i386: Let the machine be the owner of the system memory
655182f hw/hppa: Let the machine be the owner of the system memory
20dc622 hw/cris: Let the machine be the owner of the system memory
fc3c6d2 hw/arm: Let the machine be the owner of the system memory
e9f1ebc hw/alpha: Let the machine be the owner of the system memory
3ef111b hw/core: Let the machine be the owner of the system memory
31bd838 hw: Let memory_region_allocate_system_memory take MachineState argument
dae9d11 hw/alpha/dp264: Create the RAM in the board
c31eba0 hw: Drop QOM ownership on memory_region_allocate_system_memory() calls
728aa65 hw/arm/digic4: Inline digic4_board_setup_ram() function
c2a9052 hw/arm/omap1: Create the RAM in the board
4f4977f hw/arm/omap2: Create the RAM in the board
d1959b2 hw/arm/collie: Create the RAM in the board
815f5c1 hw/arm/mps2: Use the IEC binary prefix definitions
103190f hw/arm/xilinx_zynq: Use the IEC binary prefix definitions

=== OUTPUT BEGIN ===
1/21 Checking commit 103190fd9b28 (hw/arm/xilinx_zynq: Use the IEC binary prefix definitions)
2/21 Checking commit 815f5c166924 (hw/arm/mps2: Use the IEC binary prefix definitions)
3/21 Checking commit d1959b2d3e49 (hw/arm/collie: Create the RAM in the board)
4/21 Checking commit 4f4977fa8205 (hw/arm/omap2: Create the RAM in the board)
5/21 Checking commit c2a90524bd33 (hw/arm/omap1: Create the RAM in the board)
6/21 Checking commit 728aa65672f4 (hw/arm/digic4: Inline digic4_board_setup_ram() function)
7/21 Checking commit c31eba0d025b (hw: Drop QOM ownership on memory_region_allocate_system_memory() calls)
8/21 Checking commit dae9d111acb1 (hw/alpha/dp264: Create the RAM in the board)
ERROR: spaces required around that '*' (ctx:WxV)
#24: FILE: hw/alpha/alpha_sys.h:13:
+PCIBus *typhoon_init(ISABus **, qemu_irq *, AlphaCPU *[4],
                                                      ^

total: 1 errors, 0 warnings, 59 lines checked

Patch 8/21 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

9/21 Checking commit 31bd838d2d79 (hw: Let memory_region_allocate_system_memory take MachineState argument)
10/21 Checking commit 3ef111b6a62a (hw/core: Let the machine be the owner of the system memory)
11/21 Checking commit e9f1ebcb1624 (hw/alpha: Let the machine be the owner of the system memory)
12/21 Checking commit fc3c6d23b63f (hw/arm: Let the machine be the owner of the system memory)
13/21 Checking commit 20dc622c1df3 (hw/cris: Let the machine be the owner of the system memory)
14/21 Checking commit 655182f6e74a (hw/hppa: Let the machine be the owner of the system memory)
15/21 Checking commit c7b9ccda7442 (hw/i386: Let the machine be the owner of the system memory)
16/21 Checking commit dd8e9a66c1f0 (hw/lm32: Let the machine be the owner of the system memory)
17/21 Checking commit c6cd4866d6a0 (hw/m68k: Let the machine be the owner of the system memory)
18/21 Checking commit 3cf0c55fcc1b (hw/mips: Let the machine be the owner of the system memory)
19/21 Checking commit 9610d6908cbc (hw/ppc: Let the machine be the owner of the system memory)
20/21 Checking commit 67caaf05019b (hw/sparc: Let the machine be the owner of the system memory)
21/21 Checking commit 23b4e9a52318 (hw/core: Assert memory_region_allocate_system_memory has machine owner)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191020225650.3671-1-philmd@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Philippe Mathieu-Daudé Oct. 21, 2019, 8:52 a.m. UTC | #2
On 10/21/19 12:56 AM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series is based on Igor's "eliminate remaining places that
> abuse memory_region_allocate_system_memory()":
> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01601.html
> 
> It is quite simple, we enforce all machines to be the QOM owner
> of the system memory.
> 
> This changes the memory tree from:
> 
>    (qemu) info mtree -o
>    memory-region: pc.ram
>      0000000000000000-0000000007ffffff (prio 0, ram): pc.ram parent:{obj path=/machine/unattached}
> 
> to:
> 
>    (qemu) info mtree -o
>    memory-region: pc.ram
>      0000000000000000-0000000007ffffff (prio 0, ram): pc.ram owner:{obj path=/machine}
> 
> Few patches are required to clean the codebase first, to unify the
> creation of the system memory in the board/machine code. Mostly some
> old ARM machines (pre-QOM) were affected.
> 
> Please review (as a generic codebase cleanup).
> 
> Regards,
> 
> Phil.
> 
> Based-on: <20191008113318.7012-1-imammedo@redhat.com>
> 
> Philippe Mathieu-Daudé (21):
>    hw/arm/xilinx_zynq: Use the IEC binary prefix definitions
>    hw/arm/mps2: Use the IEC binary prefix definitions
>    hw/arm/collie: Create the RAM in the board
>    hw/arm/omap2: Create the RAM in the board
>    hw/arm/omap1: Create the RAM in the board
>    hw/arm/digic4: Inline digic4_board_setup_ram() function
>    hw: Drop QOM ownership on memory_region_allocate_system_memory() calls
>    hw/alpha/dp264: Create the RAM in the board
>    hw: Let memory_region_allocate_system_memory take MachineState
>      argument
>    hw/core: Let the machine be the owner of the system memory
>    hw/alpha: Let the machine be the owner of the system memory
>    hw/arm: Let the machine be the owner of the system memory
>    hw/cris: Let the machine be the owner of the system memory
>    hw/hppa: Let the machine be the owner of the system memory
>    hw/i386: Let the machine be the owner of the system memory
>    hw/lm32: Let the machine be the owner of the system memory
>    hw/m68k: Let the machine be the owner of the system memory
>    hw/mips: Let the machine be the owner of the system memory
>    hw/ppc: Let the machine be the owner of the system memory
>    hw/sparc: Let the machine be the owner of the system memory
>    hw/core: Assert memory_region_allocate_system_memory has machine owner

I forgot 4 other calls:

hw/ppc/ppc4xx_devs.c:708:    memory_region_allocate_system_memory(ram, 
NULL, "ppc4xx.sdram", ram_size);
hw/s390x/s390-virtio-ccw.c:164: 
memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size);
hw/sparc/sun4m.c:791:    memory_region_allocate_system_memory(&d->ram, 
NULL, "sun4m.ram",
hw/sparc64/niagara.c:114: 
memory_region_allocate_system_memory(&s->partition_ram, NULL,
Philippe Mathieu-Daudé Oct. 21, 2019, 2:31 p.m. UTC | #3
Hi Peter,

On 10/21/19 10:52 AM, Philippe Mathieu-Daudé wrote:
> On 10/21/19 12:56 AM, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> This series is based on Igor's "eliminate remaining places that
>> abuse memory_region_allocate_system_memory()":
>> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01601.html
>>
>> It is quite simple, we enforce all machines to be the QOM owner
>> of the system memory.
>>
>> This changes the memory tree from:
>>
>>    (qemu) info mtree -o
>>    memory-region: pc.ram
>>      0000000000000000-0000000007ffffff (prio 0, ram): pc.ram 
>> parent:{obj path=/machine/unattached}
>>
>> to:
>>
>>    (qemu) info mtree -o
>>    memory-region: pc.ram
>>      0000000000000000-0000000007ffffff (prio 0, ram): pc.ram 
>> owner:{obj path=/machine}
>>
>> Few patches are required to clean the codebase first, to unify the
>> creation of the system memory in the board/machine code. Mostly some
>> old ARM machines (pre-QOM) were affected.
>>
>> Please review (as a generic codebase cleanup).
>>
>> Regards,
>>
>> Phil.
>>
>> Based-on: <20191008113318.7012-1-imammedo@redhat.com>
>>
>> Philippe Mathieu-Daudé (21):
>>    hw/arm/xilinx_zynq: Use the IEC binary prefix definitions
>>    hw/arm/mps2: Use the IEC binary prefix definitions
>>    hw/arm/collie: Create the RAM in the board
>>    hw/arm/omap2: Create the RAM in the board
>>    hw/arm/omap1: Create the RAM in the board
>>    hw/arm/digic4: Inline digic4_board_setup_ram() function
>>    hw: Drop QOM ownership on memory_region_allocate_system_memory() calls
>>    hw/alpha/dp264: Create the RAM in the board
>>    hw: Let memory_region_allocate_system_memory take MachineState
>>      argument
>>    hw/core: Let the machine be the owner of the system memory
>>    hw/alpha: Let the machine be the owner of the system memory
>>    hw/arm: Let the machine be the owner of the system memory
>>    hw/cris: Let the machine be the owner of the system memory
>>    hw/hppa: Let the machine be the owner of the system memory
>>    hw/i386: Let the machine be the owner of the system memory
>>    hw/lm32: Let the machine be the owner of the system memory
>>    hw/m68k: Let the machine be the owner of the system memory
>>    hw/mips: Let the machine be the owner of the system memory
>>    hw/ppc: Let the machine be the owner of the system memory
>>    hw/sparc: Let the machine be the owner of the system memory
>>    hw/core: Assert memory_region_allocate_system_memory has machine owner
> 
> I forgot 4 other calls:
> 
> hw/ppc/ppc4xx_devs.c:708:    memory_region_allocate_system_memory(ram, 
> NULL, "ppc4xx.sdram", ram_size);
> hw/s390x/s390-virtio-ccw.c:164: 
> memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size);
> hw/sparc/sun4m.c:791:    memory_region_allocate_system_memory(&d->ram, 
> NULL, "sun4m.ram",
> hw/sparc64/niagara.c:114: 
> memory_region_allocate_system_memory(&s->partition_ram, NULL,

I'll need to respin this series because of this omission, but
- hw/ppc requires more cleanup patches as ARM has,
- hw/sparc is blocked until Igor sent his "convert
   memory_region_allocate_system_memory() to memdev" series.

I don't plan to rework on the ARM cleanups patches (1-6):

   hw/arm/xilinx_zynq: Use the IEC binary prefix definitions
   hw/arm/mps2: Use the IEC binary prefix definitions
   hw/arm/collie: Create the RAM in the board
   hw/arm/omap2: Create the RAM in the board
   hw/arm/omap1: Create the RAM in the board
   hw/arm/digic4: Inline digic4_board_setup_ram() function

Maybe you can take them directly in arm-next once they are reviewed,
then the rest can go via machine-next.

Thanks,

Phil.