Re: [PATCH v10 5/9] x86emul: support MOVDIR{I,64B} insns

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.05.2020 15:55, Andrew Cooper wrote:
> On 25/05/2020 15:28, Jan Beulich wrote:
>> Introduce a new blk() hook, paralleling the rmw() one in a certain way,
>> but being intended for larger data sizes, and hence its HVM intermediate
>> handling function doesn't fall back to splitting the operation if the
>> requested virtual address can't be mapped.
>>
>> Note that SDM revision 071 doesn't specify exception behavior for
>> ModRM.mod == 0b11; assuming #UD here.
> 
> Once again - I don't think this wants calling out.  That encoding space
> will be used for a new Grp at some point in the future, and be a
> different instruction.

Possible; without it spelled out one may also think (at least
for MOVDIRI) that the register-only encoding could be a
re-encoding of plain MOV. I'd prefer to keep it.

>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Reviewed-by: Paul Durrant <paul@xxxxxxx>
> 
> Acked-by: Andrew Cooper <andrew.cooper@xxxxxxxxxx>, although with one
> recommendation...

Thanks and ...

>> --- a/xen/include/public/arch-x86/cpufeatureset.h
>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
>> @@ -241,6 +241,8 @@ XEN_CPUFEATURE(AVX512_VPOPCNTDQ, 6*32+14
>>  XEN_CPUFEATURE(TSXLDTRK,      6*32+16) /*a  TSX load tracking 
>> suspend/resume insns */
>>  XEN_CPUFEATURE(RDPID,         6*32+22) /*A  RDPID instruction */
>>  XEN_CPUFEATURE(CLDEMOTE,      6*32+25) /*A  CLDEMOTE instruction */
>> +XEN_CPUFEATURE(MOVDIRI,       6*32+27) /*A  MOVDIRI instruction */
>> +XEN_CPUFEATURE(MOVDIR64B,     6*32+28) /*A  MOVDIR64B instruction */
> 
> I'd be tempted to leave these as 'a' for now, seeing as we have the ability.
> 
> These instructions aren't actually of any use for domains without PCI
> devices, and a "default" will be more migrateable as a consequence.

... okay, done.

Jan