lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAdeq_+Ppuj8PxABvCT54phuXY021HxdayYyb68G3JjkQE0WQg@mail.gmail.com>
Date: Thu, 7 Aug 2025 16:03:56 +0800
From: hugo lee <cs.hugolee@...il.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, hpa@...or.com, x86@...nel.org, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Yuguo Li <hugoolli@...cent.com>
Subject: Re: [PATCH] KVM: x86: Synchronize APIC State with QEMU when irqchip=split

On Thu, Aug 7, 2025 Sean Christopherson wrote:
>
> On Wed, Aug 06, 2025, Yuguo Li wrote:
> > When using split irqchip mode, IOAPIC is handled by QEMU while the LAPIC is
> > emulated by KVM.  When guest disables LINT0, KVM doesn't exit to QEMU for
> > synchronization, leaving IOAPIC unaware of this change.  This may cause vCPU
> > to be kicked when external devices(e.g. PIT)keep sending interrupts.
>
> I don't entirely follow what the problem is.  Is the issue that QEMU injects an
> IRQ that should have been blocked?  Or is QEMU forcing the vCPU to exit unnecessarily?
>

This issue is about QEMU keeps injecting should-be-blocked
(blocked by guest and qemu just doesn't know that) IRQs.
As a result, QEMU forces vCPU to exit unnecessarily.

> > This patch ensure that KVM exits to QEMU for synchronization when the guest
> > disables LINT0.
>
> Please wrap at ~75 characters.

Thanks for reminding, will do in the next patch.

>
> > Signed-off-by: Yuguo Li <hugoolli@...cent.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h | 1 +
> >  arch/x86/kvm/lapic.c            | 4 ++++
> >  arch/x86/kvm/x86.c              | 5 +++++
> >  include/uapi/linux/kvm.h        | 1 +
> >  4 files changed, 11 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index f19a76d3ca0e..f69ce111bbe0 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -129,6 +129,7 @@
> >       KVM_ARCH_REQ_FLAGS(32, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> >  #define KVM_REQ_UPDATE_PROTECTED_GUEST_STATE \
> >       KVM_ARCH_REQ_FLAGS(34, KVM_REQUEST_WAIT)
> > +#define KVM_REQ_LAPIC_UPDATE              KVM_ARCH_REQ(35)
> >
> >  #define CR0_RESERVED_BITS                                               \
> >       (~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index 8172c2042dd6..65ffa89bf8a6 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -2329,6 +2329,10 @@ static int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
> >                       val |= APIC_LVT_MASKED;
> >               val &= apic_lvt_mask[index];
> >               kvm_lapic_set_reg(apic, reg, val);
> > +             if (irqchip_split(apic->vcpu->kvm) && (val & APIC_LVT_MASKED)) {
>
> This applies to much more than just LINT0, and for at least LVTPC and LVTCMCI,
> KVM definitely doesn't want to exit on every change.

Actually every masking on LAPIC should be synchronized with IOAPIC.
Because any desynchronization may cause unnecessary kicks
which rarely happens due to the well-behaving guests.
Exits here won't harm, but maybe only exit when LINT0 is being masked?
Since others unlikely cause exits.

>
> Even for LINT0, it's not obvious that "pushing" from KVM is a better option than
> having QEMU "pull" as needed.
>

QEMU has no idea when LINT0 is masked by the guest. Then the problem becomes
when it is needed to "pull". The guess on this could lead to extra costs.

> At the very least, this would need to be guarded by a capability, otherwise
> the new userspace exit would confuse existing VMMs (and probably result in the
> VM being terminated).

True, I'll add this protection.

>
>
> > +                     kvm_make_request(KVM_REQ_LAPIC_UPDATE, apic->vcpu);
> > +                     kvm_vcpu_kick(apic->vcpu);
>
> Why kick?  Cross-vCPU writes to LINT0 shouldn't be a thing, i.e. the kick should
> effectivel be a nop.

It is unnecessary, I will fix it in the next version.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