LinuxLists.cc - [PATCH] xen: switch to post-init routines in xen mmu.c earlier

2014-12-10 15:56:10

Subject: [PATCH] xen: switch to post-init routines in xen mmu.c earlier

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 <[email protected]>
Signed-off-by: Juergen Gross <[email protected]>
---
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();

@@ -1236,7 +1237,6 @@ static void __init xen_pagetable_init(void)
xen_remap_memory();

xen_setup_shared_info();
- xen_post_allocator_init();
}
static void xen_write_cr2(unsigned long cr2)
{
--
2.1.2

2014-12-10 16:14:10

by Konrad Rzeszutek Wilk

[permalink] [raw]

Subject: Re: [PATCH] xen: switch to post-init routines in xen mmu.c earlier

On Wed, Dec 10, 2014 at 04:56:03PM +0100, 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.

Um, could you explain a bit more of why the CONFIG_FLATMEM
is such unique? What about the other CONFIG_*MEM cases?

>
> Reported-by: Boris Ostrovsky <[email protected]>
> Signed-off-by: Juergen Gross <[email protected]>
> ---
> 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();
>
> @@ -1236,7 +1237,6 @@ static void __init xen_pagetable_init(void)
> xen_remap_memory();
>
> xen_setup_shared_info();
> - xen_post_allocator_init();
> }
> static void xen_write_cr2(unsigned long cr2)
> {
> --
> 2.1.2
>

2014-12-10 18:07:08

by David Vrabel

[permalink] [raw]

Subject: Re: [Xen-devel] [PATCH] xen: switch to post-init routines in xen mmu.c earlier

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 <[email protected]>
> Signed-off-by: Juergen Gross <[email protected]>
> ---
> 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.

> @@ -1236,7 +1237,6 @@ static void __init xen_pagetable_init(void)
> xen_remap_memory();
>
> xen_setup_shared_info();
> - xen_post_allocator_init();
> }
> static void xen_write_cr2(unsigned long cr2)
> {
>

2014-12-11 05:17:36

by Jürgen Groß

[permalink] [raw]

Subject: Re: [PATCH] xen: switch to post-init routines in xen mmu.c earlier

On 12/10/2014 05:13 PM, Konrad Rzeszutek Wilk wrote:
> On Wed, Dec 10, 2014 at 04:56:03PM +0100, 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.
>
> Um, could you explain a bit more of why the CONFIG_FLATMEM
> is such unique? What about the other CONFIG_*MEM cases?

CONFIG_FLATMEM is just the configuration hitting a BUG() in
xen_alloc_pte_init() being used after finishing paging_init().

Juergen

>
>>
>> Reported-by: Boris Ostrovsky <[email protected]>
>> Signed-off-by: Juergen Gross <[email protected]>
>> ---
>> 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();
>>
>> @@ -1236,7 +1237,6 @@ static void __init xen_pagetable_init(void)
>> xen_remap_memory();
>>
>> xen_setup_shared_info();
>> - xen_post_allocator_init();
>> }
>> static void xen_write_cr2(unsigned long cr2)
>> {
>> --
>> 2.1.2
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2014-12-11 05:32:11

by Jürgen Groß

[permalink] [raw]

Subject: Re: [Xen-devel] [PATCH] xen: switch to post-init routines in xen mmu.c earlier

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 <[email protected]>
>> Signed-off-by: Juergen Gross <[email protected]>
>> ---
>> 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.

Juergen

>
>> @@ -1236,7 +1237,6 @@ static void __init xen_pagetable_init(void)
>> xen_remap_memory();
>>
>> xen_setup_shared_info();
>> - xen_post_allocator_init();
>> }
>> static void xen_write_cr2(unsigned long cr2)
>> {
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2014-12-11 12:15:19

by David Vrabel

[permalink] [raw]

Subject: Re: [Xen-devel] [PATCH] xen: switch to post-init routines in xen mmu.c earlier

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 <[email protected]>
>>> Signed-off-by: Juergen Gross <[email protected]>
>>> ---
>>> 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

2014-12-11 12:15:29

by David Vrabel

[permalink] [raw]