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

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

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.

Regards,

-- 
Julien Grall