[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/common: Do not allocate magic pages 1:1 for direct mapped domains


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Henry Wang <xin.wang2@xxxxxxx>
  • Date: Tue, 27 Feb 2024 22:21:47 +0800
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=suse.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=50gZi4HX0UroOWiE1CHbPHFkXdBR0q1Po8JppdrMdfo=; b=jL/STKQNEeO4313iUcvOkaT4gfTnuPvCoeEYEjn/4udOXlQLd2X2ukoPFMfxL2QaEIXCmbjlzr25RNpWC1fGjCl74h3OiVAfYnuyUBNpFojeWJmlVCXwBQepLksgbmEOADRI+e/ie02qn+Mv6rHkvKg6xX0PXG8Zc6J8kbFtduu9H7dCtMFk6pIHjqJ7RNuCpndhlcIHXL686GsBX0k8G1L3kwoTriJP1TU13flC5Jm7O0sXY5GJ4Uw53LnrytQ1MisPdGjrCU8hxZEAhjDs1Sh7Iwr3vYDD+XmL5EriZ1htoIIRusy7HvJ4pcahQCrTlnblMwX/piUtM7cLxoXtNQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MlP+jSREOZJzhnUqe2YTwttopRWtFs+KAKPPMZoUSoaYVqRtRdGExR/FOfjAlmjkZMh7/qQ+5GCAbmjJGvd1BIldGzZaWkDraUuHz0XVv8zeBoE6A9evzYvvJCSibYFaXoyNXR8bfgww8kmpSKFgD3k2h1zsGVO/WSzoeh+2CJAUP3B1V8KPWBbaDAGVLD3l9iDdwo/Jekl/xvEgLB1oIzIn/DTzouiuLXi3z+rd7Z3uwJlftRcX8DQuEvpP9PWNl1YezUYpWOfXxcXvQmQycr6taKmPEo9jsNyVjVyCMgkDuuWcRj7bXqedc/AX68elPpWmAth6rFpGdhLa92lcJw==
  • Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Alec Kwapis <alec.kwapis@xxxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 27 Feb 2024 14:22:13 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Jan,

On 2/27/2024 9:51 PM, Jan Beulich wrote:
On 27.02.2024 14:35, Henry Wang wrote:
Hi Jan,

On 2/27/2024 9:27 PM, Jan Beulich wrote:
On 27.02.2024 14:24, Henry Wang wrote:
On 2/26/2024 4:25 PM, Jan Beulich wrote:
On 26.02.2024 02:19, Henry Wang wrote:
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -219,7 +219,7 @@ static void populate_physmap(struct memop_args *a)
            }
            else
            {
-            if ( is_domain_direct_mapped(d) )
+            if ( is_domain_direct_mapped(d) && !is_magic_gpfn(gpfn) )
                {
                    mfn = _mfn(gpfn);
I wonder whether is_domain_direct_mapped() shouldn't either be cloned
into e.g. is_gfn_direct_mapped(d, gfn), or be adjusted in-place to gain
such a (then optional) 2nd parameter. (Of course there again shouldn't be
a need for every domain to define a stub is_domain_direct_mapped() - if
not defined by an arch header, the stub can be supplied in a single
central place.)
Same here, it looks like you prefer the centralized
is_domain_direct_mapped() now, as we are having more archs. I can do the
clean-up when sending v2. Just out of curiosity, do you think it is a
good practice to place the is_domain_direct_mapped() implementation in
xen/domain.h with proper arch #ifdefs?
arch #ifdefs? I'd like to avoid such, if at all possible. Generic fallbacks
generally ought to key their conditionals to the very identifier not
(already) being defined.
I meant something like this (as I saw CDF_directmap is currently gated
in this way in xen/domain.h):

#ifdef CONFIG_ARM
#define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
#else
#define is_domain_direct_mapped(d) ((void)(d), 0)
#endif

I am having trouble to think of another way to keep the function in a
centralized place while
avoiding the #ifdefs. Would you mind elaborating a bit? Thanks!
What is already there is fine to change. I took your earlier reply to
mean that you want to add an "#ifndef CONFIG_ARM" to put in place some
new fallback handler.

Of course the above could also be done without any direct CONFIG_ARM
dependency. For example, CDF_directmap could simply evaluate to zero
when direct mapped memory isn't supported.

Yes correct, that will indeed simplify the code a lot. I can do the change accordingly in follow-up versions.

Kind regards,
Henry

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.