[PATCH v3] xen/arm: dump guest stack even if not the current VCPU

[PATCH v3] xen/arm: dump guest stack even if not the current VCPU

[Frediano Ziglio]

From: Frediano Ziglio <frediano.ziglio@huawei.com>

If show_guest_stack was called from Xen context (for instance hitting
'0' key on Xen console) get_page_from_gva was not able to get the
page returning NULL.

The function get_page_from_gva is used in hot path (see
arch/arm/guestcopy.c) but always with the current domain. The function
will be used with another domain than current only when the stack of
the guest will be dumped. The code added is self-containted.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/p2m.c   | 22 +++++++++++++++++++---
 xen/arch/arm/traps.c |  2 +-
 2 files changed, 20 insertions(+), 4 deletions(-)

Changed from v2:
- add comment suggested by Julien Grall (I have some doubt about
removing my initial comments or keep it, I kept it removing the code
explanation);
- modify code path to avoid keeping IRQ disabled too much (Julien Grall);
- collapse change in a single if to improve performances.

Tested manually.

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 1585d35..2345199 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1177,12 +1177,28 @@ struct page_info *get_page_from_gva(struct
domain *d, vaddr_t va,
     struct p2m_domain *p2m = &d->arch.p2m;
     struct page_info *page = NULL;
     paddr_t maddr;
-
-    ASSERT(d == current->domain);
+    int rc;

     spin_lock(&p2m->lock);

-    if ( gvirt_to_maddr(va, &maddr, flags) )
+    if ( unlikely(d != current->domain) )
+    {
+        unsigned long irq_flags;
+
+        local_irq_save(irq_flags);
+        p2m_load_VTTBR(d);
+
+        rc = gvirt_to_maddr(va, &maddr, flags);
+
+        p2m_load_VTTBR(current->domain);
+        local_irq_restore(irq_flags);
+    }
+    else
+    {
+        rc = gvirt_to_maddr(va, &maddr, flags);
+    }
+
+    if ( rc )
         goto err;

     if ( !mfn_valid(maddr >> PAGE_SHIFT) )
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f6fc8f8..4c93250 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -892,7 +892,7 @@ static void show_guest_stack(struct vcpu *v,
struct cpu_user_regs *regs)
         return;
     }

-    page = get_page_from_gva(current->domain, sp, GV2M_READ);
+    page = get_page_from_gva(v->domain, sp, GV2M_READ);
     if ( page == NULL )
     {
         printk("Failed to convert stack to physical address\n");
-- 
1.9.1

Re: [PATCH v3] xen/arm: dump guest stack even if not the current VCPU

[Julien Grall]

On 10/22/2014 02:36 PM, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@huawei.com>
> 
> If show_guest_stack was called from Xen context (for instance hitting
> '0' key on Xen console) get_page_from_gva was not able to get the
> page returning NULL.
> 
> The function get_page_from_gva is used in hot path (see
> arch/arm/guestcopy.c) but always with the current domain. The function
> will be used with another domain than current only when the stack of
> the guest will be dumped. The code added is self-containted.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> ---
>  xen/arch/arm/p2m.c   | 22 +++++++++++++++++++---
>  xen/arch/arm/traps.c |  2 +-
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> Changed from v2:
> - add comment suggested by Julien Grall (I have some doubt about
> removing my initial comments or keep it, I kept it removing the code
> explanation);

By comment I meant after "---", explaining that this patch is bug fix
for Xen 4.5... I didn't ask to modify the commit message. Sorry for not
been clear.

You can give a look to
http://wiki.xen.org/wiki/Xen_Roadmap/4.4#Exception_guidelines_for_after_the_code_freeze.
It's for Xen 4.4 but it also apply for Xen 4.5.

Regards,

-- 
Julien Grall

Re: [PATCH v3] xen/arm: dump guest stack even if not the current VCPU

[Frediano Ziglio]

From: Frediano Ziglio <frediano.ziglio@huawei.com>

If show_guest_stack was called from Xen context (for instance hitting
'0' key on Xen console) get_page_from_gva was not able to get the
page returning NULL.
Detecting different domain and changing VTTBR register make
get_page_from_gva works for different domains.

Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
---
 xen/arch/arm/p2m.c   | 22 +++++++++++++++++++---
 xen/arch/arm/traps.c |  2 +-
 2 files changed, 20 insertions(+), 4 deletions(-)

The function get_page_from_gva is used in hot path (see
arch/arm/guestcopy.c) but always with the current domain. The function
will be used with another domain than current only when the stack of
the guest will be dumped. The code added is self-containted.

Changed from v2:
- add comment suggested by Julien Grall;
- modify code path to avoid keeping IRQ disabled too much (Julien Grall);
- collapse change in a single if to improve performances.

Tested manually.

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 1585d35..2345199 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1177,12 +1177,28 @@ struct page_info *get_page_from_gva(struct
domain *d, vaddr_t va,
     struct p2m_domain *p2m = &d->arch.p2m;
     struct page_info *page = NULL;
     paddr_t maddr;
-
-    ASSERT(d == current->domain);
+    int rc;

     spin_lock(&p2m->lock);

-    if ( gvirt_to_maddr(va, &maddr, flags) )
+    if ( unlikely(d != current->domain) )
+    {
+        unsigned long irq_flags;
+
+        local_irq_save(irq_flags);
+        p2m_load_VTTBR(d);
+
+        rc = gvirt_to_maddr(va, &maddr, flags);
+
+        p2m_load_VTTBR(current->domain);
+        local_irq_restore(irq_flags);
+    }
+    else
+    {
+        rc = gvirt_to_maddr(va, &maddr, flags);
+    }
+
+    if ( rc )
         goto err;

     if ( !mfn_valid(maddr >> PAGE_SHIFT) )
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f6fc8f8..4c93250 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -892,7 +892,7 @@ static void show_guest_stack(struct vcpu *v,
struct cpu_user_regs *regs)
         return;
     }

-    page = get_page_from_gva(current->domain, sp, GV2M_READ);
+    page = get_page_from_gva(v->domain, sp, GV2M_READ);
     if ( page == NULL )
     {
         printk("Failed to convert stack to physical address\n");
-- 
1.9.1

2014-10-22 14:43 GMT+01:00 Julien Grall <julien.grall@linaro.org>:
> On 10/22/2014 02:36 PM, Frediano Ziglio wrote:
>> From: Frediano Ziglio <frediano.ziglio@huawei.com>
>>
>> If show_guest_stack was called from Xen context (for instance hitting
>> '0' key on Xen console) get_page_from_gva was not able to get the
>> page returning NULL.
>>
>> The function get_page_from_gva is used in hot path (see
>> arch/arm/guestcopy.c) but always with the current domain. The function
>> will be used with another domain than current only when the stack of
>> the guest will be dumped. The code added is self-containted.
>>
>> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
>> ---
>>  xen/arch/arm/p2m.c   | 22 +++++++++++++++++++---
>>  xen/arch/arm/traps.c |  2 +-
>>  2 files changed, 20 insertions(+), 4 deletions(-)
>>
>> Changed from v2:
>> - add comment suggested by Julien Grall (I have some doubt about
>> removing my initial comments or keep it, I kept it removing the code
>> explanation);
>
> By comment I meant after "---", explaining that this patch is bug fix
> for Xen 4.5... I didn't ask to modify the commit message. Sorry for not
> been clear.
>
> You can give a look to
> http://wiki.xen.org/wiki/Xen_Roadmap/4.4#Exception_guidelines_for_after_the_code_freeze.
> It's for Xen 4.4 but it also apply for Xen 4.5.
>
> Regards,
>
> --
> Julien Grall