From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 3/8] xen/arm: Implement p2m_type_t as an enum Date: Thu, 05 Dec 2013 16:28:37 +0000 Message-ID: <52A0A9B5.5060705@linaro.org> References: <1386258131-755-1-git-send-email-julien.grall@linaro.org> <1386258131-755-4-git-send-email-julien.grall@linaro.org> <1386258750.20047.84.camel@kazak.uk.xensource.com> <52A0A36F.7070908@linaro.org> <1386260093.20047.101.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" 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 1Vobn8-0000FH-Lq for xen-devel@lists.xenproject.org; Thu, 05 Dec 2013 16:28:42 +0000 Received: by mail-bk0-f50.google.com with SMTP id e11so7260434bkh.37 for ; Thu, 05 Dec 2013 08:28:40 -0800 (PST) In-Reply-To: <1386260093.20047.101.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/05/2013 04:14 PM, Ian Campbell wrote: > On Thu, 2013-12-05 at 16:01 +0000, Julien Grall wrote: >> >> On 12/05/2013 03:52 PM, Ian Campbell wrote: >>> On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote: >>>> Until now, Xen doesn't know the type of the page (ram, foreign page, mmio,...). >>>> Introduce p2m_type_t with basic types: >>>> - p2m_invalid: Nothing is mapped here >>> >>> Do we really need this? Is it not equivalent to not setting the present >>> bit? I see x86 has the same type though -- Tim can you explain why. >> >> We need a default value when Xen retrieves the p2m type. I don't think >> we can assume that p2m_ram_rw (or any other type) is used by default. >> >>> Since the avail bits in the p2m pte are in pretty short supply I think >>> we can avoid unnecessary types. >> >> I plan to use directly the decimal value. So we can store up to 16 values. > > 16 is short supply in my book ;-) > > Having got a bit further through the series I see how p2m_invalid is > being used now. It is a useful pseudo-type but it doesn't need to be > represented in the avail bits I don't think. How about: > > typedef enum { > p2m_ram_rw, /* Normal read/write guest RAM */ > p2m_ram_ro, /* Read-only; writes are silently dropped */ > p2m_mmio_direct, /* Read/write mapping of genuine MMIO area / > p2m_map_foreign, /* Ram pages from foreign domain */ > > p2m_max_real_type = 16, /* Types after this are pseudo-types. */ > > p2m_invalid, /* Nothing mapped here */ > > } p2m_type_t; > > BUILD_BUG_ON(p2m_max_real_type >= 2^4); > > Now you can return it etc but it never needs to get put in an actual > pte? This solution was easier to avoid extra code in the different function. I will rework it for the next series. > > Maybe this is one for the future when we get a bit short on bits. > >>>> - p2m_ram_rw: Normal read/write guest RAM >>>> - p2m_ram_ro: Read-only guest RAM >>>> - p2m_mmio_direct: Read/write mapping of device memory >>>> - p2m_map_foreign: RAM page from foreign guest >>> >>> Is there no need for an entry for a grant mapping (and a ro >>> counterpart)? >> >> Hmmm .. actually grant table is mapped as RAM (so read/write and >> execute). Do we want to allow code execution from grant-mapping page? >> If not, then we will need to introduce specific p2m type from grant-mapping. > > If a guest is stupid enough to execute code from a page owned by another > guest then it gets what it deserves ;-) Actually X86, disable execution on grant and foreign mapping. > My question wasn't about that though -- just whether it is useful for > Xen to track whether the particular RAM mapping is normal or a grant > mapping. For now, I don't see a specific reason to track it. -- Julien Grall