Patchwork Re: [patch uq/master 5/8] Export qemu_ram_addr_from_host

login
register
mail settings
Submitter Marcelo Tosatti
Date Oct. 5, 2010, 8:13 p.m.
Message ID <20101005201319.GA5686@amt.cnet>
Download mbox | patch
Permalink /patch/66857/
State New
Headers show

Comments

Marcelo Tosatti - Oct. 5, 2010, 8:13 p.m.
On Tue, Oct 05, 2010 at 07:57:14AM -0500, Anthony Liguori wrote:
> On 10/04/2010 01:54 PM, Marcelo Tosatti wrote:
> >To be used by next patches.
> >
> >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> >
> >Index: qemu/cpu-common.h
> >===================================================================
> >--- qemu.orig/cpu-common.h
> >+++ qemu/cpu-common.h
> >@@ -47,6 +47,7 @@ void qemu_ram_free(ram_addr_t addr);
> >  /* This should only be used for ram local to a device.  */
> >  void *qemu_get_ram_ptr(ram_addr_t addr);
> >  /* This should not be used by devices.  */
> >+int do_qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr);
> 
> This is not a great name for a function.  A better way to do this
> would be to make the existing qemu_ram_addr_from_host() ->
> qemu_ram_addr_from_host_nofail().

It should fail for all callers in tree now, where address from
qemu_get_ram_ptr() is saved somewhere. MCE handler is an exception to
that.

Are you OK with this:
Anthony Liguori - Oct. 5, 2010, 8:48 p.m.
On 10/05/2010 03:13 PM, Marcelo Tosatti wrote:
> On Tue, Oct 05, 2010 at 07:57:14AM -0500, Anthony Liguori wrote:
>    
>> On 10/04/2010 01:54 PM, Marcelo Tosatti wrote:
>>      
>>> To be used by next patches.
>>>
>>> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
>>>
>>> Index: qemu/cpu-common.h
>>> ===================================================================
>>> --- qemu.orig/cpu-common.h
>>> +++ qemu/cpu-common.h
>>> @@ -47,6 +47,7 @@ void qemu_ram_free(ram_addr_t addr);
>>>   /* This should only be used for ram local to a device.  */
>>>   void *qemu_get_ram_ptr(ram_addr_t addr);
>>>   /* This should not be used by devices.  */
>>> +int do_qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr);
>>>        
>> This is not a great name for a function.  A better way to do this
>> would be to make the existing qemu_ram_addr_from_host() ->
>> qemu_ram_addr_from_host_nofail().
>>      
> It should fail for all callers in tree now, where address from
> qemu_get_ram_ptr() is saved somewhere. MCE handler is an exception to
> that.
>
> Are you OK with this:
>    

I meant the inverse of naming.  nofail means something can never fail 
(because if it does, it aborts).  That happens to be the way we 
currently use that naming convention.

An example is qdev_init() vs. qdev_init_nofail().

Regards,

Anthony Liguori

> Index: qemu/cpu-common.h
> ===================================================================
> --- qemu.orig/cpu-common.h
> +++ qemu/cpu-common.h
> @@ -47,6 +47,7 @@ void qemu_ram_free(ram_addr_t addr);
>   /* This should only be used for ram local to a device.  */
>   void *qemu_get_ram_ptr(ram_addr_t addr);
>   /* This should not be used by devices.  */
> +int qemu_ram_addr_from_host_nofail(void *ptr, ram_addr_t *ram_addr);
>   ram_addr_t qemu_ram_addr_from_host(void *ptr);
>
>   int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
> Index: qemu/exec.c
> ===================================================================
> --- qemu.orig/exec.c
> +++ qemu/exec.c
> @@ -2938,23 +2938,31 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
>       return NULL;
>   }
>
> -/* Some of the softmmu routines need to translate from a host pointer
> -   (typically a TLB entry) back to a ram offset.  */
> -ram_addr_t qemu_ram_addr_from_host(void *ptr)
> +int qemu_ram_addr_from_host_nofail(void *ptr, ram_addr_t *ram_addr)
>   {
>       RAMBlock *block;
>       uint8_t *host = ptr;
>
>       QLIST_FOREACH(block,&ram_list.blocks, next) {
>           if (host - block->host<  block->length) {
> -            return block->offset + (host - block->host);
> +            *ram_addr = block->offset + (host - block->host);
> +            return 0;
>           }
>       }
> +    return -1;
> +}
>
> -    fprintf(stderr, "Bad ram pointer %p\n", ptr);
> -    abort();
> +/* Some of the softmmu routines need to translate from a host pointer
> +   (typically a TLB entry) back to a ram offset.  */
> +ram_addr_t qemu_ram_addr_from_host(void *ptr)
> +{
> +    ram_addr_t ram_addr;
>
> -    return 0;
> +    if (qemu_ram_addr_from_host_nofail(ptr,&ram_addr)) {
> +        fprintf(stderr, "Bad ram pointer %p\n", ptr);
> +        abort();
> +    }
> +    return ram_addr;
>   }
>
>   static uint32_t unassigned_mem_readb(void *opaque, target_phys_addr_t addr)
>

Patch

Index: qemu/cpu-common.h
===================================================================
--- qemu.orig/cpu-common.h
+++ qemu/cpu-common.h
@@ -47,6 +47,7 @@  void qemu_ram_free(ram_addr_t addr);
 /* This should only be used for ram local to a device.  */
 void *qemu_get_ram_ptr(ram_addr_t addr);
 /* This should not be used by devices.  */
+int qemu_ram_addr_from_host_nofail(void *ptr, ram_addr_t *ram_addr);
 ram_addr_t qemu_ram_addr_from_host(void *ptr);
 
 int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
Index: qemu/exec.c
===================================================================
--- qemu.orig/exec.c
+++ qemu/exec.c
@@ -2938,23 +2938,31 @@  void *qemu_get_ram_ptr(ram_addr_t addr)
     return NULL;
 }
 
-/* Some of the softmmu routines need to translate from a host pointer
-   (typically a TLB entry) back to a ram offset.  */
-ram_addr_t qemu_ram_addr_from_host(void *ptr)
+int qemu_ram_addr_from_host_nofail(void *ptr, ram_addr_t *ram_addr)
 {
     RAMBlock *block;
     uint8_t *host = ptr;
 
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         if (host - block->host < block->length) {
-            return block->offset + (host - block->host);
+            *ram_addr = block->offset + (host - block->host);
+            return 0;
         }
     }
+    return -1;
+}
 
-    fprintf(stderr, "Bad ram pointer %p\n", ptr);
-    abort();
+/* Some of the softmmu routines need to translate from a host pointer
+   (typically a TLB entry) back to a ram offset.  */
+ram_addr_t qemu_ram_addr_from_host(void *ptr)
+{
+    ram_addr_t ram_addr;
 
-    return 0;
+    if (qemu_ram_addr_from_host_nofail(ptr, &ram_addr)) {
+        fprintf(stderr, "Bad ram pointer %p\n", ptr);
+        abort();
+    }
+    return ram_addr;
 }
 
 static uint32_t unassigned_mem_readb(void *opaque, target_phys_addr_t addr)