From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49331) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UlNTU-0002t7-Mb for qemu-devel@nongnu.org; Sat, 08 Jun 2013 14:02:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UlNTQ-0003TY-KN for qemu-devel@nongnu.org; Sat, 08 Jun 2013 14:02:48 -0400 Message-ID: <51B371BB.9090401@suse.de> Date: Sat, 08 Jun 2013 20:02:35 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1370417998-7061-1-git-send-email-aik@ozlabs.ru> <51B30580.70906@suse.de> <51B33BAA.2040601@ozlabs.ru> In-Reply-To: <51B33BAA.2040601@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: Alexey Kardashevskiy Cc: Mike Qiu , David Gibson , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf Am 08.06.2013 16:11, schrieb Alexey Kardashevskiy: > On 06/08/2013 08:20 PM, Andreas F=C3=A4rber wrote: >> Am 05.06.2013 09:39, schrieb Alexey Kardashevskiy: >>> From: David Gibson >>> >>> Recent (host) kernels support emulating the PAPR defined "XICS" inter= rupt >>> controller system within KVM. This patch allows qemu to initialize a= nd >>> 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 si= mple >>> 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 ic= s_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? >=20 >=20 > He did not change this patch, he found a mistype in our local source tr= ee > which I decided to merge with this patch. I did not want him not to be > mentioned at all so I added this line. Then that notation is misleading: [author: ...] usually indicates that author applied the noted changes to the patch, and just like tags - if at all - this should get recorded in chronological order, i.e. S-o-b David ... [aik: fixed mistype ... spotted by Mike ...] S-o-b you ... making clearer who signed off which version. > What is the general rule who needs > to s-o-b? For a formal description see Linux' SubmittingPatches docs. In practice, whenever you git-am or git-cherry-pick a patch it should have at least one Signed-off-by from the person you got it from. Whenever you submit a patch it should carry your Sob as last one, thereby recording the sequence of through whose hands a patch went. >>> 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 >>> =20 >>> /* >>> * ICP: Presentation layer >>> */ >>> =20 >>> 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... >=20 >=20 > Is it mandatory? I personally do not see _any_ benefit in aligning stru= ct > members with spaces. Dunno about whether that is somewhere in HACKING or CODING_STYLE, and I don't really mind either way. But let me clarify that I wasn't talking about space-alignment, I was talking about duplicating the type as you can see for pending_priority and mfrr field above being on their own line each. Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg