[Top][All Lists]

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

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

From: Alexey Kardashevskiy
Subject: Re: [PATCH qemu v13] spapr: Implement Open Firmware client interface
Date: Tue, 23 Feb 2021 23:19:38 +1100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:85.0) Gecko/20100101 Thunderbird/85.0

On 23/02/2021 14:07, David Gibson wrote:
On Tue, Feb 09, 2021 at 10:02:52PM +1100, Alexey Kardashevskiy wrote:
The PAPR platform which describes an OS environment that's presented by
a combination of a hypervisor and firmware. The features it specifies
require collaboration between the firmware and the hypervisor.


+target_ulong spapr_h_vof_client(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                                target_ulong opcode, target_ulong *args)
+    target_ulong of_client_args = ppc64_phys_to_real(args[0]);
+    struct prom_args pargs = { 0 };
+    char service[64];
+    unsigned nargs, nret, i;
+    cpu_physical_memory_read(of_client_args, &pargs, sizeof(pargs));

Need to check for read errors in case an out of bounds address is passed.

cpu_physical_memory_read() returns void and so does cpu_physical_memory_rw() but eventually called address_space_rw() returns an error code, should I switch to it?

+    nargs = be32_to_cpu(pargs.nargs);
+    if (nargs >= ARRAY_SIZE(pargs.args)) {
+        return H_PARAMETER;
+    }
+    cpu_physical_memory_read(be32_to_cpu(pargs.service), service,
+                             sizeof(service));
+    if (strnlen(service, sizeof(service)) == sizeof(service)) {
+        /* Too long service name */
+        return H_PARAMETER;
+    }
+    for (i = 0; i < nargs; ++i) {
+        pargs.args[i] = be32_to_cpu(pargs.args[i]);

In general I dislike in-place endian conversion of structs, since I
think it's less confusing to think of the endianness as a property of
the type.

The type is uint32_t and there is no be32 in QEMU. I can have 2 copies of pargs if this makes reviewing easier, should I?


reply via email to

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