Hi,
On 02/12/2022 13:43, Jan Beulich wrote:
On 02.12.2022 13:35, Ayan Kumar Halder wrote:
We want to ensure that "xenheap_bits - PAGE_SHIFT" is strictly less than
the number of bits to represent unsigned long as it is used a rhs operand
to shift mfn.
It is also important to note that the "xenheap_bits - PAGE_SHIFT" is never
used to shift an address (rather than a frame number), and going forward
then also shouldn't be (perhaps unless further precautions are taken).
Hmm, now you've lost why you are making the change in the first place:
The way things are before the patch is quite fine for the described
purpose. (This then also extends to the title. I should have noticed
this in v2 already, but didn't because I still had the v1 title in mind.)
Furthermore in the 2nd paragraph instead of 'the "xenheap_bits -
PAGE_SHIFT"' you mean '"xenheap_bits" alone'.
Let me propose a commit message:
"
xen/page_alloc: Relax the BUILD_BUG_ON() in xenheap_max_mfn()
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.
"
If you end up to take my commit message, then please add my signed-off-by.
Cheers,
--
Julien Grall
