[PATCH v10 4/9] x86/altp2m: Remove p2m_altp2m_check stubs from unsupported architectures

Petr Beneš posted 9 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v10 4/9] x86/altp2m: Remove p2m_altp2m_check stubs from unsupported architectures
Posted by Petr Beneš 3 months, 2 weeks ago
From: Petr Beneš <w1benny@gmail.com>

The p2m_altp2m_check() stub was previously declared on all architectures,
even though the altp2m feature is only supported on x86. This patch removes
the unused stub definitions from ARM, PPC, and RISC-V, and wraps the actual
usage sites in #ifdef CONFIG_ALTP2M instead.

Additionally, the declaration and definition of p2m_altp2m_check() are now
correctly nested under CONFIG_HVM, reflecting the fact that CONFIG_ALTP2M
always implies CONFIG_HVM.

Signed-off-by: Petr Beneš <w1benny@gmail.com>
---
 xen/arch/arm/include/asm/p2m.h   | 6 ------
 xen/arch/ppc/include/asm/p2m.h   | 5 -----
 xen/arch/riscv/include/asm/p2m.h | 5 -----
 xen/arch/x86/hvm/monitor.c       | 2 ++
 xen/arch/x86/include/asm/p2m.h   | 9 ++++-----
 xen/common/vm_event.c            | 2 ++
 6 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h
index 2d53bf9b61..ef98bc5f4d 100644
--- a/xen/arch/arm/include/asm/p2m.h
+++ b/xen/arch/arm/include/asm/p2m.h
@@ -180,12 +180,6 @@ static inline bool arch_acquire_resource_check(struct domain *d)
     return true;
 }
 
-static inline
-void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
-{
-    /* Not supported on ARM. */
-}
-
 /*
  * Helper to restrict "p2m_ipa_bits" according the external entity
  * (e.g. IOMMU) requirements.
diff --git a/xen/arch/ppc/include/asm/p2m.h b/xen/arch/ppc/include/asm/p2m.h
index f144ef8e1a..c96149ef74 100644
--- a/xen/arch/ppc/include/asm/p2m.h
+++ b/xen/arch/ppc/include/asm/p2m.h
@@ -88,9 +88,4 @@ static inline bool arch_acquire_resource_check(struct domain *d)
     return false;
 }
 
-static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
-{
-    /* Not supported on PPC. */
-}
-
 #endif /* __ASM_PPC_P2M_H__ */
diff --git a/xen/arch/riscv/include/asm/p2m.h b/xen/arch/riscv/include/asm/p2m.h
index 28f57a74f2..e43c559e0c 100644
--- a/xen/arch/riscv/include/asm/p2m.h
+++ b/xen/arch/riscv/include/asm/p2m.h
@@ -88,11 +88,6 @@ static inline bool arch_acquire_resource_check(struct domain *d)
     return false;
 }
 
-static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
-{
-    /* Not supported on RISCV. */
-}
-
 #endif /* ASM__RISCV__P2M_H */
 
 /*
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 523586ca98..d22a2e4644 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -178,6 +178,7 @@ int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
         break;
 
     case HVM_MONITOR_SINGLESTEP_BREAKPOINT:
+#ifdef CONFIG_ALTP2M
         if ( curr->arch.hvm.fast_single_step.enabled )
         {
             p2m_altp2m_check(curr, curr->arch.hvm.fast_single_step.p2midx);
@@ -186,6 +187,7 @@ int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
             curr->arch.hvm.fast_single_step.p2midx = 0;
             return 0;
         }
+#endif
         if ( !ad->monitor.singlestep_enabled )
             return 0;
         req.reason = VM_EVENT_REASON_SINGLESTEP;
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 58b56e575e..7375895836 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -962,17 +962,16 @@ int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
 /* Set a specific p2m view visibility */
 int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
                                    uint8_t visible);
-#else /* !CONFIG_HVM */
-struct p2m_domain *p2m_get_altp2m(struct vcpu *v);
-#endif /* CONFIG_HVM */
 
 #ifdef CONFIG_ALTP2M
 /* Check to see if vcpu should be switched to a different p2m. */
 void p2m_altp2m_check(struct vcpu *v, uint16_t idx);
-#else
-static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx) {}
 #endif
 
+#else /* !CONFIG_HVM */
+struct p2m_domain *p2m_get_altp2m(struct vcpu *v);
+#endif /* CONFIG_HVM */
+
 /* p2m access to IOMMU flags */
 static inline unsigned int p2m_access_to_iommu_flags(p2m_access_t p2ma)
 {
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 1666ff615f..98204cbf43 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -430,9 +430,11 @@ static int vm_event_resume(struct domain *d, struct vm_event_domain *ved)
              */
             vm_event_toggle_singlestep(d, v, &rsp);
 
+#ifdef CONFIG_ALTP2M
             /* Check for altp2m switch */
             if ( rsp.flags & VM_EVENT_FLAG_ALTERNATE_P2M )
                 p2m_altp2m_check(v, rsp.altp2m_idx);
+#endif
 
             if ( rsp.flags & VM_EVENT_FLAG_SET_REGISTERS )
                 vm_event_set_registers(v, &rsp);
-- 
2.34.1


Re: [PATCH v10 4/9] x86/altp2m: Remove p2m_altp2m_check stubs from unsupported architectures
Posted by Jan Beulich 3 months, 1 week ago
On 16.07.2025 22:15, Petr Beneš wrote:
> From: Petr Beneš <w1benny@gmail.com>
> 
> The p2m_altp2m_check() stub was previously declared on all architectures,
> even though the altp2m feature is only supported on x86. This patch removes
> the unused stub definitions from ARM, PPC, and RISC-V, and wraps the actual
> usage sites in #ifdef CONFIG_ALTP2M instead.

Hmm, using IS_ENABLED() would certainly be preferred. That would apparently
require ...

> --- a/xen/arch/x86/include/asm/p2m.h
> +++ b/xen/arch/x86/include/asm/p2m.h
> @@ -962,17 +962,16 @@ int p2m_altp2m_propagate_change(struct domain *d, gfn_t gfn,
>  /* Set a specific p2m view visibility */
>  int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
>                                     uint8_t visible);
> -#else /* !CONFIG_HVM */
> -struct p2m_domain *p2m_get_altp2m(struct vcpu *v);
> -#endif /* CONFIG_HVM */
>  
>  #ifdef CONFIG_ALTP2M
>  /* Check to see if vcpu should be switched to a different p2m. */
>  void p2m_altp2m_check(struct vcpu *v, uint16_t idx);

... the declaration to be generally visible (likely also outside of x86).

> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -430,9 +430,11 @@ static int vm_event_resume(struct domain *d, struct vm_event_domain *ved)
>               */
>              vm_event_toggle_singlestep(d, v, &rsp);
>  
> +#ifdef CONFIG_ALTP2M
>              /* Check for altp2m switch */
>              if ( rsp.flags & VM_EVENT_FLAG_ALTERNATE_P2M )
>                  p2m_altp2m_check(v, rsp.altp2m_idx);
> +#endif

Is (and was) this actually correct? Shouldn't the caller be notified
somehow that its use of VM_EVENT_FLAG_ALTERNATE_P2M had no effect when
ALTP2M=n?

Jan

Re: [PATCH v10 4/9] x86/altp2m: Remove p2m_altp2m_check stubs from unsupported architectures
Posted by Petr Beneš 3 months ago
On Tue, Jul 22, 2025 at 4:56 PM Jan Beulich <jbeulich@suse.com> wrote:
> Hmm, using IS_ENABLED() would certainly be preferred.

Why? Very similar usage is a few lines above, with #ifdef
CONFIG_MEM_PAGING and CONFIG_MEM_SHARING.

> That would apparently require ...
> ... the declaration to be generally visible (likely also outside of x86).

With all the other changes in this series, it seems like it only makes
sense to guard p2m_altp2m_check with CONFIG_ALTP2M.

"Having the declaration to be generally visible" is exactly what I was
trying to avoid.

P.
Re: [PATCH v10 4/9] x86/altp2m: Remove p2m_altp2m_check stubs from unsupported architectures
Posted by Jan Beulich 3 months ago
On 29.07.2025 22:39, Petr Beneš wrote:
> On Tue, Jul 22, 2025 at 4:56 PM Jan Beulich <jbeulich@suse.com> wrote:
>> Hmm, using IS_ENABLED() would certainly be preferred.
> 
> Why? Very similar usage is a few lines above, with #ifdef
> CONFIG_MEM_PAGING and CONFIG_MEM_SHARING.

Our preference towards IS_ENABLED() (where possible) likely post-dates
the addition of those #ifdef-s.

Jan