Hi Ayan,
On 13/04/2023 19:37, Ayan Kumar Halder wrote:
>
>
> dt_device_get_address() can accept uint64_t only for address and size.
> However, the address/size denotes physical addresses. Thus, they should
> be represented by 'paddr_t'.
> Consequently, we introduce a wrapper for dt_device_get_address() ie
> dt_device_get_paddr() which accepts address/size as paddr_t and inturn
> invokes dt_device_get_address() after converting address/size to
> uint64_t.
>
> The reason for introducing this is that in future 'paddr_t' may not
> always be 64-bit. Thus, we need an explicit wrapper to do the type
> conversion and return an error in case of truncation.
>
> With this, callers can now invoke dt_device_get_paddr().
>
> Signed-off-by: Ayan Kumar Halder <[email protected]>
> ---
> Changes from -
>
> v1 - 1. New patch.
>
> v2 - 1. Extracted part of "[XEN v2 05/11] xen/arm: Use paddr_t instead of u64
> for address/size"
> into this patch.
>
> 2. dt_device_get_address() callers now invoke dt_device_get_paddr() instead.
>
> 3. Logged error in case of truncation.
>
> v3 - 1. Modified the truncation checks as "dt_addr != (paddr_t)dt_addr".
> 2. Some sanity fixes.
>
> v4 - 1. Some sanity fixes.
> 2. Preserved the declaration of dt_device_get_address() in
> xen/include/xen/device_tree.h. The reason being it is currently used by
> ns16550.c. This driver requires some more changes as pointed by Jan in
> https://lore.kernel.org/xen-devel/[email protected]/
> which is to be addressed as a separate series.
>
> xen/arch/arm/domain_build.c | 10 +++----
> xen/arch/arm/gic-v2.c | 10 +++----
> xen/arch/arm/gic-v3-its.c | 4 +--
> xen/arch/arm/gic-v3.c | 10 +++----
> xen/arch/arm/pci/pci-host-common.c | 6 ++--
> xen/arch/arm/platforms/brcm-raspberry-pi.c | 2 +-
> xen/arch/arm/platforms/brcm.c | 6 ++--
> xen/arch/arm/platforms/exynos5.c | 32 ++++++++++----------
> xen/arch/arm/platforms/sunxi.c | 2 +-
> xen/arch/arm/platforms/xgene-storm.c | 2 +-
> xen/common/device_tree.c | 35 ++++++++++++++++++++++
> xen/drivers/char/cadence-uart.c | 4 +--
> xen/drivers/char/exynos4210-uart.c | 4 +--
> xen/drivers/char/imx-lpuart.c | 4 +--
> xen/drivers/char/meson-uart.c | 4 +--
> xen/drivers/char/mvebu-uart.c | 4 +--
> xen/drivers/char/omap-uart.c | 4 +--
> xen/drivers/char/pl011.c | 6 ++--
> xen/drivers/char/scif-uart.c | 4 +--
> xen/drivers/passthrough/arm/ipmmu-vmsa.c | 8 ++---
> xen/drivers/passthrough/arm/smmu-v3.c | 2 +-
> xen/drivers/passthrough/arm/smmu.c | 8 ++---
> xen/include/xen/device_tree.h | 13 ++++++++
> 23 files changed, 116 insertions(+), 68 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 6c9712ab7b..fdef74e7ff 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -955,6 +955,41 @@ int dt_device_get_address(const struct dt_device_node
> *dev, unsigned int index,
> return 0;
> }
>
> +int dt_device_get_paddr(const struct dt_device_node *dev, unsigned int index,
> + paddr_t *addr, paddr_t *size)
> +{
> + uint64_t dt_addr = 0, dt_size = 0;
no need for these assignments
> + int ret;
> +
> + ret = dt_device_get_address(dev, index, &dt_addr, &dt_size);
> + if ( ret )
> + return ret;
> +
> + if ( addr )
Since dt_device_get_paddr() is a wrapper for dt_device_get_address(), I think
you should maintain the same
error logic for addr i.e. if no addr was specified (NULL), you return -EINVAL:
if ( !addr )
return -EINVAL;
> + {
> + if ( dt_addr != (paddr_t)dt_addr )
> + {
> + printk("Error: Physical address 0x%"PRIx64" for node=%s is
> greater than max width (%zu bytes) supported\n",
> + dt_addr, dev->name, sizeof(paddr_t));
> + return -ERANGE;
> + }
> +
> + *addr = dt_addr;
> + }
> +
> + if ( size )
> + {
> + if ( dt_size != (paddr_t)dt_size )
> + {
> + printk("Error: Physical size 0x%"PRIx64" for node=%s is greater
> than max width (%zu bytes) supported\n",
> + dt_size, dev->name, sizeof(paddr_t));
> + return -ERANGE;
> + }
add empty line
~Michal
