QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries
@ 2019-09-11  4:04 David Gibson
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling David Gibson
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: David Gibson @ 2019-09-11  4:04 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: lvivier, aik, groug, clg, philmd, David Gibson

PAPR defines a feature negotiation protocol between host and guest
called ibm,client-architecture-support or CAS.  When invoked by the
guest this can either result in updated device tree information being
given to the guest to indicate the new configuration, or it can
trigger a reboot (called a CAS reboot) for more wide reaching
configuration changes.

CAS changes are time consuming and irritating for the user, so we'd
like to avoid them as much as possible.  We hope we can avoid them in
all situations, in fact, but there's some work to do to get there.

This series has some initial changes that get us closer to that mark.

Alexey Kardashevskiy (4):
  spapr: Fixes a leak in CAS
  spapr: Skip leading zeroes from memory@ DT node names
  spapr: Do not put empty properties for -kernel/-initrd/-append
  spapr: Stop providing RTAS blob

David Gibson (3):
  spapr: Simplify handling of pre ISA 3.0 guest workaround handling
  spapr: Move handling of special NVLink numa node from reset to init
  spapr: Perform machine reset in a more sensible order

 MAINTAINERS                     |   2 -
 Makefile                        |   2 +-
 configure                       |   6 +-
 hw/ppc/spapr.c                  | 128 ++++++++++++++------------------
 hw/ppc/spapr_hcall.c            |   3 +-
 hw/ppc/spapr_rtas.c             |  41 ----------
 include/hw/ppc/spapr.h          |   4 +-
 pc-bios/spapr-rtas.bin          | Bin 20 -> 0 bytes
 pc-bios/spapr-rtas/Makefile     |  27 -------
 pc-bios/spapr-rtas/spapr-rtas.S |  37 ---------
 10 files changed, 58 insertions(+), 192 deletions(-)
 delete mode 100644 pc-bios/spapr-rtas.bin
 delete mode 100644 pc-bios/spapr-rtas/Makefile
 delete mode 100644 pc-bios/spapr-rtas/spapr-rtas.S

-- 
2.21.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling
  2019-09-11  4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
@ 2019-09-11  4:04 ` David Gibson
  2019-09-11  7:09   ` Cédric Le Goater
                     ` (2 more replies)
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init David Gibson
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 25+ messages in thread
From: David Gibson @ 2019-09-11  4:04 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: lvivier, aik, groug, clg, philmd, David Gibson

Certain old guest versions don't understand the radix MMU introduced with
POWER ISA 3.0, but incorrectly select it if presented with the option at
CAS time.  We workaround this in qemu by explicitly excluding the radix
(and other ISA 3.0 linked) options if the guest doesn't explicitly note
support for ISA 3.0.

This is handled by the 'cas_legacy_guest_workaround' flag, which is pretty
vague.  Rename it to 'cas_pre_isa3_guest' to be clearer about what it's for.

In addition, we unnecessarily call spapr_populate_pa_features() with
different options when initially constructing the device tree and when
adjusting it at CAS time.  At the initial construct time cas_pre_isa3_guest
is already false, so we can still use the flag, rather than explicitly
overriding it to be false at the callsite.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c         | 10 ++++------
 hw/ppc/spapr_hcall.c   |  3 +--
 include/hw/ppc/spapr.h |  2 +-
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7124053b43..c551001f86 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -218,8 +218,7 @@ static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
 /* Populate the "ibm,pa-features" property */
 static void spapr_populate_pa_features(SpaprMachineState *spapr,
                                        PowerPCCPU *cpu,
-                                       void *fdt, int offset,
-                                       bool legacy_guest)
+                                       void *fdt, int offset)
 {
     uint8_t pa_features_206[] = { 6, 0,
         0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
@@ -285,7 +284,7 @@ static void spapr_populate_pa_features(SpaprMachineState *spapr,
     if ((spapr_get_cap(spapr, SPAPR_CAP_HTM) != 0) && pa_size > 24) {
         pa_features[24] |= 0x80;    /* Transactional memory support */
     }
-    if (legacy_guest && pa_size > 40) {
+    if (spapr->cas_pre_isa3_guest && pa_size > 40) {
         /* Workaround for broken kernels that attempt (guest) radix
          * mode when they can't handle it, if they see the radix bit set
          * in pa-features. So hide it from them. */
@@ -348,8 +347,7 @@ static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr)
             return ret;
         }
 
-        spapr_populate_pa_features(spapr, cpu, fdt, offset,
-                                   spapr->cas_legacy_guest_workaround);
+        spapr_populate_pa_features(spapr, cpu, fdt, offset);
     }
     return ret;
 }
@@ -551,7 +549,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
                           page_sizes_prop, page_sizes_prop_size)));
     }
 
-    spapr_populate_pa_features(spapr, cpu, fdt, offset, false);
+    spapr_populate_pa_features(spapr, cpu, fdt, offset);
 
     _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
                            cs->cpu_index / vcpus_per_socket)));
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 23e4bdb829..3d3a67149a 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1765,8 +1765,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
             exit(EXIT_FAILURE);
         }
     }
-    spapr->cas_legacy_guest_workaround = !spapr_ovec_test(ov1_guest,
-                                                          OV1_PPC_3_00);
+    spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
     spapr_ovec_cleanup(ov1_guest);
     if (!spapr->cas_reboot) {
         /* If spapr_machine_reset() did not set up a HPT but one is necessary
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 03111fd55b..dfec8e8e76 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -175,7 +175,7 @@ struct SpaprMachineState {
 
     /* ibm,client-architecture-support option negotiation */
     bool cas_reboot;
-    bool cas_legacy_guest_workaround;
+    bool cas_pre_isa3_guest;
     SpaprOptionVector *ov5;         /* QEMU-supported option vectors */
     SpaprOptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
     uint32_t max_compat_pvr;
-- 
2.21.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init
  2019-09-11  4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling David Gibson
@ 2019-09-11  4:04 ` David Gibson
  2019-09-11  7:28   ` Cédric Le Goater
                     ` (2 more replies)
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS David Gibson
                   ` (4 subsequent siblings)
  6 siblings, 3 replies; 25+ messages in thread
From: David Gibson @ 2019-09-11  4:04 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: lvivier, aik, groug, clg, philmd, David Gibson

The number of NUMA nodes in the system is fixed from the command line.
Therefore, there's no need to recalculate it at reset time, and we can
determine the special gpu_numa_id value used for NVLink2 devices at init
time.

This simplifies the reset path a bit which will make further improvements
easier.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c551001f86..e03e874d94 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1737,16 +1737,6 @@ static void spapr_machine_reset(MachineState *machine)
         spapr_setup_hpt_and_vrma(spapr);
     }
 
-    /*
-     * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
-     * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
-     * called from vPHB reset handler so we initialize the counter here.
-     * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
-     * must be equally distant from any other node.
-     * The final value of spapr->gpu_numa_id is going to be written to
-     * max-associativity-domains in spapr_build_fdt().
-     */
-    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
     qemu_devices_reset();
 
     /*
@@ -2885,6 +2875,17 @@ static void spapr_machine_init(MachineState *machine)
 
     }
 
+    /*
+     * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
+     * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
+     * called from vPHB reset handler so we initialize the counter here.
+     * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
+     * must be equally distant from any other node.
+     * The final value of spapr->gpu_numa_id is going to be written to
+     * max-associativity-domains in spapr_build_fdt().
+     */
+    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
+
     if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
         ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
                               spapr->max_compat_pvr)) {
-- 
2.21.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS
  2019-09-11  4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling David Gibson
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init David Gibson
@ 2019-09-11  4:04 ` David Gibson
  2019-09-11  7:28   ` Cédric Le Goater
  2019-09-11  7:36   ` Greg Kurz
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 4/7] spapr: Skip leading zeroes from memory@ DT node names David Gibson
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 25+ messages in thread
From: David Gibson @ 2019-09-11  4:04 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: lvivier, aik, groug, clg, philmd, David Gibson

From: Alexey Kardashevskiy <aik@ozlabs.ru>

Add a missing g_free(fdt) if the resulting tree is bigger
than the space allocated by SLOF.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e03e874d94..d93dacd483 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1024,6 +1024,7 @@ int spapr_h_cas_compose_response(SpaprMachineState *spapr,
     _FDT((fdt_pack(fdt)));
 
     if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
+        g_free(fdt);
         trace_spapr_cas_failed(size);
         return -1;
     }
-- 
2.21.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 4/7] spapr: Skip leading zeroes from memory@ DT node names
  2019-09-11  4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
                   ` (2 preceding siblings ...)
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS David Gibson
@ 2019-09-11  4:04 ` David Gibson
  2019-09-11  8:01   ` Greg Kurz
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append David Gibson
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: David Gibson @ 2019-09-11  4:04 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: lvivier, aik, groug, clg, philmd, David Gibson

From: Alexey Kardashevskiy <aik@ozlabs.ru>

The device tree build by QEMU at the machine reset time is used by SLOF
to build its internal device tree but the node names are not preserved
exactly so when QEMU provides a device tree update in response to H_CAS,
it might become tricky to match a node from the update blob to
the actual node in SLOF.

This removed leading zeroes from "memory@" nodes and makes
the DTC checker happy.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d93dacd483..d072c2aa3d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -386,7 +386,7 @@ static int spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,
     mem_reg_property[0] = cpu_to_be64(start);
     mem_reg_property[1] = cpu_to_be64(size);
 
-    sprintf(mem_name, "memory@" TARGET_FMT_lx, start);
+    sprintf(mem_name, "memory@%" HWADDR_PRIx, start);
     off = fdt_add_subnode(fdt, 0, mem_name);
     _FDT(off);
     _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));
-- 
2.21.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append
  2019-09-11  4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
                   ` (3 preceding siblings ...)
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 4/7] spapr: Skip leading zeroes from memory@ DT node names David Gibson
@ 2019-09-11  4:04 ` David Gibson
  2019-09-11  8:46   ` Greg Kurz
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob David Gibson
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order David Gibson
  6 siblings, 1 reply; 25+ messages in thread
From: David Gibson @ 2019-09-11  4:04 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: lvivier, aik, groug, clg, philmd, David Gibson

From: Alexey Kardashevskiy <aik@ozlabs.ru>

We are going to use spapr_build_fdt() for the boot time FDT and as an
update for SLOF during handling of H_CAS. SLOF will apply all properties
from the QEMU's FDT which is usually ok unless there are properties
changed by grub or guest kernel. The properties are:
bootargs, linux,initrd-start, linux,initrd-end, linux,stdout-path,
linux,rtas-base, linux,rtas-entry. Resetting those during CAS will most
likely cause grub failure.

This only creates such properties if we are booting with "-kernel" and
"-initrd" so they won't get included into the DT update blob and
therefore the guest is more likely to boot successfully.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d072c2aa3d..d18744268f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1177,11 +1177,16 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
 
     _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
 
