| 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: <64CBDD3CF274E966+d1faba44-e3aa-4c2f-9819-a24e4ddca468@radxa.com>
Date: Mon, 5 Jan 2026 22:00:39 +0800
From: Junhao Xie <bigfoot@...xa.com>
To: Bjorn Andersson <andersson@...nel.org>
Cc: Konrad Dybcio <konradybcio@...nel.org>, Xilin Wu <sophon@...xa.com>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-mtd@...ts.infradead.org, Junhao Xie <bigfoot@...xa.com>
Subject: Re: [PATCH 1/2] firmware: qcom: scm: Add SCM storage interface
support
On 2026/1/4 1:22, Bjorn Andersson wrote:
> On Fri, Dec 19, 2025 at 02:02:04AM +0800, Junhao Xie wrote:
>> Add infrastructure to support accessing TrustZone-protected storage
>> devices through SCM (Secure Channel Manager) calls. Some Qualcomm
>> platforms protect their firmware storage (typically SPI NOR flash)
>> via TrustZone, making it inaccessible from the non-secure world.
>>
>> Currently allowlisted for Radxa Dragon Q6A (QCS6490) where it has been
>> validated. Additional platforms can be added as they are tested.
>>
> By adding the relevant compatible, I'm able to read something from the
> SPI-NOR on the SC8280XP CRD as well.
>
> This brings us to the next question, what data do you actually have in
> your SPI-NOR? In what way do you use the mtd device that is presented?
>
> On the laptop targets, the SPI-NOR is partitioned with a GPT partition
> table, and there's one of more partitions that would be of interest to
> parse/access from the kernel...
On the Radxa Dragon Q6A, the SPI-NOR stores boot firmware (XBL, TZ, etc.)
and is accessed via SCM only when the system is booted from SPI-NOR.
Earlier Q6A revisions used LE firmware, where the QSPI controller was
directly accessible from Linux. This platform is now transitioning to WP,
where QSPI is exclusively used by TZ and SPI-NOR can only be accessed via
SCM, similar to laptop platforms.
The exposed MTD device provides generic access to this storage. One of its
practical uses is firmware update and recovery. Since Q6A is a development
board, being able to update boot firmware directly from Linux (for example
using edl-ng together with this patch series) is preferred over EFI capsule
based flows.
>> Signed-off-by: Junhao Xie <bigfoot@...xa.com>
>> Tested-by: Xilin Wu <sophon@...xa.com>
>> ---
>> drivers/firmware/qcom/qcom_scm.c | 183 +++++++++++++++++++++++++
>> drivers/firmware/qcom/qcom_scm.h | 3 +
>> include/linux/firmware/qcom/qcom_scm.h | 47 +++++++
>> 3 files changed, 233 insertions(+)
>>
>> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
>> index 731074ca1ebbe..b117e1b58e363 100644
>> --- a/drivers/firmware/qcom/qcom_scm.c
>> +++ b/drivers/firmware/qcom/qcom_scm.c
>> @@ -66,6 +66,21 @@ struct qcom_scm_mem_map_info {
>> __le64 mem_size;
>> };
>>
>> +struct qcom_scm_storage_cmd {
>> + __le64 storage_type;
>> + __le64 slot_num;
>> + __le64 lun;
>> + __le64 guid_ptr;
>> + __le64 storage_cmd;
>> +};
>> +
>> +struct qcom_scm_storage_cmd_details {
>> + __le64 lba;
>> + __le64 length;
>> + __le64 data_ptr;
>> + __le64 data_size;
>> +};
>> +
>> /**
>> * struct qcom_scm_qseecom_resp - QSEECOM SCM call response.
>> * @result: Result or status of the SCM call. See &enum qcom_scm_qseecom_result.
>> @@ -111,6 +126,15 @@ enum qcom_scm_qseecom_tz_cmd_info {
>> QSEECOM_TZ_CMD_INFO_VERSION = 3,
>> };
>>
>> +enum qcom_scm_storage_result {
> These aren't really "enumerations", they are specifically defined
> constants, please use #define.
I will convert them into macros.
>> + STORAGE_RESULT_SUCCESS = 0,
>> + STORAGE_RESULT_NO_MEMORY = 1,
>> + STORAGE_RESULT_INVALID_PARAMETER = 2,
>> + STORAGE_RESULT_STORAGE_ERROR = 3,
>> + STORAGE_RESULT_ACCESS_DENIED = 4,
>> + STORAGE_RESULT_NOT_SUPPORTED = 5,
>> +};
>> +
>> #define QSEECOM_MAX_APP_NAME_SIZE 64
>> #define SHMBRIDGE_RESULT_NOTSUPP 4
>>
>> @@ -2214,6 +2238,159 @@ static void qcom_scm_qtee_init(struct qcom_scm *scm)
>> devm_add_action_or_reset(scm->dev, qcom_scm_qtee_free, qtee_dev);
>> }
>>
>> +#if IS_ENABLED(CONFIG_MTD_QCOM_SCM_STORAGE)
>> +
>> +int qcom_scm_storage_send_cmd(enum qcom_scm_storage_type storage_type,
>> + enum qcom_scm_storage_cmd_id cmd_id,
>> + u64 lba, void *payload, size_t size)
>> +{
>> + struct qcom_scm_res scm_res = {};
>> + struct qcom_scm_desc desc = {};
>> + struct qcom_scm_storage_cmd *cmd;
>> + struct qcom_scm_storage_cmd_details *details;
>> + size_t buf_size;
>> + void *payload_buf;
>> + int ret;
>> +
>> + buf_size = sizeof(*cmd) + sizeof(*details);
>> + if (payload)
>> + buf_size += size;
>> + void *data __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
>> + buf_size,
>> + GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> + memset(data, 0, buf_size);
>> + if (payload) {
>> + payload_buf = data + sizeof(*cmd) + sizeof(*details);
>> + memcpy(payload_buf, payload, size);
>> + }
>> +
>> + cmd = data;
>> + cmd->storage_type = storage_type;
> storage_type is CPU endian, cmd->storage_type is __le64, so all of these
> needs cpu_to_le64().
I will add the missing endian conversion for all le values.
>> + cmd->storage_cmd = cmd_id;
>> +
>> + details = data + sizeof(*cmd);
>> + details->lba = lba;
[...]
>> +static bool qcom_scm_storage_machine_is_allowed(void)
>> +{
>> + struct device_node *np;
>> + bool match;
>> +
>> + np = of_find_node_by_path("/");
>> + if (!np)
>> + return false;
>> +
>> + match = of_match_node(qcom_scm_storage_allowlist, np);
>> + of_node_put(np);
>> +
>> + return match;
> This function can be rewritten as:
>
> return !!of_machine_device_match(qcom_scm_storage_allowlist);
I will replace this with a check using __qcom_scm_is_call_available(),
which avoids the need for a machine allowlist and provides a more
generic capability-based test, Thank you for your suggestion!
>> +}
>> +
[...]
>> --- a/include/linux/firmware/qcom/qcom_scm.h
>> +++ b/include/linux/firmware/qcom/qcom_scm.h
>> @@ -53,6 +53,36 @@ enum qcom_scm_ice_cipher {
>> QCOM_SCM_ICE_CIPHER_AES_256_CBC = 4,
>> };
>>
>> +enum qcom_scm_storage_cmd_id {
> As with qcom_scm_storage_result above, this isn't really an enumeration,
> but as it provides you with the type for the function parameters below,
> I think it's okay. So, feel free to leave this and qcom_scm_storage_type
> as is.
>
> Regards,
> Bjorn
>
Thank you for the review, I will incorporate these changes in v2.
Best regards,
Junhao Xie
Powered by blists - more mailing lists