On 22.07.2011, at 07:42, Liu Yu-B13201 wrote:
>
>
>> -----Original Message-----
>> From: qemu-devel-bounces+yu.liu=freescale.com@xxxxxxxxxx
>> [mailto:qemu-devel-bounces+yu.liu=freescale.com@xxxxxxxxxx]
>> On Behalf Of stefano.stabellini@xxxxxxxxxxxxx
>> Sent: Friday, May 20, 2011 1:36 AM
>> To: qemu-devel@xxxxxxxxxx
>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; agraf@xxxxxxx; Stefano Stabellini
>> Subject: [Qemu-devel] [PATCH v2 4/5] exec.c: refactor
>> cpu_physical_memory_map
>>
>> From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>>
>> Introduce qemu_ram_ptr_length that takes an address and a size as
>> parameters rather than just an address.
>>
>> Refactor cpu_physical_memory_map so that we call
>> qemu_ram_ptr_length only
>> once rather than calling qemu_get_ram_ptr one time per page.
>> This is not only more efficient but also tries to simplify
>> the logic of
>> the function.
>> Currently we are relying on the fact that all the pages are mapped
>> contiguously in qemu's address space: we have a check to make
>> sure that
>> the virtual address returned by qemu_get_ram_ptr from the
>> second call on
>> is consecutive. Now we are making this more explicit replacing all the
>> calls to qemu_get_ram_ptr with a single call to qemu_ram_ptr_length
>> passing a size argument.
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>> CC: agraf@xxxxxxx
>> CC: anthony@xxxxxxxxxxxxx
>> ---
>> cpu-common.h | 1 +
>> exec.c | 51
>> ++++++++++++++++++++++++++++++++++-----------------
>> 2 files changed, 35 insertions(+), 17 deletions(-)
>>
>> diff --git a/cpu-common.h b/cpu-common.h
>> index 151c32c..085aacb 100644
>> --- a/cpu-common.h
>> +++ b/cpu-common.h
>> @@ -64,6 +64,7 @@ void qemu_ram_free(ram_addr_t addr);
>> void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
>> /* This should only be used for ram local to a device. */
>> void *qemu_get_ram_ptr(ram_addr_t addr);
>> +void *qemu_ram_ptr_length(target_phys_addr_t addr,
>> target_phys_addr_t *size);
>> /* Same but slower, to use for migration, where the order of
>> * RAMBlocks must not change. */
>> void *qemu_safe_ram_ptr(ram_addr_t addr);
>> diff --git a/exec.c b/exec.c
>> index 21f21f0..ff9c174 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3111,6 +3111,31 @@ void *qemu_safe_ram_ptr(ram_addr_t addr)
>> return NULL;
>> }
>>
>> +/* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr
>> + * but takes a size argument */
>> +void *qemu_ram_ptr_length(target_phys_addr_t addr,
>> target_phys_addr_t *size)
>> +{
>> + if (xen_mapcache_enabled())
>> + return qemu_map_cache(addr, *size, 1);
>> + else {
>> + RAMBlock *block;
>> +
>> + QLIST_FOREACH(block, &ram_list.blocks, next) {
>> + if (addr - block->offset < block->length) {
>> + if (addr - block->offset + *size > block->length)
>> + *size = block->length - addr + block->offset;
>> + return block->host + (addr - block->offset);
>> + }
>> + }
>> +
>> + fprintf(stderr, "Bad ram offset %" PRIx64 "\n",
>> (uint64_t)addr);
>> + abort();
>> +
>> + *size = 0;
>> + return NULL;
>> + }
>> +}
>> +
>> void qemu_put_ram_ptr(void *addr)
>> {
>> trace_qemu_put_ram_ptr(addr);
>> @@ -3972,14 +3997,12 @@ void
>> *cpu_physical_memory_map(target_phys_addr_t addr,
>> int is_write)
>> {
>> target_phys_addr_t len = *plen;
>> - target_phys_addr_t done = 0;
>> + target_phys_addr_t todo = 0;
>> int l;
>> - uint8_t *ret = NULL;
>> - uint8_t *ptr;
>> target_phys_addr_t page;
>> unsigned long pd;
>> PhysPageDesc *p;
>> - unsigned long addr1;
>> + target_phys_addr_t addr1 = addr;
>>
>> while (len > 0) {
>> page = addr & TARGET_PAGE_MASK;
>> @@ -3994,7 +4017,7 @@ void
>> *cpu_physical_memory_map(target_phys_addr_t addr,
>> }
>>
>> if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM) {
>> - if (done || bounce.buffer) {
>> + if (todo || bounce.buffer) {
>> break;
>> }
>> bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE,
>> TARGET_PAGE_SIZE);
>> @@ -4003,23 +4026,17 @@ void
>> *cpu_physical_memory_map(target_phys_addr_t addr,
>> if (!is_write) {
>> cpu_physical_memory_read(addr, bounce.buffer, l);
>> }
>> - ptr = bounce.buffer;
>> - } else {
>> - addr1 = (pd & TARGET_PAGE_MASK) + (addr &
>> ~TARGET_PAGE_MASK);
>> - ptr = qemu_get_ram_ptr(addr1);
>> - }
>> - if (!done) {
>> - ret = ptr;
>> - } else if (ret + done != ptr) {
>> - break;
>> +
>> + *plen = l;
>> + return bounce.buffer;
>> }
>>
>> len -= l;
>> addr += l;
>> - done += l;
>> + todo += l;
>> }
>> - *plen = done;
>> - return ret;
>> + *plen = todo;
>> + return qemu_ram_ptr_length(addr1, plen);
>> }
>>
>> /* Unmaps a memory region previously mapped by
>> cpu_physical_memory_map().
>> --
>> 1.7.2.3
>
> Hello Stefano,
>
> This commit breaks the case that guest memory doesn't start from 0.
>
> In previous code
> addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
> This transfer guest physical addr to qemu ram_addr, and so that it can pass
> the ram range checking.
>
> But current code
> addr1 = addr
> this make it fail to pass the ram range checking.
Are you sure it's still broken with commit
8ab934f93b5ad3d0af4ad419d2531235a75d672c? If so, mind to pinpoint where exactly?
Alex
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home