On Tue, Sep 18, 2012 at 11:07 AM, Catalin Marinas <catalin.marinas@xxxxxxx> wrote: > On Tue, Sep 18, 2012 at 04:05:13PM +0100, Christoffer Dall wrote: >> On Tue, Sep 18, 2012 at 10:06 AM, Catalin Marinas >> <catalin.marinas@xxxxxxx> wrote: >> > On 18 September 2012 13:47, Will Deacon <will.deacon@xxxxxxx> wrote: >> >> On Sat, Sep 15, 2012 at 04:34:43PM +0100, Christoffer Dall wrote: >> >>> +#define L_PTE2_SHARED L_PTE_SHARED >> >>> +#define L_PTE2_READ (_AT(pteval_t, 1) << 6) /* HAP[0] */ >> >>> +#define L_PTE2_WRITE (_AT(pteval_t, 1) << 7) /* HAP[1] */ >> >> >> >> This is actually HAP[2:1], not HAP[1:0]. Also, can you follow what we do for >> >> stage 1 translation and name these RDONLY and WRONLY (do you even use >> >> that?). >> > >> > We can't use RDONLY as this would have value 0 as the HAP attributes >> > (stage 2 overriding stage 1 translation attributes). Unless you add 4 >> > definitions like NOACCESS, RDONLY, WRONLY and RDWR to cover all the >> > bit combinations. >> > >> >>> +#define L_PTE2_NORM_WB (_AT(pteval_t, 3) << 4) /* MemAttr[3:2] */ >> >>> +#define L_PTE2_INNER_WB (_AT(pteval_t, 3) << 2) /* MemAttr[1:0] */ >> >> >> >> Another minor nit: PTE2 looks awful. Maybe L_PTE_HYP_* instead? >> > >> > L_PTE_HYP may be confused with the Stage 1 Hyp translation which is >> > different from the guest Stage 2. >> >> exactly, it's misleading, how about L_PTE_STAGE2, a little verbose, >> but clear...? > > I don't mind any (apart from L_PTE_HYP_ would be confusing) for stage 2. > You could just use S2 to make it shorter. > I'm good with that, done. -Christoffer -- 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
