Re: [Minios-devel] [PATCH 02/40] mini-os: replace the L1_PAGETABLE_SHIFT with PAGE_SHIFT

[Huang Shijie <shijie.huang@xxxxxxx>]

On Mon, Nov 06, 2017 at 10:17:28AM +0000, Julien Grall wrote:
> Hi Shijie,
> 
> On 06/11/17 08:36, Huang Shijie wrote:
> > On Fri, Nov 03, 2017 at 05:22:25PM +0000, Julien Grall wrote:
> > > 
> > > 
> > > On 03/11/17 03:11, Huang Shijie wrote:
> > > > The L1_PAGETABLE_SHIFT is a confusing macro for arm.
> > > > Replace it with PAGE_SHIFT.
> > > > 
> > > > Jira: ENTOS-247
> > > > Change-Id: Iadc2eef9ce6f8a4f8a31ee43410e1204dcc8b8da
> > > > Signed-off-by: Huang Shijie <shijie.huang@xxxxxxx>
> > > > ---
> > > >    include/arm/arch_mm.h | 10 ++++------
> > > >    1 file changed, 4 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/include/arm/arch_mm.h b/include/arm/arch_mm.h
> > > > index 79d9e05..8ad0d2a 100644
> > > > --- a/include/arm/arch_mm.h
> > > > +++ b/include/arm/arch_mm.h
> > > > @@ -12,17 +12,15 @@ extern uint32_t physical_address_offset;    /* Add 
> > > > this to a virtual address to get
> > > >    #define PAGE_SIZE        (1 << PAGE_SHIFT)
> > > >    #define PAGE_MASK       (~(PAGE_SIZE-1))
> > > > -#define L1_PAGETABLE_SHIFT      12
> > > > -
> > > >    #define L1_PROT          0
> > > 
> > > If you drop L1_PAGETABLE_SHIFT, please also drop L1_PROT that does not 
> > > seem
> > > to be used and make little sense alone.
> > I think we'd better keep the L1_PROT. If we remove the L1_PROT, we have
> > to change a lot of x86 code.
> 
> You can't claim L1_PAGETABLE_SHIFT is confusing but keeping around L1_PROT
> for Arm. What does it even mean? Is it for 1GB/2MB/4K mapping?
> 
> You should drop the usage in common code and by introducing a new define (on
> x86 the value would be L1_PROT) with a name reflecting the purpose.
Okay, i will try to implement it.

thanks
Huang Shijie

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