Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On Wed, 13 Dec 2017 10:31:58 +0100 David Hildenbrand wrote: > On 13.12.2017 10:16, Christian Borntraeger wrote: > > > > > > On 12/12/2017 04:28 PM, Cornelia Huck wrote: > >> On Tue, 12 Dec 2017 16:17:17 +0100 > >> David Hildenbrand wrote: > >> > >>> On 12.12.2017 15:29, Cornelia Huck wrote: > On Tue, 12 Dec 2017 15:13:46 +0100 > Christian Borntraeger wrote: > > > On 12/12/2017 02:49 PM, Cornelia Huck wrote: > > >> One thing I noticed: You removed the caching of the flic (in the old > >> kvm inject routine), and you generally do more qom invocations (first, > >> to find the common flic; then, to translate to the qemu or kvm flic). > >> Not sure if this might be a problem (probably not). > > > > Is any of these calls on a potential fast path (e.g. guest without > > adapter > > interrupts)? If yes, then QOM is a no-go since it is really slow. > > At least the new airq interface was using QOM without caching before. > > It's basically about any interrupt; but otoh we are (for kvm) in > userspace already. Caching the flic and just keeping the casting to the > specialized flic might be ok (I'd guess that the lookup is the slowest > path.) > > >>> > >>> Please note that the lookup is already cached in s390_get_flic(); That > >>> should be sufficient, as it does the expensive lookup. One cache should > >>> be enough, no? > >> > >> Ah, missed that. So the old code actually did double caching... > >> > >>> > >>> The other conversions should be cheap (and already were in place in a > >>> couple of places before). > >> > >> Yes, object_resolve_path() is probably the most expensive one. > >> > >> Did anyone ever check if the (existing) conversions are actually > >> measurable? > > > > Did some quick measurement. > > the initial object resolve takes about 2ns, the cached ones then is > > 2-5ns. > > (measuring s390_get_flic function). > > > > > > The other conversions (S390FLICStateClass *fsc = > > S390_FLIC_COMMON_GET_CLASS(fs);) > > takes about 15-25ns (up to 100 cycles). > > For irqfd users this does not matter, but things like plan9fs might benefit > > if we speedup this lookup as well? > > Did you also measure the state conversion like QEMU_S390_FLIC() or > KVM_S390_FLIC() ? > > As we call e.g. QEMU_S390_FLIC() on a regular basis in TCG, it might > then also make sense to speed that up. > > a) cache the (converted) state and class in every function. Somewhat > uglifies the code. > > b) introduce new functions that return the cached value > > Not sure what's better. I prefer to do this as a separate addon patch > and can prepare something. If you want to do it as addon, I vote for option b). > > > > > > > PS: We can improve the initial object_resolve_path by doing the resolve not > > for > > TYPE_KVM_S390_FLIC > > but > > "/machine/" TYPE_KVM_S390_FLIC > > (2500ns instead of 2) > > but its still way too slow. > > > > Specifying the absolute path would be even faster I guess. > > /machine/s390-flic-qemu/ ... I don't think we really need to speed up the initial lookup.
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On 12/13/2017 10:31 AM, David Hildenbrand wrote: > On 13.12.2017 10:16, Christian Borntraeger wrote: >> >> >> On 12/12/2017 04:28 PM, Cornelia Huck wrote: >>> On Tue, 12 Dec 2017 16:17:17 +0100 >>> David Hildenbrand wrote: >>> On 12.12.2017 15:29, Cornelia Huck wrote: > On Tue, 12 Dec 2017 15:13:46 +0100 > Christian Borntraeger wrote: > >> On 12/12/2017 02:49 PM, Cornelia Huck wrote: > >>> One thing I noticed: You removed the caching of the flic (in the old >>> kvm inject routine), and you generally do more qom invocations (first, >>> to find the common flic; then, to translate to the qemu or kvm flic). >>> Not sure if this might be a problem (probably not). >> >> Is any of these calls on a potential fast path (e.g. guest without >> adapter >> interrupts)? If yes, then QOM is a no-go since it is really slow. > > At least the new airq interface was using QOM without caching before. > > It's basically about any interrupt; but otoh we are (for kvm) in > userspace already. Caching the flic and just keeping the casting to the > specialized flic might be ok (I'd guess that the lookup is the slowest > path.) > Please note that the lookup is already cached in s390_get_flic(); That should be sufficient, as it does the expensive lookup. One cache should be enough, no? >>> >>> Ah, missed that. So the old code actually did double caching... >>> The other conversions should be cheap (and already were in place in a couple of places before). >>> >>> Yes, object_resolve_path() is probably the most expensive one. >>> >>> Did anyone ever check if the (existing) conversions are actually >>> measurable? >> >> Did some quick measurement. >> the initial object resolve takes about 2ns, the cached ones then is >> 2-5ns. >> (measuring s390_get_flic function). >> >> >> The other conversions (S390FLICStateClass *fsc = >> S390_FLIC_COMMON_GET_CLASS(fs);) >> takes about 15-25ns (up to 100 cycles). >> For irqfd users this does not matter, but things like plan9fs might benefit >> if we speedup this lookup as well? > > Did you also measure the state conversion like QEMU_S390_FLIC() or > KVM_S390_FLIC() ? somewhat between 4 and 39 ns.
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On Wed, 13 Dec 2017 10:16:34 +0100 Christian Borntraeger wrote: > On 12/12/2017 04:28 PM, Cornelia Huck wrote: > > On Tue, 12 Dec 2017 16:17:17 +0100 > > David Hildenbrand wrote: > > > >> On 12.12.2017 15:29, Cornelia Huck wrote: > >>> On Tue, 12 Dec 2017 15:13:46 +0100 > >>> Christian Borntraeger wrote: > >>> > On 12/12/2017 02:49 PM, Cornelia Huck wrote: > >>> > > One thing I noticed: You removed the caching of the flic (in the old > > kvm inject routine), and you generally do more qom invocations (first, > > to find the common flic; then, to translate to the qemu or kvm flic). > > Not sure if this might be a problem (probably not). > > Is any of these calls on a potential fast path (e.g. guest without > adapter > interrupts)? If yes, then QOM is a no-go since it is really slow. > >>> > >>> At least the new airq interface was using QOM without caching before. > >>> > >>> It's basically about any interrupt; but otoh we are (for kvm) in > >>> userspace already. Caching the flic and just keeping the casting to the > >>> specialized flic might be ok (I'd guess that the lookup is the slowest > >>> path.) > >>> > >> > >> Please note that the lookup is already cached in s390_get_flic(); That > >> should be sufficient, as it does the expensive lookup. One cache should > >> be enough, no? > > > > Ah, missed that. So the old code actually did double caching... > > > >> > >> The other conversions should be cheap (and already were in place in a > >> couple of places before). > > > > Yes, object_resolve_path() is probably the most expensive one. > > > > Did anyone ever check if the (existing) conversions are actually > > measurable? > > Did some quick measurement. > the initial object resolve takes about 2ns, the cached ones then is 2-5ns. > (measuring s390_get_flic function). > > > The other conversions (S390FLICStateClass *fsc = > S390_FLIC_COMMON_GET_CLASS(fs);) > takes about 15-25ns (up to 100 cycles). That's probably the debug checks, the rest should not take many cycles. > For irqfd users this does not matter, but things like plan9fs might benefit > if we speedup this lookup as well? We can probably cache the fsc as well. There are still the casts from the common flic to the qemu/kvm flics; we can cache them in the individual callbacks. Looks a bit odd in the code though, I guess. An alternative would be using "fast" casts that bypass QOM. > > > PS: We can improve the initial object_resolve_path by doing the resolve not > for > TYPE_KVM_S390_FLIC > but > "/machine/" TYPE_KVM_S390_FLIC > (2500ns instead of 2) > but its still way too slow. > Yeah, caching looks like the saner approach in that case.
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On 13.12.2017 10:16, Christian Borntraeger wrote: > > > On 12/12/2017 04:28 PM, Cornelia Huck wrote: >> On Tue, 12 Dec 2017 16:17:17 +0100 >> David Hildenbrand wrote: >> >>> On 12.12.2017 15:29, Cornelia Huck wrote: On Tue, 12 Dec 2017 15:13:46 +0100 Christian Borntraeger wrote: > On 12/12/2017 02:49 PM, Cornelia Huck wrote: >> One thing I noticed: You removed the caching of the flic (in the old >> kvm inject routine), and you generally do more qom invocations (first, >> to find the common flic; then, to translate to the qemu or kvm flic). >> Not sure if this might be a problem (probably not). > > Is any of these calls on a potential fast path (e.g. guest without adapter > interrupts)? If yes, then QOM is a no-go since it is really slow. At least the new airq interface was using QOM without caching before. It's basically about any interrupt; but otoh we are (for kvm) in userspace already. Caching the flic and just keeping the casting to the specialized flic might be ok (I'd guess that the lookup is the slowest path.) >>> >>> Please note that the lookup is already cached in s390_get_flic(); That >>> should be sufficient, as it does the expensive lookup. One cache should >>> be enough, no? >> >> Ah, missed that. So the old code actually did double caching... >> >>> >>> The other conversions should be cheap (and already were in place in a >>> couple of places before). >> >> Yes, object_resolve_path() is probably the most expensive one. >> >> Did anyone ever check if the (existing) conversions are actually >> measurable? > > Did some quick measurement. > the initial object resolve takes about 2ns, the cached ones then is 2-5ns. > (measuring s390_get_flic function). > > > The other conversions (S390FLICStateClass *fsc = > S390_FLIC_COMMON_GET_CLASS(fs);) > takes about 15-25ns (up to 100 cycles). > For irqfd users this does not matter, but things like plan9fs might benefit > if we speedup this lookup as well? Did you also measure the state conversion like QEMU_S390_FLIC() or KVM_S390_FLIC() ? As we call e.g. QEMU_S390_FLIC() on a regular basis in TCG, it might then also make sense to speed that up. a) cache the (converted) state and class in every function. Somewhat uglifies the code. b) introduce new functions that return the cached value Not sure what's better. I prefer to do this as a separate addon patch and can prepare something. > > > PS: We can improve the initial object_resolve_path by doing the resolve not > for > TYPE_KVM_S390_FLIC > but > "/machine/" TYPE_KVM_S390_FLIC > (2500ns instead of 2) > but its still way too slow. > Specifying the absolute path would be even faster I guess. /machine/s390-flic-qemu/ ... -- Thanks, David / dhildenb
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On 12/12/2017 04:28 PM, Cornelia Huck wrote: > On Tue, 12 Dec 2017 16:17:17 +0100 > David Hildenbrand wrote: > >> On 12.12.2017 15:29, Cornelia Huck wrote: >>> On Tue, 12 Dec 2017 15:13:46 +0100 >>> Christian Borntraeger wrote: >>> On 12/12/2017 02:49 PM, Cornelia Huck wrote: >>> > One thing I noticed: You removed the caching of the flic (in the old > kvm inject routine), and you generally do more qom invocations (first, > to find the common flic; then, to translate to the qemu or kvm flic). > Not sure if this might be a problem (probably not). Is any of these calls on a potential fast path (e.g. guest without adapter interrupts)? If yes, then QOM is a no-go since it is really slow. >>> >>> At least the new airq interface was using QOM without caching before. >>> >>> It's basically about any interrupt; but otoh we are (for kvm) in >>> userspace already. Caching the flic and just keeping the casting to the >>> specialized flic might be ok (I'd guess that the lookup is the slowest >>> path.) >>> >> >> Please note that the lookup is already cached in s390_get_flic(); That >> should be sufficient, as it does the expensive lookup. One cache should >> be enough, no? > > Ah, missed that. So the old code actually did double caching... > >> >> The other conversions should be cheap (and already were in place in a >> couple of places before). > > Yes, object_resolve_path() is probably the most expensive one. > > Did anyone ever check if the (existing) conversions are actually > measurable? Did some quick measurement. the initial object resolve takes about 2ns, the cached ones then is 2-5ns. (measuring s390_get_flic function). The other conversions (S390FLICStateClass *fsc = S390_FLIC_COMMON_GET_CLASS(fs);) takes about 15-25ns (up to 100 cycles). For irqfd users this does not matter, but things like plan9fs might benefit if we speedup this lookup as well? PS: We can improve the initial object_resolve_path by doing the resolve not for TYPE_KVM_S390_FLIC but "/machine/" TYPE_KVM_S390_FLIC (2500ns instead of 2) but its still way too slow.
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On 12.12.2017 14:49, Cornelia Huck wrote:
> On Mon, 11 Dec 2017 14:47:31 +0100
> David Hildenbrand wrote:
>
>> Let the flic device handle it internally. This will allow us to later
>> on store floating interrupts in the flic for the TCG case.
>>
>> This now also simplifies kvm.c. All that's left is the fallback
>> interface for floating interrupts, which is no triggered directly via
>
> s/no/now/
>
>> the flic in case anything goes wrong.
>>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/intc/s390_flic.c | 31
>> hw/intc/s390_flic_kvm.c | 63
>> include/hw/s390x/s390_flic.h | 5
>> target/s390x/cpu.h | 7 -
>> target/s390x/interrupt.c | 42 +++
>> target/s390x/kvm-stub.c | 13 -
>> target/s390x/kvm.c | 68
>>
>> target/s390x/kvm_s390x.h | 10 +--
>> 8 files changed, 123 insertions(+), 116 deletions(-)
>>
>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
>> index a78bdf1d90..8d521c415a 100644
>> --- a/hw/intc/s390_flic.c
>> +++ b/hw/intc/s390_flic.c
>> @@ -131,6 +131,34 @@ static int qemu_s390_inject_airq(S390FLICState *fs,
>> uint8_t type,
>> return 0;
>> }
>>
>> +static void qemu_s390_inject_service(S390FLICState *fs, uint32_t parm)
>> +{
>> +
>> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
>> +
>> +/* FIXME: don't inject into dummy CPU */
>> +cpu_inject_service(dummy_cpu, parm);
>> +}
>> +
>> +static void qemu_s390_inject_io(S390FLICState *fs, uint16_t subchannel_id,
>> +uint16_t subchannel_nr, uint32_t
>> io_int_parm,
>> +uint32_t io_int_word)
>> +{
>> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
>> +
>> +/* FIXME: don't inject into dummy CPU */
>> +cpu_inject_io(dummy_cpu, subchannel_id, subchannel_nr, io_int_parm,
>> + io_int_word);
>> +}
>> +
>> +static void qemu_s390_inject_crw_mchk(S390FLICState *fs)
>> +{
>> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
>> +
>> +/* FIXME: don't inject into dummy CPU */
>> +cpu_inject_crw_mchk(dummy_cpu);
>> +}
>> +
>> static void qemu_s390_flic_reset(DeviceState *dev)
>> {
>> QEMUS390FLICState *flic = QEMU_S390_FLIC(dev);
>> @@ -172,6 +200,9 @@ static void qemu_s390_flic_class_init(ObjectClass *oc,
>> void *data)
>> fsc->clear_io_irq = qemu_s390_clear_io_flic;
>> fsc->modify_ais_mode = qemu_s390_modify_ais_mode;
>> fsc->inject_airq = qemu_s390_inject_airq;
>> +fsc->inject_service = qemu_s390_inject_service;
>> +fsc->inject_io = qemu_s390_inject_io;
>> +fsc->inject_crw_mchk = qemu_s390_inject_crw_mchk;
>> }
>
> As you now have a callback for ->inject_io(), make
> qemu_s390_inject_airq() invoke it directly instead of going the detour
> through s390_io_interrupt()?
Indeed, that makes sense.
--
Thanks,
David / dhildenb
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On Tue, 12 Dec 2017 16:17:17 +0100 David Hildenbrand wrote: > On 12.12.2017 15:29, Cornelia Huck wrote: > > On Tue, 12 Dec 2017 15:13:46 +0100 > > Christian Borntraeger wrote: > > > >> On 12/12/2017 02:49 PM, Cornelia Huck wrote: > > > >>> One thing I noticed: You removed the caching of the flic (in the old > >>> kvm inject routine), and you generally do more qom invocations (first, > >>> to find the common flic; then, to translate to the qemu or kvm flic). > >>> Not sure if this might be a problem (probably not). > >> > >> Is any of these calls on a potential fast path (e.g. guest without adapter > >> interrupts)? If yes, then QOM is a no-go since it is really slow. > > > > At least the new airq interface was using QOM without caching before. > > > > It's basically about any interrupt; but otoh we are (for kvm) in > > userspace already. Caching the flic and just keeping the casting to the > > specialized flic might be ok (I'd guess that the lookup is the slowest > > path.) > > > > Please note that the lookup is already cached in s390_get_flic(); That > should be sufficient, as it does the expensive lookup. One cache should > be enough, no? Ah, missed that. So the old code actually did double caching... > > The other conversions should be cheap (and already were in place in a > couple of places before). Yes, object_resolve_path() is probably the most expensive one. Did anyone ever check if the (existing) conversions are actually measurable?
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On 12.12.2017 15:29, Cornelia Huck wrote: > On Tue, 12 Dec 2017 15:13:46 +0100 > Christian Borntraeger wrote: > >> On 12/12/2017 02:49 PM, Cornelia Huck wrote: > >>> One thing I noticed: You removed the caching of the flic (in the old >>> kvm inject routine), and you generally do more qom invocations (first, >>> to find the common flic; then, to translate to the qemu or kvm flic). >>> Not sure if this might be a problem (probably not). >> >> Is any of these calls on a potential fast path (e.g. guest without adapter >> interrupts)? If yes, then QOM is a no-go since it is really slow. > > At least the new airq interface was using QOM without caching before. > > It's basically about any interrupt; but otoh we are (for kvm) in > userspace already. Caching the flic and just keeping the casting to the > specialized flic might be ok (I'd guess that the lookup is the slowest > path.) > Please note that the lookup is already cached in s390_get_flic(); That should be sufficient, as it does the expensive lookup. One cache should be enough, no? The other conversions should be cheap (and already were in place in a couple of places before). Thanks! -- Thanks, David / dhildenb
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On Tue, 12 Dec 2017 15:13:46 +0100 Christian Borntraeger wrote: > On 12/12/2017 02:49 PM, Cornelia Huck wrote: > > One thing I noticed: You removed the caching of the flic (in the old > > kvm inject routine), and you generally do more qom invocations (first, > > to find the common flic; then, to translate to the qemu or kvm flic). > > Not sure if this might be a problem (probably not). > > Is any of these calls on a potential fast path (e.g. guest without adapter > interrupts)? If yes, then QOM is a no-go since it is really slow. At least the new airq interface was using QOM without caching before. It's basically about any interrupt; but otoh we are (for kvm) in userspace already. Caching the flic and just keeping the casting to the specialized flic might be ok (I'd guess that the lookup is the slowest path.)
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On 12/12/2017 02:49 PM, Cornelia Huck wrote:
> On Mon, 11 Dec 2017 14:47:31 +0100
> David Hildenbrand wrote:
>
>> Let the flic device handle it internally. This will allow us to later
>> on store floating interrupts in the flic for the TCG case.
>>
>> This now also simplifies kvm.c. All that's left is the fallback
>> interface for floating interrupts, which is no triggered directly via
>
> s/no/now/
>
>> the flic in case anything goes wrong.
>>
>> Signed-off-by: David Hildenbrand
>> ---
>> hw/intc/s390_flic.c | 31
>> hw/intc/s390_flic_kvm.c | 63
>> include/hw/s390x/s390_flic.h | 5
>> target/s390x/cpu.h | 7 -
>> target/s390x/interrupt.c | 42 +++
>> target/s390x/kvm-stub.c | 13 -
>> target/s390x/kvm.c | 68
>>
>> target/s390x/kvm_s390x.h | 10 +--
>> 8 files changed, 123 insertions(+), 116 deletions(-)
>>
>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
>> index a78bdf1d90..8d521c415a 100644
>> --- a/hw/intc/s390_flic.c
>> +++ b/hw/intc/s390_flic.c
>> @@ -131,6 +131,34 @@ static int qemu_s390_inject_airq(S390FLICState *fs,
>> uint8_t type,
>> return 0;
>> }
>>
>> +static void qemu_s390_inject_service(S390FLICState *fs, uint32_t parm)
>> +{
>> +
>> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
>> +
>> +/* FIXME: don't inject into dummy CPU */
>> +cpu_inject_service(dummy_cpu, parm);
>> +}
>> +
>> +static void qemu_s390_inject_io(S390FLICState *fs, uint16_t subchannel_id,
>> +uint16_t subchannel_nr, uint32_t
>> io_int_parm,
>> +uint32_t io_int_word)
>> +{
>> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
>> +
>> +/* FIXME: don't inject into dummy CPU */
>> +cpu_inject_io(dummy_cpu, subchannel_id, subchannel_nr, io_int_parm,
>> + io_int_word);
>> +}
>> +
>> +static void qemu_s390_inject_crw_mchk(S390FLICState *fs)
>> +{
>> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
>> +
>> +/* FIXME: don't inject into dummy CPU */
>> +cpu_inject_crw_mchk(dummy_cpu);
>> +}
>> +
>> static void qemu_s390_flic_reset(DeviceState *dev)
>> {
>> QEMUS390FLICState *flic = QEMU_S390_FLIC(dev);
>> @@ -172,6 +200,9 @@ static void qemu_s390_flic_class_init(ObjectClass *oc,
>> void *data)
>> fsc->clear_io_irq = qemu_s390_clear_io_flic;
>> fsc->modify_ais_mode = qemu_s390_modify_ais_mode;
>> fsc->inject_airq = qemu_s390_inject_airq;
>> +fsc->inject_service = qemu_s390_inject_service;
>> +fsc->inject_io = qemu_s390_inject_io;
>> +fsc->inject_crw_mchk = qemu_s390_inject_crw_mchk;
>> }
>
> As you now have a callback for ->inject_io(), make
> qemu_s390_inject_airq() invoke it directly instead of going the detour
> through s390_io_interrupt()?
>
>>
>> static Property s390_flic_common_properties[] = {
>
> (...)
>
> Generally looks sane.
>
> One thing I noticed: You removed the caching of the flic (in the old
> kvm inject routine), and you generally do more qom invocations (first,
> to find the common flic; then, to translate to the qemu or kvm flic).
> Not sure if this might be a problem (probably not).
Is any of these calls on a potential fast path (e.g. guest without adapter
interrupts)? If yes, then QOM is a no-go since it is really slow.
Re: [Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
On Mon, 11 Dec 2017 14:47:31 +0100
David Hildenbrand wrote:
> Let the flic device handle it internally. This will allow us to later
> on store floating interrupts in the flic for the TCG case.
>
> This now also simplifies kvm.c. All that's left is the fallback
> interface for floating interrupts, which is no triggered directly via
s/no/now/
> the flic in case anything goes wrong.
>
> Signed-off-by: David Hildenbrand
> ---
> hw/intc/s390_flic.c | 31
> hw/intc/s390_flic_kvm.c | 63
> include/hw/s390x/s390_flic.h | 5
> target/s390x/cpu.h | 7 -
> target/s390x/interrupt.c | 42 +++
> target/s390x/kvm-stub.c | 13 -
> target/s390x/kvm.c | 68
>
> target/s390x/kvm_s390x.h | 10 +--
> 8 files changed, 123 insertions(+), 116 deletions(-)
>
> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> index a78bdf1d90..8d521c415a 100644
> --- a/hw/intc/s390_flic.c
> +++ b/hw/intc/s390_flic.c
> @@ -131,6 +131,34 @@ static int qemu_s390_inject_airq(S390FLICState *fs,
> uint8_t type,
> return 0;
> }
>
> +static void qemu_s390_inject_service(S390FLICState *fs, uint32_t parm)
> +{
> +
> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
> +
> +/* FIXME: don't inject into dummy CPU */
> +cpu_inject_service(dummy_cpu, parm);
> +}
> +
> +static void qemu_s390_inject_io(S390FLICState *fs, uint16_t subchannel_id,
> +uint16_t subchannel_nr, uint32_t io_int_parm,
> +uint32_t io_int_word)
> +{
> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
> +
> +/* FIXME: don't inject into dummy CPU */
> +cpu_inject_io(dummy_cpu, subchannel_id, subchannel_nr, io_int_parm,
> + io_int_word);
> +}
> +
> +static void qemu_s390_inject_crw_mchk(S390FLICState *fs)
> +{
> +S390CPU *dummy_cpu = s390_cpu_addr2state(0);
> +
> +/* FIXME: don't inject into dummy CPU */
> +cpu_inject_crw_mchk(dummy_cpu);
> +}
> +
> static void qemu_s390_flic_reset(DeviceState *dev)
> {
> QEMUS390FLICState *flic = QEMU_S390_FLIC(dev);
> @@ -172,6 +200,9 @@ static void qemu_s390_flic_class_init(ObjectClass *oc,
> void *data)
> fsc->clear_io_irq = qemu_s390_clear_io_flic;
> fsc->modify_ais_mode = qemu_s390_modify_ais_mode;
> fsc->inject_airq = qemu_s390_inject_airq;
> +fsc->inject_service = qemu_s390_inject_service;
> +fsc->inject_io = qemu_s390_inject_io;
> +fsc->inject_crw_mchk = qemu_s390_inject_crw_mchk;
> }
As you now have a callback for ->inject_io(), make
qemu_s390_inject_airq() invoke it directly instead of going the detour
through s390_io_interrupt()?
>
> static Property s390_flic_common_properties[] = {
(...)
Generally looks sane.
One thing I noticed: You removed the caching of the flic (in the old
kvm inject routine), and you generally do more qom invocations (first,
to find the common flic; then, to translate to the qemu or kvm flic).
Not sure if this might be a problem (probably not).
[Qemu-devel] [PATCH v1 for-2-12 06/15] s390x/flic: factor out injection of floating interrupts
Let the flic device handle it internally. This will allow us to later
on store floating interrupts in the flic for the TCG case.
This now also simplifies kvm.c. All that's left is the fallback
interface for floating interrupts, which is no triggered directly via
the flic in case anything goes wrong.
Signed-off-by: David Hildenbrand
---
hw/intc/s390_flic.c | 31
hw/intc/s390_flic_kvm.c | 63
include/hw/s390x/s390_flic.h | 5
target/s390x/cpu.h | 7 -
target/s390x/interrupt.c | 42 +++
target/s390x/kvm-stub.c | 13 -
target/s390x/kvm.c | 68
target/s390x/kvm_s390x.h | 10 +--
8 files changed, 123 insertions(+), 116 deletions(-)
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index a78bdf1d90..8d521c415a 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -131,6 +131,34 @@ static int qemu_s390_inject_airq(S390FLICState *fs,
uint8_t type,
return 0;
}
+static void qemu_s390_inject_service(S390FLICState *fs, uint32_t parm)
+{
+
+S390CPU *dummy_cpu = s390_cpu_addr2state(0);
+
+/* FIXME: don't inject into dummy CPU */
+cpu_inject_service(dummy_cpu, parm);
+}
+
+static void qemu_s390_inject_io(S390FLICState *fs, uint16_t subchannel_id,
+uint16_t subchannel_nr, uint32_t io_int_parm,
+uint32_t io_int_word)
+{
+S390CPU *dummy_cpu = s390_cpu_addr2state(0);
+
+/* FIXME: don't inject into dummy CPU */
+cpu_inject_io(dummy_cpu, subchannel_id, subchannel_nr, io_int_parm,
+ io_int_word);
+}
+
+static void qemu_s390_inject_crw_mchk(S390FLICState *fs)
+{
+S390CPU *dummy_cpu = s390_cpu_addr2state(0);
+
+/* FIXME: don't inject into dummy CPU */
+cpu_inject_crw_mchk(dummy_cpu);
+}
+
static void qemu_s390_flic_reset(DeviceState *dev)
{
QEMUS390FLICState *flic = QEMU_S390_FLIC(dev);
@@ -172,6 +200,9 @@ static void qemu_s390_flic_class_init(ObjectClass *oc, void
*data)
fsc->clear_io_irq = qemu_s390_clear_io_flic;
fsc->modify_ais_mode = qemu_s390_modify_ais_mode;
fsc->inject_airq = qemu_s390_inject_airq;
+fsc->inject_service = qemu_s390_inject_service;
+fsc->inject_io = qemu_s390_inject_io;
+fsc->inject_crw_mchk = qemu_s390_inject_crw_mchk;
}
static Property s390_flic_common_properties[] = {
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index 0cb5feab0c..d277ffdd2e 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -111,14 +111,64 @@ static int flic_enqueue_irqs(void *buf, uint64_t len,
return rc ? -errno : 0;
}
-int kvm_s390_inject_flic(struct kvm_s390_irq *irq)
+static void kvm_s390_inject_flic(S390FLICState *fs, struct kvm_s390_irq *irq)
{
-static KVMS390FLICState *flic;
+static bool use_flic = true;
+int r;
+
+if (use_flic) {
+r = flic_enqueue_irqs(irq, sizeof(*irq), KVM_S390_FLIC(fs));
+if (r == -ENOSYS) {
+use_flic = false;
+}
+if (!r) {
+return;
+}
+}
+/* fallback to legacy KVM IOCTL in case FLIC fails */
+kvm_s390_floating_interrupt_legacy(irq);
+}
+
+static void kvm_s390_inject_service(S390FLICState *fs, uint32_t parm)
+{
+struct kvm_s390_irq irq = {
+.type = KVM_S390_INT_SERVICE,
+.u.ext.ext_params = parm,
+};
+
+kvm_s390_inject_flic(fs, &irq);
+}
-if (unlikely(!flic)) {
-flic = KVM_S390_FLIC(s390_get_flic());
+static void kvm_s390_inject_io(S390FLICState *fs, uint16_t subchannel_id,
+ uint16_t subchannel_nr, uint32_t io_int_parm,
+ uint32_t io_int_word)
+{
+struct kvm_s390_irq irq = {
+.u.io.subchannel_id = subchannel_id,
+.u.io.subchannel_nr = subchannel_nr,
+.u.io.io_int_parm = io_int_parm,
+.u.io.io_int_word = io_int_word,
+};
+
+if (io_int_word & IO_INT_WORD_AI) {
+irq.type = KVM_S390_INT_IO(1, 0, 0, 0);
+} else {
+irq.type = KVM_S390_INT_IO(0, (subchannel_id & 0xff00) >> 8,
+ (subchannel_id & 0x0006),
+ subchannel_nr);
}
-return flic_enqueue_irqs(irq, sizeof(*irq), flic);
+kvm_s390_inject_flic(fs, &irq);
+}
+
+static void kvm_s390_inject_crw_mchk(S390FLICState *fs)
+{
+struct kvm_s390_irq irq = {
+.type = KVM_S390_MCHK,
+.u.mchk.cr14 = CR14_CHANNEL_REPORT_SC,
+.u.mchk.mcic = s390_build_validity_mcic() | MCIC_SC_CP,
+};
+
+kvm_s390_inject_flic(fs, &irq);
}
static int kvm_s390_clear_io_flic(S390FLICState *fs, uint16_t subchannel_id,
@@ -602,6 +652,9 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void
*data)
fsc->clear_io_irq = kvm_s390_clear_io_flic;
fsc->modify_ais_mode = kvm_s390_modify_ais
