Re: [Minios-devel] [PATCH v3 09/43] arm64: add the __PAGE_SIZE macro in header file

[Huang Shijie <shijie.huang@xxxxxxx>]

On Mon, Apr 23, 2018 at 11:42:41AM +0100, Julien Grall wrote:
> Hi Shijie,
> 
> On 23/04/18 10:06, Huang Shijie wrote:
> >On Thu, Apr 19, 2018 at 11:15:56AM +0100, Julien Grall wrote:
> >>
> >>
> >>On 19/04/18 11:09, Huang Shijie wrote:
> >>>On Wed, Apr 18, 2018 at 05:53:09PM +0100, Julien Grall wrote:
> >>>>Hi Shijie,
> >>>>
> >>>>On 16/04/18 07:31, Huang Shijie wrote:
> >>>>>The __PAGE_SIZE is needed by the include/posix/limits.h.
> >>>>>
> >>>>>Signed-off-by: Huang Shijie <shijie.huang@xxxxxxx>
> >>>>>---
> >>>>>  include/arm/arch_limits.h | 2 ++
> >>>>>  1 file changed, 2 insertions(+)
> >>>>>
> >>>>>diff --git a/include/arm/arch_limits.h b/include/arm/arch_limits.h
> >>>>>index 23a491f..de047b0 100644
> >>>>>--- a/include/arm/arch_limits.h
> >>>>>+++ b/include/arm/arch_limits.h
> >>>>>@@ -3,6 +3,8 @@
> >>>>>  #include <page_def.h>
> >>>>>+#define __PAGE_SIZE       (1UL << PAGE_SHIFT)
> >>>>
> >>>>This looks a bit odd to me. __* is used to define the non-underscore
> >>>>version. Have a look at posix/limits.h.
> >>>>
> >>>>This make me realize that page_def.h is introducing the non-underscore
> >>>>version. So I think page_def.h should be fixed. Most likely, you want to
> >>>>fold page_def.h in arch_limits.h because this is pointless to have a 
> >>>>header
> >>>>just defining 3 macro and included only twice.
> >>>I created the page_def.h which makes the compiler very happy.
> >>>The page_def.h is also included in the assembly code, not only twice.
> >>
> >>Mea culpa, 3 times ;). But I don't see any include in the assembly code.
> >>
> >>42sh> ack page_def
> >>
> >>
> >>
> >>include/arm/arch_mm.h
> >>40:#include <page_def.h>
> >>
> >>include/arm/arch_limits.h
> >>4:#include <page_def.h>
> >>
> >>arch/arm/arm64/minios-arm64.lds.S
> >>1:#include <page_def.h>
> >>
> >>Still, I think it is not really warrant. Anyway, what matters is you should
> >>define the __ version and not the other one.
> >Do you mean that I should _only_ define the __PAGE_SIZE, and do not define 
> >the
> >PAGE_SIZE?
> 
> Yes.
okay, I will try to fix it in the next version.

Thanks
Huang Shijie

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