Re: [Minios-devel] [UNIKRAFT PATCHv3 23/25] plat/kvm: arm64: Ensure there is no stall data in cache when boot

[Julien Grall <julien.grall@xxxxxxx>]

On 16/01/2019 13:46, Jia He wrote:
> Hi Julien
Hi,

> On 2019/1/16 18:49, Julien Grall wrote:
> > On 16/01/2019 05:59, jia he wrote:
> > > Hi Julien
> > Hi Justin,
> >
> > Please avoid to use gmail web-interface because it does not quote with '>' but 
> > with 'tab'. This makes the reading more difficult (see below).
> ok :)
Thinking about it, gmail actually have a plain text mode that will allow you to 
quote using '>'. However, the web-interface will still try to wrap lines to 80 
characters :/.
> > > On Thu, Dec 13, 2018 at 5:18 PM Wei Chen <wei.chen@xxxxxxx 
> > > <mailto:wei.chen@xxxxxxx>> wrote:
> > >     We will disable MMU and CACHE before pagetable is ready. This
> > >     means we will change memory with cache disabled, so we need to
> > >     invalidate the cache to ensure there is no stall data in it.
> > >     But we don't know the size of the RAM either. And it would be
> > >     expensive to invalidate the whole cache. In this case, just
> > >     just need to invalidate what we are going to use:
> > >     DTB, TEXT, DATA, BSS, pagetables and bootstack.
> > >
> > >     Signed-off-by: Wei Chen <wei.chen@xxxxxxx <mailto:wei.chen@xxxxxxx>>
> > >     ---
> > >       plat/kvm/arm/entry64.S | 36 +++++++++++++++++++++---------------
> > >       1 file changed, 21 insertions(+), 15 deletions(-)
> > >
> > >     diff --git a/plat/kvm/arm/entry64.S b/plat/kvm/arm/entry64.S
> > >     index 867772f..858cf9a 100644
> > >     --- a/plat/kvm/arm/entry64.S
> > >     +++ b/plat/kvm/arm/entry64.S
> > >     @@ -72,6 +72,27 @@ ENTRY(_libkvmplat_entry)
> > >              add x17, x17, #L1_TABLE_SIZE
> > >              add x17, x17, #L2_TABLE_SIZE
> > >
> > >     +       /*
> > >     +        * We will disable MMU and CACHE before pagetable is ready. This
> > >     +        * means we will change memory with cache disabled, so we need to
> > >     +        * invalidate the cache to ensure there is no stall data in it.
> > >     +        * But we don't know the size of the RAM either. And it would be
> > >     +        * expensive to invalidate the whole cache. In this case, just
> > >     +        * just need to invalidate what we are going to use:
> > >     +        * DTB, TEXT, DATA, BSS, pagetables and bootstack.
> > >     +        */
> > >     +       add x27, x26, x17
> > >     +       add x27, x27, #__STACK_SIZE
> > >     +       sub x1, x27, x25
> > >     +       bl clean_and_invalidate_dcache_range
> > >     +
> > >
> > >
> > > Do you think clean_and_invalidate_dcache_range address your concern at [1]？
> > At a first glance, I think so. Do you have a tree with the series applied, so 
> > I can confirm this is correct?
> Sure, I will put the git tree link in next version Patch 0/0.
>
> Please get all the patch series by
>
>   git clone https://github.com/Weichen81/unikraft.git -b 
> to_gerrit_follow_up_v3_irq_timer_thread_on
> PS. This branch contains all the 5 series (~50+ patches)
>
> If you want to review the 1st series,
>
> git checkout 3ec1b68
Thank you for the git link. This patch looks good to me.

Feel free to add:

Reviewed-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

--
Julien Grall

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