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
