[Top][All Lists]

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

Re: [PATCH qemu v22] spapr: Implement Open Firmware client interface

From: BALATON Zoltan
Subject: Re: [PATCH qemu v22] spapr: Implement Open Firmware client interface
Date: Fri, 9 Jul 2021 00:22:15 +0200 (CEST)

On Thu, 8 Jul 2021, David Gibson wrote:
On Fri, Jun 25, 2021 at 03:51:55PM +1000, Alexey Kardashevskiy wrote:
diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
new file mode 100644
index 000000000000..a17fd9d2fe94
--- /dev/null
+++ b/hw/ppc/vof.c
+static int path_offset(const void *fdt, const char *path)
+    g_autofree char *p = NULL;
+    char *at;
+    /*
+     * 
+     *
+     * "Conversion from numeric representation to text representation shall use
+     * the lower case forms of the hexadecimal digits in the range a..f,
+     * suppressing leading zeros".

Huh... that suggests that Zoltan's firmware which passes a caps hex
and expects it to work is doing the wrong thing.  We still need to
accomodate it, though.

It's Linux which passes both upper and lower case variants (and all that a few line apart in the same file). The Pegasos2 SmartFirmware displays these with upper case address parts but accepts both upper and lower case. Here's a device tree dump from the original board firmware:


Apple's OpenFirmware seems to have lower case addresses:


but maybe it also accepts upper case? I can't try that now.

This works for pegasos2 guests I've tried but maybe only because the only problematic query is /pci@80000000/ide@C,1. If something wanted to get /pci@C0000000/isa@C then that might fail but I think most devices are on /pci@80000000 so this problem is unlikely to happen. The most correct way would be to convert all parts between @ and / or \0 to lower case but either this or the changed version in v23 which does strrcht('@') works for the cases I have.

+static void vof_instantiate_rtas(Error **errp)
+    error_setg(errp, "The firmware should have instantiated RTAS");

Since this always fails...

+static uint32_t vof_call_method(MachineState *ms, Vof *vof, uint32_t 
+                                uint32_t ihandle, uint32_t param1,
+                                uint32_t param2, uint32_t param3,
+                                uint32_t param4, uint32_t *ret2)
+    uint32_t ret = -1;
+    char method[VOF_MAX_METHODLEN] = "";
+    OfInstance *inst;
+    if (!ihandle) {
+        goto trace_exit;
+    }
+    inst = (OfInstance *)g_hash_table_lookup(vof->of_instances,
+                                             GINT_TO_POINTER(ihandle));
+    if (!inst) {
+        goto trace_exit;
+    }
+    if (readstr(methodaddr, method, sizeof(method))) {
+        goto trace_exit;
+    }
+    if (strcmp(inst->path, "/") == 0) {
+        if (strcmp(method, "ibm,client-architecture-support") == 0) {
+            Object *vmo = object_dynamic_cast(OBJECT(ms), TYPE_VOF_MACHINE_IF);
+            if (vmo) {
+                VofMachineIfClass *vmc = VOF_MACHINE_GET_CLASS(vmo);
+                g_assert(vmc->client_architecture_support);
+                ret = vmc->client_architecture_support(ms, first_cpu, param1);
+            }
+            *ret2 = 0;
+        }
+    } else if (strcmp(inst->path, "/rtas") == 0) {
+        if (strcmp(method, "instantiate-rtas") == 0) {

... why do you even need to handle it here?

This message has helped to catch problem with instantiate-rtas so it's useful to have here even if normally it would not get here. I don't remember what was the problem, maybe too small rtas-size or similar but getting a message instead of crashing did point to the problem.


reply via email to

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