From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mOdx1-0007cg-IN for mharc-grub-devel@gnu.org; Fri, 10 Sep 2021 06:40:07 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38890) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mOdwz-0007au-MX for grub-devel@gnu.org; Fri, 10 Sep 2021 06:40:06 -0400 Received: from mail-ve1eur01on071f.outbound.protection.outlook.com ([2a01:111:f400:fe1f::71f]:33862 helo=EUR01-VE1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mOdww-0002zJ-A1 for grub-devel@gnu.org; Fri, 10 Sep 2021 06:40:04 -0400 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=HS38q8IfXm2u6t24/6aMyjZX5ggVyu7FdQwbw2kuC9Xc32PoCW8A15tB/tjBrJJ0KdmWj45DLO/TNumX/5t/zEUPKsc7zLd0PkxLpnl27oMq2ahiB2scfIxiU1o53bRAItwUM4zQpKUqnrQzcQxJI7ZhhkhjKBzh8EOyzZ84SgEvV+CjGtz3itvswdT9gN2LJrDWm6VKH+fKn+wKUw9KZ6hcDmNnzgkL9YG6wUNNxM/uWeCFizKdDH//ZADIRibGVPbdw21lpenumrTuTLHcjzk+NoY5Yl6Nmbw28nh+rA5aTGD3Qs2ax4+ZQRoph/Jkjf4JYfKfSvj/Y0pndzpQmQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=y3e5ob2xRnSpJV8NqNlufFnhKGCIZSgd15DBKZ6dP6A=; b=X7J336XzuMLv22wA2W5o6qJOS6FRcSvYz0N+iBpTIYfUSnTlrHhP2sd79hQOE0AYTEctohxYAs7G+yPsKQ/MPkKJYAycpJWDgLizc8BNI8SJCERKUYXWimEpLPV42ZmBUeWAGsU5hCgDTuAOZBnCVi0BZrfkw5OoSpDz+fuebPRqv132IMcHrbc3ZTuBcHV4lnQO/PzwaKafx0FpQmS6rKYfedPMU3aTsWz8BPtonFIU5d7xSUR+RHPd0w7OKCq+F0QoHoJZI+7p8m3Yeapa2Ck8lIoISC78oYkwWB9UxefOru8q2XqcvrrJyijP2rOd6aze5VsAskorLvWZdgYYag== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=criteo.com; dmarc=pass action=none header.from=criteo.com; dkim=pass header.d=criteo.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=criteo.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=y3e5ob2xRnSpJV8NqNlufFnhKGCIZSgd15DBKZ6dP6A=; b=ZZJIbHtkjm3jasGJDMSTiW7HrnpDWXk8DMsrH4qYk/3DHqdlbMuT5NJT299+9kq7S8SqSJlR3beDmDXVThzBHYyTKf4d9+/evIIwip5Hd6N7YiPkRWvmVV0FroWsmpUNdvFr+UIC2fdD8WL/rryTpAPmi4lPOc9TG+CBpprtVRQ= Authentication-Results: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=criteo.com; Received: from AM9PR04MB8340.eurprd04.prod.outlook.com (2603:10a6:20b:3e0::23) by AM0PR04MB4563.eurprd04.prod.outlook.com (2603:10a6:208:6b::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4478.25; Fri, 10 Sep 2021 10:34:55 +0000 Received: from AM9PR04MB8340.eurprd04.prod.outlook.com ([fe80::8496:1aec:598a:3ed1]) by AM9PR04MB8340.eurprd04.prod.outlook.com ([fe80::8496:1aec:598a:3ed1%8]) with mapi id 15.20.4500.017; Fri, 10 Sep 2021 10:34:55 +0000 Subject: Re: [PATCH v2] kern/efi: Adding efi-watchdog command To: Daniel Kiper , Erwan Velu Cc: grub-devel@gnu.org, alexander.burmashev@oracle.com, phcoder@gmail.com References: <20210902165035.1986540-1-e.velu@criteo.com> <20210909154649.me2l7z5wn2x6zf7a@tomti.i.net-space.pl> From: Erwan Velu Message-ID: <0fd67cfd-fcdd-bc2a-c532-0be66f5b0206@criteo.com> Date: Fri, 10 Sep 2021 12:34:54 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 In-Reply-To: <20210909154649.me2l7z5wn2x6zf7a@tomti.i.net-space.pl> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-ClientProxiedBy: PR0P264CA0166.FRAP264.PROD.OUTLOOK.COM (2603:10a6:100:1b::34) To AM9PR04MB8340.eurprd04.prod.outlook.com (2603:10a6:20b:3e0::23) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [IPv6:2a01:cb00:de8:8800:60d9:3723:4a46:c8d0] (2a01:cb00:de8:8800:60d9:3723:4a46:c8d0) by PR0P264CA0166.FRAP264.PROD.OUTLOOK.COM (2603:10a6:100:1b::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4500.14 via Frontend Transport; Fri, 10 Sep 2021 10:34:55 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 55fe4dba-ce52-42ea-8872-08d97446a190 X-MS-TrafficTypeDiagnostic: AM0PR04MB4563: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: J2bwIVs8iIdyJcXASFPW6+zY0aTAz3BFalxK74+FCuoNRLhr9JwsBeESjHxCgbPrmQwdP1/whVz0YbIZS/7hATqELAizaA0Uz9McjsZV4+xg86NpYEFnkuK0x4JB4bMRMfaBeHVZsi8jo9exDVV7W/NneTbhxcJnQTBMRQAQBG7rtNnJZdQX93y4Tk+UWgvD3SDdHrVez4Q1BPzzHaO9viBK8SKMSV+bIY5uI88ZBIjwjrnuULRu38vBuDB6108d5TQ0Jb6W83OCmbJCq7tQZ+NvMyv/dMRy767Cbo5xfrtmXl3PLQ/go0j2WqPjitdKR+q+gMq5aZwWx7VCzPJkEAkmqA9fdqvSSVVcQAhR9mUwO5p7JQN5gYbkMqvUPpdnTxiiVaw6jw6y2BfGl3pjaqn5+qLDQColLgiZb1pK+0sax1ICYcXO7tIDT3EyVmR/+8bjPbaNHXo002GIdLdijMGdaVRpmh503e7/TpHFy3GdHWbEsUbZwcafxEufbob4h7Dwem/zYCgh6Ovw5OaRoxQaO+a52x71+lqHVq1z0ZaNxCB9A/rIt2nhKo5Jg25OTe8GOkhsz4cwM6f8laqeMKrYVKq1si1kJex90Ig6ypRV1kcz4jb7TpDFBBS0kXkDvpr5/tlEviL1eB/PzEY10yxKtAt7EvmSVCOPDGeRI07URfe4HV1jeB0uY7p2q3NN09fnmY8xfeplUt62m976K7B0MzWOl5bDxGPtyFW4VDs= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM9PR04MB8340.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(346002)(39850400004)(376002)(136003)(396003)(31696002)(478600001)(66476007)(31686004)(66946007)(83380400001)(8936002)(2616005)(36756003)(316002)(6486002)(8676002)(186003)(110136005)(2906002)(4326008)(38100700002)(86362001)(66556008)(5660300002)(66574015)(52116002)(43740500002)(45980500001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RnRaS2hFMG5CU3QwRmRZamo1TTVCTXVFZkU3T2dqelI0MXJ1UlVJZW9MVnFP?= =?utf-8?B?M3Zqd05IZDRpZXJqVWNJLy9KU3JEZ3BrWk1YVDNJRWozUzNmNlBzbzN0Vjlx?= =?utf-8?B?QnFGaCtIYmU1aE9RMjhuNUVTa1c0Y1dUK3lqakJjZ2lzeURMVFlXTHFOQ09U?= =?utf-8?B?aVdtL1FYRUxkaTM1SUJRdDdGVitPZnQxMVl3YlZwOWJxTWlkS3BKM2JVa3pO?= =?utf-8?B?UUxzSVpKYzZSdVFBMjhtelFWTGUvTFFZaVBuWVJmQVlxTFhzdEo0TjBIR0hH?= =?utf-8?B?TWNOMFg4UzYrNFFTd0pySzUvUlM2MXAzRUxNeEpDaVFNYTZwUkpsVmQwMmJm?= =?utf-8?B?YUp4MGVaUTRkK0hKVzZ0WHdPdE5ybno4ZkswU2hVY3Q4ZWw4TzhPUmthaVVj?= =?utf-8?B?aDBNbmdCUi8zM0tnaFdoa3oySWVMYXVHd1JtNDdNdGNHc1BVQ3ZyY0lFKzJp?= =?utf-8?B?QUIvWkRkNUZxbnJ2eEdMMlpVZXBvOEswZVRGWDJmaFl5NUZLTjdWMW5tTHMy?= =?utf-8?B?VTFvZVVzdWZKMi9DSkF6MWRxeU1LM3hJNXd5WHFQeEh5aVNHTFkwdVFhSHpY?= =?utf-8?B?Vm9lb0dsVUJ6aUlOZWpvNUoxQjNUZ0RyaXc0TlMybHNDbGs5YWJhQnZ4M3B2?= =?utf-8?B?TUlPajlXRFFZNnJaZjdnU1dCdC9zK0ltMmRDVEpCZWI0eU9jbXZvSzE4aFRm?= =?utf-8?B?TWw4YVExR05kZ280RlQ4cFlVczlLOGJrcU5QL3QxVWtwVmFEaTZHZWtwRW5E?= =?utf-8?B?b2RzdXV5TDN6NHgxVXFObE9LNUhIWElZRlpORk1ySWxEUWNqUjlJa3g5UTRw?= =?utf-8?B?U3ZtWWl6YU9PVzZOZ09Sb0d3M3ZkOWlha2NmRzZXS2Q2TVRlUTFNS0xHTHhG?= =?utf-8?B?UXBMNWdkajFTL1lqbGFwdDBIbE5pS1dHZGNFYkdPZW41c2pVL1BJNG5vTzZQ?= =?utf-8?B?ZFFmOHlHMXJEZlRONnJNazgyQ2Z1a2h3L2ZCcjBOUUs3UFh2R2taRko4OWp1?= =?utf-8?B?VEZqU0cyWUk3eHlXNWkyeCsrYU5KTHhOcHI1SmV1RURpZDhVaFJRK2RKbWZv?= =?utf-8?B?Y2VrNllmYTl2aWlWeS9rNEg1cnN3OGwveUp5R1NYdGJuTUJRUklRNUp4c25y?= =?utf-8?B?T0x4UmtoNWlwQnJ2MnZBV0cyYytHQzVIZGY0OHhGMHg1RlFyZGpXOWlFSG5W?= =?utf-8?B?Q3FUc2RwRU1oWm1ic1paSGNqbUFpTmNzWmxXalFqVHBDNzFraXdtUGtPQXJQ?= =?utf-8?B?YzcvQ2JkaFBDRWxscWljNDRZZ0QrS0ZPeGgrSHlidVRSb1ovUXg3L0Qyemd6?= =?utf-8?B?b04zT1U4OW1pZVg1YXlwR2pPRWlJUU5DOVVqV2paejJtRCtmaVdnV3ltck5W?= =?utf-8?B?cThjM1kzUlRISGhFdkc1bm9wOUdWVjgxQSszeC95VnVOeTREbUVPd0ZpMWJt?= =?utf-8?B?bWpmWit0ckh4ZnE4UUpZbFh5ckZheFFDUVVVbGpTbGw3YUw1TUdjVE1ONUtv?= =?utf-8?B?amE3dVJaTzlKMUcwejVGTDRVMkY5YmhIeFJudkdCVHRDRHd0cTUrWW81NXNG?= =?utf-8?B?R3pOY0MvZjRuTCtia1ZEYnFBVmxPdDAxS0VIaXJNdnlwWGVtcHRyNCt0SG5t?= =?utf-8?B?YWZFbWRFYjBzalhDZkNFZFpnam9lcmVGcTl1Q3k5bGpVbEZIRHpnaktRV3lQ?= =?utf-8?B?UlVyaTlxMVR0OFRaVkdSdTNVcit3SS9oWUQzejBaTnhDNzNwTExaWkx5ajhO?= =?utf-8?B?Qk4zK3NYV1U5Q0NaR2JIdURhVThucG9qMVErYVA3QzdoQ1E5a3Y5cGhmOEh1?= =?utf-8?B?UHlYMVJWRHZjSExiWGg4aEcxMjRhcUhvampkU1d5SER4c1I1K0RFN1RJZHBt?= =?utf-8?Q?OYG5OOYZd8F83?= X-OriginatorOrg: criteo.com X-MS-Exchange-CrossTenant-Network-Message-Id: 55fe4dba-ce52-42ea-8872-08d97446a190 X-MS-Exchange-CrossTenant-AuthSource: AM9PR04MB8340.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Sep 2021 10:34:55.8106 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 2a35d8fd-574d-48e3-927c-8c398e225a01 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: dE9L9bVFArI9q0XVgt0IBgGA9kc8XxWMoS8OTyaYbNWe34bDZPCaTeUvsW4dKO3dVmsvJTNOiL3P3BObFftlgw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB4563 Received-SPF: pass client-ip=2a01:111:f400:fe1f::71f; envelope-from=e.velu@criteo.com; helo=EUR01-VE1-obe.outbound.protection.outlook.com X-Spam_score_int: -40 X-Spam_score: -4.1 X-Spam_bar: ---- X-Spam_report: (-4.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, MSGID_FROM_MTA_HEADER=0.001, NICE_REPLY_A=-1.975, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Sep 2021 10:40:06 -0000 Thanks for your review, I'll rework my patch according to your comments. I just kept the open points in this thread. Le 09/09/2021 à 17:46, Daniel Kiper a écrit : > > +static grub_err_t grub_efi_set_watchdog_timer(unsigned long timeout) > Please use grub_efi_uintn_t instead of unsigned long. However, please > remember that its size depends on architecture. So, it can be 32-bits or > 64-bits... I'll follow the uefi spec here so grub_efi_uintn_ is fine. >> + The numeric code to log on a watchdog timer timeout event. >> + The firmware reserves codes 0x0000 to 0xFFFF. >> + Loaders and operating systems may use other timeout codes. > This comment says you cannot use 0xb007. I would use 0xdeadb007deadb007 > by default. However, I would give an option to change the default by user. Stupid me .... I was wondering if the code should be configurable. In the initial patch it was the case but though this would give an easier command line interface to users. I mean, the aim of this code is to report mostly 'who' manipulated the watchdog, so sound good to me to get a value that is more grub depend than user centric. > >> + */ > Please format comments according to [2]. > >> + grub_efi_uint64_t code = 0xB007; >> + grub_efi_status_t status = 0; >> + >> + if (timeout >= 0xFFFF) >> + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("efi-watchdog: timeout must be lower than 65535")); > Why do you artificially limit the timeout value? Please do not do that. Mistake of my own, make sense to remove that. > >> + if (timeout > 0) >> + grub_printf("grub %s: Arming EFI watchdog at %lu seconds\n", PACKAGE_VERSION, timeout); > If you want debug messages please use grub_dprintf() instead of grub_printf(). Maybe my print isn't ideal but I found important to warn the user a watchdog is armed. If the user is not informed and the watchdog fires, the user could think there is a hardware issue on the host. So to me, this message is mandatory to say users : "beware, the system will be reset within x seconds if you don't boot"