[5/8] xen/arm: p2m: Add p2m_get_entry

Message ID 1386258131-755-6-git-send-email-julien.grall@linaro.org
State Superseded
Headers show

Commit Message

Julien Grall Dec. 5, 2013, 3:42 p.m.
Reuse the code of p2m_lookup and add an argument to retrieve the p2m type.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/p2m.c        |   11 ++++++++++-
 xen/include/asm-arm/p2m.h |    7 ++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

Comments

Ian Campbell Dec. 5, 2013, 4:07 p.m. | #1
On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote:
> Reuse the code of p2m_lookup and add an argument to retrieve the p2m type.

I'd be inclined to just add the parameter and change the existing
callers to pass NULL if they don't care about the type.

The x86 equivalent of this seems to be get_gfn_type which also takes the
p2m lock, which makes sense because you don't want the entry changing
under your feet while you do whatever you do with it.

However, I have a feeling this is related to things like CoW and PoD
(Tim?) which we don't have yet, so perhaps we can get away without this
for now? That would be best since it seems like a lot to chew for 4.4.

Ian.

> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/p2m.c        |   11 ++++++++++-
>  xen/include/asm-arm/p2m.h |    7 ++++++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 5449a35..1ae2521 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -71,11 +71,17 @@ static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr)
>   * There are no processor functions to do a stage 2 only lookup therefore we
>   * do a a software walk.
>   */
> -paddr_t p2m_lookup(struct domain *d, paddr_t paddr)
> +paddr_t p2m_get_entry(struct domain *d, paddr_t paddr, p2m_type_t *t)
>  {
>      struct p2m_domain *p2m = &d->arch.p2m;
>      lpae_t pte, *first = NULL, *second = NULL, *third = NULL;
>      paddr_t maddr = INVALID_PADDR;
> +    p2m_type_t _t;
> +
> +    /* Allow t to be NULL */
> +    t = t ?: &_t;
> +
> +    *t = p2m_invalid;
>  
>      spin_lock(&p2m->lock);
>  
> @@ -99,7 +105,10 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr)
>  
>  done:
>      if ( pte.p2m.valid )
> +    {
>          maddr = (pte.bits & PADDR_MASK & PAGE_MASK) | (paddr & ~PAGE_MASK);
> +        *t = pte.p2m.avail;
> +    }
>  
>      if (third) unmap_domain_page(third);
>      if (second) unmap_domain_page(second);
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 1cf7d36..3de69c4 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -49,7 +49,12 @@ int p2m_alloc_table(struct domain *d);
>  void p2m_load_VTTBR(struct domain *d);
>  
>  /* Look up the MFN corresponding to a domain's PFN. */
> -paddr_t p2m_lookup(struct domain *d, paddr_t gpfn);
> +paddr_t p2m_get_entry(struct domain *d, paddr_t gpfn, p2m_type_t *t);
> +
> +static inline paddr_t p2m_lookup(struct domain *d, paddr_t gpfn)
> +{
> +    return p2m_get_entry(d, gpfn, NULL);
> +}
>  
>  /* Setup p2m RAM mapping for domain d from start-end. */
>  int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
Tim Deegan Dec. 5, 2013, 4:09 p.m. | #2
B11;rgb:0000/0000/0000At 16:07 +0000 on 05 Dec (1386256032), Ian Campbell wrote:
> On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote:
> > Reuse the code of p2m_lookup and add an argument to retrieve the p2m type.
> 
> I'd be inclined to just add the parameter and change the existing
> callers to pass NULL if they don't care about the type.
> 
> The x86 equivalent of this seems to be get_gfn_type which also takes the
> p2m lock, which makes sense because you don't want the entry changing
> under your feet while you do whatever you do with it.
> 
> However, I have a feeling this is related to things like CoW and PoD
> (Tim?)

Yes, though in theory you could have the same sort of trouble with
balloon drivers, device models moving framebuffers, &c. 

Tim.

Patch

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 5449a35..1ae2521 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -71,11 +71,17 @@  static lpae_t *p2m_map_first(struct p2m_domain *p2m, paddr_t addr)
  * There are no processor functions to do a stage 2 only lookup therefore we
  * do a a software walk.
  */
-paddr_t p2m_lookup(struct domain *d, paddr_t paddr)
+paddr_t p2m_get_entry(struct domain *d, paddr_t paddr, p2m_type_t *t)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
     lpae_t pte, *first = NULL, *second = NULL, *third = NULL;
     paddr_t maddr = INVALID_PADDR;
+    p2m_type_t _t;
+
+    /* Allow t to be NULL */
+    t = t ?: &_t;
+
+    *t = p2m_invalid;
 
     spin_lock(&p2m->lock);
 
@@ -99,7 +105,10 @@  paddr_t p2m_lookup(struct domain *d, paddr_t paddr)
 
 done:
     if ( pte.p2m.valid )
+    {
         maddr = (pte.bits & PADDR_MASK & PAGE_MASK) | (paddr & ~PAGE_MASK);
+        *t = pte.p2m.avail;
+    }
 
     if (third) unmap_domain_page(third);
     if (second) unmap_domain_page(second);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 1cf7d36..3de69c4 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -49,7 +49,12 @@  int p2m_alloc_table(struct domain *d);
 void p2m_load_VTTBR(struct domain *d);
 
 /* Look up the MFN corresponding to a domain's PFN. */
-paddr_t p2m_lookup(struct domain *d, paddr_t gpfn);
+paddr_t p2m_get_entry(struct domain *d, paddr_t gpfn, p2m_type_t *t);
+
+static inline paddr_t p2m_lookup(struct domain *d, paddr_t gpfn)
+{
+    return p2m_get_entry(d, gpfn, NULL);
+}
 
 /* Setup p2m RAM mapping for domain d from start-end. */
 int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);