All of lore.kernel.org
 help / color / mirror / Atom feed
From: Razvan Cojocaru <rcojocaru@bitdefender.com>
To: Corneliu ZUZU <czuzu@bitdefender.com>, Jan Beulich <JBeulich@suse.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Tamas K Lengyel <tamas@tklengyel.com>, Keir Fraser <keir@xen.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	xen-devel@lists.xen.org, Jun Nakajima <jun.nakajima@intel.com>
Subject: Re: [PATCH v2 2/7] xen/x86: merge 2 hvm_event_... functions into 1
Date: Wed, 10 Feb 2016 19:28:37 +0200	[thread overview]
Message-ID: <56BB7345.3030703@bitdefender.com> (raw)
In-Reply-To: <56BB6D92.3050300@bitdefender.com>

On 02/10/2016 07:04 PM, Corneliu ZUZU wrote:
>>> @@ -27,9 +33,8 @@ bool_t hvm_event_cr(unsigned int index, unsigned
>>> long value,
>>>   #define hvm_event_crX(what, new, old) \
>>>       hvm_event_cr(VM_EVENT_X86_##what, new, old)
>>>   void hvm_event_msr(unsigned int msr, uint64_t value);
>>> -/* Called for current VCPU: returns -1 if no listener */
>>> -int hvm_event_int3(unsigned long rip);
>>> -int hvm_event_single_step(unsigned long rip);
>>> +int hvm_event_breakpoint(unsigned long rip,
>>> +                         enum hvm_event_breakpoint_type type);
>> I guess the comment was here for a reason, and this reason doesn't
>> go away with this code folding. But I'll leave it to the VM event code
>> maintainers to judge.
>>
>> Jan
> 
> That comment seemed & still seems wrong to me, I don't see any code
> paths out of which that function would return -1.

That seems to be true. Those functions return whatever hvm_event_traps()
returns, which is 0 on success, 1 (maybe the minus is a typo?) if
there's no ring, or whatever value vm_event_claim_slot() returns.
Vm_event_claim_slot()'s documentation says that it can only return 0 (on
success), -ENOSYS or -EBUSY, none of which translate to -1 (and the code
seems to agree with that claim).

Maybe I'm missing some macro wizardry here, but I don't think so - it
looks like the comment is stale. Tamas, maybe you remember more, should
those functions return -1 if no listener is present?


Thanks,
Razvan

  parent reply	other threads:[~2016-02-10 17:28 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-10 15:47 [PATCH v2 0/7] Vm-events: move monitor vm-events code to common-side Corneliu ZUZU
2016-02-10 15:47 ` [PATCH v2 1/7] xen/arm: fix file comments Corneliu ZUZU
2016-02-10 16:05   ` Stefano Stabellini
2016-02-10 15:50 ` [PATCH v2 2/7] xen/x86: merge 2 hvm_event_... functions into 1 Corneliu ZUZU
2016-02-10 16:18   ` Jan Beulich
2016-02-10 16:37     ` Andrew Cooper
2016-02-10 17:05       ` Jan Beulich
2016-02-10 17:04     ` Corneliu ZUZU
2016-02-10 17:11       ` Jan Beulich
2016-02-10 20:50         ` Corneliu ZUZU
2016-02-10 20:56         ` Andrew Cooper
2016-02-11 10:31           ` Jan Beulich
2016-02-11  6:03         ` Corneliu ZUZU
2016-02-10 17:28       ` Razvan Cojocaru [this message]
2016-02-10 17:52         ` Tamas K Lengyel
2016-02-10 15:52 ` [PATCH v2 3/7] xen/vm-events: Move monitor_domctl to common-side Corneliu ZUZU
2016-02-10 16:26   ` Jan Beulich
2016-02-10 16:30     ` Jan Beulich
2016-02-10 17:14       ` Corneliu ZUZU
2016-02-10 17:12     ` Corneliu ZUZU
2016-02-10 16:39   ` Tamas K Lengyel
2016-02-10 17:34     ` Corneliu ZUZU
2016-02-10 17:56       ` Tamas K Lengyel
2016-02-11  7:21         ` Corneliu ZUZU
2016-02-11 15:44           ` Tamas K Lengyel
2016-02-12  6:05             ` Corneliu ZUZU
2016-02-12  8:14               ` Corneliu ZUZU
2016-02-11  6:20     ` Corneliu ZUZU
2016-02-10 15:54 ` [PATCH v2 4/7] Rename monitor_x86.c to monitor.c and monitor_arch.h to monitor.h Corneliu ZUZU
2016-02-10 16:44   ` Tamas K Lengyel
2016-02-10 17:16     ` Jan Beulich
2016-02-10 20:54       ` Corneliu ZUZU
2016-02-10 20:36     ` Corneliu ZUZU
2016-02-10 15:56 ` [PATCH v2 5/7] xen/vm-events: Move hvm_event_* functions to common-side Corneliu ZUZU
2016-02-10 15:58 ` [PATCH v2 6/7] Rename event_x86.c to event.c and event_arch.h to event.h + minor fixes Corneliu ZUZU
2016-02-10 16:00 ` [PATCH v2 7/7] xen/vm-events: move arch_domain.monitor bits to common Corneliu ZUZU
2016-02-10 16:29   ` Jan Beulich
2016-02-10 17:14     ` Corneliu ZUZU

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56BB7345.3030703@bitdefender.com \
    --to=rcojocaru@bitdefender.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=czuzu@bitdefender.com \
    --cc=jun.nakajima@intel.com \
    --cc=keir@xen.org \
    --cc=kevin.tian@intel.com \
    --cc=tamas@tklengyel.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.