On 06/11/18 16:16, Jan Beulich wrote:
>>>> On 06.11.18 at 16:52, <andrew.coop...@citrix.com> wrote:
>> On 06/11/18 15:38, Jan Beulich wrote:
>>>>>> On 05.11.18 at 12:21, <andrew.coop...@citrix.com> wrote:
>>>> They are identical, so provide a single x86emul_cpuid() instead.
>>>>
>>>> As x86_emulate() now only uses the ->cpuid() hook for real CPUID
>> instructions,
>>>> the hook can be omitted from all special-purpose emulation ops.
>>> So I was expecting the hook to go away altogether, but I
>>> now realize that it can't because of some of the customization
>>> that's needed. That, in turn, means that the removal of the
>>> hook specification as per above will get us into problems the
>>> moment we need to check a feature that can't be taken
>>> straight from the policy object. I'm therefore unconvinced we
>>> actually want to go this far. It'll require enough care already
>>> to not blindly clone a new vcpu_has_...() wrongly from the
>>> many pre-existing examples in such a case. Thoughts?
>> All dynamic bits in CPUID are derived from other control state. e.g. we
>> check CR4.OSXSAVE, not CPUID.OSXSAVE. The other dynamic bits are APIC,
>> which comes from MSR_APIC_BASE, and OSPKE which also comes from CR4.
>>
>> In the emulator itself, I think it would be a bug if we ever had
>> vcpu_has_osxsave etc, because that isn't how pipelines actually behave.
>> The feature checks here are semantically equivalent to "do the
>> instruction decode and execution units have silicon to cope with these
>> instructions".
> I agree that vcpu_has_os* makes no sense, but the APIC bit for
> example isn't really pipeline / decoder related.
Correct, so why do you think APIC matters? All interaction with the
APIC is via its MMIO/MSR interface, rather than via dedicated instructions.
> However, one
> issue already might be that in order for bits in a (sub)leaf above
> (guest) limits to come out all clear, it is guest_cpuid() which cuts
> things off. Neither cpuid_featureset_to_policy() nor its inverse
> nor sanitise_featureset() look to zap fields above limits, in case
> they've been previously set to non-zero values. Am I overlooking
> something?
No - that is an aspect I'd overlooked, because the
DOMCTL_set_cpuid_policy work (which does this correctly) hasn't gone in yet.
I think I'll tweak recalculate_misc() to zero out beyond the max_subleaf
settings, because the intention was always that a flat cpuid_policy was
safe to use in this manner. I think there is an existing corner case
when trying to level basic.max_leaf to < 7, or extd.max_leaf to < 0x8000007.
> Furthermore I wouldn't exclude that we may need to look at a
> hypervisor or Viridian leaf at some point. The dynamic vPMU
> adjustments also look potentially problematic, if we were to
> emulate RDPMC (albeit they're DS-related only right now).
The only reason vPMU is dynamic is because we don't (yet) have a split
between default and max policies. Fixing this is on the todo list,
albeit behind a fairly long chain of dependencies.
> And then there's the dynamic exposing of MONITOR for PV; granted
> I don't expect us to ever emulate this for PV, but it shows the
> general issue.
That is to work around a deficiency in how Linux behaves. It isn't for
allowing PV guests to use MONITOR.
> Plus there's SYSCALL, the insn emulation of which
> currently doesn't check the (dynamically adjusted) CPUID bit.
No, nor should it. Intel's objection to the SYSCALL/SYSRET instructions
is mode based. (As a demonstration which proves the point of this
patch, if you hide the SYSCALL bit using masking, the instruction still
operates fine).
It seems I never got around to submitting my XSA-204 followup patch
which fixes many emulation bugs with SYS{CALL,RET,ENTER,EXIT}.
> And finally LWP, which again we can only hope we'll never have
> to emulate.
LWP doesn't exist any more, even on the hardware it used to exist on.
It was never implemented on Fam17h, and was removed from Fam15/16h in a
microcode update to make room to implement IBPB for Spectre v2 mitigations.
I recommend we purge the support completely.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
