| Message ID | 20210212200139.26911-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
Andrew Cooper writes ("[PATCH v1.1 03/10] tools/libxg: Drop stale p2m logic from ARM's meminit()"):
> Various version of gcc, when compiling with -Og, complain:
>
> xg_dom_arm.c: In function 'meminit':
> xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 420 | dom->p2m_size = p2m_size;
> | ~~~~~~~~~~~~~~^~~~~~~~~~
>
> This is actually entirely stale code since ee21f10d70^..97e34ad22d which
> removed the 1:1 identity p2m for translated domains.
>
> Drop the write of d->p2m_size, and the p2m_size local variable. Reposition
> the p2m_size field in struct xc_dom_image and correct some stale
> documentation.
>
> This change really ought to have been part of the original cleanup series.
>
> No actual change to how ARM domains are constructed.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
Hi Andrew, On 12/02/2021 20:01, Andrew Cooper wrote: > Various version of gcc, when compiling with -Og, complain: > > xg_dom_arm.c: In function 'meminit': > xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized in this function [-Werror=maybe-uninitialized] > 420 | dom->p2m_size = p2m_size; > | ~~~~~~~~~~~~~~^~~~~~~~~~ > > This is actually entirely stale code since ee21f10d70^..97e34ad22d which > removed the 1:1 identity p2m for translated domains. > > Drop the write of d->p2m_size, and the p2m_size local variable. Reposition > the p2m_size field in struct xc_dom_image and correct some stale > documentation. > > This change really ought to have been part of the original cleanup series. > > No actual change to how ARM domains are constructed. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Julien Grall <jgrall@amazon.com> Cheers, > --- > CC: Ian Jackson <iwj@xenproject.org> > CC: Wei Liu <wl@xen.org> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien@xen.org> > > v2: > * Delete stale p2m_size infrastructure. > --- > tools/include/xenguest.h | 5 ++--- > tools/libs/guest/xg_dom_arm.c | 5 ----- > 2 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h > index 775cf34c04..217022b6e7 100644 > --- a/tools/include/xenguest.h > +++ b/tools/include/xenguest.h > @@ -145,6 +145,7 @@ struct xc_dom_image { > * eventually copied into guest context. > */ > xen_pfn_t *pv_p2m; > + xen_pfn_t p2m_size; /* number of pfns covered by pv_p2m */ > > /* physical memory > * > @@ -154,12 +155,10 @@ struct xc_dom_image { > * > * An ARM guest has GUEST_RAM_BANKS regions of RAM, with > * rambank_size[i] pages in each. The lowest RAM address > - * (corresponding to the base of the p2m arrays above) is stored > - * in rambase_pfn. > + * is stored in rambase_pfn. > */ > xen_pfn_t rambase_pfn; > xen_pfn_t total_pages; > - xen_pfn_t p2m_size; /* number of pfns covered by p2m */ > struct xc_dom_phys *phys_pages; > #if defined (__arm__) || defined(__aarch64__) > xen_pfn_t rambank_size[GUEST_RAM_BANKS]; > diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c > index 94948d2b20..b4c24f15fb 100644 > --- a/tools/libs/guest/xg_dom_arm.c > +++ b/tools/libs/guest/xg_dom_arm.c > @@ -373,7 +373,6 @@ static int meminit(struct xc_dom_image *dom) > const uint64_t modsize = dtb_size + ramdisk_size; > const uint64_t ram128mb = bankbase[0] + (128<<20); > > - xen_pfn_t p2m_size; > uint64_t bank0end; > > assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]); > @@ -409,16 +408,12 @@ static int meminit(struct xc_dom_image *dom) > > ramsize -= banksize; > > - p2m_size = ( bankbase[i] + banksize - bankbase[0] ) >> XC_PAGE_SHIFT; > - > dom->rambank_size[i] = banksize >> XC_PAGE_SHIFT; > } > > assert(dom->rambank_size[0] != 0); > assert(ramsize == 0); /* Too much RAM is rejected above */ > > - dom->p2m_size = p2m_size; > - > /* setup initial p2m and allocate guest memory */ > for ( i = 0; i < GUEST_RAM_BANKS && dom->rambank_size[i]; i++ ) > { >
diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h index 775cf34c04..217022b6e7 100644 --- a/tools/include/xenguest.h +++ b/tools/include/xenguest.h @@ -145,6 +145,7 @@ struct xc_dom_image { * eventually copied into guest context. */ xen_pfn_t *pv_p2m; + xen_pfn_t p2m_size; /* number of pfns covered by pv_p2m */ /* physical memory * @@ -154,12 +155,10 @@ struct xc_dom_image { * * An ARM guest has GUEST_RAM_BANKS regions of RAM, with * rambank_size[i] pages in each. The lowest RAM address - * (corresponding to the base of the p2m arrays above) is stored - * in rambase_pfn. + * is stored in rambase_pfn. */ xen_pfn_t rambase_pfn; xen_pfn_t total_pages; - xen_pfn_t p2m_size; /* number of pfns covered by p2m */ struct xc_dom_phys *phys_pages; #if defined (__arm__) || defined(__aarch64__) xen_pfn_t rambank_size[GUEST_RAM_BANKS]; diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c index 94948d2b20..b4c24f15fb 100644 --- a/tools/libs/guest/xg_dom_arm.c +++ b/tools/libs/guest/xg_dom_arm.c @@ -373,7 +373,6 @@ static int meminit(struct xc_dom_image *dom) const uint64_t modsize = dtb_size + ramdisk_size; const uint64_t ram128mb = bankbase[0] + (128<<20); - xen_pfn_t p2m_size; uint64_t bank0end; assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]); @@ -409,16 +408,12 @@ static int meminit(struct xc_dom_image *dom) ramsize -= banksize; - p2m_size = ( bankbase[i] + banksize - bankbase[0] ) >> XC_PAGE_SHIFT; - dom->rambank_size[i] = banksize >> XC_PAGE_SHIFT; } assert(dom->rambank_size[0] != 0); assert(ramsize == 0); /* Too much RAM is rejected above */ - dom->p2m_size = p2m_size; - /* setup initial p2m and allocate guest memory */ for ( i = 0; i < GUEST_RAM_BANKS && dom->rambank_size[i]; i++ ) {
Various version of gcc, when compiling with -Og, complain: xg_dom_arm.c: In function 'meminit': xg_dom_arm.c:420:19: error: 'p2m_size' may be used uninitialized in this function [-Werror=maybe-uninitialized] 420 | dom->p2m_size = p2m_size; | ~~~~~~~~~~~~~~^~~~~~~~~~ This is actually entirely stale code since ee21f10d70^..97e34ad22d which removed the 1:1 identity p2m for translated domains. Drop the write of d->p2m_size, and the p2m_size local variable. Reposition the p2m_size field in struct xc_dom_image and correct some stale documentation. This change really ought to have been part of the original cleanup series. No actual change to how ARM domains are constructed. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Ian Jackson <iwj@xenproject.org> CC: Wei Liu <wl@xen.org> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien@xen.org> v2: * Delete stale p2m_size infrastructure. --- tools/include/xenguest.h | 5 ++--- tools/libs/guest/xg_dom_arm.c | 5 ----- 2 files changed, 2 insertions(+), 8 deletions(-)