intelhfi - Intel TD/HFI driver - Part2: Enable thermal interrupt handler for Local APIC's.

[koinec_yahoo.co.jp]

Summary

I developed the Intel Thread Director (ITD) / Hardware Feedback Interfce (HFI) device driver to obtain performance/efficiency information for each CPU core, which was implemented to improve the performance of Intel hybrid architecture CPUs. (e.g. Raptor Lake (refresh), Alder Lake, LakeField processors)

This driver simply obtains performance/efficiency information from the CPU and stores it in the "cpu_group" struct data referenced by the ULE scheduler.
However, since the ULE scheduler side is not yet supported, performance/efficiency cannot be improved by installing this driver at this time.

I will try to modify the ULE scheduler side in the future, but I posted this driver first because it can be implemented independently of this driver and it is difficult to modify the ULE scheduler.

There are seven patches, and this is the Part 2.
The performance/efficiency information of the ITD/HFI is updated using the Local APIC thermal interrupt, but the current implementation of the FreeBSD kernel does not use the Local APIC thermal interrupt. So enable this.
It also allows you to register interrupt mask control and handler functions.

Test Plan

With the remaining patches to be submitted later, I ran the tests listed below.

• FreeBSD 15-current: Be able to apply patches and build to the latest source tree as of 2024/03/20. After building, restart the OS and be able to start the kernel correctly.
• FreeBSD 14.0-Release: The source tree can be patched and built. After building, it is possible to load the driver and correctly obtain performance values from the CPU.

Due to my development environment, I developed it on FreeBSD 14.0-RELEASE and ported it to FreeBSD 15-current on a virtual environment.
However, I have also confirmed that the parts related to the code modified this time have hardly changed between 14.0-RELEASE and 15-current.
For this reason, I believe that it will probably work with 15-current, but if you are able to test it, please help confirm that it works.

[koinec_yahoo.co.jp]

Attach the full context patch file.

part2_enable_thermal_interrupt_for_lapic_fbsd15c.patch75 KBDownload

[koinec_yahoo.co.jp]

Attach the patch file created with the git format-patch command.

0002-Enable-local-APIC-s-thermal-interrupt-handler-for-th.patch75 KBDownload

[kib]

sys/x86/x86/local_apic.c

1638

1639

The line is clearly too long and should be wrapped.

Can the value for lapic_thermal_function_value change between check for != NULL and second reload to make the call? In other words, does disabling the LVT source drains the pending interrupts?

1658

Why these checks are needed?

1667

I do not think x86 can be compiled without EARLY_AP_STARTUP

[emaste]

sys/x86/x86/local_apic.c

1667

Yes EARLY_AP_STARTUP is mandatory on x86 after 792655abd64c94146ededd991652496ec9ec0cfe

[koinec_yahoo.co.jp]

• Delete the conditional branching part using the EARLY_AP_STARTUP macro constant. (because the EARLY_AP_STARTUP constant definition is now mandatory.)
• Add a comment to the x2apic_mode check block in the lapic_enable_thermal function.
• Modify the lapic_handle_thermal function so that the handle function and its argument pointers are saved to the stack before being used. (improving safety in the event that the pointers are cleared between the NULL check and the handle function call).

[koinec_yahoo.co.jp]

sys/x86/x86/local_apic.c

1658

Is this correct to say that a comment indicating the purpose of this block is missing?

Also, I'm sorry, but to be honest, I've created the LAPIC part by copying the lapic_{enable/disable/update}_pcint functions and other interrupts. I've looked at Intel SDM as much as possible, but I'm not confident about it.
If you notice any problems, I'd appreciate it if you could let me know.

[koinec_yahoo.co.jp]

sys/x86/x86/local_apic.c

1639

Thanks for pointing that out.

My understanding is that even if an interrupt is pending, it will not be issued after the LVT source is disabled.
In addition, the pointers to the handle function and its arguments are kept unchanged until the LVT source is invalidated within the intelhfi_detach() function.

For this reason, I understand that there is no need to check for NULL on the handle function or its argument pointers within the lapic_handle_thermal() function, but to be on the safe side, I want to copy them to a stack variable first, check for NULL, and then call the handle function.

[kib]

sys/x86/x86/local_apic.c

1642

Reorder loads of func and value. Load the value of func with atomic_load_acq_ptr(). No need to use atomic for load of value.

1644

Why checking the value? Or rather, the check for value != NULL should be removed.

1670

Store the function with atomic_store_rel_ptr().

1694

Store to the function using atomic_store_rel_ptr(). No need to make store to the value atomic, or even no need to set the value var to anything at all.

[koinec_yahoo.co.jp]

Modify to load and store handler function pointers using atomic(9) functions.

(Thanks for letting me know how to fix it. I've investigated and fixed the issue as much as possible based on the information you provided, but I'd appreciate any advice if you have any issues.)