From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) by mx.groups.io with SMTP id smtpd.web10.51261.1612269189071276978 for ; Tue, 02 Feb 2021 04:33:09 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=Bzzq8b3K; spf=pass (domain: linaro.org, ip: 209.85.221.52, mailfrom: ilias.apalodimas@linaro.org) Received: by mail-wr1-f52.google.com with SMTP id g10so20271478wrx.1 for ; Tue, 02 Feb 2021 04:33:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=/4UP2qmoTydeZ1ND5XZkTkJ6xP8DvhGIbc53RCD6//8=; b=Bzzq8b3KJZQHtEknGK4SzrPPNnRQ2jGE2EKlWqqG9HC3bLcjp6q7t9TmDScYehi1x/ EjtLcrJbXggDDRfIP62e0+y+KO0FgvQ5bFwpot1msDphBVLoU1+OtwlOtz7/CGouz6HS ZETvfSJ6EUjAhHtVzibQu2YiRrFhLU3ufHWaypt++hm0nIx+G4DUqItoPdltMypJhOlM eAVFRSc7Qb/utmnqLv+AGxIHu8aFgn/9p/Npuj5rWYrw5sN7Buu641mqPpmaHknUrbuW raQ4WhdnTR0+4XI3gRzSqUKy7saKGHbo/tYnMPH4dPqfOTnQUHKmoAvUjIkmkEKN2iKJ aRCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=/4UP2qmoTydeZ1ND5XZkTkJ6xP8DvhGIbc53RCD6//8=; b=TZ1p5NprHrnWc2nKzHX6kdse8g12DjDQb54x84dREambx9PC6IfxAekVWxzNrpoC9Z maj3xAFA/FHxEtO4lFd8UOHrmw6YB7ijrh3+jCS3m4EJ8gs2/lu9Y/wIqTpb3vqJNq2f CA7R/WJzE/EdumxzZvL+YiHMyWwWc0rCDfZcDVBLBEVX8hvWGrkrknncVGkfg5XR0MIs b7w2HbowxP+bwFSyyudz5/5pUFrx2F83HyElREK83Q0bt+2YFldIvIKsWTFucGNRySPz wSBYiGIF6evvpZiOC60N136mRT/EGUmpCPCi2K0ukPljkdzzqvOx3KdqV1bMu3q5UdQk SD8g== X-Gm-Message-State: AOAM532rtlXfbmSvAkFO7F7qWtdoENeFvvhvAP5fO6joSPAhPv5Oou9p uLhKPYF+/SL5+GD9lApxIIZvUA== X-Google-Smtp-Source: ABdhPJy1Wxso0MwUsc8QPJPsYAeMrD88R3MrZ1osbS+H9Rc4JYKVSBbrt9T72YiypNlKqbeuI9RjNw== X-Received: by 2002:adf:ce86:: with SMTP id r6mr24158893wrn.63.1612269187238; Tue, 02 Feb 2021 04:33:07 -0800 (PST) Return-Path: Received: from apalos.home (athedsl-376992.home.otenet.gr. [79.131.24.158]) by smtp.gmail.com with ESMTPSA id j15sm6633554wrw.9.2021.02.02.04.33.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Feb 2021 04:33:06 -0800 (PST) Date: Tue, 2 Feb 2021 14:33:04 +0200 From: "Ilias Apalodimas" To: Sami Mujawar Cc: Sughosh Ganu , "devel@edk2.groups.io" , Ard Biesheuvel , Leif Lindholm , Sahil Malhotra , nd Subject: Re: [PATCH edk2-platforms v3 1/2] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver Message-ID: References: <20201216110903.17995-1-sughosh.ganu@linaro.org> <20201216110903.17995-2-sughosh.ganu@linaro.org> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Sami, On Tue, Feb 02, 2021 at 10:40:32AM +0000, Sami Mujawar wrote: > Hi Ilias, > > Please see my response inline marked [SAMI]. > > Regards, > > Sami Mujawar > > -----Original Message----- > From: Ilias Apalodimas > Sent: 01 February 2021 02:01 PM > To: Sami Mujawar > Cc: Sughosh Ganu ; devel@edk2.groups.io; Ard Biesheuvel ; Leif Lindholm ; Sahil Malhotra > Subject: Re: [PATCH edk2-platforms v3 1/2] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver > > Hi Sami, > > > [...] > > > +STATIC > > > +EFI_STATUS > > > +ReadWriteRpmb ( > > > + UINTN SvcAct, > > > + UINTN Addr, > > > + UINTN NumBytes, > > > + UINTN Offset > > > + ) > > > +{ > > > + ARM_SVC_ARGS SvcArgs; > > > + EFI_STATUS Status; > > > + > > > + ZeroMem (&SvcArgs, sizeof (SvcArgs)); > > > + > > > + SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64; > > > + SvcArgs.Arg1 = storage_id; > > > + SvcArgs.Arg2 = 0; > > > + SvcArgs.Arg3 = SvcAct; > > > + SvcArgs.Arg4 = Addr; > > > + SvcArgs.Arg5 = NumBytes; > > > + SvcArgs.Arg6 = Offset; > > > + > > > + ArmCallSvc (&SvcArgs); > > > + if (SvcArgs.Arg3) { > > > + DEBUG ((DEBUG_ERROR, "%a: Svc Call 0x%08x Addr: 0x%08x len: 0x%x Offset: 0x%x failed with 0x%x\n", > > > + __func__, SvcAct, Addr, NumBytes, Offset, SvcArgs.Arg3)); > > > + } > > > + > > > + switch (SvcArgs.Arg3) { > > > + case ARM_SVC_SPM_RET_SUCCESS: > > > + Status = EFI_SUCCESS; > > > + break; > > > + > > > + case ARM_SVC_SPM_RET_NOT_SUPPORTED: > > > + Status = EFI_UNSUPPORTED; > > > + break; > > > + > > > + case ARM_SVC_SPM_RET_INVALID_PARAMS: > > > + Status = EFI_INVALID_PARAMETER; > > > + break; > > > + > > > + case ARM_SVC_SPM_RET_DENIED: > > > + Status = EFI_ACCESS_DENIED; > > > + break; > > > + > > > + case ARM_SVC_SPM_RET_NO_MEMORY: > > > + Status = EFI_BAD_BUFFER_SIZE; > > > + break; > > > + > > > + default: > > > + Status = EFI_ACCESS_DENIED; > > > + } > > > [SAMI] Should the error handling here be updated similar to the FF-A StandaloneMmPkg patches? > > > [/SAMI] > > > > I actually picked up the error handling from the previous non-FFA code. > > I'll check what's on Sughosh latest patches and fix it if there are > > any differences. > > Looking at it again EFI_BAD_BUFFER_SIZE can change to indicate out of > > memory properly anyway. > > > > Had another look at this. This seems fine if I just change > EFI_BAD_BUFFER_SIZE -> EFI OUT_OF_RESOURCES because OP-TEE is only > using these errors from FFA. Eventually the OP-TEE code that launches > StMM today, will move to FFA and become a separate SP, so that will > naturally be handled once that's done. I don't see a point of adding > unused error cases. > > [SAMI] Referring to the FFA specification, DEN0077A, v1.0, section 10.2 FFA_MSG_SEND_DIRECT_REQ and Table 10.8: FFA_ERROR encoding, I think the > error codes being handled above would be returned in SvcArgs.Arg2. Hmm why ? > The message flow would be as follows: > - Caller sends FFA_MSG_SEND_DIRECT_REQ to the target endpoint. > - if the message does not reach the target endpoint, an error code from Table 10.8 may be returned in w2 (i.e. SvcArgs.Arg2) That would be in the case you have a working TF-A implementation and the message is never dispatched to the endpoint right? The current driver is not implementing the whole range of that spec. The communication between secure/non secure world is still based on the OP-TEE messaging mechanism. The only part that complies to the FFA spec is the communication between the driver itself and OP-TEE. > - If the message reaches the target endpoint, then callee shall invoke one of the following interfaces: > * FFA_MSG_SEND_DIRECT_RESP So what's happening here, is that we send an SVC with ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64. The op-tee relevant code is located at ./core/arch/arm/kernel/stmm_sp.c There's 2 things we handle right now on OP-TEE: 1. set the page permissions, after relocating the executable. 2. Read/Write data on our RPMB. In both cases service_compose_direct_resp() is used to construct the response and that set the return value on x3. Regards /Ilias > * FFA_INTERRUPT > * FFA_SUCCESS > This would mean that if the callee responds with FFA_MSG_SEND_DIRECT_RESP, the callee returned error/status code shall be in w/x3-w/x7 (which I think in this case may be in SvcArgs.Arg3). > [/SAMI] > > Regards > /Ilias