Hi Michal, On 19/06/2024 07:46, Michal Orzel wrote:
Building Xen with CONFIG_STATIC_SHM=y results in a build failure:arch/arm/static-shmem.c: In function 'process_shm': arch/arm/static-shmem.c:327:41: error: 'gbase' may be used uninitialized [-Werror=maybe-uninitialized] 327 | if ( is_domain_direct_mapped(d) && (pbase != gbase) ) arch/arm/static-shmem.c:305:17: note: 'gbase' was declared here 305 | paddr_t gbase, pbase, psize; This is because the commit cb1ddafdc573 adds a check referencing gbase/pbase variables which were not yet assigned a value. Fix it. Fixes: cb1ddafdc573 ("xen/arm/static-shmem: Static-shmem should be direct-mapped for direct-mapped domains") Signed-off-by: Michal Orzel <michal.or...@amd.com> --- Rationale for 4.19: this patch fixes a build failure reported by CI: https://gitlab.com/xen-project/xen/-/jobs/7131807878 --- xen/arch/arm/static-shmem.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c index c434b96e6204..cd48d2896b7e 100644 --- a/xen/arch/arm/static-shmem.c +++ b/xen/arch/arm/static-shmem.c @@ -324,12 +324,6 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo, printk("%pd: static shared memory bank not found: '%s'", d, shm_id); return -ENOENT; } - if ( is_domain_direct_mapped(d) && (pbase != gbase) ) - { - printk("%pd: physical address 0x%"PRIpaddr" and guest address 0x%"PRIpaddr" are not direct-mapped.\n", - d, pbase, gbase); - return -EINVAL; - }pbase = boot_shm_bank->start;psize = boot_shm_bank->size; @@ -353,6 +347,13 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo, /* guest phys address is after host phys address */ gbase = dt_read_paddr(cells + addr_cells, addr_cells);+ if ( is_domain_direct_mapped(d) && (pbase != gbase) )+ { + printk("%pd: physical address 0x%"PRIpaddr" and guest address 0x%"PRIpaddr" are not direct-mapped.\n", + d, pbase, gbase); + return -EINVAL; + } +
Before this patch, the check was globally. I guess the intention was it covers the two part of the "if". But now, you only have it in when "paddr" is specified in the DT.
From a brief look at the code, I can't figure out why we don't need a similar check on the else path. Is this because it is guarantee that will be paddr == gaddr?
Cheers, -- Julien Grall
