Re: git: d8be3d523dd5 - main - vmm: Use struct vcpu in the rendezvous code.

From: FreeBSD User <freebsd_at_walstatt-de.de>
Date: Fri, 18 Nov 2022 18:35:56 UTC
Am Fri, 18 Nov 2022 18:26:44 GMT
John Baldwin <jhb@FreeBSD.org> schrieb:

> The branch main has been updated by jhb:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=d8be3d523dd50a17f48957c1bb2e0cd7bbf02cab
> 
> commit d8be3d523dd50a17f48957c1bb2e0cd7bbf02cab
> Author:     John Baldwin <jhb@FreeBSD.org>
> AuthorDate: 2022-11-18 18:03:34 +0000
> Commit:     John Baldwin <jhb@FreeBSD.org>
> CommitDate: 2022-11-18 18:25:37 +0000
> 
>     vmm: Use struct vcpu in the rendezvous code.
>     
>     Reviewed by:    corvink, markj
>     Differential Revision:  https://reviews.freebsd.org/D37165
> ---
>  sys/amd64/include/vmm.h    |  4 ++--
>  sys/amd64/vmm/io/vioapic.c | 11 +++++------
>  sys/amd64/vmm/io/vlapic.c  | 10 +++++-----
>  sys/amd64/vmm/io/vlapic.h  |  2 +-
>  sys/amd64/vmm/vmm.c        | 36 +++++++++++++++++-------------------
>  5 files changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/sys/amd64/include/vmm.h b/sys/amd64/include/vmm.h
> index 2f9a8776bb39..1c68181f5ff4 100644
> --- a/sys/amd64/include/vmm.h
> +++ b/sys/amd64/include/vmm.h
> @@ -313,8 +313,8 @@ int vm_restore_time(struct vm *vm);
>   * by 'dest' to be stalled. The caller should not rely on any vcpus making
>   * forward progress when the rendezvous is in progress.
>   */
> -typedef void (*vm_rendezvous_func_t)(struct vm *vm, int vcpuid, void *arg);
> -int vm_smp_rendezvous(struct vm *vm, int vcpuid, cpuset_t dest,
> +typedef void (*vm_rendezvous_func_t)(struct vcpu *vcpu, void *arg);
> +int vm_smp_rendezvous(struct vcpu *vcpu, cpuset_t dest,
>      vm_rendezvous_func_t func, void *arg);
>  cpuset_t vm_active_cpus(struct vm *vm);
>  cpuset_t vm_debug_cpus(struct vm *vm);
> diff --git a/sys/amd64/vmm/io/vioapic.c b/sys/amd64/vmm/io/vioapic.c
> index 66a394af0d00..aee58849dd7d 100644
> --- a/sys/amd64/vmm/io/vioapic.c
> +++ b/sys/amd64/vmm/io/vioapic.c
> @@ -237,7 +237,7 @@ vioapic_pulse_irq(struct vm *vm, int irq)
>   * configuration.
>   */
>  static void
> -vioapic_update_tmr(struct vm *vm, int vcpuid, void *arg)
> +vioapic_update_tmr(struct vcpu *vcpu, void *arg)
>  {
>  	struct vioapic *vioapic;
>  	struct vlapic *vlapic;
> @@ -245,8 +245,8 @@ vioapic_update_tmr(struct vm *vm, int vcpuid, void *arg)
>  	int delmode, pin, vector;
>  	bool level, phys;
>  
> -	vlapic = vm_lapic(vm_vcpu(vm, vcpuid));
> -	vioapic = vm_ioapic(vm);
> +	vlapic = vm_lapic(vcpu);
> +	vioapic = vm_ioapic(vcpu_vm(vcpu));
>  
>  	VIOAPIC_LOCK(vioapic);
>  	/*
> @@ -317,10 +317,9 @@ vioapic_write(struct vioapic *vioapic, struct vcpu *vcpu, uint32_t addr,
>  {
>  	uint64_t data64, mask64;
>  	uint64_t last, changed;
> -	int regnum, pin, lshift, vcpuid;
> +	int regnum, pin, lshift;
>  	cpuset_t allvcpus;
>  
> -	vcpuid = vcpu_vcpuid(vcpu);
>  	regnum = addr & 0xff;
>  	switch (regnum) {
>  	case IOAPIC_ID:
> @@ -374,7 +373,7 @@ vioapic_write(struct vioapic *vioapic, struct vcpu *vcpu, uint32_t addr,
>  			    "vlapic trigger-mode register", pin);
>  			VIOAPIC_UNLOCK(vioapic);
>  			allvcpus = vm_active_cpus(vioapic->vm);
> -			(void)vm_smp_rendezvous(vioapic->vm, vcpuid, allvcpus,
> +			(void)vm_smp_rendezvous(vcpu, allvcpus,
>  			    vioapic_update_tmr, NULL);
>  			VIOAPIC_LOCK(vioapic);
>  		}
> diff --git a/sys/amd64/vmm/io/vlapic.c b/sys/amd64/vmm/io/vlapic.c
> index 5075336e54bf..67a09401efb1 100644
> --- a/sys/amd64/vmm/io/vlapic.c
> +++ b/sys/amd64/vmm/io/vlapic.c
> @@ -1200,9 +1200,9 @@ vlapic_icrlo_write_handler(struct vlapic *vlapic, bool *retu)
>  }
>  
>  static void
> -vlapic_handle_init(struct vm *vm, int vcpuid, void *arg)
> +vlapic_handle_init(struct vcpu *vcpu, void *arg)
>  {
> -	struct vlapic *vlapic = vm_lapic(vm_vcpu(vm, vcpuid));
> +	struct vlapic *vlapic = vm_lapic(vcpu);
>  
>  	vlapic_reset(vlapic);
>  
> @@ -1211,13 +1211,13 @@ vlapic_handle_init(struct vm *vm, int vcpuid, void *arg)
>  }
>  
>  int
> -vm_handle_ipi(struct vm *vm, int vcpuid, struct vm_exit *vme, bool *retu)
> +vm_handle_ipi(struct vcpu *vcpu, struct vm_exit *vme, bool *retu)
>  {
>  	*retu = true;
>  	switch (vme->u.ipi.mode) {
>  	case APIC_DELMODE_INIT:
> -		vm_smp_rendezvous(vm, vcpuid, vme->u.ipi.dmask,
> -		    vlapic_handle_init, NULL);
> +		vm_smp_rendezvous(vcpu, vme->u.ipi.dmask, vlapic_handle_init,
> +		    NULL);
>  		break;
>  	case APIC_DELMODE_STARTUP:
>  		break;
> diff --git a/sys/amd64/vmm/io/vlapic.h b/sys/amd64/vmm/io/vlapic.h
> index f8ac42fc7514..f6dd9493a5eb 100644
> --- a/sys/amd64/vmm/io/vlapic.h
> +++ b/sys/amd64/vmm/io/vlapic.h
> @@ -115,6 +115,6 @@ void vlapic_self_ipi_handler(struct vlapic *vlapic, uint64_t val);
>  int vlapic_snapshot(struct vm *vm, struct vm_snapshot_meta *meta);
>  #endif
>  
> -int vm_handle_ipi(struct vm *vm, int vcpuid, struct vm_exit *vme, bool *retu);
> +int vm_handle_ipi(struct vcpu *vcpu, struct vm_exit *vme, bool *retu);
>  
>  #endif	/* _VLAPIC_H_ */
> diff --git a/sys/amd64/vmm/vmm.c b/sys/amd64/vmm/vmm.c
> index cb1a5b62e4f4..f19076f27bb9 100644
> --- a/sys/amd64/vmm/vmm.c
> +++ b/sys/amd64/vmm/vmm.c
> @@ -1320,15 +1320,14 @@ vcpu_require_state_locked(struct vm *vm, int vcpuid, enum vcpu_state
> newstate) }
>  
>  static int
> -vm_handle_rendezvous(struct vm *vm, int vcpuid)
> +vm_handle_rendezvous(struct vcpu *vcpu)
>  {
> +	struct vm *vm = vcpu->vm;
>  	struct thread *td;
> -	int error;
> -
> -	KASSERT(vcpuid >= 0 && vcpuid < vm->maxcpus,
> -	    ("vm_handle_rendezvous: invalid vcpuid %d", vcpuid));
> +	int error, vcpuid;
>  
>  	error = 0;
> +	vcpuid = vcpu->vcpuid;
>  	td = curthread;
>  	mtx_lock(&vm->rendezvous_mtx);
>  	while (vm->rendezvous_func != NULL) {
> @@ -1337,18 +1336,18 @@ vm_handle_rendezvous(struct vm *vm, int vcpuid)
>  
>  		if (CPU_ISSET(vcpuid, &vm->rendezvous_req_cpus) &&
>  		    !CPU_ISSET(vcpuid, &vm->rendezvous_done_cpus)) {
> -			VCPU_CTR0(vm, vcpuid, "Calling rendezvous func");
> -			(*vm->rendezvous_func)(vm, vcpuid, vm->rendezvous_arg);
> +			VMM_CTR0(vcpu, "Calling rendezvous func");
> +			(*vm->rendezvous_func)(vcpu, vm->rendezvous_arg);
>  			CPU_SET(vcpuid, &vm->rendezvous_done_cpus);
>  		}
>  		if (CPU_CMP(&vm->rendezvous_req_cpus,
>  		    &vm->rendezvous_done_cpus) == 0) {
> -			VCPU_CTR0(vm, vcpuid, "Rendezvous completed");
> +			VMM_CTR0(vcpu, "Rendezvous completed");
>  			vm->rendezvous_func = NULL;
>  			wakeup(&vm->rendezvous_func);
>  			break;
>  		}
> -		VCPU_CTR0(vm, vcpuid, "Wait for rendezvous completion");
> +		VMM_CTR0(vcpu, "Wait for rendezvous completion");
>  		mtx_sleep(&vm->rendezvous_func, &vm->rendezvous_mtx, 0,
>  		    "vmrndv", hz);
>  		if (td_ast_pending(td, TDA_SUSPEND)) {
> @@ -1626,7 +1625,7 @@ vm_handle_suspend(struct vm *vm, int vcpuid, bool *retu)
>  		} else {
>  			VCPU_CTR0(vm, vcpuid, "Rendezvous during suspend");
>  			vcpu_unlock(vcpu);
> -			error = vm_handle_rendezvous(vm, vcpuid);
> +			error = vm_handle_rendezvous(vcpu);
>  			vcpu_lock(vcpu);
>  		}
>  	}
> @@ -1818,7 +1817,7 @@ restart:
>  			    vme->u.ioapic_eoi.vector);
>  			break;
>  		case VM_EXITCODE_RENDEZVOUS:
> -			error = vm_handle_rendezvous(vm, vcpuid);
> +			error = vm_handle_rendezvous(vcpu);
>  			break;
>  		case VM_EXITCODE_HLT:
>  			intr_disabled = ((vme->u.hlt.rflags & PSL_I) == 0);
> @@ -1851,7 +1850,7 @@ restart:
>  	 */
>  	if (error == 0 && vme->exitcode == VM_EXITCODE_IPI) {
>  		retu = false;
> -		error = vm_handle_ipi(vm, vcpuid, vme, &retu);
> +		error = vm_handle_ipi(vcpu, vme, &retu);
>  	}
>  
>  	if (error == 0 && retu == false)
> @@ -2564,17 +2563,16 @@ vm_apicid2vcpuid(struct vm *vm, int apicid)
>  }
>  
>  int
> -vm_smp_rendezvous(struct vm *vm, int vcpuid, cpuset_t dest,
> +vm_smp_rendezvous(struct vcpu *vcpu, cpuset_t dest,
>      vm_rendezvous_func_t func, void *arg)
>  {
> +	struct vm *vm = vcpu->vm;
>  	int error, i;
>  
>  	/*
>  	 * Enforce that this function is called without any locks
>  	 */
>  	WITNESS_WARN(WARN_PANIC, NULL, "vm_smp_rendezvous");
> -	KASSERT(vcpuid >= 0 && vcpuid < vm->maxcpus,
> -	    ("vm_smp_rendezvous: invalid vcpuid %d", vcpuid));
>  
>  restart:
>  	mtx_lock(&vm->rendezvous_mtx);
> @@ -2584,9 +2582,9 @@ restart:
>  		 * call the rendezvous handler in case this 'vcpuid' is one
>  		 * of the targets of the rendezvous.
>  		 */
> -		VCPU_CTR0(vm, vcpuid, "Rendezvous already in progress");
> +		VMM_CTR0(vcpu, "Rendezvous already in progress");
>  		mtx_unlock(&vm->rendezvous_mtx);
> -		error = vm_handle_rendezvous(vm, vcpuid);
> +		error = vm_handle_rendezvous(vcpu);
>  		if (error != 0)
>  			return (error);
>  		goto restart;
> @@ -2594,7 +2592,7 @@ restart:
>  	KASSERT(vm->rendezvous_func == NULL, ("vm_smp_rendezvous: previous "
>  	    "rendezvous is still in progress"));
>  
> -	VCPU_CTR0(vm, vcpuid, "Initiating rendezvous");
> +	VMM_CTR0(vcpu, "Initiating rendezvous");
>  	vm->rendezvous_req_cpus = dest;
>  	CPU_ZERO(&vm->rendezvous_done_cpus);
>  	vm->rendezvous_arg = arg;
> @@ -2610,7 +2608,7 @@ restart:
>  			vcpu_notify_event(vm, i, false);
>  	}
>  
> -	return (vm_handle_rendezvous(vm, vcpuid));
> +	return (vm_handle_rendezvous(vcpu));
>  }
>  
>  struct vatpic *
> 

