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 410B5C54788 for ; Thu, 22 Feb 2024 14:43:38 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.684455.1064311 (Exim 4.92) (envelope-from ) id 1rdAIH-0008Tb-OU; Thu, 22 Feb 2024 14:43:25 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 684455.1064311; Thu, 22 Feb 2024 14:43:25 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1rdAIH-0008TU-Lb; Thu, 22 Feb 2024 14:43:25 +0000 Received: by outflank-mailman (input) for mailman id 684455; Thu, 22 Feb 2024 14:43:24 +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 1rdAIG-0008TO-Fi for xen-devel@lists.xenproject.org; Thu, 22 Feb 2024 14:43:24 +0000 Received: from support.bugseng.com (mail.bugseng.com [162.55.131.47]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id b9f4828c-d190-11ee-98f5-efadbce2ee36; Thu, 22 Feb 2024 15:43:22 +0100 (CET) Received: from support.bugseng.com (support.bugseng.com [162.55.131.47]) by support.bugseng.com (Postfix) with ESMTPA id 849064EE0737; Thu, 22 Feb 2024 15:43:21 +0100 (CET) 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: b9f4828c-d190-11ee-98f5-efadbce2ee36 MIME-Version: 1.0 Date: Thu, 22 Feb 2024 15:43:21 +0100 From: Nicola Vetrini To: Jan Beulich Cc: sstabellini@kernel.org, michal.orzel@amd.com, xenia.ragiadakou@amd.com, ayan.kumar.halder@amd.com, consulting@bugseng.com, roger.pau@citrix.com, bertrand.marquis@arm.com, julien@xen.org, Volodymyr Babchuk , George Dunlap , Wei Liu , xen-devel@lists.xenproject.org, Andrew Cooper Subject: Re: [XEN PATCH] xen: cache clearing and invalidation helpers refactoring In-Reply-To: <4b121e48-9541-4b53-8352-939c904f4f1c@suse.com> References: <45509cb67ecee3f690b5784489b5ccb4@bugseng.com> <1743b4248d30a4e8b68a150c25724caa@bugseng.com> <2ff52df443fc080875fd05614d89764d@bugseng.com> <4b121e48-9541-4b53-8352-939c904f4f1c@suse.com> Message-ID: X-Sender: nicola.vetrini@bugseng.com Organization: BUGSENG s.r.l. Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit >>>> >>>>>> --- a/xen/arch/arm/include/asm/page.h >>>>>> +++ b/xen/arch/arm/include/asm/page.h >>>>>> @@ -123,6 +123,7 @@ >>>>>> >>>>>> #ifndef __ASSEMBLY__ >>>>>> >>>>>> +#include >>>>> >>>>> This is a no-go, imo (also on x86): Adding this include here >>>>> effectively >>>>> means that nearly every CU will have a dependency on that header, >>>>> no >>>>> matter that most are entirely agnostic of grants. Each arch has a >>>>> grant_table.h - is there any reason the new, grant-specific helper >>>>> can't >>>>> be put there? >>>>> >>>> >>>> I would have to test, but I think that can be done >>>> >>> >>> The only blocker so far is that this triggers a build error due to a >>> circular dependency between xen/mm.h and asm/flushtlb.h on x86. Also >>> found some earlier evidence [1] that there are some oddities around >>> asm/flushtlb's inclusion. >>> >>> [1] >>> https://lore.kernel.org/xen-devel/20200318210540.5602-1-andrew.cooper3@citrix.com/ >> >> There could be a way of untangling asm/flushtlb.h from xen/mm.h, by >> moving "accumulate_tlbflush" and "filtered_flush_tlb_mask" introduced >> by >> commit 80943aa40e30 ("replace tlbflush check and operation with inline >> functions") [1]. >> However, these function should then be part of a generic >> xen/flushtlb.h >> header, since they are used in common code (e.g., common/page_alloc) >> and >> a bunch of common code source files should move their includes (see >> [2] >> for a partial non-working patch). Do you feel that this is a feasible >> route? > > Yeah, introducing xen/flushtlb.h to hold these sounds pretty sensible. > There is some shuffling of includes to be done to get it to build, which I'm still trying to address. Most problems are down to the use of struct page_info in page_set_tlbflush_timestamp in x86/.*/asm/flushtlb.h which then prompts the inclusion asm/mm.h probably. >> In passing it should be noted that the header ordering in >> x86/alternative.c is not the one usually prescribed, so that may be >> taken care of as well. > > I'm afraid I don't understand this remark. > I just meant to say that this #include #include #include #include #include #include #include #include #include #include #include #include #include is not the usual order of xen/*.h then asm/*.h and there is no comment justifying that ordering. So in the process of including asm/flushtlb.h here the inclusion order can be tidied up (or also indipendently), unless there is some reason I'm missing that disallows it. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)