From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v2 3/6] hw/char: pl011 don't keep setting the IRQ if nothing changed Date: Fri, 13 Mar 2015 10:38:58 +0000 Message-ID: <87ioe5b2rh.fsf@linaro.org> References: <1425479753-18349-1-git-send-email-alex.bennee@linaro.org> <1425479753-18349-4-git-send-email-alex.bennee@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: QEMU Developers , kvm-devel , Marc Zyngier , arm-mail-list , "kvmarm\@lists.cs.columbia.edu" To: Peter Maydell Return-path: Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:36907 "EHLO socrates.bennee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750963AbbCMKiv (ORCPT ); Fri, 13 Mar 2015 06:38:51 -0400 In-reply-to: Sender: kvm-owner@vger.kernel.org List-ID: Peter Maydell writes: > On 12 March 2015 at 15:51, Peter Maydell w= rote: >> On 4 March 2015 at 14:35, Alex Benn=C3=A9e = wrote: >>> While observing KVM traces I can see additional IRQ calls on pretty= much >>> every MMIO access which is just plain inefficient. Only update the = QEMU >>> IRQ level if something has actually changed from last time. Otherwi= se we >>> may be papering over other failure modes. > >> Consider this sequence of events: > > Incidentally, the code review rule of thumb that led me to > construct that scenario is: > * state inside the device struct which doesn't correspond > to real hardware state is suspicious > * state which doesn't have any handling on migration > save/restore is doubly suspicious > ...and then it was just a matter of "find the situation > where this is broken" :-) > > If we want to avoid making syscalls back into KVM it might > be better to attack the problem in the GIC object rather > than in all the devices that might be connected to it. > In general QEMU devices tend to assume they can just > always call qemu_set_irq() and it's the other end's > job to avoid doing anything too expensive in that situation. =46air enough - I'll drop this patch on the re-spin > > -- PMM --=20 Alex Benn=C3=A9e From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36757) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWMzZ-0004As-8x for qemu-devel@nongnu.org; Fri, 13 Mar 2015 06:38:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YWMzW-00087f-0g for qemu-devel@nongnu.org; Fri, 13 Mar 2015 06:38:57 -0400 Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:42845 helo=socrates.bennee.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YWMzV-00087R-R6 for qemu-devel@nongnu.org; Fri, 13 Mar 2015 06:38:53 -0400 References: <1425479753-18349-1-git-send-email-alex.bennee@linaro.org> <1425479753-18349-4-git-send-email-alex.bennee@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: Date: Fri, 13 Mar 2015 10:38:58 +0000 Message-ID: <87ioe5b2rh.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 3/6] hw/char: pl011 don't keep setting the IRQ if nothing changed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Marc Zyngier , "kvmarm@lists.cs.columbia.edu" , QEMU Developers , kvm-devel , arm-mail-list Peter Maydell writes: > On 12 March 2015 at 15:51, Peter Maydell wrote: >> On 4 March 2015 at 14:35, Alex Bennée wrote: >>> While observing KVM traces I can see additional IRQ calls on pretty much >>> every MMIO access which is just plain inefficient. Only update the QEMU >>> IRQ level if something has actually changed from last time. Otherwise we >>> may be papering over other failure modes. > >> Consider this sequence of events: > > Incidentally, the code review rule of thumb that led me to > construct that scenario is: > * state inside the device struct which doesn't correspond > to real hardware state is suspicious > * state which doesn't have any handling on migration > save/restore is doubly suspicious > ...and then it was just a matter of "find the situation > where this is broken" :-) > > If we want to avoid making syscalls back into KVM it might > be better to attack the problem in the GIC object rather > than in all the devices that might be connected to it. > In general QEMU devices tend to assume they can just > always call qemu_set_irq() and it's the other end's > job to avoid doing anything too expensive in that situation. Fair enough - I'll drop this patch on the re-spin > > -- PMM -- Alex Bennée From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex =?utf-8?Q?Benn=C3=A9e?= Subject: Re: [PATCH v2 3/6] hw/char: pl011 don't keep setting the IRQ if nothing changed Date: Fri, 13 Mar 2015 10:38:58 +0000 Message-ID: <87ioe5b2rh.fsf@linaro.org> References: <1425479753-18349-1-git-send-email-alex.bennee@linaro.org> <1425479753-18349-4-git-send-email-alex.bennee@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-reply-to: Sender: kvm-owner@vger.kernel.org To: Peter Maydell Cc: QEMU Developers , kvm-devel , Marc Zyngier , arm-mail-list , "kvmarm@lists.cs.columbia.edu" List-Id: kvmarm@lists.cs.columbia.edu Peter Maydell writes: > On 12 March 2015 at 15:51, Peter Maydell w= rote: >> On 4 March 2015 at 14:35, Alex Benn=C3=A9e = wrote: >>> While observing KVM traces I can see additional IRQ calls on pretty= much >>> every MMIO access which is just plain inefficient. Only update the = QEMU >>> IRQ level if something has actually changed from last time. Otherwi= se we >>> may be papering over other failure modes. > >> Consider this sequence of events: > > Incidentally, the code review rule of thumb that led me to > construct that scenario is: > * state inside the device struct which doesn't correspond > to real hardware state is suspicious > * state which doesn't have any handling on migration > save/restore is doubly suspicious > ...and then it was just a matter of "find the situation > where this is broken" :-) > > If we want to avoid making syscalls back into KVM it might > be better to attack the problem in the GIC object rather > than in all the devices that might be connected to it. > In general QEMU devices tend to assume they can just > always call qemu_set_irq() and it's the other end's > job to avoid doing anything too expensive in that situation. =46air enough - I'll drop this patch on the re-spin > > -- PMM --=20 Alex Benn=C3=A9e From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (Alex =?utf-8?Q?Benn=C3=A9e?=) Date: Fri, 13 Mar 2015 10:38:58 +0000 Subject: [PATCH v2 3/6] hw/char: pl011 don't keep setting the IRQ if nothing changed In-Reply-To: References: <1425479753-18349-1-git-send-email-alex.bennee@linaro.org> <1425479753-18349-4-git-send-email-alex.bennee@linaro.org> Message-ID: <87ioe5b2rh.fsf@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Peter Maydell writes: > On 12 March 2015 at 15:51, Peter Maydell wrote: >> On 4 March 2015 at 14:35, Alex Benn?e wrote: >>> While observing KVM traces I can see additional IRQ calls on pretty much >>> every MMIO access which is just plain inefficient. Only update the QEMU >>> IRQ level if something has actually changed from last time. Otherwise we >>> may be papering over other failure modes. > >> Consider this sequence of events: > > Incidentally, the code review rule of thumb that led me to > construct that scenario is: > * state inside the device struct which doesn't correspond > to real hardware state is suspicious > * state which doesn't have any handling on migration > save/restore is doubly suspicious > ...and then it was just a matter of "find the situation > where this is broken" :-) > > If we want to avoid making syscalls back into KVM it might > be better to attack the problem in the GIC object rather > than in all the devices that might be connected to it. > In general QEMU devices tend to assume they can just > always call qemu_set_irq() and it's the other end's > job to avoid doing anything too expensive in that situation. Fair enough - I'll drop this patch on the re-spin > > -- PMM -- Alex Benn?e