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

[Julien Grall <julien.grall@xxxxxxx>]

On 19/07/18 06:22, Wei Chen wrote:
> Hi Julien,
Hi Wei,

> > -----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.
If it is reserved 0, then it should already has been configured 
correctly by QEMU. AFAICT, the code below will load the current SCTLR 
and update what is necessary. So I think it is fine if you do nothing 
here. Did I miss anything?
If you really want to define it, then I would name it SCTLR_RES_B6 or 
something similar.
Cheers,

--
Julien Grall

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