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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5C029C0015E for ; Tue, 1 Aug 2023 16:19:26 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 40CEB86C9D; Tue, 1 Aug 2023 18:19:24 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="s/Zi1gN3"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C7AF586CA0; Tue, 1 Aug 2023 18:19:23 +0200 (CEST) Received: from mail-oi1-x234.google.com (mail-oi1-x234.google.com [IPv6:2607:f8b0:4864:20::234]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 34B7F86C92 for ; Tue, 1 Aug 2023 18:19:21 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-oi1-x234.google.com with SMTP id 5614622812f47-3a5abb5e2aeso4573049b6e.0 for ; Tue, 01 Aug 2023 09:19:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; t=1690906760; x=1691511560; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=8dmIJaU150KeY184yDwGCtvZmPcwVUa+WYwV7F6AO5Q=; b=s/Zi1gN3uyy5i4N3qNlmvYnOXCKZChuTsQSOGqbEiNr30+lFGODaFdxJZ5gQPZkoi0 5sFOqc2GBLCwMB4Uh8SZNZbthb0VliDVqhtPrVPTeTC0I1pRkVcnknifLhl7m4ZMGYAe COQ/hhT/qu7ih80aTEFwojVd60V8lHuAJTe6c= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690906760; x=1691511560; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=8dmIJaU150KeY184yDwGCtvZmPcwVUa+WYwV7F6AO5Q=; b=B0NfTzlKvaRKxNVzRZiN+/D0OV43Ro5HL7c+v4o1x6J2KeDN4YOYtNm6yahEHY7YNG jowp45yY6X6r9IM4htod7zec+ljCqcuqek6DW376oU2zmvtVNDX2lOWTjuI7+s1PHPhe LRA9ptfsqhM+1TWVPrA/wzEH/GTZZj7yly1kzWrtJ6lC11qTWvOsvOvBDxbZU/rSrWhv 4MKwI7ZfOJRvmpOP6kX7gAUzPJAwIaKw6JVpdANzK2UxtJ/vxq57LMZq8VSjNa+hcP65 YL6BZA2ztf9xwv5yoxzBkxQZbDO6Sq3bT6FOj7cZz+LKPLXrl9TG1EcJHvkaZYRk2JQJ XPcw== X-Gm-Message-State: ABy/qLYs383sacfTO1XGhav/LnfhxB5ffyuIy+Q0Z6d5o2DisSdIEtJQ /psVX2hXbMGEro4L+AAL104L3w== X-Google-Smtp-Source: APBJJlHP12+i3zWJCxfvna40DMA/dxOo3MSfwwE/qX0ypvpcrnyE+kbAXEnBWuMQqkm8auFsJdPSXQ== X-Received: by 2002:a05:6808:8f5:b0:3a7:4dd0:d4db with SMTP id d21-20020a05680808f500b003a74dd0d4dbmr1771562oic.11.1690906759745; Tue, 01 Aug 2023 09:19:19 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b00-6400-383c-c9ce-d23d-78ae.res6.spectrum.com. [2603:6081:7b00:6400:383c:c9ce:d23d:78ae]) by smtp.gmail.com with ESMTPSA id x3-20020a814a03000000b0057d24f8278bsm3788531ywa.104.2023.08.01.09.19.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Aug 2023 09:19:19 -0700 (PDT) Date: Tue, 1 Aug 2023 12:19:17 -0400 From: Tom Rini To: Abdellatif El Khlifi Cc: ilias.apalodimas@linaro.org, sjg@chromium.org, achin.gupta@arm.com, nd@arm.com, u-boot@lists.denx.de, jens.wiklander@linaro.org Subject: Re: [PATCH v17 09/10] arm_ffa: efi: introduce FF-A MM communication Message-ID: <20230801161917.GU3630934@bill-the-cat> References: <20230727160712.81477-1-abdellatif.elkhlifi@arm.com> <20230727160712.81477-10-abdellatif.elkhlifi@arm.com> <20230727164345.GH3630934@bill-the-cat> <20230728135415.GU3630934@bill-the-cat> <20230731114628.GA112180@e130802.arm.com> <20230801150057.GO3630934@bill-the-cat> <20230801161008.GA45125@e130802.arm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="PUNmVCgDaDudSupi" Content-Disposition: inline In-Reply-To: <20230801161008.GA45125@e130802.arm.com> X-Clacks-Overhead: GNU Terry Pratchett X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean --PUNmVCgDaDudSupi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 01, 2023 at 05:10:08PM +0100, Abdellatif El Khlifi wrote: > Hi guys, >=20 > On Tue, Aug 01, 2023 at 11:00:57AM -0400, Tom Rini wrote: > > > > > > > > > ... > > > > > > > > > Changelog: > > > > > > > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > > > > > > > > > > > > > > > v17: > > > > > > > > > > > > > > > > > > * show a debug message rather than an error when FF-A is = not detected > > > > > > > > [snip] > > > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kcon= fig > > > > > > > > > index c5835e6ef6..8fbadb9201 100644 > > > > > > > > > --- a/lib/efi_loader/Kconfig > > > > > > > > > +++ b/lib/efi_loader/Kconfig > > > > > > > > > @@ -55,13 +55,53 @@ config EFI_VARIABLE_FILE_STORE > > > > > > > > > stored as file /ubootefi.var on the EFI system pa= rtition. > > > > > > > > > > > > > > > > > > config EFI_MM_COMM_TEE > > > > > > > > > - bool "UEFI variables storage service via OP-TEE" > > > > > > > > > - depends on OPTEE > > > > > > > > > + bool "UEFI variables storage service via the truste= d world" > > > > > > > > > + depends on OPTEE && ARM_FFA_TRANSPORT > > > > > > > > > > > > > > > > You didn't get my changes in here however. If you can do EF= I_MM_COMM_TEE > > > > > > > > without ARM_FFA_TRANSPORT (as lx2160ardb_tfa_stmm_defconfig= does) then > > > > > > > > you don't make this option depend on . If FF-A is only > > > > > > > > for use here, you make FF-A depend on this, and the FF-A sp= ecific > > > > > > > > variable depend on ARM_FFA_TRANSPORT. > > > > > > > > > > > > > > Abdellatif hinted at what's going on here. When I added this= Kconfig > > > > > > > option to lx2160 FF-A wasn't implemented yet. > > > > > > > > > > > > The defconfig has existed since May 2020, which is when you add= ed > > > > > > EFI_MM_COMM_TEE itself too. So I think it's that no one did th= e check I > > > > > > did until now and saw this series was disabling what was on the= other > > > > > > platform. > > > > > > > > > > > > > Since FF-A isn't a new > > > > > > > communication mechanism but builds upon the existing SMCs to = build an > > > > > > > easier API, I asked Abdellatif to hide this complexity. > > > > > > > We had two options, either make Kconfig options for either FF= -A or the > > > > > > > traditional SMCs and remove the dependencies, or piggyback o= n FF-As > > > > > > > discovery mechanism and make the choice at runtime. The latt= er has a > > > > > > > small impact on code size, but imho makes developers' life a = lot > > > > > > > easier. > > > > > > > > > > > > I'm not sure how much you can do a run-time option here since y= ou're > > > > > > setting a bunch of default values for FF-A to 0 in Kconfig. If= we're > > > > > > supposed to be able to get them at run time, we shouldn't need = a Kconfig > > > > > > option at all. I'm also not sure how valid a use case it is wh= ere we > > > > > > won't know at build time what the rest of the firmware stack su= pports > > > > > > here. > > > > > > > > > > > > > > > > That's a fair point. FF-A in theory has APIs to discover memory. > > > > > Abdellatif, why do we need the Kconfigs for shared memory on FF-A? > > > > > > > > The statically carved out MM shared buffer address, size and offset= cannot be discovered by FF-A ABIs. > > > > The MM communication driver in U-Boot could allocate the buffer and= share it with the MM SP but > > > > we do not implement that support currently in either U-Boot or UEFI. > > >=20 > > > Ok, that's a bit unfortunate, but Tom is right. Having the FF-A > > > addresses show up is as confusing as having Kconfig options for > > > discrete options. The whole point of my suggestion was to make users' > > > lives easier. Apologies for the confusion but can you bring back the > > > ifdefs? Looking at the patch this should be minimal just use > > > ifdef ARM_FFA_TRANSPORT and ifndef ARM_FFA_TRANSPORT. > > >=20 > > > Tom you prefer that as well? > >=20 > > Pending an answer to Jens' feedback, yes, going back to #ifdef's is > > fine, especially since default values of 0 are nonsense in this case > > (and as Heinrich's patch re SYS_MALLOC_LEN shows, dangerous since 0 !=3D > > 0x0 once we do string comparisons). > >=20 >=20 > I'd like to give some context why it's important for Corstone-1000 platfo= rm > that the DT passed to the kernel matches the official kernel DT. Note that we've set aside the "should this be in DT or not" question. > There is a SystemReady IR 2.0 test checking the DT. It compares the DT > passed by U-Boot with a reference DT (the kernel DT) . The test fails if = there > is a mismatch. So, if we add a DT node in U-Boot and the node is not upst= reamed > to the kernel DT, the DT test will fail. This is overall good and progress. > To be approved by the kernel DT maintainers, the node should have a use c= ase > in the kernel which is not the case. This is, I believe / hope wrong. It needs to be in the dt-schema repository, not strictly "the kernel". For example, bootph-all (etc) are in dt-schema and so can be in the upstream kernel but are not used in the kernel itself. > There is a solution for this which is deleting the node we don't want to = pass to > the kernel using delete-node in the U-Boot DT. Something like this will likely be needed, in the end, at least for some cases. But the goal is that everything gets in to dt-schema. [snip] > With this we can get rid of the configs and the #defines: FFA_SHARED_MM_B= UF_ADDR, > FFA_SHARED_MM_BUF_OFFSET and FFA_SHARED_MM_BUF_SIZE. >=20 > Also, we will avoid setting 0 as default values for the address, size and= offset. We just need to not have default values offered. The symbols just need to depend on FFA so that they aren't asked when not used. > 2/ the FF-A specific code in efi_variable_tee.c will try to find the mm-c= omms-buf > reserved memory node. When found, it reads the buffer address, size and o= ffset. >=20 > 3/ adding #ifdef CONFIG_ARM_FFA_TRANSPORT in lib/efi_loader/efi_variable_= tee.c > for the FF-A specific code. >=20 > 4/ make EFI_MM_COMM_TEE depends on OPTEE only >=20 > What do you think guys ? Yes, we need to do 3 and 4. --=20 Tom --PUNmVCgDaDudSupi Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmTJMIIACgkQFHw5/5Y0 tyyJNwwAjLCe5nNN+qFzzhSKnOV2thZtPZXk4YAALOhhUiQnddCEexo+j3gaA2bY 2OrkX/gWpbC15tV0CRtoZRxGwsTwCaj/vqk8DEl6YxbsIzCfZG48BgjF4lnemP1o sZeS8WH1VV34XBvypB2zhwpH+F3AM1uZQZoPlqIGp5qIqaaME93/Tc7p9/Y+ygRf 2oY/0j7mj4gttfeTnt0gm1kIdK3MA0Lek9SXln9yyT26UTfbXuJtfurPVmdydGGC jvcyatT7gvH0V2lWzV/ouzXB5DOw6pAugpfmndC4/SE8WH3matJCgA0K0Z5lVgoJ EFMR5bHwdrmNv96dPMZDBYtoOh5kv+19/BQmZCHU8Gf4kbg5qzVR+WamhWZZiM5M GzWNZZkL+iTjxpONjLEfjHARjn4EP71LWEmtXFWZpwGxIlpAyW4WbRX+cwTpf/ak VWstOqleCFeugUx316imAuccViC2Jgn9CqDMTlgm9oveSLdxALeaQF+XiBFWDTRO c44pfYl/ =rCyI -----END PGP SIGNATURE----- --PUNmVCgDaDudSupi--