[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH] pseries: Support for in-kernel XICS
From: |
Alexey Kardashevskiy |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH] pseries: Support for in-kernel XICS interrupt controller |
Date: |
Sun, 09 Jun 2013 00:11:54 +1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 |
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. 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