From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x236.google.com (mail-it0-x236.google.com [IPv6:2607:f8b0:4001:c0b::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DDB371A1EFC for ; Thu, 15 Sep 2016 07:04:17 -0700 (PDT) Received: by mail-it0-x236.google.com with SMTP id 186so76253084itf.0 for ; Thu, 15 Sep 2016 07:04:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=KQIMZwSNs2zP1OaQfConoxuZmrkJi8I3YG49ik7CDxE=; b=jKC/Vwo59teuEXDreYqZPzxVnrBEBvN3N9ZWVsWkQqMKHxCPSIOxk7oPqx7TuayTF+ gf6hh+t6aYHB7wb+oIJkX8QrNSVdyEDRi4Ny6UocbVhV0/uQ4k0uO1I6yPfv1I7lxx98 HSOtkfV1ZcRHLNV/gUaTCthKJMsW7DmrqqQnQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=KQIMZwSNs2zP1OaQfConoxuZmrkJi8I3YG49ik7CDxE=; b=hWBo67itQN4K0dnnCKRLPDNGxk67y5DaPgR6vgQOKX2AIHQXmjVTPx92wLfBnMH25o jMXcB0HFfj1UYE9yeefu78YUhbavYcB0pkkhIzSP+kSbN8PtwbRez9qt7MsfNr9xK1FR Ac2UK5vxpo6w6q5l+G1HCye7q8Fq05BpkU3u/Jri65nZfaTiU745rs6hbAZfx4gR7BZr NWnycH7lRpFGbGkOYZxUOf/djsg/waWSis/0CneFWnIeDGesXspEfRxZhdMgCck8S5gR mCO49m8oZr0zyY9T3yerTxSidfqOkQ9zDI+mskYqY/Gje7AHQRa9d/cdYWAHV0Jm1hlz ILDQ== X-Gm-Message-State: AE9vXwOm0yK9Cv+dtNgpQNJcQBhswwrp2/AWanDAdXNIUC7Z5C/PCVzHJIqHe1RbozwtzvJWwpG90t9Dr7ih5KtS X-Received: by 10.107.35.209 with SMTP id j200mr16792264ioj.26.1473948256819; Thu, 15 Sep 2016 07:04:16 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Thu, 15 Sep 2016 07:04:16 -0700 (PDT) In-Reply-To: References: <1473946233-10547-1-git-send-email-ard.biesheuvel@linaro.org> <1473946233-10547-4-git-send-email-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Thu, 15 Sep 2016 15:04:16 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Shannon Zhao , Sakar Arora Subject: Re: [PATCH 3/4] ArmVirtPkg/FdtClient: add methods to iterate over memory nodes X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Sep 2016 14:04:18 -0000 Content-Type: text/plain; charset=UTF-8 On 15 September 2016 at 14:57, Laszlo Ersek wrote: > On 09/15/16 15:30, Ard Biesheuvel wrote: >> Add high level methods to iterate over all 'reg' properties of all DT >> nodes whose device_type properties have the value "memory". Since we are >> modifying the FdtClient protocol, update the protocol and the only existing >> implementation at the same time. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel >> --- >> ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 76 ++++++++++++++++++++ >> ArmVirtPkg/Include/Protocol/FdtClient.h | 26 +++++++ >> 2 files changed, 102 insertions(+) >> >> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c >> index 382e9af6bf84..099cce7821d6 100644 >> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c >> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c >> @@ -194,6 +194,80 @@ FindCompatibleNodeReg ( >> >> STATIC >> EFI_STATUS >> +EFIAPI >> +FindNextMemoryNodeReg ( >> + IN FDT_CLIENT_PROTOCOL *This, >> + IN INT32 PrevNode, >> + OUT INT32 *Node, >> + OUT CONST VOID **Reg, >> + OUT UINTN *AddressCells, >> + OUT UINTN *SizeCells, >> + OUT UINT32 *RegSize >> + ) >> +{ >> + INT32 Prev, Next; >> + CONST CHAR8 *DeviceType; >> + INT32 Len; >> + EFI_STATUS Status; >> + >> + ASSERT (mDeviceTreeBase != NULL); >> + ASSERT (Node != NULL); >> + >> + for (Prev = PrevNode;; Prev = Next) { >> + Next = fdt_next_node (mDeviceTreeBase, Prev, NULL); >> + if (Next < 0) { >> + break; >> + } >> + >> + DeviceType = fdt_getprop (mDeviceTreeBase, Next, "device_type", &Len); >> + if (DeviceType != NULL && AsciiStrCmp (DeviceType, "memory") == 0) { > > HighMemDxe uses AsciiStrnCmp (Type, "memory", Len) here. > > If we're sure that we're not looking "memory*" types here, then the > change is fine. Are we sure? > Actually, that is a bug. AsciiStrCmp () is guaranteed to check the NUL terminator, and so it will only match on "memory\0". Using AsciiStrnCmp() with the length of the property value is guaranteed *not* to check the NUL terminator, so it may match on prefixes of "memory\0" >> + > > The empty line is likely unintended. > indeed. >> + // >> + // Get the 'reg' property of this memory node. For now, we will assume >> + // 8 byte quantities for base and size, respectively. >> + // TODO use #cells root properties instead >> + // >> + Status = GetNodeProperty (This, Next, "reg", Reg, RegSize); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((EFI_D_WARN, >> + "%a: ignoring memory node with no 'reg' property\n", >> + __FUNCTION__)); >> + continue; >> + } >> + if ((*RegSize % 16) != 0) { >> + DEBUG ((EFI_D_WARN, >> + "%a: ignoring memory node with invalid 'reg' property (size == 0x%x)\n", >> + __FUNCTION__, RegSize)); > > This should be *RegSize. > OK > With the above confirmed / fixed: > > Reviewed-by: Laszlo Ersek