在 2024/6/19 7:35, Dave Jiang 写道:
On 6/18/24 9:53 AM, Shiyang Ruan wrote:
Background:
Since CXL device is a memory device, while CPU consumes a poison page of
CXL device, it always triggers a MCE by interrupt (INT18), no matter
which-First path is configured. This is the first report. Then
currently, in FW-First path, the poison event is transferred according
to the following process: CXL device -> firmware -> OS:ACPI->APEI->GHES
-> CPER -> trace report. This is the second one. These two reports
are indicating the same poisoning page, which is the so-called "duplicate
report"[1]. And the memory_failure() handling I'm trying to add in
OS-First path could also be another duplicate report.
Hope the flow below could make it easier to understand:
CPU accesses bad memory on CXL device, then
-> MCE (INT18), *always* report (1)
-> * FW-First (implemented now)
-> CXL device -> FW
-> OS:ACPI->APEI->GHES->CPER -> trace report (2.a)
* OS-First (not implemented yet, I'm working on it)
-> CXL device -> MSI
-> OS:CXL driver -> memory_failure() (2.b)
so, the (1) and (2.a/b) are duplicated.
(I didn't get response in my reply for [1] while I have to make patch to
solve this problem, so please correct me if my understanding is wrong.)
This patch adds a new notifier_block and MCE_PRIO_CXL, for CXL memdev
to check whether the current poison page has been reported (if yes,
stop the notifier chain, won't call the following memory_failure()
to report), into `x86_mce_decoder_chain`. In this way, if the poison
page already handled(recorded and reported) in (1) or (2), the other one
won't duplicate the report. The record could be clear when
cxl_clear_poison() is called.
[1]
https://lore.kernel.org/linux-cxl/664d948fb86f0_e8be29...@dwillia2-mobl3.amr.corp.intel.com.notmuch/
...
+
+static bool cxl_contains_hpa(const struct cxl_memdev *cxlmd, u64 hpa)
+{
+ struct cxl_contains_hpa_context ctx = {
+ .contains = false,
+ .hpa = hpa,
+ };
+ struct cxl_port *port;
+
+ port = cxlmd->endpoint;
+ if (port && is_cxl_endpoint(port) && cxl_num_decoders_committed(port))
Maybe no need to check is_cxl_endpoint() if the port is retrieved from
cxlmd->endpoint.
OK, I'll remove this.
Also, in order to use cxl_num_decoders_committed(), cxl_region_rwsem must be
held. See the lockdep_assert_held() in the function. Maybe add a
guard(cxl_regoin_rwsem);
before the if statement above.
Got it. I didn't realize it before. Will add it.
BTW, may I have your opinion on this proposal? I'm not sure if the
Background and problem described above are correct or not. If not, it
could lead me in the wrong direction.
Thank you very much!
--
Ruan.
DJ
