| 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: <AAcA7gD9CIeobJKAa2X9napS.3.1586689508686.Hmail.wenhu.wang@vivo.com>
Date: Sun, 12 Apr 2020 19:05:08 +0800 (GMT+08:00)
From: 王文虎 <wenhu.wang@...o.com>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] driver: rpmon: qmi message version 01
Hi,
From: Randy Dunlap <rdunlap@...radead.org>
Date: 2020-04-12 03:07:11
To: Wang Wenhu <wenhu.wang@...o.com>,gregkh@...uxfoundation.org,linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] driver: rpmon: qmi message version 01>Hi--
>
>On 4/11/20 2:53 AM, Wang Wenhu wrote:
>> Implements a RPMON_QMI message set for a certain rpmon service.
>> RPMON_QMI defines its message types modularly. Each rpmon service
>> binds to a message set and introduced as a module. This version 1.0
>> message set could be used for connection checking of remote processors.
>>
>> RPMON_QMI messages depend on QCOM_QMI_HELPERS and should be updated
>> together with QMI related modules.
>>
>> Signed-off-by: Wang Wenhu <wenhu.wang@...o.com>
>> ---
>> drivers/rpmon/Kconfig | 13 ++
>> drivers/rpmon/Makefile | 1 +
>> drivers/rpmon/rpmon_qmi.h | 77 ++++++++++
>> drivers/rpmon/rpmon_qmi_msg_v1.c | 240 +++++++++++++++++++++++++++++++
>> 4 files changed, 331 insertions(+)
>> create mode 100644 drivers/rpmon/rpmon_qmi.h
>> create mode 100644 drivers/rpmon/rpmon_qmi_msg_v1.c
>>
>
>> diff --git a/drivers/rpmon/rpmon_qmi.h b/drivers/rpmon/rpmon_qmi.h
>> new file mode 100644
>> index 000000000000..f6e7cfc97a3f
>> --- /dev/null
>> +++ b/drivers/rpmon/rpmon_qmi.h
>> @@ -0,0 +1,77 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2020 Vivo Communication Technology Co. Ltd.
>> + * Copyright (C) 2020 Wang Wenhu <wenhu.wang@...o.com>
>> + * All rights reserved.
>> + */
>> +
>> +#ifndef RPMON_QMI_INTERFACE_H
>> +#define RPMON_QMI_INTERFACE_H
>> +
>> +#define RP_NAME_LEN 255
>> +#define RPQMI_BUF_SIZE 4096
>> +#define QMI_TLV_TL_SIZE 3
>> +
>> +enum rpmon_exec_result {
>> + RPMON_EXEC_SUCCESS = 0,
>> + RPMON_EXEC_FAILURE = 1,
>> +};
>> +
>> +struct rpmon_register_req {
>> + uint8_t name_valid;
>> + char name[RP_NAME_LEN + 1];
>> + uint8_t timeout_valid;
>> + u32 timeout;
>> +};
>
>Is this struct packed or padded?
>Is it shared with/used by other drivers?
>
It is passed and repacked by qmi interfaces and then sent out as QRTR packet,
non-shared with others. So it is safe to be left without any locking protection here.
>> +
>> +struct rpmon_conn_indication {
>> + char placeholder;
>> +};
>> +
>> +struct rpmon_conn_check_resp {
>> + uint8_t result_valid;
>> + struct qmi_response_type_v01 result;
>> +};
>> +
>> +struct rpmon_response {
>> + struct qmi_response_type_v01 resp;
>> +};
>> +
>> +struct rpmon_qmi_device {
>> + struct list_head list;
>> + struct sockaddr_qrtr addr;
>> + u32 timeout;
>> + u32 flag;
>> + struct ratelimit_state ratelimit;
>> +
>> + atomic_t checks;
>> + atomic_t reports;
>> +
>> + struct rpmon_info *info;
>> + struct rpmon_qmi *rqmi;
>> +};
>> +
>> +struct rpmon_qmi {
>> + struct rpmon_info *info;
>> + struct qmi_handle qmi;
>> + struct qmi_service *svc;
>> + struct qmi_ops *ops;
>> + struct qmi_msg_handler *handlers;
>> + int (*sendmsg)(const struct rpmon_qmi_device *rdev,
>> + const void *data,
>> + u32 len);
>> +};
>> +
>> +/* rpqmi message types currently supported. */
>> +enum rpmon_qmi_msg_type {
>> + RPQMI_MSG_REGISTER = 0,
>> + RPQMI_MSG_CONNCHK_RESP,
>> + RPQMI_MSG_MAX,
>> +};
>> +
>> +int rpmon_qmi_handle_init(struct rpmon_qmi *rqmi,
>> + void (*cb)(enum rpmon_qmi_msg_type type,
>> + struct sockaddr_qrtr *sq,
>> + const void *msg));
>> +
>> +#endif /* RPMON_QMI_INTERFACE_H */
>> diff --git a/drivers/rpmon/rpmon_qmi_msg_v1.c b/drivers/rpmon/rpmon_qmi_msg_v1.c
>> new file mode 100644
>> index 000000000000..10d38d8133b0
>> --- /dev/null
>> +++ b/drivers/rpmon/rpmon_qmi_msg_v1.c
>> @@ -0,0 +1,240 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 Vivo Communication Technology Co. Ltd.
>> + * Copyright (C) 2020 Wang Wenhu <wenhu.wang@...o.com>
>> + * All rights reserved.
>> + *
>> + * RPMON An implementation of remote processor monitor freamwork
>
>typo: framework {in all 3 source files}
>
>> + * for platforms that multi-processors exists. RPMON is implemented
>
>but that sentence is confusing at best.
>
Both addressed in v2
>> + * with chardev and sysfs class as interfaces to communicate with
>> + * the user level. It supports different communication interfaces
>> + * added modularly with remote processors. Currently QMI implementation
>> + * is available, and this file implements the message type v1.
>> + *
>> + * RPMON could be used to detect the stabilities of remote processors,
>> + * collect any kinds of information you are interested with, take
>
> interested in, take
>
Addressed in v2
>> + * actions like connection status check, and so on. Enhancements can
>> + * be made upon current implementation.
>> + */
>> +
>> +#include <linux/soc/qcom/qmi.h>
>> +#include <linux/rpmon.h>
>> +#include "rpmon_qmi.h"
>> +
>> +#define RPMON_SVC_ID_V01 0x3c
>> +#define RPMON_SVC_VER_V01 0x01
>> +#define RPMON_SVC_INS_V01 0x00
>> +
>> +#define RPMON_CONN_REQ_MSG_ID_VO1 0x20
>> +#define RPMON_CONN_RESP_MSG_ID_VO1 0x20
>> +#define RPMON_CONN_IND_MSG_ID_V01 0x21
>> +#define RPMON_EXEC_COMP_REQ_MSG_ID_V01 0x22
>> +#define RPMON_EXEC_COMP_RESP_MSG_ID_V01 0x22
>> +
>> +static struct qmi_elem_info register_req_v01_ei[] = {
>> + {
>> + .data_type = QMI_OPT_FLAG,
>> + .elem_len = 1,
>> + .elem_size = sizeof(uint8_t),
>> + .array_type = NO_ARRAY,
>> + .tlv_type = 0x10,
>
>Can you use an enum or #define value for the tlv_type magic numbers?
>(multiple places)
Definitely. But actually the definitions belong to QMI module, and I will
figure this out with another patch of #include <linux/soc/qcom/qmi.h> or so.
I will Cc you if not mind?
>
>> + .offset = offsetof(struct rpmon_register_req,
>> + name_valid),
>> + },
>> + {
>> + .data_type = QMI_STRING,
>> + .elem_len = RP_NAME_LEN,
>> + .elem_size = sizeof(char),
>> + .array_type = NO_ARRAY,
>> + .tlv_type = 0x10,
>> + .offset = offsetof(struct rpmon_register_req,
>> + name),
>> + },
>> + {
>> + .data_type = QMI_OPT_FLAG,
>> + .elem_len = 1,
>> + .elem_size = sizeof(uint8_t),
>> + .array_type = NO_ARRAY,
>> + .tlv_type = 0x11,
>> + .offset = offsetof(struct rpmon_register_req,
>> + timeout_valid),
>> + },
>> + {
>> + .data_type = QMI_UNSIGNED_4_BYTE,
>> + .elem_len = 1,
>> + .elem_size = sizeof(u32),
>> + .array_type = NO_ARRAY,
>> + .tlv_type = 0x11,
>> + .offset = offsetof(struct rpmon_register_req,
>> + timeout),
>> + },
>> + {
>> + .data_type = QMI_EOTI,
>> + .array_type = NO_ARRAY,
>> + .tlv_type = QMI_COMMON_TLV_TYPE,
>> + },
>> +};
>> +
>> +static struct qmi_elem_info conn_check_resp_v01_ei[] = {
>> + {
>> + .data_type = QMI_OPT_FLAG,
>> + .elem_len = 1,
>> + .elem_size = sizeof(uint8_t),
>> + .array_type = NO_ARRAY,
>> + .tlv_type = 0x10,
>> + .offset = offsetof(struct rpmon_conn_check_resp,
>> + result_valid),
>> + },
>> + {
>> + .data_type = QMI_SIGNED_4_BYTE_ENUM,
>> + .elem_len = 1,
>> + .elem_size = sizeof(enum rpmon_exec_result),
>> + .array_type = NO_ARRAY,
>> + .tlv_type = 0x10,
>> + .offset = offsetof(struct rpmon_conn_check_resp,
>> + result),
>> + },
>> + {
>> + .data_type = QMI_EOTI,
>> + .array_type = NO_ARRAY,
>> + .tlv_type = QMI_COMMON_TLV_TYPE,
>> + },
>> +};
>> +
>> +static struct qmi_elem_info conn_indication_v01_ei[] = {
>> + {
>> + .data_type = QMI_EOTI,
>> + .array_type = NO_ARRAY,
>> + .tlv_type = QMI_COMMON_TLV_TYPE,
>> + },
>> +};
>> +
>> +static struct qmi_elem_info response_v01_ei[] = {
>> + {
>> + .data_type = QMI_STRUCT,
>> + .elem_len = 1,
>> + .elem_size = sizeof(struct qmi_response_type_v01),
>> + .array_type = NO_ARRAY,
>> + .tlv_type = 0x02,
>> + .offset = offsetof(
>> + struct rpmon_response,
>> + resp),
>
>multiple lines not needed above.
Addressed in v2
>
>> + .ei_array = qmi_response_type_v01_ei,
>> + },
>> + {
>> + .data_type = QMI_EOTI,
>> + .array_type = NO_ARRAY,
>> + .tlv_type = QMI_COMMON_TLV_TYPE,
>> + },
>> +};
>> +
>> +static void (*msg_callback)(enum rpmon_qmi_msg_type type,
>> + struct sockaddr_qrtr *sq,
>> + const void *msg);
>> +
>> +static int rpmon_qmi_sendmsg_v1(const struct rpmon_qmi_device *rdev,
>> + const void *data,
>> + u32 len)
>> +{
>> + int ret;
>> + struct sockaddr_qrtr sq;
>> +
>> + memcpy(&sq, &rdev->addr, sizeof(sq));
>> +
>> + ret = qmi_send_indication(&rdev->rqmi->qmi, &sq,
>> + RPMON_CONN_IND_MSG_ID_V01,
>> + QMI_TLV_TL_SIZE,
>> + conn_indication_v01_ei, NULL);
>> + if (ret < 0)
>> + pr_err("Error %d send indication failed", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static void rpmon_qmi_recv_register_req_v1(struct qmi_handle *qmi,
>> + struct sockaddr_qrtr *sq,
>> + struct qmi_txn *txn,
>> + const void *msg)
>> +{
>> + struct rpmon_response resp;
>> + int ret;
>> +
>> + resp.resp.result = QMI_RESULT_SUCCESS_V01;
>> + resp.resp.error = QMI_ERR_NONE_V01;
>> + ret = qmi_send_response(qmi, sq, txn,
>> + RPMON_CONN_RESP_MSG_ID_VO1,
>> + sizeof(resp.resp) + QMI_TLV_TL_SIZE * 2,
>> + response_v01_ei,
>> + &resp.resp);
>> + if (ret < 0)
>> + pr_err("Error %d send respons failed", ret);
>> +
>> + if (msg_callback)
>> + msg_callback(RPQMI_MSG_REGISTER, sq, msg);
>> +}
>> +
>> +void rpmon_qmi_recv_conn_check_resp_v1(struct qmi_handle *qmi,
>> + struct sockaddr_qrtr *sq,
>> + struct qmi_txn *txn,
>> + const void *msg)
>> +{
>> + struct rpmon_response resp;
>> + int ret;
>> +
>> + resp.resp.result = QMI_RESULT_SUCCESS_V01;
>> + resp.resp.error = QMI_ERR_NONE_V01;
>> + ret = qmi_send_response(qmi, sq, txn,
>> + RPMON_EXEC_COMP_REQ_MSG_ID_V01,
>> + sizeof(resp.resp) + QMI_TLV_TL_SIZE * 2,
>> + response_v01_ei,
>> + &resp.resp);
>> + if (ret < 0)
>> + pr_err("Error %d send respons failed", ret);
>
> response
>
Addressed in v2
>> +
>> + if (msg_callback)
>> + msg_callback(RPQMI_MSG_CONNCHK_RESP, sq, msg);
>> +}
>> +
>> +static struct qmi_msg_handler rpmon_qmi_msg_handlers_v01[] = {
>> + {
>> + .type = QMI_REQUEST,
>> + .msg_id = RPMON_CONN_REQ_MSG_ID_VO1,
>> + .ei = register_req_v01_ei,
>> + .decoded_size = sizeof(struct rpmon_register_req),
>> + .fn = rpmon_qmi_recv_register_req_v1,
>> + },
>> + {
>> + .type = QMI_REQUEST,
>> + .msg_id = RPMON_EXEC_COMP_REQ_MSG_ID_V01,
>> + .ei = conn_check_resp_v01_ei,
>> + .decoded_size = sizeof(struct rpmon_conn_check_resp),
>> + .fn = rpmon_qmi_recv_conn_check_resp_v1,
>> + },
>> +};
>> +
>> +static struct qmi_service rpmon_qmi_svc = {
>> + .service = RPMON_SVC_ID_V01,
>> + .version = RPMON_SVC_VER_V01,
>> + .instance = RPMON_SVC_INS_V01,
>> +};
>> +
>> +int rpmon_qmi_handle_init(struct rpmon_qmi *rqmi,
>> + void (*cb)(enum rpmon_qmi_msg_type type,
>> + struct sockaddr_qrtr *sq,
>> + const void *msg))
>> +{
>> + rqmi->svc = &rpmon_qmi_svc;
>> + rqmi->handlers = rpmon_qmi_msg_handlers_v01;
>> + rqmi->sendmsg = rpmon_qmi_sendmsg_v1;
>> +
>> + if (cb)
>> + msg_callback = cb;
>> + else
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(rpmon_qmi_handle_init);
>> +
>> +MODULE_AUTHOR("Wang Wenhu");
>
>Please include email address in the string above.
>About 3/4 of all uses of MODULE_AUTHOR() do so.
>
Addressed in v2
>> +MODULE_LICENSE("GPL v2");
>>
>
>thanks.
>--
>~Randy
>
Thanks,
Wenhu
Powered by blists - more mailing lists