On Wed, Aug 31, 2022, Maxim Levitsky wrote:
> On Wed, 2022-08-31 at 00:35 +0000, Sean Christopherson wrote:
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index 8209caffe3ab..3b6ef36b3963 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -168,7 +168,12 @@ static bool kvm_use_posted_timer_interrupt(struct kvm_vcpu *vcpu)
> >
> > static inline bool kvm_apic_map_get_logical_dest(struct kvm_apic_map *map,
> > u32 dest_id, struct kvm_lapic ***cluster, u16 *mask) {
> > - switch (map->mode) {
> > + switch (map->logical_mode) {
> > + case KVM_APIC_MODE_SW_DISABLED:
> > + /* Arbitrarily use the flat map so that @cluster isn't NULL. */
> > + *cluster = map->xapic_flat_map;
> > + *mask = 0;
> > + return true;
> > case KVM_APIC_MODE_X2APIC: {
> > u32 offset = (dest_id >> 16) * 16;
> > u32 max_apic_id = map->max_apic_id;
> > @@ -193,8 +198,10 @@ static inline bool kvm_apic_map_get_logical_dest(struct kvm_apic_map *map,
> > *cluster = map->xapic_cluster_map[(dest_id >> 4) & 0xf];
> > *mask = dest_id & 0xf;
> > return true;
> > + case KVM_APIC_MODE_MAP_DISABLED:
> > + return false;
> > default:
> > - /* Not optimized. */
> > + WARN_ON_ONCE(1);
>
> BTW unless I am mistaken, this warning is guest triggerable, and thus as you
> say when 'panic_on_warn=1', this will panic the host kernel.
If it's guest triggerable then it's a bug in this patch. The "default" case was
reachable with the old approach of OR-ing in bits, but the intent of this patch
is to fully enumerate all values of map->logical_mode and make the "default" case
impossible.
And I don't think it's reachable. The case statements are:
case KVM_APIC_MODE_SW_DISABLED:
case KVM_APIC_MODE_X2APIC:
case KVM_APIC_MODE_XAPIC_FLAT:
case KVM_APIC_MODE_XAPIC_CLUSTER:
case KVM_APIC_MODE_MAP_DISABLED:
default:
which covers all of the possible enum values.
enum kvm_apic_logical_mode {
KVM_APIC_MODE_SW_DISABLED,
KVM_APIC_MODE_XAPIC_CLUSTER,
KVM_APIC_MODE_XAPIC_FLAT,
KVM_APIC_MODE_X2APIC,
KVM_APIC_MODE_MAP_DISABLED,
};
The map is explicitly initialized to KVM_APIC_MODE_SW_DISABLED (to avoid relying
on KVM_APIC_MODE_SW_DISABLED==0)
new->logical_mode = KVM_APIC_MODE_SW_DISABLED;
so unless I've missed a "logical_mode |= ..." somewhere, reaching "default" should
be impossible.
