diff mbox series

[v6,2/9] xen: do not free reserved memory into heap

Message ID 20220607073031.722174-3-Penny.Zheng@arm.com (mailing list archive)
State Superseded
Headers show
Series populate/unpopulate memory when domain on static allocation | expand

Commit Message

Penny Zheng June 7, 2022, 7:30 a.m. UTC
Pages used as guest RAM for static domain, shall be reserved to this
domain only.
So in case reserved pages being used for other purpose, users
shall not free them back to heap, even when last ref gets dropped.

free_staticmem_pages will be called by free_heap_pages in runtime
for static domain freeing memory resource, so let's drop the __init
flag.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v6 changes:
- adapt to PGC_static
- remove #ifdef aroud function declaration
---
v5 changes:
- In order to avoid stub functions, we #define PGC_staticmem to non-zero only
when CONFIG_STATIC_MEMORY
- use "unlikely()" around pg->count_info & PGC_staticmem
- remove pointless "if", since mark_page_free() is going to set count_info
to PGC_state_free and by consequence clear PGC_staticmem
- move #define PGC_staticmem 0 to mm.h
---
v4 changes:
- no changes
---
v3 changes:
- fix possible racy issue in free_staticmem_pages()
- introduce a stub free_staticmem_pages() for the !CONFIG_STATIC_MEMORY case
- move the change to free_heap_pages() to cover other potential call sites
- fix the indentation
---
v2 changes:
- new commit
---
 xen/arch/arm/include/asm/mm.h |  4 +++-
 xen/common/page_alloc.c       | 12 +++++++++---
 xen/include/xen/mm.h          |  2 --
 3 files changed, 12 insertions(+), 6 deletions(-)

Comments

Jan Beulich June 7, 2022, 7:45 a.m. UTC | #1
On 07.06.2022 09:30, Penny Zheng wrote:
> Pages used as guest RAM for static domain, shall be reserved to this
> domain only.
> So in case reserved pages being used for other purpose, users
> shall not free them back to heap, even when last ref gets dropped.
> 
> free_staticmem_pages will be called by free_heap_pages in runtime
> for static domain freeing memory resource, so let's drop the __init
> flag.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
Julien Grall June 7, 2022, 9:13 a.m. UTC | #2
Hi Penny,

On 07/06/2022 08:30, Penny Zheng wrote:
> Pages used as guest RAM for static domain, shall be reserved to this
> domain only.
> So in case reserved pages being used for other purpose, users
> shall not free them back to heap, even when last ref gets dropped.
> 
> free_staticmem_pages will be called by free_heap_pages in runtime
> for static domain freeing memory resource, so let's drop the __init
> flag.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> ---
> v6 changes:
> - adapt to PGC_static
> - remove #ifdef aroud function declaration
> ---
> v5 changes:
> - In order to avoid stub functions, we #define PGC_staticmem to non-zero only
> when CONFIG_STATIC_MEMORY
> - use "unlikely()" around pg->count_info & PGC_staticmem
> - remove pointless "if", since mark_page_free() is going to set count_info
> to PGC_state_free and by consequence clear PGC_staticmem
> - move #define PGC_staticmem 0 to mm.h
> ---
> v4 changes:
> - no changes
> ---
> v3 changes:
> - fix possible racy issue in free_staticmem_pages()
> - introduce a stub free_staticmem_pages() for the !CONFIG_STATIC_MEMORY case
> - move the change to free_heap_pages() to cover other potential call sites
> - fix the indentation
> ---
> v2 changes:
> - new commit
> ---
>   xen/arch/arm/include/asm/mm.h |  4 +++-
>   xen/common/page_alloc.c       | 12 +++++++++---
>   xen/include/xen/mm.h          |  2 --
>   3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index fbff11c468..7442893e77 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -108,9 +108,11 @@ struct page_info
>     /* Page is Xen heap? */
>   #define _PGC_xen_heap     PG_shift(2)
>   #define PGC_xen_heap      PG_mask(1, 2)
> -  /* Page is static memory */

