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

[Anthony Liguori <anthony@codemonkey.ws>]

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)
>