* [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes
@ 2022-02-01 1:08 Sean Christopherson
2022-02-01 1:08 ` [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug Sean Christopherson
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 1:08 UTC (permalink / raw)
To: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, llvm, linux-kernel, Peter Zijlstra,
syzbot+6cde2282daa792c49ab8
Add uaccess macros for doing CMPXCHG on userspace addresses and use the
macros to fix KVM bugs by replacing flawed code that maps memory into the
kernel address space without proper mmu_notifier protection (or with
broken pfn calculations in one case).
Add yet another Kconfig for guarding asm_volatile_goto() to workaround a
clang-13 bug. I've verified the test passes on gcc versions of arm64,
PPC, RISC-V, and s390x that also pass the CC_HAS_ASM_GOTO_OUTPUT test.
Patches 1-4 are tagged for stable@ as patches 3 and 4 (mostly 3) need a
backportable fix, and doing CMPXCHG on the userspace address is the
simplest fix from a KVM perspective.
Peter Zijlstra (1):
x86/uaccess: Implement macros for CMPXCHG on user addresses
Sean Christopherson (4):
Kconfig: Add option for asm goto w/ tied outputs to workaround
clang-13 bug
KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses
KVM: x86: Bail to userspace if emulation of atomic user access faults
arch/x86/include/asm/uaccess.h | 131 +++++++++++++++++++++++++++++++++
arch/x86/kvm/mmu/paging_tmpl.h | 45 +----------
arch/x86/kvm/x86.c | 35 ++++-----
init/Kconfig | 4 +
4 files changed, 150 insertions(+), 65 deletions(-)
base-commit: 26291c54e111ff6ba87a164d85d4a4e134b7315c
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug
2022-02-01 1:08 [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Sean Christopherson
@ 2022-02-01 1:08 ` Sean Christopherson
2022-02-01 20:16 ` Nick Desaulniers
2022-02-01 1:08 ` [PATCH 2/5] x86/uaccess: Implement macros for CMPXCHG on user addresses Sean Christopherson
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 1:08 UTC (permalink / raw)
To: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, llvm, linux-kernel, Peter Zijlstra,
syzbot+6cde2282daa792c49ab8
Add a config option to guard (future) usage of asm_volatile_goto() that
includes "tied outputs", i.e. "+" constraints that specify both an input
and output parameter. clang-13 has a bug[1] that causes compilation of
such inline asm to fail, and KVM wants to use a "+m" constraint to
implement a uaccess form of CMPXCHG[2]. E.g. the test code fails with
<stdin>:1:29: error: invalid operand in inline asm: '.long (${1:l}) - .'
int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }
^
<stdin>:1:29: error: unknown token in expression
<inline asm>:1:9: note: instantiated into assembly here
.long () - .
^
2 errors generated.
on clang-13, but passes on gcc (with appropriate asm goto support). The
bug is fixed in clang-14, but won't be backported to clang-13 as the
changes are too invasive/risky.
[1] https://github.com/ClangBuiltLinux/linux/issues/1512
[2] https://lore.kernel.org/all/YfMruK8%2F1izZ2VHS@google.com
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
init/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/init/Kconfig b/init/Kconfig
index e9119bf54b1f..a206b21703be 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -77,6 +77,10 @@ config CC_HAS_ASM_GOTO_OUTPUT
depends on CC_HAS_ASM_GOTO
def_bool $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
+config CC_HAS_ASM_GOTO_TIED_OUTPUT
+ depends on CC_HAS_ASM_GOTO_OUTPUT
+ def_bool $(success,echo 'int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }' | $CC -x c - -c -o /dev/null)
+
config TOOLS_SUPPORT_RELR
def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(NM)" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/5] x86/uaccess: Implement macros for CMPXCHG on user addresses
2022-02-01 1:08 [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Sean Christopherson
2022-02-01 1:08 ` [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug Sean Christopherson
@ 2022-02-01 1:08 ` Sean Christopherson
2022-02-01 1:08 ` [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits Sean Christopherson
` (3 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 1:08 UTC (permalink / raw)
To: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, llvm, linux-kernel, Peter Zijlstra,
syzbot+6cde2282daa792c49ab8
From: Peter Zijlstra <peterz@infradead.org>
Add support for CMPXCHG loops on userspace addresses. Provide both an
"unsafe" version for tight loops that do their own uaccess begin/end, as
well as a "safe" version for use cases where the CMPXCHG is not buried in
a loop, e.g. KVM will resume the guest instead of looping when emulation
of a guest atomic accesses fails the CMPXCHG.
Provide 8-byte versions for 32-bit kernels so that KVM can do CMPXCHG on
guest PAE PTEs, which are accessed via userspace addresses.
Guard the asm_volatile_goto() variation with CC_HAS_ASM_GOTO_TIED_OUTPUT,
the "+m" constraint fails on some compilers that otherwise support
CC_HAS_ASM_GOTO_OUTPUT.
Cc: stable@vger.kernel.org
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/uaccess.h | 131 +++++++++++++++++++++++++++++++++
1 file changed, 131 insertions(+)
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index ac96f9b2d64b..423bfcc1ec4b 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -409,6 +409,98 @@ do { \
#endif // CONFIG_CC_HAS_ASM_GOTO_OUTPUT
+#ifdef CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
+#define __try_cmpxchg_user_asm(itype, ltype, _ptr, _pold, _new, label) ({ \
+ bool success; \
+ __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
+ __typeof__(*(_ptr)) __old = *_old; \
+ __typeof__(*(_ptr)) __new = (_new); \
+ asm_volatile_goto("\n" \
+ "1: " LOCK_PREFIX "cmpxchg"itype" %[new], %[ptr]\n"\
+ _ASM_EXTABLE_UA(1b, %l[label]) \
+ : CC_OUT(z) (success), \
+ [ptr] "+m" (*_ptr), \
+ [old] "+a" (__old) \
+ : [new] ltype (__new) \
+ : "memory" \
+ : label); \
+ if (unlikely(!success)) \
+ *_old = __old; \
+ likely(success); })
+
+#ifdef CONFIG_X86_32
+#define __try_cmpxchg64_user_asm(_ptr, _pold, _new, label) ({ \
+ bool success; \
+ __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
+ __typeof__(*(_ptr)) __old = *_old; \
+ __typeof__(*(_ptr)) __new = (_new); \
+ asm_volatile_goto("\n" \
+ "1: " LOCK_PREFIX "cmpxchg8b %[ptr]\n" \
+ _ASM_EXTABLE_UA(1b, %l[label]) \
+ : CC_OUT(z) (success), \
+ "+A" (__old), \
+ [ptr] "+m" (*_ptr) \
+ : "b" ((u32)__new), \
+ "c" ((u32)((u64)__new >> 32)) \
+ : "memory" \
+ : label); \
+ if (unlikely(!success)) \
+ *_old = __old; \
+ likely(success); })
+#endif // CONFIG_X86_32
+#else // !CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
+#define __try_cmpxchg_user_asm(itype, ltype, _ptr, _pold, _new, label) ({ \
+ int __err = 0; \
+ bool success; \
+ __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
+ __typeof__(*(_ptr)) __old = *_old; \
+ __typeof__(*(_ptr)) __new = (_new); \
+ asm volatile("\n" \
+ "1: " LOCK_PREFIX "cmpxchg"itype" %[new], %[ptr]\n"\
+ CC_SET(z) \
+ "2:\n" \
+ _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG, \
+ %[errout]) \
+ : CC_OUT(z) (success), \
+ [errout] "+r" (__err), \
+ [ptr] "+m" (*_ptr), \
+ [old] "+a" (__old) \
+ : [new] ltype (__new) \
+ : "memory", "cc"); \
+ if (unlikely(__err)) \
+ goto label; \
+ if (unlikely(!success)) \
+ *_old = __old; \
+ likely(success); })
+
+#ifdef CONFIG_X86_32
+#define __try_cmpxchg64_user_asm(_ptr, _pold, _new, label) ({ \
+ int __err = 0; \
+ bool success; \
+ __typeof__(_ptr) _old = (__typeof__(_ptr))(_pold); \
+ __typeof__(*(_ptr)) __old = *_old; \
+ __typeof__(*(_ptr)) __new = (_new); \
+ asm volatile("\n" \
+ "1: " LOCK_PREFIX "cmpxchg8b %[ptr]\n" \
+ CC_SET(z) \
+ "2:\n" \
+ _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG, \
+ %[errout]) \
+ : CC_OUT(z) (success), \
+ [errout] "+r" (__err), \
+ "+A" (__old), \
+ [ptr] "+m" (*_ptr) \
+ : "b" ((u32)__new), \
+ "c" ((u32)((u64)__new >> 32)) \
+ : "memory", "cc"); \
+ if (unlikely(__err)) \
+ goto label; \
+ if (unlikely(!success)) \
+ *_old = __old; \
+ likely(success); })
+#endif // CONFIG_X86_32
+#endif // CONFIG_CC_HAS_ASM_GOTO_TIED_OUTPUT
+
/* FIXME: this hack is definitely wrong -AK */
struct __large_struct { unsigned long buf[100]; };
#define __m(x) (*(struct __large_struct __user *)(x))
@@ -501,6 +593,45 @@ do { \
} while (0)
#endif // CONFIG_CC_HAS_ASM_GOTO_OUTPUT
+extern void __try_cmpxchg_user_wrong_size(void);
+
+#ifndef CONFIG_X86_32
+#define __try_cmpxchg64_user_asm(_ptr, _oldp, _nval, _label) \
+ __try_cmpxchg_user_asm("q", "r", (_ptr), (_oldp), (_nval), _label)
+#endif
+
+#define unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label) ({ \
+ bool __ret; \
+ switch (sizeof(*(_ptr))) { \
+ case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
+ (_ptr), (_oldp), \
+ (_nval), _label); \
+ break; \
+ case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
+ (_ptr), (_oldp), \
+ (_nval), _label); \
+ break; \
+ case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
+ (_ptr), (_oldp), \
+ (_nval), _label); \
+ break; \
+ case 8: __ret = __try_cmpxchg64_user_asm((_ptr), (_oldp), \
+ (_nval), _label); \
+ break; \
+ default: __try_cmpxchg_user_wrong_size(); \
+ } \
+ __ret; })
+
+/* "Returns" 0 on success, 1 on failure, -EFAULT if the access faults. */
+#define __try_cmpxchg_user(_ptr, _oldp, _nval, _label) ({ \
+ int __ret = -EFAULT; \
+ __uaccess_begin_nospec(); \
+ __ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
+_label: \
+ __uaccess_end(); \
+ __ret; \
+ })
+
/*
* We want the unsafe accessors to always be inlined and use
* the error labels - thus the macro games.
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
2022-02-01 1:08 [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Sean Christopherson
2022-02-01 1:08 ` [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug Sean Christopherson
2022-02-01 1:08 ` [PATCH 2/5] x86/uaccess: Implement macros for CMPXCHG on user addresses Sean Christopherson
@ 2022-02-01 1:08 ` Sean Christopherson
2022-02-01 7:01 ` kernel test robot
2022-02-01 13:25 ` kernel test robot
2022-02-01 1:08 ` [PATCH 4/5] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses Sean Christopherson
` (2 subsequent siblings)
5 siblings, 2 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 1:08 UTC (permalink / raw)
To: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, llvm, linux-kernel, Peter Zijlstra,
syzbot+6cde2282daa792c49ab8
Use the recently introduced __try_cmpxchg_user() to update guest PTE A/D
bits instead of mapping the PTE into kernel address space. The VM_PFNMAP
path is broken as it assumes that vm_pgoff is the base pfn of the mapped
VMA range, which is conceptually wrong as vm_pgoff is the offset relative
to the file and has nothing to do with the pfn. The horrific hack worked
for the original use case (backing guest memory with /dev/mem), but leads
to accessing "random" pfns for pretty much any other VM_PFNMAP case.
Fixes: bd53cb35a3e9 ("X86/KVM: Handle PFNs outside of kernel reach when touching GPTEs")
Debugged-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Reported-by: syzbot+6cde2282daa792c49ab8@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/mmu/paging_tmpl.h | 45 +---------------------------------
1 file changed, 1 insertion(+), 44 deletions(-)
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 5b5bdac97c7b..551de15f342f 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -143,49 +143,6 @@ static bool FNAME(is_rsvd_bits_set)(struct kvm_mmu *mmu, u64 gpte, int level)
FNAME(is_bad_mt_xwr)(&mmu->guest_rsvd_check, gpte);
}
-static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
- pt_element_t __user *ptep_user, unsigned index,
- pt_element_t orig_pte, pt_element_t new_pte)
-{
- int npages;
- pt_element_t ret;
- pt_element_t *table;
- struct page *page;
-
- npages = get_user_pages_fast((unsigned long)ptep_user, 1, FOLL_WRITE, &page);
- if (likely(npages == 1)) {
- table = kmap_atomic(page);
- ret = CMPXCHG(&table[index], orig_pte, new_pte);
- kunmap_atomic(table);
-
- kvm_release_page_dirty(page);
- } else {
- struct vm_area_struct *vma;
- unsigned long vaddr = (unsigned long)ptep_user & PAGE_MASK;
- unsigned long pfn;
- unsigned long paddr;
-
- mmap_read_lock(current->mm);
- vma = find_vma_intersection(current->mm, vaddr, vaddr + PAGE_SIZE);
- if (!vma || !(vma->vm_flags & VM_PFNMAP)) {
- mmap_read_unlock(current->mm);
- return -EFAULT;
- }
- pfn = ((vaddr - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
- paddr = pfn << PAGE_SHIFT;
- table = memremap(paddr, PAGE_SIZE, MEMREMAP_WB);
- if (!table) {
- mmap_read_unlock(current->mm);
- return -EFAULT;
- }
- ret = CMPXCHG(&table[index], orig_pte, new_pte);
- memunmap(table);
- mmap_read_unlock(current->mm);
- }
-
- return (ret != orig_pte);
-}
-
static bool FNAME(prefetch_invalid_gpte)(struct kvm_vcpu *vcpu,
struct kvm_mmu_page *sp, u64 *spte,
u64 gpte)
@@ -284,7 +241,7 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
if (unlikely(!walker->pte_writable[level - 1]))
continue;
- ret = FNAME(cmpxchg_gpte)(vcpu, mmu, ptep_user, index, orig_pte, pte);
+ ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
if (ret)
return ret;
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/5] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses
2022-02-01 1:08 [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Sean Christopherson
` (2 preceding siblings ...)
2022-02-01 1:08 ` [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits Sean Christopherson
@ 2022-02-01 1:08 ` Sean Christopherson
2022-02-01 9:25 ` kernel test robot
2022-02-01 1:08 ` [PATCH 5/5] KVM: x86: Bail to userspace if emulation of atomic user access faults Sean Christopherson
2022-02-01 17:09 ` [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Tadeusz Struk
5 siblings, 1 reply; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 1:08 UTC (permalink / raw)
To: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, llvm, linux-kernel, Peter Zijlstra,
syzbot+6cde2282daa792c49ab8
Use the recently introduce __try_cmpxchg_user() to emulate atomic guest
accesses via the associated userspace address instead of mapping the
backing pfn into kernel address space. Using kvm_vcpu_map() is unsafe as
it does not coordinate with KVM's mmu_notifier to ensure the hva=>pfn
translation isn't changed/unmapped in the memremap() path, i.e. when
there's no struct page and thus no elevated refcount.
Fixes: 42e35f8072c3 ("KVM/X86: Use kvm_vcpu_map in emulator_cmpxchg_emulated")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 35 ++++++++++++++---------------------
1 file changed, 14 insertions(+), 21 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 74b53a16f38a..37064d565bbc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7155,15 +7155,8 @@ static int emulator_write_emulated(struct x86_emulate_ctxt *ctxt,
exception, &write_emultor);
}
-#define CMPXCHG_TYPE(t, ptr, old, new) \
- (cmpxchg((t *)(ptr), *(t *)(old), *(t *)(new)) == *(t *)(old))
-
-#ifdef CONFIG_X86_64
-# define CMPXCHG64(ptr, old, new) CMPXCHG_TYPE(u64, ptr, old, new)
-#else
-# define CMPXCHG64(ptr, old, new) \
- (cmpxchg64((u64 *)(ptr), *(u64 *)(old), *(u64 *)(new)) == *(u64 *)(old))
-#endif
+#define emulator_try_cmpxchg_user(t, ptr, old, new) \
+ (__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
unsigned long addr,
@@ -7172,12 +7165,11 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
unsigned int bytes,
struct x86_exception *exception)
{
- struct kvm_host_map map;
struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
u64 page_line_mask;
+ unsigned long hva;
gpa_t gpa;
- char *kaddr;
- bool exchanged;
+ int r;
/* guests cmpxchg8b have to be emulated atomically */
if (bytes > 8 || (bytes & (bytes - 1)))
@@ -7201,31 +7193,32 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
if (((gpa + bytes - 1) & page_line_mask) != (gpa & page_line_mask))
goto emul_write;
- if (kvm_vcpu_map(vcpu, gpa_to_gfn(gpa), &map))
+ hva = kvm_vcpu_gfn_to_hva(vcpu, gpa_to_gfn(gpa));
+ if (kvm_is_error_hva(addr))
goto emul_write;
- kaddr = map.hva + offset_in_page(gpa);
+ hva += offset_in_page(gpa);
switch (bytes) {
case 1:
- exchanged = CMPXCHG_TYPE(u8, kaddr, old, new);
+ r = emulator_try_cmpxchg_user(u8, hva, old, new);
break;
case 2:
- exchanged = CMPXCHG_TYPE(u16, kaddr, old, new);
+ r = emulator_try_cmpxchg_user(u16, hva, old, new);
break;
case 4:
- exchanged = CMPXCHG_TYPE(u32, kaddr, old, new);
+ r = emulator_try_cmpxchg_user(u32, hva, old, new);
break;
case 8:
- exchanged = CMPXCHG64(kaddr, old, new);
+ r = emulator_try_cmpxchg_user(u64, hva, old, new);
break;
default:
BUG();
}
- kvm_vcpu_unmap(vcpu, &map, true);
-
- if (!exchanged)
+ if (r < 0)
+ goto emul_write;
+ if (r)
return X86EMUL_CMPXCHG_FAILED;
kvm_page_track_write(vcpu, gpa, new, bytes);
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/5] KVM: x86: Bail to userspace if emulation of atomic user access faults
2022-02-01 1:08 [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Sean Christopherson
` (3 preceding siblings ...)
2022-02-01 1:08 ` [PATCH 4/5] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses Sean Christopherson
@ 2022-02-01 1:08 ` Sean Christopherson
2022-02-01 17:09 ` [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Tadeusz Struk
5 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 1:08 UTC (permalink / raw)
To: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, llvm, linux-kernel, Peter Zijlstra,
syzbot+6cde2282daa792c49ab8
Exit to userspace when emulating an atomic guest access if the CMPXCHG on
the userspace address faults. Emulating the access as a write and thus
likely treating it as emulated MMIO is wrong, as KVM has already
confirmed there is a valid, writable memslot.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 37064d565bbc..66c5410dd4c3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7217,7 +7217,7 @@ static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
}
if (r < 0)
- goto emul_write;
+ return X86EMUL_UNHANDLEABLE;
if (r)
return X86EMUL_CMPXCHG_FAILED;
--
2.35.0.rc2.247.g8bbb082509-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
2022-02-01 1:08 ` [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits Sean Christopherson
@ 2022-02-01 7:01 ` kernel test robot
2022-02-01 13:25 ` kernel test robot
1 sibling, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-02-01 7:01 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: llvm, kbuild-all, Sean Christopherson, Vitaly Kuznetsov,
Wanpeng Li, Jim Mattson, Joerg Roedel, kvm
Hi Sean,
I love your patch! Yet something to improve:
[auto build test ERROR on 26291c54e111ff6ba87a164d85d4a4e134b7315c]
url: https://github.com/0day-ci/linux/commits/Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220201/202202011400.EaZmWZ48-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c880d7a9df876f20dc3acdd893c5c71f3cda5029
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
git checkout c880d7a9df876f20dc3acdd893c5c71f3cda5029
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from arch/x86/kvm/mmu/mmu.c:4246:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4246:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:610:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4246:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:614:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4250:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4250:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:610:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4250:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:614:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
6 errors generated.
vim +244 arch/x86/kvm/mmu/paging_tmpl.h
191
192 static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
193 struct kvm_mmu *mmu,
194 struct guest_walker *walker,
195 gpa_t addr, int write_fault)
196 {
197 unsigned level, index;
198 pt_element_t pte, orig_pte;
199 pt_element_t __user *ptep_user;
200 gfn_t table_gfn;
201 int ret;
202
203 /* dirty/accessed bits are not supported, so no need to update them */
204 if (!PT_HAVE_ACCESSED_DIRTY(mmu))
205 return 0;
206
207 for (level = walker->max_level; level >= walker->level; --level) {
208 pte = orig_pte = walker->ptes[level - 1];
209 table_gfn = walker->table_gfn[level - 1];
210 ptep_user = walker->ptep_user[level - 1];
211 index = offset_in_page(ptep_user) / sizeof(pt_element_t);
212 if (!(pte & PT_GUEST_ACCESSED_MASK)) {
213 trace_kvm_mmu_set_accessed_bit(table_gfn, index, sizeof(pte));
214 pte |= PT_GUEST_ACCESSED_MASK;
215 }
216 if (level == walker->level && write_fault &&
217 !(pte & PT_GUEST_DIRTY_MASK)) {
218 trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
219 #if PTTYPE == PTTYPE_EPT
220 if (kvm_x86_ops.nested_ops->write_log_dirty(vcpu, addr))
221 return -EINVAL;
222 #endif
223 pte |= PT_GUEST_DIRTY_MASK;
224 }
225 if (pte == orig_pte)
226 continue;
227
228 /*
229 * If the slot is read-only, simply do not process the accessed
230 * and dirty bits. This is the correct thing to do if the slot
231 * is ROM, and page tables in read-as-ROM/write-as-MMIO slots
232 * are only supported if the accessed and dirty bits are already
233 * set in the ROM (so that MMIO writes are never needed).
234 *
235 * Note that NPT does not allow this at all and faults, since
236 * it always wants nested page table entries for the guest
237 * page tables to be writable. And EPT works but will simply
238 * overwrite the read-only memory to set the accessed and dirty
239 * bits.
240 */
241 if (unlikely(!walker->pte_writable[level - 1]))
242 continue;
243
> 244 ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
245 if (ret)
246 return ret;
247
248 kvm_vcpu_mark_page_dirty(vcpu, table_gfn);
249 walker->ptes[level - 1] = pte;
250 }
251 return 0;
252 }
253
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
@ 2022-02-01 7:01 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-02-01 7:01 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 8813 bytes --]
Hi Sean,
I love your patch! Yet something to improve:
[auto build test ERROR on 26291c54e111ff6ba87a164d85d4a4e134b7315c]
url: https://github.com/0day-ci/linux/commits/Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220201/202202011400.EaZmWZ48-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c880d7a9df876f20dc3acdd893c5c71f3cda5029
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
git checkout c880d7a9df876f20dc3acdd893c5c71f3cda5029
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from arch/x86/kvm/mmu/mmu.c:4246:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4246:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:610:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4246:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:614:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4250:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4250:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:610:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
In file included from arch/x86/kvm/mmu/mmu.c:4250:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:614:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
6 errors generated.
vim +244 arch/x86/kvm/mmu/paging_tmpl.h
191
192 static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
193 struct kvm_mmu *mmu,
194 struct guest_walker *walker,
195 gpa_t addr, int write_fault)
196 {
197 unsigned level, index;
198 pt_element_t pte, orig_pte;
199 pt_element_t __user *ptep_user;
200 gfn_t table_gfn;
201 int ret;
202
203 /* dirty/accessed bits are not supported, so no need to update them */
204 if (!PT_HAVE_ACCESSED_DIRTY(mmu))
205 return 0;
206
207 for (level = walker->max_level; level >= walker->level; --level) {
208 pte = orig_pte = walker->ptes[level - 1];
209 table_gfn = walker->table_gfn[level - 1];
210 ptep_user = walker->ptep_user[level - 1];
211 index = offset_in_page(ptep_user) / sizeof(pt_element_t);
212 if (!(pte & PT_GUEST_ACCESSED_MASK)) {
213 trace_kvm_mmu_set_accessed_bit(table_gfn, index, sizeof(pte));
214 pte |= PT_GUEST_ACCESSED_MASK;
215 }
216 if (level == walker->level && write_fault &&
217 !(pte & PT_GUEST_DIRTY_MASK)) {
218 trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
219 #if PTTYPE == PTTYPE_EPT
220 if (kvm_x86_ops.nested_ops->write_log_dirty(vcpu, addr))
221 return -EINVAL;
222 #endif
223 pte |= PT_GUEST_DIRTY_MASK;
224 }
225 if (pte == orig_pte)
226 continue;
227
228 /*
229 * If the slot is read-only, simply do not process the accessed
230 * and dirty bits. This is the correct thing to do if the slot
231 * is ROM, and page tables in read-as-ROM/write-as-MMIO slots
232 * are only supported if the accessed and dirty bits are already
233 * set in the ROM (so that MMIO writes are never needed).
234 *
235 * Note that NPT does not allow this at all and faults, since
236 * it always wants nested page table entries for the guest
237 * page tables to be writable. And EPT works but will simply
238 * overwrite the read-only memory to set the accessed and dirty
239 * bits.
240 */
241 if (unlikely(!walker->pte_writable[level - 1]))
242 continue;
243
> 244 ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
245 if (ret)
246 return ret;
247
248 kvm_vcpu_mark_page_dirty(vcpu, table_gfn);
249 walker->ptes[level - 1] = pte;
250 }
251 return 0;
252 }
253
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/5] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses
2022-02-01 1:08 ` [PATCH 4/5] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses Sean Christopherson
@ 2022-02-01 9:25 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-02-01 9:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: llvm, kbuild-all, Sean Christopherson, Vitaly Kuznetsov,
Wanpeng Li, Jim Mattson, Joerg Roedel, kvm
Hi Sean,
I love your patch! Yet something to improve:
[auto build test ERROR on 26291c54e111ff6ba87a164d85d4a4e134b7315c]
url: https://github.com/0day-ci/linux/commits/Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220201/202202011753.zksthphR-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/5387711f4b49675e162ca30b05a3b2435326e5f9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
git checkout 5387711f4b49675e162ca30b05a3b2435326e5f9
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> arch/x86/kvm/x86.c:7213:7: error: invalid output size for constraint '+a'
r = emulator_try_cmpxchg_user(u64, hva, old, new);
^
arch/x86/kvm/x86.c:7159:3: note: expanded from macro 'emulator_try_cmpxchg_user'
(__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
>> arch/x86/kvm/x86.c:7213:7: error: invalid output size for constraint '+a'
arch/x86/kvm/x86.c:7159:3: note: expanded from macro 'emulator_try_cmpxchg_user'
(__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:610:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
>> arch/x86/kvm/x86.c:7213:7: error: invalid output size for constraint '+a'
arch/x86/kvm/x86.c:7159:3: note: expanded from macro 'emulator_try_cmpxchg_user'
(__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:614:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
3 errors generated.
vim +7213 arch/x86/kvm/x86.c
7157
7158 #define emulator_try_cmpxchg_user(t, ptr, old, new) \
7159 (__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
7160
7161 static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
7162 unsigned long addr,
7163 const void *old,
7164 const void *new,
7165 unsigned int bytes,
7166 struct x86_exception *exception)
7167 {
7168 struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
7169 u64 page_line_mask;
7170 unsigned long hva;
7171 gpa_t gpa;
7172 int r;
7173
7174 /* guests cmpxchg8b have to be emulated atomically */
7175 if (bytes > 8 || (bytes & (bytes - 1)))
7176 goto emul_write;
7177
7178 gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, NULL);
7179
7180 if (gpa == UNMAPPED_GVA ||
7181 (gpa & PAGE_MASK) == APIC_DEFAULT_PHYS_BASE)
7182 goto emul_write;
7183
7184 /*
7185 * Emulate the atomic as a straight write to avoid #AC if SLD is
7186 * enabled in the host and the access splits a cache line.
7187 */
7188 if (boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
7189 page_line_mask = ~(cache_line_size() - 1);
7190 else
7191 page_line_mask = PAGE_MASK;
7192
7193 if (((gpa + bytes - 1) & page_line_mask) != (gpa & page_line_mask))
7194 goto emul_write;
7195
7196 hva = kvm_vcpu_gfn_to_hva(vcpu, gpa_to_gfn(gpa));
7197 if (kvm_is_error_hva(addr))
7198 goto emul_write;
7199
7200 hva += offset_in_page(gpa);
7201
7202 switch (bytes) {
7203 case 1:
7204 r = emulator_try_cmpxchg_user(u8, hva, old, new);
7205 break;
7206 case 2:
7207 r = emulator_try_cmpxchg_user(u16, hva, old, new);
7208 break;
7209 case 4:
7210 r = emulator_try_cmpxchg_user(u32, hva, old, new);
7211 break;
7212 case 8:
> 7213 r = emulator_try_cmpxchg_user(u64, hva, old, new);
7214 break;
7215 default:
7216 BUG();
7217 }
7218
7219 if (r < 0)
7220 goto emul_write;
7221 if (r)
7222 return X86EMUL_CMPXCHG_FAILED;
7223
7224 kvm_page_track_write(vcpu, gpa, new, bytes);
7225
7226 return X86EMUL_CONTINUE;
7227
7228 emul_write:
7229 printk_once(KERN_WARNING "kvm: emulating exchange as write\n");
7230
7231 return emulator_write_emulated(ctxt, addr, new, bytes, exception);
7232 }
7233
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/5] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses
@ 2022-02-01 9:25 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-02-01 9:25 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6717 bytes --]
Hi Sean,
I love your patch! Yet something to improve:
[auto build test ERROR on 26291c54e111ff6ba87a164d85d4a4e134b7315c]
url: https://github.com/0day-ci/linux/commits/Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220201/202202011753.zksthphR-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/5387711f4b49675e162ca30b05a3b2435326e5f9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
git checkout 5387711f4b49675e162ca30b05a3b2435326e5f9
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> arch/x86/kvm/x86.c:7213:7: error: invalid output size for constraint '+a'
r = emulator_try_cmpxchg_user(u64, hva, old, new);
^
arch/x86/kvm/x86.c:7159:3: note: expanded from macro 'emulator_try_cmpxchg_user'
(__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
>> arch/x86/kvm/x86.c:7213:7: error: invalid output size for constraint '+a'
arch/x86/kvm/x86.c:7159:3: note: expanded from macro 'emulator_try_cmpxchg_user'
(__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:610:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
>> arch/x86/kvm/x86.c:7213:7: error: invalid output size for constraint '+a'
arch/x86/kvm/x86.c:7159:3: note: expanded from macro 'emulator_try_cmpxchg_user'
(__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
^
arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
__ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
^
arch/x86/include/asm/uaccess.h:614:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
^
arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
[old] "+a" (__old) \
^
3 errors generated.
vim +7213 arch/x86/kvm/x86.c
7157
7158 #define emulator_try_cmpxchg_user(t, ptr, old, new) \
7159 (__try_cmpxchg_user((t *)(ptr), (t *)(old), *(t *)(new), efault ## t))
7160
7161 static int emulator_cmpxchg_emulated(struct x86_emulate_ctxt *ctxt,
7162 unsigned long addr,
7163 const void *old,
7164 const void *new,
7165 unsigned int bytes,
7166 struct x86_exception *exception)
7167 {
7168 struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
7169 u64 page_line_mask;
7170 unsigned long hva;
7171 gpa_t gpa;
7172 int r;
7173
7174 /* guests cmpxchg8b have to be emulated atomically */
7175 if (bytes > 8 || (bytes & (bytes - 1)))
7176 goto emul_write;
7177
7178 gpa = kvm_mmu_gva_to_gpa_write(vcpu, addr, NULL);
7179
7180 if (gpa == UNMAPPED_GVA ||
7181 (gpa & PAGE_MASK) == APIC_DEFAULT_PHYS_BASE)
7182 goto emul_write;
7183
7184 /*
7185 * Emulate the atomic as a straight write to avoid #AC if SLD is
7186 * enabled in the host and the access splits a cache line.
7187 */
7188 if (boot_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT))
7189 page_line_mask = ~(cache_line_size() - 1);
7190 else
7191 page_line_mask = PAGE_MASK;
7192
7193 if (((gpa + bytes - 1) & page_line_mask) != (gpa & page_line_mask))
7194 goto emul_write;
7195
7196 hva = kvm_vcpu_gfn_to_hva(vcpu, gpa_to_gfn(gpa));
7197 if (kvm_is_error_hva(addr))
7198 goto emul_write;
7199
7200 hva += offset_in_page(gpa);
7201
7202 switch (bytes) {
7203 case 1:
7204 r = emulator_try_cmpxchg_user(u8, hva, old, new);
7205 break;
7206 case 2:
7207 r = emulator_try_cmpxchg_user(u16, hva, old, new);
7208 break;
7209 case 4:
7210 r = emulator_try_cmpxchg_user(u32, hva, old, new);
7211 break;
7212 case 8:
> 7213 r = emulator_try_cmpxchg_user(u64, hva, old, new);
7214 break;
7215 default:
7216 BUG();
7217 }
7218
7219 if (r < 0)
7220 goto emul_write;
7221 if (r)
7222 return X86EMUL_CMPXCHG_FAILED;
7223
7224 kvm_page_track_write(vcpu, gpa, new, bytes);
7225
7226 return X86EMUL_CONTINUE;
7227
7228 emul_write:
7229 printk_once(KERN_WARNING "kvm: emulating exchange as write\n");
7230
7231 return emulator_write_emulated(ctxt, addr, new, bytes, exception);
7232 }
7233
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
2022-02-01 1:08 ` [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits Sean Christopherson
@ 2022-02-01 13:25 ` kernel test robot
2022-02-01 13:25 ` kernel test robot
1 sibling, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-02-01 13:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: kbuild-all, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel, kvm, llvm
Hi Sean,
I love your patch! Perhaps something to improve:
[auto build test WARNING on 26291c54e111ff6ba87a164d85d4a4e134b7315c]
url: https://github.com/0day-ci/linux/commits/Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220201/202202012104.eSvVUhWh-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/c880d7a9df876f20dc3acdd893c5c71f3cda5029
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
git checkout c880d7a9df876f20dc3acdd893c5c71f3cda5029
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
arch/x86/kvm/mmu/mmu.c:695:9: sparse: sparse: context imbalance in 'walk_shadow_page_lockless_begin' - different lock contexts for basic block
arch/x86/kvm/mmu/mmu.c: note: in included file (through include/linux/rbtree.h, include/linux/mm_types.h, arch/x86/kvm/irq.h):
include/linux/rcupdate.h:725:9: sparse: sparse: context imbalance in 'walk_shadow_page_lockless_end' - unexpected unlock
arch/x86/kvm/mmu/mmu.c:2595:9: sparse: sparse: context imbalance in 'mmu_try_to_unsync_pages' - different lock contexts for basic block
arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
arch/x86/kvm/mmu/mmu.c:4549:57: sparse: sparse: cast truncates bits from constant value (ffffff33 becomes 33)
arch/x86/kvm/mmu/mmu.c:4551:56: sparse: sparse: cast truncates bits from constant value (ffffff0f becomes f)
arch/x86/kvm/mmu/mmu.c:4553:57: sparse: sparse: cast truncates bits from constant value (ffffff55 becomes 55)
vim +244 arch/x86/kvm/mmu/paging_tmpl.h
191
192 static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
193 struct kvm_mmu *mmu,
194 struct guest_walker *walker,
195 gpa_t addr, int write_fault)
196 {
197 unsigned level, index;
198 pt_element_t pte, orig_pte;
199 pt_element_t __user *ptep_user;
200 gfn_t table_gfn;
201 int ret;
202
203 /* dirty/accessed bits are not supported, so no need to update them */
204 if (!PT_HAVE_ACCESSED_DIRTY(mmu))
205 return 0;
206
207 for (level = walker->max_level; level >= walker->level; --level) {
208 pte = orig_pte = walker->ptes[level - 1];
209 table_gfn = walker->table_gfn[level - 1];
210 ptep_user = walker->ptep_user[level - 1];
211 index = offset_in_page(ptep_user) / sizeof(pt_element_t);
212 if (!(pte & PT_GUEST_ACCESSED_MASK)) {
213 trace_kvm_mmu_set_accessed_bit(table_gfn, index, sizeof(pte));
214 pte |= PT_GUEST_ACCESSED_MASK;
215 }
216 if (level == walker->level && write_fault &&
217 !(pte & PT_GUEST_DIRTY_MASK)) {
218 trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
219 #if PTTYPE == PTTYPE_EPT
220 if (kvm_x86_ops.nested_ops->write_log_dirty(vcpu, addr))
221 return -EINVAL;
222 #endif
223 pte |= PT_GUEST_DIRTY_MASK;
224 }
225 if (pte == orig_pte)
226 continue;
227
228 /*
229 * If the slot is read-only, simply do not process the accessed
230 * and dirty bits. This is the correct thing to do if the slot
231 * is ROM, and page tables in read-as-ROM/write-as-MMIO slots
232 * are only supported if the accessed and dirty bits are already
233 * set in the ROM (so that MMIO writes are never needed).
234 *
235 * Note that NPT does not allow this at all and faults, since
236 * it always wants nested page table entries for the guest
237 * page tables to be writable. And EPT works but will simply
238 * overwrite the read-only memory to set the accessed and dirty
239 * bits.
240 */
241 if (unlikely(!walker->pte_writable[level - 1]))
242 continue;
243
> 244 ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
245 if (ret)
246 return ret;
247
248 kvm_vcpu_mark_page_dirty(vcpu, table_gfn);
249 walker->ptes[level - 1] = pte;
250 }
251 return 0;
252 }
253
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
@ 2022-02-01 13:25 ` kernel test robot
0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-02-01 13:25 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 7189 bytes --]
Hi Sean,
I love your patch! Perhaps something to improve:
[auto build test WARNING on 26291c54e111ff6ba87a164d85d4a4e134b7315c]
url: https://github.com/0day-ci/linux/commits/Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
base: 26291c54e111ff6ba87a164d85d4a4e134b7315c
config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220201/202202012104.eSvVUhWh-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/c880d7a9df876f20dc3acdd893c5c71f3cda5029
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sean-Christopherson/x86-uaccess-CMPXCHG-KVM-bug-fixes/20220201-091001
git checkout c880d7a9df876f20dc3acdd893c5c71f3cda5029
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
arch/x86/kvm/mmu/mmu.c:695:9: sparse: sparse: context imbalance in 'walk_shadow_page_lockless_begin' - different lock contexts for basic block
arch/x86/kvm/mmu/mmu.c: note: in included file (through include/linux/rbtree.h, include/linux/mm_types.h, arch/x86/kvm/irq.h):
include/linux/rcupdate.h:725:9: sparse: sparse: context imbalance in 'walk_shadow_page_lockless_end' - unexpected unlock
arch/x86/kvm/mmu/mmu.c:2595:9: sparse: sparse: context imbalance in 'mmu_try_to_unsync_pages' - different lock contexts for basic block
arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
arch/x86/kvm/mmu/mmu.c: note: in included file:
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
>> arch/x86/kvm/mmu/paging_tmpl.h:244:23: sparse: sparse: dereference of noderef expression
arch/x86/kvm/mmu/mmu.c:4549:57: sparse: sparse: cast truncates bits from constant value (ffffff33 becomes 33)
arch/x86/kvm/mmu/mmu.c:4551:56: sparse: sparse: cast truncates bits from constant value (ffffff0f becomes f)
arch/x86/kvm/mmu/mmu.c:4553:57: sparse: sparse: cast truncates bits from constant value (ffffff55 becomes 55)
vim +244 arch/x86/kvm/mmu/paging_tmpl.h
191
192 static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
193 struct kvm_mmu *mmu,
194 struct guest_walker *walker,
195 gpa_t addr, int write_fault)
196 {
197 unsigned level, index;
198 pt_element_t pte, orig_pte;
199 pt_element_t __user *ptep_user;
200 gfn_t table_gfn;
201 int ret;
202
203 /* dirty/accessed bits are not supported, so no need to update them */
204 if (!PT_HAVE_ACCESSED_DIRTY(mmu))
205 return 0;
206
207 for (level = walker->max_level; level >= walker->level; --level) {
208 pte = orig_pte = walker->ptes[level - 1];
209 table_gfn = walker->table_gfn[level - 1];
210 ptep_user = walker->ptep_user[level - 1];
211 index = offset_in_page(ptep_user) / sizeof(pt_element_t);
212 if (!(pte & PT_GUEST_ACCESSED_MASK)) {
213 trace_kvm_mmu_set_accessed_bit(table_gfn, index, sizeof(pte));
214 pte |= PT_GUEST_ACCESSED_MASK;
215 }
216 if (level == walker->level && write_fault &&
217 !(pte & PT_GUEST_DIRTY_MASK)) {
218 trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
219 #if PTTYPE == PTTYPE_EPT
220 if (kvm_x86_ops.nested_ops->write_log_dirty(vcpu, addr))
221 return -EINVAL;
222 #endif
223 pte |= PT_GUEST_DIRTY_MASK;
224 }
225 if (pte == orig_pte)
226 continue;
227
228 /*
229 * If the slot is read-only, simply do not process the accessed
230 * and dirty bits. This is the correct thing to do if the slot
231 * is ROM, and page tables in read-as-ROM/write-as-MMIO slots
232 * are only supported if the accessed and dirty bits are already
233 * set in the ROM (so that MMIO writes are never needed).
234 *
235 * Note that NPT does not allow this at all and faults, since
236 * it always wants nested page table entries for the guest
237 * page tables to be writable. And EPT works but will simply
238 * overwrite the read-only memory to set the accessed and dirty
239 * bits.
240 */
241 if (unlikely(!walker->pte_writable[level - 1]))
242 continue;
243
> 244 ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
245 if (ret)
246 return ret;
247
248 kvm_vcpu_mark_page_dirty(vcpu, table_gfn);
249 walker->ptes[level - 1] = pte;
250 }
251 return 0;
252 }
253
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes
2022-02-01 1:08 [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Sean Christopherson
` (4 preceding siblings ...)
2022-02-01 1:08 ` [PATCH 5/5] KVM: x86: Bail to userspace if emulation of atomic user access faults Sean Christopherson
@ 2022-02-01 17:09 ` Tadeusz Struk
5 siblings, 0 replies; 20+ messages in thread
From: Tadeusz Struk @ 2022-02-01 17:09 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Nathan Chancellor, Nick Desaulniers
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
llvm, linux-kernel, Peter Zijlstra, syzbot+6cde2282daa792c49ab8,
Tadeusz Struk
On 1/31/22 17:08, Sean Christopherson wrote:
> Add uaccess macros for doing CMPXCHG on userspace addresses and use the
> macros to fix KVM bugs by replacing flawed code that maps memory into the
> kernel address space without proper mmu_notifier protection (or with
> broken pfn calculations in one case).
>
> Add yet another Kconfig for guarding asm_volatile_goto() to workaround a
> clang-13 bug. I've verified the test passes on gcc versions of arm64,
> PPC, RISC-V, and s390x that also pass the CC_HAS_ASM_GOTO_OUTPUT test.
>
> Patches 1-4 are tagged for stable@ as patches 3 and 4 (mostly 3) need a
> backportable fix, and doing CMPXCHG on the userspace address is the
> simplest fix from a KVM perspective.
>
> Peter Zijlstra (1):
> x86/uaccess: Implement macros for CMPXCHG on user addresses
>
> Sean Christopherson (4):
> Kconfig: Add option for asm goto w/ tied outputs to workaround
> clang-13 bug
> KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
> KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses
> KVM: x86: Bail to userspace if emulation of atomic user access faults
>
> arch/x86/include/asm/uaccess.h | 131 +++++++++++++++++++++++++++++++++
> arch/x86/kvm/mmu/paging_tmpl.h | 45 +----------
> arch/x86/kvm/x86.c | 35 ++++-----
> init/Kconfig | 4 +
> 4 files changed, 150 insertions(+), 65 deletions(-)
This also fixes the following syzbot issue:
https://syzkaller.appspot.com/bug?id=6cb6102a0a7b0c52060753dd62d070a1d1e71347
Tested-by: Tadeusz Struk <tadeusz.struk@linaro.org>
--
Thanks,
Tadeusz
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
2022-02-01 7:01 ` kernel test robot
@ 2022-02-01 19:44 ` Sean Christopherson
-1 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 19:44 UTC (permalink / raw)
To: kernel test robot
Cc: Paolo Bonzini, Nathan Chancellor, Nick Desaulniers, llvm,
kbuild-all, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm
On Tue, Feb 01, 2022, kernel test robot wrote:
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> In file included from arch/x86/kvm/mmu/mmu.c:4246:
> >> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
> ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
> ^
> arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
> __ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
> ^
> arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
> case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
> ^
> arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
> [old] "+a" (__old) \
#$*&(#$ clang.
clang isn't smart enough to avoid compiling the impossible conditions it will
throw away in the end, i.e. it compiles all cases given:
switch (8) {
case 1:
case 2:
case 4:
case 8:
}
I can fudge around that by casting the pointer, which I don't think can go sideways
if the pointer value is a signed type?
@@ -604,15 +602,15 @@ extern void __try_cmpxchg_user_wrong_size(void);
bool __ret; \
switch (sizeof(*(_ptr))) { \
case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
- (_ptr), (_oldp), \
+ (u8 *)(_ptr), (_oldp), \
(_nval), _label); \
break; \
case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
- (_ptr), (_oldp), \
+ (u16 *)(_ptr), (_oldp), \
(_nval), _label); \
break; \
case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
- (_ptr), (_oldp), \
+ (u32 *)(_ptr), (_oldp), \
(_nval), _label); \
break; \
case 8: __ret = __try_cmpxchg64_user_asm((_ptr), (_oldp), \
clang also lacks the intelligence to realize that it can/should use a single
register for encoding the memory operand and consumes both ESI and EDI, leaving
no register for the __err "+r" param in __try_cmpxchg64_user_asm(). That can be
avoided by open coding CC_SET and using a single output register for both the
result and the -EFAULT error path.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
@ 2022-02-01 19:44 ` Sean Christopherson
0 siblings, 0 replies; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 19:44 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3303 bytes --]
On Tue, Feb 01, 2022, kernel test robot wrote:
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> In file included from arch/x86/kvm/mmu/mmu.c:4246:
> >> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
> ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
> ^
> arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
> __ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
> ^
> arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
> case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
> ^
> arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
> [old] "+a" (__old) \
#$*&(#$ clang.
clang isn't smart enough to avoid compiling the impossible conditions it will
throw away in the end, i.e. it compiles all cases given:
switch (8) {
case 1:
case 2:
case 4:
case 8:
}
I can fudge around that by casting the pointer, which I don't think can go sideways
if the pointer value is a signed type?
@@ -604,15 +602,15 @@ extern void __try_cmpxchg_user_wrong_size(void);
bool __ret; \
switch (sizeof(*(_ptr))) { \
case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
- (_ptr), (_oldp), \
+ (u8 *)(_ptr), (_oldp), \
(_nval), _label); \
break; \
case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
- (_ptr), (_oldp), \
+ (u16 *)(_ptr), (_oldp), \
(_nval), _label); \
break; \
case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
- (_ptr), (_oldp), \
+ (u32 *)(_ptr), (_oldp), \
(_nval), _label); \
break; \
case 8: __ret = __try_cmpxchg64_user_asm((_ptr), (_oldp), \
clang also lacks the intelligence to realize that it can/should use a single
register for encoding the memory operand and consumes both ESI and EDI, leaving
no register for the __err "+r" param in __try_cmpxchg64_user_asm(). That can be
avoided by open coding CC_SET and using a single output register for both the
result and the -EFAULT error path.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
2022-02-01 19:44 ` Sean Christopherson
@ 2022-02-01 19:53 ` Nick Desaulniers
-1 siblings, 0 replies; 20+ messages in thread
From: Nick Desaulniers @ 2022-02-01 19:53 UTC (permalink / raw)
To: Sean Christopherson
Cc: kernel test robot, Paolo Bonzini, Nathan Chancellor, llvm,
kbuild-all, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm
On Tue, Feb 1, 2022 at 11:44 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Feb 01, 2022, kernel test robot wrote:
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> > In file included from arch/x86/kvm/mmu/mmu.c:4246:
> > >> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
> > ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
> > ^
> > arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
> > __ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
> > ^
> > arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
> > case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
> > ^
> > arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
> > [old] "+a" (__old) \
>
> #$*&(#$ clang.
>
> clang isn't smart enough to avoid compiling the impossible conditions it will
> throw away in the end, i.e. it compiles all cases given:
>
> switch (8) {
> case 1:
> case 2:
> case 4:
> case 8:
> }
This is because Clang generally handles diagnostics, while LLVM
handles optimizations, in a pipeline, in that order. In order to know
not to emit a diagnostic, optimizations need to be run. Do you prefer
that diagnostics only get emitted for code not optimized out? In this
case, yes, but in others folks could reasonably desire the others.
Sorry, but this is a BIG architectural difference between compilers.
(FWIW; I've personally implemented warnings in clang that don't work
that way. The tradeoff is excessive memory usage and work during
compile time to basically pessimistically emit the equivalent of debug
info, and do all the work that entails keeping it up to date as you
transform the code, even if it may never trigger a warning. Changing
all diagnostics to work that way would be tantamount to rewriting most
of the frontend and would be slower and use more memory at runtime).
>
> I can fudge around that by casting the pointer, which I don't think can go sideways
> if the pointer value is a signed type?
>
> @@ -604,15 +602,15 @@ extern void __try_cmpxchg_user_wrong_size(void);
> bool __ret; \
> switch (sizeof(*(_ptr))) { \
> case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
> - (_ptr), (_oldp), \
> + (u8 *)(_ptr), (_oldp), \
> (_nval), _label); \
> break; \
> case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
> - (_ptr), (_oldp), \
> + (u16 *)(_ptr), (_oldp), \
> (_nval), _label); \
> break; \
> case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
> - (_ptr), (_oldp), \
> + (u32 *)(_ptr), (_oldp), \
> (_nval), _label); \
> break; \
> case 8: __ret = __try_cmpxchg64_user_asm((_ptr), (_oldp), \
>
>
> clang also lacks the intelligence to realize that it can/should use a single
> register for encoding the memory operand and consumes both ESI and EDI, leaving
> no register for the __err "+r" param in __try_cmpxchg64_user_asm(). That can be
> avoided by open coding CC_SET and using a single output register for both the
> result and the -EFAULT error path.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits
@ 2022-02-01 19:53 ` Nick Desaulniers
0 siblings, 0 replies; 20+ messages in thread
From: Nick Desaulniers @ 2022-02-01 19:53 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4453 bytes --]
On Tue, Feb 1, 2022 at 11:44 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Feb 01, 2022, kernel test robot wrote:
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> > In file included from arch/x86/kvm/mmu/mmu.c:4246:
> > >> arch/x86/kvm/mmu/paging_tmpl.h:244:9: error: invalid output size for constraint '+a'
> > ret = __try_cmpxchg_user(ptep_user, &orig_pte, pte, fault);
> > ^
> > arch/x86/include/asm/uaccess.h:629:11: note: expanded from macro '__try_cmpxchg_user'
> > __ret = !unsafe_try_cmpxchg_user(_ptr, _oldp, _nval, _label); \
> > ^
> > arch/x86/include/asm/uaccess.h:606:18: note: expanded from macro 'unsafe_try_cmpxchg_user'
> > case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
> > ^
> > arch/x86/include/asm/uaccess.h:467:22: note: expanded from macro '__try_cmpxchg_user_asm'
> > [old] "+a" (__old) \
>
> #$*&(#$ clang.
>
> clang isn't smart enough to avoid compiling the impossible conditions it will
> throw away in the end, i.e. it compiles all cases given:
>
> switch (8) {
> case 1:
> case 2:
> case 4:
> case 8:
> }
This is because Clang generally handles diagnostics, while LLVM
handles optimizations, in a pipeline, in that order. In order to know
not to emit a diagnostic, optimizations need to be run. Do you prefer
that diagnostics only get emitted for code not optimized out? In this
case, yes, but in others folks could reasonably desire the others.
Sorry, but this is a BIG architectural difference between compilers.
(FWIW; I've personally implemented warnings in clang that don't work
that way. The tradeoff is excessive memory usage and work during
compile time to basically pessimistically emit the equivalent of debug
info, and do all the work that entails keeping it up to date as you
transform the code, even if it may never trigger a warning. Changing
all diagnostics to work that way would be tantamount to rewriting most
of the frontend and would be slower and use more memory at runtime).
>
> I can fudge around that by casting the pointer, which I don't think can go sideways
> if the pointer value is a signed type?
>
> @@ -604,15 +602,15 @@ extern void __try_cmpxchg_user_wrong_size(void);
> bool __ret; \
> switch (sizeof(*(_ptr))) { \
> case 1: __ret = __try_cmpxchg_user_asm("b", "q", \
> - (_ptr), (_oldp), \
> + (u8 *)(_ptr), (_oldp), \
> (_nval), _label); \
> break; \
> case 2: __ret = __try_cmpxchg_user_asm("w", "r", \
> - (_ptr), (_oldp), \
> + (u16 *)(_ptr), (_oldp), \
> (_nval), _label); \
> break; \
> case 4: __ret = __try_cmpxchg_user_asm("l", "r", \
> - (_ptr), (_oldp), \
> + (u32 *)(_ptr), (_oldp), \
> (_nval), _label); \
> break; \
> case 8: __ret = __try_cmpxchg64_user_asm((_ptr), (_oldp), \
>
>
> clang also lacks the intelligence to realize that it can/should use a single
> register for encoding the memory operand and consumes both ESI and EDI, leaving
> no register for the __err "+r" param in __try_cmpxchg64_user_asm(). That can be
> avoided by open coding CC_SET and using a single output register for both the
> result and the -EFAULT error path.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug
2022-02-01 1:08 ` [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug Sean Christopherson
@ 2022-02-01 20:16 ` Nick Desaulniers
2022-02-01 20:56 ` Sean Christopherson
0 siblings, 1 reply; 20+ messages in thread
From: Nick Desaulniers @ 2022-02-01 20:16 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Nathan Chancellor, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel,
Peter Zijlstra, syzbot+6cde2282daa792c49ab8
On Mon, Jan 31, 2022 at 5:08 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Add a config option to guard (future) usage of asm_volatile_goto() that
> includes "tied outputs", i.e. "+" constraints that specify both an input
> and output parameter. clang-13 has a bug[1] that causes compilation of
> such inline asm to fail, and KVM wants to use a "+m" constraint to
> implement a uaccess form of CMPXCHG[2]. E.g. the test code fails with
>
> <stdin>:1:29: error: invalid operand in inline asm: '.long (${1:l}) - .'
> int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }
> ^
> <stdin>:1:29: error: unknown token in expression
> <inline asm>:1:9: note: instantiated into assembly here
> .long () - .
> ^
> 2 errors generated.
>
> on clang-13, but passes on gcc (with appropriate asm goto support). The
> bug is fixed in clang-14, but won't be backported to clang-13 as the
> changes are too invasive/risky.
LGTM.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
If you're going to respin the series, consider adding a comment in the
source along the lines of:
```
clang-14 and gcc-11 fixed this.
```
or w/e. This helps us find (via grep) and remove cruft when the
minimum supported compiler versions are updated.
Note: gcc-10 had a bug with the symbolic references to labels when
using tied constraints.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98096
Both compilers had bugs here, and it may be worth mentioning that in
the commit message.
>
> [1] https://github.com/ClangBuiltLinux/linux/issues/1512
> [2] https://lore.kernel.org/all/YfMruK8%2F1izZ2VHS@google.com
>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> init/Kconfig | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index e9119bf54b1f..a206b21703be 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -77,6 +77,10 @@ config CC_HAS_ASM_GOTO_OUTPUT
> depends on CC_HAS_ASM_GOTO
> def_bool $(success,echo 'int foo(int x) { asm goto ("": "=r"(x) ::: bar); return x; bar: return 0; }' | $(CC) -x c - -c -o /dev/null)
>
> +config CC_HAS_ASM_GOTO_TIED_OUTPUT
> + depends on CC_HAS_ASM_GOTO_OUTPUT
> + def_bool $(success,echo 'int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }' | $CC -x c - -c -o /dev/null)
> +
> config TOOLS_SUPPORT_RELR
> def_bool $(success,env "CC=$(CC)" "LD=$(LD)" "NM=$(NM)" "OBJCOPY=$(OBJCOPY)" $(srctree)/scripts/tools-support-relr.sh)
>
> --
> 2.35.0.rc2.247.g8bbb082509-goog
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug
2022-02-01 20:16 ` Nick Desaulniers
@ 2022-02-01 20:56 ` Sean Christopherson
2022-02-01 21:15 ` Nick Desaulniers
0 siblings, 1 reply; 20+ messages in thread
From: Sean Christopherson @ 2022-02-01 20:56 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Paolo Bonzini, Nathan Chancellor, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel,
Peter Zijlstra, syzbot+6cde2282daa792c49ab8
On Tue, Feb 01, 2022, Nick Desaulniers wrote:
> On Mon, Jan 31, 2022 at 5:08 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Add a config option to guard (future) usage of asm_volatile_goto() that
> > includes "tied outputs", i.e. "+" constraints that specify both an input
> > and output parameter. clang-13 has a bug[1] that causes compilation of
> > such inline asm to fail, and KVM wants to use a "+m" constraint to
> > implement a uaccess form of CMPXCHG[2]. E.g. the test code fails with
> >
> > <stdin>:1:29: error: invalid operand in inline asm: '.long (${1:l}) - .'
> > int foo(int *x) { asm goto (".long (%l[bar]) - .\n": "+m"(*x) ::: bar); return *x; bar: return 0; }
> > ^
> > <stdin>:1:29: error: unknown token in expression
> > <inline asm>:1:9: note: instantiated into assembly here
> > .long () - .
> > ^
> > 2 errors generated.
> >
> > on clang-13, but passes on gcc (with appropriate asm goto support). The
> > bug is fixed in clang-14, but won't be backported to clang-13 as the
> > changes are too invasive/risky.
>
> LGTM.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> If you're going to respin the series, consider adding a comment in the
> source along the lines of:
> ```
> clang-14 and gcc-11 fixed this.
> ```
> or w/e. This helps us find (via grep) and remove cruft when the
> minimum supported compiler versions are updated.
Will do, a new version is definitely needed.
> Note: gcc-10 had a bug with the symbolic references to labels when
> using tied constraints.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98096
>
> Both compilers had bugs here, and it may be worth mentioning that in
> the commit message.
Is this wording accurate?
gcc also had a similar bug[3], fixed in gcc-11, where gcc failed to
account for its behavior of assigning two numbers to tied outputs (one
for input, one for output) when evaluating symbolic references.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug
2022-02-01 20:56 ` Sean Christopherson
@ 2022-02-01 21:15 ` Nick Desaulniers
0 siblings, 0 replies; 20+ messages in thread
From: Nick Desaulniers @ 2022-02-01 21:15 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Nathan Chancellor, Vitaly Kuznetsov, Wanpeng Li,
Jim Mattson, Joerg Roedel, kvm, llvm, linux-kernel,
Peter Zijlstra, syzbot+6cde2282daa792c49ab8
On Tue, Feb 1, 2022 at 12:56 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Feb 01, 2022, Nick Desaulniers wrote:
> > Note: gcc-10 had a bug with the symbolic references to labels when
> > using tied constraints.
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98096
> >
> > Both compilers had bugs here, and it may be worth mentioning that in
> > the commit message.
>
> Is this wording accurate?
>
> gcc also had a similar bug[3], fixed in gcc-11, where gcc failed to
> account for its behavior of assigning two numbers to tied outputs (one
> for input, one for output) when evaluating symbolic references.
SGTM
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-02-01 21:16 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 1:08 [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Sean Christopherson
2022-02-01 1:08 ` [PATCH 1/5] Kconfig: Add option for asm goto w/ tied outputs to workaround clang-13 bug Sean Christopherson
2022-02-01 20:16 ` Nick Desaulniers
2022-02-01 20:56 ` Sean Christopherson
2022-02-01 21:15 ` Nick Desaulniers
2022-02-01 1:08 ` [PATCH 2/5] x86/uaccess: Implement macros for CMPXCHG on user addresses Sean Christopherson
2022-02-01 1:08 ` [PATCH 3/5] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits Sean Christopherson
2022-02-01 7:01 ` kernel test robot
2022-02-01 7:01 ` kernel test robot
2022-02-01 19:44 ` Sean Christopherson
2022-02-01 19:44 ` Sean Christopherson
2022-02-01 19:53 ` Nick Desaulniers
2022-02-01 19:53 ` Nick Desaulniers
2022-02-01 13:25 ` kernel test robot
2022-02-01 13:25 ` kernel test robot
2022-02-01 1:08 ` [PATCH 4/5] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses Sean Christopherson
2022-02-01 9:25 ` kernel test robot
2022-02-01 9:25 ` kernel test robot
2022-02-01 1:08 ` [PATCH 5/5] KVM: x86: Bail to userspace if emulation of atomic user access faults Sean Christopherson
2022-02-01 17:09 ` [PATCH 0/5] x86: uaccess CMPXCHG + KVM bug fixes Tadeusz Struk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.