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 X-Spam-Level: X-Spam-Status: No, score=-16.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CDCD5C433EF for ; Thu, 9 Sep 2021 20:09:19 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 4A0EC6113E for ; Thu, 9 Sep 2021 20:09:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4A0EC6113E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5150183534; Thu, 9 Sep 2021 22:09:17 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org 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=chromium.org header.i=@chromium.org header.b="AhUSppfZ"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id ADB1383533; Thu, 9 Sep 2021 22:09:14 +0200 (CEST) Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) (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 BF6AA8353E for ; Thu, 9 Sep 2021 22:09:05 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-wm1-x336.google.com with SMTP id g74so2192906wmg.5 for ; Thu, 09 Sep 2021 13:09:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=QHBQnkS7ZKXkf2u4TmIMAsSQc1AH1LqnQi9COWKwyS0=; b=AhUSppfZdGU9un3gcvtUib45a+6li/uK2uf/WW9vzhO5nQTEHCA4ESLK8H+svvkwrl riNPdf6HKG6ezgNEMb1XH+Fy3ckD4bX/6EBaOlosybiN/J7DKk3vwxdA3lY4CW9lJdOw fsdo3k92EI056XagHifVjqKxR/eOlxec+IR5M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=QHBQnkS7ZKXkf2u4TmIMAsSQc1AH1LqnQi9COWKwyS0=; b=YZucpie6LDuOvj8OBmcvi3SEIZUOW0axyOfkBoNyij8WSYTOmxh0c6e52eGE1yxaTi Iy8CdzTpFH+wmph4ww9NH/43sfWW+EOTDetRf5ATm/Wkd6ZiVUt8hFQiYF+5xIvL56j7 rRfZc+ChDAnK/nBZvM/7O5Des2GSxGphjxBIhmiPMHQ3Jnjk8wRmKRYRDzpVQSn/NF5c uKPL2ndQwn8YLXd5Yw1HlEr4nwV4nK5m/jnnykOrQ6d6yRMdog1x5MVkO8XTjeprXn3Y LXqG61cY0zl/y8/J7Czz4OOqitCxfQNL68PJZoVlQRCWYzdXB4ty70jpDFFvwtPRLyol gUYA== X-Gm-Message-State: AOAM5338b/i+p9WJeDOKPh939JHbuZcPrbgddADhINHzOg4ADO02OdcJ nEVtoAHJbq7aybGK0b8DorjJqrLavYQYuDdOXYUAfg== X-Google-Smtp-Source: ABdhPJyxY8kUaG5Ax2V25EjZS+y364xOMcQt3GjczKaSdU5pNFkq/J6F0OsROY49+eJhn/pFCUr7oZyTyEGokBnqOQw= X-Received: by 2002:a05:600c:3b0e:: with SMTP id m14mr4888612wms.118.1631218143437; Thu, 09 Sep 2021 13:09:03 -0700 (PDT) MIME-Version: 1.0 References: <20210908133405.696481-1-sjg@chromium.org> <20210908073355.3.I64a9f2e15201e8dba68a24e903cc1ac59237169e@changeid> <3050a6d1-60d6-6221-bf28-173fba6c48d9@gmx.de> In-Reply-To: <3050a6d1-60d6-6221-bf28-173fba6c48d9@gmx.de> From: Simon Glass Date: Thu, 9 Sep 2021 14:08:52 -0600 Message-ID: Subject: Re: [PATCH 03/35] x86: Show some EFI info with the bdinfo command To: Heinrich Schuchardt Cc: Ilias Apalodimas , Bin Meng , Tom Rini , Christian Melki , U-Boot Mailing List Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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.2 at phobos.denx.de X-Virus-Status: Clean Hi Heinrich, On Thu, 9 Sept 2021 at 03:34, Heinrich Schuchardt wrote: > > > > On 9/9/21 10:57 AM, Simon Glass wrote: > > Hi Heinrich, > > > > On Wed, 8 Sept 2021 at 11:34, Heinrich Schuchardt wrote: > >> > >> > >> > >> On 9/8/21 3:33 PM, Simon Glass wrote: > >>> It is useful to see some basic EFI info with the command as it forms part > >>> of the information about a board. > >>> > >>> Add a hook for this and show the table address as a start. > >>> > >>> While here, fix an invalid cast in setup_efi_info(). > >>> > >>> Signed-off-by: Simon Glass > >>> --- > >>> > >>> arch/x86/cpu/efi/payload.c | 13 +++++++++++-- > >>> arch/x86/include/asm/efi.h | 7 +++++++ > >>> arch/x86/lib/Makefile | 1 + > >>> arch/x86/lib/bdinfo.c | 22 ++++++++++++++++++++++ > >>> 4 files changed, 41 insertions(+), 2 deletions(-) > >>> create mode 100644 arch/x86/lib/bdinfo.c > >>> > >>> diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c > >>> index 9a73b768e9b..3a9f7d72868 100644 > >>> --- a/arch/x86/cpu/efi/payload.c > >>> +++ b/arch/x86/cpu/efi/payload.c > >>> @@ -280,15 +280,24 @@ void setup_efi_info(struct efi_info *efi_info) > >>> } > >>> efi_info->efi_memdesc_size = map->desc_size; > >>> efi_info->efi_memdesc_version = map->version; > >>> - efi_info->efi_memmap = (u32)(map->desc); > >>> + efi_info->efi_memmap = (ulong)(map->desc); > >> > >> When converting a pointer to integer we use (uintptr_t). > > > > Generally in U-Boot it is ulong so I try to be consistent here. > > Currently ulong and uintprt_t are the same technically. But for the sake > of readability uintptr_t is much clearer. > > We use uintptr_t in 935 code locations already. Just grep for it. $ git grep uintptr_t |wc -l 935 $ git grep ulong |wc -l 6006 > > > Anyway, u32 is clearly not right for a 64-bit build as it gives > > warnings. > > > >> > >> arch/x86/include/asm/bootparam.h defines efi_info->efi_memmap as u32. > >> This type is too small to hold a pointer. > >> > >>> efi_info->efi_memmap_size = size - sizeof(struct efi_entry_memmap); > >>> > >>> #ifdef CONFIG_EFI_STUB_64BIT > >>> efi_info->efi_systab_hi = table->sys_table >> 32; > >>> - efi_info->efi_memmap_hi = (u64)(u32)(map->desc) >> 32; > >>> + efi_info->efi_memmap_hi = (u64)(ulong)map->desc >> 32; > >> > >> You should not use two fields to hold a single pointer. > >> > >> Please, replace all of these duplicate fields. > > > > We do need to be compatible with what the kernel expects, otherwise > > there is no point in filling out this information. > > Why should we have separate fields for the low and high 32 bits of a > pointer? This patch does not change that...you need to take it up with the Linux project, which created this requirement. We should not rely on endianness being the same between a u64 and two u32 values, if that is what you are asking. But in any case, why do you insist on critiquing code that is already there? By all means send a patch if you want to change it. But please just review what this patch does. Regards, Simon