From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AD74DC65BAF for ; Wed, 12 Dec 2018 13:09:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7AFC72086D for ; Wed, 12 Dec 2018 13:09:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7AFC72086D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727559AbeLLNJR (ORCPT ); Wed, 12 Dec 2018 08:09:17 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35074 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726741AbeLLNJQ (ORCPT ); Wed, 12 Dec 2018 08:09:16 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2F62D821CC; Wed, 12 Dec 2018 13:09:16 +0000 (UTC) Received: from vitty.brq.redhat.com.redhat.com (unknown [10.34.244.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 01BA01054FCC; Wed, 12 Dec 2018 13:09:14 +0000 (UTC) From: Vitaly Kuznetsov To: Roman Kagan , Paolo Bonzini Cc: "kvm\@vger.kernel.org" , Radim =?utf-8?B?S3LEjW0=?= =?utf-8?B?w6HFmQ==?= , "linux-kernel\@vger.kernel.org" Subject: Re: [PATCH] x86/hyper-v: Stop caring about EOI for direct stimers In-Reply-To: <20181210183918.GB13294@rkaganb.sw.ru> References: <20181205153621.14343-1-vkuznets@redhat.com> <20181210183918.GB13294@rkaganb.sw.ru> Date: Wed, 12 Dec 2018 14:09:13 +0100 Message-ID: <874lbi28p2.fsf@vitty.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 12 Dec 2018 13:09:16 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Roman Kagan writes: > [ Sorry, missed this one ] > > On Wed, Dec 05, 2018 at 04:36:21PM +0100, Vitaly Kuznetsov wrote: >> Turns out we over-engineered Direct Mode for stimers a bit: unlike >> traditional stimers where we may want to try to re-inject the message upon >> EOI, Direct Mode stimers just set the irq in APIC and kvm_apic_set_irq() >> fails only when APIC is disabled (see APIC_DM_FIXED case in >> __apic_accept_irq()). Remove the redundant part. >> >> Suggested-by: Roman Kagan >> Signed-off-by: Vitaly Kuznetsov >> --- >> arch/x86/kvm/hyperv.c | 36 +++--------------------------------- >> 1 file changed, 3 insertions(+), 33 deletions(-) >> >> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c >> index e6a2a085644a..0a16a77e6ac3 100644 >> --- a/arch/x86/kvm/hyperv.c >> +++ b/arch/x86/kvm/hyperv.c >> @@ -56,21 +56,8 @@ static inline int synic_get_sint_vector(u64 sint_value) >> static bool synic_has_vector_connected(struct kvm_vcpu_hv_synic *synic, >> int vector) >> { >> - struct kvm_vcpu *vcpu = synic_to_vcpu(synic); >> - struct kvm_vcpu_hv *hv_vcpu = vcpu_to_hv_vcpu(vcpu); >> - struct kvm_vcpu_hv_stimer *stimer; >> int i; >> >> - for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++) { >> - stimer = &hv_vcpu->stimer[i]; >> - if (stimer->config.enable && stimer->config.direct_mode && >> - stimer->config.apic_vector == vector) >> - return true; >> - } >> - >> - if (vector < HV_SYNIC_FIRST_VALID_VECTOR) >> - return false; >> - >> for (i = 0; i < ARRAY_SIZE(synic->sint); i++) { >> if (synic_get_sint_vector(synic_read_sint(synic, i)) == vector) >> return true; >> @@ -96,14 +83,14 @@ static bool synic_has_vector_auto_eoi(struct kvm_vcpu_hv_synic *synic, >> static void synic_update_vector(struct kvm_vcpu_hv_synic *synic, >> int vector) >> { >> + if (vector < HV_SYNIC_FIRST_VALID_VECTOR) >> + return; >> + >> if (synic_has_vector_connected(synic, vector)) >> __set_bit(vector, synic->vec_bitmap); >> else >> __clear_bit(vector, synic->vec_bitmap); >> >> - if (vector < HV_SYNIC_FIRST_VALID_VECTOR) >> - return; >> - >> if (synic_has_vector_auto_eoi(synic, vector)) >> __set_bit(vector, synic->auto_eoi_bitmap); >> else >> @@ -382,9 +369,7 @@ int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vpidx, u32 sint) >> >> void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector) >> { >> - struct kvm_vcpu_hv *hv_vcpu = vcpu_to_hv_vcpu(vcpu); >> struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu); >> - struct kvm_vcpu_hv_stimer *stimer; >> int i; >> >> trace_kvm_hv_synic_send_eoi(vcpu->vcpu_id, vector); >> @@ -392,14 +377,6 @@ void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector) >> for (i = 0; i < ARRAY_SIZE(synic->sint); i++) >> if (synic_get_sint_vector(synic_read_sint(synic, i)) == vector) >> kvm_hv_notify_acked_sint(vcpu, i); >> - >> - for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++) { >> - stimer = &hv_vcpu->stimer[i]; >> - if (stimer->msg_pending && stimer->config.enable && >> - stimer->config.direct_mode && >> - stimer->config.apic_vector == vector) >> - stimer_mark_pending(stimer, false); >> - } >> } >> >> static int kvm_hv_set_sint_gsi(struct kvm *kvm, u32 vpidx, u32 sint, int gsi) >> @@ -566,8 +543,6 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer) >> static int stimer_set_config(struct kvm_vcpu_hv_stimer *stimer, u64 config, >> bool host) >> { >> - struct kvm_vcpu *vcpu = stimer_to_vcpu(stimer); >> - struct kvm_vcpu_hv *hv_vcpu = vcpu_to_hv_vcpu(vcpu); >> union hv_stimer_config new_config = {.as_uint64 = config}, >> old_config = {.as_uint64 = stimer->config.as_uint64}; >> >> @@ -580,11 +555,6 @@ static int stimer_set_config(struct kvm_vcpu_hv_stimer *stimer, u64 config, >> new_config.enable = 0; >> stimer->config.as_uint64 = new_config.as_uint64; >> >> - if (old_config.direct_mode) >> - synic_update_vector(&hv_vcpu->synic, old_config.apic_vector); >> - if (new_config.direct_mode) >> - synic_update_vector(&hv_vcpu->synic, new_config.apic_vector); >> - >> stimer_mark_pending(stimer, false); >> return 0; >> } > > As discussed in another thread, it seems worth while to make > stimer_set_config reject vectors 0..15. > > Besides I'd rather sqwash this patch into the one that introduced direct > timers, before it reached Linus' tree. I'm fine either way, I'm going to send v2 with a new patch adding the check to stimer_set_config() and leave it up to Paolo if he would want to squash them all together before sending to Linus. (Actually I don't see direct stimers in kvm/queue but I think I heard 'queued' from Paolo...) -- Vitaly