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 92F62C54EBE for ; Wed, 18 Jan 2023 09:50:34 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.480307.744623 (Exim 4.92) (envelope-from ) id 1pI55B-0000uX-5u; Wed, 18 Jan 2023 09:50:13 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 480307.744623; Wed, 18 Jan 2023 09:50:13 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1pI55B-0000uQ-3F; Wed, 18 Jan 2023 09:50:13 +0000 Received: by outflank-mailman (input) for mailman id 480307; Wed, 18 Jan 2023 09:50:12 +0000 Received: from mail.xenproject.org ([104.130.215.37]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1pI55A-0000uK-02 for xen-devel@lists.xenproject.org; Wed, 18 Jan 2023 09:50:12 +0000 Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1pI559-0004bj-KT; Wed, 18 Jan 2023 09:50:11 +0000 Received: from 54-240-197-239.amazon.com ([54.240.197.239] helo=[192.168.8.239]) by xenbits.xenproject.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1pI559-000571-Ef; Wed, 18 Jan 2023 09:50:11 +0000 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" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xen.org; s=20200302mail; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID; bh=zAiykU+TtfSsKJ9F9GCiG9v0SHaPn3kOU6nFLCYsF+c=; b=dJHHF7ZslBzm9WOFp8SwCCCmQQ KK0s8cfPWoB4IA0Y/Y2o9L9UySDDSifIy3MO19SZHNJGj+abzxyFmNGL/B533JScqN1NMhWWKHqS0 Rr/yFC7ygaSgq2qJwOFGnAs2ctw+sn2NyWPCxPgcdhl9YOpaIcH/WtEP5sFiktGc5TRw=; Message-ID: <4e6d4deb-d38b-9845-2f58-e94f28196bf6@xen.org> Date: Wed, 18 Jan 2023 09:50:09 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 05/40] xen/arm64: prepare for moving MMU related code from head.S Content-Language: en-US To: Wei Chen , Penny Zheng , "xen-devel@lists.xenproject.org" Cc: Stefano Stabellini , Bertrand Marquis , Volodymyr Babchuk References: <20230113052914.3845596-1-Penny.Zheng@arm.com> <20230113052914.3845596-6-Penny.Zheng@arm.com> From: Julien Grall In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 18/01/2023 03:09, Wei Chen wrote: > Hi Julien, > >> -----Original Message----- >> From: Julien Grall >> Sent: 2023年1月18日 7:37 >> To: Penny Zheng ; xen-devel@lists.xenproject.org >> Cc: Wei Chen ; Stefano Stabellini >> ; Bertrand Marquis ; >> Volodymyr Babchuk >> Subject: Re: [PATCH v2 05/40] xen/arm64: prepare for moving MMU related >> code from head.S >> >> Hi Penny, >> >> On 13/01/2023 05:28, Penny Zheng wrote: >>> From: Wei Chen >>> >>> We want to reuse head.S for MPU systems, but there are some >>> code implemented for MMU systems only. We will move such >>> code to another MMU specific file. But before that, we will >>> do some preparations in this patch to make them easier >>> for reviewing: >> >> Well, I agree that... >> >>> 1. Fix the indentations of code comments. >> >> ... changing the indentation is better here. But... >> >>> 2. Export some symbols that will be accessed out of file >>> scope. >> >> ... I have no idea which functions are going to be used in a separate >> file. So I think they should belong to the patch moving the code. >> > > Ok, I will move these changes to the moving code patches. > >>> >>> Signed-off-by: Wei Chen >> >> Your signed-off-by is missing. >> >>> --- >>> v1 -> v2: >>> 1. New patch. >>> --- >>> xen/arch/arm/arm64/head.S | 40 +++++++++++++++++++-------------------- >>> 1 file changed, 20 insertions(+), 20 deletions(-) >>> >>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S >>> index 93f9b0b9d5..b2214bc5e3 100644 >>> --- a/xen/arch/arm/arm64/head.S >>> +++ b/xen/arch/arm/arm64/head.S >>> @@ -136,22 +136,22 @@ >>> add \xb, \xb, x20 >>> .endm >>> >>> - .section .text.header, "ax", %progbits >>> - /*.aarch64*/ >>> +.section .text.header, "ax", %progbits >>> +/*.aarch64*/ >> >> This change is not mentioned. >> > > I will add the description in commit message. > >>> >>> - /* >>> - * Kernel startup entry point. >>> - * --------------------------- >>> - * >>> - * The requirements are: >>> - * MMU = off, D-cache = off, I-cache = on or off, >>> - * x0 = physical address to the FDT blob. >>> - * >>> - * This must be the very first address in the loaded image. >>> - * It should be linked at XEN_VIRT_START, and loaded at any >>> - * 4K-aligned address. All of text+data+bss must fit in 2MB, >>> - * or the initial pagetable code below will need adjustment. >>> - */ >>> +/* >>> + * Kernel startup entry point. >>> + * --------------------------- >>> + * >>> + * The requirements are: >>> + * MMU = off, D-cache = off, I-cache = on or off, >>> + * x0 = physical address to the FDT blob. >>> + * >>> + * This must be the very first address in the loaded image. >>> + * It should be linked at XEN_VIRT_START, and loaded at any >>> + * 4K-aligned address. All of text+data+bss must fit in 2MB, >>> + * or the initial pagetable code below will need adjustment. >>> + */ >>> >>> GLOBAL(start) >>> /* >>> @@ -586,7 +586,7 @@ ENDPROC(cpu_init) >>> * >>> * Clobbers x0 - x4 >>> */ >>> -create_page_tables: >>> +ENTRY(create_page_tables) >> >> I am not sure about keeping this name. Now we have create_page_tables() >> and arch_setup_page_tables(). >> >> I would conside to name it create_boot_page_tables(). >> > > Do you need me to rename it in this patch? So looking at the rest of the series, I see you are already renaming the helper in patch #11. I think it would be better if the naming is done earlier. That said, I am not convinced that create_page_tables() should actually be called externally. In fact, you have something like: bl create_page_tables bl enable_mmu Both will need a MMU/MPU specific implementation. So it would be better if we provide a wrapper to limit the number of external functions. Cheers, -- Julien Grall