Re: [Xen-devel] [PATCH 19/22] mini-os: remove using start_info in architecture independent code

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Juergen Gross, on Tue 23 Aug 2016 17:16:05 +0200, wrote:
> The start_info structure should be used only in case of CONFIG_PARAVIRT
> defined. Remove it from being used in other places. Especially the
> usage as parameter for applications linked to the kernel is no good
> idea.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>

> ---
>  arch/arm/setup.c     |  8 --------
>  arch/x86/setup.c     |  8 --------
>  daytime.c            |  2 +-
>  include/hypervisor.h | 11 -----------
>  kernel.c             |  6 +++---
>  main.c               |  6 +++---
>  test.c               | 20 ++++++++++----------
>  7 files changed, 17 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/arm/setup.c b/arch/arm/setup.c
> index 05b405b..b65023c 100644
> --- a/arch/arm/setup.c
> +++ b/arch/arm/setup.c
> @@ -9,13 +9,6 @@
>  #include <libfdt.h>
>  
>  /*
> - * This structure contains start-of-day info, such as pagetable base pointer,
> - * address of the shared_info structure, and things like that.
> - * On x86, the hypervisor passes it to us. On ARM, we fill it in ourselves.
> - */
> -union start_info_union start_info_union;
> -
> -/*
>   * Shared page for communicating with the hypervisor.
>   * Events flags go here, for example.
>   */
> @@ -47,7 +40,6 @@ void arch_init(void *dtb_pointer, uint32_t physical_offset)
>      /* Map shared_info page */
>      HYPERVISOR_shared_info = map_shared_info(NULL);
>  
> -    /* Fill in start_info */
>      get_console(NULL);
>      get_xenbus(NULL);
>  
> diff --git a/arch/x86/setup.c b/arch/x86/setup.c
> index 278e88f..50aa504 100644
> --- a/arch/x86/setup.c
> +++ b/arch/x86/setup.c
> @@ -40,12 +40,6 @@
>  shared_info_t *HYPERVISOR_shared_info;
>  
>  /*
> - * This structure contains start-of-day info, such as pagetable base pointer,
> - * address of the shared_info structure, and things like that.
> - */
> -union start_info_union start_info_union;
> -
> -/*
>   * Just allocate the kernel stack here. SS:ESP is set up to point here
>   * in head.S.
>   */
> @@ -151,7 +145,6 @@ arch_init(void *par)
>       /* Setup memory management info from start_info. */
>       arch_mm_preinit(par);
>  
> -     /* Copy the start_info struct to a globally-accessible area. */
>       /* WARN: don't do printk before here, it uses information from
>          shared_info. Use xprintk instead. */
>       get_console(par);
> @@ -162,7 +155,6 @@ arch_init(void *par)
>       HYPERVISOR_shared_info = map_shared_info(par);
>  
>       si = par;
> -     memcpy(&start_info, si, sizeof(*si));
>  
>       /* print out some useful information  */
>       printk("Xen Minimal OS!\n");
> diff --git a/daytime.c b/daytime.c
> index 7dc0de0..6049e78 100644
> --- a/daytime.c
> +++ b/daytime.c
> @@ -60,7 +60,7 @@ void run_server(void *p)
>  }
>  
>  
> -int app_main(start_info_t *si)
> +int app_main(void *p)
>  {
>      create_thread("server", run_server, NULL);
>      return 0;
> diff --git a/include/hypervisor.h b/include/hypervisor.h
> index 7c44702..3073a8a 100644
> --- a/include/hypervisor.h
> +++ b/include/hypervisor.h
> @@ -26,17 +26,6 @@
>  #include <xen/hvm/hvm_op.h>
>  #include <mini-os/traps.h>
>  
> -/*
> - * a placeholder for the start of day information passed up from the 
> hypervisor
> - */
> -union start_info_union
> -{
> -    start_info_t start_info;
> -    char padding[512];
> -};
> -extern union start_info_union start_info_union;
> -#define start_info (start_info_union.start_info)
> -
>  /* hypervisor.c */
>  #ifndef CONFIG_PARAVIRT
>  int hvm_get_parameter(int idx, uint64_t *value);
> diff --git a/kernel.c b/kernel.c
> index f22997e..0d84a9b 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -110,9 +110,9 @@ static void shutdown_thread(void *p)
>  
>  
>  /* This should be overridden by the application we are linked against. */
> -__attribute__((weak)) int app_main(start_info_t *si)
> +__attribute__((weak)) int app_main(void *p)
>  {
> -    printk("kernel.c: dummy main: start_info=%p\n", si);
> +    printk("kernel.c: dummy main: par=%p\n", p);
>      return 0;
>  }
>  
> @@ -149,7 +149,7 @@ void start_kernel(void)
>  #endif
>  
>      /* Call (possibly overridden) app_main() */
> -    app_main(&start_info);
> +    app_main(NULL);
>  
>      /* Everything initialised, start idle thread */
>      run_idle_thread();
> diff --git a/main.c b/main.c
> index 71e3be3..263364c 100644
> --- a/main.c
> +++ b/main.c
> @@ -185,10 +185,10 @@ void _exit(int ret)
>      do_exit();
>  }
>  
> -int app_main(start_info_t *si)
> +int app_main(void *p)
>  {
> -    printk("main.c: dummy main: start_info=%p\n", si);
> -    main_thread = create_thread("main", call_main, si);
> +    printk("main.c: dummy main: par=%p\n", p);
> +    main_thread = create_thread("main", call_main, p);
>      return 0;
>  }
>  #endif
> diff --git a/test.c b/test.c
> index 154af49..42a2666 100644
> --- a/test.c
> +++ b/test.c
> @@ -552,28 +552,28 @@ static void shutdown_thread(void *p)
>  }
>  #endif
>  
> -int app_main(start_info_t *si)
> +int app_main(void *p)
>  {
> -    printk("Test main: start_info=%p\n", si);
> +    printk("Test main: par=%p\n", p);
>  #ifdef CONFIG_XENBUS
> -    create_thread("xenbus_tester", xenbus_tester, si);
> +    create_thread("xenbus_tester", xenbus_tester, p);
>  #endif
> -    create_thread("periodic_thread", periodic_thread, si);
> +    create_thread("periodic_thread", periodic_thread, p);
>  #ifdef CONFIG_NETFRONT
> -    create_thread("netfront", netfront_thread, si);
> +    create_thread("netfront", netfront_thread, p);
>  #endif
>  #ifdef CONFIG_BLKFRONT
> -    create_thread("blkfront", blkfront_thread, si);
> +    create_thread("blkfront", blkfront_thread, p);
>  #endif
>  #if defined(CONFIG_FBFRONT) && defined(CONFIG_KBDFRONT)
> -    create_thread("fbfront", fbfront_thread, si);
> -    create_thread("kbdfront", kbdfront_thread, si);
> +    create_thread("fbfront", fbfront_thread, p);
> +    create_thread("kbdfront", kbdfront_thread, p);
>  #endif
>  #ifdef CONFIG_PCIFRONT
> -    create_thread("pcifront", pcifront_thread, si);
> +    create_thread("pcifront", pcifront_thread, p);
>  #endif
>  #ifdef CONFIG_XENBUS
> -    create_thread("shutdown", shutdown_thread, si);
> +    create_thread("shutdown", shutdown_thread, p);
>  #endif
>      return 0;
>  }
> -- 
> 2.6.6
> 

-- 
Samuel
Now, it we had this sort of thing:
  yield -a     for yield to all traffic
  yield -t     for yield to trucks
  yield -f     for yield to people walking (yield foot)
  yield -d t*  for yield on days starting with t
...you'd have a lot of dead people at intersections, and traffic jams you
wouldn't believe...
(Discussion in comp.os.linux.misc on the intuitiveness of commands.)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel