Re: [PATCH v2 5/5] pdx: Add CONFIG_HAS_PDX_COMPRESSION as a common Kconfig option

[Julien Grall <julien@xxxxxxx>]

On 28/07/2023 17:36, Andrew Cooper wrote:
> On 28/07/2023 8:59 am, Alejandro Vallejo wrote:
> > Adds a new compile-time flag to allow disabling pdx compression and
> > compiles out compression-related code/data. It also shorts the pdx<->pfn
> > conversion macros and creates stubs for masking fucntions.
> >
> > While at it, removes the old arch-defined CONFIG_HAS_PDX flag, as it was
> > not removable in practice.
> >
> > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> > ---
> > v2:
> >    * Merged v1/patch2: Removal of CONFIG_HAS_PDX here (Jan)
> This series is now looking fine, except for the Kconfig aspect.
>
> This is not something any user or developer should ever be queried
> about.  The feedback on the documentation patches alone show that it's
> not understood well by the maintainers, even if the principle is accepted.
>
> There is never any reason to have this active on x86. 
I can't really speak for x86. However, for Arm, I think it could be 
useful in the case there is a system where the frametable size doesn't 
change with CONFIG_PDX=n and/or the integrator is happy to waste a bit 
more memory to gain performance.
Also, to reply to Jan's, I don't think this is a sort of option that 
should be gated by expert (againt at least on Arm). Xen ought to work 
fine with and without PDX enabled. From my understanding, the only 
differences are the amount of memory wasted vs performance gain.
Cheers,

--
Julien Grall