Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Stefano Stabellini]

Arnd, Olof,
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.


The question for you, as arm-soc maintainers, is: do you think this
should work and if we find any issues we should just fix them or report
them as bugs?
Is this entirely going away with multiplatform kernels so we shouldn't
worry about it?
Or is this a lost fight and should we find a workaround (see below if we
are curious) to make the start of memory look the same?




On Tue, 12 Nov 2013, Ian Campbell wrote:
> > However why do we even have a device tree property to express where the
> > memory start, when this property can only have a single value?
> > 
> > I think that Xen should be free to make the guest memory start at any
> > address it can express in device tree, within reasonable limits.
> > Otherwise we can go back to the world of add-hoc hardcoded constants and
> > get rid of device tree entirely.
> > If this doesn't work, why the kernel can't be fixed?
> > Am I being unreasonable about this?
> 
> Unfortunately the reality is that the Linux kernel has various
> constraints on where RAM might be and how it is aligned WRT to the
> kernel load address. This is particularly bad for non-multiplatform
> kernels (i.e. Julien had issues on Arndale) but there are aso some
> constraints on non-multiplatform ones (I don't think we've tripped over
> any of them).
> 
> We could fight this and try and fix it but it would basically be a case
> of "Xen dom0 is special compared to the real hardware", and it could
> involve some nasty start-of-day munging.
> 
> Overall a conceptually simple and well defined "ad-hoc hack" seems
> preferable to me.

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Russell King - ARM Linux]

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.

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) {
+			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);
 

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Ian Campbell]

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);
>  

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Russell King - ARM Linux]

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.

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Ian Campbell]

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.

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Michal Simek]

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

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131112/7042b660/attachment.sig>

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Russell King - ARM Linux]

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.

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Michal Simek]

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

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131112/d0555f6a/attachment.sig>

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Arnd Bergmann]

On Tuesday 12 November 2013, Stefano Stabellini wrote:

Hi Stefano,

I haven't given it too much thought, but here is what I believe should
be done:

> The question for you, as arm-soc maintainers, is: do you think this
> should work and if we find any issues we should just fix them or report
> them as bugs?

Modifying the DT to mark anything as "reserved" or absent that Dom0
should or can not touch sounds like the correct way to do this. Whether
this needs to be done by modifying the reg property of the device node
or through a different method I can't tell.

If you find bugs in the kernel that prevent this from working, but it
works fine for everyone else, it's up to you to provide a bug-fix,
which would most likely be up to Russell to apply.

> Is this entirely going away with multiplatform kernels so we shouldn't
> worry about it?

Multiplatform kernels are by definition relocatable using
CONFIG_ARM_PATCH_PHYS_VIRT, within some limitations such as the
granularity of the mapping. You certainly can't move the start of memory
to an address of smaller than 2MB (hugepage) alignment, but you might
need something larger than that.

> Or is this a lost fight and should we find a workaround (see below if we
> are curious) to make the start of memory look the same?

I don't see what hack you are referring to, can you elaborate?

My feeling is that we should maintain the requirement that that it must be
possible to enable Dom0 support on any virtualisation-capable platform
without breaking other platforms or causing an unreasonable run-time
overhead.

BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
non-LPAE ARMv6/v7 multiplatform build?

	Arnd

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Julien Grall]

On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> On Tuesday 12 November 2013, Stefano Stabellini wrote:
>
> Hi Stefano,
>
> I haven't given it too much thought, but here is what I believe should
> be done:
>
>> The question for you, as arm-soc maintainers, is: do you think this
>> should work and if we find any issues we should just fix them or report
>> them as bugs?
>
> Modifying the DT to mark anything as "reserved" or absent that Dom0
> should or can not touch sounds like the correct way to do this. Whether
> this needs to be done by modifying the reg property of the device node
> or through a different method I can't tell.
>
> If you find bugs in the kernel that prevent this from working, but it
> works fine for everyone else, it's up to you to provide a bug-fix,
> which would most likely be up to Russell to apply.
>
>> Is this entirely going away with multiplatform kernels so we shouldn't
>> worry about it?
>
> Multiplatform kernels are by definition relocatable using
> CONFIG_ARM_PATCH_PHYS_VIRT, within some limitations such as the
> granularity of the mapping. You certainly can't move the start of memory
> to an address of smaller than 2MB (hugepage) alignment, but you might
> need something larger than that.

