Re: [PATCH v2 (resend) 04/27] acpi: vmap pages in acpi_os_alloc_memory

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.06.2024 15:54, Alejandro Vallejo wrote:
> I'm late to the party but there's something bothering me a little.
> 
> On Tue Jan 16, 2024 at 7:25 PM GMT, Elias El Yandouzi wrote:
>> diff --git a/xen/common/vmap.c b/xen/common/vmap.c
>> index 171271fae3..966a7e763f 100644
>> --- a/xen/common/vmap.c
>> +++ b/xen/common/vmap.c
>> @@ -245,6 +245,11 @@ void *vmap(const mfn_t *mfn, unsigned int nr)
>>      return __vmap(mfn, 1, nr, 1, PAGE_HYPERVISOR, VMAP_DEFAULT);
>>  }
>>  
>> +void *vmap_contig(mfn_t mfn, unsigned int nr)
>> +{
>> +    return __vmap(&mfn, nr, 1, 1, PAGE_HYPERVISOR, VMAP_DEFAULT);
>> +}
>> +
>>  unsigned int vmap_size(const void *va)
>>  {
>>      unsigned int pages = vm_size(va, VMAP_DEFAULT);
> 
> How is vmap_contig() different from regular vmap()?
> 
> vmap() calls map_pages_to_xen() `nr` times, while vmap_contig() calls it just
> once. I'd expect both cases to work fine as they are. What am I missing? What
> would make...
> 
>> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
>> index 389505f786..ab80d6b2a9 100644
>> --- a/xen/drivers/acpi/osl.c
>> +++ b/xen/drivers/acpi/osl.c
>> @@ -221,7 +221,11 @@ void *__init acpi_os_alloc_memory(size_t sz)
>>      void *ptr;
>>  
>>      if (system_state == SYS_STATE_early_boot)
>> -            return mfn_to_virt(mfn_x(alloc_boot_pages(PFN_UP(sz), 1)));
>> +    {
>> +            mfn_t mfn = alloc_boot_pages(PFN_UP(sz), 1);
>> +
>> +            return vmap_contig(mfn, PFN_UP(sz));
> ... this statement not operate identically with regular vmap()? Or
> probably more interestingly, what would preclude existing calls to vmap() not
> operate under vmap_contig() instead?

Note how vmap()'s first parameter is "const mfn_t *mfn". This needs to point
to an array of "nr" MFNs. In order to use plain vmap() here, you'd first need
to set up a suitably large array, populate if with increasing MFN values, and
then make the call. Possible, but more complicated.

Jan