Re: [XEN v2 09/11] xen/arm: Introduce ARM_PA_32 to support 32 bit physical address

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 18 Jan 2023, Jan Beulich wrote:
> On 18.01.2023 12:57, Ayan Kumar Halder wrote:
> > Hi Jan,
> > 
> > On 18/01/2023 08:50, Jan Beulich wrote:
> >> On 17.01.2023 18:43, Ayan Kumar Halder wrote:
> >>> --- a/xen/arch/arm/include/asm/types.h
> >>> +++ b/xen/arch/arm/include/asm/types.h
> >>> @@ -37,9 +37,16 @@ typedef signed long long s64;
> >>>   typedef unsigned long long u64;
> >>>   typedef u32 vaddr_t;
> >>>   #define PRIvaddr PRIx32
> >>> +#if defined(CONFIG_ARM_PA_32)
> >>> +typedef u32 paddr_t;
> >>> +#define INVALID_PADDR (~0UL)
> >>> +#define PADDR_SHIFT BITS_PER_LONG
> >>> +#define PRIpaddr PRIx32
> >>> +#else
> >> With our plan to consolidate basic type definitions into xen/types.h
> >> the use of ARM_PA_32 is problematic: Preferably we'd have an arch-
> >> independent Kconfig setting, much like Linux'es PHYS_ADDR_T_64BIT
> >> (I don't think we should re-use the name directly, as we have no
> >> phys_addr_t type), which each arch selects (or not) accordingly.
> >> Perhaps architectures already selecting 64BIT wouldn't need to do
> >> this explicitly, and instead 64BIT could select the new setting
> >> (or the new setting could default to Y when 64BIT=y).
> > 
> > Looking at some of the instances where 64BIT is currently used 
> > (xen/drivers/passthrough/arm/smmu.c), I understand that it is used to 
> > define the width of **virtual** address.
> > 
> > Thus, it would not conflict with ARM_PA_32 (which is for physical address).
> > 
> > Later on if we wish to introduce something similar to Linux's 
> > PHYS_ADDR_T_64BIT (which is arch agnostic), then ARM_PA_32 can be 
> > selected by "!PHYS_ADDR_T_64BIT" && "CONFIG_ARM_32". (We may decide to 
> > drop ARM_PA_32 config option, but we can still reuse ARM_PA_32 specific 
> > changes).
> > 
> > Also, then we will need to support 32 bit physical address (ie 
> > !PHYS_ADDR_T_64BIT) with 32 bit virtual address (ie !64BIT) and 64 bit 
> > virtual address (ie 64BIT).
> > 
> > Let's confine the current changes to Arm architecture only (as I don't 
> > have knowledge about x86 or RISCV). When similar changes will be done 
> > for other architectures, then we can think about introducing an 
> > architecture agnostic option.
> 
> I disagree, not the least with the present goal of reworking xen/types.h
> vs asm/types.h. By having an arch-independent Kconfig symbol, paddr_t
> could also be manufactured uniformly in xen/types.h.

Jan is only asking to introduce the new Kconfig symbol as an
arch-independent symbol. In other words, rename ARM_PA_32 to PADDR_32
(or something like that) and move the symbol to xen/arch/Kconfig.

I think that's reasonable. And PADDR_32 could be forced to always be
unselected on x86: I don't think Jan is asking you to rework the whole
of xen/arch/x86 and xen/commons to build on x86 with paddr_t set to
uint32_t.