From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id ABBBBCCF9E3 for ; Fri, 24 Oct 2025 16:57:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=L2UFd5GZcSoUJ8rPtxJ8Mm5UIY46eTM0GXDph4PvTwc=; b=gj1K6nmglDF1hF4CsNi4oPVg89 E0DM6UBPyMs95gD8Igm+uByAuPzu0Rr2dEfQ0GL4xZf+76Yiggf1R+GSCYcOdvv7UmOvA//e+RbhN JOXY64kgWVYUIRp8oLXuD3cBArWWoC3Z0NNoXXWHzfffFpA9s4Ore3iOUH7+2BuOZ45rQP3/uYogv WzvSYfQDovPs3jkHXrLToOmBUto9etm17LIJnytjX7E9Edxz5wPq/zpaHS5VBOq/OM4xcRZwnONQ0 p+I+b1WOc8z6X9GJMpCaWNUukELCRUXhBCOp55D0mYtjc1+Nh+QnTct4LCjtFSZx/SPUVBKBEkS6U 3SVlUXAw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vCL6U-0000000A2ug-0zGe; Fri, 24 Oct 2025 16:57:26 +0000 Received: from mail-pj1-x104a.google.com ([2607:f8b0:4864:20::104a]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vCL6R-0000000A2tO-0LSD for kvm-riscv@lists.infradead.org; Fri, 24 Oct 2025 16:57:24 +0000 Received: by mail-pj1-x104a.google.com with SMTP id 98e67ed59e1d1-33bcb779733so2075943a91.3 for ; Fri, 24 Oct 2025 09:57:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1761325041; x=1761929841; darn=lists.infradead.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=kCSFoTED+bN9czctrBnViFjaUxjZI+PzjY/BcjiPyhU=; b=BsrPW85jw5FAv9Oxya6bWz5auwwE60sVlJ4s1smVv8Bn87lTbXDmyNN3nL28SeaVT4 /fnM/+EkI7sHsOfvF9ZIrH0hrC4I5qRBDSpNfwOcM7z6WmVfXH7ZiO5YTCUBhpQMHszf VoXG/E9S/AQySv6KTJOIeRDOKiU723dkEK219AZAQlAd7GYGnncktabqjL9+aQrnS+aB dffPt/neBAHmQYsxSZiFESE2V1gYNALtdeUy0NupVYWegJqTaG/Kup50/hfjeF4XWL4s xj6wYnEGDACvTQ9AsKi6qfjMIIajPILXGuij7+hrmmK3aB7ZGp2u0QAxAncoPvFmi9D9 ZooA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761325041; x=1761929841; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kCSFoTED+bN9czctrBnViFjaUxjZI+PzjY/BcjiPyhU=; b=wp9II/dT9e5SfnjzPVHW0A5DgHpPxywmhogi6gsEZlIGSraLuGdpJBafNaar0Z5nnp qZBo4WWgHCn/WboUDWk2ZkzpSzLPj+CfYK12kX01aAPBi8m9LJUFAq636++MwiDd/Gia XzUKBOijHTTL2VtZo/9ti2sQDR8tzYzSFMhNZqeSOdkwME9/RFTXcHcdx/s5yZ6ronDC n7f6ia2zAXYJWLtShJD7tUW7ueVGOmRLHc+v6QWfehBSttbAeBxuhhHt2a8zVnIdE9mg EPl0hl66djDh9MZYCF8LFVpjlkIiYGHIybvsJhV82X2BGn8fKKciY8gPoqGeSO3rU78w GG2g== X-Forwarded-Encrypted: i=1; AJvYcCWbEWXj8W6xxrBD301QsYosyHj5/ZP+HFrtOYKKGTP3Jol8lDbI8BYcT6f+fyGPFoRURl76ohld9pk=@lists.infradead.org X-Gm-Message-State: AOJu0YxtYktUuQij7qqsXnkeFmnYJWGgCogUcD0xlHBE3wq25K1NgIbz A+M6R7ju7Qf+1oQzfa04mZU7VnkrQDasjn77iyQYDWNFWo4S0Z51gnRVRjB26AKEZe/Gt8N4bLr 9AyMbIw== X-Google-Smtp-Source: AGHT+IHXmSeD3kNK6POSE1hGR7Jvio0RnirzUH7WSrLaKZ09lPaFZJIUb09S/aBT/hnx0ZxmJITFzdMKVRo= X-Received: from pjnu4.prod.google.com ([2002:a17:90a:8904:b0:339:dc19:ae5d]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a17:90b:48c8:b0:33b:ba55:f5dd with SMTP id 98e67ed59e1d1-33bcf93ab88mr32766320a91.37.1761325041555; Fri, 24 Oct 2025 09:57:21 -0700 (PDT) Date: Fri, 24 Oct 2025 09:57:20 -0700 In-Reply-To: Mime-Version: 1.0 References: <20251017003244.186495-1-seanjc@google.com> <20251017003244.186495-25-seanjc@google.com> Message-ID: Subject: Re: [PATCH v3 24/25] KVM: TDX: Guard VM state transitions with "all" the locks From: Sean Christopherson To: Yan Zhao Cc: Marc Zyngier , Oliver Upton , Tianrui Zhao , Bibo Mao , Huacai Chen , Madhavan Srinivasan , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Paolo Bonzini , "Kirill A. Shutemov" , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org, loongarch@lists.linux.dev, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, x86@kernel.org, linux-coco@lists.linux.dev, linux-kernel@vger.kernel.org, Ira Weiny , Kai Huang , Michael Roth , Vishal Annapurve , Rick Edgecombe , Ackerley Tng , Binbin Wu X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20251024_095723_125128_27E52055 X-CRM114-Status: GOOD ( 21.62 ) X-BeenThere: kvm-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kvm-riscv" Errors-To: kvm-riscv-bounces+kvm-riscv=archiver.kernel.org@lists.infradead.org On Fri, Oct 24, 2025, Yan Zhao wrote: > On Thu, Oct 16, 2025 at 05:32:42PM -0700, Sean Christopherson wrote: > > Acquire kvm->lock, kvm->slots_lock, and all vcpu->mutex locks when > > servicing ioctls that (a) transition the TD to a new state, i.e. when > > doing INIT or FINALIZE or (b) are only valid if the TD is in a specific > > state, i.e. when initializing a vCPU or memory region. Acquiring "all" > > the locks fixes several KVM_BUG_ON() situations where a SEAMCALL can fail > > due to racing actions, e.g. if tdh_vp_create() contends with either > > tdh_mr_extend() or tdh_mr_finalize(). > > > > For all intents and purposes, the paths in question are fully serialized, > > i.e. there's no reason to try and allow anything remotely interesting to > > happen. Smack 'em with a big hammer instead of trying to be "nice". > > > > Acquire kvm->lock to prevent VM-wide things from happening, slots_lock to > > prevent kvm_mmu_zap_all_fast(), and _all_ vCPU mutexes to prevent vCPUs > slots_lock to prevent kvm_mmu_zap_memslot()? > kvm_mmu_zap_all_fast() does not operate on the mirror root. Oh, right. > We may have missed a zap in the guest_memfd punch hole path: > > The SEAMCALLs tdh_mem_range_block(), tdh_mem_track() tdh_mem_page_remove() > in the guest_memfd punch hole path are only protected by the filemap invaliate > lock and mmu_lock, so they could contend with v1 version of tdh_vp_init(). > > (I'm writing a selftest to verify this, haven't been able to reproduce > tdh_vp_init(v1) returning BUSY yet. However, this race condition should be > theoretically possible.) > > Resources SHARED users EXCLUSIVE users > ------------------------------------------------------------------------ > (1) TDR tdh_mng_rdwr tdh_mng_create > tdh_vp_create tdh_mng_add_cx > tdh_vp_addcx tdh_mng_init > tdh_vp_init(v0) tdh_mng_vpflushdone > tdh_vp_enter tdh_mng_key_config > tdh_vp_flush tdh_mng_key_freeid > tdh_vp_rd_wr tdh_mr_extend > tdh_mem_sept_add tdh_mr_finalize > tdh_mem_sept_remove tdh_vp_init(v1) > tdh_mem_page_aug tdh_mem_page_add > tdh_mem_page_remove > tdh_mem_range_block > tdh_mem_track > tdh_mem_range_unblock > tdh_phymem_page_reclaim > > Do you think we can acquire the mmu_lock for cmd KVM_TDX_INIT_VCPU? Ugh, I'd rather not? Refresh me, what's the story with "v1"? Are we now on v2? If this is effectively limited to deprecated TDX modules, my vote would be to ignore the flaw and avoid even more complexity in KVM. > > @@ -3155,12 +3198,13 @@ int tdx_vcpu_unlocked_ioctl(struct kvm_vcpu *vcpu, void __user *argp) > > if (r) > > return r; > > > > + CLASS(tdx_vm_state_guard, guard)(kvm); > Should we move the guard to inside each cmd? Then there's no need to acquire the > locks in the default cases. No, I don't think it's a good tradeoff. We'd also need to move vcpu_{load,put}() into the cmd helpers, and theoretically slowing down a bad ioctl invocation due to taking extra locks is a complete non-issue. -- kvm-riscv mailing list kvm-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kvm-riscv From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f73.google.com (mail-pj1-f73.google.com [209.85.216.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3C22633A02D for ; Fri, 24 Oct 2025 16:57:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.73 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1761325044; cv=none; b=L32wc+nmDwsrHH4Y5ODJj2UWaQKsuBVJ3b/V5Euko7nDukQdAlHX3xStL1RGR1niBokcxAAzpswqgvpVeIcsdM2fNkVWo1E/tlUNDPevGnxlxiIzwXXXXlJ3UwUf0hUvZDbFGxAy/sPeXcAXzS7v5nPhVK0+EOjFe81du9SjDRU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1761325044; c=relaxed/simple; bh=znxPntvSK0iLuvDlLrIc6oUoFoqXWddOvlpeLWfjD/8=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=APrPzk8PtUlf8j//O869PiNC5oeygFTvED1eoH5oYcYGNiPIg9OoF5l7wdbo/4wuifZblvZBXs476jvnkUPMLHW9X6gDy3r4FTsCIO+2tfbSCAR8dRK3fGB0jYufKz8D+Lx9BrNaJWf+TMgg4Zp38isROWq4CNWysdw6pk+DnIo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=hKhw31zy; arc=none smtp.client-ip=209.85.216.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="hKhw31zy" Received: by mail-pj1-f73.google.com with SMTP id 98e67ed59e1d1-33bcb779733so2075941a91.3 for ; Fri, 24 Oct 2025 09:57:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1761325041; x=1761929841; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=kCSFoTED+bN9czctrBnViFjaUxjZI+PzjY/BcjiPyhU=; b=hKhw31zyrAbgqsXNoWbtQ3K4Q5NQkmPMhTqiLcUwwNDWTKJgc/p527KT7Prdtijhlk TymHKg62C+q0vryN6ELJrmV1MP2MdGyLe3ofkLmyEVzmA3GXDV2/1FexHUwi9FWnxPRM jUXc+5FeS6BYh8J0t5QUU6QXpOoQRDFUkQNZpAsqr6oRoc4SF29INiz95pkFsS4lMVc+ pL0fPLBRbihVQcw1voYIETiCGbacIIyy/xrAD8/U/kX4SER5Mm9+1CNC0Mkf4xYwTPtE GqsNbkanHvwbztdQ/5wIU86NIoZJzBlicVxqq3RF0mFJ5kHvx1oQJzkDXnUOm+aI37qi DeOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761325041; x=1761929841; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kCSFoTED+bN9czctrBnViFjaUxjZI+PzjY/BcjiPyhU=; b=wsk2LkAN61VsYJ7nw6kgZnCq1xq0mISNNOKxi6W2RIzfcDyoHFvRSJWO4Qr+0QgtFQ RMt3jLNdeUp8EWUEKlXOCMPyNOFJdUQegLyWDGNah+JPs3VKTA5JyPpdv43A35N89kgg LDlIJ203LQXWWP8dcvKsGFkTKkPYo75k7HFUrAh8E6rcc3q+0EMcN3sY9ZqOUWc45VB5 R/DI7wGbUtys5eFf5YXVdcmRWffL5CqsoBxMx4yOcXDTGvUrQpG1s9knCzH7e2hMMw2E KsGLD+oo8iUPeTo9HoA4uyroW0t9IbA023nLDSQsu1ydv5wHhRu72Gc0N+vNvG6sHNUA /mAw== X-Forwarded-Encrypted: i=1; AJvYcCV3HK/jM6Hb+7RNbqe8zK1aamBUk3P4dP7ZATW+hluTHWeYii5uwGU6aKrbnAYdZiY1cK4=@vger.kernel.org X-Gm-Message-State: AOJu0YxRlWqy7r28W9ZEAYjVKWgv87W/DeenWIVruUcgPVG3cRfctVLu BqT8BvCjkhj99Dw8j3fO7oPnHgeb6uVWcLwOFHkIRXUw83ntmiugizohJmwbck805YhoRcqt7iP zi4zDgg== X-Google-Smtp-Source: AGHT+IHXmSeD3kNK6POSE1hGR7Jvio0RnirzUH7WSrLaKZ09lPaFZJIUb09S/aBT/hnx0ZxmJITFzdMKVRo= X-Received: from pjnu4.prod.google.com ([2002:a17:90a:8904:b0:339:dc19:ae5d]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a17:90b:48c8:b0:33b:ba55:f5dd with SMTP id 98e67ed59e1d1-33bcf93ab88mr32766320a91.37.1761325041555; Fri, 24 Oct 2025 09:57:21 -0700 (PDT) Date: Fri, 24 Oct 2025 09:57:20 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: kvm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20251017003244.186495-1-seanjc@google.com> <20251017003244.186495-25-seanjc@google.com> Message-ID: Subject: Re: [PATCH v3 24/25] KVM: TDX: Guard VM state transitions with "all" the locks From: Sean Christopherson To: Yan Zhao Cc: Marc Zyngier , Oliver Upton , Tianrui Zhao , Bibo Mao , Huacai Chen , Madhavan Srinivasan , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Paolo Bonzini , "Kirill A. Shutemov" , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org, loongarch@lists.linux.dev, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, x86@kernel.org, linux-coco@lists.linux.dev, linux-kernel@vger.kernel.org, Ira Weiny , Kai Huang , Michael Roth , Vishal Annapurve , Rick Edgecombe , Ackerley Tng , Binbin Wu Content-Type: text/plain; charset="us-ascii" On Fri, Oct 24, 2025, Yan Zhao wrote: > On Thu, Oct 16, 2025 at 05:32:42PM -0700, Sean Christopherson wrote: > > Acquire kvm->lock, kvm->slots_lock, and all vcpu->mutex locks when > > servicing ioctls that (a) transition the TD to a new state, i.e. when > > doing INIT or FINALIZE or (b) are only valid if the TD is in a specific > > state, i.e. when initializing a vCPU or memory region. Acquiring "all" > > the locks fixes several KVM_BUG_ON() situations where a SEAMCALL can fail > > due to racing actions, e.g. if tdh_vp_create() contends with either > > tdh_mr_extend() or tdh_mr_finalize(). > > > > For all intents and purposes, the paths in question are fully serialized, > > i.e. there's no reason to try and allow anything remotely interesting to > > happen. Smack 'em with a big hammer instead of trying to be "nice". > > > > Acquire kvm->lock to prevent VM-wide things from happening, slots_lock to > > prevent kvm_mmu_zap_all_fast(), and _all_ vCPU mutexes to prevent vCPUs > slots_lock to prevent kvm_mmu_zap_memslot()? > kvm_mmu_zap_all_fast() does not operate on the mirror root. Oh, right. > We may have missed a zap in the guest_memfd punch hole path: > > The SEAMCALLs tdh_mem_range_block(), tdh_mem_track() tdh_mem_page_remove() > in the guest_memfd punch hole path are only protected by the filemap invaliate > lock and mmu_lock, so they could contend with v1 version of tdh_vp_init(). > > (I'm writing a selftest to verify this, haven't been able to reproduce > tdh_vp_init(v1) returning BUSY yet. However, this race condition should be > theoretically possible.) > > Resources SHARED users EXCLUSIVE users > ------------------------------------------------------------------------ > (1) TDR tdh_mng_rdwr tdh_mng_create > tdh_vp_create tdh_mng_add_cx > tdh_vp_addcx tdh_mng_init > tdh_vp_init(v0) tdh_mng_vpflushdone > tdh_vp_enter tdh_mng_key_config > tdh_vp_flush tdh_mng_key_freeid > tdh_vp_rd_wr tdh_mr_extend > tdh_mem_sept_add tdh_mr_finalize > tdh_mem_sept_remove tdh_vp_init(v1) > tdh_mem_page_aug tdh_mem_page_add > tdh_mem_page_remove > tdh_mem_range_block > tdh_mem_track > tdh_mem_range_unblock > tdh_phymem_page_reclaim > > Do you think we can acquire the mmu_lock for cmd KVM_TDX_INIT_VCPU? Ugh, I'd rather not? Refresh me, what's the story with "v1"? Are we now on v2? If this is effectively limited to deprecated TDX modules, my vote would be to ignore the flaw and avoid even more complexity in KVM. > > @@ -3155,12 +3198,13 @@ int tdx_vcpu_unlocked_ioctl(struct kvm_vcpu *vcpu, void __user *argp) > > if (r) > > return r; > > > > + CLASS(tdx_vm_state_guard, guard)(kvm); > Should we move the guard to inside each cmd? Then there's no need to acquire the > locks in the default cases. No, I don't think it's a good tradeoff. We'd also need to move vcpu_{load,put}() into the cmd helpers, and theoretically slowing down a bad ioctl invocation due to taking extra locks is a complete non-issue. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0B542CCD1BF for ; Fri, 24 Oct 2025 16:57:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=jLBTzP1kcWWYHgXwta6ptLbZEemqwXKiI1mvMAzxCEQ=; b=iD6LdOwVlW7ZOdmFG4s4SVqLef 41u4G6j90fuWHyVKSHVO5z4/zVVczwariyL51HYfqO137JOc8h8gMJsN2gVYTTEsHOnUHIRHIQMly PHpAqUmNUhcdllLpQqKfSsnIcD29cWw9aHNQLsRyM6tPRvbEFT3iqzuff8aIdQWztRbsi90LnkXcy 7pK2afCIZE1xCL1l/D4KUYuEEOT6lQ8VEksrC8SPSDrOc/eyhGNWTSd2eOwUhbsFwR/8PCYeMv88A 4KIYMihTZ0D3AWBqUy2IVsq7UOq4I/I/10n4kClf1TZ5tSUczduqV1TAMhlEKLh+STtlSAaDhU9n0 yaBBmI7Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vCL6S-0000000A2uK-3qzP; Fri, 24 Oct 2025 16:57:24 +0000 Received: from mail-pj1-x1049.google.com ([2607:f8b0:4864:20::1049]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vCL6R-0000000A2tP-0MAR for linux-riscv@lists.infradead.org; Fri, 24 Oct 2025 16:57:24 +0000 Received: by mail-pj1-x1049.google.com with SMTP id 98e67ed59e1d1-33be01bcda8so2347938a91.2 for ; Fri, 24 Oct 2025 09:57:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1761325041; x=1761929841; darn=lists.infradead.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=kCSFoTED+bN9czctrBnViFjaUxjZI+PzjY/BcjiPyhU=; b=BsrPW85jw5FAv9Oxya6bWz5auwwE60sVlJ4s1smVv8Bn87lTbXDmyNN3nL28SeaVT4 /fnM/+EkI7sHsOfvF9ZIrH0hrC4I5qRBDSpNfwOcM7z6WmVfXH7ZiO5YTCUBhpQMHszf VoXG/E9S/AQySv6KTJOIeRDOKiU723dkEK219AZAQlAd7GYGnncktabqjL9+aQrnS+aB dffPt/neBAHmQYsxSZiFESE2V1gYNALtdeUy0NupVYWegJqTaG/Kup50/hfjeF4XWL4s xj6wYnEGDACvTQ9AsKi6qfjMIIajPILXGuij7+hrmmK3aB7ZGp2u0QAxAncoPvFmi9D9 ZooA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761325042; x=1761929842; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kCSFoTED+bN9czctrBnViFjaUxjZI+PzjY/BcjiPyhU=; b=rx6dbv85tyttgzbPusKUvIlowmvvPtZ7KEuFUkXNWcGwkbSJfE3RB7VxZG8DjtPyRE Pc1Jut7wHvGB2/xwdjAFpynQZvMvjm8cbI+BPu3dMJt3kZ4a/SGqb2Q+8fIkfCMdz7Y6 0ufC9LZxOFlLPbe4UrHbRZmGLZkNrEWSVUjm4+bDF7r6vv3y8s27uAZNiinibvxnaa7B AuoKZAgAbGu8gtrTzqQoCDJQKPG8vJB4rWkBvv881E3QLi54EH/nUPZg/kFj9kROK/GC LRc+FttLzok63qZjQ1q1qPk67qf8NXLctpnunXkm4J7NTyHNQSrXbt53kFVDhO19qaU2 30wQ== X-Forwarded-Encrypted: i=1; AJvYcCUvb2QLx8XHdUqdxhFkGKckm+wpA2y5w0whfgr2tOjzDzRLHBBn69wRiUOiZsYZP6v6VzUJL9ACDNU1gg==@lists.infradead.org X-Gm-Message-State: AOJu0Yw4Y4ULNH0Fx/3yhYNnGunNOPtOzMjHcmKEuNeLdDN1l9DxILfS kE7/0N/nlbEYbNRwwX8fC+q2ZGhMBScoFdLbwLrdB7gqMwMdXbGw+jM09Sg02A+Z4vfxwCOTyml jcpfNEw== X-Google-Smtp-Source: AGHT+IHXmSeD3kNK6POSE1hGR7Jvio0RnirzUH7WSrLaKZ09lPaFZJIUb09S/aBT/hnx0ZxmJITFzdMKVRo= X-Received: from pjnu4.prod.google.com ([2002:a17:90a:8904:b0:339:dc19:ae5d]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a17:90b:48c8:b0:33b:ba55:f5dd with SMTP id 98e67ed59e1d1-33bcf93ab88mr32766320a91.37.1761325041555; Fri, 24 Oct 2025 09:57:21 -0700 (PDT) Date: Fri, 24 Oct 2025 09:57:20 -0700 In-Reply-To: Mime-Version: 1.0 References: <20251017003244.186495-1-seanjc@google.com> <20251017003244.186495-25-seanjc@google.com> Message-ID: Subject: Re: [PATCH v3 24/25] KVM: TDX: Guard VM state transitions with "all" the locks From: Sean Christopherson To: Yan Zhao Cc: Marc Zyngier , Oliver Upton , Tianrui Zhao , Bibo Mao , Huacai Chen , Madhavan Srinivasan , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Paolo Bonzini , "Kirill A. Shutemov" , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org, loongarch@lists.linux.dev, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, x86@kernel.org, linux-coco@lists.linux.dev, linux-kernel@vger.kernel.org, Ira Weiny , Kai Huang , Michael Roth , Vishal Annapurve , Rick Edgecombe , Ackerley Tng , Binbin Wu X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20251024_095723_125195_F502ECAE X-CRM114-Status: GOOD ( 21.62 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Fri, Oct 24, 2025, Yan Zhao wrote: > On Thu, Oct 16, 2025 at 05:32:42PM -0700, Sean Christopherson wrote: > > Acquire kvm->lock, kvm->slots_lock, and all vcpu->mutex locks when > > servicing ioctls that (a) transition the TD to a new state, i.e. when > > doing INIT or FINALIZE or (b) are only valid if the TD is in a specific > > state, i.e. when initializing a vCPU or memory region. Acquiring "all" > > the locks fixes several KVM_BUG_ON() situations where a SEAMCALL can fail > > due to racing actions, e.g. if tdh_vp_create() contends with either > > tdh_mr_extend() or tdh_mr_finalize(). > > > > For all intents and purposes, the paths in question are fully serialized, > > i.e. there's no reason to try and allow anything remotely interesting to > > happen. Smack 'em with a big hammer instead of trying to be "nice". > > > > Acquire kvm->lock to prevent VM-wide things from happening, slots_lock to > > prevent kvm_mmu_zap_all_fast(), and _all_ vCPU mutexes to prevent vCPUs > slots_lock to prevent kvm_mmu_zap_memslot()? > kvm_mmu_zap_all_fast() does not operate on the mirror root. Oh, right. > We may have missed a zap in the guest_memfd punch hole path: > > The SEAMCALLs tdh_mem_range_block(), tdh_mem_track() tdh_mem_page_remove() > in the guest_memfd punch hole path are only protected by the filemap invaliate > lock and mmu_lock, so they could contend with v1 version of tdh_vp_init(). > > (I'm writing a selftest to verify this, haven't been able to reproduce > tdh_vp_init(v1) returning BUSY yet. However, this race condition should be > theoretically possible.) > > Resources SHARED users EXCLUSIVE users > ------------------------------------------------------------------------ > (1) TDR tdh_mng_rdwr tdh_mng_create > tdh_vp_create tdh_mng_add_cx > tdh_vp_addcx tdh_mng_init > tdh_vp_init(v0) tdh_mng_vpflushdone > tdh_vp_enter tdh_mng_key_config > tdh_vp_flush tdh_mng_key_freeid > tdh_vp_rd_wr tdh_mr_extend > tdh_mem_sept_add tdh_mr_finalize > tdh_mem_sept_remove tdh_vp_init(v1) > tdh_mem_page_aug tdh_mem_page_add > tdh_mem_page_remove > tdh_mem_range_block > tdh_mem_track > tdh_mem_range_unblock > tdh_phymem_page_reclaim > > Do you think we can acquire the mmu_lock for cmd KVM_TDX_INIT_VCPU? Ugh, I'd rather not? Refresh me, what's the story with "v1"? Are we now on v2? If this is effectively limited to deprecated TDX modules, my vote would be to ignore the flaw and avoid even more complexity in KVM. > > @@ -3155,12 +3198,13 @@ int tdx_vcpu_unlocked_ioctl(struct kvm_vcpu *vcpu, void __user *argp) > > if (r) > > return r; > > > > + CLASS(tdx_vm_state_guard, guard)(kvm); > Should we move the guard to inside each cmd? Then there's no need to acquire the > locks in the default cases. No, I don't think it's a good tradeoff. We'd also need to move vcpu_{load,put}() into the cmd helpers, and theoretically slowing down a bad ioctl invocation due to taking extra locks is a complete non-issue. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv