Re: [PATCH 1/2] x86: CPU-qualify {disp16} / {disp32}

[Jan Beulich <jbeulich@suse.com>]

On 07.11.2023 16:50, Cui, Lili wrote:
>> Subject: Re: [PATCH 1/2] x86: CPU-qualify {disp16} / {disp32}
>>
>> On 07.11.2023 16:06, Cui, Lili wrote:
>>>> Subject: [PATCH 1/2] x86: CPU-qualify {disp16} / {disp32}
>>>>
>>>> {disp16} is invalid to use in 64-bit mode, while {disp32} is invalid
>>>> to use on
>>>> pre-386 CPUs. The latter, also affecting other (real) prefixes,
>>>> further requires that like for insns we fully check the CPU flags;
>>>> till now only Cpu64/CpuNo64 were taken into consideration.
>>>> ---
>>>> While this is consistent with i386_index_check() diagnosing wrong
>>>> {dispN} use as an error, that and the change here aren't consistent
>>>> with documentation saying "prefer", suggesting such prefixes - like
>>>> {rex}, albeit even there not fully consistent, seeing the error
>>>> md_assemble() generates when used with VEX/XOP/EVEX encoded insns -
>>>> are ignored when impossible to fulfill. Otoh the change here is
>>>> consistent with {rex} being refused (rather than ignored) outside of 64-bit
>> mode.
>>>>
>>>>>>>         {rex} vmovaps %xmm7,%xmm2
>>>>>>>         {rex} vmovaps %xmm17,%xmm2
>>>>>>>         {rex} rorx $7,%eax,%ebx
>>>>>>> +       {rex2} vmovaps %xmm7,%xmm2
>>>>>>
>>>>>> Right, but please see my "optional vs required" comment in the
>>>>>> pseudo- prefix related patch I did send earlier today. I question
>>>>>> the correctness of the {rex} related check here, which would then
>>>>>> extend to the
>>>> {rex2} one as well.
>>>>>>
>>>>>
>>>>> A REX byte that is immediately followed by a legacy prefix byte
>>>>> (LOCK, REPE, REPNE, OSIZE override, ASIZE override, or segment
>>>>> overrides) or
>>>> another REX byte is ignored and behaves as if it does not exist
>>>> (except for contributing to the instruction length), but in this case I think
>> it's correct.
>>>>
>>>> I'm afraid I can't relate this to the aspect I raised above. Perhaps
>>>> better to discuss in the context of the patch that I sent (and that I
>>>> mentioned above;
>>>> "x86: CPU-qualify {disp16} / {disp32}"). You did reply to the patch,
>>>> but you didn't reply to the more detailed description of the issue
>>>> (which I did refer to above).
>>>>
>>>
>>> I think the vex instruction is not preceded by these legacy prefix bytes
>> (LOCK, REPE, REPNE, OSIZE override, ASIZE override, or segment override), so
>> the vex instruction should not ignore the rex bytes.
>>
>> But you realize the difference between "rex" and "{rex}" as a prefix?
>> What you describe looks to talk about the former, whereas I've been talking
>> about the latter.
>>
> Oh, got it, I mixed them up, thanks for pointing it out. 
> Suppose the customer wants to insert a byte {rex} in front of VEX insn to achieve a certain byte alignment. Would it be better for us to report an error? Or just ignore it. I don't know if this really exists.

{rex} won't guarantee to insert anything, as per its documentation. If one
strictly wants a REX prefix, "rex" needs to be used.

Jan