[v2,4/6] x86/mem-paging: add minimal lock order enforcement to p2m_mem_paging_prep()

Commit Message

Jan Beulich April 23, 2020, 8:38 a.m. UTC

While full checking is impossible (as the lock is being acquired/
released down the call tree), perform at least a lock level check.

Signed-off-by: Jan Beulich <[e-mail]>

Comments

Roger Pau Monne May 14, 2020, 4:25 p.m. UTC | #1

On Thu, Apr 23, 2020 at 10:38:44AM +0200, Jan Beulich wrote:
> While full checking is impossible (as the lock is being acquired/
> released down the call tree), perform at least a lock level check.

I'm slightly confused, doesn't alloc_domheap_page already have it's
own lock order checking?

Thanks, Roger.

Jan Beulich May 15, 2020, 9:46 a.m. UTC | #2

On 14.05.2020 18:25, Roger Pau Monné wrote:
> On Thu, Apr 23, 2020 at 10:38:44AM +0200, Jan Beulich wrote:
>> While full checking is impossible (as the lock is being acquired/
>> released down the call tree), perform at least a lock level check.
> 
> I'm slightly confused, doesn't alloc_domheap_page already have it's
> own lock order checking?

I don't see how it would, as it doesn't (and can't legitimately)
include arch/x86/mm/mm-locks.h. Also maybe this comment in the
header clarifies it:

/* Page alloc lock (per-domain)
 *
 * This is an external lock, not represented by an mm_lock_t. However,
 * pod code uses it in conjunction with the p2m lock, and expecting
 * the ordering which we enforce here.
 * The lock is not recursive. */

Jan

Roger Pau Monne May 15, 2020, 10:21 a.m. UTC | #3

On Fri, May 15, 2020 at 11:46:23AM +0200, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> 
> On 14.05.2020 18:25, Roger Pau Monné wrote:
> > On Thu, Apr 23, 2020 at 10:38:44AM +0200, Jan Beulich wrote:
> >> While full checking is impossible (as the lock is being acquired/
> >> released down the call tree), perform at least a lock level check.
> > 
> > I'm slightly confused, doesn't alloc_domheap_page already have it's
> > own lock order checking?
> 
> I don't see how it would, as it doesn't (and can't legitimately)
> include arch/x86/mm/mm-locks.h. Also maybe this comment in the
> header clarifies it:
> 
> /* Page alloc lock (per-domain)
>  *
>  * This is an external lock, not represented by an mm_lock_t. However,
>  * pod code uses it in conjunction with the p2m lock, and expecting
>  * the ordering which we enforce here.
>  * The lock is not recursive. */


Thanks.

Reviewed-by: Roger Pau Monné <[e-mail]>

Patch

--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1813,6 +1813,7 @@  int p2m_mem_paging_prep(struct domain *d
             goto out;
         /* Get a free page */
         ret = -ENOMEM;
+        page_alloc_mm_pre_lock(d);
         page = alloc_domheap_page(d, 0);
         if ( unlikely(page == NULL) )
             goto out;