During some debugging on the Arndale and Midway, I found another 
constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
I have noticed that all the kernel physical addresses must be lower than 
the corresponding virtual addresses. So the delta offset compute in 
__fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
If this assertion is not validated, when the kernel will browse the 
memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will 
compute a wrong address and will result to consider all memory bank as 
highmem.

After digging in the code, it seems it's due to some optimization during 
opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?

>> Or is this a lost fight and should we find a workaround (see below if we
>> are curious) to make the start of memory look the same?
>
> I don't see what hack you are referring to, can you elaborate?
>
> My feeling is that we should maintain the requirement that that it must be
> possible to enable Dom0 support on any virtualisation-capable platform
> without breaking other platforms or causing an unreasonable run-time
> overhead.
>
> BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> non-LPAE ARMv6/v7 multiplatform build?

It can be both.

Cheers,

-- 
Julien Grall

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Ian Campbell]

On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> > non-LPAE ARMv6/v7 multiplatform build?
> 
> It can be both.

NB: v7 only, we don't do v6 at all. But yes either LPAE or regular is
fine with us.

Ian.

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Arnd Bergmann]

On Tuesday 12 November 2013, Ian Campbell wrote:
> On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> > > non-LPAE ARMv6/v7 multiplatform build?
> > 
> > It can be both.
> 
> NB: v7 only, we don't do v6 at all. But yes either LPAE or regular is
> fine with us.

Why not combined v6/v7 kernels for non-LPAE? I can't see a technical reason
preventing you from running a Dom0 or DomU kernel that can also run on
some ARMv6 platform as long as both platforms and CPUs are enabled in
Kconfig.

	Arnd

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Stefano Stabellini]

On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> On Tuesday 12 November 2013, Ian Campbell wrote:
> > On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > > BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> > > > non-LPAE ARMv6/v7 multiplatform build?
> > > 
> > > It can be both.
> > 
> > NB: v7 only, we don't do v6 at all. But yes either LPAE or regular is
> > fine with us.
> 
> Why not combined v6/v7 kernels for non-LPAE? I can't see a technical reason
> preventing you from running a Dom0 or DomU kernel that can also run on
> some ARMv6 platform as long as both platforms and CPUs are enabled in
> Kconfig.

Unfortunately today we can't support ARMv6.
>From f880b67dcbdedb49453f88d2ccb1a0937b046d82:
   
    * ARMv6 does not support cmpxchg on 16-bit words that are used in the
      Xen grant table code, so we must ensure that Xen support is only
      built on ARMv7-only kernels not combined ARMv6/v7 kernels.

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Arnd Bergmann]

On Tuesday 12 November 2013, Stefano Stabellini wrote:
> On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > On Tuesday 12 November 2013, Ian Campbell wrote:
> > > On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > > > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > > > BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> > > > > non-LPAE ARMv6/v7 multiplatform build?
> > > > 
> > > > It can be both.
> > > 
> > > NB: v7 only, we don't do v6 at all. But yes either LPAE or regular is
> > > fine with us.
> > 
> > Why not combined v6/v7 kernels for non-LPAE? I can't see a technical reason
> > preventing you from running a Dom0 or DomU kernel that can also run on
> > some ARMv6 platform as long as both platforms and CPUs are enabled in
> > Kconfig.
> 
> Unfortunately today we can't support ARMv6.
> From f880b67dcbdedb49453f88d2ccb1a0937b046d82:
>    
>     * ARMv6 does not support cmpxchg on 16-bit words that are used in the
>       Xen grant table code, so we must ensure that Xen support is only
>       built on ARMv7-only kernels not combined ARMv6/v7 kernels.

Ah, I must have made a mistake there. It's not strictly a bug, but I think
it would be better to undo the dependency I added in that patch and instead
change the Makefile to build the grant table code with -march=armv7-a:
This is safe because we know that this code will only /run/ on v7 even
in a combined v6/v7 kernel, but it lets us get better build coverage because
then we will enable Xen support in an allmodconfig or allyesconfig kernel
that today enables both v6 and v7.

	Arnd

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Ian Campbell]

On Tue, 2013-11-12 at 21:08 +0100, Arnd Bergmann wrote:
> On Tuesday 12 November 2013, Stefano Stabellini wrote:
> > On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > > On Tuesday 12 November 2013, Ian Campbell wrote:
> > > > On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > > > > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > > > > BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> > > > > > non-LPAE ARMv6/v7 multiplatform build?
> > > > > 
> > > > > It can be both.
> > > > 
> > > > NB: v7 only, we don't do v6 at all. But yes either LPAE or regular is
> > > > fine with us.
> > > 
> > > Why not combined v6/v7 kernels for non-LPAE? I can't see a technical reason
> > > preventing you from running a Dom0 or DomU kernel that can also run on
> > > some ARMv6 platform as long as both platforms and CPUs are enabled in
> > > Kconfig.
> > 
> > Unfortunately today we can't support ARMv6.
> > From f880b67dcbdedb49453f88d2ccb1a0937b046d82:
> >    
> >     * ARMv6 does not support cmpxchg on 16-bit words that are used in the
> >       Xen grant table code, so we must ensure that Xen support is only
> >       built on ARMv7-only kernels not combined ARMv6/v7 kernels.
> 
> Ah, I must have made a mistake there. It's not strictly a bug, but I think
> it would be better to undo the dependency I added in that patch and instead
> change the Makefile to build the grant table code with -march=armv7-a:
> This is safe because we know that this code will only /run/ on v7 even
> in a combined v6/v7 kernel, but it lets us get better build coverage because
> then we will enable Xen support in an allmodconfig or allyesconfig kernel
> that today enables both v6 and v7.
> 

This seems reasonable to me if it can be made to work. e.g. the uses of
such constructs would need to be in .c files not static inlines in .h
for it not to get ugly fast. Hopefully that is the case.

Another thing to watch out for is the atomics in xchg_xen_ulong which is
used by drivers/xen/events.c and uses atomic64_xchg expecting to get
exclusive load/store instructions. it looks to me like atomic64_xchg is
the same for v6 and v7 so that is ok.

The last thing to watch out for is sync_test_bit/_test_and_set etc.
Again those look the same to me on v6 and v7.

Ian.

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Stefano Stabellini]

On Wed, 13 Nov 2013, Ian Campbell wrote:
> On Tue, 2013-11-12 at 21:08 +0100, Arnd Bergmann wrote:
> > On Tuesday 12 November 2013, Stefano Stabellini wrote:
> > > On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > > > On Tuesday 12 November 2013, Ian Campbell wrote:
> > > > > On Tue, 2013-11-12 at 14:35 +0000, Julien Grall wrote:
> > > > > > On 11/12/2013 01:37 PM, Arnd Bergmann wrote:
> > > > > > > BTW, does Dom0 require an LPAE-enabled kernel or can it be a regular
> > > > > > > non-LPAE ARMv6/v7 multiplatform build?
> > > > > > 
> > > > > > It can be both.
> > > > > 
> > > > > NB: v7 only, we don't do v6 at all. But yes either LPAE or regular is
> > > > > fine with us.
> > > > 
> > > > Why not combined v6/v7 kernels for non-LPAE? I can't see a technical reason
> > > > preventing you from running a Dom0 or DomU kernel that can also run on
> > > > some ARMv6 platform as long as both platforms and CPUs are enabled in
> > > > Kconfig.
> > > 
> > > Unfortunately today we can't support ARMv6.
> > > From f880b67dcbdedb49453f88d2ccb1a0937b046d82:
> > >    
> > >     * ARMv6 does not support cmpxchg on 16-bit words that are used in the
> > >       Xen grant table code, so we must ensure that Xen support is only
> > >       built on ARMv7-only kernels not combined ARMv6/v7 kernels.
> > 
> > Ah, I must have made a mistake there. It's not strictly a bug, but I think
> > it would be better to undo the dependency I added in that patch and instead
> > change the Makefile to build the grant table code with -march=armv7-a:
> > This is safe because we know that this code will only /run/ on v7 even
> > in a combined v6/v7 kernel, but it lets us get better build coverage because
> > then we will enable Xen support in an allmodconfig or allyesconfig kernel
> > that today enables both v6 and v7.
> > 
> 
> This seems reasonable to me if it can be made to work. e.g. the uses of
> such constructs would need to be in .c files not static inlines in .h
> for it not to get ugly fast. Hopefully that is the case.
> 
> Another thing to watch out for is the atomics in xchg_xen_ulong which is
> used by drivers/xen/events.c and uses atomic64_xchg expecting to get
> exclusive load/store instructions. it looks to me like atomic64_xchg is
> the same for v6 and v7 so that is ok.
> 
> The last thing to watch out for is sync_test_bit/_test_and_set etc.
> Again those look the same to me on v6 and v7.

It is more complicated than I expected as XEN depends on
!GENERIC_ATOMIC64 because we require proper atomic instructions to
read/write memory shared with the hypervisor in events.c (see
85323a991d40681023822e86ca95f38a75262026).
Unfortunately config ARM selects GENERIC_ATOMIC64 if CPU_V6.

In addition to modifying arch/arm/Kconfig and drivers/xen/Makefile I had
to remove the XEN dependency on !GENERIC_ATOMIC64 by reimplementing
atomic64_xchg.

Finally the cmpxchg code used by grant-table.c is in a static inline
protected by #ifndef CONFIG_CPU_V6 (see arch/arm/include/asm/cmpxchg.h).
Passing -march=armv7-a from the Makefile is not enough.


---


diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 01f7013..3a888e1 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1885,8 +1885,7 @@ config XEN_DOM0
 config XEN
 	bool "Xen guest support on ARM (EXPERIMENTAL)"
 	depends on ARM && AEABI && OF
-	depends on CPU_V7 && !CPU_V6
-	depends on !GENERIC_ATOMIC64
+	depends on CPU_V7
 	select ARM_PSCI
 	select SWIOTLB_XEN
 	help
diff --git a/arch/arm/include/asm/xen/events.h b/arch/arm/include/asm/xen/events.h
index 8b1f37b..2032ee6 100644
--- a/arch/arm/include/asm/xen/events.h
+++ b/arch/arm/include/asm/xen/events.h
@@ -16,7 +16,37 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
 	return raw_irqs_disabled_flags(regs->ARM_cpsr);
 }
 
-#define xchg_xen_ulong(ptr, val) atomic64_xchg(container_of((ptr),	\
+#ifdef CONFIG_GENERIC_ATOMIC64
+/* if CONFIG_GENERIC_ATOMIC64 is defined we cannot use the generic
+ * atomic64_xchg function because it is implemented using spin locks.
+ * Here we need proper atomic instructions to read and write memory
+ * shared with the hypervisor.
+ */
+static inline u64 xen_atomic64_xchg(atomic64_t *ptr, u64 new)
+{
+	u64 result;
+	unsigned long tmp;
+
+	smp_mb();
+
+	__asm__ __volatile__("@ xen_atomic64_xchg\n"
+"1:	ldrexd	%0, %H0, [%3]\n"
+"	strexd	%1, %4, %H4, [%3]\n"
+"	teq	%1, #0\n"
+"	bne	1b"
+	: "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter)
+	: "r" (&ptr->counter), "r" (new)
+	: "cc");
+
+	smp_mb();
+
+	return result;
+}
+#else
+#define xen_atomic64_xchg atomic64_xchg
+#endif
+
+#define xchg_xen_ulong(ptr, val) xen_atomic64_xchg(container_of((ptr),	\
 							    atomic64_t,	\
 							    counter), (val))
 
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 62ccf54..d668c3c 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -33,6 +33,14 @@
 
 #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
 
+/* This is required because cmpxchg only support 32-bits operands on
+ * ARMv6, so if CONFIG_CPU_V6 is defined the cmpxchg implemention will
+ * be limited to 32-bits operands.
+ * However we know for sure that if Linux is running on Xen, the
+ * platform is >= ARMv7, so here we can safely undef CONFIG_CPU_V6.
+ */
+#undef CONFIG_CPU_V6
+
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/mm.h>

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Arnd Bergmann]

On Wednesday 13 November 2013, Stefano Stabellini wrote:
>  
> -#define xchg_xen_ulong(ptr, val) atomic64_xchg(container_of((ptr),	\
> +#ifdef CONFIG_GENERIC_ATOMIC64
> +/* if CONFIG_GENERIC_ATOMIC64 is defined we cannot use the generic
> + * atomic64_xchg function because it is implemented using spin locks.
> + * Here we need proper atomic instructions to read and write memory
> + * shared with the hypervisor.
> + */
> +static inline u64 xen_atomic64_xchg(atomic64_t *ptr, u64 new)
> +{
> +	u64 result;
> +	unsigned long tmp;
> +
> +	smp_mb();
> +
> +	__asm__ __volatile__("@ xen_atomic64_xchg\n"
> +"1:	ldrexd	%0, %H0, [%3]\n"
> +"	strexd	%1, %4, %H4, [%3]\n"
> +"	teq	%1, #0\n"
> +"	bne	1b"
> +	: "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter)
> +	: "r" (&ptr->counter), "r" (new)
> +	: "cc");
> +
> +	smp_mb();
> +
> +	return result;
> +}
> +#else
> +#define xen_atomic64_xchg atomic64_xchg
> +#endif

I would prefer not duplicating this code. Maybe we can instead change
the ARM code to always provide this function under the name of
armv7_atomic64_xchg() and conditionally defining atomic64_xchg
to use it. Let's see if Russell has an opinion on this, or perhaps
a better solution.

> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 62ccf54..d668c3c 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -33,6 +33,14 @@
>  
>  #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
>  
> +/* This is required because cmpxchg only support 32-bits operands on
> + * ARMv6, so if CONFIG_CPU_V6 is defined the cmpxchg implemention will
> + * be limited to 32-bits operands.
> + * However we know for sure that if Linux is running on Xen, the
> + * platform is >= ARMv7, so here we can safely undef CONFIG_CPU_V6.
> + */
> +#undef CONFIG_CPU_V6
> +
>  #include <linux/module.h>
>  #include <linux/sched.h>
>  #include <linux/mm.h>
> 

Same here: I'd prefer making this more explicit by changing the cmpxchg
implementation: we could split out the 16-bit cmpxchg case into a separate
function with "armv7" in the name and only call that from the main
cmpxchg() implementations when building an armv7-only kernel, but still
let you call it from Xen code that is known to run only on armv7.

	Arnd 

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Stefano Stabellini]

On Wed, 13 Nov 2013, Arnd Bergmann wrote:
> On Wednesday 13 November 2013, Stefano Stabellini wrote:
> >  
> > -#define xchg_xen_ulong(ptr, val) atomic64_xchg(container_of((ptr),	\
> > +#ifdef CONFIG_GENERIC_ATOMIC64
> > +/* if CONFIG_GENERIC_ATOMIC64 is defined we cannot use the generic
> > + * atomic64_xchg function because it is implemented using spin locks.
> > + * Here we need proper atomic instructions to read and write memory
> > + * shared with the hypervisor.
> > + */
> > +static inline u64 xen_atomic64_xchg(atomic64_t *ptr, u64 new)
> > +{
> > +	u64 result;
> > +	unsigned long tmp;
> > +
> > +	smp_mb();
> > +
> > +	__asm__ __volatile__("@ xen_atomic64_xchg\n"
> > +"1:	ldrexd	%0, %H0, [%3]\n"
> > +"	strexd	%1, %4, %H4, [%3]\n"
> > +"	teq	%1, #0\n"
> > +"	bne	1b"
> > +	: "=&r" (result), "=&r" (tmp), "+Qo" (ptr->counter)
> > +	: "r" (&ptr->counter), "r" (new)
> > +	: "cc");
> > +
> > +	smp_mb();
> > +
> > +	return result;
> > +}
> > +#else
> > +#define xen_atomic64_xchg atomic64_xchg
> > +#endif
> 
> I would prefer not duplicating this code. Maybe we can instead change
> the ARM code to always provide this function under the name of
> armv7_atomic64_xchg() and conditionally defining atomic64_xchg
> to use it. Let's see if Russell has an opinion on this, or perhaps
> a better solution.
> 
> > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> > index 62ccf54..d668c3c 100644
> > --- a/drivers/xen/grant-table.c
> > +++ b/drivers/xen/grant-table.c
> > @@ -33,6 +33,14 @@
> >  
> >  #define pr_fmt(fmt) "xen:" KBUILD_MODNAME ": " fmt
> >  
> > +/* This is required because cmpxchg only support 32-bits operands on
> > + * ARMv6, so if CONFIG_CPU_V6 is defined the cmpxchg implemention will
> > + * be limited to 32-bits operands.
> > + * However we know for sure that if Linux is running on Xen, the
> > + * platform is >= ARMv7, so here we can safely undef CONFIG_CPU_V6.
> > + */
> > +#undef CONFIG_CPU_V6
> > +
> >  #include <linux/module.h>
> >  #include <linux/sched.h>
> >  #include <linux/mm.h>
> > 
> 
> Same here: I'd prefer making this more explicit by changing the cmpxchg
> implementation: we could split out the 16-bit cmpxchg case into a separate
> function with "armv7" in the name and only call that from the main
> cmpxchg() implementations when building an armv7-only kernel, but still
> let you call it from Xen code that is known to run only on armv7.
 
