Re: [PATCH v3 5/7] qapi/parser: add QAPIExpression type
John Snow writes: > This patch creates a new type, QAPIExpression, which represents a parsed > expression complete with QAPIDoc and QAPISourceInfo. > > This patch turns parser.exprs into a list of QAPIExpression instead, > and adjusts expr.py to match. > > This allows the types we specify in parser.py to be "remembered" all the > way through expr.py and into schema.py. Several assertions around > packing and unpacking this data can be removed as a result. > > NB: This necessitates a change of typing for check_if() and > check_keys(), because mypy does not believe UserDict[str, object] ⊆ > Dict[str, object]. It will, however, accept Mapping or > MutableMapping. In this case, the immutable form is preferred as an > input parameter because we don't actually mutate the input. > > Without this change, we will observe: > qapi/expr.py:631: error: Argument 1 to "check_keys" has incompatible > type "QAPIExpression"; expected "Dict[str, object]" > > NB2: Python 3.6 has an oversight for typing UserDict that makes it > impossible to type for both runtime and analysis time. The problem is > described in detail at https://github.com/python/typing/issues/60 - this > workaround can be safely removed when 3.7 is our minimum version. > > Signed-off-by: John Snow > --- Looks good to me, just one question: [...] > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > index 1b006cdc133..87b46db7fba 100644 > --- a/scripts/qapi/parser.py > +++ b/scripts/qapi/parser.py > @@ -14,13 +14,14 @@ > # This work is licensed under the terms of the GNU GPL, version 2. > # See the COPYING file in the top-level directory. > > -from collections import OrderedDict > +from collections import OrderedDict, UserDict > import os > import re > from typing import ( > TYPE_CHECKING, > Dict, > List, > +Mapping, > Optional, > Set, > Union, > @@ -37,15 +38,30 @@ > from .schema import QAPISchemaFeature, QAPISchemaMember > > > -#: Represents a single Top Level QAPI schema expression. > -TopLevelExpr = Dict[str, object] > - > # Return value alias for get_expr(). > _ExprValue = Union[List[object], Dict[str, object], str, bool] > > -# FIXME: Consolidate and centralize definitions for TopLevelExpr, > -# _ExprValue, _JSONValue, and _JSONObject; currently scattered across > -# several modules. > + > +# FIXME: Consolidate and centralize definitions for _ExprValue, > +# JSONValue, and _JSONObject; currently scattered across several > +# modules. > + > + > +# 3.6 workaround: can be removed when Python 3.7+ is our required version. > +if TYPE_CHECKING: > +_UserDict = UserDict[str, object] > +else: > +_UserDict = UserDict > + > + > +class QAPIExpression(_UserDict): Can we subclass dict instead? > +def __init__(self, > + initialdata: Mapping[str, object], > + info: QAPISourceInfo, > + doc: Optional['QAPIDoc'] = None): > +super().__init__(initialdata) > +self.info = info > +self.doc: Optional['QAPIDoc'] = doc > > > class QAPIParseError(QAPISourceError): [...]
Re: [PATCH v3 7/7] qapi: remove JSON value FIXME
John Snow writes: > With the two major JSON-ish type hierarchies clarified for distinct > purposes; QAPIExpression for parsed expressions and JSONValue for The comment you remove talks about _ExprValue, not QAPIExpression. > introspection data, remove this FIXME as no longer an action item. > > In theory, it may be possible to define a completely agnostic > one-size-fits-all JSON type hierarchy that any other user could borrow - > in practice, it's tough to wrangle the differences between invariant, > covariant and contravariant types: input and output parameters demand > different properties of such a structure. As such, it's simply more > trouble than it's worth. I think there's a weightier counter-argument struggling to get out. As I explained under v2's cover letter, the two types represent things that are only superficially similar. _ExprValue is the obvious stupid abstract syntax tree for the QAPI schema language, with str and bool leaves (QAPI doesn't support floating-point numbers), OrderedDict and list inner nodes. It is used for parser output. QAPIExpression augments _ExprValue, adding a QAPISourceInfo (identifying the expression's source) and a QAPIDoc (the expressions documentation). It is used to represent QAPI top-level expressions. JSONValue is an annotated JSON abstract syntax tree. QAPIExpression and _ExprValue are related to it only insofar as the QAPI schema language is (rather loosely) patterned after JSON. Moreover, the two ASTs serve different purposes. QAPIExpression and _ExprValue represent *input*: they are produced by a parser and consumed by a semantic analyzer. JSONValue represents *output*: it is produced within a backend so we can separate the JSON text formatting aspect. Consolidating these two ASTs makes no sense. Suggest to work this argument into your commit message. > So, declare this "done enough for now". Agree. > Signed-off-by: John Snow > --- > scripts/qapi/parser.py | 4 > 1 file changed, 4 deletions(-) > > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > index c165bd3912c..b5afdd703e7 100644 > --- a/scripts/qapi/parser.py > +++ b/scripts/qapi/parser.py > @@ -42,10 +42,6 @@ > _ExprValue = Union[List[object], Dict[str, object], str, bool] > > > -# FIXME: Consolidate and centralize definitions for _ExprValue and > -# JSONValue; currently scattered across several modules. > - > - > # 3.6 workaround: can be removed when Python 3.7+ is our required version. > if TYPE_CHECKING: > _UserDict = UserDict[str, object]
Re: [PATCH v3 6/7] qapi: remove _JSONObject
John Snow writes: > We can remove this alias as it only has two usages now, and no longer > pays for the confusion of "yet another type". > > Signed-off-by: John Snow Reviewed-by: Markus Armbruster
[PATCH v2] [PING^3] target/i386/gdbstub: Fix a bug about order of FPU stack in 'g' packets.
This is a ping to the patch below.
https://patchew.org/QEMU/ty0pr0101mb4285923fbe9ad97ce832d95ba4...@ty0pr0101mb4285.apcprd01.prod.exchangelabs.com/
Before this commit, when GDB attached an OS working on QEMU, order of FPU
stack registers printed by GDB command 'info float' was wrong. There was a
bug causing the problem in 'g' packets sent by QEMU to GDB. The packets have
values of registers of machine emulated by QEMU containing FPU stack
registers. There are 2 ways to specify a x87 FPU stack register. The first
is specifying by absolute indexed register names (R0, ..., R7). The second
is specifying by stack top relative indexed register names (ST0, ..., ST7).
Values of the FPU stack registers should be located in 'g' packet and be
ordered by the relative index. But QEMU had located these registers ordered
by the absolute index. After this commit, when QEMU reads registers to make
a 'g' packet, QEMU specifies FPU stack registers by the relative index.
Then, the registers are ordered correctly in the packet. As a result, GDB,
the packet receiver, can print FPU stack registers in the correct order.
Signed-off-by: TaiseiIto
---
target/i386/gdbstub.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index c3a2cf6f28..786971284a 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -121,7 +121,9 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]);
}
} else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
-floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
+int st_index = n - IDX_FP_REGS;
+int r_index = (st_index + env->fpstt) % 8;
+floatx80 *fp = &env->fpregs[r_index].d;
int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
return len;
--
2.34.1
Re: [PATCH v1 RFC Zisslpcfi 2/9] target/riscv: zisslpcfi CSR, bit positions and other definitions
On 2023/2/9 14:23, Deepak Gupta wrote:
`zisslpcfi` extension adds two new CSRs. CSR_SSP and CSR_LPLR.
- CSR_SSP: This CSR holds shadow stack pointer for current privilege mode
CSR_SSP is accessible in all modes. Each mode must establish
it's own CSR_SSP.
- CSR_LPLR: This CSR holds label value set at the callsite by compiler.
On call target label check instructions are emitted by
compiler which check label value against value present in
CSR_LPRL.
Enabling of `zisslpcfi` is controlled via menvcfg (for S/HS/VS/U/VU) and
henvcfg (for VS/VU) at bit position 60.
Each mode has enable/disable bits for forward cfi. Backward cfi doesn't
have separate enable/disable bits for S and M mode. User forward cfi and
user backward cfi enable/disable bits are in mstatus/sstatus CSR.
Supervisor forward cfi enable/disable bit are in menvcfg and henvcfg CSR.
Machine mode forward cfi enable/disable bit is in mseccfg CSR.
If forward cfi enabled, all indirect branches must land on a landing pad
instruction (`lpcll`, introduced in later commits). CPU/hart tracks this
internally using a landing pad tracker called `elp` short for `expecting
landing pad`. An interrupt can occur between an indirect branch and
target. If such an event occurs `elp` is saved away in mstatus/sstatus
CSR
Signed-off-by: Deepak Gupta
Signed-off-by: Kip Walker
---
target/riscv/cpu.h | 5 +
target/riscv/cpu_bits.h | 25 +
target/riscv/pmp.h | 3 ++-
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 9a923760b2..18db61a06a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -181,6 +181,11 @@ struct CPUArchState {
uint32_t features;
+/* CFI Extension user mode registers and state */
+uint32_t lplr;
+target_ulong ssp;
+cfi_elp elp;
+
cfi_elp is not defined in current or previous patch.
#ifdef CONFIG_USER_ONLY
uint32_t elf_flags;
#endif
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 8b0d7e20ea..1663ba5775 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -39,6 +39,10 @@
/* Control and Status Registers */
+/* CFI CSRs */
+#define CSR_LPLR0x006
+#define CSR_SSP 0x020
+
/* User Trap Setup */
#define CSR_USTATUS 0x000
#define CSR_UIE 0x004
@@ -542,6 +546,10 @@
#define MSTATUS_TVM 0x0010 /* since: priv-1.10 */
#define MSTATUS_TW 0x0020 /* since: priv-1.10 */
#define MSTATUS_TSR 0x0040 /* since: priv-1.10 */
+#define MSTATUS_UFCFIEN 0x0080 /* Zisslpcfi-0.1 */
+#define MSTATUS_UBCFIEN 0x0100 /* Zisslpcfi-0.1 */
+#define MSTATUS_SPELP 0x0200 /* Zisslpcfi-0.1 */
+#define MSTATUS_MPELP 0x0400 /* Zisslpcfi-0.1 */
#define MSTATUS_GVA 0x40ULL
#define MSTATUS_MPV 0x80ULL
@@ -572,12 +580,21 @@ typedef enum {
#define SSTATUS_XS 0x00018000
#define SSTATUS_SUM 0x0004 /* since: priv-1.10 */
#define SSTATUS_MXR 0x0008
+#define SSTATUS_UFCFIEN MSTATUS_UFCFIEN /* Zisslpcfi-0.1 */
+#define SSTATUS_UBCFIEN MSTATUS_UBCFIEN /* Zisslpcfi-0.1 */
+#define SSTATUS_SPELP MSTATUS_SPELP /* Zisslpcfi-0.1 */
#define SSTATUS64_UXL 0x0003ULL
#define SSTATUS32_SD0x8000
#define SSTATUS64_SD0x8000ULL
+#define CFISTATUS_M_MASK(MSTATUS_UFCFIEN | MSTATUS_UBCFIEN | \
+ MSTATUS_MPELP | MSTATUS_SPELP)
+
+#define CFISTATUS_S_MASK(SSTATUS_UFCFIEN | SSTATUS_UBCFIEN | \
+ SSTATUS_SPELP)
+
/* hstatus CSR bits */
#define HSTATUS_VSBE 0x0020
#define HSTATUS_GVA 0x0040
@@ -747,10 +764,14 @@ typedef enum RISCVException {
#define MENVCFG_CBIE (3UL << 4)
#define MENVCFG_CBCFE BIT(6)
#define MENVCFG_CBZE BIT(7)
+#define MENVCFG_SFCFIENBIT(59)
+#define MENVCFG_CFIBIT(60)
We should use 1ULL << 59/60 here.
#define MENVCFG_PBMTE (1ULL << 62)
#define MENVCFG_STCE (1ULL << 63)
/* For RV32 */
+#define MENVCFGH_SFCFIEN BIT(27)
+#define MENVCFGH_CFI BIT(28)
#define MENVCFGH_PBMTE BIT(30)
#define MENVCFGH_STCE BIT(31)
@@ -763,10 +784,14 @@ typedef enum RISCVException {
#define HENVCFG_CBIE MENVCFG_CBIE
#define HENVCFG_CBCFE MENVCFG_CBCFE
#define HENVCFG_CBZE MENVCFG_CBZE
+#define HENVCFG_SFCFIENMENVCFG_SFCFIEN
+#define HENVCFG_CFIMENVCFG_CFI
#define HENVCFG_PBMTE MENVCFG_PBMTE
#d
Re: [PATCH v1 RFC Zisslpcfi 1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config
On 2023/2/9 14:23, Deepak Gupta wrote:
Introducing riscv `zisslpcfi` extension to riscv target. `zisslpcfi`
extension provides hardware assistance to riscv hart to enable control
flow integrity (CFI) for software.
`zisslpcfi` extension expects hart to implement `zimops`. `zimops` stands
for "unprivileged integer maybe operations". `zimops` carve out certain
reserved opcodes encodings from integer spec to "may be operations"
encodings. `zimops` opcode encodings simply move 0 to rd.
`zisslpcfi` claims some of the `zimops` encodings and use them for shadow
stack management or indirect branch tracking. Any future extension can
also claim `zimops` encodings.
This patch also adds a dependency check for `zimops` to be enabled if
`zisslpcfi` is enabled on the hart.
Signed-off-by: Deepak Gupta
Signed-off-by: Kip Walker
---
target/riscv/cpu.c | 13 +
target/riscv/cpu.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..6b4e90eb91 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -110,6 +110,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0,
ext_XVentanaCondOps),
+ISA_EXT_DATA_ENTRY(zimops, true, PRIV_VERSION_1_12_0, ext_zimops),
+ISA_EXT_DATA_ENTRY(zisslpcfi, true, PRIV_VERSION_1_12_0, ext_cfi),
By convention, it should be ext_zisslpcfi .
};
static bool isa_ext_is_enabled(RISCVCPU *cpu,
@@ -792,6 +794,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
return;
}
+if (cpu->cfg.ext_cfi && !cpu->cfg.ext_zimops) {
+error_setg(errp, "Zisslpcfi extension requires Zimops extension");
+return;
+}
+
If Zisslpcfi implicitly means Zimops is implemented as commented in
following code, I think we should just enable zimops when zisslpcfi is
enabled.
/* Set the ISA extensions, checks should have happened above */
if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
cpu->cfg.ext_zhinxmin) {
@@ -1102,6 +1109,12 @@ static Property riscv_cpu_properties[] = {
#ifndef CONFIG_USER_ONLY
DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
#endif
+/*
+ * Zisslpcfi CFI extension, Zisslpcfi implicitly means Zimops is
+ * implemented
+ */
+DEFINE_PROP_BOOL("zisslpcfi", RISCVCPU, cfg.ext_cfi, true),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),
These properties can not expose to users before all its functions are
ready. And it need add 'x-' prefix as experimental extensions currently.
Regards,
Weiwei Li
DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5609b62a2..9a923760b2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -471,6 +471,8 @@ struct RISCVCPUConfig {
uint32_t mvendorid;
uint64_t marchid;
uint64_t mimpid;
+bool ext_zimops;
+bool ext_cfi;
/* Vendor-specific custom extensions */
bool ext_XVentanaCondOps;
Re: [PATCH] target/riscv: avoid env_archcpu() in cpu_get_tb_cpu_state()
On 2023/2/10 20:38, Daniel Henrique Barboza wrote: We have a RISCVCPU *cpu pointer available at the start of the function. Signed-off-by: Daniel Henrique Barboza Reviewed-by: Weiwei Li Regards, Weiwei Li --- target/riscv/cpu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index ad8d82662c..3a9472a2ff 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -60,7 +60,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, * which is not supported by GVEC. So we set vl_eq_vlmax flag to true * only when maxsz >= 8 bytes. */ -uint32_t vlmax = vext_get_vlmax(env_archcpu(env), env->vtype); +uint32_t vlmax = vext_get_vlmax(cpu, env->vtype); uint32_t sew = FIELD_EX64(env->vtype, VTYPE, VSEW); uint32_t maxsz = vlmax << sew; bool vl_eq_vlmax = (env->vstart == 0) && (vlmax == env->vl) &&
Re: [PATCH 02/11] target/riscv: allow users to actually write the MISA CSR
On 2023/2/10 21:36, Daniel Henrique Barboza wrote:
At this moment, and apparently since ever, we have no way of enabling
RISCV_FEATURE_MISA. This means that all the code from write_misa(), all
the nuts and bolts that handles how to properly write this CSR, has
always been a no-op as well because write_misa() will always exit
earlier.
This seems to be benign in the majority of cases. Booting an Ubuntu
'virt' guest and logging all the calls to 'write_misa' shows that no
writes to MISA CSR was attempted. Writing MISA, i.e. enabling/disabling
RISC-V extensions after the machine is powered on, seems to be a niche
use.
There is a good chance that the code in write_misa() hasn't been
properly tested. Allowing users to write MISA can open the floodgates of
new breeds of bugs. We could instead remove most (if not all) of
write_misa() since it's never used. Well, as a hardware emulator,
dealing with crashes because a register write went wrong is what we're
here for.
Create a 'misa-w' CPU property to allow users to choose whether writes
to MISA should be allowed. The default is set to 'false' for all RISC-V
machines to keep compatibility with what we´ve been doing so far.
Read cpu->cfg.misa_w directly in write_misa(), instead of executing
riscv_set_feature(RISCV_FEATURE_MISA) in riscv_cpu_realize(), that would
simply reflect the cpu->cfg.misa_w bool value in 'env->features' and
require a riscv_feature() call to read it back.
Signed-off-by: Daniel Henrique Barboza
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 1 +
target/riscv/csr.c | 4 +++-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 93b52b826c..69fb9e123f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1197,6 +1197,7 @@ static void register_cpu_props(DeviceState *dev)
static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
+DEFINE_PROP_BOOL("misa-w", RISCVCPU, cfg.misa_w, false),
DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, RISCV_CPU_MARCHID),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7128438d8e..103963b386 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -498,6 +498,7 @@ struct RISCVCPUConfig {
bool pmp;
bool epmp;
bool debug;
+bool misa_w;
bool short_isa_string;
};
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e149b453da..4f9cc501b2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1329,7 +1329,9 @@ static RISCVException read_misa(CPURISCVState *env, int
csrno,
static RISCVException write_misa(CPURISCVState *env, int csrno,
target_ulong val)
{
-if (!riscv_feature(env, RISCV_FEATURE_MISA)) {
+RISCVCPU *cpu = env_archcpu(env);
+
+if (!cpu->cfg.misa_w) {
It's Ok to get it directly from cfg. However, personally, I prefer to
keep the non-isa features in a separate list.
Regards,
Weiwei Li
/* drop write to misa */
return RISCV_EXCP_NONE;
}
Re: [PATCH 01/11] target/riscv: do not mask unsupported QEMU extensions in write_misa()
On 2023/2/10 21:36, Daniel Henrique Barboza wrote:
The masking done using env->misa_ext_mask already filters any extension
that QEMU doesn't support. If the hart supports the extension then QEMU
supports it as well.
If the masking done by env->misa_ext_mask is somehow letting unsupported
QEMU extensions pass by, misa_ext_mask itself needs to be fixed instead.
Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Weiwei Li
Regards,
Weiwei Li
---
target/riscv/csr.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 1b0a0c1693..e149b453da 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1356,9 +1356,6 @@ static RISCVException write_misa(CPURISCVState *env, int
csrno,
/* Mask extensions that are not supported by this hart */
val &= env->misa_ext_mask;
-/* Mask extensions that are not supported by QEMU */
-val &= (RVI | RVE | RVM | RVA | RVF | RVD | RVC | RVS | RVU | RVV);
-
/* 'D' depends on 'F', so clear 'D' if 'F' is not present */
if ((val & RVD) && !(val & RVF)) {
val &= ~RVD;
[PATCH 1/3] hw/rtc/mc146818rtc: Rename RTCState -> MC146818RtcState
RTCState only represents a Motorola MC146818 model,
not any RTC chipset. Rename the structure as MC146818RtcState
using:
$ sed -i -e s/RTCState/MC146818RtcState/g $(git grep -wl RTCState)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/piix4.c | 2 +-
hw/isa/vt82c686.c| 2 +-
hw/rtc/mc146818rtc.c | 116 +--
include/hw/rtc/mc146818rtc.h | 6 +-
4 files changed, 63 insertions(+), 63 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index de60ceef73..e2fafc3b13 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -47,7 +47,7 @@ struct PIIX4State {
qemu_irq cpu_intr;
qemu_irq *isa;
-RTCState rtc;
+MC146818RtcState rtc;
PCIIDEState ide;
UHCIState uhci;
PIIX4PMState pm;
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 3f9bd0c04d..67cbb658aa 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -550,7 +550,7 @@ struct ViaISAState {
qemu_irq cpu_intr;
qemu_irq *isa_irqs;
ViaSuperIOState via_sio;
-RTCState rtc;
+MC146818RtcState rtc;
PCIIDEState ide;
UHCIState uhci[2];
ViaPMState pm;
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index ba612a151d..08f6c0e0c5 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -71,19 +71,19 @@
#define RTC_ISA_BASE 0x70
-static void rtc_set_time(RTCState *s);
-static void rtc_update_time(RTCState *s);
-static void rtc_set_cmos(RTCState *s, const struct tm *tm);
-static inline int rtc_from_bcd(RTCState *s, int a);
-static uint64_t get_next_alarm(RTCState *s);
+static void rtc_set_time(MC146818RtcState *s);
+static void rtc_update_time(MC146818RtcState *s);
+static void rtc_set_cmos(MC146818RtcState *s, const struct tm *tm);
+static inline int rtc_from_bcd(MC146818RtcState *s, int a);
+static uint64_t get_next_alarm(MC146818RtcState *s);
-static inline bool rtc_running(RTCState *s)
+static inline bool rtc_running(MC146818RtcState *s)
{
return (!(s->cmos_data[RTC_REG_B] & REG_B_SET) &&
(s->cmos_data[RTC_REG_A] & 0x70) <= 0x20);
}
-static uint64_t get_guest_rtc_ns(RTCState *s)
+static uint64_t get_guest_rtc_ns(MC146818RtcState *s)
{
uint64_t guest_clock = qemu_clock_get_ns(rtc_clock);
@@ -91,7 +91,7 @@ static uint64_t get_guest_rtc_ns(RTCState *s)
guest_clock - s->last_update + s->offset;
}
-static void rtc_coalesced_timer_update(RTCState *s)
+static void rtc_coalesced_timer_update(MC146818RtcState *s)
{
if (s->irq_coalesced == 0) {
timer_del(s->coalesced_timer);
@@ -104,19 +104,19 @@ static void rtc_coalesced_timer_update(RTCState *s)
}
}
-static QLIST_HEAD(, RTCState) rtc_devices =
+static QLIST_HEAD(, MC146818RtcState) rtc_devices =
QLIST_HEAD_INITIALIZER(rtc_devices);
void qmp_rtc_reset_reinjection(Error **errp)
{
-RTCState *s;
+MC146818RtcState *s;
QLIST_FOREACH(s, &rtc_devices, link) {
s->irq_coalesced = 0;
}
}
-static bool rtc_policy_slew_deliver_irq(RTCState *s)
+static bool rtc_policy_slew_deliver_irq(MC146818RtcState *s)
{
kvm_reset_irq_delivered();
qemu_irq_raise(s->irq);
@@ -125,7 +125,7 @@ static bool rtc_policy_slew_deliver_irq(RTCState *s)
static void rtc_coalesced_timer(void *opaque)
{
-RTCState *s = opaque;
+MC146818RtcState *s = opaque;
if (s->irq_coalesced != 0) {
s->cmos_data[RTC_REG_C] |= 0xc0;
@@ -140,7 +140,7 @@ static void rtc_coalesced_timer(void *opaque)
rtc_coalesced_timer_update(s);
}
-static uint32_t rtc_periodic_clock_ticks(RTCState *s)
+static uint32_t rtc_periodic_clock_ticks(MC146818RtcState *s)
{
int period_code;
@@ -158,7 +158,7 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
* is just due to period adjustment.
*/
static void
-periodic_timer_update(RTCState *s, int64_t current_time, uint32_t old_period,
bool period_change)
+periodic_timer_update(MC146818RtcState *s, int64_t current_time, uint32_t
old_period, bool period_change)
{
uint32_t period;
int64_t cur_clock, next_irq_clock, lost_clock = 0;
@@ -234,7 +234,7 @@ periodic_timer_update(RTCState *s, int64_t current_time,
uint32_t old_period, bo
static void rtc_periodic_timer(void *opaque)
{
-RTCState *s = opaque;
+MC146818RtcState *s = opaque;
periodic_timer_update(s, s->next_periodic_time, s->period, false);
s->cmos_data[RTC_REG_C] |= REG_C_PF;
@@ -255,7 +255,7 @@ static void rtc_periodic_timer(void *opaque)
}
/* handle update-ended timer */
-static void check_update_timer(RTCState *s)
+static void check_update_timer(MC146818RtcState *s)
{
uint64_t next_update_time;
uint64_t guest_nsec;
@@ -306,7 +306,7 @@ static void check_update_timer(RTCState *s)
}
}
-static inline uint8_t convert_hour(RTCState *s, uint8_t hour)
+static inline uint8_t convert_hour(MC146818RtcState *s, uint8_t hour)
{
if (!(s->cmos_data[RTC_REG_B] & REG_B_24H)) {
hour %= 12;
@@ -317,7 +317
[PATCH 3/3] hw/rtc: Rename rtc_[get|set]_memory -> mc146818rtc_[get|set]_cmos_data
rtc_get_memory() and rtc_set_memory() helpers only work with
TYPE_MC146818_RTC devices. 'memory' in their name refer to
the CMOS region. Rename them as mc146818rtc_get_cmos_data()
and mc146818rtc_set_cmos_data() to be explicit about what
they are doing.
Mechanical change doing:
$ sed -i -e 's/rtc_set_memory/mc146818rtc_set_cmos_data/g' \
$(git grep -wl rtc_set_memory)
$ sed -i -e 's/rtc_get_memory/mc146818rtc_get_cmos_data/g' \
$(git grep -wl rtc_get_memory)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/i386/microvm.c| 22 +++---
hw/i386/pc.c | 58 ++--
hw/i386/x86.c| 4 +--
hw/ppc/prep.c| 8 ++---
hw/rtc/mc146818rtc.c | 6 ++--
include/hw/rtc/mc146818rtc.h | 4 +--
6 files changed, 51 insertions(+), 51 deletions(-)
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 04b453cde5..2302810117 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -63,8 +63,8 @@ static void microvm_set_rtc(MicrovmMachineState *mms,
MC146818RtcState *s)
int val;
val = MIN(x86ms->below_4g_mem_size / KiB, 640);
-rtc_set_memory(s, 0x15, val);
-rtc_set_memory(s, 0x16, val >> 8);
+mc146818rtc_set_cmos_data(s, 0x15, val);
+mc146818rtc_set_cmos_data(s, 0x16, val >> 8);
/* extended memory (next 64MiB) */
if (x86ms->below_4g_mem_size > 1 * MiB) {
val = (x86ms->below_4g_mem_size - 1 * MiB) / KiB;
@@ -74,10 +74,10 @@ static void microvm_set_rtc(MicrovmMachineState *mms,
MC146818RtcState *s)
if (val > 65535) {
val = 65535;
}
-rtc_set_memory(s, 0x17, val);
-rtc_set_memory(s, 0x18, val >> 8);
-rtc_set_memory(s, 0x30, val);
-rtc_set_memory(s, 0x31, val >> 8);
+mc146818rtc_set_cmos_data(s, 0x17, val);
+mc146818rtc_set_cmos_data(s, 0x18, val >> 8);
+mc146818rtc_set_cmos_data(s, 0x30, val);
+mc146818rtc_set_cmos_data(s, 0x31, val >> 8);
/* memory between 16MiB and 4GiB */
if (x86ms->below_4g_mem_size > 16 * MiB) {
val = (x86ms->below_4g_mem_size - 16 * MiB) / (64 * KiB);
@@ -87,13 +87,13 @@ static void microvm_set_rtc(MicrovmMachineState *mms,
MC146818RtcState *s)
if (val > 65535) {
val = 65535;
}
-rtc_set_memory(s, 0x34, val);
-rtc_set_memory(s, 0x35, val >> 8);
+mc146818rtc_set_cmos_data(s, 0x34, val);
+mc146818rtc_set_cmos_data(s, 0x35, val >> 8);
/* memory above 4GiB */
val = x86ms->above_4g_mem_size / 65536;
-rtc_set_memory(s, 0x5b, val);
-rtc_set_memory(s, 0x5c, val >> 8);
-rtc_set_memory(s, 0x5d, val >> 16);
+mc146818rtc_set_cmos_data(s, 0x5b, val);
+mc146818rtc_set_cmos_data(s, 0x5c, val >> 8);
+mc146818rtc_set_cmos_data(s, 0x5d, val >> 16);
}
static void create_gpex(MicrovmMachineState *mms)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 606686dafc..71aaa3875f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -442,16 +442,16 @@ static uint64_t ioportF0_read(void *opaque, hwaddr addr,
unsigned size)
static void cmos_init_hd(MC146818RtcState *s, int type_ofs, int info_ofs,
int16_t cylinders, int8_t heads, int8_t sectors)
{
-rtc_set_memory(s, type_ofs, 47);
-rtc_set_memory(s, info_ofs, cylinders);
-rtc_set_memory(s, info_ofs + 1, cylinders >> 8);
-rtc_set_memory(s, info_ofs + 2, heads);
-rtc_set_memory(s, info_ofs + 3, 0xff);
-rtc_set_memory(s, info_ofs + 4, 0xff);
-rtc_set_memory(s, info_ofs + 5, 0xc0 | ((heads > 8) << 3));
-rtc_set_memory(s, info_ofs + 6, cylinders);
-rtc_set_memory(s, info_ofs + 7, cylinders >> 8);
-rtc_set_memory(s, info_ofs + 8, sectors);
+mc146818rtc_set_cmos_data(s, type_ofs, 47);
+mc146818rtc_set_cmos_data(s, info_ofs, cylinders);
+mc146818rtc_set_cmos_data(s, info_ofs + 1, cylinders >> 8);
+mc146818rtc_set_cmos_data(s, info_ofs + 2, heads);
+mc146818rtc_set_cmos_data(s, info_ofs + 3, 0xff);
+mc146818rtc_set_cmos_data(s, info_ofs + 4, 0xff);
+mc146818rtc_set_cmos_data(s, info_ofs + 5, 0xc0 | ((heads > 8) << 3));
+mc146818rtc_set_cmos_data(s, info_ofs + 6, cylinders);
+mc146818rtc_set_cmos_data(s, info_ofs + 7, cylinders >> 8);
+mc146818rtc_set_cmos_data(s, info_ofs + 8, sectors);
}
/* convert boot_device letter to something recognizable by the bios */
@@ -491,8 +491,8 @@ static void set_boot_dev(MC146818RtcState *s, const char
*boot_device,
return;
}
}
-rtc_set_memory(s, 0x3d, (bds[1] << 4) | bds[0]);
-rtc_set_memory(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 : 0x1));
+mc146818rtc_set_cmos_data(s, 0x3d, (bds[1] << 4) | bds[0]);
+mc146818rtc_set_cmos_data(s, 0x38, (bds[2] << 4) | (fd_bootchk ? 0x0 :
0x1));
}
static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
@@ -514,9 +514,9 @@ static void pc_cmos_init_floppy(MC146818RtcState
*rtc_state, ISADevice *floppy)
}
val = (cmos_get_fd_drive_type(fd_
[PATCH 2/3] hw/rtc/mc146818rtc: Pass MC146818RtcState instead of ISADevice argument
rtc_get_memory() and rtc_set_memory() methods can not take any
TYPE_ISA_DEVICE object. They expect a TYPE_MC146818_RTC one.
Simplify the API by passing a MC146818RtcState.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/i386/microvm.c| 6 ++
hw/i386/pc.c | 16 +---
hw/i386/x86.c| 4 +++-
hw/ppc/prep.c| 3 +--
hw/rtc/mc146818rtc.c | 13 ++---
include/hw/rtc/mc146818rtc.h | 8
6 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 29f30dd6d3..04b453cde5 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -57,7 +57,7 @@
#define MICROVM_QBOOT_FILENAME "qboot.rom"
#define MICROVM_BIOS_FILENAME "bios-microvm.bin"
-static void microvm_set_rtc(MicrovmMachineState *mms, ISADevice *s)
+static void microvm_set_rtc(MicrovmMachineState *mms, MC146818RtcState *s)
{
X86MachineState *x86ms = X86_MACHINE(mms);
int val;
@@ -161,7 +161,6 @@ static void microvm_devices_init(MicrovmMachineState *mms)
const char *default_firmware;
X86MachineState *x86ms = X86_MACHINE(mms);
ISABus *isa_bus;
-ISADevice *rtc_state;
GSIState *gsi_state;
int ioapics;
int i;
@@ -267,8 +266,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
if (mms->rtc == ON_OFF_AUTO_ON ||
(mms->rtc == ON_OFF_AUTO_AUTO && !kvm_enabled())) {
-rtc_state = mc146818_rtc_init(isa_bus, 2000, NULL);
-microvm_set_rtc(mms, rtc_state);
+microvm_set_rtc(mms, mc146818_rtc_init(isa_bus, 2000, NULL));
}
if (mms->isa_serial) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6e592bd969..606686dafc 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -439,7 +439,7 @@ static uint64_t ioportF0_read(void *opaque, hwaddr addr,
unsigned size)
#define REG_EQUIPMENT_BYTE 0x14
-static void cmos_init_hd(ISADevice *s, int type_ofs, int info_ofs,
+static void cmos_init_hd(MC146818RtcState *s, int type_ofs, int info_ofs,
int16_t cylinders, int8_t heads, int8_t sectors)
{
rtc_set_memory(s, type_ofs, 47);
@@ -471,7 +471,8 @@ static int boot_device2nibble(char boot_device)
return 0;
}
-static void set_boot_dev(ISADevice *s, const char *boot_device, Error **errp)
+static void set_boot_dev(MC146818RtcState *s, const char *boot_device,
+ Error **errp)
{
#define PC_MAX_BOOT_DEVICES 3
int nbds, bds[3] = { 0, };
@@ -499,7 +500,7 @@ static void pc_boot_set(void *opaque, const char
*boot_device, Error **errp)
set_boot_dev(opaque, boot_device, errp);
}
-static void pc_cmos_init_floppy(ISADevice *rtc_state, ISADevice *floppy)
+static void pc_cmos_init_floppy(MC146818RtcState *rtc_state, ISADevice *floppy)
{
int val, nb, i;
FloppyDriveType fd_type[2] = { FLOPPY_DRIVE_TYPE_NONE,
@@ -537,7 +538,7 @@ static void pc_cmos_init_floppy(ISADevice *rtc_state,
ISADevice *floppy)
}
typedef struct pc_cmos_init_late_arg {
-ISADevice *rtc_state;
+MC146818RtcState *rtc_state;
BusState *idebus[2];
} pc_cmos_init_late_arg;
@@ -604,7 +605,7 @@ static ISADevice *pc_find_fdc0(void)
static void pc_cmos_init_late(void *opaque)
{
pc_cmos_init_late_arg *arg = opaque;
-ISADevice *s = arg->rtc_state;
+MC146818RtcState *s = arg->rtc_state;
int16_t cylinders;
int8_t heads, sectors;
int val;
@@ -646,11 +647,12 @@ static void pc_cmos_init_late(void *opaque)
void pc_cmos_init(PCMachineState *pcms,
BusState *idebus0, BusState *idebus1,
- ISADevice *s)
+ ISADevice *rtc)
{
int val;
static pc_cmos_init_late_arg arg;
X86MachineState *x86ms = X86_MACHINE(pcms);
+MC146818RtcState *s = MC146818_RTC(rtc);
/* various important CMOS locations needed by PC/Bochs bios */
@@ -1304,7 +1306,7 @@ void pc_basic_device_init(struct PCMachineState *pcms,
pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
}
-*rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
+*rtc_state = ISA_DEVICE(mc146818_rtc_init(isa_bus, 2000, rtc_irq));
qemu_register_boot_set(pc_boot_set, *rtc_state);
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index eaff4227bd..5dbdd75bfc 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -151,8 +151,10 @@ void x86_cpus_init(X86MachineState *x86ms, int
default_cpu_version)
}
}
-void x86_rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
+void x86_rtc_set_cpus_count(ISADevice *s, uint16_t cpus_count)
{
+MC146818RtcState *rtc = MC146818_RTC(s);
+
if (cpus_count > 0xff) {
/*
* If the number of CPUs can't be represented in 8 bits, the
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index fcbe4c5837..076e2d0d22 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -212,10 +212,9 @@ static int PPC_NVRAM_set_params (
[PATCH 0/3] hw/rtc: Rename RTCState -> MC146818RtcState and adapt API
rtc_get_memory() and rtc_set_memory() helpers only work with MC146818 RTC devices, not any ISA device. Rename accordingly including 'MC146818' in the method names. Philippe Mathieu-Daudé (3): hw/rtc/mc146818rtc: Rename RTCState -> MC146818RtcState hw/rtc/mc146818rtc: Pass MC146818RtcState instead of ISADevice argument hw/rtc: Rename rtc_[get|set]_memory -> mc146818rtc_[get|set]_cmos_data hw/i386/microvm.c| 28 hw/i386/pc.c | 74 +++-- hw/i386/x86.c| 8 ++- hw/isa/piix4.c | 2 +- hw/isa/vt82c686.c| 2 +- hw/ppc/prep.c| 11 ++- hw/rtc/mc146818rtc.c | 125 +-- include/hw/rtc/mc146818rtc.h | 14 ++-- 8 files changed, 132 insertions(+), 132 deletions(-) -- 2.38.1
Re: [PATCH] xen/pt: fix igd passthrough for pc machine with xen accelerator
On Tue, 7 Feb 2023, Chuck Zmudzinski wrote:
> Commit 998250e97661 ("xen, gfx passthrough: register host bridge specific
> to passthrough") uses the igd-passthrough-i440FX pci host device with
> the xenfv machine type and igd-passthru=on, but using it for the pc
> machine type, xen accelerator, and igd-passtru=on was omitted from that
> commit.
>
> The igd-passthru-i440FX pci host device is also needed for guests
> configured with the pc machine type, the xen accelerator, and
> igd-passthru=on. Specifically, tests show that not using the igd-specific
> pci host device with the Intel igd passed through to the guest results
> in slower startup performance and reduced resolution of the display
> during startup. This patch fixes this issue.
>
> To simplify the logic that is needed to support both the --enable-xen
> and the --disable-xen configure options, introduce the boolean symbol
> pc_xen_igd_gfx_pt_enabled() whose value is set appropriately in the
> sysemu/xen.h header file as the test to determine whether or not
> to use the igd-passthrough-i440FX pci host device instead of the
> normal i440FX pci host device.
>
> Fixes: 998250e97661 ("xen, gfx passthrough: register host bridge specific to
> passthrough")
> Signed-off-by: Chuck Zmudzinski
I think this is OK
Acked-by: Stefano Stabellini
> ---
> This patch is intended to replace or complement a recently proposed
> patch that modifies slot_reserved_mask for the xenfv machine with
> igd-passthru=on in order to fix the problem of Qemu not reserving slot 2
> for the Intel IGD for the xenfv machine type. This patch provides a
> simple way to improve Qemu support for the Intel IGD with the xen
> accelerator without needing to change how slot_reserved_mask functions.
>
> For reference, the latest version of the patch to fix the xenfv machine
> using slot_reserved_mask is at:
>
> https://lore.kernel.org/qemu-devel/b1b4a21fe9a600b1322742dda55a40e9961daa57.1674346505.git.brchu...@aol.com/
>
> Reason for introducing the new boolean pc_xen_igd_gfx_pt_enabled():
>
> It is also possible to use xen_igd_gfx_pt_enabled() directly to check
> if the igd-passthru-i440FX pci host device is needed in this patch,
> but in that case it would be necessary to implement it in
> accel/stubs/xen-stub.c like this:
>
> bool xen_igd_gfx_pt_enabled(void)
> {
> return false;
> }
>
> to cover the case when Qemu is configured with --disable-xen. I thought
> it was simpler to introduce the same boolean prefixed with pc_ and
> set it to 0 when --disable-xen is the configure option, and that explains
> why the proposed patch introduces pc_xen_igd_gfx_pt_enabled() instead of
> using xen_igd_gfx_pt_enabled() directly.
>
> Another reason to use pc_xen_igd_gfx_pt_enabled() is to distinguish it
> from xen_igd_gfx_pt_enabled() in hw/i386/pc_piix.c, because the use of
> xen_igd_gfx_pt_enabled() is guarded by CONFIG_XEN but this patch needs
> to place the boolean in a position that is not guarded by CONFIG_XEN.
> This approach will simplify any future effort to move the code in
> pc_piix.c that is not guarded by CONFIG_XEN to a xen-specific file.
>
> hw/i386/pc_piix.c| 2 ++
> include/sysemu/xen.h | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index df64dd8dcc..fd5b9ae1eb 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -433,6 +433,8 @@ static void pc_xen_hvm_init(MachineState *machine)
> compat(machine); \
> } \
> pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \
> + pc_xen_igd_gfx_pt_enabled() ? \
> + TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : \
> TYPE_I440FX_PCI_DEVICE); \
> } \
> DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn)
> diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
> index 0ca25697e4..99ae41e619 100644
> --- a/include/sysemu/xen.h
> +++ b/include/sysemu/xen.h
> @@ -23,6 +23,7 @@
> extern bool xen_allowed;
>
> #define xen_enabled() (xen_allowed)
> +#define pc_xen_igd_gfx_pt_enabled()xen_igd_gfx_pt_enabled()
>
> #ifndef CONFIG_USER_ONLY
> void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
> @@ -33,6 +34,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
> #else /* !CONFIG_XEN_IS_POSSIBLE */
>
> #define xen_enabled() 0
> +#define pc_xen_igd_gfx_pt_enabled() 0
> #ifndef CONFIG_USER_ONLY
> static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t
> length)
> {
> --
> 2.39.1
>
[PATCH 5/9] bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt
From: Juergen Lock
Helper functions for sysctl implementations. sysctl_name2oid and
sysctl_oidfmt convert oids between host and targets
Signed-off-by: Juergen Lock
Signed-off-by: Warner Losh
---
bsd-user/freebsd/os-sys.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index e3b9f168a2b..ac5ab9b17bc 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -115,6 +115,24 @@ static int sysctl_oldcvt(void *holdp, size_t *holdlen,
uint32_t kind)
return 0;
}
+/*
+ * Convert the undocmented name2oid sysctl data for the target.
+ */
+static inline void sysctl_name2oid(uint32_t *holdp, size_t holdlen)
+{
+size_t i, num = holdlen / sizeof(uint32_t);
+
+for (i = 0; i < num; i++) {
+holdp[i] = tswap32(holdp[i]);
+}
+}
+
+static inline void sysctl_oidfmt(uint32_t *holdp)
+{
+/* byte swap the kind */
+holdp[0] = tswap32(holdp[0]);
+}
+
/* sysarch() is architecture dependent. */
abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
{
--
2.39.1
[PATCH 3/9] bsd-user: Add sysarch syscall
From: Stacey Son
Connect up the sysarch system call.
Signed-off-by: Juergen Lock
Co-authored-by: Juergen Lock
Signed-off-by: Stacey Son
Reviewed-by: Warner Losh
Signed-off-by: Warner Losh
---
bsd-user/freebsd/os-syscall.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index b4a663fc021..e00997a818c 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -491,6 +491,13 @@ static abi_long freebsd_syscall(void *cpu_env, int num,
abi_long arg1,
ret = do_bsd_undelete(arg1);
break;
+/*
+ * sys{ctl, arch, call}
+ */
+case TARGET_FREEBSD_NR_sysarch: /* sysarch(2) */
+ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
+break;
+
default:
qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
ret = -TARGET_ENOSYS;
--
2.39.1
[PATCH 1/9] bsd-user: Don't truncate the return value from freebsd_syscall
From: Doug Rabson
System call return values on FreeBSD are in a register (which is spelled
api_long in qemu). This was being assigned into an int variable which
causes problems for 64bit targets.
Resolves: https://github.com/qemu-bsd-user/qemu-bsd-user/issues/40
Signed-off-by: Doug Rabson
Reviewed-by: Warner Losh
[ Edited commit message for upstreaming into qemu-project ]
Signed-off-by: Warner Losh
---
bsd-user/freebsd/os-syscall.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 57996cad8ae..b4a663fc021 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -512,7 +512,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num,
abi_long arg1,
abi_long arg8)
{
CPUState *cpu = env_cpu(cpu_env);
-int ret;
+abi_long ret;
trace_guest_user_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6,
arg7, arg8);
if (do_strace) {
--
2.39.1
[PATCH 2/9] build: Don't specify -no-pie for --static user-mode programs
When building with clang, -no-pie gives a warning on every single build, so remove it. Signed-off-by: Warner Losh --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 64960c6000f..eb284ccf308 100755 --- a/configure +++ b/configure @@ -1313,7 +1313,7 @@ if test "$static" = "yes"; then error_exit "-static-pie not available due to missing toolchain support" else pie="no" -QEMU_CFLAGS="-fno-pie -no-pie $QEMU_CFLAGS" +QEMU_CFLAGS="-fno-pie $QEMU_CFLAGS" fi elif test "$pie" = "no"; then if compile_prog "-Werror -fno-pie" "-no-pie"; then -- 2.39.1
[PATCH 8/9] bsd-user: implement sysctlbyname(2)
From: Kyle Evans
do_freebsd_sysctlbyname needs to translate the 'name' back down to a OID
so we can intercept the special ones. Do that and call the common wrapper
do_freebsd_sysctl_oid.
Signed-off-by: Kyle Evans
Signed-off-by: Warner Losh
---
bsd-user/freebsd/os-sys.c | 58 +++
bsd-user/freebsd/os-syscall.c | 4 +++
2 files changed, 62 insertions(+)
diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index 13736936e5f..62c729dfe47 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -345,6 +345,64 @@ out:
return ret;
}
+/*
+ * This syscall was created to make sysctlbyname(3) more efficient.
+ * Unfortunately, because we have to fake some sysctls, we can't do that.
+ */
+abi_long do_freebsd_sysctlbyname(CPUArchState *env, abi_ulong namep,
+int32_t namelen, abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp,
+abi_ulong newlen)
+{
+abi_long ret;
+void *holdp = NULL, *hnewp = NULL;
+char *snamep;
+int oid[CTL_MAXNAME + 2];
+size_t holdlen, oidplen;
+abi_ulong oldlen = 0;
+
+if (oldlenp) {
+if (get_user_ual(oldlen, oldlenp)) {
+return -TARGET_EFAULT;
+}
+}
+snamep = lock_user_string(namep);
+if (snamep == NULL) {
+return -TARGET_EFAULT;
+}
+if (newp) {
+hnewp = lock_user(VERIFY_READ, newp, newlen, 1);
+if (hnewp == NULL) {
+return -TARGET_EFAULT;
+}
+}
+if (oldp) {
+holdp = lock_user(VERIFY_WRITE, oldp, oldlen, 0);
+if (holdp == NULL) {
+return -TARGET_EFAULT;
+}
+}
+holdlen = oldlen;
+
+oidplen = sizeof(oid) / sizeof(int);
+if (sysctlnametomib(snamep, oid, &oidplen) != 0) {
+return -TARGET_EINVAL;
+}
+
+ret = do_freebsd_sysctl_oid(env, oid, oidplen, holdp, &holdlen, hnewp,
+newlen);
+
+if (oldlenp) {
+put_user_ual(holdlen, oldlenp);
+}
+unlock_user(snamep, namep, 0);
+unlock_user(holdp, oldp, holdlen);
+if (hnewp) {
+unlock_user(hnewp, newp, 0);
+}
+
+return ret;
+}
+
abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen)
{
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 20ab3d4d9a1..179a20c304b 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -498,6 +498,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num,
abi_long arg1,
ret = do_freebsd_sysctl(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6);
break;
+case TARGET_FREEBSD_NR___sysctlbyname: /* sysctlbyname(2) */
+ret = do_freebsd_sysctlbyname(cpu_env, arg1, arg2, arg3, arg4, arg5,
arg6);
+break;
+
case TARGET_FREEBSD_NR_sysarch: /* sysarch(2) */
ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
break;
--
2.39.1
[PATCH 7/9] bsd-user: do_freebsd_sysctl helper for sysctl(2)
From: Kyle Evans
Implement the wrapper function for sysctl(2). This puts the oid
arguments into a standard form and calls the common
do_freebsd_sysctl_oid.
Signed-off-by: Kyle Evans
Co-Authored-by: Juergen Lock
Signed-off-by: Juergen Lock
Co-Authored-by: Stacey Son
Signed-off-by: Stacey Son
Signed-off-by: Warner Losh
---
bsd-user/freebsd/os-sys.c | 50 +++
bsd-user/freebsd/os-syscall.c | 4 +++
2 files changed, 54 insertions(+)
diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index a8fb29f36b7..13736936e5f 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -345,6 +345,56 @@ out:
return ret;
}
+abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
+abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen)
+{
+abi_long ret;
+void *hnamep, *holdp = NULL, *hnewp = NULL;
+size_t holdlen;
+abi_ulong oldlen = 0;
+int32_t *snamep = g_malloc(sizeof(int32_t) * namelen), *p, *q, i;
+
+if (oldlenp) {
+if (get_user_ual(oldlen, oldlenp)) {
+return -TARGET_EFAULT;
+}
+}
+hnamep = lock_user(VERIFY_READ, namep, namelen, 1);
+if (hnamep == NULL) {
+return -TARGET_EFAULT;
+}
+if (newp) {
+hnewp = lock_user(VERIFY_READ, newp, newlen, 1);
+if (hnewp == NULL) {
+return -TARGET_EFAULT;
+}
+}
+if (oldp) {
+holdp = lock_user(VERIFY_WRITE, oldp, oldlen, 0);
+if (holdp == NULL) {
+return -TARGET_EFAULT;
+}
+}
+holdlen = oldlen;
+for (p = hnamep, q = snamep, i = 0; i < namelen; p++, i++) {
+*q++ = tswap32(*p);
+}
+
+ret = do_freebsd_sysctl_oid(env, snamep, namelen, holdp, &holdlen, hnewp,
+newlen);
+
+if (oldlenp) {
+put_user_ual(holdlen, oldlenp);
+}
+unlock_user(hnamep, namep, 0);
+unlock_user(holdp, oldp, holdlen);
+if (hnewp) {
+unlock_user(hnewp, newp, 0);
+}
+g_free(snamep);
+return ret;
+}
+
/* sysarch() is architecture dependent. */
abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
{
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index e00997a818c..20ab3d4d9a1 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -494,6 +494,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num,
abi_long arg1,
/*
* sys{ctl, arch, call}
*/
+case TARGET_FREEBSD_NR___sysctl: /* sysctl(3) */
+ret = do_freebsd_sysctl(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6);
+break;
+
case TARGET_FREEBSD_NR_sysarch: /* sysarch(2) */
ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
break;
--
2.39.1
[PATCH 6/9] bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants
From: Juergen Lock
do_freebsd_sysctl_oid filters out some of the binary and special sysctls
where host != target. This commit focuses on the simple sysctls that can
be done in a few lines.
Signed-off-by: Juergen Lock
Co-Authored-by: Stacey Son
Signed-off-by: Stacey Son
Signed-off-by: Warner Losh
---
bsd-user/freebsd/os-sys.c | 212 ++
bsd-user/qemu.h | 5 +
2 files changed, 217 insertions(+)
diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index ac5ab9b17bc..a8fb29f36b7 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -133,6 +133,218 @@ static inline void sysctl_oidfmt(uint32_t *holdp)
holdp[0] = tswap32(holdp[0]);
}
+#define bsd_get_ncpu() 1 /* Placeholder */
+
+static abi_long do_freebsd_sysctl_oid(CPUArchState *env, int32_t *snamep,
+int32_t namelen, void *holdp, size_t *holdlenp, void *hnewp,
+size_t newlen)
+{
+uint32_t kind = 0;
+#if TARGET_ABI_BITS != HOST_LONG_BITS
+const abi_ulong maxmem = -0x100c000;
+#endif
+abi_long ret;
+size_t holdlen, oldlen;
+
+holdlen = oldlen = *holdlenp;
+oidfmt(snamep, namelen, NULL, &kind);
+
+/* Handle some arch/emulator dependent sysctl()'s here. */
+switch (snamep[0]) {
+#if defined(TARGET_PPC) || defined(TARGET_PPC64)
+case CTL_MACHDEP:
+switch (snamep[1]) {
+case 1:/* CPU_CACHELINE */
+holdlen = sizeof(uint32_t);
+(*(uint32_t *)holdp) = tswap32(env->dcache_line_size);
+ret = 0;
+goto out;
+}
+break;
+#endif
+case CTL_KERN:
+switch (snamep[1]) {
+case KERN_USRSTACK:
+if (oldlen) {
+(*(abi_ulong *)holdp) = tswapal(TARGET_USRSTACK);
+}
+holdlen = sizeof(abi_ulong);
+ret = 0;
+goto out;
+
+case KERN_PS_STRINGS:
+if (oldlen) {
+(*(abi_ulong *)holdp) = tswapal(TARGET_PS_STRINGS);
+}
+holdlen = sizeof(abi_ulong);
+ret = 0;
+goto out;
+
+default:
+break;
+}
+break;
+
+case CTL_HW:
+switch (snamep[1]) {
+case HW_MACHINE:
+holdlen = sizeof(TARGET_HW_MACHINE);
+if (holdp) {
+strlcpy(holdp, TARGET_HW_MACHINE, oldlen);
+}
+ret = 0;
+goto out;
+
+case HW_MACHINE_ARCH:
+{
+holdlen = sizeof(TARGET_HW_MACHINE_ARCH);
+if (holdp) {
+strlcpy(holdp, TARGET_HW_MACHINE_ARCH, oldlen);
+}
+ret = 0;
+goto out;
+}
+case HW_NCPU:
+if (oldlen) {
+(*(int32_t *)holdp) = tswap32(bsd_get_ncpu());
+}
+holdlen = sizeof(int32_t);
+ret = 0;
+goto out;
+#if defined(TARGET_ARM)
+case HW_FLOATINGPT:
+if (oldlen) {
+#ifdef ARM_FEATURE_VFP /* XXX FIXME XXX */
+if (env->features & ((1ULL << ARM_FEATURE_VFP)|
+ (1ULL << ARM_FEATURE_VFP3)|
+ (1ULL << ARM_FEATURE_VFP4)))
+*(int32_t *)holdp = 1;
+else
+*(int32_t *)holdp = 0;
+#else
+*(int32_t *)holdp = 1;
+#endif
+}
+holdlen = sizeof(int32_t);
+ret = 0;
+goto out;
+#endif
+
+
+#if TARGET_ABI_BITS != HOST_LONG_BITS
+case HW_PHYSMEM:
+case HW_USERMEM:
+case HW_REALMEM:
+holdlen = sizeof(abi_ulong);
+ret = 0;
+
+if (oldlen) {
+int mib[2] = {snamep[0], snamep[1]};
+unsigned long lvalue;
+size_t len = sizeof(lvalue);
+
+if (sysctl(mib, 2, &lvalue, &len, NULL, 0) == -1) {
+ret = -1;
+} else {
+if (((unsigned long)maxmem) < lvalue) {
+lvalue = maxmem;
+}
+(*(abi_ulong *)holdp) = tswapal((abi_ulong)lvalue);
+}
+}
+goto out;
+#endif
+
+default:
+{
+static int oid_hw_availpages;
+static int oid_hw_pagesizes;
+
+if (!oid_hw_availpages) {
+int real_oid[CTL_MAXNAME + 2];
+size_t len = sizeof(real_oid) / sizeof(int);
+
+if (sysctlnametomib("hw.availpages", real_oid, &len) >= 0) {
+oid_hw_availpages = real_oid[1];
+}
+}
+if (!oid_hw_pagesizes) {
+int real_oid[CTL_MAXNAME + 2];
+size_t len = sizeof(real_oid) / sizeof(int);
+
+if (sysctlnametomib("hw.pagesizes", real_oid, &len) >= 0) {
+oid_hw_pagesizes = real_oid[1];
+
[PATCH 4/9] bsd-user: Two helper routines oidfmt and sysctl_oldcvt
From: Stacey Son
oidfmt uses undocumented system call to get the type of the sysctl.
sysctl_oldcvt does the byte swapping in the data to return it to the
target.
Co-Authored-by: Sean Bruno
Signed-off-by: Sean Bruno
Co-Authored-by: Juergen Lock
Signed-off-by: Juergen Lock
Co-Authored-by: Raphael Kubo da Costa
Signed-off-by: Raphael Kubo da Costa
Signed-off-by: Stacey Son
Signed-off-by: Warner Losh
---
bsd-user/freebsd/os-sys.c | 94 +++
1 file changed, 94 insertions(+)
diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index 1676ec10f83..e3b9f168a2b 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -21,6 +21,100 @@
#include "qemu.h"
#include "target_arch_sysarch.h"
+#include
+
+/*
+ * This uses the undocumented oidfmt interface to find the kind of a requested
+ * sysctl, see /sys/kern/kern_sysctl.c:sysctl_sysctl_oidfmt() (compare to
+ * src/sbin/sysctl/sysctl.c)
+ */
+static int oidfmt(int *oid, int len, char *fmt, uint32_t *kind)
+{
+int qoid[CTL_MAXNAME + 2];
+uint8_t buf[BUFSIZ];
+int i;
+size_t j;
+
+qoid[0] = 0;
+qoid[1] = 4;
+memcpy(qoid + 2, oid, len * sizeof(int));
+
+j = sizeof(buf);
+i = sysctl(qoid, len + 2, buf, &j, 0, 0);
+if (i) {
+return i;
+}
+
+if (kind) {
+*kind = *(uint32_t *)buf;
+}
+
+if (fmt) {
+strcpy(fmt, (char *)(buf + sizeof(uint32_t)));
+}
+return 0;
+}
+
+/*
+ * try and convert sysctl return data for the target.
+ * Note: doesn't handle CTLTYPE_OPAQUE and CTLTYPE_STRUCT.
+ */
+static int sysctl_oldcvt(void *holdp, size_t *holdlen, uint32_t kind)
+{
+switch (kind & CTLTYPE) {
+case CTLTYPE_INT:
+case CTLTYPE_UINT:
+*(uint32_t *)holdp = tswap32(*(uint32_t *)holdp);
+break;
+
+#ifdef TARGET_ABI32
+case CTLTYPE_LONG:
+case CTLTYPE_ULONG:
+/*
+ * If the sysctl has a type of long/ulong but seems to be bigger than
+ * these data types, its probably an array. Double check that its
+ * evenly divisible by the size of long and convert holdp to a series
of
+ * 32bit elements instead, adjusting holdlen to the new size.
+ */
+if ((*holdlen > sizeof(abi_ulong)) &&
+((*holdlen % sizeof(abi_ulong)) == 0)) {
+int array_size = *holdlen / sizeof(long);
+int i;
+if (holdp) {
+for (i = 0; i < array_size; i++) {
+((uint32_t *)holdp)[i] = tswap32(((long *)holdp)[i]);
+}
+*holdlen = array_size * sizeof(abi_ulong);
+} else {
+*holdlen = sizeof(abi_ulong);
+}
+} else {
+*(uint32_t *)holdp = tswap32(*(long *)holdp);
+*holdlen = sizeof(uint32_t);
+}
+break;
+#else
+case CTLTYPE_LONG:
+*(uint64_t *)holdp = tswap64(*(long *)holdp);
+break;
+case CTLTYPE_ULONG:
+*(uint64_t *)holdp = tswap64(*(unsigned long *)holdp);
+break;
+#endif
+case CTLTYPE_U64:
+case CTLTYPE_S64:
+*(uint64_t *)holdp = tswap64(*(uint64_t *)holdp);
+break;
+
+case CTLTYPE_STRING:
+break;
+
+default:
+return -1;
+}
+return 0;
+}
+
/* sysarch() is architecture dependent. */
abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
{
--
2.39.1
[PATCH 0/9] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl
This group of patches gets the basic framework for sysctl upstreamed. There's a lot more to translate far too many binary blobs the kernel publishes via sysctls, but I'm leaving those out in the name of simplicity. There's also a bug fix from Doug Rabson that fixes a long int confusion leading to a trunctation of addresses (oops) I've added a new command line arg -strict for debugging when you want to stop dead in the tracks when there's something unimplemented (like system calls) rather than trying one's best to cope. It will also let whoever is working on upstreaming to get something working to run it this way and find the missing bits more easily. sysctl happens to be the first unimplemented system call for a dynamically linked hello world. There's a fix for the -static option, since clang hates -no-pie and needs only -fno-pie. Finally, I'm changing how I'm upstreaming a little. I'm doing a little deeper dives into our rather chaotic repo to find a couple of authors I might have missed. From here on out, I'll be using the original author's name as the git author. I'll also tag the co-authors better as well when there's multiple people that did something (other than reformat and/or move code around). I've discovered more code moved about than I'd previously known. This seems more in line with standard practice. Also, I've reviewed all these changes, but I don't know if I need to add Reviewed-by: or not, so I've done it for one or two and will make it consistent before the pull request. git log suggests maintainers are inconsistent about this (or I've not discovered the rules they follow). check-patch gives some line lenght warnings, but should otherwise be OK. There's several static functions that aren't used until the end of the patch series... Not sure the best way to suppress the build warnings there (but since they are just warnings...). Doug Rabson (1): bsd-user: Don't truncate the return value from freebsd_syscall Juergen Lock (2): bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants Kyle Evans (2): bsd-user: do_freebsd_sysctl helper for sysctl(2) bsd-user: implement sysctlbyname(2) Stacey Son (2): bsd-user: Add sysarch syscall bsd-user: Two helper routines oidfmt and sysctl_oldcvt Warner Losh (2): build: Don't specify -no-pie for --static user-mode programs bsd-user: Add -strict bsd-user/freebsd/os-sys.c | 432 ++ bsd-user/freebsd/os-syscall.c | 21 +- bsd-user/main.c | 5 +- bsd-user/qemu.h | 6 + configure | 2 +- 5 files changed, 463 insertions(+), 3 deletions(-) -- 2.39.1
[PATCH 9/9] bsd-user: Add -strict
Most of the time, it's useful to make our best effort, but sometimes we
want to know right away when we don't implement something. First place
we use it is for unknown syscalls.
Signed-off-by: Warner Losh
---
bsd-user/freebsd/os-syscall.c | 4
bsd-user/main.c | 5 -
bsd-user/qemu.h | 1 +
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 179a20c304b..e2b26ecb8dd 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -508,6 +508,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num,
abi_long arg1,
default:
qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
+if (bsd_user_strict) {
+printf("Unimplemented system call %d\n", num);
+abort();
+}
ret = -TARGET_ENOSYS;
break;
}
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 41290e16f98..ba0ad86ad28 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -91,9 +91,10 @@ unsigned long reserved_va = MAX_RESERVED_VA;
unsigned long reserved_va;
#endif
-static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
+const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
const char *qemu_uname_release;
char qemu_proc_pathname[PATH_MAX]; /* full path to exeutable */
+bool bsd_user_strict = false; /* Abort for unimplemned things */
unsigned long target_maxtsiz = TARGET_MAXTSIZ; /* max text size */
unsigned long target_dfldsiz = TARGET_DFLDSIZ; /* initial data size limit */
@@ -396,6 +397,8 @@ int main(int argc, char **argv)
trace_opt_parse(optarg);
} else if (!strcmp(r, "0")) {
argv0 = argv[optind++];
+} else if (!strcmp(r, "strict")) {
+bsd_user_strict = true;
} else {
usage();
}
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index e24a8cfcfb1..22bd5a3df42 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -113,6 +113,7 @@ typedef struct TaskState {
void stop_all_tasks(void);
extern const char *qemu_uname_release;
+extern bool bsd_user_strict;
/*
* TARGET_ARG_MAX defines the number of bytes allocated for arguments
--
2.39.1
[PULL 04/10] xen-hvm: reorganize xen-hvm and move common function to xen-hvm-common
From: Stefano Stabellini This patch does following: 1. creates arch_handle_ioreq() and arch_xen_set_memory(). This is done in preparation for moving most of xen-hvm code to an arch-neutral location, move the x86-specific portion of xen_set_memory to arch_xen_set_memory. Also, move handle_vmport_ioreq to arch_handle_ioreq. 2. Pure code movement: move common functions to hw/xen/xen-hvm-common.c Extract common functionalities from hw/i386/xen/xen-hvm.c and move them to hw/xen/xen-hvm-common.c. These common functions are useful for creating an IOREQ server. xen_hvm_init_pc() contains the architecture independent code for creating and mapping a IOREQ server, connecting memory and IO listeners, initializing a xen bus and registering backends. Moved this common xen code to a new function xen_register_ioreq() which can be used by both x86 and ARM machines. Following functions are moved to hw/xen/xen-hvm-common.c: xen_vcpu_eport(), xen_vcpu_ioreq(), xen_ram_alloc(), xen_set_memory(), xen_region_add(), xen_region_del(), xen_io_add(), xen_io_del(), xen_device_realize(), xen_device_unrealize(), cpu_get_ioreq_from_shared_memory(), cpu_get_ioreq(), do_inp(), do_outp(), rw_phys_req_item(), read_phys_req_item(), write_phys_req_item(), cpu_ioreq_pio(), cpu_ioreq_move(), cpu_ioreq_config(), handle_ioreq(), handle_buffered_iopage(), handle_buffered_io(), cpu_handle_ioreq(), xen_main_loop_prepare(), xen_hvm_change_state_handler(), xen_exit_notifier(), xen_map_ioreq_server(), destroy_hvm_domain() and xen_shutdown_fatal_error() 3. Removed static type from below functions: 1. xen_region_add() 2. xen_region_del() 3. xen_io_add() 4. xen_io_del() 5. xen_device_realize() 6. xen_device_unrealize() 7. xen_hvm_change_state_handler() 8. cpu_ioreq_pio() 9. xen_exit_notifier() 4. Replace TARGET_PAGE_SIZE with XC_PAGE_SIZE to match the page side with Xen. Signed-off-by: Vikram Garhwal Signed-off-by: Stefano Stabellini Reviewed-by: Stefano Stabellini Reviewed-by: Paul Durrant --- hw/i386/xen/trace-events| 14 - hw/i386/xen/xen-hvm.c | 1019 ++- hw/xen/meson.build |5 +- hw/xen/trace-events | 14 + hw/xen/xen-hvm-common.c | 874 ++ include/hw/i386/xen_arch_hvm.h | 11 + include/hw/xen/arch_hvm.h |3 + include/hw/xen/xen-hvm-common.h | 98 +++ 8 files changed, 1067 insertions(+), 971 deletions(-) create mode 100644 hw/xen/xen-hvm-common.c create mode 100644 include/hw/i386/xen_arch_hvm.h create mode 100644 include/hw/xen/arch_hvm.h create mode 100644 include/hw/xen/xen-hvm-common.h diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events index a0c89d91c4..5d0a8d6dcf 100644 --- a/hw/i386/xen/trace-events +++ b/hw/i386/xen/trace-events @@ -7,17 +7,3 @@ xen_platform_log(char *s) "xen platform: %s" xen_pv_mmio_read(uint64_t addr) "WARNING: read from Xen PV Device MMIO space (address 0x%"PRIx64")" xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (address 0x%"PRIx64")" -# xen-hvm.c -xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: 0x%lx, size 0x%lx" -xen_client_set_memory(uint64_t start_addr, unsigned long size, bool log_dirty) "0x%"PRIx64" size 0x%lx, log_dirty %i" -handle_ioreq(void *req, uint32_t type, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p type=%d dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -handle_ioreq_read(void *req, uint32_t type, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p read type=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -handle_ioreq_write(void *req, uint32_t type, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p write type=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -cpu_ioreq_pio(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p pio dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -cpu_ioreq_pio_read_reg(void *req, uint64_t data, uint64_t addr, uint32_t size) "I/O=%p pio read reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d" -cpu_ioreq_pio_write_reg(void *req, uint64_t data, uint64_t addr, uint32_t size) "I/O=%p pio write reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d" -cpu_ioreq_move(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p copy dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -xen_map_resource_ioreq(uint32_t id, void *addr) "id: %u addr: %p" -cpu_ioreq_config_read(void *req,
[PULL 09/10] hw/arm: introduce xenpvh machine
From: Vikram Garhwal
Add a new machine xenpvh which creates a IOREQ server to register/connect with
Xen Hypervisor.
Optional: When CONFIG_TPM is enabled, it also creates a tpm-tis-device, adds a
TPM emulator and connects to swtpm running on host machine via chardev socket
and support TPM functionalities for a guest domain.
Extra command line for aarch64 xenpvh QEMU to connect to swtpm:
-chardev socket,id=chrtpm,path=/tmp/myvtpm2/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-machine tpm-base-addr=0x0c00 \
swtpm implements a TPM software emulator(TPM 1.2 & TPM 2) built on libtpms and
provides access to TPM functionality over socket, chardev and CUSE interface.
Github repo: https://github.com/stefanberger/swtpm
Example for starting swtpm on host machine:
mkdir /tmp/vtpm2
swtpm socket --tpmstate dir=/tmp/vtpm2 \
--ctrl type=unixio,path=/tmp/vtpm2/swtpm-sock &
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Reviewed-by: Stefano Stabellini
---
docs/system/arm/xenpvh.rst| 34 +++
docs/system/target-arm.rst| 1 +
hw/arm/meson.build| 2 +
hw/arm/xen_arm.c | 182 ++
include/hw/arm/xen_arch_hvm.h | 9 ++
include/hw/xen/arch_hvm.h | 2 +
6 files changed, 230 insertions(+)
create mode 100644 docs/system/arm/xenpvh.rst
create mode 100644 hw/arm/xen_arm.c
create mode 100644 include/hw/arm/xen_arch_hvm.h
diff --git a/docs/system/arm/xenpvh.rst b/docs/system/arm/xenpvh.rst
new file mode 100644
index 00..e1655c7ab8
--- /dev/null
+++ b/docs/system/arm/xenpvh.rst
@@ -0,0 +1,34 @@
+XENPVH (``xenpvh``)
+=
+This machine creates a IOREQ server to register/connect with Xen Hypervisor.
+
+When TPM is enabled, this machine also creates a tpm-tis-device at a user input
+tpm base address, adds a TPM emulator and connects to a swtpm application
+running on host machine via chardev socket. This enables xenpvh to support TPM
+functionalities for a guest domain.
+
+More information about TPM use and installing swtpm linux application can be
+found at: docs/specs/tpm.rst.
+
+Example for starting swtpm on host machine:
+.. code-block:: console
+
+mkdir /tmp/vtpm2
+swtpm socket --tpmstate dir=/tmp/vtpm2 \
+--ctrl type=unixio,path=/tmp/vtpm2/swtpm-sock &
+
+Sample QEMU xenpvh commands for running and connecting with Xen:
+.. code-block:: console
+
+qemu-system-aarch64 -xen-domid 1 \
+-chardev socket,id=libxl-cmd,path=qmp-libxl-1,server=on,wait=off \
+-mon chardev=libxl-cmd,mode=control \
+-chardev socket,id=libxenstat-cmd,path=qmp-libxenstat-1,server=on,wait=off
\
+-mon chardev=libxenstat-cmd,mode=control \
+-xen-attach -name guest0 -vnc none -display none -nographic \
+-machine xenpvh -m 1301 \
+-chardev socket,id=chrtpm,path=tmp/vtpm2/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm -machine tpm-base-addr=0x0C00
+
+In above QEMU command, last two lines are for connecting xenpvh QEMU to swtpm
+via chardev socket.
diff --git a/docs/system/target-arm.rst b/docs/system/target-arm.rst
index 91ebc26c6d..af8d7c77d6 100644
--- a/docs/system/target-arm.rst
+++ b/docs/system/target-arm.rst
@@ -106,6 +106,7 @@ undocumented; you can get a complete list by running
arm/stm32
arm/virt
arm/xlnx-versal-virt
+ arm/xenpvh
Emulated CPU architecture support
=
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index b545ba0e4f..1b2a01a005 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -62,6 +62,8 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true:
files('fsl-imx7.c', 'mcimx7d-sabre.
arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c',
'mcimx6ul-evk.c'))
arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c'))
+arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen_arm.c'))
+arm_ss.add_all(xen_ss)
softmmu_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c'))
softmmu_ss.add(when: 'CONFIG_EXYNOS4', if_true: files('exynos4_boards.c'))
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
new file mode 100644
index 00..eaca65af37
--- /dev/null
+++ b/hw/arm/xen_arm.c
@@ -0,0 +1,182 @@
+/*
+ * QEMU ARM Xen PVH Machine
+ *
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "A
[PULL 08/10] meson.build: do not set have_xen_pci_passthrough for aarch64 targets
From: Stefano Stabellini
have_xen_pci_passthrough is only used for Xen x86 VMs.
Signed-off-by: Stefano Stabellini
Reviewed-by: Alex Bennée
---
meson.build | 2 ++
1 file changed, 2 insertions(+)
diff --git a/meson.build b/meson.build
index c626ccfa82..fb9fb97bb1 100644
--- a/meson.build
+++ b/meson.build
@@ -1471,6 +1471,8 @@ have_xen_pci_passthrough =
get_option('xen_pci_passthrough') \
error_message: 'Xen PCI passthrough requested but Xen not enabled')
\
.require(targetos == 'linux',
error_message: 'Xen PCI passthrough not available on this
platform') \
+ .require(cpu == 'x86' or cpu == 'x86_64',
+ error_message: 'Xen PCI passthrough not available on this
platform') \
.allowed()
--
2.25.1
[PULL 07/10] hw/xen/xen-hvm-common: Use g_new and error_report
From: Vikram Garhwal
Replace g_malloc with g_new and perror with error_report.
Signed-off-by: Vikram Garhwal
Reviewed-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
hw/xen/xen-hvm-common.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 5e3c7b073f..077c8dae5b 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -34,7 +34,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
MemoryRegion *mr,
trace_xen_ram_alloc(ram_addr, size);
nr_pfn = size >> TARGET_PAGE_BITS;
-pfn_list = g_malloc(sizeof (*pfn_list) * nr_pfn);
+pfn_list = g_new(xen_pfn_t, nr_pfn);
for (i = 0; i < nr_pfn; i++) {
pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i;
@@ -731,7 +731,7 @@ void destroy_hvm_domain(bool reboot)
return;
}
if (errno != ENOTTY /* old Xen */) {
-perror("xendevicemodel_shutdown failed");
+error_report("xendevicemodel_shutdown failed with error %d",
errno);
}
/* well, try the old thing then */
}
@@ -801,7 +801,7 @@ static void xen_do_ioreq_register(XenIOState *state,
}
/* Note: cpus is empty at this point in init */
-state->cpu_by_vcpu_id = g_malloc0(max_cpus * sizeof(CPUState *));
+state->cpu_by_vcpu_id = g_new0(CPUState *, max_cpus);
rc = xen_set_ioreq_server_state(xen_domid, state->ioservid, true);
if (rc < 0) {
@@ -810,7 +810,7 @@ static void xen_do_ioreq_register(XenIOState *state,
goto err;
}
-state->ioreq_local_port = g_malloc0(max_cpus * sizeof (evtchn_port_t));
+state->ioreq_local_port = g_new0(evtchn_port_t, max_cpus);
/* FIXME: how about if we overflow the page here? */
for (i = 0; i < max_cpus; i++) {
@@ -864,13 +864,13 @@ void xen_register_ioreq(XenIOState *state, unsigned int
max_cpus,
state->xce_handle = xenevtchn_open(NULL, 0);
if (state->xce_handle == NULL) {
-perror("xen: event channel open");
+error_report("xen: event channel open failed with error %d", errno);
goto err;
}
state->xenstore = xs_daemon_open();
if (state->xenstore == NULL) {
-perror("xen: xenstore open");
+error_report("xen: xenstore open failed with error %d", errno);
goto err;
}
--
2.25.1
[PULL 05/10] include/hw/xen/xen_common: return error from xen_create_ioreq_server
From: Stefano Stabellini
This is done to prepare for enabling xenpv support for ARM architecture.
On ARM it is possible to have a functioning xenpv machine with only the
PV backends and no IOREQ server. If the IOREQ server creation fails,
continue to the PV backends initialization.
Signed-off-by: Stefano Stabellini
Signed-off-by: Vikram Garhwal
Reviewed-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
include/hw/xen/xen_common.h | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 9a13a756ae..9ec69582b3 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -467,9 +467,10 @@ static inline void xen_unmap_pcidev(domid_t dom,
{
}
-static inline void xen_create_ioreq_server(domid_t dom,
- ioservid_t *ioservid)
+static inline int xen_create_ioreq_server(domid_t dom,
+ ioservid_t *ioservid)
{
+return 0;
}
static inline void xen_destroy_ioreq_server(domid_t dom,
@@ -600,8 +601,8 @@ static inline void xen_unmap_pcidev(domid_t dom,
PCI_FUNC(pci_dev->devfn));
}
-static inline void xen_create_ioreq_server(domid_t dom,
- ioservid_t *ioservid)
+static inline int xen_create_ioreq_server(domid_t dom,
+ ioservid_t *ioservid)
{
int rc = xendevicemodel_create_ioreq_server(xen_dmod, dom,
HVM_IOREQSRV_BUFIOREQ_ATOMIC,
@@ -609,12 +610,14 @@ static inline void xen_create_ioreq_server(domid_t dom,
if (rc == 0) {
trace_xen_ioreq_server_create(*ioservid);
-return;
+return rc;
}
*ioservid = 0;
use_default_ioreq_server = true;
trace_xen_default_ioreq_server();
+
+return rc;
}
static inline void xen_destroy_ioreq_server(domid_t dom,
--
2.25.1
[PULL 06/10] hw/xen/xen-hvm-common: skip ioreq creation on ioreq registration failure
From: Stefano Stabellini
On ARM it is possible to have a functioning xenpv machine with only the
PV backends and no IOREQ server. If the IOREQ server creation fails continue
to the PV backends initialization.
Also, moved the IOREQ registration and mapping subroutine to new function
xen_do_ioreq_register().
Signed-off-by: Stefano Stabellini
Signed-off-by: Vikram Garhwal
Reviewed-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
hw/xen/xen-hvm-common.c | 53 -
1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index c2e1e08124..5e3c7b073f 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -781,25 +781,12 @@ err:
exit(1);
}
-void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
-MemoryListener xen_memory_listener)
+static void xen_do_ioreq_register(XenIOState *state,
+ unsigned int max_cpus,
+ MemoryListener xen_memory_listener)
{
int i, rc;
-state->xce_handle = xenevtchn_open(NULL, 0);
-if (state->xce_handle == NULL) {
-perror("xen: event channel open");
-goto err;
-}
-
-state->xenstore = xs_daemon_open();
-if (state->xenstore == NULL) {
-perror("xen: xenstore open");
-goto err;
-}
-
-xen_create_ioreq_server(xen_domid, &state->ioservid);
-
state->exit.notify = xen_exit_notifier;
qemu_add_exit_notifier(&state->exit);
@@ -863,12 +850,44 @@ void xen_register_ioreq(XenIOState *state, unsigned int
max_cpus,
QLIST_INIT(&state->dev_list);
device_listener_register(&state->device_listener);
+return;
+
+err:
+error_report("xen hardware virtual machine initialisation failed");
+exit(1);
+}
+
+void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
+MemoryListener xen_memory_listener)
+{
+int rc;
+
+state->xce_handle = xenevtchn_open(NULL, 0);
+if (state->xce_handle == NULL) {
+perror("xen: event channel open");
+goto err;
+}
+
+state->xenstore = xs_daemon_open();
+if (state->xenstore == NULL) {
+perror("xen: xenstore open");
+goto err;
+}
+
+rc = xen_create_ioreq_server(xen_domid, &state->ioservid);
+if (!rc) {
+xen_do_ioreq_register(state, max_cpus, xen_memory_listener);
+} else {
+warn_report("xen: failed to create ioreq server");
+}
+
xen_bus_init();
xen_register_backend(state);
return;
+
err:
-error_report("xen hardware virtual machine initialisation failed");
+error_report("xen hardware virtual machine backend registration failed");
exit(1);
}
--
2.25.1
[PULL 01/10] hw/i386/xen/: move xen-mapcache.c to hw/xen/
From: Vikram Garhwal
xen-mapcache.c contains common functions which can be used for enabling Xen on
aarch64 with IOREQ handling. Moving it out from hw/i386/xen to hw/xen to make it
accessible for both aarch64 and x86.
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
hw/i386/meson.build | 1 +
hw/i386/xen/meson.build | 1 -
hw/i386/xen/trace-events | 5 -
hw/xen/meson.build | 4
hw/xen/trace-events | 5 +
hw/{i386 => }/xen/xen-mapcache.c | 0
6 files changed, 10 insertions(+), 6 deletions(-)
rename hw/{i386 => }/xen/xen-mapcache.c (100%)
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 213e2e82b3..cfdbfdcbcb 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -33,5 +33,6 @@ subdir('kvm')
subdir('xen')
i386_ss.add_all(xenpv_ss)
+i386_ss.add_all(xen_ss)
hw_arch += {'i386': i386_ss}
diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index be84130300..2fcc46e6ca 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -1,6 +1,5 @@
i386_ss.add(when: 'CONFIG_XEN', if_true: files(
'xen-hvm.c',
- 'xen-mapcache.c',
'xen_apic.c',
'xen_platform.c',
'xen_pvdevice.c',
diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events
index 5d6be61090..a0c89d91c4 100644
--- a/hw/i386/xen/trace-events
+++ b/hw/i386/xen/trace-events
@@ -21,8 +21,3 @@ xen_map_resource_ioreq(uint32_t id, void *addr) "id: %u addr:
%p"
cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t size,
uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t size,
uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x"
-# xen-mapcache.c
-xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64
-xen_remap_bucket(uint64_t index) "index 0x%"PRIx64
-xen_map_cache_return(void* ptr) "%p"
-
diff --git a/hw/xen/meson.build b/hw/xen/meson.build
index ae0ace3046..19d0637c46 100644
--- a/hw/xen/meson.build
+++ b/hw/xen/meson.build
@@ -22,3 +22,7 @@ else
endif
specific_ss.add_all(when: ['CONFIG_XEN', xen], if_true: xen_specific_ss)
+
+xen_ss = ss.source_set()
+
+xen_ss.add(when: 'CONFIG_XEN', if_true: files('xen-mapcache.c'))
diff --git a/hw/xen/trace-events b/hw/xen/trace-events
index 3da3fd8348..2c8f238f42 100644
--- a/hw/xen/trace-events
+++ b/hw/xen/trace-events
@@ -41,3 +41,8 @@ xs_node_vprintf(char *path, char *value) "%s %s"
xs_node_vscanf(char *path, char *value) "%s %s"
xs_node_watch(char *path) "%s"
xs_node_unwatch(char *path) "%s"
+
+# xen-mapcache.c
+xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64
+xen_remap_bucket(uint64_t index) "index 0x%"PRIx64
+xen_map_cache_return(void* ptr) "%p"
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
similarity index 100%
rename from hw/i386/xen/xen-mapcache.c
rename to hw/xen/xen-mapcache.c
--
2.25.1
[PULL 03/10] hw/i386/xen/xen-hvm: move x86-specific fields out of XenIOState
From: Stefano Stabellini
In preparation to moving most of xen-hvm code to an arch-neutral location, move:
- shared_vmport_page
- log_for_dirtybit
- dirty_bitmap
- suspend
- wakeup
out of XenIOState struct as these are only used on x86, especially the ones
related to dirty logging.
Updated XenIOState can be used for both aarch64 and x86.
Also, remove free_phys_offset as it was unused.
Signed-off-by: Stefano Stabellini
Signed-off-by: Vikram Garhwal
Reviewed-by: Paul Durrant
Reviewed-by: Alex Bennée
---
hw/i386/xen/xen-hvm.c | 58 ---
1 file changed, 27 insertions(+), 31 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 1fba0e0ae1..06c446e7be 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -73,6 +73,7 @@ struct shared_vmport_iopage {
};
typedef struct shared_vmport_iopage shared_vmport_iopage_t;
#endif
+static shared_vmport_iopage_t *shared_vmport_page;
static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
{
@@ -95,6 +96,11 @@ typedef struct XenPhysmap {
} XenPhysmap;
static QLIST_HEAD(, XenPhysmap) xen_physmap;
+static const XenPhysmap *log_for_dirtybit;
+/* Buffer used by xen_sync_dirty_bitmap */
+static unsigned long *dirty_bitmap;
+static Notifier suspend;
+static Notifier wakeup;
typedef struct XenPciDevice {
PCIDevice *pci_dev;
@@ -105,7 +111,6 @@ typedef struct XenPciDevice {
typedef struct XenIOState {
ioservid_t ioservid;
shared_iopage_t *shared_page;
-shared_vmport_iopage_t *shared_vmport_page;
buffered_iopage_t *buffered_io_page;
xenforeignmemory_resource_handle *fres;
QEMUTimer *buffered_io_timer;
@@ -125,14 +130,8 @@ typedef struct XenIOState {
MemoryListener io_listener;
QLIST_HEAD(, XenPciDevice) dev_list;
DeviceListener device_listener;
-hwaddr free_phys_offset;
-const XenPhysmap *log_for_dirtybit;
-/* Buffer used by xen_sync_dirty_bitmap */
-unsigned long *dirty_bitmap;
Notifier exit;
-Notifier suspend;
-Notifier wakeup;
} XenIOState;
/* Xen specific function for piix pci */
@@ -462,10 +461,10 @@ static int xen_remove_from_physmap(XenIOState *state,
}
QLIST_REMOVE(physmap, list);
-if (state->log_for_dirtybit == physmap) {
-state->log_for_dirtybit = NULL;
-g_free(state->dirty_bitmap);
-state->dirty_bitmap = NULL;
+if (log_for_dirtybit == physmap) {
+log_for_dirtybit = NULL;
+g_free(dirty_bitmap);
+dirty_bitmap = NULL;
}
g_free(physmap);
@@ -626,16 +625,16 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
return;
}
-if (state->log_for_dirtybit == NULL) {
-state->log_for_dirtybit = physmap;
-state->dirty_bitmap = g_new(unsigned long, bitmap_size);
-} else if (state->log_for_dirtybit != physmap) {
+if (log_for_dirtybit == NULL) {
+log_for_dirtybit = physmap;
+dirty_bitmap = g_new(unsigned long, bitmap_size);
+} else if (log_for_dirtybit != physmap) {
/* Only one range for dirty bitmap can be tracked. */
return;
}
rc = xen_track_dirty_vram(xen_domid, start_addr >> TARGET_PAGE_BITS,
- npages, state->dirty_bitmap);
+ npages, dirty_bitmap);
if (rc < 0) {
#ifndef ENODATA
#define ENODATA ENOENT
@@ -650,7 +649,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
}
for (i = 0; i < bitmap_size; i++) {
-unsigned long map = state->dirty_bitmap[i];
+unsigned long map = dirty_bitmap[i];
while (map != 0) {
j = ctzl(map);
map &= ~(1ul << j);
@@ -676,12 +675,10 @@ static void xen_log_start(MemoryListener *listener,
static void xen_log_stop(MemoryListener *listener, MemoryRegionSection
*section,
int old, int new)
{
-XenIOState *state = container_of(listener, XenIOState, memory_listener);
-
if (old & ~new & (1 << DIRTY_MEMORY_VGA)) {
-state->log_for_dirtybit = NULL;
-g_free(state->dirty_bitmap);
-state->dirty_bitmap = NULL;
+log_for_dirtybit = NULL;
+g_free(dirty_bitmap);
+dirty_bitmap = NULL;
/* Disable dirty bit tracking */
xen_track_dirty_vram(xen_domid, 0, 0, NULL);
}
@@ -1021,9 +1018,9 @@ static void handle_vmport_ioreq(XenIOState *state,
ioreq_t *req)
{
vmware_regs_t *vmport_regs;
-assert(state->shared_vmport_page);
+assert(shared_vmport_page);
vmport_regs =
-&state->shared_vmport_page->vcpu_vmport_regs[state->send_vcpu];
+&shared_vmport_page->vcpu_vmport_regs[state->send_vcpu];
QEMU_BUILD_BUG_ON(sizeof(*req) < sizeof(*vmport_regs));
current_cpu = state->cpu_by_vcpu_id[state->send_vcpu];
@@ -1468,7 +1465,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
state->memory_listener = xen_memory_listener;
[PULL 02/10] hw/i386/xen: rearrange xen_hvm_init_pc
From: Vikram Garhwal
In preparation to moving most of xen-hvm code to an arch-neutral location,
move non IOREQ references to:
- xen_get_vmport_regs_pfn
- xen_suspend_notifier
- xen_wakeup_notifier
- xen_ram_init
towards the end of the xen_hvm_init_pc() function.
This is done to keep the common ioreq functions in one place which will be
moved to new function in next patch in order to make it common to both x86 and
aarch64 machines.
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Reviewed-by: Paul Durrant
---
hw/i386/xen/xen-hvm.c | 49 ++-
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index b9a6f7f538..1fba0e0ae1 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1416,12 +1416,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
state->exit.notify = xen_exit_notifier;
qemu_add_exit_notifier(&state->exit);
-state->suspend.notify = xen_suspend_notifier;
-qemu_register_suspend_notifier(&state->suspend);
-
-state->wakeup.notify = xen_wakeup_notifier;
-qemu_register_wakeup_notifier(&state->wakeup);
-
/*
* Register wake-up support in QMP query-current-machine API
*/
@@ -1432,23 +1426,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
goto err;
}
-rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn);
-if (!rc) {
-DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
-state->shared_vmport_page =
-xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE,
- 1, &ioreq_pfn, NULL);
-if (state->shared_vmport_page == NULL) {
-error_report("map shared vmport IO page returned error %d
handle=%p",
- errno, xen_xc);
-goto err;
-}
-} else if (rc != -ENOSYS) {
-error_report("get vmport regs pfn returned error %d, rc=%d",
- errno, rc);
-goto err;
-}
-
/* Note: cpus is empty at this point in init */
state->cpu_by_vcpu_id = g_new0(CPUState *, max_cpus);
@@ -1486,7 +1463,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
#else
xen_map_cache_init(NULL, state);
#endif
-xen_ram_init(pcms, ms->ram_size, ram_memory);
qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state);
@@ -1513,6 +1489,31 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion
**ram_memory)
QLIST_INIT(&xen_physmap);
xen_read_physmap(state);
+state->suspend.notify = xen_suspend_notifier;
+qemu_register_suspend_notifier(&state->suspend);
+
+state->wakeup.notify = xen_wakeup_notifier;
+qemu_register_wakeup_notifier(&state->wakeup);
+
+rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn);
+if (!rc) {
+DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
+state->shared_vmport_page =
+xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE,
+ 1, &ioreq_pfn, NULL);
+if (state->shared_vmport_page == NULL) {
+error_report("map shared vmport IO page returned error %d
handle=%p",
+ errno, xen_xc);
+goto err;
+}
+} else if (rc != -ENOSYS) {
+error_report("get vmport regs pfn returned error %d, rc=%d",
+ errno, rc);
+goto err;
+}
+
+xen_ram_init(pcms, ms->ram_size, ram_memory);
+
/* Disable ACPI build because Xen handles it */
pcms->acpi_build_enabled = false;
--
2.25.1
[PULL 10/10] meson.build: enable xenpv machine build for ARM
From: Vikram Garhwal
Add CONFIG_XEN for aarch64 device to support build for ARM targets.
Signed-off-by: Vikram Garhwal
Signed-off-by: Stefano Stabellini
Reviewed-by: Alex Bennée
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index fb9fb97bb1..d0710a8f5f 100644
--- a/meson.build
+++ b/meson.build
@@ -135,7 +135,7 @@ endif
if cpu in ['x86', 'x86_64', 'arm', 'aarch64']
# i386 emulator provides xenpv machine type for multiple architectures
accelerator_targets += {
-'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu'],
+'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu'],
}
endif
if cpu in ['x86', 'x86_64']
--
2.25.1
[PULL 00/10] xenpvh machine
The following changes since commit 90595cc9396bb910b148391fea2e78dd8c6c8b27:
Merge tag 'migration-20230209-pull-request' of
https://gitlab.com/juan.quintela/qemu into staging (2023-02-10 10:50:21 +)
are available in the Git repository at:
https://gitlab.com/sstabellini/qemu.git xenpvh
for you to fetch changes up to 3f8ee848693872e3783cdcf2862be5421bb9cbcb:
meson.build: enable xenpv machine build for ARM (2023-02-10 14:23:47 -0800)
Stefano Stabellini (5):
hw/i386/xen/xen-hvm: move x86-specific fields out of XenIOState
xen-hvm: reorganize xen-hvm and move common function to xen-hvm-common
include/hw/xen/xen_common: return error from xen_create_ioreq_server
hw/xen/xen-hvm-common: skip ioreq creation on ioreq registration failure
meson.build: do not set have_xen_pci_passthrough for aarch64 targets
Vikram Garhwal (5):
hw/i386/xen/: move xen-mapcache.c to hw/xen/
hw/i386/xen: rearrange xen_hvm_init_pc
hw/xen/xen-hvm-common: Use g_new and error_report
hw/arm: introduce xenpvh machine
meson.build: enable xenpv machine build for ARM
docs/system/arm/xenpvh.rst | 34 ++
docs/system/target-arm.rst |1 +
hw/arm/meson.build |2 +
hw/arm/xen_arm.c | 182 +++
hw/i386/meson.build |1 +
hw/i386/xen/meson.build |1 -
hw/i386/xen/trace-events | 19 -
hw/i386/xen/xen-hvm.c| 1078 --
hw/xen/meson.build |7 +
hw/xen/trace-events | 19 +
hw/xen/xen-hvm-common.c | 893 +++
hw/{i386 => }/xen/xen-mapcache.c |0
include/hw/arm/xen_arch_hvm.h|9 +
include/hw/i386/xen_arch_hvm.h | 11 +
include/hw/xen/arch_hvm.h|5 +
include/hw/xen/xen-hvm-common.h | 98
include/hw/xen/xen_common.h | 13 +-
meson.build |4 +-
18 files changed, 1364 insertions(+), 1013 deletions(-)
create mode 100644 docs/system/arm/xenpvh.rst
create mode 100644 hw/arm/xen_arm.c
create mode 100644 hw/xen/xen-hvm-common.c
rename hw/{i386 => }/xen/xen-mapcache.c (100%)
create mode 100644 include/hw/arm/xen_arch_hvm.h
create mode 100644 include/hw/i386/xen_arch_hvm.h
create mode 100644 include/hw/xen/arch_hvm.h
create mode 100644 include/hw/xen/xen-hvm-common.h
[PATCH v4] hw/arm/smmuv3: Add GBPA register
GBPA register can be used to globally abort all
transactions.
It is described in the SMMU manual in "6.3.14 SMMU_GBPA".
ABORT reset value is IMPLEMENTATION DEFINED, it is chosen to
be zero(Do not abort incoming transactions).
Other fields have default values of Use Incoming.
If UPDATE is not set, the write is ignored. This is the only permitted
behavior in SMMUv3.2 and later.(6.3.14.1 Update procedure)
As this patch adds a new state to the SMMU (GBPA), it is added
in a new subsection for forward migration compatibility.
GBPA is only migrated if its value is different from the reset value.
It does this to be backward migration compatible if SW didn't write
the register.
Signed-off-by: Mostafa Saleh
---
Changes in v4:
- Migrate if GBPA is different from reset value, not only ABORT bit.
Changes in v3:
- Remove migrate_gbpa property as it was unnecessary.
Changes in v2:
- GBPA is effective only when SMMU is not enabled.
- Ignore GBPA write when UPDATE is not set.
- Default value for SHCFG is "Use Incoming".
- Support migration.
---
hw/arm/smmuv3-internal.h | 7 ++
hw/arm/smmuv3.c | 47 +++-
include/hw/arm/smmuv3.h | 1 +
3 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index bce161870f..e8f0ebf25e 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -79,6 +79,13 @@ REG32(CR0ACK, 0x24)
REG32(CR1, 0x28)
REG32(CR2, 0x2c)
REG32(STATUSR, 0x40)
+REG32(GBPA,0x44)
+FIELD(GBPA, ABORT,20, 1)
+FIELD(GBPA, UPDATE, 31, 1)
+
+/* Use incoming. */
+#define SMMU_GBPA_RESET_VAL 0x1000
+
REG32(IRQ_CTRL,0x50)
FIELD(IRQ_CTRL, GERROR_IRQEN,0, 1)
FIELD(IRQ_CTRL, PRI_IRQEN, 1, 1)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 955b89c8d5..ddd37f233b 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -285,6 +285,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
s->gerror = 0;
s->gerrorn = 0;
s->statusr = 0;
+s->gbpa = SMMU_GBPA_RESET_VAL;
}
static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf,
@@ -659,7 +660,11 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
*mr, hwaddr addr,
qemu_mutex_lock(&s->mutex);
if (!smmu_enabled(s)) {
-status = SMMU_TRANS_DISABLE;
+if (FIELD_EX32(s->gbpa, GBPA, ABORT)) {
+status = SMMU_TRANS_ABORT;
+} else {
+status = SMMU_TRANS_DISABLE;
+}
goto epilogue;
}
@@ -1170,6 +1175,16 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
offset,
case A_GERROR_IRQ_CFG2:
s->gerror_irq_cfg2 = data;
return MEMTX_OK;
+case A_GBPA:
+/*
+ * If UPDATE is not set, the write is ignored. This is the only
+ * permitted behavior in SMMUv3.2 and later.
+ */
+if (data & R_GBPA_UPDATE_MASK) {
+/* Ignore update bit as write is synchronous. */
+s->gbpa = data & ~R_GBPA_UPDATE_MASK;
+}
+return MEMTX_OK;
case A_STRTAB_BASE: /* 64b */
s->strtab_base = deposit64(s->strtab_base, 0, 32, data);
return MEMTX_OK;
@@ -1318,6 +1333,9 @@ static MemTxResult smmu_readl(SMMUv3State *s, hwaddr
offset,
case A_STATUSR:
*data = s->statusr;
return MEMTX_OK;
+case A_GBPA:
+*data = s->gbpa;
+return MEMTX_OK;
case A_IRQ_CTRL:
case A_IRQ_CTRL_ACK:
*data = s->irq_ctrl;
@@ -1482,6 +1500,29 @@ static const VMStateDescription vmstate_smmuv3_queue = {
},
};
+static bool smmuv3_gbpa_needed(void *opaque)
+{
+SMMUv3State *s = opaque;
+
+/* Only migrate GBPA if it has different reset value. */
+if (s->gbpa != SMMU_GBPA_RESET_VAL) {
+return true;
+}
+
+return false;
+}
+
+static const VMStateDescription vmstate_gbpa = {
+.name = "smmuv3/gbpa",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = smmuv3_gbpa_needed,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(gbpa, SMMUv3State),
+VMSTATE_END_OF_LIST()
+}
+};
+
static const VMStateDescription vmstate_smmuv3 = {
.name = "smmuv3",
.version_id = 1,
@@ -1512,6 +1553,10 @@ static const VMStateDescription vmstate_smmuv3 = {
VMSTATE_END_OF_LIST(),
},
+.subsections = (const VMStateDescription * []) {
+&vmstate_gbpa,
+NULL
+}
};
static void smmuv3_instance_init(Object *obj)
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index f1921fdf9e..9899fa1860 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -46,6 +46,7 @@ struct SMMUv3State {
uint32_t cr[3];
uint32_t cr0ack;
uint32_t statusr;
+uint32_t gbpa;
uint32_t irq_ctrl;
uint32_t gerror;
uint32_t gerrorn;
--
2.39.1.581.gbfd45094c4-goog
Re: [PATCH v4 4/4] ram: Document migration ram flags
On Fri, Feb 10, 2023 at 12:37:30AM +0100, Juan Quintela wrote: > 0x80 is RAM_SAVE_FLAG_HOOK, it is in qemu-file now. > Bigger usable flag is 0x200, noticing that. > We can reuse RAM_SAVe_FLAG_FULL. SAVE > > Signed-off-by: Juan Quintela > --- > migration/ram.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/migration/ram.c b/migration/ram.c > index 32fab7b5ee..3648cfc357 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -67,22 +67,26 @@ > /***/ > /* ram save/restore */ > > -/* RAM_SAVE_FLAG_ZERO used to be named RAM_SAVE_FLAG_COMPRESS, it > +/* > + * RAM_SAVE_FLAG_ZERO used to be named RAM_SAVE_FLAG_COMPRESS, it > * worked for pages that where filled with the same char. We switched As long as you're in the area, s/where/were/ > * it to only search for the zero value. And to avoid confusion with > * RAM_SSAVE_FLAG_COMPRESS_PAGE just rename it. s/SSAVE/SAVE/ > */ > - > -#define RAM_SAVE_FLAG_FULL 0x01 /* Obsolete, not used anymore */ > +/* > + * RAM_SAVE_FLAG_FULL was obsoleted in 2009, it can be reused now > + */ > +#define RAM_SAVE_FLAG_FULL 0x01 > #define RAM_SAVE_FLAG_ZERO 0x02 > #define RAM_SAVE_FLAG_MEM_SIZE 0x04 > #define RAM_SAVE_FLAG_PAGE 0x08 > #define RAM_SAVE_FLAG_EOS 0x10 > #define RAM_SAVE_FLAG_CONTINUE 0x20 > #define RAM_SAVE_FLAG_XBZRLE 0x40 > -/* 0x80 is reserved in migration.h start with 0x100 next */ > +/* 0x80 is reserved in qemu-file.h for RAM_SAVE_FLAG_HOOK */ > #define RAM_SAVE_FLAG_COMPRESS_PAGE0x100 > #define RAM_SAVE_FLAG_MULTIFD_SYNC 0x200 > +/* We can't use any flag that is bigger than 0x200 */ Spelling fixes are trivial; feel free to add: Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 6/7] CI: Stop building docs on centos8
Il ven 10 feb 2023, 19:09 Peter Maydell ha scritto: > On Fri, 10 Feb 2023 at 17:55, John Snow wrote: > > (The problem with just allowing sphinx to be a black box and > > continuing to happily use the 3.6-based versions is that we are > > using QAPIDoc extensions from our own codebase, which would lock > > those to 3.6. A big motivator for Markus is dropping some 3.6 > > kludges we're carrying for qapi, so I looked to the opposite > > solution - nudging Sphinx forward instead.) > > Mmm. Where on the pain spectrum does "allow python 3.8 > because CentosOS ships that, except that where our python > code gets run via Sphinx that part of the codebase must > retain 3.6 compatibility" sit ? > As far as the build system is concerned no changes are required, however Python 3.7 support was requested by Markus for scripts/qapi/ for quite some time. So it would not be painful to implement but it would remove most of the benefits of dropping support for 3.6. Paolo > thanks > -- PMM > >
Re: [PATCH v2 3/7] configure: Look for auxiliary Python installations
On Fri, Feb 10, 2023 at 10:28:42AM -0500, John Snow wrote:
> > > python=
> > > +first_python=
> > > explicit_python=no
> > > -for binary in "${PYTHON-python3}" python
> > > +# A bare 'python' is traditionally python 2.x, but some distros
> > > +# have it as python 3.x, so check in both places.
> > > +for binary in "${PYTHON-python3}" python python3.{11..6}
> >
> > This is not available in e.g. dash, so we need to use {11,10,9,8,7,6}.
> > Just a nit, I can fix it myself.
Shoot - I didn't notice v2 before I reviewed v1, where I pointed out
the same problem. But note that dash doesn't understand ANY brace
expansion; {11,10,9} is no better than {11..9}.
The list is also not testing python3 when $PYTHON is set. See my
suggestion for fixing that in my mail on v1.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PATCH 12/22] target/arm: NSTable is RES0 for the RME EL3 regime
On 2/10/23 01:36, Peter Maydell wrote:
On Tue, 24 Jan 2023 at 00:01, Richard Henderson
wrote:
Test in_space instead of in_secure so that we don't switch
out of Root space. Handle the output space change immediately,
rather than try and combine the NSTable and NS bits later.
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 31 ++-
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index c1b0b8e610..ddafb1f329 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1240,7 +1240,6 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
{
ARMCPU *cpu = env_archcpu(env);
ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
-bool is_secure = ptw->in_secure;
int32_t level;
ARMVAParameters param;
uint64_t ttbr;
@@ -1256,7 +1255,6 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
uint64_t descaddrmask;
bool aarch64 = arm_el_is_aa64(env, el);
uint64_t descriptor, new_descriptor;
-bool nstable;
/* TODO: This code does not support shareability levels. */
if (aarch64) {
@@ -1417,29 +1415,29 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
descaddrmask = MAKE_64BIT_MASK(0, 40);
}
descaddrmask &= ~indexmask_grainsize;
-
-/*
- * Secure accesses start with the page table in secure memory and
- * can be downgraded to non-secure at any step. Non-secure accesses
- * remain non-secure. We implement this by just ORing in the NSTable/NS
- * bits at each step.
- */
-tableattrs = is_secure ? 0 : (1 << 4);
+tableattrs = 0;
next_level:
descaddr |= (address >> (stride * (4 - level))) & indexmask;
descaddr &= ~7ULL;
-nstable = extract32(tableattrs, 4, 1);
-if (nstable && ptw->in_secure) {
-/*
- * Stage2_S -> Stage2 or Phys_S -> Phys_NS
- * Assert that the non-secure idx are even, and relative order.
- */
+
+/*
+ * Process the NSTable bit from the previous level. This changes
+ * the table address space and the output space from Secure to
+ * NonSecure. With RME, the EL3 translation regime does not change
+ * from Root to NonSecure.
+ */
To check my understanding, this means that the bit that the spec
describes as FEAT_RME changing the behaviour of NSTable in the EL3
stage 1 translation regime is implemented by us by having the
in_space for EL3 be different for FEAT_RME and not-FEAT_RME ?
Correct -- space is Secure for non-RME EL3, and Root for RME EL3.
attrs = new_descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50,
14));
if (!regime_is_stage2(mmu_idx)) {
-attrs |= nstable << 5; /* NS */
This removes the code where we copy the NSTable bit across to attrs,
but there's still code below here that assumes it can get the combined
NS bit from bit 5 of attrs, isn't there? (It passes it to get_S1prot().)
Oops. This gets fixed in patch 14. Some reordering needed...
r~
Re: [PATCH 3/7] configure: Look for auxiliary Python installations
On Thu, Feb 09, 2023 at 10:40:30AM -0500, John Snow wrote:
> At the moment, we look for just "python3" and "python", which is good
> enough almost all of the time. But ... if you are on a platform that
> uses an older Python by default and only offers a newer Python as an
> option, you'll have to specify --python=/usr/bin/foo every time.
>
> We can be kind and instead make a cursory attempt to locate a suitable
> Python binary ourselves, looking for the remaining well-known binaries.
>
> This configure loop will prefer, in order:
>
> 1. Whatever is specified in $PYTHON
> 2. python3
> 3. python
> 4. python3.11 down through python3.6
Makes sense.
> python=
> +first_python=
> explicit_python=no
> -for binary in "${PYTHON-python3}" python
> +# A bare 'python' is traditionally python 2.x, but some distros
> +# have it as python 3.x, so check in both places.
> +for binary in "${PYTHON-python3}" python python3.{11..6}
This does not match your commit message. If $PYTHON is set but fails,
you never check python3. Pre-existing, but now that you're calling it
out as intended, it may be better to write the list prefix as:
for binary in $PYTHON python3 python ...
except that it mishandles $PYTHON containing space, so you want the
quotes, but you don't want to test an empty binary or waste time
testing python3 twice, so more precise could be:
for binary in "${PYTHON-python3}" ${PYTHON:+python3} python ...
Meanwhioe, your use of {11.6} is a bashism, but configure is /bin/sh.
It would be nice if you could use $(seq -f python3.%g 11 -1 6), but
that's probably too specific to GNU Coreutils and won't work on other
platforms; and open-coding it in a shell loop isn't going to be any
prettier. So you'll be safest if you just manually spell it out:
python3.11 python3.10 ...
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 4/7] configure: Add nice hint to Python failure message
On Fri, Feb 10, 2023, 2:45 AM Thomas Huth wrote: > On 10/02/2023 01.31, John Snow wrote: > > If we begin requiring Python 3.7+, a few platforms are going to need to > > install an additional package. > > > > This is at least mildly annoying to the user (and I hate negative > > attention), so solve the user's problem for them before they get a > > chance to become irritated while searching on Google for how to install > > newer Python packages. > > > > Signed-off-by: John Snow > > --- > > configure | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/configure b/configure > > index ea8c973d13b..bf512273f44 100755 > > --- a/configure > > +++ b/configure > > @@ -1058,7 +1058,10 @@ fi > > > > if ! check_py_version "$python"; then > > error_exit "Cannot use '$python', Python >= 3.6 is required." \ > > - "Use --python=/path/to/python to specify a supported Python." > > + "Use --python=/path/to/python to specify a supported > Python." \ > > + "Maybe try:" \ > > + " openSUSE Leap 15.3+: zypper install python39" \ > > + " CentOS 8: dnf install python38" > > IMHO the "Python > 3.6" is already pretty clear, and the hints that you > provide here will expire pretty fast (unless you bump them regularly), so > I'd rather drop this patch. Just my 0.02 €. > > Thomas > I figured that when they expired that they also just wouldn't... get printed anymore. Just trying my best to minimize disruption as a courtesy, and as a demonstration of how gentle the deprecation could be. I get it though, it *will* rot ... It's the kind of thing that I'd want to have in for maybe a release or two before just removing it, once everyone has a chance to catch on and learn the simple remedy. (I don't want anyone with 3.6 on their system to be unaware of how to mitigate this issue, and quickly.) I could replace it with a more generic hint, too; like "Try looking for python3y or python3.y packages in your distro's software repository" that would rot at a slower pace. --js >
Re: [PATCH v2 6/7] CI: Stop building docs on centos8
On 2/10/23 18:15, Peter Maydell wrote: Right. All of these things together seem to say: * Python is not an unreasonable thing for the project to depend on * CentOS 8 is not an unreasonable distro for us to want to continue to support * Therefore we should continue to work with the Python that ships with CentOS 8... [snip] We don't have to drop python 3.6. It is a choice because of a desire to be able to use some shiny new python features without caring about back compat. Similarly we don't have to use the new meson which drops support for python 3.6, it is again a choice because we want to rely on some new meson features. QEMU could easily carry on supporting python 3.6 until the affected distros drop out ofo our support matrix, but we would have to opt out of using certain new features, or put more effort into back compat. Personally I'm still on the side of carrying on supporting 3.6 per our distro support policy, but if broad consensus is to drop 3.6 I'm not going push back anymore. This is really where I'm at as well -- we set our distro support policy, and we know that means that sometimes we have to deal with continuing to support older versions of dependencies even when it might be easier for us if we could require newer versions. There are four possibilities: * we change the support policy and stop supporting CentOS 8 and SLE 15, not a good idea since a lot of people have not migrated to CentOS 9 yet. * we keep supporting Python 3.6 until CentOS 8 and SLE 15 stop being supported. This means several more years since SLE 16 hasn't even been released. * we support Python 3.6 just for building documentation, i.e. we are careful not to use Python 3.7+ features in our Sphinx extensions but are free to use them elsewhere. CentOS 8 users can install sphinx from either RPMs (which will use Python 3.6) or pip (which will use Python 3.8). * we only support Python 3.7+, which means CentOS 8 CI and users have to either install Sphinx from pip or disable documentation. The only difference between the last two is documentation and CI configuration. The code is exactly the same. I'm reluctant to say that Python gets a special derogation from that policy. Note that its not the Python runtime but the Python dependencies, for which we already install avocado and some Python development tools in a virtual environment. So, the questions are: * to what extent can we rely on pip packages (preinstalled by the user) instead of the distro packages? * to what extent should QEMU install its own dependencies via pip in a virtual environment instead of requiring the user to preinstall them? Right now the answers for both are "avocado gets an exception for tests/, Python development tools such as mypy get an exception for python/". The Python 3.7+ series (not this one by John) currently adds "sphinx gets an exception to the first answer only". In the future I would like to unify virtual environment generation for tests/ and python/ and move it to configure. If desirable, this would make it possible to implement something like "the user need not care about Python dependencies, configure can (but need not) install them all via pip". Distros would still package the dependencies, but users would have a slightly easier time building QEMU. Thanks, Paolo
Re: [PATCH v2 6/7] CI: Stop building docs on centos8
On Fri, 10 Feb 2023 at 17:55, John Snow wrote: > (The problem with just allowing sphinx to be a black box and > continuing to happily use the 3.6-based versions is that we are > using QAPIDoc extensions from our own codebase, which would lock > those to 3.6. A big motivator for Markus is dropping some 3.6 > kludges we're carrying for qapi, so I looked to the opposite > solution - nudging Sphinx forward instead.) Mmm. Where on the pain spectrum does "allow python 3.8 because CentosOS ships that, except that where our python code gets run via Sphinx that part of the codebase must retain 3.6 compatibility" sit ? thanks -- PMM
Re: [PATCH v3] hw/arm/smmuv3: Add GBPA register
On Fri, Feb 10, 2023 at 04:11:37PM +, Peter Maydell wrote: > > I think we should check the whole register against its reset value, > not just the ABORT bit. Otherwise if the guest writes the other fields > to non default values we won't migrate them. That doesn't change the > device behaviour now but it will have the weird effect that the guest > could write the register and then after a migration find it reading > back as a different value. Plus if in future we implement actual > functionality for any of the other fields then we'll want to know > what their true values written by the guest are. > > Linux never changes any fields except ABORT so for the most interesting > guest it won't make a difference right now. I agree, I was trying to achieve maximum backward compatibility as we don’t care about other fields, but this can be weird for forward migration, if the register is not fully migrated. And if SW touches this register it would probably want to configure the ABORT at some point, so it won’t help much for backward migration. I will update this in v4. Thanks, Mostafa
Re: [PATCH v2 6/7] CI: Stop building docs on centos8
On Fri, Feb 10, 2023, 11:32 AM Peter Maydell wrote: > On Fri, 10 Feb 2023 at 16:01, John Snow wrote: > > On Fri, Feb 10, 2023, 5:41 AM Peter Maydell > wrote: > >> On Fri, 10 Feb 2023 at 00:31, John Snow wrote: > >> This confuses me. We work fine with Python 3.6 today. > > > > > > That won't last - Many tools such as mypy, pylint and flake8 which I use > to manage our python codebase have been dropping support for 3.6 and I've > had to implement an increasing number of workarounds to help keep it > possible to test 3.6 via CI while also ensuring our newest platforms work > as dev environments. > > Something somewhere seems kind of out-of-sync here. Either: > * Python is deprecating old versions too quickly and >dropping support for them too fast > I think this is true. The Python upstream ecosystem moves extremely fast and breaks backwards compatibility for tooling often. It is sometimes tough to mediate that flow rate difference. Part of the reason I added that "check-tox" CI test that is currently optional to run is specifically so it can find problems with cutting edge tooling - and it happens often. (It seems like every freeze, I find out a new upstream version of something breaks the testing, like clockwork.) On the other hand, Python 3.6 did launch some six years ago - 2016-12-22. It's probably reasonable most cutting edge tooling for f37/f38 no longer supports developing for a version this old. I can't deny that the Python package ecosystem is too cavalier about dropping for support for things too quickly, though. * CentOS is retaining old versions of Python when it needs to >ship newer ones > I'm not clear on the exact policies for Stream 8/9, but in our traditional releases we'd not ever upgrade this - we'd only do micro updates and backports. Instead, Stream 8/9 (and OpenSUSE too) offer optional Python versions that install side-by-side. So sphinx (as packaged as an rpm) continues to use the platform Python - 3.6 - but you *can* launch the "system sphinx" with your newer Python interpreter - if the version of sphinx shipped happens to cope with a newer interpreter. (If you run `python3.7 -m sphinx`, for example - it might work, but only if your rpm-packaged sphinx isn't using any features of Python that were dropped in 3.7. I'd wager it probably does work most of the time.) * Our policy for what distros we support is overly lax >and causing us to try to support ancient platforms >that we shouldn't be trying to support > Possibly. Part of the idea behind RHEL8/CentOS8 is that you'll get security fixes, but it should otherwise be a stable platform for production. Supporting this as a development environment for cutting edge versions does feel like a mismatch in visions at times. RHEL8 launched 2018-11-14, so is it reasonable to expect that cutting edge versions of QEMU 4 years in the future will build on it no problem? Meanwhile, OpenSUSE 15.4 released just eight months ago and still ships Python 3.6 as its default/platform Python. They don't ship a distro with a modern Python as default at all... Even the upcoming 15.5 release still appears to use Python 3.6 as the platform default. (Like RHEL, they offer optional additional interpreters if you want to run upstream software written within the last two years.) At least QEMU's stated support for RHEL8 runs out in another year or so. I wonder what OpenSUSE is planning to do. > > > because "use the system version of foo" should not be a big > deal -- it's not like we're trying to support decades-old > hosts here: Centos 8 was released in 2019, which is less than > five years ago. > Yeah, I have nothing cute to say to this. It's unfortunate that it's hard to support RHEL8 and Fedora 37 simultaneously as development environments for our devs. I do take great pains to reduce disruptions to everyone as I keep the python code afloat. I hope I have been successful these past few years. However, there are packaged versions of modern Python available for any platform we support that otherwise defaults to 3.6, so it's not a huge affair to continue supporting these older platforms. It'll just be as if you need one extra dependency on those systems. (Maybe two if you want to build docs.) I think that trade-off is fair; as it's only a one-time expense to developers still working on older platforms. > > The argument I'm making is: > > > > - CentOS 8 is a supported build platform > > - All platforms *do* have a Python modern enough to allow us to drop 3.6 > > - CentOS's repo version of sphinx is hardcoded to use the older 3.6, > though > > - You expressed a preference to me in the past to NOT use a pip > installed version of sphinx in preference to the system version in > "configure" > > - It's still possible to build docs on CentOS 8 after this patchset, you > just need a pip version. > > - We've used the justification that our build platform promise does not > necessarily extend to docs and tests in the past. > > -
Re: [PATCH v1 3/4] migration/multifd: Join all multifd threads in order to avoid leaks
On Fri, Feb 10, 2023 at 03:36:30AM -0300, Leonardo Bras wrote: > Current approach will only join threads that are still running. > > For the threads not joined, resources or private memory are always kept in > the process space and never reclaimed before process end, and this risks > serious memory leaks. > > This should usually not represent a big problem, since multifd migration > is usually just ran at most a few times, and after it succeeds there is > not much to be done before exiting the process. > > Yet still, it should not hurt performance to join all of them. > > Signed-off-by: Leonardo Bras Reviewed-by: Peter Xu -- Peter Xu
Re: [PATCH v1 2/4] migration/multifd: Remove unnecessary assignment on multifd_load_cleanup()
On Fri, Feb 10, 2023 at 03:36:29AM -0300, Leonardo Bras wrote: > Before assigning "p->quit = true" for every multifd channel, > multifd_load_cleanup() will call multifd_recv_terminate_threads() which > already does the same assignment, while protected by a mutex. > > So there is no point doing the same assignment again. > > Signed-off-by: Leonardo Bras Reviewed-by: Peter Xu -- Peter Xu
Re: [PATCH v3 14/14] tests/qtest: migration-test: Add tests for file-based migration
On Fri, Oct 28, 2022 at 01:39:14PM +0300, Nikolay Borisov wrote:
> Add basic tests for file-based migration as well as for the 'fixed-ram'
> feature.
>
> Signed-off-by: Nikolay Borisov
> ---
> tests/qtest/migration-test.c | 46
> 1 file changed, 46 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index ef4427ff4d41..de877473f193 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -748,6 +748,7 @@ static void test_migrate_end(QTestState *from, QTestState
> *to, bool test_dest)
> cleanup("migsocket");
> cleanup("src_serial");
> cleanup("dest_serial");
> +cleanup("migfile");
> }
>
> #ifdef CONFIG_GNUTLS
> @@ -1359,6 +1360,14 @@ static void test_precopy_common(MigrateCommon *args)
> * hanging forever if migration didn't converge */
> wait_for_migration_complete(from);
>
> +/*
> + * For file based migration the target must begin its migration after
> + * the source has finished
> + */
> +if (strstr(args->connect_uri, "file:")) {
> +migrate_incoming_qmp(to, args->connect_uri, "{}");
> +}
> +
> if (!got_stop) {
> qtest_qmp_eventwait(from, "STOP");
> }
> @@ -1514,6 +1523,39 @@ static void test_precopy_unix_xbzrle(void)
> test_precopy_common(&args);
> }
>
> +static void test_precopy_unix_file(void)
> +{
> +g_autofree char *uri = g_strdup_printf("file:%s/migfile", tmpfs);
> +MigrateCommon args = {
> +.connect_uri = uri,
> +.listen_uri = "defer",
> +};
> +
> +test_precopy_common(&args);
> +}
> +
> +static void *
> +test_migrate_fixed_ram_start(QTestState *from,
> + QTestState *to)
> +{
> +migrate_set_capability(from, "fixed-ram", true);
> +migrate_set_capability(to, "fixed-ram", true);
> +
> +return NULL;
> +}
> +
> +static void test_precopy_unix_fixed_ram(void)
> +{
> +g_autofree char *uri = g_strdup_printf("file:%s/migfile", tmpfs);
> +MigrateCommon args = {
> +.connect_uri = uri,
> +.listen_uri = "defer",
> +.start_hook = test_migrate_fixed_ram_start,
> +};
> +
> +test_precopy_common(&args);
> +}
> +
> static void test_precopy_tcp_plain(void)
> {
> MigrateCommon args = {
> @@ -2506,6 +2548,10 @@ int main(int argc, char **argv)
> test_precopy_unix_tls_psk);
> #endif
>
> +qtest_add_func("/migration/precopy/unix/file", test_precopy_unix_file);
> +qtest_add_func("/migration/precopy/unix/fixed-ram",
> + test_precopy_unix_fixed_ram);
Minor point '/unix' would indicate this is testing UNIX socket backend
for migration. The paths for the tests would be better as
/migration/precopy/file/stream-ram
/migration/precopy/file/fixed-ram
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 :|
Re: [PATCH v1 1/4] migration/multifd: Change multifd_load_cleanup() signature and usage
Leo,
Please still provide a cover letter as long as >1 patches will be posted as
a set.
Not only because it still always help to provide an overview for reviewers
before reading each of them (e.g. I have a habit of prioritizing reviews
based on cover letters first), but also when you're confident enough the
reviewer(s) can ACK the patches in one reply. :-)
On Fri, Feb 10, 2023 at 03:36:28AM -0300, Leonardo Bras wrote:
> Since it's introduction in commit f986c3d256 ("migration: Create multifd
> migration threads"), multifd_load_cleanup() never returned any value
> different than 0, neither set up any error on errp.
>
> Even though, on process_incoming_migration_bh() an if clause uses it's
> return value to decide on setting autostart = false, which will never
> happen.
>
> In order to simplify the codebase, change multifd_load_cleanup() signature
> to 'void multifd_load_cleanup(void)', and for every usage remove error
> handling or decision made based on return value != 0.
>
> Signed-off-by: Leonardo Bras
Reviewed-by: Peter Xu
--
Peter Xu
Re: [PATCH v2 6/7] CI: Stop building docs on centos8
On Fri, 10 Feb 2023 at 16:52, Daniel P. Berrangé wrote: > > On Fri, Feb 10, 2023 at 04:32:19PM +, Peter Maydell wrote: > > On Fri, 10 Feb 2023 at 16:01, John Snow wrote: > > > On Fri, Feb 10, 2023, 5:41 AM Peter Maydell > > > wrote: > > >> On Fri, 10 Feb 2023 at 00:31, John Snow wrote: > > >> This confuses me. We work fine with Python 3.6 today. > > > > > > > > > That won't last - Many tools such as mypy, pylint and flake8 which I use > > > to manage our python codebase have been dropping support for 3.6 and I've > > > had to implement an increasing number of workarounds to help keep it > > > possible to test 3.6 via CI while also ensuring our newest platforms work > > > as dev environments. > > > > Something somewhere seems kind of out-of-sync here. Either: > > * Python is deprecating old versions too quickly and > >dropping support for them too fast > > Nope, they're fine in declaring EOL whenever they like. There's > no requirement for upstreams to support arbitrary old versions > for any length of time. To be clear, yes, absolutely any other software project can set whatever lifecycle it chooses to do, and similarly distros can do what they wish to do. But somewhere along the line something's gone sideways here, because all these choices people are making seem reasonable and yet here we are :-) I put this bit in here because one response to "upstream's idea of how long it wants to support something is very short" would be "this isn't a dependency that it's suitable for QEMU as a project to have, because the mismatch is too great". (This would obviously be super-painful in this specific case...) > > * CentOS is retaining old versions of Python when it needs to > >ship newer ones > > It is also totally OK for an distro to ship and support software > versions which are EOL upstream. In fact for enterprise distros > you can generally assume that *all* the software versions they > ship are probably EOL or nearly so. The main value of enterprise > distros is that they provide long term support, where upstreams > are not willing to. But we as a project also have the choice, if it seems to us that Distro X is shipping absolutely ancient versions of dependencies, to say "we don't support distro X, because its life cycle policies are too far out of sync with ours", or to say "you're going to need to provide the dependencies yourself, here's a suggestion". (We effectively do that already with macos and Windows.) > > * Our policy for what distros we support is overly lax > >and causing us to try to support ancient platforms > >that we shouldn't be trying to support > > I don't think so. Users of distros are not anywhere near > as aggressive at updating their installations as users > are. The number of users of RHEL-8 will dwarf that of > RHEL-9 by a large margin for a decent amount of time > yet. Right. All of these things together seem to say: * Python is not an unreasonable thing for the project to depend on * CentOS 8 is not an unreasonable distro for us to want to continue to support * Therefore we should continue to work with the Python that ships with CentOS 8... [snip] > We don't have to drop python 3.6. It is a choice because > of a desire to be able to use some shiny new python > features without caring about back compat. > > Similarly we don't have to use the new meson which drops > support for python 3.6, it is again a choice because we > want to rely on some new meson features. > > QEMU could easily carry on supporting python 3.6 until > the affected distros drop out ofo our support matrix, but > we would have to opt out of using certain new features, > or put more effort into back compat. > > Personally I'm still on the side of carrying on supporting > 3.6 per our distro support policy, but if broad consensus > is to drop 3.6 I'm not going push back anymore. This is really where I'm at as well -- we set our distro support policy, and we know that means that sometimes we have to deal with continuing to support older versions of dependencies even when it might be easier for us if we could require newer versions. I'm reluctant to say that Python gets a special derogation from that policy. thanks -- PMM
Re: [PATCH v3 13/14] tests: Add migrate_incoming_qmp helper
On Fri, Oct 28, 2022 at 01:39:13PM +0300, Nikolay Borisov wrote: > file-based migration requires the target to initiate its migration after > the source has finished writing out the data in the file. Currently > there's no easy way to initiate 'migrate-incoming', allow this by > introducing migrate_incoming_qmp helper, similarly to migrate_qmp. > > Signed-off-by: Nikolay Borisov > --- > tests/qtest/migration-helpers.c | 19 +++ > tests/qtest/migration-helpers.h | 4 > 2 files changed, 23 insertions(+) Reviewed-by: Daniel P. Berrangé 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 :|
Re: [PATCH v6 00/33] Consolidate PIIX south bridges
On 10/2/23 17:27, Bernhard Beschow wrote: Am 23. Januar 2023 15:51:49 UTC schrieb Bernhard Beschow : Am 23. Januar 2023 09:25:51 UTC schrieb "Philippe Mathieu-Daudé" : On 20/1/23 13:22, Bernhard Beschow wrote: Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow : Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" : On 9/1/23 18:23, Bernhard Beschow wrote: This series consolidates the implementations of the PIIX3 and PIIX4 south bridges and is an extended version of [1]. The motivation is to share as much code as possible and to bring both device models to feature parity such that perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. This could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this list before. Bernhard Beschow (30): hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs() hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig hw/usb/hcd-uhci: Introduce TYPE_ defines for device models hw/intc/i8259: Make using the isa_pic singleton more type-safe hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC hw/i386/pc: Create RTC controllers in south bridges hw/i386/pc: No need for rtc_state to be an out-parameter hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge hw/isa/piix3: Create USB controller in host device hw/isa/piix3: Create power management controller in host device hw/isa/piix3: Create TYPE_ISA_PIC in host device hw/isa/piix3: Create IDE controller in host device hw/isa/piix3: Wire up ACPI interrupt internally hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4 hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4 hw/isa/piix3: Drop the "3" from PIIX base class hw/isa/piix4: Make PIIX4's ACPI and USB functions optional hw/isa/piix4: Remove unused inbound ISA interrupt lines hw/isa/piix4: Use TYPE_ISA_PIC device hw/isa/piix4: Reuse struct PIIXState from PIIX3 hw/isa/piix4: Rename reset control operations to match PIIX3 hw/isa/piix3: Merge hw/isa/piix4.c hw/isa/piix: Harmonize names of reset control memory regions hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 hw/isa/piix: Rename functions to be shared for interrupt triggering hw/isa/piix: Consolidate IRQ triggering hw/isa/piix: Share PIIX3's base class with PIIX4 Philippe Mathieu-Daudé (3): hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader hw/isa/piix4: Correct IRQRC[A:D] reset values I'm queuing the first 10 patches for now to alleviate the size of this series, and I'll respin a v7 with the rest to avoid making you suffer any longer :/ Thanks for insisting in this effort and I apologize it is taking me so long... Okay... What's the further plan? Is there anything missing? Ping The plan is "I'll respin a v7 with the rest". I understood that part. I wonder what the blocking issue is/was. The first part of this series contains piix3 changes such as the ISA proxy pic and movement of rtc. This seems the riskier part of the series to me which I'd like to get feedback on from the field rather sooner than later. The reason is that I can't currently forsee how fast I could react if these changes were merged during (soft) freeze. What bugs me is the i8259 proxy. I applied your patches locally, and am trying to rework to avoid using it. I amend a 'Co-developed-by' tag to patches modified: https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
Re: [PATCH v3 10/14] migration/ram: Introduce 'fixed-ram' migration stream capability
On Fri, Oct 28, 2022 at 01:39:10PM +0300, Nikolay Borisov wrote:
> Implement 'fixed-ram' feature. The core of the feature is to ensure that
> each ram page of the migration stream has a specific offset in the
> resulting migration stream. The reason why we'd want such behavior are
> two fold:
>
> - When doing a 'fixed-ram' migration the resulting file will have a
>bounded size, since pages which are dirtied multiple times will
>always go to a fixed location in the file, rather than constantly
>being added to a sequential stream. This eliminates cases where a vm
>with, say, 1g of ram can result in a migration file that's 10s of
>Gbs, provided that the workload constantly redirties memory.
>
> - It paves the way to implement DIO-enabled save/restore of the
>migration stream as the pages are ensured to be written at aligned
>offsets.
>
> The features requires changing the format. First, a bitmap is introduced
> which tracks which pages have been written (i.e are dirtied) during
> migration and subsequently it's being written in the resultin file,
> again at a fixed location for every ramblock. Zero pages are ignored as
> they'd be zero in the destination migration as well. With the changed
> format data would look like the following:
>
> |name len|name|used_len|pc*|bitmap_size|pages_offset|bitmap|pages|
>
> * pc - refers to the page_size/mr->addr members, so newly added members
> begin from "bitmap_size".
>
> This layout is initialized during ram_save_setup so instead of having a
> sequential stream of pages that follow the ramblock headers the dirty
> pages for a ramblock follow its header. Since all pages have a fixed
> location RAM_SAVE_FLAG_EOS is no longer generated on every migration
> iteration but there is effectively a single RAM_SAVE_FLAG_EOS right at
> the end.
>
> Signed-off-by: Nikolay Borisov
> ---
> include/exec/ramblock.h | 7 +++
> migration/migration.c | 51 +-
> migration/migration.h | 1 +
> migration/ram.c | 97 +
> migration/savevm.c | 1 +
> qapi/migration.json | 2 +-
> 6 files changed, 138 insertions(+), 21 deletions(-)
This patch probably ought to extends the docs/devel/migration.rst
file. Specifically the text following the 'Stream structure'
heading.
>
> diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
> index 6cbedf9e0c9a..30216c1a41d3 100644
> --- a/include/exec/ramblock.h
> +++ b/include/exec/ramblock.h
> @@ -43,6 +43,13 @@ struct RAMBlock {
> size_t page_size;
> /* dirty bitmap used during migration */
> unsigned long *bmap;
> +/* shadow dirty bitmap used when migrating to a file */
> +unsigned long *shadow_bmap;
> +/* offset in the file pages belonging to this ramblock are saved, used
> + * only during migration to a file
> + */
> +off_t bitmap_offset;
> +uint64_t pages_offset;
> /* bitmap of already received pages in postcopy */
> unsigned long *receivedmap;
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 11ceea340702..c7383845a5b4 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -165,7 +165,8 @@
> INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
> MIGRATION_CAPABILITY_XBZRLE,
> MIGRATION_CAPABILITY_X_COLO,
> MIGRATION_CAPABILITY_VALIDATE_UUID,
> -MIGRATION_CAPABILITY_ZERO_COPY_SEND);
> +MIGRATION_CAPABILITY_ZERO_COPY_SEND,
> +MIGRATION_CAPABILITY_FIXED_RAM);
>
> /* When we add fault tolerance, we could have several
> migrations at once. For now we don't need to add
> @@ -1326,6 +1327,27 @@ static bool migrate_caps_check(bool *cap_list,
> }
> #endif
>
> +if (cap_list[MIGRATION_CAPABILITY_FIXED_RAM]) {
> + if (cap_list[MIGRATION_CAPABILITY_MULTIFD]) {
> + error_setg(errp, "Directly mapped memory incompatible with
> multifd");
> + return false;
> + }
> +
> + if (cap_list[MIGRATION_CAPABILITY_XBZRLE]) {
> + error_setg(errp, "Directly mapped memory incompatible with
> xbzrle");
> + return false;
> + }
> +
> + if (cap_list[MIGRATION_CAPABILITY_COMPRESS]) {
> + error_setg(errp, "Directly mapped memory incompatible with
> compression");
> + return false;
> + }
> +
> + if (cap_list[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
> + error_setg(errp, "Directly mapped memory incompatible with
> postcopy ram");
> + return false;
> + }
> +}
>
> /* incoming side only */
> if (runstate_check(RUN_STATE_INMIGRATE) &&
> @@ -2630,6 +2652,11 @@ MultiFDCompression migrate_multifd_compression(void)
> return s->parameters.multifd_compression;
> }
>
> +int migrate_fixed_ram(void)
> +{
> +return
> migrate_get_current()->enabled_capabilities[MIGRATION_CAPABILITY_FIXED_RAM];
> +}
> +
> int migr
Re: [PATCH v3 0/3] Add gdbstub support to HVF
Hi Francesco, On 14/1/23 17:12, francesco.cag...@gmail.com wrote: From: Francesco Cagnin This patch series aims to add gdbstub support to HVF (the 'QEMU accelerator on macOS that employs Hypervisor.framework') on Apple Silicon hosts. The patch has been most recently tested working on macOS Ventura 13.1 hosts and Linux kernel 5.19 guests with the test script 'tests/guest-debug/test-gdbstub.py' (slightly updated to make it work with Linux kernels compiled on macOS). Could you share the test-gdbstub.py changes? Thanks, Phil.
Re: [PATCH v3 08/14] io: Add preadv support to QIOChannelFile
On Fri, Oct 28, 2022 at 01:39:08PM +0300, Nikolay Borisov wrote:
> preadv is going to be needed when 'fixed-ram'-enabled stream are to be
> restored. Add a minimal implementation of preadv for file channels and
> expose it via the generic io_preadv interface.
>
> Signed-off-by: Nikolay Borisov
> ---
> io/channel-file.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/io/channel-file.c b/io/channel-file.c
> index e213a0fd7cd2..d2f4706b7f6d 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -145,6 +145,32 @@ static ssize_t qio_channel_file_writev(QIOChannel *ioc,
> return ret;
> }
>
> +static ssize_t qio_channel_file_preadv(QIOChannel *ioc,
> + const struct iovec *iov,
> + size_t niov,
> + off_t offset,
> + Error **errp)
> +{
> +QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
> +ssize_t ret;
> +
> + retry:
> +ret = preadv(fioc->fd, iov, niov, offset);
> +if (ret < 0) {
> +if (errno == EAGAIN) {
> +return QIO_CHANNEL_ERR_BLOCK;
> +}
> +if (errno == EINTR) {
> +goto retry;
> +}
> +
> +error_setg_errno(errp, errno, "Unable to read from file");
> +return -1;
> +}
> +
> +return ret;
> +}
> +
> static ssize_t qio_channel_file_pwritev(QIOChannel *ioc,
> const struct iovec *iov,
> size_t niov,
> @@ -252,6 +278,7 @@ static void qio_channel_file_class_init(ObjectClass
> *klass,
> ioc_klass->io_readv = qio_channel_file_readv;
> ioc_klass->io_set_blocking = qio_channel_file_set_blocking;
> ioc_klass->io_pwritev = qio_channel_file_pwritev;
> +ioc_klass->io_preadv = qio_channel_file_preadv;
> ioc_klass->io_seek = qio_channel_file_seek;
> ioc_klass->io_close = qio_channel_file_close;
> ioc_klass->io_create_watch = qio_channel_file_create_watch;
I'd suggest this patch should just be merged into the patch which
adds the io_pwritev callback.
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 :|
[PATCH 0/4] hw/isa/piix: Housekeeping QOM names / macros
- Use QOM macros - Unify QOM type names Based-on: <20230210163744.32182-1-phi...@linaro.org> Philippe Mathieu-Daudé (4): hw/southbridge/piix: Use OBJECT_DECLARE_SIMPLE_TYPE() macro hw/isa/piix: Batch register QOM types using DEFINE_TYPES() macro hw/isa/piix: Unify QOM type name of PIIX ISA function hw/isa/piix: Unify PIIX-ISA QOM type names using qdev aliases hw/i386/pc_piix.c | 5 ++- hw/isa/piix3.c| 63 +++ hw/isa/piix4.c| 10 +++--- hw/mips/malta.c | 2 +- include/hw/southbridge/piix.h | 14 softmmu/qdev-monitor.c| 3 ++ 6 files changed, 44 insertions(+), 53 deletions(-) -- 2.38.1
Re: [PATCH v3 09/14] migration: add qemu_get_buffer_at
On Fri, Oct 28, 2022 at 01:39:09PM +0300, Nikolay Borisov wrote:
> Restoring a 'fixed-ram' enabled migration stream would require reading
> from specific offsets in the file so add a helper to QEMUFile that uses
> the newly introduced qio_channel_file_preadv.
>
> Signed-off-by: Nikolay Borisov
> ---
> migration/qemu-file.c | 23 +++
> migration/qemu-file.h | 1 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index d0e0ba6150f7..b24972d5728d 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -564,6 +564,29 @@ void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf,
> size_t buflen, off_t po
> return;
> }
>
> +
> +size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
> off_t pos)
> +{
> +Error *err = NULL;
> +struct iovec iov = { .iov_base = (char *)buf, .iov_len = buflen };
> +ssize_t ret;
> +
> +if (f->last_error) {
> +return 0;
> +}
> +
> +ret = qio_channel_io_preadv(f->ioc, &iov, 1, pos, &err);
If we have a qio_channel_io_preadv that does NOT use iovecs,
then this code gets simpler, as the iovec wrapping can be
hidden in the QIOChannel code.
> +if (ret == -1) {
> + goto error;
> +}
> +
> +return (size_t)ret;
> +
> + error:
> +qemu_file_set_error_obj(f, -EIO, err);
> +return 0;
> +}
> +
> void qemu_set_offset(QEMUFile *f, off_t off, int whence)
> {
> Error *err = NULL;
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 33cfc07b81d1..ab10c3ad7e42 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -151,6 +151,7 @@ void qemu_file_set_blocking(QEMUFile *f, bool block);
> void qemu_set_offset(QEMUFile *f, off_t off, int whence);
> off_t qemu_get_offset(QEMUFile *f);
> void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
> off_t pos);
> +size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen,
> off_t pos);
>
> void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
> void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
> --
> 2.34.1
>
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 :|
[PATCH 3/4] hw/isa/piix: Unify QOM type name of PIIX ISA function
Mechanical change doing:
$ sed -i -e 's/PIIX4_PCI_DEVICE/PIIX4_ISA/g' $(git grep -l PIIX4_PCI_DEVICE)
$ sed -i -e 's/PIIX3_XEN_DEVICE/PIIX3_ISA_XEN/g' $(git grep -l
PIIX3_XEN_DEVICE)
$ sed -i -e 's/PIIX3_DEVICE/PIIX3_ISA/g' $(git grep -l PIIX3_DEVICE)
$ sed -i -e 's/PIIX3_PCI_DEVICE/PIIX_ISA/g' $(git grep -l PIIX3_PCI_DEVICE)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/i386/pc_piix.c | 5 ++---
hw/isa/piix3.c| 20 ++--
hw/isa/piix4.c| 10 +-
hw/mips/malta.c | 2 +-
include/hw/southbridge/piix.h | 10 +-
5 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7c48ba30e0..afef5ed115 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -220,8 +220,7 @@ static void pc_init1(MachineState *machine,
if (pcmc->pci_enabled) {
PIIX3State *piix3;
PCIDevice *pci_dev;
-const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
- : TYPE_PIIX3_DEVICE;
+const char *type = xen_enabled() ? TYPE_PIIX3_ISA_XEN : TYPE_PIIX3_ISA;
pci_bus = i440fx_init(pci_type,
i440fx_host,
@@ -235,7 +234,7 @@ static void pc_init1(MachineState *machine,
pcms->bus = pci_bus;
pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
-piix3 = PIIX3_PCI_DEVICE(pci_dev);
+piix3 = PIIX3_ISA(pci_dev);
piix3->pic = x86ms->gsi;
piix3_devfn = piix3->dev.devfn;
isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 0ee94a2313..38e0c269ae 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -112,7 +112,7 @@ static void piix3_write_config(PCIDevice *dev,
{
pci_default_write_config(dev, address, val, len);
if (ranges_overlap(address, len, PIIX_PIRQCA, 4)) {
-PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+PIIX3State *piix3 = PIIX3_ISA(dev);
int pic_irq;
pci_bus_fire_intx_routing_notifier(pci_get_bus(&piix3->dev));
@@ -145,7 +145,7 @@ static void piix3_write_config_xen(PCIDevice *dev,
static void piix3_reset(DeviceState *dev)
{
-PIIX3State *d = PIIX3_PCI_DEVICE(dev);
+PIIX3State *d = PIIX3_ISA(dev);
uint8_t *pci_conf = d->dev.config;
pci_conf[0x04] = 0x07; /* master, memory and I/O */
@@ -286,7 +286,7 @@ static const MemoryRegionOps rcr_ops = {
static void pci_piix3_realize(PCIDevice *dev, Error **errp)
{
-PIIX3State *d = PIIX3_PCI_DEVICE(dev);
+PIIX3State *d = PIIX3_ISA(dev);
ISABus *isa_bus;
isa_bus = isa_bus_new(DEVICE(d), pci_address_space(dev),
@@ -349,7 +349,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void
*data)
static void piix3_realize(PCIDevice *dev, Error **errp)
{
ERRP_GUARD();
-PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+PIIX3State *piix3 = PIIX3_ISA(dev);
PCIBus *pci_bus = pci_get_bus(dev);
pci_piix3_realize(dev, errp);
@@ -372,7 +372,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
static void piix3_xen_realize(PCIDevice *dev, Error **errp)
{
ERRP_GUARD();
-PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+PIIX3State *piix3 = PIIX3_ISA(dev);
PCIBus *pci_bus = pci_get_bus(dev);
pci_piix3_realize(dev, errp);
@@ -399,7 +399,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void
*data)
static const TypeInfo piix_isa_types[] = {
{
-.name = TYPE_PIIX3_PCI_DEVICE,
+.name = TYPE_PIIX_ISA,
.parent = TYPE_PCI_DEVICE,
.instance_size = sizeof(PIIX3State),
.class_init = pci_piix3_class_init,
@@ -410,12 +410,12 @@ static const TypeInfo piix_isa_types[] = {
{ },
},
}, {
-.name = TYPE_PIIX3_DEVICE,
-.parent = TYPE_PIIX3_PCI_DEVICE,
+.name = TYPE_PIIX3_ISA,
+.parent = TYPE_PIIX_ISA,
.class_init = piix3_class_init,
}, {
-.name = TYPE_PIIX3_XEN_DEVICE,
-.parent = TYPE_PIIX3_PCI_DEVICE,
+.name = TYPE_PIIX3_ISA_XEN,
+.parent = TYPE_PIIX_ISA,
.class_init = piix3_xen_class_init,
}
};
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index ef24826993..8c51b523e5 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -56,7 +56,7 @@ struct PIIX4State {
uint8_t rcr;
};
-OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
+OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_ISA)
static void piix4_set_irq(void *opaque, int irq_num, int level)
{
@@ -81,7 +81,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
static void piix4_isa_reset(DeviceState *dev)
{
-PIIX4State *d = PIIX4_PCI_DEVICE(dev);
+PIIX4State *d = PIIX4_ISA(dev);
uint8_t *pci_conf = d->dev.config;
pci_con
[PATCH 4/4] hw/isa/piix: Unify PIIX-ISA QOM type names using qdev aliases
Unify PIIX ISA (PCI function #0) as:
pci-piix3 -> piix-isa (abstract base class)
PIIX3 -> piix3-isa (PIIX3 implementation)
PIIX3-xen -> piix3-isa-xen (PIIX3 implementation with Xen extensions)
piix4-isa -> piix4-isa (PIIX4 implementation)
Alias previous names in the QDevAlias table.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/southbridge/piix.h | 6 +++---
softmmu/qdev-monitor.c| 3 +++
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 71a82ef266..cce65e8f44 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -58,9 +58,9 @@ struct PIIX3State {
MemoryRegion rcr_mem;
};
-#define TYPE_PIIX_ISA "pci-piix3"
-#define TYPE_PIIX3_ISA "PIIX3"
-#define TYPE_PIIX3_ISA_XEN "PIIX3-xen"
+#define TYPE_PIIX_ISA "piix-isa"
+#define TYPE_PIIX3_ISA "piix3-isa"
+#define TYPE_PIIX3_ISA_XEN "piix3-isa-xen"
#define TYPE_PIIX4_ISA "piix4-isa"
OBJECT_DECLARE_SIMPLE_TYPE(PIIX3State, PIIX3_ISA)
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index b8d2c4dadd..820e7f52ad 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -72,6 +72,9 @@ static const QDevAlias qdev_alias_table[] = {
{ "ES1370", "es1370" }, /* -soundhw name */
{ "ich9-ahci", "ahci" },
{ "lsi53c895a", "lsi" },
+{ "piix-isa", "pci-piix3" },
+{ "piix3-isa", "PIIX3" },
+{ "piix3-isa-xen", "PIIX3-xen" },
{ "virtio-9p-device", "virtio-9p", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_VIRTIO_PCI },
--
2.38.1
Re: [PATCH v3 04/14] io: Add generic pwritev/preadv interface
On Fri, Feb 10, 2023 at 04:26:22PM +, Daniel P. Berrangé wrote:
> On Fri, Oct 28, 2022 at 01:39:04PM +0300, Nikolay Borisov wrote:
> > Introduce basic pwriteve/preadv support in the generic channel layer.
> > SPecific implementation will follow for the file channel as this is
> > required in order to support migration streams with fixed location of
> > each ram page.
> >
> > Signed-off-by: Nikolay Borisov
> > ---
> > include/io/channel.h | 49
> > io/channel.c | 26 +++
> > 2 files changed, 75 insertions(+)
> >
> > diff --git a/include/io/channel.h b/include/io/channel.h
> > index c680ee748021..6b10bce8bbdf 100644
> > --- a/include/io/channel.h
> > +++ b/include/io/channel.h
> > @@ -124,6 +124,16 @@ struct QIOChannelClass {
> > Error **errp);
> >
> > /* Optional callbacks */
> > +ssize_t (*io_pwritev)(QIOChannel *ioc,
> > + const struct iovec *iov,
> > + size_t niov,
> > + off_t offset,
> > + Error **errp);
> > +ssize_t (*io_preadv)(QIOChannel *ioc,
> > + const struct iovec *iov,
> > + size_t niov,
> > + off_t offset,
> > + Error **errp);
>
> nit-pick the alignment of the 2nd line of parameters onwards,
> needs to be indented by 3 more spaces.
>
>
> > +/**
> > + * qio_channel_io_pwritev
> > + * @ioc: the channel object
> > + * @iov: the array of memory regions to write data from
> > + * @niov: the length of the @iov array
> > + * @offset: offset in the channel where writes should begin
> > + * @errp: pointer to a NULL-initialized error object
> > + *
> > + * Not all implementations will support this facility, so may report an
> > error.
> > + * To avoid errors, the caller may check for the feature flag
> > + * QIO_CHANNEL_FEATURE_SEEKABLE prior to calling this method.
> > + *
> > + * Behaves as qio_channel_writev_full, apart from not supporting sending
> > of file
>
> Given this, we should have "_full" suffix on these methods
> too for consistent naming policy.
>
> > + * handles as well as beginning the write at the passed @offset
> > + *
> > + */
> > +ssize_t qio_channel_io_pwritev(QIOChannel *ioc, const struct iovec *iov,
> > + size_t niov, off_t offset, Error **errp);
In addition to giving this method the '_full' suffix, we should also
add the qio_channel_io_pwritev entrypoint which takes a single
buffer instead of iovec. The migration code in QEMUFile that is
added in a later patch in this series doesn't actually want to
use iovecs in the first place. So having the non-iov entrypoints
in QIOChannel will simplify the migration code.
> > +
> > +
> > +/**
> > + * qio_channel_io_preadv
> > + * @ioc: the channel object
> > + * @iov: the array of memory regions to read data into
> > + * @niov: the length of the @iov array
> > + * @offset: offset in the channel where writes should begin
> > + * @errp: pointer to a NULL-initialized error object
> > + *
> > + * Not all implementations will support this facility, so may report an
> > error.
> > + * To avoid errors, the caller may check for the feature flag
> > + * QIO_CHANNEL_FEATURE_SEEKABLE prior to calling this method.
> > + *
> > + * Behaves as qio_channel_readv_full, apart from not supporting receiving
> > of file
> > + * handles as well as beginning the read at the passed @offset
> > + *
> > + */
> > +ssize_t qio_channel_io_preadv(QIOChannel *ioc, const struct iovec *iov,
> > + size_t niov, off_t offset, Error **errp);
> > /**
> > * qio_channel_shutdown:
> > * @ioc: the channel object
> > diff --git a/io/channel.c b/io/channel.c
> > index 0640941ac573..f5ac9499a7ad 100644
> > --- a/io/channel.c
> > +++ b/io/channel.c
> > @@ -437,6 +437,32 @@ GSource *qio_channel_add_watch_source(QIOChannel *ioc,
> > }
> >
> >
> > +ssize_t qio_channel_io_pwritev(QIOChannel *ioc, const struct iovec *iov,
> > + size_t niov, off_t offset, Error **errp)
> > +{
> > +QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> > +
> > +if (!klass->io_pwritev) {
> > +error_setg(errp, "Channel does not support pwritev");
> > +return -1;
> > +}
>
> This also possibly benefits from a validation check
>
> if (!qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_SEEKABLE)) {
> error_setg_errno(errp, EINVAL,
> "Requested channel is not seekable")
> return -1;
> }
>
> because the QIOChannelFile impl will support io_pwritev callback,
> but not all the FDs it can use will support seeking.
>
> Same check for preadv
>
> > +
> > +return klass->io_pwritev(ioc, iov, niov, offset, errp);
> > +}
> > +
> > +ssize_t qio_channel_io_preadv(QIOChannel *ioc, const struct iovec *iov,
> > + size_t nio
[PATCH 2/4] hw/isa/piix: Batch register QOM types using DEFINE_TYPES() macro
See rationale in commit 38b5d79b2e ("qom: add helper
macro DEFINE_TYPES()").
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/piix3.c | 53 +-
1 file changed, 22 insertions(+), 31 deletions(-)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index a9cb39bf21..0ee94a2313 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -346,19 +346,6 @@ static void pci_piix3_class_init(ObjectClass *klass, void
*data)
adevc->build_dev_aml = build_pci_isa_aml;
}
-static const TypeInfo piix3_pci_type_info = {
-.name = TYPE_PIIX3_PCI_DEVICE,
-.parent = TYPE_PCI_DEVICE,
-.instance_size = sizeof(PIIX3State),
-.abstract = true,
-.class_init = pci_piix3_class_init,
-.interfaces = (InterfaceInfo[]) {
-{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
-{ TYPE_ACPI_DEV_AML_IF },
-{ },
-},
-};
-
static void piix3_realize(PCIDevice *dev, Error **errp)
{
ERRP_GUARD();
@@ -382,12 +369,6 @@ static void piix3_class_init(ObjectClass *klass, void
*data)
k->realize = piix3_realize;
}
-static const TypeInfo piix3_info = {
-.name = TYPE_PIIX3_DEVICE,
-.parent= TYPE_PIIX3_PCI_DEVICE,
-.class_init= piix3_class_init,
-};
-
static void piix3_xen_realize(PCIDevice *dev, Error **errp)
{
ERRP_GUARD();
@@ -416,17 +397,27 @@ static void piix3_xen_class_init(ObjectClass *klass, void
*data)
k->realize = piix3_xen_realize;
}
-static const TypeInfo piix3_xen_info = {
-.name = TYPE_PIIX3_XEN_DEVICE,
-.parent= TYPE_PIIX3_PCI_DEVICE,
-.class_init= piix3_xen_class_init,
+static const TypeInfo piix_isa_types[] = {
+{
+.name = TYPE_PIIX3_PCI_DEVICE,
+.parent = TYPE_PCI_DEVICE,
+.instance_size = sizeof(PIIX3State),
+.class_init = pci_piix3_class_init,
+.abstract = true,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
+{ TYPE_ACPI_DEV_AML_IF },
+{ },
+},
+}, {
+.name = TYPE_PIIX3_DEVICE,
+.parent = TYPE_PIIX3_PCI_DEVICE,
+.class_init = piix3_class_init,
+}, {
+.name = TYPE_PIIX3_XEN_DEVICE,
+.parent = TYPE_PIIX3_PCI_DEVICE,
+.class_init = piix3_xen_class_init,
+}
};
-static void piix3_register_types(void)
-{
-type_register_static(&piix3_pci_type_info);
-type_register_static(&piix3_info);
-type_register_static(&piix3_xen_info);
-}
-
-type_init(piix3_register_types)
+DEFINE_TYPES(piix_isa_types)
--
2.38.1
[PATCH 1/4] hw/southbridge/piix: Use OBJECT_DECLARE_SIMPLE_TYPE() macro
Manually convert to OBJECT_DECLARE_SIMPLE_TYPE() macro,
similarly to automatic conversion from commit 8063396bf3
("Use OBJECT_DECLARE_SIMPLE_TYPE when possible").
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/southbridge/piix.h | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0bf48e936d..a58bf13a41 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -29,7 +29,7 @@
#define PIIX_NUM_PIC_IRQS 16 /* i8259 * 2 */
#define PIIX_NUM_PIRQS 4ULL/* PIRQ[A-D] */
-struct PIIXState {
+struct PIIX3State {
PCIDevice dev;
/*
@@ -57,14 +57,12 @@ struct PIIXState {
/* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
MemoryRegion rcr_mem;
};
-typedef struct PIIXState PIIX3State;
#define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
-DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
- TYPE_PIIX3_PCI_DEVICE)
-
#define TYPE_PIIX3_DEVICE "PIIX3"
#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
+OBJECT_DECLARE_SIMPLE_TYPE(PIIX3State, PIIX3_PCI_DEVICE)
+
#endif
--
2.38.1
Re: [PATCH v2 6/7] CI: Stop building docs on centos8
On Fri, Feb 10, 2023 at 04:32:19PM +, Peter Maydell wrote: > On Fri, 10 Feb 2023 at 16:01, John Snow wrote: > > On Fri, Feb 10, 2023, 5:41 AM Peter Maydell > > wrote: > >> On Fri, 10 Feb 2023 at 00:31, John Snow wrote: > >> This confuses me. We work fine with Python 3.6 today. > > > > > > That won't last - Many tools such as mypy, pylint and flake8 which I use to > > manage our python codebase have been dropping support for 3.6 and I've had > > to implement an increasing number of workarounds to help keep it possible > > to test 3.6 via CI while also ensuring our newest platforms work as dev > > environments. > > Something somewhere seems kind of out-of-sync here. Either: > * Python is deprecating old versions too quickly and >dropping support for them too fast Nope, they're fine in declaring EOL whenever they like. There's no requirement for upstreams to support arbitrary old versions for any length of time. > * CentOS is retaining old versions of Python when it needs to >ship newer ones It is also totally OK for an distro to ship and support software versions which are EOL upstream. In fact for enterprise distros you can generally assume that *all* the software versions they ship are probably EOL or nearly so. The main value of enterprise distros is that they provide long term support, where upstreams are not willing to. > * Our policy for what distros we support is overly lax >and causing us to try to support ancient platforms >that we shouldn't be trying to support I don't think so. Users of distros are not anywhere near as aggressive at updating their installations as users are. The number of users of RHEL-8 will dwarf that of RHEL-9 by a large margin for a decent amount of time yet. The challenge here is that upstream developers want to use shiny new features from latest upstream, and at the same time don't want to keep back compat with older versions that users see in their real world deployed distros, because that is a burden. Ideally upstream developers would never care about old versions and only ever target the very latest upstream. Meanwhile for users, apps would ideally support every version of every OS that's ever existed. Somewhere in the middle we have to strike a bargain that spreads the pain fairly between the two groups, accepting that this is going to be a burden for both to some degre. Our distro support policy is a decent attempt at doing that IMHO and has worked pretty well IMHO. We don't have to drop python 3.6. It is a choice because of a desire to be able to use some shiny new python features without caring about back compat. Similarly we don't have to use the new meson which drops support for python 3.6, it is again a choice because we want to rely on some new meson features. QEMU could easily carry on supporting python 3.6 until the affected distros drop out ofo our support matrix, but we would have to opt out of using certain new features, or put more effort into back compat. Personally I'm still on the side of carrying on supporting 3.6 per our distro support policy, but if broad consensus is to drop 3.6 I'm not going push back anymore. > because "use the system version of foo" should not be a big > deal -- it's not like we're trying to support decades-old > hosts here: Centos 8 was released in 2019, which is less than > five years ago. Yeah, it isn't very old at all, and in terms of deployments will dominate over CentOS/RHEL 9 users. > > The argument I'm making is: > > > > - CentOS 8 is a supported build platform > > - All platforms *do* have a Python modern enough to allow us to drop 3.6 > > - CentOS's repo version of sphinx is hardcoded to use the older 3.6, though > > - You expressed a preference to me in the past to NOT use a pip installed > > version of sphinx in preference to the system version in "configure" > > - It's still possible to build docs on CentOS 8 after this patchset, you > > just need a pip version. > > - We've used the justification that our build platform promise does not > > necessarily extend to docs and tests in the past. > > - So just skip docs building for CentOS 8, only in the CI. > > > > If you believe docs in CI for CentOS 8 is a hard deal breaker, then I want > > to go back to discussing the possibility of using sphinx versions from pip. > > If Python 3.6 is EOL, shouldn't CentOS have shipped an updated > version of Sphinx to go with their updated Python ? CentOS isn't dropping python 3.6 support. It will exist for the lifetime of CentOS 8. It just decided to also ship some newer python versions as an opt-in. These newer python versions are just the minimal runtime though. CentOS is not shipping all the add-on python modules it has with 3.6. 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/dber
Re: [RFC PATCH 1/5] Rename the singlestep global variable to one_insn_per_tb
On Mon, 6 Feb 2023 at 20:20, Thomas Huth wrote: > > On 06/02/2023 18.13, Peter Maydell wrote: > > The 'singlestep' global variable is badly misnamed, because it has > > nothing to do with single-stepping the emulation either via the gdb > > stub or by emulation of architectural debug facilities. Instead what > > it does is force TCG to put only one instruction into each TB. > > Rename it to one_insn_per_tb, so that it reflects what it does better > > and is easier to search for in the codebase. > > > > This misnaming is also present in user-facing interfaces: the command > > line option '-singlestep', the HMP 'singlestep' command, and the QMP > > StatusInfo object. Those are harder to update because we need to > > retain backwards compatibility. Subsequent patches will add > > better-named aliases so we can eventually deprecate-and-drop the old, > > bad name. > > > > Signed-off-by: Peter Maydell > > --- > > include/exec/cpu-common.h | 2 +- > > accel/tcg/cpu-exec.c| 4 ++-- > > bsd-user/main.c | 4 ++-- > > linux-user/main.c | 4 ++-- > > softmmu/globals.c | 2 +- > > softmmu/runstate-hmp-cmds.c | 4 ++-- > > softmmu/runstate.c | 2 +- > > softmmu/vl.c| 2 +- > > 8 files changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h > > index 6feaa40ca7b..f2592a1967f 100644 > > --- a/include/exec/cpu-common.h > > +++ b/include/exec/cpu-common.h > > @@ -163,7 +163,7 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr, > > void *ptr, size_t len, bool is_write); > > > > /* vl.c */ > > -extern int singlestep; > > +extern int one_insn_per_tb; > > IMHO the global variable should completely go away - this should get a > property of the tcg accel instead. I agree in principle, but this might be tricky because of the user-mode emulators. I'll have a look. -- PMM
Re: [PATCH v10 41/59] hw/xen: Support HVM_PARAM_CALLBACK_TYPE_PCI_INTX callback
On 01/02/2023 14:31, David Woodhouse wrote: From: David Woodhouse The guest is permitted to specify an arbitrary domain/bus/device/function and INTX pin from which the callback IRQ shall appear to have come. In QEMU we can only easily do this for devices that actually exist, and even that requires us "knowing" that it's a PCMachine in order to find the PCI root bus — although that's OK really because it's always true. We also don't get to get notified of INTX routing changes, because we can't do that as a passive observer; if we try to register a notifier it will overwrite any existing notifier callback on the device. But in practice, guests using PCI_INTX will only ever use pin A on the Xen platform device, and won't swizzle the INTX routing after they set it up. So this is just fine. Signed-off-by: David Woodhouse --- hw/i386/kvm/xen_evtchn.c | 80 --- target/i386/kvm/xen-emu.c | 34 + 2 files changed, 100 insertions(+), 14 deletions(-) Reviewed-by: Paul Durrant
Re: [PATCH v3 1/3] arm: move KVM breakpoints helpers
On Sat, 14 Jan 2023 at 16:13, wrote: > > From: Francesco Cagnin > > These helpers will be also used for HVF. Aside from reformatting a > couple of comments for 'checkpatch.pl' and updating meson to compile > 'hyp_gdbstub.c', this is just code motion. > > Signed-off-by: Francesco Cagnin > --- > target/arm/hyp_gdbstub.c | 242 ++ > target/arm/internals.h | 50 +++ > target/arm/kvm64.c | 276 --- > target/arm/meson.build | 3 +- > 4 files changed, 294 insertions(+), 277 deletions(-) > create mode 100644 target/arm/hyp_gdbstub.c > > diff --git a/target/arm/hyp_gdbstub.c b/target/arm/hyp_gdbstub.c > new file mode 100644 > index 00..22b2b7de7b > --- /dev/null > +++ b/target/arm/hyp_gdbstub.c > @@ -0,0 +1,242 @@ > +#include "qemu/osdep.h" > +#include "cpu.h" > +#include "internals.h" > +#include "exec/gdbstub.h" New files must all start with the usual boilerplate comment stating the license and copyright. Sorry I didn't notice this in previous rounds of review. Otherwise Reviewed-by: Peter Maydell (which means "if you make this change in the next version of the patchset, you should put this Reviewed-by: tag into the commit message, so that reviewers know it's already been reviewed".) thanks -- PMM
[PATCH 2/2] hw/arm/smmu-common: Fix TTB1 handling
Addresses targeting the second translation table (TTB1) in the SMMU have
all upper bits set (except for the top byte when TBI is enabled). Fix
the TTB1 check.
Reported-by: Ola Hugosson
Signed-off-by: Jean-Philippe Brucker
---
hw/arm/smmu-common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 2b8c67b9a1..0a5a60ca1e 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -249,7 +249,7 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t
iova)
/* there is a ttbr0 region and we are in it (high bits all zero) */
return &cfg->tt[0];
} else if (cfg->tt[1].tsz &&
- !extract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte)) {
+sextract64(iova, 64 - cfg->tt[1].tsz, cfg->tt[1].tsz - tbi_byte) ==
-1) {
/* there is a ttbr1 region and we are in it (high bits all one) */
return &cfg->tt[1];
} else if (!cfg->tt[0].tsz) {
--
2.39.0
[PATCH 09/11] hw/isa: Rename isa_bus_irqs() -> isa_bus_register_input_irqs()
isa_bus_irqs() register an array of input IRQs on
the ISA bus. Rename it as isa_bus_register_input_irqs().
Mechanical change using:
$ sed -i -e 's/isa_bus_irqs/isa_bus_register_input_irqs/g' \
$(git grep -wl isa_bus_irqs)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/hppa/machine.c| 2 +-
hw/i386/microvm.c| 2 +-
hw/i386/pc_piix.c| 2 +-
hw/isa/i82378.c | 2 +-
hw/isa/isa-bus.c | 8
hw/isa/lpc_ich9.c| 2 +-
hw/isa/piix4.c | 2 +-
hw/isa/vt82c686.c| 2 +-
hw/mips/jazz.c | 2 +-
hw/ppc/pnv_lpc.c | 2 +-
hw/sparc64/sun4u.c | 2 +-
include/hw/isa/isa.h | 4 ++--
12 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 7ac68c943f..8fea5fa6b8 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -98,7 +98,7 @@ static ISABus *hppa_isa_bus(void)
isa_irqs = i8259_init(isa_bus,
/* qemu_allocate_irq(dino_set_isa_irq, s, 0)); */
NULL);
-isa_bus_irqs(isa_bus, isa_irqs);
+isa_bus_register_input_irqs(isa_bus, isa_irqs);
return isa_bus;
}
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 29f30dd6d3..fed468a34d 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -174,7 +174,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
isa_bus = isa_bus_new(NULL, get_system_memory(), get_system_io(),
&error_abort);
-isa_bus_irqs(isa_bus, x86ms->gsi);
+isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
ioapic_init_gsi(gsi_state, "machine");
if (ioapics > 1) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df64dd8dcc..7c48ba30e0 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -246,7 +246,7 @@ static void pc_init1(MachineState *machine,
i8257_dma_init(isa_bus, 0);
pcms->hpet_enabled = false;
}
-isa_bus_irqs(isa_bus, x86ms->gsi);
+isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
pc_i8259_create(isa_bus, gsi_state->i8259_irq);
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index d32653369d..233059c6dc 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -89,7 +89,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
/* 2 82C59 (irq) */
s->isa_irqs_in = i8259_init(isabus, s->cpu_intr);
-isa_bus_irqs(isabus, s->isa_irqs_in);
+isa_bus_register_input_irqs(isabus, s->isa_irqs_in);
/* 1 82C54 (pit) */
pit = i8254_pit_init(isabus, 0x40, 0, NULL);
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 5bd99379e9..d19826f96e 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -67,13 +67,13 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion*
address_space,
return isabus;
}
-void isa_bus_irqs(ISABus *bus, qemu_irq *irqs)
+void isa_bus_register_input_irqs(ISABus *bus, qemu_irq *irqs_in)
{
-bus->irqs = irqs;
+bus->irqs_in = irqs_in;
}
/*
- * isa_get_irq() returns the corresponding qemu_irq entry for the i8259.
+ * isa_get_irq() returns the corresponding input qemu_irq entry for the i8259.
*
* This function is only for special cases such as the 'ferr', and
* temporary use for normal devices until they are converted to qdev.
@@ -82,7 +82,7 @@ qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq)
{
assert(!dev || ISA_BUS(qdev_get_parent_bus(DEVICE(dev))) == isabus);
assert(isairq < ISA_NUM_IRQS);
-return isabus->irqs[isairq];
+return isabus->irqs_in[isairq];
}
void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq)
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 1fba3c210c..cda2f5621e 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -725,7 +725,7 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
qdev_init_gpio_out_named(dev, lpc->gsi, ICH9_GPIO_GSI, GSI_NUM_PINS);
-isa_bus_irqs(isa_bus, lpc->gsi);
+isa_bus_register_input_irqs(isa_bus, lpc->gsi);
i8257_dma_init(isa_bus, 0);
}
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index de60ceef73..ef24826993 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -212,7 +212,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
s->isa = i8259_init(isa_bus, *i8259_out_irq);
/* initialize ISA irqs */
-isa_bus_irqs(isa_bus, s->isa);
+isa_bus_register_input_irqs(isa_bus, s->isa);
/* initialize pit */
i8254_pit_init(isa_bus, 0x40, 0, NULL);
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index a913a509f7..52814cc751 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -615,7 +615,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
}
s->isa_irqs_in = i8259_init(isa_bus, s->cpu_intr);
-isa_bus_irqs(isa_bus, s->isa_irqs_in);
+isa_bus_register_input_irqs(isa_bus, s->isa_irqs_in);
i8254_pit_init(isa_bus, 0x40, 0, NULL);
i8257_dma_init(isa_bus, 0);
diff --git a/hw/mips/ja
[PATCH 0/2] hw/arm/smmu: Fixes for TTB1
Two small changes to support TTB1. Note that I had to modify the Linux
driver in order to test this (see below), but other OSes might use TTB1.
Jean-Philippe Brucker (2):
hw/arm/smmu-common: Support 64-bit addresses
hw/arm/smmu-common: Fix TTB1 handling
hw/arm/smmu-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- 8< --- Linux hacks:
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 8d772ea8a583..bf0ff699b64b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -276,6 +276,11 @@
#define CTXDESC_CD_0_TCR_IRGN0 GENMASK_ULL(9, 8)
#define CTXDESC_CD_0_TCR_ORGN0 GENMASK_ULL(11, 10)
#define CTXDESC_CD_0_TCR_SH0 GENMASK_ULL(13, 12)
+#define CTXDESC_CD_0_TCR_T1SZ GENMASK_ULL(21, 16)
+#define CTXDESC_CD_0_TCR_TG1 GENMASK_ULL(23, 22)
+#define CTXDESC_CD_0_TCR_IRGN1 GENMASK_ULL(25, 24)
+#define CTXDESC_CD_0_TCR_ORGN1 GENMASK_ULL(27, 26)
+#define CTXDESC_CD_0_TCR_SH1 GENMASK_ULL(29, 28)
#define CTXDESC_CD_0_TCR_EPD0 (1ULL << 14)
#define CTXDESC_CD_0_TCR_EPD1 (1ULL << 30)
@@ -293,6 +298,7 @@
#define CTXDESC_CD_0_ASID GENMASK_ULL(63, 48)
#define CTXDESC_CD_1_TTB0_MASK GENMASK_ULL(51, 4)
+#define CTXDESC_CD_2_TTB1_MASK GENMASK_ULL(51, 4)
/*
* When the SMMU only supports linear context descriptor tables, pick a
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index f2425b0f0cd6..3a4343e60a54 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1075,8 +1075,8 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain
*smmu_domain, int ssid,
* this substream's traffic
*/
} else { /* (1) and (2) */
- cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
- cdptr[2] = 0;
+ cdptr[1] = 0;
+ cdptr[2] = cpu_to_le64(cd->ttbr & CTXDESC_CD_2_TTB1_MASK);
cdptr[3] = cpu_to_le64(cd->mair);
/*
@@ -2108,13 +2108,13 @@ static int arm_smmu_domain_finalise_s1(struct
arm_smmu_domain *smmu_domain,
cfg->cd.asid= (u16)asid;
cfg->cd.ttbr= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
- cfg->cd.tcr = FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) |
- FIELD_PREP(CTXDESC_CD_0_TCR_TG0, tcr->tg) |
- FIELD_PREP(CTXDESC_CD_0_TCR_IRGN0, tcr->irgn) |
- FIELD_PREP(CTXDESC_CD_0_TCR_ORGN0, tcr->orgn) |
- FIELD_PREP(CTXDESC_CD_0_TCR_SH0, tcr->sh) |
+ cfg->cd.tcr = FIELD_PREP(CTXDESC_CD_0_TCR_T1SZ, tcr->tsz) |
+ FIELD_PREP(CTXDESC_CD_0_TCR_TG1, tcr->tg) |
+ FIELD_PREP(CTXDESC_CD_0_TCR_IRGN1, tcr->irgn) |
+ FIELD_PREP(CTXDESC_CD_0_TCR_ORGN1, tcr->orgn) |
+ FIELD_PREP(CTXDESC_CD_0_TCR_SH1, tcr->sh) |
FIELD_PREP(CTXDESC_CD_0_TCR_IPS, tcr->ips) |
- CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64;
+ CTXDESC_CD_0_TCR_EPD0 | CTXDESC_CD_0_AA64;
cfg->cd.mair= pgtbl_cfg->arm_lpae_s1_cfg.mair;
/*
@@ -2212,6 +2212,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain
*domain,
.pgsize_bitmap = smmu->pgsize_bitmap,
.ias= ias,
.oas= oas,
+ .quirks = IO_PGTABLE_QUIRK_ARM_TTBR1,
.coherent_walk = smmu->features & ARM_SMMU_FEAT_COHERENCY,
.tlb= &arm_smmu_flush_ops,
.iommu_dev = smmu->dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 38434203bf04..3fe154c9782d 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -677,6 +677,10 @@ static int dma_info_to_prot(enum dma_data_direction dir,
bool coherent,
}
}
+/* HACK */
+#define VA_SIZE39
+#define VA_MASK(~0ULL << VA_SIZE)
+
static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain,
size_t size, u64 dma_limit, struct device *dev)
{
@@ -706,7 +710,7 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain
*domain,
iova = alloc_iova_fast(iovad, iova_len, dma_limit >> shift,
true);
- return (dma_addr_t)iova << shift;
+ return (dma_addr_t)iova << shift | VA_MASK;
}
static void iommu_dma_free_iova(struct iommu_dma_cookie *cookie,
@@ -714,6 +718,7 @@ static void iommu_dma_free_iova(struct iommu_dma_cookie
*cookie,
{
struct iova_domain *iovad = &cookie->iovad;
+ iova &= ~VA_MASK;
/* The MSI case is only ever cleaning up its most recent allo
[PATCH 1/2] hw/arm/smmu-common: Support 64-bit addresses
Addresses targeting the second translation table (TTB1) in the SMMU have all upper bits set. Ensure the IOMMU region covers all 64 bits. Signed-off-by: Jean-Philippe Brucker --- hw/arm/smmu-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index 733c964778..2b8c67b9a1 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -439,7 +439,7 @@ static AddressSpace *smmu_find_add_as(PCIBus *bus, void *opaque, int devfn) memory_region_init_iommu(&sdev->iommu, sizeof(sdev->iommu), s->mrtypename, - OBJECT(s), name, 1ULL << SMMU_MAX_VA_BITS); + OBJECT(s), name, UINT64_MAX); address_space_init(&sdev->as, MEMORY_REGION(&sdev->iommu), name); trace_smmu_add_mr(name); -- 2.39.0
[PATCH 10/11] hw/isa: Use isa_address_space_io() to reduce access on global 'isabus'
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/isa-bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index d19826f96e..d12973103f 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -114,7 +114,7 @@ static inline void isa_init_ioport(ISADevice *dev, uint16_t
ioport)
void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start)
{
-memory_region_add_subregion(isabus->address_space_io, start, io);
+memory_region_add_subregion(isa_address_space_io(dev), start, io);
isa_init_ioport(dev, start);
}
@@ -133,7 +133,7 @@ int isa_register_portio_list(ISADevice *dev,
isa_init_ioport(dev, start);
portio_list_register(piolist, OBJECT(dev), pio_start, opaque, name,
- isabus->address_space_io, start);
+ isa_address_space_io(dev), start);
return 0;
}
--
2.38.1
[PATCH 06/11] hw/sparc64/sun4u: Keep reference to ISA input IRQs in EbusState
Keep reference to ISA input IRQs in EbusState.
To emphasize input/output distinction, rename arrays
as isa_irqs_in / isa_irqs_out.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sparc64/sun4u.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 387181ff77..8fe47e2c22 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -84,7 +84,8 @@ struct EbusState {
PCIDevice parent_obj;
ISABus *isa_bus;
-qemu_irq isa_bus_irqs[ISA_NUM_IRQS];
+qemu_irq *isa_irqs_in;
+qemu_irq isa_irqs_out[ISA_NUM_IRQS];
uint64_t console_serial_base;
MemoryRegion bar0;
MemoryRegion bar1;
@@ -287,7 +288,7 @@ static const TypeInfo power_info = {
static void ebus_isa_irq_handler(void *opaque, int n, int level)
{
EbusState *s = EBUS(opaque);
-qemu_irq irq = s->isa_bus_irqs[n];
+qemu_irq irq = s->isa_irqs_out[n];
/* Pass ISA bus IRQs onto their gpio equivalent */
trace_ebus_isa_irq_handler(n, level);
@@ -303,7 +304,6 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
ISADevice *isa_dev;
SysBusDevice *sbd;
DeviceState *dev;
-qemu_irq *isa_irq;
DriveInfo *fd[MAX_FD];
int i;
@@ -315,9 +315,9 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
}
/* ISA bus */
-isa_irq = qemu_allocate_irqs(ebus_isa_irq_handler, s, ISA_NUM_IRQS);
-isa_bus_irqs(s->isa_bus, isa_irq);
-qdev_init_gpio_out_named(DEVICE(s), s->isa_bus_irqs, "isa-irq",
+s->isa_irqs_in = qemu_allocate_irqs(ebus_isa_irq_handler, s, ISA_NUM_IRQS);
+isa_bus_irqs(s->isa_bus, s->isa_irqs_in);
+qdev_init_gpio_out_named(DEVICE(s), s->isa_irqs_out, "isa-irq",
ISA_NUM_IRQS);
/* Serial ports */
--
2.38.1
[PATCH 05/11] hw/mips/jazz: Rename ISA input IRQs as 'isa_irqs_in'
The following code: /* ISA devices */ i8259 = i8259_init(isa_bus, ...); gives the false idea that the function is creating a i8259 device. Instead this function returns an array of input IRQs. Rename the variable to clarify: /* ISA devices */ isa_irqs_in = i8259_init(isa_bus, ...); Signed-off-by: Philippe Mathieu-Daudé --- hw/mips/jazz.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c index 6aefe9a61b..fc7898006c 100644 --- a/hw/mips/jazz.c +++ b/hw/mips/jazz.c @@ -130,7 +130,7 @@ static void mips_jazz_init(MachineState *machine, MIPSCPU *cpu; MIPSCPUClass *mcc; CPUMIPSState *env; -qemu_irq *i8259; +qemu_irq *isa_irqs_in; rc4030_dma *dmas; IOMMUMemoryRegion *rc4030_dma_mr; MemoryRegion *isa_mem = g_new(MemoryRegion, 1); @@ -248,8 +248,8 @@ static void mips_jazz_init(MachineState *machine, isa_bus = isa_bus_new(NULL, isa_mem, isa_io, &error_abort); /* ISA devices */ -i8259 = i8259_init(isa_bus, env->irq[4]); -isa_bus_irqs(isa_bus, i8259); +isa_irqs_in = i8259_init(isa_bus, env->irq[4]); +isa_bus_irqs(isa_bus, isa_irqs_in); i8257_dma_init(isa_bus, 0); pit = i8254_pit_init(isa_bus, 0x40, 0, NULL); pcspk_init(isa_new(TYPE_PC_SPEAKER), isa_bus, pit); -- 2.38.1
Re: [PATCH v3 06/14] io: add and implement QIO_CHANNEL_FEATURE_SEEKABLE for channel file
On Fri, Oct 28, 2022 at 01:39:06PM +0300, Nikolay Borisov wrote:
> Add a generic QIOChannel feature SEEKABLE which would be used by the
> qemu_file* apis. For the time being this will be only implemented for
> file channels.
>
> Signed-off-by: Nikolay Borisov
> ---
> include/io/channel.h | 1 +
> io/channel-file.c| 9 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/include/io/channel.h b/include/io/channel.h
> index 6b10bce8bbdf..b645989e467c 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -41,6 +41,7 @@ enum QIOChannelFeature {
> QIO_CHANNEL_FEATURE_SHUTDOWN,
> QIO_CHANNEL_FEATURE_LISTEN,
> QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY,
> +QIO_CHANNEL_FEATURE_SEEKABLE,
> };
>
>
> diff --git a/io/channel-file.c b/io/channel-file.c
> index a7a90c12dc2b..e213a0fd7cd2 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -35,6 +35,11 @@ qio_channel_file_new_fd(int fd)
>
> ioc->fd = fd;
>
> +if (lseek(fd, 0, SEEK_CUR) != (off_t)-1) {
> +qio_channel_set_feature(QIO_CHANNEL(ioc),
> QIO_CHANNEL_FEATURE_SEEKABLE);
> +}
> +
> +qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
This second qio_channel_set_feature call is a rebase mistake I
presume. With that removed, the rest of the patch looks fine
Reviewed-by: Daniel P. Berrangé
> trace_qio_channel_file_new_fd(ioc, fd);
>
> return ioc;
> @@ -59,6 +64,10 @@ qio_channel_file_new_path(const char *path,
> return NULL;
> }
>
> +if (lseek(ioc->fd, 0, SEEK_CUR) != (off_t)-1) {
> +qio_channel_set_feature(QIO_CHANNEL(ioc),
> QIO_CHANNEL_FEATURE_SEEKABLE);
> +}
> +
> trace_qio_channel_file_new_path(ioc, path, flags, mode, ioc->fd);
>
> return ioc;
> --
> 2.34.1
>
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 :|
[PATCH 04/11] hw/isa/vt82c686: Remove intermediate IRQ forwarder
From: Philippe Mathieu-Daudé
Directly dispatch ISA IRQs to 'cpu_intr' output IRQ
by removing the intermediate via_isa_request_i8259_irq()
handler. Rename ISA IRQs array as 'isa_irqs_in' to
emphasize these are input IRQs.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/vt82c686.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 3f9bd0c04d..a913a509f7 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -548,7 +548,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(ViaISAState, VIA_ISA)
struct ViaISAState {
PCIDevice dev;
qemu_irq cpu_intr;
-qemu_irq *isa_irqs;
+qemu_irq *isa_irqs_in;
ViaSuperIOState via_sio;
RTCState rtc;
PCIIDEState ide;
@@ -595,13 +595,7 @@ static const TypeInfo via_isa_info = {
void via_isa_set_irq(PCIDevice *d, int n, int level)
{
ViaISAState *s = VIA_ISA(d);
-qemu_set_irq(s->isa_irqs[n], level);
-}
-
-static void via_isa_request_i8259_irq(void *opaque, int irq, int level)
-{
-ViaISAState *s = opaque;
-qemu_set_irq(s->cpu_intr, level);
+qemu_set_irq(s->isa_irqs_in[n], level);
}
static void via_isa_realize(PCIDevice *d, Error **errp)
@@ -609,12 +603,10 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
ViaISAState *s = VIA_ISA(d);
DeviceState *dev = DEVICE(d);
PCIBus *pci_bus = pci_get_bus(d);
-qemu_irq *isa_irq;
ISABus *isa_bus;
int i;
qdev_init_gpio_out(dev, &s->cpu_intr, 1);
-isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
errp);
@@ -622,8 +614,8 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
return;
}
-s->isa_irqs = i8259_init(isa_bus, *isa_irq);
-isa_bus_irqs(isa_bus, s->isa_irqs);
+s->isa_irqs_in = i8259_init(isa_bus, s->cpu_intr);
+isa_bus_irqs(isa_bus, s->isa_irqs_in);
i8254_pit_init(isa_bus, 0x40, 0, NULL);
i8257_dma_init(isa_bus, 0);
--
2.38.1
[PATCH 07/11] hw/isa: Reorder to separate ISABus* vs ISADevice* functions
Separate functions taking an ISABus* argument versus
functions taking a ISADevice* one.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/isa/isa.h | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 25acd5c34c..e81cd33e3c 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -74,12 +74,8 @@ struct ISADevice {
ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space,
MemoryRegion *address_space_io, Error **errp);
void isa_bus_irqs(ISABus *bus, qemu_irq *irqs);
-qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq);
-void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq);
void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16);
IsaDma *isa_get_dma(ISABus *bus, int nchan);
-MemoryRegion *isa_address_space(ISADevice *dev);
-MemoryRegion *isa_address_space_io(ISADevice *dev);
ISADevice *isa_new(const char *name);
ISADevice *isa_try_new(const char *name);
bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp);
@@ -87,6 +83,11 @@ ISADevice *isa_create_simple(ISABus *bus, const char *name);
ISADevice *isa_vga_init(ISABus *bus);
+qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq);
+void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq);
+MemoryRegion *isa_address_space(ISADevice *dev);
+MemoryRegion *isa_address_space_io(ISADevice *dev);
+
/**
* isa_register_ioport: Install an I/O port region on the ISA bus.
*
--
2.38.1
[PATCH 03/11] hw/isa/i82378: Remove intermediate IRQ forwarder
From: Philippe Mathieu-Daudé
When the i82378 model was added in commit a04ff940974 ("prep:
Add i82378 PCI-to-ISA bridge emulation") the i8259 model was
not yet QOM'ified. This happened later in commit 747c70af78f
("i8259: Convert to qdev").
Directly dispatch ISA IRQs to 'cpu_intr' output IRQ
by removing the intermediate i82378_request_out0_irq()
handler. Rename ISA IRQs array as 'isa_irqs_in' to
emphasize these are input IRQs.
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/i82378.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index 84ce761f5f..d32653369d 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -33,7 +33,7 @@ struct I82378State {
PCIDevice parent_obj;
qemu_irq cpu_intr;
-qemu_irq *i8259;
+qemu_irq *isa_irqs_in;
MemoryRegion io;
};
@@ -47,18 +47,12 @@ static const VMStateDescription vmstate_i82378 = {
},
};
-static void i82378_request_out0_irq(void *opaque, int irq, int level)
-{
-I82378State *s = opaque;
-qemu_set_irq(s->cpu_intr, level);
-}
-
static void i82378_request_pic_irq(void *opaque, int irq, int level)
{
DeviceState *dev = opaque;
I82378State *s = I82378(dev);
-qemu_set_irq(s->i8259[irq], level);
+qemu_set_irq(s->isa_irqs_in[irq], level);
}
static void i82378_realize(PCIDevice *pci, Error **errp)
@@ -94,9 +88,8 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
*/
/* 2 82C59 (irq) */
-s->i8259 = i8259_init(isabus,
- qemu_allocate_irq(i82378_request_out0_irq, s, 0));
-isa_bus_irqs(isabus, s->i8259);
+s->isa_irqs_in = i8259_init(isabus, s->cpu_intr);
+isa_bus_irqs(isabus, s->isa_irqs_in);
/* 1 82C54 (pit) */
pit = i8254_pit_init(isabus, 0x40, 0, NULL);
--
2.38.1
[PATCH 11/11] hw/isa: Factor isa_bus_get_irq() out of isa_get_irq()
isa_get_irq() was added in commit 3a38d437ca
("Add isa_reserve_irq()" Fri Aug 14 11:36:15 2009) as:
a temporary interface to be used to allocate ISA IRQs for
devices which have not yet been converted to qdev, and for
special cases which are not suited for qdev conversions,
such as the 'ferr'.
We still use it 14 years later, using the global 'isabus'
singleton. In order to get rid of such *temporary* interface,
extract isa_bus_get_irq() which can take any ISABus* object.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/isa-bus.c | 14 ++
include/hw/isa/isa.h | 8
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index d12973103f..4cf26510bf 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -72,6 +72,13 @@ void isa_bus_register_input_irqs(ISABus *bus, qemu_irq
*irqs_in)
bus->irqs_in = irqs_in;
}
+qemu_irq isa_bus_get_irq(ISABus *bus, unsigned irqnum)
+{
+assert(irqnum < ISA_NUM_IRQS);
+assert(bus->irqs_in);
+return bus->irqs_in[irqnum];
+}
+
/*
* isa_get_irq() returns the corresponding input qemu_irq entry for the i8259.
*
@@ -81,14 +88,13 @@ void isa_bus_register_input_irqs(ISABus *bus, qemu_irq
*irqs_in)
qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq)
{
assert(!dev || ISA_BUS(qdev_get_parent_bus(DEVICE(dev))) == isabus);
-assert(isairq < ISA_NUM_IRQS);
-return isabus->irqs_in[isairq];
+return isa_bus_get_irq(isabus, isairq);
}
void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq)
{
-qemu_irq irq = isa_get_irq(isadev, isairq);
-qdev_connect_gpio_out(DEVICE(isadev), gpioirq, irq);
+qemu_irq input_irq = isa_get_irq(isadev, isairq);
+qdev_connect_gpio_out(DEVICE(isadev), gpioirq, input_irq);
}
void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16)
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 0aa36d4115..ba62a2e6c8 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -76,6 +76,14 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion
*address_space,
void isa_bus_register_input_irqs(ISABus *bus, qemu_irq *irqs_in);
void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16);
IsaDma *isa_get_dma(ISABus *bus, int nchan);
+/**
+ * isa_bus_get_irq: Return input IRQ on ISA bus.
+ * @bus: the #ISABus to plug ISA devices on.
+ * @irqnum: the ISA IRQ number.
+ *
+ * Return IRQ @irqnum from the PIC associated on ISA @bus.
+ */
+qemu_irq isa_bus_get_irq(ISABus *bus, unsigned irqnum);
ISADevice *isa_new(const char *name);
ISADevice *isa_try_new(const char *name);
bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp);
--
2.38.1
[PATCH 08/11] hw/isa: Un-inline isa_bus_from_device()
No point in inlining isa_bus_from_device() which is only
used at device realization time.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/isa-bus.c | 5 +
include/hw/isa/isa.h | 6 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 4fe61d6dfe..5bd99379e9 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -162,6 +162,11 @@ bool isa_realize_and_unref(ISADevice *dev, ISABus *bus,
Error **errp)
return qdev_realize_and_unref(&dev->parent_obj, &bus->parent_obj, errp);
}
+ISABus *isa_bus_from_device(ISADevice *dev)
+{
+return ISA_BUS(qdev_get_parent_bus(DEVICE(dev)));
+}
+
ISADevice *isa_vga_init(ISABus *bus)
{
vga_interface_created = true;
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index e81cd33e3c..1691364011 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -87,6 +87,7 @@ qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq);
void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, unsigned isairq);
MemoryRegion *isa_address_space(ISADevice *dev);
MemoryRegion *isa_address_space_io(ISADevice *dev);
+ISABus *isa_bus_from_device(ISADevice *dev);
/**
* isa_register_ioport: Install an I/O port region on the ISA bus.
@@ -124,9 +125,4 @@ int isa_register_portio_list(ISADevice *dev,
const MemoryRegionPortio *portio,
void *opaque, const char *name);
-static inline ISABus *isa_bus_from_device(ISADevice *d)
-{
-return ISA_BUS(qdev_get_parent_bus(DEVICE(d)));
-}
-
#endif
--
2.38.1
[PATCH 00/11] hw/isa: More housekeeping
Trying to clarify the ISA API. Most patches should be trivial enough, so not much to describe here :) Philippe Mathieu-Daudé (11): hw/intc/i8259: Document i8259_init() hw/isa/i82378: Rename output IRQ as 'cpu_intr' hw/isa/i82378: Remove intermediate IRQ forwarder hw/isa/vt82c686: Remove intermediate IRQ forwarder hw/mips/jazz: Rename ISA input IRQs as 'isa_irqs_in' hw/sparc64/sun4u: Keep reference to ISA input IRQs in EbusState hw/isa: Reorder to separate ISABus* vs ISADevice* functions hw/isa: Un-inline isa_bus_from_device() hw/isa: Rename isa_bus_irqs() -> isa_bus_register_input_irqs() hw/isa: Use isa_address_space_io() to reduce access on global 'isabus' hw/isa: Factor isa_bus_get_irq() out of isa_get_irq() hw/hppa/machine.c | 2 +- hw/i386/microvm.c | 2 +- hw/i386/pc_piix.c | 2 +- hw/intc/i8259.c | 4 ++-- hw/isa/i82378.c | 19 ++- hw/isa/isa-bus.c| 29 - hw/isa/lpc_ich9.c | 2 +- hw/isa/piix4.c | 2 +- hw/isa/vt82c686.c | 16 hw/mips/jazz.c | 6 +++--- hw/ppc/pnv_lpc.c| 2 +- hw/sparc64/sun4u.c | 12 ++-- include/hw/intc/i8259.h | 10 +- include/hw/isa/isa.h| 27 --- 14 files changed, 72 insertions(+), 63 deletions(-) -- 2.38.1
[PATCH 01/11] hw/intc/i8259: Document i8259_init()
i8259_init() helper creates a i8259 device on an ISA bus,
connects its IRQ output to the parent's input IRQ, and
returns an array of 16 ISA input IRQs.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/intc/i8259.c | 4 ++--
include/hw/intc/i8259.h | 10 +-
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
index 0261f087b2..17910f3bcb 100644
--- a/hw/intc/i8259.c
+++ b/hw/intc/i8259.c
@@ -406,7 +406,7 @@ static void pic_realize(DeviceState *dev, Error **errp)
pc->parent_realize(dev, errp);
}
-qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
+qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq_in)
{
qemu_irq *irq_set;
DeviceState *dev;
@@ -418,7 +418,7 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
isadev = i8259_init_chip(TYPE_I8259, bus, true);
dev = DEVICE(isadev);
-qdev_connect_gpio_out(dev, 0, parent_irq);
+qdev_connect_gpio_out(dev, 0, parent_irq_in);
for (i = 0 ; i < 8; i++) {
irq_set[i] = qdev_get_gpio_in(dev, i);
}
diff --git a/include/hw/intc/i8259.h b/include/hw/intc/i8259.h
index a0e34dd990..c412575775 100644
--- a/include/hw/intc/i8259.h
+++ b/include/hw/intc/i8259.h
@@ -4,7 +4,15 @@
/* i8259.c */
extern PICCommonState *isa_pic;
-qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq);
+
+/*
+ * i8259_init()
+ *
+ * Create a i8259 device on an ISA @bus,
+ * connect its output to @parent_irq_in,
+ * return an (allocated) array of 16 input IRQs.
+ */
+qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq_in);
qemu_irq *kvm_i8259_init(ISABus *bus);
int pic_get_output(PICCommonState *s);
int pic_read_irq(PICCommonState *s);
--
2.38.1
[PATCH 02/11] hw/isa/i82378: Rename output IRQ as 'cpu_intr'
Commit a04ff94097 ("prep: Add i82378 PCI-to-ISA bridge
emulation") aimed to model the 2 output IRQs: CPU intr
and NMI. Commit 5039d6e235 ("i8257: remove cpu_request_exit
irq") removed the NMI IRQ.
Since this model only use the CPU interrupt, replace the
'out[2]' array by a single 'cpu_intr'.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/i82378.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index e3322e03bf..84ce761f5f 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -32,7 +32,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(I82378State, I82378)
struct I82378State {
PCIDevice parent_obj;
-qemu_irq out[2];
+qemu_irq cpu_intr;
qemu_irq *i8259;
MemoryRegion io;
};
@@ -50,7 +50,7 @@ static const VMStateDescription vmstate_i82378 = {
static void i82378_request_out0_irq(void *opaque, int irq, int level)
{
I82378State *s = opaque;
-qemu_set_irq(s->out[0], level);
+qemu_set_irq(s->cpu_intr, level);
}
static void i82378_request_pic_irq(void *opaque, int irq, int level)
@@ -113,7 +113,7 @@ static void i82378_init(Object *obj)
DeviceState *dev = DEVICE(obj);
I82378State *s = I82378(obj);
-qdev_init_gpio_out(dev, s->out, 1);
+qdev_init_gpio_out(dev, &s->cpu_intr, 1);
qdev_init_gpio_in(dev, i82378_request_pic_irq, 16);
}
--
2.38.1
Re: [PATCH 5/7] hw/ide/piix: Use generic ide_init_ioport()
Am 9. Februar 2023 09:04:49 UTC schrieb Bernhard Beschow :
>On Wed, Feb 8, 2023 at 1:08 AM Philippe Mathieu-Daudé
>wrote:
>
>> TYPE_PIIX3_IDE is a PCI function inheriting from QOM
>> TYPE_PCI_DEVICE. To be able to call the ISA specific
>> ide_init_ioport_isa(), we call this function passing
>> a NULL ISADevice argument. Remove this hack by calling
>> the recently added generic ide_init_ioport(), which
>> doesn't expect any ISADevice.
>>
>> Inspired-by: Bernhard Beschow
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> hw/ide/piix.c | 10 --
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
>> index a587541bb2..1cd4389611 100644
>> --- a/hw/ide/piix.c
>> +++ b/hw/ide/piix.c
>> @@ -136,15 +136,13 @@ static int pci_piix_init_ports(PCIIDEState *d)
>> {0x1f0, 0x3f6, 14},
>> {0x170, 0x376, 15},
>> };
>> -int i, ret;
>> +int i;
>>
>> for (i = 0; i < 2; i++) {
>> ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
>> -ret = ide_init_ioport_isa(&d->bus[i], NULL,
>> - port_info[i].iobase,
>> port_info[i].iobase2);
>> -if (ret) {
>> -return ret;
>> -}
>> +ide_init_ioport(&d->bus[i], OBJECT(d),
>> +pci_address_space_io(PCI_DEVICE(d)),
>> +port_info[i].iobase, port_info[i].iobase2);
>> ide_init2(&d->bus[i], isa_get_irq(NULL, port_info[i].isairq));
Let me elaborete a bit on what I mean by the patch essentially circumventing
the crash fix:
The reason for the crash with the x-remote machine is now caused by
isa_get_irq() which also uses the isabus global behind the scenes. So piix-ide
needs to be changed in two places to avoid the global usage and hence the crash.
In his crash fix [1], Thomas was lucky: First, ide_init_ioport() didn't return
a value before, so adding one didn't cause changes in other device models.
Second, ide_init_ioport() is the first call here to access the global, so it
could be used to protect the call to isa_get_irq(). Note that isa_get_irq()
couldn't be changed in a similar way without affecting all its call sites.
Fixing ide_init_ioport() to not access the global is certainly a step in the
right direction, but this means that ide_init_ioport() is now unable to protect
the isa_get_irq() call. Since isa_get_irq() can't conveniently protect itself,
we either need to avoid it or need another way to achieve that. That's why in
my series GPIOs are used for internal devices and isa_get_irq() plus fishing
out the ISA bus for user-created ones.
Fishing out the ISA bus is still a hack IMO, for two reasons: First, IIUC,
QOM'ified devices shall only care about its children while looking up one's
parent bus violates this rule. Second, using the global machine pointer to scan
for the ISA bus just trades one global for another. That's why I'm only doing
this for user-created instances. If we deprecated user-created piix IDE devices
we could eventually get rid of this hack.
Best regards,
Bernhard
[1]
https://lore.kernel.org/qemu-devel/20210416125256.2039734-1-th...@redhat.com/
"hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine"
>>
>> bmdma_init(&d->bus[i], &d->bmdma[i], d);
>> --
>> 2.38.1
>>
>> This patch essentially circumvents the mitigations introduced by
>https://lore.kernel.org/qemu-devel/20210416125256.2039734-1-th...@redhat.com/
>"hw/ide: Fix crash when plugging a piix3-ide device into the x-remote
>machine": `qemu-system-x86_64 -M x-remote -device piix3-ide` now crashes.
>This has been considered in
>https://lore.kernel.org/qemu-devel/20230126211740.66874-1-shen...@gmail.com/
>-- see cover letter there. TBH it's not entirely clear to me why we need
>two competing series here at all.
>
>Best regards,
>Bernhard
Re: [PATCH v3 05/14] io: implement io_pwritev for QIOChannelFile
On Fri, Oct 28, 2022 at 01:39:05PM +0300, Nikolay Borisov wrote: > The upcoming 'fixed-ram' feature would require qemu to write data at > specific offsets of the file. Add a minimal implementation of pwritev > and expose it via the io_pwritev interface. > > Signed-off-by: Nikolay Borisov > --- > io/channel-file.c | 25 + > 1 file changed, 25 insertions(+) Reviewed-by: Daniel P. Berrangé 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 :|
Re: [PATCH v2 6/7] CI: Stop building docs on centos8
On Fri, 10 Feb 2023 at 16:01, John Snow wrote: > On Fri, Feb 10, 2023, 5:41 AM Peter Maydell wrote: >> On Fri, 10 Feb 2023 at 00:31, John Snow wrote: >> This confuses me. We work fine with Python 3.6 today. > > > That won't last - Many tools such as mypy, pylint and flake8 which I use to > manage our python codebase have been dropping support for 3.6 and I've had to > implement an increasing number of workarounds to help keep it possible to > test 3.6 via CI while also ensuring our newest platforms work as dev > environments. Something somewhere seems kind of out-of-sync here. Either: * Python is deprecating old versions too quickly and dropping support for them too fast * CentOS is retaining old versions of Python when it needs to ship newer ones * Our policy for what distros we support is overly lax and causing us to try to support ancient platforms that we shouldn't be trying to support because "use the system version of foo" should not be a big deal -- it's not like we're trying to support decades-old hosts here: Centos 8 was released in 2019, which is less than five years ago. > The argument I'm making is: > > - CentOS 8 is a supported build platform > - All platforms *do* have a Python modern enough to allow us to drop 3.6 > - CentOS's repo version of sphinx is hardcoded to use the older 3.6, though > - You expressed a preference to me in the past to NOT use a pip installed > version of sphinx in preference to the system version in "configure" > - It's still possible to build docs on CentOS 8 after this patchset, you just > need a pip version. > - We've used the justification that our build platform promise does not > necessarily extend to docs and tests in the past. > - So just skip docs building for CentOS 8, only in the CI. > > If you believe docs in CI for CentOS 8 is a hard deal breaker, then I want to > go back to discussing the possibility of using sphinx versions from pip. If Python 3.6 is EOL, shouldn't CentOS have shipped an updated version of Sphinx to go with their updated Python ? I'm not super-happy about dropping the docs build requirement, but I guess it's probably the least-worst choice. thanks -- PMM
Re: [PATCH v6 00/33] Consolidate PIIX south bridges
Am 23. Januar 2023 15:51:49 UTC schrieb Bernhard Beschow : > > >Am 23. Januar 2023 09:25:51 UTC schrieb "Philippe Mathieu-Daudé" >: >>On 20/1/23 13:22, Bernhard Beschow wrote: >>> Am 13. Januar 2023 17:39:45 UTC schrieb Bernhard Beschow >>> : Am 13. Januar 2023 08:46:53 UTC schrieb "Philippe Mathieu-Daudé" : > On 9/1/23 18:23, Bernhard Beschow wrote: >> This series consolidates the implementations of the PIIX3 and PIIX4 south >> bridges and is an extended version of [1]. The motivation is to share as >> much >> code as possible and to bring both device models to feature parity such >> that >> perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc >> machine. This >> could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on >> this >> list before. > >> Bernhard Beschow (30): >> hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs() >> hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific >> hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific >> hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig >> hw/usb/hcd-uhci: Introduce TYPE_ defines for device models >> hw/intc/i8259: Make using the isa_pic singleton more type-safe >> hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC >> hw/i386/pc: Create RTC controllers in south bridges >> hw/i386/pc: No need for rtc_state to be an out-parameter >> hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 >> south bridge >> hw/isa/piix3: Create USB controller in host device >> hw/isa/piix3: Create power management controller in host device >> hw/isa/piix3: Create TYPE_ISA_PIC in host device >> hw/isa/piix3: Create IDE controller in host device >> hw/isa/piix3: Wire up ACPI interrupt internally >> hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS >> hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4 >> hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4 >> hw/isa/piix3: Drop the "3" from PIIX base class >> hw/isa/piix4: Make PIIX4's ACPI and USB functions optional >> hw/isa/piix4: Remove unused inbound ISA interrupt lines >> hw/isa/piix4: Use TYPE_ISA_PIC device >> hw/isa/piix4: Reuse struct PIIXState from PIIX3 >> hw/isa/piix4: Rename reset control operations to match PIIX3 >> hw/isa/piix3: Merge hw/isa/piix4.c >> hw/isa/piix: Harmonize names of reset control memory regions >> hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 >> hw/isa/piix: Rename functions to be shared for interrupt triggering >> hw/isa/piix: Consolidate IRQ triggering >> hw/isa/piix: Share PIIX3's base class with PIIX4 >> >> Philippe Mathieu-Daudé (3): >> hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition >> hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader >> hw/isa/piix4: Correct IRQRC[A:D] reset values > > I'm queuing the first 10 patches for now to alleviate the size of this > series, and I'll respin a v7 with the rest to avoid making you suffer > any longer :/ Thanks for insisting in this effort and I apologize it > is taking me so long... Okay... What's the further plan? Is there anything missing? >>> >>> Ping >> >>The plan is "I'll respin a v7 with the rest". > >I understood that part. I wonder what the blocking issue is/was. The first part of this series contains piix3 changes such as the ISA proxy pic and movement of rtc. This seems the riskier part of the series to me which I'd like to get feedback on from the field rather sooner than later. The reason is that I can't currently forsee how fast I could react if these changes were merged during (soft) freeze. Is there a possibility to at least get the piix3 part merged already? Maybe perhaps via the pc tree? Thanks, Bernhard > >Best regards, >Bernhard
Re: [PATCH v3 4/7] qapi/expr: add typing workaround for AbstractSet
On Fri, Feb 10, 2023, 10:44 AM Markus Armbruster wrote:
> John Snow writes:
>
> > mypy can only narrow the type of `Mapping[str, ...].keys() & Set[str]`
> > to `AbstractSet[str]` and not a `Set[str]`. As a result, if the type of
> > an expression is changed to a Mapping[], mypy is unsure if the .pop() is
> > safe.
> >
> > A forthcoming commit does exactly that, so wrap the expression in a
> > set() constructor to force the intermediate expression to be resolved as
> > a mutable type.
> >
> > Signed-off-by: John Snow
> > ---
> > scripts/qapi/expr.py | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> > index b56353bdf84..af802367eff 100644
> > --- a/scripts/qapi/expr.py
> > +++ b/scripts/qapi/expr.py
> > @@ -622,8 +622,8 @@ def check_expr(expr_elem: _JSONObject) -> None:
> > if 'include' in expr:
> > return
> >
> > -metas = expr.keys() & {'enum', 'struct', 'union', 'alternate',
> > - 'command', 'event'}
> > +metas = set(expr.keys() & {
> > +'enum', 'struct', 'union', 'alternate', 'command', 'event'})
> > if len(metas) != 1:
> > raise QAPISemError(
> > info,
>"expression must have exactly one key"
>" 'enum', 'struct', 'union', 'alternate',"
>" 'command', 'event'")
>meta = metas.pop()
>
> I'm mildly surprised that the result of operator & is considered
> immutable. How could it *not* be a freshly created set? Oh well.
>
I think because it's up to the LHS type to return the result.
Wait, maybe I can just switch the operand order, lol.
> Passing a set to set() is a no-op, so
>
> Reviewed-by: Markus Armbruster
>
> Regardless, the code feels a bit too clever to me. It actually did
> already when I wrote it, but I the less clever ways I could think of
> were also much more verbose.
>
> The code checks that exactly one of a set of keys is present, and which
> one it is. Any ideas?
>
Yeah, it feels too clever but I'm not sure I know something more elegant,
really. I'll consider it while I address your feedback and prepare a respin.
>
Re: [PATCH v2 3/7] configure: Look for auxiliary Python installations
On Fri, Feb 10, 2023 at 5:22 PM John Snow wrote: >> Though part of me thinks that your new loop is slightly overengineered >> and we should just require /usr/bin/env python3 and call it a day. > > Well, but that'd be a problem for CentOS 8, wouldn't it? python3 is gonna > resolve to python3.6. The user would have to specify --python=/usr/bin/python3.8, or could also set up "alternatives" so that python3 resolves to modular Python (3.8 or newer). I think it's a fair requirement for users of enterprise distributions, and it works because it forces usage of QEMU's meson submodule. My lcitool update does the former by placing ENV PYTHON "/usr/bin/python3.8" in the Dockerfile. Paolo
Re: [PATCH v3 04/14] io: Add generic pwritev/preadv interface
On Fri, Oct 28, 2022 at 01:39:04PM +0300, Nikolay Borisov wrote:
> Introduce basic pwriteve/preadv support in the generic channel layer.
> SPecific implementation will follow for the file channel as this is
> required in order to support migration streams with fixed location of
> each ram page.
>
> Signed-off-by: Nikolay Borisov
> ---
> include/io/channel.h | 49
> io/channel.c | 26 +++
> 2 files changed, 75 insertions(+)
>
> diff --git a/include/io/channel.h b/include/io/channel.h
> index c680ee748021..6b10bce8bbdf 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -124,6 +124,16 @@ struct QIOChannelClass {
> Error **errp);
>
> /* Optional callbacks */
> +ssize_t (*io_pwritev)(QIOChannel *ioc,
> + const struct iovec *iov,
> + size_t niov,
> + off_t offset,
> + Error **errp);
> +ssize_t (*io_preadv)(QIOChannel *ioc,
> + const struct iovec *iov,
> + size_t niov,
> + off_t offset,
> + Error **errp);
nit-pick the alignment of the 2nd line of parameters onwards,
needs to be indented by 3 more spaces.
> +/**
> + * qio_channel_io_pwritev
> + * @ioc: the channel object
> + * @iov: the array of memory regions to write data from
> + * @niov: the length of the @iov array
> + * @offset: offset in the channel where writes should begin
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Not all implementations will support this facility, so may report an
> error.
> + * To avoid errors, the caller may check for the feature flag
> + * QIO_CHANNEL_FEATURE_SEEKABLE prior to calling this method.
> + *
> + * Behaves as qio_channel_writev_full, apart from not supporting sending of
> file
Given this, we should have "_full" suffix on these methods
too for consistent naming policy.
> + * handles as well as beginning the write at the passed @offset
> + *
> + */
> +ssize_t qio_channel_io_pwritev(QIOChannel *ioc, const struct iovec *iov,
> + size_t niov, off_t offset, Error **errp);
> +
> +
> +/**
> + * qio_channel_io_preadv
> + * @ioc: the channel object
> + * @iov: the array of memory regions to read data into
> + * @niov: the length of the @iov array
> + * @offset: offset in the channel where writes should begin
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Not all implementations will support this facility, so may report an
> error.
> + * To avoid errors, the caller may check for the feature flag
> + * QIO_CHANNEL_FEATURE_SEEKABLE prior to calling this method.
> + *
> + * Behaves as qio_channel_readv_full, apart from not supporting receiving of
> file
> + * handles as well as beginning the read at the passed @offset
> + *
> + */
> +ssize_t qio_channel_io_preadv(QIOChannel *ioc, const struct iovec *iov,
> + size_t niov, off_t offset, Error **errp);
> /**
> * qio_channel_shutdown:
> * @ioc: the channel object
> diff --git a/io/channel.c b/io/channel.c
> index 0640941ac573..f5ac9499a7ad 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -437,6 +437,32 @@ GSource *qio_channel_add_watch_source(QIOChannel *ioc,
> }
>
>
> +ssize_t qio_channel_io_pwritev(QIOChannel *ioc, const struct iovec *iov,
> + size_t niov, off_t offset, Error **errp)
> +{
> +QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> +
> +if (!klass->io_pwritev) {
> +error_setg(errp, "Channel does not support pwritev");
> +return -1;
> +}
This also possibly benefits from a validation check
if (!qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_SEEKABLE)) {
error_setg_errno(errp, EINVAL,
"Requested channel is not seekable")
return -1;
}
because the QIOChannelFile impl will support io_pwritev callback,
but not all the FDs it can use will support seeking.
Same check for preadv
> +
> +return klass->io_pwritev(ioc, iov, niov, offset, errp);
> +}
> +
> +ssize_t qio_channel_io_preadv(QIOChannel *ioc, const struct iovec *iov,
> + size_t niov, off_t offset, Error **errp)
> +{
> +QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> +
> +if (!klass->io_preadv) {
> +error_setg(errp, "Channel does not support preadv");
> +return -1;
> +}
> +
> +return klass->io_preadv(ioc, iov, niov, offset, errp);
> +}
> +
> int qio_channel_shutdown(QIOChannel *ioc,
> QIOChannelShutdown how,
> Error **errp)
> --
> 2.34.1
>
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-h
Re: [PATCH v2 3/7] configure: Look for auxiliary Python installations
On Fri, Feb 10, 2023, 11:17 AM Paolo Bonzini wrote:
> On Fri, Feb 10, 2023 at 4:28 PM John Snow wrote:
> > PS, while you're here, how does this new loop interfere with your
> "custom python specified" flag for meson? I think meson uses the version of
> python *it* detects and not the configure script identified one, right?
> Does that mean that e.g. the qapi generator gets run with the system
> default/meson version and not the config version?
>
> Yes, if neither --python nor --meson are specified, then it could
> happen that a different python is used during ninja's execution vs.
> what is used for "other stuff" (docker cross compilers and other
> Makefile invocations of $(PYTHON)).
>
> The meson version of Python is guaranteed to be at least 3.7 as soon
> as we update to 0.63.x (which will be Real Soon Now), but it's ugly.
> The main issue I anticipate could be a problem when running from a
> virtual environment, so perhaps we can force usage of the internal
> meson if neither --python nor --meson are specified, and VIRTUAL_ENV
> is set and $VIRTUAL_ENV/bin/meson does not exist?
>
> diff --git a/configure b/configure
> index 06bcd9031903..001a79a90170 100755
> --- a/configure
> +++ b/configure
> @@ -870,8 +870,18 @@ fi
> # Suppress writing compiled files
> python="$python -B"
>
> +has_meson() {
> + if test "${VIRTUAL_ENV:+set}" = set; then
> +# Ensure that Meson and Python come from the same virtual environment
> +test -x "$(VIRTUAL_ENV}/bin/meson" &&
> + test "$(command -v meson)" -ef "$(VIRTUAL_ENV}/bin/meson"
> + else
> +has meson
> + fi
> +}
> +
> if test -z "$meson"; then
> -if test "$explicit_python" = no && has meson && version_ge
> "$(meson --version)" 0.63.0; then
> +if test "$explicit_python" = no && has_meson && version_ge
> "$(meson --version)" 0.63.0; then
> meson=meson
> elif test "$git_submodules_action" != 'ignore' ; then
> meson=git
>
> I will include it when posting the final series.
>
> > Do I need to adjust this loop to consider more binaries as "explicitly
> specified"?
>
> I don't think it's a huge problem. Outside virtual environments, the
> most likely setting is that Meson uses python3 which in turn is the
> most recent python3.X, so it should be fine overall.
>
> Though part of me thinks that your new loop is slightly overengineered
> and we should just require /usr/bin/env python3 and call it a day.
>
Well, but that'd be a problem for CentOS 8, wouldn't it? python3 is gonna
resolve to python3.6.
> Paolo
>
>
Re: [PATCH v2 3/7] configure: Look for auxiliary Python installations
On Fri, Feb 10, 2023 at 4:28 PM John Snow wrote:
> PS, while you're here, how does this new loop interfere with your "custom
> python specified" flag for meson? I think meson uses the version of python
> *it* detects and not the configure script identified one, right? Does that
> mean that e.g. the qapi generator gets run with the system default/meson
> version and not the config version?
Yes, if neither --python nor --meson are specified, then it could
happen that a different python is used during ninja's execution vs.
what is used for "other stuff" (docker cross compilers and other
Makefile invocations of $(PYTHON)).
The meson version of Python is guaranteed to be at least 3.7 as soon
as we update to 0.63.x (which will be Real Soon Now), but it's ugly.
The main issue I anticipate could be a problem when running from a
virtual environment, so perhaps we can force usage of the internal
meson if neither --python nor --meson are specified, and VIRTUAL_ENV
is set and $VIRTUAL_ENV/bin/meson does not exist?
diff --git a/configure b/configure
index 06bcd9031903..001a79a90170 100755
--- a/configure
+++ b/configure
@@ -870,8 +870,18 @@ fi
# Suppress writing compiled files
python="$python -B"
+has_meson() {
+ if test "${VIRTUAL_ENV:+set}" = set; then
+# Ensure that Meson and Python come from the same virtual environment
+test -x "$(VIRTUAL_ENV}/bin/meson" &&
+ test "$(command -v meson)" -ef "$(VIRTUAL_ENV}/bin/meson"
+ else
+has meson
+ fi
+}
+
if test -z "$meson"; then
-if test "$explicit_python" = no && has meson && version_ge
"$(meson --version)" 0.63.0; then
+if test "$explicit_python" = no && has_meson && version_ge
"$(meson --version)" 0.63.0; then
meson=meson
elif test "$git_submodules_action" != 'ignore' ; then
meson=git
I will include it when posting the final series.
> Do I need to adjust this loop to consider more binaries as "explicitly
> specified"?
I don't think it's a huge problem. Outside virtual environments, the
most likely setting is that Meson uses python3 which in turn is the
most recent python3.X, so it should be fine overall.
Though part of me thinks that your new loop is slightly overengineered
and we should just require /usr/bin/env python3 and call it a day.
Paolo
Re: [PULL 00/17] Migration 20230209 patches
On Fri, 10 Feb 2023 at 16:13, Juan Quintela wrote:
> Again, I don't know why it fails.
>
> diff --git a/tests/bench/meson.build b/tests/bench/meson.build
> index daefead58d..7477a1f401 100644
> --- a/tests/bench/meson.build
> +++ b/tests/bench/meson.build
> @@ -3,9 +3,11 @@ qht_bench = executable('qht-bench',
> sources: 'qht-bench.c',
> dependencies: [qemuutil])
>
> +if have_system
> xbzrle_bench = executable('xbzrle-bench',
> sources: 'xbzrle-bench.c',
> dependencies: [qemuutil,migration])
> +endif
>
> executable('atomic_add-bench',
> sources: files('atomic_add-bench.c'),
>
> This make it works.
Before you added your test, meson had no need to compile
any of the object files in 'migration', so it didn't. Now
you tell meson to build a new executable, and it says "OK,
I must build these object files". Only it turns out that
they won't actually compile in this config, so you get an
error.
The same issue can happen in good old Make :-)
thanks
-- PMM
Re: [PULL 00/17] Migration 20230209 patches
Peter Maydell wrote:
> On Fri, 10 Feb 2023 at 14:21, Juan Quintela wrote:
>>
>> Peter Maydell wrote:
>> > Fails to build the user-mode emulators:
>>
>> This is weird.
>
>> > https://gitlab.com/qemu-project/qemu/-/jobs/3749435025
>> >
>> > In file included from ../authz/base.c:24:
>> > ../authz/trace.h:1:10: fatal error: trace/trace-authz.h: No such file
>> > or directory
>> > 1 | #include "trace/trace-authz.h"
>>
>> This series only have one change for traces:
>>
>> diff --git a/util/trace-events b/util/trace-events
>> index c8f53d7d9f..16f78d8fe5 100644
>> --- a/util/trace-events
>> +++ b/util/trace-events
>> @@ -93,6 +93,7 @@ qemu_vfio_region_info(const char *desc, uint64_t
>> region_ofs, uint64_t region_siz
>> qemu_vfio_pci_map_bar(int index, uint64_t region_ofs, uint64_t
>> region_size, int ofs, void *host) "map region bar#%d addr
>> 0x%"PRIx64" size 0x%"PRIx64" ofs 0x%x host %p"
>>
>> #userfaultfd.c
>> +uffd_detect_open_mode(int mode) "%d"
>> uffd_query_features_nosys(int err) "errno: %i"
>> uffd_query_features_api_failed(int err) "errno: %i"
>> uffd_create_fd_nosys(int err) "errno: %i"
>>
>> Rest of trace mentions are for the removal of migration.multifd.c.orig
>>
>> And I don't play with authentication at all.
>>
>> This is Fedora 37.
>>
>> > https://gitlab.com/qemu-project/qemu/-/jobs/3749435094
>> > In file included from ../authz/simple.c:23:
>> > ../authz/trace.h:1:10: fatal error: trace/trace-authz.h: No such file
>> > or directory
>>
>> Problem is that this trace file is not generated, but I can think how
>> any change that I did can influence this.
>>
>> > 1 | #include "trace/trace-authz.h"
>> >
>> >
>> > https://gitlab.com/qemu-project/qemu/-/jobs/3749434963
>> > In file included from ../authz/listfile.c:23:
>> > ../authz/trace.h:1:10: fatal error: trace/trace-authz.h: No such file
>> > or directory
>> > 1 | #include "trace/trace-authz.h"
>>
>> Looking at the ouptut of these, they are not informatives at all.
>>
>> I am going to try to compile linux-user without system, and see if that
>> brings a clue.
>
> Yes, I suspect this is a "user-mode only build" specific failure
> (you may need --disable-system --disable-tools to see it).
git-bisect is my friend O:-)
And yes, the problem was in my PULL request.
Again, I don't know why it fails.
diff --git a/tests/bench/meson.build b/tests/bench/meson.build
index daefead58d..7477a1f401 100644
--- a/tests/bench/meson.build
+++ b/tests/bench/meson.build
@@ -3,9 +3,11 @@ qht_bench = executable('qht-bench',
sources: 'qht-bench.c',
dependencies: [qemuutil])
+if have_system
xbzrle_bench = executable('xbzrle-bench',
sources: 'xbzrle-bench.c',
dependencies: [qemuutil,migration])
+endif
executable('atomic_add-bench',
sources: files('atomic_add-bench.c'),
This make it works.
And no, I still not have a single clue how creating a new executable in
tests/bench/ can make trace files not to be generated somewhere else.
> meson.build only puts authz into trace_events_subdirs "if have_block"
> (which is to say "if have_system or have_tools"). However the
> bit of meson.build that says "subdir('authz') does not have
> the same condition on it -- it's just been put in the list without
> any condition on it. So I think that in a build-only-user-emulators
> config meson will not generate trace events for the subdirectory
> but will try to build it, which falls over.
>
> Contrast 'block', 'nbd', 'scsi', which are all guarded by
> 'if have_block' for their subdir() lines, to match the guard on
> the trace_events_subdirs. OTOH 'io' is also mismatched-guards...
>
> Why this only shows up with your pullreq I have no idea.
xbzrle_bench.
Notice that dependency on migration.
As said, I compile every user and system targets that compile in fedora
before I submit a PULL request.
But it is getting clear than that is not enough anymore.
Sorry about the noise.
Will resubmit the PULL requset.
Later, Juan.
Re: [PATCH v3 03/14] migration: Initial support of fixed-ram feature for analyze-migration.py
On Fri, Oct 28, 2022 at 01:39:03PM +0300, Nikolay Borisov wrote: > In order to allow analyze-migration.py script to work with migration > streams that have the 'fixed-ram' capability set it's required to have > access to the stream's configuration object. This commit enables this > by making migration json writer part of MigrationState struct, allowing > the configuration object be serialized to json. > > Signed-off-by: Nikolay Borisov > --- > migration/migration.c| 5 > migration/migration.h| 3 +++ > migration/savevm.c | 47 ++ > scripts/analyze-migration.py | 49 +--- > 4 files changed, 85 insertions(+), 19 deletions(-) This patch has a minor clash with a patch that's since merged into git master commit e3bf5e68e2a97898f37834c47449101172ced123 Author: David Hildenbrand Date: Tue Jan 17 12:22:43 2023 +0100 migration/savevm: Prepare vmdesc json writer in qemu_savevm_state_setup() so needs some adjustment, but nothing too major The actual goal of this change makes sense to me. 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 :|
Re: [PATCH v3] hw/arm/smmuv3: Add GBPA register
On Tue, 7 Feb 2023 at 18:31, Mostafa Saleh wrote:
>
> GBPA register can be used to globally abort all
> transactions.
>
> It is described in the SMMU manual in "6.3.14 SMMU_GBPA".
> ABORT reset value is IMPLEMENTATION DEFINED, it is chosen to
> be zero(Do not abort incoming transactions).
>
> Other fields have default values of Use Incoming.
>
> If UPDATE is not set, the write is ignored. This is the only permitted
> behavior in SMMUv3.2 and later.(6.3.14.1 Update procedure)
>
> As this patch adds a new state to the SMMU (GBPA), it is added
> in a new subsection for forward migration compatibility.
> GBPA is only migrated when GBPA.ABORT is set from SW at the time of
> migration, to avoid inconsistencies where a qemu instance is aborting
> transactions and it is migrated to another older instance bypassing
> them.
>
> Signed-off-by: Mostafa Saleh
> ---
>
> Changes in v3:
> - Remove migrate_gbpa property as it was unnecessary.
>
> Changes in v2:
> - GBPA is effective only when SMMU is not enabled.
> - Ignore GBPA write when UPDATE is not set.
> - Default value for SHCFG is "Use Incoming".
> - Support migration.
>
> hw/arm/smmuv3-internal.h | 5
> hw/arm/smmuv3.c | 52 +++-
> include/hw/arm/smmuv3.h | 1 +
> 3 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index bce161870f..3ff704248d 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -79,6 +79,11 @@ REG32(CR0ACK, 0x24)
> REG32(CR1, 0x28)
> REG32(CR2, 0x2c)
> REG32(STATUSR, 0x40)
> +REG32(GBPA,0x44)
> +FIELD(GBPA, SHCFG,12, 2)
> +FIELD(GBPA, ABORT,20, 1)
> +FIELD(GBPA, UPDATE, 31, 1)
> +
> REG32(IRQ_CTRL,0x50)
> FIELD(IRQ_CTRL, GERROR_IRQEN,0, 1)
> FIELD(IRQ_CTRL, PRI_IRQEN, 1, 1)
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 955b89c8d5..a184665181 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -285,6 +285,8 @@ static void smmuv3_init_regs(SMMUv3State *s)
> s->gerror = 0;
> s->gerrorn = 0;
> s->statusr = 0;
> +/* Use incoming as other fields. */
> +s->gbpa = FIELD_DP32(s->gbpa, GBPA, SHCFG, 1);
> }
>
> static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf,
> @@ -659,7 +661,11 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
> *mr, hwaddr addr,
> qemu_mutex_lock(&s->mutex);
>
> if (!smmu_enabled(s)) {
> -status = SMMU_TRANS_DISABLE;
> +if (FIELD_EX32(s->gbpa, GBPA, ABORT)) {
> +status = SMMU_TRANS_ABORT;
> +} else {
> +status = SMMU_TRANS_DISABLE;
> +}
> goto epilogue;
> }
>
> @@ -1170,6 +1176,16 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr
> offset,
> case A_GERROR_IRQ_CFG2:
> s->gerror_irq_cfg2 = data;
> return MEMTX_OK;
> +case A_GBPA:
> +/*
> + * If UPDATE is not set, the write is ignored. This is the only
> + * permitted behavior in SMMUv3.2 and later.
> + */
> +if (data & R_GBPA_UPDATE_MASK) {
> +/* Ignore update bit as write is synchronous. */
> +s->gbpa = data & ~R_GBPA_UPDATE_MASK;
> +}
> +return MEMTX_OK;
> case A_STRTAB_BASE: /* 64b */
> s->strtab_base = deposit64(s->strtab_base, 0, 32, data);
> return MEMTX_OK;
> @@ -1318,6 +1334,9 @@ static MemTxResult smmu_readl(SMMUv3State *s, hwaddr
> offset,
> case A_STATUSR:
> *data = s->statusr;
> return MEMTX_OK;
> +case A_GBPA:
> +*data = s->gbpa;
> +return MEMTX_OK;
> case A_IRQ_CTRL:
> case A_IRQ_CTRL_ACK:
> *data = s->irq_ctrl;
> @@ -1482,6 +1501,33 @@ static const VMStateDescription vmstate_smmuv3_queue =
> {
> },
> };
>
> +static bool smmuv3_gbpa_needed(void *opaque)
> +{
> +SMMUv3State *s = opaque;
> +
> +/*
> + * Only migrate GBPA if ABORT set from SW to 1, which is different from
> + * default behaviour. This allows maximum compatibility with old qemu
> + * instances while being forward compatible.
> + */
> +if (FIELD_EX32(s->gbpa, GBPA, ABORT)) {
> +return true;
> +}
I think we should check the whole register against its reset value,
not just the ABORT bit. Otherwise if the guest writes the other fields
to non default values we won't migrate them. That doesn't change the
device behaviour now but it will have the weird effect that the guest
could write the register and then after a migration find it reading
back as a different value. Plus if in future we implement actual
functionality for any of the other fields then we'll want to know
what their true values written by the guest are.
Linux never changes any fields except ABORT so for the most interesting
guest it won't make a difference right now.
Re: [PATCH] vhost-user-fs: add capability to allow migration
Anton Kuchin wrote:
> On 02/02/2023 11:59, Juan Quintela wrote:
>> Anton Kuchin wrote:
>>> On 01/02/2023 16:26, Juan Quintela wrote:
Anton Kuchin wrote:
> On 19/01/2023 18:02, Stefan Hajnoczi wrote:
>> On Thu, 19 Jan 2023 at 10:29, Anton Kuchin
>> wrote:
>>> On 19/01/2023 16:30, Stefan Hajnoczi wrote:
On Thu, 19 Jan 2023 at 07:43, Anton Kuchin
wrote:
> On 18/01/2023 17:52, Stefan Hajnoczi wrote:
>> On Sun, 15 Jan 2023 at 12:21, Anton Kuchin
>> wrote:
Once told that, I think that you are making your live harder in the
future when you add the other migratable devices.
static const VMStateDescription vuf_vmstate = {
.name = "vhost-user-fs",
.minimum_version_id = 0,
.version_id = 0,
.fields = (VMStateField[]) {
VMSTATE_INT8(migration_type, struct VHostUserFS),
VMSTATE_VIRTIO_DEVICE,
VMSTATE_END_OF_LIST()
},
.pre_save = vhost_user_fs_pre_save,
.subsections = (const VMStateDescription*[]) {
&vmstate_vhost_user_fs_internal_sub,
NULL
}
};
And you are done.
I will propose to use a property to set migration_type, but I didn't
want to write the code right now.
>
> I have a little problem with implementation here and need an advice:
>
> Can we make this property adjustable at runtime after device was realized?
> There is a statement in qemu wiki [1] that makes me think this is possible
> but I couldn't find any code for it or example in other devices.
>> "Properties are, by default, locked while the device is
> realized. Exceptions
>> can be made by the devices themselves in order to implement a way
> for a user
>> to interact with a device while it is realized."
>
> Or is your idea just to set this property once at construction and keep it
> constant for device lifetime?
>
> [1] https://wiki.qemu.org/Features/QOM
I have no clue here. Markus? Stefan?
I think that this proposal will make Stephan happy, and it is just
adding and extra uint8_t that is helpul to implement everything.
>>> That is exactly the approach I'm trying to implement it right now. Single
>>> flag can be convenient for orchestrator but makes it too hard to account in
>>> all cases for all devices on qemu side without breaking future
>>> compatibility.
>>> So I'm rewriting it with properties.
>> Nice. That would be my proposal. Just a bit complicated for a proof of
>> concetp.
>>
>>> BTW do you think each vhost-user device should have its own enum of
>>> migration
>>> types or maybe we could make them common for all device types?
>> I will put it for vhost-user, because as far as I know nobody else is
>> asking for this functionality.
>
> I mean do we need it for all vhost-user devices or only for vhost-user-fs
> that I'm implementing now?
I will put it only for vhost-user-fs, except if there is a central place
that is used for all vhost-user and its easy to put there.
But I don't know enough about vhost-user to know if there is any common
struct to put this.
>> The most similar device that I can think of right now is vfio devices.
>> But they are implemeting callbacks to save hardware device state, and
>> they go device by device, i.e. there is nothing general there.
>>
>> Later, Juan.
>>
Later, Juan.
Re: Byte-swapping issue on qemu-user for sparc64 guest
Hi Thomas! On Fri, 2023-02-10 at 16:52 +0100, Thomas Huth wrote: > On 10/02/2023 16.23, John Paul Adrian Glaubitz wrote: > > Hi! > > > > There is an unaddressed issue in qemu-user [1] which results in getresuid() > > returning an incorrect UID due to a byte-swapping issue on sparc64. > > Oh, there are still people out there using qemu-user on sparc64 hosts? ... This is about running sparc64 binaries on x86_64. > that reminds me of another outstanding issue which might occur there: > > https://mail.gnu.org/archive/html/qemu-devel/2021-02/msg04240.html > > ... in case someone has time for fixing this ... (I unfortunately never > found enough spare time again to revisit the issue). We're still maintaining a sparc64 port in Debian, FWIW. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [RFC PATCH] tests/avocado: retire the Aarch64 TCG tests from boot_linux.py
On Fri, 3 Feb 2023 at 18:16, Alex Bennée wrote: > > The two TCG tests for GICv2 and GICv3 are very heavy weight distros > that take a long time to boot up, especially for an --enable-debug > build. The total code coverage they give is: > > Overall coverage rate: > lines..: 11.2% (59584 of 530123 lines) > functions..: 15.0% (7436 of 49443 functions) > branches...: 6.3% (19273 of 303933 branches) > > We already get pretty close to that with the machine_aarch64_virt > tests which only does one full boot (~120s vs ~600s) of alpine. We > expand the kernel+initrd boot (~8s) to test both GICs and also add an > RNG device and a block device to generate a few IRQs and exercise the > storage layer. With that we get to a coverage of: > > Overall coverage rate: > lines..: 11.0% (58121 of 530123 lines) > functions..: 14.9% (7343 of 49443 functions) > branches...: 6.0% (18269 of 303933 branches) > > which I feel is close enough given the massive time saving. If we want > to target any more sub-systems we can use lighter weight more directed > tests. Applied to target-arm.next, thanks. This should significantly improve my pullreq-assembly experience :-) -- PMM
Re: [PATCH v2 00/11] target/arm: Housekeeping around NVIC
On Mon, 6 Feb 2023 at 22:35, Philippe Mathieu-Daudé wrote: > > Missing review: 1-3, 5, 9-10 > > Few cleanups while using link properties between CPU/NVIC: > - Simplify ID_PFR1 on useremu > - Move NVIC helpers to "hw/intc/armv7m_nvic.h" > > Since v1: addressed Richard's reviews > - Do not restrict v7-M MMU helpers to TCG sysemu since they can be > used for user-emu. Hardcode ARMMMUIdx_MUser > - Convert CPUARMState::eabi to boolean > - Split 'Restrict nvic to sysemu and store as NVICState' in 3 patches > - Dropped following (RFC) patches: > - neg_prio_requested / unrealized property problem > - use object_property_add_const_link() Applied to target-arm.next, thanks. -- PMM
