Re: [Minios-devel] [UNIKRAFT PATCHv4 29/43] plat/kvm: Enable MMU for Arm64

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxx>
> Sent: 2018年7月18日 19:15
> To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
> simon.kuenzer@xxxxxxxxx
> Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 29/43] plat/kvm: Enable MMU for
> Arm64
> 
> Hi Wei,
> 
> On 18/07/18 09:14, Wei Chen wrote:
> >>> +
> >>> +#define TCR_T1SZ_SHIFT   16
> >>> +#define TCR_T0SZ_SHIFT   0
> >>> +#define TCR_T1SZ(x)      ((x) << TCR_T1SZ_SHIFT)
> >>> +#define TCR_T0SZ(x)      ((x) << TCR_T0SZ_SHIFT)
> >>> +#define TCR_TxSZ(x)      (TCR_T1SZ(x) | TCR_T0SZ(x))
> >>> +
> >>> +#define TCR_INIT_FLAGS   (TCR_TxSZ(64 - VIRT_BITS) | TCR_ASID_16 | \
> >>> +                 TCR_TG0_4K | TCR_CACHE_ATTRS | TCR_SMP_ATTRS)
> >>> +
> >>> +/* SCTLR_EL1 - System Control Register */
> >>> +#define SCTLR_RES0       0xc8222400      /* Reserved ARMv8.0, write 0 */
> >>> +#define SCTLR_RES1       0x30d00800      /* Reserved ARMv8.0, write 1 */
> >>
> >> You don't seem to use those two defines. So I would drop them.
> >>
> >
> > Yes, currently, I haven't used them. I would drop them.
> >
> >>> +
> >>> +#define SCTLR_M          (_AC(1, UL) << 0)
> >>> +#define SCTLR_A          (_AC(1, UL) << 1)
> >>> +#define SCTLR_C          (_AC(1, UL) << 2)
> >>> +#define SCTLR_SA (_AC(1, UL) << 3)
> >>> +#define SCTLR_SA0        (_AC(1, UL) << 4)
> >>> +#define SCTLR_CP15BEN    (_AC(1, UL) << 5)
> >>> +#define SCTLR_THEE       (_AC(1, UL) << 6)
> >>
> >> I can't find this bit in the latest ARM ARM (0487C.a).
> >
> > You can find it from here, Reserve0
> >
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0500e/BABJAHDA.
> html
> 
> That's the technical reference for the Cortex-A53 and your link point to
> the 32-bit SCTLR. It seems that is also defined in 64-bit SCTLR.
> 
> However, this bit has no name in the ARM ARM, so I would rather not give
> a name here because it may be re-purposed in the future.
> 

But currently, I need to reserve this bit to zero.

> [...]
> 
> >>>           /* Load dtb address to x0 as a parameter */
> >>>           ldr x0, =_dtb
> >>> diff --git a/plat/kvm/arm/pagetable.S b/plat/kvm/arm/pagetable.S
> >>> index 8de6305..c3bb85b 100644
> >>> --- a/plat/kvm/arm/pagetable.S
> >>> +++ b/plat/kvm/arm/pagetable.S
> >>> @@ -181,6 +181,43 @@ ENTRY(create_pagetables)
> >>>           ret
> >>>    END(create_pagetables)
> >>>
> >>> +ENTRY(start_mmu)
> >>> + dsb sy
> >>
> >> What's this DSB for?
> >>
> >
> > Guarantee the create_pagetables has been done before start mmu.
> 
> Can you add a comment on top explaining it?
> 

Ok. I will do it.

> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel