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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id BC684C433F5 for ; Fri, 28 Jan 2022 05:08:54 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7C216834E4; Fri, 28 Jan 2022 06:08:52 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=ventanamicro.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=ventanamicro.com header.i=@ventanamicro.com header.b="Ll3ALyhJ"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 46E67835F4; Fri, 28 Jan 2022 06:08:51 +0100 (CET) Received: from mail-pf1-x432.google.com (mail-pf1-x432.google.com [IPv6:2607:f8b0:4864:20::432]) (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 F410A8343B for ; Fri, 28 Jan 2022 06:08:47 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=ventanamicro.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sunilvl@ventanamicro.com Received: by mail-pf1-x432.google.com with SMTP id n32so5077348pfv.11 for ; Thu, 27 Jan 2022 21:08:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=8I0vM1dGV9qmNiUd/ImGCx8jf4TOm/eeTHuAWcSH9Xk=; b=Ll3ALyhJ/5meO4EFlItQlEfJfsALOGtixxRr5VQ3Ys04fZF6FkOYtUALnKNXkav2La uCZYpYi2QbFa7hx8zXg3q8t+hBbsU2MLQXY3GNGdPhWu8cnW/n+UxV6KH0gbt39wKJQC E5/LgvpLiUY+b1Y74IEulmzaEFAPZgKMxWNI3u0HUKNZLEvNzroHyU5g9u85RJaw4ZWU uDxBL+8pGwKG+piXCD8FsuWIgr56oBPZRfKALIqUjmwgKU0bXTwW45yXFq05tXdyRCwY 63EUBfB0uFevyjLvchZDD85rUbhPX4C63jNVh8OqgTyrrt5aODp9JqPNfbyNgYz2Oaki dfMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=8I0vM1dGV9qmNiUd/ImGCx8jf4TOm/eeTHuAWcSH9Xk=; b=matXRGhnsEwmRFoubcGz6QkY7R6wIEagtEA/qLzNJ30N2iQuLfWShkUePhvjFB8ulV ir0FGDpvxMP+6LkPTrPKdg15GvigzMCG+Dn2hKl9blsffEQuixvL6DUJ8hlKcV1tP1XQ pM9lybpR20a0Lh6nh5Y/J3n8gc/u0Or5KnI4cUuvUSNIhGD1Tkv2vjrSqlzq5Zd7HaKr 9EylZ9MKZCQYKTuH8Atc1P6/CfUGJ5X/o6w1OpyfBSafyE8y4YwnfDY7PYhF6SeMAQZ8 kl+TAaB9qnCXlBmiDjh87lxn+hNAGKVNKy6jelIMlPFJVlNsTU32izvJ17QZG9qld82q ITGA== X-Gm-Message-State: AOAM530BlPVTuT/lho6wQRHklRDbnsFoVvitp1D1NdRm9aLVR3ORlHSO K5hnGxvVqx5zNj/gQ5SKVcf3FQ== X-Google-Smtp-Source: ABdhPJz6Rr8YJdXRlpufO+qXLJP0pRM5RSrlId8hlIqYIzdY2syvCGB1T3WXPLTM8JsvbhxUlhQTGQ== X-Received: by 2002:a05:6a00:140d:: with SMTP id l13mr6230524pfu.22.1643346526306; Thu, 27 Jan 2022 21:08:46 -0800 (PST) Received: from sunil-ThinkPad-T490 ([49.206.3.187]) by smtp.gmail.com with ESMTPSA id d20sm7489481pfu.72.2022.01.27.21.08.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jan 2022 21:08:46 -0800 (PST) Date: Fri, 28 Jan 2022 10:38:39 +0530 From: Sunil V L To: Heinrich Schuchardt Cc: u-boot@lists.denx.de, Ard Biesheuvel , Anup Patel , Atish Patra , Abner Chang , Jessica Clarke Subject: Re: [RFC PATCH 1/1] efi_loader: Enable RISCV_EFI_BOOT_PROTOCOL support Message-ID: <20220128050839.GA5018@sunil-ThinkPad-T490> References: <20220126110611.33325-1-sunilvl@ventanamicro.com> <20220126110611.33325-2-sunilvl@ventanamicro.com> <69be5cb4-bd88-91cf-a277-8a1b0e89d159@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <69be5cb4-bd88-91cf-a277-8a1b0e89d159@gmx.de> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.5 at phobos.denx.de X-Virus-Status: Clean Hi Heinrich, On Thu, Jan 27, 2022 at 09:44:57AM +0100, Heinrich Schuchardt wrote: > On 1/26/22 12:06, Sunil V L wrote: > > This adds support for new RISCV_EFI_BOOT_PROTOCOL to > > communicate the boot hart ID to bootloader/kernel on RISC-V > > UEFI platforms. > > > > Signed-off-by: Sunil V L > > --- > > include/efi_api.h | 4 +++ > > include/efi_loader.h | 2 ++ > > include/efi_riscv.h | 16 +++++++++ > > lib/efi_loader/Kconfig | 7 ++++ > > lib/efi_loader/Makefile | 1 + > > lib/efi_loader/efi_riscv.c | 69 ++++++++++++++++++++++++++++++++++++++ > > lib/efi_loader/efi_setup.c | 6 ++++ > > 7 files changed, 105 insertions(+) > > create mode 100644 include/efi_riscv.h > > create mode 100644 lib/efi_loader/efi_riscv.c > > > > diff --git a/include/efi_api.h b/include/efi_api.h > > index 8d5d835bd0..f123d0557c 100644 > > --- a/include/efi_api.h > > +++ b/include/efi_api.h > > @@ -438,6 +438,10 @@ struct efi_runtime_services { > > EFI_GUID(0x607f766c, 0x7455, 0x42be, 0x93, \ > > 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f) > > > > +#define RISCV_EFI_BOOT_PROTOCOL_GUID \ > > + EFI_GUID(0xccd15fec, 0x6f73, 0x4eec, 0x83, \ > > + 0x95, 0x3e, 0x69, 0xe4, 0xb9, 0x40, 0xbf) > > + > > /** > > * struct efi_configuration_table - EFI Configuration Table > > * > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index 701efcd2b6..1fa75b40fe 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -527,6 +527,8 @@ efi_status_t efi_disk_register(void); > > efi_status_t efi_rng_register(void); > > /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */ > > efi_status_t efi_tcg2_register(void); > > +/* Called by efi_init_obj_list() to install RISCV_EFI_BOOT_PROTOCOL */ > > +efi_status_t efi_riscv_register(void); > > /* Called by efi_init_obj_list() to do initial measurement */ > > efi_status_t efi_tcg2_do_initial_measurement(void); > > /* measure the pe-coff image, extend PCR and add Event Log */ > > diff --git a/include/efi_riscv.h b/include/efi_riscv.h > > new file mode 100644 > > index 0000000000..6beb2637f6 > > --- /dev/null > > +++ b/include/efi_riscv.h > > @@ -0,0 +1,16 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > +/* > > + * RISCV_EFI_BOOT_PROTOCOL > > + * > > + * Copyright (c) 2022 Ventana Micro Systems Inc > > + */ > > + > > +#include > > + > > +#define RISCV_EFI_BOOT_PROTOCOL_REVISION 0x00010000 > > + > > Please, add Sphinx style comments to describe the structure. Sure. > > > +struct riscv_efi_boot_protocol { > > + u64 revision; > > + efi_status_t (EFIAPI *get_boot_hartid) (struct riscv_efi_boot_protocol *this, > > + efi_uintn_t *boot_hartid); > > +}; > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index 24f9a2bb75..77ba6a7ea1 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -369,4 +369,11 @@ config EFI_ESRT > > help > > Enabling this option creates the ESRT UEFI system table. > > > > +config EFI_RISCV_BOOT_PROTOCOL > > + bool "RISCV_EFI_BOOT_PROTOCOL support" > > + default y > > + depends on RISCV > > + help > > + Provide a RISCV_EFI_BOOT_PROTOCOL implementation on RISC-V platform. > > + > > endif > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile > > index fd344cea29..b2c664d108 100644 > > --- a/lib/efi_loader/Makefile > > +++ b/lib/efi_loader/Makefile > > @@ -62,6 +62,7 @@ obj-$(CONFIG_GENERATE_ACPI_TABLE) += efi_acpi.o > > obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o > > obj-$(CONFIG_EFI_RNG_PROTOCOL) += efi_rng.o > > obj-$(CONFIG_EFI_TCG2_PROTOCOL) += efi_tcg2.o > > +obj-$(CONFIG_EFI_RISCV_BOOT_PROTOCOL) += efi_riscv.o > > obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o > > obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o > > > > diff --git a/lib/efi_loader/efi_riscv.c b/lib/efi_loader/efi_riscv.c > > new file mode 100644 > > index 0000000000..91b8d2b927 > > --- /dev/null > > +++ b/lib/efi_loader/efi_riscv.c > > @@ -0,0 +1,69 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Defines APIs that allow an OS to interact with UEFI firmware to query > > + * information about the boot hart ID. > > + * > > + * Copyright (c) 2022, Ventana Micro Systems Inc > > + */ > > + > > +#define LOG_CATEGORY LOGC_EFI > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +static const efi_guid_t efi_guid_riscv_boot_protocol = RISCV_EFI_BOOT_PROTOCOL_GUID; > > +static efi_uintn_t hartid; > > I think we can do without this static variable. gd->arch.boot_hart does > not change. Yes. I missed EFI_ENTRY. Without EFI_ENTRY(), this didn't work. Will fix this in RFC V2 patch. > > > + > > +/** > > + * efi_riscv_get_boot_hartid() - return boot hart ID > > + * > > + * @this: RISCV_EFI_BOOT_PROTOCOL instance > > + * @boot_hartid caller allocated memory to return boot hart id > > + > > + * Return: status code > > + */ > > +static efi_status_t EFIAPI > > +efi_riscv_get_boot_hartid(struct riscv_efi_boot_protocol *this, > > + efi_uintn_t *boot_hartid) > > +{ > > + log_err(" efi_riscv_get_boot_hartid ENTER\n"); > > This seems to be left from debugging. > > Please, use EFI_ENTRY in all UEFI API interface implementations. > > EFI_ENTRY(""); > > This will show the function name when enabling debug messages. > > > + if ((this == NULL) || (boot_hartid == NULL)) > > We try to avoid == NULL. We should check against the actual protocol. > > if (this != riscv_efi_boot_prot || !boot_hartid) > Thanks!. Will fix these issues. > > + return EFI_INVALID_PARAMETER; > > + > > + *boot_hartid = hartid; > > + > > + return EFI_SUCCESS; > > return EFI_EXIT(EFI_SUCCESS). > > > +} > > + > > +static const struct riscv_efi_boot_protocol riscv_efi_boot_prot = { > > + .revision = RISCV_EFI_BOOT_PROTOCOL_REVISION, > > + .get_boot_hartid = efi_riscv_get_boot_hartid > > +}; > > + > > +/** > > + * efi_riscv_register() - register RISCV_EFI_BOOT_PROTOCOL > > + * > > + * > > + * Return: status code > > + */ > > +efi_status_t efi_riscv_register(void) > > +{ > > + efi_status_t ret = EFI_SUCCESS; > > + > > + /* save boot hart id since gd is not accessible after launching > > + * the kernel > > gd is accessible until EXIT_BOOT_SERVICES(). After EXIT_BOOT_SERVICES > the protocol cannot be called. > > set_gd(efi_gd) called by EFI_ENTRY() takes care of restoring the gp > register if the kernel should mess it up, which an UEFI program should > not do. Thanks a lot for your comments. Will address these issues and send V2 version. Thanks! Sunil > > Best regards > > Heinrich > > > + */ > > + hartid = gd->arch.boot_hart; > > + > > + ret = efi_add_protocol(efi_root, &efi_guid_riscv_boot_protocol, > > + (void *)&riscv_efi_boot_prot); > > + if (ret != EFI_SUCCESS) { > > + log_err("Cannot install RISCV_EFI_BOOT_PROTOCOL\n"); > > + } > > + return ret; > > +} > > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c > > index 49172e3579..380adc15c8 100644 > > --- a/lib/efi_loader/efi_setup.c > > +++ b/lib/efi_loader/efi_setup.c > > @@ -247,6 +247,12 @@ efi_status_t efi_init_obj_list(void) > > goto out; > > } > > > > + if (IS_ENABLED(CONFIG_EFI_RISCV_BOOT_PROTOCOL)) { > > + ret = efi_riscv_register(); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + } > > + > > /* Secure boot */ > > ret = efi_init_secure_boot(); > > if (ret != EFI_SUCCESS) >