[Xen-devel] [PATCH v2 7/7] xen/arm: mm: Flush the TLBs even if a mapping failed in create_xen_entries

Julien Grall posted 7 patches 1 year, 11 months ago

[Xen-devel] [PATCH v2 7/7] xen/arm: mm: Flush the TLBs even if a mapping failed in create_xen_entries

Posted by Julien Grall 1 year, 11 months ago
At the moment, create_xen_entries will only flush the TLBs if the full
range has successfully been updated. This may lead to leave unwanted
entries in the TLBs if we fail to update some entries.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 8ee828d445..9d584e4cbf 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -984,7 +984,7 @@ static int create_xen_entries(enum xenmap_operation op,
                               unsigned long nr_mfns,
                               unsigned int flags)
 {
-    int rc;
+    int rc = 0;
     unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
     lpae_t pte, *entry;
     lpae_t *third = NULL;
@@ -1013,7 +1013,8 @@ static int create_xen_entries(enum xenmap_operation op,
                 {
                     printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
                            __func__, addr, mfn_x(mfn));
-                    return -EINVAL;
+                    rc = -EINVAL;
+                    goto out;
                 }
                 if ( op == RESERVE )
                     break;
@@ -1030,7 +1031,8 @@ static int create_xen_entries(enum xenmap_operation op,
                 {
                     printk("%s: trying to %s a non-existing mapping addr=%lx\n",
                            __func__, op == REMOVE ? "remove" : "modify", addr);
-                    return -EINVAL;
+                    rc = -EINVAL;
+                    goto out;
                 }
                 if ( op == REMOVE )
                     pte.bits = 0;
@@ -1043,7 +1045,8 @@ static int create_xen_entries(enum xenmap_operation op,
                     {
                         printk("%s: Incorrect combination for addr=%lx\n",
                                __func__, addr);
-                        return -EINVAL;
+                        rc = -EINVAL;
+                        goto out;
                     }
                 }
                 write_pte(entry, pte);
@@ -1052,11 +1055,14 @@ static int create_xen_entries(enum xenmap_operation op,
                 BUG();
         }
     }
+out:
+    /*
+     * Flush the TLBs even in case of failure because we may have
+     * partially modified the PT. This will prevent any unexpected
+     * behavior afterwards.
+     */
     flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
 
-    rc = 0;
-
-out:
     return rc;
 }
 
-- 
2.11.0


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

Re: [Xen-devel] [PATCH v2 7/7] xen/arm: mm: Flush the TLBs even if a mapping failed in create_xen_entries

Posted by Stefano Stabellini 1 year, 11 months ago
On Wed, 8 May 2019, Julien Grall wrote:
> At the moment, create_xen_entries will only flush the TLBs if the full
> range has successfully been updated. This may lead to leave unwanted
> entries in the TLBs if we fail to update some entries.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 8ee828d445..9d584e4cbf 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -984,7 +984,7 @@ static int create_xen_entries(enum xenmap_operation op,
>                                unsigned long nr_mfns,
>                                unsigned int flags)
>  {
> -    int rc;
> +    int rc = 0;
>      unsigned long addr = virt, addr_end = addr + nr_mfns * PAGE_SIZE;
>      lpae_t pte, *entry;
>      lpae_t *third = NULL;
> @@ -1013,7 +1013,8 @@ static int create_xen_entries(enum xenmap_operation op,
>                  {
>                      printk("%s: trying to replace an existing mapping addr=%lx mfn=%"PRI_mfn"\n",
>                             __func__, addr, mfn_x(mfn));
> -                    return -EINVAL;
> +                    rc = -EINVAL;
> +                    goto out;
>                  }
>                  if ( op == RESERVE )
>                      break;
> @@ -1030,7 +1031,8 @@ static int create_xen_entries(enum xenmap_operation op,
>                  {
>                      printk("%s: trying to %s a non-existing mapping addr=%lx\n",
>                             __func__, op == REMOVE ? "remove" : "modify", addr);
> -                    return -EINVAL;
> +                    rc = -EINVAL;
> +                    goto out;
>                  }
>                  if ( op == REMOVE )
>                      pte.bits = 0;
> @@ -1043,7 +1045,8 @@ static int create_xen_entries(enum xenmap_operation op,
>                      {
>                          printk("%s: Incorrect combination for addr=%lx\n",
>                                 __func__, addr);
> -                        return -EINVAL;
> +                        rc = -EINVAL;
> +                        goto out;
>                      }
>                  }
>                  write_pte(entry, pte);
> @@ -1052,11 +1055,14 @@ static int create_xen_entries(enum xenmap_operation op,
>                  BUG();
>          }
>      }
> +out:
> +    /*
> +     * Flush the TLBs even in case of failure because we may have
> +     * partially modified the PT. This will prevent any unexpected
> +     * behavior afterwards.
> +     */
>      flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
>  
> -    rc = 0;
> -
> -out:
>      return rc;
>  }
>  
> -- 
> 2.11.0
> 

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