Re: [Xen-devel] [PATCH for-4.12 V2] x86/p2m: fix p2m_finish_type_change()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.01.19 at 13:24, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> finish_type_change() returns a negative int on error, but the
> current code checks if ( !rc ). We also need to treat
> finish_type_change()'s return codes cumulatively in the
> success case (don't overwrite a 1 returned while processing
> the hostp2m if processing an altp2m returns 0).
> 
> The breakage was introduced by commit 0fb4b58c8b
> ("x86/altp2m: fix display frozen when switching to a new view
> early").
> 
> Properly indent the out: label while at it.
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> 
> ---
> Changes since V1:
>  - Updated description.
>  - Now treating finish_type_change()'s return value cumulatively
>    for the success case.
> ---
>  xen/arch/x86/mm/p2m.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 5451f16..91f412f 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
>  
>      rc = finish_type_change(hostp2m, first_gfn, max_nr);
>  
> -    if ( !rc )
> +    if ( rc < 0 )
>          goto out;
>  
>  #ifdef CONFIG_HVM
> @@ -1188,18 +1188,24 @@ int p2m_finish_type_change(struct domain *d,
>              if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
>              {
>                  struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
> +                int rc1;
>  
>                  p2m_lock(altp2m);
> -                rc = finish_type_change(altp2m, first_gfn, max_nr);
> +                rc1 = finish_type_change(altp2m, first_gfn, max_nr);
>                  p2m_unlock(altp2m);
>  
> -                if ( !rc )
> +                if ( rc1 < 0 )
> +                {
> +                    rc = rc1;
>                      goto out;
> +                }
> +
> +                rc = max(rc, rc1);

"rc |= rc1" would likely be cheaper in terms of generated code. But
functionally this is fine, and the change would be easy enough to
make while committing, so
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



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