From: Raghavendra Rao Ananta <rananta@google.com>
To: Reiji Watanabe <reijiw@google.com>
Cc: Marc Zyngier <maz@kernel.org>, Andrew Jones <drjones@redhat.com>,
James Morse <james.morse@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, Peter Shier <pshier@google.com>,
Ricardo Koller <ricarkol@google.com>,
Oliver Upton <oupton@google.com>,
Jing Zhang <jingzhangos@google.com>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org
Subject: Re: [RFC PATCH v3 01/11] KVM: Capture VM start
Date: Fri, 7 Jan 2022 15:43:08 -0800 [thread overview]
Message-ID: <CAJHc60za+E-zEO5v2QeKuifoXznPnt5n--g1dAN5jgsuq+SxrA@mail.gmail.com> (raw)
In-Reply-To: <CAAeT=Fxyct=WLUvfbpROKwB9huyt+QdJnKTaj8c5NKk+UY51WQ@mail.gmail.com>
Hi Reiji,
On Thu, Jan 6, 2022 at 10:07 PM Reiji Watanabe <reijiw@google.com> wrote:
>
> Hi Raghu,
>
> On Tue, Jan 4, 2022 at 11:49 AM Raghavendra Rao Ananta
> <rananta@google.com> wrote:
> >
> > Capture the start of the KVM VM, which is basically the
> > start of any vCPU run. This state of the VM is helpful
> > in the upcoming patches to prevent user-space from
> > configuring certain VM features after the VM has started
> > running.
> >
> > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> > ---
> > include/linux/kvm_host.h | 3 +++
> > virt/kvm/kvm_main.c | 9 +++++++++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index c310648cc8f1..d0bd8f7a026c 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -623,6 +623,7 @@ struct kvm {
> > struct notifier_block pm_notifier;
> > #endif
> > char stats_id[KVM_STATS_NAME_SIZE];
> > + bool vm_started;
>
> Since KVM_RUN on any vCPUs doesn't necessarily mean that the VM
> started yet, the name might be a bit misleading IMHO. I would
> think 'has_run_once' or 'ran_once' might be more clear (?).
>
I always struggle with the names; but if you feel that 'ran_once'
makes more sense for a reader, I can change it.
>
> > };
> >
> > #define kvm_err(fmt, ...) \
> > @@ -1666,6 +1667,8 @@ static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
> > }
> > }
> >
> > +#define kvm_vm_has_started(kvm) (kvm->vm_started)
> > +
> > extern bool kvm_rebooting;
> >
> > extern unsigned int halt_poll_ns;
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 72c4e6b39389..962b91ac2064 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -3686,6 +3686,7 @@ static long kvm_vcpu_ioctl(struct file *filp,
> > int r;
> > struct kvm_fpu *fpu = NULL;
> > struct kvm_sregs *kvm_sregs = NULL;
> > + struct kvm *kvm = vcpu->kvm;
> >
> > if (vcpu->kvm->mm != current->mm || vcpu->kvm->vm_dead)
> > return -EIO;
> > @@ -3723,6 +3724,14 @@ static long kvm_vcpu_ioctl(struct file *filp,
> > if (oldpid)
> > synchronize_rcu();
> > put_pid(oldpid);
> > +
> > + /*
> > + * Since we land here even on the first vCPU run,
> > + * we can mark that the VM has started running.
> > + */
>
> It might be nicer to add a comment why the code below gets kvm->lock.
>
I've been going back and forth on this one. Initially I considered
simply going with atomic_t, but the patch 4/11 (KVM: arm64: Setup a
framework for hypercall bitmap firmware registers)
kvm_arm_set_fw_reg_bmap()'s implementation felt like we need a lock to
have the whole 'is the register busy?' operation atomic. But, that's
just one of the applications.
> Anyway, the patch generally looks good to me, and thank you
> for making this change (it works for my purpose as well).
>
> Reviewed-by: Reiji Watanabe <reijiw@google.com>
>
Glad that it's helping you as well and thanks for the review.
Regards,
Raghavendra
> Thanks,
> Reiji
>
>
> > + mutex_lock(&kvm->lock);
> > + kvm->vm_started = true;
> > + mutex_unlock(&kvm->lock);
> > }
> > r = kvm_arch_vcpu_ioctl_run(vcpu);
> > trace_kvm_userspace_exit(vcpu->run->exit_reason, r);
> > --
> > 2.34.1.448.ga2b2bfdf31-goog
> >
next prev parent reply other threads:[~2022-01-07 23:43 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-04 19:49 [RFC PATCH v3 00/11] KVM: arm64: Add support for hypercall services selection Raghavendra Rao Ananta
2022-01-04 19:49 ` [RFC PATCH v3 01/11] KVM: Capture VM start Raghavendra Rao Ananta
2022-01-07 6:06 ` Reiji Watanabe
2022-01-07 23:43 ` Raghavendra Rao Ananta [this message]
2022-01-08 0:04 ` Jim Mattson
2022-01-10 23:07 ` Raghavendra Rao Ananta
2022-01-10 23:57 ` Jim Mattson
2022-01-11 18:52 ` Raghavendra Rao Ananta
2022-01-11 19:16 ` Jim Mattson
2022-01-12 18:29 ` Raghavendra Rao Ananta
2022-01-13 17:21 ` Sean Christopherson
2022-01-14 0:42 ` Raghavendra Rao Ananta
2022-01-14 1:10 ` Sean Christopherson
2022-01-14 21:51 ` Reiji Watanabe
2022-01-18 22:54 ` Raghavendra Rao Ananta
2022-01-19 0:07 ` Sean Christopherson
2022-01-19 7:47 ` Reiji Watanabe
2022-01-20 0:27 ` Sean Christopherson
2022-01-20 19:16 ` Raghavendra Rao Ananta
2022-01-25 15:15 ` Marc Zyngier
2022-01-25 15:10 ` Marc Zyngier
2022-01-11 0:03 ` Reiji Watanabe
2022-01-11 18:54 ` Raghavendra Rao Ananta
2022-01-08 1:06 ` Sean Christopherson
2022-01-10 23:23 ` Raghavendra Rao Ananta
2022-01-11 17:36 ` Sean Christopherson
2022-01-11 18:46 ` Raghavendra Rao Ananta
2022-01-11 19:04 ` Sean Christopherson
2022-01-12 18:08 ` Raghavendra Rao Ananta
2022-01-12 18:24 ` Sean Christopherson
2022-01-12 18:31 ` Sean Christopherson
2022-01-04 19:49 ` [RFC PATCH v3 02/11] KVM: arm64: Factor out firmware register handling from psci.c Raghavendra Rao Ananta
2022-01-04 19:49 ` [RFC PATCH v3 03/11] KVM: Introduce KVM_CAP_ARM_HVC_FW_REG_BMAP Raghavendra Rao Ananta
2022-01-08 5:40 ` Reiji Watanabe
2022-01-10 23:40 ` Raghavendra Rao Ananta
2022-01-11 4:33 ` Reiji Watanabe
2022-01-04 19:49 ` [RFC PATCH v3 04/11] KVM: arm64: Setup a framework for hypercall bitmap firmware registers Raghavendra Rao Ananta
2022-01-10 6:28 ` Reiji Watanabe
2022-01-11 0:50 ` Raghavendra Rao Ananta
2022-01-12 5:11 ` Reiji Watanabe
2022-01-12 18:02 ` Raghavendra Rao Ananta
2022-01-14 6:23 ` Reiji Watanabe
2022-01-19 6:42 ` Jason Wang
2022-01-19 10:21 ` Marc Zyngier
2022-01-04 19:49 ` [RFC PATCH v3 05/11] KVM: arm64: Add standard hypervisor firmware register Raghavendra Rao Ananta
2022-01-04 19:49 ` [RFC PATCH v3 06/11] KVM: arm64: Add vendor " Raghavendra Rao Ananta
2022-01-04 19:49 ` [RFC PATCH v3 07/11] Docs: KVM: Add doc for the bitmap firmware registers Raghavendra Rao Ananta
2022-01-04 19:49 ` [RFC PATCH v3 08/11] Docs: KVM: Rename psci.rst to hypercalls.rst Raghavendra Rao Ananta
2022-01-04 19:49 ` [RFC PATCH v3 09/11] tools: Import ARM SMCCC definitions Raghavendra Rao Ananta
2022-01-04 19:49 ` [RFC PATCH v3 10/11] selftests: KVM: aarch64: Introduce hypercall ABI test Raghavendra Rao Ananta
2022-01-04 19:49 ` [RFC PATCH v3 11/11] selftests: KVM: aarch64: Add the bitmap firmware registers to get-reg-list Raghavendra Rao Ananta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJHc60za+E-zEO5v2QeKuifoXznPnt5n--g1dAN5jgsuq+SxrA@mail.gmail.com \
--to=rananta@google.com \
--cc=alexandru.elisei@arm.com \
--cc=catalin.marinas@arm.com \
--cc=drjones@redhat.com \
--cc=james.morse@arm.com \
--cc=jingzhangos@google.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=oupton@google.com \
--cc=pbonzini@redhat.com \
--cc=pshier@google.com \
--cc=reijiw@google.com \
--cc=ricarkol@google.com \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).