Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
Hi Anup,
On Sun, May 3, 2020 at 12:38 PM Anup Patel wrote:
>
> On Fri, May 1, 2020 at 9:26 PM Bin Meng wrote:
> >
> > From: Bin Meng
> >
> > The RISC-V generic platform is a flattened device tree (FDT) based
> > platform where all platform specific functionality is provided based
> > on FDT passed by previous booting stage. The support was added in
> > upstream opensbi recently.
> >
> > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range
> > limit override")
> > with the following changes since v0.7 release:
> >
> > 1bb00ab lib: No need to provide default PMP region using platform
> > callbacks
> > a9eac67 include: sbi_platform: Combine reboot and shutdown into one
> > callback
> > 6585fab lib: utils: Add SiFive test device
> > 4781545 platform: Add Nuclei UX600 platform
> > 3a326af scripts: adapt binary archive script for Nuclei UX600
> > 5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
> > 01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > 0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > 243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> > a39cd6f lib: utils: Add FDT match table based node lookup
> > dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > 66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > 19e966b lib: utils: Add fdt_parse_hart_id() function
> > 44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > 1ac794c include: Add array_size() macro
> > 8ff2b94 lib: utils: Add simple FDT timer framework
> > 76f0f81 lib: utils: Add simple FDT ipi framework
> > 75322a6 lib: utils: Add simple FDT irqchip framework
> > 76a8940 lib: utils: Add simple FDT serial framework
> > 7cc6fa4 lib: utils: Add simple FDT reset framework
> > 4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > f1aa9e5 platform: Add generic FDT based platform support
> > 1f21b99 lib: sbi: Print platform hart count at boot time
> > 2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > 4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
> >
> > Update our Makefile to build the generic platform instead of building
> > virt and sifive_u separately.
> >
> > Signed-off-by: Bin Meng
> > ---
> >
> > roms/Makefile | 30 --
> > roms/opensbi | 2 +-
> > 2 files changed, 9 insertions(+), 23 deletions(-)
> >
> > diff --git a/roms/Makefile b/roms/Makefile
> > index f9acf39..cb00628 100644
> > --- a/roms/Makefile
> > +++ b/roms/Makefile
> > @@ -64,10 +64,8 @@ default help:
> > @echo " u-boot.e500-- update u-boot.e500"
> > @echo " u-boot.sam460 -- update u-boot.sam460"
> > @echo " efi-- update UEFI (edk2) platform firmware"
> > - @echo " opensbi32-virt -- update OpenSBI for 32-bit virt
> > machine"
> > - @echo " opensbi64-virt -- update OpenSBI for 64-bit virt
> > machine"
> > - @echo " opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u
> > machine"
> > - @echo " opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u
> > machine"
> > + @echo " opensbi32-generic -- update OpenSBI for 32-bit generic
> > machine"
> > + @echo " opensbi64-generic -- update OpenSBI for 64-bit generic
> > machine"
> > @echo " bios-microvm -- update bios-microvm.bin (qboot)"
> > @echo " clean -- delete the files generated by the
> > previous" \
> > "build targets"
> > @@ -170,29 +168,17 @@ skiboot:
> > efi: edk2-basetools
> > $(MAKE) -f Makefile.edk2
> >
> > -opensbi32-virt:
> > +opensbi32-generic:
> > $(MAKE) -C opensbi \
> > CROSS_COMPILE=$(riscv32_cross_prefix) \
> > - PLATFORM="qemu/virt"
> > - cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin
> > ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > + PLATFORM="generic"
> > + cp opensbi/build/platform/generic/firmware/fw_jump.bin
> > ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
>
> I think you should copy fw_jump.elf as well because QEMU Spike
> platform needs it.
>
I believe we intended only to ship default bios images for virt and
sifive_u. Spike bios image was not shipped in previous QEMU version
too.
> >
> > -opensbi64-virt:
> > +opensbi64-generic:
> > $(MAKE) -C opensbi \
> > CROSS_COMPILE=$(riscv64_cross_prefix) \
> > - PLATFORM="qemu/virt"
> > - cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin
> > ../pc-bios/opensbi-riscv64-virt-fw_jump.bin
> > -
> > -opensbi32-si
[PATCH 0/4] hw/arm/nrf51: Extend tracing
Few patches while playing with the Zephyr Project. - better display of unimplemented peripheral accesses, - better display of timers use. Philippe Mathieu-Daudé (4): hw/arm/nrf51: Add NRF51_PERIPHERAL_SIZE definition hw/arm/nrf51_soc: Mark some peripherals as unimplemented hw/timer/nrf51_timer: Display timer ID in trace events hw/timer/nrf51_timer: Add trace event of counter value update include/hw/arm/nrf51.h | 7 +-- include/hw/i2c/microbit_i2c.h | 2 +- include/hw/timer/nrf51_timer.h | 1 + hw/arm/nrf51_soc.c | 20 ++-- hw/i2c/microbit_i2c.c | 2 +- hw/timer/nrf51_timer.c | 14 +++--- hw/timer/trace-events | 5 +++-- 7 files changed, 40 insertions(+), 11 deletions(-) -- 2.21.3
[PATCH 2/4] hw/arm/nrf51_soc: Mark some peripherals as unimplemented
Map some peripherals used by Zephyr Project:
https://github.com/zephyrproject-rtos/zephyr/blob/zephyr-v2.2.0/dts/arm/nordic/nrf51822.dtsi
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/arm/nrf51.h | 4
hw/arm/nrf51_soc.c | 11 +++
2 files changed, 15 insertions(+)
diff --git a/include/hw/arm/nrf51.h b/include/hw/arm/nrf51.h
index de836beaa4..46d0cfc7a1 100644
--- a/include/hw/arm/nrf51.h
+++ b/include/hw/arm/nrf51.h
@@ -25,11 +25,15 @@
#define NRF51_IOMEM_SIZE 0x2000
#define NRF51_PERIPHERAL_SIZE 0x1000
+#define NRF51_RADIO_BASE 0x40001000
#define NRF51_UART_BASE 0x40002000
#define NRF51_TWI_BASE0x40003000
+#define NRF51_GPIOTE_BASE 0x40006000
#define NRF51_TIMER_BASE 0x40008000
+#define NRF51_TEMP_BASE 0x4000c000
#define NRF51_RNG_BASE0x4000D000
#define NRF51_NVMC_BASE 0x4001E000
+#define NRF51_PPI_BASE0x4001f000
#define NRF51_GPIO_BASE 0x5000
#define NRF51_PRIVATE_BASE0xF000
diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
index e50473fd19..6212c5cb53 100644
--- a/hw/arm/nrf51_soc.c
+++ b/hw/arm/nrf51_soc.c
@@ -170,6 +170,17 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error
**errp)
memory_region_add_subregion_overlap(&s->container,
NRF51_IOMEM_BASE, &s->clock, -1);
+create_unimplemented_device("nrf51_soc.radio", NRF51_RADIO_BASE,
+NRF51_PERIPHERAL_SIZE);
+create_unimplemented_device("nrf51_soc.uarte", NRF51_UART_BASE,
+NRF51_PERIPHERAL_SIZE);
+create_unimplemented_device("nrf51_soc.gpiote", NRF51_GPIOTE_BASE,
+NRF51_PERIPHERAL_SIZE);
+create_unimplemented_device("nrf51_soc.temp", NRF51_TEMP_BASE,
+NRF51_PERIPHERAL_SIZE);
+create_unimplemented_device("nrf51_soc.ppi", NRF51_PPI_BASE,
+NRF51_PERIPHERAL_SIZE);
+
create_unimplemented_device("nrf51_soc.io", NRF51_IOMEM_BASE,
NRF51_IOMEM_SIZE);
create_unimplemented_device("nrf51_soc.private",
--
2.21.3
[PATCH 1/4] hw/arm/nrf51: Add NRF51_PERIPHERAL_SIZE definition
On the NRF51 series, all peripherals have a fixed I/O size
of 4KiB. Define NRF51_PERIPHERAL_SIZE and use it.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/arm/nrf51.h| 3 +--
include/hw/i2c/microbit_i2c.h | 2 +-
hw/arm/nrf51_soc.c| 4 ++--
hw/i2c/microbit_i2c.c | 2 +-
hw/timer/nrf51_timer.c| 2 +-
5 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/hw/arm/nrf51.h b/include/hw/arm/nrf51.h
index 1008fee6c9..de836beaa4 100644
--- a/include/hw/arm/nrf51.h
+++ b/include/hw/arm/nrf51.h
@@ -24,11 +24,10 @@
#define NRF51_IOMEM_BASE 0x4000
#define NRF51_IOMEM_SIZE 0x2000
+#define NRF51_PERIPHERAL_SIZE 0x1000
#define NRF51_UART_BASE 0x40002000
#define NRF51_TWI_BASE0x40003000
-#define NRF51_TWI_SIZE0x1000
#define NRF51_TIMER_BASE 0x40008000
-#define NRF51_TIMER_SIZE 0x1000
#define NRF51_RNG_BASE0x4000D000
#define NRF51_NVMC_BASE 0x4001E000
#define NRF51_GPIO_BASE 0x5000
diff --git a/include/hw/i2c/microbit_i2c.h b/include/hw/i2c/microbit_i2c.h
index aad636127e..2bff36680c 100644
--- a/include/hw/i2c/microbit_i2c.h
+++ b/include/hw/i2c/microbit_i2c.h
@@ -29,7 +29,7 @@
#define MICROBIT_I2C(obj) \
OBJECT_CHECK(MicrobitI2CState, (obj), TYPE_MICROBIT_I2C)
-#define MICROBIT_I2C_NREGS (NRF51_TWI_SIZE / sizeof(uint32_t))
+#define MICROBIT_I2C_NREGS (NRF51_PERIPHERAL_SIZE / sizeof(uint32_t))
typedef struct {
SysBusDevice parent_obj;
diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
index 57eff63f0d..e50473fd19 100644
--- a/hw/arm/nrf51_soc.c
+++ b/hw/arm/nrf51_soc.c
@@ -156,7 +156,7 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error
**errp)
return;
}
-base_addr = NRF51_TIMER_BASE + i * NRF51_TIMER_SIZE;
+base_addr = NRF51_TIMER_BASE + i * NRF51_PERIPHERAL_SIZE;
sysbus_mmio_map(SYS_BUS_DEVICE(&s->timer[i]), 0, base_addr);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->timer[i]), 0,
@@ -166,7 +166,7 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error
**errp)
/* STUB Peripherals */
memory_region_init_io(&s->clock, OBJECT(dev_soc), &clock_ops, NULL,
- "nrf51_soc.clock", 0x1000);
+ "nrf51_soc.clock", NRF51_PERIPHERAL_SIZE);
memory_region_add_subregion_overlap(&s->container,
NRF51_IOMEM_BASE, &s->clock, -1);
diff --git a/hw/i2c/microbit_i2c.c b/hw/i2c/microbit_i2c.c
index 4661f05253..8024739820 100644
--- a/hw/i2c/microbit_i2c.c
+++ b/hw/i2c/microbit_i2c.c
@@ -100,7 +100,7 @@ static void microbit_i2c_realize(DeviceState *dev, Error
**errp)
MicrobitI2CState *s = MICROBIT_I2C(dev);
memory_region_init_io(&s->iomem, OBJECT(s), µbit_i2c_ops, s,
- "microbit.twi", NRF51_TWI_SIZE);
+ "microbit.twi", NRF51_PERIPHERAL_SIZE);
sysbus_init_mmio(sbd, &s->iomem);
}
diff --git a/hw/timer/nrf51_timer.c b/hw/timer/nrf51_timer.c
index e04046eb15..bc82c85a6f 100644
--- a/hw/timer/nrf51_timer.c
+++ b/hw/timer/nrf51_timer.c
@@ -313,7 +313,7 @@ static void nrf51_timer_init(Object *obj)
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
memory_region_init_io(&s->iomem, obj, &rng_ops, s,
-TYPE_NRF51_TIMER, NRF51_TIMER_SIZE);
+ TYPE_NRF51_TIMER, NRF51_PERIPHERAL_SIZE);
sysbus_init_mmio(sbd, &s->iomem);
sysbus_init_irq(sbd, &s->irq);
--
2.21.3
[PATCH 4/4] hw/timer/nrf51_timer: Add trace event of counter value update
Add trace event to display timer's counter value updates. Signed-off-by: Philippe Mathieu-Daudé --- hw/timer/nrf51_timer.c | 1 + hw/timer/trace-events | 1 + 2 files changed, 2 insertions(+) diff --git a/hw/timer/nrf51_timer.c b/hw/timer/nrf51_timer.c index 38cea0542e..42be79c736 100644 --- a/hw/timer/nrf51_timer.c +++ b/hw/timer/nrf51_timer.c @@ -240,6 +240,7 @@ static void nrf51_timer_write(void *opaque, hwaddr offset, idx = (offset - NRF51_TIMER_TASK_CAPTURE_0) / 4; s->cc[idx] = s->counter; +trace_nrf51_timer_set_count(s->id, idx, s->counter); } break; case NRF51_TIMER_EVENT_COMPARE_0 ... NRF51_TIMER_EVENT_COMPARE_3: diff --git a/hw/timer/trace-events b/hw/timer/trace-events index 43b605cc75..80ea197594 100644 --- a/hw/timer/trace-events +++ b/hw/timer/trace-events @@ -69,6 +69,7 @@ cmsdk_apb_dualtimer_reset(void) "CMSDK APB dualtimer: reset" # nrf51_timer.c nrf51_timer_read(uint8_t timer_id, uint64_t addr, uint32_t value, unsigned size) "timer %u read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u" nrf51_timer_write(uint8_t timer_id, uint64_t addr, uint32_t value, unsigned size) "timer %u write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u" +nrf51_timer_set_count(uint8_t timer_id, uint8_t counter_id, uint32_t value) "timer %u counter %u count 0x%" PRIx32 # bcm2835_systmr.c bcm2835_systmr_irq(bool enable) "timer irq state %u" -- 2.21.3
[PATCH 3/4] hw/timer/nrf51_timer: Display timer ID in trace events
The NRF51 series SoC have 3 timer peripherals, each having
4 counters. To help differentiate which peripheral is accessed,
display the timer ID in the trace events.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/timer/nrf51_timer.h | 1 +
hw/arm/nrf51_soc.c | 5 +
hw/timer/nrf51_timer.c | 11 +--
hw/timer/trace-events | 4 ++--
4 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/include/hw/timer/nrf51_timer.h b/include/hw/timer/nrf51_timer.h
index 85cad2300d..eb6815f21d 100644
--- a/include/hw/timer/nrf51_timer.h
+++ b/include/hw/timer/nrf51_timer.h
@@ -59,6 +59,7 @@ typedef struct NRF51TimerState {
MemoryRegion iomem;
qemu_irq irq;
+uint8_t id;
QEMUTimer timer;
int64_t timer_start_ns;
int64_t update_counter_ns;
diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
index 6212c5cb53..44b2624e8e 100644
--- a/hw/arm/nrf51_soc.c
+++ b/hw/arm/nrf51_soc.c
@@ -150,6 +150,11 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error
**errp)
/* TIMER */
for (i = 0; i < NRF51_NUM_TIMERS; i++) {
+object_property_set_uint(OBJECT(&s->timer[i]), i, "id", &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
if (err) {
error_propagate(errp, err);
diff --git a/hw/timer/nrf51_timer.c b/hw/timer/nrf51_timer.c
index bc82c85a6f..38cea0542e 100644
--- a/hw/timer/nrf51_timer.c
+++ b/hw/timer/nrf51_timer.c
@@ -17,6 +17,7 @@
#include "hw/arm/nrf51.h"
#include "hw/irq.h"
#include "hw/timer/nrf51_timer.h"
+#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "trace.h"
@@ -185,7 +186,7 @@ static uint64_t nrf51_timer_read(void *opaque, hwaddr
offset, unsigned int size)
__func__, offset);
}
-trace_nrf51_timer_read(offset, r, size);
+trace_nrf51_timer_read(s->id, offset, r, size);
return r;
}
@@ -197,7 +198,7 @@ static void nrf51_timer_write(void *opaque, hwaddr offset,
uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
size_t idx;
-trace_nrf51_timer_write(offset, value, size);
+trace_nrf51_timer_write(s->id, offset, value, size);
switch (offset) {
case NRF51_TIMER_TASK_START:
@@ -372,12 +373,18 @@ static const VMStateDescription vmstate_nrf51_timer = {
}
};
+static Property nrf51_timer_properties[] = {
+DEFINE_PROP_UINT8("id", NRF51TimerState, id, 0),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void nrf51_timer_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
dc->reset = nrf51_timer_reset;
dc->vmsd = &vmstate_nrf51_timer;
+device_class_set_props(dc, nrf51_timer_properties);
}
static const TypeInfo nrf51_timer_info = {
diff --git a/hw/timer/trace-events b/hw/timer/trace-events
index 29fda7870e..43b605cc75 100644
--- a/hw/timer/trace-events
+++ b/hw/timer/trace-events
@@ -67,8 +67,8 @@ cmsdk_apb_dualtimer_write(uint64_t offset, uint64_t data,
unsigned size) "CMSDK
cmsdk_apb_dualtimer_reset(void) "CMSDK APB dualtimer: reset"
# nrf51_timer.c
-nrf51_timer_read(uint64_t addr, uint32_t value, unsigned size) "read addr 0x%"
PRIx64 " data 0x%" PRIx32 " size %u"
-nrf51_timer_write(uint64_t addr, uint32_t value, unsigned size) "write addr
0x%" PRIx64 " data 0x%" PRIx32 " size %u"
+nrf51_timer_read(uint8_t timer_id, uint64_t addr, uint32_t value, unsigned
size) "timer %u read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
+nrf51_timer_write(uint8_t timer_id, uint64_t addr, uint32_t value, unsigned
size) "timer %u write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
# bcm2835_systmr.c
bcm2835_systmr_irq(bool enable) "timer irq state %u"
--
2.21.3
[Bug 1805256] Re: qemu-img hangs on rcu_call_ready_event logic in Aarch64 when converting images
PPA created with temporarily workaround in comment #34. https://launchpad.net/~ikepanhc/+archive/ubuntu/lp1805256 This PPA can solve temporarily but is not acceptable for offical release. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1805256 Title: qemu-img hangs on rcu_call_ready_event logic in Aarch64 when converting images Status in kunpeng920: Incomplete Status in QEMU: In Progress Status in qemu package in Ubuntu: Incomplete Status in qemu source package in Bionic: Incomplete Status in qemu source package in Disco: Incomplete Status in qemu source package in Eoan: Incomplete Status in qemu source package in Focal: Incomplete Bug description: Command: qemu-img convert -f qcow2 -O qcow2 ./disk01.qcow2 ./output.qcow2 Hangs indefinitely approximately 30% of the runs. Workaround: qemu-img convert -m 1 -f qcow2 -O qcow2 ./disk01.qcow2 ./output.qcow2 Run "qemu-img convert" with "a single coroutine" to avoid this issue. (gdb) thread 1 ... (gdb) bt #0 0xbf1ad81c in __GI_ppoll #1 0xaabcf73c in ppoll #2 qemu_poll_ns #3 0xaabd0764 in os_host_main_loop_wait #4 main_loop_wait ... (gdb) thread 2 ... (gdb) bt #0 syscall () #1 0xaabd41cc in qemu_futex_wait #2 qemu_event_wait (ev=ev@entry=0xaac86ce8 ) #3 0xaabed05c in call_rcu_thread #4 0xaabd34c8 in qemu_thread_start #5 0xbf25c880 in start_thread #6 0xbf1b6b9c in thread_start () (gdb) thread 3 ... (gdb) bt #0 0xbf11aa20 in __GI___sigtimedwait #1 0xbf2671b4 in __sigwait #2 0xaabd1ddc in sigwait_compat #3 0xaabd34c8 in qemu_thread_start #4 0xbf25c880 in start_thread #5 0xbf1b6b9c in thread_start (gdb) run Starting program: /usr/bin/qemu-img convert -f qcow2 -O qcow2 ./disk01.ext4.qcow2 ./output.qcow2 [New Thread 0xbec5ad90 (LWP 72839)] [New Thread 0xbe459d90 (LWP 72840)] [New Thread 0xbdb57d90 (LWP 72841)] [New Thread 0xacac9d90 (LWP 72859)] [New Thread 0xa7ffed90 (LWP 72860)] [New Thread 0xa77fdd90 (LWP 72861)] [New Thread 0xa6ffcd90 (LWP 72862)] [New Thread 0xa67fbd90 (LWP 72863)] [New Thread 0xa5ffad90 (LWP 72864)] [Thread 0xa5ffad90 (LWP 72864) exited] [Thread 0xa6ffcd90 (LWP 72862) exited] [Thread 0xa77fdd90 (LWP 72861) exited] [Thread 0xbdb57d90 (LWP 72841) exited] [Thread 0xa67fbd90 (LWP 72863) exited] [Thread 0xacac9d90 (LWP 72859) exited] [Thread 0xa7ffed90 (LWP 72860) exited] """ All the tasks left are blocked in a system call, so no task left to call qemu_futex_wake() to unblock thread #2 (in futex()), which would unblock thread #1 (doing poll() in a pipe with thread #2). Those 7 threads exit before disk conversion is complete (sometimes in the beginning, sometimes at the end). [ Original Description ] On the HiSilicon D06 system - a 96 core NUMA arm64 box - qemu-img frequently hangs (~50% of the time) with this command: qemu-img convert -f qcow2 -O qcow2 /tmp/cloudimg /tmp/cloudimg2 Where "cloudimg" is a standard qcow2 Ubuntu cloud image. This qcow2->qcow2 conversion happens to be something uvtool does every time it fetches images. Once hung, attaching gdb gives the following backtrace: (gdb) bt #0 0xae4f8154 in __GI_ppoll (fds=0xe8a67dc0, nfds=187650274213760, timeout=, timeout@entry=0x0, sigmask=0xc123b950) at ../sysdeps/unix/sysv/linux/ppoll.c:39 #1 0xbbefaf00 in ppoll (__ss=0x0, __timeout=0x0, __nfds=, __fds=) at /usr/include/aarch64-linux-gnu/bits/poll2.h:77 #2 qemu_poll_ns (fds=, nfds=, timeout=timeout@entry=-1) at util/qemu-timer.c:322 #3 0xbbefbf80 in os_host_main_loop_wait (timeout=-1) at util/main-loop.c:233 #4 main_loop_wait (nonblocking=) at util/main-loop.c:497 #5 0xbbe2aa30 in convert_do_copy (s=0xc123bb58) at qemu-img.c:1980 #6 img_convert (argc=, argv=) at qemu-img.c:2456 #7 0xbbe2333c in main (argc=7, argv=) at qemu-img.c:4975 Reproduced w/ latest QEMU git (@ 53744e0a182) To manage notifications about this bug go to: https://bugs.launchpad.net/kunpeng920/+bug/1805256/+subscriptions
Re: [PATCH 5/8] pc-bios: s390x: Move panic() into header and add infinite loop
On 04.05.20 08:46, Janosch Frank wrote:
> On 4/30/20 5:42 PM, David Hildenbrand wrote:
>> On 24.03.20 16:08, Janosch Frank wrote:
>>> panic() was defined for the ccw and net bios, i.e. twice, so it's
>>> cleaner to rather put it into the header.
>>>
>>> Also let's add an infinite loop into the assembly of disabled_wait() so
>>> the caller doesn't need to take care of it.
>>>
>>> Signed-off-by: Janosch Frank
>>> Reviewed-by: Pierre Morel
>>> ---
>>> pc-bios/s390-ccw/main.c | 7 ---
>>> pc-bios/s390-ccw/netmain.c | 8
>>> pc-bios/s390-ccw/s390-ccw.h | 7 ++-
>>> pc-bios/s390-ccw/start.S| 5 +++--
>>> 4 files changed, 9 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
>>> index 8b912454c940a390..146a50760bc70af7 100644
>>> --- a/pc-bios/s390-ccw/main.c
>>> +++ b/pc-bios/s390-ccw/main.c
>>> @@ -46,13 +46,6 @@ void write_iplb_location(void)
>>> lowcore->ptr_iplb = ptr2u32(&iplb);
>>> }
>>>
>>> -void panic(const char *string)
>>> -{
>>> -sclp_print(string);
>>> -disabled_wait();
>>> -while (1) { }
>>> -}
>>
>> I remember there was a reason why to add the endless loop afterwards.
>> Maybe because some special machine checks can actually wake it up? Or
>> buggy hypervisor?
>>
>> Anyhow, the kernel also does
>>
>> __load_psw(psw);
>> while (1);
>>
>> so it's best we keep that.
>>
>>
>> With the endless loop re-added
>
> Well, I added a loop into the disabled_wait assembly and removed it from
> the C code. It's even in the commit message.
Whops, missed that, looks good to me then!
--
Thanks,
David / dhildenb
Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
On Mon, May 4, 2020 at 12:46 PM Bin Meng wrote:
>
> Hi Anup,
>
> On Sun, May 3, 2020 at 12:38 PM Anup Patel wrote:
> >
> > On Fri, May 1, 2020 at 9:26 PM Bin Meng wrote:
> > >
> > > From: Bin Meng
> > >
> > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > platform where all platform specific functionality is provided based
> > > on FDT passed by previous booting stage. The support was added in
> > > upstream opensbi recently.
> > >
> > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range
> > > limit override")
> > > with the following changes since v0.7 release:
> > >
> > > 1bb00ab lib: No need to provide default PMP region using platform
> > > callbacks
> > > a9eac67 include: sbi_platform: Combine reboot and shutdown into one
> > > callback
> > > 6585fab lib: utils: Add SiFive test device
> > > 4781545 platform: Add Nuclei UX600 platform
> > > 3a326af scripts: adapt binary archive script for Nuclei UX600
> > > 5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > e6c1345 lib: utils/serial: Skip baudrate config if input frequency is
> > > zero
> > > 01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > 0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > 243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> > > a39cd6f lib: utils: Add FDT match table based node lookup
> > > dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > 66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > 19e966b lib: utils: Add fdt_parse_hart_id() function
> > > 44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > 1ac794c include: Add array_size() macro
> > > 8ff2b94 lib: utils: Add simple FDT timer framework
> > > 76f0f81 lib: utils: Add simple FDT ipi framework
> > > 75322a6 lib: utils: Add simple FDT irqchip framework
> > > 76a8940 lib: utils: Add simple FDT serial framework
> > > 7cc6fa4 lib: utils: Add simple FDT reset framework
> > > 4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > f1aa9e5 platform: Add generic FDT based platform support
> > > 1f21b99 lib: sbi: Print platform hart count at boot time
> > > 2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > 4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit
> > > override
> > >
> > > Update our Makefile to build the generic platform instead of building
> > > virt and sifive_u separately.
> > >
> > > Signed-off-by: Bin Meng
> > > ---
> > >
> > > roms/Makefile | 30 --
> > > roms/opensbi | 2 +-
> > > 2 files changed, 9 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/roms/Makefile b/roms/Makefile
> > > index f9acf39..cb00628 100644
> > > --- a/roms/Makefile
> > > +++ b/roms/Makefile
> > > @@ -64,10 +64,8 @@ default help:
> > > @echo " u-boot.e500-- update u-boot.e500"
> > > @echo " u-boot.sam460 -- update u-boot.sam460"
> > > @echo " efi-- update UEFI (edk2) platform
> > > firmware"
> > > - @echo " opensbi32-virt -- update OpenSBI for 32-bit virt
> > > machine"
> > > - @echo " opensbi64-virt -- update OpenSBI for 64-bit virt
> > > machine"
> > > - @echo " opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u
> > > machine"
> > > - @echo " opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u
> > > machine"
> > > + @echo " opensbi32-generic -- update OpenSBI for 32-bit generic
> > > machine"
> > > + @echo " opensbi64-generic -- update OpenSBI for 64-bit generic
> > > machine"
> > > @echo " bios-microvm -- update bios-microvm.bin (qboot)"
> > > @echo " clean -- delete the files generated by the
> > > previous" \
> > > "build targets"
> > > @@ -170,29 +168,17 @@ skiboot:
> > > efi: edk2-basetools
> > > $(MAKE) -f Makefile.edk2
> > >
> > > -opensbi32-virt:
> > > +opensbi32-generic:
> > > $(MAKE) -C opensbi \
> > > CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > - PLATFORM="qemu/virt"
> > > - cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin
> > > ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > > + PLATFORM="generic"
> > > + cp opensbi/build/platform/generic/firmware/fw_jump.bin
> > > ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
> >
> > I think you should copy fw_jump.elf as well because QEMU Spike
> > platform needs it.
> >
>
> I believe we intended only to ship default bios images for virt and
> sifive_u. Spike bios image was not shipped in previous QEMU version
> too.
Is there a specific reason for not shipping
Re: [PATCH v22 3/4] qcow2: add zstd cluster compression
On 30.04.20 15:56, Denis Plotnikov wrote:
>
>
> On 30.04.2020 14:47, Max Reitz wrote:
>> On 30.04.20 11:48, Denis Plotnikov wrote:
>>>
>>> On 30.04.2020 11:26, Max Reitz wrote:
On 29.04.20 15:02, Vladimir Sementsov-Ogievskiy wrote:
> 29.04.2020 15:17, Max Reitz wrote:
>> On 29.04.20 12:37, Vladimir Sementsov-Ogievskiy wrote:
>>> 29.04.2020 13:24, Max Reitz wrote:
On 28.04.20 22:00, Denis Plotnikov wrote:
> zstd significantly reduces cluster compression time.
> It provides better compression performance maintaining
> the same level of the compression ratio in comparison with
> zlib, which, at the moment, is the only compression
> method available.
>
> The performance test results:
> Test compresses and decompresses qemu qcow2 image with just
> installed rhel-7.6 guest.
> Image cluster size: 64K. Image on disk size: 2.2G
>
> The test was conducted with brd disk to reduce the influence
> of disk subsystem to the test results.
> The results is given in seconds.
>
> compress cmd:
> time ./qemu-img convert -O qcow2 -c -o
> compression_type=[zlib|zstd]
> src.img [zlib|zstd]_compressed.img
> decompress cmd
> time ./qemu-img convert -O qcow2
> [zlib|zstd]_compressed.img uncompressed.img
>
> compression decompression
> zlib zstd zlib zstd
>
> real 65.5 16.3 (-75 %) 1.9 1.6 (-16 %)
> user 65.0 15.8 5.3 2.5
> sys 3.3 0.2 2.0 2.0
>
> Both ZLIB and ZSTD gave the same compression ratio: 1.57
> compressed image size in both cases: 1.4G
>
> Signed-off-by: Denis Plotnikov
> QAPI part:
> Acked-by: Markus Armbruster
> ---
> docs/interop/qcow2.txt | 1 +
> configure | 2 +-
> qapi/block-core.json | 3 +-
> block/qcow2-threads.c | 169
> +
> block/qcow2.c | 7 ++
> slirp | 2 +-
> 6 files changed, 181 insertions(+), 3 deletions(-)
[...]
> diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
> index 7dbaf53489..a0b12e1b15 100644
> --- a/block/qcow2-threads.c
> +++ b/block/qcow2-threads.c
[...]
> +static ssize_t qcow2_zstd_decompress(void *dest, size_t
> dest_size,
> + const void *src, size_t
> src_size)
> +{
[...]
> + /*
> + * The compressed stream from the input buffer may consist of
> more
> + * than one zstd frame.
Can it?
>>> If not, we must require it in the specification.
>> Actually, now that you mention it, it would make sense anyway to add
>> some note to the specification on what exactly compressed with zstd
>> means.
>>
>>> Hmm. If at some point
>>> we'll want multi-threaded compression of one big (2M) cluster..
>>> Could
>>> this be implemented with zstd lib, if multiple frames are allowed,
>>> will
>>> allowing multiple frames help? I don't know actually, but I think
>>> better
>>> not to forbid it. On the other hand, I don't see any benefit in
>>> large
>>> compressed clusters. At least, in our scenarios (for compressed
>>> backups)
>>> we use 64k compressed clusters, for good granularity of incremental
>>> backups (when for running vm we use 1M clusters).
>> Is it really that important? Naïvely, it sounds rather
>> complicated to
>> introduce multithreading into block drivers.
> It is already here: compression and encryption already multithreaded.
> But of course, one cluster is handled in one thread.
Ah, good. I forgot.
>> (Also, as for compression, it can only be used in backup scenarios
>> anyway, where you write many clusters at once. So parallelism on the
>> cluster level should sufficient to get high usage, and it would
>> benefit
>> all compression types and cluster sizes.)
>>
> Yes it works in this way already :)
Well, OK then.
> So, we don't know do we want one frame restriction or not. Do you
> have a
> preference?
*shrug*
Seems like it would be preferential to allow multiple frames still. A
note in the spec would be nice (i.e., streaming format, multiple frames
per cluster possible).
>>> We don't mention anything about zlib compressing details in
Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
Hi Anup,
On Mon, May 4, 2020 at 3:52 PM Anup Patel wrote:
>
> On Mon, May 4, 2020 at 12:46 PM Bin Meng wrote:
> >
> > Hi Anup,
> >
> > On Sun, May 3, 2020 at 12:38 PM Anup Patel wrote:
> > >
> > > On Fri, May 1, 2020 at 9:26 PM Bin Meng wrote:
> > > >
> > > > From: Bin Meng
> > > >
> > > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > > platform where all platform specific functionality is provided based
> > > > on FDT passed by previous booting stage. The support was added in
> > > > upstream opensbi recently.
> > > >
> > > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush
> > > > range limit override")
> > > > with the following changes since v0.7 release:
> > > >
> > > > 1bb00ab lib: No need to provide default PMP region using platform
> > > > callbacks
> > > > a9eac67 include: sbi_platform: Combine reboot and shutdown into one
> > > > callback
> > > > 6585fab lib: utils: Add SiFive test device
> > > > 4781545 platform: Add Nuclei UX600 platform
> > > > 3a326af scripts: adapt binary archive script for Nuclei UX600
> > > > 5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > > e6c1345 lib: utils/serial: Skip baudrate config if input frequency is
> > > > zero
> > > > 01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > > 0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > > 243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > > e3ad7c1 lib: utils: Rename fdt_parse_clint() to
> > > > fdt_parse_compat_addr()
> > > > a39cd6f lib: utils: Add FDT match table based node lookup
> > > > dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > > 66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > > 19e966b lib: utils: Add fdt_parse_hart_id() function
> > > > 44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > > f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > > 1ac794c include: Add array_size() macro
> > > > 8ff2b94 lib: utils: Add simple FDT timer framework
> > > > 76f0f81 lib: utils: Add simple FDT ipi framework
> > > > 75322a6 lib: utils: Add simple FDT irqchip framework
> > > > 76a8940 lib: utils: Add simple FDT serial framework
> > > > 7cc6fa4 lib: utils: Add simple FDT reset framework
> > > > 4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > > f1aa9e5 platform: Add generic FDT based platform support
> > > > 1f21b99 lib: sbi: Print platform hart count at boot time
> > > > 2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > > 4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit
> > > > override
> > > >
> > > > Update our Makefile to build the generic platform instead of building
> > > > virt and sifive_u separately.
> > > >
> > > > Signed-off-by: Bin Meng
> > > > ---
> > > >
> > > > roms/Makefile | 30 --
> > > > roms/opensbi | 2 +-
> > > > 2 files changed, 9 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/roms/Makefile b/roms/Makefile
> > > > index f9acf39..cb00628 100644
> > > > --- a/roms/Makefile
> > > > +++ b/roms/Makefile
> > > > @@ -64,10 +64,8 @@ default help:
> > > > @echo " u-boot.e500-- update u-boot.e500"
> > > > @echo " u-boot.sam460 -- update u-boot.sam460"
> > > > @echo " efi-- update UEFI (edk2) platform
> > > > firmware"
> > > > - @echo " opensbi32-virt -- update OpenSBI for 32-bit virt
> > > > machine"
> > > > - @echo " opensbi64-virt -- update OpenSBI for 64-bit virt
> > > > machine"
> > > > - @echo " opensbi32-sifive_u -- update OpenSBI for 32-bit
> > > > sifive_u machine"
> > > > - @echo " opensbi64-sifive_u -- update OpenSBI for 64-bit
> > > > sifive_u machine"
> > > > + @echo " opensbi32-generic -- update OpenSBI for 32-bit
> > > > generic machine"
> > > > + @echo " opensbi64-generic -- update OpenSBI for 64-bit
> > > > generic machine"
> > > > @echo " bios-microvm -- update bios-microvm.bin (qboot)"
> > > > @echo " clean -- delete the files generated by
> > > > the previous" \
> > > > "build targets"
> > > > @@ -170,29 +168,17 @@ skiboot:
> > > > efi: edk2-basetools
> > > > $(MAKE) -f Makefile.edk2
> > > >
> > > > -opensbi32-virt:
> > > > +opensbi32-generic:
> > > > $(MAKE) -C opensbi \
> > > > CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > > - PLATFORM="qemu/virt"
> > > > - cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin
> > > > ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > > > + PLATFORM="generic"
> > > > + cp opensbi/build/platform/generic/firmware/fw_jump.bin
> > > > ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
> > >
> > > I thi
Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
On Mon, May 4, 2020 at 4:00 PM Bin Meng wrote:
>
> Hi Anup,
>
> On Mon, May 4, 2020 at 3:52 PM Anup Patel wrote:
> >
> > On Mon, May 4, 2020 at 12:46 PM Bin Meng wrote:
> > >
> > > Hi Anup,
> > >
> > > On Sun, May 3, 2020 at 12:38 PM Anup Patel wrote:
> > > >
> > > > On Fri, May 1, 2020 at 9:26 PM Bin Meng wrote:
> > > > >
> > > > > From: Bin Meng
> > > > >
> > > > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > > > platform where all platform specific functionality is provided based
> > > > > on FDT passed by previous booting stage. The support was added in
> > > > > upstream opensbi recently.
> > > > >
> > > > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush
> > > > > range limit override")
> > > > > with the following changes since v0.7 release:
> > > > >
> > > > > 1bb00ab lib: No need to provide default PMP region using platform
> > > > > callbacks
> > > > > a9eac67 include: sbi_platform: Combine reboot and shutdown into one
> > > > > callback
> > > > > 6585fab lib: utils: Add SiFive test device
> > > > > 4781545 platform: Add Nuclei UX600 platform
> > > > > 3a326af scripts: adapt binary archive script for Nuclei UX600
> > > > > 5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > > > e6c1345 lib: utils/serial: Skip baudrate config if input frequency
> > > > > is zero
> > > > > 01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > > > 0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > > > 243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > > > e3ad7c1 lib: utils: Rename fdt_parse_clint() to
> > > > > fdt_parse_compat_addr()
> > > > > a39cd6f lib: utils: Add FDT match table based node lookup
> > > > > dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > > > 66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > > > 19e966b lib: utils: Add fdt_parse_hart_id() function
> > > > > 44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > > > f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > > > 1ac794c include: Add array_size() macro
> > > > > 8ff2b94 lib: utils: Add simple FDT timer framework
> > > > > 76f0f81 lib: utils: Add simple FDT ipi framework
> > > > > 75322a6 lib: utils: Add simple FDT irqchip framework
> > > > > 76a8940 lib: utils: Add simple FDT serial framework
> > > > > 7cc6fa4 lib: utils: Add simple FDT reset framework
> > > > > 4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > > > f1aa9e5 platform: Add generic FDT based platform support
> > > > > 1f21b99 lib: sbi: Print platform hart count at boot time
> > > > > 2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > > > 4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit
> > > > > override
> > > > >
> > > > > Update our Makefile to build the generic platform instead of building
> > > > > virt and sifive_u separately.
> > > > >
> > > > > Signed-off-by: Bin Meng
> > > > > ---
> > > > >
> > > > > roms/Makefile | 30 --
> > > > > roms/opensbi | 2 +-
> > > > > 2 files changed, 9 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/roms/Makefile b/roms/Makefile
> > > > > index f9acf39..cb00628 100644
> > > > > --- a/roms/Makefile
> > > > > +++ b/roms/Makefile
> > > > > @@ -64,10 +64,8 @@ default help:
> > > > > @echo " u-boot.e500-- update u-boot.e500"
> > > > > @echo " u-boot.sam460 -- update u-boot.sam460"
> > > > > @echo " efi-- update UEFI (edk2) platform
> > > > > firmware"
> > > > > - @echo " opensbi32-virt -- update OpenSBI for 32-bit virt
> > > > > machine"
> > > > > - @echo " opensbi64-virt -- update OpenSBI for 64-bit virt
> > > > > machine"
> > > > > - @echo " opensbi32-sifive_u -- update OpenSBI for 32-bit
> > > > > sifive_u machine"
> > > > > - @echo " opensbi64-sifive_u -- update OpenSBI for 64-bit
> > > > > sifive_u machine"
> > > > > + @echo " opensbi32-generic -- update OpenSBI for 32-bit
> > > > > generic machine"
> > > > > + @echo " opensbi64-generic -- update OpenSBI for 64-bit
> > > > > generic machine"
> > > > > @echo " bios-microvm -- update bios-microvm.bin
> > > > > (qboot)"
> > > > > @echo " clean -- delete the files generated by
> > > > > the previous" \
> > > > > "build targets"
> > > > > @@ -170,29 +168,17 @@ skiboot:
> > > > > efi: edk2-basetools
> > > > > $(MAKE) -f Makefile.edk2
> > > > >
> > > > > -opensbi32-virt:
> > > > > +opensbi32-generic:
> > > > > $(MAKE) -C opensbi \
> > > > > CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > > > - PLATFORM="qemu/virt"
> > > > > - cp opensbi/build/platform/qemu/
Re: [PULL 0/6] virtiofs queue
* Peter Maydell (peter.mayd...@linaro.org) wrote: > On Fri, 1 May 2020 at 20:16, Dr. David Alan Gilbert (git) > wrote: > > > > From: "Dr. David Alan Gilbert" > > > > The following changes since commit 1c47613588ccff44422d4bdeea0dc36a0a308ec7: > > > > Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into > > staging (2020-04-30 19:25:41 +0100) > > > > are available in the Git repository at: > > > > https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20200501 > > > > for you to fetch changes up to 66502bbca37ca7a3bfa57e82cfc03b89a7a11eae: > > > > virtiofsd: drop all capabilities in the wait parent process (2020-05-01 > > 20:05:37 +0100) > > > > > > virtiofsd: Pull 2020-05-01 (includes CVE fix) > > > > This set includes a security fix, other fixes and improvements. > > > > Security fix: > > The security fix is for CVE-2020-10717 where, on low RAM hosts, > > the guest can potentially exceed the maximum fd limit. > > This fix adds some more configuration so that the user > > can explicitly set the limit. > > Thank you to Yuval Avrahami for reporting this. > > > > Fixes: > > > > Recursive mounting of the exported directory is now used in > > the sandbox, such that if there was a mount underneath present at > > the time the virtiofsd was started, that mount is also > > visible to the guest; in the existing code, only mounts that > > happened after startup were visible. > > > > Security improvements: > > > > The jailing for /proc/self/fd is improved - but it's something > > that shouldn't be accessible anyway. > > > > Most capabilities are now dropped at startup; again this shouldn't > > change any behaviour but is extra protection. > > > > > > > Applied, thanks. > > Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 > for any user-visible changes. > > I notice you didn't include the usual Cc: qemu-sta...@nongnu.org > lines in the commits to be backported, but I think the stable > branch maintainers can deal with the occasional manual notification. Thanks, yes I sent a mail to qemu-stable as a reply to the series saying which patches I thought should be for stable. Dave > thanks > -- PMM > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[PATCH 2/3] hw/sh4: Extract timer definitions to 'hw/timer/tmu012.h'
Extract timer definitions to 'hw/timer/tmu012.h'.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/sh4/sh.h | 9 -
include/hw/timer/tmu012.h | 23 +++
hw/sh4/sh7750.c | 1 +
hw/timer/sh_timer.c | 2 ++
4 files changed, 26 insertions(+), 9 deletions(-)
create mode 100644 include/hw/timer/tmu012.h
diff --git a/include/hw/sh4/sh.h b/include/hw/sh4/sh.h
index fe773cb01d..93f464bf4c 100644
--- a/include/hw/sh4/sh.h
+++ b/include/hw/sh4/sh.h
@@ -27,15 +27,6 @@ typedef struct {
int sh7750_register_io_device(struct SH7750State *s,
sh7750_io_device * device);
-/* sh_timer.c */
-#define TMU012_FEAT_TOCR (1 << 0)
-#define TMU012_FEAT_3CHAN (1 << 1)
-#define TMU012_FEAT_EXTCLK (1 << 2)
-void tmu012_init(MemoryRegion *sysmem, hwaddr base,
- int feat, uint32_t freq,
-qemu_irq ch0_irq, qemu_irq ch1_irq,
-qemu_irq ch2_irq0, qemu_irq ch2_irq1);
-
/* sh_serial.c */
#define SH_SERIAL_FEAT_SCIF (1 << 0)
diff --git a/include/hw/timer/tmu012.h b/include/hw/timer/tmu012.h
new file mode 100644
index 00..808ed8de1d
--- /dev/null
+++ b/include/hw/timer/tmu012.h
@@ -0,0 +1,23 @@
+/*
+ * SuperH Timer
+ *
+ * Copyright (c) 2007 Magnus Damm
+ *
+ * This code is licensed under the GPL.
+ */
+
+#ifndef HW_TIMER_TMU012_H
+#define HW_TIMER_TMU012_H
+
+#include "exec/hwaddr.h"
+
+#define TMU012_FEAT_TOCR (1 << 0)
+#define TMU012_FEAT_3CHAN (1 << 1)
+#define TMU012_FEAT_EXTCLK (1 << 2)
+
+void tmu012_init(MemoryRegion *sysmem, hwaddr base,
+ int feat, uint32_t freq,
+ qemu_irq ch0_irq, qemu_irq ch1_irq,
+ qemu_irq ch2_irq0, qemu_irq ch2_irq1);
+
+#endif
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index d660714443..f8ac3ec6e3 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -30,6 +30,7 @@
#include "sh7750_regs.h"
#include "sh7750_regnames.h"
#include "hw/sh4/sh_intc.h"
+#include "hw/timer/tmu012.h"
#include "cpu.h"
#include "exec/exec-all.h"
diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c
index 13c4051808..b9cbacf5d0 100644
--- a/hw/timer/sh_timer.c
+++ b/hw/timer/sh_timer.c
@@ -9,10 +9,12 @@
*/
#include "qemu/osdep.h"
+#include "exec/memory.h"
#include "hw/hw.h"
#include "hw/irq.h"
#include "hw/sh4/sh.h"
#include "qemu/timer.h"
+#include "hw/timer/tmu012.h"
#include "hw/ptimer.h"
//#define DEBUG_TIMER
--
2.21.3
[PATCH 0/3] hw/sh4: Trivial cleanups
Some SH4 patches worth salvaging while doing housekeeping. Philippe Mathieu-Daudé (3): hw/sh4: Use MemoryRegion typedef hw/sh4: Extract timer definitions to 'hw/timer/tmu012.h' hw/timer/sh_timer: Remove unused 'qemu/timer.h' include include/hw/sh4/sh.h | 12 +--- include/hw/timer/tmu012.h | 23 +++ hw/sh4/sh7750.c | 1 + hw/timer/sh_timer.c | 3 ++- 4 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 include/hw/timer/tmu012.h -- 2.21.3
[PATCH 1/3] hw/sh4: Use MemoryRegion typedef
Use the MemoryRegion type defined in "qemu/typedefs.h",
to keep the repository style consistent.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/sh4/sh.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/hw/sh4/sh.h b/include/hw/sh4/sh.h
index 767a2df7e2..fe773cb01d 100644
--- a/include/hw/sh4/sh.h
+++ b/include/hw/sh4/sh.h
@@ -10,9 +10,8 @@
/* sh7750.c */
struct SH7750State;
-struct MemoryRegion;
-struct SH7750State *sh7750_init(SuperHCPU *cpu, struct MemoryRegion *sysmem);
+struct SH7750State *sh7750_init(SuperHCPU *cpu, MemoryRegion *sysmem);
typedef struct {
/* The callback will be triggered if any of the designated lines change */
@@ -32,7 +31,7 @@ int sh7750_register_io_device(struct SH7750State *s,
#define TMU012_FEAT_TOCR (1 << 0)
#define TMU012_FEAT_3CHAN (1 << 1)
#define TMU012_FEAT_EXTCLK (1 << 2)
-void tmu012_init(struct MemoryRegion *sysmem, hwaddr base,
+void tmu012_init(MemoryRegion *sysmem, hwaddr base,
int feat, uint32_t freq,
qemu_irq ch0_irq, qemu_irq ch1_irq,
qemu_irq ch2_irq0, qemu_irq ch2_irq1);
--
2.21.3
Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
On Mon, May 4, 2020 at 1:35 PM Bin Meng wrote:
>
> On Mon, May 4, 2020 at 4:00 PM Bin Meng wrote:
> >
> > Hi Anup,
> >
> > On Mon, May 4, 2020 at 3:52 PM Anup Patel wrote:
> > >
> > > On Mon, May 4, 2020 at 12:46 PM Bin Meng wrote:
> > > >
> > > > Hi Anup,
> > > >
> > > > On Sun, May 3, 2020 at 12:38 PM Anup Patel wrote:
> > > > >
> > > > > On Fri, May 1, 2020 at 9:26 PM Bin Meng wrote:
> > > > > >
> > > > > > From: Bin Meng
> > > > > >
> > > > > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > > > > platform where all platform specific functionality is provided based
> > > > > > on FDT passed by previous booting stage. The support was added in
> > > > > > upstream opensbi recently.
> > > > > >
> > > > > > Bring the QEMU roms/opensbi submodule to the upstream opensbi
> > > > > > commit:
> > > > > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush
> > > > > > range limit override")
> > > > > > with the following changes since v0.7 release:
> > > > > >
> > > > > > 1bb00ab lib: No need to provide default PMP region using platform
> > > > > > callbacks
> > > > > > a9eac67 include: sbi_platform: Combine reboot and shutdown into
> > > > > > one callback
> > > > > > 6585fab lib: utils: Add SiFive test device
> > > > > > 4781545 platform: Add Nuclei UX600 platform
> > > > > > 3a326af scripts: adapt binary archive script for Nuclei UX600
> > > > > > 5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > > > > e6c1345 lib: utils/serial: Skip baudrate config if input
> > > > > > frequency is zero
> > > > > > 01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > > > > 0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > > > > 243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > > > > e3ad7c1 lib: utils: Rename fdt_parse_clint() to
> > > > > > fdt_parse_compat_addr()
> > > > > > a39cd6f lib: utils: Add FDT match table based node lookup
> > > > > > dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > > > > 66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > > > > 19e966b lib: utils: Add fdt_parse_hart_id() function
> > > > > > 44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > > > > f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > > > > 1ac794c include: Add array_size() macro
> > > > > > 8ff2b94 lib: utils: Add simple FDT timer framework
> > > > > > 76f0f81 lib: utils: Add simple FDT ipi framework
> > > > > > 75322a6 lib: utils: Add simple FDT irqchip framework
> > > > > > 76a8940 lib: utils: Add simple FDT serial framework
> > > > > > 7cc6fa4 lib: utils: Add simple FDT reset framework
> > > > > > 4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > > > > f1aa9e5 platform: Add generic FDT based platform support
> > > > > > 1f21b99 lib: sbi: Print platform hart count at boot time
> > > > > > 2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > > > > 4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit
> > > > > > override
> > > > > >
> > > > > > Update our Makefile to build the generic platform instead of
> > > > > > building
> > > > > > virt and sifive_u separately.
> > > > > >
> > > > > > Signed-off-by: Bin Meng
> > > > > > ---
> > > > > >
> > > > > > roms/Makefile | 30 --
> > > > > > roms/opensbi | 2 +-
> > > > > > 2 files changed, 9 insertions(+), 23 deletions(-)
> > > > > >
> > > > > > diff --git a/roms/Makefile b/roms/Makefile
> > > > > > index f9acf39..cb00628 100644
> > > > > > --- a/roms/Makefile
> > > > > > +++ b/roms/Makefile
> > > > > > @@ -64,10 +64,8 @@ default help:
> > > > > > @echo " u-boot.e500-- update u-boot.e500"
> > > > > > @echo " u-boot.sam460 -- update u-boot.sam460"
> > > > > > @echo " efi-- update UEFI (edk2) platform
> > > > > > firmware"
> > > > > > - @echo " opensbi32-virt -- update OpenSBI for 32-bit
> > > > > > virt machine"
> > > > > > - @echo " opensbi64-virt -- update OpenSBI for 64-bit
> > > > > > virt machine"
> > > > > > - @echo " opensbi32-sifive_u -- update OpenSBI for 32-bit
> > > > > > sifive_u machine"
> > > > > > - @echo " opensbi64-sifive_u -- update OpenSBI for 64-bit
> > > > > > sifive_u machine"
> > > > > > + @echo " opensbi32-generic -- update OpenSBI for 32-bit
> > > > > > generic machine"
> > > > > > + @echo " opensbi64-generic -- update OpenSBI for 64-bit
> > > > > > generic machine"
> > > > > > @echo " bios-microvm -- update bios-microvm.bin
> > > > > > (qboot)"
> > > > > > @echo " clean -- delete the files generated
> > > > > > by the previous" \
> > > > > > "build targets"
> > > > > > @@ -170,29 +168,17 @@ skiboot:
> > > > > > efi: edk2-basetools
> > > > > > $(MAKE) -f Makefile
[PATCH 3/3] hw/timer/sh_timer: Remove unused 'qemu/timer.h' include
Remove unused "qemu/timer.h" include. Signed-off-by: Philippe Mathieu-Daudé --- hw/timer/sh_timer.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/timer/sh_timer.c b/hw/timer/sh_timer.c index b9cbacf5d0..bb0e1c8ee5 100644 --- a/hw/timer/sh_timer.c +++ b/hw/timer/sh_timer.c @@ -13,7 +13,6 @@ #include "hw/hw.h" #include "hw/irq.h" #include "hw/sh4/sh.h" -#include "qemu/timer.h" #include "hw/timer/tmu012.h" #include "hw/ptimer.h" -- 2.21.3
Re: [PATCH v2] aspeed: Add boot stub for smp booting
On Thu, 16 Apr 2020 at 16:48, Peter Maydell wrote:
>
> On Thu, 9 Apr 2020 at 07:31, Joel Stanley wrote:
> >
> > This is a boot stub that is similar to the code u-boot runs, allowing
> > the kernel to boot the secondary CPU.
>
> > +static void aspeed_write_smpboot(ARMCPU *cpu,
> > + const struct arm_boot_info *info)
> > +{
> > +static const uint32_t poll_mailbox_ready[] = {
> > +/*
> > + * r2 = per-cpu go sign value
> > + * r1 = AST_SMP_MBOX_FIELD_ENTRY
> > + * r0 = AST_SMP_MBOX_FIELD_GOSIGN
> > + */
> > +0xee100fb0, /* mrc p15, 0, r0, c0, c0, 5 */
> > +0xe21000ff, /* andsr0, r0, #255 */
> > +0xe59f201c, /* ldr r2, [pc, #28] */
> > +0xe1822000, /* orr r2, r2, r0*/
> > +
> > +0xe59f1018, /* ldr r1, [pc, #24] */
> > +0xe59f0018, /* ldr r0, [pc, #24] */
> > +
> > +0xe320f002, /* wfe */
> > +0xe5904000, /* ldr r4, [r0] */
> > +0xe1520004, /* cmp r2, r4*/
> > +0x1afb, /* bne */
>
> Note that unlike "wfi", QEMU's "wfe" implementation is merely
> a 'yield', so a secondary-CPU boot loop that has wfe in it
> will basically be a busy-loop of those vcpu threads.
> (This is why the smpboot code in hw/arm/boot.c uses wfi.)
>
> I don't suppose the secondary boot protocol on these boards
> is such that a wfi loop will work ? (Depends on what the
> primary code in the kernel does to prod the secondary after
> writing the magic value.)
Thanks for the review.
I chose wfe as it's what aspeed's u-boot has. I don't have a strong
understand of this area of ARM processors, nor do I have any insight
into why that choice was made.
I did some testing and wfi in the qemu stub works fine with Linux so I
will respin with that instead.
Cheers,
Joel
[PATCH v3] aspeed: Add boot stub for smp booting
This is a boot stub that is similar to the code u-boot runs, allowing
the kernel to boot the secondary CPU.
u-boot works as follows:
1. Initialises the SMP mailbox area in the SCU at 0x1e6e2180 with default
values
2. Copies a stub named 'mailbox_insn' from flash to the SCU, just above the
mailbox area
3. Sets AST_SMP_MBOX_FIELD_READY to a magic value to indicate the
secondary can begin execution from the stub
4. The stub waits until the AST_SMP_MBOX_FIELD_GOSIGN register is set to
a magic value
5. Jumps to the address in AST_SMP_MBOX_FIELD_ENTRY, starting Linux
Linux indicates it is ready by writing the address of its entrypoint
function to AST_SMP_MBOX_FIELD_ENTRY and the 'go' magic number to
AST_SMP_MBOX_FIELD_GOSIGN. The secondary CPU sees this at step 4 and
breaks out of it's loop.
To be compatible, a fixed qemu stub is loaded into the mailbox area. As
qemu can ensure the stub is loaded before execution starts, we do not
need to emulate the AST_SMP_MBOX_FIELD_READY behaviour of u-boot. The
secondary CPU's program counter points to the beginning of the stub,
allowing qemu to start secondaries at step four.
Reboot behaviour is preserved by resetting AST_SMP_MBOX_FIELD_GOSIGN
when the secondaries are reset.
This is only configured when the system is booted with -kernel and qemu
does not execute u-boot first.
Reviewed-by: Cédric Le Goater
Tested-by: Cédric Le Goater
Signed-off-by: Joel Stanley
---
v3: Use WFI instead of WFE
v2: test for number of CPUs
---
hw/arm/aspeed.c | 65 +
1 file changed, 65 insertions(+)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 88bcd6ff3fbd..93970502b8a6 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -116,6 +116,58 @@ static const MemoryRegionOps max_ram_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
+#define AST_SMP_MAILBOX_BASE0x1e6e2180
+#define AST_SMP_MBOX_FIELD_ENTRY(AST_SMP_MAILBOX_BASE + 0x0)
+#define AST_SMP_MBOX_FIELD_GOSIGN (AST_SMP_MAILBOX_BASE + 0x4)
+#define AST_SMP_MBOX_FIELD_READY(AST_SMP_MAILBOX_BASE + 0x8)
+#define AST_SMP_MBOX_FIELD_POLLINSN (AST_SMP_MAILBOX_BASE + 0xc)
+#define AST_SMP_MBOX_CODE (AST_SMP_MAILBOX_BASE + 0x10)
+#define AST_SMP_MBOX_GOSIGN 0xabbaab00
+
+static void aspeed_write_smpboot(ARMCPU *cpu,
+ const struct arm_boot_info *info)
+{
+static const uint32_t poll_mailbox_ready[] = {
+/*
+ * r2 = per-cpu go sign value
+ * r1 = AST_SMP_MBOX_FIELD_ENTRY
+ * r0 = AST_SMP_MBOX_FIELD_GOSIGN
+ */
+0xee100fb0, /* mrc p15, 0, r0, c0, c0, 5 */
+0xe21000ff, /* andsr0, r0, #255 */
+0xe59f201c, /* ldr r2, [pc, #28] */
+0xe1822000, /* orr r2, r2, r0*/
+
+0xe59f1018, /* ldr r1, [pc, #24] */
+0xe59f0018, /* ldr r0, [pc, #24] */
+
+0xe320f003, /* wfi */
+0xe5904000, /* ldr r4, [r0] */
+0xe1520004, /* cmp r2, r4*/
+0x1afb, /* bne */
+0xe591f000, /* ldr pc, [r1] */
+AST_SMP_MBOX_GOSIGN,
+AST_SMP_MBOX_FIELD_ENTRY,
+AST_SMP_MBOX_FIELD_GOSIGN,
+};
+
+rom_add_blob_fixed("aspeed.smpboot", poll_mailbox_ready,
+ sizeof(poll_mailbox_ready),
+ info->smp_loader_start);
+}
+
+static void aspeed_reset_secondary(ARMCPU *cpu,
+ const struct arm_boot_info *info)
+{
+AddressSpace *as = arm_boot_address_space(cpu, info);
+CPUState *cs = CPU(cpu);
+
+/* info->smp_bootreg_addr */
+address_space_stl_notdirty(as, AST_SMP_MBOX_FIELD_GOSIGN, 0,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+cpu_set_pc(cs, info->smp_loader_start);
+}
+
#define FIRMWARE_ADDR 0x0
static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
@@ -270,6 +322,19 @@ static void aspeed_machine_init(MachineState *machine)
}
}
+if (machine->kernel_filename && bmc->soc.num_cpus > 1) {
+/* With no u-boot we must set up a boot stub for the secondary CPU */
+MemoryRegion *smpboot = g_new(MemoryRegion, 1);
+memory_region_init_ram(smpboot, OBJECT(bmc), "aspeed.smpboot",
+ 0x80, &error_abort);
+memory_region_add_subregion(get_system_memory(),
+AST_SMP_MAILBOX_BASE, smpboot);
+
+aspeed_board_binfo.write_secondary_boot = aspeed_write_smpboot;
+aspeed_board_binfo.secondary_cpu_reset_hook = aspeed_reset_secondary;
+aspeed_board_binfo.smp_loader_start = AST_SMP_MBOX_CODE;
+}
+
aspeed_board_binfo.ram_size = ram_size;
aspeed_board_binfo.loader_start = sc->memmap[ASPEED_SDRAM];
aspeed_board_binfo.nb_c
[PATCH 2/2] hw/display/edid: Add missing 'qdev-properties.h' header
To use the DEFINE_EDID_PROPERTIES() macro we need the
definitions from "hw/qdev-properties.h".
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/display/edid.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/hw/display/edid.h b/include/hw/display/edid.h
index ff99dc0a05..23371ee82c 100644
--- a/include/hw/display/edid.h
+++ b/include/hw/display/edid.h
@@ -2,6 +2,7 @@
#define EDID_H
#include "qom/object.h"
+#include "hw/qdev-properties.h"
typedef struct qemu_edid_info {
const char *vendor; /* http://www.uefi.org/pnp_id_list */
--
2.21.3
[PATCH] hw/usb: Make "hcd-ehci.h" header public
As target-specific code use this header, move it to the publicly
accessible include/ folder.
$ git grep hw/usb/hcd-ehci.h
hw/arm/allwinner-h3.c:31:#include "hw/usb/hcd-ehci.h"
hw/arm/exynos4210.c:38:#include "hw/usb/hcd-ehci.h"
hw/ppc/sam460ex.c:38:#include "hw/usb/hcd-ehci.h"
include/hw/arm/allwinner-a10.h:13:#include "hw/usb/hcd-ehci.h"
include/hw/arm/aspeed_soc.h:29:#include "hw/usb/hcd-ehci.h"
Signed-off-by: Philippe Mathieu-Daudé
---
{hw => include/hw}/usb/hcd-ehci.h | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename {hw => include/hw}/usb/hcd-ehci.h (100%)
diff --git a/hw/usb/hcd-ehci.h b/include/hw/usb/hcd-ehci.h
similarity index 100%
rename from hw/usb/hcd-ehci.h
rename to include/hw/usb/hcd-ehci.h
--
2.21.3
[PATCH 1/2] hw/display: Include local 'framebuffer.h'
The "framebuffer.h" header is not an exported include. Signed-off-by: Philippe Mathieu-Daudé --- hw/display/artist.c | 2 +- hw/display/next-fb.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/artist.c b/hw/display/artist.c index 753dbb9a77..e1d5885fed 100644 --- a/hw/display/artist.c +++ b/hw/display/artist.c @@ -21,7 +21,7 @@ #include "migration/vmstate.h" #include "ui/console.h" #include "trace.h" -#include "hw/display/framebuffer.h" +#include "framebuffer.h" #define TYPE_ARTIST "artist" #define ARTIST(obj) OBJECT_CHECK(ARTISTState, (obj), TYPE_ARTIST) diff --git a/hw/display/next-fb.c b/hw/display/next-fb.c index 2b726a10f8..b0513a8fba 100644 --- a/hw/display/next-fb.c +++ b/hw/display/next-fb.c @@ -27,7 +27,7 @@ #include "hw/hw.h" #include "hw/boards.h" #include "hw/loader.h" -#include "hw/display/framebuffer.h" +#include "framebuffer.h" #include "ui/pixel_ops.h" #include "hw/m68k/next-cube.h" -- 2.21.3
[PATCH 0/2] hw/display: Trivial include cleanups
Some 'display' patches worth salvaging while doing housekeeping. Philippe Mathieu-Daudé (2): hw/display: Include local 'framebuffer.h' hw/display/edid: Add missing 'qdev-properties.h' header include/hw/display/edid.h | 1 + hw/display/artist.c | 2 +- hw/display/next-fb.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) -- 2.21.3
Re: [PATCH v4 00/34] Configurable policy for handling deprecated interfaces
On Tue, Mar 17, 2020 at 12:54:25 +0100, Markus Armbruster wrote: > This series extends QMP introspection to cover deprecation. > Additionally, new option -compat lets you configure what to do when > deprecated interfaces get used. This is intended for testing users of > the management interfaces. It is experimental. > > -compat deprecated-input= configures what to do when > deprecated input is received. Available policies: > > * accept: Accept deprecated commands and arguments (default) > * reject: Reject them > * crash: Crash I've noticed that the 'crash' option doesn't manage to log the reason to stderr. Relevant section of libvirt's log file which agregates the stderr/out 2020-04-30 13:24:22.006+: 2072883: debug : virCommandHandshakeChild:418 : Handshake with parent is done char device redirected to /dev/pts/0 (label charserial0) 2020-04-30 13:24:31.879+: Domain id=4 is tainted: custom-monitor 2020-04-30 13:24:32.330+: shutting down, reason=crashed 'handshake' line is last of libvirt's messages pre-start of the qemu process. 'char device redirected' is reported by qemu. 'domain is tainted' is added by libvirt when I've issued the deprecated API via virsh qemu-monitor-command. 'reason=crashed' is added by libvirts VM shutdown hanlder. > > -compat deprecated-output= configures what to do when > deprecated output is sent. Available output policies: > > * accept: Emit deprecated command results and events (default) > * hide: Suppress them > > For now, -compat covers only deprecated syntactic aspects of QMP. We > may want to extend it to cover semantic aspects, CLI, and experimental > features. > > PATCH 01-04: Documentation fixes > PATCH 05-10: Test improvements > PATCH 11-24: Add feature flags to remaining user-defined types and to >struct members > PATCH 25-26: New special feature 'deprecated', visible in >introspection These are cool. I've added support for verifying that any command excercised by the libvirt unit test suite is not deprecated, or we at least know that it is and have a replacement. https://www.redhat.com/archives/libvir-list/2020-April/msg01444.html > PATCH 27-34: New -compat to set policy for handling stuff marked with >feature 'deprecated' While implementing support for this feature I noticed that it's impossible for libvirt to detect that it's available. The idea is to make a developer-centred setting in our config which will enable the compat setting if available and ignore it if not available to prevent us having to fiddle with the settings when testing various qemu versions. Thanks! Peter
[PATCH 4/4] hw/i386: Make vmmouse helpers static
The vmmouse helpers are only used in hw/i386/vmmouse.c,
make them static.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/i386/pc.h | 4
hw/i386/vmmouse.c| 22 +-
hw/i386/vmport.c | 23 +--
3 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index de49a57506..05e19455bb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -129,10 +129,6 @@ typedef struct PCMachineClass {
GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
-/* vmport.c */
-void vmmouse_get_data(uint32_t *data);
-void vmmouse_set_data(const uint32_t *data);
-
/* pc.c */
extern int fd_bootchk;
diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
index 78b36f6f5d..b3aef41327 100644
--- a/hw/i386/vmmouse.c
+++ b/hw/i386/vmmouse.c
@@ -25,11 +25,11 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "ui/console.h"
-#include "hw/i386/pc.h"
#include "hw/input/i8042.h"
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "vmport.h"
+#include "cpu.h"
/* debug only vmmouse */
//#define DEBUG_VMMOUSE
@@ -71,6 +71,26 @@ typedef struct VMMouseState
ISAKBDState *i8042;
} VMMouseState;
+static void vmmouse_get_data(uint32_t *data)
+{
+X86CPU *cpu = X86_CPU(current_cpu);
+CPUX86State *env = &cpu->env;
+
+data[0] = env->regs[R_EAX]; data[1] = env->regs[R_EBX];
+data[2] = env->regs[R_ECX]; data[3] = env->regs[R_EDX];
+data[4] = env->regs[R_ESI]; data[5] = env->regs[R_EDI];
+}
+
+static void vmmouse_set_data(const uint32_t *data)
+{
+X86CPU *cpu = X86_CPU(current_cpu);
+CPUX86State *env = &cpu->env;
+
+env->regs[R_EAX] = data[0]; env->regs[R_EBX] = data[1];
+env->regs[R_ECX] = data[2]; env->regs[R_EDX] = data[3];
+env->regs[R_ESI] = data[4]; env->regs[R_EDI] = data[5];
+}
+
static uint32_t vmmouse_get_status(VMMouseState *s)
{
DPRINTF("vmmouse_get_status()\n");
diff --git a/hw/i386/vmport.c b/hw/i386/vmport.c
index 00d47e0c4c..1b691a 100644
--- a/hw/i386/vmport.c
+++ b/hw/i386/vmport.c
@@ -23,10 +23,10 @@
*/
#include "qemu/osdep.h"
#include "hw/isa/isa.h"
-#include "hw/i386/pc.h"
#include "sysemu/hw_accel.h"
#include "qemu/log.h"
#include "vmport.h"
+#include "cpu.h"
#include "trace.h"
#define VMPORT_CMD_GETVERSION 0x0a
@@ -109,27 +109,6 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t
addr)
return ram_size;
}
-/* vmmouse helpers */
-void vmmouse_get_data(uint32_t *data)
-{
-X86CPU *cpu = X86_CPU(current_cpu);
-CPUX86State *env = &cpu->env;
-
-data[0] = env->regs[R_EAX]; data[1] = env->regs[R_EBX];
-data[2] = env->regs[R_ECX]; data[3] = env->regs[R_EDX];
-data[4] = env->regs[R_ESI]; data[5] = env->regs[R_EDI];
-}
-
-void vmmouse_set_data(const uint32_t *data)
-{
-X86CPU *cpu = X86_CPU(current_cpu);
-CPUX86State *env = &cpu->env;
-
-env->regs[R_EAX] = data[0]; env->regs[R_EBX] = data[1];
-env->regs[R_ECX] = data[2]; env->regs[R_EDX] = data[3];
-env->regs[R_ESI] = data[4]; env->regs[R_EDI] = data[5];
-}
-
static const MemoryRegionOps vmport_ops = {
.read = vmport_ioport_read,
.write = vmport_ioport_write,
--
2.21.3
[PATCH 2/4] hw/i386/vmport: Remove unused 'hw/input/i8042.h' include
Remove unused "hw/input/i8042.h" include. Signed-off-by: Philippe Mathieu-Daudé --- hw/i386/vmport.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/i386/vmport.c b/hw/i386/vmport.c index 1f31e27c8a..114141c6f3 100644 --- a/hw/i386/vmport.c +++ b/hw/i386/vmport.c @@ -24,7 +24,6 @@ #include "qemu/osdep.h" #include "hw/isa/isa.h" #include "hw/i386/pc.h" -#include "hw/input/i8042.h" #include "sysemu/hw_accel.h" #include "qemu/log.h" #include "trace.h" -- 2.21.3
[PATCH 0/4] hw/i386: Restrict vmport/vmmouse devices to x86 targets
Some x86 patches worth salvaging while doing housekeeping: Restrict vmport/vmmouse devices to x86 targets. A step forward having "hw/i386/pc.h" target-specific... Philippe Mathieu-Daudé (4): hw/i386/pc: Create 'vmport' device in place hw/i386/vmport: Remove unused 'hw/input/i8042.h' include hw/i386: Add 'vmport.h' local header hw/i386: Make vmmouse helpers static hw/i386/vmport.h | 34 ++ include/hw/i386/pc.h | 13 - hw/i386/pc.c | 3 ++- hw/i386/vmmouse.c| 23 ++- hw/i386/vmport.c | 25 ++--- 5 files changed, 60 insertions(+), 38 deletions(-) create mode 100644 hw/i386/vmport.h -- 2.21.3
Re: [PATCH 08/17] s390x/cpumodel: Fix UI to CPU features pcc-cmac-{aes,eaes}-256
On 02.05.20 07:15, Markus Armbruster wrote: > Christian Borntraeger writes: > >> On 29.04.20 10:54, Christian Borntraeger wrote: >>> >>> >>> On 28.04.20 19:13, David Hildenbrand wrote: On 28.04.20 18:34, Markus Armbruster wrote: > Both s390_features[S390_FEAT_PCC_CMAC_AES_256].name and > s390_features[S390_FEAT_PCC_CMAC_EAES_256].name is > "pcc-cmac-eaes-256". The former is obviously a pasto. > > Impact: > > * s390_feat_bitmap_to_ascii() misidentifies S390_FEAT_PCC_CMAC_AES_256 > as "pcc-cmac-eaes-256". Affects QMP commands query-cpu-definitions, > query-cpu-model-expansion, query-cpu-model-baseline, > query-cpu-model-comparison, and the error message when > s390_realize_cpu_model() fails in check_compatibility(). > > * s390_realize_cpu_model() misidentifies it in check_consistency() > warnings. > > * s390_cpu_list() likewise. Affects -cpu help. > > * s390_cpu_model_register_props() creates CPU property > "pcc-cmac-eaes-256" twice. The second one fails, but the error is > ignored (a later commit will change that). Results in a single > property "pcc-cmac-eaes-256" with the description for > S390_FEAT_PCC_CMAC_AES_256, and no property for > S390_FEAT_PCC_CMAC_EAES_256. CPU properties are visible in CLI -cpu > and -device, QMP & HMP device_add, QMP device-list-properties, and > QOM introspection. > > Fix by deleting the wayward 'e'. Very nice catch - thanks! While this sounds very bad, it's luckily not that bad in practice (currently). The feature (or rather, both features) is part of the feature group "msa4". As long as we have all sub-features part of that group (which is usually the case), we will always indicate "msa4" to the user, instead of all the separate sub-features. So, expansion, baseline, comparison will usually only work with "msa4". (in addition, current KVM is not capable of actually masking off these sub-features, so it will still, always see the feature, even if not explicitly specified via "-cpu X,pcc-cmac-aes-256=on) I think we should do stable backports. >>> >>> makes sense, but I would like to do some testing upfront (old QEMU <-> new >>> QEMU >> >> So migration does work between a qemu with and without the patch for >> host-model and >> custom model=z14. > > Is this a Tested-by? Yes. As David pointed out when a user really starts to pick manual things in MSA4 then we can have a non-migrateable guests. But this is still the right thing I guess.
[PATCH 1/4] hw/i386/pc: Create 'vmport' device in place
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/i386/pc.h | 6 --
hw/i386/pc.c | 2 +-
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6ab6eda046..26e2a3d92b 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -132,12 +132,6 @@ GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
/* vmport.c */
#define TYPE_VMPORT "vmport"
typedef uint32_t (VMPortReadFunc)(void *opaque, uint32_t address);
-
-static inline void vmport_init(ISABus *bus)
-{
-isa_create_simple(bus, TYPE_VMPORT);
-}
-
void vmport_register(unsigned char command, VMPortReadFunc *func, void
*opaque);
void vmmouse_get_data(uint32_t *data);
void vmmouse_set_data(const uint32_t *data);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5143c51653..84669ddc84 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1152,7 +1152,7 @@ static void pc_superio_init(ISABus *isa_bus, bool
create_fdctrl, bool no_vmport)
i8042 = isa_create_simple(isa_bus, "i8042");
if (!no_vmport) {
-vmport_init(isa_bus);
+isa_create_simple(isa_bus, TYPE_VMPORT);
vmmouse = isa_try_create(isa_bus, "vmmouse");
} else {
vmmouse = NULL;
--
2.21.3
[PATCH 3/4] hw/i386: Add 'vmport.h' local header
Move 'vmport' related declarations in a target-specific header.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/i386/vmport.h | 34 ++
include/hw/i386/pc.h | 3 ---
hw/i386/pc.c | 1 +
hw/i386/vmmouse.c| 1 +
hw/i386/vmport.c | 1 +
5 files changed, 37 insertions(+), 3 deletions(-)
create mode 100644 hw/i386/vmport.h
diff --git a/hw/i386/vmport.h b/hw/i386/vmport.h
new file mode 100644
index 00..47eda7a22b
--- /dev/null
+++ b/hw/i386/vmport.h
@@ -0,0 +1,34 @@
+/*
+ * QEMU VMPort emulation
+ *
+ * Copyright (C) 2007 Hervé Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef HW_I386_VMPORT_H
+#define HW_I386_VMPORT_H
+
+#define TYPE_VMPORT "vmport"
+
+typedef uint32_t (VMPortReadFunc)(void *opaque, uint32_t address);
+
+void vmport_register(unsigned char command, VMPortReadFunc *func, void
*opaque);
+
+#endif
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 26e2a3d92b..de49a57506 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -130,9 +130,6 @@ typedef struct PCMachineClass {
GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
/* vmport.c */
-#define TYPE_VMPORT "vmport"
-typedef uint32_t (VMPortReadFunc)(void *opaque, uint32_t address);
-void vmport_register(unsigned char command, VMPortReadFunc *func, void
*opaque);
void vmmouse_get_data(uint32_t *data);
void vmmouse_set_data(const uint32_t *data);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 84669ddc84..f6b8431c8b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -91,6 +91,7 @@
#include "qapi/qmp/qerror.h"
#include "config-devices.h"
#include "e820_memory_layout.h"
+#include "vmport.h"
#include "fw_cfg.h"
#include "trace.h"
diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
index e8e62bd96b..78b36f6f5d 100644
--- a/hw/i386/vmmouse.c
+++ b/hw/i386/vmmouse.c
@@ -29,6 +29,7 @@
#include "hw/input/i8042.h"
#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
+#include "vmport.h"
/* debug only vmmouse */
//#define DEBUG_VMMOUSE
diff --git a/hw/i386/vmport.c b/hw/i386/vmport.c
index 114141c6f3..00d47e0c4c 100644
--- a/hw/i386/vmport.c
+++ b/hw/i386/vmport.c
@@ -26,6 +26,7 @@
#include "hw/i386/pc.h"
#include "sysemu/hw_accel.h"
#include "qemu/log.h"
+#include "vmport.h"
#include "trace.h"
#define VMPORT_CMD_GETVERSION 0x0a
--
2.21.3
[PATCH 0/3] qom: Few trivial patches
Some QOM patches worth salvaging while doing housekeeping. Philippe Mathieu-Daudé (3): qom/object: Move Object typedef to 'qemu/typedefs.h' io/task: Move 'qom/object.h' header to source qom/object: Make reparenting error more verbose include/io/task.h | 2 -- include/qemu/typedefs.h | 1 + include/qom/object.h | 2 -- include/qom/qom-qobject.h | 2 -- include/sysemu/sysemu.h | 1 - io/task.c | 1 + qom/object.c | 7 ++- 7 files changed, 8 insertions(+), 8 deletions(-) -- 2.21.3
[PATCH 3/3] qom/object: Make reparenting error more verbose
Display child and parent names when reparenting occurs.
Signed-off-by: Philippe Mathieu-Daudé
---
qom/object.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index be700e831f..417fd90aa5 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1683,7 +1683,12 @@ void object_property_add_child(Object *obj, const char
*name,
ObjectProperty *op;
if (child->parent != NULL) {
-error_setg(errp, "child object is already parented");
+error_setg(errp, "child object '%s' is already parented (parent: '%s')
"
+ "can not be children '%s' of '%s'",
+ object_get_typename(child),
+ object_get_typename(child->parent),
+ name,
+ object_get_typename(obj));
return;
}
--
2.21.3
[PATCH 1/3] qom/object: Move Object typedef to 'qemu/typedefs.h'
We use the Object type all over the place. Forward declare it in "qemu/typedefs.h". Signed-off-by: Philippe Mathieu-Daudé --- include/qemu/typedefs.h | 1 + include/qom/object.h | 2 -- include/qom/qom-qobject.h | 2 -- include/sysemu/sysemu.h | 1 - 4 files changed, 1 insertion(+), 5 deletions(-) diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index 375770a80f..b03ec9f40a 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -75,6 +75,7 @@ typedef struct NetFilterState NetFilterState; typedef struct NICInfo NICInfo; typedef struct NodeInfo NodeInfo; typedef struct NumaNodeMem NumaNodeMem; +typedef struct Object Object; typedef struct ObjectClass ObjectClass; typedef struct PCIBridge PCIBridge; typedef struct PCIBus PCIBus; diff --git a/include/qom/object.h b/include/qom/object.h index 784c97c0e1..1edc12e64c 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -20,8 +20,6 @@ struct TypeImpl; typedef struct TypeImpl *Type; -typedef struct Object Object; - typedef struct TypeInfo TypeInfo; typedef struct InterfaceClass InterfaceClass; diff --git a/include/qom/qom-qobject.h b/include/qom/qom-qobject.h index 77cd717e3f..82136e6e80 100644 --- a/include/qom/qom-qobject.h +++ b/include/qom/qom-qobject.h @@ -13,8 +13,6 @@ #ifndef QEMU_QOM_QOBJECT_H #define QEMU_QOM_QOBJECT_H -#include "qom/object.h" - /* * object_property_get_qobject: * @obj: the object diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index ef81302e1a..ca4458e451 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -5,7 +5,6 @@ #include "qemu/timer.h" #include "qemu/notify.h" #include "qemu/uuid.h" -#include "qom/object.h" /* vl.c */ -- 2.21.3
Re: [PATCH] hw/usb: Make "hcd-ehci.h" header public
On Mon, 4 May 2020, Philippe Mathieu-Daudé wrote: As target-specific code use this header, move it to the publicly accessible include/ folder. $ git grep hw/usb/hcd-ehci.h hw/arm/allwinner-h3.c:31:#include "hw/usb/hcd-ehci.h" hw/arm/exynos4210.c:38:#include "hw/usb/hcd-ehci.h" hw/ppc/sam460ex.c:38:#include "hw/usb/hcd-ehci.h" include/hw/arm/allwinner-a10.h:13:#include "hw/usb/hcd-ehci.h" include/hw/arm/aspeed_soc.h:29:#include "hw/usb/hcd-ehci.h" All of these only need either the type #define or EHCISysBusState so splitting only those off to a public header should be enough and better than making public all of ehci's internal header. Regards, BALATON Zoltan
[PATCH 2/3] io/task: Move 'qom/object.h' header to source
We need "qom/object.h" to call object_ref()/object_unref().
Signed-off-by: Philippe Mathieu-Daudé
---
include/io/task.h | 2 --
io/task.c | 1 +
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/io/task.h b/include/io/task.h
index 1abbfb8b65..6818dfedd0 100644
--- a/include/io/task.h
+++ b/include/io/task.h
@@ -21,8 +21,6 @@
#ifndef QIO_TASK_H
#define QIO_TASK_H
-#include "qom/object.h"
-
typedef struct QIOTask QIOTask;
typedef void (*QIOTaskFunc)(QIOTask *task,
diff --git a/io/task.c b/io/task.c
index 1ae7b86488..53c0bed686 100644
--- a/io/task.c
+++ b/io/task.c
@@ -22,6 +22,7 @@
#include "io/task.h"
#include "qapi/error.h"
#include "qemu/thread.h"
+#include "qom/object.h"
#include "trace.h"
struct QIOTaskThreadData {
--
2.21.3
Re: [PATCH v2 1/3] MAINTAINERS: Cover the GDB Python scripts in the gdbstub section
On 4/30/20 9:59 AM, Kevin Wolf wrote: Am 30.04.2020 um 08:57 hat Philippe Mathieu-Daudé geschrieben: Keep an eye on these "same same, but different" files. Acked-by: Alex Bennée Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8cbc1fac2b..7a7f2b9c31 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2083,6 +2083,7 @@ R: Philippe Mathieu-Daudé S: Maintained F: gdbstub* F: gdb-xml/ +F: scripts/qemugdb/ Wouldn't it make sense to add scripts/qemu-gdb.py as well? I agree scripts/qemu-gdb.py with scripts/qemugdb/*py, but now looking at the files closely I don't think they belong to the gdbstub section (GDB protocol, with no knowledge of Python), and rather deserve a new section (GDB Python scripts, with no knowledge of GDB protocol). I am discarding this patch. Regards, Phil. Kevin
Re: [PATCH] Makefile: Let the 'help' target list the helper targets
Ping? On 4/23/20 12:43 PM, Philippe Mathieu-Daudé wrote: List the name of the helper targets when calling 'make help', along with the tool targets: $ make help [...] Helper targets: fsdev/virtfs-proxy-helper - Build virtfs-proxy-helper scsi/qemu-pr-helper- Build qemu-pr-helper qemu-bridge-helper - Build qemu-bridge-helper vhost-user-gpu - Build vhost-user-gpu virtiofsd - Build virtiofsd Tools targets: qemu-ga- Build qemu-ga tool qemu-keymap- Build qemu-keymap tool elf2dmp- Build elf2dmp tool ivshmem-client - Build ivshmem-client tool ivshmem-server - Build ivshmem-server tool qemu-nbd - Build qemu-nbd tool qemu-storage-daemon- Build qemu-storage-daemon tool qemu-img - Build qemu-img tool qemu-io- Build qemu-io tool qemu-edid - Build qemu-edid tool Signed-off-by: Philippe Mathieu-Daudé --- configure | 5 +++-- Makefile | 9 +++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/configure b/configure index 23b5e93752..caf880c38e 100755 --- a/configure +++ b/configure @@ -6374,7 +6374,7 @@ if test "$softmmu" = yes ; then if test "$linux" = yes; then if test "$virtfs" != no && test "$cap_ng" = yes && test "$attr" = yes ; then virtfs=yes - tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)" + helpers="$helpers fsdev/virtfs-proxy-helper\$(EXESUF)" else if test "$virtfs" = yes; then error_exit "VirtFS requires libcap-ng devel and libattr devel" @@ -6389,7 +6389,7 @@ if test "$softmmu" = yes ; then fi mpath=no fi -tools="$tools scsi/qemu-pr-helper\$(EXESUF)" +helpers="$helpers scsi/qemu-pr-helper\$(EXESUF)" else if test "$virtfs" = yes; then error_exit "VirtFS is supported only on Linux" @@ -7630,6 +7630,7 @@ else QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES" fi +echo "HELPERS=$helpers" >> $config_host_mak echo "TOOLS=$tools" >> $config_host_mak echo "ROMS=$roms" >> $config_host_mak echo "MAKE=$make" >> $config_host_mak diff --git a/Makefile b/Makefile index 8a9113e666..021a0cd491 100644 --- a/Makefile +++ b/Makefile @@ -336,9 +336,9 @@ $(call set-vpath, $(SRC_PATH)) LIBS+=-lz $(LIBS_TOOLS) vhost-user-json-y = -HELPERS-y = +HELPERS-y = $(HELPERS) -HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) = qemu-bridge-helper$(EXESUF) +HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) += qemu-bridge-helper$(EXESUF) ifeq ($(CONFIG_LINUX)$(CONFIG_VIRGL)$(CONFIG_GBM)$(CONFIG_TOOLS),) HELPERS-y += vhost-user-gpu$(EXESUF) @@ -1255,6 +1255,11 @@ endif $(foreach t, $(TARGET_DIRS), \ $(call print-help-run,$(t)/all,Build for $(t));) \ echo '') + @$(if $(HELPERS-y), \ + echo 'Helper targets:'; \ + $(foreach t, $(HELPERS-y), \ + $(call print-help-run,$(t),Build $(shell basename $(t)));) \ + echo '') @$(if $(TOOLS), \ echo 'Tools targets:'; \ $(foreach t, $(TOOLS), \
Re: [PATCH v3 0/9] misc: Trivial static code analyzer fixes
On 4/22/20 3:31 PM, Philippe Mathieu-Daudé wrote: Fix trivial warnings reported by the Clang static code analyzer. Only patch #2 'blockdev: Remove dead assignment' misses review. Thanks to Max this series is now fully reviewed, so... ping? The official Clang static code analyzer documentation is on: https://clang-analyzer.llvm.org/ On Fedora I simply used it as: $ sudo dnf install clang-analyzer $ ../configure $ scan-build make Since v2: - Based on lvivier/trivial-patches-for-5.1 - Removed dup patches from Kuhn Chenqun Since v1: - Addressed Markus/Zoltan/Aleksandar review comments Philippe Mathieu-Daudé (9): block: Avoid dead assignment blockdev: Remove dead assignment hw/i2c/pm_smbus: Remove dead assignment hw/input/adb-kbd: Remove dead assignment hw/ide/sii3112: Remove dead assignment hw/isa/i82378: Remove dead assignment hw/gpio/aspeed_gpio: Remove dead assignment hw/timer/stm32f2xx_timer: Remove dead assignment hw/timer/pxa2xx_timer: Add assertion to silent static analyzer warning block.c| 2 +- blockdev.c | 2 +- hw/gpio/aspeed_gpio.c | 2 +- hw/i2c/pm_smbus.c | 1 - hw/ide/sii3112.c | 5 +++-- hw/input/adb-kbd.c | 6 +- hw/isa/i82378.c| 8 hw/timer/pxa2xx_timer.c| 1 + hw/timer/stm32f2xx_timer.c | 1 - 9 files changed, 12 insertions(+), 16 deletions(-)
Re: [PATCH] MAINTAINERS: Update Keith Busch's email address
+block +trivial to raise the odds to get this patch merged. On 4/22/20 6:45 PM, Keith Busch wrote: On Tue, Apr 21, 2020 at 02:22:36PM +0200, Philippe Mathieu-Daudé wrote: keith.bu...@intel.com address is being rejected. Replace by the email address Keith is actively using. Signed-off-by: Philippe Mathieu-Daudé Thank you for sending the correction. Acked-by: Keith Busch
[Bug 1876678] [NEW] Ubuntu 20.04 QEMU Failure with nested FreeBSD bhyve
Public bug reported: BUG: Starting FreeBSD Layer 2 bhyve Guest within Layer 1 FreeBSD VM Host on Layer 0 Ubuntu 20.04 KVM / QEMU Host result in Layer 1 Guest / Host Pausing with "Emulation Failure" TESTING: My test scenario is nested virtualisation: Layer 0 - Ubuntu 20.04 Host Layer 1 - FreeBSD 12.1 with OVMF + bhyve hypervisor Guest/Host Layer 2 - FreeBSD 12.1 guest Layer 0 Host is: Ubuntu 20.04 LTS KVM / QEMU / libvirt <> $ virsh -c qemu:///system version --daemon Compiled against library: libvirt 6.0.0 Using library: libvirt 6.0.0 Using API: QEMU 6.0.0 Running hypervisor: QEMU 4.2.0 Running against daemon: 6.0.0 < <> $ cat /proc/cpuinfo | grep -c vmx 64 $ cat /sys/module/kvm_intel/parameters/nested Y <> Layer 1 Guest / Host is: FreeBSD Q35 v4.2 with OVMF: Pass Host VMX support to Layer 1 Guest via hvm /usr/share/OVMF/OVMF_CODE.fd /home/USER/swarm.bhyve.freebsd/OVMF_VARS.fd ... ... > Checked that Layer 1 - FreeBSD Quest / Host has VMX feature available: <> # uname -a FreeBSD swarm.DOMAIN.HERE 12.1-RELEASE FreeBSD 12.1-RELEASE GENERIC amd64 # grep Features /var/run/dmesg.boot Features=0xf83fbff Features2=0xfffa3223 AMD Features=0x2c100800 AMD Features2=0x121 Structured Extended Features=0x1c0fbb Structured Extended Features2=0x4 Structured Extended Features3=0xac000400 XSAVE Features=0x1 < On Layer 1 FreeBSD Guest / Host start up the Layer 2 guest.. <> # ls FreeBSD-11.2-RELEASE-amd64-bootonly.iso FreeBSD-12.1-RELEASE-amd64-dvd1.iso bee-hd1-01.img # /usr/sbin/bhyve -c 2 -m 2048 -H -A -s 0:0,hostbridge -s 1:0,lpc -s 2:0,e1000,tap0 -s 3:0,ahci-hd,bee-hd1-01.img -l com1,stdio -s 5:0,ahci-cd,./FreeBSD-12.1-RELEASE-amd64-dvd1.iso bee <> Result is that Layer 1 - FreeBSD Host guest "paused". To Layer 1 machines freezes I cannot get any further diagnostics from this machine, so I run tail on libvirt log from Layer 0 - Ubuntu Host <> char device redirected to /dev/pts/29 (label charserial0) 2020-05-04T06:09:15.310474Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(48FH).vmx-exit-load-perf-global-ctrl [bit 12] 2020-05-04T06:09:15.310531Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(490H).vmx-entry-load-perf-global-ctrl [bit 13] 2020-05-04T06:09:15.312533Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(48FH).vmx-exit-load-perf-global-ctrl [bit 12] 2020-05-04T06:09:15.312548Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(490H).vmx-entry-load-perf-global-ctrl [bit 13] 2020-05-04T06:09:15.313828Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(48FH).vmx-exit-load-perf-global-ctrl [bit 12] 2020-05-04T06:09:15.313841Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(490H).vmx-entry-load-perf-global-ctrl [bit 13] 2020-05-04T06:09:15.315185Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(48FH).vmx-exit-load-perf-global-ctrl [bit 12] 2020-05-04T06:09:15.315201Z qemu-system-x86_64: warning: host doesn't support requested feature: MSR(490H).vmx-entry-load-perf-global-ctrl [bit 13] KVM internal error. Suberror: 1 emulation failure EAX= EBX= ECX= EDX= ESI= EDI= EBP= ESP= EIP= EFL= [---] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES = 8000 DPL=0 CS = 8000 DPL=0 SS = 8000 DPL=0 DS = 8000 DPL=0 FS = 8000 DPL=0 GS = 8000 DPL=0 LDT= 8000 DPL=0 TR = 8000 DPL=0 GDT= IDT= CR0=80050033 CR2= CR3= CR4=00372060 DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 EFER=0d01 Code= ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? 2020-05-04T06:35:39.186799Z qemu-system-x86_64: terminating on signal 15 from pid 2155 (/usr/sbin/libvirtd) 2020-05-04 06:35:39.386+: shutting down, reason=destroyed <> I am reporting this bug here as result is very similar to that seen with QEMU seabios failure reported here: https://bugs.launchpad.net/qemu/+bug/1866870 However in this case my VM Layer 1 VM is using OVMF. NOTE 1: I have also tested with Q35 v3.1 and 2.12 and get the same result. NOTE 2: Due to bug in FreeBSD networking code, I had to compile custom kernel with "netmap driver disabled". This is known bug in FreeBSD that I have reported separately. NOTE 3: I will cross posted this bug report on FreeBSD bugzilla as well: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246168 NOTE 4: Have done extensive testing of Ubunt
Re: [RFC patch v1 2/3] qemu-file: add buffered mode
On Mon, Apr 27, 2020 at 01:14:33PM +0100, Dr. David Alan Gilbert wrote: > * Denis Plotnikov (dplotni...@virtuozzo.com) wrote: > > The patch adds ability to qemu-file to write the data > > asynchronously to improve the performance on writing. > > Before, only synchronous writing was supported. > > > > Enabling of the asyncronous mode is managed by new > > "enabled_buffered" callback. > > It's a bit invasive isn't it - changes a lot of functions in a lot of > places! > The multifd code separated the control headers from the data on separate > fd's - but that doesn't help your case. > > Is there any chance you could do this by using the existing 'save_page' > hook (that RDMA uses). > > In the cover letter you mention direct qemu_fflush calls - have we got a > few too many in some palces that you think we can clean out? When I first introduced the QIOChannel framework, I hoped that we could largely eliminate QEMUFile as a concept. Thus I'm a bit suspicious of the idea of introducing more functionality to QEMUFile, especially as the notion of buffering I/O is rather generic. Is there scope for having a QIOChannelBuffered object for doing buffering. Would that provide better isolation from the migration code and thus be less invasive/complex to maintain ? Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v6 3/5] 9pfs: add new function v9fs_co_readdir_many()
On Fri, 01 May 2020 16:04:41 +0200
Christian Schoenebeck wrote:
> On Donnerstag, 30. April 2020 15:30:49 CEST Greg Kurz wrote:
> > > > I agree that a client that issues concurrent readdir requests on the
> > > > same fid is probably asking for troubles, but this permitted by the
> > > > spec. Whether we should detect such conditions and warn or even fail
> > > > is discussion for another thread.
> > > >
> > > > The locking is only needed to avoid concurrent accesses to the dirent
> > > > structure returned by readdir(), otherwise we could return partially
> > > > overwritten file names to the client. It must be done for each
> > > > individual
> > > > call to readdir(), but certainly not for multiple calls.
> > >
> > > Yeah, that would resolve this issue more appropriately for 9p2000.L, since
> > > Treaddir specifies an offset, but for 9p2000.u the result of a concurrent
> > > read on a directory (9p2000.u) would still be undefined.
> >
> > The bad client behavior you want to tackle has nothing to do with
> > the locking itself. Since all the code in 9p.c runs serialized in
> > the context of the QEMU main loop, concurrent readdir requests could
> > easily be detected up-front with a simple flag in the fid structure.
>
> Well, it's fine with me. I don't really see an issue here right now. But that
> all the code was serialized is not fully true. Most of the 9p.c code is still
> heavily dispatching between main thread and worker threads back and forth.
> And
> for that reason the order of request processing might change quite
> arbitrarily
> in between. Just keep that in mind.
>
Just to make things clear. The code in 9p.c is ALWAYS exclusively run by
the main thread. Only the code called under v9fs_co_run_in_worker() is
dispatched on worker threads. So, yes the order of individual backend
operations may change, but the start of a new client request is necessarily
serialized with the completion of pending ones, which is the only thing
we care for actually.
> > > > > +
> > > > > +/* save the directory position */
> > > > > +saved_dir_pos = s->ops->telldir(&s->ctx, &fidp->fs);
> > > > > +if (saved_dir_pos < 0) {
> > > > > +err = saved_dir_pos;
> > > > > +goto out;
> > > > > +}
> > > > > +
> > > > > +while (true) {
> > > > > +/* get directory entry from fs driver */
> > > > > +err = do_readdir(pdu, fidp, &dent);
> > > > > +if (err || !dent) {
> > > > > +break;
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * stop this loop as soon as it would exceed the allowed
> > > > > maximum
> > > > > + * response message size for the directory entries collected
> > > > > so
> > > > > far, + * because anything beyond that size would need to be
> > > > > discarded by + * 9p controller (main thread / top half) anyway
> > > > > + */
> > > > > +v9fs_string_init(&name);
> > > > > +v9fs_string_sprintf(&name, "%s", dent->d_name);
> > > > > +len = v9fs_readdir_response_size(&name);
> > > > > +v9fs_string_free(&name);
> > > > > +if (size + len > maxsize) {
> > > > > +/* this is not an error case actually */
> > > > > +break;
> > > > > +}
> > > > > +
> > > > > +/* append next node to result chain */
> > > > > +if (!e) {
> > > > > +*entries = e = g_malloc0(sizeof(V9fsDirEnt));
> > > > > +} else {
> > > > > +e = e->next = g_malloc0(sizeof(V9fsDirEnt));
> > > > > +}
> > > > > +e->dent = g_malloc0(sizeof(struct dirent));
> > > >
> > > > So we're allocating a bunch of stuff here...
> > > >
> > > > > +memcpy(e->dent, dent, sizeof(struct dirent));
> > > > > +
> > > > > +/* perform a full stat() for directory entry if requested by
> > > > > caller */ +if (dostat) {
> > > > > +err = s->ops->name_to_path(
> > > > > +&s->ctx, &fidp->path, dent->d_name, &path
> > > > > +);
> > > > > +if (err < 0) {
> > > > >
> > > > > err = -errno;
> > > > >
> > > > > -} else {
> > > > > -*dent = entry;
> > > > > -err = 0;
> > > > > +break;
> > > >
> > > > ... but we're erroring out there and it seems that we're leaking
> > > > all the entries that have been allocated so far.
> > >
> > > No, they are not leaking actually.
> > >
> > > You are right that they are not deallocated in do_readdir_many(), but
> > > that's intentional: in the new implementation of v9fs_do_readdir() you
> > > see that v9fs_free_dirents(entries) is *always* called at the very end of
> > > the function, no matter if success or any error. That's one of the
> > > measures to simplify overall code as much as possible.
> >
> > Hmm... I still don't quite like the idea of having an erroring function
> > asking for extra cleanup. I suggest you come up with an i
Re: [PATCH v2 02/14] qcrypto/luks: implement encryption key management
On Sun, May 03, 2020 at 11:55:35AM +0300, Maxim Levitsky wrote:
> On Tue, 2020-04-28 at 14:16 +0100, Daniel P. Berrangé wrote:
> > On Sun, Mar 08, 2020 at 05:18:51PM +0200, Maxim Levitsky wrote:
> > > Next few patches will expose that functionality
> > > to the user.
> > >
> > > Signed-off-by: Maxim Levitsky
> > > ---
> > > crypto/block-luks.c | 398 +++-
> > > qapi/crypto.json| 61 ++-
> > > 2 files changed, 455 insertions(+), 4 deletions(-)
> > > +/*
> > > + * Given LUKSKeyslotUpdate command, set @slots_bitmap with all slots
> > > + * that will be updated with new password (or erased)
> > > + * returns 0 on success, and -1 on failure
> > > + */
> > > +static int
> > > +qcrypto_block_luks_get_update_bitmap(QCryptoBlock *block,
> > > + QCryptoBlockReadFunc readfunc,
> > > + void *opaque,
> > > + const QCryptoBlockAmendOptionsLUKS
> > > *opts,
> > > + unsigned long *slots_bitmap,
> > > + Error **errp)
> > > +{
> > > +const QCryptoBlockLUKS *luks = block->opaque;
> > > +size_t i;
> > > +
> > > +bitmap_zero(slots_bitmap, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
> > > +
> > > +if (opts->has_keyslot) {
> > > +/* keyslot set, select only this keyslot */
> > > +int keyslot = opts->keyslot;
> > > +
> > > +if (keyslot < 0 || keyslot >= QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS) {
> > > +error_setg(errp,
> > > + "Invalid slot %u specified, must be between 0 and
> > > %u",
> > > + keyslot, QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS - 1);
> > > +return -1;
> > > +}
> > > +bitmap_set(slots_bitmap, keyslot, 1);
> > > +
> > > +} else if (opts->has_old_secret) {
> > > +/* initially select all active keyslots */
> > > +for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> > > +if (qcrypto_block_luks_slot_active(luks, i)) {
> > > +bitmap_set(slots_bitmap, i, 1);
> > > +}
> > > +}
> > > +} else {
> > > +/* find a free keyslot */
> > > +int slot = qcrypto_block_luks_find_free_keyslot(luks);
> > > +
> > > +if (slot == -1) {
> > > +error_setg(errp,
> > > + "Can't add a keyslot - all key slots are in use");
> > > +return -1;
> > > +}
> > > +bitmap_set(slots_bitmap, slot, 1);
> > > +}
> > > +
> > > +if (opts->has_old_secret) {
> > > +/* now deselect all keyslots that don't contain the password */
> > > +g_autofree uint8_t *tmpkey = g_new0(uint8_t,
> > > +luks->header.master_key_len);
> > > +
> > > +for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
> > > +g_autofree char *old_password = NULL;
> > > +int rv;
> > > +
> > > +if (!test_bit(i, slots_bitmap)) {
> > > +continue;
> > > +}
> > > +
> > > +old_password =
> > > qcrypto_secret_lookup_as_utf8(opts->old_secret,
> > > + errp);
> > > +if (!old_password) {
> > > +return -1;
> > > +}
> > > +
> > > +rv = qcrypto_block_luks_load_key(block,
> > > + i,
> > > + old_password,
> > > + tmpkey,
> > > + readfunc,
> > > + opaque,
> > > + errp);
> > > +if (rv == -1) {
> > > +return -1;
> > > +} else if (rv == 0) {
> > > +bitmap_clear(slots_bitmap, i, 1);
> > > +}
> > > +}
> > > +}
> > > +return 0;
> > > +}
> >
> > I'm not really liking this function as a concept. Some of the code
> > only applies to the "add key" code path, while some of it only
> > applies to the "erase key" code path.
> >
> > I'd prefer it if qcrypto_block_luks_erase_keys directly had the
> > required logic, likewise qcrypto_block_luks_set_keys, and thus
> > get rid of the bitmap concept entirely. I thin kit'd make the
> > logic easier to understand.
>
> It used to be like that in former versions that I did send, I added the
> concept
> of the bitmap very recently to reflect the way we defined this in the spec.
> I don't mind that much coming back to older version of doing this,
> but beware that it won't be that clear either.
My view is that removing and adding keys are fundamentally different
operations, so although there's some parts that are in common, overall
it is better to keep them clearly separate.
> > > +/*
> > > + *
Re: [PATCH v2] MAINTAINERS: Mark the LatticeMico32 target as orphan
Le 16/03/2020 à 15:28, Philippe Mathieu-Daudé a écrit : > Michael Walle expressed his desire to orphan the lm32 target [*]: > > I guess it is time to pull the plug. Mainly, because I have > no time for this anymore. I've always worked on this on my > spare time and life changed. And secondly, I guess RISC-V is > taking over ;) It has a far better ecosystem. Also, to my > knowledge the only (public) user of LM32 is milkymist and this > project is dead for years now.. > > So time to say goodbye. It was fun and I've learned a lot - > technically and also how a huge open source project works. > Thank you everyone for that :) > > Basically everything still works and there are even TCG test > cases which covers all instructions the processor has. > > Many thanks to Michael for his substantial contributions to QEMU, > and for maintaining the LM32 target for various years! > > [*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg605024.html > > Acked-by: Michael Walle > Signed-off-by: Philippe Mathieu-Daudé > --- > v2: Also orphan machines, added Michael A-b tag > --- > MAINTAINERS | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 32867bc636..c89bf61989 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -183,8 +183,8 @@ F: hw/net/*i82596* > F: include/hw/net/lasi_82596.h > > LM32 TCG CPUs > -M: Michael Walle > -S: Maintained > +R: Michael Walle > +S: Orphan > F: target/lm32/ > F: disas/lm32.c > F: hw/lm32/ > @@ -945,13 +945,13 @@ F: pc-bios/hppa-firmware.img > LM32 Machines > - > EVR32 and uclinux BSP > -M: Michael Walle > -S: Maintained > +R: Michael Walle > +S: Orphan > F: hw/lm32/lm32_boards.c > > milkymist > -M: Michael Walle > -S: Maintained > +R: Michael Walle > +S: Orphan > F: hw/lm32/milkymist.c > > M68K Machines > Applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH] Fix iotest 153
On 03.05.20 18:49, Maxim Levitsky wrote:
> Commit f62514b3def5fb2acbef64d0e053c0c31fa45aff made qemu-img reject -o ""
> but this test uses it
>
> Since this test only tries to do a dry-run run of qemu-img amend, replace the
> -o "" with
> dummy -o "size=0" since due to the nature of the test, it is not going
> to reach the actual amend operation anyway
>
> Fixes: f62514b3def5fb2acbef64d0e053c0c31fa45aff
>
> Signed-off-by: Maxim Levitsky
> ---
> tests/qemu-iotests/153 | 2 +-
> tests/qemu-iotests/153.out | 12 ++--
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
> index 2b13111768..3f5029dd8f 100755
> --- a/tests/qemu-iotests/153
> +++ b/tests/qemu-iotests/153
> @@ -122,7 +122,7 @@ for opts1 in "" "read-only=on"
> "read-only=on,force-share=on"; do
> _run_cmd $QEMU_IMG check $L "${TEST_IMG}"
> _run_cmd $QEMU_IMG compare $L "${TEST_IMG}" "${TEST_IMG}"
> _run_cmd $QEMU_IMG map $L "${TEST_IMG}"
> -_run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}"
> +_run_cmd $QEMU_IMG amend -o "size=0" $L "${TEST_IMG}"
AFAIU we don’t want this command to actually change the image (hence the
empty options list, which would result in nothing being changed), so
maybe "size=$size" would be more in the spirit of the test?
Max
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 0/2] hw/mem/pc-dimm: Trivial code changes
Le 10/03/2020 à 19:05, Wainer dos Santos Moschetta a écrit : > Improve one error message and fix one code style warning. > > Wainer dos Santos Moschetta (2): > hw/mem/pc-dimm: Print slot number on error at pc_dimm_pre_plug() > hw/mem/pc-dimm: Fix line over 80 characters warning > > hw/mem/pc-dimm.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > Applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
On Mon, 4 May 2020 at 06:13, Michael S. Tsirkin wrote: > > On Tue, Apr 21, 2020 at 01:59:27PM +0100, Shameer Kolothum wrote: > > This series adds NVDIMM support to arm/virt platform. > > The series reuses some of the patches posted by Eric > > in his earlier attempt here[1]. > > > > This series previously had few fixes to qemu in general > > which were discovered while adding nvdimm support to arm/virt. > > Those were sent out seperately[2] and are now part of Qemu. > > > Mostly ACPI stuff so I can merge it if I get an ack for ARM side. Happy for you to take it; all the arm-specific bits have been reviewed by various people (thanks!) so here's my Acked-by: Peter Maydell I notice that checkpatch complains a lot about ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c: both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c found Does that need fixing, or is the checkpatch test wrong? thanks -- PMM
Re: [PATCH 02/17] qom: Clean up inconsistent use of gchar * vs. char *
On Sat, May 02, 2020 at 07:06:38AM +0200, Markus Armbruster wrote: > Eric Blake writes: > > > On 4/28/20 11:34 AM, Markus Armbruster wrote: > >> Uses of gchar * in qom/object.h: > >> > > > > Nice audit. I don't know if we can blindly assume that 'gchar' is a > > (stupid) typedef to 'char', or if they can ever differ, but avoiding > > gchar where possible always makes sense to me. > > GLib's "basic types" are one of the most misguided aspects of its > interface. There is work to re-define them in terms of stdint.h https://gitlab.gnome.org/GNOME/glib/issues/1484 with long term possible plan to deprecate them > Quote https://developer.gnome.org/glib/stable/glib-Basic-Types.html > > GLib defines a number of commonly used types, which can be divided > into several groups: > > New types which are not part of standard C (but are defined in > various C standard library header files) — gboolean, gssize. > > Stuck in the 90s. snip > Purge with fire. Note gboolean is a trap door. Any code integrating with GLib APIs that use "gboolean" in their signature must keep using that. It is *NOT* interchangable with the "bool" from stdbool.h. "gboolean" is a typedef for "gint" "bool" is a "_Bool" which is just a single byte. Also note TRUE and true are not the same value. 'TRUE' is bitwise !FALSE 'true' is 1 Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] chardev: enable distinct input for -chardev file
On Fri, May 01, 2020 at 01:25:59PM -0400, Alexander Bulekov wrote:
> char-file already supports distinct paths for input/output but it was
> only possible to specify a distinct input through QMP. With this change,
> we can also specify a distinct input with the -chardev file argument:
> qemu -chardev file,id=char1,path=/out/file,in=/in/file
>
> Signed-off-by: Alexander Bulekov
> ---
> chardev/char-file.c | 5 +
> chardev/char.c | 3 +++
> qemu-options.hx | 7 +--
> 3 files changed, 13 insertions(+), 2 deletions(-)
>
> The naming here is awkward, with path= really turning into "out" when
> in= is specified, though i'm not sure about what is a good solution.
>
> diff --git a/chardev/char-file.c b/chardev/char-file.c
> index 2fd80707e5..cc742cc234 100644
> --- a/chardev/char-file.c
> +++ b/chardev/char-file.c
> @@ -100,6 +100,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts,
> ChardevBackend *backend,
> Error **errp)
> {
> const char *path = qemu_opt_get(opts, "path");
> +const char *in = qemu_opt_get(opts, "in");
> ChardevFile *file;
>
> backend->type = CHARDEV_BACKEND_KIND_FILE;
> @@ -110,6 +111,10 @@ static void qemu_chr_parse_file_out(QemuOpts *opts,
> ChardevBackend *backend,
> file = backend->u.file.data = g_new0(ChardevFile, 1);
> qemu_chr_parse_common(opts, qapi_ChardevFile_base(file));
> file->out = g_strdup(path);
> +if (in) {
> +file->has_in = true;
> +file->in = g_strdup(qemu_opt_get(opts, "in"));
> +}
>
> file->has_append = true;
> file->append = qemu_opt_get_bool(opts, "append", false);
> diff --git a/chardev/char.c b/chardev/char.c
> index e77564060d..797574f205 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -849,6 +849,9 @@ QemuOptsList qemu_chardev_opts = {
> },{
> .name = "path",
> .type = QEMU_OPT_STRING,
> +},{
> +.name = "in",
> +.type = QEMU_OPT_STRING,
> },{
> .name = "host",
> .type = QEMU_OPT_STRING,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 292d4e7c0c..bbb091872f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2938,7 +2938,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> "-chardev
> vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
> " [,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> "-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n"
> -"-chardev
> file,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> +"-chardev
> file,id=id,path=path[,in=PATH][,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> "-chardev
> pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> #ifdef _WIN32
> "-chardev console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> @@ -3137,13 +3137,16 @@ The available backends are:
> Create a ring buffer with fixed size ``size``. size must be a power
> of two and defaults to ``64K``.
>
> -``-chardev file,id=id,path=path``
> +``-chardev file,id=id,path=path,in=in``
> Log all traffic received from the guest to a file.
>
> ``path`` specifies the path of the file to be opened. This file will
> be created if it does not already exist, and overwritten if it does.
> ``path`` is required.
>
> +``in`` specifies a separate file as the input to the chardev. If
> +``in`` is omitted, ``path`` is used for both input and output
I'd suggest "pathin" rather than just "in"
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH] aspeed: Support AST2600A1 silicon revision
There are minimal differences from Qemu's point of view between the A0
and A1 silicon revisions.
As the A1 exercises different code paths in u-boot it is desirable to
emulate that instead.
Signed-off-by: Joel Stanley
---
hw/arm/aspeed.c | 8
hw/arm/aspeed_ast2600.c | 6 +++---
hw/misc/aspeed_scu.c | 11 +--
include/hw/misc/aspeed_scu.h | 1 +
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 99a0f3fcf36e..91301efab32d 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -93,7 +93,7 @@ struct AspeedBoardState {
/* Tacoma hardware value */
#define TACOMA_BMC_HW_STRAP1 0x
-#define TACOMA_BMC_HW_STRAP2 0x
+#define TACOMA_BMC_HW_STRAP2 0x0040
/*
* The max ram region is for firmwares that scan the address space
@@ -585,7 +585,7 @@ static void
aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
mc->desc = "Aspeed AST2600 EVB (Cortex A7)";
-amc->soc_name = "ast2600-a0";
+amc->soc_name = "ast2600-a1";
amc->hw_strap1 = AST2600_EVB_HW_STRAP1;
amc->hw_strap2 = AST2600_EVB_HW_STRAP2;
amc->fmc_model = "w25q512jv";
@@ -600,8 +600,8 @@ static void aspeed_machine_tacoma_class_init(ObjectClass
*oc, void *data)
MachineClass *mc = MACHINE_CLASS(oc);
AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
-mc->desc = "Aspeed AST2600 EVB (Cortex A7)";
-amc->soc_name = "ast2600-a0";
+mc->desc = "OpenPOWER Tacoma BMC (Cortex A7)";
+amc->soc_name = "ast2600-a1";
amc->hw_strap1 = TACOMA_BMC_HW_STRAP1;
amc->hw_strap2 = TACOMA_BMC_HW_STRAP2;
amc->fmc_model = "mx66l1g45g";
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 1a869e09b96a..c6e0ab84ac86 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -557,9 +557,9 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc,
void *data)
dc->realize = aspeed_soc_ast2600_realize;
-sc->name = "ast2600-a0";
+sc->name = "ast2600-a1";
sc->cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
-sc->silicon_rev = AST2600_A0_SILICON_REV;
+sc->silicon_rev = AST2600_A1_SILICON_REV;
sc->sram_size= 0x1;
sc->spis_num = 2;
sc->ehcis_num= 2;
@@ -571,7 +571,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass *oc,
void *data)
}
static const TypeInfo aspeed_soc_ast2600_type_info = {
-.name = "ast2600-a0",
+.name = "ast2600-a1",
.parent = TYPE_ASPEED_SOC,
.instance_size = sizeof(AspeedSoCState),
.instance_init = aspeed_soc_ast2600_init,
diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 9d7482a9df19..ec4fef900e27 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -431,6 +431,7 @@ static uint32_t aspeed_silicon_revs[] = {
AST2500_A0_SILICON_REV,
AST2500_A1_SILICON_REV,
AST2600_A0_SILICON_REV,
+AST2600_A1_SILICON_REV,
};
bool is_supported_silicon_rev(uint32_t silicon_rev)
@@ -649,12 +650,10 @@ static const MemoryRegionOps aspeed_ast2600_scu_ops = {
.valid.unaligned = false,
};
-static const uint32_t ast2600_a0_resets[ASPEED_AST2600_SCU_NR_REGS] = {
-[AST2600_SILICON_REV] = AST2600_SILICON_REV,
-[AST2600_SILICON_REV2] = AST2600_SILICON_REV,
-[AST2600_SYS_RST_CTRL] = 0xF7CFFEDC | 0x100,
+static const uint32_t ast2600_a1_resets[ASPEED_AST2600_SCU_NR_REGS] = {
+[AST2600_SYS_RST_CTRL] = 0xF7C3FED8,
[AST2600_SYS_RST_CTRL2] = 0xFFFC,
-[AST2600_CLK_STOP_CTRL] = 0xEFF43E8B,
+[AST2600_CLK_STOP_CTRL] = 0x7F8A,
[AST2600_CLK_STOP_CTRL2]= 0xFFF0FFF0,
[AST2600_SDRAM_HANDSHAKE] = 0x0040, /* SoC completed DRAM init */
[AST2600_HPLL_PARAM]= 0x1000405F,
@@ -684,7 +683,7 @@ static void aspeed_2600_scu_class_init(ObjectClass *klass,
void *data)
dc->desc = "ASPEED 2600 System Control Unit";
dc->reset = aspeed_ast2600_scu_reset;
-asc->resets = ast2600_a0_resets;
+asc->resets = ast2600_a1_resets;
asc->calc_hpll = aspeed_2500_scu_calc_hpll; /* No change since AST2500 */
asc->apb_divider = 4;
asc->nr_regs = ASPEED_AST2600_SCU_NR_REGS;
diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
index 1d7f7ffc1598..a6739bb846b6 100644
--- a/include/hw/misc/aspeed_scu.h
+++ b/include/hw/misc/aspeed_scu.h
@@ -41,6 +41,7 @@ typedef struct AspeedSCUState {
#define AST2500_A0_SILICON_REV 0x04000303U
#define AST2500_A1_SILICON_REV 0x04010303U
#define AST2600_A0_SILICON_REV 0x05000303U
+#define AST2600_A1_SILICON_REV 0x05010303U
#define ASPEED_IS_AST2500(si_rev) si_rev) >> 24) & 0xff) == 0x04)
--
2.26.2
Re: [PATCH v3 04/18] accel/tcg: Add probe_access_flags
On Mon, 27 Apr 2020 at 17:00, Richard Henderson wrote: > > On 4/27/20 3:48 AM, Peter Maydell wrote: > > The old code passed size into cc->tlb_fill; the new version does not. > > The old code passed size into page_check_range(); the new version does not. > > This is the user-only version, and size is not used for tlb_fill. It is only > trivially used in page_change_range; we have just verified that addr+size does > not cross a page boundary. Yes, but: * they're APIs which take sizes * we have the size in hand so it's not difficult to pass it * in future maybe those functions will change to make more use of the size value, since they're being passed it * this change ought to be a refactoring that does the same thing the old code does What's the reason for deliberately *not* passing the size and instead using a constant 1 ? thanks -- PMM
Re: [PATCH] Fix iotest 153
On Mon, 2020-05-04 at 11:22 +0200, Max Reitz wrote:
> On 03.05.20 18:49, Maxim Levitsky wrote:
> > Commit f62514b3def5fb2acbef64d0e053c0c31fa45aff made qemu-img reject -o ""
> > but this test uses it
> >
> > Since this test only tries to do a dry-run run of qemu-img amend, replace
> > the -o "" with
> > dummy -o "size=0" since due to the nature of the test, it is not going
> > to reach the actual amend operation anyway
> >
> > Fixes: f62514b3def5fb2acbef64d0e053c0c31fa45aff
> >
> > Signed-off-by: Maxim Levitsky
> > ---
> > tests/qemu-iotests/153 | 2 +-
> > tests/qemu-iotests/153.out | 12 ++--
> > 2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
> > index 2b13111768..3f5029dd8f 100755
> > --- a/tests/qemu-iotests/153
> > +++ b/tests/qemu-iotests/153
> > @@ -122,7 +122,7 @@ for opts1 in "" "read-only=on"
> > "read-only=on,force-share=on"; do
> > _run_cmd $QEMU_IMG check $L "${TEST_IMG}"
> > _run_cmd $QEMU_IMG compare $L "${TEST_IMG}" "${TEST_IMG}"
> > _run_cmd $QEMU_IMG map $L "${TEST_IMG}"
> > -_run_cmd $QEMU_IMG amend -o "" $L "${TEST_IMG}"
> > +_run_cmd $QEMU_IMG amend -o "size=0" $L "${TEST_IMG}"
>
> AFAIU we don’t want this command to actually change the image (hence the
> empty options list, which would result in nothing being changed), so
> maybe "size=$size" would be more in the spirit of the test?
This is a good idea! Should I resend the patch or you can add this change
locally?
Best regards,
Maxim Levitsky
>
> Max
>
Re: [PATCH v4 13/18] target/arm: Update contiguous first-fault and no-fault loads
On Thu, 30 Apr 2020 at 17:28, Richard Henderson wrote: > > With sve_cont_ldst_pages, the differences between first-fault and no-fault > are minimal, so unify the routines. With cpu_probe_watchpoint, we are able > to make progress through pages with TLB_WATCHPOINT set when the watchpoint > does not actually fire. > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v4 00/18] target/arm: sve load/store improvements
On Thu, 30 Apr 2020 at 17:28, Richard Henderson wrote: > > The goal here is to support MTE, but there's some cleanup to do. > > Technically, we have sufficient interfaces in cputlb.c now, but it > requires multiple tlb lookups on different interfaces to do so. > > Adding probe_access_flags() allows probing the tlb and getting out > some of the flags buried in the tlb comparator, such as TLB_MMIO > and TLB_WATCHPOINT. In addition, we get no-fault semantics, > which we don't have via probe_acccess(). > > Looking forward to MTE, we can examine the Tagged bit on a per-page > basis and avoid dozens of mte_check calls that must be Unchecked. > That comes later, in a new version of the MTE patch set, but I do > add comments for where the checks should be added. > > Version 4 addresses some review comments. > Only 2 patches still unreviewed: > > 0004-accel-tcg-Add-probe_access_flags.patch > 0013-target-arm-Update-contiguous-first-fault-and-no-f.patch I've reviewed patch 13, but I still don't understand why you've made the size-related changes in patch 4, so I've continued our conversation in the thread on the v3 version of that patch. thanks -- PMM
Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
Hi, On 5/4/20 11:29 AM, Peter Maydell wrote: > On Mon, 4 May 2020 at 06:13, Michael S. Tsirkin wrote: >> >> On Tue, Apr 21, 2020 at 01:59:27PM +0100, Shameer Kolothum wrote: >>> This series adds NVDIMM support to arm/virt platform. >>> The series reuses some of the patches posted by Eric >>> in his earlier attempt here[1]. >>> >>> This series previously had few fixes to qemu in general >>> which were discovered while adding nvdimm support to arm/virt. >>> Those were sent out seperately[2] and are now part of Qemu. >> >> >> Mostly ACPI stuff so I can merge it if I get an ack for ARM side. > > Happy for you to take it; all the arm-specific bits have > been reviewed by various people (thanks!) so here's my > > Acked-by: Peter Maydell Tested-by: Eric Auger Thanks Eric > > I notice that checkpatch complains a lot about > > ERROR: Do not add expected files together with tests, follow > instructions in tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and > tests/qtest/bios-tables-test.c found > > Does that need fixing, or is the checkpatch test wrong? > > thanks > -- PMM >
Re: [PATCH] MAINTAINERS: Update Keith Busch's email address
Le 04/05/2020 à 10:58, Philippe Mathieu-Daudé a écrit : > +block +trivial to raise the odds to get this patch merged. > > On 4/22/20 6:45 PM, Keith Busch wrote: >> On Tue, Apr 21, 2020 at 02:22:36PM +0200, Philippe Mathieu-Daudé wrote: >>> keith.bu...@intel.com address is being rejected. >>> Replace by the email address Keith is actively using. >>> >>> Signed-off-by: Philippe Mathieu-Daudé >> >> Thank you for sending the correction. >> >> Acked-by: Keith Busch >> > Applied to my trivial-patches branch. Thanks, Laurent
Re: RFC: use VFIO over a UNIX domain socket to implement device offloading
On Fri, May 01, 2020 at 04:28:25PM +0100, Daniel P. Berrangé wrote: > On Fri, May 01, 2020 at 03:01:01PM +, Felipe Franciosi wrote: > > Hi, > > > > > On Apr 30, 2020, at 4:20 PM, Thanos Makatos > > > wrote: > > > > > More importantly, considering: > > a) Marc-André's comments about data alignment etc., and > > b) the possibility to run the server on another guest or host, > > we won't be able to use native VFIO types. If we do want to support > > that > > then > > we'll have to redefine all data formats, similar to > > https://urldefense.proofpoint.com/v2/url?u=https- > > 3A__github.com_qemu_qemu_blob_master_docs_interop_vhost- > > > > >> 2Duser.rst&d=DwIFAw&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh5Ps2zJvtw6 > > > > >> ogtti46atk736SI4vgsJiUKIyDE&m=lJC7YeMMsAaVsr99tmTYncQdjEfOXiJQkRkJ > > W7NMgRg&s=1d_kB7VWQ- > > >> 8d4t6Ikga5KSVwws4vwiVMvTyWVaS6PRU&e= . > > > > So the protocol will be more like an enhanced version of the Vhost-user > > protocol > > than VFIO. I'm fine with either direction (VFIO vs. enhanced > > Vhost-user), > > so we need to decide before proceeding as the request format is > > substantially > > different. > > >>> > > >>> Regarding the ability to use the protocol on non-AF_UNIX sockets, we can > > >>> support this future use case without unnecessarily complicating the > > >> protocol by > > >>> defining the C structs and stating that data alignment and endianness > > >>> for > > >> the > > >>> non AF_UNIX case must be the one used by GCC on a x86_64 bit machine, > > >> or can > > >>> be overridden as required. > > >> > > >> Defining it to be x86_64 semantics is effectively saying "we're not going > > >> to do anything and it is up to other arch maintainers to fix the > > >> inevitable > > >> portability problems that arise". > > > > > > Pretty much. > > > > > >> Since this is a new protocol should we take the opportunity to model it > > >> explicitly in some common standard RPC protocol language. This would have > > >> the benefit of allowing implementors to use off the shelf APIs for their > > >> wire protocol marshalling, and eliminate questions about endianness and > > >> alignment across architectures. > > > > > > The problem is that we haven't defined the scope very well. My initial > > > impression > > > was that we should use the existing VFIO structs and constants, however > > > that's > > > impossible if we're to support non AF_UNIX. We need consensus on this, > > > we're > > > open to ideas how to do this. > > > > Thanos has a point. > > > > From https://wiki.qemu.org/Features/MultiProcessQEMU, which I believe > > was written by Stefan, I read: > > > > > Inventing a new device emulation protocol from scratch has many > > > disadvantages. VFIO could be used as the protocol to avoid reinventing > > > the wheel ... > > > > At the same time, this appears to be incompatible with the (new?) > > requirement of supporting device emulation which may run in non-VFIO > > compliant OSs or even across OSs (ie. via TCP or similar). > > To be clear, I don't have any opinion on whether we need to support > cross-OS/TCP or not. > > I'm merely saying that if we do decide to support cross-OS/TCP, then > I think we need a more explicitly modelled protocol, instead of relying > on serialization of C structs. > > There could be benefits to an explicitly modelled protocol, even for > local only usage, if we want to more easily support non-C languages > doing serialization, but again I don't have a strong opinion on whether > that's neccessary to worry about or not. > > So I guess largely the question boils down to setting the scope of > what we want to be able to achieve in terms of RPC endpoints. The protocol relies on both file descriptor and memory mapping. These are hard to achieve with networking. I think the closest would be using RDMA to accelerate memory access and switching to a network notification mechanism instead of eventfd. Sooner or later someone will probably try this. I don't think it makes sense to define this transport in detail now if there are no users, but we should try to make it possible to add it in the future, if necessary. Another use case that is interesting and not yet directly addressed is: how can another VM play the role of the device? This is important in compute cloud environments where everything is a VM and running a process on the host is not possible. The virtio-vhost-user prototype showed that it's possible to add this on top of an existing vhost-user style protocol by terminating the connection in the device VMM and then communicating with the device using a new VIRTIO device. Maybe that's the way to do it here too and we don't need to worry about explicitly designing that into the vfio-user protocol, but if anyone has other approaches in mind then let's discuss them now. Finally, I think the goal of integrating this new protocol
Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
On Mon, May 04, 2020 at 10:29:18AM +0100, Peter Maydell wrote: > On Mon, 4 May 2020 at 06:13, Michael S. Tsirkin wrote: > > > > On Tue, Apr 21, 2020 at 01:59:27PM +0100, Shameer Kolothum wrote: > > > This series adds NVDIMM support to arm/virt platform. > > > The series reuses some of the patches posted by Eric > > > in his earlier attempt here[1]. > > > > > > This series previously had few fixes to qemu in general > > > which were discovered while adding nvdimm support to arm/virt. > > > Those were sent out seperately[2] and are now part of Qemu. > > > > > > Mostly ACPI stuff so I can merge it if I get an ack for ARM side. > > Happy for you to take it; all the arm-specific bits have > been reviewed by various people (thanks!) so here's my > > Acked-by: Peter Maydell > > I notice that checkpatch complains a lot about > > ERROR: Do not add expected files together with tests, follow > instructions in tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and > tests/qtest/bios-tables-test.c found > > Does that need fixing, or is the checkpatch test wrong? > > thanks > -- PMM Hmm I don't see a patch in this series that changes both tests/qtest/bios-tables-test-allowed-diff.h and tests/qtest/bios-tables-test.c. Do you? -- MST
[PATCH 2/6] qemu/bitmap: Document bitmap_new() returned pointer
Signed-off-by: Philippe Mathieu-Daudé
---
include/qemu/bitmap.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 82a1d2f41f..0b390ff576 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -90,12 +90,14 @@ int slow_bitmap_intersects(const unsigned long *bitmap1,
const unsigned long *bitmap2, long bits);
long slow_bitmap_count_one(const unsigned long *bitmap, long nbits);
+/* callers must free the returned pointer with g_free() */
static inline unsigned long *bitmap_try_new(long nbits)
{
long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
return g_try_malloc0(len);
}
+/* callers must free the returned pointer with g_free() */
static inline unsigned long *bitmap_new(long nbits)
{
unsigned long *ptr = bitmap_try_new(nbits);
--
2.21.3
[PATCH 0/6] block/nvme: Align block pages queue to host page size
The most important patch of this series is 'Align block pages queue to host page size', it start fixing an issue on PowerPC hosts. It is not sufficient, but the rest is VFIO specific, so I prefer to send it separately. The other patches are documentation I was writing down while looking at the code. The last patch is about the emulated device and writeback. Marked as RFC as I don't understand writeback/dirty_log_mask yet. Philippe Mathieu-Daudé (6): qemu/osdep: Document qemu_memalign() and friends qemu/bitmap: Document bitmap_new() returned pointer sysemu/block-backend: Document blk_read()/blk_pwrite() block/block: Document BlockSizes fields block/nvme: Align block pages queue to host page size hw/block/nvme: Make device target agnostic include/block/block.h | 8 ++-- include/qemu/bitmap.h | 2 ++ include/qemu/osdep.h | 3 +++ include/sysemu/block-backend.h | 26 ++ block/nvme.c | 2 +- hw/block/nvme.c| 4 +--- hw/block/Makefile.objs | 2 +- 7 files changed, 40 insertions(+), 7 deletions(-) -- 2.21.3
Re: [PATCH] elf_ops: Don't try to g_mapped_file_unref(NULL)
Le 23/04/2020 à 22:20, Peter Maydell a écrit :
> Calling g_mapped_file_unref() on a NULL pointer is not valid, and
> glib will assert if you try it.
>
> $ qemu-system-arm -M virt -display none -device loader,file=/tmp/bad.elf
> qemu-system-arm: -device loader,file=/tmp/bad.elf: GLib: g_mapped_file_unref:
> assertion 'file != NULL' failed
>
> (One way to produce an ELF file that fails like this is to copy just
> the first 16 bytes of a valid ELF file; this is sufficient to fool
> the code in load_elf_ram_sym() into thinking it's an ELF file and
> calling load_elf32() or load_elf64().)
>
> The failure-exit path in load_elf can be reached from various points
> in execution, and for some of those we haven't yet called
> g_mapped_file_new_from_fd(). Add a condition to the unref call so we
> only call it if we successfully created the GMappedFile to start with.
>
> This will fix the assertion; for the specific case of the generic
> loader it will then fall back from "guess this is an ELF file" to
> "maybe it's a uImage or a hex file" and eventually to "just load as
> a raw data file".
>
> Reported-by: Randy Yates
> Signed-off-by: Peter Maydell
> ---
> include/hw/elf_ops.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index e0bb47bb678..398a4a2c85b 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -606,7 +606,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
> *highaddr = (uint64_t)(elf_sword)high;
> ret = total_size;
> fail:
> -g_mapped_file_unref(mapped_file);
> +if (mapped_file) {
> +g_mapped_file_unref(mapped_file);
> +}
> g_free(phdr);
> return ret;
> }
>
Applied to my trivial-patches branch.
Thanks,
Laurent
[PATCH 4/6] block/block: Document BlockSizes fields
As it is not obvious for a block neophyte what means
the 'log' value, document it.
Signed-off-by: Philippe Mathieu-Daudé
---
include/block/block.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index c57fdecf9a..94517c92b6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -86,8 +86,8 @@ typedef enum {
} BdrvRequestFlags;
typedef struct BlockSizes {
-uint32_t phys;
-uint32_t log;
+uint32_t phys; /* physical block size */
+uint32_t log; /* logical block size */
} BlockSizes;
typedef struct HDGeometry {
--
2.21.3
[PATCH 1/6] qemu/osdep: Document qemu_memalign() and friends
Document allocator functions that require a specific de-allocator call. Signed-off-by: Philippe Mathieu-Daudé --- include/block/block.h | 4 include/qemu/osdep.h | 3 +++ include/sysemu/block-backend.h | 2 ++ 3 files changed, 9 insertions(+) diff --git a/include/block/block.h b/include/block/block.h index 8b62429aa4..c57fdecf9a 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -526,9 +526,13 @@ void bdrv_img_create(const char *filename, const char *fmt, size_t bdrv_min_mem_align(BlockDriverState *bs); /* Returns optimal alignment in bytes for bounce buffer */ size_t bdrv_opt_mem_align(BlockDriverState *bs); +/* callers must free the returned pointer with qemu_vfree() */ void *qemu_blockalign(BlockDriverState *bs, size_t size); +/* callers must free the returned pointer with qemu_vfree() */ void *qemu_blockalign0(BlockDriverState *bs, size_t size); +/* callers must free the returned pointer with qemu_vfree() */ void *qemu_try_blockalign(BlockDriverState *bs, size_t size); +/* callers must free the returned pointer with qemu_vfree() */ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size); bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov); diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 20f5c5f197..778c459c22 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -294,8 +294,11 @@ extern int daemon(int, int); #endif int qemu_daemon(int nochdir, int noclose); +/* callers must free the returned pointer with qemu_vfree() */ void *qemu_try_memalign(size_t alignment, size_t size); +/* callers must free the returned pointer with qemu_vfree() */ void *qemu_memalign(size_t alignment, size_t size); +/* callers must free the returned pointer with qemu_anon_ram_free() */ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared); void qemu_vfree(void *ptr); void qemu_anon_ram_free(void *ptr, size_t size); diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 34de7faa81..f2dcf63ae3 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -203,7 +203,9 @@ uint32_t blk_get_request_alignment(BlockBackend *blk); uint32_t blk_get_max_transfer(BlockBackend *blk); int blk_get_max_iov(BlockBackend *blk); void blk_set_guest_block_size(BlockBackend *blk, int align); +/* callers must free the returned pointer with qemu_vfree() */ void *blk_try_blockalign(BlockBackend *blk, size_t size); +/* callers must free the returned pointer with qemu_vfree() */ void *blk_blockalign(BlockBackend *blk, size_t size); bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp); void blk_op_unblock(BlockBackend *blk, BlockOpType op, Error *reason); -- 2.21.3
[PATCH 5/6] block/nvme: Align block pages queue to host page size
In nvme_create_queue_pair() we create a page list using qemu_blockalign(), then map it with qemu_vfio_dma_map(): q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_QUEUE_SIZE); r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, s->page_size * NVME_QUEUE_SIZE, ...); With: s->page_size = MAX(4096, 1 << (12 + ((cap >> 48) & 0xF))); The qemu_vfio_dma_map() documentation says "The caller need to make sure the area is aligned to page size". While we use multiple s->page_size as alignment, it might be not sufficient on some hosts. Use the qemu_real_host_page_size value to be sure the host alignment is respected. Signed-off-by: Philippe Mathieu-Daudé --- Cc: Cédric Le Goater Cc: David Gibson Cc: Laurent Vivier --- block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/nvme.c b/block/nvme.c index 7b7c0cc5d6..bde0d28b39 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -627,7 +627,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, s->page_size = MAX(4096, 1 << (12 + ((cap >> 48) & 0xF))); s->doorbell_scale = (4 << (((cap >> 32) & 0xF))) / sizeof(uint32_t); -bs->bl.opt_mem_alignment = s->page_size; +bs->bl.opt_mem_alignment = MAX(qemu_real_host_page_size, s->page_size); timeout_ms = MIN(500 * ((cap >> 24) & 0xFF), 3); /* Reset device to get a clean state. */ -- 2.21.3
[PATCH 3/6] sysemu/block-backend: Document blk_read()/blk_pwrite()
The blk_read()/blk_pwrite() return value is not obvious, document it. Signed-off-by: Philippe Mathieu-Daudé --- include/sysemu/block-backend.h | 24 1 file changed, 24 insertions(+) diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index f2dcf63ae3..823b8e94a7 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -153,7 +153,31 @@ BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset, int bytes, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque); int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags); + +/** + * blk_pread: + * + * @blk - the block backend where the buffer content is going to be read from + * @offset: position in bytes to read at + * @buf: the data buffer + * @bytes: number of bytes to read + * + * Returns: the number of bytes read on success, or a negative errno otherwise. + */ int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int bytes); + +/** + * blk_pwrite: + * + * @blk - the block backend where the buffer content is going to be written to + * @offset: position in bytes to write at + * @buf: the data buffer + * @bytes: number of bytes of @buf to write + * @flags: request flags + * + * Returns: the number of bytes consumed on success, + * or a negative errno otherwise. + */ int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes, BdrvRequestFlags flags); int64_t blk_getlength(BlockBackend *blk); -- 2.21.3
[RFC PATCH 6/6] hw/block/nvme: Make device target agnostic
The NVMe device should not use target specific API.
Use memory_region_do_writeback() (which was introduced
in commit 61c490e25e0, after the NVMe emulated device
was added) to replace qemu_ram_writeback().
Signed-off-by: Philippe Mathieu-Daudé
---
RFC because I have no clue how dirty_log_mask works.
Cc: Paolo Bonzini
Cc: Beata Michalska
---
hw/block/nvme.c| 4 +---
hw/block/Makefile.objs | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 9b453423cf..9b0ac0ea2a 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -46,7 +46,6 @@
#include "qapi/visitor.h"
#include "sysemu/hostmem.h"
#include "sysemu/block-backend.h"
-#include "exec/ram_addr.h"
#include "qemu/log.h"
#include "qemu/module.h"
@@ -1207,8 +1206,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr,
unsigned size)
*/
if (addr == 0xE08 &&
(NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
-qemu_ram_writeback(n->pmrdev->mr.ram_block,
- 0, n->pmrdev->size);
+memory_region_do_writeback(&n->pmrdev->mr, 0, n->pmrdev->size);
}
memcpy(&val, ptr + addr, size);
} else {
diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index 47960b5f0d..8855c22656 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -13,6 +13,6 @@ common-obj-$(CONFIG_SH4) += tc58128.o
obj-$(CONFIG_VIRTIO_BLK) += virtio-blk.o
obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
-obj-$(CONFIG_NVME_PCI) += nvme.o
+common-obj-$(CONFIG_NVME_PCI) += nvme.o
obj-y += dataplane/
--
2.21.3
Re: [PATCH] chardev: Add macOS to list of OSes that support -chardev serial
Le 26/04/2020 à 23:09, Mikhail Gusarov a écrit : > macOS API for dealing with serial ports/ttys is identical to BSDs. > > Signed-off-by: Mikhail Gusarov > --- > > Note that the same file has a line >> #endif /* linux || sun */ > that is severely out of date. > > chardev/char-serial.c | 2 +- > include/qemu/osdep.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/chardev/char-serial.c b/chardev/char-serial.c > index 5b833ea077..7c3d84ae24 100644 > --- a/chardev/char-serial.c > +++ b/chardev/char-serial.c > @@ -53,7 +53,7 @@ static void qmp_chardev_open_serial(Chardev *chr, > > #elif defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \ > || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) > \ > -|| defined(__GLIBC__) > +|| defined(__GLIBC__) || defined(__APPLE__) > > static void tty_serial_init(int fd, int speed, > int parity, int data_bits, int stop_bits) > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 20f5c5f197..ff7c17b857 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -379,7 +379,7 @@ void qemu_anon_ram_free(void *ptr, size_t size); > #define HAVE_CHARDEV_SERIAL 1 > #elif defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)\ > || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) > \ > -|| defined(__GLIBC__) > +|| defined(__GLIBC__) || defined(__APPLE__) > #define HAVE_CHARDEV_SERIAL 1 > #endif > > Applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH] virtiofsd: Use clone() and not unshare(), support non-root
On Fri, May 01, 2020 at 02:25:48PM -0400, Colin Walters wrote:
> I'd like to make use of virtiofs as part of our tooling in
> https://github.com/coreos/coreos-assembler
> Most of the code runs as non-root today; qemu also runs as non-root.
> We use 9p right now.
>
> virtiofsd's builtin sandboxing effectively assumes it runs as
> root.
>
> First, change the code to use `clone()` and not `unshare()+fork()`.
>
> Next, automatically use `CLONE_NEWUSER` if we're running as non root.
I'd suggest splitting these two, so that the re-factoring is separate
from introducing new functionality.
>
> This is similar logic to that in https://github.com/containers/bubblewrap
> (Which...BTW, it could make sense for virtiofs to depend on bubblewrap
> and re-exec itself rather than re-implementing the containerization
> itself)
>
> Signed-off-by: Colin Walters
> ---
> tools/virtiofsd/passthrough_ll.c | 26 +-
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c
> b/tools/virtiofsd/passthrough_ll.c
> index 4c35c95b25..468617f6d6 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2530,6 +2530,21 @@ static void print_capabilities(void)
> printf("}\n");
> }
>
> +/* Copied from bubblewrap */
> +static int
> +raw_clone(unsigned long flags, void *child_stack)
> +{
> +#if defined(__s390__) || defined(__CRIS__)
> + /*
> + * On s390 and cris the order of the first and second arguments
> + * of the raw clone() system call is reversed.
> + */
> +return (int) syscall(__NR_clone, child_stack, flags);
> +#else
> +return (int) syscall(__NR_clone, flags, child_stack);
> +#endif
> +}
What's the reason for using the raw syscall ? Was it just to avoid having
to allocate a new stack space ?
> +
> /*
> * Move to a new mount, net, and pid namespaces to isolate this process.
> */
> @@ -2547,14 +2562,15 @@ static void setup_namespaces(struct lo_data *lo,
> struct fuse_session *se)
> * an empty network namespace to prevent TCP/IP and other network
> * activity in case this process is compromised.
> */
> -if (unshare(CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWNET) != 0) {
> -fuse_log(FUSE_LOG_ERR, "unshare(CLONE_NEWPID | CLONE_NEWNS): %m\n");
> -exit(1);
> +int clone_flags = SIGCHLD | CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWNET;
> +/* If we're non root, we need a new user namespace */
> +if (getuid() != 0) {
> +clone_flags |= CLONE_NEWUSER;
> }
IIUC, with CLONE_NEWUSER we need to set a UID/GID mapping, otherwise all
file accesses will be squashed to the UID -1. Or was it intentional
that you're only trying to provide read-only access to files that are
world-accessible ?
> -child = fork();
> +child = raw_clone(clone_flags, NULL);
> if (child < 0) {
> -fuse_log(FUSE_LOG_ERR, "fork() failed: %m\n");
> +fuse_log(FUSE_LOG_ERR, "clone() failed: %m\n");
> exit(1);
> }
> if (child > 0) {
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH] hw/ide/ahci: Log lost IRQs
One might find interesting to look at AHCI IRQs.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/ide/ahci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 13d91e109a..fc82cbd5f1 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1509,6 +1509,7 @@ static void ahci_cmd_done(IDEDMA *dma)
static void ahci_irq_set(void *opaque, int n, int level)
{
+qemu_log_mask(LOG_UNIMP, "ahci: IRQ#%d level:%d\n", n, level);
}
static const IDEDMAOps ahci_dma_ops = {
--
2.21.3
Re: [PATCH 1/2] hw/display: Include local 'framebuffer.h'
Am Mon, 4 May 2020 10:20:02 +0200 schrieb Philippe Mathieu-Daudé : > The "framebuffer.h" header is not an exported include. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/display/artist.c | 2 +- > hw/display/next-fb.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Thomas Huth
Re: [PATCH v2] Compress lines for immediate return
Le 03/04/2020 à 14:53, Stefan Hajnoczi a écrit :
> On Wed, Apr 01, 2020 at 10:23:14PM +0530, Simran Singhal wrote:
>> Compress two lines into a single line if immediate return statement is found.
>>
>> It also remove variables progress, val, data, ret and sock
>> as they are no longer needed.
>>
>> Remove space between function "mixer_load" and '(' to fix the
>> checkpatch.pl error:-
>> ERROR: space prohibited between function name and open parenthesis '('
>>
>> Done using following coccinelle script:
>> @@
>> local idexpression ret;
>> expression e;
>> @@
>>
>> -ret =
>> +return
>> e;
>> -return ret;
>>
>> Signed-off-by: Simran Singhal
>> ---
>> Changes in v2:
>> -Added coccinelle script wrote for changes in commit message.
>>
>> block/file-posix.c | 3 +--
>> block/nfs.c | 3 +--
>> block/nvme.c| 4 +---
>> block/vhdx.c| 3 +--
>> hw/audio/ac97.c | 4 +---
>> hw/audio/adlib.c| 5 +
>> hw/display/cirrus_vga.c | 4 +---
>> migration/ram.c | 4 +---
>> ui/gtk.c| 3 +--
>> util/qemu-sockets.c | 5 +
>> 10 files changed, 10 insertions(+), 28 deletions(-)
>
> Reviewed-by: Stefan Hajnoczi
>
Applied to my trivial-patches branch.
Thanks,
Laurent
signature.asc
Description: OpenPGP digital signature
[PATCH] hw/char/parallel: Convert reset handler to DeviceReset
As TYPE_ISA_PARALLEL inherits from TYPE_ISA_DEVICE, it also
inherits from the DEVICE methods. Convert its reset handler
into a proper DeviceReset method.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/parallel.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 8dd67d1375..f68a96da8e 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -523,6 +523,13 @@ static int parallel_can_receive(void *opaque)
return 1;
}
+static void parallel_isa_reset(DeviceState *dev)
+{
+ISAParallelState *isa = ISA_PARALLEL(dev);
+
+parallel_reset(&isa->state);
+}
+
static void parallel_isa_realizefn(DeviceState *dev, Error **errp)
{
static int index;
@@ -552,7 +559,6 @@ static void parallel_isa_realizefn(DeviceState *dev, Error
**errp)
base = isa->iobase;
isa_init_irq(isadev, &s->irq, isa->isairq);
-qemu_register_reset(parallel_reset, s);
qemu_chr_fe_set_handlers(&s->chr, parallel_can_receive, NULL,
NULL, NULL, s, NULL, true);
@@ -625,6 +631,7 @@ static void parallel_isa_class_initfn(ObjectClass *klass,
void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+dc->reset = parallel_isa_reset;
dc->realize = parallel_isa_realizefn;
dc->vmsd = &vmstate_parallel_isa;
device_class_set_props(dc, parallel_isa_properties);
--
2.21.3
Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
On Mon, 4 May 2020 at 10:46, Michael S. Tsirkin wrote: > > On Mon, May 04, 2020 at 10:29:18AM +0100, Peter Maydell wrote: > > I notice that checkpatch complains a lot about > > > > ERROR: Do not add expected files together with tests, follow > > instructions in tests/qtest/bios-tables-test.c: both > > tests/qtest/bios-tables-test-allowed-diff.h and > > tests/qtest/bios-tables-test.c found > > > > Does that need fixing, or is the checkpatch test wrong? > Hmm I don't see a patch in this series that changes both > tests/qtest/bios-tables-test-allowed-diff.h and > tests/qtest/bios-tables-test.c. Do you? No, but that's not the pair of files that checkpatch is complaining about. It warns about: patch 1: tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c patch 2: tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.h tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_piix.c tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_q35.c tests/qtest/bios-tables-test-allowed-diff.h and include/hw/mem/nvdimm.h patch 3: tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/Kconfig tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt-acpi-build.c tests/qtest/bios-tables-test-allowed-diff.h and hw/arm/virt.c tests/qtest/bios-tables-test-allowed-diff.h and hw/mem/Kconfig tests/qtest/bios-tables-test-allowed-diff.h and include/hw/arm/virt.h etc, and the patches really do touch the pairs of files listed. (It also seems to warn about each file combination multiple times.) Are these false positives (if so, we should change the checkpatch test, since it's clearly misfiring a lot) or problems with the patches? thanks -- PMM
Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
On Tue, Apr 21, 2020 at 08:12:57AM -0700, no-re...@patchew.org wrote: > Patchew URL: > https://patchew.org/QEMU/20200421125934.14952-1-shameerali.kolothum.th...@huawei.com/ > > > > Hi, > > This series seems to have some coding style problems. See output below for > more information: > > Subject: [PATCH v4 0/7] ARM virt: Add NVDIMM support > Message-id: 20200421125934.14952-1-shameerali.kolothum.th...@huawei.com > Type: series > > === 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 === > > Switched to a new branch 'test' > c4f3ad1 tests/acpi: add expected tables for bios-tables-test > 5b55be7 bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt > f0c9bb6 tests: Update ACPI tables list for upcoming arm/virt test changes > c2dd728 hw/arm/virt: Add nvdimm hotplug support > f7dad84 hw/arm/virt: Add nvdimm hot-plug infrastructure > 5554e78 nvdimm: Use configurable ACPI IO base and size > 8058b6f hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length > > === OUTPUT BEGIN === > 1/7 Checking commit 8058b6f6d753 (hw/acpi/nvdimm: Fix for NVDIMM incorrect > DSM output buffer length) > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found > > total: 2 errors, 0 warnings, 59 lines checked OK so this is a false positive in the script. I will fix it. > Patch 1/7 has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > 2/7 Checking commit 5554e78b18ea (nvdimm: Use configurable ACPI IO base and > size) > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found This beats me. Where did we get tests/qtest/bios-tables-test-allowed-diff.h from? It's a different patch, isn't it? > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.c found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.h found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/acpi-build.h found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_piix.c found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_piix.c found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_q35.c found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and hw/i386/pc_q35.c found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and include/hw/mem/nvdimm.h found > > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-test.c: both > tests/qtest/bios-tables-test-allowed-diff.h and include/hw/mem/nvdimm.h found > > total: 12 errors, 0 warnings, 158 lines checked > > Patch 2/7 has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > 3/7 Checking commit f7dad84068ce (hw/arm/virt: Add nvdimm hot-plug > infrastructure) > ERROR: Do not add expected files together with tests, follow instructions in > tests/qtest/bios-tables-t
Re: [PATCH v2 3/6] qemu-img: Add bitmap sub-command
On 30.04.20 17:21, Eric Blake wrote:
> On 4/30/20 9:55 AM, Max Reitz wrote:
>> On 21.04.20 23:20, Eric Blake wrote:
>>> Include actions for --add, --remove, --clear, --enable, --disable, and
>>> --merge (note that --clear is a bit of fluff, because the same can be
>>> accomplished by removing a bitmap and then adding a new one in its
>>> place,
>>
>> Well, ideally, all of qemu-img is just fluff because “the same can be
>> accomplished by launching qemu and issuing the equivalent QMP
>> commands”. :)
>>
>>> but it matches what QMP commands exist). Listing is omitted,
>>> because it does not require a bitmap name and because it was already
>>> possible with 'qemu-img info'.
>>
>> Fair enough, although it can be said that qemu-img info’s output is
>> qcow2-specific. It might be nice to have some definitely
>> format-independent output. (But we don’t have persistent bitmaps in
>> anything but qcow2 yet (or do we in NBD?), so I don’t expect anyone to
>> care much.)
>
> We can add a list subcommand later if it is still desired. I agree that
> a tabular format:
>
> name enabled granularity
> bitmap1 false 65536
> bitmap2 true 512
>
> in isolation is easier to read than:
>
> bitmaps:
> [0]:
> flags:
> name: bitmap1
> granularity: 65536
> [1]:
> flags:
> [0]: auto
> name: bitmap2
> granularity: 512
>
> embedded inside even more information.
>
>>
>>> Merge can work either from another
>>> bitmap in the same image, or from a bitmap in a distinct image.
>>>
>>> While this supports --image-opts for the file being modified, I did
>>> not think it worth the extra complexity to support that for the source
>>> file in a cross-file bitmap merge. Likewise, I chose to have --merge
>>> only take a single source rather than following the QMP support for
>>> multiple merges in one go; in part to simplify the command line, and
>>> in part because an offline image can achieve the same effect by
>>> multiple qemu-img bitmap --merge calls. We can enhance that if needed
>>> in the future (the same way that 'qemu-img convert' has a mode that
>>> concatenates multiple sources into one destination).
>>>
>>> Upcoming patches will add iotest coverage of these commands while
>>> also testing other features.
>>>
>>> Signed-off-by: Eric Blake
>>> ---
>>> docs/tools/qemu-img.rst | 24 +
>>> qemu-img.c | 198
>>> qemu-img-cmds.hx | 7 ++
>>> 3 files changed, 229 insertions(+)
>>>
>>> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
>>> index 7d08c48d308f..4f3b0e2c9ace 100644
>>> --- a/docs/tools/qemu-img.rst
>>> +++ b/docs/tools/qemu-img.rst
>>> @@ -281,6 +281,30 @@ Command description:
>>> For write tests, by default a buffer filled with zeros is
>>> written. This can be
>>> overridden with a pattern byte specified by *PATTERN*.
>>>
>>> +.. option:: bitmap {--add [-g GRANULARITY] [--disabled] | --remove |
>>> --clear | --enable | --disable | --merge SOURCE_BITMAP [-b
>>> SOURCE_FILE [-F SOURCE_FMT]]} [--object OBJECTDEF] [--image-opts] [-f
>>> FMT] FILENAME BITMAP
>>
>> So I can do multiple operations in one roll, but they all use the same
>> BITMAP? Sounds a bit weird. It actually took me a while to understands
>> this, because I thought for sure that each command would take a bitmap
>> name. (And was ready to complain that it looked like they don’t, but,
>> well, that’s because they don’t.)
>
> All of the operations take one bitmap name (the final BITMAP).
> Additionally, the --merge operation takes a second bitmap name
> (SOURCE_BITMAP). None of the other operations need a second bitmap
> name, so only --merge requires an option argument. As written, the { a
> | b | c } implies that operations are mutually exclusive: you can only
> request one operation per qemu-img invocation.
Well, as I found out later it’s supposed to imply that. I always expect
{} to mean repetition.
>> Although I suppose some practical example like
>>
>> $ qemu-img bitmap --add --merge sbmap --disable foo.qcow2 nbmap
>>
>> does make sense.[1]
>>
>>
>> Would
>>
>> $ qemu-img bitmap --add nbmap --merge nbmap sbmap --enable nbmap \
>> foo.qcow2
>>
>> make more sense?
>
> That would be more transactional, and more effort to implement.
As a user, I wouldn’t expect it to be a transaction, but just executed
in order.
> My argument is that you should instead write:
>
> $ qemu-img bitmap --add foo.qcow2 nbmap
> $ qemu-img bitmap --merge sbmap foo.qcow2 nbmap
> $ qemu-img bitmap --enable foo.qcow2 nbmap
>
> where I only have to implement one operation per qemu-img.
I don’t know about the “have to”, because from an algorithm standpoint,
doing so would be trivial. (That is, collecting the operations into a
list, along with their specific arguments, and then execute the list
Re: An first try to improve PPC float simulation, not even compiled. Just ask question.
罗勇刚(Yonggang Luo) writes:
> On Mon, May 4, 2020 at 7:40 AM BALATON Zoltan wrote:
>
>> Hello,
>>
>> On Mon, 4 May 2020, 罗勇刚(Yonggang Luo) wrote:
>> > Hello Richard, Can you have a look at the following patch, and was that
>> are
>> > the right direction?
>>
>> Formatting of the patch is broken by your mailer, try sending it with
>> something that does not change it otherwise it's a bit hard to read.
>>
>> Richard suggested to add an assert to check the fp_status is correctly
>> cleared in place of helper_reset_fpstatus first for debugging so you could
>> change the helper accordingly before deleting it and run a few tests to
>> verify it still works. You'll need get some tests and benchmarks working
>> to be able to verify your changes that's why I've said that would be step
>> 0. If you checked that it still produces the same results and the assert
>> does not trigger then you can remove the helper.
>>
> That's what I need help,
> 1. How to write a assert to replace helper_reset_fpstatus .
> just directly assert? or something else
> 2. a few tests to run
> How to running these tests, and where are these tests.
All the softfloat testing is currently done in tests/fp. I think you
need to make a new version of fp-test.c (fp-test-native.c?) that instead
of comparing the qemu softfloat functions with TestFloats runs the
native functions (as emitted by the compiler). That should pass when run
on real hardware and we can compare when run under emulation.
> Do I need to add new tests? Where to start
> 3. Benchmarks
fp-bench is the raw benchmarking app which again can be built for a
guest architecture to measure throughput under emulation.
> Same as 2
>
>>
>> Regards,
>> BALATON Zoltan
>>
>> > From b4d6ca1d6376fab1f1be06eb472e10b908887c2b Mon Sep 17 00:00:00 2001
>> > From: Yonggang Luo
>> > Date: Sat, 2 May 2020 05:59:25 +0800
>> > Subject: [PATCH] [ppc fp] Step 1. Rearrange the fp helpers to eliminate
>> > helper_reset_fpstatus(). I've mentioned this before, that it's possible
>> to
>> > leave the steady-state of env->fp_status.exception_flags == 0, so there's
>> > no
>> > need for a separate function call. I suspect this is worth a decent
>> > speedup
>> > by itself.
>> >
>> > ---
>> > target/ppc/fpu_helper.c| 53 ++
>> > target/ppc/helper.h| 1 -
>> > target/ppc/translate/fp-impl.inc.c | 23 -
>> > 3 files changed, 3 insertions(+), 74 deletions(-)
>> >
>> > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
>> > index d9a8773ee1..4fc5a7ff1c 100644
>> > --- a/target/ppc/fpu_helper.c
>> > +++ b/target/ppc/fpu_helper.c
>> > @@ -821,6 +821,9 @@ static void do_float_check_status(CPUPPCState *env,
>> > uintptr_t raddr)
>> >env->error_code, raddr);
>> > }
>> > }
>> > +if (status) {
>> > +set_float_exception_flags(0, &env->fp_status);
>> > +}
>> > }
>> >
>> > void helper_float_check_status(CPUPPCState *env)
>> > @@ -828,11 +831,6 @@ void helper_float_check_status(CPUPPCState *env)
>> > do_float_check_status(env, GETPC());
>> > }
>> >
>> > -void helper_reset_fpstatus(CPUPPCState *env)
>> > -{
>> > -set_float_exception_flags(0, &env->fp_status);
>> > -}
>> > -
>> > static void float_invalid_op_addsub(CPUPPCState *env, bool set_fpcc,
>> > uintptr_t retaddr, int classes)
>> > {
>> > @@ -2110,9 +2108,6 @@ void helper_##name(CPUPPCState *env, ppc_vsr_t *xt,
>> > \
>> > {
>> > \
>> > ppc_vsr_t t = *xt;
>> > \
>> > int i;
>> > \
>> > -
>> > \
>> > -helper_reset_fpstatus(env);
>> > \
>> > -
>> > \
>> > for (i = 0; i < nels; i++) {
>> > \
>> > float_status tstat = env->fp_status;
>> > \
>> > set_float_exception_flags(0, &tstat);
>> > \
>> > @@ -2152,8 +2147,6 @@ void helper_xsaddqp(CPUPPCState *env, uint32_t
>> opcode,
>> > ppc_vsr_t t = *xt;
>> > float_status tstat;
>> >
>> > -helper_reset_fpstatus(env);
>> > -
>> > tstat = env->fp_status;
>> > if (unlikely(Rc(opcode) != 0)) {
>> > tstat.float_rounding_mode = float_round_to_odd;
>> > @@ -2189,9 +2182,6 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt,
>> > \
>> > {
>> > \
>> > ppc_vsr_t t = *xt;
>> > \
>> > int i;
>> > \
>> > -
>> > \
>> > -helper_reset_fpstatus(env);
>> > \
>> > -
>> > \
>> > for (i = 0; i < nels; i++) {
>> > \
>> > float_status tstat = env->fp_status;
>> > \
>> > set_float_exception_flags(0, &tstat);
>> > \
>> > @@ -2228,13 +2218,11 @@ void helper_xsmulqp(CPUPPCState *env, uint32_t
>> > opcode,
>> > ppc_vsr_t t = *xt;
>> > float_status tstat;
>> >
>> > -helper_reset_fpstatus(env);
>> > tstat = env->fp_status;
>> > if (unlikely(Rc(opcode) != 0)) {
>> > tstat.float_rounding_mode = float_round_to_odd;
>> > }
>> >
>> > -set_float_exception_flags(0, &tstat);
>> > t.
Re: [PATCH v3 0/9] misc: Trivial static code analyzer fixes
Le 22/04/2020 à 15:31, Philippe Mathieu-Daudé a écrit : > Fix trivial warnings reported by the Clang static code analyzer. > > Only patch #2 'blockdev: Remove dead assignment' misses review. > > The official Clang static code analyzer documentation is on: > https://clang-analyzer.llvm.org/ > > On Fedora I simply used it as: > > $ sudo dnf install clang-analyzer > $ ../configure > $ scan-build make > > Since v2: > - Based on lvivier/trivial-patches-for-5.1 > - Removed dup patches from Kuhn Chenqun > Since v1: > - Addressed Markus/Zoltan/Aleksandar review comments > > Philippe Mathieu-Daudé (9): > block: Avoid dead assignment > blockdev: Remove dead assignment > hw/i2c/pm_smbus: Remove dead assignment > hw/input/adb-kbd: Remove dead assignment > hw/ide/sii3112: Remove dead assignment > hw/isa/i82378: Remove dead assignment > hw/gpio/aspeed_gpio: Remove dead assignment > hw/timer/stm32f2xx_timer: Remove dead assignment > hw/timer/pxa2xx_timer: Add assertion to silent static analyzer warning > > block.c| 2 +- > blockdev.c | 2 +- > hw/gpio/aspeed_gpio.c | 2 +- > hw/i2c/pm_smbus.c | 1 - > hw/ide/sii3112.c | 5 +++-- > hw/input/adb-kbd.c | 6 +- > hw/isa/i82378.c| 8 > hw/timer/pxa2xx_timer.c| 1 + > hw/timer/stm32f2xx_timer.c | 1 - > 9 files changed, 12 insertions(+), 16 deletions(-) > Applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH v4 0/7] ARM virt: Add NVDIMM support
On Mon, 4 May 2020 at 10:57, Michael S. Tsirkin wrote: > > ./scripts/checkpatch.pl --mailback base.. > > 2/7 Checking commit 5554e78b18ea (nvdimm: Use configurable ACPI IO base and > > size) > > ERROR: Do not add expected files together with tests, follow instructions > > in tests/qtest/bios-tables-test.c: both > > tests/qtest/bios-tables-test-allowed-diff.h and hw/acpi/nvdimm.c found > > This beats me. Where did we get > tests/qtest/bios-tables-test-allowed-diff.h from? > It's a different patch, isn't it? Ah, this is a bug in the checkfilename() function -- it uses some globals $acpi_testexpected and $acpi_nontestexpected, but there is no code to reset these when checkpatch starts checking a new patch. So if you only check one patch in a checkpatch run (eg by just passing it a single patch file) then it will work, but if a single checkpatch execution is checking several commits (eg in the way patchew runs it to check the whole series of git commits at once, or if you pass it several patch files) then it will give wrong results for the second and later patches. I think the variables need to be reset at the top of 'sub process()'. thanks -- PMM
[PATCH v2 1/3] target: Remove unnecessary CPU() cast
The CPU() macro is defined as:
#define CPU(obj) ((CPUState *)(obj))
which expands to:
((CPUState *)object_dynamic_cast_assert((Object *)(obj), (name),
__FILE__, __LINE__, __func__))
This assertion can only fail when @obj points to something other
than its stated type, i.e. when we're in undefined behavior country.
Remove the unnecessary CPU() casts when we already know the pointer
is of CPUState type.
Patch created mechanically using spatch with this script:
@@
typedef CPUState;
CPUState *s;
@@
- CPU(s)
+ s
Acked-by: David Gibson
Reviewed-by: Cédric Le Goater
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
v2: Reword (Markus)
---
target/ppc/mmu_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 86c667b094..8972714775 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -1820,7 +1820,7 @@ static inline void do_invalidate_BAT(CPUPPCState *env,
target_ulong BATu,
if (((end - base) >> TARGET_PAGE_BITS) > 1024) {
/* Flushing 1024 4K pages is slower than a complete flush */
LOG_BATS("Flush all BATs\n");
-tlb_flush(CPU(cs));
+tlb_flush(cs);
LOG_BATS("Flush done\n");
return;
}
--
2.21.3
[PATCH v2 2/3] various: Remove unnecessary OBJECT() cast
The OBJECT() macro is defined as:
#define OBJECT(obj) ((Object *)(obj))
which expands to:
((Object *)object_dynamic_cast_assert((Object *)(obj), (name),
__FILE__, __LINE__, __func__))
This assertion can only fail when @obj points to something other
than its stated type, i.e. when we're in undefined behavior country.
Remove the unnecessary OBJECT() casts when we already know the
pointer is of Object type.
Patch created mechanically using spatch with this script:
@@
typedef Object;
Object *o;
@@
- OBJECT(o)
+ o
Acked-by: Cornelia Huck
Acked-by: Corey Minyard
Acked-by: John Snow
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
v2: Reword (Markus)
---
hw/core/bus.c | 2 +-
hw/ide/ahci-allwinner.c | 2 +-
hw/ipmi/smbus_ipmi.c| 2 +-
hw/microblaze/petalogix_ml605_mmu.c | 8
hw/s390x/sclp.c | 2 +-
monitor/misc.c | 3 +--
qom/object.c| 4 ++--
7 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 3dc0a825f0..4ea5870de8 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -25,7 +25,7 @@
void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
{
-object_property_set_link(OBJECT(bus), OBJECT(handler),
+object_property_set_link(OBJECT(bus), handler,
QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
}
diff --git a/hw/ide/ahci-allwinner.c b/hw/ide/ahci-allwinner.c
index bb8393d2b6..8536b9eb5a 100644
--- a/hw/ide/ahci-allwinner.c
+++ b/hw/ide/ahci-allwinner.c
@@ -90,7 +90,7 @@ static void allwinner_ahci_init(Object *obj)
SysbusAHCIState *s = SYSBUS_AHCI(obj);
AllwinnerAHCIState *a = ALLWINNER_AHCI(obj);
-memory_region_init_io(&a->mmio, OBJECT(obj), &allwinner_ahci_mem_ops, a,
+memory_region_init_io(&a->mmio, obj, &allwinner_ahci_mem_ops, a,
"allwinner-ahci", ALLWINNER_AHCI_MMIO_SIZE);
memory_region_add_subregion(&s->ahci.mem, ALLWINNER_AHCI_MMIO_OFF,
&a->mmio);
diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
index 2a9470d9df..f1a0148755 100644
--- a/hw/ipmi/smbus_ipmi.c
+++ b/hw/ipmi/smbus_ipmi.c
@@ -329,7 +329,7 @@ static void smbus_ipmi_init(Object *obj)
{
SMBusIPMIDevice *sid = SMBUS_IPMI(obj);
-ipmi_bmc_find_and_link(OBJECT(obj), (Object **) &sid->bmc);
+ipmi_bmc_find_and_link(obj, (Object **) &sid->bmc);
}
static void smbus_ipmi_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo *info)
diff --git a/hw/microblaze/petalogix_ml605_mmu.c
b/hw/microblaze/petalogix_ml605_mmu.c
index 0a2640c40b..52dcea9abd 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -150,9 +150,9 @@ petalogix_ml605_init(MachineState *machine)
qdev_set_nic_properties(eth0, &nd_table[0]);
qdev_prop_set_uint32(eth0, "rxmem", 0x1000);
qdev_prop_set_uint32(eth0, "txmem", 0x1000);
-object_property_set_link(OBJECT(eth0), OBJECT(ds),
+object_property_set_link(OBJECT(eth0), ds,
"axistream-connected", &error_abort);
-object_property_set_link(OBJECT(eth0), OBJECT(cs),
+object_property_set_link(OBJECT(eth0), cs,
"axistream-control-connected", &error_abort);
qdev_init_nofail(eth0);
sysbus_mmio_map(SYS_BUS_DEVICE(eth0), 0, AXIENET_BASEADDR);
@@ -163,9 +163,9 @@ petalogix_ml605_init(MachineState *machine)
cs = object_property_get_link(OBJECT(eth0),
"axistream-control-connected-target", NULL);
qdev_prop_set_uint32(dma, "freqhz", 100 * 100);
-object_property_set_link(OBJECT(dma), OBJECT(ds),
+object_property_set_link(OBJECT(dma), ds,
"axistream-connected", &error_abort);
-object_property_set_link(OBJECT(dma), OBJECT(cs),
+object_property_set_link(OBJECT(dma), cs,
"axistream-control-connected", &error_abort);
qdev_init_nofail(dma);
sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, AXIDMA_BASEADDR);
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index ede056b3ef..4132286db7 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -322,7 +322,7 @@ void s390_sclp_init(void)
object_property_add_child(qdev_get_machine(), TYPE_SCLP, new,
NULL);
-object_unref(OBJECT(new));
+object_unref(new);
qdev_init_nofail(DEVICE(new));
}
diff --git a/monitor/misc.c b/monitor/misc.c
index 6c45fa490f..57af5fa5a4 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1839,8 +1839,7 @@ void object_add_completion(ReadLineState *rs, int
nb_args, const char *str)
static int qdev_add_hotpluggable_device(Object *obj, void *opaque)
{
GSList **list = opaque;
-DeviceState *dev = (DeviceState *)object_dynamic_cast(OBJECT(obj),
-
[PATCH v2 0/3] various: Remove unnecessary casts
Remove unnecessary casts using coccinelle scripts. The CPU()/OBJECT() patches don't introduce logical change, The DEVICE() one removes various OBJECT_CHECK() calls. Since v2: - Add A-b/R-b tags - Reword description (Markus) Philippe Mathieu-Daudé (3): target: Remove unnecessary CPU() cast various: Remove unnecessary OBJECT() cast hw: Remove unnecessary DEVICE() cast hw/core/bus.c | 2 +- hw/display/artist.c | 2 +- hw/display/cg3.c| 2 +- hw/display/sm501.c | 2 +- hw/display/tcx.c| 4 ++-- hw/display/vga-isa.c| 2 +- hw/i2c/imx_i2c.c| 2 +- hw/i2c/mpc_i2c.c| 2 +- hw/ide/ahci-allwinner.c | 2 +- hw/ide/piix.c | 2 +- hw/ipmi/smbus_ipmi.c| 2 +- hw/microblaze/petalogix_ml605_mmu.c | 8 hw/misc/macio/pmu.c | 2 +- hw/net/ftgmac100.c | 3 +-- hw/net/imx_fec.c| 2 +- hw/nubus/nubus-device.c | 2 +- hw/pci-host/bonito.c| 2 +- hw/ppc/spapr.c | 2 +- hw/s390x/sclp.c | 2 +- hw/sh4/sh_pci.c | 2 +- hw/xen/xen-legacy-backend.c | 2 +- monitor/misc.c | 3 +-- qom/object.c| 4 ++-- target/ppc/mmu_helper.c | 2 +- 24 files changed, 29 insertions(+), 31 deletions(-) -- 2.21.3
[PATCH v2 3/3] hw: Remove unnecessary DEVICE() cast
The DEVICE() macro is defined as:
#define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)
which expands to:
((DeviceState *)object_dynamic_cast_assert((Object *)(obj), (name),
__FILE__, __LINE__,
__func__))
This assertion can only fail when @obj points to something other
than its stated type, i.e. when we're in undefined behavior country.
Remove the unnecessary DEVICE() casts when we already know the
pointer is of DeviceState type.
Patch created mechanically using spatch with this script:
@@
typedef DeviceState;
DeviceState *s;
@@
- DEVICE(s)
+ s
Acked-by: David Gibson
Acked-by: Paul Durrant
Reviewed-by: Markus Armbruster
Reviewed-by: Cédric Le Goater
Acked-by: John Snow
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
v2: Reword (Markus)
---
hw/display/artist.c | 2 +-
hw/display/cg3.c| 2 +-
hw/display/sm501.c | 2 +-
hw/display/tcx.c| 4 ++--
hw/display/vga-isa.c| 2 +-
hw/i2c/imx_i2c.c| 2 +-
hw/i2c/mpc_i2c.c| 2 +-
hw/ide/piix.c | 2 +-
hw/misc/macio/pmu.c | 2 +-
hw/net/ftgmac100.c | 3 +--
hw/net/imx_fec.c| 2 +-
hw/nubus/nubus-device.c | 2 +-
hw/pci-host/bonito.c| 2 +-
hw/ppc/spapr.c | 2 +-
hw/sh4/sh_pci.c | 2 +-
hw/xen/xen-legacy-backend.c | 2 +-
16 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 753dbb9a77..7e2a4556bd 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -1353,7 +1353,7 @@ static void artist_realizefn(DeviceState *dev, Error
**errp)
s->cursor_height = 32;
s->cursor_width = 32;
-s->con = graphic_console_init(DEVICE(dev), 0, &artist_ops, s);
+s->con = graphic_console_init(dev, 0, &artist_ops, s);
qemu_console_resize(s->con, s->width, s->height);
}
diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index a1ede10394..f7f1c199ce 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -321,7 +321,7 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
sysbus_init_irq(sbd, &s->irq);
-s->con = graphic_console_init(DEVICE(dev), 0, &cg3_ops, s);
+s->con = graphic_console_init(dev, 0, &cg3_ops, s);
qemu_console_resize(s->con, s->width, s->height);
}
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index de0ab9d977..2a564889bd 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1839,7 +1839,7 @@ static void sm501_init(SM501State *s, DeviceState *dev,
&s->twoD_engine_region);
/* create qemu graphic console */
-s->con = graphic_console_init(DEVICE(dev), 0, &sm501_ops, s);
+s->con = graphic_console_init(dev, 0, &sm501_ops, s);
}
static const VMStateDescription vmstate_sm501_state = {
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 76de16e8ea..1fb45b1aab 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -868,9 +868,9 @@ static void tcx_realizefn(DeviceState *dev, Error **errp)
sysbus_init_irq(sbd, &s->irq);
if (s->depth == 8) {
-s->con = graphic_console_init(DEVICE(dev), 0, &tcx_ops, s);
+s->con = graphic_console_init(dev, 0, &tcx_ops, s);
} else {
-s->con = graphic_console_init(DEVICE(dev), 0, &tcx24_ops, s);
+s->con = graphic_console_init(dev, 0, &tcx24_ops, s);
}
s->thcmisc = 0;
diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
index 0633ed382c..3aaeeeca1e 100644
--- a/hw/display/vga-isa.c
+++ b/hw/display/vga-isa.c
@@ -74,7 +74,7 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
0x000a,
vga_io_memory, 1);
memory_region_set_coalescing(vga_io_memory);
-s->con = graphic_console_init(DEVICE(dev), 0, s->hw_ops, s);
+s->con = graphic_console_init(dev, 0, s->hw_ops, s);
memory_region_add_subregion(isa_address_space(isadev),
VBE_DISPI_LFB_PHYSICAL_ADDRESS,
diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
index 30b9aea247..2e02e1c4fa 100644
--- a/hw/i2c/imx_i2c.c
+++ b/hw/i2c/imx_i2c.c
@@ -305,7 +305,7 @@ static void imx_i2c_realize(DeviceState *dev, Error **errp)
IMX_I2C_MEM_SIZE);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
-s->bus = i2c_init_bus(DEVICE(dev), NULL);
+s->bus = i2c_init_bus(dev, NULL);
}
static void imx_i2c_class_init(ObjectClass *klass, void *data)
diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c
index 0aa1be3ce7..9a724f3a3e 100644
--- a/hw/i2c/mpc_i2c.c
+++ b/hw/i2c/mpc_i2c.c
@@ -332,7 +332,7 @@ static void mpc_i2c_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&i2c->iomem, OBJECT(i2c), &i2c_ops, i2c,
Re: [PATCH] hw/usb: Make "hcd-ehci.h" header public
On 5/4/20 10:48 AM, BALATON Zoltan wrote: On Mon, 4 May 2020, Philippe Mathieu-Daudé wrote: As target-specific code use this header, move it to the publicly accessible include/ folder. $ git grep hw/usb/hcd-ehci.h hw/arm/allwinner-h3.c:31:#include "hw/usb/hcd-ehci.h" hw/arm/exynos4210.c:38:#include "hw/usb/hcd-ehci.h" hw/ppc/sam460ex.c:38:#include "hw/usb/hcd-ehci.h" include/hw/arm/allwinner-a10.h:13:#include "hw/usb/hcd-ehci.h" include/hw/arm/aspeed_soc.h:29:#include "hw/usb/hcd-ehci.h" All of these only need either the type #define or EHCISysBusState so splitting only those off to a public header should be enough and better than making public all of ehci's internal header. Ah you mean forward-declare EHCISysBusState in "qemu/typedefs.h", OK. Regards, BALATON Zoltan
Re: [PATCH v1 00/11] hw/arm: versal: Add SD and the RTC
On Mon, 27 Apr 2020 at 19:16, Edgar E. Iglesias wrote: > > From: "Edgar E. Iglesias" > > This series starts with some basic cleaning, continues with embedding > devices into the Versal SoC (as suggested by Peter in another review). > We then connect SD and the RTC to the Versal SoC and hook it all up > into the Versal Virt board. Applied to target-arm.next, thanks. -- PMM
[PATCH] xen: fix build without pci passthrough
has_igd_gfx_passthru is only available when QEMU is built with
CONFIG_XEN_PCI_PASSTHROUGH, and hence shouldn't be used in common
code without checking if it's available.
Fixes: 46472d82322d0 ('xen: convert "-machine igd-passthru" to an accelerator
property')
Signed-off-by: Roger Pau Monné
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: xen-de...@lists.xenproject.org
---
hw/xen/xen-common.c | 4
hw/xen/xen_pt.h | 7 +++
2 files changed, 11 insertions(+)
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index a15070f7f6..c800862419 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -127,6 +127,7 @@ static void xen_change_state_handler(void *opaque, int
running,
}
}
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
static bool xen_get_igd_gfx_passthru(Object *obj, Error **errp)
{
return has_igd_gfx_passthru;
@@ -136,6 +137,7 @@ static void xen_set_igd_gfx_passthru(Object *obj, bool
value, Error **errp)
{
has_igd_gfx_passthru = value;
}
+#endif
static void xen_setup_post(MachineState *ms, AccelState *accel)
{
@@ -197,11 +199,13 @@ static void xen_accel_class_init(ObjectClass *oc, void
*data)
compat_props_add(ac->compat_props, compat, G_N_ELEMENTS(compat));
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
object_class_property_add_bool(oc, "igd-passthru",
xen_get_igd_gfx_passthru, xen_set_igd_gfx_passthru,
&error_abort);
object_class_property_set_description(oc, "igd-passthru",
"Set on/off to enable/disable igd passthrou", &error_abort);
+#endif
}
#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 179775db7b..660dd8a008 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -1,6 +1,7 @@
#ifndef XEN_PT_H
#define XEN_PT_H
+#include "qemu/osdep.h"
#include "hw/xen/xen_common.h"
#include "hw/pci/pci.h"
#include "xen-host-pci-device.h"
@@ -322,7 +323,13 @@ extern void *pci_assign_dev_load_option_rom(PCIDevice *dev,
unsigned int domain,
unsigned int bus, unsigned int
slot,
unsigned int function);
+
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
extern bool has_igd_gfx_passthru;
+#else
+# define has_igd_gfx_passthru false
+#endif
+
static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
{
return (has_igd_gfx_passthru
--
2.26.2
Re: [PATCH v3 00/14] LUKS: encryption slot management using amend interface
On Sun, May 03, 2020 at 09:43:10PM +0300, Maxim Levitsky wrote: > Hi! > Here is the updated series of my patches, incorporating all the feedback I > received. > > This implements the API interface that we agreed upon except that I merged the > LUKSKeyslotActive/LUKSKeyslotInactive union into a struct because otherwise > I need nested unions which are not supported currently by QAPI parser. > This didn't change the API and thus once support for nested unions is there, > it can always be implemented in backward compatible way. > > I hope that this series will finally be considered for merging, since I am > somewhat running > out of time to finish this task. > > Patches are strictly divided by topic to 3 groups, and each group depends on > former groups. > > * Patches 1,2 implement qcrypto generic amend interface, including definition > of structs used in crypto.json and implement this in luks crypto driver > Nothing is exposed to the user at this stage > > * Patches 3-9 use the code from patches 1,2 to implement qemu-img amend based > encryption slot management > for luks and for qcow2, and add a bunch of iotests to cover that. > > * Patches 10-13 add x-blockdev-amend (I'll drop the -x prefix if you like), > and wire it > to luks and qcow2 driver to implement qmp based encryption slot management > also using > the code from patches 1,2, and also add a bunch of iotests to cover this. > > Tested with -raw,-qcow2 and -luks iotests and 'make check' > > V3: rebased, addressed most of the review feedback. > For now I kept the slot bitmap code since I am not sure that replacing it > will be better. I'm still of the opinion that the bitmaps code should be replaced. With this current design we are iterating over the slot of keys slots 4 times, doing different checks/logic in each iteration. This is really not nice for understanding how the code works. IMHO we should be iterating at most twice - once to validate the requested configuration, and once to apply the requested configuration. Even if there si duplication of logic in between the delete/add key codepaths, I think it will be better as the logic for each operation will be in one place. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] hw/usb: Make "hcd-ehci.h" header public
On 5/4/20 12:12 PM, Philippe Mathieu-Daudé wrote: On 5/4/20 10:48 AM, BALATON Zoltan wrote: On Mon, 4 May 2020, Philippe Mathieu-Daudé wrote: As target-specific code use this header, move it to the publicly accessible include/ folder. $ git grep hw/usb/hcd-ehci.h hw/arm/allwinner-h3.c:31:#include "hw/usb/hcd-ehci.h" hw/arm/exynos4210.c:38:#include "hw/usb/hcd-ehci.h" hw/ppc/sam460ex.c:38:#include "hw/usb/hcd-ehci.h" include/hw/arm/allwinner-a10.h:13:#include "hw/usb/hcd-ehci.h" include/hw/arm/aspeed_soc.h:29:#include "hw/usb/hcd-ehci.h" All of these only need either the type #define or EHCISysBusState so splitting only those off to a public header should be enough and better than making public all of ehci's internal header. Ah you mean forward-declare EHCISysBusState in "qemu/typedefs.h", OK. It won't work because AspeedSoCState and AwA10State use the full structure (not a pointer to it). Regards, BALATON Zoltan
Re: [PATCH v6 3/5] 9pfs: add new function v9fs_co_readdir_many()
On Montag, 4. Mai 2020 11:18:34 CEST Greg Kurz wrote:
> > > > > > +memcpy(e->dent, dent, sizeof(struct dirent));
> > > > > > +
> > > > > > +/* perform a full stat() for directory entry if requested
> > > > > > by
> > > > > > caller */ +if (dostat) {
> > > > > > +err = s->ops->name_to_path(
> > > > > > +&s->ctx, &fidp->path, dent->d_name, &path
> > > > > > +);
> > > > > > +if (err < 0) {
> > > > > >
> > > > > > err = -errno;
> > > > > >
> > > > > > -} else {
> > > > > > -*dent = entry;
> > > > > > -err = 0;
> > > > > > +break;
> > > > >
> > > > > ... but we're erroring out there and it seems that we're leaking
> > > > > all the entries that have been allocated so far.
> > > >
> > > > No, they are not leaking actually.
> > > >
> > > > You are right that they are not deallocated in do_readdir_many(), but
> > > > that's intentional: in the new implementation of v9fs_do_readdir() you
> > > > see that v9fs_free_dirents(entries) is *always* called at the very end
> > > > of
> > > > the function, no matter if success or any error. That's one of the
> > > > measures to simplify overall code as much as possible.
> > >
> > > Hmm... I still don't quite like the idea of having an erroring function
> > > asking for extra cleanup. I suggest you come up with an idem-potent
> > > version
> > > of v9fs_free_dirents(), move it to codir.c (I also prefer locality of
> > > calls
> > > to g_malloc and g_free in the same unit), make it extern and call it
> > > both on the error path of v9fs_co_readdir_many() and in
> > > v9fs_do_readdir().
> >
> > I understand your position of course, but I still won't find that to be a
> > good move.
> >
> > My veto here has a reason: your requested change would prevent an
> > application that I had in mind for future purpose actually: Allowing
> > "greedy" fetching
> Are you telling that this series has some kind of hidden agenda related to
> a possible future change ?!?
readdir_many() is written intended as general purpose directory retrieval
function, that is for other purposes in future in mind, yes.
What I don't do is adding code which is not explicitly needed right now of
course. That would not make sense and would make code unnecessarily bloated
and of course too complicated (e.g. readdir_many() is currently simply
directly calling v9fs_readdir_response_size() to decide whether to terminate
the loop instead of taking some complicated general-purpose loop end
"predicate" structure or callback as function argument).
But when it comes to the structure of the code that I have to add NOW, then I
indeed take potential future changes into account, yes! And this applies
specifically to the two changes you requested here inside readdir_many().
Because I already know, I would need to revert those 2 changes that you
requested later on. And I don't see any issue whatsover retaining the current
version concerning those 2.
Best regards,
Christian Schoenebeck
Re: [PATCH v1 3/4] .cirrus.yml: bump FreeBSD to the current stable release
On Fri, May 1, 2020 at 7:15 PM Alex Bennée wrote: > > Hopefully this will un-stick the test which has been broken for a long > time. > > Signed-off-by: Alex Bennée > --- > .cirrus.yml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/.cirrus.yml b/.cirrus.yml > index 90645fede6..f06f5af2b9 100644 > --- a/.cirrus.yml > +++ b/.cirrus.yml > @@ -3,7 +3,7 @@ env: > > freebsd_12_task: >freebsd_instance: > -image: freebsd-12-0-release-amd64 > +image_family: freebsd-12-1 > cpu: 8 > memory: 8G >install_script: pkg install -y > -- > 2.20.1 > Reviewed-by: Li-Wen Hsu Tested-by: Li-Wen Hsu I would be nice to also add this patch: https://github.com/lwhsu/qemu/commit/ac699f79b4d86d8195d76c3befada65ade449cc0.patch To prevent problems in the future. The error was due to the pkg version got "fixed" when building image, and was too old when VM got provisioned, then it cannot be not compatible with the package repository. Ref: https://lists.freebsd.org/pipermail/freebsd-cloud/2020-April/000234.html Best, Li-Wen
Re: [PATCH v3 00/14] LUKS: encryption slot management using amend interface
On Mon, 2020-05-04 at 11:19 +0100, Daniel P. Berrangé wrote: > On Sun, May 03, 2020 at 09:43:10PM +0300, Maxim Levitsky wrote: > > Hi! > > Here is the updated series of my patches, incorporating all the feedback I > > received. > > > > This implements the API interface that we agreed upon except that I merged > > the > > LUKSKeyslotActive/LUKSKeyslotInactive union into a struct because otherwise > > I need nested unions which are not supported currently by QAPI parser. > > This didn't change the API and thus once support for nested unions is there, > > it can always be implemented in backward compatible way. > > > > I hope that this series will finally be considered for merging, since I am > > somewhat running > > out of time to finish this task. > > > > Patches are strictly divided by topic to 3 groups, and each group depends > > on former groups. > > > > * Patches 1,2 implement qcrypto generic amend interface, including > > definition > > of structs used in crypto.json and implement this in luks crypto driver > > Nothing is exposed to the user at this stage > > > > * Patches 3-9 use the code from patches 1,2 to implement qemu-img amend > > based encryption slot management > > for luks and for qcow2, and add a bunch of iotests to cover that. > > > > * Patches 10-13 add x-blockdev-amend (I'll drop the -x prefix if you like), > > and wire it > > to luks and qcow2 driver to implement qmp based encryption slot > > management also using > > the code from patches 1,2, and also add a bunch of iotests to cover this. > > > > Tested with -raw,-qcow2 and -luks iotests and 'make check' > > > > V3: rebased, addressed most of the review feedback. > > For now I kept the slot bitmap code since I am not sure that replacing it > > will be better. > > I'm still of the opinion that the bitmaps code should be replaced. > With this current design we are iterating over the slot of keys slots > 4 times, doing different checks/logic in each iteration. This is really > not nice for understanding how the code works. IMHO we should be iterating > at most twice - once to validate the requested configuration, and once > to apply the requested configuration. Even if there si duplication of > logic in between the delete/add key codepaths, I think it will be better > as the logic for each operation will be in one place. > > > Regards, > Daniel All right then, I also agree kind of that these iterations aren't that nice. I just wanted to do this in the same way the spec defines it, which is currently kind of independent of how key add/remove operation. I'll send updated code soon. Best regards, Maxim Levitsky
[PATCH v4 6/6] net/colo-compare.c: Correct ordering in complete and finalize
In colo_compare_complete, insert CompareState into net_compares
only after everything has been initialized.
In colo_compare_finalize, remove CompareState from net_compares
before anything is deinitialized.
Signed-off-by: Lukas Straub
---
net/colo-compare.c | 45 +++--
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index c7572d75e9..6f80bcece6 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1283,15 +1283,6 @@ static void colo_compare_complete(UserCreatable *uc,
Error **errp)
s->vnet_hdr);
}
-qemu_mutex_lock(&colo_compare_mutex);
-if (!colo_compare_active) {
-qemu_mutex_init(&event_mtx);
-qemu_cond_init(&event_complete_cond);
-colo_compare_active = true;
-}
-QTAILQ_INSERT_TAIL(&net_compares, s, next);
-qemu_mutex_unlock(&colo_compare_mutex);
-
s->out_sendco.s = s;
s->out_sendco.chr = &s->chr_out;
s->out_sendco.notify_remote_frame = false;
@@ -1312,6 +1303,16 @@ static void colo_compare_complete(UserCreatable *uc,
Error **errp)
connection_destroy);
colo_compare_iothread(s);
+
+qemu_mutex_lock(&colo_compare_mutex);
+if (!colo_compare_active) {
+qemu_mutex_init(&event_mtx);
+qemu_cond_init(&event_complete_cond);
+colo_compare_active = true;
+}
+QTAILQ_INSERT_TAIL(&net_compares, s, next);
+qemu_mutex_unlock(&colo_compare_mutex);
+
return;
}
@@ -1384,19 +1385,6 @@ static void colo_compare_finalize(Object *obj)
CompareState *s = COLO_COMPARE(obj);
CompareState *tmp = NULL;
-qemu_chr_fe_deinit(&s->chr_pri_in, false);
-qemu_chr_fe_deinit(&s->chr_sec_in, false);
-qemu_chr_fe_deinit(&s->chr_out, false);
-if (s->notify_dev) {
-qemu_chr_fe_deinit(&s->chr_notify_dev, false);
-}
-
-if (s->iothread) {
-colo_compare_timer_del(s);
-}
-
-qemu_bh_delete(s->event_bh);
-
qemu_mutex_lock(&colo_compare_mutex);
QTAILQ_FOREACH(tmp, &net_compares, next) {
if (tmp == s) {
@@ -1411,6 +1399,19 @@ static void colo_compare_finalize(Object *obj)
}
qemu_mutex_unlock(&colo_compare_mutex);
+qemu_chr_fe_deinit(&s->chr_pri_in, false);
+qemu_chr_fe_deinit(&s->chr_sec_in, false);
+qemu_chr_fe_deinit(&s->chr_out, false);
+if (s->notify_dev) {
+qemu_chr_fe_deinit(&s->chr_notify_dev, false);
+}
+
+if (s->iothread) {
+colo_compare_timer_del(s);
+}
+
+qemu_bh_delete(s->event_bh);
+
AioContext *ctx = iothread_get_aio_context(s->iothread);
aio_context_acquire(ctx);
AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
--
2.20.1
pgpcjD7bZO_GZ.pgp
Description: OpenPGP digital signature
[PATCH v4 1/6] net/colo-compare.c: Create event_bh with the right AioContext
qemu_bh_new will set the bh to be executed in the main
loop. This causes crashes as colo_compare_handle_event assumes
that it has exclusive access the queues, which are also
concurrently accessed in the iothread.
Create the bh with the AioContext of the iothread to fulfill
these assumptions and fix the crashes. This is safe, because
the bh already takes the appropriate locks.
Signed-off-by: Lukas Straub
Reviewed-by: Zhang Chen
Reviewed-by: Derek Su
Tested-by: Derek Su
---
net/colo-compare.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 10c0239f9d..1de4220fe2 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -890,6 +890,7 @@ static void colo_compare_handle_event(void *opaque)
static void colo_compare_iothread(CompareState *s)
{
+AioContext *ctx = iothread_get_aio_context(s->iothread);
object_ref(OBJECT(s->iothread));
s->worker_context = iothread_get_g_main_context(s->iothread);
@@ -906,7 +907,7 @@ static void colo_compare_iothread(CompareState *s)
}
colo_compare_timer_init(s);
-s->event_bh = qemu_bh_new(colo_compare_handle_event, s);
+s->event_bh = aio_bh_new(ctx, colo_compare_handle_event, s);
}
static char *compare_get_pri_indev(Object *obj, Error **errp)
--
2.20.1
pgpkR34jRNzLs.pgp
Description: OpenPGP digital signature
[PATCH v4 0/6] colo-compare bugfixes
Hello Everyone, Here are fixes for bugs that I found in my tests. Benchmark results: Client-to-server tcp: without patch: ~63 Mbit/s with patch: ~66 Mbit/s Server-to-client tcp: without patch: ~771 Kbit/s with patch: ~702 Kbit/s Regards, Lukas Straub Version changes: v4: -fix potential deadlock with notify_remote_frame -avoid malloc and memcpy in many cases v3: -fix checkpatch.pl error v2: -better wording -fix performance-regression in patch 3 "net/colo-compare.c: Fix deadlock in compare_chr_send" -add more bugfixes Lukas Straub (6): net/colo-compare.c: Create event_bh with the right AioContext chardev/char.c: Use qemu_co_sleep_ns if in coroutine net/colo-compare.c: Fix deadlock in compare_chr_send net/colo-compare.c: Only hexdump packets if tracing is enabled net/colo-compare.c, softmmu/vl.c: Check that colo-compare is active net/colo-compare.c: Correct ordering in complete and finalize chardev/char.c | 7 +- net/colo-compare.c | 250 + net/colo-compare.h | 1 + net/colo.c | 7 ++ net/colo.h | 1 + softmmu/vl.c | 2 + 6 files changed, 204 insertions(+), 64 deletions(-) -- 2.20.1 pgpISjhSQHf_i.pgp Description: OpenPGP digital signature
Re: [PATCH v2] s390x/kvm: help valgrind in several places
On Wed, 29 Apr 2020 03:42:01 -0400 Christian Borntraeger wrote: > We need some little help in the code to reduce the valgrind noise. > This patch does this with some designated initializers for the cpu > model features and subfunctions. > > Reviewed-by: Philippe Mathieu-Daudé > Signed-off-by: Christian Borntraeger > --- > target/s390x/kvm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Thanks, applied.
[PATCH v4 2/6] chardev/char.c: Use qemu_co_sleep_ns if in coroutine
This will be needed in the next patch.
Signed-off-by: Lukas Straub
Reviewed-by: Marc-André Lureau
Reviewed-by: Zhang Chen
---
chardev/char.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/chardev/char.c b/chardev/char.c
index e77564060d..5c8014199f 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -38,6 +38,7 @@
#include "qemu/module.h"
#include "qemu/option.h"
#include "qemu/id.h"
+#include "qemu/coroutine.h"
#include "chardev/char-mux.h"
@@ -119,7 +120,11 @@ static int qemu_chr_write_buffer(Chardev *s,
retry:
res = cc->chr_write(s, buf + *offset, len - *offset);
if (res < 0 && errno == EAGAIN && write_all) {
-g_usleep(100);
+if (qemu_in_coroutine()) {
+qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 10);
+} else {
+g_usleep(100);
+}
goto retry;
}
--
2.20.1
pgpzyc_8wEPo1.pgp
Description: OpenPGP digital signature
[PATCH v4 3/6] net/colo-compare.c: Fix deadlock in compare_chr_send
The chr_out chardev is connected to a filter-redirector
running in the main loop. qemu_chr_fe_write_all might block
here in compare_chr_send if the (socket-)buffer is full.
If another filter-redirector in the main loop want's to
send data to chr_pri_in it might also block if the buffer
is full. This leads to a deadlock because both event loops
get blocked.
Fix this by converting compare_chr_send to a coroutine and
putting the packets in a send queue.
Signed-off-by: Lukas Straub
---
net/colo-compare.c | 187 ++---
net/colo.c | 7 ++
net/colo.h | 1 +
3 files changed, 150 insertions(+), 45 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 1de4220fe2..2a4e7f7c4e 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -32,6 +32,9 @@
#include "migration/migration.h"
#include "util.h"
+#include "block/aio-wait.h"
+#include "qemu/coroutine.h"
+
#define TYPE_COLO_COMPARE "colo-compare"
#define COLO_COMPARE(obj) \
OBJECT_CHECK(CompareState, (obj), TYPE_COLO_COMPARE)
@@ -77,6 +80,23 @@ static int event_unhandled_count;
*|packet | |packet +|packet | |packet +
*++ ++++ ++
*/
+
+typedef struct SendCo {
+Coroutine *co;
+struct CompareState *s;
+CharBackend *chr;
+GQueue send_list;
+bool notify_remote_frame;
+bool done;
+int ret;
+} SendCo;
+
+typedef struct SendEntry {
+uint32_t size;
+uint32_t vnet_hdr_len;
+uint8_t *buf;
+} SendEntry;
+
typedef struct CompareState {
Object parent;
@@ -91,6 +111,8 @@ typedef struct CompareState {
SocketReadState pri_rs;
SocketReadState sec_rs;
SocketReadState notify_rs;
+SendCo out_sendco;
+SendCo notify_sendco;
bool vnet_hdr;
uint32_t compare_timeout;
uint32_t expired_scan_cycle;
@@ -124,10 +146,11 @@ enum {
static int compare_chr_send(CompareState *s,
-const uint8_t *buf,
+uint8_t *buf,
uint32_t size,
uint32_t vnet_hdr_len,
-bool notify_remote_frame);
+bool notify_remote_frame,
+bool zero_copy);
static bool packet_matches_str(const char *str,
const uint8_t *buf,
@@ -145,7 +168,7 @@ static void notify_remote_frame(CompareState *s)
char msg[] = "DO_CHECKPOINT";
int ret = 0;
-ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
+ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true, false);
if (ret < 0) {
error_report("Notify Xen COLO-frame failed");
}
@@ -272,12 +295,13 @@ static void colo_release_primary_pkt(CompareState *s,
Packet *pkt)
pkt->data,
pkt->size,
pkt->vnet_hdr_len,
- false);
+ false,
+ true);
if (ret < 0) {
error_report("colo send primary packet failed");
}
trace_colo_compare_main("packet same and release packet");
-packet_destroy(pkt, NULL);
+packet_destroy_partial(pkt, NULL);
}
/*
@@ -699,65 +723,115 @@ static void colo_compare_connection(void *opaque, void
*user_data)
}
}
-static int compare_chr_send(CompareState *s,
-const uint8_t *buf,
-uint32_t size,
-uint32_t vnet_hdr_len,
-bool notify_remote_frame)
+static void coroutine_fn _compare_chr_send(void *opaque)
{
+SendCo *sendco = opaque;
+CompareState *s = sendco->s;
int ret = 0;
-uint32_t len = htonl(size);
-if (!size) {
-return 0;
-}
+while (!g_queue_is_empty(&sendco->send_list)) {
+SendEntry *entry = g_queue_pop_tail(&sendco->send_list);
+uint32_t len = htonl(entry->size);
-if (notify_remote_frame) {
-ret = qemu_chr_fe_write_all(&s->chr_notify_dev,
-(uint8_t *)&len,
-sizeof(len));
-} else {
-ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
-}
+ret = qemu_chr_fe_write_all(sendco->chr, (uint8_t *)&len, sizeof(len));
-if (ret != sizeof(len)) {
-goto err;
-}
+if (ret != sizeof(len)) {
+g_free(entry->buf);
+g_slice_free(SendEntry, entry);
+goto err;
+}
-if (s->vnet_hdr) {
-/*
- * We send vnet header len make other module(like filter-redirector)
- * know how to parse net packet correctly.
- */
-len = htonl(vnet_hdr_len);
+if (!sendco->notify_remote_frame && s->vnet_hdr) {
+/*
+ * We send vnet header len make ot
Re: [PATCH] xen: fix build without pci passthrough
Hi Roger,
On 5/4/20 12:14 PM, Roger Pau Monne wrote:
has_igd_gfx_passthru is only available when QEMU is built with
CONFIG_XEN_PCI_PASSTHROUGH, and hence shouldn't be used in common
code without checking if it's available.
Fixes: 46472d82322d0 ('xen: convert "-machine igd-passthru" to an accelerator
property')
Signed-off-by: Roger Pau Monné
See Kconfig fix suggested here:
https://www.mail-archive.com/xen-devel@lists.xenproject.org/msg61844.html
---
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: xen-de...@lists.xenproject.org
---
hw/xen/xen-common.c | 4
hw/xen/xen_pt.h | 7 +++
2 files changed, 11 insertions(+)
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index a15070f7f6..c800862419 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -127,6 +127,7 @@ static void xen_change_state_handler(void *opaque, int
running,
}
}
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
static bool xen_get_igd_gfx_passthru(Object *obj, Error **errp)
{
return has_igd_gfx_passthru;
@@ -136,6 +137,7 @@ static void xen_set_igd_gfx_passthru(Object *obj, bool
value, Error **errp)
{
has_igd_gfx_passthru = value;
}
+#endif
static void xen_setup_post(MachineState *ms, AccelState *accel)
{
@@ -197,11 +199,13 @@ static void xen_accel_class_init(ObjectClass *oc, void
*data)
compat_props_add(ac->compat_props, compat, G_N_ELEMENTS(compat));
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
object_class_property_add_bool(oc, "igd-passthru",
xen_get_igd_gfx_passthru, xen_set_igd_gfx_passthru,
&error_abort);
object_class_property_set_description(oc, "igd-passthru",
"Set on/off to enable/disable igd passthrou", &error_abort);
+#endif
}
#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 179775db7b..660dd8a008 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -1,6 +1,7 @@
#ifndef XEN_PT_H
#define XEN_PT_H
+#include "qemu/osdep.h"
#include "hw/xen/xen_common.h"
#include "hw/pci/pci.h"
#include "xen-host-pci-device.h"
@@ -322,7 +323,13 @@ extern void *pci_assign_dev_load_option_rom(PCIDevice *dev,
unsigned int domain,
unsigned int bus, unsigned int
slot,
unsigned int function);
+
+#ifdef CONFIG_XEN_PCI_PASSTHROUGH
extern bool has_igd_gfx_passthru;
+#else
+# define has_igd_gfx_passthru false
+#endif
+
static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
{
return (has_igd_gfx_passthru
[PATCH v4 5/6] net/colo-compare.c, softmmu/vl.c: Check that colo-compare is active
If the colo-compare object is removed before failover and a
checkpoint happens, qemu crashes because it tries to lock
the destroyed event_mtx in colo_notify_compares_event.
Fix this by checking if everything is initialized by
introducing a new variable colo_compare_active which
is protected by a new mutex colo_compare_mutex. The new mutex
also protects against concurrent access of the net_compares
list and makes sure that colo_notify_compares_event isn't
active while we destroy event_mtx and event_complete_cond.
With this it also is again possible to use colo without
colo-compare (periodic mode) and to use multiple colo-compare
for multiple network interfaces.
Signed-off-by: Lukas Straub
---
net/colo-compare.c | 35 +--
net/colo-compare.h | 1 +
softmmu/vl.c | 2 ++
3 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 56db3d3bfc..c7572d75e9 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -54,6 +54,8 @@ static NotifierList colo_compare_notifiers =
#define REGULAR_PACKET_CHECK_MS 3000
#define DEFAULT_TIME_OUT_MS 3000
+static QemuMutex colo_compare_mutex;
+static bool colo_compare_active;
static QemuMutex event_mtx;
static QemuCond event_complete_cond;
static int event_unhandled_count;
@@ -906,6 +908,12 @@ static void check_old_packet_regular(void *opaque)
void colo_notify_compares_event(void *opaque, int event, Error **errp)
{
CompareState *s;
+qemu_mutex_lock(&colo_compare_mutex);
+
+if (!colo_compare_active) {
+qemu_mutex_unlock(&colo_compare_mutex);
+return;
+}
qemu_mutex_lock(&event_mtx);
QTAILQ_FOREACH(s, &net_compares, next) {
@@ -919,6 +927,7 @@ void colo_notify_compares_event(void *opaque, int event,
Error **errp)
}
qemu_mutex_unlock(&event_mtx);
+qemu_mutex_unlock(&colo_compare_mutex);
}
static void colo_compare_timer_init(CompareState *s)
@@ -1274,7 +1283,14 @@ static void colo_compare_complete(UserCreatable *uc,
Error **errp)
s->vnet_hdr);
}
+qemu_mutex_lock(&colo_compare_mutex);
+if (!colo_compare_active) {
+qemu_mutex_init(&event_mtx);
+qemu_cond_init(&event_complete_cond);
+colo_compare_active = true;
+}
QTAILQ_INSERT_TAIL(&net_compares, s, next);
+qemu_mutex_unlock(&colo_compare_mutex);
s->out_sendco.s = s;
s->out_sendco.chr = &s->chr_out;
@@ -1290,9 +1306,6 @@ static void colo_compare_complete(UserCreatable *uc,
Error **errp)
g_queue_init(&s->conn_list);
-qemu_mutex_init(&event_mtx);
-qemu_cond_init(&event_complete_cond);
-
s->connection_track_table = g_hash_table_new_full(connection_key_hash,
connection_key_equal,
g_free,
@@ -1384,12 +1397,19 @@ static void colo_compare_finalize(Object *obj)
qemu_bh_delete(s->event_bh);
+qemu_mutex_lock(&colo_compare_mutex);
QTAILQ_FOREACH(tmp, &net_compares, next) {
if (tmp == s) {
QTAILQ_REMOVE(&net_compares, s, next);
break;
}
}
+if (QTAILQ_EMPTY(&net_compares)) {
+colo_compare_active = false;
+qemu_mutex_destroy(&event_mtx);
+qemu_cond_destroy(&event_complete_cond);
+}
+qemu_mutex_unlock(&colo_compare_mutex);
AioContext *ctx = iothread_get_aio_context(s->iothread);
aio_context_acquire(ctx);
@@ -1413,15 +1433,18 @@ static void colo_compare_finalize(Object *obj)
object_unref(OBJECT(s->iothread));
}
-qemu_mutex_destroy(&event_mtx);
-qemu_cond_destroy(&event_complete_cond);
-
g_free(s->pri_indev);
g_free(s->sec_indev);
g_free(s->outdev);
g_free(s->notify_dev);
}
+void colo_compare_init_globals(void)
+{
+colo_compare_active = false;
+qemu_mutex_init(&colo_compare_mutex);
+}
+
static const TypeInfo colo_compare_info = {
.name = TYPE_COLO_COMPARE,
.parent = TYPE_OBJECT,
diff --git a/net/colo-compare.h b/net/colo-compare.h
index 22ddd512e2..eb483ac586 100644
--- a/net/colo-compare.h
+++ b/net/colo-compare.h
@@ -17,6 +17,7 @@
#ifndef QEMU_COLO_COMPARE_H
#define QEMU_COLO_COMPARE_H
+void colo_compare_init_globals(void);
void colo_notify_compares_event(void *opaque, int event, Error **errp);
void colo_compare_register_notifier(Notifier *notify);
void colo_compare_unregister_notifier(Notifier *notify);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 32c0047889..a913ed5469 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -112,6 +112,7 @@
#include "qapi/qmp/qerror.h"
#include "sysemu/iothread.h"
#include "qemu/guest-random.h"
+#include "net/colo-compare.h"
#define MAX_VIRTIO_CONSOLES 1
@@ -2906,6 +2907,7 @@ void qemu_init(int argc, char **argv, char **envp)
precopy_infrastructure_init();
postcopy_infrastructure_init();
monitor_init_globals();
+colo_com
Re: [PATCH 2/4] device-core: use RCU for list of childs of a bus
On Thu, Apr 16, 2020 at 11:36:22PM +0300, Maxim Levitsky wrote:
> @@ -90,9 +92,13 @@ static void bus_reset_child_foreach(Object *obj,
> ResettableChildCallback cb,
> BusState *bus = BUS(obj);
> BusChild *kid;
>
> -QTAILQ_FOREACH(kid, &bus->children, sibling) {
> +rcu_read_lock();
> +
> +QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
> cb(OBJECT(kid->child), opaque, type);
> }
> +
> +rcu_read_unlock();
> }
>
> static void qbus_realize(BusState *bus, DeviceState *parent, const char
> *name)
> @@ -138,10 +144,15 @@ static void bus_unparent(Object *obj)
> /* Only the main system bus has no parent, and that bus is never freed */
> assert(bus->parent);
>
> -while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
> +rcu_read_lock();
> +
> +while ((kid = QTAILQ_FIRST_RCU(&bus->children)) != NULL) {
> DeviceState *dev = kid->child;
> object_unparent(OBJECT(dev));
> }
> +
> +rcu_read_unlock();
rcu_read_lock() is called but this looks like a list write operation.
If I understand correctly bus->children list writes can only be called
with the QEMU global mutex and therefore rcu_read_lock() is not required
here?
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 85f062def7..f0c87e582e 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -50,26 +50,37 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> return dc->vmsd;
> }
>
> +static void bus_free_bus_child(BusChild *kid)
> +{
> +object_unref(OBJECT(kid->child));
Users like scsi_device_find() do not take a refcount on the child. If
the device is removed then bus_free_bus_child may call object_unref()
while another thread is still accessing the child.
Maybe I'm missing something that prevents this scenario?
If not, then another patch is necessary first that introduces stricter
refcount discipline across the codebase. This applies both to users who
directly access bus->children as well as to those who call walk() and
stash child pointers in their callback function.
> +g_free(kid);
> +}
> +
> static void bus_remove_child(BusState *bus, DeviceState *child)
> {
> BusChild *kid;
>
> -QTAILQ_FOREACH(kid, &bus->children, sibling) {
> +rcu_read_lock();
List write under rcu_read_lock().
> @@ -82,7 +93,9 @@ static void bus_add_child(BusState *bus, DeviceState *child)
> kid->child = child;
> object_ref(OBJECT(kid->child));
>
> -QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
> +rcu_read_lock();
> +QTAILQ_INSERT_HEAD_RCU(&bus->children, kid, sibling);
> +rcu_read_unlock();
List write under rcu_read_lock().
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 472bbd233b..b0f4a35f81 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -367,12 +367,16 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s,
> VirtIOSCSIReq *req)
> case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
> target = req->req.tmf.lun[1];
> s->resetting++;
> -QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
> +
> +rcu_read_lock();
> +QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
We need a QTAILQ_FOREACH_WITH_RCU_READ_LOCK() macro that combines
WITH_RCU_READ_LOCK() and QTAILQ_FOREACH_RCU(). :-)
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 38c9399cd4..58733f28e2 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -128,8 +128,11 @@ void virtio_bus_set_vdev_config(VirtioBusState *bus,
> uint8_t *config);
> static inline VirtIODevice *virtio_bus_get_device(VirtioBusState *bus)
> {
> BusState *qbus = &bus->parent_obj;
> -BusChild *kid = QTAILQ_FIRST(&qbus->children);
> -DeviceState *qdev = kid ? kid->child : NULL;
> +BusChild *kid;
> +DeviceState *qdev;
> +
> +kid = QTAILQ_FIRST(&qbus->children);
QTAILQ_FIRST_RCU()
signature.asc
Description: PGP signature
