netdev - RE: [PATCH v6 09/11] pps: generators: Add PPS Generator TIO 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] [day] [month] [year] [list]

Message-ID: <CY8PR11MB7364070B11D5D9E4711613F0C4082@CY8PR11MB7364.namprd11.prod.outlook.com>
Date: Tue, 16 Apr 2024 10:19:44 +0000
From: "D, Lakshmi Sowjanya" <lakshmi.sowjanya.d@...el.com>
To: Thomas Gleixner <tglx@...utronix.de>, "jstultz@...gle.com"
	<jstultz@...gle.com>, "giometti@...eenne.com" <giometti@...eenne.com>,
	"corbet@....net" <corbet@....net>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
CC: "x86@...nel.org" <x86@...nel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, "andriy.shevchenko@...ux.intel.com"
	<andriy.shevchenko@...ux.intel.com>, "Dong, Eddie" <eddie.dong@...el.com>,
	"Hall, Christopher S" <christopher.s.hall@...el.com>, "Brandeburg, Jesse"
	<jesse.brandeburg@...el.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
	"joabreu@...opsys.com" <joabreu@...opsys.com>, "mcoquelin.stm32@...il.com"
	<mcoquelin.stm32@...il.com>, "perex@...ex.cz" <perex@...ex.cz>,
	"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>, "Nguyen, Anthony
 L" <anthony.l.nguyen@...el.com>, "peter.hilber@...nsynergy.com"
	<peter.hilber@...nsynergy.com>, "N, Pandith" <pandith.n@...el.com>, "Mohan,
 Subramanian" <subramanian.mohan@...el.com>, "T R, Thejesh Reddy"
	<thejesh.reddy.t.r@...el.com>
Subject: RE: [PATCH v6 09/11] pps: generators: Add PPS Generator TIO Driver



> -----Original Message-----
> From: Thomas Gleixner <tglx@...utronix.de>
> Sent: Thursday, April 11, 2024 3:59 AM
> To: D, Lakshmi Sowjanya <lakshmi.sowjanya.d@...el.com>;
> jstultz@...gle.com; giometti@...eenne.com; corbet@....net; linux-
> kernel@...r.kernel.org
> Cc: x86@...nel.org; netdev@...r.kernel.org; linux-doc@...r.kernel.org; intel-
> wired-lan@...ts.osuosl.org; andriy.shevchenko@...ux.intel.com; Dong, Eddie
> <eddie.dong@...el.com>; Hall, Christopher S <christopher.s.hall@...el.com>;
> Brandeburg, Jesse <jesse.brandeburg@...el.com>; davem@...emloft.net;
> alexandre.torgue@...s.st.com; joabreu@...opsys.com;
> mcoquelin.stm32@...il.com; perex@...ex.cz; linux-sound@...r.kernel.org;
> Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> peter.hilber@...nsynergy.com; N, Pandith <pandith.n@...el.com>; Mohan,
> Subramanian <subramanian.mohan@...el.com>; T R, Thejesh Reddy
> <thejesh.reddy.t.r@...el.com>; D, Lakshmi Sowjanya
> <lakshmi.sowjanya.d@...el.com>
> Subject: Re: [PATCH v6 09/11] pps: generators: Add PPS Generator TIO Driver
> 
> On Wed, Apr 10 2024 at 17:18, lakshmi.sowjanya.d@...el.com wrote:
> > +static bool pps_generate_next_pulse(struct pps_tio *tio, ktime_t
> > +expires) {
> > +	u64 art;
> > +
> > +	if (!ktime_real_to_base_clock(expires, CSID_X86_ART, &art)) {
> > +		pps_tio_disable(tio);
> > +		return false;
> > +	}
> > +
> > +	pps_compv_write(tio, art - ART_HW_DELAY_CYCLES);
> > +	return true;
> > +}
> > +
> > +static enum hrtimer_restart hrtimer_callback(struct hrtimer *timer) {
> > +	struct pps_tio *tio = container_of(timer, struct pps_tio, timer);
> > +	ktime_t expires, now;
> > +	u32 event_count;
> > +
> > +	guard(spinlock)(&tio->lock);
> > +
> > +	/* Check if any event is missed. If an event is missed, TIO will be
> disabled*/
> > +	event_count = pps_tio_read(tio, TIOEC);
> > +	if (!tio->prev_count && tio->prev_count == event_count)
> > +		goto err;
> > +	tio->prev_count = event_count;
> > +	expires = hrtimer_get_expires(timer);
> > +	now = ktime_get_real();
> > +
> > +	if (now - expires < SAFE_TIME_NS) {
> > +		if (!pps_generate_next_pulse(tio, expires + SAFE_TIME_NS))
> > +			goto err;
> > +	}
> 
> If the hrtimer callback is invoked late so that now - expires is >= SAFE_TIME_NS
> then this just forwards the timer and tries again.

Yes we will introduce a return HRTIMER_NORESTART if the time is expired.

> 
> This lacks any form of explanation why this is correct as obviously there will be a
> missing pulse, no?

We had added an event count check to detect the missed pulse(i.e if we had programmed an expired time). 
Timed I/O hardware has an event count register to log the number of pulses generated.

> 
> > +	hrtimer_forward(timer, now, NSEC_PER_SEC / 2);
> > +	return HRTIMER_RESTART;
> > +err:
> > +	dev_err(tio->dev, "Event missed, Disabling Timed I/O");
> > +	pps_tio_disable(tio);
> 
> Why does this disable it again? The failure path in
> pps_generate_next_pulse() does so already, no?

will remove disabling twice in the next version of patchset.

> 
> > +	return HRTIMER_NORESTART;
> > +}
> > +
> 
> Thanks,
> 
>         tglx