>From: dunl...@gmail.com [mailto:dunl...@gmail.com] On Behalf Of George >Dunlap >Sent: Friday, July 10, 2015 10:03 AM > >On Fri, Jul 10, 2015 at 1:52 AM, Ed White <edmund.h.wh...@intel.com> >wrote: >> From: George Dunlap <george.dun...@eu.citrix.com> >> >> The existing ept_set_entry() and ept_get_entry() routines are extended >> to optionally set/get suppress_ve. Passing -1 will set suppress_ve on >> new p2m entries, or retain suppress_ve flag on existing entries. >> >> Signed-off-by: George Dunlap <george.dun...@eu.citrix.com> > >So because my patch contained code written by Ed, and this patch now >contains code written by you, I'm pretty sure that a strict observance of >protocol would require his SoB to be retained (as I think I did when I sent >it), >and your SoB to be added, for copyright purposes. > >In this particular case a lawyer might argue that the code snippets inquestion >were so small or obvious as to be uncopyrightable, but it doesn't really hurt >to >be a bit more strict than we need to be. :-)
I can add my Signed-off-by to cover both Ed and my code (from Intel perspective). > >Also, a description of what you had changed could have helped speed review. >(It seems you've only added the bits requested to the p2m-pt >implementation?) That’s correct (from discussion with Ed). > >Finally, one thing I missed in the discussion before... > >> @@ -1528,16 +1528,17 @@ bool_t p2m_mem_access_check(paddr_t gpa, >unsigned long gla, >> vm_event_request_t *req; >> int rc; >> unsigned long eip = guest_cpu_user_regs()->eip; >> + bool_t sve; >> >> /* First, handle rx2rw conversion automatically. >> * These calls to p2m->set_entry() must succeed: we have the gfn >> * locked and just did a successful get_entry(). */ >> gfn_lock(p2m, gfn, 0); >> - mfn = p2m->get_entry(p2m, gfn, &p2mt, &p2ma, 0, NULL); >> + mfn = p2m->get_entry(p2m, gfn, &p2mt, &p2ma, 0, NULL, &sve); >> >> if ( npfec.write_access && p2ma == p2m_access_rx2rw ) >> { >> - rc = p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, >p2m_access_rw); >> + rc = p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, >> + p2m_access_rw, sve); >> ASSERT(rc == 0); >> gfn_unlock(p2m, gfn, 0); >> return 1; >> @@ -1546,7 +1547,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, >unsigned long gla, >> { >> ASSERT(npfec.write_access || npfec.read_access || npfec.insn_fetch); >> rc = p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, >> - p2mt, p2m_access_rwx); >> + p2mt, p2m_access_rwx, -1); >> ASSERT(rc == 0); >> } >> gfn_unlock(p2m, gfn, 0); > >This definitely should not be "sve" in the 'if' clause and "-1" in the 'else' >clause. >Because I was looking only at the patch, I missed that when Jan raised the >issue before. That's a mistake on my part -- would you mind doing as Jan >suggests, and just making these "NULL" and "-1" >throughout this file? Ok I will make this change but I would like you to review it - could I send you a version on Saturday so you can review it (since you are not working on Monday) Thanks, Ravi > >Thanks! > -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel