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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 28/07/2023 08:59, Alejandro Vallejo wrote:
> diff --git a/xen/include/xen/pdx.h b/xen/include/xen/pdx.h
> index 5a82b6bde2..dfb475c8dc 100644
> --- a/xen/include/xen/pdx.h
> +++ b/xen/include/xen/pdx.h
> @@ -67,8 +67,6 @@
>    * region involved.
>    */
>   
> -#ifdef CONFIG_HAS_PDX
> -
>   extern unsigned long max_pdx;
>   
>   #define PDX_GROUP_COUNT ((1 << PDX_GROUP_SHIFT) / \
> @@ -100,6 +98,8 @@ bool __mfn_valid(unsigned long mfn);
>   #define mfn_to_pdx(mfn) pfn_to_pdx(mfn_x(mfn))
>   #define pdx_to_mfn(pdx) _mfn(pdx_to_pfn(pdx))
>   
> +#ifdef CONFIG_PDX_COMPRESSION
> +
>   extern unsigned long pfn_pdx_bottom_mask, ma_va_bottom_mask;
>   extern unsigned int pfn_pdx_hole_shift;
>   extern unsigned long pfn_hole_mask;
> @@ -205,8 +205,39 @@ static inline uint64_t directmapoff_to_maddr(unsigned long 
> offset)
>    *             position marks a potentially compressible bit.
>    */
>   void pfn_pdx_hole_setup(unsigned long mask);
> +#else /* CONFIG_PDX_COMPRESSION */
Looking at other places, we tend to put the reason the #else be 
executed. In this case, it is !CONFIG_PDX_COMPRESSION.
Other than that, the changes looks good to me:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall