Xen devel - Apr 2011 - Re: xen: do not create the extra e820 region at an addr lower than 4G

I can''t find this patch on any mailing list to respond to so starting a
new thread. In case I''ve not missed it but instead it really
wasn''t
posted -- in the future can we try and ensure that any patches have been
posted at least once. Same for the pull request, it should go to a list.

BTW the version in Konrad''s tree is Stefano''s 2.6.39-rc1-fixes
branch
but Stefano''s tree only has 2.6.39-rc2-fixes now -- is that important?

Anyway, the patch itself:

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 9c38bd1..f831568 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -229,7 +229,7 @@ char * __init xen_memory_setup(void)
 
        memcpy(map_raw, map, sizeof(map));
        e820.nr_map = 0;
-       xen_extra_mem_start = mem_end;
+       xen_extra_mem_start = mem_end < (1UL<<32) ? (1UL<<32) :
mem_end;
        for (i = 0; i < memmap.nr_entries; i++) {
                unsigned long long end;

"(1UL<<32)" will overflow on a 32 bit kernel.

Oh hang on... that''s the difference between the -rc1 and -rc2 versions
of Stefano''s branches:

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index f831568..ee44c56 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -229,7 +229,7 @@ char * __init xen_memory_setup(void)
 
        memcpy(map_raw, map, sizeof(map));
        e820.nr_map = 0;
-       xen_extra_mem_start = mem_end < (1UL<<32) ? (1UL<<32) :
mem_end;
+       xen_extra_mem_start = max((1ULL <<32), mem_end);
        for (i = 0; i < memmap.nr_entries; i++) {
                unsigned long long end;

So I guess it was important!

(also the whitespace in "(1ULL <<32)" is a bit funky, checkpatch
whinges
too)

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, 7 Apr 2011, Ian Campbell wrote:
> I can''t find this patch on any mailing list to respond to so
starting a
> new thread. In case I''ve not missed it but instead it really
wasn''t
> posted -- in the future can we try and ensure that any patches have been
> posted at least once. Same for the pull request, it should go to a list.
This patch hasn''t been sent anywhere yet because it is part of a series
that is not ready yet.

> BTW the version in Konrad''s tree is Stefano''s
2.6.39-rc1-fixes branch
> but Stefano''s tree only has 2.6.39-rc2-fixes now -- is that
important?
I rebased 2.6.39-rc1-fixes on 2.6.39-rc2, dropped 2.6.39-rc1-fixes, and
still doing some refactoring (and force-pushing) until the series is
ready to be sent upstream.

> Anyway, the patch itself:
> 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index 9c38bd1..f831568 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -229,7 +229,7 @@ char * __init xen_memory_setup(void)
>  
>         memcpy(map_raw, map, sizeof(map));
>         e820.nr_map = 0;
> -       xen_extra_mem_start = mem_end;
> +       xen_extra_mem_start = mem_end < (1UL<<32) ?
(1UL<<32) : mem_end;
>         for (i = 0; i < memmap.nr_entries; i++) {
>                 unsigned long long end;
> 
> "(1UL<<32)" will overflow on a 32 bit kernel.
> 
> Oh hang on... that''s the difference between the -rc1 and -rc2
versions
> of Stefano''s branches:
> 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index f831568..ee44c56 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -229,7 +229,7 @@ char * __init xen_memory_setup(void)
>  
>         memcpy(map_raw, map, sizeof(map));
>         e820.nr_map = 0;
> -       xen_extra_mem_start = mem_end < (1UL<<32) ?
(1UL<<32) : mem_end;
> +       xen_extra_mem_start = max((1ULL <<32), mem_end);
>         for (i = 0; i < memmap.nr_entries; i++) {
>                 unsigned long long end;
> 
> So I guess it was important!
> 
> (also the whitespace in "(1ULL <<32)" is a bit funky,
checkpatch whinges
> too)
 
thanks, I''ll fix

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, 2011-04-07 at 11:14 +0100, Stefano Stabellini
wrote:
> On Thu, 7 Apr 2011, Ian Campbell wrote:
> > I can''t find this patch on any mailing list to respond to so
starting a
> > new thread. In case I''ve not missed it but instead it really
wasn''t
> > posted -- in the future can we try and ensure that any patches have
been
> > posted at least once. Same for the pull request, it should go to a
list.
> 
> This patch hasn''t been sent anywhere yet because it is part of a
series
> that is not ready yet.
OK, I was concerned because I saw it get merged in Konrad''s tree...
> > (also the whitespace in "(1ULL <<32)" is a bit funky,
checkpatch whinges
> > too)
>  
> thanks, I''ll fix
Thanks.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, Apr 07, 2011 at 11:21:48AM +0100, Ian Campbell
wrote:
> On Thu, 2011-04-07 at 11:14 +0100, Stefano Stabellini wrote:
> > On Thu, 7 Apr 2011, Ian Campbell wrote:
> > > I can''t find this patch on any mailing list to respond
to so starting a
> > > new thread. In case I''ve not missed it but instead it
really wasn''t
> > > posted -- in the future can we try and ensure that any patches
have been
> > > posted at least once. Same for the pull request, it should go to
a list.
> > 
> > This patch hasn''t been sent anywhere yet because it is part
of a series
> > that is not ready yet.
> 
> OK, I was concerned because I saw it get merged in Konrad''s
tree...
Oversight. Will back it out and use the rc2 version.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel