Re: [PATCH 05/22] x86/srat: vmap the pages for acpi_slit

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.12.2022 12:31, Julien Grall wrote:
> On 20/12/2022 15:30, Jan Beulich wrote:
>> On 16.12.2022 12:48, Julien Grall wrote:
>>> From: Hongyan Xia <hongyxia@xxxxxxxxxx>
>>>
>>> This avoids the assumption that boot pages are in the direct map.
>>>
>>> Signed-off-by: Hongyan Xia <hongyxia@xxxxxxxxxx>
>>> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
>>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> However, ...
>>
>>> --- a/xen/arch/x86/srat.c
>>> +++ b/xen/arch/x86/srat.c
>>> @@ -139,7 +139,8 @@ void __init acpi_numa_slit_init(struct acpi_table_slit 
>>> *slit)
>>>             return;
>>>     }
>>>     mfn = alloc_boot_pages(PFN_UP(slit->header.length), 1);
>>> -   acpi_slit = mfn_to_virt(mfn_x(mfn));
>>> +   acpi_slit = vmap_contig_pages(mfn, PFN_UP(slit->header.length));
>>
>> ... with the increased use of vmap space the VA range used will need
>> growing. And that's perhaps better done ahead of time than late.
> 
> I will have a look to increase the vmap().
> 
>>
>>> +   BUG_ON(!acpi_slit);
>>
>> Similarly relevant for the earlier patch: It would be nice if boot
>> failure for optional things like NUMA data could be avoided.
> 
> If you can't map (or allocate the memory), then you are probably in a 
> very bad situation because both should really not fail at boot.
> 
> So I think this is correct to crash early because the admin will be able 
> to look what went wrong. Otherwise, it may be missed in the noise.

Well, I certainly can see one taking this view. However, at least in
principle allocation (or mapping) may fail _because_ of NUMA issues.
At which point it would be better to boot with NUMA support turned off.

Jan