From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35770) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UlJsB-0002Sf-MB for qemu-devel@nongnu.org; Sat, 08 Jun 2013 10:12:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UlJsA-0006C4-HR for qemu-devel@nongnu.org; Sat, 08 Jun 2013 10:12:03 -0400 Received: from mail-pd0-f179.google.com ([209.85.192.179]:43829) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UlJsA-0006Bo-B2 for qemu-devel@nongnu.org; Sat, 08 Jun 2013 10:12:02 -0400 Received: by mail-pd0-f179.google.com with SMTP id q11so5865609pdj.10 for ; Sat, 08 Jun 2013 07:12:00 -0700 (PDT) Message-ID: <51B33BAA.2040601@ozlabs.ru> Date: Sun, 09 Jun 2013 00:11:54 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1370417998-7061-1-git-send-email-aik@ozlabs.ru> <51B30580.70906@suse.de> In-Reply-To: <51B30580.70906@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] pseries: Support for in-kernel XICS interrupt controller List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: Mike Qiu , David Gibson , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf On 06/08/2013 08:20 PM, Andreas Färber wrote: > Am 05.06.2013 09:39, schrieb Alexey Kardashevskiy: >> From: David Gibson >> >> Recent (host) kernels support emulating the PAPR defined "XICS" interrupt >> controller system within KVM. This patch allows qemu to initialize and >> configure the in-kernel XICS, and keep its state in sync with qemu's XICS >> state as necessary. >> >> This should give considerable performance improvements. e.g. on a simple >> IPI ping-pong test between hardware threads, using qemu XICS gives us >> around 5,000 irqs/second, whereas the in-kernel XICS gives us around >> 70,000 irqs/s on the same hardware configuration. >> >> [Mike Qiu : fixed mistype which caused ics_set_kvm_state() to fail] >> Signed-off-by: David Gibson >> Signed-off-by: Alexey Kardashevskiy > > If a Mike Qiu changed this patch, don't we require his Signed-off-by? He did not change this patch, he found a mistype in our local source tree which I decided to merge with this patch. I did not want him not to be mentioned at all so I added this line. What is the general rule who needs to s-o-b? > CPUState usage looks fine, can't judge the kernel interface, two > nitpicks below. > > [...] >> diff --git a/hw/ppc/xics.c b/hw/ppc/xics.c >> index 02e44a0..b83f19f 100644 >> --- a/hw/ppc/xics.c >> +++ b/hw/ppc/xics.c >> @@ -29,12 +29,19 @@ >> #include "trace.h" >> #include "hw/ppc/spapr.h" >> #include "hw/ppc/xics.h" >> +#include "kvm_ppc.h" >> +#include "sysemu/kvm.h" >> +#include "config.h" >> +#include "qemu/config-file.h" >> + >> +#include >> >> /* >> * ICP: Presentation layer >> */ >> >> struct icp_server_state { >> + CPUState *cs; >> uint32_t xirr; >> uint8_t pending_priority; >> uint8_t mfrr; >> @@ -53,6 +60,9 @@ struct icp_state { >> uint32_t nr_servers; >> struct icp_server_state *ss; >> struct ics_state *ics; >> + uint32_t set_xive_token, get_xive_token, >> + int_off_token, int_on_token; > > FWIW normally we place struct fields below each other... Is it mandatory? I personally do not see _any_ benefit in aligning struct members with spaces. -- Alexey