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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AC95CC433ED for ; Thu, 13 May 2021 22:44:47 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 476CD613CB for ; Thu, 13 May 2021 22:44:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 476CD613CB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.127097.238787 (Exim 4.92) (envelope-from ) id 1lhK4I-00081Z-IQ; Thu, 13 May 2021 22:44:34 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 127097.238787; Thu, 13 May 2021 22:44:34 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lhK4I-00081S-FH; Thu, 13 May 2021 22:44:34 +0000 Received: by outflank-mailman (input) for mailman id 127097; Thu, 13 May 2021 22:44:33 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lhK4G-00081M-W1 for xen-devel@lists.xenproject.org; Thu, 13 May 2021 22:44:33 +0000 Received: from mail.kernel.org (unknown [198.145.29.99]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id b17e4acb-9970-4262-b398-cbdb75f85b4e; Thu, 13 May 2021 22:44:32 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 1D13C613CB; Thu, 13 May 2021 22:44:31 +0000 (UTC) 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: b17e4acb-9970-4262-b398-cbdb75f85b4e DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1620945871; bh=uusHZ6Hu18cGkfqehEHDwxB05WhNIrzTKdoOVktZntA=; h=Date:From:To:cc:Subject:In-Reply-To:References:From; b=Tl4cK54NmaePGQhluNWOJ8aKN1yjCZ7ovpnHW3XZL9r/i/N8FOs/P0PRlmRBnVBA1 OVfiDKdFJA9vlkvEL3VTjOYnDtWuep0Wgaus/MnA1LiXvZjrbHvAX8jMZX9hfS8e/5 4UA59IFCrW2AYNwXsmpUrf4nDKYpgJOquaDmkpP436doLBwSokIxfBjAuQWyTnrmwA 5jfcpYR2CO1gFgyL/QaxkIeFxNOG9Qy/qbo3ZGChe5jz4EazHXY5mV7DghyIVwgm10 TpeUfUeCPq1bqRxgxpp2vDS0KWTFGI0/zGLH00PRaSOsNfY6/kw69M9XvAiJgOmEs3 Z5jvOaSZ/Gvuw== Date: Thu, 13 May 2021 15:44:30 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-T480s To: Julien Grall cc: Stefano Stabellini , xen-devel@lists.xenproject.org, Wei.Chen@arm.com, Henry.Wang@arm.com, Penny.Zheng@arm.com, Bertrand.Marquis@arm.com, Julien Grall , Volodymyr Babchuk Subject: Re: [PATCH RFCv2 02/15] xen/arm: lpae: Use the generic helpers to defined the Xen PT helpers In-Reply-To: Message-ID: References: <20210425201318.15447-1-julien@xen.org> <20210425201318.15447-3-julien@xen.org> <94e364a7-de40-93ab-6cde-a2f493540439@xen.org> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Wed, 12 May 2021, Julien Grall wrote: > Hi Stefano, > > On 12/05/2021 22:30, Stefano Stabellini wrote: > > On Wed, 12 May 2021, Julien Grall wrote: > > > > > +#define LPAE_SHIFT LPAE_SHIFT_GS(PAGE_SHIFT) > > > > > +#define LPAE_ENTRIES LPAE_ENTRIES_GS(PAGE_SHIFT) > > > > > +#define LPAE_ENTRY_MASK LPAE_ENTRY_MASK_GS(PAGE_SHIFT) > > > > > > > > > > +#define LEVEL_SHIFT(lvl) LEVEL_SHIFT_GS(PAGE_SHIFT, lvl) > > > > > +#define LEVEL_ORDER(lvl) LEVEL_ORDER_GS(PAGE_SHIFT, lvl) > > > > > +#define LEVEL_SIZE(lvl) LEVEL_SIZE_GS(PAGE_SHIFT, lvl) > > > > > +#define LEVEL_MASK(lvl) (~(LEVEL_SIZE(lvl) - 1)) > > > > > > > > I would avoid adding these 4 macros. It would be OK if they were just > > > > used within this file but lpae.h is a header: they could end up be used > > > > anywhere in the xen/ code and they have a very generic name. My > > > > suggestion would be to skip them and just do: > > > > > > Those macros will be used in follow-up patches. They are pretty useful to > > > avoid introduce static array with the different information for each > > > level. > > > > > > Would prefix them with XEN_ be better? > > > > Maybe. The concern I have is that there are multiple page granularities > > (4kb, 16kb, etc) and multiple page sizes (4kb, 2mb, etc). If I just see > > LEVEL_ORDER it is not immediately obvious what granularity and what size > > we are talking about. > > I am a bit puzzled with your answer. AFAIU, you are happy with the existing > macros (THIRD_*, SECOND_*) but not with the new macros. > > In reality, there is no difference because THIRD_* doesn't tell you the exact > size but only "this is a level 3 mapping". > > So can you clarify what you are after? IOW is it reworking the current naming > scheme? You are right -- there is no real difference between THIRD_*, SECOND_* and LEVEL_*. The original reason for my comments is that I hadn't read the following patches, and the definition of LEVEL_* macros is simple, they could be open coded. It looked like they were only going to be used to make the definition of THIRD_*, SECOND_* a bit easier. So, at first, I was wondering if they were needed at all. Secondly, I realized that they were going to be used in *.c files by other patches. That's why they are there. But I started thinking whether we should find a way to make it a bit clearer that they are for Xen pages, currently at 4KB granularity. THIRD_*, SECOND_*, etc. are already generic names which don't convey the granularity or whether they are Xen pages at all. But LEVEL_* seem even more generic. As I mentioned, I don't have any good suggestions for changes to make here, so unless you can come up with a good idea let's keep it as is.