On 20.10.2016 07:12, David Gibson wrote:
> Currently spapr_create_fdt_skel() takes a bunch of individual parameters
> for various things it will put in the device tree. Some of these can
> already be taken directly from sPAPRMachineState. This patch alters it so
> that all of them can be taken from there, which will allow this code to
> be moved away from its current caller in future.
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> Reviewed-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> ---
> hw/ppc/spapr.c | 81
> ++++++++++++++++++++++----------------------------
> include/hw/ppc/spapr.h | 4 +++
> 2 files changed, 40 insertions(+), 45 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c4baca9..25c16d3 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -271,16 +271,12 @@ static void add_str(GString *s, const gchar *s1)
> g_string_append_len(s, s1, strlen(s1) + 1);
> }
>
> -static void *spapr_create_fdt_skel(hwaddr initrd_base,
> - hwaddr initrd_size,
> - hwaddr kernel_size,
> - bool little_endian,
> - const char *kernel_cmdline,
> - uint32_t epow_irq)
> +static void *spapr_create_fdt_skel(sPAPRMachineState *spapr)
> {
> + MachineState *machine = MACHINE(spapr);
> void *fdt;
> - uint32_t start_prop = cpu_to_be32(initrd_base);
> - uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
> + uint32_t start_prop = cpu_to_be32(spapr->initrd_base);
> + uint32_t end_prop = cpu_to_be32(spapr->initrd_base + spapr->initrd_size);
> GString *hypertas = g_string_sized_new(256);
> GString *qemu_hypertas = g_string_sized_new(256);
> uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
> @@ -305,11 +301,13 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
> fdt = g_malloc0(FDT_MAX_SIZE);
> _FDT((fdt_create(fdt, FDT_MAX_SIZE)));
>
> - if (kernel_size) {
> - _FDT((fdt_add_reservemap_entry(fdt, KERNEL_LOAD_ADDR, kernel_size)));
> + if (spapr->kernel_size) {
> + _FDT((fdt_add_reservemap_entry(fdt, KERNEL_LOAD_ADDR,
> + spapr->kernel_size)));
> }
> - if (initrd_size) {
> - _FDT((fdt_add_reservemap_entry(fdt, initrd_base, initrd_size)));
> + if (spapr->initrd_size) {
> + _FDT((fdt_add_reservemap_entry(fdt, spapr->initrd_base,
> + spapr->initrd_size)));
> }
> _FDT((fdt_finish_reservemap(fdt)));
>
> @@ -354,17 +352,17 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
> /* Set Form1_affinity */
> _FDT((fdt_property(fdt, "ibm,architecture-vec-5", vec5, sizeof(vec5))));
>
> - _FDT((fdt_property_string(fdt, "bootargs", kernel_cmdline)));
> + _FDT((fdt_property_string(fdt, "bootargs", machine->kernel_cmdline)));
> _FDT((fdt_property(fdt, "linux,initrd-start",
> &start_prop, sizeof(start_prop))));
> _FDT((fdt_property(fdt, "linux,initrd-end",
> &end_prop, sizeof(end_prop))));
> - if (kernel_size) {
> + if (spapr->kernel_size) {
> uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
> - cpu_to_be64(kernel_size) };
> + cpu_to_be64(spapr->kernel_size) };
>
> _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop))));
> - if (little_endian) {
> + if (spapr->kernel_le) {
> _FDT((fdt_property(fdt, "qemu,boot-kernel-le", NULL, 0)));
> }
> }
> @@ -441,7 +439,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
> _FDT((fdt_end_node(fdt)));
>
> /* event-sources */
> - spapr_events_fdt_skel(fdt, epow_irq);
> + spapr_events_fdt_skel(fdt, spapr->check_exception_irq);
>
> /* /hypervisor node */
> if (kvm_enabled()) {
Since kernel_size is used multiple times in this function, you could
simplify your patch a little bit by introducing a new local variable
"long kernel_size = spapr->kernel_size;" at the beginning of the
function... but anyway, patch looks good to me, so:
Reviewed-by: Thomas Huth <th...@redhat.com>
