[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory
Hi Edgar,
On 16/09/2016 18:17, Edgar E. Iglesias wrote:
On Fri, Sep 16, 2016 at 04:21:12PM +0200, Julien Grall wrote:
Hi Edgar,
On 07/09/2016 08:56, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
Add support for describing normal non-cacheable memory.
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
---
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/
+ * 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.
+ */
+ 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?
Something that shows up after doing the rename and otherwise keeping the
patch is that we treat the XN bit differently for p2m_mmio_direct_nc
and p2m_mmio_direct_c.
Is there any reason why we can't allow execution for p2m_mmio_direct_c
mappings?
If so, perhaps that same reason also applies to p2m_mmio_direct_nc and
both should be non-executable.
I guess you mean your new p2m_mmio_direct_nc and not the current one. If
so, I think it is more a safety. Until now, the p2m type was used to
share ACPI tables which should not be executable.
I am not sure what would be the implication to unset the NX bit by
default. The question to answer before any relaxation is could a
potential misbehave guest harm the platform?
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
