|
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?
-- Alexey
[Prev in Thread] | Current Thread | [Next in Thread] |