From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Lezcano Subject: Re: [PATCH V9 1/3] irq: Allow to pass the IRQF_TIMER flag with percpu irq request Date: Mon, 24 Apr 2017 21:59:48 +0200 Message-ID: <20170424195948.GE2137@mai> References: <1493042494-14057-1-git-send-email-daniel.lezcano@linaro.org> <398f3f3d-c567-0f1f-1a43-9b8d5805d5fd@arm.com> <20170424185909.GD2137@mai> <92e2a022-93d4-d4f3-78af-c9b5d51bb867@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 2EB3A40DA5 for ; Mon, 24 Apr 2017 15:57:07 -0400 (EDT) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0cXePA4yfbeP for ; Mon, 24 Apr 2017 15:57:05 -0400 (EDT) Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 4597040D88 for ; Mon, 24 Apr 2017 15:57:05 -0400 (EDT) Received: by mail-wm0-f41.google.com with SMTP id w64so6091713wma.0 for ; Mon, 24 Apr 2017 12:59:53 -0700 (PDT) Content-Disposition: inline In-Reply-To: <92e2a022-93d4-d4f3-78af-c9b5d51bb867@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Marc Zyngier Cc: linux-samsung-soc@vger.kernel.org, kernel@stlinux.com, kvm@vger.kernel.org, Vineet Gupta , Patrice Chotard , Krzysztof Kozlowski , linux-kernel@vger.kernel.org, Javier Martinez Canillas , Kukjin Kim , linux-arm-kernel@lists.infradead.org, Paolo Bonzini , tglx@linutronix.de, linux-snps-arc@lists.infradead.org, kvmarm@lists.cs.columbia.edu List-Id: kvmarm@lists.cs.columbia.edu T24gTW9uLCBBcHIgMjQsIDIwMTcgYXQgMDg6MTQ6NTRQTSArMDEwMCwgTWFyYyBaeW5naWVyIHdy b3RlOgo+IE9uIDI0LzA0LzE3IDE5OjU5LCBEYW5pZWwgTGV6Y2FubyB3cm90ZToKPiA+IE9uIE1v biwgQXByIDI0LCAyMDE3IGF0IDA3OjQ2OjQzUE0gKzAxMDAsIE1hcmMgWnluZ2llciB3cm90ZToK PiA+PiBPbiAyNC8wNC8xNyAxNTowMSwgRGFuaWVsIExlemNhbm8gd3JvdGU6Cj4gPj4+IEluIHRo ZSBuZXh0IGNoYW5nZXMsIHdlIHRyYWNrIHdoZW4gdGhlIGludGVycnVwdHMgb2NjdXIgaW4gb3Jk ZXIgdG8KPiA+Pj4gc3RhdGlzdGljYWxseSBjb21wdXRlIHdoZW4gaXMgc3VwcG9zZWQgdG8gaGFw cGVuIHRoZSBuZXh0IGludGVycnVwdC4KPiA+Pj4KPiA+Pj4gSW4gYWxsIHRoZSBpbnRlcnJ1cHRp b25zLCBpdCBkb2VzIG5vdCBtYWtlIHNlbnNlIHRvIHN0b3JlIHRoZSB0aW1lciBpbnRlcnJ1cHQK PiA+Pj4gb2NjdXJlbmNlcyBhbmQgdHJ5IHRvIHByZWRpY3QgdGhlIG5leHQgaW50ZXJydXB0IGFz IHdoZW4ga25vdyB0aGUgZXhwaXJhdGlvbgo+ID4+PiB0aW1lLgo+ID4+Pgo+ID4+PiBUaGUgcmVx dWVzdF9pcnEoKSBoYXMgYSBpcnEgZmxhZ3MgcGFyYW1ldGVyIGFuZCB0aGUgdGltZXIgZHJpdmVy cyB1c2UgaXQgdG8KPiA+Pj4gcGFzcyB0aGUgSVJRRl9USU1FUiBmbGFnLCBsZXR0aW5nIHVzIGtu b3cgdGhlIGludGVycnVwdCBpcyBjb21pbmcgZnJvbSBhIHRpbWVyLgo+ID4+PiBCYXNlZCBvbiB0 aGlzIGZsYWcsIHdlIGNhbiBkaXNjYXJkIHRoZXNlIGludGVycnVwdHMgd2hlbiB0cmFja2luZyB0 aGVtLgo+ID4+Pgo+ID4+PiBCdXQsIHRoZSBBUEkgcmVxdWVzdF9wZXJjcHVfaXJxIGRvZXMgbm90 IGFsbG93IHRvIHBhc3MgYSBmbGFnLCBoZW5jZSBzcGVjaWZ5aW5nCj4gPj4+IGlmIHRoZSBpbnRl cnJ1cHQgdHlwZSBpcyBhIHRpbWVyLgo+ID4+Pgo+ID4+PiBBZGQgYSBmdW5jdGlvbiByZXF1ZXN0 X3BlcmNwdV9pcnFfZmxhZ3MoKSB3aGVyZSB3ZSBjYW4gc3BlY2lmeSB0aGUgZmxhZ3MuIFRoZQo+ ID4+PiByZXF1ZXN0X3BlcmNwdV9pcnEoKSBmdW5jdGlvbiBpcyBjaGFuZ2VkIHRvIGJlIGEgd3Jh cHBlciB0bwo+ID4+PiByZXF1ZXN0X3BlcmNwdV9pcnFfZmxhZ3MoKSBwYXNzaW5nIGEgemVybyBm bGFnIHBhcmFtZXRlci4KPiA+Pj4KPiA+Pj4gQ2hhbmdlIHRoZSB0aW1lcnMgdXNpbmcgcmVxdWVz dF9wZXJjcHVfaXJxKCkgdG8gdXNlIHJlcXVlc3RfcGVyY3B1X2lycV9mbGFncygpCj4gPj4+IGlu c3RlYWQgd2l0aCB0aGUgSVJRRl9USU1FUiBmbGFnIHNldC4KPiA+Pj4KPiA+Pj4gRm9yIG5vdywg aW4gb3JkZXIgdG8gcHJldmVudCBhIG1pc3VzYWdlIG9mIHRoaXMgcGFyYW1ldGVyLCBvbmx5IHRo ZSBJUlFGX1RJTUVSCj4gPj4+IGZsYWcgKG9yIHplcm8pIGlzIGEgdmFsaWQgcGFyYW1ldGVyIHRv IGJlIHBhc3NlZCB0byB0aGUKPiA+Pj4gcmVxdWVzdF9wZXJjcHVfaXJxX2ZsYWdzKCkgZnVuY3Rp b24uCj4gPj4KPiA+PiBbLi4uXQo+ID4+Cj4gPj4+IGRpZmYgLS1naXQgYS92aXJ0L2t2bS9hcm0v YXJjaF90aW1lci5jIGIvdmlydC9rdm0vYXJtL2FyY2hfdGltZXIuYwo+ID4+PiBpbmRleCAzNWQ3 MTAwLi42MDJlMGE4IDEwMDY0NAo+ID4+PiAtLS0gYS92aXJ0L2t2bS9hcm0vYXJjaF90aW1lci5j Cj4gPj4+ICsrKyBiL3ZpcnQva3ZtL2FybS9hcmNoX3RpbWVyLmMKPiA+Pj4gQEAgLTUyMyw4ICs1 MjMsOSBAQCBpbnQga3ZtX3RpbWVyX2h5cF9pbml0KHZvaWQpCj4gPj4+ICAJCWhvc3RfdnRpbWVy X2lycV9mbGFncyA9IElSUUZfVFJJR0dFUl9MT1c7Cj4gPj4+ICAJfQo+ID4+PiAgCj4gPj4+IC0J ZXJyID0gcmVxdWVzdF9wZXJjcHVfaXJxKGhvc3RfdnRpbWVyX2lycSwga3ZtX2FyY2hfdGltZXJf aGFuZGxlciwKPiA+Pj4gLQkJCQkgImt2bSBndWVzdCB0aW1lciIsIGt2bV9nZXRfcnVubmluZ192 Y3B1cygpKTsKPiA+Pj4gKwllcnIgPSByZXF1ZXN0X3BlcmNwdV9pcnFfZmxhZ3MoaG9zdF92dGlt ZXJfaXJxLCBrdm1fYXJjaF90aW1lcl9oYW5kbGVyLAo+ID4+PiArCQkJCSAgICAgICBJUlFGX1RJ TUVSLCAia3ZtIGd1ZXN0IHRpbWVyIiwKPiA+Pj4gKwkJCQkgICAgICAga3ZtX2dldF9ydW5uaW5n X3ZjcHVzKCkpOwo+ID4+PiAgCWlmIChlcnIpIHsKPiA+Pj4gIAkJa3ZtX2Vycigia3ZtX2FyY2hf dGltZXI6IGNhbid0IHJlcXVlc3QgaW50ZXJydXB0ICVkICglZClcbiIsCj4gPj4+ICAJCQlob3N0 X3Z0aW1lcl9pcnEsIGVycik7Cj4gPj4+Cj4gPj4KPiA+PiBIb3cgaXMgdGhhdCB1c2VmdWw/IFRo aXMgdGltZXIgaXMgY29udHJvbGxlZCBieSB0aGUgZ3Vlc3QgT1MsIGFuZCBub3QKPiA+PiB0aGUg aG9zdCBrZXJuZWwuIENhbiB5b3UgZXhwbGFpbiBob3cgeW91IGludGVuZCB0byBtYWtlIHVzZSBv ZiB0aGF0Cj4gPj4gaW5mb3JtYXRpb24gaW4gdGhpcyBjYXNlPwo+ID4gCj4gPiBJc24ndCBpdCBh IHNvdXJjZSBvZiBpbnRlcnJ1cHRpb24gb24gdGhlIGhvc3Qga2VybmVsPwo+IAo+IE9ubHkgdG8g Y2F1c2UgYW4gZXhpdCBvZiB0aGUgVk0sIGFuZCBub3QgdW5kZXIgdGhlIGNvbnRyb2wgb2YgdGhl IGhvc3QuCj4gVGhpcyBpc24ndCB0cmlnZ2VyaW5nIGFueSB0aW1lciByZWxhdGVkIGFjdGlvbiBv biB0aGUgaG9zdCBjb2RlIGVpdGhlci4KPiAKPiBZb3VyIHBhdGNoIHNlcmllcyBzZWVtcyB0byBh c3N1bWUgc29tZSBraW5kIG9mIHByZWRpY3RhYmlsaXR5IG9mIHRoZQo+IHRpbWVyIGludGVycnVw dCwgd2hpY2ggY2FuIG1ha2Ugc2Vuc2Ugb24gdGhlIGhvc3QuIEhlcmUsIHRoaXMgaW50ZXJydXB0 Cj4gaXMgc2hhcmVkIGFtb25nICphbGwqIGd1ZXN0cyBydW5uaW5nIG9uIHRoaXMgc3lzdGVtLgo+ IAo+IE1heWJlIHlvdSBjb3VsZCBleHBsYWluIHdoeSB5b3UgdGhpbmsgdGhpcyBpbnRlcnJ1cHQg aXMgcmVsZXZhbnQgdG8gd2hhdAo+IHlvdSdyZSB0cnlpbmcgdG8gYWNoaWV2ZT8KCklmIHRoaXMg aW50ZXJydXB0IGRvZXMgbm90IGhhcHBlbiBvbiB0aGUgaG9zdCwgd2UgZG9uJ3QgY2FyZS4KClRo ZSBmbGFnIElSUUZfVElNRVIgaXMgdXNlZCBieSB0aGUgc3B1cmlvdXMgaXJxIGhhbmRsZXIgaW4g dGhlIHRyeV9vbmVfaXJxKCkKZnVuY3Rpb24uIEhvd2V2ZXIgdGhlIHBlciBjcHUgdGltZXIgaW50 ZXJydXB0IHdpbGwgYmUgZGlzY2FyZGVkIGluIHRoZSBmdW5jdGlvbgpiZWZvcmUgYmVjYXVzZSBp dCBpcyBwZXIgY3B1LgoKSU1PLCBmb3IgY29uc2lzdGVuY3kgcmVhc29uLCBhZGRpbmcgdGhlIElS UUZfVElNRVIgbWFrZXMgc2Vuc2UuIE90aGVyIHRoYW4KdGhhdCwgYXMgdGhlIGludGVycnVwdCBp cyBub3QgaGFwcGVuaW5nIG9uIHRoZSBob3N0LCB0aGlzIGZsYWcgd29uJ3QgYmUgdXNlZC4KCkRv IHlvdSB3YW50IHRvIGRyb3AgdGhpcyBjaGFuZ2U/CgogIC0tIERhbmllbAoKCgotLSAKCiA8aHR0 cDovL3d3dy5saW5hcm8ub3JnLz4gTGluYXJvLm9yZyDilIIgT3BlbiBzb3VyY2Ugc29mdHdhcmUg Zm9yIEFSTSBTb0NzCgpGb2xsb3cgTGluYXJvOiAgPGh0dHA6Ly93d3cuZmFjZWJvb2suY29tL3Bh Z2VzL0xpbmFybz4gRmFjZWJvb2sgfAo8aHR0cDovL3R3aXR0ZXIuY29tLyMhL2xpbmFyb29yZz4g VHdpdHRlciB8CjxodHRwOi8vd3d3LmxpbmFyby5vcmcvbGluYXJvLWJsb2cvPiBCbG9nCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmt2bWFybSBtYWlsaW5n IGxpc3QKa3ZtYXJtQGxpc3RzLmNzLmNvbHVtYmlhLmVkdQpodHRwczovL2xpc3RzLmNzLmNvbHVt YmlhLmVkdS9tYWlsbWFuL2xpc3RpbmZvL2t2bWFybQo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel.lezcano@linaro.org (Daniel Lezcano) Date: Mon, 24 Apr 2017 21:59:48 +0200 Subject: [PATCH V9 1/3] irq: Allow to pass the IRQF_TIMER flag with percpu irq request In-Reply-To: <92e2a022-93d4-d4f3-78af-c9b5d51bb867@arm.com> References: <1493042494-14057-1-git-send-email-daniel.lezcano@linaro.org> <398f3f3d-c567-0f1f-1a43-9b8d5805d5fd@arm.com> <20170424185909.GD2137@mai> <92e2a022-93d4-d4f3-78af-c9b5d51bb867@arm.com> List-ID: Message-ID: <20170424195948.GE2137@mai> To: linux-snps-arc@lists.infradead.org On Mon, Apr 24, 2017@08:14:54PM +0100, Marc Zyngier wrote: > On 24/04/17 19:59, Daniel Lezcano wrote: > > On Mon, Apr 24, 2017@07:46:43PM +0100, Marc Zyngier wrote: > >> On 24/04/17 15:01, Daniel Lezcano wrote: > >>> In the next changes, we track when the interrupts occur in order to > >>> statistically compute when is supposed to happen the next interrupt. > >>> > >>> In all the interruptions, it does not make sense to store the timer interrupt > >>> occurences and try to predict the next interrupt as when know the expiration > >>> time. > >>> > >>> The request_irq() has a irq flags parameter and the timer drivers use it to > >>> pass the IRQF_TIMER flag, letting us know the interrupt is coming from a timer. > >>> Based on this flag, we can discard these interrupts when tracking them. > >>> > >>> But, the API request_percpu_irq does not allow to pass a flag, hence specifying > >>> if the interrupt type is a timer. > >>> > >>> Add a function request_percpu_irq_flags() where we can specify the flags. The > >>> request_percpu_irq() function is changed to be a wrapper to > >>> request_percpu_irq_flags() passing a zero flag parameter. > >>> > >>> Change the timers using request_percpu_irq() to use request_percpu_irq_flags() > >>> instead with the IRQF_TIMER flag set. > >>> > >>> For now, in order to prevent a misusage of this parameter, only the IRQF_TIMER > >>> flag (or zero) is a valid parameter to be passed to the > >>> request_percpu_irq_flags() function. > >> > >> [...] > >> > >>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c > >>> index 35d7100..602e0a8 100644 > >>> --- a/virt/kvm/arm/arch_timer.c > >>> +++ b/virt/kvm/arm/arch_timer.c > >>> @@ -523,8 +523,9 @@ int kvm_timer_hyp_init(void) > >>> host_vtimer_irq_flags = IRQF_TRIGGER_LOW; > >>> } > >>> > >>> - err = request_percpu_irq(host_vtimer_irq, kvm_arch_timer_handler, > >>> - "kvm guest timer", kvm_get_running_vcpus()); > >>> + err = request_percpu_irq_flags(host_vtimer_irq, kvm_arch_timer_handler, > >>> + IRQF_TIMER, "kvm guest timer", > >>> + kvm_get_running_vcpus()); > >>> if (err) { > >>> kvm_err("kvm_arch_timer: can't request interrupt %d (%d)\n", > >>> host_vtimer_irq, err); > >>> > >> > >> How is that useful? This timer is controlled by the guest OS, and not > >> the host kernel. Can you explain how you intend to make use of that > >> information in this case? > > > > Isn't it a source of interruption on the host kernel? > > Only to cause an exit of the VM, and not under the control of the host. > This isn't triggering any timer related action on the host code either. > > Your patch series seems to assume some kind of predictability of the > timer interrupt, which can make sense on the host. Here, this interrupt > is shared among *all* guests running on this system. > > Maybe you could explain why you think this interrupt is relevant to what > you're trying to achieve? If this interrupt does not happen on the host, we don't care. The flag IRQF_TIMER is used by the spurious irq handler in the try_one_irq() function. However the per cpu timer interrupt will be discarded in the function before because it is per cpu. IMO, for consistency reason, adding the IRQF_TIMER makes sense. Other than that, as the interrupt is not happening on the host, this flag won't be used. Do you want to drop this change? -- Daniel -- Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel.lezcano@linaro.org (Daniel Lezcano) Date: Mon, 24 Apr 2017 21:59:48 +0200 Subject: [PATCH V9 1/3] irq: Allow to pass the IRQF_TIMER flag with percpu irq request In-Reply-To: <92e2a022-93d4-d4f3-78af-c9b5d51bb867@arm.com> References: <1493042494-14057-1-git-send-email-daniel.lezcano@linaro.org> <398f3f3d-c567-0f1f-1a43-9b8d5805d5fd@arm.com> <20170424185909.GD2137@mai> <92e2a022-93d4-d4f3-78af-c9b5d51bb867@arm.com> Message-ID: <20170424195948.GE2137@mai> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Apr 24, 2017 at 08:14:54PM +0100, Marc Zyngier wrote: > On 24/04/17 19:59, Daniel Lezcano wrote: > > On Mon, Apr 24, 2017 at 07:46:43PM +0100, Marc Zyngier wrote: > >> On 24/04/17 15:01, Daniel Lezcano wrote: > >>> In the next changes, we track when the interrupts occur in order to > >>> statistically compute when is supposed to happen the next interrupt. > >>> > >>> In all the interruptions, it does not make sense to store the timer interrupt > >>> occurences and try to predict the next interrupt as when know the expiration > >>> time. > >>> > >>> The request_irq() has a irq flags parameter and the timer drivers use it to > >>> pass the IRQF_TIMER flag, letting us know the interrupt is coming from a timer. > >>> Based on this flag, we can discard these interrupts when tracking them. > >>> > >>> But, the API request_percpu_irq does not allow to pass a flag, hence specifying > >>> if the interrupt type is a timer. > >>> > >>> Add a function request_percpu_irq_flags() where we can specify the flags. The > >>> request_percpu_irq() function is changed to be a wrapper to > >>> request_percpu_irq_flags() passing a zero flag parameter. > >>> > >>> Change the timers using request_percpu_irq() to use request_percpu_irq_flags() > >>> instead with the IRQF_TIMER flag set. > >>> > >>> For now, in order to prevent a misusage of this parameter, only the IRQF_TIMER > >>> flag (or zero) is a valid parameter to be passed to the > >>> request_percpu_irq_flags() function. > >> > >> [...] > >> > >>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c > >>> index 35d7100..602e0a8 100644 > >>> --- a/virt/kvm/arm/arch_timer.c > >>> +++ b/virt/kvm/arm/arch_timer.c > >>> @@ -523,8 +523,9 @@ int kvm_timer_hyp_init(void) > >>> host_vtimer_irq_flags = IRQF_TRIGGER_LOW; > >>> } > >>> > >>> - err = request_percpu_irq(host_vtimer_irq, kvm_arch_timer_handler, > >>> - "kvm guest timer", kvm_get_running_vcpus()); > >>> + err = request_percpu_irq_flags(host_vtimer_irq, kvm_arch_timer_handler, > >>> + IRQF_TIMER, "kvm guest timer", > >>> + kvm_get_running_vcpus()); > >>> if (err) { > >>> kvm_err("kvm_arch_timer: can't request interrupt %d (%d)\n", > >>> host_vtimer_irq, err); > >>> > >> > >> How is that useful? This timer is controlled by the guest OS, and not > >> the host kernel. Can you explain how you intend to make use of that > >> information in this case? > > > > Isn't it a source of interruption on the host kernel? > > Only to cause an exit of the VM, and not under the control of the host. > This isn't triggering any timer related action on the host code either. > > Your patch series seems to assume some kind of predictability of the > timer interrupt, which can make sense on the host. Here, this interrupt > is shared among *all* guests running on this system. > > Maybe you could explain why you think this interrupt is relevant to what > you're trying to achieve? If this interrupt does not happen on the host, we don't care. The flag IRQF_TIMER is used by the spurious irq handler in the try_one_irq() function. However the per cpu timer interrupt will be discarded in the function before because it is per cpu. IMO, for consistency reason, adding the IRQF_TIMER makes sense. Other than that, as the interrupt is not happening on the host, this flag won't be used. Do you want to drop this change? -- Daniel -- Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1173892AbdDXUAJ (ORCPT ); Mon, 24 Apr 2017 16:00:09 -0400 Received: from mail-wm0-f50.google.com ([74.125.82.50]:38266 "EHLO mail-wm0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1164639AbdDXT77 (ORCPT ); Mon, 24 Apr 2017 15:59:59 -0400 Date: Mon, 24 Apr 2017 21:59:48 +0200 From: Daniel Lezcano To: Marc Zyngier Cc: tglx@linutronix.de, Mark Rutland , Vineet Gupta , Patrice Chotard , Kukjin Kim , Krzysztof Kozlowski , Javier Martinez Canillas , Christoffer Dall , Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-snps-arc@lists.infradead.org, kernel@stlinux.com, linux-samsung-soc@vger.kernel.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org Subject: Re: [PATCH V9 1/3] irq: Allow to pass the IRQF_TIMER flag with percpu irq request Message-ID: <20170424195948.GE2137@mai> References: <1493042494-14057-1-git-send-email-daniel.lezcano@linaro.org> <398f3f3d-c567-0f1f-1a43-9b8d5805d5fd@arm.com> <20170424185909.GD2137@mai> <92e2a022-93d4-d4f3-78af-c9b5d51bb867@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <92e2a022-93d4-d4f3-78af-c9b5d51bb867@arm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 24, 2017 at 08:14:54PM +0100, Marc Zyngier wrote: > On 24/04/17 19:59, Daniel Lezcano wrote: > > On Mon, Apr 24, 2017 at 07:46:43PM +0100, Marc Zyngier wrote: > >> On 24/04/17 15:01, Daniel Lezcano wrote: > >>> In the next changes, we track when the interrupts occur in order to > >>> statistically compute when is supposed to happen the next interrupt. > >>> > >>> In all the interruptions, it does not make sense to store the timer interrupt > >>> occurences and try to predict the next interrupt as when know the expiration > >>> time. > >>> > >>> The request_irq() has a irq flags parameter and the timer drivers use it to > >>> pass the IRQF_TIMER flag, letting us know the interrupt is coming from a timer. > >>> Based on this flag, we can discard these interrupts when tracking them. > >>> > >>> But, the API request_percpu_irq does not allow to pass a flag, hence specifying > >>> if the interrupt type is a timer. > >>> > >>> Add a function request_percpu_irq_flags() where we can specify the flags. The > >>> request_percpu_irq() function is changed to be a wrapper to > >>> request_percpu_irq_flags() passing a zero flag parameter. > >>> > >>> Change the timers using request_percpu_irq() to use request_percpu_irq_flags() > >>> instead with the IRQF_TIMER flag set. > >>> > >>> For now, in order to prevent a misusage of this parameter, only the IRQF_TIMER > >>> flag (or zero) is a valid parameter to be passed to the > >>> request_percpu_irq_flags() function. > >> > >> [...] > >> > >>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c > >>> index 35d7100..602e0a8 100644 > >>> --- a/virt/kvm/arm/arch_timer.c > >>> +++ b/virt/kvm/arm/arch_timer.c > >>> @@ -523,8 +523,9 @@ int kvm_timer_hyp_init(void) > >>> host_vtimer_irq_flags = IRQF_TRIGGER_LOW; > >>> } > >>> > >>> - err = request_percpu_irq(host_vtimer_irq, kvm_arch_timer_handler, > >>> - "kvm guest timer", kvm_get_running_vcpus()); > >>> + err = request_percpu_irq_flags(host_vtimer_irq, kvm_arch_timer_handler, > >>> + IRQF_TIMER, "kvm guest timer", > >>> + kvm_get_running_vcpus()); > >>> if (err) { > >>> kvm_err("kvm_arch_timer: can't request interrupt %d (%d)\n", > >>> host_vtimer_irq, err); > >>> > >> > >> How is that useful? This timer is controlled by the guest OS, and not > >> the host kernel. Can you explain how you intend to make use of that > >> information in this case? > > > > Isn't it a source of interruption on the host kernel? > > Only to cause an exit of the VM, and not under the control of the host. > This isn't triggering any timer related action on the host code either. > > Your patch series seems to assume some kind of predictability of the > timer interrupt, which can make sense on the host. Here, this interrupt > is shared among *all* guests running on this system. > > Maybe you could explain why you think this interrupt is relevant to what > you're trying to achieve? If this interrupt does not happen on the host, we don't care. The flag IRQF_TIMER is used by the spurious irq handler in the try_one_irq() function. However the per cpu timer interrupt will be discarded in the function before because it is per cpu. IMO, for consistency reason, adding the IRQF_TIMER makes sense. Other than that, as the interrupt is not happening on the host, this flag won't be used. Do you want to drop this change? -- Daniel -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog