All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone
@ 2013-06-09 16:10 Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 01/18] dump: Move stubs into libqemustub.a Andreas Färber
                   ` (19 more replies)
  0 siblings, 20 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jens Freimann, lcapitulino, Vincent Rabin, qiaonuohan,
	Paolo Bonzini, Andreas Färber

Hello,

As requested by Paolo, this replaces Kate's previous memory_mapping split
and my follow-ups and instead goes directly for moving things to CPUState.

All knowledge about dump / memory mapping are moved away from configure.

v4 incorporates minor API changes, suggested by Luiz, as well as two
CPUArchState cleanups previously on qom-cpu-10 and goes on to make
memory_mapping.c target-independent.

Note: dump.c depends on TARGET_PAGE_SIZE. Since it is only used in ifs and
function arguments, we could easily introduce a target_get_page_size() helper
to abstract this info out from dump.c, but that's outside the scope of this
guest-memory-dump series.

Available for testing here:
git://github.com/afaerber/qemu-cpu.git qom-cpu-dump-stubs.v4
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-dump-stubs.v4

Regards,
Andreas

v3 -> v4:
* Made cpu_paging_enabled() CPUState argument const.
* Added Error** argument to cpu_get_memory_mapping().
* Inserted patch to change cpu_paging_enabled() default to false.
* Redid qemu_get_guest_memory_mapping() cleanup with two CPU iterations and Error.
* Appended two patches to clean up CPUArchState uses.
* Re-appended target_ulong replacement patch.

v2 -> v3:
* Incorporate Luiz' x86 bugfix.
* Append a patch to resolve the open-coded CPU loops.
* Massage CONFIG_HAVE_* commit messages (they were previously reordered).

v1 -> v2:
* Dropped Kate's memory_mapping split
* Dropped target_ulong cleanup and replaced with typedef
* Amended CPUArchState cleanups with introducing hooks in CPUClass
* Drop memory_memory stubs instead of moving them

Cc: Wen Congyang <wency@cn.fujitsu.com>
Cc: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
Cc: Jens Freimann <jfrei@linux.vnet.ibm.com>
Cc: Vincent Rabin <rabin@rab.in>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>

Andreas Färber (13):
  dump: Move stubs into libqemustub.a
  cpu: Turn cpu_paging_enabled() into a CPUState hook
  memory_mapping: Move MemoryMappingList typedef to qemu/typedefs.h
  cpu: Turn cpu_get_memory_mapping() into a CPUState hook
  memory_mapping: Drop qemu_get_memory_mapping() stub
  dump: Drop qmp_dump_guest_memory() stub and build for all targets
  cpu: Change default for CPUClass::get_paging_enabled()
  memory_mapping: Cleanup qemu_get_guest_memory_mapping()
  dump: Abstract dump_init() with cpu_synchronize_all_states()
  dump: Abstract dump_init() further with qemu_for_each_cpu()
  dump: Abstract write_elf{64,32}_notes() with qemu_for_each_cpu()
  memory_mapping: Use hwaddr type for MemoryMapping virt_addr field
  memory_mapping: Build only once

Eduardo Habkost (3):
  pc: Create pc-*-1.6 machine-types
  target-i386: Update model values on Conroe/Penryn/Nehalem CPU models
  target-i386: Set level=4 on Conroe/Penryn/Nehalem

Igor Mammedov (2):
  pc: Fix crash when attempting to hotplug CPU with negative ID
  target-i386: cpu: Fix potential buffer overrun in
    get_register_name_32()

 Makefile.objs                     |   1 +
 Makefile.target                   |   7 +-
 configure                         |   8 --
 dump.c                            | 157 +++++++++++++++++++++++++++-----------
 hmp-commands.hx                   |   2 -
 hw/i386/pc.c                      |   5 ++
 hw/i386/pc_piix.c                 |  18 ++++-
 hw/i386/pc_q35.c                  |  16 +++-
 include/exec/cpu-all.h            |  28 -------
 include/exec/cpu-common.h         |  33 ++++++++
 include/hw/i386/pc.h              |  28 +++++++
 include/qemu/typedefs.h           |   2 +
 include/qom/cpu.h                 |  24 ++++++
 include/sysemu/memory_mapping.h   |  19 ++---
 memory_mapping-stub.c             |  33 --------
 memory_mapping.c                  |  57 ++++++++------
 qom/cpu.c                         |  29 +++++++
 stubs/Makefile.objs               |   1 +
 dump-stub.c => stubs/dump.c       |   8 --
 target-i386/arch_memory_mapping.c |  16 ++--
 target-i386/cpu-qom.h             |   3 +
 target-i386/cpu.c                 |  26 ++++---
 22 files changed, 336 insertions(+), 185 deletions(-)
 delete mode 100644 memory_mapping-stub.c
 rename dump-stub.c => stubs/dump.c (65%)

-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 01/18] dump: Move stubs into libqemustub.a
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 02/18] pc: Fix crash when attempting to hotplug CPU with negative ID Andreas Färber
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

This allows us to drop CONFIG_NO_CORE_DUMP with its indirect dependency
on CONFIG_HAVE_CORE_DUMP.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 Makefile.target             | 2 --
 stubs/Makefile.objs         | 1 +
 dump-stub.c => stubs/dump.c | 0
 3 files changed, 1 insertion(+), 2 deletions(-)
 rename dump-stub.c => stubs/dump.c (100%)

diff --git a/Makefile.target b/Makefile.target
index ce4391f..1cafb17 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -64,7 +64,6 @@ CONFIG_NO_PCI = $(if $(subst n,,$(CONFIG_PCI)),n,y)
 CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
 CONFIG_NO_XEN = $(if $(subst n,,$(CONFIG_XEN)),n,y)
 CONFIG_NO_GET_MEMORY_MAPPING = $(if $(subst n,,$(CONFIG_HAVE_GET_MEMORY_MAPPING)),n,y)
-CONFIG_NO_CORE_DUMP = $(if $(subst n,,$(CONFIG_HAVE_CORE_DUMP)),n,y)
 
 #########################################################
 # cpu emulator library
@@ -114,7 +113,6 @@ obj-y += memory.o savevm.o cputlb.o
 obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
 obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
 obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
-obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
 LIBS+=$(libs_softmmu)
 
 # xen support
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 03dff20..9b701b4 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -2,6 +2,7 @@ stub-obj-y += arch-query-cpu-def.o
 stub-obj-y += clock-warp.o
 stub-obj-y += cpu-get-clock.o
 stub-obj-y += cpu-get-icount.o
+stub-obj-y += dump.o
 stub-obj-y += fdset-add-fd.o
 stub-obj-y += fdset-find-fd.o
 stub-obj-y += fdset-get-fd.o
diff --git a/dump-stub.c b/stubs/dump.c
similarity index 100%
rename from dump-stub.c
rename to stubs/dump.c
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 02/18] pc: Fix crash when attempting to hotplug CPU with negative ID
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 01/18] dump: Move stubs into libqemustub.a Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 03/18] pc: Create pc-*-1.6 machine-types Andreas Färber
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: Igor Mammedov, qiaonuohan, Anthony Liguori, Andreas Färber,
	lcapitulino

From: Igor Mammedov <imammedo@redhat.com>

QMP command "{ 'execute': 'cpu-add', 'arguments': { 'id': -1 }}" may cause
QEMU SIGSEGV at:
 piix4_cpu_hotplug_req ()
    ...
    g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
    ...

Since for PC in current implementation id should be in range [0...maxcpus)
and maxcpus is already checked, add check for lower bound and error out
on incorrect value.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/i386/pc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4844a6b..553becb 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -927,6 +927,11 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
     DeviceState *icc_bridge;
     int64_t apic_id = x86_cpu_apic_id_from_index(id);
 
+    if (id < 0) {
+        error_setg(errp, "Invalid CPU id: %" PRIi64, id);
+        return;
+    }
+
     if (cpu_exists(apic_id)) {
         error_setg(errp, "Unable to add CPU: %" PRIi64
                    ", it already exists", id);
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 03/18] pc: Create pc-*-1.6 machine-types
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 01/18] dump: Move stubs into libqemustub.a Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 02/18] pc: Fix crash when attempting to hotplug CPU with negative ID Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 04/18] target-i386: Update model values on Conroe/Penryn/Nehalem CPU models Andreas Färber
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: qiaonuohan, Anthony Liguori, Eduardo Habkost,
	Andreas Färber, lcapitulino

From: Eduardo Habkost <ehabkost@redhat.com>

Some CPU model fixes are going to be included and they will require
compatibility properties in the pc-*-1.5 machine-types.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/i386/pc_piix.c | 14 ++++++++++++--
 hw/i386/pc_q35.c  | 12 +++++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d618570..7fb2bce 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -327,8 +327,8 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
 }
 #endif
 
-static QEMUMachine pc_i440fx_machine_v1_5 = {
-    .name = "pc-i440fx-1.5",
+static QEMUMachine pc_i440fx_machine_v1_6 = {
+    .name = "pc-i440fx-1.6",
     .alias = "pc",
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci,
@@ -338,6 +338,15 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
     DEFAULT_MACHINE_OPTIONS,
 };
 
+static QEMUMachine pc_i440fx_machine_v1_5 = {
+    .name = "pc-i440fx-1.5",
+    .desc = "Standard PC (i440FX + PIIX, 1996)",
+    .init = pc_init_pci,
+    .hot_add_cpu = pc_hot_add_cpu,
+    .max_cpus = 255,
+    DEFAULT_MACHINE_OPTIONS,
+};
+
 static QEMUMachine pc_i440fx_machine_v1_4 = {
     .name = "pc-i440fx-1.4",
     .desc = "Standard PC (i440FX + PIIX, 1996)",
@@ -735,6 +744,7 @@ static QEMUMachine xenfv_machine = {
 
 static void pc_machine_init(void)
 {
+    qemu_register_machine(&pc_i440fx_machine_v1_6);
     qemu_register_machine(&pc_i440fx_machine_v1_5);
     qemu_register_machine(&pc_i440fx_machine_v1_4);
     qemu_register_machine(&pc_machine_v1_3);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 7888dfe..db5c46e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -215,9 +215,18 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
     pc_q35_init(args);
 }
 
+static QEMUMachine pc_q35_machine_v1_6 = {
+    .name = "pc-q35-1.6",
+    .alias = "q35",
+    .desc = "Standard PC (Q35 + ICH9, 2009)",
+    .init = pc_q35_init,
+    .hot_add_cpu = pc_hot_add_cpu,
+    .max_cpus = 255,
+    DEFAULT_MACHINE_OPTIONS,
+};
+
 static QEMUMachine pc_q35_machine_v1_5 = {
     .name = "pc-q35-1.5",
-    .alias = "q35",
     .desc = "Standard PC (Q35 + ICH9, 2009)",
     .init = pc_q35_init,
     .hot_add_cpu = pc_hot_add_cpu,
@@ -239,6 +248,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
 
 static void pc_q35_machine_init(void)
 {
+    qemu_register_machine(&pc_q35_machine_v1_6);
     qemu_register_machine(&pc_q35_machine_v1_5);
     qemu_register_machine(&pc_q35_machine_v1_4);
 }
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 04/18] target-i386: Update model values on Conroe/Penryn/Nehalem CPU models
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (2 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 03/18] pc: Create pc-*-1.6 machine-types Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 05/18] target-i386: Set level=4 on Conroe/Penryn/Nehalem Andreas Färber
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel
  Cc: qiaonuohan, Anthony Liguori, Eduardo Habkost,
	Andreas Färber, lcapitulino

From: Eduardo Habkost <ehabkost@redhat.com>

The CPUID model values on Conroe, Penryn, and Nehalem are too
conservative and don't reflect the values found on real Conroe, Penryn,
and Nehalem CPUs.

This causes at least one known problems: Windows XP disables sysenter
when (family == 6 && model <= 2), but Skype tries to use the sysenter
instruction anyway because it is reported as available on CPUID, making
it crash.

This patch sets appropriate model values that correspond to real Conroe,
Penryn, and Nehalem CPUs.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/i386/pc_piix.c    |  4 ++++
 hw/i386/pc_q35.c     |  4 ++++
 include/hw/i386/pc.h | 16 ++++++++++++++++
 target-i386/cpu.c    |  6 +++---
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7fb2bce..69eb2a1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -344,6 +344,10 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
     .init = pc_init_pci,
     .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        PC_COMPAT_1_5,
+        { /* end of list */ }
+    },
     DEFAULT_MACHINE_OPTIONS,
 };
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index db5c46e..bb0ce6a 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -231,6 +231,10 @@ static QEMUMachine pc_q35_machine_v1_5 = {
     .init = pc_q35_init,
     .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        PC_COMPAT_1_5,
+        { /* end of list */ }
+    },
     DEFAULT_MACHINE_OPTIONS,
 };
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6154058..f232d75 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -185,7 +185,23 @@ int pvpanic_init(ISABus *bus);
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
 
+#define PC_COMPAT_1_5 \
+        {\
+            .driver   = "Conroe-" TYPE_X86_CPU,\
+            .property = "model",\
+            .value    = stringify(2),\
+        },{\
+            .driver   = "Penryn-" TYPE_X86_CPU,\
+            .property = "model",\
+            .value    = stringify(2),\
+        },{\
+            .driver   = "Nehalem-" TYPE_X86_CPU,\
+            .property = "model",\
+            .value    = stringify(2),\
+        }
+
 #define PC_COMPAT_1_4 \
+        PC_COMPAT_1_5, \
         {\
             .driver   = "scsi-hd",\
             .property = "discard_granularity",\
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1a501d9..6b48562 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -672,7 +672,7 @@ static x86_def_t builtin_x86_defs[] = {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
-        .model = 2,
+        .model = 15,
         .stepping = 3,
         .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
@@ -694,7 +694,7 @@ static x86_def_t builtin_x86_defs[] = {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
-        .model = 2,
+        .model = 23,
         .stepping = 3,
         .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
@@ -717,7 +717,7 @@ static x86_def_t builtin_x86_defs[] = {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
-        .model = 2,
+        .model = 26,
         .stepping = 3,
         .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 05/18] target-i386: Set level=4 on Conroe/Penryn/Nehalem
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (3 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 04/18] target-i386: Update model values on Conroe/Penryn/Nehalem CPU models Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 06/18] target-i386: cpu: Fix potential buffer overrun in get_register_name_32() Andreas Färber
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Eduardo Habkost, Andreas Färber, lcapitulino

From: Eduardo Habkost <ehabkost@redhat.com>

The CPUID level value on Conroe, Penryn, and Nehalem are too low. This
causes at least one known problem: the -smp "threads" option doesn't
work as expect if level is < 4, because thread count information is
provided to the guest on CPUID[EAX=4,ECX=2].EAX

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/hw/i386/pc.h | 12 ++++++++++++
 target-i386/cpu.c    |  6 +++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index f232d75..fab9be5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -191,13 +191,25 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
             .property = "model",\
             .value    = stringify(2),\
         },{\
+            .driver   = "Conroe-" TYPE_X86_CPU,\
+            .property = "level",\
+            .value    = stringify(2),\
+        },{\
             .driver   = "Penryn-" TYPE_X86_CPU,\
             .property = "model",\
             .value    = stringify(2),\
         },{\
+            .driver   = "Penryn-" TYPE_X86_CPU,\
+            .property = "level",\
+            .value    = stringify(2),\
+        },{\
             .driver   = "Nehalem-" TYPE_X86_CPU,\
             .property = "model",\
             .value    = stringify(2),\
+        },{\
+            .driver   = "Nehalem-" TYPE_X86_CPU,\
+            .property = "level",\
+            .value    = stringify(2),\
         }
 
 #define PC_COMPAT_1_4 \
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6b48562..762baad 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -669,7 +669,7 @@ static x86_def_t builtin_x86_defs[] = {
     },
     {
         .name = "Conroe",
-        .level = 2,
+        .level = 4,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 15,
@@ -691,7 +691,7 @@ static x86_def_t builtin_x86_defs[] = {
     },
     {
         .name = "Penryn",
-        .level = 2,
+        .level = 4,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 23,
@@ -714,7 +714,7 @@ static x86_def_t builtin_x86_defs[] = {
     },
     {
         .name = "Nehalem",
-        .level = 2,
+        .level = 4,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
         .model = 26,
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 06/18] target-i386: cpu: Fix potential buffer overrun in get_register_name_32()
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (4 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 05/18] target-i386: Set level=4 on Conroe/Penryn/Nehalem Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 07/18] cpu: Turn cpu_paging_enabled() into a CPUState hook Andreas Färber
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, qiaonuohan, Andreas Färber, lcapitulino

From: Igor Mammedov <imammedo@redhat.com>

Spotted by Coverity,
x86_reg_info_32[] is CPU_NB_REGS32 elements long, so accessing
x86_reg_info_32[CPU_NB_REGS32] will be one element off array.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: liguang <lig.fnst@cn.fujitsu.com>
Reviewed by: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 762baad..4b2da0d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -221,7 +221,7 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
 
 const char *get_register_name_32(unsigned int reg)
 {
-    if (reg > CPU_NB_REGS32) {
+    if (reg >= CPU_NB_REGS32) {
         return NULL;
     }
     return x86_reg_info_32[reg].name;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 07/18] cpu: Turn cpu_paging_enabled() into a CPUState hook
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (5 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 06/18] target-i386: cpu: Fix potential buffer overrun in get_register_name_32() Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-11  8:06   ` Jens Freimann
  2013-06-11 14:52   ` Luiz Capitulino
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 08/18] memory_mapping: Move MemoryMappingList typedef to qemu/typedefs.h Andreas Färber
                   ` (12 subsequent siblings)
  19 siblings, 2 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

Relocate assignment of x86 get_arch_id to have all hooks in one place.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/qom/cpu.h                 | 10 ++++++++++
 include/sysemu/memory_mapping.h   |  1 -
 memory_mapping-stub.c             |  6 ------
 memory_mapping.c                  |  2 +-
 qom/cpu.c                         | 13 +++++++++++++
 target-i386/arch_memory_mapping.c |  6 +-----
 target-i386/cpu.c                 | 11 +++++++++--
 7 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7cd9442..1f70240 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -48,6 +48,7 @@ typedef struct CPUState CPUState;
  * @reset: Callback to reset the #CPUState to its initial state.
  * @do_interrupt: Callback for interrupt handling.
  * @get_arch_id: Callback for getting architecture-dependent CPU ID.
+ * @get_paging_enabled: Callback for inquiring whether paging is enabled.
  * @vmsd: State description for migration.
  *
  * Represents a CPU family or model.
@@ -62,6 +63,7 @@ typedef struct CPUClass {
     void (*reset)(CPUState *cpu);
     void (*do_interrupt)(CPUState *cpu);
     int64_t (*get_arch_id)(CPUState *cpu);
+    bool (*get_paging_enabled)(const CPUState *cpu);
 
     const struct VMStateDescription *vmsd;
     int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
@@ -138,6 +140,14 @@ struct CPUState {
 };
 
 /**
+ * cpu_paging_enabled:
+ * @cpu: The CPU whose state is to be inspected.
+ *
+ * Returns: %true if paging is enabled, %false otherwise.
+ */
+bool cpu_paging_enabled(const CPUState *cpu);
+
+/**
  * cpu_write_elf64_note:
  * @f: pointer to a function that writes memory to a file
  * @cpu: The CPU whose memory is to be dumped
diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index 1256125..6f01524 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -31,7 +31,6 @@ typedef struct MemoryMappingList {
 } MemoryMappingList;
 
 int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
-bool cpu_paging_enabled(CPUArchState *env);
 
 /*
  * add or merge the memory region [phys_addr, phys_addr + length) into the
diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
index 24d5d67..6c0dfeb 100644
--- a/memory_mapping-stub.c
+++ b/memory_mapping-stub.c
@@ -25,9 +25,3 @@ int cpu_get_memory_mapping(MemoryMappingList *list,
 {
     return -1;
 }
-
-bool cpu_paging_enabled(CPUArchState *env)
-{
-    return true;
-}
-
diff --git a/memory_mapping.c b/memory_mapping.c
index ff45b3a..0790aac 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -170,7 +170,7 @@ static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu)
     CPUArchState *env;
 
     for (env = start_cpu; env != NULL; env = env->next_cpu) {
-        if (cpu_paging_enabled(env)) {
+        if (cpu_paging_enabled(ENV_GET_CPU(env))) {
             return env;
         }
     }
diff --git a/qom/cpu.c b/qom/cpu.c
index 04aefbb..9f6da0f 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -50,6 +50,18 @@ bool cpu_exists(int64_t id)
     return data.found;
 }
 
+bool cpu_paging_enabled(const CPUState *cpu)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
+    return cc->get_paging_enabled(cpu);
+}
+
+static bool cpu_common_get_paging_enabled(const CPUState *cpu)
+{
+    return true;
+}
+
 /* CPU hot-plug notifiers */
 static NotifierList cpu_added_notifiers =
     NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
@@ -176,6 +188,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
     k->class_by_name = cpu_common_class_by_name;
     k->reset = cpu_common_reset;
     k->get_arch_id = cpu_common_get_arch_id;
+    k->get_paging_enabled = cpu_common_get_paging_enabled;
     k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
     k->write_elf32_note = cpu_common_write_elf32_note;
     k->write_elf64_qemunote = cpu_common_write_elf64_qemunote;
diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c
index 5096fbd..c5a10ec 100644
--- a/target-i386/arch_memory_mapping.c
+++ b/target-i386/arch_memory_mapping.c
@@ -241,7 +241,7 @@ static void walk_pml4e(MemoryMappingList *list,
 
 int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
 {
-    if (!cpu_paging_enabled(env)) {
+    if (!cpu_paging_enabled(ENV_GET_CPU(env))) {
         /* paging is disabled */
         return 0;
     }
@@ -273,7 +273,3 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
     return 0;
 }
 
-bool cpu_paging_enabled(CPUArchState *env)
-{
-    return env->cr[0] & CR0_PG_MASK;
-}
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4b2da0d..f6fa7fa 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2505,6 +2505,13 @@ static int64_t x86_cpu_get_arch_id(CPUState *cs)
     return env->cpuid_apic_id;
 }
 
+static bool x86_cpu_get_paging_enabled(const CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+
+    return cpu->env.cr[0] & CR0_PG_MASK;
+}
+
 static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 {
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
@@ -2519,6 +2526,8 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->reset = x86_cpu_reset;
 
     cc->do_interrupt = x86_cpu_do_interrupt;
+    cc->get_arch_id = x86_cpu_get_arch_id;
+    cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 #ifndef CONFIG_USER_ONLY
     cc->write_elf64_note = x86_cpu_write_elf64_note;
     cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
@@ -2526,8 +2535,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->write_elf32_qemunote = x86_cpu_write_elf32_qemunote;
 #endif
     cpu_class_set_vmsd(cc, &vmstate_x86_cpu);
-
-    cc->get_arch_id = x86_cpu_get_arch_id;
 }
 
 static const TypeInfo x86_cpu_type_info = {
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 08/18] memory_mapping: Move MemoryMappingList typedef to qemu/typedefs.h
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (6 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 07/18] cpu: Turn cpu_paging_enabled() into a CPUState hook Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook Andreas Färber
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

This will avoid issues with hwaddr and ram_addr_t when including
sysemu/memory_mapping.h for CONFIG_USER_ONLY.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/qemu/typedefs.h         | 2 ++
 include/sysemu/memory_mapping.h | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index afe4ec7..698fc03 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -22,6 +22,8 @@ typedef struct AddressSpace AddressSpace;
 typedef struct MemoryRegion MemoryRegion;
 typedef struct MemoryRegionSection MemoryRegionSection;
 
+typedef struct MemoryMappingList MemoryMappingList;
+
 typedef struct NICInfo NICInfo;
 typedef struct HCIInfo HCIInfo;
 typedef struct AudioState AudioState;
diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index 6f01524..1f71c32 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -15,6 +15,7 @@
 #define MEMORY_MAPPING_H
 
 #include "qemu/queue.h"
+#include "qemu/typedefs.h"
 
 /* The physical and virtual address in the memory mapping are contiguous. */
 typedef struct MemoryMapping {
@@ -24,11 +25,11 @@ typedef struct MemoryMapping {
     QTAILQ_ENTRY(MemoryMapping) next;
 } MemoryMapping;
 
-typedef struct MemoryMappingList {
+struct MemoryMappingList {
     unsigned int num;
     MemoryMapping *last_mapping;
     QTAILQ_HEAD(, MemoryMapping) head;
-} MemoryMappingList;
+};
 
 int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (7 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 08/18] memory_mapping: Move MemoryMappingList typedef to qemu/typedefs.h Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-11  9:20   ` Jens Freimann
  2013-06-11 14:56   ` Luiz Capitulino
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 10/18] memory_mapping: Drop qemu_get_memory_mapping() stub Andreas Färber
                   ` (10 subsequent siblings)
  19 siblings, 2 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

Change error reporting from return value to Error argument.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/qom/cpu.h                 | 14 ++++++++++++++
 include/sysemu/memory_mapping.h   |  2 --
 memory_mapping-stub.c             |  6 ------
 memory_mapping.c                  |  7 ++++---
 qom/cpu.c                         | 16 ++++++++++++++++
 target-i386/arch_memory_mapping.c | 12 +++++++-----
 target-i386/cpu-qom.h             |  3 +++
 target-i386/cpu.c                 |  1 +
 8 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 1f70240..254be2e 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -23,6 +23,7 @@
 #include <signal.h>
 #include "hw/qdev-core.h"
 #include "qemu/thread.h"
+#include "qemu/typedefs.h"
 
 typedef int (*WriteCoreDumpFunction)(void *buf, size_t size, void *opaque);
 
@@ -49,6 +50,7 @@ typedef struct CPUState CPUState;
  * @do_interrupt: Callback for interrupt handling.
  * @get_arch_id: Callback for getting architecture-dependent CPU ID.
  * @get_paging_enabled: Callback for inquiring whether paging is enabled.
+ * @get_memory_mapping: Callback for obtaining the memory mappings.
  * @vmsd: State description for migration.
  *
  * Represents a CPU family or model.
@@ -64,6 +66,8 @@ typedef struct CPUClass {
     void (*do_interrupt)(CPUState *cpu);
     int64_t (*get_arch_id)(CPUState *cpu);
     bool (*get_paging_enabled)(const CPUState *cpu);
+    void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
+                               Error **errp);
 
     const struct VMStateDescription *vmsd;
     int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
@@ -148,6 +152,16 @@ struct CPUState {
 bool cpu_paging_enabled(const CPUState *cpu);
 
 /**
+ * @cpu: The CPU whose memory mappings are to be obtained.
+ * @list: Where to write the memory mappings to.
+ * @errp: Pointer for reporting an #Error.
+ *
+ * Returns: 0 if successful.
+ */
+void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
+                            Error **errp);
+
+/**
  * cpu_write_elf64_note:
  * @f: pointer to a function that writes memory to a file
  * @cpu: The CPU whose memory is to be dumped
diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index 1f71c32..c47e6ee 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -31,8 +31,6 @@ struct MemoryMappingList {
     QTAILQ_HEAD(, MemoryMapping) head;
 };
 
-int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
-
 /*
  * add or merge the memory region [phys_addr, phys_addr + length) into the
  * memory mapping's list. The region's virtual address starts with virt_addr,
diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
index 6c0dfeb..989dc00 100644
--- a/memory_mapping-stub.c
+++ b/memory_mapping-stub.c
@@ -19,9 +19,3 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
 {
     return -2;
 }
-
-int cpu_get_memory_mapping(MemoryMappingList *list,
-					                                          CPUArchState *env)
-{
-    return -1;
-}
diff --git a/memory_mapping.c b/memory_mapping.c
index 0790aac..9bd24ce 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -183,13 +183,14 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
     CPUArchState *env, *first_paging_enabled_cpu;
     RAMBlock *block;
     ram_addr_t offset, length;
-    int ret;
 
     first_paging_enabled_cpu = find_paging_enabled_cpu(first_cpu);
     if (first_paging_enabled_cpu) {
         for (env = first_paging_enabled_cpu; env != NULL; env = env->next_cpu) {
-            ret = cpu_get_memory_mapping(list, env);
-            if (ret < 0) {
+            Error *err = NULL;
+            cpu_get_memory_mapping(ENV_GET_CPU(env), list, &err);
+            if (err) {
+                error_free(err);
                 return -1;
             }
         }
diff --git a/qom/cpu.c b/qom/cpu.c
index 9f6da0f..b25fbc9 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -62,6 +62,21 @@ static bool cpu_common_get_paging_enabled(const CPUState *cpu)
     return true;
 }
 
+void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
+                            Error **errp)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
+    return cc->get_memory_mapping(cpu, list, errp);
+}
+
+static void cpu_common_get_memory_mapping(CPUState *cpu,
+                                          MemoryMappingList *list,
+                                          Error **errp)
+{
+    error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
+}
+
 /* CPU hot-plug notifiers */
 static NotifierList cpu_added_notifiers =
     NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
@@ -189,6 +204,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
     k->reset = cpu_common_reset;
     k->get_arch_id = cpu_common_get_arch_id;
     k->get_paging_enabled = cpu_common_get_paging_enabled;
+    k->get_memory_mapping = cpu_common_get_memory_mapping;
     k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
     k->write_elf32_note = cpu_common_write_elf32_note;
     k->write_elf64_qemunote = cpu_common_write_elf64_qemunote;
diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c
index c5a10ec..2566a04 100644
--- a/target-i386/arch_memory_mapping.c
+++ b/target-i386/arch_memory_mapping.c
@@ -239,11 +239,15 @@ static void walk_pml4e(MemoryMappingList *list,
 }
 #endif
 
-int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
+void x86_cpu_get_memory_mapping(CPUState *cs, MemoryMappingList *list,
+                                Error **errp)
 {
-    if (!cpu_paging_enabled(ENV_GET_CPU(env))) {
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+
+    if (!cpu_paging_enabled(cs)) {
         /* paging is disabled */
-        return 0;
+        return;
     }
 
     if (env->cr[4] & CR4_PAE_MASK) {
@@ -269,7 +273,5 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
         pse = !!(env->cr[4] & CR4_PSE_MASK);
         walk_pde2(list, pde_addr, env->a20_mask, pse);
     }
-
-    return 0;
 }
 
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 849cedf..e0ac072 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -98,4 +98,7 @@ int x86_cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
 int x86_cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
                                  void *opaque);
 
+void x86_cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
+                                Error **errp);
+
 #endif
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f6fa7fa..a7154af 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2529,6 +2529,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->get_arch_id = x86_cpu_get_arch_id;
     cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 #ifndef CONFIG_USER_ONLY
+    cc->get_memory_mapping = x86_cpu_get_memory_mapping;
     cc->write_elf64_note = x86_cpu_write_elf64_note;
     cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
     cc->write_elf32_note = x86_cpu_write_elf32_note;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 10/18] memory_mapping: Drop qemu_get_memory_mapping() stub
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (8 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 11/18] dump: Drop qmp_dump_guest_memory() stub and build for all targets Andreas Färber
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

dump.c:dump_init() never checked for the return code anyway.
If paging is not enabled, it will fall back to an identity map.
If paging is enabled and getting memory mapping list is not
implemented, qemu_get_guest_memory_mapping() will return an error.

Since the targets not implementing memory mapping also don't implement
dump support, we will not reach this code today and can worry about
changing cpu_paging_enabled() default when the need arises.

This allows us to drop CONFIG_HAVE_GET_MEMORY_SUPPORT.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 Makefile.target       |  4 +---
 configure             |  4 ----
 memory_mapping-stub.c | 21 ---------------------
 3 files changed, 1 insertion(+), 28 deletions(-)
 delete mode 100644 memory_mapping-stub.c

diff --git a/Makefile.target b/Makefile.target
index 1cafb17..f9e1d89 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -63,7 +63,6 @@ all: $(PROGS) stap
 CONFIG_NO_PCI = $(if $(subst n,,$(CONFIG_PCI)),n,y)
 CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
 CONFIG_NO_XEN = $(if $(subst n,,$(CONFIG_XEN)),n,y)
-CONFIG_NO_GET_MEMORY_MAPPING = $(if $(subst n,,$(CONFIG_HAVE_GET_MEMORY_MAPPING)),n,y)
 
 #########################################################
 # cpu emulator library
@@ -110,9 +109,8 @@ obj-y += hw/
 obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
 obj-y += memory.o savevm.o cputlb.o
-obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
+obj-y += memory_mapping.o
 obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
-obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
 LIBS+=$(libs_softmmu)
 
 # xen support
diff --git a/configure b/configure
index 1654413..6401762 100755
--- a/configure
+++ b/configure
@@ -4298,10 +4298,6 @@ case "$target_arch2" in
       fi
     fi
 esac
-case "$target_arch2" in
-  i386|x86_64)
-    echo "CONFIG_HAVE_GET_MEMORY_MAPPING=y" >> $config_target_mak
-esac
 if test "$target_bigendian" = "yes" ; then
   echo "TARGET_WORDS_BIGENDIAN=y" >> $config_target_mak
 fi
diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
deleted file mode 100644
index 989dc00..0000000
--- a/memory_mapping-stub.c
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * QEMU memory mapping
- *
- * Copyright Fujitsu, Corp. 2011, 2012
- *
- * Authors:
- *     Wen Congyang <wency@cn.fujitsu.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#include "cpu.h"
-#include "exec/cpu-all.h"
-#include "sysemu/memory_mapping.h"
-
-int qemu_get_guest_memory_mapping(MemoryMappingList *list)
-{
-    return -2;
-}
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 11/18] dump: Drop qmp_dump_guest_memory() stub and build for all targets
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (9 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 10/18] memory_mapping: Drop qemu_get_memory_mapping() stub Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 12/18] cpu: Change default for CPUClass::get_paging_enabled() Andreas Färber
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

qmp_dump_guest_memory() calls dump_init() and returns an Error when
cpu_get_dump_info() returns an error, as done by the stub.
So there is no need to have a stub for qmp_dump_guest_memory().

Enable the documentation of the always-present dump-guest-memory command.

That way we can drop CONFIG_HAVE_CORE_DUMP and leave configure
completely out of the picture for target CPU features.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 Makefile.target | 2 +-
 configure       | 4 ----
 hmp-commands.hx | 2 --
 stubs/dump.c    | 8 --------
 4 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index f9e1d89..b0be124 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -110,7 +110,7 @@ obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
 obj-y += memory.o savevm.o cputlb.o
 obj-y += memory_mapping.o
-obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
+obj-y += dump.o
 LIBS+=$(libs_softmmu)
 
 # xen support
diff --git a/configure b/configure
index 6401762..c61d862 100755
--- a/configure
+++ b/configure
@@ -4303,10 +4303,6 @@ if test "$target_bigendian" = "yes" ; then
 fi
 if test "$target_softmmu" = "yes" ; then
   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
-  case "$target_arch2" in
-    i386|x86_64)
-      echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
-  esac
 fi
 if test "$target_user_only" = "yes" ; then
   echo "CONFIG_USER_ONLY=y" >> $config_target_mak
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 396691a..915b0d1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -991,7 +991,6 @@ server will ask the spice/vnc client to automatically reconnect using the
 new parameters (if specified) once the vm migration finished successfully.
 ETEXI
 
-#if defined(CONFIG_HAVE_CORE_DUMP)
     {
         .name       = "dump-guest-memory",
         .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
@@ -1015,7 +1014,6 @@ gdb.
     length: the memory size, in bytes. It's optional, and should be specified
             with begin together.
 ETEXI
-#endif
 
     {
         .name       = "snapshot_blkdev",
diff --git a/stubs/dump.c b/stubs/dump.c
index b3f42cb..43c9a3f 100644
--- a/stubs/dump.c
+++ b/stubs/dump.c
@@ -16,14 +16,6 @@
 #include "qapi/qmp/qerror.h"
 #include "qmp-commands.h"
 
-/* we need this function in hmp.c */
-void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
-                           int64_t begin, bool has_length, int64_t length,
-                           Error **errp)
-{
-    error_set(errp, QERR_UNSUPPORTED);
-}
-
 int cpu_get_dump_info(ArchDumpInfo *info)
 {
     return -1;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 12/18] cpu: Change default for CPUClass::get_paging_enabled()
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (10 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 11/18] dump: Drop qmp_dump_guest_memory() stub and build for all targets Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-11  9:00   ` Jens Freimann
  2013-06-11 15:01   ` Luiz Capitulino
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 13/18] memory_mapping: Cleanup qemu_get_guest_memory_mapping() Andreas Färber
                   ` (7 subsequent siblings)
  19 siblings, 2 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

qemu_get_guest_memory_mapping() uses cpu_paging_enabled() to determine
whether to use cpu_get_memory_mapping() to return mappings or whether to
fall back to a simple identity map.

Since by default CPUClass::get_memory_mapping() is not implemented,
change the default to false to use the identity map by default.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 qom/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index b25fbc9..dba4a11 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -59,7 +59,7 @@ bool cpu_paging_enabled(const CPUState *cpu)
 
 static bool cpu_common_get_paging_enabled(const CPUState *cpu)
 {
-    return true;
+    return false;
 }
 
 void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 13/18] memory_mapping: Cleanup qemu_get_guest_memory_mapping()
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (11 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 12/18] cpu: Change default for CPUClass::get_paging_enabled() Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-11 15:52   ` Luiz Capitulino
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 14/18] dump: Abstract dump_init() with cpu_synchronize_all_states() Andreas Färber
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

We relied on the CPUClass::get_memory_mapping() implementation being a
no-op when paging is disabled for that CPU. Therefore we can just as well
iterate over all CPUs to retrieve mappings.

This allows to use qemu_for_each_cpu() instead of open-coding CPU loops.

Pass any Error out into dump_init() and have it actually stop on errors.
Whether it is unsupported on a certain CPU can be checked by looking for
a NULL CPUClass::get_memory_mapping field.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 dump.c                          |  7 +++++-
 include/sysemu/memory_mapping.h |  8 +------
 memory_mapping.c                | 52 ++++++++++++++++++++++++-----------------
 3 files changed, 38 insertions(+), 29 deletions(-)

diff --git a/dump.c b/dump.c
index c0d3da5..b44dafc 100644
--- a/dump.c
+++ b/dump.c
@@ -706,6 +706,7 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
 {
     CPUArchState *env;
     int nr_cpus;
+    Error *err = NULL;
     int ret;
 
     if (runstate_is_running()) {
@@ -756,7 +757,11 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
     /* get memory mapping */
     memory_mapping_list_init(&s->list);
     if (paging) {
-        qemu_get_guest_memory_mapping(&s->list);
+        qemu_get_guest_memory_mapping(&s->list, &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            goto cleanup;
+        }
     } else {
         qemu_get_guest_simple_memory_mapping(&s->list);
     }
diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index c47e6ee..6dfb68d 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -45,13 +45,7 @@ void memory_mapping_list_free(MemoryMappingList *list);
 
 void memory_mapping_list_init(MemoryMappingList *list);
 
-/*
- * Return value:
- *    0: success
- *   -1: failed
- *   -2: unsupported
- */
-int qemu_get_guest_memory_mapping(MemoryMappingList *list);
+void qemu_get_guest_memory_mapping(MemoryMappingList *list, Error **errp);
 
 /* get guest's memory mapping without do paging(virtual address is 0). */
 void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list);
diff --git a/memory_mapping.c b/memory_mapping.c
index 9bd24ce..a19be54 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -165,36 +165,48 @@ void memory_mapping_list_init(MemoryMappingList *list)
     QTAILQ_INIT(&list->head);
 }
 
-static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu)
+static void find_paging_enabled_cpu(CPUState *cpu, void *data)
 {
-    CPUArchState *env;
+    bool *ret = data;
 
-    for (env = start_cpu; env != NULL; env = env->next_cpu) {
-        if (cpu_paging_enabled(ENV_GET_CPU(env))) {
-            return env;
-        }
+    if (*ret) {
+        return;
     }
+    *ret = cpu_paging_enabled(cpu);
+}
+
+typedef struct GetGuestMemoryMappingData {
+    MemoryMappingList *list;
+    Error *err;
+} GetGuestMemoryMappingData;
+
+static void qemu_get_one_guest_memory_mapping(CPUState *cpu, void *data)
+{
+    GetGuestMemoryMappingData *s = data;
 
-    return NULL;
+    if (s->err != NULL || !cpu_paging_enabled(cpu)) {
+        return;
+    }
+    cpu_get_memory_mapping(cpu, s->list, &s->err);
 }
 
-int qemu_get_guest_memory_mapping(MemoryMappingList *list)
+void qemu_get_guest_memory_mapping(MemoryMappingList *list, Error **errp)
 {
-    CPUArchState *env, *first_paging_enabled_cpu;
+    GetGuestMemoryMappingData s = {
+        .list = list,
+        .err = NULL,
+    };
+    bool paging_enabled = false;
     RAMBlock *block;
     ram_addr_t offset, length;
 
-    first_paging_enabled_cpu = find_paging_enabled_cpu(first_cpu);
-    if (first_paging_enabled_cpu) {
-        for (env = first_paging_enabled_cpu; env != NULL; env = env->next_cpu) {
-            Error *err = NULL;
-            cpu_get_memory_mapping(ENV_GET_CPU(env), list, &err);
-            if (err) {
-                error_free(err);
-                return -1;
-            }
+    qemu_for_each_cpu(find_paging_enabled_cpu, &paging_enabled);
+    if (paging_enabled) {
+        qemu_for_each_cpu(qemu_get_one_guest_memory_mapping, &s);
+        if (s.err != NULL) {
+            error_propagate(errp, s.err);
         }
-        return 0;
+        return;
     }
 
     /*
@@ -206,8 +218,6 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
         length = block->length;
         create_new_memory_mapping(list, offset, offset, length);
     }
-
-    return 0;
 }
 
 void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 14/18] dump: Abstract dump_init() with cpu_synchronize_all_states()
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (12 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 13/18] memory_mapping: Cleanup qemu_get_guest_memory_mapping() Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-11 15:55   ` Luiz Capitulino
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 15/18] dump: Abstract dump_init() further with qemu_for_each_cpu() Andreas Färber
                   ` (5 subsequent siblings)
  19 siblings, 1 reply; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

Instead of calling cpu_synchronize_state() for each CPU, call the
existing cpu_synchronize_all_states() helper.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 dump.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/dump.c b/dump.c
index b44dafc..44a1339 100644
--- a/dump.c
+++ b/dump.c
@@ -21,6 +21,7 @@
 #include "sysemu/dump.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/memory_mapping.h"
+#include "sysemu/cpus.h"
 #include "qapi/error.h"
 #include "qmp-commands.h"
 
@@ -732,12 +733,12 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
      * If the target architecture is not supported, cpu_get_dump_info() will
      * return -1.
      *
-     * if we use kvm, we should synchronize the register before we get dump
+     * If we use KVM, we should synchronize the registers before we get dump
      * info.
      */
+    cpu_synchronize_all_states();
     nr_cpus = 0;
     for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        cpu_synchronize_state(env);
         nr_cpus++;
     }
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 15/18] dump: Abstract dump_init() further with qemu_for_each_cpu()
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (13 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 14/18] dump: Abstract dump_init() with cpu_synchronize_all_states() Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-11 15:55   ` Luiz Capitulino
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 16/18] dump: Abstract write_elf{64, 32}_notes() " Andreas Färber
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

