Re: [Xen-devel] [PATCH for-4.5 v6 13/17] xen/arm: Data abort exception (R/W) mem_events.

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Mon, Sep 22, 2014 at 11:11 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:

On Fri, 2014-09-19 at 11:05 +0200, Tamas K Lengyel wrote:
>
>
> On Thu, Sep 18, 2014 at 10:09 PM, Tamas K Lengyel
> <tamas.lengyel@xxxxxxxxxxxx> wrote:
>
>
>
>         On Thu, Sep 18, 2014 at 8:54 PM, Ian Campbell
>         <ian.campbell@xxxxxxxxxx> wrote:
>                 On Mon, 2014-09-15 at 16:02 +0200, Tamas K Lengyel
>                 wrote:
>
>                 > -        if ( is_mapping_aligned(*addr, end_gpaddr,
>                 0, level_size) )
>                 > +        if ( level < 3 && p2m_access_rwx != a )
>                 > +        {
>                 > +            /* We create only 4k pages when
>                 mem_access is in use. */
>
>                 I wonder if it might turn out cleaner to integrate
>                 this check into
>                 is_mapping_aligned (which really is more of a "can we
>                 use a superpage"
>                 function).
>
>                 i.e.
>                         /* mem access cannot use super pages */
>                         if ( a != p2m_access_rwx && level_size !=
>                 THIRD_SIZE )
>                                 return false;
>
>
>         Ack, that would make sense.
>
>
>
>
>
> One a closer look, is_mapping_aligned is also used when removing
> pages, where we would need to add an exception.. so it doesn't look
> cleaner after all.

Hrm. The two things I'd like to try and avoid are the open coding of the
memaccess logic and more importantly the weird "empty if clause" thing
which this adds.

Perhaps an is_memaccess_enabled(...) (or /is..._active/used? or
is_superpage_allowed) predicate folded in with the existing
is_mapping_aligned check?

BTW, how come you only do this for ALLOCATE and not INSERT? I'd be
surprised if you were hitting the ALLOCATE case since it is only used
for the non-1:1 dom0 case (which no platform actually uses today)

Ian.

Hm, I might have lost the block in this revision which does it for INSERT as well.

Tamas

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