From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH v5 09/17] xen/arm: p2m type definitions and changes Date: Fri, 12 Sep 2014 22:20:24 +0200 Message-ID: References: <1410355726-5599-1-git-send-email-tklengyel@sec.in.tum.de> <1410355726-5599-10-git-send-email-tklengyel@sec.in.tum.de> <54120AD7.4080201@linaro.org> <54134C4C.10806@linaro.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6876390472353989770==" Return-path: In-Reply-To: <54134C4C.10806@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: Ian Campbell , Tim Deegan , Ian Jackson , "xen-devel@lists.xen.org" , Stefano Stabellini , Andres Lagar-Cavilla , Jan Beulich , Daniel De Graaf , Tamas K Lengyel List-Id: xen-devel@lists.xenproject.org --===============6876390472353989770== Content-Type: multipart/alternative; boundary=001a113a53d2df76ce0502e4035c --001a113a53d2df76ce0502e4035c Content-Type: text/plain; charset=ISO-8859-1 On Fri, Sep 12, 2014 at 9:41 PM, Julien Grall wrote: > Hello Tamas, > > On 12/09/14 01:31, Tamas K Lengyel wrote: > >> >> >> On Thu, Sep 11, 2014 at 10:49 PM, Julien Grall > > 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 --001a113a53d2df76ce0502e4035c Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable


On Fri, Sep 12, 2014 at 9:41 PM, Julien Grall <julien.grall@lina= ro.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:jul= ien.grall@linaro.org>> wrote:

=A0 =A0 Hi Tamas,

=A0 =A0 On 10/09/14 06:28, Tamas K Lengyel wrote:
=A0 =A0 =A0 >=A0 =A0static lpae_t mfn_to_p2m_entry(unsigned long mfn, un= signed int
=A0 =A0 mattr,

=A0 =A0 =A0 =A0 -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0p2m_type_t t)
=A0 =A0 =A0 =A0 +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0p2m_type_t t, p2m_access_t a)
=A0 =A0 =A0 =A0 =A0 =A0{
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0paddr_t pa =3D ((paddr_t) mfn) << PAGE= _SHIFT;
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* sh, xn and write bit will be defined in t= he following
=A0 =A0 =A0 =A0 switches
=A0 =A0 =A0 =A0 @@ -347,7 +350,7 @@ static int p2m_create_table(struct doma= in
=A0 =A0 =A0 =A0 *d, lpae_t *entry,
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 for ( i=3D0 ; i < LPAE_ENTRIES; = i++ )
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pte =3D mfn_to_p2m_entry(ba= se_pfn +
=A0 =A0 =A0 =A0 (i<<(level_shift-LPAE_SHIFT)),
=A0 =A0 =A0 =A0 -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 MATTR_MEM, t);
=A0 =A0 =A0 =A0 +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 MATTR_MEM, t,
=A0 =A0 =A0 =A0 p2m->default_access);


=A0 =A0 I though I've talked about it in an earlier version. I don'= t think
=A0 =A0 we should use the default_access to the P2M table.

=A0 =A0 Let assume a user decides to set default to another access type tha= n
=A0 =A0 p2m_access_rwx, Xen will receive the same trap forever for the
=A0 =A0 domain because the access is not store in the radix tree (see your<= br> =A0 =A0 patch #12). Therefore the guest will try to access back to the
=A0 =A0 guest, but the page attribute has not changed.

=A0 =A0 Also, you can't associate a PFN to this page because the table = is
=A0 =A0 internal to Xen.

=A0 =A0 IHMO, I would use p2m_access_rwx for the table L1 and L2 tables. Fo= r
=A0 =A0 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 !=3D 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.
= =A0



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 !=3D 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.
= =A0
Regards,

--
Julien Grall

Thank= s!
Tamas
--001a113a53d2df76ce0502e4035c-- --===============6876390472353989770== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============6876390472353989770==--