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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.11.2020 12:27, Jürgen Groß wrote:
> On 12.11.20 12:05, Jan Beulich wrote:
>> On 12.11.2020 11:48, Jürgen Groß wrote:
>>> 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:
>>>>>>>     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?
>>
>> Yes, and in the former case for VMX it would be handed on to do_nmi(),
>> with the guest register state. For SVM it would get handled on the
>> next STGI, i.e. would indeed never surface from HVM guest mode.
>>
>>> I don't see how this test should ever result in xen_mode being
>>> false for an HVM guest.
>>
>> I think, because of hvm_invalidate_regs_fields(), on VMX it would be
>> consistently true in release builds and consistently false in debug
>> ones.
> 
> Ah, okay. I searched for do_nmi(), but the vmx code uses the exception
> table instead.
> 
> So I guess this should be:
> 
> xen_mode = !guest_mode(regs);

Yes, I think so. Just that guest_mode() also has its issues (my patch
"x86: refine guest_mode()" improving it at least some is still pending
Andrew's go / no-go / improvement suggestions), so whether it's
suitable to use here may need some careful evaluation.

Jan