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 BEBA1C433F5 for ; Fri, 21 Jan 2022 13:15:17 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7EB9981F6D; Fri, 21 Jan 2022 14:15:15 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org 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=linaro.org header.i=@linaro.org header.b="SeVQyobX"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 0370381DCC; Fri, 21 Jan 2022 14:15:14 +0100 (CET) Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) (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 2DBF68141C for ; Fri, 21 Jan 2022 14:15:11 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-wm1-x32e.google.com with SMTP id r132-20020a1c448a000000b0034e043aaac7so5000410wma.5 for ; Fri, 21 Jan 2022 05:15:11 -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=vu+amSYdnS5VbtRL5jyB0o4puGbXyKDHw5d6UNSwU/o=; b=SeVQyobXNZG/kJXH0Pd4ZTS4xwKXgD/vZwUCCjpf0vgPuPlb22TT/K0BeAEykw3ngq sZR9fcDQUJPjNN240VUIrUAnAkWVxv29Xw+9D3tAkALvejdQFEQWIGQw+aMITDsFmhYo PhxyzRNvEksyDwXoxyTxa6NlbYCmOuVaBIVWLVDknRVnNkZlwdYNu7fIWW4VjzNnN4kh lNBo+h79+MwAKwcmAgA5wq3dUBp2XRJ8GWbAa3ZVSd18IxYSJu7rxZO8D2uURwU6edFY hatiOYAEBkWDi69ErXd8kBRRwvrtPrCXDc4+0Vsw4Iu1OZTtsMSjZT00Z33wpofd6B74 SxFw== 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=vu+amSYdnS5VbtRL5jyB0o4puGbXyKDHw5d6UNSwU/o=; b=7BriKWJzVWc2/2JQQ9i4iHauCSF45S9O37ZdfQY0vDUaQz1wClRpnSX5M18TfZ6GDm fnD88xJINkC9Ep/Wpx+GcEwlHgjJsMJI3f9k6Qny0qYpPvaFHpDlN6NCluHfxySmD4im 6d13G0PV4bIbiVWJfslMDsrpgZ3OH2pqt1HqKvpNcUviE8lq0zSB9yTy7vxrlEL3lu+B JmyOIF4P5eJOi3o//ingO6yMVC1diZUhzbVbWF5XmfU5jOC8u3J4LaLfDZWGeJfVqzBk LJk5HT6rgYcQD8ZoakMZD/23SS+0+UbRXs8ah5RzBOmTcJ9G1vbsaGGt/e7cp5BqIBzK U/zw== X-Gm-Message-State: AOAM5313BGYPkwJeKK7/6mqBAqfQkcdsoJdr2IbOYs8JOP5Bld2xP2dk cEAIaOFgBapyvHSmsd7U1lIGtw== X-Google-Smtp-Source: ABdhPJxjp8HIVyZ6TZCA8ZYjPKU3E6dPyq388ABEwGl6jK7N36kbE/5ZzEI0tTzFEcd8IVSWQzIqIA== X-Received: by 2002:a7b:c76f:: with SMTP id x15mr685675wmk.37.1642770910551; Fri, 21 Jan 2022 05:15:10 -0800 (PST) Received: from hades (athedsl-4461669.home.otenet.gr. [94.71.4.85]) by smtp.gmail.com with ESMTPSA id 12sm4966809wmf.18.2022.01.21.05.15.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Jan 2022 05:15:10 -0800 (PST) Date: Fri, 21 Jan 2022 15:15:07 +0200 From: Ilias Apalodimas To: Sughosh Ganu Cc: u-boot@lists.denx.de, Masami Hiramatsu , Patrick Delaunay , Patrice Chotard , Heinrich Schuchardt , Alexander Graf , AKASHI Takahiro , Simon Glass , Bin Meng , Jose Marinho , Grant Likely , Tom Rini , Etienne Carriere Subject: Re: [RFC PATCH v3 6/9] FWU: Add boot time checks as highlighted by the FWU specification Message-ID: References: <20220119185548.16730-1-sughosh.ganu@linaro.org> <20220119185548.16730-7-sughosh.ganu@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220119185548.16730-7-sughosh.ganu@linaro.org> 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 Sughosh [...] > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +static u8 trial_state = 0; > +static u8 boottime_check = 0; > + > +static int fwu_trial_state_check(void) > +{ > + int ret, i; > + efi_status_t status; > + efi_uintn_t var_size; > + u16 trial_state_ctr; > + u32 nimages, active_bank, var_attributes, active_idx; > + struct fwu_mdata *mdata = NULL; > + struct fwu_image_entry *img_entry; > + struct fwu_image_bank_info *img_bank_info; > + > + ret = fwu_get_mdata(&mdata); > + if (ret < 0) > + return ret; > + > + ret = 0; > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK; > + active_bank = mdata->active_index; > + img_entry = &mdata->img_entry[0]; > + for (i = 0; i < nimages; i++) { > + img_bank_info = &img_entry[i].img_bank_info[active_bank]; > + if (!img_bank_info->accepted) { > + trial_state = 1; > + break; > + } > + } > + > + if (trial_state) { That's personal preference and feel free to ignore it, but I generally find the code easier to read like if (!trial_state) do stuff; goto out; > + var_size = (efi_uintn_t)sizeof(trial_state_ctr); > + log_info("System booting in Trial State\n"); > + var_attributes = EFI_VARIABLE_NON_VOLATILE | > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS; Do we really need it with runtime access? > + status = efi_get_variable_int(L"TrialStateCtr", > + &efi_global_variable_guid, > + &var_attributes, > + &var_size, &trial_state_ctr, > + NULL); > + if (status != EFI_SUCCESS) { > + log_err("Unable to read TrialStateCtr variable\n"); > + ret = -1; > + goto out; > + } > + > + ++trial_state_ctr; > + if (trial_state_ctr > CONFIG_FWU_TRIAL_STATE_CNT) { > + log_info("Trial State count exceeded. Revert back to previous_active_index\n"); > + active_idx = mdata->active_index; > + ret = fwu_revert_boot_index(); > + if (ret < 0) { > + log_err("Unable to revert active_index\n"); > + goto out; > + } > + > + trial_state_ctr = 0; > + status = efi_set_variable_int(L"TrialStateCtr", > + &efi_global_variable_guid, > + var_attributes, > + 0, > + &trial_state_ctr, false); > + if (status != EFI_SUCCESS) { > + log_err("Unable to clear TrialStateCtr variable\n"); > + ret = -1; > + goto out; > + } > + } else { > + status = efi_set_variable_int(L"TrialStateCtr", > + &efi_global_variable_guid, > + var_attributes, > + var_size, > + &trial_state_ctr, false); > + if (status != EFI_SUCCESS) { > + log_err("Unable to increment TrialStateCtr variable\n"); > + ret = -1; > + goto out; > + } else { > + ret = 0; > + } > + } > + } else { > + trial_state_ctr = 0; > + ret = 0; > + status = efi_set_variable_int(L"TrialStateCtr", > + &efi_global_variable_guid, > + 0, > + 0, &trial_state_ctr, > + NULL); We can't completely ignore the failed setvariable here, as it will affect the entire code path from that point onwards. Any checks after an update will fail since the variable wont be there. > + } > + > +out: > + free(mdata); > + return ret; > +} > + > +u8 fwu_update_checks_pass(void) > +{ > + return !trial_state && boottime_check; > +} > + > +int fwu_boottime_checks(void) > +{ > + int ret; > + u32 boot_idx, active_idx; > + > + ret = fwu_mdata_check(); > + if (ret < 0) { > + boottime_check = 0; > + return 0; > + } > + > + /* > + * Get the Boot Index, i.e. the bank from > + * which the platform has booted. This value > + * gets passed from the ealier stage bootloader > + * which booted u-boot, e.g. tf-a. If the > + * boot index is not the same as the > + * active_index read from the FWU metadata, > + * update the active_index. > + */ > + fwu_plat_get_bootidx(&boot_idx); > + if (boot_idx >= CONFIG_FWU_NUM_BANKS) { > + log_err("Received incorrect value of boot_index\n"); > + boottime_check = 0; > + return 0; > + } > + > + ret = fwu_get_active_index(&active_idx); > + if (ret < 0) { > + log_err("Unable to read active_index\n"); > + boottime_check = 0; > + return 0; > + } > + > + if (boot_idx != active_idx) { > + log_info("Boot idx %u is not matching active idx %u, changing active_idx\n", > + boot_idx, active_idx); > + ret = fwu_update_active_index(boot_idx); > + if (ret < 0) > + boottime_check = 0; > + else > + boottime_check = 1; > + > + return 0; > + } > + > + ret = fwu_trial_state_check(); > + if (ret < 0) > + boottime_check = 0; > + else > + boottime_check = 1; > + > + return 0; > +} > -- cheers /Ilias > 2.17.1 >