Re: [Qemu-ppc] [PATCH qemu v2 2/4] vfio/spapr: Rename local systempagesize variable

[Alexey Kardashevskiy]

On 15/02/2019 14:32, David Gibson wrote:
> On Thu, Feb 14, 2019 at 04:21:42PM +1100, Alexey Kardashevskiy wrote:
>> The "systempagesize" name suggests that it is the host system page size
>> while it is the smallest page size of memory backing the guest RAM so
>> let's rename it to stop confusion. This should cause no behavioral change.
>>
>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>> ---
>>  hw/vfio/spapr.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
>> index 302d6b0..1498fee 100644
>> --- a/hw/vfio/spapr.c
>> +++ b/hw/vfio/spapr.c
>> @@ -148,14 +148,14 @@ int vfio_spapr_create_window(VFIOContainer *container,
>>      uint64_t pagesize = memory_region_iommu_get_min_page_size(iommu_mr);
>>      unsigned entries, bits_total, bits_per_level, max_levels;
>>      struct vfio_iommu_spapr_tce_create create = { .argsz = sizeof(create) };
>> -    long systempagesize = qemu_getrampagesize();
>> +    long minpagesize = qemu_getrampagesize();
> 
> I'd prefer "rampagesize" or "minrampagesize" here.  "minpagesize"
> doesn't make it clear what it's the minimum of, whereas rampagesize at
> least makes it clear it's the same thing that qemu_getrampagesize() is
> working with.

In my mind "ram" in this context is a guest RAM but here is it not (is
is backend) so I just avoided using any specifics in the name (system,
ram) to make the reader of this code look around where the value comes
from which is the qemu_getrampagesize() helper (more descriptive name).



> 
>>      /*
>>       * The host might not support the guest supported IOMMU page size,
>>       * so we will use smaller physical IOMMU pages to back them.
>>       */
>> -    if (pagesize > systempagesize) {
>> -        pagesize = systempagesize;
>> +    if (pagesize > minpagesize) {
>> +        pagesize = minpagesize;
>>      }
>>      pagesize = 1ULL << (63 - clz64(container->pgsizes &
>>                                     (pagesize | (pagesize - 1))));
> 

-- 
Alexey