feat(driver/acpi_pm): Implement ACPI PM Timer #772

1037827920 · 2024-04-26T00:55:37Z

feat: 实现 ACPI PM Timer

本 PR 实现了 ACPI的 PM Timer 功能，PM Timer 用于在没有hpet时校准tsc。

这次更改包括：

实现了 ACPI PM Timer 的初始化功能
添加了 ACPI PM Timer 的读取
在hpet读取失败时进行初始化acpi_pm

相关 Issue：#741

dragonosbot · 2024-04-26T00:55:44Z

dragonosbot has assigned @fslongjin.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

fslongjin · 2024-04-26T02:25:01Z

rv64编译报错的话，按照构建内核的那个文档，编译一下试试看看错哪了

fslongjin · 2024-04-26T03:11:13Z

kernel/src/arch/x86_64/acpi.rs

@@ -34,3 +42,24 @@ pub(super) fn early_acpi_boot_init() -> Result<(), SystemError> {
    // todo!("early_acpi_boot_init")
    return Ok(());
 }
+
+/// # 解析fadt
+fn acpi_parse_fadt() -> Result<(), SystemError> {


这些操作应该放在这个PM timer的初始化代码里面，并且按照现在这种写法，一旦机器上没有PM timer，就会直接报错无法启动。这是不正确的

fslongjin · 2024-04-26T03:13:27Z

kernel/src/driver/clocksource/acpi_pm.rs

+///
+/// ## 返回值
+/// - u32: 读取到的acpi_pmtmr值
+pub fn acpi_pm_read_verified() -> u32 {


这些参考了linux的话，把linux里面的代码链接也贴一贴

fslongjin · 2024-04-26T03:14:12Z

kernel/src/driver/clocksource/acpi_pm.rs

+}
+
+lazy_static! {
+    pub static ref CLOCKSOURCE_ACPI_PM: Arc<Acpipm> = Acpipm::new();


不要使用lazy static，而是弄个option。这样才能在确定的初始化时机，把它初始化了。这样有助于以后排除bug

fslongjin · 2024-04-26T03:15:25Z

kernel/src/time/clocksource.rs

@@ -57,12 +57,15 @@ pub static mut FINISHED_BOOTING: AtomicBool = AtomicBool::new(false);

 /// Interval: 0.5sec Threshold: 0.0625s
 /// 系统节拍率
-pub const HZ: u64 = 250;
+pub const HZ: u64 = 4;


这个地方要改回去

fslongjin · 2024-04-26T03:15:40Z

kernel/src/time/clocksource.rs

 /// watchdog检查间隔
 pub const WATCHDOG_INTERVAL: u64 = HZ >> 1;
 /// 最大能接受的误差大小
 pub const WATCHDOG_THRESHOLD: u32 = NSEC_PER_SEC >> 4;

+pub const MAX_SKEW_USEC: u64 = 125 * WATCHDOG_INTERVAL / HZ;


这些值从哪个地方弄过来的？麻烦打个链接

fslongjin · 2024-04-26T03:17:16Z

然后麻烦把对应的那个issue的链接，粘贴到PR的简介里面，并且编写PR简介

1037827920 · 2024-04-26T06:04:34Z

@dragonosbot review

fslongjin · 2024-04-27T07:38:26Z

kernel/src/driver/clocksource/acpi_pm.rs

+    }
+}
+
+#[cfg(target_arch = "riscv64")]


按照linux的语义，这个应写为#[cfg(not(target_arch = "x86_64"))]，所有linux里面为#ifndef x86_64的地方，都要写成#[cfg(not(target_arch = "x86_64"))]，而不是#[cfg(target_arch = "riscv64")]

fslongjin · 2024-04-27T07:40:26Z

kernel/src/driver/acpi/pmtmr.rs

+}
+
+#[inline(always)]
+#[cfg(target_arch = "riscv64")]


这里也是。

