From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39843) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UmQsf-0002bg-NT for qemu-devel@nongnu.org; Tue, 11 Jun 2013 11:53:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UmQsZ-0003cc-HQ for qemu-devel@nongnu.org; Tue, 11 Jun 2013 11:53:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64090) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UmQsZ-0003cN-8G for qemu-devel@nongnu.org; Tue, 11 Jun 2013 11:53:03 -0400 Date: Tue, 11 Jun 2013 11:52:56 -0400 From: Luiz Capitulino Message-ID: <20130611115256.76476459@redhat.com> In-Reply-To: <1370794247-28267-14-git-send-email-afaerber@suse.de> References: <1370794247-28267-1-git-send-email-afaerber@suse.de> <1370794247-28267-14-git-send-email-afaerber@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH qom-cpu v4 13/18] memory_mapping: Cleanup qemu_get_guest_memory_mapping() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?UTF-8?B?RsOkcmJlcg==?= Cc: qiaonuohan@cn.fujitsu.com, qemu-devel@nongnu.org On Sun, 9 Jun 2013 18:10:42 +0200 Andreas F=C3=A4rber 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. >=20 > This allows to use qemu_for_each_cpu() instead of open-coding CPU loops. >=20 > 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. >=20 > Signed-off-by: Andreas F=C3=A4rber Reviewed-by: Luiz Capitulino > --- > dump.c | 7 +++++- > include/sysemu/memory_mapping.h | 8 +------ > memory_mapping.c | 52 ++++++++++++++++++++++++-----------= ------ > 3 files changed, 38 insertions(+), 29 deletions(-) >=20 > 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 pagin= g, bool has_filter, > { > CPUArchState *env; > int nr_cpus; > + Error *err =3D NULL; > int ret; > =20 > if (runstate_is_running()) { > @@ -756,7 +757,11 @@ static int dump_init(DumpState *s, int fd, bool pagi= ng, 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 !=3D 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_mapp= ing.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); > =20 > void memory_mapping_list_init(MemoryMappingList *list); > =20 > -/* > - * 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= ); > =20 > /* 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 *li= st) > QTAILQ_INIT(&list->head); > } > =20 > -static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu) > +static void find_paging_enabled_cpu(CPUState *cpu, void *data) > { > - CPUArchState *env; > + bool *ret =3D data; > =20 > - for (env =3D start_cpu; env !=3D NULL; env =3D env->next_cpu) { > - if (cpu_paging_enabled(ENV_GET_CPU(env))) { > - return env; > - } > + if (*ret) { > + return; > } > + *ret =3D 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 =3D data; > =20 > - return NULL; > + if (s->err !=3D NULL || !cpu_paging_enabled(cpu)) { > + return; > + } > + cpu_get_memory_mapping(cpu, s->list, &s->err); > } > =20 > -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 =3D { > + .list =3D list, > + .err =3D NULL, > + }; > + bool paging_enabled =3D false; > RAMBlock *block; > ram_addr_t offset, length; > =20 > - first_paging_enabled_cpu =3D find_paging_enabled_cpu(first_cpu); > - if (first_paging_enabled_cpu) { > - for (env =3D first_paging_enabled_cpu; env !=3D NULL; env =3D en= v->next_cpu) { > - Error *err =3D 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 !=3D NULL) { > + error_propagate(errp, s.err); > } > - return 0; > + return; > } > =20 > /* > @@ -206,8 +218,6 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *= list) > length =3D block->length; > create_new_memory_mapping(list, offset, offset, length); > } > - > - return 0; > } > =20 > void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)