Re: [PATCH v1 2/5] xen/riscv: implement maddr_to_virt()

[oleksii.kurochko@xxxxxxxxx]

On Mon, 2024-10-21 at 08:56 +0100, Alejandro Vallejo wrote:
> On Fri Oct 18, 2024 at 2:17 PM BST, oleksii.kurochko wrote:
> > On Thu, 2024-10-17 at 16:55 +0200, Jan Beulich wrote:
> > > On 16.10.2024 11:15, Oleksii Kurochko wrote:
> > > > --- a/xen/arch/riscv/include/asm/mm.h
> > > > +++ b/xen/arch/riscv/include/asm/mm.h
> > > > @@ -25,8 +25,12 @@
> > > >  
> > > >  static inline void *maddr_to_virt(paddr_t ma)
> > > >  {
> > > > -    BUG_ON("unimplemented");
> > > > -    return NULL;
> > > > +    /* Offset in the direct map, accounting for pdx
> > > > compression */
> > > > +    unsigned long va_offset = maddr_to_directmapoff(ma);
> > > 
> > > Why the mentioning of PDX compression?
> > It was mentioned because if PDX will be enabled
> > maddr_to_directmapoff()
> > will take into account PDX stuff.
> > 
> > >  At least right now it's unavailable
> > > for RISC-V afaics. Are there plans to change that any time soon?
> > At the moment, I don't have such plans, looking at available
> > platform
> > there are no a lot of benefits of having PDX compression now.
> > 
> > Perhaps it would be good to add
> > BUILD_BUG_ON(IS_ENABLED(PDX_COMPRESSION)) for the places which
> > should
> > be updated when CONFIG_PDX will be enabled.
> > 
> > ~ Oleksii
> 
> I'd just forget about it unless you ever notice you're wasting a lot
> of entries
> in the frame table due to empty space in the memory map. Julien
> measured the
> effect on Amazon's Live Migration as a 10% improvement in downtime
> with PDX
> off.
> 
> PDX compression shines when you have separate RAM banks at very, very
> disparately far addresses (specifics in pdx.h). Unfortunately the
> flip side of
> this compression is that you get several memory accesses for each
> single
> pdx-(to/from)-mfn conversion. And we do a lot of those. One possible
> solution
> would be to alt-patch the values in the code-stream and avoid the
> perf-hit, but
> that's not merged. Jan had some patches but that didn't make it to
> staging,
> IIRC.
Could you please give me some links in the mailing list with mentioned
patches?

~ Oleksii