From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
To: Julien Grall <julien.grall@arm.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
sstabellini@kernel.org, xen-devel@lists.xen.org
Subject: Re: [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory
Date: Fri, 16 Sep 2016 17:08:03 +0200 [thread overview]
Message-ID: <20160916150803.GM28422@toto> (raw)
In-Reply-To: <cf1efc60-709c-4efb-0694-c83f421406a1@arm.com>
On Fri, Sep 16, 2016 at 04:21:12PM +0200, Julien Grall wrote:
> Hi Edgar,
Hi Julien,
Thanks for the review!
> On 07/09/2016 08:56, Edgar E. Iglesias wrote:
> >From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> >Add support for describing normal non-cacheable memory.
> >
> >Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >---
> > xen/arch/arm/p2m.c | 18 ++++++++++++++++++
> > xen/include/asm-arm/p2m.h | 1 +
> > xen/include/asm-arm/page.h | 1 +
> > 3 files changed, 20 insertions(+)
> >
> >diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> >index b648a9d..bfef77b 100644
> >--- a/xen/arch/arm/p2m.c
> >+++ b/xen/arch/arm/p2m.c
> >@@ -282,6 +282,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a)
> > /* First apply type permissions */
> > switch ( t )
> > {
> >+ case p2m_mem_nc:
> > case p2m_ram_rw:
> > e->p2m.xn = 0;
> > e->p2m.write = 1;
> >@@ -376,6 +377,23 @@ static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t, p2m_access_t a)
> > e.p2m.sh = LPAE_SH_OUTER;
> > break;
> >
> >+ /*
> >+ * ARM ARM: Overlaying the shareability attribute (DDI
> >+ * 0406C.b B3-1376 to 1377)
> >+ *
> >+ * A memory region with a resultant memory type attribute of Normal,
> >+ * and a resultant cacheability attribute of Inner Non-cacheable,
> >+ * Outer Non-cacheable, must have a resultant shareability attribute
> >+ * of Outer Shareable, otherwise shareability is UNPREDICTABLE.
> >+ *
> >+ * On ARMv8 sharability is ignored and explicitly treated as Outer
>
> I know you copied it from mfn_to_xen_entry, but can we fixed the copy with:
>
> s/sharability/shareability/
Fixed for v4.
>
> >+ * Shareable for Normal Inner Non_cacheable, Outer Non-cacheable.
>
> s/_/-/
>
> Also I would like to see a spec reference for the ARMv8. I think it is the
> note in D4-1788 ARM DDI 0487A.j.
Fixed for v4.
>
> >+ */
> >+ case p2m_mem_nc:
> >+ e.p2m.mattr = MATTR_MEM_NC;
> >+ e.p2m.sh = LPAE_SH_OUTER;
> >+ break;
> >+
> > default:
> > e.p2m.mattr = MATTR_MEM;
> > e.p2m.sh = LPAE_SH_INNER;
> >diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> >index 53c4d78..b012d50 100644
> >--- a/xen/include/asm-arm/p2m.h
> >+++ b/xen/include/asm-arm/p2m.h
> >@@ -93,6 +93,7 @@ typedef enum {
> > p2m_ram_ro, /* Read-only; writes are silently dropped */
> > p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area non-cacheable */
> > p2m_mmio_direct_c, /* Read/write mapping of genuine MMIO area cacheable */
> >+ p2m_mem_nc, /* Read/write mapping of Non-cacheable Memory */
>
> I find the name a bit confusing. Technically p2m_mem_nc is p2m_mmio_direct_c
> version but non-cacheable.
>
> I have got the feeling that the naming I used on a recent patch is not
> correct. Because p2m_mmio_direct_nc is not doing what is expect (i.e mapping
> non-cacheable). It maps with the device memory attribute.
>
> Maybe we should rename p2m_mmio_direct_nc to p2m_mmio_direct_dev (because it
> will use the device memory attribute) and then use p2m_mmio_direct_nc for
> your purpose.
>
> Any opinions?
Sounds good, I'll do that.
To keep the patches more readable, I thought i'd first do the rename in
a first patch without any functional changes. Then I'll follow up with
re-adding the p2m_mmio_direct_nc for my purposes. Let me know if you prefer
something different.
Thanks,
Edgar
>
>
> > p2m_map_foreign, /* Ram pages from foreign domain */
> > p2m_grant_map_rw, /* Read/write grant mapping */
> > p2m_grant_map_ro, /* Read-only grant mapping */
> >diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> >index 05d9f82..9adc973 100644
> >--- a/xen/include/asm-arm/page.h
> >+++ b/xen/include/asm-arm/page.h
> >@@ -73,6 +73,7 @@
> > *
> > */
> > #define MATTR_DEV 0x1
> >+#define MATTR_MEM_NC 0x5
> > #define MATTR_MEM 0xf
> >
> > /* Flags for get_page_from_gva, gvirt_to_maddr etc */
> >
>
> Regards,
>
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-09-16 15:08 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-07 6:56 [PATCH v3 0/6] xen/arm: Add support for mapping mmio-sram nodes into dom0 Edgar E. Iglesias
2016-09-07 6:56 ` [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory Edgar E. Iglesias
2016-09-16 14:21 ` Julien Grall
2016-09-16 15:08 ` Edgar E. Iglesias [this message]
2016-09-16 16:17 ` Edgar E. Iglesias
2016-09-19 15:22 ` Julien Grall
2016-09-23 17:39 ` Edgar E. Iglesias
2016-09-16 14:34 ` Julien Grall
2016-09-16 15:54 ` Edgar E. Iglesias
2016-09-07 6:56 ` [PATCH v3 2/6] xen/arm: Rename and generalize un/map_regions_rw_cache Edgar E. Iglesias
2016-09-16 14:24 ` Julien Grall
2016-09-16 15:31 ` Edgar E. Iglesias
2016-09-07 6:56 ` [PATCH v3 3/6] xen/device-tree: Add __DT_MATCH macros without braces Edgar E. Iglesias
2016-09-07 6:56 ` [PATCH v3 4/6] xen/device-tree: Make dt_match_node match props Edgar E. Iglesias
2016-09-16 14:28 ` Julien Grall
2016-09-16 15:47 ` Edgar E. Iglesias
2016-09-07 6:56 ` [PATCH v3 5/6] xen/arm: domain_build: Plumb for different mapping attributes Edgar E. Iglesias
2016-09-16 14:31 ` Julien Grall
2016-09-16 16:00 ` Edgar E. Iglesias
2016-09-07 6:56 ` [PATCH v3 6/6] xen/arm: Map mmio-sram nodes as un-cached memory Edgar E. Iglesias
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160916150803.GM28422@toto \
--to=edgar.iglesias@xilinx.com \
--cc=edgar.iglesias@gmail.com \
--cc=julien.grall@arm.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).