Re: [PATCH for-4.5 v6 10/17] xen/arm: p2m type definitions and changes

[Julien Grall <julien.grall@linaro.org>]

Hi Ian,

On 16/09/14 06:27, Ian Campbell wrote:
> On Tue, 2014-09-16 at 10:49 +0200, Tamas K Lengyel wrote:
>>
>>
>> On Tue, Sep 16, 2014 at 12:35 AM, Ian Campbell
>> <ian.campbell@citrix.com> wrote:
>>          On Mon, 2014-09-15 at 16:02 +0200, Tamas K Lengyel wrote:
>>          > Define p2m_access_t in ARM and add necessary changes for
>>          page table
>>          > construction routines to pass the default access
>>          information. Also,
>>          > define the Radix tree that will hold access permission
>>          settings as
>>          > the PTE's don't have enough software programmable bits
>>          available.
>>
>>          So my main concern here is the overhead for non-xenaccess
>>          users. I think
>>          it amounts to a few extra fields in the p2m_domain struct
>>          which I can
>>          see here and presumably some NULL vs. non-NULL type checks
>>          which I guess
>>          we will get to later. The important thing is that the fast
>>          paths for the
>>          common case don't get a lot of extra overhead.
>>
>>          WRT the xenaccess performance did you consider any options
>>          other than a
>>          radix tree (which seems quite expensive to me)? e.g. perhaps
>>          allocating
>>          (only when needed) as second page for each real T page as a
>>          shadow/extended region? Perhaps pointed to by a filed in the
>>          real PT
>>          struct page_info. I'm sure there are other possible ideas too.
>>
>>          Ian,
>>
>>
>> Those would all be possible solutions. I used the Radix tree
>> implementation already in Xen as a matter of convenience and because
>> it has an acceptable overhead. I certainly don't oppose further
>> optimizing this code, I'm just not sure if it needs to happen now,
>> provided feature freeze is rapidly approaching. The main concern
>> should be impact on non-xenaccess code-paths, which I agree are a
>> showstopper of any feature like this. If you say it can't be merged
>> unless the xenaccess code-path is also optimized I'm afraid this
>> series will be postponed till 4.6 as I won't have the time to test out
>> which approach puts the least overhead on the system under what
>> usage-scenarios etc in this timeframe.
>
> WRT merging I'm only concerned about the impact on non-xenaccess uses,
> the stuff about the xenaccess-on case was just idle wondering, sorry for
> not making that clear.

I didn't write this code, but read it multiple time and ask the similar 
question to Tamas few version ago.

So, the radix tree is only used when the access type of the page is 
different than access_rwx. This is the default access when xenaccess is 
not used.

AFAIU, the only overhead we have is the new fields in the arch_domain 
structure.

Regards,

-- 
Julien Grall