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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.01.16 at 15:42, <tlengyel@xxxxxxxxxxx> wrote:
> On Jan 28, 2016 6:38 AM, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> >>> On 27.01.16 at 21:06, <tlengyel@xxxxxxxxxxx> 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.

But this should then be mentioned in the description.

>> > +    /* altp2m view 0 is treated as the hostp2m */
>> > +    if ( altp2m_idx )
>> > +    {
>> > +        if ( altp2m_idx >= MAX_ALTP2M ||
>> > +             d->arch.altp2m_eptp[altp2m_idx] == INVALID_MFN )
>> > +            return -EINVAL;
>> > +
>> > +        p2m = d->arch.altp2m_p2m[altp2m_idx];
>> > +    }
>> > +    else
>> > +        p2m = hp2m;
>>
>> And I don't see why you need separate p2m and hp2m local
>> variables.
> 
> It was also used above for returning default access, while p2m is the
> actually active one here.

But all that could be done with just the one variable that was already
there.

Jan


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