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

["Lengyel, Tamas" <tlengyel@xxxxxxxxxxx>]

On Jan 28, 2016 7:56 AM, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>
> >>> 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 = ""> > >> >
> >> > -Â Â Â Â 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

Sure, SGTM.

Tamas

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