Messages in this thread | | | Date | Fri, 30 Jan 2015 16:14:42 +0100 | From | Radim Krčmář <> | Subject | Re: [PATCH 8/8] KVM: x86: simplify kvm_apic_map |
| |
2015-01-30 10:18+0100, Paolo Bonzini: > On 29/01/2015 22:48, Radim Krčmář wrote: > > +static inline bool > > +apic_logical_id(struct kvm_apic_map *map, u32 ldr, u16 *cid, u16 *lid) > > { > > + switch (map->mode) { > > + case KVM_APIC_MODE_XAPIC_FLAT: > > + *cid = 0; > > + *lid = ldr & 0xff; > > + return true; > > + case KVM_APIC_MODE_XAPIC_CLUSTER: > > + *cid = (ldr >> 4) & 0xf; > > + *lid = ldr & 0xf; > > + return true; > > + case KVM_APIC_MODE_X2APIC: > > + *cid = ldr >> 16; > > + *lid = ldr & 0xffff; > > + return true; > > + } > > We need some optimization here. You can make the defines equal to the > size of the lid: CLUSTER = 1 << 3, FLAT = 1 << 2, X2APIC = 1 << 4: > > BUILD_BUG_ON(...FLAT != 4); > BUILD_BUG_ON(...CLUSTER != 8);
(Swapped.)
> BUILD_BUG_ON(...X2APIC != 16);
(Check the mode and return false here.)
> lid_bits = mode; > cid_bits = mode & (16 | 4); > lid_mask = (1 << lid_bits) - 1; > cid_mask = (1 << cid_bits) - 1; > > *cid = (ldr >> lid_bits) & cid_mask; > *lid = ldr & lid_mask;
Would jump predictor fail on the switch? Or is size of the code that important? This code is shorter, but is going to execute far more operations, so I think it would be slower ... (And harder to read.)
> Please move this to lapic.c since you are at it.
Ok.
| |