On 06/08/2013 08:20 PM, Andreas Färber wrote:
> Am 05.06.2013 09:39, schrieb Alexey Kardashevskiy:
>> From: David Gibson <da...@gibson.dropbear.id.au>
>>
>> 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 <qiud...@linux.vnet.ibm.com>: fixed mistype which caused
>> ics_set_kvm_state() to fail]
>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>
> 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 <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.
--
Alexey
