diff mbox

[Xen-devel,1/3] xen: arm: avoid reusing incorrect mappings when walking the p2m.

Message ID 1405355225-4623-1-git-send-email-ian.campbell@citrix.com
State New
Headers show

Commit Message

Ian Campbell July 14, 2014, 4:27 p.m. UTC
If we handle a change in a superpage then we need to invalidate any cached
mappings further down, otherwise we risk using them because their offset might
match but be based on a different offset further up the table.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/p2m.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Julien Grall July 14, 2014, 6:17 p.m. UTC | #1
Hi Ian,

On 07/14/2014 05:27 PM, Ian Campbell wrote:
> If we handle a change in a superpage then we need to invalidate any cached
> mappings further down, otherwise we risk using them because their offset might
> match but be based on a different offset further up the table.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/p2m.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index c0c011a..917baa3 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -719,6 +719,8 @@ static int apply_p2m_changes(struct domain *d,
>                  goto out;
>              }
>              cur_first_page = p2m_first_level_index(addr);
> +            /* Any mapping further down is now invalid */
> +            cur_second_offset = ~0;

I think you miss to invalid cur_first_offset.

Also, I think we have the same issue on Xen 4.4.

Regards,
Ian Campbell July 15, 2014, 9:36 a.m. UTC | #2
On Mon, 2014-07-14 at 19:17 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 07/14/2014 05:27 PM, Ian Campbell wrote:
> > If we handle a change in a superpage then we need to invalidate any cached
> > mappings further down, otherwise we risk using them because their offset might
> > match but be based on a different offset further up the table.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/p2m.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index c0c011a..917baa3 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -719,6 +719,8 @@ static int apply_p2m_changes(struct domain *d,
> >                  goto out;
> >              }
> >              cur_first_page = p2m_first_level_index(addr);
> > +            /* Any mapping further down is now invalid */
> > +            cur_second_offset = ~0;
> 
> I think you miss to invalid cur_first_offset.

I think you are right.

> Also, I think we have the same issue on Xen 4.4.

I'll add it to my list to check. I suspect the fix will need
reimplementing there rather than backporting.

Ian.
diff mbox

Patch

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index c0c011a..917baa3 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -719,6 +719,8 @@  static int apply_p2m_changes(struct domain *d,
                 goto out;
             }
             cur_first_page = p2m_first_level_index(addr);
+            /* Any mapping further down is now invalid */
+            cur_second_offset = ~0;
         }
 
         /* We only use a 3 level p2m at the moment, so no level 0,
@@ -741,6 +743,8 @@  static int apply_p2m_changes(struct domain *d,
             if (second) unmap_domain_page(second);
             second = map_domain_page(first[first_table_offset(addr)].p2m.base);
             cur_first_offset = first_table_offset(addr);
+            /* Any mapping further down is now invalid */
+            cur_second_offset = ~0;
         }
         /* else: second already valid */