Re: [PATCH v3] i386: Add ratelimit for bus locks acquired in guest
+Cc qemu-devel@nongnu.org
On 5/14/2021 9:07 AM, Chenyi Qiang wrote:
Hi Paolo, Eduardo
Any comments on this version?
On 4/30/2021 6:33 PM, Chenyi Qiang wrote:
A bus lock is acquired through either split locked access to writeback
(WB) memory or any locked access to non-WB memory. It is typically >1000
cycles slower than an atomic operation within a cache and can also
disrupts performance on other cores.
Virtual Machines can exploit bus locks to degrade the performance of
system. To address this kind of performance DOS attack coming from the
VMs, bus lock VM exit is introduced in KVM and it can report the bus
locks detected in guest. If enabled in KVM, it would exit to the
userspace to let the user enforce throttling policies once bus locks
acquired in VMs.
The availability of bus lock VM exit can be detected through the
KVM_CAP_X86_BUS_LOCK_EXIT. The returned bitmap contains the potential
policies supported by KVM. The field KVM_BUS_LOCK_DETECTION_EXIT in
bitmap is the only supported strategy at present. It indicates that KVM
will exit to userspace to handle the bus locks.
This patch adds a ratelimit on the bus locks acquired in guest as a
mitigation policy.
Introduce a new field "bus_lock_ratelimit" to record the limited speed
of bus locks in the target VM. The user can specify it through the
"bus-lock-ratelimit" as a machine property. In current implementation,
the default value of the speed is 0 per second, which means no
restrictions on the bus locks
As for ratelimit on detected bus locks, simply set the ratelimit
interval to 1s and restrict the quota of bus lock occurence to the value
of "bus_lock_ratelimit". A potential alternative is to introduce the
time slice as a property which can help the user achieve more precise
control.
The detail of Bus lock VM exit can be found in spec:
https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html
Signed-off-by: Chenyi Qiang
---
Changes from v2:
- do some rename work (bus-lock-ratelimit and BUS_LOCK_TIME_SLICE).
(Eduardo)
- change to register a class property at the x86_machine_class_init()
and write the gettter/setter for the bus_lock_ratelimit property.
(Eduardo)
- add the lock to access the Ratelimit instance to avoid vcpu thread
race condition. (Eduardo)
- v2:
https://lore.kernel.org/qemu-devel/20210420093736.17613-1-chenyi.qi...@intel.com/
Changes from RFC v1:
- Remove the rip info output, as the rip can't reflect the bus lock
position correctly. (Xiaoyao)
- RFC v1:
https://lore.kernel.org/qemu-devel/20210317084709.15605-1-chenyi.qi...@intel.com/
---
hw/i386/x86.c | 24 ++
include/hw/i386/x86.h | 9 +
target/i386/kvm/kvm.c | 46 +++
3 files changed, 79 insertions(+)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index ed796fe6ba..d30cf27e29 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1246,6 +1246,23 @@ static void x86_machine_set_oem_table_id(Object
*obj, const char *value,
strncpy(x86ms->oem_table_id, value, 8);
}
+static void x86_machine_get_bus_lock_ratelimit(Object *obj, Visitor *v,
+ const char *name, void *opaque, Error
**errp)
+{
+ X86MachineState *x86ms = X86_MACHINE(obj);
+ uint64_t bus_lock_ratelimit = x86ms->bus_lock_ratelimit;
+
+ visit_type_uint64(v, name, _lock_ratelimit, errp);
+}
+
+static void x86_machine_set_bus_lock_ratelimit(Object *obj, Visitor *v,
+ const char *name, void *opaque, Error
**errp)
+{
+ X86MachineState *x86ms = X86_MACHINE(obj);
+
+ visit_type_uint64(v, name, >bus_lock_ratelimit, errp);
+}
+
static void x86_machine_initfn(Object *obj)
{
X86MachineState *x86ms = X86_MACHINE(obj);
@@ -1256,6 +1273,7 @@ static void x86_machine_initfn(Object *obj)
x86ms->pci_irq_mask = ACPI_BUILD_PCI_IRQS;
x86ms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
+ x86ms->bus_lock_ratelimit = 0;
}
static void x86_machine_class_init(ObjectClass *oc, void *data)
@@ -1299,6 +1317,12 @@ static void x86_machine_class_init(ObjectClass
*oc, void *data)
"Override the default
value of field OEM Table ID "
"in ACPI table header."
"The string may be up to 8
bytes in size");
+
+ object_class_property_add(oc, X86_MACHINE_BUS_LOCK_RATELIMIT,
"uint64_t",
+ x86_machine_get_bus_lock_ratelimit,
+ x86_machine_set_bus_lock_ratelimit,
NULL, NULL);
+ object_class_property_set_description(oc,
X86_MACHINE_BUS_LOCK_RATELIMIT,
+ "Set the ratelimit for the bus locks acquired in VMs");
}
static const TypeInfo x86_machine_info = {
diff --git
Re: [Qemu-devel] [PATCH] MAINTAINERS: convert to JSON
On 13/05/21 23:02, Willian Rampazzo wrote: I think I understand why you brought this thread back now. Finding a way to correlate maintainers and tags is, indeed, a nice feature that can help with your idea about correlating tags and maintainers to use on tests. --verbose? :) Paolo
Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
On 14/05/21 01:35, David Gibson wrote: "FDT" is set by meson.build if the library is available, LIBFDT is set by the board to link with the library. In other words CONFIG_FDT is per-build, while CONFIG_LIBFDT is per-target. Oof... that's highly non-obvious. Could we call it HAVE_LIBFDT and USE_LIBFDT instead? Certainly okay by me. Paolo
[PATCH] target/riscv: Remove unnecessary riscv_*_names[] declaration
riscv_excp_names[] and riscv_intr_names[] are only referenced by
target/riscv/cpu.c locally.
Signed-off-by: Bin Meng
---
target/riscv/cpu.h | 2 --
target/riscv/cpu.c | 4 ++--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7e879fb9ca..adba2ff533 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -318,8 +318,6 @@ static inline bool riscv_feature(CPURISCVState *env, int
feature)
extern const char * const riscv_int_regnames[];
extern const char * const riscv_fpr_regnames[];
-extern const char * const riscv_excp_names[];
-extern const char * const riscv_intr_names[];
const char *riscv_cpu_get_trap_name(target_ulong cause, bool async);
void riscv_cpu_do_interrupt(CPUState *cpu);
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3191fd0082..7ee31f97a3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -51,7 +51,7 @@ const char * const riscv_fpr_regnames[] = {
"f30/ft10", "f31/ft11"
};
-const char * const riscv_excp_names[] = {
+static const char * const riscv_excp_names[] = {
"misaligned_fetch",
"fault_fetch",
"illegal_instruction",
@@ -78,7 +78,7 @@ const char * const riscv_excp_names[] = {
"guest_store_page_fault",
};
-const char * const riscv_intr_names[] = {
+static const char * const riscv_intr_names[] = {
"u_software",
"s_software",
"vs_software",
--
2.25.1
Re: GICv3 for MTTCG
On 5/13/21 8:20 PM, Alex Bennée wrote: > > Andrey Shinkevich writes: > >> Dear colleagues, >> >> Thank you all very much for your responses. Let me reply with one message. >> >> I configured QEMU for AARCH64 guest: >> $ ./configure --target-list=aarch64-softmmu >> >> When I start QEMU with GICv3 on an x86 host: >> qemu-system-aarch64 -machine virt-6.0,accel=tcg,gic-version=3 > > Hmm are you sure you are running your built QEMU? For me the following > works fine: No doubt I run my built QEMU because I am debugging it and watching the run of it with gcc. > >./aarch64-softmmu/qemu-system-aarch64 -machine > virt-6.0,gic-version=3,accel=tcg -cpu max -serial mon:stdio -nic > user,model=virtio-net-pci,hostfwd=tcp::-:22 -device virtio-scsi-pci > -device scsi-hd,drive=hd0 -blockdev > driver=raw,node-name=hd0,discard=unmap,file.driver=host_device,file.filename=/dev/zvol/hackpool-0/debian-buster-arm64 > -kernel > ~/lsrc/linux.git/builds/arm64.nopreempt/arch/arm64/boot/Image -append > "console=ttyAMA0 root=/dev/sda2" -display none -m 8G,maxmem=8G -smp 12 > > Which source code are you using for building your QEMU? Would you please send me the link if it is a source other than github.com/qemu/qemu? I downloaded and pulled the latest commit 3e9f48bcdabe57f8f and applied the series "[PATCH v3 0/8] GICv3 LPI and ITS feature implementation" ONLY. Did you do the same? I have NOT applied the series "[PATCH v2 0/7] accel/tcg: remove implied BQL from cpu_handle_interrupt/exception path" yet because it is old and the manual applying takes more time (will do it later). Is it a possible reason that my guest hangs with locks at start? Andrey >> >> QEMU reports this error from hw/pci/msix.c: >> error_setg(errp, "MSI-X is not supported by interrupt controller"); >> >> Probably, the variable 'msi_nonbroken' would be initialized in >> hw/intc/arm_gicv3_its_common.c: >> gicv3_its_init_mmio(..) >> >> I guess that it works with KVM acceleration only rather than with TCG. >> >> The error persists after applying the series: >> https://lists.gnu.org/archive/html/qemu-arm/2021-04/msg00944.html >> "GICv3 LPI and ITS feature implementation" >> (special thanks for referring me to that) >> >> Please, make me clear and advise ideas how that error can be fixed? >> Should the MSI-X support be implemented with GICv3 extra? >> >> When successful, I would like to test QEMU for a maximum number of cores >> to get the best MTTCG performance. >> Probably, we will get just some percentage of performance enhancement >> with the BQL series applied, won't we? I will test it as well. >> >> Best regards, >> Andrey Shinkevich >> >> >> On 5/12/21 6:43 PM, Alex Bennée wrote: >>> >>> Andrey Shinkevich writes: >>> Dear colleagues, I am looking for ways to accelerate the MTTCG for ARM guest on x86-64 host. The maximum number of CPUs for MTTCG that uses GICv2 is limited by 8: include/hw/intc/arm_gic_common.h:#define GIC_NCPU 8 The version 3 of the Generic Interrupt Controller (GICv3) is not supported in QEMU for some reason unknown to me. It would allow to increase the limit of CPUs and accelerate the MTTCG performance on a multiple core hypervisor. >>> >>> It is supported, you just need to select it. >>> I have got an idea to implement the Interrupt Translation Service (ITS) for using by MTTCG for ARM architecture. >>> >>> There is some work to support ITS under TCG already posted: >>> >>> Subject: [PATCH v3 0/8] GICv3 LPI and ITS feature implementation >>> Date: Thu, 29 Apr 2021 19:41:53 -0400 >>> Message-Id: <20210429234201.125565-1-shashi.mall...@linaro.org> >>> >>> please do review and test. >>> Do you find that idea useful and feasible? If yes, how much time do you estimate for such a project to complete by one developer? If no, what are reasons for not implementing GICv3 for MTTCG in QEMU? >>> >>> As far as MTTCG performance is concerned there is a degree of >>> diminishing returns to be expected as the synchronisation cost between >>> threads will eventually outweigh the gains of additional threads. >>> >>> There are a number of parts that could improve this performance. The >>> first would be picking up the BQL reduction series from your FutureWei >>> colleges who worked on the problem when they were Linaro assignees: >>> >>> Subject: [PATCH v2 0/7] accel/tcg: remove implied BQL from >>> cpu_handle_interrupt/exception path >>> Date: Wed, 19 Aug 2020 14:28:49 -0400 >>> Message-Id: <20200819182856.4893-1-robert.fo...@linaro.org> >>> >>> There was also a longer series moving towards per-CPU locks: >>> >>> Subject: [PATCH v10 00/73] per-CPU locks >>> Date: Wed, 17 Jun 2020 17:01:18 -0400 >>> Message-Id: <20200617210231.4393-1-robert.fo...@linaro.org> >>> >>> I believe the initial measurements showed that the BQL cost started to >>> edge up with GIC interactions. We did discuss approaches for this and I >>>
Re: [PATCH v5 00/12] net: Pad short frames for network backends
Hi Jason, On Mon, Mar 22, 2021 at 3:10 PM Jason Wang wrote: > > > 在 2021/3/17 下午2:26, Bin Meng 写道: > > The minimum Ethernet frame length is 60 bytes. For short frames with > > smaller length like ARP packets (only 42 bytes), on a real world NIC > > it can choose either padding its length to the minimum required 60 > > bytes, or sending it out directly to the wire. Such behavior can be > > hardcoded or controled by a register bit. Similarly on the receive > > path, NICs can choose either dropping such short frames directly or > > handing them over to software to handle. > > > > On the other hand, for the network backends like SLiRP/TAP, they > > don't expose a way to control the short frame behavior. As of today > > they just send/receive data from/to the other end connected to them, > > which means any sized packet is acceptable. So they can send and > > receive short frames without any problem. It is observed that ARP > > packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send > > these ARP packets to the other end which might be a NIC model that > > does not allow short frames to pass through. > > > > To provide better compatibility, for packets sent from QEMU network > > backends like SLiRP/TAP, we change to pad short frames before sending > > it out to the other end, if the other end does not forbid it via the > > nc->do_not_pad flag. This ensures a backend as an Ethernet sender > > does not violate the spec. But with this change, the behavior of > > dropping short frames from SLiRP/TAP interfaces in the NIC model > > cannot be emulated because it always receives a packet that is spec > > complaint. The capability of sending short frames from NIC models is > > still supported and short frames can still pass through SLiRP/TAP. > > > > This series should be able to fix the issue as reported with some > > NIC models before, that ARP requests get dropped, preventing the > > guest from becoming visible on the network. It was workarounded in > > these NIC models on the receive path, that when a short frame is > > received, it is padded up to 60 bytes. > > > > Changes in v5: > > - minor update on commit message > > - update the eth_pad_short_frame() comment > > > > Changes in v4: > > - change 'ethernet' to 'Ethernet' > > - do not inline the helper > > - check the padded buffer size to avoid buffer overflow > > - squash slirp/tap commits into one > > > > Changes in v3: > > - use 'without' instead of 'sans' > > - add a helper to pad short frames > > - add a comment to 'do_not_pad' > > - use the pad_short_frame() helper > > > > Bin Meng (12): > >net: eth: Add a helper to pad a short Ethernet frame > >net: Add a 'do_not_pad" to NetClientState > >net: Pad short frames to minimum size before sending from SLiRP/TAP > >hw/net: virtio-net: Initialize nc->do_not_pad to true > >hw/net: e1000: Remove the logic of padding short frames in the receive > > path > >hw/net: vmxnet3: Remove the logic of padding short frames in the > > receive path > >hw/net: i82596: Remove the logic of padding short frames in the > > receive path > >hw/net: ne2000: Remove the logic of padding short frames in the > > receive path > >hw/net: pcnet: Remove the logic of padding short frames in the receive > > path > >hw/net: rtl8139: Remove the logic of padding short frames in the > > receive path > >hw/net: sungem: Remove the logic of padding short frames in the > > receive path > >hw/net: sunhme: Remove the logic of padding short frames in the > > receive path > > > > hw/net/e1000.c | 11 +-- > > hw/net/i82596.c | 18 -- > > hw/net/ne2000.c | 12 > > hw/net/pcnet.c | 9 - > > hw/net/rtl8139.c| 12 > > hw/net/sungem.c | 14 -- > > hw/net/sunhme.c | 11 --- > > hw/net/virtio-net.c | 4 > > hw/net/vmxnet3.c| 10 -- > > include/net/eth.h | 17 + > > include/net/net.h | 1 + > > net/eth.c | 17 + > > net/slirp.c | 10 ++ > > net/tap-win32.c | 10 ++ > > net/tap.c | 10 ++ > > 15 files changed, 70 insertions(+), 96 deletions(-) > > > I've queued patch 1-4 for 6.0 and the reset for 6.1. It seems the reset has not been applied for 6.1? Regards, Bin
Re: [PATCH v9 2/3] target/arm: Add support for FEAT_TLBIOS
On 5/12/21 1:23 PM, Rebecca Cran wrote: ARMv8.4 adds the mandatory FEAT_TLBIOS. It provides TLBI maintenance instructions that extend to the Outer Shareable domain. Signed-off-by: Rebecca Cran --- target/arm/cpu.h| 5 +++ target/arm/helper.c | 43 2 files changed, 48 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v9 1/3] target/arm: Add support for FEAT_TLBIRANGE
On 5/12/21 1:23 PM, Rebecca Cran wrote: ARMv8.4 adds the mandatory FEAT_TLBIRANGE. It provides TLBI maintenance instructions that apply to a range of input addresses. Signed-off-by: Rebecca Cran --- target/arm/cpu.h| 5 + target/arm/helper.c | 281 2 files changed, 286 insertions(+) Reviewed-by: Richard Henderson r~
[Bug 1917184] Re: qemu-user vm86() segfaults handling interrupt with ss:sp in same page as cs:ip
Bug still present in latest master ** Changed in: qemu Status: Incomplete => New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1917184 Title: qemu-user vm86() segfaults handling interrupt with ss:sp in same page as cs:ip Status in QEMU: New Bug description: When using qemu-i386 to run a program that uses vm86(), if the vm86 code calls an interrupt while cs:ip and ss:sp both point within the same page, do_int tries to write to the page while it is not writable, causing a segfault. qemu version 5.2.0, x86-64 host. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1917184/+subscriptions
Re: [PATCH] target/riscv: Dump CSR mscratch/sscratch/satp
On Wed, Apr 28, 2021 at 8:29 PM Changbin Du wrote:
>
> This dumps the CSR mscratch/sscratch/satp and meanwhile aligns
> the output of CSR mtval/stval.
>
> Signed-off-by: Changbin Du
> ---
> target/riscv/cpu.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7d6ed80f6b67..73af6f5445ba 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -281,12 +281,15 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f,
> int flags)
> if (riscv_has_ext(env, RVH)) {
> qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vscause ", env->vscause);
> }
> -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval ", env->mtval);
> -qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "stval ", env->sbadaddr);
> +qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval ", env->mtval);
> +qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "stval ", env->sbadaddr);
> if (riscv_has_ext(env, RVH)) {
> qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "htval ", env->htval);
> qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtval2 ", env->mtval2);
I believe you also need some alignment for these 2
> }
> +qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mscratch", env->mscratch);
> +qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "sscratch", env->sscratch);
> +qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "satp", env->satp);
> #endif
>
> for (i = 0; i < 32; i++) {
Reviewed-by: Bin Meng
Re: [PATCH 02/10] hw/ide/Kconfig: Add missing dependency PCI -> QDEV
On Fri, May 14, 2021 at 12:56 AM Philippe Mathieu-Daudé
wrote:
>
> The pci_ide_create_devs() function is declared i hw/ide/qdev.c:
>
> $ git grep ide_create_drive
> hw/ide/pci.c:491:ide_create_drive(d->bus + bus[i], unit[i],
> hd_table[i]);
> hw/ide/qdev.c:127:IDEDevice *ide_create_drive(IDEBus *bus, int unit,
> DriveInfo *drive)
> include/hw/ide/internal.h:653:IDEDevice *ide_create_drive(IDEBus *bus, int
> unit, DriveInfo *drive);
>
> Fix the correct symbol dependency to avoid build failure when
> deselecting some machines:
>
> /usr/bin/ld: libcommon.fa.p/hw_ide_pci.c.o: in function
> `pci_ide_create_devs':
> hw/ide/pci.c:491: undefined reference to `ide_create_drive'
>
> Fixes: 8f01b41e109 ("ide: express dependencies with Kconfig")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/ide/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Bin Meng
Re: [PATCH 01/10] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
On Fri, May 14, 2021 at 12:53 AM Philippe Mathieu-Daudé wrote: > > Per the kconfig.rst: > > A device should be listed [...] ``imply`` if (depending on > the QEMU command line) the board may or may not be started > without it. > > This is the case with the NVDIMM device (it is certainly possible > to start a machine without NVDIMM) , so use the 'imply' weak > reverse dependency to select the symbol. > > Signed-off-by: Philippe Mathieu-Daudé > --- > default-configs/devices/ppc64-softmmu.mak | 1 - > hw/arm/Kconfig| 1 + > hw/i386/Kconfig | 1 + > hw/mem/Kconfig| 2 -- > hw/ppc/Kconfig| 1 + > 5 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/default-configs/devices/ppc64-softmmu.mak > b/default-configs/devices/ppc64-softmmu.mak > index ae0841fa3a1..cca52665d90 100644 > --- a/default-configs/devices/ppc64-softmmu.mak > +++ b/default-configs/devices/ppc64-softmmu.mak > @@ -8,4 +8,3 @@ CONFIG_POWERNV=y > > # For pSeries > CONFIG_PSERIES=y > -CONFIG_NVDIMM=y I think only removing this one makes sense. I fail to see any difference of other changes in this patch. > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index b887f6a5b17..67723d9ea6a 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -6,6 +6,7 @@ config ARM_VIRT > imply VFIO_PLATFORM > imply VFIO_XGMAC > imply TPM_TIS_SYSBUS > +imply NVDIMM > select ARM_GIC > select ACPI > select ARM_SMMUV3 > diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig > index 7f91f30877f..66838fa397b 100644 > --- a/hw/i386/Kconfig > +++ b/hw/i386/Kconfig > @@ -23,6 +23,7 @@ config PC > imply TPM_TIS_ISA > imply VGA_PCI > imply VIRTIO_VGA > +imply NVDIMM > select FDC > select I8259 > select I8254 > diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig > index a0ef2cf648e..8b19fdc49f1 100644 > --- a/hw/mem/Kconfig > +++ b/hw/mem/Kconfig > @@ -7,6 +7,4 @@ config MEM_DEVICE > > config NVDIMM > bool > -default y > -depends on (PC || PSERIES || ARM_VIRT) > select MEM_DEVICE > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig > index e51e0e5e5ac..66e0b15d9ef 100644 > --- a/hw/ppc/Kconfig > +++ b/hw/ppc/Kconfig > @@ -3,6 +3,7 @@ config PSERIES > imply PCI_DEVICES > imply TEST_DEVICES > imply VIRTIO_VGA > +imply NVDIMM > select DIMM > select PCI > select SPAPR_VSCSI Regards, Bin
Re: [PATCH 09/10] default-configs/devices: Remove implicy SEMIHOSTING config
On Fri, May 14, 2021 at 10:53 AM Bin Meng wrote:
>
> On Fri, May 14, 2021 at 12:58 AM Philippe Mathieu-Daudé
> wrote:
> >
> > Cmmit 56b5170c87e ("semihosting: Move ARM semihosting code to
typo: Commit
> > shared directories"), added the ARM_COMPATIBLE_SEMIHOSTING symbol
> > which selects SEMIHOSTING.
> >
> > Since the ARM/RISC-V targets select ARM_COMPATIBLE_SEMIHOSTING,
> > they don't need to select SEMIHOSTING manually. Simplify.
> >
> > Signed-off-by: Philippe Mathieu-Daudé
> > ---
> > default-configs/devices/arm-softmmu.mak | 1 -
> > default-configs/devices/riscv32-softmmu.mak | 1 -
> > default-configs/devices/riscv64-softmmu.mak | 1 -
> > 3 files changed, 3 deletions(-)
> >
>
> Reviewed-by: Bin Meng
Re: [PATCH 09/10] default-configs/devices: Remove implicy SEMIHOSTING config
On Fri, May 14, 2021 at 12:58 AM Philippe Mathieu-Daudé
wrote:
>
> Cmmit 56b5170c87e ("semihosting: Move ARM semihosting code to
> shared directories"), added the ARM_COMPATIBLE_SEMIHOSTING symbol
> which selects SEMIHOSTING.
>
> Since the ARM/RISC-V targets select ARM_COMPATIBLE_SEMIHOSTING,
> they don't need to select SEMIHOSTING manually. Simplify.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> default-configs/devices/arm-softmmu.mak | 1 -
> default-configs/devices/riscv32-softmmu.mak | 1 -
> default-configs/devices/riscv64-softmmu.mak | 1 -
> 3 files changed, 3 deletions(-)
>
Reviewed-by: Bin Meng
Re: [PATCH 08/10] hw/ppc/Kconfig: Add missing dependency E500 -> DS1338 RTC
On Fri, May 14, 2021 at 1:02 AM Philippe Mathieu-Daudé
wrote:
>
> Commit 7abb479c7ab ("PPC: E500: Add FSL I2C controller and integrate
> RTC with it") added a global dependency on the DS1338 model, instead
> of a machine one (via Kconfig). This gives trouble when building
> standalone machines not exposing I2C bus:
>
> The following clauses were found for DS1338
>
> CONFIG_DS1338=y
> config DS1338 depends on I2C
>
> Fix by selecting the DS1338 symbol in the single machine requiring
> it, the E500.
>
> Fixes: 7abb479c7ab ("PPC: E500: Add FSL I2C controller and integrate RTC with
> it")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Amit Singh Tomar
> Cc: Andrew Randrianasulu
> ---
> default-configs/devices/ppc-softmmu.mak | 1 -
> hw/ppc/Kconfig | 1 +
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Bin Meng
Re: [PATCH 07/10] hw/riscv/Kconfig: Restrict NUMA to Virt & Spike machines
On Fri, May 14, 2021 at 12:58 AM Philippe Mathieu-Daudé wrote: > > Only the Virt and Spike machines use NUMA. Add a RISCV_NUMA Kconfig > symbol and only have these machines select it. Adapt the Meson file > to only built it if required. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/riscv/Kconfig | 5 + > hw/riscv/meson.build | 2 +- > 2 files changed, 6 insertions(+), 1 deletion(-) > Reviewed-by: Bin Meng
Re: [PATCH 06/10] hw/riscv/Kconfig: Add missing dependency MICROCHIP_PFSOC -> SERIAL
Hi Philippe,
On Fri, May 14, 2021 at 12:57 AM Philippe Mathieu-Daudé
wrote:
>
> Commit a8fb0a500a6 ("hw/char: Add Microchip PolarFire SoC MMUART
> emulation") added a dependency on the SERIAL model, but forgot to
> add the Kconfig selector. Fix that to solve when built the
> MICROCHIP_PFSOC machine stand-alone:
>
> /usr/bin/ld: libcommon.fa.p/hw_char_mchp_pfsoc_mmuart.c.o: in function
> `mchp_pfsoc_mmuart_create':
> hw/char/mchp_pfsoc_mmuart.c:79: undefined reference to `serial_mm_init'
>
> Fixes: a8fb0a500a6 ("hw/char: Add Microchip PolarFire SoC MMUART emulation")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Bin Meng
> Cc: Alistair Francis
> ---
> hw/riscv/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index 86957ec7b06..b72a248809e 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -12,6 +12,7 @@ config MICROCHIP_PFSOC
> select SIFIVE_CLINT
> select SIFIVE_PDMA
> select SIFIVE_PLIC
> +select SERIAL
This dependency should be expressed by MCHP_PFSOC_MMUART in hw/char/Kconfig
> select UNIMP
>
> config OPENTITAN
> --
Regards,
Bin
Re: [PATCH 05/10] hw/arm/Kconfig: Add missing SDHCI symbol to FSL_IMX25
On Fri, May 14, 2021 at 12:56 AM Philippe Mathieu-Daudé
wrote:
>
> Commit bfae1772c43 ("hw/arm/fsl-imx25: Wire up eSDHC controllers")
> added a dependency on the TYPE_IMX_USDHC model, but forgot to add
> the Kconfig selector. Fix that to solve when built stand-alone:
>
> $ qemu-system-arm -M imx25-pdk
> qemu-system-arm: missing object type 'imx-usdhc'
> Aborted (core dumped)
>
> Fixes: bfae1772c43 ("hw/arm/fsl-imx25: Wire up eSDHC controllers")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Guenter Roeck
> ---
> hw/arm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Bin Meng
Re: [PATCH 04/10] hw/arm/Kconfig: Remove unused DS1338 symbol from i.MX25 PDK Board
On Fri, May 14, 2021 at 12:56 AM Philippe Mathieu-Daudé
wrote:
>
> In commit c4f00daa5b3 ("imx25-pdk: create ds1338 for qtest inside
> the test") we removed the DS1338 device from the i.MX25 machine
> but forgot to remove it in the machine Kconfig definitions, do
> it now.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/arm/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
Reviewed-by: Bin Meng
Re: [PATCH 03/10] hw/arm/Kconfig: Add missing dependency NPCM7XX -> SMBUS
On Fri, May 14, 2021 at 12:53 AM Philippe Mathieu-Daudé
wrote:
>
> The TYPE_NPCM7XX_SMBUS device model exposes an SMBus, but
> this isn't advertised with proper Kconfig symbol, leading
> to an early build failure when building NPCM7XX machines
> standalone:
>
> The following clauses were found for AT24C
>
> config AT24C depends on I2C
> select AT24C if NPCM7XX
>
> Fix by adding SMBUS to NPCM7XX.
>
> Fixes: 94e77879395 ("hw/i2c: Implement NPCM7XX SMBus Module Single Mode")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Doug Evans
> Cc: Hao Wu
> Cc: Tyrong Ting
> Cc: Corey Minyard
> ---
> hw/arm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Bin Meng
Re: RFC: Memory region accesses where .valid.min_access_size < .impl.min_access_size
On Thu, 13 May 2021, at 22:30, Jonathan Cameron wrote: > On Thu, 13 May 2021 14:36:27 +0200 > Philippe Mathieu-Daudé wrote: > > > On 5/13/21 2:23 PM, Peter Maydell wrote: > > > On Thu, 13 May 2021 at 12:49, Jonathan Cameron > > > wrote: > > >> My initial suggestion was to fix this by adding the relatively > > >> simple code needed in the driver to implement byte read / write, > > >> but Ben pointed at the QEMU docs - docs/devel/memory.rst which > > >> says > > >> " > > >> .impl.min_access_size, .impl.max_access_size define the access sizes > > >>(in bytes) supported by the *implementation*; other access sizes will > > >> be > > >>emulated using the ones available. For example a 4-byte write will be > > >>emulated using four 1-byte writes, if .impl.max_access_size = 1. > > >> " > > >> > > >> This isn't true when we have the situation where > > >> .valid.min_access_size < .imp.min_access_size > > >> > > >> So change the docs or try to make this work? > > > > See also this patch from Francisco: > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg636935.html > > > > And full unaligned access support from Andrew: > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg461247.html Much better to use lore.kernel.org: https://lore.kernel.org/qemu-devel/20170630030058.28943-1-and...@aj.id.au/ because... > > Thanks - that's very similar to what I was carrying, but I think it > only covers the read case. That's backed up by the comment: > /* XXX: Can't do this hack for writes */ It becomes easier to find Paolo's suggestion to fix that here: https://lore.kernel.org/qemu-devel/cd1aba90-176f-9ec6-3e2b-d1135156a...@redhat.com/ Would love to see this resolved! Unfortunately I haven't had the bandwidth to fix it all up for ... a long time now. Andrew
Re: [PATCH updated v2] spapr: Fix EEH capability issue on KVM guest for PCI passthru
On Thu, May 13, 2021 at 2:22 PM David Gibson
wrote:
>
> On Wed, May 05, 2021 at 08:18:27PM +0530, Mahesh Salgaonkar wrote:
> > With upstream kernel, especially after commit 98ba956f6a389
> > ("powerpc/pseries/eeh: Rework device EEH PE determination") we see that KVM
> > guest isn't able to enable EEH option for PCI pass-through devices anymore.
> >
> > [root@atest-guest ~]# dmesg | grep EEH
> > [0.032337] EEH: pSeries platform initialized
> > [0.298207] EEH: No capable adapters found: recovery disabled.
> > [root@atest-guest ~]#
> >
> > So far the linux kernel was assuming pe_config_addr equal to device's
> > config_addr and using it to enable EEH on the PE through ibm,set-eeh-option
> > RTAS call. Which wasn't the correct way as per PAPR. The linux kernel
> > commit 98ba956f6a389 fixed this flow. With that fixed, linux now uses PE
> > config address returned by ibm,get-config-addr-info2 RTAS call to enable
> > EEH option per-PE basis instead of per-device basis. However this has
> > uncovered a bug in qemu where ibm,set-eeh-option is treating PE config
> > address as per-device config address.
>
> Huh. To be fair, the stuff about this in PAPR is nearly
> incomprehensible, so we probably used what the kernel was doing as a
> guide instead.
I found the PAPR documentation made some sense after I learned how EEH
was handled on PCI(-X) systems. What's in Linux never made sense,
unfortunately.
> Hmm.. shouldn't we at least check that the supplied config_addr
> matches the one it should be for this PHB, rather than just ignoring
> it?
I think that'd cause issues with older kernels. Prior to the rework
mentioned by Mahesh (linux commit 98ba956f6a389 ("powerpc/pseries/eeh:
Rework device EEH PE determination")) the kernel would call
eeh-set-option for each device in the PE using the device's
config_address as the argument rather than the PE address. If we
return an error from eeh-set-option when the argument isn't a valid PE
address then older kernels will interpret that as EEH not being
supported. That really needs to be called out in a comment though.
Preferably with kernel version numbers, etc.
> ..and, looking back at rtas_ibm_get_config_addr_info2(), I think
> that looks wrong in the case of PCI bridges. AFAICT it gives an
> address that depends on the bus, but in other places we assume that
> the entire PHB is a single PE on the guest side, so it really
> shouldn't.
Yep, get_config_addr_info2 should map every device inside that PE to
the same PE address, even when they're on child busses. That said, I'm
not sure how well EEH works when there's a mix of real (vfio) and
emulated (qemu bridges) devices in the same PHB. Can VFIO pass through
a bridge?
Re: [PATCH] migration/rdma: Fix cm_event used before being initialized
On 14/05/2021 01.15, Dr. David Alan Gilbert wrote:
> * Li Zhijian (lizhij...@cn.fujitsu.com) wrote:
>> A segmentation fault was triggered when i try to abort a postcopy + rdma
>> migration.
>>
>> since rdma_ack_cm_event releases a uninitialized cm_event in thise case.
>>
>> like below:
>> 2496 ret = rdma_get_cm_event(rdma->channel, _event);
>> 2497 if (ret) {
>> 2498 perror("rdma_get_cm_event after rdma_connect");
>> 2499 ERROR(errp, "connecting to destination!");
>> 2500 rdma_ack_cm_event(cm_event); cause segmentation fault
>> 2501 goto err_rdma_source_connect;
>> 2502 }
>>
>> Signed-off-by: Li Zhijian
> OK, that's an easy fix then; but I wonder if we should perhaps remove
> that rdma_ack_cm_event, if it's the get_cm_event that's failed?
I also wondered, i checked the man page get_cm_event(3) which has not documented
and checked some rdma examples, some of them try to ack it[1], but some not[2].
[1]:
https://github.com/linux-rdma/rdma-core/blob/e381334c2915a5290565694947790d4aebaf/librdmacm/examples/mckey.c#L451
[2]:
https://github.com/linux-rdma/rdma-core/blob/e381334c2915a5290565694947790d4aebaf/librdmacm/examples/mckey.c#L342
Thanks
>
> Still,
>
>
> Reviewed-by: Dr. David Alan Gilbert
>
>> ---
>> migration/rdma.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index 00eac34232..2dadb62aed 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -2466,7 +2466,7 @@ static int qemu_rdma_connect(RDMAContext *rdma, Error
>> **errp)
>> .private_data = ,
>> .private_data_len = sizeof(cap),
>> };
>> -struct rdma_cm_event *cm_event;
>> +struct rdma_cm_event *cm_event = NULL;
>> int ret;
>>
>> /*
>> --
>> 2.30.2
>>
>>
>>
[Bug 1891748] Re: qemu-arm-static 5.1 can't run gcc
@aurent-vivier Checked. This value does not affect the bug, after changing it, I still get an error. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1891748 Title: qemu-arm-static 5.1 can't run gcc Status in QEMU: Fix Released Status in Juju Charms Collection: New Bug description: Issue discovered while trying to build pikvm (1) Long story short: when using qemu-arm-static 5.1, gcc exits whith message: Allocating guest commpage: Operation not permitted when using qemu-arm-static v5.0, gcc "works" Steps to reproduce will follow (1) https://github.com/pikvm/pikvm/blob/master/pages/building_os.md To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1891748/+subscriptions
Re: [PATCH v4 29/31] target/ppc: Implement cfuged instruction
On 5/13/21 7:24 AM, Matheus K. Ferst wrote:
+static bool trans_CFUGED(DisasContext *ctx, arg_X *a)
+{
+ REQUIRE_64BIT(ctx);
+ REQUIRE_INSNS_FLAGS2(ctx, ISA310);
+#if defined(TARGET_PPC64)
+ gen_helper_cfuged(cpu_gpr[a->ra], cpu_gpr[a->rt], cpu_gpr[a->rb]);
+#else
+ gen_invalid(ctx);
+#endif
+ return true;
+}
Given that this helper will also be used by vcfuged, there's no point in
hiding it in a TARGET_PPC64 block, and thus you can drop the ifdefs.
r~
If I remove it, the build for ppc will fail, because cpu_gpr is declared as
TCGv, and the helper uses i64 to match {get,set}_cpu_vsr{l,h}. REQUIRE_64BIT
makes the helper call unreachable for ppc, but it's a runtime check. At build
time, the compiler will check the types anyway, and give us an error.
Hmm, yes. Just change the gen_invalid above to qemu_build_not_reached().
r~
Re: [RFC PATCH 1/5] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
On Thu, May 13, 2021 at 05:20:06PM +0200, Paolo Bonzini wrote: > On 12/05/21 09:02, David Gibson wrote: > > > But with "imply" you could end up with a PSERIES that does not have NVDIMM > > > when also using --without-default-devices, couldn't you? Why don't you use > > > "select" instead of "imply" ? > > Oh.. clearly I misunderstand the semantics of "imply". If we don't > > need NVDIMM for PSERIES, why does there need to be any Kconfig > > connection between them at all? > > Because you still want it in the binary by default (i.e. unless > --without-default-devices). > > Basically, > > config PSERIES > imply NVDIMM > > is the same as > > config NVDIMM > default y if PSERIES Ah, ok, I get it now. "imply" is a terrible word for this, but ok. > Both of them are a way to say "PSERIES can work with NVDIMM so you want to > include it unless you want some fine tuning". In Linux "imply" is very > rarely used, while in QEMU it's quite common because it keeps the many > per-board defaults close together. > > Paolo > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [PATCH 01/10] hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on'
On Thu, May 13, 2021 at 06:38:49PM +0200, Philippe Mathieu-Daudé wrote: > Per the kconfig.rst: > > A device should be listed [...] ``imply`` if (depending on > the QEMU command line) the board may or may not be started > without it. > > This is the case with the NVDIMM device (it is certainly possible > to start a machine without NVDIMM) , so use the 'imply' weak > reverse dependency to select the symbol. > > Signed-off-by: Philippe Mathieu-Daudé ppc part Acked-by: David Gibson > --- > default-configs/devices/ppc64-softmmu.mak | 1 - > hw/arm/Kconfig| 1 + > hw/i386/Kconfig | 1 + > hw/mem/Kconfig| 2 -- > hw/ppc/Kconfig| 1 + > 5 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/default-configs/devices/ppc64-softmmu.mak > b/default-configs/devices/ppc64-softmmu.mak > index ae0841fa3a1..cca52665d90 100644 > --- a/default-configs/devices/ppc64-softmmu.mak > +++ b/default-configs/devices/ppc64-softmmu.mak > @@ -8,4 +8,3 @@ CONFIG_POWERNV=y > > # For pSeries > CONFIG_PSERIES=y > -CONFIG_NVDIMM=y > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index b887f6a5b17..67723d9ea6a 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -6,6 +6,7 @@ config ARM_VIRT > imply VFIO_PLATFORM > imply VFIO_XGMAC > imply TPM_TIS_SYSBUS > +imply NVDIMM > select ARM_GIC > select ACPI > select ARM_SMMUV3 > diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig > index 7f91f30877f..66838fa397b 100644 > --- a/hw/i386/Kconfig > +++ b/hw/i386/Kconfig > @@ -23,6 +23,7 @@ config PC > imply TPM_TIS_ISA > imply VGA_PCI > imply VIRTIO_VGA > +imply NVDIMM > select FDC > select I8259 > select I8254 > diff --git a/hw/mem/Kconfig b/hw/mem/Kconfig > index a0ef2cf648e..8b19fdc49f1 100644 > --- a/hw/mem/Kconfig > +++ b/hw/mem/Kconfig > @@ -7,6 +7,4 @@ config MEM_DEVICE > > config NVDIMM > bool > -default y > -depends on (PC || PSERIES || ARM_VIRT) > select MEM_DEVICE > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig > index e51e0e5e5ac..66e0b15d9ef 100644 > --- a/hw/ppc/Kconfig > +++ b/hw/ppc/Kconfig > @@ -3,6 +3,7 @@ config PSERIES > imply PCI_DEVICES > imply TEST_DEVICES > imply VIRTIO_VGA > +imply NVDIMM > select DIMM > select PCI > select SPAPR_VSCSI -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [RFC PATCH 4/5] hw/ppc/fdt: Drop dependency on libfdt
On Thu, May 13, 2021 at 05:26:37PM +0200, Paolo Bonzini wrote: > On 13/05/21 05:46, David Gibson wrote: > > > The patch makes sense in general. The file is only needed for pseries and > > > powernv, not for e.g. e500 which does need fdt. > > > > Yes, agreed. > > > > > I would get rid of FDT_PPC completely. First, before patch 3, you can > > > move > > > fdt.c to PSERIES and POWERNV (it's too small to need its own Kconfig > > > symbol) > > > and only leave > > > > > > ppc_ss.add(when: 'CONFIG_FDT_PPC', if_true: fdt) > > > > Uh... why do we need even this? > > To tell Meson that a board requires QEMU to be linked with libfdt. This > symbol is then renamed to CONFIG_LIBFDT once it can be used with all targets > (rather than just hw/ppc). Oh, I thought CONFIG_LIBFDT already did that. > > > Since you are at it, remove the silly #ifdef TARGET_PPC64 in the > > > hw/ppc/fdt.c file. > > > > > > Then in patch 3: > > > > > > - add to Kconfig.host > > > > > > config FDT > > > bool > > > > > > config LIBFDT > > > bool > > > depends on FDT > > > > Um.. I'm not sure what semantic difference you're envisaging between > > FDT and LIBFDT. > > "FDT" is set by meson.build if the library is available, LIBFDT is set by > the board to link with the library. In other words CONFIG_FDT is per-build, > while CONFIG_LIBFDT is per-target. Oof... that's highly non-obvious. Could we call it HAVE_LIBFDT and USE_LIBFDT instead? > If a board selects LIBFDT but the library is not available, minikconf will > report a contradiction due to "CONFIG_PSERIES=y" -> "config PSERIES select > LIBFDT" -> "config LIBFDT depends on FDT" -> "CONFIG_FDT=n". > > > > - for all the boards I listed in my review, add "select LIBFDT" in > > > addition > > > to "depends on FDT". > > This is actually unnecessary---"depends on FDT" is not needed in the boards > because LIBFDT already has the dependency. > > Paolo > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [PATCH 08/10] hw/ppc/Kconfig: Add missing dependency E500 -> DS1338 RTC
On Thu, May 13, 2021 at 06:38:56PM +0200, Philippe Mathieu-Daudé wrote:
> Commit 7abb479c7ab ("PPC: E500: Add FSL I2C controller and integrate
> RTC with it") added a global dependency on the DS1338 model, instead
> of a machine one (via Kconfig). This gives trouble when building
> standalone machines not exposing I2C bus:
>
> The following clauses were found for DS1338
>
> CONFIG_DS1338=y
> config DS1338 depends on I2C
>
> Fix by selecting the DS1338 symbol in the single machine requiring
> it, the E500.
>
> Fixes: 7abb479c7ab ("PPC: E500: Add FSL I2C controller and integrate RTC with
> it")
> Signed-off-by: Philippe Mathieu-Daudé
Acked-by: David Gibson
> ---
> Cc: Amit Singh Tomar
> Cc: Andrew Randrianasulu
> ---
> default-configs/devices/ppc-softmmu.mak | 1 -
> hw/ppc/Kconfig | 1 +
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/default-configs/devices/ppc-softmmu.mak
> b/default-configs/devices/ppc-softmmu.mak
> index c2d41198cde..5a7f99d1e45 100644
> --- a/default-configs/devices/ppc-softmmu.mak
> +++ b/default-configs/devices/ppc-softmmu.mak
> @@ -1,7 +1,6 @@
> # Default configuration for ppc-softmmu
>
> # For embedded PPCs:
> -CONFIG_DS1338=y
> CONFIG_E500=y
> CONFIG_PPC405=y
> CONFIG_PPC440=y
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 66e0b15d9ef..2e4c56eb770 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -128,6 +128,7 @@ config E500
> select SERIAL
> select MPC_I2C
> select FDT_PPC
> +select DS1338
>
> config VIRTEX
> bool
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH 00/72] Convert floatx80 and float128 to FloatParts
On 5/13/21 8:33 AM, Alex Bennée wrote: Now add and mul columns are going down when the only change is to muladd? Is this just more noise? Running again more times I think it is a real effect: I don't believe it. If source code for a given function is not changing then the generated code should not change (much, especially with FLATTEN), and thus the runtime should not change (much). Are you absolutely sure that you're measuring what you think you are measuring? Is your compiler mis-behaving somehow and not inlining stuff? r~
Re: [PATCH 09/11] include/exec: added functions to the stubs in exec-all.h
On 5/13/21 9:03 AM, Lucas Mateus Martins Araujo e Castro wrote: tlb_set_page is called by many ppc_hash64_handle_mmu_fault, ppc_radix64_handle_mmu_fault and ppc_hash32_handle_mmu_fault, all of which from what I've seen are only used inside #if defined(CONFIG_SOFTMMU). tlb_set_page should only be called from one place: ppc_cpu_tlb_fill. The other functions should fill in data, much like get_physical_address. So what is the best way to deal with these tlb_set_page calls? Should these part of the _handle_mmu_fault functions never be reached or should these functions never be called? There is some duplication between get_physical_address* and *handle_mmu_fault that should be fixed. What should be happening is that you have one function (per mmu type) that takes a virtual address and resolves a physical address. This bit of code should be written so that it is usable by both CPUClass.get_phys_page_attrs_debug and TCGCPUOps.tlb_fill. It appears as if ppc_radix64_xlate is the right interface for this. It appears that real mode handling is duplicated between hash64 and radix64, which could be unified. You should only call tlb_set_page from TCGCPUOps.tlb_fill, aka ppc_cpu_tlb_fill. TCGCPUOps.tlb_fill is obviously TCG only. The version you are looking at here is system emulation specific (sysemu, !defined(CONFIG_USER_ONLY)). There is a second version of this function, with the same signature, that is used for user emulation in the helpfully named user_only_helper.c. r~
Re: [PATCH 07/11] target/ppc: added KVM fallback to fpscr manipulation
On 5/13/21 11:36 AM, Bruno Piazera Larsen wrote:
On 12/05/2021 15:20, Richard Henderson wrote:
On 5/12/21 9:08 AM, Bruno Larsen (billionai) wrote:
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 104a308abb..a8a720eb48 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2947,3 +2947,17 @@ bool kvm_arch_cpu_check_are_resettable(void)
{
return true;
}
+
+void kvmppc_store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask)
+{
+ CPUState *cs = env_cpu(env);
+ struct kvm_one_reg reg;
+ int ret;
+ reg.id = KVM_REG_PPC_FPSCR;
+ reg.addr = (uintptr_t) >fpscr;
+ ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ if (ret < 0)
+ {
+ fprintf(stderr, "Unable to set FPSCR to KVM: %s", strerror(errno));
+ }
+}
This is all unnecessary. All you need to do is store to env->fpscr and the
value will be synced back with kvm_put_fp.
I'll note that some of the trouble you may be having with extracting
helper_store_fpscr to a ppc_store_fpscr function is due to an existing bug in
the tcg code:
Storing to fpscr should *never* raise an exception -- see MTFSF, MTFSB0,
MTFSB1. Thus the mucking about with cs->exception_index and env->error_code
is incorrect.
In addition, the masking is being done weirdly and could use a complete
overhaul.
This could all be rewritten as
-- %< -- cpu.h
/* Invalid operation exception summary */
- #define fpscr_ix ((env->fpscr) & ((1 << FPSCR_VXSNAN) ...
+ #define FPSCR_IX ((1 << FPSCR_VXSNAN) | ...)
-- %< -- cpu.c
// move fpscr_set_rounding_mode here
void ppc_store_fpscr(CPUPPCState *env, target_ulong val)
{
/* Recompute exception summary state. */
val &= ~(FP_VX | FP_FEX);
if (val & FPSCR_IX) {
val |= FP_VX;
}
if ((val >> FPSCR_XX) & (val >> FPSCR_XE) & 0x1f) {
val |= FP_FEX;
}
env->fpscr = val;
if (tcg_enabled()) {
fpscr_set_rounding_mode(env);
}
}
-- %< -- fpu_helper.c
void helper_store_fpscr(CPUPPCState *env, target_ulong val,
uint32_t nibbles)
{
target_ulong mask = 0;
/* TODO: Push this expansion back to translation time. */
for (int i = 0; i < sizeof(target_ulong) * 2; ++i) {
if (nibbles & (1 << i)) {
mask |= (target_ulong)0xf << (4 * i);
}
}
val = (val & mask) | (env->fpscr & ~mask);
ppc_store_fpscr(env, val);
}
That expansion can't be moved to translation time, because gdbstub would also
need that code in a variety of functions, so better to keep it in that central
location,
void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
{
uint32_t mask = 1u << bit;
if (env->fpscr & mask) {
ppc_store_fpscr(env, env->fpscr & ~mask);
}
}
void helper_fpscr_setbit(CPUPPCState *env, uint32_t bit)
{
uint32_t mask = 1u << bit;
if (!(env->fpscr & mask)) {
ppc_store_fpscr(env, env->fpscr | mask);
}
}
There are a couple of other uses of fpscr_set_rounding_mode, where the
softfloat value is changed temporarily (do_fri, VSX_ROUND). These should
simply save the previous softfloat value (using get_float_rounding_mode)
around the operation instead of re-computing from fpscr.
Which leaves us with exactly one use of fpscr_set_rounding_mode, which can
then be moved to cpu.c next to ppc_store_fpscr.
r~
I was implementing this solution, but ran into a problem: We needed store_fpscr
for gdbstub.c, that's the original reason we made that new function to begin
with. This solution, although it improves the handling of fpscr, doesn't fix
the original problem.
Why not? Did you miss the cpu.c cut at the very top?
What I think we can do is put the logic that is in helper_store_fpscr into
store_fpscr, move it to cpu.c, and have the helper call the non-helper
function. That way we conserve helper_* as TCG-specific and have the overhaul.
That is exactly what I have written above.
r~
[GIT PULL] I2C changes for next qemu release
This is the addition of a new devices used in a lot of BMCs, and adds the infrastructure so other devices of it's type can be added. I will update the Changelog as necessary. This was through several revisions on the mailing list recently. Thanks, -corey The following changes since commit 8fe9f1f891eff4e37f82622b7480ee748bf4af74: Update version for v6.0.0-rc3 release (2021-04-14 22:06:18 +0100) are available in the Git repository at: https://github.com/cminyard/qemu.git tags/for-qemu-6.1-v1 for you to fetch changes up to 065177eeceff552a5316bff9435188b50a2da1b6: hw/i2c: add pca954x i2c-mux switch (2021-04-15 07:10:39 -0500) Add a bus multiplexer device This patch set adds a bus multiplexer and the necessary infrastructure in the I2C code to allow it to work. These are common on systems with lots of I2C devices, like an IPMI BMC. Patrick Venture (4): hw/i2c: name I2CNode list in I2CBus hw/i2c: add match method for device search hw/i2c: move search to i2c_scan_bus method hw/i2c: add pca954x i2c-mux switch MAINTAINERS | 6 + hw/i2c/Kconfig | 4 + hw/i2c/core.c| 55 ++-- hw/i2c/i2c_mux_pca954x.c | 290 +++ hw/i2c/meson.build | 1 + hw/i2c/trace-events | 5 + include/hw/i2c/i2c.h | 17 ++- include/hw/i2c/i2c_mux_pca954x.h | 19 +++ 8 files changed, 383 insertions(+), 14 deletions(-) create mode 100644 hw/i2c/i2c_mux_pca954x.c create mode 100644 include/hw/i2c/i2c_mux_pca954x.h
Re: [PATCH v3 06/33] util/async: aio_co_schedule(): support reschedule in same ctx
On 12/05/21 09:15, Vladimir Sementsov-Ogievskiy wrote:
I don't understand. Why doesn't aio_co_enter go through the ctx !=
qemu_get_current_aio_context() branch and just do aio_co_schedule?
That was at least the idea behind aio_co_wake and aio_co_enter.
Because ctx is exactly qemu_get_current_aio_context(), as we are not in
iothread but in nbd connection thread. So,
qemu_get_current_aio_context() returns qemu_aio_context.
So the problem is that threads other than the main thread and
the I/O thread should not return qemu_aio_context. The vCPU thread
may need to return it when running with BQL taken, though.
Something like this (untested):
diff --git a/include/block/aio.h b/include/block/aio.h
index 5f342267d5..10fcae1515 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -691,10 +691,13 @@ void aio_co_enter(AioContext *ctx, struct Coroutine *co);
* Return the AioContext whose event loop runs in the current thread.
*
* If called from an IOThread this will be the IOThread's AioContext. If
- * called from another thread it will be the main loop AioContext.
+ * called from the main thread or with the "big QEMU lock" taken it
+ * will be the main loop AioContext.
*/
AioContext *qemu_get_current_aio_context(void);
+void qemu_set_current_aio_context(AioContext *ctx);
+
/**
* aio_context_setup:
* @ctx: the aio context
diff --git a/iothread.c b/iothread.c
index 7f086387be..22b967e77c 100644
--- a/iothread.c
+++ b/iothread.c
@@ -39,11 +39,23 @@ DECLARE_CLASS_CHECKERS(IOThreadClass, IOTHREAD,
#define IOTHREAD_POLL_MAX_NS_DEFAULT 0ULL
#endif
-static __thread IOThread *my_iothread;
+static __thread AioContext *my_aiocontext;
+
+void qemu_set_current_aio_context(AioContext *ctx)
+{
+assert(!my_aiocontext);
+my_aiocontext = ctx;
+}
AioContext *qemu_get_current_aio_context(void)
{
-return my_iothread ? my_iothread->ctx : qemu_get_aio_context();
+if (my_aiocontext) {
+return my_aiocontext;
+}
+if (qemu_mutex_iothread_locked()) {
+return qemu_get_aio_context();
+}
+return NULL;
}
static void *iothread_run(void *opaque)
@@ -56,7 +68,7 @@ static void *iothread_run(void *opaque)
* in this new thread uses glib.
*/
g_main_context_push_thread_default(iothread->worker_context);
-my_iothread = iothread;
+qemu_set_current_aio_context(iothread->ctx);
iothread->thread_id = qemu_get_thread_id();
qemu_sem_post(>init_done_sem);
diff --git a/stubs/iothread.c b/stubs/iothread.c
index 8cc9e28c55..25ff398894 100644
--- a/stubs/iothread.c
+++ b/stubs/iothread.c
@@ -6,3 +6,7 @@ AioContext *qemu_get_current_aio_context(void)
{
return qemu_get_aio_context();
}
+
+void qemu_set_current_aio_context(AioContext *ctx)
+{
+}
diff --git a/tests/unit/iothread.c b/tests/unit/iothread.c
index afde12b4ef..cab38b3da8 100644
--- a/tests/unit/iothread.c
+++ b/tests/unit/iothread.c
@@ -30,13 +30,26 @@ struct IOThread {
bool stopping;
};
-static __thread IOThread *my_iothread;
+static __thread AioContext *my_aiocontext;
+
+void qemu_set_current_aio_context(AioContext *ctx)
+{
+assert(!my_aiocontext);
+my_aiocontext = ctx;
+}
AioContext *qemu_get_current_aio_context(void)
{
-return my_iothread ? my_iothread->ctx : qemu_get_aio_context();
+if (my_aiocontext) {
+return my_aiocontext;
+}
+if (qemu_mutex_iothread_locked()) {
+return qemu_get_aio_context();
+}
+return NULL;
}
+
static void iothread_init_gcontext(IOThread *iothread)
{
GSource *source;
@@ -54,7 +67,7 @@ static void *iothread_run(void *opaque)
rcu_register_thread();
-my_iothread = iothread;
+qemu_set_current_aio_context(iothread->ctx);
qemu_mutex_lock(>init_done_lock);
iothread->ctx = aio_context_new(_abort);
diff --git a/util/main-loop.c b/util/main-loop.c
index d9c55df6f5..4ae5b23e99 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -170,6 +170,7 @@ int qemu_init_main_loop(Error **errp)
if (!qemu_aio_context) {
return -EMFILE;
}
+qemu_set_current_aio_context(qemu_aio_context);
qemu_notify_bh = qemu_bh_new(notify_event_cb, NULL);
gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
src = aio_get_g_source(qemu_aio_context);
If it works for you, I can post it as a formal patch.
Paolo
Re: [Qemu-devel] [PATCH] MAINTAINERS: convert to JSON
FYI, I had to setup Thunderbird to reply to you because Gmail web interface was crashing when I was trying to expand the e-mail to delete the code. So, as Peter mentioned, a nice April fools' joke. On 5/13/21 3:32 PM, Philippe Mathieu-Daudé wrote: Cc'ing John/Willian/Wainer I think I understand why you brought this thread back now. Finding a way to correlate maintainers and tags is, indeed, a nice feature that can help with your idea about correlating tags and maintainers to use on tests. The problem I see is that JSON is a good machine-readable file. It is a pain to edit it manually. So, despite the April fools' joke, my suggestion would be to create the JSON or any other machine-readable file from the MAINTAINERS file using a script. On 4/1/19 1:29 PM, Paolo Bonzini wrote: JSON is already in wide use within QEMU for QMP and QAPI, enable wider usage of MAINTAINERS by applying a well-defined machine-readable format. Converting scripts/getmaintainers.pl is left for next year. Year-base contribution!
Re: [Virtio-fs] [PATCH 7/7] virtiofsd: Set req->reply_sent right after sending reply
On 5/11/21 4:37 PM, Vivek Goyal wrote:
> There is no reason to set it in label "err". We should be able to set
> it right after sending reply. It is easier to read.
>
> Signed-off-by: Vivek Goyal
> ---
> tools/virtiofsd/fuse_virtio.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> index aa53808ef9..b1767dd5b9 100644
> --- a/tools/virtiofsd/fuse_virtio.c
> +++ b/tools/virtiofsd/fuse_virtio.c
> @@ -446,12 +446,9 @@ int virtio_send_data_iov(struct fuse_session *se, struct
> fuse_chan *ch,
> vu_queue_notify(dev, q);
> pthread_mutex_unlock(>vq_lock);
> vu_dispatch_unlock(qi->virtio_dev);
> +req->reply_sent = true;
>
> err:
Just a really minor comment: after all these changes, I would venture
that "out" is a more appropriate label name than "err" at this point.
> -if (ret == 0) {
> -req->reply_sent = true;
> -}
> -
> return ret;
> }
>
>
Re: [Virtio-fs] [PATCH 0/7] virtiofsd: Few cleanups in virtio_send_data_iov()
On 5/11/21 4:37 PM, Vivek Goyal wrote: > Hi, > > Code in virtio_send_data_iov() little twisted and complicated. This > patch series just tries to simplify it a bit to make it little easier > to read this piece of code. > > Thanks > Vivek > > Vivek Goyal (7): > virtiofsd: Check for EINTR in preadv() and retry > virtiofsd: Get rid of unreachable code in read > virtiofsd: Use iov_discard_front() to skip bytes > virtiofsd: get rid of in_sg_left variable > virtiofsd: Simplify skip byte logic > virtiofsd: Check EOF before short read > virtiofsd: Set req->reply_sent right after sending reply > > tools/virtiofsd/fuse_virtio.c | 67 +++ > 1 file changed, 21 insertions(+), 46 deletions(-) > With the codestyle fix to appease the bot: Reviewed-by: Connor Kuehl (For the series)
Re: [PATCH V3 00/22] Live Update
I will add to MAINTAINERS incrementally instead of at the end to make checkpatch happy. I will use qemu_strtol even though I thought the message "consider using qemu_strtol" was giving me a choice. You can't fight The Man when the man is a robot. - Steve On 5/7/2021 9:00 AM, no-re...@patchew.org wrote: > Patchew URL: > https://patchew.org/QEMU/1620390320-301716-1-git-send-email-steven.sist...@oracle.com/ > > Hi, > > This series seems to have some coding style problems. See output below for > more information: > > Type: series > Message-id: 1620390320-301716-1-git-send-email-steven.sist...@oracle.com > Subject: [PATCH V3 00/22] Live Update > > === TEST SCRIPT BEGIN === > #!/bin/bash > git rev-parse base > /dev/null || exit 0 > git config --local diff.renamelimit 0 > git config --local diff.renames True > git config --local diff.algorithm histogram > ./scripts/checkpatch.pl --mailback base.. > === TEST SCRIPT END === > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > From https://github.com/patchew-project/qemu > * [new tag] > patchew/1620390320-301716-1-git-send-email-steven.sist...@oracle.com -> > patchew/1620390320-301716-1-git-send-email-steven.sist...@oracle.com > - [tag update] patchew/20210504124140.1100346-1-li...@roeck-us.net -> > patchew/20210504124140.1100346-1-li...@roeck-us.net > - [tag update] patchew/20210506185641.284821-1-dgilb...@redhat.com -> > patchew/20210506185641.284821-1-dgilb...@redhat.com > - [tag update] patchew/20210506193341.140141-1-lviv...@redhat.com -> > patchew/20210506193341.140141-1-lviv...@redhat.com > - [tag update] patchew/20210506194358.3925-1-peter.mayd...@linaro.org > -> patchew/20210506194358.3925-1-peter.mayd...@linaro.org > Switched to a new branch 'test' > 8c778e6 simplify savevm > aca4f09 cpr: maintainers > 697f8d0 cpr: only-cpr-capable option > 0a8c20e chardev: cpr for sockets > cb270f4 chardev: cpr for pty > 279230e chardev: cpr for simple devices > b122cfa chardev: cpr framework > 6596676 hostmem-memfd: cpr support > 8cb6348 vhost: reset vhost devices upon cprsave > e3ae86d vfio-pci: cpr part 2 > 02c628d vfio-pci: cpr part 1 > d93623c vfio-pci: refactor for cpr > bc63b3e pci: export functions for cpr > 2b10bdd cpr: HMP interfaces > 29bc20a cpr: QMP interfaces > 3f84e6c cpr > 0a32588 vl: add helper to request re-exec > 466b4cf machine: memfd-alloc option > 50c3e84 util: env var helpers > 76c3550 oslib: qemu_clr_cloexec > d819bd4 qemu_ram_volatile > c466ddf as_flat_walk > > === OUTPUT BEGIN === > 1/22 Checking commit c466ddfd2209 (as_flat_walk) > 2/22 Checking commit d819bd4dcc09 (qemu_ram_volatile) > 3/22 Checking commit 76c3550a677b (oslib: qemu_clr_cloexec) > 4/22 Checking commit 50c3e84cf5a6 (util: env var helpers) > Use of uninitialized value $acpi_testexpected in string eq at > ./scripts/checkpatch.pl line 1529. > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #19: > new file mode 100644 > > ERROR: consider using qemu_strtol in preference to strtol > #72: FILE: util/env.c:20: > +res = strtol(val, 0, 10); > > total: 1 errors, 1 warnings, 129 lines checked > > Patch 4/22 has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > 5/22 Checking commit 466b4cf4ce8c (machine: memfd-alloc option) > 6/22 Checking commit 0a32588de76e (vl: add helper to request re-exec) > 7/22 Checking commit 3f84e6c38bd6 (cpr) > Use of uninitialized value $acpi_testexpected in string eq at > ./scripts/checkpatch.pl line 1529. > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #55: > new file mode 100644 > > total: 0 errors, 1 warnings, 314 lines checked > > Patch 7/22 has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > 8/22 Checking commit 29bc20ab5870 (cpr: QMP interfaces) > Use of uninitialized value $acpi_testexpected in string eq at > ./scripts/checkpatch.pl line 1529. > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #81: > new file mode 100644 > > total: 0 errors, 1 warnings, 136 lines checked > > Patch 8/22 has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > 9/22 Checking commit 2b10bdd5edb3 (cpr: HMP interfaces) > 10/22 Checking commit bc63b3edc621 (pci: export functions for cpr) > 11/22 Checking commit d93623c4da4d (vfio-pci: refactor for cpr) > 12/22 Checking commit 02c628d50b57 (vfio-pci: cpr part 1) > Use of uninitialized value $acpi_testexpected in string eq at > ./scripts/checkpatch.pl line 1529. > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #271: > new file mode 100644 > > total: 0 errors, 1 warnings, 566 lines checked > > Patch 12/22 has style problems, please review. If any of these errors >
Re: [PULL v2 00/32] Misc (mostly i386) patches for 2021-05-11
On 13/05/21 21:12, Peter Maydell wrote: Something weird has happened here. I think I applied v1 of this pullreq (merge commit 31589644ba069ba06c5 now in master), and then when I came along to try to process this one the tag was an entirely different set of commits. Can you check whether what is in master is what you intended, and send a revert/fixup patch if necessary, please? Yes, it is. You processed the tag that had already moved to v2, but replied to the buggy v1. Paolo
Re: [PATCH V3 07/22] cpr
On 5/12/2021 12:19 PM, Stefan Hajnoczi wrote: > On Fri, May 07, 2021 at 05:25:05AM -0700, Steve Sistare wrote: >> To use the restart mode, qemu must be started with the memfd-alloc machine >> option. The memfd's are saved to the environment and kept open across exec, >> after which they are found from the environment and re-mmap'd. Hence guest >> ram is preserved in place, albeit with new virtual addresses in the qemu >> process. The caller resumes the guest by calling cprload, which loads >> state from the file. If the VM was running at cprsave time, then VM >> execution resumes. cprsave supports any type of guest image and block >> device, but the caller must not modify guest block devices between cprsave >> and cprload. > > Does QEMU's existing -object memory-backend-file on tmpfs or hugetlbfs > achieve the same thing? Not quite. Various secondary anonymous memory objects are allocated via ram_block_add and must be preserved, such as these on x86_64. vga.vram pc.ram pc.bios pc.rom vga.rom rom@etc/acpi/tables rom@etc/table-loader rom@etc/acpi/rsdp Even the read-only areas must be preserved rather than recreated from files in the updated qemu, as their contents may have changed. - Steve
Re: [PATCH V3 00/22] Live Update
On 5/12/2021 12:42 PM, Stefan Hajnoczi wrote: > On Fri, May 07, 2021 at 05:24:58AM -0700, Steve Sistare wrote: >> Provide the cprsave and cprload commands for live update. These save and >> restore VM state, with minimal guest pause time, so that qemu may be updated >> to a new version in between. >> >> cprsave stops the VM and saves vmstate to an ordinary file. It supports two >> modes: restart and reboot. For restart, cprsave exec's the qemu binary (or >> /usr/bin/qemu-exec if it exists) with the same argv. qemu restarts in a >> paused state and waits for the cprload command. > > I think cprsave/cprload could be generalized by using QMP to stash the > file descriptors. The 'getfd' QMP command already exists and QEMU code > already opens fds passed using this mechanism. > > I haven't checked but it may be possible to drop some patches by reusing > QEMU's monitor file descriptor passing since the code already knows how > to open from 'getfd' fds. > > The reason why using QMP is interesting is because it eliminates the > need for execve(2). QEMU may be unable to execute a program due to > chroot, seccomp, etc. > > QMP would enable cprsave/cprload to work both with and without > execve(2). > > One tricky thing with this approach might be startup ordering: how to > get fds via the QMP monitor in the new process before processing the > entire command-line. Early on I experimented with a similar approach. Old qemu passed descriptors to an escrow process and exited; new qemu started and retrieved the descriptors from escrow. vfio mostly worked after I hacked the kernel to suppress the original-pid owner check. I suspect my recent vfio extensions would smooth the rough edges. However, the main issue is that guest ram must be backed by named shared memory, and we would need to add code to support shared memory for all the secondary memory objects. That makes it less interesting for us at this time; we care about updating legacy qemu instances with anonymous guest memory. Having said all that, this would be an interesting project, just not the one I want to push now. In the future we could add a new cprsave mode to support it in a backward compatible manner. - Steve
Re: [PATCH V3 16/22] chardev: cpr framework
On 5/7/2021 10:33 AM, Eric Blake wrote: > On 5/7/21 7:25 AM, Steve Sistare wrote: >> Add QEMU_CHAR_FEATURE_CPR for devices that support cpr. >> Add the chardev close_on_cpr option for devices that can be closed on cpr >> and reopened after exec. >> cpr is allowed only if either QEMU_CHAR_FEATURE_CPR or close_on_cpr is set >> for all chardevs in the configuration. >> >> Signed-off-by: Steve Sistare >> --- > >> +++ b/qapi/char.json >> @@ -204,12 +204,15 @@ >> # @logfile: The name of a logfile to save output >> # @logappend: true to append instead of truncate >> # (default to false to truncate) >> +# @close-on-cpr: if true, close device's fd on cprsave. defaults to false. >> +#since 6.0. > > 6.1, actually. > >> @@ -3182,6 +3196,10 @@ The general form of a character device option is: >> ``logappend`` option controls whether the log file will be truncated >> or appended to when opened. >> >> +Every backend supports the ``close-on-cpr`` option. If on, the >> +devices's descriptor is closed during cprsave, and reopened after exec. > > device's Thanks, will fix both - Steve
Re: [PATCH V3 06/22] vl: add helper to request re-exec
On 5/12/2021 12:27 PM, Stefan Hajnoczi wrote:
> On Fri, May 07, 2021 at 05:25:04AM -0700, Steve Sistare wrote:
>> @@ -660,6 +673,16 @@ void qemu_system_debug_request(void)
>> qemu_notify_event();
>> }
>>
>> +static void qemu_exec(void)
>> +{
>> +const char *helper = "/usr/bin/qemu-exec";
>
> The network up script is get_relocated_path(CONFIG_SYSCONFDIR
> "/qemu-ifup"). For consistency maybe this should use the same path
> rather than /usr/bin/.
CONFIG_QEMU_HELPERDIR=/usr/libexec looks like a good choice.
And maybe rename qemu-exec to qemu-exec-helper, analogous to qemu-bridge-helper.
- Steve
Re: [PATCH V3 06/22] vl: add helper to request re-exec
On 5/7/2021 10:31 AM, Eric Blake wrote:
> On 5/7/21 7:25 AM, Steve Sistare wrote:
>> Add a qemu_exec_requested() hook that causes the main loop to exit and
>> re-exec qemu using the same initial arguments. If /usr/bin/qemu-exec
>> exists, exec that instead. This is an optional site-specific trampoline
>> that may alter the environment before exec'ing the qemu binary.
>>
>> Signed-off-by: Steve Sistare
>> ---
>
>> +static void qemu_exec(void)
>> +{
>> +const char *helper = "/usr/bin/qemu-exec";
>> +const char *bin = !access(helper, X_OK) ? helper : argv_main[0];
>
> Reads awkwardly; I would have used '...= access(helper, X_OK) == 0 ?...'
Will fix.
>> +
>> +execvp(bin, argv_main);
>> +error_report("execvp failed, errno %d.", errno);
>
> error_report should not be used with a trailing dot.
Will fix. I was not sure because I see examples both ways, though no dot
prevails.
Perhaps it should be added to the style guide and checkpatch.
> Also, %d for errno is awkward, better is:
>
> error_report("execvp failed: %s", strerror(errno));
I shy away from strerror because it is not thread safe, but I see qemu uses it
extensively. Will fix.
>
>> +exit(1);
>
> We aren't consistent about use of EXIT_FAILED.
OK, I will use EXIT_FAILURE.
Thanks for reviewing.
- Steve
Re: [PATCH v2 0/6] hw/southbridge: QOM'ify vt82c686 as VT82C686B_SOUTHBRIDGE
On 5/13/21 1:54 PM, BALATON Zoltan wrote: > On Thu, 13 May 2021, Philippe Mathieu-Daudé wrote: >> On 5/11/21 3:09 PM, BALATON Zoltan wrote: >>> On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote: Hi Zoltan, On 5/11/21 1:28 PM, BALATON Zoltan wrote: > On Tue, 11 May 2021, Philippe Mathieu-Daudé wrote: >> The motivation behind this series is to remove the >> isa_get_irq(NULL) call to simplify the ISA generic model. >> >> Since v1: >> - rebased on top of remotes/dg-gitlab/tags/ppc-for-6.1-20210504 > > I'll try to have a look at these later but some notes: The pegasos2 > changes are now in master so if this was before that maybe rebasing on > master is now enough. This is what this series does, simply rebase on top of your merged patches. > However I wonder if any changes to pegasos2.c is > needed due to changed init of the chip model or is that only affecting > 82c686b? There is no change in 'init' in this series, it is only QOM boilerplate code churn, no logical change intended. > Please also note that pegasos2 is not enabled by default due to > needing undistributable firmware ROM so to test it you need to > enable it > in default-configs/devices/ppc-softmmu.mak I remember you said you were mostly interested in the VT8231, not the VT82C686. This series only QOM'ify the latter. >>> >>> OK as I said I haven't looked at it in detail. >>> What is your idea? Send the firmware off-list and explain how the OS works and how (what) to test? >>> >>> I've already sent you this info: >>> >>> https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html >> >> Well, if you have everything setup, it is easier to test and send >> a Tested-by tag. > > I indend to test it when I'll have some time but I could not get to it yet. > >>> but I can't write a test case so if you want to automate this and make >>> it part of QEMU tests then some help with that would be appreciated. >> >> You are not the only want wanting that. I'm working on a solution to run >> such tests (depending on binary blobs) in your own namespace, but it >> will take me time (doing it in my free time, without help). > > I did not mean to say you should do this urgently, just sent this as a > reminder about how this could be tested in case you forgot because > you've asked about testing. Unrelated to this series, with master (dab59ce0312) I sometime get: Initializing KBD...0012 FAILED. and then the mouse isn't working. Sometimes: Initializing KBD... Done. and the mouse is crazy (similar to my host mouse). Anyway, there is smth wrong with patch #2 of this series: "Simplify removing unuseful qemu_allocate_irqs() call".
Re: [PATCH] tests/qtest/fuzz: Fix build failure
On 210513 1820, Philippe Mathieu-Daudé wrote: > On Fedora 32, using clang (version 10.0.1-3.fc32) we get: > > tests/qtest/fuzz/fuzz.c:237:5: error: implicit declaration of function > 'qemu_init' is invalid in C99 [-Werror,-Wimplicit-function-declaration] > qemu_init(result.we_wordc, result.we_wordv, NULL); > ^ > > qemu_init() is declared in "sysemu/sysemu.h", include this > header to fix. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alexander Bulekov Thank you
Re: [PATCH v6 00/82] target/arm: Implement SVE2
On Fri, 30 Apr 2021 at 21:26, Richard Henderson
wrote:
>
> Based-on: 20210430132740.10391-1-peter.mayd...@linaro.org
> ("target/arm: Split translate-*.c.inc into separate compilation units")
>
> Based-on: 20210427214108.88503-1-richard.hender...@linaro.org
> ("linux-user/aarch64: Enable hwcap for RND, BTI, and MTE")
>
> Since I believe Peter has both queued on his target-arm.next,
> and I didn't want to get in the way of his bulk code movement.
>
> Changes for v6:
> * Rebase as per above.
> * Rename isar_feature_sve2_{i8mm,f32mm,f64mm} to sve_*,
> because these extensions do not depend on sve2.
> * Update hwcap for the many extensions.
I think I'm done with reviewing this series now...
-- PMM
Re: [PATCH v6 80/82] target/arm: Implement integer matrix multiply accumulate
On Fri, 30 Apr 2021 at 22:36, Richard Henderson
wrote:
>
> This is {S,U,US}MMLA for both AArch64 AdvSIMD and SVE,
> and V{S,U,US}MMLA.S8 for AArch32 NEON.
>
> Signed-off-by: Richard Henderson
> ---
> target/arm/helper.h | 7
> target/arm/neon-shared.decode | 7
> target/arm/sve.decode | 6 +++
> target/arm/translate-a64.c| 18
> target/arm/translate-neon.c | 27
> target/arm/translate-sve.c| 27
> target/arm/vec_helper.c | 77 +++
> 7 files changed, 169 insertions(+)
I have to say the decode parts for SVE and A32 (using decodetree
were much easier to review than the A64 part...
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH v6 82/82] target/arm: Enable SVE2 and related extensions
On Fri, 30 Apr 2021 at 22:37, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/cpu.c | 1 + > target/arm/cpu64.c | 13 + > 2 files changed, 14 insertions(+) > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 0dd623e590..30fd5d5ff7 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1464,6 +1464,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error > **errp) > > u = cpu->isar.id_isar6; > u = FIELD_DP32(u, ID_ISAR6, JSCVT, 0); > +u = FIELD_DP32(u, ID_ISAR6, I8MM, 0); > cpu->isar.id_isar6 = u; > > u = cpu->isar.mvfr0; > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index f0a9e968c9..379f90fab8 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -662,6 +662,7 @@ static void aarch64_max_initfn(Object *obj) > t = FIELD_DP64(t, ID_AA64ISAR1, SPECRES, 1); > t = FIELD_DP64(t, ID_AA64ISAR1, FRINTTS, 1); > t = FIELD_DP64(t, ID_AA64ISAR1, LRCPC, 2); /* ARMv8.4-RCPC */ > +t = FIELD_DP64(t, ID_AA64ISAR1, I8MM, 1); > cpu->isar.id_aa64isar1 = t; > > t = cpu->isar.id_aa64pfr0; > @@ -702,6 +703,17 @@ static void aarch64_max_initfn(Object *obj) > t = FIELD_DP64(t, ID_AA64MMFR2, ST, 1); /* TTST */ > cpu->isar.id_aa64mmfr2 = t; > > +t = cpu->isar.id_aa64zfr0; > +t = FIELD_DP64(t, ID_AA64ZFR0, SVEVER, 1); > +t = FIELD_DP64(t, ID_AA64ZFR0, AES, 2); /* PMULL */ > +t = FIELD_DP64(t, ID_AA64ZFR0, BITPERM, 1); > +t = FIELD_DP64(t, ID_AA64ZFR0, SHA3, 1); > +t = FIELD_DP64(t, ID_AA64ZFR0, SM4, 1); > +t = FIELD_DP64(t, ID_AA64ZFR0, I8MM, 1); > +t = FIELD_DP64(t, ID_AA64ZFR0, F32MM, 1); > +t = FIELD_DP64(t, ID_AA64ZFR0, F64MM, 1); > +cpu->isar.id_aa64zfr0 = t; > + > /* Replicate the same data to the 32-bit id registers. */ > u = cpu->isar.id_isar5; > u = FIELD_DP32(u, ID_ISAR5, AES, 2); /* AES + PMULL */ > @@ -718,6 +730,7 @@ static void aarch64_max_initfn(Object *obj) > u = FIELD_DP32(u, ID_ISAR6, FHM, 1); > u = FIELD_DP32(u, ID_ISAR6, SB, 1); > u = FIELD_DP32(u, ID_ISAR6, SPECRES, 1); > +u = FIELD_DP32(u, ID_ISAR6, I8MM, 1); > cpu->isar.id_isar6 = u; > > u = cpu->isar.id_pfr0; Do we need to clear any of these in the "user set has_neon and/or has_vfp to false" code in arm_cpu_realizefn() ? thanks -- PMM
Re: [PATCH v6 81/82] linux-user/aarch64: Enable hwcap bits for sve2 and related extensions
On Fri, 30 Apr 2021 at 22:43, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > linux-user/elfload.c | 10 ++ > 1 file changed, 10 insertions(+) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v6 79/82] target/arm: Implement aarch32 VSUDOT, VUSDOT
On Fri, 30 Apr 2021 at 22:20, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/cpu.h | 5 + > target/arm/neon-shared.decode | 6 ++ > target/arm/translate-neon.c | 27 +++ > 3 files changed, 38 insertions(+) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v6 78/82] target/arm: Split decode of VSDOT and VUDOT
On Fri, 30 Apr 2021 at 22:21, Richard Henderson wrote: > > Now that we have a common helper, sharing decode does not > save much. Also, this will solve an upcoming naming problem. > > Signed-off-by: Richard Henderson > --- > target/arm/neon-shared.decode | 9 ++--- > target/arm/translate-neon.c | 30 ++ > 2 files changed, 28 insertions(+), 11 deletions(-) It also deals with the issue I had while reviewing the previous patch of trying to find "VDOT" in the Arm ARM and failing :-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v6 77/82] target/arm: Fix decode for VDOT (indexed)
On Fri, 30 Apr 2021 at 22:23, Richard Henderson wrote: > > We were extracting the M register twice, once incorrectly > as M:vm and once correctly as rm. Remove the incorrect > name and remove the incorrect decode. > > Signed-off-by: Richard Henderson > --- > target/arm/neon-shared.decode | 4 +- > target/arm/translate-neon.c | 90 +++ > 2 files changed, 40 insertions(+), 54 deletions(-) > > diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode > index ca0c699072..facb621450 100644 > --- a/target/arm/neon-shared.decode > +++ b/target/arm/neon-shared.decode > @@ -61,8 +61,8 @@ VCMLA_scalar 1110 0 . rot:2 1000 . q:1 > index:1 0 vm:4 \ > VCMLA_scalar 1110 1 . rot:2 1000 . q:1 . 0 \ > vm=%vm_dp vn=%vn_dp vd=%vd_dp size=2 index=0 > > -VDOT_scalar 1110 0 . 10 1101 . q:1 index:1 u:1 rm:4 \ > - vm=%vm_dp vn=%vn_dp vd=%vd_dp > +VDOT_scalar 1110 0 . 10 1101 . q:1 index:1 u:1 vm:4 \ > + vn=%vn_dp vd=%vd_dp Is it possible to make this kind of bug a decodetree error? It seems unlikely that there's a use for having a bit which is decoded both by a %foo field specification and also in some other way... > > %vfml_scalar_q0_rm 0:3 5:1 > %vfml_scalar_q1_index 5:1 3:1 > diff --git a/target/arm/translate-neon.c b/target/arm/translate-neon.c > index a0e267694b..52b75ff76f 100644 > --- a/target/arm/translate-neon.c > +++ b/target/arm/translate-neon.c > @@ -151,6 +151,36 @@ static void neon_store_element64(int reg, int ele, MemOp > size, TCGv_i64 var) > } > } > > +static bool do_neon_ddda(DisasContext *s, int q, int vd, int vn, int vm, > + int data, gen_helper_gvec_4 *fn_gvec) This patch seems to be doing more than its commit message suggests. If we want to share code between trans_VDOT and trans_VDOT_scalar can we do that refactoring in its own patch, please ? thanks -- PMM
Re: [PATCH v2 03/10] Python: add utility function for retrieving port redirection
On 4/11/21 10:09 PM, Cleber Rosa wrote: At this time I don't have a need for it in the PyPI upload, but I wonder if this exception is justified. I mean, what would be gained, besides dealing with the exception itself, by not including it? I just don't want to support or maintain little one-off misc utilities and things, but also don't wish to impose a larger design burden on you to integrate it in a more holistic and subjectively pleasant way. Having a misc/utils package where we just stick "kinda-sorta" stuff but never intend to ship or support is a useful middle ground. It's for your benefit so that we don't have to agonize about the interfaces, but still create common code that the rest of the QEMU tree can use. Even shipping 0.x stuff, releasing it onto PyPI imposes quite a design burden. At least within the QEMU tree I can see who is using which interfaces and how and avoid breakage. Once we pull that lever ... we won't have that benefit anymore. --js
Re: [PULL v2 00/32] Misc (mostly i386) patches for 2021-05-11
On Wed, 12 May 2021 at 09:43, Paolo Bonzini wrote: > > The following changes since commit e58c7a3bba3076890592f02d2b0e596bf191b5c2: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20210510-1' into staging (2021-05-10 > 17:28:11 +0100) > > are available in the Git repository at: > > https://gitlab.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to e804f892b90e58861edd79aafa4d1f4dbdeb3819: > > coverity-scan: list components, move model to scripts/coverity-scan > (2021-05-12 04:06:50 -0400) > > > * AccelCPUClass and sysemu/user split for i386 (Claudio) > * i386 page walk unification > * Fix detection of gdbus-codegen > * Misc refactoring > > > v1->v2: dropped incorrect snapshot-load patch Something weird has happened here. I think I applied v1 of this pullreq (merge commit 31589644ba069ba06c5 now in master), and then when I came along to try to process this one the tag was an entirely different set of commits. Can you check whether what is in master is what you intended, and send a revert/fixup patch if necessary, please? PS: my scripts detected this issue: Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé ERROR: pull request includes tag with UTF-8 error in person name which you probably want to fix before you send a future pullreq with those commits in. (This error is what nudged me into looking more closely and realizing something weird was going on...) Side note: it is more robust if you don't reuse pullreq tag names... thanks -- PMM
Re: [PATCH v3 03/11] Python: add utility function for retrieving port redirection
On 4/12/21 12:46 AM, Cleber Rosa wrote:
Slightly different versions for the same utility code are currently
present on different locations. This unifies them all, giving
preference to the version from virtiofs_submounts.py, because of the
last tweaks added to it.
While at it, this adds a "qemu.utils" module to host the utility
function and a test.
Signed-off-by: Cleber Rosa
Reviewed-by: Wainer dos Santos Moschetta
Reviewed-by: Eric Auger
Reviewed-by: Willian Rampazzo
---
python/qemu/utils.py | 33
tests/acceptance/info_usernet.py | 29 +
tests/acceptance/linux_ssh_mips_malta.py | 16 +---
tests/acceptance/virtiofs_submounts.py | 21 ---
tests/vm/basevm.py | 7 ++---
5 files changed, 76 insertions(+), 30 deletions(-)
create mode 100644 python/qemu/utils.py
create mode 100644 tests/acceptance/info_usernet.py
diff --git a/python/qemu/utils.py b/python/qemu/utils.py
new file mode 100644
index 000..5ed789275ee
--- /dev/null
+++ b/python/qemu/utils.py
@@ -0,0 +1,33 @@
+"""
+QEMU utility library
+
+This offers miscellaneous utility functions, which may not be easily
+distinguishable or numerous to be in their own module.
+"""
+
+# Copyright (C) 2021 Red Hat Inc.
+#
+# Authors:
+# Cleber Rosa
+#
+# This work is licensed under the terms of the GNU GPL, version 2. See
+# the COPYING file in the top-level directory.
+#
+
+import re
+from typing import Optional
+
+
+def get_info_usernet_hostfwd_port(info_usernet_output: str) -> Optional[int]:
+"""
+Returns the port given to the hostfwd parameter via info usernet
+
+:param info_usernet_output: output generated by hmp command "info usernet"
+:return: the port number allocated by the hostfwd option
+"""
+for line in info_usernet_output.split('\r\n'):
+regex = r'TCP.HOST_FORWARD.*127\.0\.0\.1\s+(\d+)\s+10\.'
+match = re.search(regex, line)
+if match is not None:
+return int(match[1])
+return None
diff --git a/tests/acceptance/info_usernet.py b/tests/acceptance/info_usernet.py
new file mode 100644
index 000..9c1fd903a0b
--- /dev/null
+++ b/tests/acceptance/info_usernet.py
@@ -0,0 +1,29 @@
+# Test for the hmp command "info usernet"
+#
+# Copyright (c) 2021 Red Hat, Inc.
+#
+# Author:
+# Cleber Rosa
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+from avocado_qemu import Test
+
+from qemu.utils import get_info_usernet_hostfwd_port
+
+
+class InfoUsernet(Test):
+
+def test_hostfwd(self):
+self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22')
+self.vm.launch()
+res = self.vm.command('human-monitor-command',
+ command_line='info usernet')
+port = get_info_usernet_hostfwd_port(res)
+self.assertIsNotNone(port,
+ ('"info usernet" output content does not seem to '
+ 'contain the redirected port'))
+self.assertGreater(port, 0,
+ ('Found a redirected port that is not greater than'
+' zero'))
diff --git a/tests/acceptance/linux_ssh_mips_malta.py
b/tests/acceptance/linux_ssh_mips_malta.py
index 6dbd02d49d5..052008f02d4 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -18,6 +18,8 @@
from avocado.utils import archive
from avocado.utils import ssh
+from qemu.utils import get_info_usernet_hostfwd_port
+
class LinuxSSH(Test):
@@ -70,18 +72,14 @@ def get_kernel_info(self, endianess, wordsize):
def setUp(self):
super(LinuxSSH, self).setUp()
-def get_portfwd(self):
+def ssh_connect(self, username, password):
+self.ssh_logger = logging.getLogger('ssh')
res = self.vm.command('human-monitor-command',
command_line='info usernet')
-line = res.split('\r\n')[2]
-port = re.split(r'.*TCP.HOST_FORWARD.*127\.0\.0\.1 (\d+)\s+10\..*',
-line)[1]
+port = get_info_usernet_hostfwd_port(res)
+if not port:
+self.cancel("Failed to retrieve SSH port")
self.log.debug("sshd listening on port:" + port)
-return port
-
-def ssh_connect(self, username, password):
-self.ssh_logger = logging.getLogger('ssh')
-port = self.get_portfwd()
self.ssh_session = ssh.Session(self.VM_IP, port=int(port),
user=username, password=password)
for i in range(10):
diff --git a/tests/acceptance/virtiofs_submounts.py
b/tests/acceptance/virtiofs_submounts.py
index ca64b76301f..57a7047342f 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -9,6 +9,8 @@
from avocado_qemu
Re: [RFC PATCH 14/15] gitlab-ci: Allow forks to use different set of jobs
On 5/11/21 8:48 AM, Philippe Mathieu-Daudé wrote: > Not sure if following up this thread or start a new one, but I got > blocked again from Gitlab, tagged as a crypto miner for running QEMU > CI... > [1] > https://about.gitlab.com/handbook/support/workflows/investigate_blocked_pipeline.html#trends--high-priority-cases > > I pushed 5 different branches to my repository in less than 1h, > kicking 580 jobs [*]. > > I didn't try to stress Gitlab, it was a simple "one time in the month > rebase unmerged branches, push them before respining on the mailing > list". FYI I got my account unlocked (without any notification update). 36h passed, maybe it is something automatic (block the user for 36h if suspected of crypto mining?).
Re: [Qemu-devel] [PATCH] MAINTAINERS: convert to JSON
On Thu, 13 May 2021 at 19:36, Philippe Mathieu-Daudé wrote: > > Cc'ing John/Willian/Wainer > > On 4/1/19 1:29 PM, Paolo Bonzini wrote: > > JSON is already in wide use within QEMU for QMP and QAPI, enable > > wider usage of MAINTAINERS by applying a well-defined machine-readable > > format. ...why are you bringing up an old april-fool's-patch ? (Luckily I checked the date on Paolo's original patch before replying :-)) thanks -- PMM
Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind
On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote: As with gdbserver, valgrind delays the test execution, so the default QMP socket timeout timeout too soon. Signed-off-by: Emanuele Giuseppe Esposito --- python/qemu/machine.py| 2 +- tests/qemu-iotests/iotests.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index d6142271c2..dce96e1858 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -410,7 +410,7 @@ def _launch(self) -> None: shell=False, close_fds=False) -if 'gdbserver' in self._wrapper: +if 'gdbserver' in self._wrapper or 'valgrind' in self._wrapper: This approaches me suggesting that we just change __init__ to accept a parameter that lets the caller decide what kind of timeout(s) they find acceptable. They know more about what they're trying to run than we do. Certainly after launch occurs, the user is free to just grab the qmp object and tinker around with the timeouts, but that does not allow us to change the timeout(s) for accept itself. D'oh. (Spilled milk: It was probably a mistake to make the default launch behavior here have a timeout of 15 seconds. That logic likely belongs to the iotests implementation. The default here probably ought to indeed be "wait forever".) In the here and now ... would it be acceptable to change the launch() method to add a timeout parameter? It's still a little awkward, because conceptually it's a timeout for just QMP and not for the actual duration of the entire launch process. But, I guess, it's *closer* to the truth. If you wanted to route it that way, I take back what I said about not wanting to pass around variables to event loop hooks. If we defined the timeout as something that applies exclusively to the launching process, then it'd be appropriate to route that to the launch-related functions ... and subclasses would have to be adjusted to be made aware that they're expected to operate within those parameters, which is good. Sorry for my waffling back and forth on this. Let me know what the actual requirements are if you figure out which timeouts you need / don't need and I'll give you some review priority. If you attack this series again, can you please split out the python/* patches into its own little series and CC me? --js self._qmp_timer = None self._post_launch() diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index a2e8604674..94597433fa 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -489,7 +489,7 @@ def log(msg: Msg, class Timeout: def __init__(self, seconds, errmsg="Timeout"): -if qemu_gdb: +if qemu_gdb or qemu_valgrind: self.seconds = 3000 else: self.seconds = seconds @@ -700,7 +700,7 @@ def qmp_to_opts(self, obj): return ','.join(output_list) def get_qmp_events(self, wait: bool = False) -> List[QMPMessage]: -if qemu_gdb: +if qemu_gdb or qemu_valgrind: wait = 0.0 return super().get_qmp_events(wait=wait)
Re: GICv3 for MTTCG
I built QEMU from the source files downloaded from https://github.com/qemu/qemu latest commit 3e9f48bcdabe57f8 I have applied the series "GICv3 LPI and ITS feature implementation". When I tried to start QEMU back then with the '-kernel' option, the boot loader failed to locate the rootfs disk by its correct ID. Specifying 'root=/dev/sda2' didn't help me also. So, I used virt-manager successfully which runs QEMU with the following arguments: /usr/local/bin/qemu-system-aarch64 -name guest=EulerOS-2.8-Rich,debug-threads=on -S -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-33-EulerOS-2.8-Rich/master-key.aes -machine virt-6.1,accel=tcg,usb=off,dump-guest-core=off,gic-version=3 -cpu max -drive file=/usr/share/AAVMF/AAVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on -drive file=/var/lib/libvirt/qemu/nvram/EulerOS-2.8-Rich_VARS.fd,if=pflash,format=raw,unit=1 -m 4096 -smp 8,sockets=8,cores=1,threads=1 -uuid c95e0e92-011b-449a-8e3f-b5f09387 -display none -no-user-config -nodefaults -chardev socket,id=charmonitor,fd=26,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -boot strict=on -device pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x1 -device pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 -device pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 -device pcie-root-port,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 -device qemu-xhci,p2=8,p3=8,id=usb,bus=pci.2,addr=0x0 -device virtio-scsi-pci,id=scsi0,bus=pci.3,addr=0x0 -drive file=/var/lib/libvirt/images/EulerOS-2.8-Rich.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0 -device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=1 -drive if=none,id=drive-scsi0-0-0-1,readonly=on -device scsi-cd,bus=scsi0.0,channel=0,scsi-id=0,lun=1,drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 -netdev tap,fd=28,id=hostnet0 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:f9:e0:69,bus=pci.1,addr=0x0 -chardev pty,id=charserial0 -serial chardev:charserial0 -msg timestamp=on Best regards, Andrey. On 5/13/21 8:20 PM, Alex Bennée wrote: > > Andrey Shinkevich writes: > >> Dear colleagues, >> >> Thank you all very much for your responses. Let me reply with one message. >> >> I configured QEMU for AARCH64 guest: >> $ ./configure --target-list=aarch64-softmmu >> >> When I start QEMU with GICv3 on an x86 host: >> qemu-system-aarch64 -machine virt-6.0,accel=tcg,gic-version=3 > > Hmm are you sure you are running your built QEMU? For me the following > works fine: > >./aarch64-softmmu/qemu-system-aarch64 -machine > virt-6.0,gic-version=3,accel=tcg -cpu max -serial mon:stdio -nic > user,model=virtio-net-pci,hostfwd=tcp::-:22 -device virtio-scsi-pci > -device scsi-hd,drive=hd0 -blockdev > driver=raw,node-name=hd0,discard=unmap,file.driver=host_device,file.filename=/dev/zvol/hackpool-0/debian-buster-arm64 > -kernel > ~/lsrc/linux.git/builds/arm64.nopreempt/arch/arm64/boot/Image -append > "console=ttyAMA0 root=/dev/sda2" -display none -m 8G,maxmem=8G -smp 12 > > >> >> QEMU reports this error from hw/pci/msix.c: >> error_setg(errp, "MSI-X is not supported by interrupt controller"); >> >> Probably, the variable 'msi_nonbroken' would be initialized in >> hw/intc/arm_gicv3_its_common.c: >> gicv3_its_init_mmio(..) >> >> I guess that it works with KVM acceleration only rather than with TCG. >> >> The error persists after applying the series: >> https://lists.gnu.org/archive/html/qemu-arm/2021-04/msg00944.html >> "GICv3 LPI and ITS feature implementation" >> (special thanks for referring me to that) >> >> Please, make me clear and advise ideas how that error can be fixed? >> Should the MSI-X support be implemented with GICv3 extra? >> >> When successful, I would like to test QEMU for a maximum number of cores >> to get the best MTTCG performance. >> Probably, we will get just some percentage of performance enhancement >> with the BQL series applied, won't we? I will test it as well. >> >> Best regards, >> Andrey Shinkevich >> >> >> On 5/12/21 6:43 PM, Alex Bennée wrote: >>> >>> Andrey Shinkevich writes: >>> Dear colleagues, I am looking for ways to accelerate the MTTCG for ARM guest on x86-64 host. The maximum number of CPUs for MTTCG that uses GICv2 is limited by 8: include/hw/intc/arm_gic_common.h:#define GIC_NCPU 8 The version 3 of the Generic Interrupt Controller (GICv3) is not supported in QEMU for some reason unknown to me. It would allow to increase the limit of CPUs and accelerate the MTTCG performance on a multiple core hypervisor. >>> >>> It is supported, you just need to select it. >>> I have got an idea to implement the Interrupt Translation Service (ITS) for using by MTTCG for ARM architecture. >>> >>> There is some work to support ITS under TCG already posted: >>> >>>
Re: GICv3 for MTTCG
Hi Shashi, Thank you very much for letting me know. I changed virt machine to the version 6.1 and the error disappeared. But the guest OS is experiencing severe delays while booting and starting. The delays take minutes mostly here: #0 0x7f1d0932554d in __lll_lock_wait () at /lib64/libpthread.so.0 #1 0x7f1d09320e9b in _L_lock_883 () at /lib64/libpthread.so.0 #2 0x7f1d09320d68 in pthread_mutex_lock () at /lib64/libpthread.so.0 #3 0x560bf51637b3 in qemu_mutex_lock_impl (mutex=0x560bf5e05820 , file=0x560bf56db84b "../util/main-loop.c", line=252) at ../util/qemu-thread-posix.c:79 #4 0x560bf4d65403 in qemu_mutex_lock_iothread_impl (file=0x560bf56db84b "../util/main-loop.c", line=252) at ../softmmu/cpus.c:491 #5 0x560bf516faa5 in os_host_main_loop_wait (timeout=2367975) at ../util/main-loop.c:252 #6 0x560bf516fbb0 in main_loop_wait (nonblocking=0) at ../util/main-loop.c:530 #7 0x560bf4ddc186 in qemu_main_loop () at ../softmmu/runstate.c:725 #8 0x560bf473ae42 in main (argc=63, argv=0x7ffc5920eba8, envp=0x7ffc5920eda8) at ../softmmu/main.c:50 and here: #0 0x7f1d0903cd8f in ppoll () at /lib64/libc.so.6 #1 0x560bf512e2d0 in qemu_poll_ns (fds=0x560bf70f12b0, nfds=5, timeout=35025900) at ../util/qemu-timer.c:348 #2 0x560bf516fa8c in os_host_main_loop_wait (timeout=35025900) at ../util/main-loop.c:249 #3 0x560bf516fbb0 in main_loop_wait (nonblocking=0) at ../util/main-loop.c:530 #4 0x560bf4ddc186 in qemu_main_loop () at ../softmmu/runstate.c:725 #5 0x560bf473ae42 in main (argc=63, argv=0x7ffc5920eba8, envp=0x7ffc5920eda8) at ../softmmu/main.c:50 Eventually, the guest hangs at the second back trace above. Best regards, Andrey On 5/13/21 7:45 PM, Shashi Mallela wrote: > Hi Andrey, > > To clarify, the patch series > > https://lists.gnu.org/archive/html/qemu-arm/2021-04/msg00944.html > "GICv3 LPI and ITS feature implementation" > > is applicable for virt machine 6.1 onwards,i.e ITS TCG functionality is > not available for version 6.0 that is being tried > here. > > Thanks > Shashi > > On May 13 2021, at 12:35 pm, Andrey Shinkevich > wrote: > > Dear colleagues, > > Thank you all very much for your responses. Let me reply with one > message. > > I configured QEMU for AARCH64 guest: > $ ./configure --target-list=aarch64-softmmu > > When I start QEMU with GICv3 on an x86 host: > qemu-system-aarch64 -machine virt-6.0,accel=tcg,gic-version=3 > > QEMU reports this error from hw/pci/msix.c: > error_setg(errp, "MSI-X is not supported by interrupt controller"); > > Probably, the variable 'msi_nonbroken' would be initialized in > hw/intc/arm_gicv3_its_common.c: > gicv3_its_init_mmio(..) > > I guess that it works with KVM acceleration only rather than with TCG. > > The error persists after applying the series: > https://lists.gnu.org/archive/html/qemu-arm/2021-04/msg00944.html > "GICv3 LPI and ITS feature implementation" > (special thanks for referring me to that) > > Please, make me clear and advise ideas how that error can be fixed? > Should the MSI-X support be implemented with GICv3 extra? > > When successful, I would like to test QEMU for a maximum number of cores > to get the best MTTCG performance. > Probably, we will get just some percentage of performance enhancement > with the BQL series applied, won't we? I will test it as well. > > Best regards, > Andrey Shinkevich > > > On 5/12/21 6:43 PM, Alex Bennée wrote: > > > > Andrey Shinkevich writes: > > > >> Dear colleagues, > >> > >> I am looking for ways to accelerate the MTTCG for ARM guest on > x86-64 host. > >> The maximum number of CPUs for MTTCG that uses GICv2 is limited > by 8: > >> > >> include/hw/intc/arm_gic_common.h:#define GIC_NCPU 8 > >> > >> The version 3 of the Generic Interrupt Controller (GICv3) is not > >> supported in QEMU for some reason unknown to me. It would allow to > >> increase the limit of CPUs and accelerate the MTTCG performance on a > >> multiple core hypervisor. > > > > It is supported, you just need to select it. > > > >> I have got an idea to implement the Interrupt Translation > Service (ITS) > >> for using by MTTCG for ARM architecture. > > > > There is some work to support ITS under TCG already posted: > > > > Subject: [PATCH v3 0/8] GICv3 LPI and ITS feature implementation > > Date: Thu, 29 Apr 2021 19:41:53 -0400 > > Message-Id: <20210429234201.125565-1-shashi.mall...@linaro.org> > > > > please do review and test. > > > >> Do you find that idea useful and feasible? > >> If yes, how much time do you estimate for such a project to > complete by > >> one developer? > >> If no, what are reasons for not implementing GICv3 for MTTCG in >
Re: [PATCH] hw/input/ps2: Use ps2_raise_irq() instead of open coding it
Inspired-by: Volker Rümelin
Signed-off-by: Philippe Mathieu-Daudé
---
hw/input/ps2.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 72cdb80ae1c..02d7e5da723 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -213,7 +213,7 @@ void ps2_raise_irq(PS2State *s)
void ps2_queue(PS2State *s, int b)
{
ps2_queue_noirq(s, b);
-s->update_irq(s->update_arg, 1);
+ps2_raise_irq(s);
}
void ps2_queue_2(PS2State *s, int b1, int b2)
@@ -224,7 +224,7 @@ void ps2_queue_2(PS2State *s, int b1, int b2)
ps2_queue_noirq(s, b1);
ps2_queue_noirq(s, b2);
-s->update_irq(s->update_arg, 1);
+ps2_raise_irq(s);
}
void ps2_queue_3(PS2State *s, int b1, int b2, int b3)
@@ -236,7 +236,7 @@ void ps2_queue_3(PS2State *s, int b1, int b2, int b3)
ps2_queue_noirq(s, b1);
ps2_queue_noirq(s, b2);
ps2_queue_noirq(s, b3);
-s->update_irq(s->update_arg, 1);
+ps2_raise_irq(s);
}
void ps2_queue_4(PS2State *s, int b1, int b2, int b3, int b4)
@@ -249,7 +249,7 @@ void ps2_queue_4(PS2State *s, int b1, int b2, int b3, int
b4)
ps2_queue_noirq(s, b2);
ps2_queue_noirq(s, b3);
ps2_queue_noirq(s, b4);
-s->update_irq(s->update_arg, 1);
+ps2_raise_irq(s);
}
/* keycode is the untranslated scancode in the current scancode set. */
Hi Philippe,
can you read my local qemu repository? :-)
Reviewed-by: Volker Rümelin
Re: [PATCH v3 05/15] qemu-iotests: delay QMP socket timers
On 5/3/21 11:02 AM, Max Reitz wrote: On 30.04.21 23:03, Emanuele Giuseppe Esposito wrote: On 30/04/2021 13:59, Max Reitz wrote: On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote: Attaching a gdbserver implies that the qmp socket should wait indefinitely for an answer from QEMU. Signed-off-by: Emanuele Giuseppe Esposito --- python/qemu/machine.py | 3 +++ tests/qemu-iotests/iotests.py | 10 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 12752142c9..d6142271c2 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -409,6 +409,9 @@ def _launch(self) -> None: stderr=subprocess.STDOUT, shell=False, close_fds=False) + + if 'gdbserver' in self._wrapper: + self._qmp_timer = None Why doesn’t __init__() evaluate this? This here doesn’t feel like the right place for it. If we want to evaluate it here, self._qmp_timer shouldn’t exist, and instead the timeout should be a _post_launch() parameter. (Which I would have nothing against, by the way.) Uhm.. I got another comment in a previous version where for the "event" callbacks it was better a property than passing around a parameter. Which I honestly agree. I think that comment was in the sense of providing a default value, which can be expressed by having a property that is set in __init__. My comment was along the lines that "_post_launch()" is behaving as an event loop hook and not the sort of thing I want to pass parameters to. It's a private method, so the only possibility for someone passing a parameter to is another class method anyway. We have a hierarchy of things that depend on the Machine class and I didn't want to start cascading optional parameters into the subclasses. It was my intent that the information needed to run _post_launch() correctly should be known by the state of the object -- which I think should be true anyway. I don’t have anything against making this a property, but I also don’t have anything against making it a _post_launch() parameter. I could even live with both, i.e. set _qmp_timer to 15 in __init__, then have a _post_launch parameter, and pass either self._qmp_timer or None if self._wrapper includes 'gdbserver'. What I do mind is that I don’t understand why the property is modified here. The value of self._qmp_timer is supposed to be 15 by default and None if self._wrapper includes 'gdbserver'. It should thus be changed to None the moment self._wrapper is made to include 'gdbserver'. Because self._wrapper is set only in __init__, this should happen in __init__. What should __init__() do? The check here is to see if the invocation has gdb (and a couple of patches ahead also valgrind), to remove the timer. If I understand what you mean, you want something like def __init__(self, timer): Oh, no. We can optionally do that perhaps later, but what I meant is just to put this in __init__() (without adding any parameters to it): self._qmp_timer = 15.0 if 'gdbserver' not in self._wrapper else None I think self._qmp_timer should always reflect what timeout we are going to use when a VM is launched. So if the conditions influencing the timeout change, it should be updated immediately to reflect this. The only condition we have right now is the content of self._wrapper, which is only set in __init__, so self._qmp_timer should be set once in __init__ and not changed afterwards. That sounds academic, but imagine what would happen if we had a set_qmp_timer() method: The timout could be adjusted, but launch() would just ignore it and update the property, even though the conditions influencing the timout didn’t change between set_qmp_timer() and launch(). Or if we had a get_qmp_timer(); a caller would read a timeout of 15.0 before launch(), even though the timeout is going to be None. Therefore, I think a property should not be updated just before it is read, but instead when any condition that’s supposed to influence its value changes. I agree with Max's reasoning here. I am also not a fan of squishing magic into this class; changing class behavior based on introspection of wrapper arguments feels like a layering violation. Maybe what you want is a subclass or a wrapper class that knows how to run QEMU using gdbserver, and changes some behaviors accordingly? The factoring of Machine is quite bad already, admittedly, and is in need of a good spit-shine. Too many init parameters, too many state variables, too many methods that got patched in to support one specific use-case at one point or another. At a certain point, I begin to worry about how it's possible to audit how all of these one-off features behave and interact. It's getting complex. Is it time to dream up a refactoring for how the Machine class
Re: [PATCH 10/10] default-configs/devices: Clarify ARM_COMPATIBLE_SEMIHOSTING is required
On 5/13/21 7:58 PM, Paolo Bonzini wrote: > On 13/05/21 18:38, Philippe Mathieu-Daudé wrote: >> ARM_COMPATIBLE_SEMIHOSTING is not optional on these targets. >> Move the variable assignment out of the block documented with >> "Uncomment the following lines to disable these optional devices". > > Can you document why it is always required in the commit message? OK, basically SEMIHOSTING is not a device but an architectural thing. Maybe we are mixing various things, like a semihosting console (yes, a device) and the architectural part, I will discuss that with Alex. > I suppose you cannot move it to default-configs/targets/ because you > want to have CONFIG_SEMIHOSTING selected automatically (patch 9). That and other things. This is annoying me since more than 1 year. I'm waiting Claudio's ARM KVM/TCG series get merged before getting my hands dirty with meson.build. Let's postpone this patch (and the previous one) for now. Thanks for the review, Phil.
Re: [PATCH 00/10] hw: Various Kconfig fixes
On 13/05/21 18:38, Philippe Mathieu-Daudé wrote: Various Kconfig fixes when building stand-alone machine binaries. FYI I'm looking at a way to test this automatically, by generating all the configurations using: https://docs.gitlab.com/ee/ci/parent_child_pipelines.html#dynamic-child-pipel= ines This way we could run this on release candidate tags. Philippe Mathieu-Daud=C3=A9 (10): hw/mem/nvdimm: Use Kconfig 'imply' instead of 'depends on' hw/ide/Kconfig: Add missing dependency PCI -> QDEV hw/arm/Kconfig: Add missing dependency NPCM7XX -> SMBUS hw/arm/Kconfig: Remove unused DS1338 symbol from i.MX25 PDK Board hw/arm/Kconfig: Add missing SDHCI symbol to FSL_IMX25 hw/riscv/Kconfig: Add missing dependency MICROCHIP_PFSOC -> SERIAL hw/riscv/Kconfig: Restrict NUMA to Virt & Spike machines hw/ppc/Kconfig: Add missing dependency E500 -> DS1338 RTC default-configs/devices: Remove implicy SEMIHOSTING config default-configs/devices: Clarify ARM_COMPATIBLE_SEMIHOSTING is required default-configs/devices/arm-softmmu.mak | 5 +++-- default-configs/devices/ppc-softmmu.mak | 1 - default-configs/devices/ppc64-softmmu.mak | 1 - default-configs/devices/riscv32-softmmu.mak | 5 +++-- default-configs/devices/riscv64-softmmu.mak | 5 +++-- hw/arm/Kconfig | 4 +++- hw/i386/Kconfig | 1 + hw/ide/Kconfig | 2 +- hw/mem/Kconfig | 2 -- hw/ppc/Kconfig | 2 ++ hw/riscv/Kconfig| 6 ++ hw/riscv/meson.build| 2 +- 12 files changed, 23 insertions(+), 13 deletions(-) --=20 2.26.3 For patches 1-8 Acked-by: Paolo Bonzini Feel free to send a pull request once the target maintainers for ARM/RISC-V/PPC have had a look. Paolo
Re: [PATCH 10/10] default-configs/devices: Clarify ARM_COMPATIBLE_SEMIHOSTING is required
On 13/05/21 18:38, Philippe Mathieu-Daudé wrote: ARM_COMPATIBLE_SEMIHOSTING is not optional on these targets. Move the variable assignment out of the block documented with "Uncomment the following lines to disable these optional devices". Can you document why it is always required in the commit message? I suppose you cannot move it to default-configs/targets/ because you want to have CONFIG_SEMIHOSTING selected automatically (patch 9). Paolo Signed-off-by: Philippe Mathieu-Daudé --- default-configs/devices/arm-softmmu.mak | 4 +++- default-configs/devices/riscv32-softmmu.mak | 4 +++- default-configs/devices/riscv64-softmmu.mak | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak index 341d439de6f..2713e5e9eb7 100644 --- a/default-configs/devices/arm-softmmu.mak +++ b/default-configs/devices/arm-softmmu.mak @@ -1,5 +1,8 @@ # Default configuration for arm-softmmu +# TODO: semihosting is always required - move to default-configs/targets/ +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y + # TODO: ARM_V7M is currently always required - make this more flexible! CONFIG_ARM_V7M=y @@ -41,5 +44,4 @@ CONFIG_MICROBIT=y CONFIG_FSL_IMX25=y CONFIG_FSL_IMX7=y CONFIG_FSL_IMX6UL=y -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y CONFIG_ALLWINNER_H3=y diff --git a/default-configs/devices/riscv32-softmmu.mak b/default-configs/devices/riscv32-softmmu.mak index 5c9ad2590ef..7219f9749ad 100644 --- a/default-configs/devices/riscv32-softmmu.mak +++ b/default-configs/devices/riscv32-softmmu.mak @@ -1,9 +1,11 @@ # Default configuration for riscv32-softmmu +# TODO: semihosting is always required - move to default-configs/targets/ +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y + # Uncomment the following lines to disable these optional devices: # #CONFIG_PCI_DEVICES=n -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y # Boards: # diff --git a/default-configs/devices/riscv64-softmmu.mak b/default-configs/devices/riscv64-softmmu.mak index 8a92f0a2c74..1d1b8bb9f68 100644 --- a/default-configs/devices/riscv64-softmmu.mak +++ b/default-configs/devices/riscv64-softmmu.mak @@ -1,9 +1,11 @@ # Default configuration for riscv64-softmmu +# TODO: semihosting is always required - move to default-configs/targets/ +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y + # Uncomment the following lines to disable these optional devices: # #CONFIG_PCI_DEVICES=n -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y # Boards: #
[PULL 12/17] migration/ram: Handle RAM block resizes during postcopy
From: David Hildenbrand
Resizing while migrating is dangerous and does not work as expected.
The whole migration code works with the usable_length of a ram block and
does not expect this value to change at random points in time.
In the case of postcopy, relying on used_length is racy as soon as the
guest is running. Also, when used_length changes we might leave the
uffd handler registered for some memory regions, reject valid pages
when migrating and fail when sending the recv bitmap to the source.
Resizing can be trigger *after* (but not during) a reset in
ACPI code by the guest
- hw/arm/virt-acpi-build.c:acpi_ram_update()
- hw/i386/acpi-build.c:acpi_ram_update()
Let's remember the original used_length in a separate variable and
use it in relevant postcopy code. Make sure to update it when we resize
during precopy, when synchronizing the RAM block sizes with the source.
Reviewed-by: Peter Xu
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-9-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
include/exec/ramblock.h | 10 ++
migration/postcopy-ram.c | 15 ---
migration/ram.c | 11 +--
3 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
index 07d50864d8..664701b759 100644
--- a/include/exec/ramblock.h
+++ b/include/exec/ramblock.h
@@ -59,6 +59,16 @@ struct RAMBlock {
*/
unsigned long *clear_bmap;
uint8_t clear_bmap_shift;
+
+/*
+ * RAM block length that corresponds to the used_length on the migration
+ * source (after RAM block sizes were synchronized). Especially, after
+ * starting to run the guest, used_length and postcopy_length can differ.
+ * Used to register/unregister uffd handlers and as the size of the
received
+ * bitmap. Receiving any page beyond this length will bail out, as it
+ * could not have been valid on the source.
+ */
+ram_addr_t postcopy_length;
};
#endif
#endif
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index ab482adef1..2e9697bdd2 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -17,6 +17,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/rcu.h"
#include "exec/target_page.h"
#include "migration.h"
#include "qemu-file.h"
@@ -30,6 +31,7 @@
#include "qemu/error-report.h"
#include "trace.h"
#include "hw/boards.h"
+#include "exec/ramblock.h"
/* Arbitrary limit on size of each discard command,
* keeps them around ~200 bytes
@@ -452,6 +454,13 @@ static int init_range(RAMBlock *rb, void *opaque)
ram_addr_t length = qemu_ram_get_used_length(rb);
trace_postcopy_init_range(block_name, host_addr, offset, length);
+/*
+ * Save the used_length before running the guest. In case we have to
+ * resize RAM blocks when syncing RAM block sizes from the source during
+ * precopy, we'll update it manually via the ram block notifier.
+ */
+rb->postcopy_length = length;
+
/*
* We need the whole of RAM to be truly empty for postcopy, so things
* like ROMs and any data tables built during init must be zero'd
@@ -474,7 +483,7 @@ static int cleanup_range(RAMBlock *rb, void *opaque)
const char *block_name = qemu_ram_get_idstr(rb);
void *host_addr = qemu_ram_get_host_addr(rb);
ram_addr_t offset = qemu_ram_get_offset(rb);
-ram_addr_t length = qemu_ram_get_used_length(rb);
+ram_addr_t length = rb->postcopy_length;
MigrationIncomingState *mis = opaque;
struct uffdio_range range_struct;
trace_postcopy_cleanup_range(block_name, host_addr, offset, length);
@@ -580,7 +589,7 @@ static int nhp_range(RAMBlock *rb, void *opaque)
const char *block_name = qemu_ram_get_idstr(rb);
void *host_addr = qemu_ram_get_host_addr(rb);
ram_addr_t offset = qemu_ram_get_offset(rb);
-ram_addr_t length = qemu_ram_get_used_length(rb);
+ram_addr_t length = rb->postcopy_length;
trace_postcopy_nhp_range(block_name, host_addr, offset, length);
/*
@@ -624,7 +633,7 @@ static int ram_block_enable_notify(RAMBlock *rb, void
*opaque)
struct uffdio_register reg_struct;
reg_struct.range.start = (uintptr_t)qemu_ram_get_host_addr(rb);
-reg_struct.range.len = qemu_ram_get_used_length(rb);
+reg_struct.range.len = rb->postcopy_length;
reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING;
/* Now tell our userfault_fd that it's responsible for this area */
diff --git a/migration/ram.c b/migration/ram.c
index 26ed42b87d..6d09ca78bc 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -240,7 +240,7 @@ int64_t ramblock_recv_bitmap_send(QEMUFile *file,
return -1;
}
-nbits = block->used_length >> TARGET_PAGE_BITS;
+nbits = block->postcopy_length >> TARGET_PAGE_BITS;
/*
* Make sure the tmp bitmap buffer is big enough, e.g., on 32bit
@@ -3530,7 +3530,13 @@ static int ram_load_postcopy(QEMUFile
Re: [PATCH v3 02/15] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine
On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote: Signed-off-by: Emanuele Giuseppe Esposito Assuming it doesn't make the linter explode, which I didn't run: Reviewed-by: John Snow --- python/qemu/qtest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py index 39a0cf62fe..c18eae96c6 100644 --- a/python/qemu/qtest.py +++ b/python/qemu/qtest.py @@ -111,6 +111,7 @@ class QEMUQtestMachine(QEMUMachine): def __init__(self, binary: str, args: Sequence[str] = (), + wrapper: Sequence[str] = (), name: Optional[str] = None, test_dir: str = "/var/tmp", socket_scm_helper: Optional[str] = None, @@ -119,7 +120,8 @@ def __init__(self, name = "qemu-%d" % os.getpid() if sock_dir is None: sock_dir = test_dir -super().__init__(binary, args, name=name, test_dir=test_dir, +super().__init__(binary, args, wrapper=wrapper, name=name, + test_dir=test_dir, socket_scm_helper=socket_scm_helper, sock_dir=sock_dir) self._qtest: Optional[QEMUQtestProtocol] = None
[PULL 11/17] migration/ram: Simplify host page handling in ram_load_postcopy()
From: David Hildenbrand
Add two new helper functions. This will come in come handy once we want to
handle ram block resizes while postcopy is active.
Note that ram_block_from_stream() will already print proper errors.
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-8-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
dgilbert: Added brackets in host_page_from_ram_block_offset
to cause uintptr_t to cast the sum, to fix armhf-cross build
---
migration/ram.c | 55 -
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index e1d081d334..26ed42b87d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3085,6 +3085,20 @@ static inline void *host_from_ram_block_offset(RAMBlock
*block,
return block->host + offset;
}
+static void *host_page_from_ram_block_offset(RAMBlock *block,
+ ram_addr_t offset)
+{
+/* Note: Explicitly no check against offset_in_ramblock(). */
+return (void *)QEMU_ALIGN_DOWN((uintptr_t)(block->host + offset),
+ block->page_size);
+}
+
+static ram_addr_t host_page_offset_from_ram_block_offset(RAMBlock *block,
+ ram_addr_t offset)
+{
+return ((uintptr_t)block->host + offset) & (block->page_size - 1);
+}
+
static inline void *colo_cache_from_block_offset(RAMBlock *block,
ram_addr_t offset, bool record_bitmap)
{
@@ -3481,13 +3495,12 @@ static int ram_load_postcopy(QEMUFile *f)
MigrationIncomingState *mis = migration_incoming_get_current();
/* Temporary page that is later 'placed' */
void *postcopy_host_page = mis->postcopy_tmp_page;
-void *this_host = NULL;
+void *host_page = NULL;
bool all_zero = true;
int target_pages = 0;
while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
ram_addr_t addr;
-void *host = NULL;
void *page_buffer = NULL;
void *place_source = NULL;
RAMBlock *block = NULL;
@@ -3512,9 +3525,12 @@ static int ram_load_postcopy(QEMUFile *f)
if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
RAM_SAVE_FLAG_COMPRESS_PAGE)) {
block = ram_block_from_stream(f, flags);
+if (!block) {
+ret = -EINVAL;
+break;
+}
-host = host_from_ram_block_offset(block, addr);
-if (!host) {
+if (!offset_in_ramblock(block, addr)) {
error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
ret = -EINVAL;
break;
@@ -3532,19 +3548,17 @@ static int ram_load_postcopy(QEMUFile *f)
* of a host page in one chunk.
*/
page_buffer = postcopy_host_page +
- ((uintptr_t)host & (block->page_size - 1));
+ host_page_offset_from_ram_block_offset(block, addr);
+/* If all TP are zero then we can optimise the place */
if (target_pages == 1) {
-this_host = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
-block->page_size);
-} else {
+host_page = host_page_from_ram_block_offset(block, addr);
+} else if (host_page != host_page_from_ram_block_offset(block,
+addr)) {
/* not the 1st TP within the HP */
-if (QEMU_ALIGN_DOWN((uintptr_t)host, block->page_size) !=
-(uintptr_t)this_host) {
-error_report("Non-same host page %p/%p",
- host, this_host);
-ret = -EINVAL;
-break;
-}
+error_report("Non-same host page %p/%p", host_page,
+ host_page_from_ram_block_offset(block, addr));
+ret = -EINVAL;
+break;
}
/*
@@ -3623,16 +3637,11 @@ static int ram_load_postcopy(QEMUFile *f)
}
if (!ret && place_needed) {
-/* This gets called at the last target page in the host page */
-void *place_dest = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
- block->page_size);
-
if (all_zero) {
-ret = postcopy_place_page_zero(mis, place_dest,
- block);
+ret = postcopy_place_page_zero(mis, host_page, block);
} else {
-ret = postcopy_place_page(mis, place_dest,
- place_source, block);
+ret = postcopy_place_page(mis, host_page, place_source,
+
Re: [PATCH v3 01/15] python: qemu: add timer parameter for qmp.accept socket
On 4/14/21 1:03 PM, Emanuele Giuseppe Esposito wrote: Add a new _qmp_timer field to the QEMUMachine class. The default timer is 15 sec, as per the default in the qmp accept() function. Fine enough for now. What's the exact need for this change, exactly? Is it just to prevent any unbounded blocking waits? I assume this came up in development or you'd not have added it. Reviewed-by: John Snow Signed-off-by: Emanuele Giuseppe Esposito --- python/qemu/machine.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/qemu/machine.py b/python/qemu/machine.py index 6e44bda337..12752142c9 100644 --- a/python/qemu/machine.py +++ b/python/qemu/machine.py @@ -110,6 +110,7 @@ def __init__(self, self._binary = binary self._args = list(args) self._wrapper = wrapper +self._qmp_timer = 15.0 self._name = name or "qemu-%d" % os.getpid() self._test_dir = test_dir @@ -323,7 +324,7 @@ def _pre_launch(self) -> None: def _post_launch(self) -> None: if self._qmp_connection: -self._qmp.accept() +self._qmp.accept(self._qmp_timer) def _post_shutdown(self) -> None: """
Re: [RFC PATCH 2/9] replace machine phase_check with machine_is_initialized/ready calls
On 13/05/21 10:25, Mirela Grujic wrote:
Once we define MachineInitPhase in qapi, the generated enumeration
constants will be prefixed with the MACHINE_INIT_PHASE_.
We need to define the MachineInitPhase enum in qapi in order to
add the QMP command that will query current machine init phase.
Since in the existing definition of enum MachineInitPhase the
enumeration constants are prefixed with PHASE_, there will be a
massive find/replace to rename the existing enum constants.
We'll do this in 2 phases:
1) hide explicit use of PHASE_ prefixed enums by replacing
phase_check(PHASE_MACHINE_INITIALIZED) -> machine_is_initialized()
phase_check(PHASE_MACHINE_READY) -> machine_is_ready()
2) rename enums
Once 1) and 2) are done MachineInitPhase enum will be generated.
Is it so much churn to just rename everything to MACHINE_INIT_PHASE_*
and keep phase_check() as is? Or is it because the QAPI-generated names
are quite long?
Paolo
Signed-off-by: Mirela Grujic
---
include/hw/qdev-core.h | 2 ++
hw/core/machine-qmp-cmds.c | 2 +-
hw/core/machine.c | 2 +-
hw/core/qdev.c | 12 +++-
hw/pci/pci.c | 2 +-
hw/usb/core.c | 2 +-
hw/virtio/virtio-iommu.c | 2 +-
monitor/hmp.c | 2 +-
softmmu/qdev-monitor.c | 9 -
softmmu/vl.c | 2 +-
ui/console.c | 2 +-
11 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 6e52240d92..5e3c6d4482 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -842,5 +842,7 @@ typedef enum MachineInitPhase {
extern bool phase_check(MachineInitPhase phase);
extern void phase_advance(MachineInitPhase phase);
extern MachineInitPhase phase_get(void);
+extern bool machine_is_initialized(void);
+extern bool machine_is_ready(void);
#endif
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 68a942595a..be286882cc 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -149,7 +149,7 @@ HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error
**errp)
void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
{
-if (phase_check(PHASE_MACHINE_INITIALIZED)) {
+if (machine_is_initialized()) {
error_setg(errp, "The command is permitted only before the machine has
been created");
return;
}
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 40def78183..eba046924d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1239,7 +1239,7 @@ static NotifierList machine_init_done_notifiers =
void qemu_add_machine_init_done_notifier(Notifier *notify)
{
notifier_list_add(_init_done_notifiers, notify);
-if (phase_check(PHASE_MACHINE_READY)) {
+if (machine_is_ready()) {
notify->notify(notify, NULL);
}
}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 4a4a4d8c52..71906170f9 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -904,7 +904,7 @@ static void device_initfn(Object *obj)
{
DeviceState *dev = DEVICE(obj);
-if (phase_check(PHASE_MACHINE_READY)) {
+if (machine_is_ready()) {
dev->hotplugged = 1;
qdev_hot_added = true;
}
@@ -1155,6 +1155,16 @@ MachineInitPhase phase_get(void)
return machine_phase;
}
+bool machine_is_initialized(void)
+{
+return machine_phase >= PHASE_MACHINE_INITIALIZED;
+}
+
+bool machine_is_ready(void)
+{
+return machine_phase >= PHASE_MACHINE_READY;
+}
+
static const TypeInfo device_type_info = {
.name = TYPE_DEVICE,
.parent = TYPE_OBJECT,
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 8f35e13a0c..19b584c3d1 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1071,7 +1071,7 @@ static PCIDevice *do_pci_register_device(PCIDevice
*pci_dev,
address_space_init(_dev->bus_master_as,
_dev->bus_master_container_region, pci_dev->name);
-if (phase_check(PHASE_MACHINE_READY)) {
+if (machine_is_ready()) {
pci_init_bus_master(pci_dev);
}
pci_dev->irq_state = 0;
diff --git a/hw/usb/core.c b/hw/usb/core.c
index 975f76250a..2ec0dea6a0 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -97,7 +97,7 @@ void usb_wakeup(USBEndpoint *ep, unsigned int stream)
USBDevice *dev = ep->dev;
USBBus *bus = usb_bus_from_device(dev);
-if (!phase_check(PHASE_MACHINE_READY)) {
+if (!machine_is_ready()) {
/*
* This is machine init cold plug. No need to wakeup anyone,
* all devices will be reset anyway. And trying to wakeup can
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1b23e8e18c..8b1bcb2848 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -948,7 +948,7 @@ static int
virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
* accept it. Having a different masks is possible but the guest will use
* sub-optimal
[PULL 08/17] migration/ram: Handle RAM block resizes during precopy
From: David Hildenbrand
Resizing while migrating is dangerous and does not work as expected.
The whole migration code works on the usable_length of ram blocks and does
not expect this to change at random points in time.
In the case of precopy, the ram block size must not change on the source,
after syncing the RAM block list in ram_save_setup(), so as long as the
guest is still running on the source.
Resizing can be trigger *after* (but not during) a reset in
ACPI code by the guest
- hw/arm/virt-acpi-build.c:acpi_ram_update()
- hw/i386/acpi-build.c:acpi_ram_update()
Use the ram block notifier to get notified about resizes. Let's simply
cancel migration and indicate the reason. We'll continue running on the
source. No harm done.
Update the documentation. Postcopy will be handled separately.
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-5-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
Manual merge
---
include/exec/memory.h | 10 ++
migration/migration.c | 9 +++--
migration/migration.h | 1 +
migration/ram.c | 30 ++
softmmu/physmem.c | 5 +++--
5 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5728a681b2..c8b9088924 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -131,7 +131,7 @@ typedef struct IOMMUTLBEvent {
#define RAM_SHARED (1 << 1)
/* Only a portion of RAM (used_length) is actually used, and migrated.
- * This used_length size can change across reboots.
+ * Resizing RAM while migrating can result in the migration being canceled.
*/
#define RAM_RESIZEABLE (1 << 2)
@@ -955,7 +955,9 @@ void memory_region_init_ram_shared_nomigrate(MemoryRegion
*mr,
* RAM. Accesses into the region will
* modify memory directly. Only an initial
* portion of this RAM is actually used.
- * The used size can change across reboots.
+ * Changing the size while migrating
+ * can result in the migration being
+ * canceled.
*
* @mr: the #MemoryRegion to be initialized.
* @owner: the object that tracks the region's reference count
@@ -1586,8 +1588,8 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr);
/* memory_region_ram_resize: Resize a RAM region.
*
- * Only legal before guest might have detected the memory size: e.g. on
- * incoming migration, or right after reset.
+ * Resizing RAM while migrating can result in the migration being canceled.
+ * Care has to be taken if the guest might have already detected the memory.
*
* @mr: a memory region created with @memory_region_init_resizeable_ram.
* @newsize: the new size the region
diff --git a/migration/migration.c b/migration/migration.c
index fdadee290e..4698b47442 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -223,13 +223,18 @@ void migration_object_init(void)
dirty_bitmap_mig_init();
}
+void migration_cancel(void)
+{
+migrate_fd_cancel(current_migration);
+}
+
void migration_shutdown(void)
{
/*
* Cancel the current migration - that will (eventually)
* stop the migration using this structure
*/
-migrate_fd_cancel(current_migration);
+migration_cancel();
object_unref(OBJECT(current_migration));
/*
@@ -2307,7 +2312,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
void qmp_migrate_cancel(Error **errp)
{
-migrate_fd_cancel(migrate_get_current());
+migration_cancel();
}
void qmp_migrate_continue(MigrationStatus state, Error **errp)
diff --git a/migration/migration.h b/migration/migration.h
index db6708326b..f7b388d718 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -375,5 +375,6 @@ int foreach_not_ignored_block(RAMBlockIterFunc func, void
*opaque);
void migration_make_urgent_request(void);
void migration_consume_urgent_request(void);
bool migration_rate_limit(void);
+void migration_cancel(void);
#endif
diff --git a/migration/ram.c b/migration/ram.c
index bb52bd97db..77922c445e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4096,8 +4096,38 @@ static SaveVMHandlers savevm_ram_handlers = {
.resume_prepare = ram_resume_prepare,
};
+static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host,
+ size_t old_size, size_t new_size)
+{
+ram_addr_t offset;
+RAMBlock *rb = qemu_ram_block_from_host(host, false, );
+Error *err = NULL;
+
+if (ramblock_is_ignored(rb)) {
+return;
+}
+
+if (!migration_is_idle()) {
+/*
+ * Precopy code on the source cannot deal with the size of RAM blocks
+ * changing at random points in time - especially after sending the
+ * RAM
Re: [RFC PATCH 0/9] Initial support for machine creation via QMP
Hi Mirela, this is very interesting! It's unfortunate that I completely missed the discussions in January/February. You might have noticed that in the 5.2/6.0 timeframe I worked on cleaning up the machine initialization phases and qemu_init. The idea behind the cleanup was to identify clearly the steps from one phase to the next. I am very happy that you are already benefitting from that work in this series and you were able to make a prototype with so little code. My plan was a bit more ambitious though :) and it is laid out at https://wiki.qemu.org/User:Paolo_Bonzini/Machine_init_sequence. My plan was to have completely different binaries than the current qemu-system-*. These would only have a handful of command line options (such as -name, -sandbox, -trace, -L) and would open a QMP connection on stdio. All other command line option would be either considered legacy or adjusted to be part of two new QMP commands, machine-set and accel-set, which would advance through the phases like this: PHASE_NO_MACHINE -> machine-set -> PHASE_MACHINE_CREATED -> -> accel-set -> PHASE_ACCEL_CREATED -> PHASE_MACHINE_INITIALIZED -> -> finish-machine-init -> PHASE_MACHINE_READY -> cont I think this idea would work well with your plan below, because the preconfiguration that you need to do happens mostly at PHASE_MACHINE_INITIALIZED. Of course, the disadvantage of my approach is that, in the initial version, a lot of capabilities of QEMU are not available (especially with respect to the UI, since there's no "display-add" command). However, the basic implementation of machine-set and accel-set should not really be *that* much more work, and it should be doable in parallel with the conversion efforts for those options. For example, today I posted a series that maps -smp to -M (and then, SMP configuration would automatically become available in machine-set). I have placed the skeleton of the work I was doing at https://gitlab.com/bonzini/qemu/ in the branch qemu-qmp-targets. You can see a sample execution at https://asciinema.org/a/TXMX8EZh8Md0fZnuE7uhfv6cO. I have not touched some of the patches in a long time, but I plan to give them a quick test tomorrow. Starting from the code in that branch, it should not be hard to implement the machine-set and accel-set commands in the same fashion as QEMU 5.2's implementation of object-add. Thanks for posting these patches, I have started a light review of them. Paolo On 13/05/21 10:25, Mirela Grujic wrote: The direction for this work has been set in the discussion thread: "About creating machines on the command line" in January/February 2021: https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg01839.html https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg01070.html To customize a machine via QMP we need the ability to stop QEMU at a specific machine initialization phase. Currently, machine initialization phases are: 1) no-machine: machine does not exist yet (current_machine == NULL) 2) machine-created: machine exists, but its accelerator does not (current_machine->accelerator == NULL) 3) accel-created: machine's accelerator is configured (current_machine->accelerator != NULL), but machine class's init() has not been called (no properties validated, machine_init_done notifiers not registered, no sysbus, etc.) 4) initialized: machine class's init() has been called, thus machine properties are validated, machine_init_done notifiers registered, sysbus realized, etc. Devices added at this phase are considered to be cold-plugged. 5) ready: machine_init_done notifiers are called, then QEMU is ready to start CPUs. Devices added at this phase are considered to be hot-plugged. QEMU can be stopped today using the -preconfig CLI option at phase 3 (accel-created). This option was introduced to enable the QMP configuration of parameters that affect the machine initialization. We cannot add devices at this point because the machine class's init() has not been called, thus sysbus does not exist yet (a device cannot be added because there is no bus to attach it to). QEMU can be also stopped using the -S CLI option at the machine ready phase. However, it is too late to add devices at this phase because the machine is already configured, and any devices added at this point are considered to be hot-plugged. Since the existing -preconfig CLI option stops QEMU too early, and the -S option stops too late, we need a way to stop QEMU in between (after the machine is initialized and before it becomes ready).
[PULL 17/17] tests/migration: introduce multifd into guestperf
From: Hyman
Guestperf tool does not cover the multifd-enabled migration
currently, it is worth supporting so that developers can
analysis the migration performance with all kinds of
migration.
To request that multifd is enabled, with 4 channels:
$ ./tests/migration/guestperf.py \
--multifd --multifd-channels 4 --output output.json
To run the entire standardized set of multifd-enabled
comparisons, with unix migration:
$ ./tests/migration/guestperf-batch.py \
--dst-host localhost --transport unix \
--filter compr-multifd* --output outputdir
Signed-off-by: Hyman Huang(黄勇)
Message-Id:
Reviewed-by: Daniel P. Berrangé
Signed-off-by: Dr. David Alan Gilbert
---
tests/migration/guestperf/comparison.py | 14 ++
tests/migration/guestperf/engine.py | 16
tests/migration/guestperf/scenario.py | 12 ++--
tests/migration/guestperf/shell.py | 10 +-
4 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/tests/migration/guestperf/comparison.py
b/tests/migration/guestperf/comparison.py
index ba2edbe546..c03b3f6d7e 100644
--- a/tests/migration/guestperf/comparison.py
+++ b/tests/migration/guestperf/comparison.py
@@ -121,4 +121,18 @@ def __init__(self, name, scenarios):
Scenario("compr-xbzrle-cache-50",
compression_xbzrle=True, compression_xbzrle_cache=50),
]),
+
+
+# Looking at effect of multifd with
+# varying numbers of channels
+Comparison("compr-multifd", scenarios = [
+Scenario("compr-multifd-channels-4",
+ multifd=True, multifd_channels=2),
+Scenario("compr-multifd-channels-8",
+ multifd=True, multifd_channels=8),
+Scenario("compr-multifd-channels-32",
+ multifd=True, multifd_channels=32),
+Scenario("compr-multifd-channels-64",
+ multifd=True, multifd_channels=64),
+]),
]
diff --git a/tests/migration/guestperf/engine.py
b/tests/migration/guestperf/engine.py
index 6b49aed579..208e095794 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -188,6 +188,22 @@ def _migrate(self, hardware, scenario, src, dst,
connect_uri):
1024 * 1024 * 1024 / 100 *
scenario._compression_xbzrle_cache))
+if scenario._multifd:
+resp = src.command("migrate-set-capabilities",
+ capabilities = [
+ { "capability": "multifd",
+ "state": True }
+ ])
+resp = src.command("migrate-set-parameters",
+ multifd_channels=scenario._multifd_channels)
+resp = dst.command("migrate-set-capabilities",
+ capabilities = [
+ { "capability": "multifd",
+ "state": True }
+ ])
+resp = dst.command("migrate-set-parameters",
+ multifd_channels=scenario._multifd_channels)
+
resp = src.command("migrate", uri=connect_uri)
post_copy = False
diff --git a/tests/migration/guestperf/scenario.py
b/tests/migration/guestperf/scenario.py
index 28ef36c26d..de70d9b2f5 100644
--- a/tests/migration/guestperf/scenario.py
+++ b/tests/migration/guestperf/scenario.py
@@ -29,7 +29,8 @@ def __init__(self, name,
post_copy=False, post_copy_iters=5,
auto_converge=False, auto_converge_step=10,
compression_mt=False, compression_mt_threads=1,
- compression_xbzrle=False, compression_xbzrle_cache=10):
+ compression_xbzrle=False, compression_xbzrle_cache=10,
+ multifd=False, multifd_channels=2):
self._name = name
@@ -56,6 +57,9 @@ def __init__(self, name,
self._compression_xbzrle = compression_xbzrle
self._compression_xbzrle_cache = compression_xbzrle_cache # percentage
of guest RAM
+self._multifd = multifd
+self._multifd_channels = multifd_channels
+
def serialize(self):
return {
"name": self._name,
@@ -73,6 +77,8 @@ def serialize(self):
"compression_mt_threads": self._compression_mt_threads,
"compression_xbzrle": self._compression_xbzrle,
"compression_xbzrle_cache": self._compression_xbzrle_cache,
+"multifd": self._multifd,
+"multifd_channels": self._multifd_channels,
}
@classmethod
@@ -92,4 +98,6 @@ def deserialize(cls, data):
data["compression_mt"],
data["compression_mt_threads"],
data["compression_xbzrle"],
-data["compression_xbzrle_cache"])
+data["compression_xbzrle_cache"],
+data["multifd"],
+data["multifd_channels"])
diff
[PULL 07/17] numa: Make all callbacks of ram block notifiers optional
From: David Hildenbrand
Let's make add/remove optional. We want to introduce a RAM block
notifier for RAM migration that is only interested in resize events.
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-4-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
hw/core/numa.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 4c58b2348d..1058d3697b 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -818,8 +818,11 @@ static int ram_block_notify_add_single(RAMBlock *rb, void
*opaque)
void ram_block_notifier_add(RAMBlockNotifier *n)
{
QLIST_INSERT_HEAD(_list.ramblock_notifiers, n, next);
+
/* Notify about all existing ram blocks. */
-qemu_ram_foreach_block(ram_block_notify_add_single, n);
+if (n->ram_block_added) {
+qemu_ram_foreach_block(ram_block_notify_add_single, n);
+}
}
void ram_block_notifier_remove(RAMBlockNotifier *n)
@@ -832,7 +835,9 @@ void ram_block_notify_add(void *host, size_t size, size_t
max_size)
RAMBlockNotifier *notifier;
QLIST_FOREACH(notifier, _list.ramblock_notifiers, next) {
-notifier->ram_block_added(notifier, host, size, max_size);
+if (notifier->ram_block_added) {
+notifier->ram_block_added(notifier, host, size, max_size);
+}
}
}
@@ -841,7 +846,9 @@ void ram_block_notify_remove(void *host, size_t size,
size_t max_size)
RAMBlockNotifier *notifier;
QLIST_FOREACH(notifier, _list.ramblock_notifiers, next) {
-notifier->ram_block_removed(notifier, host, size, max_size);
+if (notifier->ram_block_removed) {
+notifier->ram_block_removed(notifier, host, size, max_size);
+}
}
}
--
2.31.1
[PULL 16/17] tests/qtest/migration-test: Use g_autofree to avoid leaks on error paths
From: Peter Maydell
Coverity notices that several places in the migration-test code fail
to free memory in error-exit paths. This is pretty unimportant in
test case code, but we can avoid having to manually free the memory
entirely by using g_autofree.
The places where Coverity spotted a leak were relating to early exits
not freeing 'uri' in test_precopy_unix(), do_test_validate_uuid(),
migrate_postcopy_prepare() and test_migrate_auto_converge(). This
patch converts all the string-allocation in the test code to
g_autofree for consistency.
Fixes: Coverity CID 1432313, 1432315, 1432352, 1432364
Signed-off-by: Peter Maydell
Message-Id: <20210506185819.9010-1-peter.mayd...@linaro.org>
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: Dr. David Alan Gilbert
---
tests/qtest/migration-test.c | 61
1 file changed, 20 insertions(+), 41 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 4d989f191b..2b028df687 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -110,13 +110,12 @@ static void init_bootfile(const char *bootpath, void
*content, size_t len)
*/
static void wait_for_serial(const char *side)
{
-char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
+g_autofree char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
FILE *serialfile = fopen(serialpath, "r");
const char *arch = qtest_get_arch();
int started = (strcmp(side, "src_serial") == 0 &&
strcmp(arch, "ppc64") == 0) ? 0 : 1;
-g_free(serialpath);
do {
int readvalue = fgetc(serialfile);
@@ -274,10 +273,9 @@ static void check_guests_ram(QTestState *who)
static void cleanup(const char *filename)
{
-char *path = g_strdup_printf("%s/%s", tmpfs, filename);
+g_autofree char *path = g_strdup_printf("%s/%s", tmpfs, filename);
unlink(path);
-g_free(path);
}
static char *SocketAddress_to_str(SocketAddress *addr)
@@ -374,11 +372,8 @@ static char *migrate_get_parameter_str(QTestState *who,
static void migrate_check_parameter_str(QTestState *who, const char *parameter,
const char *value)
{
-char *result;
-
-result = migrate_get_parameter_str(who, parameter);
+g_autofree char *result = migrate_get_parameter_str(who, parameter);
g_assert_cmpstr(result, ==, value);
-g_free(result);
}
static void migrate_set_parameter_str(QTestState *who, const char *parameter,
@@ -495,12 +490,14 @@ static void migrate_start_destroy(MigrateStart *args)
static int test_migrate_start(QTestState **from, QTestState **to,
const char *uri, MigrateStart *args)
{
-gchar *arch_source, *arch_target;
-gchar *cmd_source, *cmd_target;
+g_autofree gchar *arch_source = NULL;
+g_autofree gchar *arch_target = NULL;
+g_autofree gchar *cmd_source = NULL;
+g_autofree gchar *cmd_target = NULL;
const gchar *ignore_stderr;
-char *bootpath = NULL;
-char *shmem_opts;
-char *shmem_path;
+g_autofree char *bootpath = NULL;
+g_autofree char *shmem_opts = NULL;
+g_autofree char *shmem_path = NULL;
const char *arch = qtest_get_arch();
const char *machine_opts = NULL;
const char *memory_size;
@@ -559,8 +556,6 @@ static int test_migrate_start(QTestState **from, QTestState
**to,
g_assert_not_reached();
}
-g_free(bootpath);
-
if (!getenv("QTEST_LOG") && args->hide_stderr) {
ignore_stderr = "2>/dev/null";
} else {
@@ -588,11 +583,9 @@ static int test_migrate_start(QTestState **from,
QTestState **to,
memory_size, tmpfs,
arch_source, shmem_opts, args->opts_source,
ignore_stderr);
-g_free(arch_source);
if (!args->only_target) {
*from = qtest_init(cmd_source);
}
-g_free(cmd_source);
cmd_target = g_strdup_printf("-accel kvm -accel tcg%s%s "
"-name target,debug-threads=on "
@@ -605,18 +598,14 @@ static int test_migrate_start(QTestState **from,
QTestState **to,
memory_size, tmpfs, uri,
arch_target, shmem_opts,
args->opts_target, ignore_stderr);
-g_free(arch_target);
*to = qtest_init(cmd_target);
-g_free(cmd_target);
-g_free(shmem_opts);
/*
* Remove shmem file immediately to avoid memory leak in test failed case.
* It's valid becase QEMU has already opened this file
*/
if (args->use_shmem) {
unlink(shmem_path);
-g_free(shmem_path);
}
out:
@@ -662,7 +651,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
QTestState **to_ptr,
MigrateStart *args)
{
-char *uri =
Re: [RFC PATCH 4/9] qapi: Implement 'query-machine-phase' command
On 13/05/21 10:25, Mirela Grujic wrote:
The command returns current machine initialization phase.
From now on, the MachineInitPhase enum is generated.
Signed-off-by: Mirela Grujic
---
qapi/machine.json | 54 ++
include/hw/qdev-core.h | 29 +---
hw/core/machine-qmp-cmds.c | 9 +++
3 files changed, 64 insertions(+), 28 deletions(-)
diff --git a/qapi/machine.json b/qapi/machine.json
index 6e90d463fc..47bdbec817 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1274,3 +1274,57 @@
##
{ 'event': 'MEM_UNPLUG_ERROR',
'data': { 'device': 'str', 'msg': 'str' } }
+
+##
+# @MachineInitPhase:
+#
+# Enumeration of machine initialization phases.
+#
+# @no-machine: machine does not exist
+#
+# @machine-created: machine is created, but its accelerator is not
+#
+# @accel-created: accelerator is created, but the machine properties have not
+# been validated and machine initialization is not done yet
+#
+# @initialized: machine is initialized, thus creating any embedded devices and
+# validating machine properties. Devices created at this time are
+# considered to be cold-plugged.
+#
+# @ready: QEMU is ready to start CPUs and devices created at this time are
+# considered to be hot-plugged. The monitor is not restricted to
+# "preconfig" commands.
+##
+{ 'enum': 'MachineInitPhase',
+ 'data': [ 'no-machine', 'machine-created', 'accel-created', 'initialized',
+'ready' ] }
+
+##
+# @MachineInitPhaseStatus:
+#
+# Information about machine initialization phase
+#
+# @phase: the machine initialization phase
+#
+# Since: #FIXME
+##
+{ 'struct': 'MachineInitPhaseStatus',
+ 'data': { 'phase': 'MachineInitPhase' } }
+
+##
+# @query-machine-phase:
+#
+# Return machine initialization phase
+#
+# Since: #FIXME
+#
+# Returns: MachineInitPhaseStatus
+#
+# Example:
+#
+# -> { "execute": "query-machine-phase" }
+# <- { "return": { "phase": "initialized" } }
+#
+##
+{ 'command': 'query-machine-phase', 'returns': 'MachineInitPhaseStatus',
+ 'allow-preconfig': true }
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index dc2f63478b..ca39b77ae6 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -1,6 +1,7 @@
#ifndef QDEV_CORE_H
#define QDEV_CORE_H
+#include "qapi/qapi-types-machine.h"
#include "qemu/queue.h"
#include "qemu/bitmap.h"
#include "qemu/rcu.h"
@@ -811,34 +812,6 @@ void device_listener_unregister(DeviceListener *listener);
*/
bool qdev_should_hide_device(QemuOpts *opts);
-typedef enum MachineInitPhase {
-/* current_machine is NULL. */
-MACHINE_INIT_PHASE_NO_MACHINE,
-
-/* current_machine is not NULL, but current_machine->accel is NULL. */
-MACHINE_INIT_PHASE_MACHINE_CREATED,
-
-/*
- * current_machine->accel is not NULL, but the machine properties have
- * not been validated and machine_class->init has not yet been called.
- */
-MACHINE_INIT_PHASE_ACCEL_CREATED,
-
-/*
- * machine_class->init has been called, thus creating any embedded
- * devices and validating machine properties. Devices created at
- * this time are considered to be cold-plugged.
- */
-MACHINE_INIT_PHASE_INITIALIZED,
-
-/*
- * QEMU is ready to start CPUs and devices created at this time
- * are considered to be hot-plugged. The monitor is not restricted
- * to "preconfig" commands.
- */
-MACHINE_INIT_PHASE_READY,
-} MachineInitPhase;
-
extern bool phase_check(MachineInitPhase phase);
extern void phase_advance(MachineInitPhase phase);
extern MachineInitPhase phase_get(void);
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index be286882cc..23f837dadb 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -198,3 +198,12 @@ MemdevList *qmp_query_memdev(Error **errp)
object_child_foreach(obj, query_memdev, );
return list;
}
+
+MachineInitPhaseStatus *qmp_query_machine_phase(Error **errp)
+{
+MachineInitPhaseStatus *status = g_malloc0(sizeof(*status));
+
+status->phase = phase_get();
+
+return status;
+}
This command is a good idea, and we can in fact even include it already
in QEMU.
Paolo
Re: [PULL 00/17] migration queue
* Peter Maydell (peter.mayd...@linaro.org) wrote: > On Tue, 11 May 2021 at 16:08, Dr. David Alan Gilbert (git) > wrote: > > > > From: "Dr. David Alan Gilbert" > > > > The following changes since commit e4f3ede95ce813d5705c65e1c0e1c80c70739ebb: > > > > Merge remote-tracking branch > > 'remotes/kraxel/tags/usb-20210505-pull-request' into staging (2021-05-10 > > 19:55:06 +0100) > > > > are available in the Git repository at: > > > > git://github.com/dagrh/qemu.git tags/pull-migration-20210511a > > > > for you to fetch changes up to 872df23afc70985af5a458e0c4bd2b984559015c: > > > > tests/migration: introduce multifd into guestperf (2021-05-11 11:26:19 > > +0100) > > > > > > Migration pull 2021-05-11 > > > > The largest change in this set is David's changes for ram block size > > changing; then there's a pile of other cleanups and fixes. > > > > Signed-off-by: Dr. David Alan Gilbert > > > > Fails to build on the gitlab cross build job: > https://gitlab.com/qemu-project/qemu/-/jobs/1258047823 > > ../migration/ram.c: In function 'host_page_from_ram_block_offset': > ../migration/ram.c:3092:12: error: cast to pointer from integer of > different size [-Werror=int-to-pointer-cast] > return (void *)QEMU_ALIGN_DOWN((uintptr_t)block->host + offset, > ^ OK, resent with a pair of brackets around the (block->host + offset), so that the result ends up as the uintptr_t. Seems to work for me in the docker test that otherwise failed. Dave > > thanks > -- PMM > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[PULL 04/17] migration: Drop redundant query-migrate result @blocked
From: Markus Armbruster
Result @blocked is redundant. Unfortunately, we realized this too
close to the release to risk dropping it, so we deprecated it
instead, in commit e11ce6c06.
Since it was deprecated from the start, we can delete it without
the customary grace period. Do so.
Signed-off-by: Markus Armbruster
Reviewed-by: Dr. David Alan Gilbert
Reviewed-by: Daniel P. Berrangé
Message-Id: <20210429140424.2802929-1-arm...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
migration/migration.c | 29 +
monitor/hmp-cmds.c| 2 +-
qapi/migration.json | 6 --
3 files changed, 14 insertions(+), 23 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 8ca034136b..fdadee290e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1073,27 +1073,24 @@ static void populate_vfio_info(MigrationInfo *info)
static void fill_source_migration_info(MigrationInfo *info)
{
MigrationState *s = migrate_get_current();
+GSList *cur_blocker = migration_blockers;
-info->blocked = migration_is_blocked(NULL);
-info->has_blocked_reasons = info->blocked;
info->blocked_reasons = NULL;
-if (info->blocked) {
-GSList *cur_blocker = migration_blockers;
-/*
- * There are two types of reasons a migration might be blocked;
- * a) devices marked in VMState as non-migratable, and
- * b) Explicit migration blockers
- * We need to add both of them here.
- */
-qemu_savevm_non_migratable_list(>blocked_reasons);
+/*
+ * There are two types of reasons a migration might be blocked;
+ * a) devices marked in VMState as non-migratable, and
+ * b) Explicit migration blockers
+ * We need to add both of them here.
+ */
+qemu_savevm_non_migratable_list(>blocked_reasons);
-while (cur_blocker) {
-QAPI_LIST_PREPEND(info->blocked_reasons,
- g_strdup(error_get_pretty(cur_blocker->data)));
-cur_blocker = g_slist_next(cur_blocker);
-}
+while (cur_blocker) {
+QAPI_LIST_PREPEND(info->blocked_reasons,
+ g_strdup(error_get_pretty(cur_blocker->data)));
+cur_blocker = g_slist_next(cur_blocker);
}
+info->has_blocked_reasons = info->blocked_reasons != NULL;
switch (s->state) {
case MIGRATION_STATUS_NONE:
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 0ad5b77477..d9bef63373 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -224,7 +224,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
migration_global_dump(mon);
-if (info->blocked) {
+if (info->blocked_reasons) {
strList *reasons = info->blocked_reasons;
monitor_printf(mon, "Outgoing migration blocked:\n");
while (reasons) {
diff --git a/qapi/migration.json b/qapi/migration.json
index 0b17cce46b..7a5bdf9a0d 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -228,11 +228,6 @@
# Present and non-empty when migration is blocked.
# (since 6.0)
#
-# @blocked: True if outgoing migration is blocked (since 6.0)
-#
-# Features:
-# @deprecated: Member @blocked is deprecated. Use @blocked-reasons instead.
-#
# Since: 0.14
##
{ 'struct': 'MigrationInfo',
@@ -246,7 +241,6 @@
'*setup-time': 'int',
'*cpu-throttle-percentage': 'int',
'*error-desc': 'str',
- 'blocked': { 'type': 'bool', 'features': [ 'deprecated' ] },
'*blocked-reasons': ['str'],
'*postcopy-blocktime' : 'uint32',
'*postcopy-vcpu-blocktime': ['uint32'],
--
2.31.1
[PULL 14/17] migration/ram: Use offset_in_ramblock() in range checks
From: David Hildenbrand
We never read or write beyond the used_length of memory blocks when
migrating. Make this clearer by using offset_in_ramblock() consistently.
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-11-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
migration/ram.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 6d09ca78bc..60ea913c54 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1342,8 +1342,8 @@ static bool find_dirty_block(RAMState *rs,
PageSearchStatus *pss, bool *again)
*again = false;
return false;
}
-if ram_addr_t)pss->page) << TARGET_PAGE_BITS)
->= pss->block->used_length) {
+if (!offset_in_ramblock(pss->block,
+((ram_addr_t)pss->page) << TARGET_PAGE_BITS)) {
/* Didn't find anything in this RAM Block */
pss->page = 0;
pss->block = QLIST_NEXT_RCU(pss->block, next);
@@ -1863,7 +1863,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t
start, ram_addr_t len)
rs->last_req_rb = ramblock;
}
trace_ram_save_queue_pages(ramblock->idstr, start, len);
-if (start + len > ramblock->used_length) {
+if (!offset_in_ramblock(ramblock, start + len - 1)) {
error_report("%s request overrun start=" RAM_ADDR_FMT " len="
RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
__func__, start, len, ramblock->used_length);
@@ -3696,8 +3696,8 @@ void colo_flush_ram_cache(void)
while (block) {
offset = migration_bitmap_find_dirty(ram_state, block, offset);
-if (((ram_addr_t)offset) << TARGET_PAGE_BITS
->= block->used_length) {
+if (!offset_in_ramblock(block,
+((ram_addr_t)offset) << TARGET_PAGE_BITS))
{
offset = 0;
block = QLIST_NEXT_RCU(block, next);
} else {
--
2.31.1
Re: [RFC PATCH 7/9] qdev-monitor: Restructure and fix the check for command availability
On 13/05/21 10:25, Mirela Grujic wrote: The existing code had to be restructured to make room for adding checks that are specific to the machine phases. The fix is related to the way that commands with the 'allow-preconfig' option are treated. Commands labelled with the 'allow-preconfig' option were meant to be allowed during the 'preconfig' state, i.e. before the machine is initialized. The equivalent of 'preconfig' state (after its removal) is machine init phase MACHINE_INIT_PHASE_ACCEL_CREATED. Therefore, commands with 'allow-preconfig' option should be allowed to run while the machine is in MACHINE_INIT_PHASE_ACCEL_CREATED phase. Before this patch, those commands were allowed to run if the machine is not ready, which includes three more phases besides the accel-created phase. Since there were no means to enter other phases via QMP, the implementation was fine. However, with the introduction of 'next-machine-phase' and 'advance-machine-phase' commands, one could enter machine 'initialized' phase and still have available 'preconfig' commands, which is incorrect. This patch makes available 'allow-preconfig' commands only when they're needed - before the machine is initialized, in the accel-created phase. To enable a command after the machine gets initialized and before it becomes ready, one should use 'allow-init-config' option that will be introduced in the following patch. There aren't many commands that are valid only for the accel created or machine initialized phase. I think adding allow-init-config is more churn than keeping only allow-preconfig, and calling phase_check in the individual commands. (Or even better, in the internal APIs that they call, so that QMP is completely oblivious to phases and just gets the Error* back). In other words, allow-preconfig is there because there are many commands that are allowed only after the machine-ready phase, but anything in the middle can be handled just fine from C code. Paolo
[PULL 15/17] tests/migration-test: Fix "true" vs true
From: "Dr. David Alan Gilbert"
Accidental use of "true" as a boolean; spotted by coverity
and Peter.
Fixes: b99784ef6c3
Fixes: d795f47466e
Reported-by: Peter Maydell
Reported-by: Coverity (CID 1432373, 1432292, 1432288)
Signed-off-by: Dr. David Alan Gilbert
Message-Id: <20210504100545.112213-1-dgilb...@redhat.com>
Reviewed-by: Thomas Huth
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Dr. David Alan Gilbert
---
tests/qtest/migration-test.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 3a711bb492..4d989f191b 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -898,8 +898,8 @@ static void test_xbzrle(const char *uri)
migrate_set_parameter_int(from, "xbzrle-cache-size", 33554432);
-migrate_set_capability(from, "xbzrle", "true");
-migrate_set_capability(to, "xbzrle", "true");
+migrate_set_capability(from, "xbzrle", true);
+migrate_set_capability(to, "xbzrle", true);
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
@@ -1246,8 +1246,8 @@ static void test_multifd_tcp(const char *method)
migrate_set_parameter_str(from, "multifd-compression", method);
migrate_set_parameter_str(to, "multifd-compression", method);
-migrate_set_capability(from, "multifd", "true");
-migrate_set_capability(to, "multifd", "true");
+migrate_set_capability(from, "multifd", true);
+migrate_set_capability(to, "multifd", true);
/* Start incoming migration from the 1st socket */
rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
@@ -1330,8 +1330,8 @@ static void test_multifd_tcp_cancel(void)
migrate_set_parameter_int(from, "multifd-channels", 16);
migrate_set_parameter_int(to, "multifd-channels", 16);
-migrate_set_capability(from, "multifd", "true");
-migrate_set_capability(to, "multifd", "true");
+migrate_set_capability(from, "multifd", true);
+migrate_set_capability(to, "multifd", true);
/* Start incoming migration from the 1st socket */
rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
@@ -1358,7 +1358,7 @@ static void test_multifd_tcp_cancel(void)
migrate_set_parameter_int(to2, "multifd-channels", 16);
-migrate_set_capability(to2, "multifd", "true");
+migrate_set_capability(to2, "multifd", true);
/* Start incoming migration from the 1st socket */
rsp = wait_command(to2, "{ 'execute': 'migrate-incoming',"
--
2.31.1
[PULL 02/17] migration/ram: Reduce unnecessary rate limiting
From: Kunkun Jiang
When the host page is a huge page and something is sent in the
current iteration, migration_rate_limit() should be executed.
If not, it can be omitted.
Signed-off-by: Keqian Zhu
Signed-off-by: Kunkun Jiang
Reviewed-by: David Edmondson
Reviewed-by: Dr. David Alan Gilbert
Message-Id: <20210316125716.1243-2-jiangkun...@huawei.com>
Signed-off-by: Dr. David Alan Gilbert
---
migration/ram.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index bee2756cd3..00b579b981 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2035,8 +2035,13 @@ static int ram_save_host_page(RAMState *rs,
PageSearchStatus *pss,
pages += tmppages;
pss->page++;
-/* Allow rate limiting to happen in the middle of huge pages */
-migration_rate_limit();
+/*
+ * Allow rate limiting to happen in the middle of huge pages if
+ * something is sent in the current iteration.
+ */
+if (pagesize_bits > 1 && tmppages > 0) {
+migration_rate_limit();
+}
} while ((pss->page & (pagesize_bits - 1)) &&
offset_in_ramblock(pss->block,
((ram_addr_t)pss->page) << TARGET_PAGE_BITS));
--
2.31.1
[PULL 09/17] exec: Relax range check in ram_block_discard_range()
From: David Hildenbrand
We want to make use of ram_block_discard_range() in the RAM block resize
callback when growing a RAM block, *before* used_length is changed.
Let's relax the check. As RAM blocks always mmap the whole max_length area,
we cannot corrupt unrelated data.
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-6-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
softmmu/physmem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 813a3efe8e..e1da81ed2f 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3500,7 +3500,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start,
size_t length)
goto err;
}
-if ((start + length) <= rb->used_length) {
+if ((start + length) <= rb->max_length) {
bool need_madvise, need_fallocate;
if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
error_report("ram_block_discard_range: Unaligned length: %zx",
@@ -3567,7 +3567,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start,
size_t length)
} else {
error_report("ram_block_discard_range: Overrun block '%s' (%" PRIu64
"/%zx/" RAM_ADDR_FMT")",
- rb->idstr, start, length, rb->used_length);
+ rb->idstr, start, length, rb->max_length);
}
err:
--
2.31.1
[PULL 13/17] migration/multifd: Print used_length of memory block
From: David Hildenbrand
We actually want to print the used_length, against which we check.
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-10-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
migration/multifd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index a6677c45c8..0a4803cfcc 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -361,7 +361,7 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p,
Error **errp)
if (offset > (block->used_length - qemu_target_page_size())) {
error_setg(errp, "multifd: offset too long %" PRIu64
" (max " RAM_ADDR_FMT ")",
- offset, block->max_length);
+ offset, block->used_length);
return -1;
}
p->pages->iov[i].iov_base = block->host + offset;
--
2.31.1
[PULL 03/17] migration/ram: Optimize ram_save_host_page()
From: Kunkun Jiang
Starting from pss->page, ram_save_host_page() will check every page
and send the dirty pages up to the end of the current host page or
the boundary of used_length of the block. If the host page size is
a huge page, the step "check" will take a lot of time.
It will improve performance to use migration_bitmap_find_dirty().
Tested on Kunpeng 920; VM parameters: 1U 4G (page size 1G)
The time of ram_save_host_page() in the last round of ram saving:
before optimize: 9250us after optimize: 34us
Signed-off-by: Keqian Zhu
Signed-off-by: Kunkun Jiang
Reviewed-by: Peter Xu
Message-Id: <20210316125716.1243-3-jiangkun...@huawei.com>
Signed-off-by: Dr. David Alan Gilbert
---
migration/ram.c | 39 +++
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 00b579b981..bb52bd97db 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2013,6 +2013,8 @@ static int ram_save_host_page(RAMState *rs,
PageSearchStatus *pss,
int tmppages, pages = 0;
size_t pagesize_bits =
qemu_ram_pagesize(pss->block) >> TARGET_PAGE_BITS;
+unsigned long hostpage_boundary =
+QEMU_ALIGN_UP(pss->page + 1, pagesize_bits);
unsigned long start_page = pss->page;
int res;
@@ -2023,30 +2025,27 @@ static int ram_save_host_page(RAMState *rs,
PageSearchStatus *pss,
do {
/* Check the pages is dirty and if it is send it */
-if (!migration_bitmap_clear_dirty(rs, pss->block, pss->page)) {
-pss->page++;
-continue;
-}
-
-tmppages = ram_save_target_page(rs, pss, last_stage);
-if (tmppages < 0) {
-return tmppages;
-}
+if (migration_bitmap_clear_dirty(rs, pss->block, pss->page)) {
+tmppages = ram_save_target_page(rs, pss, last_stage);
+if (tmppages < 0) {
+return tmppages;
+}
-pages += tmppages;
-pss->page++;
-/*
- * Allow rate limiting to happen in the middle of huge pages if
- * something is sent in the current iteration.
- */
-if (pagesize_bits > 1 && tmppages > 0) {
-migration_rate_limit();
+pages += tmppages;
+/*
+ * Allow rate limiting to happen in the middle of huge pages if
+ * something is sent in the current iteration.
+ */
+if (pagesize_bits > 1 && tmppages > 0) {
+migration_rate_limit();
+}
}
-} while ((pss->page & (pagesize_bits - 1)) &&
+pss->page = migration_bitmap_find_dirty(rs, pss->block, pss->page);
+} while ((pss->page < hostpage_boundary) &&
offset_in_ramblock(pss->block,
((ram_addr_t)pss->page) << TARGET_PAGE_BITS));
-/* The offset we leave with is the last one we looked at */
-pss->page--;
+/* The offset we leave with is the min boundary of host page and block */
+pss->page = MIN(pss->page, hostpage_boundary) - 1;
res = ram_save_release_protection(rs, pss, start_page);
return (res < 0 ? res : pages);
--
2.31.1
[PULL 10/17] migration/ram: Discard RAM when growing RAM blocks after ram_postcopy_incoming_init()
From: David Hildenbrand
In case we grow our RAM after ram_postcopy_incoming_init() (e.g., when
synchronizing the RAM block state with the migration source), the resized
part would not get discarded. Let's perform that when being notified
about a resize while postcopy has been advised, but is not listening
yet. With precopy, the process is as following:
1. VM created
- RAM blocks are created
2. Incomming migration started
- Postcopy is advised
- All pages in RAM blocks are discarded
3. Precopy starts
- RAM blocks are resized to match the size on the migration source.
- RAM pages from precopy stream are loaded
- Uffd handler is registered, postcopy starts listening
4. Guest started, postcopy running
- Pagefaults get resolved, pages get placed
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-7-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
migration/ram.c | 30 ++
1 file changed, 30 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 77922c445e..e1d081d334 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4099,6 +4099,7 @@ static SaveVMHandlers savevm_ram_handlers = {
static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host,
size_t old_size, size_t new_size)
{
+PostcopyState ps = postcopy_state_get();
ram_addr_t offset;
RAMBlock *rb = qemu_ram_block_from_host(host, false, );
Error *err = NULL;
@@ -4119,6 +4120,35 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier
*n, void *host,
error_free(err);
migration_cancel();
}
+
+switch (ps) {
+case POSTCOPY_INCOMING_ADVISE:
+/*
+ * Update what ram_postcopy_incoming_init()->init_range() does at the
+ * time postcopy was advised. Syncing RAM blocks with the source will
+ * result in RAM resizes.
+ */
+if (old_size < new_size) {
+if (ram_discard_range(rb->idstr, old_size, new_size - old_size)) {
+error_report("RAM block '%s' discard of resized RAM failed",
+ rb->idstr);
+}
+}
+break;
+case POSTCOPY_INCOMING_NONE:
+case POSTCOPY_INCOMING_RUNNING:
+case POSTCOPY_INCOMING_END:
+/*
+ * Once our guest is running, postcopy does no longer care about
+ * resizes. When growing, the new memory was not available on the
+ * source, no handler needed.
+ */
+break;
+default:
+error_report("RAM block '%s' resized during postcopy state: %d",
+ rb->idstr, ps);
+exit(-1);
+}
}
static RAMBlockNotifier ram_mig_ram_notifier = {
--
2.31.1
[PULL 05/17] util: vfio-helpers: Factor out and fix processing of existing ram blocks
From: David Hildenbrand
Factor it out into common code when a new notifier is registered, just
as done with the memory region notifier. This keeps logic about how to
process existing ram blocks at a central place.
Just like when adding a new ram block, we have to register the max_length.
Ram blocks are only "fake resized". All memory (max_length) is mapped.
Print the warning from inside qemu_vfio_ram_block_added().
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-2-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
hw/core/numa.c| 14 ++
include/exec/cpu-common.h | 1 +
softmmu/physmem.c | 5 +
util/vfio-helpers.c | 29 -
4 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/hw/core/numa.c b/hw/core/numa.c
index ac6bed5817..134ebc2b72 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -802,9 +802,23 @@ void query_numa_node_mem(NumaNodeMem node_mem[],
MachineState *ms)
}
}
+static int ram_block_notify_add_single(RAMBlock *rb, void *opaque)
+{
+const ram_addr_t max_size = qemu_ram_get_max_length(rb);
+void *host = qemu_ram_get_host_addr(rb);
+RAMBlockNotifier *notifier = opaque;
+
+if (host) {
+notifier->ram_block_added(notifier, host, max_size);
+}
+return 0;
+}
+
void ram_block_notifier_add(RAMBlockNotifier *n)
{
QLIST_INSERT_HEAD(_list.ramblock_notifiers, n, next);
+/* Notify about all existing ram blocks. */
+qemu_ram_foreach_block(ram_block_notify_add_single, n);
}
void ram_block_notifier_remove(RAMBlockNotifier *n)
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 5a0a2d93e0..ccabed4003 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -57,6 +57,7 @@ const char *qemu_ram_get_idstr(RAMBlock *rb);
void *qemu_ram_get_host_addr(RAMBlock *rb);
ram_addr_t qemu_ram_get_offset(RAMBlock *rb);
ram_addr_t qemu_ram_get_used_length(RAMBlock *rb);
+ram_addr_t qemu_ram_get_max_length(RAMBlock *rb);
bool qemu_ram_is_shared(RAMBlock *rb);
bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
void qemu_ram_set_uf_zeroable(RAMBlock *rb);
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 5232696571..0a05533ed0 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1694,6 +1694,11 @@ ram_addr_t qemu_ram_get_used_length(RAMBlock *rb)
return rb->used_length;
}
+ram_addr_t qemu_ram_get_max_length(RAMBlock *rb)
+{
+return rb->max_length;
+}
+
bool qemu_ram_is_shared(RAMBlock *rb)
{
return rb->flags & RAM_SHARED;
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 97dfa3fd57..92b9565797 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -463,8 +463,14 @@ static void qemu_vfio_ram_block_added(RAMBlockNotifier *n,
void *host, size_t size)
{
QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier);
+int ret;
+
trace_qemu_vfio_ram_block_added(s, host, size);
-qemu_vfio_dma_map(s, host, size, false, NULL);
+ret = qemu_vfio_dma_map(s, host, size, false, NULL);
+if (ret) {
+error_report("qemu_vfio_dma_map(%p, %zu) failed: %s", host, size,
+ strerror(-ret));
+}
}
static void qemu_vfio_ram_block_removed(RAMBlockNotifier *n,
@@ -477,33 +483,14 @@ static void qemu_vfio_ram_block_removed(RAMBlockNotifier
*n,
}
}
-static int qemu_vfio_init_ramblock(RAMBlock *rb, void *opaque)
-{
-void *host_addr = qemu_ram_get_host_addr(rb);
-ram_addr_t length = qemu_ram_get_used_length(rb);
-int ret;
-QEMUVFIOState *s = opaque;
-
-if (!host_addr) {
-return 0;
-}
-ret = qemu_vfio_dma_map(s, host_addr, length, false, NULL);
-if (ret) {
-fprintf(stderr, "qemu_vfio_init_ramblock: failed %p %" PRId64 "\n",
-host_addr, (uint64_t)length);
-}
-return 0;
-}
-
static void qemu_vfio_open_common(QEMUVFIOState *s)
{
qemu_mutex_init(>lock);
s->ram_notifier.ram_block_added = qemu_vfio_ram_block_added;
s->ram_notifier.ram_block_removed = qemu_vfio_ram_block_removed;
-ram_block_notifier_add(>ram_notifier);
s->low_water_mark = QEMU_VFIO_IOVA_MIN;
s->high_water_mark = QEMU_VFIO_IOVA_MAX;
-qemu_ram_foreach_block(qemu_vfio_init_ramblock, s);
+ram_block_notifier_add(>ram_notifier);
}
/**
--
2.31.1
[PULL 06/17] numa: Teach ram block notifiers about resizeable ram blocks
From: David Hildenbrand
Ram block notifiers are currently not aware of resizes. To properly
handle resizes during migration, we want to teach ram block notifiers about
resizeable ram.
Introduce the basic infrastructure but keep using max_size in the
existing notifiers. Supply the max_size when adding and removing ram
blocks. Also, notify on resizes.
Acked-by: Paul Durrant
Reviewed-by: Peter Xu
Cc: xen-de...@lists.xenproject.org
Cc: haxm-t...@intel.com
Cc: Paul Durrant
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Wenchao Wang
Cc: Colin Xu
Signed-off-by: David Hildenbrand
Message-Id: <20210429112708.12291-3-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
hw/core/numa.c | 22 +-
hw/i386/xen/xen-mapcache.c | 7 ---
include/exec/ramlist.h | 13 +
softmmu/physmem.c | 12 ++--
target/i386/hax/hax-mem.c | 5 +++--
target/i386/sev.c | 18 ++
util/vfio-helpers.c| 16
7 files changed, 61 insertions(+), 32 deletions(-)
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 134ebc2b72..4c58b2348d 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -805,11 +805,12 @@ void query_numa_node_mem(NumaNodeMem node_mem[],
MachineState *ms)
static int ram_block_notify_add_single(RAMBlock *rb, void *opaque)
{
const ram_addr_t max_size = qemu_ram_get_max_length(rb);
+const ram_addr_t size = qemu_ram_get_used_length(rb);
void *host = qemu_ram_get_host_addr(rb);
RAMBlockNotifier *notifier = opaque;
if (host) {
-notifier->ram_block_added(notifier, host, max_size);
+notifier->ram_block_added(notifier, host, size, max_size);
}
return 0;
}
@@ -826,20 +827,31 @@ void ram_block_notifier_remove(RAMBlockNotifier *n)
QLIST_REMOVE(n, next);
}
-void ram_block_notify_add(void *host, size_t size)
+void ram_block_notify_add(void *host, size_t size, size_t max_size)
{
RAMBlockNotifier *notifier;
QLIST_FOREACH(notifier, _list.ramblock_notifiers, next) {
-notifier->ram_block_added(notifier, host, size);
+notifier->ram_block_added(notifier, host, size, max_size);
}
}
-void ram_block_notify_remove(void *host, size_t size)
+void ram_block_notify_remove(void *host, size_t size, size_t max_size)
{
RAMBlockNotifier *notifier;
QLIST_FOREACH(notifier, _list.ramblock_notifiers, next) {
-notifier->ram_block_removed(notifier, host, size);
+notifier->ram_block_removed(notifier, host, size, max_size);
+}
+}
+
+void ram_block_notify_resize(void *host, size_t old_size, size_t new_size)
+{
+RAMBlockNotifier *notifier;
+
+QLIST_FOREACH(notifier, _list.ramblock_notifiers, next) {
+if (notifier->ram_block_resized) {
+notifier->ram_block_resized(notifier, host, old_size, new_size);
+}
}
}
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index e82b7dcdd2..bd47c3d672 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -169,7 +169,8 @@ static void xen_remap_bucket(MapCacheEntry *entry,
if (entry->vaddr_base != NULL) {
if (!(entry->flags & XEN_MAPCACHE_ENTRY_DUMMY)) {
-ram_block_notify_remove(entry->vaddr_base, entry->size);
+ram_block_notify_remove(entry->vaddr_base, entry->size,
+entry->size);
}
/*
@@ -224,7 +225,7 @@ static void xen_remap_bucket(MapCacheEntry *entry,
}
if (!(entry->flags & XEN_MAPCACHE_ENTRY_DUMMY)) {
-ram_block_notify_add(vaddr_base, size);
+ram_block_notify_add(vaddr_base, size, size);
}
entry->vaddr_base = vaddr_base;
@@ -465,7 +466,7 @@ static void xen_invalidate_map_cache_entry_unlocked(uint8_t
*buffer)
}
pentry->next = entry->next;
-ram_block_notify_remove(entry->vaddr_base, entry->size);
+ram_block_notify_remove(entry->vaddr_base, entry->size, entry->size);
if (munmap(entry->vaddr_base, entry->size) != 0) {
perror("unmap fails");
exit(-1);
diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index 26704aa3b0..ece6497ee2 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -65,15 +65,20 @@ void qemu_mutex_lock_ramlist(void);
void qemu_mutex_unlock_ramlist(void);
struct RAMBlockNotifier {
-void (*ram_block_added)(RAMBlockNotifier *n, void *host, size_t size);
-void (*ram_block_removed)(RAMBlockNotifier *n, void *host, size_t size);
+void (*ram_block_added)(RAMBlockNotifier *n, void *host, size_t size,
+size_t max_size);
+void (*ram_block_removed)(RAMBlockNotifier *n, void *host, size_t size,
+ size_t max_size);
+void (*ram_block_resized)(RAMBlockNotifier *n, void *host, size_t old_size,
+ size_t new_size);
QLIST_ENTRY(RAMBlockNotifier) next;
};
void
[PULL 00/17] migration queue
From: "Dr. David Alan Gilbert" The following changes since commit dab59ce031228066eb95a9c518846fcacfb0dbbf: Merge remote-tracking branch 'remotes/philmd/tags/pflash-20210511' into staging (2021-05-13 14:45:38 +0100) are available in the Git repository at: git://github.com/dagrh/qemu.git tags/pull-migration-20210513a for you to fetch changes up to 1c3baa1ac4dee2b52837fda89d1d9deeb5da512e: tests/migration: introduce multifd into guestperf (2021-05-13 18:21:14 +0100) Migration pull 2021-05-13 Fix of the 2021-05-11 version, with a fix to build on the armhf cross. The largest change in this set is David's changes for ram block size changing; then there's a pile of other cleanups and fixes. Signed-off-by: Dr. David Alan Gilbert David Hildenbrand (11): migrate/ram: remove "ram_bulk_stage" and "fpo_enabled" util: vfio-helpers: Factor out and fix processing of existing ram blocks numa: Teach ram block notifiers about resizeable ram blocks numa: Make all callbacks of ram block notifiers optional migration/ram: Handle RAM block resizes during precopy exec: Relax range check in ram_block_discard_range() migration/ram: Discard RAM when growing RAM blocks after ram_postcopy_incoming_init() migration/ram: Simplify host page handling in ram_load_postcopy() migration/ram: Handle RAM block resizes during postcopy migration/multifd: Print used_length of memory block migration/ram: Use offset_in_ramblock() in range checks Dr. David Alan Gilbert (1): tests/migration-test: Fix "true" vs true Hyman (1): tests/migration: introduce multifd into guestperf Kunkun Jiang (2): migration/ram: Reduce unnecessary rate limiting migration/ram: Optimize ram_save_host_page() Markus Armbruster (1): migration: Drop redundant query-migrate result @blocked Peter Maydell (1): tests/qtest/migration-test: Use g_autofree to avoid leaks on error paths hw/core/numa.c | 41 +- hw/i386/xen/xen-mapcache.c | 7 +- hw/virtio/virtio-balloon.c | 4 +- hw/virtio/virtio-mem.c | 3 - include/exec/cpu-common.h | 1 + include/exec/memory.h | 10 +- include/exec/ramblock.h | 10 ++ include/exec/ramlist.h | 13 +- include/migration/misc.h| 1 - migration/migration.c | 38 ++--- migration/migration.h | 1 + migration/multifd.c | 2 +- migration/postcopy-ram.c| 15 +- migration/ram.c | 246 ++-- monitor/hmp-cmds.c | 2 +- qapi/migration.json | 6 - softmmu/physmem.c | 26 +++- target/i386/hax/hax-mem.c | 5 +- target/i386/sev.c | 18 +-- tests/migration/guestperf/comparison.py | 14 ++ tests/migration/guestperf/engine.py | 16 +++ tests/migration/guestperf/scenario.py | 12 +- tests/migration/guestperf/shell.py | 10 +- tests/qtest/migration-test.c| 75 -- util/vfio-helpers.c | 41 ++ 25 files changed, 367 insertions(+), 250 deletions(-)
[PULL 01/17] migrate/ram: remove "ram_bulk_stage" and "fpo_enabled"
From: David Hildenbrand
The bulk stage is kind of weird: migration_bitmap_find_dirty() will
indicate a dirty page, however, ram_save_host_page() will never save it, as
migration_bitmap_clear_dirty() detects that it is not dirty.
We already fill the bitmap in ram_list_init_bitmaps() with ones, marking
everything dirty - it didn't used to be that way, which is why we needed
an explicit first bulk stage.
Let's simplify: make the bitmap the single source of thuth. Explicitly
handle the "xbzrle_enabled after first round" case.
Regarding XBZRLE (implicitly handled via "ram_bulk_stage = false" right
now), there is now a slight change in behavior:
- Colo: When starting, it will be disabled (was implicitly enabled)
until the first round actually finishes.
- Free page hinting: When starting, XBZRLE will be disabled (was implicitly
enabled) until the first round actually finished.
- Snapshots: When starting, XBZRLE will be disabled. We essentially only
do a single run, so I guess it will never actually get disabled.
Postcopy seems to indirectly disable it in ram_save_page(), so there
shouldn't be really any change.
Cc: "Michael S. Tsirkin"
Cc: Juan Quintela
Cc: "Dr. David Alan Gilbert"
Cc: Andrey Gruzdev
Cc: Peter Xu
Signed-off-by: David Hildenbrand
Message-Id: <20210216105039.40680-1-da...@redhat.com>
Signed-off-by: Dr. David Alan Gilbert
---
hw/virtio/virtio-balloon.c | 4 +-
hw/virtio/virtio-mem.c | 3 --
include/migration/misc.h | 1 -
migration/ram.c| 78 +-
4 files changed, 18 insertions(+), 68 deletions(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index d120bf8f43..4b5d9e5e50 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -663,9 +663,6 @@ virtio_balloon_free_page_hint_notify(NotifierWithReturn *n,
void *data)
}
switch (pnd->reason) {
-case PRECOPY_NOTIFY_SETUP:
-precopy_enable_free_page_optimization();
-break;
case PRECOPY_NOTIFY_BEFORE_BITMAP_SYNC:
virtio_balloon_free_page_stop(dev);
break;
@@ -685,6 +682,7 @@ virtio_balloon_free_page_hint_notify(NotifierWithReturn *n,
void *data)
*/
virtio_balloon_free_page_done(dev);
break;
+case PRECOPY_NOTIFY_SETUP:
case PRECOPY_NOTIFY_COMPLETE:
break;
default:
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 655824ff81..75aa7d6f1b 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -902,9 +902,6 @@ static int virtio_mem_precopy_notify(NotifierWithReturn *n,
void *data)
PrecopyNotifyData *pnd = data;
switch (pnd->reason) {
-case PRECOPY_NOTIFY_SETUP:
-precopy_enable_free_page_optimization();
-break;
case PRECOPY_NOTIFY_AFTER_BITMAP_SYNC:
virtio_mem_precopy_exclude_unplugged(vmem);
break;
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 738675ef52..465906710d 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -37,7 +37,6 @@ void precopy_infrastructure_init(void);
void precopy_add_notifier(NotifierWithReturn *n);
void precopy_remove_notifier(NotifierWithReturn *n);
int precopy_notify(PrecopyNotifyReason reason, Error **errp);
-void precopy_enable_free_page_optimization(void);
void ram_mig_init(void);
void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index ace8ad431c..bee2756cd3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -311,10 +311,6 @@ struct RAMState {
ram_addr_t last_page;
/* last ram version we have seen */
uint32_t last_version;
-/* We are in the first round */
-bool ram_bulk_stage;
-/* The free page optimization is enabled */
-bool fpo_enabled;
/* How many times we have dirty too many pages */
int dirty_rate_high_cnt;
/* these variables are used for bitmap sync */
@@ -330,6 +326,8 @@ struct RAMState {
uint64_t xbzrle_pages_prev;
/* Amount of xbzrle encoded bytes since the beginning of the period */
uint64_t xbzrle_bytes_prev;
+/* Start using XBZRLE (e.g., after the first round). */
+bool xbzrle_enabled;
/* compression statistics since the beginning of the period */
/* amount of count that no free thread to compress data */
@@ -383,15 +381,6 @@ int precopy_notify(PrecopyNotifyReason reason, Error
**errp)
return notifier_with_return_list_notify(_notifier_list, );
}
-void precopy_enable_free_page_optimization(void)
-{
-if (!ram_state) {
-return;
-}
-
-ram_state->fpo_enabled = true;
-}
-
uint64_t ram_bytes_remaining(void)
{
return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
@@ -664,7 +653,7 @@ static void mig_throttle_guest_down(uint64_t
bytes_dirty_period,
*/
static void xbzrle_cache_zero_page(RAMState *rs, ram_addr_t current_addr)
{
-if (rs->ram_bulk_stage || !migrate_use_xbzrle())
Re: [PATCH v6 75/82] target/arm: Split out do_neon_ddda_fpst
On Fri, 30 Apr 2021 at 22:35, Richard Henderson wrote: > > Split out a helper that can handle the 4-register > format for helpers shared with SVE. > > Signed-off-by: Richard Henderson > --- > target/arm/translate-neon.c | 98 - > 1 file changed, 43 insertions(+), 55 deletions(-) > > diff --git a/target/arm/translate-neon.c b/target/arm/translate-neon.c > index 45fa5166f3..41a7b2208e 100644 > --- a/target/arm/translate-neon.c > +++ b/target/arm/translate-neon.c > @@ -151,24 +151,21 @@ static void neon_store_element64(int reg, int ele, > MemOp size, TCGv_i64 var) > } > } > > -static bool trans_VCMLA(DisasContext *s, arg_VCMLA *a) > +static bool do_neon_ddda_fpst(DisasContext *s, int q, int vd, int vn, int vm, > + int data, ARMFPStatusFlavour fp_flavor, Can we keep the UK "flavour" spelling for the argument to match the type name, please? :-) Otherwise Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH] target/avr: Ignore unimplemented WDR opcode
Le 13/05/2021 à 19:04, Philippe Mathieu-Daudé a écrit :
> Cc'ing qemu-trivial@
Applied to my trivial-patches branch.
Thanks,
Laurent
> On 5/5/21 3:35 PM, Fred Konrad wrote:
>> Le 5/2/21 à 9:09 PM, Philippe Mathieu-Daudé a écrit :
>>> Running the WDR opcode triggers a segfault:
>>>
>>> $ cat > foo.S << EOF
>>> > __start:
>>> > wdr
>>> > EOF
>>> $ avr-gcc -nostdlib -nostartfiles -mmcu=avr6 foo.S -o foo.elf
>>> $ qemu-system-avr -serial mon:stdio -nographic -no-reboot \
>>> -M mega -bios foo.elf -d in_asm --singlestep
>>> IN:
>>> 0x: WDR
>>> Segmentation fault (core dumped)
>>>
>>> (gdb) bt
>>> #0 0xadd0b23a in gdb_get_cpu_pid (cpu=0xaf5a4af0)
>>> at ../gdbstub.c:718
>>> #1 0xadd0b2dd in gdb_get_cpu_process
>>> (cpu=0xaf5a4af0) at ../gdbstub.c:743
>>> #2 0xadd0e477 in gdb_set_stop_cpu (cpu=0xaf5a4af0)
>>> at ../gdbstub.c:2742
>>> #3 0xadc99b96 in cpu_handle_guest_debug
>>> (cpu=0xaf5a4af0) at ../softmmu/cpus.c:306
>>> #4 0xadcc66ab in rr_cpu_thread_fn (arg=0xaf5a4af0)
>>> at ../accel/tcg/tcg-accel-ops-rr.c:224
>>> #5 0xadefaf12 in qemu_thread_start
>>> (args=0xaf5d9870) at ../util/qemu-thread-posix.c:521
>>> #6 0x7f692d940ea5 in start_thread () from
>>> /lib64/libpthread.so.0
>>> #7 0x7f692d6699fd in clone () from /lib64/libc.so.6
>>>
>>> Since the watchdog peripheral is not implemented, simply
>>> log the opcode as unimplemented and keep going.
>>>
>>> Reported-by: Fred Konrad
>>> Signed-off-by: Philippe Mathieu-Daudé
>>> ---
>>> target/avr/helper.c | 6 +-
>>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/target/avr/helper.c b/target/avr/helper.c
>>> index 35e10195940..981c29da453 100644
>>> --- a/target/avr/helper.c
>>> +++ b/target/avr/helper.c
>>> @@ -188,11 +188,7 @@ void helper_break(CPUAVRState *env)
>>> void helper_wdr(CPUAVRState *env)
>>> {
>>> - CPUState *cs = env_cpu(env);
>>> -
>>> - /* WD is not implemented yet, placeholder */
>>> - cs->exception_index = EXCP_DEBUG;
>>> - cpu_loop_exit(cs);
>>> + qemu_log_mask(LOG_UNIMP, "WDG reset (not implemented)\n");
>>> }
>>
>> Seems ok to me, at least better than having a segfault.
>>
>> Reviewed-by: KONRAD Frederic
>>
>>> /*
>>>
>>
>
[PATCH] hw/input/ps2: Use ps2_raise_irq() instead of open coding it
Inspired-by: Volker Rümelin
Signed-off-by: Philippe Mathieu-Daudé
---
hw/input/ps2.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 72cdb80ae1c..02d7e5da723 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -213,7 +213,7 @@ void ps2_raise_irq(PS2State *s)
void ps2_queue(PS2State *s, int b)
{
ps2_queue_noirq(s, b);
-s->update_irq(s->update_arg, 1);
+ps2_raise_irq(s);
}
void ps2_queue_2(PS2State *s, int b1, int b2)
@@ -224,7 +224,7 @@ void ps2_queue_2(PS2State *s, int b1, int b2)
ps2_queue_noirq(s, b1);
ps2_queue_noirq(s, b2);
-s->update_irq(s->update_arg, 1);
+ps2_raise_irq(s);
}
void ps2_queue_3(PS2State *s, int b1, int b2, int b3)
@@ -236,7 +236,7 @@ void ps2_queue_3(PS2State *s, int b1, int b2, int b3)
ps2_queue_noirq(s, b1);
ps2_queue_noirq(s, b2);
ps2_queue_noirq(s, b3);
-s->update_irq(s->update_arg, 1);
+ps2_raise_irq(s);
}
void ps2_queue_4(PS2State *s, int b1, int b2, int b3, int b4)
@@ -249,7 +249,7 @@ void ps2_queue_4(PS2State *s, int b1, int b2, int b3, int
b4)
ps2_queue_noirq(s, b2);
ps2_queue_noirq(s, b3);
ps2_queue_noirq(s, b4);
-s->update_irq(s->update_arg, 1);
+ps2_raise_irq(s);
}
/* keycode is the untranslated scancode in the current scancode set. */
--
2.26.3
Re: GICv3 for MTTCG
Andrey Shinkevich writes: > Dear colleagues, > > Thank you all very much for your responses. Let me reply with one message. > > I configured QEMU for AARCH64 guest: > $ ./configure --target-list=aarch64-softmmu > > When I start QEMU with GICv3 on an x86 host: > qemu-system-aarch64 -machine virt-6.0,accel=tcg,gic-version=3 Hmm are you sure you are running your built QEMU? For me the following works fine: ./aarch64-softmmu/qemu-system-aarch64 -machine virt-6.0,gic-version=3,accel=tcg -cpu max -serial mon:stdio -nic user,model=virtio-net-pci,hostfwd=tcp::-:22 -device virtio-scsi-pci -device scsi-hd,drive=hd0 -blockdev driver=raw,node-name=hd0,discard=unmap,file.driver=host_device,file.filename=/dev/zvol/hackpool-0/debian-buster-arm64 -kernel ~/lsrc/linux.git/builds/arm64.nopreempt/arch/arm64/boot/Image -append "console=ttyAMA0 root=/dev/sda2" -display none -m 8G,maxmem=8G -smp 12 > > QEMU reports this error from hw/pci/msix.c: > error_setg(errp, "MSI-X is not supported by interrupt controller"); > > Probably, the variable 'msi_nonbroken' would be initialized in > hw/intc/arm_gicv3_its_common.c: > gicv3_its_init_mmio(..) > > I guess that it works with KVM acceleration only rather than with TCG. > > The error persists after applying the series: > https://lists.gnu.org/archive/html/qemu-arm/2021-04/msg00944.html > "GICv3 LPI and ITS feature implementation" > (special thanks for referring me to that) > > Please, make me clear and advise ideas how that error can be fixed? > Should the MSI-X support be implemented with GICv3 extra? > > When successful, I would like to test QEMU for a maximum number of cores > to get the best MTTCG performance. > Probably, we will get just some percentage of performance enhancement > with the BQL series applied, won't we? I will test it as well. > > Best regards, > Andrey Shinkevich > > > On 5/12/21 6:43 PM, Alex Bennée wrote: >> >> Andrey Shinkevich writes: >> >>> Dear colleagues, >>> >>> I am looking for ways to accelerate the MTTCG for ARM guest on x86-64 host. >>> The maximum number of CPUs for MTTCG that uses GICv2 is limited by 8: >>> >>> include/hw/intc/arm_gic_common.h:#define GIC_NCPU 8 >>> >>> The version 3 of the Generic Interrupt Controller (GICv3) is not >>> supported in QEMU for some reason unknown to me. It would allow to >>> increase the limit of CPUs and accelerate the MTTCG performance on a >>> multiple core hypervisor. >> >> It is supported, you just need to select it. >> >>> I have got an idea to implement the Interrupt Translation Service (ITS) >>> for using by MTTCG for ARM architecture. >> >> There is some work to support ITS under TCG already posted: >> >>Subject: [PATCH v3 0/8] GICv3 LPI and ITS feature implementation >>Date: Thu, 29 Apr 2021 19:41:53 -0400 >>Message-Id: <20210429234201.125565-1-shashi.mall...@linaro.org> >> >> please do review and test. >> >>> Do you find that idea useful and feasible? >>> If yes, how much time do you estimate for such a project to complete by >>> one developer? >>> If no, what are reasons for not implementing GICv3 for MTTCG in QEMU? >> >> As far as MTTCG performance is concerned there is a degree of >> diminishing returns to be expected as the synchronisation cost between >> threads will eventually outweigh the gains of additional threads. >> >> There are a number of parts that could improve this performance. The >> first would be picking up the BQL reduction series from your FutureWei >> colleges who worked on the problem when they were Linaro assignees: >> >>Subject: [PATCH v2 0/7] accel/tcg: remove implied BQL from >> cpu_handle_interrupt/exception path >>Date: Wed, 19 Aug 2020 14:28:49 -0400 >>Message-Id: <20200819182856.4893-1-robert.fo...@linaro.org> >> >> There was also a longer series moving towards per-CPU locks: >> >>Subject: [PATCH v10 00/73] per-CPU locks >>Date: Wed, 17 Jun 2020 17:01:18 -0400 >>Message-Id: <20200617210231.4393-1-robert.fo...@linaro.org> >> >> I believe the initial measurements showed that the BQL cost started to >> edge up with GIC interactions. We did discuss approaches for this and I >> think one idea was use non-BQL locking for the GIC. You would need to >> revert: >> >>Subject: [PATCH-for-5.2] exec: Remove MemoryRegion::global_locking field >>Date: Thu, 6 Aug 2020 17:07:26 +0200 >>Message-Id: <20200806150726.962-1-phi...@redhat.com> >> >> and then implement a more fine tuned locking in the GIC emulation >> itself. However I think the BQL and per-CPU locks are lower hanging >> fruit to tackle first. >> >>> >>> Best regards, >>> Andrey Shinkevich >> >> -- Alex Bennée
Re: [PATCH] target/avr: Ignore unimplemented WDR opcode
Cc'ing qemu-trivial@
On 5/5/21 3:35 PM, Fred Konrad wrote:
> Le 5/2/21 à 9:09 PM, Philippe Mathieu-Daudé a écrit :
>> Running the WDR opcode triggers a segfault:
>>
>> $ cat > foo.S << EOF
>> > __start:
>> > wdr
>> > EOF
>> $ avr-gcc -nostdlib -nostartfiles -mmcu=avr6 foo.S -o foo.elf
>> $ qemu-system-avr -serial mon:stdio -nographic -no-reboot \
>> -M mega -bios foo.elf -d in_asm --singlestep
>> IN:
>> 0x: WDR
>> Segmentation fault (core dumped)
>>
>> (gdb) bt
>> #0 0xadd0b23a in gdb_get_cpu_pid (cpu=0xaf5a4af0)
>> at ../gdbstub.c:718
>> #1 0xadd0b2dd in gdb_get_cpu_process
>> (cpu=0xaf5a4af0) at ../gdbstub.c:743
>> #2 0xadd0e477 in gdb_set_stop_cpu (cpu=0xaf5a4af0)
>> at ../gdbstub.c:2742
>> #3 0xadc99b96 in cpu_handle_guest_debug
>> (cpu=0xaf5a4af0) at ../softmmu/cpus.c:306
>> #4 0xadcc66ab in rr_cpu_thread_fn (arg=0xaf5a4af0)
>> at ../accel/tcg/tcg-accel-ops-rr.c:224
>> #5 0xadefaf12 in qemu_thread_start
>> (args=0xaf5d9870) at ../util/qemu-thread-posix.c:521
>> #6 0x7f692d940ea5 in start_thread () from
>> /lib64/libpthread.so.0
>> #7 0x7f692d6699fd in clone () from /lib64/libc.so.6
>>
>> Since the watchdog peripheral is not implemented, simply
>> log the opcode as unimplemented and keep going.
>>
>> Reported-by: Fred Konrad
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> target/avr/helper.c | 6 +-
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/target/avr/helper.c b/target/avr/helper.c
>> index 35e10195940..981c29da453 100644
>> --- a/target/avr/helper.c
>> +++ b/target/avr/helper.c
>> @@ -188,11 +188,7 @@ void helper_break(CPUAVRState *env)
>> void helper_wdr(CPUAVRState *env)
>> {
>> - CPUState *cs = env_cpu(env);
>> -
>> - /* WD is not implemented yet, placeholder */
>> - cs->exception_index = EXCP_DEBUG;
>> - cpu_loop_exit(cs);
>> + qemu_log_mask(LOG_UNIMP, "WDG reset (not implemented)\n");
>> }
>
> Seems ok to me, at least better than having a segfault.
>
> Reviewed-by: KONRAD Frederic
>
>> /*
>>
>
[Bug 1916343] Re: -daemonize not working on macOS
** Changed in: qemu Status: Incomplete => New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1916343 Title: -daemonize not working on macOS Status in QEMU: New Bug description: Basically e.g, if I try with below command on macOS: qemu-system-x86_64 \ -m 4G \ -vga virtio \ -display default,show-cursor=on \ -usb \ -device usb-tablet \ -machine type=q35,accel=hvf \ -smp 2 \ -drive file=ubuntu.qcow2,if=virtio -cpu max \ -net nic -net user,hostfwd=tcp::50022-:22,hostfwd=tcp::8000-:80 \ -daemonize With this command, the QEMU processes hang there forever. I guess there is a bug when forking a child and kill the parent? Because when this issue occurs, there are actually 2 QEMU processes running ``` 501 14952 14951 0 7:08PM ?? 0:00.00 (qemu-system-x86_) 501 14953 1 0 7:08PM ?? 0:00.03 qemu-system-x86_64 -m 4G -vga virtio -display default,show-cursor=on -usb -device usb-tablet -machine type=q35,accel=hvf -smp 2 -drive file=ubuntu.qcow2,if=virtio -cpu max -net nic -net user,hostfwd=tcp::50022-:22,hostfwd=tcp::8000-:80 -daemonize 501 14951 14626 0 7:08PM ttys0000:00.03 qemu-system-x86_64 -m 4G -vga virtio -display default,show-cursor=on -usb -device usb-tablet -machine type=q35,accel=hvf -smp 2 -drive file=ubuntu.qcow2,if=virtio -cpu max -net nic -net user,hostfwd=tcp::50022-:22,hostfwd=tcp::8000-:80 -daemonize ``` To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1916343/+subscriptions
Re: [PATCH v6 76/82] target/arm: Remove unused fpst from VDOT_scalar
On Fri, 30 Apr 2021 at 22:29, Richard Henderson wrote: > > Cut and paste error from another pattern. > > Signed-off-by: Richard Henderson > --- > target/arm/translate-neon.c | 3 --- > 1 file changed, 3 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH 09/10] default-configs/devices: Remove implicy SEMIHOSTING config
Forgot to Cc Alex.
On 5/13/21 6:38 PM, Philippe Mathieu-Daudé wrote:
> Cmmit 56b5170c87e ("semihosting: Move ARM semihosting code to
> shared directories"), added the ARM_COMPATIBLE_SEMIHOSTING symbol
> which selects SEMIHOSTING.
>
> Since the ARM/RISC-V targets select ARM_COMPATIBLE_SEMIHOSTING,
> they don't need to select SEMIHOSTING manually. Simplify.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> default-configs/devices/arm-softmmu.mak | 1 -
> default-configs/devices/riscv32-softmmu.mak | 1 -
> default-configs/devices/riscv64-softmmu.mak | 1 -
> 3 files changed, 3 deletions(-)
>
> diff --git a/default-configs/devices/arm-softmmu.mak
> b/default-configs/devices/arm-softmmu.mak
> index 0500156a0c7..341d439de6f 100644
> --- a/default-configs/devices/arm-softmmu.mak
> +++ b/default-configs/devices/arm-softmmu.mak
> @@ -41,6 +41,5 @@ CONFIG_MICROBIT=y
> CONFIG_FSL_IMX25=y
> CONFIG_FSL_IMX7=y
> CONFIG_FSL_IMX6UL=y
> -CONFIG_SEMIHOSTING=y
> CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> CONFIG_ALLWINNER_H3=y
> diff --git a/default-configs/devices/riscv32-softmmu.mak
> b/default-configs/devices/riscv32-softmmu.mak
> index d847bd5692e..5c9ad2590ef 100644
> --- a/default-configs/devices/riscv32-softmmu.mak
> +++ b/default-configs/devices/riscv32-softmmu.mak
> @@ -3,7 +3,6 @@
> # Uncomment the following lines to disable these optional devices:
> #
> #CONFIG_PCI_DEVICES=n
> -CONFIG_SEMIHOSTING=y
> CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
>
> # Boards:
> diff --git a/default-configs/devices/riscv64-softmmu.mak
> b/default-configs/devices/riscv64-softmmu.mak
> index bc69301fa4a..8a92f0a2c74 100644
> --- a/default-configs/devices/riscv64-softmmu.mak
> +++ b/default-configs/devices/riscv64-softmmu.mak
> @@ -3,7 +3,6 @@
> # Uncomment the following lines to disable these optional devices:
> #
> #CONFIG_PCI_DEVICES=n
> -CONFIG_SEMIHOSTING=y
> CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
>
> # Boards:
>
Re: [PULL v2 0/1] readthedoc theme patch
On Thu, 13 May 2021 at 17:03, Marc-André Lureau
wrote:
> This should work, can you check?:
>
> diff --git a/docs/conf.py b/docs/conf.py
> index 3802b70d62..9e3d7cee0e 100644
> --- a/docs/conf.py
> +++ b/docs/conf.py
> @@ -29,6 +29,7 @@
> import os
> import sys
> import sphinx
> +from packaging.version import parse as parse_version
> from sphinx.errors import ConfigError
>
> # Make Sphinx fail cleanly if using an old Python, rather than obscurely
> @@ -162,7 +163,7 @@
> # Theme options are theme-specific and customize the look and feel of a theme
> # further. For a list of options available for each theme, see the
> # documentation.
> -if html_theme == 'sphinx_rtd_theme':
> +if parse_version(sphinx_rtd_theme.__version__) >= parse_version('0.4.3'):
> html_theme_options = {
> "style_nav_header_background": "#802400",
> }
This fails:
../../docs/meson.build:30: WARNING: /usr/bin/sphinx-build:
Configuration error:
There is a programable error in your configuration file:
Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/sphinx/config.py", line 157, in __init__
execfile_(filename, config)
File "/usr/lib/python3/dist-packages/sphinx/util/pycompat.py", line
150, in execfile_
exec_(code, _globals)
File "conf.py", line 32, in
from packaging.version import parse as parse_version
ModuleNotFoundError: No module named 'packaging'
> However, we agreed before to not have support fallbacks for missing deps.
> Perhaps we should require rtd >= 0.4.3 instead?
That would prevent the docs from building on too many platforms,
I think. 0.4.3 was only released upstream in Feb 2019.
thanks
-- PMM