-    _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline));
-    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
-                          spapr->initrd_base));
-    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
-                          spapr->initrd_base + spapr->initrd_size));
+    if (machine->kernel_cmdline && machine->kernel_cmdline[0]) {
+        _FDT(fdt_setprop_string(fdt, chosen, "bootargs",
+                                machine->kernel_cmdline));
+    }
+    if (spapr->initrd_size) {
+        _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
+                              spapr->initrd_base));
+        _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
+                              spapr->initrd_base + spapr->initrd_size));
+    }
 
     if (spapr->kernel_size) {
         uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
-- 
2.21.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob
  2019-09-11  4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
                   ` (4 preceding siblings ...)
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append David Gibson
@ 2019-09-11  4:04 ` David Gibson
  2019-09-11  9:16   ` Greg Kurz
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order David Gibson
  6 siblings, 1 reply; 25+ messages in thread
From: David Gibson @ 2019-09-11  4:04 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: lvivier, aik, groug, clg, philmd, David Gibson

From: Alexey Kardashevskiy <aik@ozlabs.ru>

SLOF implements one itself so let's remove it from QEMU. It is one less
image and simpler setup as the RTAS blob never stays in its initial place
anyway as the guest OS always decides where to put it.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 MAINTAINERS                     |   2 --
 Makefile                        |   2 +-
 configure                       |   6 +----
 hw/ppc/spapr.c                  |  32 ++-----------------------
 hw/ppc/spapr_rtas.c             |  41 --------------------------------
 include/hw/ppc/spapr.h          |   2 --
 pc-bios/spapr-rtas.bin          | Bin 20 -> 0 bytes
 pc-bios/spapr-rtas/Makefile     |  27 ---------------------
 pc-bios/spapr-rtas/spapr-rtas.S |  37 ----------------------------
 9 files changed, 4 insertions(+), 145 deletions(-)
 delete mode 100644 pc-bios/spapr-rtas.bin
 delete mode 100644 pc-bios/spapr-rtas/Makefile
 delete mode 100644 pc-bios/spapr-rtas/spapr-rtas.S

diff --git a/MAINTAINERS b/MAINTAINERS
index 50eaf005f4..9823f40213 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1077,8 +1077,6 @@ F: hw/*/spapr*
 F: include/hw/*/spapr*
 F: hw/*/xics*
 F: include/hw/*/xics*
-F: pc-bios/spapr-rtas/*
-F: pc-bios/spapr-rtas.bin
 F: pc-bios/slof.bin
 F: docs/specs/ppc-spapr-hcalls.txt
 F: docs/specs/ppc-spapr-hotplug.txt
diff --git a/Makefile b/Makefile
index ae17a83067..4637f95371 100644
--- a/Makefile
+++ b/Makefile
@@ -764,7 +764,7 @@ qemu-nsis.bmp \
 bamboo.dtb canyonlands.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
 multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin \
 s390-ccw.img s390-netboot.img \
-spapr-rtas.bin slof.bin skiboot.lid \
+slof.bin skiboot.lid \
 palcode-clipper \
 u-boot.e500 u-boot-sam460-20100605.bin \
 qemu_vga.ndrv \
diff --git a/configure b/configure
index 95134c0180..b79d38592b 100755
--- a/configure
+++ b/configure
@@ -6211,9 +6211,6 @@ if { test "$cpu" = "i386" || test "$cpu" = "x86_64"; } && \
         fi
     done
 fi
-if test "$ARCH" = "ppc64" && test "$targetos" != "Darwin" ; then
-  roms="$roms spapr-rtas"
-fi
 
 # Only build s390-ccw bios if we're on s390x and the compiler has -march=z900
 if test "$cpu" = "s390x" ; then
@@ -7930,14 +7927,13 @@ fi
 DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
 DIRS="$DIRS tests/fp tests/qgraph"
 DIRS="$DIRS docs docs/interop fsdev scsi"
-DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
+DIRS="$DIRS pc-bios/optionrom pc-bios/s390-ccw"
 DIRS="$DIRS roms/seabios roms/vgabios"
 LINKS="Makefile tests/tcg/Makefile"
 LINKS="$LINKS tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
 LINKS="$LINKS tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile"
 LINKS="$LINKS tests/fp/Makefile"
 LINKS="$LINKS pc-bios/optionrom/Makefile pc-bios/keymaps"
-LINKS="$LINKS pc-bios/spapr-rtas/Makefile"
 LINKS="$LINKS pc-bios/s390-ccw/Makefile"
 LINKS="$LINKS roms/seabios/Makefile roms/vgabios/Makefile"
 LINKS="$LINKS pc-bios/qemu-icon.bmp"
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d18744268f..5a919a6cc1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -94,7 +94,6 @@
  * We load our kernel at 4M, leaving space for SLOF initial image
  */
 #define FDT_MAX_SIZE            0x100000
-#define RTAS_MAX_SIZE           0x10000
 #define RTAS_MAX_ADDR           0x80000000 /* RTAS must stay below that */
 #define FW_MAX_SIZE             0x400000
 #define FW_FILE_NAME            "slof.bin"
@@ -1721,8 +1720,7 @@ static void spapr_machine_reset(MachineState *machine)
 {
     SpaprMachineState *spapr = SPAPR_MACHINE(machine);
     PowerPCCPU *first_ppc_cpu;
-    uint32_t rtas_limit;
-    hwaddr rtas_addr, fdt_addr;
+    hwaddr fdt_addr;
     void *fdt;
     int rc;
 
@@ -1786,14 +1784,10 @@ static void spapr_machine_reset(MachineState *machine)
      * or just below 2GB, whichever is lower, so that it can be
      * processed with 32-bit real mode code if necessary
      */
-    rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR);
-    rtas_addr = rtas_limit - RTAS_MAX_SIZE;
-    fdt_addr = rtas_addr - FDT_MAX_SIZE;
+    fdt_addr = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FDT_MAX_SIZE;
 
     fdt = spapr_build_fdt(spapr);
 
-    spapr_load_rtas(spapr, fdt, rtas_addr);
-
     rc = fdt_pack(fdt);
 
     /* Should only fail if we've built a corrupted tree */
@@ -2953,28 +2947,6 @@ static void spapr_machine_init(MachineState *machine)
         spapr_create_lmb_dr_connectors(spapr);
     }
 
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
-    if (!filename) {
-        error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
-        exit(1);
-    }
-    spapr->rtas_size = get_image_size(filename);
-    if (spapr->rtas_size < 0) {
-        error_report("Could not get size of LPAR rtas '%s'", filename);
-        exit(1);
-    }
-    spapr->rtas_blob = g_malloc(spapr->rtas_size);
-    if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
-        error_report("Could not load LPAR rtas '%s'", filename);
-        exit(1);
-    }
-    if (spapr->rtas_size > RTAS_MAX_SIZE) {
-        error_report("RTAS too big ! 0x%zx bytes (max is 0x%x)",
-                     (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
-        exit(1);
-    }
-    g_free(filename);
-
     /* Set up RTAS event infrastructure */
     spapr_events_init(spapr);
 
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index bee3835214..8d8d8cdfcb 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -477,47 +477,6 @@ void spapr_dt_rtas_tokens(void *fdt, int rtas)
     }
 }
 
-void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr)
-{
-    int rtas_node;
-    int ret;
-
-    /* Copy RTAS blob into guest RAM */
-    cpu_physical_memory_write(addr, spapr->rtas_blob, spapr->rtas_size);
-
-    ret = fdt_add_mem_rsv(fdt, addr, spapr->rtas_size);
-    if (ret < 0) {
-        error_report("Couldn't add RTAS reserve entry: %s",
-                     fdt_strerror(ret));
-        exit(1);
-    }
-
-    /* Update the device tree with the blob's location */
-    rtas_node = fdt_path_offset(fdt, "/rtas");
-    assert(rtas_node >= 0);
-
-    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-base", addr);
-    if (ret < 0) {
-        error_report("Couldn't add linux,rtas-base property: %s",
-                     fdt_strerror(ret));
-        exit(1);
-    }
-
-    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-entry", addr);
-    if (ret < 0) {
-        error_report("Couldn't add linux,rtas-entry property: %s",
-                     fdt_strerror(ret));
-        exit(1);
-    }
-
-    ret = fdt_setprop_cell(fdt, rtas_node, "rtas-size", spapr->rtas_size);
-    if (ret < 0) {
-        error_report("Couldn't add rtas-size property: %s",
-                     fdt_strerror(ret));
-        exit(1);
-    }
-}
-
 static void core_rtas_register_types(void)
 {
     spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index dfec8e8e76..cbd1a4c9f3 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -154,8 +154,6 @@ struct SpaprMachineState {
 
     hwaddr rma_size;
     int vrma_adjust;
-    ssize_t rtas_size;
-    void *rtas_blob;
     uint32_t fdt_size;
     uint32_t fdt_initial_size;
     void *fdt_blob;
diff --git a/pc-bios/spapr-rtas.bin b/pc-bios/spapr-rtas.bin
deleted file mode 100644
index fc24c8ed8b..0000000000
Binary files a/pc-bios/spapr-rtas.bin and /dev/null differ
diff --git a/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
deleted file mode 100644
index 4b9bb12306..0000000000
--- a/pc-bios/spapr-rtas/Makefile
+++ /dev/null
@@ -1,27 +0,0 @@
-all: build-all
-# Dummy command so that make thinks it has done something
-	@true
-
-include ../../config-host.mak
-include $(SRC_PATH)/rules.mak
-
-$(call set-vpath, $(SRC_PATH)/pc-bios/spapr-rtas)
-
-.PHONY : all clean build-all
-
-#CFLAGS += -I$(SRC_PATH)
-#QEMU_CFLAGS = $(CFLAGS)
-
-build-all: spapr-rtas.bin
-
-%.o: %.S
-	$(call quiet-command,$(CCAS) -mbig -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
-
-%.img: %.o
-	$(call quiet-command,$(CC) -nostdlib -mbig -o $@ $<,"Building","$(TARGET_DIR)$@")
-
-%.bin: %.img
-	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"Building","$(TARGET_DIR)$@")
-
-clean:
-	rm -f *.o *.d *.img *.bin *~
diff --git a/pc-bios/spapr-rtas/spapr-rtas.S b/pc-bios/spapr-rtas/spapr-rtas.S
deleted file mode 100644
index 903bec2150..0000000000
--- a/pc-bios/spapr-rtas/spapr-rtas.S
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
- *
- * Trivial in-partition RTAS implementation, based on a hypercall
- *
- * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- */
-
-#define KVMPPC_HCALL_BASE       0xf000
-#define KVMPPC_H_RTAS           (KVMPPC_HCALL_BASE + 0x0)
-
-.globl	_start
-_start:
-	mr	4,3
-	lis	3,KVMPPC_H_RTAS@h
-	ori	3,3,KVMPPC_H_RTAS@l
-	sc	1
-	blr
-- 
2.21.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order
  2019-09-11  4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
                   ` (5 preceding siblings ...)
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob David Gibson
@ 2019-09-11  4:04 ` David Gibson
  2019-09-11  7:40   ` Alexey Kardashevskiy
  2019-09-11  7:54   ` Cédric Le Goater
  6 siblings, 2 replies; 25+ messages in thread
From: David Gibson @ 2019-09-11  4:04 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: lvivier, aik, groug, clg, philmd, David Gibson

We've made several changes in the past to the machine reset order to fix
specific problems.  However, we've ended up with an order that doesn't make
a lot of logical sense.  This is an attempt to rectify this.

First we reset global CAS options, since that should depend on nothing
else.  Then we reset the CPUs, which shouldn't depend on external devices.
Then the irq subsystem, then the bulk of devices (which might rely on
irqs).  Finally we set up the entry state ready for boot, which could
potentially rely on a bunch of other things.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 47 +++++++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5a919a6cc1..1560a11738 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1724,6 +1724,28 @@ static void spapr_machine_reset(MachineState *machine)
     void *fdt;
     int rc;
 
+    /*
+     * If this reset wasn't generated by CAS, we should reset our
+     * negotiated options and start from scratch
+     */
+    if (!spapr->cas_reboot) {
+        spapr_ovec_cleanup(spapr->ov5_cas);
+        spapr->ov5_cas = spapr_ovec_new();
+
+        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
+    }
+
+    /*
+     * There is no CAS under qtest. Simulate one to please the code that
+     * depends on spapr->ov5_cas. This is especially needed to test device
+     * unplug, so we do that before resetting the DRCs.
+     */
+    if (qtest_enabled()) {
+        spapr_ovec_cleanup(spapr->ov5_cas);
+        spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
+    }
+
+    /* Reset the CPUs */
     spapr_caps_apply(spapr);
 
     first_ppc_cpu = POWERPC_CPU(first_cpu);
@@ -1741,34 +1763,15 @@ static void spapr_machine_reset(MachineState *machine)
         spapr_setup_hpt_and_vrma(spapr);
     }
 
-    qemu_devices_reset();
-
-    /*
-     * If this reset wasn't generated by CAS, we should reset our
-     * negotiated options and start from scratch
-     */
-    if (!spapr->cas_reboot) {
-        spapr_ovec_cleanup(spapr->ov5_cas);
-        spapr->ov5_cas = spapr_ovec_new();
-
-        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
-    }
-
+    /* Reset IRQ subsystem */
     /*
      * This is fixing some of the default configuration of the XIVE
      * devices. To be called after the reset of the machine devices.
      */
     spapr_irq_reset(spapr, &error_fatal);
 
-    /*
-     * There is no CAS under qtest. Simulate one to please the code that
-     * depends on spapr->ov5_cas. This is especially needed to test device
-     * unplug, so we do that before resetting the DRCs.
-     */
-    if (qtest_enabled()) {
-        spapr_ovec_cleanup(spapr->ov5_cas);
-        spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
-    }
+    /* Reset other devices */
+    qemu_devices_reset();
 
     /* DRC reset may cause a device to be unplugged. This will cause troubles
      * if this device is used by another device (eg, a running vhost backend
-- 
2.21.0



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling David Gibson
@ 2019-09-11  7:09   ` Cédric Le Goater
  2019-09-11  7:26   ` Greg Kurz
  2019-09-11  7:37   ` Alexey Kardashevskiy
  2 siblings, 0 replies; 25+ messages in thread
From: Cédric Le Goater @ 2019-09-11  7:09 UTC (permalink / raw)
  To: David Gibson, qemu-devel, qemu-ppc; +Cc: aik, lvivier, philmd, groug

On 11/09/2019 06:04, David Gibson wrote:
> Certain old guest versions don't understand the radix MMU introduced with
> POWER ISA 3.0, but incorrectly select it if presented with the option at
> CAS time.  We workaround this in qemu by explicitly excluding the radix
> (and other ISA 3.0 linked) options if the guest doesn't explicitly note
> support for ISA 3.0.
> 
> This is handled by the 'cas_legacy_guest_workaround' flag, which is pretty
> vague.  Rename it to 'cas_pre_isa3_guest' to be clearer about what it's for.
> 
> In addition, we unnecessarily call spapr_populate_pa_features() with
> different options when initially constructing the device tree and when
> adjusting it at CAS time.  At the initial construct time cas_pre_isa3_guest
> is already false, so we can still use the flag, rather than explicitly
> overriding it to be false at the callsite.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/ppc/spapr.c         | 10 ++++------
>  hw/ppc/spapr_hcall.c   |  3 +--
>  include/hw/ppc/spapr.h |  2 +-
>  3 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7124053b43..c551001f86 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -218,8 +218,7 @@ static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
>  /* Populate the "ibm,pa-features" property */
>  static void spapr_populate_pa_features(SpaprMachineState *spapr,
>                                         PowerPCCPU *cpu,
> -                                       void *fdt, int offset,
> -                                       bool legacy_guest)
> +                                       void *fdt, int offset)
>  {
>      uint8_t pa_features_206[] = { 6, 0,
>          0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> @@ -285,7 +284,7 @@ static void spapr_populate_pa_features(SpaprMachineState *spapr,
>      if ((spapr_get_cap(spapr, SPAPR_CAP_HTM) != 0) && pa_size > 24) {
>          pa_features[24] |= 0x80;    /* Transactional memory support */
>      }
> -    if (legacy_guest && pa_size > 40) {
> +    if (spapr->cas_pre_isa3_guest && pa_size > 40) {
>          /* Workaround for broken kernels that attempt (guest) radix
>           * mode when they can't handle it, if they see the radix bit set
>           * in pa-features. So hide it from them. */
> @@ -348,8 +347,7 @@ static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr)
>              return ret;
>          }
>  
> -        spapr_populate_pa_features(spapr, cpu, fdt, offset,
> -                                   spapr->cas_legacy_guest_workaround);
> +        spapr_populate_pa_features(spapr, cpu, fdt, offset);
>      }
>      return ret;
>  }
> @@ -551,7 +549,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>                            page_sizes_prop, page_sizes_prop_size)));
>      }
>  
> -    spapr_populate_pa_features(spapr, cpu, fdt, offset, false);
> +    spapr_populate_pa_features(spapr, cpu, fdt, offset);
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
>                             cs->cpu_index / vcpus_per_socket)));
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 23e4bdb829..3d3a67149a 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1765,8 +1765,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>              exit(EXIT_FAILURE);
>          }
>      }
> -    spapr->cas_legacy_guest_workaround = !spapr_ovec_test(ov1_guest,
> -                                                          OV1_PPC_3_00);
> +    spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
>      spapr_ovec_cleanup(ov1_guest);
>      if (!spapr->cas_reboot) {
>          /* If spapr_machine_reset() did not set up a HPT but one is necessary
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 03111fd55b..dfec8e8e76 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -175,7 +175,7 @@ struct SpaprMachineState {
>  
>      /* ibm,client-architecture-support option negotiation */
>      bool cas_reboot;
> -    bool cas_legacy_guest_workaround;
> +    bool cas_pre_isa3_guest;
>      SpaprOptionVector *ov5;         /* QEMU-supported option vectors */
>      SpaprOptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
>      uint32_t max_compat_pvr;
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling David Gibson
  2019-09-11  7:09   ` Cédric Le Goater
@ 2019-09-11  7:26   ` Greg Kurz
  2019-09-11  7:37   ` Alexey Kardashevskiy
  2 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2019-09-11  7:26 UTC (permalink / raw)
  To: David Gibson; +Cc: lvivier, aik, qemu-devel, qemu-ppc, clg, philmd

On Wed, 11 Sep 2019 14:04:46 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Certain old guest versions don't understand the radix MMU introduced with
> POWER ISA 3.0, but incorrectly select it if presented with the option at
> CAS time.  We workaround this in qemu by explicitly excluding the radix
> (and other ISA 3.0 linked) options if the guest doesn't explicitly note
> support for ISA 3.0.
> 
> This is handled by the 'cas_legacy_guest_workaround' flag, which is pretty
> vague.  Rename it to 'cas_pre_isa3_guest' to be clearer about what it's for.
> 
> In addition, we unnecessarily call spapr_populate_pa_features() with
> different options when initially constructing the device tree and when
> adjusting it at CAS time.  At the initial construct time cas_pre_isa3_guest
> is already false, so we can still use the flag, rather than explicitly
> overriding it to be false at the callsite.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c         | 10 ++++------
>  hw/ppc/spapr_hcall.c   |  3 +--
>  include/hw/ppc/spapr.h |  2 +-
>  3 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7124053b43..c551001f86 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -218,8 +218,7 @@ static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
>  /* Populate the "ibm,pa-features" property */
>  static void spapr_populate_pa_features(SpaprMachineState *spapr,
>                                         PowerPCCPU *cpu,
> -                                       void *fdt, int offset,
> -                                       bool legacy_guest)
> +                                       void *fdt, int offset)
>  {
>      uint8_t pa_features_206[] = { 6, 0,
>          0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> @@ -285,7 +284,7 @@ static void spapr_populate_pa_features(SpaprMachineState *spapr,
>      if ((spapr_get_cap(spapr, SPAPR_CAP_HTM) != 0) && pa_size > 24) {
>          pa_features[24] |= 0x80;    /* Transactional memory support */
>      }
> -    if (legacy_guest && pa_size > 40) {
> +    if (spapr->cas_pre_isa3_guest && pa_size > 40) {
>          /* Workaround for broken kernels that attempt (guest) radix
>           * mode when they can't handle it, if they see the radix bit set
>           * in pa-features. So hide it from them. */
> @@ -348,8 +347,7 @@ static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr)
>              return ret;
>          }
>  
> -        spapr_populate_pa_features(spapr, cpu, fdt, offset,
> -                                   spapr->cas_legacy_guest_workaround);
> +        spapr_populate_pa_features(spapr, cpu, fdt, offset);
>      }
>      return ret;
>  }
> @@ -551,7 +549,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>                            page_sizes_prop, page_sizes_prop_size)));
>      }
>  
> -    spapr_populate_pa_features(spapr, cpu, fdt, offset, false);
> +    spapr_populate_pa_features(spapr, cpu, fdt, offset);
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
>                             cs->cpu_index / vcpus_per_socket)));
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 23e4bdb829..3d3a67149a 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1765,8 +1765,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>              exit(EXIT_FAILURE);
>          }
>      }
> -    spapr->cas_legacy_guest_workaround = !spapr_ovec_test(ov1_guest,
> -                                                          OV1_PPC_3_00);
> +    spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
>      spapr_ovec_cleanup(ov1_guest);
>      if (!spapr->cas_reboot) {
>          /* If spapr_machine_reset() did not set up a HPT but one is necessary
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 03111fd55b..dfec8e8e76 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -175,7 +175,7 @@ struct SpaprMachineState {
>  
>      /* ibm,client-architecture-support option negotiation */
>      bool cas_reboot;
> -    bool cas_legacy_guest_workaround;
> +    bool cas_pre_isa3_guest;
>      SpaprOptionVector *ov5;         /* QEMU-supported option vectors */
>      SpaprOptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
>      uint32_t max_compat_pvr;



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS David Gibson
@ 2019-09-11  7:28   ` Cédric Le Goater
  2019-09-11  7:36   ` Greg Kurz
  1 sibling, 0 replies; 25+ messages in thread
From: Cédric Le Goater @ 2019-09-11  7:28 UTC (permalink / raw)
  To: David Gibson, qemu-devel, qemu-ppc; +Cc: aik, lvivier, philmd, groug

On 11/09/2019 06:04, David Gibson wrote:
> From: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Add a missing g_free(fdt) if the resulting tree is bigger
> than the space allocated by SLOF.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/ppc/spapr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e03e874d94..d93dacd483 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1024,6 +1024,7 @@ int spapr_h_cas_compose_response(SpaprMachineState *spapr,
>      _FDT((fdt_pack(fdt)));
>  
>      if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
> +        g_free(fdt);
>          trace_spapr_cas_failed(size);
>          return -1;
>      }
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init David Gibson
@ 2019-09-11  7:28   ` Cédric Le Goater
  2019-09-11  7:33   ` Greg Kurz
  2019-09-11  7:41   ` Alexey Kardashevskiy
  2 siblings, 0 replies; 25+ messages in thread
From: Cédric Le Goater @ 2019-09-11  7:28 UTC (permalink / raw)
  To: David Gibson, qemu-devel, qemu-ppc; +Cc: aik, lvivier, philmd, groug

On 11/09/2019 06:04, David Gibson wrote:
> The number of NUMA nodes in the system is fixed from the command line.
> Therefore, there's no need to recalculate it at reset time, and we can
> determine the special gpu_numa_id value used for NVLink2 devices at init
> time.
> 
> This simplifies the reset path a bit which will make further improvements
> easier.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/ppc/spapr.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c551001f86..e03e874d94 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1737,16 +1737,6 @@ static void spapr_machine_reset(MachineState *machine)
>          spapr_setup_hpt_and_vrma(spapr);
>      }
>  
> -    /*
> -     * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
> -     * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
> -     * called from vPHB reset handler so we initialize the counter here.
> -     * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
> -     * must be equally distant from any other node.
> -     * The final value of spapr->gpu_numa_id is going to be written to
> -     * max-associativity-domains in spapr_build_fdt().
> -     */
> -    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
>      qemu_devices_reset();
>  
>      /*
> @@ -2885,6 +2875,17 @@ static void spapr_machine_init(MachineState *machine)
>  
>      }
>  
> +    /*
> +     * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
> +     * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
> +     * called from vPHB reset handler so we initialize the counter here.
> +     * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
> +     * must be equally distant from any other node.
> +     * The final value of spapr->gpu_numa_id is going to be written to
> +     * max-associativity-domains in spapr_build_fdt().
> +     */
> +    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
> +
>      if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
>          ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
>                                spapr->max_compat_pvr)) {
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init David Gibson
  2019-09-11  7:28   ` Cédric Le Goater
@ 2019-09-11  7:33   ` Greg Kurz
  2019-09-11  7:41   ` Alexey Kardashevskiy
  2 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2019-09-11  7:33 UTC (permalink / raw)
  To: David Gibson; +Cc: lvivier, aik, qemu-devel, qemu-ppc, clg, philmd

On Wed, 11 Sep 2019 14:04:47 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The number of NUMA nodes in the system is fixed from the command line.
> Therefore, there's no need to recalculate it at reset time, and we can
> determine the special gpu_numa_id value used for NVLink2 devices at init
> time.
> 
> This simplifies the reset path a bit which will make further improvements
> easier.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c551001f86..e03e874d94 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1737,16 +1737,6 @@ static void spapr_machine_reset(MachineState *machine)
>          spapr_setup_hpt_and_vrma(spapr);
>      }
>  
> -    /*
> -     * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
> -     * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
> -     * called from vPHB reset handler so we initialize the counter here.
> -     * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
> -     * must be equally distant from any other node.
> -     * The final value of spapr->gpu_numa_id is going to be written to
> -     * max-associativity-domains in spapr_build_fdt().
> -     */
> -    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
>      qemu_devices_reset();
>  
>      /*
> @@ -2885,6 +2875,17 @@ static void spapr_machine_init(MachineState *machine)
>  
>      }
>  
> +    /*
> +     * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
> +     * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
> +     * called from vPHB reset handler so we initialize the counter here.
> +     * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
> +     * must be equally distant from any other node.
> +     * The final value of spapr->gpu_numa_id is going to be written to
> +     * max-associativity-domains in spapr_build_fdt().
> +     */
> +    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
> +
>      if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
>          ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
>                                spapr->max_compat_pvr)) {



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS David Gibson
  2019-09-11  7:28   ` Cédric Le Goater
@ 2019-09-11  7:36   ` Greg Kurz
  1 sibling, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2019-09-11  7:36 UTC (permalink / raw)
  To: David Gibson; +Cc: lvivier, aik, qemu-devel, qemu-ppc, clg, philmd

On Wed, 11 Sep 2019 14:04:48 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> From: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Add a missing g_free(fdt) if the resulting tree is bigger
> than the space allocated by SLOF.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e03e874d94..d93dacd483 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1024,6 +1024,7 @@ int spapr_h_cas_compose_response(SpaprMachineState *spapr,
>      _FDT((fdt_pack(fdt)));
>  
>      if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
> +        g_free(fdt);
>          trace_spapr_cas_failed(size);
>          return -1;
>      }



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling David Gibson
  2019-09-11  7:09   ` Cédric Le Goater
  2019-09-11  7:26   ` Greg Kurz
@ 2019-09-11  7:37   ` Alexey Kardashevskiy
  2 siblings, 0 replies; 25+ messages in thread
From: Alexey Kardashevskiy @ 2019-09-11  7:37 UTC (permalink / raw)
  To: David Gibson, qemu-devel, qemu-ppc; +Cc: lvivier, philmd, groug, clg



On 11/09/2019 14:04, David Gibson wrote:
> Certain old guest versions don't understand the radix MMU introduced with
> POWER ISA 3.0, but incorrectly select it if presented with the option at
> CAS time.  We workaround this in qemu by explicitly excluding the radix
> (and other ISA 3.0 linked) options if the guest doesn't explicitly note
> support for ISA 3.0.
> 
> This is handled by the 'cas_legacy_guest_workaround' flag, which is pretty
> vague.  Rename it to 'cas_pre_isa3_guest' to be clearer about what it's for.
> 
> In addition, we unnecessarily call spapr_populate_pa_features() with
> different options when initially constructing the device tree and when
> adjusting it at CAS time.  At the initial construct time cas_pre_isa3_guest
> is already false, so we can still use the flag, rather than explicitly
> overriding it to be false at the callsite.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>



Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>




> ---
>  hw/ppc/spapr.c         | 10 ++++------
>  hw/ppc/spapr_hcall.c   |  3 +--
>  include/hw/ppc/spapr.h |  2 +-
>  3 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7124053b43..c551001f86 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -218,8 +218,7 @@ static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
>  /* Populate the "ibm,pa-features" property */
>  static void spapr_populate_pa_features(SpaprMachineState *spapr,
>                                         PowerPCCPU *cpu,
> -                                       void *fdt, int offset,
> -                                       bool legacy_guest)
> +                                       void *fdt, int offset)
>  {
>      uint8_t pa_features_206[] = { 6, 0,
>          0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> @@ -285,7 +284,7 @@ static void spapr_populate_pa_features(SpaprMachineState *spapr,
>      if ((spapr_get_cap(spapr, SPAPR_CAP_HTM) != 0) && pa_size > 24) {
>          pa_features[24] |= 0x80;    /* Transactional memory support */
>      }
> -    if (legacy_guest && pa_size > 40) {
> +    if (spapr->cas_pre_isa3_guest && pa_size > 40) {
>          /* Workaround for broken kernels that attempt (guest) radix
>           * mode when they can't handle it, if they see the radix bit set
>           * in pa-features. So hide it from them. */
> @@ -348,8 +347,7 @@ static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr)
>              return ret;
>          }
>  
> -        spapr_populate_pa_features(spapr, cpu, fdt, offset,
> -                                   spapr->cas_legacy_guest_workaround);
> +        spapr_populate_pa_features(spapr, cpu, fdt, offset);
>      }
>      return ret;
>  }
> @@ -551,7 +549,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>                            page_sizes_prop, page_sizes_prop_size)));
>      }
>  
> -    spapr_populate_pa_features(spapr, cpu, fdt, offset, false);
> +    spapr_populate_pa_features(spapr, cpu, fdt, offset);
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
>                             cs->cpu_index / vcpus_per_socket)));
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 23e4bdb829..3d3a67149a 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1765,8 +1765,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>              exit(EXIT_FAILURE);
>          }
>      }
> -    spapr->cas_legacy_guest_workaround = !spapr_ovec_test(ov1_guest,
> -                                                          OV1_PPC_3_00);
> +    spapr->cas_pre_isa3_guest = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00);
>      spapr_ovec_cleanup(ov1_guest);
>      if (!spapr->cas_reboot) {
>          /* If spapr_machine_reset() did not set up a HPT but one is necessary
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 03111fd55b..dfec8e8e76 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -175,7 +175,7 @@ struct SpaprMachineState {
>  
>      /* ibm,client-architecture-support option negotiation */
>      bool cas_reboot;
> -    bool cas_legacy_guest_workaround;
> +    bool cas_pre_isa3_guest;
>      SpaprOptionVector *ov5;         /* QEMU-supported option vectors */
>      SpaprOptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
>      uint32_t max_compat_pvr;
> 

-- 
Alexey


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order David Gibson
@ 2019-09-11  7:40   ` Alexey Kardashevskiy
  2019-09-11  7:51     ` David Gibson
  2019-09-11  7:54   ` Cédric Le Goater
  1 sibling, 1 reply; 25+ messages in thread
From: Alexey Kardashevskiy @ 2019-09-11  7:40 UTC (permalink / raw)
  To: David Gibson, qemu-devel, qemu-ppc; +Cc: lvivier, philmd, groug, clg



On 11/09/2019 14:04, David Gibson wrote:
> We've made several changes in the past to the machine reset order to fix
> specific problems.  However, we've ended up with an order that doesn't make
> a lot of logical sense.  This is an attempt to rectify this.
> 
> First we reset global CAS options, since that should depend on nothing
> else.  Then we reset the CPUs, which shouldn't depend on external devices.
> Then the irq subsystem, then the bulk of devices (which might rely on
> irqs).  Finally we set up the entry state ready for boot, which could
> potentially rely on a bunch of other things.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Breaks console on P8 and asserts on rebooting a P9 guest.



> ---
>  hw/ppc/spapr.c | 47 +++++++++++++++++++++++++----------------------
>  1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5a919a6cc1..1560a11738 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1724,6 +1724,28 @@ static void spapr_machine_reset(MachineState *machine)
>      void *fdt;
>      int rc;
>  
> +    /*
> +     * If this reset wasn't generated by CAS, we should reset our
> +     * negotiated options and start from scratch
> +     */
> +    if (!spapr->cas_reboot) {
> +        spapr_ovec_cleanup(spapr->ov5_cas);
> +        spapr->ov5_cas = spapr_ovec_new();
> +
> +        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
> +    }
> +
> +    /*
> +     * There is no CAS under qtest. Simulate one to please the code that
> +     * depends on spapr->ov5_cas. This is especially needed to test device
> +     * unplug, so we do that before resetting the DRCs.
> +     */
> +    if (qtest_enabled()) {
> +        spapr_ovec_cleanup(spapr->ov5_cas);
> +        spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
> +    }
> +
> +    /* Reset the CPUs */
>      spapr_caps_apply(spapr);
>  
>      first_ppc_cpu = POWERPC_CPU(first_cpu);
> @@ -1741,34 +1763,15 @@ static void spapr_machine_reset(MachineState *machine)
>          spapr_setup_hpt_and_vrma(spapr);
>      }
>  
> -    qemu_devices_reset();
> -
> -    /*
> -     * If this reset wasn't generated by CAS, we should reset our
> -     * negotiated options and start from scratch
> -     */
> -    if (!spapr->cas_reboot) {
> -        spapr_ovec_cleanup(spapr->ov5_cas);
> -        spapr->ov5_cas = spapr_ovec_new();
> -
> -        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
> -    }
> -
> +    /* Reset IRQ subsystem */
>      /*
>       * This is fixing some of the default configuration of the XIVE
>       * devices. To be called after the reset of the machine devices.
>       */
>      spapr_irq_reset(spapr, &error_fatal);
>  
> -    /*
> -     * There is no CAS under qtest. Simulate one to please the code that
> -     * depends on spapr->ov5_cas. This is especially needed to test device
> -     * unplug, so we do that before resetting the DRCs.
> -     */
> -    if (qtest_enabled()) {
> -        spapr_ovec_cleanup(spapr->ov5_cas);
> -        spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
> -    }
> +    /* Reset other devices */
> +    qemu_devices_reset();
>  
>      /* DRC reset may cause a device to be unplugged. This will cause troubles
>       * if this device is used by another device (eg, a running vhost backend
> 

-- 
Alexey


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init David Gibson
  2019-09-11  7:28   ` Cédric Le Goater
  2019-09-11  7:33   ` Greg Kurz
@ 2019-09-11  7:41   ` Alexey Kardashevskiy
  2 siblings, 0 replies; 25+ messages in thread
From: Alexey Kardashevskiy @ 2019-09-11  7:41 UTC (permalink / raw)
  To: David Gibson, qemu-devel, qemu-ppc; +Cc: lvivier, philmd, groug, clg



On 11/09/2019 14:04, David Gibson wrote:
> The number of NUMA nodes in the system is fixed from the command line.
> Therefore, there's no need to recalculate it at reset time, and we can
> determine the special gpu_numa_id value used for NVLink2 devices at init
> time.
> 
> This simplifies the reset path a bit which will make further improvements
> easier.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>


> ---
>  hw/ppc/spapr.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c551001f86..e03e874d94 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1737,16 +1737,6 @@ static void spapr_machine_reset(MachineState *machine)
>          spapr_setup_hpt_and_vrma(spapr);
>      }
>  
> -    /*
> -     * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
> -     * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
> -     * called from vPHB reset handler so we initialize the counter here.
> -     * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
> -     * must be equally distant from any other node.
> -     * The final value of spapr->gpu_numa_id is going to be written to
> -     * max-associativity-domains in spapr_build_fdt().
> -     */
> -    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
>      qemu_devices_reset();
>  
>      /*
> @@ -2885,6 +2875,17 @@ static void spapr_machine_init(MachineState *machine)
>  
>      }
>  
> +    /*
> +     * NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
> +     * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
> +     * called from vPHB reset handler so we initialize the counter here.
> +     * If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
> +     * must be equally distant from any other node.
> +     * The final value of spapr->gpu_numa_id is going to be written to
> +     * max-associativity-domains in spapr_build_fdt().
> +     */
> +    spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
> +
>      if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
>          ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
>                                spapr->max_compat_pvr)) {
> 

-- 
Alexey


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order
  2019-09-11  7:40   ` Alexey Kardashevskiy
@ 2019-09-11  7:51     ` David Gibson
  0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2019-09-11  7:51 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: lvivier, qemu-devel, groug, qemu-ppc, clg, philmd

[-- Attachment #1: Type: text/plain, Size: 3979 bytes --]

On Wed, Sep 11, 2019 at 05:40:58PM +1000, Alexey Kardashevskiy wrote:
> 
> 
> On 11/09/2019 14:04, David Gibson wrote:
> > We've made several changes in the past to the machine reset order to fix
> > specific problems.  However, we've ended up with an order that doesn't make
> > a lot of logical sense.  This is an attempt to rectify this.
> > 
> > First we reset global CAS options, since that should depend on nothing
> > else.  Then we reset the CPUs, which shouldn't depend on external devices.
> > Then the irq subsystem, then the bulk of devices (which might rely on
> > irqs).  Finally we set up the entry state ready for boot, which could
> > potentially rely on a bunch of other things.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> 
> Breaks console on P8 and asserts on rebooting a P9 guest.

Yeah, I jumped the gun on this one - I need to rethink and test more thoroughly.

> 
> 
> 
> > ---
> >  hw/ppc/spapr.c | 47 +++++++++++++++++++++++++----------------------
> >  1 file changed, 25 insertions(+), 22 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 5a919a6cc1..1560a11738 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1724,6 +1724,28 @@ static void spapr_machine_reset(MachineState *machine)
> >      void *fdt;
> >      int rc;
> >  
> > +    /*
> > +     * If this reset wasn't generated by CAS, we should reset our
> > +     * negotiated options and start from scratch
> > +     */
> > +    if (!spapr->cas_reboot) {
> > +        spapr_ovec_cleanup(spapr->ov5_cas);
> > +        spapr->ov5_cas = spapr_ovec_new();
> > +
> > +        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
> > +    }
> > +
> > +    /*
> > +     * There is no CAS under qtest. Simulate one to please the code that
> > +     * depends on spapr->ov5_cas. This is especially needed to test device
> > +     * unplug, so we do that before resetting the DRCs.
> > +     */
> > +    if (qtest_enabled()) {
> > +        spapr_ovec_cleanup(spapr->ov5_cas);
> > +        spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
> > +    }
> > +
> > +    /* Reset the CPUs */
> >      spapr_caps_apply(spapr);
> >  
> >      first_ppc_cpu = POWERPC_CPU(first_cpu);
> > @@ -1741,34 +1763,15 @@ static void spapr_machine_reset(MachineState *machine)
> >          spapr_setup_hpt_and_vrma(spapr);
> >      }
> >  
> > -    qemu_devices_reset();
> > -
> > -    /*
> > -     * If this reset wasn't generated by CAS, we should reset our
> > -     * negotiated options and start from scratch
> > -     */
> > -    if (!spapr->cas_reboot) {
> > -        spapr_ovec_cleanup(spapr->ov5_cas);
> > -        spapr->ov5_cas = spapr_ovec_new();
> > -
> > -        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
> > -    }
> > -
> > +    /* Reset IRQ subsystem */
> >      /*
> >       * This is fixing some of the default configuration of the XIVE
> >       * devices. To be called after the reset of the machine devices.
> >       */
> >      spapr_irq_reset(spapr, &error_fatal);
> >  
> > -    /*
> > -     * There is no CAS under qtest. Simulate one to please the code that
> > -     * depends on spapr->ov5_cas. This is especially needed to test device
> > -     * unplug, so we do that before resetting the DRCs.
> > -     */
> > -    if (qtest_enabled()) {
> > -        spapr_ovec_cleanup(spapr->ov5_cas);
> > -        spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
> > -    }
> > +    /* Reset other devices */
> > +    qemu_devices_reset();
> >  
> >      /* DRC reset may cause a device to be unplugged. This will cause troubles
> >       * if this device is used by another device (eg, a running vhost backend
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order David Gibson
  2019-09-11  7:40   ` Alexey Kardashevskiy
@ 2019-09-11  7:54   ` Cédric Le Goater
  1 sibling, 0 replies; 25+ messages in thread
From: Cédric Le Goater @ 2019-09-11  7:54 UTC (permalink / raw)
  To: David Gibson, qemu-devel, qemu-ppc; +Cc: aik, lvivier, philmd, groug

On 11/09/2019 06:04, David Gibson wrote:
> We've made several changes in the past to the machine reset order to fix
> specific problems.  However, we've ended up with an order that doesn't make
> a lot of logical sense.  This is an attempt to rectify this.

There are some more problems though. See below.

> 
> First we reset global CAS options, since that should depend on nothing
> else.  Then we reset the CPUs, which shouldn't depend on external devices.
> Then the irq subsystem, then the bulk of devices (which might rely on
> irqs).  Finally we set up the entry state ready for boot, which could
> potentially rely on a bunch of other things.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 47 +++++++++++++++++++++++++----------------------
>  1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5a919a6cc1..1560a11738 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1724,6 +1724,28 @@ static void spapr_machine_reset(MachineState *machine)
>      void *fdt;
>      int rc;
>  
> +    /*
> +     * If this reset wasn't generated by CAS, we should reset our
> +     * negotiated options and start from scratch
> +     */
> +    if (!spapr->cas_reboot) {
> +        spapr_ovec_cleanup(spapr->ov5_cas);
> +        spapr->ov5_cas = spapr_ovec_new();
> +
> +        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
> +    }
> +
> +    /*
> +     * There is no CAS under qtest. Simulate one to please the code that
> +     * depends on spapr->ov5_cas. This is especially needed to test device
> +     * unplug, so we do that before resetting the DRCs.
> +     */
> +    if (qtest_enabled()) {
> +        spapr_ovec_cleanup(spapr->ov5_cas);
> +        spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
> +    }
> +
> +    /* Reset the CPUs */
>      spapr_caps_apply(spapr);
>  
>      first_ppc_cpu = POWERPC_CPU(first_cpu);
> @@ -1741,34 +1763,15 @@ static void spapr_machine_reset(MachineState *machine)
>          spapr_setup_hpt_and_vrma(spapr);
>      }
>  
> -    qemu_devices_reset();
> -
> -    /*
> -     * If this reset wasn't generated by CAS, we should reset our
> -     * negotiated options and start from scratch
> -     */
> -    if (!spapr->cas_reboot) {
> -        spapr_ovec_cleanup(spapr->ov5_cas);
> -        spapr->ov5_cas = spapr_ovec_new();
> -
> -        ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
> -    }
> -
> +    /* Reset IRQ subsystem */
>      /*
>       * This is fixing some of the default configuration of the XIVE
>       * devices. To be called after the reset of the machine devices.
>       */
>      spapr_irq_reset(spapr, &error_fatal);

