ARM: mm: Extend the runtime patch stub for PAE systems

Fix remainder types used when converting back and forth between physical and virtual addresses.

Cc Nicolas Pitre <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

---
 arch/arm/include/asm/memory.h |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

arch/arm/include/asm/memory.h +16 -6

index 584786f..93376e4 100644
@@ -185,22 +185,32 @@ extern unsigned long __pv_phys_offset;
 	: "=r" (to)					\
 	: "r" (from), "I" (type))
 
-static inline unsigned long __virt_to_phys(unsigned long x)
+static inline phys_addr_t __virt_to_phys(unsigned long x)
 {
 	unsigned long t;
 	__pv_stub(x, t, "add", __PV_BITS_31_24);
 	return t;
 }
 
-static inline unsigned long __phys_to_virt(unsigned long x)
+static inline unsigned long __phys_to_virt(phys_addr_t x)
 {
 	unsigned long t;
 	__pv_stub(x, t, "sub", __PV_BITS_31_24);
 	return t;
 }
+
 #else
-#define __virt_to_phys(x)	((x) - PAGE_OFFSET + PHYS_OFFSET)
-#define __phys_to_virt(x)	((x) - PHYS_OFFSET + PAGE_OFFSET)
+
+static inline phys_addr_t __virt_to_phys(unsigned long x)
+{
+	return (phys_addr_t)x - PAGE_OFFSET + PHYS_OFFSET;
+}
+
+static inline unsigned long __phys_to_virt(phys_addr_t x)
+{
+	return x - PHYS_OFFSET + PAGE_OFFSET;
+}
+
 #endif
 #endif
 #endif /* __ASSEMBLY__ */

@@ -238,14 +248,14 @@ static inline phys_addr_t virt_to_phys(const volatile void *x)
 
 static inline void *phys_to_virt(phys_addr_t x)
 {
-	return (void *)(__phys_to_virt((unsigned long)(x)));
+	return (void *)__phys_to_virt(x);
 }
 
 /*
  * Drivers should NOT use these either.
  */
 #define __pa(x)			__virt_to_phys((unsigned long)(x))
-#define __va(x)			((void *)__phys_to_virt((unsigned long)(x)))
+#define __va(x)			((void *)__phys_to_virt((phys_addr_t)(x)))
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
 
 /*

Signature · 1 line

1.7.9.5

From: Sricharan R <[email protected]>

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.

Quoted 16 lines · Santosh Shilimkar

From: Sricharan R <[email protected]>

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.

Quoted 6 lines

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.

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.

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?

It is easy to create (2) just by having a different user:kernel address space split. However I wonder if (1) is likely. Sure you need a memory alias in physical space to be able to boot, however you shouldn't need to address that memory alias via virtual addresses for any significant amount of time. In fact, as soon as the MMU is turned on, there shouldn't be any issue simply using the final physical memory addresses right away.

What am I missing?

Quoted 18 lines · Nicolas Pitre

Quoted 16 lines · Santosh Shilimkar

From: Sricharan R <[email protected]>

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.

Quoted 7 lines

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

So is there really a case where:

1) physical memory is crossing the 4GB mark, and ...

Quoted 3 lines

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

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

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

Quoted 39 lines · Santosh Shilimkar

Quoted 18 lines · Nicolas Pitre

Quoted 16 lines · Santosh Shilimkar

From: Sricharan R <[email protected]>

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.

Quoted 6 lines

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

So is there really a case where:

1) physical memory is crossing the 4GB mark, and ...

Quoted 7 lines

Quoted 3 lines

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

Quoted 1 line

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

Quoted 2 lines

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

Quoted 65 lines · Nicolas Pitre

Quoted 39 lines · Santosh Shilimkar

Quoted 18 lines · Nicolas Pitre

Quoted 16 lines · Santosh Shilimkar

From: Sricharan R <[email protected]>

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.

Quoted 6 lines

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

So is there really a case where:

1) physical memory is crossing the 4GB mark, and ...

Quoted 7 lines

Quoted 3 lines

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

Quoted 1 line

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

Quoted 2 lines

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:

Quoted 7 lines

	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?

Agreed. In our case we do not have the Physical address crossing across
4GB. So ignoring the carry would have be been OK. But we are
also addressing the other case where it would really crossover.

Quoted 16 lines

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

Quoted 5 lines

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

Extending pv_offset to 64bit is really neat way. When PA > VA, then pv_offset is going to be actual value and not 2's complement. Fine here. When running from higher physical address space, we will always fall here.

So for the second case where pv_offset is 0xffffffff .., (PA < VA) is a problem only when we run from lower physical address. So we can safely assume that the higher 32bits of PA are '0' and stub it initially. In this way we can avoid the special case.

Quoted 70 lines · Sricharan R

Quoted 69 lines · Nicolas Pitre

Quoted 39 lines · Santosh Shilimkar

Quoted 18 lines · Nicolas Pitre

Quoted 16 lines · Santosh Shilimkar

From: Sricharan R <[email protected]>

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.

Quoted 6 lines

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

So is there really a case where:

1) physical memory is crossing the 4GB mark, and ...

Quoted 7 lines

Quoted 3 lines

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

Quoted 1 line

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

Quoted 2 lines

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

Quoted 7 lines

Quoted 3 lines

My question is: how likely is this?

What is your actual physical memory start address?

Agreed. In our case we do not have the Physical address crossing across
4GB. So ignoring the carry would have be been OK. But we are
also addressing the other case where it would really crossover.

Quoted 25 lines

Quoted 16 lines

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

Quoted 5 lines

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

Extending pv_offset to 64bit is really neat way. When PA > VA, then pv_offset is going to be actual value and not 2's complement. Fine here. When running from higher physical address space, we will always fall here.

Quoted 4 lines

So for the second case where pv_offset is 0xffffffff .., (PA < VA) is a problem only when we run from lower physical address. So we can safely assume that the higher 32bits of PA are '0' and stub it initially. In this way we can avoid the special case.

Sorry, I missed one more point here. In the second case,we should patch it with 0x0 when (PA > VA) and with 0xffffffff when (PA < VA).

Quoted 102 lines · Sricharan R

Quoted 65 lines · Nicolas Pitre

Quoted 39 lines · Santosh Shilimkar

Quoted 18 lines · Nicolas Pitre

Quoted 16 lines · Santosh Shilimkar

From: Sricharan R <[email protected]>

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.

Quoted 6 lines

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

So is there really a case where:

1) physical memory is crossing the 4GB mark, and ...

Quoted 7 lines

Quoted 3 lines

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

Quoted 1 line

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

Quoted 2 lines

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:

Quoted 7 lines

	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?

Agreed. In our case we do not have the Physical address crossing across
4GB. So ignoring the carry would have be been OK. But we are
also addressing the other case where it would really crossover.

Quoted 16 lines

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

Quoted 5 lines

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

Extending pv_offset to 64bit is really neat way. When PA > VA, then pv_offset is going to be actual value and not 2's complement. Fine here. When running from higher physical address space, we will always fall here.

Quoted 4 lines

So for the second case where pv_offset is 0xffffffff .., (PA < VA) is a problem only when we run from lower physical address. So we can safely assume that the higher 32bits of PA are '0' and stub it initially. In this way we can avoid the special case.

Sorry, I missed one more point here. In the second case,we should patch it with 0x0 when (PA > VA) and with 0xffffffff when (PA < VA).

Quoted 103 lines · Sricharan R

Quoted 102 lines · Sricharan R

Quoted 65 lines · Nicolas Pitre

Quoted 39 lines · Santosh Shilimkar

Quoted 18 lines · Nicolas Pitre

Quoted 16 lines · Santosh Shilimkar

From: Sricharan R <[email protected]>

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.

Quoted 6 lines

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

So is there really a case where:

1) physical memory is crossing the 4GB mark, and ...

Quoted 7 lines

Quoted 3 lines

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

Quoted 1 line

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

Quoted 2 lines

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:

Quoted 7 lines

	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?

Agreed. In our case we do not have the Physical address crossing across
4GB. So ignoring the carry would have be been OK. But we are
also addressing the other case where it would really crossover.

Quoted 16 lines

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

Quoted 5 lines

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

Extending pv_offset to 64bit is really neat way. When PA > VA, then pv_offset is going to be actual value and not 2's complement. Fine here. When running from higher physical address space, we will always fall here.

Quoted 6 lines

Quoted 4 lines

So for the second case where pv_offset is 0xffffffff .., (PA < VA) is a problem only when we run from lower physical address. So we can safely assume that the higher 32bits of PA are '0' and stub it initially. In this way we can avoid the special case.

Sorry, I missed one more point here. In the second case,we should patch it with 0x0 when (PA > VA) and with 0xffffffff when (PA < VA).

I don't think I follow you here.

Let's assume:

phys_addr_t __pv_offset = PHYS_START - VIRT_START;

If PA = 0x0-8000-0000 and VA = 0xc000-0000 then
__pv_offset = 0xffff-ffff-c000-0000.

If PA = 0x2-8000-0000 and VA = 0xc000-0000 then
__pv_offset = 0x1-c000-0000.

So the __virt_to_phys() assembly stub could look like:

static inline phys_addr_t __virt_to_phys(unsigned long x)

{
	phys_addr_t t;

	if if (sizeof(phys_addr_t) == 4) {
		__pv_stub(x, t, "add", __PV_BITS_31_24);
	} else {
		__pv_movhi_stub(t);
		__pv_add_carry_stub(x, t);
	}

	return t;
}

And...

#define __pv_movhi_stub(y) \
	__asm__("@ __pv_movhi_stub\n" \
	"1:	mov	%R0, %1\n" \
	"	.pushsection .pv_table,\"a\"\n" \
	"	.long	1b\n" \
	"	.popsection\n" \
	: "=r" (y) \
	: "I" (__PV_BITS_8_0))

#define __pv_add_carry_stub(x, y) \
	__asm__("@ __pv_add_carry_stub\n" \
	"1:	adds	%Q0, %1, %2\n" \
	"	adc	%R0, %R0, #0\n" \
	"	.pushsection .pv_table,\"a\"\n" \
	"	.long	1b\n" \
	"	.popsection\n" \
	: "+r" (y) \
	: "r" (x), "I" (__PV_BITS_31_24) \
	: "cc")

The stub bits such as __PV_BITS_8_0 can be augmented with more bits in the middle to determine the type of fixup needed. The fixup code would determine the shift needed on the value, and whether or not the low or high word of __pv_offset should be used according to those bits.

Then, in the case where a mov is patched, you need to check if the high word of __pv_offset is 0xffffffff and if so the mov should be turned into a "mvn rn, #0".

And there you are with all possible cases handled.

Nicolas

Hi Nicolas,

On Thursday 25 July 2013 01:51 AM, Nicolas Pitre wrote:

Quoted 55 lines

I don't think I follow you here.

Let's assume:

phys_addr_t __pv_offset = PHYS_START - VIRT_START;

If PA = 0x0-8000-0000 and VA = 0xc000-0000 then
__pv_offset = 0xffff-ffff-c000-0000.

If PA = 0x2-8000-0000 and VA = 0xc000-0000 then
__pv_offset = 0x1-c000-0000.

So the __virt_to_phys() assembly stub could look like:

static inline phys_addr_t __virt_to_phys(unsigned long x)

{
	phys_addr_t t;

	if if (sizeof(phys_addr_t) == 4) {
		__pv_stub(x, t, "add", __PV_BITS_31_24);
	} else {
		__pv_movhi_stub(t);
		__pv_add_carry_stub(x, t);
	}

	return t;
}

And...

#define __pv_movhi_stub(y) \
	__asm__("@ __pv_movhi_stub\n" \
	"1:	mov	%R0, %1\n" \
	"	.pushsection .pv_table,\"a\"\n" \
	"	.long	1b\n" \
	"	.popsection\n" \
	: "=r" (y) \
	: "I" (__PV_BITS_8_0))

#define __pv_add_carry_stub(x, y) \
	__asm__("@ __pv_add_carry_stub\n" \
	"1:	adds	%Q0, %1, %2\n" \
	"	adc	%R0, %R0, #0\n" \
	"	.pushsection .pv_table,\"a\"\n" \
	"	.long	1b\n" \
	"	.popsection\n" \
	: "+r" (y) \
	: "r" (x), "I" (__PV_BITS_31_24) \
	: "cc")

The stub bits such as __PV_BITS_8_0 can be augmented with more bits in the middle to determine the type of fixup needed. The fixup code would determine the shift needed on the value, and whether or not the low or high word of __pv_offset should be used according to those bits.

Quoted 6 lines

Then, in the case where a mov is patched, you need to check if the high word of __pv_offset is 0xffffffff and if so the mov should be turned into a "mvn rn, #0".

And there you are with all possible cases handled.

Thanks and you have given the full details here.

Sorry if i was not clear on my previous response.

1) When i said special case can be avoided, i meant that we need not differentiate the 0xfffffff case inside the __virt_to_phy macro, but can handle it at the time of patching. Your above code makes that clear.

2) I would have ended creating separate tables for 'mov' and 'add' case. But again thanks to your above idea of augumenting the __PV_BITS, with which we can find out run time. And 'mvn' would be needed for moving '0xffffffff' . Now I can get rid of the separate section that i created for 'mov' in my previous version.

Based on discussion/debate on Cyril's generic code patching framework, we cooked up this series which basically trying to extend the existing v2p runtime patching for LPAE machines which can have physical memory beyond 4 GB. Keystone is one such ARM machine.

We think the 64 bit patching can be still made better than the proposed patch in the series and hence seeking expert comments from RMK, Nico and others. Last patch in the series added to just give perspective on how machine code will make use of the available bits from the series.

Quoted 5 lines · Santosh Shilimkar

Based on discussion/debate on Cyril's generic code patching framework, we cooked up this series which basically trying to extend the existing v2p runtime patching for LPAE machines which can have physical memory beyond 4 GB. Keystone is one such ARM machine.

Quoted 4 lines

We think the 64 bit patching can be still made better than the proposed patch in the series and hence seeking expert comments from RMK, Nico and others. Last patch in the series added to just give perspective on how machine code will make use of the available bits from the series.

FYI I'm currently on vacation and only handling quick emails.

I'll review your series in a week or so.

Quoted 9 lines · Santosh Shilimkar

Based on discussion/debate on Cyril's generic code patching framework, we cooked up this series which basically trying to extend the existing v2p runtime patching for LPAE machines which can have physical memory beyond 4 GB. Keystone is one such ARM machine.

We think the 64 bit patching can be still made better than the proposed patch in the series and hence seeking expert comments from RMK, Nico and others. Last patch in the series added to just give perspective on how machine code will make use of the available bits from the series.

FYI I'm currently on vacation and only handling quick emails.

I'll review your series in a week or so.