On Fri, 2022-02-25 at 14:56 +0000, David Woodhouse wrote: > On Fri, 2022-02-25 at 16:46 +0200, Maxim Levitsky wrote: > > On Fri, 2022-02-25 at 16:22 +0800, Zeng Guang wrote: > > > From: Maxim Levitsky < > > > mlevitsk@xxxxxxxxxx > > > > > > 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@xxxxxxxxxx/ > > > > > > > > > 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. > > > > > > Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx> > > > Signed-off-by: Zeng Guang <guang.zeng@xxxxxxxxx> > > > > Assuming that this is approved and accepted upstream, > > that is even better that my proposal of doing this > > when APICv is enabled. > > > > Since now apic id is always read only, now we should not > > forget to clean up some parts of kvm like kvm_recalculate_apic_map, > > which are not needed anymore > > Can we also now optimise kvm_get_vcpu_by_id() so it doesn't have to do > a linear search over all the vCPUs when there isn't a 1:1 > correspondence with the vCPU index? I don't think so since vcpu id can still be set by userspace to anything, and this is even used to encode topology in it. However a hash table can still be used there to speed it up regardless of read-only apic id IMHO. Or, even better than a hash table, I see that KVM already limits vcpu_id to KVM_MAX_VCPUS * 4 with a comment that only two extra bits of topology are used: "In the worst case, we'll need less than one extra bit for the * Core ID, and less than one extra bit for the Package (Die) ID, * so ratio of 4 should be enough" Thus, we could in theory standardize location of those bits in apic_id (even with a new KVM extension and do linear search for legacy userspace), and then just mask/shift the topology bits. The kvm extension would be defining how many low (or high?) bits of vcpu_id are topology bits. Best regards, Maxim Levitsky
