Re: [PATCH v7 07/19] xen: implement hook to fetch e820 memory map

This also looks fine, with the style nit noted in the patch.

On Thu, Dec 19, 2013 at 07:54:44PM +0100, Roger Pau Monne wrote:
> ---
>  sys/amd64/amd64/machdep.c   |   50 ++++++++++++++++++++++++++----------------
>  sys/amd64/include/pc/bios.h |    2 +
>  sys/amd64/include/sysarch.h |    1 +
>  sys/x86/xen/pv.c            |   26 ++++++++++++++++++++++
>  4 files changed, 60 insertions(+), 19 deletions(-)
> 
> diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c
> index a2dcb90..6bbfe5a 100644
> --- a/sys/amd64/amd64/machdep.c
> +++ b/sys/amd64/amd64/machdep.c
> _at__at_ -177,11 +177,15 _at__at_ SYSINIT(cpu, SI_SUB_CPU, SI_ORDER_FIRST, cpu_startup, NULL);
>  /* Preload data parse function */
>  static caddr_t native_parse_preload_data(u_int64_t);
>  
> +/* Native function to fetch and parse the e820 map */
> +static void native_parse_memmap(caddr_t, vm_paddr_t *, int *);
> +
>  /* Default init_ops implementation. */
>  struct init_ops init_ops = {
>  	.parse_preload_data =	native_parse_preload_data,
>  	.early_delay_init =	i8254_init,
>  	.early_delay =		i8254_delay,
> +	.parse_memmap =		native_parse_memmap,
>  };
>  
>  /*
> _at__at_ -1418,21 +1422,12 _at__at_ add_physmap_entry(uint64_t base, uint64_t length, vm_paddr_t *physmap,
>  	return (1);
>  }
>  
> -static void
> -add_smap_entries(struct bios_smap *smapbase, vm_paddr_t *physmap,
> -    int *physmap_idx)
> +void
> +bios_add_smap_entries(struct bios_smap *smapbase, u_int32_t smapsize,
> +                      vm_paddr_t *physmap, int *physmap_idx)
>  {
>  	struct bios_smap *smap, *smapend;
> -	u_int32_t smapsize;
>  
> -	/*
> -	 * Memory map from INT 15:E820.
> -	 *
> -	 * subr_module.c says:
> -	 * "Consumer may safely assume that size value precedes data."
> -	 * ie: an int32_t immediately precedes smap.
> -	 */
> -	smapsize = *((u_int32_t *)smapbase - 1);
>  	smapend = (struct bios_smap *)((uintptr_t)smapbase + smapsize);
>  
>  	for (smap = smapbase; smap < smapend; smap++) {
> _at__at_ -1449,6 +1444,29 _at__at_ add_smap_entries(struct bios_smap *smapbase, vm_paddr_t *physmap,
>  	}
>  }
>  
> +static void
> +native_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap, int *physmap_idx)
> +{
> +	struct bios_smap *smap;
> +	u_int32_t size;
> +
> +	/*
> +	 * Memory map from INT 15:E820.
> +	 *
> +	 * subr_module.c says:
> +	 * "Consumer may safely assume that size value precedes data."
> +	 * ie: an int32_t immediately precedes smap.
> +	 */
> +
> +	smap = (struct bios_smap *)preload_search_info(kmdp,
> +	    MODINFO_METADATA | MODINFOMD_SMAP);
> +	if (smap == NULL)
> +		panic("No BIOS smap info from loader!");
> +	size = *((u_int32_t *)smap - 1);
> +
> +	bios_add_smap_entries(smap, size, physmap, physmap_idx);
> +}
> +
>  /*
>   * Populate the (physmap) array with base/bound pairs describing the
>   * available physical memory in the system, then test this memory and
> _at__at_ -1466,19 +1484,13 _at__at_ getmemsize(caddr_t kmdp, u_int64_t first)
>  	vm_paddr_t pa, physmap[PHYSMAP_SIZE];
>  	u_long physmem_start, physmem_tunable, memtest;
>  	pt_entry_t *pte;
> -	struct bios_smap *smapbase;
>  	quad_t dcons_addr, dcons_size;
>  
>  	bzero(physmap, sizeof(physmap));
>  	basemem = 0;
>  	physmap_idx = 0;
>  
> -	smapbase = (struct bios_smap *)preload_search_info(kmdp,
> -	    MODINFO_METADATA | MODINFOMD_SMAP);
> -	if (smapbase == NULL)
> -		panic("No BIOS smap info from loader!");
> -
> -	add_smap_entries(smapbase, physmap, &physmap_idx);
> +	init_ops.parse_memmap(kmdp, physmap, &physmap_idx);
>  
>  	/*
>  	 * Find the 'base memory' segment for SMP
> diff --git a/sys/amd64/include/pc/bios.h b/sys/amd64/include/pc/bios.h
> index e7d568e..92d4265 100644
> --- a/sys/amd64/include/pc/bios.h
> +++ b/sys/amd64/include/pc/bios.h
> _at__at_ -106,6 +106,8 _at__at_ struct bios_oem {
>  int	bios_oem_strings(struct bios_oem *oem, u_char *buffer, size_t maxlen);
>  uint32_t bios_sigsearch(uint32_t start, u_char *sig, int siglen, int paralen,
>  	    int sigofs);
> +void bios_add_smap_entries(struct bios_smap *smapbase, u_int32_t smapsize,
> +                           vm_paddr_t *physmap, int *physmap_idx);
This uses the GNU formatting conventions, please indent the continuation
line same as the previous line.