Allows to drop CPUArchState variable.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 dump.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/dump.c b/dump.c
index 44a1339..4e6b855 100644
--- a/dump.c
+++ b/dump.c
@@ -702,10 +702,16 @@ static ram_addr_t get_start_block(DumpState *s)
     return -1;
 }
 
+static void count_one_cpu(CPUState *cpu, void *data)
+{
+    int *nr = data;
+
+    *nr = *nr + 1;
+}
+
 static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
                      int64_t begin, int64_t length, Error **errp)
 {
-    CPUArchState *env;
     int nr_cpus;
     Error *err = NULL;
     int ret;
@@ -738,9 +744,7 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
      */
     cpu_synchronize_all_states();
     nr_cpus = 0;
-    for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        nr_cpus++;
-    }
+    qemu_for_each_cpu(count_one_cpu, &nr_cpus);
 
     ret = cpu_get_dump_info(&s->dump_info);
     if (ret < 0) {
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 16/18] dump: Abstract write_elf{64, 32}_notes() with qemu_for_each_cpu()
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (14 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 15/18] dump: Abstract dump_init() further with qemu_for_each_cpu() Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 17/18] memory_mapping: Use hwaddr type for MemoryMapping virt_addr field Andreas Färber
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

Removes the last use of CPUArchState.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 dump.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 97 insertions(+), 36 deletions(-)

diff --git a/dump.c b/dump.c
index 4e6b855..d3cb8e2 100644
--- a/dump.c
+++ b/dump.c
@@ -273,29 +273,62 @@ static inline int cpu_index(CPUState *cpu)
     return cpu->cpu_index + 1;
 }
 
-static int write_elf64_notes(DumpState *s)
-{
-    CPUArchState *env;
-    CPUState *cpu;
+typedef struct WriteELFNoteData {
+    DumpState *state;
     int ret;
+} WriteELFNoteData;
+
+static void write_one_elf64_note(CPUState *cpu, void *data)
+{
+    WriteELFNoteData *d = data;
     int id;
+    int ret;
 
-    for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        cpu = ENV_GET_CPU(env);
-        id = cpu_index(cpu);
-        ret = cpu_write_elf64_note(fd_write_vmcore, cpu, id, s);
-        if (ret < 0) {
-            dump_error(s, "dump: failed to write elf notes.\n");
-            return -1;
-        }
+    if (d->ret != 0) {
+        return;
     }
 
-    for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        ret = cpu_write_elf64_qemunote(fd_write_vmcore, cpu, s);
-        if (ret < 0) {
-            dump_error(s, "dump: failed to write CPU status.\n");
-            return -1;
-        }
+    id = cpu_index(cpu);
+    ret = cpu_write_elf64_note(fd_write_vmcore, cpu, id, d->state);
+    if (ret < 0) {
+        dump_error(d->state, "dump: failed to write elf notes.\n");
+        d->ret = -1;
+        return;
+    }
+}
+
+static void write_one_elf64_qemunote(CPUState *cpu, void *data)
+{
+    WriteELFNoteData *d = data;
+    int ret;
+
+    if (d->ret != 0) {
+        return;
+    }
+
+    ret = cpu_write_elf64_qemunote(fd_write_vmcore, cpu, d->state);
+    if (ret < 0) {
+        dump_error(d->state, "dump: failed to write CPU status.\n");
+        d->ret = -1;
+        return;
+    }
+}
+
+static int write_elf64_notes(DumpState *s)
+{
+    WriteELFNoteData data = {
+        .state = s,
+        .ret = 0,
+    };
+
+    qemu_for_each_cpu(write_one_elf64_note, &data);
+    if (data.ret != 0) {
+        return data.ret;
+    }
+
+    qemu_for_each_cpu(write_one_elf64_qemunote, &data);
+    if (data.ret != 0) {
+        return data.ret;
     }
 
     return 0;
@@ -325,29 +358,57 @@ static int write_elf32_note(DumpState *s)
     return 0;
 }
 
-static int write_elf32_notes(DumpState *s)
+static void write_one_elf32_note(CPUState *cpu, void *data)
 {
-    CPUArchState *env;
-    CPUState *cpu;
-    int ret;
+    WriteELFNoteData *d = data;
     int id;
+    int ret;
 
-    for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        cpu = ENV_GET_CPU(env);
-        id = cpu_index(cpu);
-        ret = cpu_write_elf32_note(fd_write_vmcore, cpu, id, s);
-        if (ret < 0) {
-            dump_error(s, "dump: failed to write elf notes.\n");
-            return -1;
-        }
+    if (d->ret != 0) {
+        return;
     }
 
-    for (env = first_cpu; env != NULL; env = env->next_cpu) {
-        ret = cpu_write_elf32_qemunote(fd_write_vmcore, cpu, s);
-        if (ret < 0) {
-            dump_error(s, "dump: failed to write CPU status.\n");
-            return -1;
-        }
+    id = cpu_index(cpu);
+    ret = cpu_write_elf32_note(fd_write_vmcore, cpu, id, d->state);
+    if (ret < 0) {
+        dump_error(d->state, "dump: failed to write elf notes.\n");
+        d->ret = -1;
+        return;
+    }
+}
+
+static void write_one_elf32_qemunote(CPUState *cpu, void *data)
+{
+    WriteELFNoteData *d = data;
+    int ret;
+
+    if (d->ret != 0) {
+        return;
+    }
+
+    ret = cpu_write_elf32_qemunote(fd_write_vmcore, cpu, d->state);
+    if (ret < 0) {
+        dump_error(d->state, "dump: failed to write CPU status.\n");
+        d->ret = -1;
+        return;
+    }
+}
+
+static int write_elf32_notes(DumpState *s)
+{
+    WriteELFNoteData data = {
+        .state = s,
+        .ret = 0,
+    };
+
+    qemu_for_each_cpu(write_one_elf32_note, &data);
+    if (data.ret != 0) {
+        return data.ret;
+    }
+
+    qemu_for_each_cpu(write_one_elf32_qemunote, &data);
+    if (data.ret != 0) {
+        return data.ret;
     }
 
     return 0;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 17/18] memory_mapping: Use hwaddr type for MemoryMapping virt_addr field
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (15 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 16/18] dump: Abstract write_elf{64, 32}_notes() " Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 17:17   ` Peter Maydell
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 18/18] memory_mapping: Build only once Andreas Färber
                   ` (2 subsequent siblings)
  19 siblings, 1 reply; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

The memory mapping API uses hwaddr, so use it in the struct, too.
This avoids a header dependency on target_ulong type.

Cc: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/sysemu/memory_mapping.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index 6dfb68d..1411cc7 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -20,7 +20,7 @@
 /* The physical and virtual address in the memory mapping are contiguous. */
 typedef struct MemoryMapping {
     hwaddr phys_addr;
-    target_ulong virt_addr;
+    hwaddr virt_addr;
     ram_addr_t length;
     QTAILQ_ENTRY(MemoryMapping) next;
 } MemoryMapping;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu v4 18/18] memory_mapping: Build only once
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (16 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 17/18] memory_mapping: Use hwaddr type for MemoryMapping virt_addr field Andreas Färber
@ 2013-06-09 16:10 ` Andreas Färber
  2013-06-09 17:29   ` Peter Maydell
  2013-06-09 16:19 ` [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
  2013-06-11 16:54 ` Andreas Färber
  19 siblings, 1 reply; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: qiaonuohan, Andreas Färber, lcapitulino

Move RAMBlock, RAMList and ram_list to exec/cpu-common.h since they do
not depend on CPUArchState. We can then use it together with qom/cpu.h
to replace cpu.h and exec/cpu-all.h includes, making it fully
target-independent.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 Makefile.objs                   |  1 +
 Makefile.target                 |  1 -
 include/exec/cpu-all.h          | 28 ----------------------------
 include/exec/cpu-common.h       | 33 +++++++++++++++++++++++++++++++++
 include/sysemu/memory_mapping.h |  1 +
 memory_mapping.c                |  4 ++--
 6 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 5b288ba..89f28a1 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -54,6 +54,7 @@ common-obj-y += migration.o migration-tcp.o
 common-obj-y += qemu-char.o #aio.o
 common-obj-y += block-migration.o
 common-obj-y += page_cache.o xbzrle.o
+common-obj-y += memory_mapping.o
 
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
 
diff --git a/Makefile.target b/Makefile.target
index b0be124..e73c68b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -109,7 +109,6 @@ obj-y += hw/
 obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
 obj-y += memory.o savevm.o cputlb.o
-obj-y += memory_mapping.o
 obj-y += dump.o
 LIBS+=$(libs_softmmu)
 
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index e9c3717..bd09cd1 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -22,7 +22,6 @@
 #include "qemu-common.h"
 #include "qemu/tls.h"
 #include "exec/cpu-common.h"
-#include "qemu/thread.h"
 
 /* some important defines:
  *
@@ -465,33 +464,6 @@ extern ram_addr_t ram_size;
 /* RAM is pre-allocated and passed into qemu_ram_alloc_from_ptr */
 #define RAM_PREALLOC_MASK   (1 << 0)
 
-typedef struct RAMBlock {
-    struct MemoryRegion *mr;
-    uint8_t *host;
-    ram_addr_t offset;
-    ram_addr_t length;
-    uint32_t flags;
-    char idstr[256];
-    /* Reads can take either the iothread or the ramlist lock.
-     * Writes must take both locks.
-     */
-    QTAILQ_ENTRY(RAMBlock) next;
-#if defined(__linux__) && !defined(TARGET_S390X)
-    int fd;
-#endif
-} RAMBlock;
-
-typedef struct RAMList {
-    QemuMutex mutex;
-    /* Protected by the iothread lock.  */
-    uint8_t *phys_dirty;
-    RAMBlock *mru_block;
-    /* Protected by the ramlist lock.  */
-    QTAILQ_HEAD(, RAMBlock) blocks;
-    uint32_t version;
-} RAMList;
-extern RAMList ram_list;
-
 extern const char *mem_path;
 extern int mem_prealloc;
 
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index e061e21..0e5c63f 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -11,6 +11,7 @@
 
 #include "qemu/bswap.h"
 #include "qemu/queue.h"
+#include "qemu/thread.h"
 
 /**
  * CPUListState:
@@ -43,6 +44,38 @@ typedef uintptr_t ram_addr_t;
 #  define RAM_ADDR_FMT "%" PRIxPTR
 #endif
 
+#if !defined(CONFIG_USER_ONLY)
+
+typedef struct RAMBlock {
+    struct MemoryRegion *mr;
+    uint8_t *host;
+    ram_addr_t offset;
+    ram_addr_t length;
+    uint32_t flags;
+    char idstr[256];
+    /* Reads can take either the iothread or the ramlist lock.
+     * Writes must take both locks.
+     */
+    QTAILQ_ENTRY(RAMBlock) next;
+#if defined(__linux__) && !defined(TARGET_S390X)
+    int fd;
+#endif
+} RAMBlock;
+
+typedef struct RAMList {
+    QemuMutex mutex;
+    /* Protected by the iothread lock.  */
+    uint8_t *phys_dirty;
+    RAMBlock *mru_block;
+    /* Protected by the ramlist lock.  */
+    QTAILQ_HEAD(, RAMBlock) blocks;
+    uint32_t version;
+} RAMList;
+
+extern RAMList ram_list;
+
+#endif
+
 /* memory API */
 
 typedef void CPUWriteMemoryFunc(void *opaque, hwaddr addr, uint32_t value);
diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index 1411cc7..161cfc3 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -16,6 +16,7 @@
 
 #include "qemu/queue.h"
 #include "qemu/typedefs.h"
+#include "qom/cpu.h"
 
 /* The physical and virtual address in the memory mapping are contiguous. */
 typedef struct MemoryMapping {
diff --git a/memory_mapping.c b/memory_mapping.c
index a19be54..446a927 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -11,8 +11,8 @@
  *
  */
 
-#include "cpu.h"
-#include "exec/cpu-all.h"
+#include "qemu-common.h"
+#include "exec/cpu-common.h"
 #include "sysemu/memory_mapping.h"
 
 static void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list,
-- 
1.8.1.4

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (17 preceding siblings ...)
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 18/18] memory_mapping: Build only once Andreas Färber
@ 2013-06-09 16:19 ` Andreas Färber
  2013-06-11 16:54 ` Andreas Färber
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 16:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Vincent Rabin, Paolo Bonzini, Jens Freimann, qiaonuohan, lcapitulino

Am 09.06.2013 18:10, schrieb Andreas Färber:
> Andreas Färber (13):
>   dump: Move stubs into libqemustub.a
[...]
> Eduardo Habkost (3):
>   pc: Create pc-*-1.6 machine-types
>   target-i386: Update model values on Conroe/Penryn/Nehalem CPU models
>   target-i386: Set level=4 on Conroe/Penryn/Nehalem
> 
> Igor Mammedov (2):
>   pc: Fix crash when attempting to hotplug CPU with negative ID
>   target-i386: cpu: Fix potential buffer overrun in
>     get_register_name_32()

Please ignore patches 1-6, I accidentally sent out based from master
rather than qom-cpu (v3 did so because I had unqueued some patches).

(If you do have comments to them, please reply to the original patches.)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 17/18] memory_mapping: Use hwaddr type for MemoryMapping virt_addr field
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 17/18] memory_mapping: Use hwaddr type for MemoryMapping virt_addr field Andreas Färber
@ 2013-06-09 17:17   ` Peter Maydell
  2013-06-09 17:25     ` Andreas Färber
  0 siblings, 1 reply; 37+ messages in thread
From: Peter Maydell @ 2013-06-09 17:17 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel, lcapitulino

On 9 June 2013 17:10, Andreas Färber <afaerber@suse.de> wrote:
> The memory mapping API uses hwaddr, so use it in the struct, too.
> This avoids a header dependency on target_ulong type.

> --- a/include/sysemu/memory_mapping.h
> +++ b/include/sysemu/memory_mapping.h
> @@ -20,7 +20,7 @@
>  /* The physical and virtual address in the memory mapping are contiguous. */
>  typedef struct MemoryMapping {
>      hwaddr phys_addr;
> -    target_ulong virt_addr;
> +    hwaddr virt_addr;
>      ram_addr_t length;

This seems kind of odd given that HACKING specifically
says that target_ulong is for virtual addresses and hwaddr
for physical addresses. And in fact all the places that call
memory_mapping_list_add_merge_sorted() actually pass a
target_ulong as the virt_addr parameter, so maybe what we
should be fixing is the function prototype?

Incidentally that use of ram_addr_t for length looks
kind of suspicious to me; it should probably be a hwaddr.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 17/18] memory_mapping: Use hwaddr type for MemoryMapping virt_addr field
  2013-06-09 17:17   ` Peter Maydell
@ 2013-06-09 17:25     ` Andreas Färber
  0 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 17:25 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qiaonuohan, qemu-devel, lcapitulino

Am 09.06.2013 19:17, schrieb Peter Maydell:
> On 9 June 2013 17:10, Andreas Färber <afaerber@suse.de> wrote:
>> The memory mapping API uses hwaddr, so use it in the struct, too.
>> This avoids a header dependency on target_ulong type.
> 
>> --- a/include/sysemu/memory_mapping.h
>> +++ b/include/sysemu/memory_mapping.h
>> @@ -20,7 +20,7 @@
>>  /* The physical and virtual address in the memory mapping are contiguous. */
>>  typedef struct MemoryMapping {
>>      hwaddr phys_addr;
>> -    target_ulong virt_addr;
>> +    hwaddr virt_addr;
>>      ram_addr_t length;
> 
> This seems kind of odd given that HACKING specifically
> says that target_ulong is for virtual addresses and hwaddr
> for physical addresses. And in fact all the places that call
> memory_mapping_list_add_merge_sorted() actually pass a
> target_ulong as the virt_addr parameter, so maybe what we
> should be fixing is the function prototype?

Maybe. The benefit with going with the overwhelming use of hwaddr is
that memory_mapping.o can be compiled once for all targets, saving
compilation time.

The benefit of target_ulong would be saving a few bytes for 32-bit
targets at runtime.

> Incidentally that use of ram_addr_t for length looks
> kind of suspicious to me; it should probably be a hwaddr.

Looking at it strictly from a CPU and build perspective I'll happily
leave further code cleanups to someone else. :) Somewhere there's also a
misspelling of "memory" left to fix.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 18/18] memory_mapping: Build only once
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 18/18] memory_mapping: Build only once Andreas Färber
@ 2013-06-09 17:29   ` Peter Maydell
  2013-06-09 17:36     ` Andreas Färber
  0 siblings, 1 reply; 37+ messages in thread
From: Peter Maydell @ 2013-06-09 17:29 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel, lcapitulino

On 9 June 2013 17:10, Andreas Färber <afaerber@suse.de> wrote:
> Move RAMBlock, RAMList and ram_list to exec/cpu-common.h since they do
> not depend on CPUArchState. We can then use it together with qom/cpu.h
> to replace cpu.h and exec/cpu-all.h includes, making it fully
> target-independent.

Messing directly with RAMBlocks is (or should be) limited to
pretty small amounts of the codebase, so maybe it's worth putting
these structs in their own header in include/exec which only gets
pulled in by the code that really needs it?

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 18/18] memory_mapping: Build only once
  2013-06-09 17:29   ` Peter Maydell
@ 2013-06-09 17:36     ` Andreas Färber
  0 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-09 17:36 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qiaonuohan, qemu-devel, lcapitulino

Am 09.06.2013 19:29, schrieb Peter Maydell:
> On 9 June 2013 17:10, Andreas Färber <afaerber@suse.de> wrote:
>> Move RAMBlock, RAMList and ram_list to exec/cpu-common.h since they do
>> not depend on CPUArchState. We can then use it together with qom/cpu.h
>> to replace cpu.h and exec/cpu-all.h includes, making it fully
>> target-independent.
> 
> Messing directly with RAMBlocks is (or should be) limited to
> pretty small amounts of the codebase, so maybe it's worth putting
> these structs in their own header in include/exec which only gets
> pulled in by the code that really needs it?

I thought about that, but this way seemed easier to get v4 out finally
and the qom-cpu-10 series built on it.

The patches I really care about in this series are 7-11 because they
determine how arm and s390x implementations are to be added. I'll
happily respin or drop part of the remainder of the series after I get
my qom-cpu queue with the dump refactoring flushed out. ;)

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 07/18] cpu: Turn cpu_paging_enabled() into a CPUState hook
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 07/18] cpu: Turn cpu_paging_enabled() into a CPUState hook Andreas Färber
@ 2013-06-11  8:06   ` Jens Freimann
  2013-06-11 14:52   ` Luiz Capitulino
  1 sibling, 0 replies; 37+ messages in thread
From: Jens Freimann @ 2013-06-11  8:06 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel, lcapitulino

On Sun, Jun 09, 2013 at 06:10:36PM +0200, Andreas Färber wrote:
> Relocate assignment of x86 get_arch_id to have all hooks in one place.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

> ---
>  include/qom/cpu.h                 | 10 ++++++++++
>  include/sysemu/memory_mapping.h   |  1 -
>  memory_mapping-stub.c             |  6 ------
>  memory_mapping.c                  |  2 +-
>  qom/cpu.c                         | 13 +++++++++++++
>  target-i386/arch_memory_mapping.c |  6 +-----
>  target-i386/cpu.c                 | 11 +++++++++--
>  7 files changed, 34 insertions(+), 15 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 7cd9442..1f70240 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -48,6 +48,7 @@ typedef struct CPUState CPUState;
>   * @reset: Callback to reset the #CPUState to its initial state.
>   * @do_interrupt: Callback for interrupt handling.
>   * @get_arch_id: Callback for getting architecture-dependent CPU ID.
> + * @get_paging_enabled: Callback for inquiring whether paging is enabled.
>   * @vmsd: State description for migration.
>   *
>   * Represents a CPU family or model.
> @@ -62,6 +63,7 @@ typedef struct CPUClass {
>      void (*reset)(CPUState *cpu);
>      void (*do_interrupt)(CPUState *cpu);
>      int64_t (*get_arch_id)(CPUState *cpu);
> +    bool (*get_paging_enabled)(const CPUState *cpu);
>  
>      const struct VMStateDescription *vmsd;
>      int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
> @@ -138,6 +140,14 @@ struct CPUState {
>  };
>  
>  /**
> + * cpu_paging_enabled:
> + * @cpu: The CPU whose state is to be inspected.
> + *
> + * Returns: %true if paging is enabled, %false otherwise.
> + */
> +bool cpu_paging_enabled(const CPUState *cpu);
> +
> +/**
>   * cpu_write_elf64_note:
>   * @f: pointer to a function that writes memory to a file
>   * @cpu: The CPU whose memory is to be dumped
> diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
> index 1256125..6f01524 100644
> --- a/include/sysemu/memory_mapping.h
> +++ b/include/sysemu/memory_mapping.h
> @@ -31,7 +31,6 @@ typedef struct MemoryMappingList {
>  } MemoryMappingList;
>  
>  int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
> -bool cpu_paging_enabled(CPUArchState *env);
>  
>  /*
>   * add or merge the memory region [phys_addr, phys_addr + length) into the
> diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
> index 24d5d67..6c0dfeb 100644
> --- a/memory_mapping-stub.c
> +++ b/memory_mapping-stub.c
> @@ -25,9 +25,3 @@ int cpu_get_memory_mapping(MemoryMappingList *list,
>  {
>      return -1;
>  }
> -
> -bool cpu_paging_enabled(CPUArchState *env)
> -{
> -    return true;
> -}
> -
> diff --git a/memory_mapping.c b/memory_mapping.c
> index ff45b3a..0790aac 100644
> --- a/memory_mapping.c
> +++ b/memory_mapping.c
> @@ -170,7 +170,7 @@ static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu)
>      CPUArchState *env;
>  
>      for (env = start_cpu; env != NULL; env = env->next_cpu) {
> -        if (cpu_paging_enabled(env)) {
> +        if (cpu_paging_enabled(ENV_GET_CPU(env))) {
>              return env;
>          }
>      }
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 04aefbb..9f6da0f 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -50,6 +50,18 @@ bool cpu_exists(int64_t id)
>      return data.found;
>  }
>  
> +bool cpu_paging_enabled(const CPUState *cpu)
> +{
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +    return cc->get_paging_enabled(cpu);
> +}
> +
> +static bool cpu_common_get_paging_enabled(const CPUState *cpu)
> +{
> +    return true;
> +}
> +
>  /* CPU hot-plug notifiers */
>  static NotifierList cpu_added_notifiers =
>      NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
> @@ -176,6 +188,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>      k->class_by_name = cpu_common_class_by_name;
>      k->reset = cpu_common_reset;
>      k->get_arch_id = cpu_common_get_arch_id;
> +    k->get_paging_enabled = cpu_common_get_paging_enabled;
>      k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
>      k->write_elf32_note = cpu_common_write_elf32_note;
>      k->write_elf64_qemunote = cpu_common_write_elf64_qemunote;
> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c
> index 5096fbd..c5a10ec 100644
> --- a/target-i386/arch_memory_mapping.c
> +++ b/target-i386/arch_memory_mapping.c
> @@ -241,7 +241,7 @@ static void walk_pml4e(MemoryMappingList *list,
>  
>  int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
>  {
> -    if (!cpu_paging_enabled(env)) {
> +    if (!cpu_paging_enabled(ENV_GET_CPU(env))) {
>          /* paging is disabled */
>          return 0;
>      }
> @@ -273,7 +273,3 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
>      return 0;
>  }
>  
> -bool cpu_paging_enabled(CPUArchState *env)
> -{
> -    return env->cr[0] & CR0_PG_MASK;
> -}
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4b2da0d..f6fa7fa 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2505,6 +2505,13 @@ static int64_t x86_cpu_get_arch_id(CPUState *cs)
>      return env->cpuid_apic_id;
>  }
>  
> +static bool x86_cpu_get_paging_enabled(const CPUState *cs)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +
> +    return cpu->env.cr[0] & CR0_PG_MASK;
> +}
> +
>  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>  {
>      X86CPUClass *xcc = X86_CPU_CLASS(oc);
> @@ -2519,6 +2526,8 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      cc->reset = x86_cpu_reset;
>  
>      cc->do_interrupt = x86_cpu_do_interrupt;
> +    cc->get_arch_id = x86_cpu_get_arch_id;
> +    cc->get_paging_enabled = x86_cpu_get_paging_enabled;
>  #ifndef CONFIG_USER_ONLY
>      cc->write_elf64_note = x86_cpu_write_elf64_note;
>      cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
> @@ -2526,8 +2535,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      cc->write_elf32_qemunote = x86_cpu_write_elf32_qemunote;
>  #endif
>      cpu_class_set_vmsd(cc, &vmstate_x86_cpu);
> -
> -    cc->get_arch_id = x86_cpu_get_arch_id;
>  }
>  
>  static const TypeInfo x86_cpu_type_info = {
> -- 
> 1.8.1.4
> 
> 

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 12/18] cpu: Change default for CPUClass::get_paging_enabled()
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 12/18] cpu: Change default for CPUClass::get_paging_enabled() Andreas Färber
@ 2013-06-11  9:00   ` Jens Freimann
  2013-06-11 15:01   ` Luiz Capitulino
  1 sibling, 0 replies; 37+ messages in thread
From: Jens Freimann @ 2013-06-11  9:00 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel, lcapitulino

On Sun, Jun 09, 2013 at 06:10:41PM +0200, Andreas Färber wrote:
> qemu_get_guest_memory_mapping() uses cpu_paging_enabled() to determine
> whether to use cpu_get_memory_mapping() to return mappings or whether to
> fall back to a simple identity map.
> 
> Since by default CPUClass::get_memory_mapping() is not implemented,
> change the default to false to use the identity map by default.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Jens Freimann <jfrei@linux.vnet.ibm.com> 

> ---
>  qom/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index b25fbc9..dba4a11 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -59,7 +59,7 @@ bool cpu_paging_enabled(const CPUState *cpu)
>  
>  static bool cpu_common_get_paging_enabled(const CPUState *cpu)
>  {
> -    return true;
> +    return false;
>  }
>  
>  void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
> -- 
> 1.8.1.4
> 
> 

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook Andreas Färber
@ 2013-06-11  9:20   ` Jens Freimann
  2013-06-11 14:56   ` Luiz Capitulino
  1 sibling, 0 replies; 37+ messages in thread
From: Jens Freimann @ 2013-06-11  9:20 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel, lcapitulino

On Sun, Jun 09, 2013 at 06:10:38PM +0200, Andreas Färber wrote:
> Change error reporting from return value to Error argument.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewd-by: Jens Freimann <jfrei@linux.vnet.ibm.com>

> ---
>  include/qom/cpu.h                 | 14 ++++++++++++++
>  include/sysemu/memory_mapping.h   |  2 --
>  memory_mapping-stub.c             |  6 ------
>  memory_mapping.c                  |  7 ++++---
>  qom/cpu.c                         | 16 ++++++++++++++++
>  target-i386/arch_memory_mapping.c | 12 +++++++-----
>  target-i386/cpu-qom.h             |  3 +++
>  target-i386/cpu.c                 |  1 +
>  8 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1f70240..254be2e 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -23,6 +23,7 @@
>  #include <signal.h>
>  #include "hw/qdev-core.h"
>  #include "qemu/thread.h"
> +#include "qemu/typedefs.h"
>  
>  typedef int (*WriteCoreDumpFunction)(void *buf, size_t size, void *opaque);
>  
> @@ -49,6 +50,7 @@ typedef struct CPUState CPUState;
>   * @do_interrupt: Callback for interrupt handling.
>   * @get_arch_id: Callback for getting architecture-dependent CPU ID.
>   * @get_paging_enabled: Callback for inquiring whether paging is enabled.
> + * @get_memory_mapping: Callback for obtaining the memory mappings.
>   * @vmsd: State description for migration.
>   *
>   * Represents a CPU family or model.
> @@ -64,6 +66,8 @@ typedef struct CPUClass {
>      void (*do_interrupt)(CPUState *cpu);
>      int64_t (*get_arch_id)(CPUState *cpu);
>      bool (*get_paging_enabled)(const CPUState *cpu);
> +    void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
> +                               Error **errp);
>  
>      const struct VMStateDescription *vmsd;
>      int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
> @@ -148,6 +152,16 @@ struct CPUState {
>  bool cpu_paging_enabled(const CPUState *cpu);
>  
>  /**
> + * @cpu: The CPU whose memory mappings are to be obtained.
> + * @list: Where to write the memory mappings to.
> + * @errp: Pointer for reporting an #Error.
> + *
> + * Returns: 0 if successful.
> + */
> +void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
> +                            Error **errp);
> +
> +/**
>   * cpu_write_elf64_note:
>   * @f: pointer to a function that writes memory to a file
>   * @cpu: The CPU whose memory is to be dumped
> diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
> index 1f71c32..c47e6ee 100644
> --- a/include/sysemu/memory_mapping.h
> +++ b/include/sysemu/memory_mapping.h
> @@ -31,8 +31,6 @@ struct MemoryMappingList {
>      QTAILQ_HEAD(, MemoryMapping) head;
>  };
>  
> -int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
> -
>  /*
>   * add or merge the memory region [phys_addr, phys_addr + length) into the
>   * memory mapping's list. The region's virtual address starts with virt_addr,
> diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
> index 6c0dfeb..989dc00 100644
> --- a/memory_mapping-stub.c
> +++ b/memory_mapping-stub.c
> @@ -19,9 +19,3 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
>  {
>      return -2;
>  }
> -
> -int cpu_get_memory_mapping(MemoryMappingList *list,
> -					                                          CPUArchState *env)
> -{
> -    return -1;
> -}
> diff --git a/memory_mapping.c b/memory_mapping.c
> index 0790aac..9bd24ce 100644
> --- a/memory_mapping.c
> +++ b/memory_mapping.c
> @@ -183,13 +183,14 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
>      CPUArchState *env, *first_paging_enabled_cpu;
>      RAMBlock *block;
>      ram_addr_t offset, length;
> -    int ret;
>  
>      first_paging_enabled_cpu = find_paging_enabled_cpu(first_cpu);
>      if (first_paging_enabled_cpu) {
>          for (env = first_paging_enabled_cpu; env != NULL; env = env->next_cpu) {
> -            ret = cpu_get_memory_mapping(list, env);
> -            if (ret < 0) {
> +            Error *err = NULL;
> +            cpu_get_memory_mapping(ENV_GET_CPU(env), list, &err);
> +            if (err) {
> +                error_free(err);
>                  return -1;
>              }
>          }
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 9f6da0f..b25fbc9 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -62,6 +62,21 @@ static bool cpu_common_get_paging_enabled(const CPUState *cpu)
>      return true;
>  }
>  
> +void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
> +                            Error **errp)
> +{
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +    return cc->get_memory_mapping(cpu, list, errp);
> +}
> +
> +static void cpu_common_get_memory_mapping(CPUState *cpu,
> +                                          MemoryMappingList *list,
> +                                          Error **errp)
> +{
> +    error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
> +}
> +
>  /* CPU hot-plug notifiers */
>  static NotifierList cpu_added_notifiers =
>      NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
> @@ -189,6 +204,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>      k->reset = cpu_common_reset;
>      k->get_arch_id = cpu_common_get_arch_id;
>      k->get_paging_enabled = cpu_common_get_paging_enabled;
> +    k->get_memory_mapping = cpu_common_get_memory_mapping;
>      k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
>      k->write_elf32_note = cpu_common_write_elf32_note;
>      k->write_elf64_qemunote = cpu_common_write_elf64_qemunote;
> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c
> index c5a10ec..2566a04 100644
> --- a/target-i386/arch_memory_mapping.c
> +++ b/target-i386/arch_memory_mapping.c
> @@ -239,11 +239,15 @@ static void walk_pml4e(MemoryMappingList *list,
>  }
>  #endif
>  
> -int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
> +void x86_cpu_get_memory_mapping(CPUState *cs, MemoryMappingList *list,
> +                                Error **errp)
>  {
> -    if (!cpu_paging_enabled(ENV_GET_CPU(env))) {
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +
> +    if (!cpu_paging_enabled(cs)) {
>          /* paging is disabled */
> -        return 0;
> +        return;
>      }
>  
>      if (env->cr[4] & CR4_PAE_MASK) {
> @@ -269,7 +273,5 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
>          pse = !!(env->cr[4] & CR4_PSE_MASK);
>          walk_pde2(list, pde_addr, env->a20_mask, pse);
>      }
> -
> -    return 0;
>  }
>  
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 849cedf..e0ac072 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -98,4 +98,7 @@ int x86_cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
>  int x86_cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
>                                   void *opaque);
>  
> +void x86_cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
> +                                Error **errp);
> +
>  #endif
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index f6fa7fa..a7154af 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2529,6 +2529,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      cc->get_arch_id = x86_cpu_get_arch_id;
>      cc->get_paging_enabled = x86_cpu_get_paging_enabled;
>  #ifndef CONFIG_USER_ONLY
> +    cc->get_memory_mapping = x86_cpu_get_memory_mapping;
>      cc->write_elf64_note = x86_cpu_write_elf64_note;
>      cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
>      cc->write_elf32_note = x86_cpu_write_elf32_note;
> -- 
> 1.8.1.4
> 
> 

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 07/18] cpu: Turn cpu_paging_enabled() into a CPUState hook
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 07/18] cpu: Turn cpu_paging_enabled() into a CPUState hook Andreas Färber
  2013-06-11  8:06   ` Jens Freimann
@ 2013-06-11 14:52   ` Luiz Capitulino
  1 sibling, 0 replies; 37+ messages in thread
From: Luiz Capitulino @ 2013-06-11 14:52 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel

On Sun,  9 Jun 2013 18:10:36 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Relocate assignment of x86 get_arch_id to have all hooks in one place.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  include/qom/cpu.h                 | 10 ++++++++++
>  include/sysemu/memory_mapping.h   |  1 -
>  memory_mapping-stub.c             |  6 ------
>  memory_mapping.c                  |  2 +-
>  qom/cpu.c                         | 13 +++++++++++++
>  target-i386/arch_memory_mapping.c |  6 +-----
>  target-i386/cpu.c                 | 11 +++++++++--
>  7 files changed, 34 insertions(+), 15 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 7cd9442..1f70240 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -48,6 +48,7 @@ typedef struct CPUState CPUState;
>   * @reset: Callback to reset the #CPUState to its initial state.
>   * @do_interrupt: Callback for interrupt handling.
>   * @get_arch_id: Callback for getting architecture-dependent CPU ID.
> + * @get_paging_enabled: Callback for inquiring whether paging is enabled.
>   * @vmsd: State description for migration.
>   *
>   * Represents a CPU family or model.
> @@ -62,6 +63,7 @@ typedef struct CPUClass {
>      void (*reset)(CPUState *cpu);
>      void (*do_interrupt)(CPUState *cpu);
>      int64_t (*get_arch_id)(CPUState *cpu);
> +    bool (*get_paging_enabled)(const CPUState *cpu);
>  
>      const struct VMStateDescription *vmsd;
>      int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
> @@ -138,6 +140,14 @@ struct CPUState {
>  };
>  
>  /**
> + * cpu_paging_enabled:
> + * @cpu: The CPU whose state is to be inspected.
> + *
> + * Returns: %true if paging is enabled, %false otherwise.
> + */
> +bool cpu_paging_enabled(const CPUState *cpu);
> +
> +/**
>   * cpu_write_elf64_note:
>   * @f: pointer to a function that writes memory to a file
>   * @cpu: The CPU whose memory is to be dumped
> diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
> index 1256125..6f01524 100644
> --- a/include/sysemu/memory_mapping.h
> +++ b/include/sysemu/memory_mapping.h
> @@ -31,7 +31,6 @@ typedef struct MemoryMappingList {
>  } MemoryMappingList;
>  
>  int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
> -bool cpu_paging_enabled(CPUArchState *env);
>  
>  /*
>   * add or merge the memory region [phys_addr, phys_addr + length) into the
> diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
> index 24d5d67..6c0dfeb 100644
> --- a/memory_mapping-stub.c
> +++ b/memory_mapping-stub.c
> @@ -25,9 +25,3 @@ int cpu_get_memory_mapping(MemoryMappingList *list,
>  {
>      return -1;
>  }
> -
> -bool cpu_paging_enabled(CPUArchState *env)
> -{
> -    return true;
> -}
> -
> diff --git a/memory_mapping.c b/memory_mapping.c
> index ff45b3a..0790aac 100644
> --- a/memory_mapping.c
> +++ b/memory_mapping.c
> @@ -170,7 +170,7 @@ static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu)
>      CPUArchState *env;
>  
>      for (env = start_cpu; env != NULL; env = env->next_cpu) {
> -        if (cpu_paging_enabled(env)) {
> +        if (cpu_paging_enabled(ENV_GET_CPU(env))) {
>              return env;
>          }
>      }
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 04aefbb..9f6da0f 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -50,6 +50,18 @@ bool cpu_exists(int64_t id)
>      return data.found;
>  }
>  
> +bool cpu_paging_enabled(const CPUState *cpu)
> +{
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +    return cc->get_paging_enabled(cpu);
> +}
> +
> +static bool cpu_common_get_paging_enabled(const CPUState *cpu)
> +{
> +    return true;
> +}
> +
>  /* CPU hot-plug notifiers */
>  static NotifierList cpu_added_notifiers =
>      NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
> @@ -176,6 +188,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>      k->class_by_name = cpu_common_class_by_name;
>      k->reset = cpu_common_reset;
>      k->get_arch_id = cpu_common_get_arch_id;
> +    k->get_paging_enabled = cpu_common_get_paging_enabled;
>      k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
>      k->write_elf32_note = cpu_common_write_elf32_note;
>      k->write_elf64_qemunote = cpu_common_write_elf64_qemunote;
> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c
> index 5096fbd..c5a10ec 100644
> --- a/target-i386/arch_memory_mapping.c
> +++ b/target-i386/arch_memory_mapping.c
> @@ -241,7 +241,7 @@ static void walk_pml4e(MemoryMappingList *list,
>  
>  int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
>  {
> -    if (!cpu_paging_enabled(env)) {
> +    if (!cpu_paging_enabled(ENV_GET_CPU(env))) {
>          /* paging is disabled */
>          return 0;
>      }
> @@ -273,7 +273,3 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
>      return 0;
>  }
>  
> -bool cpu_paging_enabled(CPUArchState *env)
> -{
> -    return env->cr[0] & CR0_PG_MASK;
> -}
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4b2da0d..f6fa7fa 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2505,6 +2505,13 @@ static int64_t x86_cpu_get_arch_id(CPUState *cs)
>      return env->cpuid_apic_id;
>  }
>  
> +static bool x86_cpu_get_paging_enabled(const CPUState *cs)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +
> +    return cpu->env.cr[0] & CR0_PG_MASK;
> +}
> +
>  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>  {
>      X86CPUClass *xcc = X86_CPU_CLASS(oc);
> @@ -2519,6 +2526,8 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      cc->reset = x86_cpu_reset;
>  
>      cc->do_interrupt = x86_cpu_do_interrupt;
> +    cc->get_arch_id = x86_cpu_get_arch_id;
> +    cc->get_paging_enabled = x86_cpu_get_paging_enabled;
>  #ifndef CONFIG_USER_ONLY
>      cc->write_elf64_note = x86_cpu_write_elf64_note;
>      cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
> @@ -2526,8 +2535,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      cc->write_elf32_qemunote = x86_cpu_write_elf32_qemunote;
>  #endif
>      cpu_class_set_vmsd(cc, &vmstate_x86_cpu);
> -
> -    cc->get_arch_id = x86_cpu_get_arch_id;
>  }
>  
>  static const TypeInfo x86_cpu_type_info = {

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook Andreas Färber
  2013-06-11  9:20   ` Jens Freimann
@ 2013-06-11 14:56   ` Luiz Capitulino
  2013-06-11 16:03     ` Andreas Färber
  1 sibling, 1 reply; 37+ messages in thread
From: Luiz Capitulino @ 2013-06-11 14:56 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel

On Sun,  9 Jun 2013 18:10:38 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Change error reporting from return value to Error argument.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  include/qom/cpu.h                 | 14 ++++++++++++++
>  include/sysemu/memory_mapping.h   |  2 --
>  memory_mapping-stub.c             |  6 ------
>  memory_mapping.c                  |  7 ++++---
>  qom/cpu.c                         | 16 ++++++++++++++++
>  target-i386/arch_memory_mapping.c | 12 +++++++-----
>  target-i386/cpu-qom.h             |  3 +++
>  target-i386/cpu.c                 |  1 +
>  8 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1f70240..254be2e 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -23,6 +23,7 @@
>  #include <signal.h>
>  #include "hw/qdev-core.h"
>  #include "qemu/thread.h"
> +#include "qemu/typedefs.h"
>  
>  typedef int (*WriteCoreDumpFunction)(void *buf, size_t size, void *opaque);
>  
> @@ -49,6 +50,7 @@ typedef struct CPUState CPUState;
>   * @do_interrupt: Callback for interrupt handling.
>   * @get_arch_id: Callback for getting architecture-dependent CPU ID.
>   * @get_paging_enabled: Callback for inquiring whether paging is enabled.
> + * @get_memory_mapping: Callback for obtaining the memory mappings.
>   * @vmsd: State description for migration.
>   *
>   * Represents a CPU family or model.
> @@ -64,6 +66,8 @@ typedef struct CPUClass {
>      void (*do_interrupt)(CPUState *cpu);
>      int64_t (*get_arch_id)(CPUState *cpu);
>      bool (*get_paging_enabled)(const CPUState *cpu);
> +    void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
> +                               Error **errp);
>  
>      const struct VMStateDescription *vmsd;
>      int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu,
> @@ -148,6 +152,16 @@ struct CPUState {
>  bool cpu_paging_enabled(const CPUState *cpu);
>  
>  /**
> + * @cpu: The CPU whose memory mappings are to be obtained.
> + * @list: Where to write the memory mappings to.
> + * @errp: Pointer for reporting an #Error.
> + *
> + * Returns: 0 if successful.
> + */

It turns void now, but you can add:

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

When you respin.

> +void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
> +                            Error **errp);
> +
> +/**
>   * cpu_write_elf64_note:
>   * @f: pointer to a function that writes memory to a file
>   * @cpu: The CPU whose memory is to be dumped
> diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
> index 1f71c32..c47e6ee 100644
> --- a/include/sysemu/memory_mapping.h
> +++ b/include/sysemu/memory_mapping.h
> @@ -31,8 +31,6 @@ struct MemoryMappingList {
>      QTAILQ_HEAD(, MemoryMapping) head;
>  };
>  
> -int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
> -
>  /*
>   * add or merge the memory region [phys_addr, phys_addr + length) into the
>   * memory mapping's list. The region's virtual address starts with virt_addr,
> diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
> index 6c0dfeb..989dc00 100644
> --- a/memory_mapping-stub.c
> +++ b/memory_mapping-stub.c
> @@ -19,9 +19,3 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
>  {
>      return -2;
>  }
> -
> -int cpu_get_memory_mapping(MemoryMappingList *list,
> -					                                          CPUArchState *env)
> -{
> -    return -1;
> -}
> diff --git a/memory_mapping.c b/memory_mapping.c
> index 0790aac..9bd24ce 100644
> --- a/memory_mapping.c
> +++ b/memory_mapping.c
> @@ -183,13 +183,14 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
>      CPUArchState *env, *first_paging_enabled_cpu;
>      RAMBlock *block;
>      ram_addr_t offset, length;
> -    int ret;
>  
>      first_paging_enabled_cpu = find_paging_enabled_cpu(first_cpu);
>      if (first_paging_enabled_cpu) {
>          for (env = first_paging_enabled_cpu; env != NULL; env = env->next_cpu) {
> -            ret = cpu_get_memory_mapping(list, env);
> -            if (ret < 0) {
> +            Error *err = NULL;
> +            cpu_get_memory_mapping(ENV_GET_CPU(env), list, &err);
> +            if (err) {
> +                error_free(err);
>                  return -1;
>              }
>          }
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 9f6da0f..b25fbc9 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -62,6 +62,21 @@ static bool cpu_common_get_paging_enabled(const CPUState *cpu)
>      return true;
>  }
>  
> +void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
> +                            Error **errp)
> +{
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +    return cc->get_memory_mapping(cpu, list, errp);
> +}
> +
> +static void cpu_common_get_memory_mapping(CPUState *cpu,
> +                                          MemoryMappingList *list,
> +                                          Error **errp)
> +{
> +    error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
> +}
> +
>  /* CPU hot-plug notifiers */
>  static NotifierList cpu_added_notifiers =
>      NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
> @@ -189,6 +204,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>      k->reset = cpu_common_reset;
>      k->get_arch_id = cpu_common_get_arch_id;
>      k->get_paging_enabled = cpu_common_get_paging_enabled;
> +    k->get_memory_mapping = cpu_common_get_memory_mapping;
>      k->write_elf32_qemunote = cpu_common_write_elf32_qemunote;
>      k->write_elf32_note = cpu_common_write_elf32_note;
>      k->write_elf64_qemunote = cpu_common_write_elf64_qemunote;
> diff --git a/target-i386/arch_memory_mapping.c b/target-i386/arch_memory_mapping.c
> index c5a10ec..2566a04 100644
> --- a/target-i386/arch_memory_mapping.c
> +++ b/target-i386/arch_memory_mapping.c
> @@ -239,11 +239,15 @@ static void walk_pml4e(MemoryMappingList *list,
>  }
>  #endif
>  
> -int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
> +void x86_cpu_get_memory_mapping(CPUState *cs, MemoryMappingList *list,
> +                                Error **errp)
>  {
> -    if (!cpu_paging_enabled(ENV_GET_CPU(env))) {
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +
> +    if (!cpu_paging_enabled(cs)) {
>          /* paging is disabled */
> -        return 0;
> +        return;
>      }
>  
>      if (env->cr[4] & CR4_PAE_MASK) {
> @@ -269,7 +273,5 @@ int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
>          pse = !!(env->cr[4] & CR4_PSE_MASK);
>          walk_pde2(list, pde_addr, env->a20_mask, pse);
>      }
> -
> -    return 0;
>  }
>  
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 849cedf..e0ac072 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -98,4 +98,7 @@ int x86_cpu_write_elf64_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
>  int x86_cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
>                                   void *opaque);
>  
> +void x86_cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
> +                                Error **errp);
> +
>  #endif
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index f6fa7fa..a7154af 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2529,6 +2529,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>      cc->get_arch_id = x86_cpu_get_arch_id;
>      cc->get_paging_enabled = x86_cpu_get_paging_enabled;
>  #ifndef CONFIG_USER_ONLY
> +    cc->get_memory_mapping = x86_cpu_get_memory_mapping;
>      cc->write_elf64_note = x86_cpu_write_elf64_note;
>      cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
>      cc->write_elf32_note = x86_cpu_write_elf32_note;

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 12/18] cpu: Change default for CPUClass::get_paging_enabled()
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 12/18] cpu: Change default for CPUClass::get_paging_enabled() Andreas Färber
  2013-06-11  9:00   ` Jens Freimann
@ 2013-06-11 15:01   ` Luiz Capitulino
  1 sibling, 0 replies; 37+ messages in thread
From: Luiz Capitulino @ 2013-06-11 15:01 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel

On Sun,  9 Jun 2013 18:10:41 +0200
Andreas Färber <afaerber@suse.de> wrote:

> qemu_get_guest_memory_mapping() uses cpu_paging_enabled() to determine
> whether to use cpu_get_memory_mapping() to return mappings or whether to
> fall back to a simple identity map.
> 
> Since by default CPUClass::get_memory_mapping() is not implemented,
> change the default to false to use the identity map by default.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  qom/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index b25fbc9..dba4a11 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -59,7 +59,7 @@ bool cpu_paging_enabled(const CPUState *cpu)
>  
>  static bool cpu_common_get_paging_enabled(const CPUState *cpu)
>  {
> -    return true;
> +    return false;
>  }
>  
>  void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 13/18] memory_mapping: Cleanup qemu_get_guest_memory_mapping()
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 13/18] memory_mapping: Cleanup qemu_get_guest_memory_mapping() Andreas Färber
@ 2013-06-11 15:52   ` Luiz Capitulino
  2013-06-11 17:47     ` Andreas Färber
  0 siblings, 1 reply; 37+ messages in thread
From: Luiz Capitulino @ 2013-06-11 15:52 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel

On Sun,  9 Jun 2013 18:10:42 +0200
Andreas Färber <afaerber@suse.de> wrote:

> We relied on the CPUClass::get_memory_mapping() implementation being a
> no-op when paging is disabled for that CPU. Therefore we can just as well
> iterate over all CPUs to retrieve mappings.
> 
> This allows to use qemu_for_each_cpu() instead of open-coding CPU loops.
> 
> Pass any Error out into dump_init() and have it actually stop on errors.
> Whether it is unsupported on a certain CPU can be checked by looking for
> a NULL CPUClass::get_memory_mapping field.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  dump.c                          |  7 +++++-
>  include/sysemu/memory_mapping.h |  8 +------
>  memory_mapping.c                | 52 ++++++++++++++++++++++++-----------------
>  3 files changed, 38 insertions(+), 29 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index c0d3da5..b44dafc 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -706,6 +706,7 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>  {
>      CPUArchState *env;
>      int nr_cpus;
> +    Error *err = NULL;
>      int ret;
>  
>      if (runstate_is_running()) {
> @@ -756,7 +757,11 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>      /* get memory mapping */
>      memory_mapping_list_init(&s->list);
>      if (paging) {
> -        qemu_get_guest_memory_mapping(&s->list);
> +        qemu_get_guest_memory_mapping(&s->list, &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            goto cleanup;
> +        }
>      } else {
>          qemu_get_guest_simple_memory_mapping(&s->list);
>      }
> diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
> index c47e6ee..6dfb68d 100644
> --- a/include/sysemu/memory_mapping.h
> +++ b/include/sysemu/memory_mapping.h
> @@ -45,13 +45,7 @@ void memory_mapping_list_free(MemoryMappingList *list);
>  
>  void memory_mapping_list_init(MemoryMappingList *list);
>  
> -/*
> - * Return value:
> - *    0: success
> - *   -1: failed
> - *   -2: unsupported
> - */
> -int qemu_get_guest_memory_mapping(MemoryMappingList *list);
> +void qemu_get_guest_memory_mapping(MemoryMappingList *list, Error **errp);
>  
>  /* get guest's memory mapping without do paging(virtual address is 0). */
>  void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list);
> diff --git a/memory_mapping.c b/memory_mapping.c
> index 9bd24ce..a19be54 100644
> --- a/memory_mapping.c
> +++ b/memory_mapping.c
> @@ -165,36 +165,48 @@ void memory_mapping_list_init(MemoryMappingList *list)
>      QTAILQ_INIT(&list->head);
>  }
>  
> -static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu)
> +static void find_paging_enabled_cpu(CPUState *cpu, void *data)
>  {
> -    CPUArchState *env;
> +    bool *ret = data;
>  
> -    for (env = start_cpu; env != NULL; env = env->next_cpu) {
> -        if (cpu_paging_enabled(ENV_GET_CPU(env))) {
> -            return env;
> -        }
> +    if (*ret) {
> +        return;
>      }
> +    *ret = cpu_paging_enabled(cpu);
> +}
> +
> +typedef struct GetGuestMemoryMappingData {
> +    MemoryMappingList *list;
> +    Error *err;
> +} GetGuestMemoryMappingData;
> +
> +static void qemu_get_one_guest_memory_mapping(CPUState *cpu, void *data)
> +{
> +    GetGuestMemoryMappingData *s = data;
>  
> -    return NULL;
> +    if (s->err != NULL || !cpu_paging_enabled(cpu)) {
> +        return;
> +    }
> +    cpu_get_memory_mapping(cpu, s->list, &s->err);
>  }
>  
> -int qemu_get_guest_memory_mapping(MemoryMappingList *list)
> +void qemu_get_guest_memory_mapping(MemoryMappingList *list, Error **errp)
>  {
> -    CPUArchState *env, *first_paging_enabled_cpu;
> +    GetGuestMemoryMappingData s = {
> +        .list = list,
> +        .err = NULL,
> +    };
> +    bool paging_enabled = false;
>      RAMBlock *block;
>      ram_addr_t offset, length;
>  
> -    first_paging_enabled_cpu = find_paging_enabled_cpu(first_cpu);
> -    if (first_paging_enabled_cpu) {
> -        for (env = first_paging_enabled_cpu; env != NULL; env = env->next_cpu) {
> -            Error *err = NULL;
> -            cpu_get_memory_mapping(ENV_GET_CPU(env), list, &err);
> -            if (err) {
> -                error_free(err);
> -                return -1;
> -            }
> +    qemu_for_each_cpu(find_paging_enabled_cpu, &paging_enabled);
> +    if (paging_enabled) {
> +        qemu_for_each_cpu(qemu_get_one_guest_memory_mapping, &s);
> +        if (s.err != NULL) {
> +            error_propagate(errp, s.err);
>          }
> -        return 0;
> +        return;
>      }
>  
>      /*
> @@ -206,8 +218,6 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
>          length = block->length;
>          create_new_memory_mapping(list, offset, offset, length);
>      }
> -
> -    return 0;
>  }
>  
>  void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 14/18] dump: Abstract dump_init() with cpu_synchronize_all_states()
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 14/18] dump: Abstract dump_init() with cpu_synchronize_all_states() Andreas Färber
@ 2013-06-11 15:55   ` Luiz Capitulino
  2013-06-11 17:46     ` Andreas Färber
  0 siblings, 1 reply; 37+ messages in thread
From: Luiz Capitulino @ 2013-06-11 15:55 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel

On Sun,  9 Jun 2013 18:10:43 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Instead of calling cpu_synchronize_state() for each CPU, call the
> existing cpu_synchronize_all_states() helper.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  dump.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index b44dafc..44a1339 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -21,6 +21,7 @@
>  #include "sysemu/dump.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/memory_mapping.h"
> +#include "sysemu/cpus.h"
>  #include "qapi/error.h"
>  #include "qmp-commands.h"
>  
> @@ -732,12 +733,12 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>       * If the target architecture is not supported, cpu_get_dump_info() will
>       * return -1.
>       *
> -     * if we use kvm, we should synchronize the register before we get dump
> +     * If we use KVM, we should synchronize the registers before we get dump
>       * info.
>       */
> +    cpu_synchronize_all_states();
>      nr_cpus = 0;
>      for (env = first_cpu; env != NULL; env = env->next_cpu) {
> -        cpu_synchronize_state(env);
>          nr_cpus++;
>      }
>  

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 15/18] dump: Abstract dump_init() further with qemu_for_each_cpu()
  2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 15/18] dump: Abstract dump_init() further with qemu_for_each_cpu() Andreas Färber
@ 2013-06-11 15:55   ` Luiz Capitulino
  0 siblings, 0 replies; 37+ messages in thread
From: Luiz Capitulino @ 2013-06-11 15:55 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qiaonuohan, qemu-devel

On Sun,  9 Jun 2013 18:10:44 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Allows to drop CPUArchState variable.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  dump.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index 44a1339..4e6b855 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -702,10 +702,16 @@ static ram_addr_t get_start_block(DumpState *s)
>      return -1;
>  }
>  
> +static void count_one_cpu(CPUState *cpu, void *data)
> +{
> +    int *nr = data;
> +
> +    *nr = *nr + 1;
> +}
> +
>  static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>                       int64_t begin, int64_t length, Error **errp)
>  {
> -    CPUArchState *env;
>      int nr_cpus;
>      Error *err = NULL;
>      int ret;
> @@ -738,9 +744,7 @@ static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
>       */
>      cpu_synchronize_all_states();
>      nr_cpus = 0;
> -    for (env = first_cpu; env != NULL; env = env->next_cpu) {
> -        nr_cpus++;
> -    }
> +    qemu_for_each_cpu(count_one_cpu, &nr_cpus);

Isn't it worth it to have an API for this?

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

>  
>      ret = cpu_get_dump_info(&s->dump_info);
>      if (ret < 0) {

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook
  2013-06-11 14:56   ` Luiz Capitulino
@ 2013-06-11 16:03     ` Andreas Färber
  0 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-11 16:03 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: qiaonuohan, qemu-devel, Jens Freimann

Am 11.06.2013 16:56, schrieb Luiz Capitulino:
> On Sun,  9 Jun 2013 18:10:38 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Change error reporting from return value to Error argument.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  include/qom/cpu.h                 | 14 ++++++++++++++
>>  include/sysemu/memory_mapping.h   |  2 --
>>  memory_mapping-stub.c             |  6 ------
>>  memory_mapping.c                  |  7 ++++---
>>  qom/cpu.c                         | 16 ++++++++++++++++
>>  target-i386/arch_memory_mapping.c | 12 +++++++-----
>>  target-i386/cpu-qom.h             |  3 +++
>>  target-i386/cpu.c                 |  1 +
>>  8 files changed, 45 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index 1f70240..254be2e 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
[...]
>> @@ -148,6 +152,16 @@ struct CPUState {
>>  bool cpu_paging_enabled(const CPUState *cpu);
>>  
>>  /**
>> + * @cpu: The CPU whose memory mappings are to be obtained.
>> + * @list: Where to write the memory mappings to.
>> + * @errp: Pointer for reporting an #Error.
>> + *
>> + * Returns: 0 if successful.
>> + */
> 
> It turns void now, but you can add:
> 
> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
> 
> When you respin.

Thanks, fixing up as follows:

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 254be2e..a5bb515 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -152,11 +152,10 @@ struct CPUState {
 bool cpu_paging_enabled(const CPUState *cpu);

 /**
+ * cpu_get_memory_mapping:
  * @cpu: The CPU whose memory mappings are to be obtained.
  * @list: Where to write the memory mappings to.
  * @errp: Pointer for reporting an #Error.
- *
- * Returns: 0 if successful.
  */
 void cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
                             Error **errp);

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone
  2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
                   ` (18 preceding siblings ...)
  2013-06-09 16:19 ` [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
@ 2013-06-11 16:54 ` Andreas Färber
  19 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-11 16:54 UTC (permalink / raw)
  To: Jens Freimann, Luiz Capitulino
  Cc: Vincent Rabin, Paolo Bonzini, qiaonuohan, qemu-devel

Am 09.06.2013 18:10, schrieb Andreas Färber:
> Andreas Färber (13):
[...]
>   cpu: Turn cpu_paging_enabled() into a CPUState hook
>   memory_mapping: Move MemoryMappingList typedef to qemu/typedefs.h
>   cpu: Turn cpu_get_memory_mapping() into a CPUState hook
>   memory_mapping: Drop qemu_get_memory_mapping() stub
>   dump: Drop qmp_dump_guest_memory() stub and build for all targets
>   cpu: Change default for CPUClass::get_paging_enabled()
[snip]

Thanks, applied these to qom-cpu (with one suggested fixup):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 14/18] dump: Abstract dump_init() with cpu_synchronize_all_states()
  2013-06-11 15:55   ` Luiz Capitulino
@ 2013-06-11 17:46     ` Andreas Färber
  0 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-11 17:46 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: qiaonuohan, qemu-devel

Am 11.06.2013 17:55, schrieb Luiz Capitulino:
> On Sun,  9 Jun 2013 18:10:43 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Instead of calling cpu_synchronize_state() for each CPU, call the
>> existing cpu_synchronize_all_states() helper.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH qom-cpu v4 13/18] memory_mapping: Cleanup qemu_get_guest_memory_mapping()
  2013-06-11 15:52   ` Luiz Capitulino
@ 2013-06-11 17:47     ` Andreas Färber
  0 siblings, 0 replies; 37+ messages in thread
From: Andreas Färber @ 2013-06-11 17:47 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: qiaonuohan, qemu-devel

Am 11.06.2013 17:52, schrieb Luiz Capitulino:
> On Sun,  9 Jun 2013 18:10:42 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> We relied on the CPUClass::get_memory_mapping() implementation being a
>> no-op when paging is disabled for that CPU. Therefore we can just as well
>> iterate over all CPUs to retrieve mappings.
>>
>> This allows to use qemu_for_each_cpu() instead of open-coding CPU loops.
>>
>> Pass any Error out into dump_init() and have it actually stop on errors.
>> Whether it is unsupported on a certain CPU can be checked by looking for
>> a NULL CPUClass::get_memory_mapping field.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

Thanks, reverted CPU loop changes and applied to qom-cpu as pure error
handling improvement:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

end of thread, other threads:[~2013-06-11 17:47 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-09 16:10 [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 01/18] dump: Move stubs into libqemustub.a Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 02/18] pc: Fix crash when attempting to hotplug CPU with negative ID Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 03/18] pc: Create pc-*-1.6 machine-types Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 04/18] target-i386: Update model values on Conroe/Penryn/Nehalem CPU models Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 05/18] target-i386: Set level=4 on Conroe/Penryn/Nehalem Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 06/18] target-i386: cpu: Fix potential buffer overrun in get_register_name_32() Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 07/18] cpu: Turn cpu_paging_enabled() into a CPUState hook Andreas Färber
2013-06-11  8:06   ` Jens Freimann
2013-06-11 14:52   ` Luiz Capitulino
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 08/18] memory_mapping: Move MemoryMappingList typedef to qemu/typedefs.h Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 09/18] cpu: Turn cpu_get_memory_mapping() into a CPUState hook Andreas Färber
2013-06-11  9:20   ` Jens Freimann
2013-06-11 14:56   ` Luiz Capitulino
2013-06-11 16:03     ` Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 10/18] memory_mapping: Drop qemu_get_memory_mapping() stub Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 11/18] dump: Drop qmp_dump_guest_memory() stub and build for all targets Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 12/18] cpu: Change default for CPUClass::get_paging_enabled() Andreas Färber
2013-06-11  9:00   ` Jens Freimann
2013-06-11 15:01   ` Luiz Capitulino
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 13/18] memory_mapping: Cleanup qemu_get_guest_memory_mapping() Andreas Färber
2013-06-11 15:52   ` Luiz Capitulino
2013-06-11 17:47     ` Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 14/18] dump: Abstract dump_init() with cpu_synchronize_all_states() Andreas Färber
2013-06-11 15:55   ` Luiz Capitulino
2013-06-11 17:46     ` Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 15/18] dump: Abstract dump_init() further with qemu_for_each_cpu() Andreas Färber
2013-06-11 15:55   ` Luiz Capitulino
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 16/18] dump: Abstract write_elf{64, 32}_notes() " Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 17/18] memory_mapping: Use hwaddr type for MemoryMapping virt_addr field Andreas Färber
2013-06-09 17:17   ` Peter Maydell
2013-06-09 17:25     ` Andreas Färber
2013-06-09 16:10 ` [Qemu-devel] [PATCH qom-cpu v4 18/18] memory_mapping: Build only once Andreas Färber
2013-06-09 17:29   ` Peter Maydell
2013-06-09 17:36     ` Andreas Färber
2013-06-09 16:19 ` [Qemu-devel] [PATCH qom-cpu v4 00/18] dump: Build cleanups, redone Andreas Färber
2013-06-11 16:54 ` Andreas Färber

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.