Re: [Xen-devel] [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory
On Mon, Sep 19, 2016 at 05:22:27PM +0200, Julien Grall wrote:
> Hi Edgar,
Hi Julien,
Sorry for the late reply!
>
> 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"
> >>>
> >>>Add support for describing normal non-cacheable memory.
> >>>
> >>>Signed-off-by: Edgar E. Iglesias
> >>>---
> >>>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.
Yes, I meant that it's a little strange to have them differ, e.g:
p2m_mmio_direct_nc xn = 0
p2m_mmio_direct_cxn = 1
Because intuitively, when looking at those types I would expect
them to be identical except for the cacheability...
> 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?
Agreed. I personally don't see a reason why normal memory would cause problems
but it may be a little late in the release cycle to risk anything. I can
make the new p2m_mmio_direct_nc XN=1 and we can discuss a relaxation for the
next release?
Cheers,
Edgar
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
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"
Add support for describing normal non-cacheable memory.
Signed-off-by: Edgar E. Iglesias
---
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@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory
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"
> >
> >Add support for describing normal non-cacheable memory.
> >
> >Signed-off-by: Edgar E. Iglesias
> >---
> > 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 think there are use-cases for running guests that will want to
execute from on chip memories. But we can cross that bridge when
we come to it...
Cheers,
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
Re: [Xen-devel] [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory
On Fri, Sep 16, 2016 at 04:34:36PM +0200, Julien Grall wrote:
> Hi Edgar,
>
> On 07/09/2016 08:56, Edgar E. Iglesias wrote:
> >From: "Edgar E. Iglesias"
> >
> >Add support for describing normal non-cacheable memory.
>
> I am a bit confused, you introduced this new p2m type but I don't see any
> usage of it in this series. Is it expected?
Hi Julien,
It's supposed to be used in patch 6
xen/arm: Map mmio-sram nodes as un-cached memory
But there's a typo using p2m_mmio_direct_c instead of p2m_mem_nc :-)
I'll fix that in v4.
Cheers,
Edgar
> Regards,
>
> >
> >Signed-off-by: Edgar E. Iglesias
> >---
> > 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
> >+ * Shareable for Normal Inner Non_cacheable, Outer Non-cacheable.
> >+ */
> >+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 */
> > 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 */
> >
>
> --
> Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory
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"
> >
> >Add support for describing normal non-cacheable memory.
> >
> >Signed-off-by: Edgar E. Iglesias
> >---
> > 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
Re: [Xen-devel] [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory
Hi Edgar,
On 07/09/2016 08:56, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias"
Add support for describing normal non-cacheable memory.
I am a bit confused, you introduced this new p2m type but I don't see
any usage of it in this series. Is it expected?
Regards,
Signed-off-by: Edgar E. Iglesias
---
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
+ * Shareable for Normal Inner Non_cacheable, Outer Non-cacheable.
+ */
+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 */
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 */
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/6] xen/arm: p2m: Add support for normal non-cacheable memory
Hi Edgar,
On 07/09/2016 08:56, Edgar E. Iglesias wrote:
From: "Edgar E. Iglesias"
Add support for describing normal non-cacheable memory.
Signed-off-by: Edgar E. Iglesias
---
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?
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
