From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Michael Kelley <mikelley@microsoft.com>
Cc: "catalin.marinas\@arm.com" <catalin.marinas@arm.com>,
"mark.rutland\@arm.com" <mark.rutland@arm.com>,
"will.deacon\@arm.com" <will.deacon@arm.com>,
"marc.zyngier\@arm.com" <marc.zyngier@arm.com>,
"linux-arm-kernel\@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"gregkh\@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-hyperv\@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
"olaf\@aepfle.de" <olaf@aepfle.de>,
"apw\@canonical.com" <apw@canonical.com>,
"jasowang\@redhat.com" <jasowang@redhat.com>,
"marcelo.cerri\@canonical.com" <marcelo.cerri@canonical.com>,
Sunil Muthuswamy <sunilmut@microsoft.com>,
KY Srinivasan <kys@microsoft.com>
Subject: RE: [PATCH 1/2] Drivers: hv: Move Hyper-V clockevents code to new clocksource driver
Date: Wed, 13 Mar 2019 18:17:15 +0100 [thread overview]
Message-ID: <87mulysnhg.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <DM5PR2101MB09180753D788F8587B19B993D74A0@DM5PR2101MB0918.namprd21.prod.outlook.com>
Michael Kelley <mikelley@microsoft.com> writes:
> From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Wednesday, March 13, 2019 7:23 AM
>
>> >> > 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).
>>
>
> That line just controls whether hyperv_syntimer.o is built. It doesn't put
> it in the hv_vmbus module. All of the clocksource .o files that are built go
> into the kernel, not in a module. But thinking about it more, the above works
> correctly when CONFIG_HYPERV=y, but not when CONFIG_HYPERV=m.
Yes, that's what I meant.
> I'll have to introduce CONFIG_HYPERV_TIMER after all. Will fix this in v2. Thanks
> for the discussion!
Thanks!
--
Vitaly
next prev parent reply other threads:[~2019-03-13 17:17 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-12 21:42 [PATCH 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate clocksource driver Michael Kelley
2019-03-12 21:42 ` [PATCH 1/2] Drivers: hv: Move Hyper-V clockevents code to new " Michael Kelley
2019-03-13 8:28 ` Vitaly Kuznetsov
2019-03-13 13:38 ` Michael Kelley
2019-03-13 14:23 ` Vitaly Kuznetsov
2019-03-13 16:19 ` Michael Kelley
2019-03-13 17:17 ` Vitaly Kuznetsov [this message]
2019-03-12 21:42 ` [PATCH 2/2] Drivers: hv: Move Hyper-V clocksource " Michael Kelley
2019-03-12 21:46 ` [PATCH 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate " gregkh
2019-03-12 21:53 ` Michael Kelley
2019-03-12 22:01 ` gregkh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87mulysnhg.fsf@vitty.brq.redhat.com \
--to=vkuznets@redhat.com \
--cc=apw@canonical.com \
--cc=catalin.marinas@arm.com \
--cc=gregkh@linuxfoundation.org \
--cc=jasowang@redhat.com \
--cc=kys@microsoft.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=marcelo.cerri@canonical.com \
--cc=mark.rutland@arm.com \
--cc=mikelley@microsoft.com \
--cc=olaf@aepfle.de \
--cc=sunilmut@microsoft.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).