Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: vmm(4)/amd64: reduce use of extern
To:
Hans-Jörg Höxer <hshoexer@genua.de>
Cc:
tech@openbsd.org, Hans-Joerg_Hoexer@genua.de
Date:
Mon, 28 Apr 2025 13:20:15 +0200

Download raw body.

Thread
> Date: Mon, 28 Apr 2025 12:57:18 +0200
> From: Hans-Jörg Höxer <hshoexer@genua.de>
> 
> Hi,
> 
> this diff reduces the use of "extern" in amd64/vmm_machdep.c and related
> files.  There might be some more low hanging fruits in eg. pmap.c.

The problem is that <machine/cpu.h> ends up getting included by a lot
of files (even from userland).  So there is a big risk of namespace
pollution.  This is somewhat mitigated by #ifdef _KERNEL, but some of
the userland code in base does do a #define _KERNEL.  It is also
mitigated by using a cpu_ prefix for the symbol names.

> 
> Take care,
> HJ.
> ---------------------------------------------------------------------
> commit 16aef1f12194538826704172daf34311931778d5
> Author: Hans-Joerg Hoexer <hshoexer@genua.de>
> Date:   Mon Apr 28 11:49:20 2025 +0200
> 
>     kern: cleanup vmm(4)/amd64 related use of "extern"
> 
> diff --git a/sys/arch/amd64/amd64/identcpu.c b/sys/arch/amd64/amd64/identcpu.c
> index 1a9a99e9bbb..78d88b3983b 100644
> --- a/sys/arch/amd64/amd64/identcpu.c
> +++ b/sys/arch/amd64/amd64/identcpu.c
> @@ -147,7 +147,6 @@ intelcore_update_sensor(void *args)
>  void
>  cpu_hz_update_sensor(void *args)
>  {
> -	extern uint64_t	 tsc_frequency;
>  	struct cpu_info	*ci = args;
>  	uint64_t	 mperf, aperf, mdelta, adelta, val;
>  	unsigned long	 s;
> diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c
> index 7f85f92cb88..8b909d0304d 100644
> --- a/sys/arch/amd64/amd64/machdep.c
> +++ b/sys/arch/amd64/amd64/machdep.c
> @@ -483,7 +483,6 @@ bios_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
>  	/* NOTREACHED */
>  }
>  
> -extern int tsc_is_invariant;
>  extern int amd64_has_xcrypt;
>  extern int need_retpoline;
>  
> @@ -504,7 +503,6 @@ int
>  cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
>      size_t newlen, struct proc *p)
>  {
> -	extern uint64_t tsc_frequency;
>  	dev_t consdev;
>  	dev_t dev;
>  
> diff --git a/sys/arch/amd64/amd64/pmap.c b/sys/arch/amd64/amd64/pmap.c
> index bfd667f99bf..278330495bb 100644
> --- a/sys/arch/amd64/amd64/pmap.c
> +++ b/sys/arch/amd64/amd64/pmap.c
> @@ -307,9 +307,6 @@ void pmap_pdp_ctor_intel(pd_entry_t *);
>  extern vaddr_t msgbuf_vaddr;
>  extern paddr_t msgbuf_paddr;
>  
> -extern vaddr_t idt_vaddr;			/* we allocate IDT early */
> -extern paddr_t idt_paddr;
> -
>  extern vaddr_t lo32_vaddr;
>  extern vaddr_t lo32_paddr;
>  
> diff --git a/sys/arch/amd64/amd64/vmm_machdep.c b/sys/arch/amd64/amd64/vmm_machdep.c
> index 9b5d56da27b..8bd747ef476 100644
> --- a/sys/arch/amd64/amd64/vmm_machdep.c
> +++ b/sys/arch/amd64/amd64/vmm_machdep.c
> @@ -181,9 +181,6 @@ struct vmm_reg_debug_info {
>  };
>  #endif /* VMM_DEBUG */
>  
> -extern uint64_t tsc_frequency;
> -extern int tsc_is_invariant;
> -
>  const char *vmm_hv_signature = VMM_HV_SIGNATURE;
>  
>  const struct kmem_pa_mode vmm_kp_contig = {
> @@ -193,9 +190,6 @@ const struct kmem_pa_mode vmm_kp_contig = {
>  	.kp_zero = 1,
>  };
>  
> -extern struct cfdriver vmm_cd;
> -extern const struct cfattach vmm_ca;
> -
>  /*
>   * Helper struct to easily get the VMCS field IDs needed in vmread/vmwrite
>   * to access the individual fields of the guest segment registers. This
> @@ -225,16 +219,6 @@ const struct {
>  	  VMCS_GUEST_IA32_TR_AR, VMCS_GUEST_IA32_TR_BASE }
>  };
>  
> -/* Pools for VMs and VCPUs */
> -extern struct pool vm_pool;
> -extern struct pool vcpu_pool;
> -
> -extern struct vmm_softc *vmm_softc;
> -
> -/* IDT information used when populating host state area */
> -extern vaddr_t idt_vaddr;
> -extern struct gate_descriptor *idt;
> -
>  /* Constants used in "CR access exit" */
>  #define CR_WRITE	0
>  #define CR_READ		1
> diff --git a/sys/arch/amd64/include/cpu.h b/sys/arch/amd64/include/cpu.h
> index 77ad7bf0f34..07b3ad1df35 100644
> --- a/sys/arch/amd64/include/cpu.h
> +++ b/sys/arch/amd64/include/cpu.h
> @@ -419,6 +419,10 @@ extern int amd64_pos_cbit;
>  extern int amd64_min_noes_asid;
>  
>  /* machdep.c */
> +extern vaddr_t idt_vaddr;
> +extern paddr_t idt_paddr;
> +extern struct gate_descriptor *idt;
> +
>  void	dumpconf(void);
>  void	cpu_set_vendor(struct cpu_info *, int _level, const char *_vendor);
>  void	cpu_reset(void);
> @@ -476,6 +480,10 @@ void k1x_setperf(int);
>  void est_init(struct cpu_info *);
>  void est_setperf(int);
>  
> +/* tsc.c */
> +extern uint64_t tsc_frequency;
> +extern int tsc_is_invariant;
> +
>  #ifdef MULTIPROCESSOR
>  /* mp_setperf.c */
>  void mp_setperf_init(void);
> diff --git a/sys/dev/vmm/vmm.h b/sys/dev/vmm/vmm.h
> index 00cf820c9f1..f27433b9e1b 100644
> --- a/sys/dev/vmm/vmm.h
> +++ b/sys/dev/vmm/vmm.h
> @@ -228,6 +228,12 @@ struct vmm_softc {
>  	uint8_t			vpids[512];	/* [p] bitmap of VPID/ASIDs */
>  };
>  
> +extern struct vmm_softc *vmm_softc;
> +extern struct pool vm_pool;
> +extern struct pool vcpu_pool;
> +extern struct cfdriver vmm_cd;
> +extern const struct cfattach vmm_ca;
> +
>  int vmm_probe(struct device *, void *, void *);
>  int vmm_activate(struct device *, int);
>  void vmm_attach(struct device *, struct device *,  void *);
> 
> [2:application/pkcs7-signature Show Save:smime.p7s (5kB)]
>