qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob
@ 2019-07-16  5:35 Alexey Kardashevskiy
  2019-07-16  5:39 ` no-reply
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Alexey Kardashevskiy @ 2019-07-16  5:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, David Gibson

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.

This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
hence RFC.

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

diff --git a/configure b/configure
index 4983c8b53300..a132d2eb5666 100755
--- a/configure
+++ b/configure
@@ -6205,9 +6205,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
@@ -7919,14 +7916,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/Makefile b/Makefile
index 1fcbaed62c76..d780f4eebceb 100644
--- a/Makefile
+++ b/Makefile
@@ -764,7 +764,7 @@ efi-e1000e.rom efi-vmxnet3.rom \
 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/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
deleted file mode 100644
index 4b9bb1230658..000000000000
--- 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/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 60553d32c4fa..b6640370c839 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -152,8 +152,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/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8783b433960c..36cd45bd78b3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -89,7 +89,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"
@@ -1704,8 +1703,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;
 
@@ -1783,14 +1781,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 = 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 */
@@ -2906,28 +2900,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 a618a2ac0f3f..b30a5817d7a8 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -431,47 +431,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/MAINTAINERS b/MAINTAINERS
index cc9636b43aab..11b9a4f6fa25 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1070,8 +1070,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/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

diff --git a/pc-bios/spapr-rtas/spapr-rtas.S b/pc-bios/spapr-rtas/spapr-rtas.S
deleted file mode 100644
index 903bec215077..000000000000
--- 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.17.1



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

* Re: [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-16  5:35 [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob Alexey Kardashevskiy
@ 2019-07-16  5:39 ` no-reply
  2019-07-18  7:20 ` Thomas Huth
  2019-07-22  5:07 ` Alexey Kardashevskiy
  2 siblings, 0 replies; 10+ messages in thread
From: no-reply @ 2019-07-16  5:39 UTC (permalink / raw)
  To: aik; +Cc: aik, qemu-ppc, qemu-devel, david

Patchew URL: https://patchew.org/QEMU/20190716053522.78813-1-aik@ozlabs.ru/



Hi,

This series failed build test on FreeBSD host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
if qemu-system-x86_64 --help >/dev/null 2>&1; then
  QEMU=qemu-system-x86_64
elif /usr/libexec/qemu-kvm --help >/dev/null 2>&1; then
  QEMU=/usr/libexec/qemu-kvm
else
  exit 1
