On Wed, Jul 24, 2024 at 11:13 AM Maxim Levitsky <mlevitsk@xxxxxxxxxx> wrote:
>
> 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.
Funny that you mention this. I'm about to send out another change to
curtail some ratelimited spam that *quickly* fills up the host log.
> 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.
Alternatively, why not devise a way to inform userspace that the guest
has exercised an unsupported feature? Unless you're a hobbyist working
on your desktop, kernel messages are a *terrible* mechanism for
communicating with the end user.
> Best regards,
> Maxim Levitsky
>
