2024-03-22 12:44:13

by Sebastian Ene

[permalink] [raw]
Subject: [PATCH 1/2] KVM: arm64: Fix the identification range for the FF-A smcs

The FF-A spec 1.2 reserves the following ranges for identifying FF-A
calls:
0x84000060-0x840000FF: FF-A 32-bit calls
0xC4000060-0xC40000FF: FF-A 64-bit calls.

Use the range identification according to the spec and allow calls that
are currently out of the range(eg. FFA_MSG_SEND_DIRECT_REQ2) to be
identified correctly.

Signed-off-by: Sebastian Ene <[email protected]>
---
arch/arm64/kvm/hyp/include/nvhe/ffa.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/include/nvhe/ffa.h b/arch/arm64/kvm/hyp/include/nvhe/ffa.h
index d9fd5e6c7d3c..146e0aebfa1c 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/ffa.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/ffa.h
@@ -9,7 +9,7 @@
#include <asm/kvm_host.h>

#define FFA_MIN_FUNC_NUM 0x60
-#define FFA_MAX_FUNC_NUM 0x7F
+#define FFA_MAX_FUNC_NUM 0xFF

int hyp_ffa_init(void *pages);
bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt, u32 func_id);
--
2.44.0.396.g6e790dbe36-goog



2024-03-22 12:44:27

by Sebastian Ene

[permalink] [raw]
Subject: [PATCH 2/2] KVM: arm64: Allow only the specified FF-A calls to be forwarded to TZ

The previous logic used a deny list to filter the FF-A calls. Because of
this, some of the calls escaped the check and they were forwarded by
default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64
bit version of the call was not).
Modify the logic to use an allowlist and allow only the calls specified in
the filter function to be proxied to TZ from the hypervisor.

Signed-off-by: Sebastian Ene <[email protected]>
---
arch/arm64/kvm/hyp/nvhe/ffa.c | 46 +++++++++++++++++------------------
1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 320f2eaa14a9..fc4f04209618 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -580,35 +580,35 @@ static void do_ffa_mem_reclaim(struct arm_smccc_res *res,
ffa_to_smccc_res(res, ret);
}

-/*
- * Is a given FFA function supported, either by forwarding on directly
- * or by handling at EL2?
- */
static bool ffa_call_supported(u64 func_id)
{
switch (func_id) {
- /* Unsupported memory management calls */
- case FFA_FN64_MEM_RETRIEVE_REQ:
- case FFA_MEM_RETRIEVE_RESP:
- case FFA_MEM_RELINQUISH:
- case FFA_MEM_OP_PAUSE:
- case FFA_MEM_OP_RESUME:
- case FFA_MEM_FRAG_RX:
- case FFA_FN64_MEM_DONATE:
- /* Indirect message passing via RX/TX buffers */
- case FFA_MSG_SEND:
- case FFA_MSG_POLL:
- case FFA_MSG_WAIT:
- /* 32-bit variants of 64-bit calls */
+ /* Discovery functions */
+ case FFA_FEATURES:
+ case FFA_ID_GET:
+ case FFA_VERSION:
+ case FFA_PARTITION_INFO_GET:
+
+ /* Memory management calls */
+ case FFA_FN64_RXTX_MAP:
+ case FFA_RXTX_UNMAP:
+ case FFA_MEM_SHARE:
+ case FFA_FN64_MEM_SHARE:
+ case FFA_MEM_RECLAIM:
+ case FFA_MEM_LEND:
+ case FFA_FN64_MEM_LEND:
+ case FFA_MEM_FRAG_TX:
+
+ /* State management */
+ case FFA_RUN:
+
+ /* Message passing */
case FFA_MSG_SEND_DIRECT_REQ:
- case FFA_MSG_SEND_DIRECT_RESP:
- case FFA_RXTX_MAP:
- case FFA_MEM_DONATE:
- case FFA_MEM_RETRIEVE_REQ:
- return false;
+ case FFA_FN64_MSG_SEND_DIRECT_REQ:
+ return true;
}

- return true;
+ return false;
}

static bool do_ffa_features(struct arm_smccc_res *res,
--
2.44.0.396.g6e790dbe36-goog


2024-03-23 02:08:11

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH 2/2] KVM: arm64: Allow only the specified FF-A calls to be forwarded to TZ

On Fri, Mar 22, 2024 at 12:43:03PM +0000, Sebastian Ene wrote:
> The previous logic used a deny list to filter the FF-A calls. Because of
> this, some of the calls escaped the check and they were forwarded by
> default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64
> bit version of the call was not).
> Modify the logic to use an allowlist and allow only the calls specified in
> the filter function to be proxied to TZ from the hypervisor.

I had discussed this with Will back when the feature was upstreamed and
he said there's a lot of off-label calls that necessitate a denylist
implementation. Has anything changed to give us confidence that we can
be restrictive, at least on the FF-A range?

--
Thanks,
Oliver

2024-03-25 14:54:46

by Sebastian Ene

[permalink] [raw]
Subject: Re: [PATCH 2/2] KVM: arm64: Allow only the specified FF-A calls to be forwarded to TZ

On Fri, Mar 22, 2024 at 07:07:52PM -0700, Oliver Upton wrote:
> On Fri, Mar 22, 2024 at 12:43:03PM +0000, Sebastian Ene wrote:
> > The previous logic used a deny list to filter the FF-A calls. Because of
> > this, some of the calls escaped the check and they were forwarded by
> > default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64
> > bit version of the call was not).
> > Modify the logic to use an allowlist and allow only the calls specified in
> > the filter function to be proxied to TZ from the hypervisor.

Hi Oliver,

>
> I had discussed this with Will back when the feature was upstreamed and
> he said there's a lot of off-label calls that necessitate a denylist
> implementation. Has anything changed to give us confidence that we can
> be restrictive, at least on the FF-A range?
>

I remember your proposal for having an allowlist instead. The current change makes
sense if we have https://lore.kernel.org/kvmarm/[email protected]/
which opens the window for more FF-A calls to be forwarded to TZ.

Let me know if this clarifies, thanks
Seb

> --
> Thanks,
> Oliver

2024-03-26 08:42:48

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH 2/2] KVM: arm64: Allow only the specified FF-A calls to be forwarded to TZ

On Mon, Mar 25, 2024 at 11:29:39AM +0000, Sebastian Ene wrote:
> On Fri, Mar 22, 2024 at 07:07:52PM -0700, Oliver Upton wrote:
> > On Fri, Mar 22, 2024 at 12:43:03PM +0000, Sebastian Ene wrote:
> > > The previous logic used a deny list to filter the FF-A calls. Because of
> > > this, some of the calls escaped the check and they were forwarded by
> > > default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64
> > > bit version of the call was not).
> > > Modify the logic to use an allowlist and allow only the calls specified in
> > > the filter function to be proxied to TZ from the hypervisor.
>
> Hi Oliver,
>
> >
> > I had discussed this with Will back when the feature was upstreamed and
> > he said there's a lot of off-label calls that necessitate a denylist
> > implementation. Has anything changed to give us confidence that we can
> > be restrictive, at least on the FF-A range?
> >
>
> I remember your proposal for having an allowlist instead. The current change makes
> sense if we have https://lore.kernel.org/kvmarm/[email protected]/
> which opens the window for more FF-A calls to be forwarded to TZ.

Got it. Last time I didn't catch the level of abuse we expect to endure
from vendors, but now it seems we will not support non-conforming calls
that appear in standardized SMC ranges.

Adding mention of this to the changelog might be a good idea then.

--
Thanks,
Oliver

2024-03-28 14:08:44

by Sebastian Ene

[permalink] [raw]
Subject: Re: [PATCH 2/2] KVM: arm64: Allow only the specified FF-A calls to be forwarded to TZ

On Tue, Mar 26, 2024 at 01:42:26AM -0700, Oliver Upton wrote:
> On Mon, Mar 25, 2024 at 11:29:39AM +0000, Sebastian Ene wrote:
> > On Fri, Mar 22, 2024 at 07:07:52PM -0700, Oliver Upton wrote:
> > > On Fri, Mar 22, 2024 at 12:43:03PM +0000, Sebastian Ene wrote:
> > > > The previous logic used a deny list to filter the FF-A calls. Because of
> > > > this, some of the calls escaped the check and they were forwarded by
> > > > default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64
> > > > bit version of the call was not).
> > > > Modify the logic to use an allowlist and allow only the calls specified in
> > > > the filter function to be proxied to TZ from the hypervisor.
> >
> > Hi Oliver,
> >
> > >
> > > I had discussed this with Will back when the feature was upstreamed and
> > > he said there's a lot of off-label calls that necessitate a denylist
> > > implementation. Has anything changed to give us confidence that we can
> > > be restrictive, at least on the FF-A range?
> > >
> >
> > I remember your proposal for having an allowlist instead. The current change makes
> > sense if we have https://lore.kernel.org/kvmarm/[email protected]/
> > which opens the window for more FF-A calls to be forwarded to TZ.
>
> Got it. Last time I didn't catch the level of abuse we expect to endure
> from vendors, but now it seems we will not support non-conforming calls
> that appear in standardized SMC ranges.
>
> Adding mention of this to the changelog might be a good idea then.
>

That's a good point. I didn't create a changelog for this but I should
add one and specify this.

> --
> Thanks,
> Oliver

Thanks,
Sebastian