On Thu, Jan 28, 2016 at 9:32 AM, Razvan Cojocaru <rcojoc...@bitdefender.com> wrote:
> On 01/28/2016 05:58 PM, Lengyel, Tamas wrote: > > > > > > On Thu, Jan 28, 2016 at 8:20 AM, Razvan Cojocaru > > <rcojoc...@bitdefender.com <mailto:rcojoc...@bitdefender.com>> wrote: > > > > On 01/28/2016 05:12 PM, Lengyel, Tamas wrote: > > > > > > On Jan 28, 2016 8:02 AM, "Razvan Cojocaru" < > rcojoc...@bitdefender.com <mailto:rcojoc...@bitdefender.com> > > > <mailto:rcojoc...@bitdefender.com <mailto: > rcojoc...@bitdefender.com>>> wrote: > > >> > > >> On 01/28/2016 04:42 PM, Lengyel, Tamas wrote: > > >> > > > >> > On Jan 28, 2016 6:38 AM, "Jan Beulich" <jbeul...@suse.com > <mailto:jbeul...@suse.com> > > > <mailto:jbeul...@suse.com <mailto:jbeul...@suse.com>> > > >> > <mailto:jbeul...@suse.com <mailto:jbeul...@suse.com> > > <mailto:jbeul...@suse.com <mailto:jbeul...@suse.com>>>> wrote: > > >> >> > > >> >> >>> On 27.01.16 at 21:06, <tleng...@novetta.com <mailto: > tleng...@novetta.com> > > > <mailto:tleng...@novetta.com <mailto:tleng...@novetta.com>> > > >> > <mailto:tleng...@novetta.com <mailto:tleng...@novetta.com> > > <mailto:tleng...@novetta.com <mailto:tleng...@novetta.com>>>> wrote: > > >> >> > --- a/xen/arch/x86/mm/p2m.c > > >> >> > +++ b/xen/arch/x86/mm/p2m.c > > >> >> > @@ -1572,7 +1572,9 @@ void > p2m_mem_access_emulate_check(struct > > > vcpu *v, > > >> >> > bool_t violation = 1; > > >> >> > const struct vm_event_mem_access *data = > > &rsp->u.mem_access; > > >> >> > > > >> >> > - if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), > > >> > &access) == 0 ) > > >> >> > + if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), > > >> >> > + altp2m_active(v->domain) ? > > >> > vcpu_altp2m(v).p2midx : 0, > > >> >> > + &access) == 0 ) > > >> >> > > >> >> This looks to be a behavioral change beyond what title and > > >> >> description say, and it's not clear whether that's actually the > > >> >> behavior everyone wants. > > >> > > > >> > I'm fairly comfident its exactly the expected behavior when one > > uses > > >> > mem_access in altp2m tables and emulation. Right now because > > the lack of > > >> > this AFAIK emulation would not work correctly with altp2m. But > > Razvan > > >> > probably can chime in as he uses this path actively. > > >> > > >> I've done an experiment to see how much slower using altp2m would > > be as > > >> compared to emulation - so I'm not a big user of the feature, but > > I did > > >> find it cumbersome to have to work with two sets of APIs (one for > > what > > >> could arguably be called the default altp2m view, i.e. the regular > > >> xc_set_mem_access(), and one for altp2m, i.e. > > >> xc_altp2m_set_mem_access()). Furthermore, the APIs do not > currently > > >> offer the same features (most notably, xc_altp2m_get_mem_access() > is > > >> completely missing). I've mentioned this to Tamas while initially > > trying > > >> to get it to work. > > >> > > >> Now, whether the behaviour I expect is what everyone expects is, > of > > >> course, wide open to debate. But I think we can all agree that the > > >> altp2m interface can, and probably should, be improved. > > >> > > > > > > There is that, but also, what is the exact logic behind doing this > > check > > > before emulation? AFAIU emulation happens in response to a > vm_event so > > > we should be fairly certain that this check succeeds as it just > > verifies > > > that indeed the permissions are restricted by mem_access in the > > p2m (and > > > with altp2m this should be the active one). But when is this check > > > normally expected to fail? > > > > That check is important, please do not remove it. A vm_event is sent > > into userspace to our monitoring application, but the monitoring > > application can actually remove the page restrictions before > replying, > > so in that case emulation is pointless - there will be no more page > > faults for that instruction. > > > > > > I see, but then why would you reply with VM_EVENT_FLAG_EMULATE? You know > > you removed the permission before sending the reply, so this sounds like > > something specific to your application. > > It's cheap insurance that things go right. If there's some issue with > page rights, or some external tool somehow does an xc_set_mem_access(), > things won't go wrong. I can see this working for your application if you don't cache the mem_access permissions locally and you don't want to query for it before deciding to send the emulate flag in the response or not. Although, I think that would be the best way to go here. > And they will go wrong if Xen thinks it should > emulate the next instruction and the next instruction is not the one > that has caused the original fault. How could that happen? When the vCPU is resumed after the fault, isn't the same instruction guaranteed to be retried? > I would think that benefits any > application. > It's just a bit of an obscure exception. From an API perspective I would rather have Xen do what I tell it to do - in this case emulate - rather then it doing something else silently behind the scenes that you really only find out about if you read the code. Tamas
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
