lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2bdbba78c1e3162bd7e385ed48715fd949133253.camel@intel.com>
Date:   Wed, 21 Jun 2023 14:50:19 +0000
From:   "Zhang, Rui" <rui.zhang@...el.com>
To:     "srinivas.pandruvada@...ux.intel.com" 
        <srinivas.pandruvada@...ux.intel.com>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>
CC:     "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/7] thermal: int340x: processor_thermal: Add interrupt
 configuration

On Tue, 2023-06-20 at 16:01 -0700, Srinivas Pandruvada wrote:
> Some features on this PCI devices require interrupt support. Here
> interrupts are enabled/disabled via sending mailbox commands. The
> mailbox command ID is 0x1E for read and 0x1F for write.
> 
> The interrupt configuration will require mutex protection as it
> involved read-modify-write operation. Since mutex are already used
> in the mailbox read/write functions: send_mbox_write_cmd() and
> send_mbox_read_cmd(), there will be double locking. But, this can
> be avoided by moving mutexes from mailbox read/write processing
> functions to the calling (exported) functions.
> 
> Signed-off-by: Srinivas Pandruvada
> <srinivas.pandruvada@...ux.intel.com>
> ---
>  .../processor_thermal_device.h                |  2 +
>  .../int340x_thermal/processor_thermal_mbox.c  | 85 ++++++++++++++---
> --
>  2 files changed, 68 insertions(+), 19 deletions(-)
> 
> diff --git
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.h
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.h
> index 7cdeca2edc21..defc919cb020 100644
> ---
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.h
> +++
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.h
> @@ -91,6 +91,8 @@ void proc_thermal_wlt_req_remove(struct pci_dev
> *pdev);
>  
>  int processor_thermal_send_mbox_read_cmd(struct pci_dev *pdev, u16
> id, u64 *resp);
>  int processor_thermal_send_mbox_write_cmd(struct pci_dev *pdev, u16
> id, u32 data);
> +int processor_thermal_mbox_interrupt_config(struct pci_dev *pdev,
> bool enable, int enable_bit,
> +                                           int time_window);
>  int proc_thermal_add(struct device *dev, struct proc_thermal_device
> *priv);
>  void proc_thermal_remove(struct proc_thermal_device *proc_priv);
>  int proc_thermal_suspend(struct device *dev);
> diff --git
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_mbox.c
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_mbox.c
> index ec766c5615b7..7ef0af3f5bef 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_mbox.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_mbox.c
> @@ -45,23 +45,16 @@ static int send_mbox_write_cmd(struct pci_dev
> *pdev, u16 id, u32 data)
>         int ret;
>  
>         proc_priv = pci_get_drvdata(pdev);
> -
> -       mutex_lock(&mbox_lock);
> -
>         ret = wait_for_mbox_ready(proc_priv);
>         if (ret)
> -               goto unlock_mbox;
> +               return ret;
>  
>         writel(data, (proc_priv->mmio_base + MBOX_OFFSET_DATA));
>         /* Write command register */
>         reg_data = BIT_ULL(MBOX_BUSY_BIT) | id;
>         writel(reg_data, (proc_priv->mmio_base +
> MBOX_OFFSET_INTERFACE));
>  
> -       ret = wait_for_mbox_ready(proc_priv);
> -
> -unlock_mbox:
> -       mutex_unlock(&mbox_lock);
> -       return ret;
> +       return wait_for_mbox_ready(proc_priv);
>  }
>  
>  static int send_mbox_read_cmd(struct pci_dev *pdev, u16 id, u64
> *resp)
> @@ -71,12 +64,9 @@ static int send_mbox_read_cmd(struct pci_dev
> *pdev, u16 id, u64 *resp)
>         int ret;
>  
>         proc_priv = pci_get_drvdata(pdev);
> -
> -       mutex_lock(&mbox_lock);
> -
>         ret = wait_for_mbox_ready(proc_priv);
>         if (ret)
> -               goto unlock_mbox;
> +               return ret;
>  
>         /* Write command register */
>         reg_data = BIT_ULL(MBOX_BUSY_BIT) | id;
> @@ -84,28 +74,85 @@ static int send_mbox_read_cmd(struct pci_dev
> *pdev, u16 id, u64 *resp)
>  
>         ret = wait_for_mbox_ready(proc_priv);
>         if (ret)
> -               goto unlock_mbox;
> +               return ret;
>  
>         if (id == MBOX_CMD_WORKLOAD_TYPE_READ)
>                 *resp = readl(proc_priv->mmio_base +
> MBOX_OFFSET_DATA);
>         else
>                 *resp = readq(proc_priv->mmio_base +
> MBOX_OFFSET_DATA);
>  
> -unlock_mbox:
> -       mutex_unlock(&mbox_lock);
> -       return ret;
> +       return 0;
>  }
>  
>  int processor_thermal_send_mbox_read_cmd(struct pci_dev *pdev, u16
> id, u64 *resp)
>  {
> -       return send_mbox_read_cmd(pdev, id, resp);
> +       int ret;
> +
> +       mutex_lock(&mbox_lock);
> +       ret = send_mbox_read_cmd(pdev, id, resp);
> +       mutex_unlock(&mbox_lock);
> +
> +       return ret;
>  }
>  EXPORT_SYMBOL_NS_GPL(processor_thermal_send_mbox_read_cmd,
> INT340X_THERMAL);
>  
>  int processor_thermal_send_mbox_write_cmd(struct pci_dev *pdev, u16
> id, u32 data)
>  {
> -       return send_mbox_write_cmd(pdev, id, data);
> +       int ret;
> +
> +       mutex_lock(&mbox_lock);
> +       ret = send_mbox_write_cmd(pdev, id, data);
> +       mutex_unlock(&mbox_lock);
> +
> +       return ret;
>  }
>  EXPORT_SYMBOL_NS_GPL(processor_thermal_send_mbox_write_cmd,
> INT340X_THERMAL);
>  
> +#define MBOX_CAMARILLO_RD_INTR_CONFIG  0x1E
> +#define MBOX_CAMARILLO_WR_INTR_CONFIG  0x1F
> +#define WLT_TW_MASK                    GENMASK_ULL(30, 24)
> +#define SOC_PREDICTION_TW_SHIFT                24
> +
> +int processor_thermal_mbox_interrupt_config(struct pci_dev *pdev,
> bool enable,
> +                                           int enable_bit, int
> time_window)

