On 06.12.2022 05:33, Demi Marie Obenour wrote: > --- a/xen/arch/x86/include/asm/processor.h > +++ b/xen/arch/x86/include/asm/processor.h > @@ -92,13 +92,33 @@ > X86_EFLAGS_NT|X86_EFLAGS_DF|X86_EFLAGS_IF| \ > X86_EFLAGS_TF) > > +/* Individual entries in IA32_CR_PAT */ > +#define MSR_PAT_UC _AC(0x00, ULL) > +#define MSR_PAT_WC _AC(0x01, ULL) > +#define MSR_PAT_RESERVED_1 _AC(0x02, ULL) > +#define MSR_PAT_RESERVED_2 _AC(0x03, ULL) > +#define MSR_PAT_WT _AC(0x04, ULL) > +#define MSR_PAT_WP _AC(0x05, ULL) > +#define MSR_PAT_WB _AC(0x06, ULL) > +#define MSR_PAT_UCM _AC(0x07, ULL)
This would be the at least 4th instance of these constants in our tree (see xen/arch/x86/include/asm/mtrr.h, xen/include/public/domctl.h, and xen/include/public/hvm/dm_op.h). Since they're part of the public interface, I'm inclined to suggest to use those. If that faces opposition, the next best approach would apparently be to move mtrr.h's into x86-defns.h (including the largely identical MTRR values), thus then also allowing hvmloader sources (which currently open-code the MTRR values in cacheattr.c). > /* > * Host IA32_CR_PAT value to cover all memory types. This is not the default > * MSR_PAT value, and is an ABI with PV guests. > */ > -#define XEN_MSR_PAT _AC(0x050100070406, ULL) > +#define XEN_MSR_PAT (MSR_PAT_WB << 0x00 | \ > + MSR_PAT_WT << 0x08 | \ > + MSR_PAT_UCM << 0x10 | \ > + MSR_PAT_UC << 0x18 | \ > + MSR_PAT_WC << 0x20 | \ > + MSR_PAT_WP << 0x28 | \ > + MSR_PAT_UC << 0x30 | \ > + MSR_PAT_UC << 0x38 | \ > + 0) > > #ifndef __ASSEMBLY__ > +_Static_assert(XEN_MSR_PAT == 0x050100070406ULL, > + "wrong XEN_MSR_PAT breaks PV guests"); I'm afraid this won't compile on all gcc versions we support; see xen/include/xen/lib.h's conditional use thereof for BUILD_BUG_ON(). You really want to use BUILD_BUG_ON() here instead of partly open- coding it. (Yes, it requires to be put where a statement can be put, not just a declaration, but the check doesn't strictly need to live in a header file, so that's reasonably easy to accommodate for.) Jan
