[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/9] target/ppc: add errp to kvmppc_read_int_cpu_dt()

From: Cédric Le Goater
Subject: Re: [PATCH 2/9] target/ppc: add errp to kvmppc_read_int_cpu_dt()
Date: Tue, 5 Jul 2022 11:44:57 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0

On 7/5/22 11:39, Daniel Henrique Barboza wrote:

On 7/5/22 03:51, Mark Cave-Ayland wrote:
On 04/07/2022 18:34, Cédric Le Goater wrote:

On 7/2/22 15:34, Daniel Henrique Barboza wrote:

On 7/2/22 03:24, Cédric Le Goater wrote:
On 6/30/22 21:42, Daniel Henrique Barboza wrote:
The function can't just return 0 whether an error happened and call it a
day. We must provide a way of letting callers know if the zero return is
legitimate or due to an error.

Add an Error pointer to kvmppc_read_int_cpu_dt() that will be filled
with an appropriate error, if one occurs. Callers are then free to pass
an Error pointer and handle it.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
  target/ppc/kvm.c | 16 +++++++++-------
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 109823136d..bc17437097 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1925,15 +1925,17 @@ static uint64_t kvmppc_read_int_dt(const char *filename)
   * Read a CPU node property from the host device tree that's a single
- * integer (32-bit or 64-bit).  Returns 0 if anything goes wrong
- * (can't find or open the property, or doesn't understand the format)
+ * integer (32-bit or 64-bit).  Returns 0 and set errp if anything goes
+ * wrong (can't find or open the property, or doesn't understand the
+ * format)
-static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
+static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp)
      char buf[PATH_MAX], *tmp;
      uint64_t val;
      if (kvmppc_find_cpu_dt(buf, sizeof(buf))) {
+        error_setg(errp, "Failed to read CPU property %s", propname);
          return 0;
@@ -1946,12 +1948,12 @@ static uint64_t kvmppc_read_int_cpu_dt(const char 
  uint64_t kvmppc_get_clockfreq(void)
-    return kvmppc_read_int_cpu_dt("clock-frequency");
+    return kvmppc_read_int_cpu_dt("clock-frequency", NULL);

This should be fatal. no ?

I'm not sure. I went under the assumption that there might be some weird
condition where 'clock-frequency' might be missing from the DT, and this
is why we're not exiting out immediately.

That said, the advantage of turning this into fatal is that we won't need
all the other patches that handles error on this function. We're going to
assume that if 'clock-frequency' is missing then we can't boot. I'm okay
with that.

I think so. Some machines behave badly when 'clock-frequency' is bogus,
division by zero, no console, etc. We could check easily with pseries
which is the only KVM PPC platform.

Well not quite true ;)  I haven't tested it during the last release cycle, but 
the Mac machines were still working fine to boot OS X with KVM-PR on my G4 Mac 
Mini last time I checked.

We can't just error_fatal by default then.

Each machine should be able to determine how to handle this missing DT
element. If I want to error_fatal for pseries then I can do so in patch
9/9, but other than that I'll keep the existing behavior.

Or add an errp here :

hw/ppc/e500.c:        clock_freq = kvmppc_get_clockfreq();
hw/ppc/sam460ex.c:        clock_freq = kvmppc_get_clockfreq();
hw/ppc/ppc440_bamboo.c:        clock_freq = kvmppc_get_clockfreq();
hw/ppc/spapr.c:    uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 


reply via email to

[Prev in Thread] Current Thread [Next in Thread]