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
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
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.
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
© 2016 - 2025 Red Hat, Inc.