Re: [Xen-devel] [PATCH 1/7] x86emul: don't special case fetching the immediate of PUSH

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/08/16 14:26, Jan Beulich wrote:
>>>> On 11.08.16 at 14:58, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 11/08/16 13:03, Jan Beulich wrote:
>>> These immediates follow the standard patterns in all modes, so they're
>>> better fetched by the generic source operand handling code.
>>>
>>> To facilitate testing, instead of adding yet another of these pretty
>>> convoluted individual test cases, simply introduce another blowfish run
>>> with -mno-accumulate-outgoing-args (the additional -Dstatic is to
>>> keep the compiler from converting the calling convention to
>>> "regparm(3)", which I did observe it does).
>>>
>>> To make this introduction of a new blowfish pass (and potential further
>>> ones later one) have less impact on the readability of the final code,
>>> abstract all such "binary blob" executions via a table to iterate
>>> through.
>>>
>>> The resulting native code execution adjustment also uncovered a lack of
>>> clobbers on the asm() in the 64-bit case, which is being fixed at once.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> It would be helpful if the blob refactoring was in a separate patch to
>> the emulator bugfix.
> There's no bug fix here really, which is why I didn't think splitting
> it was strictly necessary. Can be done, but if at all possible I'd
> prefer not to spend extra time on disentangling the two, the more
> that in this case I'm convinced the reviewing of the split changes
> wouldn't be any easier than that of them being lumped together.
>
>>> @@ -983,20 +993,27 @@ int main(int argc, char **argv)
>>>               (regs.eax != 2) || (regs.edx != 1) )
>>>              goto fail;
>>>          printf("okay\n");
>>> -    }
>>>  
>>> -    printf("%-40s", "Testing blowfish native execution...");    
>>> -    asm volatile (
>>> +        if ( ctxt.addr_size != sizeof(void *) * CHAR_BIT )
>>> +            continue;
>> This wants to be at the top of the loop, or we will attempt to emulate
>> 64bit code in a 32bit build of the test before hitting this condition.
> Certainly not: We do want to test 32-bit code in a 64-bit build,
> and 64-bit code simply mustn't be included in a 32-bit build (hence
> the __x86_64__ conditionals inside the blobs[] initializer). Quite
> the opposite - not running 32-bit code natively here on a 64-bit
> system is solely because that would require extra setup (but not
> much value), not because it's impossible.

Ah yes.

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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