Re: [PATCH v3 01/12] x86/mm: Avoid hard-coding PAT in get_page_from_l1e()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.12.2022 00:11, Demi Marie Obenour wrote:
> get_page_from_l1e() relied on Xen's choice of PAT, which is brittle in
> the face of future PAT changes.  Use the proper _PAGE_* constants
> instead.  Also, treat the two unused cases as if they are cacheable, as
> future changes may make them cacheable.

This still does not cover ...

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -959,14 +959,19 @@ get_page_from_l1e(
>              flip = _PAGE_RW;
>          }
>  
> +        /* Force cacheable memtypes to UC */
>          switch ( l1f & PAGE_CACHE_ATTRS )
>          {
> -        case 0: /* WB */
> -            flip |= _PAGE_PWT | _PAGE_PCD;
> +        case _PAGE_UC:
> +        case _PAGE_UCM:
> +        case _PAGE_WC:
> +            /* not cached */
>              break;
> -        case _PAGE_PWT: /* WT */
> -        case _PAGE_PWT | _PAGE_PAT: /* WP */
> -            flip |= _PAGE_PCD | (l1f & _PAGE_PAT);
> +        case _PAGE_WB:
> +        case _PAGE_WT:
> +        case _PAGE_WP:
> +        default:
> +            flip |= (l1f & PAGE_CACHE_ATTRS) ^ _PAGE_UC;
>              break;

... the three cases here assuming certain properties wrt the flipping of
_PAGE_UC. As said before - going from one kind of assumption to another
_may_ be a good thing to do, but needs justifying as actually being an
improvement. Alternatively such assumptions could be checked by suitable
BUILD_BUG_ON(), which then at the same serve as documentation thereof.

Jan