Re: [Xen-devel] [PATCH v4 14/16] xen/grant: Switch common/grant_table.c to use typesafe MFN

[Jan Beulich]

>>> On 02.03.18 at 16:59,  wrote:
> On 02/03/18 15:54, Jan Beulich wrote:
> On 21.02.18 at 15:02,  wrote:
>>> @@ -872,7 +879,7 @@ map_grant_ref(
>>>   struct grant_table *lgt, *rgt;
>>>   struct vcpu   *led;
>>>   grant_handle_t handle;
>>> -unsigned long  frame = 0;
>>> +mfn_t frame = _mfn(0);
>> 
>> If the initializer is needed at all, I think it should again become
>> INVALID_MFN. Same in a few other places.
> 
> I didn't switch to INVALID_MFN because I wasn't sure if some place in 
> the code where relying on 0. If you think it is fine, then I am more 
> thank happy to switch to INVALID_MFN.

Well, as said - it looks as if the initializer isn't needed at all, in
which case it's value really is a don't care.

Jan


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

Re: [Xen-devel] [PATCH v4 14/16] xen/grant: Switch common/grant_table.c to use typesafe MFN

[Julien Grall]

Hi Jan,

On 02/03/18 15:54, Jan Beulich wrote:

On 21.02.18 at 15:02,  wrote:

@@ -872,7 +879,7 @@ map_grant_ref(
  struct grant_table *lgt, *rgt;
  struct vcpu   *led;
  grant_handle_t handle;
-unsigned long  frame = 0;
+mfn_t frame = _mfn(0);


If the initializer is needed at all, I think it should again become
INVALID_MFN. Same in a few other places.


I didn't switch to INVALID_MFN because I wasn't sure if some place in 
the code where relying on 0. If you think it is fine, then I am more 
thank happy to switch to INVALID_MFN.





--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -76,7 +76,7 @@ static inline unsigned int gnttab_dom0_max(void)
  #define gnttab_status_gmfn(d, t, i) \
  (mfn_to_gmfn(d, gnttab_status_mfn(t, i)))
  
-#define gnttab_mark_dirty(d, f) paging_mark_dirty((d), _mfn(f))

+#define gnttab_mark_dirty(d, f) paging_mark_dirty((d), f)


Please take the opportunity and also drop the stray parentheses
around d.


Sure.



With these taken care of and with Wei's R-b
Acked-by: Jan Beulich 


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 14/16] xen/grant: Switch common/grant_table.c to use typesafe MFN

[Jan Beulich]

>>> On 21.02.18 at 15:02,  wrote:
> @@ -872,7 +879,7 @@ map_grant_ref(
>  struct grant_table *lgt, *rgt;
>  struct vcpu   *led;
>  grant_handle_t handle;
> -unsigned long  frame = 0;
> +mfn_t frame = _mfn(0);

If the initializer is needed at all, I think it should again become
INVALID_MFN. Same in a few other places.

> --- a/xen/include/asm-x86/grant_table.h
> +++ b/xen/include/asm-x86/grant_table.h
> @@ -76,7 +76,7 @@ static inline unsigned int gnttab_dom0_max(void)
>  #define gnttab_status_gmfn(d, t, i) \
>  (mfn_to_gmfn(d, gnttab_status_mfn(t, i)))
>  
> -#define gnttab_mark_dirty(d, f) paging_mark_dirty((d), _mfn(f))
> +#define gnttab_mark_dirty(d, f) paging_mark_dirty((d), f)

Please take the opportunity and also drop the stray parentheses
around d.

With these taken care of and with Wei's R-b
Acked-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v4 14/16] xen/grant: Switch common/grant_table.c to use typesafe MFN

[Wei Liu]

On Wed, Feb 21, 2018 at 02:02:57PM +, Julien Grall wrote:
> No functional change intended.
> 
> 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 14/16] xen/grant: Switch common/grant_table.c to use typesafe MFN

[Julien Grall]

No functional change intended.

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 |   2 +-
 xen/common/grant_table.c  | 145 --
 xen/include/asm-arm/grant_table.h |   2 +-
 xen/include/asm-x86/grant_table.h |   2 +-
 4 files changed, 81 insertions(+), 70 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 4268dd5c2d..db74466a16 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1413,7 +1413,7 @@ void gnttab_clear_flag(unsigned long nr, uint16_t *addr)
 } while (cmpxchg(addr, old, old & mask) != old);
 }
 
-void gnttab_mark_dirty(struct domain *d, unsigned long l)
+void gnttab_mark_dirty(struct domain *d, mfn_t mfn)
 {
 /* XXX: mark dirty */
 static int warning;
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index c555c1d451..e9a81b66be 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -40,6 +40,12 @@
 #include 
 #include 
 
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef page_to_mfn
+#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
+#undef mfn_to_page
+#define mfn_to_page(mfn) __mfn_to_page(mfn_x(mfn))
+
 /* Per-domain grant information. */
 struct grant_table {
 /*
@@ -132,7 +138,7 @@ struct gnttab_unmap_common {
 
 /* Shared state beteen *_unmap and *_unmap_complete */
 uint16_t done;
-unsigned long frame;
+mfn_t frame;
 struct domain *rd;
 grant_ref_t ref;
 };
@@ -231,7 +237,7 @@ struct active_grant_entry {
 grant.*/
 grant_ref_t   trans_gref;
 struct domain *trans_domain;
-unsigned long frame;  /* Frame being granted. */
+mfn_t frame;  /* Frame being granted. */
 #ifndef NDEBUG
 gfn_t gfn;/* Guest's idea of the frame being granted. */
 #endif
@@ -336,14 +342,14 @@ static inline unsigned int 
grant_to_status_frames(unsigned int grant_frames)
If rc == GNTST_okay, *page contains the page struct with a ref taken.
Caller must do put_page(*page).
If any error, *page = NULL, *frame = INVALID_MFN, no ref taken. */
-static int get_paged_frame(unsigned long gfn, unsigned long *frame,
+static int get_paged_frame(unsigned long gfn, mfn_t *frame,
struct page_info **page, bool readonly,
struct domain *rd)
 {
 int rc = GNTST_okay;
 p2m_type_t p2mt;
 
-*frame = mfn_x(INVALID_MFN);
+*frame = INVALID_MFN;
 *page = get_page_from_gfn(rd, gfn, ,
   readonly ? P2M_ALLOC : P2M_UNSHARE);
 if ( !*page )
@@ -788,7 +794,7 @@ static int _set_status(unsigned gt_version,
 
 static struct active_grant_entry *grant_map_exists(const struct domain *ld,
struct grant_table *rgt,
-   unsigned long mfn,
+   mfn_t mfn,
grant_ref_t *cur_ref)
 {
 grant_ref_t ref, max_iter;
@@ -807,7 +813,8 @@ static struct active_grant_entry *grant_map_exists(const 
struct domain *ld,
 {
 struct active_grant_entry *act = active_entry_acquire(rgt, ref);
 
-if ( act->pin && act->domid == ld->domain_id && act->frame == mfn )
+if ( act->pin && act->domid == ld->domain_id &&
+ mfn_eq(act->frame, mfn) )
 return act;
 active_entry_release(act);
 }
@@ -824,7 +831,7 @@ static struct active_grant_entry *grant_map_exists(const 
struct domain *ld,
 #define MAPKIND_READ 1
 #define MAPKIND_WRITE 2
 static unsigned int mapkind(
-struct grant_table *lgt, const struct domain *rd, unsigned long mfn)
+struct grant_table *lgt, const struct domain *rd, mfn_t mfn)
 {
 struct grant_mapping *map;
 grant_handle_t handle, limit = lgt->maptrack_limit;
@@ -849,7 +856,7 @@ static unsigned int mapkind(
 if ( !(map->flags & (GNTMAP_device_map|GNTMAP_host_map)) ||
  map->domid != rd->domain_id )
 continue;
-if ( _active_entry(rd->grant_table, map->ref).frame == mfn )
+if ( mfn_eq(_active_entry(rd->grant_table, map->ref).frame, mfn) )
 kind |= map->flags & GNTMAP_readonly ?
 MAPKIND_READ : MAPKIND_WRITE;
 }
@@ -872,7 +879,7 @@ map_grant_ref(
 struct grant_table *lgt, *rgt;
 struct vcpu   *led;
 grant_handle_t handle;
-