Re: [PATCH 2/9] x86emul: support WRMSRNS

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.04.2023 23:29, Andrew Cooper wrote:
> On 04/04/2023 3:50 pm, Jan Beulich wrote:
>> This insn differs from WRMSR solely in the lack of serialization. Hence
>> the code used there can simply be used here as well, plus a feature
>> check of course.
> 
> I have a feeling this is a bit stale now that 0f01.c has moved into a
> separate file ?

Hmm, no, not really. This code was written long after the split anyway.
"Used here as well" is meant as "copy that code", not "goto ...".

>>  As there's no other infrastructure needed beyond
>> permitting the insn for PV privileged-op emulation (in particular no
>> separate new VMEXIT) we can expose the insn to guests right away.
> 
> There are good reasons not to expose this to PV guests.
> 
> The #GP fault to trigger emulation is serialising, so from the guest's
> point of view there is literally no difference between WRMSR and WRMSRNS.
> 
> All code is going to need to default to WRMSR for compatibility reasons,
> so not advertising WRMSRNS will save the guest going through and
> self-modifying itself to use a "more efficient" instruction which is not
> actually more efficient.

Well, sure, I can drop that again. I don't view "self-modifying itself"
as meaningful wastage. Plus I'd consider it reasonable to expose the
feature by default only to HVM, while permitting (non-default) exposure
to PV (in which case the emul-priv-op.c change would want retaining),
except that we can't express this afaict. Even without exposing at all
I'd consider it kind of reasonable to allow use of the insn, in case a
guest infers its availability (or simply knows from executing raw CPUID).

> The emulation implementation is fine, so Reviewed-by: Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx> but dependent on the PV guest changes being
> dropped.

Thanks.

Jan