Re: [Qemu-devel] QEMU 1.2 Test Day - August 16 2012
On Thu, Aug 16, 2012 at 10:24 AM, Stefan Hajnoczi stefa...@gmail.com wrote: On Tue, Aug 14, 2012 at 2:37 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Thu, Aug 2, 2012 at 1:22 PM, Stefan Hajnoczi stefa...@gmail.com wrote: I have set up the QEMU 1.2 Testing wiki page and suggest August 16 as the Test Day: http://wiki.qemu.org/Planning/1.2/Testing QEMU 1.2-rc0 has been tagged. A tarball is available: http://wiki.qemu.org/downloads/qemu-1.2.0-rc0.tar.bz2 Help test this release: try the features, guests, or host platforms that you care about. Add to the wiki page here: http://wiki.qemu.org/Planning/1.2/Testing To allow everyone to participate, the new official Test Day is Monday 20th August. Feel free to test before or after that, if necessary. Discussion is on the #qemu irc.oftc.net channel or qemu-devel@nongnu.org mailing list. Happy testing, Stefan
Re: [Qemu-devel] 2 issues with qemu-master / 1.2 ehci code
Hi,
On 08/16/2012 09:26 PM, Gerd Hoffmann wrote:
snip
I can get things working by turning ehci_fill_queue() into a nop...
Investigating this further. But if you've any insights they would be
appreciated. I'm thinking this may be caused by packets completing
out of order, which could be caused by the special handling of some
ctrl commands ...
usbredir wouldn't see multiple parallel inflight requests per endpoint
unless you explicitly enable this using usb_ep_set_pipeline(). Doing
that on bulk endpoints should give you a nice performance boost for usb
sticks. Doing it on the control endpoint is asking for trouble ;)
Can you turn on all ehci tracepoints send me a log?
No need for that, I've been debugging this almost the entire day yesterday
and I've significantly narrowed it down further, the problem is that
the assert triggers when:
1) There are more packets then 1 in the queue (iow fill_queue did something)
2) A packet completion is not followed by an advqueue call
2) happens when a packet fails, and the queue should be halted, in this case
ehci_state_writeback() correctly does not call advqueue, bot does a horizqh
instead. The problem is that once this happens p-qtdaddr == q-qtdaddr is no
longer true ...
I've already come up with multiple approaches to try and fix this, but none
work sofar :|
Another problem with failing packets is that hw/usb/core.c will happily execute
the next packet in the ep queue, even though the spec says the ep-queue should
be halted, giving the guest a chance to cancel transfers after the failed one
without them ever executing. I've a poc patch fixing this too.
I've attached a wip patch with my work on this so-far. Note that currently
the halted bit at the ehci level never gets cleared (since we've queues at 2
levels, we need to track halting at 2 levels too)
Regards,
Hans
diff --git a/hw/usb.h b/hw/usb.h
index 432ccae..6fa3088 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -179,6 +179,7 @@ struct USBEndpoint {
uint8_t ifnum;
int max_packet_size;
bool pipeline;
+bool halted;
USBDevice *dev;
QTAILQ_HEAD(, USBPacket) queue;
};
@@ -375,6 +376,8 @@ void usb_ep_set_max_packet_size(USBDevice *dev, int pid,
int ep,
uint16_t raw);
int usb_ep_get_max_packet_size(USBDevice *dev, int pid, int ep);
void usb_ep_set_pipeline(USBDevice *dev, int pid, int ep, bool enabled);
+void usb_ep_process_queue(USBEndpoint *ep);
+void usb_ep_clear_halt(USBEndpoint *ep);
void usb_attach(USBPort *port);
void usb_detach(USBPort *port);
diff --git a/hw/usb/core.c b/hw/usb/core.c
index c7e5bc0..c9ad57e 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -382,12 +382,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
usb_packet_check_state(p, USB_PACKET_SETUP);
assert(p-ep != NULL);
+/* Submitting a new packet clears halt */
+p-ep-halted = false;
+
if (QTAILQ_EMPTY(p-ep-queue) || p-ep-pipeline) {
ret = usb_process_one(p);
if (ret == USB_RET_ASYNC) {
usb_packet_set_state(p, USB_PACKET_ASYNC);
QTAILQ_INSERT_TAIL(p-ep-queue, p, queue);
} else {
+/*
+ * When pipelining is enabled usb-devices must always return async,
+ * otherwise packets can complete out of order!
+ */
+assert(!p-ep-pipeline);
p-result = ret;
usb_packet_set_state(p, USB_PACKET_COMPLETE);
}
@@ -402,33 +410,26 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
/* Notify the controller that an async packet is complete. This should only
be called for packets previously deferred by returning USB_RET_ASYNC from
handle_packet. */
-void usb_packet_complete(USBDevice *dev, USBPacket *p)
+static void __usb_packet_complete(USBDevice *dev, USBPacket *p)
{
USBEndpoint *ep = p-ep;
-int ret;
-usb_packet_check_state(p, USB_PACKET_ASYNC);
-assert(QTAILQ_FIRST(ep-queue) == p);
+if (p-result 0) {
+//ep-halted = true;
+}
usb_packet_set_state(p, USB_PACKET_COMPLETE);
QTAILQ_REMOVE(ep-queue, p, queue);
dev-port-ops-complete(dev-port, p);
+}
-while (!QTAILQ_EMPTY(ep-queue)) {
-p = QTAILQ_FIRST(ep-queue);
-if (p-state == USB_PACKET_ASYNC) {
-break;
-}
-usb_packet_check_state(p, USB_PACKET_QUEUED);
-ret = usb_process_one(p);
-if (ret == USB_RET_ASYNC) {
-usb_packet_set_state(p, USB_PACKET_ASYNC);
-break;
-}
-p-result = ret;
-usb_packet_set_state(p, USB_PACKET_COMPLETE);
-QTAILQ_REMOVE(ep-queue, p, queue);
-dev-port-ops-complete(dev-port, p);
-}
+void usb_packet_complete(USBDevice *dev, USBPacket *p)
+{
+USBEndpoint *ep = p-ep;
+
+usb_packet_check_state(p, USB_PACKET_ASYNC);
+assert(QTAILQ_FIRST(ep-queue) == p);
+__usb_packet_complete(dev, p);
+usb_ep_process_queue(ep);
}
/* Cancel an active
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Fri, Aug 17, 2012 at 9:38 AM, Steven wangwangk...@gmail.com wrote: Hi, Max, I appreciate your help and got some results using your patch. But I still have two questions as blow. I see that with the following patch diff --git a/softmmu_template.h b/softmmu_template.h index b8bd700..2d02133 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -114,6 +114,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM target_phys_addr_t ioaddr; uintptr_t retaddr; +fprintf(stderr, %s: %08x\n, __func__, addr); /* test if there is match for unaligned or IO access */ /* XXX: could done more in memory macro in a non portable way */ index = (addr TARGET_PAGE_BITS) (CPU_TLB_SIZE - 1); I get some memory accesses logged, but not all. That's due to fast path in tcg_out_qemu_ld in case there's TLB hit. I guess you can play with tcg_out_qemu_ld and make it produce a call to a helper function, like qemu_ld_helpers, that will print addresses for all memory access attempts. Easier solution would be to disable fast path and always go through softmmu helpers, like this (specific for x86 host): diff --git a/softmmu_template.h b/softmmu_template.h index b8bd700..2d02133 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -114,6 +114,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM target_phys_addr_t ioaddr; uintptr_t retaddr; +fprintf(stderr, %s: %08x\n, __func__, addr); /* test if there is match for unaligned or IO access */ /* XXX: could done more in memory macro in a non portable way */ index = (addr TARGET_PAGE_BITS) (CPU_TLB_SIZE - 1); diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c index da17bba..ec68c19 100644 --- a/tcg/i386/tcg-target.c +++ b/tcg/i386/tcg-target.c @@ -1062,7 +1062,7 @@ static inline void tcg_out_tlb_load(TCGContext *s, int addrlo_idx, tcg_out_mov(s, type, r0, addrlo); /* jne label1 */ -tcg_out8(s, OPC_JCC_short + JCC_JNE); +tcg_out8(s, OPC_JMP_short); label_ptr[0] = s-code_ptr; s-code_ptr++; IN: 0xc13e3a33: mov0x8(%ebp),%ebx (guest code in the tb) __ldl_mmu: c13a9fdc So 0xc13a9fdc is the guest virtual memory address of 0x8(%ebp). Is this correct? Right. IN: 0xc13e3a36: mov%eax,-0x10(%ebp) However, for this instruction, no ldl_mmu is logged. Does that mean the patch you provided does not cover this case? Yes, this is not 'ld', it is 'st'; to see it too I guess you need this: diff --git a/softmmu_template.h b/softmmu_template.h index b8bd700..b2ae078 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -114,6 +114,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM target_phys_addr_t ioaddr; uintptr_t retaddr; +fprintf(stderr, %s: %08x\n, __func__, addr); /* test if there is match for unaligned or IO access */ /* XXX: could done more in memory macro in a non portable way */ index = (addr TARGET_PAGE_BITS) (CPU_TLB_SIZE - 1); @@ -263,6 +264,7 @@ void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM uintptr_t retaddr; int index; +fprintf(stderr, %s: %08x\n, __func__, addr); index = (addr TARGET_PAGE_BITS) (CPU_TLB_SIZE - 1); redo: tlb_addr = env-tlb_table[mmu_idx][index].addr_write; -- Thanks. -- Max
Re: [Qemu-devel] 2 issues with qemu-master / 1.2 ehci code
Hi, 2) happens when a packet fails, and the queue should be halted, in this case Should we just cancel all queued packets on endpoint halts then? If the guest decides to go on we'll easily re-queue everything (with the existing code). If the guest does something else we don't have to do anything special. Not canceling, then trying to figure what the new state of the already queued packets is could become tricky ... Another problem with failing packets is that hw/usb/core.c will happily execute the next packet in the ep queue, even though the spec says the ep-queue should be halted, giving the guest a chance to cancel transfers after the failed one without them ever executing. I've a poc patch fixing this too. Indeed, the core should stop processing them. Question is what to do then. If the host controller cancels all packets anyway we don't have to do much extra work on the core. Just stop processing on error and implicitly un-halt the endpoint when the queue becomes empty. Maybe some extra state tracking and asserts() to catch bugs. cheers, Gerd
Re: [Qemu-devel] [PATCH] block: handle filenames with colons better
On Thu, Aug 16, 2012 at 11:24:11PM +0400, Michael Tokarev wrote: On 16.08.2012 18:58, Iustin Pop wrote: Commit 947995c (block: protect path_has_protocol from filenames with colons) introduced a way to handle filenames with colons based on whether the path contains a slash or not. IMHO this is not optimal, since we shouldn't rely on the contents of the path but rather on whether the given path exists as a file or not. As such, this patch tries to handle both files with and without slashes by falling back to opening them as files if no drivers supporting the protocol has been identified. I for one dislike this idea entirely: I think there should be a way to stop qemu from trying to open something as a file. It opens a security hole after all, what if such a file will actually exist? I'm not sure I understand the concern here. You pass what is a file path (and not an existing protocol path), and you want qemu not to open it? Or are you worried that a typo in the protocol name can lead to attacks? If I can vote, I'm voting against this with both hands. It's fine to have a way to stop QEMU opening something as a file, but please tell me how I can make it so that qemu -hda x:0 works for both regular files and block/char devices. Right now, it behaves differently for these two, and from the code it looks like this difference is rather accidental than intentional. regards, iustin
Re: [Qemu-devel] 2 issues with qemu-master / 1.2 ehci code
Hi, On 08/17/2012 09:07 AM, Gerd Hoffmann wrote: Hi, 2) happens when a packet fails, and the queue should be halted, in this case Should we just cancel all queued packets on endpoint halts then? If the guest decides to go on we'll easily re-queue everything (with the existing code). If the guest does something else we don't have to do anything special. Not canceling, then trying to figure what the new state of the already queued packets is could become tricky ... Yeah, actually I've come to the same conclusion, after wasting almost a day trying to get things to work the figure what the new state of the already queued packets is way, and that indeed is not the way to go! Spo I've just written a patch cancelling all the queued up packets, testing that now :) Another problem with failing packets is that hw/usb/core.c will happily execute the next packet in the ep queue, even though the spec says the ep-queue should be halted, giving the guest a chance to cancel transfers after the failed one without them ever executing. I've a poc patch fixing this too. Indeed, the core should stop processing them. Question is what to do then. If the host controller cancels all packets anyway we don't have to do much extra work on the core. Just stop processing on error and implicitly un-halt the endpoint when the queue becomes empty. Maybe some extra state tracking and asserts() to catch bugs. Right, also done in my wip patch. I'll go test it, then split it up in multiple patches and then submit. We really should get this in 1.2 btw!
Re: [Qemu-devel] [PATCH] linux-user: fix emulation of getdents
CC'ed to linux-user maintainer, Riku.
On Fri, Aug 17, 2012 at 01:20:19AM +0400, Dmitry V. Levin wrote:
In case when TARGET_ABI_BITS == 32 HOST_LONG_BITS == 64, the last
byte of the target dirent structure (aka d_type byte) was never copied
from the native dirent structure, thus breaking everything that relies
on valid d_type value, e.g. glob(3).
Signed-off-by: Dmitry V. Levin l...@altlinux.org
---
linux-user/syscall.c | 7 +++
linux-user/syscall_defs.h | 3 ++-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 41c869b..2b6025b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7030,10 +7030,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
tde-d_ino = tswapal(de-d_ino);
tde-d_off = tswapal(de-d_off);
tnamelen = treclen - (2 * sizeof(abi_long) + 2);
- if (tnamelen 256)
-tnamelen = 256;
-/* XXX: may not be correct */
-pstrcpy(tde-d_name, tnamelen, de-d_name);
+ if (tnamelen sizeof(tde-d_name))
+tnamelen = sizeof(tde-d_name);
+memcpy(tde-d_name, de-d_name, tnamelen);
de = (struct linux_dirent *)((char *)de + reclen);
len -= reclen;
tde = (struct target_dirent *)((char *)tde + treclen);
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 2cfda5a..f27a8d4 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -258,7 +258,8 @@ struct target_dirent {
abi_longd_ino;
abi_longd_off;
unsigned short d_reclen;
- chard_name[256]; /* We must not include limits.h! */
+ chard_name[257];/* We must not include limits.h! */
+ /* 257 = NAME_MAX + '\0' + d_type */
};
struct target_dirent64 {
--
ldv
--
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] memory: could we add extra input param for memory_region_init_io()?
On Fri, Aug 17, 2012 at 10:52 AM, liu ping fan qemul...@gmail.com wrote:
On Thu, Aug 16, 2012 at 5:23 PM, Avi Kivity a...@redhat.com wrote:
On 08/16/2012 06:22 AM, liu ping fan wrote:
On Tue, Aug 14, 2012 at 6:49 PM, Avi Kivity a...@redhat.com wrote:
On 08/14/2012 11:30 AM, liu ping fan wrote:
To make memoryRegion survive without the protection of qemu big lock,
we need to pin its based Object.
In current code, the type of mr-opaque are quite different. Lots of
them are Object, but quite of them are not yet.
The most challenge for changing from memory_region_init_io(..., void
*opaque, ...) to memory_region_init_io(..., Object *opaque,...) is
such codes:
hw/ide/cmd646.c:
static void setup_cmd646_bar(PCIIDEState *d, int bus_num)
{
IDEBus *bus = d-bus[bus_num];
CMD646BAR *bar = d-cmd646_bar[bus_num];
bar-bus = bus;
bar-pci_dev = d;
memory_region_init_io(bar-cmd, cmd646_cmd_ops, bar, cmd646-cmd,
4);
memory_region_init_io(bar-data, cmd646_data_ops, bar,
cmd646-data, 8);
}
If we passed in mr's based Object @d to substitute @bar, then we can
not pass the extra info @bus_num.
To solve such issue, introduce extra member Object *base for
MemoryRegion.
diff --git a/memory.c b/memory.c
index 643871b..afd5dea 100644
--- a/memory.c
+++ b/memory.c
@@ -931,6 +931,7 @@ static void memory_region_dispatch_write(MemoryRegion
*mr,
void memory_region_init_io(MemoryRegion *mr,
const MemoryRegionOps *ops,
+ Object *base,
void *opaque,
const char *name,
uint64_t size)
@@ -938,6 +939,7 @@ void memory_region_init_io(MemoryRegion *mr,
memory_region_init(mr, name, size);
mr-ops = ops;
mr-opaque = opaque;
+mr-base = base;
mr-terminates = true;
mr-destructor = memory_region_destructor_iomem;
mr-ram_addr = ~(ram_addr_t)0;
diff --git a/memory.h b/memory.h
index bd1bbae..2746e70 100644
--- a/memory.h
+++ b/memory.h
@@ -120,6 +120,7 @@ struct MemoryRegion {
/* All fields are private - violators will be prosecuted */
const MemoryRegionOps *ops;
void *opaque;
+Object *base;
MemoryRegion *parent;
Int128 size;
target_phys_addr_t addr;
Any comment?
I prefer that we convert the third parameter (opaque) to be an Object.
That is a huge change, but I think it will improve the code base overall.
Object may be many different opaque, and each has different
MemoryRegionOps. We need to pass in both object and opaque.
Why? Usually there's a 1:1 mapping between object and opaque. Can you
show cases where there isn't?
As mentioned ahead, setup_cmd646_bar(PCIIDEState *d, int bus_num),
Object=@d, but opaque are
d-cmd646_bar[bus_num], so that is 1:n mapping. And when I browsing
the code, this is the main issue prevent us to transfer from void* to
Object* for memory_region_init_io()
Maybe we can use Object's property to store the pair (mr, opaque),
then we can use mr as key to get opaque in mmio-dispatch, but the
property's query will hurt the performance.
Or define a new struct X {Object *base, void *opaque}, and pass it to
memory_region_init_io() to substitute void *opaque . Finally,
reclaim X in memory_region_destroy().
Usually the access callback can just cast the object to the real type.
That's all that's needed.
OK, I see
Other options are:
1) add MemoryRegionOps::ref(MemoryRegion *) and ::unref(MemoryRegion *)
If NULL, these callbacks are ignored. If not, they are called with the
MemoryRegion as a parameter. Their responsibility is to derive the
Object from the MemoryRegion (through the opaque or using
container_of()) and ref or unref it respectively.
2) add Object *MemoryRegionOps::object(MemoryRegion *)
Similar; if NULL it is ignored, otherwise it is used to derive the
Object, which the memory core will ref and unref.
3) add bool MemoryRegionOps::opaque_is_object
Tells the memory core that it is safe to cast the opaque into an Object.
Above methods, the process of derive the Object will be hard, we can
not tell opaque is Object or not without something like trycatch
Take for example e1000. It passes E1000State as the opaque, which is a
PCIDevice, which is a DeviceState, which is an Object. So for that
device, nothing needs to be done.
The same example, in setup_cmd646_bar(PCIIDEState *d, int bus_num), I
think we can not decide which is the type for @bar. If using
object_dynamic_cast(@bar, TYPE_OBJECT) to tell whether it is Object or
not, it will raise exception.
And something like omap_mpu_timer_init() in file hw/omap1.c , the
opaque(omap_mpu_timer_s) is got from g_malloc0, which makes things
even harder to handle. And the DO_CAST can not work for such issue.
Regards, pingfan
4) add memory_region_set_object(MemoryRegion *, Object *)
Like your proposal, but avoids adding an extra
Re: [Qemu-devel] x86, nops settings result in kernel crash
- Original Message - On 08/16/2012 11:53 AM, Alan Cox wrote: Yes, if I remove the break statement (introduced by this commit), it works fine. What version of qemu is this - do we have qemu bug here I wonder. Also, is it 32 or 64 bits? It's 64-bit. Regards, Tomas -hpa
Re: [Qemu-devel] [PATCH] block: handle filenames with colons better
Am 17.08.2012 09:15, schrieb Iustin Pop: On Thu, Aug 16, 2012 at 11:24:11PM +0400, Michael Tokarev wrote: On 16.08.2012 18:58, Iustin Pop wrote: Commit 947995c (block: protect path_has_protocol from filenames with colons) introduced a way to handle filenames with colons based on whether the path contains a slash or not. IMHO this is not optimal, since we shouldn't rely on the contents of the path but rather on whether the given path exists as a file or not. As such, this patch tries to handle both files with and without slashes by falling back to opening them as files if no drivers supporting the protocol has been identified. I for one dislike this idea entirely: I think there should be a way to stop qemu from trying to open something as a file. It opens a security hole after all, what if such a file will actually exist? I'm not sure I understand the concern here. You pass what is a file path (and not an existing protocol path), and you want qemu not to open it? Or are you worried that a typo in the protocol name can lead to attacks? That, or just the fact that you don't know exactly which protocols are supported by this qemu binary and expect one that isn't there. The other concern I have is about the opposite direction: When a new qemu version introduces a new protocol, the same string that meant a file before would start meaning the protocol, which makes it an incompatible change. Essentially, that would mean that we can't add any new protocols any more. Doesn't sound like a great idea to me. If I can vote, I'm voting against this with both hands. It's fine to have a way to stop QEMU opening something as a file, but please tell me how I can make it so that qemu -hda x:0 works for both regular files and block/char devices. Right now, it behaves differently for these two, and from the code it looks like this difference is rather accidental than intentional. It was probably accidental in the beginning, but it's now a well-known misfeature that we can't get rid of for compatibility reasons. People rely on devices with colons being accessible this way. (We once changed that, but had to take this part back) Kevin
Re: [Qemu-devel] x86, nops settings result in kernel crash
On Fri, Aug 17, 2012 at 03:43:56AM -0400, Tomas Racek wrote:
Well, I've added some debug statements to the code:
void __init arch_init_ideal_nops(void)
{
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_INTEL:
/*
* Due to a decoder implementation quirk, some
* specific Intel CPUs actually perform better with
* the k8_nops than with the SDM-recommended NOPs.
*/
if (boot_cpu_data.x86 == 6
boot_cpu_data.x86_model = 0x0f
boot_cpu_data.x86_model != 0x1c
boot_cpu_data.x86_model != 0x26
boot_cpu_data.x86_model != 0x27
boot_cpu_data.x86_model 0x30) {
printk(NOPS: Option 1\n);
ideal_nops = k8_nops;
} else if (boot_cpu_has(X86_FEATURE_NOPL)) {
printk(NOPS: Option 2\n);
ideal_nops = p6_nops;
} else {
printk(NOPS: Option 3\n);
#ifdef CONFIG_X86_64
ideal_nops = k8_nops;
#else
ideal_nops = intel_nops;
#endif
}
break;
default:
#ifdef CONFIG_X86_64
ideal_nops = k8_nops;
#else
if (boot_cpu_has(X86_FEATURE_K8))
ideal_nops = k8_nops;
else if (boot_cpu_has(X86_FEATURE_K7))
ideal_nops = k7_nops;
else
ideal_nops = intel_nops;
#endif
}
}
This gives me Option 1 with -cpu host and Option 2 without.
This looks like an emulation bug. The interesting thing is that your
both traces from the bugzilla point to generic_make_request_checks but
it could also be due to timing.
Decoding the instruction stream in the second trace in the bugzilla gives:
[ 278.595106] Code: 03 48 89 03 48 8b 57 70 48 89 53 10 48 2b 01 8b 3f 48 89 45
98 48 8b 82 90 00 00 00 89 7d 94 48 8b 80 60 02 00 00 48 89 45 88 ac 17 00 00
c8 45 85 e4 74 30 48 8b 43 10 48 8b 40 08 48 8b 40 48
All code
0: 03 48 89add-0x77(%rax),%ecx
3: 03 48 8badd-0x75(%rax),%ecx
6: 57 push %rdi
7: 70 48 jo 0x51
9: 89 53 10mov%edx,0x10(%rbx)
c: 48 2b 01sub(%rcx),%rax
f: 8b 3f mov(%rdi),%edi
11: 48 89 45 98 mov%rax,-0x68(%rbp)
15: 48 8b 82 90 00 00 00mov0x90(%rdx),%rax
1c: 89 7d 94mov%edi,-0x6c(%rbp)
1f: 48 8b 80 60 02 00 00mov0x260(%rax),%rax
26: 48 89 45 88 mov%rax,-0x78(%rbp)
2a: ac lods %ds:(%rsi),%al
2b:* 17 (bad) -- trapping instruction
2c: 00 00 add%al,(%rax)
2e: c8 45 85 e4 enterq $0x8545,$0xe4
32: 74 30 je 0x64
34: 48 8b 43 10 mov0x10(%rbx),%rax
38: 48 8b 40 08 mov0x8(%rax),%rax
3c: 48 8b 40 48 mov0x48(%rax),%rax
...
Code starting with the faulting instruction
===
0: 17 (bad)
1: 00 00 add%al,(%rax)
3: c8 45 85 e4 enterq $0x8545,$0xe4
7: 74 30 je 0x39
9: 48 8b 43 10 mov0x10(%rbx),%rax
d: 48 8b 40 08 mov0x8(%rax),%rax
11: 48 8b 40 48 mov0x48(%rax),%rax
and an instruction with opcode 0x17 in 64-bit mode is, AFAICT,
invalid (on 32-bit it is pop %ss according to this thing:
http://www.onlinedisassembler.com).
--
Regards/Gruss,
Boris.
Re: [Qemu-devel] How does ARM VFP is emulated?
On Thursday, August 16, 2012, Oi Khote oikh...@hotmail.com wrote: So how exactly does VFP is being emulated. QEMU uses a library for FP computations, based on the softfloat package. Laurent
[Qemu-devel] [PATCH v2] register reset handler to write image into memory
Instead of add rom blobs, this patch just write them directly to memory.
This patch registers reset handler uimage_reset() and image_file_reset()
which load images into RAM during initial bootup and VM reset.
v2: use g_file_get_content() to load load image file.
Signed-off-by: Olivia Yin hong-hua@freescale.com
---
This patch is based on branch 'ppc-next' of Alex's upstream QEMU repo:
http://repo.or.cz/r/qemu/agraf.git
hw/loader.c | 88 ---
1 files changed, 66 insertions(+), 22 deletions(-)
diff --git a/hw/loader.c b/hw/loader.c
index 33acc2f..0be8bf7 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -56,6 +56,12 @@
static int roms_loaded;
+typedef struct ImageFile ImageFile;
+struct ImageFile {
+char *name;
+target_phys_addr_t addr;
+};
+
/* return the size or -1 if error */
int get_image_size(const char *filename)
{
@@ -86,6 +92,24 @@ int load_image(const char *filename, uint8_t *addr)
return size;
}
+static void image_file_reset(void *opaque)
+{
+ImageFile *image = opaque;
+GError *err = NULL;
+gboolean res;
+gchar *content;
+gsize size;
+
+res = g_file_get_contents(image-name, content, size, err);
+if (res == FALSE) {
+ error_report(failed to read image file: %s\n, image-name);
+ g_error_free(err);
+} else {
+ cpu_physical_memory_rw(image-addr, (uint8_t *)content, size, 1);
+ g_free(content);
+}
+}
+
/* read()-like version */
ssize_t read_targphys(const char *name,
int fd, target_phys_addr_t dst_addr, size_t nbytes)
@@ -112,7 +136,12 @@ int load_image_targphys(const char *filename,
return -1;
}
if (size 0) {
-rom_add_file_fixed(filename, addr, -1);
+ImageFile *image;
+image = g_malloc0(sizeof(*image));
+image-name = g_strdup(filename);
+image-addr = addr;
+
+qemu_register_reset(image_file_reset, image);
}
return size;
}
@@ -433,15 +462,14 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t
*src,
return dstbytes;
}
-/* Load a U-Boot image. */
-int load_uimage(const char *filename, target_phys_addr_t *ep,
-target_phys_addr_t *loadaddr, int *is_linux)
+/* write uimage into memory */
+static int uimage_physical_loader(const char *filename, uint8_t **data,
+ target_phys_addr_t *loadaddr)
{
int fd;
int size;
uboot_image_header_t h;
uboot_image_header_t *hdr = h;
-uint8_t *data = NULL;
int ret = -1;
fd = open(filename, O_RDONLY | O_BINARY);
@@ -474,18 +502,9 @@ int load_uimage(const char *filename, target_phys_addr_t
*ep,
goto out;
}
-/* TODO: Check CPU type. */
-if (is_linux) {
-if (hdr-ih_os == IH_OS_LINUX)
-*is_linux = 1;
-else
-*is_linux = 0;
-}
-
-*ep = hdr-ih_ep;
-data = g_malloc(hdr-ih_size);
+*data = g_malloc(hdr-ih_size);
-if (read(fd, data, hdr-ih_size) != hdr-ih_size) {
+if (read(fd, *data, hdr-ih_size) != hdr-ih_size) {
fprintf(stderr, Error reading file\n);
goto out;
}
@@ -495,11 +514,11 @@ int load_uimage(const char *filename, target_phys_addr_t
*ep,
size_t max_bytes;
ssize_t bytes;
-compressed_data = data;
+compressed_data = *data;
max_bytes = UBOOT_MAX_GUNZIP_BYTES;
-data = g_malloc(max_bytes);
+*data = g_malloc(max_bytes);
-bytes = gunzip(data, max_bytes, compressed_data, hdr-ih_size);
+bytes = gunzip(*data, max_bytes, compressed_data, hdr-ih_size);
g_free(compressed_data);
if (bytes 0) {
fprintf(stderr, Unable to decompress gzipped image!\n);
@@ -508,7 +527,6 @@ int load_uimage(const char *filename, target_phys_addr_t
*ep,
hdr-ih_size = bytes;
}
-rom_add_blob_fixed(filename, data, hdr-ih_size, hdr-ih_load);
if (loadaddr)
*loadaddr = hdr-ih_load;
@@ -516,12 +534,38 @@ int load_uimage(const char *filename, target_phys_addr_t
*ep,
ret = hdr-ih_size;
out:
-if (data)
-g_free(data);
close(fd);
return ret;
}
+static void uimage_reset(void *opaque)
+{
+ImageFile *image = opaque;
+uint8_t *data = NULL;
+int size;
+
+size = uimage_physical_loader(image-name, data, image-addr);
+cpu_physical_memory_rw(image-addr, data, size, 1);
+g_free(data);
+}
+
+/* Load a U-Boot image. */
+int load_uimage(const char *filename, target_phys_addr_t *ep,
+target_phys_addr_t *loadaddr, int *is_linux)
+{
+int size;
+ImageFile *image;
+uint8_t *data = NULL;
+
+size= uimage_physical_loader(filename, data, loadaddr);
+g_free(data);
+image = g_malloc0(sizeof(*image));
+image-name = g_strdup(filename);
+image-addr = *loadaddr;
+qemu_register_reset(uimage_reset, image);
+return size;
+}
+
/*
*
Re: [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command.
Am 15.08.2012 20:48, schrieb Benoît Canet:
This additionnal --machine=json option make qemu-img info output on
stdout a JSON formated representation of the image informations.
--machine=json was choosen instead of --format=json because the
info command already have a -f parameter.
Which is not a problem. A machine doesn't care about shortcut options,
always specifying the long option is fine.
However, I wouldn't call that option --format (which would intuitively
relate to the image format) nor --machine (--machine=human looks really
weird), but --output=(json|human)
#ifdef _WIN32
@@ -84,6 +88,7 @@ static void help(void)
'-p' show progress of command (only certain commands)\n
'-S' indicates the consecutive number of bytes that must
contain only zeros\n
for qemu-img to create a sparse image during conversion\n
+ '-m' or '--machine' takes the format in which the output must
be done (json)\n
You should mention both options, not only the non-default one.
\n
Parameters to check subcommand:\n
'-r' tries to repair any inconsistencies that are found during
the check.\n
@@ -1102,21 +1107,86 @@ static void dump_snapshots(BlockDriverState *bs)
g_free(sn_tab);
}
+static void collect_snapshots(BlockDriverState *bs , ImageInfo *image_info)
+{
+int i, sn_count;
+QEMUSnapshotInfo *sn_tab = NULL;
+SnapshotInfoList *sn_info_list, *cur_item = NULL;
+sn_count = bdrv_snapshot_list(bs, sn_tab);
+
+for (i = 0; i sn_count; i++) {
+image_info-has_snapshots = true;
+sn_info_list = g_new0(SnapshotInfoList, 1);
+
+sn_info_list-value = g_new0(SnapshotInfo, 1);
+sn_info_list-value-id = g_strdup(sn_tab[i].id_str);
+sn_info_list-value-name = g_strdup(sn_tab[i].name);
+sn_info_list-value-vm_state_size = sn_tab[i].vm_state_size;
+sn_info_list-value-date_sec = sn_tab[i].date_sec;
+sn_info_list-value-date_nsec = sn_tab[i].date_nsec;
+sn_info_list-value-vm_clock_nsec = sn_tab[i].vm_clock_nsec;
Aligning the = to the same column wouldn't hurt.
+
+/* XXX: waiting for the qapi to support GSList */
Is this even planned? I would agree with qemu-queue.h structures
available from QAPI, but not GSList. Please change or remove the comment.
+if (!cur_item) {
+image_info-snapshots = cur_item = sn_info_list;
+} else {
+cur_item-next = sn_info_list;
+cur_item = sn_info_list;
+}
+
+}
+
+g_free(sn_tab);
+}
+
+static void dump_json_image_info(ImageInfo *image_info)
+{
+Error *errp = NULL;
+QString *str;
+QmpOutputVisitor *ov = qmp_output_visitor_new();
+QObject *obj;
+visit_type_ImageInfo(qmp_output_get_visitor(ov),
+ image_info, NULL, errp);
+obj = qmp_output_get_qobject(ov);
+str = qobject_to_json_pretty(obj);
+assert(str != NULL);
+printf(%s\n, qstring_get_str(str));
+qobject_decref(obj);
+qmp_output_visitor_cleanup(ov);
+QDECREF(str);
+}
+
+#define PRINTH(human, args...) do { \
+if (human) {\
+printf(args); \
+} } while (0);
Extra semicolon,
The existence of this macro doesn't look quite right at the first sight,
but let me check how it's used.
+
static int img_info(int argc, char **argv)
{
int c;
-const char *filename, *fmt;
-BlockDriverState *bs;
+bool human = true;
+const char *filename, *fmt, *machine;
+BlockDriverState *bs, *backing_bs = NULL;
char size_buf[128], dsize_buf[128];
uint64_t total_sectors;
int64_t allocated_size;
char backing_filename[1024];
char backing_filename2[1024];
BlockDriverInfo bdi;
+ImageInfo *image_info;
fmt = NULL;
+machine = NULL;
for(;;) {
-c = getopt(argc, argv, f:h);
+int option_index = 0;
+static struct option long_options[] = {
+{help, no_argument, 0, 'h'},
+{format, required_argument, 0, 'f'},
+{machine, required_argument, 0, 'm'},
+{0, 0, 0, 0}
+};
+c = getopt_long(argc, argv, f:h,
+long_options, option_index);
if (c == -1) {
break;
}
@@ -1128,6 +1198,9 @@ static int img_info(int argc, char **argv)
case 'f':
fmt = optarg;
break;
+case 'm':
+machine = optarg;
+break;
}
}
if (optind = argc) {
@@ -1135,8 +1208,14 @@ static int img_info(int argc, char **argv)
}
filename = argv[optind++];
+image_info = g_new0(ImageInfo, 1);
+if (machine !strncmp(machine, json, strlen(json))) {
+human = false;
+}
Check against the valid values and exit with an error if it's
[Qemu-devel] [PATCH 1/3] usb: Halt ep queue en cancel pending packets on a packet error
For controllers which queue up more then 1 packet at a time, we must halt the
ep queue, and inside the controller code cancel all pending packets on an
error.
There are multiple reasons for this:
1) Guests expect the controllers to halt ep queues on error, so that they
get the opportunity to cancel transfers which the scheduled after the failing
one, before processing continues
2) Not cancelling queued up packets after a failed transfer also messes up
the controller state machine, in the case of EHCI causing the following
assert to trigger: assert(p-qtdaddr == q-qtdaddr) at hcd-ehci.c:2075
3) For bulk endpoints with pipelining enabled (redirection to a real USB
device), we must cancel all the transfers after this a failed one so that:
a) If they've completed already, they are not processed further causing more
stalls to be reported, originating from the same failed transfer
b) If still in flight, they are cancelled before the guest does
a clear stall, otherwise the guest and device can loose sync!
Note this patch only touches the ehci and uhci controller changes, since AFAIK
no other controllers actually queue up multiple transfer. If I'm wrong on this
other controllers need to be updated too!
Also note that this patch was heavily tested with the ehci code, where I had
a reproducer for a device causing a transfer to fail. The uhci code is not
tested with actually failing transfers and could do with a thorough review!
Signed-off-by: Hans de Goede hdego...@redhat.com
---
hw/usb.h | 1 +
hw/usb/core.c | 32 +---
hw/usb/hcd-ehci.c | 13 +
hw/usb/hcd-uhci.c | 16
4 files changed, 55 insertions(+), 7 deletions(-)
diff --git a/hw/usb.h b/hw/usb.h
index 432ccae..e574477 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -179,6 +179,7 @@ struct USBEndpoint {
uint8_t ifnum;
int max_packet_size;
bool pipeline;
+bool halted;
USBDevice *dev;
QTAILQ_HEAD(, USBPacket) queue;
};
diff --git a/hw/usb/core.c b/hw/usb/core.c
index c7e5bc0..7ed0e45 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -382,12 +382,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
usb_packet_check_state(p, USB_PACKET_SETUP);
assert(p-ep != NULL);
+/* Submitting a new packet clears halt */
+p-ep-halted = false;
+
if (QTAILQ_EMPTY(p-ep-queue) || p-ep-pipeline) {
ret = usb_process_one(p);
if (ret == USB_RET_ASYNC) {
usb_packet_set_state(p, USB_PACKET_ASYNC);
QTAILQ_INSERT_TAIL(p-ep-queue, p, queue);
} else {
+/*
+ * When pipelining is enabled usb-devices must always return async,
+ * otherwise packets can complete out of order!
+ */
+assert(!p-ep-pipeline);
p-result = ret;
usb_packet_set_state(p, USB_PACKET_COMPLETE);
}
@@ -402,6 +410,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
/* Notify the controller that an async packet is complete. This should only
be called for packets previously deferred by returning USB_RET_ASYNC from
handle_packet. */
+static void __usb_packet_complete(USBDevice *dev, USBPacket *p)
+{
+USBEndpoint *ep = p-ep;
+
+assert(p-result != USB_RET_ASYNC p-result != USB_RET_NAK);
+
+if (p-result 0) {
+ep-halted = true;
+}
+usb_packet_set_state(p, USB_PACKET_COMPLETE);
+QTAILQ_REMOVE(ep-queue, p, queue);
+dev-port-ops-complete(dev-port, p);
+}
+
void usb_packet_complete(USBDevice *dev, USBPacket *p)
{
USBEndpoint *ep = p-ep;
@@ -409,11 +431,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
usb_packet_check_state(p, USB_PACKET_ASYNC);
assert(QTAILQ_FIRST(ep-queue) == p);
-usb_packet_set_state(p, USB_PACKET_COMPLETE);
-QTAILQ_REMOVE(ep-queue, p, queue);
-dev-port-ops-complete(dev-port, p);
+__usb_packet_complete(dev, p);
-while (!QTAILQ_EMPTY(ep-queue)) {
+while (!ep-halted !QTAILQ_EMPTY(ep-queue)) {
p = QTAILQ_FIRST(ep-queue);
if (p-state == USB_PACKET_ASYNC) {
break;
@@ -425,9 +445,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
break;
}
p-result = ret;
-usb_packet_set_state(p, USB_PACKET_COMPLETE);
-QTAILQ_REMOVE(ep-queue, p, queue);
-dev-port-ops-complete(dev-port, p);
+__usb_packet_complete(ep-dev, p);
}
}
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 41d663d..a6b8527 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2137,6 +2137,19 @@ static int ehci_state_writeback(EHCIQueue *q)
* bit is clear.
*/
if (q-qh.token QTD_TOKEN_HALT) {
+/*
+ * We should not do any further processing on a halted queue!
+ * This is esp. important for bulk endpoints with pipelining enabled
+ * (redirection to a real USB device), where we must cancel all the
+ * transfers
[Qemu-devel] [PATCH 3/3] ehci: simplify ehci_state_executing
ehci_state_executing does not need to check for p-usb_status == USB_RET_ASYNC
or USB_RET_PROCERR, since ehci_execute_complete already does a similar check
and will trigger an assert if either value is encountered.
USB_RET_ASYNC should never be the packet status when execute_complete runs
for obvious reasons, and USB_RET_PROCERR is only used by ehci_state_execute /
ehci_execute not by ehci_state_executing / ehci_execute_complete.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
hw/usb/hcd-ehci.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 4750175..378b42b 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2070,19 +2070,11 @@ out:
static int ehci_state_executing(EHCIQueue *q)
{
EHCIPacket *p = QTAILQ_FIRST(q-packets);
-int again = 0;
assert(p != NULL);
assert(p-qtdaddr == q-qtdaddr);
ehci_execute_complete(q);
-if (p-usb_status == USB_RET_ASYNC) {
-goto out;
-}
-if (p-usb_status == USB_RET_PROCERR) {
-again = -1;
-goto out;
-}
// 4.10.3
if (!q-async) {
@@ -2100,11 +2092,8 @@ static int ehci_state_executing(EHCIQueue *q)
ehci_set_state(q-ehci, q-async, EST_WRITEBACK);
}
-again = 1;
-
-out:
ehci_flush_qh(q);
-return again;
+return 1;
}
--
1.7.11.4
[Qemu-devel] [PATCH 2/3] usb: controllers do not need to check for babble themselves
If an (emulated) usb-device tries to write more data to a packet then
its iov len, this will trigger an assert in usb_packet_copy(), and if
a driver somehow circumvents that check and writes more data to the
iov then there is space, we have a much bigger problem then not correctly
reporting babble to the guest.
In practice babble will only happen with (real) redirected devices, and there
both the usb-host os and the qemu usb-device code already check for it.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
hw/usb/hcd-ehci.c | 4
hw/usb/hcd-uhci.c | 5 -
2 files changed, 9 deletions(-)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index a6b8527..4750175 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1460,10 +1460,6 @@ static void ehci_execute_complete(EHCIQueue *q)
assert(0);
break;
}
-} else if ((p-usb_status p-tbytes) (p-pid == USB_TOKEN_IN)) {
-p-usb_status = USB_RET_BABBLE;
-q-qh.token |= (QTD_TOKEN_HALT | QTD_TOKEN_BABBLE);
-ehci_raise_irq(q-ehci, USBSTS_ERRINT);
} else {
// TODO check 4.12 for splits
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 8987734..4e43d20 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -729,11 +729,6 @@ static int uhci_complete_td(UHCIState *s, UHCI_TD *td,
UHCIAsync *async, uint32_
*int_mask |= 0x01;
if (pid == USB_TOKEN_IN) {
-if (len max_len) {
-ret = USB_RET_BABBLE;
-goto out;
-}
-
if ((td-ctrl TD_CTRL_SPD) len max_len) {
*int_mask |= 0x02;
/* short packet: do not update QH */
--
1.7.11.4
Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing
Hi Peter,
Thanks for the reminder. I'll update the second patch to use
g_file_get_contents().
Best Regards,
Olivia
-Original Message-
From: Peter Maydell [mailto:peter.mayd...@linaro.org]
Sent: Thursday, August 16, 2012 7:36 PM
To: Yin Olivia-R63875
Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from
rom_add_file() for reusing
On 14 August 2012 08:49, Olivia Yin hong-hua@freescale.com wrote:
Sanity check in rom_add_file() could be reused by other image loaders.
Signed-off-by: Olivia Yin hong-hua@freescale.com
---
This patch is based on branch 'ppc-next' of Alex's upstream QEMU repo:
http://repo.or.cz/r/qemu/agraf.git
hw/loader.c | 61 +++
---
1 files changed, 33 insertions(+), 28 deletions(-)
diff --git a/hw/loader.c b/hw/loader.c index 33acc2f..f2099b6 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -86,6 +86,36 @@ int load_image(const char *filename, uint8_t *addr)
return size;
}
+static int file_load(const char *file, uint8_t **data) {
+int fd = -1;
+ssize_t rc, size;
+
+fd = open(file, O_RDONLY | O_BINARY);
+if (fd == -1) {
+fprintf(stderr, Could not open file '%s': %s\n,
+file, strerror(errno));
+return -1;
+}
+
+size = lseek(fd, 0, SEEK_END);
+*data = g_malloc0(size);
+lseek(fd, 0, SEEK_SET);
+rc = read(fd, *data, size);
+if (rc != size) {
+fprintf(stderr, file %-20s: read error: rc=%zd
(expected %zd)\n,
+file, rc, size);
+goto err;
+}
+close(fd);
+return size;
+err:
+if (fd != -1)
+close(fd);
+g_free(*data);
+return -1;
+}
Isn't this function effectively a reimplementation of the glib
g_file_get_contents() function? It would probably be better to just make
the callers use that instead.
-- PMM
Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing
Hi Avi, Thanks. Exactly the second patch is more important which saves the QEMU memory. Best Regards, Olivia -Original Message- From: Avi Kivity [mailto:a...@redhat.com] Sent: Thursday, August 16, 2012 7:23 PM To: Yin Olivia-R63875 Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; blauwir...@gmail.com; b...@kernel.crashing.org Subject: Re: [PATCH 1/2] extract file_load() function from rom_add_file() for reusing On 08/16/2012 02:13 PM, Yin Olivia-R63875 wrote: Hi Avi Ben, I've got feedback from qemu-ppc list and revised this patch according to Blue's comments. Could you please give any comments? I don't really have any comments, those files are outside my area of expertise. They look okay to me, but that doesn't mean much as I'm not familiar with ppc. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [Bug 1037606] ProcModules.txt
apport information ** Attachment added: ProcModules.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265239/+files/ProcModules.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1037606] WifiSyslog.txt
apport information ** Attachment added: WifiSyslog.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265242/+files/WifiSyslog.txt ** Description changed: vmwgfx driver fails to initialize inside kvm. - tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) - --- + tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu + based, any Ubuntu live CD would do) + + Apport data collected with qantal alpha live CD (somewhat older kernel). + + The error is shjown in CurrentDmesg.txt + https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt + + --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: - eth0 no wireless extensions. - - lono wireless extensions. + eth0 no wireless extensions. + + lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: - TERM=linux - PATH=(custom, no user) - LANG=en_US.UTF-8 - SHELL=/bin/bash + TERM=linux + PATH=(custom, no user) + LANG=en_US.UTF-8 + SHELL=/bin/bash ProcFB: - + ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: - linux-restricted-modules-3.5.0-6-generic N/A - linux-backports-modules-3.5.0-6-generic N/A - linux-firmware 1.85 + linux-restricted-modules-3.5.0-6-generic N/A + linux-backports-modules-3.5.0-6-generic N/A + linux-firmware 1.85 RfKill: - + Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: - + dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to:
[Qemu-devel] [Bug 1037606] ProcCpuinfo.txt
apport information ** Attachment added: ProcCpuinfo.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265237/+files/ProcCpuinfo.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1037606] UdevDb.txt
apport information ** Attachment added: UdevDb.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265240/+files/UdevDb.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1037606] ProcInterrupts.txt
apport information ** Attachment added: ProcInterrupts.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265238/+files/ProcInterrupts.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
Re: [Qemu-devel] [PATCH] block: handle filenames with colons better
On Fri, Aug 17, 2012 at 09:56:35AM +0200, Kevin Wolf wrote: Am 17.08.2012 09:15, schrieb Iustin Pop: On Thu, Aug 16, 2012 at 11:24:11PM +0400, Michael Tokarev wrote: On 16.08.2012 18:58, Iustin Pop wrote: Commit 947995c (block: protect path_has_protocol from filenames with colons) introduced a way to handle filenames with colons based on whether the path contains a slash or not. IMHO this is not optimal, since we shouldn't rely on the contents of the path but rather on whether the given path exists as a file or not. As such, this patch tries to handle both files with and without slashes by falling back to opening them as files if no drivers supporting the protocol has been identified. I for one dislike this idea entirely: I think there should be a way to stop qemu from trying to open something as a file. It opens a security hole after all, what if such a file will actually exist? I'm not sure I understand the concern here. You pass what is a file path (and not an existing protocol path), and you want qemu not to open it? Or are you worried that a typo in the protocol name can lead to attacks? That, or just the fact that you don't know exactly which protocols are supported by this qemu binary and expect one that isn't there. The other concern I have is about the opposite direction: When a new qemu version introduces a new protocol, the same string that meant a file before would start meaning the protocol, which makes it an incompatible change. Essentially, that would mean that we can't add any new protocols any more. Doesn't sound like a great idea to me. So in this sense, you prefer to have it remain so that files with colons are not accepted by qemu, if I understand you correctly, so that new procotol can be added, right? If I can vote, I'm voting against this with both hands. It's fine to have a way to stop QEMU opening something as a file, but please tell me how I can make it so that qemu -hda x:0 works for both regular files and block/char devices. Right now, it behaves differently for these two, and from the code it looks like this difference is rather accidental than intentional. It was probably accidental in the beginning, but it's now a well-known misfeature that we can't get rid of for compatibility reasons. People rely on devices with colons being accessible this way. (We once changed that, but had to take this part back) OK, then I think what is needed is to update the documentation then. I was not able to find any restrictions on the file names, hence this patch. I think what is needed is to actually add a new 'file:' protocol, that can open paths containing no matter what characters they contain (as long as the filesystem supports them, of course); as the current situation where files have no protocol but all the other do seems asymmetric. thanks, iustin
[Qemu-devel] [Bug 1037606] Re: vmwgfx does not work with kvm vmware vga
apport information ** Tags added: apport-collected quantal ** Description changed: vmwgfx driver fails to initialize inside kvm. - tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu - based, any Ubuntu live CD would do) + tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) + --- + ApportVersion: 2.4-0ubuntu8 + Architecture: amd64 + AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: + CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. + CasperVersion: 1.320 + DistroRelease: Ubuntu 12.10 + IwConfig: + eth0 no wireless extensions. + + lono wireless extensions. + LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) + Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 + MachineType: Bochs Bochs + Package: linux (not installed) + ProcEnviron: + TERM=linux + PATH=(custom, no user) + LANG=en_US.UTF-8 + SHELL=/bin/bash + ProcFB: + + ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity + ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 + PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. + RelatedPackageVersions: + linux-restricted-modules-3.5.0-6-generic N/A + linux-backports-modules-3.5.0-6-generic N/A + linux-firmware 1.85 + RfKill: + + Tags: quantal + Uname: Linux 3.5.0-6-generic x86_64 + UpgradeStatus: No upgrade log present (probably fresh install) + UserGroups: + + dmi.bios.date: 01/01/2007 + dmi.bios.vendor: Bochs + dmi.bios.version: Bochs + dmi.chassis.type: 1 + dmi.chassis.vendor: Bochs + dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: + dmi.product.name: Bochs + dmi.sys.vendor: Bochs ** Attachment added: AcpiTables.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265232/+files/AcpiTables.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
Re: [Qemu-devel] [PATCH] block: handle filenames with colons better
Am 17.08.2012 12:05, schrieb Iustin Pop: On Fri, Aug 17, 2012 at 09:56:35AM +0200, Kevin Wolf wrote: Am 17.08.2012 09:15, schrieb Iustin Pop: On Thu, Aug 16, 2012 at 11:24:11PM +0400, Michael Tokarev wrote: On 16.08.2012 18:58, Iustin Pop wrote: Commit 947995c (block: protect path_has_protocol from filenames with colons) introduced a way to handle filenames with colons based on whether the path contains a slash or not. IMHO this is not optimal, since we shouldn't rely on the contents of the path but rather on whether the given path exists as a file or not. As such, this patch tries to handle both files with and without slashes by falling back to opening them as files if no drivers supporting the protocol has been identified. I for one dislike this idea entirely: I think there should be a way to stop qemu from trying to open something as a file. It opens a security hole after all, what if such a file will actually exist? I'm not sure I understand the concern here. You pass what is a file path (and not an existing protocol path), and you want qemu not to open it? Or are you worried that a typo in the protocol name can lead to attacks? That, or just the fact that you don't know exactly which protocols are supported by this qemu binary and expect one that isn't there. The other concern I have is about the opposite direction: When a new qemu version introduces a new protocol, the same string that meant a file before would start meaning the protocol, which makes it an incompatible change. Essentially, that would mean that we can't add any new protocols any more. Doesn't sound like a great idea to me. So in this sense, you prefer to have it remain so that files with colons are not accepted by qemu, if I understand you correctly, so that new procotol can be added, right? The current rule is that protocols never contain slashes (and we won't add such protocols), and therefore anything containing a slash is a file name. I already felt a bit uncomfortable with that, but at least it introduces no ambiguity. You're implying that you can't use files that contain a colon. That's not true because you can include a slash in every file name. Instead of foo:bar write ./foo:bar and you get it interpreted as you wanted. If I can vote, I'm voting against this with both hands. It's fine to have a way to stop QEMU opening something as a file, but please tell me how I can make it so that qemu -hda x:0 works for both regular files and block/char devices. Right now, it behaves differently for these two, and from the code it looks like this difference is rather accidental than intentional. It was probably accidental in the beginning, but it's now a well-known misfeature that we can't get rid of for compatibility reasons. People rely on devices with colons being accessible this way. (We once changed that, but had to take this part back) OK, then I think what is needed is to update the documentation then. I was not able to find any restrictions on the file names, hence this patch. Sure, that makes sense. Care to send a patch? I think what is needed is to actually add a new 'file:' protocol, that can open paths containing no matter what characters they contain (as long as the filesystem supports them, of course); as the current situation where files have no protocol but all the other do seems asymmetric. This is what I've been advocating before commit 947995c, but I think other people had concerns (that I can't remember now). Kevin
[Qemu-devel] [Bug 1037606] UdevLog.txt
apport information ** Attachment added: UdevLog.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265241/+files/UdevLog.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1037606] CurrentDmesg.txt
apport information ** Attachment added: CurrentDmesg.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265235/+files/CurrentDmesg.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1037606] Lspci.txt
apport information ** Attachment added: Lspci.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265236/+files/Lspci.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
Re: [Qemu-devel] qemu log function to print out the registers of the guest
To verify what is translation time and what is the run time, I log the register information before disassembling each guest code. I copied some results from the log file, which is generated at run time of a guest machine. EAX= EBX=6ffc IN: 0x000f2087: mov$0xf5588,%eax EAX=000f5588 EBX=6ffc IN: 0x000f208B: move 0x4(%ebx) %eax The first instruction load eax with the value 0xf5588, so the eax at the second instruction is EAX=000f5588. So can I consider the memory address of 0x4(%ebx) as (6ffc + 4)? I think this should be the run time information I need. Please correct me if there is anything wrong. Thanks. IIRC, -d in_asm only give you what has been translated not executed. Remember you said you log the register information before disassembling each guest code? In other words, (guest) ebx might not be the value you saw here. This is just my opinion. Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] [PATCH 1/3] usb: Halt ep queue en cancel pending packets on a packet error
Hi,
Note this patch only touches the ehci and uhci controller changes, since AFAIK
no other controllers actually queue up multiple transfer. If I'm wrong on this
other controllers need to be updated too!
xhci does it too (although it is hard to test as xhci can happily submit
256k transfers where ehci and uhci have to use a bunch of smaller
packets instead).
Some minor nits below (the other two patches look good):
+/* Submitting a new packet clears halt */
+p-ep-halted = false;
check that the queue is empty when halted is set (i.e. enforce cancel on
error) ?
+
if (QTAILQ_EMPTY(p-ep-queue) || p-ep-pipeline) {
ret = usb_process_one(p);
if (ret == USB_RET_ASYNC) {
usb_packet_set_state(p, USB_PACKET_ASYNC);
QTAILQ_INSERT_TAIL(p-ep-queue, p, queue);
} else {
+/*
+ * When pipelining is enabled usb-devices must always return
async,
+ * otherwise packets can complete out of order!
+ */
+assert(!p-ep-pipeline);
Strictly speaking returning something != async is fine for the first
package in the queue, but I guess in practice this doesn't matter as
enabling pipelining doesn't make sense unless you actually go async.
p-result = ret;
usb_packet_set_state(p, USB_PACKET_COMPLETE);
}
@@ -402,6 +410,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
/* Notify the controller that an async packet is complete. This should only
be called for packets previously deferred by returning USB_RET_ASYNC from
handle_packet. */
That comments should be ...
+static void __usb_packet_complete(USBDevice *dev, USBPacket *p)
+{
+USBEndpoint *ep = p-ep;
+
+assert(p-result != USB_RET_ASYNC p-result != USB_RET_NAK);
+
+if (p-result 0) {
+ep-halted = true;
+}
+usb_packet_set_state(p, USB_PACKET_COMPLETE);
+QTAILQ_REMOVE(ep-queue, p, queue);
+dev-port-ops-complete(dev-port, p);
+}
+
... here, where the function for the external users is.
void usb_packet_complete(USBDevice *dev, USBPacket *p)
{
cheers,
Gerd
[Qemu-devel] [Bug 1037606] BootDmesg.txt
apport information ** Attachment added: BootDmesg.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265234/+files/BootDmesg.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1037606] AlsaInfo.txt
apport information ** Attachment added: AlsaInfo.txt https://bugs.launchpad.net/bugs/1037606/+attachment/3265233/+files/AlsaInfo.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037606 Title: vmwgfx does not work with kvm vmware vga Status in QEMU: New Status in “linux” package in Ubuntu: Confirmed Bug description: vmwgfx driver fails to initialize inside kvm. tried: kvm -m 2048 -vga vmware -cdrom RebeccaBlackLinux.iso (Ubuntu based, any Ubuntu live CD would do) Apport data collected with qantal alpha live CD (somewhat older kernel). The error is shjown in CurrentDmesg.txt https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1037606/+attachment/3265235/+files/CurrentDmesg.txt --- ApportVersion: 2.4-0ubuntu8 Architecture: amd64 AudioDevicesInUse: Error: command ['fuser', '-v', '/dev/snd/seq', '/dev/snd/timer'] failed with exit code 1: CRDA: Error: command ['iw', 'reg', 'get'] failed with exit code 1: nl80211 not found. CasperVersion: 1.320 DistroRelease: Ubuntu 12.10 IwConfig: eth0 no wireless extensions. lono wireless extensions. LiveMediaBuild: Ubuntu 12.10 Quantal Quetzal - Alpha amd64 (20120724.2) Lsusb: Error: command ['lsusb'] failed with exit code 1: unable to initialize libusb: -99 MachineType: Bochs Bochs Package: linux (not installed) ProcEnviron: TERM=linux PATH=(custom, no user) LANG=en_US.UTF-8 SHELL=/bin/bash ProcFB: ProcKernelCmdLine: file=/cdrom/preseed/hostname.seed boot=casper initrd=/casper/initrd.lz quiet splash -- maybe-ubiquity ProcVersionSignature: Ubuntu 3.5.0-6.6-generic 3.5.0 PulseList: Error: command ['pacmd', 'list'] failed with exit code 1: No PulseAudio daemon running, or not running as session daemon. RelatedPackageVersions: linux-restricted-modules-3.5.0-6-generic N/A linux-backports-modules-3.5.0-6-generic N/A linux-firmware 1.85 RfKill: Tags: quantal Uname: Linux 3.5.0-6-generic x86_64 UpgradeStatus: No upgrade log present (probably fresh install) UserGroups: dmi.bios.date: 01/01/2007 dmi.bios.vendor: Bochs dmi.bios.version: Bochs dmi.chassis.type: 1 dmi.chassis.vendor: Bochs dmi.modalias: dmi:bvnBochs:bvrBochs:bd01/01/2007:svnBochs:pnBochs:pvr:cvnBochs:ct1:cvr: dmi.product.name: Bochs dmi.sys.vendor: Bochs To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037606/+subscriptions
[Qemu-devel] [Bug 1037675] Re: Guest Kernel Panic if using -cpu host in qemu-kvm 1.1.1
sorry for the not very usefull information i provides above. i will try to reproduce the the failure with a vailla kernel (host) in a few days. currently the server is in use and cannot be restarted. the kernel panic was in the guest (thought this is clear by my vm panic). If it is reproducable with a vannilla kernel (host) i will provide more detailed information here. the vm is using a 3.3.8 gentoo kernel (not hardened). -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037675 Title: Guest Kernel Panic if using -cpu host in qemu-kvm 1.1.1 Status in QEMU: New Bug description: After Upgrading to qemu-kvm-1.1.1-r1 from version 1.0.1-r1 my virtual machines (running gentoo linux) panic at intel_pmu_init. (detailed information including stacktrace are in the uploaded screenshot). When i remove the -cpu host option, the system starts normally. the command line from whicht the system is bootet: qemu-kvm -vnc :7 -usbdevice tablet -daemonize -m 256 -drive file=/data/virtual_machines/wgs-l08.img,if=virtio -boot c -k de -net nic,model=virtio,macaddr=12:12:00:12:34:63,vlan=0 -net tap,ifname=qtap6,script=no,downscript=no,vlan=0 -smp 2 -enable-kvm -cpu host -monitor unix:/var/run/qemu- kvm/wgs-l08.monitor,server,nowait also reported on gentoo bug tracker (with some more details of the host): https://bugs.gentoo.org/show_bug.cgi?id=431640 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037675/+subscriptions
Re: [Qemu-devel] [PATCH v3 2/7] memory: Flush coalesced MMIO on selected region access
On 2012-07-10 12:41, Jan Kiszka wrote: On 2012-07-02 11:07, Avi Kivity wrote: On 06/29/2012 07:37 PM, Jan Kiszka wrote: Instead of flushing pending coalesced MMIO requests on every vmexit, this provides a mechanism to selectively flush when memory regions related to the coalesced one are accessed. This first of all includes the coalesced region itself but can also applied to other regions, e.g. of the same device, by calling memory_region_set_flush_coalesced. Looks fine. I have a hard time deciding whether this should go through the kvm tree or memory tree. Anthony, perhaps you can commit it directly to avoid the livelock? Reviewed-by: Avi Kivity a...@redhat.com Anthony, ping? Argh, missed that this series was forgotten. Patch 1 is a bug fix, will resend it separately so that it can make it into 1.2. Will repost the rest once master reopens. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [RESEND][PATCH for 1.2] i82378: Remove bogus MMIO coalescing
This MMIO area is an entry gate to legacy PC ISA devices, addressed via PIO over there. Quite a few of the PIO ports have side effects on access like starting/stopping timers that must be executed properly ordered /wrt the CPU. So we have to remove the coalescing mark. Acked-by: Hervé Poussineau hpous...@reactos.org Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/i82378.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/hw/i82378.c b/hw/i82378.c index 9b11d90..2123c14 100644 --- a/hw/i82378.c +++ b/hw/i82378.c @@ -225,7 +225,6 @@ static int pci_i82378_init(PCIDevice *dev) pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, s-io); memory_region_init_io(s-mem, i82378_mem_ops, s, i82378-mem, 0x0100); -memory_region_set_coalescing(s-mem); pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, s-mem); /* Make I/O address read only */ -- 1.7.3.4
Re: [Qemu-devel] [RESEND][PATCH for 1.2] i82378: Remove bogus MMIO coalescing
On 17.08.2012 14:56, Jan Kiszka wrote: This MMIO area is an entry gate to legacy PC ISA devices, addressed via PIO over there. Quite a few of the PIO ports have side effects on access like starting/stopping timers that must be executed properly ordered /wrt the CPU. So we have to remove the coalescing mark. This appears to be 1.1-stable material right? Thanks, /mjt
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Thu, Aug 16, 2012 at 7:49 PM, Steven wangwangk...@gmail.com wrote: [...] I want to get the guest memory address in the instruction mov 0x4(%ebx) %eax, whic is 0x4(%ebx). Since %ebx is not resolved until the execution time, the code in softmmu_header.h does not generate any hit or miss information. Do you know any place that I could resolve the memory access address? Thanks. You'll have to generate code. Look at how helpers work. Hi, Laurent, do you mean the target-i386/op_helper.c/helper.c or the tcg helper? Thanks. What do you mean by resolve the memory access address? Do you want to get guest virtual address for each guest memory access, right? As Max mentioned before (you can also read [1]), there are fast and slow path in QEMU softmmu, tlb hit and tlb miss respectively. Max provided patch for slow path. As for fast path, take a look on tcg_out_tlb_load (tcg /i386/tcg-target.c). tcg_out_tlb_load will generate native code in the code cache to do tlb lookup, I think you cannot use the trick Max used since tcg_out_tlb_load will not be called when the fast path executed, it generates code instead. Therefore, you might have to insert your instrument code in the code cache, perhaps modifying tcg_out_tlb_load to log value of addrlo (see comments above tcg_out_tlb_load). HTH, chenwj [1] http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg03060.html -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] [PATCH v2 2/2] vmdk: Read footer for streamOptimized images
On 08/16/2012 05:50 AM, Kevin Wolf wrote:
The footer takes precedence over the header when it exists. It contains
the real grain directory offset that is missing in the header. Without
this patch, streamOptimized images with a footer cannot be read.
Signed-off-by: Kevin Wolf kw...@redhat.com
---
v2:
- Enough footer sanity checks, I hope? :-)
block/vmdk.c | 56
1 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 9648398..bba4c61 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -35,6 +35,7 @@
#define VMDK4_FLAG_RGD (1 1)
#define VMDK4_FLAG_COMPRESS (1 16)
#define VMDK4_FLAG_MARKER (1 17)
+#define VMDK4_GD_AT_END 0xULL
typedef struct {
uint32_t version;
@@ -115,6 +116,13 @@ typedef struct VmdkGrainMarker {
uint8_t data[0];
} VmdkGrainMarker;
+enum {
+MARKER_END_OF_STREAM= 0,
+MARKER_GRAIN_TABLE = 1,
+MARKER_GRAIN_DIRECTORY = 2,
+MARKER_FOOTER = 3,
+};
+
static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
{
uint32_t magic;
@@ -451,6 +459,54 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
if (header.capacity == 0 header.desc_offset) {
return vmdk_open_desc_file(bs, flags, header.desc_offset 9);
}
+
+if (le64_to_cpu(header.gd_offset) == VMDK4_GD_AT_END) {
+/*
+ * The footer takes precedence over the header, so read it in. The
+ * footer starts at offset -1024 from the end: One sector for the
+ * footer, and another one for the end-of-stream marker.
+ */
+struct {
+struct {
+uint64_t val;
+uint32_t size;
+uint32_t type;
+uint8_t pad[512 - 16];
+} QEMU_PACKED footer_marker;
+
+uint32_t magic;
+VMDK4Header header;
+uint8_t pad[512 - 4 - sizeof(VMDK4Header)];
+
+struct {
+uint64_t val;
+uint32_t size;
+uint32_t type;
+uint8_t pad[512 - 16];
+} QEMU_PACKED eos_marker;
+} QEMU_PACKED footer;
+
+ret = bdrv_pread(file,
+bs-file-total_sectors * 512 - 1536,
+footer, sizeof(footer));
+if (ret 0) {
+return ret;
+}
+
+/* Some sanity checks for the footer */
+if (be32_to_cpu(footer.magic) != VMDK4_MAGIC ||
+le32_to_cpu(footer.footer_marker.size) != 0 ||
+le32_to_cpu(footer.footer_marker.type) != MARKER_FOOTER ||
+le64_to_cpu(footer.eos_marker.val) != 0 ||
+le32_to_cpu(footer.eos_marker.size) != 0 ||
+le32_to_cpu(footer.eos_marker.type) != MARKER_END_OF_STREAM)
+{
+return -EINVAL;
+}
+
+header = footer.header;
+}
+
l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
* le64_to_cpu(header.granularity);
if (l1_entry_sectors == 0) {
Reviewed-by: Jeff Cody jc...@redhat.com
Re: [Qemu-devel] How does ARM VFP is emulated?
On Fri, Aug 17, 2012 at 10:29:24AM +0200, Laurent Desnogues wrote:
On Thursday, August 16, 2012, Oi Khote oikh...@hotmail.com wrote:
So how exactly does VFP is being emulated.
QEMU uses a library for FP computations, based on the softfloat package.
I thought QEMU emulates VFP itself, something like
float VFP_helper(float a, float b) {
...
}
But I am wrong, it acutally does this, right?
float64 VFP_HELPER(sqrt, d)(float64 a, CPUARMState *env)
{
return float64_sqrt(a, env-vfp.fp_status);
}
And float64_sqrt is implemented in fpu/* .
Regards,
chenwj
--
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj
[Qemu-devel] [PATCH v5 1.2 queue 0/4] QXL_IO_MONITORS_CONFIG_ASYNC + misc
Hi Gerd, Rebased on the lastest, redid ifdefs to use a single line, QXL_HAS_IO_MONITORS_CONFIG_ASYNC is 0 by default, 1 if spice-protocol is new enough. Also available at git://people.freedesktop.org/~alon/qemu qxl/pull Alon Levy (4): qxl/update_area_io: guest_bug on invalid parameters qxl: disallow unknown revisions qxl: add QXL_IO_MONITORS_CONFIG_ASYNC configure: print spice-protocol and spice-server versions configure | 7 - hw/qxl.c | 78 +++--- hw/qxl.h | 7 + trace-events | 1 + ui/spice-display.h | 1 + 5 files changed, 90 insertions(+), 4 deletions(-) -- 1.7.11.2
[Qemu-devel] [PATCH v5 1.2 queue 1/4] qxl/update_area_io: guest_bug on invalid parameters
Signed-off-by: Alon Levy al...@redhat.com
---
hw/qxl.c | 12
1 file changed, 12 insertions(+)
diff --git a/hw/qxl.c b/hw/qxl.c
index c2dd3b4..6c48eb9 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1385,6 +1385,18 @@ async_common:
QXLCookie *cookie = NULL;
QXLRect update = d-ram-update_area;
+if (d-ram-update_surface NUM_SURFACES) {
+qxl_set_guest_bug(d, QXL_IO_UPDATE_AREA: invalid surface id %d\n,
+ d-ram-update_surface);
+return;
+}
+if (update.left = update.right || update.top = update.bottom) {
+qxl_set_guest_bug(d,
+QXL_IO_UPDATE_AREA: invalid area (%ux%u)x(%ux%u)\n,
+update.left, update.top, update.right, update.bottom);
+return;
+}
+
if (async == QXL_ASYNC) {
cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
QXL_IO_UPDATE_AREA_ASYNC);
--
1.7.11.2
[Qemu-devel] [PATCH v5 1.2 queue 4/4] configure: print spice-protocol and spice-server versions
Signed-off-by: Alon Levy al...@redhat.com --- configure | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/configure b/configure index dbf3af6..af4f68d 100755 --- a/configure +++ b/configure @@ -2657,6 +2657,8 @@ EOF spice=yes libs_softmmu=$libs_softmmu $spice_libs QEMU_CFLAGS=$QEMU_CFLAGS $spice_cflags +spice_protocol_version=$($pkg_config --modversion spice-protocol) +spice_server_version=$($pkg_config --modversion spice-server) if $pkg_config --atleast-version=0.12.0 spice-protocol /dev/null 21; then QEMU_CFLAGS=$QEMU_CFLAGS -DQXL_HAS_IO_MONITORS_CONFIG_ASYNC=1 fi @@ -3114,7 +3116,7 @@ echo libcap-ng support $cap_ng echo vhost-net support $vhost_net echo Trace backend $trace_backend echo Trace output file $trace_file-pid -echo spice support $spice +echo spice support $spice ($spice_protocol_version/$spice_server_version) echo rbd support $rbd echo xfsctl support$xfs echo nss used $smartcard_nss -- 1.7.11.2
[Qemu-devel] [PATCH v5 1.2 queue 2/4] qxl: disallow unknown revisions
Signed-off-by: Alon Levy al...@redhat.com --- hw/qxl.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/qxl.c b/hw/qxl.c index 6c48eb9..c978f5e 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1797,10 +1797,13 @@ static int qxl_init_common(PCIQXLDevice *qxl) io_size = 16; break; case 3: /* qxl-3 */ -default: pci_device_rev = QXL_DEFAULT_REVISION; io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1); break; +default: +error_report(Invalid revision %d for qxl device (max %d), + qxl-revision, QXL_DEFAULT_REVISION); +return -1; } pci_set_byte(config[PCI_REVISION_ID], pci_device_rev); -- 1.7.11.2
[Qemu-devel] [PATCH v5 1.2 queue 3/4] qxl: add QXL_IO_MONITORS_CONFIG_ASYNC
Revision bumped to 4 for new IO support, enabled for spice-server =
0.11.1. New io enabled iff spice-server = 0.11.1 spice-protocol =
0.12.0.
On migration reissue spice_qxl_monitors_config_async.
RHBZ: 770842
Signed-off-by: Alon Levy al...@redhat.com
---
configure | 3 +++
hw/qxl.c | 61 --
hw/qxl.h | 7 +++
trace-events | 1 +
ui/spice-display.h | 1 +
5 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index cc774b5..dbf3af6 100755
--- a/configure
+++ b/configure
@@ -2657,6 +2657,9 @@ EOF
spice=yes
libs_softmmu=$libs_softmmu $spice_libs
QEMU_CFLAGS=$QEMU_CFLAGS $spice_cflags
+if $pkg_config --atleast-version=0.12.0 spice-protocol /dev/null 21;
then
+QEMU_CFLAGS=$QEMU_CFLAGS -DQXL_HAS_IO_MONITORS_CONFIG_ASYNC=1
+fi
else
if test $spice = yes ; then
feature_not_found spice
diff --git a/hw/qxl.c b/hw/qxl.c
index c978f5e..16b147e 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -27,6 +27,10 @@
#include qxl.h
+#ifndef QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+#define QXL_HAS_IO_MONITORS_CONFIG_ASYNC 0
+#endif
+
/*
* NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as
* such can be changed by the guest, so to avoid a guest trigerrable
@@ -249,6 +253,20 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl,
qxl_async_io async)
}
}
+/* 0x00b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl)
+{
+trace_qxl_spice_monitors_config(qxl-id);
+qxl-guest_monitors_config = qxl-ram-monitors_config;
+spice_qxl_monitors_config_async(qxl-ssd.qxl,
+qxl-ram-monitors_config,
+MEMSLOT_GROUP_GUEST,
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+ QXL_IO_MONITORS_CONFIG_ASYNC));
+}
+#endif
+
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
{
trace_qxl_spice_reset_image_cache(qxl-id);
@@ -538,6 +556,9 @@ static const char *io_port_to_string(uint32_t io_port)
= QXL_IO_DESTROY_ALL_SURFACES_ASYNC,
[QXL_IO_FLUSH_SURFACES_ASYNC] = QXL_IO_FLUSH_SURFACES_ASYNC,
[QXL_IO_FLUSH_RELEASE] = QXL_IO_FLUSH_RELEASE,
+#if QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+[QXL_IO_MONITORS_CONFIG_ASYNC] = QXL_IO_MONITORS_CONFIG_ASYNC,
+#endif
};
return io_port_to_string[io_port];
}
@@ -819,7 +840,10 @@ static void interface_async_complete_io(PCIQXLDevice *qxl,
QXLCookie *cookie)
case QXL_IO_DESTROY_PRIMARY_ASYNC:
case QXL_IO_UPDATE_AREA_ASYNC:
case QXL_IO_FLUSH_SURFACES_ASYNC:
+#if QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+case QXL_IO_MONITORS_CONFIG_ASYNC:
break;
+#endif
case QXL_IO_CREATE_PRIMARY_ASYNC:
qxl_create_guest_primary_complete(qxl);
break;
@@ -894,6 +918,8 @@ static void interface_async_complete(QXLInstance *sin,
uint64_t cookie_token)
case QXL_COOKIE_TYPE_RENDER_UPDATE_AREA:
qxl_render_update_area_done(qxl, cookie);
break;
+case QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG:
+break;
default:
fprintf(stderr, qxl: %s: unexpected cookie type %d\n,
__func__, cookie-type);
@@ -1333,7 +1359,7 @@ static void ioport_write(void *opaque, target_phys_addr_t
addr,
io_port, io_port_to_string(io_port));
/* be nice to buggy guest drivers */
if (io_port = QXL_IO_UPDATE_AREA_ASYNC
-io_port = QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
+io_port QXL_IO_RANGE_SIZE) {
qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
}
return;
@@ -1361,6 +1387,10 @@ static void ioport_write(void *opaque,
target_phys_addr_t addr,
io_port = QXL_IO_DESTROY_ALL_SURFACES;
goto async_common;
case QXL_IO_FLUSH_SURFACES_ASYNC:
+/* 0x000b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+#endif
async_common:
async = QXL_ASYNC;
qemu_mutex_lock(d-async_lock);
@@ -1502,6 +1532,13 @@ async_common:
d-mode = QXL_MODE_UNDEFINED;
qxl_spice_destroy_surfaces(d, async);
break;
+/* 0x000b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 \
+defined(QXL_HAS_IO_MONITORS_CONFIG_ASYNC)
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+qxl_spice_monitors_config_async(d);
+break;
+#endif
default:
qxl_set_guest_bug(d, %s: unexpected ioport=0x%x\n, __func__,
io_port);
}
@@ -1797,9 +1834,15 @@ static int qxl_init_common(PCIQXLDevice *qxl)
io_size = 16;
break;
case 3: /* qxl-3 */
+pci_device_rev = QXL_REVISION_STABLE_V10;
+io_size = 32; /* PCI region size must be pow2 */
+break;
+#if SPICE_SERVER_VERSION = 0x000b01 /* 0.11.1 */
+case 4: /*
Re: [Qemu-devel] qemu log function to print out the registers of the guest
On Fri, Aug 17, 2012 at 3:14 PM, 陳韋任 (Wei-Ren Chen) che...@iis.sinica.edu.tw wrote: On Thu, Aug 16, 2012 at 7:49 PM, Steven wangwangk...@gmail.com wrote: [...] I want to get the guest memory address in the instruction mov 0x4(%ebx) %eax, whic is 0x4(%ebx). Since %ebx is not resolved until the execution time, the code in softmmu_header.h does not generate any hit or miss information. Do you know any place that I could resolve the memory access address? Thanks. You'll have to generate code. Look at how helpers work. Hi, Laurent, do you mean the target-i386/op_helper.c/helper.c or the tcg helper? Thanks. What do you mean by resolve the memory access address? Do you want to get guest virtual address for each guest memory access, right? As Max mentioned before (you can also read [1]), there are fast and slow path in QEMU softmmu, tlb hit and tlb miss respectively. Max provided patch for slow path. As for fast path, take a look on tcg_out_tlb_load (tcg /i386/tcg-target.c). tcg_out_tlb_load will generate native code in the code cache to do tlb lookup, I think you cannot use the trick Max used since tcg_out_tlb_load will not be called when the fast path executed, That's why I've posted the following hunk that should have made all accesses go via slow path: diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c index da17bba..ec68c19 100644 --- a/tcg/i386/tcg-target.c +++ b/tcg/i386/tcg-target.c @@ -1062,7 +1062,7 @@ static inline void tcg_out_tlb_load(TCGContext *s, int addrlo_idx, tcg_out_mov(s, type, r0, addrlo); /* jne label1 */ -tcg_out8(s, OPC_JCC_short + JCC_JNE); +tcg_out8(s, OPC_JMP_short); label_ptr[0] = s-code_ptr; s-code_ptr++; it generates code instead. Therefore, you might have to insert your instrument code in the code cache, perhaps modifying tcg_out_tlb_load to log value of addrlo (see comments above tcg_out_tlb_load). -- Thanks. -- Max
Re: [Qemu-devel] How does ARM VFP is emulated?
On 17 August 2012 12:27, 陳韋任 (Wei-Ren Chen) che...@iis.sinica.edu.tw wrote:
On Fri, Aug 17, 2012 at 10:29:24AM +0200, Laurent Desnogues wrote:
On Thursday, August 16, 2012, Oi Khote oikh...@hotmail.com wrote:
So how exactly does VFP is being emulated.
QEMU uses a library for FP computations, based on the softfloat package.
I thought QEMU emulates VFP itself
Depends how you think about it. We do emulate all the VFP
with integer operations...
But I am wrong, it acutally does this, right?
float64 VFP_HELPER(sqrt, d)(float64 a, CPUARMState *env)
{
return float64_sqrt(a, env-vfp.fp_status);
}
And float64_sqrt is implemented in fpu/* .
...it just happens that the code that does generic IEEE floating
point emulation lives in fpu/ and is shared between all targets.
(That code is a library called 'softfloat' but we have modified
our copy of it quite a bit over the years.)
-- PMM
Re: [Qemu-devel] [RESEND][PATCH for 1.2] i82378: Remove bogus MMIO coalescing
On 2012-08-17 13:13, Michael Tokarev wrote: On 17.08.2012 14:56, Jan Kiszka wrote: This MMIO area is an entry gate to legacy PC ISA devices, addressed via PIO over there. Quite a few of the PIO ports have side effects on access like starting/stopping timers that must be executed properly ordered /wrt the CPU. So we have to remove the coalescing mark. This appears to be 1.1-stable material right? True, adding qemu-stable. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] vmware_vga: Redraw only visible area
On 17.08.2012 06:55, Marek Vasut wrote:
Disallow negative value boundaries of the redraw rectangle.
This fixes a segfault when using -vga vmware.
Signed-off-by: Marek Vasut ma...@denx.de
---
hw/vmware_vga.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
NOTE: I tested this by emulating some recent version of ubuntu. The rect-x
value was set to -65 for some reason at one point, which caused the
kvm to crash. Trimming the rectangle fixed the issue.
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index f5e4f44..62e5887 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -337,8 +337,8 @@ static inline void vmsvga_update_rect_delayed(struct
vmsvga_state_s *s,
{
struct vmsvga_rect_s *rect = s-redraw_fifo[s-redraw_fifo_last ++];
s-redraw_fifo_last = REDRAW_FIFO_LEN - 1;
-rect-x = x;
-rect-y = y;
+rect-x = (x 0) ? 0 : x;
+rect-y = (y 0) ? 0 : y;
rect-w = w;
rect-h = h;
}
Is it the same as https://bugs.launchpad.net/bugs/918791 ?
At least it appears to be the same theme... But there,
the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff)
also updates width/height. My comment:
https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/21
So indeed, some (upstream) verification is needed here -- where these
negative values are coming from, whenever it is EVER okay to have them,
what to do with these, and where to check (I guess the check should be
done somewhere in the upper layer).
Especially the last part about the layer.
Thanks,
/mjt
Re: [Qemu-devel] Windows slow boot: contractor wanted
Hi Avi, Thanks to you and several others for offering help. We will work with Avi at first, but are grateful for all the other offers of help. We have a number of other qemu-related projects which we'd be interested in getting done, and will get in touch with these names (and anyone else who comes forward) to see if any are of interest to you. This slow boot problem is intermittent and varys in how slow the boots are, but I managed to trigger it this morning with medium slow booting (5-10 minutes) and link to the requested traces below. The host in question has 128GB RAM and dual AMD Opteron 6128 (16 cores total). It is running kernel 3.5.1 and qemu-kvm 1.1.1. In this morning's test, we have 3 guests, all booting Windows with 40GB RAM and 8 cores each (we have seen small VMs go slow as I originally said, but it is easier to trigger with big VMs): pid 15665: qemu-kvm -nodefaults -m 40960 -smp 8 -cpu host,hv_relaxed \ -vga cirrus -usbdevice tablet -vnc :99 -monitor stdio -hda test1.raw pid 15676: qemu-kvm -nodefaults -m 40960 -smp 8 -cpu host,hv_relaxed \ -vga cirrus -usbdevice tablet -vnc :98 -monitor stdio -hda test2.raw pid 15653: qemu-kvm -nodefaults -m 40960 -smp 8 -cpu host,hv_relaxed \ -vga cirrus -usbdevice tablet -vnc :97 -monitor stdio -hda test3.raw We are running with hv_relaxed since this was suggested in the previous thread, but we see intermittent slow boots with and without this flag. All 3 VMs are booting slowly for most of the attached capture, which I started after confirming the slow boots and stopped as soon as the first of them (15665) had booted. In terms of visible symptoms, the VMs are showing the Windows boot progress bar, which is moving very slowly. In top, the VMs are at 400% CPU and their resident state size (RES) memory is slowly counting up until it reaches the full VM size, at which point they finish booting. Here are the trace files: http://users.org.uk/slow-win-boot-1/ps.txt (ps auxwwwf as root) http://users.org.uk/slow-win-boot-1/top.txt (top with 2 VMs still slow) http://users.org.uk/slow-win-boot-1/trace-console.txt (running trace-cmd) http://users.org.uk/slow-win-boot-1/trace.dat (the 1.7G trace data file) http://users.org.uk/slow-win-boot-1/trace-report.txt (the 4G trace report) Please let me know if there is anything else which I can provide? Thank you, Richard.
Re: [Qemu-devel] [PATCH] vmware_vga: Redraw only visible area
Dear Michael Tokarev,
On 17.08.2012 06:55, Marek Vasut wrote:
Disallow negative value boundaries of the redraw rectangle.
This fixes a segfault when using -vga vmware.
Signed-off-by: Marek Vasut ma...@denx.de
---
hw/vmware_vga.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
NOTE: I tested this by emulating some recent version of ubuntu. The
rect-x
value was set to -65 for some reason at one point, which caused the
kvm to crash. Trimming the rectangle fixed the issue.
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index f5e4f44..62e5887 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -337,8 +337,8 @@ static inline void vmsvga_update_rect_delayed(struct
vmsvga_state_s *s,
{
struct vmsvga_rect_s *rect = s-redraw_fifo[s-redraw_fifo_last
++]; s-redraw_fifo_last = REDRAW_FIFO_LEN - 1;
-rect-x = x;
-rect-y = y;
+rect-x = (x 0) ? 0 : x;
+rect-y = (y 0) ? 0 : y;
rect-w = w;
rect-h = h;
}
Is it the same as https://bugs.launchpad.net/bugs/918791 ?
At least it appears to be the same theme... But there,
the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff)
also updates width/height. My comment:
https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/21
Looks to be the same ... though my patch tries to squash the issue as early as
possible.
You're right that x and y might overflow to the other side too. Also, you're
right about w and h.
Shall I send updated patch?
So indeed, some (upstream) verification is needed here -- where these
negative values are coming from, whenever it is EVER okay to have them,
what to do with these, and where to check (I guess the check should be
done somewhere in the upper layer).
Especially the last part about the layer.
Where's the upper layer though, isn't that what's pouring out of the virtual
machine itself?
Thanks,
Thank you for guidance !
/mjt
Best regards,
Marek Vasut
[Qemu-devel] [PATCH v6 1.2] qxl: add QXL_IO_MONITORS_CONFIG_ASYNC
Revision bumped to 4 for new IO support, enabled for spice-server =
0.11.1. New io enabled iff spice-server = 0.11.1 spice-protocol =
0.12.0.
On migration reissue spice_qxl_monitors_config_async.
RHBZ: 770842
Signed-off-by: Alon Levy al...@redhat.com
---
Left in one defined, fixed here.
configure | 3 +++
hw/qxl.c | 60 --
hw/qxl.h | 7 +++
trace-events | 1 +
ui/spice-display.h | 1 +
5 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index cc774b5..dbf3af6 100755
--- a/configure
+++ b/configure
@@ -2657,6 +2657,9 @@ EOF
spice=yes
libs_softmmu=$libs_softmmu $spice_libs
QEMU_CFLAGS=$QEMU_CFLAGS $spice_cflags
+if $pkg_config --atleast-version=0.12.0 spice-protocol /dev/null 21;
then
+QEMU_CFLAGS=$QEMU_CFLAGS -DQXL_HAS_IO_MONITORS_CONFIG_ASYNC=1
+fi
else
if test $spice = yes ; then
feature_not_found spice
diff --git a/hw/qxl.c b/hw/qxl.c
index c978f5e..25d7759 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -27,6 +27,10 @@
#include qxl.h
+#ifndef QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+#define QXL_HAS_IO_MONITORS_CONFIG_ASYNC 0
+#endif
+
/*
* NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as
* such can be changed by the guest, so to avoid a guest trigerrable
@@ -249,6 +253,20 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl,
qxl_async_io async)
}
}
+/* 0x00b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl)
+{
+trace_qxl_spice_monitors_config(qxl-id);
+qxl-guest_monitors_config = qxl-ram-monitors_config;
+spice_qxl_monitors_config_async(qxl-ssd.qxl,
+qxl-ram-monitors_config,
+MEMSLOT_GROUP_GUEST,
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+ QXL_IO_MONITORS_CONFIG_ASYNC));
+}
+#endif
+
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
{
trace_qxl_spice_reset_image_cache(qxl-id);
@@ -538,6 +556,9 @@ static const char *io_port_to_string(uint32_t io_port)
= QXL_IO_DESTROY_ALL_SURFACES_ASYNC,
[QXL_IO_FLUSH_SURFACES_ASYNC] = QXL_IO_FLUSH_SURFACES_ASYNC,
[QXL_IO_FLUSH_RELEASE] = QXL_IO_FLUSH_RELEASE,
+#if QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+[QXL_IO_MONITORS_CONFIG_ASYNC] = QXL_IO_MONITORS_CONFIG_ASYNC,
+#endif
};
return io_port_to_string[io_port];
}
@@ -819,7 +840,10 @@ static void interface_async_complete_io(PCIQXLDevice *qxl,
QXLCookie *cookie)
case QXL_IO_DESTROY_PRIMARY_ASYNC:
case QXL_IO_UPDATE_AREA_ASYNC:
case QXL_IO_FLUSH_SURFACES_ASYNC:
+#if QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+case QXL_IO_MONITORS_CONFIG_ASYNC:
break;
+#endif
case QXL_IO_CREATE_PRIMARY_ASYNC:
qxl_create_guest_primary_complete(qxl);
break;
@@ -894,6 +918,8 @@ static void interface_async_complete(QXLInstance *sin,
uint64_t cookie_token)
case QXL_COOKIE_TYPE_RENDER_UPDATE_AREA:
qxl_render_update_area_done(qxl, cookie);
break;
+case QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG:
+break;
default:
fprintf(stderr, qxl: %s: unexpected cookie type %d\n,
__func__, cookie-type);
@@ -1333,7 +1359,7 @@ static void ioport_write(void *opaque, target_phys_addr_t
addr,
io_port, io_port_to_string(io_port));
/* be nice to buggy guest drivers */
if (io_port = QXL_IO_UPDATE_AREA_ASYNC
-io_port = QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
+io_port QXL_IO_RANGE_SIZE) {
qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
}
return;
@@ -1361,6 +1387,10 @@ static void ioport_write(void *opaque,
target_phys_addr_t addr,
io_port = QXL_IO_DESTROY_ALL_SURFACES;
goto async_common;
case QXL_IO_FLUSH_SURFACES_ASYNC:
+/* 0x000b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+#endif
async_common:
async = QXL_ASYNC;
qemu_mutex_lock(d-async_lock);
@@ -1502,6 +1532,12 @@ async_common:
d-mode = QXL_MODE_UNDEFINED;
qxl_spice_destroy_surfaces(d, async);
break;
+/* 0x000b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+qxl_spice_monitors_config_async(d);
+break;
+#endif
default:
qxl_set_guest_bug(d, %s: unexpected ioport=0x%x\n, __func__,
io_port);
}
@@ -1797,9 +1833,15 @@ static int qxl_init_common(PCIQXLDevice *qxl)
io_size = 16;
break;
case 3: /* qxl-3 */
+pci_device_rev = QXL_REVISION_STABLE_V10;
+io_size = 32; /* PCI region size must be pow2 */
+break;
+#if SPICE_SERVER_VERSION = 0x000b01 /* 0.11.1
Re: [Qemu-devel] [PATCH 1/3] usb: Halt ep queue en cancel pending packets on a packet error
Hi,
On 08/17/2012 12:30 PM, Gerd Hoffmann wrote:
Hi,
Note this patch only touches the ehci and uhci controller changes, since AFAIK
no other controllers actually queue up multiple transfer. If I'm wrong on this
other controllers need to be updated too!
xhci does it too (although it is hard to test as xhci can happily submit
256k transfers where ehci and uhci have to use a bunch of smaller
packets instead).
Good point, I'm completely unfamiliar with the xhci code I'm afraid (at some
point
in time this will need to change, but not right now), any chance you could fill
in
the xhci part of this patch? Feel free to merge it with my patch so as to avoid
having a broken state in git.
Some minor nits below (the other two patches look good):
+/* Submitting a new packet clears halt */
+p-ep-halted = false;
check that the queue is empty when halted is set (i.e. enforce cancel on
error) ?
Good point will fix.
+
if (QTAILQ_EMPTY(p-ep-queue) || p-ep-pipeline) {
ret = usb_process_one(p);
if (ret == USB_RET_ASYNC) {
usb_packet_set_state(p, USB_PACKET_ASYNC);
QTAILQ_INSERT_TAIL(p-ep-queue, p, queue);
} else {
+/*
+ * When pipelining is enabled usb-devices must always return async,
+ * otherwise packets can complete out of order!
+ */
+assert(!p-ep-pipeline);
Strictly speaking returning something != async is fine for the first
package in the queue, but I guess in practice this doesn't matter as
enabling pipelining doesn't make sense unless you actually go async.
p-result = ret;
usb_packet_set_state(p, USB_PACKET_COMPLETE);
}
@@ -402,6 +410,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
/* Notify the controller that an async packet is complete. This should only
be called for packets previously deferred by returning USB_RET_ASYNC from
handle_packet. */
That comments should be ...
+static void __usb_packet_complete(USBDevice *dev, USBPacket *p)
+{
+USBEndpoint *ep = p-ep;
+
+assert(p-result != USB_RET_ASYNC p-result != USB_RET_NAK);
+
+if (p-result 0) {
+ep-halted = true;
+}
+usb_packet_set_state(p, USB_PACKET_COMPLETE);
+QTAILQ_REMOVE(ep-queue, p, queue);
+dev-port-ops-complete(dev-port, p);
+}
+
... here, where the function for the external users is.
Fixed, will send a new version (without the xhci changes) right away, to
cut back on the spam I'm only going to resend 1/3.
Regards,
Hans
Re: [Qemu-devel] qemu-kvm-1.0.1 - unable to exit if vcpu is in infinite loop
On 2012-08-06 17:11, Stefan Hajnoczi wrote: On Thu, Jun 28, 2012 at 2:05 PM, Peter Lieven p...@dlhnet.de wrote: i debugged my initial problem further and found out that the problem happens to be that the main thread is stuck in pause_all_vcpus() on reset or quit commands in the monitor if one cpu is stuck in the do-while loop kvm_cpu_exec. If I modify the condition from while (ret == 0) to while ((ret == 0) !env-stop); it works, but is this the right fix? Quit command seems to work, but on Reset the VM enterns pause state. I think I'm hitting something similar. I installed a F17 amd64 guest (3.5 kernel) but before booting entered the GRUB boot menu edit mode. The guest seemed unresponsive so I switched to the monitor, which also froze shortly afterwards. The VNC screen ended up being all black. qemu-kvm.git/master 3e4305694fd891b69e4450e59ec4c65420907ede Linux 3.2.0-3-amd64 from Debian testing $ qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive if=virtio,cache=none,file=f17.img,aio=native -serial stdio (gdb) thread apply all bt Thread 3 (Thread 0x7f8008e23700 (LWP 367)): #0 0x7f800f891727 in ioctl () at ../sysdeps/unix/syscall-template.S:82 #1 0x7f80137b92c9 in kvm_vcpu_ioctl (env=env@entry=0x7f8015b49640, type=type@entry=44672) at /home/stefanha/qemu-kvm/kvm-all.c:1619 #2 0x7f80137b93fe in kvm_cpu_exec (env=env@entry=0x7f8015b49640) at /home/stefanha/qemu-kvm/kvm-all.c:1506 #3 0x7f8013766f31 in qemu_kvm_cpu_thread_fn (arg=0x7f8015b49640) at /home/stefanha/qemu-kvm/cpus.c:756 #4 0x7f800fb4db50 in start_thread (arg=optimized out) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () This vcpu is still executing guest code and I've seen it successfully dispatching I/O. The problem is it's missing the exit_request... Thread 2 (Thread 0x7f8008622700 (LWP 368)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=optimized out, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013766eff in qemu_kvm_wait_io_event (env=optimized out) at /home/stefanha/qemu-kvm/cpus.c:724 #3 qemu_kvm_cpu_thread_fn (arg=0x7f8015b67450) at /home/stefanha/qemu-kvm/cpus.c:761 #4 0x7f800fb4db50 in start_thread (arg=optimized out) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () No problems here. Thread 1 (Thread 0x7f801347b8c0 (LWP 365)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=cond@entry=0x7f801402fd80, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013768949 in pause_all_vcpus () at /home/stefanha/qemu-kvm/cpus.c:962 #3 0x7f80136028c8 in main (argc=optimized out, argv=optimized out, envp=optimized out) at /home/stefanha/qemu-kvm/vl.c:3695 We're deadlocked in pause_all_vcpus(), waiting for vcpu #0 to pause. Unfortunately vcpu #0 has -exit_request=0 although -stop=1. Here are the vcpus: (gdb) p first_cpu $6 = (struct CPUX86State *) 0x7f8015b49640 (gdb) p first_cpu-next_cpu $7 = (struct CPUX86State *) 0x7f8015b67450 (gdb) p first_cpu-next_cpu-next_cpu $8 = (struct CPUX86State *) 0x0 (gdb) p first_cpu-stop $9 = 1 (gdb) p first_cpu-stopped $10 = 0 (gdb) p first_cpu-exit_request $11 = 0 CPUState::exit_request is only set on specific synchronous events, see target-i386/kvm.c. More interesting is CPUState::thread_kicked. If it's set, qemu_cpu_kick will skip the kicking via a signal. Maybe there is some race. Let me think about such possibilities again... Jan :( This isn't easy to reproduce. I tried entering the GRUB boot menu again and there was no deadlock. Stefan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH] ui: Fix spelling in comment (ressource - resource)
The function is called interface_release_resource. Signed-off-by: Stefan Weil s...@weilnetz.de --- ui/spice-display.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/spice-display.c b/ui/spice-display.c index 3e8f0b3..277843c 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -217,7 +217,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) } /* - * Called from spice server thread context (via interface_release_ressource) + * Called from spice server thread context (via interface_release_resource) * We do *not* hold the global qemu mutex here, so extra care is needed * when calling qemu functions. QEMU interfaces used: *- g_free (underlying glibc free is re-entrant). -- 1.7.10
[Qemu-devel] [PATCH] vdi: Fix warning from clang
ccc-analyzer reports these warnings:
block/vdi.c:704:13: warning: Dereference of null pointer
bmap[i] = VDI_UNALLOCATED;
^
block/vdi.c:702:13: warning: Dereference of null pointer
bmap[i] = i;
^
Moving some code into the if block fixes this.
It also avoids calling function write with 0 bytes of data.
Signed-off-by: Stefan Weil s...@weilnetz.de
---
block/vdi.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index c4f1529..d80114a 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -628,7 +628,6 @@ static int vdi_create(const char *filename,
QEMUOptionParameter *options)
VdiHeader header;
size_t i;
size_t bmap_size;
-uint32_t *bmap;
logout(\n);
@@ -693,21 +692,21 @@ static int vdi_create(const char *filename,
QEMUOptionParameter *options)
result = -errno;
}
-bmap = NULL;
if (bmap_size 0) {
-bmap = (uint32_t *)g_malloc0(bmap_size);
-}
-for (i = 0; i blocks; i++) {
-if (image_type == VDI_TYPE_STATIC) {
-bmap[i] = i;
-} else {
-bmap[i] = VDI_UNALLOCATED;
+uint32_t *bmap = (uint32_t *)g_malloc0(bmap_size);
+for (i = 0; i blocks; i++) {
+if (image_type == VDI_TYPE_STATIC) {
+bmap[i] = i;
+} else {
+bmap[i] = VDI_UNALLOCATED;
+}
}
+if (write(fd, bmap, bmap_size) 0) {
+result = -errno;
+}
+g_free(bmap);
}
-if (write(fd, bmap, bmap_size) 0) {
-result = -errno;
-}
-g_free(bmap);
+
if (image_type == VDI_TYPE_STATIC) {
if (ftruncate(fd, sizeof(header) + bmap_size + blocks * block_size)) {
result = -errno;
--
1.7.10
[Qemu-devel] [PATCH] usb: Halt ep queue en cancel pending packets on a packet error
For controllers which queue up more then 1 packet at a time, we must halt the
ep queue, and inside the controller code cancel all pending packets on an
error.
There are multiple reasons for this:
1) Guests expect the controllers to halt ep queues on error, so that they
get the opportunity to cancel transfers which the scheduled after the failing
one, before processing continues
2) Not cancelling queued up packets after a failed transfer also messes up
the controller state machine, in the case of EHCI causing the following
assert to trigger: assert(p-qtdaddr == q-qtdaddr) at hcd-ehci.c:2075
3) For bulk endpoints with pipelining enabled (redirection to a real USB
device), we must cancel all the transfers after this a failed one so that:
a) If they've completed already, they are not processed further causing more
stalls to be reported, originating from the same failed transfer
b) If still in flight, they are cancelled before the guest does
a clear stall, otherwise the guest and device can loose sync!
Note this patch only touches the ehci and uhci controller changes, since AFAIK
no other controllers actually queue up multiple transfer. If I'm wrong on this
other controllers need to be updated too!
Also note that this patch was heavily tested with the ehci code, where I had
a reproducer for a device causing a transfer to fail. The uhci code is not
tested with actually failing transfers and could do with a thorough review!
Signed-off-by: Hans de Goede hdego...@redhat.com
---
hw/usb.h | 1 +
hw/usb/core.c | 35 ---
hw/usb/hcd-ehci.c | 13 +
hw/usb/hcd-uhci.c | 16
4 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/hw/usb.h b/hw/usb.h
index 432ccae..e574477 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -179,6 +179,7 @@ struct USBEndpoint {
uint8_t ifnum;
int max_packet_size;
bool pipeline;
+bool halted;
USBDevice *dev;
QTAILQ_HEAD(, USBPacket) queue;
};
diff --git a/hw/usb/core.c b/hw/usb/core.c
index c7e5bc0..28b840e 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -382,12 +382,23 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
usb_packet_check_state(p, USB_PACKET_SETUP);
assert(p-ep != NULL);
+/* Submitting a new packet clears halt */
+if (p-ep-halted) {
+assert(QTAILQ_EMPTY(p-ep-queue));
+p-ep-halted = false;
+}
+
if (QTAILQ_EMPTY(p-ep-queue) || p-ep-pipeline) {
ret = usb_process_one(p);
if (ret == USB_RET_ASYNC) {
usb_packet_set_state(p, USB_PACKET_ASYNC);
QTAILQ_INSERT_TAIL(p-ep-queue, p, queue);
} else {
+/*
+ * When pipelining is enabled usb-devices must always return async,
+ * otherwise packets can complete out of order!
+ */
+assert(!p-ep-pipeline);
p-result = ret;
usb_packet_set_state(p, USB_PACKET_COMPLETE);
}
@@ -399,6 +410,20 @@ int usb_handle_packet(USBDevice *dev, USBPacket *p)
return ret;
}
+static void __usb_packet_complete(USBDevice *dev, USBPacket *p)
+{
+USBEndpoint *ep = p-ep;
+
+assert(p-result != USB_RET_ASYNC p-result != USB_RET_NAK);
+
+if (p-result 0) {
+ep-halted = true;
+}
+usb_packet_set_state(p, USB_PACKET_COMPLETE);
+QTAILQ_REMOVE(ep-queue, p, queue);
+dev-port-ops-complete(dev-port, p);
+}
+
/* Notify the controller that an async packet is complete. This should only
be called for packets previously deferred by returning USB_RET_ASYNC from
handle_packet. */
@@ -409,11 +434,9 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
usb_packet_check_state(p, USB_PACKET_ASYNC);
assert(QTAILQ_FIRST(ep-queue) == p);
-usb_packet_set_state(p, USB_PACKET_COMPLETE);
-QTAILQ_REMOVE(ep-queue, p, queue);
-dev-port-ops-complete(dev-port, p);
+__usb_packet_complete(dev, p);
-while (!QTAILQ_EMPTY(ep-queue)) {
+while (!ep-halted !QTAILQ_EMPTY(ep-queue)) {
p = QTAILQ_FIRST(ep-queue);
if (p-state == USB_PACKET_ASYNC) {
break;
@@ -425,9 +448,7 @@ void usb_packet_complete(USBDevice *dev, USBPacket *p)
break;
}
p-result = ret;
-usb_packet_set_state(p, USB_PACKET_COMPLETE);
-QTAILQ_REMOVE(ep-queue, p, queue);
-dev-port-ops-complete(dev-port, p);
+__usb_packet_complete(ep-dev, p);
}
}
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 623b3e6..378b42b 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2122,6 +2122,19 @@ static int ehci_state_writeback(EHCIQueue *q)
* bit is clear.
*/
if (q-qh.token QTD_TOKEN_HALT) {
+/*
+ * We should not do any further processing on a halted queue!
+ * This is esp. important for bulk endpoints with pipelining enabled
+ * (redirection to a real USB device), where we must cancel all the
+
Re: [Qemu-devel] [PATCH 4/6] sheepdog: don't leak socket file descriptor upon connection failure
Kevin Wolf wrote: Am 16.05.2012 15:07, schrieb Jim Meyering: From: Jim Meyering meyer...@redhat.com Signed-off-by: Jim Meyering meyer...@redhat.com Acked-by: Kevin Wolf kw...@redhat.com Hi Kevin, AFAICS, only one of these 6 patches has been applied. From what I recall (it's been nearly 3mo), there was good feedback and I posted at least one V2 patch. For reference, here's the start of the series: http://marc.info/?l=qemu-develm=133717388221635w=2 Let me know if there's anything I can do to help. Jim Meyering (5): qemu-ga: don't leak a file descriptor upon failed lockf linux-user: do_msgrcv: don't leak host_mb upon TARGET_EFAULT failure sheepdog: don't leak socket file descriptor upon connection failure arm-semi: don't leak 1KB user string lock buffer upon TARGET_SYS_OPEN softmmu-semi: fix lock_user* functions not to deref NULL upon OOM block/sheepdog.c |1 + linux-user/syscall.c |4 ++-- qemu-ga.c |3 +++ softmmu-semi.h|5 - target-arm/arm-semi.c | 13 +++-- 5 files changed, 17 insertions(+), 9 deletions(-)
[Qemu-devel] [PATCH] monitor: Fix warning from clang
ccc-analyzer reports these warnings:
monitor.c:3532:21: warning: Division by zero
val %= val2;
^
monitor.c:3530:21: warning: Division by zero
val /= val2;
^
Rewriting the code fixes this (and also a style issue).
Signed-off-by: Stefan Weil s...@weilnetz.de
---
monitor.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0c34934..0ea2c14 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3524,12 +3524,13 @@ static int64_t expr_prod(Monitor *mon)
break;
case '/':
case '%':
-if (val2 == 0)
+if (val2 == 0) {
expr_error(mon, division by zero);
-if (op == '/')
+} else if (op == '/') {
val /= val2;
-else
+} else {
val %= val2;
+}
break;
}
}
--
1.7.10
Re: [Qemu-devel] [PATCHv2 2/2] envlist.c: handle strdup failure
Jim Meyering wrote:
From: Jim Meyering meyer...@redhat.com
Without this, envlist_to_environ may silently fail to copy all
strings into the destination buffer, and both callers would leak
any env strings allocated after a failing strdup, because the
freeing code stops at the first NULL pointer.
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/envlist.c b/envlist.c
index be0addb..7532091 100644
--- a/envlist.c
+++ b/envlist.c
@@ -234,8 +234,16 @@ envlist_to_environ(const envlist_t *envlist, size_t
*count)
return (NULL);
for (entry = envlist-el_entries.lh_first; entry != NULL;
-entry = entry-ev_link.le_next) {
-*(penv++) = strdup(entry-ev_var);
+ entry = entry-ev_link.le_next, penv++) {
+*penv = strdup(entry-ev_var);
+if (*penv == NULL) {
+char **e = env;
+while (e = penv) {
+free(*e++);
+}
+free(env);
+return NULL;
+}
}
*penv = NULL; /* NULL terminate the list */
It seems this has been lost in this list's high volume of patches.
Anyone interested? Repost desired?
b/envlist.c | 256 ++--
envlist.c | 12 ++
2 files changed, 138 insertions(+), 130 deletions(-)
Re: [Qemu-devel] [PATCH 0/9] convert many more globals to static
Jim Meyering wrote: From: Jim Meyering meyer...@redhat.com Following up on discussion here, http://marc.info/?t=13375948768r=1w=2 here are patches to limit the scope of the remaining global variables. Most changes simply added a preceding static. However, in some cases, I've made minor additional changes, e.g., to make a static table const as well (which sometimes required making an iterator pointer const, too), and to move or remove declarations of variables that the compiler then was able to identify as unused. Initially I put the changes to each file in a separate commit, but that got old quickly, and I lumped all of the remaining changes into the 9th commit. If that's a problem, let me know and I'll separate it. Jim Meyering (9): ccid: declare DEFAULT_ATR table to be static const tcg: declare __jit_debug_descriptor to be static alpha-dis: remove unused global; declare others to be static linux-user: arg_table need not have global scope ccid: make backend_enum_table static const and adjust users sheepdog: declare bdrv_sheepdog to be static mips-dis: declare four globals to be static bonito: declare bonito_state to be static convert many more globals to static alpha-dis.c | 26 ++-- arm-dis.c | 8 ++--- block/sheepdog.c | 2 +- cpus.c| 4 +-- cris-dis.c| 2 +- hw/9pfs/virtio-9p-synth.c | 2 +- hw/bonito.c | 2 +- hw/ccid-card-emulated.c | 6 ++-- hw/ccid-card-passthru.c | 2 +- hw/ide/pci.c | 2 +- hw/leon3.c| 2 +- hw/mips_fulong2e.c| 2 +- hw/s390-virtio-bus.c | 2 +- hw/spapr_rtas.c | 2 +- hw/xen_platform.c | 2 +- hw/xgmac.c| 2 +- linux-user/main.c | 6 ++-- m68k-dis.c| 79 --- memory.c | 2 +- microblaze-dis.c | 6 ++-- mips-dis.c| 15 + ppc-dis.c | 26 sh4-dis.c | 2 +- target-cris/translate.c | 2 +- target-i386/cpu.c | 4 +-- target-i386/kvm.c | 2 +- tcg/tcg.c | 2 +- tests/fdc-test.c | 2 +- vl.c | 12 --- 29 files changed, 110 insertions(+), 118 deletions(-) I've just rebased my local branch with these patches and see that they are still pending. Let me know if you're interested.
Re: [Qemu-devel] [PATCH 4/6] sheepdog: don't leak socket file descriptor upon connection failure
Am 17.08.2012 15:30, schrieb Jim Meyering: Kevin Wolf wrote: Am 16.05.2012 15:07, schrieb Jim Meyering: From: Jim Meyering meyer...@redhat.com Signed-off-by: Jim Meyering meyer...@redhat.com Acked-by: Kevin Wolf kw...@redhat.com Hi Kevin, AFAICS, only one of these 6 patches has been applied. From what I recall (it's been nearly 3mo), there was good feedback and I posted at least one V2 patch. For reference, here's the start of the series: http://marc.info/?l=qemu-develm=133717388221635w=2 Let me know if there's anything I can do to help. Oh, that's bad. This series is spreads across several subsystems, so by acking the sheepdog patch (the only block layer one) I was intending to signal that I'm okay with merging it, but that I expect a global maintainer to actually commit it. Did all your other series get merged? There were a lot more patches with small fixes and I can't see them in git master at all. I seem to remember that they got delayed because you posted them late during the last freeze, but obviously they should have been long committed now. Anthony, what happened with these series? I think it makes sense to pull them into -rc1 because all of them were bug fixes, even though mostly minor ones. Kevin Jim Meyering (5): qemu-ga: don't leak a file descriptor upon failed lockf linux-user: do_msgrcv: don't leak host_mb upon TARGET_EFAULT failure sheepdog: don't leak socket file descriptor upon connection failure arm-semi: don't leak 1KB user string lock buffer upon TARGET_SYS_OPEN softmmu-semi: fix lock_user* functions not to deref NULL upon OOM block/sheepdog.c |1 + linux-user/syscall.c |4 ++-- qemu-ga.c |3 +++ softmmu-semi.h|5 - target-arm/arm-semi.c | 13 +++-- 5 files changed, 17 insertions(+), 9 deletions(-)
Re: [Qemu-devel] [PATCH 4/6] sheepdog: don't leak socket file descriptor upon connection failure
Kevin Wolf wrote: Am 17.08.2012 15:30, schrieb Jim Meyering: Kevin Wolf wrote: Am 16.05.2012 15:07, schrieb Jim Meyering: From: Jim Meyering meyer...@redhat.com Signed-off-by: Jim Meyering meyer...@redhat.com Acked-by: Kevin Wolf kw...@redhat.com Hi Kevin, AFAICS, only one of these 6 patches has been applied. From what I recall (it's been nearly 3mo), there was good feedback and I posted at least one V2 patch. For reference, here's the start of the series: http://marc.info/?l=qemu-develm=133717388221635w=2 Let me know if there's anything I can do to help. Oh, that's bad. This series is spreads across several subsystems, so by acking the sheepdog patch (the only block layer one) I was intending to signal that I'm okay with merging it, but that I expect a global maintainer to actually commit it. Did all your other series get merged? There were a lot more patches with small fixes and I can't see them in git master at all. I seem to remember that they got delayed because you posted them late during the last freeze, but obviously they should have been long committed now. I'm going through them now. So far, it looks like most have been deferred.
Re: [Qemu-devel] [PATCH 0/9] convert many more globals to static
Am 17.08.2012 15:37, schrieb Jim Meyering: Jim Meyering wrote: From: Jim Meyering meyer...@redhat.com Following up on discussion here, http://marc.info/?t=13375948768r=1w=2 here are patches to limit the scope of the remaining global variables. Most changes simply added a preceding static. However, in some cases, I've made minor additional changes, e.g., to make a static table const as well (which sometimes required making an iterator pointer const, too), and to move or remove declarations of variables that the compiler then was able to identify as unused. Initially I put the changes to each file in a separate commit, but that got old quickly, and I lumped all of the remaining changes into the 9th commit. If that's a problem, let me know and I'll separate it. Jim Meyering (9): ccid: declare DEFAULT_ATR table to be static const tcg: declare __jit_debug_descriptor to be static alpha-dis: remove unused global; declare others to be static linux-user: arg_table need not have global scope ccid: make backend_enum_table static const and adjust users sheepdog: declare bdrv_sheepdog to be static mips-dis: declare four globals to be static bonito: declare bonito_state to be static convert many more globals to static alpha-dis.c | 26 ++-- arm-dis.c | 8 ++--- block/sheepdog.c | 2 +- cpus.c| 4 +-- cris-dis.c| 2 +- hw/9pfs/virtio-9p-synth.c | 2 +- hw/bonito.c | 2 +- hw/ccid-card-emulated.c | 6 ++-- hw/ccid-card-passthru.c | 2 +- hw/ide/pci.c | 2 +- hw/leon3.c| 2 +- hw/mips_fulong2e.c| 2 +- hw/s390-virtio-bus.c | 2 +- hw/spapr_rtas.c | 2 +- hw/xen_platform.c | 2 +- hw/xgmac.c| 2 +- linux-user/main.c | 6 ++-- m68k-dis.c| 79 --- memory.c | 2 +- microblaze-dis.c | 6 ++-- mips-dis.c| 15 + ppc-dis.c | 26 sh4-dis.c | 2 +- target-cris/translate.c | 2 +- target-i386/cpu.c | 4 +-- target-i386/kvm.c | 2 +- tcg/tcg.c | 2 +- tests/fdc-test.c | 2 +- vl.c | 12 --- 29 files changed, 110 insertions(+), 118 deletions(-) I've just rebased my local branch with these patches and see that they are still pending. Let me know if you're interested. Hi Jim, I think several of your patches are still missing in QEMU git master. As some of them fix real or potential bugs, they should be applied (if possible to QEMU 1.2 which will be released soon). The usual way to remind people of missing patches is to send a reply mail with ping. CC'ing Anthony Liguori also helps. Regards, Stefan Weil
[Qemu-devel] [PATCH for 1.2] console: Fix warning from clang (and potential crash)
ccc-analyzer reports this warning:
console.c:1090:29: warning: Dereference of null pointer
if (active_console-cursor_timer) {
^
Function console_select allows active_console to be NULL,
but would crash when accessing cursor_timer. Fix this.
Signed-off-by: Stefan Weil s...@weilnetz.de
---
Please note that I don't have a test case which triggers the crash.
Regards,
Stefan Weil
console.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/console.c b/console.c
index 4525cc7..f5e8814 100644
--- a/console.c
+++ b/console.c
@@ -1087,7 +1087,7 @@ void console_select(unsigned int index)
if (s) {
DisplayState *ds = s-ds;
-if (active_console-cursor_timer) {
+if (active_console active_console-cursor_timer) {
qemu_del_timer(active_console-cursor_timer);
}
active_console = s;
--
1.7.10
Re: [Qemu-devel] [PATCH] monitor: Fix warning from clang
On Fri, 17 Aug 2012 15:34:04 +0200
Stefan Weil s...@weilnetz.de wrote:
ccc-analyzer reports these warnings:
monitor.c:3532:21: warning: Division by zero
val %= val2;
^
monitor.c:3530:21: warning: Division by zero
val /= val2;
^
Rewriting the code fixes this (and also a style issue).
Signed-off-by: Stefan Weil s...@weilnetz.de
Reviewed-by: Luiz Capitulino lcapitul...@redhat.com
Although I wonder how far we're going fixing clang warnings/false positives...
---
monitor.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0c34934..0ea2c14 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3524,12 +3524,13 @@ static int64_t expr_prod(Monitor *mon)
break;
case '/':
case '%':
-if (val2 == 0)
+if (val2 == 0) {
expr_error(mon, division by zero);
-if (op == '/')
+} else if (op == '/') {
val /= val2;
-else
+} else {
val %= val2;
+}
break;
}
}
Re: [Qemu-devel] [PATCH 2/7] ppc: Make kvm_arch_put_registers() put *all* the registers
On 08/15/2012 06:33 AM, David Gibson wrote:
At least when invoked with high enough 'level' arguments,
kvm_arch_put_registers() is supposed to copy essentially all the cpu state
as encoded in qemu's internal structures into the kvm state. Currently
the ppc version does not do this - it never calls KVM_SET_SREGS, for
example, and therefore never sets the SDR1 and various other important
though rarely changed registers.
Instead, the code paths which need to set these registers need to
explicitly make (conditional) kvm calls which transfer the changes to kvm.
This breaks the usual model of handling state updates in qemu, where code
just changes the internal model and has it flushed out to kvm automatically
at some later point.
This patch fixes this for Book S ppc CPUs by adding a suitable call to
KVM_SET_SREGS and als to KVM_SET_ONE_REG to set the HIOR (the only register
that is set with that call so far). This lets us remove the hacks to
explicitly set these registers from the kvmppc_set_papr() function.
HIOR is a read-only register from the guest's point of view when running
in PAPR mode, so we don't need to sync it back again. The same goes for
SDR1, though resetting that is valid for non-PAPR guests.
Overall, does a normal system reset on PPC guarantee that the SRs and
SLBs are reset? At least OpenBIOS boots up in real mode and overwrites
all SR/SLB entries while still in real mode.
Alex
The problem still exists for Book E CPUs (which use a different version of
the kvm_sregs structure). But fixing that has some complications of its
own so can be left to another day.
Lkewise, there is still some ugly code for setting the PVR through special
calls to SET_SREGS which is left in for now. The PVR needs to be set
especially early because it can affect what other features are available
on the CPU, so I need to do more thinking to see if it can be integrated
into the normal paths or not.
Signed-off-by: David Gibson da...@gibson.dropbear.id.au
---
target-ppc/kvm.c | 89 ++
1 file changed, 50 insertions(+), 39 deletions(-)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index a31d278..1a7489b 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -60,6 +60,7 @@ static int cap_booke_sregs;
static int cap_ppc_smt;
static int cap_ppc_rma;
static int cap_spapr_tce;
+static int cap_hior;
/* XXX We have a race condition where we actually have a level triggered
* interrupt, but the infrastructure can't expose that yet, so the guest
@@ -86,6 +87,7 @@ int kvm_arch_init(KVMState *s)
cap_ppc_smt = kvm_check_extension(s, KVM_CAP_PPC_SMT);
cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
+cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
if (!cap_interrupt_level) {
fprintf(stderr, KVM: Couldn't find level irq capability. Expect the
@@ -469,6 +471,53 @@ int kvm_arch_put_registers(CPUPPCState *env, int level)
env-tlb_dirty = false;
}
+if (cap_segstate (level = KVM_PUT_RESET_STATE)) {
+struct kvm_sregs sregs;
+
+sregs.pvr = env-spr[SPR_PVR];
+
+sregs.u.s.sdr1 = env-spr[SPR_SDR1];
+
+/* Sync SLB */
+#ifdef TARGET_PPC64
+for (i = 0; i 64; i++) {
+sregs.u.s.ppc64.slb[i].slbe = env-slb[i].esid;
+sregs.u.s.ppc64.slb[i].slbv = env-slb[i].vsid;
+}
+#endif
+
+/* Sync SRs */
+for (i = 0; i 16; i++) {
+sregs.u.s.ppc32.sr[i] = env-sr[i];
+}
+
+/* Sync BATs */
+for (i = 0; i 8; i++) {
+sregs.u.s.ppc32.dbat[i] = ((uint64_t)env-DBAT[1][i] 32)
+| env-DBAT[0][i];
+sregs.u.s.ppc32.ibat[i] = ((uint64_t)env-IBAT[1][i] 32)
+| env-IBAT[0][i];
+}
+
+ret = kvm_vcpu_ioctl(env, KVM_SET_SREGS, sregs);
+if (ret) {
+return ret;
+}
+}
+
+if (cap_hior (level = KVM_PUT_RESET_STATE)) {
+uint64_t hior = env-spr[SPR_HIOR];
+struct kvm_one_reg reg = {
+.id = KVM_REG_PPC_HIOR,
+.addr = (uintptr_t) hior,
+};
+
+ret = kvm_vcpu_ioctl(env, KVM_SET_ONE_REG, reg);
+if (ret) {
+return ret;
+}
+}
+
return ret;
}
@@ -946,52 +995,14 @@ int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len)
void kvmppc_set_papr(CPUPPCState *env)
{
struct kvm_enable_cap cap = {};
-struct kvm_one_reg reg = {};
-struct kvm_sregs sregs = {};
int ret;
-uint64_t hior = env-spr[SPR_HIOR];
cap.cap = KVM_CAP_PPC_PAPR;
ret = kvm_vcpu_ioctl(env, KVM_ENABLE_CAP, cap);
if (ret) {
-goto fail;
-}
-
-/*
- * XXX We set HIOR here. It really should be a qdev property of
- * the CPU node, but we don't have CPUs converted to qdev yet.
- *
- *
Re: [Qemu-devel] [PATCH] monitor: Fix warning from clang
Stefan Weil s...@weilnetz.de writes:
ccc-analyzer reports these warnings:
monitor.c:3532:21: warning: Division by zero
val %= val2;
^
monitor.c:3530:21: warning: Division by zero
val /= val2;
^
Rewriting the code fixes this (and also a style issue).
I'm afraid this doesn't actually fix anything, because...
Signed-off-by: Stefan Weil s...@weilnetz.de
---
monitor.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0c34934..0ea2c14 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3524,12 +3524,13 @@ static int64_t expr_prod(Monitor *mon)
break;
case '/':
case '%':
-if (val2 == 0)
+if (val2 == 0) {
expr_error(mon, division by zero);
-if (op == '/')
+} else if (op == '/') {
val /= val2;
-else
+} else {
val %= val2;
+}
break;
}
}
... expr_error() longjmp()s out. The expression evaluator commonly
exploits that.
If expr_error() returned, the code would be just as wrong after your
patch as before.
Perhaps the checker can be shut up by making expr_error() QEMU_NORETURN.
Re: [Qemu-devel] [PATCH for 1.2] console: Fix warning from clang (and potential crash)
On 2012-08-17 15:50, Stefan Weil wrote:
ccc-analyzer reports this warning:
console.c:1090:29: warning: Dereference of null pointer
if (active_console-cursor_timer) {
^
Function console_select allows active_console to be NULL,
but would crash when accessing cursor_timer. Fix this.
Signed-off-by: Stefan Weil s...@weilnetz.de
---
Please note that I don't have a test case which triggers the crash.
Regards,
Stefan Weil
console.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/console.c b/console.c
index 4525cc7..f5e8814 100644
--- a/console.c
+++ b/console.c
@@ -1087,7 +1087,7 @@ void console_select(unsigned int index)
if (s) {
DisplayState *ds = s-ds;
-if (active_console-cursor_timer) {
+if (active_console active_console-cursor_timer) {
qemu_del_timer(active_console-cursor_timer);
}
active_console = s;
The only path that could trigger this is console_select() in the absence
of any console. Not sure if that is possible, but the above is surely
consistent with existing code.
Reviewed-by: Jan Kiszka jan.kis...@siemens.com
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] monitor: Fix warning from clang
On Fri, 17 Aug 2012 16:10:12 +0200
Markus Armbruster arm...@redhat.com wrote:
Stefan Weil s...@weilnetz.de writes:
ccc-analyzer reports these warnings:
monitor.c:3532:21: warning: Division by zero
val %= val2;
^
monitor.c:3530:21: warning: Division by zero
val /= val2;
^
Rewriting the code fixes this (and also a style issue).
I'm afraid this doesn't actually fix anything, because...
Signed-off-by: Stefan Weil s...@weilnetz.de
---
monitor.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0c34934..0ea2c14 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3524,12 +3524,13 @@ static int64_t expr_prod(Monitor *mon)
break;
case '/':
case '%':
-if (val2 == 0)
+if (val2 == 0) {
expr_error(mon, division by zero);
-if (op == '/')
+} else if (op == '/') {
val /= val2;
-else
+} else {
val %= val2;
+}
break;
}
}
... expr_error() longjmp()s out. The expression evaluator commonly
exploits that.
And that's correct. As far far I understood it's fixing clang, not qemu.
If expr_error() returned, the code would be just as wrong after your
patch as before.
Hmm, how? It checks for val2 == 0 first.
Perhaps the checker can be shut up by making expr_error() QEMU_NORETURN.
That's indeed a better solution.
Re: [Qemu-devel] [RFC PATCH] vfio: add fixup for broken PCI devices
On Fri, Jun 22, 2012 at 6:16 PM, Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 07/06/12 09:17, Alex Williamson wrote:
On Fri, 2012-05-25 at 17:35 +1000, Alexey Kardashevskiy wrote:
Some adapters (like NEC PCI USB controller) do not flush their config
on a sioftware reset and remember DMA config, etc.
If we use such an adapter with QEMU, then crash QEMU (stop it with
ctrl-A ctrl-X), and try to use it in QEMU again, it may start working
immediately with previous config when pci_enable_device() is called
on that PCI function.
To eliminate such effect, some quirk should be called. The proposed
pci_fixup_final does its job well for mentioned NEC PCI USB but not
sure if it is 100% correct.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
drivers/vfio/pci/vfio_pci.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 1e5315c..6e7c12d 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -88,6 +88,8 @@ static void vfio_pci_disable(struct vfio_pci_device
*vdev)
{
int bar;
+pci_fixup_device(pci_fixup_final, vdev-pdev);
+
pci_disable_device(vdev-pdev);
vfio_pci_set_irqs_ioctl(vdev, VFIO_IRQ_SET_DATA_NONE |
Sorry, just taking a look at this again. Do you have any idea what
fixup it is that makes it work? Calling a fixup at this point seems
rather odd. I suspect the problem is that vfio is only calling
pci_load_and_free_saved_state if pci_reset_function reports that it
worked. kvm device assignment doesn't do that and I'm not sure why I
did that. If you unconditionally call pci_load_and_free_saved_state a
bit further down in this function, does it solve the problem? Thanks,
Checked again.
Seems to be a false alarm, cannot reproduce the bad behavior anymore,
looks like it was caused by
another issue which Alex fixed.
So although the problem may arise again, there is nothing urgent to do at
the moment.
While doing cleanups in my SPAPR IOMMU driver for VFIO,
I found that I have not unmapped all the pages on module_exit.
Heh. My bad. So I implemented that and got a lot of strange accidental
crashes of the host kernel when I tried to debug
USB Controller: NEC Corporation USB (rev 43).
I applied the quoted patch and it has gone.
You asked what fixup exactly does but honestly I do not know.
From the code, it enables a device, does some tricks in
quirk_usb_handoff_ohci()/quirk_usb_disable_ehci() and
disables the device back. The comments of these quirks
say that they basically disable interrupts and do shutdown/reset
via OHCI/EHCI specific registers as pci_disable_device() does
not do the job like it does not reset the device's DMA config so when
it gets enabled back, it continues DMA transfer to/from addresses
which were actual at the moment of QEMU shutdown/group release.
So do we add a new class of quirks?
--
Alexey
Re: [Qemu-devel] qemu-kvm-1.0.1 - unable to exit if vcpu is in infinite loop
On 2012-08-17 15:11, Jan Kiszka wrote: On 2012-08-06 17:11, Stefan Hajnoczi wrote: On Thu, Jun 28, 2012 at 2:05 PM, Peter Lieven p...@dlhnet.de wrote: i debugged my initial problem further and found out that the problem happens to be that the main thread is stuck in pause_all_vcpus() on reset or quit commands in the monitor if one cpu is stuck in the do-while loop kvm_cpu_exec. If I modify the condition from while (ret == 0) to while ((ret == 0) !env-stop); it works, but is this the right fix? Quit command seems to work, but on Reset the VM enterns pause state. I think I'm hitting something similar. I installed a F17 amd64 guest (3.5 kernel) but before booting entered the GRUB boot menu edit mode. The guest seemed unresponsive so I switched to the monitor, which also froze shortly afterwards. The VNC screen ended up being all black. qemu-kvm.git/master 3e4305694fd891b69e4450e59ec4c65420907ede Linux 3.2.0-3-amd64 from Debian testing $ qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive if=virtio,cache=none,file=f17.img,aio=native -serial stdio (gdb) thread apply all bt Thread 3 (Thread 0x7f8008e23700 (LWP 367)): #0 0x7f800f891727 in ioctl () at ../sysdeps/unix/syscall-template.S:82 #1 0x7f80137b92c9 in kvm_vcpu_ioctl (env=env@entry=0x7f8015b49640, type=type@entry=44672) at /home/stefanha/qemu-kvm/kvm-all.c:1619 #2 0x7f80137b93fe in kvm_cpu_exec (env=env@entry=0x7f8015b49640) at /home/stefanha/qemu-kvm/kvm-all.c:1506 #3 0x7f8013766f31 in qemu_kvm_cpu_thread_fn (arg=0x7f8015b49640) at /home/stefanha/qemu-kvm/cpus.c:756 #4 0x7f800fb4db50 in start_thread (arg=optimized out) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () This vcpu is still executing guest code and I've seen it successfully dispatching I/O. The problem is it's missing the exit_request... Thread 2 (Thread 0x7f8008622700 (LWP 368)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=optimized out, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013766eff in qemu_kvm_wait_io_event (env=optimized out) at /home/stefanha/qemu-kvm/cpus.c:724 #3 qemu_kvm_cpu_thread_fn (arg=0x7f8015b67450) at /home/stefanha/qemu-kvm/cpus.c:761 #4 0x7f800fb4db50 in start_thread (arg=optimized out) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () No problems here. Thread 1 (Thread 0x7f801347b8c0 (LWP 365)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=cond@entry=0x7f801402fd80, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013768949 in pause_all_vcpus () at /home/stefanha/qemu-kvm/cpus.c:962 #3 0x7f80136028c8 in main (argc=optimized out, argv=optimized out, envp=optimized out) at /home/stefanha/qemu-kvm/vl.c:3695 We're deadlocked in pause_all_vcpus(), waiting for vcpu #0 to pause. Unfortunately vcpu #0 has -exit_request=0 although -stop=1. Here are the vcpus: (gdb) p first_cpu $6 = (struct CPUX86State *) 0x7f8015b49640 (gdb) p first_cpu-next_cpu $7 = (struct CPUX86State *) 0x7f8015b67450 (gdb) p first_cpu-next_cpu-next_cpu $8 = (struct CPUX86State *) 0x0 (gdb) p first_cpu-stop $9 = 1 (gdb) p first_cpu-stopped $10 = 0 (gdb) p first_cpu-exit_request $11 = 0 CPUState::exit_request is only set on specific synchronous events, see target-i386/kvm.c. More interesting is CPUState::thread_kicked. If it's set, qemu_cpu_kick will skip the kicking via a signal. Maybe there is some race. Let me think about such possibilities again... diff --git a/cpus.c b/cpus.c index e476a3c..30f3228 100644 --- a/cpus.c +++ b/cpus.c @@ -726,6 +726,9 @@ static void qemu_kvm_wait_io_event(CPUArchState *env) } qemu_kvm_eat_signals(env); +/* Ensure that checking env-stop cannot overtake signal processing so + * that we lose the latter without stopping. */ +smp_rmb(); qemu_wait_io_event_common(env); } Can anyone imagine that such a barrier may actually be required? If it is currently possible that env-stop is evaluated before we called into sigtimedwait in qemu_kvm_eat_signals, then we could actually eat the signal without properly processing its reason (stop). Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] qemu-kvm-1.0.1 - unable to exit if vcpu is in infinite loop
On 2012-08-17 16:36, Jan Kiszka wrote: On 2012-08-17 15:11, Jan Kiszka wrote: On 2012-08-06 17:11, Stefan Hajnoczi wrote: On Thu, Jun 28, 2012 at 2:05 PM, Peter Lieven p...@dlhnet.de wrote: i debugged my initial problem further and found out that the problem happens to be that the main thread is stuck in pause_all_vcpus() on reset or quit commands in the monitor if one cpu is stuck in the do-while loop kvm_cpu_exec. If I modify the condition from while (ret == 0) to while ((ret == 0) !env-stop); it works, but is this the right fix? Quit command seems to work, but on Reset the VM enterns pause state. I think I'm hitting something similar. I installed a F17 amd64 guest (3.5 kernel) but before booting entered the GRUB boot menu edit mode. The guest seemed unresponsive so I switched to the monitor, which also froze shortly afterwards. The VNC screen ended up being all black. qemu-kvm.git/master 3e4305694fd891b69e4450e59ec4c65420907ede Linux 3.2.0-3-amd64 from Debian testing $ qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive if=virtio,cache=none,file=f17.img,aio=native -serial stdio (gdb) thread apply all bt Thread 3 (Thread 0x7f8008e23700 (LWP 367)): #0 0x7f800f891727 in ioctl () at ../sysdeps/unix/syscall-template.S:82 #1 0x7f80137b92c9 in kvm_vcpu_ioctl (env=env@entry=0x7f8015b49640, type=type@entry=44672) at /home/stefanha/qemu-kvm/kvm-all.c:1619 #2 0x7f80137b93fe in kvm_cpu_exec (env=env@entry=0x7f8015b49640) at /home/stefanha/qemu-kvm/kvm-all.c:1506 #3 0x7f8013766f31 in qemu_kvm_cpu_thread_fn (arg=0x7f8015b49640) at /home/stefanha/qemu-kvm/cpus.c:756 #4 0x7f800fb4db50 in start_thread (arg=optimized out) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () This vcpu is still executing guest code and I've seen it successfully dispatching I/O. The problem is it's missing the exit_request... Thread 2 (Thread 0x7f8008622700 (LWP 368)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=optimized out, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013766eff in qemu_kvm_wait_io_event (env=optimized out) at /home/stefanha/qemu-kvm/cpus.c:724 #3 qemu_kvm_cpu_thread_fn (arg=0x7f8015b67450) at /home/stefanha/qemu-kvm/cpus.c:761 #4 0x7f800fb4db50 in start_thread (arg=optimized out) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () No problems here. Thread 1 (Thread 0x7f801347b8c0 (LWP 365)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=cond@entry=0x7f801402fd80, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013768949 in pause_all_vcpus () at /home/stefanha/qemu-kvm/cpus.c:962 #3 0x7f80136028c8 in main (argc=optimized out, argv=optimized out, envp=optimized out) at /home/stefanha/qemu-kvm/vl.c:3695 We're deadlocked in pause_all_vcpus(), waiting for vcpu #0 to pause. Unfortunately vcpu #0 has -exit_request=0 although -stop=1. Here are the vcpus: (gdb) p first_cpu $6 = (struct CPUX86State *) 0x7f8015b49640 (gdb) p first_cpu-next_cpu $7 = (struct CPUX86State *) 0x7f8015b67450 (gdb) p first_cpu-next_cpu-next_cpu $8 = (struct CPUX86State *) 0x0 (gdb) p first_cpu-stop $9 = 1 (gdb) p first_cpu-stopped $10 = 0 (gdb) p first_cpu-exit_request $11 = 0 CPUState::exit_request is only set on specific synchronous events, see target-i386/kvm.c. More interesting is CPUState::thread_kicked. If it's set, qemu_cpu_kick will skip the kicking via a signal. Maybe there is some race. Let me think about such possibilities again... diff --git a/cpus.c b/cpus.c index e476a3c..30f3228 100644 --- a/cpus.c +++ b/cpus.c @@ -726,6 +726,9 @@ static void qemu_kvm_wait_io_event(CPUArchState *env) } qemu_kvm_eat_signals(env); +/* Ensure that checking env-stop cannot overtake signal processing so + * that we lose the latter without stopping. */ +smp_rmb(); rmb is nonsense. Should be a plain barrier() - if at all. qemu_wait_io_event_common(env); } Can anyone imagine that such a barrier may actually be required? If it is currently possible that env-stop is evaluated before we called into sigtimedwait in qemu_kvm_eat_signals, then we could actually eat the signal without properly processing its reason (stop). Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] monitor: Fix warning from clang
Luiz Capitulino lcapitul...@redhat.com writes:
On Fri, 17 Aug 2012 16:10:12 +0200
Markus Armbruster arm...@redhat.com wrote:
Stefan Weil s...@weilnetz.de writes:
ccc-analyzer reports these warnings:
monitor.c:3532:21: warning: Division by zero
val %= val2;
^
monitor.c:3530:21: warning: Division by zero
val /= val2;
^
Rewriting the code fixes this (and also a style issue).
I'm afraid this doesn't actually fix anything, because...
Signed-off-by: Stefan Weil s...@weilnetz.de
---
monitor.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0c34934..0ea2c14 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3524,12 +3524,13 @@ static int64_t expr_prod(Monitor *mon)
break;
case '/':
case '%':
-if (val2 == 0)
+if (val2 == 0) {
expr_error(mon, division by zero);
-if (op == '/')
+} else if (op == '/') {
val /= val2;
-else
+} else {
val %= val2;
+}
break;
}
}
... expr_error() longjmp()s out. The expression evaluator commonly
exploits that.
And that's correct. As far far I understood it's fixing clang, not qemu.
If expr_error() returned, the code would be just as wrong after your
patch as before.
Hmm, how? It checks for val2 == 0 first.
It would evaluate A % 0 into A, which is wrong.
Perhaps the checker can be shut up by making expr_error() QEMU_NORETURN.
That's indeed a better solution.
Stefan, could you try that for us?
Re: [Qemu-devel] [PATCH] monitor: Fix warning from clang
On Fri, 17 Aug 2012 16:41:34 +0200
Markus Armbruster arm...@redhat.com wrote:
Luiz Capitulino lcapitul...@redhat.com writes:
On Fri, 17 Aug 2012 16:10:12 +0200
Markus Armbruster arm...@redhat.com wrote:
Stefan Weil s...@weilnetz.de writes:
ccc-analyzer reports these warnings:
monitor.c:3532:21: warning: Division by zero
val %= val2;
^
monitor.c:3530:21: warning: Division by zero
val /= val2;
^
Rewriting the code fixes this (and also a style issue).
I'm afraid this doesn't actually fix anything, because...
Signed-off-by: Stefan Weil s...@weilnetz.de
---
monitor.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0c34934..0ea2c14 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3524,12 +3524,13 @@ static int64_t expr_prod(Monitor *mon)
break;
case '/':
case '%':
-if (val2 == 0)
+if (val2 == 0) {
expr_error(mon, division by zero);
-if (op == '/')
+} else if (op == '/') {
val /= val2;
-else
+} else {
val %= val2;
+}
break;
}
}
... expr_error() longjmp()s out. The expression evaluator commonly
exploits that.
And that's correct. As far far I understood it's fixing clang, not qemu.
If expr_error() returned, the code would be just as wrong after your
patch as before.
Hmm, how? It checks for val2 == 0 first.
It would evaluate A % 0 into A, which is wrong.
Oh, you're talking about the result that would be returned by expr_prod().
I thought you were saying that val2 == 0 was still possible.
Perhaps the checker can be shut up by making expr_error() QEMU_NORETURN.
That's indeed a better solution.
Stefan, could you try that for us?
Re: [Qemu-devel] qemu-kvm-1.0.1 - unable to exit if vcpu is in infinite loop
On 2012-08-17 16:41, Jan Kiszka wrote: On 2012-08-17 16:36, Jan Kiszka wrote: On 2012-08-17 15:11, Jan Kiszka wrote: On 2012-08-06 17:11, Stefan Hajnoczi wrote: On Thu, Jun 28, 2012 at 2:05 PM, Peter Lieven p...@dlhnet.de wrote: i debugged my initial problem further and found out that the problem happens to be that the main thread is stuck in pause_all_vcpus() on reset or quit commands in the monitor if one cpu is stuck in the do-while loop kvm_cpu_exec. If I modify the condition from while (ret == 0) to while ((ret == 0) !env-stop); it works, but is this the right fix? Quit command seems to work, but on Reset the VM enterns pause state. I think I'm hitting something similar. I installed a F17 amd64 guest (3.5 kernel) but before booting entered the GRUB boot menu edit mode. The guest seemed unresponsive so I switched to the monitor, which also froze shortly afterwards. The VNC screen ended up being all black. qemu-kvm.git/master 3e4305694fd891b69e4450e59ec4c65420907ede Linux 3.2.0-3-amd64 from Debian testing $ qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive if=virtio,cache=none,file=f17.img,aio=native -serial stdio (gdb) thread apply all bt Thread 3 (Thread 0x7f8008e23700 (LWP 367)): #0 0x7f800f891727 in ioctl () at ../sysdeps/unix/syscall-template.S:82 #1 0x7f80137b92c9 in kvm_vcpu_ioctl (env=env@entry=0x7f8015b49640, type=type@entry=44672) at /home/stefanha/qemu-kvm/kvm-all.c:1619 #2 0x7f80137b93fe in kvm_cpu_exec (env=env@entry=0x7f8015b49640) at /home/stefanha/qemu-kvm/kvm-all.c:1506 #3 0x7f8013766f31 in qemu_kvm_cpu_thread_fn (arg=0x7f8015b49640) at /home/stefanha/qemu-kvm/cpus.c:756 #4 0x7f800fb4db50 in start_thread (arg=optimized out) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () This vcpu is still executing guest code and I've seen it successfully dispatching I/O. The problem is it's missing the exit_request... Thread 2 (Thread 0x7f8008622700 (LWP 368)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=optimized out, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013766eff in qemu_kvm_wait_io_event (env=optimized out) at /home/stefanha/qemu-kvm/cpus.c:724 #3 qemu_kvm_cpu_thread_fn (arg=0x7f8015b67450) at /home/stefanha/qemu-kvm/cpus.c:761 #4 0x7f800fb4db50 in start_thread (arg=optimized out) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () No problems here. Thread 1 (Thread 0x7f801347b8c0 (LWP 365)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=cond@entry=0x7f801402fd80, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013768949 in pause_all_vcpus () at /home/stefanha/qemu-kvm/cpus.c:962 #3 0x7f80136028c8 in main (argc=optimized out, argv=optimized out, envp=optimized out) at /home/stefanha/qemu-kvm/vl.c:3695 We're deadlocked in pause_all_vcpus(), waiting for vcpu #0 to pause. Unfortunately vcpu #0 has -exit_request=0 although -stop=1. Here are the vcpus: (gdb) p first_cpu $6 = (struct CPUX86State *) 0x7f8015b49640 (gdb) p first_cpu-next_cpu $7 = (struct CPUX86State *) 0x7f8015b67450 (gdb) p first_cpu-next_cpu-next_cpu $8 = (struct CPUX86State *) 0x0 (gdb) p first_cpu-stop $9 = 1 (gdb) p first_cpu-stopped $10 = 0 (gdb) p first_cpu-exit_request $11 = 0 CPUState::exit_request is only set on specific synchronous events, see target-i386/kvm.c. More interesting is CPUState::thread_kicked. If it's set, qemu_cpu_kick will skip the kicking via a signal. Maybe there is some race. Let me think about such possibilities again... diff --git a/cpus.c b/cpus.c index e476a3c..30f3228 100644 --- a/cpus.c +++ b/cpus.c @@ -726,6 +726,9 @@ static void qemu_kvm_wait_io_event(CPUArchState *env) } qemu_kvm_eat_signals(env); +/* Ensure that checking env-stop cannot overtake signal processing so + * that we lose the latter without stopping. */ +smp_rmb(); rmb is nonsense. Should be a plain barrier() - if at all. qemu_wait_io_event_common(env); } Can anyone imagine that such a barrier may actually be required? If it is currently possible that env-stop is evaluated before we called into sigtimedwait in qemu_kvm_eat_signals, then we could actually eat the signal without properly processing its reason (stop). Should not be required (TM): Both signal eating / stop checking and stop setting / signal generation happens under the BQL, thus the ordering must not make a difference here. Don't see where we could lose a signal. Maybe due to a subtle memory
Re: [Qemu-devel] [PATCHv2 2/2] envlist.c: handle strdup failure
Am 17.08.2012 15:35, schrieb Jim Meyering:
Jim Meyering wrote:
From: Jim Meyering meyer...@redhat.com
Without this, envlist_to_environ may silently fail to copy all
strings into the destination buffer, and both callers would leak
any env strings allocated after a failing strdup, because the
freeing code stops at the first NULL pointer.
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/envlist.c b/envlist.c
index be0addb..7532091 100644
--- a/envlist.c
+++ b/envlist.c
@@ -234,8 +234,16 @@ envlist_to_environ(const envlist_t *envlist, size_t
*count)
return (NULL);
for (entry = envlist-el_entries.lh_first; entry != NULL;
-entry = entry-ev_link.le_next) {
-*(penv++) = strdup(entry-ev_var);
+ entry = entry-ev_link.le_next, penv++) {
+*penv = strdup(entry-ev_var);
+if (*penv == NULL) {
+char **e = env;
+while (e = penv) {
+free(*e++);
+}
+free(env);
+return NULL;
+}
}
*penv = NULL; /* NULL terminate the list */
It seems this has been lost in this list's high volume of patches.
Anyone interested? Repost desired?
You announced a v3 but replied to v2? Indentation looks odd in 2/2, too.
Andreas
b/envlist.c | 256
++--
envlist.c | 12 ++
2 files changed, 138 insertions(+), 130 deletions(-)
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH] usb-redir: Never return USB_RET_NAK for async handled packets
USB_RET_NAK is not a valid response for async handled packets (and will trigger an assert as such). Also drop the warning when receiving a status of cancelled for packets not cancelled by qemu itself, this can happen when a device gets unredirected by the usbredir-host while transfers are pending. Signed-off-by: Hans de Goede hdego...@redhat.com --- hw/usb/redirect.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 1460515..9ba964e 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -1055,11 +1055,14 @@ static int usbredir_handle_status(USBRedirDevice *dev, case usb_redir_stall: return USB_RET_STALL; case usb_redir_cancelled: -WARNING(returning cancelled packet to HC?\n); -return USB_RET_NAK; +/* + * When the usbredir-host unredirects a device, it will report a status + * of cancelled for all pending packets, followed by a disconnect msg. + */ +return USB_RET_IOERROR; case usb_redir_inval: WARNING(got invalid param error from usb-host?\n); -return USB_RET_NAK; +return USB_RET_IOERROR; case usb_redir_babble: return USB_RET_BABBLE; case usb_redir_ioerror: -- 1.7.11.4
Re: [Qemu-devel] [PATCHv2 2/2] envlist.c: handle strdup failure
Andreas Färber wrote:
Am 17.08.2012 15:35, schrieb Jim Meyering:
Jim Meyering wrote:
From: Jim Meyering meyer...@redhat.com
Without this, envlist_to_environ may silently fail to copy all
strings into the destination buffer, and both callers would leak
any env strings allocated after a failing strdup, because the
freeing code stops at the first NULL pointer.
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/envlist.c b/envlist.c
index be0addb..7532091 100644
--- a/envlist.c
+++ b/envlist.c
@@ -234,8 +234,16 @@ envlist_to_environ(const envlist_t *envlist, size_t
*count)
return (NULL);
for (entry = envlist-el_entries.lh_first; entry != NULL;
-entry = entry-ev_link.le_next) {
-*(penv++) = strdup(entry-ev_var);
+ entry = entry-ev_link.le_next, penv++) {
+*penv = strdup(entry-ev_var);
+if (*penv == NULL) {
+char **e = env;
+while (e = penv) {
+free(*e++);
+}
+free(env);
+return NULL;
+}
}
*penv = NULL; /* NULL terminate the list */
It seems this has been lost in this list's high volume of patches.
Anyone interested? Repost desired?
You announced a v3 but replied to v2?
Here are links to v3:
[PATCHv3 1/2] envlist.c: convert each TAB(width-4) to equivalent spaces
https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02967.html
[Qemu-devel] [PATCHv3 2/2] envlist.c: handle strdup failure
https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02966.html
Indentation looks odd in 2/2, too.
That was fixed in v3.
[Qemu-devel] [PATCH v7 1.2] qxl: add QXL_IO_MONITORS_CONFIG_ASYNC
Revision bumped to 4 for new IO support, enabled for spice-server =
0.11.1. New io enabled iff spice-server = 0.11.1 spice-protocol =
0.12.0.
On migration reissue spice_qxl_monitors_config_async.
RHBZ: 770842
Signed-off-by: Alon Levy al...@redhat.com
---
Fixed another defined I missed. This time used grep to verify it's the last
one..
Alon
configure | 3 +++
hw/qxl.c | 60 --
hw/qxl.h | 7 +++
trace-events | 1 +
ui/spice-display.h | 1 +
5 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index cc774b5..dbf3af6 100755
--- a/configure
+++ b/configure
@@ -2657,6 +2657,9 @@ EOF
spice=yes
libs_softmmu=$libs_softmmu $spice_libs
QEMU_CFLAGS=$QEMU_CFLAGS $spice_cflags
+if $pkg_config --atleast-version=0.12.0 spice-protocol /dev/null 21;
then
+QEMU_CFLAGS=$QEMU_CFLAGS -DQXL_HAS_IO_MONITORS_CONFIG_ASYNC=1
+fi
else
if test $spice = yes ; then
feature_not_found spice
diff --git a/hw/qxl.c b/hw/qxl.c
index c978f5e..25d7759 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -27,6 +27,10 @@
#include qxl.h
+#ifndef QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+#define QXL_HAS_IO_MONITORS_CONFIG_ASYNC 0
+#endif
+
/*
* NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as
* such can be changed by the guest, so to avoid a guest trigerrable
@@ -249,6 +253,20 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl,
qxl_async_io async)
}
}
+/* 0x00b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl)
+{
+trace_qxl_spice_monitors_config(qxl-id);
+qxl-guest_monitors_config = qxl-ram-monitors_config;
+spice_qxl_monitors_config_async(qxl-ssd.qxl,
+qxl-ram-monitors_config,
+MEMSLOT_GROUP_GUEST,
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+ QXL_IO_MONITORS_CONFIG_ASYNC));
+}
+#endif
+
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
{
trace_qxl_spice_reset_image_cache(qxl-id);
@@ -538,6 +556,9 @@ static const char *io_port_to_string(uint32_t io_port)
= QXL_IO_DESTROY_ALL_SURFACES_ASYNC,
[QXL_IO_FLUSH_SURFACES_ASYNC] = QXL_IO_FLUSH_SURFACES_ASYNC,
[QXL_IO_FLUSH_RELEASE] = QXL_IO_FLUSH_RELEASE,
+#if QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+[QXL_IO_MONITORS_CONFIG_ASYNC] = QXL_IO_MONITORS_CONFIG_ASYNC,
+#endif
};
return io_port_to_string[io_port];
}
@@ -819,7 +840,10 @@ static void interface_async_complete_io(PCIQXLDevice *qxl,
QXLCookie *cookie)
case QXL_IO_DESTROY_PRIMARY_ASYNC:
case QXL_IO_UPDATE_AREA_ASYNC:
case QXL_IO_FLUSH_SURFACES_ASYNC:
+#if QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+case QXL_IO_MONITORS_CONFIG_ASYNC:
break;
+#endif
case QXL_IO_CREATE_PRIMARY_ASYNC:
qxl_create_guest_primary_complete(qxl);
break;
@@ -894,6 +918,8 @@ static void interface_async_complete(QXLInstance *sin,
uint64_t cookie_token)
case QXL_COOKIE_TYPE_RENDER_UPDATE_AREA:
qxl_render_update_area_done(qxl, cookie);
break;
+case QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG:
+break;
default:
fprintf(stderr, qxl: %s: unexpected cookie type %d\n,
__func__, cookie-type);
@@ -1333,7 +1359,7 @@ static void ioport_write(void *opaque, target_phys_addr_t
addr,
io_port, io_port_to_string(io_port));
/* be nice to buggy guest drivers */
if (io_port = QXL_IO_UPDATE_AREA_ASYNC
-io_port = QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
+io_port QXL_IO_RANGE_SIZE) {
qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
}
return;
@@ -1361,6 +1387,10 @@ static void ioport_write(void *opaque,
target_phys_addr_t addr,
io_port = QXL_IO_DESTROY_ALL_SURFACES;
goto async_common;
case QXL_IO_FLUSH_SURFACES_ASYNC:
+/* 0x000b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+#endif
async_common:
async = QXL_ASYNC;
qemu_mutex_lock(d-async_lock);
@@ -1502,6 +1532,12 @@ async_common:
d-mode = QXL_MODE_UNDEFINED;
qxl_spice_destroy_surfaces(d, async);
break;
+/* 0x000b01 == 0.11.1 */
+#if SPICE_SERVER_VERSION = 0x000b01 QXL_HAS_IO_MONITORS_CONFIG_ASYNC
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+qxl_spice_monitors_config_async(d);
+break;
+#endif
default:
qxl_set_guest_bug(d, %s: unexpected ioport=0x%x\n, __func__,
io_port);
}
@@ -1797,9 +1833,15 @@ static int qxl_init_common(PCIQXLDevice *qxl)
io_size = 16;
break;
case 3: /* qxl-3 */
+pci_device_rev = QXL_REVISION_STABLE_V10;
+io_size = 32; /* PCI region size must be pow2 */
+
Re: [Qemu-devel] [PATCHv3 1/2] envlist.c: convert each TAB(width-4) to equivalent spaces
Am 22.05.2012 12:16, schrieb Jim Meyering:
From: Jim Meyering meyer...@redhat.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 256
+++---
1 file changed, 128 insertions(+), 128 deletions(-)
diff --git a/envlist.c b/envlist.c
index f2303cd..e44889b 100644
--- a/envlist.c
+++ b/envlist.c
@@ -8,13 +8,13 @@
#include envlist.h
struct envlist_entry {
- const char *ev_var; /* actual env value */
- QLIST_ENTRY(envlist_entry) ev_link;
+const char *ev_var; /* actual env value */
+QLIST_ENTRY(envlist_entry) ev_link;
};
struct envlist {
- QLIST_HEAD(, envlist_entry) el_entries; /* actual entries */
- size_t el_count;/* number of entries */
+QLIST_HEAD(, envlist_entry) el_entries; /* actual entries */
+size_t el_count;/* number of entries */
};
static int envlist_parse(envlist_t *envlist,
@@ -27,15 +27,15 @@ static int envlist_parse(envlist_t *envlist,
envlist_t *
envlist_create(void)
{
- envlist_t *envlist;
+envlist_t *envlist;
- if ((envlist = malloc(sizeof (*envlist))) == NULL)
- return (NULL);
+if ((envlist = malloc(sizeof (*envlist))) == NULL)
+return (NULL);
Please add braces.
- QLIST_INIT(envlist-el_entries);
- envlist-el_count = 0;
+QLIST_INIT(envlist-el_entries);
+envlist-el_count = 0;
- return (envlist);
+return (envlist);
}
/*
@@ -44,18 +44,18 @@ envlist_create(void)
void
envlist_free(envlist_t *envlist)
{
- struct envlist_entry *entry;
+struct envlist_entry *entry;
- assert(envlist != NULL);
+assert(envlist != NULL);
- while (envlist-el_entries.lh_first != NULL) {
- entry = envlist-el_entries.lh_first;
- QLIST_REMOVE(entry, ev_link);
+while (envlist-el_entries.lh_first != NULL) {
+entry = envlist-el_entries.lh_first;
+QLIST_REMOVE(entry, ev_link);
- free((char *)entry-ev_var);
- free(entry);
- }
- free(envlist);
+free((char *)entry-ev_var);
+free(entry);
+}
+free(envlist);
}
/*
@@ -72,7 +72,7 @@ envlist_free(envlist_t *envlist)
int
envlist_parse_set(envlist_t *envlist, const char *env)
{
- return (envlist_parse(envlist, env, envlist_setenv));
+return (envlist_parse(envlist, env, envlist_setenv));
}
/*
@@ -84,7 +84,7 @@ envlist_parse_set(envlist_t *envlist, const char *env)
int
envlist_parse_unset(envlist_t *envlist, const char *env)
{
- return (envlist_parse(envlist, env, envlist_unsetenv));
+return (envlist_parse(envlist, env, envlist_unsetenv));
}
/*
@@ -97,32 +97,32 @@ static int
envlist_parse(envlist_t *envlist, const char *env,
int (*callback)(envlist_t *, const char *))
{
- char *tmpenv, *envvar;
- char *envsave = NULL;
-
- assert(callback != NULL);
-
- if ((envlist == NULL) || (env == NULL))
- return (EINVAL);
-
- /*
- * We need to make temporary copy of the env string
- * as strtok_r(3) modifies it while it tokenizes.
- */
- if ((tmpenv = strdup(env)) == NULL)
- return (errno);
-
- envvar = strtok_r(tmpenv, ,, envsave);
- while (envvar != NULL) {
- if ((*callback)(envlist, envvar) != 0) {
- free(tmpenv);
- return (errno);
- }
- envvar = strtok_r(NULL, ,, envsave);
- }
-
- free(tmpenv);
- return (0);
+char *tmpenv, *envvar;
+char *envsave = NULL;
+
+assert(callback != NULL);
+
+if ((envlist == NULL) || (env == NULL))
+return (EINVAL);
Braces
+
+/*
+ * We need to make temporary copy of the env string
+ * as strtok_r(3) modifies it while it tokenizes.
+ */
+if ((tmpenv = strdup(env)) == NULL)
+return (errno);
Braces
+
+envvar = strtok_r(tmpenv, ,, envsave);
+while (envvar != NULL) {
+if ((*callback)(envlist, envvar) != 0) {
+free(tmpenv);
+return (errno);
+}
+envvar = strtok_r(NULL, ,, envsave);
+}
+
+free(tmpenv);
+return (0);
}
/*
@@ -134,46 +134,46 @@ envlist_parse(envlist_t *envlist, const char *env,
int
envlist_setenv(envlist_t *envlist, const char *env)
{
- struct envlist_entry *entry = NULL;
- const char *eq_sign;
- size_t envname_len;
-
- if ((envlist == NULL) || (env == NULL))
- return (EINVAL);
-
- /* find out first equals sign in given env */
- if ((eq_sign = strchr(env, '=')) == NULL)
- return (EINVAL);
- envname_len = eq_sign - env + 1;
-
- /*
- * If there already exists variable with given name
- * we remove and release it before allocating a whole
-
Re: [Qemu-devel] [PATCHv3 2/2] envlist.c: handle strdup failure
Am 22.05.2012 12:16, schrieb Jim Meyering:
From: Jim Meyering meyer...@redhat.com
Without this, envlist_to_environ may silently fail to copy all
strings into the destination buffer, and both callers would leak
any env strings allocated after a failing strdup, because the
freeing code stops at the first NULL pointer.
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/envlist.c b/envlist.c
index e44889b..df5c723 100644
--- a/envlist.c
+++ b/envlist.c
@@ -234,8 +234,16 @@ envlist_to_environ(const envlist_t *envlist, size_t
*count)
return (NULL);
for (entry = envlist-el_entries.lh_first; entry != NULL;
-entry = entry-ev_link.le_next) {
-*(penv++) = strdup(entry-ev_var);
+ entry = entry-ev_link.le_next, penv++) {
Scratch my comment on 1/2, there's an added penv++ that I overlooked.
Not changing the indentation twice would still be nice.
+*penv = strdup(entry-ev_var);
+if (*penv == NULL) {
+char **e = env;
+while (e = penv) {
+free(*e++);
+}
+free(env);
+return NULL;
+}
}
*penv = NULL; /* NULL terminate the list */
This leak fix looks good then.
For anyone wondering like me, the env here is not the usual
CPUArchState *env but a local char **env.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCHv2 2/2] envlist.c: handle strdup failure
Am 17.08.2012 17:34, schrieb Jim Meyering:
Andreas Färber wrote:
Am 17.08.2012 15:35, schrieb Jim Meyering:
Jim Meyering wrote:
From: Jim Meyering meyer...@redhat.com
Without this, envlist_to_environ may silently fail to copy all
strings into the destination buffer, and both callers would leak
any env strings allocated after a failing strdup, because the
freeing code stops at the first NULL pointer.
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/envlist.c b/envlist.c
index be0addb..7532091 100644
--- a/envlist.c
+++ b/envlist.c
@@ -234,8 +234,16 @@ envlist_to_environ(const envlist_t *envlist, size_t
*count)
return (NULL);
for (entry = envlist-el_entries.lh_first; entry != NULL;
-entry = entry-ev_link.le_next) {
-*(penv++) = strdup(entry-ev_var);
+ entry = entry-ev_link.le_next, penv++) {
+*penv = strdup(entry-ev_var);
+if (*penv == NULL) {
+char **e = env;
+while (e = penv) {
+free(*e++);
+}
+free(env);
+return NULL;
+}
}
*penv = NULL; /* NULL terminate the list */
It seems this has been lost in this list's high volume of patches.
Anyone interested? Repost desired?
You announced a v3 but replied to v2?
Here are links to v3:
[PATCHv3 1/2] envlist.c: convert each TAB(width-4) to equivalent spaces
https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02967.html
[Qemu-devel] [PATCHv3 2/2] envlist.c: handle strdup failure
https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02966.html
I commented on v3. For practical reasons please ping the latest patches
and prefer Patchwork links so that no address@hidden gets committed by
accident.
Thanks for your efforts in reviewing and fixing all these issues!
Andreas
Indentation looks odd in 2/2, too.
That was fixed in v3.
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RESEND][PATCH for-1.2] i82378: Remove bogus MMIO coalescing
Am 17.08.2012 12:56, schrieb Jan Kiszka: This MMIO area is an entry gate to legacy PC ISA devices, addressed via PIO over there. Quite a few of the PIO ports have side effects on access like starting/stopping timers that must be executed properly ordered /wrt the CPU. So we have to remove the coalescing mark. Acked-by: Hervé Poussineau hpous...@reactos.org (I would expect this to go under the SoB, documenting the chronological order...) Signed-off-by: Jan Kiszka jan.kis...@siemens.com I had consented to this fix and expected it to go in alongside the series this came in (kvm/uq-master?). Anthony, do you want a prep PULL for this now? Otherwise explicitly: Acked-by: Andreas Färber andreas.faer...@web.de Regards, Andreas --- hw/i82378.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/hw/i82378.c b/hw/i82378.c index 9b11d90..2123c14 100644 --- a/hw/i82378.c +++ b/hw/i82378.c @@ -225,7 +225,6 @@ static int pci_i82378_init(PCIDevice *dev) pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, s-io); memory_region_init_io(s-mem, i82378_mem_ops, s, i82378-mem, 0x0100); -memory_region_set_coalescing(s-mem); pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, s-mem); /* Make I/O address read only */
Re: [Qemu-devel] [PATCH 1/3] usb: Halt ep queue en cancel pending packets on a packet error
Not being too familiar with the USB code I wonder if $subject was supposed to say and cancel? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] qemu-ga : Guest Agent : Windows 2008 : Unknown command guest-fsfreeze-freeze
Guest-Agent : Windows 2008 Error : Relase 1.1.90
error : internal error unable to execute QEMU command 'guest-fsfreeze-freeze':
this feature or command is not currently supported.
Guest-info shows the command is available. Is there any information available
on the list of commands supported inside Windows ? Appreciate if you have any
pointers.
/Suresh Mandava
-- data
Environment : Windows Server 2008 R2
Guest Agent Version : 1.1.90 compiled from latest qemu source 1.2.0rc0
Libvirt : XML for VM.
channel type='unix'
source mode='bind'
path='/var/lib/libvirt/qemu/va-2008-ga-libvirt.0.sock'/
target type='virtio' name='org.qemu.guest_agent.0'/
address type='virtio-serial' controller='0' bus='0' port='1'/
/channel
channel type='unix'
source mode='bind'
path='/var/lib/libvirt/qemu/va-2008-ga-cloud.0.sock'/
target type='virtio' name='org.qemu.guest_agent.1'/
address type='virtio-serial' controller='0' bus='0' port='2'/
/channel
Test Case Success 1:
echo {'execute':'guest-info'} |socat stdio,ignoreeof
unix-connect:/var/lib/libvirt/qemu/va-2008-ga-cloud.0.sock
Result :
{return: {version: 1.1.90, supported_commands: [{enabled: true,
name: guest-network-get-interfaces}, {enabled: true, name:
guest-suspend-hybrid}, {enabled: true, name: guest-suspend-ram},
{enabled: true, name: guest-suspend-disk}, {enabled: true, name:
guest-fstrim}, {enabled: true, name: guest-fsfreeze-thaw}, {enabled:
true, name: guest-fsfreeze-freeze}, {enabled: true, name:
guest-fsfreeze-status}, {enabled: true, name: guest-file-flush},
{enabled: true, name: guest-file-seek}, {enabled: true, name:
guest-file-write}, {enabled: true, name: guest-file-read}, {enabled:
true, name: guest-file-close}, {enabled: true, name:
guest-file-open}, {enabled: true, name: guest-shutdown}, {enabled:
true, name: guest-info}, {enabled: true, name: guest-ping},
{enabled: true, name: guest-sync}, {enabled: true, name:
guest-sync-delimited}]}}
Test case Failure 2 :
virsh snapshot-create $myvm --no-metadata --disk-only --quiesce --atomic
internal error unable to execute QEMU command 'guest-fsfreeze-freeze': this
feature or command is not currently supported
Please let me know if you need any data.
--
Re: [Qemu-devel] [PATCH 07/21] target-i386: convert cpuid features into properties
On Thu, Aug 16, 2012 at 03:10:50PM -0300, Eduardo Habkost wrote:
On Wed, Aug 15, 2012 at 06:13:27PM +0200, Igor Mammedov wrote:
Signed-off-by: Igor Mammedov imamm...@redhat.com
--
v2:
* replaced mask/ffs tricks by plain 'for (bit = 0; bit 32; bit++)'
as suggested by Eduardo Habkost
---
target-i386/cpu.c | 101
++
1 file changed, 101 insertions(+)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 37ba5ef..440e724 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -606,6 +606,101 @@ static int check_features_against_host(x86_def_t
*guest_def)
return rv;
}
+static bool is_feature_set(const char *name, const uint32_t featbitmap,
+ const char **featureset)
+{
+uint32_t bit;
+
+for (bit = 0; bit 32; ++bit) {
+if (featureset[bit] !altcmp(name, NULL, featureset[bit])) {
+if (featbitmap (1 bit)) {
+return true;
+}
+}
+}
+return false;
+}
+
+static void x86_cpuid_get_feature(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+bool value = true;
+
+if (!is_feature_set(name, env-cpuid_features, feature_name)
+ !is_feature_set(name, env-cpuid_ext_features, ext_feature_name)
+ !is_feature_set(name, env-cpuid_ext2_features, ext2_feature_name)
+ !is_feature_set(name, env-cpuid_ext3_features, ext3_feature_name)
+ !is_feature_set(name, env-cpuid_kvm_features, kvm_feature_name)
+ !is_feature_set(name, env-cpuid_svm_features, svm_feature_name)) {
+value = false;
+}
+
+visit_type_bool(v, value, name, errp);
+}
+
+static void x86_cpuid_set_feature(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = cpu-env;
+uint32_t mask = 0;
+uint32_t *dst_features;
+bool value;
+
+visit_type_bool(v, value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+
+if (lookup_feature(mask, name, NULL, feature_name)) {
+dst_features = env-cpuid_features;
+} else if (lookup_feature(mask, name, NULL, ext_feature_name)) {
+dst_features = env-cpuid_ext_features;
+} else if (lookup_feature(mask, name, NULL, ext2_feature_name)) {
+dst_features = env-cpuid_ext2_features;
+} else if (lookup_feature(mask, name, NULL, ext3_feature_name)) {
+dst_features = env-cpuid_ext3_features;
+} else if (lookup_feature(mask, name, NULL, kvm_feature_name)) {
+dst_features = env-cpuid_kvm_features;
+} else if (lookup_feature(mask, name, NULL, svm_feature_name)) {
+dst_features = env-cpuid_svm_features;
+} else {
+error_set(errp, QERR_PROPERTY_NOT_FOUND, , name);
+return;
+}
Some feature names are duplicated on feature_names and
ext_feature_names. On AMD CPU models, we have to set both, on Intel
models we need to set the bits only on cpuid_features.
Maybe it's better to:
1) eliminate the duplication and set the names only on feature_name
array;
2) At the end of CPU initialization, set the features on
cpuid_ext2_features as copies of the corresponding cpuid_features
bits if CPU vendor == AMD (or, maybe, if some boolean
ext2_features_aliases flag is set, to make it not
vendor-dependent).
I implemented the above (after killing cpudef support), and pushed to:
https://github.com/ehabkost/qemu-hacks/tree/work/unduplicate-feature-names-v0.4-2012-08-17
I will submit it as an RFC soon.
I also made an experimental rebase of your series on top of that branch,
at:
https://github.com/ehabkost/qemu-hacks/tree/work/cpu-properties-igor-rebase-v3.4-2012-08-17
and also rebased my CPUID code work-in-progress, at:
https://github.com/ehabkost/qemu-hacks/tree/work/cpuid-refactor-v0.9-2012-08-15
+
+if (value) {
+*dst_features |= mask;
+} else {
+*dst_features = ~mask;
+}
+}
+
+static void x86_register_cpuid_properties(Object *obj, const char
**featureset)
+{
+uint32_t bit;
+
+for (bit = 0; bit 32; ++bit) {
+if (featureset[bit]) {
+char *feature_name, *save_ptr;
+char buf[32];
+if (strlen(featureset[bit]) sizeof(buf) - 1) {
+abort();
+}
+pstrcpy(buf, sizeof(buf), featureset[bit]);
+feature_name = strtok_r(buf, |, save_ptr);
+while (feature_name) {
+object_property_add(obj, feature_name, bool,
+x86_cpuid_get_feature,
+
Re: [Qemu-devel] Q35 OS install status
On Wed, 15 Aug 2012 17:05:44 +0200 Alexander Graf ag...@suse.de wrote: BSD --- Luiz tested these for me. OpenBSD 5.1, FreeBSD 9.0 and NetBSD all worked with the ide controller passed on the command line. AHCI didn't work. Sorry if I wasn't clear in my report, I've only tried ahci with OpenBSD. Will try it with the others shortly. But yeah, ide controller worked with all of them. Btw, I didn't try any fancy settings, only plain default install. I do remember that with PIIX, one of the BSDs failed, while all the others worked just fine. I don't remember which one. Could have been OpenBSD. Skimming through the source, it looked as if it was searching for an IDE controller and only then activated AHCI on it - which totally contradicts the usual workflow of AHCI on ICH. I was able to run a more complete set of tests today. I've tested all three bsds on q35 and with ide and ahci on master (1.2.0-rc0). Summary: FreeBSD 9.0 installed on the three cases. OpenBSD 5.1 and NetBSD 5.1, installed with ide on master, but failed on q35 and with ahci on master. Details === On all tests the default installation was performed, always selecting a minimum set of packages. Once installation was complete, I rebooted the OS and checked dmesg. master with ide === Command-line example: # ./qemu -hda disks/openbsd-test.img -enable-kvm -m 1G -monitor stdio \ -cdrom /home/lcapitulino/isos/bsd/openbsd-amd64-install51.iso -boot d Results: o FreeBSD 9.0: OK o OpenBSD 5.1: OK o NetBSD 5.1: OK master with ahci Command-line example: # ./qemu -device ahci,id=ahci0 \ -drive file=disks/freebsd-test.img,if=none,format=raw,id=drive-sata0-0-0 \ -device ide-drive,bus=ahci0.0,drive=drive-sata0-0-0,id=sata0-0-0 \ -enable-kvm -m 1G -monitor stdio \ -cdrom /home/lcapitulino/isos/bsd/FreeBSD-9.0-RELEASE-amd64-disc1.iso -boot d Results: o FreeBSD 9.0: OK o OpenBSD 5.1: FAILED Fails while partitioning the disk with the following error: fdisk: sd0: Device not configured And then the installation program keeps asking you to partition the disk again. o NetBSD 5.1: FAILED Fails during boot. A little bit after printing: ahcisata0 port 0: device present, speed: 1.5Gb/s ahcisata0 port 1: device present, speed: 1.5Gb/s The kernel oopses: fatal integer divide fault in supervisor mode q35 === Command-line example: # ./qemu -hda disks/netbsd-q35.img -enable-kvm -m 1G -monitor stdio \ -M pc_q35 -bios /home/lcapitulino/work/src/q35-seabios/out/bios.bin \ -acpitable file=/home/lcapitulino/work/src/q35-seabios/out/q35-acpi-dsdt.aml -cdrom /home/lcapitulino/isos/bsd/netbsd-amd64cd-5.1.iso -boot d Results: o FreeBSD 9.0: OK o OpenBSD 5.1: FAILED (same error as with ahci on master) o NetBSD 5.1: FAILED (same error as with ahci on master)
[Qemu-devel] [RFC 4/6] i386: kvm: use a #define for the set of alias feature bits
Instea of using a hardcoded hex constant, define CPUID_EXT2_AMD_ALIASES as the set of CPUID[8000_0001].EDX bits that on AMD are the same as the bits of CPUID[1].EDX. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/cpu.h | 12 target-i386/kvm.c | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 7b6340c..ba147ac 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -409,6 +409,7 @@ #define CPUID_EXT_HYPERVISOR (1 31) #define CPUID_EXT2_FPU (1 0) +#define CPUID_EXT2_VME (1 1) #define CPUID_EXT2_DE (1 2) #define CPUID_EXT2_PSE (1 3) #define CPUID_EXT2_TSC (1 4) @@ -436,6 +437,17 @@ #define CPUID_EXT2_3DNOWEXT (1 30) #define CPUID_EXT2_3DNOW (1 31) +/* CPUID[8000_0001].EDX bits that are aliase of CPUID[1].EDX bits on AMD CPUs */ +#define CPUID_EXT2_AMD_ALIASES (CPUID_EXT2_FPU | CPUID_EXT2_VME | \ +CPUID_EXT2_DE | CPUID_EXT2_PSE | \ +CPUID_EXT2_TSC | CPUID_EXT2_MSR | \ +CPUID_EXT2_PAE | CPUID_EXT2_MCE | \ +CPUID_EXT2_CX8 | CPUID_EXT2_APIC | \ +CPUID_EXT2_MTRR | CPUID_EXT2_PGE | \ +CPUID_EXT2_MCA | CPUID_EXT2_CMOV | \ +CPUID_EXT2_PAT | CPUID_EXT2_PSE36 | \ +CPUID_EXT2_MMX | CPUID_EXT2_FXSR) + #define CPUID_EXT3_LAHF_LM (1 0) #define CPUID_EXT3_CMP_LEG (1 1) #define CPUID_EXT3_SVM (1 2) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index bab1ef8..63dac56 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -163,7 +163,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, * so add missing bits according to the AMD spec: */ cpuid_1_edx = kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX); -ret |= cpuid_1_edx 0x183f3ff; +ret |= cpuid_1_edx CPUID_EXT2_AMD_ALIASES; break; } break; -- 1.7.11.2
[Qemu-devel] [RFC 2/6] i386: kill cpudef config section support
It's nice to have a flexible system to maintain CPU models as data, but
this is holding us from making improvements in the CPU code because it's
not using the common infra-structure, and because the machine-type data
is still inside C code.
Users who want to configure CPU features directly may simply use the
-cpu command-line option (and maybe an equivalent -device option in
the future) to set CPU features.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 101 ++
1 file changed, 2 insertions(+), 99 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 519a104..71c2ee7 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -237,7 +237,6 @@ typedef struct x86_def_t {
uint32_t xlevel;
char model_id[48];
int vendor_override;
-uint32_t flags;
/* Store the results of Centaur's CPUID instructions */
uint32_t ext4_features;
uint32_t xlevel2;
@@ -1286,7 +1285,7 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
char buf[256];
for (def = x86_defs; def; def = def-next) {
-snprintf(buf, sizeof (buf), def-flags ? [%s]: %s, def-name);
+snprintf(buf, sizeof(buf), %s, def-name);
(*cpu_fprintf)(f, x86 %16s %-48s\n, buf, def-model_id);
}
if (kvm_enabled()) {
@@ -1380,16 +1379,6 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
}
#if !defined(CONFIG_USER_ONLY)
-/* copy vendor id string to 32 bit register, nul pad as needed
- */
-static void cpyid(const char *s, uint32_t *id)
-{
-char *d = (char *)id;
-char i;
-
-for (i = sizeof (*id); i--; )
-*d++ = *s ? *s++ : '\0';
-}
/* interpret radix and convert from string to arbitrary scalar,
* otherwise flag failure
@@ -1403,87 +1392,6 @@ static void cpyid(const char *s, uint32_t *id)
*str !*pend ? (*pval = ul) : (*perr = 1);\
}
-/* map cpuid options to feature bits, otherwise return failure
- * (option tags in *str are delimited by whitespace)
- */
-static void setfeatures(uint32_t *pval, const char *str,
-const char **featureset, int *perr)
-{
-const char *p, *q;
-
-for (q = p = str; *p || *q; q = p) {
-while (iswhite(*p))
-q = ++p;
-while (*p !iswhite(*p))
-++p;
-if (!*q !*p)
-return;
-if (!lookup_feature(pval, q, p, featureset)) {
-fprintf(stderr, error: feature \%.*s\ not available in set\n,
-(int)(p - q), q);
-*perr = 1;
-return;
-}
-}
-}
-
-/* map config file options to x86_def_t form
- */
-static int cpudef_setfield(const char *name, const char *str, void *opaque)
-{
-x86_def_t *def = opaque;
-int err = 0;
-
-if (!strcmp(name, name)) {
-g_free((void *)def-name);
-def-name = g_strdup(str);
-} else if (!strcmp(name, model_id)) {
-strncpy(def-model_id, str, sizeof (def-model_id));
-} else if (!strcmp(name, level)) {
-setscalar(def-level, str, err)
-} else if (!strcmp(name, vendor)) {
-cpyid(str[0], def-vendor1);
-cpyid(str[4], def-vendor2);
-cpyid(str[8], def-vendor3);
-} else if (!strcmp(name, family)) {
-setscalar(def-family, str, err)
-} else if (!strcmp(name, model)) {
-setscalar(def-model, str, err)
-} else if (!strcmp(name, stepping)) {
-setscalar(def-stepping, str, err)
-} else if (!strcmp(name, feature_edx)) {
-setfeatures(def-features, str, feature_name, err);
-} else if (!strcmp(name, feature_ecx)) {
-setfeatures(def-ext_features, str, ext_feature_name, err);
-} else if (!strcmp(name, extfeature_edx)) {
-setfeatures(def-ext2_features, str, ext2_feature_name, err);
-} else if (!strcmp(name, extfeature_ecx)) {
-setfeatures(def-ext3_features, str, ext3_feature_name, err);
-} else if (!strcmp(name, xlevel)) {
-setscalar(def-xlevel, str, err)
-} else {
-fprintf(stderr, error: unknown option [%s = %s]\n, name, str);
-return (1);
-}
-if (err) {
-fprintf(stderr, error: bad option value [%s = %s]\n, name, str);
-return (1);
-}
-return (0);
-}
-
-/* register config file entry as x86_def_t
- */
-static int cpudef_register(QemuOpts *opts, void *opaque)
-{
-x86_def_t *def = g_malloc0(sizeof (x86_def_t));
-
-qemu_opt_foreach(opts, cpudef_setfield, def, 1);
-def-next = x86_defs;
-x86_defs = def;
-return (0);
-}
-
void cpu_clear_apic_feature(CPUX86State *env)
{
env-cpuid_features = ~CPUID_APIC;
@@ -1491,8 +1399,7 @@ void cpu_clear_apic_feature(CPUX86State *env)
#endif /* !CONFIG_USER_ONLY */
-/* register cpudef models defined in configuration file. Here we first
- * preload any built-in definitions
+/* Initialize list of CPU models, filling some non-static fields if necessary
*/
void x86_cpudef_setup(void)
{
@@ -1502,7 +1409,6 @@ void
[Qemu-devel] [RFC 0/6] i386: CPU: remove duplicate feature names
The problem: - Some features are report at the same time on both CPUID[1].EDX and CPUID[8000_0001].EDX on AMD CPUs (e.g. fpu, tsc, msr, pae, mmx). - -cpu model,+feature should enable the bit only on CPUID[1] if it's not an AMD CPU, but it should enable the bit on both CPUID[1] and CPUID[8000_0001] if it's an AMD CPU. - The same should happen when implementing CPU properties: setting the property that enables a feature should set the duplicate CPUID[8000_0001].EDX bit only if CPU vendor is AMD. Reference: http://article.gmane.org/gmane.comp.emulators.qemu/166024 The solution implemented by this series is: - On the CPU model table and while parsing CPU options/properties, set the bit only on CPUID[1] (the x86_def_t.features field). - When finishing initialization of the CPU cpuid fields, duplicate those feature bits on cpuid_ext2_features if and only if the CPU vendor is AMD. This series also removes the cpudef config support, to make this work easier (because the cpudef interface is based on low-level CPUID leaf+register specification, instead of a set of higher-level per-feature object properties). Eduardo Habkost (6): x86_cpudef_setup: coding style change i386: kill cpudef config section support i386: kvm: bit 10 of CPUID[8000_0001].EDX is reserved i386: kvm: use a #define for the set of alias feature bits i386: cpu: eliminate duplicate feature names i386: -cpu help: remove reference to specific CPUID leaves/registers target-i386/cpu.c | 153 +- target-i386/cpu.h | 12 + target-i386/kvm.c | 2 +- 3 files changed, 50 insertions(+), 117 deletions(-) -- 1.7.11.2
[Qemu-devel] [RFC 3/6] i386: kvm: bit 10 of CPUID[8000_0001].EDX is reserved
Bit 10 of CPUID[8000_0001].EDX is not defined as an alias of CPUID[1].EDX[10], so do not duplicate it on kvm_arch_get_supported_cpuid(). Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 696b14a..bab1ef8 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -163,7 +163,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, * so add missing bits according to the AMD spec: */ cpuid_1_edx = kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX); -ret |= cpuid_1_edx 0x183f7ff; +ret |= cpuid_1_edx 0x183f3ff; break; } break; -- 1.7.11.2
[Qemu-devel] [RFC 6/6] i386: -cpu help: remove reference to specific CPUID leaves/registers
The -cpu configuration interface is based on a list of feature names or properties, on a single namespace, so there's no need to mention on which CPUID leaf/register each flag is located. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/cpu.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index a71c2fc..0a03c80 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1297,13 +1297,13 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf) } (*cpu_fprintf)(f, \nRecognized CPUID flags:\n); listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1); -(*cpu_fprintf)(f, f_edx: %s\n, buf); +(*cpu_fprintf)(f, %s\n, buf); listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1); -(*cpu_fprintf)(f, f_ecx: %s\n, buf); +(*cpu_fprintf)(f, %s\n, buf); listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1); -(*cpu_fprintf)(f, extf_edx: %s\n, buf); +(*cpu_fprintf)(f, %s\n, buf); listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1); -(*cpu_fprintf)(f, extf_ecx: %s\n, buf); +(*cpu_fprintf)(f, %s\n, buf); } CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp) -- 1.7.11.2
[Qemu-devel] [RFC 1/6] x86_cpudef_setup: coding style change
Make source code lines shorter.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 96ec9e6..519a104 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1500,20 +1500,23 @@ void x86_cpudef_setup(void)
static const char *model_with_versions[] = { qemu32, qemu64, athlon
};
for (i = 0; i ARRAY_SIZE(builtin_x86_defs); ++i) {
-builtin_x86_defs[i].next = x86_defs;
-builtin_x86_defs[i].flags = 1;
+x86_def_t *def = builtin_x86_defs[i];
+def-next = x86_defs;
+def-flags = 1;
/* Look for specific cpudef models that */
/* have the QEMU version in .model_id */
for (j = 0; j ARRAY_SIZE(model_with_versions); j++) {
-if (strcmp(model_with_versions[j], builtin_x86_defs[i].name) == 0)
{
-pstrcpy(builtin_x86_defs[i].model_id,
sizeof(builtin_x86_defs[i].model_id), QEMU Virtual CPU version );
-pstrcat(builtin_x86_defs[i].model_id,
sizeof(builtin_x86_defs[i].model_id), qemu_get_version());
+if (strcmp(model_with_versions[j], def-name) == 0) {
+pstrcpy(def-model_id, sizeof(def-model_id),
+QEMU Virtual CPU version );
+pstrcat(def-model_id, sizeof(def-model_id),
+qemu_get_version());
break;
}
}
-x86_defs = builtin_x86_defs[i];
+x86_defs = def;
}
#if !defined(CONFIG_USER_ONLY)
qemu_opts_foreach(qemu_find_opts(cpudef), cpudef_register, NULL, 0);
--
1.7.11.2
[Qemu-devel] [RFC 5/6] i386: cpu: eliminate duplicate feature names
Instead of having duplicate feature names on the ext2_feature array for
the AMD feature bit aliases, we keep the feature names only on the
feature_name[] array, and copy the corresponding bits to
cpuid_ext2_features in case the CPU vendor is AMD.
This will:
- Make sure we don't set the feature bit aliases on Intel CPUs;
- Make it easier to convert feature bits to CPU properties, as now we
have a single bit on the x86_def_t struct for each CPU feature.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 31 +++
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 71c2ee7..a71c2fc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -58,15 +58,19 @@ static const char *ext_feature_name[] = {
tsc-deadline, aes, xsave, osxsave,
avx, NULL, NULL, hypervisor,
};
+/* Feature names that are already defined on feature_name[] but are set on
+ * CPUID[8000_0001].EDX on AMD CPUs don't have their names on
+ * ext2_feature_name[]. They are copied automatically to cpuid_ext2_features
+ * if and only if CPU vendor is AMD.
+ */
static const char *ext2_feature_name[] = {
-fpu, vme, de, pse,
-tsc, msr, pae, mce,
-cx8 /* AMD CMPXCHG8B */, apic, NULL, syscall,
-mtrr, pge, mca, cmov,
-pat, pse36, NULL, NULL /* Linux mp */,
-nx|xd, NULL, mmxext, mmx,
-fxsr, fxsr_opt|ffxsr, pdpe1gb /* AMD Page1GB */, rdtscp,
-NULL, lm|i64, 3dnowext, 3dnow,
+NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
+NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
+NULL /* cx8 */ /* AMD CMPXCHG8B */, NULL /* apic */, NULL, syscall,
+NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
+NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
+nx|xd, NULL, mmxext, NULL /* mmx */,
+NULL /* fxsr */, fxsr_opt|ffxsr, pdpe1gb /* AMD Page1GB */, rdtscp,
};
static const char *ext3_feature_name[] = {
lahf_lm /* AMD LahfSahf */, cmp_legacy, svm, extapic /* AMD
ExtApicSpace */,
@@ -1359,6 +1363,17 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
env-cpuid_xlevel2 = def-xlevel2;
object_property_set_int(OBJECT(cpu), (int64_t)def-tsc_khz * 1000,
tsc-frequency, error);
+
+/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
+ * CPUID[1].EDX.
+ */
+if (env-cpuid_vendor1 == CPUID_VENDOR_AMD_1
+env-cpuid_vendor2 == CPUID_VENDOR_AMD_2
+env-cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
+env-cpuid_ext2_features = ~CPUID_EXT2_AMD_ALIASES;
+env-cpuid_ext2_features |= (def-features CPUID_EXT2_AMD_ALIASES);
+}
+
if (!kvm_enabled()) {
env-cpuid_features = TCG_FEATURES;
env-cpuid_ext_features = TCG_EXT_FEATURES;
--
1.7.11.2
[Qemu-devel] [PATCHv4 0/2] envlist.c: handle strdup failure
From: Jim Meyering meyer...@redhat.com Differences from v3 (no semantic change): - change 1/2 so this file conforms more closely to QEMU's coding style, by adding braces around each one-line if body (there was no one-line else- or while-block). - move an indentation correction from 2/2 into 1/2 Jim Meyering (2): envlist.c: conform to QEMU's coding style envlist.c: handle strdup failure envlist.c | 284 +- 1 file changed, 152 insertions(+), 132 deletions(-) -- 1.7.12.rc2
[Qemu-devel] [PATCHv4 1/2] envlist.c: conform to QEMU's coding style
From: Jim Meyering meyer...@redhat.com
Convert each TAB(width-4) to equivalent spaces.
Put braces around each one-line if-body.
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 268 --
1 file changed, 140 insertions(+), 128 deletions(-)
diff --git a/envlist.c b/envlist.c
index f2303cd..230596f 100644
--- a/envlist.c
+++ b/envlist.c
@@ -8,13 +8,13 @@
#include envlist.h
struct envlist_entry {
- const char *ev_var; /* actual env value */
- QLIST_ENTRY(envlist_entry) ev_link;
+const char *ev_var; /* actual env value */
+QLIST_ENTRY(envlist_entry) ev_link;
};
struct envlist {
- QLIST_HEAD(, envlist_entry) el_entries; /* actual entries */
- size_t el_count;/* number of entries */
+QLIST_HEAD(, envlist_entry) el_entries; /* actual entries */
+size_t el_count;/* number of entries */
};
static int envlist_parse(envlist_t *envlist,
@@ -27,15 +27,16 @@ static int envlist_parse(envlist_t *envlist,
envlist_t *
envlist_create(void)
{
- envlist_t *envlist;
+envlist_t *envlist;
- if ((envlist = malloc(sizeof (*envlist))) == NULL)
- return (NULL);
+if ((envlist = malloc(sizeof(*envlist))) == NULL) {
+return NULL;
+}
- QLIST_INIT(envlist-el_entries);
- envlist-el_count = 0;
+QLIST_INIT(envlist-el_entries);
+envlist-el_count = 0;
- return (envlist);
+return envlist;
}
/*
@@ -44,18 +45,18 @@ envlist_create(void)
void
envlist_free(envlist_t *envlist)
{
- struct envlist_entry *entry;
+struct envlist_entry *entry;
- assert(envlist != NULL);
+assert(envlist != NULL);
- while (envlist-el_entries.lh_first != NULL) {
- entry = envlist-el_entries.lh_first;
- QLIST_REMOVE(entry, ev_link);
+while (envlist-el_entries.lh_first != NULL) {
+entry = envlist-el_entries.lh_first;
+QLIST_REMOVE(entry, ev_link);
- free((char *)entry-ev_var);
- free(entry);
- }
- free(envlist);
+free((char *)entry-ev_var);
+free(entry);
+}
+free(envlist);
}
/*
@@ -72,7 +73,7 @@ envlist_free(envlist_t *envlist)
int
envlist_parse_set(envlist_t *envlist, const char *env)
{
- return (envlist_parse(envlist, env, envlist_setenv));
+return envlist_parse(envlist, env, envlist_setenv);
}
/*
@@ -84,7 +85,7 @@ envlist_parse_set(envlist_t *envlist, const char *env)
int
envlist_parse_unset(envlist_t *envlist, const char *env)
{
- return (envlist_parse(envlist, env, envlist_unsetenv));
+return envlist_parse(envlist, env, envlist_unsetenv);
}
/*
@@ -97,32 +98,34 @@ static int
envlist_parse(envlist_t *envlist, const char *env,
int (*callback)(envlist_t *, const char *))
{
- char *tmpenv, *envvar;
- char *envsave = NULL;
-
- assert(callback != NULL);
-
- if ((envlist == NULL) || (env == NULL))
- return (EINVAL);
-
- /*
-* We need to make temporary copy of the env string
-* as strtok_r(3) modifies it while it tokenizes.
-*/
- if ((tmpenv = strdup(env)) == NULL)
- return (errno);
-
- envvar = strtok_r(tmpenv, ,, envsave);
- while (envvar != NULL) {
- if ((*callback)(envlist, envvar) != 0) {
- free(tmpenv);
- return (errno);
- }
- envvar = strtok_r(NULL, ,, envsave);
- }
-
- free(tmpenv);
- return (0);
+char *tmpenv, *envvar;
+char *envsave = NULL;
+
+assert(callback != NULL);
+
+if ((envlist == NULL) || (env == NULL)) {
+return EINVAL;
+}
+
+/*
+ * We need to make temporary copy of the env string
+ * as strtok_r(3) modifies it while it tokenizes.
+ */
+if ((tmpenv = strdup(env)) == NULL) {
+return errno;
+}
+
+envvar = strtok_r(tmpenv, ,, envsave);
+while (envvar != NULL) {
+if ((*callback)(envlist, envvar) != 0) {
+free(tmpenv);
+return errno;
+}
+envvar = strtok_r(NULL, ,, envsave);
+}
+
+free(tmpenv);
+return 0;
}
/*
@@ -134,46 +137,50 @@ envlist_parse(envlist_t *envlist, const char *env,
int
envlist_setenv(envlist_t *envlist, const char *env)
{
- struct envlist_entry *entry = NULL;
- const char *eq_sign;
- size_t envname_len;
-
- if ((envlist == NULL) || (env == NULL))
- return (EINVAL);
-
- /* find out first equals sign in given env */
- if ((eq_sign = strchr(env, '=')) == NULL)
- return (EINVAL);
- envname_len = eq_sign - env + 1;
-
- /*
-* If there already exists variable with given name
-* we remove and release it before allocating a whole
-* new entry.
-*/
- for (entry =
[Qemu-devel] [PATCHv4 2/2] envlist.c: handle strdup failure
From: Jim Meyering meyer...@redhat.com
Without this, envlist_to_environ may silently fail to copy all
strings into the destination buffer, and both callers would leak
any env strings allocated after a failing strdup, because the
freeing code stops at the first NULL pointer.
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/envlist.c b/envlist.c
index 230596f..cf3f2d8 100644
--- a/envlist.c
+++ b/envlist.c
@@ -245,8 +245,16 @@ envlist_to_environ(const envlist_t *envlist, size_t *count)
}
for (entry = envlist-el_entries.lh_first; entry != NULL;
- entry = entry-ev_link.le_next) {
-*(penv++) = strdup(entry-ev_var);
+ entry = entry-ev_link.le_next, penv++) {
+*penv = strdup(entry-ev_var);
+if (*penv == NULL) {
+char **e = env;
+while (e = penv) {
+free(*e++);
+}
+free(env);
+return NULL;
+}
}
*penv = NULL; /* NULL terminate the list */
--
1.7.12.rc2
Re: [Qemu-devel] [PATCHv3 2/2] envlist.c: handle strdup failure
Andreas Färber wrote:
Am 22.05.2012 12:16, schrieb Jim Meyering:
From: Jim Meyering meyer...@redhat.com
Without this, envlist_to_environ may silently fail to copy all
strings into the destination buffer, and both callers would leak
any env strings allocated after a failing strdup, because the
freeing code stops at the first NULL pointer.
Signed-off-by: Jim Meyering meyer...@redhat.com
---
envlist.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/envlist.c b/envlist.c
index e44889b..df5c723 100644
--- a/envlist.c
+++ b/envlist.c
@@ -234,8 +234,16 @@ envlist_to_environ(const envlist_t *envlist, size_t
*count)
return (NULL);
for (entry = envlist-el_entries.lh_first; entry != NULL;
-entry = entry-ev_link.le_next) {
-*(penv++) = strdup(entry-ev_var);
+ entry = entry-ev_link.le_next, penv++) {
Scratch my comment on 1/2, there's an added penv++ that I overlooked.
Not changing the indentation twice would still be nice.
I posted a V4 that does that, and removes parens around return arguments
as well. However, I did not remove assignments from conditionals. IMHO,
that would require an inordinate amount of change for the marginal gain
of making legacy code conform.
