Re: [Xen-devel] [PATCH for-xen-4-5 v3] xen/arm: dump guest stack even if not the current VCPU

[Frediano Ziglio <freddy77@xxxxxxxxx>]

2014-10-23 13:14 GMT+01:00 Ian Campbell <Ian.Campbell@xxxxxxxxxx>:
> On Thu, 2014-10-23 at 09:46 +0100, Frediano Ziglio wrote:
>> From: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>>  xen/arch/arm/p2m.c   | 22 +++++++++++++++++++---
>>  xen/arch/arm/traps.c |  2 +-
>>  2 files changed, 20 insertions(+), 4 deletions(-)
>>
>> This is a bug fix to fix guest stack dumps.
>>
>> 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.
>
> I succeeding in applying for real this time but:
> p2m.c: In function âget_page_from_gvaâ:
> p2m.c:1185:45: error: âmaddrâ may be used uninitialized in this function 
> [-Werror=uninitialized]
> cc1: all warnings being treated as errors
> make[4]: *** [p2m.o] Error 1
>
> This was on arm32. So not pushed.
>

Mmm... probably your compiler is detecting a false error. maddr is set
on both if paths. Should I rewrite the patch to workaround this
compiler problem? Obviously if we pretend to support this compiler. Or
perhaps we should silence this warning
(http://stackoverflow.com/questions/19374122/gcc-removing-is-used-uninitialized-in-this-function-warning).
I think setting variable to 0 at declaration would be quite a fine
solution.

>> 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,
>
> Please avoid word wrapping patches in the future.
> http://wiki.xen.org/wiki/Submitting_Xen_Patches has some guidance for
> different mail clients. I fixed it by hand this time.
>
> Ian.
>

I'll keep more attention. Perhaps I did some wrong copy&paste.

Frediano

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel