| Message ID | 20230629210150.69525-1-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: grant-table: Correct the prototype of the arch helpers | expand |
Sorry I forgot to CC Bertrand and Stefano. On 29/06/2023 22:01, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Both the stub and the x86 prototypes for replace_grant_host_mapping() > and create_grant_host_mapping() will define the first parameter (and > third for the former) as uint64_t. Yet Arm will define it as > 'unsigned long'. > > While there are no differences for 64-bit, for 32-bit it means > that the address should be truncated as 32-bit guest could support > up to 40-bit addresses. > > So replace 'unsigned long' with 'uint64_t' for the first parameter > (and third parameter for replace_grant_host_mapping()). > > Signed-off-by: Julien Grall <jgrall@amazon.com> > --- > > Cc: federico.serafini@bugseng.com > --- > xen/arch/arm/include/asm/grant_table.h | 6 +++--- > xen/arch/arm/mm.c | 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/include/asm/grant_table.h b/xen/arch/arm/include/asm/grant_table.h > index f2d115b97d8b..d3c518a926b9 100644 > --- a/xen/arch/arm/include/asm/grant_table.h > +++ b/xen/arch/arm/include/asm/grant_table.h > @@ -36,10 +36,10 @@ static inline bool gnttab_release_host_mappings(const struct domain *d) > return true; > } > > -int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > +int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > unsigned int flags, unsigned int cache_flags); > -int replace_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > - unsigned long new_gpaddr, unsigned int flags); > +int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > + uint64_t new_gpaddr, unsigned int flags); > > /* > * The region used by Xen on the memory will never be mapped in DOM0 > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 0a3e1f3b64b6..53773368d036 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -1579,7 +1579,7 @@ void put_page_type(struct page_info *page) > return; > } > > -int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > +int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > unsigned int flags, unsigned int cache_flags) > { > int rc; > @@ -1600,8 +1600,8 @@ int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > return GNTST_okay; > } > > -int replace_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > - unsigned long new_gpaddr, unsigned int flags) > +int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > + uint64_t new_gpaddr, unsigned int flags) > { > gfn_t gfn = gaddr_to_gfn(gpaddr); > struct domain *d = current->domain;
On Thu, 29 Jun 2023, Julien Grall wrote: > Sorry I forgot to CC Bertrand and Stefano. > > > On 29/06/2023 22:01, Julien Grall wrote: > > From: Julien Grall <jgrall@amazon.com> > > > > Both the stub and the x86 prototypes for replace_grant_host_mapping() > > and create_grant_host_mapping() will define the first parameter (and > > third for the former) as uint64_t. Yet Arm will define it as > > 'unsigned long'. > > > > While there are no differences for 64-bit, for 32-bit it means > > that the address should be truncated as 32-bit guest could support > > up to 40-bit addresses. > > > > So replace 'unsigned long' with 'uint64_t' for the first parameter > > (and third parameter for replace_grant_host_mapping()). > > > > Signed-off-by: Julien Grall <jgrall@amazon.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > > --- > > > > Cc: federico.serafini@bugseng.com > > --- > > xen/arch/arm/include/asm/grant_table.h | 6 +++--- > > xen/arch/arm/mm.c | 6 +++--- > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/xen/arch/arm/include/asm/grant_table.h > > b/xen/arch/arm/include/asm/grant_table.h > > index f2d115b97d8b..d3c518a926b9 100644 > > --- a/xen/arch/arm/include/asm/grant_table.h > > +++ b/xen/arch/arm/include/asm/grant_table.h > > @@ -36,10 +36,10 @@ static inline bool gnttab_release_host_mappings(const > > struct domain *d) > > return true; > > } > > -int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > > +int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > > unsigned int flags, unsigned int > > cache_flags); > > -int replace_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > > - unsigned long new_gpaddr, unsigned int > > flags); > > +int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > > + uint64_t new_gpaddr, unsigned int flags); > > /* > > * The region used by Xen on the memory will never be mapped in DOM0 > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > > index 0a3e1f3b64b6..53773368d036 100644 > > --- a/xen/arch/arm/mm.c > > +++ b/xen/arch/arm/mm.c > > @@ -1579,7 +1579,7 @@ void put_page_type(struct page_info *page) > > return; > > } > > -int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > > +int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > > unsigned int flags, unsigned int > > cache_flags) > > { > > int rc; > > @@ -1600,8 +1600,8 @@ int create_grant_host_mapping(unsigned long gpaddr, > > mfn_t frame, > > return GNTST_okay; > > } > > -int replace_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > > - unsigned long new_gpaddr, unsigned int > > flags) > > +int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > > + uint64_t new_gpaddr, unsigned int flags) > > { > > gfn_t gfn = gaddr_to_gfn(gpaddr); > > struct domain *d = current->domain; > > -- > Julien Grall >
Hi Julien, > -----Original Message----- > Subject: [PATCH] xen/arm: grant-table: Correct the prototype of the arch > helpers > > From: Julien Grall <jgrall@amazon.com> > > Both the stub and the x86 prototypes for replace_grant_host_mapping() > and create_grant_host_mapping() will define the first parameter (and > third for the former) as uint64_t. Yet Arm will define it as > 'unsigned long'. > > While there are no differences for 64-bit, for 32-bit it means > that the address should be truncated as 32-bit guest could support > up to 40-bit addresses. > > So replace 'unsigned long' with 'uint64_t' for the first parameter > (and third parameter for replace_grant_host_mapping()). > > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Henry Wang <Henry.Wang@arm.com> Tested-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
Hi Julien, > On 29 Jun 2023, at 23:01, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@amazon.com> > > Both the stub and the x86 prototypes for replace_grant_host_mapping() > and create_grant_host_mapping() will define the first parameter (and > third for the former) as uint64_t. Yet Arm will define it as > 'unsigned long'. > > While there are no differences for 64-bit, for 32-bit it means > that the address should be truncated as 32-bit guest could support > up to 40-bit addresses. > > So replace 'unsigned long' with 'uint64_t' for the first parameter > (and third parameter for replace_grant_host_mapping()). > > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > --- > > Cc: federico.serafini@bugseng.com > --- > xen/arch/arm/include/asm/grant_table.h | 6 +++--- > xen/arch/arm/mm.c | 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/include/asm/grant_table.h b/xen/arch/arm/include/asm/grant_table.h > index f2d115b97d8b..d3c518a926b9 100644 > --- a/xen/arch/arm/include/asm/grant_table.h > +++ b/xen/arch/arm/include/asm/grant_table.h > @@ -36,10 +36,10 @@ static inline bool gnttab_release_host_mappings(const struct domain *d) > return true; > } > > -int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > +int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > unsigned int flags, unsigned int cache_flags); > -int replace_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > - unsigned long new_gpaddr, unsigned int flags); > +int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > + uint64_t new_gpaddr, unsigned int flags); > > /* > * The region used by Xen on the memory will never be mapped in DOM0 > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 0a3e1f3b64b6..53773368d036 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -1579,7 +1579,7 @@ void put_page_type(struct page_info *page) > return; > } > > -int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > +int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > unsigned int flags, unsigned int cache_flags) > { > int rc; > @@ -1600,8 +1600,8 @@ int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > return GNTST_okay; > } > > -int replace_grant_host_mapping(unsigned long gpaddr, mfn_t frame, > - unsigned long new_gpaddr, unsigned int flags) > +int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame, > + uint64_t new_gpaddr, unsigned int flags) > { > gfn_t gfn = gaddr_to_gfn(gpaddr); > struct domain *d = current->domain; > -- > 2.40.1 > >
Hi Bertrand, On 04/07/2023 15:35, Bertrand Marquis wrote: >> On 29 Jun 2023, at 23:01, Julien Grall <julien@xen.org> wrote: >> >> From: Julien Grall <jgrall@amazon.com> >> >> Both the stub and the x86 prototypes for replace_grant_host_mapping() >> and create_grant_host_mapping() will define the first parameter (and >> third for the former) as uint64_t. Yet Arm will define it as >> 'unsigned long'. >> >> While there are no differences for 64-bit, for 32-bit it means >> that the address should be truncated as 32-bit guest could support >> up to 40-bit addresses. >> >> So replace 'unsigned long' with 'uint64_t' for the first parameter >> (and third parameter for replace_grant_host_mapping()). >> >> Signed-off-by: Julien Grall <jgrall@amazon.com> > > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Thanks. It is now committed. Cheers,
diff --git a/xen/arch/arm/include/asm/grant_table.h b/xen/arch/arm/include/asm/grant_table.h index f2d115b97d8b..d3c518a926b9 100644 --- a/xen/arch/arm/include/asm/grant_table.h +++ b/xen/arch/arm/include/asm/grant_table.h @@ -36,10 +36,10 @@ static inline bool gnttab_release_host_mappings(const struct domain *d) return true; } -int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, +int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame, unsigned int flags, unsigned int cache_flags); -int replace_grant_host_mapping(unsigned long gpaddr, mfn_t frame, - unsigned long new_gpaddr, unsigned int flags); +int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame, + uint64_t new_gpaddr, unsigned int flags); /* * The region used by Xen on the memory will never be mapped in DOM0 diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 0a3e1f3b64b6..53773368d036 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1579,7 +1579,7 @@ void put_page_type(struct page_info *page) return; } -int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, +int create_grant_host_mapping(uint64_t gpaddr, mfn_t frame, unsigned int flags, unsigned int cache_flags) { int rc; @@ -1600,8 +1600,8 @@ int create_grant_host_mapping(unsigned long gpaddr, mfn_t frame, return GNTST_okay; } -int replace_grant_host_mapping(unsigned long gpaddr, mfn_t frame, - unsigned long new_gpaddr, unsigned int flags) +int replace_grant_host_mapping(uint64_t gpaddr, mfn_t frame, + uint64_t new_gpaddr, unsigned int flags) { gfn_t gfn = gaddr_to_gfn(gpaddr); struct domain *d = current->domain;