On Tue, Jan 27, 2026 at 11:21:06AM +0100, Jan Beulich wrote:
> Only explicit writes are subject to e.g. the checking of the MSR intercept
> bitmap, which extends to VM-event's hvm_monitor_msr(). Indicate, by way of
> a new boolean parameter, to the hook functions which variant it is.
>
> Fixes: 6eb43fcf8a0b ("x86emul: support SWAPGS")
> Reported-by: Andrew Cooper <[email protected]>
> Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Roger Pau Monné <[email protected]>
> ---
> Later, in particular for nested, ->read_msr() may need to gain a similar
> parameter.
>
> Prior to nested making use of it, the new parameter is effectively dead
> code with VM_EVENT=n. If we accepted Misra rule 2.2, something would
> likely need doing about this.
Hm, yes, it propagates into hvm_msr_write_intercept() which then turns
into `if ( may_defer && false )` in the VM_EVENT=n. But then you
could say the same about the code inside the if block above for the
VM_EVENT=n, it's also effectively unreachable code.
> I've suitably re-based "x86emul: misc additions" on top of this, but I
> don't think I'll re-post it just for that.
>
> --- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> +++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> @@ -569,7 +569,8 @@ static int fuzz_read_msr(
> static int fuzz_write_msr(
> unsigned int reg,
> uint64_t val,
> - struct x86_emulate_ctxt *ctxt)
> + struct x86_emulate_ctxt *ctxt,
> + bool explicit)
> {
> struct fuzz_state *s = ctxt->data;
> struct fuzz_corpus *c = s->corpus;
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1705,7 +1705,8 @@ static int cf_check hvmemul_write_io_dis
> static int cf_check hvmemul_write_msr_discard(
> unsigned int reg,
> uint64_t val,
> - struct x86_emulate_ctxt *ctxt)
> + struct x86_emulate_ctxt *ctxt,
> + bool explicit)
> {
> return X86EMUL_OKAY;
> }
> @@ -2427,9 +2428,10 @@ static int cf_check hvmemul_read_msr(
> static int cf_check hvmemul_write_msr(
> unsigned int reg,
> uint64_t val,
> - struct x86_emulate_ctxt *ctxt)
> + struct x86_emulate_ctxt *ctxt,
> + bool explicit)
> {
> - int rc = hvm_msr_write_intercept(reg, val, true);
> + int rc = hvm_msr_write_intercept(reg, val, explicit);
I've wondered whether we also want to rename the parameter of
hvm_msr_write_intercept() into something different than may_defer. It
feels weird to translate a parameter that denotes an explicit MSR
access into one that signals whether it's fine to defer the operation
or not.
Thanks, Roger.
