On 7/5/22 08:57, Cédric Le Goater wrote:
On 7/5/22 08: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 <danielhb...@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 
*propname)
  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.

Oh. Sorry. and I still have access to a real G5 running the latest debian.
I should give it a try one day.

I gave KVM a try on a :

    cpu         : PPC970MP, altivec supported
    clock       : 2000.000000MHz
    revision    : 1.0 (pvr 0044 0100)
processor : 1
    cpu         : PPC970MP, altivec supported
    clock       : 2000.000000MHz
    revision    : 1.0 (pvr 0044 0100)
timebase : 33333333
    platform    : PowerMac
    model       : PowerMac11,2
    machine     : PowerMac11,2
    motherboard : PowerMac11,2 MacRISC4 Power Macintosh
    detected as : 337 (PowerMac G5 Dual Core)
    pmac flags  : 00000000
    L2 cache    : 1024K unified
    pmac-generation     : NewWorld
running debian with kernel 5.18.0-2-powerpc64. With the installed QEMU 7.0.0,

    qemu-system-ppc64 -M mac99 -cpu host -accel kvm ...

doesn't go very far. Program exception is quickly reached and host says:

    [56450.118422] Couldn't emulate instruction 0x00000000 (op 0 xop 0)
    [56450.119060] kvmppc_exit_pr_progint: emulation at 100 failed (00000000)

Anything special I should know ?

Thanks,

C.

Reply via email to