FME_PR_INTFC_ID is used as compat_id for fpga manager and region,
but high 64 bits and low 64 bits of the compat_id are swapped by
mistake. This patch fixes this problem by fixing register address.
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-mgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Current driver checks if input bitstream file size is aligned or
not per PR data width (default 32bits). It requires one additional
step for end user when they generate the bitstream file, padding
extra zeros to bitstream file to align its size per PR data width,
but they don't have to as hardware will drop extra padding bytes
automatically.
In order to simplify the user steps, this patch aligns PR buffer
size per PR data width in driver, to allow user to pass unaligned
size bitstream files to driver.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-pr.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -74,6 +74,7 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)structdfl_fme*fme;unsignedlongminsz;void*buf=NULL;+size_tlength;intret=0;u64v;
@@ -85,9 +86,6 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)if(port_pr.argsz<minsz||port_pr.flags)return-EINVAL;-if(!IS_ALIGNED(port_pr.buffer_size,4))-return-EINVAL;-/* get fme header region */fme_hdr=dfl_get_feature_ioaddr_by_id(&pdev->dev,FME_FEATURE_ID_HEADER);
@@ -103,7 +101,13 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)port_pr.buffer_size))return-EFAULT;-buf=vmalloc(port_pr.buffer_size);+/*+*alignPRbufferperPRbandwidth,asHWignorestheextrapadding+*dataautomatically.+*/+length=ALIGN(port_pr.buffer_size,4);++buf=vmalloc(length);if(!buf)return-ENOMEM;
@@ -140,7 +144,7 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)fpga_image_info_free(region->info);info->buf=buf;-info->count=port_pr.buffer_size;+info->count=length;info->region_id=port_pr.port_id;region->info=info;
In order to support virtualization usage via PCIe SRIOV, this patch
adds two ioctls under FPGA Management Engine (FME) to release and
assign back the port device. In order to safely turn Port from PF
into VF and enable PCIe SRIOV, it requires user to invoke this
PORT_RELEASE ioctl to release port firstly to remove userspace
interfaces, and then configure the PF/VF access register in FME.
After disable SRIOV, it requires user to invoke this PORT_ASSIGN
ioctl to attach the port back to PF.
Ioctl interfaces:
* DFL_FPGA_FME_PORT_RELEASE
Release platform device of given port, it deletes port platform
device to remove related userspace interfaces on PF, then
configures PF/VF access mode to VF.
* DFL_FPGA_FME_PORT_ASSIGN
Assign platform device of given port back to PF, it configures
PF/VF access mode to PF, then adds port platform device back to
re-enable related userspace interfaces on PF.
Signed-off-by: Zhang Yi Z <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-main.c | 54 +++++++++++++++++++++
drivers/fpga/dfl.c | 107 +++++++++++++++++++++++++++++++++++++-----
drivers/fpga/dfl.h | 10 ++++
include/uapi/linux/fpga-dfl.h | 32 +++++++++++++
4 files changed, 191 insertions(+), 12 deletions(-)
@@ -462,6 +466,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)pdata->dev=fdev;pdata->num=binfo->feature_num;pdata->dfl_cdev=binfo->cdev;+pdata->id=FEATURE_DEV_ID_UNUSED;mutex_init(&pdata->lock);/*
@@ -959,25 +964,27 @@ void dfl_fpga_feature_devs_remove(struct dfl_fpga_cdev *cdev){structdfl_feature_platform_data*pdata,*ptmp;-remove_feature_devs(cdev);-mutex_lock(&cdev->lock);-if(cdev->fme_dev){-/* the fme should be unregistered. */-WARN_ON(device_is_registered(cdev->fme_dev));+if(cdev->fme_dev)put_device(cdev->fme_dev);-}list_for_each_entry_safe(pdata,ptmp,&cdev->port_dev_list,node){structplatform_device*port_dev=pdata->dev;-/* the port should be unregistered. */-WARN_ON(device_is_registered(&port_dev->dev));+/* remove released ports */+if(!device_is_registered(&port_dev->dev)){+dfl_id_free(feature_dev_id_type(port_dev),+port_dev->id);+platform_device_put(port_dev);+}+list_del(&pdata->node);put_device(&port_dev->dev);}mutex_unlock(&cdev->lock);+remove_feature_devs(cdev);+fpga_region_unregister(cdev->region);devm_kfree(cdev->parent,cdev);}
This patch adds virtualization support description for DFL based
FPGA devices (based on PCIe SRIOV), and introductions to new
interfaces added by new dfl private features.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/fpga/dfl.txt | 115 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 115 insertions(+)
@@ -84,6 +84,8 @@ The following functions are exposed through ioctls: Get driver API version (DFL_FPGA_GET_API_VERSION) Check for extensions (DFL_FPGA_CHECK_EXTENSION) Program bitstream (DFL_FPGA_FME_PORT_PR)+ Assign port to PF (DFL_FPGA_FME_PORT_ASSIGN)+ Release port from PF (DFL_FPGA_FME_PORT_RELEASE) More functions are exposed through sysfs (/sys/class/fpga_region/regionX/dfl-fme.n/):
@@ -99,6 +101,24 @@ More functions are exposed through sysfs one FPGA device may have more than one port, this sysfs interface indicates how many ports the FPGA device has.+ Power management (power_mgmt/)+ power management sysfs interfaces allow user to read power management+ information (power consumption, power limits, throttling thresholds,+ thresholds status, etc) and configure power thresholds for different+ throttling levels.++ Thermal management (thermal_mgmt/)+ thermal management sysfs interfaces allow user to read temperature,+ thresholds, thresholds status and other thermal related information.++ Global error reporting management (errors/)+ error reporting sysfs interfaces allow user to read errors detected by the+ hardware, and clear the logged errors.++ Performance counters (perf/)+ performance counters sysfs interfaces allow user to use different counters+ to get performance data.+ FIU - PORT ==========
@@ -139,6 +159,10 @@ More functions are exposed through sysfs: Read Accelerator GUID (afu_id) afu_id indicates which PR bitstream is programmed to this AFU.+ Error reporting (errors/)+ error reporting sysfs interfaces allow user to read port/afu errors+ detected by the hardware, and clear the logged errors.+ DFL Framework Overview ======================
@@ -212,6 +236,97 @@ the compat_id exposed by the target FPGA region. This check is usually done by userspace before calling the reconfiguration IOCTL.+FPGA virtualization - PCIe SRIOV+================================+This section describes the virtualization support on DFL based FPGA device to+enable accessing an accelerator from applications running in a virtual machine+(VM). This section only describes the PCIe based FPGA device with SRIOV support.++Features supported by the particular FPGA device are exposed through Device+Feature Lists, as illustrated below:++ +-------------------------------+ +-------------++ | PF | | VF |+ +-------------------------------+ +-------------++ ^ ^ ^ ^+ | | | |++-----|------------|---------|--------------|-------++| | | | | |+| +-----+ +-------+ +-------+ +-------+ |+| | FME | | Port0 | | Port1 | | Port2 | |+| +-----+ +-------+ +-------+ +-------+ |+| ^ ^ ^ |+| | | | |+| +-------+ +------+ +-------+ |+| | AFU | | AFU | | AFU | |+| +-------+ +------+ +-------+ |+| |+| DFL based FPGA PCIe Device |++---------------------------------------------------+++FME is always accessed through the physical function (PF).++Ports (and related AFUs) are accessed via PF by default, but could be exposed+through virtual function (VF) devices via PCIe SRIOV. Each VF only contains+1 Port and 1 AFU for isolation. Users could assign individual VFs (accelerators)+created via PCIe SRIOV interface, to virtual machines.++The driver organization in virtualization case is illustrated below:++ +-------++------++------+ |+ | FME || FME || FME | |+ | FPGA || FPGA || FPGA | |+ |Manager||Bridge||Region| |+ +-------++------++------+ |+ +-----------------------+ +--------+ | +--------++ | FME | | AFU | | | AFU |+ | Module | | Module | | | Module |+ +-----------------------+ +--------+ | +--------++ +-----------------------+ | +-----------------------++ | FPGA Container Device | | | FPGA Container Device |+ | (FPGA Base Region) | | | (FPGA Base Region) |+ +-----------------------+ | +-----------------------++ +------------------+ | +------------------++ | FPGA PCIE Module | | Virtual | FPGA PCIE Module |+ +------------------+ Host | Machine +------------------++ -------------------------------------- | ------------------------------+ +---------------+ | +---------------++ | PCI PF Device | | | PCI VF Device |+ +---------------+ | +---------------+++FPGA PCIe device driver is always loaded first once a FPGA PCIe PF or VF device+is detected. It:++ a) finish enumeration on both FPGA PCIe PF and VF device using common+ interfaces from DFL framework.+ b) supports SRIOV.++The FME device driver plays a management role in this driver architecture, it+provides ioctls to release Port from PF and assign Port to PF. After release+a port from PF, then it's safe to expose this port through a VF via PCIe SRIOV+sysfs interface.++To enable accessing an accelerator from applications running in a VM, the+respective AFU's port needs to be assigned to a VF using the following steps:++ a) The PF owns all AFU ports by default. Any port that needs to be+ reassigned to a VF must first be released through the+ DFL_FPGA_FME_PORT_RELEASE ioctl on the FME device.++ b) Once N ports are released from PF, then user can use command below+ to enable SRIOV and VFs. Each VF owns only one Port with AFU.++ echo N > $PCI_DEVICE_PATH/sriov_numvfs++ c) Pass through the VFs to VMs++ d) The AFU under VF is accessible from applications in VM (using the+ same driver inside the VF).++Note that an FME can't be assigned to a VF, thus PR and other management+functions are only available via the PF.++ Device enumeration ================== This section introduces how applications enumerate the fpga device from
This patch introduces more sysfs interfaces for Accelerated
Function Unit (AFU). These interfaces allow users to read
current AFU Power State (APx), read / clear AFU Power (APx)
events which are sticky to identify transient APx state,
and manage AFU's LTR (latency tolerance reporting).
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-port | 30 +++++
drivers/fpga/dfl-afu-main.c | 144 ++++++++++++++++++++++
drivers/fpga/dfl.h | 11 ++
3 files changed, 185 insertions(+)
@@ -14,3 +14,33 @@ Description: Read-only. User can program different PR bitstreams to FPGA Accelerator Function Unit (AFU) for different functions. It returns uuid which could be used to identify which PR bitstream is programmed in this AFU.++What: /sys/bus/platform/devices/dfl-port.0/power_state+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It reports the APx (AFU Power) state, different APx+ means different throttling level. When reading this file, it+ returns "0" - Normal / "1" - AP1 / "2" - AP2 / "6" - AP6.++What: /sys/bus/platform/devices/dfl-port.0/ap1_event+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-write. Read or set 1 to clear AP1 (AFU Power State 1)+ event. It's used to indicate transient AP1 state.++What: /sys/bus/platform/devices/dfl-port.0/ap2_event+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-write. Read or set 1 to clear AP2 (AFU Power State 2)+ event. It's used to indicate transient AP2 state.++What: /sys/bus/platform/devices/dfl-port.0/ltr+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-write. Read and set AFU latency tolerance reporting value.+ Set ltr to 1 if the AFU can tolerate latency >= 40us or set it+ to 0 if it is latency sensitive.
This patch introduces userclock sysfs interfaces for AFU, user
could use these interfaces for clock setting to AFU.
Please note that, this is only working for port header feature
with revision 0, for later revisions, userclock setting is moved
to a separated private feature, so one revision sysfs interface
is exposed to userspace application for this purpose too.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Russ Weight <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-port | 35 +++++++
drivers/fpga/dfl-afu-main.c | 114 +++++++++++++++++++++-
drivers/fpga/dfl.h | 4 +
3 files changed, 152 insertions(+), 1 deletion(-)
@@ -44,3 +44,38 @@ Contact: Wu Hao <hao.wu@intel.com> Description: Read-write. Read and set AFU latency tolerance reporting value. Set ltr to 1 if the AFU can tolerate latency >= 40us or set it to 0 if it is latency sensitive.++What: /sys/bus/platform/devices/dfl-port.0/revision+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the revision of port header+ feature.++What: /sys/bus/platform/devices/dfl-port.0/userclk_freqcmd+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Write-only. User writes command to this interface to set+ userclock to AFU.++What: /sys/bus/platform/devices/dfl-port.0/userclk_freqsts+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the status of issued command+ to userclck_freqcmd.++What: /sys/bus/platform/devices/dfl-port.0/userclk_freqcntrcmd+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Write-only. User writes command to this interface to set+ userclock counter.++What: /sys/bus/platform/devices/dfl-port.0/userclk_freqcntrsts+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the status of issued command+ to userclck_freqcntrcmd.
@@ -274,6 +276,21 @@ static int dfl_feature_instance_init(struct platform_device *pdev,returnret;}+staticbooldfl_feature_drv_match(structdfl_feature*feature,+structdfl_feature_driver*driver)+{+conststructdfl_feature_id*ids=driver->id_table;++if(ids){+while(ids->id){+if(ids->id==feature->id)+returntrue;+ids++;+}+}+returnfalse;+}+/***dfl_fpga_dev_feature_init-initforsubfeaturesofdflfeaturedevice*@pdev:featuredevice.
@@ -294,8 +311,7 @@ int dfl_fpga_dev_feature_init(struct platform_device *pdev,while(drv->ops){dfl_fpga_dev_for_each_feature(pdata,feature){-/* match feature and drv using id */-if(feature->id==drv->id){+if(dfl_feature_drv_match(feature,drv)){ret=dfl_feature_instance_init(pdev,pdata,feature,drv);if(ret)
@@ -30,8 +30,8 @@/* plus one for fme device */#define MAX_DFL_FEATURE_DEV_NUM (MAX_DFL_FPGA_PORT_NUM + 1)-/* Reserved 0x0 for Header Group Register and 0xff for AFU */-#define FEATURE_ID_FIU_HEADER 0x0+/* Reserved 0xfe for Header Group Register and 0xff for AFU */+#define FEATURE_ID_FIU_HEADER 0xfe#define FEATURE_ID_AFU 0xff#define FME_FEATURE_ID_HEADER FEATURE_ID_FIU_HEADER
As these two functions are used by other private features. e.g.
in error reporting private feature, it requires to check port status
and reset port for error clearing.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-afu-main.c | 25 ++++++++++++++-----------
drivers/fpga/dfl-afu.h | 3 +++
2 files changed, 17 insertions(+), 11 deletions(-)
This patch adds support to thermal management private feature for DFL
FPGA Management Engine (FME). As thermal throttling is handled by
hardware automatically per pre-defined thresholds, this private
feature driver only provides read-only sysfs interfaces for user
to read temperature, thresholds, threshold policy and other info.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Russ Weight <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++++
drivers/fpga/dfl-fme-main.c | 202 +++++++++++++++++++++++
2 files changed, 258 insertions(+)
@@ -44,3 +44,59 @@ Description: Read-only. It returns socket_id to indicate which socket this FPGA belongs to, only valid for integrated solution. User only needs this information, in case standard numa node can't provide correct information.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/temperature+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns temperature (in Celsius) of this FPGA+ device.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold1+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold2+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/trip_threshold+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns trip threshold (in Celsius), once FPGA+ temperature reaches trip threshold, it triggers a fatal event+ to board management controller (BMC) to shutdown FPGA.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold1,+ otherwise 0. Once temperature reaches threshold1, hardware+ will automatically enter throttling state (AP1 - 50%+ or AP2 - 90% throttling, see 'threshold1_policy').++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold2,+ otherwise 0. Once temperature reaches threshold2, hardware+ will automatically enter the deepest throttling state (AP6+ - 100% throttling).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_policy+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the policy of temperature+ threshold1. It only supports two value (policy):+ 0 - AP2 state (90% throttling)+ 1 - AP1 state (50% throttling)
This patch adds support for power management private feature under
FPGA Management Engine (FME), sysfs interfaces are introduced for
different power management functions, users could use these sysfs
interface to get current number of consumed power, throttling
thresholds, threshold status and other information, and configure
different value for throttling thresholds too.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++
drivers/fpga/dfl-fme-main.c | 257 +++++++++++++++++++++++
2 files changed, 313 insertions(+)
@@ -100,3 +100,59 @@ Description: Read-only. Read this file to get the policy of temperature threshold1. It only supports two value (policy): 0 - AP2 state (90% throttling) 1 - AP1 state (50% throttling)++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/consumed+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns current power consumed by FPGA.++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-Write. Read/Write this file to get/set current power+ threshold1 in Watts.++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-Write. Read/Write this file to get/set current power+ threshold2 in Watts.++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if power consumption reaches the+ threshold1, otherwise 0.++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if power consumption reaches the+ threshold2, otherwise 0.++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/ltr+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get current Latency Tolerance+ Reporting (ltr) value, it's only valid for integrated+ solution as it blocks CPU on low power state.++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/xeon_limit+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get power limit for xeon, it+ is only valid for integrated solution.++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/fpga_limit+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get power limit for fpga, it+ is only valid for integrated solution.
@@ -507,6 +507,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)structdfl_feature*feature=&pdata->features[index];/* save resource information for each feature */+feature->pdev=fdev;feature->id=finfo->fid;feature->resource_index=index;feature->ioaddr=finfo->ioaddr;
@@ -156,3 +156,61 @@ KernelVersion: 5.2 Contact: Wu Hao <hao.wu@intel.com> Description: Read-only. Read this file to get power limit for fpga, it is only valid for integrated solution.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get errors detected by hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/first_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/next_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the second error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/clear+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Write-only. Write error code to this file to clear errors. If+ the input error code doesn't match, it returns -EBUSY.++What: /sys/bus/platform/devices/dfl-fme.0/errors/pcie0_errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns errors detected on pcie0 link.++What: /sys/bus/platform/devices/dfl-fme.0/errors/pcie1_errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns errors detected on pcie1 link.++What: /sys/bus/platform/devices/dfl-fme.0/errors/nonfatal_errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns non-fatal errors detected.++What: /sys/bus/platform/devices/dfl-fme.0/errors/catfatal_errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns catastrophic and fatal errors detected.++What: /sys/bus/platform/devices/dfl-fme.0/errors/inject_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-Write. Write this file to inject errors for testing+ purpose. Read this file to check errors injected.
Error reporting is one important private feature, it reports error
detected on port and accelerated function unit (AFU). It introduces
several sysfs interfaces to allow userspace to check and clear
errors detected by hardware.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-port | 29 +++
drivers/fpga/Makefile | 1 +
drivers/fpga/dfl-afu-error.c | 225 ++++++++++++++++++++++
drivers/fpga/dfl-afu-main.c | 4 +
drivers/fpga/dfl-afu.h | 4 +
5 files changed, 263 insertions(+)
create mode 100644 drivers/fpga/dfl-afu-error.c
@@ -79,3 +79,32 @@ KernelVersion: 5.2 Contact: Wu Hao <hao.wu@intel.com> Description: Read-only. Read this file to get the status of issued command to userclck_freqcntrcmd.++What: /sys/bus/platform/devices/dfl-port.0/errors/errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get errors detected on port and+ Accelerated Function Unit (AFU).++What: /sys/bus/platform/devices/dfl-port.0/errors/first_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-port.0/errors/first_malformed_req+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first malformed request+ captured by hardware.++What: /sys/bus/platform/devices/dfl-port.0/errors/clear+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Write-only. Write error code to this file to clear errors. If+ the input error code doesn't match, it returns -EBUSY error+ code.
@@ -0,0 +1,225 @@+// SPDX-License-Identifier: GPL-2.0+/*+*DriverforFPGAAcceleratedFunctionUnit(AFU)ErrorReporting+*+*Copyright2019IntelCorporation,Inc.+*+*Authors:+*WuHao<hao.wu@linux.intel.com>+*XiaoGuangrong<guangrong.xiao@linux.intel.com>+*JosephGrecco<joe.grecco@intel.com>+*EnnoLuebbers<enno.luebbers@intel.com>+*TimWhisonant<tim.whisonant@intel.com>+*AnandaRavuri<ananda.ravuri@intel.com>+*MitchelHenry<henry.mitchel@intel.com>+*/++#include<linux/uaccess.h>++#include"dfl-afu.h"++#define PORT_ERROR_MASK 0x8+#define PORT_ERROR 0x10+#define PORT_FIRST_ERROR 0x18+#define PORT_MALFORMED_REQ0 0x20+#define PORT_MALFORMED_REQ1 0x28++#define ERROR_MASK GENMASK_ULL(63, 0)++/* mask or unmask port errors by the error mask register. */+staticvoid__port_err_mask(structdevice*dev,boolmask)+{+void__iomem*base;++base=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);++writeq(mask?ERROR_MASK:0,base+PORT_ERROR_MASK);+}++/* clear port errors. */+staticint__port_err_clear(structdevice*dev,u64err)+{+structplatform_device*pdev=to_platform_device(dev);+void__iomem*base_err,*base_hdr;+intret;+u64v;++base_err=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);+base_hdr=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_HEADER);++/*+*clearPortErrors+*+*-CheckforAP6State+*-HaltPortbykeepingPortinreset+*-SetPORTErrormasktoall1tomaskerrors+*-Clearallerrors+*-SetPortmasktoall0toenableerrors+*-Allerrorsstartcapturingnewerrors+*-EnablePortbypullingtheportoutofreset+*/++/* if device is still in AP6 power state, can not clear any error. */+v=readq(base_hdr+PORT_HDR_STS);+if(FIELD_GET(PORT_STS_PWR_STATE,v)==PORT_STS_PWR_STATE_AP6){+dev_err(dev,"Could not clear errors, device in AP6 state.\n");+return-EBUSY;+}++/* Halt Port by keeping Port in reset */+ret=__port_disable(pdev);+if(ret)+returnret;++/* Mask all errors */+__port_err_mask(dev,true);++/* Clear errors if err input matches with current port errors.*/+v=readq(base_err+PORT_ERROR);++if(v==err){+writeq(v,base_err+PORT_ERROR);++v=readq(base_err+PORT_FIRST_ERROR);+writeq(v,base_err+PORT_FIRST_ERROR);+}else{+ret=-EBUSY;+}++/* Clear mask */+__port_err_mask(dev,false);++/* Enable the Port by clear the reset */+__port_enable(pdev);++returnret;+}++staticssize_trevision_show(structdevice*dev,structdevice_attribute*attr,+char*buf)+{+void__iomem*base;++base=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);++returnscnprintf(buf,PAGE_SIZE,"%u\n",dfl_feature_revision(base));+}+staticDEVICE_ATTR_RO(revision);++staticssize_terrors_show(structdevice*dev,structdevice_attribute*attr,+char*buf)+{+structdfl_feature_platform_data*pdata=dev_get_platdata(dev);+void__iomem*base;+u64error;++base=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);++mutex_lock(&pdata->lock);+error=readq(base+PORT_ERROR);+mutex_unlock(&pdata->lock);++returnscnprintf(buf,PAGE_SIZE,"0x%llx\n",(unsignedlonglong)error);+}+staticDEVICE_ATTR_RO(errors);++staticssize_tfirst_error_show(structdevice*dev,+structdevice_attribute*attr,char*buf)+{+structdfl_feature_platform_data*pdata=dev_get_platdata(dev);+void__iomem*base;+u64error;++base=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);++mutex_lock(&pdata->lock);+error=readq(base+PORT_FIRST_ERROR);+mutex_unlock(&pdata->lock);++returnscnprintf(buf,PAGE_SIZE,"0x%llx\n",(unsignedlonglong)error);+}+staticDEVICE_ATTR_RO(first_error);++staticssize_tfirst_malformed_req_show(structdevice*dev,+structdevice_attribute*attr,+char*buf)+{+structdfl_feature_platform_data*pdata=dev_get_platdata(dev);+void__iomem*base;+u64req0,req1;++base=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);++mutex_lock(&pdata->lock);+req0=readq(base+PORT_MALFORMED_REQ0);+req1=readq(base+PORT_MALFORMED_REQ1);+mutex_unlock(&pdata->lock);++returnscnprintf(buf,PAGE_SIZE,"0x%016llx%016llx\n",+(unsignedlonglong)req1,(unsignedlonglong)req0);+}+staticDEVICE_ATTR_RO(first_malformed_req);++staticssize_tclear_store(structdevice*dev,structdevice_attribute*attr,+constchar*buff,size_tcount)+{+structdfl_feature_platform_data*pdata=dev_get_platdata(dev);+u64value;+intret;++if(kstrtou64(buff,0,&value))+return-EINVAL;++mutex_lock(&pdata->lock);+ret=__port_err_clear(dev,value);+mutex_unlock(&pdata->lock);++returnret?ret:count;+}+staticDEVICE_ATTR_WO(clear);++staticstructattribute*port_err_attrs[]={+&dev_attr_revision.attr,+&dev_attr_errors.attr,+&dev_attr_first_error.attr,+&dev_attr_first_malformed_req.attr,+&dev_attr_clear.attr,+NULL,+};++staticstructattribute_groupport_err_attr_group={+.attrs=port_err_attrs,+.name="errors",+};++staticintport_err_init(structplatform_device*pdev,+structdfl_feature*feature)+{+structdfl_feature_platform_data*pdata=dev_get_platdata(&pdev->dev);++dev_dbg(&pdev->dev,"PORT ERR Init.\n");++mutex_lock(&pdata->lock);+__port_err_mask(&pdev->dev,false);+mutex_unlock(&pdata->lock);++returnsysfs_create_group(&pdev->dev.kobj,&port_err_attr_group);+}++staticvoidport_err_uinit(structplatform_device*pdev,+structdfl_feature*feature)+{+dev_dbg(&pdev->dev,"PORT ERR UInit.\n");++sysfs_remove_group(&pdev->dev.kobj,&port_err_attr_group);+}++conststructdfl_feature_idport_err_id_table[]={+{.id=PORT_FEATURE_ID_ERROR,},+{0,}+};++conststructdfl_feature_opsport_err_ops={+.init=port_err_init,+.uinit=port_err_uinit,+};
STP (SignalTap) is one of the private features under the port for
debugging. This patch adds private feature driver support for it
to allow userspace applications to mmap related mmio region and
provide STP service.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-afu-main.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
@@ -21,3 +21,26 @@ Contact: Wu Hao <hao.wu@intel.com> Description: Read-only. It returns Bitstream (static FPGA region) meta data, which includes the synthesis date, seed and other information of this static FPGA region.++What: /sys/bus/platform/devices/dfl-fme.0/cache_size+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns cache size of this FPGA device.++What: /sys/bus/platform/devices/dfl-fme.0/fabric_version+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns fabric version of this FPGA device.+ Userspace applications need this information to select+ best data channels per different fabric design.++What: /sys/bus/platform/devices/dfl-fme.0/socket_id+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns socket_id to indicate which socket+ this FPGA belongs to, only valid for integrated solution.+ User only needs this information, in case standard numa node+ can't provide correct information.
This patch enables the standard sriov support. It allows user to
enable SRIOV (and VFs), then user could pass through accelerators
(VFs) into virtual machine or use VFs directly in host.
Signed-off-by: Zhang Yi Z <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-pci.c | 40 ++++++++++++++++++++++++++++++++++++++++
drivers/fpga/dfl.c | 41 +++++++++++++++++++++++++++++++++++++++++
drivers/fpga/dfl.h | 1 +
3 files changed, 82 insertions(+)
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-main.c | 3 ++
drivers/fpga/dfl-fme-mgr.c | 75 +++++++++++++++++++++++++++++++++++++--------
drivers/fpga/dfl-fme-pr.c | 45 ++++++++++++++++-----------
drivers/fpga/dfl-fme.h | 2 ++
drivers/fpga/dfl.h | 5 +++
5 files changed, 99 insertions(+), 31 deletions(-)
@@ -83,7 +83,7 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)if(copy_from_user(&port_pr,argp,minsz))return-EFAULT;-if(port_pr.argsz<minsz||port_pr.flags)+if(port_pr.argsz<minsz||port_pr.flags||!port_pr.buffer_size)return-EINVAL;/* get fme header region */
@@ -101,15 +101,25 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)port_pr.buffer_size))return-EFAULT;+mutex_lock(&pdata->lock);+fme=dfl_fpga_pdata_get_private(pdata);+/* fme device has been unregistered. */+if(!fme){+ret=-EINVAL;+gotounlock_exit;+}+/**alignPRbufferperPRbandwidth,asHWignorestheextrapadding*dataautomatically.*/-length=ALIGN(port_pr.buffer_size,4);+length=ALIGN(port_pr.buffer_size,fme->pr_datawidth);buf=vmalloc(length);-if(!buf)-return-ENOMEM;+if(!buf){+ret=-ENOMEM;+gotounlock_exit;+}if(copy_from_user(buf,(void__user*)(unsignedlong)port_pr.buffer_address,
@@ -127,18 +137,10 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)info->flags|=FPGA_MGR_PARTIAL_RECONFIG;-mutex_lock(&pdata->lock);-fme=dfl_fpga_pdata_get_private(pdata);-/* fme device has been unregistered. */-if(!fme){-ret=-EINVAL;-gotounlock_exit;-}-region=dfl_fme_region_find(fme,port_pr.port_id);if(!region){ret=-EINVAL;-gotounlock_exit;+gotofree_exit;}fpga_image_info_free(region->info);
@@ -159,13 +161,10 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)fpga_bridges_put(®ion->bridge_list);put_device(®ion->dev);-unlock_exit:-mutex_unlock(&pdata->lock);free_exit:vfree(buf);-if(copy_to_user((void__user*)arg,&port_pr,minsz))-return-EFAULT;-+unlock_exit:+mutex_unlock(&pdata->lock);returnret;}
@@ -391,6 +390,16 @@ static int pr_mgmt_init(struct platform_device *pdev,mutex_lock(&pdata->lock);priv=dfl_fpga_pdata_get_private(pdata);+/*+*InitializePRdatawidth.+*Onlyrevision2supports512bitdatawidthforbetterperformance,+*otherrevisionsusedefault32bitdatawidth.+*/+if(dfl_feature_revision(feature->ioaddr)==2)+priv->pr_datawidth=64;+else+priv->pr_datawidth=4;+/* Initialize the region and bridge sub device list */INIT_LIST_HEAD(&priv->region_list);INIT_LIST_HEAD(&priv->bridge_list);
From: Alan Tull <atull@kernel.org> Date: 2019-03-25 17:29:02
On Sun, Mar 24, 2019 at 10:23 PM Wu Hao [off-list ref] wrote:
Hi Hao,
FME_PR_INTFC_ID is used as compat_id for fpga manager and region,
but high 64 bits and low 64 bits of the compat_id are swapped by
mistake. This patch fixes this problem by fixing register address.
Signed-off-by: Wu Hao <redacted>
Acked-by: Alan Tull <atull@kernel.org>
Thanks,
Alan
From: Alan Tull <atull@kernel.org> Date: 2019-03-25 17:51:21
On Sun, Mar 24, 2019 at 10:23 PM Wu Hao [off-list ref] wrote:
Hi Hao,
Looks good, one question below.
quoted hunk
Current driver checks if input bitstream file size is aligned or
not per PR data width (default 32bits). It requires one additional
step for end user when they generate the bitstream file, padding
extra zeros to bitstream file to align its size per PR data width,
but they don't have to as hardware will drop extra padding bytes
automatically.
In order to simplify the user steps, this patch aligns PR buffer
size per PR data width in driver, to allow user to pass unaligned
size bitstream files to driver.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-pr.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -74,6 +74,7 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)structdfl_fme*fme;unsignedlongminsz;void*buf=NULL;+size_tlength;intret=0;u64v;
@@ -85,9 +86,6 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)if(port_pr.argsz<minsz||port_pr.flags)return-EINVAL;-if(!IS_ALIGNED(port_pr.buffer_size,4))-return-EINVAL;-/* get fme header region */fme_hdr=dfl_get_feature_ioaddr_by_id(&pdev->dev,FME_FEATURE_ID_HEADER);
@@ -103,7 +101,13 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)port_pr.buffer_size))return-EFAULT;-buf=vmalloc(port_pr.buffer_size);+/*+*alignPRbufferperPRbandwidth,asHWignorestheextrapadding+*dataautomatically.+*/+length=ALIGN(port_pr.buffer_size,4);++buf=vmalloc(length);
Since it may not be completely filled, would it be worthwhile to alloc
a zero'ed buff?
Alan
From: Alan Tull <atull@kernel.org> Date: 2019-03-25 18:49:33
On Sun, Mar 24, 2019 at 10:23 PM Wu Hao [off-list ref] wrote:
Hi Hao,
This looks fine.
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
From: Scott Wood <hidden> Date: 2019-03-25 22:53:53
On Mon, 2019-03-25 at 11:07 +0800, Wu Hao wrote:
quoted hunk
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-main.c | 3 ++
drivers/fpga/dfl-fme-mgr.c | 75 +++++++++++++++++++++++++++++++++++++---
-----
drivers/fpga/dfl-fme-pr.c | 45 ++++++++++++++++-----------
drivers/fpga/dfl-fme.h | 2 ++
drivers/fpga/dfl.h | 5 +++
5 files changed, 99 insertions(+), 31 deletions(-)
Shouldn't there be some sort of check that AVX512 is actually supported
on the running system?
Also, src should be const, and the asm statement should have a memory
clobber.
Likewise, this will be called if a revision 2 device is used on non-x86
(or on x86 with an old binutils). The driver should fall back to 32-bit
in such cases.
From: Scott Wood <hidden> Date: 2019-03-25 22:58:40
On Mon, 2019-03-25 at 17:53 -0500, Scott Wood wrote:
On Mon, 2019-03-25 at 11:07 +0800, Wu Hao wrote:
quoted
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-main.c | 3 ++
drivers/fpga/dfl-fme-mgr.c | 75 +++++++++++++++++++++++++++++++++++++-
--
-----
drivers/fpga/dfl-fme-pr.c | 45 ++++++++++++++++-----------
drivers/fpga/dfl-fme.h | 2 ++
drivers/fpga/dfl.h | 5 +++
5 files changed, 99 insertions(+), 31 deletions(-)
Shouldn't there be some sort of check that AVX512 is actually supported
on the running system?
Also, src should be const, and the asm statement should have a memory
clobber.
Likewise, this will be called if a revision 2 device is used on non-x86
(or on x86 with an old binutils). The driver should fall back to 32-bit
in such cases.
Sorry, I missed the comment about revision 2 only being on integrated
devices -- but will that always be the case? Seems worthwhile to check for
AVX512 support anyway. And there's still the possibility of being built
with an old binutils such that CONFIG_AS_AVX512 is not set, or running on a
kernel where avx512 was disabled via a boot option.
What about future revisions >= 2? Currently the driver will treat them as
if they were revision < 2. Is that intended?
-Scott
On Mon, Mar 25, 2019 at 12:50:40PM -0500, Alan Tull wrote:
On Sun, Mar 24, 2019 at 10:23 PM Wu Hao [off-list ref] wrote:
Hi Hao,
Looks good, one question below.
quoted
Current driver checks if input bitstream file size is aligned or
not per PR data width (default 32bits). It requires one additional
step for end user when they generate the bitstream file, padding
extra zeros to bitstream file to align its size per PR data width,
but they don't have to as hardware will drop extra padding bytes
automatically.
In order to simplify the user steps, this patch aligns PR buffer
size per PR data width in driver, to allow user to pass unaligned
size bitstream files to driver.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-pr.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -74,6 +74,7 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)structdfl_fme*fme;unsignedlongminsz;void*buf=NULL;+size_tlength;intret=0;u64v;
@@ -85,9 +86,6 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)if(port_pr.argsz<minsz||port_pr.flags)return-EINVAL;-if(!IS_ALIGNED(port_pr.buffer_size,4))-return-EINVAL;-/* get fme header region */fme_hdr=dfl_get_feature_ioaddr_by_id(&pdev->dev,FME_FEATURE_ID_HEADER);
@@ -103,7 +101,13 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)port_pr.buffer_size))return-EFAULT;-buf=vmalloc(port_pr.buffer_size);+/*+*alignPRbufferperPRbandwidth,asHWignorestheextrapadding+*dataautomatically.+*/+length=ALIGN(port_pr.buffer_size,4);++buf=vmalloc(length);
Since it may not be completely filled, would it be worthwhile to alloc
a zero'ed buff?
Hi Alan,
Thanks for the review, acutally per spec, hw doesn't care about the
extra padding data. So for now, i guess we don't need this.
Thanks
Hao
From: Alan Tull <atull@kernel.org> Date: 2019-03-26 19:34:34
On Mon, Mar 25, 2019 at 5:58 PM Scott Wood [off-list ref] wrote:
Hi Scott,
On Mon, 2019-03-25 at 17:53 -0500, Scott Wood wrote:
quoted
On Mon, 2019-03-25 at 11:07 +0800, Wu Hao wrote:
quoted
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-main.c | 3 ++
drivers/fpga/dfl-fme-mgr.c | 75 +++++++++++++++++++++++++++++++++++++-
--
-----
drivers/fpga/dfl-fme-pr.c | 45 ++++++++++++++++-----------
drivers/fpga/dfl-fme.h | 2 ++
drivers/fpga/dfl.h | 5 +++
5 files changed, 99 insertions(+), 31 deletions(-)
Shouldn't there be some sort of check that AVX512 is actually supported
on the running system?
Also, src should be const, and the asm statement should have a memory
clobber.
Likewise, this will be called if a revision 2 device is used on non-x86
(or on x86 with an old binutils). The driver should fall back to 32-bit
in such cases.
Sorry, I missed the comment about revision 2 only being on integrated
devices -- but will that always be the case? Seems worthwhile to check for
AVX512 support anyway. And there's still the possibility of being built
with an old binutils such that CONFIG_AS_AVX512 is not set, or running on a
kernel where avx512 was disabled via a boot option.
The code checks for CONFIG_AS_AVX512 above.
What boot option are you referring to?
Alan
What about future revisions >= 2? Currently the driver will treat them as
if they were revision < 2. Is that intended?
-Scott
Shouldn't there be some sort of check that AVX512 is actually
supported
on the running system?
Also, src should be const, and the asm statement should have a memory
clobber.
Likewise, this will be called if a revision 2 device is used on non-
x86
(or on x86 with an old binutils). The driver should fall back to 32-
bit
in such cases.
Sorry, I missed the comment about revision 2 only being on integrated
devices -- but will that always be the case? Seems worthwhile to check
for
AVX512 support anyway. And there's still the possibility of being built
with an old binutils such that CONFIG_AS_AVX512 is not set, or running
on a
kernel where avx512 was disabled via a boot option.
The code checks for CONFIG_AS_AVX512 above.
That just indicates that binutils supports it. Plus, the code does not
check for CONFIG_AS_AVX512 when deciding whether to set pr_datawidth to 64
(and thus call copy512), so you'll get a WARN_ON rather than falling back to
32-bit.
Shouldn't there be some sort of check that AVX512 is actually
supported
on the running system?
Also, src should be const, and the asm statement should have a memory
clobber.
Likewise, this will be called if a revision 2 device is used on non-
x86
(or on x86 with an old binutils). The driver should fall back to 32-
bit
in such cases.
Unfortunately revision 2 is only for integrated FPGA solution, and it doesn't
support any fallback solution (original 32bit data partial reconfiguration is
not supported any more), so driver has to WARN in such path.
quoted
quoted
Sorry, I missed the comment about revision 2 only being on integrated
devices -- but will that always be the case? Seems worthwhile to check
for
AVX512 support anyway. And there's still the possibility of being built
with an old binutils such that CONFIG_AS_AVX512 is not set, or running
on a
kernel where avx512 was disabled via a boot option.
The code checks for CONFIG_AS_AVX512 above.
That just indicates that binutils supports it. Plus, the code does not
check for CONFIG_AS_AVX512 when deciding whether to set pr_datawidth to 64
(and thus call copy512), so you'll get a WARN_ON rather than falling back to
32-bit.
quoted
What boot option are you referring to?
clearcpuid=304
Just tried it, my system was down after running above AVX512 with this option.
I agree that it needs to add some check code to make sure it's safe to run
such instructions. I will add some cpu_feature_enabled() check in the next
version.
Thanks a lot for the review and comments.
Hao
On Mon, Mar 25, 2019 at 05:58:36PM -0500, Scott Wood wrote:
On Mon, 2019-03-25 at 17:53 -0500, Scott Wood wrote:
quoted
On Mon, 2019-03-25 at 11:07 +0800, Wu Hao wrote:
quoted
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-main.c | 3 ++
drivers/fpga/dfl-fme-mgr.c | 75 +++++++++++++++++++++++++++++++++++++-
--
-----
drivers/fpga/dfl-fme-pr.c | 45 ++++++++++++++++-----------
drivers/fpga/dfl-fme.h | 2 ++
drivers/fpga/dfl.h | 5 +++
5 files changed, 99 insertions(+), 31 deletions(-)
What is this going to be used for? Under what circumstances will the
driver version be bumped? What does it have to do with 512-bit writes?
This patchset adds more features to this driver, so i would like to add
a DRV_VERSION there as an initial one. In the future, if some new features
or extensions for existing features (e.g. new revision of a private feature)
are added we need to bump this version.
Shouldn't there be some sort of check that AVX512 is actually supported
on the running system?
Also, src should be const, and the asm statement should have a memory
clobber.
Likewise, this will be called if a revision 2 device is used on non-x86
(or on x86 with an old binutils). The driver should fall back to 32-bit
in such cases.
Sorry, I missed the comment about revision 2 only being on integrated
devices -- but will that always be the case? Seems worthwhile to check for
AVX512 support anyway. And there's still the possibility of being built
with an old binutils such that CONFIG_AS_AVX512 is not set, or running on a
kernel where avx512 was disabled via a boot option.
What about future revisions >= 2? Currently the driver will treat them as
if they were revision < 2. Is that intended?
Yes, it's intended. Currently we don't have any hardware with revisions > 2,
and support new revisions may need new code. :) e.g. currently revision is
used to tell 32bit vs 512bit PR, but in future revisions, it may have new
capability registers for this purpose.
Thanks
Hao
This code is not needed at all but added by mistake i think.
Sorry, i should move these code into a separated patch with proper comments
to avoid confusion.
Thanks
Hao
Likewise, this will be called if a revision 2 device is used on non-
quoted
quoted
quoted
quoted
x86
(or on x86 with an old binutils). The driver should fall back to
32-
bit
in such cases.
Unfortunately revision 2 is only for integrated FPGA solution, and it
doesn't
support any fallback solution (original 32bit data partial reconfiguration
is
not supported any more), so driver has to WARN in such path.
From the commit message it seemed like this was just an optimization, not
something necessary to support revision 2.
If there's no way to program the device without AVX512, then printing an
error message and returning an error to userspace would be better than
WARN_ON, since it's not actually a kernel bug.
-Scott
Likewise, this will be called if a revision 2 device is used on non-
quoted
quoted
quoted
quoted
quoted
x86
(or on x86 with an old binutils). The driver should fall back to
32-
bit
in such cases.
Unfortunately revision 2 is only for integrated FPGA solution, and it
doesn't
support any fallback solution (original 32bit data partial reconfiguration
is
not supported any more), so driver has to WARN in such path.
quoted
From the commit message it seemed like this was just an optimization, not
something necessary to support revision 2.
If there's no way to program the device without AVX512, then printing an
error message and returning an error to userspace would be better than
WARN_ON, since it's not actually a kernel bug.
Fair enough. Will do. Thanks for the suggestion.
Hao
From: Scott Wood <hidden> Date: 2019-03-27 06:19:33
On Wed, 2019-03-27 at 13:10 +0800, Wu Hao wrote:
On Mon, Mar 25, 2019 at 05:58:36PM -0500, Scott Wood wrote:
quoted
On Mon, 2019-03-25 at 17:53 -0500, Scott Wood wrote:
quoted
On Mon, 2019-03-25 at 11:07 +0800, Wu Hao wrote:
quoted
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-main.c | 3 ++
drivers/fpga/dfl-fme-mgr.c | 75
+++++++++++++++++++++++++++++++++++++-
--
-----
drivers/fpga/dfl-fme-pr.c | 45 ++++++++++++++++-----------
drivers/fpga/dfl-fme.h | 2 ++
drivers/fpga/dfl.h | 5 +++
5 files changed, 99 insertions(+), 31 deletions(-)
What is this going to be used for? Under what circumstances will the
driver version be bumped? What does it have to do with 512-bit
writes?
This patchset adds more features to this driver, so i would like to add
a DRV_VERSION there as an initial one. In the future, if some new features
or extensions for existing features (e.g. new revision of a private
feature)
are added we need to bump this version.
This doesn't seem like a good way of advertising API availability... Besides
being awkward to query, what happens if a distro kernel has backported some
features but not others that came before? What does it advertise?
I'd suggest some sort of feature flag mechanism that can be queried via
ioctl (e.g. along the lines of KVM capabilities), if "try the API and fall
back if it fails" is unsatisfactory.
Plus, if it's about new APIs being exposed, this doesn't seem like the right
patch for it to be in...
quoted
Sorry, I missed the comment about revision 2 only being on integrated
devices -- but will that always be the case? Seems worthwhile to check
for
AVX512 support anyway. And there's still the possibility of being built
with an old binutils such that CONFIG_AS_AVX512 is not set, or running
on a
kernel where avx512 was disabled via a boot option.
What about future revisions >= 2? Currently the driver will treat them
as
if they were revision < 2. Is that intended?
Yes, it's intended. Currently we don't have any hardware with revisions >
2,
and support new revisions may need new code. :) e.g. currently revision
is
used to tell 32bit vs 512bit PR, but in future revisions, it may have new
capability registers for this purpose.
The driver should refuse to bind to unrecognized revisions, if they're not
expected to be compatible.
-Scott
On Wed, Mar 27, 2019 at 01:19:29AM -0500, Scott Wood wrote:
On Wed, 2019-03-27 at 13:10 +0800, Wu Hao wrote:
quoted
On Mon, Mar 25, 2019 at 05:58:36PM -0500, Scott Wood wrote:
quoted
On Mon, 2019-03-25 at 17:53 -0500, Scott Wood wrote:
quoted
On Mon, 2019-03-25 at 11:07 +0800, Wu Hao wrote:
quoted
In early partial reconfiguration private feature, it only
supports 32bit data width when writing data to hardware for
PR. 512bit data width PR support is an important optimization
for some specific solutions (e.g. XEON with FPGA integrated),
it allows driver to use AVX512 instruction to improve the
performance of partial reconfiguration. e.g. programming one
100MB bitstream image via this 512bit data width PR hardware
only takes ~300ms, but 32bit revision requires ~3s per test
result.
Please note now this optimization is only done on revision 2
of this PR private feature which is only used in integrated
solution that AVX512 is always supported.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-main.c | 3 ++
drivers/fpga/dfl-fme-mgr.c | 75
+++++++++++++++++++++++++++++++++++++-
--
-----
drivers/fpga/dfl-fme-pr.c | 45 ++++++++++++++++-----------
drivers/fpga/dfl-fme.h | 2 ++
drivers/fpga/dfl.h | 5 +++
5 files changed, 99 insertions(+), 31 deletions(-)
What is this going to be used for? Under what circumstances will the
driver version be bumped? What does it have to do with 512-bit
writes?
This patchset adds more features to this driver, so i would like to add
a DRV_VERSION there as an initial one. In the future, if some new features
or extensions for existing features (e.g. new revision of a private
feature)
are added we need to bump this version.
This doesn't seem like a good way of advertising API availability... Besides
being awkward to query, what happens if a distro kernel has backported some
features but not others that came before? What does it advertise?
DRV_VERSION here is not used for API availablity. :)
I'd suggest some sort of feature flag mechanism that can be queried via
ioctl (e.g. along the lines of KVM capabilities), if "try the API and fall
back if it fails" is unsatisfactory.
Plus, if it's about new APIs being exposed, this doesn't seem like the right
patch for it to be in...
Actually this patch doesn't introduce new APIs, I am trying to make this
transparent to endusers. That means users don't need to know it's a 32bit
PR or a faster 512bit one, they still use the same IOCTL interface for PR.
the API_VERSION and CHECK_EXTENSION ioctls have been defined, but I think
at least we don't need to bump them for this change. How do you think?
quoted
quoted
Sorry, I missed the comment about revision 2 only being on integrated
devices -- but will that always be the case? Seems worthwhile to check
for
AVX512 support anyway. And there's still the possibility of being built
with an old binutils such that CONFIG_AS_AVX512 is not set, or running
on a
kernel where avx512 was disabled via a boot option.
What about future revisions >= 2? Currently the driver will treat them
as
if they were revision < 2. Is that intended?
Yes, it's intended. Currently we don't have any hardware with revisions >
2,
and support new revisions may need new code. :) e.g. currently revision
is
used to tell 32bit vs 512bit PR, but in future revisions, it may have new
capability registers for this purpose.
The driver should refuse to bind to unrecognized revisions, if they're not
expected to be compatible.
From: Alan Tull <atull@kernel.org> Date: 2019-03-28 17:13:52
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
This patch introduces more sysfs interfaces for Accelerated
Function Unit (AFU). These interfaces allow users to read
current AFU Power State (APx), read / clear AFU Power (APx)
events which are sticky to identify transient APx state,
and manage AFU's LTR (latency tolerance reporting).
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
Acked-by: Alan Tull <atull@kernel.org>
Thanks,
Alan
From: Alan Tull <atull@kernel.org> Date: 2019-03-28 18:50:52
On Mon, Mar 25, 2019 at 7:44 PM Wu Hao [off-list ref] wrote:
On Mon, Mar 25, 2019 at 12:50:40PM -0500, Alan Tull wrote:
quoted
On Sun, Mar 24, 2019 at 10:23 PM Wu Hao [off-list ref] wrote:
Hi Hao,
Looks good, one question below.
quoted
Current driver checks if input bitstream file size is aligned or
not per PR data width (default 32bits). It requires one additional
step for end user when they generate the bitstream file, padding
extra zeros to bitstream file to align its size per PR data width,
but they don't have to as hardware will drop extra padding bytes
automatically.
In order to simplify the user steps, this patch aligns PR buffer
size per PR data width in driver, to allow user to pass unaligned
size bitstream files to driver.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
Acked-by: Alan Tull <atull@kernel.org>
Thanks,
Alan
From: Alan Tull <atull@kernel.org> Date: 2019-03-28 22:03:45
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
This patch enables the standard sriov support. It allows user to
enable SRIOV (and VFs), then user could pass through accelerators
(VFs) into virtual machine or use VFs directly in host.
Signed-off-by: Zhang Yi Z <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
From: Alan Tull <atull@kernel.org> Date: 2019-03-28 22:04:20
On Sun, Mar 24, 2019 at 10:23 PM Wu Hao [off-list ref] wrote:
In order to support virtualization usage via PCIe SRIOV, this patch
adds two ioctls under FPGA Management Engine (FME) to release and
assign back the port device. In order to safely turn Port from PF
into VF and enable PCIe SRIOV, it requires user to invoke this
PORT_RELEASE ioctl to release port firstly to remove userspace
interfaces, and then configure the PF/VF access register in FME.
After disable SRIOV, it requires user to invoke this PORT_ASSIGN
ioctl to attach the port back to PF.
Ioctl interfaces:
* DFL_FPGA_FME_PORT_RELEASE
Release platform device of given port, it deletes port platform
device to remove related userspace interfaces on PF, then
configures PF/VF access mode to VF.
* DFL_FPGA_FME_PORT_ASSIGN
Assign platform device of given port back to PF, it configures
PF/VF access mode to PF, then adds port platform device back to
re-enable related userspace interfaces on PF.
Signed-off-by: Zhang Yi Z <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
From: Moritz Fischer <mdf@kernel.org> Date: 2019-04-01 19:54:52
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:28AM +0800, Wu Hao wrote:
quoted hunk
FME_PR_INTFC_ID is used as compat_id for fpga manager and region,
but high 64 bits and low 64 bits of the compat_id are swapped by
mistake. This patch fixes this problem by fixing register address.
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-mgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Alan Tull <atull@kernel.org> Date: 2019-04-01 21:42:33
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
Looks fine.
This patch introduces userclock sysfs interfaces for AFU, user
could use these interfaces for clock setting to AFU.
Please note that, this is only working for port header feature
with revision 0, for later revisions, userclock setting is moved
to a separated private feature, so one revision sysfs interface
is exposed to userspace application for this purpose too.
Signed-off-by: Ananda Ravuri <redacted>
Signed-off-by: Russ Weight <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
On Mon, Apr 01, 2019 at 12:54:47PM -0700, Moritz Fischer wrote:
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:28AM +0800, Wu Hao wrote:
quoted
FME_PR_INTFC_ID is used as compat_id for fpga manager and region,
but high 64 bits and low 64 bits of the compat_id are swapped by
mistake. This patch fixes this problem by fixing register address.
Signed-off-by: Wu Hao <redacted>
---
drivers/fpga/dfl-fme-mgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Hi Moritz,
This is just a bug fixing for wrong offsets given to these 2 registers
according to spec. I think this is not endianess related, and per my
understanding we don't need more code on endianess handling as that
should be done inside the readq function already. :)
Thanks
Hao
From: Moritz Fischer <mdf@kernel.org> Date: 2019-04-02 13:33:50
Hi Wu,
On Tue, Apr 02, 2019 at 12:38:45PM +0800, Wu Hao wrote:
On Mon, Apr 01, 2019 at 12:54:47PM -0700, Moritz Fischer wrote:
quoted
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:28AM +0800, Wu Hao wrote:
quoted
FME_PR_INTFC_ID is used as compat_id for fpga manager and region,
but high 64 bits and low 64 bits of the compat_id are swapped by
mistake. This patch fixes this problem by fixing register address.
Signed-off-by: Wu Hao <redacted>
Hi Moritz,
This is just a bug fixing for wrong offsets given to these 2 registers
according to spec. I think this is not endianess related, and per my
understanding we don't need more code on endianess handling as that
should be done inside the readq function already. :)
Thanks
Hao
From: Moritz Fischer <mdf@kernel.org> Date: 2019-04-02 14:59:29
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:41AM +0800, Wu Hao wrote:
quoted hunk
This patch adds support to thermal management private feature for DFL
FPGA Management Engine (FME). As thermal throttling is handled by
hardware automatically per pre-defined thresholds, this private
feature driver only provides read-only sysfs interfaces for user
to read temperature, thresholds, threshold policy and other info.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Russ Weight <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++++
drivers/fpga/dfl-fme-main.c | 202 +++++++++++++++++++++++
2 files changed, 258 insertions(+)
@@ -44,3 +44,59 @@ Description: Read-only. It returns socket_id to indicate which socket this FPGA belongs to, only valid for integrated solution. User only needs this information, in case standard numa node can't provide correct information.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/temperature+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns temperature (in Celsius) of this FPGA+ device.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold1+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold2+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/trip_threshold+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns trip threshold (in Celsius), once FPGA+ temperature reaches trip threshold, it triggers a fatal event+ to board management controller (BMC) to shutdown FPGA.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold1,+ otherwise 0. Once temperature reaches threshold1, hardware+ will automatically enter throttling state (AP1 - 50%+ or AP2 - 90% throttling, see 'threshold1_policy').++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold2,+ otherwise 0. Once temperature reaches threshold2, hardware+ will automatically enter the deepest throttling state (AP6+ - 100% throttling).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_policy+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the policy of temperature+ threshold1. It only supports two value (policy):+ 0 - AP2 state (90% throttling)+ 1 - AP1 state (50% throttling)
These look like they could directly map to the linux thermal framework,
any reason you can't use the thermal framework?
The trip stuff literally maps 1:1 to what a thermal driver does, I think
that's something you'd wanna consider.
Cheers,
Moritz
From: Moritz Fischer <mdf@kernel.org> Date: 2019-04-02 15:07:25
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:39AM +0800, Wu Hao wrote:
STP (SignalTap) is one of the private features under the port for
debugging. This patch adds private feature driver support for it
to allow userspace applications to mmap related mmio region and
provide STP service.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
From: Moritz Fischer <mdf@kernel.org> Date: 2019-04-02 15:10:00
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:36AM +0800, Wu Hao wrote:
This patch adds id_table for each dfl private feature driver,
it allows to reuse same private feature driver to match and support
multiple dfl private features.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
@@ -274,6 +276,21 @@ static int dfl_feature_instance_init(struct platform_device *pdev,returnret;}+staticbooldfl_feature_drv_match(structdfl_feature*feature,+structdfl_feature_driver*driver)+{+conststructdfl_feature_id*ids=driver->id_table;++if(ids){+while(ids->id){+if(ids->id==feature->id)+returntrue;+ids++;+}+}+returnfalse;+}+/***dfl_fpga_dev_feature_init-initforsubfeaturesofdflfeaturedevice*@pdev:featuredevice.
@@ -294,8 +311,7 @@ int dfl_fpga_dev_feature_init(struct platform_device *pdev,while(drv->ops){dfl_fpga_dev_for_each_feature(pdata,feature){-/* match feature and drv using id */-if(feature->id==drv->id){+if(dfl_feature_drv_match(feature,drv)){ret=dfl_feature_instance_init(pdev,pdata,feature,drv);if(ret)
@@ -30,8 +30,8 @@/* plus one for fme device */#define MAX_DFL_FEATURE_DEV_NUM (MAX_DFL_FPGA_PORT_NUM + 1)-/* Reserved 0x0 for Header Group Register and 0xff for AFU */-#define FEATURE_ID_FIU_HEADER 0x0+/* Reserved 0xfe for Header Group Register and 0xff for AFU */+#define FEATURE_ID_FIU_HEADER 0xfe#define FEATURE_ID_AFU 0xff#define FME_FEATURE_ID_HEADER FEATURE_ID_FIU_HEADER
From: Moritz Fischer <mdf@kernel.org> Date: 2019-04-02 15:50:23
On Mon, Mar 25, 2019 at 11:07:37AM +0800, Wu Hao wrote:
As these two functions are used by other private features. e.g.
in error reporting private feature, it requires to check port status
and reset port for error clearing.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
On Tue, Apr 02, 2019 at 07:59:25AM -0700, Moritz Fischer wrote:
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:41AM +0800, Wu Hao wrote:
quoted
This patch adds support to thermal management private feature for DFL
FPGA Management Engine (FME). As thermal throttling is handled by
hardware automatically per pre-defined thresholds, this private
feature driver only provides read-only sysfs interfaces for user
to read temperature, thresholds, threshold policy and other info.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Russ Weight <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++++
drivers/fpga/dfl-fme-main.c | 202 +++++++++++++++++++++++
2 files changed, 258 insertions(+)
@@ -44,3 +44,59 @@ Description: Read-only. It returns socket_id to indicate which socket this FPGA belongs to, only valid for integrated solution. User only needs this information, in case standard numa node can't provide correct information.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/temperature+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns temperature (in Celsius) of this FPGA+ device.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold1+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold2+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/trip_threshold+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns trip threshold (in Celsius), once FPGA+ temperature reaches trip threshold, it triggers a fatal event+ to board management controller (BMC) to shutdown FPGA.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold1,+ otherwise 0. Once temperature reaches threshold1, hardware+ will automatically enter throttling state (AP1 - 50%+ or AP2 - 90% throttling, see 'threshold1_policy').++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold2,+ otherwise 0. Once temperature reaches threshold2, hardware+ will automatically enter the deepest throttling state (AP6+ - 100% throttling).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_policy+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the policy of temperature+ threshold1. It only supports two value (policy):+ 0 - AP2 state (90% throttling)+ 1 - AP1 state (50% throttling)
These look like they could directly map to the linux thermal framework,
any reason you can't use the thermal framework?
The trip stuff literally maps 1:1 to what a thermal driver does, I think
that's something you'd wanna consider.
Hi Moritz,
Thanks a lot for the suggestion, actually I feel that the trip points in thermal
zone are used to indicate cooling actions required for thermal software either
in kernel or userspace. But in this case, such FPGA hardware handles cooling
automatically (yes, driver only expose Read-only sysfs for information), so
software doesn't need to take care of this at all. For this purpose, it seems
that we don't have to put these thresholds as trip points. And per my
understanding, if people use such FPGA device, then they may need to know
what's the current hardware throttling behavior, e.g. 50% vs 90%. These
information can't be provided by standard thermal zone sysfs, so anyway user
needs these sysfs interfaces to know it. But it seems that we still could
create a thermal zone without trip points, it could help if user wants to
connect some external cooling devices via userspace thermal daemon, they can
define whatever trip points they like to activate the external cooling
device. I will consider this further more and come up with a new patch in
v2 patchset.
Thanks
Hao
From: Moritz Fischer <mdf@kernel.org> Date: 2019-04-03 18:09:13
Hi Hao,
On Thu, Apr 04, 2019 at 12:31:47AM +0800, Wu Hao wrote:
On Tue, Apr 02, 2019 at 07:59:25AM -0700, Moritz Fischer wrote:
quoted
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:41AM +0800, Wu Hao wrote:
quoted
This patch adds support to thermal management private feature for DFL
FPGA Management Engine (FME). As thermal throttling is handled by
hardware automatically per pre-defined thresholds, this private
feature driver only provides read-only sysfs interfaces for user
to read temperature, thresholds, threshold policy and other info.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Russ Weight <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++++
drivers/fpga/dfl-fme-main.c | 202 +++++++++++++++++++++++
2 files changed, 258 insertions(+)
@@ -44,3 +44,59 @@ Description: Read-only. It returns socket_id to indicate which socket this FPGA belongs to, only valid for integrated solution. User only needs this information, in case standard numa node can't provide correct information.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/temperature+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns temperature (in Celsius) of this FPGA+ device.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold1+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold2+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/trip_threshold+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns trip threshold (in Celsius), once FPGA+ temperature reaches trip threshold, it triggers a fatal event+ to board management controller (BMC) to shutdown FPGA.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold1,+ otherwise 0. Once temperature reaches threshold1, hardware+ will automatically enter throttling state (AP1 - 50%+ or AP2 - 90% throttling, see 'threshold1_policy').++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold2,+ otherwise 0. Once temperature reaches threshold2, hardware+ will automatically enter the deepest throttling state (AP6+ - 100% throttling).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_policy+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the policy of temperature+ threshold1. It only supports two value (policy):+ 0 - AP2 state (90% throttling)+ 1 - AP1 state (50% throttling)
These look like they could directly map to the linux thermal framework,
any reason you can't use the thermal framework?
The trip stuff literally maps 1:1 to what a thermal driver does, I think
that's something you'd wanna consider.
Hi Moritz,
Thanks a lot for the suggestion, actually I feel that the trip points in thermal
zone are used to indicate cooling actions required for thermal software either
in kernel or userspace. But in this case, such FPGA hardware handles cooling
automatically (yes, driver only expose Read-only sysfs for information), so
software doesn't need to take care of this at all. For this purpose, it seems
that we don't have to put these thresholds as trip points. And per my
understanding, if people use such FPGA device, then they may need to know
what's the current hardware throttling behavior, e.g. 50% vs 90%. These
information can't be provided by standard thermal zone sysfs, so anyway user
needs these sysfs interfaces to know it. But it seems that we still could
create a thermal zone without trip points, it could help if user wants to
connect some external cooling devices via userspace thermal daemon, they can
define whatever trip points they like to activate the external cooling
device. I will consider this further more and come up with a new patch in
v2 patchset.
Generally speaking extending an existing framework with the
functionality you want is preferable over rolling 100% your own.
So please look into this.
Thanks,
Moritz
On Wed, Apr 03, 2019 at 11:09:09AM -0700, Moritz Fischer wrote:
Hi Hao,
On Thu, Apr 04, 2019 at 12:31:47AM +0800, Wu Hao wrote:
quoted
On Tue, Apr 02, 2019 at 07:59:25AM -0700, Moritz Fischer wrote:
quoted
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:41AM +0800, Wu Hao wrote:
quoted
This patch adds support to thermal management private feature for DFL
FPGA Management Engine (FME). As thermal throttling is handled by
hardware automatically per pre-defined thresholds, this private
feature driver only provides read-only sysfs interfaces for user
to read temperature, thresholds, threshold policy and other info.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Russ Weight <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++++
drivers/fpga/dfl-fme-main.c | 202 +++++++++++++++++++++++
2 files changed, 258 insertions(+)
@@ -44,3 +44,59 @@ Description: Read-only. It returns socket_id to indicate which socket this FPGA belongs to, only valid for integrated solution. User only needs this information, in case standard numa node can't provide correct information.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/temperature+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns temperature (in Celsius) of this FPGA+ device.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold1+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the temperature threshold2+ (in Celsius).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/trip_threshold+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns trip threshold (in Celsius), once FPGA+ temperature reaches trip threshold, it triggers a fatal event+ to board management controller (BMC) to shutdown FPGA.++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold1,+ otherwise 0. Once temperature reaches threshold1, hardware+ will automatically enter throttling state (AP1 - 50%+ or AP2 - 90% throttling, see 'threshold1_policy').++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold2_status+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns 1 if temperature reaches threshold2,+ otherwise 0. Once temperature reaches threshold2, hardware+ will automatically enter the deepest throttling state (AP6+ - 100% throttling).++What: /sys/bus/platform/devices/dfl-fme.0/thermal_mgmt/threshold1_policy+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the policy of temperature+ threshold1. It only supports two value (policy):+ 0 - AP2 state (90% throttling)+ 1 - AP1 state (50% throttling)
These look like they could directly map to the linux thermal framework,
any reason you can't use the thermal framework?
The trip stuff literally maps 1:1 to what a thermal driver does, I think
that's something you'd wanna consider.
Hi Moritz,
Thanks a lot for the suggestion, actually I feel that the trip points in thermal
zone are used to indicate cooling actions required for thermal software either
in kernel or userspace. But in this case, such FPGA hardware handles cooling
automatically (yes, driver only expose Read-only sysfs for information), so
software doesn't need to take care of this at all. For this purpose, it seems
that we don't have to put these thresholds as trip points. And per my
understanding, if people use such FPGA device, then they may need to know
what's the current hardware throttling behavior, e.g. 50% vs 90%. These
information can't be provided by standard thermal zone sysfs, so anyway user
needs these sysfs interfaces to know it. But it seems that we still could
create a thermal zone without trip points, it could help if user wants to
connect some external cooling devices via userspace thermal daemon, they can
define whatever trip points they like to activate the external cooling
device. I will consider this further more and come up with a new patch in
v2 patchset.
Generally speaking extending an existing framework with the
functionality you want is preferable over rolling 100% your own.
So please look into this.
Yes, agree, will look into this and try to fix this in next version.
Thanks for the comments.
Hao
From: Alan Tull <atull@kernel.org> Date: 2019-04-09 20:58:18
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
quoted hunk
Error reporting is one important private feature, it reports error
detected on port and accelerated function unit (AFU). It introduces
several sysfs interfaces to allow userspace to check and clear
errors detected by hardware.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-port | 29 +++
drivers/fpga/Makefile | 1 +
drivers/fpga/dfl-afu-error.c | 225 ++++++++++++++++++++++
drivers/fpga/dfl-afu-main.c | 4 +
drivers/fpga/dfl-afu.h | 4 +
5 files changed, 263 insertions(+)
create mode 100644 drivers/fpga/dfl-afu-error.c
@@ -79,3 +79,32 @@ KernelVersion: 5.2 Contact: Wu Hao <hao.wu@intel.com> Description: Read-only. Read this file to get the status of issued command to userclck_freqcntrcmd.++What: /sys/bus/platform/devices/dfl-port.0/errors/errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get errors detected on port and+ Accelerated Function Unit (AFU).++What: /sys/bus/platform/devices/dfl-port.0/errors/first_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-port.0/errors/first_malformed_req+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first malformed request+ captured by hardware.++What: /sys/bus/platform/devices/dfl-port.0/errors/clear+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Write-only. Write error code to this file to clear errors. If+ the input error code doesn't match, it returns -EBUSY error+ code.
I understand how -EBUSY could be the right error code for when the
hardware is in a state where the error can't be cleared. But if the
input error code doesn't match, shouldn't the code be -EINVAL? Also
as noted below, the way this is currently coded, -ETIMEDOUT could get
returned.
@@ -0,0 +1,225 @@+// SPDX-License-Identifier: GPL-2.0+/*+*DriverforFPGAAcceleratedFunctionUnit(AFU)ErrorReporting+*+*Copyright2019IntelCorporation,Inc.+*+*Authors:+*WuHao<hao.wu@linux.intel.com>+*XiaoGuangrong<guangrong.xiao@linux.intel.com>+*JosephGrecco<joe.grecco@intel.com>+*EnnoLuebbers<enno.luebbers@intel.com>+*TimWhisonant<tim.whisonant@intel.com>+*AnandaRavuri<ananda.ravuri@intel.com>+*MitchelHenry<henry.mitchel@intel.com>+*/++#include<linux/uaccess.h>++#include"dfl-afu.h"++#define PORT_ERROR_MASK 0x8+#define PORT_ERROR 0x10+#define PORT_FIRST_ERROR 0x18+#define PORT_MALFORMED_REQ0 0x20+#define PORT_MALFORMED_REQ1 0x28++#define ERROR_MASK GENMASK_ULL(63, 0)++/* mask or unmask port errors by the error mask register. */+staticvoid__port_err_mask(structdevice*dev,boolmask)+{+void__iomem*base;++base=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);++writeq(mask?ERROR_MASK:0,base+PORT_ERROR_MASK);+}++/* clear port errors. */+staticint__port_err_clear(structdevice*dev,u64err)+{+structplatform_device*pdev=to_platform_device(dev);+void__iomem*base_err,*base_hdr;+intret;+u64v;++base_err=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);+base_hdr=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_HEADER);++/*+*clearPortErrors+*+*-CheckforAP6State+*-HaltPortbykeepingPortinreset+*-SetPORTErrormasktoall1tomaskerrors+*-Clearallerrors+*-SetPortmasktoall0toenableerrors+*-Allerrorsstartcapturingnewerrors+*-EnablePortbypullingtheportoutofreset+*/++/* if device is still in AP6 power state, can not clear any error. */+v=readq(base_hdr+PORT_HDR_STS);+if(FIELD_GET(PORT_STS_PWR_STATE,v)==PORT_STS_PWR_STATE_AP6){+dev_err(dev,"Could not clear errors, device in AP6 state.\n");+return-EBUSY;+}++/* Halt Port by keeping Port in reset */+ret=__port_disable(pdev);+if(ret)+returnret;
__port_disable can return -ETIMEDOUT which will then get returned from
clear_store. The sysfs document only talks about -EBUSY. You could
either document -ETIMEDOUT in the sysfs doc or you could change the
code to adjust the returned error code.
+
+ /* Mask all errors */
+ __port_err_mask(dev, true);
+
+ /* Clear errors if err input matches with current port errors.*/
+ v = readq(base_err + PORT_ERROR);
+
+ if (v == err) {
+ writeq(v, base_err + PORT_ERROR);
+
+ v = readq(base_err + PORT_FIRST_ERROR);
+ writeq(v, base_err + PORT_FIRST_ERROR);
+ } else {
+ ret = -EBUSY;
+ }
+
+ /* Clear mask */
+ __port_err_mask(dev, false);
+
+ /* Enable the Port by clear the reset */
+ __port_enable(pdev);
+
+ return ret;
+}
+
+static ssize_t revision_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
+
+ return scnprintf(buf, PAGE_SIZE, "%u\n", dfl_feature_revision(base));
+}
+static DEVICE_ATTR_RO(revision);
This appears to be adding a
/sys/bus/platform/devices/dfl-port.0/errors/revision attribute that
isn't documented in the sysfs document.
@@ -21,3 +21,26 @@ Contact: Wu Hao <hao.wu@intel.com> Description: Read-only. It returns Bitstream (static FPGA region) meta data, which includes the synthesis date, seed and other information of this static FPGA region.++What: /sys/bus/platform/devices/dfl-fme.0/cache_size+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns cache size of this FPGA device.++What: /sys/bus/platform/devices/dfl-fme.0/fabric_version+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns fabric version of this FPGA device.+ Userspace applications need this information to select+ best data channels per different fabric design.++What: /sys/bus/platform/devices/dfl-fme.0/socket_id+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns socket_id to indicate which socket+ this FPGA belongs to, only valid for integrated solution.+ User only needs this information, in case standard numa node+ can't provide correct information.
@@ -156,3 +156,61 @@ KernelVersion: 5.2 Contact: Wu Hao <hao.wu@intel.com> Description: Read-only. Read this file to get power limit for fpga, it is only valid for integrated solution.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get errors detected by hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/first_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/next_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the second error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/clear+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Write-only. Write error code to this file to clear errors. If+ the input error code doesn't match, it returns -EBUSY.
As with the afu errors patch, seems like -EINVAL would be better.
quoted hunk
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/pcie0_errors
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns errors detected on pcie0 link.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/pcie1_errors
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns errors detected on pcie1 link.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/nonfatal_errors
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns non-fatal errors detected.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/catfatal_errors
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns catastrophic and fatal errors detected.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/inject_error
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Write this file to inject errors for testing
+ purpose. Read this file to check errors injected.
from kstrtox.c:
* Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
Your sysfs doc could be updated to explain these return codes.
+
+ feature = dfl_get_feature_by_id(err_dev, FME_FEATURE_ID_GLOBAL_ERR);
+ base = feature->ioaddr;
+
+ mutex_lock(&pdata->lock);
+ writeq(GENMASK_ULL(63, 0), base + FME_ERROR_MASK);
+
+ v = readq(base + FME_ERROR);
+ if (val != v) {
+ ret = -EINVAL;
Oh wait, that's what I thought it should be ;) so the doc just needs to change.
+ goto done;
It would be easy to avoid using 'goto' here.
+ }
+
+ writeq(v, base + FME_ERROR);
+ v = readq(base + FME_FIRST_ERROR);
+ writeq(v, base + FME_FIRST_ERROR);
+ v = readq(base + FME_NEXT_ERROR);
+ writeq(v, base + FME_NEXT_ERROR);
+
+done:
+ /* Workaround: disable MBP_ERROR if feature revision is 0 */
+ writeq(dfl_feature_revision(feature->ioaddr) ? 0ULL : MBP_ERROR,
+ base + FME_ERROR_MASK);
+ mutex_unlock(&pdata->lock);
+ return ret ? ret : count;
+}
+static DEVICE_ATTR_WO(clear);
+
+static ssize_t revision_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct device *err_dev = dev->parent;
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(err_dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ return scnprintf(buf, PAGE_SIZE, "%u\n", dfl_feature_revision(base));
+}
+static DEVICE_ATTR_RO(revision);
The revision attr to be documented in the sysfs doc.
@@ -156,3 +156,61 @@ KernelVersion: 5.2 Contact: Wu Hao <hao.wu@intel.com> Description: Read-only. Read this file to get power limit for fpga, it is only valid for integrated solution.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get errors detected by hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/first_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/next_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the second error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-fme.0/errors/fme-errors/clear+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Write-only. Write error code to this file to clear errors. If+ the input error code doesn't match, it returns -EBUSY.
As with the afu errors patch, seems like -EINVAL would be better.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/pcie0_errors
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns errors detected on pcie0 link.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/pcie1_errors
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns errors detected on pcie1 link.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/nonfatal_errors
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns non-fatal errors detected.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/catfatal_errors
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns catastrophic and fatal errors detected.
+
+What: /sys/bus/platform/devices/dfl-fme.0/errors/inject_error
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Write this file to inject errors for testing
+ purpose. Read this file to check errors injected.
from kstrtox.c:
* Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
Your sysfs doc could be updated to explain these return codes.
Sure, will fix this.
quoted
+
+ feature = dfl_get_feature_by_id(err_dev, FME_FEATURE_ID_GLOBAL_ERR);
+ base = feature->ioaddr;
+
+ mutex_lock(&pdata->lock);
+ writeq(GENMASK_ULL(63, 0), base + FME_ERROR_MASK);
+
+ v = readq(base + FME_ERROR);
+ if (val != v) {
+ ret = -EINVAL;
Oh wait, that's what I thought it should be ;) so the doc just needs to change.
Sure, will capture the detailed return values in doc.
quoted
+ goto done;
It would be easy to avoid using 'goto' here.
Yes, agree.
quoted
+ }
+
+ writeq(v, base + FME_ERROR);
+ v = readq(base + FME_FIRST_ERROR);
+ writeq(v, base + FME_FIRST_ERROR);
+ v = readq(base + FME_NEXT_ERROR);
+ writeq(v, base + FME_NEXT_ERROR);
+
+done:
+ /* Workaround: disable MBP_ERROR if feature revision is 0 */
+ writeq(dfl_feature_revision(feature->ioaddr) ? 0ULL : MBP_ERROR,
+ base + FME_ERROR_MASK);
+ mutex_unlock(&pdata->lock);
+ return ret ? ret : count;
+}
+static DEVICE_ATTR_WO(clear);
+
+static ssize_t revision_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct device *err_dev = dev->parent;
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(err_dev, FME_FEATURE_ID_GLOBAL_ERR);
+
+ return scnprintf(buf, PAGE_SIZE, "%u\n", dfl_feature_revision(base));
+}
+static DEVICE_ATTR_RO(revision);
The revision attr to be documented in the sysfs doc.
On Tue, Apr 09, 2019 at 03:57:37PM -0500, Alan Tull wrote:
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
quoted
Error reporting is one important private feature, it reports error
detected on port and accelerated function unit (AFU). It introduces
several sysfs interfaces to allow userspace to check and clear
errors detected by hardware.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-port | 29 +++
drivers/fpga/Makefile | 1 +
drivers/fpga/dfl-afu-error.c | 225 ++++++++++++++++++++++
drivers/fpga/dfl-afu-main.c | 4 +
drivers/fpga/dfl-afu.h | 4 +
5 files changed, 263 insertions(+)
create mode 100644 drivers/fpga/dfl-afu-error.c
@@ -79,3 +79,32 @@ KernelVersion: 5.2 Contact: Wu Hao <hao.wu@intel.com> Description: Read-only. Read this file to get the status of issued command to userclck_freqcntrcmd.++What: /sys/bus/platform/devices/dfl-port.0/errors/errors+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get errors detected on port and+ Accelerated Function Unit (AFU).++What: /sys/bus/platform/devices/dfl-port.0/errors/first_error+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first error detected by+ hardware.++What: /sys/bus/platform/devices/dfl-port.0/errors/first_malformed_req+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. Read this file to get the first malformed request+ captured by hardware.++What: /sys/bus/platform/devices/dfl-port.0/errors/clear+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Write-only. Write error code to this file to clear errors. If+ the input error code doesn't match, it returns -EBUSY error+ code.
I understand how -EBUSY could be the right error code for when the
hardware is in a state where the error can't be cleared. But if the
input error code doesn't match, shouldn't the code be -EINVAL? Also
as noted below, the way this is currently coded, -ETIMEDOUT could get
returned.
Thanks for the comments, let me try to capture all possible error return
values in doc in the next version to avoid confusion.
@@ -0,0 +1,225 @@+// SPDX-License-Identifier: GPL-2.0+/*+*DriverforFPGAAcceleratedFunctionUnit(AFU)ErrorReporting+*+*Copyright2019IntelCorporation,Inc.+*+*Authors:+*WuHao<hao.wu@linux.intel.com>+*XiaoGuangrong<guangrong.xiao@linux.intel.com>+*JosephGrecco<joe.grecco@intel.com>+*EnnoLuebbers<enno.luebbers@intel.com>+*TimWhisonant<tim.whisonant@intel.com>+*AnandaRavuri<ananda.ravuri@intel.com>+*MitchelHenry<henry.mitchel@intel.com>+*/++#include<linux/uaccess.h>++#include"dfl-afu.h"++#define PORT_ERROR_MASK 0x8+#define PORT_ERROR 0x10+#define PORT_FIRST_ERROR 0x18+#define PORT_MALFORMED_REQ0 0x20+#define PORT_MALFORMED_REQ1 0x28++#define ERROR_MASK GENMASK_ULL(63, 0)++/* mask or unmask port errors by the error mask register. */+staticvoid__port_err_mask(structdevice*dev,boolmask)+{+void__iomem*base;++base=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);++writeq(mask?ERROR_MASK:0,base+PORT_ERROR_MASK);+}++/* clear port errors. */+staticint__port_err_clear(structdevice*dev,u64err)+{+structplatform_device*pdev=to_platform_device(dev);+void__iomem*base_err,*base_hdr;+intret;+u64v;++base_err=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_ERROR);+base_hdr=dfl_get_feature_ioaddr_by_id(dev,PORT_FEATURE_ID_HEADER);++/*+*clearPortErrors+*+*-CheckforAP6State+*-HaltPortbykeepingPortinreset+*-SetPORTErrormasktoall1tomaskerrors+*-Clearallerrors+*-SetPortmasktoall0toenableerrors+*-Allerrorsstartcapturingnewerrors+*-EnablePortbypullingtheportoutofreset+*/++/* if device is still in AP6 power state, can not clear any error. */+v=readq(base_hdr+PORT_HDR_STS);+if(FIELD_GET(PORT_STS_PWR_STATE,v)==PORT_STS_PWR_STATE_AP6){+dev_err(dev,"Could not clear errors, device in AP6 state.\n");+return-EBUSY;+}++/* Halt Port by keeping Port in reset */+ret=__port_disable(pdev);+if(ret)+returnret;
__port_disable can return -ETIMEDOUT which will then get returned from
clear_store. The sysfs document only talks about -EBUSY. You could
either document -ETIMEDOUT in the sysfs doc or you could change the
code to adjust the returned error code.
Yes, agree.
quoted
+
+ /* Mask all errors */
+ __port_err_mask(dev, true);
+
+ /* Clear errors if err input matches with current port errors.*/
+ v = readq(base_err + PORT_ERROR);
+
+ if (v == err) {
+ writeq(v, base_err + PORT_ERROR);
+
+ v = readq(base_err + PORT_FIRST_ERROR);
+ writeq(v, base_err + PORT_FIRST_ERROR);
+ } else {
+ ret = -EBUSY;
+ }
+
+ /* Clear mask */
+ __port_err_mask(dev, false);
+
+ /* Enable the Port by clear the reset */
+ __port_enable(pdev);
+
+ return ret;
+}
+
+static ssize_t revision_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ void __iomem *base;
+
+ base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
+
+ return scnprintf(buf, PAGE_SIZE, "%u\n", dfl_feature_revision(base));
+}
+static DEVICE_ATTR_RO(revision);
This appears to be adding a
/sys/bus/platform/devices/dfl-port.0/errors/revision attribute that
isn't documented in the sysfs document.
Sorry, will fix all above issues in the next version.
Thanks again for the code review and comments.
Hao
From: Alan Tull <atull@kernel.org> Date: 2019-04-11 20:08:16
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
This patch adds support for power management private feature under
FPGA Management Engine (FME), sysfs interfaces are introduced for
different power management functions, users could use these sysfs
interface to get current number of consumed power, throttling
How about
s/number/measurement/
?
quoted hunk
thresholds, threshold status and other information, and configure
different value for throttling thresholds too.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++
drivers/fpga/dfl-fme-main.c | 257 +++++++++++++++++++++++
2 files changed, 313 insertions(+)
@@ -100,3 +100,59 @@ Description: Read-only. Read this file to get the policy of temperature threshold1. It only supports two value (policy): 0 - AP2 state (90% throttling) 1 - AP1 state (50% throttling)++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/consumed+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns current power consumed by FPGA.
What are the units?
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold1 in Watts.
Perhaps document error codes here and for threshold2 below.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold2 in Watts.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold1, otherwise 0.
I'm used to things like this requiring user to reset the status, so it
may be worth making it explicit that it will return to zero if
consumption drops below threshold if that's what's happening here.
If it's correct, perhaps could just say something like 'returns 1 if
power consumption is currently at or above threshold1, otherwise 0'
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold2, otherwise 0.
Same here.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/ltr
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get current Latency Tolerance
+ Reporting (ltr) value, it's only valid for integrated
+ solution as it blocks CPU on low power state.
If we're not on the integrated solution, it returns a value but it is
not really real?
quoted hunk
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/xeon_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for xeon, it
+ is only valid for integrated solution.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/fpga_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for fpga, it
+ is only valid for integrated solution.
This is a nit, but I would expect to see these listed in the same
order as their show/store functions above. So ltr_attr would come
between threshold2_status_attr and xeon_limit_attr.
From: Alan Tull <atull@kernel.org> Date: 2019-04-11 20:42:16
On Tue, Apr 2, 2019 at 10:07 AM Moritz Fischer [off-list ref] wrote:
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:39AM +0800, Wu Hao wrote:
quoted
STP (SignalTap) is one of the private features under the port for
debugging. This patch adds private feature driver support for it
to allow userspace applications to mmap related mmio region and
provide STP service.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
Acked-by: Moritz Fischer <mdf@kernel.org>
Acked-by: Alan Tull <atull@kernel.org>
Thanks,
Alan
From: Alan Tull <atull@kernel.org> Date: 2019-04-11 20:46:35
On Tue, Apr 2, 2019 at 10:50 AM Moritz Fischer [off-list ref] wrote:
Hi Hao,
On Mon, Mar 25, 2019 at 11:07:37AM +0800, Wu Hao wrote:
quoted
As these two functions are used by other private features. e.g.
in error reporting private feature, it requires to check port status
and reset port for error clearing.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
Acked-by: Moritz Fischer <mdf@kernel.org>
Acked-by: Alan Tull <atull@kernel.org>
Thanks,
Alan
From: Alan Tull <atull@kernel.org> Date: 2019-04-11 20:56:06
On Tue, Apr 2, 2019 at 10:09 AM Moritz Fischer [off-list ref] wrote:
Hi Wu,
On Mon, Mar 25, 2019 at 11:07:36AM +0800, Wu Hao wrote:
quoted
This patch adds id_table for each dfl private feature driver,
it allows to reuse same private feature driver to match and support
multiple dfl private features.
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
Acked-by: Moritz Fischer <mdf@kernel.org>
Acked-by: Alan Tull <atull@kernel.org>
Thanks,
Alan
On Thu, Apr 11, 2019 at 03:07:35PM -0500, Alan Tull wrote:
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
quoted
This patch adds support for power management private feature under
FPGA Management Engine (FME), sysfs interfaces are introduced for
different power management functions, users could use these sysfs
interface to get current number of consumed power, throttling
How about
s/number/measurement/
?
Sounds better. : )
quoted
thresholds, threshold status and other information, and configure
different value for throttling thresholds too.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++
drivers/fpga/dfl-fme-main.c | 257 +++++++++++++++++++++++
2 files changed, 313 insertions(+)
@@ -100,3 +100,59 @@ Description: Read-only. Read this file to get the policy of temperature threshold1. It only supports two value (policy): 0 - AP2 state (90% throttling) 1 - AP1 state (50% throttling)++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/consumed+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns current power consumed by FPGA.
What are the units?
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold1 in Watts.
Perhaps document error codes here and for threshold2 below.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold2 in Watts.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold1, otherwise 0.
I'm used to things like this requiring user to reset the status, so it
may be worth making it explicit that it will return to zero if
consumption drops below threshold if that's what's happening here.
If it's correct, perhaps could just say something like 'returns 1 if
power consumption is currently at or above threshold1, otherwise 0'
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold2, otherwise 0.
Same here.
Sure, will fix all above comments in this sysfs doc.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/ltr
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get current Latency Tolerance
+ Reporting (ltr) value, it's only valid for integrated
+ solution as it blocks CPU on low power state.
If we're not on the integrated solution, it returns a value but it is
not really real?
Currently only integrated solution is implementing this private feature, other
devices e.g. Intel PAC card is not using this private feature, so user will
not see these sysfs interfaces at all.
If in the future, other devices want
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/xeon_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for xeon, it
+ is only valid for integrated solution.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/fpga_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for fpga, it
+ is only valid for integrated solution.
Are these #defines necessary? Seems like you could just use DEVICE_ATTR*
Actually it adds a prefix power_attr_xxx there to avoid name conflicts with
other ones from different private features, e.g. for the thermal threshold.
This is a nit, but I would expect to see these listed in the same
order as their show/store functions above. So ltr_attr would come
between threshold2_status_attr and xeon_limit_attr.
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
quoted
This patch adds support for power management private feature under
FPGA Management Engine (FME), sysfs interfaces are introduced for
different power management functions, users could use these sysfs
interface to get current number of consumed power, throttling
How about
s/number/measurement/
?
quoted
thresholds, threshold status and other information, and configure
different value for throttling thresholds too.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++
drivers/fpga/dfl-fme-main.c | 257 +++++++++++++++++++++++
2 files changed, 313 insertions(+)
@@ -100,3 +100,59 @@ Description: Read-only. Read this file to get the policy of temperature threshold1. It only supports two value (policy): 0 - AP2 state (90% throttling) 1 - AP1 state (50% throttling)++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/consumed+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns current power consumed by FPGA.
What are the units?
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold1 in Watts.
Perhaps document error codes here and for threshold2 below.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold2 in Watts.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold1, otherwise 0.
I'm used to things like this requiring user to reset the status, so it
may be worth making it explicit that it will return to zero if
consumption drops below threshold if that's what's happening here.
If it's correct, perhaps could just say something like 'returns 1 if
power consumption is currently at or above threshold1, otherwise 0'
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold2, otherwise 0.
Same here.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/ltr
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get current Latency Tolerance
+ Reporting (ltr) value, it's only valid for integrated
+ solution as it blocks CPU on low power state.
If we're not on the integrated solution, it returns a value but it is
not really real?
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/xeon_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for xeon, it
+ is only valid for integrated solution.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/fpga_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for fpga, it
+ is only valid for integrated solution.
This is a nit, but I would expect to see these listed in the same
order as their show/store functions above. So ltr_attr would come
between threshold2_status_attr and xeon_limit_attr.
From: Alan Tull <atull@kernel.org> Date: 2019-04-15 21:18:31
On Thu, Apr 11, 2019 at 10:06 PM Wu Hao [off-list ref] wrote:
On Thu, Apr 11, 2019 at 03:07:35PM -0500, Alan Tull wrote:
quoted
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
quoted
This patch adds support for power management private feature under
FPGA Management Engine (FME), sysfs interfaces are introduced for
different power management functions, users could use these sysfs
interface to get current number of consumed power, throttling
How about
s/number/measurement/
?
Sounds better. : )
quoted
quoted
thresholds, threshold status and other information, and configure
different value for throttling thresholds too.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++
drivers/fpga/dfl-fme-main.c | 257 +++++++++++++++++++++++
2 files changed, 313 insertions(+)
@@ -100,3 +100,59 @@ Description: Read-only. Read this file to get the policy of temperature threshold1. It only supports two value (policy): 0 - AP2 state (90% throttling) 1 - AP1 state (50% throttling)++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/consumed+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns current power consumed by FPGA.
What are the units?
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold1 in Watts.
Perhaps document error codes here and for threshold2 below.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold2 in Watts.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold1, otherwise 0.
I'm used to things like this requiring user to reset the status, so it
may be worth making it explicit that it will return to zero if
consumption drops below threshold if that's what's happening here.
If it's correct, perhaps could just say something like 'returns 1 if
power consumption is currently at or above threshold1, otherwise 0'
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold2, otherwise 0.
Same here.
Sure, will fix all above comments in this sysfs doc.
quoted
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/ltr
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get current Latency Tolerance
+ Reporting (ltr) value, it's only valid for integrated
+ solution as it blocks CPU on low power state.
If we're not on the integrated solution, it returns a value but it is
not really real?
Currently only integrated solution is implementing this private feature, other
devices e.g. Intel PAC card is not using this private feature, so user will
not see these sysfs interfaces at all.
OK then perhaps the "it's only valid for integrated solution as it
blocks CPU on low power state" explanation doesn't need to be here and
can lead to confusion.
If in the future, other devices want
quoted
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/xeon_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for xeon, it
+ is only valid for integrated solution.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/fpga_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for fpga, it
+ is only valid for integrated solution.
Are these #defines necessary? Seems like you could just use DEVICE_ATTR*
Actually it adds a prefix power_attr_xxx there to avoid name conflicts with
other ones from different private features, e.g. for the thermal threshold.
This is a nit, but I would expect to see these listed in the same
order as their show/store functions above. So ltr_attr would come
between threshold2_status_attr and xeon_limit_attr.
Hi Moritz,
Thanks a lot for the suggestion, yes, agree, and patch for thermal
management should be the similar case too. Let me see if i can make
thermal / power management code to hwmon in the next version. : )
Hao
Cheers,
Moritz
On Thu, Apr 11, 2019 at 1:08 PM Alan Tull [off-list ref] wrote:
quoted
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
quoted
This patch adds support for power management private feature under
FPGA Management Engine (FME), sysfs interfaces are introduced for
different power management functions, users could use these sysfs
interface to get current number of consumed power, throttling
How about
s/number/measurement/
?
quoted
thresholds, threshold status and other information, and configure
different value for throttling thresholds too.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++
drivers/fpga/dfl-fme-main.c | 257 +++++++++++++++++++++++
2 files changed, 313 insertions(+)
@@ -100,3 +100,59 @@ Description: Read-only. Read this file to get the policy of temperature threshold1. It only supports two value (policy): 0 - AP2 state (90% throttling) 1 - AP1 state (50% throttling)++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/consumed+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns current power consumed by FPGA.
What are the units?
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold1 in Watts.
Perhaps document error codes here and for threshold2 below.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold2 in Watts.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold1, otherwise 0.
I'm used to things like this requiring user to reset the status, so it
may be worth making it explicit that it will return to zero if
consumption drops below threshold if that's what's happening here.
If it's correct, perhaps could just say something like 'returns 1 if
power consumption is currently at or above threshold1, otherwise 0'
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold2, otherwise 0.
Same here.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/ltr
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get current Latency Tolerance
+ Reporting (ltr) value, it's only valid for integrated
+ solution as it blocks CPU on low power state.
If we're not on the integrated solution, it returns a value but it is
not really real?
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/xeon_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for xeon, it
+ is only valid for integrated solution.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/fpga_limit
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get power limit for fpga, it
+ is only valid for integrated solution.
This is a nit, but I would expect to see these listed in the same
order as their show/store functions above. So ltr_attr would come
between threshold2_status_attr and xeon_limit_attr.
On Mon, Apr 15, 2019 at 04:17:48PM -0500, Alan Tull wrote:
On Thu, Apr 11, 2019 at 10:06 PM Wu Hao [off-list ref] wrote:
quoted
On Thu, Apr 11, 2019 at 03:07:35PM -0500, Alan Tull wrote:
quoted
On Sun, Mar 24, 2019 at 10:24 PM Wu Hao [off-list ref] wrote:
Hi Hao,
quoted
This patch adds support for power management private feature under
FPGA Management Engine (FME), sysfs interfaces are introduced for
different power management functions, users could use these sysfs
interface to get current number of consumed power, throttling
How about
s/number/measurement/
?
Sounds better. : )
quoted
quoted
thresholds, threshold status and other information, and configure
different value for throttling thresholds too.
Signed-off-by: Luwei Kang <redacted>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <redacted>
---
Documentation/ABI/testing/sysfs-platform-dfl-fme | 56 +++++
drivers/fpga/dfl-fme-main.c | 257 +++++++++++++++++++++++
2 files changed, 313 insertions(+)
@@ -100,3 +100,59 @@ Description: Read-only. Read this file to get the policy of temperature threshold1. It only supports two value (policy): 0 - AP2 state (90% throttling) 1 - AP1 state (50% throttling)++What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/consumed+Date: March 2019+KernelVersion: 5.2+Contact: Wu Hao <hao.wu@intel.com>+Description: Read-only. It returns current power consumed by FPGA.
What are the units?
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold1 in Watts.
Perhaps document error codes here and for threshold2 below.
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-Write. Read/Write this file to get/set current power
+ threshold2 in Watts.
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold1_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold1, otherwise 0.
I'm used to things like this requiring user to reset the status, so it
may be worth making it explicit that it will return to zero if
consumption drops below threshold if that's what's happening here.
If it's correct, perhaps could just say something like 'returns 1 if
power consumption is currently at or above threshold1, otherwise 0'
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/threshold2_status
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. It returns 1 if power consumption reaches the
+ threshold2, otherwise 0.
Same here.
Sure, will fix all above comments in this sysfs doc.
quoted
quoted
+
+What: /sys/bus/platform/devices/dfl-fme.0/power_mgmt/ltr
+Date: March 2019
+KernelVersion: 5.2
+Contact: Wu Hao [off-list ref]
+Description: Read-only. Read this file to get current Latency Tolerance
+ Reporting (ltr) value, it's only valid for integrated
+ solution as it blocks CPU on low power state.
If we're not on the integrated solution, it returns a value but it is
not really real?
Currently only integrated solution is implementing this private feature, other
devices e.g. Intel PAC card is not using this private feature, so user will
not see these sysfs interfaces at all.
OK then perhaps the "it's only valid for integrated solution as it
blocks CPU on low power state" explanation doesn't need to be here and
can lead to confusion.
Sure, will fix it in the next version. Thanks!
Hao