diff mbox series

[XEN,v3,6/9] xen/arm: Introduce choice to enable 64/32 bit physical addressing

Message ID 20230208120529.22313-7-ayan.kumar.halder@amd.com (mailing list archive)
State Superseded
Headers show
Series Add support for 32 bit physical address | expand

Commit Message

Ayan Kumar Halder Feb. 8, 2023, 12:05 p.m. UTC
Some Arm based hardware platforms which does not support LPAE
(eg Cortex-R52), uses 32 bit physical addresses.
Also, users may choose to use 32 bits to represent physical addresses
for optimization.

To support the above use cases, we have introduced arch independent
configs to choose if the physical address can be represented using
32 bits (PHYS_ADDR_32) or 64 bits (PHYS_ADDR_64).
For now only ARM_32 provides support to enable 32 bit physical
addressing.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---

Changes from -
v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr".

v2 - 1. Introduced Kconfig choice. ARM_64 can select PHYS_ADDR_64 only whereas
ARM_32 can select PHYS_ADDR_32 or PHYS_ADDR_64.
2. For CONFIG_ARM_PA_32, paddr_t is defined as 'unsigned long'.

(Jan,Julien please let me know if I understood your suggestion about Kconfig correctly).

 xen/arch/Kconfig                     | 12 +++++++++++
 xen/arch/arm/Kconfig                 | 31 ++++++++++++++++++++++++++++
 xen/arch/arm/include/asm/page-bits.h |  2 ++
 xen/arch/arm/include/asm/types.h     |  6 ++++++
 4 files changed, 51 insertions(+)

Comments

Stefano Stabellini Feb. 11, 2023, 12:34 a.m. UTC | #1
On Wed, 8 Feb 2023, Ayan Kumar Halder wrote:
> Some Arm based hardware platforms which does not support LPAE
> (eg Cortex-R52), uses 32 bit physical addresses.
> Also, users may choose to use 32 bits to represent physical addresses
> for optimization.
> 
> To support the above use cases, we have introduced arch independent
> configs to choose if the physical address can be represented using
> 32 bits (PHYS_ADDR_32) or 64 bits (PHYS_ADDR_64).
> For now only ARM_32 provides support to enable 32 bit physical
> addressing.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> 
> Changes from -
> v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr".
> 
> v2 - 1. Introduced Kconfig choice. ARM_64 can select PHYS_ADDR_64 only whereas
> ARM_32 can select PHYS_ADDR_32 or PHYS_ADDR_64.
> 2. For CONFIG_ARM_PA_32, paddr_t is defined as 'unsigned long'.
> 
> (Jan,Julien please let me know if I understood your suggestion about Kconfig correctly).
> 
>  xen/arch/Kconfig                     | 12 +++++++++++
>  xen/arch/arm/Kconfig                 | 31 ++++++++++++++++++++++++++++
>  xen/arch/arm/include/asm/page-bits.h |  2 ++
>  xen/arch/arm/include/asm/types.h     |  6 ++++++
>  4 files changed, 51 insertions(+)
> 
> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
> index 7028f7b74f..1eff312b51 100644
> --- a/xen/arch/Kconfig
> +++ b/xen/arch/Kconfig
> @@ -1,6 +1,18 @@
>  config 64BIT
>  	bool
>  
> +config PHYS_ADDR_32
> +	bool
> +	help
> +	  Select this option if the physical addresses can be represented by
> +	  32 bits.
> +
> +config PHYS_ADDR_64
> +	bool
> +	help
> +	  Select this option if the physical addresses can be represented
> +	  64 bits.

These two config symbols should be defined in xen/arch/arm/Kconfig
(unless you plan to also define them for x86).


>  config NR_CPUS
>  	int "Maximum number of CPUs"
>  	range 1 4095
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 239d3aed3c..0955891e86 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -18,6 +18,37 @@ config ARM
>  	select HAS_PDX
>  	select HAS_PMAP
>  	select IOMMU_FORCE_PT_SHARE
> +	choice

This shows up at the very beginning of the global menu. I think it would
be better to move it under "Architecture Features"

Also given that there is not choice on arm64, I don't think the choice
option should be visible to the user (because the user cannot change
selection for arm64).

> +		bool "Representative width for any physical address (default 64bit)"
> +		optional

should not be "optional", optional means that neither ARM_PA_64 nor
ARM_PA_32 could be selected. I think we want one of the two to always be
selected, ARM_PA_64 being the default option, and the only option on
arm64.

> +		---help---
> +		  You may want to specify the width to represent the physical
> +		  address space.
> +		  By default, the physical addresses are represented using
> +		  64 bits.
> +
> +		  However in certain platforms, the physical addresses may be
> +		  represented using 32 bits.
> +		  Also, the user may decide that the physical addresses can be
> +		  represented using 32 bits for a given SoC. In those cases,
> +		  user may want to enable 32 bit physical address for
> +		  optimization.
> +		  For now, we have enabled this choice for ARM_32 only.
> +
> +		config ARM_PA_64
> +			select PHYS_ADDR_64
> +			bool "Select 64 bits to represent physical address"
> +			---help---
> +			  Use 64 bits to represent physical address.
> +
> +		config ARM_PA_32
> +			select PHYS_ADDR_32
> +			depends on ARM_32
> +			bool "Select 32 bits to represent physical address"
> +			---help---
> +			  Use 32 bits to represent physical address.
> +
> +    endchoice
>  
>  config ARCH_DEFCONFIG
>  	string
> diff --git a/xen/arch/arm/include/asm/page-bits.h b/xen/arch/arm/include/asm/page-bits.h
> index 5d6477e599..8f4dcebcfd 100644
> --- a/xen/arch/arm/include/asm/page-bits.h
> +++ b/xen/arch/arm/include/asm/page-bits.h
> @@ -5,6 +5,8 @@
>  
>  #ifdef CONFIG_ARM_64
>  #define PADDR_BITS              48
> +#elif CONFIG_ARM_PA_32
> +#define PADDR_BITS              32
>  #else
>  #define PADDR_BITS              40
>  #endif
> diff --git a/xen/arch/arm/include/asm/types.h b/xen/arch/arm/include/asm/types.h
> index e218ed77bd..26144bc87e 100644
> --- a/xen/arch/arm/include/asm/types.h
> +++ b/xen/arch/arm/include/asm/types.h
> @@ -34,9 +34,15 @@ typedef signed long long s64;
>  typedef unsigned long long u64;
>  typedef u32 vaddr_t;
>  #define PRIvaddr PRIx32
> +#if defined(CONFIG_ARM_PA_32)
> +typedef unsigned long paddr_t;
> +#define INVALID_PADDR (~0UL)
> +#define PRIpaddr "08lx"
> +#else
>  typedef u64 paddr_t;
>  #define INVALID_PADDR (~0ULL)
>  #define PRIpaddr "016llx"
> +#endif
>  typedef u32 register_t;
>  #define PRIregister "08x"
>  #elif defined (CONFIG_ARM_64)
> -- 
> 2.17.1
> 
>
Julien Grall Feb. 11, 2023, 10:01 a.m. UTC | #2
Hi Ayan,

On 08/02/2023 12:05, Ayan Kumar Halder wrote:
> Some Arm based hardware platforms which does not support LPAE
> (eg Cortex-R52), uses 32 bit physical addresses.
> Also, users may choose to use 32 bits to represent physical addresses
> for optimization.
> 
> To support the above use cases, we have introduced arch independent
> configs to choose if the physical address can be represented using
> 32 bits (PHYS_ADDR_32) or 64 bits (PHYS_ADDR_64).
> For now only ARM_32 provides support to enable 32 bit physical
> addressing.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> 
> Changes from -
> v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr".
> 
> v2 - 1. Introduced Kconfig choice. ARM_64 can select PHYS_ADDR_64 only whereas
> ARM_32 can select PHYS_ADDR_32 or PHYS_ADDR_64.
> 2. For CONFIG_ARM_PA_32, paddr_t is defined as 'unsigned long'.
> 
> (Jan,Julien please let me know if I understood your suggestion about Kconfig correctly).
> 
>   xen/arch/Kconfig                     | 12 +++++++++++
>   xen/arch/arm/Kconfig                 | 31 ++++++++++++++++++++++++++++
>   xen/arch/arm/include/asm/page-bits.h |  2 ++
>   xen/arch/arm/include/asm/types.h     |  6 ++++++
>   4 files changed, 51 insertions(+)
> 
> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
> index 7028f7b74f..1eff312b51 100644
> --- a/xen/arch/Kconfig
> +++ b/xen/arch/Kconfig
> @@ -1,6 +1,18 @@
>   config 64BIT
>   	bool
>   
> +config PHYS_ADDR_32
> +	bool
> +	help
> +	  Select this option if the physical addresses can be represented by
> +	  32 bits.
> +
> +config PHYS_ADDR_64
> +	bool
> +	help
> +	  Select this option if the physical addresses can be represented
> +	  64 bits.
> +
So you likely don't want to allow the user to select them directly (IOW 
remove the help section). However, I don't see any code using it. Did 
you actually intended to use PHYS_ADDR_{32, 64} in the code?

>   config NR_CPUS
>   	int "Maximum number of CPUs"
>   	range 1 4095
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 239d3aed3c..0955891e86 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -18,6 +18,37 @@ config ARM
>   	select HAS_PDX
>   	select HAS_PMAP
>   	select IOMMU_FORCE_PT_SHARE
> +	choice

I think this is a bit odd that this choice is part of CONFIG_ARM. It 
would be better it is separate. You can do that by removing one indentation.

> +		bool "Representative width for any physical address (default 64bit)"
> +		optional
> +		---help---
> +		  You may want to specify the width to represent the physical
> +		  address space.
> +		  By default, the physical addresses are represented using
> +		  64 bits.
> +
> +		  However in certain platforms, the physical addresses may be
> +		  represented using 32 bits.
> +		  Also, the user may decide that the physical addresses can be
> +		  represented using 32 bits for a given SoC. In those cases,
> +		  user may want to enable 32 bit physical address for
> +		  optimization.
> +		  For now, we have enabled this choice for ARM_32 only.
> +
> +		config ARM_PA_64
> +			select PHYS_ADDR_64
> +			bool "Select 64 bits to represent physical address"
> +			---help---
> +			  Use 64 bits to represent physical address.
> +
> +		config ARM_PA_32
> +			select PHYS_ADDR_32
> +			depends on ARM_32
> +			bool "Select 32 bits to represent physical address"
> +			---help---
> +			  Use 32 bits to represent physical address.

As I wrote in v2, I think this is a bit odd to ask the user what would 
be the width of paddr_t. It is also not scalable if we decide in the 
future to define different PADDR_BITS (i.e. 48, 40, 36, 32).

So it would be better to allow the user to define PADDR_BITS that can 
then be translated by Xen to which ever paddr_t is suitable.

Something like:

choice
    prompt: "Physical address space size" if ARM_32
    default ARM_PA_48 if ARM_64
    default AMR_PA_40 if ARM_32
    help
      ...

config ARM_PA_BITS_32
    bool "32-bit"
    help
        XXX Add help here to explain the benefits of using 32-bit.

    select PHYS_ADDR_T_32
    depends on ARM_32

config ARM_PA_BITS_40
    bool "40-bit"
    select PHYS_ADDR_T_64
    depends on ARM_32

config ARM_PA_BITS_48
    bool "40-bit"
    select PHYS_ADDR_T_64
    depends on ARM_48
endchoice

config PADDR_BITS
    int
    default 32 if ARM_PA_BITS_32
    default 40 if ARM_PA_BITS_40
    default 48 if ARM_PA_BITS_48

With this approach, there would be no structural change in the Kconfig 
if we wanted to support 32/40-bit on Arm64.

> +
> +    endchoice
>   
>   config ARCH_DEFCONFIG
>   	string
> diff --git a/xen/arch/arm/include/asm/page-bits.h b/xen/arch/arm/include/asm/page-bits.h
> index 5d6477e599..8f4dcebcfd 100644
> --- a/xen/arch/arm/include/asm/page-bits.h
> +++ b/xen/arch/arm/include/asm/page-bits.h
> @@ -5,6 +5,8 @@
>   
>   #ifdef CONFIG_ARM_64
>   #define PADDR_BITS              48
> +#elif CONFIG_ARM_PA_32
> +#define PADDR_BITS              32
>   #else
>   #define PADDR_BITS              40
>   #endif

With what I suggested above. This would be replaced with:

#define PADDR_BITS CONFIG_PADDR_BITS

> diff --git a/xen/arch/arm/include/asm/types.h b/xen/arch/arm/include/asm/types.h
> index e218ed77bd..26144bc87e 100644
> --- a/xen/arch/arm/include/asm/types.h
> +++ b/xen/arch/arm/include/asm/types.h
> @@ -34,9 +34,15 @@ typedef signed long long s64;
>   typedef unsigned long long u64;
>   typedef u32 vaddr_t;
>   #define PRIvaddr PRIx32
> +#if defined(CONFIG_ARM_PA_32)

And this could be replaced with
#ifdef CONFIG_PHY_ADDR_T_32

I would also consider to add the following in mm.c

BUILD_BUG_ON((sizeof(paddr_t) * 8) < PADDR_BITS);

This is to make sure that the PADDR_BITS will always fit in paddr_t.

> +typedef unsigned long paddr_t;
> +#define INVALID_PADDR (~0UL)
> +#define PRIpaddr "08lx"
> +#else
>   typedef u64 paddr_t;
>   #define INVALID_PADDR (~0ULL)
>   #define PRIpaddr "016llx"
> +#endif
>   typedef u32 register_t;
>   #define PRIregister "08x"
>   #elif defined (CONFIG_ARM_64)

Cheers,
Julien Grall Feb. 11, 2023, 10:07 a.m. UTC | #3
Hi Stefano,

On 11/02/2023 00:34, Stefano Stabellini wrote:
> On Wed, 8 Feb 2023, Ayan Kumar Halder wrote:
>> Some Arm based hardware platforms which does not support LPAE
>> (eg Cortex-R52), uses 32 bit physical addresses.
>> Also, users may choose to use 32 bits to represent physical addresses
>> for optimization.
>>
>> To support the above use cases, we have introduced arch independent
>> configs to choose if the physical address can be represented using
>> 32 bits (PHYS_ADDR_32) or 64 bits (PHYS_ADDR_64).
>> For now only ARM_32 provides support to enable 32 bit physical
>> addressing.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>>
>> Changes from -
>> v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr".
>>
>> v2 - 1. Introduced Kconfig choice. ARM_64 can select PHYS_ADDR_64 only whereas
>> ARM_32 can select PHYS_ADDR_32 or PHYS_ADDR_64.
>> 2. For CONFIG_ARM_PA_32, paddr_t is defined as 'unsigned long'.
>>
>> (Jan,Julien please let me know if I understood your suggestion about Kconfig correctly).
>>
>>   xen/arch/Kconfig                     | 12 +++++++++++
>>   xen/arch/arm/Kconfig                 | 31 ++++++++++++++++++++++++++++
>>   xen/arch/arm/include/asm/page-bits.h |  2 ++
>>   xen/arch/arm/include/asm/types.h     |  6 ++++++
>>   4 files changed, 51 insertions(+)
>>
>> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
>> index 7028f7b74f..1eff312b51 100644
>> --- a/xen/arch/Kconfig
>> +++ b/xen/arch/Kconfig
>> @@ -1,6 +1,18 @@
>>   config 64BIT
>>   	bool
>>   
>> +config PHYS_ADDR_32
>> +	bool
>> +	help
>> +	  Select this option if the physical addresses can be represented by
>> +	  32 bits.
>> +
>> +config PHYS_ADDR_64
>> +	bool
>> +	help
>> +	  Select this option if the physical addresses can be represented
>> +	  64 bits.
> 
> These two config symbols should be defined in xen/arch/arm/Kconfig
> (unless you plan to also define them for x86).

We discussed with Jan to consolidate types.h as RISC-V may want to use a 
32-bit paddr_t. This would mean paddr_t would be defined a common header 
and therefore those config needs to be in common.

I am not asking Ayan to work on the consolidation, but I think they 
should be defined in common (assuming arm will make use of it) and I 
would at least consider to add "select PHYS_ADDR_64" in "config X86".

Cheers,
Ayan Kumar Halder Feb. 27, 2023, 3:21 p.m. UTC | #4
On 11/02/2023 10:01, Julien Grall wrote:
> Hi Ayan,

Hi Julien,

I need some clarification.

>
> On 08/02/2023 12:05, Ayan Kumar Halder wrote:
>> Some Arm based hardware platforms which does not support LPAE
>> (eg Cortex-R52), uses 32 bit physical addresses.
>> Also, users may choose to use 32 bits to represent physical addresses
>> for optimization.
>>
>> To support the above use cases, we have introduced arch independent
>> configs to choose if the physical address can be represented using
>> 32 bits (PHYS_ADDR_32) or 64 bits (PHYS_ADDR_64).
>> For now only ARM_32 provides support to enable 32 bit physical
>> addressing.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>>
>> Changes from -
>> v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations 
>> required to support 32bit paddr".
>>
>> v2 - 1. Introduced Kconfig choice. ARM_64 can select PHYS_ADDR_64 
>> only whereas
>> ARM_32 can select PHYS_ADDR_32 or PHYS_ADDR_64.
>> 2. For CONFIG_ARM_PA_32, paddr_t is defined as 'unsigned long'.
>>
>> (Jan,Julien please let me know if I understood your suggestion about 
>> Kconfig correctly).
>>
>>   xen/arch/Kconfig                     | 12 +++++++++++
>>   xen/arch/arm/Kconfig                 | 31 ++++++++++++++++++++++++++++
>>   xen/arch/arm/include/asm/page-bits.h |  2 ++
>>   xen/arch/arm/include/asm/types.h     |  6 ++++++
>>   4 files changed, 51 insertions(+)
>>
>> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
>> index 7028f7b74f..1eff312b51 100644
>> --- a/xen/arch/Kconfig
>> +++ b/xen/arch/Kconfig
>> @@ -1,6 +1,18 @@
>>   config 64BIT
>>       bool
>>   +config PHYS_ADDR_32
>> +    bool
>> +    help
>> +      Select this option if the physical addresses can be 
>> represented by
>> +      32 bits.
>> +
>> +config PHYS_ADDR_64
>> +    bool
>> +    help
>> +      Select this option if the physical addresses can be represented
>> +      64 bits.
>> +
> So you likely don't want to allow the user to select them directly 
> (IOW remove the help section). However, I don't see any code using it. 
> Did you actually intended to use PHYS_ADDR_{32, 64} in the code?
>
>>   config NR_CPUS
>>       int "Maximum number of CPUs"
>>       range 1 4095
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index 239d3aed3c..0955891e86 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -18,6 +18,37 @@ config ARM
>>       select HAS_PDX
>>       select HAS_PMAP
>>       select IOMMU_FORCE_PT_SHARE
>> +    choice
>
> I think this is a bit odd that this choice is part of CONFIG_ARM. It 
> would be better it is separate. You can do that by removing one 
> indentation.
>
>> +        bool "Representative width for any physical address (default 
>> 64bit)"
>> +        optional
>> +        ---help---
>> +          You may want to specify the width to represent the physical
>> +          address space.
>> +          By default, the physical addresses are represented using
>> +          64 bits.
>> +
>> +          However in certain platforms, the physical addresses may be
>> +          represented using 32 bits.
>> +          Also, the user may decide that the physical addresses can be
>> +          represented using 32 bits for a given SoC. In those cases,
>> +          user may want to enable 32 bit physical address for
>> +          optimization.
>> +          For now, we have enabled this choice for ARM_32 only.
>> +
>> +        config ARM_PA_64
>> +            select PHYS_ADDR_64
>> +            bool "Select 64 bits to represent physical address"
>> +            ---help---
>> +              Use 64 bits to represent physical address.
>> +
>> +        config ARM_PA_32
>> +            select PHYS_ADDR_32
>> +            depends on ARM_32
>> +            bool "Select 32 bits to represent physical address"
>> +            ---help---
>> +              Use 32 bits to represent physical address.
>
> As I wrote in v2, I think this is a bit odd to ask the user what would 
> be the width of paddr_t. It is also not scalable if we decide in the 
> future to define different PADDR_BITS (i.e. 48, 40, 36, 32).
>
> So it would be better to allow the user to define PADDR_BITS that can 
> then be translated by Xen to which ever paddr_t is suitable.
>
> Something like:
>
> choice
>    prompt: "Physical address space size" if ARM_32
>    default ARM_PA_48 if ARM_64
>    default AMR_PA_40 if ARM_32
>    help
>      ...
>
> config ARM_PA_BITS_32
>    bool "32-bit"
>    help
>        XXX Add help here to explain the benefits of using 32-bit.
>
>    select PHYS_ADDR_T_32
>    depends on ARM_32
>
> config ARM_PA_BITS_40
>    bool "40-bit"
>    select PHYS_ADDR_T_64
>    depends on ARM_32
>
> config ARM_PA_BITS_48
>    bool "40-bit"
>    select PHYS_ADDR_T_64
>    depends on ARM_48
> endchoice
>
> config PADDR_BITS
>    int
>    default 32 if ARM_PA_BITS_32
>    default 40 if ARM_PA_BITS_40
>    default 48 if ARM_PA_BITS_48
>
> With this approach, there would be no structural change in the Kconfig 
> if we wanted to support 32/40-bit on Arm64.
>
>> +
>> +    endchoice
>>     config ARCH_DEFCONFIG
>>       string
>> diff --git a/xen/arch/arm/include/asm/page-bits.h 
>> b/xen/arch/arm/include/asm/page-bits.h
>> index 5d6477e599..8f4dcebcfd 100644
>> --- a/xen/arch/arm/include/asm/page-bits.h
>> +++ b/xen/arch/arm/include/asm/page-bits.h
>> @@ -5,6 +5,8 @@
>>     #ifdef CONFIG_ARM_64
>>   #define PADDR_BITS              48
>> +#elif CONFIG_ARM_PA_32
>> +#define PADDR_BITS              32
>>   #else
>>   #define PADDR_BITS              40
>>   #endif
>
> With what I suggested above. This would be replaced with:
>
> #define PADDR_BITS CONFIG_PADDR_BITS
>
>> diff --git a/xen/arch/arm/include/asm/types.h 
>> b/xen/arch/arm/include/asm/types.h
>> index e218ed77bd..26144bc87e 100644
>> --- a/xen/arch/arm/include/asm/types.h
>> +++ b/xen/arch/arm/include/asm/types.h
>> @@ -34,9 +34,15 @@ typedef signed long long s64;
>>   typedef unsigned long long u64;
>>   typedef u32 vaddr_t;
>>   #define PRIvaddr PRIx32
>> +#if defined(CONFIG_ARM_PA_32)
>
> And this could be replaced with
> #ifdef CONFIG_PHY_ADDR_T_32
>
> I would also consider to add the following in mm.c
>
> BUILD_BUG_ON((sizeof(paddr_t) * 8) < PADDR_BITS);
>
> This is to make sure that the PADDR_BITS will always fit in paddr_t.
>
>> +typedef unsigned long paddr_t;
>> +#define INVALID_PADDR (~0UL)
>> +#define PRIpaddr "08lx"
>> +#else
>>   typedef u64 paddr_t;
>>   #define INVALID_PADDR (~0ULL)
>>   #define PRIpaddr "016llx"
>> +#endif
>>   typedef u32 register_t;
>>   #define PRIregister "08x"
>>   #elif defined (CONFIG_ARM_64)
>
> Cheers,

I have followed your approach with some modifications. Let me know if it 
makes sense.


Subject: [PATCH] xen/arm: Introduce choice to enable 64/32 bit physical
  addressing

Some Arm based hardware platforms which does not support LPAE
(eg Cortex-R52), uses 32 bit physical addresses.
Also, users may choose to use 32 bits to represent physical addresses
for optimization.

To support the above use cases, we have introduced arch independent
configs to choose if the physical address can be represented using
32 bits (PHYS_ADDR_T_32) or 64 bits (PHYS_ADDR_T_64).
For now only ARM_32 provides support to enable 32 bit physical
addressing.

When PHYS_ADDR_T_32 is defined, PADDR_BITS is set to 32.
When PHYS_ADDR_T_64 is defined with ARM_32, PADDR_BITS is set to 40.
When PHYS_ADDR_T_64 is defined with ARM_64, PADDR_BITS is set to 48.
The last two is same as the current configuration used today on Xen.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
  xen/arch/Kconfig                     |  6 ++++
  xen/arch/arm/Kconfig                 | 41 ++++++++++++++++++++++++++--
  xen/arch/arm/include/asm/page-bits.h |  6 +---
  xen/arch/arm/include/asm/types.h     |  6 ++++
  xen/arch/arm/mm.c                    |  1 +
  5 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
index 7028f7b74f..89096c77a4 100644
--- a/xen/arch/Kconfig
+++ b/xen/arch/Kconfig
@@ -1,6 +1,12 @@
  config 64BIT
     bool

+config PHYS_ADDR_T_32
+   bool
+
+config PHYS_ADDR_T_64
+   bool
+
  config NR_CPUS
     int "Maximum number of CPUs"
     range 1 4095
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 239d3aed3c..b23ee56376 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -2,6 +2,7 @@ config ARM_32
     def_bool y
     depends on "$(ARCH)" = "arm32"
     select ARCH_MAP_DOMAIN_PAGE
+   select PHYS_ADDR_T_64

  config ARM_64
     def_bool y
@@ -9,6 +10,7 @@ config ARM_64
     select 64BIT
     select ARM_EFI
     select HAS_FAST_MULTIPLY
+   select PHYS_ADDR_T_64

  config ARM
     def_bool y
@@ -19,13 +21,48 @@ config ARM
     select HAS_PMAP
     select IOMMU_FORCE_PT_SHARE

+menu "Architecture Features"
+
+choice
+   prompt "Physical address space size" if ARM_32
+   default ARM_PA_BITS_48 if ARM_64
+   default ARM_PA_BITS_40 if ARM_32
+   help
+     User can choose to represent the width of physical address. This can
+     sometimes help in optimizing the size of image when user chooses a
+     smaller size to present physical address.
+
+config ARM_PA_BITS_32
+   bool "32-bit"
+   help
+     On platforms where any physical address can be represented within 
32 bits
+     , user should choose this option. This will help is reduced size 
of the
+     binary.
+   select PHYS_ADDR_T_32
+   depends on ARM_32
+
+config ARM_PA_BITS_40
+   bool "40-bit"
+   depends on ARM_32
+
+config ARM_PA_BITS_48
+   bool "40-bit"
+   depends on ARM_48
+endchoice
+
+config PADDR_BITS
+   int
+   default 32 if ARM_PA_BITS_32 || PHYS_ADDR_T_32
+   default 40 if ARM_PA_BITS_40 || (PHYS_ADDR_T_64 && ARM_32)

+   default 48 if ARM_PA_BITS_48 || (PHYS_ADDR_T_64 && ARM_64)

/*

* The reason for this '||' condition is that ARM_PA_BITS_32, 
ARM_PA_BITS_40 or ARM_PA_BITS_48 aren't visible for ARM_64.

* We don't want PADDR_BITS to be undefined in ARM_64. Thus, we have 
added the || condition.

* We could also remove ARM_PA_BITS_32, ARM_PA_BITS_40, ARM_PA_BITS_48 
(from the || ), but have kept for documentation sake.

* Let me know if you want it to be removed.

*/

+
  config ARCH_DEFCONFIG
     string
     default "arch/arm/configs/arm32_defconfig" if ARM_32
     default "arch/arm/configs/arm64_defconfig" if ARM_64

-menu "Architecture Features"
-
  source "arch/Kconfig"

  config ACPI
diff --git a/xen/arch/arm/include/asm/page-bits.h 
b/xen/arch/arm/include/asm/page-bits.h
index 5d6477e599..deb381ceeb 100644
--- a/xen/arch/arm/include/asm/page-bits.h
+++ b/xen/arch/arm/include/asm/page-bits.h
@@ -3,10 +3,6 @@

  #define PAGE_SHIFT              12

-#ifdef CONFIG_ARM_64
-#define PADDR_BITS              48
-#else
-#define PADDR_BITS              40
-#endif
+#define PADDR_BITS              CONFIG_PADDR_BITS

  #endif /* __ARM_PAGE_SHIFT_H__ */
diff --git a/xen/arch/arm/include/asm/types.h 
b/xen/arch/arm/include/asm/types.h
index e218ed77bd..56d27af162 100644
--- a/xen/arch/arm/include/asm/types.h
+++ b/xen/arch/arm/include/asm/types.h
@@ -34,9 +34,15 @@ typedef signed long long s64;
  typedef unsigned long long u64;
  typedef u32 vaddr_t;
  #define PRIvaddr PRIx32
+#if defined(PHYS_ADDR_T_32)
+typedef unsigned long paddr_t;
+#define INVALID_PADDR (~0UL)
+#define PRIpaddr "08lx"
+#else
  typedef u64 paddr_t;
  #define INVALID_PADDR (~0ULL)
  #define PRIpaddr "016llx"
+#endif
  typedef u32 register_t;
  #define PRIregister "08x"
  #elif defined (CONFIG_ARM_64)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b99806af99..d8b43ef38c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -690,6 +690,7 @@ void __init setup_frametable_mappings(paddr_t ps, 
paddr_t pe)
      const unsigned long mapping_size = frametable_size < MB(32) ? 
MB(2) : MB(32);
      int rc;

+    BUILD_BUG_ON((sizeof(paddr_t) * 8) < PADDR_BITS);
      BUILD_BUG_ON(sizeof(struct page_info) != PAGE_INFO_SIZE);

      if ( frametable_size > FRAMETABLE_SIZE )
diff mbox series

Patch

diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
index 7028f7b74f..1eff312b51 100644
--- a/xen/arch/Kconfig
+++ b/xen/arch/Kconfig
@@ -1,6 +1,18 @@ 
 config 64BIT
 	bool
 
+config PHYS_ADDR_32
+	bool
+	help
+	  Select this option if the physical addresses can be represented by
+	  32 bits.
+
+config PHYS_ADDR_64
+	bool
+	help
+	  Select this option if the physical addresses can be represented
+	  64 bits.
+
 config NR_CPUS
 	int "Maximum number of CPUs"
 	range 1 4095
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 239d3aed3c..0955891e86 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -18,6 +18,37 @@  config ARM
 	select HAS_PDX
 	select HAS_PMAP
 	select IOMMU_FORCE_PT_SHARE
+	choice
+		bool "Representative width for any physical address (default 64bit)"
+		optional
+		---help---
+		  You may want to specify the width to represent the physical
+		  address space.
+		  By default, the physical addresses are represented using
+		  64 bits.
+
+		  However in certain platforms, the physical addresses may be
+		  represented using 32 bits.
+		  Also, the user may decide that the physical addresses can be
+		  represented using 32 bits for a given SoC. In those cases,
+		  user may want to enable 32 bit physical address for
+		  optimization.
+		  For now, we have enabled this choice for ARM_32 only.
+
+		config ARM_PA_64
+			select PHYS_ADDR_64
+			bool "Select 64 bits to represent physical address"
+			---help---
+			  Use 64 bits to represent physical address.
+
+		config ARM_PA_32
+			select PHYS_ADDR_32
+			depends on ARM_32
+			bool "Select 32 bits to represent physical address"
+			---help---
+			  Use 32 bits to represent physical address.
+
+    endchoice
 
 config ARCH_DEFCONFIG
 	string
diff --git a/xen/arch/arm/include/asm/page-bits.h b/xen/arch/arm/include/asm/page-bits.h
index 5d6477e599..8f4dcebcfd 100644
--- a/xen/arch/arm/include/asm/page-bits.h
+++ b/xen/arch/arm/include/asm/page-bits.h
@@ -5,6 +5,8 @@ 
 
 #ifdef CONFIG_ARM_64
 #define PADDR_BITS              48
+#elif CONFIG_ARM_PA_32
+#define PADDR_BITS              32
 #else
 #define PADDR_BITS              40
 #endif
diff --git a/xen/arch/arm/include/asm/types.h b/xen/arch/arm/include/asm/types.h
index e218ed77bd..26144bc87e 100644
--- a/xen/arch/arm/include/asm/types.h
+++ b/xen/arch/arm/include/asm/types.h
@@ -34,9 +34,15 @@  typedef signed long long s64;
 typedef unsigned long long u64;
 typedef u32 vaddr_t;
 #define PRIvaddr PRIx32
+#if defined(CONFIG_ARM_PA_32)
+typedef unsigned long paddr_t;
+#define INVALID_PADDR (~0UL)
+#define PRIpaddr "08lx"
+#else
 typedef u64 paddr_t;
 #define INVALID_PADDR (~0ULL)
 #define PRIpaddr "016llx"
+#endif
 typedef u32 register_t;
 #define PRIregister "08x"
 #elif defined (CONFIG_ARM_64)