Re: [Minios-devel] [UNIKRAFT PATCHv3 07/25] plat/common: Clean and invalidate D-Cache only when boot for Arm64

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Jia He,

Please find the comments inline.

Thanks & Regards
Sharan

On 1/11/19 10:04 AM, Jia He wrote:
> Hi Sharan
>
> On 2019/1/10 22:48, Sharan Santhanam wrote:
> > Hello Wei Chen,
> >
> > This patch is functionally fine.
> >
> > check_patch report a warning in the commit message which should be fixed.
> >
> > Reviewed-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> >
> > Thanks & Regards
> > Sharan
> >
> > On 12/13/18 10:15 AM, Wei Chen wrote:
> > > The boot code will not rewrite itself, and we don’t have any relocation
>                                               Maybe  ^ this is a non-ascii
>
> But it will be helpful if you can provide the checkpatch result.
>
> I didn't see any error or warning for this patch with the checkpatch.pl
The length of the commit message.

WARNING: Possible unwrapped commit description (prefer a maximum 75 
chars per line)
#7:
    The boot code will not rewrite itself, and we don’t have any 
relocation

> Cheers,
>
> Justin (Jia He)
>
> > > operation for code area. So clean and invalidate I-Cache is unnecessary.
> > >
> > > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > > ---
> > >   plat/common/arm/cache64.S | 26 +++++++-------------------
> > >   plat/kvm/arm/pagetable.S  |  6 +++---
> > >   2 files changed, 10 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/plat/common/arm/cache64.S b/plat/common/arm/cache64.S
> > > index a725557..9962d71 100644
> > > --- a/plat/common/arm/cache64.S
> > > +++ b/plat/common/arm/cache64.S
> > > @@ -38,25 +38,17 @@
> > >   #include <arm/cpu_defs.h>
> > >     /*
> > > - * Function to invalidate I/D cache. This takes the start address in 
> > > x0,
> > > + * Clean and invalidate data cache. This takes the start address in x0,
> > >    * length in x1. It will corrupt x0 ~ x5.
> > >    */
> > > -ENTRY(invalidate_idcache_range)
> > > +ENTRY(clean_and_invalidate_dcache_range)
> > >       /* Get information about the caches from CTR_EL0 */
> > > -    mrs    x3, ctr_el0
> > > +    mrs    x4, ctr_el0
> > >       mov    x2, #CTR_BYTES_PER_WORD
> > >         /* Get minimum D cache line size */
> > > -    ubfx    x4, x3, #CTR_DMINLINE_SHIFT, #CTR_DMINLINE_WIDTH
> > > -    lsl    x4, x2, x4
> > > -
> > > -    /* Get minimum I cache line size */
> > > -    and    x5, x3, #CTR_IMINLINE_MASK
> > > -    lsl    x5, x2, x5
> > > -
> > > -    /* Select the smaller one as I/D cache line size */
> > > -    cmp    x5, x4
> > > -    csel    x3, x5, x4, le
> > > +    ubfx    x3, x4, #CTR_DMINLINE_SHIFT, #CTR_DMINLINE_WIDTH
> > > +    lsl    x3, x2, x3
> > >         /* Align the start address to line size */
> > >       sub    x4, x3, #1
> > > @@ -64,14 +56,10 @@ ENTRY(invalidate_idcache_range)
> > >       add    x1, x1, x2
> > >       bic    x0, x0, x4
> > >   1:
> > > -    /* clean and invalidate D cache by I/D cache line size */
> > > +    /* clean and invalidate D cache by D cache line size */
> > >       dc    civac, x0
> > >       dsb    ish
> > >   -    /* clean and invalidate I cache by I/D cache line size */
> > > -    ic    ivau, x0
> > > -    dsb    ish
> > > -
> > >       /* Move to next line and reduce the size */
> > >       add    x0, x0, x3
> > >       subs    x1, x1, x3
> > > @@ -82,4 +70,4 @@ ENTRY(invalidate_idcache_range)
> > >       isb
> > >         ret
> > > -END(invalidate_idcache_range)
> > > +END(clean_and_invalidate_dcache_range)
> > > diff --git a/plat/kvm/arm/pagetable.S b/plat/kvm/arm/pagetable.S
> > > index 9120c4e..a3c82e2 100644
> > > --- a/plat/kvm/arm/pagetable.S
> > > +++ b/plat/kvm/arm/pagetable.S
> > > @@ -222,8 +222,8 @@ ENTRY(start_mmu)
> > >       mov x22, x30
> > >             /*
> > > -     * Invalidate the I/D cache to avoid using invalid data that 
> > > existed
> > > -     * in I/D cache. Invalidate ranges: DTB, TEXT, DATA, BSS, PAGETABLE
> > > +     * Invalidate the D-Cache to avoid using invalid data that existed
> > > +     * in D-Cache. Invalidate ranges: DTB, TEXT, DATA, BSS, PAGETABLE
> > >        * and BOOTSTACK.
> > >        */
> > >       ldr x0, =_dtb
> > > @@ -231,7 +231,7 @@ ENTRY(start_mmu)
> > >       add x1, x1, #PAGE_TABLE_SIZE
> > >       add x1, x1, #__STACK_SIZE
> > >       sub x1, x1, x0
> > > -    bl invalidate_idcache_range
> > > +    bl clean_and_invalidate_dcache_range
> > >         /* Setup SCTLR */
> > >       ldr x2, =SCTLR_SET_BITS
> > _______________________________________________
> > Minios-devel mailing list
> > Minios-devel@xxxxxxxxxxxxxxxxxxxx
> > https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel