On Wed, May 18, 2016 at 05:46:55PM +0100, Andre Przywara wrote: > Hi, > > >>> + > >>> +/* generate a mask that covers 1024 interrupts with <b> bits per IRQ */ > >> > >> Hmmm. I'd appreciate some additional comments, specially when it comes > >> to the various restrictions. May I'd suggest something like: > >> > >> /* > >> * Generate a mask that covers the number of bytes required to address > >> * up to 1024 interrupts, each represented by <b> bits. This assumes > >> * that <b> is a power of two. > >> * > >> * ilog2(b) + ilog2(1024) is the number of bits required to bit-address > >> * 1024 interrupts, each represented by b bits. Minus ilog2(8) converts > >> * this to a byte address. > > > > So I'm guessting this is a rewrite of ilog2( (b * 1024) / 8), but I'm > > stupid enough to not understand our use of logarithms here. Can someone > > remind me whatever I forgot at CS 101? > > I guess it was more me not seeing the wood for the trees here: > Indeed doing the multiplication first and then calling ilog2 seems to > make more sense. Also I was thinking: Isn't > "GENMASK_ULL(ilog2(n) - 1, 0)" the same as "n - 1"? if there's no integer rounding taking place with ilog2 (iow. n is a power of 2) then yes, I believe it is. > > So can't we just write: > > #define VGIC_ADDR_IRQ_MASK(bpi) (((bpi) * 1024 / 8) - 1) that certianly all of the sudden feels intuitive. > > Proven by enumeration - over the values we use ;-) > > I'd keep the first paragraph of Marc's comment above then, but we can > avoid mentioning Advanced Maths textbooks about binary logarithmic ;-) > Haha, you saved my day with that comment. I feel slightly less idiotic now, yes, let's call it advanced quantum math or something instead of CS 101 :) -Christoffer > > >> */ > >> > >>> +#define VGIC_ADDR_IRQ_MASK(b) GENMASK_ULL(ilog2(b) + ilog2(1024) - \ > >>> + ilog2(BITS_PER_BYTE) - 1, 0) > >> > >> /* > >> * Convert a base address into a base interrupt (each interrupt > >> * represented by <bits> bits. This assumes that <bits> is a power > >> * of two, that <addr> both part of a memory region aligned on a > > > > did you mean '<addr> *is* both part of' ? > > > >> * <b> bits boundary, and itself aligned on that same boundary > >> * (for regions that describe an interrupt with more than a single > >> * byte of data). > >> */ > >> > > > > In any case, thanks for the commentary, I was faily lost here. > > -- 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
