All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Vrabel <david.vrabel@citrix.com>
To: Juergen Gross <jgross@suse.com>,
	David Vrabel <david.vrabel@citrix.com>,
	<linux-kernel@vger.kernel.org>, <xen-devel@lists.xensource.com>,
	<konrad.wilk@oracle.com>, <boris.ostrovsky@oracle.com>
Subject: Re: [Xen-devel] [PATCH] xen: switch to post-init routines in xen mmu.c earlier
Date: Thu, 11 Dec 2014 12:15:21 +0000	[thread overview]
Message-ID: <54898AD9.2030009@citrix.com> (raw)
In-Reply-To: <54892C58.5070707@suse.com>

On 11/12/14 05:32, Juergen Gross wrote:
> On 12/10/2014 07:07 PM, David Vrabel wrote:
>> On 10/12/14 15:56, Juergen Gross wrote:
>>> With the virtual mapped linear p2m list the post-init mmu operations
>>> must be used for setting up the p2m mappings, as in case of
>>> CONFIG_FLATMEM the init routines may trigger BUGs.
>>>
>>> Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>>   arch/x86/xen/mmu.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
>>> index 6ab6150..a1a429a 100644
>>> --- a/arch/x86/xen/mmu.c
>>> +++ b/arch/x86/xen/mmu.c
>>> @@ -1225,6 +1225,7 @@ static void __init xen_pagetable_p2m_setup(void)
>>>   static void __init xen_pagetable_init(void)
>>>   {
>>>       paging_init();
>>> +    xen_post_allocator_init();
>>>
>>>       xen_pagetable_p2m_setup();
>>>
>>
>> This feels very chicken-and-egg to me:  To setup the P2M we need to use
>> the MMU ops that use the P2M...
>>
>> Please explain very clearly why this is all safe.
> 
> Okay. paging_init() sets up all infrastructure needed to switch to the
> post-init mmu ops done by xen_post_allocator_init(). With the virtual
> mapped linear p2m list we need some mmu ops during setup of this list,
> so we have to switch to the correct mmu ops as soon as possible.
> 
> The p2m list is usable from the beginning, just expansion requires to
> have established the new linear mapping. So the call of
> xen_remap_memory() had to be introduced, but this is not due to the
> mmu ops requiring this.
> 
> Summing it up: calling xen_post_allocator_init() not directly after
> paging_init() was conceptually wrong in the beginning, it just didn't
> matter up to now as no functions used between the two calls needed
> some critical mmu ops (e.g. alloc_pte). This has changed now, so I
> corrected it.

I've added this to the commit message and applied to
devel/for-linus-3.19.  If the tests pass I will consider sending a
further pull request for 3.19 including the linear p2m changes early
next week.

Thanks.

David

WARNING: multiple messages have this Message-ID (diff)
From: David Vrabel <david.vrabel@citrix.com>
To: Juergen Gross <jgross@suse.com>,
	David Vrabel <david.vrabel@citrix.com>,
	linux-kernel@vger.kernel.org, xen-devel@lists.xensource.com,
	konrad.wilk@oracle.com, boris.ostrovsky@oracle.com
Subject: Re: [Xen-devel] [PATCH] xen: switch to post-init routines in xen mmu.c earlier
Date: Thu, 11 Dec 2014 12:15:21 +0000	[thread overview]
Message-ID: <54898AD9.2030009@citrix.com> (raw)
In-Reply-To: <54892C58.5070707@suse.com>

On 11/12/14 05:32, Juergen Gross wrote:
> On 12/10/2014 07:07 PM, David Vrabel wrote:
>> On 10/12/14 15:56, Juergen Gross wrote:
>>> With the virtual mapped linear p2m list the post-init mmu operations
>>> must be used for setting up the p2m mappings, as in case of
>>> CONFIG_FLATMEM the init routines may trigger BUGs.
>>>
>>> Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>>   arch/x86/xen/mmu.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
>>> index 6ab6150..a1a429a 100644
>>> --- a/arch/x86/xen/mmu.c
>>> +++ b/arch/x86/xen/mmu.c
>>> @@ -1225,6 +1225,7 @@ static void __init xen_pagetable_p2m_setup(void)
>>>   static void __init xen_pagetable_init(void)
>>>   {
>>>       paging_init();
>>> +    xen_post_allocator_init();
>>>
>>>       xen_pagetable_p2m_setup();
>>>
>>
>> This feels very chicken-and-egg to me:  To setup the P2M we need to use
>> the MMU ops that use the P2M...
>>
>> Please explain very clearly why this is all safe.
> 
> Okay. paging_init() sets up all infrastructure needed to switch to the
> post-init mmu ops done by xen_post_allocator_init(). With the virtual
> mapped linear p2m list we need some mmu ops during setup of this list,
> so we have to switch to the correct mmu ops as soon as possible.
> 
> The p2m list is usable from the beginning, just expansion requires to
> have established the new linear mapping. So the call of
> xen_remap_memory() had to be introduced, but this is not due to the
> mmu ops requiring this.
> 
> Summing it up: calling xen_post_allocator_init() not directly after
> paging_init() was conceptually wrong in the beginning, it just didn't
> matter up to now as no functions used between the two calls needed
> some critical mmu ops (e.g. alloc_pte). This has changed now, so I
> corrected it.

I've added this to the commit message and applied to
devel/for-linus-3.19.  If the tests pass I will consider sending a
further pull request for 3.19 including the linear p2m changes early
next week.

Thanks.

David

  parent reply	other threads:[~2014-12-11 12:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-10 15:56 [PATCH] xen: switch to post-init routines in xen mmu.c earlier Juergen Gross
2014-12-10 16:13 ` Konrad Rzeszutek Wilk
2014-12-11  5:17   ` Juergen Gross
2014-12-10 18:07 ` [Xen-devel] " David Vrabel
2014-12-10 18:07   ` David Vrabel
2014-12-11  5:32   ` Juergen Gross
2014-12-11 12:15     ` David Vrabel
2014-12-11 12:15     ` David Vrabel [this message]
2014-12-11 12:15       ` David Vrabel

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=54898AD9.2030009@citrix.com \
    --to=david.vrabel@citrix.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=jgross@suse.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.