stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
@ 2025-12-19  1:01 Ariadne Conill
  2025-12-19  3:56 ` Borislav Petkov
  2025-12-19 16:32 ` Teddy Astie
  0 siblings, 2 replies; 12+ messages in thread
From: Ariadne Conill @ 2025-12-19  1:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: mario.limonciello, darwi, sandipan.das, kai.huang, me,
	yazen.ghannam, riel, peterz, hpa, x86, tglx, mingo, bp,
	dave.hansen, Ariadne Conill, xen-devel, stable

Xen domU cannot access the given MMIO address for security reasons,
resulting in a failed hypercall in ioremap() due to permissions.

Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
Cc: xen-devel@lists.xenproject.org
Cc: stable@vger.kernel.org
---
 arch/x86/kernel/cpu/amd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index a6f88ca1a6b4..99308fba4d7d 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -29,6 +29,8 @@
 # include <asm/mmconfig.h>
 #endif
 
+#include <xen/xen.h>
+
 #include "cpu.h"
 
 u16 invlpgb_count_max __ro_after_init = 1;
@@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
 		return 0;
 
+	/* Xen PV domU cannot access hardware directly, so bail for domU case */
+	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
+		return 0;
+
 	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
 	if (!addr)
 		return 0;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19  1:01 [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU Ariadne Conill
@ 2025-12-19  3:56 ` Borislav Petkov
  2025-12-19 16:09   ` Sean Christopherson
  2025-12-19 16:32 ` Teddy Astie
  1 sibling, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2025-12-19  3:56 UTC (permalink / raw)
  To: Ariadne Conill, linux-kernel, seanjc
  Cc: mario.limonciello, darwi, sandipan.das, kai.huang, me,
	yazen.ghannam, riel, peterz, hpa, x86, tglx, mingo, dave.hansen,
	xen-devel, stable

On December 19, 2025 1:01:31 AM UTC, Ariadne Conill <ariadne@ariadne.space> wrote:
>Xen domU cannot access the given MMIO address for security reasons,
>resulting in a failed hypercall in ioremap() due to permissions.
>
>Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
>Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
>Cc: xen-devel@lists.xenproject.org
>Cc: stable@vger.kernel.org
>---
> arch/x86/kernel/cpu/amd.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>index a6f88ca1a6b4..99308fba4d7d 100644
>--- a/arch/x86/kernel/cpu/amd.c
>+++ b/arch/x86/kernel/cpu/amd.c
>@@ -29,6 +29,8 @@
> # include <asm/mmconfig.h>
> #endif
> 
>+#include <xen/xen.h>
>+
> #include "cpu.h"
> 
> u16 invlpgb_count_max __ro_after_init = 1;
>@@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
> 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
> 		return 0;
> 
>+	/* Xen PV domU cannot access hardware directly, so bail for domU case */
>+	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
>+		return 0;
>+
> 	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
> 	if (!addr)
> 		return 0;

Sean, looka here. The other hypervisor wants other checks.

Time to whip out the X86_FEATURE_HYPERVISOR check.
-- 
Small device. Typos and formatting crap

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19  3:56 ` Borislav Petkov
@ 2025-12-19 16:09   ` Sean Christopherson
  2025-12-19 16:26     ` Andrew Cooper
                       ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Sean Christopherson @ 2025-12-19 16:09 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ariadne Conill, linux-kernel, mario.limonciello, darwi,
	sandipan.das, kai.huang, me, yazen.ghannam, riel, peterz, hpa,
	x86, tglx, mingo, dave.hansen, xen-devel, stable

On Fri, Dec 19, 2025, Borislav Petkov wrote:
> On December 19, 2025 1:01:31 AM UTC, Ariadne Conill <ariadne@ariadne.space> wrote:
> >Xen domU cannot access the given MMIO address for security reasons,
> >resulting in a failed hypercall in ioremap() due to permissions.

Why does that matter though?  Ah, because set_pte() assumes success, and so
presumably the failed hypercall goes unnoticed and trying to access the MMIO
#PFs due to !PRESENT mapping.

> >Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
> >Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
> >Cc: xen-devel@lists.xenproject.org
> >Cc: stable@vger.kernel.org
> >---
> > arch/x86/kernel/cpu/amd.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> >diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> >index a6f88ca1a6b4..99308fba4d7d 100644
> >--- a/arch/x86/kernel/cpu/amd.c
> >+++ b/arch/x86/kernel/cpu/amd.c
> >@@ -29,6 +29,8 @@
> > # include <asm/mmconfig.h>
> > #endif
> > 
> >+#include <xen/xen.h>
> >+
> > #include "cpu.h"
> > 
> > u16 invlpgb_count_max __ro_after_init = 1;
> >@@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
> > 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
> > 		return 0;
> > 
> >+	/* Xen PV domU cannot access hardware directly, so bail for domU case */

Heh, Xen on Zen crime.

> >+	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
> >+		return 0;
> >+
> > 	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
> > 	if (!addr)
> > 		return 0;
> 
> Sean, looka here. The other hypervisor wants other checks.
>
> Time to whip out the X86_FEATURE_HYPERVISOR check.

LOL, Ariadne, be honest, how much did Boris pay you?  :-D

Jokes aside, I suppose I'm fine adding a HYPERVISOR check, but at the same time,
how is this not a Xen bug?  Refusing to create a mapping because the VM doesn't
have a device defined at a given GPA is pretty hostile behavior for a hypervisor.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19 16:09   ` Sean Christopherson
@ 2025-12-19 16:26     ` Andrew Cooper
  2025-12-19 17:36       ` Sean Christopherson
  2025-12-19 23:16     ` Borislav Petkov
  2025-12-19 23:19     ` Ariadne Conill
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2025-12-19 16:26 UTC (permalink / raw)
  To: Sean Christopherson, Borislav Petkov
  Cc: Andrew Cooper, Ariadne Conill, linux-kernel, mario.limonciello,
	darwi, sandipan.das, kai.huang, me, yazen.ghannam, riel, peterz,
	hpa, x86, tglx, mingo, dave.hansen, xen-devel, stable

On 19/12/2025 4:09 pm, Sean Christopherson wrote:
> On Fri, Dec 19, 2025, Borislav Petkov wrote:
>> On December 19, 2025 1:01:31 AM UTC, Ariadne Conill <ariadne@ariadne.space> wrote:
>>> Xen domU cannot access the given MMIO address for security reasons,
>>> resulting in a failed hypercall in ioremap() due to permissions.
> Why does that matter though?  Ah, because set_pte() assumes success, and so
> presumably the failed hypercall goes unnoticed and trying to access the MMIO
> #PFs due to !PRESENT mapping.
>
>>> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
>>> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
>>> Cc: xen-devel@lists.xenproject.org
>>> Cc: stable@vger.kernel.org
>>> ---
>>> arch/x86/kernel/cpu/amd.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>>> index a6f88ca1a6b4..99308fba4d7d 100644
>>> --- a/arch/x86/kernel/cpu/amd.c
>>> +++ b/arch/x86/kernel/cpu/amd.c
>>> @@ -29,6 +29,8 @@
>>> # include <asm/mmconfig.h>
>>> #endif
>>>
>>> +#include <xen/xen.h>
>>> +
>>> #include "cpu.h"
>>>
>>> u16 invlpgb_count_max __ro_after_init = 1;
>>> @@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
>>> 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>>> 		return 0;
>>>
>>> +	/* Xen PV domU cannot access hardware directly, so bail for domU case */
> Heh, Xen on Zen crime.
>
>>> +	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
>>> +		return 0;
>>> +
>>> 	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
>>> 	if (!addr)
>>> 		return 0;
>> Sean, looka here. The other hypervisor wants other checks.
>>
>> Time to whip out the X86_FEATURE_HYPERVISOR check.
> LOL, Ariadne, be honest, how much did Boris pay you?  :-D
>
> Jokes aside, I suppose I'm fine adding a HYPERVISOR check, but at the same time,
> how is this not a Xen bug?  Refusing to create a mapping because the VM doesn't
> have a device defined at a given GPA is pretty hostile behavior for a hypervisor.
>

This is a Xen PV guest.  No SVM/VT-x.

A PV Guest (by it's contract with Xen) cannot create mappings to host
physical addresses it doesn't own.  Xen rejects the attempt, and Linux
is ignoring the failure in this case.  This has been ABI for 25 years.

From a more practical point of view, because guests can read their own
pagetables, Xen can't swap the requested PTE for safe alternative to
trap the MMIO access, because that violates Linux's model of what's
mapped in this position.

~Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19  1:01 [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU Ariadne Conill
  2025-12-19  3:56 ` Borislav Petkov
@ 2025-12-19 16:32 ` Teddy Astie
  2025-12-19 17:38   ` Sean Christopherson
  1 sibling, 1 reply; 12+ messages in thread
From: Teddy Astie @ 2025-12-19 16:32 UTC (permalink / raw)
  To: Ariadne Conill, linux-kernel
  Cc: mario.limonciello, darwi, sandipan.das, kai.huang, me,
	yazen.ghannam, riel, peterz, hpa, x86, tglx, mingo, bp,
	dave.hansen, xen-devel, stable

Le 19/12/2025 à 02:04, Ariadne Conill a écrit :
> Xen domU cannot access the given MMIO address for security reasons,
> resulting in a failed hypercall in ioremap() due to permissions.
>
> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
> Cc: xen-devel@lists.xenproject.org
> Cc: stable@vger.kernel.org
> ---
>   arch/x86/kernel/cpu/amd.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index a6f88ca1a6b4..99308fba4d7d 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -29,6 +29,8 @@
>   # include <asm/mmconfig.h>
>   #endif
>
> +#include <xen/xen.h>
> +
>   #include "cpu.h"
>
>   u16 invlpgb_count_max __ro_after_init = 1;
> @@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
>   	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>   		return 0;
>
> +	/* Xen PV domU cannot access hardware directly, so bail for domU case */
> +	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
> +		return 0;
> +
>   	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
>   	if (!addr)
>   		return 0;

Such MMIO only has a meaning in a physical machine, but the feature
check is bogus as being on Zen arch is not enough for ensuring this.

I think this also translates in most hypervisors with odd reset codes
being reported; without being specific to Xen PV (Zen CPU is
unfortunately not enough to ensuring such MMIO exists).

Aside that, attempting unexpected MMIO in a SEV-ES/SNP guest can cause
weird problems since they may not handled MMIO-NAE and could lead the
hypervisor to crash the guest instead (unexpected NPF).

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19 16:26     ` Andrew Cooper
@ 2025-12-19 17:36       ` Sean Christopherson
  2025-12-19 23:14         ` Borislav Petkov
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2025-12-19 17:36 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Borislav Petkov, Ariadne Conill, linux-kernel, mario.limonciello,
	darwi, sandipan.das, kai.huang, me, yazen.ghannam, riel, peterz,
	hpa, x86, tglx, mingo, dave.hansen, xen-devel, stable

On Fri, Dec 19, 2025, Andrew Cooper wrote:
> On 19/12/2025 4:09 pm, Sean Christopherson wrote:
> > On Fri, Dec 19, 2025, Borislav Petkov wrote:
> >> On December 19, 2025 1:01:31 AM UTC, Ariadne Conill <ariadne@ariadne.space> wrote:
> >>> Xen domU cannot access the given MMIO address for security reasons,
> >>> resulting in a failed hypercall in ioremap() due to permissions.
> > Why does that matter though?  Ah, because set_pte() assumes success, and so
> > presumably the failed hypercall goes unnoticed and trying to access the MMIO
> > #PFs due to !PRESENT mapping.
> >
> >>> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
> >>> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
> >>> Cc: xen-devel@lists.xenproject.org
> >>> Cc: stable@vger.kernel.org
> >>> ---
> >>> arch/x86/kernel/cpu/amd.c | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> >>> index a6f88ca1a6b4..99308fba4d7d 100644
> >>> --- a/arch/x86/kernel/cpu/amd.c
> >>> +++ b/arch/x86/kernel/cpu/amd.c
> >>> @@ -29,6 +29,8 @@
> >>> # include <asm/mmconfig.h>
> >>> #endif
> >>>
> >>> +#include <xen/xen.h>
> >>> +
> >>> #include "cpu.h"
> >>>
> >>> u16 invlpgb_count_max __ro_after_init = 1;
> >>> @@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
> >>> 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
> >>> 		return 0;
> >>>
> >>> +	/* Xen PV domU cannot access hardware directly, so bail for domU case */
> > Heh, Xen on Zen crime.
> >
> >>> +	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
> >>> +		return 0;
> >>> +
> >>> 	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
> >>> 	if (!addr)
> >>> 		return 0;
> >> Sean, looka here. The other hypervisor wants other checks.
> >>
> >> Time to whip out the X86_FEATURE_HYPERVISOR check.
> > LOL, Ariadne, be honest, how much did Boris pay you?  :-D
> >
> > Jokes aside, I suppose I'm fine adding a HYPERVISOR check, but at the same time,
> > how is this not a Xen bug?  Refusing to create a mapping because the VM doesn't
> > have a device defined at a given GPA is pretty hostile behavior for a hypervisor.
>
> This is a Xen PV guest.  No SVM/VT-x.
> 
> A PV Guest (by it's contract with Xen) cannot create mappings to host
> physical addresses it doesn't own.

Huh, assuming wiki.xenproject.org[*] can be trusted, TIL Xen PV has no notion
of a virtual motherboard/chipset:

  In a paravirtualized VM, guests run with fully paravirtualized disk and
  network interfaces; interrupts and timers are paravirtualized; there is no
  emulated motherboard or device bus; guests boot directly into the kernel
  in the mode the kernel wishes to run in (32-bit or 64-bit), without needing
  to start in 16-bit mode or go through a BIOS; all privileged instructions
  are replaced with paravirtualized equivalents (hypercalls), and access to
  the page tables was paravirtualized as well.

[*] https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum

> Xen rejects the attempt, and Linux is ignoring the failure in this case. 
> This has been ABI for 25 years.

Heh, I suspected as much.

> From a more practical point of view, because guests can read their own
> pagetables, Xen can't swap the requested PTE for safe alternative to
> trap the MMIO access, because that violates Linux's model of what's
> mapped in this position.

That all makes sense, but shouldn't the inability to ioremap() chipset MMIO be
addressed in ioremap()?  E.g. after poking around a bit, commit 6a92b11169a6
("x86/EISA: Don't probe EISA bus for Xen PV guests") fudged around the same
underlying flaw in eisa_bus_probe().  Ha!  Though maybe that's no longer necessary
as of 80a4da05642c ("x86/EISA: Use memremap() to probe for the EISA BIOS signature")?

Anyways, I'm still opposed to using HYPERVISOR.  E.g. given that Dom0 can apparently
access host MMIO just fine, and that it's perfectly reasonable for a KVM-based VMM
to emulate the chipset, assuming a guest doesn't have access to some asset is simply
wrong.

But rather than play whack-a-mole, can't we deal with the ignore PTE write failures
in ioremap()?  E.g. something like this?

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 12c8180ca1ba..b7e2c752af1d 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -29,6 +29,8 @@
 #include <asm/memtype.h>
 #include <asm/setup.h>
 
+#include <xen/xen.h>
+
 #include "physaddr.h"
 
 /*
@@ -301,6 +303,20 @@ __ioremap_caller(resource_size_t phys_addr, unsigned long size,
        if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot))
                goto err_free_area;
 
+       /*
+        * Verify the range was actually mapped when running as a Xen PV DomU
+        * guest.  Xen PV doesn't emulate a virtual chipset/motherboard, and
+        * disallows DomU from mapping host physical addresses that the domain
+        * doesn't own.  Unfortunately, the PTE APIs assume success, and so
+        * Xen's rejection of the mapping is ignored.
+        */
+       if (xen_pv_domain() && !xen_initial_domain()) {
+               int level;
+
+               if (!lookup_address(vaddr, &level))
+                       goto err_free_area;
+       }
+
        ret_addr = (void __iomem *) (vaddr + offset);
        mmiotrace_ioremap(unaligned_phys_addr, unaligned_size, ret_addr);

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19 16:32 ` Teddy Astie
@ 2025-12-19 17:38   ` Sean Christopherson
  2025-12-20  1:44     ` Teddy Astie
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2025-12-19 17:38 UTC (permalink / raw)
  To: Teddy Astie
  Cc: Ariadne Conill, linux-kernel, mario.limonciello, darwi,
	sandipan.das, kai.huang, me, yazen.ghannam, riel, peterz, hpa,
	x86, tglx, mingo, bp, dave.hansen, xen-devel, stable

On Fri, Dec 19, 2025, Teddy Astie wrote:
> Le 19/12/2025 à 02:04, Ariadne Conill a écrit :
> > Xen domU cannot access the given MMIO address for security reasons,
> > resulting in a failed hypercall in ioremap() due to permissions.
> >
> > Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
> > Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
> > Cc: xen-devel@lists.xenproject.org
> > Cc: stable@vger.kernel.org
> > ---
> >   arch/x86/kernel/cpu/amd.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> > index a6f88ca1a6b4..99308fba4d7d 100644
> > --- a/arch/x86/kernel/cpu/amd.c
> > +++ b/arch/x86/kernel/cpu/amd.c
> > @@ -29,6 +29,8 @@
> >   # include <asm/mmconfig.h>
> >   #endif
> >
> > +#include <xen/xen.h>
> > +
> >   #include "cpu.h"
> >
> >   u16 invlpgb_count_max __ro_after_init = 1;
> > @@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
> >   	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
> >   		return 0;
> >
> > +	/* Xen PV domU cannot access hardware directly, so bail for domU case */
> > +	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
> > +		return 0;
> > +
> >   	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
> >   	if (!addr)
> >   		return 0;
> 
> Such MMIO only has a meaning in a physical machine, but the feature
> check is bogus as being on Zen arch is not enough for ensuring this.
> 
> I think this also translates in most hypervisors with odd reset codes
> being reported; without being specific to Xen PV (Zen CPU is
> unfortunately not enough to ensuring such MMIO exists).
> 
> Aside that, attempting unexpected MMIO in a SEV-ES/SNP guest can cause
> weird problems since they may not handled MMIO-NAE and could lead the
> hypervisor to crash the guest instead (unexpected NPF).

IMO, terminating an SEV-ES+ guest because it accesses an unknown MMIO range is
unequivocally a hypervisor bug.  The right behavior there is to configure a
reserved NPT entry to reflect the access into the guest as a #VC.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19 17:36       ` Sean Christopherson
@ 2025-12-19 23:14         ` Borislav Petkov
  0 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2025-12-19 23:14 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Andrew Cooper, Ariadne Conill, linux-kernel, mario.limonciello,
	darwi, sandipan.das, kai.huang, me, yazen.ghannam, riel, peterz,
	hpa, x86, tglx, mingo, dave.hansen, xen-devel, stable

On Fri, Dec 19, 2025 at 09:36:45AM -0800, Sean Christopherson wrote:
> @@ -301,6 +303,20 @@ __ioremap_caller(resource_size_t phys_addr, unsigned long size,
>         if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot))
>                 goto err_free_area;
>  
> +       /*
> +        * Verify the range was actually mapped when running as a Xen PV DomU
> +        * guest.  Xen PV doesn't emulate a virtual chipset/motherboard, and
> +        * disallows DomU from mapping host physical addresses that the domain
> +        * doesn't own.  Unfortunately, the PTE APIs assume success, and so
> +        * Xen's rejection of the mapping is ignored.
> +        */
> +       if (xen_pv_domain() && !xen_initial_domain()) {
> +               int level;
> +
> +               if (!lookup_address(vaddr, &level))
> +                       goto err_free_area;
> +       }

This activates my ancient allergies caused by the sprinkling "if (XEN)"
randomly across the kernel tree. If this is a PV guest there probably
should be a PV ioremap variant which hides all that gunk away...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19 16:09   ` Sean Christopherson
  2025-12-19 16:26     ` Andrew Cooper
@ 2025-12-19 23:16     ` Borislav Petkov
  2025-12-19 23:19     ` Ariadne Conill
  2 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2025-12-19 23:16 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Ariadne Conill, linux-kernel, mario.limonciello, darwi,
	sandipan.das, kai.huang, me, yazen.ghannam, riel, peterz, hpa,
	x86, tglx, mingo, dave.hansen, xen-devel, stable

On Fri, Dec 19, 2025 at 08:09:31AM -0800, Sean Christopherson wrote:
> LOL, Ariadne, be honest, how much did Boris pay you?  :-D

Ha, now there's a thought: win the lottery and then pay people to do
specially crafted reports influencing the kernel design. Woahahahah, /me
laughs ominously.

One problem though: winning the lottery.

;-P

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19 16:09   ` Sean Christopherson
  2025-12-19 16:26     ` Andrew Cooper
  2025-12-19 23:16     ` Borislav Petkov
@ 2025-12-19 23:19     ` Ariadne Conill
  2 siblings, 0 replies; 12+ messages in thread
From: Ariadne Conill @ 2025-12-19 23:19 UTC (permalink / raw)
  To: Sean Christopherson, Borislav Petkov
  Cc: linux-kernel, mario.limonciello, darwi, sandipan.das, kai.huang,
	me, yazen.ghannam, riel, peterz, hpa, x86, tglx, mingo,
	dave.hansen, xen-devel, stable

Hi,

On 12/19/25 08:09, Sean Christopherson wrote:
> On Fri, Dec 19, 2025, Borislav Petkov wrote:
>> On December 19, 2025 1:01:31 AM UTC, Ariadne Conill <ariadne@ariadne.space> wrote:
>>> Xen domU cannot access the given MMIO address for security reasons,
>>> resulting in a failed hypercall in ioremap() due to permissions.
> Why does that matter though?  Ah, because set_pte() assumes success, and so
> presumably the failed hypercall goes unnoticed and trying to access the MMIO
> #PFs due to !PRESENT mapping.

Yes, which results in the guest panicing on Zen platforms.

>>> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
>>> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
>>> Cc: xen-devel@lists.xenproject.org
>>> Cc: stable@vger.kernel.org
>>> ---
>>> arch/x86/kernel/cpu/amd.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>>> index a6f88ca1a6b4..99308fba4d7d 100644
>>> --- a/arch/x86/kernel/cpu/amd.c
>>> +++ b/arch/x86/kernel/cpu/amd.c
>>> @@ -29,6 +29,8 @@
>>> # include <asm/mmconfig.h>
>>> #endif
>>>
>>> +#include <xen/xen.h>
>>> +
>>> #include "cpu.h"
>>>
>>> u16 invlpgb_count_max __ro_after_init = 1;
>>> @@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
>>> 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>>> 		return 0;
>>>
>>> +	/* Xen PV domU cannot access hardware directly, so bail for domU case */
> Heh, Xen on Zen crime.
>
>>> +	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
>>> +		return 0;
>>> +
>>> 	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
>>> 	if (!addr)
>>> 		return 0;
>> Sean, looka here. The other hypervisor wants other checks.
>>
>> Time to whip out the X86_FEATURE_HYPERVISOR check.
> LOL, Ariadne, be honest, how much did Boris pay you?  :-D

Nothing :)

At Edera we have been running with this patch for a few months, I just 
forgot to upstream it.

I was reminded of this patch when an Alpine user opened a bug[0] 
demonstrating the same behavior on 6.18.

[0]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/17789

> Jokes aside, I suppose I'm fine adding a HYPERVISOR check, but at the same time,
> how is this not a Xen bug?  Refusing to create a mapping because the VM doesn't
> have a device defined at a given GPA is pretty hostile behavior for a hypervisor.

I think it would be better to fix this in a more generic way if we can.

Ariadne


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-19 17:38   ` Sean Christopherson
@ 2025-12-20  1:44     ` Teddy Astie
  2025-12-22 15:46       ` Sean Christopherson
  0 siblings, 1 reply; 12+ messages in thread
From: Teddy Astie @ 2025-12-20  1:44 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Ariadne Conill, linux-kernel, mario.limonciello, darwi,
	sandipan.das, kai.huang, me, yazen.ghannam, riel, peterz, hpa,
	x86, tglx, mingo, bp, dave.hansen, xen-devel, stable

Le 19/12/2025 à 18:40, Sean Christopherson a écrit :
> On Fri, Dec 19, 2025, Teddy Astie wrote:
>> Le 19/12/2025 à 02:04, Ariadne Conill a écrit :
>>> Xen domU cannot access the given MMIO address for security reasons,
>>> resulting in a failed hypercall in ioremap() due to permissions.
>>>
>>> Fixes: ab8131028710 ("x86/CPU/AMD: Print the reason for the last reset")
>>> Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
>>> Cc: xen-devel@lists.xenproject.org
>>> Cc: stable@vger.kernel.org
>>> ---
>>>    arch/x86/kernel/cpu/amd.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>>> index a6f88ca1a6b4..99308fba4d7d 100644
>>> --- a/arch/x86/kernel/cpu/amd.c
>>> +++ b/arch/x86/kernel/cpu/amd.c
>>> @@ -29,6 +29,8 @@
>>>    # include <asm/mmconfig.h>
>>>    #endif
>>>
>>> +#include <xen/xen.h>
>>> +
>>>    #include "cpu.h"
>>>
>>>    u16 invlpgb_count_max __ro_after_init = 1;
>>> @@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
>>>    	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>>>    		return 0;
>>>
>>> +	/* Xen PV domU cannot access hardware directly, so bail for domU case */
>>> +	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
>>> +		return 0;
>>> +
>>>    	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
>>>    	if (!addr)
>>>    		return 0;
>>
>> Such MMIO only has a meaning in a physical machine, but the feature
>> check is bogus as being on Zen arch is not enough for ensuring this.
>>
>> I think this also translates in most hypervisors with odd reset codes
>> being reported; without being specific to Xen PV (Zen CPU is
>> unfortunately not enough to ensuring such MMIO exists).
>>
>> Aside that, attempting unexpected MMIO in a SEV-ES/SNP guest can cause
>> weird problems since they may not handled MMIO-NAE and could lead the
>> hypervisor to crash the guest instead (unexpected NPF).
>
> IMO, terminating an SEV-ES+ guest because it accesses an unknown MMIO range is
> unequivocally a hypervisor bug.

Terminating may be a bit excessive, but the hypervisor can respond #GP
to either unexpected MMIO-NAE and NPF-AE if it doesn't know how to deal
with this MMIO/NPF (xAPIC has a similar behavior when it is disabled).

> The right behavior there is to configure a reserved NPT entry
> to reflect the access into the guest as a #VC.

I'm not sure this is the best approach, that would allow the guest to
trick the hypervisor into making a unbounded amount of reserved entries.

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU
  2025-12-20  1:44     ` Teddy Astie
@ 2025-12-22 15:46       ` Sean Christopherson
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2025-12-22 15:46 UTC (permalink / raw)
  To: Teddy Astie
  Cc: Ariadne Conill, linux-kernel, mario.limonciello, darwi,
	sandipan.das, kai.huang, me, yazen.ghannam, riel, peterz, hpa,
	x86, tglx, mingo, bp, dave.hansen, xen-devel, stable

On Sat, Dec 20, 2025, Teddy Astie wrote:
> Le 19/12/2025 à 18:40, Sean Christopherson a écrit :
> > On Fri, Dec 19, 2025, Teddy Astie wrote:
> >>> @@ -1333,6 +1335,10 @@ static __init int print_s5_reset_status_mmio(void)
> >>>    	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
> >>>    		return 0;
> >>>
> >>> +	/* Xen PV domU cannot access hardware directly, so bail for domU case */
> >>> +	if (cpu_feature_enabled(X86_FEATURE_XENPV) && !xen_initial_domain())
> >>> +		return 0;
> >>> +
> >>>    	addr = ioremap(FCH_PM_BASE + FCH_PM_S5_RESET_STATUS, sizeof(value));
> >>>    	if (!addr)
> >>>    		return 0;
> >>
> >> Such MMIO only has a meaning in a physical machine, but the feature
> >> check is bogus as being on Zen arch is not enough for ensuring this.
> >>
> >> I think this also translates in most hypervisors with odd reset codes
> >> being reported; without being specific to Xen PV (Zen CPU is
> >> unfortunately not enough to ensuring such MMIO exists).
> >>
> >> Aside that, attempting unexpected MMIO in a SEV-ES/SNP guest can cause
> >> weird problems since they may not handled MMIO-NAE and could lead the
> >> hypervisor to crash the guest instead (unexpected NPF).
> >
> > IMO, terminating an SEV-ES+ guest because it accesses an unknown MMIO range is
> > unequivocally a hypervisor bug.
> 
> Terminating may be a bit excessive, but the hypervisor can respond #GP
> to either unexpected MMIO-NAE and NPF-AE if it doesn't know how to deal
> with this MMIO/NPF (xAPIC has a similar behavior when it is disabled).

Maybe with a very liberal interpretation of AMD specs, e.g. to mimic the reserved
HyperTransport region behavior.  Defining a virtual platform/bus that #GPs on
accesses to any "unknown" MMIO region would be incredibly hostile behavior for
a hypervisor.

> > The right behavior there is to configure a reserved NPT entry
> > to reflect the access into the guest as a #VC.
> 
> I'm not sure this is the best approach, that would allow the guest to
> trick the hypervisor into making a unbounded amount of reserved entries.

No, the maximum number of reserved entries is bounded by the number of vCPUs in
the VM, because each reserved entry only needs to exist long enough to refect
the access into the guest.  Recycling NPT page tables after every MMIO-NAE would
be comically agressively, but it's very doable for a hypervisor to set a reasonable
limit on the number of NPT page tables it creates for a VM.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-12-22 15:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-19  1:01 [PATCH] x86/CPU/AMD: avoid printing reset reasons on Xen domU Ariadne Conill
2025-12-19  3:56 ` Borislav Petkov
2025-12-19 16:09   ` Sean Christopherson
2025-12-19 16:26     ` Andrew Cooper
2025-12-19 17:36       ` Sean Christopherson
2025-12-19 23:14         ` Borislav Petkov
2025-12-19 23:16     ` Borislav Petkov
2025-12-19 23:19     ` Ariadne Conill
2025-12-19 16:32 ` Teddy Astie
2025-12-19 17:38   ` Sean Christopherson
2025-12-20  1:44     ` Teddy Astie
2025-12-22 15:46       ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).