Re: [Qemu-devel] [PATCH] kvmvapic: Fix TB invalidation after instruction patching
Jan Kiszka a écrit : > From: Jan Kiszka > > Since 0b57e287, cpu_memory_rw_debug already triggers a TB invalidation. > As it doesn't (and cannot) set is_cpu_write_access=1 but "consumes" the > currently executed TB, the tb_invalidate_phys_page_range call from > patch_instruction didn't work anymore. > > Fix this by open-coding the required bits to restore the CPU state from > the current TB position before patching and resume execution on the > patched instruction afterward. > > Signed-off-by: Jan Kiszka > --- > Tested-by: Hervé Poussineau However, I had to initialize current_pc, current_cs_base and current_flags to 0 to prevent uninitialized warning. (GCC 4.7.1, KVM disabled by configure) Regards, Hervé
[Qemu-devel] [PATCH v2] kvmvapic: Fix TB invalidation after instruction patching
From: Jan Kiszka
Since 0b57e287, cpu_memory_rw_debug already triggers a TB invalidation.
As it doesn't (and cannot) set is_cpu_write_access=1 but "consumes" the
currently executed TB, the tb_invalidate_phys_page_range call from
patch_instruction didn't work anymore.
Fix this by open-coding the required bits to restore the CPU state from
the current TB position before patching and resume execution on the
patched instruction afterward.
Signed-off-by: Jan Kiszka
---
Changes in v2:
- make the compiler happier by initializing local variables to 0
hw/kvmvapic.c | 20
1 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/hw/kvmvapic.c b/hw/kvmvapic.c
index dc111ee..09c6433 100644
--- a/hw/kvmvapic.c
+++ b/hw/kvmvapic.c
@@ -384,10 +384,13 @@ static void patch_call(VAPICROMState *s, CPUX86State
*env, target_ulong ip,
static void patch_instruction(VAPICROMState *s, CPUX86State *env, target_ulong
ip)
{
-hwaddr paddr;
VAPICHandlers *handlers;
uint8_t opcode[2];
uint32_t imm32;
+TranslationBlock *current_tb;
+target_ulong current_pc = 0;
+target_ulong current_cs_base = 0;
+int current_flags = 0;
if (smp_cpus == 1) {
handlers = &s->rom_state.up;
@@ -395,6 +398,13 @@ static void patch_instruction(VAPICROMState *s,
CPUX86State *env, target_ulong i
handlers = &s->rom_state.mp;
}
+if (!kvm_enabled()) {
+current_tb = tb_find_pc(env->mem_io_pc);
+cpu_restore_state(current_tb, env, env->mem_io_pc);
+cpu_get_tb_cpu_state(env, ¤t_pc, ¤t_cs_base,
+ ¤t_flags);
+}
+
pause_all_vcpus();
cpu_memory_rw_debug(env, ip, opcode, sizeof(opcode), 0);
@@ -430,9 +440,11 @@ static void patch_instruction(VAPICROMState *s,
CPUX86State *env, target_ulong i
resume_all_vcpus();
-paddr = cpu_get_phys_page_debug(env, ip);
-paddr += ip & ~TARGET_PAGE_MASK;
-tb_invalidate_phys_page_range(paddr, paddr + 1, 1);
+if (!kvm_enabled()) {
+env->current_tb = NULL;
+tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+cpu_resume_from_signal(env, NULL);
+}
}
void vapic_report_tpr_access(DeviceState *dev, void *cpu, target_ulong ip,
--
1.7.3.4
[Qemu-devel] [PATCH] memory: Reintroduce dirty flag to optimize changes on disabled regions
From: Jan Kiszka
Cirrus is triggering this, e.g. during Win2k boot: Changes only on
disabled regions require no topology update when transaction depth drops
to 0 again.
Signed-off-by: Jan Kiszka
---
memory.c | 17 +++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/memory.c b/memory.c
index d5150f8..122a58e 100644
--- a/memory.c
+++ b/memory.c
@@ -22,7 +22,8 @@
#include "memory-internal.h"
-unsigned memory_region_transaction_depth = 0;
+static unsigned memory_region_transaction_depth;
+static bool memory_region_update_pending;
static bool global_dirty_log = false;
static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners
@@ -741,7 +742,8 @@ void memory_region_transaction_commit(void)
assert(memory_region_transaction_depth);
--memory_region_transaction_depth;
-if (!memory_region_transaction_depth) {
+if (!memory_region_transaction_depth && memory_region_update_pending) {
+memory_region_update_pending = false;
MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
@@ -1060,6 +1062,7 @@ void memory_region_set_log(MemoryRegion *mr, bool log,
unsigned client)
memory_region_transaction_begin();
mr->dirty_log_mask = (mr->dirty_log_mask & ~mask) | (log * mask);
+memory_region_update_pending |= mr->enabled;
memory_region_transaction_commit();
}
@@ -1097,6 +1100,7 @@ void memory_region_set_readonly(MemoryRegion *mr, bool
readonly)
if (mr->readonly != readonly) {
memory_region_transaction_begin();
mr->readonly = readonly;
+memory_region_update_pending |= mr->enabled;
memory_region_transaction_commit();
}
}
@@ -1106,6 +1110,7 @@ void memory_region_rom_device_set_readable(MemoryRegion
*mr, bool readable)
if (mr->readable != readable) {
memory_region_transaction_begin();
mr->readable = readable;
+memory_region_update_pending |= mr->enabled;
memory_region_transaction_commit();
}
}
@@ -1248,6 +1253,7 @@ void memory_region_add_eventfd(MemoryRegion *mr,
memmove(&mr->ioeventfds[i+1], &mr->ioeventfds[i],
sizeof(*mr->ioeventfds) * (mr->ioeventfd_nb-1 - i));
mr->ioeventfds[i] = mrfd;
+memory_region_update_pending |= mr->enabled;
memory_region_transaction_commit();
}
@@ -1280,6 +1286,7 @@ void memory_region_del_eventfd(MemoryRegion *mr,
--mr->ioeventfd_nb;
mr->ioeventfds = g_realloc(mr->ioeventfds,
sizeof(*mr->ioeventfds)*mr->ioeventfd_nb +
1);
+memory_region_update_pending |= mr->enabled;
memory_region_transaction_commit();
}
@@ -1323,6 +1330,7 @@ static void
memory_region_add_subregion_common(MemoryRegion *mr,
}
QTAILQ_INSERT_TAIL(&mr->subregions, subregion, subregions_link);
done:
+memory_region_update_pending |= mr->enabled && subregion->enabled;
memory_region_transaction_commit();
}
@@ -1353,6 +1361,7 @@ void memory_region_del_subregion(MemoryRegion *mr,
assert(subregion->parent == mr);
subregion->parent = NULL;
QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
+memory_region_update_pending |= mr->enabled && subregion->enabled;
memory_region_transaction_commit();
}
@@ -1363,6 +1372,7 @@ void memory_region_set_enabled(MemoryRegion *mr, bool
enabled)
}
memory_region_transaction_begin();
mr->enabled = enabled;
+memory_region_update_pending = true;
memory_region_transaction_commit();
}
@@ -1397,6 +1407,7 @@ void memory_region_set_alias_offset(MemoryRegion *mr,
hwaddr offset)
memory_region_transaction_begin();
mr->alias_offset = offset;
+memory_region_update_pending |= mr->enabled;
memory_region_transaction_commit();
}
@@ -1543,6 +1554,7 @@ void address_space_init(AddressSpace *as, MemoryRegion
*root)
flatview_init(as->current_map);
QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
as->name = NULL;
+memory_region_update_pending = true;
memory_region_transaction_commit();
address_space_init_dispatch(as);
}
@@ -1552,6 +1564,7 @@ void address_space_destroy(AddressSpace *as)
/* Flush out anything from MemoryListeners listening in on this */
memory_region_transaction_begin();
as->root = NULL;
+memory_region_update_pending = true;
memory_region_transaction_commit();
QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
address_space_destroy_dispatch(as);
--
1.7.3.4
Re: [Qemu-devel] [PATCH 2/4 v5] vmware_vga: Remove duplicated info from local state
On Sun, 4 Nov 2012, Jan Kiszka wrote: I cannot comment on why it was done like this, just that this patch breaks the Linux vmware X driver. Please fix or revert. I'll look into it. If I (or someone else) don't come up with a fix today you can revert. Regards, BALATON Zoltan
[Qemu-devel] XP install cores with SCSI LSI 53C895A disks - follow up
Hello, Clean XP install cores with SCSI LSI 53C89A disk when copying files. Isn't on the same file, so looks like a timing problem. Reproduceable. Driver used is sym_hi. Details are below. See also: https://lists.gnu.org/archive/html/qemu-devel/2012-03/msg00523.html Looks like problem is from Paolo's commit: 2f0772c5b4818d4b2078be9dace0036d1030faee qemu-system-x86_64: hw/lsi53c895a.c:351: lsi_soft_reset: Assertion `((&s->queue)->tqh_first == ((void *)0))' failed. So SCSI queue isn't empty (was an assumption and asserted), so qdev_reset_all(&s->dev.qdev); might not work or some other timing related issues. Any ideas to solve? Reproduceable? I'm using BIOS from SEABIOS. Thank you. Ciao, Gerhard Image created with: qemu-img create -f qcow2 XP-TEST.qcow2 10G qemu-kvm: 4d9367b76f71c6d938cf8201392abe4bfb1136cb /root/download/qemu/git/qemu-kvm/x86_64-softmmu/qemu-system-x86_64 -device nec-usb-xhci,id=usb0 -drive file=VM-XP-TEST/XP-TEST.qcow2,media=disk,if=scsi,bus=0,unit=0 -drive if=ide,index=3,media=cdrom,file=ISO/XP.iso -boot order=dac,menu=on -m 2048 -k de -vga vmware -vnc :0 -bios /root/download/seabios/git/seabios/out/bios.bin -chardev stdio,id=seabios -device isa-debugcon,iobase=0x402,chardev=seabios -device rtl8139,mac=00:02:44:92:87:6a,vlan=0,romfile= -net tap,ifname=tap0,script=no,downscript=no,vlan=0
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Ping? On 01.11.2012 21:06, Gerhard Wiesinger wrote: Fix crash with VNC under NT 4.0 and VMWare VGA and window which is outside of the visible area. Backtrace: #0 set_bit (addr=, nr=-3) at ./bitops.h:122 #1 vnc_dpy_update (ds=, x=-48, y=145, w=57, h=161) at ui/vnc.c:452 #2 0x7f1ce057e2ec in dpy_update (s=0x7f1ce1c8c880, h=16, w=66, y=145, x=-57) at ./console.h:242 #3 vmsvga_update_rect (h=16, w=66, y=145, x=-57, s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:324 #4 vmsvga_update_rect_flush (s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:357 #5 vmsvga_update_display (opaque=0x7f1ce1cb3dd0) at hw/vmware_vga.c:960 #6 0x7f1ce05f0b37 in vnc_refresh (opaque=0x7f1cd8526010) at ui/vnc.c:2590 #7 0x7f1ce05c002b in qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:392 #8 qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:373 #9 0x7f1ce05c028d in qemu_run_all_timers () at qemu-timer.c:449 #10 0x7f1ce058f2ee in main_loop_wait (nonblocking=) at main-loop.c:502 #11 0x7f1ce047acb3 in main_loop () at vl.c:1655 #12 main (argc=, argv=, envp=out>) at vl.c:3826 Signed-off-by: Gerhard Wiesinger --- ui/vnc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/ui/vnc.c b/ui/vnc.c index 7c120e6..ae6d819 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -453,6 +453,11 @@ static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h) w = MIN(x + w, width) - x; h = MIN(h, height); +x = MAX(x, 0); +y = MAX(y, 0); +w = MAX(w, 0); +h = MAX(h, 0); + for (; y < h; y++) for (i = 0; i < w; i += 16) set_bit((x + i) / 16, s->dirty[y]);
[Qemu-devel] [PATCH] block: Workaround for older versions of MinGW gcc
Versions before gcc-4.6 don't support unnamed fields in initializers
(see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676).
Offset and OffsetHigh belong to an unnamed struct which is part of an
unnamed union. Therefore the original code does not work with older
versions of gcc.
Signed-off-by: Stefan Weil
---
This patch is needed for Debian's amd64-mingw32msvc-gcc-4.4.4
which I use for MinGW-w64 cross compilation.
Regards
Stefan W.
block/win32-aio.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/win32-aio.c b/block/win32-aio.c
index c34dc73..92f25a9 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -167,11 +167,11 @@ BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
waiocb->is_linear = true;
}
-waiocb->ov = (OVERLAPPED) {
-.Offset = (DWORD) offset,
-.OffsetHigh = (DWORD) (offset >> 32),
-.hEvent = event_notifier_get_handle(&aio->e)
-};
+memset(&waiocb->ov, 0, sizeof(waiocb->ov));
+waiocb->ov.Offset = (DWORD)offset;
+waiocb->ov.OffsetHigh = (DWORD)(offset >> 32);
+waiocb->ov.hEvent = event_notifier_get_handle(&aio->e);
+
aio->count++;
if (type & QEMU_AIO_READ) {
--
1.7.10.4
Re: [Qemu-devel] [PATCH] Fix out-of-tree and cross compile builds for pixman
Am 03.11.2012 21:15, schrieb Blue Swirl: On Sat, Nov 3, 2012 at 7:02 PM, Peter Maydell wrote: On 3 November 2012 19:47, Blue Swirl wrote: --- a/Makefile +++ b/Makefile @@ -122,7 +122,7 @@ subdir-pixman: pixman/Makefile $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pixman V="$(V)" all,) pixman/Makefile: $(SRC_PATH)/pixman/configure - (cd pixman; $(SRC_PATH)/pixman/configure --disable-shared --enable-static) + (cd pixman; CC=$(CC) LD=$(LD) AR=$(AR) NM=$(NM) RANLIB=$(RANLIB) $(SRC_PATH)/pixman/configure --disable-shared --enable-static) Not tested, but aren't there quoting issues here if you're building with --cc='ccache gcc' ? Yes. Also configure fails because the variables are not expanded and directory pixman/pixman does not exist. Funny how it worked earlier. I struggle with the same issue, and there are more problems caused by the internal pixman code. The dependencies are wrong because pixman is built too late: $(TOOLS) also depends on it. It is not trivial to model them correctly. IMHO it would be better to build the internal pixman immediately when QEMU's configure is called. Then QEMU's make can always rely on an existing pixman. The internal pixman code is also too old for cross compilations with MinGW-w64. It already fails when running configure. Newer versions of pixman compile after a trivial modification which is needed to avoid redefined symbols: diff --git a/pixman/pixman-mmx.c b/pixman/pixman-mmx.c index 1a014fd..723c245 100644 --- a/pixman/pixman-mmx.c +++ b/pixman/pixman-mmx.c @@ -61,7 +61,7 @@ _mm_empty (void) #endif #ifdef USE_X86_MMX -# if (defined(__SUNPRO_C) || defined(_MSC_VER)) +# if (defined(__SUNPRO_C) || defined(_MSC_VER) || defined(__WIN64)) # include # else /* We have to compile with -msse to use xmmintrin.h, but that causes SSE More changes are needed to avoid typical MinGW-w64 compiler warnings (pointer to int conversions without uintptr_t). Up to now, I did not test the resulting code, so maybe there will be more surprises. Regards Stefan
Re: [Qemu-devel] [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical address space
> -Original Message-
> From: Jan Kiszka [mailto:jan.kis...@web.de]
> Sent: Saturday, November 03, 2012 6:55 PM
> To: Hao, Xudong
> Cc: qemu-devel@nongnu.org; a...@redhat.com; k...@vger.kernel.org
> Subject: Re: [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical
> address space
>
> On 2012-11-02 06:38, Xudong Hao wrote:
> > For 64 bit processor, emulate 40 bits physical address if the host physical
> > address space >= 40bits, else guest physical is same as host.
> >
> > Signed-off-by: Xudong Hao
> > ---
> > target-i386/cpu.c |5 -
> > 1 files changed, 4 insertions(+), 1 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 423e009..3a78881 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1584,7 +1584,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
> index, uint32_t count,
> > if (env->cpuid_ext2_features & CPUID_EXT2_LM) {
> > /* 64 bit processor */
> > /* XXX: The physical address space is limited to 42 bits in exec.c. */
> > -*eax = 0x3028; /* 48 bits virtual, 40 bits physical */
> > +/* XXX: 40 bits physical if host physical address space >= 40 bits */
> > +uint32_t a, b, c, d;
> > +host_cpuid(0x8008, 0, &a, &b, &c, &d);
> > +*eax = a < 0x3028 ? a : 0x3028;
>
> This variation will not only affect -cpu host, right? That can create
> problems when migrating between hosts with different address widths, and
> then we will need some control knob to adjust what it reported to the guest.
>
Oh, I did not consider migrating to different platform(addr widths).
But I think the fixed value 40 bits may cause problem: in VT-d case, when a
host support GAW < 40 bits, and qemu emulate 40 bits guest physical address
space, will bring bug on:
drivers/iommu/intel-iommu.c
static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
unsigned long pfn, int target_level)
{
int addr_width = agaw_to_width(domain->agaw) - VTD_PAGE_SHIFT;
...
BUG_ON(!domain->pgd);
BUG_ON(addr_width < BITS_PER_LONG && pfn >> addr_width);
> Jan
>
> > } else {
> > if (env->cpuid_features & CPUID_PSE36)
> > *eax = 0x0024; /* 36 bits physical */
> >
>
Re: [Qemu-devel] [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical address space
On 2012-11-04 13:15, Hao, Xudong wrote:
>> -Original Message-
>> From: Jan Kiszka [mailto:jan.kis...@web.de]
>> Sent: Saturday, November 03, 2012 6:55 PM
>> To: Hao, Xudong
>> Cc: qemu-devel@nongnu.org; a...@redhat.com; k...@vger.kernel.org
>> Subject: Re: [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical
>> address space
>>
>> On 2012-11-02 06:38, Xudong Hao wrote:
>>> For 64 bit processor, emulate 40 bits physical address if the host physical
>>> address space >= 40bits, else guest physical is same as host.
>>>
>>> Signed-off-by: Xudong Hao
>>> ---
>>> target-i386/cpu.c |5 -
>>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>>> index 423e009..3a78881 100644
>>> --- a/target-i386/cpu.c
>>> +++ b/target-i386/cpu.c
>>> @@ -1584,7 +1584,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
>> index, uint32_t count,
>>> if (env->cpuid_ext2_features & CPUID_EXT2_LM) {
>>> /* 64 bit processor */
>>> /* XXX: The physical address space is limited to 42 bits in exec.c. */
>>> -*eax = 0x3028; /* 48 bits virtual, 40 bits physical */
>>> +/* XXX: 40 bits physical if host physical address space >= 40 bits */
>>> +uint32_t a, b, c, d;
>>> +host_cpuid(0x8008, 0, &a, &b, &c, &d);
>>> +*eax = a < 0x3028 ? a : 0x3028;
>>
>> This variation will not only affect -cpu host, right? That can create
>> problems when migrating between hosts with different address widths, and
>> then we will need some control knob to adjust what it reported to the guest.
>>
>
> Oh, I did not consider migrating to different platform(addr widths).
> But I think the fixed value 40 bits may cause problem: in VT-d case, when a
> host support GAW < 40 bits, and qemu emulate 40 bits guest physical address
> space, will bring bug on:
>
> drivers/iommu/intel-iommu.c
> static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
> unsigned long pfn, int target_level)
> {
> int addr_width = agaw_to_width(domain->agaw) - VTD_PAGE_SHIFT;
> ...
> BUG_ON(!domain->pgd);
> BUG_ON(addr_width < BITS_PER_LONG && pfn >> addr_width);
>
Does it mean that buggy or malicious user space can trigger a kernel
bug? Then this must be fixed of course.
Jan
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH] vmware_vga: Add back some info in local state partially reverting aa32b38c
Keep saving display surface parameters at init and using these cached
values instead of getting them when needed. Not sure why this is
needed (maybe due to the interaction with the vga device) but not
doing this broke the Xorg vmware driver at least.
Signed-off-by: BALATON Zoltan
---
hw/vmware_vga.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 7c766fb..834588d 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -39,6 +39,8 @@ struct vmsvga_state_s {
VGACommonState vga;
int invalidated;
+int depth;
+int bypp;
int enable;
int config;
struct {
@@ -55,6 +57,9 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
+uint32_t wred;
+uint32_t wgreen;
+uint32_t wblue;
int syncing;
MemoryRegion fifo_ram;
@@ -718,25 +723,25 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t
address)
return SVGA_MAX_HEIGHT;
case SVGA_REG_DEPTH:
-return ds_get_depth(s->vga.ds);
+return s->depth;
case SVGA_REG_BITS_PER_PIXEL:
-return ds_get_bits_per_pixel(s->vga.ds);
+return (s->depth + 7) & ~7;
case SVGA_REG_PSEUDOCOLOR:
return 0x0;
case SVGA_REG_RED_MASK:
-return ds_get_rmask(s->vga.ds);
+return s->wred;
case SVGA_REG_GREEN_MASK:
-return ds_get_gmask(s->vga.ds);
+return s->wgreen;
case SVGA_REG_BLUE_MASK:
-return ds_get_bmask(s->vga.ds);
+return s->wblue;
case SVGA_REG_BYTES_PER_LINE:
-return ds_get_bytes_per_pixel(s->vga.ds) * s->new_width;
+return s->bypp * s->new_width;
case SVGA_REG_FB_START: {
struct pci_vmsvga_state_s *pci_vmsvga
@@ -801,7 +806,7 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t
address)
return s->cursor.on;
case SVGA_REG_HOST_BITS_PER_PIXEL:
-return ds_get_bits_per_pixel(s->vga.ds);
+return (s->depth + 7) & ~7;
case SVGA_REG_SCRATCH_SIZE:
return s->scratch_size;
@@ -864,7 +869,7 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
break;
case SVGA_REG_BITS_PER_PIXEL:
-if (value != ds_get_bits_per_pixel(s->vga.ds)) {
+if (value != s->depth) {
printf("%s: Bad bits per pixel: %i bits\n", __func__, value);
s->config = 0;
}
@@ -1084,7 +1089,7 @@ static const VMStateDescription
vmstate_vmware_vga_internal = {
.minimum_version_id_old = 0,
.post_load = vmsvga_post_load,
.fields = (VMStateField[]) {
-VMSTATE_UNUSED(4), /* was depth */
+VMSTATE_INT32_EQUAL(depth, struct vmsvga_state_s),
VMSTATE_INT32(enable, struct vmsvga_state_s),
VMSTATE_INT32(config, struct vmsvga_state_s),
VMSTATE_INT32(cursor.id, struct vmsvga_state_s),
@@ -1137,6 +1142,13 @@ static void vmsvga_init(struct vmsvga_state_s *s,
vga_common_init(&s->vga);
vga_init(&s->vga, address_space, io, true);
vmstate_register(NULL, 0, &vmstate_vga_common, &s->vga);
+/* Save some values here in case they are changed later.
+ * This is suspicious and needs more though why it is needed. */
+s->depth = ds_get_bits_per_pixel(s->vga.ds);
+s->bypp = ds_get_bytes_per_pixel(s->vga.ds);
+s->wred = ds_get_rmask(s->vga.ds);
+s->wgreen = ds_get_gmask(s->vga.ds);
+s->wblue = ds_get_bmask(s->vga.ds);
}
static uint64_t vmsvga_io_read(void *opaque, hwaddr addr, unsigned size)
--
1.7.10
Re: [Qemu-devel] [PATCH 2/4 v5] vmware_vga: Remove duplicated info from local state
On Sun, 4 Nov 2012, Jan Kiszka wrote: breaks the Linux vmware X driver. Please fix or revert. Couldn't really figure out what's happening but sent a patch which reverts just these parts which seems to fix it for me. Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH] tests/tcg: new test for i386 FPREM and FPREM1
Ping? Original patch still applies on master. On Mon, Oct 29, 2012 at 4:30 AM, Catalin Patulea wrote: > On Mon, Oct 29, 2012 at 4:04 AM, Peter Maydell > wrote: >> I'm not fantastically enthused about bitfields, but since this is a test >> program and not part of QEMU proper I don't think rewriting to avoid >> them is justified. > I could maybe check some conditions (perhaps sizeof(union float80u) == > sizeof(long double)?) to at least warn if the test turns out to be > inconclusive? > > Maybe even construct some values from fields and check for equality > against a long double literal?
Re: [Qemu-devel] [PATCH] Fix off-by-1 error in RAM migration code
David Gibson wrote: > On Fri, Nov 02, 2012 at 11:58:32AM +0100, Juan Quintela wrote: >> David Gibson wrote: >> > On Wed, Oct 31, 2012 at 01:08:16PM +0200, Orit Wasserman wrote: >> >> On 10/31/2012 05:43 AM, David Gibson wrote: >> >> Reviewed-by: Juan Quintela >> >> Good catch, I missunderstood the function when fixing a different bug, >> and never undrestood why it fixed it. > > Actually.. it just occurred to me that I think there has to be another > bug here somewhere.. I am at KVM Forum and LinuxCon for this week, so I can't test anything. For some reason, I missunderstood bitmap_set() and though this was the value that we "initiliazed" the bitmap with. So, I changed from 0 to 1, and . I was sending half the pages over the wire. Yes, that is right, just half of them. So clearly we have some bug somewhere else :-() > > I haven't actually observed any effects from the memory corruption - > though it's certainly a real bug. I found this because another effect > of this bug is that migration_dirty_pages count was set to 1 more than > the actual number of dirty bits in the bitmap. That meant the dirty > pages count was never reaching zero and so the migration/savevm never > terminated. I wonder what is on page 0 on an x86, problably some BIOS data that never changes? No clue about pseries. > Except.. that every so often the migration *did* terminate (maybe 1 > time in 5). Also I kind of hope somebody would have noticed this > earlier if migrations never terminated on x86 too. But as far as I > can tell, if initially mismatched like this it ought to be impossible > for the dirty page count to ever reach zero. Which suggests there is > another bug with the dirty count tracking :(. We use the dirty bitmap count to know how many pages are dirty, but once that the number is low enough, we just sent "the rest" of the pages. So, it would always converge (or not) independent of that bug by one. We never test for zero dirty pages, we test "are we able to send this many pages" over "max_downtime". So this explains why it works for you sometimes. > > It's possible the memory corruption could account for this, of course > - since that in theory at least, could have almost any strange effect > on the program's behavior. But that doesn't seem particularly likely > to me. This depends on _what_ is on page zero, if that is differente for whatever we put there during boot, and it we ever wrote to that page again, we would mark that pge dirty anyways, so I would put that "corruption" problem as highly improbable. Not that we shouldn't fix the bug, but I doubt that you are getting memory corruption due to this bug. The only way that you can get memory corruption is if you write to that page just before you do migration, and then you never wrote to it again. What is on hardware page zero on pseries? or it is just a normal page? Later, Juan.
Re: [Qemu-devel] [PATCH] memory: Reintroduce dirty flag to optimize changes on disabled regions
On 11/04/2012 10:30 AM, Jan Kiszka wrote: > From: Jan Kiszka > > Cirrus is triggering this, e.g. during Win2k boot: Changes only on > disabled regions require no topology update when transaction depth drops > to 0 again. 817dcc5368988b0 (pci: give each device its own address space) mad this much worse by multiplying the number of address spaces. Each change is now evaluated N+2 times, where N is the number of PCI devices. It also causes a corresponding expansion in memory usage. I want to address this by caching AddressSpaceDispatch trees with the key being the contents of the FlatView for that address space. This will drop the number of distinct trees to 2-4 (3 if some devices have PCI_COMMAND_MASTER disabled, 4 if the PCI address space is different from the cpu memory address space) but will fail if we make each address space different (for example filtering out the device's own BARs). If this change also improves cpu usage sufficiently, then it will be better than your patch, which doesn't recognize changes in an enabled region inside a disabled or hidden region. In other words, your patch fits the problem at hand but isn't general. On the other hand my approach doesn't eliminate render_memory_region(), just the exec.c stuff and listener updates. So we need to understand where the slowness comes from. > > @@ -1543,6 +1554,7 @@ void address_space_init(AddressSpace *as, MemoryRegion > *root) > flatview_init(as->current_map); > QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link); > as->name = NULL; > +memory_region_update_pending = true; > memory_region_transaction_commit(); > address_space_init_dispatch(as); > } > @@ -1552,6 +1564,7 @@ void address_space_destroy(AddressSpace *as) > /* Flush out anything from MemoryListeners listening in on this */ > memory_region_transaction_begin(); > as->root = NULL; > +memory_region_update_pending = true; > memory_region_transaction_commit(); > QTAILQ_REMOVE(&address_spaces, as, address_spaces_link); > address_space_destroy_dispatch(as); init and destroy cannot happen to a region that is mapped (and cannot happen within a transaction), so these two are redundant. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] qemu git master fails to build
Paolo, On current git master, if I do: ./configure --prefix=/home/somlo/KVM-OSX/SCRATCH --target-list=x86_64-softmmu make clean make I end up with: CCqemu-timer.o qemu-timer.c: In function `init_timer_alarm': qemu-timer.c:780:5: error: implicit declaration of function `pthread_atfork' [-Werror=implicit-function-declaration] qemu-timer.c:780:5: error: nested extern declaration of `pthread_atfork' [-Werror=nested-externs] cc1: all warnings being treated as errors make: *** [qemu-timer.o] Error 1 A 'git bisect' points at c8122c35e611385b31e2d8ccb059d0687540244a (qemu-timer: reinitialize timers after fork) as the possible culprit. Also, adding '#include ' to qemu-timer.c seems to make it happy. Thanks, --Gabriel
[Qemu-devel] [PATCH] qemu-timer: Fix compilation for POSIX and non-POSIX hosts
This compiler error is fixed by including pthread.h:
qemu-timer.c: In function ‘init_timer_alarm’:
qemu-timer.c:782: error: implicit declaration of function ‘pthread_atfork’
Another compiler warning is caused by the unused local function
reinit_timers on non-POSIX hosts. Include that function only for
POSIX hosts.
Signed-off-by: Stefan Weil
---
qemu-timer.c |5 +
1 file changed, 5 insertions(+)
diff --git a/qemu-timer.c b/qemu-timer.c
index 7b2217a..3bc86cf 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -34,6 +34,9 @@
#ifdef _WIN32
#include
#endif
+#ifdef CONFIG_POSIX
+#include /* pthread_atfork */
+#endif
/***/
/* timers */
@@ -742,6 +745,7 @@ static void quit_timers(void)
t->stop(t);
}
+#ifdef CONFIG_POSIX
static void reinit_timers(void)
{
struct qemu_alarm_timer *t = alarm_timer;
@@ -752,6 +756,7 @@ static void reinit_timers(void)
}
qemu_rearm_alarm_timer(t);
}
+#endif /* CONFIG_POSIX */
int init_timer_alarm(void)
{
--
1.7.10.4
[Qemu-devel] [PATCH] target-mips: Fix compiler warnings caused by very large constants
Those constants are larger than 32 bits and need a suffix to avoid
warnings from some versions of gcc.
Signed-off-by: Stefan Weil
---
target-mips/dsp_helper.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index b59133e..2ab9956 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -3553,7 +3553,7 @@ target_ulong helper_dextr_rs_w(target_ulong ac,
target_ulong shift,
if (temp128 == 0) {
temp[0] = 0x0;
} else {
-temp[0] = 0x01;
+temp[0] = 0x01ULL;
}
set_DSPControl_overflow_flag(1, 23, env);
}
@@ -3653,7 +3653,7 @@ target_ulong helper_extr_s_h(target_ulong ac,
target_ulong shift,
if (temp > (int64_t)0x7FFF) {
temp = 0x7FFF;
set_DSPControl_overflow_flag(1, 23, env);
-} else if (temp < (int64_t)0x8000) {
+} else if (temp < (int64_t)0x8000LL) {
temp = 0x8000;
set_DSPControl_overflow_flag(1, 23, env);
}
--
1.7.10.4
Re: [Qemu-devel] qemu git master fails to build
"Gabriel L. Somlo" writes: > Paolo, > > On current git master, if I do: > > ./configure --prefix=/home/somlo/KVM-OSX/SCRATCH --target-list=x86_64-softmmu > make clean > make I just pushed a fix for this. Regards, Anthony Liguori > > > I end up with: > > CCqemu-timer.o > qemu-timer.c: In function `init_timer_alarm': > qemu-timer.c:780:5: error: implicit declaration of function `pthread_atfork' > [-Werror=implicit-function-declaration] > qemu-timer.c:780:5: error: nested extern declaration of `pthread_atfork' > [-Werror=nested-externs] > cc1: all warnings being treated as errors > make: *** [qemu-timer.o] Error 1 > > > A 'git bisect' points at c8122c35e611385b31e2d8ccb059d0687540244a > (qemu-timer: reinitialize timers after fork) as the possible culprit. > Also, adding '#include ' to qemu-timer.c seems to make it happy. > > > Thanks, > --Gabriel
[Qemu-devel] [PATCH] qemu-timer: Fix compilation for non-POSIX hosts
A compiler warning is caused by the unused local function reinit_timers
on non-POSIX hosts. Include that function only for POSIX hosts.
Signed-off-by: Stefan Weil
---
qemu-timer.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/qemu-timer.c b/qemu-timer.c
index 8d9cf38..0d2bb94 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -745,6 +745,7 @@ static void quit_timers(void)
t->stop(t);
}
+#ifdef CONFIG_POSIX
static void reinit_timers(void)
{
struct qemu_alarm_timer *t = alarm_timer;
@@ -755,6 +756,7 @@ static void reinit_timers(void)
}
qemu_rearm_alarm_timer(t);
}
+#endif /* CONFIG_POSIX */
int init_timer_alarm(void)
{
--
1.7.10.4
Re: [Qemu-devel] 1.1.1 -> 1.1.2 migrate /managedsave issue
Avi Kivity writes: > On 10/22/2012 09:04 AM, Philipp Hahn wrote: >> Hello Doug, >> >> On Saturday 20 October 2012 00:46:43 Doug Goldstein wrote: >>> I'm using libvirt 0.10.2 and I had qemu-kvm 1.1.1 running all my VMs. >> ... >>> I had upgraded to qemu-kvm 1.1.2 >> ... >>> qemu: warning: error while loading state for instance 0x0 of device 'ram' >>> load of migration failed >> >> That error can be from many things. For me it was that the PXE-ROM images >> for >> the network cards were updated as well. Their size changed over the next >> power-of-two size, so kvm needed to allocate less/more memory and changed >> some PCI configuration registers, where the size of the ROM region is stored. >> On loading the saved state those sizes were compared and failed to validate. >> KVM then aborts loading the saved state with that little helpful message. >> >> So you might want to check, if your case is similar to mine. >> >> I diagnosed that using gdb to single step kvm until I found >> hw/pci.c#get_pci_config_device() returning -EINVAL. >> > > Seems reasonable. Doug, please verify to see if it's the same issue or > another one. > > Juan, how can we fix this? It's clear that the option ROM size has to > be fixed and not change whenever the blob is updated. This will fix it > for future releases. But what to do about the ones in the field? This is not a problem upstream because we don't alter the ROMs. If we did, we would keep the old ROMs around and set the romfile property in the compatible machine. This is what distros that are shipping ROMs outside of QEMU ought to do. It's a bug to unconditionally change the ROMs (in a guest visible way) without adding compatibility support. Regards, Anthony Liguori > > -- > error compiling committee.c: too many arguments to function > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Testing migration under stress
On Fri, Nov 02, 2012 at 02:07:45PM +0100, Juan Quintela wrote: > David Gibson wrote: > > Asking for some advice on the list. > > > > I have prorotype savevm and migration support ready for the pseries > > machine. They seem to work under simple circumstances (idle guest). > > To test them more extensively I've been attempting to perform live > > migrations (just over tcp->localhost) which the guest is active with > > something. In particular I've tried while using octave to do matrix > > multiply (so exercising the FP unit) and my colleague Alexey has tried > > during some video encoding. > > > > However, in each of these cases, we've found that the migration only > > completes and the source instance only stops after the intensive > > workload has (just) completed. What I surmise is happening is that > > the workload is touching memory pages fast enough that the ram > > migration code is never getting below the threshold to complete the > > migration until the guest is idle again. > > > > Does anyone have some ideas for testing this better: workloads that > > are less likely to trigger this behaviour, or settings to tweak in the > > migration itself to make it more likely to complete migration while > > the workload is still active. > > You can: > > migrate_set_downtime 2s (or so) > > I normally run stress, and you move the memory that it dirties until it > converges (depends a lot of your networking). So, I'm using tcp to localhost, so it should be really fast, but it doesn't seem to be :/. I suspect there are some other bugs here. > Doing anything that is really memory intensive is basically never gonig > to converge. Well, I didn't think the loads I chose would be memory limited (especially the video encode), but.. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] Testing migration under stress
On Fri, Nov 02, 2012 at 02:12:25PM +0200, Orit Wasserman wrote: > On 11/02/2012 05:10 AM, David Gibson wrote: > > Asking for some advice on the list. > > > > I have prorotype savevm and migration support ready for the pseries > > machine. They seem to work under simple circumstances (idle guest). > > To test them more extensively I've been attempting to perform live > > migrations (just over tcp->localhost) which the guest is active with > > something. In particular I've tried while using octave to do matrix > > multiply (so exercising the FP unit) and my colleague Alexey has tried > > during some video encoding. > As you are doing local migration one option is to setting the speed > higher than line speed , as we don't actually send the data, another > is to set high downtime. I'm not entirely sure what you mean by that. But I do have suspicions based on this and other factors that the default bandwidth it is limiting to is horribly, horribly low. > > However, in each of these cases, we've found that the migration only > > completes and the source instance only stops after the intensive > > workload has (just) completed. What I surmise is happening is that > > the workload is touching memory pages fast enough that the ram > > migration code is never getting below the threshold to complete the > > migration until the guest is idle again. > > > The workload you chose is really bad for live migration, as all the > guest does is dirtying his memory. Well, I realised that was true of the matrix multiply. For video encode though, the output data should be much, much smaller than the input, so I wouldn't expect it to be dirtying memory that fast. > I recommend looking for workload > that does some networking or disk IO. Vinod succeeded running > SwingBench and SLOB benchmarks that converged ok, I don't know if > they run on pseries, but similar workload should be ok(small > database/warehouse). We found out that SpecJbb on the other hand is > hard to converge. Web workload or video streaming also do the > trick. Hrm. As something really simple and stupid, I did try migrationg an ls -lR /, but even that didn't converge :/. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH] Fix off-by-1 error in RAM migration code
On Sun, Nov 04, 2012 at 08:17:29PM +0100, Juan Quintela wrote: > David Gibson wrote: > > On Fri, Nov 02, 2012 at 11:58:32AM +0100, Juan Quintela wrote: > >> David Gibson wrote: > >> > On Wed, Oct 31, 2012 at 01:08:16PM +0200, Orit Wasserman wrote: > >> >> On 10/31/2012 05:43 AM, David Gibson wrote: > >> > >> Reviewed-by: Juan Quintela > >> > >> Good catch, I missunderstood the function when fixing a different bug, > >> and never undrestood why it fixed it. > > > > Actually.. it just occurred to me that I think there has to be another > > bug here somewhere.. > > I am at KVM Forum and LinuxCon for this week, so I can't test anything. > > For some reason, I missunderstood bitmap_set() and though this was the > value that we "initiliazed" the bitmap with. So, I changed from 0 to 1, > and . I was sending half the pages over the wire. Yes, that is > right, just half of them. So clearly we have some bug somewhere > else :-() Ah. > > I haven't actually observed any effects from the memory corruption - > > though it's certainly a real bug. I found this because another effect > > of this bug is that migration_dirty_pages count was set to 1 more than > > the actual number of dirty bits in the bitmap. That meant the dirty > > pages count was never reaching zero and so the migration/savevm never > > terminated. > > I wonder what is on page 0 on an x86, problably some BIOS data that > never changes? No clue about pseries. On pseries it will be exception vectors. So it will change on the firmware->kernel transition. And I think there may be a very few special variables in there, so it will change later. Note that the migration_dirty_pages count will remain mismatched, whether or not page 0 gets touched. > > Except.. that every so often the migration *did* terminate (maybe 1 > > time in 5). Also I kind of hope somebody would have noticed this > > earlier if migrations never terminated on x86 too. But as far as I > > can tell, if initially mismatched like this it ought to be impossible > > for the dirty page count to ever reach zero. Which suggests there is > > another bug with the dirty count tracking :(. > > We use the dirty bitmap count to know how many pages are dirty, but once > that the number is low enough, we just sent "the rest" of the pages. > So, it would always converge (or not) independent of that bug by one. > We never test for zero dirty pages, we test "are we able to send this > many pages" over "max_downtime". So this explains why it works for you > sometimes. Hrm. It explains why it works sometimes (phew the bug I though must be there needn't be). It doesn't explain why it mostly didn't - I actually saw it spinning there on ram_save_iterate(), not completing with one dirty page remaining. But t > > It's possible the memory corruption could account for this, of course > > - since that in theory at least, could have almost any strange effect > > on the program's behavior. But that doesn't seem particularly likely > > to me. > > This depends on _what_ is on page zero, if that is differente for > whatever we put there during boot, and it we ever wrote to that page > again, we would mark that pge dirty anyways, so I would put that > "corruption" problem as highly improbable. Not that we shouldn't fix > the bug, but I doubt that you are getting memory corruption due to this > bug. > > The only way that you can get memory corruption is if you write to that > page just before you do migration, and then you never wrote to it again. > What is on hardware page zero on pseries? or it is just a normal page? You misunderstand. I wasn't talking about potential corruption of guest memory because we fail to transmit page 0 (though that is also a bug). I'm talking about corruption of qemu internal memory because the bitmap_set() writes one 1 bit beyond the end of the space allocated for the migration bitmap - it has no kind of bounds checking. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical address space
> -Original Message-
> From: Jan Kiszka [mailto:jan.kis...@web.de]
> Sent: Sunday, November 04, 2012 8:55 PM
> To: Hao, Xudong
> Cc: qemu-devel@nongnu.org; a...@redhat.com; k...@vger.kernel.org
> Subject: Re: [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical
> address space
>
> On 2012-11-04 13:15, Hao, Xudong wrote:
> >> -Original Message-
> >> From: Jan Kiszka [mailto:jan.kis...@web.de]
> >> Sent: Saturday, November 03, 2012 6:55 PM
> >> To: Hao, Xudong
> >> Cc: qemu-devel@nongnu.org; a...@redhat.com; k...@vger.kernel.org
> >> Subject: Re: [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical
> >> address space
> >>
> >> On 2012-11-02 06:38, Xudong Hao wrote:
> >>> For 64 bit processor, emulate 40 bits physical address if the host
> >>> physical
> >>> address space >= 40bits, else guest physical is same as host.
> >>>
> >>> Signed-off-by: Xudong Hao
> >>> ---
> >>> target-i386/cpu.c |5 -
> >>> 1 files changed, 4 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >>> index 423e009..3a78881 100644
> >>> --- a/target-i386/cpu.c
> >>> +++ b/target-i386/cpu.c
> >>> @@ -1584,7 +1584,10 @@ void cpu_x86_cpuid(CPUX86State *env,
> uint32_t
> >> index, uint32_t count,
> >>> if (env->cpuid_ext2_features & CPUID_EXT2_LM) {
> >>> /* 64 bit processor */
> >>> /* XXX: The physical address space is limited to 42 bits in exec.c. */
> >>> -*eax = 0x3028; /* 48 bits virtual, 40 bits physical
> */
> >>> +/* XXX: 40 bits physical if host physical address space >= 40 bits */
> >>> +uint32_t a, b, c, d;
> >>> +host_cpuid(0x8008, 0, &a, &b, &c, &d);
> >>> +*eax = a < 0x3028 ? a : 0x3028;
> >>
> >> This variation will not only affect -cpu host, right? That can create
> >> problems when migrating between hosts with different address widths, and
> >> then we will need some control knob to adjust what it reported to the
> >> guest.
> >>
> >
> > Oh, I did not consider migrating to different platform(addr widths).
> > But I think the fixed value 40 bits may cause problem: in VT-d case, when a
> host support GAW < 40 bits, and qemu emulate 40 bits guest physical address
> space, will bring bug on:
> >
> > drivers/iommu/intel-iommu.c
> > static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
> > unsigned long pfn, int target_level)
> > {
> > int addr_width = agaw_to_width(domain->agaw) - VTD_PAGE_SHIFT;
> > ...
> > BUG_ON(!domain->pgd);
> > BUG_ON(addr_width < BITS_PER_LONG && pfn >> addr_width);
> >
>
> Does it mean that buggy or malicious user space can trigger a kernel
> bug? Then this must be fixed of course.
>
Probably yes, when guest RAM is large enough or allocate MMIO to very high
address.
Jan, I'm not familiar the migration, do you have interest to add the migration
part fixing?
Re: [Qemu-devel] [PATCH] xhci: Fix some DMA host endian bugs
On Fri, Nov 02, 2012 at 08:31:23AM +0100, Gerd Hoffmann wrote:
> Hi,
>
> > +static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
> > + uint32_t *buf, size_t len)
> > +{
> > +int i;
> > +
> > +for (i = 0; i < (len / sizeof(uint32_t)); i++) {
> > +buf[i] = cpu_to_le32(buf[i]);
> > +}
> > +pci_dma_write(&xhci->pci_dev, addr, buf, len);
> > +}
>
> I think we should use a temporary buffer here, otherwise you leave the
> values byteswapped in buf which likely has unwanted side effects.
Here's an updated version that uses a temporary buffer.
>From 588a8f874c8d5a658ef95e35164e182a915091db Mon Sep 17 00:00:00 2001
From: David Gibson
Date: Mon, 5 Nov 2012 14:29:01 +1100
Subject: [PATCH] xhci: Fix some DMA host endian bugs
The xhci device does correct endian switches on the results of some DMAs
but not all. In particular, there are many DMAs of what are essentially
arrays of 32-bit integers which never get byteswapped. This causes them
to be interpreted incorrectly on big-endian hosts, since (as per the xhci
spec) these arrays are always little-endian in guest memory.
This patch adds some helper functions to fix these bugs. This may not be
all the endian bugs in the xhci code, but it's certainly some of them and
the Linux guest xhci driver certainly gets further with these fixes.
Signed-off-by: David Gibson
---
v2:
* Change xhci_dma_write_u32s to use a temporary buffer rather than
altering the given data in place.
---
hw/usb/hcd-xhci.c | 81 +++--
1 file changed, 54 insertions(+), 27 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 7b65741..1465685 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -607,6 +607,34 @@ static inline dma_addr_t xhci_mask64(uint64_t addr)
}
}
+static inline void xhci_dma_read_u32s(XHCIState *xhci, dma_addr_t addr,
+ uint32_t *buf, size_t len)
+{
+int i;
+
+assert((len % sizeof(uint32_t)) == 0);
+
+pci_dma_read(&xhci->pci_dev, addr, buf, len);
+
+for (i = 0; i < (len / sizeof(uint32_t)); i++) {
+buf[i] = le32_to_cpu(buf[i]);
+}
+}
+
+static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
+ uint32_t *buf, size_t len)
+{
+int i;
+uint32_t tmp[len / sizeof(uint32_t)];
+
+assert((len % sizeof(uint32_t)) == 0);
+
+for (i = 0; i < (len / sizeof(uint32_t)); i++) {
+tmp[i] = cpu_to_le32(buf[i]);
+}
+pci_dma_write(&xhci->pci_dev, addr, tmp, len);
+}
+
static XHCIPort *xhci_lookup_port(XHCIState *xhci, struct USBPort *uport)
{
int index;
@@ -1018,14 +1046,14 @@ static void xhci_set_ep_state(XHCIState *xhci,
XHCIEPContext *epctx,
{
uint32_t ctx[5];
-pci_dma_read(&xhci->pci_dev, epctx->pctx, ctx, sizeof(ctx));
+xhci_dma_read_u32s(xhci, epctx->pctx, ctx, sizeof(ctx));
ctx[0] &= ~EP_STATE_MASK;
ctx[0] |= state;
ctx[2] = epctx->ring.dequeue | epctx->ring.ccs;
ctx[3] = (epctx->ring.dequeue >> 16) >> 16;
DPRINTF("xhci: set epctx: " DMA_ADDR_FMT " state=%d dequeue=%08x%08x\n",
epctx->pctx, state, ctx[3], ctx[2]);
-pci_dma_write(&xhci->pci_dev, epctx->pctx, ctx, sizeof(ctx));
+xhci_dma_write_u32s(xhci, epctx->pctx, ctx, sizeof(ctx));
epctx->state = state;
}
@@ -1857,14 +1885,14 @@ static TRBCCode xhci_address_slot(XHCIState *xhci,
unsigned int slotid,
assert(slotid >= 1 && slotid <= xhci->numslots);
dcbaap = xhci_addr64(xhci->dcbaap_low, xhci->dcbaap_high);
-pci_dma_read(&xhci->pci_dev, dcbaap + 8*slotid, &poctx, sizeof(poctx));
+poctx = ldq_le_pci_dma(&xhci->pci_dev, dcbaap + 8*slotid);
ictx = xhci_mask64(pictx);
-octx = xhci_mask64(le64_to_cpu(poctx));
+octx = xhci_mask64(poctx);
DPRINTF("xhci: input context at "DMA_ADDR_FMT"\n", ictx);
DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx);
-pci_dma_read(&xhci->pci_dev, ictx, ictl_ctx, sizeof(ictl_ctx));
+xhci_dma_read_u32s(xhci, ictx, ictl_ctx, sizeof(ictl_ctx));
if (ictl_ctx[0] != 0x0 || ictl_ctx[1] != 0x3) {
fprintf(stderr, "xhci: invalid input context control %08x %08x\n",
@@ -1872,8 +1900,8 @@ static TRBCCode xhci_address_slot(XHCIState *xhci,
unsigned int slotid,
return CC_TRB_ERROR;
}
-pci_dma_read(&xhci->pci_dev, ictx+32, slot_ctx, sizeof(slot_ctx));
-pci_dma_read(&xhci->pci_dev, ictx+64, ep0_ctx, sizeof(ep0_ctx));
+xhci_dma_read_u32s(xhci, ictx+32, slot_ctx, sizeof(slot_ctx));
+xhci_dma_read_u32s(xhci, ictx+64, ep0_ctx, sizeof(ep0_ctx));
DPRINTF("xhci: input slot context: %08x %08x %08x %08x\n",
slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]);
@@ -1923,8 +1951,8 @@ static TRBCCode xhci_address_slot(XHCIState *xhci,
unsigned int slotid,
DPRINTF("xhci: output ep0 context: %08x %08x %08x %08x %08x\n",
Re: [Qemu-devel] [patch v5 7/8] memory: introduce tls context to record nested dma
On Mon, Oct 29, 2012 at 4:51 PM, Paolo Bonzini wrote:
> Il 29/10/2012 00:48, Liu Ping Fan ha scritto:
>> Signed-off-by: Liu Ping Fan
>> ---
>> cpus.c|3 ++
>> exec.c| 58
>> +
>> qemu-thread.h |8 +++
>> vl.c |1 +
>> 4 files changed, 70 insertions(+), 0 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 191cbf5..e67d80f 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -733,6 +733,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>>
>> qemu_mutex_lock(&qemu_global_mutex);
>> qemu_thread_get_self(cpu->thread);
>> +qemu_thread_init_context();
>> env->thread_id = qemu_get_thread_id();
>> cpu_single_env = env;
>>
>> @@ -774,6 +775,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
>>
>> qemu_mutex_lock_iothread();
>> qemu_thread_get_self(cpu->thread);
>> +qemu_thread_init_context();
>> env->thread_id = qemu_get_thread_id();
>>
>> sigemptyset(&waitset);
>> @@ -813,6 +815,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>>
>> qemu_tcg_init_cpu_signals();
>> qemu_thread_get_self(cpu->thread);
>> +qemu_thread_init_context();
>>
>> /* signal CPU creation */
>> qemu_mutex_lock(&qemu_global_mutex);
>> diff --git a/exec.c b/exec.c
>> index 46da08c..ea672c6 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3449,6 +3449,49 @@ static bool
>> address_space_section_lookup_ref(AddressSpace *as,
>> return safe_ref;
>> }
>>
>> +typedef struct ThreadContext {
>> + DispatchType dispatch_type;
>> + unsigned int mmio_req_pending;
>> +} ThreadContext;
>> +
>> +static __thread ThreadContext *thread_context;
>> +
>> +void qemu_thread_init_context(void)
>> +{
>> +thread_context = g_new(ThreadContext, 1);
>> +thread_context->dispatch_type = DISPATCH_INIT;
>> +thread_context->mmio_req_pending = 0;
>> +}
>
> No need for this:
>
> static __thread ThreadContext thread_context = {
> .dispatch_type = DISPATCH_INIT,
> .mmio_req_pending = 0
> };
>
applied, thanks.
> Paolo
>
>
Re: [Qemu-devel] [patch v5 7/8] memory: introduce tls context to record nested dma
On Fri, Nov 2, 2012 at 6:39 PM, Jan Kiszka wrote:
> On 2012-10-29 00:48, Liu Ping Fan wrote:
>> Signed-off-by: Liu Ping Fan
>> ---
>> cpus.c|3 ++
>> exec.c| 58
>> +
>> qemu-thread.h |8 +++
>> vl.c |1 +
>> 4 files changed, 70 insertions(+), 0 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 191cbf5..e67d80f 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -733,6 +733,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>>
>> qemu_mutex_lock(&qemu_global_mutex);
>> qemu_thread_get_self(cpu->thread);
>> +qemu_thread_init_context();
>> env->thread_id = qemu_get_thread_id();
>> cpu_single_env = env;
>>
>> @@ -774,6 +775,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
>>
>> qemu_mutex_lock_iothread();
>> qemu_thread_get_self(cpu->thread);
>> +qemu_thread_init_context();
>> env->thread_id = qemu_get_thread_id();
>>
>> sigemptyset(&waitset);
>> @@ -813,6 +815,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>>
>> qemu_tcg_init_cpu_signals();
>> qemu_thread_get_self(cpu->thread);
>> +qemu_thread_init_context();
>>
>> /* signal CPU creation */
>> qemu_mutex_lock(&qemu_global_mutex);
>> diff --git a/exec.c b/exec.c
>> index 46da08c..ea672c6 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3449,6 +3449,49 @@ static bool
>> address_space_section_lookup_ref(AddressSpace *as,
>> return safe_ref;
>> }
>>
>> +typedef struct ThreadContext {
>> + DispatchType dispatch_type;
>> + unsigned int mmio_req_pending;
>> +} ThreadContext;
>> +
>> +static __thread ThreadContext *thread_context;
>> +
>> +void qemu_thread_init_context(void)
>> +{
>> +thread_context = g_new(ThreadContext, 1);
>> +thread_context->dispatch_type = DISPATCH_INIT;
>> +thread_context->mmio_req_pending = 0;
>> +}
>> +
>> +void qemu_thread_set_dispatch_type(DispatchType type)
>> +{
>> +thread_context->dispatch_type = type;
>> +}
>> +
>> +void qemu_thread_reset_dispatch_type(void)
>> +{
>> +thread_context->dispatch_type = DISPATCH_INIT;
>> +}
>> +
>> +static bool address_space_inc_req_pending(void)
>> +{
>> +bool nested = false;
>> +
>> +/* currently, only mmio out of big lock, and need this to avoid dead
>> lock */
>> +if (thread_context->dispatch_type == DISPATCH_MMIO) {
>> +nested = ++thread_context->mmio_req_pending > 1 ? true : false;
>> +}
>> +
>> +return nested;
>> +}
>> +
>> +static void address_space_dec_req_pending(void)
>> +{
>> +if (thread_context->dispatch_type == DISPATCH_MMIO) {
>> +thread_context->mmio_req_pending--;
>> +}
>> +}
>> +
>> void address_space_rw(AddressSpace *as, target_phys_addr_t addr, uint8_t
>> *buf,
>>int len, bool is_write)
>> {
>> @@ -3459,6 +3502,7 @@ void address_space_rw(AddressSpace *as,
>> target_phys_addr_t addr, uint8_t *buf,
>> target_phys_addr_t page;
>> bool safe_ref = false;
>> MemoryRegionSection *section, obj_mrs;
>> +bool nested_dma = false;
>>
>> while (len > 0) {
>> page = addr & TARGET_PAGE_MASK;
>> @@ -3485,10 +3529,17 @@ void address_space_rw(AddressSpace *as,
>> target_phys_addr_t addr, uint8_t *buf,
>> memory_region_section_lookup_ref(d, page, &obj_mrs);
>> }
>> section = &obj_mrs;
>> +nested_dma = address_space_inc_req_pending();
>>
>> if (is_write) {
>> if (!memory_region_is_ram(section->mr)) {
>> target_phys_addr_t addr1;
>> +
>> +/* To fix, will filter iommu case */
>> +if (nested_dma) {
>> +fprintf(stderr, "can not support nested DMA");
>> +abort();
>> +}
>> addr1 = memory_region_section_addr(section, addr);
>> /* XXX: could force cpu_single_env to NULL to avoid
>> potential bugs */
>> @@ -3522,6 +3573,12 @@ void address_space_rw(AddressSpace *as,
>> target_phys_addr_t addr, uint8_t *buf,
>> if (!(memory_region_is_ram(section->mr) ||
>>memory_region_is_romd(section->mr))) {
>> target_phys_addr_t addr1;
>> +
>> +/* To fix, will filter iommu case */
>> +if (nested_dma) {
>> +fprintf(stderr, "can not support nested DMA");
>> +abort();
>> +}
>> /* I/O case */
>> addr1 = memory_region_section_addr(section, addr);
>> if (l >= 4 && ((addr1 & 3) == 0)) {
>> @@ -3549,6 +3606,7 @@ void address_space_rw(AddressSpace *as,
>> target_phys_addr_t addr, uint8_t *buf,
>> qemu_put_ram_ptr(ptr);
>> }
>> }
>> +address_space_dec_req_pending();
>> memory_region_section_unref(&obj_mrs);
>> len -= l;
>> buf += l;
>> diff --git a/qem
[Qemu-devel] [PATCH v6 0/8] push mmio dispatch out of big lock
From: Liu Ping Fan v1: https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03312.html v2: http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01275.html v3: http://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg01474.html v4: http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg03857.html v5: https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg04867.html changes v5->v6: Apply fine-grain lock for all address space. Introduce separated interface to allow mmio dispatcher called with/without big lock. Liu Ping Fan (8): atomic: introduce atomic operations qom: apply atomic on object's refcount hotplug: introduce qdev_unplug_complete() to remove device from views pci: remove pci device from mem view when unplug memory: introduce local lock for address space memory: make mmio dispatch able to be out of biglock memory: introduce tls context to trace nested mmio request issue vcpu: push mmio dispatcher out of big lock cpu-common.h |3 + docs/memory.txt |4 + exec.c| 219 + hw/acpi_piix4.c |2 +- hw/pci.c | 13 +++- hw/pci.h |1 + hw/qdev.c | 26 ++ hw/qdev.h |3 +- include/qemu/atomic.h | 63 ++ include/qemu/object.h |3 +- kvm-all.c |6 +- memory-internal.h |1 + memory.c |1 + memory.h |5 + qemu-thread.h |7 ++ qom/object.c | 11 +-- 16 files changed, 340 insertions(+), 28 deletions(-) create mode 100644 include/qemu/atomic.h -- 1.7.4.4
[Qemu-devel] [PATCH v6 1/8] atomic: introduce atomic operations
From: Liu Ping Fan
If out of global lock, we will be challenged by SMP in low level,
so need atomic ops.
This file is a wrapper of GCC atomic builtin.
Signed-off-by: Liu Ping Fan
---
include/qemu/atomic.h | 63 +
1 files changed, 63 insertions(+), 0 deletions(-)
create mode 100644 include/qemu/atomic.h
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
new file mode 100644
index 000..a9e6d35
--- /dev/null
+++ b/include/qemu/atomic.h
@@ -0,0 +1,63 @@
+/*
+ * Simple wrapper of gcc Atomic-Builtins
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#ifndef __QEMU_ATOMIC_H
+#define __QEMU_ATOMIC_H
+
+typedef struct Atomic {
+volatile int counter;
+} Atomic;
+
+static inline void atomic_set(Atomic *v, int i)
+{
+v->counter = i;
+}
+
+static inline int atomic_read(Atomic *v)
+{
+return v->counter;
+}
+
+static inline int atomic_return_and_add(int i, Atomic *v)
+{
+int ret;
+
+ret = __sync_fetch_and_add(&v->counter, i);
+return ret;
+}
+
+static inline int atomic_return_and_sub(int i, Atomic *v)
+{
+int ret;
+
+ret = __sync_fetch_and_sub(&v->counter, i);
+return ret;
+}
+
+/**
+ * * atomic_inc - increment atomic variable
+ * * @v: pointer of type Atomic
+ **
+ * * Atomically increments @v by 1.
+ * */
+static inline void atomic_inc(Atomic *v)
+{
+__sync_fetch_and_add(&v->counter, 1);
+}
+
+/**
+ * * atomic_dec - decrement atomic variable
+ * * @v: pointer of type Atomic
+ **
+ * * Atomically decrements @v by 1.
+ * */
+static inline void atomic_dec(Atomic *v)
+{
+__sync_fetch_and_sub(&v->counter, 1);
+}
+
+#endif
--
1.7.4.4
[Qemu-devel] [PATCH v6 2/8] qom: apply atomic on object's refcount
From: Liu Ping Fan
Signed-off-by: Liu Ping Fan
---
include/qemu/object.h |3 ++-
qom/object.c | 11 +--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index cc75fee..0c02614 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -18,6 +18,7 @@
#include
#include
#include "qemu-queue.h"
+#include "qemu/atomic.h"
struct Visitor;
struct Error;
@@ -262,7 +263,7 @@ struct Object
/*< private >*/
ObjectClass *class;
QTAILQ_HEAD(, ObjectProperty) properties;
-uint32_t ref;
+Atomic ref;
Object *parent;
};
diff --git a/qom/object.c b/qom/object.c
index e3e9242..34ec2a1 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -383,7 +383,7 @@ void object_finalize(void *data)
object_deinit(obj, ti);
object_property_del_all(obj);
-g_assert(obj->ref == 0);
+g_assert(atomic_read(&obj->ref) == 0);
}
Object *object_new_with_type(Type type)
@@ -410,7 +410,7 @@ Object *object_new(const char *typename)
void object_delete(Object *obj)
{
object_unparent(obj);
-g_assert(obj->ref == 1);
+g_assert(atomic_read(&obj->ref) == 1);
object_unref(obj);
g_free(obj);
}
@@ -600,16 +600,15 @@ GSList *object_class_get_list(const char *implements_type,
void object_ref(Object *obj)
{
-obj->ref++;
+atomic_inc(&obj->ref);
}
void object_unref(Object *obj)
{
-g_assert(obj->ref > 0);
-obj->ref--;
+g_assert(atomic_read(&obj->ref) > 0);
/* parent always holds a reference to its children */
-if (obj->ref == 0) {
+if (atomic_return_and_sub(1, &obj->ref) == 1) {
object_finalize(obj);
}
}
--
1.7.4.4
[Qemu-devel] [PATCH v6 6/8] memory: make mmio dispatch able to be out of biglock
From: Liu Ping Fan
Without biglock, we try to protect the mr by increase refcnt.
If we can inc refcnt, go backward and resort to biglock.
Another point is memory radix-tree can be flushed by another
thread, so we should get the copy of terminal mr to survive
from such issue.
Signed-off-by: Liu Ping Fan
---
docs/memory.txt |4 ++
exec.c| 154 +++-
memory-internal.h |1 +
memory.h |2 +
4 files changed, 146 insertions(+), 15 deletions(-)
diff --git a/docs/memory.txt b/docs/memory.txt
index 5bbee8e..6b3d15a 100644
--- a/docs/memory.txt
+++ b/docs/memory.txt
@@ -170,3 +170,7 @@ various constraints can be supplied to control how these
callbacks are called:
- .old_portio and .old_mmio can be used to ease porting from code using
cpu_register_io_memory() and register_ioport(). They should not be used
in new code.
+
+MMIO regions are provided with ->ref() and ->unref() callbacks; This pair
callbacks
+are optional. When ref() return non-zero, Both MemoryRegion and its opaque are
+safe to use.
diff --git a/exec.c b/exec.c
index 750008c..fa34ef9 100644
--- a/exec.c
+++ b/exec.c
@@ -2280,7 +2280,7 @@ static void register_multipage(AddressSpaceDispatch *d,
MemoryRegionSection *sec
section_index);
}
-static void mem_add(MemoryListener *listener, MemoryRegionSection *section)
+static void mem_nop(MemoryListener *listener, MemoryRegionSection *section)
{
AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch,
listener);
MemoryRegionSection now = *section, remain = *section;
@@ -2314,6 +2314,26 @@ static void mem_add(MemoryListener *listener,
MemoryRegionSection *section)
}
}
+static void mem_add(MemoryListener *listener, MemoryRegionSection *section)
+{
+MemoryRegion *mr = section->mr;
+
+if (mr->ops && mr->ops->ref) {
+mr->ops->ref(mr);
+}
+mem_nop(listener, section);
+}
+
+static void mem_del(MemoryListener *listener,
+MemoryRegionSection *section)
+{
+MemoryRegion *mr = section->mr;
+
+if (mr->ops && mr->ops->unref) {
+mr->ops->unref(mr);
+}
+}
+
void qemu_flush_coalesced_mmio_buffer(void)
{
if (kvm_enabled())
@@ -3165,11 +3185,23 @@ static void io_mem_init(void)
static void mem_begin(MemoryListener *listener)
{
AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch,
listener);
+AddressSpace *as = d->as;
+/* protect the updating process of mrs in memory core agaist readers */
+qemu_mutex_lock(&as->lock);
destroy_all_mappings(d);
d->phys_map.ptr = PHYS_MAP_NODE_NIL;
}
+static void mem_commit(MemoryListener *listener)
+{
+AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch,
+listener);
+AddressSpace *as = d->as;
+
+qemu_mutex_unlock(&as->lock);
+}
+
static void core_begin(MemoryListener *listener)
{
phys_sections_clear();
@@ -3243,11 +3275,14 @@ void address_space_init_dispatch(AddressSpace *as)
d->phys_map = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .is_leaf = 0 };
d->listener = (MemoryListener) {
.begin = mem_begin,
+.commit = mem_commit,
.region_add = mem_add,
-.region_nop = mem_add,
+.region_del = mem_del,
+.region_nop = mem_nop,
.priority = 0,
};
as->dispatch = d;
+as->dispatch->as = as;
memory_listener_register(&d->listener, as);
}
@@ -3345,6 +3380,68 @@ static void invalidate_and_set_dirty(target_phys_addr_t
addr,
xen_modified_memory(addr, length);
}
+static MemoryRegionSection *subpage_get_terminal(subpage_t *mmio,
+target_phys_addr_t addr)
+{
+MemoryRegionSection *section;
+unsigned int idx = SUBPAGE_IDX(addr);
+
+section = &phys_sections[mmio->sub_section[idx]];
+return section;
+}
+
+static bool memory_region_section_ref(MemoryRegionSection *mrs)
+{
+MemoryRegion *mr;
+bool ret = false;
+
+mr = mrs->mr;
+if (mr->ops && mr->ops->ref) {
+mr->ops->ref(mr);
+ret = true;
+}
+return ret;
+}
+
+static void memory_region_section_unref(MemoryRegionSection *mrs)
+{
+MemoryRegion *mr;
+
+mr = mrs->mr;
+if (mr->ops && mr->ops->unref) {
+mr->ops->unref(mr);
+}
+}
+
+static bool memory_region_section_lookup_ref(AddressSpaceDispatch *d,
+target_phys_addr_t addr, MemoryRegionSection *mrs)
+{
+MemoryRegionSection *section;
+bool ret;
+
+section = phys_page_find(d, addr >> TARGET_PAGE_BITS);
+if (section->mr->subpage) {
+section = subpage_get_terminal(section->mr->opaque, addr);
+}
+*mrs = *section;
+ret = memory_region_section_ref(mrs);
+
+return ret;
+}
+
+static bool address_space_section_lookup_ref(AddressSpace *as,
+target_phys_addr_t page, MemoryRegionSection *mrs)
+{
+bool safe_ref;
+AddressSpaceDispatch *d = as->dispatch;
+
+qemu_mutex_lock
Re: [Qemu-devel] 1.1.1 -> 1.1.2 migrate /managedsave issue
On Sun, Nov 4, 2012 at 3:51 PM, Anthony Liguori wrote: > Avi Kivity writes: > >> On 10/22/2012 09:04 AM, Philipp Hahn wrote: >>> Hello Doug, >>> >>> On Saturday 20 October 2012 00:46:43 Doug Goldstein wrote: I'm using libvirt 0.10.2 and I had qemu-kvm 1.1.1 running all my VMs. >>> ... I had upgraded to qemu-kvm 1.1.2 >>> ... qemu: warning: error while loading state for instance 0x0 of device 'ram' load of migration failed >>> >>> That error can be from many things. For me it was that the PXE-ROM images >>> for >>> the network cards were updated as well. Their size changed over the next >>> power-of-two size, so kvm needed to allocate less/more memory and changed >>> some PCI configuration registers, where the size of the ROM region is >>> stored. >>> On loading the saved state those sizes were compared and failed to validate. >>> KVM then aborts loading the saved state with that little helpful message. >>> >>> So you might want to check, if your case is similar to mine. >>> >>> I diagnosed that using gdb to single step kvm until I found >>> hw/pci.c#get_pci_config_device() returning -EINVAL. >>> >> >> Seems reasonable. Doug, please verify to see if it's the same issue or >> another one. >> >> Juan, how can we fix this? It's clear that the option ROM size has to >> be fixed and not change whenever the blob is updated. This will fix it >> for future releases. But what to do about the ones in the field? > > This is not a problem upstream because we don't alter the ROMs. If we > did, we would keep the old ROMs around and set the romfile property in > the compatible machine. > > This is what distros that are shipping ROMs outside of QEMU ought to > do. It's a bug to unconditionally change the ROMs (in a guest visible > way) without adding compatibility support. > > Regards, > > Anthony Liguori > Anthony, Gerd updated seabios on August 7th and before that on April 17. The default VGA ROM size also changed in recent releases. There are no old versions of the ROMs included once these updates are performed so a user building a new version from source will hit this problem. Juan Quintela even mentioned that he has been bit by this issue and had to use gdb to track it down as did Philipp that responded earlier in the thread. The patch is a simple fprintf() which would have saved at least 3 users the effort of tracking down an issue with gdb. So I urge you to reconsider. Thanks. -- Doug Goldstein
Re: [Qemu-devel] [Qemu-trivial] [PATCH] block: Workaround for older versions of MinGW gcc
On Sun, Nov 04, 2012 at 12:09:34PM +0100, Stefan Weil wrote: > Versions before gcc-4.6 don't support unnamed fields in initializers > (see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676). > > Offset and OffsetHigh belong to an unnamed struct which is part of an > unnamed union. Therefore the original code does not work with older > versions of gcc. > > Signed-off-by: Stefan Weil > --- > > This patch is needed for Debian's amd64-mingw32msvc-gcc-4.4.4 > which I use for MinGW-w64 cross compilation. > > Regards > Stefan W. > > block/win32-aio.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Stefan Hajnoczi
[Qemu-devel] [PATCH v6 8/8] vcpu: push mmio dispatcher out of big lock
From: Liu Ping Fan
To anti the recursive big lock, introduce separate interfaces to allow
address space dispatcher called with/without big lock.
Signed-off-by: Liu Ping Fan
---
cpu-common.h |3 +++
exec.c | 22 ++
kvm-all.c|6 +-
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/cpu-common.h b/cpu-common.h
index c0d27af..69c1d7a 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -51,6 +51,9 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name,
DeviceState *dev);
void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
int len, int is_write);
+void cpu_physical_memory_nolock_rw(target_phys_addr_t addr, uint8_t *buf,
+int len, int is_write);
+
static inline void cpu_physical_memory_read(target_phys_addr_t addr,
void *buf, int len)
{
diff --git a/exec.c b/exec.c
index 1eb920d..73d5242 100644
--- a/exec.c
+++ b/exec.c
@@ -3484,8 +3484,8 @@ static void address_space_dec_req_pending(void)
}
}
-void address_space_rw(AddressSpace *as, target_phys_addr_t addr, uint8_t *buf,
- int len, bool is_write)
+static void address_space_rw_internal(AddressSpace *as, target_phys_addr_t
addr,
+ uint8_t *buf, int len, bool is_write, bool biglock)
{
AddressSpaceDispatch *d = as->dispatch;
int l;
@@ -3506,7 +3506,7 @@ void address_space_rw(AddressSpace *as,
target_phys_addr_t addr, uint8_t *buf,
qemu_mutex_unlock(&as->lock);
address_space_check_inc_req_pending(&obj_mrs);
-if (!safe_ref) {
+if (!safe_ref && !biglock) {
qemu_mutex_lock_iothread();
qemu_mutex_lock(&as->lock);
/* when 2nd try, mem map can change, need to judge it again */
@@ -3588,12 +3588,18 @@ void address_space_rw(AddressSpace *as,
target_phys_addr_t addr, uint8_t *buf,
len -= l;
buf += l;
addr += l;
-if (!safe_ref) {
+if (!safe_ref && !biglock) {
qemu_mutex_unlock_iothread();
}
}
}
+void address_space_rw(AddressSpace *as, target_phys_addr_t addr, uint8_t *buf,
+ int len, bool is_write)
+{
+address_space_rw_internal(as, addr, buf, len, is_write, true);
+}
+
void address_space_write(AddressSpace *as, target_phys_addr_t addr,
const uint8_t *buf, int len)
{
@@ -3619,6 +3625,14 @@ void cpu_physical_memory_rw(target_phys_addr_t addr,
uint8_t *buf,
return address_space_rw(&address_space_memory, addr, buf, len, is_write);
}
+void cpu_physical_memory_nolock_rw(target_phys_addr_t addr, uint8_t *buf,
+int len, int is_write)
+{
+return address_space_rw_internal(&address_space_memory, addr, buf, len,
+ is_write, false);
+}
+
+
/* used for ROM loading : can write in RAM and ROM */
void cpu_physical_memory_write_rom(target_phys_addr_t addr,
const uint8_t *buf, int len)
diff --git a/kvm-all.c b/kvm-all.c
index c2c6909..718f257 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1573,10 +1573,14 @@ int kvm_cpu_exec(CPUArchState *env)
break;
case KVM_EXIT_MMIO:
DPRINTF("handle_mmio\n");
-cpu_physical_memory_rw(run->mmio.phys_addr,
+qemu_mutex_unlock_iothread();
+qemu_thread_set_dispatch_type(DISPATCH_MMIO);
+cpu_physical_memory_nolock_rw(run->mmio.phys_addr,
run->mmio.data,
run->mmio.len,
run->mmio.is_write);
+qemu_thread_reset_dispatch_type();
+qemu_mutex_lock_iothread();
ret = 0;
break;
case KVM_EXIT_IRQ_WINDOW_OPEN:
--
1.7.4.4
[Qemu-devel] [PATCH v6 3/8] hotplug: introduce qdev_unplug_complete() to remove device from views
From: Liu Ping Fan
When device unplug has been ack by guest, we first remove it from memory
to prevent incoming access from dispatcher. Then we isolate it from
device composition tree
Signed-off-by: Liu Ping Fan
---
hw/qdev.c | 26 ++
hw/qdev.h |3 ++-
2 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 9b9aba3..681e133 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -98,6 +98,14 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
bus_add_child(bus, dev);
}
+static void qdev_unset_parent(DeviceState *dev)
+{
+BusState *b = dev->parent_bus;
+
+object_unparent(OBJECT(dev));
+bus_remove_child(b, dev);
+}
+
/* Create a new device. This only initializes the device state structure
and allows properties to be set. qdev_init should be called to
initialize the actual device emulation. */
@@ -187,6 +195,24 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int
alias_id,
dev->alias_required_for_version = required_for_version;
}
+static int qdev_unmap(DeviceState *dev)
+{
+DeviceClass *dc = DEVICE_GET_CLASS(dev);
+if (dc->unmap) {
+dc->unmap(dev);
+}
+return 0;
+}
+
+void qdev_unplug_complete(DeviceState *dev, Error **errp)
+{
+/* isolate from mem view */
+qdev_unmap(dev);
+/* isolate from device tree */
+qdev_unset_parent(dev);
+object_unref(OBJECT(dev));
+}
+
void qdev_unplug(DeviceState *dev, Error **errp)
{
DeviceClass *dc = DEVICE_GET_CLASS(dev);
diff --git a/hw/qdev.h b/hw/qdev.h
index c6ac636..71eb9ca 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -47,7 +47,7 @@ typedef struct DeviceClass {
/* callbacks */
void (*reset)(DeviceState *dev);
-
+void (*unmap)(DeviceState *dev);
/* device state */
const VMStateDescription *vmsd;
@@ -160,6 +160,7 @@ void qdev_init_nofail(DeviceState *dev);
void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
int required_for_version);
void qdev_unplug(DeviceState *dev, Error **errp);
+void qdev_unplug_complete(DeviceState *dev, Error **errp);
void qdev_free(DeviceState *dev);
int qdev_simple_unplug_cb(DeviceState *dev);
void qdev_machine_creation_done(void);
--
1.7.4.4
[Qemu-devel] [PATCH v6 7/8] memory: introduce tls context to trace nested mmio request issue
From: Liu Ping Fan
After breaking down big lock, nested MMIO request which not targeting
at RAM can cause deadlock issue. Supposing the scene: dev_a,b with
fine-grain locks lockA/B, then ABBA dealock issue can be triggered.
We fix this by tracing and rejecting such request.
Signed-off-by: Liu Ping Fan
---
exec.c| 47 +++
qemu-thread.h |7 +++
2 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/exec.c b/exec.c
index fa34ef9..1eb920d 100644
--- a/exec.c
+++ b/exec.c
@@ -3442,6 +3442,48 @@ static bool
address_space_section_lookup_ref(AddressSpace *as,
return safe_ref;
}
+typedef struct ThreadContext {
+ DispatchType dispatch_type;
+ unsigned int mmio_req_pending;
+} ThreadContext;
+
+static __thread ThreadContext thread_context = {
+.dispatch_type = DISPATCH_INIT,
+.mmio_req_pending = 0
+};
+
+void qemu_thread_set_dispatch_type(DispatchType type)
+{
+thread_context.dispatch_type = type;
+}
+
+void qemu_thread_reset_dispatch_type(void)
+{
+thread_context.dispatch_type = DISPATCH_INIT;
+}
+
+static void address_space_check_inc_req_pending(MemoryRegionSection *section)
+{
+bool nested = false;
+
+/* currently, only mmio out of big lock, and need this to avoid dead lock
*/
+if (thread_context.dispatch_type == DISPATCH_MMIO) {
+nested = ++thread_context.mmio_req_pending > 1 ? true : false;
+/* To fix, will filter iommu case */
+if (nested && !memory_region_is_ram(section->mr)) {
+fprintf(stderr, "mmio: nested target not RAM is not support");
+abort();
+}
+}
+}
+
+static void address_space_dec_req_pending(void)
+{
+if (thread_context.dispatch_type == DISPATCH_MMIO) {
+thread_context.mmio_req_pending--;
+}
+}
+
void address_space_rw(AddressSpace *as, target_phys_addr_t addr, uint8_t *buf,
int len, bool is_write)
{
@@ -3462,6 +3504,8 @@ void address_space_rw(AddressSpace *as,
target_phys_addr_t addr, uint8_t *buf,
qemu_mutex_lock(&as->lock);
safe_ref = memory_region_section_lookup_ref(d, page, &obj_mrs);
qemu_mutex_unlock(&as->lock);
+address_space_check_inc_req_pending(&obj_mrs);
+
if (!safe_ref) {
qemu_mutex_lock_iothread();
qemu_mutex_lock(&as->lock);
@@ -3477,6 +3521,7 @@ void address_space_rw(AddressSpace *as,
target_phys_addr_t addr, uint8_t *buf,
if (is_write) {
if (!memory_region_is_ram(section->mr)) {
target_phys_addr_t addr1;
+
addr1 = memory_region_section_addr(section, addr);
/* XXX: could force cpu_single_env to NULL to avoid
potential bugs */
@@ -3510,6 +3555,7 @@ void address_space_rw(AddressSpace *as,
target_phys_addr_t addr, uint8_t *buf,
if (!(memory_region_is_ram(section->mr) ||
memory_region_is_romd(section->mr))) {
target_phys_addr_t addr1;
+
/* I/O case */
addr1 = memory_region_section_addr(section, addr);
if (l >= 4 && ((addr1 & 3) == 0)) {
@@ -3537,6 +3583,7 @@ void address_space_rw(AddressSpace *as,
target_phys_addr_t addr, uint8_t *buf,
qemu_put_ram_ptr(ptr);
}
}
+address_space_dec_req_pending();
memory_region_section_unref(&obj_mrs);
len -= l;
buf += l;
diff --git a/qemu-thread.h b/qemu-thread.h
index 05fdaaf..fc9e17b 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -7,6 +7,11 @@
typedef struct QemuMutex QemuMutex;
typedef struct QemuCond QemuCond;
typedef struct QemuThread QemuThread;
+typedef enum {
+ DISPATCH_INIT = 0,
+ DISPATCH_MMIO,
+ DISPATCH_IO,
+} DispatchType;
#ifdef _WIN32
#include "qemu-thread-win32.h"
@@ -46,4 +51,6 @@ void qemu_thread_get_self(QemuThread *thread);
bool qemu_thread_is_self(QemuThread *thread);
void qemu_thread_exit(void *retval);
+void qemu_thread_set_dispatch_type(DispatchType type);
+void qemu_thread_reset_dispatch_type(void);
#endif
--
1.7.4.4
[Qemu-devel] [PATCH v6 5/8] memory: introduce local lock for address space
From: Liu Ping Fan
For those address spaces which want to be able out of big lock, they
will be protected by their own local.
Signed-off-by: Liu Ping Fan
---
memory.c |1 +
memory.h |3 +++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/memory.c b/memory.c
index 2f68d67..18425fd 100644
--- a/memory.c
+++ b/memory.c
@@ -1535,6 +1535,7 @@ void memory_listener_unregister(MemoryListener *listener)
void address_space_init(AddressSpace *as, MemoryRegion *root)
{
memory_region_transaction_begin();
+qemu_mutex_init(&as->lock);
as->root = root;
as->current_map = g_new(FlatView, 1);
flatview_init(as->current_map);
diff --git a/memory.h b/memory.h
index 79393f1..13a9e3e 100644
--- a/memory.h
+++ b/memory.h
@@ -22,6 +22,7 @@
#include "cpu-common.h"
#include "targphys.h"
#include "qemu-queue.h"
+#include "qemu-thread.h"
#include "iorange.h"
#include "ioport.h"
#include "int128.h"
@@ -164,6 +165,7 @@ typedef struct AddressSpace AddressSpace;
*/
struct AddressSpace {
/* All fields are private. */
+QemuMutex lock;
const char *name;
MemoryRegion *root;
struct FlatView *current_map;
@@ -801,6 +803,7 @@ void mtree_info(fprintf_function mon_printf, void *f);
*
* @as: an uninitialized #AddressSpace
* @root: a #MemoryRegion that routes addesses for the address space
+ * @lock: if true, the physmap protected by local lock, otherwise big lock
*/
void address_space_init(AddressSpace *as, MemoryRegion *root);
--
1.7.4.4
[Qemu-devel] [PATCH v6 4/8] pci: remove pci device from mem view when unplug
From: Liu Ping Fan
Signed-off-by: Liu Ping Fan
---
hw/acpi_piix4.c |2 +-
hw/pci.c| 13 -
hw/pci.h|1 +
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 15275cf..b45a016 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -306,7 +306,7 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned
slots)
if (pc->no_hotplug) {
slot_free = false;
} else {
-qdev_free(qdev);
+qdev_unplug_complete(qdev, NULL);
}
}
}
diff --git a/hw/pci.c b/hw/pci.c
index 7eeaac0..9ba589e 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -869,7 +869,6 @@ static int pci_unregister_device(DeviceState *dev)
PCIDevice *pci_dev = PCI_DEVICE(dev);
PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
-pci_unregister_io_regions(pci_dev);
pci_del_option_rom(pci_dev);
if (pc->exit) {
@@ -880,6 +879,17 @@ static int pci_unregister_device(DeviceState *dev)
return 0;
}
+static void pci_unmap_device(DeviceState *dev)
+{
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
+
+pci_unregister_io_regions(pci_dev);
+if (pc->unmap) {
+pc->unmap(pci_dev);
+}
+}
+
void pci_register_bar(PCIDevice *pci_dev, int region_num,
uint8_t type, MemoryRegion *memory)
{
@@ -2105,6 +2115,7 @@ static void pci_device_class_init(ObjectClass *klass,
void *data)
DeviceClass *k = DEVICE_CLASS(klass);
k->init = pci_qdev_init;
k->unplug = pci_unplug_device;
+k->unmap = pci_unmap_device;
k->exit = pci_unregister_device;
k->bus_type = TYPE_PCI_BUS;
k->props = pci_props;
diff --git a/hw/pci.h b/hw/pci.h
index 1f902f5..898cc5e 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -154,6 +154,7 @@ typedef struct PCIDeviceClass {
DeviceClass parent_class;
int (*init)(PCIDevice *dev);
+void (*unmap)(PCIDevice *dev);
PCIUnregisterFunc *exit;
PCIConfigReadFunc *config_read;
PCIConfigWriteFunc *config_write;
--
1.7.4.4
Re: [Qemu-devel] [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical address space
On 2012-11-05 03:42, Hao, Xudong wrote:
>> -Original Message-
>> From: Jan Kiszka [mailto:jan.kis...@web.de]
>> Sent: Sunday, November 04, 2012 8:55 PM
>> To: Hao, Xudong
>> Cc: qemu-devel@nongnu.org; a...@redhat.com; k...@vger.kernel.org
>> Subject: Re: [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical
>> address space
>>
>> On 2012-11-04 13:15, Hao, Xudong wrote:
-Original Message-
From: Jan Kiszka [mailto:jan.kis...@web.de]
Sent: Saturday, November 03, 2012 6:55 PM
To: Hao, Xudong
Cc: qemu-devel@nongnu.org; a...@redhat.com; k...@vger.kernel.org
Subject: Re: [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical
address space
On 2012-11-02 06:38, Xudong Hao wrote:
> For 64 bit processor, emulate 40 bits physical address if the host
> physical
> address space >= 40bits, else guest physical is same as host.
>
> Signed-off-by: Xudong Hao
> ---
> target-i386/cpu.c |5 -
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 423e009..3a78881 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1584,7 +1584,10 @@ void cpu_x86_cpuid(CPUX86State *env,
>> uint32_t
index, uint32_t count,
> if (env->cpuid_ext2_features & CPUID_EXT2_LM) {
> /* 64 bit processor */
> /* XXX: The physical address space is limited to 42 bits in exec.c. */
> -*eax = 0x3028; /* 48 bits virtual, 40 bits physical
>> */
> +/* XXX: 40 bits physical if host physical address space >= 40 bits */
> +uint32_t a, b, c, d;
> +host_cpuid(0x8008, 0, &a, &b, &c, &d);
> +*eax = a < 0x3028 ? a : 0x3028;
This variation will not only affect -cpu host, right? That can create
problems when migrating between hosts with different address widths, and
then we will need some control knob to adjust what it reported to the
guest.
>>>
>>> Oh, I did not consider migrating to different platform(addr widths).
>>> But I think the fixed value 40 bits may cause problem: in VT-d case, when a
>> host support GAW < 40 bits, and qemu emulate 40 bits guest physical address
>> space, will bring bug on:
>>>
>>> drivers/iommu/intel-iommu.c
>>> static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
>>> unsigned long pfn, int target_level)
>>> {
>>> int addr_width = agaw_to_width(domain->agaw) - VTD_PAGE_SHIFT;
>>> ...
>>> BUG_ON(!domain->pgd);
>>> BUG_ON(addr_width < BITS_PER_LONG && pfn >> addr_width);
>>>
>>
>> Does it mean that buggy or malicious user space can trigger a kernel
>> bug? Then this must be fixed of course.
>>
> Probably yes, when guest RAM is large enough or allocate MMIO to very high
> address.
...and those things are under user space control. If you have an idea
how to trigger this, please give it a try. This is an availability issue
as untrusted user space could bring down the whole system.
>
> Jan, I'm not familiar the migration, do you have interest to add the
> migration part fixing?
>
I'm not up to date with what is going on in the context of CPU feature
configuration, CC'ing folks who reworked this recently.
In any case, the general pattern is: make this configurable (=> CPU
feature flag) and then possibly also adjust it for compat QEMU machine
types.
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] memory: Reintroduce dirty flag to optimize changes on disabled regions
On 2012-11-04 20:21, Avi Kivity wrote: > On 11/04/2012 10:30 AM, Jan Kiszka wrote: >> From: Jan Kiszka >> >> Cirrus is triggering this, e.g. during Win2k boot: Changes only on >> disabled regions require no topology update when transaction depth drops >> to 0 again. > > 817dcc5368988b0 (pci: give each device its own address space) mad this > much worse by multiplying the number of address spaces. Each change is > now evaluated N+2 times, where N is the number of PCI devices. It also > causes a corresponding expansion in memory usage. I know... But this regression predates your changes, is already visible right after 02e2b95fb4. > > I want to address this by caching AddressSpaceDispatch trees with the > key being the contents of the FlatView for that address space. This > will drop the number of distinct trees to 2-4 (3 if some devices have > PCI_COMMAND_MASTER disabled, 4 if the PCI address space is different > from the cpu memory address space) but will fail if we make each address > space different (for example filtering out the device's own BARs). > > If this change also improves cpu usage sufficiently, then it will be > better than your patch, which doesn't recognize changes in an enabled > region inside a disabled or hidden region. True, though the question is how common such scenarios are. This one (cirrus with win2k) is already special. > In other words, your patch > fits the problem at hand but isn't general. On the other hand my > approach doesn't eliminate render_memory_region(), just the exec.c stuff > and listener updates. So we need to understand where the slowness comes > from. I would just like to have some even intermediate solution for 1.3. We can still make it more perfect later on if required. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] vmware_vga: Add back some info in local state partially reverting aa32b38c
On 2012-11-04 18:41, BALATON Zoltan wrote:
> Keep saving display surface parameters at init and using these cached
> values instead of getting them when needed. Not sure why this is
> needed (maybe due to the interaction with the vga device) but not
> doing this broke the Xorg vmware driver at least.
>
> Signed-off-by: BALATON Zoltan
Tested-by: Jan Kiszka
Thanks!
Jan
> ---
> hw/vmware_vga.c | 30 +-
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index 7c766fb..834588d 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -39,6 +39,8 @@ struct vmsvga_state_s {
> VGACommonState vga;
>
> int invalidated;
> +int depth;
> +int bypp;
> int enable;
> int config;
> struct {
> @@ -55,6 +57,9 @@ struct vmsvga_state_s {
> int new_height;
> uint32_t guest;
> uint32_t svgaid;
> +uint32_t wred;
> +uint32_t wgreen;
> +uint32_t wblue;
> int syncing;
>
> MemoryRegion fifo_ram;
> @@ -718,25 +723,25 @@ static uint32_t vmsvga_value_read(void *opaque,
> uint32_t address)
> return SVGA_MAX_HEIGHT;
>
> case SVGA_REG_DEPTH:
> -return ds_get_depth(s->vga.ds);
> +return s->depth;
>
> case SVGA_REG_BITS_PER_PIXEL:
> -return ds_get_bits_per_pixel(s->vga.ds);
> +return (s->depth + 7) & ~7;
>
> case SVGA_REG_PSEUDOCOLOR:
> return 0x0;
>
> case SVGA_REG_RED_MASK:
> -return ds_get_rmask(s->vga.ds);
> +return s->wred;
>
> case SVGA_REG_GREEN_MASK:
> -return ds_get_gmask(s->vga.ds);
> +return s->wgreen;
>
> case SVGA_REG_BLUE_MASK:
> -return ds_get_bmask(s->vga.ds);
> +return s->wblue;
>
> case SVGA_REG_BYTES_PER_LINE:
> -return ds_get_bytes_per_pixel(s->vga.ds) * s->new_width;
> +return s->bypp * s->new_width;
>
> case SVGA_REG_FB_START: {
> struct pci_vmsvga_state_s *pci_vmsvga
> @@ -801,7 +806,7 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t
> address)
> return s->cursor.on;
>
> case SVGA_REG_HOST_BITS_PER_PIXEL:
> -return ds_get_bits_per_pixel(s->vga.ds);
> +return (s->depth + 7) & ~7;
>
> case SVGA_REG_SCRATCH_SIZE:
> return s->scratch_size;
> @@ -864,7 +869,7 @@ static void vmsvga_value_write(void *opaque, uint32_t
> address, uint32_t value)
> break;
>
> case SVGA_REG_BITS_PER_PIXEL:
> -if (value != ds_get_bits_per_pixel(s->vga.ds)) {
> +if (value != s->depth) {
> printf("%s: Bad bits per pixel: %i bits\n", __func__, value);
> s->config = 0;
> }
> @@ -1084,7 +1089,7 @@ static const VMStateDescription
> vmstate_vmware_vga_internal = {
> .minimum_version_id_old = 0,
> .post_load = vmsvga_post_load,
> .fields = (VMStateField[]) {
> -VMSTATE_UNUSED(4), /* was depth */
> +VMSTATE_INT32_EQUAL(depth, struct vmsvga_state_s),
> VMSTATE_INT32(enable, struct vmsvga_state_s),
> VMSTATE_INT32(config, struct vmsvga_state_s),
> VMSTATE_INT32(cursor.id, struct vmsvga_state_s),
> @@ -1137,6 +1142,13 @@ static void vmsvga_init(struct vmsvga_state_s *s,
> vga_common_init(&s->vga);
> vga_init(&s->vga, address_space, io, true);
> vmstate_register(NULL, 0, &vmstate_vga_common, &s->vga);
> +/* Save some values here in case they are changed later.
> + * This is suspicious and needs more though why it is needed. */
> +s->depth = ds_get_bits_per_pixel(s->vga.ds);
> +s->bypp = ds_get_bytes_per_pixel(s->vga.ds);
> +s->wred = ds_get_rmask(s->vga.ds);
> +s->wgreen = ds_get_gmask(s->vga.ds);
> +s->wblue = ds_get_bmask(s->vga.ds);
> }
>
> static uint64_t vmsvga_io_read(void *opaque, hwaddr addr, unsigned size)
>
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v6 6/8] memory: make mmio dispatch able to be out of biglock
On 2012-11-05 06:38, Liu Ping Fan wrote: > From: Liu Ping Fan > > Without biglock, we try to protect the mr by increase refcnt. > If we can inc refcnt, go backward and resort to biglock. s/can/cannot/ - could create confusion. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v6 7/8] memory: introduce tls context to trace nested mmio request issue
On 2012-11-05 06:38, Liu Ping Fan wrote:
> From: Liu Ping Fan
>
> After breaking down big lock, nested MMIO request which not targeting
> at RAM can cause deadlock issue. Supposing the scene: dev_a,b with
> fine-grain locks lockA/B, then ABBA dealock issue can be triggered.
> We fix this by tracing and rejecting such request.
>
> Signed-off-by: Liu Ping Fan
> ---
> exec.c| 47 +++
> qemu-thread.h |7 +++
> 2 files changed, 54 insertions(+), 0 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index fa34ef9..1eb920d 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3442,6 +3442,48 @@ static bool
> address_space_section_lookup_ref(AddressSpace *as,
> return safe_ref;
> }
>
> +typedef struct ThreadContext {
> + DispatchType dispatch_type;
> + unsigned int mmio_req_pending;
> +} ThreadContext;
> +
> +static __thread ThreadContext thread_context = {
Again, you will have to work on qemu-tls.h and then use DEFINE_TLS. The
above is not portable.
> +.dispatch_type = DISPATCH_INIT,
> +.mmio_req_pending = 0
> +};
> +
> +void qemu_thread_set_dispatch_type(DispatchType type)
> +{
> +thread_context.dispatch_type = type;
> +}
> +
> +void qemu_thread_reset_dispatch_type(void)
> +{
> +thread_context.dispatch_type = DISPATCH_INIT;
> +}
> +
> +static void address_space_check_inc_req_pending(MemoryRegionSection *section)
> +{
> +bool nested = false;
> +
> +/* currently, only mmio out of big lock, and need this to avoid dead
> lock */
> +if (thread_context.dispatch_type == DISPATCH_MMIO) {
> +nested = ++thread_context.mmio_req_pending > 1 ? true : false;
> +/* To fix, will filter iommu case */
> +if (nested && !memory_region_is_ram(section->mr)) {
> +fprintf(stderr, "mmio: nested target not RAM is not support");
> +abort();
> +}
> +}
This should already take PIO into account, thus all scenarios: If we are
dispatching MMIO or PIO, reject any further requests that are not
targeting RAM.
I don't think we need mmio_req_pending for this. We are not interested
in differentiating between MMIO and PIO, both will be problematic. We
just store the information if a request is going on in the TLS variable
here, not before entering cpu_physical_memory_xxx. And then we can
simply bail out if another non-RAM request is arriving, the nesting
level will never be >1.
And with bailing out I mean warn once + ignore request, not abort().
This would be a needless guest triggerable VM termination.
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v6 0/8] push mmio dispatch out of big lock
On 2012-11-05 06:38, Liu Ping Fan wrote: > From: Liu Ping Fan > > v1: > https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03312.html > > v2: > http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01275.html > > v3: > http://lists.nongnu.org/archive/html/qemu-devel/2012-09/msg01474.html > > v4: > http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg03857.html > > v5: > https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg04867.html > > changes v5->v6: > Apply fine-grain lock for all address space. > Introduce separated interface to allow mmio dispatcher called with/without > big lock. > > Liu Ping Fan (8): > atomic: introduce atomic operations > qom: apply atomic on object's refcount > hotplug: introduce qdev_unplug_complete() to remove device from views > pci: remove pci device from mem view when unplug > memory: introduce local lock for address space > memory: make mmio dispatch able to be out of biglock > memory: introduce tls context to trace nested mmio request issue > vcpu: push mmio dispatcher out of big lock > > cpu-common.h |3 + > docs/memory.txt |4 + > exec.c| 219 > + > hw/acpi_piix4.c |2 +- > hw/pci.c | 13 +++- > hw/pci.h |1 + > hw/qdev.c | 26 ++ > hw/qdev.h |3 +- > include/qemu/atomic.h | 63 ++ > include/qemu/object.h |3 +- > kvm-all.c |6 +- > memory-internal.h |1 + > memory.c |1 + > memory.h |5 + > qemu-thread.h |7 ++ > qom/object.c | 11 +-- > 16 files changed, 340 insertions(+), 28 deletions(-) > create mode 100644 include/qemu/atomic.h > Very good! My feeling is we are getting closer. There are some minor style issues I'm not yet commenting on. We can go through this once everyone is happy with the design. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] target-mips: use ULL for 64 bit constants
On Sat, Nov 03, 2012 at 06:48:35PM +, Blue Swirl wrote:
> Fix build on a 32 bit host:
> CCmips-softmmu/target-mips/dsp_helper.o
> /src/qemu/target-mips/dsp_helper.c: In function 'helper_dextr_rs_w':
> /src/qemu/target-mips/dsp_helper.c:3556: error: integer constant is too large
> for 'long' type
> /src/qemu/target-mips/dsp_helper.c: In function 'helper_extr_s_h':
> /src/qemu/target-mips/dsp_helper.c:3656: error: integer constant is too large
> for 'long' type
>
> Signed-off-by: Blue Swirl
> ---
> target-mips/dsp_helper.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index b59133e..e7949c2 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -3553,7 +3553,7 @@ target_ulong helper_dextr_rs_w(target_ulong ac,
> target_ulong shift,
> if (temp128 == 0) {
> temp[0] = 0x0;
> } else {
> -temp[0] = 0x01;
> +temp[0] = 0x01ULL;
> }
> set_DSPControl_overflow_flag(1, 23, env);
> }
> @@ -3653,7 +3653,7 @@ target_ulong helper_extr_s_h(target_ulong ac,
> target_ulong shift,
> if (temp > (int64_t)0x7FFF) {
> temp = 0x7FFF;
> set_DSPControl_overflow_flag(1, 23, env);
> -} else if (temp < (int64_t)0x8000) {
> +} else if (temp < (int64_t)0x8000ULL) {
> temp = 0x8000;
> set_DSPControl_overflow_flag(1, 23, env);
> }
Thanks, applied.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] target-mips: Fix compiler warnings caused by very large constants
On Sun, Nov 04, 2012 at 09:29:35PM +0100, Stefan Weil wrote:
> Those constants are larger than 32 bits and need a suffix to avoid
> warnings from some versions of gcc.
>
> Signed-off-by: Stefan Weil
> ---
> target-mips/dsp_helper.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index b59133e..2ab9956 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -3553,7 +3553,7 @@ target_ulong helper_dextr_rs_w(target_ulong ac,
> target_ulong shift,
> if (temp128 == 0) {
> temp[0] = 0x0;
> } else {
> -temp[0] = 0x01;
> +temp[0] = 0x01ULL;
> }
> set_DSPControl_overflow_flag(1, 23, env);
> }
> @@ -3653,7 +3653,7 @@ target_ulong helper_extr_s_h(target_ulong ac,
> target_ulong shift,
> if (temp > (int64_t)0x7FFF) {
> temp = 0x7FFF;
> set_DSPControl_overflow_flag(1, 23, env);
> -} else if (temp < (int64_t)0x8000) {
> +} else if (temp < (int64_t)0x8000LL) {
> temp = 0x8000;
> set_DSPControl_overflow_flag(1, 23, env);
> }
Blue Swirl proposed the same patch a bit earlier then you, and I have
just applied it.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] [PATCH v3 for 1.3 0/3] tcg/arm: misc fixes
This patch series fixes the TCG arm backend for the MIPS target, as well as for big endian targets when not using the ARMv6+ instructions set. The corresponding patches are candidate for a stable release. -- Changes v2 -> v3: - patch 1: - The new code allow up to 20 bits to be loaded (and not 24 bits). Change the comments and the assert accordingly. Changes v1 -> v2: - patch 1: - added an assert to make sure the TLB offset fits within 24 bits - added an assert to make sure both registers are different in ldr_wb - patches 4 and 5 (optimizations) have been dropped and will be resubmitted again (when I can find some time to work on them). Aurelien Jarno (3): tcg/arm: fix TLB access in qemu-ld/st ops tcg/arm: fix cross-endian qemu_st16 target-openrisc: remove conflicting definitions from cpu.h target-openrisc/cpu.h | 18 - tcg/arm/tcg-target.c | 98 ++--- 2 files changed, 60 insertions(+), 56 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH v3 for 1.3 1/3] tcg/arm: fix TLB access in qemu-ld/st ops
The TCG arm backend considers likely that the offset to the TLB
entries does not exceed 12 bits for mem_index = 0. In practice this is
not true for at least the MIPS target.
The current patch fixes that by loading the bits 23-12 with a separate
instruction, and using loads with address writeback, independently of
the value of mem_idx. In total this allow a 24-bit offset, which is a
lot more than needed.
Cc: Andrzej Zaborowski
Cc: Peter Maydell
Cc: qemu-sta...@nongnu.org
Signed-off-by: Aurelien Jarno
---
tcg/arm/tcg-target.c | 78 +++---
1 file changed, 42 insertions(+), 36 deletions(-)
diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index e790bf0..9550102 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -639,6 +639,22 @@ static inline void tcg_out_ld32_12(TCGContext *s, int cond,
(rn << 16) | (rd << 12) | ((-im) & 0xfff));
}
+/* Offset pre-increment with base writeback. */
+static inline void tcg_out_ld32_12wb(TCGContext *s, int cond,
+ int rd, int rn, tcg_target_long im)
+{
+/* ldr with writeback and both register equals is UNPREDICTABLE */
+assert(rd != rn);
+
+if (im >= 0) {
+tcg_out32(s, (cond << 28) | 0x05b0 |
+(rn << 16) | (rd << 12) | (im & 0xfff));
+} else {
+tcg_out32(s, (cond << 28) | 0x0530 |
+(rn << 16) | (rd << 12) | ((-im) & 0xfff));
+}
+}
+
static inline void tcg_out_st32_12(TCGContext *s, int cond,
int rd, int rn, tcg_target_long im)
{
@@ -1071,7 +1087,7 @@ static inline void tcg_out_qemu_ld(TCGContext *s, const
TCGArg *args, int opc)
{
int addr_reg, data_reg, data_reg2, bswap;
#ifdef CONFIG_SOFTMMU
-int mem_index, s_bits;
+int mem_index, s_bits, tlb_offset;
TCGReg argreg;
# if TARGET_LONG_BITS == 64
int addr_reg2;
@@ -,19 +1127,15 @@ static inline void tcg_out_qemu_ld(TCGContext *s, const
TCGArg *args, int opc)
TCG_REG_R0, TCG_REG_R8, CPU_TLB_SIZE - 1);
tcg_out_dat_reg(s, COND_AL, ARITH_ADD, TCG_REG_R0, TCG_AREG0,
TCG_REG_R0, SHIFT_IMM_LSL(CPU_TLB_ENTRY_BITS));
-/* In the
- * ldr r1 [r0, #(offsetof(CPUArchState,
tlb_table[mem_index][0].addr_read))]
- * below, the offset is likely to exceed 12 bits if mem_index != 0 and
- * not exceed otherwise, so use an
- * add r0, r0, #(mem_index * sizeof *CPUArchState.tlb_table)
- * before.
- */
-if (mem_index)
+/* We assume that the offset is contained within 20 bits. */
+tlb_offset = offsetof(CPUArchState, tlb_table[mem_index][0].addr_read);
+assert(tlb_offset & ~0xf == 0);
+if (tlb_offset > 0xfff) {
tcg_out_dat_imm(s, COND_AL, ARITH_ADD, TCG_REG_R0, TCG_REG_R0,
-(mem_index << (TLB_SHIFT & 1)) |
-((16 - (TLB_SHIFT >> 1)) << 8));
-tcg_out_ld32_12(s, COND_AL, TCG_REG_R1, TCG_REG_R0,
-offsetof(CPUArchState, tlb_table[0][0].addr_read));
+0xa00 | (tlb_offset >> 12));
+tlb_offset &= 0xfff;
+}
+tcg_out_ld32_12wb(s, COND_AL, TCG_REG_R1, TCG_REG_R0, tlb_offset);
tcg_out_dat_reg(s, COND_AL, ARITH_CMP, 0, TCG_REG_R1,
TCG_REG_R8, SHIFT_IMM_LSL(TARGET_PAGE_BITS));
/* Check alignment. */
@@ -1131,15 +1143,14 @@ static inline void tcg_out_qemu_ld(TCGContext *s, const
TCGArg *args, int opc)
tcg_out_dat_imm(s, COND_EQ, ARITH_TST,
0, addr_reg, (1 << s_bits) - 1);
# if TARGET_LONG_BITS == 64
-/* XXX: possibly we could use a block data load or writeback in
- * the first access. */
-tcg_out_ld32_12(s, COND_EQ, TCG_REG_R1, TCG_REG_R0,
-offsetof(CPUArchState, tlb_table[0][0].addr_read) + 4);
+/* XXX: possibly we could use a block data load in the first access. */
+tcg_out_ld32_12(s, COND_EQ, TCG_REG_R1, TCG_REG_R0, 4);
tcg_out_dat_reg(s, COND_EQ, ARITH_CMP, 0,
TCG_REG_R1, addr_reg2, SHIFT_IMM_LSL(0));
# endif
tcg_out_ld32_12(s, COND_EQ, TCG_REG_R1, TCG_REG_R0,
-offsetof(CPUArchState, tlb_table[0][0].addend));
+offsetof(CPUTLBEntry, addend)
+- offsetof(CPUTLBEntry, addr_read));
switch (opc) {
case 0:
@@ -1288,7 +1299,7 @@ static inline void tcg_out_qemu_st(TCGContext *s, const
TCGArg *args, int opc)
{
int addr_reg, data_reg, data_reg2, bswap;
#ifdef CONFIG_SOFTMMU
-int mem_index, s_bits;
+int mem_index, s_bits, tlb_offset;
TCGReg argreg;
# if TARGET_LONG_BITS == 64
int addr_reg2;
@@ -1325,19 +1336,15 @@ static inline void tcg_out_qemu_st(TCGContext *s, const
TCGArg *args, int opc)
TCG_REG_R0, TCG_REG_R8, CPU_TLB_SIZE - 1);
tcg_out_dat_reg(s, COND_AL, ARITH_ADD, TCG_REG_R0,
TCG_AREG0, TCG_RE
[Qemu-devel] [PATCH v3 for 1.3 3/3] target-openrisc: remove conflicting definitions from cpu.h
On an ARM host, the registers definitions from cpu.h clash
with /usr/include/sys/ucontext.h. As there are unused, just remove
them.
Cc: Jia Liu
Cc: qemu-sta...@nongnu.org
Reviewed-by: Peter Maydell
Signed-off-by: Aurelien Jarno
---
target-openrisc/cpu.h | 18 --
1 file changed, 18 deletions(-)
diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
index d42ffb0..ebb5ad3 100644
--- a/target-openrisc/cpu.h
+++ b/target-openrisc/cpu.h
@@ -89,24 +89,6 @@ enum {
/* Interrupt */
#define NR_IRQS 32
-/* Registers */
-enum {
-R0 = 0, R1, R2, R3, R4, R5, R6, R7, R8, R9, R10,
-R11, R12, R13, R14, R15, R16, R17, R18, R19, R20,
-R21, R22, R23, R24, R25, R26, R27, R28, R29, R30,
-R31
-};
-
-/* Register aliases */
-enum {
-R_ZERO = R0,
-R_SP = R1,
-R_FP = R2,
-R_LR = R9,
-R_RV = R11,
-R_RVH = R12
-};
-
/* Unit presece register */
enum {
UPR_UP = (1 << 0),
--
1.7.10.4
[Qemu-devel] [PATCH v3 for 1.3 2/3] tcg/arm: fix cross-endian qemu_st16
The bswap16 TCG opcode assumes that the high bytes of the temp equal
to 0 before calling it. The ARM backend implementation takes this
assumption to slightly optimize the generated code.
The same implementation is called for implementing the cross-endian
qemu_st16 opcode, where this assumption is not true anymore. One way to
fix that would be to zero the high bytes before calling it. Given the
store instruction just ignore them, it is possible to provide a slightly
more optimized version. With ARMv6+ the rev16 instruction does the work
correctly. For lower ARM versions the patch provides a version which
behaves correctly with non-zero high bytes, but fill them with junk.
Cc: Andrzej Zaborowski
Cc: Peter Maydell
Cc: qemu-sta...@nongnu.org
Reviewed-by: Peter Maydell
Signed-off-by: Aurelien Jarno
---
tcg/arm/tcg-target.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index 9550102..47612fe 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -611,6 +611,22 @@ static inline void tcg_out_bswap16(TCGContext *s, int
cond, int rd, int rn)
}
}
+/* swap the two low bytes assuming that the two high input bytes and the
+ two high output bit can hold any value. */
+static inline void tcg_out_bswap16st(TCGContext *s, int cond, int rd, int rn)
+{
+if (use_armv6_instructions) {
+/* rev16 */
+tcg_out32(s, 0x06bf0fb0 | (cond << 28) | (rd << 12) | rn);
+} else {
+tcg_out_dat_reg(s, cond, ARITH_MOV,
+TCG_REG_R8, 0, rn, SHIFT_IMM_LSR(8));
+tcg_out_dat_imm(s, cond, ARITH_AND, TCG_REG_R8, TCG_REG_R8, 0xff);
+tcg_out_dat_reg(s, cond, ARITH_ORR,
+rd, TCG_REG_R8, rn, SHIFT_IMM_LSL(8));
+}
+}
+
static inline void tcg_out_bswap32(TCGContext *s, int cond, int rd, int rn)
{
if (use_armv6_instructions) {
@@ -1367,7 +1383,7 @@ static inline void tcg_out_qemu_st(TCGContext *s, const
TCGArg *args, int opc)
break;
case 1:
if (bswap) {
-tcg_out_bswap16(s, COND_EQ, TCG_REG_R0, data_reg);
+tcg_out_bswap16st(s, COND_EQ, TCG_REG_R0, data_reg);
tcg_out_st16_r(s, COND_EQ, TCG_REG_R0, addr_reg, TCG_REG_R1);
} else {
tcg_out_st16_r(s, COND_EQ, data_reg, addr_reg, TCG_REG_R1);
@@ -1453,7 +1469,7 @@ static inline void tcg_out_qemu_st(TCGContext *s, const
TCGArg *args, int opc)
break;
case 1:
if (bswap) {
-tcg_out_bswap16(s, COND_AL, TCG_REG_R0, data_reg);
+tcg_out_bswap16st(s, COND_AL, TCG_REG_R0, data_reg);
tcg_out_st16_8(s, COND_AL, TCG_REG_R0, addr_reg, 0);
} else {
tcg_out_st16_8(s, COND_AL, data_reg, addr_reg, 0);
--
1.7.10.4
Re: [Qemu-devel] [PATCH v3] correct error message qemu-img reported
在 2012-11-02五的 08:16 +0100，Stefan Hajnoczi写道：
> On Fri, Nov 2, 2012 at 6:11 AM, liguang wrote:
> > diff --git a/qemu-img.c b/qemu-img.c
> > index b41e670..d4ea800 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -337,10 +337,15 @@ static int img_create(int argc, char **argv)
> >
> > /* Get image size, if specified */
> > if (optind < argc) {
> > -int64_t sval;
> > +int64_t sval = 0;
>
> sval is assigned below so there is no need for this change.
>
> > char *end;
> > sval = strtosz_suffix(argv[optind++], &end, STRTOSZ_DEFSUFFIX_B);
> > -if (sval < 0 || *end) {
> > +if (sval < 0) {
> > +error_report("image size is too large!");
>
> I suggest being specific about the upper limit so the user knows which
> values are valid:
> "Image size must be less than 8 exabytes!"
>
> Stefan
>
OK, will fix.
--
liguanglig.f...@cn.fujitsu.com
FNST linux kernel team
Re: [Qemu-devel] [PATCH 2/2] qemu-kvm/pci-assign: 64 bits bar emulation
> -Original Message-
> From: Blue Swirl [mailto:blauwir...@gmail.com]
> Sent: Saturday, November 03, 2012 6:44 PM
> To: Hao, Xudong
> Cc: qemu-devel@nongnu.org; a...@redhat.com; k...@vger.kernel.org
> Subject: Re: [Qemu-devel] [PATCH 2/2] qemu-kvm/pci-assign: 64 bits bar
> emulation
>
> On Fri, Nov 2, 2012 at 5:38 AM, Xudong Hao wrote:
> > Enable 64 bits bar emulation.
> >
> > Signed-off-by: Xudong Hao
> > ---
> > hw/kvm/pci-assign.c | 18 --
> > 1 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
> > index 05b93d9..f1f8d1e 100644
> > --- a/hw/kvm/pci-assign.c
> > +++ b/hw/kvm/pci-assign.c
> > @@ -46,6 +46,7 @@
> > #define IORESOURCE_IRQ 0x0400
> > #define IORESOURCE_DMA 0x0800
> > #define IORESOURCE_PREFETCH 0x2000 /* No side effects */
> > +#define IORESOURCE_MEM_64 0x0010
> >
> > //#define DEVICE_ASSIGNMENT_DEBUG
> >
> > @@ -442,9 +443,13 @@ static int assigned_dev_register_regions(PCIRegion
> *io_regions,
> >
> > /* handle memory io regions */
> > if (cur_region->type & IORESOURCE_MEM) {
> > -int t = cur_region->type & IORESOURCE_PREFETCH
> > -? PCI_BASE_ADDRESS_MEM_PREFETCH
> > -: PCI_BASE_ADDRESS_SPACE_MEMORY;
> > +int t = PCI_BASE_ADDRESS_SPACE_MEMORY;
> > +if (cur_region->type & IORESOURCE_PREFETCH) {
> > +t |= PCI_BASE_ADDRESS_MEM_PREFETCH;
> > +}
> > +if (cur_region->type & IORESOURCE_MEM_64) {
> > +t |= PCI_BASE_ADDRESS_MEM_TYPE_64;
> > +}
> >
> > /* map physical memory */
> > pci_dev->v_addrs[i].u.r_virtbase = mmap(NULL,
> cur_region->size,
> > @@ -468,8 +473,8 @@ static int assigned_dev_register_regions(PCIRegion
> *io_regions,
> > (cur_region->base_addr & 0xFFF);
> >
> > if (cur_region->size & 0xFFF) {
> > -error_report("PCI region %d at address 0x%" PRIx64 "
> has "
> > - "size 0x%" PRIx64 ", which is not a
> multiple of "
> > +error_report("PCI region %d at address 0lx%" PRIx64 "
> has "
> > + "size 0lx%" PRIx64 ", which is not a
> multiple of "
>
> Adding 'l' to '0x' prefix does not make sense.
>
Thanks review it, changes to:
+error_report("PCI region %d at address 0x%016" PRIx64 " has "
+ "size 0x%016" PRIx64 ", which is not a multiple
of "
[Qemu-devel] [PATCH v4] correct error message qemu-img reported
qemu-img will complain when qcow or qcow2
size overflow for 64 bits, report the right
message in this condition.
Signed-off-by: liguang
---
qemu-img.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index b41e670..d9434ad 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -340,7 +340,12 @@ static int img_create(int argc, char **argv)
int64_t sval;
char *end;
sval = strtosz_suffix(argv[optind++], &end, STRTOSZ_DEFSUFFIX_B);
-if (sval < 0 || *end) {
+if (sval < 0) {
+error_report("Image size must be less than 8 exabytes!");
+ret = -1;
+goto out;
+}
+if (*end) {
error_report("Invalid image size specified! You may use k, M, G or
"
"T suffixes for ");
error_report("kilobytes, megabytes, gigabytes and terabytes.");
--
1.7.1
Re: [Qemu-devel] [PATCH trace] Avoid all systemtap reserved words
On Fri, Nov 02, 2012 at 12:00:53PM +, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > Over time various systemtap reserved words have been blacklisted > in the trace backend generator. The list is not complete though, > so there is continued risk of problems in the future. Preempt > such problems by specifying the full list of systemtap keywords > listed in its parser as identified here: > > http://sourceware.org/ml/systemtap/2012-q4/msg00157.html > > Signed-off-by: Daniel P. Berrange > --- > scripts/tracetool/backend/dtrace.py | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) Thanks, applied to the tracing tree: https://github.com/stefanha/qemu/commits/tracing Stefan
Re: [Qemu-devel] [PATCH] trace: allow disabling events in events file
On Fri, Oct 26, 2012 at 01:46:34PM +0200, Gerd Hoffmann wrote: > Disable trace events prefixed with a '-'. Useful > to enable a group of tracepoints with exceptions, > like this: > > usb_xhci_port_* > -usb_xhci_port_read > > which will enable all xhci port tracepoints except reads. > > Signed-off-by: Gerd Hoffmann > --- > trace/control.c |9 - > 1 files changed, 8 insertions(+), 1 deletions(-) Sorry for the late reply. Looks good I will send a patch to update the documentation. Applied to the tracing tree: http://github.com/stefanha/qemu/commits/tracing Stefan
[Qemu-devel] [PATCH] trace: document '-' syntax for disabling events
Signed-off-by: Stefan Hajnoczi --- docs/tracing.txt | 4 1 file changed, 4 insertions(+) diff --git a/docs/tracing.txt b/docs/tracing.txt index c541133..7901409 100644 --- a/docs/tracing.txt +++ b/docs/tracing.txt @@ -139,6 +139,10 @@ having a common prefix in a batch. For example, virtio-blk trace events could be enabled using: trace-event virtio_blk_* on +If a line in the "-trace events=" file begins with a '-', the trace event +will be disabled instead of enabled. This is useful when a wildcard was used +to enable an entire family of events but one noisy event needs to be disabled. + == Trace backends == The "tracetool" script automates tedious trace event code generation and also -- 1.7.12.1
Re: [Qemu-devel] [PATCH v2 1/5] compiler: support Darwin weak references
On Fri, Nov 2, 2012 at 10:43 PM, Paolo Bonzini wrote:
> Weakrefs only tell you if the symbol was defined elsewhere, so you
> need a further check at runtime to pick the default definition
> when needed.
>
> This could be automated by the compiler, but it does not do it.
>
> Signed-off-by: Paolo Bonzini
> ---
> v1->v2: add unused attribute
>
> compiler.h | 9 -
> osdep.c| 56
> oslib-win32.c | 12 +++-
> qemu-sockets.c | 40 ++--
> qmp.c | 2 ++
> 5 file modificati, 71 inserzioni(+), 48 rimozioni(-)
>
> diff --git a/compiler.h b/compiler.h
> index 58865d6..55d7d74 100644
> --- a/compiler.h
> +++ b/compiler.h
> @@ -50,8 +50,15 @@
> # define __printf__ __gnu_printf__
> # endif
> # endif
> -# define QEMU_WEAK_ALIAS(newname, oldname) \
> +# if defined(__APPLE__)
> +# define QEMU_WEAK_ALIAS(newname, oldname) \
> +static typeof(oldname) weak_##newname __attribute__((unused,
> weakref(#oldname)))
> +# define QEMU_WEAK_REF(newname, oldname) (weak_##newname ? weak_##newname :
> oldname)
> +# else
> +# define QEMU_WEAK_ALIAS(newname, oldname) \
> typeof(oldname) newname __attribute__((weak, alias (#oldname)))
> +# define QEMU_WEAK_REF(newname, oldname) newname
> +# endif
> #else
> #define GCC_ATTR /**/
> #define GCC_FMT_ATTR(n, m)
> diff --git a/osdep.c b/osdep.c
> index a87d4a4..2f7a491 100644
> --- a/osdep.c
> +++ b/osdep.c
> @@ -54,6 +54,38 @@ static bool fips_enabled = false;
>
> static const char *qemu_version = QEMU_VERSION;
>
> +static int default_fdset_get_fd(int64_t fdset_id, int flags)
> +{
> +return -1;
> +}
> +QEMU_WEAK_ALIAS(monitor_fdset_get_fd, default_fdset_get_fd);
> +#define monitor_fdset_get_fd \
> +QEMU_WEAK_REF(monitor_fdset_get_fd, default_fdset_get_fd)
> +
> +static int default_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
> +{
> +return -1;
> +}
> +QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_add, default_fdset_dup_fd_add);
> +#define monitor_fdset_dup_fd_add \
> +QEMU_WEAK_REF(monitor_fdset_dup_fd_add, default_fdset_dup_fd_add)
> +
> +static int default_fdset_dup_fd_remove(int dup_fd)
> +{
> +return -1;
> +}
> +QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_remove);
> +#define monitor_fdset_dup_fd_remove \
> +QEMU_WEAK_REF(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_remove)
> +
> +static int default_fdset_dup_fd_find(int dup_fd)
> +{
> +return -1;
> +}
> +QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_find, default_fdset_dup_fd_find);
> +#define monitor_fdset_dup_fd_find \
> +QEMU_WEAK_REF(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_find)
Should here be QEMU_WEAK_REF(monitor_fdset_dup_fd_find,
default_fdset_dup_fd_find)?
Another issue: Some weird codes were generated on MinGW + gcc 4.5.1:
244 fdset_id = monitor_fdset_dup_fd_find(fd);
(gdb) x/3i $pc
=> 0x553e9e :mov-0x1c(%ebp),%eax
0x553ea1 :mov%eax,(%esp)
0x553ea4 :
call 0x637bb6
It's called directly into the middle of monitor_fdset_dup_fd_find_remove.
> +
> int socket_set_cork(int fd, int v)
> {
> #if defined(SOL_TCP) && defined(TCP_CORK)
> @@ -400,27 +432,3 @@ bool fips_get_state(void)
> return fips_enabled;
> }
>
> -
> -static int default_fdset_get_fd(int64_t fdset_id, int flags)
> -{
> -return -1;
> -}
> -QEMU_WEAK_ALIAS(monitor_fdset_get_fd, default_fdset_get_fd);
> -
> -static int default_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
> -{
> -return -1;
> -}
> -QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_add, default_fdset_dup_fd_add);
> -
> -static int default_fdset_dup_fd_remove(int dup_fd)
> -{
> -return -1;
> -}
> -QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_remove);
> -
> -static int default_fdset_dup_fd_find(int dup_fd)
> -{
> -return -1;
> -}
> -QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_find, default_fdset_dup_fd_find);
> diff --git a/oslib-win32.c b/oslib-win32.c
> index 9ca83df..326a2bd 100644
> --- a/oslib-win32.c
> +++ b/oslib-win32.c
> @@ -32,6 +32,13 @@
> #include "trace.h"
> #include "qemu_socket.h"
>
> +static void default_qemu_fd_register(int fd)
> +{
> +}
> +QEMU_WEAK_ALIAS(qemu_fd_register, default_qemu_fd_register);
> +#define qemu_fd_register \
> +QEMU_WEAK_REF(qemu_fd_register, default_qemu_fd_register)
> +
> void *qemu_oom_check(void *ptr)
> {
> if (ptr == NULL) {
> @@ -150,8 +157,3 @@ int qemu_get_thread_id(void)
> {
> return GetCurrentThreadId();
> }
> -
> -static void default_qemu_fd_register(int fd)
> -{
> -}
> -QEMU_WEAK_ALIAS(qemu_fd_register, default_qemu_fd_register);
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index f2a6371..abcd791 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -61,6 +61,28 @@ static QemuOptsList dummy_opts = {
> },
> };
>
> +static int default_monitor_get_fd(Monitor *mon, const char *name, Error
> **errp)
> +{
> +error_setg(errp, "only QEMU support
