[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [XEN v2 05/11] xen/arm: Use paddr_t instead of u64 for address/size
Hi Ayan,
On 07/02/2023 10:59, Ayan Kumar Halder wrote:
On 07/02/2023 09:03, Julien Grall wrote:
On 06/02/2023 19:21, Ayan Kumar Halder wrote:
Hi Stefano,
On 19/01/2023 23:24, Stefano Stabellini wrote:
On Tue, 17 Jan 2023, Ayan Kumar Halder wrote:
One should now be able to use 'paddr_t' to represent address and size.
Consequently, one should use 'PRIpaddr' as a format specifier for
paddr_t.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
---
Changes from -
v1 - 1. Rebased the patch.
xen/arch/arm/domain_build.c | 9 +++++----
xen/arch/arm/gic-v3.c | 2 +-
xen/arch/arm/platforms/exynos5.c | 26 +++++++++++++-------------
xen/drivers/char/exynos4210-uart.c | 2 +-
xen/drivers/char/ns16550.c | 8 ++++----
xen/drivers/char/omap-uart.c | 2 +-
xen/drivers/char/pl011.c | 4 ++--
xen/drivers/char/scif-uart.c | 2 +-
xen/drivers/passthrough/arm/smmu.c | 6 +++---
9 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 72b9afbb4c..cf8ae37a14 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1666,7 +1666,7 @@ static int __init find_memory_holes(const
struct kernel_info *kinfo,
dt_for_each_device_node( dt_host, np )
{
unsigned int naddr;
- u64 addr, size;
+ paddr_t addr, size;
naddr = dt_number_of_address(np);
@@ -2445,7 +2445,7 @@ static int __init handle_device(struct domain
*d, struct dt_device_node *dev,
unsigned int naddr;
unsigned int i;
int res;
- u64 addr, size;
+ paddr_t addr, size;
bool own_device = !dt_device_for_passthrough(dev);
/*
* We want to avoid mapping the MMIO in dom0 for the
following cases:
@@ -2941,9 +2941,10 @@ static int __init
handle_passthrough_prop(struct kernel_info *kinfo,
if ( res )
{
printk(XENLOG_ERR "Unable to permit to dom%d access to"
- " 0x%"PRIx64" - 0x%"PRIx64"\n",
+ " 0x%"PRIpaddr" - 0x%"PRIpaddr"\n",
kinfo->d->domain_id,
- mstart & PAGE_MASK, PAGE_ALIGN(mstart + size) -
1);
+ (paddr_t) (mstart & PAGE_MASK),
+ (paddr_t) (PAGE_ALIGN(mstart + size) - 1));
Why do you need the casts here? mstart is already defined as paddr_t
Actually, this is because the PAGE_MASK is defined as 'long'. See
xen/include/xen/page-size.h :-
#define PAGE_SIZE (_AC(1,L) << PAGE_SHIFT)
#define PAGE_MASK (~(PAGE_SIZE-1))
So, the resultant type inferred is 'long unsigned int'. Thus, we need
to add an explicit cast.
Hmmm... I am a bit confused with this statement. If the resultant type
inferred is actually 'long unsigned int', then why was the current
specifier worked before ('long unsigned int' is 32-bit on Arm32)?
Before this change, mstart was of type paddr_t (ie u64 ie unsigned long
long on Arm_32). When 'unsigned long long' was operated with 'long' (ie
PAGE_SIZE), then the resultant type is 'unsigned long long'. The rule
from 'C Standard 2011'
(https://web.cs.dal.ca/~vlado/pl/C_Standard_2011-n1570.pdf) , page 53
(Section 6.3.1.8 Usual arithmetic conversions) is :-
"Otherwise, if the operand that has unsigned integer type has rank
greater or equal to the rank of the type of the other operand, then the
operand with signed integer type is converted to the type of the operand
with unsigned integer type."
And from 6.3.1.1
"The rank of long long int shall be greater than the rank of long int,
which shall be greater than the rank of int, which shall be greater than
the rank of short int, which shall be greater than the rank of signed char.
The rank of any unsigned integer type shall equal the rank of the
corresponding signed integer type, if any."
And using 'PRIx64' to print 'unsigned long long' will not require any cast.
Now with the change,
mstart is of paddr_t (ie 'unsigned int'). 'unsigned int' operated with
'long', then the result is 'unsigned long int'. As the rank of
'unsigned long int' > 'int', thus it cannot be printed using PRIx32
(integer format specifier) without an explicit cast.
Please let me know if this makes sense.
Thanks for the explanation. I would expect that there will be several
places in Xen where we would need such cast.
So it sounds like we want to define paddr_t as ``unsigned long`` and
update PRIpaddr accordingly.
Cheers,
--
Julien Grall
|