NITpicking: You added this comment in patch #1 and now removing the 
space. Any reason to drop the space?

> +#ifdef CONFIG_STATIC_MEMORY

I think this change ought to be explained in the commit message. AFAIU, 
this is necessary to allow the compiler to remove code and avoid linking 
issues. Is that correct?

> +/* Page is static memory */
>   #define _PGC_static    PG_shift(3)
>   #define PGC_static     PG_mask(1, 3)
> +#endif
>   /* ... */
>   /* Page is broken? */
>   #define _PGC_broken       PG_shift(7)
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 9e5c757847..6876869fa6 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1443,6 +1443,13 @@ static void free_heap_pages(
>   
>       ASSERT(order <= MAX_ORDER);
>   
> +    if ( unlikely(pg->count_info & PGC_static) )
> +    {
> +        /* Pages of static memory shall not go back to the heap. */
> +        free_staticmem_pages(pg, 1UL << order, need_scrub);
I can't remember whether I asked this before (I couldn't find a thread).

free_staticmem_pages() doesn't seem to be protected by any lock. So how 
do you prevent the concurrent access to the page info with the acquire part?

Cheers,
Penny Zheng June 9, 2022, 5:54 a.m. UTC | #3
> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: Tuesday, June 7, 2022 5:13 PM
> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org
> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>;
> Jan Beulich <jbeulich@suse.com>; Wei Liu <wl@xen.org>
> Subject: Re: [PATCH v6 2/9] xen: do not free reserved memory into heap
> 
> Hi Penny,
> 

Hi Julien

> On 07/06/2022 08:30, Penny Zheng wrote:
> > Pages used as guest RAM for static domain, shall be reserved to this
> > domain only.
> > So in case reserved pages being used for other purpose, users shall
> > not free them back to heap, even when last ref gets dropped.
> >
> > free_staticmem_pages will be called by free_heap_pages in runtime for
> > static domain freeing memory resource, so let's drop the __init flag.
> >
> > Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> > ---
> > v6 changes:
> > - adapt to PGC_static
> > - remove #ifdef aroud function declaration
> > ---
> > v5 changes:
> > - In order to avoid stub functions, we #define PGC_staticmem to
> > non-zero only when CONFIG_STATIC_MEMORY
> > - use "unlikely()" around pg->count_info & PGC_staticmem
> > - remove pointless "if", since mark_page_free() is going to set
> > count_info to PGC_state_free and by consequence clear PGC_staticmem
> > - move #define PGC_staticmem 0 to mm.h
> > ---
> > v4 changes:
> > - no changes
> > ---
> > v3 changes:
> > - fix possible racy issue in free_staticmem_pages()
> > - introduce a stub free_staticmem_pages() for the
> > !CONFIG_STATIC_MEMORY case
> > - move the change to free_heap_pages() to cover other potential call
> > sites
> > - fix the indentation
> > ---
> > v2 changes:
> > - new commit
> > ---
> >   xen/arch/arm/include/asm/mm.h |  4 +++-
> >   xen/common/page_alloc.c       | 12 +++++++++---
> >   xen/include/xen/mm.h          |  2 --
> >   3 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/xen/arch/arm/include/asm/mm.h
> > b/xen/arch/arm/include/asm/mm.h index fbff11c468..7442893e77 100644
> > --- a/xen/arch/arm/include/asm/mm.h
> > +++ b/xen/arch/arm/include/asm/mm.h
> > @@ -108,9 +108,11 @@ struct page_info
> >     /* Page is Xen heap? */
> >   #define _PGC_xen_heap     PG_shift(2)
> >   #define PGC_xen_heap      PG_mask(1, 2)
> > -  /* Page is static memory */
> 
> NITpicking: You added this comment in patch #1 and now removing the space.
> Any reason to drop the space?
> 
> > +#ifdef CONFIG_STATIC_MEMORY
> 
> I think this change ought to be explained in the commit message. AFAIU, this is
> necessary to allow the compiler to remove code and avoid linking issues. Is
> that correct?
> 
> > +/* Page is static memory */
> >   #define _PGC_static    PG_shift(3)
> >   #define PGC_static     PG_mask(1, 3)
> > +#endif
> >   /* ... */
> >   /* Page is broken? */
> >   #define _PGC_broken       PG_shift(7)
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
> > 9e5c757847..6876869fa6 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -1443,6 +1443,13 @@ static void free_heap_pages(
> >
> >       ASSERT(order <= MAX_ORDER);
> >
> > +    if ( unlikely(pg->count_info & PGC_static) )
> > +    {
> > +        /* Pages of static memory shall not go back to the heap. */
> > +        free_staticmem_pages(pg, 1UL << order, need_scrub);
> I can't remember whether I asked this before (I couldn't find a thread).
> 
> free_staticmem_pages() doesn't seem to be protected by any lock. So how do
> you prevent the concurrent access to the page info with the acquire part?

True, last time you suggested that rsv_page_list needs to be protected with a
spinlock (mostly like d->page_alloc_lock). I haven't thought it thoroughly, sorry
about that.
So for freeing part, I shall get the lock at arch_free_heap_page(), where we insert
the page to the rsv_page_list, and release the lock at the end of the free_staticmem_page.
And for acquiring part, I've already put the lock around 
page = page_list_remove_head(&d->resv_page_list);

> Cheers,
> 
> --
> Julien Grall
Julien Grall June 9, 2022, 9:21 a.m. UTC | #4
Hi,

On 09/06/2022 06:54, Penny Zheng wrote:
> 
> 
>> -----Original Message-----
>> From: Julien Grall <julien@xen.org>
>> Sent: Tuesday, June 7, 2022 5:13 PM
>> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org
>> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
>> <sstabellini@kernel.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
>> <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>;
>> Jan Beulich <jbeulich@suse.com>; Wei Liu <wl@xen.org>
>> Subject: Re: [PATCH v6 2/9] xen: do not free reserved memory into heap
>>
>> Hi Penny,
>>
> 
> Hi Julien
> 
>> On 07/06/2022 08:30, Penny Zheng wrote:
>>> Pages used as guest RAM for static domain, shall be reserved to this
>>> domain only.
>>> So in case reserved pages being used for other purpose, users shall
>>> not free them back to heap, even when last ref gets dropped.
>>>
>>> free_staticmem_pages will be called by free_heap_pages in runtime for
>>> static domain freeing memory resource, so let's drop the __init flag.
>>>
>>> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
>>> ---
>>> v6 changes:
>>> - adapt to PGC_static
>>> - remove #ifdef aroud function declaration
>>> ---
>>> v5 changes:
>>> - In order to avoid stub functions, we #define PGC_staticmem to
>>> non-zero only when CONFIG_STATIC_MEMORY
>>> - use "unlikely()" around pg->count_info & PGC_staticmem
>>> - remove pointless "if", since mark_page_free() is going to set
>>> count_info to PGC_state_free and by consequence clear PGC_staticmem
>>> - move #define PGC_staticmem 0 to mm.h
>>> ---
>>> v4 changes:
>>> - no changes
>>> ---
>>> v3 changes:
>>> - fix possible racy issue in free_staticmem_pages()
>>> - introduce a stub free_staticmem_pages() for the
>>> !CONFIG_STATIC_MEMORY case
>>> - move the change to free_heap_pages() to cover other potential call
>>> sites
>>> - fix the indentation
>>> ---
>>> v2 changes:
>>> - new commit
>>> ---
>>>    xen/arch/arm/include/asm/mm.h |  4 +++-
>>>    xen/common/page_alloc.c       | 12 +++++++++---
>>>    xen/include/xen/mm.h          |  2 --
>>>    3 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/include/asm/mm.h
>>> b/xen/arch/arm/include/asm/mm.h index fbff11c468..7442893e77 100644
>>> --- a/xen/arch/arm/include/asm/mm.h
>>> +++ b/xen/arch/arm/include/asm/mm.h
>>> @@ -108,9 +108,11 @@ struct page_info
>>>      /* Page is Xen heap? */
>>>    #define _PGC_xen_heap     PG_shift(2)
>>>    #define PGC_xen_heap      PG_mask(1, 2)
>>> -  /* Page is static memory */
>>
>> NITpicking: You added this comment in patch #1 and now removing the space.
>> Any reason to drop the space?
>>
>>> +#ifdef CONFIG_STATIC_MEMORY
>>
>> I think this change ought to be explained in the commit message. AFAIU, this is
>> necessary to allow the compiler to remove code and avoid linking issues. Is
>> that correct?
>>
>>> +/* Page is static memory */
>>>    #define _PGC_static    PG_shift(3)
>>>    #define PGC_static     PG_mask(1, 3)
>>> +#endif
>>>    /* ... */
>>>    /* Page is broken? */
>>>    #define _PGC_broken       PG_shift(7)
>>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
>>> 9e5c757847..6876869fa6 100644
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -1443,6 +1443,13 @@ static void free_heap_pages(
>>>
>>>        ASSERT(order <= MAX_ORDER);
>>>
>>> +    if ( unlikely(pg->count_info & PGC_static) )
>>> +    {
>>> +        /* Pages of static memory shall not go back to the heap. */
>>> +        free_staticmem_pages(pg, 1UL << order, need_scrub);
>> I can't remember whether I asked this before (I couldn't find a thread).
>>
>> free_staticmem_pages() doesn't seem to be protected by any lock. So how do
>> you prevent the concurrent access to the page info with the acquire part?
> 
> True, last time you suggested that rsv_page_list needs to be protected with a
> spinlock (mostly like d->page_alloc_lock). I haven't thought it thoroughly, sorry
> about that.
> So for freeing part, I shall get the lock at arch_free_heap_page(), where we insert
> the page to the rsv_page_list, and release the lock at the end of the free_staticmem_page.

In general, a lock is better to be lock/unlock in the same function 
because it is easier to verify. However, I am not sure that extending 
the locking from d->page_alloc_lock up after free_heap_pages() is right.

The first reason being that they are other callers of free_heap_pages(). 
So now all the callers of the helpers would need to know whether they 
need to help d->page_alloc_lock.

Secondly, free_staticmem_pages() is meant to be the reverse of 
prepare_staticmem_pages(). We should prevent both of them to be called 
concurrently. It sounds strange to use the d->page_alloc_lock to protect 
it (a page is technically not belonging to a domain at this point).

To me it looks like we want to add the pages on the rsv_page_list 
*after* the pages have been freed. So we know that all the pages on that 
list have been marked as freed (i.e. free_staticmem_pages() completed).

In addition to that, we would need the code in free_staticmem_pages() to 
be protected by the heap_lock (at least so it is match 
prepare_staticmem_pages()).

Any thoughts?

Cheers,
Penny Zheng June 13, 2022, 3:27 a.m. UTC | #5
Hi Julien

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: Thursday, June 9, 2022 5:22 PM
> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org
> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>;
> Jan Beulich <jbeulich@suse.com>; Wei Liu <wl@xen.org>
> Subject: Re: [PATCH v6 2/9] xen: do not free reserved memory into heap
> 
> Hi,
> 
> On 09/06/2022 06:54, Penny Zheng wrote:
> >
> >
> >> -----Original Message-----
> >> From: Julien Grall <julien@xen.org>
> >> Sent: Tuesday, June 7, 2022 5:13 PM
> >> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org
> >> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
> >> <sstabellini@kernel.org>; Bertrand Marquis
> >> <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> >> <Volodymyr_Babchuk@epam.com>; Andrew Cooper
> >> <andrew.cooper3@citrix.com>; George Dunlap
> >> <george.dunlap@citrix.com>; Jan Beulich <jbeulich@suse.com>; Wei Liu
> >> <wl@xen.org>
> >> Subject: Re: [PATCH v6 2/9] xen: do not free reserved memory into
> >> heap
> >>
> >> Hi Penny,
> >>
> >
> > Hi Julien
> >
> >> On 07/06/2022 08:30, Penny Zheng wrote:
> >>> Pages used as guest RAM for static domain, shall be reserved to this
> >>> domain only.
> >>> So in case reserved pages being used for other purpose, users shall
> >>> not free them back to heap, even when last ref gets dropped.
> >>>
> >>> free_staticmem_pages will be called by free_heap_pages in runtime
> >>> for static domain freeing memory resource, so let's drop the __init flag.
> >>>
> >>> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> >>> ---
> >>> v6 changes:
> >>> - adapt to PGC_static
> >>> - remove #ifdef aroud function declaration
> >>> ---
> >>> v5 changes:
> >>> - In order to avoid stub functions, we #define PGC_staticmem to
> >>> non-zero only when CONFIG_STATIC_MEMORY
> >>> - use "unlikely()" around pg->count_info & PGC_staticmem
> >>> - remove pointless "if", since mark_page_free() is going to set
> >>> count_info to PGC_state_free and by consequence clear PGC_staticmem
> >>> - move #define PGC_staticmem 0 to mm.h
> >>> ---
> >>> v4 changes:
> >>> - no changes
> >>> ---
> >>> v3 changes:
> >>> - fix possible racy issue in free_staticmem_pages()
> >>> - introduce a stub free_staticmem_pages() for the
> >>> !CONFIG_STATIC_MEMORY case
> >>> - move the change to free_heap_pages() to cover other potential call
> >>> sites
> >>> - fix the indentation
> >>> ---
> >>> v2 changes:
> >>> - new commit
> >>> ---
> >>>    xen/arch/arm/include/asm/mm.h |  4 +++-
> >>>    xen/common/page_alloc.c       | 12 +++++++++---
> >>>    xen/include/xen/mm.h          |  2 --
> >>>    3 files changed, 12 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/include/asm/mm.h
> >>> b/xen/arch/arm/include/asm/mm.h index fbff11c468..7442893e77 100644
> >>> --- a/xen/arch/arm/include/asm/mm.h
> >>> +++ b/xen/arch/arm/include/asm/mm.h
> >>> @@ -108,9 +108,11 @@ struct page_info
> >>>      /* Page is Xen heap? */
> >>>    #define _PGC_xen_heap     PG_shift(2)
> >>>    #define PGC_xen_heap      PG_mask(1, 2)
> >>> -  /* Page is static memory */
> >>
> >> NITpicking: You added this comment in patch #1 and now removing the
> space.
> >> Any reason to drop the space?
> >>
> >>> +#ifdef CONFIG_STATIC_MEMORY
> >>
> >> I think this change ought to be explained in the commit message.
> >> AFAIU, this is necessary to allow the compiler to remove code and
> >> avoid linking issues. Is that correct?
> >>
> >>> +/* Page is static memory */
> >>>    #define _PGC_static    PG_shift(3)
> >>>    #define PGC_static     PG_mask(1, 3)
> >>> +#endif
> >>>    /* ... */
> >>>    /* Page is broken? */
> >>>    #define _PGC_broken       PG_shift(7)
> >>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
> >>> 9e5c757847..6876869fa6 100644
> >>> --- a/xen/common/page_alloc.c
> >>> +++ b/xen/common/page_alloc.c
> >>> @@ -1443,6 +1443,13 @@ static void free_heap_pages(
> >>>
> >>>        ASSERT(order <= MAX_ORDER);
> >>>
> >>> +    if ( unlikely(pg->count_info & PGC_static) )
> >>> +    {
> >>> +        /* Pages of static memory shall not go back to the heap. */
> >>> +        free_staticmem_pages(pg, 1UL << order, need_scrub);
> >> I can't remember whether I asked this before (I couldn't find a thread).
> >>
> >> free_staticmem_pages() doesn't seem to be protected by any lock. So
> >> how do you prevent the concurrent access to the page info with the acquire
> part?
> >
> > True, last time you suggested that rsv_page_list needs to be protected
> > with a spinlock (mostly like d->page_alloc_lock). I haven't thought it
> > thoroughly, sorry about that.
> > So for freeing part, I shall get the lock at arch_free_heap_page(),
> > where we insert the page to the rsv_page_list, and release the lock at the
> end of the free_staticmem_page.
> 
> In general, a lock is better to be lock/unlock in the same function because it is
> easier to verify. However, I am not sure that extending the locking from d-
> >page_alloc_lock up after free_heap_pages() is right.
> 
> The first reason being that they are other callers of free_heap_pages().
> So now all the callers of the helpers would need to know whether they need to
> help d->page_alloc_lock.
> 
> Secondly, free_staticmem_pages() is meant to be the reverse of
> prepare_staticmem_pages(). We should prevent both of them to be called
> concurrently. It sounds strange to use the d->page_alloc_lock to protect it (a
> page is technically not belonging to a domain at this point).
> 
> To me it looks like we want to add the pages on the rsv_page_list
> *after* the pages have been freed. So we know that all the pages on that list
> have been marked as freed (i.e. free_staticmem_pages() completed).
> 

Yes! That fixes everything!
If we add the pages on the rsv_page_list *after* the pages have been freed, we
could make sure that page acquired from rsv_page_list has been totally freed.
Hmmm, so in such case, do we still need to add lock here? 
We already could make sure that page acquired from rsv_page_list must be totally
freed, without the lock.
Or in facts, we use the lock to let prepare_staticmem_pages() not fail if free_staticmem_pages
happen concurrently?
Trying to understand the intents of the lock here more clearly, hope it's not a dumb question~

`> In addition to that, we would need the code in free_staticmem_pages() to be
> protected by the heap_lock (at least so it is match
> prepare_staticmem_pages()).
> 
> Any thoughts?
> 
> Cheers,
> 
> --
> Julien Grall
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index fbff11c468..7442893e77 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -108,9 +108,11 @@  struct page_info
   /* Page is Xen heap? */
 #define _PGC_xen_heap     PG_shift(2)
 #define PGC_xen_heap      PG_mask(1, 2)
-  /* Page is static memory */
+#ifdef CONFIG_STATIC_MEMORY
+/* Page is static memory */
 #define _PGC_static    PG_shift(3)
 #define PGC_static     PG_mask(1, 3)
+#endif
 /* ... */
 /* Page is broken? */
 #define _PGC_broken       PG_shift(7)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 9e5c757847..6876869fa6 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1443,6 +1443,13 @@  static void free_heap_pages(
 
     ASSERT(order <= MAX_ORDER);
 
+    if ( unlikely(pg->count_info & PGC_static) )
+    {
+        /* Pages of static memory shall not go back to the heap. */
+        free_staticmem_pages(pg, 1UL << order, need_scrub);
+        return;
+    }
+
     spin_lock(&heap_lock);
 
     for ( i = 0; i < (1 << order); i++ )
@@ -2636,8 +2643,8 @@  struct domain *get_pg_owner(domid_t domid)
 
 #ifdef CONFIG_STATIC_MEMORY
 /* Equivalent of free_heap_pages to free nr_mfns pages of static memory. */
-void __init free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
-                                 bool need_scrub)
+void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
+                          bool need_scrub)
 {
     mfn_t mfn = page_to_mfn(pg);
     unsigned long i;
@@ -2652,7 +2659,6 @@  void __init free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
             scrub_one_page(pg);
         }
 
-        /* In case initializing page of static memory, mark it PGC_static. */
         pg[i].count_info |= PGC_static;
     }
 }
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 3be754da92..1c4ddb336b 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -85,13 +85,11 @@  bool scrub_free_pages(void);
 } while ( false )
 #define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
 
-#ifdef CONFIG_STATIC_MEMORY
 /* These functions are for static memory */
 void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
                           bool need_scrub);
 int acquire_domstatic_pages(struct domain *d, mfn_t smfn, unsigned int nr_mfns,
                             unsigned int memflags);
-#endif
 
 /* Map machine page range in Xen virtual address space. */
 int map_pages_to_xen(