Re: [xen-unstable test] 13539: regressions - FAIL

[Christoph Egger <Christoph.Egger@amd.com>]

On 08/03/12 09:59, Jan Beulich wrote:

>>>> On 03.08.12 at 09:48, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Fri, 2012-08-03 at 06:12 +0100, Ian Campbell wrote:
>>> On Fri, 2012-08-03 at 00:41 +0100, xen.org wrote:
>>>> flight 13539 xen-unstable real [real]
>>>> http://www.chiark.greenend.org.uk/~xensrcts/logs/13539/ 
>>>>
>>>> Regressions :-(
>>>>
>>>> Tests which did not succeed and are blocking,
>>>> including tests which could not be run:
>>>>  build-i386-oldkern            4 xen-build                 fail REGR. vs. 
>> 13536
>>>>  build-i386                    4 xen-build                 fail REGR. vs. 
>> 13536
>>
>> 8<----------------------------------
>>
>> # HG changeset patch
>> # User Ian Campbell <ian.campbell@citrix.com>
>> # Date 1343980045 -3600
>> # Node ID 23fdca3adb3346090ea8b65b77cad7d279cf9daf
>> # Parent  95a4ab632ac25ce0ec6a245dcc46ad57d3c7030f
>> nestedhvm: fix nested page fault build error on 32-bit
>>
>>     cc1: warnings being treated as errors
>>     hvm.c: In function ‘hvm_hap_nested_page_fault’:
>>     hvm.c:1282: error: passing argument 2 of 
>> ‘nestedhvm_hap_nested_page_fault’ from incompatible pointer type 
>> /local/scratch/ianc/devel/xen-unstable.hg/xen/include/asm/hvm/nestedhvm.h:55: 
>> note: expected ‘paddr_t *’ but argument is of type ‘long unsigned int *’
>>
>> hvm_hap_nested_page_fault takes an unsigned long gpa and passes &gpa
>> to nestedhvm_hap_nested_page_fault which takes a paddr_t *. Since both
>> of the callers of hvm_hap_nested_page_fault (svm_do_nested_pgfault and
>> ept_handle_violation) actually have the gpa which they pass to
>> hvm_hap_nested_page_fault as a paddr_t I think it makes sense to
>> change the argument to hvm_hap_nested_page_fault.
> 
> And that's even outside of the current build failure - it just
> can't have worked for >4Gb guests on the 32-bit hypervisor.
> 
>> The other user of gpa in hvm_hap_nested_page_fault is a call to
>> p2m_mem_access_check, which currently also takes a paddr_t gpa but I
>> think a paddr_t is appropriate there too.
>>
>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Christoph Egger <Christoph.Egger@amd.com>

> 
>> diff -r 95a4ab632ac2 -r 23fdca3adb33 xen/arch/x86/hvm/hvm.c
>> --- a/xen/arch/x86/hvm/hvm.c	Fri Aug 03 08:43:10 2012 +0100
>> +++ b/xen/arch/x86/hvm/hvm.c	Fri Aug 03 08:47:25 2012 +0100
>> @@ -1242,7 +1242,7 @@ void hvm_inject_page_fault(int errcode, 
>>      hvm_inject_trap(&trap);
>>  }
>>  
>> -int hvm_hap_nested_page_fault(unsigned long gpa,
>> +int hvm_hap_nested_page_fault(paddr_t gpa,
>>                                bool_t gla_valid,
>>                                unsigned long gla,
>>                                bool_t access_r,
>> diff -r 95a4ab632ac2 -r 23fdca3adb33 xen/arch/x86/mm/p2m.c
>> --- a/xen/arch/x86/mm/p2m.c	Fri Aug 03 08:43:10 2012 +0100
>> +++ b/xen/arch/x86/mm/p2m.c	Fri Aug 03 08:47:25 2012 +0100
>> @@ -1233,7 +1233,7 @@ void p2m_mem_paging_resume(struct domain
>>      }
>>  }
>>  
>> -bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned 
>> long gla, 
>> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long 
>> gla, 
>>                            bool_t access_r, bool_t access_w, bool_t 
>> access_x,
>>                            mem_event_request_t **req_ptr)
>>  {
>> diff -r 95a4ab632ac2 -r 23fdca3adb33 xen/include/asm-x86/hvm/hvm.h
>> --- a/xen/include/asm-x86/hvm/hvm.h	Fri Aug 03 08:43:10 2012 +0100
>> +++ b/xen/include/asm-x86/hvm/hvm.h	Fri Aug 03 08:47:25 2012 +0100
>> @@ -433,7 +433,7 @@ static inline void hvm_set_info_guest(st
>>  
>>  int hvm_debug_op(struct vcpu *v, int32_t op);
>>  
>> -int hvm_hap_nested_page_fault(unsigned long gpa,
>> +int hvm_hap_nested_page_fault(paddr_t gpa,
>>                                bool_t gla_valid, unsigned long gla,
>>                                bool_t access_r,
>>                                bool_t access_w,
>> diff -r 95a4ab632ac2 -r 23fdca3adb33 xen/include/asm-x86/p2m.h
>> --- a/xen/include/asm-x86/p2m.h	Fri Aug 03 08:43:10 2012 +0100
>> +++ b/xen/include/asm-x86/p2m.h	Fri Aug 03 08:47:25 2012 +0100
>> @@ -589,7 +589,7 @@ static inline void p2m_mem_paging_popula
>>   * been promoted with no underlying vcpu pause. If the req_ptr has been 
>> populated, 
>>   * then the caller must put the event in the ring (once having released 
>> get_gfn*
>>   * locks -- caller must also xfree the request. */
>> -bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned 
>> long gla, 
>> +bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, unsigned long 
>> gla, 
>>                            bool_t access_r, bool_t access_w, bool_t 
>> access_x,
>>                            mem_event_request_t **req_ptr);
>>  /* Resumes the running of the VCPU, restarting the last instruction */
>> @@ -606,7 +606,7 @@ int p2m_get_mem_access(struct domain *d,
>>                         hvmmem_access_t *access);
>>  
>>  #else
>> -static inline bool_t p2m_mem_access_check(unsigned long gpa, bool_t 
>> gla_valid, 
>> +static inline bool_t p2m_mem_access_check(paddr_t gpa, bool_t gla_valid, 
>>                                          unsigned long gla, bool_t access_r, 
>>
>>                                          bool_t access_w, bool_t access_x,
>>                                          mem_event_request_t **req_ptr)
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org 
>> http://lists.xen.org/xen-devel 



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


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