Re: [Xen-devel] [PATCH] x86emul: correct stub invocation constraints

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.04.17 at 10:07, <JBeulich@xxxxxxxx> wrote:
>>>> On 26.04.17 at 16:01, <boris.ostrovsky@xxxxxxxxxx> wrote:
>> On 04/25/2017 05:04 AM, Jan Beulich wrote:
>>> Stub invocations need to have the space the stub occupies as an input,
>>> to prevent the compiler from re-ordering (or omitting) writes to it.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>> @@ -837,7 +837,8 @@ do{ asm volatile (
>>>                     ".popsection\n\t"                                    \
>>>                     _ASM_EXTABLE(.Lret%=, .Lfix%=)                       \
>>>                     : [exn] "+g" (res_), constraints,                    \
>>> -                     [stub] "rm" (stub.func) );                         \
>>> +                     [stub] "rm" (stub.func),                           \
>>> +                     "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.ptr) );   \
>>>      if ( unlikely(~res_.raw) )                                          \
>>>      {                                                                   \
>>>          gprintk(XENLOG_WARNING,                                         \
>>> @@ -853,7 +854,8 @@ do{ asm volatile (
>>>  #else
>>>  # define invoke_stub(pre, post, constraints...)                         \
>>>      asm volatile ( pre "\n\tcall *%[stub]\n\t" post                     \
>>> -                   : constraints, [stub] "rm" (stub.func) )
>>> +                   : constraints, [stub] "rm" (stub.func),              \
>>> +                     "m" (*(uint8_t(*)[MAX_INST_LEN + 1])stub.buf) )
>>>  #endif
>>>  
>>>  #define emulate_stub(dst, src...) do {                                  \
>>>
>> 
>> 
>> This breaks on old compilers:
>> 
>> FC-64
>> <build@build-mk2:~/xtt-x86_64/bootstrap/xen.git/tools/fuzz/x86_instruction_em
>> ulator>
>> gcc --version
>> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
> 
> Btw., I've just realized that I did use an old gcc only on the
> hypervisor build. Do you see the same issue there, or is this tools
> side specific?

And now that I've extracted it into a smaller example and thus was
able to try, I can see the issue with 4.3.x. The problem is that with

struct stub {
    unsigned long addr;
    void *ptr;
    unsigned char buf[16];
};

void test(const struct stub*ptr) {
        asm("" :: "m" (*(unsigned char(*)[16])ptr->addr));
        asm("" :: "m" (*(unsigned char(*)[16])ptr->ptr));
        asm("" :: "m" (*(unsigned char(*)[16])ptr->buf));
        asm("" :: "m" (*(unsigned char(*)[16])&ptr->buf));
        asm("" :: "m" (*(unsigned char(*)[16])&ptr->buf[0]));
}

none of the last three work, so we'll have to resort to using the
first. I'll have to verify that this is good enough for the case
where I did actually observe things to break without the extra
constraint (with a not yet submitted patch).

Jan


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