At 03/01/2012 02:01 PM, HATAYAMA Daisuke Wrote: > From: Wen Congyang <we...@cn.fujitsu.com> > Subject: [RFC][PATCH 04/14 v7] Add API to get memory mapping > Date: Thu, 01 Mar 2012 10:43:13 +0800 > >> +int qemu_get_guest_memory_mapping(MemoryMappingList *list) >> +{ >> + CPUState *env; >> + MemoryMapping *memory_mapping; >> + RAMBlock *block; >> + ram_addr_t offset, length; >> + int ret; >> + >> +#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING) >> + for (env = first_cpu; env != NULL; env = env->next_cpu) { >> + ret = cpu_get_memory_mapping(list, env); >> + if (ret < 0) { >> + return -1; >> + } >> + } >> +#else >> + return -2; >> +#endif >> + >> + /* some memory may be not mapped, add them into memory mapping's list */ > > The part from here is logic fully for 2nd kernel? If so, I think it > better to describe why this addtional mapping is needed; we should > assume most people doesn't know kdump mechanism.
Not only for 2nd kernel. If the guest has 1 vcpu, and is in the 2nd kernel, we need this logic for 1st kernel. Thanks Wen Congyang > >> + QLIST_FOREACH(block, &ram_list.blocks, next) { >> + offset = block->offset; >> + length = block->length; >> + >> + QTAILQ_FOREACH(memory_mapping, &list->head, next) { >> + if (memory_mapping->phys_addr >= (offset + length)) { >> + /* >> + * memory_mapping's list does not conatin the region >> + * [offset, offset+length) >> + */ >> + create_new_memory_mapping(list, offset, 0, length); >> + length = 0; >> + break; >> + } >> + >> + if ((memory_mapping->phys_addr + memory_mapping->length) <= >> + offset) { >> + continue; >> + } >> + >> + if (memory_mapping->phys_addr > offset) { >> + /* >> + * memory_mapping's list does not conatin the region >> + * [offset, memory_mapping->phys_addr) >> + */ >> + create_new_memory_mapping(list, offset, 0, >> + memory_mapping->phys_addr - >> offset); >> + } >> + >> + if ((offset + length) <= >> + (memory_mapping->phys_addr + memory_mapping->length)) { >> + length = 0; >> + break; >> + } >> + length -= memory_mapping->phys_addr + memory_mapping->length - >> + offset; >> + offset = memory_mapping->phys_addr + memory_mapping->length; >> + } >> + >> + if (length > 0) { >> + /* >> + * memory_mapping's list does not conatin the region >> + * [offset, memory_mapping->phys_addr) >> + */ >> + create_new_memory_mapping(list, offset, 0, length); >> + } >> + } >> + >> + return 0; >> +} > > I think it more readable if shortening memory_mapping->phys_addr and > memmory_maping->length at the berinning of the innermost foreach loop. > > m_phys_addr = memory_mapping->phys_addr; > m_length = memory_mapping->length; > > Then, each conditionals becomes compact. > > Thanks. > HATAYAMA, Daisuke > >