All the callers of this API in this patch series uses
SOC_WLT_PREDICTION_INT_ENABLE_BIT as the enable_bit, so this parameter
is redundant?
or do we expect a different enable_bit on other/future platforms?

thanks,
rui

> +{
> +       u64 data;
> +       int ret;
> +
> +       if (!pdev)
> +               return -ENODEV;
> +
> +       mutex_lock(&mbox_lock);
> +
> +       /* Do read modify write for MBOX_CAMARILLO_RD_INTR_CONFIG */
> +
> +       ret = send_mbox_read_cmd(pdev,
> MBOX_CAMARILLO_RD_INTR_CONFIG,  &data);
> +       if (ret) {
> +               dev_err(&pdev->dev, "MBOX_CAMARILLO_RD_INTR_CONFIG
> failed\n");
> +               goto unlock;
> +       }
> +
> +       if (time_window >= 0) {
> +               data &= ~WLT_TW_MASK;
> +
> +               /* Program notification delay */
> +               data |= (time_window << SOC_PREDICTION_TW_SHIFT);
> +       }
> +
> +       if (enable)
> +               data |= BIT(enable_bit);
> +       else
> +               data &= ~BIT(enable_bit);
> +
> +       ret = send_mbox_write_cmd(pdev,
> MBOX_CAMARILLO_WR_INTR_CONFIG, data);
> +       if (ret)
> +               dev_err(&pdev->dev, "MBOX_CAMARILLO_WR_INTR_CONFIG
> failed\n");
> +
> +unlock:
> +       mutex_unlock(&mbox_lock);
> +
> +       return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(processor_thermal_mbox_interrupt_config,
> INT340X_THERMAL);
> +
>  MODULE_LICENSE("GPL v2");

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