<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On Tuesday 10 October 2017 08:05 AM,
Stewart Smith wrote:<br>
</div>
<blockquote type="cite" cite="mid:87o9pfenm0.fsf@linux.vnet.ibm.com">
<pre wrap="">Madhavan Srinivasan <a class="moz-txt-link-rfc2396E" href="mailto:maddy@linux.vnet.ibm.com"><maddy@linux.vnet.ibm.com></a> writes:
</pre>
<blockquote type="cite">
<pre wrap="">IMC nest counters has both in-band (ucode) and out of
band access to it. Since not all nest counter configurations
are supported by ucode, out of band tools are used to
characterize other configuration.
So it is prefer to pause the nest microcode at boot to aid the
out of band tools. If the ucode not paused and OS does not
have IMC driver support, then out to band tools will race with
ucode and end up getting undesirable values. Patch to check and
pause the ucode at boot.
</pre>
</blockquote>
<pre wrap="">Please also mention how this works with the OPAL API as the API is that
OPAL_IMC_COUNTERS_INIT and START must be called for any counters that the OS
wants to use.</pre>
</blockquote>
<br>
Yes sure. Will do it.<br>
<br>
<blockquote type="cite" cite="mid:87o9pfenm0.fsf@linux.vnet.ibm.com">
<pre wrap="">
So, really, we were allowing buggy OSs to work as we were starting with
some counters running.</pre>
</blockquote>
<br>
Yeah, my bad. But OS will not have an impact ssince ucode<br>
update the counter data to reserve memory (HOMER).<br>
<br>
<blockquote type="cite" cite="mid:87o9pfenm0.fsf@linux.vnet.ibm.com">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">Signed-off-by: Madhavan Srinivasan <a class="moz-txt-link-rfc2396E" href="mailto:maddy@linux.vnet.ibm.com"><maddy@linux.vnet.ibm.com></a>
---
Currently only Linux driver support In-Memory Collection counter
hardware, which do pause the nest imc counters during driver
initialisation. So this change should not have any impact.
hw/imc.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/hw/imc.c b/hw/imc.c
index 10505f69d4bd..6f92c60d285d 100644
--- a/hw/imc.c
+++ b/hw/imc.c
@@ -128,6 +128,18 @@ static bool is_nest_mem_initialized(struct imc_chip_cb *ptr)
return true;
}
+static void pause_microcode_at_boot(void)
+{
+ struct proc_chip *chip;
+ struct imc_chip_cb *cb;
+
+ for_each_chip(chip) {
+ cb = (struct imc_chip_cb *)(chip->homer_base + P9_CB_STRUCT_OFFSET);
+ if (is_nest_mem_initialized(cb))
+ cb->imc_chip_command = cpu_to_be64(NEST_IMC_DISABLE);
+ }
+}
</pre>
</blockquote>
<pre wrap="">get_imc_cb() does all of this logic.</pre>
</blockquote>
<br>
Yes thats true. Will rewrite it.<br>
<br>
<blockquote type="cite" cite="mid:87o9pfenm0.fsf@linux.vnet.ibm.com">
<pre wrap="">
The command here doesn't have the mambo quirk that
opal_imc_counters_stop() has.
</pre>
</blockquote>
<br>
Thats yes. Incase of mambo, the function will be skip-ed at the
caller.<br>
<br>
Thanks for the review<br>
Maddy<br>
<blockquote type="cite" cite="mid:87o9pfenm0.fsf@linux.vnet.ibm.com">
<blockquote type="cite">
<pre wrap="">+
/*
* A Quad contains 4 cores in Power 9, and there are 4 addresses for
* the Core Hardware Trace Macro (CHTM) attached to each core.
@@ -543,6 +555,18 @@ imc_mambo:
return;
/*
+ * IMC nest counters has both in-band (ucode access) and out of band
+ * access to it. Since not all nest counter configurations are supported
+ * by ucode, out of band tools are used to characterize other
+ * configuration.
+ *
+ * If the ucode not paused and OS does not have IMC driver support,
+ * then out to band tools will race with ucode and end up getting
+ * undesirable values. Hence pause the ucode if it is already running.
+ */
+ pause_microcode_at_boot();
+
+ /*
* If the dt_attach_root() fails, "imc-counters" node will not be
* seen in the device-tree and hence OS should not make any
* OPAL_IMC_* calls.
--
2.7.4
</pre>
</blockquote>
</blockquote>
<br>
</body>
</html>