Re: [PATCH v2 7/8] target/mips: Enable MSA ASE for mips32r6-generic

2024-10-19 Thread Aleksandar Rikalo
>
> Enable MSA ASE for mips32r6-generic CPU.
>
> Cherry-picked 0186e83a0613e90aff6d4c12c91cdb080d695d37
> from https://github.com/MIPS/gnutools-qemu
>
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Faraz Shahbazker 
> Signed-off-by: Aleksandar Rakic 
> ---
>  target/mips/cpu-defs.c.inc | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>

Reviewed-by: Aleksandar Rikalo 

-- Aleksandar


Re: [PATCH v2 8/8] target/mips: Enable MSA ASE for mips64R2-generic

2024-10-19 Thread Aleksandar Rikalo
>
> Enable MSA ASE for mips64R2-generic CPU.
>
> Cherry-picked 60f6ae8d3d685ba1ea5d301222fb72b67f39264f
> from  https://github.com/MIPS/gnutools-qemu
>
> Signed-off-by: Faraz Shahbazker 
> Signed-off-by: Aleksandar Rakic 
> ---
>  target/mips/cpu-defs.c.inc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>

R2 CPUs usually don't support MSA, I think we should skip this.

-- Aleksandar


Re: [PATCH v2 5/8] Add micromips to P5600

2024-10-19 Thread Aleksandar Rikalo
>
> Add micromips to P5600.
>
> Cherry-picked d7bf2c2f7f2e03b55c6e9c57eec5c3e6207005a0
> from https://github.com/MIPS/gnutools-qemu
>
> Signed-off-by: Faraz Shahbazker 
> Signed-off-by: Matthew Fortune 
> Signed-off-by: Aleksandar Rakic 
> ---
>  target/mips/cpu-defs.c.inc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Aleksandar Rikalo 

-- Aleksandar


Re: [PATCH v2 2/8] Add support for emulation of CRC32 instructions

2024-10-19 Thread Aleksandar Rikalo
>
> Add emulation of MIPS' CRC32 (Cyclic Redundancy Check) instructions.
> Reuse zlib crc32() and Linux crc32c().
>
> Cherry-picked 4cc974938aee1588f852590509004e340c072940
> from https://github.com/MIPS/gnutools-qemu
>
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Aleksandar Rakic 
> ---
>  target/mips/helper.h|  2 ++
>  target/mips/meson.build |  1 +
>  target/mips/tcg/op_helper.c | 26 ++
>  target/mips/tcg/translate.c | 37 +
>  target/mips/tcg/translate.h |  1 +
>  5 files changed, 67 insertions(+)
>

Reviewed-by: Aleksandar Rikalo 

-- Aleksandar


[Question] What is the definition of “private” fields in QOM?

2024-10-19 Thread Zhao Liu
Hi maintainers and list,

In the QOM structure, the class and object structs have two members:
parent_class and parent_obj, which are often marked as "< private >" in
the comment.

I couldn’t find information on why to define ‘private’ and ‘public’,
even in the earliest QOM commits and the patch emails I could find.

Does ‘private’ refer to the internal implementation code of QOM, or does
it refer to the specific code that defines and implements this object
and its class?

I understand the original idea of private field indicates it cannot be
accessed directly out of the "private" scope.

Regards,
Zhao




[RFC PATCH v2 1/7] plugins: add API for registering trap related callbacks

2024-10-19 Thread Julian Ganz
The plugin API allows registration of callbacks for a variety of VCPU
related events, such as VCPU reset, idle and resume. However, traps of
any kind, i.e. interrupts or exceptions, were previously not covered.
These kinds of events are arguably quite significant and usually go hand
in hand with a PC discontinuity and, on most platforms, a transition
from some "mode" to another. Thus, plugins for the analysis of
(virtualized) embedded systems may benefit from or even require the
possiblity to perform work on the occurance of an interrupt or
exception.

This change introduces an interface for the registration of trap related
callbacks. Specifically we (loosely) define interrupts, exceptions and
semihosting events across all platforms and introduce one callback for
each type of event. Because possible modes and concepts for enumeration
of traps vary greatly between different architectures the callbacks are
`qemu_plugin_vcpu_simple_cb_t`. That is, they only receive the VCPU id
and may need to call other API functions for retrieving additional
information.

Signed-off-by: Julian Ganz 
---
 include/qemu/plugin-event.h  |  3 +++
 include/qemu/qemu-plugin.h   | 45 
 plugins/core.c   | 18 +++
 plugins/qemu-plugins.symbols |  3 +++
 4 files changed, 69 insertions(+)

diff --git a/include/qemu/plugin-event.h b/include/qemu/plugin-event.h
index 7056d8427b..2b69a3821b 100644
--- a/include/qemu/plugin-event.h
+++ b/include/qemu/plugin-event.h
@@ -20,6 +20,9 @@ enum qemu_plugin_event {
 QEMU_PLUGIN_EV_VCPU_SYSCALL_RET,
 QEMU_PLUGIN_EV_FLUSH,
 QEMU_PLUGIN_EV_ATEXIT,
+QEMU_PLUGIN_EV_VCPU_INTERRUPT,
+QEMU_PLUGIN_EV_VCPU_EXCEPTION,
+QEMU_PLUGIN_EV_VCPU_SEMIHOSTING,
 QEMU_PLUGIN_EV_MAX, /* total number of plugin events we support */
 };
 
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 622c9a0232..94c3ccd496 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -230,6 +230,51 @@ QEMU_PLUGIN_API
 void qemu_plugin_register_vcpu_resume_cb(qemu_plugin_id_t id,
  qemu_plugin_vcpu_simple_cb_t cb);
 
+/**
+ * qemu_plugin_register_vcpu_interrupt_cb() - register a vCPU interrupt 
callback
+ * @id: plugin ID
+ * @cb: callback function
+ *
+ * The @cb function is called every time a vCPU receives an interrupt, after 
the
+ * vCPU was prepared to handle the interrupt. An interrupt, in this context and
+ * across all architectures, is defined as an asynchronous event, usually
+ * originating from outside the CPU. Preparation usually entails updating the 
PC
+ * to some interrupt handler or trap vector entry.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_register_vcpu_interrupt_cb(qemu_plugin_id_t id,
+qemu_plugin_vcpu_simple_cb_t cb);
+
+/**
+ * qemu_plugin_register_vcpu_exception_cb() - register a vCPU exception 
callback
+ * @id: plugin ID
+ * @cb: callback function
+ *
+ * The @cb function is called every time a vCPU receives an exception 
(excluding
+ * semihosting events), after the vCPU was prepared to handle the expection. An
+ * exception, in this context and across all architectures, is defined as a
+ * synchronous event in response to a specific instruction being executed.
+ * Preparation usually entails updating the PC to some exception handler or 
trap
+ * vector entry.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_register_vcpu_exception_cb(qemu_plugin_id_t id,
+qemu_plugin_vcpu_simple_cb_t cb);
+
+/**
+ * qemu_plugin_register_vcpu_semihosting_cb() - register a vCPU semihosting cb
+ * @id: plugin ID
+ * @cb: callback function
+ *
+ * The @cb function is called every time a vCPU encounters a semihosting event,
+ * after the event was handled. A semihosting event can be thought of as a
+ * special kind of exception that is not handled by code run by the vCPU but
+ * machinery outside the vCPU.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_register_vcpu_semihosting_cb(qemu_plugin_id_t id,
+  qemu_plugin_vcpu_simple_cb_t cb);
+
 /** struct qemu_plugin_tb - Opaque handle for a translation block */
 struct qemu_plugin_tb;
 /** struct qemu_plugin_insn - Opaque handle for a translated instruction */
diff --git a/plugins/core.c b/plugins/core.c
index bb105e8e68..9de997069c 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -559,6 +559,24 @@ void qemu_plugin_register_vcpu_resume_cb(qemu_plugin_id_t 
id,
 plugin_register_cb(id, QEMU_PLUGIN_EV_VCPU_RESUME, cb);
 }
 
+void qemu_plugin_register_vcpu_interrupt_cb(qemu_plugin_id_t id,
+qemu_plugin_vcpu_simple_cb_t cb)
+{
+plugin_register_cb(id, QEMU_PLUGIN_EV_VCPU_INTERRUPT, cb);
+}
+
+void qemu_plugin_register_vcpu_exception_cb(qemu_plugin_id_t id,
+qemu_plugin_vcpu_simple_cb_t cb)
+{
+plugin_register_cb(

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

2024-10-19 Thread Philippe Mathieu-Daudé

On 18/10/24 12:42, Peter Maydell wrote:

On Tue, 15 Oct 2024 at 14:57, Jan Luebbe  wrote:


For testing eMMC-specific functionality (such as handling boot
partitions), it would be very useful to attach them to generic VMs such
as x86_64 via the sdhci-pci device:
  ...
  -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
  -device sdhci-pci \
  -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
  ...

While most eMMCs are soldered to boards, they can also be connected to
SD controllers with just a passive adapter, such as:
  https://docs.radxa.com/en/accessories/emmc-to-usd
  https://github.com/voltlog/emmc-wfbga153-microsd

The only change necessary to make the options above work is to avoid
disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
kernel already supports this just fine.

Signed-off-by: Jan Luebbe 


Applied to target-arm.next, thanks (unless anybody would
prefer it to go via some other route).


Thanks!




[RFC PATCH v2 4/7] target/arm: call plugin trap callbacks

2024-10-19 Thread Julian Ganz
We recently introduced API for registering callbacks for trap related
events as well as the corresponding hook functions. Due to differences
between architectures, the latter need to be called from target specific
code.

This change places hooks for ARM (and Aarch64) targets. We decided to
treat the (V)IRQ, (VI/VF)NMI, (V)FIQ and VSERR exceptions as interrupts
since they are, presumably, async in nature.

Signed-off-by: Julian Ganz 
---
 target/arm/helper.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0a731a38e8..f636e216c8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -31,6 +31,7 @@
 #endif
 #include "cpregs.h"
 #include "target/arm/gtimer.h"
+#include "qemu/plugin.h"
 
 #define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
 
@@ -11147,6 +11148,24 @@ static void take_aarch32_exception(CPUARMState *env, 
int new_mode,
 }
 }
 
+static void arm_do_plugin_vcpu_interrupt_cb(CPUState *cs)
+{
+switch (cs->exception_index) {
+case EXCP_IRQ:
+case EXCP_VIRQ:
+case EXCP_NMI:
+case EXCP_VINMI:
+case EXCP_FIQ:
+case EXCP_VFIQ:
+case EXCP_VFNMI:
+case EXCP_VSERR:
+qemu_plugin_vcpu_interrupt_cb(cs);
+break;
+default:
+qemu_plugin_vcpu_exception_cb(cs);
+}
+}
+
 static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs)
 {
 /*
@@ -11819,6 +11838,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
 if (tcg_enabled() && arm_is_psci_call(cpu, cs->exception_index)) {
 arm_handle_psci_call(cpu);
 qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
+arm_do_plugin_vcpu_interrupt_cb(cs);
 return;
 }
 
@@ -11830,6 +11850,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
 #ifdef CONFIG_TCG
 if (cs->exception_index == EXCP_SEMIHOST) {
 tcg_handle_semihosting(cs);
+qemu_plugin_vcpu_semihosting_cb(cs);
 return;
 }
 #endif
@@ -11855,6 +11876,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
 if (!kvm_enabled()) {
 cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
 }
+
+arm_do_plugin_vcpu_interrupt_cb(cs);
 }
 #endif /* !CONFIG_USER_ONLY */
 
-- 
2.45.2




[RFC PATCH v2 0/7] tcg-plugins: add hooks for interrupts, exceptions and traps

2024-10-19 Thread Julian Ganz
Some analysis greatly benefits, or depends on, information about
interrupts. For example, we may need to handle the execution of a new
translation block differently if it is not the result of normal program
flow but of an interrupt.

Even with the existing interfaces, it is more or less possible to
discern these situations, e.g. as done by the cflow plugin. However,
this process poses a considerable overhead to the core analysis one may
intend to perform.

These changes introduce a generic and easy-to-use interface for plugin
authors in the form of callbacks for different types of traps. Patch 1
defines the callback registration functions and supplies a somewhat
narrow definition of the event the callbacks are called. Patch 2 adds
some hooks for triggering the callbacks. Patch 3 adds an example plugin
showcasing the new API. The remaining patches call the hooks for a
selection of architectures, mapping architecture specific events to the
three categories defined in patch 1. Future non-RFC patchsets will call
these hooks for all architectures (that have some concept of trap or
interrupt).

Sidenote: I'm likely doing something wrong for one architecture or
the other. For example, with the old version Alex Bennée suggested
registering a helper function with arm_register_el_change_hook() for
arm, which is not what I ended up doing. And for AVR my approach to just
assume only (asynchroneous) interrupts exist is also likely too naïve.

Since v1:
  - Split the one callback into multiple callbacks
  - Added a target-agnostic definition of the relevant event(s)
  - Call hooks from architecture-code rather than accel/tcg/cpu-exec.c
  - Added a plugin showcasing API usage

Julian Ganz (7):
  plugins: add API for registering trap related callbacks
  plugins: add hooks for new trap related callbacks
  contrib/plugins: add plugin showcasing new trap related API
  target/arm: call plugin trap callbacks
  target/avr: call plugin trap callbacks
  target/riscv: call plugin trap callbacks
  target/sparc: call plugin trap callbacks

 contrib/plugins/Makefile |  1 +
 contrib/plugins/traps.c  | 89 
 include/qemu/plugin-event.h  |  3 ++
 include/qemu/plugin.h| 12 +
 include/qemu/qemu-plugin.h   | 45 ++
 plugins/core.c   | 42 +
 plugins/qemu-plugins.symbols |  3 ++
 target/arm/helper.c  | 23 ++
 target/avr/helper.c  |  3 ++
 target/riscv/cpu_helper.c|  8 
 target/sparc/int32_helper.c  |  7 +++
 target/sparc/int64_helper.c  | 10 
 12 files changed, 246 insertions(+)
 create mode 100644 contrib/plugins/traps.c

-- 
2.45.2




[RFC PATCH v2 6/7] target/riscv: call plugin trap callbacks

2024-10-19 Thread Julian Ganz
We recently introduced API for registering callbacks for trap related
events as well as the corresponding hook functions. Due to differences
between architectures, the latter need to be called from target specific
code.

This change places hooks for RISC-V targets.

Signed-off-by: Julian Ganz 
---
 target/riscv/cpu_helper.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index a935377b4a..2a95869339 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -33,6 +33,7 @@
 #include "cpu_bits.h"
 #include "debug.h"
 #include "tcg/oversized-guest.h"
+#include "qemu/plugin.h"
 
 int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
 {
@@ -1678,6 +1679,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 case RISCV_EXCP_SEMIHOST:
 do_common_semihosting(cs);
 env->pc += 4;
+qemu_plugin_vcpu_semihosting_cb(cs);
 return;
 #endif
 case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
@@ -1839,6 +1841,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 riscv_cpu_set_mode(env, PRV_M, virt);
 }
 
+if (async) {
+qemu_plugin_vcpu_interrupt_cb(cs);
+} else {
+qemu_plugin_vcpu_exception_cb(cs);
+}
+
 /*
  * NOTE: it is not necessary to yield load reservations here. It is only
  * necessary for an SC from "another hart" to cause a load reservation
-- 
2.45.2




[RFC PATCH v2 5/7] target/avr: call plugin trap callbacks

2024-10-19 Thread Julian Ganz
We recently introduced API for registering callbacks for trap related
events as well as the corresponding hook functions. Due to differences
between architectures, the latter need to be called from target specific
code.

This change places the hook for AVR targets. That architecture appears
to only know interrupts.

Signed-off-by: Julian Ganz 
---
 target/avr/helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/avr/helper.c b/target/avr/helper.c
index 345708a1b3..be94552674 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -28,6 +28,7 @@
 #include "exec/cpu_ldst.h"
 #include "exec/address-spaces.h"
 #include "exec/helper-proto.h"
+#include "qemu/plugin.h"
 
 bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 {
@@ -97,6 +98,8 @@ void avr_cpu_do_interrupt(CPUState *cs)
 env->sregI = 0; /* clear Global Interrupt Flag */
 
 cs->exception_index = -1;
+
+qemu_plugin_vcpu_interrupt_cb(cs);
 }
 
 hwaddr avr_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
-- 
2.45.2




[RFC PATCH v2 7/7] target/sparc: call plugin trap callbacks

2024-10-19 Thread Julian Ganz
We recently introduced API for registering callbacks for trap related
events as well as the corresponding hook functions. Due to differences
between architectures, the latter need to be called from target specific
code.

This change places hooks for SPARC (32bit and 64bit) targets. We treat
any interrupt other than EXTINT and IVEC as exceptions as they appear to
be synchroneous events. Also note that SPARC targets do not have
semihosting events.

Signed-off-by: Julian Ganz 
---
 target/sparc/int32_helper.c |  7 +++
 target/sparc/int64_helper.c | 10 ++
 2 files changed, 17 insertions(+)

diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
index f2dd8bcb2e..4eb41694f7 100644
--- a/target/sparc/int32_helper.c
+++ b/target/sparc/int32_helper.c
@@ -24,6 +24,7 @@
 #include "exec/cpu_ldst.h"
 #include "exec/log.h"
 #include "sysemu/runstate.h"
+#include "qemu/plugin.h"
 
 static const char * const excp_names[0x80] = {
 [TT_TFAULT] = "Instruction Access Fault",
@@ -172,4 +173,10 @@ void sparc_cpu_do_interrupt(CPUState *cs)
 env->qemu_irq_ack(env, intno);
 }
 #endif
+
+if (intno == TT_EXTINT) {
+qemu_plugin_vcpu_interrupt_cb(cs);
+} else {
+qemu_plugin_vcpu_exception_cb(cs);
+}
 }
diff --git a/target/sparc/int64_helper.c b/target/sparc/int64_helper.c
index bd14c7a0db..f4175ef912 100644
--- a/target/sparc/int64_helper.c
+++ b/target/sparc/int64_helper.c
@@ -23,6 +23,7 @@
 #include "exec/helper-proto.h"
 #include "exec/log.h"
 #include "trace.h"
+#include "qemu/plugin.h"
 
 #define DEBUG_PCALL
 
@@ -253,6 +254,15 @@ void sparc_cpu_do_interrupt(CPUState *cs)
 }
 env->npc = env->pc + 4;
 cs->exception_index = -1;
+
+switch (intno) {
+case TT_EXTINT:
+case TT_IVEC:
+qemu_plugin_vcpu_interrupt_cb(cs);
+break;
+default:
+qemu_plugin_vcpu_exception_cb(cs);
+}
 }
 
 trap_state *cpu_tsptr(CPUSPARCState* env)
-- 
2.45.2




[RFC PATCH v2 2/7] plugins: add hooks for new trap related callbacks

2024-10-19 Thread Julian Ganz
The plugin API allows registration of callbacks for a variety of VCPU
related events, such as VCPU reset, idle and resume. In addition, we
recently introduced API for registering callbacks for trap events,
specifically for interrupts, exceptions and semihosting events.

This change introduces the corresponding hooks called from target
specific code inside qemu.

Signed-off-by: Julian Ganz 
---
 include/qemu/plugin.h | 12 
 plugins/core.c| 24 
 2 files changed, 36 insertions(+)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 9726a9ebf3..71f03b83f4 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -160,6 +160,9 @@ void qemu_plugin_vcpu_exit_hook(CPUState *cpu);
 void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb);
 void qemu_plugin_vcpu_idle_cb(CPUState *cpu);
 void qemu_plugin_vcpu_resume_cb(CPUState *cpu);
+void qemu_plugin_vcpu_interrupt_cb(CPUState *cpu);
+void qemu_plugin_vcpu_exception_cb(CPUState *cpu);
+void qemu_plugin_vcpu_semihosting_cb(CPUState *cpu);
 void
 qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1,
  uint64_t a2, uint64_t a3, uint64_t a4, uint64_t a5,
@@ -242,6 +245,15 @@ static inline void qemu_plugin_vcpu_idle_cb(CPUState *cpu)
 static inline void qemu_plugin_vcpu_resume_cb(CPUState *cpu)
 { }
 
+void qemu_plugin_vcpu_interrupt_cb(CPUState *cpu)
+{ }
+
+void qemu_plugin_vcpu_exception_cb(CPUState *cpu)
+{ }
+
+void qemu_plugin_vcpu_semihosting_cb(CPUState *cpu)
+{ }
+
 static inline void
 qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
  uint64_t a3, uint64_t a4, uint64_t a5, uint64_t a6,
diff --git a/plugins/core.c b/plugins/core.c
index 9de997069c..4f80f1cb72 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -100,6 +100,9 @@ static void plugin_vcpu_cb__simple(CPUState *cpu, enum 
qemu_plugin_event ev)
 case QEMU_PLUGIN_EV_VCPU_EXIT:
 case QEMU_PLUGIN_EV_VCPU_IDLE:
 case QEMU_PLUGIN_EV_VCPU_RESUME:
+case QEMU_PLUGIN_EV_VCPU_INTERRUPT:
+case QEMU_PLUGIN_EV_VCPU_EXCEPTION:
+case QEMU_PLUGIN_EV_VCPU_SEMIHOSTING:
 /* iterate safely; plugins might uninstall themselves at any time */
 QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
 qemu_plugin_vcpu_simple_cb_t func = cb->f.vcpu_simple;
@@ -547,6 +550,27 @@ void qemu_plugin_vcpu_resume_cb(CPUState *cpu)
 }
 }
 
+void qemu_plugin_vcpu_interrupt_cb(CPUState *cpu)
+{
+if (cpu->cpu_index < plugin.num_vcpus) {
+plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_INTERRUPT);
+}
+}
+
+void qemu_plugin_vcpu_exception_cb(CPUState *cpu)
+{
+if (cpu->cpu_index < plugin.num_vcpus) {
+plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_EXCEPTION);
+}
+}
+
+void qemu_plugin_vcpu_semihosting_cb(CPUState *cpu)
+{
+if (cpu->cpu_index < plugin.num_vcpus) {
+plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_SEMIHOSTING);
+}
+}
+
 void qemu_plugin_register_vcpu_idle_cb(qemu_plugin_id_t id,
qemu_plugin_vcpu_simple_cb_t cb)
 {
-- 
2.45.2




Re: [RFC PATCH v2 5/7] target/avr: call plugin trap callbacks

2024-10-19 Thread Michael Rolnik
Reviewed-by: Michael Rolnik 

On Sat, Oct 19, 2024 at 7:39 PM Julian Ganz  wrote:

> We recently introduced API for registering callbacks for trap related
> events as well as the corresponding hook functions. Due to differences
> between architectures, the latter need to be called from target specific
> code.
>
> This change places the hook for AVR targets. That architecture appears
> to only know interrupts.
>
> Signed-off-by: Julian Ganz 
> ---
>  target/avr/helper.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 345708a1b3..be94552674 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -28,6 +28,7 @@
>  #include "exec/cpu_ldst.h"
>  #include "exec/address-spaces.h"
>  #include "exec/helper-proto.h"
> +#include "qemu/plugin.h"
>
>  bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
> @@ -97,6 +98,8 @@ void avr_cpu_do_interrupt(CPUState *cs)
>  env->sregI = 0; /* clear Global Interrupt Flag */
>
>  cs->exception_index = -1;
> +
> +qemu_plugin_vcpu_interrupt_cb(cs);
>  }
>
>  hwaddr avr_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> --
> 2.45.2
>
>

-- 
Best Regards,
Michael Rolnik


Re: [PATCH net-next v7] ptp: Add support for the AMZNC10C 'vmclock' device

2024-10-19 Thread David Woodhouse
On Mon, 2024-10-14 at 13:12 -0700, Jakub Kicinski wrote:
> On Mon, 14 Oct 2024 08:25:35 +0100 David Woodhouse wrote:
> > On Wed, 2024-10-09 at 17:32 -0700, Jakub Kicinski wrote:
> > > On Sun, 06 Oct 2024 08:17:58 +0100 David Woodhouse wrote:  
> > > > +config PTP_1588_CLOCK_VMCLOCK
> > > > +   tristate "Virtual machine PTP clock"
> > > > +   depends on X86_TSC || ARM_ARCH_TIMER
> > > > +   depends on PTP_1588_CLOCK && ACPI && ARCH_SUPPORTS_INT128
> > > > +   default y  
> > > 
> > > Why default to enabled? Linus will not be happy..  
> > 
> > Want an incremental patch to change that?
> 
> Yes please and thank you! We gotta straighten it out before 
> the merge window.

Hm, as I (finally) come to do that, I realise that many of the others
defined in drivers/ptp/Kconfig are also 'default y'. Which is only
really 'default PTP_1588_CLOCK' in practice since they all depend on
that.

Most importantly, PTP_1588_CLOCK_KVM is 'default y'. And that one is
fundamentally broken (at least in the presence of live migration if
guests care about their clock suddenly being wrong) which is why it's
being superseded by the new VMCLOCK thing. We absolutely don't want to
leave the _KVM one enabled by default and not its _VMCLOCK replacement.

Please advise... I suspect the best answer is to leave it as it is? 


smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH] sockets: Remove deadcode

2024-10-19 Thread Dr. David Alan Gilbert
* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Thu, Sep 19, 2024 at 12:26:33AM +0100, d...@treblig.org wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > socket_remote_address hasn't been used since it was added in
> >   17c55decec ("sockets: add helpers for creating SocketAddress from a 
> > socket")
> > 
> > inet_connect hasn't been used since 2017's
> >   8ecc2f9eab ("sheepdog: Use SocketAddress and socket_connect()")
> 
> Excellent ! The more of sockets.h we can remove the better.
> 
> > 
> > Remove them.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  include/qemu/sockets.h | 16 
> >  util/qemu-sockets.c| 35 ---
> >  2 files changed, 51 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé 
> 
> and queued.

Hi Dan,
  Just checking, I noticed this one and the crypto patch haven't made
it to master yet.

Dave

> 
> With regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
> 
-- 
 -Open up your eyes, open up your mind, open up your code ---   
/ Dr. David Alan Gilbert|   Running GNU/Linux   | Happy  \ 
\dave @ treblig.org |   | In Hex /
 \ _|_ http://www.treblig.org   |___/



Re: [PATCH v2] block/file-posix: optimize append write

2024-10-19 Thread Damien Le Moal
On 10/18/24 23:37, Kevin Wolf wrote:
> Am 04.10.2024 um 12:41 hat Sam Li geschrieben:
>> When the file-posix driver emulates append write, it holds the lock
>> whenever accessing wp, which limits the IO queue depth to one.
>>
>> The write IO flow can be optimized to allow concurrent writes. The lock
>> is held in two cases:
>> 1. Assumed that the write IO succeeds, update the wp before issuing the
>> write.
>> 2. If the write IO fails, report that zone and use the reported value
>> as the current wp.
> 
> What happens with the concurrent writes that started later and may not
> have completed yet? Can we really just reset to the reported value
> before all other requests have completed, too?

Yes, because if one write fails, we know that the following writes will fail too
as they will not be aligned to the write pointer. These subsequent failed writes
will again trigger the report zones and update, but that is fine. All of them
have failed and the report will give the same wp again.

This is a typical pattern with zoned block device: if one write fails in a zone,
the user has to expect failures for all other writes issued to the same zone, do
a report zone to get the wp and restart writing from there.

-- 
Damien Le Moal
Western Digital Research



[PATCH v5 01/19] hw/s390x/ipl: Provide more memory to the s390-ccw.img firmware

2024-10-19 Thread jrossi
From: Jared Rossi 

We are going to link the SLOF libc into the s390-ccw.img, and this
libc needs more memory for providing space for malloc() and friends.
Thus bump the memory size that we reserve for the bios to 3 MiB
instead of only 2 MiB. While we're at it, add a proper check that
there is really enough memory assigned to the machine before blindly
using it.

Co-authored by: Thomas Huth 
Signed-off-by: Jared Rossi 
---
 hw/s390x/ipl.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 5ab7433908..5f60977ceb 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -45,6 +45,7 @@
 #define INITRD_PARM_START   0x010408UL
 #define PARMFILE_START  0x001000UL
 #define ZIPL_IMAGE_START0x009000UL
+#define BIOS_MAX_SIZE   0x30UL
 #define IPL_PSW_MASK(PSW_MASK_32 | PSW_MASK_64)
 
 static bool iplb_extended_needed(void *opaque)
@@ -144,7 +145,14 @@ static void s390_ipl_realize(DeviceState *dev, Error 
**errp)
  * even if an external kernel has been defined.
  */
 if (!ipl->kernel || ipl->enforce_bios) {
-uint64_t fwbase = (MIN(ms->ram_size, 0x8000U) - 0x20) & 
~0xUL;
+uint64_t fwbase;
+
+if (ms->ram_size < BIOS_MAX_SIZE) {
+error_setg(errp, "not enough RAM to load the BIOS file");
+return;
+}
+
+fwbase = (MIN(ms->ram_size, 0x8000U) - BIOS_MAX_SIZE) & ~0xUL;
 
 bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, ipl->firmware);
 if (bios_filename == NULL) {
-- 
2.45.1




[PATCH v5 00/19] s390x: Add Full Boot Order Support

2024-10-19 Thread jrossi
From: Jared Rossi 

changes v4 -> v5:
- Fix a bug with per-deice loadparm support:
The machine loadparm is no longer overwritten by device values, which now
allows an empty machine loadparm to propagate to later devices even if
the primary boot device set an initial loadparm
- Fix two instances where changes were squashed into wrong patch
- Fix an instance where NULL_BLOCK_NR was returned instead of ERROR_BLOCK_NR
- Fix an instance of logical AND being used instead of bitwise AND
- Standardize all error values to be negative in all device type paths
- Minor stylistic changes and code simplification


changes v3 -> v4:
- Ensure signed-ness of return values is appropriate
- Add missing newline character in replacements of sclp_print_int()
- Add a missing return in a SCSI error path
- Restore break that was incorrectly removed for Virtio CU devices
- Remove an extra/early return that caused probing to end early
- Convert "good" device to scsi-cd in a cdrom-test for better coverage
- Minor stylistic clean-ups and variable name clarifications

Jared Rossi (19):
  hw/s390x/ipl: Provide more memory to the s390-ccw.img firmware
  pc-bios/s390-ccw: Use the libc from SLOF and remove sclp prints
  pc-bios/s390-ccw: Link the netboot code into the main s390-ccw.img
binary
  hw/s390x: Remove the possibility to load the s390-netboot.img binary
  pc-bios/s390-ccw: Merge netboot.mak into the main Makefile
  docs/system/s390x/bootdevices: Update the documentation about network
booting
  pc-bios/s390-ccw: Remove panics from ISO IPL path
  pc-bios/s390-ccw: Remove panics from ECKD IPL path
  pc-bios/s390-ccw: Remove panics from SCSI IPL path
  pc-bios/s390-ccw: Remove panics from DASD IPL path
  pc-bios/s390-ccw: Remove panics from Netboot IPL path
  pc-bios/s390-ccw: Enable failed IPL to return after error
  include/hw/s390x: Add include files for common IPL structs
  s390x: Add individual loadparm assignment to CCW device
  hw/s390x: Build an IPLB for each boot device
  s390x: Rebuild IPLB for SCSI device directly from DIAG308
  pc-bios/s390x: Enable multi-device boot loop
  docs/system: Update documentation for s390x IPL
  tests/qtest: Add s390x boot order tests to cdrom-test.c

 docs/system/bootindex.rst |   7 +-
 docs/system/s390x/bootdevices.rst |  29 +-
 pc-bios/s390-ccw/netboot.mak  |  62 
 hw/s390x/ccw-device.h |   2 +
 hw/s390x/ipl.h| 123 +---
 include/hw/s390x/ipl/qipl.h   | 127 +
 pc-bios/s390-ccw/bootmap.h|  20 +-
 pc-bios/s390-ccw/cio.h|   2 +
 pc-bios/s390-ccw/dasd-ipl.h   |   2 +-
 pc-bios/s390-ccw/iplb.h   | 108 ++-
 pc-bios/s390-ccw/libc.h   |  89 --
 pc-bios/s390-ccw/s390-ccw.h   |  36 +--
 pc-bios/s390-ccw/virtio.h |   3 +-
 hw/s390x/ccw-device.c |  46 +++
 hw/s390x/ipl.c| 282 +-
 hw/s390x/s390-virtio-ccw.c|  28 +-
 hw/s390x/sclp.c   |   9 +-
 pc-bios/s390-ccw/bootmap.c| 455 --
 pc-bios/s390-ccw/cio.c|  81 +++---
 pc-bios/s390-ccw/dasd-ipl.c   |  71 ++---
 pc-bios/s390-ccw/jump2ipl.c   |  22 +-
 pc-bios/s390-ccw/libc.c   |  88 --
 pc-bios/s390-ccw/main.c   |  97 ---
 pc-bios/s390-ccw/menu.c   |  51 ++--
 pc-bios/s390-ccw/netmain.c|  38 ++-
 pc-bios/s390-ccw/sclp.c   |   7 +-
 pc-bios/s390-ccw/virtio-blkdev.c  |  12 +-
 pc-bios/s390-ccw/virtio-net.c |   7 +-
 pc-bios/s390-ccw/virtio-scsi.c| 160 +++
 pc-bios/s390-ccw/virtio.c |  67 +++--
 target/s390x/diag.c   |   9 +-
 tests/qtest/cdrom-test.c  |  24 ++
 pc-bios/meson.build   |   1 -
 pc-bios/s390-ccw/Makefile |  69 -
 pc-bios/s390-netboot.img  | Bin 67232 -> 0 bytes
 35 files changed, 1158 insertions(+), 1076 deletions(-)
 delete mode 100644 pc-bios/s390-ccw/netboot.mak
 create mode 100644 include/hw/s390x/ipl/qipl.h
 delete mode 100644 pc-bios/s390-ccw/libc.h
 delete mode 100644 pc-bios/s390-ccw/libc.c
 delete mode 100644 pc-bios/s390-netboot.img

-- 
2.45.1




[PATCH v5 08/19] pc-bios/s390-ccw: Remove panics from ECKD IPL path

2024-10-19 Thread jrossi
From: Jared Rossi 

Remove panic-on-error from ECKD block device IPL specific functions so that
error recovery may be possible in the future.

Functions that would previously panic now provide a return code.

Signed-off-by: Jared Rossi 
---
 pc-bios/s390-ccw/bootmap.h |   1 +
 pc-bios/s390-ccw/bootmap.c | 187 +
 2 files changed, 130 insertions(+), 58 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 3cb573b86b..95943441d3 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -16,6 +16,7 @@
 
 typedef uint64_t block_number_t;
 #define NULL_BLOCK_NR 0xULL
+#define ERROR_BLOCK_NR 0xfffeULL
 
 #define FREE_SPACE_FILLER '\xAA'
 
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index af73254acb..b9596e28c7 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -145,14 +145,17 @@ static block_number_t load_eckd_segments(block_number_t 
blk, bool ldipl,
 bool more_data;
 
 memset(_bprs, FREE_SPACE_FILLER, sizeof(_bprs));
-read_block(blk, bprs, "BPRS read failed");
+if (virtio_read(blk, bprs)) {
+puts("BPRS read failed");
+return ERROR_BLOCK_NR;
+}
 
 do {
 more_data = false;
 for (j = 0;; j++) {
 block_nr = gen_eckd_block_num(&bprs[j].xeckd, ldipl);
 if (is_null_block_number(block_nr)) { /* end of chunk */
-break;
+return NULL_BLOCK_NR;
 }
 
 /* we need the updated blockno for the next indirect entry
@@ -163,15 +166,20 @@ static block_number_t load_eckd_segments(block_number_t 
blk, bool ldipl,
 }
 
 /* List directed pointer does not store block size */
-IPL_assert(ldipl || block_size_ok(bprs[j].xeckd.bptr.size),
-   "bad chunk block size");
+if (!ldipl && !block_size_ok(bprs[j].xeckd.bptr.size)) {
+puts("Bad chunk block size");
+return ERROR_BLOCK_NR;
+}
 
 if (!eckd_valid_address(&bprs[j].xeckd, ldipl)) {
 /*
  * If an invalid address is found during LD-IPL then break and
- * retry as CCW
+ * retry as CCW-IPL, otherwise abort on error
  */
-IPL_assert(ldipl, "bad chunk ECKD addr");
+if (!ldipl) {
+puts("Bad chunk ECKD address");
+return ERROR_BLOCK_NR;
+}
 break;
 }
 
@@ -189,7 +197,10 @@ static block_number_t load_eckd_segments(block_number_t 
blk, bool ldipl,
  * I.e. the next ptr must point to the unused memory area
  */
 memset(_bprs, FREE_SPACE_FILLER, sizeof(_bprs));
-read_block(block_nr, bprs, "BPRS continuation read failed");
+if (virtio_read(block_nr, bprs)) {
+puts("BPRS continuation read failed");
+return ERROR_BLOCK_NR;
+}
 more_data = true;
 break;
 }
@@ -198,7 +209,10 @@ static block_number_t load_eckd_segments(block_number_t 
blk, bool ldipl,
  * to memory (address).
  */
 rc = virtio_read_many(block_nr, (void *)(*address), count + 1);
-IPL_assert(rc == 0, "code chunk read failed");
+if (rc != 0) {
+puts("Code chunk read failed");
+return ERROR_BLOCK_NR;
+}
 
 *address += (count + 1) * virtio_get_block_size();
 }
@@ -232,7 +246,10 @@ static int eckd_get_boot_menu_index(block_number_t 
s1b_block_nr)
 
 /* Get Stage1b data */
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
-read_block(s1b_block_nr, s1b, "Cannot read stage1b boot loader");
+if (virtio_read(s1b_block_nr, s1b)) {
+puts("Cannot read stage1b boot loader");
+return -EIO;
+}
 
 memset(_s2, FREE_SPACE_FILLER, sizeof(_s2));
 
@@ -244,7 +261,10 @@ static int eckd_get_boot_menu_index(block_number_t 
s1b_block_nr)
 break;
 }
 
-read_block(cur_block_nr, s2_cur_blk, "Cannot read stage2 boot loader");
+if (virtio_read(cur_block_nr, s2_cur_blk)) {
+puts("Cannot read stage2 boot loader");
+return -EIO;
+}
 
 if (find_zipl_boot_menu_banner(&banner_offset)) {
 /*
@@ -252,8 +272,10 @@ static int eckd_get_boot_menu_index(block_number_t 
s1b_block_nr)
  * possibility of menu data spanning multiple blocks.
  */
 if (prev_block_nr) {
-read_block(prev_block_nr, s2_prev_blk,
-   "Cannot read stage2 boot loader");
+if (virtio_read(prev_block_nr, s2_prev_blk)) {
+puts("Cannot read stage2 boot loader");
+   

[PATCH v5 05/19] pc-bios/s390-ccw: Merge netboot.mak into the main Makefile

2024-10-19 Thread jrossi
From: Jared Rossi 

Now that the netboot code has been merged into the main s390-ccw.img,
it also does not make sense to keep the build rules in a separate
file. Thus let's merge netboot.mak into the main Makefile.

Co-authored by: Thomas Huth 
Signed-off-by: Jared Rossi 
---
 pc-bios/s390-ccw/netboot.mak |  45 -
 pc-bios/s390-ccw/Makefile|  47 ++-
 pc-bios/s390-netboot.img | Bin 67232 -> 0 bytes
 3 files changed, 46 insertions(+), 46 deletions(-)
 delete mode 100644 pc-bios/s390-ccw/netboot.mak
 delete mode 100644 pc-bios/s390-netboot.img

diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
deleted file mode 100644
index 0a24257ff4..00
--- a/pc-bios/s390-ccw/netboot.mak
+++ /dev/null
@@ -1,45 +0,0 @@
-
-# libc files:
-
-LIBC_CFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
- -MMD -MP -MT $@ -MF $(@:%.o=%.d)
-
-CTYPE_OBJS = isdigit.o isxdigit.o toupper.o
-%.o : $(SLOF_DIR)/lib/libc/ctype/%.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-STRING_OBJS = strcat.o strchr.o strrchr.o strcpy.o strlen.o strncpy.o \
- strcmp.o strncmp.o strcasecmp.o strncasecmp.o strstr.o \
- memset.o memcpy.o memmove.o memcmp.o
-%.o : $(SLOF_DIR)/lib/libc/string/%.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-STDLIB_OBJS = atoi.o atol.o strtoul.o strtol.o rand.o malloc.o free.o
-%.o : $(SLOF_DIR)/lib/libc/stdlib/%.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-STDIO_OBJS = sprintf.o snprintf.o vfprintf.o vsnprintf.o vsprintf.o fprintf.o \
-printf.o putc.o puts.o putchar.o stdchnls.o fileno.o
-%.o : $(SLOF_DIR)/lib/libc/stdio/%.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-sbrk.o: $(SLOF_DIR)/slof/sbrk.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-LIBCOBJS := $(STRING_OBJS) $(CTYPE_OBJS) $(STDLIB_OBJS) $(STDIO_OBJS) sbrk.o
-
-libc.a: $(LIBCOBJS)
-   $(call quiet-command,$(AR) -rc $@ $^,Creating static library)
-
-# libnet files:
-
-LIBNETOBJS := args.o dhcp.o dns.o icmpv6.o ipv6.o tcp.o udp.o bootp.o \
- dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o pxelinux.o
-LIBNETCFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
-  -DDHCPARCH=0x1F -MMD -MP -MT $@ -MF $(@:%.o=%.d)
-
-%.o : $(SLOF_DIR)/lib/libnet/%.c
-   $(call quiet-command,$(CC) $(LIBNETCFLAGS) -c -o $@ $<,Compiling)
-
-libnet.a: $(LIBNETOBJS)
-   $(call quiet-command,$(AR) -rc $@ $^,Creating static library)
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index cf6859823a..27cbb354af 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -61,7 +61,52 @@ config-cc.mak: Makefile
$(call cc-option,-march=z900,-march=z10)) 3> config-cc.mak
 -include config-cc.mak
 
-include $(SRC_PATH)/netboot.mak
+# libc files:
+
+LIBC_CFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
+ -MMD -MP -MT $@ -MF $(@:%.o=%.d)
+
+CTYPE_OBJS = isdigit.o isxdigit.o toupper.o
+%.o : $(SLOF_DIR)/lib/libc/ctype/%.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+STRING_OBJS = strcat.o strchr.o strrchr.o strcpy.o strlen.o strncpy.o \
+ strcmp.o strncmp.o strcasecmp.o strncasecmp.o strstr.o \
+ memset.o memcpy.o memmove.o memcmp.o
+%.o : $(SLOF_DIR)/lib/libc/string/%.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+STDLIB_OBJS = atoi.o atol.o strtoul.o strtol.o rand.o malloc.o free.o
+%.o : $(SLOF_DIR)/lib/libc/stdlib/%.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+STDIO_OBJS = sprintf.o snprintf.o vfprintf.o vsnprintf.o vsprintf.o fprintf.o \
+printf.o putc.o puts.o putchar.o stdchnls.o fileno.o
+%.o : $(SLOF_DIR)/lib/libc/stdio/%.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+sbrk.o: $(SLOF_DIR)/slof/sbrk.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+LIBCOBJS := $(STRING_OBJS) $(CTYPE_OBJS) $(STDLIB_OBJS) $(STDIO_OBJS) sbrk.o
+
+libc.a: $(LIBCOBJS)
+   $(call quiet-command,$(AR) -rc $@ $^,Creating static library)
+
+# libnet files:
+
+LIBNETOBJS := args.o dhcp.o dns.o icmpv6.o ipv6.o tcp.o udp.o bootp.o \
+ dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o pxelinux.o
+LIBNETCFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
+  -DDHCPARCH=0x1F -MMD -MP -MT $@ -MF $(@:%.o=%.d)
+
+%.o : $(SLOF_DIR)/lib/libnet/%.c
+   $(call quiet-command,$(CC) $(LIBNETCFLAGS) -c -o $@ $<,Compiling)
+
+libnet.a: $(LIBNETOBJS)
+   $(call quiet-command,$(AR) -rc $@ $^,Creating static library)
+
+# Main targets:
 
 build-all: s390-ccw.img
 
diff --git a/pc-bios/s390-netboot.img b/pc-bios/s390-netboot.img
deleted file mode 100644
index 
6908e49f06801808b826d3a01f88132cf1b2f57c..00

[PATCH v5 03/19] pc-bios/s390-ccw: Link the netboot code into the main s390-ccw.img binary

2024-10-19 Thread jrossi
From: Jared Rossi 

We originally built a separate binary for the netboot code since it
was considered as experimental and we could not be sure that the
necessary SLOF module had been checked out. Time passed, the code
proved its usefulness, and the build system nowadays makes sure that
the SLOF module is checked out if you have a s390x compiler available
for building the s390-ccw bios. So there is no real compelling reason
anymore to keep the netboot code in a separate binary. Linking the
code together with the main s390-ccw.img will make future enhancements
much easier, like supporting more than one boot device.

Co-authored by: Thomas Huth 
Signed-off-by: Jared Rossi 
---
 pc-bios/s390-ccw/netboot.mak | 14 --
 pc-bios/s390-ccw/cio.h   |  2 ++
 pc-bios/s390-ccw/iplb.h  |  4 ++--
 pc-bios/s390-ccw/s390-ccw.h  |  3 +++
 pc-bios/s390-ccw/virtio.h|  1 -
 pc-bios/s390-ccw/bootmap.c   |  2 +-
 pc-bios/s390-ccw/main.c  | 10 +++---
 pc-bios/s390-ccw/netmain.c   | 15 ++-
 pc-bios/s390-ccw/Makefile| 13 +++--
 9 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
index d2b3d8ee74..0a24257ff4 100644
--- a/pc-bios/s390-ccw/netboot.mak
+++ b/pc-bios/s390-ccw/netboot.mak
@@ -1,18 +1,4 @@
 
-NETOBJS := start.o sclp.o cio.o virtio.o virtio-net.o jump2ipl.o netmain.o
-
-LIBNET_INC := -I$(SLOF_DIR)/lib/libnet
-
-NETLDFLAGS := $(LDFLAGS) -Wl,-Ttext=0x780
-
-$(NETOBJS): EXTRA_CFLAGS += $(LIBC_INC) $(LIBNET_INC)
-
-s390-netboot.elf: $(NETOBJS) libnet.a libc.a
-   $(call quiet-command,$(CC) $(NETLDFLAGS) -o $@ $^,Linking)
-
-s390-netboot.img: s390-netboot.elf
-   $(call quiet-command,$(STRIP) --strip-unneeded $< -o $@,Stripping $< 
into)
-
 # libc files:
 
 LIBC_CFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index 8b18153deb..6a5e86ba01 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -361,6 +361,8 @@ typedef struct CcwSearchIdData {
 uint8_t record;
 } __attribute__((packed)) CcwSearchIdData;
 
+extern SubChannelId net_schid;
+
 int enable_mss_facility(void);
 void enable_subchannel(SubChannelId schid);
 uint16_t cu_type(SubChannelId schid);
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index cb6ac8a880..3758698468 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -87,9 +87,9 @@ extern IplParameterBlock iplb 
__attribute__((__aligned__(PAGE_SIZE)));
 struct QemuIplParameters {
 uint8_t  qipl_flags;
 uint8_t  reserved1[3];
-uint64_t netboot_start_addr;
+uint64_t reserved2;
 uint32_t boot_menu_timeout;
-uint8_t  reserved2[12];
+uint8_t  reserved3[12];
 } __attribute__ ((packed));
 typedef struct QemuIplParameters QemuIplParameters;
 
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 6f6d95d170..6abb34e563 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -55,6 +55,9 @@ void write_iplb_location(void);
 unsigned int get_loadparm_index(void);
 void main(void);
 
+/* netmain.c */
+void netmain(void);
+
 /* sclp.c */
 void sclp_print(const char *string);
 void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask);
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 85bd9d1695..6f9a558ff5 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -253,7 +253,6 @@ struct VDev {
 uint8_t scsi_dev_heads;
 bool scsi_device_selected;
 ScsiDevice selected_scsi_device;
-uint64_t netboot_start_addr;
 uint32_t max_transfer;
 uint32_t guest_features[2];
 };
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 3cc79706be..414c3f1b47 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -929,7 +929,7 @@ void zipl_load(void)
 }
 
 if (virtio_get_device_type() == VIRTIO_ID_NET) {
-jump_to_IPL_code(vdev->netboot_start_addr);
+netmain();
 }
 
 ipl_scsi();
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 203df20965..fc44da3161 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -38,8 +38,13 @@ LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
  */
 void write_subsystem_identification(void)
 {
-lowcore->subchannel_id = blk_schid.sch_id;
-lowcore->subchannel_nr = blk_schid.sch_no;
+if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() == VIRTIO_ID_NET) 
{
+lowcore->subchannel_id = net_schid.sch_id;
+lowcore->subchannel_nr = net_schid.sch_no;
+} else {
+lowcore->subchannel_id = blk_schid.sch_id;
+lowcore->subchannel_nr = blk_schid.sch_no;
+}
 lowcore->io_int_parm = 0;
 }
 
@@ -231,7 +236,6 @@ static int virtio_setup(void)
 switch (vdev->senseid.cu_model) {
 case VIRTIO_ID_NET:
 puts("Network boot device detected");
-vdev->netboo

[PATCH v5 16/19] s390x: Rebuild IPLB for SCSI device directly from DIAG308

2024-10-19 Thread jrossi
From: Jared Rossi 

Because virtio-scsi type devices use a non-architected IPLB pbt code they cannot
be set and stored normally. Instead, the IPLB must be rebuilt during re-ipl.

As s390x does not natively support multiple boot devices, the devno field is
used to store the position in the boot order for the device.

Handling the rebuild as part of DIAG308 removes the need to check the devices
for invalid IPLBs later in the IPL.

Signed-off-by: Jared Rossi 
Acked-by: Thomas Huth 
---
 hw/s390x/ipl.h  | 11 --
 include/hw/s390x/ipl/qipl.h |  3 +-
 hw/s390x/ipl.c  | 74 ++---
 pc-bios/s390-ccw/jump2ipl.c | 11 --
 target/s390x/diag.c |  9 -
 5 files changed, 38 insertions(+), 70 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 54eb48fd6e..d7d0b7bfd2 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -24,6 +24,7 @@
 
 void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp);
 void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp);
+void s390_rebuild_iplb(uint16_t index, IplParameterBlock *iplb);
 void s390_ipl_update_diag308(IplParameterBlock *iplb);
 int s390_ipl_prepare_pv_header(Error **errp);
 int s390_ipl_pv_unpack(void);
@@ -65,7 +66,8 @@ struct S390IPLState {
 bool enforce_bios;
 bool iplb_valid;
 bool iplb_valid_pv;
-bool netboot;
+bool rebuilt_iplb;
+uint16_t iplb_index;
 /* reset related properties don't have to be migrated or reset */
 enum s390_reset reset_type;
 int reset_cpu_index;
@@ -172,11 +174,14 @@ static inline bool iplb_valid_pv(IplParameterBlock *iplb)
 
 static inline bool iplb_valid(IplParameterBlock *iplb)
 {
+uint32_t len = be32_to_cpu(iplb->len);
+
 switch (iplb->pbt) {
 case S390_IPL_TYPE_FCP:
-return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_FCP_LEN;
+return len >= S390_IPLB_MIN_FCP_LEN;
 case S390_IPL_TYPE_CCW:
-return be32_to_cpu(iplb->len) >= S390_IPLB_MIN_CCW_LEN;
+return len >= S390_IPLB_MIN_CCW_LEN;
+case S390_IPL_TYPE_QEMU_SCSI:
 default:
 return false;
 }
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 1da4f75aa8..682439 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -29,7 +29,8 @@
  */
 struct QemuIplParameters {
 uint8_t  qipl_flags;
-uint8_t  reserved1[3];
+uint8_t  index;
+uint8_t  reserved1[2];
 uint64_t reserved2;
 uint32_t boot_menu_timeout;
 uint8_t  reserved3[2];
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index ed152a9dd2..6702c6f80f 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -448,7 +448,6 @@ void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t 
*ebcdic_lp)
 
 static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
 {
-S390IPLState *ipl = get_ipl_device();
 CcwDevice *ccw_dev = NULL;
 SCSIDevice *sd;
 int devtype;
@@ -481,9 +480,6 @@ static bool s390_build_iplb(DeviceState *dev_st, 
IplParameterBlock *iplb)
 iplb->ccw.ssid = ccw_dev->sch->ssid & 3;
 break;
 case CCW_DEVTYPE_VIRTIO_NET:
-/* The S390IPLState netboot is true if ANY IPLB may use netboot */
-ipl->netboot = true;
-/* Fall through to CCW_DEVTYPE_VIRTIO case */
 case CCW_DEVTYPE_VIRTIO:
 iplb->len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
 iplb->blk0_len =
@@ -508,6 +504,16 @@ static bool s390_build_iplb(DeviceState *dev_st, 
IplParameterBlock *iplb)
 return false;
 }
 
+void s390_rebuild_iplb(uint16_t dev_index, IplParameterBlock *iplb)
+{
+S390IPLState *ipl = get_ipl_device();
+uint16_t index;
+index = ipl->rebuilt_iplb ? ipl->iplb_index : dev_index;
+
+ipl->rebuilt_iplb = s390_build_iplb(get_boot_device(index), iplb);
+ipl->iplb_index = index;
+}
+
 static bool s390_init_all_iplbs(S390IPLState *ipl)
 {
 int iplb_num = 0;
@@ -564,44 +570,6 @@ static bool s390_init_all_iplbs(S390IPLState *ipl)
 return iplb_num;
 }
 
-static bool is_virtio_ccw_device_of_type(IplParameterBlock *iplb,
- int virtio_id)
-{
-uint8_t cssid;
-uint8_t ssid;
-uint16_t devno;
-uint16_t schid;
-SubchDev *sch = NULL;
-
-if (iplb->pbt != S390_IPL_TYPE_CCW) {
-return false;
-}
-
-devno = be16_to_cpu(iplb->ccw.devno);
-ssid = iplb->ccw.ssid & 3;
-
-for (schid = 0; schid < MAX_SCHID; schid++) {
-for (cssid = 0; cssid < MAX_CSSID; cssid++) {
-sch = css_find_subch(1, cssid, ssid, schid);
-
-if (sch && sch->devno == devno) {
-return sch->id.cu_model == virtio_id;
-}
-}
-}
-return false;
-}
-
-static bool is_virtio_net_device(IplParameterBlock *iplb)
-{
-return is_virtio_ccw_device_of_type(iplb, VIRTIO_ID_NET);
-}
-
-static bool is_virtio_scsi_device(IplParameterBlock *iplb)
-{
-return is_virtio_cc

[PATCH v5 14/19] s390x: Add individual loadparm assignment to CCW device

2024-10-19 Thread jrossi
From: Jared Rossi 

Add a loadparm property to the VirtioCcwDevice object so that different
loadparms can be defined on a per-device basis for CCW boot devices.

The machine/global loadparm is still supported. If both a global and per-device
loadparm are defined, the per-device value will override the global value for
that device, but any other devices that do not specify a per-device loadparm
will still use the global loadparm.

It is invalid to assign a loadparm to a non-boot device.

Signed-off-by: Jared Rossi 
---
 hw/s390x/ccw-device.h   |  2 ++
 hw/s390x/ipl.h  |  3 +-
 include/hw/s390x/ipl/qipl.h |  1 +
 hw/s390x/ccw-device.c   | 46 +
 hw/s390x/ipl.c  | 68 ++---
 hw/s390x/s390-virtio-ccw.c  | 18 +-
 hw/s390x/sclp.c |  9 ++---
 pc-bios/s390-ccw/main.c | 10 --
 8 files changed, 102 insertions(+), 55 deletions(-)

diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 5feeb0ee7a..1e1737c0f3 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -26,6 +26,8 @@ struct CcwDevice {
 CssDevId dev_id;
 /* The actual busid of the virtual subchannel. */
 CssDevId subch_id;
+/* If set, use this loadparm value when device is boot target */
+uint8_t loadparm[8];
 };
 typedef struct CcwDevice CcwDevice;
 
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index fa394c339d..b670bad551 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -21,7 +21,8 @@
 
 #define DIAG308_FLAGS_LP_VALID 0x80
 
-int s390_ipl_set_loadparm(uint8_t *loadparm);
+void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp);
+void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp);
 void s390_ipl_update_diag308(IplParameterBlock *iplb);
 int s390_ipl_prepare_pv_header(Error **errp);
 int s390_ipl_pv_unpack(void);
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 0ef04af027..b67d2ae061 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -18,6 +18,7 @@
 
 #define QIPL_ADDRESS  0xcc
 #define LOADPARM_LEN8
+#define NO_LOADPARM "\0\0\0\0\0\0\0\0"
 
 /*
  * The QEMU IPL Parameters will be stored at absolute address
diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index 14c24e3890..230cc09e03 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -13,6 +13,10 @@
 #include "ccw-device.h"
 #include "hw/qdev-properties.h"
 #include "qemu/module.h"
+#include "ipl.h"
+#include "qapi/visitor.h"
+#include "qemu/ctype.h"
+#include "qapi/error.h"
 
 static void ccw_device_refill_ids(CcwDevice *dev)
 {
@@ -37,10 +41,52 @@ static bool ccw_device_realize(CcwDevice *dev, Error **errp)
 return true;
 }
 
+static void ccw_device_get_loadparm(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+CcwDevice *dev = CCW_DEVICE(obj);
+char *str = g_strndup((char *) dev->loadparm, sizeof(dev->loadparm));
+
+visit_type_str(v, name, &str, errp);
+g_free(str);
+}
+
+static void ccw_device_set_loadparm(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+CcwDevice *dev = CCW_DEVICE(obj);
+char *val;
+int index;
+
+index = object_property_get_int(obj, "bootindex", NULL);
+
+if (index < 0) {
+error_setg(errp, "LOADPARM is only valid for boot devices!");
+}
+
+if (!visit_type_str(v, name, &val, errp)) {
+return;
+}
+
+s390_ipl_fmt_loadparm(dev->loadparm, val, errp);
+}
+
+static const PropertyInfo ccw_loadparm = {
+.name  = "ccw_loadparm",
+.description = "Up to 8 chars in set of [A-Za-z0-9. ] to pass"
+" to the guest loader/kernel",
+.get = ccw_device_get_loadparm,
+.set = ccw_device_set_loadparm,
+};
+
 static Property ccw_device_properties[] = {
 DEFINE_PROP_CSS_DEV_ID("devno", CcwDevice, devno),
 DEFINE_PROP_CSS_DEV_ID_RO("dev_id", CcwDevice, dev_id),
 DEFINE_PROP_CSS_DEV_ID_RO("subch_id", CcwDevice, subch_id),
+DEFINE_PROP("loadparm", CcwDevice, loadparm, ccw_loadparm,
+typeof(uint8_t[8])),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 8c490eeb52..656996b500 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -34,6 +34,7 @@
 #include "qemu/config-file.h"
 #include "qemu/cutils.h"
 #include "qemu/option.h"
+#include "qemu/ctype.h"
 #include "standard-headers/linux/virtio_ids.h"
 
 #define KERN_IMAGE_START0x01UL
@@ -397,12 +398,43 @@ static CcwDevice *s390_get_ccw_device(DeviceState 
*dev_st, int *devtype)
 return ccw_dev;
 }
 
+void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp)
+{
+int i;
+
+/* Initialize the loadparm with spaces */
+memset(loadparm, ' ', LOADPARM_LEN);
+for (i = 0; i < LOADPARM_LEN && str[i]; i++) {
+   

[PATCH v5 17/19] pc-bios/s390x: Enable multi-device boot loop

2024-10-19 Thread jrossi
From: Jared Rossi 

Allow attempts to boot from multiple IPL devices. If the first device fails to
IPL, select the pre-built IPLB for the next device in the boot order and attempt
to IPL from it. Continue this process until IPL is successful or there are no
devices left to try.

Signed-off-by: Jared Rossi 
---
 pc-bios/s390-ccw/iplb.h | 24 
 pc-bios/s390-ccw/jump2ipl.c |  7 +++---
 pc-bios/s390-ccw/main.c | 45 +++--
 pc-bios/s390-ccw/netmain.c  |  2 +-
 4 files changed, 57 insertions(+), 21 deletions(-)

diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 16643f5879..08f259ff31 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -17,9 +17,11 @@
 #endif
 
 #include 
+#include 
 
 extern QemuIplParameters qipl;
 extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
+extern bool have_iplb;
 
 #define S390_IPL_TYPE_FCP 0x00
 #define S390_IPL_TYPE_CCW 0x02
@@ -49,4 +51,26 @@ static inline bool set_iplb(IplParameterBlock *iplb)
 return manage_iplb(iplb, false);
 }
 
+/*
+ * The IPL started on the device, but failed in some way.  If the IPLB chain
+ * still has more devices left to try, use the next device in order.
+ */
+static inline bool load_next_iplb(void)
+{
+IplParameterBlock *next_iplb;
+
+if (qipl.chain_len < 1) {
+return false;
+}
+
+qipl.index++;
+next_iplb = (IplParameterBlock *) qipl.next_iplb;
+memcpy(&iplb, next_iplb, sizeof(IplParameterBlock));
+
+qipl.chain_len--;
+qipl.next_iplb = qipl.next_iplb + sizeof(IplParameterBlock);
+
+return true;
+}
+
 #endif /* IPLB_H */
diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
index 99d18947d1..86321d0f46 100644
--- a/pc-bios/s390-ccw/jump2ipl.c
+++ b/pc-bios/s390-ccw/jump2ipl.c
@@ -45,9 +45,10 @@ int jump_to_IPL_code(uint64_t address)
  */
 if (iplb.pbt == S390_IPL_TYPE_QEMU_SCSI) {
 iplb.devno = qipl.index;
-if (!set_iplb(&iplb)) {
-panic("Failed to set IPLB");
-}
+}
+
+if (have_iplb && !set_iplb(&iplb)) {
+panic("Failed to set IPLB");
 }
 
 /*
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index ab4709e16e..a4d1c05aac 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -23,7 +23,7 @@ static SubChannelId blk_schid = { .one = 1 };
 static char loadparm_str[LOADPARM_LEN + 1];
 QemuIplParameters qipl;
 IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
-static bool have_iplb;
+bool have_iplb;
 static uint16_t cutype;
 LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
 
@@ -55,6 +55,12 @@ void write_iplb_location(void)
 }
 }
 
+static void copy_qipl(void)
+{
+QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
+memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
+}
+
 unsigned int get_loadparm_index(void)
 {
 return atoi(loadparm_str);
@@ -152,6 +158,7 @@ static void menu_setup(void)
 
 /* If loadparm was set to any other value, then do not enable menu */
 if (memcmp(loadparm_str, LOADPARM_EMPTY, LOADPARM_LEN) != 0) {
+menu_set_parms(qipl.qipl_flags & ~BOOT_MENU_FLAG_MASK, 0);
 return;
 }
 
@@ -183,7 +190,6 @@ static void css_setup(void)
 static void boot_setup(void)
 {
 char lpmsg[] = "LOADPARM=[]\n";
-have_iplb = store_iplb(&iplb);
 
 if (memcmp(iplb.loadparm, NO_LOADPARM, LOADPARM_LEN) != 0) {
 ebcdic_to_ascii((char *) iplb.loadparm, loadparm_str, LOADPARM_LEN);
@@ -191,6 +197,10 @@ static void boot_setup(void)
 sclp_get_loadparm_ascii(loadparm_str);
 }
 
+if (have_iplb) {
+menu_setup();
+}
+
 memcpy(lpmsg + 10, loadparm_str, 8);
 puts(lpmsg);
 
@@ -208,6 +218,7 @@ static bool find_boot_device(void)
 
 switch (iplb.pbt) {
 case S390_IPL_TYPE_CCW:
+vdev->scsi_device_selected = false;
 debug_print_int("device no. ", iplb.ccw.devno);
 blk_schid.ssid = iplb.ccw.ssid & 0x3;
 debug_print_int("ssid ", blk_schid.ssid);
@@ -231,15 +242,8 @@ static bool find_boot_device(void)
 static int virtio_setup(void)
 {
 VDev *vdev = virtio_get_device();
-QemuIplParameters *early_qipl = (QemuIplParameters *)QIPL_ADDRESS;
 int ret;
 
-memcpy(&qipl, early_qipl, sizeof(QemuIplParameters));
-
-if (have_iplb) {
-menu_setup();
-}
-
 switch (vdev->senseid.cu_model) {
 case VIRTIO_ID_NET:
 puts("Network boot device detected");
@@ -271,10 +275,9 @@ static void ipl_boot_device(void)
 dasd_ipl(blk_schid, cutype);
 break;
 case CU_TYPE_VIRTIO:
-if (virtio_setup()) {
-return;/* Only returns in case of errors */
+if (virtio_setup() == 0) {
+zipl_load();
 }
-zipl_load();
 break;
 default:
 printf("Attempting to boot from unexpected device type 0x%X\n", 
cutype);
@@ -307,14 +310,22 @@

[PATCH v5 12/19] pc-bios/s390-ccw: Enable failed IPL to return after error

2024-10-19 Thread jrossi
From: Jared Rossi 

Remove panic-on-error from IPL functions such that a return code is propagated
back to the main IPL calling function (rather than terminating immediately),
which facilitates possible error recovery in the future.

A select few panics remain, which indicate fatal non-devices errors that must
result in termination.

Signed-off-by: Jared Rossi 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/s390-ccw.h  |  2 +-
 pc-bios/s390-ccw/virtio.h|  2 +-
 pc-bios/s390-ccw/bootmap.c   | 53 ++
 pc-bios/s390-ccw/cio.c   |  3 +-
 pc-bios/s390-ccw/jump2ipl.c  |  5 ++-
 pc-bios/s390-ccw/main.c  | 32 +---
 pc-bios/s390-ccw/virtio-blkdev.c |  2 +-
 pc-bios/s390-ccw/virtio.c| 65 +---
 8 files changed, 108 insertions(+), 56 deletions(-)

diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 344ad15655..6cdce3e5e5 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -78,7 +78,7 @@ void zipl_load(void);
 
 /* jump2ipl.c */
 void write_reset_psw(uint64_t psw);
-void jump_to_IPL_code(uint64_t address);
+int jump_to_IPL_code(uint64_t address);
 void jump_to_low_kernel(void);
 
 /* menu.c */
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 6f9a558ff5..9faf3986b1 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -274,7 +274,7 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags);
 int vr_poll(VRing *vr);
 int vring_wait_reply(void);
 int virtio_run(VDev *vdev, int vqid, VirtioCmd *cmd);
-void virtio_setup_ccw(VDev *vdev);
+int virtio_setup_ccw(VDev *vdev);
 
 int virtio_net_init(void *mac_addr);
 
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 95ef9104d0..56f2f75640 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -62,15 +62,34 @@ static void *s2_prev_blk = _s2;
 static void *s2_cur_blk = _s2 + MAX_SECTOR_SIZE;
 static void *s2_next_blk = _s2 + MAX_SECTOR_SIZE * 2;
 
-static inline void verify_boot_info(BootInfo *bip)
+static inline int verify_boot_info(BootInfo *bip)
 {
-IPL_assert(magic_match(bip->magic, ZIPL_MAGIC), "No zIPL sig in BootInfo");
-IPL_assert(bip->version == BOOT_INFO_VERSION, "Wrong zIPL version");
-IPL_assert(bip->bp_type == BOOT_INFO_BP_TYPE_IPL, "DASD is not for IPL");
-IPL_assert(bip->dev_type == BOOT_INFO_DEV_TYPE_ECKD, "DASD is not ECKD");
-IPL_assert(bip->flags == BOOT_INFO_FLAGS_ARCH, "Not for this arch");
-IPL_assert(block_size_ok(bip->bp.ipl.bm_ptr.eckd.bptr.size),
-   "Bad block size in zIPL section of the 1st record.");
+if (!magic_match(bip->magic, ZIPL_MAGIC)) {
+puts("No zIPL sig in BootInfo");
+return -EINVAL;
+}
+if (bip->version != BOOT_INFO_VERSION) {
+puts("Wrong zIPL version");
+return -EINVAL;
+}
+if (bip->bp_type != BOOT_INFO_BP_TYPE_IPL) {
+puts("DASD is not for IPL");
+return -ENODEV;
+}
+if (bip->dev_type != BOOT_INFO_DEV_TYPE_ECKD) {
+puts("DASD is not ECKD");
+return -ENODEV;
+}
+if (bip->flags != BOOT_INFO_FLAGS_ARCH) {
+puts("Not for this arch");
+return -EINVAL;
+}
+if (!block_size_ok(bip->bp.ipl.bm_ptr.eckd.bptr.size)) {
+puts("Bad block size in zIPL section of 1st record");
+return -EINVAL;
+}
+
+return 0;
 }
 
 static void eckd_format_chs(ExtEckdBlockPtr *ptr,  bool ldipl,
@@ -367,8 +386,8 @@ static int run_eckd_boot_script(block_number_t bmt_block_nr,
 puts("Unknown script entry type");
 return -EINVAL;
 }
-write_reset_psw(bms->entry[i].address.load_address); /* no return */
-jump_to_IPL_code(0); /* no return */
+write_reset_psw(bms->entry[i].address.load_address);
+jump_to_IPL_code(0);
 return -1;
 }
 
@@ -1067,16 +1086,19 @@ void zipl_load(void)
 
 if (vdev->is_cdrom) {
 ipl_iso_el_torito();
-panic("\n! Cannot IPL this ISO image !\n");
+puts("Failed to IPL this ISO image!");
+return;
 }
 
 if (virtio_get_device_type() == VIRTIO_ID_NET) {
 netmain();
-panic("\n! Cannot IPL from this network !\n");
+puts("Failed to IPL from this network!");
+return;
 }
 
 if (ipl_scsi()) {
-panic("\n! Cannot IPL this SCSI device !\n");
+puts("Failed to IPL from this SCSI device!");
+return;
 }
 
 switch (virtio_get_device_type()) {
@@ -1087,8 +1109,9 @@ void zipl_load(void)
 zipl_load_vscsi();
 break;
 default:
-panic("\n! Unknown IPL device type !\n");
+puts("Unknown IPL device type!");
+return;
 }
 
-puts("zIPL load failed.");
+puts("zIPL load failed!");
 }
diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
index 7b09a38c96..5d543da73f 100644
--- a/pc-bios/s390-ccw/cio.c
+++ b/pc-bios/s390-ccw/cio.c
@@ -59,7 +59,8 @@ u

[PATCH v5 04/19] hw/s390x: Remove the possibility to load the s390-netboot.img binary

2024-10-19 Thread jrossi
From: Jared Rossi 

Since the netboot code has now been merged into the main s390-ccw.img
binary, we don't need the separate s390-netboot.img anymore. Remove
it and the code that was responsible for loading it.

Co-authored by: Thomas Huth 
Signed-off-by: Jared Rossi 
---
 hw/s390x/ipl.h | 12 +++--
 hw/s390x/ipl.c | 55 --
 hw/s390x/s390-virtio-ccw.c | 10 ++-
 pc-bios/meson.build|  1 -
 4 files changed, 6 insertions(+), 72 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 57cd125769..b2105b616a 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -134,11 +134,8 @@ void s390_ipl_clear_reset_request(void);
 /*
  * The QEMU IPL Parameters will be stored at absolute address
  * 204 (0xcc) which means it is 32-bit word aligned but not
- * double-word aligned.
- * Placement of data fields in this area must account for
- * their alignment needs. E.g., netboot_start_address must
- * have an offset of 4 + n * 8 bytes within the struct in order
- * to keep it double-word aligned.
+ * double-word aligned. Placement of 64-bit data fields in this
+ * area must account for their alignment needs.
  * The total size of the struct must never exceed 28 bytes.
  * This definition must be kept in sync with the definition
  * in pc-bios/s390-ccw/iplb.h.
@@ -146,9 +143,9 @@ void s390_ipl_clear_reset_request(void);
 struct QemuIplParameters {
 uint8_t  qipl_flags;
 uint8_t  reserved1[3];
-uint64_t netboot_start_addr;
+uint64_t reserved2;
 uint32_t boot_menu_timeout;
-uint8_t  reserved2[12];
+uint8_t  reserved3[12];
 } QEMU_PACKED;
 typedef struct QemuIplParameters QemuIplParameters;
 
@@ -178,7 +175,6 @@ struct S390IPLState {
 char *initrd;
 char *cmdline;
 char *firmware;
-char *netboot_fw;
 uint8_t cssid;
 uint8_t ssid;
 uint16_t devno;
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 5f60977ceb..8c490eeb52 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -288,7 +288,6 @@ static Property s390_ipl_properties[] = {
 DEFINE_PROP_STRING("initrd", S390IPLState, initrd),
 DEFINE_PROP_STRING("cmdline", S390IPLState, cmdline),
 DEFINE_PROP_STRING("firmware", S390IPLState, firmware),
-DEFINE_PROP_STRING("netboot_fw", S390IPLState, netboot_fw),
 DEFINE_PROP_BOOL("enforce_bios", S390IPLState, enforce_bios, false),
 DEFINE_PROP_BOOL("iplbext_migration", S390IPLState, iplbext_migration,
  true),
@@ -480,56 +479,6 @@ int s390_ipl_set_loadparm(uint8_t *loadparm)
 return -1;
 }
 
-static int load_netboot_image(Error **errp)
-{
-MachineState *ms = MACHINE(qdev_get_machine());
-S390IPLState *ipl = get_ipl_device();
-char *netboot_filename;
-MemoryRegion *sysmem =  get_system_memory();
-MemoryRegion *mr = NULL;
-void *ram_ptr = NULL;
-int img_size = -1;
-
-mr = memory_region_find(sysmem, 0, 1).mr;
-if (!mr) {
-error_setg(errp, "Failed to find memory region at address 0");
-return -1;
-}
-
-ram_ptr = memory_region_get_ram_ptr(mr);
-if (!ram_ptr) {
-error_setg(errp, "No RAM found");
-goto unref_mr;
-}
-
-netboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, ipl->netboot_fw);
-if (netboot_filename == NULL) {
-error_setg(errp, "Could not find network bootloader '%s'",
-   ipl->netboot_fw);
-goto unref_mr;
-}
-
-img_size = load_elf_ram(netboot_filename, NULL, NULL, NULL,
-&ipl->start_addr,
-NULL, NULL, NULL, 1, EM_S390, 0, 0, NULL,
-false);
-
-if (img_size < 0) {
-img_size = load_image_size(netboot_filename, ram_ptr, ms->ram_size);
-ipl->start_addr = KERN_IMAGE_START;
-}
-
-if (img_size < 0) {
-error_setg(errp, "Failed to load network bootloader");
-}
-
-g_free(netboot_filename);
-
-unref_mr:
-memory_region_unref(mr);
-return img_size;
-}
-
 static bool is_virtio_ccw_device_of_type(IplParameterBlock *iplb,
  int virtio_id)
 {
@@ -754,10 +703,6 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
 ipl->iplb_valid = s390_gen_initial_iplb(ipl);
 }
 }
-if (ipl->netboot) {
-load_netboot_image(&error_fatal);
-ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
-}
 s390_ipl_set_boot_menu(ipl);
 s390_ipl_prepare_qipl(cpu);
 }
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 5aa8d207a3..529e53f308 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -197,11 +197,10 @@ static void s390_memory_init(MemoryRegion *ram)
 static void s390_init_ipl_dev(const char *kernel_filename,
   const char *kernel_cmdline,
   const char *initrd_filename, const char 
*firmware,
-  const char *netb

[PATCH v5 19/19] tests/qtest: Add s390x boot order tests to cdrom-test.c

2024-10-19 Thread jrossi
From: Jared Rossi 

Add two new qtests to verify that a valid IPL device can successfully boot after
failed IPL attempts from one or more invalid devices.

cdrom-test/as-fallback-device: Defines the primary boot target as a device that
is invalid for IPL and a second boot target that is valid for IPL. Ensures that
the valid device will be selected after the initial failed IPL.

cdrom-test/as-last-option: Defines the maximum number of boot devices (8)
where only the final entry in the boot order is valid. Ensures that a valid
device will be selected even after multiple failed IPL attempts from both
virtio-blk and virtio-scsi device types.

Signed-off-by: Jared Rossi 
---
 tests/qtest/cdrom-test.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index 9d72b24e4b..c86725a511 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -213,6 +213,30 @@ static void add_s390x_tests(void)
 "-drive driver=null-co,read-zeroes=on,if=none,id=d1 "
 "-device virtio-blk,drive=d2,bootindex=1 "
 "-drive if=none,id=d2,media=cdrom,file=", test_cdboot);
+qtest_add_data_func("cdrom/boot/as-fallback-device",
+"-device virtio-serial -device virtio-scsi "
+"-device virtio-blk,drive=d1,bootindex=1 "
+"-drive driver=null-co,read-zeroes=on,if=none,id=d1 "
+"-device virtio-blk,drive=d2,bootindex=2 "
+"-drive if=none,id=d2,media=cdrom,file=", test_cdboot);
+qtest_add_data_func("cdrom/boot/as-last-option",
+"-device virtio-serial -device virtio-scsi "
+"-device virtio-blk,drive=d1,bootindex=1 "
+"-drive driver=null-co,read-zeroes=on,if=none,id=d1 "
+"-device virtio-blk,drive=d2,bootindex=2 "
+"-drive driver=null-co,read-zeroes=on,if=none,id=d2 "
+"-device virtio-blk,drive=d3,bootindex=3 "
+"-drive driver=null-co,read-zeroes=on,if=none,id=d3 "
+"-device scsi-hd,drive=d4,bootindex=4 "
+"-drive driver=null-co,read-zeroes=on,if=none,id=d4 "
+"-device scsi-hd,drive=d5,bootindex=5 "
+"-drive driver=null-co,read-zeroes=on,if=none,id=d5 "
+"-device virtio-blk,drive=d6,bootindex=6 "
+"-drive driver=null-co,read-zeroes=on,if=none,id=d6 "
+"-device scsi-hd,drive=d7,bootindex=7 "
+"-drive driver=null-co,read-zeroes=on,if=none,id=d7 "
+"-device scsi-cd,drive=d8,bootindex=8 "
+"-drive if=none,id=d8,media=cdrom,file=", test_cdboot);
 if (qtest_has_device("x-terminal3270")) {
 qtest_add_data_func("cdrom/boot/without-bootindex",
 "-device virtio-scsi -device virtio-serial "
-- 
2.45.1




[PATCH v5 07/19] pc-bios/s390-ccw: Remove panics from ISO IPL path

2024-10-19 Thread jrossi
From: Jared Rossi 

Remove panic-on-error from IPL ISO El Torito specific functions so that error
recovery may be possible in the future.

Functions that would previously panic now provide a return code.

Signed-off-by: Jared Rossi 
---
 pc-bios/s390-ccw/bootmap.h  | 15 +++
 pc-bios/s390-ccw/s390-ccw.h |  1 +
 pc-bios/s390-ccw/bootmap.c  | 87 -
 3 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 4a7d8a91f1..3cb573b86b 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -385,17 +385,14 @@ static inline uint32_t iso_733_to_u32(uint64_t x)
 
 #define ISO_PRIMARY_VD_SECTOR 16
 
-static inline void read_iso_sector(uint32_t block_offset, void *buf,
-   const char *errmsg)
-{
-IPL_assert(virtio_read_many(block_offset, buf, 1) == 0, errmsg);
-}
-
-static inline void read_iso_boot_image(uint32_t block_offset, void *load_addr,
+static inline int read_iso_boot_image(uint32_t block_offset, void *load_addr,
uint32_t blks_to_load)
 {
-IPL_assert(virtio_read_many(block_offset, load_addr, blks_to_load) == 0,
-   "Failed to read boot image!");
+if (virtio_read_many(block_offset, load_addr, blks_to_load)) {
+puts("Failed to read boot image!");
+return -1;
+}
+return 0;
 }
 
 #define ISO9660_MAX_DIR_DEPTH 8
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 6abb34e563..3e844abd71 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -30,6 +30,7 @@ typedef unsigned long long u64;
 #define EIO 1
 #define EBUSY   2
 #define ENODEV  3
+#define EINVAL  4
 
 #ifndef MIN
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 414c3f1b47..af73254acb 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -678,8 +678,10 @@ static bool is_iso_bc_entry_compatible(IsoBcSection *s)
 if (s->unused || !s->sector_count) {
 return false;
 }
-read_iso_sector(bswap32(s->load_rba), magic_sec,
-"Failed to read image sector 0");
+if (virtio_read(bswap32(s->load_rba), magic_sec)) {
+puts("Failed to read image sector 0");
+return false;
+}
 
 /* Checking bytes 8 - 32 for S390 Linux magic */
 return !memcmp(magic_sec + 8, linux_s390_magic, 24);
@@ -692,28 +694,35 @@ static uint32_t sec_offset[ISO9660_MAX_DIR_DEPTH];
 /* Remained directory space in bytes */
 static uint32_t dir_rem[ISO9660_MAX_DIR_DEPTH];
 
-static inline uint32_t iso_get_file_size(uint32_t load_rba)
+static inline long iso_get_file_size(uint32_t load_rba)
 {
 IsoVolDesc *vd = (IsoVolDesc *)sec;
 IsoDirHdr *cur_record = &vd->vd.primary.rootdir;
 uint8_t *temp = sec + ISO_SECTOR_SIZE;
 int level = 0;
 
-read_iso_sector(ISO_PRIMARY_VD_SECTOR, sec,
-"Failed to read ISO primary descriptor");
+if (virtio_read(ISO_PRIMARY_VD_SECTOR, sec)) {
+puts("Failed to read ISO primary descriptor");
+return -EIO;
+}
+
 sec_loc[0] = iso_733_to_u32(cur_record->ext_loc);
 dir_rem[0] = 0;
 sec_offset[0] = 0;
 
 while (level >= 0) {
-IPL_assert(sec_offset[level] <= ISO_SECTOR_SIZE,
-   "Directory tree structure violation");
+if (sec_offset[level] > ISO_SECTOR_SIZE) {
+puts("Directory tree structure violation");
+return -EIO;
+}
 
 cur_record = (IsoDirHdr *)(temp + sec_offset[level]);
 
 if (sec_offset[level] == 0) {
-read_iso_sector(sec_loc[level], temp,
-"Failed to read ISO directory");
+if (virtio_read(sec_loc[level], temp)) {
+puts("Failed to read ISO directory");
+return -EIO;
+}
 if (dir_rem[level] == 0) {
 /* Skip self and parent records */
 dir_rem[level] = iso_733_to_u32(cur_record->data_len) -
@@ -758,8 +767,10 @@ static inline uint32_t iso_get_file_size(uint32_t load_rba)
 if (dir_rem[level] == 0) {
 /* Nothing remaining */
 level--;
-read_iso_sector(sec_loc[level], temp,
-"Failed to read ISO directory");
+if (virtio_read(sec_loc[level], temp)) {
+puts("Failed to read ISO directory");
+return -EIO;
+}
 }
 }
 
@@ -774,19 +785,24 @@ static void load_iso_bc_entry(IsoBcSection *load)
  * is padded and ISO_SECTOR_SIZE bytes aligned
  */
 uint32_t blks_to_load = bswap16(s.sector_count) >> ET_SECTOR_SHIFT;
-uint32_t real_size = iso_get_file_size(bswap32(s.load_rba));
+long real_size = iso_get_file_size(bswap32(s.load_rba));
 
-if (real_size) {
+if (real_size > 0) {
 /* Round 

[PATCH v5 15/19] hw/s390x: Build an IPLB for each boot device

2024-10-19 Thread jrossi
From: Jared Rossi 

Build an IPLB for any device with a bootindex (up to a maximum of 8 devices).

The IPLB chain is placed immediately before the BIOS in memory. Because this
is not a fixed address, the location of the next IPLB and number of remaining
boot devices is stored in the QIPL global variable for possible later access by
the guest during IPL.

Signed-off-by: Jared Rossi 
---
 hw/s390x/ipl.h  |   1 +
 include/hw/s390x/ipl/qipl.h |   4 +-
 hw/s390x/ipl.c  | 129 
 3 files changed, 105 insertions(+), 29 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index b670bad551..54eb48fd6e 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -20,6 +20,7 @@
 #include "qom/object.h"
 
 #define DIAG308_FLAGS_LP_VALID 0x80
+#define MAX_BOOT_DEVS 8 /* Max number of devices that may have a bootindex */
 
 void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t *ebcdic_lp);
 void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp);
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index b67d2ae061..1da4f75aa8 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -32,7 +32,9 @@ struct QemuIplParameters {
 uint8_t  reserved1[3];
 uint64_t reserved2;
 uint32_t boot_menu_timeout;
-uint8_t  reserved3[12];
+uint8_t  reserved3[2];
+uint16_t chain_len;
+uint64_t next_iplb;
 } QEMU_PACKED;
 typedef struct QemuIplParameters QemuIplParameters;
 
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 656996b500..ed152a9dd2 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -56,6 +56,13 @@ static bool iplb_extended_needed(void *opaque)
 return ipl->iplbext_migration;
 }
 
+/* Place the IPLB chain immediately before the BIOS in memory */
+static uint64_t find_iplb_chain_addr(uint64_t bios_addr, uint16_t count)
+{
+return (bios_addr & TARGET_PAGE_MASK)
+- (count * sizeof(IplParameterBlock));
+}
+
 static const VMStateDescription vmstate_iplb_extended = {
 .name = "ipl/iplb_extended",
 .version_id = 0,
@@ -398,6 +405,17 @@ static CcwDevice *s390_get_ccw_device(DeviceState *dev_st, 
int *devtype)
 return ccw_dev;
 }
 
+static uint64_t s390_ipl_map_iplb_chain(IplParameterBlock *iplb_chain)
+{
+S390IPLState *ipl = get_ipl_device();
+uint16_t count = ipl->qipl.chain_len;
+uint64_t len = sizeof(IplParameterBlock) * count;
+uint64_t chain_addr = find_iplb_chain_addr(ipl->bios_start_addr, count);
+
+cpu_physical_memory_write(chain_addr, iplb_chain, len);
+return chain_addr;
+}
+
 void s390_ipl_fmt_loadparm(uint8_t *loadparm, char *str, Error **errp)
 {
 int i;
@@ -428,54 +446,51 @@ void s390_ipl_convert_loadparm(char *ascii_lp, uint8_t 
*ebcdic_lp)
 }
 }
 
-static bool s390_gen_initial_iplb(S390IPLState *ipl)
+static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
 {
-DeviceState *dev_st;
+S390IPLState *ipl = get_ipl_device();
 CcwDevice *ccw_dev = NULL;
 SCSIDevice *sd;
 int devtype;
 uint8_t *lp;
 
-dev_st = get_boot_device(0);
-if (dev_st) {
-ccw_dev = s390_get_ccw_device(dev_st, &devtype);
-}
-
 /*
  * Currently allow IPL only from CCW devices.
  */
+ccw_dev = s390_get_ccw_device(dev_st, &devtype);
 if (ccw_dev) {
 lp = ccw_dev->loadparm;
 
 switch (devtype) {
 case CCW_DEVTYPE_SCSI:
 sd = SCSI_DEVICE(dev_st);
-ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN);
-ipl->iplb.blk0_len =
+iplb->len = cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN);
+iplb->blk0_len =
 cpu_to_be32(S390_IPLB_MIN_QEMU_SCSI_LEN - 
S390_IPLB_HEADER_LEN);
-ipl->iplb.pbt = S390_IPL_TYPE_QEMU_SCSI;
-ipl->iplb.scsi.lun = cpu_to_be32(sd->lun);
-ipl->iplb.scsi.target = cpu_to_be16(sd->id);
-ipl->iplb.scsi.channel = cpu_to_be16(sd->channel);
-ipl->iplb.scsi.devno = cpu_to_be16(ccw_dev->sch->devno);
-ipl->iplb.scsi.ssid = ccw_dev->sch->ssid & 3;
+iplb->pbt = S390_IPL_TYPE_QEMU_SCSI;
+iplb->scsi.lun = cpu_to_be32(sd->lun);
+iplb->scsi.target = cpu_to_be16(sd->id);
+iplb->scsi.channel = cpu_to_be16(sd->channel);
+iplb->scsi.devno = cpu_to_be16(ccw_dev->sch->devno);
+iplb->scsi.ssid = ccw_dev->sch->ssid & 3;
 break;
 case CCW_DEVTYPE_VFIO:
-ipl->iplb.len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
-ipl->iplb.pbt = S390_IPL_TYPE_CCW;
-ipl->iplb.ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
-ipl->iplb.ccw.ssid = ccw_dev->sch->ssid & 3;
+iplb->len = cpu_to_be32(S390_IPLB_MIN_CCW_LEN);
+iplb->pbt = S390_IPL_TYPE_CCW;
+iplb->ccw.devno = cpu_to_be16(ccw_dev->sch->devno);
+iplb->ccw.ssid = ccw_dev->sch->ssid & 3;
 break;
  

Ping: [PATCH v2] net/vmnet: Pad short Ethernet frames

2024-10-19 Thread William Hooper
On Sat, Aug 17, 2024 at 11:33 PM William Hooper  wrote:
> At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> host's ARP replies, to the minimum size (60 bytes before the frame check
> sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> drivers may drop them with "frame too short" errors.
>
> This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> and net/slirp.c. Thanks to Bin Meng and Philippe Mathieu-Daudé for
> reviewing an earlier version.
>
> Signed-off-by: William Hooper 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> ---
>  net/vmnet-common.m | 22 +++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> index 30c4e53c13..bce1cc590d 100644
> --- a/net/vmnet-common.m
> +++ b/net/vmnet-common.m
> @@ -18,6 +18,7 @@
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "sysemu/runstate.h"
> +#include "net/eth.h"
>
>  #include 
>  #include 
> @@ -147,10 +148,25 @@ static int vmnet_read_packets(VmnetState *s)
>   */
>  static void vmnet_write_packets_to_qemu(VmnetState *s)
>  {
> +uint8_t *pkt;
> +size_t pktsz;
> +uint8_t min_pkt[ETH_ZLEN];
> +size_t min_pktsz;
> +
>  while (s->packets_send_current_pos < s->packets_send_end_pos) {
> -ssize_t size = qemu_send_packet_async(&s->nc,
> -  
> s->iov_buf[s->packets_send_current_pos].iov_base,
> -  
> s->packets_buf[s->packets_send_current_pos].vm_pkt_size,
> +pkt = s->iov_buf[s->packets_send_current_pos].iov_base;
> +pktsz = s->packets_buf[s->packets_send_current_pos].vm_pkt_size;
> +
> +if (net_peer_needs_padding(&s->nc)) {
> +min_pktsz = sizeof(min_pkt);
> +
> +if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pktsz)) {
> +pkt = min_pkt;
> +pktsz = min_pktsz;
> +}
> +}
> +
> +ssize_t size = qemu_send_packet_async(&s->nc, pkt, pktsz,
>vmnet_send_completed);
>
>  if (size == 0) {
> --
> 2.37.1

Ping?



Re: [PATCH v2 1/8] Add CP0 MemoryMapID register implementation

2024-10-19 Thread Aleksandar Rikalo
Add CP0 MemoryMapID register implementation.
>
> Cherry-picked 9e0cb40adb110c2c76e2e97719ba8afcce72bcf5
> from https://github.com/MIPS/gnutools-qemu
>
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Aleksandar Rakic 
> ---
>  target/mips/sysemu/machine.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)


Reviewed-by: Aleksandar Rikalo 

-- Aleksandar


Re: [PULL 08/20] virtio-net: Add only one queue pair when realizing

2024-10-19 Thread Akihiko Odaki

On 2024/10/18 13:50, Jason Wang wrote:

On Thu, Oct 17, 2024 at 5:42 PM Akihiko Odaki  wrote:


On 2024/10/17 18:17, Laurent Vivier wrote:

On 17/10/2024 11:07, Akihiko Odaki wrote:

On 2024/10/17 16:32, Laurent Vivier wrote:

On 17/10/2024 08:59, Jason Wang wrote:

On Mon, Oct 14, 2024 at 11:16 PM Laurent Vivier 
wrote:


On 14/10/2024 10:30, Laurent Vivier wrote:

Hi Akihiko,

On 04/06/2024 09:37, Jason Wang wrote:

From: Akihiko Odaki 

Multiqueue usage is not negotiated yet when realizing. If more than
one queue is added and the guest never requests to enable
multiqueue,
the extra queues will not be deleted when unrealizing and leak.

Fixes: f9d6dbf0bf6e ("virtio-net: remove virtio queues if the
guest doesn't support
multiqueue")
Signed-off-by: Akihiko Odaki 
Signed-off-by: Jason Wang 
---
hw/net/virtio-net.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3cee2ef3ac..a8db8bfd9c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3743,9 +3743,7 @@ static void
virtio_net_device_realize(DeviceState *dev, Error **errp)
n->net_conf.tx_queue_size =
MIN(virtio_net_max_tx_queue_size(n),
n->net_conf.tx_queue_size);
-for (i = 0; i < n->max_queue_pairs; i++) {
-virtio_net_add_queue(n, i);
-}
+virtio_net_add_queue(n, 0);
n->ctrl_vq = virtio_add_queue(vdev, 64,
virtio_net_handle_ctrl);
qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);


This change breaks virtio net migration when multiqueue is enabled.

I think this is because virtqueues are half initialized after
migration : they are
initialized on guest side (kernel is using them) but not on QEMU
side (realized has only
initialized one). After migration, they are not initialized by the
call to
virtio_net_set_multiqueue() from virtio_net_set_features() because
virtio_get_num_queues()
reports already n->max_queue_pairs as this value is coming from
the source guest memory.

I don't think we have a way to half-initialize a virtqueue (to
initialize them only on
QEMU side as they are already initialized on kernel side).

I think this change should be reverted to fix the migration issue.



Moreover, if I look in the code of virtio_load() and
virtio_add_queue() we can guess it's
not correct to migrate a virtqueue that is not initialized on the
destination side because
fields like 'vdev->vq[i].handle_output' or 'vdev->vq[i].used_elems'
cannot be initialized
by virtio_load() and neither by virtio_add_queue() after
virtio_load() as fields like
'vring.num' are already initialized by virtio_load().

For instance, in virtio_load() we set:

   for (i = 0; i < num; i++) {
   vdev->vq[i].vring.num = qemu_get_be32(f);

and in virtio_add_queue() we search for the firt available queue to
add with:

   for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
   if (vdev->vq[i].vring.num == 0)
   break;
   }

So virtio_add_queue() cannot be used to set:

   vdev->vq[i].handle_output = handle_output;
   vdev->vq[i].used_elems = g_new0(VirtQueueElement, queue_size);

Moreover it would overwrite fields already set by virtio_load():

   vdev->vq[i].vring.num = queue_size;
   vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;

It also explains why virtio_net_change_num_queue_pairs()
(indirectly called by
virtio_net_set_features()) doesn't update the queue pair numbers:
vring.num is already set
so it thinks there is no more queues to add.

Thanks,
LAurent



I agree.

Laurent, would you like to send a patch to revert this?



Yes. I will also try to fix the leak in unrealize that the patch
wanted to fix initially.


I wrote a fix so I will submit it once internal testing is done. You
can see the change at:
https://gitlab.com/akihiko.odaki/qemu-kvm/-/
commit/22161221aa2d2031d7ad1be7701852083aa9109a


It works fine for me but I don't know if it's a good idea to add queues
while the state is loading.


I couldn't come up with other options. The problem is that the number of
queues added during realization does not match with the loaded state. We
need to add queues after knowing the negotiated feature set and before
loading the queue states to fix this problem.

Reverting will add queues that are used when the multiqueue feature is
negotiated so it will fix migration for such cases, but will also break
the other cases (i.e., the multiqueue feature is not negotiated) as it
adds too many queues.

Regards,
Akihiko Odaki


I wonder if the following is much more simpler:

1) introducing booleans whether the queue has been deleted
2) in unrelize, deleted only the queue that has not been deleted


The memory leak problem is trivial to solve, but the problem with queue 
state loading is not. We need to ensure the number of queues are 
consistent with the number of loaded queues.


We currently have too few queues if the multiqueue feature is 
negotiated, which results in queues partially

Re: [PATCH v2 6/8] Revert use of clock_gettime for benchmarking

2024-10-19 Thread Philippe Mathieu-Daudé

Hi,

On 18/10/24 10:20, Aleksandar Rakic wrote:

This patch reverts the commit (with SHA
50290c002c045280f8defad911901e16bfb52884 from
https://github.com/MIPS/gnutools-qemu) that breaks for mingw builds,
where clock_gettime and CLOCK_MONOTONIC are not available.


Isn't get_clock() what we want here?


Cherry-picked d57c735e1af1ca719dbd0c3a904ad70c9c31cbb7
from https://github.com/MIPS/gnutools-qemu

Signed-off-by: Faraz Shahbazker 
Signed-off-by: Aleksandar Rakic 
---
  qemu-io-cmds.c | 77 +-
  1 file changed, 39 insertions(+), 38 deletions(-)


Please Cc maintainers (done now):

$ ./scripts/get_maintainer.pl -f qemu-io-cmds.c
Kevin Wolf  (supporter:Block layer core)
Hanna Reitz  (supporter:Block layer core)


@@ -904,7 +905,7 @@ static const cmdinfo_t readv_cmd = {
  
  static int readv_f(BlockBackend *blk, int argc, char **argv)

  {
-struct timespec t1, t2;
+struct timeval t1, t2;
  bool Cflag = false, qflag = false, vflag = false;
  int c, cnt, ret;
  char *buf;
@@ -964,9 +965,9 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
  return -EINVAL;
  }
  
-clock_gettime(CLOCK_MONOTONIC, &t1);

+gettimeofday(&t1, NULL);
  ret = do_aio_readv(blk, &qiov, offset, flags, &total);
-clock_gettime(CLOCK_MONOTONIC, &t2);
+gettimeofday(&t2, NULL);
  
  if (ret < 0) {

  printf("readv failed: %s\n", strerror(-ret));





[RFC PATCH v2 3/7] contrib/plugins: add plugin showcasing new trap related API

2024-10-19 Thread Julian Ganz
We recently introduced new plugin API for registration of trap related
callbacks. This change introduces a minimal plugin showcasing the new
API. It simply counts the occurances of interrupts, exceptions and
semihosting events per CPU and reports the counts when exitting.

Signed-off-by: Julian Ganz 
---
 contrib/plugins/Makefile |  1 +
 contrib/plugins/traps.c  | 89 
 2 files changed, 90 insertions(+)
 create mode 100644 contrib/plugins/traps.c

diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
index bbddd4800f..6085fd701f 100644
--- a/contrib/plugins/Makefile
+++ b/contrib/plugins/Makefile
@@ -31,6 +31,7 @@ NAMES += drcov
 NAMES += ips
 NAMES += stoptrigger
 NAMES += cflow
+NAMES += traps
 
 ifeq ($(CONFIG_WIN32),y)
 SO_SUFFIX := .dll
diff --git a/contrib/plugins/traps.c b/contrib/plugins/traps.c
new file mode 100644
index 00..2a38dbb8b3
--- /dev/null
+++ b/contrib/plugins/traps.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (C) 2024, Julian Ganz 
+ *
+ * Traps - count traps
+ *
+ * License: GNU GPL, version 2 or later.
+ *   See the COPYING file in the top-level directory.
+ */
+
+#include 
+
+#include 
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+typedef struct {
+uint64_t interrupts;
+uint64_t exceptions;
+uint64_t semihosting;
+bool active;
+} TrapCounters;
+
+static TrapCounters *traps;
+size_t max_vcpus;
+
+static void vcpu_init(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+traps[vcpu_index].active = true;
+}
+
+static void vcpu_interrupt(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+traps[vcpu_index].interrupts++;
+}
+
+static void vcpu_exception(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+traps[vcpu_index].exceptions++;
+}
+
+static void vcpu_semihosting(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+traps[vcpu_index].semihosting++;
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+g_autoptr(GString) report;
+report = g_string_new("VCPU, interrupts, exceptions, semihosting\n");
+int vcpu;
+
+for (vcpu = 0; vcpu < max_vcpus; vcpu++) {
+TrapCounters *rec = &traps[vcpu];
+if (rec->active) {
+g_string_append_printf(report,
+   "% 4d, % 10"PRId64", % 10"PRId64", % 10"
+   PRId64"\n",
+   vcpu,
+   rec->interrupts, rec->exceptions,
+   rec->semihosting);
+}
+}
+
+qemu_plugin_outs(report->str);
+}
+
+QEMU_PLUGIN_EXPORT
+int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
+int argc, char **argv)
+{
+if (!info->system_emulation) {
+fputs("trap plugin can only be used in system emulation mode.\n",
+  stderr);
+return -1;
+}
+
+max_vcpus = info->system.max_vcpus;
+traps = calloc(max_vcpus, sizeof(TrapCounters));
+qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
+qemu_plugin_vcpu_for_each(id, vcpu_init);
+
+qemu_plugin_register_vcpu_interrupt_cb(id, vcpu_interrupt);
+qemu_plugin_register_vcpu_exception_cb(id, vcpu_exception);
+qemu_plugin_register_vcpu_semihosting_cb(id, vcpu_semihosting);
+
+qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+
+return 0;
+}
-- 
2.45.2




[PATCH v5 13/19] include/hw/s390x: Add include files for common IPL structs

2024-10-19 Thread jrossi
From: Jared Rossi 

Currently, structures defined in both hw/s390x/ipl.h and pc-bios/s390-ccw/iplb.h
must be kept in sync, which is prone to error. Instead, create a new directory
at include/hw/s390x/ipl/ to contain the definitions that must be shared.

Signed-off-by: Jared Rossi 
Reviewed-by: Thomas Huth 
---
 hw/s390x/ipl.h  | 104 +-
 include/hw/s390x/ipl/qipl.h | 123 
 pc-bios/s390-ccw/iplb.h |  84 ++--
 pc-bios/s390-ccw/Makefile   |   2 +-
 4 files changed, 130 insertions(+), 183 deletions(-)
 create mode 100644 include/hw/s390x/ipl/qipl.h

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index b2105b616a..fa394c339d 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -16,95 +16,11 @@
 #include "cpu.h"
 #include "exec/address-spaces.h"
 #include "hw/qdev-core.h"
+#include "hw/s390x/ipl/qipl.h"
 #include "qom/object.h"
 
-struct IPLBlockPVComp {
-uint64_t tweak_pref;
-uint64_t addr;
-uint64_t size;
-} QEMU_PACKED;
-typedef struct IPLBlockPVComp IPLBlockPVComp;
-
-struct IPLBlockPV {
-uint8_t  reserved18[87];/* 0x18 */
-uint8_t  version;   /* 0x6f */
-uint32_t reserved70;/* 0x70 */
-uint32_t num_comp;  /* 0x74 */
-uint64_t pv_header_addr;/* 0x78 */
-uint64_t pv_header_len; /* 0x80 */
-struct IPLBlockPVComp components[0];
-} QEMU_PACKED;
-typedef struct IPLBlockPV IPLBlockPV;
-
-struct IplBlockCcw {
-uint8_t  reserved0[85];
-uint8_t  ssid;
-uint16_t devno;
-uint8_t  vm_flags;
-uint8_t  reserved3[3];
-uint32_t vm_parm_len;
-uint8_t  nss_name[8];
-uint8_t  vm_parm[64];
-uint8_t  reserved4[8];
-} QEMU_PACKED;
-typedef struct IplBlockCcw IplBlockCcw;
-
-struct IplBlockFcp {
-uint8_t  reserved1[305 - 1];
-uint8_t  opt;
-uint8_t  reserved2[3];
-uint16_t reserved3;
-uint16_t devno;
-uint8_t  reserved4[4];
-uint64_t wwpn;
-uint64_t lun;
-uint32_t bootprog;
-uint8_t  reserved5[12];
-uint64_t br_lba;
-uint32_t scp_data_len;
-uint8_t  reserved6[260];
-uint8_t  scp_data[0];
-} QEMU_PACKED;
-typedef struct IplBlockFcp IplBlockFcp;
-
-struct IplBlockQemuScsi {
-uint32_t lun;
-uint16_t target;
-uint16_t channel;
-uint8_t  reserved0[77];
-uint8_t  ssid;
-uint16_t devno;
-} QEMU_PACKED;
-typedef struct IplBlockQemuScsi IplBlockQemuScsi;
-
 #define DIAG308_FLAGS_LP_VALID 0x80
 
-union IplParameterBlock {
-struct {
-uint32_t len;
-uint8_t  reserved0[3];
-uint8_t  version;
-uint32_t blk0_len;
-uint8_t  pbt;
-uint8_t  flags;
-uint16_t reserved01;
-uint8_t  loadparm[8];
-union {
-IplBlockCcw ccw;
-IplBlockFcp fcp;
-IPLBlockPV pv;
-IplBlockQemuScsi scsi;
-};
-} QEMU_PACKED;
-struct {
-uint8_t  reserved1[110];
-uint16_t devno;
-uint8_t  reserved2[88];
-uint8_t  reserved_ext[4096 - 200];
-} QEMU_PACKED;
-} QEMU_PACKED;
-typedef union IplParameterBlock IplParameterBlock;
-
 int s390_ipl_set_loadparm(uint8_t *loadparm);
 void s390_ipl_update_diag308(IplParameterBlock *iplb);
 int s390_ipl_prepare_pv_header(Error **errp);
@@ -131,24 +47,6 @@ void s390_ipl_clear_reset_request(void);
 #define QIPL_FLAG_BM_OPTS_CMD   0x80
 #define QIPL_FLAG_BM_OPTS_ZIPL  0x40
 
-/*
- * The QEMU IPL Parameters will be stored at absolute address
- * 204 (0xcc) which means it is 32-bit word aligned but not
- * double-word aligned. Placement of 64-bit data fields in this
- * area must account for their alignment needs.
- * The total size of the struct must never exceed 28 bytes.
- * This definition must be kept in sync with the definition
- * in pc-bios/s390-ccw/iplb.h.
- */
-struct QemuIplParameters {
-uint8_t  qipl_flags;
-uint8_t  reserved1[3];
-uint64_t reserved2;
-uint32_t boot_menu_timeout;
-uint8_t  reserved3[12];
-} QEMU_PACKED;
-typedef struct QemuIplParameters QemuIplParameters;
-
 #define TYPE_S390_IPL "s390-ipl"
 OBJECT_DECLARE_SIMPLE_TYPE(S390IPLState, S390_IPL)
 
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
new file mode 100644
index 00..0ef04af027
--- /dev/null
+++ b/include/hw/s390x/ipl/qipl.h
@@ -0,0 +1,123 @@
+/*
+ * S/390 boot structures
+ *
+ * Copyright 2024 IBM Corp.
+ * Author(s): Jared Rossi 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#ifndef S390X_QIPL_H
+#define S390X_QIPL_H
+
+/* Boot Menu flags */
+#define QIPL_FLAG_BM_OPTS_CMD   0x80
+#define QIPL_FLAG_BM_OPTS_ZIPL  0x40
+
+#define QIPL_ADDRESS  0xcc
+#define LOADPARM_LEN8
+
+/*
+ * The QEMU IPL Parameters will be stored at absolute address
+ * 204 (0xcc) which means it is 32-bit word aligned but not
+ * double-word aligned. Placement 

[PATCH v5 02/19] pc-bios/s390-ccw: Use the libc from SLOF and remove sclp prints

2024-10-19 Thread jrossi
From: Jared Rossi 

We are already using the libc from SLOF for the s390-netboot.img, and
this libc implementation is way more complete and accurate than the
simple implementation that we currently use for the s390-ccw.img binary.
Since we are now always assuming that the SLOF submodule is available
when building the s390-ccw bios (see commit bf6903f6944f), we can drop
the simple implementation and use the SLOF libc for the s390-ccw.img
binary, too.

Additionally replace sclp_print calls with puts/printf now that it is avaliable.

Co-authored by: Thomas Huth 
Signed-off-by: Jared Rossi 
---
 pc-bios/s390-ccw/netboot.mak |  3 --
 pc-bios/s390-ccw/bootmap.h   |  4 +-
 pc-bios/s390-ccw/libc.h  | 89 
 pc-bios/s390-ccw/s390-ccw.h  | 30 ---
 pc-bios/s390-ccw/bootmap.c   | 47 -
 pc-bios/s390-ccw/cio.c   | 78 +---
 pc-bios/s390-ccw/dasd-ipl.c  |  5 +-
 pc-bios/s390-ccw/jump2ipl.c  |  5 +-
 pc-bios/s390-ccw/libc.c  | 88 ---
 pc-bios/s390-ccw/main.c  | 14 ++---
 pc-bios/s390-ccw/menu.c  | 51 +-
 pc-bios/s390-ccw/netmain.c   | 10 ++--
 pc-bios/s390-ccw/sclp.c  |  7 +--
 pc-bios/s390-ccw/virtio-blkdev.c |  6 +--
 pc-bios/s390-ccw/virtio-scsi.c   | 17 +++---
 pc-bios/s390-ccw/virtio.c|  2 +-
 pc-bios/s390-ccw/Makefile| 15 --
 17 files changed, 136 insertions(+), 335 deletions(-)
 delete mode 100644 pc-bios/s390-ccw/libc.h
 delete mode 100644 pc-bios/s390-ccw/libc.c

diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
index 046aa35587..d2b3d8ee74 100644
--- a/pc-bios/s390-ccw/netboot.mak
+++ b/pc-bios/s390-ccw/netboot.mak
@@ -1,9 +1,6 @@
 
-SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
-
 NETOBJS := start.o sclp.o cio.o virtio.o virtio-net.o jump2ipl.o netmain.o
 
-LIBC_INC := -nostdinc -I$(SLOF_DIR)/lib/libc/include
 LIBNET_INC := -I$(SLOF_DIR)/lib/libnet
 
 NETLDFLAGS := $(LDFLAGS) -Wl,-Ttext=0x780
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index d4690a88c2..4a7d8a91f1 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -336,9 +336,7 @@ static inline void print_volser(const void *volser)
 
 ebcdic_to_ascii((char *)volser, ascii, 6);
 ascii[6] = '\0';
-sclp_print("VOLSER=[");
-sclp_print(ascii);
-sclp_print("]\n");
+printf("VOLSER=[%s]\n", ascii);
 }
 
 static inline bool unused_space(const void *p, size_t size)
diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h
deleted file mode 100644
index bcdc45732d..00
--- a/pc-bios/s390-ccw/libc.h
+++ /dev/null
@@ -1,89 +0,0 @@
-/*
- * libc-style definitions and functions
- *
- * Copyright (c) 2013 Alexander Graf 
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- */
-
-#ifndef S390_CCW_LIBC_H
-#define S390_CCW_LIBC_H
-
-typedef unsigned long  size_t;
-typedef intbool;
-typedef unsigned char  uint8_t;
-typedef unsigned short uint16_t;
-typedef unsigned int   uint32_t;
-typedef unsigned long long uint64_t;
-
-static inline void *memset(void *s, int c, size_t n)
-{
-size_t i;
-unsigned char *p = s;
-
-for (i = 0; i < n; i++) {
-p[i] = c;
-}
-
-return s;
-}
-
-static inline void *memcpy(void *s1, const void *s2, size_t n)
-{
-uint8_t *dest = s1;
-const uint8_t *src = s2;
-size_t i;
-
-for (i = 0; i < n; i++) {
-dest[i] = src[i];
-}
-
-return s1;
-}
-
-static inline int memcmp(const void *s1, const void *s2, size_t n)
-{
-size_t i;
-const uint8_t *p1 = s1, *p2 = s2;
-
-for (i = 0; i < n; i++) {
-if (p1[i] != p2[i]) {
-return p1[i] > p2[i] ? 1 : -1;
-}
-}
-
-return 0;
-}
-
-static inline size_t strlen(const char *str)
-{
-size_t i;
-for (i = 0; *str; i++) {
-str++;
-}
-return i;
-}
-
-static inline char *strcat(char *dest, const char *src)
-{
-int i;
-char *dest_end = dest + strlen(dest);
-
-for (i = 0; i <= strlen(src); i++) {
-dest_end[i] = src[i];
-}
-return dest;
-}
-
-static inline int isdigit(int c)
-{
-return (c >= '0') && (c <= '9');
-}
-
-uint64_t atoui(const char *str);
-char *uitoa(uint64_t num, char *str, size_t len);
-
-#endif
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index c977a52b50..6f6d95d170 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -13,6 +13,11 @@
 
 /* #define DEBUG */
 
+#include 
+#include 
+#include 
+#include 
+
 typedef unsigned char  u8;
 typedef unsigned short u16;
 typedef unsigned int   u32;
@@ -26,9 +31,6 @@ typedef unsigned long long u64;
 #define EBUSY   

[PATCH v5 18/19] docs/system: Update documentation for s390x IPL

2024-10-19 Thread jrossi
From: Jared Rossi 

Update docs to show that s390x PC BIOS can support more than one boot device.

Signed-off-by: Jared Rossi 
Reviewed-by: Thomas Huth 
---
 docs/system/bootindex.rst | 7 ---
 docs/system/s390x/bootdevices.rst | 9 ++---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/docs/system/bootindex.rst b/docs/system/bootindex.rst
index 8b057f812f..988f7b3beb 100644
--- a/docs/system/bootindex.rst
+++ b/docs/system/bootindex.rst
@@ -49,10 +49,11 @@ Limitations
 ---
 
 Some firmware has limitations on which devices can be considered for
-booting.  For instance, the PC BIOS boot specification allows only one
-disk to be bootable.  If boot from disk fails for some reason, the BIOS
+booting.  For instance, the x86 PC BIOS boot specification allows only one
+disk to be bootable.  If boot from disk fails for some reason, the x86 BIOS
 won't retry booting from other disk.  It can still try to boot from
-floppy or net, though.
+floppy or net, though. In the case of s390x BIOS, the BIOS will try up to
+8 total devices, any number of which may be disks.
 
 Sometimes, firmware cannot map the device path QEMU wants firmware to
 boot from to a boot method.  It doesn't happen for devices the firmware
diff --git a/docs/system/s390x/bootdevices.rst 
b/docs/system/s390x/bootdevices.rst
index c97efb8fc0..1a1a764c1c 100644
--- a/docs/system/s390x/bootdevices.rst
+++ b/docs/system/s390x/bootdevices.rst
@@ -6,9 +6,7 @@ Booting with bootindex parameter
 
 For classical mainframe guests (i.e. LPAR or z/VM installations), you always
 have to explicitly specify the disk where you want to boot from (or "IPL" from,
-in s390x-speak -- IPL means "Initial Program Load"). In particular, there can
-also be only one boot device according to the architecture specification, thus
-specifying multiple boot devices is not possible (yet).
+in s390x-speak -- IPL means "Initial Program Load").
 
 So for booting an s390x guest in QEMU, you should always mark the
 device where you want to boot from with the ``bootindex`` property, for
@@ -17,6 +15,11 @@ example::
  qemu-system-s390x -drive if=none,id=dr1,file=guest.qcow2 \
-device virtio-blk,drive=dr1,bootindex=1
 
+Multiple devices may have a bootindex. The lowest bootindex is assigned to the
+device to IPL first.  If the IPL fails for the first, the device with the 
second
+lowest bootindex will be tried and so on until IPL is successful or there are 
no
+remaining boot devices to try.
+
 For booting from a CD-ROM ISO image (which needs to include El-Torito boot
 information in order to be bootable), it is recommended to specify a 
``scsi-cd``
 device, for example like this::
-- 
2.45.1




[PATCH v5 06/19] docs/system/s390x/bootdevices: Update the documentation about network booting

2024-10-19 Thread jrossi
From: Jared Rossi 

Remove the information about the separate s390-netboot.img from
the documentation.

Co-authored by: Thomas Huth 
Signed-off-by: Jared Rossi 
---
 docs/system/s390x/bootdevices.rst | 20 +++-
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/docs/system/s390x/bootdevices.rst 
b/docs/system/s390x/bootdevices.rst
index 1a7a18b43b..c97efb8fc0 100644
--- a/docs/system/s390x/bootdevices.rst
+++ b/docs/system/s390x/bootdevices.rst
@@ -82,23 +82,17 @@ Note that ``0`` can be used to boot the default entry.
 Booting from a network device
 -
 
-Beside the normal guest firmware (which is loaded from the file 
``s390-ccw.img``
-in the data directory of QEMU, or via the ``-bios`` option), QEMU ships with
-a small TFTP network bootloader firmware for virtio-net-ccw devices, too. This
-firmware is loaded from a file called ``s390-netboot.img`` in the QEMU data
-directory. In case you want to load it from a different filename instead,
-you can specify it via the ``-global s390-ipl.netboot_fw=filename``
-command line option.
-
-The ``bootindex`` property is especially important for booting via the network.
-If you don't specify the ``bootindex`` property here, the network bootloader
-firmware code won't get loaded into the guest memory so that the network boot
-will fail. For a successful network boot, try something like this::
+The firmware that ships with QEMU includes a small TFTP network bootloader
+for virtio-net-ccw devices.  The ``bootindex`` property is especially
+important for booting via the network. If you don't specify the ``bootindex``
+property here, the network bootloader won't be taken into consideration and
+the network boot will fail. For a successful network boot, try something
+like this::
 
  qemu-system-s390x -netdev user,id=n1,tftp=...,bootfile=... \
-device virtio-net-ccw,netdev=n1,bootindex=1
 
-The network bootloader firmware also has basic support for pxelinux.cfg-style
+The network bootloader also has basic support for pxelinux.cfg-style
 configuration files. See the `PXELINUX Configuration page
 `__
 for details how to set up the configuration file on your TFTP server.
-- 
2.45.1




[PATCH v5 11/19] pc-bios/s390-ccw: Remove panics from Netboot IPL path

2024-10-19 Thread jrossi
From: Jared Rossi 

Remove panic-on-error from Netboot specific functions so that error recovery
may be possible in the future.

Functions that would previously panic now provide a return code.

Signed-off-by: Jared Rossi 
---
 pc-bios/s390-ccw/s390-ccw.h   |  2 +-
 pc-bios/s390-ccw/bootmap.c|  1 +
 pc-bios/s390-ccw/netmain.c| 17 +++--
 pc-bios/s390-ccw/virtio-net.c |  7 +--
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 3e844abd71..344ad15655 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -57,7 +57,7 @@ unsigned int get_loadparm_index(void);
 void main(void);
 
 /* netmain.c */
-void netmain(void);
+int netmain(void);
 
 /* sclp.c */
 void sclp_print(const char *string);
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 652807a16a..95ef9104d0 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -1072,6 +1072,7 @@ void zipl_load(void)
 
 if (virtio_get_device_type() == VIRTIO_ID_NET) {
 netmain();
+panic("\n! Cannot IPL from this network !\n");
 }
 
 if (ipl_scsi()) {
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index bc6ad8695f..d1a6c9a91c 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -464,7 +464,7 @@ static bool find_net_dev(Schib *schib, int dev_no)
 return false;
 }
 
-static void virtio_setup(void)
+static bool virtio_setup(void)
 {
 Schib schib;
 int ssid;
@@ -495,10 +495,10 @@ static void virtio_setup(void)
 }
 }
 
-IPL_assert(found, "No virtio net device found");
+return found;
 }
 
-void netmain(void)
+int netmain(void)
 {
 filename_ip_t fn_ip;
 int rc, fnlen;
@@ -506,11 +506,15 @@ void netmain(void)
 sclp_setup();
 puts("Network boot starting...");
 
-virtio_setup();
+if (!virtio_setup()) {
+puts("No virtio net device found.");
+return -1;
+}
 
 rc = net_init(&fn_ip);
 if (rc) {
-panic("Network initialization failed. Halting.");
+puts("Network initialization failed.");
+return -1;
 }
 
 fnlen = strlen(fn_ip.filename);
@@ -528,5 +532,6 @@ void netmain(void)
 jump_to_low_kernel();
 }
 
-panic("Failed to load OS from network.");
+puts("Failed to load OS from network.");
+return -1;
 }
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index 2fcb0a58c5..f9854a22c3 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -54,8 +54,11 @@ int virtio_net_init(void *mac_addr)
 vdev->guest_features[0] = VIRTIO_NET_F_MAC_BIT;
 virtio_setup_ccw(vdev);
 
-IPL_assert(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT,
-   "virtio-net device does not support the MAC address feature");
+if (!(vdev->guest_features[0] & VIRTIO_NET_F_MAC_BIT)) {
+puts("virtio-net device does not support the MAC address feature");
+return -1;
+}
+
 memcpy(mac_addr, vdev->config.net.mac, ETH_ALEN);
 
 for (i = 0; i < 64; i++) {
-- 
2.45.1




[PATCH v5 10/19] pc-bios/s390-ccw: Remove panics from DASD IPL path

2024-10-19 Thread jrossi
From: Jared Rossi 

Remove panic-on-error from DASD IPL specific functions so that error recovery
may be possible in the future.

Functions that would previously panic now provide a return code.

Signed-off-by: Jared Rossi 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/dasd-ipl.h |  2 +-
 pc-bios/s390-ccw/dasd-ipl.c | 66 -
 2 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/pc-bios/s390-ccw/dasd-ipl.h b/pc-bios/s390-ccw/dasd-ipl.h
index c394828906..eb1898c84a 100644
--- a/pc-bios/s390-ccw/dasd-ipl.h
+++ b/pc-bios/s390-ccw/dasd-ipl.h
@@ -11,6 +11,6 @@
 #ifndef DASD_IPL_H
 #define DASD_IPL_H
 
-void dasd_ipl(SubChannelId schid, uint16_t cutype);
+int dasd_ipl(SubChannelId schid, uint16_t cutype);
 
 #endif /* DASD_IPL_H */
diff --git a/pc-bios/s390-ccw/dasd-ipl.c b/pc-bios/s390-ccw/dasd-ipl.c
index ae751adec1..babece95ea 100644
--- a/pc-bios/s390-ccw/dasd-ipl.c
+++ b/pc-bios/s390-ccw/dasd-ipl.c
@@ -111,38 +111,29 @@ static void make_readipl(void)
 ccwIplRead->count = 0x18; /* Read 0x18 bytes of data */
 }
 
-static void run_readipl(SubChannelId schid, uint16_t cutype)
+static int run_readipl(SubChannelId schid, uint16_t cutype)
 {
-if (do_cio(schid, cutype, 0x00, CCW_FMT0)) {
-panic("dasd-ipl: Failed to run Read IPL channel program\n");
-}
+return do_cio(schid, cutype, 0x00, CCW_FMT0);
 }
 
 /*
  * The architecture states that IPL1 data should consist of a psw followed by
  * format-0 READ and TIC CCWs. Let's sanity check.
  */
-static void check_ipl1(void)
+static bool check_ipl1(void)
 {
 Ccw0 *ccwread = (Ccw0 *)0x08;
 Ccw0 *ccwtic = (Ccw0 *)0x10;
 
-if (ccwread->cmd_code != CCW_CMD_DASD_READ ||
-ccwtic->cmd_code != CCW_CMD_TIC) {
-panic("dasd-ipl: IPL1 data invalid. Is this disk really bootable?\n");
-}
+return (ccwread->cmd_code == CCW_CMD_DASD_READ &&
+ccwtic->cmd_code == CCW_CMD_TIC);
 }
 
-static void check_ipl2(uint32_t ipl2_addr)
+static bool check_ipl2(uint32_t ipl2_addr)
 {
 Ccw0 *ccw = u32toptr(ipl2_addr);
 
-if (ipl2_addr == 0x00) {
-panic("IPL2 address invalid. Is this disk really bootable?\n");
-}
-if (ccw->cmd_code == 0x00) {
-panic("IPL2 ccw data invalid. Is this disk really bootable?\n");
-}
+return (ipl2_addr != 0x00 && ccw->cmd_code != 0x00);
 }
 
 static uint32_t read_ipl2_addr(void)
@@ -188,52 +179,67 @@ static void ipl1_fixup(void)
 ccwSearchTic->cda = ptr2u32(ccwSearchID);
 }
 
-static void run_ipl1(SubChannelId schid, uint16_t cutype)
+static int run_ipl1(SubChannelId schid, uint16_t cutype)
  {
 uint32_t startAddr = 0x08;
 
-if (do_cio(schid, cutype, startAddr, CCW_FMT0)) {
-panic("dasd-ipl: Failed to run IPL1 channel program\n");
-}
+return do_cio(schid, cutype, startAddr, CCW_FMT0);
 }
 
-static void run_ipl2(SubChannelId schid, uint16_t cutype, uint32_t addr)
+static int run_ipl2(SubChannelId schid, uint16_t cutype, uint32_t addr)
 {
-if (run_dynamic_ccw_program(schid, cutype, addr)) {
-panic("dasd-ipl: Failed to run IPL2 channel program\n");
-}
+return run_dynamic_ccw_program(schid, cutype, addr);
 }
 
 /*
  * Limitations in vfio-ccw support complicate the IPL process. Details can
  * be found in docs/devel/s390-dasd-ipl.rst
  */
-void dasd_ipl(SubChannelId schid, uint16_t cutype)
+int dasd_ipl(SubChannelId schid, uint16_t cutype)
 {
 PSWLegacy *pswl = (PSWLegacy *) 0x00;
 uint32_t ipl2_addr;
 
 /* Construct Read IPL CCW and run it to read IPL1 from boot disk */
 make_readipl();
-run_readipl(schid, cutype);
+if (run_readipl(schid, cutype)) {
+puts("Failed to run Read IPL channel program");
+return -EIO;
+}
+
 ipl2_addr = read_ipl2_addr();
-check_ipl1();
+
+if (!check_ipl1()) {
+puts("IPL1 invalid for DASD-IPL");
+return -EINVAL;
+}
 
 /*
  * Fixup IPL1 channel program to account for vfio-ccw limitations, then run
  * it to read IPL2 channel program from boot disk.
  */
 ipl1_fixup();
-run_ipl1(schid, cutype);
-check_ipl2(ipl2_addr);
+if (run_ipl1(schid, cutype)) {
+puts("Failed to run IPL1 channel program");
+return -EIO;
+}
+
+if (!check_ipl2(ipl2_addr)) {
+puts("IPL2 invalid for DASD-IPL");
+return -EINVAL;
+}
 
 /*
  * Run IPL2 channel program to read operating system code from boot disk
  */
-run_ipl2(schid, cutype, ipl2_addr);
+if (run_ipl2(schid, cutype, ipl2_addr)) {
+puts("Failed to run IPL2 channel program");
+return -EIO;
+}
 
 /* Transfer control to the guest operating system */
 pswl->mask |= PSW_MASK_EAMODE;   /* Force z-mode */
 pswl->addr |= PSW_MASK_BAMODE;   /* ...  */
 jump_to_low_kernel();
+return -1;
 }
-- 
2.45.1




[PATCH v5 09/19] pc-bios/s390-ccw: Remove panics from SCSI IPL path

2024-10-19 Thread jrossi
From: Jared Rossi 

Remove panic-on-error from virtio-scsi IPL specific functions so that error
recovery may be possible in the future.

Functions that would previously panic now provide a return code.

Signed-off-by: Jared Rossi 
---
 pc-bios/s390-ccw/bootmap.c   |  88 ++-
 pc-bios/s390-ccw/virtio-blkdev.c |   4 +-
 pc-bios/s390-ccw/virtio-scsi.c   | 143 +--
 3 files changed, 164 insertions(+), 71 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index b9596e28c7..652807a16a 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -595,7 +595,7 @@ static int ipl_eckd(void)
  * IPL a SCSI disk
  */
 
-static void zipl_load_segment(ComponentEntry *entry)
+static int zipl_load_segment(ComponentEntry *entry)
 {
 const int max_entries = (MAX_SECTOR_SIZE / sizeof(ScsiBlockPtr));
 ScsiBlockPtr *bprs = (void *)sec;
@@ -615,7 +615,10 @@ static void zipl_load_segment(ComponentEntry *entry)
 do {
 memset(bprs, FREE_SPACE_FILLER, bprs_size);
 fill_hex_val(blk_no, &blockno, sizeof(blockno));
-read_block(blockno, bprs, err_msg);
+if (virtio_read(blockno, bprs)) {
+puts(err_msg);
+return -EIO;
+}
 
 for (i = 0;; i++) {
 uint64_t *cur_desc = (void *)&bprs[i];
@@ -643,23 +646,37 @@ static void zipl_load_segment(ComponentEntry *entry)
 }
 address = virtio_load_direct(cur_desc[0], cur_desc[1], 0,
  (void *)address);
-IPL_assert(address != -1, "zIPL load segment failed");
+if (!address) {
+puts("zIPL load segment failed");
+return -EIO;
+}
 }
 } while (blockno);
+
+return 0;
 }
 
 /* Run a zipl program */
-static void zipl_run(ScsiBlockPtr *pte)
+static int zipl_run(ScsiBlockPtr *pte)
 {
 ComponentHeader *header;
 ComponentEntry *entry;
 uint8_t tmp_sec[MAX_SECTOR_SIZE];
 
-read_block(pte->blockno, tmp_sec, "Cannot read header");
+if (virtio_read(pte->blockno, tmp_sec)) {
+puts("Cannot read header");
+return -EIO;
+}
 header = (ComponentHeader *)tmp_sec;
 
-IPL_assert(magic_match(tmp_sec, ZIPL_MAGIC), "No zIPL magic in header");
-IPL_assert(header->type == ZIPL_COMP_HEADER_IPL, "Bad header type");
+if (!magic_match(tmp_sec, ZIPL_MAGIC)) {
+puts("No zIPL magic in header");
+return -EINVAL;
+}
+if (header->type != ZIPL_COMP_HEADER_IPL) {
+puts("Bad header type");
+return -EINVAL;
+}
 
 dputs("start loading images\n");
 
@@ -674,22 +691,30 @@ static void zipl_run(ScsiBlockPtr *pte)
 continue;
 }
 
-zipl_load_segment(entry);
+if (zipl_load_segment(entry)) {
+return -1;
+}
 
 entry++;
 
-IPL_assert((uint8_t *)(&entry[1]) <= (tmp_sec + MAX_SECTOR_SIZE),
-   "Wrong entry value");
+if ((uint8_t *)(&entry[1]) > (tmp_sec + MAX_SECTOR_SIZE)) {
+puts("Wrong entry value");
+return -EINVAL;
+}
 }
 
-IPL_assert(entry->component_type == ZIPL_COMP_ENTRY_EXEC, "No EXEC entry");
+if (entry->component_type != ZIPL_COMP_ENTRY_EXEC) {
+puts("No EXEC entry");
+return -EINVAL;
+}
 
 /* should not return */
 write_reset_psw(entry->compdat.load_psw);
 jump_to_IPL_code(0);
+return -1;
 }
 
