Re: [Minios-devel] [PATCH 04/40] arm64: change physical_address_offset to paddr_t

[Volodymyr Babchuk <vlad.babchuk@xxxxxxxxx>]

Hi Huang,

On 3 November 2017 at 05:11, Huang Shijie <shijie.huang@xxxxxxx> wrote:
> The "uint32_t" is not enough for arm64 code,
> This patch changes it to paddr_t which be used by
> the arm32 and arm64.
>
> Change-Id: I304b3d9f0915399a8ed0e8d0c8c95936c970f346
> Jira: ENTOS-247
> Signed-off-by: Huang Shijie <shijie.huang@xxxxxxx>
> ---
>  arch/arm/mm.c         | 2 +-
>  arch/arm/setup.c      | 5 +++--
>  include/arm/arch_mm.h | 4 +++-
>  3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mm.c b/arch/arm/mm.c
> index f806c9f..3d88d3b 100644
> --- a/arch/arm/mm.c
> +++ b/arch/arm/mm.c
> @@ -6,7 +6,7 @@
>  #include <libfdt.h>
>  #include <lib.h>
>
> -uint32_t physical_address_offset;
> +paddr_t physical_address_offset;
>  struct e820entry e820_map[1] = {
>      {
>          .addr = 0,
> diff --git a/arch/arm/setup.c b/arch/arm/setup.c
> index b65023c..bde30c6 100644
> --- a/arch/arm/setup.c
> +++ b/arch/arm/setup.c
> @@ -19,7 +19,7 @@ void *device_tree;
>  /*
>   * INITIAL C ENTRY POINT.
>   */
> -void arch_init(void *dtb_pointer, uint32_t physical_offset)
> +void arch_init(void *dtb_pointer, paddr_t physical_offset)
>  {
>      int r;
>
> @@ -27,7 +27,8 @@ void arch_init(void *dtb_pointer, uint32_t physical_offset)
>
>      physical_address_offset = physical_offset;
>
> -    xprintk("Virtual -> physical offset = %x\n", physical_address_offset);
> +    xprintk("Virtual -> physical offset = %lx\n",
> +            (unsigned long)physical_address_offset);
Why you are casting physical_address_offset there?
>
>      xprintk("Checking DTB at %p...\n", dtb_pointer);
>
> diff --git a/include/arm/arch_mm.h b/include/arm/arch_mm.h
> index 8ad0d2a..7ce8dd8 100644
> --- a/include/arm/arch_mm.h
> +++ b/include/arm/arch_mm.h
> @@ -6,7 +6,9 @@ typedef uint64_t paddr_t;
>  extern char _text, _etext, _erodata, _edata, _end, __bss_start;
>  extern int _boot_stack[];
>  extern int _boot_stack_end[];
> -extern uint32_t physical_address_offset;       /* Add this to a virtual 
> address to get the physical address (wraps at 4GB) */
Commit message does not describe this change.

> +
> +/* Add this to a virtual address to get the physical address */
> +extern paddr_t physical_address_offset;
>
>  #define PAGE_SHIFT        12
>  #define PAGE_SIZE        (1 << PAGE_SHIFT)
> --
> 2.7.4
>

IIRC, like you used PAGE_SHIFT in patch 2. But you introduce it only
in this patch.  I think you should move this change to the right
place.

-- 
WBR Volodymyr Babchuk aka lorc [+380976646013]
mailto: vlad.babchuk@xxxxxxxxx

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel