From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 07004EEDC for ; Fri, 21 Jun 2024 13:15:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718975715; cv=none; b=V8E4SWSPWzXmwCSkY3zLeQYFxclHBNxrspFQzkjugrAoM0clkrLwDBEYcQnz0s/SLUwooL0VcLjT3RpiGyL8OjI2/E99nz2l1tER0Uij1OwhYns5tI8EEdQUTLB1yYFXDVPh8RE+kSbiEkScwDZm5tfjN24iZ0rGVNRlBehih0g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718975715; c=relaxed/simple; bh=mQ7wmAolxTUXR3TDySGFFEBFu0+G4nfni9DtqD1DkcY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=alSWdyjgRZoZB9uElWwTVYV2m0hMG9wAQLYvMvXlbcehptu7CrbVvjvgZ0Bv0SAnNiaZDcZh5fx0XFoQBvEW1sZ2lJdo2sfkxGsIj4kEbuQcpjngxQIVVArDAC5YKfwAa0s0F5fJHzhW2gVftWMVz1psQu8z/4Fxa1+2O7bFbdg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ventanamicro.com; spf=pass smtp.mailfrom=ventanamicro.com; dkim=pass (2048-bit key) header.d=ventanamicro.com header.i=@ventanamicro.com header.b=Gwxa6YpY; arc=none smtp.client-ip=209.85.208.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ventanamicro.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ventanamicro.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ventanamicro.com header.i=@ventanamicro.com header.b="Gwxa6YpY" Received: by mail-ed1-f47.google.com with SMTP id 4fb4d7f45d1cf-57ccd1111aeso2458754a12.0 for ; Fri, 21 Jun 2024 06:15:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1718975712; x=1719580512; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=cD16Eg0jMB1tLt142Rk+A9t4pdlm9NDQxO24pzXHIlI=; b=Gwxa6YpYwPnU1fvJaYdvEg7BG1HarfT1uE81qaTj1WKttIAWtOET+UuSJ25JcJQqLP BaYuPv7MdDuVL7KmpbEqpMAsGzx4smSWbgivSU76eVE4PX8//+7vMit8/YH1hnSOQ2mB Zxs2J83sE7FWhqzAvhdF8UVgZP8RX8dg3muaqIxHC+v/QgkeNPeasCChZPxZhlL43oPl Pqvq/sHmQMibi6nqXZ7+uD1cdcOovc2guF9nS1Z9w8pa13rMmHUV2Vll+1Ew+De3QxlR P11HfywhKHbQGDxZ4zfBbFa4U1m4cQm/f6zVHS5lq+Ep3gCbfK8kuMkkaFeSRYTrURIO p6mA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718975712; x=1719580512; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=cD16Eg0jMB1tLt142Rk+A9t4pdlm9NDQxO24pzXHIlI=; b=mn4O8jQiYgWWKqMJKpp2PDHdFT0y3w4jTXFdhCYQv/VspEFuq9lpnVro2pRc1aFkpB NoQ+h24iUavcx4fK4xrLPTB6lOR9cAdKub/phwO/FT+w2yRuEq6MkqBjFU8wA8cLusJy qC/Lb87NOBiZiLSfcphFt+Xav/z4G92GOhmX8rDCrdVOus/5Q2TKyw3GRECcKn5Qb7qf CiavJNsvS7GyQS0MazszX4b5Sq4zbLjHQtZNUApdrF4rkPUwWXPTwOvxu2H/QC5TyzeC M9n4u4vyxzEPSQ5svBVfBEs/w2Slczd3X6mA1sS/Ma2O3DzmCn8+PKlGAWNORySUEHLO aDLw== X-Forwarded-Encrypted: i=1; AJvYcCXFf4HLogoRY+vZUG96NYbS+W20OIAP16cgRBhbxjmMe9qsncXKAPogvjl4XqaEFPyuu0o0IkrDcnrGo5oP3bGxRS/xRdCuuJjlXw== X-Gm-Message-State: AOJu0YzOFEL89C7T7uq+SvzxZHtQjjjk4odm+X7iR6zIhzPZyJEX2Jod ru0ltBEUwsaIWfjc8l8JGmGmY0hk5TxhWDVNafwIBebMk4IRuWvPkQxp9QgN7IQ= X-Google-Smtp-Source: AGHT+IEHzhMsJ0Rmre1Op5vsKrU4YAx88gVFBKQkrx9xQgMhlCsZix0dkICFq5gGjvtBpIaF8RFzng== X-Received: by 2002:a50:8719:0:b0:57c:571f:e52d with SMTP id 4fb4d7f45d1cf-57d07ebeaaamr5721984a12.36.1718975712142; Fri, 21 Jun 2024 06:15:12 -0700 (PDT) Received: from localhost (2001-1ae9-1c2-4c00-20f-c6b4-1e57-7965.ip6.tmcz.cz. [2001:1ae9:1c2:4c00:20f:c6b4:1e57:7965]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-57d3042fd72sm954912a12.48.2024.06.21.06.15.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Jun 2024 06:15:11 -0700 (PDT) Date: Fri, 21 Jun 2024 15:15:10 +0200 From: Andrew Jones To: Alexandre Ghiti Cc: Conor Dooley , Anup Patel , Conor Dooley , Yong-Xuan Wang , linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, kvm-riscv@lists.infradead.org, kvm@vger.kernel.org, greentime.hu@sifive.com, vincent.chen@sifive.com, Rob Herring , Krzysztof Kozlowski , Paul Walmsley , Palmer Dabbelt , Albert Ou , devicetree@vger.kernel.org Subject: Re: [PATCH v5 2/4] dt-bindings: riscv: Add Svade and Svadu Entries Message-ID: <20240621-9bf9365533a2f8f97cbf1f5e@orel> References: <20240605121512.32083-1-yongxuan.wang@sifive.com> <20240605121512.32083-3-yongxuan.wang@sifive.com> <20240605-atrium-neuron-c2512b34d3da@spud> <40a7d568-3855-48fb-a73c-339e1790f12f@ghiti.fr> <20240621-viewless-mural-f5992a247992@wendy> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Fri, Jun 21, 2024 at 02:42:15PM GMT, Alexandre Ghiti wrote: > > On 21/06/2024 12:17, Conor Dooley wrote: > > On Fri, Jun 21, 2024 at 10:37:21AM +0200, Alexandre Ghiti wrote: > > > On 20/06/2024 08:25, Anup Patel wrote: > > > > On Wed, Jun 5, 2024 at 10:25 PM Conor Dooley wrote: > > > > > On Wed, Jun 05, 2024 at 08:15:08PM +0800, Yong-Xuan Wang wrote: > > > > > > Add entries for the Svade and Svadu extensions to the riscv,isa-extensions > > > > > > property. > > > > > > > > > > > > Signed-off-by: Yong-Xuan Wang > > > > > > --- > > > > > > .../devicetree/bindings/riscv/extensions.yaml | 30 +++++++++++++++++++ > > > > > > 1 file changed, 30 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml > > > > > > index 468c646247aa..1e30988826b9 100644 > > > > > > --- a/Documentation/devicetree/bindings/riscv/extensions.yaml > > > > > > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml > > > > > > @@ -153,6 +153,36 @@ properties: > > > > > > ratified at commit 3f9ed34 ("Add ability to manually trigger > > > > > > workflow. (#2)") of riscv-time-compare. > > > > > > > > > > > > + - const: svade > > > > > > + description: | > > > > > > + The standard Svade supervisor-level extension for raising page-fault > > > > > > + exceptions when PTE A/D bits need be set as ratified in the 20240213 > > > > > > + version of the privileged ISA specification. > > > > > > + > > > > > > + Both Svade and Svadu extensions control the hardware behavior when > > > > > > + the PTE A/D bits need to be set. The default behavior for the four > > > > > > + possible combinations of these extensions in the device tree are: > > > > > > + 1. Neither svade nor svadu in DT: default to svade. > > > > > I think this needs to be expanded on, as to why nothing means svade. > > > > Actually if both Svade and Svadu are not present in DT then > > > > it is left to the platform and OpenSBI does nothing. > > > > > > > > > > + 2. Only svade in DT: use svade. > > > > > That's a statement of the obvious, right? > > > > > > > > > > > + 3. Only svadu in DT: use svadu. > > > > > This is not relevant for Svade. > > > > > > > > > > > + 4. Both svade and svadu in DT: default to svade (Linux can switch to > > > > > > + svadu once the SBI FWFT extension is available). > > > > > "The privilege level to which this devicetree has been provided can switch to > > > > > Svadu if the SBI FWFT extension is available". > > > > > > > > > > > + - const: svadu > > > > > > + description: | > > > > > > + The standard Svadu supervisor-level extension for hardware updating > > > > > > + of PTE A/D bits as ratified at commit c1abccf ("Merge pull request > > > > > > + #25 from ved-rivos/ratified") of riscv-svadu. > > > > > > + > > > > > > + Both Svade and Svadu extensions control the hardware behavior when > > > > > > + the PTE A/D bits need to be set. The default behavior for the four > > > > > > + possible combinations of these extensions in the device tree are: > > > > > @Anup/Drew/Alex, are we missing some wording in here about it only being > > > > > valid to have Svadu in isolation if the provider of the devicetree has > > > > > actually turned on Svadu? The binding says "the default behaviour", but > > > > > it is not the "default" behaviour, the behaviour is a must AFAICT. If > > > > > you set Svadu in isolation, you /must/ have turned it on. If you set > > > > > Svadu and Svade, you must have Svadu turned off? > > > > Yes, the wording should be more of requirement style using > > > > must or may. > > > > > > > > How about this ? > > > > 1) Both Svade and Svadu not present in DT => Supervisor may > > > > assume Svade to be present and enabled or it can discover > > > > based on mvendorid, marchid, and mimpid. > > > > 2) Only Svade present in DT => Supervisor must assume Svade > > > > to be always enabled. (Obvious) > > > > 3) Only Svadu present in DT => Supervisor must assume Svadu > > > > to be always enabled. (Obvious) > > > > > > I agree with all of that, but the problem is how can we guarantee that > > > openSBI actually enabled svadu? > > Conflation of an SBI implementation and OpenSBI aside, if the devicetree > > property is defined to mean that "the supervisor must assume svadu to be > > always enabled", then either it is, or the firmware's description of the > > hardware is broken and it's not the supervisor's problem any more. It's > > not the kernel's job to validate that the devicetree matches the > > hardware. > > > > > This is not the case for now. > > What "is not the case for now"? My understanding was that, at the > > moment, nothing happens with Svadu in OpenSBI. In turn, this means that > > there should be no devicetrees containing Svadu (per this binding's > > description) and therefore no problem? > > > What prevents a dtb to be passed with svadu to an old version of opensbi > which does not support the enablement of svadu? The svadu extension will end > up being present in the kernel but not enabled right? > I understand the concern; old SBI implementations will leave svadu in the DT but not actually enable it. Then, since svade may not be in the DT if the platform doesn't support it or it was left out on purpose, Linux will only see svadu and get unexpected exceptions. This is something we could force easily with QEMU and an SBI implementation which doesn't do anything for svadu. I hope vendors of real platforms, which typically provide their own firmware and DTs, would get this right, though, especially since Linux should fail fast in their testing when they get it wrong. Thanks, drew