Re: [Xen-devel] [PATCH 6/6] x86/emul: dedup hvmemul_cpuid() and pv_emul_cpuid()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.11.18 at 18:16, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 06/11/18 16:16, Jan Beulich wrote:
>>>>> On 06.11.18 at 16:52, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 06/11/18 15:38, Jan Beulich wrote:
>>>>>>> On 05.11.18 at 12:21, <andrew.cooper3@xxxxxxxxxx> 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.

It was a general example, not something that specifically matters here.

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

I certainly don't mind; I'd prefer though if such a withdrawal of
functionality came actually from AMD. Brian?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel