On Wed, Sep 27, 2023 at 4:18 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> Gah, sorry. This is why I usually inline patches, I forget to actually att=
> ach
> the darn things 50% of the time.
Thanks for that, and apologies for only just getting around to taking
a closer look. Out of the box, this doesn't build on macOS at least,
as that's missing <linux/types.h>. I tried going down the rabbit hole
of pulling that header and its various transitive dependencies in from
the Linux tree, but ended up in a horrible mess where those headers
try to define things like bool, which libcflat has already pulled in
from the standard system headers (on Linux I suspect the libc #include
guards match up with the stuff in <linux/*>, so there's no issue).
On macOS, the problem is easy to resolve via a cut-down types.h with a
minimal set of definitions:
#include <libcflat.h>
typedef u8 __u8;
typedef u32 __u32;
typedef u64 __u64;
typedef s64 __s64;
…but I assume that breaks things on Linux. I'm thinking something like
this might work:
#if __LINUX__
#include_next <linux/types.h>
#else
[minimal types.h definitions]
#endif
But I'm unsure if that's really the direction you'd want to go with
this? (And I still need to set myself up with a dev environment on a
physical Linux box that I can test this all on.)
Another option might be a symlinked linux/types.h created by
./configure if not running on Linux?
On the substance of the patch itself:
> unsigned long a0 = 0xFFFFFFFF, a1 = 0, a2 = 0xFFFFFFFF, a3 = 0x0;
> - if (!test_device_enabled())
> + if (!this_cpu_has(X86_FEATURE_KVM_PV_SEND_IPI))
So this check will (erroneously IMO) succeed if we're running on a
non-KVM hypervisor which happens to expose a flag at bit 11 of ecx on
CPUID leaf 0x40000001 page 0, right? With this in mind, your earlier
idea seems better:
if (!is_hypervisor_kvm() ||
!this_cpu_has(X86_FEATURE_KVM_PV_SEND_IPI)) {
So I've gone ahead and made an attempt at fixing up your draft
implementation of is_hypervisor_kvm() below.
The partial struct memcmp in get_hypervisor_cpuid_base is a bit icky;
I'm not sure if that's worth fixing up at the cost of readability.
Thoughts?
(I've attached the full set of WIP changes on top of yours as another
patch. Feel free to squash it all into one if you decide to run with
it.)
Thanks,
Phil
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 7a7048f9..3d3930c8 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -240,6 +240,7 @@ static inline bool is_intel(void)
#define X86_FEATURE_XSAVE (CPUID(0x1, 0, ECX, 26))
#define X86_FEATURE_OSXSAVE (CPUID(0x1, 0, ECX, 27))
#define X86_FEATURE_RDRAND (CPUID(0x1, 0, ECX, 30))
+#define X86_FEATURE_HYPERVISOR (CPUID(0x1, 0, ECX, 31))
#define X86_FEATURE_MCE (CPUID(0x1, 0, EDX, 7))
#define X86_FEATURE_APIC (CPUID(0x1, 0, EDX, 9))
#define X86_FEATURE_CLFLUSH (CPUID(0x1, 0, EDX, 19))
@@ -286,7 +287,8 @@ static inline bool is_intel(void)
#define X86_FEATURE_VNMI (CPUID(0x8000000A, 0, EDX, 25))
#define X86_FEATURE_AMD_PMU_V2 (CPUID(0x80000022, 0, EAX, 0))
-#define X86_FEATURE_KVM_PV_SEND_IPI (CPUID(KVM_CPUID_FEATURES, 0,
EAX, KVM_FEATURE_PV_SEND_IPI))
+#define X86_FEATURE_KVM_PV_SEND_IPI \
+ (CPUID(KVM_CPUID_FEATURES, 0, EAX, KVM_FEATURE_PV_SEND_IPI))
static inline bool this_cpu_has(u64 feature)
{
@@ -303,6 +305,40 @@ static inline bool this_cpu_has(u64 feature)
return ((*(tmp + (output_reg % 32))) & (1 << bit));
}
+static inline u32 get_hypervisor_cpuid_base(const char *sig)
+{
+ u32 base;
+ struct cpuid signature;
+
+ if (!this_cpu_has(X86_FEATURE_HYPERVISOR))
+ return 0;
+
+ for (base = 0x40000000; base < 0x40010000; base += 0x100) {
+ signature = cpuid(base);
+
+ if (!memcmp(sig, &signature.b, 12))
+ return base;
+ }
+
+ return 0;
+}
+
+static inline bool is_hypervisor_kvm(void)
+{
+ u32 base = get_hypervisor_cpuid_base(KVM_SIGNATURE);
+
+ if (!base)
+ return false;
+
+ /*
+ * Require that KVM be placed at its default base so that macros can be
+ * used to query individual KVM feature bits.
+ */
+ assert_msg(base == KVM_CPUID_SIGNATURE,
+ "Expect KVM at its default cpuid base (now at: 0x%x)", base);
+ return true;
+}
+
struct far_pointer32 {
u32 offset;
u16 selector;
Attachment:
0002-x86-apic-test_pv_ipi-cpuid-check-refinements.patch
Description: Binary data
