Received: by 2002:a6b:500f:0:0:0:0:0 with SMTP id e15csp223862iob; Wed, 18 May 2022 00:19:52 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxqfnHK1JxJMaSgQ0Qq7my7rgnytDfzdZYKAjC5oG2oYJDXOjYBPiN36rW5tR1WnpnmKyUb X-Received: by 2002:a62:fb0e:0:b0:505:fd9e:9218 with SMTP id x14-20020a62fb0e000000b00505fd9e9218mr26584328pfm.78.1652858392639; Wed, 18 May 2022 00:19:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1652858392; cv=none; d=google.com; s=arc-20160816; b=KdqyRfmTeXwCkmksNvmI55ZEu7unwbLVuyP+a7jvOkDyaNt+C4Q2FN2MLH8CCixO9g XNoWudV6nJYnuLvJaJeU8/HnESaBu27LtTJ7U9O+rlGKGmuJyAtzXHwiWex1vdzAYbnT Zz6SaHxaCrzywz7f/AXwLFACCbdjORNnQueg8IJj7bjQ+/fSSdUwqDVVR4euEbIp0I/L ffkPcllGkJxTKuKmEOGqHGpXPcWMENzhQh2giaNv287Wx/UAr4djnlulS257z61DYrx/ EBzoGPXJp9518w+BmuD7oOjkVf1C25zPqQ6XmZrWDZ0y4qtpG2soQ9FTD8Inm3BHk5O5 CIvw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=x2VyQkSwJ0gmkWqOa24LVzUq9CN5c1BBpPSPLP+5kwA=; b=E9YAdCisrJJ0ZAvCPnj5SHFJNg5Hkf90WfXvTt9CFxTPKsC63DjkKWv5rjP4iilSbR BBn892v91+vrblqFlTccryf/k8bIAkoKFaftFRQXB62UYiGB042TLMv4Frz30/I8Kyt6 SbqKqbF6BZlfI4a4QUxQw2dfkBBWdbKQiIQdsHqlA0B64do4uJY7+KOfdT+a0LV+GG33 +k8qurgiEc+jObw/reGAkXykFvSrYt9vZoKHP9DNsz6s31HLoYYt7rV4YaUpD1XqF3s2 1JrOU/b2RJpWhazt3fY7zKRhWKKdIe/Bt4Nc9NlVEyqKYrTR8SN3NpbyDwWqthlJgcmG w0+A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=kjNzYqfr; spf=softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net. [23.128.96.19]) by mx.google.com with ESMTPS id n44-20020a056a000d6c00b0050dfa520d25si1805746pfv.157.2022.05.18.00.19.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 May 2022 00:19:52 -0700 (PDT) Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=kjNzYqfr; spf=softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 56860EC302; Wed, 18 May 2022 00:18:09 -0700 (PDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232007AbiERHSA (ORCPT + 99 others); Wed, 18 May 2022 03:18:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33834 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231947AbiERHR6 (ORCPT ); Wed, 18 May 2022 03:17:58 -0400 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 14F442F3B2; Wed, 18 May 2022 00:17:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1652858277; x=1684394277; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=q+JFlMjJAbHftH3ICVKYgfpJzrW4prf2V94k/9k9eo0=; b=kjNzYqfr+LU8FIUfavgY4mWbwQQhokbimq4fybA0Hvwm7C6IWkcVJmwi u3/6vtRnBwo46Wk/k90QhXJN400mZEshalGLunBkSrId2eNaIP5wNPKY+ 2phyRNcWLrYXxGJVluw3Qlagz0yEAix5ZosCnHGNw57Pc7AphO2eQTrj0 QSLiFIrqy5dVuX4GUk/Wm1977UGS5B37XA0W0Xuk86BRXwap+xhgOlQvE WenBWMTkMP1kXdnlj9Bc4rVu9whyGylIAoGuACGK7yRaX7g17XWj6kOye D5S/aDiw9Ex1w7zvsQxk98T8vby20YunTIA+gMZyEKqcD7nJzSmafGeYI g==; X-IronPort-AV: E=McAfee;i="6400,9594,10350"; a="252042965" X-IronPort-AV: E=Sophos;i="5.91,234,1647327600"; d="scan'208";a="252042965" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 May 2022 00:17:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.91,234,1647327600"; d="scan'208";a="545297940" Received: from yilunxu-optiplex-7050.sh.intel.com (HELO localhost) ([10.239.159.135]) by orsmga006.jf.intel.com with ESMTP; 18 May 2022 00:17:43 -0700 Date: Wed, 18 May 2022 15:10:10 +0800 From: Xu Yilun To: Russ Weight Cc: mdf@kernel.org, hao.wu@intel.com, lee.jones@linaro.org, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, trix@redhat.com, marpagan@redhat.com, lgoncalv@redhat.com, matthew.gerlach@linux.intel.com, basheer.ahmed.muddebihal@intel.com, tianfei.zhang@intel.com Subject: Re: [PATCH v20 3/5] fpga: m10bmc-sec: expose max10 flash update count Message-ID: <20220518071010.GA55267@yilunxu-OptiPlex-7050> References: <20220516234941.592886-1-russell.h.weight@intel.com> <20220516234941.592886-4-russell.h.weight@intel.com> <20220517041310.GA40711@yilunxu-OptiPlex-7050> <8fb5dea0-9ce6-a17c-1253-64a43c86c82c@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <8fb5dea0-9ce6-a17c-1253-64a43c86c82c@intel.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 17, 2022 at 09:16:50AM -0700, Russ Weight wrote: > > > On 5/16/22 21:13, Xu Yilun wrote: > > On Mon, May 16, 2022 at 04:49:39PM -0700, Russ Weight wrote: > >> Extend the MAX10 BMC Secure Update driver to provide a sysfs file to > >> expose the flash update count. > >> > >> Signed-off-by: Russ Weight > >> Reviewed-by: Tom Rix > >> --- > >> v20: > >> - No change > >> v19: > >> - Change "card bmc" naming back to "m10 bmc" naming to be consistent > >> with the parent driver. > >> v18: > >> - No change > >> v17: > >> - Update the Date and KernelVersion for the ABI documentation to Jul 2022 > >> and 5.19 respectively. > >> - Change "m10bmc" in symbol names to "cardbmc" to reflect the fact that the > >> future devices will not necessarily use the MAX10. > >> v16: > >> - No Change > >> v15: > >> - Updated the Dates and KernelVersions in the ABI documentation > >> v14: > >> - No change > >> v13: > >> - Updated ABI documentation date and kernel version > >> v12: > >> - Updated Date and KernelVersion fields in ABI documentation > >> v11: > >> - No change > >> v10: > >> - Changed the path expression in the sysfs documentation to > >> replace the n3000 reference with something more generic to > >> accomodate other devices that use the same driver. > >> v9: > >> - Rebased to 5.12-rc2 next > >> - Updated Date and KernelVersion in ABI documentation > >> v8: > >> - Previously patch 3/6, otherwise no change > >> v7: > >> - Updated Date and KernelVersion in ABI documentation > >> v6: > >> - Changed flash_count_show() parameter list to achieve > >> reverse-christmas tree format. > >> - Added WARN_ON() call for (FLASH_COUNT_SIZE / stride) to ensure > >> that the proper count is passed to regmap_bulk_read(). > >> v5: > >> - Renamed sysfs node user_flash_count to flash_count and updated the > >> sysfs documentation accordingly. > >> v4: > >> - Moved the sysfs file for displaying the flash count from the > >> FPGA Security Manager class driver to here. The > >> m10bmc_user_flash_count() function is removed and the > >> functionality is moved into a user_flash_count_show() > >> function. > >> - Added ABI documentation for the new sysfs entry > >> v3: > >> - Changed: iops -> sops, imgr -> smgr, IFPGA_ -> FPGA_, ifpga_ to fpga_ > >> - Changed "MAX10 BMC Secure Engine driver" to "MAX10 BMC Secure Update > >> driver" > >> - Removed wrapper functions (m10bmc_raw_*, m10bmc_sys_*). The > >> underlying functions are now called directly. > >> v2: > >> - Renamed get_qspi_flash_count() to m10bmc_user_flash_count() > >> - Minor code cleanup per review comments > >> - Added m10bmc_ prefix to functions in m10bmc_iops structure > >> --- > >> .../sysfs-driver-intel-m10-bmc-sec-update | 8 +++++ > >> drivers/fpga/intel-m10-bmc-sec-update.c | 36 +++++++++++++++++++ > >> 2 files changed, 44 insertions(+) > >> > >> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-sec-update b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-sec-update > >> index 2bb271695e14..1132e39b2125 100644 > >> --- a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-sec-update > >> +++ b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-sec-update > >> @@ -27,3 +27,11 @@ Description: Read only. Returns the root entry hash for the BMC image > >> "hash not programmed". This file is only visible if the > >> underlying device supports it. > >> Format: string. > >> + > >> +What: /sys/bus/platform/drivers/intel-m10bmc-sec-update/.../security/flash_count > >> +Date: Jul 2022 > >> +KernelVersion: 5.19 > >> +Contact: Russ Weight > >> +Description: Read only. Returns number of times the secure update > >> + staging area has been flashed. > >> + Format: "%u". > >> diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c > >> index f9f39d2cfe5b..3f183202de3b 100644 > >> --- a/drivers/fpga/intel-m10-bmc-sec-update.c > >> +++ b/drivers/fpga/intel-m10-bmc-sec-update.c > >> @@ -78,7 +78,43 @@ DEVICE_ATTR_SEC_REH_RO(bmc, BMC_PROG_MAGIC, BMC_PROG_ADDR, BMC_REH_ADDR); > >> DEVICE_ATTR_SEC_REH_RO(sr, SR_PROG_MAGIC, SR_PROG_ADDR, SR_REH_ADDR); > >> DEVICE_ATTR_SEC_REH_RO(pr, PR_PROG_MAGIC, PR_PROG_ADDR, PR_REH_ADDR); > >> > >> +#define FLASH_COUNT_SIZE 4096 /* count stored as inverted bit vector */ > >> + > >> +static ssize_t flash_count_show(struct device *dev, > >> + struct device_attribute *attr, char *buf) > >> +{ > >> + struct m10bmc_sec *sec = dev_get_drvdata(dev); > >> + unsigned int stride, num_bits; > >> + u8 *flash_buf; > >> + int cnt, ret; > >> + > >> + stride = regmap_get_reg_stride(sec->m10bmc->regmap); > >> + num_bits = FLASH_COUNT_SIZE * 8; > >> + > >> + flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL); > >> + if (!flash_buf) > >> + return -ENOMEM; > >> + > >> + WARN_ON(FLASH_COUNT_SIZE % stride); > > The same concern here. Stop users from getting the broken value. > > Sure - I will change this. > > I was using WARN_ON() because it indicates a problem in the device or > the driver itself. It is not really validating information from userspace. Understood. So it is OK we keep the WARN_ON, or WARN_ON_ONCE?But we should still have some code to handle the issue gracefully rather than just passing the broken value to user. Thanks Yilun > > As I understand it, WARN_ON() is used to log such events to the kernel > log. If this isn't an appropriate use of WARN_ON(), then when should I > consider using it? > > Thanks, > - Russ > > > >> + ret = regmap_bulk_read(sec->m10bmc->regmap, STAGING_FLASH_COUNT, > >> + flash_buf, FLASH_COUNT_SIZE / stride); > >> + if (ret) { > >> + dev_err(sec->dev, > >> + "failed to read flash count: %x cnt %x: %d\n", > >> + STAGING_FLASH_COUNT, FLASH_COUNT_SIZE / stride, ret); > >> + goto exit_free; > >> + } > >> + cnt = num_bits - bitmap_weight((unsigned long *)flash_buf, num_bits); > >> + > >> +exit_free: > >> + kfree(flash_buf); > >> + > >> + return ret ? : sysfs_emit(buf, "%u\n", cnt); > >> +} > >> +static DEVICE_ATTR_RO(flash_count); > >> + > >> static struct attribute *m10bmc_security_attrs[] = { > >> + &dev_attr_flash_count.attr, > >> &dev_attr_bmc_root_entry_hash.attr, > >> &dev_attr_sr_root_entry_hash.attr, > >> &dev_attr_pr_root_entry_hash.attr, > >> -- > >> 2.25.1