fi
make vm-build-freebsd J=21 QEMU=$QEMU
exit 0
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190716053522.78813-1-aik@ozlabs.ru/testing.FreeBSD/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-16  5:35 [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob Alexey Kardashevskiy
  2019-07-16  5:39 ` no-reply
@ 2019-07-18  7:20 ` Thomas Huth
  2019-07-18  7:55   ` Alexey Kardashevskiy
  2019-07-19  2:22   ` [Qemu-devel] " David Gibson
  2019-07-22  5:07 ` Alexey Kardashevskiy
  2 siblings, 2 replies; 10+ messages in thread
From: Thomas Huth @ 2019-07-18  7:20 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel; +Cc: qemu-ppc, David Gibson

On 16/07/2019 07.35, Alexey Kardashevskiy wrote:
> 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.
> 
> This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
> hence RFC.

Patch looks basically fine for me, but I wonder whether we should wait
for one or two releases until we really remove it from QEMU, so that it
is still possible to test the latest QEMU with older SLOF releases for a
while (which is sometimes useful when hunting bugs). Or should this
maybe even go through the official deprecation process (i.e. with an
entry in qemu-deprecated.texi)?

 Thomas


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

* Re: [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-18  7:20 ` Thomas Huth
@ 2019-07-18  7:55   ` Alexey Kardashevskiy
  2019-07-18 10:40     ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
  2019-07-19  2:22   ` [Qemu-devel] " David Gibson
  1 sibling, 1 reply; 10+ messages in thread
From: Alexey Kardashevskiy @ 2019-07-18  7:55 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel; +Cc: qemu-ppc, David Gibson



On 18/07/2019 17:20, Thomas Huth wrote:
> On 16/07/2019 07.35, Alexey Kardashevskiy wrote:
>> 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.
>>
>> This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
>> hence RFC.
> 
> Patch looks basically fine for me, but I wonder whether we should wait
> for one or two releases until we really remove it from QEMU, so that it
> is still possible to test the latest QEMU with older SLOF releases for a
> while (which is sometimes useful when hunting bugs). Or should this
> maybe even go through the official deprecation process (i.e. with an
> entry in qemu-deprecated.texi)?

I worry more about slof being distributed as a separate package in RHEL, 
easy enough to get qemu/slof out of sync.


-- 
Alexey


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

* Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-18  7:55   ` Alexey Kardashevskiy
@ 2019-07-18 10:40     ` Greg Kurz
  2019-07-18 10:49       ` Thomas Huth
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kurz @ 2019-07-18 10:40 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: Thomas Huth, qemu-ppc, qemu-devel, David Gibson

On Thu, 18 Jul 2019 17:55:12 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> 
> 
> On 18/07/2019 17:20, Thomas Huth wrote:
> > On 16/07/2019 07.35, Alexey Kardashevskiy wrote:
> >> 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.
> >>
> >> This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
> >> hence RFC.
> > 
> > Patch looks basically fine for me, but I wonder whether we should wait
> > for one or two releases until we really remove it from QEMU, so that it
> > is still possible to test the latest QEMU with older SLOF releases for a
> > while (which is sometimes useful when hunting bugs). Or should this
> > maybe even go through the official deprecation process (i.e. with an
> > entry in qemu-deprecated.texi)?
> 
> I worry more about slof being distributed as a separate package in RHEL, 
> easy enough to get qemu/slof out of sync.
> 

Then it seems to call for keeping the code around in QEMU in case RHEL's
slof doesn't implement the RTAS blob. Following the official deprecation
process looks like a good option IMHO.

> 



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

* Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-18 10:40     ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2019-07-18 10:49       ` Thomas Huth
  2019-07-19  1:23         ` Alexey Kardashevskiy
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Huth @ 2019-07-18 10:49 UTC (permalink / raw)
  To: Greg Kurz, Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel, David Gibson

On 18/07/2019 12.40, Greg Kurz wrote:
> On Thu, 18 Jul 2019 17:55:12 +1000
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>>
>>
>> On 18/07/2019 17:20, Thomas Huth wrote:
>>> On 16/07/2019 07.35, Alexey Kardashevskiy wrote:
>>>> 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.
>>>>
>>>> This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
>>>> hence RFC.
>>>
>>> Patch looks basically fine for me, but I wonder whether we should wait
>>> for one or two releases until we really remove it from QEMU, so that it
>>> is still possible to test the latest QEMU with older SLOF releases for a
>>> while (which is sometimes useful when hunting bugs). Or should this
>>> maybe even go through the official deprecation process (i.e. with an
>>> entry in qemu-deprecated.texi)?
>>
>> I worry more about slof being distributed as a separate package in RHEL, 
>> easy enough to get qemu/slof out of sync.
>>
> 
> Then it seems to call for keeping the code around in QEMU in case RHEL's
> slof doesn't implement the RTAS blob. Following the official deprecation
> process looks like a good option IMHO.

We can of course make the qemu rpm depend on the new SLOF rpm, so that
you can not install an older SLOF with a newer QEMU.

But anyway, to avoid confusion and ease debugging, I'd also rather vote
for the official deprecation process here, and remove the RTAS blob from
QEMU after the official deprecation period.

 Thomas


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

* Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-18 10:49       ` Thomas Huth
@ 2019-07-19  1:23         ` Alexey Kardashevskiy
  2019-07-19  8:08           ` Thomas Huth
  0 siblings, 1 reply; 10+ messages in thread
From: Alexey Kardashevskiy @ 2019-07-19  1:23 UTC (permalink / raw)
  To: Thomas Huth, Greg Kurz; +Cc: qemu-ppc, qemu-devel, David Gibson



On 18/07/2019 20:49, Thomas Huth wrote:
> On 18/07/2019 12.40, Greg Kurz wrote:
>> On Thu, 18 Jul 2019 17:55:12 +1000
>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>>>
>>>
>>> On 18/07/2019 17:20, Thomas Huth wrote:
>>>> On 16/07/2019 07.35, Alexey Kardashevskiy wrote:
>>>>> 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.
>>>>>
>>>>> This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
>>>>> hence RFC.
>>>>
>>>> Patch looks basically fine for me, but I wonder whether we should wait
>>>> for one or two releases until we really remove it from QEMU, so that it
>>>> is still possible to test the latest QEMU with older SLOF releases for a
>>>> while (which is sometimes useful when hunting bugs). Or should this
>>>> maybe even go through the official deprecation process (i.e. with an
>>>> entry in qemu-deprecated.texi)?
>>>
>>> I worry more about slof being distributed as a separate package in RHEL,
>>> easy enough to get qemu/slof out of sync.
>>>
>>
>> Then it seems to call for keeping the code around in QEMU in case RHEL's
>> slof doesn't implement the RTAS blob. Following the official deprecation
>> process looks like a good option IMHO.
> 
> We can of course make the qemu rpm depend on the new SLOF rpm, so that
> you can not install an older SLOF with a newer QEMU.

Cool, let's do that.

> But anyway, to avoid confusion and ease debugging,

There is a little confusion ("why did the guest stop after Device tree 
struct  0x000000000aff0000 -> 0x000000000b000000") and what will make 
the debugging harder if we drop rtas from qemu now? I think I should 
have known the answer by now but I do not :)

> I'd also rather vote
> for the official deprecation process here, and remove the RTAS blob from
> QEMU after the official deprecation period.

We won't be able to enjoy one less binary for another year and we 
already have bugs fixes for which would benefit from not having rtas blob.



-- 
Alexey


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

* Re: [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-18  7:20 ` Thomas Huth
  2019-07-18  7:55   ` Alexey Kardashevskiy
@ 2019-07-19  2:22   ` David Gibson
  1 sibling, 0 replies; 10+ messages in thread
From: David Gibson @ 2019-07-19  2:22 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Alexey Kardashevskiy, qemu-ppc, qemu-devel

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

On Thu, Jul 18, 2019 at 09:20:56AM +0200, Thomas Huth wrote:
> On 16/07/2019 07.35, Alexey Kardashevskiy wrote:
> > 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.
> > 
> > This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
> > hence RFC.
> 
> Patch looks basically fine for me, but I wonder whether we should wait
> for one or two releases until we really remove it from QEMU, so that it
> is still possible to test the latest QEMU with older SLOF releases for a
> while (which is sometimes useful when hunting bugs). Or should this
> maybe even go through the official deprecation process (i.e. with an
> entry in qemu-deprecated.texi)?

I don't really like this idea.  It blocks some cleanups I'd like to do
which rely on it being truly gone, not just sometimes-gone.

I think it's reasonable to do this, since the "raw" rtas blob from
qemu was never supposed to be a guest visible artifact - any guest
that was relying on this was going out of its way to be fragile.

-- 
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] 10+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-19  1:23         ` Alexey Kardashevskiy
@ 2019-07-19  8:08           ` Thomas Huth
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Huth @ 2019-07-19  8:08 UTC (permalink / raw)
  To: Alexey Kardashevskiy, Greg Kurz; +Cc: qemu-ppc, qemu-devel, David Gibson

On 19/07/2019 03.23, Alexey Kardashevskiy wrote:
> 
> 
> On 18/07/2019 20:49, Thomas Huth wrote:
>> On 18/07/2019 12.40, Greg Kurz wrote:
>>> On Thu, 18 Jul 2019 17:55:12 +1000
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>>>
>>>>
>>>> On 18/07/2019 17:20, Thomas Huth wrote:
>>>>> On 16/07/2019 07.35, Alexey Kardashevskiy wrote:
>>>>>> 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.
>>>>>>
>>>>>> This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
>>>>>> hence RFC.
>>>>>
>>>>> Patch looks basically fine for me, but I wonder whether we should wait
>>>>> for one or two releases until we really remove it from QEMU, so
>>>>> that it
>>>>> is still possible to test the latest QEMU with older SLOF releases
>>>>> for a
>>>>> while (which is sometimes useful when hunting bugs). Or should this
>>>>> maybe even go through the official deprecation process (i.e. with an
>>>>> entry in qemu-deprecated.texi)?
>>>>
>>>> I worry more about slof being distributed as a separate package in
>>>> RHEL,
>>>> easy enough to get qemu/slof out of sync.
>>>>
>>>
>>> Then it seems to call for keeping the code around in QEMU in case RHEL's
>>> slof doesn't implement the RTAS blob. Following the official deprecation
>>> process looks like a good option IMHO.
>>
>> We can of course make the qemu rpm depend on the new SLOF rpm, so that
>> you can not install an older SLOF with a newer QEMU.
> 
> Cool, let's do that.
> 
>> But anyway, to avoid confusion and ease debugging,
> 
> There is a little confusion ("why did the guest stop after Device tree
> struct  0x000000000aff0000 -> 0x000000000b000000") and what will make
> the debugging harder if we drop rtas from qemu now? I think I should
> have known the answer by now but I do not :)

I meant bugs where you are not sure whether it's a problem of QEMU or
SLOF. In that case, it's useful to use older SLOF versions with newer
QEMU versions, to see where it breaks.
But since SLOF is not that much updated recently, I think it's not so
important to keep this "feature".

>> I'd also rather vote
>> for the official deprecation process here, and remove the RTAS blob from
>> QEMU after the official deprecation period.
> 
> We won't be able to enjoy one less binary for another year and we
> already have bugs fixes for which would benefit from not having rtas blob.

Ok, if you have other things in the pipe that depend on this clean-up,
then please ignore my suggestion and remove it right away. It's not a
"feature" that was directly visible to the guest OS or the users, so I
don't think we urgently need the deprecation process here.

 Thomas


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

* Re: [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob
  2019-07-16  5:35 [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob Alexey Kardashevskiy
  2019-07-16  5:39 ` no-reply
  2019-07-18  7:20 ` Thomas Huth
@ 2019-07-22  5:07 ` Alexey Kardashevskiy
  2 siblings, 0 replies; 10+ messages in thread
From: Alexey Kardashevskiy @ 2019-07-22  5:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, David Gibson



On 16/07/2019 15:35, Alexey Kardashevskiy wrote:
> 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.
> 
> This totally depends on https://patchwork.ozlabs.org/patch/1132440/ ,
> hence RFC.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   configure                       |   6 +----
>   Makefile                        |   2 +-
>   pc-bios/spapr-rtas/Makefile     |  27 ---------------------
>   include/hw/ppc/spapr.h          |   2 --
>   hw/ppc/spapr.c                  |  32 ++-----------------------
>   hw/ppc/spapr_rtas.c             |  41 --------------------------------
>   MAINTAINERS                     |   2 --
>   pc-bios/spapr-rtas.bin          | Bin 20 -> 0 bytes
>   pc-bios/spapr-rtas/spapr-rtas.S |  37 ----------------------------
>   9 files changed, 4 insertions(+), 145 deletions(-)
>   delete mode 100644 pc-bios/spapr-rtas/Makefile
>   delete mode 100644 pc-bios/spapr-rtas.bin
>   delete mode 100644 pc-bios/spapr-rtas/spapr-rtas.S
> 
> diff --git a/configure b/configure
> index 4983c8b53300..a132d2eb5666 100755
> --- a/configure
> +++ b/configure
> @@ -6205,9 +6205,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
> @@ -7919,14 +7916,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/Makefile b/Makefile
> index 1fcbaed62c76..d780f4eebceb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -764,7 +764,7 @@ efi-e1000e.rom efi-vmxnet3.rom \
>   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/pc-bios/spapr-rtas/Makefile b/pc-bios/spapr-rtas/Makefile
> deleted file mode 100644
> index 4b9bb1230658..000000000000
> --- 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/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 60553d32c4fa..b6640370c839 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -152,8 +152,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/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8783b433960c..36cd45bd78b3 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -89,7 +89,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"
> @@ -1704,8 +1703,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;
>   
> @@ -1783,14 +1781,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 = RTAS_MAX_ADDR - FDT_MAX_SIZE;



Meanwhile a bug is here, should have been:

+    fdt_addr = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FDT_MAX_SIZE;


Appears only on p9 (radix?). Thanks,


-- 
Alexey


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

end of thread, other threads:[~2019-07-22  5:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-16  5:35 [Qemu-devel] [RFC PATCH qemu] spapr: Stop providing RTAS blob Alexey Kardashevskiy
2019-07-16  5:39 ` no-reply
2019-07-18  7:20 ` Thomas Huth
2019-07-18  7:55   ` Alexey Kardashevskiy
2019-07-18 10:40     ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2019-07-18 10:49       ` Thomas Huth
2019-07-19  1:23         ` Alexey Kardashevskiy
2019-07-19  8:08           ` Thomas Huth
2019-07-19  2:22   ` [Qemu-devel] " David Gibson
2019-07-22  5:07 ` Alexey Kardashevskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).