Re: [PATCH v7 5/6] soc: qcom: add pd-mapper implementation

2024-04-26 Thread Chris Lew 




On 4/24/2024 2:28 AM, Dmitry Baryshkov wrote:


+static int qcom_pdm_start(void)
+{
+   const struct of_device_id *match;
+   const struct qcom_pdm_domain_data * const *domains;
+   struct device_node *root;
+   int ret, i;
+
+   root = of_find_node_by_path("/");
+   if (!root)
+   return -ENODEV;
+
+   match = of_match_node(qcom_pdm_domains, root);
+   of_node_put(root);
+   if (!match) {
+   pr_notice("PDM: no support for the platform, userspace daemon might 
be required.\n");
+   return -ENODEV;
+   }
+
+   domains = match->data;
+   if (!domains) {
+   pr_debug("PDM: no domains\n");
+   return -ENODEV;
+   }
+
+   mutex_lock(_pdm_mutex);
+   for (i = 0; domains[i]; i++) {
+   ret = qcom_pdm_add_domain(domains[i]);
+   if (ret)
+   goto free_domains;
+   }
+
+   ret = qmi_handle_init(_pdm_handle, 1024,
+ NULL, qcom_pdm_msg_handlers);


1024 here seems arbitrary, I think most other usage of qmi_handle_init 
has a macro defined for the max message length of the qmi service.



+   if (ret)
+   goto free_domains;
+
+   ret = qmi_add_server(_pdm_handle, SERVREG_LOCATOR_SERVICE,
+SERVREG_QMI_VERSION, SERVREG_QMI_INSTANCE);
+   if (ret) {
+   pr_err("PDM: error adding server %d\n", ret);
+   goto release_handle;
+   }
+   mutex_unlock(_pdm_mutex);
+
+   return 0;
+
+release_handle:
+   qmi_handle_release(_pdm_handle);
+
+free_domains:
+   qcom_pdm_free_domains();
+   mutex_unlock(_pdm_mutex);
+
+   return ret;
+}
+
+static void qcom_pdm_stop(void)
+{
+   qmi_del_server(_pdm_handle, SERVREG_LOCATOR_SERVICE,
+  SERVREG_QMI_VERSION, SERVREG_QMI_INSTANCE);
+
+   qmi_handle_release(_pdm_handle);
+


I don't think doing an explicit qmi_del_server() is necessary. As part 
of the qmi_handle_release(), the qrtr socket will be closed and the qrtr 
ns will broadcast a DEL_SERVER and DEL_CLIENT notification as part of 
the cleanup.



+   qcom_pdm_free_domains();
+}
+

Re: [PATCH v7 5/6] soc: qcom: add pd-mapper implementation

2024-04-26 Thread Alexey Minnekhanov 

On 24.04.2024 12:28, Dmitry Baryshkov wrote:

Existing userspace protection domain mapper implementation has several
issue. It doesn't play well with CONFIG_EXTRA_FIRMWARE, it doesn't
reread JSON files if firmware location is changed (or if firmware was
not available at the time pd-mapper was started but the corresponding
directory is mounted later), etc.

Provide in-kernel service implementing protection domain mapping
required to work with several services, which are provided by the DSP
firmware.

This module is loaded automatically by the remoteproc drivers when
necessary via the symbol dependency. It uses a root node to match a
protection domains map for a particular board. It is not possible to
implement it as a 'driver' as there is no corresponding device.

Signed-off-by: Dmitry Baryshkov 




diff --git a/drivers/soc/qcom/qcom_pd_mapper.c 
b/drivers/soc/qcom/qcom_pd_mapper.c

...

+
+static const struct qcom_pdm_domain_data *sdm660_domains[] = {
+   _audio_pd,
+   _root_pd,
+   _wlan_pd,
+   NULL,
+};
+


On my SDM660 device (xiaomi-lavender) I see also the following files on 
modem partition:

 - adsps.jsn with sensor_pd (instance id 74)
 - cdspr.jsn with cdsp root_pd (instance id 76)
 - modemr.jsn with root_pd (instance id 180)

I see these numbers match those you already have defined above, so 
perhaps sdm660_domains should also have adsp_sensor_pd, cdsp_root_pd, 
and mpss_root_pd?


I'm not sure how useful they are currently, as AFAIK cdsp is not added 
to sdm660 DT at all; and ADSP sensors are very hard to use/test, needs 
very special userspace...


--
Regards,
Alexey Minnekhanov
postmarketOS developer

[PATCH v7 5/6] soc: qcom: add pd-mapper implementation

2024-04-24 Thread Dmitry Baryshkov 
Existing userspace protection domain mapper implementation has several
issue. It doesn't play well with CONFIG_EXTRA_FIRMWARE, it doesn't
reread JSON files if firmware location is changed (or if firmware was
not available at the time pd-mapper was started but the corresponding
directory is mounted later), etc.

Provide in-kernel service implementing protection domain mapping
required to work with several services, which are provided by the DSP
firmware.

This module is loaded automatically by the remoteproc drivers when
necessary via the symbol dependency. It uses a root node to match a
protection domains map for a particular board. It is not possible to
implement it as a 'driver' as there is no corresponding device.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/soc/qcom/Kconfig   |  10 +
 drivers/soc/qcom/Makefile  |   1 +
 drivers/soc/qcom/pdr_internal.h|  14 +
 drivers/soc/qcom/qcom_pd_mapper.c  | 656 +
 drivers/soc/qcom/qcom_pdr_msg.c|  34 ++
 include/linux/soc/qcom/pd_mapper.h |  28 ++
 6 files changed, 743 insertions(+)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 95973c6b828f..f666366841b8 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -72,6 +72,16 @@ config QCOM_OCMEM
  requirements. This is typically used by the GPU, camera/video, and
  audio components on some Snapdragon SoCs.
 
+config QCOM_PD_MAPPER
+   tristate "Qualcomm Protection Domain Mapper"
+   select QCOM_QMI_HELPERS
+   depends on NET && QRTR
+   help
+ The Protection Domain Mapper maps registered services to the domains
+ and instances handled by the remote DSPs. This is a kernel-space
+ implementation of the service. It is a simpler alternative to the
+ userspace daemon.
+
 config QCOM_PDR_HELPERS
tristate
select QCOM_QMI_HELPERS
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 3110ac3288bc..d3560f861085 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
 obj-$(CONFIG_QCOM_GSBI)+=  qcom_gsbi.o
 obj-$(CONFIG_QCOM_MDT_LOADER)  += mdt_loader.o
 obj-$(CONFIG_QCOM_OCMEM)   += ocmem.o
+obj-$(CONFIG_QCOM_PD_MAPPER)   += qcom_pd_mapper.o
 obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
 obj-$(CONFIG_QCOM_PDR_MSG) += qcom_pdr_msg.o
 obj-$(CONFIG_QCOM_PMIC_GLINK)  += pmic_glink.o
diff --git a/drivers/soc/qcom/pdr_internal.h b/drivers/soc/qcom/pdr_internal.h
index 7e5bb5a95275..8d17f7fb79e7 100644
--- a/drivers/soc/qcom/pdr_internal.h
+++ b/drivers/soc/qcom/pdr_internal.h
@@ -13,6 +13,8 @@
 #define SERVREG_SET_ACK_REQ0x23
 #define SERVREG_RESTART_PD_REQ 0x24
 
+#define SERVREG_LOC_PFR_REQ0x24
+
 #define SERVREG_DOMAIN_LIST_LENGTH 32
 #define SERVREG_RESTART_PD_REQ_MAX_LEN 67
 #define SERVREG_REGISTER_LISTENER_REQ_LEN  71
@@ -20,6 +22,7 @@
 #define SERVREG_GET_DOMAIN_LIST_REQ_MAX_LEN74
 #define SERVREG_STATE_UPDATED_IND_MAX_LEN  79
 #define SERVREG_GET_DOMAIN_LIST_RESP_MAX_LEN   2389
+#define SERVREG_LOC_PFR_RESP_MAX_LEN   10
 
 struct servreg_location_entry {
char name[SERVREG_NAME_LENGTH + 1];
@@ -79,6 +82,15 @@ struct servreg_set_ack_resp {
struct qmi_response_type_v01 resp;
 };
 
+struct servreg_loc_pfr_req {
+   char service[SERVREG_NAME_LENGTH + 1];
+   char reason[257];
+};
+
+struct servreg_loc_pfr_resp {
+   struct qmi_response_type_v01 rsp;
+};
+
 extern const struct qmi_elem_info servreg_location_entry_ei[];
 extern const struct qmi_elem_info servreg_get_domain_list_req_ei[];
 extern const struct qmi_elem_info servreg_get_domain_list_resp_ei[];
@@ -89,5 +101,7 @@ extern const struct qmi_elem_info 
servreg_restart_pd_resp_ei[];
 extern const struct qmi_elem_info servreg_state_updated_ind_ei[];
 extern const struct qmi_elem_info servreg_set_ack_req_ei[];
 extern const struct qmi_elem_info servreg_set_ack_resp_ei[];
+extern const struct qmi_elem_info servreg_loc_pfr_req_ei[];
+extern const struct qmi_elem_info servreg_loc_pfr_resp_ei[];
 
 #endif
diff --git a/drivers/soc/qcom/qcom_pd_mapper.c 
b/drivers/soc/qcom/qcom_pd_mapper.c
new file mode 100644
index ..ba5440506c95
--- /dev/null
+++ b/drivers/soc/qcom/qcom_pd_mapper.c
@@ -0,0 +1,656 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Protection Domain mapper
+ *
+ * Copyright (c) 2023 Linaro Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "pdr_internal.h"
+
+#define SERVREG_QMI_VERSION 0x101
+#define SERVREG_QMI_INSTANCE 0
+
+#define TMS_SERVREG_SERVICE "tms/servreg"
+
+struct qcom_pdm_domain_data {
+   const char *domain;
+   u32 instance_id;
+   /* NULL-terminated array */
+   const char *