Re: [PATCH v4] page_alloc: assert IRQs are enabled in heap alloc/free

[Julien Grall <julien@xxxxxxx>]

Hi David,

On 25/04/2022 14:28, David Vrabel wrote:
> From: David Vrabel <dvrabel@xxxxxxxxxxxx>
>
> Heap pages can only be safely allocated and freed with interuupts
> enabled as they may require a TLB flush which will send IPIs (on x86).
>
> Normally spinlock debugging would catch calls from the incorrect
> context, but not from stop_machine_run() action functions as these are
> called with spin lock debugging disabled.
>
> Enhance the assertions in alloc_xenheap_pages() and
> alloc_domheap_pages() to check interrupts are enabled. For consistency
> the same asserts are used when freeing heap pages.
>
> As an exception, when only 1 PCPU is online, allocations are permitted
> with interrupts disabled as any TLB flushes would be local only. This
> is necessary during early boot.
>
> Signed-off-by: David Vrabel <dvrabel@xxxxxxxxxxxx>
> ---
> Changes in v4:
> - Tweak comment.
>
> Changes in v3:
> - Use num_online_cpus() in assert.
>
> Changes in v2:
> - Set SYS_STATE_smp_boot on arm.
> ---
>   xen/common/page_alloc.c | 23 +++++++++++++++--------
>   1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 319029140f..739ca6e74b 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -162,6 +162,13 @@
>   static char __initdata opt_badpage[100] = "";
>   string_param("badpage", opt_badpage);
>   
> +/*
> + * Heap allocations may need TLB flushes which require IRQs to be
The comment needs to be updated to reflect the fact that at least Arm 
doesn't use IPI to flush TLBs.
The update can possibly be done on commit.

> + * enabled (except when only 1 PCPU is online).
> + */
Cheers,

--
Julien Grall