Re: [PATCH] Update mach_debug interfaces to use struct.

[Samuel Thibault]

Applied, thanks!

Flavio Cruz, le mar. 22 nov. 2022 23:46:31 -0500, a ecrit:
> The new interfaces will be compatible both with a 64 bits kernel and
> userland and 64 bits kernel and 32 bit userland. Also removed many
> of the uses of natural_t where an unsigned int suffices. Ideally we
> should replace natural_t with something more portable such as uintptr_t
> or a basic int type for counters since for the most part we don't
> need counters to have the same width as the pointer type.
> 
> * i386/include/mach/i386/vm_types.h: Define rpc_unsigned_long.
> * include/mach/mach_types.defs: Define type rpc_vm_offset_t.
> * include/mach_debug/hash_info.h: Use unsigned int instead of natural_t.
> * include/mach_debug/mach_debug_types.defs: Update cache_info_t,
>   hash_info_bucket_t, vm_region_info_t, vm_object_info_t and
>   vm_page_info_t to use struct.
> * include/mach_debug/slab_info.h: Use rpc_vm_size_t and
>   rpc_unsigned_long for compatibility with 32 bits userland.
> * include/mach_debug/vm_info.h: Update struct fields to use the correct
>   types and avoid natural_t whenever we just want to keep a count of
>   something.
> ---
>  i386/include/mach/i386/machine_types.defs | 14 +++++
>  i386/include/mach/i386/vm_types.h         |  2 +
>  include/mach/mach_types.defs              |  4 +-
>  include/mach_debug/hash_info.h            |  2 +-
>  include/mach_debug/mach_debug_types.defs  | 65 +++++++++++++++++++++--
>  include/mach_debug/slab_info.h            | 20 +++----
>  include/mach_debug/vm_info.h              | 38 ++++++-------
>  7 files changed, 109 insertions(+), 36 deletions(-)
> 
> diff --git a/i386/include/mach/i386/machine_types.defs 
> b/i386/include/mach/i386/machine_types.defs
> index dfbc521e..0e94999b 100755
> --- a/i386/include/mach/i386/machine_types.defs
> +++ b/i386/include/mach/i386/machine_types.defs
> @@ -58,6 +58,20 @@ type natural_t = uint32_t;
>   */
>  type integer_t = int32_t;
>  
> +/*
> + * unsigned long for kernel <-> userland interfaces size depends on the 
> architecture
> + * of both kernel and userland.
> + */
> +#if defined(KERNEL) && defined(USER32)
> +type rpc_unsigned_long = uint32_t;
> +#else /* KERNEL and USER32 */
> +#if defined(__x86_64__)
> +type rpc_unsigned_long = uint64_t;
> +#else /* __x86_64__ */
> +type rpc_unsigned_long = uint32_t;
> +#endif /* __x86_64__ */
> +#endif /* KERNEL_SERVER and USER32 */
> +
>  /*
>   * Physical address size
>   */
> diff --git a/i386/include/mach/i386/vm_types.h 
> b/i386/include/mach/i386/vm_types.h
> index 7a43d75d..9daaa15e 100644
> --- a/i386/include/mach/i386/vm_types.h
> +++ b/i386/include/mach/i386/vm_types.h
> @@ -116,12 +116,14 @@ static inline __mach_uint32_t 
> convert_vm_to_user(__mach_uint64_t kaddr)
>      assert(kaddr <= 0xFFFFFFFF);
>      return (__mach_uint32_t)kaddr;
>  }
> +typedef __mach_uint32_t rpc_unsigned_long;
>  #else /* MACH_KERNEL */
>  typedef vm_offset_t  rpc_vm_address_t;
>  typedef vm_offset_t  rpc_vm_offset_t;
>  typedef vm_size_t    rpc_vm_size_t;
>  #define convert_vm_to_user null_conversion
>  #define convert_vm_from_user null_conversion
> +typedef unsigned long rpc_unsigned_long;
>  #endif /* MACH_KERNEL */
>  
>  #endif       /* __ASSEMBLER__ */
> diff --git a/include/mach/mach_types.defs b/include/mach/mach_types.defs
> index 4412d726..f7106946 100644
> --- a/include/mach/mach_types.defs
> +++ b/include/mach/mach_types.defs
> @@ -120,6 +120,8 @@ type rpc_vm_size_t = uint32_t;
>  #endif /* __x86_64__ */
>  #endif /* KERNEL_SERVER and USER32 */
>  
> +type rpc_vm_offset_t = rpc_vm_size_t;
> +
>  type vm_address_t = rpc_vm_size_t
>  #if defined(KERNEL_SERVER)
>      intran: vm_address_t convert_vm_from_user(rpc_vm_address_t)
> @@ -128,7 +130,7 @@ type vm_address_t = rpc_vm_size_t
>      ctype: rpc_vm_address_t
>  #endif
>      ;
> -type vm_offset_t = rpc_vm_size_t
> +type vm_offset_t = rpc_vm_offset_t
>  #if defined(KERNEL_SERVER)
>      intran: vm_offset_t convert_vm_from_user(rpc_vm_offset_t)
>      outtran: rpc_vm_offset_t convert_vm_to_user(vm_offset_t)
> diff --git a/include/mach_debug/hash_info.h b/include/mach_debug/hash_info.h
> index 6944277d..8e6f19cf 100644
> --- a/include/mach_debug/hash_info.h
> +++ b/include/mach_debug/hash_info.h
> @@ -33,7 +33,7 @@
>   */
>  
>  typedef struct hash_info_bucket {
> -     natural_t       hib_count;      /* number of records in bucket */
> +     unsigned int hib_count; /* number of records in bucket */
>  } hash_info_bucket_t;
>  
>  typedef hash_info_bucket_t *hash_info_bucket_array_t;
> diff --git a/include/mach_debug/mach_debug_types.defs 
> b/include/mach_debug/mach_debug_types.defs
> index 23c2026d..c138dc40 100644
> --- a/include/mach_debug/mach_debug_types.defs
> +++ b/include/mach_debug/mach_debug_types.defs
> @@ -32,19 +32,74 @@
>  
>  #include <mach/std_types.defs>
>  
> -type cache_info_t = struct[19] of integer_t;
> +#define CACHE_NAME_MAX_LEN 32
> +type cache_name_t = struct[CACHE_NAME_MAX_LEN] of char;
> +#undef CACHE_NAME_MAX_LEN
> +type cache_info_t = struct {
> +   integer_t flags;
> +   rpc_vm_size_t cpu_pool_size;
> +   rpc_vm_size_t obj_size;
> +   rpc_vm_size_t align;
> +   rpc_vm_size_t buf_size;
> +   rpc_vm_size_t slab_size;
> +   rpc_unsigned_long bufs_per_slab;
> +   rpc_unsigned_long nr_objs;
> +   rpc_unsigned_long nr_bufs;
> +   rpc_unsigned_long nr_slabs;
> +   rpc_unsigned_long nr_free_slabs;
> +   cache_name_t name;
> +};
>  type cache_info_array_t = array[] of cache_info_t;
>  
> -type hash_info_bucket_t = struct[1] of natural_t;
> +type hash_info_bucket_t = struct {
> +   unsigned hib_count;
> +};
>  type hash_info_bucket_array_t = array[] of hash_info_bucket_t;
>  
> -type vm_region_info_t = struct[11] of natural_t;
> +type vm_region_info_t = struct {
> +   rpc_vm_offset_t vri_start;
> +   rpc_vm_offset_t vri_end;
> +   vm_prot_t vri_protection;
> +   vm_prot_t vri_max_protection;
> +   vm_inherit_t vri_inheritance;
> +   unsigned vri_wired_count;
> +   unsigned vri_user_wired_count;
> +   rpc_vm_offset_t vri_object;
> +   rpc_vm_offset_t vri_offset;
> +   integer_t vri_needs_copy;
> +   unsigned vri_sharing;
> +};
>  type vm_region_info_array_t = array[] of vm_region_info_t;
>  
> -type vm_object_info_t = struct[14] of natural_t;
> +type vm_object_info_state_t = uint32_t;
> +type vm_object_info_t = struct {
> +    rpc_vm_offset_t voi_object;
> +    rpc_vm_size_t voi_pagesize;
> +    rpc_vm_size_t voi_size;
> +    unsigned voi_ref_count;
> +    unsigned voi_resident_page_count;
> +    unsigned voi_absent_count;
> +    rpc_vm_offset_t voi_copy;
> +    rpc_vm_offset_t voi_shadow;
> +    rpc_vm_offset_t voi_shadow_offset;
> +    rpc_vm_offset_t voi_paging_offset;
> +    memory_object_copy_strategy_t voi_copy_strategy;
> +    rpc_vm_offset_t voi_last_alloc;
> +    unsigned voi_paging_in_progress;
> +    vm_object_info_state_t voi_state;
> +};
>  type vm_object_info_array_t = array[] of vm_object_info_t;
>  
> -type vm_page_info_t = struct[6] of natural_t;
> +type vm_page_info_state_t = uint32_t;
> +
> +type vm_page_info_t = struct {
> +   rpc_vm_offset_t vpi_offset;
> +   rpc_vm_offset_t vpi_phys_addr;
> +   unsigned vpi_wire_count;
> +   vm_prot_t vpi_page_lock;
> +   vm_prot_t vpi_unlock_request;
> +   vm_page_info_state_t vpi_state;
> +};
>  type vm_page_info_array_t = array[] of vm_page_info_t;
>  
>  type symtab_name_t = (MACH_MSG_TYPE_STRING_C, 8*32);
> diff --git a/include/mach_debug/slab_info.h b/include/mach_debug/slab_info.h
> index 7d12cc18..19a87307 100644
> --- a/include/mach_debug/slab_info.h
> +++ b/include/mach_debug/slab_info.h
> @@ -38,16 +38,16 @@
>  
>  typedef struct cache_info {
>       int flags;
> -     size_t cpu_pool_size;
> -     size_t obj_size;
> -     size_t align;
> -     size_t buf_size;
> -     size_t slab_size;
> -     unsigned long bufs_per_slab;
> -     unsigned long nr_objs;
> -     unsigned long nr_bufs;
> -     unsigned long nr_slabs;
> -     unsigned long nr_free_slabs;
> +     rpc_vm_size_t cpu_pool_size;
> +     rpc_vm_size_t obj_size;
> +     rpc_vm_size_t align;
> +     rpc_vm_size_t buf_size;
> +     rpc_vm_size_t slab_size;
> +     rpc_unsigned_long bufs_per_slab;
> +     rpc_unsigned_long nr_objs;
> +     rpc_unsigned_long nr_bufs;
> +     rpc_unsigned_long nr_slabs;
> +     rpc_unsigned_long nr_free_slabs;
>       char name[CACHE_NAME_MAX_LEN];
>  } cache_info_t;
>  
> diff --git a/include/mach_debug/vm_info.h b/include/mach_debug/vm_info.h
> index e68bb1d5..2c1b019f 100644
> --- a/include/mach_debug/vm_info.h
> +++ b/include/mach_debug/vm_info.h
> @@ -39,6 +39,7 @@
>  #include <mach/vm_inherit.h>
>  #include <mach/vm_prot.h>
>  #include <mach/memory_object.h>
> +#include <stdint.h>
>  
>  /*
>   *   Remember to update the mig type definitions
> @@ -49,22 +50,22 @@ typedef struct vm_region_info {
>       rpc_vm_offset_t vri_start;              /* start of region */
>       rpc_vm_offset_t vri_end;                /* end of region */
>  
> -/*vm_prot_t*/natural_t vri_protection;       /* protection code */
> -/*vm_prot_t*/natural_t vri_max_protection;   /* maximum protection */
> -/*vm_inherit_t*/natural_t vri_inheritance;   /* inheritance */
> -     natural_t vri_wired_count;      /* number of times wired */
> -     natural_t vri_user_wired_count; /* number of times user has wired */
> +     vm_prot_t vri_protection;       /* protection code */
> +     vm_prot_t vri_max_protection;   /* maximum protection */
> +     vm_inherit_t vri_inheritance;   /* inheritance */
> +     unsigned int vri_wired_count;   /* number of times wired */
> +     unsigned int vri_user_wired_count; /* number of times user has wired */
>  
>       rpc_vm_offset_t vri_object;             /* the mapped object */
>       rpc_vm_offset_t vri_offset;             /* offset into object */
>  /*boolean_t*/integer_t vri_needs_copy;       /* does object need to be 
> copied? */
> -     natural_t vri_sharing;  /* share map references */
> +     unsigned int vri_sharing;       /* share map references */
>  } vm_region_info_t;
>  
>  typedef vm_region_info_t *vm_region_info_array_t;
>  
>  
> -typedef natural_t vm_object_info_state_t;
> +typedef uint32_t vm_object_info_state_t;
>  
>  #define VOI_STATE_PAGER_CREATED              0x00000001
>  #define VOI_STATE_PAGER_INITIALIZED  0x00000002
> @@ -80,24 +81,23 @@ typedef struct vm_object_info {
>       rpc_vm_offset_t voi_object;             /* this object */
>       rpc_vm_size_t voi_pagesize;             /* object's page size */
>       rpc_vm_size_t voi_size;         /* object size (valid if internal) */
> -     natural_t voi_ref_count;        /* number of references */
> -     natural_t voi_resident_page_count; /* number of resident pages */
> -     natural_t voi_absent_count;     /* number requested but not filled */
> +     unsigned int voi_ref_count;     /* number of references */
> +     unsigned int voi_resident_page_count; /* number of resident pages */
> +     unsigned int voi_absent_count;  /* number requested but not filled */
>       rpc_vm_offset_t voi_copy;               /* copy object */
>       rpc_vm_offset_t voi_shadow;             /* shadow object */
>       rpc_vm_offset_t voi_shadow_offset;      /* offset into shadow object */
>       rpc_vm_offset_t voi_paging_offset;      /* offset into memory object */
> -/*memory_object_copy_strategy_t*/integer_t voi_copy_strategy;
> +     memory_object_copy_strategy_t voi_copy_strategy;
>                                       /* how to handle data copy */
>       rpc_vm_offset_t voi_last_alloc; /* offset of last allocation */
> -     natural_t voi_paging_in_progress; /* paging references */
> +     unsigned int voi_paging_in_progress; /* paging references */
>       vm_object_info_state_t voi_state; /* random state bits */
>  } vm_object_info_t;
>  
>  typedef vm_object_info_t *vm_object_info_array_t;
>  
> -
> -typedef natural_t vm_page_info_state_t;
> +typedef uint32_t vm_page_info_state_t;
>  
>  #define VPI_STATE_BUSY               0x00000001
>  #define VPI_STATE_WANTED     0x00000002
> @@ -118,11 +118,11 @@ typedef natural_t vm_page_info_state_t;
>  #define VPI_STATE_PAGER              0x80000000      /* pager has the page */
>  
>  typedef struct vm_page_info {
> -     vm_offset_t vpi_offset;         /* offset in object */
> -     vm_offset_t vpi_phys_addr;      /* physical address */
> -     natural_t vpi_wire_count;       /* number of times wired */
> -/*vm_prot_t*/natural_t vpi_page_lock;        /* XP access restrictions */
> -/*vm_prot_t*/natural_t vpi_unlock_request;   /* outstanding unlock requests 
> */
> +     rpc_vm_offset_t vpi_offset;     /* offset in object */
> +     rpc_vm_offset_t vpi_phys_addr;  /* physical address */
> +     unsigned int vpi_wire_count;    /* number of times wired */
> +     vm_prot_t vpi_page_lock;        /* XP access restrictions */
> +     vm_prot_t vpi_unlock_request;   /* outstanding unlock requests */
>       vm_page_info_state_t vpi_state; /* random state bits */
>  } vm_page_info_t;
>  
> -- 
> 2.37.2
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.