[PATCH] x86/pod: fix decrease_reservation() clearing of M2P entries

[PATCH] x86/pod: fix decrease_reservation() clearing of M2P entries

[Roger Pau Monne]

Add the missing index increase in the M2P clearing loop, otherwise the loop
keeps pointlessly setting the same MFN entry repeatedly.  This seems to be
an oversight from the change that introduced support to process high order
pages in one go.

Fixes: 3c352011c0d3 ("x86/PoD: shorten certain operations on higher order ranges")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/mm/p2m-pod.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 05633fe2ac88..22dde913cc3c 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -655,7 +655,7 @@ decrease_reservation(struct domain *d, gfn_t gfn, unsigned int order)
             }
             p2m_tlb_flush_sync(p2m);
             for ( j = 0; j < n; ++j )
-                set_gpfn_from_mfn(mfn_x(mfn), INVALID_M2P_ENTRY);
+                set_gpfn_from_mfn(mfn_x(mfn) + j, INVALID_M2P_ENTRY);
             p2m_pod_cache_add(p2m, page, cur_order);
 
             ioreq_request_mapcache_invalidate(d);
-- 
2.51.0

Re: [PATCH] x86/pod: fix decrease_reservation() clearing of M2P entries

[Jan Beulich]

On 10.12.2025 10:35, Roger Pau Monne wrote:
> Add the missing index increase in the M2P clearing loop, otherwise the loop
> keeps pointlessly setting the same MFN entry repeatedly.  This seems to be
> an oversight from the change that introduced support to process high order
> pages in one go.

Ouch.

> Fixes: 3c352011c0d3 ("x86/PoD: shorten certain operations on higher order ranges")
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

I'd like to note that things were broken in the same way before that commit,
too, simply because the order wasn't taken into account all. (This is not a
request to change the Fixes: tag, though. It's just an observation.)

Jan

Re: [PATCH] x86/pod: fix decrease_reservation() clearing of M2P entries

[Roger Pau Monné]

On Wed, Dec 10, 2025 at 11:07:00AM +0100, Jan Beulich wrote:
> On 10.12.2025 10:35, Roger Pau Monne wrote:
> > Add the missing index increase in the M2P clearing loop, otherwise the loop
> > keeps pointlessly setting the same MFN entry repeatedly.  This seems to be
> > an oversight from the change that introduced support to process high order
> > pages in one go.
> 
> Ouch.
> 
> > Fixes: 3c352011c0d3 ("x86/PoD: shorten certain operations on higher order ranges")
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> I'd like to note that things were broken in the same way before that commit,
> too, simply because the order wasn't taken into account all. (This is not a
> request to change the Fixes: tag, though. It's just an observation.)

Are you sure?  Previous to that commit the order is not taken into
account, and each 4K page is processed independently: the `i` index is
strictly increased with +1 for each loop.

Thanks, Roger.

Re: [PATCH] x86/pod: fix decrease_reservation() clearing of M2P entries

[Jan Beulich]

On 10.12.2025 11:10, Roger Pau Monné wrote:
> On Wed, Dec 10, 2025 at 11:07:00AM +0100, Jan Beulich wrote:
>> On 10.12.2025 10:35, Roger Pau Monne wrote:
>>> Add the missing index increase in the M2P clearing loop, otherwise the loop
>>> keeps pointlessly setting the same MFN entry repeatedly.  This seems to be
>>> an oversight from the change that introduced support to process high order
>>> pages in one go.
>>
>> Ouch.
>>
>>> Fixes: 3c352011c0d3 ("x86/PoD: shorten certain operations on higher order ranges")
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>> I'd like to note that things were broken in the same way before that commit,
>> too, simply because the order wasn't taken into account all. (This is not a
>> request to change the Fixes: tag, though. It's just an observation.)
> 
> Are you sure?  Previous to that commit the order is not taken into
> account, and each 4K page is processed independently: the `i` index is
> strictly increased with +1 for each loop.

Indeed. No idea what I was thinking I was seeing.

Jan