| 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
| ||
|
Message-ID: <CH3PR11MB841473286D5B95D43AA96A66E382A@CH3PR11MB8414.namprd11.prod.outlook.com>
Date: Thu, 30 Nov 2023 17:22:20 +0000
From: "Michalik, Michal" <michal.michalik@...el.com>
To: Simon Horman <horms@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"vadim.fedorenko@...ux.dev" <vadim.fedorenko@...ux.dev>, "Kubalewski,
Arkadiusz" <arkadiusz.kubalewski@...el.com>, "jonathan.lemon@...il.com"
<jonathan.lemon@...il.com>, "pabeni@...hat.com" <pabeni@...hat.com>, poros
<poros@...hat.com>, "Olech, Milena" <milena.olech@...el.com>, mschmidt
<mschmidt@...hat.com>, "linux-clk@...r.kernel.org"
<linux-clk@...r.kernel.org>, "bvanassche@....org" <bvanassche@....org>,
"kuba@...nel.org" <kuba@...nel.org>, "davem@...emloft.net"
<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>
Subject: RE: [PATCH RFC net-next v4 1/2] netdevsim: implement DPLL for
subsystem selftests
On 23 November 2023 3:41 PM CET, Simon Horman wrote:
>
> On Thu, Nov 23, 2023 at 05:52:42AM -0500, Michal Michalik wrote:
>> DPLL subsystem integration tests require a module which mimics the
>> behavior of real driver which supports DPLL hardware. To fully test the
>> subsystem the netdevsim is amended with DPLL implementation.
>>
>> Signed-off-by: Michal Michalik <michal.michalik@...el.com>
>
> ...
>
>> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>> index b4d3b9cde8bd..76da4e8aa9af 100644
>> --- a/drivers/net/netdevsim/dev.c
>> +++ b/drivers/net/netdevsim/dev.c
>> @@ -342,6 +342,17 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
>> debugfs_create_file("max_vfs", 0600, nsim_dev->ddir,
>> nsim_dev, &nsim_dev_max_vfs_fops);
>>
>> + debugfs_create_u64("dpll_clock_id", 0600,
>> + nsim_dev->ddir, &nsim_dev->dpll.dpll_e_pd.clock_id);
>> + debugfs_create_u32("dpll_e_status", 0600, nsim_dev->ddir,
>> + &nsim_dev->dpll.dpll_e_pd.status);
>> + debugfs_create_u32("dpll_p_status", 0600, nsim_dev->ddir,
>> + &nsim_dev->dpll.dpll_p_pd.status);
>> + debugfs_create_u32("dpll_e_temp", 0600, nsim_dev->ddir,
>> + &nsim_dev->dpll.dpll_e_pd.temperature);
>> + debugfs_create_u32("dpll_p_temp", 0600, nsim_dev->ddir,
>> + &nsim_dev->dpll.dpll_p_pd.temperature);
>> +
>> nsim_dev->nodes_ddir = debugfs_create_dir("rate_nodes", nsim_dev->ddir);
>> if (IS_ERR(nsim_dev->nodes_ddir)) {
>> err = PTR_ERR(nsim_dev->nodes_ddir);
>> @@ -1601,14 +1612,21 @@ int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
>> if (err)
>> goto err_psample_exit;
>>
>> - err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
>> + err = nsim_dpll_init_owner(&nsim_dev->dpll, nsim_bus_dev->port_count);
>> if (err)
>> goto err_hwstats_exit;
>>
>> + err = nsim_dev_port_add_all(nsim_dev, nsim_bus_dev->port_count);
>> + if (err)
>> + goto err_teardown_dpll;
>> +
>> nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_LEGACY;
>> devl_unlock(devlink);
>> +
>> return 0;
>>
>> +err_teardown_dpll:
>> + nsim_dpll_free_owner(&nsim_dev->dpll);
>> err_hwstats_exit:
>> nsim_dev_hwstats_exit(nsim_dev);
>> err_psample_exit:
>> @@ -1656,6 +1674,7 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
>> }
>>
>> nsim_dev_port_del_all(nsim_dev);
>> + nsim_dpll_free_owner(&nsim_dev->dpll);
>> nsim_dev_hwstats_exit(nsim_dev);
>> nsim_dev_psample_exit(nsim_dev);
>> nsim_dev_health_exit(nsim_dev);
>> diff --git a/drivers/net/netdevsim/dpll.c b/drivers/net/netdevsim/dpll.c
>> new file mode 100644
>> index 000000000000..26a8b0f3be16
>> --- /dev/null
>> +++ b/drivers/net/netdevsim/dpll.c
>> @@ -0,0 +1,489 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2023, Intel Corporation.
>> + * Author: Michal Michalik <michal.michalik@...el.com>
>> + */
>> +#include "netdevsim.h"
>> +
>> +#define EEC_DPLL_DEV 0
>> +#define EEC_DPLL_TEMPERATURE 20
>> +#define PPS_DPLL_DEV 1
>> +#define PPS_DPLL_TEMPERATURE 30
>> +
>> +#define PIN_GNSS 0
>> +#define PIN_GNSS_CAPABILITIES DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE
>> +#define PIN_GNSS_PRIORITY 5
>> +
>> +#define PIN_PPS 1
>> +#define PIN_PPS_CAPABILITIES \
>> + (DPLL_PIN_CAPABILITIES_DIRECTION_CAN_CHANGE | \
>> + DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE | \
>> + DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE)
>> +#define PIN_PPS_PRIORITY 6
>> +
>> +#define PIN_RCLK 2
>> +#define PIN_RCLK_CAPABILITIES \
>> + (DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE | \
>> + DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE)
>> +#define PIN_RCLK_PRIORITY 7
>> +
>> +#define EEC_PINS_NUMBER 3
>> +#define PPS_PINS_NUMBER 2
>> +
>> +static int nsim_fill_pin_properties(struct dpll_pin_properties *pp,
>> + const char *label, enum dpll_pin_type type,
>> + unsigned long caps, u32 freq_supp_num,
>> + u64 fmin, u64 fmax)
>> +{
>> + struct dpll_pin_frequency *freq_supp;
>> +
>> + freq_supp = kzalloc(sizeof(*freq_supp), GFP_KERNEL);
>> + if (!freq_supp)
>> + goto freq_supp;
>> + freq_supp->min = fmin;
>> + freq_supp->max = fmax;
>> +
>> + pp->board_label = kasprintf(GFP_KERNEL, "%s_brd", label);
>> + if (!pp->board_label)
>> + goto board_label;
>> + pp->panel_label = kasprintf(GFP_KERNEL, "%s_pnl", label);
>> + if (!pp->panel_label)
>> + goto panel_label;
>> + pp->package_label = kasprintf(GFP_KERNEL, "%s_pcg", label);
>> + if (!pp->package_label)
>> + goto package_label;
>> + pp->freq_supported_num = freq_supp_num;
>> + pp->freq_supported = freq_supp;
>> + pp->capabilities = caps;
>> + pp->type = type;
>> +
>> + return 0;
>> +
>> +package_label:
>> + kfree(pp->panel_label);
>> +panel_label:
>> + kfree(pp->board_label);
>> +board_label:
>> + kfree(freq_supp);
>> +freq_supp:
>> + return -ENOMEM;
>> +}
>> +
>> +static void nsim_fill_pin_pd(struct nsim_pin_priv_data *pd, u64 frequency,
>> + u32 prio, enum dpll_pin_direction direction)
>> +{
>> + pd->state_dpll = DPLL_PIN_STATE_DISCONNECTED;
>> + pd->state_pin = DPLL_PIN_STATE_DISCONNECTED;
>> + pd->frequency = frequency;
>> + pd->direction = direction;
>> + pd->prio = prio;
>> +}
>> +
>> +static int nsim_dds_ops_mode_get(const struct dpll_device *dpll,
>> + void *dpll_priv, enum dpll_mode *mode,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_dpll_priv_data *pd = dpll_priv;
>> + *mode = pd->mode;
>> + return 0;
>> +};
>> +
>> +static bool nsim_dds_ops_mode_supported(const struct dpll_device *dpll,
>> + void *dpll_priv,
>> + const enum dpll_mode mode,
>> + struct netlink_ext_ack *extack)
>> +{
>> + return true;
>> +};
>> +
>> +static int nsim_dds_ops_lock_status_get(const struct dpll_device *dpll,
>> + void *dpll_priv,
>> + enum dpll_lock_status *status,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_dpll_priv_data *pd = dpll_priv;
>> +
>> + *status = pd->status;
>> + return 0;
>> +};
>> +
>> +static int nsim_dds_ops_temp_get(const struct dpll_device *dpll,
>> + void *dpll_priv, s32 *temp,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_dpll_priv_data *pd = dpll_priv;
>> +
>> + *temp = pd->temperature;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_frequency_set(const struct dpll_pin *pin, void *pin_priv,
>> + const struct dpll_device *dpll,
>> + void *dpll_priv, const u64 frequency,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + pd->frequency = frequency;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_frequency_get(const struct dpll_pin *pin, void *pin_priv,
>> + const struct dpll_device *dpll,
>> + void *dpll_priv, u64 *frequency,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + *frequency = pd->frequency;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_direction_set(const struct dpll_pin *pin, void *pin_priv,
>> + const struct dpll_device *dpll,
>> + void *dpll_priv,
>> + const enum dpll_pin_direction direction,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + pd->direction = direction;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_direction_get(const struct dpll_pin *pin, void *pin_priv,
>> + const struct dpll_device *dpll,
>> + void *dpll_priv,
>> + enum dpll_pin_direction *direction,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + *direction = pd->direction;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv,
>> + const struct dpll_pin *parent_pin,
>> + void *parent_priv,
>> + enum dpll_pin_state *state,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + *state = pd->state_pin;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_state_on_dpll_get(const struct dpll_pin *pin,
>> + void *pin_priv,
>> + const struct dpll_device *dpll,
>> + void *dpll_priv,
>> + enum dpll_pin_state *state,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + *state = pd->state_dpll;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_state_on_pin_set(const struct dpll_pin *pin, void *pin_priv,
>> + const struct dpll_pin *parent_pin,
>> + void *parent_priv,
>> + const enum dpll_pin_state state,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + pd->state_pin = state;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_state_on_dpll_set(const struct dpll_pin *pin,
>> + void *pin_priv,
>> + const struct dpll_device *dpll,
>> + void *dpll_priv,
>> + const enum dpll_pin_state state,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + pd->state_dpll = state;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_prio_get(const struct dpll_pin *pin, void *pin_priv,
>> + const struct dpll_device *dpll, void *dpll_priv,
>> + u32 *prio, struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + *prio = pd->prio;
>> + return 0;
>> +};
>> +
>> +static int nsim_pin_prio_set(const struct dpll_pin *pin, void *pin_priv,
>> + const struct dpll_device *dpll, void *dpll_priv,
>> + const u32 prio, struct netlink_ext_ack *extack)
>> +{
>> + struct nsim_pin_priv_data *pd = pin_priv;
>> +
>> + pd->prio = prio;
>> + return 0;
>> +};
>> +
>> +static void nsim_free_pin_properties(struct dpll_pin_properties *pp)
>> +{
>> + kfree(pp->board_label);
>> + kfree(pp->panel_label);
>> + kfree(pp->package_label);
>> + kfree(pp->freq_supported);
>> +}
>> +
>> +static struct dpll_device_ops nsim_dds_ops = {
>> + .mode_get = nsim_dds_ops_mode_get,
>> + .mode_supported = nsim_dds_ops_mode_supported,
>> + .lock_status_get = nsim_dds_ops_lock_status_get,
>> + .temp_get = nsim_dds_ops_temp_get,
>> +};
>> +
>> +static struct dpll_pin_ops nsim_pin_ops = {
>> + .frequency_set = nsim_pin_frequency_set,
>> + .frequency_get = nsim_pin_frequency_get,
>> + .direction_set = nsim_pin_direction_set,
>> + .direction_get = nsim_pin_direction_get,
>> + .state_on_pin_get = nsim_pin_state_on_pin_get,
>> + .state_on_dpll_get = nsim_pin_state_on_dpll_get,
>> + .state_on_pin_set = nsim_pin_state_on_pin_set,
>> + .state_on_dpll_set = nsim_pin_state_on_dpll_set,
>> + .prio_get = nsim_pin_prio_get,
>> + .prio_set = nsim_pin_prio_set,
>> +};
>> +
>> +int nsim_dpll_init_owner(struct nsim_dpll *dpll, unsigned int ports_count)
>> +{
>> + u64 clock_id;
>> + int err;
>> +
>> + get_random_bytes(&clock_id, sizeof(clock_id));
>> +
>> + /* Create EEC DPLL */
>> + dpll->dpll_e = dpll_device_get(clock_id, EEC_DPLL_DEV, THIS_MODULE);
>> + if (IS_ERR(dpll->dpll_e))
>> + return -EFAULT;
>> +
>> + dpll->dpll_e_pd.temperature = EEC_DPLL_TEMPERATURE;
>> + dpll->dpll_e_pd.mode = DPLL_MODE_AUTOMATIC;
>> + dpll->dpll_e_pd.clock_id = clock_id;
>> + dpll->dpll_e_pd.status = DPLL_LOCK_STATUS_UNLOCKED;
>> +
>> + err = dpll_device_register(dpll->dpll_e, DPLL_TYPE_EEC, &nsim_dds_ops,
>> + &dpll->dpll_e_pd);
>> + if (err)
>> + goto e_reg;
>> +
>> + /* Create PPS DPLL */
>> + dpll->dpll_p = dpll_device_get(clock_id, PPS_DPLL_DEV, THIS_MODULE);
>> + if (IS_ERR(dpll->dpll_p))
>> + goto dpll_p;
>> +
>> + dpll->dpll_p_pd.temperature = PPS_DPLL_TEMPERATURE;
>> + dpll->dpll_p_pd.mode = DPLL_MODE_MANUAL;
>> + dpll->dpll_p_pd.clock_id = clock_id;
>> + dpll->dpll_p_pd.status = DPLL_LOCK_STATUS_UNLOCKED;
>> +
>> + err = dpll_device_register(dpll->dpll_p, DPLL_TYPE_PPS, &nsim_dds_ops,
>> + &dpll->dpll_p_pd);
>> + if (err)
>> + goto p_reg;
>> +
>> + /* Create first pin (GNSS) */
>> + err = nsim_fill_pin_properties(&dpll->pp_gnss, "GNSS",
>> + DPLL_PIN_TYPE_GNSS,
>> + PIN_GNSS_CAPABILITIES, 1,
>> + DPLL_PIN_FREQUENCY_1_HZ,
>> + DPLL_PIN_FREQUENCY_1_HZ);
>> + if (err)
>> + goto pp_gnss;
>> + dpll->p_gnss =
>> + dpll_pin_get(clock_id, PIN_GNSS, THIS_MODULE, &dpll->pp_gnss);
>> + if (IS_ERR(dpll->p_gnss))
>> + goto p_gnss;
>
> Hi Michal,
>
> I think that err needs to be set to something inside the if condition
> above.
>
Hi Simon,
You have such a good eye, thanks - fixing that. I have also fixed another place where I made the
very same mistake.
>> + nsim_fill_pin_pd(&dpll->p_gnss_pd, DPLL_PIN_FREQUENCY_1_HZ,
>> + PIN_GNSS_PRIORITY, DPLL_PIN_DIRECTION_INPUT);
>> + err = dpll_pin_register(dpll->dpll_e, dpll->p_gnss, &nsim_pin_ops,
>> + &dpll->p_gnss_pd);
>> + if (err)
>> + goto e_gnss_reg;
>> +
>> + /* Create second pin (PPS) */
>> + err = nsim_fill_pin_properties(&dpll->pp_pps, "PPS", DPLL_PIN_TYPE_EXT,
>> + PIN_PPS_CAPABILITIES, 1,
>> + DPLL_PIN_FREQUENCY_1_HZ,
>> + DPLL_PIN_FREQUENCY_1_HZ);
>> + if (err)
>> + goto pp_pps;
>> + dpll->p_pps =
>> + dpll_pin_get(clock_id, PIN_PPS, THIS_MODULE, &dpll->pp_pps);
>> + if (IS_ERR(dpll->p_pps)) {
>> + err = -EFAULT;
>> + goto p_pps;
>> + }
>> + nsim_fill_pin_pd(&dpll->p_pps_pd, DPLL_PIN_FREQUENCY_1_HZ,
>> + PIN_PPS_PRIORITY, DPLL_PIN_DIRECTION_INPUT);
>> + err = dpll_pin_register(dpll->dpll_e, dpll->p_pps, &nsim_pin_ops,
>> + &dpll->p_pps_pd);
>> + if (err)
>> + goto e_pps_reg;
>> + err = dpll_pin_register(dpll->dpll_p, dpll->p_pps, &nsim_pin_ops,
>> + &dpll->p_pps_pd);
>> + if (err)
>> + goto p_pps_reg;
>> +
>> + dpll->pp_rclk =
>> + kcalloc(ports_count, sizeof(*dpll->pp_rclk), GFP_KERNEL);
>> + dpll->p_rclk = kcalloc(ports_count, sizeof(*dpll->p_rclk), GFP_KERNEL);
>> + dpll->p_rclk_pd =
>> + kcalloc(ports_count, sizeof(*dpll->p_rclk_pd), GFP_KERNEL);
>> +
>> + return 0;
>> +
>> +p_pps_reg:
>> + dpll_pin_unregister(dpll->dpll_e, dpll->p_pps, &nsim_pin_ops,
>> + &dpll->p_pps_pd);
>> +e_pps_reg:
>> + dpll_pin_put(dpll->p_pps);
>> +p_pps:
>> + nsim_free_pin_properties(&dpll->pp_pps);
>> +pp_pps:
>> + dpll_pin_unregister(dpll->dpll_e, dpll->p_gnss, &nsim_pin_ops,
>> + &dpll->p_gnss_pd);
>> +e_gnss_reg:
>> + dpll_pin_put(dpll->p_gnss);
>> +p_gnss:
>> + nsim_free_pin_properties(&dpll->pp_gnss);
>> +pp_gnss:
>> + dpll_device_unregister(dpll->dpll_p, &nsim_dds_ops, &dpll->dpll_p_pd);
>> +p_reg:
>> + dpll_device_put(dpll->dpll_p);
>> +dpll_p:
>> + dpll_device_unregister(dpll->dpll_e, &nsim_dds_ops, &dpll->dpll_e_pd);
>> +e_reg:
>> + dpll_device_put(dpll->dpll_e);
>> + return err;
>> +}
>
> ...
>
Powered by blists - more mailing lists