Introduce an extra check and comment to ensure the outer caller has
possibly taken an extra reference on the foreign page that's about to be
removed from the p2m.  Otherwise the put_page() in p2m_entry_modify() won't
be safe to do ahead of the entry being removed form the p2m and any cached
states purged.

While there also replace the error codes for unreachable paths to use
EILSEQ.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/include/asm/p2m.h | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index ef6b02ff0bb6..1ceb248b9da6 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -1066,7 +1066,7 @@ static inline int p2m_entry_modify(struct p2m_domain *p2m, p2m_type_t nt,
         if ( !mfn_valid(nfn) || p2m != p2m_get_hostp2m(p2m->domain) )
         {
             ASSERT_UNREACHABLE();
-            return -EINVAL;
+            return -EILSEQ;
         }
 
         if ( !page_get_owner_and_reference(mfn_to_page(nfn)) )
@@ -1088,14 +1088,26 @@ static inline int p2m_entry_modify(struct p2m_domain *p2m, p2m_type_t nt,
         break;
 
     case p2m_map_foreign:
-        if ( !mfn_valid(ofn) || p2m != p2m_get_hostp2m(p2m->domain) )
+    {
+        struct page_info *pg = mfn_valid(ofn) ? mfn_to_page(ofn) : NULL;
+        unsigned long ci = pg ? ACCESS_ONCE(pg->count_info) : 0;
+
+        if ( !pg || p2m != p2m_get_hostp2m(p2m->domain) ||
+             /*
+              * Rely on the caller also holding a reference to the page, so
+              * that the put_page() below doesn't cause the page to be
+              * freed, as it still has to be removed from the p2m.
+              */
+             (ci & PGC_count_mask) <= (ci & PGC_allocated ? 2 : 1) ||
+             !p2m->nr_foreign )
         {
             ASSERT_UNREACHABLE();
-            return -EINVAL;
+            return -EILSEQ;
         }
-        put_page(mfn_to_page(ofn));
+        put_page(pg);
         p2m->nr_foreign--;
         break;
+    }
 
     default:
         break;
-- 
2.51.0

On 20.02.2026 11:05, Roger Pau Monne wrote:
> Introduce an extra check and comment to ensure the outer caller has
> possibly taken an extra reference on the foreign page that's about to be
> removed from the p2m.  Otherwise the put_page() in p2m_entry_modify() won't
> be safe to do ahead of the entry being removed form the p2m and any cached
> states purged.
> 
> While there also replace the error codes for unreachable paths to use
> EILSEQ.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> --- a/xen/arch/x86/include/asm/p2m.h
> +++ b/xen/arch/x86/include/asm/p2m.h
> @@ -1066,7 +1066,7 @@ static inline int p2m_entry_modify(struct p2m_domain *p2m, p2m_type_t nt,
>          if ( !mfn_valid(nfn) || p2m != p2m_get_hostp2m(p2m->domain) )
>          {
>              ASSERT_UNREACHABLE();
> -            return -EINVAL;
> +            return -EILSEQ;
>          }
>  
>          if ( !page_get_owner_and_reference(mfn_to_page(nfn)) )
> @@ -1088,14 +1088,26 @@ static inline int p2m_entry_modify(struct p2m_domain *p2m, p2m_type_t nt,
>          break;
>  
>      case p2m_map_foreign:
> -        if ( !mfn_valid(ofn) || p2m != p2m_get_hostp2m(p2m->domain) )
> +    {
> +        struct page_info *pg = mfn_valid(ofn) ? mfn_to_page(ofn) : NULL;
> +        unsigned long ci = pg ? ACCESS_ONCE(pg->count_info) : 0;
> +
> +        if ( !pg || p2m != p2m_get_hostp2m(p2m->domain) ||
> +             /*
> +              * Rely on the caller also holding a reference to the page, so
> +              * that the put_page() below doesn't cause the page to be
> +              * freed, as it still has to be removed from the p2m.
> +              */
> +             (ci & PGC_count_mask) <= (ci & PGC_allocated ? 2 : 1) ||
> +             !p2m->nr_foreign )
>          {
>              ASSERT_UNREACHABLE();
> -            return -EINVAL;
> +            return -EILSEQ;
>          }
> -        put_page(mfn_to_page(ofn));
> +        put_page(pg);
>          p2m->nr_foreign--;
>          break;
> +    }
>  
>      default:
>          break;

Unrelated to the particular change here, I wonder whether it's about time to
out-of-line this ever-growing function.

Jan