-static void ipl_scsi(void)
+static int ipl_scsi(void)
 {
 ScsiMbr *mbr = (void *)sec;
 int program_table_entries = 0;
@@ -700,10 +725,13 @@ static void ipl_scsi(void)
 
 /* Grab the MBR */
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
-read_block(0, mbr, "Cannot read block 0");
+if (virtio_read(0, mbr)) {
+puts("Cannot read block 0");
+return -EIO;
+}
 
 if (!magic_match(mbr->magic, ZIPL_MAGIC)) {
-return;
+return 0;
 }
 
 puts("Using SCSI scheme.");
@@ -711,11 +739,20 @@ static void ipl_scsi(void)
 IPL_check(mbr->version_id == 1,
   "Unknown MBR layout version, assuming version 1");
 debug_print_int("program table", mbr->pt.blockno);
-IPL_assert(mbr->pt.blockno, "No Program Table");
+if (!mbr->pt.blockno) {
+puts("No Program Table");
+return -EINVAL;
+}
 
 /* Parse the program table */
-read_block(mbr->pt.blockno, sec, "Error reading Program Table");
-IPL_assert(magic_match(sec, ZIPL_MAGIC), "No zIPL magic in PT");
+if (virtio_read(mbr->pt.blockno, sec)) {
+puts("Error reading Program Table");
+return -EIO;
+}
+if (!magic_match(sec, ZIPL_MAGIC)) {
+puts("No zIPL magic in Program Table");
+return -EINVAL;
+}
 
 for (i = 0; i < MAX_BOOT_ENTRIES; i++) {
 if (prog_table->entry[i].scsi.blockno) {
@@ -725,17 +762,22 @@ static void ipl_scsi(void)
 }
 
 debug_prin

Re: [PATCH v2] intel_iommu: Introduce property "x-stale-tm" to control Transient Mapping (TM) field

2024-10-19 Thread Michael S. Tsirkin
On Thu, Oct 17, 2024 at 07:57:53AM +, Duan, Zhenzhong wrote:
> 
> 
> >-Original Message-
> >From: Jason Wang 
> >Subject: Re: [PATCH v2] intel_iommu: Introduce property "x-stale-tm" to 
> >control
> >Transient Mapping (TM) field
> >
> >On Thu, Oct 10, 2024 at 3:57 PM Zhenzhong Duan 
> >wrote:
> >>
> >> VT-d spec removed Transient Mapping (TM) field from second-level 
> >> page-tables
> >> and treat the field as Reserved(0) since revision 3.2.
> >>
> >> Changing the field as reserved(0) will break backward compatibility, so
> >> introduce a property "x-stale-tm" to allow user to control the setting.
> >
> >Nit: I think we probably don't need the x prefix? As we try to
> >maintain the compatibility via:
> >
> >> +{ TYPE_INTEL_IOMMU_DEVICE, "x-stale-tm", "on" },
> >
> >?
> 
> I'm fine to remove it. But,
> The prefix "x-" is used to indicate that a feature is experimental.

No, it is used to indicate that the feature is not part of
a stable API.

> Couldn't we have a property both experimental and compatible?
> I see a lot of such properties:
> 
> # grep "x-" /sdb/qemu/hw/i386/pc.c
> { "ICH9-LPC", "x-smi-swsmi-timer", "off" },
> { "ICH9-LPC", "x-smi-periodic-timer", "off" },
> { TYPE_INTEL_IOMMU_DEVICE, "x-stale-tm", "on" },
> { TYPE_X86_CPU, "x-amd-topoext-features-only", "false" },
> { TYPE_X86_CPU, "x-l1-cache-per-thread", "false" },
> { "ICH9-LPC", "x-keep-pci-slot-hpc", "false" },
> { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" },
> { "ICH9-LPC", "x-keep-pci-slot-hpc", "true" },
> { "ICH9-LPC", "x-smi-cpu-hotunplug", "off" },
> { "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
> { TYPE_X86_CPU, "x-intel-pt-auto-level", "off" },
> { TYPE_X86_CPU, "x-hv-synic-kvm-only", "on" },
> { TYPE_X86_CPU, "x-migrate-smi-count", "off" },
> { TYPE_X86_CPU, "x-hv-max-vps", "0x40" },
> { "i440FX-pcihost", "x-pci-hole64-fix", "off" },
> { "q35-pcihost", "x-pci-hole64-fix", "off" },
> { "kvmclock", "x-mach-use-reliable-get-clock", "off" },
> { "ICH9-LPC", "x-smi-broadcast", "off" },
> 
> Thanks
> Zhenzhong