On 11/13/24 17:24, Doug Covelli wrote:
No worries, you're not hijacking :) The only reason is that it would
be more code for a seldom used feature and anyway with worse performance.
(To be clear, CR8 based accesses are allowed, but stores cause an exit
in order to check the new TPR against IRR. That's because KVM's API
does not have an equivalent of the TPR threshold as you point out below).
I have not really looked at the code but it seems like it could also
simplify things as CR8 would be handled more uniformly regardless of
who is virtualizing the local APIC.
Not much because CR8 basically does not exist at all (it's just a byte
in memory) with userspace APIC. So it's not easy to make it simpler, even
though it's less uniform.
That said, there is an optimization: you only get KVM_EXIT_SET_TPR if
CR8 decreases.
Also I could not find these documented anywhere but with MSFT's APIC our monitor
relies on extensions for trapping certain events such as INIT/SIPI plus LINT0
and SVR writes:
UINT64 X64ApicInitSipiExitTrap : 1; // WHvRunVpExitReasonX64ApicInitSipiTrap
UINT64 X64ApicWriteLint0ExitTrap : 1; // WHvRunVpExitReasonX64ApicWriteTrap
UINT64 X64ApicWriteLint1ExitTrap : 1; // WHvRunVpExitReasonX64ApicWriteTrap
UINT64 X64ApicWriteSvrExitTrap : 1; // WHvRunVpExitReasonX64ApicWriteTrap
There's no need for this in KVM's in-kernel APIC model. INIT and
SIPI are handled in the hypervisor and you can get the current
state of APs via KVM_GET_MPSTATE. LINT0 and LINT1 are injected
with KVM_INTERRUPT and KVM_NMI respectively, and they obey IF/PPR
and NMI blocking respectively, plus the interrupt shadow; so
there's no need for userspace to know when LINT0/LINT1 themselves
change. The spurious interrupt vector register is also handled
completely in kernel.
I realize that KVM can handle LINT0/SVR updates themselves but our
interrupt subsystem relies on knowing the current values of these
registers even when not virtualizing the local APIC. I suppose we
could use KVM_GET_LAPIC to sync things up on demand but that seems
like it might nor be great from a performance point of view.
Ah no, you're right---you want to track the CPU that has ExtINT enabled
and send KVM_INTERRUPT to that one, I guess? And you need the spurious
vector registers because writes can set the mask bit in LINTx, but
essentially you want to trap LINT0 changes.
Something like this (missing the KVM_ENABLE_CAP and KVM_CHECK_EXTENSION
code) is good, feel free to include it in your v2 (Co-developed-by
and Signed-off-by me):
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 5fb29ca3263b..b7dd89c99613 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -122,6 +122,7 @@
#define KVM_REQ_HV_TLB_FLUSH \
KVM_ARCH_REQ_FLAGS(32, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
#define KVM_REQ_UPDATE_PROTECTED_GUEST_STATE KVM_ARCH_REQ(34)
+#define KVM_REQ_REPORT_LINT0_ACCESS KVM_ARCH_REQ(35)
#define CR0_RESERVED_BITS \
(~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \
@@ -775,6 +776,7 @@ struct kvm_vcpu_arch {
u64 smi_count;
bool at_instruction_boundary;
bool tpr_access_reporting;
+ bool lint0_access_reporting;
bool xfd_no_write_intercept;
u64 ia32_xss;
u64 microcode_version;
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 88dc43660d23..0e070f447aa2 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1561,6 +1561,21 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
apic->divide_count));
}
+static void __report_lint0_access(struct kvm_lapic *apic, u32 value)
+{
+ struct kvm_vcpu *vcpu = apic->vcpu;
+ struct kvm_run *run = vcpu->run;
+
+ kvm_make_request(KVM_REQ_REPORT_LINT0_ACCESS, vcpu);
+ run->lint0_access.value = value;
+}
+
+static inline void report_lint0_access(struct kvm_lapic *apic, u32 value)
+{
+ if (apic->vcpu->arch.lint0_access_reporting)
+ __report_lint0_access(apic, value);
+}
+
static void __report_tpr_access(struct kvm_lapic *apic, bool write)
{
struct kvm_vcpu *vcpu = apic->vcpu;
@@ -2312,8 +2327,10 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
int i;
for (i = 0; i < apic->nr_lvt_entries; i++) {
- kvm_lapic_set_reg(apic, APIC_LVTx(i),
- kvm_lapic_get_reg(apic, APIC_LVTx(i)) | APIC_LVT_MASKED);
+ u32 old = kvm_lapic_get_reg(apic, APIC_LVTx(i));
+ kvm_lapic_set_reg(apic, APIC_LVTx(i), old | APIC_LVT_MASKED);
+ if (i == 0 && !(old & APIC_LVT_MASKED))
+ report_lint0_access(apic, old | APIC_LVT_MASKED);
}
apic_update_lvtt(apic);
atomic_set(&apic->lapic_timer.pending, 0);
@@ -2352,6 +2369,8 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
if (!kvm_apic_sw_enabled(apic))
val |= APIC_LVT_MASKED;
val &= apic_lvt_mask[index];
+ if (index == 0 && val != kvm_lapic_get_reg(apic, reg))
+ report_lint0_access(apic, val);
kvm_lapic_set_reg(apic, reg, val);
break;
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d0d3dc3b7ef6..2b039b372c3f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10879,6 +10879,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
kvm_vcpu_flush_tlb_guest(vcpu);
#endif
+ if (kvm_check_request(KVM_REQ_REPORT_LINT0_ACCESS, vcpu)) {
+ vcpu->run->exit_reason = KVM_EXIT_LINT0_ACCESS;
+ r = 0;
+ goto out;
+ }
if (kvm_check_request(KVM_REQ_REPORT_TPR_ACCESS, vcpu)) {
vcpu->run->exit_reason = KVM_EXIT_TPR_ACCESS;
r = 0;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 637efc055145..ec97727f9de4 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -178,6 +178,7 @@ struct kvm_xen_exit {
#define KVM_EXIT_NOTIFY 37
#define KVM_EXIT_LOONGARCH_IOCSR 38
#define KVM_EXIT_MEMORY_FAULT 39
+#define KVM_EXIT_LINT0_ACCESS 40
/* For KVM_EXIT_INTERNAL_ERROR */
/* Emulate instruction failed. */
@@ -283,6 +284,10 @@ struct kvm_run {
__u64 flags;
};
} hypercall;
+ /* KVM_EXIT_LINT0_ACCESS */
+ struct {
+ __u32 value;
+ } lint0_access;
/* KVM_EXIT_TPR_ACCESS */
struct {
__u64 rip;
For LINT1, it should be less performance critical; if it's possible
to just go through all vCPUs, and do KVM_GET_LAPIC to check who you
should send a KVM_NMI to, then I'd do that. I'd also accept a patch
that adds a VM-wide KVM_NMI ioctl that does the same in the hypervisor
if it's useful for you.
And since I've been proven wrong already, what do you need INIT/SIPI for?
Paolo
