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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5945ECDB483 for ; Fri, 13 Oct 2023 20:28:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232248AbjJMU2W (ORCPT ); Fri, 13 Oct 2023 16:28:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37564 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232091AbjJMU2T (ORCPT ); Fri, 13 Oct 2023 16:28:19 -0400 Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 62C16DC for ; Fri, 13 Oct 2023 13:28:11 -0700 (PDT) Received: by mail-pl1-x62f.google.com with SMTP id d9443c01a7336-1c9d4f08d7cso13155ad.0 for ; Fri, 13 Oct 2023 13:28:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1697228891; x=1697833691; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=hQUYjiIQQuKU5hgKTdO4y33bIv6BP+oihrBBQRGii7o=; b=UYA8RGf51Wm2aL9KLEoBcMOSHOhgyks35se5vv9WlreKNxE3WgsMtlPVMUcq+IxrZU F2CqkLOyuyzm1XTZ/zMScp7bY9uAyYB7ZViTjBDlyGgvby1D/naFJrFTrcW5sf/ysRRZ IKexW0WeDXtOiKJE0RSYvyLXvwNEQ2t9GRWQS7kYUCnAQHCVsp0Nl4MMvUT5mwiJHO1E bX0Ka8j8xIznky4MbbX5FFjE48eST84GA8Hjoqb3wUTDyA+vgXa4KNR+ejkuuU68Dd7C jNWoZtN8YV4P3Cn9sEZ0mQbZt/iySIABLjl2w/r3yyuLcbyPJ3IlDottr7dviW9ITdyJ viXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697228891; x=1697833691; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=hQUYjiIQQuKU5hgKTdO4y33bIv6BP+oihrBBQRGii7o=; b=o/P3wocTqkmCKd8efAXZ+ACN7EDLfTh0PtXMYI237LFAxmcl0U7Nx96rEzSteNAAKF dZLG77KZ4nyTi4cie7jdLDY2KCva0rPjYk35/k02kCBygiR+tviCYKSzMeHwZemZjXCy tGJ67RVPbxWp34kdq4ZdNPtNvthDSRiyr15azQh7dcsqYfx5e+xuZNMhGBCchW98YgcO D7MsdMgsGtvZOLXsCgqJb1r3feuqQoTm9c49YfpNuPJ3u8ZM5E7dhkB7IRKmSO8Mxlut xJPPSaG9S1xtCIZa7XzFz/uoqH6IvnXj/LMQCsn+obQbKzfGRSbi8M593g8rNbLx7cd7 Ggbg== X-Gm-Message-State: AOJu0YwjcSgQhpTVbu/oLxiKUv8YZ7wuBagOEf/qZz6VqIZhX/BTpVvG e8ZVM1KRP7353k9Z+G7e+eeHAejbSuUijJyJS01c+Q== X-Google-Smtp-Source: AGHT+IF+wbBQUzCClYbIsavRQnVYMQpZNLI2ELD+7IF1suaSE/kY6/BRJ0kMNOpbN7SpoHfHj5jpKNFUpadFhMTbM8Q= X-Received: by 2002:a17:903:40c1:b0:1ca:42a:1773 with SMTP id t1-20020a17090340c100b001ca042a1773mr33105pld.12.1697228890595; Fri, 13 Oct 2023 13:28:10 -0700 (PDT) MIME-Version: 1.0 References: <20231009230858.3444834-1-rananta@google.com> <20231009230858.3444834-3-rananta@google.com> In-Reply-To: From: Raghavendra Rao Ananta Date: Fri, 13 Oct 2023 13:27:58 -0700 Message-ID: Subject: Re: [PATCH v7 02/12] KVM: arm64: PMU: Set the default PMU for the guest before vCPU reset To: Oliver Upton Cc: Marc Zyngier , Alexandru Elisei , James Morse , Suzuki K Poulose , Paolo Bonzini , Zenghui Yu , Shaoqin Huang , Jing Zhang , Reiji Watanabe , Colton Lewis , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, kvm@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 10, 2023 at 3:25=E2=80=AFPM Oliver Upton wrote: > > Hi Raghu, > > On Mon, Oct 09, 2023 at 11:08:48PM +0000, Raghavendra Rao Ananta wrote: > > From: Reiji Watanabe > > > > The following patches will use the number of counters information > > from the arm_pmu and use this to set the PMCR.N for the guest > > during vCPU reset. However, since the guest is not associated > > with any arm_pmu until userspace configures the vPMU device > > attributes, and a reset can happen before this event, assign a > > default PMU to the guest just before doing the reset. > > > > Signed-off-by: Reiji Watanabe > > Signed-off-by: Raghavendra Rao Ananta > > --- > > arch/arm64/kvm/arm.c | 20 ++++++++++++++++++++ > > arch/arm64/kvm/pmu-emul.c | 12 ++---------- > > include/kvm/arm_pmu.h | 6 ++++++ > > 3 files changed, 28 insertions(+), 10 deletions(-) > > > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > > index 78b0970eb8e6..708a53b70a7b 100644 > > --- a/arch/arm64/kvm/arm.c > > +++ b/arch/arm64/kvm/arm.c > > @@ -1313,6 +1313,23 @@ static bool kvm_vcpu_init_changed(struct kvm_vcp= u *vcpu, > > KVM_VCPU_MAX_FEATURES); > > } > > > > +static int kvm_vcpu_set_pmu(struct kvm_vcpu *vcpu) > > +{ > > + struct kvm *kvm =3D vcpu->kvm; > > + > > + if (!kvm_arm_support_pmu_v3()) > > + return -EINVAL; > > This check is pointless; the vCPU feature flags have been sanitised at > this point, and a requirement of having PMUv3 is that this predicate is > true. > Oh yes. I'll avoid this in v8. > > + /* > > + * When the vCPU has a PMU, but no PMU is set for the guest > > + * yet, set the default one. > > + */ > > + if (unlikely(!kvm->arch.arm_pmu)) > > + return kvm_arm_set_default_pmu(kvm); > > + > > + return 0; > > +} > > + > > Apologies, I believe I was unclear last time around as to what I was > wanting here. Let's call this thing kvm_setup_vcpu() such that we can > add other one-time setup activities to it in the future. > > Something like: > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 96641e442039..4896a44108e0 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1265,19 +1265,17 @@ static bool kvm_vcpu_init_changed(struct kvm_vcpu= *vcpu, > KVM_VCPU_MAX_FEATURES); > } > > -static int kvm_vcpu_set_pmu(struct kvm_vcpu *vcpu) > +static int kvm_setup_vcpu(struct kvm_vcpu *vcpu) > { > struct kvm *kvm =3D vcpu->kvm; > > - if (!kvm_arm_support_pmu_v3()) > - return -EINVAL; > - > /* > * When the vCPU has a PMU, but no PMU is set for the guest > * yet, set the default one. > */ > - if (unlikely(!kvm->arch.arm_pmu)) > - return kvm_arm_set_default_pmu(kvm); > + if (kvm_vcpu_has_pmu(vcpu) && !kvm->arch.arm_pmu && > + kvm_arm_set_default_pmu(kvm)) > + return -EINVAL; > > return 0; > } > @@ -1297,7 +1295,8 @@ static int __kvm_vcpu_set_target(struct kvm_vcpu *v= cpu, > > bitmap_copy(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEAT= URES); > > - if (kvm_vcpu_has_pmu(vcpu) && kvm_vcpu_set_pmu(vcpu)) > + ret =3D kvm_setup_vcpu(vcpu); > + if (ret) > goto out_unlock; > > /* Now we know what it is, we can reset it. */ > Introducing kvm_setup_vcpu() seems better than directly calling kvm_vcpu_set_pmu(), which feels like it's crashing a party. Thank you. Raghavendra > -- > Thanks, > Oliver