From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Attilio Rao <attilio.rao@citrix.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>,
Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org, x86@kernel.org,
xen-devel@lists.xensource.com
Subject: Re: [PATCH 0/5] X86/XEN: Merge x86_init.paging.pagetable_setup_start and x86_init.paging.pagetable_setup_done PVOPS and document the semantic
Date: Tue, 21 Aug 2012 10:53:07 -0400 [thread overview]
Message-ID: <20120821145307.GE20289@phenom.dumpdata.com> (raw)
In-Reply-To: <1345511646-12427-1-git-send-email-attilio.rao@citrix.com>
On Tue, Aug 21, 2012 at 02:14:01AM +0100, Attilio Rao wrote:
> Currently the definition of x86_init.paging.pagetable_setup_start and
> x86_init.paging.pagetable_setup_done is twisted and not really well
> defined (in terms of prototypes desired). More specifically:
> pagetable_setup_start:
> * it is a nop on x86_32
> * it is a nop for the XEN case
> * cleans up the boot time page table in the x86_64 case
Is it safe to call that 'boot time page table' in Xen case?
Since that is what it would be doing? Did you test it with dom0 and
PV guest and with 2GB, 3GB, 4GB, and 8GB layouts? I think those were
the ones that caught earlier mistakes.
>
> pagetable_setup_done:
> * it is a nop on x86_32
> * sets up accessor functions for pagetable manipulation, for the
> XEN case
> * it is a nop on x86_64
>
> Most of this logic can be skipped by creating a new PVOPS that can handle
> pagetable setup and pre/post operations on it.
> The new PVOPS must be called only once, during boot-time setup and
> after the direct mapping for physical memory is available.
Looks like you are missing the other crucial bit of information:
It removes two of the pvops and replaces them with just one.
>
> Attilio Rao (5):
> XEN: Remove the base argument from
> x86_init.paging.pagetable_setup_done PVOPS
> XEN: Remove the base argument from
> x86_init.paging.pagetable_setup_start PVOPS
> X86/XEN: Introduce the x86_init.paging.pagetable_init() PVOPS
> X86/XEN: Retire now unused x86_init.paging.pagetable_setup_start and
> x86_init.paging.pagetable_setup_done PVOPS
> X86/XEN: Add few lines explaining simple semantic for
> x86_init.paging.pagetable_init PVOPS
>
> arch/x86/include/asm/pgtable_types.h | 6 ++----
> arch/x86/include/asm/x86_init.h | 11 +++++++----
> arch/x86/kernel/setup.c | 4 +---
> arch/x86/kernel/x86_init.c | 4 +---
> arch/x86/mm/init_32.c | 12 ++++++------
> arch/x86/xen/mmu.c | 18 +++++++-----------
> 6 files changed, 24 insertions(+), 31 deletions(-)
>
> --
> 1.7.2.5
next prev parent reply other threads:[~2012-08-21 15:06 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-21 1:14 [PATCH 0/5] X86/XEN: Merge x86_init.paging.pagetable_setup_start and x86_init.paging.pagetable_setup_done PVOPS and document the semantic Attilio Rao
2012-08-21 1:14 ` [PATCH 1/5] XEN: Remove the base argument from x86_init.paging.pagetable_setup_done PVOPS Attilio Rao
2012-08-21 1:14 ` [PATCH 2/5] XEN: Remove the base argument from x86_init.paging.pagetable_setup_start PVOPS Attilio Rao
2012-08-21 15:41 ` Thomas Gleixner
2012-08-21 15:49 ` Attilio Rao
2012-08-21 16:04 ` Thomas Gleixner
2012-08-21 1:14 ` [PATCH 3/5] X86/XEN: Introduce the x86_init.paging.pagetable_init PVOPS Attilio Rao
2012-08-21 15:44 ` Thomas Gleixner
2012-08-21 20:26 ` Attilio Rao
2012-08-21 1:14 ` [PATCH 4/5] X86/XEN: Retire now unused x86_init.paging.pagetable_setup_start and x86_init.paging.pagetable_setup_done PVOPS Attilio Rao
2012-08-21 1:14 ` [PATCH 5/5] X86/XEN: Add few lines explaining simple semantic for x86_init.paging.pagetable_init PVOPS Attilio Rao
2012-08-21 11:09 ` [PATCH 0/5] X86/XEN: Merge x86_init.paging.pagetable_setup_start and x86_init.paging.pagetable_setup_done PVOPS and document the semantic Stefano Stabellini
2012-08-21 14:53 ` Konrad Rzeszutek Wilk [this message]
2012-08-21 15:22 ` Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120821145307.GE20289@phenom.dumpdata.com \
--to=konrad.wilk@oracle.com \
--cc=Ian.Campbell@citrix.com \
--cc=Stefano.Stabellini@eu.citrix.com \
--cc=attilio.rao@citrix.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox