Re: [RFC PATCH 04/19] memory: Introduce memory_region_can_be_private()

[Claudio Fontana <cfontana@suse.de>]

On 8/1/23 18:48, Claudio Fontana wrote:
> On 7/31/23 18:21, Xiaoyao Li wrote:
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>  include/exec/memory.h | 9 +++++++++
>>  softmmu/memory.c      | 5 +++++
>>  2 files changed, 14 insertions(+)
>>
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 61e31c7b9874..e119d3ce1a1d 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -1679,6 +1679,15 @@ static inline bool memory_region_is_romd(MemoryRegion *mr)
>>   */
>>  bool memory_region_is_protected(MemoryRegion *mr);
>>  
>> +/**
>> + * memory_region_can_be_private: check whether a memory region can be private
> 
> The name of the function is not particularly informative,
> 
>> + *
>> + * Returns %true if a memory region's ram_block has valid gmem fd assigned.
> 
> but in your comment you describe more accurately what it does, why not make it the function name?
> 
> bool memory_region_has_valid_gmem_fd()


btw can a memory region have an invalid gmem_fd ?

If an invalid gmem_fd is just used to mark whether gmem_fd is present or not,

we could make it just:

bool memory_region_has_gmem_fd()


Thanks,

C

> 
>> + *
>> + * @mr: the memory region being queried
>> + */
>> +bool memory_region_can_be_private(MemoryRegion *mr);
> 
> 
> bool memory_region_has_valid_gmem_fd()
> 
> 
> Thanks,
> 
> C
> 
>> +
>>  /**
>>   * memory_region_get_iommu: check whether a memory region is an iommu
>>   *
>> diff --git a/softmmu/memory.c b/softmmu/memory.c
>> index 4f8f8c0a02e6..336c76ede660 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -1855,6 +1855,11 @@ bool memory_region_is_protected(MemoryRegion *mr)
>>      return mr->ram && (mr->ram_block->flags & RAM_PROTECTED);
>>  }
>>  
>> +bool memory_region_can_be_private(MemoryRegion *mr)
>> +{
>> +    return mr->ram_block && mr->ram_block->gmem_fd >= 0;
>> +}
>> +
>>  uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr)
>>  {
>>      uint8_t mask = mr->dirty_log_mask;
>