| Message ID | 20190218113600.9540-5-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Properly disable M2P on Arm. | expand |
>>> On 18.02.19 at 12:35, <julien.grall@arm.com> wrote: > --- a/xen/include/asm-arm/grant_table.h > +++ b/xen/include/asm-arm/grant_table.h > @@ -67,15 +67,15 @@ void gnttab_mark_dirty(struct domain *d, mfn_t mfn); > } while ( 0 ) > > #define gnttab_get_frame_gfn(gt, st, idx) ({ \ > - _gfn((st) ? gnttab_status_gmfn(NULL, gt, idx) \ > - : gnttab_shared_gmfn(NULL, gt, idx)); \ > + (st) ? gnttab_status_gfn(NULL, gt, idx) \ > + : gnttab_shared_gfn(NULL, gt, idx); \ Please also adjust indentation of the latter line. > --- a/xen/include/asm-x86/grant_table.h > +++ b/xen/include/asm-x86/grant_table.h > @@ -43,24 +43,20 @@ static inline int replace_grant_host_mapping(uint64_t > addr, mfn_t frame, > #define gnttab_destroy_arch(gt) do {} while ( 0 ) > #define gnttab_set_frame_gfn(gt, st, idx, gfn) do {} while ( 0 ) > #define gnttab_get_frame_gfn(gt, st, idx) ({ \ > - unsigned long mfn_ = (st) ? gnttab_status_mfn(gt, idx) \ > - : gnttab_shared_mfn(gt, idx); \ > - unsigned long gpfn_ = get_gpfn_from_mfn(mfn_); \ > + mfn_t mfn_ = (st) ? gnttab_status_mfn(gt, idx) \ > + : gnttab_shared_mfn(gt, idx); \ > + unsigned long gpfn_ = get_gpfn_from_mfn(mfn_x(mfn_)); \ > VALID_M2P(gpfn_) ? _gfn(gpfn_) : INVALID_GFN; \ > }) > > -#define gnttab_shared_mfn(t, i) \ > - ((virt_to_maddr((t)->shared_raw[i]) >> PAGE_SHIFT)) > +#define gnttab_shared_mfn(t, i) _mfn(__virt_to_mfn((t)->shared_raw[i])) > > -#define gnttab_shared_gmfn(d, t, i) \ > - (mfn_to_gmfn(d, gnttab_shared_mfn(t, i))) > +#define gnttab_shared_gfn(d, t, i) _gfn(mfn_to_gfn(d, gnttab_shared_mfn(t, i))) This and ... > +#define gnttab_status_mfn(t, i) _mfn(__virt_to_mfn((t)->status[i])) > > -#define gnttab_status_mfn(t, i) \ > - ((virt_to_maddr((t)->status[i]) >> PAGE_SHIFT)) > - > -#define gnttab_status_gmfn(d, t, i) \ > - (mfn_to_gmfn(d, gnttab_status_mfn(t, i))) > +#define gnttab_status_gfn(d, t, i) \ > + _gfn(mfn_to_gfn(d, gnttab_status_mfn(t, i))) ... this would already benefit from mfn_to_gfn() returning gfn_t. Either way non-Arm parts Acked-by: Jan Beulich <jbeulich@suse.com> Jan
On Mon, 18 Feb 2019, Julien Grall wrote: > No functional changes intended. Please list here the changes this patch is making: - renaming gnttab_shared_gmfn to gnttab_shared_gfn to follow the naming convention (and others.) - making gnttab_shared_gmfn returning gfn_t With the better commit message, add my acked-by. > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > xen/common/grant_table.c | 4 ++-- > xen/include/asm-arm/grant_table.h | 12 ++++++------ > xen/include/asm-x86/grant_table.h | 20 ++++++++------------ > 3 files changed, 16 insertions(+), 20 deletions(-) > > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > index fd099a8f25..e7a65b38e0 100644 > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -1935,7 +1935,7 @@ gnttab_setup_table( > op.status = GNTST_okay; > for ( i = 0; i < op.nr_frames; i++ ) > { > - xen_pfn_t gmfn = gnttab_shared_gmfn(d, gt, i); > + xen_pfn_t gmfn = gfn_x(gnttab_shared_gfn(d, gt, i)); > > /* Grant tables cannot be shared */ > BUG_ON(SHARED_M2P(gmfn)); > @@ -3147,7 +3147,7 @@ gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop, > > for ( i = 0; i < op.nr_frames; i++ ) > { > - gmfn = gnttab_status_gmfn(d, gt, i); > + gmfn = gfn_x(gnttab_status_gfn(d, gt, i)); > if ( copy_to_guest_offset(op.frame_list, i, &gmfn, 1) ) > op.status = GNTST_bad_virt_addr; > } > diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h > index 816e3c6d68..4c44b720f2 100644 > --- a/xen/include/asm-arm/grant_table.h > +++ b/xen/include/asm-arm/grant_table.h > @@ -67,15 +67,15 @@ void gnttab_mark_dirty(struct domain *d, mfn_t mfn); > } while ( 0 ) > > #define gnttab_get_frame_gfn(gt, st, idx) ({ \ > - _gfn((st) ? gnttab_status_gmfn(NULL, gt, idx) \ > - : gnttab_shared_gmfn(NULL, gt, idx)); \ > + (st) ? gnttab_status_gfn(NULL, gt, idx) \ > + : gnttab_shared_gfn(NULL, gt, idx); \ > }) > > -#define gnttab_shared_gmfn(d, t, i) \ > - gfn_x(((i) >= nr_grant_frames(t)) ? INVALID_GFN : (t)->arch.shared_gfn[i]) > +#define gnttab_shared_gfn(d, t, i) \ > + (((i) >= nr_grant_frames(t)) ? INVALID_GFN : (t)->arch.shared_gfn[i]) > > -#define gnttab_status_gmfn(d, t, i) \ > - gfn_x(((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i]) > +#define gnttab_status_gfn(d, t, i) \ > + (((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i]) > > #define gnttab_need_iommu_mapping(d) \ > (is_domain_direct_mapped(d) && need_iommu_pt_sync(d)) > diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h > index 4b8c4f9160..8736d7286d 100644 > --- a/xen/include/asm-x86/grant_table.h > +++ b/xen/include/asm-x86/grant_table.h > @@ -43,24 +43,20 @@ static inline int replace_grant_host_mapping(uint64_t addr, mfn_t frame, > #define gnttab_destroy_arch(gt) do {} while ( 0 ) > #define gnttab_set_frame_gfn(gt, st, idx, gfn) do {} while ( 0 ) > #define gnttab_get_frame_gfn(gt, st, idx) ({ \ > - unsigned long mfn_ = (st) ? gnttab_status_mfn(gt, idx) \ > - : gnttab_shared_mfn(gt, idx); \ > - unsigned long gpfn_ = get_gpfn_from_mfn(mfn_); \ > + mfn_t mfn_ = (st) ? gnttab_status_mfn(gt, idx) \ > + : gnttab_shared_mfn(gt, idx); \ > + unsigned long gpfn_ = get_gpfn_from_mfn(mfn_x(mfn_)); \ > VALID_M2P(gpfn_) ? _gfn(gpfn_) : INVALID_GFN; \ > }) > > -#define gnttab_shared_mfn(t, i) \ > - ((virt_to_maddr((t)->shared_raw[i]) >> PAGE_SHIFT)) > +#define gnttab_shared_mfn(t, i) _mfn(__virt_to_mfn((t)->shared_raw[i])) > > -#define gnttab_shared_gmfn(d, t, i) \ > - (mfn_to_gmfn(d, gnttab_shared_mfn(t, i))) > +#define gnttab_shared_gfn(d, t, i) _gfn(mfn_to_gfn(d, gnttab_shared_mfn(t, i))) > > +#define gnttab_status_mfn(t, i) _mfn(__virt_to_mfn((t)->status[i])) > > -#define gnttab_status_mfn(t, i) \ > - ((virt_to_maddr((t)->status[i]) >> PAGE_SHIFT)) > - > -#define gnttab_status_gmfn(d, t, i) \ > - (mfn_to_gmfn(d, gnttab_status_mfn(t, i))) > +#define gnttab_status_gfn(d, t, i) \ > + _gfn(mfn_to_gfn(d, gnttab_status_mfn(t, i))) > > #define gnttab_mark_dirty(d, f) paging_mark_dirty((d), f)
Hi, On 4/15/19 11:03 PM, Stefano Stabellini wrote: > On Mon, 18 Feb 2019, Julien Grall wrote: >> No functional changes intended. > > Please list here the changes this patch is making: > > - renaming gnttab_shared_gmfn to gnttab_shared_gfn to follow the naming > convention (and others.) Ok for this but... > - making gnttab_shared_gmfn returning gfn_t ... it does not make sense to mention one macro and not all the other that was modified because of the change. This is also already pretty clear from the commit title that all macros will be typesafe. So I would rather not mention this. Cheers,
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index fd099a8f25..e7a65b38e0 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -1935,7 +1935,7 @@ gnttab_setup_table( op.status = GNTST_okay; for ( i = 0; i < op.nr_frames; i++ ) { - xen_pfn_t gmfn = gnttab_shared_gmfn(d, gt, i); + xen_pfn_t gmfn = gfn_x(gnttab_shared_gfn(d, gt, i)); /* Grant tables cannot be shared */ BUG_ON(SHARED_M2P(gmfn)); @@ -3147,7 +3147,7 @@ gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop, for ( i = 0; i < op.nr_frames; i++ ) { - gmfn = gnttab_status_gmfn(d, gt, i); + gmfn = gfn_x(gnttab_status_gfn(d, gt, i)); if ( copy_to_guest_offset(op.frame_list, i, &gmfn, 1) ) op.status = GNTST_bad_virt_addr; } diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h index 816e3c6d68..4c44b720f2 100644 --- a/xen/include/asm-arm/grant_table.h +++ b/xen/include/asm-arm/grant_table.h @@ -67,15 +67,15 @@ void gnttab_mark_dirty(struct domain *d, mfn_t mfn); } while ( 0 ) #define gnttab_get_frame_gfn(gt, st, idx) ({ \ - _gfn((st) ? gnttab_status_gmfn(NULL, gt, idx) \ - : gnttab_shared_gmfn(NULL, gt, idx)); \ + (st) ? gnttab_status_gfn(NULL, gt, idx) \ + : gnttab_shared_gfn(NULL, gt, idx); \ }) -#define gnttab_shared_gmfn(d, t, i) \ - gfn_x(((i) >= nr_grant_frames(t)) ? INVALID_GFN : (t)->arch.shared_gfn[i]) +#define gnttab_shared_gfn(d, t, i) \ + (((i) >= nr_grant_frames(t)) ? INVALID_GFN : (t)->arch.shared_gfn[i]) -#define gnttab_status_gmfn(d, t, i) \ - gfn_x(((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i]) +#define gnttab_status_gfn(d, t, i) \ + (((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i]) #define gnttab_need_iommu_mapping(d) \ (is_domain_direct_mapped(d) && need_iommu_pt_sync(d)) diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h index 4b8c4f9160..8736d7286d 100644 --- a/xen/include/asm-x86/grant_table.h +++ b/xen/include/asm-x86/grant_table.h @@ -43,24 +43,20 @@ static inline int replace_grant_host_mapping(uint64_t addr, mfn_t frame, #define gnttab_destroy_arch(gt) do {} while ( 0 ) #define gnttab_set_frame_gfn(gt, st, idx, gfn) do {} while ( 0 ) #define gnttab_get_frame_gfn(gt, st, idx) ({ \ - unsigned long mfn_ = (st) ? gnttab_status_mfn(gt, idx) \ - : gnttab_shared_mfn(gt, idx); \ - unsigned long gpfn_ = get_gpfn_from_mfn(mfn_); \ + mfn_t mfn_ = (st) ? gnttab_status_mfn(gt, idx) \ + : gnttab_shared_mfn(gt, idx); \ + unsigned long gpfn_ = get_gpfn_from_mfn(mfn_x(mfn_)); \ VALID_M2P(gpfn_) ? _gfn(gpfn_) : INVALID_GFN; \ }) -#define gnttab_shared_mfn(t, i) \ - ((virt_to_maddr((t)->shared_raw[i]) >> PAGE_SHIFT)) +#define gnttab_shared_mfn(t, i) _mfn(__virt_to_mfn((t)->shared_raw[i])) -#define gnttab_shared_gmfn(d, t, i) \ - (mfn_to_gmfn(d, gnttab_shared_mfn(t, i))) +#define gnttab_shared_gfn(d, t, i) _gfn(mfn_to_gfn(d, gnttab_shared_mfn(t, i))) +#define gnttab_status_mfn(t, i) _mfn(__virt_to_mfn((t)->status[i])) -#define gnttab_status_mfn(t, i) \ - ((virt_to_maddr((t)->status[i]) >> PAGE_SHIFT)) - -#define gnttab_status_gmfn(d, t, i) \ - (mfn_to_gmfn(d, gnttab_status_mfn(t, i))) +#define gnttab_status_gfn(d, t, i) \ + _gfn(mfn_to_gfn(d, gnttab_status_mfn(t, i))) #define gnttab_mark_dirty(d, f) paging_mark_dirty((d), f)
No functional changes intended. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/common/grant_table.c | 4 ++-- xen/include/asm-arm/grant_table.h | 12 ++++++------ xen/include/asm-x86/grant_table.h | 20 ++++++++------------ 3 files changed, 16 insertions(+), 20 deletions(-)