[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] pseries: Support for in-kernel XICS interrupt c
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH] pseries: Support for in-kernel XICS interrupt controller |
Date: |
Sat, 08 Jun 2013 20:02:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 |
Am 08.06.2013 16:11, schrieb Alexey Kardashevskiy:
> On 06/08/2013 08:20 PM, Andreas Färber wrote:
>> Am 05.06.2013 09:39, schrieb Alexey Kardashevskiy:
>>> From: David Gibson <address@hidden>
>>>
>>> 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 <address@hidden>: fixed mistype which caused ics_set_kvm_state()
>>> to fail]
>>> Signed-off-by: David Gibson <address@hidden>
>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>
>> 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.
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 <sys/ioctl.h>
>>>
>>> /*
>>> * 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.
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
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg