Do away with the "pod_target_out_unlock" label. In particular by folding
if()-s, the logic can be expressed with less code (and no goto-s) this
way.

Limit scope of "p2m", constifying it at the same time.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4777,7 +4777,6 @@ long arch_memory_op(unsigned long cmd, X
     {
         xen_pod_target_t target;
         struct domain *d;
-        struct p2m_domain *p2m;
 
         if ( copy_from_guest(&target, arg, 1) )
             return -EFAULT;
@@ -4789,23 +4788,17 @@ long arch_memory_op(unsigned long cmd, X
         if ( !is_hvm_domain(d) )
             rc = -EINVAL;
         else if ( cmd == XENMEM_set_pod_target )
-            rc = xsm_set_pod_target(XSM_PRIV, d);
-        else
-            rc = xsm_get_pod_target(XSM_PRIV, d);
-
-        if ( rc != 0 )
-            goto pod_target_out_unlock;
-
-        if ( cmd == XENMEM_set_pod_target )
         {
-            if ( target.target_pages > d->max_pages )
-            {
+            rc = xsm_set_pod_target(XSM_PRIV, d);
+            if ( rc )
+                ASSERT(rc < 0);
+            else if ( target.target_pages > d->max_pages )
                 rc = -EINVAL;
-                goto pod_target_out_unlock;
-            }
-
-            rc = p2m_pod_set_mem_target(d, target.target_pages);
+            else
+                rc = p2m_pod_set_mem_target(d, target.target_pages);
         }
+        else
+            rc = xsm_get_pod_target(XSM_PRIV, d);
 
         if ( rc == -ERESTART )
         {
@@ -4814,19 +4807,16 @@ long arch_memory_op(unsigned long cmd, X
         }
         else if ( rc >= 0 )
         {
-            p2m = p2m_get_hostp2m(d);
+            const struct p2m_domain *p2m = p2m_get_hostp2m(d);
+
             target.tot_pages       = domain_tot_pages(d);
             target.pod_cache_pages = p2m->pod.count;
             target.pod_entries     = p2m->pod.entry_count;
 
             if ( __copy_to_guest(arg, &target, 1) )
-            {
-                rc= -EFAULT;
-                goto pod_target_out_unlock;
-            }
+                rc = -EFAULT;
         }
 
-    pod_target_out_unlock:
         rcu_unlock_domain(d);
         return rc;
     }

On 13.09.2021 10:55, Jan Beulich wrote:
> Do away with the "pod_target_out_unlock" label. In particular by folding
> if()-s, the logic can be expressed with less code (and no goto-s) this
> way.
> 
> Limit scope of "p2m", constifying it at the same time.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

And this is of course patch 2/2. I'm sorry for the screw-up.

Jan

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4777,7 +4777,6 @@ long arch_memory_op(unsigned long cmd, X
>      {
>          xen_pod_target_t target;
>          struct domain *d;
> -        struct p2m_domain *p2m;
>  
>          if ( copy_from_guest(&target, arg, 1) )
>              return -EFAULT;
> @@ -4789,23 +4788,17 @@ long arch_memory_op(unsigned long cmd, X
>          if ( !is_hvm_domain(d) )
>              rc = -EINVAL;
>          else if ( cmd == XENMEM_set_pod_target )
> -            rc = xsm_set_pod_target(XSM_PRIV, d);
> -        else
> -            rc = xsm_get_pod_target(XSM_PRIV, d);
> -
> -        if ( rc != 0 )
> -            goto pod_target_out_unlock;
> -
> -        if ( cmd == XENMEM_set_pod_target )
>          {
> -            if ( target.target_pages > d->max_pages )
> -            {
> +            rc = xsm_set_pod_target(XSM_PRIV, d);
> +            if ( rc )
> +                ASSERT(rc < 0);
> +            else if ( target.target_pages > d->max_pages )
>                  rc = -EINVAL;
> -                goto pod_target_out_unlock;
> -            }
> -
> -            rc = p2m_pod_set_mem_target(d, target.target_pages);
> +            else
> +                rc = p2m_pod_set_mem_target(d, target.target_pages);
>          }
> +        else
> +            rc = xsm_get_pod_target(XSM_PRIV, d);
>  
>          if ( rc == -ERESTART )
>          {
> @@ -4814,19 +4807,16 @@ long arch_memory_op(unsigned long cmd, X
>          }
>          else if ( rc >= 0 )
>          {
> -            p2m = p2m_get_hostp2m(d);
> +            const struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +
>              target.tot_pages       = domain_tot_pages(d);
>              target.pod_cache_pages = p2m->pod.count;
>              target.pod_entries     = p2m->pod.entry_count;
>  
>              if ( __copy_to_guest(arg, &target, 1) )
> -            {
> -                rc= -EFAULT;
> -                goto pod_target_out_unlock;
> -            }
> +                rc = -EFAULT;
>          }
>  
> -    pod_target_out_unlock:
>          rcu_unlock_domain(d);
>          return rc;
>      }
> 
>