Re: [Xen-devel] [PATCH 3/8] xen/arm: Implement p2m_type_t as an enum

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 12/05/2013 04:38 PM, Ian Campbell wrote:
> On Thu, 2013-12-05 at 16:28 +0000, Julien Grall wrote:
> > 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.
> "This" is what I suggested here or what you wrote already?
The code I wrote.

> > > 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.
> I guess consistency is a good reason to do the same then.
Ok. So I will add p2m_grant_map_ro and p2m_grant_map_rw.

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel