Re: [PATCH v3 4/9] xen/riscv: setup fixmap mapping

[oleksii.kurochko@xxxxxxxxx]

On Tue, 2024-07-30 at 14:11 +0200, Jan Beulich wrote:
> On 30.07.2024 13:25, oleksii.kurochko@xxxxxxxxx wrote:
> > On Tue, 2024-07-30 at 09:49 +0200, Jan Beulich wrote:
> > > On 29.07.2024 18:11, oleksii.kurochko@xxxxxxxxx wrote:
> > > > On Mon, 2024-07-29 at 15:35 +0200, Jan Beulich wrote:
> > > > > On 24.07.2024 17:31, Oleksii Kurochko wrote:
> > > > > > @@ -81,6 +82,14 @@ static inline void
> > > > > > flush_page_to_ram(unsigned
> > > > > > long mfn, bool sync_icache)
> > > > > >      BUG_ON("unimplemented");
> > > > > >  }
> > > > > >  
> > > > > > +/* Write a pagetable entry. */
> > > > > > +static inline void write_pte(pte_t *p, pte_t pte)
> > > > > > +{
> > > > > > +    RISCV_FENCE(rw, rw);
> > > > > > +    *p = pte;
> > > > > > +    RISCV_FENCE(rw, rw);
> > > > > > +}
> > > > > 
> > > > > Why the first of the two fences? 
> > > > To ensure that writes have completed with the old mapping.
> > > 
> > > Wait: There can certainly be uncompleted writes, but those must
> > > have
> > > walked the page tables already, or else a (synchronous) fault
> > > could
> > > not be delivered on the originating store instruction. Or am I
> > > misunderstanding how paging (and associated faults) work on RISC-
> > > V?
> > I am not sure that I correctly understand the part regarding (
> > synchronous ) fault. Could you please come up with an example?
> > 
> > If something during page table walk will go wrong then a fault will
> > be
> > raised.
> 
> On the very insn, with subsequent insns not having started executing
> (from an architectural perspective, i.e. leaving aside speculation).
> That is what my use of "synchronous" meant.
> 
> > My initial intension was to be sure if I will be writing to an
> > actively
> > in-use page table that other cores can see at the time then fences
> > above are required. It is not the case for now as we have only one
> > CPUs
> > but I assume that it will be a case when SMP will be enabled and
> > more
> > then one CPU will be able to work with the same page table.
> 
> Would that first fence really help there? The other CPU could use
> the page tables in the window between the fence and the write. My
> understanding of the need for fences is for them to be used at times
> where ordering of memory accesses matters. For the moment I don't
> see this as an aspect for the 1st fence here, but I may be
> overlooking something.
AFAIU there are cases when we need the first fence, i.e. when superpage
breakup is happening and IIUC in this case we would first do a fence (
assuming pages are already allocated ), one fence between each
superpage breakup and its PTE, and one after each regular PTE write.
But in this case the first fence could be outside write_pte() function.

So we could eliminate the first fence in write_pte() and place it
outside the function ( if it will be needed for some cases ), thus
eliminating double fences.

~ Oleksii