| Message ID | 20220722234838.2160385-3-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: Fix Clang build issues in KVM_ASM_SAFE() | expand |
On Fri, Jul 22, 2022, David Matlack wrote: > Change the mov in KVM_ASM_SAFE() that zeroes @vector to a movb to > make it unambiguous. > > This fixes a build failure with Clang since, unlike the GNU assembler, > the LLVM integrated assembler rejects ambiguous X86 instructions that > don't have suffixes: > > In file included from x86_64/hyperv_features.c:13: > include/x86_64/processor.h:825:9: error: ambiguous instructions require an explicit suffix (could be 'movb', 'movw', 'movl', or 'movq') > return kvm_asm_safe("wrmsr", "a"(val & -1u), "d"(val >> 32), "c"(msr)); > ^ > include/x86_64/processor.h:802:15: note: expanded from macro 'kvm_asm_safe' > asm volatile(KVM_ASM_SAFE(insn) \ > ^ > include/x86_64/processor.h:788:16: note: expanded from macro 'KVM_ASM_SAFE' > "1: " insn "\n\t" \ > ^ > <inline asm>:5:2: note: instantiated into assembly here > mov $0, 15(%rsp) > ^ > > It seems like this change could introduce undesirable behavior in the > future, e.g. if someone used a type larger than a u8 for @vector, since > KVM_ASM_SAFE() will only zero the bottom byte. I tried changing the type > of @vector to an int to see what would happen. GCC failed to compile due > to a size mismatch between `movb` and `%eax`. Clang succeeded in > compiling, but the generated code looked correct, so perhaps it will not > be an issue. That being said it seems like there could be a better > solution to this issue that does not assume @vector is a u8. Hrm, IIRC my intent was to not care about the size of "vector", but that may just be revisionist thinking because the: "mov %%r9b, %[vector]\n\t" \ suggests it's nothing more than a screwed up. A static assert on the size would be nice, but I don't know how to make that work since the macros dump directly into the asm. > Fixes: 3b23054cd3f5 ("KVM: selftests: Add x86-64 support for exception fixup") > Signed-off-by: David Matlack <dmatlack@google.com> Reviewed-by: Sean Christopherson <seanjc@google.com>
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h index 51c6661aca77..0cbc71b7af50 100644 --- a/tools/testing/selftests/kvm/include/x86_64/processor.h +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h @@ -786,7 +786,7 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector, "lea 1f(%%rip), %%r10\n\t" \ "lea 2f(%%rip), %%r11\n\t" \ "1: " insn "\n\t" \ - "mov $0, %[vector]\n\t" \ + "movb $0, %[vector]\n\t" \ "jmp 3f\n\t" \ "2:\n\t" \ "mov %%r9b, %[vector]\n\t" \
Change the mov in KVM_ASM_SAFE() that zeroes @vector to a movb to make it unambiguous. This fixes a build failure with Clang since, unlike the GNU assembler, the LLVM integrated assembler rejects ambiguous X86 instructions that don't have suffixes: In file included from x86_64/hyperv_features.c:13: include/x86_64/processor.h:825:9: error: ambiguous instructions require an explicit suffix (could be 'movb', 'movw', 'movl', or 'movq') return kvm_asm_safe("wrmsr", "a"(val & -1u), "d"(val >> 32), "c"(msr)); ^ include/x86_64/processor.h:802:15: note: expanded from macro 'kvm_asm_safe' asm volatile(KVM_ASM_SAFE(insn) \ ^ include/x86_64/processor.h:788:16: note: expanded from macro 'KVM_ASM_SAFE' "1: " insn "\n\t" \ ^ <inline asm>:5:2: note: instantiated into assembly here mov $0, 15(%rsp) ^ It seems like this change could introduce undesirable behavior in the future, e.g. if someone used a type larger than a u8 for @vector, since KVM_ASM_SAFE() will only zero the bottom byte. I tried changing the type of @vector to an int to see what would happen. GCC failed to compile due to a size mismatch between `movb` and `%eax`. Clang succeeded in compiling, but the generated code looked correct, so perhaps it will not be an issue. That being said it seems like there could be a better solution to this issue that does not assume @vector is a u8. Fixes: 3b23054cd3f5 ("KVM: selftests: Add x86-64 support for exception fixup") Signed-off-by: David Matlack <dmatlack@google.com> --- tools/testing/selftests/kvm/include/x86_64/processor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)