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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-08-03 at 08:59 +0100, Jan Beulich wrote:
> >>> On 03.08.12 at 09:48, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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@xxxxxxxxxx>
> > # 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.

Right. I must admit I was surprised to find that nestedhvm was a feature
of 32 bit at all, I had expect the fix to be changing some sort of stub
function...

> 
> > 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@xxxxxxxxxx>
> 
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> > 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@xxxxxxxxxxxxx 
> > http://lists.xen.org/xen-devel 
> 
> 



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