lkml.org 
[lkml]   [2022]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v8 6/9] KVM: x86: lapic: don't allow to change APIC ID unconditionally
From
Date
On Fri, 2022-04-15 at 14:39 +0000, Sean Christopherson wrote:
> On Mon, Apr 11, 2022, Zeng Guang wrote:
> > From: Maxim Levitsky <mlevitsk@redhat.com>
> >
> > No normal guest has any reason to change physical APIC IDs, and
> > allowing this introduces bugs into APIC acceleration code.
> >
> > And Intel recent hardware just ignores writes to APIC_ID in
> > xAPIC mode. More background can be found at:
> > https://lore.kernel.org/lkml/Yfw5ddGNOnDqxMLs@google.com/
> >
> > Looks there is no much value to support writable xAPIC ID in
> > guest except supporting some old and crazy use cases which
> > probably would fail on real hardware. So, make xAPIC ID
> > read-only for KVM guests.
>
> AFAIK, the plan is to add a capability to let userspace opt-in to a fully read-only
> APIC ID[*], but I haven't seen patches...
>
> Maxim?

Yep, I will start working on this pretty much today.

I was busy last 3 weeks stablilizing nested AVIC
(I am getting ~600,000 IPIs/s instead of ~40,000 in L2 VM with nested AVIC!),
with 700,000-900,000 IPIs native with AVIC,
almost bare metal IPI performance in L2!

(the result is from test which I will soon publish makes all
vCPUs send IPIs in round robin fashion, and a vCPU sends IPI only after
it received it from previous vCPU - the number is total
number of IPIs send on 8 vCPUs).


The fact that the dreadful AVIC errata dominates my testing again,
supports my feeling that I mostly fixed nested AVIC bugs.'
Tomorrow I'll send RFC v2 of the patches.


About read-only apic ID cap,
I have few questions before I start implementing it:

Paolo gave me recently an idea to make the APIC ID always read-only for
guest writes, and only allow host APIC ID changes (unless the cap is set).

I am kind of torn about it - assuming that no guest writes APIC ID this will work just fine
in empty logical sense, but if a guest actually writes an apic id,
while it will migrate fine to a newer KVM, but then after a reboot
it won't be able to set its APIC ID again.

On the other hand, versus fully unconditional read-only apic id,
that will support that very unlikely case if the hypervisor
itself is actually the one that for some reason changes the apic id,
from the initial value it gave.


In terms of what I need:

- In nested AVIC I strongly prefer read-only apic ids, and I can
make nested AVIC be conditional on the new cap.
IPI virtualization also can be made conditional on the new cap.


- I also would love to remove broken code from *non nested* AVIC,
which tries to support APIC ID change.

I can make non nested AVIC *also* depend on the new cap,
but that will technically be a regression, since this way users of
older qemu and new kernel will suddenly have their AVIC inhibited.

I don't know if that is such a big issue though because AVIC is
(sadly) by default disabled anyway.

If that is not possible the other way to solve this is to inhibit AVIC
as soon as the guest tries to change APIC ID.

- I would also want to remove the ability to relocate apic base,
likely depending on the new cap as well, but if there are objections
I can drop this. I don't need this, but it is just nice to do while we
are at it.


Paolo, Sean, and everyone else: What do you think?

Also:
Suggestions for the name for the new cap? Is this all right if
the same cap would make both apic id read only and apic base
(this is also something Paolo suggested to me)

Best regards,
Maxim Levitsky


>
> [*] https://lore.kernel.org/all/c903e82ed2a1e98f66910c35b5aabdcf56e08e72.camel@redhat.com
>


\
 
 \ /
  Last update: 2022-04-19 16:12    [W:0.077 / U:0.472 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site