linux-kernel - Re: [PATCH 1/6] clocksource: arm_arch_timer: Migrate to new 'set-state' interface

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:	Thu, 11 Jun 2015 18:27:33 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	linaro-kernel@...ts.linaro.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	Marc Zyngier <marc.zyngier@....com>
Subject: Re: [PATCH 1/6] clocksource: arm_arch_timer: Migrate to new
 'set-state' interface

On 11-06-15, 14:23, Daniel Lezcano wrote:
> [ ... ]
> 
> Instead of returning zero in the functions
> 'timer_shutdown_virt|phys|virt_mem|phys_mem', do it here directly.
> 

> 
> As suggested above:
> 
> 	return timer_shutdown(ARCH_TIMER_VIRT_ACCESS, clk);
> 
> >  }

Sure.

> >@@ -286,17 +286,18 @@ static void __arch_timer_setup(unsigned type,

> >-	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, clk);
> >+	clk->set_state_oneshot = arch_timer_set_oneshot;
> >+	clk->set_state_shutdown(clk);
> 
> Why don't you call clockevent_shutdown(clk) ?

I haven't changed the behavior as the current code doesn't do it that
way. If we want to change it, then it should be a separate patch.

Having said that, in this particular instance the device is not yet
registered with the core and we probably didn't wanted 'mode' to be
set to SHUTDOWN. That would have also hit a WARN/BUG as clockevents
core expected the device to be in UNUSED mode at registration.

> There is some initialization there, no ?

We don't want it at this point of time.

> >  	clockevents_config_and_register(clk, arch_timer_rate, 0xf, 0x7fffffff);
> >  }
> >@@ -506,7 +507,7 @@ static void arch_timer_stop(struct clock_event_device *clk)
> >  			disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
> >  	}
> >
> >-	clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk);
> >+	clk->set_state_shutdown(clk);
> >  }
> 
> Ditto.

This is called from the CPU_DYING notifier, not sure what the
intention was behind not letting the clockevents core about mode
change.

But if it has to change, then it should happen in a separate patch.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/