On 13.06.23 23:40, Sean Christopherson wrote: > On Thu, 13 Apr 2023 20:42:03 +0200, Mathias Krause wrote: >> v1: https://lore.kernel.org/kvm/b6322bd0-3639-fb2a-7211-974386865bac@xxxxxxxxxxxxxx/ >> >> This is v2 of the "non-canonical memory access" test. It evolved into a >> small series, bringing cleanups and fixes along the way. >> >> I integrated Sean's feedback and changed the test to make use of >> ASM_TRY() instead of using the hand-rolled exception handler. I also >> switched all other users in emulator64.c to ASM_TRY() and was able to >> drop the one-off exception handler all together. >> >> [...] > > Applied everything except the code label change to kvm-x86 next. I replied to > that specific patch, feel free to follow-up there. I did, but I have no strong opinion on getting it merged. It's just the coding style I'm used to. If KUT's different, then that be it. > > I tweaked "x86/run_in_user: Reload SS after successful return" to use an "rm" > constraint instead of hardcoding use of AX, holler if that's wrong for some > reason. I'm planning on sending a pull request later this week, so you've got > a few days to object. I used "i" as a constraint as that's what KERNEL_DS really is: an integer constant. But I can see that the stunt of actually loading %ss without clobbering a register isn't all that nice looking. I used (R)AX specifically as that avoids allocating yet another register for this ASM block. However, there are still enough left and the stack pointer is already restored at that point so using "rm" instead should be fine. Just noticed that the constraints for 'rax' should be "=&a" instead of "+a" as the ASM doesn't care about its initial value, just needs to prevent the compiler from allocating AX for any of the input register variables. But that can be a separate cleanup. > > Thanks a ton for the cleanups! > > [01/16] x86: Drop types.h > https://github.com/kvm-x86/kvm-unit-tests/commit/0452fa5aecea > [02/16] x86: Use symbolic names in exception_mnemonic() > https://github.com/kvm-x86/kvm-unit-tests/commit/8cfb268d401b > [03/16] x86: Add vendor specific exception vectors > https://github.com/kvm-x86/kvm-unit-tests/commit/f224dba008df > [04/16] x86/cet: Use symbolic name for #CP > https://github.com/kvm-x86/kvm-unit-tests/commit/00d585d8731b > [05/16] x86/access: Use 'bool' type as defined via libcflat.h > https://github.com/kvm-x86/kvm-unit-tests/commit/c304eda6ae7f > [06/16] x86/run_in_user: Change type of code label > DID NOT APPLY > [07/16] x86/run_in_user: Preserve exception handler > https://github.com/kvm-x86/kvm-unit-tests/commit/d0ef95181cfb > [08/16] x86/run_in_user: Relax register constraints of inline asm > https://github.com/kvm-x86/kvm-unit-tests/commit/45bafaf28fbb > [09/16] x86/run_in_user: Reload SS after successful return > https://github.com/kvm-x86/kvm-unit-tests/commit/8338209b8245 > [10/16] x86/fault_test: Preserve exception handler > https://github.com/kvm-x86/kvm-unit-tests/commit/11aac640d01b > [11/16] x86/emulator64: Relax register constraints for usr_gs_mov() > https://github.com/kvm-x86/kvm-unit-tests/commit/c66547850058 > [12/16] x86/emulator64: Switch test_sreg() to ASM_TRY() > https://github.com/kvm-x86/kvm-unit-tests/commit/9d74b31d1c81 > [13/16] x86/emulator64: Add non-null selector test > https://github.com/kvm-x86/kvm-unit-tests/commit/23c647d0ef29 > [14/16] x86/emulator64: Switch test_jmp_noncanonical() to ASM_TRY() > https://github.com/kvm-x86/kvm-unit-tests/commit/ac4f843474b4 > [15/16] x86/emulator64: Switch test_mmx_movq_mf() to ASM_TRY() > https://github.com/kvm-x86/kvm-unit-tests/commit/5a3515ea1bc2 > [16/16] x86/emulator64: Test non-canonical memory access exceptions > https://github.com/kvm-x86/kvm-unit-tests/commit/e3a9b2f5490e Thanks, Mathias > > -- > https://github.com/kvm-x86/kvm-unit-tests/tree/next
