From: Marc Zyngier <maz@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 5/5] KVM: arm64: Disable privileged hypercalls after pKVM finalisation
Date: Thu, 23 Sep 2021 14:11:17 +0100 [thread overview]
Message-ID: <87y27ntp1m.wl-maz@kernel.org> (raw)
In-Reply-To: <20210923130211.GB15934@willie-the-truck>
On Thu, 23 Sep 2021 14:02:11 +0100,
Will Deacon <will@kernel.org> wrote:
>
> On Thu, Sep 23, 2021 at 01:56:21PM +0100, Marc Zyngier wrote:
> > On Thu, 23 Sep 2021 12:22:56 +0100,
> > Will Deacon <will@kernel.org> wrote:
[...]
> > > static void handle_host_hcall(struct kvm_cpu_context *host_ctxt)
> > > {
> > > DECLARE_REG(unsigned long, id, host_ctxt, 0);
> > > + unsigned long hcall_min = 0;
> > > hcall_t hfn;
> > >
> > > + if (static_branch_unlikely(&kvm_protected_mode_initialized))
> > > + hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize;
> > > +
> > > id -= KVM_HOST_SMCCC_ID(0);
> > >
> > > - if (unlikely(id >= ARRAY_SIZE(host_hcall)))
> > > + if (unlikely(id < hcall_min || id >= ARRAY_SIZE(host_hcall)))
> >
> > So I can still issue a pkvm_prot_finalize after finalisation? Seems
> > odd. As hcall_min has to be inclusive, you probably want it to be set
> > to __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp once protected.
>
> Yeah, I ended up addresing that one in the previous patch. The problem is
> that we need to allow pkvm_prot_finalize to be called on each CPU, so I
> think we'd end up having an extra "really finalize damnit!" call to be
> issued _once_ after each CPU is done with the finalisation if we want
> to lock it down.
>
> The approach I took instead is to make pkvm_prot_finalize return -EBUSY
> if it's called on a CPU where it's already been called.
Ah, I see. Serves me right for reading patches out of order. Finalise
is of course per-CPU, and the static key global. Epic fail.
Probably deserves a comment, because I'm surely going to jump at that
again in three months.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
Quentin Perret <qperret@google.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH 5/5] KVM: arm64: Disable privileged hypercalls after pKVM finalisation
Date: Thu, 23 Sep 2021 14:11:17 +0100 [thread overview]
Message-ID: <87y27ntp1m.wl-maz@kernel.org> (raw)
In-Reply-To: <20210923130211.GB15934@willie-the-truck>
On Thu, 23 Sep 2021 14:02:11 +0100,
Will Deacon <will@kernel.org> wrote:
>
> On Thu, Sep 23, 2021 at 01:56:21PM +0100, Marc Zyngier wrote:
> > On Thu, 23 Sep 2021 12:22:56 +0100,
> > Will Deacon <will@kernel.org> wrote:
[...]
> > > static void handle_host_hcall(struct kvm_cpu_context *host_ctxt)
> > > {
> > > DECLARE_REG(unsigned long, id, host_ctxt, 0);
> > > + unsigned long hcall_min = 0;
> > > hcall_t hfn;
> > >
> > > + if (static_branch_unlikely(&kvm_protected_mode_initialized))
> > > + hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize;
> > > +
> > > id -= KVM_HOST_SMCCC_ID(0);
> > >
> > > - if (unlikely(id >= ARRAY_SIZE(host_hcall)))
> > > + if (unlikely(id < hcall_min || id >= ARRAY_SIZE(host_hcall)))
> >
> > So I can still issue a pkvm_prot_finalize after finalisation? Seems
> > odd. As hcall_min has to be inclusive, you probably want it to be set
> > to __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp once protected.
>
> Yeah, I ended up addresing that one in the previous patch. The problem is
> that we need to allow pkvm_prot_finalize to be called on each CPU, so I
> think we'd end up having an extra "really finalize damnit!" call to be
> issued _once_ after each CPU is done with the finalisation if we want
> to lock it down.
>
> The approach I took instead is to make pkvm_prot_finalize return -EBUSY
> if it's called on a CPU where it's already been called.
Ah, I see. Serves me right for reading patches out of order. Finalise
is of course per-CPU, and the static key global. Epic fail.
Probably deserves a comment, because I'm surely going to jump at that
again in three months.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-09-23 13:11 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-23 11:22 [PATCH 0/5] KVM: arm64: Restrict host hypercalls when pKVM is enabled Will Deacon
2021-09-23 11:22 ` Will Deacon
2021-09-23 11:22 ` [PATCH 1/5] arm64: Prevent kexec and hibernation if is_protected_kvm_enabled() Will Deacon
2021-09-23 11:22 ` Will Deacon
2021-09-23 11:45 ` Mark Rutland
2021-09-23 11:45 ` Mark Rutland
2021-09-23 12:29 ` Will Deacon
2021-09-23 12:29 ` Will Deacon
2021-09-23 11:22 ` [PATCH 2/5] KVM: arm64: Reject stub hypercalls after pKVM has been initialised Will Deacon
2021-09-23 11:22 ` Will Deacon
2021-09-29 13:37 ` Quentin Perret
2021-09-29 13:37 ` Quentin Perret
2021-09-23 11:22 ` [PATCH 3/5] KVM: arm64: Propagate errors from __pkvm_prot_finalize hypercall Will Deacon
2021-09-23 11:22 ` Will Deacon
2021-09-29 13:36 ` Quentin Perret
2021-09-29 13:36 ` Quentin Perret
2021-10-05 11:30 ` Will Deacon
2021-10-05 11:30 ` Will Deacon
2021-09-23 11:22 ` [PATCH 4/5] KVM: arm64: Prevent re-finalisation of pKVM for a given CPU Will Deacon
2021-09-23 11:22 ` Will Deacon
2021-09-29 13:41 ` Quentin Perret
2021-09-29 13:41 ` Quentin Perret
2021-09-23 11:22 ` [PATCH 5/5] KVM: arm64: Disable privileged hypercalls after pKVM finalisation Will Deacon
2021-09-23 11:22 ` Will Deacon
2021-09-23 12:56 ` Marc Zyngier
2021-09-23 12:56 ` Marc Zyngier
2021-09-23 13:02 ` Will Deacon
2021-09-23 13:02 ` Will Deacon
2021-09-23 13:11 ` Marc Zyngier [this message]
2021-09-23 13:11 ` Marc Zyngier
2021-09-23 12:58 ` Will Deacon
2021-09-23 12:58 ` Will Deacon
2021-09-23 12:21 ` [PATCH 0/5] KVM: arm64: Restrict host hypercalls when pKVM is enabled Marc Zyngier
2021-09-23 12:21 ` Marc Zyngier
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=87y27ntp1m.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--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 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.