linux-kernel - RE: [PATCH 1/2] Drivers: hv: Move Hyper-V clockevents code to new clocksource driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 Mar 2019 16:19:16 +0000
From:   Michael Kelley <mikelley@...rosoft.com>
To:     vkuznets <vkuznets@...hat.com>
CC:     "catalin.marinas@....com" <catalin.marinas@....com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "will.deacon@....com" <will.deacon@....com>,
        "marc.zyngier@....com" <marc.zyngier@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "olaf@...fle.de" <olaf@...fle.de>,
        "apw@...onical.com" <apw@...onical.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "marcelo.cerri@...onical.com" <marcelo.cerri@...onical.com>,
        Sunil Muthuswamy <sunilmut@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>
Subject: RE: [PATCH 1/2] Drivers: hv: Move Hyper-V clockevents code to new
 clocksource driver

From: Vitaly Kuznetsov <vkuznets@...hat.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.  I'll
have to introduce CONFIG_HYPERV_TIMER after all.  Will fix this in v2.  Thanks
for the discussion!
 
Michael