Re: [PATCH 4/4] KVM: selftests: Check VMPTRLD with active eVMCS

Vitaly Kuznetsov <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.kvm
Message-ID <[email protected]>
[email protected] writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The hardcoded `VMPTRLD_INSN_SIZE` of 5 bytes is brittle
> because `vmptrld` is executed using an inline assembly memory operand
> constraint (`"m"`), which results in a variable-sized instruction
> depending on compiler optimizations and register allocation.

True that, but afair we don't currently have an instruction decoder in
selftests. I see three ways to move forward:

- Calculate the size of the instruction with two labels, something like
  (untested):

    static inline int __vmptrld(u64 vmcs_pa, int *insn_size)
    {
            u8 ret;

            __asm__ __volatile__ ("movl $(2f-1f), %[size];"
                                  "1: vmptrld %[pa]; 2:"
                                  "setna %[ret]"
                    : [ret]"=rm"(ret), [size]"=m"(*insn_size)
                    : [pa]"m"(vmcs_pa)
                    : "cc", "memory");

            return ret;
    }

  and use insn_size in the tests.

- Stuf __vmptrld() with NOPs after vmptrld and keep the constant jump,
  just make it loner (e.g. 10 bytes should realistically be enough).

- Make peace with the fragility (it's just a selftest after all) and
  leave a comment in vmptrld() that changing the asm there make break
  the assumption.

Any preferences? Personally, I'm feeling adventurous and can go with the
first one - calculating the exact size with labels.

-- 
Vitaly
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.