Received: by 2002:a05:6a10:22f:0:0:0:0 with SMTP id 15csp2102506pxk; Sat, 26 Sep 2020 17:21:55 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzuAwvEmtCBp9kXn/ro8dsKrDNl5wLYgmIiht/ELk8lHYAOH7J7IKD3CpFebWHzLc2AH3Bm X-Received: by 2002:a17:906:fcc8:: with SMTP id qx8mr9128088ejb.13.1601166115096; Sat, 26 Sep 2020 17:21:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1601166115; cv=none; d=google.com; s=arc-20160816; b=eGZ3STtPHOrykBZ/LA38fwW3vs1JmDFvTyydiIUJDDoyT/vyhdIw7Nxvns1OO3On0q PJnS3ks5Kx1+AiZ5IZ7VtJc/Zv2R8mo4aPT9dHQ5jtkP+sh1Jdoo6cR36c/y9lcd4zvI mXBWdIDtRbt1DDQqzwPghqDDXcMCZLOkdn7aDk/1EAORugzPNrw72SWJrv586GBCV923 JKtXaPpptq/plDM6gbXF88p2GRbLtWfh65PijIlKMCs4ZlmYlby3bQsIFeWJYSfSIEQi xiACvFnM4eESySzcqFlS1ns0ORLktPmpjByfmyCFTy5XXJfLT13e3EuzHCIjK9U/raZk LT8w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:to:from:cc:in-reply-to:subject:date:dkim-signature; bh=Hs9mLaM9QJjA7QhpUv/f+9+N0sU3vQhU7QQqL2xb8zU=; b=tnxsaq7IEnIgE2Me07d270D/qzmzsbQ6L18rKhNk32p/lh6JFFAmY0YWVxCXG7u/u1 bod/wAEfgF0qUMqtc5J6rX3CEFpgAf6+J8A2o6uC/7V9k6y9NStczadAn2Qdd7EVrFDC MtoJlm0n/+JUkLROqusbp8Ynrs+enXoS8y1hH6yjN4Bbn2DqK6X90XsF+oTn8KIl+LiY 3NWSczMx6Zj7p4ujGnAPropPHfNt4amebLvhb0+4NNkx83RUIqW/F83VRf/rCbBCFmSk eZRzpvysl0P6GdyqDsOdEapaHY7mdvkHmEVmvfU+QAZvTz1fT9wUQyndMoAZmfx5PZOr xjow== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=KgYHR3ev; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id ay19si4499259ejb.626.2020.09.26.17.21.31; Sat, 26 Sep 2020 17:21:55 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=KgYHR3ev; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728585AbgI0AUO (ORCPT + 99 others); Sat, 26 Sep 2020 20:20:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56044 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726840AbgI0AUN (ORCPT ); Sat, 26 Sep 2020 20:20:13 -0400 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BE6B7C0613CE for ; Sat, 26 Sep 2020 17:20:13 -0700 (PDT) Received: by mail-pj1-x1041.google.com with SMTP id mm21so1491743pjb.4 for ; Sat, 26 Sep 2020 17:20:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:subject:in-reply-to:cc:from:to:message-id:mime-version :content-transfer-encoding; bh=Hs9mLaM9QJjA7QhpUv/f+9+N0sU3vQhU7QQqL2xb8zU=; b=KgYHR3evLAlofZjmxy7xtcAssDoyckmUck6gmNfmVG1EL2iTR188aSUJtHSDSUZ0nE xdC+ewr7xw4AyO2CdGamgs2aGkb/k4ssA/QFlBNU/S0eQmKTr2bQxOc/IfrMaTqJ7oj3 HMCmF2I1Z3qsdwGEOIaGR4Vlja3UhrUTgW2nK+C9ChSgaeKij3W1ld0FsaG93eJ2DDHn 87E2Q131HOs1rp9i6xb9jXwnfWnZnarbC/EAgVTSPXX6SdagZbxMTe4zQdVjMtni5rEz COebEo9K4rUZQX5WnLf17bP7E3ngbcJKqJxn/2O8CDMNzMnaMYeQ+b3k6SWvpzRSK5VS scWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id :mime-version:content-transfer-encoding; bh=Hs9mLaM9QJjA7QhpUv/f+9+N0sU3vQhU7QQqL2xb8zU=; b=UtHkxGBrSknqSSzBhPBEyktCpDheD/EWMdXAnipqJTohdt9S0TkQsck5apRBgt7+/Q 0bh3cDv+8q6DHQIx3Cw2Dtmi07LQBG6YjcFzbGy96N/E2C96KTsqGpHhX2aHWQEhOUe8 HiPNHT+IbQoIq4QiRQ492AwswNnrxFOgYxFpbrpinjmf5hPePmfZlBpMLSvlBFP/97ux PlJeMxKPGaCXRf3YawI/pDdwq0AjoZFZKEDREOEWUCs0szKi1ayIQCVF4hHqpeh7U6Hl Z6nG2VfVzqqsoEkOML9A5Dg6QxJZ2SK6vIYNFkTkYpRjZLui5NOe3POwZPdpAKRXkfGA 8AcQ== X-Gm-Message-State: AOAM533RpEOt+SPjEzYIWNkSqgZYwkvzgang51YfPaEBDW7ordL4PUKi 7s7ERx6H7o+7iC3BlYtRubQ19Q== X-Received: by 2002:a17:90a:528a:: with SMTP id w10mr3580342pjh.107.1601166013023; Sat, 26 Sep 2020 17:20:13 -0700 (PDT) Received: from localhost (76-210-143-223.lightspeed.sntcca.sbcglobal.net. [76.210.143.223]) by smtp.gmail.com with ESMTPSA id p11sm6640791pfq.130.2020.09.26.17.20.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 26 Sep 2020 17:20:12 -0700 (PDT) Date: Sat, 26 Sep 2020 17:20:12 -0700 (PDT) X-Google-Original-Date: Sat, 26 Sep 2020 17:20:10 PDT (-0700) Subject: Re: [PATCH v2] RISC-V: Check clint_time_val before use In-Reply-To: <4950cd11fcf66bc3606f309d6289577b5dc65b2e.camel@wdc.com> CC: Paul Walmsley , Anup Patel , aou@eecs.berkeley.edu, anup@brainfault.org, linux-riscv@lists.infradead.org, Atish Patra , Alistair Francis , linux-kernel@vger.kernel.org From: Palmer Dabbelt To: Damien Le Moal Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 26 Sep 2020 03:31:29 PDT (-0700), Damien Le Moal wrote: > On Sat, 2020-09-26 at 15:51 +0530, Anup Patel wrote: >> The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6 >> because the get_cycles() and friends are called very early from >> rand_initialize() before CLINT driver is probed. To fix this, we >> should check clint_time_val before use in get_cycles() and friends. I don't think this is the right way to solve that problem, as we're essentially just lying about the timer rather than informing the system we can't get timer-based entropy right now. MIPS is explicit about this, I don't see any reason why we shouldn't be as well. Does this fix the boot issue (see below for the NULL)? There's some other random-related arch functions so this might not be quite the right way to do it. diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h index 7f659dda0032..7e39b0068932 100644 --- a/arch/riscv/include/asm/timex.h +++ b/arch/riscv/include/asm/timex.h @@ -33,6 +33,18 @@ static inline u32 get_cycles_hi(void) #define get_cycles_hi get_cycles_hi #endif /* CONFIG_64BIT */ +/* + * Much like MIPS, we may not have a viable counter to use at an early point in + * the boot process. Unfortunately we don't have a fallback, so instead we + * just return 0. + */ +static inline unsigned long random_get_entropy(void) +{ + if (unlikely(clint_time_val == NULL)) + return 0; + return get_cycles(); +} + #else /* CONFIG_RISCV_M_MODE */ static inline cycles_t get_cycles(void) >> Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation >> for M-mode systems") >> Signed-off-by: Anup Patel >> --- >> Changes since v1: >> - Explicitly initialize clint_time_val to NULL in CLINT driver to >> avoid hang on Kendryte K210 >> --- >> arch/riscv/include/asm/timex.h | 12 +++++++++--- >> drivers/clocksource/timer-clint.c | 2 +- >> 2 files changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h >> index 7f659dda0032..6e7b04874755 100644 >> --- a/arch/riscv/include/asm/timex.h >> +++ b/arch/riscv/include/asm/timex.h >> @@ -17,18 +17,24 @@ typedef unsigned long cycles_t; >> #ifdef CONFIG_64BIT >> static inline cycles_t get_cycles(void) >> { >> - return readq_relaxed(clint_time_val); >> + if (clint_time_val) >> + return readq_relaxed(clint_time_val); >> + return 0; >> } >> #else /* !CONFIG_64BIT */ >> static inline u32 get_cycles(void) >> { >> - return readl_relaxed(((u32 *)clint_time_val)); >> + if (clint_time_val) >> + return readl_relaxed(((u32 *)clint_time_val)); >> + return 0; >> } >> #define get_cycles get_cycles >> >> static inline u32 get_cycles_hi(void) >> { >> - return readl_relaxed(((u32 *)clint_time_val) + 1); >> + if (clint_time_val) >> + return readl_relaxed(((u32 *)clint_time_val) + 1); >> + return 0; >> } >> #define get_cycles_hi get_cycles_hi >> #endif /* CONFIG_64BIT */ >> diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c >> index d17367dee02c..8dbec85979fd 100644 >> --- a/drivers/clocksource/timer-clint.c >> +++ b/drivers/clocksource/timer-clint.c >> @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq; >> static unsigned int clint_timer_irq; >> >> #ifdef CONFIG_RISCV_M_MODE >> -u64 __iomem *clint_time_val; >> +u64 __iomem *clint_time_val = NULL; This one I definately don't get. According the internet, the C standard says If an object that has static storage duration is not initialized explicitly, it is initialized implicitly as if every member that has arithmetic type were assigned 0 and every member that has pointer type were assigned a null pointer constant. so unless I'm missing something there shouldn't be any difference between these two lines. When I just apply this I get exactly the same "objdump -dt" before and after. I do see some difference in assembly, but only when I don't pass "-fno-common" and that ends up being passed during my Linux builds. >> #endif >> >> static void clint_send_ipi(const struct cpumask *target) > > For Kendryte: > > Tested-by: Damien Le Moal > > -- > Damien Le Moal > Western Digital