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

[Tamas K Lengyel <tamas.lengyel@zentific.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3394 bytes --]

On Tue, Sep 16, 2014 at 10:38 PM, Julien Grall <julien.grall@linaro.org>
wrote:

> 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
>


Yeap, that's the basic outline of things (except that one thing I mentioned
in the other thread, which is already fixed in v7 of the series). I looked
into switching to an extended struct page_info and that looks like a good
alternative to consider. It seems to be like a faster lookup to get that
struct with get_page_from_gva then the radix lookup if the radix tree is
large.

Tamas

[-- Attachment #1.2: Type: text/html, Size: 4606 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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