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@amd.com>
---
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));
return res;
}
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index bb59ea94cd..391dfa53d7 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1393,7 +1393,7 @@ static void __init gicv3_dt_init(void)
for ( i = 0; i < gicv3.rdist_count; i++ )
{
- uint64_t rdist_base, rdist_size;
+ paddr_t rdist_base, rdist_size;
res = dt_device_get_address(node, 1 + i, &rdist_base, &rdist_size);
if ( res )
diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
index 6560507092..f79fad9957 100644
--- a/xen/arch/arm/platforms/exynos5.c
+++ b/xen/arch/arm/platforms/exynos5.c
@@ -42,8 +42,8 @@ static int exynos5_init_time(void)
void __iomem *mct;
int rc;
struct dt_device_node *node;
- u64 mct_base_addr;
- u64 size;
+ paddr_t mct_base_addr;
+ paddr_t size;
node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-mct");
if ( !node )
@@ -59,7 +59,7 @@ static int exynos5_init_time(void)
return -ENXIO;
}
- dprintk(XENLOG_INFO, "mct_base_addr: %016llx size: %016llx\n",
+ dprintk(XENLOG_INFO, "mct_base_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"\n",
mct_base_addr, size);
mct = ioremap_nocache(mct_base_addr, size);
@@ -97,9 +97,9 @@ static int __init exynos5_smp_init(void)
struct dt_device_node *node;
void __iomem *sysram;
char *compatible;
- u64 sysram_addr;
- u64 size;
- u64 sysram_offset;
+ paddr_t sysram_addr;
+ paddr_t size;
+ paddr_t sysram_offset;
int rc;
node = dt_find_compatible_node(NULL, NULL, "samsung,secure-firmware");
@@ -131,7 +131,7 @@ static int __init exynos5_smp_init(void)
dprintk(XENLOG_ERR, "Error in %s\n", compatible);
return -ENXIO;
}
- dprintk(XENLOG_INFO, "sysram_addr: %016llx size: %016llx offset: %016llx\n",
+ dprintk(XENLOG_INFO,"sysram_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"offset: 0x%"PRIpaddr"\n",
sysram_addr, size, sysram_offset);
sysram = ioremap_nocache(sysram_addr, size);
@@ -189,7 +189,7 @@ static int exynos5_cpu_power_up(void __iomem *power, int cpu)
return 0;
}
-static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
+static int exynos5_get_pmu_baseandsize(paddr_t *power_base_addr, paddr_t *size)
{
struct dt_device_node *node;
int rc;
@@ -215,7 +215,7 @@ static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
return -ENXIO;
}
- dprintk(XENLOG_DEBUG, "power_base_addr: %016llx size: %016llx\n",
+ dprintk(XENLOG_DEBUG, "power_base_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"\n",
*power_base_addr, *size);
return 0;
@@ -223,8 +223,8 @@ static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
static int exynos5_cpu_up(int cpu)
{
- u64 power_base_addr;
- u64 size;
+ paddr_t power_base_addr;
+ paddr_t size;
void __iomem *power;
int rc;
@@ -256,8 +256,8 @@ static int exynos5_cpu_up(int cpu)
static void exynos5_reset(void)
{
- u64 power_base_addr;
- u64 size;
+ paddr_t power_base_addr;
+ paddr_t size;
void __iomem *pmu;
int rc;
diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index 43aaf02e18..32cc8c78b5 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -303,7 +303,7 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
const char *config = data;
struct exynos4210_uart *uart;
int res;
- u64 addr, size;
+ paddr_t addr, size;
if ( strcmp(config, "") )
printk("WARNING: UART configuration is not supported\n");
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 58d0ccd889..8ef895a2bb 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -42,8 +42,8 @@
static struct ns16550 {
int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
- u64 io_base; /* I/O port or memory-mapped I/O address. */
- u64 io_size;
+ paddr_t io_base; /* I/O port or memory-mapped I/O address. */
+ paddr_t io_size;
int reg_shift; /* Bits to shift register offset by */
int reg_width; /* Size of access to use, the registers
* themselves are still bytes */
@@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst uart_config[] =
static int __init
pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
{
- u64 orig_base = uart->io_base;
+ paddr_t orig_base = uart->io_base;
unsigned int b, d, f, nextf, i;
/* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
@@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
else
size = len & PCI_BASE_ADDRESS_MEM_MASK;
- uart->io_base = ((u64)bar_64 << 32) |
+ uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
(bar & PCI_BASE_ADDRESS_MEM_MASK);
}
/* IO based */
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index d6a5d59aa2..3b53e1909a 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -324,7 +324,7 @@ static int __init omap_uart_init(struct dt_device_node *dev,
struct omap_uart *uart;
u32 clkspec;
int res;
- u64 addr, size;
+ paddr_t addr, size;
if ( strcmp(config, "") )
printk("WARNING: UART configuration is not supported\n");
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index be67242bc0..256ec11e3f 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -222,7 +222,7 @@ static struct uart_driver __read_mostly pl011_driver = {
.vuart_info = pl011_vuart,
};
-static int __init pl011_uart_init(int irq, u64 addr, u64 size, bool sbsa)
+static int __init pl011_uart_init(int irq, paddr_t addr, paddr_t size, bool sbsa)
{
struct pl011 *uart;
@@ -258,7 +258,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
{
const char *config = data;
int res;
- u64 addr, size;
+ paddr_t addr, size;
if ( strcmp(config, "") )
{
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index 2fccafe340..b425881d06 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -311,7 +311,7 @@ static int __init scif_uart_init(struct dt_device_node *dev,
const char *config = data;
struct scif_uart *uart;
int res;
- u64 addr, size;
+ paddr_t addr, size;
if ( strcmp(config, "") )
printk("WARNING: UART configuration is not supported\n");
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 0a514821b3..490d253d44 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -73,8 +73,8 @@
/* Xen: Helpers to get device MMIO and IRQs */
struct resource
{
- u64 addr;
- u64 size;
+ paddr_t addr;
+ paddr_t size;
unsigned int type;
};
@@ -169,7 +169,7 @@ static void __iomem *devm_ioremap_resource(struct device *dev,
ptr = ioremap_nocache(res->addr, res->size);
if (!ptr) {
dev_err(dev,
- "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n",
+ "ioremap failed (addr 0x%"PRIpaddr" size 0x%"PRIpaddr")\n",
res->addr, res->size);
return ERR_PTR(-ENOMEM);
}
--
2.17.1Hi,
On 17/01/2023 17:43, 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@amd.com>
> ---
>
> 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;
Without the next patch, this change is incorrect because
dt_device_get_address() expects a 64-bit value rather than paddr_t.
So the type change wants to be moved in the next patch. The same goes
for any variable you modifed in this patch used by dt_device_get_address().
Cheers,
--
Julien Grall
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@amd.com>
> ---
>
> 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
> return res;
> }
>
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index bb59ea94cd..391dfa53d7 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1393,7 +1393,7 @@ static void __init gicv3_dt_init(void)
>
> for ( i = 0; i < gicv3.rdist_count; i++ )
> {
> - uint64_t rdist_base, rdist_size;
> + paddr_t rdist_base, rdist_size;
>
> res = dt_device_get_address(node, 1 + i, &rdist_base, &rdist_size);
> if ( res )
> diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
> index 6560507092..f79fad9957 100644
> --- a/xen/arch/arm/platforms/exynos5.c
> +++ b/xen/arch/arm/platforms/exynos5.c
> @@ -42,8 +42,8 @@ static int exynos5_init_time(void)
> void __iomem *mct;
> int rc;
> struct dt_device_node *node;
> - u64 mct_base_addr;
> - u64 size;
> + paddr_t mct_base_addr;
> + paddr_t size;
>
> node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-mct");
> if ( !node )
> @@ -59,7 +59,7 @@ static int exynos5_init_time(void)
> return -ENXIO;
> }
>
> - dprintk(XENLOG_INFO, "mct_base_addr: %016llx size: %016llx\n",
> + dprintk(XENLOG_INFO, "mct_base_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"\n",
> mct_base_addr, size);
>
> mct = ioremap_nocache(mct_base_addr, size);
> @@ -97,9 +97,9 @@ static int __init exynos5_smp_init(void)
> struct dt_device_node *node;
> void __iomem *sysram;
> char *compatible;
> - u64 sysram_addr;
> - u64 size;
> - u64 sysram_offset;
> + paddr_t sysram_addr;
> + paddr_t size;
> + paddr_t sysram_offset;
> int rc;
>
> node = dt_find_compatible_node(NULL, NULL, "samsung,secure-firmware");
> @@ -131,7 +131,7 @@ static int __init exynos5_smp_init(void)
> dprintk(XENLOG_ERR, "Error in %s\n", compatible);
> return -ENXIO;
> }
> - dprintk(XENLOG_INFO, "sysram_addr: %016llx size: %016llx offset: %016llx\n",
> + dprintk(XENLOG_INFO,"sysram_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"offset: 0x%"PRIpaddr"\n",
> sysram_addr, size, sysram_offset);
>
> sysram = ioremap_nocache(sysram_addr, size);
> @@ -189,7 +189,7 @@ static int exynos5_cpu_power_up(void __iomem *power, int cpu)
> return 0;
> }
>
> -static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
> +static int exynos5_get_pmu_baseandsize(paddr_t *power_base_addr, paddr_t *size)
> {
> struct dt_device_node *node;
> int rc;
> @@ -215,7 +215,7 @@ static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
> return -ENXIO;
> }
>
> - dprintk(XENLOG_DEBUG, "power_base_addr: %016llx size: %016llx\n",
> + dprintk(XENLOG_DEBUG, "power_base_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"\n",
> *power_base_addr, *size);
>
> return 0;
> @@ -223,8 +223,8 @@ static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
>
> static int exynos5_cpu_up(int cpu)
> {
> - u64 power_base_addr;
> - u64 size;
> + paddr_t power_base_addr;
> + paddr_t size;
> void __iomem *power;
> int rc;
>
> @@ -256,8 +256,8 @@ static int exynos5_cpu_up(int cpu)
>
> static void exynos5_reset(void)
> {
> - u64 power_base_addr;
> - u64 size;
> + paddr_t power_base_addr;
> + paddr_t size;
> void __iomem *pmu;
> int rc;
>
> diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
> index 43aaf02e18..32cc8c78b5 100644
> --- a/xen/drivers/char/exynos4210-uart.c
> +++ b/xen/drivers/char/exynos4210-uart.c
> @@ -303,7 +303,7 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
> const char *config = data;
> struct exynos4210_uart *uart;
> int res;
> - u64 addr, size;
> + paddr_t addr, size;
>
> if ( strcmp(config, "") )
> printk("WARNING: UART configuration is not supported\n");
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 58d0ccd889..8ef895a2bb 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -42,8 +42,8 @@
>
> static struct ns16550 {
> int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
> - u64 io_base; /* I/O port or memory-mapped I/O address. */
> - u64 io_size;
> + paddr_t io_base; /* I/O port or memory-mapped I/O address. */
> + paddr_t io_size;
> int reg_shift; /* Bits to shift register offset by */
> int reg_width; /* Size of access to use, the registers
> * themselves are still bytes */
> @@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst uart_config[] =
> static int __init
> pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
> {
> - u64 orig_base = uart->io_base;
> + paddr_t orig_base = uart->io_base;
> unsigned int b, d, f, nextf, i;
>
> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
> @@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
> else
> size = len & PCI_BASE_ADDRESS_MEM_MASK;
>
> - uart->io_base = ((u64)bar_64 << 32) |
> + uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
> (bar & PCI_BASE_ADDRESS_MEM_MASK);
> }
> /* IO based */
> diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
> index d6a5d59aa2..3b53e1909a 100644
> --- a/xen/drivers/char/omap-uart.c
> +++ b/xen/drivers/char/omap-uart.c
> @@ -324,7 +324,7 @@ static int __init omap_uart_init(struct dt_device_node *dev,
> struct omap_uart *uart;
> u32 clkspec;
> int res;
> - u64 addr, size;
> + paddr_t addr, size;
>
> if ( strcmp(config, "") )
> printk("WARNING: UART configuration is not supported\n");
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index be67242bc0..256ec11e3f 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -222,7 +222,7 @@ static struct uart_driver __read_mostly pl011_driver = {
> .vuart_info = pl011_vuart,
> };
>
> -static int __init pl011_uart_init(int irq, u64 addr, u64 size, bool sbsa)
> +static int __init pl011_uart_init(int irq, paddr_t addr, paddr_t size, bool sbsa)
> {
> struct pl011 *uart;
>
> @@ -258,7 +258,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
> {
> const char *config = data;
> int res;
> - u64 addr, size;
> + paddr_t addr, size;
>
> if ( strcmp(config, "") )
> {
> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
> index 2fccafe340..b425881d06 100644
> --- a/xen/drivers/char/scif-uart.c
> +++ b/xen/drivers/char/scif-uart.c
> @@ -311,7 +311,7 @@ static int __init scif_uart_init(struct dt_device_node *dev,
> const char *config = data;
> struct scif_uart *uart;
> int res;
> - u64 addr, size;
> + paddr_t addr, size;
>
> if ( strcmp(config, "") )
> printk("WARNING: UART configuration is not supported\n");
> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index 0a514821b3..490d253d44 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -73,8 +73,8 @@
> /* Xen: Helpers to get device MMIO and IRQs */
> struct resource
> {
> - u64 addr;
> - u64 size;
> + paddr_t addr;
> + paddr_t size;
> unsigned int type;
> };
>
> @@ -169,7 +169,7 @@ static void __iomem *devm_ioremap_resource(struct device *dev,
> ptr = ioremap_nocache(res->addr, res->size);
> if (!ptr) {
> dev_err(dev,
> - "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n",
> + "ioremap failed (addr 0x%"PRIpaddr" size 0x%"PRIpaddr")\n",
> res->addr, res->size);
> return ERR_PTR(-ENOMEM);
> }
> --
> 2.17.1
>
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@amd.com>
>> ---
>>
>> 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.
- Ayan
>
>
>> return res;
>> }
>>
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index bb59ea94cd..391dfa53d7 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -1393,7 +1393,7 @@ static void __init gicv3_dt_init(void)
>>
>> for ( i = 0; i < gicv3.rdist_count; i++ )
>> {
>> - uint64_t rdist_base, rdist_size;
>> + paddr_t rdist_base, rdist_size;
>>
>> res = dt_device_get_address(node, 1 + i, &rdist_base, &rdist_size);
>> if ( res )
>> diff --git a/xen/arch/arm/platforms/exynos5.c b/xen/arch/arm/platforms/exynos5.c
>> index 6560507092..f79fad9957 100644
>> --- a/xen/arch/arm/platforms/exynos5.c
>> +++ b/xen/arch/arm/platforms/exynos5.c
>> @@ -42,8 +42,8 @@ static int exynos5_init_time(void)
>> void __iomem *mct;
>> int rc;
>> struct dt_device_node *node;
>> - u64 mct_base_addr;
>> - u64 size;
>> + paddr_t mct_base_addr;
>> + paddr_t size;
>>
>> node = dt_find_compatible_node(NULL, NULL, "samsung,exynos4210-mct");
>> if ( !node )
>> @@ -59,7 +59,7 @@ static int exynos5_init_time(void)
>> return -ENXIO;
>> }
>>
>> - dprintk(XENLOG_INFO, "mct_base_addr: %016llx size: %016llx\n",
>> + dprintk(XENLOG_INFO, "mct_base_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"\n",
>> mct_base_addr, size);
>>
>> mct = ioremap_nocache(mct_base_addr, size);
>> @@ -97,9 +97,9 @@ static int __init exynos5_smp_init(void)
>> struct dt_device_node *node;
>> void __iomem *sysram;
>> char *compatible;
>> - u64 sysram_addr;
>> - u64 size;
>> - u64 sysram_offset;
>> + paddr_t sysram_addr;
>> + paddr_t size;
>> + paddr_t sysram_offset;
>> int rc;
>>
>> node = dt_find_compatible_node(NULL, NULL, "samsung,secure-firmware");
>> @@ -131,7 +131,7 @@ static int __init exynos5_smp_init(void)
>> dprintk(XENLOG_ERR, "Error in %s\n", compatible);
>> return -ENXIO;
>> }
>> - dprintk(XENLOG_INFO, "sysram_addr: %016llx size: %016llx offset: %016llx\n",
>> + dprintk(XENLOG_INFO,"sysram_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"offset: 0x%"PRIpaddr"\n",
>> sysram_addr, size, sysram_offset);
>>
>> sysram = ioremap_nocache(sysram_addr, size);
>> @@ -189,7 +189,7 @@ static int exynos5_cpu_power_up(void __iomem *power, int cpu)
>> return 0;
>> }
>>
>> -static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
>> +static int exynos5_get_pmu_baseandsize(paddr_t *power_base_addr, paddr_t *size)
>> {
>> struct dt_device_node *node;
>> int rc;
>> @@ -215,7 +215,7 @@ static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
>> return -ENXIO;
>> }
>>
>> - dprintk(XENLOG_DEBUG, "power_base_addr: %016llx size: %016llx\n",
>> + dprintk(XENLOG_DEBUG, "power_base_addr: 0x%"PRIpaddr" size: 0x%"PRIpaddr"\n",
>> *power_base_addr, *size);
>>
>> return 0;
>> @@ -223,8 +223,8 @@ static int exynos5_get_pmu_baseandsize(u64 *power_base_addr, u64 *size)
>>
>> static int exynos5_cpu_up(int cpu)
>> {
>> - u64 power_base_addr;
>> - u64 size;
>> + paddr_t power_base_addr;
>> + paddr_t size;
>> void __iomem *power;
>> int rc;
>>
>> @@ -256,8 +256,8 @@ static int exynos5_cpu_up(int cpu)
>>
>> static void exynos5_reset(void)
>> {
>> - u64 power_base_addr;
>> - u64 size;
>> + paddr_t power_base_addr;
>> + paddr_t size;
>> void __iomem *pmu;
>> int rc;
>>
>> diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
>> index 43aaf02e18..32cc8c78b5 100644
>> --- a/xen/drivers/char/exynos4210-uart.c
>> +++ b/xen/drivers/char/exynos4210-uart.c
>> @@ -303,7 +303,7 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
>> const char *config = data;
>> struct exynos4210_uart *uart;
>> int res;
>> - u64 addr, size;
>> + paddr_t addr, size;
>>
>> if ( strcmp(config, "") )
>> printk("WARNING: UART configuration is not supported\n");
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index 58d0ccd889..8ef895a2bb 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -42,8 +42,8 @@
>>
>> static struct ns16550 {
>> int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
>> - u64 io_base; /* I/O port or memory-mapped I/O address. */
>> - u64 io_size;
>> + paddr_t io_base; /* I/O port or memory-mapped I/O address. */
>> + paddr_t io_size;
>> int reg_shift; /* Bits to shift register offset by */
>> int reg_width; /* Size of access to use, the registers
>> * themselves are still bytes */
>> @@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst uart_config[] =
>> static int __init
>> pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>> {
>> - u64 orig_base = uart->io_base;
>> + paddr_t orig_base = uart->io_base;
>> unsigned int b, d, f, nextf, i;
>>
>> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
>> @@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>> else
>> size = len & PCI_BASE_ADDRESS_MEM_MASK;
>>
>> - uart->io_base = ((u64)bar_64 << 32) |
>> + uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
>> (bar & PCI_BASE_ADDRESS_MEM_MASK);
>> }
>> /* IO based */
>> diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
>> index d6a5d59aa2..3b53e1909a 100644
>> --- a/xen/drivers/char/omap-uart.c
>> +++ b/xen/drivers/char/omap-uart.c
>> @@ -324,7 +324,7 @@ static int __init omap_uart_init(struct dt_device_node *dev,
>> struct omap_uart *uart;
>> u32 clkspec;
>> int res;
>> - u64 addr, size;
>> + paddr_t addr, size;
>>
>> if ( strcmp(config, "") )
>> printk("WARNING: UART configuration is not supported\n");
>> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
>> index be67242bc0..256ec11e3f 100644
>> --- a/xen/drivers/char/pl011.c
>> +++ b/xen/drivers/char/pl011.c
>> @@ -222,7 +222,7 @@ static struct uart_driver __read_mostly pl011_driver = {
>> .vuart_info = pl011_vuart,
>> };
>>
>> -static int __init pl011_uart_init(int irq, u64 addr, u64 size, bool sbsa)
>> +static int __init pl011_uart_init(int irq, paddr_t addr, paddr_t size, bool sbsa)
>> {
>> struct pl011 *uart;
>>
>> @@ -258,7 +258,7 @@ static int __init pl011_dt_uart_init(struct dt_device_node *dev,
>> {
>> const char *config = data;
>> int res;
>> - u64 addr, size;
>> + paddr_t addr, size;
>>
>> if ( strcmp(config, "") )
>> {
>> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
>> index 2fccafe340..b425881d06 100644
>> --- a/xen/drivers/char/scif-uart.c
>> +++ b/xen/drivers/char/scif-uart.c
>> @@ -311,7 +311,7 @@ static int __init scif_uart_init(struct dt_device_node *dev,
>> const char *config = data;
>> struct scif_uart *uart;
>> int res;
>> - u64 addr, size;
>> + paddr_t addr, size;
>>
>> if ( strcmp(config, "") )
>> printk("WARNING: UART configuration is not supported\n");
>> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
>> index 0a514821b3..490d253d44 100644
>> --- a/xen/drivers/passthrough/arm/smmu.c
>> +++ b/xen/drivers/passthrough/arm/smmu.c
>> @@ -73,8 +73,8 @@
>> /* Xen: Helpers to get device MMIO and IRQs */
>> struct resource
>> {
>> - u64 addr;
>> - u64 size;
>> + paddr_t addr;
>> + paddr_t size;
>> unsigned int type;
>> };
>>
>> @@ -169,7 +169,7 @@ static void __iomem *devm_ioremap_resource(struct device *dev,
>> ptr = ioremap_nocache(res->addr, res->size);
>> if (!ptr) {
>> dev_err(dev,
>> - "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n",
>> + "ioremap failed (addr 0x%"PRIpaddr" size 0x%"PRIpaddr")\n",
>> res->addr, res->size);
>> return ERR_PTR(-ENOMEM);
>> }
>> --
>> 2.17.1
>>
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@amd.com>
>>> ---
>>>
>>> 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)?
Cheers,
--
Julien Grall
Hi Julien,
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@amd.com>
>>>> ---
>>>>
>>>> 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.
-Ayan
>
> Cheers,
>
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@amd.com>
>>>>> ---
>>>>>
>>>>> 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
On 17.01.2023 18:43, 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@amd.com>
> ---
>
> 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 ++++----
Please make sure you Cc all maintainers.
> @@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst uart_config[] =
> static int __init
> pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
> {
> - u64 orig_base = uart->io_base;
> + paddr_t orig_base = uart->io_base;
> unsigned int b, d, f, nextf, i;
>
> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
> @@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
> else
> size = len & PCI_BASE_ADDRESS_MEM_MASK;
>
> - uart->io_base = ((u64)bar_64 << 32) |
> + uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
> (bar & PCI_BASE_ADDRESS_MEM_MASK);
> }
This looks wrong to me: You shouldn't blindly truncate to 32 bits. You need
to refuse acting on 64-bit BARs with the upper address bits non-zero.
If already you correct logic even in code not used on Arm (which I appreciate),
then there's actually also related command line handling which needs adjustment.
The use of simple_strtoul() to obtain ->io_base is bogus - this then needs to
be simple_strtoull() (perhaps in a separate prereq patch), and in the 32-bit-
paddr case you'd again need to check for truncation (in the patch here).
While doing the review I've noticed this
uart->io_size = spcr->serial_port.bit_width;
in ns16550_acpi_uart_init(). This was introduced in 17b516196c55 ("ns16550:
add ACPI support for ARM only"), so Wei, Julien: Doesn't the right hand value
need DIV_ROUND_UP(, 8) to convert from bit count to byte count?
Jan
Hi Jan,
On 18/01/2023 08:40, Jan Beulich wrote:
> On 17.01.2023 18:43, 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@amd.com>
>> ---
>>
>> 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 ++++----
> Please make sure you Cc all maintainers.
Ack.
>
>> @@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst uart_config[] =
>> static int __init
>> pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>> {
>> - u64 orig_base = uart->io_base;
>> + paddr_t orig_base = uart->io_base;
>> unsigned int b, d, f, nextf, i;
>>
>> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
>> @@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>> else
>> size = len & PCI_BASE_ADDRESS_MEM_MASK;
>>
>> - uart->io_base = ((u64)bar_64 << 32) |
>> + uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
>> (bar & PCI_BASE_ADDRESS_MEM_MASK);
>> }
> This looks wrong to me: You shouldn't blindly truncate to 32 bits. You need
> to refuse acting on 64-bit BARs with the upper address bits non-zero.
Yes, I was treating this like others (where Xen does not detect for
truncation while getting the address/size from device-tree and
typecasting it to paddr_t).
However in this case, as Xen is reading from PCI registers, so it needs
to check for truncation.
I think the following change should do good.
@@ -1180,6 +1180,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
skip_amt, unsigned int idx)
unsigned int bar_idx = 0, port_idx = idx;
uint32_t bar, bar_64 = 0, len, len_64;
u64 size = 0;
+ uint64_t io_base = 0;
const struct ns16550_config_param *param = uart_param;
nextf = (f || (pci_conf_read16(PCI_SBDF(0, b, d, f),
@@ -1260,8 +1261,11 @@ pci_uart_config(struct ns16550 *uart, bool_t
skip_amt, unsigned int idx)
else
size = len & PCI_BASE_ADDRESS_MEM_MASK;
- uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
+ io_base = ((u64)bar_64 << 32) |
(bar & PCI_BASE_ADDRESS_MEM_MASK);
+
+ uart->io_base = (paddr_t) io_base;
+ ASSERT(uart->io_base == io_base); /* Detect
truncation */
}
/* IO based */
else if ( !param->mmio && (bar &
PCI_BASE_ADDRESS_SPACE_IO) )
>
> If already you correct logic even in code not used on Arm (which I appreciate),
> then there's actually also related command line handling which needs adjustment.
> The use of simple_strtoul() to obtain ->io_base is bogus - this then needs to
> be simple_strtoull() (perhaps in a separate prereq patch), and in the 32-bit-
> paddr case you'd again need to check for truncation (in the patch here).
Agreed this needs to be done in a separate prereq patch.
>
> While doing the review I've noticed this
>
> uart->io_size = spcr->serial_port.bit_width;
>
> in ns16550_acpi_uart_init(). This was introduced in 17b516196c55 ("ns16550:
> add ACPI support for ARM only"), so Wei, Julien: Doesn't the right hand value
> need DIV_ROUND_UP(, 8) to convert from bit count to byte count?
Yes, I think it should be
uart->io_size = DIV_ROUND_UP(spcr->serial_port.bit_width, BITS_PER_BYTE);
However, Julien/Wei can confirm this.
- Ayan
>
> Jan
On 18.01.2023 12:15, Ayan Kumar Halder wrote:
> On 18/01/2023 08:40, Jan Beulich wrote:
>> On 17.01.2023 18:43, Ayan Kumar Halder wrote:
>>> @@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst uart_config[] =
>>> static int __init
>>> pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>>> {
>>> - u64 orig_base = uart->io_base;
>>> + paddr_t orig_base = uart->io_base;
>>> unsigned int b, d, f, nextf, i;
>>>
>>> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
>>> @@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>>> else
>>> size = len & PCI_BASE_ADDRESS_MEM_MASK;
>>>
>>> - uart->io_base = ((u64)bar_64 << 32) |
>>> + uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
>>> (bar & PCI_BASE_ADDRESS_MEM_MASK);
>>> }
>> This looks wrong to me: You shouldn't blindly truncate to 32 bits. You need
>> to refuse acting on 64-bit BARs with the upper address bits non-zero.
>
> Yes, I was treating this like others (where Xen does not detect for
> truncation while getting the address/size from device-tree and
> typecasting it to paddr_t).
>
> However in this case, as Xen is reading from PCI registers, so it needs
> to check for truncation.
>
> I think the following change should do good.
>
> @@ -1180,6 +1180,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
> skip_amt, unsigned int idx)
> unsigned int bar_idx = 0, port_idx = idx;
> uint32_t bar, bar_64 = 0, len, len_64;
> u64 size = 0;
> + uint64_t io_base = 0;
> const struct ns16550_config_param *param = uart_param;
>
> nextf = (f || (pci_conf_read16(PCI_SBDF(0, b, d, f),
> @@ -1260,8 +1261,11 @@ pci_uart_config(struct ns16550 *uart, bool_t
> skip_amt, unsigned int idx)
> else
> size = len & PCI_BASE_ADDRESS_MEM_MASK;
>
> - uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
> + io_base = ((u64)bar_64 << 32) |
> (bar & PCI_BASE_ADDRESS_MEM_MASK);
> +
> + uart->io_base = (paddr_t) io_base;
> + ASSERT(uart->io_base == io_base); /* Detect
> truncation */
> }
> /* IO based */
> else if ( !param->mmio && (bar &
> PCI_BASE_ADDRESS_SPACE_IO) )
An assertion can only ever check assumption on behavior elsewhere in Xen.
Anything external needs handling properly, including in non-debug builds.
Jan
On Wed, Jan 18, 2023 at 1:15 PM Jan Beulich <jbeulich@suse.com> wrote:
> On 18.01.2023 12:15, Ayan Kumar Halder wrote:
> > On 18/01/2023 08:40, Jan Beulich wrote:
> >> On 17.01.2023 18:43, Ayan Kumar Halder wrote:
> >>> @@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst
> uart_config[] =
> >>> static int __init
> >>> pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int
> idx)
> >>> {
> >>> - u64 orig_base = uart->io_base;
> >>> + paddr_t orig_base = uart->io_base;
> >>> unsigned int b, d, f, nextf, i;
> >>>
> >>> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on
> bus 0. */
> >>> @@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
> skip_amt, unsigned int idx)
> >>> else
> >>> size = len & PCI_BASE_ADDRESS_MEM_MASK;
> >>>
> >>> - uart->io_base = ((u64)bar_64 << 32) |
> >>> + uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
> >>> (bar &
> PCI_BASE_ADDRESS_MEM_MASK);
> >>> }
> >> This looks wrong to me: You shouldn't blindly truncate to 32 bits. You
> need
> >> to refuse acting on 64-bit BARs with the upper address bits non-zero.
> >
> > Yes, I was treating this like others (where Xen does not detect for
> > truncation while getting the address/size from device-tree and
> > typecasting it to paddr_t).
> >
> > However in this case, as Xen is reading from PCI registers, so it needs
> > to check for truncation.
> >
> > I think the following change should do good.
> >
> > @@ -1180,6 +1180,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
> > skip_amt, unsigned int idx)
> > unsigned int bar_idx = 0, port_idx = idx;
> > uint32_t bar, bar_64 = 0, len, len_64;
> > u64 size = 0;
> > + uint64_t io_base = 0;
> > const struct ns16550_config_param *param = uart_param;
> >
> > nextf = (f || (pci_conf_read16(PCI_SBDF(0, b, d, f),
> > @@ -1260,8 +1261,11 @@ pci_uart_config(struct ns16550 *uart, bool_t
> > skip_amt, unsigned int idx)
> > else
> > size = len & PCI_BASE_ADDRESS_MEM_MASK;
> >
> > - uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
> > + io_base = ((u64)bar_64 << 32) |
> > (bar & PCI_BASE_ADDRESS_MEM_MASK);
> > +
> > + uart->io_base = (paddr_t) io_base;
> > + ASSERT(uart->io_base == io_base); /* Detect
> > truncation */
> > }
> > /* IO based */
> > else if ( !param->mmio && (bar &
> > PCI_BASE_ADDRESS_SPACE_IO) )
>
> An assertion can only ever check assumption on behavior elsewhere in Xen.
> Anything external needs handling properly, including in non-debug builds.
>
Except in this case, it's detecting the result of the compiler cast just
above it, isn't it? In which case it seems like it should be a
BUILD_BUG_ON() of some sort.
-George
On 18.01.2023 14:34, George Dunlap wrote:
> On Wed, Jan 18, 2023 at 1:15 PM Jan Beulich <jbeulich@suse.com> wrote:
>
>> On 18.01.2023 12:15, Ayan Kumar Halder wrote:
>>> On 18/01/2023 08:40, Jan Beulich wrote:
>>>> On 17.01.2023 18:43, Ayan Kumar Halder wrote:
>>>>> @@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst
>> uart_config[] =
>>>>> static int __init
>>>>> pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int
>> idx)
>>>>> {
>>>>> - u64 orig_base = uart->io_base;
>>>>> + paddr_t orig_base = uart->io_base;
>>>>> unsigned int b, d, f, nextf, i;
>>>>>
>>>>> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on
>> bus 0. */
>>>>> @@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
>> skip_amt, unsigned int idx)
>>>>> else
>>>>> size = len & PCI_BASE_ADDRESS_MEM_MASK;
>>>>>
>>>>> - uart->io_base = ((u64)bar_64 << 32) |
>>>>> + uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
>>>>> (bar &
>> PCI_BASE_ADDRESS_MEM_MASK);
>>>>> }
>>>> This looks wrong to me: You shouldn't blindly truncate to 32 bits. You
>> need
>>>> to refuse acting on 64-bit BARs with the upper address bits non-zero.
>>>
>>> Yes, I was treating this like others (where Xen does not detect for
>>> truncation while getting the address/size from device-tree and
>>> typecasting it to paddr_t).
>>>
>>> However in this case, as Xen is reading from PCI registers, so it needs
>>> to check for truncation.
>>>
>>> I think the following change should do good.
>>>
>>> @@ -1180,6 +1180,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
>>> skip_amt, unsigned int idx)
>>> unsigned int bar_idx = 0, port_idx = idx;
>>> uint32_t bar, bar_64 = 0, len, len_64;
>>> u64 size = 0;
>>> + uint64_t io_base = 0;
>>> const struct ns16550_config_param *param = uart_param;
>>>
>>> nextf = (f || (pci_conf_read16(PCI_SBDF(0, b, d, f),
>>> @@ -1260,8 +1261,11 @@ pci_uart_config(struct ns16550 *uart, bool_t
>>> skip_amt, unsigned int idx)
>>> else
>>> size = len & PCI_BASE_ADDRESS_MEM_MASK;
>>>
>>> - uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
>>> + io_base = ((u64)bar_64 << 32) |
>>> (bar & PCI_BASE_ADDRESS_MEM_MASK);
>>> +
>>> + uart->io_base = (paddr_t) io_base;
>>> + ASSERT(uart->io_base == io_base); /* Detect
>>> truncation */
>>> }
>>> /* IO based */
>>> else if ( !param->mmio && (bar &
>>> PCI_BASE_ADDRESS_SPACE_IO) )
>>
>> An assertion can only ever check assumption on behavior elsewhere in Xen.
>> Anything external needs handling properly, including in non-debug builds.
>>
>
> Except in this case, it's detecting the result of the compiler cast just
> above it, isn't it?
Not really, no - it checks whether there was truncation, but the
absence (or presence) thereof is still a property of the underlying
system, not one in Xen.
> In which case it seems like it should be a BUILD_BUG_ON() of some sort.
Such a check would then be to make sure paddr_t == uint64_t, which is
precisely an equivalence Ayan wants to do away with.
Jan
On Wed, Jan 18, 2023 at 1:58 PM Jan Beulich <jbeulich@suse.com> wrote:
> On 18.01.2023 14:34, George Dunlap wrote:
> > On Wed, Jan 18, 2023 at 1:15 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> >> On 18.01.2023 12:15, Ayan Kumar Halder wrote:
> >>> On 18/01/2023 08:40, Jan Beulich wrote:
> >>>> On 17.01.2023 18:43, Ayan Kumar Halder wrote:
> >>>>> @@ -1166,7 +1166,7 @@ static const struct ns16550_config __initconst
> >> uart_config[] =
> >>>>> static int __init
> >>>>> pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int
> >> idx)
> >>>>> {
> >>>>> - u64 orig_base = uart->io_base;
> >>>>> + paddr_t orig_base = uart->io_base;
> >>>>> unsigned int b, d, f, nextf, i;
> >>>>>
> >>>>> /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on
> >> bus 0. */
> >>>>> @@ -1259,7 +1259,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
> >> skip_amt, unsigned int idx)
> >>>>> else
> >>>>> size = len & PCI_BASE_ADDRESS_MEM_MASK;
> >>>>>
> >>>>> - uart->io_base = ((u64)bar_64 << 32) |
> >>>>> + uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
> >>>>> (bar &
> >> PCI_BASE_ADDRESS_MEM_MASK);
> >>>>> }
> >>>> This looks wrong to me: You shouldn't blindly truncate to 32 bits. You
> >> need
> >>>> to refuse acting on 64-bit BARs with the upper address bits non-zero.
> >>>
> >>> Yes, I was treating this like others (where Xen does not detect for
> >>> truncation while getting the address/size from device-tree and
> >>> typecasting it to paddr_t).
> >>>
> >>> However in this case, as Xen is reading from PCI registers, so it needs
> >>> to check for truncation.
> >>>
> >>> I think the following change should do good.
> >>>
> >>> @@ -1180,6 +1180,7 @@ pci_uart_config(struct ns16550 *uart, bool_t
> >>> skip_amt, unsigned int idx)
> >>> unsigned int bar_idx = 0, port_idx = idx;
> >>> uint32_t bar, bar_64 = 0, len, len_64;
> >>> u64 size = 0;
> >>> + uint64_t io_base = 0;
> >>> const struct ns16550_config_param *param =
> uart_param;
> >>>
> >>> nextf = (f || (pci_conf_read16(PCI_SBDF(0, b, d, f),
> >>> @@ -1260,8 +1261,11 @@ pci_uart_config(struct ns16550 *uart, bool_t
> >>> skip_amt, unsigned int idx)
> >>> else
> >>> size = len & PCI_BASE_ADDRESS_MEM_MASK;
> >>>
> >>> - uart->io_base = (paddr_t) ((u64)bar_64 << 32) |
> >>> + io_base = ((u64)bar_64 << 32) |
> >>> (bar &
> PCI_BASE_ADDRESS_MEM_MASK);
> >>> +
> >>> + uart->io_base = (paddr_t) io_base;
> >>> + ASSERT(uart->io_base == io_base); /* Detect
> >>> truncation */
> >>> }
> >>> /* IO based */
> >>> else if ( !param->mmio && (bar &
> >>> PCI_BASE_ADDRESS_SPACE_IO) )
> >>
> >> An assertion can only ever check assumption on behavior elsewhere in
> Xen.
> >> Anything external needs handling properly, including in non-debug
> builds.
> >>
> >
> > Except in this case, it's detecting the result of the compiler cast just
> > above it, isn't it?
>
> Not really, no - it checks whether there was truncation, but the
> absence (or presence) thereof is still a property of the underlying
> system, not one in Xen.
>
Ah, gotcha. Ayan, it might be helpful to take a look at the 'Handling
unexpected conditions' section of our CODING_STYLE [1] for a discussion of
when (and when not) to use ASSERT().
-George
[1] https://github.com/xen-project/xen/blob/master/CODING_STYLE
© 2016 - 2026 Red Hat, Inc.