From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web11.8034.1623852633218031817 for ; Wed, 16 Jun 2021 07:10:33 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=uLcTpE2F; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id AA9CD6115B for ; Wed, 16 Jun 2021 14:10:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1623852632; bh=z2iBhFRkexynbJjQ0frn3q4KBkYaYN7fHB4yvE5s69k=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=uLcTpE2F5L4UJLrqJx7dBiI5GHjsKTxHYIaxcsnni+TSU5j0zPJrMibq1Mg+1Qstv iaAyZijTudKb4uaTVn9eD3jrQNG66bnZcQAetEflEzoHtb+k6Q2prYI2wBYG7HE8sn PbKuyCMulGMDlttFoPipsFCluuutlHWDBtFjtsTB8vi+y/yQhEiGegoWOBuI6ivmZW KlqdbWWrEqOzzW3RgdQoP+5JI8uxli1ajQnVIKxV38ZVNXJXiYGxiCDq6pSII3ACoD zraBvpZtZq/mpOuUsE/fag5f69o0n3/nk3vlkJB9nNSXi6pCU771yxwlCAATsEwNKF b6x2R1Yeyaj2A== Received: by mail-ot1-f51.google.com with SMTP id b18-20020a0568301052b0290449ba7eff3cso785521otp.7 for ; Wed, 16 Jun 2021 07:10:32 -0700 (PDT) X-Gm-Message-State: AOAM530dsXv3sYklYoINwkhHAJQQA9hkQ9vr5CbVMVG/qh4uxM/hzPz5 cZ2hCw1TstEaMXLkD0pTvIaivzQDbwx8Pb2qiL4= X-Google-Smtp-Source: ABdhPJzRCpPpaD30vfHtK3oXo+sVzv45s1EM0TPWSZUxtHg9s+K+Go5RjMCxJZIOlZLJ1HjDiMB2ZMF9jv3LuFpavsk= X-Received: by 2002:a05:6830:1d63:: with SMTP id l3mr100308oti.108.1623852632043; Wed, 16 Jun 2021 07:10:32 -0700 (PDT) MIME-Version: 1.0 References: <20210608142112.87183-1-huangming@linux.alibaba.com> <7536fafd-fa8e-9940-beec-a1cd357ecb03@linux.alibaba.com> In-Reply-To: From: "Ard Biesheuvel" Date: Wed, 16 Jun 2021 16:10:20 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [Patch] StandaloneMmPkg: Fixed communicating from TF-A failed issue To: Omkar Kulkarni Cc: "devel@edk2.groups.io" , "huangming@linux.alibaba.com" , Ard Biesheuvel , Sami Mujawar , Jiewen Yao , Supreeth Venkatesh , "guoheyi@linux.alibaba.com" , nd Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 16 Jun 2021 at 07:30, Omkar Kulkarni wrote= : > > > On 6/10/21 6:44 AM, Ming Huang via groups.io wrote: > > On 6/9/21 3:10 PM, Ard Biesheuvel wrote: > > > On Tue, 8 Jun 2021 at 16:21, Ming Huang > > wrote: > > >> > > >> TF-A: TrustedFirmware-a > > >> SPM: Secure Partition Manager(MM) > > >> > > >> For AArch64, when SPM enable in TF-A, TF-A may communicate to MM > > with > > >> buffer address (PLAT_SPM_BUF_BASE). The address is different from > > >> PcdMmBufferBase which use in edk2. > > > > > > Then why do we have PcdMmBufferBase? > > > > ArmPkg use this Pcd for the base address of non-secure communication > > buffer. > > > > > > > > Is it possible to set PcdMmBufferBase to the correct value? > > > > The secure communication may interrupt the non-secure communication. i= f > > we use the same address (PcdMmBufferBase and PLAT_SPM_BUF_BASE), the > > date in communication buffer may be corrupted. > > > > Best Regards, > > Ming > > In case where an interrupt handler executing from EL3 makes a call into = StandaloneMM, the handler in EL3 makes an spm call into StandaloneMM using = PLAT_SPM_BUF_BASE buffer base address. This PLAT_SPM_BUF_BASE is a shared b= uffer between EL3 and S-EL0. This is where the following check fails and le= ads to spm call failure. So this change would help resolve this issue. > But is it the right fix? Why would EDK2 even be aware of how EL3 and S-EL0 communicate with each other, and where the buffer is located? > > > > > > > >> Checking address will let TF-A communicate failed to MM. So remove > > >> below checking code: > > >> if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) { > > >> return EFI_ACCESS_DENIED; > > >> } > > >> > > >> Signed-off-by: Ming Huang > > >> --- > > >> StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c | > > 4 > > >> ---- > > >> 1 file changed, 4 deletions(-) > > >> > > >> diff --git > > >> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c > > >> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c > > >> index 63fbe26642..fe98d3181d 100644 > > >> --- > > a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c > > >> +++ > > b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c > > >> @@ -103,10 +103,6 @@ PiMmStandaloneArmTfCpuDriverEntry ( > > >> return EFI_INVALID_PARAMETER; > > >> } > > >> > > >> - if (NsCommBufferAddr < mNsCommBuffer.PhysicalStart) { > > >> - return EFI_ACCESS_DENIED; > > >> - } > > >> - > > >> if ((NsCommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=3D > > >> (mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize)) = { > > >> return EFI_INVALID_PARAMETER; > > >> -- > > >> 2.17.1 > > >> > > > > > >=20 > > >