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