[Xen-devel] Re: [PATCH 1/2] xen/mmu: Add workaround "x86-64, mm: Put early page table high"

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 4 May 2011, Daniel Kiper wrote:
> On Tue, May 03, 2011 at 09:51:41PM +0200, Daniel Kiper wrote:
> > On Tue, May 03, 2011 at 11:12:06AM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Tue, May 03, 2011 at 02:55:27AM +0200, Daniel Kiper wrote:
> > > > On Mon, May 02, 2011 at 01:22:21PM -0400, Konrad Rzeszutek Wilk wrote:
> 
> [...]
> 
> > > > I think that (Stefano please confirm or not) this patch was prepared
> > > > as workaround for similar issues. However, I do not like this patch
> > > > because on systems with small amount of memory it leaves huge (to some
> > > > extent) hole between max_low_pfn and 4G. Additionally, it affects
> > > > memory hotplug a bit because it allocates memory starting from current
> > > > max_mfn. It also breaks memory hotplug on i386 (maybe also others
> > > > thinks, however, I could not confirm that). If it stay for some
> > > > reason it should be amended in follwing way:
> > > >
> > > > #ifdef CONFIG_X86_32
> > > >         xen_extra_mem_start = mem_end;
> > > > #else
> > > >         xen_extra_mem_start = max((1ULL << 32), mem_end);
> > > > #endif
> > > >
> > > > Regarding comment for this patch it should be mentioned that without 
> > > > this
> > > > patch e820_end_of_low_ram_pfn() is not broken. It is not called simply.
> > > >
> > > > Last but least. I found that memory sizes below and including exactly 1 
> > > > GiB and
> > > > exactly 2 GiB, 3 GiB (maybe higher, i.e. 4 GiB, 5 GiB, ...; I was not 
> > > > able to test
> > > > them because I do not have sufficient memory) are magic. It means that 
> > > > if memory
> > > > is set with those sizes everything is working good (without 
> > > > 4b239f458c229de044d6905c2b0f9fe16ed9e01e
> > > > and 24bdb0b62cc82120924762ae6bc85afc8c3f2b26 applied). It means that 
> > > > domU
> > > > should be tested with sizes which are not power of two nor multiple of 
> > > > that.
> > >
> > > Hmm, I thought I did test 1500M.
> >
> > It does not work on my machine (24bdb0b62cc82120924762ae6bc85afc8c3f2b26
> > removed and 4b239f458c229de044d6905c2b0f9fe16ed9e01e applied).
> 
> It does not work on my machine (x86_64) with Linux Kernel Ver. 2.6.39-rc6 
> without
> git commit 24bdb0b62cc82120924762ae6bc85afc8c3f2b26 (xen: do not create the 
> extra
> e820 region at an addr lower than 4G). As I said ealier bug introduced by git
> commit 4b239f458c229de044d6905c2b0f9fe16ed9e01e (x86-64, mm: Put early page 
> table
> high) is probably hidden (repaird/workarounded ???) by git commit
> 24bdb0b62cc82120924762ae6bc85afc8c3f2b26 (xen: do not create the extra
> e820 region at an addr lower than 4G).
> 
> Konrad, Stefano could you confirm that ??? If it is true
> how could I help you in removing this bug ???

The reason why "xen: do not create the extra e820 region at an addr
lower than 4G" is needed is the following:

starting the extra memory region at an address lower than 4G is
dangerous because if the extra memory region spans the 4G boundary then
e820_end_of_ram_pfn will get confused and return 0x100000000, therefore
init_memory_mapping will setup 1:1 mapping for 0-0x100000000, including
the whole 3G-4G range.
Of course this is wrong because among other things will cause the kernel
to remap the IOAPIC MMIO registers that should go through the fixmap
instead.
I don't think "x86-64, mm: Put early page table high" is related to the
bug that this commit is trying to solve.

Could you please explain in details what problems do this patch create?

In any case you are right about the fact that the change is not needed
on X86_32 so if it has any bad side effects on X86_32 we can always do
the following, like you suggested:

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 90bac0a..721f576 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -227,7 +227,11 @@ char * __init xen_memory_setup(void)
 
        memcpy(map_raw, map, sizeof(map));
        e820.nr_map = 0;
+#ifdef CONFIG_X86_32
+       xen_extra_mem_start = mem_end;
+#else
        xen_extra_mem_start = max((1ULL << 32), mem_end);
+#endif
        for (i = 0; i < memmap.nr_entries; i++) {
                unsigned long long end;
 

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