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 lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7A9D5C02188 for ; Mon, 27 Jan 2025 17:41:40 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.878168.1288342 (Exim 4.92) (envelope-from ) id 1tcT6s-00014y-7M; Mon, 27 Jan 2025 17:41:18 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 878168.1288342; Mon, 27 Jan 2025 17:41:18 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1tcT6s-00014r-4f; Mon, 27 Jan 2025 17:41:18 +0000 Received: by outflank-mailman (input) for mailman id 878168; Mon, 27 Jan 2025 17:41:16 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1tcT6q-00010f-9X for xen-devel@lists.xenproject.org; Mon, 27 Jan 2025 17:41:16 +0000 Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [2a00:1450:4864:20::12c]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id e2e2afdf-dcd5-11ef-99a4-01e77a169b0f; Mon, 27 Jan 2025 18:41:06 +0100 (CET) Received: by mail-lf1-x12c.google.com with SMTP id 2adb3069b0e04-54024aa9febso5023380e87.1 for ; Mon, 27 Jan 2025 09:41:06 -0800 (PST) Received: from [192.168.219.191] ([94.75.70.14]) by smtp.gmail.com with ESMTPSA id 2adb3069b0e04-543c83684basm1340780e87.109.2025.01.27.09.41.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 27 Jan 2025 09:41:05 -0800 (PST) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: e2e2afdf-dcd5-11ef-99a4-01e77a169b0f DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1737999666; x=1738604466; darn=lists.xenproject.org; h=in-reply-to:content-language:references:cc:to:from:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=zgGt0UWgv/Np04JsWjifwQUIQ9PfPuCBVZ7oSviphrw=; b=b9cx0n76VFihzdQeon3OavbZ/KiYSqghvshIzJqYhL4IsLm2xcyj10nvBhc7hunIWd MUbevs93YZvCxCJkSk82zkoMMv8X1l6cu52XBFdp9MuVqi8GVaGi/S3QA+9r8Yq8/z4A fC9t2/l/eaPIx6nzT+fygQwNO5GWytR9jm9GBW943a7JxJcnaWKtUbJbg7jX71GYTi8Q v1UiV8Fc6whyFtTlzLyYZxSM2WPjIWQX9/LEGrK7o+ufyZenw7XhdQv3XIURtewh8/Ql 7HTXY/C0yJpqnu4c945RFtH8KB5T0/bPfur/hs36rWaSYxBZDJYLbasFT2gRs5yl+pwA kuew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737999666; x=1738604466; h=in-reply-to:content-language:references:cc:to:from:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=zgGt0UWgv/Np04JsWjifwQUIQ9PfPuCBVZ7oSviphrw=; b=Z2IZf7FWS7EGH8WMXcCz9Ww4hjzyJQVrd6KjhEiHik4j+tKKi7/WNv9n3mkvla0AZj LOXo2DgXxZnffhX/rZPefnNF17S2wSwtUYJ/0oMa4futKkxAes1utVh68kBrpVxWj/c4 G1SaY/UxoSBuYeEqdef2Wyvm+Y4ubdi37BWlnNevGCuX+bnSyJRKFrX7O5mj3eHXzljs 0MS0K2yClPFcv6E62nBH35/XqmHVDnxrlV47GRIghGPxESRNKRKPeSIMvLeyeHbBY1na kEsu+wW9NJKko7VUtpE7WMm+5+ANxOcskqfH/UJFcFbiVKtIDpeIHmhj0qlphbdJeKgJ 0Oww== X-Forwarded-Encrypted: i=1; AJvYcCVdefueCi8IFdH5BNRKVQYhhXOfRY9vdi8T38A56yvAv2qZ8uI4wyA/PutnTB8+jJxsF+rIYZgIp5Y=@lists.xenproject.org X-Gm-Message-State: AOJu0YyxDD7dEBU7TvVwKZJYKRh43HempunCL7+7uT6WYR9qfXnCATop P5si/ERBmbG6kIxBJ1e3xXMVvSOE2/H+HvoGptDEV/GKe319W0Zm X-Gm-Gg: ASbGncv6xTMH57II+iT7BWW5GuL7JjRI+WU0VORwruzBj4LIlwTmq9pscNKNqnKVCbL 2Gvqq5Ag0ROvEhSqnv1qsb1BzpwcXfLCnECqylQNOWD/rxYlIsDygAuvIAqYb3SagueVewirZ/T aKhbr7CmYOf5CSuBvpPJ1YN2FbvKDQdeeAO148jCsvqRohzSST9Eps60bOqCwPxCiSAOoIrdTuW LTfYdS8QscTsFyeQYkpw6+Rd33ngS27By7Y4iFVT+L8PkTkSImbHYw6V1YgZcSlI+ucnpqAL/SZ kF7lcDO3bq0qsk3/sw== X-Google-Smtp-Source: AGHT+IF0YCjbqvn/5iYJcQNS/PYdLHDRoSCxQuoIvLYVpZYcJCymOz2g7JCsqJHPMsqUvwte4/Nf4w== X-Received: by 2002:ac2:4acb:0:b0:540:1a0c:9ba6 with SMTP id 2adb3069b0e04-5439c282d2bmr12671485e87.34.1737999665681; Mon, 27 Jan 2025 09:41:05 -0800 (PST) Content-Type: multipart/alternative; boundary="------------NR3E6JH0lAFy9RT57IGkjVGJ" Message-ID: <67446e8f-71d4-480e-8566-1a464b6f6639@gmail.com> Date: Mon, 27 Jan 2025 18:41:04 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 1/3] xen/riscv: implement software page table walking From: Oleksii Kurochko To: Jan Beulich Cc: Alistair Francis , Bob Eshleman , Connor Davis , Andrew Cooper , Anthony PERARD , Michal Orzel , Julien Grall , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= , Stefano Stabellini , xen-devel@lists.xenproject.org References: <00dfc71569bc9971b53e29b36a80e9e020ac61ac.1737391102.git.oleksii.kurochko@gmail.com> <21bfd2f5-74b8-409e-956c-dd736a3c0be2@suse.com> Content-Language: en-US In-Reply-To: This is a multi-part message in MIME format. --------------NR3E6JH0lAFy9RT57IGkjVGJ Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 1/27/25 6:22 PM, Oleksii Kurochko wrote: > > > On 1/27/25 1:57 PM, Jan Beulich wrote: >> On 27.01.2025 13:29, Oleksii Kurochko wrote: >>> On 1/27/25 11:06 AM, Jan Beulich wrote: >>>> On 20.01.2025 17:54, Oleksii Kurochko wrote: >>>>> RISC-V doesn't have hardware feature to ask MMU to translate >>>>> virtual address to physical address ( like Arm has, for example ), >>>>> so software page table walking in implemented. >>>>> >>>>> Signed-off-by: Oleksii Kurochko >>>>> --- >>>>> xen/arch/riscv/include/asm/mm.h | 2 ++ >>>>> xen/arch/riscv/pt.c | 56 +++++++++++++++++++++++++++++++++ >>>>> 2 files changed, 58 insertions(+) >>>>> >>>>> diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h >>>>> index 292aa48fc1..d46018c132 100644 >>>>> --- a/xen/arch/riscv/include/asm/mm.h >>>>> +++ b/xen/arch/riscv/include/asm/mm.h >>>>> @@ -15,6 +15,8 @@ >>>>> >>>>> extern vaddr_t directmap_virt_start; >>>>> >>>>> +paddr_t pt_walk(vaddr_t va); >>>> In the longer run, is returning just the PA really going to be sufficient? >>>> If not, perhaps say a word on the limitation in the description. >>> In the long run, this function's prototype looks like|paddr_t pt_walk(vaddr_t root, vaddr_t va, bool is_xen)| [1]. However, I'm not sure if it will stay that way, >>> as I think|is_xen| could be skipped, since using|map_table()| should be sufficient (as it now considers|system_state|) and I'm not really sure if I need root argument >>> as initial goal was to use this function for debug only purposes and I've never used it for guest page table (stage-1) walking. >>> Anyway, yes, it is still returning a physical address, and that seems enough to me. >>> >>> Could you share your thoughts on what I should take into account for returning value, probably, I am missing something really useful? >> Often you care about the permissions as well. Sometimes it may even be relevant >> to know the (super-)page size of the mapping. > Perhaps it would be better to change the prototype to: > bool pt_walk(vaddr_t va, mfn_t *ret_pa); > or even > void pt_walk(vaddr_t va, mfn_t *ret_pa); > In this case,|ret_pa = INVALID_MFN| could serve as a signal that|pt_walk()| failed. > If there's a need to return permissions or (super-)page size in the future, another argument could be added. > What do you think? Would this approach be better? We have to return mfn_t or paddr_t as pt_walk() is used invmap_to_mfn(). ~ Oleksii > > I am also considering returning a structure containing the|mfn| (or|paddr_t|) and adding other properties (such as permissions or > page size) as needed in the future. Both solutions seem more or less equivalent. > > ~ Oleksii --------------NR3E6JH0lAFy9RT57IGkjVGJ Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit


On 1/27/25 6:22 PM, Oleksii Kurochko wrote:


On 1/27/25 1:57 PM, Jan Beulich wrote:
On 27.01.2025 13:29, Oleksii Kurochko wrote:

On 1/27/25 11:06 AM, Jan Beulich wrote:

On 20.01.2025 17:54, Oleksii Kurochko wrote:

RISC-V doesn't have hardware feature to ask MMU to translate
virtual address to physical address ( like Arm has, for example ),
so software page table walking in implemented.

Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
---
  xen/arch/riscv/include/asm/mm.h |  2 ++
  xen/arch/riscv/pt.c             | 56 +++++++++++++++++++++++++++++++++
  2 files changed, 58 insertions(+)

diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 292aa48fc1..d46018c132 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -15,6 +15,8 @@
  
  extern vaddr_t directmap_virt_start;
  
+paddr_t pt_walk(vaddr_t va);

In the longer run, is returning just the PA really going to be sufficient?
If not, perhaps say a word on the limitation in the description.

In the long run, this function's prototype looks like|paddr_t pt_walk(vaddr_t root, vaddr_t va, bool is_xen)| [1]. However, I'm not sure if it will stay that way,
as I think|is_xen| could be skipped, since using|map_table()| should be sufficient (as it now considers|system_state|) and I'm not really sure if I need root argument
as initial goal was to use this function for debug only purposes and I've never used it for guest page table (stage-1) walking.
Anyway, yes, it is still returning a physical address, and that seems enough to me.

Could you share your thoughts on what I should take into account for returning value, probably, I am missing something really useful?

Often you care about the permissions as well. Sometimes it may even be relevant
to know the (super-)page size of the mapping.
Perhaps it would be better to change the prototype to:
  bool pt_walk(vaddr_t va, mfn_t *ret_pa);
or even
  void pt_walk(vaddr_t va, mfn_t *ret_pa);
  In this case, ret_pa = INVALID_MFN could serve as a signal that pt_walk() failed.
If there's a need to return permissions or (super-)page size in the future, another argument could be added.
What do you think? Would this approach be better?
We have to return mfn_t or paddr_t as pt_walk() is used in vmap_to_mfn().


~ Oleksii


I am also considering returning a structure containing the mfn (or paddr_t) and adding other properties (such as permissions or
page size) as needed in the future. Both solutions seem more or less equivalent.

~ Oleksii
--------------NR3E6JH0lAFy9RT57IGkjVGJ--