[linux-6.6.y] LoongArch: Add LoongArch security engine driver support #625
Conversation
Reviewer's Guide by SourceryThis pull request introduces the LoongArch security engine driver, comprising a core driver ( Sequence diagram for sending a command to the SEsequenceDiagram
participant User
participant lsse_sdf_cdev.c
participant lsse_sdf_dev
participant lsse_ch
participant loongson_se.c
User->>lsse_sdf_cdev.c: write()
lsse_sdf_cdev.c->>lsse_sdf_dev: lsse_sdf_send(buf, count, user)
lsse_sdf_dev->>lsse_ch: se_send_ch_requeset(ch)
lsse_ch->>loongson_se.c: se_send_ch_requeset(ch)
loongson_se.c->>loongson_se.c: se_writel(int_bit, SE_L2SINT_SET)
loongson_se.c-->>lsse_ch: Returns
lsse_ch-->>lsse_sdf_dev: Returns
lsse_sdf_dev-->>lsse_sdf_cdev.c: Returns
lsse_sdf_cdev.c-->>User: Returns
Sequence diagram for SE interrupt handlingsequenceDiagram
participant loongson_se.c
participant lsse_ch
loongson_se.c->>loongson_se.c: loongson_se_irq(irq, dev_id)
alt int_status & SE_INT_SETUP
loongson_se.c->>loongson_se.c: complete(&se->cmd_completion)
else int_status & other interrupts
loop for each interrupt
loongson_se.c->>lsse_ch: ch->complete(ch)
lsse_ch-->>loongson_se.c: Returns
end
end
loongson_se.c-->>loongson_se.c: Returns
Class diagram for Loongson SE driverclassDiagram
class loongson_se {
-struct device *dev
-void __iomem *base
-u32 version
-u32 ch_status
-spinlock_t cmd_lock
-spinlock_t dev_lock
-void *mem_base
-dma_addr_t mem_addr
-unsigned long *mem_map
-int mem_map_size
-void *smsg
-void *rmsg
-struct completion cmd_completion
-struct lsse_ch chs[SE_CH_MAX]
}
class lsse_ch {
-u32 id
-u32 int_bit
-struct loongson_se *se
-void *priv
-spinlock_t ch_lock
-void *smsg
-void *rmsg
-int msg_size
-void *data_buffer
-dma_addr_t data_addr
-int data_size
-void (*complete)(struct lsse_ch *se_ch)
}
loongson_se -- lsse_ch : contains
note for loongson_se "Represents the Loongson Security Engine device."
note for lsse_ch "Represents a channel within the Loongson Security Engine."
Class diagram for lsse_sdf_devclassDiagram
class lsse_sdf_dev {
-struct lsse_ch *se_ch
-struct mutex data_lock
-bool processing_cmd
-wait_queue_head_t wq
}
class lsse_ch
lsse_sdf_dev -- lsse_ch : contains
note for lsse_sdf_dev "Represents the Loongson SE SDF device."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi @AaronDot. Thanks for your PR. I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @AaronDot - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment block at the beginning of each file describing its purpose.
- It would be helpful to have some examples of how to use the new API.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
drivers/char/loongson_se.c
Outdated
| spin_unlock_irqrestore(&se->dev_lock, flag); | ||
| } | ||
|
|
||
| static int se_send_requeset(struct loongson_se *se, |
There was a problem hiding this comment.
suggestion (typo): Typo in function name: 'se_send_requeset' should be 'se_send_request'.
Correcting the spelling will improve clarity and consistency throughout the code.
| static int se_send_requeset(struct loongson_se *se, | |
| static int se_send_request(struct loongson_se *se, |
| wake_up(&se->wq); | ||
| } | ||
|
|
||
| static int se_send_sdf_cmd(struct lsse_sdf_dev *se, int len, int retry) |
There was a problem hiding this comment.
issue (complexity): Consider improving code readability and maintainability by extracting retry logic into helper functions and reducing nested error handling.
Consider reducing the nested error handling and retry logic by extracting these behaviors into helper functions. For example:
-
Extracting Retry Logic:
Instead of embedding a label andgotoinse_send_sdf_cmd(), create a helper function to handle the retry loop. This makes the logic self‐documenting and easier to maintain.static int send_cmd_with_retry(struct lsse_sdf_dev *se, int len, int retries) { int err = -EIO; // or an appropriate error code while (retries-- > 0) { pr_debug("Send sdf cmd, remaining retries: %d\n", retries); err = se_send_ch_requeset(se->se_ch); if (!err) return 0; udelay(5); } return err; }
Then update
se_send_sdf_cmd():static int se_send_sdf_cmd(struct lsse_sdf_dev *se, int len, int retry) { struct se_sdf_msg *smsg = (struct se_sdf_msg *)se->se_ch->smsg; unsigned long flag; int err; spin_lock_irqsave(&se->se_ch->ch_lock, flag); smsg->cmd = SE_CMD_SDF; smsg->data_off = se->se_ch->data_buffer - se->se_ch->se->mem_base; smsg->data_len = len; err = send_cmd_with_retry(se, len, retry); spin_unlock_irqrestore(&se->se_ch->ch_lock, flag); return err; }
-
Streamlining Error Paths:
In functions likelsse_sdf_send()andlsse_sdf_recv(), try to reduce deep nesting by returning early when an error occurs. This helps flatten the control flow and improves readability. For instance, refactor as:static int lsse_sdf_send(struct sdf_file_pvt_data *pvt, const char *buf, size_t count, int user) { int ret, se_ret; struct sdf_handle *ph = NULL; struct sdf_kernel_command *skc; struct lsse_sdf_dev *se = pvt->se; if (!se->se_ch->smsg) { pr_err("se device is not ready\n"); return 0; } if (count > se->se_ch->data_size) { pr_err("Invalid size in send: count=%zd, size=%d\n", count, se->se_ch->data_size); return -EIO; } ret = user ? mutex_lock_interruptible(&se->data_lock) : (mutex_lock(&se->data_lock), 0); if (ret) return ret; if (user) { ret = copy_from_user(se->se_ch->data_buffer, buf, count); if (ret) { ret = -EFAULT; goto out_unlock; } skc = (struct sdf_kernel_command *)se->se_ch->data_buffer; if (skc->header.command == SDF_OPENSESSION) ph = find_sdf_handle(skc->handle, pvt); } else { memcpy(se->se_ch->data_buffer, buf, count); } se->processing_cmd = true; ret = se_send_sdf_cmd(se, count, 5); if (ret) { pr_err("se_send_sdf_cmd failed\n"); goto out_unlock; } ret = lsse_sdf_recv(pvt, (char *)buf, 0, user, &se_ret); if (ret) { pr_err("recv failed ret: %x\n", ret); goto out_unlock; } if (ph && !se_ret) { list_del(&ph->handle_list); kfree(ph); } out_unlock: mutex_unlock(&se->data_lock); return ret; }
These small changes help isolate specific concerns (retry logic, error handling) and flatten nested conditions while keeping the original functionality intact.
|
Fix typo |
Signed-off-by: Hongchen Zhang <zhanghongchen@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Signed-off-by: Zhao Qunqin <zhaoqunqin@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
|
Fix typo: se_send_ch_requeset -> se_send_ch_request |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: opsiff, sourcery-ai[bot] The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Add LoongArch security engine driver support
Summary by Sourcery
This pull request introduces support for the Loongson Security Engine (SE) in the Linux kernel. It includes drivers for the SE module itself and its Secure Data File (SDF) interface, along with necessary header files, device tree bindings, and build configurations. The drivers enable the use of hardware-accelerated cryptographic functions provided by the Loongson SE.
New Features:
Enhancements:
Build: