On Wed, 2024-07-24 at 11:05 -0700, Jim Mattson wrote:
> On Tue, Jun 25, 2024 at 4:56 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote:
> > KVM does not support changing the APIC's base address. Prior to commit
> > 3743c2f02517 ("KVM: x86: inhibit APICv/AVIC on changes to APIC ID or
> > APIC base"), it emitted a rate-limited warning about this. Now, it's
> > just silently broken.
> >
> > Use vcpu_unimpl() to complain about this unsupported operation. Even a
> > rate-limited error message is better than complete silence.
> >
> > Fixes: 3743c2f02517 ("KVM: x86: inhibit APICv/AVIC on changes to APIC ID or APIC base")
> > Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> > ---
> > Changes in v2:
> > * Changed format specifiers from "%#llx" to "%#x"
> > * Cast apic->base_address to unsigned int for printing
> >
> > arch/x86/kvm/lapic.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index acd7d48100a1..43ac05d10b2e 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -2583,6 +2583,9 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
> >
> > if ((value & MSR_IA32_APICBASE_ENABLE) &&
> > apic->base_address != APIC_DEFAULT_PHYS_BASE) {
> > + vcpu_unimpl(vcpu, "APIC base %#x is not %#x",
> > + (unsigned int)apic->base_address,
> > + APIC_DEFAULT_PHYS_BASE);
> > kvm_set_apicv_inhibit(apic->vcpu->kvm,
> > APICV_INHIBIT_REASON_APIC_BASE_MODIFIED);
> > }
> > --
> > 2.45.2.741.gdbec12cfda-goog
>
> Ping.
>
I think that we talked about this once, that nobody looks at these dmesg warnings,
its just a way for a malicious guest to fill up the host log (yes rate limit helps,
but slowly you can still fill it up),
but if you think that this is valuable, I am not against putting it back.
I wonder....
What if we introduce a new KVM capability, say CAP_DISABLE_UNSUPPORTED_FEATURES,
and when enabled, outright crash the guest when it attempts things like changing APIC base,
APIC IDs, and other unsupported things like that?
Then we can make qemu set it by default, and if users have to use an unsupported feature,
they could always add a qemu flag that will disable this capability.
Best regards,
Maxim Levitsky
