From: Sudeep Holla <sudeep.holla@arm.com>
To: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Sebastian Ene <sebastianene@google.com>,
catalin.marinas@arm.com, james.morse@arm.com,
jean-philippe@linaro.org, maz@kernel.org,
Sudeep Holla <sudeep.holla@arm.com>,
oliver.upton@linux.dev, qperret@google.com, qwandor@google.com,
suzuki.poulose@arm.com, tabba@google.com, will@kernel.org,
yuzenghui@huawei.com, vdonnefort@google.com,
kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, kernel-team@android.com
Subject: Re: [PATCH] KVM: arm64: Add support for FFA_PARTITION_INFO_GET
Date: Thu, 11 Apr 2024 10:05:17 +0100 [thread overview]
Message-ID: <ZhenzfndkNwIehhj@bogus> (raw)
In-Reply-To: <Zhaoz9E/sw6jVnci@lpieralisi>
On Wed, Apr 10, 2024 at 04:57:19PM +0200, Lorenzo Pieralisi wrote:
> On Tue, Apr 09, 2024 at 03:19:08PM +0000, Sebastian Ene wrote:
> > Handle the FFA_PARTITION_INFO_GET host call inside the pKVM hypervisor
> > and copy the response message back to the host buffers. Save the
> > returned FF-A version as we will need it later to interpret the response
> > from the TEE.
> >
> > Signed-off-by: Sebastian Ene <sebastianene@google.com>
> > ---
> > arch/arm64/kvm/hyp/nvhe/ffa.c | 49 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > index 320f2eaa14a9..72fc365bc7a8 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > @@ -67,6 +67,7 @@ struct kvm_ffa_buffers {
> > */
> > static struct kvm_ffa_buffers hyp_buffers;
> > static struct kvm_ffa_buffers host_buffers;
> > +static u32 ffa_version;
> >
> > static void ffa_to_smccc_error(struct arm_smccc_res *res, u64 ffa_errno)
> > {
> > @@ -640,6 +641,49 @@ static bool do_ffa_features(struct arm_smccc_res *res,
> > return true;
> > }
> >
> > +static void do_ffa_part_get(struct arm_smccc_res *res,
> > + struct kvm_cpu_context *ctxt)
> > +{
> > + DECLARE_REG(u32, uuid0, ctxt, 1);
> > + DECLARE_REG(u32, uuid1, ctxt, 2);
> > + DECLARE_REG(u32, uuid2, ctxt, 3);
> > + DECLARE_REG(u32, uuid3, ctxt, 4);
> > + DECLARE_REG(u32, flags, ctxt, 5);
> > + u32 off, count, sz, buf_sz;
> > +
> > + hyp_spin_lock(&host_buffers.lock);
> > + if (!host_buffers.rx) {
> > + ffa_to_smccc_res(res, FFA_RET_INVALID_PARAMETERS);
> > + goto out_unlock;
> > + }
> > +
> > + arm_smccc_1_1_smc(FFA_PARTITION_INFO_GET, uuid0, uuid1,
> > + uuid2, uuid3, flags, 0, 0,
> > + res);
> > +
> > + if (res->a0 != FFA_SUCCESS)
> > + goto out_unlock;
> > +
> > + count = res->a2;
> > + if (!count)
> > + goto out_unlock;
> > +
> > + if (ffa_version > FFA_VERSION_1_0) {
> > + buf_sz = sz = res->a3;
> > + if (sz > sizeof(struct ffa_partition_info))
> > + buf_sz = sizeof(struct ffa_partition_info);
>
> We are copying buf_sz but (correctly ?) returning res->a3 to the caller,
> which is allowed to expect res->a3 bytes to be filled since that's what
> firmware reported.
>
> Technically this is not a problem at present, because the caller
> (ie the FF-A driver) and the hypervisor rely on the same descriptor
> structures (and buf_sz can't be != sizeof(struct ffa_partition_info),
> anything else is a bug as we stand); they must be kept in sync though as
> the firmware version changes (*if* there are changes in the partition
> descriptor - eg fields are added).
>
Indeed, this will break if the size of the descriptor changes in the future
and the kernel has not yet added the support for that though it is unlikely
as we negotiate the version and the response for all the messages should
be as per the negotiated version.
> An option would consist in just copying res->a3 bytes as firmware reports
> (obviously keeping the RX buffer boundary checks for the memcpy).
>
Yes I prefer that.
> It is just a heads-up because I noticed it, no more, I will let Sudeep
> comment on this since he knows better.
>
I think you had it all covered, nothing much to add.
--
Regards,
Sudeep
prev parent reply other threads:[~2024-04-11 9:05 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 15:19 [PATCH] KVM: arm64: Add support for FFA_PARTITION_INFO_GET Sebastian Ene
2024-04-09 16:15 ` Vincent Donnefort
2024-04-10 9:15 ` Sebastian Ene
2024-04-10 9:53 ` Vincent Donnefort
2024-04-10 10:18 ` Sebastian Ene
2024-04-10 17:10 ` Vincent Donnefort
2024-04-11 9:03 ` Sebastian Ene
2024-04-10 14:57 ` Lorenzo Pieralisi
2024-04-11 8:52 ` Sebastian Ene
2024-04-11 9:05 ` Sudeep Holla [this message]
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=ZhenzfndkNwIehhj@bogus \
--to=sudeep.holla@arm.com \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=jean-philippe@linaro.org \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=qperret@google.com \
--cc=qwandor@google.com \
--cc=sebastianene@google.com \
--cc=suzuki.poulose@arm.com \
--cc=tabba@google.com \
--cc=vdonnefort@google.com \
--cc=will@kernel.org \
--cc=yuzenghui@huawei.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).