Re: [PATCH] target/riscv: reduce overhead of MSTATUS_SUM change

[LIU Zhiwei]

On 2023/3/22 0:10, Richard Henderson wrote:
> On 3/20/23 23:37, fei2.wu@intel.com wrote:
> > From: Fei Wu <fei2.wu@intel.com>
> >
> > Kernel needs to access user mode memory e.g. during syscalls, the window
> > is usually opened up for a very limited time through MSTATUS.SUM, the
> > overhead is too much if tlb_flush() gets called for every SUM change.
> > This patch saves addresses accessed when SUM=1, and flushs only these
> > pages when SUM changes to 0. If the buffer is not large enough to save
> > all the pages during SUM=1, it will fall back to tlb_flush when
> > necessary.
> >
> > The buffer size is set to 4 since in this MSTATUS.SUM open-up window,
> > most of the time kernel accesses 1 or 2 pages, it's very rare to see
> > more than 4 pages accessed.
> >
> > It's not necessary to save/restore these new added status, as
> > tlb_flush() is always called after restore.
> >
> > Result of 'pipe 10' from unixbench boosts from 223656 to 1327407. Many
> > other syscalls benefit a lot from this one too.
>
> This is not the correct approach.
>
> You should be making use of different softmmu indexes, similar to how 
> ARM uses a separate index for PAN (privileged access never) mode.  If 
> I read the manual properly, PAN == !SUM.
>
> When you do this, you need no additional flushing.

Hi Fei,

Let's follow Richard's advice.

Zhiwei

> r~