Re: [PATCH v5 11/44] x86/boot: split bootstrap_map_addr() out of bootstrap_map()

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 10/9/24 11:38, Jan Beulich wrote:
> On 06.10.2024 23:49, Daniel P. Smith wrote:
> > From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >
> > Using an interface based on addresses directly, not modules.
> >
> > No functional change.
> >
> > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> > ---
> >   xen/arch/x86/include/asm/setup.h |  1 +
> >   xen/arch/x86/setup.c             | 19 +++++++++++++------
> >   2 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
> > index 3d189521189d..213584b05fb2 100644
> > --- a/xen/arch/x86/include/asm/setup.h
> > +++ b/xen/arch/x86/include/asm/setup.h
> > @@ -36,6 +36,7 @@ extern struct boot_info xen_boot_info;
> >   
> >   unsigned long initial_images_nrpages(nodeid_t node);
> >   void discard_initial_images(void);
> > +void *bootstrap_map_addr(paddr_t start, paddr_t end);
> Nothing is being said about why this function needs a declaration here
> and ...
You are correct, as far as this series is concerned, nothing external 
uses this. Will drop this declaration....
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -437,24 +437,22 @@ static void __init normalise_cpu_order(void)
> >    * Ensure a given physical memory range is present in the bootstrap mappings.
> >    * Use superpage mappings to ensure that pagetable memory needn't be 
> > allocated.
> >    */
> > -void *__init bootstrap_map(const module_t *mod)
> > +void *__init bootstrap_map_addr(paddr_t start, paddr_t end)
> ... isn't instead static here. Bugseng folks have put in quite a bit of
> effort to remove such anomalies (which Misra doesn't like) from the code
> base; I don't think we should introduce new ones. I didn't peek ahead
> further than just the next patch, where the function gains a new use,
> but could still be static, so it's possible I'm simply missing a
> subsequent use from another CU. Yet then the function ought to become
> non-static only there.
...and will make this static.

v/r,
dps