On 09.07.2024 08:09, Sergiy Kibrik wrote: > --- a/xen/arch/x86/include/asm/hvm/ioreq.h > +++ b/xen/arch/x86/include/asm/hvm/ioreq.h > @@ -13,6 +13,11 @@ > #define IOREQ_STATUS_UNHANDLED X86EMUL_UNHANDLEABLE > #define IOREQ_STATUS_RETRY X86EMUL_RETRY > > +#ifdef CONFIG_VMX > +bool arch_vcpu_ioreq_completion(enum vio_completion completion); > +#define arch_vcpu_ioreq_completion > +#endif
Putting the (or some kind of) #define here is certainly fine, but moving ... > --- a/xen/include/xen/ioreq.h > +++ b/xen/include/xen/ioreq.h > @@ -111,7 +111,6 @@ void ioreq_domain_init(struct domain *d); > int ioreq_server_dm_op(struct xen_dm_op *op, struct domain *d, bool > *const_op); > > bool arch_ioreq_complete_mmio(void); > -bool arch_vcpu_ioreq_completion(enum vio_completion completion); > int arch_ioreq_server_map_pages(struct ioreq_server *s); > void arch_ioreq_server_unmap_pages(struct ioreq_server *s); > void arch_ioreq_server_enable(struct ioreq_server *s); ... the declaration from here requires that all architectures wanting to implement the function need to have identical copies. That's unnecessary risk of going out of sync. As to the #define itself: It expanding to nothing means the call site de-generates to #ifdef arch_vcpu_ioreq_completion res = (completion); #else which hardly is what is meant (despite compiling fine, and it likely only being Eclair which would then tell us about the issue). Further there you're also removing a blank line, I don't see why you're doing that. Jan