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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-12-09 at 12:16 +0100, Egger, Christoph wrote:
> On 05.12.13 16:56, Ian Campbell wrote:
> > On Thu, 2013-12-05 at 16:51 +0100, Egger, Christoph wrote:
> >> On 05.12.13 16:42, 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
> >>>     - 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
> >>>
> >>> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> >>> ---
> >>>  xen/include/asm-arm/p2m.h |   13 ++++++++++---
> >>>  1 file changed, 10 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> >>> index f079f00..b24f94a 100644
> >>> --- a/xen/include/asm-arm/p2m.h
> >>> +++ b/xen/include/asm-arm/p2m.h
> >>> @@ -20,6 +20,16 @@ struct p2m_domain {
> >>>      uint8_t vmid;
> >>>  };
> >>>  
> >>> +typedef enum {
> >>> +    p2m_invalid = 0,        /* Nothing mapped here */
> >>> +    p2m_ram_rw = 1,         /* Normal read/write guest RAM */
> >>> +    p2m_ram_ro = 2,         /* Read-only; writes are silently dropped */
> >>> +    p2m_mmio_direct = 3,    /* Read/write mapping of genuine MMIO area */
> >>> +    p2m_map_foreign = 4,    /* Ram pages from foreign domain */
> >>> +} p2m_type_t;
> >>> +
> >>> +#define p2m_is_foreign(_t) ((_t) == p2m_map_foreign)
> >>> +
> >>
> >> Is it possible to merge p2m_type_t with x86 and move into common code?
> > 
> > Not really, the p2m handling is very different on the two arches, even
> > if they look superficially similar at this level.
> 
> That does not imply there is no common logic from the algorithm POV.
> Do you see a way split the algorithm into architecture-specific and
> architecture-independent parts?

The question was can we move p2m_type_t to common code. This is clearly
not possible because the PTEs in both cases have different number of
available bits, and the x86 case has particular requirements about which
type is represented by pattern 0 (due to some AMD IOMMU behaviour,
judging from the comment)

> > x86 has more types than can fit in the available pte space on ARM for a
> > start.
> >
> > I'd like to keep them separate please.
> 
> Ok. Then leave the declaration in the architecture specific part
> and make the accessors available for the common code.
> In OO-languages this is called an 'interface'.

Thanks, I had never heard of an 'interface'. </sarcasm>.

What do you think we are defining here if it is not an interface?

Ian.


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