On 29.11.2022 21:36, Andrew Cooper wrote:
> On 29/11/2022 14:51, Jan Beulich wrote:
>> Up until f61685a66903 ("x86: remove defunct init/load/save_msr()
>> hvm_funcs") the check of the _rsvd field served as an error check for
>> the earlier hvm_funcs.save_msr() invocation. With that invocation gone
>> the check makes no sense anymore. While dropping it also merge the two
>> paths setting "err" to -ENXIO.
>>
>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>> ---
>> We could go further here, removing the local "err" variable altogether,
>> by using "return -ENXIO". Thoughts.
>
> 'err' is a non-standard variable name, so yeah, why not.
Okay, I'll make a v2 for this.
> That said, the current code has a split loop checking the incoming _rsvd
> fields in a first pass, and then calling guest_wrmsr() on the second
> pass. This was also made pointless by the identified changeset, so the
> two loops ought to be merged.
Not really, no - it would violate the "Checking finished" comment (but
of course we could also delete that one), but I'd also prefer to keep
checking for all errors we can check for early _before_ starting to
make any changes to the guest. Therefore if you really wanted that, I
guess you'd need to make a follow-on change yourself, with a convincing
justification (I wouldn't outright object to such a change, but I
probably also wouldn't ack it, leaving that to someone else).
Jan
