On 30/11/2022 07:09, Jan Beulich wrote:
On 29.11.2022 19:18, Ayan Kumar Halder wrote:
On 29/11/2022 15:52, Julien Grall wrote:
On 29/11/2022 16:23, Ayan Kumar Halder wrote:
On 29/11/2022 14:52, Julien Grall wrote:
On 29/11/2022 14:57, Ayan Kumar Halder wrote:
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2245,7 +2245,9 @@ void __init xenheap_max_mfn(unsigned long mfn)
{
ASSERT(!first_node_initialised);
ASSERT(!xenheap_bits);
+#ifndef CONFIG_AARCH32_V8R
BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
+#endif
BUILD_BUG_ON() are used to indicate that the code would fall over the
check pass. I can't find the justification for this change in the
commit message.
I had a look at the following commit which introduced this, but I
couldn't get the explaination for this.
commit 88e3ed61642bb393458acc7a9bd2f96edc337190
Author: Jan Beulich <jbeul...@suse.com>
Date: Tue Sep 1 14:02:57 2015 +0200
@Jan :- Do you know why BUILD_BUG_ON() was introduced ?
You've cut too much context - the next line explains this all by itself,
I think:
xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);
Clearly addresses used for the Xen heap need to be representable in an
unsigned long (which we assume to be the same size as void *).
But I'm afraid there's further context missing for your question: Why
would that construct be a problem in your case? Is it just that you'd
need it to be > rather than the >= that's used presently?
In my case (for Cortex-R52 build) :-
PADDR_BITS = 32
BITS_PER_LONG = 32
Thus, "BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG)" gets triggered.
I think the physical adresses are representable using "unsigned long".
Yes, using "BUILD_BUG_ON(PADDR_BITS > BITS_PER_LONG)" looks correct to me.
If so, why
do you add an #ifdef rather than dealing with the (apparent) off-by-1?
(I say "apparent" because I haven't checked whether the >= is really
depended upon anywhere.)
The only callers of xenheap_max_mfn() are from xen/arch/x86/setup.c as
follows :-
1. xenheap_max_mfn(PFN_DOWN(highmem_start - 1));
2. xenheap_max_mfn(limit);
Looking at "min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);", I do not
understand the reason for "... -1" (ie subtracting by 1).
Do you know the reason ?
- Ayan
Jan
