[PATCH 6/8] ARM: mm: LPAE: Correct virt_to_phys patching for 64 bit physical addresses

[nicolas.pitre@linaro.org (Nicolas Pitre)]

On Tue, 23 Jul 2013, Santosh Shilimkar wrote:

> On Tuesday 23 July 2013 09:10 PM, Nicolas Pitre wrote:
> > On Fri, 21 Jun 2013, Santosh Shilimkar wrote:
> > 
> >> From: Sricharan R <r.sricharan@ti.com>
> >>
> >> The current phys_to_virt patching mechanism does not work
> >> for 64 bit physical addressesp. Note that constant used in add/sub
> >> instructions is encoded in to the last 8 bits of the opcode. So shift
> >> the _pv_offset constant by 24 to get it in to the correct place.
> >>
> >> The v2p patching mechanism patches the higher 32bits of physical
> >> address with a constant. While this is correct, in those platforms
> >> where the lowmem addressable physical memory spawns across 4GB boundary,
> >> a carry bit can be produced as a result of addition of lower 32bits.
> >> This has to be taken in to account and added in to the upper. The patched
> >> __pv_offset and va are added in lower 32bits, where __pv_offset can be
> >> in two's complement form when PA_START < VA_START and that can result
> >> in a false carry bit.
> >>
> >> e.g PA = 0x80000000 VA = 0xC0000000
> >> __pv_offset = PA - VA = 0xC0000000 (2's complement)
> >>
> >> So adding __pv_offset + VA should never result in a true overflow. So in
> >> order to differentiate between a true carry, a extra flag __pv_sign_flag
> >> is introduced.
> >
> First of all thanks for the review.
>  
> > I'm still wondering if this is worth bothering about.
> > 
> > If PA = 0x80000000 and VA = 0xC0000000 there will never be a real carry 
> > to propagate to the high word of the physical address as the VA space 
> > cannot be larger than 0x40000000.
> > 
> Agreed.
> 
> > So is there really a case where:
> > 
> > 1) physical memory is crossing the 4GB mark, and ...
> > 
> > 2) physical memory start address is higher than virtual memory start 
> >    address needing a carry due to the 32-bit add overflow?
> > 
> Consider below two cases of memory layout apart from one mentioned
> above where the carry is bit irrelevant as you rightly said.
> 
> 1) PA = 0x8_0000_0000, VA= 0xC000_0000, absolute pv_offset = 0x7_4000_0000

This can be patched as:

	mov	phys_hi, #0x8
	add	phys_lo, virt, #0x40000000  @ carry ignored

> 2) PA = 0x2_8000_0000, VA= 0xC000_000, absolute pv_offset = 0x1_C000_0000

	mov	phys_hi, #0x2
	add	phys_lo, virt, #0xc0000000  @ carry ignored

> In both of these cases there a true carry which needs to be
> considered.

Well, not really.  However, if you have:

3) PA = 0x2_8000_0000, VA = 0x4000-0000, pv_offset = 0x2-4000-0000

... then you need:

	mov	phys_hi, #0x2
	adds	phys_lo, virt, #0x40000000
	adc	phys_hi, phys_hi, #0

My question is: how likely is this?

What is your actual physical memory start address?

If we really need to cope with the carry, then the __pv_sign_flag should 
instead be represented in pv_offset directly:

Taking example #2 above, that would be:

	mov	phys_hi, #0x1
	adds	phys_lo, virt, #0xc0000000
	adc	phys_hi, phys_hi, #0

If PA = 0x8000-0000 and VA = 0xc000-0000 then pv_offset is 
0xffff-ffff-c000-0000, meaning:

	mvn	phys_hi, #0
	add	phys_lo, virt, #0xc0000000
	adc	phys_hi, phys_hi, #0

So that would require a special case in the patching code where a mvn 
with 0 is used if the high part of pv_offset is 0xffffffff.


Nicolas