Re: [PATCH 2/2] altp2m: Implement p2m_get_mem_access for altp2m views

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 01/28/2016 04:42 PM, Lengyel, Tamas wrote:
> 
> On Jan 28, 2016 6:38 AM, "Jan Beulich" <JBeulich@suse.com
> <mailto:JBeulich@suse.com>> wrote:
>>
>> >>> On 27.01.16 at 21:06, <tlengyel@novetta.com
> <mailto:tlengyel@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.


Thanks,
Razvan