Re: [Qemu-devel] How to emulate block I/O timeout on qemu side?
On 11/12/2018 03:13 PM, Marc Olson via Qemu-devel wrote: > On 11/3/18 10:24 AM, Dongli Zhang wrote: >> Hi all, >> >> I tried with the patch at: >> >> https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg00394.html >> >> The patch is applied to qemu-3.0.0. >> >> >> Below configuration is used to test the feature for guest VM nvme. >> >> # qemu-system-x86_64 \ >> -smp 4 -m 2000M -enable-kvm -vnc :0 -monitor stdio \ >> -net nic -net user,hostfwd=tcp::5022-:22 \ >> -drive file=virtio-disk.img,format=raw,if=none,id=disk0 \ >> -device virtio-blk-pci,drive=disk0,id=disk0-dev,num-queues=2,iothread=io1 \ >> -object iothread,id=io1 \ >> -device nvme,drive=nvme1,serial=deadbeaf1 \ >> -drive file=blkdebug:blkdebug.config:nvme.img,if=none,id=nvme1 >> >> # cat blkdebug.config >> [delay] >> event = "write_aio" >> latency = "99" >> sector = "40960" >> >> >> The 'write' latency of sector=40960 is set to a very large value. When the >> I/O >> is stalled in guest due to that sector=40960 is accessed, I do see below >> messages in guest log: >> >> [ 80.807755] nvme nvme0: I/O 11 QID 2 timeout, aborting >> [ 80.808095] nvme nvme0: Abort status: 0x4001 >> >> >> However, then nothing happens further. nvme I/O hangs in guest. I am not >> able to >> kill the qemu process with Ctrl+C. Both vnc and qemu user net do not work. I >> need to kill qemu with "kill -9" >> >> >> The same result for virtio-scsi and qemu is stuck as well. > While I didn't try virtio-scsi, I wasn't able to reproduce this behavior using > nvme on Ubuntu 18.04 (4.15). What image and kernel version are you trying > against? Would you like to reproduce the "aborting" message or the qemu hang? guest image: ubuntu 16.04 guest kernel: mainline linux kernel (and default kernel in ubuntu 16.04) qemu: qemu-3.0.0 (with the blkdebug delay patch) Would you be able to see the nvme abort (which is indeed not supported by qemu) message in guest kernel? Once I see that message, I would not be able to kill the qemu-system-x86_64 command line with Ctrl+C. Dongli Zhang
Re: [Qemu-devel] How to emulate block I/O timeout on qemu side?
On 11/3/18 10:24 AM, Dongli Zhang wrote: Hi all, I tried with the patch at: https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg00394.html The patch is applied to qemu-3.0.0. Below configuration is used to test the feature for guest VM nvme. # qemu-system-x86_64 \ -smp 4 -m 2000M -enable-kvm -vnc :0 -monitor stdio \ -net nic -net user,hostfwd=tcp::5022-:22 \ -drive file=virtio-disk.img,format=raw,if=none,id=disk0 \ -device virtio-blk-pci,drive=disk0,id=disk0-dev,num-queues=2,iothread=io1 \ -object iothread,id=io1 \ -device nvme,drive=nvme1,serial=deadbeaf1 \ -drive file=blkdebug:blkdebug.config:nvme.img,if=none,id=nvme1 # cat blkdebug.config [delay] event = "write_aio" latency = "99" sector = "40960" The 'write' latency of sector=40960 is set to a very large value. When the I/O is stalled in guest due to that sector=40960 is accessed, I do see below messages in guest log: [ 80.807755] nvme nvme0: I/O 11 QID 2 timeout, aborting [ 80.808095] nvme nvme0: Abort status: 0x4001 However, then nothing happens further. nvme I/O hangs in guest. I am not able to kill the qemu process with Ctrl+C. Both vnc and qemu user net do not work. I need to kill qemu with "kill -9" The same result for virtio-scsi and qemu is stuck as well. While I didn't try virtio-scsi, I wasn't able to reproduce this behavior using nvme on Ubuntu 18.04 (4.15). What image and kernel version are you trying against? /marc
[Qemu-devel] [PATCH v3 3/3] blkdebug: Add latency injection rule type
Add a new rule type for blkdebug that instead of returning an error, can
inject latency to an IO.
Signed-off-by: Marc Olson
---
block/blkdebug.c | 79 +++---
docs/devel/blkdebug.txt| 35 ++--
qapi/block-core.json | 31 ++
tests/qemu-iotests/071 | 63
tests/qemu-iotests/071.out | 31 ++
5 files changed, 226 insertions(+), 13 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 7739849..6b1f2d6 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -65,6 +65,7 @@ typedef struct BlkdebugSuspendedReq {
enum {
ACTION_INJECT_ERROR,
+ACTION_INJECT_DELAY,
ACTION_SET_STATE,
ACTION_SUSPEND,
};
@@ -81,6 +82,9 @@ typedef struct BlkdebugRule {
int immediately;
} inject_error;
struct {
+int64_t latency;
+} delay;
+struct {
int new_state;
} set_state;
struct {
@@ -123,6 +127,34 @@ static QemuOptsList inject_error_opts = {
},
};
+static QemuOptsList inject_delay_opts = {
+.name = "inject-delay",
+.head = QTAILQ_HEAD_INITIALIZER(inject_delay_opts.head),
+.desc = {
+{
+.name = "event",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "state",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "latency",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "sector",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "once",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end of list */ }
+},
+};
+
static QemuOptsList set_state_opts = {
.name = "set-state",
.head = QTAILQ_HEAD_INITIALIZER(set_state_opts.head),
@@ -145,6 +177,7 @@ static QemuOptsList set_state_opts = {
static QemuOptsList *config_groups[] = {
_error_opts,
+_delay_opts,
_state_opts,
NULL
};
@@ -194,6 +227,11 @@ static int add_rule(void *opaque, QemuOpts *opts, Error
**errp)
qemu_opt_get_bool(opts, "immediately", 0);
break;
+case ACTION_INJECT_DELAY:
+rule->options.delay.latency =
+qemu_opt_get_number(opts, "latency", 100) * SCALE_US;
+break;
+
case ACTION_SET_STATE:
rule->options.set_state.new_state =
qemu_opt_get_number(opts, "new_state", 0);
@@ -226,6 +264,12 @@ static void remove_rule(BlkdebugRule *rule)
g_free(rule);
}
+static void remove_active_rule(BDRVBlkdebugState *s, BlkdebugRule *rule)
+{
+QSIMPLEQ_REMOVE(>active_rules, rule, BlkdebugRule, active_next);
+remove_rule(rule);
+}
+
static int read_config(BDRVBlkdebugState *s, const char *filename,
QDict *options, Error **errp)
{
@@ -264,6 +308,14 @@ static int read_config(BDRVBlkdebugState *s, const char
*filename,
goto fail;
}
+d.action = ACTION_INJECT_DELAY;
+qemu_opts_foreach(_delay_opts, add_rule, , _err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
+
d.action = ACTION_SET_STATE;
qemu_opts_foreach(_state_opts, add_rule, , _err);
if (local_err) {
@@ -275,6 +327,7 @@ static int read_config(BDRVBlkdebugState *s, const char
*filename,
ret = 0;
fail:
qemu_opts_reset(_error_opts);
+qemu_opts_reset(_delay_opts);
qemu_opts_reset(_state_opts);
if (f) {
fclose(f);
@@ -474,7 +527,8 @@ static int rule_check(BlockDriverState *bs, uint64_t
offset, uint64_t bytes)
{
BDRVBlkdebugState *s = bs->opaque;
BlkdebugRule *rule = NULL;
-BlkdebugRule *error_rule = NULL;
+BlkdebugRule *error_rule = NULL, *delay_rule = NULL;
+int64_t latency;
int error;
bool immediately;
int ret = 0;
@@ -484,20 +538,36 @@ static int rule_check(BlockDriverState *bs, uint64_t
offset, uint64_t bytes)
(bytes && rule->offset >= offset &&
rule->offset < offset + bytes))
{
-if (rule->action == ACTION_INJECT_ERROR) {
+if (!error_rule && rule->action == ACTION_INJECT_ERROR) {
error_rule = rule;
+} else if (!delay_rule && rule->action == ACTION_INJECT_DELAY) {
+delay_rule = rule;
+}
+
+if (error_rule && delay_rule) {
break;
}
}
}
+if (delay_rule) {
+latency = delay_rule->options.delay.latency;
+
+if (delay_rule->once) {
+remove_active_rule(s, delay_rule);
+}
+
+if (latency != 0) {
+qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, latency);
+}
+}
+
if (error_rule) {
immediately = error_rule->options.inject_error.immediately;
error = error_rule->options.inject_error.error;
if (error_rule->once)
[Qemu-devel] [PATCH v3 2/3] blkdebug: Extend rule check for additional types
Break out the more common parts of the BlkdebugRule struct, and make
rule_check() more explicit about operating only on error injection types
so that additional rule types can be added in the future.
Signed-off-by: Marc Olson
---
block/blkdebug.c | 59 +---
1 file changed, 31 insertions(+), 28 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 327049b..7739849 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -73,13 +73,13 @@ typedef struct BlkdebugRule {
BlkdebugEvent event;
int action;
int state;
+int once;
+int64_t offset;
union {
struct {
int error;
int immediately;
-int once;
-int64_t offset;
-} inject;
+} inject_error;
struct {
int new_state;
} set_state;
@@ -182,16 +182,16 @@ static int add_rule(void *opaque, QemuOpts *opts, Error
**errp)
.state = qemu_opt_get_number(opts, "state", 0),
};
+rule->once = qemu_opt_get_bool(opts, "once", 0);
+sector = qemu_opt_get_number(opts, "sector", -1);
+rule->offset = sector == -1 ? -1 : sector * BDRV_SECTOR_SIZE;
+
/* Parse action-specific options */
switch (d->action) {
case ACTION_INJECT_ERROR:
-rule->options.inject.error = qemu_opt_get_number(opts, "errno", EIO);
-rule->options.inject.once = qemu_opt_get_bool(opts, "once", 0);
-rule->options.inject.immediately =
+rule->options.inject_error.error = qemu_opt_get_number(opts, "errno",
EIO);
+rule->options.inject_error.immediately =
qemu_opt_get_bool(opts, "immediately", 0);
-sector = qemu_opt_get_number(opts, "sector", -1);
-rule->options.inject.offset =
-sector == -1 ? -1 : sector * BDRV_SECTOR_SIZE;
break;
case ACTION_SET_STATE:
@@ -474,38 +474,41 @@ static int rule_check(BlockDriverState *bs, uint64_t
offset, uint64_t bytes)
{
BDRVBlkdebugState *s = bs->opaque;
BlkdebugRule *rule = NULL;
+BlkdebugRule *error_rule = NULL;
int error;
bool immediately;
+int ret = 0;
QSIMPLEQ_FOREACH(rule, >active_rules, active_next) {
-uint64_t inject_offset = rule->options.inject.offset;
-
-if (inject_offset == -1 ||
-(bytes && inject_offset >= offset &&
- inject_offset < offset + bytes))
+if (rule->offset == -1 ||
+(bytes && rule->offset >= offset &&
+ rule->offset < offset + bytes))
{
-break;
+if (rule->action == ACTION_INJECT_ERROR) {
+error_rule = rule;
+break;
+}
}
}
-if (!rule) {
-return 0;
-}
+if (error_rule) {
+immediately = error_rule->options.inject_error.immediately;
+error = error_rule->options.inject_error.error;
-immediately = rule->options.inject.immediately;
-error = rule->options.inject.error;
+if (error_rule->once) {
+QSIMPLEQ_REMOVE(>active_rules, error_rule, BlkdebugRule,
active_next);
+remove_rule(error_rule);
+}
-if (rule->options.inject.once) {
-QSIMPLEQ_REMOVE(>active_rules, rule, BlkdebugRule, active_next);
-remove_rule(rule);
-}
+if (error && !immediately) {
+aio_co_schedule(qemu_get_current_aio_context(),
qemu_coroutine_self());
+qemu_coroutine_yield();
+}
-if (error && !immediately) {
-aio_co_schedule(qemu_get_current_aio_context(), qemu_coroutine_self());
-qemu_coroutine_yield();
+ret = -error;
}
-return -error;
+return ret;
}
static int coroutine_fn
--
2.7.4
[Qemu-devel] [PATCH v3 1/3] blkdebug: fix one shot rule processing
If 'once' is specified, the rule should execute just once, regardless if
it is supposed to return an error or not. Take the example where you
want the first IO to an LBA to succeed, but subsequent IOs to fail. You
could either use state transitions, or create two rules, one with
error = 0 and once set to true, and one with a non-zero error.
Signed-off-by: Marc Olson
---
block/blkdebug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 0759452..327049b 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -488,7 +488,7 @@ static int rule_check(BlockDriverState *bs, uint64_t
offset, uint64_t bytes)
}
}
-if (!rule || !rule->options.inject.error) {
+if (!rule) {
return 0;
}
@@ -500,7 +500,7 @@ static int rule_check(BlockDriverState *bs, uint64_t
offset, uint64_t bytes)
remove_rule(rule);
}
-if (!immediately) {
+if (error && !immediately) {
aio_co_schedule(qemu_get_current_aio_context(), qemu_coroutine_self());
qemu_coroutine_yield();
}
--
2.7.4
[Qemu-devel] [PATCH 1/3] unify len and addr type for memory/address APIs
Some address/memory APIs have different type between 'hwaddr addr' and
'int len'. It is very unsafety, espcially some APIs will be passed a non-int
len by caller which might cause overflow quietly.
Below is an potential overflow case:
dma_memory_read(uint32_t len)
-> dma_memory_rw(uint32_t len)
-> dma_memory_rw_relaxed(uint32_t len)
-> address_space_rw(int len) # len overflow
CC: Paolo Bonzini
CC: Peter Crosthwaite
CC: Richard Henderson
Signed-off-by: Li Zhijian
---
exec.c| 49 ---
include/exec/cpu-all.h| 2 +-
include/exec/cpu-common.h | 10 +-
include/exec/memory.h | 20 +--
4 files changed, 41 insertions(+), 40 deletions(-)
diff --git a/exec.c b/exec.c
index bb6170d..05823ae 100644
--- a/exec.c
+++ b/exec.c
@@ -2719,7 +2719,8 @@ static const MemoryRegionOps notdirty_mem_ops = {
};
/* Generate a debug exception if a watchpoint has been hit. */
-static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
+static void check_watchpoint(hwaddr offset, unsigned len,
+ MemTxAttrs attrs, int flags)
{
CPUState *cpu = current_cpu;
CPUClass *cc = CPU_GET_CLASS(cpu);
@@ -2848,10 +2849,10 @@ static const MemoryRegionOps watch_mem_ops = {
};
static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
- MemTxAttrs attrs, uint8_t *buf, int len);
+ MemTxAttrs attrs, uint8_t *buf, hwaddr
len);
static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
- const uint8_t *buf, int len);
-static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
+ const uint8_t *buf, hwaddr len);
+static bool flatview_access_valid(FlatView *fv, hwaddr addr, hwaddr len,
bool is_write, MemTxAttrs attrs);
static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
@@ -3099,9 +3100,10 @@ MemoryRegion *get_system_io(void)
/* physical memory access (slow version, mainly for debug) */
#if defined(CONFIG_USER_ONLY)
int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
-uint8_t *buf, int len, int is_write)
+uint8_t *buf, hwaddr len, int is_write)
{
-int l, flags;
+hwaddr l;
+int flags;
target_ulong page;
void * p;
@@ -3215,7 +3217,7 @@ static bool prepare_mmio_access(MemoryRegion *mr)
static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
MemTxAttrs attrs,
const uint8_t *buf,
- int len, hwaddr addr1,
+ hwaddr len, hwaddr addr1,
hwaddr l, MemoryRegion *mr)
{
uint8_t *ptr;
@@ -3260,7 +3262,7 @@ static MemTxResult flatview_write_continue(FlatView *fv,
hwaddr addr,
/* Called from RCU critical section. */
static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
- const uint8_t *buf, int len)
+ const uint8_t *buf, hwaddr len)
{
hwaddr l;
hwaddr addr1;
@@ -3278,7 +3280,7 @@ static MemTxResult flatview_write(FlatView *fv, hwaddr
addr, MemTxAttrs attrs,
/* Called within RCU critical section. */
MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
MemTxAttrs attrs, uint8_t *buf,
- int len, hwaddr addr1, hwaddr l,
+ hwaddr len, hwaddr addr1, hwaddr l,
MemoryRegion *mr)
{
uint8_t *ptr;
@@ -3321,7 +3323,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr
addr,
/* Called from RCU critical section. */
static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
- MemTxAttrs attrs, uint8_t *buf, int len)
+ MemTxAttrs attrs, uint8_t *buf, hwaddr len)
{
hwaddr l;
hwaddr addr1;
@@ -3334,7 +3336,7 @@ static MemTxResult flatview_read(FlatView *fv, hwaddr
addr,
}
MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
-MemTxAttrs attrs, uint8_t *buf, int len)
+MemTxAttrs attrs, uint8_t *buf, hwaddr len)
{
MemTxResult result = MEMTX_OK;
FlatView *fv;
@@ -3351,7 +3353,7 @@ MemTxResult address_space_read_full(AddressSpace *as,
hwaddr addr,
MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
MemTxAttrs attrs,
-const uint8_t *buf, int len)
+const uint8_t *buf, hwaddr len)
{
MemTxResult
[Qemu-devel] [PATCH 2/3] change load_image() reture type to ssize_t
This patch allow load_iamge to load >=2G file
Signed-off-by: Li Zhijian
---
hw/core/loader.c| 5 +++--
include/hw/loader.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc..0d53229 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -75,9 +75,10 @@ int64_t get_image_size(const char *filename)
/* return the size or -1 if error */
/* deprecated, because caller does not specify buffer size! */
-int load_image(const char *filename, uint8_t *addr)
+ssize_t load_image(const char *filename, uint8_t *addr)
{
-int fd, size;
+int fd;
+ssize_t size;
fd = open(filename, O_RDONLY | O_BINARY);
if (fd < 0)
return -1;
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 67a0af8..49bb189 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -11,7 +11,7 @@
* On error, errno is also set as appropriate.
*/
int64_t get_image_size(const char *filename);
-int load_image(const char *filename, uint8_t *addr); /* deprecated */
+ssize_t load_image(const char *filename, uint8_t *addr); /* deprecated */
ssize_t load_image_size(const char *filename, void *addr, size_t size);
/**load_image_targphys_as:
--
2.7.4
[Qemu-devel] [PATCH 3/3] x86: allow load initrd below 4G for recent linux
a new field xloadflags was added to recent x86 linux, and BIT 1:
XLF_CAN_BE_LOADED_ABOVE_4G is used to tell bootload that where initrd can be
loaded saftly.
Current QEMU always load initrd below below_4g_mem_size which always
less than 4G, so here limit initrd_max to 4G - 1 simply is enough if
this bit is set.
CC: Paolo Bonzini
CC: Richard Henderson
CC: Eduardo Habkost
CC: "Michael S. Tsirkin"
CC: Marcel Apfelbaum
Signed-off-by: Li Zhijian
---
hw/i386/pc.c | 12
1 file changed, 12 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f095725..5e2f83c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -92,6 +92,7 @@
#define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
#define E820_NR_ENTRIES16
+#define XLF_CAN_BE_LOADED_ABOVE_4G_MASK (1 << 1)
struct e820_entry {
uint64_t address;
@@ -916,6 +917,17 @@ static void load_linux(PCMachineState *pcms,
} else {
initrd_max = 0x37ff;
}
+if (protocol >= 0x20c) {
+unsigned int xloadflags = lduw_p(header+0x236);
+if (xloadflags & XLF_CAN_BE_LOADED_ABOVE_4G_MASK) {
+/*
+ * Although kernel allow initrd loading to above 4G, here we
+ * limit initrd_max to 4G -1 due to current QEMU always loads
+ * initrd below pcms->below_4g_mem_size
+ */
+initrd_max = UINT32_MAX;
+}
+}
if (initrd_max >= pcms->below_4g_mem_size - pcmc->acpi_data_size) {
initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1;
--
2.7.4
Re: [Qemu-devel] [RFC/PoC PATCH 1/3] i386: set initrd_max to 4G - 1 to allow up to 4G initrd
On 11/11/18 10:19 PM, Ingo Molnar wrote: > > I might be a bit dense early in the morning, but could you elaborate? > What do you mean by mapping all data areas? > Heh. I need to pack for LPC and get some sleep before my flight lest I'll be denser than depleted uranium; I'll write an explanation tomorrow. -hpa
Re: [Qemu-devel] [RFC/PoC PATCH 1/3] i386: set initrd_max to 4G - 1 to allow up to 4G initrd
* H. Peter Anvin wrote: > > Such an extended header could use a more modern (self-extending) ABI as > > well. > > Yes, although I don't really think it is as much of an issue as it seems at > this point. > > The limit comes from having used a one-byte jump instruction at the beginning; > however, these days that limit is functionally walled. > > It is of course possible to address this if it should become necessary, > however, the current protocol has lasted for 23 years so far and we haven't > run out yet, even with occasional missteps. As such, I don't think we are in a > huge hurry to address this particular aspect. Agreed, fair enough! > In part as a result of this exchange I have spent some time thinking > about the boot protocol and its dependencies, and there is, in fact, a > much more serious problem that needs to be addressed: it is not > currently possible in a forward-compatible way to map all data areas > that may be occupied by bootloader-provided data. The kernel proper has > an advantage here, in that the kernel will by definition always be the > "owner of the protocol" (anything the kernel doesn't know how to map > won't be used by the kernel anyway), but it really isn't a good > situation. So I'm currently trying to think up a way to make that > possible. I might be a bit dense early in the morning, but could you elaborate? What do you mean by mapping all data areas? Thanks, Ingo
Re: [Qemu-devel] [RFC/PoC PATCH 1/3] i386: set initrd_max to 4G - 1 to allow up to 4G initrd
On 11/11/18 8:56 PM, Ingo Molnar wrote: > >> Also note that the ext_ramdisk_image and ext_ramdisk_size are part of >> struct boot_params as opposed to struct setup_header, which means that >> they are not supported when entering via the 16-bit BIOS entry point, >> and I am willing to bet that there will be, ahem, "strangeness" if >> entered via the 32-bit entry point if at least the command line is >> loaded above the 4 GB mark; the initrd should be fine, though. >> >> This is obviosly not an issue in EFI environments, where we enter >> through the EFI handover entry point. >> >> The main reason these were not added to struct setup_header is that >> there are only 24 bytes left in that header and so space is highly >> precious. One way to deal with that if we really, really need to would >> be to add an initrd/initramfs type of setup_data. > > Is there no way to extend that header by making an extended header part > of the payload? > > IIRC that header is small and fixed size to be part of a single sector at > the very beginning of boot images, but accessing any extended header bits > from the payload section shouldn't really be an issue for a modern > bootloader to handle, right? > > Such an extended header could use a more modern (self-extending) ABI as > well. > Yes, although I don't really think it is as much of an issue as it seems at this point. The limit comes from having used a one-byte jump instruction at the beginning; however, these days that limit is functionally walled. It is of course possible to address this if it should become necessary, however, the current protocol has lasted for 23 years so far and we haven't run out yet, even with occasional missteps. As such, I don't think we are in a huge hurry to address this particular aspect. In part as a result of this exchange I have spent some time thinking about the boot protocol and its dependencies, and there is, in fact, a much more serious problem that needs to be addressed: it is not currently possible in a forward-compatible way to map all data areas that may be occupied by bootloader-provided data. The kernel proper has an advantage here, in that the kernel will by definition always be the "owner of the protocol" (anything the kernel doesn't know how to map won't be used by the kernel anyway), but it really isn't a good situation. So I'm currently trying to think up a way to make that possible. -hpa
Re: [Qemu-devel] [RFC PATCH 08/11] target/mips: Add a decodetree stub
> Subject: [RFC PATCH 08/11] target/mips: Add a decodetree stub
There is no plan to use decodetree for MIPS target. MIPS decoding engine is
mostly stable mature code that was well tested over many years, and there is no
point in introducing such drastic change to the code that works.
Thanks,
Aleksandar
From: Philippe Mathieu-Daudé on behalf of
Philippe Mathieu-Daudé
Sent: Monday, November 12, 2018 12:36:19 AM
To: Bastian Koppelmann; Peer Adelt; Richard Henderson
Cc: Philippe Mathieu-Daudé; qemu-devel@nongnu.org; Aurelien Jarno; Aleksandar
Markovic
Subject: [RFC PATCH 08/11] target/mips: Add a decodetree stub
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/Makefile.objs | 8
target/mips/insns.decode| 2 ++
target/mips/translate.c | 7 +++
target/mips/translate.inc.c | 13 +
4 files changed, 30 insertions(+)
create mode 100644 target/mips/insns.decode
create mode 100644 target/mips/translate.inc.c
diff --git a/target/mips/Makefile.objs b/target/mips/Makefile.objs
index 651f36f517..3510835d57 100644
--- a/target/mips/Makefile.objs
+++ b/target/mips/Makefile.objs
@@ -2,3 +2,11 @@ obj-y += translate.o dsp_helper.o op_helper.o lmi_helper.o
helper.o cpu.o
obj-y += gdbstub.o msa_helper.o mips-semi.o
obj-$(CONFIG_SOFTMMU) += machine.o cp0_timer.o
obj-$(CONFIG_KVM) += kvm.o
+
+DECODETREE = $(SRC_PATH)/scripts/decodetree.py
+
+target/mips/decode.inc.c: $(SRC_PATH)/target/mips/insns.decode $(DECODETREE)
+ $(call quiet-command,\
+ $(PYTHON) $(DECODETREE) -o $@ $<, "GEN", $(TARGET_DIR)$@)
+
+target/mips/translate.o: target/mips/decode.inc.c
diff --git a/target/mips/insns.decode b/target/mips/insns.decode
new file mode 100644
index 00..7fbf21cbb9
--- /dev/null
+++ b/target/mips/insns.decode
@@ -0,0 +1,2 @@
+# MIPS32/MIPS64 Instruction Set
+#
diff --git a/target/mips/translate.c b/target/mips/translate.c
index e726f3ec00..560325c563 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27848,6 +27848,8 @@ static void gen_msa(CPUMIPSState *env, DisasContext
*ctx)
}
+#include "translate.inc.c"
+
static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
{
int32_t offset;
@@ -27872,6 +27874,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext
*ctx)
gen_set_label(l1);
}
+/* Transition to the auto-generated decoder. */
+if (decode(ctx, ctx->opcode)) {
+return;
+}
+
op = MASK_OP_MAJOR(ctx->opcode);
rs = (ctx->opcode >> 21) & 0x1f;
rt = (ctx->opcode >> 16) & 0x1f;
diff --git a/target/mips/translate.inc.c b/target/mips/translate.inc.c
new file mode 100644
index 00..69fe78ac89
--- /dev/null
+++ b/target/mips/translate.inc.c
@@ -0,0 +1,13 @@
+/*
+ * MIPS emulation for QEMU - MIPS32 translation routines
+ *
+ * Copyright (c) 2004-2005 Jocelyn Mayer
+ * Copyright (c) 2006 Marius Groeger (FPU operations)
+ * Copyright (c) 2006 Thiemo Seufer (MIPS32R2 support)
+ * Copyright (c) 2018 Philippe Mathieu-Daudé
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+/* Include the auto-generated decoder. */
+#include "decode.inc.c"
--
2.17.2
Re: [Qemu-devel] [RFC/PoC PATCH 1/3] i386: set initrd_max to 4G - 1 to allow up to 4G initrd
* H. Peter Anvin wrote: > On 11/9/18 5:40 AM, Li Zhijian wrote: > > Just noticed that there is a field xloadflags at recent protocol > > 60 Protocol 2.12: (Kernel 3.8) Added the xloadflags field and > > extension fields > > 61 to struct boot_params for loading bzImage and ramdisk > > 62 above 4G in 64bit. > > [snip] > > 617 Field name: xloadflags > > 618 Type: read > > 619 Offset/size: 0x236/2 > > 620 Protocol: 2.12+ > > 621 > > 622 This field is a bitmask. > > 623 > > 624 Bit 0 (read): XLF_KERNEL_64 > > 625 - If 1, this kernel has the legacy 64-bit entry point at > > 0x200. > > 626 > > 627 Bit 1 (read): XLF_CAN_BE_LOADED_ABOVE_4G > > 628 - If 1, kernel/boot_params/cmdline/ramdisk can be above 4G. > > 629 > > > > maybe we can reuse this field and append a new Bit 5 > > XLF_INITRD_MAX_SIZE_4G and increase header version. > > For the old protocol version 2.12+, if XLF_CAN_BE_LOADED_ABOVE_4G is > > set, we can also realize ~4GB initrd is allowed. > > > > bootloader side: > > if protocol >= 2.15 > > if XLF_INITRD_LOAD_BELOW_4G > > support ~4G initrd > > fi > > else if protocol >=2.12 > > if XLF_CAN_BE_LOADED_ABOVE_4G > > support ~4G initrd > > fi > > fi > > > > The two are equivalent. Obviously you have to load above 4 GB if you > have more than 4 GB of initrd. If XLF_CAN_BE_LOADED_ABOVE_4G is not > set, then you most likely are on a 32-bit kernel and there are more > fundamental limits (even if you were to load it above the 2 GB mark, you > would be limited by the size of kernel memory.) > > So, in case you are wondering: the bootloader that broke when setting > the initrd_max field above 2 GB was, of course, Grub. > > So just use XLF_CAN_BE_LOADED_ABOVE_4G. There is no need for a new flag > or new field. That's nice, and that's the best solution! > Also note that the ext_ramdisk_image and ext_ramdisk_size are part of > struct boot_params as opposed to struct setup_header, which means that > they are not supported when entering via the 16-bit BIOS entry point, > and I am willing to bet that there will be, ahem, "strangeness" if > entered via the 32-bit entry point if at least the command line is > loaded above the 4 GB mark; the initrd should be fine, though. > > This is obviosly not an issue in EFI environments, where we enter > through the EFI handover entry point. > > The main reason these were not added to struct setup_header is that > there are only 24 bytes left in that header and so space is highly > precious. One way to deal with that if we really, really need to would > be to add an initrd/initramfs type of setup_data. Is there no way to extend that header by making an extended header part of the payload? IIRC that header is small and fixed size to be part of a single sector at the very beginning of boot images, but accessing any extended header bits from the payload section shouldn't really be an issue for a modern bootloader to handle, right? Such an extended header could use a more modern (self-extending) ABI as well. Thanks, Ingo
Re: [Qemu-devel] [PATCH RFC v7 5/9] migration: fix the multifd code when sending less channels
Hi Juan,
Kindly ping, as this multifd migration topic needs your suggestions. :)
Have a nice day, thanks
Fei
On 11/03/2018 12:33 AM, Dr. David Alan Gilbert wrote:
* Peter Xu (pet...@redhat.com) wrote:
On Fri, Nov 02, 2018 at 11:00:24AM +0800, Fei Li wrote:
On 11/02/2018 10:37 AM, Peter Xu wrote:
On Thu, Nov 01, 2018 at 06:17:11PM +0800, Fei Li wrote:
Set the migration state to "failed" instead of "setup" when failing
to send packet via some channel.
Could you please provide more information in the commit message?
E.g., what will happen if without this patch? Will it crash the
source or stall the source migration or others? Otherwise it's a bit
hard for me to understand what's this patch for.
Sorry for the inadequate description , I was intended to say that when
failing
to do the live migration using multifd, e.g. sending less channels, the src
status displays "setup" when running `info migrate`. I assume we should tell
users that the "Migration status" is "failed" now (and along with the
failure reason).
The current src status when failed inmultifd_new_send_channel_async()：
(qemu) migrate_set_capability x-multifd on
(qemu) migrate_set_parameter x-multifd-channels 4
(qemu) migrate -d tcp:192.168.190.98:
(qemu) qemu-system-x86_64: failed in multifd_new_send_channel_async due to
...
(qemu) info migrate
globals:
store-global-state: on
only-migratable: off
send-configuration: on
send-section-footer: on
decompress-error-check: on
capabilities: xbzrle: off rdma-pin-all: off auto-converge: off zero-blocks:
off compress: off events: off postcopy-ram: off x-colo: off release-ram: off
block: off return-path: off pause-before-switchover: off x-multifd: on
dirty-bitmaps: off postcopy-blocktime: off late-block-activate: off
Migration status: setup
total time: 0 milliseconds
Thanks for the information.
I had a quick look. For now we do this:
multifd_save_setup (without waiting for channels to be ready)
create thread migration_thread
(in thread)
ram_save_setup
multifd_send_sync_main (wait for the channels)
The thing is that we didn't get the notification when one of the
multifd channel is failed. IMHO instead of setting the global
migration state in a per-channel function, we should just report the
error upwards, then the main thread should decide how to change the
state machine of the migration.
Best to wait for Juan on that; I've got vague memories that reporting
errors among the threads was a bit tricky.
Dave
And we have set it in migrate_set_error() after all so the main thread
should be able to know somehow (though IMHO I'll even prefer to have a
per-channel variable to keep the state of the channel, then the
per-channel functions won't touch any globals which offers better
isolation).
I'm not sure how Juan thinks about it, but I'd prefer some work to
provide such isolation and also some mechanism to allow the main
thread to detect the per-channel errors not only during setup phase
but also during the migration (e.g., when network is suddenly down).
Then we don't touch any globals (e.g., we shouldn't call
migrate_get_current in any per-channel function like
multifd_new_send_channel_async).
Normally I would prefer to not touch global states in feature specific
code path, but I'd like to know the problem more first...
Thanks,
Cc: Peter Xu
Signed-off-by: Fei Li
---
migration/ram.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 4db3b3e8f4..c84d164fc8 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1072,6 +1072,7 @@ out:
static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
{
MultiFDSendParams *p = opaque;
+MigrationState *s = migrate_get_current();
QIOChannel *sioc = QIO_CHANNEL(qio_task_get_source(task));
Error *local_err = NULL;
@@ -1083,6 +1084,7 @@ static void multifd_new_send_channel_async(QIOTask *task,
gpointer opaque)
if (multifd_save_cleanup(_err) != 0) {
migrate_set_error(migrate_get_current(), local_err);
}
+migrate_set_state(>state, s->state, MIGRATION_STATUS_FAILED);
} else {
p->c = QIO_CHANNEL(sioc);
qio_channel_set_delay(p->c, false);
--
2.13.7
Regards,
Regards,
--
Peter Xu
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] ppc/spapr: Receive and store device tree blob from SLOF
On 12/11/2018 05:10, Greg Kurz wrote:
> Hi Alexey,
>
> Just a few remarks. See below.
>
> On Thu, 8 Nov 2018 12:44:06 +1100
> Alexey Kardashevskiy wrote:
>
>> SLOF receives a device tree and updates it with various properties
>> before switching to the guest kernel and QEMU is not aware of any changes
>> made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
>> sense to pass the SLOF final device tree to QEMU to let it implement
>> RTAS related tasks better, such as PCI host bus adapter hotplug.
>>
>> Specifially, now QEMU can find out the actual XICS phandle (for PHB
>> hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
>> assisted NMI - FWNMI).
>>
>> This stores the initial DT blob in the sPAPR machine and replaces it
>> in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
>>
>> This adds an @update_dt_enabled machine property to allow backward
>> migration.
>>
>> SLOF already has a hypercall since
>> https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
>>
>> Signed-off-by: Alexey Kardashevskiy
>> ---
>> include/hw/ppc/spapr.h | 7 ++-
>> hw/ppc/spapr.c | 29 -
>> hw/ppc/spapr_hcall.c | 32
>> hw/ppc/trace-events| 2 ++
>> 4 files changed, 68 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index ad4d7cfd97..f5dcaf44cb 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -100,6 +100,7 @@ struct sPAPRMachineClass {
>>
>> /*< public >*/
>> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
>> +bool update_dt_enabled;/* enable KVMPPC_H_UPDATE_DT */
>> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
>> bool pre_2_10_has_unused_icps;
>> bool legacy_irq_allocation;
>> @@ -136,6 +137,9 @@ struct sPAPRMachineState {
>> int vrma_adjust;
>> ssize_t rtas_size;
>> void *rtas_blob;
>> +uint32_t fdt_size;
>> +uint32_t fdt_initial_size;
>
> I don't quite see the purpose of fdt_initial_size... it seems to be only
> used to print a trace.
Ah, lost in rebase. The purpose was to test if the new device tree has
not grown too much.
>
>> +void *fdt_blob;
>> long kernel_size;
>> bool kernel_le;
>> uint32_t initrd_base;
>> @@ -462,7 +466,8 @@ struct sPAPRMachineState {
>> #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
>> /* Client Architecture support */
>> #define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
>> -#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
>> +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
>> +#define KVMPPC_HCALL_MAXKVMPPC_H_UPDATE_DT
>>
>> typedef struct sPAPRDeviceTreeUpdateHeader {
>> uint32_t version_id;
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index c08130facb..5e2d4d211c 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1633,7 +1633,10 @@ static void spapr_machine_reset(void)
>> /* Load the fdt */
>> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
>> cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
>> -g_free(fdt);
>> +g_free(spapr->fdt_blob);
>> +spapr->fdt_size = fdt_totalsize(fdt);
>> +spapr->fdt_initial_size = spapr->fdt_size;
>> +spapr->fdt_blob = fdt;
>
> Hmm... It looks weird to store state in a reset handler. I'd rather zeroe
> both fdt_blob and fdt_size here.
The device tree is built from the reset handler and the idea is that we
want to always have some tree in the machine.
>
>>
>> /* Set up the entry state */
>> spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
>> @@ -1887,6 +1890,27 @@ static const VMStateDescription vmstate_spapr_irq_map
>> = {
>> },
>> };
>>
>> +static bool spapr_dtb_needed(void *opaque)
>> +{
>> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
>> +
>> +return smc->update_dt_enabled;
>
> This means we always migrate the fdt, even if migration occurs before
> SLOF could call KVMPPC_H_UPDATE_DT.
>
> With spapr->fdt_blob set to NULL on reset, a better check would be:
>
> sPAPRMachineState *spapr = SPAPR_MACHINE(opaque);
>
> return smc->update_dt_enabled && spapr->fdt_blob;
>
>> +}
>> +
>> +static const VMStateDescription vmstate_spapr_dtb = {
>> +.name = "spapr_dtb",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +.needed = spapr_dtb_needed,
>> +.fields = (VMStateField[]) {
>> +VMSTATE_UINT32(fdt_initial_size, sPAPRMachineState),
>> +VMSTATE_UINT32(fdt_size, sPAPRMachineState),
>> +VMSTATE_VBUFFER_ALLOC_UINT32(fdt_blob, sPAPRMachineState, 0, NULL,
>> + fdt_size),
>> +VMSTATE_END_OF_LIST()
>> +},
>> +};
>> +
>> static const VMStateDescription vmstate_spapr = {
>> .name = "spapr",
>> .version_id = 3,
>> @@ -1915,6 +1939,7 @@ static const VMStateDescription
[Qemu-devel] [PATCH V6 6/6] pvpanic : update pvpanic document
Add mmio support info in docs/specs/pvpanic.txt. Signed-off-by: Peng Hao --- docs/specs/pvpanic.txt | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt index c7bbacc..5d8e9dc 100644 --- a/docs/specs/pvpanic.txt +++ b/docs/specs/pvpanic.txt @@ -1,7 +1,7 @@ PVPANIC DEVICE == -pvpanic device is a simulated ISA device, through which a guest panic +pvpanic device is a simulated device, through which a guest panic event is sent to qemu, and a QMP event is generated. This allows management apps (e.g. libvirt) to be notified and respond to the event. @@ -9,6 +9,10 @@ The management app has the option of waiting for GUEST_PANICKED events, and/or polling for guest-panicked RunState, to learn when the pvpanic device has fired a panic event. +When pvpanic device is implemented as a ISA device, it supports IOPORT +mode. If pvpanic device supports MMIO mode, it will be implemented as +a SYSBUS device. + ISA Interface - @@ -19,6 +23,13 @@ Software should set only bits both itself and the device recognize. Currently, only bit 0 is recognized, setting it indicates a guest panic has happened. +SYSBUS Interface +-- + +It is basically the same as ISA interface except that it uses MMIO. Pvpanic exposes +a address space region 0x0906--0x09060001 in arm virt machine. +Currently only the first byte is used. + ACPI Interface -- -- 1.8.3.1
[Qemu-devel] [PATCH V6 4/6] hw/arm/virt: Use the pvpanic device
Add pvpanic device in arm virt machine.
Signed-off-by: Peng Hao
Signed-off-by: Philippe Mathieu-Daudé
---
default-configs/arm-softmmu.mak | 1 +
hw/arm/virt.c | 21 +
include/hw/arm/virt.h | 1 +
3 files changed, 23 insertions(+)
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 2420491..50345df 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -159,3 +159,4 @@ CONFIG_PCI_DESIGNWARE=y
CONFIG_STRONGARM=y
CONFIG_HIGHBANK=y
CONFIG_MUSICPAL=y
+CONFIG_PVPANIC=y
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9f67782..c4f29c8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -59,6 +59,7 @@
#include "qapi/visitor.h"
#include "standard-headers/linux/input.h"
#include "hw/arm/smmuv3.h"
+#include "hw/misc/pvpanic.h"
#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -143,6 +144,7 @@ static const MemMapEntry a15memmap[] = {
[VIRT_GPIO] = { 0x0903, 0x1000 },
[VIRT_SECURE_UART] ={ 0x0904, 0x1000 },
[VIRT_SMMU] = { 0x0905, 0x0002 },
+[VIRT_PVPANIC] ={ 0x0907, 0x0002 },
[VIRT_MMIO] = { 0x0a00, 0x0200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
[VIRT_PLATFORM_BUS] = { 0x0c00, 0x0200 },
@@ -190,6 +192,23 @@ static bool cpu_type_valid(const char *cpu)
return false;
}
+static void create_pvpanic_device(const VirtMachineState *vms)
+{
+char *nodename;
+hwaddr base = vms->memmap[VIRT_PVPANIC].base;
+hwaddr size = vms->memmap[VIRT_PVPANIC].size;
+
+sysbus_create_simple(TYPE_PVPANIC_MMIO, base, NULL);
+
+nodename = g_strdup_printf("/pvpanic-mmio@%" PRIx64, base);
+qemu_fdt_add_subnode(vms->fdt, nodename);
+qemu_fdt_setprop_string(vms->fdt, nodename,
+"compatible", "qemu,pvpanic-mmio");
+qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
+ 2, base, 2, size);
+g_free(nodename);
+}
+
static void create_fdt(VirtMachineState *vms)
{
void *fdt = create_device_tree(>fdt_size);
@@ -1531,6 +1550,8 @@ static void machvirt_init(MachineState *machine)
create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
+create_pvpanic_device(vms);
+
create_gic(vms, pic);
fdt_add_pmu_nodes(vms);
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 4cc57a7..937c124 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -66,6 +66,7 @@ enum {
VIRT_GIC_REDIST,
VIRT_GIC_REDIST2,
VIRT_SMMU,
+VIRT_PVPANIC,
VIRT_UART,
VIRT_MMIO,
VIRT_RTC,
--
1.8.3.1
[Qemu-devel] [PATCH V6 5/6] hw/arm/virt: add pvpanic device in virt acpi table
add pvpanic device in virt acpi table, so when kenrel command line uses
acpi=force, kernel can get info from acpi table in aarch64.
Signed-off-by: Peng Hao
---
hw/arm/virt-acpi-build.c | 16
1 file changed, 16 insertions(+)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 5785fb6..d126cee 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -61,6 +61,21 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
}
}
+static void acpi_dsdt_add_pvpanic(Aml *scope, const MemMapEntry
*pvpanic_memmap)
+{
+Aml *dev = aml_device("PANC");
+aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+Aml *crs = aml_resource_template();
+aml_append(crs, aml_memory32_fixed(pvpanic_memmap->base,
+ pvpanic_memmap->size, AML_READ_WRITE));
+
+aml_append(dev, aml_name_decl("_CRS", crs));
+
+aml_append(scope, dev);
+}
+
static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
uint32_t uart_irq)
{
@@ -770,6 +785,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
acpi_dsdt_add_cpus(scope, vms->smp_cpus);
acpi_dsdt_add_uart(scope, [VIRT_UART],
(irqmap[VIRT_UART] + ARM_SPI_BASE));
+acpi_dsdt_add_pvpanic(scope, [VIRT_PVPANIC]);
acpi_dsdt_add_flash(scope, [VIRT_FLASH]);
acpi_dsdt_add_fw_cfg(scope, [VIRT_FW_CFG]);
acpi_dsdt_add_virtio(scope, [VIRT_MMIO],
--
1.8.3.1
[Qemu-devel] [PATCH V6 0/5] add pvpanic mmio support
The first patches are simple cleanups: - patch 1 move the pvpanic device with the 'ocmmon objects' so we compile it once for the x86/arm/aarch64 archs, - patch 2 simply renames ISA fields/definitions to generic ones. Then instead of add/use the MMIO pvpanic device in the virt machine in an unique patch, I split it in two distinct patches: - patch 3 uses Peng Hao's work, but add the MMIO interface to the existing device (no logical change). - patch 4 is Peng Hao's work in the virt machine (no logical change). - patch 5 add pvpanic device in acpi table in virt machine v2 from Peng Hao is: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg03433.html v3 --> v4 patch 1,2 no modification. patch 3, add TYPE_PANIC_MMIO for distinguishing different bus device, virt + isa_pvpanic will abnormally terminate virtual machine. patch 4, "pvpanic,mmio" --> "qemu,pvpanic-mmio". patch 5, newly added. v4 --> v5 patch 1,2 no modification. patch 3 delete PvpanicCommonState structure. patch 4 VIRT_PVPANIC_MMIO --> VIRT_PVPANIC correct VIRT_PVPANIC's overlap start address patch 5 no modification. v5 --> v6 add document. the kernel part of the series: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/log/?h=char-misc-testing misc/pvpanic: remove a redundant comma misc/pvpanic: convert to SPDX license tags misc/pvpanic: change header file sort style misc/pvpanic: remove unnecessary header file misc/pvpanic : break dependency on ACPI misc/pvpanic : grouping ACPI related stuff misc/pvpanic: add support to get pvpanic device info FDT dt-bindings: misc/pvpanic: add document for pvpanic-mmio misc/pvpanic: add MMIO support misc/pvpanic: simplify the code using acpi_dev_resource_io pvpanic: move pvpanic to misc as common driver Philippe Mathieu-Daudé (2): hw/misc/pvpanic: Build the pvpanic device in $(common-obj) hw/misc/pvpanic: Cosmetic renaming Peng Hao (3): hw/misc/pvpanic: Add the MMIO interface hw/arm/virt: Use the pvpanic device hw/arm/virt: add pvpanic device in virt acpi table pvpanic : update pvpanic document default-configs/aarch64-softmmu.mak | 1 + hw/arm/virt-acpi-build.c| 16 hw/arm/virt.c | 21 ++ hw/misc/Makefile.objs | 2 +- hw/misc/pvpanic.c | 78 ++--- include/hw/arm/virt.h | 1 + include/hw/misc/pvpanic.h | 2 + docs/specs/pvpanic.txt | 13 - 8 files changed, 118 insertions(+), 17 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH V6 1/6] hw/misc/pvpanic: Build the pvpanic device in $(common-obj)
From: Philippe Mathieu-Daudé The 'pvpanic' ISA device can be use by any machine with an ISA bus. Signed-off-by: Philippe Mathieu-Daudé --- hw/misc/Makefile.objs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs index 6d50b03..24997d6 100644 --- a/hw/misc/Makefile.objs +++ b/hw/misc/Makefile.objs @@ -8,6 +8,7 @@ common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o common-obj-$(CONFIG_EDU) += edu.o common-obj-$(CONFIG_PCA9552) += pca9552.o +common-obj-$(CONFIG_PVPANIC) += pvpanic.o common-obj-y += unimp.o common-obj-$(CONFIG_FW_CFG_DMA) += vmcoreinfo.o @@ -70,7 +71,6 @@ obj-$(CONFIG_IOTKIT_SECCTL) += iotkit-secctl.o obj-$(CONFIG_IOTKIT_SYSCTL) += iotkit-sysctl.o obj-$(CONFIG_IOTKIT_SYSINFO) += iotkit-sysinfo.o -obj-$(CONFIG_PVPANIC) += pvpanic.o obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o obj-$(CONFIG_AUX) += auxbus.o obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o -- 1.8.3.1
[Qemu-devel] [PATCH V6 3/6] hw/misc/pvpanic: Add the MMIO interface
Add pvpanic new type "TYPE_PVPANIC_MMIO"
Signed-off-by: Peng Hao
Signed-off-by: Philippe Mathieu-Daudé
---
hw/misc/pvpanic.c | 81 +--
include/hw/misc/pvpanic.h | 1 +
2 files changed, 65 insertions(+), 17 deletions(-)
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index dd3aef2..5d0fbc6 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -2,10 +2,12 @@
* QEMU simulated pvpanic device.
*
* Copyright Fujitsu, Corp. 2013
+ * Copyright (c) 2018 ZTE Ltd.
*
* Authors:
* Wen Congyang
* Hu Tao
+ * Peng Hao
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -25,9 +27,6 @@
/* The pv event value */
#define PVPANIC_PANICKED(1 << PVPANIC_F_PANICKED)
-#define PVPANIC(obj)\
-OBJECT_CHECK(PVPanicState, (obj), TYPE_PVPANIC)
-
static void handle_event(int event)
{
static bool logged;
@@ -45,30 +44,50 @@ static void handle_event(int event)
#include "hw/isa/isa.h"
-typedef struct PVPanicState {
-/* private */
-ISADevice isadev;
+/* PVPanicISAState for ISA device and
+ * use ioport.
+ */
+typedef struct PVPanicISAState {
+ /* private */
+ ISADevice isadev;
+ uint16_t ioport;
/* public */
MemoryRegion mr;
-uint16_t ioport;
-} PVPanicState;
+} PVPanicISAState;
+
+/* PVPanicMMIOState for sysbus device and
+ * use mmio.
+ */
+typedef struct PVPanicMMIOState {
+/* private */
+SysBusDevice busdev;
+
+ /* public */
+MemoryRegion mr;
+} PVPanicMMIOState;
+
+#define PVPANIC_ISA(obj)\
+OBJECT_CHECK(PVPanicISAState, (obj), TYPE_PVPANIC)
+
+#define PVPANIC_MMIO(obj)\
+OBJECT_CHECK(PVPanicMMIOState, (obj), TYPE_PVPANIC_MMIO)
/* return supported events on read */
-static uint64_t pvpanic_ioport_read(void *opaque, hwaddr addr, unsigned size)
+static uint64_t pvpanic_read(void *opaque, hwaddr addr, unsigned size)
{
return PVPANIC_PANICKED;
}
-static void pvpanic_ioport_write(void *opaque, hwaddr addr, uint64_t val,
+static void pvpanic_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
{
handle_event(val);
}
static const MemoryRegionOps pvpanic_ops = {
-.read = pvpanic_ioport_read,
-.write = pvpanic_ioport_write,
+.read = pvpanic_read,
+.write = pvpanic_write,
.impl = {
.min_access_size = 1,
.max_access_size = 1,
@@ -77,15 +96,16 @@ static const MemoryRegionOps pvpanic_ops = {
static void pvpanic_isa_initfn(Object *obj)
{
-PVPanicState *s = PVPANIC(obj);
+PVPanicISAState *s = PVPANIC_ISA(obj);
-memory_region_init_io(>mr, OBJECT(s), _ops, s, "pvpanic", 1);
+memory_region_init_io(>mr, OBJECT(s), _ops, s,
+ TYPE_PVPANIC, 1);
}
static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
{
ISADevice *d = ISA_DEVICE(dev);
-PVPanicState *s = PVPANIC(dev);
+PVPanicISAState *s = PVPANIC_ISA(dev);
FWCfgState *fw_cfg = fw_cfg_find();
uint16_t *pvpanic_port;
@@ -102,7 +122,7 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error
**errp)
}
static Property pvpanic_isa_properties[] = {
-DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicState, ioport, 0x505),
+DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicISAState, ioport, 0x505),
DEFINE_PROP_END_OF_LIST(),
};
@@ -118,14 +138,41 @@ static void pvpanic_isa_class_init(ObjectClass *klass,
void *data)
static TypeInfo pvpanic_isa_info = {
.name = TYPE_PVPANIC,
.parent= TYPE_ISA_DEVICE,
-.instance_size = sizeof(PVPanicState),
+.instance_size = sizeof(PVPanicISAState),
.instance_init = pvpanic_isa_initfn,
.class_init= pvpanic_isa_class_init,
};
+
+static void pvpanic_mmio_initfn(Object *obj)
+{
+PVPanicMMIOState *s = PVPANIC_MMIO(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+memory_region_init_io(>mr, OBJECT(s), _ops, s,
+ TYPE_PVPANIC_MMIO, 2);
+sysbus_init_mmio(sbd, >mr);
+}
+
+static void pvpanic_mmio_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static TypeInfo pvpanic_mmio_info = {
+.name = TYPE_PVPANIC_MMIO,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(PVPanicMMIOState),
+.instance_init = pvpanic_mmio_initfn,
+.class_init= pvpanic_mmio_class_init,
+};
+
static void pvpanic_register_types(void)
{
type_register_static(_isa_info);
+type_register_static(_mmio_info);
}
type_init(pvpanic_register_types)
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index 1ee071a..19c0fbb 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -17,6 +17,7 @@
#define TYPE_PVPANIC "pvpanic"
#define PVPANIC_IOPORT_PROP "ioport"
+#define
[Qemu-devel] [PATCH V6 2/6] hw/misc/pvpanic: Cosmetic renaming
From: Philippe Mathieu-Daudé
To ease the MMIO device addition in the next patch, rename:
- ISA_PVPANIC_DEVICE -> PVPANIC (this just returns a generic Object),
- ISADevice parent_obj -> isadev,
- MemoryRegion io -> mr.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/misc/pvpanic.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 9d8961b..dd3aef2 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -25,7 +25,7 @@
/* The pv event value */
#define PVPANIC_PANICKED(1 << PVPANIC_F_PANICKED)
-#define ISA_PVPANIC_DEVICE(obj)\
+#define PVPANIC(obj)\
OBJECT_CHECK(PVPanicState, (obj), TYPE_PVPANIC)
static void handle_event(int event)
@@ -46,9 +46,11 @@ static void handle_event(int event)
#include "hw/isa/isa.h"
typedef struct PVPanicState {
-ISADevice parent_obj;
+/* private */
+ISADevice isadev;
-MemoryRegion io;
+/* public */
+MemoryRegion mr;
uint16_t ioport;
} PVPanicState;
@@ -75,15 +77,15 @@ static const MemoryRegionOps pvpanic_ops = {
static void pvpanic_isa_initfn(Object *obj)
{
-PVPanicState *s = ISA_PVPANIC_DEVICE(obj);
+PVPanicState *s = PVPANIC(obj);
-memory_region_init_io(>io, OBJECT(s), _ops, s, "pvpanic", 1);
+memory_region_init_io(>mr, OBJECT(s), _ops, s, "pvpanic", 1);
}
static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp)
{
ISADevice *d = ISA_DEVICE(dev);
-PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
+PVPanicState *s = PVPANIC(dev);
FWCfgState *fw_cfg = fw_cfg_find();
uint16_t *pvpanic_port;
@@ -96,7 +98,7 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error
**errp)
fw_cfg_add_file(fw_cfg, "etc/pvpanic-port", pvpanic_port,
sizeof(*pvpanic_port));
-isa_register_ioport(d, >io, s->ioport);
+isa_register_ioport(d, >mr, s->ioport);
}
static Property pvpanic_isa_properties[] = {
--
1.8.3.1
Re: [Qemu-devel] [PATCH v2 0/5] migration: improve multithreads
Hi, Ping... On 11/6/18 8:20 PM, guangrong.x...@gmail.com wrote: From: Xiao Guangrong Changelog in v2: These changes are based on Paolo's suggestion: 1) rename the lockless multithreads model to threaded workqueue 2) hugely improve the internal design, that make all the request be a large array, properly partition it, assign requests to threads respectively and use bitmaps to sync up threads and the submitter, after that ptr_ring and spinlock are dropped 3) introduce event wait for the submitter These changes are based on Emilio's review: 4) make more detailed description for threaded workqueue 5) add a benchmark for threaded workqueue The previous version can be found at https://marc.info/?l=kvm=153968821910007=2 There's the simple performance measurement comparing these two versions, the environment is the same as we listed in the previous version. Use 8 threads to compress the data in the source QEMU - with compress-wait-thread = off total timebusy-ratio -- v11250660.38 v21204440.35 - with compress-wait-thread = on total timebusy-ratio -- v11644260 v21426090 The v2 win slightly. Xiao Guangrong (5): bitops: introduce change_bit_atomic util: introduce threaded workqueue migration: use threaded workqueue for compression migration: use threaded workqueue for decompression tests: add threaded-workqueue-bench include/qemu/bitops.h | 13 + include/qemu/threaded-workqueue.h | 94 +++ migration/ram.c | 538 ++ tests/Makefile.include| 5 +- tests/threaded-workqueue-bench.c | 256 ++ util/Makefile.objs| 1 + util/threaded-workqueue.c | 466 + 7 files changed, 1030 insertions(+), 343 deletions(-) create mode 100644 include/qemu/threaded-workqueue.h create mode 100644 tests/threaded-workqueue-bench.c create mode 100644 util/threaded-workqueue.c
Re: [Qemu-devel] [PATCH] virtio-net: support RSC v4/v6 tcp traffic for Windows HCK
On Sun, Nov 11, 2018 at 12:18:54PM +0200, Yuri Benditovich wrote:
> > @@ -66,12 +143,16 @@ typedef struct VirtIONet {
> > VirtIONetQueue *vqs;
> > VirtQueue *ctrl_vq;
> > NICState *nic;
> > + QTAILQ_HEAD(, NetRscChain) rsc_chains;
>
> what exactly happens with these chains on migration?
>
>
> This feature (software implementation of RSC in QEMU) is intended to be used
> in
> the environment of certification tests which never uses migration.
Should this feature disable migration then?
> These chains
> and accumulated segments (if any) are lost in case of migration. I'll add the
> note about it
> in commit's message.
>
If it's a functional limitation it belongs in a code comment
not in the commit log.
>
>
> > uint32_t tx_timeout;
> > int32_t tx_burst;
> > uint32_t has_vnet_hdr;
> > size_t host_hdr_len;
> > size_t guest_hdr_len;
> > uint64_t host_features;
> > + uint32_t rsc_timeout;
> > + uint8_t rsc4_enabled;
> > + uint8_t rsc6_enabled;
> > uint8_t has_ufo;
> > uint32_t mergeable_rx_bufs;
> > uint8_t promisc;
> > diff --git a/include/net/eth.h b/include/net/eth.h
> > index e6dc8a7ba0..7f45c678e7 100644
> > --- a/include/net/eth.h
> > +++ b/include/net/eth.h
> > @@ -177,6 +177,8 @@ struct tcp_hdr {
> > #define TH_PUSH 0x08
> > #define TH_ACK 0x10
> > #define TH_URG 0x20
> > +#define TH_ECE 0x40
> > +#define TH_CWR 0x80
> > u_short th_win; /* window */
> > u_short th_sum; /* checksum */
> > u_short th_urp; /* urgent pointer */
> > diff --git a/include/standard-headers/linux/virtio_net.h b/include/
> standard-headers/linux/virtio_net.h
> > index 260c3681d7..0d8658c06a 100644
> > --- a/include/standard-headers/linux/virtio_net.h
> > +++ b/include/standard-headers/linux/virtio_net.h
> > @@ -57,6 +57,10 @@
> > * Steering */
> > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> >
> > +#define VIRTIO_NET_F_RSC_EXT 38
>
> Should it be VIRTIO_NET_F_GUEST_RSC_EXT ?
>
>
> IMO, not. In the spec the name of the feature is VIRTIO_NET_F_RSC_EXT and it
> is
> actually host feature
> and its effect is how the host sets the fields in the header.
Isn't the same true for GUEST_GSO?
>
>
> > +#define VIRTIO_NET_F_GUEST_RSC4_DONT_USE 41 /* reserved */
> > +#define VIRTIO_NET_F_GUEST_RSC6_DONT_USE 42 /* reserved */
> > +
> > #define VIRTIO_NET_F_STANDBY 62 /* Act as standby for another
> device
> > * with the same MAC.
> > */
> > @@ -104,6 +108,7 @@ struct virtio_net_config {
> > struct virtio_net_hdr_v1 {
> > #define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start,
> csum_offset */
> > #define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */
> > +#define VIRTIO_NET_HDR_F_RSC_INFO 4 /* rsc_ext data in csum_
> fields */
> > uint8_t flags;
> > #define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame
> */
> > #define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP
> (TSO) */
> > @@ -118,6 +123,9 @@ struct virtio_net_hdr_v1 {
> > __virtio16 num_buffers; /* Number of merged rx buffers */
> > };
> >
> > +#define rsc_ext_num_packets csum_start
> > +#define rsc_ext_num_dupacks csum_offset
>
> I would prefer an inline function to set the field, or a union.
>
>
> > +
> > #ifndef VIRTIO_NET_NO_LEGACY
> > /* This header comes first in the scatter-gather list.
> > * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>
> This part needs to get into the Linux header. Pls post there.
> Until it does you can put it in virtio-net.c
>
>
> > --
> > 2.17.1
>
Re: [Qemu-devel] [PATCH] target/xtensa: drop num_[core_]regs from dc232b/dc233c configs
On Sun, Nov 11, 2018 at 3:53 PM Philippe Mathieu-Daudé wrote: > > Hi Max, > > On Thu, Nov 1, 2018 at 12:02 AM Richard Henderson > wrote: > > > > On 10/31/18 9:35 PM, Max Filippov wrote: > > > gdb_regmap::num_core_regs field is initialized incorrectly in the dc232b > > > and dc233c configurations. As a result the following message is > > > displayed when attaching to an xtensa linux-user process: > > > > > > "Register 105 is not available", > > > > > > and gdb is unable to control the inferior. > > > > > > Now that xtensa_count_regs does the right thing, remove manual > > > initialization of these fields from the affected configurations and let > > > xtensa_finalize_config initialize them. > > > > > > Cc: qemu-sta...@nongnu.org > > > Signed-off-by: Max Filippov > > > --- > > > target/xtensa/core-dc232b.c | 2 -- > > > target/xtensa/core-dc233c.c | 2 -- > > > 2 files changed, 4 deletions(-) > > > > Reviewed-by: Richard Henderson > > Can we include this patch for the 3.2 release? I've found that this version is wrong (register lists are not terminated for these two cores), so I sent v2. After that I've found that it's still wrong, i.e. the register 105 is still not available with v2. I dug a bit deeper and it looks like register handling should probably be the same for both system and linux-user cases. I'll post v3 after some more testing in a few days. -- Thanks. -- Max
[Qemu-devel] [PATCH 0/2] virtio-9p: qmp interface for set/query io throttle for fsdev devices
These patches provide the qmp interface, to set/query the io throttle status of the all fsdev devices that are present in a vm. Some of the patches also remove the duplicate code that was present in block and fsdev files. Zhide Xie (2): fsdev-qmp: qmp interface for set/query io throttle for fsdev devices. fsdev-qmp: fix coding style issue Makefile| 20 +++- Makefile.objs | 8 ++ block/throttle.c | 6 +- blockdev.c | 96 + fsdev/qemu-fsdev-dummy.c| 11 ++ fsdev/qemu-fsdev-throttle.c| 144 +- fsdev/qemu-fsdev-throttle.h | 6 +- fsdev/qemu-fsdev.c| 29 ++ hmp-commands-info.hx | 15 +++ hmp-commands.hx | 15 +++ hmp.c | 83 +-- hmp.h | 4 + include/qemu/throttle-options.h| 3 +- include/qemu/throttle.h | 4 +- include/qemu/typedefs.h | 1 + monitor.c | 4 + qapi/block-core.json | 122 +- qapi/fsdev.json | 96 + qapi/qapi-schema.json | 1 + qapi/tlimits.json | 89 qmp.c | 12 +++ util/throttle.c| 224 ++-- 22 files changed, 639 insertions(+), 354 deletions(-) create mode 100644 qapi/fsdev.json create mode 100644 qapi/tlimits.json -- 1.8.3.1
Re: [Qemu-devel] [PATCH 2/2] virtio-9p: fix coding style issue
fix two coding style issue Signed-off-by: x00390961 mailto:xiezh...@huawei.com>> --- fsdev/qemu-fsdev-throttle.c | 2 +- include/qemu/throttle-options.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c index 7d313f2..fa2b0c8 100644 --- a/fsdev/qemu-fsdev-throttle.c +++ b/fsdev/qemu-fsdev-throttle.c @@ -116,7 +116,7 @@ void fsdev_get_io_throttle(FsThrottle *fst, FsdevIOThrottle **fs9pcfg, ThrottleLimits *tlimits; FsdevIOThrottle *fscfg = g_malloc(sizeof(*fscfg)); tlimits = qapi_FsdevIOThrottle_base(fscfg); - fscfg->has_id = true; +fscfg->has_id = true; fscfg->id = g_strdup(fsdevice); throttle_config_to_limits(, tlimits); *fs9pcfg = fscfg; diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h index 7134690..3eb1825 100644 --- a/include/qemu/throttle-options.h +++ b/include/qemu/throttle-options.h @@ -111,6 +111,5 @@ .type = QEMU_OPT_NUMBER,\ .help = "when limiting by iops max size of an I/O in bytes",\ } - void throttle_parse_options(ThrottleConfig *, QemuOpts *); #endif -- 1.8.3.1
[Qemu-devel] [PATCH 1/2] virtio-9p: qmp interface to set/query io throttle for fsdev devices
This patch provide qmp interface to set/query io throttle for fsdev devices. This patch include following work: 1. port Pradeep Jagadeesh's patches, details please review http://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00173.html 2. fix two issue: (1). qmp set io throttle code dump when qemu start CLI not include io throttle params (2). issue Berto comment at: http://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03150.html 3. resolve back-compat issue Eric comment at: http://lists.gnu.org/archive/html/qemu-devel/2017-10/msg03149.html Signed-off-by: x00390961 mailto:xiezh...@huawei.com>> --- Makefile | 20 +++- Makefile.objs| 8 ++ block/throttle.c| 6 +- blockdev.c | 96 + fsdev/qemu-fsdev-dummy.c | 11 ++ fsdev/qemu-fsdev-throttle.c | 144 +- fsdev/qemu-fsdev-throttle.h | 6 +- fsdev/qemu-fsdev.c | 29 ++ hmp-commands-info.hx | 15 +++ hmp-commands.hx | 15 +++ hmp.c | 83 +-- hmp.h | 4 + include/qemu/throttle-options.h | 4 +- include/qemu/throttle.h | 4 +- include/qemu/typedefs.h| 1 + monitor.c | 4 + qapi/block-core.json | 122 +- qapi/fsdev.json | 96 + qapi/qapi-schema.json | 1 + qapi/tlimits.json | 89 qmp.c| 12 +++ util/throttle.c | 224 ++-- 22 files changed, 640 insertions(+), 354 deletions(-) create mode 100644 qapi/fsdev.json create mode 100644 qapi/tlimits.json diff --git a/Makefile b/Makefile index f294718..9ae2460 100644 --- a/Makefile +++ b/Makefile @@ -94,6 +94,7 @@ GENERATED_FILES += qapi/qapi-types-block-core.h qapi/qapi-types-block-core.c GENERATED_FILES += qapi/qapi-types-block.h qapi/qapi-types-block.c GENERATED_FILES += qapi/qapi-types-char.h qapi/qapi-types-char.c GENERATED_FILES += qapi/qapi-types-common.h qapi/qapi-types-common.c +GENERATED_FILES += qapi/qapi-types-tlimits.h qapi/qapi-types-tlimits.c GENERATED_FILES += qapi/qapi-types-crypto.h qapi/qapi-types-crypto.c GENERATED_FILES += qapi/qapi-types-introspect.h qapi/qapi-types-introspect.c GENERATED_FILES += qapi/qapi-types-job.h qapi/qapi-types-job.c @@ -107,12 +108,14 @@ GENERATED_FILES += qapi/qapi-types-tpm.h qapi/qapi-types-tpm.c GENERATED_FILES += qapi/qapi-types-trace.h qapi/qapi-types-trace.c GENERATED_FILES += qapi/qapi-types-transaction.h qapi/qapi-types-transaction.c GENERATED_FILES += qapi/qapi-types-ui.h qapi/qapi-types-ui.c +GENERATED_FILES += qapi/qapi-types-fsdev.h qapi/qapi-types-fsdev.c GENERATED_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c GENERATED_FILES += qapi/qapi-visit.h qapi/qapi-visit.c GENERATED_FILES += qapi/qapi-visit-block-core.h qapi/qapi-visit-block-core.c GENERATED_FILES += qapi/qapi-visit-block.h qapi/qapi-visit-block.c GENERATED_FILES += qapi/qapi-visit-char.h qapi/qapi-visit-char.c GENERATED_FILES += qapi/qapi-visit-common.h qapi/qapi-visit-common.c +GENERATED_FILES += qapi/qapi-visit-tlimits.h qapi/qapi-visit-tlimits.c GENERATED_FILES += qapi/qapi-visit-crypto.h qapi/qapi-visit-crypto.c GENERATED_FILES += qapi/qapi-visit-introspect.h qapi/qapi-visit-introspect.c GENERATED_FILES += qapi/qapi-visit-job.h qapi/qapi-visit-job.c @@ -126,11 +129,13 @@ GENERATED_FILES += qapi/qapi-visit-tpm.h qapi/qapi-visit-tpm.c GENERATED_FILES += qapi/qapi-visit-trace.h qapi/qapi-visit-trace.c GENERATED_FILES += qapi/qapi-visit-transaction.h qapi/qapi-visit-transaction.c GENERATED_FILES += qapi/qapi-visit-ui.h qapi/qapi-visit-ui.c +GENERATED_FILES += qapi/qapi-visit-fsdev.h qapi/qapi-visit-fsdev.c GENERATED_FILES += qapi/qapi-commands.h qapi/qapi-commands.c GENERATED_FILES += qapi/qapi-commands-block-core.h qapi/qapi-commands-block-core.c GENERATED_FILES += qapi/qapi-commands-block.h qapi/qapi-commands-block.c GENERATED_FILES += qapi/qapi-commands-char.h qapi/qapi-commands-char.c GENERATED_FILES += qapi/qapi-commands-common.h qapi/qapi-commands-common.c +GENERATED_FILES += qapi/qapi-commands-tlimits.h qapi/qapi-commands-tlimits.c GENERATED_FILES += qapi/qapi-commands-crypto.h qapi/qapi-commands-crypto.c GENERATED_FILES += qapi/qapi-commands-introspect.h qapi/qapi-commands-introspect.c GENERATED_FILES += qapi/qapi-commands-job.h qapi/qapi-commands-job.c @@ -144,11 +149,13 @@ GENERATED_FILES += qapi/qapi-commands-tpm.h qapi/qapi-commands-tpm.c GENERATED_FILES += qapi/qapi-commands-trace.h
Re: [Qemu-devel] [Qemu-arm] [PATCH 0/2] Fix the last Hyp mode bug and turn it on for A7, A15
Hi Peter, On Fri, Nov 9, 2018 at 6:36 PM Peter Maydell wrote: > > This patchset fixes the last serious bug in our implementation > of Hyp mode (aka EL2 for AArch32), and turns the feature bit > on for the Cortex-A7 and Cortex-A15 CPUs. > > The bug is that Hyp mode is an exception to the previous > general rule that every AArch32 mode (except SYS, which > always shares with USR) has its own banked r13, r14 and > SPSR. Instead Hyp has a banked r13 and SPSR, but r14 is > shared with USR and SYS. We were accidentally implementing > it as banked, which results in remarkably nonobvious > failure modes. > > With this fix, I can boot an AArch32 guest that uses KVM to > boot an AArch32 nested guest, and I can also boot an L4Re/ > Fiasco guest successfully. Nice! More acceptance tests to add :) > > Not entirely sure what to do about this for 3.1 -- maybe > put in the bugfix patch but hold off on actually setting > the feature bit til 4.0? The bugfix surely fits. Do you think enabling the feature isn't well tested and might trigger unexpected side effects? It is certainly not tested... except by you. But if you include it, it might be more tested :) > > thanks > -- PMM > > Peter Maydell (2): > target/arm: Hyp mode R14 is shared with User and System > target/arm/cpu: Give Cortex-A15 and -A7 the EL2 feature > > target/arm/internals.h | 16 > target/arm/cpu.c | 2 ++ > target/arm/helper.c| 29 +++-- > target/arm/kvm32.c | 4 ++-- > target/arm/op_helper.c | 2 +- > 5 files changed, 36 insertions(+), 17 deletions(-) > > -- > 2.19.1 > >
Re: [Qemu-devel] [Qemu-arm] [PATCH 2/2] target/arm/cpu: Give Cortex-A15 and -A7 the EL2 feature
On Fri, Nov 9, 2018 at 6:42 PM Peter Maydell wrote: > > The Cortex-A15 and Cortex-A7 both have EL2; now we've implemented still PL2 there :) > it properly we can enable the feature bit. > > Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé > --- > target/arm/cpu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 784a4c2dfcc..b7185234d85 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1587,6 +1587,7 @@ static void cortex_a7_initfn(Object *obj) > set_feature(>env, ARM_FEATURE_GENERIC_TIMER); > set_feature(>env, ARM_FEATURE_DUMMY_C15_REGS); > set_feature(>env, ARM_FEATURE_CBAR_RO); > +set_feature(>env, ARM_FEATURE_EL2); > set_feature(>env, ARM_FEATURE_EL3); > cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7; > cpu->midr = 0x410fc075; > @@ -1633,6 +1634,7 @@ static void cortex_a15_initfn(Object *obj) > set_feature(>env, ARM_FEATURE_GENERIC_TIMER); > set_feature(>env, ARM_FEATURE_DUMMY_C15_REGS); > set_feature(>env, ARM_FEATURE_CBAR_RO); > +set_feature(>env, ARM_FEATURE_EL2); > set_feature(>env, ARM_FEATURE_EL3); > cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15; > cpu->midr = 0x412fc0f1; > -- > 2.19.1 > >
Re: [Qemu-devel] [Qemu-arm] [PATCH for-3.1] target/arm: Remove antique TODO comment
On Tue, Nov 6, 2018 at 5:41 PM Peter Maydell wrote:
>
> Remove a TODO comment about implementing the vectored interrupt
> controller. We have had an implementation of that for a decade;
> it's in hw/intc/pl190.c.
>
> Signed-off-by: Peter Maydell
On Fri, Nov 9, 2018 at 2:49 PM Peter Maydell wrote:
>
> Currently we track the state of the four irq lines from the GIC
> only via the cs->interrupt_request or KVM irq state. That means
> that we assume that an interrupt is asserted if and only if the
> external line is set. This assumption is incorrect for VIRQ
> and VFIQ, because the HCR_EL2.{VI,VF} bits allow assertion
> of VIRQ and VFIQ separately from the state of the external line.
>
> To handle this, start tracking the state of the external lines
> explicitly in a CPU state struct field, as is common practice
> for devices.
>
> The complicated part of this is dealing with inbound migration
> from an older QEMU which didn't have this state. We assume in
> that case that the older QEMU did not implement the HCR_EL2.{VI,VF}
> bits as generating interrupts, and so the line state matches
> the current state in cs->interrupt_request. (This is not quite
> true between commit 8a0fc3a29fc2315325400c7 and its revert, but
> that commit is broken and never made it into any released QEMU
> version.)
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/arm/helper.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index ec56becc394..851ea9aa977 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8378,7 +8378,6 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
> return;
> }
>
> -/* TODO: Vectored interrupt controller. */
> switch (cs->exception_index) {
> case EXCP_UDEF:
> new_mode = ARM_CPU_MODE_UND;
> --
> 2.19.1
>
>
Re: [Qemu-devel] [Qemu-arm] [PATCH for-v3.1 1/3] Revert "target/arm: Implement HCR.VI and VF"
On Fri, Nov 9, 2018 at 2:48 PM Peter Maydell wrote:
>
> This reverts commit 8a0fc3a29fc2315325400c738f807d0d4ae0ab7f.
>
> The implementation of HCR.VI and VF in that commit is not
> correct -- they do not track the overall "is there a pending
> VIRQ or VFIQ" status, but whether there is a pending interrupt
> due to "this mechanism", ie the hypervisor having set the VI/VF
> bits. The overall pending state for VIRQ and VFIQ is effectively
> the logical OR of the inbound lines from the GIC with the
> VI and VF bits. Commit 8a0fc3a29fc231 would result in pending
> VIRQ/VFIQ possibly being lost when the hypervisor wrote to HCR.
>
> As a preliminary to implementing the HCR.VI/VF feature properly,
> revert the broken one entirely.
>
> Signed-off-by: Peter Maydell
On Fri, Nov 9, 2018 at 2:49 PM Peter Maydell wrote:
>
> Currently we track the state of the four irq lines from the GIC
> only via the cs->interrupt_request or KVM irq state. That means
> that we assume that an interrupt is asserted if and only if the
> external line is set. This assumption is incorrect for VIRQ
> and VFIQ, because the HCR_EL2.{VI,VF} bits allow assertion
> of VIRQ and VFIQ separately from the state of the external line.
>
> To handle this, start tracking the state of the external lines
> explicitly in a CPU state struct field, as is common practice
> for devices.
>
> The complicated part of this is dealing with inbound migration
> from an older QEMU which didn't have this state. We assume in
> that case that the older QEMU did not implement the HCR_EL2.{VI,VF}
> bits as generating interrupts, and so the line state matches
> the current state in cs->interrupt_request. (This is not quite
> true between commit 8a0fc3a29fc2315325400c7 and its revert, but
> that commit is broken and never made it into any released QEMU
> version.)
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/arm/helper.c | 47 -
> 1 file changed, 4 insertions(+), 43 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 851ea9aa977..f3878f505b7 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -3931,7 +3931,6 @@ static const ARMCPRegInfo el3_no_el2_v8_cp_reginfo[] = {
> static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t
> value)
> {
> ARMCPU *cpu = arm_env_get_cpu(env);
> -CPUState *cs = ENV_GET_CPU(env);
> uint64_t valid_mask = HCR_MASK;
>
> if (arm_feature(env, ARM_FEATURE_EL3)) {
> @@ -3950,28 +3949,6 @@ static void hcr_write(CPUARMState *env, const
> ARMCPRegInfo *ri, uint64_t value)
> /* Clear RES0 bits. */
> value &= valid_mask;
>
> -/*
> - * VI and VF are kept in cs->interrupt_request. Modifying that
> - * requires that we have the iothread lock, which is done by
> - * marking the reginfo structs as ARM_CP_IO.
> - * Note that if a write to HCR pends a VIRQ or VFIQ it is never
> - * possible for it to be taken immediately, because VIRQ and
> - * VFIQ are masked unless running at EL0 or EL1, and HCR
> - * can only be written at EL2.
> - */
> -g_assert(qemu_mutex_iothread_locked());
> -if (value & HCR_VI) {
> -cs->interrupt_request |= CPU_INTERRUPT_VIRQ;
> -} else {
> -cs->interrupt_request &= ~CPU_INTERRUPT_VIRQ;
> -}
> -if (value & HCR_VF) {
> -cs->interrupt_request |= CPU_INTERRUPT_VFIQ;
> -} else {
> -cs->interrupt_request &= ~CPU_INTERRUPT_VFIQ;
> -}
> -value &= ~(HCR_VI | HCR_VF);
> -
> /* These bits change the MMU setup:
> * HCR_VM enables stage 2 translation
> * HCR_PTW forbids certain page-table setups
> @@ -3999,32 +3976,16 @@ static void hcr_writelow(CPUARMState *env, const
> ARMCPRegInfo *ri,
> hcr_write(env, NULL, value);
> }
>
> -static uint64_t hcr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> -{
> -/* The VI and VF bits live in cs->interrupt_request */
> -uint64_t ret = env->cp15.hcr_el2 & ~(HCR_VI | HCR_VF);
> -CPUState *cs = ENV_GET_CPU(env);
> -
> -if (cs->interrupt_request & CPU_INTERRUPT_VIRQ) {
> -ret |= HCR_VI;
> -}
> -if (cs->interrupt_request & CPU_INTERRUPT_VFIQ) {
> -ret |= HCR_VF;
> -}
> -return ret;
> -}
> -
> static const ARMCPRegInfo el2_cp_reginfo[] = {
> { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> - .type = ARM_CP_IO,
>.opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
>.access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),
> - .writefn = hcr_write, .readfn = hcr_read },
> + .writefn = hcr_write },
> { .name = "HCR", .state = ARM_CP_STATE_AA32,
> - .type = ARM_CP_ALIAS | ARM_CP_IO,
> + .type = ARM_CP_ALIAS,
>.cp = 15, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
>.access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),
> - .writefn = hcr_writelow, .readfn = hcr_read },
> +
Re: [Qemu-devel] [Qemu-arm] [PATCH for-v3.1 3/3] target/arm: Correctly implement handling of HCR_EL2.{VI, VF}
On Fri, Nov 9, 2018 at 2:48 PM Peter Maydell wrote:
>
> In commit 8a0fc3a29fc2315325400 we tried to implement HCR_EL2.{VI,VF},
> but we got it wrong and had to revert it.
>
> In that commit we implemented them as simply tracking whether there
> is a pending virtual IRQ or virtual FIQ. This is not correct -- these
> bits cause a software-generated VIRQ/VFIQ, which is distinct from
> whether there is a hardware-generated VIRQ/VFIQ caused by the
> external interrupt controller. So we need to track separately
> the HCR_EL2 bit state and the external virq/vfiq line state, and
> OR the two together to get the actual pending VIRQ/VFIQ state.
>
> Fixes: 8a0fc3a29fc2315325400c738f807d0d4ae0ab7f
> Signed-off-by: Peter Maydell
On Fri, Nov 9, 2018 at 2:49 PM Peter Maydell wrote:
>
> Currently we track the state of the four irq lines from the GIC
> only via the cs->interrupt_request or KVM irq state. That means
> that we assume that an interrupt is asserted if and only if the
> external line is set. This assumption is incorrect for VIRQ
> and VFIQ, because the HCR_EL2.{VI,VF} bits allow assertion
> of VIRQ and VFIQ separately from the state of the external line.
>
> To handle this, start tracking the state of the external lines
> explicitly in a CPU state struct field, as is common practice
> for devices.
>
> The complicated part of this is dealing with inbound migration
> from an older QEMU which didn't have this state. We assume in
> that case that the older QEMU did not implement the HCR_EL2.{VI,VF}
> bits as generating interrupts, and so the line state matches
> the current state in cs->interrupt_request. (This is not quite
> true between commit 8a0fc3a29fc2315325400c7 and its revert, but
> that commit is broken and never made it into any released QEMU
> version.)
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/arm/internals.h | 18
> target/arm/cpu.c | 48 +-
> target/arm/helper.c| 20 --
> 3 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 6c2bb2deebd..a32d359dd03 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -871,4 +871,22 @@ static inline const char *aarch32_mode_name(uint32_t psr)
> return cpu_mode_names[psr & 0xf];
> }
>
> +/**
> + * arm_cpu_update_virq: Update CPU_INTERRUPT_VIRQ bit in
> cs->interrupt_request
> + *
> + * Update the CPU_INTERRUPT_VIRQ bit in cs->interrupt_request, following
> + * a change to either the input VIRQ line from the GIC or the HCR_EL2.VI bit.
> + * Must be called with the iothread lock held.
> + */
> +void arm_cpu_update_virq(ARMCPU *cpu);
> +
> +/**
> + * arm_cpu_update_vfiq: Update CPU_INTERRUPT_VFIQ bit in
> cs->interrupt_request
> + *
> + * Update the CPU_INTERRUPT_VFIQ bit in cs->interrupt_request, following
> + * a change to either the input VFIQ line from the GIC or the HCR_EL2.VF bit.
> + * Must be called with the iothread lock held.
> + */
> +void arm_cpu_update_vfiq(ARMCPU *cpu);
> +
> #endif
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 45c16ae90ba..6fbea4dc88c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -436,6 +436,48 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
> interrupt_request)
> }
> #endif
>
> +void arm_cpu_update_virq(ARMCPU *cpu)
> +{
> +/*
> + * Update the interrupt level for VIRQ, which is the logical OR of
> + * the HCR_EL2.VI bit and the input line level from the GIC.
> + */
> +CPUARMState *env = >env;
> +CPUState *cs = CPU(cpu);
> +
> +bool new_state = (env->cp15.hcr_el2 & HCR_VI) ||
> +(env->irq_line_state & CPU_INTERRUPT_VIRQ);
> +
> +if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VIRQ) != 0)) {
> +if (new_state) {
> +cpu_interrupt(cs, CPU_INTERRUPT_VIRQ);
> +} else {
> +cpu_reset_interrupt(cs, CPU_INTERRUPT_VIRQ);
> +}
> +}
> +}
> +
> +void arm_cpu_update_vfiq(ARMCPU *cpu)
> +{
> +/*
> + * Update the interrupt level for VFIQ, which is the logical OR of
> + * the HCR_EL2.VF bit and the input line level from the GIC.
> + */
> +CPUARMState *env = >env;
> +CPUState *cs = CPU(cpu);
> +
> +bool new_state = (env->cp15.hcr_el2 & HCR_VF) ||
> +(env->irq_line_state & CPU_INTERRUPT_VFIQ);
> +
> +if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VFIQ) != 0)) {
> +if (new_state) {
> +cpu_interrupt(cs, CPU_INTERRUPT_VFIQ);
> +} else {
> +cpu_reset_interrupt(cs, CPU_INTERRUPT_VFIQ);
> +}
> +}
> +}
> +
> #ifndef CONFIG_USER_ONLY
> static void arm_cpu_set_irq(void *opaque, int irq, int level)
> {
> @@ -457,9 +499,13 @@ static void arm_cpu_set_irq(void *opaque, int irq, int
> level)
>
> switch (irq) {
> case ARM_CPU_VIRQ:
> +assert(arm_feature(env, ARM_FEATURE_EL2));
> +
Re: [Qemu-devel] [PATCH resend for-3.1] make-release: add skiboot .version file
On Fri, Nov 9, 2018 at 5:16 PM Michael Roth wrote:
>
> This is needed to build skiboot from tarball-distributed sources
> since the git data the make_release.sh script relies on to generate
> it is not available.
>
> Cc: qemu-sta...@nongnu.org
> Reported-by: Michael Tokarev
> Signed-off-by: Michael Roth
Reviewed-by: Philippe Mathieu-Daudé
> ---
> scripts/make-release | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/make-release b/scripts/make-release
> index 04fa9defdc..c14f75b12c 100755
> --- a/scripts/make-release
> +++ b/scripts/make-release
> @@ -19,6 +19,7 @@ pushd ${destination}
> git checkout "v${version}"
> git submodule update --init
> (cd roms/seabios && git describe --tags --long --dirty > .version)
> +(cd roms/skiboot && ./make_version.sh > .version)
> # FIXME: The following line is a workaround for avoiding filename collisions
> # when unpacking u-boot sources on case-insensitive filesystems. Once we
> # update to something with u-boot commit 610eec7f0 we can drop this line.
> --
> 2.17.1
>
>
Re: [Qemu-devel] [PATCH] target/xtensa: drop num_[core_]regs from dc232b/dc233c configs
Hi Max, On Thu, Nov 1, 2018 at 12:02 AM Richard Henderson wrote: > > On 10/31/18 9:35 PM, Max Filippov wrote: > > gdb_regmap::num_core_regs field is initialized incorrectly in the dc232b > > and dc233c configurations. As a result the following message is > > displayed when attaching to an xtensa linux-user process: > > > > "Register 105 is not available", > > > > and gdb is unable to control the inferior. > > > > Now that xtensa_count_regs does the right thing, remove manual > > initialization of these fields from the affected configurations and let > > xtensa_finalize_config initialize them. > > > > Cc: qemu-sta...@nongnu.org > > Signed-off-by: Max Filippov > > --- > > target/xtensa/core-dc232b.c | 2 -- > > target/xtensa/core-dc233c.c | 2 -- > > 2 files changed, 4 deletions(-) > > Reviewed-by: Richard Henderson Can we include this patch for the 3.2 release? Thanks, Phil.
Re: [Qemu-devel] [PATCH] hw: virtio-pci: drop DO_UPCAST
On Sat, Nov 3, 2018 at 4:42 PM Li Qiang wrote:
>
> Use VIRTIO_PCI MACRO instead.
>
> Signed-off-by: Li Qiang
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/virtio/virtio-pci.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index a954799267..277dc20c81 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -597,7 +597,7 @@ virtio_address_space_read(VirtIOPCIProxy *proxy, hwaddr
> addr,
> static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> uint32_t val, int len)
> {
> -VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> +VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
> VirtIODevice *vdev = virtio_bus_get_device(>bus);
> struct virtio_pci_cfg_cap *cfg;
>
> @@ -630,7 +630,7 @@ static void virtio_write_config(PCIDevice *pci_dev,
> uint32_t address,
> static uint32_t virtio_read_config(PCIDevice *pci_dev,
> uint32_t address, int len)
> {
> -VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> +VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
> struct virtio_pci_cfg_cap *cfg;
>
> if (proxy->config_cap &&
> --
> 2.17.1
>
>
>
[Qemu-devel] [RFC PATCH 03/11] target/mips: Move the !ISA_MIPS32R6 check out of decode_opc_special2_legacy
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/mips/translate.c b/target/mips/translate.c
index 60320cbe69..f5e8d0b4d2 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -25649,8 +25649,6 @@ static void decode_opc_special2_legacy(CPUMIPSState
*env, DisasContext *ctx)
int rs, rt, rd;
uint32_t op1;
-check_insn_opc_removed(ctx, ISA_MIPS32R6);
-
rs = (ctx->opcode >> 21) & 0x1f;
rt = (ctx->opcode >> 16) & 0x1f;
rd = (ctx->opcode >> 11) & 0x1f;
@@ -27890,6 +27888,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext
*ctx)
} else if (ctx->insn_flags & ASE_MXU) {
decode_opc_mxu(env, ctx);
} else {
+check_insn_opc_removed(ctx, ISA_MIPS32R6);
decode_opc_special2_legacy(env, ctx);
}
break;
--
2.17.2
Re: [Qemu-devel] [PATCH for-3.2 v3 05/14] qdev: move qdev_prop_register_global_list() to tests
On Wed, Nov 7, 2018 at 1:40 PM Marc-André Lureau
wrote:
>
> The function is only used by a test, move it there.
>
> Signed-off-by: Marc-André Lureau
> Reviewed-by: Eduardo Habkost
Reviewed-by: Philippe Mathieu-Daudé
> ---
> include/hw/qdev-properties.h | 1 -
> hw/core/qdev-properties.c | 9 -
> tests/test-qdev-global-props.c | 18 ++
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index a95f4a73eb..3ab9cd2eb6 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -249,7 +249,6 @@ void qdev_prop_set_enum(DeviceState *dev, const char
> *name, int value);
> void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
>
> void qdev_prop_register_global(GlobalProperty *prop);
> -void qdev_prop_register_global_list(GlobalProperty *props);
> int qdev_prop_check_globals(void);
> void qdev_prop_set_globals(DeviceState *dev);
> void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index ab61d502fd..bd84c4ea4c 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1180,15 +1180,6 @@ void qdev_prop_register_global(GlobalProperty *prop)
> global_props = g_list_append(global_props, prop);
> }
>
> -void qdev_prop_register_global_list(GlobalProperty *props)
> -{
> -int i;
> -
> -for (i = 0; props[i].driver != NULL; i++) {
> -qdev_prop_register_global(props+i);
> -}
> -}
> -
> int qdev_prop_check_globals(void)
> {
> GList *l;
> diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> index ccdf6c57c1..b1eb505442 100644
> --- a/tests/test-qdev-global-props.c
> +++ b/tests/test-qdev-global-props.c
> @@ -89,6 +89,16 @@ static void test_static_prop(void)
> g_test_trap_assert_stdout("");
> }
>
> +static void register_global_properties(GlobalProperty *props)
> +{
> +int i;
> +
> +for (i = 0; props[i].driver != NULL; i++) {
> +qdev_prop_register_global(props + i);
> +}
> +}
> +
> +
> /* Test setting of static property using global properties */
> static void test_static_globalprop_subprocess(void)
> {
> @@ -98,7 +108,7 @@ static void test_static_globalprop_subprocess(void)
> {}
> };
>
> -qdev_prop_register_global_list(props);
> +register_global_properties(props);
>
> mt = STATIC_TYPE(object_new(TYPE_STATIC_PROPS));
> qdev_init_nofail(DEVICE(mt));
> @@ -216,7 +226,7 @@ static void test_dynamic_globalprop_subprocess(void)
> };
> int global_error;
>
> -qdev_prop_register_global_list(props);
> +register_global_properties(props);
>
> mt = DYNAMIC_TYPE(object_new(TYPE_DYNAMIC_PROPS));
> qdev_init_nofail(DEVICE(mt));
> @@ -261,7 +271,7 @@ static void
> test_dynamic_globalprop_nouser_subprocess(void)
> };
> int global_error;
>
> -qdev_prop_register_global_list(props);
> +register_global_properties(props);
>
> mt = DYNAMIC_TYPE(object_new(TYPE_DYNAMIC_PROPS));
> qdev_init_nofail(DEVICE(mt));
> @@ -299,7 +309,7 @@ static void test_subclass_global_props(void)
> {}
> };
>
> -qdev_prop_register_global_list(props);
> +register_global_properties(props);
>
> mt = STATIC_TYPE(object_new(TYPE_SUBCLASS));
> qdev_init_nofail(DEVICE(mt));
> --
> 2.19.1.708.g4ede3d42df
>
>
[Qemu-devel] [RFC PATCH 00/11] decodetree: Add tokens to ease checking ISA flags
Hi Richard, I have been wondering how we can simplify when dealing with multiple ISAs. I used the MIPS arch because it aims to be simple, but handling the multiple ISAs/ASEs as once is a mess, with the particular case of the MIPS32R6. First I wanted to split the translate.c in various ISA/ASE-related files, but this there are too many inlined func involved, I found it handy to use the ?cond token, so we can link all translate functions without worrying about #ifdef'ry. The translating functions are now smaller/easier to read. Then I wanted to add stricter ISA check, to not deal with multiple specifications included more than once, and ease overlapping patterns. I'm not super happy with this series (in particular the token added are MIPS oriented, you can not use spaces in the condition), but I'm interested by what you think :) Rebasing decodetree specs is painful, so better figure what's the best now before continuing. At some point I'd like to get to the one ISA/ASE per file, so we can share compilation units between targets (via $common-obj) and also be able to disable completely some ISAs, for whatever reason (downstream/obsolete/...). Regards, Phil. Philippe Mathieu-Daudé (11): MAINTAINERS: Add scripts/decodetree.py to the TCG section decodetree: Add multiple include guard target/mips: Move the !ISA_MIPS32R6 check out of decode_opc_special2_legacy target/mips: Avoid access to CPUMIPSState from decode* functions decodetree: Force Python to print unsigned values scripts/decodetree: Allow empty specifications scripts/decodetree: Add add_func_check() target/mips: Add a decodetree stub target/mips: Port SYNCI to decodetree scripts/decodetree: Add add_cond_check() target/mips: Port MIPS64 DCL[Z/O] to decodetree MAINTAINERS | 1 + scripts/decodetree.py | 62 ++--- target/mips/Makefile.objs | 8 + target/mips/insns.decode| 22 + target/mips/translate.c | 20 ++-- target/mips/translate.inc.c | 32 +++ 6 files changed, 123 insertions(+), 22 deletions(-) create mode 100644 target/mips/insns.decode create mode 100644 target/mips/translate.inc.c -- 2.17.2
Re: [Qemu-devel] [Qemu-arm] [PATCH for-v3.1 2/3] target/arm: Track the state of our irq lines from the GIC explicitly
On Fri, Nov 9, 2018 at 2:49 PM Peter Maydell wrote:
>
> Currently we track the state of the four irq lines from the GIC
> only via the cs->interrupt_request or KVM irq state. That means
> that we assume that an interrupt is asserted if and only if the
> external line is set. This assumption is incorrect for VIRQ
> and VFIQ, because the HCR_EL2.{VI,VF} bits allow assertion
> of VIRQ and VFIQ separately from the state of the external line.
>
> To handle this, start tracking the state of the external lines
> explicitly in a CPU state struct field, as is common practice
> for devices.
>
> The complicated part of this is dealing with inbound migration
> from an older QEMU which didn't have this state. We assume in
> that case that the older QEMU did not implement the HCR_EL2.{VI,VF}
> bits as generating interrupts, and so the line state matches
> the current state in cs->interrupt_request. (This is not quite
> true between commit 8a0fc3a29fc2315325400c7 and its revert, but
> that commit is broken and never made it into any released QEMU
> version.)
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/arm/cpu.h | 3 +++
> target/arm/cpu.c | 16 ++
> target/arm/machine.c | 51
> 3 files changed, 70 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index b5eff79f73b..f1913cdad26 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -538,6 +538,9 @@ typedef struct CPUARMState {
> uint64_t esr;
> } serror;
>
> +/* State of our input IRQ/FIQ/VIRQ/VFIQ lines */
> +uint32_t irq_line_state;
> +
> /* Thumb-2 EE state. */
> uint32_t teecr;
> uint32_t teehbr;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 784a4c2dfcc..45c16ae90ba 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -449,6 +449,12 @@ static void arm_cpu_set_irq(void *opaque, int irq, int
> level)
> [ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ
> };
>
> +if (level) {
> +env->irq_line_state |= mask[irq];
> +} else {
> +env->irq_line_state &= ~mask[irq];
> +}
> +
> switch (irq) {
> case ARM_CPU_VIRQ:
> case ARM_CPU_VFIQ:
> @@ -473,17 +479,27 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq,
> int level)
> ARMCPU *cpu = opaque;
> CPUState *cs = CPU(cpu);
> int kvm_irq = KVM_ARM_IRQ_TYPE_CPU << KVM_ARM_IRQ_TYPE_SHIFT;
> +uint32_t linestate_bit;
>
> switch (irq) {
> case ARM_CPU_IRQ:
> kvm_irq |= KVM_ARM_IRQ_CPU_IRQ;
> +linestate_bit = CPU_INTERRUPT_HARD;
> break;
> case ARM_CPU_FIQ:
> kvm_irq |= KVM_ARM_IRQ_CPU_FIQ;
> +linestate_bit = CPU_INTERRUPT_FIQ;
> break;
> default:
> g_assert_not_reached();
> }
> +
> +if (level) {
> +env->irq_line_state |= linestate_bit;
> +} else {
> +env->irq_line_state &= ~linestate_bit;
> +}
> +
> kvm_irq |= cs->cpu_index << KVM_ARM_IRQ_VCPU_SHIFT;
> kvm_set_irq(kvm_state, kvm_irq, level ? 1 : 0);
> #endif
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index 239fe4e84d1..2033816a64e 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -192,6 +192,22 @@ static const VMStateDescription vmstate_serror = {
> }
> };
>
> +static bool irq_line_state_needed(void *opaque)
> +{
> +return true;
> +}
> +
> +static const VMStateDescription vmstate_irq_line_state = {
> +.name = "cpu/irq-line-state",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = irq_line_state_needed,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(env.irq_line_state, ARMCPU),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> static bool m_needed(void *opaque)
> {
> ARMCPU *cpu = opaque;
> @@ -625,11 +641,44 @@ static int cpu_pre_save(void *opaque)
> return 0;
> }
>
> +static int cpu_pre_load(void *opaque)
> +{
> +ARMCPU *cpu = opaque;
> +CPUARMState *env = >env;
> +
> +/*
> + * Pre-initialize irq_line_state to a value that's never valid as
> + * real data, so cpu_post_load() can tell whether we've seen the
> + * irq-line-state subsection in the incoming migration state.
> + */
> +env->irq_line_state = UINT32_MAX;
> +
> +return 0;
> +}
> +
> static int cpu_post_load(void *opaque, int version_id)
> {
> ARMCPU *cpu = opaque;
> +CPUARMState *env = >env;
> int i, v;
>
> +/*
> + * Handle migration compatibility from old QEMU which didn't
> + * send the irq-line-state subsection. A QEMU without it did not
> + * implement the HCR_EL2.{VI,VF} bits as generating interrupts,
> + * so for TCG the line state matches the bits set in
> cs->interrupt_request.
> + * For KVM the line state is not stored in cs->interrupt_request
> + * and so this will leave irq_line_state as 0, but this is OK because
> + * we only need to care
[Qemu-devel] [RFC PATCH 08/11] target/mips: Add a decodetree stub
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/Makefile.objs | 8
target/mips/insns.decode| 2 ++
target/mips/translate.c | 7 +++
target/mips/translate.inc.c | 13 +
4 files changed, 30 insertions(+)
create mode 100644 target/mips/insns.decode
create mode 100644 target/mips/translate.inc.c
diff --git a/target/mips/Makefile.objs b/target/mips/Makefile.objs
index 651f36f517..3510835d57 100644
--- a/target/mips/Makefile.objs
+++ b/target/mips/Makefile.objs
@@ -2,3 +2,11 @@ obj-y += translate.o dsp_helper.o op_helper.o lmi_helper.o
helper.o cpu.o
obj-y += gdbstub.o msa_helper.o mips-semi.o
obj-$(CONFIG_SOFTMMU) += machine.o cp0_timer.o
obj-$(CONFIG_KVM) += kvm.o
+
+DECODETREE = $(SRC_PATH)/scripts/decodetree.py
+
+target/mips/decode.inc.c: $(SRC_PATH)/target/mips/insns.decode $(DECODETREE)
+ $(call quiet-command,\
+ $(PYTHON) $(DECODETREE) -o $@ $<, "GEN", $(TARGET_DIR)$@)
+
+target/mips/translate.o: target/mips/decode.inc.c
diff --git a/target/mips/insns.decode b/target/mips/insns.decode
new file mode 100644
index 00..7fbf21cbb9
--- /dev/null
+++ b/target/mips/insns.decode
@@ -0,0 +1,2 @@
+# MIPS32/MIPS64 Instruction Set
+#
diff --git a/target/mips/translate.c b/target/mips/translate.c
index e726f3ec00..560325c563 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27848,6 +27848,8 @@ static void gen_msa(CPUMIPSState *env, DisasContext
*ctx)
}
+#include "translate.inc.c"
+
static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
{
int32_t offset;
@@ -27872,6 +27874,11 @@ static void decode_opc(CPUMIPSState *env, DisasContext
*ctx)
gen_set_label(l1);
}
+/* Transition to the auto-generated decoder. */
+if (decode(ctx, ctx->opcode)) {
+return;
+}
+
op = MASK_OP_MAJOR(ctx->opcode);
rs = (ctx->opcode >> 21) & 0x1f;
rt = (ctx->opcode >> 16) & 0x1f;
diff --git a/target/mips/translate.inc.c b/target/mips/translate.inc.c
new file mode 100644
index 00..69fe78ac89
--- /dev/null
+++ b/target/mips/translate.inc.c
@@ -0,0 +1,13 @@
+/*
+ * MIPS emulation for QEMU - MIPS32 translation routines
+ *
+ * Copyright (c) 2004-2005 Jocelyn Mayer
+ * Copyright (c) 2006 Marius Groeger (FPU operations)
+ * Copyright (c) 2006 Thiemo Seufer (MIPS32R2 support)
+ * Copyright (c) 2018 Philippe Mathieu-Daudé
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+/* Include the auto-generated decoder. */
+#include "decode.inc.c"
--
2.17.2
Re: [Qemu-devel] [Qemu-arm] [PATCH v3 5/5] target/arm: Convert t32ee from feature bit to isar3 test
On Thu, Nov 8, 2018 at 7:02 PM Richard Henderson
wrote:
>
> Reviewed-by: Peter Maydell
> Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/arm/cpu.h | 6 +-
> linux-user/elfload.c | 2 +-
> target/arm/cpu.c | 4
> target/arm/helper.c | 2 +-
> target/arm/kvm32.c | 3 ---
> target/arm/machine.c | 3 +--
> 6 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index b5eff79f73..5c2c77c31d 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1575,7 +1575,6 @@ enum arm_features {
> ARM_FEATURE_NEON,
> ARM_FEATURE_M, /* Microcontroller profile. */
> ARM_FEATURE_OMAPCP, /* OMAP specific CP15 ops handling. */
> -ARM_FEATURE_THUMB2EE,
> ARM_FEATURE_V7MP,/* v7 Multiprocessing Extensions */
> ARM_FEATURE_V7VE, /* v7 Virtualization Extensions (non-EL2 parts) */
> ARM_FEATURE_V4T,
> @@ -3172,6 +3171,11 @@ static inline bool isar_feature_jazelle(const
> ARMISARegisters *id)
> return FIELD_EX32(id->id_isar1, ID_ISAR1, JAZELLE) != 0;
> }
>
> +static inline bool isar_feature_t32ee(const ARMISARegisters *id)
> +{
> +return FIELD_EX32(id->id_isar3, ID_ISAR3, T32EE) != 0;
> +}
> +
> static inline bool isar_feature_aa32_aes(const ARMISARegisters *id)
> {
> return FIELD_EX32(id->id_isar5, ID_ISAR5, AES) != 0;
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 5bccd2e243..a3503c83c9 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -466,7 +466,7 @@ static uint32_t get_elf_hwcap(void)
> GET_FEATURE(ARM_FEATURE_V5, ARM_HWCAP_ARM_EDSP);
> GET_FEATURE(ARM_FEATURE_VFP, ARM_HWCAP_ARM_VFP);
> GET_FEATURE(ARM_FEATURE_IWMMXT, ARM_HWCAP_ARM_IWMMXT);
> -GET_FEATURE(ARM_FEATURE_THUMB2EE, ARM_HWCAP_ARM_THUMBEE);
> +GET_FEATURE_ID(t32ee, ARM_HWCAP_ARM_THUMBEE);
> GET_FEATURE(ARM_FEATURE_NEON, ARM_HWCAP_ARM_NEON);
> GET_FEATURE(ARM_FEATURE_VFP3, ARM_HWCAP_ARM_VFPv3);
> GET_FEATURE(ARM_FEATURE_V6K, ARM_HWCAP_ARM_TLS);
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 784a4c2dfc..d4dc0bc225 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1451,7 +1451,6 @@ static void cortex_a8_initfn(Object *obj)
> set_feature(>env, ARM_FEATURE_V7);
> set_feature(>env, ARM_FEATURE_VFP3);
> set_feature(>env, ARM_FEATURE_NEON);
> -set_feature(>env, ARM_FEATURE_THUMB2EE);
> set_feature(>env, ARM_FEATURE_DUMMY_C15_REGS);
> set_feature(>env, ARM_FEATURE_EL3);
> cpu->midr = 0x410fc080;
> @@ -1520,7 +1519,6 @@ static void cortex_a9_initfn(Object *obj)
> set_feature(>env, ARM_FEATURE_VFP3);
> set_feature(>env, ARM_FEATURE_VFP_FP16);
> set_feature(>env, ARM_FEATURE_NEON);
> -set_feature(>env, ARM_FEATURE_THUMB2EE);
> set_feature(>env, ARM_FEATURE_EL3);
> /* Note that A9 supports the MP extensions even for
> * A9UP and single-core A9MP (which are both different
> @@ -1583,7 +1581,6 @@ static void cortex_a7_initfn(Object *obj)
> set_feature(>env, ARM_FEATURE_V7VE);
> set_feature(>env, ARM_FEATURE_VFP4);
> set_feature(>env, ARM_FEATURE_NEON);
> -set_feature(>env, ARM_FEATURE_THUMB2EE);
> set_feature(>env, ARM_FEATURE_GENERIC_TIMER);
> set_feature(>env, ARM_FEATURE_DUMMY_C15_REGS);
> set_feature(>env, ARM_FEATURE_CBAR_RO);
> @@ -1629,7 +1626,6 @@ static void cortex_a15_initfn(Object *obj)
> set_feature(>env, ARM_FEATURE_V7VE);
> set_feature(>env, ARM_FEATURE_VFP4);
> set_feature(>env, ARM_FEATURE_NEON);
> -set_feature(>env, ARM_FEATURE_THUMB2EE);
> set_feature(>env, ARM_FEATURE_GENERIC_TIMER);
> set_feature(>env, ARM_FEATURE_DUMMY_C15_REGS);
> set_feature(>env, ARM_FEATURE_CBAR_RO);
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 96301930cc..e28770833a 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -5457,7 +5457,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> define_arm_cp_regs(cpu, vmsa_pmsa_cp_reginfo);
> define_arm_cp_regs(cpu, vmsa_cp_reginfo);
> }
> -if (arm_feature(env, ARM_FEATURE_THUMB2EE)) {
> +if (cpu_isar_feature(t32ee, cpu)) {
> define_arm_cp_regs(cpu, t2ee_cp_reginfo);
> }
> if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) {
> diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
> index 9ededa3c73..8b2c9b3075 100644
> --- a/target/arm/kvm32.c
> +++ b/target/arm/kvm32.c
> @@ -115,9 +115,6 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures
> *ahcf)
> set_feature(, ARM_FEATURE_VFP3);
> set_feature(, ARM_FEATURE_GENERIC_TIMER);
>
> -if (extract32(id_pfr0, 12, 4) == 1) {
> -set_feature(, ARM_FEATURE_THUMB2EE);
> -}
> if (extract32(ahcf->isar.mvfr1, 20, 4) == 1) {
> set_feature(, ARM_FEATURE_VFP_FP16);
> }
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index 239fe4e84d..07f904709a 100644
> ---
[Qemu-devel] [RFC PATCH 02/11] decodetree: Add multiple include guard
It is necessary when splitting an ISA, or when using multiple ISAs.
Signed-off-by: Philippe Mathieu-Daudé
---
TODO: explain why, use case
TODO: escape full path?
---
scripts/decodetree.py | 5 +
1 file changed, 5 insertions(+)
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 0bc73b5990..5dea15e7a5 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1030,7 +1030,11 @@ def main():
else:
output_fd = sys.stdout
+hdr_guard = filename.split(os.path.sep)[-1].upper().replace('.', '_') +
"_H"
+
output_autogen()
+output('#ifndef ' + hdr_guard + '\n')
+output('#define ' + hdr_guard + '\n')
for n in sorted(arguments.keys()):
f = arguments[n]
f.output_def()
@@ -1066,6 +1070,7 @@ def main():
t.output_code(4, False, 0, 0)
output('}\n')
+output('#endif /* ' + hdr_guard + ' */\n')
if output_file:
output_fd.close()
--
2.17.2
[Qemu-devel] [RFC PATCH 06/11] scripts/decodetree: Allow empty specifications
Starting with empty specifications allow to write stubs/templates,
useful when testing/rebasing.
This fixes:
decode.inc.c: In function ‘decode’:
decode.inc.c:9:7: error: unused variable ‘u’ [-Werror=unused-variable]
} u;
^
Signed-off-by: Philippe Mathieu-Daudé
---
scripts/decodetree.py | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 08aa52d544..41ed67132d 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1060,12 +1060,13 @@ def main():
output(decode_scope, 'bool ', decode_function,
'(DisasContext *ctx, ', insntype, ' insn)\n{\n')
-i4 = str_indent(4)
-output(i4, 'union {\n')
-for n in sorted(arguments.keys()):
-f = arguments[n]
-output(i4, i4, f.struct_name(), ' f_', f.name, ';\n')
-output(i4, '} u;\n\n')
+if arguments:
+i4 = str_indent(4)
+output(i4, 'union {\n')
+for n in sorted(arguments.keys()):
+f = arguments[n]
+output(i4, i4, f.struct_name(), ' f_', f.name, ';\n')
+output(i4, '} u;\n\n')
t.output_code(4, False, 0, 0)
--
2.17.2
Re: [Qemu-devel] [Qemu-block] KVM Forum block no[td]es
On Mon, Nov 12, 2018 at 12:25 AM Max Reitz wrote: > This is what I’ve taken from two or three BoF-like get-togethers on > blocky things. Amendments are more than welcome, of course. ... > Bitmaps > === > > (Got this section from sneaking into a BoF I wasn’t invited to. Oh > well. Won’t hurt to include them here.) > > Currently, when dirty bitmaps are loaded, all IN_USE bitmaps are just > not loaded at all and completely ignored. That isn’t correct, though, > they should either still be loaded (and automatically treated and > written back as fully dirty), or at least qemu-img check should > “repair” them (i.e. fully dirtying them). > I'm not sure making bitmaps dirty is better. When bitmap is marked IN_USE, it means that we cannot use it for backup. Deleting the bitmap or making it as bad so it cannot be used for the next backup is the same. Making the bitmap as dirty will full the management layer that everything was fine when the next backup includes the entire disk. It is better to cause the next backup to fail in a verbose way, since the backup software can recover by doing a full backup. Sometimes qemu (running in a mode as bare as possible) is better than > using qemu-img convert, for instance. It gives you more control > (through QMP; you get e.g. better progress reporting), you get all of > the mirror optimizations (we do have optimizations for convert, too, > but whether it’s any good to write the same (or different?) > optimizations twice is another question), and you get a common > interface for everything (online and offline). > Note that besides a bare qemu we’ve also always wanted to convert as > many qemu-img operations into frontends for block jobs as possible. > We have only done this for commit, however, even though convert looked > like basically the ideal target. It was just too hard with too little > apparent gain, like always (and convert supports additional features > like concatenation which we don’t have in the runtime block layer > yet). > I'm not sure it is better to run qemu and use qemu-img as thin wrapper over qmp. For example, management system may ascociate qemu-img with a sanlock lease, and sanlock may try to terminate qemu-img when a lease is invalidated. In this case sanlock will succeed while qemu is till accessing storage it should not access. ... > Transacitonable bitmap primitives (e.g. copying a bitmap) would be > nice so you can use them when creating a snapshot. Then it’d be up to > the management layer to make use of them: > - Do you want to continue using the very same bitmap? Copy it then > (or move it, depending on what exactly you want to do and what > primitives there are) > - Do you want to start with a new bitmap? Then just create a new one > along with the overlay. > Having both options sounds good, but we can start with the first. Nir
[Qemu-devel] [RFC PATCH 07/11] scripts/decodetree: Add add_func_check()
The '>' token allow to call a check(arg) function.
This is useful to assert an instruction is supported by an ISA.
Signed-off-by: Philippe Mathieu-Daudé
---
scripts/decodetree.py | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 41ed67132d..2450cc1a63 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -420,7 +420,7 @@ class Arguments:
class General:
"""Common code between instruction formats and instruction patterns"""
-def __init__(self, name, lineno, base, fixb, fixm, udfm, fldm, flds):
+def __init__(self, name, lineno, base, fixb, fixm, udfm, fldm, flds,
chkfs):
self.name = name
self.file = input_file
self.lineno = lineno
@@ -430,6 +430,7 @@ class General:
self.undefmask = udfm
self.fieldmask = fldm
self.fields = flds
+self.check_funcs = chkfs
def __str__(self):
r = self.name
@@ -480,6 +481,8 @@ class Pattern(General):
output(ind, self.base.extract_name(), '(_', arg, ', insn);\n')
for n, f in self.fields.items():
output(ind, 'u.f_', arg, '.', n, ' = ', f.str_extract(), ';\n')
+for f, a in self.check_funcs:
+output(ind, 'check_', f, '(ctx, ', a, ');\n')
output(ind, 'return ', translate_prefix, '_', self.name,
'(ctx, _', arg, ');\n')
# end Pattern
@@ -583,6 +586,11 @@ def add_field_byname(lineno, flds, new_name, old_name):
return add_field(lineno, flds, new_name, lookup_field(lineno, old_name))
+def add_func_check(lineno, chkfns, check_funcname, check_arg):
+chkfns += [(check_funcname, check_arg)]
+return chkfns
+
+
def infer_argument_set(flds):
global arguments
global decode_function
@@ -624,7 +632,7 @@ def infer_format(arg, fieldmask, flds):
if not arg:
arg = infer_argument_set(flds)
-fmt = Format(name, 0, arg, 0, 0, 0, fieldmask, var_flds)
+fmt = Format(name, 0, arg, 0, 0, 0, fieldmask, var_flds, [])
formats[name] = fmt
return (fmt, const_flds)
@@ -646,6 +654,7 @@ def parse_generic(lineno, is_format, name, toks):
undefmask = 0
width = 0
flds = {}
+chkfns = []
arg = None
fmt = None
for t in toks:
@@ -690,6 +699,13 @@ def parse_generic(lineno, is_format, name, toks):
flds = add_field(lineno, flds, fname, ConstField(value))
continue
+# '>FUNC=ARG' calls check_FUNC(ctx, ARG).
+if t[0] == '>':
+(fname, farg) = t[1:].split('=')
+tt = t[2 + len(fname) + len(farg):]
+chkfns = add_func_check(lineno, chkfns, fname, farg)
+continue
+
# Pattern of 0s, 1s, dots and dashes indicate required zeros,
# required ones, or dont-cares.
if re_fullmatch('[01.-]+', t):
@@ -752,7 +768,7 @@ def parse_generic(lineno, is_format, name, toks):
if name in formats:
error(lineno, 'duplicate format name', name)
fmt = Format(name, lineno, arg, fixedbits, fixedmask,
- undefmask, fieldmask, flds)
+ undefmask, fieldmask, flds, chkfns)
formats[name] = fmt
else:
# Patterns can reference a format ...
@@ -779,7 +795,7 @@ def parse_generic(lineno, is_format, name, toks):
if f not in flds.keys() and f not in fmt.fields.keys():
error(lineno, 'field {0} not initialized'.format(f))
pat = Pattern(name, lineno, fmt, fixedbits, fixedmask,
- undefmask, fieldmask, flds)
+ undefmask, fieldmask, flds, chkfns)
patterns.append(pat)
# Validate the masks that we have assembled.
--
2.17.2
[Qemu-devel] [RFC PATCH 09/11] target/mips: Port SYNCI to decodetree
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/insns.decode| 8
target/mips/translate.c | 6 --
target/mips/translate.inc.c | 7 +++
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/target/mips/insns.decode b/target/mips/insns.decode
index 7fbf21cbb9..8a1a7acf3a 100644
--- a/target/mips/insns.decode
+++ b/target/mips/insns.decode
@@ -1,2 +1,10 @@
# MIPS32/MIPS64 Instruction Set
#
+# From:
+# - MIPS32 Architecture For Programmers Volume II-A (Document Number: MD00086)
+
+
+# System Instructions
+
+
+synci 01 - 1 >insn=ISA_MIPS32R2
diff --git a/target/mips/translate.c b/target/mips/translate.c
index 560325c563..760cca8262 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27948,12 +27948,6 @@ static void decode_opc(CPUMIPSState *env, DisasContext
*ctx)
check_insn(ctx, ISA_MIPS32R6);
generate_exception_end(ctx, EXCP_RI);
break;
-case OPC_SYNCI:
-check_insn(ctx, ISA_MIPS32R2);
-/* Break the TB to be able to sync copied instructions
- immediately */
-ctx->base.is_jmp = DISAS_STOP;
-break;
case OPC_BPOSGE32:/* MIPS DSP branch */
#if defined(TARGET_MIPS64)
case OPC_BPOSGE64:
diff --git a/target/mips/translate.inc.c b/target/mips/translate.inc.c
index 69fe78ac89..f3dcd32f98 100644
--- a/target/mips/translate.inc.c
+++ b/target/mips/translate.inc.c
@@ -11,3 +11,10 @@
/* Include the auto-generated decoder. */
#include "decode.inc.c"
+
+static bool trans_synci(DisasContext *dc, arg_synci *a)
+{
+/* Break the TB to be able to sync copied instructions immediately */
+dc->base.is_jmp = DISAS_STOP;
+return true;
+}
--
2.17.2
[Qemu-devel] [RFC PATCH 04/11] target/mips: Avoid access to CPUMIPSState from decode* functions
The DisasContext is already initialized from the CPUMIPSState in
mips_tr_init_disas_context().
Suggested-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/mips/translate.c b/target/mips/translate.c
index f5e8d0b4d2..e726f3ec00 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -16534,7 +16534,7 @@ static void decode_micromips32_opc(CPUMIPSState *env,
DisasContext *ctx)
check_insn(ctx, ASE_MIPS3D);
/* Fall through */
do_cp1branch:
-if (env->CP0_Config1 & (1 << CP0C1_FP)) {
+if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
check_cp1_enabled(ctx);
gen_compute_branch1(ctx, mips32_op,
(ctx->opcode >> 18) & 0x7, imm << 1);
@@ -23809,7 +23809,7 @@ static void decode_opc_special_legacy(CPUMIPSState
*env, DisasContext *ctx)
break;
case OPC_MOVCI:
check_insn(ctx, ISA_MIPS4 | ISA_MIPS32);
-if (env->CP0_Config1 & (1 << CP0C1_FP)) {
+if (ctx->CP0_Config1 & (1 << CP0C1_FP)) {
check_cp1_enabled(ctx);
gen_movci(ctx, rd, rs, (ctx->opcode >> 18) & 0x7,
(ctx->opcode >> 16) & 1);
--
2.17.2
[Qemu-devel] [RFC PATCH 01/11] MAINTAINERS: Add scripts/decodetree.py to the TCG section
Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 126fe0be7e..45e4bfcd87 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -112,6 +112,7 @@ S: Maintained F: cpus.c F: exec.c F: accel/tcg/ +F: scripts/decodetree.py F: include/exec/cpu*.h F: include/exec/exec-all.h F: include/exec/helper*.h -- 2.17.2
[Qemu-devel] [RFC PATCH 11/11] target/mips: Port MIPS64 DCL[Z/O] to decodetree
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/insns.decode| 12
target/mips/translate.inc.c | 12
2 files changed, 24 insertions(+)
diff --git a/target/mips/insns.decode b/target/mips/insns.decode
index 8a1a7acf3a..e256220211 100644
--- a/target/mips/insns.decode
+++ b/target/mips/insns.decode
@@ -2,9 +2,21 @@
#
# From:
# - MIPS32 Architecture For Programmers Volume II-A (Document Number: MD00086)
+# - MIPS64 Architecture For Programmers Volume II-A (Document Number: MD00087)
+
+_rt_rd rs rt rd
+
+@rs_rt_rd .. rs:5 rt:5 rd:5 0 .. _rt_rd
# System Instructions
synci 01 - 1 >insn=ISA_MIPS32R2
+
+
+# Special2 Instructions
+
+
+dclz011100 . . . . 100100 @rs_rt_rd
?ctx->insn_flags_MIPS64
+dclo011100 . . . . 100101 @rs_rt_rd
?ctx->insn_flags_MIPS64
diff --git a/target/mips/translate.inc.c b/target/mips/translate.inc.c
index f3dcd32f98..90fe868605 100644
--- a/target/mips/translate.inc.c
+++ b/target/mips/translate.inc.c
@@ -18,3 +18,15 @@ static bool trans_synci(DisasContext *dc, arg_synci *a)
dc->base.is_jmp = DISAS_STOP;
return true;
}
+
+static bool trans_dclz(DisasContext *ctx, arg_rs_rt_rd *a)
+{
+gen_cl(ctx, OPC_DCLZ, a->rd, a->rs);
+return true;
+}
+
+static bool trans_dclo(DisasContext *ctx, arg_rs_rt_rd *a)
+{
+gen_cl(ctx, OPC_DCLO, a->rd, a->rs);
+return true;
+}
--
2.17.2
[Qemu-devel] [RFC PATCH 10/11] scripts/decodetree: Add add_cond_check()
The '?' token allow to check for a condition.
This is useful to take the translate the instruction only if the
condition is valid.
Signed-off-by: Philippe Mathieu-Daudé
---
scripts/decodetree.py | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 2450cc1a63..ba53ee589e 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -420,7 +420,7 @@ class Arguments:
class General:
"""Common code between instruction formats and instruction patterns"""
-def __init__(self, name, lineno, base, fixb, fixm, udfm, fldm, flds,
chkfs):
+def __init__(self, name, lineno, base, fixb, fixm, udfm, fldm, flds,
chkfs, chkif):
self.name = name
self.file = input_file
self.lineno = lineno
@@ -431,6 +431,7 @@ class General:
self.fieldmask = fldm
self.fields = flds
self.check_funcs = chkfs
+self.check_cond = chkif
def __str__(self):
r = self.name
@@ -483,6 +484,9 @@ class Pattern(General):
output(ind, 'u.f_', arg, '.', n, ' = ', f.str_extract(), ';\n')
for f, a in self.check_funcs:
output(ind, 'check_', f, '(ctx, ', a, ');\n')
+if self.check_cond:
+output(ind, 'if (!(', self.check_cond, '))\n')
+output(ind, 'return false;\n')
output(ind, 'return ', translate_prefix, '_', self.name,
'(ctx, _', arg, ');\n')
# end Pattern
@@ -591,6 +595,10 @@ def add_func_check(lineno, chkfns, check_funcname,
check_arg):
return chkfns
+def add_cond_check(lineno, chkifs, condition):
+return condition
+
+
def infer_argument_set(flds):
global arguments
global decode_function
@@ -632,7 +640,7 @@ def infer_format(arg, fieldmask, flds):
if not arg:
arg = infer_argument_set(flds)
-fmt = Format(name, 0, arg, 0, 0, 0, fieldmask, var_flds, [])
+fmt = Format(name, 0, arg, 0, 0, 0, fieldmask, var_flds, [], None)
formats[name] = fmt
return (fmt, const_flds)
@@ -655,6 +663,7 @@ def parse_generic(lineno, is_format, name, toks):
width = 0
flds = {}
chkfns = []
+chkifs = None
arg = None
fmt = None
for t in toks:
@@ -706,6 +715,13 @@ def parse_generic(lineno, is_format, name, toks):
chkfns = add_func_check(lineno, chkfns, fname, farg)
continue
+# '?condition' calls if(condition).
+if t[0] == '?':
+cond = t[1:]
+tt = t[1 + len(cond):]
+chkifs = add_cond_check(lineno, chkifs, cond)
+continue
+
# Pattern of 0s, 1s, dots and dashes indicate required zeros,
# required ones, or dont-cares.
if re_fullmatch('[01.-]+', t):
@@ -768,7 +784,7 @@ def parse_generic(lineno, is_format, name, toks):
if name in formats:
error(lineno, 'duplicate format name', name)
fmt = Format(name, lineno, arg, fixedbits, fixedmask,
- undefmask, fieldmask, flds, chkfns)
+ undefmask, fieldmask, flds, chkfns, chkifs)
formats[name] = fmt
else:
# Patterns can reference a format ...
@@ -795,7 +811,7 @@ def parse_generic(lineno, is_format, name, toks):
if f not in flds.keys() and f not in fmt.fields.keys():
error(lineno, 'field {0} not initialized'.format(f))
pat = Pattern(name, lineno, fmt, fixedbits, fixedmask,
- undefmask, fieldmask, flds, chkfns)
+ undefmask, fieldmask, flds, chkfns, chkifs)
patterns.append(pat)
# Validate the masks that we have assembled.
--
2.17.2
[Qemu-devel] [RFC PATCH 05/11] decodetree: Force Python to print unsigned values
Python internal representation is signed, so unsigned values
bigger than 31-bit are interpreted as signed (and printed with
a '-' signed).
Mask out to force unsigned values.
Signed-off-by: Philippe Mathieu-Daudé
---
TODO: display error encountered:
case 0x-1:
---
scripts/decodetree.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 5dea15e7a5..08aa52d544 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -900,12 +900,12 @@ class Tree:
def str_case(b):
return '0x{0:08x}'.format(b)
-output(ind, 'switch (', str_switch(self.thismask), ') {\n')
+output(ind, 'switch (', str_switch(self.thismask & insnmask), ') {\n')
for b, s in sorted(self.subs):
assert (self.thismask & ~s.fixedmask) == 0
innermask = outermask | self.thismask
innerbits = outerbits | b
-output(ind, 'case ', str_case(b), ':\n')
+output(ind, 'case ', str_case(b & insnmask), ':\n')
output(ind, '/* ',
str_match_bits(innerbits, innermask), ' */\n')
s.output_code(i + 4, extracted, innerbits, innermask)
--
2.17.2
Re: [Qemu-devel] [PATCH v4] tests: vm: auto_install OpenBSD
ping.
On 10/30/2018 10:57 PM, Fam Zheng wrote:
Upgrade OpenBSD to 6.4 using auto_install. Especially, drop SDL1,
include SDL2.
Also do the build in $HOME since both /var/tmp and /tmp are tmpfs with
limited capacities.
Signed-off-by: Fam Zheng
---
v4: Use 6.4. [Brad]
---
tests/vm/basevm.py | 6 ++--
tests/vm/openbsd | 85 +++---
2 files changed, 76 insertions(+), 15 deletions(-)
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 5caf77d6b8..6fb446d4c5 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -68,8 +68,6 @@ class BaseVM(object):
self._args = [ \
"-nodefaults", "-m", "4G",
"-cpu", "max",
-"-netdev", "user,id=vnet,hostfwd=:127.0.0.1:0-:22",
-"-device", "virtio-net-pci,netdev=vnet",
"-vnc", "127.0.0.1:0,to=20",
"-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
if vcpus and vcpus > 1:
@@ -146,8 +144,10 @@ class BaseVM(object):
"-device",
"virtio-blk,drive=%s,serial=%s,bootindex=1" %
(name, name)]
-def boot(self, img, extra_args=[]):
+def boot(self, img, extra_args=[], extra_usernet_args=""):
args = self._args + [
+"-netdev", "user,id=vnet,hostfwd=:127.0.0.1:0-:22" +
extra_usernet_args,
+"-device", "virtio-net-pci,netdev=vnet",
"-device", "VGA",
"-drive", "file=%s,if=none,id=drive0,cache=writeback" % img,
"-device", "virtio-blk,drive=drive0,bootindex=0"]
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index cfe0572c59..99a7e98d80 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -14,6 +14,9 @@
import os
import sys
import subprocess
+import time
+import atexit
+import tempfile
import basevm
class OpenBSDVM(basevm.BaseVM):
@@ -21,25 +24,83 @@ class OpenBSDVM(basevm.BaseVM):
arch = "x86_64"
BUILD_SCRIPT = """
set -e;
-rm -rf /var/tmp/qemu-test.*
-cd $(mktemp -d /var/tmp/qemu-test.XX);
+rm -rf $HOME/qemu-test.*
+cd $(mktemp -d $HOME/qemu-test.XX);
tar -xf /dev/rsd1c;
-./configure --cc=x86_64-unknown-openbsd6.1-gcc-4.9.4
--python=python2.7 {configure_opts};
+./configure {configure_opts};
gmake --output-sync -j{jobs} {verbose};
# XXX: "gmake check" seems to always hang or fail
#gmake --output-sync -j{jobs} check {verbose};
"""
+def _install_os(self, img):
+tmpdir = tempfile.mkdtemp()
+pxeboot =
self._download_with_cache("https://fastly.cdn.openbsd.org/pub/OpenBSD/6.4/amd64/pxeboot;,
+
sha256sum="d87ab39d941ff926d693943a927585945456ccedb76ea504a251b4b93cd4c266")
+bsd_rd =
self._download_with_cache("https://fastly.cdn.openbsd.org/pub/OpenBSD/6.4/amd64/bsd.rd;,
+
sha256sum="89505c683cbcd75582fe475e847ed53d89e2b8180c3e3d61f4eb4b76b5e11f5c")
+install =
self._download_with_cache("https://fastly.cdn.openbsd.org/pub/OpenBSD/6.4/amd64/install64.iso;,
+
sha256sum='81833b79e23dc0f961ac5fb34484bca66386deb3181ddb8236870fa4f488cdd2')
+subprocess.check_call(["qemu-img", "create", img, "32G"])
+subprocess.check_call(["cp", pxeboot, os.path.join(tmpdir,
"auto_install")])
+subprocess.check_call(["cp", bsd_rd, os.path.join(tmpdir, "bsd")])
+
+self._gen_install_conf(tmpdir)
+# BOOTP filename being auto_install makes sure OpenBSD installer
+# not prompt for "auto install mode"
+usernet_args = ",tftp=%s,bootfile=/auto_install" % tmpdir
+usernet_args += ",tftp-server-name=10.0.2.4"
+usernet_args += ",guestfwd=tcp:10.0.2.4:80-cmd:cat %s" % \
+os.path.join(tmpdir, "install.conf")
+self.boot(img,
+ extra_args=["-boot", "once=n", "-no-reboot",
+ "-cdrom", install],
+ extra_usernet_args=usernet_args)
+self.wait()
+
+def _gen_install_conf(self, tmpdir):
+contents = """\
+HTTP/1.0 200 OK
+
+System hostname = qemu-openbsd
+Password for root = qemupass
+Public ssh key for root = {pub_key}
+Allow root ssh login = yes
+Network interfaces = vio0
+IPv4 address for vio0 = dhcp
+Setup a user = qemu
+Password for user = qemupass
+Public ssh key for user = {pub_key}
+What timezone are you in = US/Eastern
+Server = fastly.cdn.openbsd.org
+Use http = yes
+Default IPv4 route = 10.0.2.2
+Location of sets = cd0
+Set name(s) = all
+Continue without verification = yes
+""".format(pub_key=basevm.SSH_PUB_KEY)
+with open(os.path.join(tmpdir, "install.conf"), "w") as f:
+f.write(contents)
+
def build_image(self, img):
-cimg =
self._download_with_cache("http://download.patchew.org/openbsd-6.1-amd64.img.xz;,
-
Re: [Qemu-devel] [PATCH] decodetree: Force Python to print unsigned values
Hi Richard, On Sun, Nov 11, 2018 at 12:27 PM Richard Henderson wrote: > On 11/11/18 1:02 AM, Philippe Mathieu-Daudé wrote: > > Python internal representation is signed, so unsigned values > > bigger than 31-bit are interpreted as signed (and printed with > > a '-' signed). > > Mask out to force unsigned values. > > > > Signed-off-by: Philippe Mathieu-Daudé > > --- > > scripts/decodetree.py | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Queued, thanks. Can you drop this from your queue? I'll send a cleaner approach (as RFC, and I also want to log the error I got in the commit msg). Thanks, Phil.
[Qemu-devel] KVM Forum block no[td]es
This is what I’ve taken from two or three BoF-like get-togethers on blocky things. Amendments are more than welcome, of course. Permission system = GRAPH_MOD - We need some way for the commit job to prevent graph changes on its chain while it is running. Our current blocker doesn’t do the job, however. What to do? - We have no idea how to make a *permission* work. Maybe the biggest problem is that it just doesn’t work as a permission, because the commit job doesn’t own the BdrvChildren that would need to be blocked (namely the @backing BdrvChild). - A property of BdrvChild that can be set by a non-parent seems more feasible, e.g. a counter where changing the child is possible only if the counter is 0. This also actually makes sense in what it means. (We never quite knew what “taking the GRAPH_PERMISSION” or “unsharing the GRPAH_MOD permission” was supposed to mean. Figuring that out always took like half an our in any face-to-face meeting, and then we decided it was pretty much useless for any case we had at hand.) Reopen -- How should permissions be handled while the reopen is under way? Maybe we should take the union of @perm before and after, and the intersection of @shared before and after? - Taking permissions is a transaction that can fail. Reopen, too, is a transaction, and we want to go from the intermediate to the final permissions in reopen’s commit part, so that transition is not allowed to fail. Since with the above model we would only relax things during that transition (relinquishing bits from @perm and adding bits to @shared), this transition should in theory be possible without any failure. However, in practice things are different, as permission changes with file-posix nodes imply lock changes on the filesystem -- which may always fail. Arguably failures from changing the file-posix locks can be ignored, because that just means that the file claims more permissions to be taken and less to be shared than is actually the case. Which means you may not be able to open the file in some other application, while you should be, but that’s the benign kind of error. You won’t be able to access data in a way you shouldn’t be able to. - Note that we have this issue already, so in general dropping permissions sometimes aborts because code assumes that dropping permissions is always safe and can never result in an error. It seems best to ignore such protocol layer errors in the generic block layer rather than handling this in every protocol driver itself. (The block layer should discard errors from dropping permissions on the protocol layer.) - Is it possible that changing an option may require taking an intermediate permission that is required neither before nor after the reopen process? Changing a child link comes to mind (like changing a child from one BDS to another, where the visible data changes, which would mean we may want to e.g. unshare CONSISTENT_READ during the reopen). However: 1. It is unfeasible to unshare that for all child changes. Effectively everything requires CONSISTENT_READ, and for good reason. 2. Why would a user even change a BDS to something of a different content? 3. Anything that currently allows you to change a child node assumes that the user always changes it to something of the same content (some take extra care to verify this, like mirror, which makes sure that @replaces and the target are connected, and there are only filter nodes in between). Always using the same enforcing model as mirror does (no. 3 above) does not really work, though, because one use case is to copy a backing file offline to some different storage and then replace the files via QMP. To qemu, both files are completely unrelated. Block jobs, including blockdev-copy === Example for use of the fleecing filter: - The real target is on slow storage. Put an overlay on fast storage on top of it. Then use that overlay as the target of the fleecing filter (and commit the data later or on the side), so that the backup job does not slow down the guest. For a unified copy job, having a backup/fleecing filter is not a problem on the way. One thing we definitely have to and can do is to copy common functionality into a shared file so that the different jobs can at least share that. COR/Stream: - There should be a way to discard ranges that have been copied into the overlay from the backing files to save space - Also, the COR filter should integrated with the stream job (at some point, as always) Hole punching with active commit: - Putting data into the target and punching holes in the overlays to make it visible on the active disk may be reasonable for some, but not for others -- it should be an option. You want this if saving space is important, but you may not
[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave
Changes reviewable in a decent web-ui here - https://github.com/qemu/qemu/compare/master...berkus:mojave-cocoa- fix?expand=1 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1802684 Title: QEMU gui crashes on macOS Mojave Status in QEMU: New Bug description: QEMU release 3.0.0 as well as a recent head build /usr/local/Cellar/qemu/HEAD-03c1ca1 (147 files, 257.2MB) Built from source on 2018-11-06 at 13:41:32 with: --with-gtk+3 --with-sdl2 --with-libusb /usr/local/Cellar/qemu/3.0.0 (137 files, 261.6MB) * Poured from bottle on 2018-11-10 at 22:58:32 with: --with-gtk+3 --with-libusb --with-sdl2 Crashes when attempting to use any gui interface (tried SDL and default Cocoa): 2018-11-10 22:58:41.799 qemu-system-aarch64[42982:1102466] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSWindow drag regions should only be invalidated on the Main Thread!' *** First throw call stack: ( 0 CoreFoundation 0x7fff3ea96ecd __exceptionPreprocess + 256 1 libobjc.A.dylib 0x7fff6ab49720 objc_exception_throw + 48 2 CoreFoundation 0x7fff3eab095d -[NSException raise] + 9 3 AppKit 0x7fff3bfb13fa -[NSWindow(NSWindow_Theme) _postWindowNeedsToResetDragMarginsUnlessPostingDisabled] + 324 4 AppKit 0x7fff3bfb6850 -[NSView setFrameSize:] + 2082 5 AppKit 0x7fff3c02747d -[NSVisualEffectView setFrameSize:] + 171 6 AppKit 0x7fff3c0811b1 -[NSTitlebarView setFrameSize:] + 84 7 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 8 AppKit 0x7fff3c081154 -[NSTitlebarView resizeWithOldSuperviewSize:] + 100 9 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 10 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 11 AppKit 0x7fff3c9773c0 -[NSTitlebarContainerView setFrameSize:] + 142 12 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 13 AppKit 0x7fff3bfbcdb5 -[NSView resizeWithOldSuperviewSize:] + 776 14 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 15 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 16 AppKit 0x7fff3c024570 -[NSThemeFrame setFrameSize:] + 495 17 AppKit 0x7fff3c011223 -[NSWindow _setFrame:updateBorderViewSize:] + 966 18 AppKit 0x7fff3c010b46 -[NSWindow _oldPlaceWindow:] + 547 19 AppKit 0x7fff3c010151 -[NSWindow _setFrameCommon:display:stashSize:] + 3006 20 AppKit 0x7fff3c00f57d -[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] + 192 21 AppKit 0x7fff3c019ff8 -[NSWindow setFrame:display:animate:] + 567 22 qemu-system-aarch64 0x00010b7b2abf qemu-system-aarch64 + 3668671 23 qemu-system-aarch64 0x00010b7b6356 qemu-system-aarch64 + 3683158 24 qemu-system-aarch64 0x00010b7ad836 qemu-system-aarch64 + 3647542 25 qemu-system-aarch64 0x00010b4ce769 qemu-system-aarch64 + 636777 26 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 27 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 28 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 29 qemu-system-aarch64 0x00010b4414aa qemu-system-aarch64 + 58538 30 qemu-system-aarch64 0x00010b4f78c3 qemu-system-aarch64 + 805059 31 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 32 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 33 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 34 qemu-system-aarch64 0x00010b4b8f57 qemu-system-aarch64 + 548695 35 qemu-system-aarch64 0x00010b49c3af qemu-system-aarch64 + 431023 36 ??? 0x0001117891f3 0x0 + 4588081651 ) libc++abi.dylib: terminating with uncaught exception of type NSException
[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave
Patches emailed. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1802684 Title: QEMU gui crashes on macOS Mojave Status in QEMU: New Bug description: QEMU release 3.0.0 as well as a recent head build /usr/local/Cellar/qemu/HEAD-03c1ca1 (147 files, 257.2MB) Built from source on 2018-11-06 at 13:41:32 with: --with-gtk+3 --with-sdl2 --with-libusb /usr/local/Cellar/qemu/3.0.0 (137 files, 261.6MB) * Poured from bottle on 2018-11-10 at 22:58:32 with: --with-gtk+3 --with-libusb --with-sdl2 Crashes when attempting to use any gui interface (tried SDL and default Cocoa): 2018-11-10 22:58:41.799 qemu-system-aarch64[42982:1102466] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSWindow drag regions should only be invalidated on the Main Thread!' *** First throw call stack: ( 0 CoreFoundation 0x7fff3ea96ecd __exceptionPreprocess + 256 1 libobjc.A.dylib 0x7fff6ab49720 objc_exception_throw + 48 2 CoreFoundation 0x7fff3eab095d -[NSException raise] + 9 3 AppKit 0x7fff3bfb13fa -[NSWindow(NSWindow_Theme) _postWindowNeedsToResetDragMarginsUnlessPostingDisabled] + 324 4 AppKit 0x7fff3bfb6850 -[NSView setFrameSize:] + 2082 5 AppKit 0x7fff3c02747d -[NSVisualEffectView setFrameSize:] + 171 6 AppKit 0x7fff3c0811b1 -[NSTitlebarView setFrameSize:] + 84 7 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 8 AppKit 0x7fff3c081154 -[NSTitlebarView resizeWithOldSuperviewSize:] + 100 9 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 10 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 11 AppKit 0x7fff3c9773c0 -[NSTitlebarContainerView setFrameSize:] + 142 12 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 13 AppKit 0x7fff3bfbcdb5 -[NSView resizeWithOldSuperviewSize:] + 776 14 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 15 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 16 AppKit 0x7fff3c024570 -[NSThemeFrame setFrameSize:] + 495 17 AppKit 0x7fff3c011223 -[NSWindow _setFrame:updateBorderViewSize:] + 966 18 AppKit 0x7fff3c010b46 -[NSWindow _oldPlaceWindow:] + 547 19 AppKit 0x7fff3c010151 -[NSWindow _setFrameCommon:display:stashSize:] + 3006 20 AppKit 0x7fff3c00f57d -[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] + 192 21 AppKit 0x7fff3c019ff8 -[NSWindow setFrame:display:animate:] + 567 22 qemu-system-aarch64 0x00010b7b2abf qemu-system-aarch64 + 3668671 23 qemu-system-aarch64 0x00010b7b6356 qemu-system-aarch64 + 3683158 24 qemu-system-aarch64 0x00010b7ad836 qemu-system-aarch64 + 3647542 25 qemu-system-aarch64 0x00010b4ce769 qemu-system-aarch64 + 636777 26 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 27 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 28 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 29 qemu-system-aarch64 0x00010b4414aa qemu-system-aarch64 + 58538 30 qemu-system-aarch64 0x00010b4f78c3 qemu-system-aarch64 + 805059 31 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 32 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 33 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 34 qemu-system-aarch64 0x00010b4b8f57 qemu-system-aarch64 + 548695 35 qemu-system-aarch64 0x00010b49c3af qemu-system-aarch64 + 431023 36 ??? 0x0001117891f3 0x0 + 4588081651 ) libc++abi.dylib: terminating with uncaught exception of type NSException fish: 'qemu-system-aarch64 -M raspi3 -…' terminated by signal SIGABRT (Abort) macOS Mojave 10.14.2 Beta
[Qemu-devel] [PATCH] Fix for crashes and non-responsive UI on macOS Mojave
It seems that Cocoa checks are stricter on Mojave and some callbacks that
worked from non-GUI thread on High Sierra are no longer working.
The fixes included here are:
* Deferring qemu_main() to another thread so that the actual main thread is
reserved for the Cocoa UI; it also removes blocking from
applicationDidFinishLoading: delegate. I beleive this alone caused complete UI
blockage on Mojave.
* Deferring UI-related updates in callbacks to the UI thread using
invokeOnMainThread helper function. This function uses DDInvocationGrabber
object courtesy of Dave Dribin, licensed under MIT license.
Here’s relevant blog post for his code:
https://www.dribin.org/dave/blog/archives/2008/05/22/invoke_on_main_thread/
NSInvocation is used here instead of plain
performSelectorOnMainThread:withObject:waitUntilDone: because we want to be
able to pass non-id types to the handlers.
These changes are ought to work on OSX 10.6, although I don’t have a machine
handy to test it.
Fixes https://bugs.launchpad.net/qemu/+bug/1802684
From 8f86e30f3710d782d78dccdbe7a1564ae79220c7 Mon Sep 17 00:00:00 2001
From: Berkus Decker
Date: Sun, 11 Nov 2018 21:58:17 +0200
Subject: [PATCH 1/2] ui/cocoa: Defer qemu to another thread, leaving main
thread for the UI
This prevents blocking in applicationDidFinishLoading: which is
not recommended and now causes complete UI lock on macOS Mojave.
Signed-off-by: Berkus Decker
---
ui/cocoa.m | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index ecf12bfc2e..f69f7105f2 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1089,9 +1089,13 @@ QemuCocoaView *cocoaView;
{
COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
-int status;
-status = qemu_main(argc, argv, *_NSGetEnviron());
-exit(status);
+dispatch_queue_t qemu_runner = dispatch_queue_create("qemu-runner",
DISPATCH_QUEUE_SERIAL);
+
+dispatch_async(qemu_runner, ^{
+int status;
+status = qemu_main(argc, argv, *_NSGetEnviron());
+exit(status);
+});
}
/* We abstract the method called by the Enter Fullscreen menu item
--
2.18.0
From 467b0f67d94616ef98d2ec1e8d16eeb5e9506b4e Mon Sep 17 00:00:00 2001
From: Berkus Decker
Date: Sun, 11 Nov 2018 20:22:01 +0200
Subject: [PATCH 2/2] ui/cocoa: Fix UI crashes on macOS Mojave
Signed-off-by: Berkus Decker
---
ui/DDInvocationGrabber.h | 124
ui/DDInvocationGrabber.m | 171 +++
ui/Makefile.objs | 2 +-
ui/cocoa.m | 57 -
4 files changed, 333 insertions(+), 21 deletions(-)
create mode 100644 ui/DDInvocationGrabber.h
create mode 100644 ui/DDInvocationGrabber.m
diff --git a/ui/DDInvocationGrabber.h b/ui/DDInvocationGrabber.h
new file mode 100644
index 00..7218421d74
--- /dev/null
+++ b/ui/DDInvocationGrabber.h
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) 2007-2008 Dave Dribin
+ *
+ * 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.
+ */
+
+
+/*
+ * This class is based on CInvocationGrabber:
+ *
+ * Copyright (c) 2007, Toxic Software
+ * All rights reserved.
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of the Toxic Software nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND
Re: [Qemu-devel] [PATCH v4 15/15] block: Assert that flags are up-to-date in bdrv_reopen_prepare()
On 07.11.18 13:59, Alberto Garcia wrote:
> Towards the end of bdrv_reopen_queue_child(), before starting to
> process the children, the update_flags_from_options() function is
> called in order to have BDRVReopenState.flags in sync with the options
> from the QDict.
>
> This is necessary because during the reopen process flags must be
> updated for all nodes in the queue so bdrv_is_writable_after_reopen()
> and the permission checks work correctly.
>
> Because of that, calling update_flags_from_options() again in
> bdrv_reopen_prepare() doesn't really change the flags (they are
> already up-to-date). But we need to call it in order to remove those
> options from QemuOpts and that way indicate that they have been
> processed.
>
> Signed-off-by: Alberto Garcia
> ---
> block.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/block.c b/block.c
> index 68f1e3b45e..03277b3d19 100644
> --- a/block.c
> +++ b/block.c
> @@ -3178,6 +3178,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
> BlockReopenQueue *queue,
> Error **errp)
> {
> int ret = -1;
> +int old_flags;
> Error *local_err = NULL;
> BlockDriver *drv;
> QemuOpts *opts;
> @@ -3203,7 +3204,12 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
> BlockReopenQueue *queue,
> goto error;
> }
>
> +/* This was already called in bdrv_reopen_queue_child() so the flags
> + * are up-to-date. This time we simply want to remove the options from
> + * QemuOpts in order to indicate that they have been processed. */
> +old_flags = reopen_state->flags;
> update_flags_from_options(_state->flags, opts);
> +assert(old_flags == reopen_state->flags);
Reviewed-by: Max Reitz
Although as my bike-shedding for today I'd like to say that I'd find it
more intuitive to store the "just remove the options" call result into
old_flags instead (or rather something renamed), i.e.
flags_copy = reopen_state->flags;
update_flags_from_options(_copy, opts);
assert(flags_copy == reopen_state->flags);
Not that it matters.
Max
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 14/15] block: Remove assertions from update_flags_from_options()
On 07.11.18 13:59, Alberto Garcia wrote:
> This function takes three options (cache.direct, cache.no-flush and
> read-only) from a QemuOpts object and updates the flags accordingly.
and auto-read-only now
>
> If any of those options is not set (because it was missing from the
> original QDict or because it had an invalid value) then the function
> aborts with a failed assertion:
>
>$ qemu-io -c 'reopen -o read-only=foo' hd.qcow2
>block.c:1126: update_flags_from_options: Assertion `qemu_opt_find(opts,
> BDRV_OPT_CACHE_DIRECT)' failed.
>Aborted
>
> This assertion is unnecessary, and it forces any caller of
> bdrv_reopen() to pass all the aforementioned three options. This may
*four
> have made sense in order to remove ambiguity when bdrv_reopen() was
> taking both flags and options, but that's not the case anymore.
>
> It's also unnecessary if we want to validate the option values,
> because bdrv_reopen_prepare() already takes care of that, as we can
> see if we remove the assertions:
>
>$ qemu-io -c 'reopen -o read-only=foo' hd.qcow2
>Parameter 'read-only' expects 'on' or 'off'
>
> Signed-off-by: Alberto Garcia
> ---
> block.c| 4
> tests/qemu-iotests/133 | 8
> tests/qemu-iotests/133.out | 6 ++
> 3 files changed, 14 insertions(+), 4 deletions(-)
Hm, seems like one way to solve it and I can't really find issue with
it. So, let's first give a
Reviewed-by: Max Reitz
However, I wonder why you dropped your patch from v1 for this. It
seemed more reasonable to me. You're basically trading half-updating
the flags for just not touching them at all (and the latter seems
better, even though it's all an error in the end anyway).
> diff --git a/block.c b/block.c
> index 8bc808d6f3..68f1e3b45e 100644
> --- a/block.c
> +++ b/block.c
> @@ -1139,24 +1139,20 @@ static void update_flags_from_options(int *flags,
> QemuOpts *opts)
> {
> *flags &= ~BDRV_O_CACHE_MASK;
>
> -assert(qemu_opt_find(opts, BDRV_OPT_CACHE_NO_FLUSH));
> if (qemu_opt_get_bool_del(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) {
> *flags |= BDRV_O_NO_FLUSH;
> }
>
> -assert(qemu_opt_find(opts, BDRV_OPT_CACHE_DIRECT));
> if (qemu_opt_get_bool_del(opts, BDRV_OPT_CACHE_DIRECT, false)) {
> *flags |= BDRV_O_NOCACHE;
> }
>
> *flags &= ~BDRV_O_RDWR;
Unrelated to this patch, but isn't BDRV_O_AUTO_RDONLY missing here?
Max
>
> -assert(qemu_opt_find(opts, BDRV_OPT_READ_ONLY));
> if (!qemu_opt_get_bool_del(opts, BDRV_OPT_READ_ONLY, false)) {
> *flags |= BDRV_O_RDWR;
> }
>
> -assert(qemu_opt_find(opts, BDRV_OPT_AUTO_READ_ONLY));
> if (qemu_opt_get_bool_del(opts, BDRV_OPT_AUTO_READ_ONLY, false)) {
> *flags |= BDRV_O_AUTO_RDONLY;
> }
> diff --git a/tests/qemu-iotests/133 b/tests/qemu-iotests/133
> index 14e6b3b972..59d5e2ea25 100755
> --- a/tests/qemu-iotests/133
> +++ b/tests/qemu-iotests/133
> @@ -101,6 +101,14 @@ $QEMU_IO -c 'reopen -w -o read-only=on' $TEST_IMG
> $QEMU_IO -c 'reopen -c none -o cache.direct=on' $TEST_IMG
> $QEMU_IO -c 'reopen -c writeback -o cache.direct=on' $TEST_IMG
> $QEMU_IO -c 'reopen -c directsync -o cache.no-flush=on' $TEST_IMG
> +
> +echo
> +echo "=== Check that invalid options are handled correctly ==="
> +echo
> +
> +$QEMU_IO -c 'reopen -o read-only=foo' $TEST_IMG
> +$QEMU_IO -c 'reopen -o cache.no-flush=bar' $TEST_IMG
> +$QEMU_IO -c 'reopen -o cache.direct=baz' $TEST_IMG
> # success, all done
> echo "*** done"
> rm -f $seq.full
> diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out
> index 48a9d087f0..551096a9c4 100644
> --- a/tests/qemu-iotests/133.out
> +++ b/tests/qemu-iotests/133.out
> @@ -32,4 +32,10 @@ Cannot set both -r/-w and 'read-only'
> Cannot set both -c and the cache options
> Cannot set both -c and the cache options
> Cannot set both -c and the cache options
> +
> +=== Check that invalid options are handled correctly ===
> +
> +Parameter 'read-only' expects 'on' or 'off'
> +Parameter 'cache.no-flush' expects 'on' or 'off'
> +Parameter 'cache.direct' expects 'on' or 'off'
> *** done
>
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 13/15] block: Stop passing flags to bdrv_reopen_queue_child()
On 07.11.18 13:59, Alberto Garcia wrote: > Now that all callers are passing the new options using the QDict we no > longer need the 'flags' parameter. > > This patch makes the following changes: > >1) The update_options_from_flags() call is no longer necessary > so it can be removed. > >2) The update_flags_from_options() call is now used in all cases, > and is moved down a few lines so it happens after the options > QDict contains the final set of values. > >3) The flags parameter is removed. Now the flags are initialized > using the current value (for the top-level node) or the parent > flags (after inherit_options()). In both cases the initial > values are updated to reflect the new options in the QDict. This > happens in bdrv_reopen_queue_child() (as explained above) and in > bdrv_reopen_prepare(). > > Signed-off-by: Alberto Garcia > --- > block.c | 48 +++- > 1 file changed, 19 insertions(+), 29 deletions(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 11/15] block: Clean up reopen_backing_file() in block/replication.c
On 07.11.18 13:59, Alberto Garcia wrote: > This function is used to put the hidden and secondary disks in > read-write mode before launching the backup job, and back in read-only > mode afterwards. > > This patch does the following changes: > > - Use an options QDict with the "read-only" option instead of > passing the changes as flags only. > > - Simplify the code (it was unnecessarily complicated and verbose). > > - Fix a bug due to which the secondary disk was not being put back > in read-only mode when writable=false (because in this case > orig_secondary_flags always had the BDRV_O_RDWR flag set). > > - Stop clearing the BDRV_O_INACTIVE flag. > > The flags parameter to bdrv_reopen_queue() becomes redundant and we'll > be able to get rid of it in a subsequent patch. > > Signed-off-by: Alberto Garcia > --- > block/replication.c | 45 + > 1 file changed, 21 insertions(+), 24 deletions(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 10/15] qemu-io: Put flag changes in the options QDict in reopen_f()
On 07.11.18 13:59, Alberto Garcia wrote:
> When reopen_f() puts a block device in the reopen queue, some of the
> new options are passed using a QDict, but others ("read-only" and the
> cache options) are passed as flags.
>
> This patch puts those flags in the QDict. This way the flags parameter
> becomes redundant and we'll be able to get rid of it in a subsequent
> patch.
>
> Signed-off-by: Alberto Garcia
> ---
> qemu-io-cmds.c | 27 ++-
> tests/qemu-iotests/133 | 9 +
> tests/qemu-iotests/133.out | 8
> 3 files changed, 43 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave
Ok I think I found places where code was invalid in Cocoa and fixed it. I can see qemu running my kernel and all interface is responsive. I also believe it should be working on as old as macOS 10.6 machines as well - do you have some CI machines with these versions to test? I don't. For SDL i didn't look into the details yet. Will try to set up a reproducible case for SDL2 folks over the week. Will send the patches to mailing list as suggested. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1802684 Title: QEMU gui crashes on macOS Mojave Status in QEMU: New Bug description: QEMU release 3.0.0 as well as a recent head build /usr/local/Cellar/qemu/HEAD-03c1ca1 (147 files, 257.2MB) Built from source on 2018-11-06 at 13:41:32 with: --with-gtk+3 --with-sdl2 --with-libusb /usr/local/Cellar/qemu/3.0.0 (137 files, 261.6MB) * Poured from bottle on 2018-11-10 at 22:58:32 with: --with-gtk+3 --with-libusb --with-sdl2 Crashes when attempting to use any gui interface (tried SDL and default Cocoa): 2018-11-10 22:58:41.799 qemu-system-aarch64[42982:1102466] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSWindow drag regions should only be invalidated on the Main Thread!' *** First throw call stack: ( 0 CoreFoundation 0x7fff3ea96ecd __exceptionPreprocess + 256 1 libobjc.A.dylib 0x7fff6ab49720 objc_exception_throw + 48 2 CoreFoundation 0x7fff3eab095d -[NSException raise] + 9 3 AppKit 0x7fff3bfb13fa -[NSWindow(NSWindow_Theme) _postWindowNeedsToResetDragMarginsUnlessPostingDisabled] + 324 4 AppKit 0x7fff3bfb6850 -[NSView setFrameSize:] + 2082 5 AppKit 0x7fff3c02747d -[NSVisualEffectView setFrameSize:] + 171 6 AppKit 0x7fff3c0811b1 -[NSTitlebarView setFrameSize:] + 84 7 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 8 AppKit 0x7fff3c081154 -[NSTitlebarView resizeWithOldSuperviewSize:] + 100 9 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 10 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 11 AppKit 0x7fff3c9773c0 -[NSTitlebarContainerView setFrameSize:] + 142 12 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 13 AppKit 0x7fff3bfbcdb5 -[NSView resizeWithOldSuperviewSize:] + 776 14 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 15 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 16 AppKit 0x7fff3c024570 -[NSThemeFrame setFrameSize:] + 495 17 AppKit 0x7fff3c011223 -[NSWindow _setFrame:updateBorderViewSize:] + 966 18 AppKit 0x7fff3c010b46 -[NSWindow _oldPlaceWindow:] + 547 19 AppKit 0x7fff3c010151 -[NSWindow _setFrameCommon:display:stashSize:] + 3006 20 AppKit 0x7fff3c00f57d -[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] + 192 21 AppKit 0x7fff3c019ff8 -[NSWindow setFrame:display:animate:] + 567 22 qemu-system-aarch64 0x00010b7b2abf qemu-system-aarch64 + 3668671 23 qemu-system-aarch64 0x00010b7b6356 qemu-system-aarch64 + 3683158 24 qemu-system-aarch64 0x00010b7ad836 qemu-system-aarch64 + 3647542 25 qemu-system-aarch64 0x00010b4ce769 qemu-system-aarch64 + 636777 26 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 27 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 28 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 29 qemu-system-aarch64 0x00010b4414aa qemu-system-aarch64 + 58538 30 qemu-system-aarch64 0x00010b4f78c3 qemu-system-aarch64 + 805059 31 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 32 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 33 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 34 qemu-system-aarch64
Re: [Qemu-devel] [PATCH v5 0/5] off-by-one and NULL pointer accesses detected by static analysis
On 05.11.18 22:38, Liam Merwick wrote: > Below are a number of fixes to some off-by-one, read outside array bounds, and > NULL pointer accesses detected by an internal Oracle static analysis tool > (Parfait). > https://labs.oracle.com/pls/apex/f?p=labs:49:P49_PROJECT_ID:13 I decided to just fix the issue I had in patch 4 (dropped the "if" block that was not doing much, and fixed the DLOG() indentation) and applied the patch to my block branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v5 4/5] block: Fix potential Null pointer dereferences in vvfat.c
On 05.11.18 22:38, Liam Merwick wrote:
> The calls to find_mapping_for_cluster() may return NULL but it
> isn't always checked for before dereferencing the value returned.
> Additionally, add some asserts to cover cases where NULL can't
> be returned but which might not be obvious at first glance.
>
> Signed-off-by: Liam Merwick
> ---
> block/vvfat.c | 50 ++
> 1 file changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index fc41841a5c3c..263274d9739a 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
[...]
> @@ -2428,16 +2424,13 @@ static int commit_direntries(BDRVVVFATState* s,
> direntry_t* direntry = array_get(&(s->directory), dir_index);
> uint32_t first_cluster = dir_index == 0 ? 0 :
> begin_of_direntry(direntry);
> mapping_t* mapping = find_mapping_for_cluster(s, first_cluster);
> -
> int factor = 0x10 * s->sectors_per_cluster;
> int old_cluster_count, new_cluster_count;
> -int current_dir_index = mapping->info.dir.first_dir_index;
> -int first_dir_index = current_dir_index;
> +int current_dir_index;
> +int first_dir_index;
> int ret, i;
> uint32_t c;
>
> -DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n",
> mapping->path, parent_mapping_index));
> -
> assert(direntry);
> assert(mapping);
Oh, having moved the condition below the declarations brings an
interesting point to light, which is that there is an assertion for it
here already. So...
> assert(mapping->begin == first_cluster);
> @@ -2445,6 +2438,15 @@ DLOG(fprintf(stderr, "commit_direntries for %s,
> parent_mapping_index %d\n", mapp
> assert(mapping->mode & MODE_DIRECTORY);
> assert(dir_index == 0 || is_directory(direntry));
>
> +if (mapping == NULL) {
> +return -1;
> +}
> +
...this should just not be added altogether.
> +DLOG(fprintf(stderr, "commit_direntries for %s, parent_mapping_index %d\n",
> +mapping->path, parent_mapping_index));
Moving this and the following dereferencing statements below that
assertion is reasonable, though. I think you should indent the DLOG()
while you're at it, though, because there is no reason not to, and the
way it is here just violates the coding style. (Disregarding that
vvfat.c effectively is a complete violation of the qemu coding style.
*cough*)
> +
> +current_dir_index = mapping->info.dir.first_dir_index;
> +first_dir_index = current_dir_index;
> mapping->info.dir.parent_mapping_index = parent_mapping_index;
>
> if (first_cluster == 0) {
So with the "if (mapping == NULL) {}" block above (hunk @@2445) dropped
and the DLOG() indented:
Reviewed-by: Max Reitz
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v5 2/5] block: Null pointer dereference in blk_root_get_parent_desc()
On 05.11.18 22:38, Liam Merwick wrote: > The dev_id returned by the call to blk_get_attached_dev_id() in > blk_root_get_parent_desc() can be NULL (an internal call to > object_get_canonical_path may have returned NULL). > > Instead of just checking this case before before dereferencing, > adjust blk_get_attached_dev_id() to return the empty string if no > object path can be found (similar to the case when blk->dev is NULL > and an empty string is returned). > > Signed-off-by: Liam Merwick > --- > block/block-backend.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] ppc/spapr: Receive and store device tree blob from SLOF
Hi Alexey,
Just a few remarks. See below.
On Thu, 8 Nov 2018 12:44:06 +1100
Alexey Kardashevskiy wrote:
> SLOF receives a device tree and updates it with various properties
> before switching to the guest kernel and QEMU is not aware of any changes
> made by SLOF. Since there is no real RTAS (QEMU implements it), it makes
> sense to pass the SLOF final device tree to QEMU to let it implement
> RTAS related tasks better, such as PCI host bus adapter hotplug.
>
> Specifially, now QEMU can find out the actual XICS phandle (for PHB
> hotplug) and the RTAS linux,rtas-entry/base properties (for firmware
> assisted NMI - FWNMI).
>
> This stores the initial DT blob in the sPAPR machine and replaces it
> in the KVMPPC_H_UPDATE_DT (new private hypercall) handler.
>
> This adds an @update_dt_enabled machine property to allow backward
> migration.
>
> SLOF already has a hypercall since
> https://github.com/aik/SLOF/commit/e6fc84652c9c0073f9183
>
> Signed-off-by: Alexey Kardashevskiy
> ---
> include/hw/ppc/spapr.h | 7 ++-
> hw/ppc/spapr.c | 29 -
> hw/ppc/spapr_hcall.c | 32
> hw/ppc/trace-events| 2 ++
> 4 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ad4d7cfd97..f5dcaf44cb 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -100,6 +100,7 @@ struct sPAPRMachineClass {
>
> /*< public >*/
> bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> +bool update_dt_enabled;/* enable KVMPPC_H_UPDATE_DT */
> bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */
> bool pre_2_10_has_unused_icps;
> bool legacy_irq_allocation;
> @@ -136,6 +137,9 @@ struct sPAPRMachineState {
> int vrma_adjust;
> ssize_t rtas_size;
> void *rtas_blob;
> +uint32_t fdt_size;
> +uint32_t fdt_initial_size;
I don't quite see the purpose of fdt_initial_size... it seems to be only
used to print a trace.
> +void *fdt_blob;
> long kernel_size;
> bool kernel_le;
> uint32_t initrd_base;
> @@ -462,7 +466,8 @@ struct sPAPRMachineState {
> #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
> /* Client Architecture support */
> #define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
> -#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
> +#define KVMPPC_H_UPDATE_DT (KVMPPC_HCALL_BASE + 0x3)
> +#define KVMPPC_HCALL_MAXKVMPPC_H_UPDATE_DT
>
> typedef struct sPAPRDeviceTreeUpdateHeader {
> uint32_t version_id;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c08130facb..5e2d4d211c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1633,7 +1633,10 @@ static void spapr_machine_reset(void)
> /* Load the fdt */
> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
> -g_free(fdt);
> +g_free(spapr->fdt_blob);
> +spapr->fdt_size = fdt_totalsize(fdt);
> +spapr->fdt_initial_size = spapr->fdt_size;
> +spapr->fdt_blob = fdt;
Hmm... It looks weird to store state in a reset handler. I'd rather zeroe
both fdt_blob and fdt_size here.
>
> /* Set up the entry state */
> spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT, fdt_addr);
> @@ -1887,6 +1890,27 @@ static const VMStateDescription vmstate_spapr_irq_map
> = {
> },
> };
>
> +static bool spapr_dtb_needed(void *opaque)
> +{
> +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(opaque);
> +
> +return smc->update_dt_enabled;
This means we always migrate the fdt, even if migration occurs before
SLOF could call KVMPPC_H_UPDATE_DT.
With spapr->fdt_blob set to NULL on reset, a better check would be:
sPAPRMachineState *spapr = SPAPR_MACHINE(opaque);
return smc->update_dt_enabled && spapr->fdt_blob;
> +}
> +
> +static const VMStateDescription vmstate_spapr_dtb = {
> +.name = "spapr_dtb",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = spapr_dtb_needed,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT32(fdt_initial_size, sPAPRMachineState),
> +VMSTATE_UINT32(fdt_size, sPAPRMachineState),
> +VMSTATE_VBUFFER_ALLOC_UINT32(fdt_blob, sPAPRMachineState, 0, NULL,
> + fdt_size),
> +VMSTATE_END_OF_LIST()
> +},
> +};
> +
> static const VMStateDescription vmstate_spapr = {
> .name = "spapr",
> .version_id = 3,
> @@ -1915,6 +1939,7 @@ static const VMStateDescription vmstate_spapr = {
> _spapr_cap_sbbc,
> _spapr_cap_ibs,
> _spapr_irq_map,
> +_spapr_dtb,
This needs to be rebased.
<<<
_spapr_cap_nested_kvm_hv,
===
_spapr_dtb,
>>>
I'll try to find some time to respin the PHB hotplug series and I'll happily
give a try to this patch.
> NULL
> }
> };
> @@ -3849,6 +3874,7 @@ static void
Re: [Qemu-devel] [Qemu-block] Change in qemu 2.12 causes qemu-img convert to NBD to write more data
On Wed, Nov 7, 2018 at 7:55 PM Nir Soffer wrote:
> On Wed, Nov 7, 2018 at 7:27 PM Kevin Wolf wrote:
>
>> Am 07.11.2018 um 15:56 hat Nir Soffer geschrieben:
>> > Wed, Nov 7, 2018 at 4:36 PM Richard W.M. Jones
>> wrote:
>> >
>> > > Another thing I tried was to change the NBD server (nbdkit) so that it
>> > > doesn't advertise zero support to the client:
>> > >
>> > > $ nbdkit --filter=log --filter=nozero memory size=6G
>> logfile=/tmp/log \
>> > > --run './qemu-img convert ./fedora-28.img -n $nbd'
>> > > $ grep '\.\.\.$' /tmp/log | sed 's/.*\([A-Z][a-z]*\).*/\1/' | uniq
>> -c
>> > >2154 Write
>> > >
>> > > Not surprisingly no zero commands are issued. The size of the write
>> > > commands is very uneven -- it appears to be send one command per block
>> > > of zeroes or data.
>> > >
>> > > Nir: If we could get information from imageio about whether zeroing is
>> > > implemented efficiently or not by the backend, we could change
>> > > virt-v2v / nbdkit to advertise this back to qemu.
>> >
>> > There is no way to detect the capability, ioctl(BLKZEROOUT) always
>> > succeeds, falling back to manual zeroing in the kernel silently
>> >
>> > Even if we could, sending zero on the wire from qemu may be even
>> > slower, and it looks like qemu send even more requests in this case
>> > (2154 vs ~1300).
>> >
>> > Looks like this optimization in qemu side leads to worse performance,
>> > so it should not be enabled by default.
>>
>> Well, that's overgeneralising your case a bit. If the backend does
>> support efficient zero writes (which file systems, the most common case,
>> generally do), doing one big write_zeroes request at the start can
>> improve performance quite a bit.
>>
>> It seems the problem is that we can't really know whether the operation
>> will be efficient because the backends generally don't tell us. Maybe
>> NBD could introduce a flag for this, but in the general case it appears
>> to me that we'll have to have a command line option.
>>
>> However, I'm curious what your exact use case and the backend used in it
>> is? Can something be improved there to actually get efficient zero
>> writes and get even better performance than by just disabling the big
>> zero write?
>
>
> The backend is some NetApp storage connected via FC. I don't have
> more info on this. We get zero rate of about 1G/s on this storage, which
> is quite slow compared with other storage we tested.
>
> One option we check now is if this is the kernel silent fallback to manual
> zeroing when the server advertise wrong value of write_same_max_bytes.
>
We eliminated this using blkdiscard. This is what we get on with this
storage
zeroing 100G LV:
for i in 1 2 4 8 16 32; do time blkdiscard -z -p ${i}m
/dev/6e1d84f9-f939-46e9-b108-0427a08c280c/2d5c06ce-6536-4b3c-a7b6-13c6d8e55ade;
done
real 4m50.851s
user 0m0.065s
sys 0m1.482s
real 4m30.504s
user 0m0.047s
sys 0m0.870s
real 4m19.443s
user 0m0.029s
sys 0m0.508s
real 4m13.016s
user 0m0.020s
sys 0m0.284s
real 2m45.888s
user 0m0.011s
sys 0m0.162s
real 2m10.153s
user 0m0.003s
sys 0m0.100s
We are investigating why we get low throughput on this server, and also
will check
several other servers.
Having a command line option to control this behavior sounds good. I don't
> have enough data to tell what should be the default, but I think the safe
> way would be to keep old behavior.
>
We file this bug:
https://bugzilla.redhat.com/1648622
Nir
[Qemu-devel] [PATCH v2 1/1] virtio-net: support RSC v4/v6 tcp traffic for Windows HCK
This commit adds implementation of RX packets coalescing, compatible with requirements of Windows Hardware compatibility kit. The device enables feature VIRTIO_NET_F_RSC_EXT in host features if it supports extended RSC functionality as defined in the specification. This feature requires at least one of VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6. Windows guest driver acks this feature only if VIRTIO_NET_F_CTRL_GUEST_OFFLOADS is also present. If the guest driver acks VIRTIO_NET_F_RSC_EXT feature, the device coalesces TCPv4 and TCPv6 packets (if respective VIRTIO_NET_F_GUEST_TSO feature is on, populates extended RSC information in virtio header and sets VIRTIO_NET_HDR_F_RSC_INFO bit in header flags. The device does not recalculate checksums in the coalesced packet, so they are not valid. In this case: All the data packets in a tcp connection are cached to a single buffer in every receive interval, and will be sent out via a timer, the 'virtio_net_rsc_timeout' controls the interval, this value may impact the performance and response time of tcp connection, 5(50us) is an experience value to gain a performance improvement, since the whql test sends packets every 100us, so '30(300us)' passes the test case, it is the default value as well, tune it via the command line parameter 'rsc_interval' within 'virtio-net-pci' device, for example, to launch a guest with interval set as '50': 'virtio-net-pci,netdev=hostnet1,bus=pci.0,id=net1,mac=00, guest_rsc_ext=on,rsc_interval=50' The timer will only be triggered if the packets pool is not empty, and it'll drain off all the cached packets. 'NetRscChain' is used to save the segments of IPv4/6 in a VirtIONet device. A new segment becomes a 'Candidate' as well as it passed sanity check, the main handler of TCP includes TCP window update, duplicated ACK check and the real data coalescing. An 'Candidate' segment means: 1. Segment is within current window and the sequence is the expected one. 2. 'ACK' of the segment is in the valid window. Sanity check includes: 1. Incorrect version in IP header 2. An IP options or IP fragment 3. Not a TCP packet 4. Sanity size check to prevent buffer overflow attack. 5. An ECN packet Even though, there might more cases should be considered such as ip identification other flags, while it breaks the test because windows set it to the same even it's not a fragment. Normally it includes 2 typical ways to handle a TCP control flag, 'bypass' and 'finalize', 'bypass' means should be sent out directly, while 'finalize' means the packets should also be bypassed, but this should be done after search for the same connection packets in the pool and drain all of them out, this is to avoid out of order fragment. All the 'SYN' packets will be bypassed since this always begin a new' connection, other flags such 'URG/FIN/RST/CWR/ECE' will trigger a finalization, because this normally happens upon a connection is going to be closed, an 'URG' packet also finalize current coalescing unit. Statistics can be used to monitor the basic coalescing status, the 'out of order' and 'out of window' means how many retransmitting packets, thus describe the performance intuitively. Difference between ip v4 and v6 processing: Fragment length in ipv4 header includes itself, while it's not included for ipv6, thus means ipv6 can carry a real 65535 payload. Note that main goal of implementing this feature in software is to create reference setup for certification tests. In such setups guest migration is not required, so the coalesced packets not yet delivered to the guest will be lost in case of migration. Signed-off-by: Wei Xu Signed-off-by: Yuri Benditovich --- hw/net/virtio-net.c| 667 - include/hw/virtio/virtio-net.h | 81 include/net/eth.h | 2 + 3 files changed, 749 insertions(+), 1 deletion(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 385b1a03e9..5a3952f84b 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -41,6 +41,47 @@ #define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE #define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE +#define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */ + +#define VIRTIO_NET_TCP_FLAG 0x3F +#define VIRTIO_NET_TCP_HDR_LENGTH 0xF000 + +/* IPv4 max payload, 16 bits in the header */ +#define VIRTIO_NET_MAX_IP4_PAYLOAD (65535 - sizeof(struct ip_header)) +#define VIRTIO_NET_MAX_TCP_PAYLOAD 65535 + +/* header length value in ip header without option */ +#define VIRTIO_NET_IP4_HEADER_LENGTH 5 + +#define VIRTIO_NET_IP6_ADDR_SIZE 32 /* ipv6 saddr + daddr */ +#define VIRTIO_NET_MAX_IP6_PAYLOAD VIRTIO_NET_MAX_TCP_PAYLOAD + +/* Purge coalesced packets timer interval, This value affects the performance + a lot, and should be tuned carefully, '30'(300us) is the recommended + value to pass the WHQL test, '5' can gain 2x netperf throughput with +
[Qemu-devel] [PATCH v2 0/1] virtio-net: support RSC v4/v6 tcp traffic for Windows HCK
Changes from v1: * Changes in standard header moved to virtio_net.c * added inline procedures for header fields access * Removed unused define * Removed unrelated changes * Renamed RSC timeout define * Removed unnecessary check for VHOST * RSC struct names prefixed by Virtio * Added note about migration * Mail address in signature * Style fixes Yuri Benditovich (1): virtio-net: support RSC v4/v6 tcp traffic for Windows HCK hw/net/virtio-net.c| 667 - include/hw/virtio/virtio-net.h | 81 include/net/eth.h | 2 + 3 files changed, 749 insertions(+), 1 deletion(-) -- 2.17.1
Re: [Qemu-devel] Change in qemu 2.12 causes qemu-img convert to NBD to write more data
On Wed, Nov 7, 2018 at 6:42 PM Eric Blake wrote: > On 11/7/18 6:13 AM, Richard W.M. Jones wrote: > > (I'm not going to claim this is a bug, but it causes a large, easily > > measurable performance regression in virt-v2v). > > I haven't closely looked at at this email thread yet, but a quick first > impression: > > > > In qemu 2.12 this behaviour changed: > > > >$ nbdkit --filter=log memory size=6G logfile=/tmp/log \ > >--run './qemu-img convert ./fedora-28.img -n $nbd' > >$ grep '\.\.\.$' /tmp/log | sed 's/.*\([A-Z][a-z]*\).*/\1/' | uniq -c > >193 Zero > > 1246 Write > > > > It now zeroes the whole disk up front and then writes data over the > > top of the zeroed blocks. > > > > The reason for the performance regression is that in the first case we > > write 6G in total. In the second case we write 6G of zeroes up front, > > followed by the amount of data in the disk image (in this case the > > test disk image contains 1G of non-sparse data, so we write about 7G > > in total). > > There was talk on the NBD list a while ago about the idea of letting the > server advertise to the client when the image is known to start in an > all-zero state, so that the client doesn't have to waste time writing > zeroes (or relying on repeated NBD_CMD_BLOCK_STATUS calls to learn the > same). This may be justification for reviving that topic. > This is a good idea in general, since in some cases we know that a volume is already zeroed (e.g. new file on NFS/Gluster storage). But with block storage, we typically don't have any guarantee about storage content, and qemu need to zero or write the entire device, so this does not solve the issue discussed in this thread. Nir
Re: [Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave
> On Nov 11, 2018, at 6:55 AM, qemu-devel-requ...@nongnu.org wrote: > > The code for the cocoa stuff is in ui/cocoa.m. Quick notes on structure: > > * there is a weird thing where cocoa.m provides its own main(), and arranges > that the function which is main() for every other UI is renamed qemu_main() > and called later (I'd like to get rid of that one day if we could, it's just > weird) > * cocoa_display_init() is the "initialize the display" entry point -- this > will always be called from on the main thread (strictly, from whichever > thread OSX calls our applicationDidFinishLaunching callback on, but I assume > that's the main thread) > * the runtime entry points into the cocoa UI code are just the functions in > the DisplayChangeListener struct: cocoa_update(), cocoa_switch() and > cocoa_refresh() > > Arranging for the last 3 to schedule their operation onto the main > thread is probably what's needed. Things I don't know: > > * should this "run thing on main thread" be synchronous or asynchronous? > (sync is probably safest) synchronous sounds good. > * what's the right OSX API to do this? https://developer.apple.com/documentation/objectivec/nsobject/1414900-performselectoronmainthread?language=objc > * how can we most cleanly do this in a way that still works on OSX 10.6 (the > oldest we currently support)? (I suspect we'll need ifdefs and fall back to > "just run on this thread" on older versions) I would make another function called switchSurfaceInternal: and then move all the code from switchSurface: to this new function. Then have the switchSurface: method call the switchSurfaceInternal: method by using [performSelectorOnMainThread:withObject:waitUntilDone:]. The call would look like this: [self performSelectorOnMainThread: @selector(switchSurface:) withObject: surface waitUntilDone: YES] I'm not sure if waitUntilDone should be set to YES. QEMU might work faster if it is set to NO. Thank you.
Re: [Qemu-devel] [PATCH v3 7/7] arm: fix aa64_generate_debug_exceptions to work with EL2
On 9 November 2018 at 15:21, Alex Bennée wrote: > The test was incomplete and incorrectly caused debug exceptions to be > generated when returning to EL2 after a failed attempt to single-step > an EL1 instruction. Fix this while cleaning up the function a little. > @@ -2833,9 +2845,6 @@ static inline bool > aa32_generate_debug_exceptions(CPUARMState *env) > * since the pseudocode has it at all callsites except for the one in > * CheckSoftwareStep(), where it is elided because both branches would > * always return the same value. > - * > - * Parts of the pseudocode relating to EL2 and EL3 are omitted because we > - * don't yet implement those exception levels or their associated trap bits. > */ In hindsight I regret not standardizing on a greppable tag for marking these kinds of "we don't do X because we don't implement feature Y" comments... thanks -- PMM
[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave
> On Nov 11, 2018, at 2:39 AM, qemu-devel-requ...@nongnu.org wrote: > > Thanks for the bug report. It looks like Mojave is pickier about apps > not calling various GUI update functions from the "wrong" thread. We > probably need to figure out how to dispatch those to the main thread > instead of whatever thread we were on. Unfortunately we don't really > have anybody in QEMU upstream who knows much about OSX or its GUI, and I > suspect we don't have anybody with Mojave (my system is still High > Sierra and I don't plan to upgrade it for a while); help and patches > appreciated from anybody who does... > 21 AppKit 0x7fff3c019ff8 -[NSWindow > setFrame:display:animate:] + 567 I would use the [performSelectorOnMainThread: withObject: waitUntilDone:] method to fix this problem. It should go here in the call stack. I would make the patch myself but I don't know where this call takes place in qemu-system-aarch64. > 22 qemu-system-aarch64 0x00010b7b2abf > qemu-system-aarch64 + 3668671
Re: [Qemu-devel] [PATCH v3 7/7] arm: fix aa64_generate_debug_exceptions to work with EL2
On 11/9/18 4:21 PM, Alex Bennée wrote: > The test was incomplete and incorrectly caused debug exceptions to be > generated when returning to EL2 after a failed attempt to single-step > an EL1 instruction. Fix this while cleaning up the function a little. > > Signed-off-by: Alex Bennée > > --- > v3 > - further re-arrangement as suggested by rth > --- > target/arm/cpu.h | 39 --- > 1 file changed, 24 insertions(+), 15 deletions(-) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH v3 5/7] tests/guest-debug: don't use symbol resolution for PC checks
On 11/9/18 4:21 PM, Alex Bennée wrote: > It turns out symbol resolution isn't enough as modern kernels are > often padded with check code at the start of functions. GDB seems to > put the breakpoint at the first non-check instruction which causes > comparisons with the symbol resolution to fail. If you want breakpoints at a fixed location, use "*symbol", which will disable gdb's prologue checking. r~
Re: [Qemu-devel] [PATCH v3 1/7] target/arm64: properly handle DBGVR RESS bits
On 11/9/18 4:21 PM, Alex Bennée wrote: > This only fails with some (broken) versions of gdb but we should > treat the top bits of DBGBVR as RESS. Properly sign extend QEMU's > reference copy of dbgbvr and also update the register descriptions in > the comment. > > Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson r~
[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave
Thanks for having a look at this. The cocoa UI does work for me on High Sierra, for what that's worth. https://wiki.qemu.org/Contribute/SubmitAPatch has our patch submission process. My feeling on SDL is that this would be a bug to fix in upstream SDL, assuming we're not breaking any "which thread" requirements in the SDL API. It's the job of the SDL abstraction layer to work around host-OS- specific issues. (I didn't realize that the SDL display code worked on OSX QEMU, though -- the only one I've ever used is the Cocoa one, and I would expect anything else to interact weirdly with the way the cocoa UI frontend assumes it's in control.) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1802684 Title: QEMU gui crashes on macOS Mojave Status in QEMU: New Bug description: QEMU release 3.0.0 as well as a recent head build /usr/local/Cellar/qemu/HEAD-03c1ca1 (147 files, 257.2MB) Built from source on 2018-11-06 at 13:41:32 with: --with-gtk+3 --with-sdl2 --with-libusb /usr/local/Cellar/qemu/3.0.0 (137 files, 261.6MB) * Poured from bottle on 2018-11-10 at 22:58:32 with: --with-gtk+3 --with-libusb --with-sdl2 Crashes when attempting to use any gui interface (tried SDL and default Cocoa): 2018-11-10 22:58:41.799 qemu-system-aarch64[42982:1102466] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSWindow drag regions should only be invalidated on the Main Thread!' *** First throw call stack: ( 0 CoreFoundation 0x7fff3ea96ecd __exceptionPreprocess + 256 1 libobjc.A.dylib 0x7fff6ab49720 objc_exception_throw + 48 2 CoreFoundation 0x7fff3eab095d -[NSException raise] + 9 3 AppKit 0x7fff3bfb13fa -[NSWindow(NSWindow_Theme) _postWindowNeedsToResetDragMarginsUnlessPostingDisabled] + 324 4 AppKit 0x7fff3bfb6850 -[NSView setFrameSize:] + 2082 5 AppKit 0x7fff3c02747d -[NSVisualEffectView setFrameSize:] + 171 6 AppKit 0x7fff3c0811b1 -[NSTitlebarView setFrameSize:] + 84 7 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 8 AppKit 0x7fff3c081154 -[NSTitlebarView resizeWithOldSuperviewSize:] + 100 9 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 10 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 11 AppKit 0x7fff3c9773c0 -[NSTitlebarContainerView setFrameSize:] + 142 12 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 13 AppKit 0x7fff3bfbcdb5 -[NSView resizeWithOldSuperviewSize:] + 776 14 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 15 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 16 AppKit 0x7fff3c024570 -[NSThemeFrame setFrameSize:] + 495 17 AppKit 0x7fff3c011223 -[NSWindow _setFrame:updateBorderViewSize:] + 966 18 AppKit 0x7fff3c010b46 -[NSWindow _oldPlaceWindow:] + 547 19 AppKit 0x7fff3c010151 -[NSWindow _setFrameCommon:display:stashSize:] + 3006 20 AppKit 0x7fff3c00f57d -[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] + 192 21 AppKit 0x7fff3c019ff8 -[NSWindow setFrame:display:animate:] + 567 22 qemu-system-aarch64 0x00010b7b2abf qemu-system-aarch64 + 3668671 23 qemu-system-aarch64 0x00010b7b6356 qemu-system-aarch64 + 3683158 24 qemu-system-aarch64 0x00010b7ad836 qemu-system-aarch64 + 3647542 25 qemu-system-aarch64 0x00010b4ce769 qemu-system-aarch64 + 636777 26 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 27 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 28 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 29 qemu-system-aarch64 0x00010b4414aa qemu-system-aarch64 + 58538 30 qemu-system-aarch64 0x00010b4f78c3 qemu-system-aarch64 + 805059 31 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 32 qemu-system-aarch64
Re: [Qemu-devel] proper-dev dependencies..
On 11 November 2018 at 12:07, watk...@hungry.com wrote: > I've been following the list for awhile does anyone have a proper list of > all the ubuntu dependencies? > So, one can compile the whole ball of wax properly? If you're running on Ubuntu and have your apt sources set up to include source packages, the simplest thing is to run "apt build-dep qemu", which will install all the packages that the Ubuntu package of QEMU build-depends on. QEMU's build-dependencies don't change radically over time, so that's usually sufficient (even though the Ubuntu package will be for a QEMU that's a little older than the one you're going to be building from source). You can also look at the build page on our wiki which has a list of required and extra packages: https://wiki.qemu.org/Hosts/Linux#QEMU_on_Linux_hosts If there are any missing from the list let us know and we can update the wiki. thanks -- PMM
[Qemu-devel] proper-dev dependencies..
I've been following the list for awhile does anyone have a proper list of all the ubuntu dependencies? So, one can compile the whole ball of wax properly?
[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave
I made DisplayChangeListener callbacks dispatch updates to the main thread and it stopped crashing. However, pure Cocoa UI seems non- functional - I can't focus the window, I don't see any application menus, and the fb does not update. I'm looking at making SDL code thread-safe the same way - because it also calls into Cocoa, and crashes in the same way. What is the process for submitting PRs to qemu? I'm used to github but I see you use your own git hosting. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1802684 Title: QEMU gui crashes on macOS Mojave Status in QEMU: New Bug description: QEMU release 3.0.0 as well as a recent head build /usr/local/Cellar/qemu/HEAD-03c1ca1 (147 files, 257.2MB) Built from source on 2018-11-06 at 13:41:32 with: --with-gtk+3 --with-sdl2 --with-libusb /usr/local/Cellar/qemu/3.0.0 (137 files, 261.6MB) * Poured from bottle on 2018-11-10 at 22:58:32 with: --with-gtk+3 --with-libusb --with-sdl2 Crashes when attempting to use any gui interface (tried SDL and default Cocoa): 2018-11-10 22:58:41.799 qemu-system-aarch64[42982:1102466] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSWindow drag regions should only be invalidated on the Main Thread!' *** First throw call stack: ( 0 CoreFoundation 0x7fff3ea96ecd __exceptionPreprocess + 256 1 libobjc.A.dylib 0x7fff6ab49720 objc_exception_throw + 48 2 CoreFoundation 0x7fff3eab095d -[NSException raise] + 9 3 AppKit 0x7fff3bfb13fa -[NSWindow(NSWindow_Theme) _postWindowNeedsToResetDragMarginsUnlessPostingDisabled] + 324 4 AppKit 0x7fff3bfb6850 -[NSView setFrameSize:] + 2082 5 AppKit 0x7fff3c02747d -[NSVisualEffectView setFrameSize:] + 171 6 AppKit 0x7fff3c0811b1 -[NSTitlebarView setFrameSize:] + 84 7 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 8 AppKit 0x7fff3c081154 -[NSTitlebarView resizeWithOldSuperviewSize:] + 100 9 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 10 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 11 AppKit 0x7fff3c9773c0 -[NSTitlebarContainerView setFrameSize:] + 142 12 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 13 AppKit 0x7fff3bfbcdb5 -[NSView resizeWithOldSuperviewSize:] + 776 14 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 15 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 16 AppKit 0x7fff3c024570 -[NSThemeFrame setFrameSize:] + 495 17 AppKit 0x7fff3c011223 -[NSWindow _setFrame:updateBorderViewSize:] + 966 18 AppKit 0x7fff3c010b46 -[NSWindow _oldPlaceWindow:] + 547 19 AppKit 0x7fff3c010151 -[NSWindow _setFrameCommon:display:stashSize:] + 3006 20 AppKit 0x7fff3c00f57d -[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] + 192 21 AppKit 0x7fff3c019ff8 -[NSWindow setFrame:display:animate:] + 567 22 qemu-system-aarch64 0x00010b7b2abf qemu-system-aarch64 + 3668671 23 qemu-system-aarch64 0x00010b7b6356 qemu-system-aarch64 + 3683158 24 qemu-system-aarch64 0x00010b7ad836 qemu-system-aarch64 + 3647542 25 qemu-system-aarch64 0x00010b4ce769 qemu-system-aarch64 + 636777 26 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 27 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 28 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 29 qemu-system-aarch64 0x00010b4414aa qemu-system-aarch64 + 58538 30 qemu-system-aarch64 0x00010b4f78c3 qemu-system-aarch64 + 805059 31 qemu-system-aarch64 0x00010b487c24 qemu-system-aarch64 + 347172 32 qemu-system-aarch64 0x00010b487a15 qemu-system-aarch64 + 346645 33 qemu-system-aarch64 0x00010b4878f1 qemu-system-aarch64 + 346353 34
[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave
The code for the cocoa stuff is in ui/cocoa.m. Quick notes on structure: * there is a weird thing where cocoa.m provides its own main(), and arranges that the function which is main() for every other UI is renamed qemu_main() and called later (I'd like to get rid of that one day if we could, it's just weird) * cocoa_display_init() is the "initialize the display" entry point -- this will always be called from on the main thread (strictly, from whichever thread OSX calls our applicationDidFinishLaunching callback on, but I assume that's the main thread) * the runtime entry points into the cocoa UI code are just the functions in the DisplayChangeListener struct: cocoa_update(), cocoa_switch() and cocoa_refresh() Arranging for the last 3 to schedule their operation onto the main thread is probably what's needed. Things I don't know: * should this "run thing on main thread" be synchronous or asynchronous? (sync is probably safest) * what's the right OSX API to do this? * how can we most cleanly do this in a way that still works on OSX 10.6 (the oldest we currently support)? (I suspect we'll need ifdefs and fall back to "just run on this thread" on older versions) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1802684 Title: QEMU gui crashes on macOS Mojave Status in QEMU: New Bug description: QEMU release 3.0.0 as well as a recent head build /usr/local/Cellar/qemu/HEAD-03c1ca1 (147 files, 257.2MB) Built from source on 2018-11-06 at 13:41:32 with: --with-gtk+3 --with-sdl2 --with-libusb /usr/local/Cellar/qemu/3.0.0 (137 files, 261.6MB) * Poured from bottle on 2018-11-10 at 22:58:32 with: --with-gtk+3 --with-libusb --with-sdl2 Crashes when attempting to use any gui interface (tried SDL and default Cocoa): 2018-11-10 22:58:41.799 qemu-system-aarch64[42982:1102466] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSWindow drag regions should only be invalidated on the Main Thread!' *** First throw call stack: ( 0 CoreFoundation 0x7fff3ea96ecd __exceptionPreprocess + 256 1 libobjc.A.dylib 0x7fff6ab49720 objc_exception_throw + 48 2 CoreFoundation 0x7fff3eab095d -[NSException raise] + 9 3 AppKit 0x7fff3bfb13fa -[NSWindow(NSWindow_Theme) _postWindowNeedsToResetDragMarginsUnlessPostingDisabled] + 324 4 AppKit 0x7fff3bfb6850 -[NSView setFrameSize:] + 2082 5 AppKit 0x7fff3c02747d -[NSVisualEffectView setFrameSize:] + 171 6 AppKit 0x7fff3c0811b1 -[NSTitlebarView setFrameSize:] + 84 7 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 8 AppKit 0x7fff3c081154 -[NSTitlebarView resizeWithOldSuperviewSize:] + 100 9 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 10 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 11 AppKit 0x7fff3c9773c0 -[NSTitlebarContainerView setFrameSize:] + 142 12 AppKit 0x7fff3bfb5859 -[NSView setFrame:] + 478 13 AppKit 0x7fff3bfbcdb5 -[NSView resizeWithOldSuperviewSize:] + 776 14 AppKit 0x7fff3bfbc95e -[NSView resizeSubviewsWithOldSize:] + 502 15 AppKit 0x7fff3bfb66d9 -[NSView setFrameSize:] + 1707 16 AppKit 0x7fff3c024570 -[NSThemeFrame setFrameSize:] + 495 17 AppKit 0x7fff3c011223 -[NSWindow _setFrame:updateBorderViewSize:] + 966 18 AppKit 0x7fff3c010b46 -[NSWindow _oldPlaceWindow:] + 547 19 AppKit 0x7fff3c010151 -[NSWindow _setFrameCommon:display:stashSize:] + 3006 20 AppKit 0x7fff3c00f57d -[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] + 192 21 AppKit 0x7fff3c019ff8 -[NSWindow setFrame:display:animate:] + 567 22 qemu-system-aarch64 0x00010b7b2abf qemu-system-aarch64 + 3668671 23 qemu-system-aarch64 0x00010b7b6356 qemu-system-aarch64 + 3683158 24 qemu-system-aarch64 0x00010b7ad836 qemu-system-aarch64 + 3647542 25 qemu-system-aarch64 0x00010b4ce769 qemu-system-aarch64 + 636777 26 qemu-system-aarch64 0x00010b487c24
Re: [Qemu-devel] [RFC] [PATCH] kvm: arm: Introduce error code KVM_EINVARIANT
On Sat, 10 Nov 2018 22:18:47 +, Manish Jaggi wrote: > > > CCing a larger audience. > Please review. > > On 10/23/2018 03:51 PM, Jaggi, Manish wrote: > > From: Manish Jaggi > > > > This patch introduces an error code KVM_EINVARIANT which is returned > > by KVM when userland tries to set an invariant register. > > > > The need for this error code is in VM Migration for arm64. > > ARM64 systems use mainly -machine virt -cpu host as parameter to qemu. > > Migration requires both Source and destination machines to have same > > physical cpu. There are cases where the overall architecture of CPU is > > same but the next version of the chip with some bug fixes which have no > > effect on qemu operation. In such cases invariant registers like MIDR > > have a different value. > > Currently Migration fails in such cases. > > > > Rather than sending a EINVAL, a specifc error code will help > > userland program the guest invariant register by querying the migrated > > host machines invariant registers. But failing migration is a good thing, right? How do you expect that the guest will be happy to see a new CPU revision right in the middle of its execution? Do you also propose that QEMU starts doing that for big-little systems? After all, if ignoring the differences in some registers is harmless for migration, surely that should be the case in a single system, right? > > > > Qemu will have a parameter -hostinvariant along with checking of this > > error code. So it can be safely assumed that the feature is opt-in You're changing the ABI without any buy in from userspace, which is not acceptable. As it stands, this patch creates a number of issues without solving any. Things to think about: - How does errata management works when migrating to a different system? - big-little, as mentioned above - Are all invariant registers equal? A different MIDR has the same effect as a different MMFR0? Instead of papering over architectural constants i a system, how about allowing the relevant ID registers to be overloaded when not incompatible? Thanks, M. -- Jazz is not dead, it just smell funny.
Re: [Qemu-devel] [PATCH 1/2] MAINTAINERS: Add scripts/decodetree.py to the TCG section
On 11/10/18 10:13 PM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) Queued, thanks. r~
Re: [Qemu-devel] [PATCH] decodetree: Force Python to print unsigned values
On 11/11/18 1:02 AM, Philippe Mathieu-Daudé wrote: > Python internal representation is signed, so unsigned values > bigger than 31-bit are interpreted as signed (and printed with > a '-' signed). > Mask out to force unsigned values. > > Signed-off-by: Philippe Mathieu-Daudé > --- > scripts/decodetree.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Queued, thanks. r~
Re: [Qemu-devel] [PATCH 2/2] decodetree: Add multiple include guard
On 11/10/18 10:13 PM, Philippe Mathieu-Daudé wrote: > It is necessary when splitting an ISA, or when using multiple ISAs. > > Signed-off-by: Philippe Mathieu-Daudé > --- > scripts/decodetree.py | 5 + > 1 file changed, 5 insertions(+) I guess I have no problems with this, but can you explain when these headers get included more than once? r~
Re: [Qemu-devel] [PATCH v2 05/22] hw/rdma: Add support for MAD packets
On Sat, Nov 10, 2018 at 08:15:27PM +0200, Marcel Apfelbaum wrote:
> Hi Yuval
>
> On 11/8/18 6:08 PM, Yuval Shaia wrote:
> > MAD (Management Datagram) packets are widely used by various modules
> > both in kernel and in user space for example the rdma_* API which is
> > used to create and maintain "connection" layer on top of RDMA uses
> > several types of MAD packets.
>
> Can you add a link to MAD spec to commit or event in the code?
Have no idea where to take it from, does it requires some subscription or
so?
>
> > To support MAD packets the device uses an external utility
> > (contrib/rdmacm-mux) to relay packets from and to the guest driver.
>
> Can the device be used without MADs support?
Since we have a support now i don't see a reason why we like to use (or
even expose) device with no MAD support.
> If not, can you update the pvrdma documentation to
> reflect the changes?
Sure, missed that, will document the changes in v3.
>
> > Signed-off-by: Yuval Shaia
> > ---
> > hw/rdma/rdma_backend.c | 263 +++-
> > hw/rdma/rdma_backend.h | 4 +-
> > hw/rdma/rdma_backend_defs.h | 10 +-
> > hw/rdma/vmw/pvrdma.h| 2 +
> > hw/rdma/vmw/pvrdma_main.c | 4 +-
> > 5 files changed, 273 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> > index 1e148398a2..3eb0099f8d 100644
> > --- a/hw/rdma/rdma_backend.c
> > +++ b/hw/rdma/rdma_backend.c
> > @@ -16,8 +16,13 @@
> > #include "qemu/osdep.h"
> > #include "qemu/error-report.h"
> > #include "qapi/error.h"
> > +#include "qapi/qmp/qlist.h"
> > +#include "qapi/qmp/qnum.h"
> > #include
> > +#include
> > +#include
> > +#include
> > #include "trace.h"
> > #include "rdma_utils.h"
> > @@ -33,16 +38,25 @@
> > #define VENDOR_ERR_MAD_SEND 0x206
> > #define VENDOR_ERR_INVLKEY 0x207
> > #define VENDOR_ERR_MR_SMALL 0x208
> > +#define VENDOR_ERR_INV_MAD_BUFF 0x209
> > +#define VENDOR_ERR_INV_NUM_SGE 0x210
> > #define THR_NAME_LEN 16
> > #define THR_POLL_TO 5000
> > +#define MAD_HDR_SIZE sizeof(struct ibv_grh)
> > +
> > typedef struct BackendCtx {
> > -uint64_t req_id;
> > void *up_ctx;
> > bool is_tx_req;
> > +struct ibv_sge sge; /* Used to save MAD recv buffer */
> > } BackendCtx;
> > +struct backend_umad {
> > +struct ib_user_mad hdr;
> > +char mad[RDMA_MAX_PRIVATE_DATA];
> > +};
> > +
> > static void (*comp_handler)(int status, unsigned int vendor_err, void
> > *ctx);
> > static void dummy_comp_handler(int status, unsigned int vendor_err, void
> > *ctx)
> > @@ -286,6 +300,49 @@ static int build_host_sge_array(RdmaDeviceResources
> > *rdma_dev_res,
> > return 0;
> > }
> > +static int mad_send(RdmaBackendDev *backend_dev, struct ibv_sge *sge,
> > +uint32_t num_sge)
> > +{
> > +struct backend_umad umad = {0};
> > +char *hdr, *msg;
> > +int ret;
> > +
> > +pr_dbg("num_sge=%d\n", num_sge);
> > +
> > +if (num_sge != 2) {
> > +return -EINVAL;
> > +}
> > +
> > +umad.hdr.length = sge[0].length + sge[1].length;
> > +pr_dbg("msg_len=%d\n", umad.hdr.length);
> > +
> > +if (umad.hdr.length > sizeof(umad.mad)) {
> > +return -ENOMEM;
> > +}
> > +
> > +umad.hdr.addr.qpn = htobe32(1);
> > +umad.hdr.addr.grh_present = 1;
> > +umad.hdr.addr.gid_index = backend_dev->backend_gid_idx;
> > +memcpy(umad.hdr.addr.gid, backend_dev->gid.raw,
> > sizeof(umad.hdr.addr.gid));
> > +umad.hdr.addr.hop_limit = 1;
> > +
> > +hdr = rdma_pci_dma_map(backend_dev->dev, sge[0].addr, sge[0].length);
> > +msg = rdma_pci_dma_map(backend_dev->dev, sge[1].addr, sge[1].length);
> > +
> > +memcpy([0], hdr, sge[0].length);
> > +memcpy([sge[0].length], msg, sge[1].length);
> > +
> > +rdma_pci_dma_unmap(backend_dev->dev, msg, sge[1].length);
> > +rdma_pci_dma_unmap(backend_dev->dev, hdr, sge[0].length);
> > +
> > +ret = qemu_chr_fe_write(backend_dev->mad_chr_be, (const uint8_t
> > *),
> > +sizeof(umad));
> > +
> > +pr_dbg("qemu_chr_fe_write=%d\n", ret);
> > +
> > +return (ret != sizeof(umad));
> > +}
> > +
> > void rdma_backend_post_send(RdmaBackendDev *backend_dev,
> > RdmaBackendQP *qp, uint8_t qp_type,
> > struct ibv_sge *sge, uint32_t num_sge,
> > @@ -304,9 +361,13 @@ void rdma_backend_post_send(RdmaBackendDev
> > *backend_dev,
> > comp_handler(IBV_WC_GENERAL_ERR, VENDOR_ERR_QP0, ctx);
> > } else if (qp_type == IBV_QPT_GSI) {
> > pr_dbg("QP1\n");
> > -comp_handler(IBV_WC_GENERAL_ERR, VENDOR_ERR_MAD_SEND, ctx);
> > +rc = mad_send(backend_dev, sge, num_sge);
> > +if (rc) {
> > +comp_handler(IBV_WC_GENERAL_ERR, VENDOR_ERR_MAD_SEND, ctx);
> > +} else {
> > +
Re: [Qemu-devel] [PATCH] virtio-net: support RSC v4/v6 tcp traffic for Windows HCK
On Fri, Nov 9, 2018 at 8:11 PM Michael S. Tsirkin wrote: > Looks good to me. Some comments below > > On Fri, Nov 09, 2018 at 04:58:27PM +0200, Yuri Benditovich wrote: > > This commit adds implementation of RX packets > > coalescing, compatible with requirements of Windows > > Hardware compatibility kit. > > > > The device enables feature VIRTIO_NET_F_RSC_EXT in > > host features if it supports extended RSC functionality > > as defined in the specification. > > This feature requires at least one of VIRTIO_NET_F_GUEST_TSO4, > > VIRTIO_NET_F_GUEST_TSO6. Windows guest driver acks > > this feature only if VIRTIO_NET_F_CTRL_GUEST_OFFLOADS > > is also present. > > > > In case vhost is enabled the feature bit is cleared in > > host_features during device initialization. > > > > If the guest driver acks VIRTIO_NET_F_RSC_EXT feature, > > the device coalesces TCPv4 and TCPv6 packets (if > > respective VIRTIO_NET_F_GUEST_TSO feature is on, > > populates extended RSC information in virtio header > > and sets VIRTIO_NET_HDR_F_RSC_INFO bit in header flags. > > The device does not recalculate checksums in the coalesced > > packet, so they are not valid. > > > > In this case: > > All the data packets in a tcp connection are cached > > to a single buffer in every receive interval, and will > > be sent out via a timer, the 'virtio_net_rsc_timeout' > > controls the interval, this value may impact the > > performance and response time of tcp connection, > > 5(50us) is an experience value to gain a performance > > improvement, since the whql test sends packets every 100us, > > so '30(300us)' passes the test case, it is the default > > value as well, tune it via the command line parameter > > 'rsc_interval' within 'virtio-net-pci' device, for example, > > to launch a guest with interval set as '50': > > > > 'virtio-net-pci,netdev=hostnet1,bus=pci.0,id=net1,mac=00, > > guest_rsc_ext=on,rsc_interval=50' > > > > The timer will only be triggered if the packets pool is not empty, > > and it'll drain off all the cached packets. > > > > 'NetRscChain' is used to save the segments of IPv4/6 in a > > VirtIONet device. > > > > A new segment becomes a 'Candidate' as well as it passed sanity check, > > the main handler of TCP includes TCP window update, duplicated > > ACK check and the real data coalescing. > > > > An 'Candidate' segment means: > > 1. Segment is within current window and the sequence is the expected one. > > 2. 'ACK' of the segment is in the valid window. > > > > Sanity check includes: > > 1. Incorrect version in IP header > > 2. An IP options or IP fragment > > 3. Not a TCP packet > > 4. Sanity size check to prevent buffer overflow attack. > > 5. An ECN packet > > > > Even though, there might more cases should be considered such as > > ip identification other flags, while it breaks the test because > > windows set it to the same even it's not a fragment. > > > > Normally it includes 2 typical ways to handle a TCP control flag, > > 'bypass' and 'finalize', 'bypass' means should be sent out directly, > > while 'finalize' means the packets should also be bypassed, but this > > should be done after search for the same connection packets in the > > pool and drain all of them out, this is to avoid out of order fragment. > > > > All the 'SYN' packets will be bypassed since this always begin a new' > > connection, other flags such 'URG/FIN/RST/CWR/ECE' will trigger a > > finalization, because this normally happens upon a connection is going > > to be closed, an 'URG' packet also finalize current coalescing unit. > > > > Statistics can be used to monitor the basic coalescing status, the > > 'out of order' and 'out of window' means how many retransmitting packets, > > thus describe the performance intuitively. > > > > Difference between ip v4 and v6 processing: > > Fragment length in ipv4 header includes itself, while it's not > > included for ipv6, thus means ipv6 can carry a real 65535 payload. > > > > Signed-off-by: Wei Xu > > Pls put in the full address, QEMU doesn't accept anonymous > code donations. > > > Signed-off-by: Yuri Benditovich > > --- > > hw/net/virtio-net.c | 648 +++- > > include/hw/virtio/virtio-net.h | 81 +++ > > include/net/eth.h | 2 + > > include/standard-headers/linux/virtio_net.h | 8 + > > 4 files changed, 734 insertions(+), 5 deletions(-) > > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > index 385b1a03e9..43a7021409 100644 > > --- a/hw/net/virtio-net.c > > +++ b/hw/net/virtio-net.c > > @@ -41,6 +41,28 @@ > > #define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE > > #define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE > > > > +#define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */ > > + > > +#define VIRTIO_NET_TCP_FLAG 0x3F > > +#define VIRTIO_NET_TCP_HDR_LENGTH 0xF000 > > + > > +/* IPv4 max payload, 16 bits in the header */ >
Re: [Qemu-devel] [PATCH for-3.1] fdc: fix segfault in fdctrl_stop_transfer() when DMA is disabled
On Sun, Nov 11, 2018 at 10:41 AM Mark Cave-Ayland
wrote:
> Commit c8a35f1cf0f "fdc: use IsaDma interface instead of global DMA_*
> functions" accidentally introduced a segfault in fdctrl_stop_transfer() for
> non-DMA transfers.
>
> If fdctrl->dma_chann has not been configured then the fdctrl->dma interface
> reference isn't initialised during isabus_fdc_realize(). Unfortunately
> fdctrl_stop_transfer() unconditionally references the DMA interface when
> finishing the transfer causing a NULL pointer dereference.
>
> Fix the issue by adding a check in fdctrl_stop_transfer() so that the DMA
> interface reference and release method is only invoked if fdctrl->dma_chann
> has been set.
>
> (This issue was discovered by Martin testing a recent change in the NetBSD
> installer under qemu-system-sparc)
>
> Reported-by: Martin Husemann
> Signed-off-by: Mark Cave-Ayland
> ---
> hw/block/fdc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 2e9c1e1e2f..6f19f127a5 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -1617,7 +1617,7 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl,
> uint8_t status0,
> fdctrl->fifo[5] = cur_drv->sect;
> fdctrl->fifo[6] = FD_SECTOR_SC;
> fdctrl->data_dir = FD_DIR_READ;
> -if (!(fdctrl->msr & FD_MSR_NONDMA)) {
> +if (fdctrl->dma_chann != -1 && !(fdctrl->msr & FD_MSR_NONDMA)) {
Reviewed-by: Philippe Mathieu-Daudé
> IsaDmaClass *k = ISADMA_GET_CLASS(fdctrl->dma);
> k->release_DREQ(fdctrl->dma, fdctrl->dma_chann);
> }
> --
> 2.11.0
>
>
[Qemu-devel] [PATCH for-3.1] fdc: fix segfault in fdctrl_stop_transfer() when DMA is disabled
Commit c8a35f1cf0f "fdc: use IsaDma interface instead of global DMA_*
functions" accidentally introduced a segfault in fdctrl_stop_transfer() for
non-DMA transfers.
If fdctrl->dma_chann has not been configured then the fdctrl->dma interface
reference isn't initialised during isabus_fdc_realize(). Unfortunately
fdctrl_stop_transfer() unconditionally references the DMA interface when
finishing the transfer causing a NULL pointer dereference.
Fix the issue by adding a check in fdctrl_stop_transfer() so that the DMA
interface reference and release method is only invoked if fdctrl->dma_chann
has been set.
(This issue was discovered by Martin testing a recent change in the NetBSD
installer under qemu-system-sparc)
Reported-by: Martin Husemann
Signed-off-by: Mark Cave-Ayland
---
hw/block/fdc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 2e9c1e1e2f..6f19f127a5 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1617,7 +1617,7 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t
status0,
fdctrl->fifo[5] = cur_drv->sect;
fdctrl->fifo[6] = FD_SECTOR_SC;
fdctrl->data_dir = FD_DIR_READ;
-if (!(fdctrl->msr & FD_MSR_NONDMA)) {
+if (fdctrl->dma_chann != -1 && !(fdctrl->msr & FD_MSR_NONDMA)) {
IsaDmaClass *k = ISADMA_GET_CLASS(fdctrl->dma);
k->release_DREQ(fdctrl->dma, fdctrl->dma_chann);
}
--
2.11.0
Re: [Qemu-devel] [PATCH v2 03/22] hw/rdma: Return qpn 1 if ibqp is NULL
On Sat, Nov 10, 2018 at 07:59:00PM +0200, Marcel Apfelbaum wrote:
> Hi Yuval,
>
> On 11/8/18 6:07 PM, Yuval Shaia wrote:
> > Device is not supporting QP0, only QP1.
> >
> > Signed-off-by: Yuval Shaia
> > ---
> > hw/rdma/rdma_backend.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h
> > index 86e8fe8ab6..3ccc9a2494 100644
> > --- a/hw/rdma/rdma_backend.h
> > +++ b/hw/rdma/rdma_backend.h
> > @@ -33,7 +33,7 @@ static inline union ibv_gid
> > *rdma_backend_gid(RdmaBackendDev *dev)
> > static inline uint32_t rdma_backend_qpn(const RdmaBackendQP *qp)
> > {
> > -return qp->ibqp ? qp->ibqp->qp_num : 0;
> > +return qp->ibqp ? qp->ibqp->qp_num : 1;
>
> Just to be sure, what are the cases we don't get a qp_num?
> Can we assume all of them are MADs?
>
> Thanks,
> Marcel
qp->ibqp is set only in case that QP type is not QP 1 (see
rdma_backend_create_qp()) so we can safely assume that this is QP 1.
>
> > }
> > static inline uint32_t rdma_backend_mr_lkey(const RdmaBackendMR *mr)
>
Re: [Qemu-devel] [PATCH v2 08/22] hw/pvrdma: Set the correct opcode for recv completion
On Sat, Nov 10, 2018 at 08:18:58PM +0200, Marcel Apfelbaum wrote: > > > On 11/8/18 6:08 PM, Yuval Shaia wrote: > > The function pvrdma_post_cqe populates CQE entry with opcode from the > > given completion element. For receive operation value was not set. Fix > > it by setting it to IBV_WC_RECV. > > > > Signed-off-by: Yuval Shaia > > --- > > hw/rdma/vmw/pvrdma_qp_ops.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c > > index 762700a205..7b0f440fda 100644 > > --- a/hw/rdma/vmw/pvrdma_qp_ops.c > > +++ b/hw/rdma/vmw/pvrdma_qp_ops.c > > @@ -196,8 +196,9 @@ int pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle) > > comp_ctx = g_malloc(sizeof(CompHandlerCtx)); > > comp_ctx->dev = dev; > > comp_ctx->cq_handle = qp->recv_cq_handle; > > -comp_ctx->cqe.qp = qp_handle; > > comp_ctx->cqe.wr_id = wqe->hdr.wr_id; > > +comp_ctx->cqe.qp = qp_handle; > > Not sure the above chunk is needed. Right, it is not related to the change but did it "while there" to be consisted with settings order in pvrdma_qp_send :) > > > +comp_ctx->cqe.opcode = IBV_WC_RECV; > > Anyway > > Reviewed-by: Marcel Apfelbaum Thanks. > > Thanks, > Marcel > > > rdma_backend_post_recv(>backend_dev, >rdma_dev_res, > > >backend_qp, qp->qp_type, >
Re: [Qemu-devel] [PATCH v2 09/22] hw/pvrdma: Set the correct opcode for send completion
On Sat, Nov 10, 2018 at 08:21:51PM +0200, Marcel Apfelbaum wrote: > > > On 11/8/18 6:08 PM, Yuval Shaia wrote: > > opcode for WC should be set by the device and not taken from work > > element. > > > > Signed-off-by: Yuval Shaia > > --- > > hw/rdma/vmw/pvrdma_qp_ops.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c > > index 7b0f440fda..3388be1926 100644 > > --- a/hw/rdma/vmw/pvrdma_qp_ops.c > > +++ b/hw/rdma/vmw/pvrdma_qp_ops.c > > @@ -154,7 +154,7 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle) > > comp_ctx->cq_handle = qp->send_cq_handle; > > comp_ctx->cqe.wr_id = wqe->hdr.wr_id; > > comp_ctx->cqe.qp = qp_handle; > > -comp_ctx->cqe.opcode = wqe->hdr.opcode; > > +comp_ctx->cqe.opcode = IBV_WC_SEND; > > That is interesting, what should happen if the opcode in hdr is different? > Maybe fail the operation? openmpi builds its entire IB state machine on that, see here: https://github.com/open-mpi/ompi/blob/3dc1629771177a883cd8f1be6e97ab152e0f4584/opal/mca/btl/openib/btl_openib_component.c#L3512 > > Thanks, > Marcel > > > rdma_backend_post_send(>backend_dev, >backend_qp, > > qp->qp_type, > > (struct ibv_sge *)>sge[0], > > wqe->hdr.num_sge, >