spapr_irq_reset() is now called before qemu_devices_reset(). So it will
break the XIVE emulated model.

KVM P8 guests are also broken : 

 qemu-system-ppc64: kernel_irqchip requested but unavailable: Unable to restore KVM interrupt controller state (0x0) for CPU 0: Invalid argument

Something wrong in kvmppc_xics_set_icp(). I need to look closer.

and TCG P9 guests still do the reset after CAS. 

C.

>  
> -    /*
> -     * There is no CAS under qtest. Simulate one to please the code that
> -     * depends on spapr->ov5_cas. This is especially needed to test device
> -     * unplug, so we do that before resetting the DRCs.
> -     */
> -    if (qtest_enabled()) {
> -        spapr_ovec_cleanup(spapr->ov5_cas);
> -        spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
> -    }
> +    /* Reset other devices */
> +    qemu_devices_reset();
>  
>      /* DRC reset may cause a device to be unplugged. This will cause troubles
>       * if this device is used by another device (eg, a running vhost backend
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 4/7] spapr: Skip leading zeroes from memory@ DT node names
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 4/7] spapr: Skip leading zeroes from memory@ DT node names David Gibson
@ 2019-09-11  8:01   ` Greg Kurz
  0 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2019-09-11  8:01 UTC (permalink / raw)
  To: David Gibson; +Cc: lvivier, aik, qemu-devel, qemu-ppc, clg, philmd

On Wed, 11 Sep 2019 14:04:49 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> From: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> The device tree build by QEMU at the machine reset time is used by SLOF
> to build its internal device tree but the node names are not preserved
> exactly so when QEMU provides a device tree update in response to H_CAS,
> it might become tricky to match a node from the update blob to
> the actual node in SLOF.
> 
> This removed leading zeroes from "memory@" nodes and makes
> the DTC checker happy.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d93dacd483..d072c2aa3d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -386,7 +386,7 @@ static int spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start,
>      mem_reg_property[0] = cpu_to_be64(start);
>      mem_reg_property[1] = cpu_to_be64(size);
>  
> -    sprintf(mem_name, "memory@" TARGET_FMT_lx, start);
> +    sprintf(mem_name, "memory@%" HWADDR_PRIx, start);
>      off = fdt_add_subnode(fdt, 0, mem_name);
>      _FDT(off);
>      _FDT((fdt_setprop_string(fdt, off, "device_type", "memory")));



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append David Gibson
@ 2019-09-11  8:46   ` Greg Kurz
  2019-09-12  1:59     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2019-09-11  8:46 UTC (permalink / raw)
  To: David Gibson; +Cc: lvivier, aik, qemu-devel, qemu-ppc, clg, philmd

On Wed, 11 Sep 2019 14:04:50 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> From: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> We are going to use spapr_build_fdt() for the boot time FDT and as an
> update for SLOF during handling of H_CAS. SLOF will apply all properties
> from the QEMU's FDT which is usually ok unless there are properties
> changed by grub or guest kernel. The properties are:
> bootargs, linux,initrd-start, linux,initrd-end, linux,stdout-path,
> linux,rtas-base, linux,rtas-entry. Resetting those during CAS will most
> likely cause grub failure.
> 

s/Resetting/Clearing ? They still get reset to the initial setup if "-kernel"
and "-initrd" were passed, but it is okay since neither grub, nor the guest
kernel is supposed to change them in this case, correct ?

> This only creates such properties if we are booting with "-kernel" and
> "-initrd" so they won't get included into the DT update blob and

so they won't get included {if we're not booting with "-kernel" ...}

> therefore the guest is more likely to boot successfully.
> 

Maybe rephrase like:

Don't create such properties if we're booting without "-kernel" and
"-initrd" ...

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d072c2aa3d..d18744268f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1177,11 +1177,16 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
>  
>      _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
>  
> -    _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline));
> -    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
> -                          spapr->initrd_base));
> -    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
> -                          spapr->initrd_base + spapr->initrd_size));
> +    if (machine->kernel_cmdline && machine->kernel_cmdline[0]) {

machine->kernel_cmdline cannot be NULL.

From vl.c:

    if (!kernel_cmdline) {
        kernel_cmdline = "";
        current_machine->kernel_cmdline = (char *)kernel_cmdline;
    }

Also this doesn't check if we're booting with -kernel but rather
that we're booting with -append ${some_not_empty_string}... what
about checking spapr->kernel_size, pretty much like you do for
the initrd ?

> +        _FDT(fdt_setprop_string(fdt, chosen, "bootargs",
> +                                machine->kernel_cmdline));
> +    }
> +    if (spapr->initrd_size) {
> +        _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
> +                              spapr->initrd_base));
> +        _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
> +                              spapr->initrd_base + spapr->initrd_size));
> +    }
>  
>      if (spapr->kernel_size) {
>          uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob
  2019-09-11  4:04 ` [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob David Gibson
@ 2019-09-11  9:16   ` Greg Kurz
  2019-09-12  1:50     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2019-09-11  9:16 UTC (permalink / raw)
  To: David Gibson; +Cc: lvivier, aik, qemu-devel, qemu-ppc, clg, philmd

On Wed, 11 Sep 2019 14:04:51 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> From: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> SLOF implements one itself so let's remove it from QEMU. It is one less
> image and simpler setup as the RTAS blob never stays in its initial place
> anyway as the guest OS always decides where to put it.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  MAINTAINERS                     |   2 --
>  Makefile                        |   2 +-
>  configure                       |   6 +----
>  hw/ppc/spapr.c                  |  32 ++-----------------------
>  hw/ppc/spapr_rtas.c             |  41 --------------------------------
>  include/hw/ppc/spapr.h          |   2 --
>  pc-bios/spapr-rtas.bin          | Bin 20 -> 0 bytes
>  pc-bios/spapr-rtas/Makefile     |  27 ---------------------
>  pc-bios/spapr-rtas/spapr-rtas.S |  37 ----------------------------
>  9 files changed, 4 insertions(+), 145 deletions(-)
>  delete mode 100644 pc-bios/spapr-rtas.bin
>  delete mode 100644 pc-bios/spapr-rtas/Makefile
>  delete mode 100644 pc-bios/spapr-rtas/spapr-rtas.S
> 

Nice diffstat :)

But pwclient fails to apply it :(

[greg@bahia qemu-spapr]$ pwclient git-am 1160642
Applying patch #1160642 using 'git am'
Description: [6/7] spapr: Stop providing RTAS blob
Applying: spapr: Stop providing RTAS blob
error: cannot apply binary patch to 'pc-bios/spapr-rtas.bin' without full index line
error: pc-bios/spapr-rtas.bin: patch does not apply
Patch failed at 0001 spapr: Stop providing RTAS blob
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
'git am' failed with exit status 128

and

[greg@bahia qemu-spapr]$ git am --show-current-patch | patch -p1 --merge 
patching file MAINTAINERS
patching file Makefile
patching file configure
patching file hw/ppc/spapr.c
patching file hw/ppc/spapr_rtas.c
patching file include/hw/ppc/spapr.h
patching file pc-bios/spapr-rtas.bin
Not deleting file pc-bios/spapr-rtas.bin as content differs from patch

Not sure what's happening here...

patching file pc-bios/spapr-rtas/Makefile
patching file pc-bios/spapr-rtas/spapr-rtas.S

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50eaf005f4..9823f40213 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1077,8 +1077,6 @@ F: hw/*/spapr*
>  F: include/hw/*/spapr*
>  F: hw/*/xics*
>  F: include/hw/*/xics*
> -F: pc-bios/spapr-rtas/*
> -F: pc-bios/spapr-rtas.bin
>  F: pc-bios/slof.bin
>  F: docs/specs/ppc-spapr-hcalls.txt
>  F: docs/specs/ppc-spapr-hotplug.txt
> diff --git a/Makefile b/Makefile
> index ae17a83067..4637f95371 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -764,7 +764,7 @@ qemu-nsis.bmp \
>  bamboo.dtb canyonlands.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
>  multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin \
>  s390-ccw.img s390-netboot.img \
> -spapr-rtas.bin slof.bin skiboot.lid \
> +slof.bin skiboot.lid \
>  palcode-clipper \
>  u-boot.e500 u-boot-sam460-20100605.bin \
>  qemu_vga.ndrv \
> diff --git a/configure b/configure
> index 95134c0180..b79d38592b 100755
> --- a/configure
> +++ b/configure
> @@ -6211,9 +6211,6 @@ if { test "$cpu" = "i386" || test "$cpu" = "x86_64"; } && \
>          fi
>      done
>  fi
> -if test "$ARCH" = "ppc64" && test "$targetos" != "Darwin" ; then
> -  roms="$roms spapr-rtas"
> -fi
>  
>  # Only build s390-ccw bios if we're on s390x and the compiler has -march=z900
>  if test "$cpu" = "s390x" ; then
> @@ -7930,14 +7927,13 @@ fi
>  DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
>  DIRS="$DIRS tests/fp tests/qgraph"
>  DIRS="$DIRS docs docs/interop fsdev scsi"
> -DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
> +DIRS="$DIRS pc-bios/optionrom pc-bios/s390-ccw"
>  DIRS="$DIRS roms/seabios roms/vgabios"
>  LINKS="Makefile tests/tcg/Makefile"
>  LINKS="$LINKS tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
>  LINKS="$LINKS tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile"
>  LINKS="$LINKS tests/fp/Makefile"
>  LINKS="$LINKS pc-bios/optionrom/Makefile pc-bios/keymaps"
> -LINKS="$LINKS pc-bios/spapr-rtas/Makefile"
>  LINKS="$LINKS pc-bios/s390-ccw/Makefile"
>  LINKS="$LINKS roms/seabios/Makefile roms/vgabios/Makefile"
>  LINKS="$LINKS pc-bios/qemu-icon.bmp"
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d18744268f..5a919a6cc1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -94,7 +94,6 @@
>   * We load our kernel at 4M, leaving space for SLOF initial image
>   */
>  #define FDT_MAX_SIZE            0x100000
> -#define RTAS_MAX_SIZE           0x10000
>  #define RTAS_MAX_ADDR           0x80000000 /* RTAS must stay below that */
>  #define FW_MAX_SIZE             0x400000
>  #define FW_FILE_NAME            "slof.bin"
> @@ -1721,8 +1720,7 @@ static void spapr_machine_reset(MachineState *machine)
>  {
>      SpaprMachineState *spapr = SPAPR_MACHINE(machine);
>      PowerPCCPU *first_ppc_cpu;
> -    uint32_t rtas_limit;
> -    hwaddr rtas_addr, fdt_addr;
> +    hwaddr fdt_addr;
>      void *fdt;
>      int rc;
>  
> @@ -1786,14 +1784,10 @@ static void spapr_machine_reset(MachineState *machine)
>       * or just below 2GB, whichever is lower, so that it can be
>       * processed with 32-bit real mode code if necessary
>       */
> -    rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR);
> -    rtas_addr = rtas_limit - RTAS_MAX_SIZE;
> -    fdt_addr = rtas_addr - FDT_MAX_SIZE;
> +    fdt_addr = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FDT_MAX_SIZE;
>  
>      fdt = spapr_build_fdt(spapr);
>  
> -    spapr_load_rtas(spapr, fdt, rtas_addr);
> -
>      rc = fdt_pack(fdt);
>  
>      /* Should only fail if we've built a corrupted tree */
> @@ -2953,28 +2947,6 @@ static void spapr_machine_init(MachineState *machine)
>          spapr_create_lmb_dr_connectors(spapr);
>      }
>  
> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
> -    if (!filename) {
> -        error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
> -        exit(1);
> -    }
> -    spapr->rtas_size = get_image_size(filename);
> -    if (spapr->rtas_size < 0) {
> -        error_report("Could not get size of LPAR rtas '%s'", filename);
> -        exit(1);
> -    }
> -    spapr->rtas_blob = g_malloc(spapr->rtas_size);
> -    if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
> -        error_report("Could not load LPAR rtas '%s'", filename);
> -        exit(1);
> -    }
> -    if (spapr->rtas_size > RTAS_MAX_SIZE) {
> -        error_report("RTAS too big ! 0x%zx bytes (max is 0x%x)",
> -                     (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
> -        exit(1);
> -    }
> -    g_free(filename);
> -
>      /* Set up RTAS event infrastructure */
>      spapr_events_init(spapr);
>  
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index bee3835214..8d8d8cdfcb 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -477,47 +477,6 @@ void spapr_dt_rtas_tokens(void *fdt, int rtas)
>      }
>  }
>  
> -void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr)
> -{
> -    int rtas_node;
> -    int ret;
> -
> -    /* Copy RTAS blob into guest RAM */
> -    cpu_physical_memory_write(addr, spapr->rtas_blob, spapr->rtas_size);
> -
> -    ret = fdt_add_mem_rsv(fdt, addr, spapr->rtas_size);
> -    if (ret < 0) {
> -        error_report("Couldn't add RTAS reserve entry: %s",
> -                     fdt_strerror(ret));
> -        exit(1);
> -    }
> -
> -    /* Update the device tree with the blob's location */
> -    rtas_node = fdt_path_offset(fdt, "/rtas");
> -    assert(rtas_node >= 0);
> -
> -    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-base", addr);
> -    if (ret < 0) {
> -        error_report("Couldn't add linux,rtas-base property: %s",
> -                     fdt_strerror(ret));
> -        exit(1);
> -    }
> -
> -    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-entry", addr);
> -    if (ret < 0) {
> -        error_report("Couldn't add linux,rtas-entry property: %s",
> -                     fdt_strerror(ret));
> -        exit(1);
> -    }
> -
> -    ret = fdt_setprop_cell(fdt, rtas_node, "rtas-size", spapr->rtas_size);
> -    if (ret < 0) {
> -        error_report("Couldn't add rtas-size property: %s",
> -                     fdt_strerror(ret));
> -        exit(1);
> -    }
> -}
> -
>  static void core_rtas_register_types(void)
>  {
>      spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index dfec8e8e76..cbd1a4c9f3 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -154,8 +154,6 @@ struct SpaprMachineState {
>  
>      hwaddr rma_size;
>      int vrma_adjust;
> -    ssize_t rtas_size;
> -    void *rtas_blob;
>      uint32_t fdt_size;
>      uint32_t fdt_initial_size;
>      void *fdt_blob;
> diff --git a/pc-bios/spapr-rtas.bin b/pc-bios/spapr-rtas.bin
> deleted file mode 100644
> index fc24c8ed8b..0000000000
> Binary files a/pc-bios/spapr-rtas.bin and /dev/null differ
> diff --git a/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
> deleted file mode 100644
> index 4b9bb12306..0000000000
> --- a/pc-bios/spapr-rtas/Makefile
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -all: build-all
> -# Dummy command so that make thinks it has done something
> -	@true
> -
> -include ../../config-host.mak
> -include $(SRC_PATH)/rules.mak
> -
> -$(call set-vpath, $(SRC_PATH)/pc-bios/spapr-rtas)
> -
> -.PHONY : all clean build-all
> -
> -#CFLAGS += -I$(SRC_PATH)
> -#QEMU_CFLAGS = $(CFLAGS)
> -
> -build-all: spapr-rtas.bin
> -
> -%.o: %.S
> -	$(call quiet-command,$(CCAS) -mbig -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
> -
> -%.img: %.o
> -	$(call quiet-command,$(CC) -nostdlib -mbig -o $@ $<,"Building","$(TARGET_DIR)$@")
> -
> -%.bin: %.img
> -	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"Building","$(TARGET_DIR)$@")
> -
> -clean:
> -	rm -f *.o *.d *.img *.bin *~
> diff --git a/pc-bios/spapr-rtas/spapr-rtas.S b/pc-bios/spapr-rtas/spapr-rtas.S
> deleted file mode 100644
> index 903bec2150..0000000000
> --- a/pc-bios/spapr-rtas/spapr-rtas.S
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -/*
> - * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
> - *
> - * Trivial in-partition RTAS implementation, based on a hypercall
> - *
> - * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - *
> - */
> -
> -#define KVMPPC_HCALL_BASE       0xf000
> -#define KVMPPC_H_RTAS           (KVMPPC_HCALL_BASE + 0x0)
> -
> -.globl	_start
> -_start:
> -	mr	4,3
> -	lis	3,KVMPPC_H_RTAS@h
> -	ori	3,3,KVMPPC_H_RTAS@l
> -	sc	1
> -	blr



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob
  2019-09-11  9:16   ` Greg Kurz
@ 2019-09-12  1:50     ` Alexey Kardashevskiy
  2019-09-12  7:20       ` Greg Kurz
  0 siblings, 1 reply; 25+ messages in thread
From: Alexey Kardashevskiy @ 2019-09-12  1:50 UTC (permalink / raw)
  To: Greg Kurz, David Gibson; +Cc: lvivier, philmd, qemu-ppc, qemu-devel, clg



On 11/09/2019 19:16, Greg Kurz wrote:
> On Wed, 11 Sep 2019 14:04:51 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
>> From: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> SLOF implements one itself so let's remove it from QEMU. It is one less
>> image and simpler setup as the RTAS blob never stays in its initial place
>> anyway as the guest OS always decides where to put it.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  MAINTAINERS                     |   2 --
>>  Makefile                        |   2 +-
>>  configure                       |   6 +----
>>  hw/ppc/spapr.c                  |  32 ++-----------------------
>>  hw/ppc/spapr_rtas.c             |  41 --------------------------------
>>  include/hw/ppc/spapr.h          |   2 --
>>  pc-bios/spapr-rtas.bin          | Bin 20 -> 0 bytes
>>  pc-bios/spapr-rtas/Makefile     |  27 ---------------------
>>  pc-bios/spapr-rtas/spapr-rtas.S |  37 ----------------------------
>>  9 files changed, 4 insertions(+), 145 deletions(-)
>>  delete mode 100644 pc-bios/spapr-rtas.bin
>>  delete mode 100644 pc-bios/spapr-rtas/Makefile
>>  delete mode 100644 pc-bios/spapr-rtas/spapr-rtas.S
>>
> 
> Nice diffstat :)
> 
> But pwclient fails to apply it :(
> 
> [greg@bahia qemu-spapr]$ pwclient git-am 1160642
> Applying patch #1160642 using 'git am'
> Description: [6/7] spapr: Stop providing RTAS blob
> Applying: spapr: Stop providing RTAS blob
> error: cannot apply binary patch to 'pc-bios/spapr-rtas.bin' without full index line


Some git feature/bug with removing binaries:

https://stackoverflow.com/questions/17152171/git-cannot-apply-binary-patch-without-full-index-line


David posted with this:
===
diff --git a/pc-bios/spapr-rtas.bin b/pc-bios/spapr-rtas.bin
deleted file mode 100644
index fc24c8ed8b..0000000000
Binary files a/pc-bios/spapr-rtas.bin and /dev/null differ
===

And my patch has a bigger chunk:

git format-patch -1 --stdout 1a5efb9283c2
(there is no additional flag needed to my git 2.17.1):

===
diff --git a/pc-bios/spapr-rtas.bin b/pc-bios/spapr-rtas.bin
deleted file mode 100644
index
fc24c8ed8b92a3a441aed6e2bd013b2ccece9229..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 20
bcmb<Pk*=^wU|>i{{=neEz@X&Uz@PvCJTV0q
===

I do not know why are these different.

Thy this one:
https://patchwork.ozlabs.org/patch/1132443/



> error: pc-bios/spapr-rtas.bin: patch does not apply
> Patch failed at 0001 spapr: Stop providing RTAS blob
> hint: Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 'git am' failed with exit status 128
> 
> and
> 
> [greg@bahia qemu-spapr]$ git am --show-current-patch | patch -p1 --merge 
> patching file MAINTAINERS
> patching file Makefile
> patching file configure
> patching file hw/ppc/spapr.c
> patching file hw/ppc/spapr_rtas.c
> patching file include/hw/ppc/spapr.h
> patching file pc-bios/spapr-rtas.bin
> Not deleting file pc-bios/spapr-rtas.bin as content differs from patch
> 
> Not sure what's happening here...
> 
> patching file pc-bios/spapr-rtas/Makefile
> patching file pc-bios/spapr-rtas/spapr-rtas.S
> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 50eaf005f4..9823f40213 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1077,8 +1077,6 @@ F: hw/*/spapr*
>>  F: include/hw/*/spapr*
>>  F: hw/*/xics*
>>  F: include/hw/*/xics*
>> -F: pc-bios/spapr-rtas/*
>> -F: pc-bios/spapr-rtas.bin
>>  F: pc-bios/slof.bin
>>  F: docs/specs/ppc-spapr-hcalls.txt
>>  F: docs/specs/ppc-spapr-hotplug.txt
>> diff --git a/Makefile b/Makefile
>> index ae17a83067..4637f95371 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -764,7 +764,7 @@ qemu-nsis.bmp \
>>  bamboo.dtb canyonlands.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
>>  multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin \
>>  s390-ccw.img s390-netboot.img \
>> -spapr-rtas.bin slof.bin skiboot.lid \
>> +slof.bin skiboot.lid \
>>  palcode-clipper \
>>  u-boot.e500 u-boot-sam460-20100605.bin \
>>  qemu_vga.ndrv \
>> diff --git a/configure b/configure
>> index 95134c0180..b79d38592b 100755
>> --- a/configure
>> +++ b/configure
>> @@ -6211,9 +6211,6 @@ if { test "$cpu" = "i386" || test "$cpu" = "x86_64"; } && \
>>          fi
>>      done
>>  fi
>> -if test "$ARCH" = "ppc64" && test "$targetos" != "Darwin" ; then
>> -  roms="$roms spapr-rtas"
>> -fi
>>  
>>  # Only build s390-ccw bios if we're on s390x and the compiler has -march=z900
>>  if test "$cpu" = "s390x" ; then
>> @@ -7930,14 +7927,13 @@ fi
>>  DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
>>  DIRS="$DIRS tests/fp tests/qgraph"
>>  DIRS="$DIRS docs docs/interop fsdev scsi"
>> -DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
>> +DIRS="$DIRS pc-bios/optionrom pc-bios/s390-ccw"
>>  DIRS="$DIRS roms/seabios roms/vgabios"
>>  LINKS="Makefile tests/tcg/Makefile"
>>  LINKS="$LINKS tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
>>  LINKS="$LINKS tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile"
>>  LINKS="$LINKS tests/fp/Makefile"
>>  LINKS="$LINKS pc-bios/optionrom/Makefile pc-bios/keymaps"
>> -LINKS="$LINKS pc-bios/spapr-rtas/Makefile"
>>  LINKS="$LINKS pc-bios/s390-ccw/Makefile"
>>  LINKS="$LINKS roms/seabios/Makefile roms/vgabios/Makefile"
>>  LINKS="$LINKS pc-bios/qemu-icon.bmp"
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index d18744268f..5a919a6cc1 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -94,7 +94,6 @@
>>   * We load our kernel at 4M, leaving space for SLOF initial image
>>   */
>>  #define FDT_MAX_SIZE            0x100000
>> -#define RTAS_MAX_SIZE           0x10000
>>  #define RTAS_MAX_ADDR           0x80000000 /* RTAS must stay below that */
>>  #define FW_MAX_SIZE             0x400000
>>  #define FW_FILE_NAME            "slof.bin"
>> @@ -1721,8 +1720,7 @@ static void spapr_machine_reset(MachineState *machine)
>>  {
>>      SpaprMachineState *spapr = SPAPR_MACHINE(machine);
>>      PowerPCCPU *first_ppc_cpu;
>> -    uint32_t rtas_limit;
>> -    hwaddr rtas_addr, fdt_addr;
>> +    hwaddr fdt_addr;
>>      void *fdt;
>>      int rc;
>>  
>> @@ -1786,14 +1784,10 @@ static void spapr_machine_reset(MachineState *machine)
>>       * or just below 2GB, whichever is lower, so that it can be
>>       * processed with 32-bit real mode code if necessary
>>       */
>> -    rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR);
>> -    rtas_addr = rtas_limit - RTAS_MAX_SIZE;
>> -    fdt_addr = rtas_addr - FDT_MAX_SIZE;
>> +    fdt_addr = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FDT_MAX_SIZE;
>>  
>>      fdt = spapr_build_fdt(spapr);
>>  
>> -    spapr_load_rtas(spapr, fdt, rtas_addr);
>> -
>>      rc = fdt_pack(fdt);
>>  
>>      /* Should only fail if we've built a corrupted tree */
>> @@ -2953,28 +2947,6 @@ static void spapr_machine_init(MachineState *machine)
>>          spapr_create_lmb_dr_connectors(spapr);
>>      }
>>  
>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
>> -    if (!filename) {
>> -        error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
>> -        exit(1);
>> -    }
>> -    spapr->rtas_size = get_image_size(filename);
>> -    if (spapr->rtas_size < 0) {
>> -        error_report("Could not get size of LPAR rtas '%s'", filename);
>> -        exit(1);
>> -    }
>> -    spapr->rtas_blob = g_malloc(spapr->rtas_size);
>> -    if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
>> -        error_report("Could not load LPAR rtas '%s'", filename);
>> -        exit(1);
>> -    }
>> -    if (spapr->rtas_size > RTAS_MAX_SIZE) {
>> -        error_report("RTAS too big ! 0x%zx bytes (max is 0x%x)",
>> -                     (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
>> -        exit(1);
>> -    }
>> -    g_free(filename);
>> -
>>      /* Set up RTAS event infrastructure */
>>      spapr_events_init(spapr);
>>  
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index bee3835214..8d8d8cdfcb 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -477,47 +477,6 @@ void spapr_dt_rtas_tokens(void *fdt, int rtas)
>>      }
>>  }
>>  
>> -void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr)
>> -{
>> -    int rtas_node;
>> -    int ret;
>> -
>> -    /* Copy RTAS blob into guest RAM */
>> -    cpu_physical_memory_write(addr, spapr->rtas_blob, spapr->rtas_size);
>> -
>> -    ret = fdt_add_mem_rsv(fdt, addr, spapr->rtas_size);
>> -    if (ret < 0) {
>> -        error_report("Couldn't add RTAS reserve entry: %s",
>> -                     fdt_strerror(ret));
>> -        exit(1);
>> -    }
>> -
>> -    /* Update the device tree with the blob's location */
>> -    rtas_node = fdt_path_offset(fdt, "/rtas");
>> -    assert(rtas_node >= 0);
>> -
>> -    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-base", addr);
>> -    if (ret < 0) {
>> -        error_report("Couldn't add linux,rtas-base property: %s",
>> -                     fdt_strerror(ret));
>> -        exit(1);
>> -    }
>> -
>> -    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-entry", addr);
>> -    if (ret < 0) {
>> -        error_report("Couldn't add linux,rtas-entry property: %s",
>> -                     fdt_strerror(ret));
>> -        exit(1);
>> -    }
>> -
>> -    ret = fdt_setprop_cell(fdt, rtas_node, "rtas-size", spapr->rtas_size);
>> -    if (ret < 0) {
>> -        error_report("Couldn't add rtas-size property: %s",
>> -                     fdt_strerror(ret));
>> -        exit(1);
>> -    }
>> -}
>> -
>>  static void core_rtas_register_types(void)
>>  {
>>      spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index dfec8e8e76..cbd1a4c9f3 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -154,8 +154,6 @@ struct SpaprMachineState {
>>  
>>      hwaddr rma_size;
>>      int vrma_adjust;
>> -    ssize_t rtas_size;
>> -    void *rtas_blob;
>>      uint32_t fdt_size;
>>      uint32_t fdt_initial_size;
>>      void *fdt_blob;
>> diff --git a/pc-bios/spapr-rtas.bin b/pc-bios/spapr-rtas.bin
>> deleted file mode 100644
>> index fc24c8ed8b..0000000000
>> Binary files a/pc-bios/spapr-rtas.bin and /dev/null differ
>> diff --git a/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
>> deleted file mode 100644
>> index 4b9bb12306..0000000000
>> --- a/pc-bios/spapr-rtas/Makefile
>> +++ /dev/null
>> @@ -1,27 +0,0 @@
>> -all: build-all
>> -# Dummy command so that make thinks it has done something
>> -	@true
>> -
>> -include ../../config-host.mak
>> -include $(SRC_PATH)/rules.mak
>> -
>> -$(call set-vpath, $(SRC_PATH)/pc-bios/spapr-rtas)
>> -
>> -.PHONY : all clean build-all
>> -
>> -#CFLAGS += -I$(SRC_PATH)
>> -#QEMU_CFLAGS = $(CFLAGS)
>> -
>> -build-all: spapr-rtas.bin
>> -
>> -%.o: %.S
>> -	$(call quiet-command,$(CCAS) -mbig -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
>> -
>> -%.img: %.o
>> -	$(call quiet-command,$(CC) -nostdlib -mbig -o $@ $<,"Building","$(TARGET_DIR)$@")
>> -
>> -%.bin: %.img
>> -	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"Building","$(TARGET_DIR)$@")
>> -
>> -clean:
>> -	rm -f *.o *.d *.img *.bin *~
>> diff --git a/pc-bios/spapr-rtas/spapr-rtas.S b/pc-bios/spapr-rtas/spapr-rtas.S
>> deleted file mode 100644
>> index 903bec2150..0000000000
>> --- a/pc-bios/spapr-rtas/spapr-rtas.S
>> +++ /dev/null
>> @@ -1,37 +0,0 @@
>> -/*
>> - * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
>> - *
>> - * Trivial in-partition RTAS implementation, based on a hypercall
>> - *
>> - * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
>> - *
>> - * Permission is hereby granted, free of charge, to any person obtaining a copy
>> - * of this software and associated documentation files (the "Software"), to deal
>> - * in the Software without restriction, including without limitation the rights
>> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> - * copies of the Software, and to permit persons to whom the Software is
>> - * furnished to do so, subject to the following conditions:
>> - *
>> - * The above copyright notice and this permission notice shall be included in
>> - * all copies or substantial portions of the Software.
>> - *
>> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> - * THE SOFTWARE.
>> - *
>> - */
>> -
>> -#define KVMPPC_HCALL_BASE       0xf000
>> -#define KVMPPC_H_RTAS           (KVMPPC_HCALL_BASE + 0x0)
>> -
>> -.globl	_start
>> -_start:
>> -	mr	4,3
>> -	lis	3,KVMPPC_H_RTAS@h
>> -	ori	3,3,KVMPPC_H_RTAS@l
>> -	sc	1
>> -	blr
> 

-- 
Alexey


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append
  2019-09-11  8:46   ` Greg Kurz
@ 2019-09-12  1:59     ` Alexey Kardashevskiy
  0 siblings, 0 replies; 25+ messages in thread
From: Alexey Kardashevskiy @ 2019-09-12  1:59 UTC (permalink / raw)
  To: Greg Kurz, David Gibson; +Cc: lvivier, philmd, qemu-ppc, qemu-devel, clg



On 11/09/2019 18:46, Greg Kurz wrote:
> On Wed, 11 Sep 2019 14:04:50 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
>> From: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> We are going to use spapr_build_fdt() for the boot time FDT and as an
>> update for SLOF during handling of H_CAS. SLOF will apply all properties
>> from the QEMU's FDT which is usually ok unless there are properties
>> changed by grub or guest kernel. The properties are:
>> bootargs, linux,initrd-start, linux,initrd-end, linux,stdout-path,
>> linux,rtas-base, linux,rtas-entry. Resetting those during CAS will most
>> likely cause grub failure.
>>
> 
> s/Resetting/Clearing ? They still get reset to the initial setup if "-kernel"
> and "-initrd" were passed, but it is okay since neither grub, nor the guest
> kernel is supposed to change them in this case, correct ?

Correct.



>> This only creates such properties if we are booting with "-kernel" and
>> "-initrd" so they won't get included into the DT update blob and
> 
> so they won't get included {if we're not booting with "-kernel" ...}
> 
>> therefore the guest is more likely to boot successfully.
>>
> 
> Maybe rephrase like:
> 
> Don't create such properties if we're booting without "-kernel" and
> "-initrd" ...
> 
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  hw/ppc/spapr.c | 15 ++++++++++-----
>>  1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index d072c2aa3d..d18744268f 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1177,11 +1177,16 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
>>  
>>      _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
>>  
>> -    _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline));
>> -    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
>> -                          spapr->initrd_base));
>> -    _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
>> -                          spapr->initrd_base + spapr->initrd_size));
>> +    if (machine->kernel_cmdline && machine->kernel_cmdline[0]) {
> 
> machine->kernel_cmdline cannot be NULL.
> 
> From vl.c:
> 
>     if (!kernel_cmdline) {
>         kernel_cmdline = "";
>         current_machine->kernel_cmdline = (char *)kernel_cmdline;
>     }

I do not see the point in having an empty string instead of NULL really
and probably one day somebody else will think the same so I prepared :)

> 
> Also this doesn't check if we're booting with -kernel but rather
> that we're booting with -append ${some_not_empty_string}... what
> about checking spapr->kernel_size, pretty much like you do for
> the initrd ?


We are preserving here "bootargs" which either comes from grub or
"-append" so I do just this. Having -kernel usually (always?) means
there is no grub which we are fixing here but this is just a consequence
of a weird command line.



>> +        _FDT(fdt_setprop_string(fdt, chosen, "bootargs",
>> +                                machine->kernel_cmdline));
>> +    }
>> +    if (spapr->initrd_size) {
>> +        _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
>> +                              spapr->initrd_base));
>> +        _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
>> +                              spapr->initrd_base + spapr->initrd_size));
>> +    }
>>  
>>      if (spapr->kernel_size) {
>>          uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
> 

-- 
Alexey


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob
  2019-09-12  1:50     ` Alexey Kardashevskiy
@ 2019-09-12  7:20       ` Greg Kurz
  0 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2019-09-12  7:20 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: lvivier, qemu-devel, qemu-ppc, clg, philmd, David Gibson

On Thu, 12 Sep 2019 11:50:53 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> 
> 
> On 11/09/2019 19:16, Greg Kurz wrote:
> > On Wed, 11 Sep 2019 14:04:51 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> >> From: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>
> >> SLOF implements one itself so let's remove it from QEMU. It is one less
> >> image and simpler setup as the RTAS blob never stays in its initial place
> >> anyway as the guest OS always decides where to put it.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >> ---
> >>  MAINTAINERS                     |   2 --
> >>  Makefile                        |   2 +-
> >>  configure                       |   6 +----
> >>  hw/ppc/spapr.c                  |  32 ++-----------------------
> >>  hw/ppc/spapr_rtas.c             |  41 --------------------------------
> >>  include/hw/ppc/spapr.h          |   2 --
> >>  pc-bios/spapr-rtas.bin          | Bin 20 -> 0 bytes
> >>  pc-bios/spapr-rtas/Makefile     |  27 ---------------------
> >>  pc-bios/spapr-rtas/spapr-rtas.S |  37 ----------------------------
> >>  9 files changed, 4 insertions(+), 145 deletions(-)
> >>  delete mode 100644 pc-bios/spapr-rtas.bin
> >>  delete mode 100644 pc-bios/spapr-rtas/Makefile
> >>  delete mode 100644 pc-bios/spapr-rtas/spapr-rtas.S
> >>
> > 
> > Nice diffstat :)
> > 
> > But pwclient fails to apply it :(
> > 
> > [greg@bahia qemu-spapr]$ pwclient git-am 1160642
> > Applying patch #1160642 using 'git am'
> > Description: [6/7] spapr: Stop providing RTAS blob
> > Applying: spapr: Stop providing RTAS blob
> > error: cannot apply binary patch to 'pc-bios/spapr-rtas.bin' without full index line
> 
> 
> Some git feature/bug with removing binaries:
> 
> https://stackoverflow.com/questions/17152171/git-cannot-apply-binary-patch-without-full-index-line
> 
> 
> David posted with this:
> ===
> diff --git a/pc-bios/spapr-rtas.bin b/pc-bios/spapr-rtas.bin
> deleted file mode 100644
> index fc24c8ed8b..0000000000
> Binary files a/pc-bios/spapr-rtas.bin and /dev/null differ
> ===
> 
> And my patch has a bigger chunk:
> 
> git format-patch -1 --stdout 1a5efb9283c2
> (there is no additional flag needed to my git 2.17.1):
> 
> ===
> diff --git a/pc-bios/spapr-rtas.bin b/pc-bios/spapr-rtas.bin
> deleted file mode 100644
> index
> fc24c8ed8b92a3a441aed6e2bd013b2ccece9229..0000000000000000000000000000000000000000
> GIT binary patch
> literal 0
> HcmV?d00001
> 
> literal 20
> bcmb<Pk*=^wU|>i{{=neEz@X&Uz@PvCJTV0q
> ===
> 
> I do not know why are these different.
> 
> Thy this one:
> https://patchwork.ozlabs.org/patch/1132443/
> 

This one applies cleanly.

And so does:

https://github.com/dgibson/qemu/commit/c14ffa033ea0519d235f172723dd465ab6bf9777.patch

from David's cas branch... That's confusing.

Anyway, the non-binary changes look good.

Reviewed-by: Greg Kurz <groug@kaod.org>

> 
> 
> > error: pc-bios/spapr-rtas.bin: patch does not apply
> > Patch failed at 0001 spapr: Stop providing RTAS blob
> > hint: Use 'git am --show-current-patch' to see the failed patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> > 'git am' failed with exit status 128
> > 
> > and
> > 
> > [greg@bahia qemu-spapr]$ git am --show-current-patch | patch -p1 --merge 
> > patching file MAINTAINERS
> > patching file Makefile
> > patching file configure
> > patching file hw/ppc/spapr.c
> > patching file hw/ppc/spapr_rtas.c
> > patching file include/hw/ppc/spapr.h
> > patching file pc-bios/spapr-rtas.bin
> > Not deleting file pc-bios/spapr-rtas.bin as content differs from patch
> > 
> > Not sure what's happening here...
> > 
> > patching file pc-bios/spapr-rtas/Makefile
> > patching file pc-bios/spapr-rtas/spapr-rtas.S
> > 
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 50eaf005f4..9823f40213 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -1077,8 +1077,6 @@ F: hw/*/spapr*
> >>  F: include/hw/*/spapr*
> >>  F: hw/*/xics*
> >>  F: include/hw/*/xics*
> >> -F: pc-bios/spapr-rtas/*
> >> -F: pc-bios/spapr-rtas.bin
> >>  F: pc-bios/slof.bin
> >>  F: docs/specs/ppc-spapr-hcalls.txt
> >>  F: docs/specs/ppc-spapr-hotplug.txt
> >> diff --git a/Makefile b/Makefile
> >> index ae17a83067..4637f95371 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -764,7 +764,7 @@ qemu-nsis.bmp \
> >>  bamboo.dtb canyonlands.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
> >>  multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin \
> >>  s390-ccw.img s390-netboot.img \
> >> -spapr-rtas.bin slof.bin skiboot.lid \
> >> +slof.bin skiboot.lid \
> >>  palcode-clipper \
> >>  u-boot.e500 u-boot-sam460-20100605.bin \
> >>  qemu_vga.ndrv \
> >> diff --git a/configure b/configure
> >> index 95134c0180..b79d38592b 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -6211,9 +6211,6 @@ if { test "$cpu" = "i386" || test "$cpu" = "x86_64"; } && \
> >>          fi
> >>      done
> >>  fi
> >> -if test "$ARCH" = "ppc64" && test "$targetos" != "Darwin" ; then
> >> -  roms="$roms spapr-rtas"
> >> -fi
> >>  
> >>  # Only build s390-ccw bios if we're on s390x and the compiler has -march=z900
> >>  if test "$cpu" = "s390x" ; then
> >> @@ -7930,14 +7927,13 @@ fi
> >>  DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
> >>  DIRS="$DIRS tests/fp tests/qgraph"
> >>  DIRS="$DIRS docs docs/interop fsdev scsi"
> >> -DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
> >> +DIRS="$DIRS pc-bios/optionrom pc-bios/s390-ccw"
> >>  DIRS="$DIRS roms/seabios roms/vgabios"
> >>  LINKS="Makefile tests/tcg/Makefile"
> >>  LINKS="$LINKS tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit"
> >>  LINKS="$LINKS tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile"
> >>  LINKS="$LINKS tests/fp/Makefile"
> >>  LINKS="$LINKS pc-bios/optionrom/Makefile pc-bios/keymaps"
> >> -LINKS="$LINKS pc-bios/spapr-rtas/Makefile"
> >>  LINKS="$LINKS pc-bios/s390-ccw/Makefile"
> >>  LINKS="$LINKS roms/seabios/Makefile roms/vgabios/Makefile"
> >>  LINKS="$LINKS pc-bios/qemu-icon.bmp"
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index d18744268f..5a919a6cc1 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -94,7 +94,6 @@
> >>   * We load our kernel at 4M, leaving space for SLOF initial image
> >>   */
> >>  #define FDT_MAX_SIZE            0x100000
> >> -#define RTAS_MAX_SIZE           0x10000
> >>  #define RTAS_MAX_ADDR           0x80000000 /* RTAS must stay below that */
> >>  #define FW_MAX_SIZE             0x400000
> >>  #define FW_FILE_NAME            "slof.bin"
> >> @@ -1721,8 +1720,7 @@ static void spapr_machine_reset(MachineState *machine)
> >>  {
> >>      SpaprMachineState *spapr = SPAPR_MACHINE(machine);
> >>      PowerPCCPU *first_ppc_cpu;
> >> -    uint32_t rtas_limit;
> >> -    hwaddr rtas_addr, fdt_addr;
> >> +    hwaddr fdt_addr;
> >>      void *fdt;
> >>      int rc;
> >>  
> >> @@ -1786,14 +1784,10 @@ static void spapr_machine_reset(MachineState *machine)
> >>       * or just below 2GB, whichever is lower, so that it can be
> >>       * processed with 32-bit real mode code if necessary
> >>       */
> >> -    rtas_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR);
> >> -    rtas_addr = rtas_limit - RTAS_MAX_SIZE;
> >> -    fdt_addr = rtas_addr - FDT_MAX_SIZE;
> >> +    fdt_addr = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FDT_MAX_SIZE;
> >>  
> >>      fdt = spapr_build_fdt(spapr);
> >>  
> >> -    spapr_load_rtas(spapr, fdt, rtas_addr);
> >> -
> >>      rc = fdt_pack(fdt);
> >>  
> >>      /* Should only fail if we've built a corrupted tree */
> >> @@ -2953,28 +2947,6 @@ static void spapr_machine_init(MachineState *machine)
> >>          spapr_create_lmb_dr_connectors(spapr);
> >>      }
> >>  
> >> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
> >> -    if (!filename) {
> >> -        error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
> >> -        exit(1);
> >> -    }
> >> -    spapr->rtas_size = get_image_size(filename);
> >> -    if (spapr->rtas_size < 0) {
> >> -        error_report("Could not get size of LPAR rtas '%s'", filename);
> >> -        exit(1);
> >> -    }
> >> -    spapr->rtas_blob = g_malloc(spapr->rtas_size);
> >> -    if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
> >> -        error_report("Could not load LPAR rtas '%s'", filename);
> >> -        exit(1);
> >> -    }
> >> -    if (spapr->rtas_size > RTAS_MAX_SIZE) {
> >> -        error_report("RTAS too big ! 0x%zx bytes (max is 0x%x)",
> >> -                     (size_t)spapr->rtas_size, RTAS_MAX_SIZE);
> >> -        exit(1);
> >> -    }
> >> -    g_free(filename);
> >> -
> >>      /* Set up RTAS event infrastructure */
> >>      spapr_events_init(spapr);
> >>  
> >> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >> index bee3835214..8d8d8cdfcb 100644
> >> --- a/hw/ppc/spapr_rtas.c
> >> +++ b/hw/ppc/spapr_rtas.c
> >> @@ -477,47 +477,6 @@ void spapr_dt_rtas_tokens(void *fdt, int rtas)
> >>      }
> >>  }
> >>  
> >> -void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr)
> >> -{
> >> -    int rtas_node;
> >> -    int ret;
> >> -
> >> -    /* Copy RTAS blob into guest RAM */
> >> -    cpu_physical_memory_write(addr, spapr->rtas_blob, spapr->rtas_size);
> >> -
> >> -    ret = fdt_add_mem_rsv(fdt, addr, spapr->rtas_size);
> >> -    if (ret < 0) {
> >> -        error_report("Couldn't add RTAS reserve entry: %s",
> >> -                     fdt_strerror(ret));
> >> -        exit(1);
> >> -    }
> >> -
> >> -    /* Update the device tree with the blob's location */
> >> -    rtas_node = fdt_path_offset(fdt, "/rtas");
> >> -    assert(rtas_node >= 0);
> >> -
> >> -    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-base", addr);
> >> -    if (ret < 0) {
> >> -        error_report("Couldn't add linux,rtas-base property: %s",
> >> -                     fdt_strerror(ret));
> >> -        exit(1);
> >> -    }
> >> -
> >> -    ret = fdt_setprop_cell(fdt, rtas_node, "linux,rtas-entry", addr);
> >> -    if (ret < 0) {
> >> -        error_report("Couldn't add linux,rtas-entry property: %s",
> >> -                     fdt_strerror(ret));
> >> -        exit(1);
> >> -    }
> >> -
> >> -    ret = fdt_setprop_cell(fdt, rtas_node, "rtas-size", spapr->rtas_size);
> >> -    if (ret < 0) {
> >> -        error_report("Couldn't add rtas-size property: %s",
> >> -                     fdt_strerror(ret));
> >> -        exit(1);
> >> -    }
> >> -}
> >> -
> >>  static void core_rtas_register_types(void)
> >>  {
> >>      spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index dfec8e8e76..cbd1a4c9f3 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -154,8 +154,6 @@ struct SpaprMachineState {
> >>  
> >>      hwaddr rma_size;
> >>      int vrma_adjust;
> >> -    ssize_t rtas_size;
> >> -    void *rtas_blob;
> >>      uint32_t fdt_size;
> >>      uint32_t fdt_initial_size;
> >>      void *fdt_blob;
> >> diff --git a/pc-bios/spapr-rtas.bin b/pc-bios/spapr-rtas.bin
> >> deleted file mode 100644
> >> index fc24c8ed8b..0000000000
> >> Binary files a/pc-bios/spapr-rtas.bin and /dev/null differ
> >> diff --git a/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
> >> deleted file mode 100644
> >> index 4b9bb12306..0000000000
> >> --- a/pc-bios/spapr-rtas/Makefile
> >> +++ /dev/null
> >> @@ -1,27 +0,0 @@
> >> -all: build-all
> >> -# Dummy command so that make thinks it has done something
> >> -	@true
> >> -
> >> -include ../../config-host.mak
> >> -include $(SRC_PATH)/rules.mak
> >> -
> >> -$(call set-vpath, $(SRC_PATH)/pc-bios/spapr-rtas)
> >> -
> >> -.PHONY : all clean build-all
> >> -
> >> -#CFLAGS += -I$(SRC_PATH)
> >> -#QEMU_CFLAGS = $(CFLAGS)
> >> -
> >> -build-all: spapr-rtas.bin
> >> -
> >> -%.o: %.S
> >> -	$(call quiet-command,$(CCAS) -mbig -c -o $@ $<,"CCAS","$(TARGET_DIR)$@")
> >> -
> >> -%.img: %.o
> >> -	$(call quiet-command,$(CC) -nostdlib -mbig -o $@ $<,"Building","$(TARGET_DIR)$@")
> >> -
> >> -%.bin: %.img
> >> -	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"Building","$(TARGET_DIR)$@")
> >> -
> >> -clean:
> >> -	rm -f *.o *.d *.img *.bin *~
> >> diff --git a/pc-bios/spapr-rtas/spapr-rtas.S b/pc-bios/spapr-rtas/spapr-rtas.S
> >> deleted file mode 100644
> >> index 903bec2150..0000000000
> >> --- a/pc-bios/spapr-rtas/spapr-rtas.S
> >> +++ /dev/null
> >> @@ -1,37 +0,0 @@
> >> -/*
> >> - * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
> >> - *
> >> - * Trivial in-partition RTAS implementation, based on a hypercall
> >> - *
> >> - * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
> >> - *
> >> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> >> - * of this software and associated documentation files (the "Software"), to deal
> >> - * in the Software without restriction, including without limitation the rights
> >> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> >> - * copies of the Software, and to permit persons to whom the Software is
> >> - * furnished to do so, subject to the following conditions:
> >> - *
> >> - * The above copyright notice and this permission notice shall be included in
> >> - * all copies or substantial portions of the Software.
> >> - *
> >> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> >> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> >> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> >> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> >> - * THE SOFTWARE.
> >> - *
> >> - */
> >> -
> >> -#define KVMPPC_HCALL_BASE       0xf000
> >> -#define KVMPPC_H_RTAS           (KVMPPC_HCALL_BASE + 0x0)
> >> -
> >> -.globl	_start
> >> -_start:
> >> -	mr	4,3
> >> -	lis	3,KVMPPC_H_RTAS@h
> >> -	ori	3,3,KVMPPC_H_RTAS@l
> >> -	sc	1
> >> -	blr
> > 
> 



^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, back to index

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11  4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
2019-09-11  4:04 ` [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling David Gibson
2019-09-11  7:09   ` Cédric Le Goater
2019-09-11  7:26   ` Greg Kurz
2019-09-11  7:37   ` Alexey Kardashevskiy
2019-09-11  4:04 ` [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init David Gibson
2019-09-11  7:28   ` Cédric Le Goater
2019-09-11  7:33   ` Greg Kurz
2019-09-11  7:41   ` Alexey Kardashevskiy
2019-09-11  4:04 ` [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS David Gibson
2019-09-11  7:28   ` Cédric Le Goater
2019-09-11  7:36   ` Greg Kurz
2019-09-11  4:04 ` [Qemu-devel] [PATCH 4/7] spapr: Skip leading zeroes from memory@ DT node names David Gibson
2019-09-11  8:01   ` Greg Kurz
2019-09-11  4:04 ` [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append David Gibson
2019-09-11  8:46   ` Greg Kurz
2019-09-12  1:59     ` Alexey Kardashevskiy
2019-09-11  4:04 ` [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob David Gibson
2019-09-11  9:16   ` Greg Kurz
2019-09-12  1:50     ` Alexey Kardashevskiy
2019-09-12  7:20       ` Greg Kurz
2019-09-11  4:04 ` [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order David Gibson
2019-09-11  7:40   ` Alexey Kardashevskiy
2019-09-11  7:51     ` David Gibson
2019-09-11  7:54   ` Cédric Le Goater

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org qemu-devel@archiver.kernel.org
	public-inbox-index qemu-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox