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=-4.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 56FF1C43381 for ; Wed, 13 Mar 2019 14:23:21 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1CDD6214AE for ; Wed, 13 Mar 2019 14:23:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="EOh5/mJ6" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1CDD6214AE 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-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/zQmhh3RwpB9iPwgaJjXMDzp73dD62XXYPZV7rkjgkE=; b=EOh5/mJ6PxlLOD pUHO9dKvvLqeEI+C6VJ/lNU3Ckpv+PTwO0FHVgEUFzOh4PQLUtN93KcWgZD6J37vDTonee7dmkLs2 QhQ4IeE/+BSyjKGFE/wHTsatUPNDT5yRZb7nWrLoH5DqlpuAEt6RBOax2UsgrfU5dg+7GxaNfcKHn YhOH34INimTIA3qOliRjddHCCx+5Vtt+lILYRnD4LaVgwfGC0P0rrFGylYL3E1mJSZ4CIAslSwc7V vD0zNqHUnA95WrNFpniIf/69EgDCWTJYajwTbd2PwnyLlw6pQYA3X2VGGPnjz+80U8czpJBrU7Omb tB35zABIr5YPuGBtxjOg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h44mo-0001TI-BI; Wed, 13 Mar 2019 14:23:14 +0000 Received: from mail-wr1-f66.google.com ([209.85.221.66]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h44mj-0001SV-LZ for linux-arm-kernel@lists.infradead.org; Wed, 13 Mar 2019 14:23:11 +0000 Received: by mail-wr1-f66.google.com with SMTP id t5so2221064wri.7 for ; Wed, 13 Mar 2019 07:23:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=JhxrnjEHkuyKpKcbEYaXIjWSELRlFJwUywf4gz8eEUk=; b=qNpsQQvFNS+rMiXtoGckwh3rorwNI506gnpwqEB+2LNi8rzdOWxzRN5lCJCaqckRcK M7EHgMnf2fPkk+LKmT30qT4Sn83R36HAlWpORGYZr+FGiXfUVYatlNdw7wHsYkMAhGcD 8kF/17ssjXahM3s9Ce/TCauIKfpC7KGnednW7b5emioIA0a20EKsTL7pkna+Tdy10Pqw KOf/GjXilBccS9CBAGqK7tbjwFfFR6Gieo6Ktt1K3JqiiFOzl/eQXr0zvtNurUcgXPba 9LgSn/0eP5E5Fzq+VTiCA1FOnVliwxmMseFY5GOpoFDwKkAJ0Mko9hNScUUOqHvMkItf Fdow== X-Gm-Message-State: APjAAAWmTq5hYSt8IQPVImxI+Bfgc74wNBEpUYC0w9ykoEadnhTQO4uC vJQG7+CTFgueEfcmWVbfavH8Fg== X-Google-Smtp-Source: APXvYqwTXsmRHHxQw6VZ5anozxEeTAS9ZmnJC+VyIV7EQSB8zaO7m2tpDoeY/K1T8e/s77uvTf3Rvg== X-Received: by 2002:adf:e7c2:: with SMTP id e2mr1672580wrn.275.1552486987828; Wed, 13 Mar 2019 07:23:07 -0700 (PDT) Received: from vitty.brq.redhat.com (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id g2sm9185637wrh.7.2019.03.13.07.23.06 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 13 Mar 2019 07:23:06 -0700 (PDT) From: Vitaly Kuznetsov To: Michael Kelley Subject: RE: [PATCH 1/2] Drivers: hv: Move Hyper-V clockevents code to new clocksource driver In-Reply-To: References: <1552426813-9568-1-git-send-email-mikelley@microsoft.com> <1552426813-9568-2-git-send-email-mikelley@microsoft.com> <874l87tbz2.fsf@vitty.brq.redhat.com> Date: Wed, 13 Mar 2019 15:23:05 +0100 Message-ID: <87pnqusvjq.fsf@vitty.brq.redhat.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190313_072309_706953_3A66CEE8 X-CRM114-Status: GOOD ( 26.06 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "mark.rutland@arm.com" , "linux-hyperv@vger.kernel.org" , "marc.zyngier@arm.com" , "catalin.marinas@arm.com" , "jasowang@redhat.com" , "will.deacon@arm.com" , "linux-kernel@vger.kernel.org" , "marcelo.cerri@canonical.com" , "olaf@aepfle.de" , "gregkh@linuxfoundation.org" , "apw@canonical.com" , Sunil Muthuswamy , KY Srinivasan , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Michael Kelley writes: > From: Vitaly Kuznetsov Sent: Wednesday, March 13, 2019 1:28 AM >> >> > Clockevents code for Hyper-V synthetic timers is currently mixed >> > in with other Hyper-V code. Move the code to a Hyper-V specific >> > driver in the "clocksource" directory. Update the VMbus driver >> > to call initialization and cleanup routines since the Hyper-V >> > synthetic timers are not independently enumerated in ACPI. >> > >> >> I like the idea! Would it also make sense to consider moving Hyper-V >> clocksource from arch/x86/hyperv/hv_init.c? The thing is that we use it >> from arch-independent code (e.g. seee 'hyperv_cs' usage in >> drivers/hv/hv_util.c). > > That's what the second patch in the series does. :-) But let me > know if there's something you think I've missed. > Oh, sorry, it's just me - your subject lines messed with my brain :-) I like your idea even more then! >> >> Nitpicking: >> >> This has nothing to do with your patch but we have a mix of 'stimer', >> 'timer', 'syntimer' names when we refer to Hyper-V Synthetic timers, it >> may make sense to try to converge on something (stimer would probably be >> my personal preference but I don't really care as long as we use the >> same abbreviation). Examples: >> >> hv_init_timer_config(...) >> HV_MSR_SYNTIMER_AVAILABLE >> HYPERV_STIMER0_VECTOR >> hv_syntimer_init(...) >> ... > > Yes, you are right about the mish-mash of names. I also like converging > on "stimer". I'll certainly change things like hv_syntimer_init() to > hv_stimer_init() as part of the patch and avoid making the problem > worse. > > What about the name of the new .c and .h files? They include code > both the stimer-based clockevents, as well as the Hyper-V > reference time source based clocksource. Stay with "syntimer" or > shorten to "stimer", even though the code is slightly broader than > just the stimers? (which highlights the generic Linux naming issue that > "clocksource drivers" include code for both clockevents and > clocksources) "hyperv_timers.*" maybe? (those who read TLFS may get confused because 'Synthetic timers' doesn't refer to clocksources - just to clockevents). > >> >> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile >> > index be6e0fb..a887955 100644 >> > --- a/drivers/clocksource/Makefile >> > +++ b/drivers/clocksource/Makefile >> > @@ -83,3 +83,4 @@ obj-$(CONFIG_ATCPIT100_TIMER) += timer-atcpit100.o >> > obj-$(CONFIG_RISCV_TIMER) += timer-riscv.o >> > obj-$(CONFIG_CSKY_MP_TIMER) += timer-mp-csky.o >> > obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o >> > +obj-$(CONFIG_HYPERV) += hyperv_syntimer.o >> >> (just a couple of spare thoughs) >> >> CONFIG_HYPERV can also be a module, are we OK with that? (we'll have to >> support module loading/unloading then and honestly I see no reason for >> that. I would prefer everything but VMBus devices to be in >> kernel.) If we don't want it to be a module we can create a hidden >> CONFIG_HYPERV_STIMER or something like that - just like we already do >> for CONFIG_HYPERV_TSCPAGE. >> >> There is, however, one additional dependency here: when running in >> non-direct mode, Hyper-V clockevent devices require functional Hyper-V >> messaging - which currently lives in VMBus code so that may explain why >> you may want to keep stimer code in the same entity. Or, alternatively, >> we can move Hyper-V messaging out of VMBus code (is it actually >> architecture-agnostic?) >> > > I thought about introducing CONFIG_HYPERV_STIMER, but in my > judgment it was just unnecessary complexity. The Hyper-V clocksource > driver can't exist independent of Hyper-V, and vice versa. When both the > clocksource and clockevents code is considered, the VMbus driver and > Hyper-V initialization code has to call directly into the driver since the > Hyper-V synthetic timers and reference time counter aren't independently > enumerated. Even if we could get the Hyper-V messaging out of VMbus > code, we would still need the clocksource initialization call directly from > hyperv_init(), which is not in a module (see the 2nd patch of the series). > Right, so hv_init_clocksource() cannot live in hv_vmbus module and we need to somehow prevent hyperv_syntimer.o from going in there. And +obj-$(CONFIG_HYPERV) += hyperv_syntimer.o will do exactly the opposite - put it in hv_vmbus module. Or am I missing something? (I haven't tried to build your code yet, sorry). > Michael -- Vitaly _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel