Re: [Xen-devel] [PATCH] x86/setup: Correct register clobbers for the asm statement when resyncing the stack

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.10.14 at 12:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 21/10/14 09:44, Jan Beulich wrote:
>>>>> On 20.10.14 at 19:30, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -836,6 +836,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>>              l2_pgentry_t *pl2e;
>>>              uint64_t load_start;
>>>              int i, j, k;
>>> +            long _discard;
>> I can't see why you couldn't use i, j, or k for the discarding purposes.
> 
> A parameter with the name "_discard" is far more explicit about its
> purpose when read as part of the output parameter list.
> 
> I could switch to i and leave a comment by the parameters if you insist.

I don't strictly insist, but I dislike variables to be added without real
need. At the very least the leading underscore needs to be dropped
as needlessly being in conflict with C library standard naming rules.

>>> @@ -902,11 +903,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>>                  "movq %%cr4,%%rsi ; "
>>>                  "andb $0x7f,%%sil ; "
>>>                  "movq %%rsi,%%cr4 ; " /* CR4.PGE == 0 */
>>> -                "movq %0,%%cr3    ; " /* CR3 == new pagetables */
>>> +                "movq %6,%%cr3    ; " /* CR3 == new pagetables */
>>>                  "orb $0x80,%%sil  ; "
>>>                  "movq %%rsi,%%cr4   " /* CR4.PGE == 1 */
>>> -                : : "r" (__pa(idle_pg_table)), "S" (cpu0_stack),
>>> -                "D" (__va(__pa(cpu0_stack))), "c" (STACK_SIZE / 8) : 
> "memory" );
>>> +                : "=&S"(_discard), "=&D"(_discard), "=&c"(_discard)
>>> +                : "0"(cpu0_stack), "1"(__va(__pa(cpu0_stack))),
>>> +                  "2"(STACK_SIZE / 8), "r"(__pa(idle_pg_table))
>>> +                : "memory" );
>> Among the inputs, please put the one actively used by number first,
>> or convert to labeled operands.
> 
> As far as I can tell, it is impossible to get the idle pagetables to
> parameter %0 as it is not present in the output list.  I have switched
> to a named parameter.

I didn't say make it %0 again (and specifically said "among the
inputs"), but it could have been moved ahead of all the other
inputs.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel