Re: [Xen-devel] [PATCH] x86/mm: free_page_type() is PV-only

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 5/10/19 3:12 PM, Jan Beulich wrote:
> While it already has a CONFIG_PV wrapped around its entire body, it is
> still uselessly invoking mfn_to_gmfn(), which is about to be replaced.
> Avoid morphing this code into even more suspicious shape and remove the
> effectively dead code - translated mode has been made impossible for PV
> quite some time ago.
> 
> Adjust and extend the assertions at the same time: The original
> ASSERT(!shadow_mode_refcounts(owner)) really means
> ASSERT(!shadow_mode_enabled(owner) || !paging_mode_refcounts(owner)),
> which isn't what we want here.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2632,7 +2632,6 @@ int free_page_type(struct page_info *pag
>  {
>  #ifdef CONFIG_PV
>      struct domain *owner = page_get_owner(page);
> -    unsigned long gmfn;
>      int rc;
>  
>      if ( likely(owner != NULL) && unlikely(paging_mode_enabled(owner)) )
> @@ -2640,11 +2639,11 @@ int free_page_type(struct page_info *pag
>          /* A page table is dirtied when its type count becomes zero. */
>          paging_mark_dirty(owner, page_to_mfn(page));
>  
> -        ASSERT(!shadow_mode_refcounts(owner));
> +        ASSERT(shadow_mode_enabled(owner));
> +        ASSERT(!paging_mode_refcounts(owner));
> +        ASSERT(!paging_mode_translate(owner));

In the context of my patch to CODING_STYLE about the use of ASSERTs,
thinking about ASSERT vs BUG_ON vs something else here.  I guess in this
case:

1. PV guests can't be in translate mode
2. If that ever changed, we'd probably trip over the ASSERT() while
debugging

So I guess ASSERT() is probably fine.

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

So does that mean, though, that SHARED_M2P_ENTRY & friends are entirely
vestigal now, and can be removed?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel