[PATCH v2] generic/altp2m: align and simplify altp2m_vcpu_idx()

Dmytro Prokopchuk1 posted 1 patch 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/f246f66c9020d5ab5f6ca20e0645b93b6210b4a2.1783608944.git.dmytro._5Fprokopchuk1@epam.com
xen/include/asm-generic/altp2m.h | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
[PATCH v2] generic/altp2m: align and simplify altp2m_vcpu_idx()
Posted by Dmytro Prokopchuk1 2 weeks ago
The return type of altp2m_vcpu_idx() in the generic altp2m.h header is
currently 'unsigned int', which is inconsistent with its uint16_t
return type on x86 and the altp2m_idx member of the monitor structures.

To fix this type inconsistency and simplify the header, this patch
replaces the static inline implementation of altp2m_vcpu_idx()
(contained a BUG() stub) with a simple function declaration returning
uint16_t.

For architectures using the generic altp2m.h header (such as ARM when
CONFIG_VM_EVENT is enabled), common code calls to altp2m_vcpu_idx() in
common/monitor.c are guarded by altp2m_active(), which statically
returns false. The compiler DCE will optimize out these calls, avoiding
any linker issues for the missing definition.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
Changes in v2:
- add Stefano's comment regarding return type
- rewrite commit subject and message
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2665080481
---
 xen/include/asm-generic/altp2m.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/xen/include/asm-generic/altp2m.h b/xen/include/asm-generic/altp2m.h
index 39865a842a..df6b9a9c01 100644
--- a/xen/include/asm-generic/altp2m.h
+++ b/xen/include/asm-generic/altp2m.h
@@ -15,12 +15,7 @@ static inline bool altp2m_active(const struct domain *d)
 }
 
 /* Alternate p2m VCPU */
-static inline unsigned int altp2m_vcpu_idx(const struct vcpu *v)
-{
-    /* Not implemented on GENERIC, should not be reached. */
-    BUG();
-    return 0;
-}
+uint16_t altp2m_vcpu_idx(const struct vcpu *v);
 
 #endif /* __ASM_GENERIC_ALTP2M_H */
 
-- 
2.43.0
Re: [PATCH v2] generic/altp2m: align and simplify altp2m_vcpu_idx()
Posted by Jan Beulich 1 week ago
On 09.07.2026 17:23, Dmytro Prokopchuk1 wrote:
> The return type of altp2m_vcpu_idx() in the generic altp2m.h header is
> currently 'unsigned int', which is inconsistent with its uint16_t
> return type on x86 and the altp2m_idx member of the monitor structures.
> 
> To fix this type inconsistency and simplify the header, this patch
> replaces the static inline implementation of altp2m_vcpu_idx()
> (contained a BUG() stub) with a simple function declaration returning
> uint16_t.

While I appreciate that this may be the easiest route, I disagree. Not
even considering ./CODING_STYLE, uint16_t may be an appropriate choice
on x86, but it pretty certainly isn't a good one in generic code. I
have a vague recollection that it might actually have been me to request
use of unsigned int when the stub was added. If so, it was for said
reason.

> For architectures using the generic altp2m.h header (such as ARM when
> CONFIG_VM_EVENT is enabled), common code calls to altp2m_vcpu_idx() in
> common/monitor.c are guarded by altp2m_active(), which statically
> returns false. The compiler DCE will optimize out these calls, avoiding
> any linker issues for the missing definition.

This part is definitely fine in principle. However, if a declaration
suffices, that would then better also be what x86 uses when !ALTP2M.
I.e. I think you want to #include <asm-generic/altp2m.h> from that
section of x86'es asm/altp2m.h

Jan