On Wed, Sep 09, 2020 at 05:20:02PM -0700, Amit Sunil Dhamne wrote:
> From: Tejas Patel <[email protected]>
>
> Validate IOCTL ID for ZynqMP and Versal before calling
> zynqmp_pm_invoke_fn().
>
> Signed-off-by: Tejas Patel <[email protected]>
> Signed-off-by: Amit Sunil Dhamne <[email protected]>
> ---
> drivers/firmware/xilinx/zynqmp.c | 117
> +++++++++++++++++++++++++++++++--------
> 1 file changed, 95 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/firmware/xilinx/zynqmp.c
> b/drivers/firmware/xilinx/zynqmp.c
> index 8d1ff24..8fe0912 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -514,6 +514,89 @@ int zynqmp_pm_clock_getparent(u32 clock_id, u32
> *parent_id)
> EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent);
>
> /**
> + * versal_is_valid_ioctl() - Check whether IOCTL ID is valid or not for
> versal
> + * @ioctl_id: IOCTL ID
> + *
> + * Return: 1 if IOCTL is valid else 0
> + */
> +static inline int versal_is_valid_ioctl(u32 ioctl_id)
> +{
> + switch (ioctl_id) {
> + case IOCTL_SD_DLL_RESET:
> + case IOCTL_SET_SD_TAPDELAY:
> + case IOCTL_SET_PLL_FRAC_MODE:
> + case IOCTL_GET_PLL_FRAC_MODE:
> + case IOCTL_SET_PLL_FRAC_DATA:
> + case IOCTL_GET_PLL_FRAC_DATA:
> + case IOCTL_WRITE_GGS:
> + case IOCTL_READ_GGS:
> + case IOCTL_WRITE_PGGS:
> + case IOCTL_READ_PGGS:
> + case IOCTL_SET_BOOT_HEALTH_STATUS:
> + return 1;
> + default:
> + return 0;
bool is nicer, right?
> + }
> +}
> +
> +/**
> + * zynqmp_is_valid_ioctl() - Check whether IOCTL ID is valid or not
> + * @ioctl_id: IOCTL ID
> + *
> + * Return: 1 if IOCTL is valid else 0
> + */
> +static inline int zynqmp_is_valid_ioctl(u32 ioctl_id)
> +{
> + switch (ioctl_id) {
> + case IOCTL_SD_DLL_RESET:
> + case IOCTL_SET_SD_TAPDELAY:
> + case IOCTL_SET_PLL_FRAC_MODE:
> + case IOCTL_GET_PLL_FRAC_MODE:
> + case IOCTL_SET_PLL_FRAC_DATA:
> + case IOCTL_GET_PLL_FRAC_DATA:
> + case IOCTL_WRITE_GGS:
> + case IOCTL_READ_GGS:
> + case IOCTL_WRITE_PGGS:
> + case IOCTL_READ_PGGS:
> + case IOCTL_SET_BOOT_HEALTH_STATUS:
> + return 1;
> + default:
> + return 0;
> + }
> +}
> +
> +/**
> + * zynqmp_pm_ioctl() - PM IOCTL API for device control and configs
> + * @node_id: Node ID of the device
> + * @ioctl_id: ID of the requested IOCTL
> + * @arg1: Argument 1 to requested IOCTL call
> + * @arg2: Argument 2 to requested IOCTL call
> + * @out: Returned output value
> + *
> + * This function calls IOCTL to firmware for device control and
> configuration.
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +static int zynqmp_pm_ioctl(u32 node_id, u32 ioctl_id, u32 arg1, u32 arg2,
> + u32 *out)
> +{
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "xlnx,versal");
> + if (np) {
> + if (!versal_is_valid_ioctl(ioctl_id))
> + return -EINVAL;
Wrong error value.
> + } else {
> + if (!zynqmp_is_valid_ioctl(ioctl_id))
> + return -EINVAL;
Wrong error value.
> + }
> + of_node_put(np);
> +
> + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, ioctl_id, arg1, arg2,
> + out);
No other checking of ioctl commands and arguments? Brave...
> +}
> +
> +/**
> * zynqmp_pm_set_pll_frac_mode() - PM API for set PLL mode
> *
> * @clk_id: PLL clock ID
> @@ -525,8 +608,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent);
> */
> int zynqmp_pm_set_pll_frac_mode(u32 clk_id, u32 mode)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_PLL_FRAC_MODE,
> - clk_id, mode, NULL);
> + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_MODE, clk_id, mode,
> NULL);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode);
>
> @@ -542,8 +624,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode);
> */
> int zynqmp_pm_get_pll_frac_mode(u32 clk_id, u32 *mode)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_PLL_FRAC_MODE,
> - clk_id, 0, mode);
> + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_MODE, clk_id, 0, mode);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode);
>
> @@ -560,8 +641,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode);
> */
> int zynqmp_pm_set_pll_frac_data(u32 clk_id, u32 data)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_PLL_FRAC_DATA,
> - clk_id, data, NULL);
> + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_DATA, clk_id, data,
> NULL);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data);
>
> @@ -577,8 +657,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data);
> */
> int zynqmp_pm_get_pll_frac_data(u32 clk_id, u32 *data)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_PLL_FRAC_DATA,
> - clk_id, 0, data);
> + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_DATA, clk_id, 0, data);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data);
>
> @@ -595,8 +674,8 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data);
> */
> int zynqmp_pm_set_sd_tapdelay(u32 node_id, u32 type, u32 value)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, IOCTL_SET_SD_TAPDELAY,
> - type, value, NULL);
> + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, value,
> + NULL);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay);
>
> @@ -612,8 +691,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay);
> */
> int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, IOCTL_SET_SD_TAPDELAY,
> - type, 0, NULL);
> + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, 0, NULL);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset);
>
> @@ -628,8 +706,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset);
> */
> int zynqmp_pm_write_ggs(u32 index, u32 value)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_GGS,
> - index, value, NULL);
> + return zynqmp_pm_ioctl(0, IOCTL_WRITE_GGS, index, value, NULL);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs);
>
> @@ -644,8 +721,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs);
> */
> int zynqmp_pm_read_ggs(u32 index, u32 *value)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_GGS,
> - index, 0, value);
> + return zynqmp_pm_ioctl(0, IOCTL_READ_GGS, index, 0, value);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs);
>
> @@ -661,8 +737,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs);
> */
> int zynqmp_pm_write_pggs(u32 index, u32 value)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_PGGS, index,
> value,
> - NULL);
> + return zynqmp_pm_ioctl(0, IOCTL_WRITE_PGGS, index, value, NULL);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs);
>
> @@ -678,8 +753,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs);
> */
> int zynqmp_pm_read_pggs(u32 index, u32 *value)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_PGGS, index, 0,
> - value);
> + return zynqmp_pm_ioctl(0, IOCTL_READ_PGGS, index, 0, value);
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs);
>
> @@ -694,8 +768,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs);
> */
> int zynqmp_pm_set_boot_health_status(u32 value)
> {
> - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_BOOT_HEALTH_STATUS,
> - value, 0, NULL);
> + return zynqmp_pm_ioctl(0, IOCTL_SET_BOOT_HEALTH_STATUS, value, 0,
> NULL);
> }
>
> /**
> --
> 2.7.4
>
> This email and any attachments are intended for the sole use of the named
> recipient(s) and contain(s) confidential information that may be proprietary,
> privileged or copyrighted under applicable law. If you are not the intended
> recipient, do not read, copy, or forward this email message or any
> attachments. Delete this email message and any attachments immediately.
Oops, this means I have to drop this, it's not compatible with kernel
development, sorry.
greg k-h
