Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 2014/11/20 15:50, Jason Wang wrote:
Maybe just initialize iov unconditionally at the beginning and check
dot1q_buf instead of iov for the rest of the functions. (Need deal with
size ETHER_ADDR_LEN * 2)
More complicated, because we can't initialize iov when
size ETHER_ADDR_LEN * 2.
Best regards,
-Gonglei
Probably not: you can just do something like:
if (dot1q_buf size ETHER_ADDR_LEN * 2) {
dot1q_buf = NULL;
}
and check dot1q_buf afterwards. Or just drop the packet since its size
was less than mininum frame length that Ethernet allows.
Sorry, I don't understand. But,
what's your meaning initialize iov unconditionally at the beginning?
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 11/20/2014 04:05 PM, Gonglei wrote:
On 2014/11/20 15:50, Jason Wang wrote:
Maybe just initialize iov unconditionally at the beginning and check
dot1q_buf instead of iov for the rest of the functions. (Need deal with
size ETHER_ADDR_LEN * 2)
More complicated, because we can't initialize iov when
size ETHER_ADDR_LEN * 2.
Best regards,
-Gonglei
Probably not: you can just do something like:
if (dot1q_buf size ETHER_ADDR_LEN * 2) {
dot1q_buf = NULL;
}
and check dot1q_buf afterwards. Or just drop the packet since its size
was less than mininum frame length that Ethernet allows.
Sorry, I don't understand. But,
what's your meaning initialize iov unconditionally at the beginning?
Something like:
@@ -1774,7 +1774,12 @@ static uint32_t
rtl8139_RxConfig_read(RTL8139State *s)
static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
int do_interrupt, const uint8_t *dot1q_buf)
{
-struct iovec *iov = NULL;
+struct iovec iov[3] = {
+{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 },
+{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN },
+{ .iov_base = buf + ETHER_ADDR_LEN * 2,
+ .iov_len = size - ETHER_ADDR_LEN * 2 },
+};
and assign dot1q_buf to NULL is size is not ok.
Just a suggestion, your call.
Re: [Qemu-devel] [PATCH 2/5] exec: qemu_ram_alloc_device, qemu_ram_resize
Hi, I don't know why RHEL7 SeaBIOS does not work on RHEL6. But note that it's a really old version (0.12). Hmm, works for me on a quick smoke test. Do you remember what exactly broke and which version it was? Maybe the 1.7.2 - 1.7.5 update fixed it? Or was it live-migration by chance? rhel6 qemu doesn't emulate pam registers correctly, and seabios has a workaround for that. So live-migrating between versions with and without correct pam emulation creates some *ahem* very interesting corner cases ... cheers, Gerd
Re: [Qemu-devel] [PATCH for-2.2] acpi-build: mark RAM dirty on table update
On Thu, 20 Nov 2014 09:49:20 +0530
Amit Shah amit.s...@redhat.com wrote:
On (Wed) 19 Nov 2014 [11:08:46], Igor Mammedov wrote:
On Wed, 19 Nov 2014 12:51:00 +0530
Amit Shah amit.s...@redhat.com wrote:
-static void *acpi_add_rom_blob(AcpiBuildState *build_state, GArray
*blob, +static ram_addr_t acpi_add_rom_blob(AcpiBuildState
*build_state, GArray *blob, const char *name)
{
return rom_add_blob(name, blob-data, acpi_data_len(blob), -1,
name, @@ -1777,6 +1781,7 @@ void acpi_setup(PcGuestInfo *guest_info)
/* Now expose it all to Guest */
build_state-table_ram = acpi_add_rom_blob(build_state,
tables.table_data, ACPI_BUILD_TABLE_FILE);
+assert(build_state-table_ram != RAM_ADDR_MAX);
build_state-table_size = acpi_data_len(tables.table_data);
Isn't an assert too strong if this happens during hotplug?
I'm trying to follow this code, but looks like this isn't called in
the hotplug path - is that right?
yep, it's called only at startup
Thanks; what's the path taken for hotplug, then? (Ensuring we have
that case covered too).
In case of hotplug ACPI blob doesn't change at all (it has all
possible device objects in it). It changes on the first time BIOS
fetches ROM blob and on reset.
Later during hotplug, guest gets SCI interrupt and runs related to
hotplug event AML method, which fishes out bits necessary for hotplug
via corresponding mmio interface.
Amit
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 2014/11/20 16:11, Jason Wang wrote:
On 11/20/2014 04:05 PM, Gonglei wrote:
On 2014/11/20 15:50, Jason Wang wrote:
Maybe just initialize iov unconditionally at the beginning and check
dot1q_buf instead of iov for the rest of the functions. (Need deal with
size ETHER_ADDR_LEN * 2)
More complicated, because we can't initialize iov when
size ETHER_ADDR_LEN * 2.
Best regards,
-Gonglei
Probably not: you can just do something like:
if (dot1q_buf size ETHER_ADDR_LEN * 2) {
dot1q_buf = NULL;
}
and check dot1q_buf afterwards. Or just drop the packet since its size
was less than mininum frame length that Ethernet allows.
Sorry, I don't understand. But,
what's your meaning initialize iov unconditionally at the beginning?
Something like:
@@ -1774,7 +1774,12 @@ static uint32_t
rtl8139_RxConfig_read(RTL8139State *s)
static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
int do_interrupt, const uint8_t *dot1q_buf)
{
-struct iovec *iov = NULL;
+struct iovec iov[3] = {
+{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 },
+{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN },
+{ .iov_base = buf + ETHER_ADDR_LEN * 2,
+ .iov_len = size - ETHER_ADDR_LEN * 2 },
+};
and assign dot1q_buf to NULL is size is not ok.
If size ETHER_ADDR_LEN * 2, .iov_len = size - ETHER_ADDR_LEN * 2 will be
negative value;
and if dot1q_buf is NULL, .iov_base = (void *) dot1q_buf will be NULL too. Any
side-effect?
Just a suggestion, your call.
Thanks, Jason :)
Best regards,
-Gonglei
Re: [Qemu-devel] [RFC][PATCH v2] block: add write threshold reporting for block devices
- Original Message -
From: Stefan Hajnoczi stefa...@redhat.com
To: Francesco Romani from...@redhat.com
Cc: kw...@redhat.com, Stefan Hajnoczi stefa...@gmail.com,
mdr...@linux.vnet.ibm.com, qemu-devel@nongnu.org,
lcapitul...@redhat.com
Sent: Wednesday, November 19, 2014 4:52:51 PM
Subject: Re: [Qemu-devel] [RFC][PATCH v2] block: add write threshold
reporting for block devices
On Tue, Nov 18, 2014 at 03:12:12AM -0500, Francesco Romani wrote:
+static int coroutine_fn before_write_notify(NotifierWithReturn
*notifier,
+void *opaque)
+{
+BdrvTrackedRequest *req = opaque;
+BlockDriverState *bs = req-bs;
+int64_t amount = 0;
+
+assert((req-offset (BDRV_SECTOR_SIZE - 1)) == 0);
+assert((req-bytes (BDRV_SECTOR_SIZE - 1)) == 0);
Does the code still make these assumptions or are the asserts left over
from previous versions of the patch?
It's a leftover.
I understood they don't hurt and add a bit of safety, but if they are
confusing
I'll remove them.
Yes, it made me wonder why. Probably best to remove them.
Will do
[...]
At risk of being redundant, in oVirt the usecase is QCOW2 over lvm block
device,
and we'd like to be notified about the allocation of the lvm block device,
which (IIUC)
is the last bs-file.
This is a simple topology (unless I'm missing something), and that's
the reason why I go down just one level.
Of course I want a general solution for this change, so...
There is a block driver for error injection called blkdebug (see
docs/blkdebug.txt). Here is an example of the following topology:
raw_bsd (drive0) - blkdebug - raw-posix (test.img)
qemu-system-x86_64 -drive
if=virtio,format=raw,file.driver=blkdebug,file.image.filename=test.img
The blkdebug driver is interposing between the raw_bsd (drive0) root and
the raw-posix leaf node.
Thanks, I'll have a look
[...]
The management tool should not need to inspect the graph because the
graph can change at runtime (e.g. imagine I/O throttling is implemented
as a BlockDriverState node then it could appear/disapper when the
feature is activated/deactivated). Instead the management tool should
name the nodes it knows about and then use those node names.
Agreed - and indeed simpler for us (oVirt), which it doesn't hurt :)
If we descend the bs-file chain, AFAIU the easiest mapping, being the
device name,
is easily lost because only the outermost BlockDriverState has a device
name attached, so when the
notification trigger
bdrv_get_device_name() will return NULL
In the worst case a name string can be passed in along with the
threshold values.
OK, I guess to keep a copy of the string with g_strdup() could be good enough
start,
at least for further discussion.
Thanks for your review and for the informations,
I'll submit a new revision of the patch in a couple of days,
to give to other reviewers some time to jump in.
Bests,
--
Francesco Romani
RedHat Engineering Virtualization R D
Phone: 8261328
IRC: fromani
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 11/20/2014 04:18 PM, Gonglei wrote:
On 2014/11/20 16:11, Jason Wang wrote:
On 11/20/2014 04:05 PM, Gonglei wrote:
On 2014/11/20 15:50, Jason Wang wrote:
Maybe just initialize iov unconditionally at the beginning and check
dot1q_buf instead of iov for the rest of the functions. (Need deal with
size ETHER_ADDR_LEN * 2)
More complicated, because we can't initialize iov when
size ETHER_ADDR_LEN * 2.
Best regards,
-Gonglei
Probably not: you can just do something like:
if (dot1q_buf size ETHER_ADDR_LEN * 2) {
dot1q_buf = NULL;
}
and check dot1q_buf afterwards. Or just drop the packet since its size
was less than mininum frame length that Ethernet allows.
Sorry, I don't understand. But,
what's your meaning initialize iov unconditionally at the beginning?
Something like:
@@ -1774,7 +1774,12 @@ static uint32_t
rtl8139_RxConfig_read(RTL8139State *s)
static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
int do_interrupt, const uint8_t *dot1q_buf)
{
-struct iovec *iov = NULL;
+struct iovec iov[3] = {
+{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 },
+{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN },
+{ .iov_base = buf + ETHER_ADDR_LEN * 2,
+ .iov_len = size - ETHER_ADDR_LEN * 2 },
+};
and assign dot1q_buf to NULL is size is not ok.
If size ETHER_ADDR_LEN * 2, .iov_len = size - ETHER_ADDR_LEN * 2 will be
negative value;
and if dot1q_buf is NULL, .iov_base = (void *) dot1q_buf will be NULL too.
Any side-effect?
Then you need check dot1q_buf instead of iov after. Iov won't be used if
dot1q_buf is NULL.
Just a suggestion, your call.
Thanks, Jason :)
Best regards,
-Gonglei
Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 v3 1/1] -machine vmport=auto: Fix handling of VMWare ioport emulation for xen
On Wed, Nov 19, 2014 at 07:38:10PM -0500, Don Slutz wrote:
c/s 9b23cfb76b3a5e9eb5cc899eaf2f46bc46d33ba4
or
c/s b154537ad07598377ebf98252fb7d2aff127983b
moved the testing of xen_enabled() from pc_init1() to
pc_machine_initfn().
xen_enabled() does not return the correct value in
pc_machine_initfn().
Changed vmport from a bool to an enum. Added the value auto to do
the old way.
Signed-off-by: Don Slutz dsl...@verizon.com
This looks fine to me. A couple of minor comments below.
Also this changes qapi schema file, let's get ack from maintainers -
my understanding is that just adding a definition there won't
affect any users, correct?
---
hw/i386/pc.c | 23 ++-
hw/i386/pc_piix.c| 27 ++-
hw/i386/pc_q35.c | 27 ++-
include/hw/i386/pc.h | 2 +-
qapi-schema.json | 16
qemu-options.hx | 8 +---
vl.c | 2 +-
7 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1205db8..2f68e4d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1711,18 +1711,23 @@ static void pc_machine_set_max_ram_below_4g(Object
*obj, Visitor *v,
pcms-max_ram_below_4g = value;
}
-static bool pc_machine_get_vmport(Object *obj, Error **errp)
+static void pc_machine_get_vmport(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
PCMachineState *pcms = PC_MACHINE(obj);
+int vmport = pcms-vmport;
-return pcms-vmport;
+visit_type_enum(v, vmport, vmport_lookup, NULL, name, errp);
}
-static void pc_machine_set_vmport(Object *obj, bool value, Error **errp)
+static void pc_machine_set_vmport(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
{
PCMachineState *pcms = PC_MACHINE(obj);
+int vmport;
-pcms-vmport = value;
+visit_type_enum(v, vmport, vmport_lookup, NULL, name, errp);
+pcms-vmport = vmport;
}
static void pc_machine_initfn(Object *obj)
@@ -1737,11 +1742,11 @@ static void pc_machine_initfn(Object *obj)
pc_machine_get_max_ram_below_4g,
pc_machine_set_max_ram_below_4g,
NULL, NULL, NULL);
-pcms-vmport = !xen_enabled();
-object_property_add_bool(obj, PC_MACHINE_VMPORT,
- pc_machine_get_vmport,
- pc_machine_set_vmport,
- NULL);
+pcms-vmport = VMPORT_AUTO;
+object_property_add(obj, PC_MACHINE_VMPORT, str,
+pc_machine_get_vmport,
+pc_machine_set_vmport,
+NULL, NULL, NULL);
}
static void pc_machine_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7bb97a4..81a7b83 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -101,6 +101,7 @@ static void pc_init1(MachineState *machine,
FWCfgState *fw_cfg = NULL;
PcGuestInfo *guest_info;
ram_addr_t lowmem;
+bool no_vmport;
/* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory).
* If it doesn't, we need to split it in chunks below and above 4G.
@@ -234,9 +235,33 @@ static void pc_init1(MachineState *machine,
pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
Probably should be assert(pc_machine-vmport != VMPORT_MAX) -
we never get any values except on/off/auto.
+if (xen_enabled()) {
+switch (pc_machine-vmport) {
+case VMPORT_MAX:
+case VMPORT_AUTO:
+case VMPORT_OFF:
+no_vmport = true;
+break;
+case VMPORT_ON:
+no_vmport = false;
+break;
+}
+} else {
+switch (pc_machine-vmport) {
+case VMPORT_MAX:
+case VMPORT_OFF:
+no_vmport = true;
+break;
+case VMPORT_AUTO:
+case VMPORT_ON:
+no_vmport = false;
+break;
+}
+}
+
Can't above be moved to a function in pc.c, and be reused between piix
and q35? It's big enough to avoid open-coding, IMHO.
/* init basic PC hardware */
pc_basic_device_init(isa_bus, gsi, rtc_state, floppy,
- !pc_machine-vmport, 0x4);
+ no_vmport, 0x4);
pc_nic_init(isa_bus, pci_bus);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 598e679..4f932d6 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -88,6 +88,7 @@ static void pc_q35_init(MachineState *machine)
PcGuestInfo *guest_info;
ram_addr_t lowmem;
DriveInfo *hd[MAX_SATA_PORTS];
+bool no_vmport;
/* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
* and 256 Mbytes for PCI Express Enhanced
Re: [Qemu-devel] [PATCH v1 RFC 4/9] qemu-iotests: fix test 039
On 2014-11-20 at 09:08, Mao Chuan Li wrote: The intention is to disable the core dump, if there is another way we can achieve that, switching to root is not necessary. Any other alternative way? Thanks! Mao Chuan Li Hi, I cannot think of a way; on the other hand, I don't think disabling the core dump is necessary either. Simply filtering out '(core dumped) ' is not pretty but still suffices for me (we can put that into a filter function in common.filter and if other people see other messages for a core dump, they can expand that list). If we really want to disable core dumps, we should change qemu-io not to use abort() on -c abort (raise(SIGKILL) seems like a good alternative to me); or, alternatively, introduce a new command 'kill' which then does raise(SIGKILL) so we don't have to break compatibility. Max
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 2014/11/20 16:24, Jason Wang wrote:
On 11/20/2014 04:18 PM, Gonglei wrote:
On 2014/11/20 16:11, Jason Wang wrote:
On 11/20/2014 04:05 PM, Gonglei wrote:
On 2014/11/20 15:50, Jason Wang wrote:
Maybe just initialize iov unconditionally at the beginning and check
dot1q_buf instead of iov for the rest of the functions. (Need deal
with
size ETHER_ADDR_LEN * 2)
More complicated, because we can't initialize iov when
size ETHER_ADDR_LEN * 2.
Best regards,
-Gonglei
Probably not: you can just do something like:
if (dot1q_buf size ETHER_ADDR_LEN * 2) {
dot1q_buf = NULL;
}
and check dot1q_buf afterwards. Or just drop the packet since its size
was less than mininum frame length that Ethernet allows.
Sorry, I don't understand. But,
what's your meaning initialize iov unconditionally at the beginning?
Something like:
@@ -1774,7 +1774,12 @@ static uint32_t
rtl8139_RxConfig_read(RTL8139State *s)
static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
int do_interrupt, const uint8_t *dot1q_buf)
{
-struct iovec *iov = NULL;
+struct iovec iov[3] = {
+{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 },
+{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN },
+{ .iov_base = buf + ETHER_ADDR_LEN * 2,
+ .iov_len = size - ETHER_ADDR_LEN * 2 },
+};
and assign dot1q_buf to NULL is size is not ok.
If size ETHER_ADDR_LEN * 2, .iov_len = size - ETHER_ADDR_LEN * 2 will be
negative value;
and if dot1q_buf is NULL, .iov_base = (void *) dot1q_buf will be NULL too.
Any side-effect?
Then you need check dot1q_buf instead of iov after. Iov won't be used if
dot1q_buf is NULL.
But that's hacking IMHO. Let's don't do this. ;)
Just a suggestion, your call.
Thanks, Jason :)
Best regards,
-Gonglei
[Qemu-devel] [PATCH v2] mips: Correctly save/restore the FP flush-to-zero state
Fix the FP state save/restore operations by saving the `flush_to_zero'
rather than the `float_detect_tininess' setting. There is no provision
for the latter in MIPS hardware, whereas the former is controlled by the
CP1.FCSR.FS bit. As a result all the older saved state images are
invalid as they do not restore the FP state corresponding to the
CP1.FCSR.FS bit and may execute differently when resumed compared to the
case where no save/restore operations have ever been made. Therefore
reject any such older images too and do not allow them to be loaded.
According to the architecture manual[1][2]:
Flush to Zero (Flush Subnormals).
[...]
Encoding Meaning
0Input subnormal values and tiny non-
zero results are not altered. Unimple-
mented Operation Exception may be
signaled as needed.
1When FS is one, subnormal results are
flushed to zero. The Unimplemented
Operation Exception is NOT signalled
for this reason.
Every tiny non-zero result is
replaced with zero of the same sign.
Prior to Release 5 it is implementa-
tion dependent whether subnormal
operand values are flushed to zero
before the operation is carried out.
As of Release 5 every input subnor-
mal value is replaced with zero of the
same sign.
References:
[1] MIPS Architecture For Programmers, Volume I-A: Introduction to the
MIPS32 Architecture, MIPS Technologies, Inc., Document Number:
MD00082, Revision 5.03, Sept. 9, 2013, Table 5.7 FCSR Register
Field Descriptions, p. 81.
[2] MIPS Architecture For Programmers, Volume I-A: Introduction to the
MIPS64 Architecture, Imagination Technologies, Inc., Document
Number: MD00083, Revision 6.00, March 31, 2014, Table 6.7 FCSR
Register Field Descriptions, p. 93.
Signed-off-by: Maciej W. Rozycki ma...@codesourcery.com
---
Hi,
Updates from v1:
- use qemu_put_8s/qemu_get_8s rather than qemu_put_s8s/qemu_get_s8s.
For some reason `float_detect_tininess' is signed while `flush_to_zero'
is unsigned -- what's the point?
Background information: I didn't realize the QEMU build process does
not enable -Werror by default. I have now noticed it and explicitly
enabled the flag having spent a few hours debugging an issue that should
have been caught by the compilation process (an incompatible pointer
type used in a function call) -- shouldn't QEMU default to -Werror for
development sources and only disable it for releases?
Fortunately this is the only change affected from these I have
submitted so far.
I haven't updated the subject to have `target-mips:' rather than
`mips:' as the tag so that automatic patch processing tools pick up this
change as a replacement for the previous version. I can resend with
such an adjustment if that was desired. Otherwise please apply.
Maciej
qemu-mips-fp_status.diff
Index: qemu-git-trunk/target-mips/cpu.h
===
--- qemu-git-trunk.orig/target-mips/cpu.h 2014-11-20 08:07:13.0
+
+++ qemu-git-trunk/target-mips/cpu.h2014-11-20 08:10:28.568927441 +
@@ -615,7 +615,7 @@ void mips_cpu_list (FILE *f, fprintf_fun
extern void cpu_wrdsp(uint32_t rs, uint32_t mask_num, CPUMIPSState *env);
extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
-#define CPU_SAVE_VERSION 5
+#define CPU_SAVE_VERSION 6
/* MMU modes definitions. We carefully match the indices with our
hflags layout. */
Index: qemu-git-trunk/target-mips/machine.c
===
--- qemu-git-trunk.orig/target-mips/machine.c 2014-11-20 08:07:13.0
+
+++ qemu-git-trunk/target-mips/machine.c2014-11-20 08:29:03.688928596
+
@@ -34,7 +34,7 @@ static void save_fpu(QEMUFile *f, CPUMIP
for(i = 0; i 32; i++)
qemu_put_be64s(f, fpu-fpr[i].d);
-qemu_put_s8s(f, fpu-fp_status.float_detect_tininess);
+qemu_put_8s(f, fpu-fp_status.flush_to_zero);
qemu_put_s8s(f, fpu-fp_status.float_rounding_mode);
qemu_put_s8s(f, fpu-fp_status.float_exception_flags);
qemu_put_be32s(f, fpu-fcr0);
@@ -162,7 +162,7 @@ void cpu_save(QEMUFile *f, void *opaque)
save_fpu(f, env-fpus[i]);
}
-static void load_tc(QEMUFile *f, TCState *tc, int version_id)
+static void load_tc(QEMUFile *f, TCState *tc)
{
int i;
@@ -184,9 +184,7 @@ static void load_tc(QEMUFile *f, TCState
qemu_get_betls(f, tc-CP0_TCSchedule);
qemu_get_betls(f, tc-CP0_TCScheFBack);
qemu_get_sbe32s(f, tc-CP0_Debug_tcstatus);
-if (version_id = 4) {
-qemu_get_betls(f, tc-CP0_UserLocal);
-}
+qemu_get_betls(f, tc-CP0_UserLocal);
}
static void
[Qemu-devel] [PATCH] pcie: fix improper use of negative value
From: Gonglei arei.gong...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- hw/pci/pcie.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 58455bd..2902f7d 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -229,7 +229,7 @@ static void pcie_cap_slot_hotplug_common(PCIDevice *hotplug_dev, /* the slot is electromechanically locked. * This error is propagated up to qdev and then to HMP/QMP. */ -error_setg_errno(errp, -EBUSY, slot is electromechanically locked); +error_setg_errno(errp, EBUSY, slot is electromechanically locked); } } -- 1.7.12.4
Re: [Qemu-devel] [PATCH for-2.3 2/4] blockdev: check for BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE
On 2014-11-19 at 15:19, Stefan Hajnoczi wrote: The BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE op blocker exists but was never used! Let's fix that so snapshot delete can be blocked. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- blockdev.c | 4 1 file changed, 4 insertions(+) Reviewed-by: Max Reitz mre...@redhat.com
[Qemu-devel] [PATCH] vnc-enc-tight: fix Arguments in wrong order
From: Gonglei arei.gong...@huawei.com
Arguments in wrong order (SWAPPED_ARGUMENTS)
The positions of arguments in the call to
tight_fill_palette do not match the ordering of the parameters:
fg is passed to bg
bg is passed to fg
Cc: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Gonglei arei.gong...@huawei.com
---
ui/vnc-enc-tight.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 3d1b5cd..9a9ddf2 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -1489,7 +1489,7 @@ static int send_sub_rect(VncState *vs, int x, int y, int
w, int h)
}
#endif
-colors = tight_fill_palette(vs, x, y, w * h, fg, bg, palette);
+colors = tight_fill_palette(vs, x, y, w * h, bg, fg, palette);
#ifdef CONFIG_VNC_JPEG
if (allow_jpeg vs-tight.quality != (uint8_t)-1) {
--
1.7.12.4
Re: [Qemu-devel] [PATCH v2 1/3] pc-dimm: add a function to calculate VM's current RAM size
On Wed, Nov 19, 2014 at 09:31:35AM -0700, Eric Blake wrote: On 11/19/2014 09:06 AM, Michael S. Tsirkin wrote: This affects QMP right? I think later patches will tell how. CC'ing Eric. As far as I can tell, this is just correcting a reporting issue; the existing QMP commands/events for tracking balloon size will now properly account for hotplugged memory. What I don't know is if this change in semantics will affect any users. Libvirt is not yet supporting memory hotplug, so ideally, fixing this bug before libvirt uses memory hotplug means libvirt will never have to worry about qemu versions that do incorrect reporting. The alternative is to declare that the existing QMP commands cannot change in semantics for the existing members that it reports, and must instead report additional dictionary members describing the amount of hot-plugged memory, and then require that the client add the numbers together itself. That sounds mean to the client, so I'm hoping we don't have to go there. IOW you ack this patch for 2.2? Is memory hotplug one of the new features in 2.2? If so, then yes, we should get its semantics right from the start (this is a bug fix to avoid a release with broken semantics). On the other hand, if hotplug existed in 2.1, then we already have a release with odd semantics, so delaying this fix until 2.3 and leaving 2.2 with the same odd semantics would not hurt, and it then becomes a judgment call of whether we are rushing in a possibly incomplete solution by trying to get this into 2.2. (Sorry I haven't been following the history of memory hotplug closer) AFAIK it's there since 2.0. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 v3 1/1] -machine vmport=auto: Fix handling of VMWare ioport emulation for xen
On Wed, Nov 19, 2014 at 09:11:41PM -0700, Eric Blake wrote:
On 11/19/2014 05:38 PM, Don Slutz wrote:
c/s 9b23cfb76b3a5e9eb5cc899eaf2f46bc46d33ba4
or
c/s b154537ad07598377ebf98252fb7d2aff127983b
moved the testing of xen_enabled() from pc_init1() to
pc_machine_initfn().
xen_enabled() does not return the correct value in
pc_machine_initfn().
Changed vmport from a bool to an enum. Added the value auto to do
the old way.
+++ b/qapi-schema.json
@@ -3513,3 +3513,19 @@
# Since: 2.1
##
{ 'command': 'rtc-reset-reinjection' }
+
+##
+# @vmport
+#
+# An enumeration of the options on enabling of VMWare ioport emulation
+#
+# @auto: system selects the old default
+#
+# @on: provide the vmport device
+#
+# @off: do not provide the vmport device
+#
+# Since: 2.2
+##
+{ 'enum': 'vmport',
All other enums in .json files are named in StudlyCaps. Please name
this starting with a capital letter, and reserve all-lower-case names
for commands and built-in types.
Hi Eric,
The values here are not vmport-specific.
Do you think we should have a generic OnOffAuto type?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH for-2.3 1/4] blockdev: acquire AioContext in blockdev-snapshot-delete-internal-sync
On 2014-11-19 at 15:19, Stefan Hajnoczi wrote: Add dataplane support to the blockdev-snapshot-delete-internal-sync QMP command. By acquiring the AioContext we avoid race conditions with the dataplane thread which may also be accessing the BlockDriverState. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- blockdev.c | 16 +--- hw/block/dataplane/virtio-blk.c | 2 ++ 2 files changed, 15 insertions(+), 3 deletions(-) Reviewed-by: Max Reitz mre...@redhat.com
Re: [Qemu-devel] [PATCH for-2.3 3/4] blockdev: acquire AioContext in eject, change, and block_passwd
On 2014-11-19 at 15:19, Stefan Hajnoczi wrote: By acquiring the AioContext we avoid race conditions with the dataplane thread which may also be accessing the BlockDriverState. Fix up eject, change, and block_passwd in a single patch because qmp_eject() and qmp_change_blockdev() both call eject_device(). Also fix block_passwd while we're tackling a command that takes a block encryption password. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- blockdev.c | 36 +--- hw/block/dataplane/virtio-blk.c | 1 + 2 files changed, 30 insertions(+), 7 deletions(-) You could've used blk_get_aio_context() for acquiring the AioContext in qmp_eject() instead of in eject_device() (and qmp_change_blockdev(), which is the other caller of eject_device(), holds the context anyway). Anyway: Reviewed-by: Max Reitz mre...@redhat.com
Re: [Qemu-devel] [PATCH 4/4] rtl8139: fix Pointer to local outside scope
On 20/11/2014 09:24, Jason Wang wrote:
On 11/20/2014 04:18 PM, Gonglei wrote:
On 2014/11/20 16:11, Jason Wang wrote:
On 11/20/2014 04:05 PM, Gonglei wrote:
On 2014/11/20 15:50, Jason Wang wrote:
Maybe just initialize iov unconditionally at the beginning and check
dot1q_buf instead of iov for the rest of the functions. (Need deal
with
size ETHER_ADDR_LEN * 2)
More complicated, because we can't initialize iov when
size ETHER_ADDR_LEN * 2.
Best regards,
-Gonglei
Probably not: you can just do something like:
if (dot1q_buf size ETHER_ADDR_LEN * 2) {
dot1q_buf = NULL;
}
and check dot1q_buf afterwards. Or just drop the packet since its size
was less than mininum frame length that Ethernet allows.
Sorry, I don't understand. But,
what's your meaning initialize iov unconditionally at the beginning?
Something like:
@@ -1774,7 +1774,12 @@ static uint32_t
rtl8139_RxConfig_read(RTL8139State *s)
static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
int do_interrupt, const uint8_t *dot1q_buf)
{
-struct iovec *iov = NULL;
+struct iovec iov[3] = {
+{ .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 },
+{ .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN },
+{ .iov_base = buf + ETHER_ADDR_LEN * 2,
+ .iov_len = size - ETHER_ADDR_LEN * 2 },
+};
and assign dot1q_buf to NULL is size is not ok.
If size ETHER_ADDR_LEN * 2, .iov_len = size - ETHER_ADDR_LEN * 2 will be
negative value;
and if dot1q_buf is NULL, .iov_base = (void *) dot1q_buf will be NULL too.
Any side-effect?
Then you need check dot1q_buf instead of iov after. Iov won't be used if
dot1q_buf is NULL.
Or just ignore the rules and use an initializer in the middle of the code:
if (size ETHER_ADDR_LEN * 2) {
dot1q_buf = NULL;
}
struct iovec iov[3] = { ... };
and then check dot1q_buf instead of iov. Plenty of choices, choose your
favorite.
Paolo
Re: [Qemu-devel] [PATCH for-2.3 4/4] blockdev: acquire AioContext in change-backing-file
On 2014-11-19 at 15:19, Stefan Hajnoczi wrote: Add dataplane support to the change-backing-file QMP commands. By acquiring the AioContext we avoid race conditions with the dataplane thread which may also be accessing the BlockDriverState. Note that this command operates on both bs and a node in its chain (image_bs). The bdrv_chain_contains(bs, image_bs) check guarantees that bs and image_bs are in the same AioContext. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- blockdev.c | 19 +-- hw/block/dataplane/virtio-blk.c | 1 + 2 files changed, 14 insertions(+), 6 deletions(-) Reviewed-by: Max Reitz mre...@redhat.com
Re: [Qemu-devel] [PATCH for-2.3 0/4] blockdev: support dataplane in remaining QMP commands
On 2014-11-19 at 15:19, Stefan Hajnoczi wrote: This patch series adds virtio-blk dataplane support for the following QMP commands: * eject * change * change-backing-file * block_passwd * blockdev-snapshot-delete-internal-sync This requires acquiring and releasing the BlockDriverState's AioContext so that the IOThread does not run while the monitor command is executing. Monitor commands that use AioContext can be unblocked in the virtio-blk dataplane op blockers list. After this series only the transaction QMP command remains to be converted. Stefan Hajnoczi (4): blockdev: acquire AioContext in blockdev-snapshot-delete-internal-sync blockdev: check for BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE blockdev: acquire AioContext in eject, change, and block_passwd blockdev: acquire AioContext in change-backing-file blockdev.c | 75 - hw/block/dataplane/virtio-blk.c | 4 +++ 2 files changed, 63 insertions(+), 16 deletions(-) Thanks, applied to my block-next tree: https://github.com/XanClic/qemu/commits/block-next
Re: [Qemu-devel] Fwd: Re: Tunneled Migration with Non-Shared Storage
* Gary R Hook (grhookatw...@gmail.com) wrote: Ugh, I wish I could teach Thunderbird to understand how to reply to a newsgroup. Apologies to Paolo for the direct note. On 11/19/14 4:19 AM, Paolo Bonzini wrote: On 19/11/2014 10:35, Dr. David Alan Gilbert wrote: * Paolo Bonzini (pbonz...@redhat.com) wrote: On 18/11/2014 21:28, Dr. David Alan Gilbert wrote: This seems odd, since as far as I know the tunneling code is quite separate to the migration code; I thought the only thing that the migration code sees different is the file descriptors it gets past. (Having said that, again I don't know storage stuff, so if this is a storage special there may be something there...) Tunnelled migration uses the old block-migration.c code. Non-tunnelled migration uses the NBD server and block/mirror.c. OK, that explains that. Is that because the tunneling code can't deal with tunneling the NBD server connection? The main problem with the old code is that uses a possibly unbounded amount of memory in mig_save_device_dirty and can have huge jitter if any serious workload is running in the guest. So that's sending dirty blocks iteratively? Not that I can see when the allocations get freed; but is the amount allocated there related to total disk size (as Gary suggested) or to the amount of dirty blocks? It should be related to the maximum rate limit (which can be set to arbitrarily high values, however). This makes no sense. The code in block_save_iterate() specifically attempts to control the rate of transfer. But when qemu_file_get_rate_limit() returns a number like 922337203685372723 (0xCCB) I'm under the impression that no bandwidth constraints are being imposed at this layer. Why, then, would that transfer be occurring at 20MB/sec (simple, under-utilized 1 gigE connection) with no clear bottleneck in CPU or network? What other relation might exist? Disk IO on the disk that you're trying to transfer? The reads are started, then the ones that are ready are sent and the blocks are freed in flush_blks. The jitter happens when the guest reads a lot but only writes a few blocks. In that case, the bdrv_drain_all in mig_save_device_dirty can be called relatively often and it can be expensive because it also waits for all guest-initiated reads to complete. Pardon my ignorance, but this does not match my observations. What I am seeing is the process size of the source qemu grow steadily until the COR completes; during this time the backing file on the destination system does not change/grow at all, which implies that no blocks are being transferred. (I have tested this with a 25GB VM disk, and larger; no network activity occurs during this period.) Once the COR is done and the in-memory copy ready (marked by a Completed 100% message from blk_mig_save_builked_block()) the transfer begins. At an abysmally slow rate, I'll add, per the above. Another problem to be investigated. Odd thought; can you try dropping your migration bandwidth limit (migrate_set_speed) - try something low, like 10M - does the behaviour stay the same, or does it start transmitting disk data before it's read the lot? The bulk phase is similar, just with different functions (the reads are done in mig_save_device_bulk). With a high rate limit, the total allocated memory can reach a few gigabytes indeed. Much, much more than that. It's definitely dependent upon the disk file size. Tiny VM disks are a nit; big VM disks are a problem. Well, if as you say it's not starting transmitting for some reason until it's read the lot then that would make sense. Depending on the scenario, a possible disadvantage of NBD migration is that it can only throttle each disk separately, while the old code will apply a single limit to all migrations. How about no throttling at all? And just to be very clear, the goal is fast (NBD-based) migrations of VMs using non-shared storage over an encrypted channel. Safest, worst-case scenario. Aside from gaining an understanding of this code. There are vague plans to add TLS support for encrypting these streams internally to qemu; but they're just thoughts at the moment. Thank you for your attention. Dave -- Gary R Hook Senior Kernel Engineer NIMBOXX, Inc -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 2/5] exec: qemu_ram_alloc_device, qemu_ram_resize
On 20/11/2014 09:12, Gerd Hoffmann wrote: Hi, I don't know why RHEL7 SeaBIOS does not work on RHEL6. But note that it's a really old version (0.12). Hmm, works for me on a quick smoke test. Do you remember what exactly broke and which version it was? Maybe the 1.7.2 - 1.7.5 update fixed it? Possibly. I tested 1.7.2 about a month ago and it failed. Or was it live-migration by chance? rhel6 qemu doesn't emulate pam registers correctly, and seabios has a workaround for that. So live-migrating between versions with and without correct pam emulation creates some *ahem* very interesting corner cases ... No, it was not live migration. Laszlo found it when he was trying to identify a live migration bug as a QEMU bug or SeaBIOS bug; but he didn't even get to the point of doing live migration. :) Paolo
Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 v3 1/1] -machine vmport=auto: Fix handling of VMWare ioport emulation for xen
* Paolo Bonzini (pbonz...@redhat.com) wrote:
On 20/11/2014 01:58, Eduardo Habkost wrote:
if (pc_machine-vmport == VMPORT_AUTO) {
no_vmport = xen_enabled();
} else {
no_vmport = (pc_machine-vmport == VMPORT_ON);
}
I'm still not sure why the configuration should differ for -M pc
depending on whether xen is enabled.
I think this goes back to:
commit 1611977c3d8fdbdac6090cbd1fcee4aed6d9
Author: Anthony PERARD anthony.per...@citrix.com
Date: Tue May 3 17:06:54 2011 +0100
pc, Disable vmport initialisation with Xen.
This is because there is not synchronisation of the vcpu register
between Xen and QEMU, so vmport can't work properly.
This patch introduces no_vmport parameter to pc_basic_device_init.
Signed-off-by: Anthony PERARD anthony.per...@citrix.com
Signed-off-by: Alexander Graf ag...@suse.de
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 3/4] pcnet: fix Negative array index read
On 20/11/2014 08:38, Gonglei wrote:
On 2014/11/20 15:08, Paolo Bonzini wrote:
On 20/11/2014 07:44, Gonglei wrote:
Maybe not, since two branch are if and else if not if and else,
so this change make the below code segment's wide ...
bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT);
s-phys_mem_read(s-dma_opaque, PHYSADDR(s, tmd.tbadr),
s-buffer + s-xmit_pos, bcnt, CSR_BSWP(s));
s-xmit_pos += bcnt;
... more extensive.
After your patch that fixes the coverity report, they are
if (a b)
else if (b)
so you can change it to
if (!b) goto txdone;
if (a) ...
else ...
and then
if (!b) goto txdone;
common part
if (!a) {
extra part from else
}
Paolo
I know your mean now, thanks ;)
What about this below way? Maybe more clear.
As you prefer.
Paolo
if (s-xmit_pos 0) {
goto txdone;
}
int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT);
s-phys_mem_read(s-dma_opaque, PHYSADDR(s, tmd.tbadr),
s-buffer + s-xmit_pos, bcnt, CSR_BSWP(s));
s-xmit_pos += bcnt;
if (!GET_FIELD(tmd.status, TMDS, ENP)) {
goto txdone;
}
#ifdef PCNET_DEBUG
printf(pcnet_transmit size=%d\n, s-xmit_pos);
#endif
if (CSR_LOOP(s)) {
if (BCR_SWSTYLE(s) == 1)
add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS);
s-looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC;
pcnet_receive(qemu_get_queue(s-nic), s-buffer, s-xmit_pos);
s-looptest = 0;
} else
if (s-nic)
qemu_send_packet(qemu_get_queue(s-nic), s-buffer,
s-xmit_pos);
s-csr[0] = ~0x0008; /* clear TDMD */
s-csr[4] |= 0x0004;/* set TXSTRT */
s-xmit_pos = -1;
txdone:
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH v2 0/3] Migration-safe ACPI table sizing algorithm
On 20/11/2014 08:55, Michael S. Tsirkin wrote: On Thu, Nov 20, 2014 at 08:11:05AM +0100, Paolo Bonzini wrote: On 20/11/2014 07:55, Michael S. Tsirkin wrote: I thought we agreed we'll consider alternate approaches after 2.2? I would prefer not to have yet another mode to support if we can help it. I agree, but: 1) looks like there is stronger opposition to your patch than I thought, so a 2.2 solution as in this patch becomes more palatable Why the urgency? It's not fixing any regressions, is it? I would rather not add yet another mode for 2.2, we'll likely have a new mode in 2.3 but I'd like that to be the last one. I don't think there's a need to add both patches. If mine goes in, and it can go in 2.2 since it is just another mode, there is no need for resizable MemoryRegions. Paolo 2) reviewing patches is always nice, and helps evaluating the advantages of either approach Paolo I'll do my best, sorry about the delay - I'm trying to prioritize 2.2 work at the moment.
Re: [Qemu-devel] [RFC][PATCH v2] block: add write threshold reporting for block devices
Am 17.11.2014 um 17:49 hat Stefan Hajnoczi geschrieben:
On Fri, Nov 07, 2014 at 02:12:13PM +0100, Francesco Romani wrote:
+void bdrv_set_usage_threshold(BlockDriverState *bs, int64_t
threshold_bytes)
+{
+BlockDriverState *target_bs = bs;
+if (bs-file) {
+target_bs = bs-file;
+}
Hmm...I think now I understand why you are trying to use bs-file. This
is an attempt to make image formats work with the threshold.
Unfortunately the BlockDriverState topology can be more complicated than
just 1 level.
If we hardcode a strategy to traverse bs-file then it will work in most
cases:
while (bs-file) {
bs = bs-file;
}
But there are cases like VMDK extent files where a BlockDriverState
actually has multiple children.
One way to solve this is to require that the management tool tells QEMU
which exact BlockDriverState node the threshold applies to. Then QEMU
doesn't need any hardcoded policy. But I'm not sure how realistic that
it at the moment (whether management tools are uses node names for each
node yet), so it may be best to hardcode the bs-file traversal that
I've suggested.
Kevin: Do you agree?
I have a feeling that we would regret this in the long run because it
would allow only one special case of a general problem (watching a BDS).
This means that we'll get inconsistent APIs.
We're only talking about an optimisation here, even though a very
useful one, so I wouldn't easily make compromises here. We should
probably insist on using the node-name. Management tools need new code
anyway to make use of the new functionality, so they can implement
node-name support as well while they're at it.
Kevin
pgptAWMrPmrn0.pgp
Description: PGP signature
[Qemu-devel] [PATCH v2] persistent dirty bitmap: add QDB file spec.
QDB file is for storing dirty bitmap. The specification is based on qcow2 specification. Saving several bitmaps is necessary when server shutdowns during backup. In this case 2 tables for each disk are available. One collected for a previous period and one active. Though this feature is discussable. Big endian format and Standard Cluster Descriptor are used to simplify integration with qcow2, to support internal bitmaps for qcow2 in future. The idea is that the same procedure writing the data to QDB file could do the same for QCOW2. The only difference is cluster refcount table. Should we use it here or not is still questionable. Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@parallels.com --- docs/specs/qdb.txt | 132 + 1 file changed, 132 insertions(+) create mode 100644 docs/specs/qdb.txt diff --git a/docs/specs/qdb.txt b/docs/specs/qdb.txt new file mode 100644 index 000..d570a69 --- /dev/null +++ b/docs/specs/qdb.txt @@ -0,0 +1,132 @@ +== General == + +QDB means Qemu Dirty Bitmaps. QDB file can store several dirty bitmaps. +QDB file is organized in units of constant size, which are called clusters. + +All numbers in QDB are stored in Big Endian byte order. + +== Header == + +The first cluster of a QDB image contains the file header: + +Byte 0 - 3: magic +QDB magic string (QDB\0) + + 4 - 7: version +Version number (valid value is 1) + + 8 - 11: cluster_bits +Number of bits that are used for addressing an offset +within a cluster (1 cluster_bits is the cluster size). +Must not be less than 9 (i.e. 512 byte clusters). + + 12 - 15: nb_bitmaps +Number of bitmaps contained in the file + + 16 - 23: bitmaps_offset +Offset into the QDB file at which the bitmap table starts. +Must be aligned to a cluster boundary. + + 24 - 27: header_length +Length of the header structure in bytes. + +Like in qcow2, directly after the image header, optional sections called header extensions can +be stored. Each extension has a structure like the following: + +Byte 0 - 3: Header extension type: +0x - End of the header extension area +other - Unknown header extension, can be safely + ignored + + 4 - 7: Length of the header extension data + + 8 - n: Header extension data + + n - m: Padding to round up the header extension size to the next +multiple of 8. + +Unless stated otherwise, each header extension type shall appear at most once +in the same image. + +== Cluster mapping == + +QDB uses a ONE-level structure for the mapping of +bitmaps to host clusters. It is called L1 table. + +The L1 table has a variable size (stored in the Bitmap table entry) and may +use multiple clusters, however it must be contiguous in the QDB file. + +Given a offset into the bitmap, the offset into the QDB file can be +obtained as follows: + +offset = l1_table[offset / cluster_size] + (offset % cluster_size) + +L1 table entry: + +Bit 0 - 61: Cluster descriptor + +62 - 63: Reserved + +Standard Cluster Descriptor (the same as in qcow2): + +Bit 0:If set to 1, the cluster reads as all zeros. The host +cluster offset can be used to describe a preallocation, +but it won't be used for reading data from this cluster, +nor is data read from the backing file if the cluster is +unallocated. + + 1 - 8:Reserved (set to 0) + + 9 - 55:Bits 9-55 of host cluster offset. Must be aligned to a +cluster boundary. If the offset is 0, the cluster is +unallocated. + +56 - 61:Reserved (set to 0) + +If a cluster is unallocated, read requests shall read zero. + +== Bitmap table == + +QDB supports storing of several bitmaps. + +A directory of all bitmaps is stored in the bitmap table, a contiguous area +in the QDB file, whose starting offset and length are given by the header +fields bitmaps_offset and nb_bitmaps. The entries of the bitmap table +have variable length, depending on the length of name and extra data. + +Bitmap table entry: + +Byte 0 - 7:Offset into the QDB file at which the L1 table for the +bitmap starts. Must be aligned to a cluster boundary. + + 8 - 11:Number of entries in the L1 table of the bitmap + +12 - 15:Bitmap granularity +As represented in HBitmap structure. Given a granularity of +G, each bit in the bitmap will actually represent a group +of 2^G bytes. + +16 - 23:Bitmap size
Re: [Qemu-devel] [PATCH v2] persistent dirty bitmap: add QDB file spec.
Also, it may be better to make this as qcow2 extension. And bitmap will be saved in separate qcow2 file, which will contain only the bitmap(s) and no other data (no disk, no snapshots). Best regards, Vladimir On 20.11.2014 13:34, Vladimir Sementsov-Ogievskiy wrote: QDB file is for storing dirty bitmap. The specification is based on qcow2 specification. Saving several bitmaps is necessary when server shutdowns during backup. In this case 2 tables for each disk are available. One collected for a previous period and one active. Though this feature is discussable. Big endian format and Standard Cluster Descriptor are used to simplify integration with qcow2, to support internal bitmaps for qcow2 in future. The idea is that the same procedure writing the data to QDB file could do the same for QCOW2. The only difference is cluster refcount table. Should we use it here or not is still questionable. Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@parallels.com --- docs/specs/qdb.txt | 132 + 1 file changed, 132 insertions(+) create mode 100644 docs/specs/qdb.txt diff --git a/docs/specs/qdb.txt b/docs/specs/qdb.txt new file mode 100644 index 000..d570a69 --- /dev/null +++ b/docs/specs/qdb.txt @@ -0,0 +1,132 @@ +== General == + +QDB means Qemu Dirty Bitmaps. QDB file can store several dirty bitmaps. +QDB file is organized in units of constant size, which are called clusters. + +All numbers in QDB are stored in Big Endian byte order. + +== Header == + +The first cluster of a QDB image contains the file header: + +Byte 0 - 3: magic +QDB magic string (QDB\0) + + 4 - 7: version +Version number (valid value is 1) + + 8 - 11: cluster_bits +Number of bits that are used for addressing an offset +within a cluster (1 cluster_bits is the cluster size). +Must not be less than 9 (i.e. 512 byte clusters). + + 12 - 15: nb_bitmaps +Number of bitmaps contained in the file + + 16 - 23: bitmaps_offset +Offset into the QDB file at which the bitmap table starts. +Must be aligned to a cluster boundary. + + 24 - 27: header_length +Length of the header structure in bytes. + +Like in qcow2, directly after the image header, optional sections called header extensions can +be stored. Each extension has a structure like the following: + +Byte 0 - 3: Header extension type: +0x - End of the header extension area +other - Unknown header extension, can be safely + ignored + + 4 - 7: Length of the header extension data + + 8 - n: Header extension data + + n - m: Padding to round up the header extension size to the next +multiple of 8. + +Unless stated otherwise, each header extension type shall appear at most once +in the same image. + +== Cluster mapping == + +QDB uses a ONE-level structure for the mapping of +bitmaps to host clusters. It is called L1 table. + +The L1 table has a variable size (stored in the Bitmap table entry) and may +use multiple clusters, however it must be contiguous in the QDB file. + +Given a offset into the bitmap, the offset into the QDB file can be +obtained as follows: + +offset = l1_table[offset / cluster_size] + (offset % cluster_size) + +L1 table entry: + +Bit 0 - 61: Cluster descriptor + +62 - 63: Reserved + +Standard Cluster Descriptor (the same as in qcow2): + +Bit 0:If set to 1, the cluster reads as all zeros. The host +cluster offset can be used to describe a preallocation, +but it won't be used for reading data from this cluster, +nor is data read from the backing file if the cluster is +unallocated. + + 1 - 8:Reserved (set to 0) + + 9 - 55:Bits 9-55 of host cluster offset. Must be aligned to a +cluster boundary. If the offset is 0, the cluster is +unallocated. + +56 - 61:Reserved (set to 0) + +If a cluster is unallocated, read requests shall read zero. + +== Bitmap table == + +QDB supports storing of several bitmaps. + +A directory of all bitmaps is stored in the bitmap table, a contiguous area +in the QDB file, whose starting offset and length are given by the header +fields bitmaps_offset and nb_bitmaps. The entries of the bitmap table +have variable length, depending on the length of name and extra data. + +Bitmap table entry: + +Byte 0 - 7:Offset into the QDB file at which the L1 table for the +bitmap starts. Must be aligned to a cluster boundary. + + 8 - 11:Number of entries in the L1 table of the
Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 v3 1/1] -machine vmport=auto: Fix handling of VMWare ioport emulation for xen
On 20/11/2014 11:00, Dr. David Alan Gilbert wrote: I'm still not sure why the configuration should differ for -M pc depending on whether xen is enabled. I think this goes back to: commit 1611977c3d8fdbdac6090cbd1fcee4aed6d9 Author: Anthony PERARD anthony.per...@citrix.com Date: Tue May 3 17:06:54 2011 +0100 pc, Disable vmport initialisation with Xen. This is because there is not synchronisation of the vcpu register between Xen and QEMU, so vmport can't work properly. This patch introduces no_vmport parameter to pc_basic_device_init. Signed-off-by: Anthony PERARD anthony.per...@citrix.com Signed-off-by: Alexander Graf ag...@suse.de Yes, but Xen has since implemented vmport (commit 37f9e258). It's fine to have a conservative default for -M xenfv and possibly -M pc-2.1, but -M pc can require the latest hypervisor. Paolo
Re: [Qemu-devel] [RFC][PATCH v2] block: add write threshold reporting for block devices
On Thu, Nov 20, 2014 at 11:30:53AM +0100, Kevin Wolf wrote:
Am 17.11.2014 um 17:49 hat Stefan Hajnoczi geschrieben:
On Fri, Nov 07, 2014 at 02:12:13PM +0100, Francesco Romani wrote:
+void bdrv_set_usage_threshold(BlockDriverState *bs, int64_t
threshold_bytes)
+{
+BlockDriverState *target_bs = bs;
+if (bs-file) {
+target_bs = bs-file;
+}
Hmm...I think now I understand why you are trying to use bs-file. This
is an attempt to make image formats work with the threshold.
Unfortunately the BlockDriverState topology can be more complicated than
just 1 level.
If we hardcode a strategy to traverse bs-file then it will work in most
cases:
while (bs-file) {
bs = bs-file;
}
But there are cases like VMDK extent files where a BlockDriverState
actually has multiple children.
One way to solve this is to require that the management tool tells QEMU
which exact BlockDriverState node the threshold applies to. Then QEMU
doesn't need any hardcoded policy. But I'm not sure how realistic that
it at the moment (whether management tools are uses node names for each
node yet), so it may be best to hardcode the bs-file traversal that
I've suggested.
Kevin: Do you agree?
I have a feeling that we would regret this in the long run because it
would allow only one special case of a general problem (watching a BDS).
This means that we'll get inconsistent APIs.
We're only talking about an optimisation here, even though a very
useful one, so I wouldn't easily make compromises here. We should
probably insist on using the node-name. Management tools need new code
anyway to make use of the new functionality, so they can implement
node-name support as well while they're at it.
Using node-name is the best thing to do.
My concern is just whether libvirt and other management tools are
actually using node-name yet.
Stefan
pgpyKyWSIYpyf.pgp
Description: PGP signature
[Qemu-devel] [PATCH] target-mips: gdbstub: Clean up FPU register handling
Rewrite the FPU register access parts of `mips_cpu_gdb_read_register'
and `mips_cpu_gdb_write_register' for consistency between each other.
Signed-off-by: Maciej W. Rozycki ma...@codesourcery.com
---
Hi,
This is the FPU register handling cleanup previously promised.
It was regression-tested by running the GDB test suite for the
mips-sde-elf target and a -EB (o32) multilib, on a 24Kf processor.
QEMU in the system emulation mode was used as the remote stub for GDB.
Apart from overall register accesses the test suite includes specific
tests that make manual function calls with FP arguments and/or FP
results and these rely on FP registers to be passed around correctly.
I also visually inspected test logs and verified that the values
reported for CP1.FIR and CP1.FCSR did not change.
Please apply.
Maciej
qemu-mips-gdbstub-cleanup.diff
Index: qemu-git-trunk/target-mips/gdbstub.c
===
--- qemu-git-trunk.orig/target-mips/gdbstub.c 2014-11-20 10:44:24.058944521
+
+++ qemu-git-trunk/target-mips/gdbstub.c2014-11-20 10:44:28.058940153
+
@@ -29,8 +29,13 @@ int mips_cpu_gdb_read_register(CPUState
if (n 32) {
return gdb_get_regl(mem_buf, env-active_tc.gpr[n]);
}
-if (env-CP0_Config1 (1 CP0C1_FP)) {
-if (n = 38 n 70) {
+if (env-CP0_Config1 (1 CP0C1_FP) n = 38 n 72) {
+switch (n) {
+case 70:
+return gdb_get_regl(mem_buf, (int32_t)env-active_fpu.fcr31);
+case 71:
+return gdb_get_regl(mem_buf, (int32_t)env-active_fpu.fcr0);
+default:
if (env-CP0_Status (1 CP0St_FR)) {
return gdb_get_regl(mem_buf,
env-active_fpu.fpr[n - 38].d);
@@ -39,12 +44,6 @@ int mips_cpu_gdb_read_register(CPUState
env-active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX]);
}
}
-switch (n) {
-case 70:
-return gdb_get_regl(mem_buf, (int32_t)env-active_fpu.fcr31);
-case 71:
-return gdb_get_regl(mem_buf, (int32_t)env-active_fpu.fcr0);
-}
}
switch (n) {
case 32:
@@ -64,8 +63,10 @@ int mips_cpu_gdb_read_register(CPUState
return gdb_get_regl(mem_buf, 0); /* fp */
case 89:
return gdb_get_regl(mem_buf, (int32_t)env-CP0_PRid);
-}
-if (n = 73 n = 88) {
+default:
+if (n 89) {
+return 0;
+}
/* 16 embedded regs. */
return gdb_get_regl(mem_buf, 0);
}
@@ -89,15 +90,7 @@ int mips_cpu_gdb_write_register(CPUState
env-active_tc.gpr[n] = tmp;
return sizeof(target_ulong);
}
-if (env-CP0_Config1 (1 CP0C1_FP)
- n = 38 n 72) {
-if (n 70) {
-if (env-CP0_Status (1 CP0St_FR)) {
-env-active_fpu.fpr[n - 38].d = tmp;
-} else {
-env-active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
-}
-}
+if (env-CP0_Config1 (1 CP0C1_FP) n = 38 n 72) {
switch (n) {
case 70:
env-active_fpu.fcr31 = tmp 0xFF83;
@@ -107,6 +100,12 @@ int mips_cpu_gdb_write_register(CPUState
case 71:
/* FIR is read-only. Ignore writes. */
break;
+default:
+if (env-CP0_Status (1 CP0St_FR))
+env-active_fpu.fpr[n - 38].d = tmp;
+else
+env-active_fpu.fpr[n - 38].w[FP_ENDIAN_IDX] = tmp;
+break;
}
return sizeof(target_ulong);
}
[Qemu-devel] [PATCH] target-mips: Also apply the CP0.Status mask to MTTC0
Make CP0.Status writes made with the MTTC0 instruction respect this
register's mask just like all the other places. Also preserve the
current values of masked out bits.
Signed-off-by: Maciej W. Rozycki ma...@codesourcery.com
---
Hi,
This should be obvious. Also quite obviously, we are missing a lot of
stuff in this area so as it is added this is something to watch out for,
e.g. CP0.ConfigX writes will have to respect the respective masks too.
But that's another matter. For the time being, please apply.
Maciej
qemu-mips-mttc-status.diff
Index: qemu-git-trunk/target-mips/op_helper.c
===
--- qemu-git-trunk.orig/target-mips/op_helper.c 2014-11-12 07:41:26.597542010
+
+++ qemu-git-trunk/target-mips/op_helper.c 2014-11-12 07:43:02.107518555
+
@@ -1413,9 +1413,10 @@ void helper_mtc0_status(CPUMIPSState *en
void helper_mttc0_status(CPUMIPSState *env, target_ulong arg1)
{
int other_tc = env-CP0_VPEControl (0xff CP0VPECo_TargTC);
+uint32_t mask = env-CP0_Status_rw_bitmask ~0xf118;
CPUMIPSState *other = mips_cpu_map_tc(env, other_tc);
-other-CP0_Status = arg1 ~0xf118;
+other-CP0_Status = (other-CP0_Status ~mask) | (arg1 mask);
sync_c0_status(env, other, other_tc);
}
Re: [Qemu-devel] [Spice-devel] screen freezed for 2-3 minutes on spice connect on xen windows 7 domU's with qxl after save/restore
Il 13/11/2014 13:22, Fabio Fantoni ha scritto: Il 13/11/2014 11:14, Fabio Fantoni ha scritto: Il 19/09/2014 15:18, Fabio Fantoni ha scritto: Il 12/09/2014 16:46, Fabio Fantoni ha scritto: Il 08/07/2014 12:34, Fabio Fantoni ha scritto: Il 08/07/2014 12:06, Fabio Fantoni ha scritto: Il 08/07/2014 10:53, David Jaša ha scritto: Hi, On Út, 2014-07-08 at 10:13 +0200, Fabio Fantoni wrote: On xen 4.5 (tried with qemu 2.0.0/2.1-rc0, spice 0.12.5 and client with spice-gtk 0.23/0.25) windows 7 domUs with qxl vga works good as kvm except for one problem after xl save/restore, when after restore on spice client connect the domU's screen freezed for 2-3 minutes (and seems also windows), after this time seems that all return to works correctly. This problem happen also if spice client connect long time after restore. With stdvga not have this problem but stdvga has many missed resolutions and bad refresh performance. If you need more tests/informations tell me and I'll post them. Client and server logs would certainly help. Please run: * virt-viewer with --spice-debug option * spice-server with SPICE_DEBUG_LEVEL environment variable set to 4 or 5 (if you use qemu+libvirt, use qemu:env element: http://libvirt.org/drvqemu.html#qemucommand ) and note the location in the logs where the freeze takes place. Regards, David Thanks for your reply, in attachments: - domU's xl cfg: W7.cfg - xl -vvv create/save/restore: xen logs.txt - remote-viewer with --spice-debug after domU's start until xl save: spicelog-1.txt (zipped) - remote-viewer with --spice-debug after domU's xl restore: spicelog-2.txt Sorry for my forgetfulness, here also qemu's log: - after domU's start until xl save: qemu-dm-W7.log.1 - after domU's xl restore: qemu-dm-W7.log If you need more tests/informations tell me and I'll post them. Thanks for any reply and sorry for my bad english. ___ Spice-devel mailing list spice-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel The problem persist, this time I saw these in xl dmesg after restore: (XEN) HVM2 restore: CPU 0 (XEN) HVM2 restore: CPU 1 (XEN) HVM2 restore: PIC 0 (XEN) HVM2 restore: PIC 1 (XEN) HVM2 restore: IOAPIC 0 (XEN) HVM2 restore: LAPIC 0 (XEN) HVM2 restore: LAPIC 1 (XEN) HVM2 restore: LAPIC_REGS 0 (XEN) HVM2 restore: LAPIC_REGS 1 (XEN) HVM2 restore: PCI_IRQ 0 (XEN) HVM2 restore: ISA_IRQ 0 (XEN) HVM2 restore: PCI_LINK 0 (XEN) HVM2 restore: PIT 0 (XEN) HVM2 restore: RTC 0 (XEN) HVM2 restore: HPET 0 (XEN) HVM2 restore: PMTIMER 0 (XEN) HVM2 restore: MTRR 0 (XEN) HVM2 restore: MTRR 1 (XEN) HVM2 restore: VIRIDIAN_DOMAIN 0 (XEN) HVM2 restore: VIRIDIAN_VCPU 0 (XEN) HVM2 restore: VIRIDIAN_VCPU 1 (XEN) HVM2 restore: VMCE_VCPU 0 (XEN) HVM2 restore: VMCE_VCPU 1 (XEN) HVM2 restore: TSC_ADJUST 0 (XEN) HVM2 restore: TSC_ADJUST 1 (XEN) memory.c:216:d2v0 Domain 2 page number 77579 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7757a invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7757b invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7757c invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7757d invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7757e invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7757f invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77580 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77581 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77582 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77583 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77584 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77585 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77586 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77587 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77588 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77589 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7758a invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7758b invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7758c invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7758d invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7758e invalid (XEN) memory.c:216:d2v0 Domain 2 page number 7758f invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77590 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77591 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77592 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77593 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77594 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77595 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77596 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77597 invalid (XEN) memory.c:216:d2v0 Domain 2 page number 77598 invalid (XEN) grant_table.c:1272:d2v0 Expanding dom (2) grant table from (4) to (32) frames. (XEN) irq.c:380: Dom2 callback via changed to GSI 24 Tested on latest staging (commit 7d203b337fb2dcd148d2df850e25b67c792d4d0b)
Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration
On 24/06/2014 08:23, Gonglei (Arei) wrote: -Original Message- From: Juan Quintela [mailto:quint...@redhat.com] Sent: Friday, March 21, 2014 9:26 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; owass...@redhat.com; pbonz...@redhat.com; ebl...@redhat.com; dgilb...@redhat.com; chenliang (T) Subject: Re: [PATCH] migration: static variables will not be reset at second migration arei.gong...@huawei.com wrote: From: ChenLiang chenlian...@huawei.com The static variables in migration_bitmap_sync will not be reset in the case of a second attempted migration. Signed-off-by: ChenLiang chenlian...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com Good catch. Applied.. Hi, Juan? Ping... please :) Juan, what happened to this patch? Paolo
Re: [Qemu-devel] [RFC][PATCH v2] block: add write threshold reporting for block devices
Am 20.11.2014 um 12:04 hat Stefan Hajnoczi geschrieben:
On Thu, Nov 20, 2014 at 11:30:53AM +0100, Kevin Wolf wrote:
Am 17.11.2014 um 17:49 hat Stefan Hajnoczi geschrieben:
On Fri, Nov 07, 2014 at 02:12:13PM +0100, Francesco Romani wrote:
+void bdrv_set_usage_threshold(BlockDriverState *bs, int64_t
threshold_bytes)
+{
+BlockDriverState *target_bs = bs;
+if (bs-file) {
+target_bs = bs-file;
+}
Hmm...I think now I understand why you are trying to use bs-file. This
is an attempt to make image formats work with the threshold.
Unfortunately the BlockDriverState topology can be more complicated than
just 1 level.
If we hardcode a strategy to traverse bs-file then it will work in most
cases:
while (bs-file) {
bs = bs-file;
}
But there are cases like VMDK extent files where a BlockDriverState
actually has multiple children.
One way to solve this is to require that the management tool tells QEMU
which exact BlockDriverState node the threshold applies to. Then QEMU
doesn't need any hardcoded policy. But I'm not sure how realistic that
it at the moment (whether management tools are uses node names for each
node yet), so it may be best to hardcode the bs-file traversal that
I've suggested.
Kevin: Do you agree?
I have a feeling that we would regret this in the long run because it
would allow only one special case of a general problem (watching a BDS).
This means that we'll get inconsistent APIs.
We're only talking about an optimisation here, even though a very
useful one, so I wouldn't easily make compromises here. We should
probably insist on using the node-name. Management tools need new code
anyway to make use of the new functionality, so they can implement
node-name support as well while they're at it.
Using node-name is the best thing to do.
My concern is just whether libvirt and other management tools are
actually using node-name yet.
I don't think so. They also don't use blockdev-add yet.
But that's not a reason for us to add hacks that allow libvirt and other
management tools to avoid the proper APIs even in the future. They just
need to add support for node-names if they want to use new qemu features.
New features require support for new infrastructure, I think that's fair.
If they feel that representing complete BDS graphs in their code is too
much work for now, they can still keep temporary hacks with hardcoded
assumptions in their management code (like setting file.node-name and
ignoring other setups). At least it would be temporary hacks there; if
we did them in qemu, they would be a permanent API.
Kevin
pgpmPx5IZ5nrz.pgp
Description: PGP signature
[Qemu-devel] [PATCH v2 for-2.2 0/4] net: fix high impact outstanding defects reported by Coverity
From: Gonglei arei.gong...@huawei.com Please see details in every patch. v2 - v1: - rewrite patch 3 and patch 4 by Paolo's suggestion. Thanks. - add Jason's R-b tag in patch 1~3. Thanks too. Cc: Paolo Bonzini pbonz...@redhat.com Cc: Stefan Hajnoczi stefa...@redhat.com Cc: Jason Wang jasow...@redhat.com Gonglei (4): net/slirp: fix memory leak net/socket: fix Uninitialized scalar variable pcnet: fix Negative array index read rtl8139: fix Pointer to local outside scope hw/net/pcnet.c | 55 ++- hw/net/rtl8139.c | 4 net/slirp.c | 3 +-- net/socket.c | 11 ++- 4 files changed, 41 insertions(+), 32 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH v2 for-2.2 1/4] net/slirp: fix memory leak
From: Gonglei arei.gong...@huawei.com
commit b412eb61 introduce 'cmd:' target for guestfwd,
and fwd don't be used in this scenario, and will leak
memory in true branch with 'cmd:'. Let's allocate memory
for fwd variable just in else statement.
Cc: Alexander Graf ag...@suse.de
Signed-off-by: Gonglei arei.gong...@huawei.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
net/slirp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index dc89e6b..377d7ef 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -643,17 +643,16 @@ static int slirp_guestfwd(SlirpState *s, const char
*config_str,
goto fail_syntax;
}
-fwd = g_malloc(sizeof(struct GuestFwd));
snprintf(buf, sizeof(buf), guestfwd.tcp.%d, port);
if ((strlen(p) 4) !strncmp(p, cmd:, 4)) {
if (slirp_add_exec(s-slirp, 0, p[4], server, port) 0) {
error_report(conflicting/invalid host:port in guest forwarding
rule '%s', config_str);
-g_free(fwd);
return -1;
}
} else {
+fwd = g_malloc(sizeof(struct GuestFwd));
fwd-hd = qemu_chr_new(buf, p, NULL);
if (!fwd-hd) {
error_report(could not open guest forwarding device '%s', buf);
--
1.7.12.4
[Qemu-devel] [PATCH v2 for-2.2 4/4] rtl8139: fix Pointer to local outside scope
From: Gonglei arei.gong...@huawei.com
Coverity spot:
Assigning: iov = struct iovec [3]({{buf, 12UL},
{(void *)dot1q_buf, 4UL},
{buf + 12, size - 12}})
(address of temporary variable of type struct iovec [3]).
out_of_scope: Temporary variable of type struct iovec [3] goes out of scope.
Pointer to local outside scope (RETURN_LOCAL)
use_invalid:
Using iov, which points to an out-of-scope temporary variable of type struct
iovec [3].
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/net/rtl8139.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 8b8a1b1..5f0197c 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -1775,6 +1775,7 @@ static void rtl8139_transfer_frame(RTL8139State *s,
uint8_t *buf, int size,
int do_interrupt, const uint8_t *dot1q_buf)
{
struct iovec *iov = NULL;
+struct iovec vlan_iov[3];
if (!size)
{
@@ -1789,6 +1790,9 @@ static void rtl8139_transfer_frame(RTL8139State *s,
uint8_t *buf, int size,
{ .iov_base = buf + ETHER_ADDR_LEN * 2,
.iov_len = size - ETHER_ADDR_LEN * 2 },
};
+
+memcpy(vlan_iov, iov, sizeof(vlan_iov));
+iov = vlan_iov;
}
if (TxLoopBack == (s-TxConfig TxLoopBack))
--
1.7.12.4
[Qemu-devel] [PATCH v2 for-2.2 2/4] net/socket: fix Uninitialized scalar variable
From: Gonglei arei.gong...@huawei.com
If is_connected parameter is false, the saddr
variable will no initialize. Coverity report:
uninit_use: Using uninitialized value saddr.sin_port.
We don't need add saddr information to nc-info_str
when is_connected is false.
Signed-off-by: Gonglei arei.gong...@huawei.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
net/socket.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index ca4b8ba..68a93cd 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -389,11 +389,6 @@ static NetSocketState
*net_socket_fd_init_dgram(NetClientState *peer,
nc = qemu_new_net_client(net_dgram_socket_info, peer, model, name);
-snprintf(nc-info_str, sizeof(nc-info_str),
-socket: fd=%d (%s mcast=%s:%d),
-fd, is_connected ? cloned : ,
-inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
-
s = DO_UPCAST(NetSocketState, nc, nc);
s-fd = fd;
@@ -404,6 +399,12 @@ static NetSocketState
*net_socket_fd_init_dgram(NetClientState *peer,
/* mcast: save bound address as dst */
if (is_connected) {
s-dgram_dst = saddr;
+snprintf(nc-info_str, sizeof(nc-info_str),
+ socket: fd=%d (cloned mcast=%s:%d),
+ fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+} else {
+snprintf(nc-info_str, sizeof(nc-info_str),
+ socket: fd=%d, fd);
}
return s;
--
1.7.12.4
Re: [Qemu-devel] [PATCH v2] persistent dirty bitmap: add QDB file spec.
On Thu, Nov 20, 2014 at 01:41:14PM +0300, Vladimir Sementsov-Ogievskiy wrote: Also, it may be better to make this as qcow2 extension. And bitmap will be saved in separate qcow2 file, which will contain only the bitmap(s) and no other data (no disk, no snapshots). I think you are on to something with the idea of making the persistent dirty bitmap itself a disk image. That way drive-mirror and other commands can be used to live migrate the dirty bitmap along with the guest's disks. This allows both QEMU and management tools to reuse existing code. (We may need to allow multiple block jobs per BlockDriverState to make this work but in theory that can be done.) There is a constraint if we want to get live migration for free: The bitmap contents must be accessible with bdrv_read() and bdrv_get_block_status() to skip zero regions. Putting the dirty bitmap into its own data structure in qcow2 and not accessible as a BlockDriverState bdrv_read() means custom code must be written to migrate the dirty bitmap. So I suggest putting the bitmap contents into a disk image that can be accessed as a BlockDriverState with bdrv_read(). The metadata (bitmap name, granularity, etc) doesn't need to be stored in the image file because management tools must be aware of it anyway. The only thing besides the data that really needs to be stored is the up-to-date flag to decide whether this dirty bitmap was synced cleanly. A much simpler format would do for that. Stefan pgp2_wMK4EiKn.pgp Description: PGP signature
[Qemu-devel] [PATCH v2 for-2.2 3/4] pcnet: fix Negative array index read
From: Gonglei arei.gong...@huawei.com
s-xmit_pos maybe assigned to a negative value (-1),
but in this branch variable s-xmit_pos as an index to
array s-buffer. Let's add a check for s-xmit_pos.
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
hw/net/pcnet.c | 55 ++-
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index d344c15..f409b92 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1212,7 +1212,7 @@ static void pcnet_transmit(PCNetState *s)
hwaddr xmit_cxda = 0;
int count = CSR_XMTRL(s)-1;
int add_crc = 0;
-
+int bcnt;
s-xmit_pos = -1;
if (!CSR_TXON(s)) {
@@ -1247,35 +1247,40 @@ static void pcnet_transmit(PCNetState *s)
s-xmit_pos = -1;
goto txdone;
}
+
+if (s-xmit_pos 0) {
+goto txdone;
+}
+
+bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT);
+s-phys_mem_read(s-dma_opaque, PHYSADDR(s, tmd.tbadr),
+ s-buffer + s-xmit_pos, bcnt, CSR_BSWP(s));
+s-xmit_pos += bcnt;
+
if (!GET_FIELD(tmd.status, TMDS, ENP)) {
-int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT);
-s-phys_mem_read(s-dma_opaque, PHYSADDR(s, tmd.tbadr),
- s-buffer + s-xmit_pos, bcnt, CSR_BSWP(s));
-s-xmit_pos += bcnt;
-} else if (s-xmit_pos = 0) {
-int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT);
-s-phys_mem_read(s-dma_opaque, PHYSADDR(s, tmd.tbadr),
- s-buffer + s-xmit_pos, bcnt, CSR_BSWP(s));
-s-xmit_pos += bcnt;
+goto txdone;
+}
+
#ifdef PCNET_DEBUG
-printf(pcnet_transmit size=%d\n, s-xmit_pos);
+printf(pcnet_transmit size=%d\n, s-xmit_pos);
#endif
-if (CSR_LOOP(s)) {
-if (BCR_SWSTYLE(s) == 1)
-add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS);
-s-looptest = add_crc ? PCNET_LOOPTEST_CRC :
PCNET_LOOPTEST_NOCRC;
-pcnet_receive(qemu_get_queue(s-nic), s-buffer, s-xmit_pos);
-s-looptest = 0;
-} else
-if (s-nic)
-qemu_send_packet(qemu_get_queue(s-nic), s-buffer,
- s-xmit_pos);
-
-s-csr[0] = ~0x0008; /* clear TDMD */
-s-csr[4] |= 0x0004;/* set TXSTRT */
-s-xmit_pos = -1;
+if (CSR_LOOP(s)) {
+if (BCR_SWSTYLE(s) == 1)
+add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS);
+s-looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC;
+pcnet_receive(qemu_get_queue(s-nic), s-buffer, s-xmit_pos);
+s-looptest = 0;
+} else {
+if (s-nic) {
+qemu_send_packet(qemu_get_queue(s-nic), s-buffer,
+ s-xmit_pos);
+}
}
+s-csr[0] = ~0x0008; /* clear TDMD */
+s-csr[4] |= 0x0004;/* set TXSTRT */
+s-xmit_pos = -1;
+
txdone:
SET_FIELD(tmd.status, TMDS, OWN, 0);
TMDSTORE(tmd, PHYSADDR(s,CSR_CXDA(s)));
--
1.7.12.4
Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration
On 2014/11/20 19:30, Paolo Bonzini wrote: On 24/06/2014 08:23, Gonglei (Arei) wrote: -Original Message- From: Juan Quintela [mailto:quint...@redhat.com] Sent: Friday, March 21, 2014 9:26 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; owass...@redhat.com; pbonz...@redhat.com; ebl...@redhat.com; dgilb...@redhat.com; chenliang (T) Subject: Re: [PATCH] migration: static variables will not be reset at second migration arei.gong...@huawei.com wrote: From: ChenLiang chenlian...@huawei.com The static variables in migration_bitmap_sync will not be reset in the case of a second attempted migration. Signed-off-by: ChenLiang chenlian...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com Good catch. Applied.. Hi, Juan? Ping... please :) Juan, what happened to this patch? Paolo Nearly for half a year, I forgot it completely :( Thanks for your prompt, Paolo. Best regards, -Gonglei
[Qemu-devel] [Bug 1394550] [NEW] qemu: linux kernel too old to load a ram disk
Public bug reported: I was built kernel-genkernel-x86_64-3.17.3-gentoo-gnu and initramfs-genkernel-x86_64-3.17.3-gentoo-gnu in Gentoo Linux from sys- kernel/gentoo-sources/gentoo-sources-3.17.3.ebuild When I run this kernel with switches -kernel -initrd -append (and others), qemu gives misleading message: qemu: linux kernel too old to load a ram disk this is not an old linux! Just something should be configured better. ** Affects: qemu Importance: Undecided Status: New ** Description changed: I was built kernel-genkernel-x86_64-3.17.3-gentoo-gnu and initramfs-genkernel-x86_64-3.17.3-gentoo-gnu in Gentoo Linux from sys- kernel/gentoo-sources/gentoo-sources-3.17.3.ebuild When I run this kernel with switches -kernel -initrd -append (and others), qemu gives misleading message: qemu: linux kernel too old to load a ram disk - this is not an old linux! Jusr something should be configured better. + this is not an old linux! Just something should be configured better. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1394550 Title: qemu: linux kernel too old to load a ram disk Status in QEMU: New Bug description: I was built kernel-genkernel-x86_64-3.17.3-gentoo-gnu and initramfs-genkernel-x86_64-3.17.3-gentoo-gnu in Gentoo Linux from sys- kernel/gentoo-sources/gentoo-sources-3.17.3.ebuild When I run this kernel with switches -kernel -initrd -append (and others), qemu gives misleading message: qemu: linux kernel too old to load a ram disk this is not an old linux! Just something should be configured better. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1394550/+subscriptions
Re: [Qemu-devel] [PATCH v4 33/47] Postcopy: Postcopy startup in migration thread
* Paolo Bonzini (pbonz...@redhat.com) wrote: Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto: From: Dr. David Alan Gilbert dgilb...@redhat.com Rework the migration thread to setup and start postcopy. Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com --- include/migration/migration.h | 3 + migration.c | 201 ++ 2 files changed, 185 insertions(+), 19 deletions(-) diff --git a/include/migration/migration.h b/include/migration/migration.h index b01cc17..f401775 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -125,6 +125,9 @@ struct MigrationState /* Flag set once the migration has been asked to enter postcopy */ volatile bool start_postcopy; +/* Flag set once the migration thread is running (and needs joining) */ +volatile bool started_migration_thread; volatile almost never does what you think it does. :) True. In this case, I think only one thread reads/writes the variable so volatile is unnecessary. Lets just check that; so it's set by 'migrate_fd_connect' (from the main thread) when it spawns the thread, and it's cleared by migrate_fd_cleanup that's always run as a bh, so should always be in the main thread; so yes - always the same thread, that's nice and simple; volatile evaporated. Otherwise, you would need to add actual memory barriers, atomic operations, or synchronization primitives. For start_postcopy, it is okay because it is just a hint to the compiler and the processor will eventually see the assignment. Yes, in this case my understanding is that it's necessary to stop the compiler potentially moving the check outside the loop. For this case QEMU has atomic_read/atomic_set (corresponding to __ATOMIC_RELAXED in C/C++1x), so you could use those as well. Ah, so those look like they just volatile cast anyway. (I've probably got some other flags I need to think about reading/writing atomically/safely). Dave (I'll take the other issues in this mail separately since there are quite a few). -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 2/4] net/socket: fix Uninitialized scalar variable
On Thu, Nov 20, 2014 at 01:57:12PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com If is_connected parameter is false, the saddr variable will no initialize. Coverity report: uninit_use: Using uninitialized value saddr.sin_port. We don't need add saddr information to nc-info_str when is_connected is false. Signed-off-by: Gonglei arei.gong...@huawei.com --- net/socket.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpebySdloTSs.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 0/4] net: fix high impact outstanding defects reported by Coverity
On Thu, Nov 20, 2014 at 01:57:10PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com Please see details in every patch. Cc: Paolo Bonzini pbonz...@redhat.com Cc: Stefan Hajnoczi stefa...@redhat.com Gonglei (4): net/slirp: fix memory leak net/socket: fix Uninitialized scalar variable pcnet: fix Negative array index read rtl8139: fix Pointer to local outside scope hw/net/pcnet.c | 2 +- hw/net/rtl8139.c | 36 net/slirp.c | 3 +-- net/socket.c | 11 ++- 4 files changed, 20 insertions(+), 32 deletions(-) Thanks, please respin with updated Patch 3 4. Stefan pgpqN_u0BXbyE.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v2 0/3] Migration-safe ACPI table sizing algorithm
On Thu, Nov 20, 2014 at 11:04:13AM +0100, Paolo Bonzini wrote: On 20/11/2014 08:55, Michael S. Tsirkin wrote: On Thu, Nov 20, 2014 at 08:11:05AM +0100, Paolo Bonzini wrote: On 20/11/2014 07:55, Michael S. Tsirkin wrote: I thought we agreed we'll consider alternate approaches after 2.2? I would prefer not to have yet another mode to support if we can help it. I agree, but: 1) looks like there is stronger opposition to your patch than I thought, so a 2.2 solution as in this patch becomes more palatable Why the urgency? It's not fixing any regressions, is it? I would rather not add yet another mode for 2.2, we'll likely have a new mode in 2.3 but I'd like that to be the last one. I don't think there's a need to add both patches. If mine goes in, and it can go in 2.2 since it is just another mode, It's a mode we don't need - adding it does not fix any bugs. there is no need for resizable MemoryRegions. Paolo There will be need - otherwise each change will keep adding modes. 2) reviewing patches is always nice, and helps evaluating the advantages of either approach Paolo I'll do my best, sorry about the delay - I'm trying to prioritize 2.2 work at the moment. -- MST
Re: [Qemu-devel] [PATCH] hw/arm/realview.c: Fix memory leak in realview_init()
On Wed, 19 Nov 2014, Peter Maydell wrote: Not for 2.2, Fair enough. and I'm still not really convinced in general that it's worthwhile at all. I'm surprised that this small patch caused so much controversy. It seems very simple and straightforward to me. This patch fixes a memory leak. The fact that it indeed was a memory leak is indicated by Valgrind output (Memcheck's false-positives are extremely rare unless you do some really nasty things with your pointers). It can be verified manually too: there are only 4 occurrences of 'ram_lo' in realview.c. By fixing memory leak this patch silences warnings from automatic checking tools like Valgrind. Not having minor warnings is good because it simplifies usage of such tools in order to find new and important bugs. This patch is local: it does not affect any other function except realview_init. Given all this I can see benefits of this patch with no real downsides to it. Is this enough to proof it's worthwhile? -- Kirill
Re: [Qemu-devel] [PATCH 0/4] net: fix high impact outstanding defects reported by Coverity
On 2014/11/20 19:51, Stefan Hajnoczi wrote: On Thu, Nov 20, 2014 at 01:57:10PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com Please see details in every patch. Cc: Paolo Bonzini pbonz...@redhat.com Cc: Stefan Hajnoczi stefa...@redhat.com Gonglei (4): net/slirp: fix memory leak net/socket: fix Uninitialized scalar variable pcnet: fix Negative array index read rtl8139: fix Pointer to local outside scope hw/net/pcnet.c | 2 +- hw/net/rtl8139.c | 36 net/slirp.c | 3 +-- net/socket.c | 11 ++- 4 files changed, 20 insertions(+), 32 deletions(-) Thanks, please respin with updated Patch 3 4. Stefan Hi, Stefan I had posted v2 a few minutes ago :) [PATCH v2 for-2.2 0/4] net: fix high impact outstanding defects reported by Coverity Best regards, -Gonglei
Re: [Qemu-devel] [PATCH 1/4] net/slirp: fix memory leak
On Thu, Nov 20, 2014 at 01:57:11PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com commit b412eb61 introduce 'cmd:' target for guestfwd, and fwd don't be used in this scenario, and will leak memory in true branch with 'cmd:'. Let's allocate memory for fwd variable just in else statement. Cc: Alexander Graf ag...@suse.de Signed-off-by: Gonglei arei.gong...@huawei.com --- net/slirp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpznQIomLBdF.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] hw/arm/realview.c: Fix memory leak in realview_init()
On 20 November 2014 11:53, Kirill Batuzov batuz...@ispras.ru wrote: I'm surprised that this small patch caused so much controversy. It seems very simple and straightforward to me. This patch fixes a memory leak. The fact that it indeed was a memory leak is indicated by Valgrind output (Memcheck's false-positives are extremely rare unless you do some really nasty things with your pointers). It can be verified manually too: there are only 4 occurrences of 'ram_lo' in realview.c. It's in exactly the same situation as the other blocks of memory like ram_hi in that file: we allocate it and then don't care about freeing it, because we don't happen to have a board state struct. The correct fix if you care about this kind of thing would be to have a board state struct which had MemoryRegion fields (not MemoryRegion* fields). We have lots of bits of memory that we allocate once on startup and then don't care about freeing. I'll probably put it in, because it's not very harmful. It just doesn't seem to me very useful to merely silence the warning rather than actually fixing the underlying thing that the warning is telling you about. -- PMM
[Qemu-devel] [PATCH] armv7m: optional -kernel if -gdb present
For standalone emulation, the image must be specified via -kernel,
but when using QEMU as a GDB server, the presence of -kernel is
no longer mandatory, since the image can be loaded by the GDB client.
Signed-off-by: Liviu Ionescu i...@livius.net
---
hw/arm/armv7m.c | 3 ++-
include/sysemu/sysemu.h | 1 +
vl.c| 7 ++-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 696de12..9d1669c 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -14,6 +14,7 @@
#include sysemu/qtest.h
#include qemu/error-report.h
#include hw/boards.h
+#include sysemu/sysemu.h
static struct arm_boot_info armv7m_binfo;
@@ -241,7 +242,7 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory,
big_endian = 0;
#endif
-if (!kernel_filename !qtest_enabled()) {
+if (!kernel_filename !qtest_enabled() !with_gdb) {
fprintf(stderr, Guest image must be specified (using -kernel)\n);
exit(1);
}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index c145d94..08bbe71 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -104,6 +104,7 @@ typedef enum DisplayType
} DisplayType;
extern int autostart;
+extern int with_gdb;
typedef enum {
VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL,
diff --git a/vl.c b/vl.c
index a88e5da..4a03fb8 100644
--- a/vl.c
+++ b/vl.c
@@ -138,6 +138,7 @@ bool enable_mlock = false;
int nb_nics;
NICInfo nd_table[MAX_NICS];
int autostart;
+int with_gdb;
static int rtc_utc = 1;
static int rtc_date_offset = -1; /* -1 means no change */
QEMUClockType rtc_clock;
@@ -2797,6 +2798,7 @@ int main(int argc, char **argv, char **envp)
uint64_t ram_slots = 0;
FILE *vmstate_dump_file = NULL;
Error *main_loop_err = NULL;
+with_gdb = false;
atexit(qemu_run_exit_notifiers);
error_set_progname(argv[0]);
@@ -3241,9 +3243,11 @@ int main(int argc, char **argv, char **envp)
break;
case QEMU_OPTION_s:
add_device_config(DEV_GDB, tcp:: DEFAULT_GDBSTUB_PORT);
+with_gdb = true;
break;
case QEMU_OPTION_gdb:
add_device_config(DEV_GDB, optarg);
+with_gdb = true;
break;
case QEMU_OPTION_L:
if (data_dir_idx ARRAY_SIZE(data_dir)) {
@@ -4443,7 +4447,8 @@ int main(int argc, char **argv, char **envp)
error_free(local_err);
exit(1);
}
-} else if (autostart) {
+} else if (autostart kernel_filename) {
+/* If an image is defined and no -S is requested, start it. */
vm_start();
}
--
1.9.3 (Apple Git-50)
Re: [Qemu-devel] [PATCH] functional ARM semihosting under GDB
Hi,
with the latest submitted patches, the functionality I expect for
qemu-system-arm is complete. (I have some more cosmetic suggestions, to be
discussed later).
to test the functionality, you can download an unit test application from:
https://dl.dropboxusercontent.com/u/78151643/gcm.elf
there are two use cases:
1) standalone (usually unit tests integrated in a continuous integration system)
$ ./qemu-system-arm -machine lm3s6965evb -kernel gcm.elf -semihosting-config
target=native,cmdline=gcm --gtest_output=xml:gcm.xml
(notice the gcm.xml file created in the current folder)
2) via GDB
$ ./qemu-system-arm -machine lm3s6965evb -semihosting-config
target=native,cmdline=gcm --gtest_output=xml:gcm.xml -gdb tcp::1234 -S
(notice the image is no longer passed here)
and from another terminal
$ arm-none-eabi-gdb gcm.elf
(gdb) target remote localhost:1234
(gdb) load
(gdb) system_reset
(gdb) break main
(gdb) continue
Breakpoint 1, main (argc=2, argv=0x21b0 argv_buf.5809) at
../gmock/src/gmock_main.cc:49
49 {
(gdb) continue
Continuing.
[Inferior 1 (Remote target) exited normally]
(notice the system_reset required after loading the image to initialise the PC
SP)
regards,
Liviu
[Qemu-devel] [PATCH v3 0/4] Add TriCore RCPW, RCRR, RCRW, RLC and RCR instructions
Hi, this patch depends on the previous TriCore patches (https://patchwork.ozlabs.org/patch/405459/) and will hopefully end up in 2.3 QEMU. Other than adding the RCPW, RCRR, RCRW, RLC and RCR instructions, it cleans up how ISA versions in the feature bitmask are handled, to simplify the checks, when instructions are available. Thanks, Bastian v2 - v3: - madd/msub and maddu/msubu now use 64 bit arithmetic instead of 128 bit. - helper madd64_ssov/suov and msub64_ssov/suov now use 64 bit arithmetic for the mul. - cleaned up double setting of PSW_USB_V/SV in helper_msub64_suov. Bastian Koppelmann (4): target-tricore: Make TRICORE_FEATURES implying others. target-tricore: Add instructions of RCPW, RCRR and RCRW opcode format target-tricore: Add instructions of RLC opcode format target-tricore: Add instructions of RCR opcode format target-tricore/cpu.c | 9 + target-tricore/csfr.def | 124 +++ target-tricore/helper.h | 11 + target-tricore/op_helper.c | 202 +++ target-tricore/translate.c | 730 ++- target-tricore/tricore-opcodes.h | 4 +- 6 files changed, 1073 insertions(+), 7 deletions(-) create mode 100644 target-tricore/csfr.def -- 2.1.3
[Qemu-devel] [PATCH v3 3/4] target-tricore: Add instructions of RLC opcode format
Add instructions of RLC opcode format. Add helper psw_write/read. Add microcode generator gen_mtcr/mfcr, which loads/stores a value to a core special function register, which are defined in csfr.def Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de Reviewed-by: Richard Henderson r...@twiddle.net --- target-tricore/csfr.def | 124 +++ target-tricore/helper.h | 3 + target-tricore/op_helper.c | 11 target-tricore/translate.c | 113 +++ target-tricore/tricore-opcodes.h | 1 + 5 files changed, 252 insertions(+) create mode 100644 target-tricore/csfr.def diff --git a/target-tricore/csfr.def b/target-tricore/csfr.def new file mode 100644 index 000..5b219b4 --- /dev/null +++ b/target-tricore/csfr.def @@ -0,0 +1,124 @@ +/* A(ll) access permited + R(ead only) access + E(nd init protected) access + + A|R|E(offset, register, feature introducing reg) + + NOTE: PSW is handled as a special case in gen_mtcr/mfcr */ + +A(0xfe00, PCXI, TRICORE_FEATURE_13) +A(0xfe08, PC, TRICORE_FEATURE_13) +A(0xfe14, SYSCON, TRICORE_FEATURE_13) +R(0xfe18, CPU_ID, TRICORE_FEATURE_13) +E(0xfe20, BIV, TRICORE_FEATURE_13) +E(0xfe24, BTV, TRICORE_FEATURE_13) +E(0xfe28, ISP, TRICORE_FEATURE_13) +A(0xfe2c, ICR, TRICORE_FEATURE_13) +A(0xfe38, FCX, TRICORE_FEATURE_13) +A(0xfe3c, LCX, TRICORE_FEATURE_13) +E(0x9400, COMPAT, TRICORE_FEATURE_131) +/* memory protection register */ +A(0xC000, DPR0_0L, TRICORE_FEATURE_13) +A(0xC004, DPR0_0U, TRICORE_FEATURE_13) +A(0xC008, DPR0_1L, TRICORE_FEATURE_13) +A(0xC00C, DPR0_1U, TRICORE_FEATURE_13) +A(0xC010, DPR0_2L, TRICORE_FEATURE_13) +A(0xC014, DPR0_2U, TRICORE_FEATURE_13) +A(0xC018, DPR0_3L, TRICORE_FEATURE_13) +A(0xC01C, DPR0_3U, TRICORE_FEATURE_13) +A(0xC400, DPR1_0L, TRICORE_FEATURE_13) +A(0xC404, DPR1_0U, TRICORE_FEATURE_13) +A(0xC408, DPR1_1L, TRICORE_FEATURE_13) +A(0xC40C, DPR1_1U, TRICORE_FEATURE_13) +A(0xC410, DPR1_2L, TRICORE_FEATURE_13) +A(0xC414, DPR1_2U, TRICORE_FEATURE_13) +A(0xC418, DPR1_3L, TRICORE_FEATURE_13) +A(0xC41C, DPR1_3U, TRICORE_FEATURE_13) +A(0xC800, DPR2_0L, TRICORE_FEATURE_13) +A(0xC804, DPR2_0U, TRICORE_FEATURE_13) +A(0xC808, DPR2_1L, TRICORE_FEATURE_13) +A(0xC80C, DPR2_1U, TRICORE_FEATURE_13) +A(0xC810, DPR2_2L, TRICORE_FEATURE_13) +A(0xC814, DPR2_2U, TRICORE_FEATURE_13) +A(0xC818, DPR2_3L, TRICORE_FEATURE_13) +A(0xC81C, DPR2_3U, TRICORE_FEATURE_13) +A(0xCC00, DPR3_0L, TRICORE_FEATURE_13) +A(0xCC04, DPR3_0U, TRICORE_FEATURE_13) +A(0xCC08, DPR3_1L, TRICORE_FEATURE_13) +A(0xCC0C, DPR3_1U, TRICORE_FEATURE_13) +A(0xCC10, DPR3_2L, TRICORE_FEATURE_13) +A(0xCC14, DPR3_2U, TRICORE_FEATURE_13) +A(0xCC18, DPR3_3L, TRICORE_FEATURE_13) +A(0xCC1C, DPR3_3U, TRICORE_FEATURE_13) +A(0xD000, CPR0_0L, TRICORE_FEATURE_13) +A(0xD004, CPR0_0U, TRICORE_FEATURE_13) +A(0xD008, CPR0_1L, TRICORE_FEATURE_13) +A(0xD00C, CPR0_1U, TRICORE_FEATURE_13) +A(0xD010, CPR0_2L, TRICORE_FEATURE_13) +A(0xD014, CPR0_2U, TRICORE_FEATURE_13) +A(0xD018, CPR0_3L, TRICORE_FEATURE_13) +A(0xD01C, CPR0_3U, TRICORE_FEATURE_13) +A(0xD400, CPR1_0L, TRICORE_FEATURE_13) +A(0xD404, CPR1_0U, TRICORE_FEATURE_13) +A(0xD408, CPR1_1L, TRICORE_FEATURE_13) +A(0xD40C, CPR1_1U, TRICORE_FEATURE_13) +A(0xD410, CPR1_2L, TRICORE_FEATURE_13) +A(0xD414, CPR1_2U, TRICORE_FEATURE_13) +A(0xD418, CPR1_3L, TRICORE_FEATURE_13) +A(0xD41C, CPR1_3U, TRICORE_FEATURE_13) +A(0xD800, CPR2_0L, TRICORE_FEATURE_13) +A(0xD804, CPR2_0U, TRICORE_FEATURE_13) +A(0xD808, CPR2_1L, TRICORE_FEATURE_13) +A(0xD80C, CPR2_1U, TRICORE_FEATURE_13) +A(0xD810, CPR2_2L, TRICORE_FEATURE_13) +A(0xD814, CPR2_2U, TRICORE_FEATURE_13) +A(0xD818, CPR2_3L, TRICORE_FEATURE_13) +A(0xD81C, CPR2_3U, TRICORE_FEATURE_13) +A(0xDC00, CPR3_0L, TRICORE_FEATURE_13) +A(0xDC04, CPR3_0U, TRICORE_FEATURE_13) +A(0xDC08, CPR3_1L, TRICORE_FEATURE_13) +A(0xDC0C, CPR3_1U, TRICORE_FEATURE_13) +A(0xDC10, CPR3_2L, TRICORE_FEATURE_13) +A(0xDC14, CPR3_2U, TRICORE_FEATURE_13) +A(0xDC18, CPR3_3L, TRICORE_FEATURE_13) +A(0xDC1C, CPR3_3U, TRICORE_FEATURE_13) +A(0xE000, DPM0, TRICORE_FEATURE_13) +A(0xE080, DPM1, TRICORE_FEATURE_13) +A(0xE100, DPM2, TRICORE_FEATURE_13) +A(0xE180, DPM3, TRICORE_FEATURE_13) +A(0xE200, CPM0, TRICORE_FEATURE_13) +A(0xE280, CPM1, TRICORE_FEATURE_13) +A(0xE300, CPM2, TRICORE_FEATURE_13) +A(0xE380, CPM3, TRICORE_FEATURE_13) +/* memory Managment Registers */ +A(0x8000, MMU_CON, TRICORE_FEATURE_13) +A(0x8004, MMU_ASI, TRICORE_FEATURE_13) +A(0x800C, MMU_TVA, TRICORE_FEATURE_13) +A(0x8010, MMU_TPA, TRICORE_FEATURE_13) +A(0x8014, MMU_TPX, TRICORE_FEATURE_13) +A(0x8018, MMU_TFA, TRICORE_FEATURE_13) +E(0x9004, BMACON, TRICORE_FEATURE_131) +E(0x900C, SMACON, TRICORE_FEATURE_131) +A(0x9020, DIEAR, TRICORE_FEATURE_131) +A(0x9024, DIETR, TRICORE_FEATURE_131) +A(0x9028, CCDIER, TRICORE_FEATURE_131) +E(0x9044, MIECON, TRICORE_FEATURE_131) +A(0x9210, PIEAR, TRICORE_FEATURE_131) +A(0x9214, PIETR, TRICORE_FEATURE_131) +A(0x9218, CCPIER, TRICORE_FEATURE_131) +/* debug
[Qemu-devel] [PATCH v3 4/4] target-tricore: Add instructions of RCR opcode format
Add instructions of RCR opcode format.
Add helper for madd32/64_ssov and madd32/64_suov.
Add helper for msub32/64_ssov and msub32/64_suov.
Add microcode generator function madd/msub for 32bit and 64bit, which calculate
a mul and a add/sub.
OPC2_32_RCR_MSUB_U_32 - OPC2_32_RCR_MSUB_U_32.
Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
---
v2 - v3:
- madd/msub and maddu/msubu now use 64 bit arithmetic instead of 128 bit.
- helper madd64_ssov/suov and msub64_ssov/suov now use 64 bit arithmetic
for the mul.
- cleaned up double setting of PSW_USB_V/SV in helper_msub64_suov.
target-tricore/helper.h | 8 +
target-tricore/op_helper.c | 191
target-tricore/translate.c | 479 +++
target-tricore/tricore-opcodes.h | 3 +-
4 files changed, 680 insertions(+), 1 deletion(-)
diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index 2eb33ea..6c07bd7 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -24,6 +24,14 @@ DEF_HELPER_3(mul_ssov, i32, env, i32, i32)
DEF_HELPER_3(mul_suov, i32, env, i32, i32)
DEF_HELPER_3(sha_ssov, i32, env, i32, i32)
DEF_HELPER_3(absdif_ssov, i32, env, i32, i32)
+DEF_HELPER_4(madd32_ssov, i32, env, i32, i32, i32)
+DEF_HELPER_4(madd32_suov, i32, env, i32, i32, i32)
+DEF_HELPER_4(madd64_ssov, i64, env, i32, i64, i32)
+DEF_HELPER_4(madd64_suov, i64, env, i32, i64, i32)
+DEF_HELPER_4(msub32_ssov, i32, env, i32, i32, i32)
+DEF_HELPER_4(msub32_suov, i32, env, i32, i32, i32)
+DEF_HELPER_4(msub64_ssov, i64, env, i32, i64, i32)
+DEF_HELPER_4(msub64_suov, i64, env, i32, i64, i32)
/* CSA */
DEF_HELPER_2(call, void, env, i32)
DEF_HELPER_1(ret, void, env)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 0b6b471..434839f 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -56,6 +56,16 @@ uint32_t helper_circ_update(uint32_t reg, uint32_t off)
return reg - index + new_index;
}
+static void add128(uint64_t *plow, uint64_t *phigh, uint64_t a, uint64_t b)
+{
+*plow += a;
+/* carry test */
+if (*plow a) {
+(*phigh)++;
+}
+*phigh += b;
+}
+
#define SSOV(env, ret, arg, len) do { \
int64_t max_pos = INT##len ##_MAX; \
int64_t max_neg = INT##len ##_MIN; \
@@ -198,6 +208,187 @@ target_ulong helper_absdif_ssov(CPUTriCoreState *env,
target_ulong r1,
SSOV(env, ret, result, 32);
return ret;
}
+
+target_ulong helper_madd32_ssov(CPUTriCoreState *env, target_ulong r1,
+target_ulong r2, target_ulong r3)
+{
+target_ulong ret;
+int64_t t1 = sextract64(r1, 0, 32);
+int64_t t2 = sextract64(r2, 0, 32);
+int64_t t3 = sextract64(r3, 0, 32);
+int64_t result;
+
+result = t2 + (t1 * t3);
+SSOV(env, ret, result, 32);
+return ret;
+}
+
+target_ulong helper_madd32_suov(CPUTriCoreState *env, target_ulong r1,
+target_ulong r2, target_ulong r3)
+{
+target_ulong ret;
+uint64_t t1 = extract64(r1, 0, 32);
+uint64_t t2 = extract64(r2, 0, 32);
+uint64_t t3 = extract64(r3, 0, 32);
+int64_t result;
+
+result = t2 + (t1 * t3);
+SUOV(env, ret, result, 32);
+return ret;
+}
+
+uint64_t helper_madd64_ssov(CPUTriCoreState *env, target_ulong r1,
+uint64_t r2, target_ulong r3)
+{
+uint64_t ret_low, ret_high;
+uint64_t r2_high;
+int64_t t1 = sextract64(r1, 0, 32);
+int64_t t3 = sextract64(r3, 0, 32);
+
+ret_low = t1 * t3;
+ret_high = ((int64_t)ret_low 63);
+r2_high = ((int64_t)r2 63);
+add128(ret_low, ret_high, r2, r2_high);
+
+/* check for saturate */
+t1 = (int64_t)ret_low 63;
+if (t1 != ret_high) {
+env-PSW_USB_V = (1 31);
+env-PSW_USB_SV = (1 31);
+if (t1 == 0x0) {
+ret_low = INT64_MIN;
+} else {
+ret_low = INT64_MAX;
+}
+} else {
+env-PSW_USB_V = 0;
+}
+t1 = ret_low 32;
+env-PSW_USB_AV = t1 ^ t1 * 2u;
+env-PSW_USB_SAV |= env-PSW_USB_AV;
+
+return ret_low;
+}
+
+uint64_t helper_madd64_suov(CPUTriCoreState *env, target_ulong r1,
+uint64_t r2, target_ulong r3)
+{
+uint64_t ret_low, ret_high;
+uint64_t t1 = extract64(r1, 0, 32);
+uint64_t t3 = extract64(r3, 0, 32);
+
+ret_low = t1 * t3;
+ret_high = 0;
+add128(ret_low, ret_high, r2, 0);
+
+if (ret_high != 0) {
+env-PSW_USB_V = (1 31);
+env-PSW_USB_SV = (1 31);
+ret_low = UINT64_MAX;
+} else if ((ret_high (1LL 63)) != 0) {
+ret_low = 0;
+env-PSW_USB_V = (1 31);
+env-PSW_USB_SV = (1 31);
+} else {
+env-PSW_USB_V = 0;
+}
+t1 = ret_low 32;
+env-PSW_USB_AV = t1 ^ t1 * 2u;
+env-PSW_USB_SAV |= env-PSW_USB_AV;
+return ret_low;
+}
+
+target_ulong helper_msub32_ssov(CPUTriCoreState *env,
[Qemu-devel] [PATCH v3 1/4] target-tricore: Make TRICORE_FEATURES implying others.
Since all the TriCore instructionsets are subsets of each other (1.3 C 1.3.1 C
1.6),
make the features implying each other, e.g 1.6 also has 1.3.1 and 1.3. This way
we only need to check our features for the instructionset, where a instruction
was first introduced.
Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
Reviewed-by: Richard Henderson r...@twiddle.net
---
target-tricore/cpu.c | 9 +
target-tricore/translate.c | 6 +++---
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
index 7bf041a..abe16fa 100644
--- a/target-tricore/cpu.c
+++ b/target-tricore/cpu.c
@@ -63,8 +63,17 @@ static bool tricore_cpu_has_work(CPUState *cs)
static void tricore_cpu_realizefn(DeviceState *dev, Error **errp)
{
CPUState *cs = CPU(dev);
+TriCoreCPU *cpu = TRICORE_CPU(dev);
TriCoreCPUClass *tcc = TRICORE_CPU_GET_CLASS(dev);
+CPUTriCoreState *env = cpu-env;
+/* Some features automatically imply others */
+if (tricore_feature(env, TRICORE_FEATURE_16)) {
+set_feature(env, TRICORE_FEATURE_131);
+}
+if (tricore_feature(env, TRICORE_FEATURE_131)) {
+set_feature(env, TRICORE_FEATURE_13);
+}
cpu_reset(cs);
qemu_init_vcpu(cs);
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 1daf26d..3775374 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -2206,17 +2206,17 @@ static void
decode_bo_addrmode_post_pre_base(CPUTriCoreState *env,
case OPC2_32_BO_CACHEI_WI_SHORTOFF:
case OPC2_32_BO_CACHEI_W_SHORTOFF:
/* TODO: Raise illegal opcode trap,
- if tricore_feature(TRICORE_FEATURE_13) */
+ if !tricore_feature(TRICORE_FEATURE_131) */
break;
case OPC2_32_BO_CACHEI_W_POSTINC:
case OPC2_32_BO_CACHEI_WI_POSTINC:
-if (!tricore_feature(env, TRICORE_FEATURE_13)) {
+if (tricore_feature(env, TRICORE_FEATURE_131)) {
tcg_gen_addi_tl(cpu_gpr_d[r2], cpu_gpr_d[r2], off10);
} /* TODO: else raise illegal opcode trap */
break;
case OPC2_32_BO_CACHEI_W_PREINC:
case OPC2_32_BO_CACHEI_WI_PREINC:
-if (!tricore_feature(env, TRICORE_FEATURE_13)) {
+if (tricore_feature(env, TRICORE_FEATURE_131)) {
tcg_gen_addi_tl(cpu_gpr_d[r2], cpu_gpr_d[r2], off10);
} /* TODO: else raise illegal opcode trap */
break;
--
2.1.3
[Qemu-devel] [PATCH v3 2/4] target-tricore: Add instructions of RCPW, RCRR and RCRW opcode format
Add instructions of RCPW, RCRR and RCRW opcode format.
Add microcode generator function gen_insert.
Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
Reviewed-by: Richard Henderson r...@twiddle.net
---
target-tricore/translate.c | 132 +++--
1 file changed, 129 insertions(+), 3 deletions(-)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 3775374..689596f 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -869,7 +869,28 @@ static inline void gen_eqany_hi(TCGv ret, TCGv r1, int32_t
con)
tcg_temp_free(h0);
tcg_temp_free(h1);
}
+/* mask = ((1 width) -1) pos;
+ ret = (r1 ~mask) | (r2 pos) mask); */
+static inline void gen_insert(TCGv ret, TCGv r1, TCGv r2, TCGv width, TCGv pos)
+{
+TCGv mask = tcg_temp_new();
+TCGv temp = tcg_temp_new();
+TCGv temp2 = tcg_temp_new();
+tcg_gen_movi_tl(mask, 1);
+tcg_gen_shl_tl(mask, mask, width);
+tcg_gen_subi_tl(mask, mask, 1);
+tcg_gen_shl_tl(mask, mask, pos);
+
+tcg_gen_shl_tl(temp, r2, pos);
+tcg_gen_and_tl(temp, temp, mask);
+tcg_gen_andc_tl(temp2, r1, mask);
+tcg_gen_or_tl(ret, temp, temp2);
+
+tcg_temp_free(mask);
+tcg_temp_free(temp);
+tcg_temp_free(temp2);
+}
/* helpers for generating program flow micro-ops */
@@ -3128,14 +3149,92 @@ static void decode_rc_mul(CPUTriCoreState *env,
DisasContext *ctx)
}
}
+/* RCPW format */
+static void decode_rcpw_insert(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1, r2;
+int32_t pos, width, const4;
+
+TCGv temp;
+
+op2= MASK_OP_RCPW_OP2(ctx-opcode);
+r1 = MASK_OP_RCPW_S1(ctx-opcode);
+r2 = MASK_OP_RCPW_D(ctx-opcode);
+const4 = MASK_OP_RCPW_CONST4(ctx-opcode);
+width = MASK_OP_RCPW_WIDTH(ctx-opcode);
+pos= MASK_OP_RCPW_POS(ctx-opcode);
+
+switch (op2) {
+case OPC2_32_RCPW_IMASK:
+/* if pos + width 31 undefined result */
+if (pos + width = 31) {
+tcg_gen_movi_tl(cpu_gpr_d[r2+1], ((1u width) - 1) pos);
+tcg_gen_movi_tl(cpu_gpr_d[r2], (const4 pos));
+}
+break;
+case OPC2_32_RCPW_INSERT:
+/* if pos + width 32 undefined result */
+if (pos + width = 32) {
+temp = tcg_const_i32(const4);
+tcg_gen_deposit_tl(cpu_gpr_d[r2], cpu_gpr_d[r1], temp, pos, width);
+tcg_temp_free(temp);
+}
+break;
+}
+}
+
+/* RCRW format */
+
+static void decode_rcrw_insert(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1, r3, r4;
+int32_t width, const4;
+
+TCGv temp, temp2, temp3;
+
+op2= MASK_OP_RCRW_OP2(ctx-opcode);
+r1 = MASK_OP_RCRW_S1(ctx-opcode);
+r3 = MASK_OP_RCRW_S3(ctx-opcode);
+r4 = MASK_OP_RCRW_D(ctx-opcode);
+width = MASK_OP_RCRW_WIDTH(ctx-opcode);
+const4 = MASK_OP_RCRW_CONST4(ctx-opcode);
+
+temp = tcg_temp_new();
+temp2 = tcg_temp_new();
+
+switch (op2) {
+case OPC2_32_RCRW_IMASK:
+tcg_gen_andi_tl(temp, cpu_gpr_d[r4], 0x1f);
+tcg_gen_movi_tl(temp2, (1 width) - 1);
+tcg_gen_shl_tl(cpu_gpr_d[r3 + 1], temp2, temp);
+tcg_gen_movi_tl(temp2, const4);
+tcg_gen_shl_tl(cpu_gpr_d[r3], temp2, temp);
+break;
+case OPC2_32_RCRW_INSERT:
+temp3 = tcg_temp_new();
+
+tcg_gen_movi_tl(temp, width);
+tcg_gen_movi_tl(temp2, const4);
+tcg_gen_andi_tl(temp3, cpu_gpr_d[r4], 0x1f);
+gen_insert(cpu_gpr_d[r3], cpu_gpr_d[r1], temp2, temp, temp3);
+
+tcg_temp_free(temp3);
+break;
+}
+tcg_temp_free(temp);
+tcg_temp_free(temp2);
+}
+
static void decode_32Bit_opc(CPUTriCoreState *env, DisasContext *ctx)
{
int op1;
-int32_t r1, r2;
-int32_t address;
+int32_t r1, r2, r3;
+int32_t address, const16;
int8_t b, const4;
int32_t bpos;
-TCGv temp, temp2;
+TCGv temp, temp2, temp3;
op1 = MASK_OP_MAJOR(ctx-opcode);
@@ -3309,6 +3408,33 @@ static void decode_32Bit_opc(CPUTriCoreState *env,
DisasContext *ctx)
case OPCM_32_RC_MUL:
decode_rc_mul(env, ctx);
break;
+/* RCPW Format */
+case OPCM_32_RCPW_MASK_INSERT:
+decode_rcpw_insert(env, ctx);
+break;
+/* RCRR Format */
+case OPC1_32_RCRR_INSERT:
+r1 = MASK_OP_RCRR_S1(ctx-opcode);
+r2 = MASK_OP_RCRR_S3(ctx-opcode);
+r3 = MASK_OP_RCRR_D(ctx-opcode);
+const16 = MASK_OP_RCRR_CONST4(ctx-opcode);
+temp = tcg_const_i32(const16);
+temp2 = tcg_temp_new(); /* width*/
+temp3 = tcg_temp_new(); /* pos */
+
+tcg_gen_andi_tl(temp2, cpu_gpr_d[r3+1], 0x1f);
+tcg_gen_andi_tl(temp3, cpu_gpr_d[r3], 0x1f);
+
+gen_insert(cpu_gpr_d[r2], cpu_gpr_d[r1], temp, temp2, temp3);
+
+tcg_temp_free(temp);
+tcg_temp_free(temp2);
+tcg_temp_free(temp3);
+
Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration
On (Thu) 20 Nov 2014 [19:39:11], Gonglei wrote: The static variables in migration_bitmap_sync will not be reset in the case of a second attempted migration. Signed-off-by: ChenLiang chenlian...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com Good catch. Applied.. Hi, Juan? Ping... please :) Juan, what happened to this patch? Nearly for half a year, I forgot it completely :( Thanks for your prompt, Paolo. Yeah; unfortunate. I feel the patch could've been done in a better way, but since it's been a while... Paolo, Dave, can you please give me r-b and I'll pick it up for 2.2. Thanks, Amit
Re: [Qemu-devel] [PATCH] armv7m: optional -kernel if -gdb present
On 20 November 2014 12:05, Liviu Ionescu i...@livius.net wrote:
For standalone emulation, the image must be specified via -kernel,
but when using QEMU as a GDB server, the presence of -kernel is
no longer mandatory, since the image can be loaded by the GDB client.
I think the correct fix for this issue is:
-if (!kernel_filename !qtest_enabled()) {
+if (!kernel_filename !qtest_enabled() !with_gdb) {
fprintf(stderr, Guest image must be specified (using -kernel)\n);
exit(1);
}
just delete this entire if() statement. This is how we've handled
similar issues with the ARM A profile boards. (Some of the boards
still have those checks but that's just because nobody's bothered
to fix them yet.)
thanks
-- PMM
Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration
* Paolo Bonzini (pbonz...@redhat.com) wrote: On 24/06/2014 08:23, Gonglei (Arei) wrote: -Original Message- From: Juan Quintela [mailto:quint...@redhat.com] Sent: Friday, March 21, 2014 9:26 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; owass...@redhat.com; pbonz...@redhat.com; ebl...@redhat.com; dgilb...@redhat.com; chenliang (T) Subject: Re: [PATCH] migration: static variables will not be reset at second migration arei.gong...@huawei.com wrote: From: ChenLiang chenlian...@huawei.com The static variables in migration_bitmap_sync will not be reset in the case of a second attempted migration. Signed-off-by: ChenLiang chenlian...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com Good catch. Applied.. Hi, Juan? Ping... please :) Juan, what happened to this patch? I think we should put this in now; it's obvious it was intended to go in. Dave Paolo -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] armv7m: optional -kernel if -gdb present
On 20 Nov 2014, at 14:29, Peter Maydell peter.mayd...@linaro.org wrote:
-if (!kernel_filename !qtest_enabled()) {
+if (!kernel_filename !qtest_enabled() !with_gdb) {
fprintf(stderr, Guest image must be specified (using -kernel)\n);
exit(1);
}
just delete this entire if() statement. This is how we've handled
similar issues with the ARM A profile boards. (Some of the boards
still have those checks but that's just because nobody's bothered
to fix them yet.)
I'm a bit confused. if not running with gdb, what is the expected behaviour if
the image is missing?
Liviu
Re: [Qemu-devel] [PATCH] i386/helper: add cpu dump APIC information
On 22/07/2014 05:00, Chen Fan wrote: When KVM exit reason is KVM_EXIT_SHUTDOWN, there will cause guest to reset, but we can't get any information to fix. we knew KVM handle triple fault will set exit_reason to KVM_EXIT_SHUTDOWN, so we also should dump the APIC information to help to fix. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com Hi, old (16-bit) guests will triple fault in order to get out of protected mode, so it's not possible to dump anything when you get KVM_EXIT_SHUTDOWN. :( Paolo
Re: [Qemu-devel] [PATCH v2 for-2.2 0/4] net: fix high impact outstanding defects reported by Coverity
Reviewed-by: Paolo Bonzini pbonz...@redhat.com Thanks! Paolo On 20/11/2014 12:34, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com Please see details in every patch. v2 - v1: - rewrite patch 3 and patch 4 by Paolo's suggestion. Thanks. - add Jason's R-b tag in patch 1~3. Thanks too. Cc: Paolo Bonzini pbonz...@redhat.com Cc: Stefan Hajnoczi stefa...@redhat.com Cc: Jason Wang jasow...@redhat.com Gonglei (4): net/slirp: fix memory leak net/socket: fix Uninitialized scalar variable pcnet: fix Negative array index read rtl8139: fix Pointer to local outside scope hw/net/pcnet.c | 55 ++- hw/net/rtl8139.c | 4 net/slirp.c | 3 +-- net/socket.c | 11 ++- 4 files changed, 41 insertions(+), 32 deletions(-)
Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration
* Amit Shah (amit.s...@redhat.com) wrote: On (Thu) 20 Nov 2014 [19:39:11], Gonglei wrote: The static variables in migration_bitmap_sync will not be reset in the case of a second attempted migration. Signed-off-by: ChenLiang chenlian...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com Good catch. Applied.. Hi, Juan? Ping... please :) Juan, what happened to this patch? Nearly for half a year, I forgot it completely :( Thanks for your prompt, Paolo. Yeah; unfortunate. I feel the patch could've been done in a better way, but since it's been a while... Paolo, Dave, can you please give me r-b and I'll pick it up for 2.2. Reviewed-by: Dr. David Alan Gilbert dgilb...@redhat.com Note that there are some more statics that have ended up in migration_bitmap_sync since that patch landed, but still it would be good to get this fix going. Dave Thanks, Amit -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()
On 2014/11/19 15:47, Fam Zheng wrote:
On Tue, 11/04 20:47, Shannon Zhao wrote:
set_host_notifier() is introduced into virtio-mmio now. Most of codes came
from virtio-pci.
Signed-off-by: Ying-Shiuan Pan yingshiuan@gmail.com
Signed-off-by: Li Liu john.li...@huawei.com
Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com
---
hw/virtio/virtio-mmio.c | 70
+++
1 files changed, 70 insertions(+), 0 deletions(-)
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 2450c13..d8ec2d1 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -23,6 +23,7 @@
#include hw/virtio/virtio.h
#include qemu/host-utils.h
#include hw/virtio/virtio-bus.h
+#include qemu/error-report.h
/* #define DEBUG_VIRTIO_MMIO */
@@ -87,8 +88,58 @@ typedef struct {
uint32_t guest_page_shift;
/* virtio-bus */
VirtioBusState bus;
+bool ioeventfd_disabled;
+bool ioeventfd_started;
} VirtIOMMIOProxy;
+static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
+ int n, bool assign, bool
set_handler)
I didn't review the code, but checkpatch.pl noticed this line and one more
below [*] is too long (over 80 columes).
+{
+VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
+VirtQueue *vq = virtio_get_queue(vdev, n);
+EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
+int r = 0;
+
+if (assign) {
+r = event_notifier_init(notifier, 1);
+if (r 0) {
+error_report(%s: unable to init event notifier: %d,
+ __func__, r);
+return r;
+}
+virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
+memory_region_add_eventfd(proxy-iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+ true, n, notifier);
+} else {
+memory_region_del_eventfd(proxy-iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+ true, n, notifier);
+virtio_queue_set_host_notifier_fd_handler(vq, false, false);
+event_notifier_cleanup(notifier);
+}
+return r;
+}
+
+static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+int r;
+int n;
+VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
+
+if (!proxy-ioeventfd_started) {
+return;
+}
+
+for (n = 0; n VIRTIO_PCI_QUEUE_MAX; n++) {
+if (!virtio_queue_get_num(vdev, n)) {
+continue;
+}
+
+r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
+assert(r = 0);
+}
+proxy-ioeventfd_started = false;
+}
+
static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
{
VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
@@ -342,6 +393,24 @@ static void virtio_mmio_reset(DeviceState *d)
proxy-guest_page_shift = 0;
}
+static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool
assign)
[*]
No need to respin yet just for this. Please wait for a serious review.
Ok,thanks,
Shannon
Thanks,
Fam
.
--
Shannon
[Qemu-devel] [PATCH 3/3] hmp: Expose read-only option for 'change'
Expose the new read-only option of qmp_change_blockdev() for the
'change' HMP command.
Signed-off-by: Max Reitz mre...@redhat.com
---
hmp-commands.hx | 24 +---
hmp.c | 17 -
2 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e37bc8b..f0ec0da 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -196,8 +196,8 @@ ETEXI
{
.name = change,
-.args_type = device:B,target:F,arg:s?,
-.params = device filename [format],
+.args_type = device:B,target:F,arg:s?,read-only:s?,
+.params = device filename [format [read-only]],
.help = change a removable medium, optional format,
.mhandler.cmd = hmp_change,
},
@@ -209,7 +209,7 @@ STEXI
Change the configuration of a device.
@table @option
-@item change @var{diskdevice} @var{filename} [@var{format}]
+@item change @var{diskdevice} @var{filename} [@var{format} [@var{read-only}]]
Change the medium for a removable disk device to point to @var{filename}. eg
@example
@@ -218,6 +218,24 @@ Change the medium for a removable disk device to point to
@var{filename}. eg
@var{format} is optional.
+@var{read-only} may be used to change the read-only status of the device. It
+accepts the following values:
+
+@table @var
+@item retain
+Retains the current status; this is the default.
+
+@item ro
+Makes the device read-only.
+
+@item rw
+Makes the device writable.
+
+@item auto
+If @var{filename} can be opened with write access, the block device will be
+writable; otherwise it will be read-only.
+@end table
+
@item change vnc @var{display},@var{options}
Change the configuration of the VNC server. The valid syntax for @var{display}
and @var{options} are described at @ref{sec_invocation}. eg
diff --git a/hmp.c b/hmp.c
index 0719fa3..c25e7dc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -23,6 +23,7 @@
#include monitor/monitor.h
#include qapi/opts-visitor.h
#include qapi/string-output-visitor.h
+#include qapi/util.h
#include qapi-visit.h
#include ui/console.h
#include block/qapi.h
@@ -1122,6 +1123,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
const char *device = qdict_get_str(qdict, device);
const char *target = qdict_get_str(qdict, target);
const char *arg = qdict_get_try_str(qdict, arg);
+const char *read_only = qdict_get_try_str(qdict, read-only);
+BlockdevChangeReadOnlyMode read_only_mode = 0;
Error *err = NULL;
if (strcmp(device, vnc) == 0
@@ -1133,7 +1136,19 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
}
-qmp_change(device, target, !!arg, arg, false, 0, err);
+if (read_only) {
+read_only_mode = qapi_enum_parse(BlockdevChangeReadOnlyMode_lookup,
+ read_only,
+ BLOCKDEV_CHANGE_READ_ONLY_MODE_MAX,
+ BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN,
+ err);
+if (err) {
+hmp_handle_error(mon, err);
+return;
+}
+}
+
+qmp_change(device, target, !!arg, arg, !!read_only, read_only_mode, err);
if (err
error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
error_free(err);
--
1.9.3
[Qemu-devel] [PATCH 0/3] blockdev: Add read-only option to change-blockdev
The 'change' QMP and HMP command allows replacing the medium in drives which support this, e.g. floppy disk drives. For some drives, the medium carries information about whether it can be written to or not (again, floppy drives). Therefore, it should be possible to change the read-only state of block devices when changing the loaded medium. This series adds an optional additional parameter to the 'change' QMP and HMP command which allows changing the read-only state in four ways: - 'retain': Just keep the status as it was before; this is the current behavior and thus this will be the default. - 'ro': Force read-only access - 'rw': Force writable access - 'auto': This opens the new file R/W first, if that fails, the file is opened read-only. Max Reitz (3): blockdev: Add read-only option to change-blockdev qmp: Expose read-only option for 'change' hmp: Expose read-only option for 'change' blockdev.c| 41 ++--- hmp-commands.hx | 24 +--- hmp.c | 17 - include/sysemu/blockdev.h | 3 ++- qapi-schema.json | 27 ++- qmp-commands.hx | 24 +++- qmp.c | 14 -- 7 files changed, 138 insertions(+), 12 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 1/3] blockdev: Add read-only option to change-blockdev
Add an option to qmp_change_blockdev() which allows changing the
read-only status of the block device to be changed.
Some drives do not have a inherently fixed read-only status; for
instance, floppy disks can be set read-only or writable independently of
the drive. Some users may find it useful to be able to therefore change
the read-only status of a block device when changing the medium.
Signed-off-by: Max Reitz mre...@redhat.com
---
blockdev.c| 41 ++---
include/sysemu/blockdev.h | 3 ++-
qapi-schema.json | 20
qmp.c | 3 ++-
4 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a52f205..4140a27 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1721,7 +1721,8 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs,
const char *filename,
}
void qmp_change_blockdev(const char *device, const char *filename,
- const char *format, Error **errp)
+ const char *format,
+ BlockdevChangeReadOnlyMode read_only, Error **errp)
{
BlockBackend *blk;
BlockDriverState *bs;
@@ -1754,10 +1755,44 @@ void qmp_change_blockdev(const char *device, const char
*filename,
goto out;
}
-bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
+switch (read_only) {
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN:
+bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_RO:
+bdrv_flags = 0;
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_RW:
+bdrv_flags = BDRV_O_RDWR;
+break;
+
+case BLOCKDEV_CHANGE_READ_ONLY_MODE_AUTO:
+/* try RDWR first */
+bdrv_flags = BDRV_O_RDWR;
+break;
+
+default:
+abort();
+}
+
bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
-qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
+qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, err);
+
+if (err) {
+if (read_only == BLOCKDEV_CHANGE_READ_ONLY_MODE_AUTO) {
+error_free(err);
+err = NULL;
+
+/* RDWR did not work, try RO now */
+bdrv_flags = ~BDRV_O_RDWR;
+qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
+} else {
+error_propagate(errp, err);
+}
+}
out:
aio_context_release(aio_context);
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 09d1e30..14b4dfb 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -66,7 +66,8 @@ DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType
block_default_type);
DriveInfo *add_init_drive(const char *opts);
void qmp_change_blockdev(const char *device, const char *filename,
- const char *format, Error **errp);
+ const char *format,
+ BlockdevChangeReadOnlyMode read_only, Error **errp);
void do_commit(Monitor *mon, const QDict *qdict);
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index d0926d9..441e001 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1543,6 +1543,26 @@
{ 'command': 'change-vnc-password', 'data': {'password': 'str'} }
##
+# @BlockdevChangeReadOnlyMode:
+#
+# Specifies the new read-only mode of a block device subject to the @change
+# command.
+#
+# @retain: Retains the current read-only mode
+#
+# @ro: Makes the device read-only
+#
+# @rw: Makes the device writable
+#
+# @auto:If the file specified can be opened with write access, the block
+# device will be writable; otherwise it will be read-only
+#
+# Since: 2.3
+##
+{ 'enum': 'BlockdevChangeReadOnlyMode',
+ 'data': ['retain', 'ro', 'rw', 'auto'] }
+
+##
# @change:
#
# This command is multiple commands multiplexed together.
diff --git a/qmp.c b/qmp.c
index 0b4f131..bd63cf4 100644
--- a/qmp.c
+++ b/qmp.c
@@ -402,7 +402,8 @@ void qmp_change(const char *device, const char *target,
if (strcmp(device, vnc) == 0) {
qmp_change_vnc(target, has_arg, arg, errp);
} else {
-qmp_change_blockdev(device, target, arg, errp);
+qmp_change_blockdev(device, target, arg,
+BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, errp);
}
}
--
1.9.3
[Qemu-devel] [PATCH 2/3] qmp: Expose read-only option for 'change'
Expose the new read-only option of qmp_change_blockdev() for the
'change' QMP command. Leave it unset for HMP for now.
Signed-off-by: Max Reitz mre...@redhat.com
---
hmp.c| 2 +-
qapi-schema.json | 7 ++-
qmp-commands.hx | 24 +++-
qmp.c| 15 ---
4 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/hmp.c b/hmp.c
index 94b27df..0719fa3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1133,7 +1133,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
}
-qmp_change(device, target, !!arg, arg, err);
+qmp_change(device, target, !!arg, arg, false, 0, err);
if (err
error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
error_free(err);
diff --git a/qapi-schema.json b/qapi-schema.json
index 441e001..80aaa63 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1581,6 +1581,10 @@
# password to set. If this argument is an empty string, then no
future
# logins will be allowed.
#
+# @read-only: #optional, only valid for block devices. This option allows
+# changing the read-only mode of the block device; defaults to
+# 'retain'. (Since 2.3)
+#
# Returns: Nothing on success.
# If @device is not a valid block device, DeviceNotFound
# If the new block device is encrypted, DeviceEncrypted. Note that
@@ -1595,7 +1599,8 @@
# Since: 0.14.0
##
{ 'command': 'change',
- 'data': {'device': 'str', 'target': 'str', '*arg': 'str'} }
+ 'data': {'device': 'str', 'target': 'str', '*arg': 'str',
+ '*read-only': 'BlockdevChangeReadOnlyMode'} }
##
# @ObjectTypeInfo:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6ef1b28..edc1783 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -109,7 +109,7 @@ EQMP
{
.name = change,
-.args_type = device:B,target:F,arg:s?,
+.args_type = device:B,target:F,arg:s?,read-only:s?,
.mhandler.cmd_new = qmp_marshal_input_change,
},
@@ -124,6 +124,8 @@ Arguments:
- device: device name (json-string)
- target: filename or item (json-string)
- arg: additional argument (json-string, optional)
+- read-only: new read-only mode (json-string, optional)
+ - Possible values: retain (default), ro, rw, auto
Examples:
@@ -141,6 +143,26 @@ Examples:
arg: foobar1 } }
- { return: {} }
+3. Load a read-only medium into a writable drive
+
+- { execute: change,
+ arguments: { device: isa-fd0,
+target: /srv/images/ro.img,
+arg: raw,
+read-only: retain } }
+
+- { error:
+ { class: GenericError,
+ desc: Could not open '/srv/images/ro.img': Permission denied } }
+
+- { execute: change,
+ arguments: { device: isa-fd0,
+target: /srv/images/ro.img,
+arg: raw,
+read-only: auto } }
+
+- { return: {} }
+
EQMP
{
diff --git a/qmp.c b/qmp.c
index bd63cf4..d12035b 100644
--- a/qmp.c
+++ b/qmp.c
@@ -397,13 +397,22 @@ static void qmp_change_vnc(const char *target, bool
has_arg, const char *arg,
#endif /* !CONFIG_VNC */
void qmp_change(const char *device, const char *target,
-bool has_arg, const char *arg, Error **errp)
+bool has_arg, const char *arg,
+bool has_read_only, BlockdevChangeReadOnlyMode read_only,
+Error **errp)
{
+if (!has_read_only) {
+read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN;
+}
+
if (strcmp(device, vnc) == 0) {
+if (has_read_only) {
+error_setg(errp, Parameter 'read-only' is invalid for VNC);
+return;
+}
qmp_change_vnc(target, has_arg, arg, errp);
} else {
-qmp_change_blockdev(device, target, arg,
-BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, errp);
+qmp_change_blockdev(device, target, arg, read_only, errp);
}
}
--
1.9.3
Re: [Qemu-devel] [PATCH] armv7m: optional -kernel if -gdb present
On 20 November 2014 12:34, Liviu Ionescu i...@livius.net wrote:
On 20 Nov 2014, at 14:29, Peter Maydell peter.mayd...@linaro.org wrote:
-if (!kernel_filename !qtest_enabled()) {
+if (!kernel_filename !qtest_enabled() !with_gdb) {
fprintf(stderr, Guest image must be specified (using -kernel)\n);
exit(1);
}
just delete this entire if() statement. This is how we've handled
similar issues with the ARM A profile boards. (Some of the boards
still have those checks but that's just because nobody's bothered
to fix them yet.)
I'm a bit confused. if not running with gdb, what is the expected
behaviour if the image is missing?
Same thing as if you start a hardware board with nothing loaded
into the flash. (Probably this means go into an infinite loop
of taking exceptions.)
-- PMM
Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration
On (Thu) 20 Nov 2014 [12:35:54], Dr. David Alan Gilbert wrote: * Amit Shah (amit.s...@redhat.com) wrote: On (Thu) 20 Nov 2014 [19:39:11], Gonglei wrote: The static variables in migration_bitmap_sync will not be reset in the case of a second attempted migration. Signed-off-by: ChenLiang chenlian...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com Good catch. Applied.. Hi, Juan? Ping... please :) Juan, what happened to this patch? Nearly for half a year, I forgot it completely :( Thanks for your prompt, Paolo. Yeah; unfortunate. I feel the patch could've been done in a better way, but since it's been a while... Paolo, Dave, can you please give me r-b and I'll pick it up for 2.2. Reviewed-by: Dr. David Alan Gilbert dgilb...@redhat.com Thanks; pull req sent. Amit
[Qemu-devel] [PULL] migration: fix for unbreaking stats/autoconverge on repeat migrations
The following changes since commit af3ff19b48f0bbf3a8bd35c47460358e8c6ae5e5: Update version for v2.2.0-rc2 release (2014-11-18 18:00:58 +) are available in the git repository at: git://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/for-2.2-2 for you to fetch changes up to 6c1b663c4c3725bc4bc33f78ed266ddef80a2ca8: migration: static variables will not be reset at second migration (2014-11-20 18:17:22 +0530) Fix from a while back that unfortunately got ignored. Dave Gilbert says it may actually fix a case where autoconverge would break on a repeat migration (and not just fix stats). ChenLiang (1): migration: static variables will not be reset at second migration arch_init.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) Amit
[Qemu-devel] [RFC] Break cross migration from qemu-1.5 to qemu-2.1. because of input/hid rewriting
Hi, Gerd
I encounter a problem that breaking migration from qemu-1.5 to qemu-2.1.
The error message as below:
qemu-system-x86_64: hw/input/hid.c:121: hid_pointer_event: Assertion `hs-n
16' failed.
Qemu assert in hid_pointer_event().
I get the value of hs-n which is 16 by reproduction. And the code of qemu-1.5
:
static void hid_pointer_event(void *opaque,
int x1, int y1, int z1, int buttons_state)
{
HIDState *hs = opaque;
unsigned use_slot = (hs-head + hs-n - 1) QUEUE_MASK;
unsigned previous_slot = (use_slot - 1) QUEUE_MASK;
if (hs-n == QUEUE_LENGTH) {
/* Queue full. Discard old button state, combine motion normally. */
hs-ptr.queue[use_slot].buttons_state = buttons_state;
}
Which indicate it is legal when hs-n == QUEUE_LENGTH.
But now:
static void hid_pointer_event(DeviceState *dev, QemuConsole *src,
InputEvent *evt)
{
static const int bmap[INPUT_BUTTON_MAX] = {
[INPUT_BUTTON_LEFT] = 0x01,
[INPUT_BUTTON_RIGHT] = 0x02,
[INPUT_BUTTON_MIDDLE] = 0x04,
};
HIDState *hs = (HIDState *)dev;
HIDPointerEvent *e;
assert(hs-n QUEUE_LENGTH);
e = hs-ptr.queue[(hs-head + hs-n) QUEUE_MASK];
...
static void hid_pointer_sync(DeviceState *dev)
{
HIDState *hs = (HIDState *)dev;
HIDPointerEvent *prev, *curr, *next;
bool event_compression = false;
if (hs-n == QUEUE_LENGTH-1) {
/*
* Queue full. We are losing information, but we at least
* keep track of most recent button state.
*/
return;
}
What about this patch:
diff --git a/hw/input/hid.c b/hw/input/hid.c
index 148c003..56e0637 100644
--- a/hw/input/hid.c
+++ b/hw/input/hid.c
@@ -116,7 +116,7 @@ static void hid_pointer_event(DeviceState *dev, QemuConsole
*src,
HIDState *hs = (HIDState *)dev;
HIDPointerEvent *e;
-assert(hs-n QUEUE_LENGTH);
+assert(hs-n = QUEUE_LENGTH);
e = hs-ptr.queue[(hs-head + hs-n) QUEUE_MASK];
switch (evt-kind) {
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration
On 2014/11/20 21:00, Amit Shah wrote: On (Thu) 20 Nov 2014 [12:35:54], Dr. David Alan Gilbert wrote: * Amit Shah (amit.s...@redhat.com) wrote: On (Thu) 20 Nov 2014 [19:39:11], Gonglei wrote: The static variables in migration_bitmap_sync will not be reset in the case of a second attempted migration. Signed-off-by: ChenLiang chenlian...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com Good catch. Applied.. Hi, Juan? Ping... please :) Juan, what happened to this patch? Nearly for half a year, I forgot it completely :( Thanks for your prompt, Paolo. Yeah; unfortunate. I feel the patch could've been done in a better way, but since it's been a while... Paolo, Dave, can you please give me r-b and I'll pick it up for 2.2. Reviewed-by: Dr. David Alan Gilbert dgilb...@redhat.com Thanks; pull req sent. Amit Thanks :) Best regards, -Gonglei
Re: [Qemu-devel] [PATCH] armv7m: optional -kernel if -gdb present
On 20 Nov 2014, at 14:50, Peter Maydell peter.mayd...@linaro.org wrote: Same thing as if you start a hardware board with nothing loaded into the flash. (Probably this means go into an infinite loop of taking exceptions.) hmmm... and you consider this behaviour to meet the user-friendly requirements? I tried it, and the program simply hangs, without any stdout/stderr messages. if you find this behaviour acceptable for unix users, ok, you don't have to update the other profiles, but for most cortex-m users it is confusing, and user-friendliness is not only appreciated, but required. I would appreciate you reconsider the patch. thank you, Liviu
Re: [Qemu-devel] [PATCH] armv7m: optional -kernel if -gdb present
On 20 November 2014 13:09, Liviu Ionescu i...@livius.net wrote: On 20 Nov 2014, at 14:50, Peter Maydell peter.mayd...@linaro.org wrote: Same thing as if you start a hardware board with nothing loaded into the flash. (Probably this means go into an infinite loop of taking exceptions.) hmmm... and you consider this behaviour to meet the user-friendly requirements? I tried it, and the program simply hangs, without any stdout/stderr messages. Yes. That's what hardware does in that situation. I don't think that whether or not a debugger has been connected should change our behaviour. (And even with your patch, if you connect a debugger and just hit its 'run' button without loading an image then we'll do the same exception-loop.) if you find this behaviour acceptable for unix users, ok, you don't have to update the other profiles, but for most cortex-m users it is confusing, and user-friendliness is not only appreciated, but required. I agree it's not very user friendly. But I don't like inconsistency between our behaviour for different boards either. (And for some boards we're going to have a bios or other firmware which will run if you don't specify -kernel.) In general I think many of the concerns you're raising here are real problems, and our user-friendliness is indeed poor in a lot of places. However the solutions you're proposing are often specific to M-profile ARM, whereas I have to consider the whole project and would prefer solutions which clean up and deal with an issue for all boards and all CPUs. That's obviously harder than a more local and restricted fix, but the benefit is greater. thanks -- PMM
Re: [Qemu-devel] [PATCH] armv7m: optional -kernel if -gdb present
On 20 Nov 2014, at 15:20, Peter Maydell peter.mayd...@linaro.org wrote: ... However the solutions you're proposing are often specific to M-profile ARM, ok, I'll keep this local to my branch. what about the previous patch, is it acceptable? regards, Liviu
[Qemu-devel] How to access guest memory from qemu device internal
Hello, all I added a custom device to qemu. This device is attached to sysbus by mmio and has an address register in which device should access the guest memory the register point to. I write a bare-metal program that pass an address like 0x1234ABCD to this address register. Inside qemu device code I added, if device reads value from register and directly accesses this value of 0x1234ABCD, it will access host memory 0x1234ABCD rather than guest memory 0x1234ABCD. Does qemu provide some functions that allow device to access guest memory address? Thanks, Kaiyuan Liang
Re: [Qemu-devel] [PATCH] Add -semihosting-config ....cmdline=string.
On 19 November 2014 22:05, Liviu Ionescu i...@livius.net wrote:
A new sub-option was added to -semihosting-config to define the entire
semihosting command line (cmdline=string).
This string is passed down to armv7m.c; if not defined, for
compatibility reasons, the -kernel -append values are used.
The armv7m_init() and stellaris_init() interfaces were streamlined,
to use the MachineState structure instead of separate strings.
The semihosting_cmdline was added to the structures MachineState and
arm_boot_info.
I think you can avoid having to plumb the command line
string into the MachineState and arm_boot_info structures,
because you can just have the semihosting code look the
option up by name:
QemuOpts *opts =
qemu_opts_find(qemu_find_opts(semihosting-config), NULL);
cmdline = qemu_opt_get(opts, cmdline);
if (cmdline) {
...
} else {
fall back to constructing from kernel/append args;
}
That will also automatically make the command line option
work for A profile CPUs as well.
thanks
-- PMM
Re: [Qemu-devel] [PATCH 2/5] exec: qemu_ram_alloc_device, qemu_ram_resize
Michael S. Tsirkin m...@redhat.com writes: On Wed, Nov 19, 2014 at 11:16:57AM +0100, Markus Armbruster wrote: Michael S. Tsirkin m...@redhat.com writes: On Wed, Nov 19, 2014 at 10:19:22AM +0100, Juan Quintela wrote: Michael S. Tsirkin m...@redhat.com wrote: On Tue, Nov 18, 2014 at 07:03:58AM +0100, Paolo Bonzini wrote: On 17/11/2014 21:08, Michael S. Tsirkin wrote: Add API to manage on-device RAM. This looks just like regular RAM from migration POV, but has two special properties internally: - block is sized on migration, making it easier to extend without breaking migration compatibility or wasting virtual memory - callers must specify an upper bound on size Also, I am afraid that this design could make it easier to introduce backwards-incompatible changes. Well the point is exactly to make it easy to make *compatible* changes. As I mentioned in the cover letter, it's not just ACPI. For example, we now change boot index dynamically. People using large fw cfg blobs, e.g. -initrd, would benefit from ability to change the blob dynamically. There could be other examples. changing the size of the initrd, on the fly and wanting to migrate? Is that a real use case? One that we should really care? I'm not sure. At the moment one can swap kernels by doing halt in guest and restarting with the new one. If we wanted to allow reboot in guest to bring a new kernel instead, that would be one way to implement it. I was merely pointing out that the capability might find other uses. I very much prefer to have user-controlled ACPI information (coming from the command-line) byte-for-byte identical for a given machine type. Patches for that have been on the list for almost two months, and it's not nice. Paolo I guess we just disagree on whether these patches will effectively achieve this goal. For example, some people want to rewrite iasl bits, generating everything in C. This will affect static bits too. I don't want to make every single change in code conditional on a machine type. You can't migrate with a different BIOS on destination, period. This claim is very wrong. This would make is impossible to change BIOS bus without breaking migration. Look at history of qemu, we change BIOS every release. Since migration doesn't transport configuration, we require a compatibly configured target, and that includes identical memory sizes. RAM size is explicit and the user's problem. ROM size is generally implicit, and we use machine type compatibility machinery to keep it fixed. BIOS changes can break migration only when we screw up or forget the compatibility machinery. Same as for lots of other stuff. No big deal, really, just a consequence of not migrating configuration. You don't get to maintain it, so it's no big deal for you. I see pain every single release and code is becoming spaghetty-like very quickly. We thought it would work. It does not. We do need a solution. And the pain is completely self-inflicted: we already migrate all necessary information! It's just a question of adjusting our datastructures to it. That is what is making this whole issue complicated. We have two clear options: a- require BIOS memory regions to be exactly the same in both sides. No need to add compat machinery. b- trying to accomodate any potential change that could appear and use the same BIOS. IMHO, b) is just asking for trouble. Being able to go from random changes to random changes look strange. Yes, it is hard to support. But it's a required feature, and in fact, it's an existing one. Just think about it for a second. We are sending more data for some regions that it was allocated. And we just grow the regions and expect that everything is going to be ok. It is me, or this goes against every security discipline that I can think of? Later, Juan. We have many devices that just get N from stream, do malloc(N), then read data from stream into it. You think it's unsafe? Go ahead and fix them all. However, my patch does address your concern: callers specify the upper limit on the region size. Trying to migrate in a 1Gbyte region Are you proposing to make incoming migration adjust some or all memory sizes on the target from whatever was configured during startup to whatever is configured on the source? Yes. At the moment, I only propose this for internal on-device RAM, for the simple reason that users don't know or care about it. So migrating it just removes maintainance pain. It wouldn't be hard to extend it for user-specified RAM, but I don't know whether that is useful. Possibly with some limitations, such as can only adjust downwards? Yes. Can adjust downwards is too limiting, since especially downstreams want
Re: [Qemu-devel] [PATCH v2 21/21] iotests: Add test for different refcount widths
On 2014-11-18 at 21:26, Eric Blake wrote: On 11/17/2014 05:06 AM, Max Reitz wrote: Umm, that sounds backwards from what you document. It's a good test of the _new_ reftable needing a second round of allocations. So keep it with corrected comments. But I think you _intended_ to write a test that starts with a refcount_width=64 image and resize to a refcount_width=1, where the _old_ reftable then suffers a reallocation as part of allocating refblocks for the new table. It may even help if you add a tracepoint for every iteration through the walk function callback, to prove we are indeed executing it 3 times instead of the usual 2, for these test cases. I'm currently thinking about a way to test the old reftable reallocation issue, and I can't find any. So, for the old reftable to require a reallocation it must grow. For it to grow we need some allocation beyond what it can currently represent. For this to happen during the refblock allocation walk, this allocation must be the allocation of a new refblock. If the refblock is allocated beyond the current reftable's limit, this means that either all clusters between free_cluster_index and that point are already taken. If the reftable is then reallocated, it will therefore *always* be allocated behind that refblock, which is beyond its old limit. Therefore, that walk through the old reftable will never miss that new allocation. So the issue can only occur if the old reftable is resized after the walk through it, that is, when allocating the new reftable. That is indeed an issue but I think it manifests itself basically like the issue I'm testing here: There is now an area in the old refcount structures which was free before but has is used now, and the allocation causing that was the allocation of the new reftable. The only difference is whether the it's the old or the new reftable that resides in the previously free area. Thus, I think I'll leave it at this test – but if you can describe to me how to create an image for a different rewalk path, I'm all ears. = The test you wrote does: original image, pre-walk: reftable is one cluster; with one refblock and 63 zero entries that refblock holds 4096 width-1 refcounts; of those, the first 63 are non-zero, the remaining are zero. Image is 32256 bytes long During the first walk, we call operation() 64 times - the first time with refblock_empty false, the remaining 63 times with refblock_empty true. after first walk but before reftable allocation, we have allocated one refblock that holds 64 width-64 refcounts (all zero, because we don't populate them until the final walk); and the old table now has 64 refcounts populated. Image is 32768 bytes long. Then we allocating a new reftable; so far, we only created one refblock for it to hold, so one cluster is sufficient. The allocation causes the old table to now have 65 refcounts populated. Image is now 33280 bytes long. On the second pass, we call operation() 64 times; now the first two walks have refblock_empty as false, which means we allocate a new refblock. This allocation causes the old table to now have 66 refcounts populated. Image is now 33792 bytes long. So we free our first attempt at a new reftable, and allocate another (a single cluster is still sufficient to hold two refblocks); I'm not sure whether this free/realloc will reuse cluster 65 or if it will pick up cluster 67 and leave a hole in 65. [I guess it depends on whether cluster allocation is done by first-fit analysis or whether it blindly favors allocating at the end of the image]. There is a free_cluster_index to speed up finding the first fit. It's reset when freeing clusters before that index, therefore cluster 65 should be reused. Either way, we have to do a third iteration, because the second iteration allocated a refblock and reallocated a reftable. On the third pass, operation() is still called 64 times, but because the only two calls with refblock_empty as false already have an allocated refblock, no further allocations are needed, and we are done with the do loop; the fourth walk can set refcounts. = The test I thought you were writing would start original image, pre-walk: reftable is one cluster; with one refblock and 63 zero entries that refblock holds 64 width-64 refcounts; of those, the first 63 are non-zero, the remaining are zero. Image is 32256 bytes long During the first walk, we call operation() 1 time, with refblock_empty false. after first walk but before reftable allocation, we have allocated one refblock that holds 4096 width-1 refcounts (all zero, because we don't populate them until the final walk); and the old table now has 64 refcounts populated. Image is 32768 bytes long. Then we allocating a new reftable; so far, we only created one refblock for it to hold, so one cluster is sufficient. The allocation causes the old table to now have 66 refcounts populated (one for the new refblock, but also one for an additional refblock in the
[Qemu-devel] [PULL 2.2 1/3] target-ppc: Fix breakpoint registers for e300
From: Fabien Chouteau chout...@adacore.com In the previous patch, the registers were added to init_proc_G2LE instead of init_proc_e300. Signed-off-by: Fabien Chouteau chout...@adacore.com Signed-off-by: Alexander Graf ag...@suse.de --- target-ppc/translate_init.c | 52 ++--- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 20d58c0..1fece7b 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -4374,32 +4374,6 @@ static void init_proc_G2LE (CPUPPCState *env) SPR_NOACCESS, SPR_NOACCESS, spr_read_generic, spr_write_generic, 0x); -/* Breakpoints */ -/* XXX : not implemented */ -spr_register(env, SPR_DABR, DABR, - SPR_NOACCESS, SPR_NOACCESS, - spr_read_generic, spr_write_generic, - 0x); -/* XXX : not implemented */ -spr_register(env, SPR_DABR2, DABR2, - SPR_NOACCESS, SPR_NOACCESS, - spr_read_generic, spr_write_generic, - 0x); -/* XXX : not implemented */ -spr_register(env, SPR_IABR2, IABR2, - SPR_NOACCESS, SPR_NOACCESS, - spr_read_generic, spr_write_generic, - 0x); -/* XXX : not implemented */ -spr_register(env, SPR_IBCR, IBCR, - SPR_NOACCESS, SPR_NOACCESS, - spr_read_generic, spr_write_generic, - 0x); -/* XXX : not implemented */ -spr_register(env, SPR_DBCR, DBCR, - SPR_NOACCESS, SPR_NOACCESS, - spr_read_generic, spr_write_generic, - 0x); /* Memory management */ gen_low_BATs(env); @@ -4628,6 +4602,32 @@ static void init_proc_e300 (CPUPPCState *env) SPR_NOACCESS, SPR_NOACCESS, spr_read_generic, spr_write_generic, 0x); +/* Breakpoints */ +/* XXX : not implemented */ +spr_register(env, SPR_DABR, DABR, + SPR_NOACCESS, SPR_NOACCESS, + spr_read_generic, spr_write_generic, + 0x); +/* XXX : not implemented */ +spr_register(env, SPR_DABR2, DABR2, + SPR_NOACCESS, SPR_NOACCESS, + spr_read_generic, spr_write_generic, + 0x); +/* XXX : not implemented */ +spr_register(env, SPR_IABR2, IABR2, + SPR_NOACCESS, SPR_NOACCESS, + spr_read_generic, spr_write_generic, + 0x); +/* XXX : not implemented */ +spr_register(env, SPR_IBCR, IBCR, + SPR_NOACCESS, SPR_NOACCESS, + spr_read_generic, spr_write_generic, + 0x); +/* XXX : not implemented */ +spr_register(env, SPR_DBCR, DBCR, + SPR_NOACCESS, SPR_NOACCESS, + spr_read_generic, spr_write_generic, + 0x); /* Memory management */ gen_low_BATs(env); gen_high_BATs(env); -- 1.8.1.4
[Qemu-devel] [PULL 2.2 3/3] target-ppc: Altivec's mtvscr Decodes Wrong Register
From: Tom Musta tommu...@gmail.com The Move to Vector Status and Control Register (mtvscr) instruction uses VRB as the source register. Fix the code generator to correctly decode the VRB field. That is, use rB(ctx-opcode) instead of rD(ctx-opcode). Signed-off-by: Tom Musta tommu...@gmail.com Signed-off-by: Alexander Graf ag...@suse.de --- target-ppc/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-ppc/translate.c b/target-ppc/translate.c index 910ce56..d381632 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -6848,7 +6848,7 @@ static void gen_mtvscr(DisasContext *ctx) gen_exception(ctx, POWERPC_EXCP_VPU); return; } -p = gen_avr_ptr(rD(ctx-opcode)); +p = gen_avr_ptr(rB(ctx-opcode)); gen_helper_mtvscr(cpu_env, p); tcg_temp_free_ptr(p); } -- 1.8.1.4
[Qemu-devel] [PULL 2.2 2/3] kvm: Fix memory slot page alignment logic
Memory slots have to be page aligned to get entered into KVM. There
is existing logic that tries to ensure that we pad memory slots that
are not page aligned to the biggest region that would still fit in the
alignment requirements.
Unfortunately, that logic is broken. It tries to calculate the start
offset based on the region size.
Fix up the logic to do the thing it was intended to do and document it
properly in the comment above it.
With this patch applied, I can successfully run an e500 guest with more
than 3GB RAM (at which point RAM starts overlapping subpage memory regions).
Cc: qemu-sta...@nongnu.org
Signed-off-by: Alexander Graf ag...@suse.de
---
kvm-all.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index 44a5e72..596e7ce 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -634,8 +634,10 @@ static void kvm_set_phys_mem(MemoryRegionSection *section,
bool add)
unsigned delta;
/* kvm works in page size chunks, but the function may be called
- with sub-page size and unaligned start address. */
-delta = TARGET_PAGE_ALIGN(size) - size;
+ with sub-page size and unaligned start address. Pad the start
+ address to next and truncate size to previous page boundary. */
+delta = (TARGET_PAGE_SIZE - (start_addr ~TARGET_PAGE_MASK));
+delta = ~TARGET_PAGE_MASK;
if (delta size) {
return;
}
--
1.8.1.4
[Qemu-devel] [PULL 2.2 0/3] ppc patch queue 2014-11-20
Hi Peter, This is my current patch queue for ppc. Please pull. Alex The following changes since commit af3ff19b48f0bbf3a8bd35c47460358e8c6ae5e5: Update version for v2.2.0-rc2 release (2014-11-18 18:00:58 +) are available in the git repository at: git://github.com/agraf/qemu.git tags/signed-ppc-for-upstream for you to fetch changes up to 76cb6584196b6f35d6e9b5124974d3eba643f772: target-ppc: Altivec's mtvscr Decodes Wrong Register (2014-11-20 14:52:01 +0100) Patch queue for ppc - 2014-11-20 Hopefully the last few fixups for 2.2: - KVM memory slot fix (should usually only occur on PPC) - e300 fix - Altivec mtvscr instruction fix Alexander Graf (1): kvm: Fix memory slot page alignment logic Fabien Chouteau (1): target-ppc: Fix breakpoint registers for e300 Tom Musta (1): target-ppc: Altivec's mtvscr Decodes Wrong Register kvm-all.c | 6 -- target-ppc/translate.c | 2 +- target-ppc/translate_init.c | 52 ++--- 3 files changed, 31 insertions(+), 29 deletions(-)
Re: [Qemu-devel] [PULL] migration: fix for unbreaking stats/autoconverge on repeat migrations
On 20 November 2014 12:59, Amit Shah amit.s...@redhat.com wrote: The following changes since commit af3ff19b48f0bbf3a8bd35c47460358e8c6ae5e5: Update version for v2.2.0-rc2 release (2014-11-18 18:00:58 +) are available in the git repository at: git://git.kernel.org/pub/scm/virt/qemu/amit/migration.git tags/for-2.2-2 for you to fetch changes up to 6c1b663c4c3725bc4bc33f78ed266ddef80a2ca8: migration: static variables will not be reset at second migration (2014-11-20 18:17:22 +0530) Fix from a while back that unfortunately got ignored. Dave Gilbert says it may actually fix a case where autoconverge would break on a repeat migration (and not just fix stats). ChenLiang (1): migration: static variables will not be reset at second migration Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v2 21/21] iotests: Add test for different refcount widths
On 2014-11-19 at 06:52, Eric Blake wrote:
On 11/18/2014 01:26 PM, Eric Blake wrote:
Now, in response to your question about some other 3-pass inducing
pattern, let's think back to v1, where you questioned what would happen
if a hole in the reftable gets turned into data due to a later
allocation. Let's see if I can come up with a scenario for that...
Let's stick with a cluster size of 512, and use 32-bit and 64-bit widths
as our two sizes. If we downsize from 64 to 32 bits, then every two
refblock clusters in the old table results in one call to operation()
for the new table; conversely, if we upsize, then every refblock cluster
in the old table gives two calls to operation() in the new table. The
trick at hand is to come up with some image where we punch a hole so
that on the first pass, we call operation() with refblock_empty true for
one iteration (necessarily a call later than the first, since the image
header guarantees the first refblock is not empty), but where we have
data after the hole, where it is the later data that triggers the
allocation that will finally start to fill the hole.
How about starting with an image that occupies between 1.5 and 2
refblocks worth of 32-width clusters (so an image anywhere between 193
and 256 clusters, or between 98816 and 131072 bytes). You should be
able to figure out how many clusters this consumes for L1, L2, plus 1
for header, reftable, and 2 for refblocks, in order to figure out how
many remaining clusters are dedicated to data; ideally, the data
clusters are contiguous, and occupy a swath that covers at least
clusters 126 through 192. Widening to 64-bit width will require 4
refblocks instead of 2, if all refblocks are needed. But the whole idea
of punching a hole is that we don't need a refblock if it will be
all-zero entries. So take this original image, and discard the data
clusters from physical index 126 through 192, (this is NOT the data
visible at guest offset 31744, but whatever actual offset of guest data
that maps to physical offset 31744). The old reftable now looks like {
refblock_o1 [0-125 occupied, 126 and 127 empty], refblock_o2 [128-191
empty, 192-whatever occupied, tail empty] }. With no allocations
required, this would in turn would map to the following new refblocks: {
refblock_n1 [0-64 occupied], refblock_n2 [65-125 occupied, 126-127
empty], NULL, refblock_n4 [192-whatever occupied] }. Note that we do
not need to allocate refblock_n3 because of the hole in the old
refblock; we DO end up allocating three refblocks, but in the sequence
of things, refblock_n1 and refblock_n2 are allocated while we are
visiting refblock_o1 and still fit in refblock_o1, while refblock_n4 is
not allocated until after we have already passed over the first half of
refblock_o2.
Thus, the second walk over the image will see that we need to allocate
refblock_n3 because it now contains entries (in particular, the entry
for refblock_n4, but also the 1-cluster entry for the proposed reftable
that is allocated between the walks). So, while your test used the
allocation of the reftable as the spillover point, my scenario here uses
the allocation of later refblocks as the spillover point that got missed
during the first iteration.
Oops,...
which means the reftable now looks like { refblock1, NULL, refblock3,
NULL... }; and where refblock1 now has at least two free entries
(possibly three, if the just-freed refblock2 happened to live before
cluster 62). is we can also free refblock2
...forgot to delete these random thoughts that I typed up but no longer
needed after reworking the above text.
At any rate, I'm not certain we can come up with a four-pass scenario;
if it is even possible, it would be quite complex.
[snip] (But rest assured, I read it all ;-))
At this point, I've spent far too long writing this email. I haven't
completely ruled out the possibility of a corner case needing four
passes through the do loop, but the image sizes required to get there
are starting to be quite large compared to your simpler test of needing
three passes through the do loop.
Right, see test 026. Without an SSD, it takes more than ten minutes, not
least because it tests resizing the reftable which means writing a lot
of data to an image with 512 byte clusters.
I won't be bothered if we call it
good, and quit trying to come up with any other interesting allocation
sequencing.
The problem is, in my opinion, that we won't gain a whole lot from
proving that there are cases where you need a fourth pass and test these
cases. Fundamentally, they are not different from cases with three
passes (technically, not even different from two pass cases). You scan
through the refcounts, you detect that you need refblocks which you have
not yet allocated, you allocate them, then you respin until all
allocations are done. The only problem would be whether it'd be possible
to run into an infinite loop: Can allocating new refblocks lead to a
case where we have
Re: [Qemu-devel] [PATCH 2/5] exec: qemu_ram_alloc_device, qemu_ram_resize
On Thu, Nov 20, 2014 at 02:35:14PM +0100, Markus Armbruster wrote: What am I missing here that can justify the complexity of partially overriding target configuration in the migration stream plus infrastructure for resizing memory? The justification is that sizing it properly is an unsolved problem. The difference with real hardware is that size of the flash depends dynamically on the machine configuration. And it's drastic: you can have from 1 to 256 CPUs, 0 to 256 PCI bridges on each root, etc. And I do believe the infrastructure will be handy for other things. For example, boot order ROM is now dynamic too, with enough bootable devices it will start overflowing a page and then we will have the same problem. And the patchset is all of 150 lines with comments, the point is that everything follows the same path: it's enough to test one cross-version migration, e.g. 2.1-2.3 or whatever, to make sure resizing works properly. Unlike extra modes which need testing of each machine type with each guest. -- MST
Re: [Qemu-devel] [PATCH] target-ppc: Load/Store Vector Element Storage Alignment
On 17.11.14 21:58, Tom Musta wrote: The Load Vector Element Indexed and Store Vector Element Indexed instructions compute an effective address in the usual manner. However, they truncate that address to the natural boundary. For example, the lvewx instruction will ignore the least significant two bits of the address and thus load the aligned word of storage. Fix the generators for these instruction to properly perform this truncation. Signed-off-by: Tom Musta tommu...@gmail.com Thanks, applied to ppc-next-2.3 Alex
Re: [Qemu-devel] [2.3 V2 PATCH 2/6] target-ppc: Fix Floating Point Move Instructions That Set CR1
On 12.11.14 22:46, Tom Musta wrote:
The Floating Point Move instructions (fmr., fabs., fnabs., fneg.,
and fcpsgn.) incorrectly copy FPSCR[FPCC] instead of [FX,FEX,VX,OX].
Furthermore, the current code does this via a call to gen_compute_fprf,
which is awkward since these instructions do not actually set FPRF.
Change the code to use the gen_set_cr1_from_fpscr utility.
Signed-off-by: Tom Musta tommu...@gmail.com
---
target-ppc/translate.c | 50 ---
1 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 910ce56..2d79e39 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2077,6 +2077,21 @@ static void gen_srd(DisasContext *ctx)
}
#endif
+#if defined(TARGET_PPC64)
+static void gen_set_cr1_from_fpscr(DisasContext *ctx)
+{
+TCGv_i32 tmp = tcg_temp_new_i32();
+tcg_gen_trunc_tl_i32(tmp, cpu_fpscr);
+tcg_gen_shri_i32(cpu_crf[1], tmp, 28);
+tcg_temp_free_i32(tmp);
+}
+#else
+static void gen_set_cr1_from_fpscr(DisasContext *ctx)
+{
+tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28);
+}
+#endif
+
/*** Floating-Point arithmetic
***/
#define _GEN_FLOAT_ACB(name, op, op1, op2, isfloat, set_fprf, type)
\
static void gen_f##name(DisasContext *ctx)
\
@@ -2370,7 +2385,9 @@ static void gen_fabs(DisasContext *ctx)
}
tcg_gen_andi_i64(cpu_fpr[rD(ctx-opcode)], cpu_fpr[rB(ctx-opcode)],
~(1ULL 63));
-gen_compute_fprf(cpu_fpr[rD(ctx-opcode)], 0, Rc(ctx-opcode) != 0);
+if (unlikely(Rc(ctx-opcode))) {
+gen_set_cr1_from_fpscr(ctx);
+}
I don't quite understand this. We set cr1 based on fpscr, but we don't
recalculate the respective fpscr bits?
Wouldn't we get outdated comparison data?
Alex
Re: [Qemu-devel] [PATCH] hw/arm/realview.c: Fix memory leak in realview_init()
On 20 November 2014 11:53, Kirill Batuzov batuz...@ispras.ru wrote: I'm surprised that this small patch caused so much controversy. It seems very simple and straightforward to me. This patch fixes a memory leak. The fact that it indeed was a memory leak is indicated by Valgrind output (Memcheck's false-positives are extremely rare unless you do some really nasty things with your pointers). It can be verified manually too: there are only 4 occurrences of 'ram_lo' in realview.c. It's in exactly the same situation as the other blocks of memory like ram_hi in that file: we allocate it and then don't care about freeing it, because we don't happen to have a board state struct. The correct fix if you care about this kind of thing would be to have a board state struct which had MemoryRegion fields (not MemoryRegion* fields). We have lots of bits of memory that we allocate once on startup and then don't care about freeing. I think we are talking about a bit different problems here. Indeed ram_hi is allocated, then used until QEMU exits but is never freed. Yet it is never completely lost: there is at least one pointer to it in memory hierarchy. Valgrind calls such situations still reachable and does not consider them errors (because memory is in use until the very moment the program exits; at least it can not be proven different). ram_lo is different. It can be added to memory hierarchy in which case it will behave exactly the same way as ram_hi. But it may be not used at all in which case all pointers to it will be lost. This is the real memory leak. Valgrind reports such situations as definitely lost and they are considered errors (because it can be proven that memory was allocated, is not in use and was not freed). In our case ram_lo was reported as definitely lost while ram_hi was still reachable and was never reported as error. This patch addresses the second problem (when ram_lo is definitely lost) because it has very short and simple solution. While you are arguing that we need to address the first problem - which is also true but it is different problem that will need different solution and a much larger one. It just doesn't seem to me very useful to merely silence the warning rather than actually fixing the underlying thing that the warning is telling you about. As I described above, it actually solves the problem Valgrind reports. It is just a different problem than you are talking about. I'll probably put it in, because it's not very harmful. Either way is fine with me. I'm still sure this patch is worthwhile but on the other hand it is not that big of an issue to be arguing about it for too long. -- Kirill
Re: [Qemu-devel] [PATCH v6] qcow2: Buffer L1 table in snapshot refcount update
On 2014-11-11 at 16:27, Max Reitz wrote: From: Zhang Haoyu zhan...@sangfor.com Buffer the active L1 table in qcow2_update_snapshot_refcount() in order to prevent in-place conversion of the L1 table buffer in the BDRVQcowState to big endian and back, which would lead to data corruption if that buffer was accessed concurrently. This should not happen but better being safe than sorry. Signed-off-by: Zhang Haoyu zhan...@sangfor.com Signed-off-by: Max Reitz mre...@redhat.com --- v6 for snapshot: use local variable to bdrv_pwrite_sync L1 table (I changed the commit message wording to make it more clear what this patch does and why we want it). Changes in v6: - Only copy the local buffer back into s-l1_table if we are indeed accessing the local L1 table - Use qemu_vfree() instead of g_free() --- block/qcow2-refcount.c | 30 ++ 1 file changed, 14 insertions(+), 16 deletions(-) Ping
Re: [Qemu-devel] [2.3 V2 PATCH 2/6] target-ppc: Fix Floating Point Move Instructions That Set CR1
On 11/20/2014 8:14 AM, Alexander Graf wrote:
On 12.11.14 22:46, Tom Musta wrote:
The Floating Point Move instructions (fmr., fabs., fnabs., fneg.,
and fcpsgn.) incorrectly copy FPSCR[FPCC] instead of [FX,FEX,VX,OX].
Furthermore, the current code does this via a call to gen_compute_fprf,
which is awkward since these instructions do not actually set FPRF.
Change the code to use the gen_set_cr1_from_fpscr utility.
Signed-off-by: Tom Musta tommu...@gmail.com
---
target-ppc/translate.c | 50
---
1 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 910ce56..2d79e39 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2077,6 +2077,21 @@ static void gen_srd(DisasContext *ctx)
}
#endif
+#if defined(TARGET_PPC64)
+static void gen_set_cr1_from_fpscr(DisasContext *ctx)
+{
+TCGv_i32 tmp = tcg_temp_new_i32();
+tcg_gen_trunc_tl_i32(tmp, cpu_fpscr);
+tcg_gen_shri_i32(cpu_crf[1], tmp, 28);
+tcg_temp_free_i32(tmp);
+}
+#else
+static void gen_set_cr1_from_fpscr(DisasContext *ctx)
+{
+tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28);
+}
+#endif
+
/*** Floating-Point arithmetic
***/
#define _GEN_FLOAT_ACB(name, op, op1, op2, isfloat, set_fprf, type)
\
static void gen_f##name(DisasContext *ctx)
\
@@ -2370,7 +2385,9 @@ static void gen_fabs(DisasContext *ctx)
}
tcg_gen_andi_i64(cpu_fpr[rD(ctx-opcode)], cpu_fpr[rB(ctx-opcode)],
~(1ULL 63));
-gen_compute_fprf(cpu_fpr[rD(ctx-opcode)], 0, Rc(ctx-opcode) != 0);
+if (unlikely(Rc(ctx-opcode))) {
+gen_set_cr1_from_fpscr(ctx);
+}
I don't quite understand this. We set cr1 based on fpscr, but we don't
recalculate the respective fpscr bits?
Wouldn't we get outdated comparison data?
Alex
Nope.
The floating point move instructions don't actually even alter the FPSCR. From
the ISA (see the last sentence):
4.6.5 Floating-Point Move Instructions
These instructions copy data from one floating-point
register to another, altering the sign bit (bit 0) as
described below for fneg, fabs, fnabs, and fcpsgn.
These instructions treat NaNs just like any other kind of
value (e.g., the sign bit of a NaN may be altered by
fneg, fabs, fnabs, and fcpsgn). These instructions do
not alter the FPSCR.
Re: [Qemu-devel] [2.3 V2 PATCH 2/6] target-ppc: Fix Floating Point Move Instructions That Set CR1
On 20.11.14 15:32, Tom Musta wrote:
On 11/20/2014 8:14 AM, Alexander Graf wrote:
On 12.11.14 22:46, Tom Musta wrote:
The Floating Point Move instructions (fmr., fabs., fnabs., fneg.,
and fcpsgn.) incorrectly copy FPSCR[FPCC] instead of [FX,FEX,VX,OX].
Furthermore, the current code does this via a call to gen_compute_fprf,
which is awkward since these instructions do not actually set FPRF.
Change the code to use the gen_set_cr1_from_fpscr utility.
Signed-off-by: Tom Musta tommu...@gmail.com
---
target-ppc/translate.c | 50
---
1 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 910ce56..2d79e39 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2077,6 +2077,21 @@ static void gen_srd(DisasContext *ctx)
}
#endif
+#if defined(TARGET_PPC64)
+static void gen_set_cr1_from_fpscr(DisasContext *ctx)
+{
+TCGv_i32 tmp = tcg_temp_new_i32();
+tcg_gen_trunc_tl_i32(tmp, cpu_fpscr);
+tcg_gen_shri_i32(cpu_crf[1], tmp, 28);
+tcg_temp_free_i32(tmp);
+}
+#else
+static void gen_set_cr1_from_fpscr(DisasContext *ctx)
+{
+tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28);
+}
+#endif
+
/*** Floating-Point arithmetic
***/
#define _GEN_FLOAT_ACB(name, op, op1, op2, isfloat, set_fprf, type)
\
static void gen_f##name(DisasContext *ctx)
\
@@ -2370,7 +2385,9 @@ static void gen_fabs(DisasContext *ctx)
}
tcg_gen_andi_i64(cpu_fpr[rD(ctx-opcode)], cpu_fpr[rB(ctx-opcode)],
~(1ULL 63));
-gen_compute_fprf(cpu_fpr[rD(ctx-opcode)], 0, Rc(ctx-opcode) != 0);
+if (unlikely(Rc(ctx-opcode))) {
+gen_set_cr1_from_fpscr(ctx);
+}
I don't quite understand this. We set cr1 based on fpscr, but we don't
recalculate the respective fpscr bits?
Wouldn't we get outdated comparison data?
Alex
Nope.
The floating point move instructions don't actually even alter the FPSCR.
From the ISA (see the last sentence):
4.6.5 Floating-Point Move Instructions
These instructions copy data from one floating-point
register to another, altering the sign bit (bit 0) as
described below for fneg, fabs, fnabs, and fcpsgn.
These instructions treat NaNs just like any other kind of
value (e.g., the sign bit of a NaN may be altered by
fneg, fabs, fnabs, and fcpsgn). These instructions do
not alter the FPSCR.
Thanks, applied with the following squashed in:
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 7aef089..35c3a16 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2088,7 +2088,7 @@ static void gen_set_cr1_from_fpscr(DisasContext *ctx)
#else
static void gen_set_cr1_from_fpscr(DisasContext *ctx)
{
-tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28);
+tcg_gen_shri_tl(cpu_crf[1], cpu_fpscr, 28);
}
#endif
Alex
Re: [Qemu-devel] [2.3 V2 PATCH 0/6] target-ppc: Assorted Floating Point Bugs and Cleanup
On 12.11.14 22:45, Tom Musta wrote: This patch series corrects some issues with floating point emulation on Power. Patch 1 corrects a corner case in the square root instructions, which incorrectly react to NaN whose sign bit is a 1. Patches 2-6 correct a rather pervasive problem with modeling of the CR[1] field (i.e. the dot form instructions of the FPU). The bugs were found by running random test patterns through actual Power hardware (P7 and P8) and comparing against QEMU. The patches conflict quite a bit with Paolo's series that splits CR into 32 one bit registers. Paolo: is V3 of your patch series coming anytime soon? V2 Reworked patches to pick up the gen_set_cr1_from_fpscr() utility that was recently added by Paolo Bonzini. Thanks, applied all to ppc-next-2.3. Alex
[Qemu-devel] Embroidery Patches
Dear Sir/Madam, Good day! This is Lisa from WellSucceed Embroidery. WellSucceed Embroidery is a factory direct manufacturer of patches.We can supply high quality embroidered patches, woven patches, and PVC patches. Both small patch and back patches can be produced in our factory. Sew on, Iron on and Velco Patches are available. Custom order is welcome. Besides, we also supply Keychains, lanyard, wristband, button badge, metal pins, dog tag, baseball cap,beanies, poster flag and etc. We normally produce all products with MOQ 100 pcs. Free sample production. If you are interested in, pls feel free to contact me. Best regards! Lisa Tang Sales Manager | Sales Department WELLSUCCEED EMBROIDERY (One Stop Sourcing of Patches, Keychains, Tin button badges, Poster Flags, Wristbands, Beanies, Baseball caps, Lanyards, Printing Books or stickers, Baby Bibs, Handbags, etc..) Cell:+86 18521590289| Fax:+86 769 81855570 Address: 3rd Floor, Tuozhan Building, Jijie, Chashan, Dongguan, Guangdong. 523380. China. Good News: Our factory now can produce felt keychain, felt coasters, baby bibs, cushions, etc..
Re: [Qemu-devel] [PULL 2.2 0/3] ppc patch queue 2014-11-20
On 20 November 2014 13:55, Alexander Graf ag...@suse.de wrote: Hi Peter, This is my current patch queue for ppc. Please pull. Alex The following changes since commit af3ff19b48f0bbf3a8bd35c47460358e8c6ae5e5: Update version for v2.2.0-rc2 release (2014-11-18 18:00:58 +) are available in the git repository at: git://github.com/agraf/qemu.git tags/signed-ppc-for-upstream for you to fetch changes up to 76cb6584196b6f35d6e9b5124974d3eba643f772: target-ppc: Altivec's mtvscr Decodes Wrong Register (2014-11-20 14:52:01 +0100) Patch queue for ppc - 2014-11-20 Hopefully the last few fixups for 2.2: - KVM memory slot fix (should usually only occur on PPC) - e300 fix - Altivec mtvscr instruction fix Applied, thanks. -- PMM
Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 v3 1/1] -machine vmport=auto: Fix handling of VMWare ioport emulation for xen
On Thu, Nov 20, 2014 at 12:00:19PM +0100, Paolo Bonzini wrote: On 20/11/2014 11:00, Dr. David Alan Gilbert wrote: I'm still not sure why the configuration should differ for -M pc depending on whether xen is enabled. I think this goes back to: commit 1611977c3d8fdbdac6090cbd1fcee4aed6d9 Author: Anthony PERARD anthony.per...@citrix.com Date: Tue May 3 17:06:54 2011 +0100 pc, Disable vmport initialisation with Xen. This is because there is not synchronisation of the vcpu register between Xen and QEMU, so vmport can't work properly. This patch introduces no_vmport parameter to pc_basic_device_init. Signed-off-by: Anthony PERARD anthony.per...@citrix.com Signed-off-by: Alexander Graf ag...@suse.de Yes, but Xen has since implemented vmport (commit 37f9e258). It's fine to have a conservative default for -M xenfv and possibly -M pc-2.1, but -M pc can require the latest hypervisor. Are there any ABI stability expectations when using -machine pc-2.1,accel=xen? I guess there aren't any, and in this case the first patch (the one changing default_machine_opts) would be enough. -- Eduardo
