Re: [PATCH] riscv: Invalid instruction cache after copy the xol area

Re: [PATCH] riscv: Invalid instruction cache after copy the xol area

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 20058 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220518081753.29589-1-po-kai.chi@sifive.com>
References: <20220518081753.29589-1-po-kai.chi@sifive.com>
TO: "Po-Kai Chi" <po-kai.chi@sifive.com>
TO: Paul Walmsley <paul.walmsley@sifive.com>
TO: Palmer Dabbelt <palmer@dabbelt.com>
TO: Albert Ou <aou@eecs.berkeley.edu>
TO: linux-riscv(a)lists.infradead.org
TO: linux-kernel(a)vger.kernel.org
CC: "Po-Kai Chi" <po-kai.chi@sifive.com>

Hi Po-Kai,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.18-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Po-Kai-Chi/riscv-Invalid-instruction-cache-after-copy-the-xol-area/20220518-162054
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 210e04ff768142b96452030c4c2627512b30ad95
:::::: branch date: 19 hours ago
:::::: commit date: 19 hours ago
config: riscv-randconfig-c006-20220518 (https://download.01.org/0day-ci/archive/20220519/202205191114.vxK8wEiI-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/363d56cc11ac60e351b4ebe886faacc40864e6a9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Po-Kai-Chi/riscv-Invalid-instruction-cache-after-copy-the-xol-area/20220518-162054
        git checkout 363d56cc11ac60e351b4ebe886faacc40864e6a9
        # save the config file
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
           ^
   fs/crypto/keysetup_v1.c:279:8: note: Calling 'derive_key_aes'
           err = derive_key_aes(raw_master_key, ci->ci_nonce,
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/crypto/keysetup_v1.c:57:2: note: Taking false branch
           if (IS_ERR(tfm)) {
           ^
   fs/crypto/keysetup_v1.c:63:8: note: Calling 'skcipher_request_alloc'
           req = skcipher_request_alloc(tfm, GFP_KERNEL);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/crypto/skcipher.h:502:8: note: Memory is allocated
           req = kmalloc(sizeof(struct skcipher_request) +
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/crypto/skcipher.h:505:6: note: Assuming 'req' is non-null
           if (likely(req))
               ^
   include/linux/compiler.h:77:38: note: expanded from macro 'likely'
   # define likely(x)      __builtin_expect(!!(x), 1)
                                             ^~~~
   include/crypto/skcipher.h:505:2: note: Taking true branch
           if (likely(req))
           ^
   fs/crypto/keysetup_v1.c:63:8: note: Returned allocated memory
           req = skcipher_request_alloc(tfm, GFP_KERNEL);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/crypto/keysetup_v1.c:64:7: note: 'req' is non-null
           if (!req) {
                ^~~
   fs/crypto/keysetup_v1.c:64:2: note: Taking false branch
           if (!req) {
           ^
   fs/crypto/keysetup_v1.c:72:6: note: Assuming 'res' is < 0
           if (res < 0)
               ^~~~~~~
   fs/crypto/keysetup_v1.c:72:2: note: Taking true branch
           if (res < 0)
           ^
   fs/crypto/keysetup_v1.c:73:3: note: Control jumps to line 81
                   goto out;
                   ^
   fs/crypto/keysetup_v1.c:82:2: note: Potential leak of memory pointed to by 'req'
           crypto_free_skcipher(tfm);
           ^
   fs/crypto/keysetup_v1.c:192:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memcpy(&hash_key, ci->ci_policy.v1.master_key_descriptor,
           ^~~~~~
   fs/crypto/keysetup_v1.c:192:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
           memcpy(&hash_key, ci->ci_policy.v1.master_key_descriptor,
           ^~~~~~
   fs/crypto/keysetup_v1.c:239:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memcpy(dk->dk_descriptor, ci->ci_policy.v1.master_key_descriptor,
           ^~~~~~
   fs/crypto/keysetup_v1.c:239:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
           memcpy(dk->dk_descriptor, ci->ci_policy.v1.master_key_descriptor,
           ^~~~~~
   fs/crypto/keysetup_v1.c:241:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memcpy(dk->dk_raw, raw_key, ci->ci_mode->keysize);
           ^~~~~~
   fs/crypto/keysetup_v1.c:241:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
           memcpy(dk->dk_raw, raw_key, ci->ci_mode->keysize);
           ^~~~~~
   Suppressed 44 warnings (44 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   10 warnings generated.
   crypto/jitterentropy-kcapi.c:69:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memcpy(dest, src, n);
           ^~~~~~
   crypto/jitterentropy-kcapi.c:69:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
           memcpy(dest, src, n);
           ^~~~~~
   Suppressed 9 warnings (9 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   12 warnings generated.
   crypto/ghash-generic.c:50:2: warning: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memset(dctx, 0, sizeof(*dctx));
           ^~~~~~
   crypto/ghash-generic.c:50:2: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
           memset(dctx, 0, sizeof(*dctx));
           ^~~~~~
   crypto/ghash-generic.c:68:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memcpy(&k, key, GHASH_BLOCK_SIZE); /* avoid violating alignment rules */
           ^~~~~~
   crypto/ghash-generic.c:68:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
           memcpy(&k, key, GHASH_BLOCK_SIZE); /* avoid violating alignment rules */
           ^~~~~~
   crypto/ghash-generic.c:138:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memcpy(dst, buf, GHASH_BLOCK_SIZE);
           ^~~~~~
   crypto/ghash-generic.c:138:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
           memcpy(dst, buf, GHASH_BLOCK_SIZE);
           ^~~~~~
   Suppressed 9 warnings (9 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   17 warnings generated.
   Suppressed 17 warnings (17 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   28 warnings generated.
   Suppressed 28 warnings (28 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   46 warnings generated.
>> arch/riscv/kernel/probes/uprobes.c:168:16: warning: Value stored to 'addr' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
           unsigned long addr = (unsigned long)dst;
                         ^~~~   ~~~~~~~~~~~~~~~~~~
   arch/riscv/kernel/probes/uprobes.c:168:16: note: Value stored to 'addr' during its initialization is never read
           unsigned long addr = (unsigned long)dst;
                         ^~~~   ~~~~~~~~~~~~~~~~~~
   arch/riscv/kernel/probes/uprobes.c:170:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memcpy(dst, src, len);
           ^~~~~~
   arch/riscv/kernel/probes/uprobes.c:170:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
           memcpy(dst, src, len);
           ^~~~~~
   Suppressed 44 warnings (43 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   31 warnings generated.
   Suppressed 31 warnings (31 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   31 warnings generated.
   Suppressed 31 warnings (31 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   28 warnings generated.
   Suppressed 28 warnings (28 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   29 warnings generated.
   arch/riscv/kernel/module.c:190:12: warning: Value stored to 'offset' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
           ptrdiff_t offset = (void *)v - (void *)location;
                     ^~~~~~   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/kernel/module.c:190:12: note: Value stored to 'offset' during its initialization is never read
           ptrdiff_t offset = (void *)v - (void *)location;
                     ^~~~~~   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   Suppressed 28 warnings (28 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   9 warnings generated.
   Suppressed 9 warnings (9 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   31 warnings generated.
   Suppressed 31 warnings (31 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   31 warnings generated.
   Suppressed 31 warnings (31 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   31 warnings generated.
   Suppressed 31 warnings (31 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   58 warnings generated.
   fs/btrfs/file.c:287:2: warning: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memset(&range, 0, sizeof(range));
           ^~~~~~
   fs/btrfs/file.c:287:2: note: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11
           memset(&range, 0, sizeof(range));
           ^~~~~~
   fs/btrfs/file.c:528:3: warning: Value stored to 'modified' is never read [clang-analyzer-deadcode.DeadStores]
                   modified = false;
                   ^          ~~~~~
   fs/btrfs/file.c:528:3: note: Value stored to 'modified' is never read
                   modified = false;
                   ^          ~~~~~
   fs/btrfs/file.c:816:4: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
                           memcpy(&new_key, &key, sizeof(new_key));
                           ^~~~~~
   fs/btrfs/file.c:816:4: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
                           memcpy(&new_key, &key, sizeof(new_key));
                           ^~~~~~
   fs/btrfs/file.c:872:4: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
                           memcpy(&new_key, &key, sizeof(new_key));
                           ^~~~~~
   fs/btrfs/file.c:872:4: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
                           memcpy(&new_key, &key, sizeof(new_key));
                           ^~~~~~
   fs/btrfs/file.c:1121:2: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memcpy(&new_key, &key, sizeof(new_key));
           ^~~~~~
   fs/btrfs/file.c:1121:2: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
           memcpy(&new_key, &key, sizeof(new_key));
           ^~~~~~
   fs/btrfs/file.c:1379:5: warning: Value stored to 'err' is never read [clang-analyzer-deadcode.DeadStores]
                                   err = 0;
                                   ^     ~
   fs/btrfs/file.c:1379:5: note: Value stored to 'err' is never read
                                   err = 0;
                                   ^     ~
   fs/btrfs/file.c:2897:19: warning: Access to field 'block_rsv' results in a dereference of a null pointer (loaded from variable 'trans') [clang-analyzer-core.NullDereference]
           trans->block_rsv = &fs_info->trans_block_rsv;
           ~~~~~            ^
   fs/btrfs/file.c:2738:6: note: Assuming 'end' is > 'start'
           if (end <= start)
               ^~~~~~~~~~~~
   fs/btrfs/file.c:2738:2: note: Taking false branch
           if (end <= start)
           ^
   fs/btrfs/file.c:2742:6: note: Assuming 'rsv' is non-null
           if (!rsv) {
               ^~~~
   fs/btrfs/file.c:2742:2: note: Taking false branch
           if (!rsv) {
           ^
   fs/btrfs/file.c:2755:6: note: Left side of '||' is false
           if (!btrfs_fs_incompat(fs_info, NO_HOLES) || extent_info)
               ^
   fs/btrfs/file.c:2755:6: note: Assuming pointer value is null
           if (!btrfs_fs_incompat(fs_info, NO_HOLES) || extent_info)

vim +/addr +168 arch/riscv/kernel/probes/uprobes.c

74784081aac8a0 Guo Ren    2020-12-17  161  
74784081aac8a0 Guo Ren    2020-12-17  162  void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
74784081aac8a0 Guo Ren    2020-12-17  163  			   void *src, unsigned long len)
74784081aac8a0 Guo Ren    2020-12-17  164  {
74784081aac8a0 Guo Ren    2020-12-17  165  	/* Initialize the slot */
74784081aac8a0 Guo Ren    2020-12-17  166  	void *kaddr = kmap_atomic(page);
74784081aac8a0 Guo Ren    2020-12-17  167  	void *dst = kaddr + (vaddr & ~PAGE_MASK);
363d56cc11ac60 Po-Kai Chi 2022-05-18 @168  	unsigned long addr = (unsigned long)dst;

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH] riscv: Invalid instruction cache after copy the xol area

[Zong Li]

> On Wed, 18 May 2022 01:17:53 PDT (-0700), po-kai.chi@sifive.com wrote:
> > We need to invalid the relevant instruction cache after
> > copying the xol area, to ensure the changes takes effect.
> >
> > Signed-off-by: Po-Kai Chi <po-kai.chi@sifive.com>
> > ---
> >  arch/riscv/kernel/probes/uprobes.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
> > index 7a057b5f0adc..9d52beeac73c 100644
> > --- a/arch/riscv/kernel/probes/uprobes.c
> > +++ b/arch/riscv/kernel/probes/uprobes.c
> > @@ -165,6 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >       /* Initialize the slot */
> >       void *kaddr = kmap_atomic(page);
> >       void *dst = kaddr + (vaddr & ~PAGE_MASK);
> > +     unsigned long addr = (unsigned long)dst;
> >
> >       memcpy(dst, src, len);
> >
> > @@ -177,10 +178,9 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >       kunmap_atomic(kaddr);
> >
> >       /*
> > -      * We probably need flush_icache_user_page() but it needs vma.
> > -      * This should work on most of architectures by default. If
> > -      * architecture needs to do something different it can define
> > -      * its own version of the function.
> > +      * Flush both I/D cache to ensure instruction modification
> > +      * takes effect.
> >        */
> >       flush_dcache_page(page);
> > +     flush_icache_range(addr, addr + len);
> >  }
>
> This brings up a handful of issues:
>
> * This always inserts a 32-bit breakpoint, but that's not quite correct.
>   This should really be checking the _next_ instruction as well to
>   insert a 16-bit breakpoint if it's a 16-bit instruction as otherwise
>   userspace might jump into the middle of the breakpoint.
> * These instructions can be concurrently executing, which relies on some
>   instruction fetch ordering that's very lightly specified.  We don't
>   rely on that elsewhere (see stop_machine() in kprobes), but we
>   probably should.  It's probably worth adding something to probe the HW
>   to make sure this is supported.
> * Adding the icache flush defeats a uprobes advantage in that we'll now
>   be triggering remote execution (to do the remote fence.i).  One option
>   could be to defer the fence and wait on it, but not sure if that's
>   sane and it'd likely require a lot of
>
> This also leaves a bit undefined WRT icache aliasing, at least in terms
> of the API used.  IMO it'd be
>
>     diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
>     index 9d52beeac73c..c857346864fc 100644
>     --- a/arch/riscv/kernel/probes/uprobes.c
>     +++ b/arch/riscv/kernel/probes/uprobes.c
>     @@ -165,7 +165,6 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>         /* Initialize the slot */
>         void *kaddr = kmap_atomic(page);
>         void *dst = kaddr + (vaddr & ~PAGE_MASK);
>     -   unsigned long addr = (unsigned long)dst;
>
>         memcpy(dst, src, len);
>
>     @@ -179,8 +178,10 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>
>         /*
>          * Flush both I/D cache to ensure instruction modification
>     -    * takes effect.
>     +    * takes effect.  We don't need to flush the whole icache, but that's
>     +    * all RISC-V defines so rather than worry about aliasing this just
>     +    * flushes everything.
>          */
>         flush_dcache_page(page);
>     -   flush_icache_range(addr, addr + len);
>     +   flush_icache_all();
>      }
>
> which will end up doing the same thing but avoids the ambiguity.  I went
> ahead and put this at palmer/riscv-uprobe_fencei with that and some
> other minor things fixed up, LMK if that looks OK and I'll take it on
> fixes.
>

Hi Palmer,
Could I know if you have plans to include this fix in the upcoming RC
versions? Thanks

> Thanks!

Re: [PATCH] riscv: Invalid instruction cache after copy the xol area

[Zong Li]

> On Wed, 18 May 2022 01:17:53 PDT (-0700), po-kai.chi@sifive.com wrote:
> > We need to invalid the relevant instruction cache after
> > copying the xol area, to ensure the changes takes effect.
> >
> > Signed-off-by: Po-Kai Chi <po-kai.chi@sifive.com>
> > ---
> >  arch/riscv/kernel/probes/uprobes.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
> > index 7a057b5f0adc..9d52beeac73c 100644
> > --- a/arch/riscv/kernel/probes/uprobes.c
> > +++ b/arch/riscv/kernel/probes/uprobes.c
> > @@ -165,6 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >       /* Initialize the slot */
> >       void *kaddr = kmap_atomic(page);
> >       void *dst = kaddr + (vaddr & ~PAGE_MASK);
> > +     unsigned long addr = (unsigned long)dst;
> >
> >       memcpy(dst, src, len);
> >
> > @@ -177,10 +178,9 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >       kunmap_atomic(kaddr);
> >
> >       /*
> > -      * We probably need flush_icache_user_page() but it needs vma.
> > -      * This should work on most of architectures by default. If
> > -      * architecture needs to do something different it can define
> > -      * its own version of the function.
> > +      * Flush both I/D cache to ensure instruction modification
> > +      * takes effect.
> >        */
> >       flush_dcache_page(page);
> > +     flush_icache_range(addr, addr + len);
> >  }
>
> This brings up a handful of issues:
>
> * This always inserts a 32-bit breakpoint, but that's not quite correct.
>   This should really be checking the _next_ instruction as well to
>   insert a 16-bit breakpoint if it's a 16-bit instruction as otherwise
>   userspace might jump into the middle of the breakpoint.
> * These instructions can be concurrently executing, which relies on some
>   instruction fetch ordering that's very lightly specified.  We don't
>   rely on that elsewhere (see stop_machine() in kprobes), but we
>   probably should.  It's probably worth adding something to probe the HW
>   to make sure this is supported.
> * Adding the icache flush defeats a uprobes advantage in that we'll now
>   be triggering remote execution (to do the remote fence.i).  One option
>   could be to defer the fence and wait on it, but not sure if that's
>   sane and it'd likely require a lot of
>
> This also leaves a bit undefined WRT icache aliasing, at least in terms
> of the API used.  IMO it'd be
>
>     diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
>     index 9d52beeac73c..c857346864fc 100644
>     --- a/arch/riscv/kernel/probes/uprobes.c
>     +++ b/arch/riscv/kernel/probes/uprobes.c
>     @@ -165,7 +165,6 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>         /* Initialize the slot */
>         void *kaddr = kmap_atomic(page);
>         void *dst = kaddr + (vaddr & ~PAGE_MASK);
>     -   unsigned long addr = (unsigned long)dst;
>
>         memcpy(dst, src, len);
>
>     @@ -179,8 +178,10 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>
>         /*
>          * Flush both I/D cache to ensure instruction modification
>     -    * takes effect.
>     +    * takes effect.  We don't need to flush the whole icache, but that's
>     +    * all RISC-V defines so rather than worry about aliasing this just
>     +    * flushes everything.
>          */
>         flush_dcache_page(page);
>     -   flush_icache_range(addr, addr + len);
>     +   flush_icache_all();
>      }
>
> which will end up doing the same thing but avoids the ambiguity.  I went
> ahead and put this at palmer/riscv-uprobe_fencei with that and some
> other minor things fixed up, LMK if that looks OK and I'll take it on
> fixes.
>

Hi Palmer,
Could I know if you have plans to include this fix in the upcoming RC
versions? Thanks

> Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Invalid instruction cache after copy the xol area

[Po-Kai Chi]

Hello Palmer,

Replacing flush_icache_range() with flush_icache_all() looks good to me.
And I think this is the necessary evil because we don't know whether
hardware is performing speculative access to the XOL area or not.


On Fri, Jun 17, 2022 at 6:04 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 18 May 2022 01:17:53 PDT (-0700), po-kai.chi@sifive.com wrote:
> > We need to invalid the relevant instruction cache after
> > copying the xol area, to ensure the changes takes effect.
> >
> > Signed-off-by: Po-Kai Chi <po-kai.chi@sifive.com>
> > ---
> >  arch/riscv/kernel/probes/uprobes.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
> > index 7a057b5f0adc..9d52beeac73c 100644
> > --- a/arch/riscv/kernel/probes/uprobes.c
> > +++ b/arch/riscv/kernel/probes/uprobes.c
> > @@ -165,6 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >       /* Initialize the slot */
> >       void *kaddr = kmap_atomic(page);
> >       void *dst = kaddr + (vaddr & ~PAGE_MASK);
> > +     unsigned long addr = (unsigned long)dst;
> >
> >       memcpy(dst, src, len);
> >
> > @@ -177,10 +178,9 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >       kunmap_atomic(kaddr);
> >
> >       /*
> > -      * We probably need flush_icache_user_page() but it needs vma.
> > -      * This should work on most of architectures by default. If
> > -      * architecture needs to do something different it can define
> > -      * its own version of the function.
> > +      * Flush both I/D cache to ensure instruction modification
> > +      * takes effect.
> >        */
> >       flush_dcache_page(page);
> > +     flush_icache_range(addr, addr + len);
> >  }
>
> This brings up a handful of issues:
>
> * This always inserts a 32-bit breakpoint, but that's not quite correct.
>   This should really be checking the _next_ instruction as well to
>   insert a 16-bit breakpoint if it's a 16-bit instruction as otherwise
>   userspace might jump into the middle of the breakpoint.
> * These instructions can be concurrently executing, which relies on some
>   instruction fetch ordering that's very lightly specified.  We don't
>   rely on that elsewhere (see stop_machine() in kprobes), but we
>   probably should.  It's probably worth adding something to probe the HW
>   to make sure this is supported.
> * Adding the icache flush defeats a uprobes advantage in that we'll now
>   be triggering remote execution (to do the remote fence.i).  One option
>   could be to defer the fence and wait on it, but not sure if that's
>   sane and it'd likely require a lot of
>
> This also leaves a bit undefined WRT icache aliasing, at least in terms
> of the API used.  IMO it'd be
>
>     diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
>     index 9d52beeac73c..c857346864fc 100644
>     --- a/arch/riscv/kernel/probes/uprobes.c
>     +++ b/arch/riscv/kernel/probes/uprobes.c
>     @@ -165,7 +165,6 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>         /* Initialize the slot */
>         void *kaddr = kmap_atomic(page);
>         void *dst = kaddr + (vaddr & ~PAGE_MASK);
>     -   unsigned long addr = (unsigned long)dst;
>
>         memcpy(dst, src, len);
>
>     @@ -179,8 +178,10 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>
>         /*
>          * Flush both I/D cache to ensure instruction modification
>     -    * takes effect.
>     +    * takes effect.  We don't need to flush the whole icache, but that's
>     +    * all RISC-V defines so rather than worry about aliasing this just
>     +    * flushes everything.
>          */
>         flush_dcache_page(page);
>     -   flush_icache_range(addr, addr + len);
>     +   flush_icache_all();
>      }
>
> which will end up doing the same thing but avoids the ambiguity.  I went
> ahead and put this at palmer/riscv-uprobe_fencei with that and some
> other minor things fixed up, LMK if that looks OK and I'll take it on
> fixes.
>
> Thanks!



-- 
BR,
Po-Kai Chi

Re: [PATCH] riscv: Invalid instruction cache after copy the xol area

[Po-Kai Chi]

Hello Palmer,

Replacing flush_icache_range() with flush_icache_all() looks good to me.
And I think this is the necessary evil because we don't know whether
hardware is performing speculative access to the XOL area or not.


On Fri, Jun 17, 2022 at 6:04 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 18 May 2022 01:17:53 PDT (-0700), po-kai.chi@sifive.com wrote:
> > We need to invalid the relevant instruction cache after
> > copying the xol area, to ensure the changes takes effect.
> >
> > Signed-off-by: Po-Kai Chi <po-kai.chi@sifive.com>
> > ---
> >  arch/riscv/kernel/probes/uprobes.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
> > index 7a057b5f0adc..9d52beeac73c 100644
> > --- a/arch/riscv/kernel/probes/uprobes.c
> > +++ b/arch/riscv/kernel/probes/uprobes.c
> > @@ -165,6 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >       /* Initialize the slot */
> >       void *kaddr = kmap_atomic(page);
> >       void *dst = kaddr + (vaddr & ~PAGE_MASK);
> > +     unsigned long addr = (unsigned long)dst;
> >
> >       memcpy(dst, src, len);
> >
> > @@ -177,10 +178,9 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
> >       kunmap_atomic(kaddr);
> >
> >       /*
> > -      * We probably need flush_icache_user_page() but it needs vma.
> > -      * This should work on most of architectures by default. If
> > -      * architecture needs to do something different it can define
> > -      * its own version of the function.
> > +      * Flush both I/D cache to ensure instruction modification
> > +      * takes effect.
> >        */
> >       flush_dcache_page(page);
> > +     flush_icache_range(addr, addr + len);
> >  }
>
> This brings up a handful of issues:
>
> * This always inserts a 32-bit breakpoint, but that's not quite correct.
>   This should really be checking the _next_ instruction as well to
>   insert a 16-bit breakpoint if it's a 16-bit instruction as otherwise
>   userspace might jump into the middle of the breakpoint.
> * These instructions can be concurrently executing, which relies on some
>   instruction fetch ordering that's very lightly specified.  We don't
>   rely on that elsewhere (see stop_machine() in kprobes), but we
>   probably should.  It's probably worth adding something to probe the HW
>   to make sure this is supported.
> * Adding the icache flush defeats a uprobes advantage in that we'll now
>   be triggering remote execution (to do the remote fence.i).  One option
>   could be to defer the fence and wait on it, but not sure if that's
>   sane and it'd likely require a lot of
>
> This also leaves a bit undefined WRT icache aliasing, at least in terms
> of the API used.  IMO it'd be
>
>     diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
>     index 9d52beeac73c..c857346864fc 100644
>     --- a/arch/riscv/kernel/probes/uprobes.c
>     +++ b/arch/riscv/kernel/probes/uprobes.c
>     @@ -165,7 +165,6 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>         /* Initialize the slot */
>         void *kaddr = kmap_atomic(page);
>         void *dst = kaddr + (vaddr & ~PAGE_MASK);
>     -   unsigned long addr = (unsigned long)dst;
>
>         memcpy(dst, src, len);
>
>     @@ -179,8 +178,10 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>
>         /*
>          * Flush both I/D cache to ensure instruction modification
>     -    * takes effect.
>     +    * takes effect.  We don't need to flush the whole icache, but that's
>     +    * all RISC-V defines so rather than worry about aliasing this just
>     +    * flushes everything.
>          */
>         flush_dcache_page(page);
>     -   flush_icache_range(addr, addr + len);
>     +   flush_icache_all();
>      }
>
> which will end up doing the same thing but avoids the ambiguity.  I went
> ahead and put this at palmer/riscv-uprobe_fencei with that and some
> other minor things fixed up, LMK if that looks OK and I'll take it on
> fixes.
>
> Thanks!



-- 
BR,
Po-Kai Chi

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Invalid instruction cache after copy the xol area

[Palmer Dabbelt]

On Wed, 18 May 2022 01:17:53 PDT (-0700), po-kai.chi@sifive.com wrote:
> We need to invalid the relevant instruction cache after
> copying the xol area, to ensure the changes takes effect.
>
> Signed-off-by: Po-Kai Chi <po-kai.chi@sifive.com>
> ---
>  arch/riscv/kernel/probes/uprobes.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
> index 7a057b5f0adc..9d52beeac73c 100644
> --- a/arch/riscv/kernel/probes/uprobes.c
> +++ b/arch/riscv/kernel/probes/uprobes.c
> @@ -165,6 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>  	/* Initialize the slot */
>  	void *kaddr = kmap_atomic(page);
>  	void *dst = kaddr + (vaddr & ~PAGE_MASK);
> +	unsigned long addr = (unsigned long)dst;
>
>  	memcpy(dst, src, len);
>
> @@ -177,10 +178,9 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>  	kunmap_atomic(kaddr);
>
>  	/*
> -	 * We probably need flush_icache_user_page() but it needs vma.
> -	 * This should work on most of architectures by default. If
> -	 * architecture needs to do something different it can define
> -	 * its own version of the function.
> +	 * Flush both I/D cache to ensure instruction modification
> +	 * takes effect.
>  	 */
>  	flush_dcache_page(page);
> +	flush_icache_range(addr, addr + len);
>  }

This brings up a handful of issues:

* This always inserts a 32-bit breakpoint, but that's not quite correct.  
  This should really be checking the _next_ instruction as well to 
  insert a 16-bit breakpoint if it's a 16-bit instruction as otherwise 
  userspace might jump into the middle of the breakpoint.
* These instructions can be concurrently executing, which relies on some 
  instruction fetch ordering that's very lightly specified.  We don't 
  rely on that elsewhere (see stop_machine() in kprobes), but we 
  probably should.  It's probably worth adding something to probe the HW 
  to make sure this is supported.
* Adding the icache flush defeats a uprobes advantage in that we'll now 
  be triggering remote execution (to do the remote fence.i).  One option 
  could be to defer the fence and wait on it, but not sure if that's 
  sane and it'd likely require a lot of 

This also leaves a bit undefined WRT icache aliasing, at least in terms 
of the API used.  IMO it'd be

    diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
    index 9d52beeac73c..c857346864fc 100644
    --- a/arch/riscv/kernel/probes/uprobes.c
    +++ b/arch/riscv/kernel/probes/uprobes.c
    @@ -165,7 +165,6 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
     	/* Initialize the slot */
     	void *kaddr = kmap_atomic(page);
     	void *dst = kaddr + (vaddr & ~PAGE_MASK);
    -	unsigned long addr = (unsigned long)dst;
    
     	memcpy(dst, src, len);
    
    @@ -179,8 +178,10 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
    
     	/*
     	 * Flush both I/D cache to ensure instruction modification
    -	 * takes effect.
    +	 * takes effect.  We don't need to flush the whole icache, but that's
    +	 * all RISC-V defines so rather than worry about aliasing this just
    +	 * flushes everything.
     	 */
     	flush_dcache_page(page);
    -	flush_icache_range(addr, addr + len);
    +	flush_icache_all();
     }

which will end up doing the same thing but avoids the ambiguity.  I went 
ahead and put this at palmer/riscv-uprobe_fencei with that and some 
other minor things fixed up, LMK if that looks OK and I'll take it on 
fixes.

Thanks!

Re: [PATCH] riscv: Invalid instruction cache after copy the xol area

[Palmer Dabbelt]

On Wed, 18 May 2022 01:17:53 PDT (-0700), po-kai.chi@sifive.com wrote:
> We need to invalid the relevant instruction cache after
> copying the xol area, to ensure the changes takes effect.
>
> Signed-off-by: Po-Kai Chi <po-kai.chi@sifive.com>
> ---
>  arch/riscv/kernel/probes/uprobes.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
> index 7a057b5f0adc..9d52beeac73c 100644
> --- a/arch/riscv/kernel/probes/uprobes.c
> +++ b/arch/riscv/kernel/probes/uprobes.c
> @@ -165,6 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>  	/* Initialize the slot */
>  	void *kaddr = kmap_atomic(page);
>  	void *dst = kaddr + (vaddr & ~PAGE_MASK);
> +	unsigned long addr = (unsigned long)dst;
>
>  	memcpy(dst, src, len);
>
> @@ -177,10 +178,9 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>  	kunmap_atomic(kaddr);
>
>  	/*
> -	 * We probably need flush_icache_user_page() but it needs vma.
> -	 * This should work on most of architectures by default. If
> -	 * architecture needs to do something different it can define
> -	 * its own version of the function.
> +	 * Flush both I/D cache to ensure instruction modification
> +	 * takes effect.
>  	 */
>  	flush_dcache_page(page);
> +	flush_icache_range(addr, addr + len);
>  }

This brings up a handful of issues:

* This always inserts a 32-bit breakpoint, but that's not quite correct.  
  This should really be checking the _next_ instruction as well to 
  insert a 16-bit breakpoint if it's a 16-bit instruction as otherwise 
  userspace might jump into the middle of the breakpoint.
* These instructions can be concurrently executing, which relies on some 
  instruction fetch ordering that's very lightly specified.  We don't 
  rely on that elsewhere (see stop_machine() in kprobes), but we 
  probably should.  It's probably worth adding something to probe the HW 
  to make sure this is supported.
* Adding the icache flush defeats a uprobes advantage in that we'll now 
  be triggering remote execution (to do the remote fence.i).  One option 
  could be to defer the fence and wait on it, but not sure if that's 
  sane and it'd likely require a lot of 

This also leaves a bit undefined WRT icache aliasing, at least in terms 
of the API used.  IMO it'd be

    diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
    index 9d52beeac73c..c857346864fc 100644
    --- a/arch/riscv/kernel/probes/uprobes.c
    +++ b/arch/riscv/kernel/probes/uprobes.c
    @@ -165,7 +165,6 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
     	/* Initialize the slot */
     	void *kaddr = kmap_atomic(page);
     	void *dst = kaddr + (vaddr & ~PAGE_MASK);
    -	unsigned long addr = (unsigned long)dst;
    
     	memcpy(dst, src, len);
    
    @@ -179,8 +178,10 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
    
     	/*
     	 * Flush both I/D cache to ensure instruction modification
    -	 * takes effect.
    +	 * takes effect.  We don't need to flush the whole icache, but that's
    +	 * all RISC-V defines so rather than worry about aliasing this just
    +	 * flushes everything.
     	 */
     	flush_dcache_page(page);
    -	flush_icache_range(addr, addr + len);
    +	flush_icache_all();
     }

which will end up doing the same thing but avoids the ambiguity.  I went 
ahead and put this at palmer/riscv-uprobe_fencei with that and some 
other minor things fixed up, LMK if that looks OK and I'll take it on 
fixes.

Thanks!

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH] riscv: Invalid instruction cache after copy the xol area

[Po-Kai Chi]

We need to invalid the relevant instruction cache after
copying the xol area, to ensure the changes takes effect.

Signed-off-by: Po-Kai Chi <po-kai.chi@sifive.com>
---
 arch/riscv/kernel/probes/uprobes.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
index 7a057b5f0adc..9d52beeac73c 100644
--- a/arch/riscv/kernel/probes/uprobes.c
+++ b/arch/riscv/kernel/probes/uprobes.c
@@ -165,6 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 	/* Initialize the slot */
 	void *kaddr = kmap_atomic(page);
 	void *dst = kaddr + (vaddr & ~PAGE_MASK);
+	unsigned long addr = (unsigned long)dst;
 
 	memcpy(dst, src, len);
 
@@ -177,10 +178,9 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 	kunmap_atomic(kaddr);
 
 	/*
-	 * We probably need flush_icache_user_page() but it needs vma.
-	 * This should work on most of architectures by default. If
-	 * architecture needs to do something different it can define
-	 * its own version of the function.
+	 * Flush both I/D cache to ensure instruction modification
+	 * takes effect.
 	 */
 	flush_dcache_page(page);
+	flush_icache_range(addr, addr + len);
 }
-- 
2.36.1

[PATCH] riscv: Invalid instruction cache after copy the xol area

[Po-Kai Chi]

We need to invalid the relevant instruction cache after
copying the xol area, to ensure the changes takes effect.

Signed-off-by: Po-Kai Chi <po-kai.chi@sifive.com>
---
 arch/riscv/kernel/probes/uprobes.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
index 7a057b5f0adc..9d52beeac73c 100644
--- a/arch/riscv/kernel/probes/uprobes.c
+++ b/arch/riscv/kernel/probes/uprobes.c
@@ -165,6 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 	/* Initialize the slot */
 	void *kaddr = kmap_atomic(page);
 	void *dst = kaddr + (vaddr & ~PAGE_MASK);
+	unsigned long addr = (unsigned long)dst;
 
 	memcpy(dst, src, len);
 
@@ -177,10 +178,9 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
 	kunmap_atomic(kaddr);
 
 	/*
-	 * We probably need flush_icache_user_page() but it needs vma.
-	 * This should work on most of architectures by default. If
-	 * architecture needs to do something different it can define
-	 * its own version of the function.
+	 * Flush both I/D cache to ensure instruction modification
+	 * takes effect.
 	 */
 	flush_dcache_page(page);
+	flush_icache_range(addr, addr + len);
 }
-- 
2.36.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv