[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH qom-cpu v3 9/9] memory_mapping: Change qemu_get_
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH qom-cpu v3 9/9] memory_mapping: Change qemu_get_guest_memory_mapping() semantics |
Date: |
Wed, 05 Jun 2013 15:48:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 |
Am 31.05.2013 16:14, schrieb Luiz Capitulino:
> On Thu, 30 May 2013 17:08:01 +0200
> Andreas Färber <address@hidden> wrote:
>
>> Previously it would search for the first CPU with paging enabled and
>> retrieve memory mappings from this and any following CPUs or return an
>> error if that fails.
>>
>> Instead walk all CPUs and if paging is enabled retrieve the memory
>> mappings. Fail only if retrieving memory mappings on a CPU with paging
>> enabled fails.
>>
>> This not only allows to more easily use one qemu_for_each_cpu() instead
>> of open-coding two CPU loops and drops find_first_paging_enabled_cpu()
>> but removes the implicit assumption of heterogeneity between CPUs n..N
>> in having paging enabled.
>>
>> Cc: Wen Congyang <address@hidden>
>> Signed-off-by: Andreas Färber <address@hidden>
>> ---
>> memory_mapping.c | 42 +++++++++++++++++++++++-------------------
>> 1 file changed, 23 insertions(+), 19 deletions(-)
>>
>> diff --git a/memory_mapping.c b/memory_mapping.c
>> index 481530a..55ac2b8 100644
>> --- a/memory_mapping.c
>> +++ b/memory_mapping.c
>> @@ -165,35 +165,39 @@ void memory_mapping_list_init(MemoryMappingList *list)
>> QTAILQ_INIT(&list->head);
>> }
>>
>> -static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu)
>> +typedef struct GetGuestMemoryMappingData {
>> + MemoryMappingList *list;
>> + int ret;
>> +} GetGuestMemoryMappingData;
>> +
>> +static void qemu_get_one_guest_memory_mapping(CPUState *cpu, void *data)
>> {
>> - CPUArchState *env;
>> + GetGuestMemoryMappingData *s = data;
>> + int ret;
>>
>> - for (env = start_cpu; env != NULL; env = env->next_cpu) {
>> - if (cpu_paging_enabled(ENV_GET_CPU(env))) {
>> - return env;
>> - }
>> + if (!cpu_paging_enabled(cpu) || s->ret == -1) {
>> + return;
>> + }
>> + ret = cpu_get_memory_mapping(cpu, s->list);
>> + if (ret < 0) {
>> + s->ret = -1;
>> + } else {
>> + s->ret = 0;
>> }
>
> Does cpu_get_memory_mapping() ever return a positive value or a negative
> value != -1 ?
>
> It it doesn't, I'd do:
>
> s->ret = cpu_get_memory_mapping();
> assert(s->ret == 0 || s->ret == -1);
This no longer applies after returning an Error* from
cpu_get_memory_mapping() instead. :)
>
>> -
>> - return NULL;
>> }
>>
>> int qemu_get_guest_memory_mapping(MemoryMappingList *list)
>> {
>> - CPUArchState *env, *first_paging_enabled_cpu;
>> + GetGuestMemoryMappingData s = {
>> + .list = list,
>> + .ret = -2,
>> + };
>> 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(ENV_GET_CPU(env), list);
>> - if (ret < 0) {
>> - return -1;
>> - }
>> - }
>> - return 0;
>> + qemu_for_each_cpu(qemu_get_one_guest_memory_mapping, &s);
>> + if (s.ret != -2) {
>> + return s.ret;
>> }
>
> I see we ignore error in dump_init(). Doesn't matter today because
> x86_cpu_get_memory_mapping() never fails. But as you're doing this cleanup,
> shouldn't you add proper error handling?
This patch is not needed for arm/s390x, so we can easily postpone it. I
included it here for early feedback since my series building on this
still needs to be tested and tweaked for bsd-user.
I figure passing through Error** would be the logical follow-up here?
Yet s.ret is being reused to check if any CPU provided any mappings at
all - we could instead do two loops, one for checking if any CPU has
paging enabled and then one passing out any Error* and propagating that.
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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH qom-cpu v3 9/9] memory_mapping: Change qemu_get_guest_memory_mapping() semantics,
Andreas Färber <=