fslongjin · 2024-04-27T07:41:34Z

kernel/src/arch/riscv64/asm/mach_timer.rs

+pub const CALIBRATE_LATCH: u64 = (PIT_TICK_RATE * CALIBRATE_TIME_MSEC + 1000 / 2) / 1000;
+
+#[inline(always)]
+pub fn mach_prepare_counter() {


这些代码明明是x86的，为什么写到riscv64里面来？

这里的代码使用是在非x86的配置下使用的，如果放在x86里面不能用了。要不把这个文件的东西写到acpi_pm.rs里面？我看linux这这个文件的方法和常量仅在acpi_pm里面使用

要不把这个文件的东西写到acpi_pm.rs里面？

Ok我感觉可以

fslongjin · 2024-04-27T07:42:07Z

kernel/src/driver/clocksource/acpi_pm.rs

+
+    fn update_clocksource_data(&self, _data: ClocksourceData) -> Result<(), SystemError> {
+        let d = &mut self.0.lock_irqsave().data;
+        d.set_flags(_data.flags);


_data用到了的话就不用加_前缀了。

fslongjin · 2024-04-27T07:44:31Z

kernel/src/driver/clocksource/acpi_pm.rs

+        let value1 = clocksource_acpi_pm().read().data();
+        let mut i = 0;
+        for _ in 0..ACPI_PM_READ_CHECKS {
+            i += 1;


按照linux里面的含义，这个+=1的操作应当是在每次循环之后才加，而不是在循环开始的地方加。

fslongjin · 2024-04-27T07:45:33Z

kernel/src/driver/clocksource/acpi_pm.rs

+    }
+
+    // 检查ACPI PM Timer的频率是否正确
+    if verify_pmtmr_rate() != 0 {


这个函数linux里面的含义就是返回一个bool。因此需要修改一下。

fslongjin · 2024-04-27T07:48:49Z

kernel/src/time/clocksource.rs

+        let mut sft = 1;
+
+        // 计算限制转换范围的shift
+        let mut tmp = (maxsec * from) as u64 >> 32;


应当是两个都转成u64之后再相乘。

fslongjin · 2024-04-27T07:51:21Z

kernel/src/time/clocksource.rs

+    /// # 计算时钟源可以进行的最大调整量
+    fn clocksource_max_adjustment(&self) -> u32 {
+        let cs_data = self.clocksource_data();
+        let ret = cs_data.mult * 11 / 100;


cs_data.mult先转u64再乘11

fslongjin · 2024-04-27T07:53:57Z

kernel/src/time/clocksource.rs

+        }
+
+        let mut cs_data = self.clocksource_data();
+        cs_data.set_mult(tmp as u32);


这里的设置的操作应当在调用方那边写。linux里面这个函数是个通用函数，而你这样写的话是把这函数跟cs_data耦合起来了。因此应当把mult和shift返回出去，并且在外层调用cs_data相关的方法。

1037827920 · 2024-04-27T12:07:09Z

@dragonosbot review

* feat: Implement ACPI PM Timer

feat: Implement ACPI PM Timer

b8afbac

dragonosbot assigned fslongjin Apr 26, 2024

dragonosbot added O-x86_64 Target: x86_64 S-等待审查 Status: 等待assignee以及相关方的审查。 T-driver Relevant to the driver team, which will review and decide on the PR/issue. labels Apr 26, 2024

github-actions bot added the enhancement New feature or request label Apr 26, 2024

fix: execute make fmt and modify

a90b1ad

1037827920 changed the title ~~feat: Implement ACPI PM Timer~~ feat(driver/acpi_pm): Implement ACPI PM Timer Apr 26, 2024

github-actions bot removed the enhancement New feature or request label Apr 26, 2024

1037827920 added 4 commits April 26, 2024 09:28

fix: error reported under riscv run

4a10204

fix: execute make fmt and modify

3de0bbb

fix: error reported under riscv_run

7b2000b

fix: error reported under riscv_run

32ff14c

fslongjin requested changes Apr 26, 2024

View reviewed changes

dragonosbot added S-等待作者修改 Status: 这正在等待作者的一些操作（例如代码更改或更多信息）。 and removed S-等待审查 Status: 等待assignee以及相关方的审查。 labels Apr 26, 2024

github-actions bot added the enhancement New feature or request label Apr 26, 2024

fix: error reported under riscv_run

37cdbc8

dragonosbot added the O-riscv64 Target: riscv64 label Apr 26, 2024

1037827920 added 2 commits April 26, 2024 13:22

refactor: Apply reviewer feedback for code modifications

18969cd

Merge branch 'master' into patch-feat-acpi_pm

c3f497c

dragonosbot added S-等待审查 Status: 等待assignee以及相关方的审查。 and removed S-等待作者修改 Status: 这正在等待作者的一些操作（例如代码更改或更多信息）。 labels Apr 26, 2024

fslongjin requested changes Apr 27, 2024

View reviewed changes

dragonosbot added S-等待作者修改 Status: 这正在等待作者的一些操作（例如代码更改或更多信息）。 and removed S-等待审查 Status: 等待assignee以及相关方的审查。 labels Apr 27, 2024

1037827920 added 3 commits April 27, 2024 18:57

rafactor: Apply reviewer feedback for code modifications 2

09fa26d

配置改为编译x86架构

db83fc0

Merge branch 'master' into patch-feat-acpi_pm

fcdcb8a

dragonosbot added S-等待审查 Status: 等待assignee以及相关方的审查。 and removed S-等待作者修改 Status: 这正在等待作者的一些操作（例如代码更改或更多信息）。 labels Apr 27, 2024

fix

a9f0748

fslongjin approved these changes Apr 28, 2024

View reviewed changes

fslongjin merged commit dd8e74e into DragonOS-Community:master Apr 28, 2024
7 checks passed

1037827920 mentioned this pull request May 31, 2024

REQUEST: New membership for 1037827920 DragonOS-Community/teams_data#15

Closed

BrahmaMantra pushed a commit to BrahmaMantra/DragonOS that referenced this pull request Dec 9, 2024

feat(driver/acpi_pm): Implement ACPI PM Timer (DragonOS-Community#772)

344fd7f

* feat: Implement ACPI PM Timer

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(driver/acpi_pm): Implement ACPI PM Timer #772

feat(driver/acpi_pm): Implement ACPI PM Timer #772

1037827920 commented Apr 26, 2024 •

edited

Loading

dragonosbot commented Apr 26, 2024

fslongjin commented Apr 26, 2024

fslongjin Apr 26, 2024

fslongjin Apr 26, 2024

fslongjin Apr 26, 2024

fslongjin Apr 26, 2024

fslongjin Apr 26, 2024

fslongjin commented Apr 26, 2024

1037827920 commented Apr 26, 2024

fslongjin Apr 27, 2024

fslongjin Apr 27, 2024

fslongjin Apr 27, 2024

1037827920 Apr 27, 2024

fslongjin Apr 27, 2024

fslongjin Apr 27, 2024

fslongjin Apr 27, 2024

fslongjin Apr 27, 2024

fslongjin Apr 27, 2024

fslongjin Apr 27, 2024

fslongjin Apr 27, 2024

1037827920 commented Apr 27, 2024

feat(driver/acpi_pm): Implement ACPI PM Timer #772

feat(driver/acpi_pm): Implement ACPI PM Timer #772

Conversation

1037827920 commented Apr 26, 2024 • edited Loading

dragonosbot commented Apr 26, 2024

fslongjin commented Apr 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fslongjin commented Apr 26, 2024

1037827920 commented Apr 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1037827920 commented Apr 27, 2024

1037827920 commented Apr 26, 2024 •

edited

Loading