:p
atchew
Login
Returning a literal number is a bad idea anyway when all other returns use IOREQ_STATUS_* values. While that's maybe intended on Arm (mapping to IO_ABORT), mapping to X86EMUL_OKAY is surely wrong on x86. Fixes: f6bf39f84f82 ("x86/hvm: add support for broadcast of buffered ioreqs...") Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Judging from history, it may want to be IOREQ_STATUS_UNHANDLED instead, eliminating the need for IOREQ_STATUS_BAD. That'll be a behavioral change on Arm then too, though. Shouldn't IOREQ_READ requests also be rejected here, for the result of a read not possibly coming from anywhere, yet a (bogus) caller then assuming some data was actually returned? --- a/xen/arch/arm/include/asm/ioreq.h +++ b/xen/arch/arm/include/asm/ioreq.h @@ -XXX,XX +XXX,XX @@ static inline void msix_write_completion #define IOREQ_STATUS_HANDLED IO_HANDLED #define IOREQ_STATUS_UNHANDLED IO_UNHANDLED #define IOREQ_STATUS_RETRY IO_RETRY +#define IOREQ_STATUS_BAD IO_ABORT #endif /* __ASM_ARM_IOREQ_H__ */ --- a/xen/arch/x86/include/asm/hvm/ioreq.h +++ b/xen/arch/x86/include/asm/hvm/ioreq.h @@ -XXX,XX +XXX,XX @@ #define IOREQ_STATUS_HANDLED X86EMUL_OKAY #define IOREQ_STATUS_UNHANDLED X86EMUL_UNHANDLEABLE #define IOREQ_STATUS_RETRY X86EMUL_RETRY +#define IOREQ_STATUS_BAD X86EMUL_UNRECOGNIZED #endif /* __ASM_X86_HVM_IOREQ_H__ */ --- a/xen/common/ioreq.c +++ b/xen/common/ioreq.c @@ -XXX,XX +XXX,XX @@ static int ioreq_send_buffered(struct io return IOREQ_STATUS_UNHANDLED; /* - * Return 0 for the cases we can't deal with: + * Return BAD for the cases we can't deal with: * - 'addr' is only a 20-bit field, so we cannot address beyond 1MB * - we cannot buffer accesses to guest memory buffers, as the guest * may expect the memory buffer to be synchronously accessed @@ -XXX,XX +XXX,XX @@ static int ioreq_send_buffered(struct io * support data_is_ptr we do not waste space for the count field either */ if ( (p->addr > 0xfffffUL) || p->data_is_ptr || (p->count != 1) ) - return 0; + return IOREQ_STATUS_BAD; switch ( p->size ) {
Returning a literal number is a bad idea anyway when all other returns use IOREQ_STATUS_* values. The function is dead on Arm, and mapping to X86EMUL_OKAY is surely wrong on x86. Fixes: f6bf39f84f82 ("x86/hvm: add support for broadcast of buffered ioreqs...") Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Shouldn't IOREQ_READ requests also be rejected here, for the result of a read not possibly coming from anywhere, yet a (bogus) caller then assuming some data was actually returned? ioreq_send_buffered() being built on Arm is a violation of Misra rule 2.1, which apparently Eclair doesn't flag (the rule is marked clean). --- v2: Use IOREQ_STATUS_UNHANDLED. --- unstable.orig/xen/common/ioreq.c 2024-09-30 12:22:03.759445625 +0200 +++ unstable/xen/common/ioreq.c 2024-09-30 12:24:06.516408920 +0200 @@ -XXX,XX +XXX,XX @@ static int ioreq_send_buffered(struct io return IOREQ_STATUS_UNHANDLED; /* - * Return 0 for the cases we can't deal with: + * Return UNHANDLED for the cases we can't deal with: * - 'addr' is only a 20-bit field, so we cannot address beyond 1MB * - we cannot buffer accesses to guest memory buffers, as the guest * may expect the memory buffer to be synchronously accessed @@ -XXX,XX +XXX,XX @@ static int ioreq_send_buffered(struct io * support data_is_ptr we do not waste space for the count field either */ if ( (p->addr > 0xfffffUL) || p->data_is_ptr || (p->count != 1) ) - return 0; + return IOREQ_STATUS_UNHANDLED; switch ( p->size ) {