| Message ID | 20131112132001.GJ16735@n2100.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2013-11-12 at 13:20 +0000, Russell King - ARM Linux wrote: > On Tue, Nov 12, 2013 at 12:25:18PM +0000, Stefano Stabellini wrote: > > Arnd, Olof, > > This isn't really an arm-soc thing, it's a core ARM thing... > > > we have been having this discussion on xen-devel regarding whether Xen > > should be allowed to modify the start address of the physical memory > > region in device tree before passing it to dom0 or not. > > > > The reason why this question is coming up now, is that we realized that > > we are going to have to live with the 1:1 pseudo-physical to physical > > mapping for dom0 for a while. This limits the ability of the hypervisor > > of allocating dom0 memory wherever it wants. Xen can allocate dom0 > > memory from the low end but maybe not exactly from the start. > > > > As a result we would adjust the start of physical memory in device tree > > to match the start of the memory region allocated for dom0. For example > > on the Arndale it could be 0x80800000 instead of 0x80000000. > > > > Unfortunately not all the platforms can cope with this very well. In > > particular the Arndale seems to have issues. > > That should be no problem provided that: > > (a) you load the kernel somewhere between 0x80800000 and 0x80ffffff - > the decompressor will decide that the start of memory is 0x80800000, and > place the kernel at 0x80808000. > (b) _at the moment_ you modify DT to specify that memory starts at > 0x80800000 and not 0x80000000. NB we also modify the size. > (b) is going to change soon: the shmobile and zynq platforms already have > a problem with their memory setup which needs a patch in this area, and > the patch will have the side effect of automatically removing (in your > case) 0x80000000 to 0x80800000. See the patch below. I think this is OK. Under Xen we might only give dom0 e.g. 128MB of RAM, which would be at e.g. 0x808000000-0x88800000, so it sounds like the fix in question is actually what we would want. > If there's any other issues with multiplatform, then yes, we want to hear > about them. I think some of the issues we've been seeing were with non-MP kernels. Specifically they were on Arndale, which appeared to be unhappy with RAM being much higher than the normal base address. I believe Arndale/Exynos is currently not (fully?) MP? Or maybe wasn't when this came up? Ian. > arch/arm/kernel/setup.c | 11 +++++++++++ > 1 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c > index f52150d2ec00..1957d54198ad 100644 > --- a/arch/arm/kernel/setup.c > +++ b/arch/arm/kernel/setup.c > @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size) > } > #endif > > + if (aligned_start < PHYS_OFFSET) { > + if (aligned_start + size < PHYS_OFFSET) { > + pr_info("Ignoring memory below PHYS_OFFSET: 0x%08llx-0x%08llx\n", > + aligned_start, aligned_start + size); > + return -EINVAL; > + } > + > + size -= PHYS_OFFSET - aligned_start; > + aligned_start = PHYS_OFFSET; > + } > + > bank->start = aligned_start; > bank->size = size & ~(phys_addr_t)(PAGE_SIZE - 1); >
On Tue, Nov 12, 2013 at 02:38:55PM +0000, Ian Campbell wrote: > On Tue, 2013-11-12 at 13:20 +0000, Russell King - ARM Linux wrote: > > If there's any other issues with multiplatform, then yes, we want to hear > > about them. > > I think some of the issues we've been seeing were with non-MP kernels. > Specifically they were on Arndale, which appeared to be unhappy with RAM > being much higher than the normal base address. I believe Arndale/Exynos > is currently not (fully?) MP? Or maybe wasn't when this came up? I'm not aware of there ever being any SMP/UP constraints on memory in the non-platform specific code. It's possible that some of the platform specific SMP bringup code relies on physical memory at certain addresses (eg, due to restrictions in their boot protocol for starting secondary CPUs) but running a non-MP kernel wouldn't include that code so shouldn't be affected by this.
On Tue, 2013-11-12 at 14:44 +0000, Russell King - ARM Linux wrote: > On Tue, Nov 12, 2013 at 02:38:55PM +0000, Ian Campbell wrote: > > On Tue, 2013-11-12 at 13:20 +0000, Russell King - ARM Linux wrote: > > > If there's any other issues with multiplatform, then yes, we want to hear > > > about them. > > > > I think some of the issues we've been seeing were with non-MP kernels. > > Specifically they were on Arndale, which appeared to be unhappy with RAM > > being much higher than the normal base address. I believe Arndale/Exynos > > is currently not (fully?) MP? Or maybe wasn't when this came up? > > I'm not aware of there ever being any SMP/UP constraints on memory in the > non-platform specific code. Sorry MP == multiplatform here. That was a stupid/confusing abbreviation for me to choose... Ian.
On 11/12/2013 02:20 PM, Russell King - ARM Linux wrote: > On Tue, Nov 12, 2013 at 12:25:18PM +0000, Stefano Stabellini wrote: >> Arnd, Olof, > > This isn't really an arm-soc thing, it's a core ARM thing... > >> we have been having this discussion on xen-devel regarding whether Xen >> should be allowed to modify the start address of the physical memory >> region in device tree before passing it to dom0 or not. >> >> The reason why this question is coming up now, is that we realized that >> we are going to have to live with the 1:1 pseudo-physical to physical >> mapping for dom0 for a while. This limits the ability of the hypervisor >> of allocating dom0 memory wherever it wants. Xen can allocate dom0 >> memory from the low end but maybe not exactly from the start. >> >> As a result we would adjust the start of physical memory in device tree >> to match the start of the memory region allocated for dom0. For example >> on the Arndale it could be 0x80800000 instead of 0x80000000. >> >> Unfortunately not all the platforms can cope with this very well. In >> particular the Arndale seems to have issues. > > That should be no problem provided that: > > (a) you load the kernel somewhere between 0x80800000 and 0x80ffffff - > the decompressor will decide that the start of memory is 0x80800000, and > place the kernel at 0x80808000. > (b) _at the moment_ you modify DT to specify that memory starts at > 0x80800000 and not 0x80000000. > > (b) is going to change soon: the shmobile and zynq platforms already have > a problem with their memory setup which needs a patch in this area, and > the patch will have the side effect of automatically removing (in your > case) 0x80000000 to 0x80800000. See the patch below. I wouldn't say problem here - just use case where we want/need to ignore the part of memory. The patch below works nicely for us. > > If there's any other issues with multiplatform, then yes, we want to hear > about them. > > arch/arm/kernel/setup.c | 11 +++++++++++ > 1 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c > index f52150d2ec00..1957d54198ad 100644 > --- a/arch/arm/kernel/setup.c > +++ b/arch/arm/kernel/setup.c > @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size) > } > #endif > > + if (aligned_start < PHYS_OFFSET) { > + if (aligned_start + size < PHYS_OFFSET) { just a note I sent to Russell. Here should be "<=" instead of just "<" > + pr_info("Ignoring memory below PHYS_OFFSET: 0x%08llx-0x%08llx\n", > + aligned_start, aligned_start + size); > + return -EINVAL; > + } > + > + size -= PHYS_OFFSET - aligned_start; > + aligned_start = PHYS_OFFSET; We should printk a message here to be aware that alignment was done. Thanks, Michal
On Tue, Nov 12, 2013 at 04:24:46PM +0100, Michal Simek wrote: > On 11/12/2013 02:20 PM, Russell King - ARM Linux wrote: > > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c > > index f52150d2ec00..1957d54198ad 100644 > > --- a/arch/arm/kernel/setup.c > > +++ b/arch/arm/kernel/setup.c > > @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size) > > } > > #endif > > > > + if (aligned_start < PHYS_OFFSET) { > > + if (aligned_start + size < PHYS_OFFSET) { > > just a note I sent to Russell. Here should be "<=" instead of just "<" > > > + pr_info("Ignoring memory below PHYS_OFFSET: 0x%08llx-0x%08llx\n", > > + aligned_start, aligned_start + size); > > + return -EINVAL; > > + } > > + > > + size -= PHYS_OFFSET - aligned_start; > > + aligned_start = PHYS_OFFSET; > > We should printk a message here to be aware that alignment was done. Yep, you sent me an updated patch, I haven't added anything to my kernel tree for about a week and a bit now as I want to get what's already there into Linus' tree. Once that stuff has gone, I'll see about adding some of these fixes we need into the queue.
On 11/12/2013 04:39 PM, Russell King - ARM Linux wrote: > On Tue, Nov 12, 2013 at 04:24:46PM +0100, Michal Simek wrote: >> On 11/12/2013 02:20 PM, Russell King - ARM Linux wrote: >>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c >>> index f52150d2ec00..1957d54198ad 100644 >>> --- a/arch/arm/kernel/setup.c >>> +++ b/arch/arm/kernel/setup.c >>> @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size) >>> } >>> #endif >>> >>> + if (aligned_start < PHYS_OFFSET) { >>> + if (aligned_start + size < PHYS_OFFSET) { >> >> just a note I sent to Russell. Here should be "<=" instead of just "<" >> >>> + pr_info("Ignoring memory below PHYS_OFFSET: 0x%08llx-0x%08llx\n", >>> + aligned_start, aligned_start + size); >>> + return -EINVAL; >>> + } >>> + >>> + size -= PHYS_OFFSET - aligned_start; >>> + aligned_start = PHYS_OFFSET; >> >> We should printk a message here to be aware that alignment was done. > > Yep, you sent me an updated patch, I haven't added anything to my > kernel tree for about a week and a bit now as I want to get what's > already there into Linus' tree. Once that stuff has gone, I'll see > about adding some of these fixes we need into the queue. No problem. Take you time. Thanks, Michal
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index f52150d2ec00..1957d54198ad 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -660,6 +660,17 @@ int __init arm_add_memory(u64 start, u64 size) } #endif + if (aligned_start < PHYS_OFFSET) { + if (aligned_start + size < PHYS_OFFSET) { + pr_info("Ignoring memory below PHYS_OFFSET: 0x%08llx-0x%08llx\n", + aligned_start, aligned_start + size); + return -EINVAL; + } + + size -= PHYS_OFFSET - aligned_start; + aligned_start = PHYS_OFFSET; + } + bank->start = aligned_start; bank->size = size & ~(phys_addr_t)(PAGE_SIZE - 1);