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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F305EC433EF for ; Fri, 13 May 2022 21:25:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1384662AbiEMVZe (ORCPT ); Fri, 13 May 2022 17:25:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54746 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1358795AbiEMVZa (ORCPT ); Fri, 13 May 2022 17:25:30 -0400 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BCDD8663FD for ; Fri, 13 May 2022 14:25:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1652477128; x=1684013128; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=l6Vls9wnN+x00vvDZEEHtOYdZPzsd8ahPI1ukzsIW6A=; b=nO/Q31FD+tqSRLnr5rJ+KbWanPdla8H2wpg9JU9hCwj0Rp2NQNDknYqO 5khjTFzgOqJh5MychSdM+XAPGLj4cBpNqlr9XQdHqjmB+5uBYRL+zdPhq QMMev1r2nU5Fb6ltC9H16pDLkMWTHvZFk/Otn9kRNLMmA18B69dMkulBt gOVnMlNe8WyL5yoAO1yjnVFT9iNQIzPQk+ciUKp4dqFJStkblWbNDigtp teR7oVJIh9AzI66F7ja0Nx/pkoUuH6DkYMjjJ2z8wmYoSqU/1XeFN5L6L bqgAbl33qPdU6eSH7dxZNeheDeM3RhH1YNZNrdwA+i/NyOH15pFFRnIF3 w==; X-IronPort-AV: E=McAfee;i="6400,9594,10346"; a="270350414" X-IronPort-AV: E=Sophos;i="5.91,223,1647327600"; d="scan'208";a="270350414" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 May 2022 14:25:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,223,1647327600"; d="scan'208";a="712562541" Received: from ranerica-svr.sc.intel.com ([172.25.110.23]) by fmsmga001.fm.intel.com with ESMTP; 13 May 2022 14:25:28 -0700 Date: Fri, 13 May 2022 14:29:03 -0700 From: Ricardo Neri To: Thomas Gleixner Cc: x86@kernel.org, Tony Luck , Andi Kleen , Stephane Eranian , Andrew Morton , Joerg Roedel , Suravee Suthikulpanit , David Woodhouse , Lu Baolu , Nicholas Piggin , "Ravi V. Shankar" , Ricardo Neri , iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 15/29] x86/hpet: Add helper function hpet_set_comparator_periodic() Message-ID: <20220513212903.GF22683@ranerica-svr.sc.intel.com> References: <20220506000008.30892-1-ricardo.neri-calderon@linux.intel.com> <20220506000008.30892-16-ricardo.neri-calderon@linux.intel.com> <87mtfufifa.ffs@tglx> <87ilqifhxj.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ilqifhxj.ffs@tglx> User-Agent: Mutt/1.9.4 (2018-02-28) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 06, 2022 at 11:51:52PM +0200, Thomas Gleixner wrote: > On Fri, May 06 2022 at 23:41, Thomas Gleixner wrote: > > On Thu, May 05 2022 at 16:59, Ricardo Neri wrote: > >> Programming an HPET channel as periodic requires setting the > >> HPET_TN_SETVAL bit in the channel configuration. Plus, the comparator > >> register must be written twice (once for the comparator value and once for > >> the periodic value). Since this programming might be needed in several > >> places (e.g., the HPET clocksource and the HPET-based hardlockup detector), > >> add a helper function for this purpose. > >> > >> A helper function hpet_set_comparator_oneshot() could also be implemented. > >> However, such function would only program the comparator register and the > >> function would be quite small. Hence, it is better to not bloat the code > >> with such an obvious function. > > > > This word salad above does not provide a single reason why the periodic > > programming function is required and better suited for the NMI watchdog > > case and then goes on and blurbs about why a function which is not > > required is not implemented. The argument about not bloating the code > > with an "obvious???" function which is quite small is slightly beyond my > > comprehension level. > > What's even more uncomprehensible is that the patch which actually sets > up that NMI watchdog cruft has: > > > + if (hc->boot_cfg & HPET_TN_PERIODIC_CAP) > > + hld_data->has_periodic = true; > > So how the heck does that work with a HPET which does not support > periodic mode? If the HPET channel does not support periodic mode (as indicated by the flag above), it will read the HPET counter into a local variable, increment that local variable, and write comparator of the HPET channel. If the HPET channel does support periodic mode, it will not kick it again. It will only kick a periodic HPET channel if needed (e.g., if the NMI watchdog was idle of watchdog_thresh changed its value). > > That watchdog muck will still happily invoke that set periodic function > in the hope that it works by chance? It will not. It will check hld_data->has_periodic and act accordingly. FWIW, I have tested this NMI watchdog with periodic and non-periodic HPET channels. Thanks and BR, Ricardo 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 Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 08B9CC433EF for ; Fri, 13 May 2022 21:25:32 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 9FF184015A; Fri, 13 May 2022 21:25:32 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id SVbuDQ9ENxqq; Fri, 13 May 2022 21:25:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id 63B3840241; Fri, 13 May 2022 21:25:31 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1D9CAC0032; Fri, 13 May 2022 21:25:31 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id BF0B3C002D for ; Fri, 13 May 2022 21:25:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id A690E83F2F for ; Fri, 13 May 2022 21:25:29 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp1.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=intel.com Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qVyZF-MunJ6Y for ; Fri, 13 May 2022 21:25:29 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by smtp1.osuosl.org (Postfix) with ESMTPS id 105A483F11 for ; Fri, 13 May 2022 21:25:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1652477129; x=1684013129; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=l6Vls9wnN+x00vvDZEEHtOYdZPzsd8ahPI1ukzsIW6A=; b=M8uYktimlCBQbO8kwV+v9s9Mj+jgIDFRF3kKoF6MwadyiCHL/yHkq7nQ B1QGRizAOML1XLD2ARqlzCQfeVLV/IIgef1xOcnvuofShPNg3ACpsjJ3s szbDdNkijedbtwiJ0W64hzNBNtms2o+dZwphPmM9SSP3EUYyVFDMtocce jUTg4EM07XE3+WYxzuwd9FXDNvsOk+64qZAVkyLVr94K6q9tq+Tye+3rB lcciR2LttwargCYldDX18N8hCdboQheDdpfib+ZYyKaeixLlRZZGpexIl 0iwObs2vcqJDfr8Zv8TwBGV6A23sMtGMVkQbxxd29KJZ6LvObyGnvJguO g==; X-IronPort-AV: E=McAfee;i="6400,9594,10346"; a="270547856" X-IronPort-AV: E=Sophos;i="5.91,223,1647327600"; d="scan'208";a="270547856" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 May 2022 14:25:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,223,1647327600"; d="scan'208";a="712562541" Received: from ranerica-svr.sc.intel.com ([172.25.110.23]) by fmsmga001.fm.intel.com with ESMTP; 13 May 2022 14:25:28 -0700 Date: Fri, 13 May 2022 14:29:03 -0700 From: Ricardo Neri To: Thomas Gleixner Subject: Re: [PATCH v6 15/29] x86/hpet: Add helper function hpet_set_comparator_periodic() Message-ID: <20220513212903.GF22683@ranerica-svr.sc.intel.com> References: <20220506000008.30892-1-ricardo.neri-calderon@linux.intel.com> <20220506000008.30892-16-ricardo.neri-calderon@linux.intel.com> <87mtfufifa.ffs@tglx> <87ilqifhxj.ffs@tglx> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87ilqifhxj.ffs@tglx> User-Agent: Mutt/1.9.4 (2018-02-28) Cc: "Ravi V. Shankar" , Andi Kleen , linuxppc-dev@lists.ozlabs.org, x86@kernel.org, Ricardo Neri , Stephane Eranian , linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Tony Luck , Nicholas Piggin , Andrew Morton , David Woodhouse X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" On Fri, May 06, 2022 at 11:51:52PM +0200, Thomas Gleixner wrote: > On Fri, May 06 2022 at 23:41, Thomas Gleixner wrote: > > On Thu, May 05 2022 at 16:59, Ricardo Neri wrote: > >> Programming an HPET channel as periodic requires setting the > >> HPET_TN_SETVAL bit in the channel configuration. Plus, the comparator > >> register must be written twice (once for the comparator value and once for > >> the periodic value). Since this programming might be needed in several > >> places (e.g., the HPET clocksource and the HPET-based hardlockup detector), > >> add a helper function for this purpose. > >> > >> A helper function hpet_set_comparator_oneshot() could also be implemented. > >> However, such function would only program the comparator register and the > >> function would be quite small. Hence, it is better to not bloat the code > >> with such an obvious function. > > > > This word salad above does not provide a single reason why the periodic > > programming function is required and better suited for the NMI watchdog > > case and then goes on and blurbs about why a function which is not > > required is not implemented. The argument about not bloating the code > > with an "obvious???" function which is quite small is slightly beyond my > > comprehension level. > > What's even more uncomprehensible is that the patch which actually sets > up that NMI watchdog cruft has: > > > + if (hc->boot_cfg & HPET_TN_PERIODIC_CAP) > > + hld_data->has_periodic = true; > > So how the heck does that work with a HPET which does not support > periodic mode? If the HPET channel does not support periodic mode (as indicated by the flag above), it will read the HPET counter into a local variable, increment that local variable, and write comparator of the HPET channel. If the HPET channel does support periodic mode, it will not kick it again. It will only kick a periodic HPET channel if needed (e.g., if the NMI watchdog was idle of watchdog_thresh changed its value). > > That watchdog muck will still happily invoke that set periodic function > in the hope that it works by chance? It will not. It will check hld_data->has_periodic and act accordingly. FWIW, I have tested this NMI watchdog with periodic and non-periodic HPET channels. Thanks and BR, Ricardo _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu 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 Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9ACE8C433EF for ; Fri, 13 May 2022 21:27:12 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4L0MDG6hDWz3cGF for ; Sat, 14 May 2022 07:27:10 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=L8g3xkv2; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=linux.intel.com (client-ip=192.55.52.93; helo=mga11.intel.com; envelope-from=ricardo.neri-calderon@linux.intel.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=L8g3xkv2; dkim-atps=neutral Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4L0MCZ0nCXz3bxk for ; Sat, 14 May 2022 07:26:32 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1652477194; x=1684013194; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=l6Vls9wnN+x00vvDZEEHtOYdZPzsd8ahPI1ukzsIW6A=; b=L8g3xkv2dDggp4eJtqsrQTlG3JHxSmYos5Wlqxw0ppNJb6khRJ3l7lY1 XakJMX8adLu99AxtKjjSXRpT35NNlrlqOEFe3z11cD7IPMrznCAL84dI4 7am7gvy1JggUl1z7i5FOD8etpwDXnmFwyve7043Z3msVSRX892dSsISVr LmOTIvA1UfOtmjBXWDiI+wN+WkZO27tJPo5aqNCNgOM55Bx/o3YcUDkcu cBNocwboEykt3UX2Ewy9CcSXyQii739tf1GdGpp+fFRmlqTIcmm7lEz6c MjE7jAsqHp9hNGM/7jBPD9QhQ4NqyiNIeHFxMvO0HY9BE1UdWJzYuLXF3 A==; X-IronPort-AV: E=McAfee;i="6400,9594,10346"; a="268005198" X-IronPort-AV: E=Sophos;i="5.91,223,1647327600"; d="scan'208";a="268005198" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 May 2022 14:25:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,223,1647327600"; d="scan'208";a="712562541" Received: from ranerica-svr.sc.intel.com ([172.25.110.23]) by fmsmga001.fm.intel.com with ESMTP; 13 May 2022 14:25:28 -0700 Date: Fri, 13 May 2022 14:29:03 -0700 From: Ricardo Neri To: Thomas Gleixner Subject: Re: [PATCH v6 15/29] x86/hpet: Add helper function hpet_set_comparator_periodic() Message-ID: <20220513212903.GF22683@ranerica-svr.sc.intel.com> References: <20220506000008.30892-1-ricardo.neri-calderon@linux.intel.com> <20220506000008.30892-16-ricardo.neri-calderon@linux.intel.com> <87mtfufifa.ffs@tglx> <87ilqifhxj.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ilqifhxj.ffs@tglx> User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Ravi V. Shankar" , Andi Kleen , linuxppc-dev@lists.ozlabs.org, Joerg Roedel , x86@kernel.org, Ricardo Neri , Stephane Eranian , linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Tony Luck , Nicholas Piggin , Suravee Suthikulpanit , Andrew Morton , David Woodhouse , Lu Baolu Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Fri, May 06, 2022 at 11:51:52PM +0200, Thomas Gleixner wrote: > On Fri, May 06 2022 at 23:41, Thomas Gleixner wrote: > > On Thu, May 05 2022 at 16:59, Ricardo Neri wrote: > >> Programming an HPET channel as periodic requires setting the > >> HPET_TN_SETVAL bit in the channel configuration. Plus, the comparator > >> register must be written twice (once for the comparator value and once for > >> the periodic value). Since this programming might be needed in several > >> places (e.g., the HPET clocksource and the HPET-based hardlockup detector), > >> add a helper function for this purpose. > >> > >> A helper function hpet_set_comparator_oneshot() could also be implemented. > >> However, such function would only program the comparator register and the > >> function would be quite small. Hence, it is better to not bloat the code > >> with such an obvious function. > > > > This word salad above does not provide a single reason why the periodic > > programming function is required and better suited for the NMI watchdog > > case and then goes on and blurbs about why a function which is not > > required is not implemented. The argument about not bloating the code > > with an "obvious???" function which is quite small is slightly beyond my > > comprehension level. > > What's even more uncomprehensible is that the patch which actually sets > up that NMI watchdog cruft has: > > > + if (hc->boot_cfg & HPET_TN_PERIODIC_CAP) > > + hld_data->has_periodic = true; > > So how the heck does that work with a HPET which does not support > periodic mode? If the HPET channel does not support periodic mode (as indicated by the flag above), it will read the HPET counter into a local variable, increment that local variable, and write comparator of the HPET channel. If the HPET channel does support periodic mode, it will not kick it again. It will only kick a periodic HPET channel if needed (e.g., if the NMI watchdog was idle of watchdog_thresh changed its value). > > That watchdog muck will still happily invoke that set periodic function > in the hope that it works by chance? It will not. It will check hld_data->has_periodic and act accordingly. FWIW, I have tested this NMI watchdog with periodic and non-periodic HPET channels. Thanks and BR, Ricardo