On 05/24/2012 01:00 AM, Thomas Gleixner wrote: > Thought more about that. > > We have a clear distinction between HW accessed data and software > accessed data. > > If I look at TPR then it is special cased already and it does: > > case APIC_TASKPRI: > report_tpr_access(apic, false|true); > /* fall thru */ > > And the fall through is using the general accessor for all not special > cased registers. > > So all you have to do is > > case APIC_TASKPRI: > report_tpr_access(apic, false|true); > + return access_mapped_reg(...); > > Instead of the fall through. > > So there is no synchronizing back and forth problem simply because you > already have a special case for that register. > > I know you'll argue that the tpr reporting is a special hack for > windows guests, at least that's what the changelog tells. > > But even if we have a few more registers accessed by hardware and if > they do not require a special casing, I really doubt that the overhead > of special casing those few regs will be worse than not having the > obvious optimization in place. > > And looking deeper it's a total non issue. The apic mapping is 4k. The > register stride is strictly 0x10. That makes a total of 256 possible > registers. > > So now you have two possibilites: > > 1) Create a 256 bit == 64byte bitfield to select the one or the other > representation. > > The overhead of checking the bit is not going to be observable. > > 2) Create a 256 function pointer array == 2k resp. 1k (64 / 32bit) > > That's not a lot of memory even if you have to maintain two > separate variants for read and write, but it allows you to get rid > of the already horribly compiled switch case in apic_read/write and > you'll get the optional stuff like report_tpr_access() w/o extra > conditionals just for free. > > An extra goodie is that you can catch any access to a non existing > register which you now just silently ignore. And that allows you > to react on any future hardware oddities without adding a single > runtime conditional. > > This is stricly x86 and x86 is way better at dealing with indirect > calls than with the mess gcc creates compiling those switch case > constructs. > > So I'd go for that and rather spend the memory and the time in > setting up the function pointers on init/ioctl than dealing with > the inconsistency of HW/SW representation with magic hacks. > I like the bitmap version, it seems very lightweight. But by itself it doesn't allow us to use bitmap_weight (or the other bitmap accessors), unless you assume beforehand that those registers will never be in the hardware-layout region. (you also need extra code for copying the APIC state to and from userspace; right now we just memcpy the APIC page) -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
