Re: [PATCH v5 09/17] xen/arm: p2m type definitions and changes

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

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

On Fri, Sep 12, 2014 at 9:41 PM, Julien Grall <julien.grall@linaro.org>
wrote:

> Hello Tamas,
>
> On 12/09/14 01:31, Tamas K Lengyel wrote:
>
>>
>>
>> On Thu, Sep 11, 2014 at 10:49 PM, Julien Grall <julien.grall@linaro.org
>> <mailto:julien.grall@linaro.org>> wrote:
>>
>>     Hi Tamas,
>>
>>     On 10/09/14 06:28, Tamas K Lengyel wrote:
>>       >   static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int
>>     mattr,
>>
>>         -                               p2m_type_t t)
>>         +                               p2m_type_t t, p2m_access_t a)
>>            {
>>                paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT;
>>                /* sh, xn and write bit will be defined in the following
>>         switches
>>         @@ -347,7 +350,7 @@ static int p2m_create_table(struct domain
>>         *d, lpae_t *entry,
>>                     for ( i=0 ; i < LPAE_ENTRIES; i++ )
>>                     {
>>                         pte = mfn_to_p2m_entry(base_pfn +
>>         (i<<(level_shift-LPAE_SHIFT)),
>>         -                                    MATTR_MEM, t);
>>         +                                    MATTR_MEM, t,
>>         p2m->default_access);
>>
>>
>>     I though I've talked about it in an earlier version. I don't think
>>     we should use the default_access to the P2M table.
>>
>>     Let assume a user decides to set default to another access type than
>>     p2m_access_rwx, Xen will receive the same trap forever for the
>>     domain because the access is not store in the radix tree (see your
>>     patch #12). Therefore the guest will try to access back to the
>>     guest, but the page attribute has not changed.
>>
>>     Also, you can't associate a PFN to this page because the table is
>>     internal to Xen.
>>
>>     IHMO, I would use p2m_access_rwx for the table L1 and L2 tables. For
>>     L3, you will have to set the permission in the radix tree.
>>
>>
>> I'm not sure I follow exactly what you are saying. Setting
>> default_access only effects pages created after the fact and I see that
>> if a large page is created this still causes a problem.. Is that what
>> you mean? A better solution would be to only allow creating 4k pages if
>> default_access != rwx IMHO and then store the setting in the radix tree
>> automatically.
>>
>
> That what I was trying to say.
>
> You can do it easily in the function is_mapping_aligned.
>
> But for the intermediate page table (see the mfn_to_p2m_entry(...,...,
> p2m_invalid), you should not use default_access but directly access_rwx.
>

Ack.


>
>
>
>> I was also contemplating if it would make sense if a setting was not
>> found in the radix tree, to check the violation against the
>> default_access.
>>
>
> The violation of the page may be different than the default access. I
> think it could be a mess for the guest. The radix tree should containe very
> pte where the access != rwx.
>
>
>  That way we would not have to store those entries in the
>> radix tree that are the same as the default_access (and rwx of course
>> would not go in the radix tree either). Might cut down the size of the
>> tree.
>>
>
> The default_access may change after the insertion of the PTE. How will you
> handle this case? IHMO, I don't think there is simple solution.
>
>
Yea, I gave it some thought today and it became quite complex so it's not
worth the trouble.


> Regards,
>
> --
> Julien Grall
>

Thanks!
Tamas

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

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

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