Re: [XEN v3] Xen: Ensure "xenheap_bits - PAGE_SHIFT" can be used as a rhs operand of shift operator

[Julien Grall <julien@xxxxxxx>]

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