Re: [Xen-devel] [PATCH 1/4] x86: remove CR reads from exit-to-guest path

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.01.18 at 12:01, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel