From: Petr Beneš <w1benny@gmail.com>

Commit 7e5b662 fixed p2m_altp2m_get_or_propagate() to use the altp2m's
default_access when propagating entries from the host p2m. However, the same
fix was not applied to altp2m_get_effective_entry(), which has the same issue.

When altp2m_get_effective_entry() prepopulates a superpage from the host
p2m, it incorrectly uses the host p2m's access permissions instead of
the altp2m's default_access. This causes problems when the superpage is
later split (e.g., when setting mem_access on a specific 4K page): all
512 entries inherit the host p2m's access rights instead of the altp2m's
default_access.

This issue became apparent after commit 50baf2d, which causes the host p2m
to use superpages more frequently. Before that commit, the host p2m
typically had 4K entries after VM restore, so the prepopulate branch was
rarely taken.

Symptoms include memory-access events firing for unexpected pages when
using VMI tools with altp2m, particularly after VM resume.
The issue can be worked around by booting with "hap_1gb=0 hap_2mb=0".

Fixes: 7e5b662 ("x86/altp2m: p2m_altp2m_get_or_propagate() should honor ap2m->default_access")
Signed-off-by: Petr Beneš <w1benny@gmail.com>
---
 xen/arch/x86/mm/altp2m.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index 0261360aae..0bc9b9ad2f 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -194,6 +194,9 @@ int altp2m_get_effective_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
             gfn_t gfn_aligned = _gfn(gfn_x(gfn) & mask);
             mfn_t mfn_aligned = _mfn(mfn_x(*mfn) & mask);
 
+            /* Override the altp2m entry with its default access. */
+            *a = ap2m->default_access;
+
             rc = ap2m->set_entry(ap2m, gfn_aligned, mfn_aligned, page_order, *t, *a, 1);
             if ( rc )
                 return rc;
-- 
2.34.1

On 09.01.2026 19:28, Petr Beneš wrote:
> From: Petr Beneš <w1benny@gmail.com>
> 
> Commit 7e5b662 fixed p2m_altp2m_get_or_propagate() to use the altp2m's
> default_access when propagating entries from the host p2m. However, the same
> fix was not applied to altp2m_get_effective_entry(), which has the same issue.
> 
> When altp2m_get_effective_entry() prepopulates a superpage from the host
> p2m, it incorrectly uses the host p2m's access permissions instead of
> the altp2m's default_access. This causes problems when the superpage is
> later split (e.g., when setting mem_access on a specific 4K page): all
> 512 entries inherit the host p2m's access rights instead of the altp2m's
> default_access.
> 
> This issue became apparent after commit 50baf2d, which causes the host p2m
> to use superpages more frequently. Before that commit, the host p2m
> typically had 4K entries after VM restore, so the prepopulate branch was
> rarely taken.
> 
> Symptoms include memory-access events firing for unexpected pages when
> using VMI tools with altp2m, particularly after VM resume.
> The issue can be worked around by booting with "hap_1gb=0 hap_2mb=0".
> 
> Fixes: 7e5b662 ("x86/altp2m: p2m_altp2m_get_or_propagate() should honor ap2m->default_access")

You didn't even Cc Tamas, who I think once again will need to ack this.
Already with the referenced change I didn't quite understand the
reasoning.

However, two formal points: Please use 12-digit hashes, as demanded by
sending-patches.pandoc. Plus I don't think Fixes: is quite right here.
That earlier change of yours didn't mean to do more than it did, by its
title and description. We relatively recently introduced Amends:, which
may be a suitable fit here.

Jan

On 12/01/2026 11:09 am, Jan Beulich wrote:
> On 09.01.2026 19:28, Petr Beneš wrote:
>> From: Petr Beneš <w1benny@gmail.com>
>>
>> Commit 7e5b662 fixed p2m_altp2m_get_or_propagate() to use the altp2m's
>> default_access when propagating entries from the host p2m. However, the same
>> fix was not applied to altp2m_get_effective_entry(), which has the same issue.
>>
>> When altp2m_get_effective_entry() prepopulates a superpage from the host
>> p2m, it incorrectly uses the host p2m's access permissions instead of
>> the altp2m's default_access. This causes problems when the superpage is
>> later split (e.g., when setting mem_access on a specific 4K page): all
>> 512 entries inherit the host p2m's access rights instead of the altp2m's
>> default_access.
>>
>> This issue became apparent after commit 50baf2d, which causes the host p2m
>> to use superpages more frequently. Before that commit, the host p2m
>> typically had 4K entries after VM restore, so the prepopulate branch was
>> rarely taken.
>>
>> Symptoms include memory-access events firing for unexpected pages when
>> using VMI tools with altp2m, particularly after VM resume.
>> The issue can be worked around by booting with "hap_1gb=0 hap_2mb=0".
>>
>> Fixes: 7e5b662 ("x86/altp2m: p2m_altp2m_get_or_propagate() should honor ap2m->default_access")
> You didn't even Cc Tamas, who I think once again will need to ack this.
> Already with the referenced change I didn't quite understand the
> reasoning.
>
> However, two formal points: Please use 12-digit hashes, as demanded by
> sending-patches.pandoc. Plus I don't think Fixes: is quite right here.
> That earlier change of yours didn't mean to do more than it did, by its
> title and description. We relatively recently introduced Amends:, which
> may be a suitable fit here.

I beg your pardon?  Fixes are and Amends are synonyms.  You cannot use
them like this, and you absolutely cannot expect contributors to know
your personal interpretation of the words.

~Andrew

On 12.01.2026 12:18, Andrew Cooper wrote:
> On 12/01/2026 11:09 am, Jan Beulich wrote:
>> On 09.01.2026 19:28, Petr Beneš wrote:
>>> From: Petr Beneš <w1benny@gmail.com>
>>>
>>> Commit 7e5b662 fixed p2m_altp2m_get_or_propagate() to use the altp2m's
>>> default_access when propagating entries from the host p2m. However, the same
>>> fix was not applied to altp2m_get_effective_entry(), which has the same issue.
>>>
>>> When altp2m_get_effective_entry() prepopulates a superpage from the host
>>> p2m, it incorrectly uses the host p2m's access permissions instead of
>>> the altp2m's default_access. This causes problems when the superpage is
>>> later split (e.g., when setting mem_access on a specific 4K page): all
>>> 512 entries inherit the host p2m's access rights instead of the altp2m's
>>> default_access.
>>>
>>> This issue became apparent after commit 50baf2d, which causes the host p2m
>>> to use superpages more frequently. Before that commit, the host p2m
>>> typically had 4K entries after VM restore, so the prepopulate branch was
>>> rarely taken.
>>>
>>> Symptoms include memory-access events firing for unexpected pages when
>>> using VMI tools with altp2m, particularly after VM resume.
>>> The issue can be worked around by booting with "hap_1gb=0 hap_2mb=0".
>>>
>>> Fixes: 7e5b662 ("x86/altp2m: p2m_altp2m_get_or_propagate() should honor ap2m->default_access")
>> You didn't even Cc Tamas, who I think once again will need to ack this.
>> Already with the referenced change I didn't quite understand the
>> reasoning.
>>
>> However, two formal points: Please use 12-digit hashes, as demanded by
>> sending-patches.pandoc. Plus I don't think Fixes: is quite right here.
>> That earlier change of yours didn't mean to do more than it did, by its
>> title and description. We relatively recently introduced Amends:, which
>> may be a suitable fit here.
> 
> I beg your pardon?  Fixes are and Amends are synonyms.

This is news to me. To me a "fix" addresses a bug in the referenced commit.
Whereas making a related change which isn't strictly a bugfix to the
referenced earlier change is what Amends: was introduced for. If both were
synonyms, why would you not have objected to the introduction of Amends:?

>  You cannot use
> them like this, and you absolutely cannot expect contributors to know
> your personal interpretation of the words.

"My personal interpretation of the words" has become the community's with
the committing of the change introducing Amends:. And I think I can expect
contributors to read sending-patches.pandoc?

Jan

On 12.01.2026 12:24, Jan Beulich wrote:
> On 12.01.2026 12:18, Andrew Cooper wrote:
>> On 12/01/2026 11:09 am, Jan Beulich wrote:
>>> On 09.01.2026 19:28, Petr Beneš wrote:
>>>> From: Petr Beneš <w1benny@gmail.com>
>>>>
>>>> Commit 7e5b662 fixed p2m_altp2m_get_or_propagate() to use the altp2m's
>>>> default_access when propagating entries from the host p2m. However, the same
>>>> fix was not applied to altp2m_get_effective_entry(), which has the same issue.
>>>>
>>>> When altp2m_get_effective_entry() prepopulates a superpage from the host
>>>> p2m, it incorrectly uses the host p2m's access permissions instead of
>>>> the altp2m's default_access. This causes problems when the superpage is
>>>> later split (e.g., when setting mem_access on a specific 4K page): all
>>>> 512 entries inherit the host p2m's access rights instead of the altp2m's
>>>> default_access.
>>>>
>>>> This issue became apparent after commit 50baf2d, which causes the host p2m
>>>> to use superpages more frequently. Before that commit, the host p2m
>>>> typically had 4K entries after VM restore, so the prepopulate branch was
>>>> rarely taken.
>>>>
>>>> Symptoms include memory-access events firing for unexpected pages when
>>>> using VMI tools with altp2m, particularly after VM resume.
>>>> The issue can be worked around by booting with "hap_1gb=0 hap_2mb=0".
>>>>
>>>> Fixes: 7e5b662 ("x86/altp2m: p2m_altp2m_get_or_propagate() should honor ap2m->default_access")
>>> You didn't even Cc Tamas, who I think once again will need to ack this.
>>> Already with the referenced change I didn't quite understand the
>>> reasoning.
>>>
>>> However, two formal points: Please use 12-digit hashes, as demanded by
>>> sending-patches.pandoc. Plus I don't think Fixes: is quite right here.
>>> That earlier change of yours didn't mean to do more than it did, by its
>>> title and description. We relatively recently introduced Amends:, which
>>> may be a suitable fit here.
>>
>> I beg your pardon?  Fixes are and Amends are synonyms.
> 
> This is news to me. To me a "fix" addresses a bug in the referenced commit.
> Whereas making a related change which isn't strictly a bugfix to the
> referenced earlier change is what Amends: was introduced for. If both were
> synonyms, why would you not have objected to the introduction of Amends:?

Having thought about this some more, would Complements: or Supplements:
possibly better suit you?

Jan

On Mon, Jan 12, 2026 at 12:24 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.01.2026 12:18, Andrew Cooper wrote:
> > On 12/01/2026 11:09 am, Jan Beulich wrote:
> >> You didn't even Cc Tamas, who I think once again will need to ack this.

I _considered_ Ccing Tamas when I saw add_maintainers.pl didn't add
him. I couldn't remember if I added him manually at that time or not.
However, in the end, I just trusted the add_maintainers.pl and didn't
modify it.

> >> However, two formal points: Please use 12-digit hashes, as demanded by
> >> sending-patches.pandoc.

I apologize. This is the first time I'm learning of the existence of
sending-patches.pandoc. Up until now I was following the
"Submitting_Xen_Project_Patches" wiki page where nothing about how
many characters of the hash should be included. I'll keep that in
mind.

> >> That earlier change of yours didn't mean to do more than it did, by its
> >> title and description.

True. Had I known at that time that the fix is needed to be applied in
multiple places, the commit message would look different.

> >> We relatively recently introduced Amends:, which
> >> may be a suitable fit here.

So, what should be my next steps? Should I re-send the patch where I
Cc Tamas, use 12-char hashes and replace Fixes with Amends?

P.

On 12.01.2026 13:09, Petr Beneš wrote:
> On Mon, Jan 12, 2026 at 12:24 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 12.01.2026 12:18, Andrew Cooper wrote:
>>> On 12/01/2026 11:09 am, Jan Beulich wrote:
>>>> We relatively recently introduced Amends:, which
>>>> may be a suitable fit here.
> 
> So, what should be my next steps? Should I re-send the patch where I
> Cc Tamas, use 12-char hashes and replace Fixes with Amends?

I'd give Tamas a little bit of time to possibly comment, which might then
save one round of re-submission.

Jan

On Mon, Jan 12, 2026 at 7:12 AM Jan Beulich <jbeulich@suse.com> wrote:

> On 12.01.2026 13:09, Petr Beneš wrote:
> > On Mon, Jan 12, 2026 at 12:24 PM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 12.01.2026 12:18, Andrew Cooper wrote:
> >>> On 12/01/2026 11:09 am, Jan Beulich wrote:
> >>>> We relatively recently introduced Amends:, which
> >>>> may be a suitable fit here.
> >
> > So, what should be my next steps? Should I re-send the patch where I
> > Cc Tamas, use 12-char hashes and replace Fixes with Amends?
>
> I'd give Tamas a little bit of time to possibly comment, which might then
> save one round of re-submission.


Thanks for waiting for my review, appreciated!

Acked-by: Tamas K Lengyel <tamas@tklengyel.com>