Buildworld fails on this commit:

[...]
/usr/obj/usr/src/amd64.amd64/tmp/usr/include/machine/vmm.h:798:21: warning: declaration of
'struct vcpu' will not be visible outside of this function [-Wvisibility] vm_inject_ud(struct
vcpu *vcpu) ^
/usr/obj/usr/src/amd64.amd64/tmp/usr/include/machine/vmm.h:800:36: error: too few arguments to
function call, expected 5, have 4 vm_inject_fault(vcpu, IDT_UD, 0, 0);
        ~~~~~~~~~~~~~~~                   ^
/usr/obj/usr/src/amd64.amd64/tmp/usr/include/machine/vmm.h:794:6: note: 'vm_inject_fault'
declared here void vm_inject_fault(void *vm, int vcpuid, int vector, int errcode_valid,
     ^
/usr/obj/usr/src/amd64.amd64/tmp/usr/include/machine/vmm.h:804:21: warning: declaration of
'struct vcpu' will not be visible outside of this function [-Wvisibility] vm_inject_gp(struct
vcpu *vcpu) ^
/usr/obj/usr/src/amd64.amd64/tmp/usr/include/machine/vmm.h:806:36: error: too few arguments to
function call, expected 5, have 4 vm_inject_fault(vcpu, IDT_GP, 1, 0);



Kind regards

O. Hartmann
-- 
O. Hartmann