Re: [PATCH v4 2/7] xen/riscv: set up fixmap mappings

[oleksii.kurochko@xxxxxxxxx]

On Wed, 2024-08-14 at 17:08 +0200, Jan Beulich wrote:
> On 14.08.2024 16:21, oleksii.kurochko@xxxxxxxxx wrote:
> > On Tue, 2024-08-13 at 10:22 +0200, Jan Beulich wrote:
> > > On 09.08.2024 18:19, Oleksii Kurochko wrote:
> > > > --- a/xen/arch/riscv/include/asm/page.h
> > > > +++ b/xen/arch/riscv/include/asm/page.h
> > > > @@ -81,6 +81,12 @@ 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)
> > > > +{
> > > > +    *p = pte;
> > > > +}
> > > 
> > > No use of write_atomic() here? And no read_pte() counterpart
> > > right
> > > away (then
> > > also properly using read_atomic())?
> > I wanted to avoid the fence before "*p=pte" which in case of
> > write_atomic() will be present.
> 
> Well, this goes back to write_atomic() resolving to write{b,w,l,q}()
> for
> unclear reasons; even the comment in our atomic.h says "For some
> reason".
> The fence there is of course getting in the way here. I keep
> forgetting
> about that aspect, because neither x86 nor Arm has anything similar
> afaics.
Good point. I overlooked something here ( and misinterpreted your
comments regarding that write_atomic() implementation ) but I think it
would be better to use write{b,w,l,q}_cpu() for write_atomic.

~ Oleksii

> 
> > Won't it be enough to use here WRITE_ONCE()?
> 
> Maybe. I'm not entirely sure.
> 
> Jan