[PATCH for-4.22(?) v2] gnttab: simplify (really: drop) gnttab_set_frame_gfn()

[PATCH for-4.22(?) v2] gnttab: simplify (really: drop) gnttab_set_frame_gfn()

[Jan Beulich]

It's not really doing anything for valid GFNs, which renders its one use
site pretty pointless. The other isn't so much about setting anything, but
rather about clearing.

The main point here, however, is about Rafal spotting the double
fetching of the GFN (first in gnttab_unpopulate_status_frames(), then
again in gnttab_set_frame_gfn()). Re-purpose the macro parameter to pass
in the already fetched GFN, while dropping the no longer used parameters.

As the result is a mere wrapper around guest_physmap_remove_page(), drop
the hook altogether.

Suggested-by: Rafal Wojtczuk <rafal.wojtczuk@7bulls.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Drop hook altogether.

--- a/xen/arch/arm/include/asm/grant_table.h
+++ b/xen/arch/arm/include/asm/grant_table.h
@@ -50,13 +50,6 @@ int replace_grant_host_mapping(uint64_t
 #define gnttab_dom0_frames()                                             \
     min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext - _stext))
 
-#define gnttab_set_frame_gfn(gt, st, idx, gfn, mfn)                      \
-    (gfn_eq(gfn, INVALID_GFN)                                            \
-     ? guest_physmap_remove_page((gt)->domain,                           \
-                                 gnttab_get_frame_gfn(gt, st, idx),      \
-                                 mfn, 0)                                 \
-     : 0)
-
 #define gnttab_get_frame_gfn(gt, st, idx) ({                             \
    (st) ? gnttab_status_gfn(NULL, gt, idx)                               \
         : gnttab_shared_gfn(NULL, gt, idx);                              \
--- a/xen/arch/x86/include/asm/grant_table.h
+++ b/xen/arch/x86/include/asm/grant_table.h
@@ -32,12 +32,6 @@ static inline int replace_grant_host_map
     return replace_grant_pv_mapping(addr, frame, new_addr, flags);
 }
 
-#define gnttab_set_frame_gfn(gt, st, idx, gfn, mfn)                      \
-    (gfn_eq(gfn, INVALID_GFN)                                            \
-     ? guest_physmap_remove_page((gt)->domain,                           \
-                                 gnttab_get_frame_gfn(gt, st, idx),      \
-                                 mfn, 0)                                 \
-     : 0 /* Handled in add_to_physmap_one(). */)
 #define gnttab_get_frame_gfn(gt, st, idx) ({                             \
     mfn_t mfn_ = (st) ? gnttab_status_mfn(gt, idx)                       \
                       : gnttab_shared_mfn(gt, idx);                      \
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1844,8 +1844,7 @@ gnttab_unpopulate_status_frames(struct d
         {
             int rc = gfn_eq(gfn, INVALID_GFN)
                      ? 0
-                     : gnttab_set_frame_gfn(gt, true, i, INVALID_GFN,
-                                            page_to_mfn(pg));
+                     : guest_physmap_remove_page(d, gfn, page_to_mfn(pg), 0);
 
             if ( rc )
             {
@@ -4285,8 +4284,6 @@ int gnttab_map_frame_begin(
          */
         if ( !get_page(pg, d) )
             rc = -EBUSY;
-        else if ( (rc = gnttab_set_frame_gfn(gt, status, idx, gfn, *mfn)) )
-            put_page(pg);
     }
 
     if ( rc )

Re: [PATCH for-4.22(?) v2] gnttab: simplify (really: drop) gnttab_set_frame_gfn()

[Andrew Cooper]

On 12/05/2026 3:46 pm, Jan Beulich wrote:
> It's not really doing anything for valid GFNs, which renders its one use
> site pretty pointless. The other isn't so much about setting anything, but
> rather about clearing.
>
> The main point here, however, is about Rafal spotting the double
> fetching of the GFN (first in gnttab_unpopulate_status_frames(), then
> again in gnttab_set_frame_gfn()). Re-purpose the macro parameter to pass
> in the already fetched GFN, while dropping the no longer used parameters.
>
> As the result is a mere wrapper around guest_physmap_remove_page(), drop
> the hook altogether.
>
> Suggested-by: Rafal Wojtczuk <rafal.wojtczuk@7bulls.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH for-4.22(?) v2] gnttab: simplify (really: drop) gnttab_set_frame_gfn()

[Oleksii Kurochko]

On 5/12/26 4:46 PM, Jan Beulich wrote:
> It's not really doing anything for valid GFNs, which renders its one use
> site pretty pointless. The other isn't so much about setting anything, but
> rather about clearing.
> 
> The main point here, however, is about Rafal spotting the double
> fetching of the GFN (first in gnttab_unpopulate_status_frames(), then
> again in gnttab_set_frame_gfn()). Re-purpose the macro parameter to pass
> in the already fetched GFN, while dropping the no longer used parameters.
> 
> As the result is a mere wrapper around guest_physmap_remove_page(), drop
> the hook altogether.
> 
> Suggested-by: Rafal Wojtczuk <rafal.wojtczuk@7bulls.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii