Re: [Xen-devel] [PATCH v4 13/16] xen/grant: Switch {create, replace}_grant_p2m_mapping to typesafe MFN

[Jan Beulich]

>>> On 21.02.18 at 15:02,  wrote:
> The current prototype is slightly confusing because it takes a guest
> physical address and a machine physical frame (not address!). Switching to
> MFN will improve safety and reduce the chance to mistakenly invert the
> 2 parameters.
> 
> Signed-off-by: Julien grall 

Reviewed-by: Jan Beulich 



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 13/16] xen/grant: Switch {create, replace}_grant_p2m_mapping to typesafe MFN

[Wei Liu]

On Wed, Feb 21, 2018 at 02:02:56PM +, Julien Grall wrote:
> The current prototype is slightly confusing because it takes a guest
> physical address and a machine physical frame (not address!). Switching to
> MFN will improve safety and reduce the chance to mistakenly invert the
> 2 parameters.
> 
> Signed-off-by: Julien grall 
> 

Reviewed-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 13/16] xen/grant: Switch {create, replace}_grant_p2m_mapping to typesafe MFN

[Julien Grall]

The current prototype is slightly confusing because it takes a guest
physical address and a machine physical frame (not address!). Switching to
MFN will improve safety and reduce the chance to mistakenly invert the
2 parameters.

Signed-off-by: Julien grall 

---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 

Changes in v4:
- Patch added
---
 xen/arch/arm/mm.c | 10 +-
 xen/arch/x86/hvm/grant_table.c| 14 +++---
 xen/arch/x86/pv/grant_table.c | 10 +-
 xen/common/grant_table.c  |  8 
 xen/include/asm-arm/grant_table.h |  9 -
 xen/include/asm-x86/grant_table.h |  4 ++--
 xen/include/asm-x86/hvm/grant_table.h |  8 
 xen/include/asm-x86/pv/grant_table.h  |  8 
 8 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index f17907ace8..4268dd5c2d 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1423,7 +1423,7 @@ void gnttab_mark_dirty(struct domain *d, unsigned long l)
 }
 }
 
-int create_grant_host_mapping(unsigned long addr, unsigned long frame,
+int create_grant_host_mapping(unsigned long addr, mfn_t frame,
   unsigned int flags, unsigned int cache_flags)
 {
 int rc;
@@ -1436,7 +1436,7 @@ int create_grant_host_mapping(unsigned long addr, 
unsigned long frame,
 t = p2m_grant_map_ro;
 
 rc = guest_physmap_add_entry(current->domain, gaddr_to_gfn(addr),
- _mfn(frame), 0, t);
+ frame, 0, t);
 
 if ( rc )
 return GNTST_general_error;
@@ -1444,8 +1444,8 @@ int create_grant_host_mapping(unsigned long addr, 
unsigned long frame,
 return GNTST_okay;
 }
 
-int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
-unsigned long new_addr, unsigned int flags)
+int replace_grant_host_mapping(unsigned long addr, mfn_t mfn,
+   unsigned long new_addr, unsigned int flags)
 {
 gfn_t gfn = gaddr_to_gfn(addr);
 struct domain *d = current->domain;
@@ -1454,7 +1454,7 @@ int replace_grant_host_mapping(unsigned long addr, 
unsigned long mfn,
 if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
 return GNTST_general_error;
 
-rc = guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
+rc = guest_physmap_remove_page(d, gfn, mfn, 0);
 
 return rc ? GNTST_general_error : GNTST_okay;
 }
diff --git a/xen/arch/x86/hvm/grant_table.c b/xen/arch/x86/hvm/grant_table.c
index 9ca9fe0425..ecd7d078ab 100644
--- a/xen/arch/x86/hvm/grant_table.c
+++ b/xen/arch/x86/hvm/grant_table.c
@@ -25,7 +25,7 @@
 
 #include 
 
-int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
+int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
  unsigned int flags,
  unsigned int cache_flags)
 {
@@ -41,14 +41,14 @@ int create_grant_p2m_mapping(uint64_t addr, unsigned long 
frame,
 p2mt = p2m_grant_map_rw;
 rc = guest_physmap_add_entry(current->domain,
  _gfn(addr >> PAGE_SHIFT),
- _mfn(frame), PAGE_ORDER_4K, p2mt);
+ frame, PAGE_ORDER_4K, p2mt);
 if ( rc )
 return GNTST_general_error;
 else
 return GNTST_okay;
 }
 
-int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
+int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
   uint64_t new_addr, unsigned int flags)
 {
 unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
@@ -60,15 +60,15 @@ int replace_grant_p2m_mapping(uint64_t addr, unsigned long 
frame,
 return GNTST_general_error;
 
 old_mfn = get_gfn(d, gfn, );
-if ( !p2m_is_grant(type) || mfn_x(old_mfn) != frame )
+if ( !p2m_is_grant(type) || !mfn_eq(old_mfn, frame) )
 {
 put_gfn(d, gfn);
 gdprintk(XENLOG_WARNING,
- "old mapping invalid (type %d, mfn %" PRI_mfn ", frame 
%lx)\n",
- type, mfn_x(old_mfn), frame);
+ "old mapping invalid (type %d, mfn %" PRI_mfn ", frame 
%"PRI_mfn")\n",
+ type, mfn_x(old_mfn), mfn_x(frame));
 return GNTST_general_error;
 }
-if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K) )
+if ( guest_physmap_remove_page(d, _gfn(gfn), frame, PAGE_ORDER_4K) )
 {
 put_gfn(d, gfn);
 return GNTST_general_error;
diff --git a/xen/arch/x86/pv/grant_table.c b/xen/arch/x86/pv/grant_table.c
index 4dbc550366..458085e1b6