On 10.12.2021 15:10, Oleksandr Andrushchenko wrote:
> On 10.12.21 11:40, Jan Beulich wrote:
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
>>           break;
>>       }
>>       case XENMAPSPACE_dev_mmio:
>> -        rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
>> +        rc = map_dev_mmio_region(d, gfn, _mfn(idx));
> Technically this is ok, but reads odd now: the function maps a single
> page, but its name has "region" in it (which might also be ok, e.g.
> for a region of a single page).
> 
> I think it is worth either implementing full mfn range check inside
> map_dev_mmio_region or renaming it to something else:
> with mfn check map_dev_mmio_region will indeed be able to map
> a region consisting of multiple pages and perform required validation.

Well, I had no maintainer comments on v1 regarding the name. I'd be
happy to rename to e.g. map_dev_mmio_page(), so long as there can be
an agreed upon name before I submit a possible v3. Julien, Stefano?

Jan


Reply via email to