Re: [XEN v4 07/11] xen/arm: Introduce choice to enable 64/32 bit physical addressing

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.03.2023 19:33, Ayan Kumar Halder wrote:
> On 21/03/2023 16:53, Jan Beulich wrote:
>> On 21.03.2023 17:15, Ayan Kumar Halder wrote:
>>> On 21/03/2023 14:22, Jan Beulich wrote:
>>>> (Using "unsigned long" for a 32-bit paddr_t is of
>>>> course suspicious as well - this ought to be uint32_t.)
>>> The problem with using uint32_t for paddr_t is that there are instances
>>> where the paddr_t is modified with PAGE_MASK or PAGE_ALIGN.
>>>
>>> For eg , handle_passthrough_prop()
>>>
>>>               printk(XENLOG_ERR "Unable to permit to dom%d access to"
>>>                      " 0x%"PRIpaddr" - 0x%"PRIpaddr"\n",
>>>                      kinfo->d->domain_id,
>>>                      mstart & PAGE_MASK, PAGE_ALIGN(mstart + size) - 1);
>>>
>>> And in xen/include/xen/page-size.h,
>>>
>>> #define PAGE_SIZE           (_AC(1,L) << PAGE_SHIFT)
>>> #define PAGE_MASK           (~(PAGE_SIZE-1))
>>>
>>> Thus, the resulting types are unsigned long. This cannot be printed
>>> using %u for PRIpaddr.
>> Is there anything wrong with making PAGE_SIZE expand to (1 << PAGE_SHIFT)
>> when physical addresses are only 32 bits wide?
> 
> I don't have a strong objection except that this is similar to what 
> linux is doing today.
> 
> https://elixir.bootlin.com/linux/latest/source/arch/arm/include/asm/page.h#L12
> 
>>
>>> I remember some discussion (or comment) that the physical addresses
>>> should be represented using 'unsigned long'.
>> A reference would be helpful.
> 
> https://lists.xenproject.org/archives/html/xen-devel/2023-02/msg00305.html

I see. I guess this will be okay as long as only 32-bit arches elect to
use 32-bit physical addresses. Maybe there should be a BUILD_BUG_ON()
somewhere, accompanied by a suitable comment?

Jan