tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

firmware: stratix10-svc: Add Multi SVC clients support

In the current implementation, SVC client drivers such as socfpga-hwmon,
intel_fcs, stratix10-soc, stratix10-rsu each send an SMC command that
triggers a single thread in the stratix10-svc driver. Upon receiving a
callback, the initiating client driver sends a stratix10-svc-done signal,
terminating the thread without waiting for other pending SMC commands to
complete. This leads to a timeout issue in the firmware SVC mailbox service
when multiple client drivers send SMC commands concurrently.

To resolve this issue, a dedicated thread is now created per channel. The
stratix10-svc driver will support up to the number of channels defined by
SVC_NUM_CHANNEL. Thread synchronization is handled using a mutex to prevent
simultaneous issuance of SMC commands by multiple threads.

SVC_NUM_DATA_IN_FIFO is reduced from 32 to 8, since each channel now has
its own dedicated FIFO and the SDM processes commands one at a time.
8 entries per channel is sufficient while keeping the total aggregate
capacity the same (4 channels x 8 = 32 entries).

Additionally, a thread task is now validated before invoking kthread_stop
when the user aborts, ensuring safe termination.

Timeout values have also been adjusted to accommodate the increased load
from concurrent client driver activity.

Fixes: 7ca5ce896524 ("firmware: add Intel Stratix10 service layer driver")
Cc: stable@vger.kernel.org
Signed-off-by: Ang Tien Sung <tien.sung.ang@altera.com>
Signed-off-by: Fong, Yan Kei <yankei.fong@altera.com>
Signed-off-by: Muhammad Amirul Asyraf Mohamad Jamian <muhammad.amirul.asyraf.mohamad.jamian@altera.com>
Link: https://lore.kernel.org/all/20260305093151.2678-1-muhammad.amirul.asyraf.mohamad.jamian@altera.com
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>

authored by

Muhammad Amirul Asyraf Mohamad Jamian and committed by
Dinh Nguyen 22fd7f7f 6de23f81

+130 -106
+126 -102
drivers/firmware/stratix10-svc.c
··· 37 37 * service layer will return error to FPGA manager when timeout occurs, 38 38 * timeout is set to 30 seconds (30 * 1000) at Intel Stratix10 SoC. 39 39 */ 40 - #define SVC_NUM_DATA_IN_FIFO 32 40 + #define SVC_NUM_DATA_IN_FIFO 8 41 41 #define SVC_NUM_CHANNEL 4 42 - #define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 200 42 + #define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 2000 43 43 #define FPGA_CONFIG_STATUS_TIMEOUT_SEC 30 44 44 #define BYTE_TO_WORD_SIZE 4 45 45 46 46 /* stratix10 service layer clients */ 47 47 #define STRATIX10_RSU "stratix10-rsu" 48 - #define INTEL_FCS "intel-fcs" 49 48 50 49 /* Maximum number of SDM client IDs. */ 51 50 #define MAX_SDM_CLIENT_IDS 16 ··· 104 105 /** 105 106 * struct stratix10_svc - svc private data 106 107 * @stratix10_svc_rsu: pointer to stratix10 RSU device 107 - * @intel_svc_fcs: pointer to the FCS device 108 108 */ 109 109 struct stratix10_svc { 110 110 struct platform_device *stratix10_svc_rsu; 111 - struct platform_device *intel_svc_fcs; 112 111 }; 113 112 114 113 /** ··· 248 251 * @num_active_client: number of active service client 249 252 * @node: list management 250 253 * @genpool: memory pool pointing to the memory region 251 - * @task: pointer to the thread task which handles SMC or HVC call 252 - * @svc_fifo: a queue for storing service message data 253 254 * @complete_status: state for completion 254 - * @svc_fifo_lock: protect access to service message data queue 255 255 * @invoke_fn: function to issue secure monitor call or hypervisor call 256 256 * @svc: manages the list of client svc drivers 257 + * @sdm_lock: only allows a single command single response to SDM 257 258 * @actrl: async control structure 258 259 * 259 260 * This struct is used to create communication channels for service clients, to ··· 264 269 int num_active_client; 265 270 struct list_head node; 266 271 struct gen_pool *genpool; 267 - struct task_struct *task; 268 - struct kfifo svc_fifo; 269 272 struct completion complete_status; 270 - spinlock_t svc_fifo_lock; 271 273 svc_invoke_fn *invoke_fn; 272 274 struct stratix10_svc *svc; 275 + struct mutex sdm_lock; 273 276 struct stratix10_async_ctrl actrl; 274 277 }; 275 278 ··· 276 283 * @ctrl: pointer to service controller which is the provider of this channel 277 284 * @scl: pointer to service client which owns the channel 278 285 * @name: service client name associated with the channel 286 + * @task: pointer to the thread task which handles SMC or HVC call 287 + * @svc_fifo: a queue for storing service message data (separate fifo for every channel) 288 + * @svc_fifo_lock: protect access to service message data queue (locking pending fifo) 279 289 * @lock: protect access to the channel 280 290 * @async_chan: reference to asynchronous channel object for this channel 281 291 * ··· 289 293 struct stratix10_svc_controller *ctrl; 290 294 struct stratix10_svc_client *scl; 291 295 char *name; 296 + struct task_struct *task; 297 + struct kfifo svc_fifo; 298 + spinlock_t svc_fifo_lock; 292 299 spinlock_t lock; 293 300 struct stratix10_async_chan *async_chan; 294 301 }; ··· 526 527 */ 527 528 static int svc_normal_to_secure_thread(void *data) 528 529 { 529 - struct stratix10_svc_controller 530 - *ctrl = (struct stratix10_svc_controller *)data; 531 - struct stratix10_svc_data *pdata; 532 - struct stratix10_svc_cb_data *cbdata; 530 + struct stratix10_svc_chan *chan = (struct stratix10_svc_chan *)data; 531 + struct stratix10_svc_controller *ctrl = chan->ctrl; 532 + struct stratix10_svc_data *pdata = NULL; 533 + struct stratix10_svc_cb_data *cbdata = NULL; 533 534 struct arm_smccc_res res; 534 535 unsigned long a0, a1, a2, a3, a4, a5, a6, a7; 535 536 int ret_fifo = 0; ··· 554 555 a6 = 0; 555 556 a7 = 0; 556 557 557 - pr_debug("smc_hvc_shm_thread is running\n"); 558 + pr_debug("%s: %s: Thread is running!\n", __func__, chan->name); 558 559 559 560 while (!kthread_should_stop()) { 560 - ret_fifo = kfifo_out_spinlocked(&ctrl->svc_fifo, 561 + ret_fifo = kfifo_out_spinlocked(&chan->svc_fifo, 561 562 pdata, sizeof(*pdata), 562 - &ctrl->svc_fifo_lock); 563 + &chan->svc_fifo_lock); 563 564 564 565 if (!ret_fifo) 565 566 continue; ··· 568 569 (unsigned int)pdata->paddr, pdata->command, 569 570 (unsigned int)pdata->size); 570 571 572 + /* SDM can only process one command at a time */ 573 + pr_debug("%s: %s: Thread is waiting for mutex!\n", 574 + __func__, chan->name); 575 + if (mutex_lock_interruptible(&ctrl->sdm_lock)) { 576 + /* item already dequeued; notify client to unblock it */ 577 + cbdata->status = BIT(SVC_STATUS_ERROR); 578 + cbdata->kaddr1 = NULL; 579 + cbdata->kaddr2 = NULL; 580 + cbdata->kaddr3 = NULL; 581 + if (pdata->chan->scl) 582 + pdata->chan->scl->receive_cb(pdata->chan->scl, 583 + cbdata); 584 + break; 585 + } 586 + 571 587 switch (pdata->command) { 572 588 case COMMAND_RECONFIG_DATA_CLAIM: 573 589 svc_thread_cmd_data_claim(ctrl, pdata, cbdata); 590 + mutex_unlock(&ctrl->sdm_lock); 574 591 continue; 575 592 case COMMAND_RECONFIG: 576 593 a0 = INTEL_SIP_SMC_FPGA_CONFIG_START; ··· 715 700 break; 716 701 default: 717 702 pr_warn("it shouldn't happen\n"); 718 - break; 703 + mutex_unlock(&ctrl->sdm_lock); 704 + continue; 719 705 } 720 - pr_debug("%s: before SMC call -- a0=0x%016x a1=0x%016x", 721 - __func__, 706 + pr_debug("%s: %s: before SMC call -- a0=0x%016x a1=0x%016x", 707 + __func__, chan->name, 722 708 (unsigned int)a0, 723 709 (unsigned int)a1); 724 710 pr_debug(" a2=0x%016x\n", (unsigned int)a2); ··· 728 712 pr_debug(" a5=0x%016x\n", (unsigned int)a5); 729 713 ctrl->invoke_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res); 730 714 731 - pr_debug("%s: after SMC call -- res.a0=0x%016x", 732 - __func__, (unsigned int)res.a0); 715 + pr_debug("%s: %s: after SMC call -- res.a0=0x%016x", 716 + __func__, chan->name, (unsigned int)res.a0); 733 717 pr_debug(" res.a1=0x%016x, res.a2=0x%016x", 734 718 (unsigned int)res.a1, (unsigned int)res.a2); 735 719 pr_debug(" res.a3=0x%016x\n", (unsigned int)res.a3); ··· 744 728 cbdata->kaddr2 = NULL; 745 729 cbdata->kaddr3 = NULL; 746 730 pdata->chan->scl->receive_cb(pdata->chan->scl, cbdata); 731 + mutex_unlock(&ctrl->sdm_lock); 747 732 continue; 748 733 } 749 734 ··· 818 801 break; 819 802 820 803 } 804 + 805 + mutex_unlock(&ctrl->sdm_lock); 821 806 } 822 807 823 808 kfree(cbdata); ··· 1715 1696 if (!p_data) 1716 1697 return -ENOMEM; 1717 1698 1718 - /* first client will create kernel thread */ 1719 - if (!chan->ctrl->task) { 1720 - chan->ctrl->task = 1721 - kthread_run_on_cpu(svc_normal_to_secure_thread, 1722 - (void *)chan->ctrl, 1723 - cpu, "svc_smc_hvc_thread"); 1724 - if (IS_ERR(chan->ctrl->task)) { 1699 + /* first caller creates the per-channel kthread */ 1700 + if (!chan->task) { 1701 + struct task_struct *task; 1702 + 1703 + task = kthread_run_on_cpu(svc_normal_to_secure_thread, 1704 + (void *)chan, 1705 + cpu, "svc_smc_hvc_thread"); 1706 + if (IS_ERR(task)) { 1725 1707 dev_err(chan->ctrl->dev, 1726 1708 "failed to create svc_smc_hvc_thread\n"); 1727 1709 kfree(p_data); 1728 1710 return -EINVAL; 1729 1711 } 1712 + 1713 + spin_lock(&chan->lock); 1714 + if (chan->task) { 1715 + /* another caller won the race; discard our thread */ 1716 + spin_unlock(&chan->lock); 1717 + kthread_stop(task); 1718 + } else { 1719 + chan->task = task; 1720 + spin_unlock(&chan->lock); 1721 + } 1730 1722 } 1731 1723 1732 - pr_debug("%s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__, 1733 - p_msg->payload, p_msg->command, 1724 + pr_debug("%s: %s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__, 1725 + chan->name, p_msg->payload, p_msg->command, 1734 1726 (unsigned int)p_msg->payload_length); 1735 1727 1736 1728 if (list_empty(&svc_data_mem)) { ··· 1777 1747 p_data->arg[2] = p_msg->arg[2]; 1778 1748 p_data->size = p_msg->payload_length; 1779 1749 p_data->chan = chan; 1780 - pr_debug("%s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", __func__, 1781 - (unsigned int)p_data->paddr, p_data->command, 1782 - (unsigned int)p_data->size); 1783 - ret = kfifo_in_spinlocked(&chan->ctrl->svc_fifo, p_data, 1750 + pr_debug("%s: %s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", 1751 + __func__, 1752 + chan->name, 1753 + (unsigned int)p_data->paddr, 1754 + p_data->command, 1755 + (unsigned int)p_data->size); 1756 + 1757 + ret = kfifo_in_spinlocked(&chan->svc_fifo, p_data, 1784 1758 sizeof(*p_data), 1785 - &chan->ctrl->svc_fifo_lock); 1759 + &chan->svc_fifo_lock); 1786 1760 1787 1761 kfree(p_data); 1788 1762 ··· 1807 1773 */ 1808 1774 void stratix10_svc_done(struct stratix10_svc_chan *chan) 1809 1775 { 1810 - /* stop thread when thread is running AND only one active client */ 1811 - if (chan->ctrl->task && chan->ctrl->num_active_client <= 1) { 1812 - pr_debug("svc_smc_hvc_shm_thread is stopped\n"); 1813 - kthread_stop(chan->ctrl->task); 1814 - chan->ctrl->task = NULL; 1776 + /* stop thread when thread is running */ 1777 + if (chan->task) { 1778 + pr_debug("%s: %s: svc_smc_hvc_shm_thread is stopping\n", 1779 + __func__, chan->name); 1780 + kthread_stop(chan->task); 1781 + chan->task = NULL; 1815 1782 } 1816 1783 } 1817 1784 EXPORT_SYMBOL_GPL(stratix10_svc_done); ··· 1852 1817 pmem->paddr = pa; 1853 1818 pmem->size = s; 1854 1819 list_add_tail(&pmem->node, &svc_data_mem); 1855 - pr_debug("%s: va=%p, pa=0x%016x\n", __func__, 1856 - pmem->vaddr, (unsigned int)pmem->paddr); 1820 + pr_debug("%s: %s: va=%p, pa=0x%016x\n", __func__, 1821 + chan->name, pmem->vaddr, (unsigned int)pmem->paddr); 1857 1822 1858 1823 return (void *)va; 1859 1824 } ··· 1890 1855 {}, 1891 1856 }; 1892 1857 1858 + static const char * const chan_names[SVC_NUM_CHANNEL] = { 1859 + SVC_CLIENT_FPGA, 1860 + SVC_CLIENT_RSU, 1861 + SVC_CLIENT_FCS, 1862 + SVC_CLIENT_HWMON 1863 + }; 1864 + 1893 1865 static int stratix10_svc_drv_probe(struct platform_device *pdev) 1894 1866 { 1895 1867 struct device *dev = &pdev->dev; ··· 1904 1862 struct stratix10_svc_chan *chans; 1905 1863 struct gen_pool *genpool; 1906 1864 struct stratix10_svc_sh_memory *sh_memory; 1907 - struct stratix10_svc *svc; 1865 + struct stratix10_svc *svc = NULL; 1908 1866 1909 1867 svc_invoke_fn *invoke_fn; 1910 1868 size_t fifo_size; 1911 - int ret; 1869 + int ret, i = 0; 1912 1870 1913 1871 /* get SMC or HVC function */ 1914 1872 invoke_fn = get_invoke_func(dev); ··· 1947 1905 controller->num_active_client = 0; 1948 1906 controller->chans = chans; 1949 1907 controller->genpool = genpool; 1950 - controller->task = NULL; 1951 1908 controller->invoke_fn = invoke_fn; 1909 + INIT_LIST_HEAD(&controller->node); 1952 1910 init_completion(&controller->complete_status); 1953 1911 1954 1912 ret = stratix10_svc_async_init(controller); ··· 1959 1917 } 1960 1918 1961 1919 fifo_size = sizeof(struct stratix10_svc_data) * SVC_NUM_DATA_IN_FIFO; 1962 - ret = kfifo_alloc(&controller->svc_fifo, fifo_size, GFP_KERNEL); 1963 - if (ret) { 1964 - dev_err(dev, "failed to allocate FIFO\n"); 1965 - goto err_async_exit; 1920 + mutex_init(&controller->sdm_lock); 1921 + 1922 + for (i = 0; i < SVC_NUM_CHANNEL; i++) { 1923 + chans[i].scl = NULL; 1924 + chans[i].ctrl = controller; 1925 + chans[i].name = (char *)chan_names[i]; 1926 + spin_lock_init(&chans[i].lock); 1927 + ret = kfifo_alloc(&chans[i].svc_fifo, fifo_size, GFP_KERNEL); 1928 + if (ret) { 1929 + dev_err(dev, "failed to allocate FIFO %d\n", i); 1930 + goto err_free_fifos; 1931 + } 1932 + spin_lock_init(&chans[i].svc_fifo_lock); 1966 1933 } 1967 - spin_lock_init(&controller->svc_fifo_lock); 1968 - 1969 - chans[0].scl = NULL; 1970 - chans[0].ctrl = controller; 1971 - chans[0].name = SVC_CLIENT_FPGA; 1972 - spin_lock_init(&chans[0].lock); 1973 - 1974 - chans[1].scl = NULL; 1975 - chans[1].ctrl = controller; 1976 - chans[1].name = SVC_CLIENT_RSU; 1977 - spin_lock_init(&chans[1].lock); 1978 - 1979 - chans[2].scl = NULL; 1980 - chans[2].ctrl = controller; 1981 - chans[2].name = SVC_CLIENT_FCS; 1982 - spin_lock_init(&chans[2].lock); 1983 - 1984 - chans[3].scl = NULL; 1985 - chans[3].ctrl = controller; 1986 - chans[3].name = SVC_CLIENT_HWMON; 1987 - spin_lock_init(&chans[3].lock); 1988 1934 1989 1935 list_add_tail(&controller->node, &svc_ctrl); 1990 1936 platform_set_drvdata(pdev, controller); ··· 1981 1951 svc = devm_kzalloc(dev, sizeof(*svc), GFP_KERNEL); 1982 1952 if (!svc) { 1983 1953 ret = -ENOMEM; 1984 - goto err_free_kfifo; 1954 + goto err_free_fifos; 1985 1955 } 1986 1956 controller->svc = svc; 1987 1957 ··· 1989 1959 if (!svc->stratix10_svc_rsu) { 1990 1960 dev_err(dev, "failed to allocate %s device\n", STRATIX10_RSU); 1991 1961 ret = -ENOMEM; 1992 - goto err_free_kfifo; 1962 + goto err_free_fifos; 1993 1963 } 1994 1964 1995 1965 ret = platform_device_add(svc->stratix10_svc_rsu); 1996 - if (ret) { 1997 - platform_device_put(svc->stratix10_svc_rsu); 1998 - goto err_free_kfifo; 1999 - } 2000 - 2001 - svc->intel_svc_fcs = platform_device_alloc(INTEL_FCS, 1); 2002 - if (!svc->intel_svc_fcs) { 2003 - dev_err(dev, "failed to allocate %s device\n", INTEL_FCS); 2004 - ret = -ENOMEM; 2005 - goto err_unregister_rsu_dev; 2006 - } 2007 - 2008 - ret = platform_device_add(svc->intel_svc_fcs); 2009 - if (ret) { 2010 - platform_device_put(svc->intel_svc_fcs); 2011 - goto err_unregister_rsu_dev; 2012 - } 1966 + if (ret) 1967 + goto err_put_device; 2013 1968 2014 1969 ret = of_platform_default_populate(dev_of_node(dev), NULL, dev); 2015 1970 if (ret) 2016 - goto err_unregister_fcs_dev; 1971 + goto err_unregister_rsu_dev; 2017 1972 2018 1973 pr_info("Intel Service Layer Driver Initialized\n"); 2019 1974 2020 1975 return 0; 2021 1976 2022 - err_unregister_fcs_dev: 2023 - platform_device_unregister(svc->intel_svc_fcs); 2024 1977 err_unregister_rsu_dev: 2025 1978 platform_device_unregister(svc->stratix10_svc_rsu); 2026 - err_free_kfifo: 2027 - kfifo_free(&controller->svc_fifo); 2028 - err_async_exit: 1979 + goto err_free_fifos; 1980 + err_put_device: 1981 + platform_device_put(svc->stratix10_svc_rsu); 1982 + err_free_fifos: 1983 + /* only remove from list if list_add_tail() was reached */ 1984 + if (!list_empty(&controller->node)) 1985 + list_del(&controller->node); 1986 + /* free only the FIFOs that were successfully allocated */ 1987 + while (i--) 1988 + kfifo_free(&chans[i].svc_fifo); 2029 1989 stratix10_svc_async_exit(controller); 2030 1990 err_destroy_pool: 2031 1991 gen_pool_destroy(genpool); 1992 + 2032 1993 return ret; 2033 1994 } 2034 1995 2035 1996 static void stratix10_svc_drv_remove(struct platform_device *pdev) 2036 1997 { 1998 + int i; 2037 1999 struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev); 2038 2000 struct stratix10_svc *svc = ctrl->svc; 2039 2001 ··· 2033 2011 2034 2012 of_platform_depopulate(ctrl->dev); 2035 2013 2036 - platform_device_unregister(svc->intel_svc_fcs); 2037 2014 platform_device_unregister(svc->stratix10_svc_rsu); 2038 2015 2039 - kfifo_free(&ctrl->svc_fifo); 2040 - if (ctrl->task) { 2041 - kthread_stop(ctrl->task); 2042 - ctrl->task = NULL; 2016 + for (i = 0; i < SVC_NUM_CHANNEL; i++) { 2017 + if (ctrl->chans[i].task) { 2018 + kthread_stop(ctrl->chans[i].task); 2019 + ctrl->chans[i].task = NULL; 2020 + } 2021 + kfifo_free(&ctrl->chans[i].svc_fifo); 2043 2022 } 2023 + 2044 2024 if (ctrl->genpool) 2045 2025 gen_pool_destroy(ctrl->genpool); 2046 2026 list_del(&ctrl->node);
+4 -4
include/linux/firmware/intel/stratix10-svc-client.h
··· 68 68 * timeout value used in Stratix10 FPGA manager driver. 69 69 * timeout value used in RSU driver 70 70 */ 71 - #define SVC_RECONFIG_REQUEST_TIMEOUT_MS 300 72 - #define SVC_RECONFIG_BUFFER_TIMEOUT_MS 720 73 - #define SVC_RSU_REQUEST_TIMEOUT_MS 300 71 + #define SVC_RECONFIG_REQUEST_TIMEOUT_MS 5000 72 + #define SVC_RECONFIG_BUFFER_TIMEOUT_MS 5000 73 + #define SVC_RSU_REQUEST_TIMEOUT_MS 2000 74 74 #define SVC_FCS_REQUEST_TIMEOUT_MS 2000 75 75 #define SVC_COMPLETED_TIMEOUT_MS 30000 76 - #define SVC_HWMON_REQUEST_TIMEOUT_MS 300 76 + #define SVC_HWMON_REQUEST_TIMEOUT_MS 2000 77 77 78 78 struct stratix10_svc_chan; 79 79