[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 2/4] Add NVMM accelerator: x86 CPU support
From: |
Reinoud Zandijk |
Subject: |
Re: [PATCH v6 2/4] Add NVMM accelerator: x86 CPU support |
Date: |
Thu, 1 Apr 2021 14:32:37 +0200 |
On Thu, Apr 01, 2021 at 10:35:40AM +0200, Paolo Bonzini wrote:
> On 31/03/21 22:07, Reinoud Zandijk wrote:
> > +void nvmm_vcpu_kick(CPUState *cpu);
>
> Not defined anywhere.
Hmmm, indeed. I think its a leftover of the former patch. Good catch.
> > +{
> > +#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.
Are you worried the in-kernel thread will somehow get stuck or halt on exit of
Qemu and left as a zombie?
> Also, can you just drop support for NVMM_USER_VERSION == 1?
Now thats a good suggestion. We could add support for it in the pkgsrc
package. When 9.0 gets retired, we could then retire it there without the need
to patch Qemu again.
> > 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.
I am not sure what you mean by that. You provided a patch for the meson.build
file, will that not suffice?
With regards,
Reinoud
signature.asc
Description: PGP signature