On 16.07.2025 22:15, Petr Beneš wrote: > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -2686,8 +2686,8 @@ static int cf_check hvmemul_tlb_op( > return rc; > } > > -static int cf_check hvmemul_vmfunc( > - struct x86_emulate_ctxt *ctxt) > +#ifdef CONFIG_ALTP2M > +static int cf_check hvmemul_vmfunc(struct x86_emulate_ctxt *ctxt)
Please don't needlessly alter formatting, and ... > { > int rc; > > @@ -2699,6 +2699,12 @@ static int cf_check hvmemul_vmfunc( > > return rc; > } > +#else > +static int cf_check hvmemul_vmfunc(struct x86_emulate_ctxt *ctxt) > +{ > + return X86EMUL_UNHANDLEABLE; > +} > +#endif ... please don't duplicate function headers when such #ifdef-s can easily be put inside the function body. Much like you do ... > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -4608,6 +4608,7 @@ static int hvmop_get_param( > static int do_altp2m_op( > XEN_GUEST_HANDLE_PARAM(void) arg) > { > +#ifdef CONFIG_ALTP2M > struct xen_hvm_altp2m_op a; > struct domain *d = NULL; > int rc = 0; > @@ -4944,6 +4945,9 @@ static int do_altp2m_op( > rcu_unlock_domain(d); > > return rc; > +#else /* !CONFIG_ALTP2M */ > + return -EOPNOTSUPP; > +#endif /* CONFIG_ALTP2M */ > } ... here. > @@ -5261,6 +5269,7 @@ void hvm_toggle_singlestep(struct vcpu *v) > > void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx) > { > +#ifdef CONFIG_ALTP2M > ASSERT(atomic_read(&v->pause_count)); > > if ( !hvm_is_singlestep_supported() ) > @@ -5272,6 +5281,7 @@ void hvm_fast_singlestep(struct vcpu *v, uint16_t > p2midx) > v->arch.hvm.single_step = true; > v->arch.hvm.fast_single_step.enabled = true; > v->arch.hvm.fast_single_step.p2midx = p2midx; > +#endif > } This function would better be unreachable when ALTP2M=n, imo. Putting an #ifdef in vm_event_toggle_singlestep() would be the simple solution, but maybe we could to better. > @@ -707,6 +709,7 @@ static inline bool hvm_nested_virt_supported(void) > return hvm_funcs.caps.nested_virt; > } > > +#ifdef CONFIG_ALTP2M > /* updates the current hardware p2m */ > static inline void altp2m_vcpu_update_p2m(struct vcpu *v) > { > @@ -731,6 +734,11 @@ static inline bool altp2m_vcpu_emulate_ve(struct vcpu *v) > } > return false; > } > +#else /* !CONFIG_ALTP2M */ > +void altp2m_vcpu_update_p2m(struct vcpu *v); > +void altp2m_vcpu_update_vmfunc_ve(struct vcpu *v); > +bool altp2m_vcpu_emulate_ve(struct vcpu *v); > +#endif /* CONFIG_ALTP2M */ Why would the altp2m_vcpu_update_{p2m,vmfunc_ve}() declarations be needed? All uses are from altp2m.c. Jan