On 28.07.2023 09:59, Alejandro Vallejo wrote:
> --- a/xen/common/pdx.c
> +++ b/xen/common/pdx.c
> @@ -31,11 +31,15 @@ unsigned long __read_mostly pdx_group_valid[BITS_TO_LONGS(
>
> bool __mfn_valid(unsigned long mfn)
> {
> - if ( unlikely(evaluate_nospec(mfn >= max_page)) )
> + bool invalid = mfn >= max_page;
> +#ifdef CONFIG_PDX_COMPRESSION
> + invalid |= mfn & pfn_hole_mask;
> +#endif
Nit: Declaration(s) and statement(s) separated by a blank line please.
> @@ -49,6 +53,8 @@ void set_pdx_range(unsigned long smfn, unsigned long emfn)
> __set_bit(idx, pdx_group_valid);
> }
>
> +#ifdef CONFIG_PDX_COMPRESSION
> +
> /*
> * Diagram to make sense of the following variables. The masks and shifts
> * are done on mfn values in order to convert to/from pdx:
Nit: With a blank line after #ifdef,
> @@ -175,6 +181,7 @@ void __init pfn_pdx_hole_setup(unsigned long mask)
> pfn_top_mask = ~(pfn_pdx_bottom_mask | pfn_hole_mask);
> ma_top_mask = pfn_top_mask << PAGE_SHIFT;
> }
> +#endif /* CONFIG_PDX_COMPRESSION */
>
>
> /*
... we would typically also have one before #endif. In the case here
you could even leverage that there are already (wrongly) two consecutive
blank lines.
> @@ -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 */
> +
> +/* Without PDX compression we can skip some computations */
Same here for the #else then.
Jan
