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.coop...@citrix.com> but dependent on the PV guest changes being > dropped. Thanks. Jan
