From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 04/10] xen/arm: Store p2m type in each page of the guest Date: Mon, 09 Dec 2013 16:37:06 +0000 Message-ID: <52A5F1B2.4070604@linaro.org> References: <1386560047-17500-1-git-send-email-julien.grall@linaro.org> <1386560047-17500-5-git-send-email-julien.grall@linaro.org> <1386605026.7812.37.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Vq3pW-0004fC-Hw for xen-devel@lists.xenproject.org; Mon, 09 Dec 2013 16:37:10 +0000 Received: by mail-ea0-f182.google.com with SMTP id a15so1678538eae.41 for ; Mon, 09 Dec 2013 08:37:08 -0800 (PST) In-Reply-To: <1386605026.7812.37.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 12/09/2013 04:03 PM, Ian Campbell wrote: > On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote: >> Use the field 'avail' to store the type of the page. Rename it to 'p2mt' for >> convenience. > > The p2m is redundant in most uses, and the t isn't very descriptive. I > think "type" would be fine. > >> The information stored in this field will be retrieved in a future patch to >> hange the behaviour when the page is removed. > > "change" > >> Also introduce guest_physmap_add_entry to map and set a specific p2m type for >> a page. >> >> Signed-off-by: Julien Grall >> >> --- >> Changes in v2: >> - Rename 'avail' field to 'p2mt' in the p2m structure >> - Add BUILD_BUG_ON to check if the enum value will fit in the field >> - Implement grant mapping type >> --- >> xen/arch/arm/p2m.c | 58 ++++++++++++++++++++++++++++++++------------ >> xen/include/asm-arm/p2m.h | 18 +++++++++++--- >> xen/include/asm-arm/page.h | 2 +- >> 3 files changed, 58 insertions(+), 20 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index 8f8b47e..e43804c 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -124,7 +124,8 @@ int p2m_pod_decrease_reservation(struct domain *d, >> return -ENOSYS; >> } >> >> -static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) >> +static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr, >> + p2m_type_t t) >> { >> paddr_t pa = ((paddr_t) mfn) << PAGE_SHIFT; >> lpae_t e = (lpae_t) { >> @@ -132,12 +133,34 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr) >> .p2m.af = 1, >> .p2m.sh = LPAE_SH_OUTER, >> .p2m.read = 1, >> - .p2m.write = 1, >> .p2m.mattr = mattr, >> .p2m.table = 1, >> .p2m.valid = 1, >> + .p2m.p2mt = t, >> }; >> >> + BUILD_BUG_ON(p2m_max_real_type > (2 << 4)); >> + >> + switch (t) >> + { >> + case p2m_grant_map_rw: >> + e.p2m.xn = 1; >> + /* Fallthrough */ >> + case p2m_ram_rw: >> + case p2m_mmio_direct: >> + case p2m_map_foreign: >> + e.p2m.write = 1; >> + break; >> + >> + case p2m_grant_map_ro: >> + e.p2m.xn = 1; >> + /* Fallthrough */ >> + case p2m_invalid: >> + case p2m_ram_ro: >> + default: > > I think you've enumerated all the options, so it is better to leave out > the default -- then at least some compilers will complain if we add a > new type and forget to add it here. > Ok. I will remove it. -- Julien Grall