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 <a...@ozlabs.ru>
>> ---
>> 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
