Re: [PATCH v6 2/4] Add NVMM accelerator: x86 CPU support

From: Paolo Bonzini <pbonzini@redhat.com>
To: Reinoud Zandijk <reinoud@NetBSD.org>, qemu-devel@nongnu.org
Cc: Kamil Rytarowski <kamil@NetBSD.org>,
	Ryo ONODERA <ryoon@netbsd.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [PATCH v6 2/4] Add NVMM accelerator: x86 CPU support
Date: Thu, 1 Apr 2021 10:35:40 +0200	[thread overview]
Message-ID: <5afd10b1-bd32-2f06-b311-246815428bfc@redhat.com> (raw)
In-Reply-To: <20210331200800.24168-3-reinoud@NetBSD.org>

On 31/03/21 22:07, Reinoud Zandijk wrote:
> +void nvmm_vcpu_kick(CPUState *cpu);

Not defined anywhere.

> +{
> +#if NVMM_USER_VERSION == 1
> +    struct sigaction sigact;
> +    sigset_t set;
> +
> +    /* Install the IPI handler. */
> +    memset(&sigact, 0, sizeof(sigact));
> +    sigact.sa_handler = nvmm_ipi_signal;
> +    sigaction(SIG_IPI, &sigact, NULL);
> +
> +    /* Allow IPIs on the current thread. */
> +    sigprocmask(SIG_BLOCK, NULL, &set);
> +    sigdelset(&set, SIG_IPI);
> +    pthread_sigmask(SIG_SETMASK, &set, NULL);
> +#else
> +    /*
> +     * We use the nvmm_vcpu_stop() mechanism, and don't use signals.
> +     * Nothing to do.
> +     */
> +#endif

Since nvmm_vcpu_stop is very similar to KVM's immediate_exit mechanism, 
I think you still need to have a dummy signal handler to kick the VM out 
of the run loop *if it is in the kernel*.  The signal handler however 
can just do nothing.

Also, can you just drop support for NVMM_USER_VERSION == 1?

> diff --git a/target/i386/nvmm/meson.build b/target/i386/nvmm/meson.build
> new file mode 100644
> index 0000000000..c154e78014
> --- /dev/null
> +++ b/target/i386/nvmm/meson.build
> @@ -0,0 +1,4 @@
> +i386_softmmu_ss.add(when: 'CONFIG_NVMM', if_true: files(
> +  'nvmm-all.c',
> +  'nvmm-accel-ops.c',
> +))

The nvmm library should be added here.

Paolo