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

[Huang Shijie <shijie.huang@xxxxxxx>]

On Mon, Nov 06, 2017 at 10:55:23AM +0000, Julien Grall wrote:
> Hi Shijie,
> 
> On 06/11/17 09:33, Huang Shijie wrote:
> > On Fri, Nov 03, 2017 at 04:22:59PM +0200, Volodymyr Babchuk wrote:
> > > 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?
> > For arm32, it the @physical_address_offset should be 32bit, so we need a
> > casting here.
> 
> Using a cast to accommodate a format is usually plain wrong. For instance,
> because Arm32 could support more than 32-bits physical address, you would
> end up masking the top bits of the address.
> 
> If a format does not work for the two architecture, then you should define
> PRIpaddr that will be "llx" for Arm32 and "lx" for Arm64. That is assuming
> you still use 64-bit for paddr_t.
okay, thanks, I will try to use the "PRIpaddr" later.

Thanks
Huang Shijie

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