From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v4 09/16] xen/arm: p2m type definitions and changes Date: Mon, 08 Sep 2014 17:06:58 -0700 Message-ID: <540E44A2.1050301@linaro.org> References: <1409907524-12509-1-git-send-email-tklengyel@sec.in.tum.de> <1409907524-12509-10-git-send-email-tklengyel@sec.in.tum.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1409907524-12509-10-git-send-email-tklengyel@sec.in.tum.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas K Lengyel , xen-devel@lists.xen.org Cc: ian.campbell@citrix.com, tim@xen.org, ian.jackson@eu.citrix.com, stefano.stabellini@citrix.com, andres@lagarcavilla.org, jbeulich@suse.com, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org Hi Tamas, On 05/09/14 01:58, Tamas K Lengyel wrote: > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index 06c93a0..b2009ee 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -2,9 +2,54 @@ [..] > +#include > +#include Why do you need this 2 includes? > > struct domain; > > +/* List of possible type for each page in the p2m entry. > + * The number of available bit per page in the pte for this purpose is 4 bits. > + * So it's possible to only have 16 fields. If we run out of value in the > + * future, it's possible to use higher value for pseudo-type and don't store > + * them in the p2m entry. > + */ > +typedef enum { > + p2m_invalid = 0, /* Nothing mapped here */ > + 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_grant_map_rw, /* Read/write grant mapping */ > + p2m_grant_map_ro, /* Read-only grant mapping */ > + /* The types below are only used to decide the page attribute in the P2M */ > + p2m_iommu_map_rw, /* Read/write iommu mapping */ > + p2m_iommu_map_ro, /* Read-only iommu mapping */ > + p2m_max_real_type, /* Types after this won't be store in the p2m */ > +} p2m_type_t; > + Any reason to move the enum earlier? If not, I would prefer to keep at the same place. It will be easier with git-blame to find when a new type has been added. > +/* > + * Additional access types, which are used to further restrict > + * the permissions given by the p2m_type_t memory type. Violations > + * caused by p2m_access_t restrictions are sent to the mem_event > + * interface. > + * > + * The access permissions are soft state: when any ambigious change of page ambiguous [..] > + /* Default P2M access type for each page in the the domain: new pages, > + * swapped in pages, cleared pages, and pages that are ambiquously Did you intend to mean ambiguously rather than ambiquously? [..] > +/* get host p2m table */ > +#define p2m_get_hostp2m(d) (&((d)->arch.p2m)) > + > +/* mem_event and mem_access are supported on all ARM */ I don't find "all ARM" clear. I would replace by "any ARM guest" Regards, -- Julien Grall