Re: [Qemu-devel] [PATCH 3/3] memory: move RAM test functions to memory

[Avi Kivity]

On 04/09/2012 10:19 PM, Blue Swirl wrote:
> Rename functions with memory_region_ prefix to avoid future clashes.
>
> Change the RAM/ROM/ROMD test functions to take MemoryRegion
> instead of MemoryRegionSection.
> -
>  #endif
>  #endif
> diff --git a/exec.c b/exec.c
> index 0dda7b5..5080dde 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -507,10 +507,10 @@ target_phys_addr_t
> memory_region_section_get_iotlb(CPUArchState *env,
>      target_phys_addr_t iotlb;
>      CPUWatchpoint *wp;
>
> -    if (is_ram_rom(section)) {
> +    if (memory_region_is_ram_rom(section->mr)) {

Just call memory_region_is_ram()

>  /**
> + * memory_region_is_ram_rom: check whether a memory region is RAM or ROM
> + *
> + * Returns %true is a memory region is RAM or ROM
> + *
> + * @mr: the memory region being queried
> + */
> +static inline bool memory_region_is_ram_rom(MemoryRegion *mr)
> +{
> +    return memory_region_is_ram(mr);
> +}

Redundant

> +
> +/**
> + * memory_region_is_romd: check whether a memory region is ROMD
> + *
> + * Returns %true is a memory region is ROMD

and currently set to allow direct reads.

> + *
> + * @mr: the memory region being queried
> + */
> +static inline bool memory_region_is_romd(MemoryRegion *mr)
> +{
> +    return mr->rom_device && mr->readable;
> +}
> +
> +/**
> + * memory_region_is_ram_rom_romd: check whether a memory region is
> + * RAM, ROM or ROMD
> + *
> + * Returns %true is a memory region is RAM, ROM or ROMD
> + *
> + * @mr: the memory region being queried
> + */
> +static inline bool memory_region_is_ram_rom_romd(MemoryRegion *mr)
> +{
> +    return memory_region_is_ram_rom(mr) || memory_region_is_romd(mr);
> +}

Doesn't seem to warrant a public API.  Fold into callers?

> +
> +/**
>   * memory_region_name: get a memory region's name
>   *
>   * Returns the string that was used to initialize the memory region.
> @@ -666,6 +703,22 @@ void memory_region_set_alias_offset(MemoryRegion *mr,
>  MemoryRegionSection memory_region_find(MemoryRegion *address_space,
>                                         target_phys_addr_t addr, uint64_t 
> size);
>
> +/**
> + * memory_region_section_addr: get offset within MemoryRegionSection
> + *
> + * Returns offset within MemoryRegionSection
> + *
> + * @section: the memory region section being queried
> + * @addr: address in address space
> + */
> +static inline target_phys_addr_t
> +memory_region_section_addr(MemoryRegionSection *section,
> +                           target_phys_addr_t addr)
> +{
> +    addr -= section->offset_within_address_space;
> +    addr += section->offset_within_region;
> +    return addr;
> +}
>

Looks generally useful.

-- 
error compiling committee.c: too many arguments to function