[v2] Add skip_dump flag to ignore memory region during dump

The PCI MMIO might be disabled or the device in the reset state.
Make sure we do not dump these memory regions.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---

V1: 
   * Make the flag generic in place of using vfio_mmap (Alex)

 hw/misc/vfio.c        |  1 +
 include/exec/memory.h | 19 +++++++++++++++++++
 memory.c              | 11 +++++++++++
 memory_mapping.c      |  3 ++-
 4 files changed, 33 insertions(+), 1 deletion(-)

On Fri, 2014-09-12 at 16:27 +0530, Nikunj A Dadhania wrote:
> The PCI MMIO might be disabled or the device in the reset state.
> Make sure we do not dump these memory regions.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---

Looks ok to me.

Acked-by: Alex Williamson <alex.williamson@redhat.com>

> V1: 
>    * Make the flag generic in place of using vfio_mmap (Alex)
> 
>  hw/misc/vfio.c        |  1 +
>  include/exec/memory.h | 19 +++++++++++++++++++
>  memory.c              | 11 +++++++++++
>  memory_mapping.c      |  3 ++-
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index d69bb29..c9c0398 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -2722,6 +2722,7 @@ static int vfio_mmap_bar(VFIODevice *vdev, VFIOBAR *bar,
>          }
>  
>          memory_region_init_ram_ptr(submem, OBJECT(vdev), name, size, *map);
> +        memory_region_set_skip_dump(submem);
>      } else {
>  empty_region:
>          /* Create a zero sized sub-region to make cleanup easy. */
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index fc6e93d..2b5c1f2 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -144,6 +144,7 @@ struct MemoryRegion {
>      bool terminates;
>      bool romd_mode;
>      bool ram;
> +    bool skip_dump;
>      bool readonly; /* For RAM regions */
>      bool enabled;
>      bool rom_device;
> @@ -434,6 +435,24 @@ uint64_t memory_region_size(MemoryRegion *mr);
>  bool memory_region_is_ram(MemoryRegion *mr);
>  
>  /**
> + * memory_region_is_skip_dump: check whether a memory region should not be
> + *                             dumped
> + *
> + * Returns %true is a memory region should not be dumped(e.g. VFIO BAR MMAP).
> + *
> + * @mr: the memory region being queried
> + */
> +bool memory_region_is_skip_dump(MemoryRegion *mr);
> +
> +/**
> + * memory_region_set_skip_dump: Set skip_dump flag, dump will ignore this memory
> + *                              region
> + *
> + * @mr: the memory region being queried
> + */
> +void memory_region_set_skip_dump(MemoryRegion *mr);
> +
> +/**
>   * memory_region_is_romd: check whether a memory region is in ROMD mode
>   *
>   * Returns %true if a memory region is a ROM device and currently set to allow
> diff --git a/memory.c b/memory.c
> index 664d3e6..cbd8cad 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -847,6 +847,7 @@ void memory_region_init(MemoryRegion *mr,
>      mr->enabled = true;
>      mr->terminates = false;
>      mr->ram = false;
> +    mr->skip_dump = false;
>      mr->romd_mode = true;
>      mr->readonly = false;
>      mr->rom_device = false;
> @@ -1032,6 +1033,11 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
>      mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr);
>  }
>  
> +void memory_region_set_skip_dump(MemoryRegion *mr)
> +{
> +    mr->skip_dump = true;
> +}
> +
>  void memory_region_init_alias(MemoryRegion *mr,
>                                Object *owner,
>                                const char *name,
> @@ -1129,6 +1135,11 @@ bool memory_region_is_ram(MemoryRegion *mr)
>      return mr->ram;
>  }
>  
> +bool memory_region_is_skip_dump(MemoryRegion *mr)
> +{
> +    return mr->skip_dump;
> +}
> +
>  bool memory_region_is_logging(MemoryRegion *mr)
>  {
>      return mr->dirty_log_mask;
> diff --git a/memory_mapping.c b/memory_mapping.c
> index 87a6ed5..7b69801 100644
> --- a/memory_mapping.c
> +++ b/memory_mapping.c
> @@ -203,7 +203,8 @@ static void guest_phys_blocks_region_add(MemoryListener *listener,
>      GuestPhysBlock *predecessor;
>  
>      /* we only care about RAM */
> -    if (!memory_region_is_ram(section->mr)) {
> +    if (!memory_region_is_ram(section->mr) ||
> +        memory_region_is_skip_dump(section->mr)) {
>          return;
>      }
>

On 12.09.14 17:28, Alex Williamson wrote:
> On Fri, 2014-09-12 at 16:27 +0530, Nikunj A Dadhania wrote:
>> The PCI MMIO might be disabled or the device in the reset state.
>> Make sure we do not dump these memory regions.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

I think you want to CC Paolo, as according to MAINTAINERS he's
maintaining the memory API.


Alex

>> ---
> 
> Looks ok to me.
> 
> Acked-by: Alex Williamson <alex.williamson@redhat.com>
> 
>> V1: 
>>    * Make the flag generic in place of using vfio_mmap (Alex)
>>
>>  hw/misc/vfio.c        |  1 +
>>  include/exec/memory.h | 19 +++++++++++++++++++
>>  memory.c              | 11 +++++++++++
>>  memory_mapping.c      |  3 ++-
>>  4 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
>> index d69bb29..c9c0398 100644
>> --- a/hw/misc/vfio.c
>> +++ b/hw/misc/vfio.c
>> @@ -2722,6 +2722,7 @@ static int vfio_mmap_bar(VFIODevice *vdev, VFIOBAR *bar,
>>          }
>>  
>>          memory_region_init_ram_ptr(submem, OBJECT(vdev), name, size, *map);
>> +        memory_region_set_skip_dump(submem);
>>      } else {
>>  empty_region:
>>          /* Create a zero sized sub-region to make cleanup easy. */
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index fc6e93d..2b5c1f2 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -144,6 +144,7 @@ struct MemoryRegion {
>>      bool terminates;
>>      bool romd_mode;
>>      bool ram;
>> +    bool skip_dump;
>>      bool readonly; /* For RAM regions */
>>      bool enabled;
>>      bool rom_device;
>> @@ -434,6 +435,24 @@ uint64_t memory_region_size(MemoryRegion *mr);
>>  bool memory_region_is_ram(MemoryRegion *mr);
>>  
>>  /**
>> + * memory_region_is_skip_dump: check whether a memory region should not be
>> + *                             dumped
>> + *
>> + * Returns %true is a memory region should not be dumped(e.g. VFIO BAR MMAP).
>> + *
>> + * @mr: the memory region being queried
>> + */
>> +bool memory_region_is_skip_dump(MemoryRegion *mr);
>> +
>> +/**
>> + * memory_region_set_skip_dump: Set skip_dump flag, dump will ignore this memory
>> + *                              region
>> + *
>> + * @mr: the memory region being queried
>> + */
>> +void memory_region_set_skip_dump(MemoryRegion *mr);
>> +
>> +/**
>>   * memory_region_is_romd: check whether a memory region is in ROMD mode
>>   *
>>   * Returns %true if a memory region is a ROM device and currently set to allow
>> diff --git a/memory.c b/memory.c
>> index 664d3e6..cbd8cad 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -847,6 +847,7 @@ void memory_region_init(MemoryRegion *mr,
>>      mr->enabled = true;
>>      mr->terminates = false;
>>      mr->ram = false;
>> +    mr->skip_dump = false;
>>      mr->romd_mode = true;
>>      mr->readonly = false;
>>      mr->rom_device = false;
>> @@ -1032,6 +1033,11 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
>>      mr->ram_addr = qemu_ram_alloc_from_ptr(size, ptr, mr);
>>  }
>>  
>> +void memory_region_set_skip_dump(MemoryRegion *mr)
>> +{
>> +    mr->skip_dump = true;
>> +}
>> +
>>  void memory_region_init_alias(MemoryRegion *mr,
>>                                Object *owner,
>>                                const char *name,
>> @@ -1129,6 +1135,11 @@ bool memory_region_is_ram(MemoryRegion *mr)
>>      return mr->ram;
>>  }
>>  
>> +bool memory_region_is_skip_dump(MemoryRegion *mr)
>> +{
>> +    return mr->skip_dump;
>> +}
>> +
>>  bool memory_region_is_logging(MemoryRegion *mr)
>>  {
>>      return mr->dirty_log_mask;
>> diff --git a/memory_mapping.c b/memory_mapping.c
>> index 87a6ed5..7b69801 100644
>> --- a/memory_mapping.c
>> +++ b/memory_mapping.c
>> @@ -203,7 +203,8 @@ static void guest_phys_blocks_region_add(MemoryListener *listener,
>>      GuestPhysBlock *predecessor;
>>  
>>      /* we only care about RAM */
>> -    if (!memory_region_is_ram(section->mr)) {
>> +    if (!memory_region_is_ram(section->mr) ||
>> +        memory_region_is_skip_dump(section->mr)) {
>>          return;
>>      }
>>  
> 
> 
>