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

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

On Fri, Sep 12, 2014 at 9:41 PM, Julien Grall <julien.grall@xxxxxxxxxx> 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@xxxxxxxxxx

<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

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