From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-io1-f54.google.com (mail-io1-f54.google.com [209.85.166.54]) by mx.groups.io with SMTP id smtpd.web09.7752.1614337115461245483 for ; Fri, 26 Feb 2021 02:58:35 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=gpPg1SnV; spf=pass (domain: linaro.org, ip: 209.85.166.54, mailfrom: sughosh.ganu@linaro.org) Received: by mail-io1-f54.google.com with SMTP id f20so9180101ioo.10 for ; Fri, 26 Feb 2021 02:58:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+RV4Fk9u8kL4XBJteuvEiMk/QNLBCcdTCAMpS0TQ48k=; b=gpPg1SnVh84TE1a/MVRFiSYMzYOFHFNuSlyZNWXbbCjqpMT7eV0aQKqUZ3O+T4c95r ICLqJmQI7KInqvAn+qRKI3gmJzWLXpsdbJsZ7eo8M8kkCIRT7KB48lhvi17c8a+JcHpj L5wAjyELOE1aOLnUqOmR1fYo5ekqTZo72d9OKOVxXJ+Tl5Q8mFqs6wSAuPCAbhobigs7 IOaghAj92J1x48WeruKSCU0idZfqQ8fDL9o0rJyKtWn3yHGY3dYl4++gE4jdDZ4lWzn9 AsHZxvtXlTSF09ZeHfcbfdu6Ty39+bTTCuzywhbF/fRb2UCMkyDBO15qO7iB53kTePII 26DA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+RV4Fk9u8kL4XBJteuvEiMk/QNLBCcdTCAMpS0TQ48k=; b=WWHY2McbFDwRnpP7pqeKzk1Nb1KoecRdiYaQqX7PeXKD3mlKXCeYxEmPvAwpK9z37I Dcg4fHJGd7C9pMrVoF5aGZThzYwdBojOcaF6FJGgVcRwiB+ZCHRMz1tKYZLRdwgG7v1z Gm5IKCha1S7CWp7uwLf+1qGZXPSUZHm0NsrdRQEaFD3P8ZwbzpNFka+ZRFXFaAT31p6m fisp0IwDgydVICLy5sTKPBi8v3yRxW4y5iiT/OC1RhQ/DWwGq73GTdPBJGVwCLB1QsHY 5b6YmsYvz63akPYASn4zx51773iG7eq0zvl6/yPPhP/Ul4f+G/+/Ja7nAsrQM4OljKCP iIWw== X-Gm-Message-State: AOAM531rpRckwRlOOUCc4GRLN2nDmqwU1rbxRMuzLykN7utLx7zvdP4i yEEuWmQOE/HCB0FTC/JKjf92EI7aK6KuC3cwTWhQrh3MwbE= X-Google-Smtp-Source: ABdhPJzgDwOL6EtOCRuv5xrorzW9mwU9VeVQBl1HirbeQS+eKy39Q6jNfPmQq0JZr122r59YpaXM/2wM6Uu1h7czbt4= X-Received: by 2002:a05:6638:eca:: with SMTP id q10mr2328488jas.116.1614337114608; Fri, 26 Feb 2021 02:58:34 -0800 (PST) MIME-Version: 1.0 References: <20210225171110.41324-1-sami.mujawar@arm.com> In-Reply-To: <20210225171110.41324-1-sami.mujawar@arm.com> From: "Sughosh Ganu" Date: Fri, 26 Feb 2021 16:28:23 +0530 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 1/1] ArmPkg: Fix uninitialised variable in ArmMmuStandaloneMmLib To: devel@edk2.groups.io, Sami Mujawar Cc: Ard Biesheuvel , Leif Lindholm , Matteo.Carlini@arm.com, Ben.Adderson@arm.com, nd Content-Type: multipart/alternative; boundary="00000000000043af3305bc3b26a8" --00000000000043af3305bc3b26a8 Content-Type: text/plain; charset="UTF-8" On Thu, 25 Feb 2021 at 22:41, Sami Mujawar wrote: > The following patches added support for StandaloneMM using FF-A: > 9da5ee116a28 ArmPkg: Allow FF-A calls to set memory region's attributes > 0e43e02b9bd8 ArmPkg: Allow FF-A calls to get memory region's attributes > > However, in the error handling logic for the Get/Set Memory attributes, > the CLANG compiler reports that a status variable could be used without > initialisation. This issue is a false positive and is not seen with GCC. > > The Get/Set Memory attributes operation is atomic and therefore an > FFA_INTERRUPT or FFA_SUCCESS response is not expected in response > to FFA_MSG_SEND_DIRECT_REQ. So the remaining cases that could occur > are: > - the target sends FFA_MSG_SEND_DIRECT_RESP with a success or > failure code. > or > - FFA_MSG_SEND_DIRECT_REQ transmission failure. > > Therefore, > - reorder the error handling conditions such that it prevents the > uninitialised variable issue being flagged by CLANG. > - move the repetitive code to a static helper function and add > documentation at the appropriate places. > - fix error handling in functions that invoke GetMemoryPermissions(). > > Signed-off-by: Sami Mujawar > --- > The changes can be seen at: > https://github.com/samimujawar/edk2/tree/1657_stmm_ffa_fix_unused_var_v2 Tested the changes on the StandaloneMm image on the Qemu platform. Tested-by: Sughosh Ganu Reviewed-by: Sughosh Ganu -sughosh --00000000000043af3305bc3b26a8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

On Thu, 25 Feb 2021 at 22:41, Sami Mujawa= r <sami.mujawar@arm.com> = wrote:
The follo= wing patches added support for StandaloneMM using FF-A:
9da5ee116a28 ArmPkg: Allow FF-A calls to set memory region's attributes=
0e43e02b9bd8 ArmPkg: Allow FF-A calls to get memory region's attributes=

However, in the error handling logic for the Get/Set Memory attributes,
the CLANG compiler reports that a status variable could be used without
initialisation. This issue is a false positive and is not seen with GCC.
The Get/Set Memory attributes operation is atomic and therefore an
FFA_INTERRUPT or FFA_SUCCESS response is not expected in response
to FFA_MSG_SEND_DIRECT_REQ. So the remaining cases that could occur
are:
=C2=A0- the target sends FFA_MSG_SEND_DIRECT_RESP with a success or
=C2=A0 =C2=A0failure code.
=C2=A0or
=C2=A0- FFA_MSG_SEND_DIRECT_REQ transmission failure.

Therefore,
=C2=A0- reorder the error handling conditions such that it prevents the
=C2=A0 =C2=A0uninitialised variable issue being flagged by CLANG.
=C2=A0- move the repetitive code to a static helper function and add
=C2=A0 =C2=A0documentation at the appropriate places.
=C2=A0- fix error handling in functions that invoke GetMemoryPermissions().=

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at:
https://github.com/samimujaw= ar/edk2/tree/1657_stmm_ffa_fix_unused_var_v2

=C2=A0Tested the changes on the StandaloneMm image on the Qemu platfo= rm.

Tested-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed= -by: Sughosh Ganu <sughosh.ga= nu@linaro.org>

-sughosh
--00000000000043af3305bc3b26a8--