On Wed, 2012-07-04 at 16:29 +0200, Alexander Graf wrote: > > +#ifdef CONFIG_KVM_BOOKE_HV > > +#define KVM_BOOKE_HV_MFSPR(reg, spr) \ > > + BEGIN_FTR_SECTION \ > > + mfspr reg, spr; \ > > + END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV) > > +#else > > +#define KVM_BOOKE_HV_MFSPR(reg, spr) > > +#endif > > Bleks - this is ugly. Do we really need to open-code the #ifdef here? > Can't the feature section code determine that the feature is disabled > and just always not include the code? You can't but in any case I don't see the point of the conditional here, we'll eventually have to load srr1 no ? We can move the load up to here in all cases or can't we ? If really not, we could have it inside DO_KVM and be done with it no ? > > + > > /* Exception prolog code for all exceptions */ > > -#define EXCEPTION_PROLOG(n, type, srr0, srr1, addition) \ > > +#define EXCEPTION_PROLOG(n, intnum, type, srr0, srr1, addition) \ > > mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \ > > mfspr r13,SPRN_SPRG_PACA; /* get PACA */ \ > > std r10,PACA_EX##type+EX_R10(r13); \ > > std r11,PACA_EX##type+EX_R11(r13); \ > > mfcr r10; /* save CR */ \ > > + KVM_BOOKE_HV_MFSPR(r11,srr1); \ > > + DO_KVM intnum,srr1; \ > > So if DO_KVM already knows srr1, why explicitly do something with it > the line above, and not in DO_KVM itself? Yeah that or just move things around in the prolog. > > addition; /* additional code for that exc. */ \ > > std r1,PACA_EX##type+EX_R1(r13); /* save old r1 in the PACA */ \ > > stw r10,PACA_EX##type+EX_CR(r13); /* save old CR in the PACA */ \ > > @@ -69,17 +82,21 @@ > > ld r1,PACA_MC_STACK(r13); \ > > subi r1,r1,SPECIAL_EXC_FRAME_SIZE; > > > > -#define NORMAL_EXCEPTION_PROLOG(n, addition) \ > > - EXCEPTION_PROLOG(n, GEN, SPRN_SRR0, SPRN_SRR1, addition##_GEN(n)) > > +#define NORMAL_EXCEPTION_PROLOG(n, intnum, addition) \ > > + EXCEPTION_PROLOG(n, intnum, GEN, SPRN_SRR0, SPRN_SRR1, \ > > We would we want to pass in 2 numbers? Let's please confine this onto > a single ID per interrupt vector. Either we use the hardcoded ones > available here in the KVM code or we use the KVM ones instead of the > hardcoded ones here. But not both please. Just because it's like that > on 32bit doesn't count as an excuse :). Right. Also I already objected to the explicit passing of the srr's anyway. Cheers, Ben. -- 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
