From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [RFC PATCH] Adding support for coverage informations Date: Wed, 30 Jan 2013 16:34:19 -0500 Message-ID: <20130130213418.GC1885@konrad-lan.dumpdata.com> References: <1359407799.32148.1.camel@nemesi> <1359457004.12252.103.camel@zakaz.uk.xensource.com> <7CE799CC0E4DE04B88D5FDF226E18AC201058D458342@LONPMAILBOX01.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <7CE799CC0E4DE04B88D5FDF226E18AC201058D458342@LONPMAILBOX01.citrite.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Frediano Ziglio Cc: George Dunlap , "Keir (Xen.org)" , Ian Campbell , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On Tue, Jan 29, 2013 at 12:02:55PM +0000, Frediano Ziglio wrote: > On Tue, 2013-01-29 at 10:56 +0000, Ian Campbell wrote: > > On Mon, 2013-01-28 at 21:16 +0000, Frediano Ziglio wrote: > > > From: Frediano Ziglio > > > > > > This patch introduce coverage support to Xen. > > > Currently it allows to compile Xen with coverage support but there is no way > > > to extract them. > > > > > > The declarations came from Linux source files (as you can see from file > > > headers). > > > > > > It also add a new hypercall (can somebody reserve a number for this stuff?). > > > > You can simply patch xen/include/public/xen.h to declare the new > > __HYPERVISOR_foo_op. (which I now see you have done in this patch!). If > > you just want to reserve the number it is also allowed to send just that > > hunk to reserve a number pending the implementation. > > > > BTW I'd suggest leaving the stub implementation out of the patch until > > you've decided what it will look like, so it returns ENOSYS instead of > > EINVAL (or change your stub to return ENOSYS). > > > > Easy to change. Actually I discovered that my patch does not even > compile if you disable TEST_COVERAGE (and it should be disabled by > default too). EINVAL say that the hypercall is present but the value > passed in invalid. > > The reason why adding a new hypercall instead of a new sysctl is simply > because is easier to have a zero cost if you disable coverage > informations. The best thing would be redirect do_coverage_op to > do_ni_hypercall using linker options but even two small stub would do > (these stubs will return ENOSYS instead). I am not sure I follow. Is the sysctl hypercall code path "longer" than the hypercall path you are introducing? What is the zero cost?