On 01/12/2022 10:26, Julien Grall wrote:
Hi Ayan,
Hi Julien,
I have a question.
On 01/12/2022 10:03, Ayan Kumar Halder wrote:
It is possible for a pointer to represent physical memory of the same
size.
In other words, a 32 bit pointer can represent 32 bit addressable
physical
memory.
Thus, issue a compilation failure only when the count of physical
address bits
is greater than BITS_PER_LONG (ie count of bits in void*).
I am having difficult to understand how this description is related to
the BUILD_BUG_ON(). AFAIU, it is used to check that xenheap_bits can
be used in shift.
If the unsigned long is 32-bit, then a shift of 32 could be undefined.
Looking at the current use, the shift are used with "xenheap_bits -
PAGE_SHIFT". So as long as PAGE_SHIFT is not 0, you would be fine.
Ack
Signed-off-by: Ayan Kumar Halder <[email protected]>
---
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 | Don't know | 64 |
| x86 | 52 | 64 |
-------------------------------------------------
The Arm_32 line is a bit confusing because one would wonder why we
haven't seen this issue yet. So I think you want to clarify that the
code path is not used by Arm32.
Do you want this clarification and the above/below explanation to be a
part of the commit message ?
I will then send out a v2 with the fix.
- Ayan
However, this will change when we introduce a platform (For eg
Cortex-R52) which
supports 32 bit physical address and BITS_PER_LONG.
Thus, I have introduced this change as I don't see it causing a
regression on
any of the supported platforms.
xen/common/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 62afb07bc6..cd390a0956 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 > BITS_PER_LONG);
Based on the above, I think this wants to be "(PADDR_BITS -
PAGE_SHIFT) >= BITS_PER_LONG)".
Ack
xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);
printk(XENLOG_INFO "Xen heap: %u bits\n", xenheap_bits);
}
Cheers,
