Re: [Xen-devel] [PATCH RFC v1 44/74] xen/pvshim: keep track of unused pages

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 14:05, <wei.liu2@xxxxxxxxxx> wrote:
> Simple infrastructure to keep track of allocate and free unused pages,
> so that we can use them to map special pages like shared info and
> grant table.
> 
> As rangeset depends on malloc being ready we introduce
> hypervisor_setup for things that can be initialised late in the
> process.
> 
> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  xen/arch/x86/guest/xen.c        | 48 
> +++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/setup.c            |  3 +++
>  xen/include/asm-x86/guest/xen.h | 22 +++++++++++++++++++
>  3 files changed, 73 insertions(+)
> 
> diff --git a/xen/arch/x86/guest/xen.c b/xen/arch/x86/guest/xen.c
> index 0319a5f9e8..f66c10fbe5 100644
> --- a/xen/arch/x86/guest/xen.c
> +++ b/xen/arch/x86/guest/xen.c
> @@ -21,6 +21,7 @@
>  #include <xen/init.h>
>  #include <xen/types.h>
>  #include <xen/pv_console.h>
> +#include <xen/rangeset.h>
>  
>  #include <asm/apic.h>
>  #include <asm/guest.h>
> @@ -34,6 +35,7 @@ bool xen_guest;
>  static uint32_t xen_cpuid_base;
>  static uint8_t evtchn_upcall_vector;
>  extern char hypercall_page[];
> +static struct rangeset *mem;
>  
>  static void __init find_xen_leaves(void)
>  {
> @@ -161,9 +163,38 @@ static void __init init_evtchn(void)
>      ap_setup_event_channels(true);
>  }
>  
> +static void __init init_memmap(void)
> +{
> +    unsigned int i;
> +
> +    mem = rangeset_new(NULL, "host memory map", 0);
> +    if ( !mem )
> +        panic("failed to allocate host memory rangeset");

"host" is meant from the perspective of the shim on itself here aiui,
not the underlying entity? I find using that term here at least
misleading.

> +    /* Mark up to the last memory page (or 4GB) as RAM. */
> +    if ( rangeset_add_range(mem, 0, max_t(unsigned long, max_page,
> +                                          (GB(4) - 1) >> PAGE_SHIFT)) )

Don't you also need "max_page - 1" then? Also - why the
saturation to 4Gb?

> +        panic("unable to add RAM to memory rangeset");
> +
> +    for ( i = 0; i < e820.nr_map; i++ )
> +    {
> +        struct e820entry *e = &e820.map[i];
> +
> +        if ( rangeset_add_range(mem, e->addr >> PAGE_SHIFT,
> +                                (e->addr + e->size) >> PAGE_SHIFT) )

PFN_DOWN() and PFN_UP() respectively. Plus aren't rangeset
ranges exclusive of their upper ends, making it necessary to
subtract 1 from the upper bound?

> @@ -43,11 +46,30 @@ static inline void hypervisor_early_setup(struct e820map 
> *e820)
>  {
>      ASSERT_UNREACHABLE();
>  };
> +
> +static inline void hypervisor_setup(void)
> +{
> +    ASSERT_UNREACHABLE();
> +}
> +
>  static inline void hypervisor_ap_setup(void)
>  {
>      ASSERT_UNREACHABLE();
>  };
>  
> +static inline int hypervisor_alloc_unused_page(mfn_t *mfn)
> +{
> +
> +    ASSERT_UNREACHABLE();
> +    return 0;
> +}
> +
> +static inline int hypervisor_free_unused_page(mfn_t mfn)
> +{
> +    ASSERT_UNREACHABLE();
> +    return 0;
> +}

I can see the value of hypervisor_setup() stub, but are the other
two really going to be needed, i.e. are any such allocations being
placed into not shim specific code (doesn't seem very likely)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel