On 11.06.2025 13:32, Andrew Cooper wrote:
> On 11/06/2025 11:44 am, Jan Beulich wrote:
>> Address size is entirely irrelevant to operand size determination; For
>> VMREAD and VMWRITE outside of 64-bit mode operand size is 32 bits, while
>> in 64-bit mode it's (naturally) 64 bits. For all other insns it's 64
>> bits (a physical address) or 128 bits (INVEPT, INVVPID). To limit the
>> amount of change here, keep the latter at reading only 64 bits from
>> guest space.
>>
>> Fixes: 09fce8016596 ("Nested VMX: Emulation of guest VMXON/OFF instruction")
>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>> ---
>> Beyond the wrong operand handling for INVEPT and INVVPID, the latter
>> also doesn't even have the part read checked to have bits 16 and above
>> all clear.
>
> There are more bugs than these.
>
> https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=shortlog;h=refs/heads/x86-next-euan
>
> Judging by the choice of branch name, I think I'd collected those
> pending the re-opening of a dev window, and apparently forgotten.
>
> At this point I don't think we want to take the branch as is (especially
> as I've forgotten why it wasn't taken before), but I think there is some
> salvageable work in there, beyond the changes in this patch.
Interesting. Apparently it has been too long since this was posted, as I
don't recall it at all. However, I'm not convinced everything's right
there. E.g. in "x86/vvmx: Use correct sizes when reading operands" there
is this hunk:
@@ -427,6 +427,7 @@ static int decode_vmx_inst(struct cpu_user_regs *regs,
struct segment_register seg;
unsigned long base, index, seg_base, disp, offset;
int scale, size;
+ unsigned int bytes = vmx_guest_x86_mode(v);
__vmread(VMX_INSTRUCTION_INFO, &offset);
info.word = offset;
Which is hardly correct for 16-bit protected mode. INVEPT and INVVPID
handling also isn't moving to 128-bit operands, despite what the title
suggests. The commit message wrongly claims the upper halves don't need
to be read.
Jan
