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?




--
Alexey

Reply via email to