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

[Julien Grall <julien.grall@linaro.org>]

Hi Frediano,

On 10/22/2014 11:05 AM, 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.
> 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   | 14 +++++++++++++-
>  xen/arch/arm/traps.c |  2 +-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> Changes from v1:
> - disable IRQ if different domain (as suggested by Julien Grall)

If you want this patch applied for Xen 4.5, you need to explain why it
will be useful and what are the drawbacks.

Though, I'd like to see this patch for Xen 4.5 so... The function
get_page_from_gva is used in hotpath (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.

(Can you keep this explanation or write your own in case you want this
for Xen 4.5).

> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 1585d35..6956eab 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1177,8 +1177,13 @@ 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;
> +    unsigned long irq_flags = 0;
> 
> -    ASSERT(d == current->domain);
> +    if ( unlikely(d != current->domain) )
> +    {
> +        local_irq_save(irq_flags);
> +        p2m_load_VTTBR(d);
> +    }
> 
>      spin_lock(&p2m->lock);

I though a bit more about the code path. I would first take the lock,
then switch to the VTTBR and disable IRQ if necessary.

This would avoid to disable the IRQ for a long time if the lock is
already taken.

Regards,

-- 
Julien Grall