>>> On 30.01.18 at 12:01, <andrew.coop...@citrix.com> wrote: > On 23/01/18 10:36, Jan Beulich wrote: >> --- a/xen/include/asm-x86/asm_defns.h >> +++ b/xen/include/asm-x86/asm_defns.h >> @@ -206,13 +206,12 @@ void ret_from_intr(void); >> #define ASM_STAC ASM_AC(STAC) >> #define ASM_CLAC ASM_AC(CLAC) >> >> -.macro write_cr3 val:req, tmp1:req, tmp2:req >> - mov %cr4, %\tmp1 >> - mov %\tmp1, %\tmp2 >> - and $~X86_CR4_PGE, %\tmp1 >> - mov %\tmp1, %cr4 >> +.macro write_cr3 val:req, cr4:req, tmp:req >> + mov %\cr4, %\tmp >> + and $~X86_CR4_PGE, %\cr4 >> + mov %\cr4, %cr4 > > This is confusing to read; It took me a while to work out why it > assembled in the first place. Given that there are only two instances > of write_cr3 now, I'd suggest expanding this in the two sites. It will > also make it clear which registers have real values and which are > temporary, which isn't clear from the current callsites.
Hmm, part of the reason I didn't want to drop the macro altogether is its similarity with write_cr3(), so it would turn up in grep-s for that one. How about switching the two use sites to specify named arguments to the macro? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
