[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 2/3] xen/oprofile: use NMI continuation for sending virq to guest



On 12.11.20 11:23, Jan Beulich wrote:
On 11.11.2020 16:48, Jürgen Groß wrote:
On 11.11.20 16:45, Jan Beulich wrote:
On 09.11.2020 10:50, Juergen Gross wrote:
@@ -83,14 +85,28 @@ void passive_domain_destroy(struct vcpu *v)
                model->free_msr(v);
   }
+bool nmi_oprofile_send_virq(void)
+{
+       struct vcpu *v = this_cpu(nmi_cont_vcpu);
+
+       if ( v )
+               send_guest_vcpu_virq(v, VIRQ_XENOPROF);
+
+       this_cpu(nmi_cont_vcpu) = NULL;

What if, by the time we make it here, a 2nd NMI has arrived? I
agree the next overflow interrupt shouldn't arrive this
quickly, but I also think you want to zap the per-CPU variable
first here, and ...

How could that happen? This function is activated only from NMI
context in case the NMI happened in guest mode. And it will be
executed with higher priority than any guest, so there is a zero
chance another NMI in guest mode can happen in between.

While I'll admit I didn't pay attention to the bogus (as far as
HVM is concerned) xen_mode check, my understanding is that the
self-IPI will be delivered once we're back in guest mode, as
that's the first time IRQs would be on again (even event checking
gets deferred by sending a self-IPI). If another NMI was latched
by that time, it would take precedence over the IRQ and would
also be delivered on the guest mode insn that the IRET returned
to.

I agree though that this is benign, as the vCPU wouldn't have
been context switched out yet, i.e. current is still the same
and there'll then merely be two NMI instances folded into one.

Correct.


However, I still think the ordering would better be changed, to
set a good precedent.

Okay, if you want that.


   static int nmi_callback(const struct cpu_user_regs *regs, int cpu)
   {
        int xen_mode, ovf;
ovf = model->check_ctrs(cpu, &cpu_msrs[cpu], regs);
        xen_mode = ring_0(regs);

Unrelated to the patch here (i.e. just as an observation), this
use of ring_0() looks bogus when the NMI occurred in HVM guest
mode.

An NMI in an HVM guest due to oprofile would be a VMEXIT with NMI
reason, or just be handled completely inside the guest, right?

I don't see how this test should ever result in xen_mode being
false for an HVM guest.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.