diff mbox series

[XEN,v4] xen/page_alloc: Relax the BUILD_BUG_ON() in xenheap_max_mfn()

Message ID 20221205094817.21062-1-ayan.kumar.halder@amd.com (mailing list archive)
State New, archived
Headers show
Series [XEN,v4] xen/page_alloc: Relax the BUILD_BUG_ON() in xenheap_max_mfn() | expand

Commit Message

Ayan Kumar Halder Dec. 5, 2022, 9:48 a.m. UTC 
In the near future, we are considering to use a common xen/domain heap for
Arm 32-bit V8-R. In this setup, both PADDR_BITS and BITS_PER_LONG will be
32. Therefore, the condition PADDR_BITS >= BITS_PER_LONG will become true.

Looking at the commit that introduce the BUILD_BUG_ON (88e3ed61642b
"x86/NUMA: make init_node_heap() respect Xen heap limit") and the current
use, it seems this check was mainly to ensure that the shift of xenheap_bits
is not going to be undefined (>> N for a N-bit type is undefined).

So far, all the shifts are using "xenheap_bits - PAGE_SHIFT". Therefore, the
existing code should work for 32-bit architecture as long as the result of
the substraction is below 32.

Therefore relax the BUILD_BUG_ON() to check that (PADDR_BITS - PAGE_SHIFT)
is not equal of above BITS_PER_LONG.

Note that we would need further precaution if we ended up to use
'xenheap_bits' alone in shifts.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Signed-off-by: Julien Grall <julien@xen.org>
---

Currently this change will not have any impact on the existing architectures.
The following table illustrates PADDR_BITS vs BITS_PER_LONG of different archs

------------------------------------------------
| Arch      |   PADDR_BITS    |   BITS_PER_LONG |
------------------------------------------------
| Arm_64    |   48            |   64            |
| Arm_32    |   40            |   32            |
| RISCV_64  |   56            |   64            |
| x86       |   52            |   64            |
-------------------------------------------------

However, this will change when we introduce a platform (For eg Cortex-R52) which
supports 32 bit physical address and BITS_PER_LONG. This platform does not follow
the same code path as Arm_32.
Thus, I have introduced this change as I don't see it causing a regression on
any of the supported platforms.

Changes from v1:-
1. Changed the check from "(PADDR_BITS > BITS_PER_LONG)" to "((PADDR_BITS - PAGE_SHIFT) >= BITS_PER_LONG)"
2. Updated the commit message to explain the reason for this.

v2 :-
1. Updated the commit message.

v3 :-
1. Updated the commit message.
2. Added Julien's SOB.

 xen/common/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Beulich Dec. 5, 2022, 11:46 a.m. UTC | #1 
On 05.12.2022 10:48, Ayan Kumar Halder wrote:
> In the near future, we are considering to use a common xen/domain heap for
> Arm 32-bit V8-R. In this setup, both PADDR_BITS and BITS_PER_LONG will be
> 32. Therefore, the condition PADDR_BITS >= BITS_PER_LONG will become true.
> 
> Looking at the commit that introduce the BUILD_BUG_ON (88e3ed61642b
> "x86/NUMA: make init_node_heap() respect Xen heap limit") and the current
> use, it seems this check was mainly to ensure that the shift of xenheap_bits
> is not going to be undefined (>> N for a N-bit type is undefined).
> 
> So far, all the shifts are using "xenheap_bits - PAGE_SHIFT". Therefore, the
> existing code should work for 32-bit architecture as long as the result of
> the substraction is below 32.
> 
> Therefore relax the BUILD_BUG_ON() to check that (PADDR_BITS - PAGE_SHIFT)
> is not equal of above BITS_PER_LONG.
> 
> Note that we would need further precaution if we ended up to use
> 'xenheap_bits' alone in shifts.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> Signed-off-by: Julien Grall <julien@xen.org>

Acked-by: Jan Beulich <jbeulich@suse.com>
diff mbox series

Patch

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 62afb07bc6..c5b8c7444f 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2245,7 +2245,7 @@  void __init xenheap_max_mfn(unsigned long mfn)
 {
     ASSERT(!first_node_initialised);
     ASSERT(!xenheap_bits);
-    BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
+    BUILD_BUG_ON((PADDR_BITS - PAGE_SHIFT) >= BITS_PER_LONG);
     xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);
     printk(XENLOG_INFO "Xen heap: %u bits\n", xenheap_bits);
 }