Re: [PATCH] x86emul: allow ->write_msr() to distinguish origins of writes

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.01.2026 11:00, Roger Pau Monné wrote:
> 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 <andrew.cooper3@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

>> ---
>> 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.

Unreachable and dead code are different things to Misra, though. It is my
understanding that we deliberately permit constructs reducing to "if (0)"
in certain configurations, relying on DCE: There's then no generated code
for the construct, and hence nothing that cannot be reached. The
situation is different for a parameter that has no use: Its removal (in
the specific configuration) wouldn't alter the behavior. Hence the
parameter and all arguments passed for it are "dead".

>> @@ -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.

I did think the same, just that - considering all use sites - I couldn't
even come close to thinking of some sensible new name.

Jan