what do you think of:

http://marc.info/?l=linux-arm-kernel&m=138444245625671&w=2

?

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Russell King - ARM Linux]

On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
> During some debugging on the Arndale and Midway, I found another  
> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
> I have noticed that all the kernel physical addresses must be lower than  
> the corresponding virtual addresses. So the delta offset compute in  
> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
> If this assertion is not validated, when the kernel will browse the  
> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will  
> compute a wrong address and will result to consider all memory bank as  
> highmem.
>
> After digging in the code, it seems it's due to some optimization during  
> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?

Are you talking about the code in v3.12 or the code in -next ?

[Xen-devel] Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Julien Grall]

On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
> On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
>> During some debugging on the Arndale and Midway, I found another
>> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
>> I have noticed that all the kernel physical addresses must be lower than
>> the corresponding virtual addresses. So the delta offset compute in
>> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
>> If this assertion is not validated, when the kernel will browse the
>> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will
>> compute a wrong address and will result to consider all memory bank as
>> highmem.
>>
>> After digging in the code, it seems it's due to some optimization during
>> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
>
> Are you talking about the code in v3.12 or the code in -next ?

I was talking about 3.12. I have just checked -next and my issue seems 
to be fixed by the commit f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
I should have checked earlier, thanks.

-- 
Julien Grall

[Xen-devel] Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Ian Campbell]

On Tue, 2013-11-12 at 14:52 +0000, Julien Grall wrote:
> 
> On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
> > On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
> >> During some debugging on the Arndale and Midway, I found another
> >> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
> >> I have noticed that all the kernel physical addresses must be lower than
> >> the corresponding virtual addresses. So the delta offset compute in
> >> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
> >> If this assertion is not validated, when the kernel will browse the
> >> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will
> >> compute a wrong address and will result to consider all memory bank as
> >> highmem.
> >>
> >> After digging in the code, it seems it's due to some optimization during
> >> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
> >
> > Are you talking about the code in v3.12 or the code in -next ?
> 
> I was talking about 3.12. I have just checked -next and my issue seems 
> to be fixed by the commit f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
> I should have checked earlier, thanks.

Should we revert your Xen side fix^Wworkaround then:

commit 6c21cb36e263de2db8716b477157a5b6cd531e1e
Author: Julien Grall <julien.grall@linaro.org>
Date:   Tue Oct 22 11:51:48 2013 +0100

    xen/arm: Allocate memory for dom0 from the bottom with the 1:1 Workaround
    
    On Linux, the option CONFIG_ARM_PATCH_PHYS_VIRT (by default enabled) allow
    the Kernel to be loaded anywhere (or nearly) by patching the translation
    pv<->virt at boot time.
    
    The current solution in Linux assuming that the delta physical address -
    virtual address is always negative. A positive delta will destroy all the
    optimisation to modify only a part of the translation instruction (add/sub
    
    By default, Xen is allocating memory from the top of memory and then
    goes down. To avoid booting issue with Linux, we must allocate memory
    from the bottom (ie starting from 0).
    
    Signed-off-by: Julien Grall <julien.grall@linaro.org>
    Acked-by: Ian Campbell <ian.campbell@citrix.com>

The plus side of reverting it is that we can help find any future issues
with Linux. The down side of reverting it is that we can help find any
future issues with Linux...

Ian.

[Xen-devel] Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Julien Grall]

On 11/12/2013 02:57 PM, Ian Campbell wrote:
> On Tue, 2013-11-12 at 14:52 +0000, Julien Grall wrote:
>>
>> On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
>>> On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
>>>> During some debugging on the Arndale and Midway, I found another
>>>> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
>>>> I have noticed that all the kernel physical addresses must be lower than
>>>> the corresponding virtual addresses. So the delta offset compute in
>>>> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
>>>> If this assertion is not validated, when the kernel will browse the
>>>> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will
>>>> compute a wrong address and will result to consider all memory bank as
>>>> highmem.
>>>>
>>>> After digging in the code, it seems it's due to some optimization during
>>>> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
>>>
>>> Are you talking about the code in v3.12 or the code in -next ?
>>
>> I was talking about 3.12. I have just checked -next and my issue seems
>> to be fixed by the commit f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
>> I should have checked earlier, thanks.
>
> Should we revert your Xen side fix^Wworkaround then:

For now it's only in -next. I would wait until this commit is at least 
in Linux master, otherwise we will likely break vanilla/distro kernel 
for Xen 4.4.

-- 
Julien Grall

[Xen-devel] Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Ian Campbell]

On Tue, 2013-11-12 at 15:24 +0000, Julien Grall wrote:
> 
> On 11/12/2013 02:57 PM, Ian Campbell wrote:
> > On Tue, 2013-11-12 at 14:52 +0000, Julien Grall wrote:
> >>
> >> On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
> >>> On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
> >>>> During some debugging on the Arndale and Midway, I found another
> >>>> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
> >>>> I have noticed that all the kernel physical addresses must be lower than
> >>>> the corresponding virtual addresses. So the delta offset compute in
> >>>> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
> >>>> If this assertion is not validated, when the kernel will browse the
> >>>> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will
> >>>> compute a wrong address and will result to consider all memory bank as
> >>>> highmem.
> >>>>
> >>>> After digging in the code, it seems it's due to some optimization during
> >>>> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
> >>>
> >>> Are you talking about the code in v3.12 or the code in -next ?
> >>
> >> I was talking about 3.12. I have just checked -next and my issue seems
> >> to be fixed by the commit f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
> >> I should have checked earlier, thanks.
> >
> > Should we revert your Xen side fix^Wworkaround then:
> 
> For now it's only in -next. I would wait until this commit is at least 
> in Linux master, otherwise we will likely break vanilla/distro kernel 
> for Xen 4.4.

OK. Can you keep an eye on it and let me know if/when the time comes to
revert?

Is the fix a candidate for stable? Seems like a bit of a big exciting
fix for what was an quite recently an obscure corner case...

Ian.

[Xen-devel] Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Julien Grall]

On 11/12/2013 03:32 PM, Ian Campbell wrote:
> On Tue, 2013-11-12 at 15:24 +0000, Julien Grall wrote:
>>
>> On 11/12/2013 02:57 PM, Ian Campbell wrote:
>>> On Tue, 2013-11-12 at 14:52 +0000, Julien Grall wrote:
>>>>
>>>> On 11/12/2013 02:41 PM, Russell King - ARM Linux wrote:
>>>>> On Tue, Nov 12, 2013 at 02:35:10PM +0000, Julien Grall wrote:
>>>>>> During some debugging on the Arndale and Midway, I found another
>>>>>> constraint with CONFIG_ARM_PATCH_PHYS_VIRT.
>>>>>> I have noticed that all the kernel physical addresses must be lower than
>>>>>> the corresponding virtual addresses. So the delta offset compute in
>>>>>> __fixup_pv_table (arch/arm/kernel/head.S) must always be negative.
>>>>>> If this assertion is not validated, when the kernel will browse the
>>>>>> memory bank (sanity_check_info in arch/arm/mm/mmu.c), __phys(...) will
>>>>>> compute a wrong address and will result to consider all memory bank as
>>>>>> highmem.
>>>>>>
>>>>>> After digging in the code, it seems it's due to some optimization during
>>>>>> opcode fixup in __fixup_a_pvtable. Is it a wanted constraint?
>>>>>
>>>>> Are you talking about the code in v3.12 or the code in -next ?
>>>>
>>>> I was talking about 3.12. I have just checked -next and my issue seems
>>>> to be fixed by the commit f52bb722547f43caeaecbcc62db9f3c3b80ead9b.
>>>> I should have checked earlier, thanks.
>>>
>>> Should we revert your Xen side fix^Wworkaround then:
>>
>> For now it's only in -next. I would wait until this commit is at least
>> in Linux master, otherwise we will likely break vanilla/distro kernel
>> for Xen 4.4.
>
> OK. Can you keep an eye on it and let me know if/when the time comes to
> revert?

No problem.

>
> Is the fix a candidate for stable? Seems like a bit of a big exciting
> fix for what was an quite recently an obscure corner case...

I think yes, as soon as Linux contains this fix we will be able to 
revert the commit in Xen.

-- 
Julien Grall

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Stefano Stabellini]

On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > Or is this a lost fight and should we find a workaround (see below if we
> > are curious) to make the start of memory look the same?
> 
> I don't see what hack you are referring to, can you elaborate?

The hack would be mapping dom0 memory at the same start address as it
would be on the native platform but allocating its memory contiguously
at an offset.

For example having dom0 psedo-physical memory start at 0x80000000 (as it
would expect) and end at 0x88000000 (because let's sat that we only give
128MB of memory to dom0). The actual physical memory would be allocated
contiguously from 0xA0000000. The swiotlb-xen code in Linux would be
made aware of the offset 0xA0000000-0x80000000 via device tree and
would calculate the right physical address to use for dma operations.
Today swiotlb-xen assumes that pseudo-phisical addresses correspond to
physical addresses for dom0.

It feels to me like an hack to work around Linux limitations.

Physical memory start contraints in the Linux kernel (Was: Re: Xen osstest on Calxeda midway progress (Was: Re: [Xen-devel] [xen-unstable test] 21486: tolerable FAIL - PUSHED))

[Russell King - ARM Linux]

On Tue, Nov 12, 2013 at 03:00:40PM +0000, Stefano Stabellini wrote:
> On Tue, 12 Nov 2013, Arnd Bergmann wrote:
> > > Or is this a lost fight and should we find a workaround (see below if we
> > > are curious) to make the start of memory look the same?
> > 
> > I don't see what hack you are referring to, can you elaborate?
> 
> The hack would be mapping dom0 memory at the same start address as it
> would be on the native platform but allocating its memory contiguously
> at an offset.
> 
> For example having dom0 psedo-physical memory start at 0x80000000 (as it
> would expect) and end at 0x88000000 (because let's sat that we only give
> 128MB of memory to dom0). The actual physical memory would be allocated
> contiguously from 0xA0000000. The swiotlb-xen code in Linux would be
> made aware of the offset 0xA0000000-0x80000000 via device tree and
> would calculate the right physical address to use for dma operations.
> Today swiotlb-xen assumes that pseudo-phisical addresses correspond to
> physical addresses for dom0.

First, let's get something right here.  Conceptually, DMA addresses have
_never_ been the same as physical addresses in the kernel (with the
exception of DMA masks being interpreted strangely.)

Physical addresses have always been what's required to be programmed into
things like the MMU to gain access to RAM.  We have virt_to_phys() etc
for translations of this nature for lowmem, and kmap() etc for highmem.

DMA addresses have always been the value which you program into the DMA
controller for it to be able to access RAM.  These use the DMA API to
obtain the DMA address.  Behind the DMA API, we have dma_to_virt() and
virt_to_dma() etc which do the appropriate translation for this.

Of course, for virtualisation, replacing the DMA ops is probably the
better solution to deal with this, where you can hook into the mapping/
unmapping/coherent functions and return the correct DMA addresses.

Remember though - anything which assumes virtual_address =
phys_to_virt(dma_addr_t) is basically violating the conceptual model
here, and should be fixed.