On 06.12.2022 05:33, Demi Marie Obenour wrote:
> This still hard-codes the assumption that the two spare values are
> mapped to UC. Removing this assumption would require a more complex
> patch.
>
> Signed-off-by: Demi Marie Obenour <d...@invisiblethingslab.com>
You eliminate some assumptions at the price of introducing new ones.
With that I consider the description insufficient; really there isn't
any description here, just a statement on something you do _not_ do.
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -961,13 +961,10 @@ get_page_from_l1e(
>
> switch ( l1f & PAGE_CACHE_ATTRS )
> {
> - case 0: /* WB */
> - flip |= _PAGE_PWT | _PAGE_PCD;
> - break;
> - case _PAGE_PWT: /* WT */
> - case _PAGE_PWT | _PAGE_PAT: /* WP */
> - flip |= _PAGE_PCD | (l1f & _PAGE_PAT);
> - break;
> + case _PAGE_WB:
> + case _PAGE_WT:
> + case _PAGE_WP:
> + flip |= (l1f & PAGE_CACHE_ATTRS) ^ _PAGE_UC;
> }
Please never drop "break" from case blocks, even if it's (at a given
point in time) the last in its switch(). That's a recipe for someone
else then forgetting to add it when another case block is added.
Jan
