From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f202.google.com (mail-pg1-f202.google.com [209.85.215.202]) (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 8405247A5C for ; Tue, 5 Nov 2024 02:22:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730773362; cv=none; b=ZldSZa79bgBgSNFUTxO3FM82dVb8IJO6/AKcNY8GEsi5Eif95cg7WSTJtuSGSisZEmG840n04jEL1/bJWM6llLZRTjfQKCfnTZRKeo3Axy7RjJddgc5gJSINlqvryOcfaU/qg6Y+U/JPsesGgsQ/AjfxbsPEa7+Iq/ur0nyZ+mE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730773362; c=relaxed/simple; bh=fASZrlOrGSWP18gmYt8bKLkLXj2wDQ1MzYPz3q9daX0=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=Lys316NTc2nNM7T8S+xVt7k2s3A0tgT+PZvP7GjFdCbH61yjf38EKCKjVzXTQPQRYTKhanpMjMiVIVWlaAro8HFQxvVxR69UVjN+t/KPyqPxc45/QlvpF1rhprF/9c0pCGr3Ur6zMW8Xc9noPmi5UJLDLnJHaoV9caNsAmmHLWQ= 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=iAKmoQB7; arc=none smtp.client-ip=209.85.215.202 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="iAKmoQB7" Received: by mail-pg1-f202.google.com with SMTP id 41be03b00d2f7-7d4dee4dfdcso4785122a12.2 for ; Mon, 04 Nov 2024 18:22:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730773360; x=1731378160; 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=rTrj4Vb+gjmZyvOereONM4xaExQ9e1t81Zeg65PUba4=; b=iAKmoQB71pLTav7c9f1jte4hXpXhoLYra/46jA8BKdnxGvHglOC5M81NOxHFyIAovM c6J65z/iZHJO5ta4k+EoRB5e6JsDvkRAZox+K/b7JeaiGcXNT/LeT9yV+MjYEeTuqPd9 +Hk1rUmsuBH+ogF9a5u/EAyUIUkd/FedM/oxeuj53T84f6RZ4rRD06vkfMm3S7T4h2ff L22bJoHwqM3viX3/34rIz35z5wqOPR5dfRSkZBlt3w11TdGImoVk7Inp9B+6KnzuGKc3 eqb/AZ2jG/KGOWftp40HLfBdO1icyzxMubQrKX1U4qCdH6YMuSagxgA71KqgSGB9ct7q 8SwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730773360; x=1731378160; 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=rTrj4Vb+gjmZyvOereONM4xaExQ9e1t81Zeg65PUba4=; b=pvDW7nME5+w1VbhKEh9Gj3NO5vKia442jtx15HN89xvVTKIeNp5DZJZfpQW0XcL+Hk PKVSZJhSAX+5eXed7VXlxhCjcGdfuAkSC0yrQADKMTy2QuHLGKQTh+cIdCbCLYLNxiyA 1znA9HFKoHrFcHXAubf4dKpylBPjJleMNJrIlGeU4b51YjJPrckN8U/zYHBXjSndZGE7 IuXpK9KEIr8MMDcrqYl4WTfAgDWNYf2MAwZV0DytpGTCEuGFmX+hI0uVYAzIeq8f2tB7 IZM4HofhB2gJqQoWzbcMdAUESuJFzChcEz/SSKLEhhFJJni9RIvYaBgtMNGDNffkAP9U FOvQ== X-Gm-Message-State: AOJu0YyukkZ1YU8Lz1wUVdSQdUVC8HDQsG8QbMKtUHizNElmWwRZiHwy GbYIQYNSAxJef8EeU9k3VRWhm9vkUCvK0qx+/hwhWrFD6barlHxAjuOWdVD05XKSPL0g6z5oIJ5 yKg== X-Google-Smtp-Source: AGHT+IH0unGMBQfLgioqGmr0yzEMUoopms0yCwfTp21z4CVHah4e9K57FkA7ZEnCFrqk1qMnNAGjr3IC698= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:9d:3983:ac13:c240]) (user=seanjc job=sendgmr) by 2002:a05:6a02:4d07:b0:7ea:97c3:7a8f with SMTP id 41be03b00d2f7-7ee3a90d751mr34651a12.10.1730773359794; Mon, 04 Nov 2024 18:22:39 -0800 (PST) Date: Mon, 4 Nov 2024 18:22:38 -0800 In-Reply-To: <74089281-3208-435d-93b3-22f1d794dfae@amd.com> Precedence: bulk X-Mailing-List: kvm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241004053341.5726-1-manali.shukla@amd.com> <20241004053341.5726-3-manali.shukla@amd.com> <74089281-3208-435d-93b3-22f1d794dfae@amd.com> Message-ID: Subject: Re: [PATCH v3 2/4] KVM: SVM: Enable Bus lock threshold exit From: Sean Christopherson To: Manali Shukla Cc: kvm@vger.kernel.org, linux-kselftest@vger.kernel.org, pbonzini@redhat.com, shuah@kernel.org, nikunj@amd.com, thomas.lendacky@amd.com, vkuznets@redhat.com, bp@alien8.de, babu.moger@amd.com, Neeraj Upadhyay Content-Type: text/plain; charset="us-ascii" On Sun, Nov 03, 2024, Manali Shukla wrote: > On 10/15/2024 11:19 PM, Sean Christopherson wrote: > > On Fri, Oct 04, 2024, Manali Shukla wrote: > ... > >> > >> +static int bus_lock_exit(struct kvm_vcpu *vcpu) > >> +{ > >> + struct vcpu_svm *svm = to_svm(vcpu); > >> + > >> + vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK; > >> + vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK; > >> + > >> + /* > >> + * Reload the counter with value greater than '0'. > > > > The value quite obviously must be exactly '1', not simply greater than '0. I also > > think this is the wrong place to set the counter. Rather than set the counter at > > the time of exit, KVM should implement a vcpu->arch.complete_userspace_io callback > > and set the counter to '1' if and only if RIP (or LIP, but I have no objection to > > keeping things simple) is unchanged. It's a bit of extra complexity, but it will > > make it super obvious why KVM is setting the counter to '1'. And, if userspace > > wants to stuff state and move past the instruction, e.g. by emulating the guilty > > instruction, then KVM won't unnecessarily allow a bus lock in the guest. > > > > And then the comment can be: > > > > /* > > * If userspace has NOT change RIP, then KVM's ABI is to let the guest > > * execute the bus-locking instruction. Set the bus lock counter to '1' > > * to effectively step past the bus lock. > > */ > > > > The bus lock threshold intercept feature is available for SEV-ES and SEV-SNP > guests too. The rip where the bus lock exit occurred, is not available in > bus_lock_exit handler for SEV-ES and SEV-SNP guests, so the above-mentioned > solution won't work with SEV-ES and SEV-SNP guests. > > I would propose to add the above-mentioned solution only for normal and SEV guests > and unconditionally reloading of bus_lock_counter to 1 in complete_userspace_io > for SEV-ES and SEV-SNP guests. Yeah, that works. Though I would condition the check on guest_state_protected. Actually, and this is going to seem really stupid, but everything will Just Work if you use kvm_get_linear_rip() and kvm_is_linear_rip(), because kvm_get_linear_rip() returns '0' for vCPUs with protected state. I.e. KVM will do a rather superfluous cui() callback, but otherwise it's fine. Silly, but in many ways preferable to special casing ES and SNP guests. On a related topic, can you add a refacotring prep patch to move linear_rip out of kvm_pio_request and place it next to complete_userspace_io? There's nothing port I/O specific about that field, it just so happens to that port I/O is the only case where KVM's ABI is to let userspace stuff state (to emulate RESET) without first completing the I/O instruction. I.e. diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 8e8ca6dab2b2..8617b15096a6 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -406,7 +406,6 @@ struct kvm_rmap_head { }; struct kvm_pio_request { - unsigned long linear_rip; unsigned long count; int in; int port; @@ -884,6 +883,7 @@ struct kvm_vcpu_arch { bool emulate_regs_need_sync_to_vcpu; bool emulate_regs_need_sync_from_vcpu; int (*complete_userspace_io)(struct kvm_vcpu *vcpu); + unsigned long cui_linear_rip; gpa_t time; struct pvclock_vcpu_time_info hv_clock; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 425a301911a6..7704d3901481 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -9308,7 +9308,7 @@ static int complete_fast_pio_out(struct kvm_vcpu *vcpu) { vcpu->arch.pio.count = 0; - if (unlikely(!kvm_is_linear_rip(vcpu, vcpu->arch.pio.linear_rip))) + if (unlikely(!kvm_is_linear_rip(vcpu, vcpu->arch.cui_linear_rip))) return 1; return kvm_skip_emulated_instruction(vcpu); @@ -9333,7 +9333,7 @@ static int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, complete_fast_pio_out_port_0x7e; kvm_skip_emulated_instruction(vcpu); } else { - vcpu->arch.pio.linear_rip = kvm_get_linear_rip(vcpu); + vcpu->arch.cui_linear_rip = kvm_get_linear_rip(vcpu); vcpu->arch.complete_userspace_io = complete_fast_pio_out; } return 0; @@ -9346,7 +9346,7 @@ static int complete_fast_pio_in(struct kvm_vcpu *vcpu) /* We should only ever be called with arch.pio.count equal to 1 */ BUG_ON(vcpu->arch.pio.count != 1); - if (unlikely(!kvm_is_linear_rip(vcpu, vcpu->arch.pio.linear_rip))) { + if (unlikely(!kvm_is_linear_rip(vcpu, vcpu->arch.cui_linear_rip))) { vcpu->arch.pio.count = 0; return 1; } @@ -9375,7 +9375,7 @@ static int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, return ret; } - vcpu->arch.pio.linear_rip = kvm_get_linear_rip(vcpu); + vcpu->arch.cui_linear_rip = kvm_get_linear_rip(vcpu); vcpu->arch.complete_userspace_io = complete_fast_pio_in; return 0;