Re: [Qemu-devel] [PATCH] floppy: Parametrize ISA base and IRQ
Andreas Färber writes: > Am 28.01.2011 um 13:41 schrieb Markus Armbruster: > >> Andreas Färber writes: >> >>> From: Hervé Poussineau >>> >>> v1: >>> * Rebased. >> >> Patch versioning goes below the --- line, so it doesn't end up in git. > > I disagree: I always put it into the commit message, so that we can > see which version got committed and so that those that contributed > ideas get credited. When patch history contains bits that should be preserved for posterity, I recommend working them into the commit message proper. [...]
Re: [Qemu-devel] [PATCHv2 1/2] Add bootindex handling into usb storage device.
Both patches look fine to me.
Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.l...@st.com wrote:
> From: Christophe Lyon
>
> Handle corner cases where the addition of the rounding constant could
> cause overflows.
After applying this patch, I get the following gcc warning:
CCtranslate.o
cc1: warnings being treated as errors
qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in
this function
make: *** [translate.o] Error 1
> Signed-off-by: Christophe Lyon
> ---
> target-arm/neon_helper.c | 61 ++---
> target-arm/translate.c | 17 ++--
> 2 files changed, 65 insertions(+), 13 deletions(-)
>
> diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
> index bf29bbe..5971275 100644
> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -540,6 +540,9 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t
> shiftop)
> return val;
> }
>
> +/* The addition of the rounding constant may overflow, so we use an
> + * intermediate 64 bits accumulator, which is really needed only when
> + * dealing with 32 bits input values. */
> #define NEON_FN(dest, src1, src2) do { \
> int8_t tmp; \
> tmp = (int8_t)src2; \
> @@ -548,11 +551,12 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t
> shiftop)
> } else if (tmp < -(ssize_t)sizeof(src1) * 8) { \
> dest = src1 >> (sizeof(src1) * 8 - 1); \
> } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
> -dest = src1 >> (tmp - 1); \
> +dest = src1 >> (-tmp - 1); \
> dest++; \
> dest >>= 1; \
> } else if (tmp < 0) { \
> -dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
> +int64_t big_dest = ((int64_t)src1 + (1 << (-1 - tmp))); \
> +dest = big_dest >> -tmp; \
> } else { \
> dest = src1 << tmp; \
> }} while (0)
> @@ -561,6 +565,8 @@ NEON_VOP(rshl_s16, neon_s16, 2)
> NEON_VOP(rshl_s32, neon_s32, 1)
> #undef NEON_FN
>
> +/* Handling addition overflow with 64 bits inputs values is more
> + * tricky than with 32 bits values. */
> uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
> {
> int8_t shift = (int8_t)shiftop;
> @@ -569,18 +575,37 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t
> shiftop)
> val = 0;
> } else if (shift < -64) {
> val >>= 63;
> -} else if (shift == -63) {
> +} else if (shift == -64) {
> val >>= 63;
> val++;
> val >>= 1;
> } else if (shift < 0) {
> -val = (val + ((int64_t)1 << (-1 - shift))) >> -shift;
> +int64_t round = (int64_t)1 << (-1 - shift);
> +/* Reduce the range as long as the addition overflows. It's
> + * sufficient to check if (val+round) is < 0 and val > 0
> + * because round is > 0. */
> +while ((val > 0) && ((val + round) < 0) && round > 1) {
> +shift++;
> +round >>= 1;
> +val >>= 1;
> +}
> +if ((val > 0) && (val + round) < 0) {
> +/* If addition still overflows at this point, it means
> + * that round==1, thus shift==-1, and also that
> + * val==0x7FFF. */
> +val = 0x4000LL;
> +} else {
> +val = (val + round) >> -shift;
> +}
> } else {
> val <<= shift;
> }
> return val;
> }
>
> +/* The addition of the rounding constant may overflow, so we use an
> + * intermediate 64 bits accumulator, which is really needed only when
> + * dealing with 32 bits input values. */
> #define NEON_FN(dest, src1, src2) do { \
> int8_t tmp; \
> tmp = (int8_t)src2; \
> @@ -588,9 +613,10 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t
> shiftop)
> tmp < -(ssize_t)sizeof(src1) * 8) { \
> dest = 0; \
> } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
> -dest = src1 >> (tmp - 1); \
> +dest = src1 >> (-tmp - 1); \
> } else if (tmp < 0) { \
> -dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
> +uint64_t big_dest = ((uint64_t)src1 + (1 << (-1 - tmp))); \
> +dest = big_dest >> -tmp; \
> } else { \
> dest = src1 << tmp; \
> }} while (0)
> @@ -602,14 +628,29 @@ NEON_VOP(rshl_u32, neon_u32, 1)
> uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
> {
> int8_t shift = (uint8_t)shiftop;
> -if (shift >= 64 || shift < 64) {
> +if (shift >= 64 || shift < -64) {
> val = 0;
> } else if (shift == -64) {
> /* Rounding a 1-bit result just preserves that bit. */
> val >>= 63;
> -} if (shift < 0) {
> -val = (val + ((uint64_t)1 << (-1 - shift))) >> -shift;
> -val >>= -shift;
> +} else if (shift < 0) {
> +uint64_t round = (uint64_t)1 << (-1 - shift);
> +/* Reduce the range as long as the addition overflo
Re: [Qemu-devel] [PATCH 5/7] tcg-i386: Implement deposit operation.
Hi, On Tue, Jan 25, 2011 at 01:27:49PM +0100, Edgar E. Iglesias wrote: > On Mon, Jan 10, 2011 at 07:23:46PM -0800, Richard Henderson wrote: > > Special case deposits that are implementable with byte and word stores. > > Otherwise implement with double-word shift plus rotates. > > > > Expose tcg_scratch_alloc to the backend for allocation of scratch registers. > > > > Signed-off-by: Richard Henderson > > Hi, > > I've tested this patch a bit and got mixed results. I tested with patched > CRIS and MicroBlaze translators. The patch works OK (it doesn't break > anything) for the usecases I had but I saw a bit of a slowdown with > MicroBlaze (compare to not using deposit at all). > This week-end I have tested it emulating an x86-64 machine on x86-64, with all the patch series applied. I have measured the boot time from the bootloader up to the graphical environment of a Debian installation I used -snapshot to make sure the host hard-drive is not introducing any noise in the measurement (so that the whole image is in the host cache), and did the measurement 10 times. The machine is a Core 2 Q9650, nothing else was running on the machine except the few standard daemons. I have found that the boot time is roughly 1.8% faster with the patch series applied. It's undoubtedly an improvement, but still close to the measurement noise. This is a bit disappointing... -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] Re: [PATCH] qcow2-refcount: remove write-only variables
Am 29.01.2011 09:41, schrieb Blue Swirl: > Variables l2_modified and l2_size are not really used, remove them. > Spotted by GCC 4.6.0: > CCblock/qcow2-refcount.o > /src/qemu/block/qcow2-refcount.c: In function > 'qcow2_update_snapshot_refcount': > /src/qemu/block/qcow2-refcount.c:708:37: error: variable 'l2_modified' > set but not used [-Werror=unused-but-set-variable] > /src/qemu/block/qcow2-refcount.c:708:9: error: variable 'l2_size' set > but not used [-Werror=unused-but-set-variable] > > CC: Kevin Wolf > Signed-off-by: Blue Swirl Thanks, applied to the block branch. Kevin
[Qemu-devel] [PATCH] fix linuxboot.bin and multiboot.bin to not hijack int19
Currently linuxboot.bin and multiboot.bin option roms override int19 vector to intercept boot process. No sane option rom should do that. Provide bev entry instead that will be called by BIOS if option rom is selected for booting. Signed-off-by: Gleb Natapov --- Note that this patch should be applied after qemu will upgrade to Seabios that supports boot order. Otherwise there will be change in behavior since option rom will not be selected for booting by default. diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S index c109363..79b3ae8 100644 --- a/pc-bios/optionrom/linuxboot.S +++ b/pc-bios/optionrom/linuxboot.S @@ -22,6 +22,8 @@ #include "optionrom.h" +#define PRODUCT "Linux loader" + BOOT_ROM_START run_linuxboot: diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S index 9131837..069c4e8 100644 --- a/pc-bios/optionrom/multiboot.S +++ b/pc-bios/optionrom/multiboot.S @@ -20,6 +20,8 @@ #include "optionrom.h" +#define PRODUCT "multiboot loader" + #define MULTIBOOT_MAGIC0x2badb002 #define GS_PROT_JUMP 0 diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h index fbdd48a..95dbdda 100644 --- a/pc-bios/optionrom/optionrom.h +++ b/pc-bios/optionrom/optionrom.h @@ -93,31 +93,41 @@ .global _start; \ _start:; \ .short 0xaa55; \ - .byte (_end - _start) / 512; + .byte (_end - _start) / 512; \ + lret; \ + .org0x18; \ + .short 0; \ + .short _pnph; \ +_pnph: \ + .ascii "$PnP"; \ + .byte 0x01; \ + .byte ( _pnph_len / 16 ); \ + .short 0x; \ + .byte 0x00; \ + .byte 0x00; \ + .long 0x; \ + .short _manufacturer; \ + .short _product; \ + .long 0x; \ + .short 0x; \ + .short 0x; \ + .short _bev; \ + .short 0x; \ + .short 0x; \ + .equ_pnph_len, . - _pnph; #define BOOT_ROM_START \ OPTION_ROM_START\ - push%eax; \ - push%ds;\ - \ - /* setup ds so we can access the IVT */ \ - xor %ax, %ax; \ - mov %ax, %ds; \ - \ - /* install our int 19 handler */\ - movw$int19_handler, (0x19*4); \ - mov %cs, (0x19*4+2);\ - \ - pop %ds;\ - pop %eax; \ - lret; \ - \ -int19_handler:;\ +_bev:; \ /* DS = CS */ \ movw%cs, %ax; \ movw%ax, %ds; #define OPTION_ROM_END \ +_manufacturer:;\ +.asciz "QEMU"; \ +_product:; \ +.asciz PRODUCT;\ .align 512, 0; \ _end: -- Gleb.
Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
On 31.01.2011 09:20, Aurelien Jarno wrote: > On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.l...@st.com wrote: >> From: Christophe Lyon >> >> Handle corner cases where the addition of the rounding constant could >> cause overflows. > > After applying this patch, I get the following gcc warning: > CCtranslate.o > cc1: warnings being treated as errors > qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’: > qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in > this function > make: *** [translate.o] Error 1 > Which GCC version are you using? I don't have such a warning (using GCC-4.5.1 on x86_64). Christophe.
Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
On Mon, Jan 31, 2011 at 10:35:30AM +0100, Christophe Lyon wrote: > On 31.01.2011 09:20, Aurelien Jarno wrote: > > On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.l...@st.com wrote: > >> From: Christophe Lyon > >> > >> Handle corner cases where the addition of the rounding constant could > >> cause overflows. > > > > After applying this patch, I get the following gcc warning: > > CCtranslate.o > > cc1: warnings being treated as errors > > qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’: > > qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in > > this function > > make: *** [translate.o] Error 1 > > > > Which GCC version are you using? I don't have such a warning (using GCC-4.5.1 > on x86_64). > I get this error with GCC 4.3.5, GCC 4.4.5, GCC 4.5.2 and GCC 4.6.0 (r169270). This is also on x86_64. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] Re: [PATCH 01/22] Prevent abortion on multiple VCPU kicks
On 01/27/2011 03:09 PM, Jan Kiszka wrote: If we call qemu_cpu_kick more than once before the target was able to process the signal, pthread_kill will fail, and qemu will abort. Prevent this by avoiding the redundant signal. Doesn't fit with the manual page (or with the idea that signals are asynchronous): NAME pthread_kill - send a signal to a thread ... ERRORS ESRCH No thread with the ID thread could be found. EINVAL An invalid signal was specified. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 04/22] Leave inner main_loop faster on pending requests
On 01/27/2011 03:09 PM, Jan Kiszka wrote:
If there is any pending request that requires us to leave the inner loop
if main_loop, makes sure we do this as soon as possible by enforcing
non-blocking IO processing.
At this change, move variable definitions out of the inner loop to
improve readability.
Signed-off-by: Jan Kiszka
---
vl.c | 11 +++
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/vl.c b/vl.c
index 5fad700..2ebc55b 100644
--- a/vl.c
+++ b/vl.c
@@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown;
static void main_loop(void)
{
+bool nonblocking = false;
+#ifdef CONFIG_PROFILER
+int64_t ti;
+#endif
int r;
qemu_main_loop_start();
for (;;) {
do {
-bool nonblocking = false;
-#ifdef CONFIG_PROFILER
-int64_t ti;
-#endif
#ifndef CONFIG_IOTHREAD
nonblocking = cpu_exec_all();
+if (!vm_can_run()) {
+nonblocking = true;
+}
Doesn't this cause vmstop to spin? We'll never execute
main_loop_wait(false) if I read the code correctly?
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
On Mon, Jan 31, 2011 at 10:44:14AM +0100, Aurelien Jarno wrote:
> On Mon, Jan 31, 2011 at 10:35:30AM +0100, Christophe Lyon wrote:
> > On 31.01.2011 09:20, Aurelien Jarno wrote:
> > > On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.l...@st.com wrote:
> > >> From: Christophe Lyon
> > >>
> > >> Handle corner cases where the addition of the rounding constant could
> > >> cause overflows.
> > >
> > > After applying this patch, I get the following gcc warning:
> > > CCtranslate.o
> > > cc1: warnings being treated as errors
> > > qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’:
> > > qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized
> > > in this function
> > > make: *** [translate.o] Error 1
> > >
> >
> > Which GCC version are you using? I don't have such a warning (using
> > GCC-4.5.1 on x86_64).
> >
>
> I get this error with GCC 4.3.5, GCC 4.4.5, GCC 4.5.2 and GCC 4.6.0
> (r169270). This is also on x86_64.
>
gcc 4.6.0 turned on warnings also on set but not used variables, there
are a few places that need fixes (pretty simple). This is the patch
I have applied:
commit 53bf1becf85a14c399698ae0961eb2b08d231986
Author: Alon Levy
Date: Fri Jan 28 11:04:43 2011 +0200
(temp) gcc 4.6.0 corrections for build
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index e37e226..da1a2e9 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -708,6 +708,9 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
int l2_size, i, j, l1_modified, l2_modified, nb_csectors, refcount;
int ret;
+(void)l2_modified;
+(void)l2_size;
+
l2_table = NULL;
l1_table = NULL;
l1_size2 = l1_size * sizeof(uint64_t);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7dfc357..a7019ee 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -892,7 +892,7 @@ static int kvm_get_xsave(CPUState *env)
#ifdef KVM_CAP_XSAVE
struct kvm_xsave* xsave;
int ret, i;
-uint16_t cwd, swd, twd, fop;
+uint16_t cwd, swd, twd;
if (!kvm_has_xsave())
return kvm_get_fpu(env);
@@ -907,7 +907,6 @@ static int kvm_get_xsave(CPUState *env)
cwd = (uint16_t)xsave->region[0];
swd = (uint16_t)(xsave->region[0] >> 16);
twd = (uint16_t)xsave->region[1];
-fop = (uint16_t)(xsave->region[1] >> 16);
env->fpstt = (swd >> 11) & 7;
env->fpus = swd;
env->fpuc = cwd;
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 5dd6a2c..b4d230b 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -582,6 +582,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned
int flags,
nparam = gen_opparam_ptr++;
#ifdef TCG_TARGET_I386
call_type = (flags & TCG_CALL_TYPE_MASK);
+(void)call_type;
#endif
if (ret != TCG_CALL_DUMMY_ARG) {
#if TCG_TARGET_REG_BITS < 64
diff --git a/ui/sdl.c b/ui/sdl.c
index f599d42..a1458ce 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -87,12 +87,6 @@ static void sdl_update(DisplayState *ds, int x, int y, int
w, int h)
static void sdl_setdata(DisplayState *ds)
{
-SDL_Rect rec;
-rec.x = 0;
-rec.y = 0;
-rec.w = real_screen->w;
-rec.h = real_screen->h;
-
if (guest_screen != NULL) SDL_FreeSurface(guest_screen);
guest_screen = SDL_CreateRGBSurfaceFrom(ds_get_data(ds), ds_get_width(ds),
ds_get_height(ds),
> --
> Aurelien JarnoGPG: 1024D/F1BCDB73
> aurel...@aurel32.net http://www.aurel32.net
>
[Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
On 01/27/2011 04:33 PM, Jan Kiszka wrote: Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between checking for exit_request on vcpu entry and timer signals arriving before KVM starts to catch them. Plug it by blocking both timer related signals also on !CONFIG_IOTHREAD and process those via signalfd. As this fix depends on real signalfd support (otherwise the timer signals only kick the compat helper thread, and the main thread hangs), we need to detect the invalid constellation and abort configure. Signed-off-by: Jan Kiszka CC: Stefan Hajnoczi --- I don't want to invest that much into !IOTHREAD anymore, so let's see if the proposed catch&abort is acceptable. I don't understand the dependency on signalfd. The normal way of doing things, either waiting for the signal in sigtimedwait() or in ioctl(KVM_RUN), works with SIGALRM just fine. -- error compiling committee.c: too many arguments to function
[Qemu-devel] KVM call agenda for Feb 1
Please send in any agenda items you are interested incovering. Thanks, Juan.
[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop
On 01/27/2011 03:10 PM, Jan Kiszka wrote:
Align with qemu-kvm and prepare for IO exit fix: There is no need to run
kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
this service processes will first cause an exit from kvm_cpu_exec
anyway. And we will have to reenter the kernel on IO exits
unconditionally, something that the current logic prevents.
Signed-off-by: Jan Kiszka
---
kvm-all.c | 11 ++-
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index 5bfa8c0..46ecc1c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
DPRINTF("kvm_cpu_exec()\n");
+if (kvm_arch_process_irqchip_events(env)) {
+env->exit_request = 0;
+env->exception_index = EXCP_HLT;
+return 0;
+}
+
do {
#ifndef CONFIG_IOTHREAD
if (env->exit_request) {
@@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
}
We check for ->exit_request here
#endif
-if (kvm_arch_process_irqchip_events(env)) {
-ret = 0;
-break;
-}
-
But this checks for ->interrupt_request. What ensures that we exit when
->interrupt_request is set?
if (env->kvm_vcpu_dirty) {
kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
env->kvm_vcpu_dirty = 0;
--
error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 00/22] [uq/master] Patch queue, part II
On 01/27/2011 03:09 PM, Jan Kiszka wrote: This second round of patches focus on issues in cpus.c, primarily signal related. The highlights are - Add missing KVM_RUN continuation after I/O exits - Fix for timer signal race in KVM entry code under !CONFIG_IOTHREAD (based on Stefan's findings) - MCE signal processing under !CONFIG_IOTHREAD - Prevent abortion on multiple VCPU kicks - Fixed synchronous (ie. VCPU-issued) reset request processing Topics remaining for a third round are cleanups and refactorings of the KVM VCPU entry/exit path, MCE rework, and a few assorted cleanups. But I will wait at least until these bits here are ready for an upstream merge. Pretty nice. Is this standalone or does it depend on unmerged changes? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] SPARC: Fix Leon3 cache control
On 01/28/2011 11:27 PM, Blue Swirl wrote:
On Fri, Jan 28, 2011 at 1:55 PM, Fabien Chouteau wrote:
On 01/27/2011 11:40 AM, Fabien Chouteau wrote:
The "leon3_cache_control_int" (op_helper.c) function is called within
leon3.c
which leads to segfault error with the global "env".
Now cache control is a CPU feature and everything is handled in
op_helper.c.
Signed-off-by: Fabien Chouteau
---
hw/leon3.c |1 -
target-sparc/cpu.h |4 ++--
target-sparc/helper.c|2 +-
target-sparc/op_helper.c | 23 +--
4 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/hw/leon3.c b/hw/leon3.c
index 69d8f3b..aaf26fd 100644
--- a/hw/leon3.c
+++ b/hw/leon3.c
@@ -59,7 +59,6 @@ static void main_cpu_reset(void *opaque)
static void leon3_irq_ack(void *irq_manager, int intno)
{
grlib_irqmp_ack((DeviceState *)irq_manager, intno);
-leon3_cache_control_int();
}
static void leon3_set_pil_in(void *opaque, uint32_t pil_in)
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 6f5990b..8141b32 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -268,6 +268,8 @@ typedef struct sparc_def_t {
#define CPU_FEATURE_GL (1<<13)
#define CPU_FEATURE_TA0_SHUTDOWN (1<<14) /* Shutdown on "ta 0x0" */
#define CPU_FEATURE_ASR17(1<<15)
+#define CPU_FEATURE_CACHE_CTRL (1<<16)
+
#ifndef TARGET_SPARC64
#define CPU_DEFAULT_FEATURES (CPU_FEATURE_FLOAT | CPU_FEATURE_SWAP | \
CPU_FEATURE_MUL | CPU_FEATURE_DIV | \
@@ -477,8 +479,6 @@ int cpu_cwp_inc(CPUState *env1, int cwp);
int cpu_cwp_dec(CPUState *env1, int cwp);
void cpu_set_cwp(CPUState *env1, int new_cwp);
-void leon3_cache_control_int(void);
-
/* sun4m.c, sun4u.c */
void cpu_check_irqs(CPUSPARCState *env);
diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 2f3d1e6..b2d4d70 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -1289,7 +1289,7 @@ static const sparc_def_t sparc_defs[] = {
.mmu_trcr_mask = 0x,
.nwindows = 8,
.features = CPU_DEFAULT_FEATURES | CPU_FEATURE_TA0_SHUTDOWN |
-CPU_FEATURE_ASR17,
+CPU_FEATURE_ASR17 | CPU_FEATURE_CACHE_CTRL,
},
#endif
};
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index d3e1b63..698c159 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -1653,7 +1653,7 @@ static void dump_asi(const char *txt, target_ulong
addr, int asi, int size,
/* Leon3 cache control */
-void leon3_cache_control_int(void)
+static void leon3_cache_control_int(void)
{
uint32_t state = 0;
@@ -1741,7 +1741,7 @@ static uint64_t leon3_cache_control_ld(target_ulong
addr, int size)
DPRINTF_CACHE_CONTROL("read unknown register %08x\n", addr);
break;
};
-DPRINTF_CACHE_CONTROL("st addr:%08x, ret:%" PRIx64 ", size:%d\n",
+DPRINTF_CACHE_CONTROL("ld addr:%08x, ret:0x%" PRIx64 ", size:%d\n",
addr, ret, size);
return ret;
}
@@ -1760,7 +1760,9 @@ uint64_t helper_ld_asi(target_ulong addr, int asi,
int size, int sign)
case 0x00: /* Leon3 Cache Control */
case 0x08: /* Leon3 Instruction Cache config */
case 0x0C: /* Leon3 Date Cache config */
-ret = leon3_cache_control_ld(addr, size);
+if (env->def->features&CPU_FEATURE_CACHE_CTRL) {
+ret = leon3_cache_control_ld(addr, size);
+}
break;
case 0x01c00a00: /* MXCC control register */
if (size == 8)
@@ -1994,7 +1996,9 @@ void helper_st_asi(target_ulong addr, uint64_t val,
int asi, int size)
case 0x00: /* Leon3 Cache Control */
case 0x08: /* Leon3 Instruction Cache config */
case 0x0C: /* Leon3 Date Cache config */
-leon3_cache_control_st(addr, val, size);
+if (env->def->features&CPU_FEATURE_CACHE_CTRL) {
+leon3_cache_control_st(addr, val, size);
+}
break;
case 0x01c0: /* MXCC stream data register 0 */
@@ -4325,9 +4329,16 @@ void do_interrupt(CPUState *env)
#if !defined(CONFIG_USER_ONLY)
/* IRQ acknowledgment */
-if ((intno&~15) == TT_EXTINT&&env->qemu_irq_ack != NULL) {
-env->qemu_irq_ack(env->irq_manager, intno);
+if ((intno&~15) == TT_EXTINT) {
+if (env->qemu_irq_ack != NULL) {
+env->qemu_irq_ack(env->irq_manager, intno);
+}
+
+if (env->def->features&CPU_FEATURE_CACHE_CTRL) {
+leon3_cache_control_int();
+}
}
+
#endif
}
#endif
Any comment?
This would slow down the IRQ path for non-Leon case a bit. How about
adding a new irq manager function in op_helper.c for Leon, which just
calls leon3_irq_ack() and then leon3_cache_control_int()? Then the
feature check would not be nee
[Qemu-devel] [PATCH v2] SPARC: Fix Leon3 cache control
The "leon3_cache_control_int" (op_helper.c) function is called within leon3.c
which leads to segfault error with the global "env".
Now cache control is a CPU feature and everything is handled in op_helper.c.
Signed-off-by: Fabien Chouteau
---
hw/leon3.c |5 ++---
target-sparc/cpu.h |8 ++--
target-sparc/helper.c|2 +-
target-sparc/op_helper.c | 18 ++
4 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/hw/leon3.c b/hw/leon3.c
index 69d8f3b..919f49f 100644
--- a/hw/leon3.c
+++ b/hw/leon3.c
@@ -56,10 +56,9 @@ static void main_cpu_reset(void *opaque)
env->npc= s->entry + 4;
}
-static void leon3_irq_ack(void *irq_manager, int intno)
+void leon3_irq_ack(void *irq_manager, int intno)
{
grlib_irqmp_ack((DeviceState *)irq_manager, intno);
-leon3_cache_control_int();
}
static void leon3_set_pil_in(void *opaque, uint32_t pil_in)
@@ -130,7 +129,7 @@ static void leon3_generic_hw_init(ram_addr_t ram_size,
/* Allocate IRQ manager */
grlib_irqmp_create(0x8200, env, &cpu_irqs, MAX_PILS,
&leon3_set_pil_in);
-env->qemu_irq_ack = leon3_irq_ack;
+env->qemu_irq_ack = leon3_irq_manager;
/* Allocate RAM */
if ((uint64_t)ram_size > (1UL << 30)) {
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 6f5990b..320530e 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -268,6 +268,8 @@ typedef struct sparc_def_t {
#define CPU_FEATURE_GL (1 << 13)
#define CPU_FEATURE_TA0_SHUTDOWN (1 << 14) /* Shutdown on "ta 0x0" */
#define CPU_FEATURE_ASR17(1 << 15)
+#define CPU_FEATURE_CACHE_CTRL (1 << 16)
+
#ifndef TARGET_SPARC64
#define CPU_DEFAULT_FEATURES (CPU_FEATURE_FLOAT | CPU_FEATURE_SWAP | \
CPU_FEATURE_MUL | CPU_FEATURE_DIV | \
@@ -476,12 +478,14 @@ void cpu_put_cwp64(CPUState *env1, int cwp);
int cpu_cwp_inc(CPUState *env1, int cwp);
int cpu_cwp_dec(CPUState *env1, int cwp);
void cpu_set_cwp(CPUState *env1, int new_cwp);
-
-void leon3_cache_control_int(void);
+void leon3_irq_manager(void *irq_manager, int intno);
/* sun4m.c, sun4u.c */
void cpu_check_irqs(CPUSPARCState *env);
+/* leon3.c */
+void leon3_irq_ack(void *irq_manager, int intno);
+
#if defined (TARGET_SPARC64)
static inline int compare_masked(uint64_t x, uint64_t y, uint64_t mask)
diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 2f3d1e6..b2d4d70 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -1289,7 +1289,7 @@ static const sparc_def_t sparc_defs[] = {
.mmu_trcr_mask = 0x,
.nwindows = 8,
.features = CPU_DEFAULT_FEATURES | CPU_FEATURE_TA0_SHUTDOWN |
-CPU_FEATURE_ASR17,
+CPU_FEATURE_ASR17 | CPU_FEATURE_CACHE_CTRL,
},
#endif
};
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index d3e1b63..854f168 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -1653,7 +1653,7 @@ static void dump_asi(const char *txt, target_ulong addr,
int asi, int size,
/* Leon3 cache control */
-void leon3_cache_control_int(void)
+static void leon3_cache_control_int(void)
{
uint32_t state = 0;
@@ -1741,11 +1741,17 @@ static uint64_t leon3_cache_control_ld(target_ulong
addr, int size)
DPRINTF_CACHE_CONTROL("read unknown register %08x\n", addr);
break;
};
-DPRINTF_CACHE_CONTROL("st addr:%08x, ret:%" PRIx64 ", size:%d\n",
+DPRINTF_CACHE_CONTROL("ld addr:%08x, ret:0x%" PRIx64 ", size:%d\n",
addr, ret, size);
return ret;
}
+void leon3_irq_manager(void *irq_manager, int intno)
+{
+leon3_irq_ack(irq_manager, intno);
+leon3_cache_control_int();
+}
+
uint64_t helper_ld_asi(target_ulong addr, int asi, int size, int sign)
{
uint64_t ret = 0;
@@ -1760,7 +1766,9 @@ uint64_t helper_ld_asi(target_ulong addr, int asi, int
size, int sign)
case 0x00: /* Leon3 Cache Control */
case 0x08: /* Leon3 Instruction Cache config */
case 0x0C: /* Leon3 Date Cache config */
-ret = leon3_cache_control_ld(addr, size);
+if (env->def->features & CPU_FEATURE_CACHE_CTRL) {
+ret = leon3_cache_control_ld(addr, size);
+}
break;
case 0x01c00a00: /* MXCC control register */
if (size == 8)
@@ -1994,7 +2002,9 @@ void helper_st_asi(target_ulong addr, uint64_t val, int
asi, int size)
case 0x00: /* Leon3 Cache Control */
case 0x08: /* Leon3 Instruction Cache config */
case 0x0C: /* Leon3 Date Cache config */
-leon3_cache_control_st(addr, val, size);
+if (env->def->features & CPU_FEATURE_CACHE_CTRL) {
+leon3_cache_control_st(addr, val, size);
+}
break;
case 0x01c0: /* MXCC stream data register 0 */
--
1.7.1
Re: [Qemu-devel] Re: [PATCH] mingw32: Fix definitions for PRId64, PRIx64, PRIu64, PRIo64
On 30/01/11 22:14, Blue Swirl wrote: Well, mine doesn't. The change was introduced in mingw-runtime 3.15, which was released in September 2008, but Debian still hasn't updated from 3.13. Maybe other distros are not so lagging and someone who wishes to build QEMU on Windows is not pampered with distro support for MinGW anyway. Perhaps a configure time check should be added? FWIW anyone using MingW now should be using the new GCC 4.5 based builds with the associated runtime rather than anything that old. I suspect most people using MingW for anything serious will be doing this already. I think adding the configure check would be the best solution here. QEMU defines __USE_MINGW_ANSI_STDIO, and __mingw_vfprintf understands C9x standard length specifiers. BTW, MinGW FAQ page http://www.mingw.org/wiki/FAQ still mentions that %ll formats are not supported. Since MingW uses the MSVCRT then by default it won't accept %ll, but my understanding is that Stefan is correct, i.e. defining __USE_MINGW_ANSI_STDIO causes MingW to use an internal standards-compliant implementation instead of the in-built MSVCRT version that *does* understand these extras. HTH, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs
[Qemu-devel] [PATCH] hw/slavio_intctl.c: fix gcc warning about array bounds overrun
The Ubuntu 10.10 gcc for ARM complains that we might be overrunning
the cpu_irqs[][] array: silence this by correcting the bounds on the
loop. (In fact we would not have overrun the array because bit
MAX_PILS in pil_pending and irl_out will always be 0.)
Also add a comment about why the loop's lower bound is OK.
Signed-off-by: Peter Maydell
---
I've tested that with this change we still boot the sparc
Debian image from http://people.debian.org/~aurel32/qemu/sparc/
and the change makes sense according to my understanding of
http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/NCR89C105.txt
hw/slavio_intctl.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c
index fd69354..a83e5b8 100644
--- a/hw/slavio_intctl.c
+++ b/hw/slavio_intctl.c
@@ -289,7 +289,12 @@ static void slavio_check_interrupts(SLAVIO_INTCTLState *s,
int set_irqs)
pil_pending |= (s->slaves[i].intreg_pending & CPU_SOFTIRQ_MASK) >> 16;
if (set_irqs) {
-for (j = MAX_PILS; j > 0; j--) {
+/* Since there is not really an interrupt 0 (and pil_pending
+ * and irl_out bit zero are thus always zero) there is no need
+ * to do anything with cpu_irqs[i][0] and it is OK not to do
+ * the j=0 iteration of this loop.
+ */
+for (j = MAX_PILS-1; j > 0; j--) {
if (pil_pending & (1 << j)) {
if (!(s->slaves[i].irl_out & (1 << j))) {
qemu_irq_raise(s->cpu_irqs[i][j]);
--
1.7.1
[Qemu-devel] [PATCH v3 07/11] blockdev: Replace drive_add()'s fmt, ... by optstr parameter
Let the callers build the optstr. Only one wants to. All the others
become simpler, because they don't have to worry about escaping '%'.
Signed-off-by: Markus Armbruster
---
v2: Clean up shadowing of local variable buf[] in main(). Pointed out
by Kevin.
blockdev.c |8 +---
blockdev.h |5 +
vl.c | 23 +--
3 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 60dba1a..a5e56ee 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -81,17 +81,11 @@ QemuOpts *drive_def(const char *optstr)
}
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
-const char *fmt, ...)
+const char *optstr)
{
-va_list ap;
-char optstr[1024];
QemuOpts *opts;
char buf[32];
-va_start(ap, fmt);
-vsnprintf(optstr, sizeof(optstr), fmt, ap);
-va_end(ap);
-
opts = drive_def(optstr);
if (!opts) {
return NULL;
diff --git a/blockdev.h b/blockdev.h
index eb76573..3b9c5fd 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -46,10 +46,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
QemuOpts *drive_def(const char *optstr);
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
-const char *fmt, ...) /*GCC_FMT_ATTR(4, 5)*/;
-/* GCC_FMT_ATTR() commented out to avoid the (pretty useless)
- * "zero-length gnu_printf format string" warning we insist to
- * enable */
+const char *optstr);
DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
/* device-hotplug */
diff --git a/vl.c b/vl.c
index 5399e33..979852b 100644
--- a/vl.c
+++ b/vl.c
@@ -621,7 +621,6 @@ static int bt_parse(const char *opt)
/***/
/* QEMU Block devices */
-/* Any % in the following strings must be escaped as %% */
#define HD_OPTS "media=disk"
#define CDROM_OPTS "media=cdrom"
#define FD_OPTS ""
@@ -2027,17 +2026,21 @@ int main(int argc, char **argv, char **envp)
initrd_filename = optarg;
break;
case QEMU_OPTION_hda:
-if (cyls == 0)
-hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS);
-else
-hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS
-",cyls=%d,heads=%d,secs=%d%s",
- cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
+{
+char buf[256];
+if (cyls == 0)
+snprintf(buf, sizeof(buf), "%s", HD_OPTS);
+else
+snprintf(buf, sizeof(buf),
+ "%s,cyls=%d,heads=%d,secs=%d%s",
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
",trans=lba" :
- translation == BIOS_ATA_TRANSLATION_NONE ?
+ translation == BIOS_ATA_TRANSLATION_NONE ?
",trans=none" : "");
- break;
+drive_add(IF_DEFAULT, 0, optarg, buf);
+break;
+}
case QEMU_OPTION_hdb:
case QEMU_OPTION_hdc:
case QEMU_OPTION_hdd:
--
1.7.2.3
[Qemu-devel] Re: [PATCH v2 00/11] -drive/drive_add fixes and cleanups
Am 28.01.2011 11:21, schrieb Markus Armbruster:
> Kevin found a bug in my recent "[PATCH 3+5/5] -drive/drive_add fixes".
> This is a rework of those two patches, plus a fix for the -drive
> if=scsi,index=N regression, plus the odd bonus fix found on the way.
>
> v2: fix for the -drive if=scsi,index=N regression. PATCH 4 fixed up
> slightly, PATCH 5 inserted, rest trivially rediffed.
>
> Markus Armbruster (11):
> scsi hotplug: Set DriveInfo member bus correctly
> blockdev: New drive_get_next(), replacing qdev_init_bdrv()
> blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h
> blockdev: Put BlockInterfaceType names and max_devs in tables
> blockdev: Fix regression in -drive if=scsi,index=N
> blockdev: Make drive_add() take explicit type, index parameters
> blockdev: Replace drive_add()'s fmt, ... by optstr parameter
> blockdev: Factor drive_index_to_{bus,unit}_id out of drive_init()
> blockdev: New drive_get_by_index()
> blockdev: Reject multiple definitions for the same drive
> blockdev: Fix drive_add for drives without media
>
> blockdev.c | 143
> +++
> blockdev.h | 18 +--
> hw/device-hotplug.c |5 +-
> hw/ide.h|2 +
> hw/ide/ahci.c |1 -
> hw/pci-hotplug.c|1 +
> hw/pl181.c |7 ++-
> hw/qdev.c | 15 -
> hw/qdev.h |2 -
> hw/scsi.h |3 +-
> hw/ssi-sd.c |7 ++-
> hw/usb-msd.c|3 +-
> qemu-common.h |6 --
> savevm.c|1 -
> vl.c| 94 +++--
> 15 files changed, 171 insertions(+), 137 deletions(-)
Thanks, applied all to the block branch.
Kevin
[Qemu-devel] Re: [PATCH 01/22] Prevent abortion on multiple VCPU kicks
On 2011-01-31 10:44, Avi Kivity wrote: > On 01/27/2011 03:09 PM, Jan Kiszka wrote: >> If we call qemu_cpu_kick more than once before the target was able to >> process the signal, pthread_kill will fail, and qemu will abort. Prevent >> this by avoiding the redundant signal. >> > > Doesn't fit with the manual page (or with the idea that signals are > asynchronous): > > NAME > pthread_kill - send a signal to a thread > > > ... > > ERRORS > ESRCH No thread with the ID thread could be found. > > EINVAL An invalid signal was specified. > Valid remark, but I was receiving EAGAIN for blocked RT signals. Don't know if this is Linux-specific. A quick glance at the man pages did not reveal if this is allowed or at least gray area. However, even when selectively ignoring this, it's more efficient to catch the redundant signaling in user space. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH 04/22] Leave inner main_loop faster on pending requests
On 2011-01-31 10:52, Avi Kivity wrote:
> On 01/27/2011 03:09 PM, Jan Kiszka wrote:
>> If there is any pending request that requires us to leave the inner loop
>> if main_loop, makes sure we do this as soon as possible by enforcing
>> non-blocking IO processing.
>>
>> At this change, move variable definitions out of the inner loop to
>> improve readability.
>>
>> Signed-off-by: Jan Kiszka
>> ---
>> vl.c | 11 +++
>> 1 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 5fad700..2ebc55b 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown;
>>
>> static void main_loop(void)
>> {
>> +bool nonblocking = false;
>> +#ifdef CONFIG_PROFILER
>> +int64_t ti;
>> +#endif
>> int r;
>>
>> qemu_main_loop_start();
>>
>> for (;;) {
>> do {
>> -bool nonblocking = false;
>> -#ifdef CONFIG_PROFILER
>> -int64_t ti;
>> -#endif
>> #ifndef CONFIG_IOTHREAD
>> nonblocking = cpu_exec_all();
>> +if (!vm_can_run()) {
>> +nonblocking = true;
>> +}
>
> Doesn't this cause vmstop to spin? We'll never execute
> main_loop_wait(false) if I read the code correctly?
>
The code path is not changed, we just poll instead of wait in
main_loop_wait.
Also, I didn't get your error scenario yet. Even if we left the loop
here, what magic would main_loop_wait do to vmstop processing? The stop
request is handled outside the loop, that's why we should leave ASAP.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
On 2011-01-31 11:03, Avi Kivity wrote: > On 01/27/2011 04:33 PM, Jan Kiszka wrote: >> Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between >> checking for exit_request on vcpu entry and timer signals arriving >> before KVM starts to catch them. Plug it by blocking both timer related >> signals also on !CONFIG_IOTHREAD and process those via signalfd. >> >> As this fix depends on real signalfd support (otherwise the timer >> signals only kick the compat helper thread, and the main thread hangs), >> we need to detect the invalid constellation and abort configure. >> >> Signed-off-by: Jan Kiszka >> CC: Stefan Hajnoczi >> --- >> >> I don't want to invest that much into !IOTHREAD anymore, so let's see if >> the proposed catch&abort is acceptable. >> > > I don't understand the dependency on signalfd. The normal way of doing > things, either waiting for the signal in sigtimedwait() or in > ioctl(KVM_RUN), works with SIGALRM just fine. And how would you be kicked out of the select() call if it is waiting with a timeout? We only have a single thread here. The only alternative is Stefan's original proposal. But that required fiddling with the signal mask twice per KVM_RUN. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop
On 2011-01-31 11:08, Avi Kivity wrote:
> On 01/27/2011 03:10 PM, Jan Kiszka wrote:
>> Align with qemu-kvm and prepare for IO exit fix: There is no need to run
>> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
>> this service processes will first cause an exit from kvm_cpu_exec
>> anyway. And we will have to reenter the kernel on IO exits
>> unconditionally, something that the current logic prevents.
>>
>> Signed-off-by: Jan Kiszka
>> ---
>> kvm-all.c | 11 ++-
>> 1 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 5bfa8c0..46ecc1c 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
>>
>> DPRINTF("kvm_cpu_exec()\n");
>>
>> +if (kvm_arch_process_irqchip_events(env)) {
>> +env->exit_request = 0;
>> +env->exception_index = EXCP_HLT;
>> +return 0;
>> +}
>> +
>> do {
>> #ifndef CONFIG_IOTHREAD
>> if (env->exit_request) {
>> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
>> }
>
> We check for ->exit_request here
>
>> #endif
>>
>> -if (kvm_arch_process_irqchip_events(env)) {
>> -ret = 0;
>> -break;
>> -}
>> -
>
> But this checks for ->interrupt_request. What ensures that we exit when
> ->interrupt_request is set?
Good question, need to check again. But if that turns out to be an
issue, qemu-kvm would be broken as well. I'm just aligning the code here.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
[Qemu-devel] Re: Google Summer of Code 2011
On Sun, 30 Jan 2011 16:06:20 +0100 Alexander Graf wrote: > > On 28.01.2011, at 21:10, Luiz Capitulino wrote: > > > Hi there, > > > > GSoC 2011 has been announced[1]. As we were pretty successful last year, > > I think we should participate again. I've already created a wiki page: > > > > http://wiki.qemu.org/Google_Summer_of_Code_2011 > > > > We should now populate it with projects and people willing to be mentors > > should say so (or just add a project)[2]. > > > > Also, I'd like to do something different this year, I'd like to invite > > libvirt people to join. There are two ways of doing this: > > > > 1. They join in the program as a regular mentoring organization, or > > > > 2. They join with QEMU > > > > The second option means that libvirt can suggest and run its own projects > > (preferably with QEMU relevance), but from a GSoC perspective, the project > > will be part of the QEMU org. > > Keep in mind that every full org gets a free trip to the west coast for 2 > people ;). So splitting up means we could almost do a mini-summit at the > google campus on google's expenses ;). Actually, they have a limited budget and if you live too far (say, in Brazil), the trip might not be 100% free :) > Please coordinate that with Carol. Apparently traction for GSOC is declining > (according to last year's summit). So there might be plenty of available > slots this year. So I'd say sign up separately for now and if you don't get > accepted, just join forces with us! Yes, that's a good plan and I fully agree that we get more benefits if we apply separately. It's a call to libvirt's people.
Re: [Qemu-devel] [PATCH] fix linuxboot.bin and multiboot.bin to not hijack int19
On 31.01.2011, at 10:02, Gleb Natapov wrote: > Currently linuxboot.bin and multiboot.bin option roms override int19 > vector to intercept boot process. No sane option rom should do that. > Provide bev entry instead that will be called by BIOS if option rom > is selected for booting. > > Signed-off-by: Gleb Natapov > --- > > Note that this patch should be applied after qemu will upgrade to Seabios > that supports boot order. Otherwise there will be change in behavior > since option rom will not be selected for booting by default. > > diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S > index c109363..79b3ae8 100644 > --- a/pc-bios/optionrom/linuxboot.S > +++ b/pc-bios/optionrom/linuxboot.S > @@ -22,6 +22,8 @@ > > #include "optionrom.h" > > +#define PRODUCT "Linux loader" > + > BOOT_ROM_START > > run_linuxboot: > diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S > index 9131837..069c4e8 100644 > --- a/pc-bios/optionrom/multiboot.S > +++ b/pc-bios/optionrom/multiboot.S > @@ -20,6 +20,8 @@ > > #include "optionrom.h" > > +#define PRODUCT "multiboot loader" > + > #define MULTIBOOT_MAGIC 0x2badb002 > > #define GS_PROT_JUMP 0 > diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h > index fbdd48a..95dbdda 100644 > --- a/pc-bios/optionrom/optionrom.h > +++ b/pc-bios/optionrom/optionrom.h > @@ -93,31 +93,41 @@ > .global _start; \ > _start:; \ > .short 0xaa55; \ > - .byte (_end - _start) / 512; > + .byte (_end - _start) / 512; \ > + lret; \ > + .org0x18; \ > + .short 0; \ > + .short _pnph; \ > +_pnph: \ > + .ascii "$PnP"; \ The idea behind the OPTION_ROM and BOOT_ROM split was to have a generic header that can be used as template for random option roms or boot roms alike. Your patch munges those two use-cases together by providing bev logic in the generic option rom part. Please split it out into the BOOT_ROM macro, or - if necessary - create a new macro. Alex
Re: [Qemu-devel] [PATCH] fix linuxboot.bin and multiboot.bin to not hijack int19
On Mon, Jan 31, 2011 at 12:41:46PM +0100, Alexander Graf wrote: > > Signed-off-by: Gleb Natapov > The idea behind the OPTION_ROM and BOOT_ROM split was to have a generic > header that can be used as template for random option roms or boot roms > alike. Your patch munges those two use-cases together by providing bev logic > in the generic option rom part. > > Please split it out into the BOOT_ROM macro, or - if necessary - create a new > macro. > > Like this? diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S index c109363..79b3ae8 100644 --- a/pc-bios/optionrom/linuxboot.S +++ b/pc-bios/optionrom/linuxboot.S @@ -22,6 +22,8 @@ #include "optionrom.h" +#define PRODUCT "Linux loader" + BOOT_ROM_START run_linuxboot: diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S index 9131837..069c4e8 100644 --- a/pc-bios/optionrom/multiboot.S +++ b/pc-bios/optionrom/multiboot.S @@ -20,6 +20,8 @@ #include "optionrom.h" +#define PRODUCT "multiboot loader" + #define MULTIBOOT_MAGIC0x2badb002 #define GS_PROT_JUMP 0 diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h index fbdd48a..2d4f40e 100644 --- a/pc-bios/optionrom/optionrom.h +++ b/pc-bios/optionrom/optionrom.h @@ -97,27 +97,37 @@ #define BOOT_ROM_START \ OPTION_ROM_START\ - push%eax; \ - push%ds;\ - \ - /* setup ds so we can access the IVT */ \ - xor %ax, %ax; \ - mov %ax, %ds; \ - \ - /* install our int 19 handler */\ - movw$int19_handler, (0x19*4); \ - mov %cs, (0x19*4+2);\ - \ - pop %ds;\ - pop %eax; \ lret; \ - \ -int19_handler:;\ + .org0x18; \ + .short 0; \ + .short _pnph; \ +_pnph: \ + .ascii "$PnP"; \ + .byte 0x01; \ + .byte ( _pnph_len / 16 ); \ + .short 0x; \ + .byte 0x00; \ + .byte 0x00; \ + .long 0x; \ + .short _manufacturer; \ + .short _product; \ + .long 0x; \ + .short 0x; \ + .short 0x; \ + .short _bev; \ + .short 0x; \ + .short 0x; \ + .equ_pnph_len, . - _pnph; +_bev:; \ /* DS = CS */ \ movw%cs, %ax; \ movw%ax, %ds; #define OPTION_ROM_END \ +_manufacturer:;\ +.asciz "QEMU"; \ +_product:; \ +.asciz PRODUCT;\ .align 512, 0; \ _end: -- Gleb.
Re: [Qemu-devel] [PATCH] fix linuxboot.bin and multiboot.bin to not hijack int19
On 31.01.2011, at 12:50, Gleb Natapov wrote: > On Mon, Jan 31, 2011 at 12:41:46PM +0100, Alexander Graf wrote: >>> Signed-off-by: Gleb Natapov >> The idea behind the OPTION_ROM and BOOT_ROM split was to have a generic >> header that can be used as template for random option roms or boot roms >> alike. Your patch munges those two use-cases together by providing bev logic >> in the generic option rom part. >> >> Please split it out into the BOOT_ROM macro, or - if necessary - create a >> new macro. >> >> > Like this? > > diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S > index c109363..79b3ae8 100644 > --- a/pc-bios/optionrom/linuxboot.S > +++ b/pc-bios/optionrom/linuxboot.S > @@ -22,6 +22,8 @@ > > #include "optionrom.h" > > +#define PRODUCT "Linux loader" Please make this BOOT_ROM_PRODUCT Otherwise, yes, a lot better :) Alex
[Qemu-devel] Re: [PATCH 00/22] [uq/master] Patch queue, part II
On 2011-01-31 11:12, Avi Kivity wrote: > On 01/27/2011 03:09 PM, Jan Kiszka wrote: >> This second round of patches focus on issues in cpus.c, primarily signal >> related. The highlights are >> >> - Add missing KVM_RUN continuation after I/O exits >> - Fix for timer signal race in KVM entry code under !CONFIG_IOTHREAD >> (based on Stefan's findings) >> - MCE signal processing under !CONFIG_IOTHREAD >> - Prevent abortion on multiple VCPU kicks >> - Fixed synchronous (ie. VCPU-issued) reset request processing >> >> Topics remaining for a third round are cleanups and refactorings of the >> KVM VCPU entry/exit path, MCE rework, and a few assorted cleanups. But I >> will wait at least until these bits here are ready for an upstream >> merge. >> > > Pretty nice. Is this standalone or does it depend on unmerged changes? > It only depends on my previous patches in current uq/master. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
On Mon, Jan 31, 2011 at 11:27 AM, Jan Kiszka wrote: > On 2011-01-31 11:03, Avi Kivity wrote: >> On 01/27/2011 04:33 PM, Jan Kiszka wrote: >>> Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between >>> checking for exit_request on vcpu entry and timer signals arriving >>> before KVM starts to catch them. Plug it by blocking both timer related >>> signals also on !CONFIG_IOTHREAD and process those via signalfd. >>> >>> As this fix depends on real signalfd support (otherwise the timer >>> signals only kick the compat helper thread, and the main thread hangs), >>> we need to detect the invalid constellation and abort configure. >>> >>> Signed-off-by: Jan Kiszka >>> CC: Stefan Hajnoczi >>> --- >>> >>> I don't want to invest that much into !IOTHREAD anymore, so let's see if >>> the proposed catch&abort is acceptable. >>> >> >> I don't understand the dependency on signalfd. The normal way of doing >> things, either waiting for the signal in sigtimedwait() or in >> ioctl(KVM_RUN), works with SIGALRM just fine. > > And how would you be kicked out of the select() call if it is waiting > with a timeout? We only have a single thread here. > > The only alternative is Stefan's original proposal. But that required > fiddling with the signal mask twice per KVM_RUN. I think my original patch messed with the sigmask in the wrong place, as you mentioned doing it twice per KVM_RUN isn't a good idea. I wonder if we can enable SIGALRM only in blocking calls and guest code execution but without signalfd. It might be possible, I don't see an immediate problem with doing that, we might have to use pselect(2) or similar in a few places. Stefan
Re: [Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
On 2011-01-31 13:13, Stefan Hajnoczi wrote: > On Mon, Jan 31, 2011 at 11:27 AM, Jan Kiszka wrote: >> On 2011-01-31 11:03, Avi Kivity wrote: >>> On 01/27/2011 04:33 PM, Jan Kiszka wrote: Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between checking for exit_request on vcpu entry and timer signals arriving before KVM starts to catch them. Plug it by blocking both timer related signals also on !CONFIG_IOTHREAD and process those via signalfd. As this fix depends on real signalfd support (otherwise the timer signals only kick the compat helper thread, and the main thread hangs), we need to detect the invalid constellation and abort configure. Signed-off-by: Jan Kiszka CC: Stefan Hajnoczi --- I don't want to invest that much into !IOTHREAD anymore, so let's see if the proposed catch&abort is acceptable. >>> >>> I don't understand the dependency on signalfd. The normal way of doing >>> things, either waiting for the signal in sigtimedwait() or in >>> ioctl(KVM_RUN), works with SIGALRM just fine. >> >> And how would you be kicked out of the select() call if it is waiting >> with a timeout? We only have a single thread here. >> >> The only alternative is Stefan's original proposal. But that required >> fiddling with the signal mask twice per KVM_RUN. > > I think my original patch messed with the sigmask in the wrong place, > as you mentioned doing it twice per KVM_RUN isn't a good idea. I > wonder if we can enable SIGALRM only in blocking calls and guest code > execution but without signalfd. It might be possible, I don't see an > immediate problem with doing that, we might have to use pselect(2) or > similar in a few places. My main concern about alternative approaches is that IOTHREAD is about to become the default, and hardly anyone (of the few upstream KVM users) will run without it in the foreseeable future. The next step will be the removal of any !CONFIG_IOTHREAD section. So, how much do we want to invest here (provided my proposal has not remaining issues)? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop
On 2011-01-31 12:36, Jan Kiszka wrote:
> On 2011-01-31 11:08, Avi Kivity wrote:
>> On 01/27/2011 03:10 PM, Jan Kiszka wrote:
>>> Align with qemu-kvm and prepare for IO exit fix: There is no need to run
>>> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
>>> this service processes will first cause an exit from kvm_cpu_exec
>>> anyway. And we will have to reenter the kernel on IO exits
>>> unconditionally, something that the current logic prevents.
>>>
>>> Signed-off-by: Jan Kiszka
>>> ---
>>> kvm-all.c | 11 ++-
>>> 1 files changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index 5bfa8c0..46ecc1c 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
>>>
>>> DPRINTF("kvm_cpu_exec()\n");
>>>
>>> +if (kvm_arch_process_irqchip_events(env)) {
>>> +env->exit_request = 0;
>>> +env->exception_index = EXCP_HLT;
>>> +return 0;
>>> +}
>>> +
>>> do {
>>> #ifndef CONFIG_IOTHREAD
>>> if (env->exit_request) {
>>> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
>>> }
>>
>> We check for ->exit_request here
>>
>>> #endif
>>>
>>> -if (kvm_arch_process_irqchip_events(env)) {
>>> -ret = 0;
>>> -break;
>>> -}
>>> -
>>
>> But this checks for ->interrupt_request. What ensures that we exit when
>> ->interrupt_request is set?
>
> Good question, need to check again. But if that turns out to be an
> issue, qemu-kvm would be broken as well. I'm just aligning the code here.
>
The only thing we miss by moving process_irqchip_events is a self-INIT
of an AP - if such thing exists in real life. In that case, the AP would
cause a reset of itself, followed by a transition to HALT state.
A self-SIPI has no effect as A) a CPU can't send a SIPI from
wait-on-SIPI state and B) SIPIs are ignored in any other state.
Will post a version that additionally checks for pending INIT as well
and injects a self-ipi then.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCHv2] fix linuxboot.bin and multiboot.bin to not hijack int19
Currently linuxboot.bin and multiboot.bin option roms override int19 vector to intercept boot process. No sane option rom should do that. Provide bev entry instead that will be called by BIOS if option rom is selected for booting. Signed-off-by: Gleb Natapov --- Note that this patch should be applied after qemu will upgrade to Seabios that supports boot order. Otherwise there will be change in behavior since option rom will not be selected for booting by default. v1->v2: - change PRODUCT to BOOT_ROM_PRODUCT - move pnp header from OPTION_ROM_START to BOOT_ROM_START diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S index c109363..748c831 100644 --- a/pc-bios/optionrom/linuxboot.S +++ b/pc-bios/optionrom/linuxboot.S @@ -22,6 +22,8 @@ #include "optionrom.h" +#define BOOT_ROM_PRODUCT "Linux loader" + BOOT_ROM_START run_linuxboot: diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S index 9131837..cc5ca1b 100644 --- a/pc-bios/optionrom/multiboot.S +++ b/pc-bios/optionrom/multiboot.S @@ -20,6 +20,8 @@ #include "optionrom.h" +#define BOOT_ROM_PRODUCT "multiboot loader" + #define MULTIBOOT_MAGIC0x2badb002 #define GS_PROT_JUMP 0 diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h index fbdd48a..aa783de 100644 --- a/pc-bios/optionrom/optionrom.h +++ b/pc-bios/optionrom/optionrom.h @@ -97,22 +97,28 @@ #define BOOT_ROM_START \ OPTION_ROM_START\ - push%eax; \ - push%ds;\ - \ - /* setup ds so we can access the IVT */ \ - xor %ax, %ax; \ - mov %ax, %ds; \ - \ - /* install our int 19 handler */\ - movw$int19_handler, (0x19*4); \ - mov %cs, (0x19*4+2);\ - \ - pop %ds;\ - pop %eax; \ lret; \ - \ -int19_handler:;\ + .org0x18; \ + .short 0; \ + .short _pnph; \ +_pnph: \ + .ascii "$PnP"; \ + .byte 0x01; \ + .byte ( _pnph_len / 16 ); \ + .short 0x; \ + .byte 0x00; \ + .byte 0x00; \ + .long 0x; \ + .short _manufacturer; \ + .short _product; \ + .long 0x; \ + .short 0x; \ + .short 0x; \ + .short _bev; \ + .short 0x; \ + .short 0x; \ + .equ_pnph_len, . - _pnph; \ +_bev:; \ /* DS = CS */ \ movw%cs, %ax; \ movw%ax, %ds; @@ -122,5 +128,9 @@ _end: #define BOOT_ROM_END \ +_manufacturer:;\ + .asciz "QEMU"; \ +_product:; \ + .asciz BOOT_ROM_PRODUCT;\ OPTION_ROM_END -- Gleb.
[Qemu-devel] [PULL 0.14] pci
The following changes since commit 45d1aa828f8c94b082a0aa2dbf76535594ee45df: Update OpenBIOS images to r1018 (2011-01-30 13:10:10 +) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony Isaku Yamahata (2): pci: memory leak of PCIDevice::rom_file pci: typo in pcibus_get_dev_path() Michael S. Tsirkin (1): pci: bridge control fixup hw/pci.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] Re: [PATCH 01/22] Prevent abortion on multiple VCPU kicks
On 01/31/2011 01:19 PM, Jan Kiszka wrote:
On 2011-01-31 10:44, Avi Kivity wrote:
> On 01/27/2011 03:09 PM, Jan Kiszka wrote:
>> If we call qemu_cpu_kick more than once before the target was able to
>> process the signal, pthread_kill will fail, and qemu will abort. Prevent
>> this by avoiding the redundant signal.
>>
>
> Doesn't fit with the manual page (or with the idea that signals are
> asynchronous):
>
> NAME
> pthread_kill - send a signal to a thread
>
>
> ...
>
> ERRORS
> ESRCH No thread with the ID thread could be found.
>
> EINVAL An invalid signal was specified.
>
Valid remark, but I was receiving EAGAIN for blocked RT signals. Don't
know if this is Linux-specific. A quick glance at the man pages did not
reveal if this is allowed or at least gray area.
} else if (!is_si_special(info)) {
if (sig >= SIGRTMIN && info->si_code != SI_USER) {
/*
* Queue overflow, abort. We may abort if the
* signal was rt and sent by user using something
* other than kill().
*/
trace_signal_overflow_fail(sig, group, info);
return -EAGAIN;
}
However, even when selectively ignoring this, it's more efficient to
catch the redundant signaling in user space.
Yes.
--
error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 04/22] Leave inner main_loop faster on pending requests
On 01/31/2011 01:22 PM, Jan Kiszka wrote:
On 2011-01-31 10:52, Avi Kivity wrote:
> On 01/27/2011 03:09 PM, Jan Kiszka wrote:
>> If there is any pending request that requires us to leave the inner loop
>> if main_loop, makes sure we do this as soon as possible by enforcing
>> non-blocking IO processing.
>>
>> At this change, move variable definitions out of the inner loop to
>> improve readability.
>>
>> Signed-off-by: Jan Kiszka
>> ---
>>vl.c | 11 +++
>>1 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 5fad700..2ebc55b 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown;
>>
>>static void main_loop(void)
>>{
>> +bool nonblocking = false;
>> +#ifdef CONFIG_PROFILER
>> +int64_t ti;
>> +#endif
>>int r;
>>
>>qemu_main_loop_start();
>>
>>for (;;) {
>>do {
>> -bool nonblocking = false;
>> -#ifdef CONFIG_PROFILER
>> -int64_t ti;
>> -#endif
>>#ifndef CONFIG_IOTHREAD
>>nonblocking = cpu_exec_all();
>> +if (!vm_can_run()) {
>> +nonblocking = true;
>> +}
>
> Doesn't this cause vmstop to spin? We'll never execute
> main_loop_wait(false) if I read the code correctly?
>
The code path is not changed, we just poll instead of wait in
main_loop_wait.
Where do we wait then?
Also, I didn't get your error scenario yet. Even if we left the loop
here, what magic would main_loop_wait do to vmstop processing? The stop
request is handled outside the loop, that's why we should leave ASAP.
I'm just missing the alternate place where we sleep. If there's no such
place, we spin.
--
error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
On 01/31/2011 01:27 PM, Jan Kiszka wrote: On 2011-01-31 11:03, Avi Kivity wrote: > On 01/27/2011 04:33 PM, Jan Kiszka wrote: >> Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between >> checking for exit_request on vcpu entry and timer signals arriving >> before KVM starts to catch them. Plug it by blocking both timer related >> signals also on !CONFIG_IOTHREAD and process those via signalfd. >> >> As this fix depends on real signalfd support (otherwise the timer >> signals only kick the compat helper thread, and the main thread hangs), >> we need to detect the invalid constellation and abort configure. >> >> Signed-off-by: Jan Kiszka >> CC: Stefan Hajnoczi >> --- >> >> I don't want to invest that much into !IOTHREAD anymore, so let's see if >> the proposed catch&abort is acceptable. >> > > I don't understand the dependency on signalfd. The normal way of doing > things, either waiting for the signal in sigtimedwait() or in > ioctl(KVM_RUN), works with SIGALRM just fine. And how would you be kicked out of the select() call if it is waiting with a timeout? We only have a single thread here. If we use signalfd() (either kernel provided or thread+pipe), we kick out of select by select()ing it (though I don't see how it works without an iothread, since an fd can't stop a vcpu unless you enable SIGIO on it, which is silly for signalfd) If you leave it as a naked signal, then it can break out of either pselect() or vcpu. Since the goal is to drop !CONFIG_IOTHREAD, the first path seems better, I just don't understand the problem with emulated signalfd(). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
On Mon, Jan 31, 2011 at 12:18 PM, Jan Kiszka wrote: > On 2011-01-31 13:13, Stefan Hajnoczi wrote: >> On Mon, Jan 31, 2011 at 11:27 AM, Jan Kiszka wrote: >>> On 2011-01-31 11:03, Avi Kivity wrote: On 01/27/2011 04:33 PM, Jan Kiszka wrote: > Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between > checking for exit_request on vcpu entry and timer signals arriving > before KVM starts to catch them. Plug it by blocking both timer related > signals also on !CONFIG_IOTHREAD and process those via signalfd. > > As this fix depends on real signalfd support (otherwise the timer > signals only kick the compat helper thread, and the main thread hangs), > we need to detect the invalid constellation and abort configure. > > Signed-off-by: Jan Kiszka > CC: Stefan Hajnoczi > --- > > I don't want to invest that much into !IOTHREAD anymore, so let's see if > the proposed catch&abort is acceptable. > I don't understand the dependency on signalfd. The normal way of doing things, either waiting for the signal in sigtimedwait() or in ioctl(KVM_RUN), works with SIGALRM just fine. >>> >>> And how would you be kicked out of the select() call if it is waiting >>> with a timeout? We only have a single thread here. >>> >>> The only alternative is Stefan's original proposal. But that required >>> fiddling with the signal mask twice per KVM_RUN. >> >> I think my original patch messed with the sigmask in the wrong place, >> as you mentioned doing it twice per KVM_RUN isn't a good idea. I >> wonder if we can enable SIGALRM only in blocking calls and guest code >> execution but without signalfd. It might be possible, I don't see an >> immediate problem with doing that, we might have to use pselect(2) or >> similar in a few places. > > My main concern about alternative approaches is that IOTHREAD is about > to become the default, and hardly anyone (of the few upstream KVM users) > will run without it in the foreseeable future. The next step will be the > removal of any !CONFIG_IOTHREAD section. So, how much do we want to > invest here (provided my proposal has not remaining issues)? Yes, you're right. I'm not volunteering to dig more into this, the best case would be to switch to a non-I/O thread world that works for everybody. Stefan
Re: [Qemu-devel] [PATCH 0/4] new Blackfin QEMU port
On Mon, Jan 24, 2011 at 05:23:01AM -0500, Mike Frysinger wrote: > I finally got around to getting this port past the "only crashes" phase. > Using the GNU sim Blackfin port as a nice working standard, it was just > a matter of replacing the sim bits with TCG ops. Now we have a linux-user > port for people to play with. No immediate plans to move on to the system > step, but it is something I want to figure out at some point. > > The port is at the point I think where people can bang on it. The core > (bfin-sim.c) needs a bit more cleanup though before it'll be accepted, > but that should be largely related to the insns that I haven't gotten > around to implementing. The rest of the code should be fair game for > review though if someone feels like it. > > This does require some patches in order for userspace programs to be > generally useful, but they've all been submitted at this point. > > I don't know how people feel about test sizes. We developed quite a > comprehension testsuite for the GNU sim, and I've imported about half > of it. Personally, I find it invaluable to catch regressions while > developing, but its raw size can be a bit intimidating. > > As a simple comparison, we typically get about ~8mips with the GNU sim > while executing Blackfin code, but I easily see about ~400mips with qemu. > Obviously your numbers will vary based on workload, but hopefully not > too much ... > > My git tree with everything necessary can be found here: > git://sources.blackfin.uclinux.org/git/users/vapier/qemu.git Hi, First of all, nice work! Good to see that you've managed to get your port running. I've had a quick look at your git and have a few (mostly general) comments: * There's quite a bit of dead code left around. Please remove as much as possible to make things easier to read. * There's a lot of code that doesn't use braces around if code bodies, for example in op_helper.c. * Some operations seem to translate to large amounts of tcg code, maybe you should consider using helpers for those. An example is gen_rot_tl. * Many of the helpers seem to be PURE and CONST but not declared so. See tcg/README for more info. * Maybe you should rename target-bfin/opcode/bfin.h into target-bfin/opcodes.h? or similiar.. * gen_hwloop_check() seems to be looking at env->lbreg from the translator, is that OK? What happens if env->lbreg changes at runtime? * cpu_get_tb_cpu_state() doesn't define any tb flags? target-bfin/README: "There are some things we don't bother handling in the port for speed reasons. If you want an accurate (but not as fast) simulator, then use the GNU sim as found in the GNU toolchain (part of gdb)." I think QEMU should aim to be fast and correct. There is no need to be cycle accurate but emulation should IMO always aim to produce the correct results. If people provide patches to improve correctness, IMO those patches should be encouraged & accepted as long as they dont introduce unnecessary slowdowns (e.g, unless there are faster ways to achieve the same level of correctness). My suggestion is that you just remove those first sentences. "- transactional parallel instructions - on the hardware, if a load/store causes an exception, the other insns do not change register states either. in qemu, they do, but since those exceptions will kill the program anyways, who cares. no intermediate store buffers!" This might be true for user emulation but I assume you'd want to fix this for system emulation (at some point in time). Maybe you should just note that it's not supported and leave it at that. "- unaligned memory access exceptions - qemu itself doesn't support this for targets" There are targets that emulate trapping on unaligned memory accesses, for example MicroBlaze and IIRC SPARC. Cheers
Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
On 31.01.2011 10:44, Aurelien Jarno wrote: > On Mon, Jan 31, 2011 at 10:35:30AM +0100, Christophe Lyon wrote: >> On 31.01.2011 09:20, Aurelien Jarno wrote: >>> On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.l...@st.com wrote: From: Christophe Lyon Handle corner cases where the addition of the rounding constant could cause overflows. >>> >>> After applying this patch, I get the following gcc warning: >>> CCtranslate.o >>> cc1: warnings being treated as errors >>> qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’: >>> qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in >>> this function >>> make: *** [translate.o] Error 1 >>> >> >> Which GCC version are you using? I don't have such a warning (using >> GCC-4.5.1 on x86_64). >> > > I get this error with GCC 4.3.5, GCC 4.4.5, GCC 4.5.2 and GCC 4.6.0 > (r169270). This is also on x86_64. > Well, I can't reproduce this error :-( For the record, I configure with --target-list=arm-softmmu,arm-linux-user --disable-bluez --enable-debug --disable-sdl and point --host-cc and --cc to GCC-4.5.1. In verbose mode, I confirm that GCC is invoked with -Werror -m64 -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fstack-protector-all -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits I have tried from a freshly cloned qemu.git, and I have no error. Can you send me your translate.c & translate.i (pre-processed by GCC's C-preprocesssor with -E option) Christophe.
[Qemu-devel] Re: [0.14] Call for testing volunteers
On Thu, Jan 27, 2011 at 8:04 PM, Stefan Hajnoczi wrote: > Let's plan testing for the upcoming 0.14 release here: > > http://wiki.qemu.org/Planning/0.14/Testing > > It's easy. Add yourself for the testing you are willing to do on the > release candidates. Folks have started to add the QEMU 0.14 release candidate testing they intend to do on the wiki page. There are still plenty of areas to help test, hence this shameless bump :). Stefan
[Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
On 2011-01-31 14:22, Avi Kivity wrote: > On 01/31/2011 01:27 PM, Jan Kiszka wrote: >> On 2011-01-31 11:03, Avi Kivity wrote: >>> On 01/27/2011 04:33 PM, Jan Kiszka wrote: Found by Stefan Hajnoczi: There is a race in kvm_cpu_exec between checking for exit_request on vcpu entry and timer signals arriving before KVM starts to catch them. Plug it by blocking both timer related signals also on !CONFIG_IOTHREAD and process those via signalfd. As this fix depends on real signalfd support (otherwise the timer signals only kick the compat helper thread, and the main thread hangs), we need to detect the invalid constellation and abort configure. Signed-off-by: Jan Kiszka CC: Stefan Hajnoczi --- I don't want to invest that much into !IOTHREAD anymore, so let's see if the proposed catch&abort is acceptable. >>> >>> I don't understand the dependency on signalfd. The normal way of doing >>> things, either waiting for the signal in sigtimedwait() or in >>> ioctl(KVM_RUN), works with SIGALRM just fine. >> >> And how would you be kicked out of the select() call if it is waiting >> with a timeout? We only have a single thread here. > > If we use signalfd() (either kernel provided or thread+pipe), we kick > out of select by select()ing it (though I don't see how it works without > an iothread, since an fd can't stop a vcpu unless you enable SIGIO on > it, which is silly for signalfd) > > If you leave it as a naked signal, then it can break out of either > pselect() or vcpu. > > Since the goal is to drop !CONFIG_IOTHREAD, the first path seems better, > I just don't understand the problem with emulated signalfd(). > With the emulated signalfd, there won't be any signal for the VCPU while in KVM_RUN. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH 04/22] Leave inner main_loop faster on pending requests
On 2011-01-31 14:17, Avi Kivity wrote:
> On 01/31/2011 01:22 PM, Jan Kiszka wrote:
>> On 2011-01-31 10:52, Avi Kivity wrote:
>>> On 01/27/2011 03:09 PM, Jan Kiszka wrote:
If there is any pending request that requires us to leave the inner loop
if main_loop, makes sure we do this as soon as possible by enforcing
non-blocking IO processing.
At this change, move variable definitions out of the inner loop to
improve readability.
Signed-off-by: Jan Kiszka
---
vl.c | 11 +++
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/vl.c b/vl.c
index 5fad700..2ebc55b 100644
--- a/vl.c
+++ b/vl.c
@@ -1384,18 +1384,21 @@ qemu_irq qemu_system_powerdown;
static void main_loop(void)
{
+bool nonblocking = false;
+#ifdef CONFIG_PROFILER
+int64_t ti;
+#endif
int r;
qemu_main_loop_start();
for (;;) {
do {
-bool nonblocking = false;
-#ifdef CONFIG_PROFILER
-int64_t ti;
-#endif
#ifndef CONFIG_IOTHREAD
nonblocking = cpu_exec_all();
+if (!vm_can_run()) {
+nonblocking = true;
+}
>>>
>>> Doesn't this cause vmstop to spin? We'll never execute
>>> main_loop_wait(false) if I read the code correctly?
>>>
>>
>> The code path is not changed, we just poll instead of wait in
>> main_loop_wait.
>
> Where do we wait then?
>
>> Also, I didn't get your error scenario yet. Even if we left the loop
>> here, what magic would main_loop_wait do to vmstop processing? The stop
>> request is handled outside the loop, that's why we should leave ASAP.
>
> I'm just missing the alternate place where we sleep. If there's no such
> place, we spin.
>
Probably you are misled by the name of vm_can_run. It should better be
renamed to requests_pending or something like that - iow, it is only
true if some request is pending and not if just the vm is in stop mode.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC][PATCH v6 00/23] virtagent: host/guest RPC communication agent
On 01/18/2011 08:13 AM, Anthony Liguori wrote:
On 01/18/2011 08:02 AM, Gerd Hoffmann wrote:
On 01/17/11 15:53, Michael Roth wrote:
On 01/17/2011 07:53 AM, Gerd Hoffmann wrote:
What is your plan to handle system-level queries+actions (such as
reboot) vs. per-user stuff (such as cut+paste)?
This is an area that hasn't been well-defined yet and is definitely open
for suggestions.
One option would be to have two virtio-serial channels, one for the
system and one for the user stuff. gdm could grant the desktop user
access to the user channel like it does with sound devices and simliar
stuff, so the user agent has access to it.
Another option is to have some socket where the user agent can talk to
the system agent and have it relay the requests.
I think this is the best approach. One requirement we've been working
with is that all actions from guest agents are logged. This is to give
an administrator confidence that the hypervisor isn't doing anything
stupid. If you route all of the user traffic through a privileged
daemon, you can log everything to syslog or an appropriate log file.
A brain-dump on how we're planning to do this. I tried to keep it
concise, but that only went so far :)
We extend qemu-va, the virtagent guest agent, to be able to create a
unix socket in the guest that listens for connections.
We also extend qemu-va so that it can run as a user-level daemon that
can connect to the aforementioned socket.
root: qemu-va -c virtio-serial -p
/dev/virtio-ports/org.qemu.virtagent
--user-socket=/var/run/virtagent.user.sock
user: qemu-va -c unix-connect -p /var/run/virtagent.user.sock
[--username=user] --user-daemon
The user-level daemon will drop priviledges to specified username if
necessary, or run as the invoking user. User->daemon mappings will be
determined by the system-level agent using getsockopt on new connections.
The user-level daemon operates in the same manner as the system-level
daemon: it can execute RPCs in the host, and the host can execute RPCs
it implements. This should theoretically make it capable of vdagent-like
functionality by adding the appropriate host/guest-side RPCs. We would
have to incorporate the X11 fd into the main select() for events and
such, but that should be fairly straight-forward.
User-specific host->guest RPCs will add an http header,
"X-Virtagent-Username", specifying the intended recipient. If that
recipient has no associated running user-level daemon, an error will be
returned by the system-level daemon. An RPC may be provided so the host
can query the system-level agent for connected users.
User-generated guest->host RPCs will be routed directly to the host,
with the same "X-Virtagent-Username" http header inserted by the
system-level agent. The host will re-insert this header field in it's
responses so the system-level agent can route the responses back to the
user.
We will only allow one daemon per user at a time, and enforce this with
a pid/lock file. If this is circumvented somehow (or we allow a
user-specifiable pid file), a new connection specifying a user that's
already been mapped to a user-level daemon will overwrite the existing
mapping, and the old connection will be closed by the system-level
daemon if it is still active.
Some caveats to consider however:
1) There may be user-level RPCs that are generally desirable...such as
being able to execute a script as an unpriveledged user, or access a
file in the same way. So it makes sense to invoke user-level agent
startup with a login script. But for X-related things like Spice, an
.xsession hook makes more sense. If we do both, the .xsession-spawned
daemon will "take over", but any existing stateful interactions with
that user will be effectively reset. This may be undesirable. One option
is to XOpenDisplay() "on demand", rather than at startup. We won't know
what $env{DISPLAY} is then, however, and there could be multiple
displays for that user. I'm not sure if there's a reliable way to query
such a thing, but assuming you can...we could implement stateful RPCs so
establish a connection later: There could be multiple displays as well.
My first inclination would be to have something like:
on host:
query_displays(user)
connect_display(user, display_name)
query_screens(user, display_name)
etc..
This requires either the host to poll for displays, or for the guest
agent to do it and then notify the host, however. The only way I see
around this is to only start the user-level daemon via .xsession or similar.
2) Pulling X11 dependencies into virtagent...I'm not sure how well this
would go over. We may end up needing to move qemu-va into a seperate
repo that pulls in deps from qemu.git
-Mike
Re: [Qemu-devel] [PATCH 1/3] FVD: Added support for 'qemu-img update'
> After thinking about it more, qemu-img update does also serve a > purpose. Sometimes it is necessary to set options on many images in > bulk or from provisioning scripts instead of at runtime. > > I guess my main fear of qemu-img update is that it adds a new > interface that only FVD exploits so far. If it never catches on with > other formats then we have this special feature that must be > maintained but is rarely used. I'd hold off this patch until code > that can make use of it has been merged into qemu.git. I am fine with holding it off. Actually, 'qemu-img rebase' is already a special type of 'qemu-img update'. Without a general update interface, every parameter change would have to use a special interface like 'rebase'. > There's a lot of room for studying the behavior and making > improvements. Coming up with throttling strategies that make the > prefetch I/O an "idle task" only when there's bandwidth available is > difficult because the problem is more complex than just one greedy > QEMU process. In a cloud environment there will be any physical > hosts, each with multiple VMs, on a shared network and no single QEMU > process has global knowledge. It's more like TCP where you need to > try seeing how much data the connection can carry, fall back on packet > loss, and then gradually try again. But I'm not sure we have a > feedback mechanism to say "you're doing too much prefetching". Your observation on the similarity to TCP is incisive. To my knowledge, the two papers below are the most relevant work on congestion control for VM storage. I evaluated [1] below, and found that it is not most reliable. I copied some text from my paper below. Basically, since a storage system does not have packet loss, there are two ways of detecting congestion in a storage system: 1) making decisions based on increased latency, or 2) making decisions based on reduced throughput. The paper [1] below uses latency but I found it not always reliable. The current implementation in FVD is based on throughput, which tends to be more reliable. But as you said,this is still a problem that has a lot of room for future study. === "[1] also performs adaptive prefetching. It halves the prefetch rate if a certain “percentage” of recent requests experiencing a high latency. Our experiments show that it is hard to set a proper “percentage” to reliably detect contentions. Because storage servers and disk controllers perform readahead in large chunks for sequential reads, a very large percentage (e.g., 90%) of a VM’s prefetching reads hit in read-ahead caches and experience a low latency. When a storage server becomes busy, the “percentage” of requests that hit in read-ahead caches may change little, but the response time of those cache-miss requests may increase dramatically. In other words, this “percentage” does not correlate well with the achieved disk I/O throughput." The paper [2] below from VMware is informative but cannot be adopted by us directly, as the problem domain is different. I previously had a paper on general congestion control, https://sites.google.com/site/tangchq/papers/NCI-USENIX09.pdf?attredirects=0 , and attended a conference together with an author of paper [2] , and had some discussions. It is an interesting paper. [1] http://suif.stanford.edu/papers/nsdi05.pdf . [2] http://www.usenix.org/events/fast09/tech/full_papers/gulati/gulati.pdf Regards, ChunQiang (CQ) Tang Homepage: http://www.research.ibm.com/people/c/ctang
Re: [Qemu-devel] [PATCH] pcibus_get_dev_path: correct pci device path construction
On 18/01/11 21:10, Igor V. Kovalenko wrote:
From: Igor V. Kovalenko
- fix snprintf off by one
pci domain and slot number formatting snprintf calls
require extra space for trailing null character
without this change devices are assigned the same path name
which triggers assertion in vmstate_register_with_alias_id
- while iterating over devices from root pci device
use PCI_SLOT and PCI_FUNC of each device on the path
instead of always extracting PCI_FUNC of original device
Signed-off-by: Igor V. Kovalenko
---
hw/pci.c | 28
1 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 8d0e3df..182ee25 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -2036,6 +2036,8 @@ static char *pcibus_get_dev_path(DeviceState *dev)
int slot_len = strlen(":SS.F");
int path_len;
char *path, *p;
+PCIDevice** pci_path;
+int i;
/* Calculate # of slots on path between device and root. */;
slot_depth = 0;
@@ -2045,21 +2047,31 @@ static char *pcibus_get_dev_path(DeviceState *dev)
path_len = domain_len + slot_len * slot_depth;
-/* Allocate memory, fill in the terminating null byte. */
+/* Allocate memory. String will be null-terminated by snprintf calls. */
path = malloc(path_len + 1 /* For '\0' */);
-path[path_len] = '\0';
/* First field is the domain. */
-snprintf(path, domain_len, "%04x:00", pci_find_domain(d->bus));
+snprintf(path, domain_len+1, "%04x:00", pci_find_domain(d->bus));
+
+/* Store pci devices on the path walking up from device to root.
+ * We need them later in the reverse order, last to first. */
+pci_path = qemu_malloc(slot_depth * sizeof(PCIDevice *));
-/* Fill in slot numbers. We walk up from device to root, so need to print
- * them in the reverse order, last to first. */
-p = path + path_len;
+i = slot_depth;
for (t = d; t; t = t->bus->parent_dev) {
-p -= slot_len;
-snprintf(p, slot_len, ":%02x.%x", PCI_SLOT(t->devfn),
PCI_FUNC(d->devfn));
+pci_path[--i] = t;
}
+/* Fill in slot numbers using stored path from root pci device. */
+p = path + domain_len;
+for (i = 0; i< slot_depth; ++i) {
+t = pci_path[i];
+snprintf(p + i * slot_len, slot_len+1,
+ ":%02x.%x", PCI_SLOT(t->devfn), PCI_FUNC(t->devfn));
+}
+
+qemu_free(pci_path);
+
return path;
}
Has anyone had a chance to look at this patch yet?
ATB,
Mark.
--
Mark Cave-Ayland - Senior Technical Architect
PostgreSQL - PostGIS
Sirius Corporation plc - control through freedom
http://www.siriusit.co.uk
t: +44 870 608 0063
Sirius Labs: http://www.siriusit.co.uk/labs
[Qemu-devel] [PATCH 01/11] .gitignore: ignore vi swap files and ctags files
Signed-off-by: Dmitry Eremin-Solenikov --- .gitignore |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 3efb4ec..26703e1 100644 --- a/.gitignore +++ b/.gitignore @@ -51,6 +51,8 @@ QMP/qmp-commands.txt *.vr *.d *.o +*.swp +*.orig .pc patches pc-bios/bios-pq/status @@ -60,3 +62,4 @@ pc-bios/optionrom/multiboot.bin pc-bios/optionrom/multiboot.raw .stgit-* cscope.* +tags -- 1.7.2.3
[Qemu-devel] [PATCH 02/11] sysbus: print amount of irqs in dev_print
Signed-off-by: Dmitry Eremin-Solenikov
---
hw/sysbus.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/hw/sysbus.c b/hw/sysbus.c
index 1583bd8..1928b51 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -178,6 +178,7 @@ static void sysbus_dev_print(Monitor *mon, DeviceState
*dev, int indent)
SysBusDevice *s = sysbus_from_qdev(dev);
int i;
+monitor_printf(mon, "%*sirq %d\n", indent, "", s->num_irq);
for (i = 0; i < s->num_mmio; i++) {
monitor_printf(mon, "%*smmio " TARGET_FMT_plx "/" TARGET_FMT_plx "\n",
indent, "", s->mmio[i].addr, s->mmio[i].size);
--
1.7.2.3
[Qemu-devel] [PATCH 03/11] arm: drop unused irq-related part of CPUARMState
These two fields were added as a part of ARMv7 support patch (back in
2007), were never used by any code, so can be dropped.
Signed-off-by: Dmitry Eremin-Solenikov
---
target-arm/cpu.h |4
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5bcd53a..83d9982 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -157,10 +157,6 @@ typedef struct CPUARMState {
/* Internal CPU feature flags. */
uint32_t features;
-/* Callback for vectored interrupt controller. */
-int (*get_irq_vector)(struct CPUARMState *);
-void *irq_opaque;
-
/* VFP coprocessor state. */
struct {
float64 regs[32];
--
1.7.2.3
[Qemu-devel] [PATCH 04/11] arm-pic: add one extra interrupt to support EXITTB interrupts
Some ARM processors (consider PXA2xx, Omap1, etc.) want to be able to send
CPU_INTERRUPT_EXITTB to the cpu. Support doing that through common arm_pic.
Signed-off-by: Dmitry Eremin-Solenikov
---
hw/arm-misc.h |1 +
hw/arm_pic.c |6 +-
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index 010acb4..f2e45ee 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -14,6 +14,7 @@
/* The CPU is also modeled as an interrupt controller. */
#define ARM_PIC_CPU_IRQ 0
#define ARM_PIC_CPU_FIQ 1
+#define ARM_PIC_CPU_WAKE 2
qemu_irq *arm_pic_init_cpu(CPUState *env);
/* armv7m.c */
diff --git a/hw/arm_pic.c b/hw/arm_pic.c
index f44568c..bd5ce55 100644
--- a/hw/arm_pic.c
+++ b/hw/arm_pic.c
@@ -38,6 +38,10 @@ static void arm_pic_cpu_handler(void *opaque, int irq, int
level)
else
cpu_reset_interrupt(env, CPU_INTERRUPT_FIQ);
break;
+case ARM_PIC_CPU_WAKE:
+if (env->halted && level)
+cpu_interrupt(env, CPU_INTERRUPT_EXITTB);
+ break;
default:
hw_error("arm_pic_cpu_handler: Bad interrput line %d\n", irq);
}
@@ -45,5 +49,5 @@ static void arm_pic_cpu_handler(void *opaque, int irq, int
level)
qemu_irq *arm_pic_init_cpu(CPUState *env)
{
-return qemu_allocate_irqs(arm_pic_cpu_handler, env, 2);
+return qemu_allocate_irqs(arm_pic_cpu_handler, env, 3);
}
--
1.7.2.3
[Qemu-devel] [PATCH 09/11] Drop unnecessary inclusions of pxa.h header
Seceral files contained onnecessary dependencies on hw/pxa.h header.
Drop unused references.
Signed-off-by: Dmitry Eremin-Solenikov
---
hw/mainstone.c |2 +-
hw/mainstone.h |2 +-
hw/mst_fpga.c |9 -
hw/tc6393xb.c |1 -
hw/zaurus.c|1 -
5 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/hw/mainstone.c b/hw/mainstone.c
index 58e3f86..61cac8a 100644
--- a/hw/mainstone.c
+++ b/hw/mainstone.c
@@ -117,7 +117,7 @@ static void mainstone_common_init(ram_addr_t ram_size,
}
}
-mst_irq = mst_irq_init(cpu, MST_FPGA_PHYS, PXA2XX_PIC_GPIO_0);
+mst_irq = mst_irq_init(MST_FPGA_PHYS, qdev_get_gpio_in(cpu->pic,
PXA2XX_PIC_GPIO_0));
/* setup keypad */
printf("map addr %p\n", &map);
diff --git a/hw/mainstone.h b/hw/mainstone.h
index 9618c06..35329f1 100644
--- a/hw/mainstone.h
+++ b/hw/mainstone.h
@@ -33,6 +33,6 @@
#define S1_IRQ15
extern qemu_irq
-*mst_irq_init(PXA2xxState *cpu, uint32_t base, int irq);
+*mst_irq_init(uint32_t base, qemu_irq irq);
#endif /* __MAINSTONE_H__ */
diff --git a/hw/mst_fpga.c b/hw/mst_fpga.c
index 67b544f..3a49c84 100644
--- a/hw/mst_fpga.c
+++ b/hw/mst_fpga.c
@@ -9,7 +9,6 @@
*/
#include "hw.h"
#include "qdev.h"
-#include "pxa.h"
#include "mainstone.h"
/* Mainstone FPGA for extern irqs */
@@ -110,7 +109,7 @@ mst_fpga_readb(void *opaque, target_phys_addr_t addr)
return s->pcmcia1;
default:
printf("Mainstone - mst_fpga_readb: Bad register offset "
- REG_FMT " \n", addr);
+ "0x" TARGET_FMT_plx " \n", addr);
}
return 0;
}
@@ -161,7 +160,7 @@ mst_fpga_writeb(void *opaque, target_phys_addr_t addr,
uint32_t value)
break;
default:
printf("Mainstone - mst_fpga_writeb: Bad register offset "
- REG_FMT " \n", addr);
+ "0x" TARGET_FMT_plx " \n", addr);
}
}
@@ -217,7 +216,7 @@ mst_fpga_load(QEMUFile *f, void *opaque, int version_id)
return 0;
}
-qemu_irq *mst_irq_init(PXA2xxState *cpu, uint32_t base, int irq)
+qemu_irq *mst_irq_init(uint32_t base, qemu_irq irq)
{
mst_irq_state *s;
int iomemtype;
@@ -226,7 +225,7 @@ qemu_irq *mst_irq_init(PXA2xxState *cpu, uint32_t base, int
irq)
s = (mst_irq_state *)
qemu_mallocz(sizeof(mst_irq_state));
- s->parent = qdev_get_gpio_in(cpu->pic, irq);
+ s->parent = irq;
/* alloc the external 16 irqs */
qi = qemu_allocate_irqs(mst_fpga_set_irq, s, MST_NUM_IRQS);
diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c
index 0cadcde..ed49e94 100644
--- a/hw/tc6393xb.c
+++ b/hw/tc6393xb.c
@@ -8,7 +8,6 @@
* This code is licensed under the GNU GPL v2.
*/
#include "hw.h"
-#include "pxa.h"
#include "devices.h"
#include "flash.h"
#include "console.h"
diff --git a/hw/zaurus.c b/hw/zaurus.c
index 90fedc9..5d65793 100644
--- a/hw/zaurus.c
+++ b/hw/zaurus.c
@@ -16,7 +16,6 @@
* with this program; if not, see
[Qemu-devel] [PATCH 05/11] pxa2xx_pic: update to use qdev and arm-pic
pxa2xx_pic duplicated some code from arm-pic. Drop it, replacing with
references to arm-pic. Also use qdev/sysbus framework to handle
pxa2xx-pic.
Signed-off-by: Dmitry Eremin-Solenikov
---
hw/pxa2xx_pic.c | 125 +--
1 files changed, 66 insertions(+), 59 deletions(-)
diff --git a/hw/pxa2xx_pic.c b/hw/pxa2xx_pic.c
index a36da23..5b5ce72 100644
--- a/hw/pxa2xx_pic.c
+++ b/hw/pxa2xx_pic.c
@@ -10,6 +10,8 @@
#include "hw.h"
#include "pxa.h"
+#include "arm-misc.h"
+#include "sysbus.h"
#define ICIP 0x00/* Interrupt Controller IRQ Pending register */
#define ICMR 0x04/* Interrupt Controller Mask register */
@@ -31,7 +33,10 @@
#define PXA2XX_PIC_SRCS40
typedef struct {
-CPUState *cpu_env;
+SysBusDevice busdev;
+qemu_irq hard_irq;
+qemu_irq fiq_irq;
+qemu_irq wake_irq;
uint32_t int_enabled[2];
uint32_t int_pending[2];
uint32_t is_fiq[2];
@@ -44,25 +49,16 @@ static void pxa2xx_pic_update(void *opaque)
uint32_t mask[2];
PXA2xxPICState *s = (PXA2xxPICState *) opaque;
-if (s->cpu_env->halted) {
-mask[0] = s->int_pending[0] & (s->int_enabled[0] | s->int_idle);
-mask[1] = s->int_pending[1] & (s->int_enabled[1] | s->int_idle);
-if (mask[0] || mask[1])
-cpu_interrupt(s->cpu_env, CPU_INTERRUPT_EXITTB);
-}
+mask[0] = s->int_pending[0] & (s->int_enabled[0] | s->int_idle);
+mask[1] = s->int_pending[1] & (s->int_enabled[1] | s->int_idle);
+qemu_set_irq(s->wake_irq, mask[0] || mask[1]);
mask[0] = s->int_pending[0] & s->int_enabled[0];
mask[1] = s->int_pending[1] & s->int_enabled[1];
-if ((mask[0] & s->is_fiq[0]) || (mask[1] & s->is_fiq[1]))
-cpu_interrupt(s->cpu_env, CPU_INTERRUPT_FIQ);
-else
-cpu_reset_interrupt(s->cpu_env, CPU_INTERRUPT_FIQ);
+qemu_set_irq(s->fiq_irq, ((mask[0] & s->is_fiq[0]) || (mask[1] &
s->is_fiq[1])));
-if ((mask[0] & ~s->is_fiq[0]) || (mask[1] & ~s->is_fiq[1]))
-cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
-else
-cpu_reset_interrupt(s->cpu_env, CPU_INTERRUPT_HARD);
+qemu_set_irq(s->hard_irq, ((mask[0] & ~s->is_fiq[0]) || (mask[1] &
~s->is_fiq[1])));
}
/* Note: Here level means state of the signal on a pin, not
@@ -241,53 +237,36 @@ static CPUWriteMemoryFunc * const pxa2xx_pic_writefn[] = {
pxa2xx_pic_mem_write,
};
-static void pxa2xx_pic_save(QEMUFile *f, void *opaque)
-{
-PXA2xxPICState *s = (PXA2xxPICState *) opaque;
-int i;
-
-for (i = 0; i < 2; i ++)
-qemu_put_be32s(f, &s->int_enabled[i]);
-for (i = 0; i < 2; i ++)
-qemu_put_be32s(f, &s->int_pending[i]);
-for (i = 0; i < 2; i ++)
-qemu_put_be32s(f, &s->is_fiq[i]);
-qemu_put_be32s(f, &s->int_idle);
-for (i = 0; i < PXA2XX_PIC_SRCS; i ++)
-qemu_put_be32s(f, &s->priority[i]);
-}
-
-static int pxa2xx_pic_load(QEMUFile *f, void *opaque, int version_id)
+static int pxa2xx_pic_post_load(void *opaque, int version_id)
{
-PXA2xxPICState *s = (PXA2xxPICState *) opaque;
-int i;
-
-for (i = 0; i < 2; i ++)
-qemu_get_be32s(f, &s->int_enabled[i]);
-for (i = 0; i < 2; i ++)
-qemu_get_be32s(f, &s->int_pending[i]);
-for (i = 0; i < 2; i ++)
-qemu_get_be32s(f, &s->is_fiq[i]);
-qemu_get_be32s(f, &s->int_idle);
-for (i = 0; i < PXA2XX_PIC_SRCS; i ++)
-qemu_get_be32s(f, &s->priority[i]);
-
pxa2xx_pic_update(opaque);
return 0;
}
qemu_irq *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env)
{
-PXA2xxPICState *s;
-int iomemtype;
qemu_irq *qi;
+DeviceState *dev;
-s = (PXA2xxPICState *)
-qemu_mallocz(sizeof(PXA2xxPICState));
-if (!s)
-return NULL;
+qi = arm_pic_init_cpu(env);
-s->cpu_env = env;
+dev = sysbus_create_varargs("pxa2xx_pic", base,
+qi[ARM_PIC_CPU_IRQ],
+qi[ARM_PIC_CPU_FIQ],
+qi[ARM_PIC_CPU_WAKE],
+NULL);
+
+/* Enable IC coprocessor access. */
+cpu_arm_set_cp_io(env, 6, pxa2xx_pic_cp_read, pxa2xx_pic_cp_write, dev);
+
+/* FIXME */
+return dev->gpio_in;
+}
+
+static int pxa2xx_pic_initfn(SysBusDevice *dev)
+{
+PXA2xxPICState *s = FROM_SYSBUS(PXA2xxPICState, dev);
+int iomemtype;
s->int_pending[0] = 0;
s->int_pending[1] = 0;
@@ -296,18 +275,46 @@ qemu_irq *pxa2xx_pic_init(target_phys_addr_t base,
CPUState *env)
s->is_fiq[0] = 0;
s->is_fiq[1] = 0;
-qi = qemu_allocate_irqs(pxa2xx_pic_set_irq, s, PXA2XX_PIC_SRCS);
+sysbus_init_irq(dev, &s->hard_irq);
+sysbus_init_irq(dev, &s->fiq_irq);
+sysbus_init_irq(dev, &s->wake_irq);
+
+qdev_init_gpio_in(&dev->qdev, pxa2xx_pic_set_irq, PXA2XX_PIC_SRCS);
/* Enable IC memory-mapped registers access. */
iomemtype = cpu_register_io_memory(pxa2xx_pic_readfn,
pxa2xx_pic_writefn, s, DEVICE_NATIVE_ENDIAN);
-cpu_regist
[Qemu-devel] [PATCH 11/11] Merge mainstone.h header into mainstone.c
Now the only user of mainstone.h is mainstone.c file. Merge header
into board file.
Signed-off-by: Dmitry Eremin-Solenikov
---
hw/mainstone.c | 23 ++-
hw/mainstone.h | 35 ---
2 files changed, 22 insertions(+), 36 deletions(-)
delete mode 100644 hw/mainstone.h
diff --git a/hw/mainstone.c b/hw/mainstone.c
index 7d13972..6d10ecb 100644
--- a/hw/mainstone.c
+++ b/hw/mainstone.c
@@ -14,12 +14,33 @@
#include "net.h"
#include "devices.h"
#include "boards.h"
-#include "mainstone.h"
#include "sysemu.h"
#include "flash.h"
#include "blockdev.h"
#include "sysbus.h"
+/* Device addresses */
+#define MST_FPGA_PHYS 0x0800
+#define MST_ETH_PHYS 0x1300
+#define MST_FLASH_00x
+#define MST_FLASH_10x0400
+
+/* IRQ definitions */
+#define MMC_IRQ 0
+#define USIM_IRQ 1
+#define USBC_IRQ 2
+#define ETHERNET_IRQ 3
+#define AC97_IRQ 4
+#define PEN_IRQ 5
+#define MSINS_IRQ 6
+#define EXBRD_IRQ 7
+#define S0_CD_IRQ 9
+#define S0_STSCHG_IRQ 10
+#define S0_IRQ11
+#define S1_CD_IRQ 13
+#define S1_STSCHG_IRQ 14
+#define S1_IRQ15
+
static struct keymap map[0xE0] = {
[0 ... 0xDF] = { -1, -1 },
[0x1e] = {0,0}, /* a */
diff --git a/hw/mainstone.h b/hw/mainstone.h
deleted file mode 100644
index e6a2b67..000
--- a/hw/mainstone.h
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * PXA270-based Intel Mainstone platforms.
- *
- * Copyright (c) 2007 by Armin Kuster or
- *
- *
- * This code is licensed under the GNU GPL v2.
- */
-
-#ifndef __MAINSTONE_H__
-#define __MAINSTONE_H__
-
-/* Device addresses */
-#define MST_FPGA_PHYS 0x0800
-#define MST_ETH_PHYS 0x1300
-#define MST_FLASH_00x
-#define MST_FLASH_10x0400
-
-/* IRQ definitions */
-#define MMC_IRQ 0
-#define USIM_IRQ 1
-#define USBC_IRQ 2
-#define ETHERNET_IRQ 3
-#define AC97_IRQ 4
-#define PEN_IRQ 5
-#define MSINS_IRQ 6
-#define EXBRD_IRQ 7
-#define S0_CD_IRQ 9
-#define S0_STSCHG_IRQ 10
-#define S0_IRQ11
-#define S1_CD_IRQ 13
-#define S1_STSCHG_IRQ 14
-#define S1_IRQ15
-
-#endif /* __MAINSTONE_H__ */
--
1.7.2.3
[Qemu-devel] [PATCH 06/11] pxa2xx_pic: fully encapsulate pic into DeviceState
Currently pxa2xx_pic exposes it's internal gpio_in array. Replace all
references to the array with calls to qdev_get_gpio_in().
Signed-off-by: Dmitry Eremin-Solenikov
---
hw/mst_fpga.c |7 ---
hw/pxa.h | 10 +-
hw/pxa2xx.c | 53 ++---
hw/pxa2xx_gpio.c |8
hw/pxa2xx_pic.c |5 ++---
hw/pxa2xx_timer.c | 16
6 files changed, 49 insertions(+), 50 deletions(-)
diff --git a/hw/mst_fpga.c b/hw/mst_fpga.c
index 5252fc5..67b544f 100644
--- a/hw/mst_fpga.c
+++ b/hw/mst_fpga.c
@@ -8,6 +8,7 @@
* This code is licensed under the GNU GPL v2.
*/
#include "hw.h"
+#include "qdev.h"
#include "pxa.h"
#include "mainstone.h"
@@ -28,7 +29,7 @@
#define MST_PCMCIA10xe4
typedef struct mst_irq_state{
- qemu_irq *parent;
+ qemu_irq parent;
qemu_irq *pins;
uint32_t prev_level;
@@ -72,7 +73,7 @@ mst_fpga_set_irq(void *opaque, int irq, int level)
if(s->intmskena & (1u << irq)) {
s->intsetclr = 1u << irq;
- qemu_set_irq(s->parent[0], level);
+ qemu_set_irq(s->parent, level);
}
}
@@ -225,7 +226,7 @@ qemu_irq *mst_irq_init(PXA2xxState *cpu, uint32_t base, int
irq)
s = (mst_irq_state *)
qemu_mallocz(sizeof(mst_irq_state));
- s->parent = &cpu->pic[irq];
+ s->parent = qdev_get_gpio_in(cpu->pic, irq);
/* alloc the external 16 irqs */
qi = qemu_allocate_irqs(mst_fpga_set_irq, s, MST_NUM_IRQS);
diff --git a/hw/pxa.h b/hw/pxa.h
index f73d33b..e9a6f7d 100644
--- a/hw/pxa.h
+++ b/hw/pxa.h
@@ -63,15 +63,15 @@
# define PXA2XX_INTERNAL_SIZE 0x4
/* pxa2xx_pic.c */
-qemu_irq *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env);
+DeviceState *pxa2xx_pic_init(target_phys_addr_t base, CPUState *env);
/* pxa2xx_timer.c */
-void pxa25x_timer_init(target_phys_addr_t base, qemu_irq *irqs);
-void pxa27x_timer_init(target_phys_addr_t base, qemu_irq *irqs, qemu_irq irq4);
+void pxa25x_timer_init(target_phys_addr_t base, DeviceState *pic);
+void pxa27x_timer_init(target_phys_addr_t base, DeviceState *pic);
/* pxa2xx_gpio.c */
DeviceState *pxa2xx_gpio_init(target_phys_addr_t base,
-CPUState *env, qemu_irq *pic, int lines);
+CPUState *env, DeviceState *pic, int lines);
void pxa2xx_gpio_read_notifier(DeviceState *dev, qemu_irq handler);
/* pxa2xx_dma.c */
@@ -125,7 +125,7 @@ typedef struct PXA2xxFIrState PXA2xxFIrState;
typedef struct {
CPUState *env;
-qemu_irq *pic;
+DeviceState *pic;
qemu_irq reset;
PXA2xxDMAState *dma;
DeviceState *gpio;
diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
index d966846..89550dd 100644
--- a/hw/pxa2xx.c
+++ b/hw/pxa2xx.c
@@ -888,7 +888,7 @@ static int pxa2xx_ssp_init(SysBusDevice *dev)
static inline void pxa2xx_rtc_int_update(PXA2xxState *s)
{
-qemu_set_irq(s->pic[PXA2XX_PIC_RTCALARM], !!(s->rtsr & 0x2553));
+qemu_set_irq(qdev_get_gpio_in(s->pic, PXA2XX_PIC_RTCALARM), !!(s->rtsr &
0x2553));
}
static void pxa2xx_rtc_hzupdate(PXA2xxState *s)
@@ -2065,10 +2065,9 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const
char *revision)
s->pic = pxa2xx_pic_init(0x40d0, s->env);
-s->dma = pxa27x_dma_init(0x4000, s->pic[PXA2XX_PIC_DMA]);
+s->dma = pxa27x_dma_init(0x4000, qdev_get_gpio_in(s->pic,
PXA2XX_PIC_DMA));
-pxa27x_timer_init(0x40a0, &s->pic[PXA2XX_PIC_OST_0],
-s->pic[PXA27X_PIC_OST_4_11]);
+pxa27x_timer_init(0x40a0, s->pic);
s->gpio = pxa2xx_gpio_init(0x40e0, s->env, s->pic, 121);
@@ -2078,26 +2077,26 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const
char *revision)
exit(1);
}
s->mmc = pxa2xx_mmci_init(0x4110, dinfo->bdrv,
- s->pic[PXA2XX_PIC_MMC], s->dma);
+ qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC),
s->dma);
for (i = 0; pxa270_serial[i].io_base; i ++)
if (serial_hds[i])
#ifdef TARGET_WORDS_BIGENDIAN
serial_mm_init(pxa270_serial[i].io_base, 2,
- s->pic[pxa270_serial[i].irqn], 14857000/16,
+ qdev_get_gpio_in(s->pic, pxa270_serial[i].irqn),
14857000/16,
serial_hds[i], 1, 1);
#else
serial_mm_init(pxa270_serial[i].io_base, 2,
- s->pic[pxa270_serial[i].irqn], 14857000/16,
+ qdev_get_gpio_in(s->pic, pxa270_serial[i].irqn),
14857000/16,
serial_hds[i], 1, 0);
#endif
else
break;
if (serial_hds[i])
-s->fir = pxa2xx_fir_init(0x4080, s->pic[PXA2XX_PIC_ICP],
+s->fir = pxa2xx_fir_init(0x4080, qdev_get_gpio_in(s->pic,
PXA2XX_PIC_ICP),
s->dma, serial_hds[i]);
-s->lcd = pxa2xx_lcdc_init(0x4400, s-
[Qemu-devel] [PATCH 08/11] Add scoop post_load callback that sets IRQs to loaded levels
Signed-off-by: Dmitry Eremin-Solenikov
---
hw/zaurus.c | 19 ++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/hw/zaurus.c b/hw/zaurus.c
index fca11a5..90fedc9 100644
--- a/hw/zaurus.c
+++ b/hw/zaurus.c
@@ -181,17 +181,34 @@ static int scoop_init(SysBusDevice *dev)
return 0;
}
+static int scoop_post_load(void *opaque, int version_id)
+{
+ScoopInfo *s = (ScoopInfo *) opaque;
+int i;
+uint32_t level;
+
+level = s->gpio_level & s->gpio_dir;
+
+for (i = 1; i < 1 << 16; i <<= 1) {
+qemu_set_irq(s->handler[i], level & i);
+}
+
+s->prev_level = level;
+
+return 0;
+}
+
static bool is_version_0 (void *opaque, int version_id)
{
return version_id == 0;
}
-
static const VMStateDescription vmstate_scoop_regs = {
.name = "scoop",
.version_id = 1,
.minimum_version_id = 0,
.minimum_version_id_old = 0,
+.post_load = scoop_post_load,
.fields = (VMStateField []) {
VMSTATE_UINT16(status, ScoopInfo),
VMSTATE_UINT16(power, ScoopInfo),
--
1.7.2.3
[Qemu-devel] [PATCH 01/28] strtosz(): use unsigned char and switch to qemu_isspace()
From: Jes Sorensen
isspace() behavior is undefined for signed char.
Bug pointed out by Eric Blake, thanks!
Signed-off-by: Jes Sorensen
Signed-off-by: Kevin Wolf
---
cutils.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/cutils.c b/cutils.c
index 4d2e27c..a067cf4 100644
--- a/cutils.c
+++ b/cutils.c
@@ -294,7 +294,8 @@ int fcntl_setfl(int fd, int flag)
int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
{
int64_t retval = -1;
-char *endptr, c, d;
+char *endptr;
+unsigned char c, d;
int mul_required = 0;
double val, mul, integral, fraction;
@@ -314,7 +315,7 @@ int64_t strtosz_suffix(const char *nptr, char **end, const
char default_suffix)
*/
c = *endptr;
d = c;
-if (isspace(c) || c == '\0' || c == ',') {
+if (qemu_isspace(c) || c == '\0' || c == ',') {
c = 0;
if (default_suffix) {
d = default_suffix;
@@ -361,7 +362,7 @@ int64_t strtosz_suffix(const char *nptr, char **end, const
char default_suffix)
*/
if (c != 0) {
endptr++;
-if (!isspace(*endptr) && *endptr != ',' && *endptr != 0) {
+if (!qemu_isspace(*endptr) && *endptr != ',' && *endptr != 0) {
goto fail;
}
}
--
1.7.2.3
[Qemu-devel] [PATCH 07/11] tc6393xb: correct NAND isr assertion
Signed-off-by: Dmitry Eremin-Solenikov --- hw/tc6393xb.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/tc6393xb.c b/hw/tc6393xb.c index c3fbe4e..0cadcde 100644 --- a/hw/tc6393xb.c +++ b/hw/tc6393xb.c @@ -381,7 +381,7 @@ static void tc6393xb_nand_writeb(TC6393xbState *s, target_phys_addr_t addr, uint case NAND_DATA + 2: case NAND_DATA + 3: nand_setio(s->flash, value); -s->nand.isr &= 1; +s->nand.isr |= 1; tc6393xb_nand_irq(s); return; case NAND_MODE: -- 1.7.2.3
[Qemu-devel] [PULL 00/28] Block patches
The following changes since commit 45d1aa828f8c94b082a0aa2dbf76535594ee45df:
Update OpenBIOS images to r1018 (2011-01-30 13:10:10 +)
are available in the git repository at:
git://repo.or.cz/qemu/kevin.git block
Blue Swirl (1):
qcow2-refcount: remove write-only variables
Christoph Hellwig (3):
block: add block_resize monitor command
block: tell drivers about an image resize
virtio-blk: tell the guest about size changes
Jes Sorensen (6):
strtosz(): use unsigned char and switch to qemu_isspace()
strtosz() use qemu_toupper() to simplify switch statement
strtosz(): Fix name confusion in use of modf()
strtosz(): Use suffix macros in switch() statement
Add documentation for STRTOSZ_DEFSUFFIX_ macros
Reorganize struct Qcow2Cache for better struct packing
Kevin Wolf (3):
qemu-io: Fix discard command
qcow2: Add bdrv_discard support
raw-win32: Fix bdrv_flush return value
MORITA Kazutaka (1):
sheepdog: support creating images on remote hosts
Markus Armbruster (11):
scsi hotplug: Set DriveInfo member bus correctly
blockdev: New drive_get_next(), replacing qdev_init_bdrv()
blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h
blockdev: Put BlockInterfaceType names and max_devs in tables
blockdev: Fix regression in -drive if=scsi,index=N
blockdev: Make drive_add() take explicit type, index parameters
blockdev: Factor drive_index_to_{bus,unit}_id out of drive_init()
blockdev: New drive_get_by_index()
blockdev: Reject multiple definitions for the same drive
blockdev: Replace drive_add()'s fmt, ... by optstr parameter
blockdev: Fix drive_add for drives without media
Stefan Hajnoczi (3):
virtio-pci: Disable virtio-ioeventfd when !CONFIG_IOTHREAD
ahci: Fix cpu_physical_memory_unmap() argument ordering
qed: Images with backing file do not require QED_F_NEED_CHECK
block.c| 12 ++-
block.h|3 +-
block/qcow2-cache.c|2 +-
block/qcow2-cluster.c | 82 +++
block/qcow2-refcount.c |5 +-
block/qcow2.c |8 ++
block/qcow2.h |2 +
block/qed.c| 24 +--
block/raw-win32.c |2 +-
block/sheepdog.c | 17 -
block_int.h|5 +-
blockdev.c | 173 ---
blockdev.h | 19 --
cutils.c | 28 -
hmp-commands.hx| 19 +
hw/device-hotplug.c|5 +-
hw/ide.h |2 +
hw/ide/ahci.c | 11 ++--
hw/ide/core.c |6 ++-
hw/pci-hotplug.c |1 +
hw/pl181.c |7 +-
hw/qdev.c | 15
hw/qdev.h |2 -
hw/scsi.h |3 +-
hw/sd.c|7 ++-
hw/ssi-sd.c|7 +-
hw/usb-msd.c |3 +-
hw/virtio-blk.c| 10 +++
kvm-all.c |8 ++-
qemu-common.h | 13 ++--
qemu-io.c |2 +-
qmp-commands.hx| 28
savevm.c |1 -
vl.c | 104 +
34 files changed, 447 insertions(+), 189 deletions(-)
[Qemu-devel] [PATCH 08/28] virtio-pci: Disable virtio-ioeventfd when !CONFIG_IOTHREAD
From: Stefan Hajnoczi
It is not possible to use virtio-ioeventfd when building without an I/O
thread. We rely on a signal to kick us out of vcpu execution. Timers
and AIO use SIGALRM and SIGUSR2 respectively. Unfortunately eventfd
does not support O_ASYNC (SIGIO) so eventfd cannot be used in a signal
driven manner.
Signed-off-by: Stefan Hajnoczi
Acked-by: Michael S. Tsirkin
Signed-off-by: Kevin Wolf
---
kvm-all.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index 255b6fa..8f0e17c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -449,10 +449,14 @@ int kvm_check_extension(KVMState *s, unsigned int
extension)
static int kvm_check_many_ioeventfds(void)
{
-/* Older kernels have a 6 device limit on the KVM io bus. Find out so we
+/* Userspace can use ioeventfd for io notification. This requires a host
+ * that supports eventfd(2) and an I/O thread; since eventfd does not
+ * support SIGIO it cannot interrupt the vcpu.
+ *
+ * Older kernels have a 6 device limit on the KVM io bus. Find out so we
* can avoid creating too many ioeventfds.
*/
-#ifdef CONFIG_EVENTFD
+#if defined(CONFIG_EVENTFD) && defined(CONFIG_IOTHREAD)
int ioeventfds[7];
int i, ret = 0;
for (i = 0; i < ARRAY_SIZE(ioeventfds); i++) {
--
1.7.2.3
[Qemu-devel] [PATCH 04/28] strtosz(): Use suffix macros in switch() statement
From: Jes Sorensen
Signed-off-by: Jes Sorensen
Signed-off-by: Kevin Wolf
---
cutils.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/cutils.c b/cutils.c
index 369a016..8d562b2 100644
--- a/cutils.c
+++ b/cutils.c
@@ -324,26 +324,26 @@ int64_t strtosz_suffix(const char *nptr, char **end,
const char default_suffix)
}
}
switch (qemu_toupper(d)) {
-case 'B':
+case STRTOSZ_DEFSUFFIX_B:
mul = 1;
if (mul_required) {
goto fail;
}
break;
-case 'K':
+case STRTOSZ_DEFSUFFIX_KB:
mul = 1 << 10;
break;
case 0:
if (mul_required) {
goto fail;
}
-case 'M':
+case STRTOSZ_DEFSUFFIX_MB:
mul = 1ULL << 20;
break;
-case 'G':
+case STRTOSZ_DEFSUFFIX_GB:
mul = 1ULL << 30;
break;
-case 'T':
+case STRTOSZ_DEFSUFFIX_TB:
mul = 1ULL << 40;
break;
default:
--
1.7.2.3
[Qemu-devel] [PATCH 07/28] virtio-blk: tell the guest about size changes
From: Christoph Hellwig
Raise a config change interrupt when the size changed. This allows
virtio-blk guest drivers to read-read the information from the
config space once it got the config chaged interrupt.
Signed-off-by: Christoph Hellwig
Signed-off-by: Kevin Wolf
---
hw/virtio-blk.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 9f2a9c0..ffac5a4 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -514,6 +514,15 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int
version_id)
return 0;
}
+static void virtio_blk_change_cb(void *opaque, int reason)
+{
+VirtIOBlock *s = opaque;
+
+if (reason & CHANGE_SIZE) {
+virtio_notify_config(&s->vdev);
+}
+}
+
VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
{
VirtIOBlock *s;
@@ -556,6 +565,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf
*conf)
register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
virtio_blk_save, virtio_blk_load, s);
bdrv_set_removable(s->bs, 0);
+bdrv_set_change_cb(s->bs, virtio_blk_change_cb, s);
s->bs->buffer_alignment = conf->logical_block_size;
add_boot_device_path(conf->bootindex, dev, "/disk@0,0");
--
1.7.2.3
[Qemu-devel] [PATCH 10/11] mainstone: convert FPGA emulation code to use QDev/SysBus
Signed-off-by: Dmitry Eremin-Solenikov
---
hw/mainstone.c | 10 +++--
hw/mainstone.h |3 --
hw/mst_fpga.c | 103 +--
3 files changed, 53 insertions(+), 63 deletions(-)
diff --git a/hw/mainstone.c b/hw/mainstone.c
index 61cac8a..7d13972 100644
--- a/hw/mainstone.c
+++ b/hw/mainstone.c
@@ -18,6 +18,7 @@
#include "sysemu.h"
#include "flash.h"
#include "blockdev.h"
+#include "sysbus.h"
static struct keymap map[0xE0] = {
[0 ... 0xDF] = { -1, -1 },
@@ -77,7 +78,7 @@ static void mainstone_common_init(ram_addr_t ram_size,
uint32_t sector_len = 256 * 1024;
target_phys_addr_t mainstone_flash_base[] = { MST_FLASH_0, MST_FLASH_1 };
PXA2xxState *cpu;
-qemu_irq *mst_irq;
+DeviceState *mst_irq;
DriveInfo *dinfo;
int i;
int be;
@@ -117,16 +118,17 @@ static void mainstone_common_init(ram_addr_t ram_size,
}
}
-mst_irq = mst_irq_init(MST_FPGA_PHYS, qdev_get_gpio_in(cpu->pic,
PXA2XX_PIC_GPIO_0));
+mst_irq = sysbus_create_simple("mainstone-fpga", MST_FPGA_PHYS,
+qdev_get_gpio_in(cpu->pic, PXA2XX_PIC_GPIO_0));
/* setup keypad */
printf("map addr %p\n", &map);
pxa27x_register_keypad(cpu->kp, map, 0xe0);
/* MMC/SD host */
-pxa2xx_mmci_handlers(cpu->mmc, NULL, mst_irq[MMC_IRQ]);
+pxa2xx_mmci_handlers(cpu->mmc, NULL, qdev_get_gpio_in(mst_irq, MMC_IRQ));
-smc91c111_init(&nd_table[0], MST_ETH_PHYS, mst_irq[ETHERNET_IRQ]);
+smc91c111_init(&nd_table[0], MST_ETH_PHYS, qdev_get_gpio_in(mst_irq,
ETHERNET_IRQ));
mainstone_binfo.kernel_filename = kernel_filename;
mainstone_binfo.kernel_cmdline = kernel_cmdline;
diff --git a/hw/mainstone.h b/hw/mainstone.h
index 35329f1..e6a2b67 100644
--- a/hw/mainstone.h
+++ b/hw/mainstone.h
@@ -32,7 +32,4 @@
#define S1_STSCHG_IRQ 14
#define S1_IRQ15
-extern qemu_irq
-*mst_irq_init(uint32_t base, qemu_irq irq);
-
#endif /* __MAINSTONE_H__ */
diff --git a/hw/mst_fpga.c b/hw/mst_fpga.c
index 3a49c84..847053f 100644
--- a/hw/mst_fpga.c
+++ b/hw/mst_fpga.c
@@ -8,8 +8,7 @@
* This code is licensed under the GNU GPL v2.
*/
#include "hw.h"
-#include "qdev.h"
-#include "mainstone.h"
+#include "sysbus.h"
/* Mainstone FPGA for extern irqs */
#define FPGA_GPIO_PIN 0
@@ -28,8 +27,9 @@
#define MST_PCMCIA10xe4
typedef struct mst_irq_state{
+ SysBusDevice busdev;
+
qemu_irq parent;
- qemu_irq *pins;
uint32_t prev_level;
uint32_t leddat1;
@@ -55,7 +55,7 @@ mst_fpga_update_gpio(mst_irq_state *s)
for (diff = s->prev_level ^ level; diff; diff ^= 1 << bit) {
bit = ffs(diff) - 1;
- qemu_set_irq(s->pins[bit], (level >> bit) & 1 );
+ qemu_set_irq(qdev_get_gpio_in(&s->busdev.qdev, bit), (level >>
bit) & 1 );
}
s->prev_level = level;
}
@@ -175,66 +175,57 @@ static CPUWriteMemoryFunc * const mst_fpga_writefn[] = {
mst_fpga_writeb,
};
-static void
-mst_fpga_save(QEMUFile *f, void *opaque)
-{
- struct mst_irq_state *s = (mst_irq_state *) opaque;
-
- qemu_put_be32s(f, &s->prev_level);
- qemu_put_be32s(f, &s->leddat1);
- qemu_put_be32s(f, &s->leddat2);
- qemu_put_be32s(f, &s->ledctrl);
- qemu_put_be32s(f, &s->gpswr);
- qemu_put_be32s(f, &s->mscwr1);
- qemu_put_be32s(f, &s->mscwr2);
- qemu_put_be32s(f, &s->mscwr3);
- qemu_put_be32s(f, &s->mscrd);
- qemu_put_be32s(f, &s->intmskena);
- qemu_put_be32s(f, &s->intsetclr);
- qemu_put_be32s(f, &s->pcmcia0);
- qemu_put_be32s(f, &s->pcmcia1);
-}
-
-static int
-mst_fpga_load(QEMUFile *f, void *opaque, int version_id)
-{
- mst_irq_state *s = (mst_irq_state *) opaque;
-
- qemu_get_be32s(f, &s->prev_level);
- qemu_get_be32s(f, &s->leddat1);
- qemu_get_be32s(f, &s->leddat2);
- qemu_get_be32s(f, &s->ledctrl);
- qemu_get_be32s(f, &s->gpswr);
- qemu_get_be32s(f, &s->mscwr1);
- qemu_get_be32s(f, &s->mscwr2);
- qemu_get_be32s(f, &s->mscwr3);
- qemu_get_be32s(f, &s->mscrd);
- qemu_get_be32s(f, &s->intmskena);
- qemu_get_be32s(f, &s->intsetclr);
- qemu_get_be32s(f, &s->pcmcia0);
- qemu_get_be32s(f, &s->pcmcia1);
- return 0;
-}
-
-qemu_irq *mst_irq_init(uint32_t base, qemu_irq irq)
+static int mst_fpga_init(SysBusDevice *dev)
{
mst_irq_state *s;
int iomemtype;
- qemu_irq *qi;
- s = (mst_irq_state *)
- qemu_mallocz(sizeof(mst_irq_state));
+ s = FROM_SYSBUS(mst_irq_state, dev);
- s->parent = irq;
+ sysbus_init_irq(dev, &s->parent);
/* alloc the external 16 irqs */
- qi = qemu_allocate_irqs(mst_fpga_set_irq, s, MST_NUM_IRQS);
- s->pins = qi;
+ qdev_init_gpio_in(&dev->qdev, mst_fpga_set_irq, MST_NUM_IRQS);
iomemtype = cpu_register_io_memory(mst_fpga_readfn,
m
[Qemu-devel] [PATCH 18/28] qcow2-refcount: remove write-only variables
From: Blue Swirl
Variables l2_modified and l2_size are not really used, remove them.
Spotted by GCC 4.6.0:
CCblock/qcow2-refcount.o
/src/qemu/block/qcow2-refcount.c: In function 'qcow2_update_snapshot_refcount':
/src/qemu/block/qcow2-refcount.c:708:37: error: variable 'l2_modified' set but
not used [-Werror=unused-but-set-variable]
/src/qemu/block/qcow2-refcount.c:708:9: error: variable 'l2_size' set but not
used [-Werror=unused-but-set-variable]
CC: Kevin Wolf
Signed-off-by: Blue Swirl
Signed-off-by: Kevin Wolf
---
block/qcow2-refcount.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index e37e226..915d85a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -705,7 +705,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
BDRVQcowState *s = bs->opaque;
uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2, l1_allocated;
int64_t old_offset, old_l2_offset;
-int l2_size, i, j, l1_modified, l2_modified, nb_csectors, refcount;
+int i, j, l1_modified, nb_csectors, refcount;
int ret;
l2_table = NULL;
@@ -729,14 +729,12 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
l1_allocated = 0;
}
-l2_size = s->l2_size * sizeof(uint64_t);
l1_modified = 0;
for(i = 0; i < l1_size; i++) {
l2_offset = l1_table[i];
if (l2_offset) {
old_l2_offset = l2_offset;
l2_offset &= ~QCOW_OFLAG_COPIED;
-l2_modified = 0;
ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset,
(void**) &l2_table);
@@ -788,7 +786,6 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
s->refcount_block_cache);
}
l2_table[j] = cpu_to_be64(offset);
-l2_modified = 1;
qcow2_cache_entry_mark_dirty(s->l2_table_cache,
l2_table);
}
}
--
1.7.2.3
[Qemu-devel] [PATCH 02/28] strtosz() use qemu_toupper() to simplify switch statement
From: Jes Sorensen
Signed-off-by: Jes Sorensen
Signed-off-by: Kevin Wolf
---
cutils.c |7 +--
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/cutils.c b/cutils.c
index a067cf4..78d35e2 100644
--- a/cutils.c
+++ b/cutils.c
@@ -323,16 +323,14 @@ int64_t strtosz_suffix(const char *nptr, char **end,
const char default_suffix)
d = c;
}
}
-switch (d) {
+switch (qemu_toupper(d)) {
case 'B':
-case 'b':
mul = 1;
if (mul_required) {
goto fail;
}
break;
case 'K':
-case 'k':
mul = 1 << 10;
break;
case 0:
@@ -340,15 +338,12 @@ int64_t strtosz_suffix(const char *nptr, char **end,
const char default_suffix)
goto fail;
}
case 'M':
-case 'm':
mul = 1ULL << 20;
break;
case 'G':
-case 'g':
mul = 1ULL << 30;
break;
case 'T':
-case 't':
mul = 1ULL << 40;
break;
default:
--
1.7.2.3
[Qemu-devel] [PATCH 14/28] qcow2: Add bdrv_discard support
This adds a bdrv_discard function to qcow2 that frees the discarded clusters.
It does not yet pass the discard on to the underlying file system driver, but
the space can be reused by future writes to the image.
Signed-off-by: Kevin Wolf
Reviewed-by: Stefan Hajnoczi
---
block/qcow2-cluster.c | 82 +
block/qcow2.c |8 +
block/qcow2.h |2 +
3 files changed, 92 insertions(+), 0 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1c2003a..5fb8c66 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -888,3 +888,85 @@ int qcow2_decompress_cluster(BlockDriverState *bs,
uint64_t cluster_offset)
}
return 0;
}
+
+/*
+ * This discards as many clusters of nb_clusters as possible at once (i.e.
+ * all clusters in the same L2 table) and returns the number of discarded
+ * clusters.
+ */
+static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
+unsigned int nb_clusters)
+{
+BDRVQcowState *s = bs->opaque;
+uint64_t l2_offset, *l2_table;
+int l2_index;
+int ret;
+int i;
+
+ret = get_cluster_table(bs, offset, &l2_table, &l2_offset, &l2_index);
+if (ret < 0) {
+return ret;
+}
+
+/* Limit nb_clusters to one L2 table */
+nb_clusters = MIN(nb_clusters, s->l2_size - l2_index);
+
+for (i = 0; i < nb_clusters; i++) {
+uint64_t old_offset;
+
+old_offset = be64_to_cpu(l2_table[l2_index + i]);
+old_offset &= ~QCOW_OFLAG_COPIED;
+
+if (old_offset == 0) {
+continue;
+}
+
+/* First remove L2 entries */
+qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+l2_table[l2_index + i] = cpu_to_be64(0);
+
+/* Then decrease the refcount */
+qcow2_free_any_clusters(bs, old_offset, 1);
+}
+
+ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
+if (ret < 0) {
+return ret;
+}
+
+return nb_clusters;
+}
+
+int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
+int nb_sectors)
+{
+BDRVQcowState *s = bs->opaque;
+uint64_t end_offset;
+unsigned int nb_clusters;
+int ret;
+
+end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
+
+/* Round start up and end down */
+offset = align_offset(offset, s->cluster_size);
+end_offset &= ~(s->cluster_size - 1);
+
+if (offset > end_offset) {
+return 0;
+}
+
+nb_clusters = size_to_clusters(s, end_offset - offset);
+
+/* Each L2 table is handled by its own loop iteration */
+while (nb_clusters > 0) {
+ret = discard_single_l2(bs, offset, nb_clusters);
+if (ret < 0) {
+return ret;
+}
+
+nb_clusters -= ret;
+offset += (ret * s->cluster_size);
+}
+
+return 0;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index 49bf7b9..dbe4fdd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1084,6 +1084,13 @@ static int qcow2_make_empty(BlockDriverState *bs)
return 0;
}
+static int qcow2_discard(BlockDriverState *bs, int64_t sector_num,
+int nb_sectors)
+{
+return qcow2_discard_clusters(bs, sector_num << BDRV_SECTOR_BITS,
+nb_sectors);
+}
+
static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
{
BDRVQcowState *s = bs->opaque;
@@ -1349,6 +1356,7 @@ static BlockDriver bdrv_qcow2 = {
.bdrv_aio_writev= qcow2_aio_writev,
.bdrv_aio_flush = qcow2_aio_flush,
+.bdrv_discard = qcow2_discard,
.bdrv_truncate = qcow2_truncate,
.bdrv_write_compressed = qcow2_write_compressed,
diff --git a/block/qcow2.h b/block/qcow2.h
index 6d80120..a019831 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -209,6 +209,8 @@ uint64_t
qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
int compressed_size);
int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
+int nb_sectors);
/* qcow2-snapshot.c functions */
int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
--
1.7.2.3
[Qemu-devel] [PATCH 15/28] qed: Images with backing file do not require QED_F_NEED_CHECK
From: Stefan Hajnoczi
The consistency check on open is necessary in order to fix inconsistent
table offsets left as a result of a crash mid-operation. Images with a
backing file actually flush before updating table offsets and are
therefore guaranteed to be consistent. Do not mark these images dirty.
Signed-off-by: Stefan Hajnoczi
Signed-off-by: Kevin Wolf
---
block/qed.c | 24 +---
1 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/block/qed.c b/block/qed.c
index a46f9ef..3273448 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -977,6 +977,19 @@ static void qed_aio_write_prefill(void *opaque, int ret)
}
/**
+ * Check if the QED_F_NEED_CHECK bit should be set during allocating write
+ */
+static bool qed_should_set_need_check(BDRVQEDState *s)
+{
+/* The flush before L2 update path ensures consistency */
+if (s->bs->backing_hd) {
+return false;
+}
+
+return !(s->header.features & QED_F_NEED_CHECK);
+}
+
+/**
* Write new data cluster
*
* @acb:Write request
@@ -1001,15 +1014,12 @@ static void qed_aio_write_alloc(QEDAIOCB *acb, size_t
len)
acb->cur_cluster = qed_alloc_clusters(s, acb->cur_nclusters);
qemu_iovec_copy(&acb->cur_qiov, acb->qiov, acb->qiov_offset, len);
-/* Write new cluster if the image is already marked dirty */
-if (s->header.features & QED_F_NEED_CHECK) {
+if (qed_should_set_need_check(s)) {
+s->header.features |= QED_F_NEED_CHECK;
+qed_write_header(s, qed_aio_write_prefill, acb);
+} else {
qed_aio_write_prefill(acb, 0);
-return;
}
-
-/* Mark the image dirty before writing the new cluster */
-s->header.features |= QED_F_NEED_CHECK;
-qed_write_header(s, qed_aio_write_prefill, acb);
}
/**
--
1.7.2.3
[Qemu-devel] [PATCH 12/28] sheepdog: support creating images on remote hosts
From: MORITA Kazutaka
This patch parses the input filename in sd_create(), and enables us
specifying a target server to create sheepdog images.
Signed-off-by: MORITA Kazutaka
Signed-off-by: Kevin Wolf
---
block/sheepdog.c | 17 ++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index e62820a..a54e0de 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1294,12 +1294,23 @@ static int do_sd_create(char *filename, int64_t
vdi_size,
static int sd_create(const char *filename, QEMUOptionParameter *options)
{
int ret;
-uint32_t vid = 0;
+uint32_t vid = 0, base_vid = 0;
int64_t vdi_size = 0;
char *backing_file = NULL;
+BDRVSheepdogState s;
+char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN];
+uint32_t snapid;
strstart(filename, "sheepdog:", (const char **)&filename);
+memset(&s, 0, sizeof(s));
+memset(vdi, 0, sizeof(vdi));
+memset(tag, 0, sizeof(tag));
+if (parse_vdiname(&s, filename, vdi, &snapid, tag) < 0) {
+error_report("invalid filename\n");
+return -EINVAL;
+}
+
while (options && options->name) {
if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
vdi_size = options->value.n;
@@ -1338,11 +1349,11 @@ static int sd_create(const char *filename,
QEMUOptionParameter *options)
return -EINVAL;
}
-vid = s->inode.vdi_id;
+base_vid = s->inode.vdi_id;
bdrv_delete(bs);
}
-return do_sd_create((char *)filename, vdi_size, vid, NULL, 0, NULL, NULL);
+return do_sd_create((char *)vdi, vdi_size, base_vid, &vid, 0, s.addr,
s.port);
}
static void sd_close(BlockDriverState *bs)
--
1.7.2.3
[Qemu-devel] [PATCH 05/28] block: add block_resize monitor command
From: Christoph Hellwig
Add a monitor command that allows resizing of block devices while
qemu is running. It uses the existing bdrv_truncate method already
used by qemu-img to do it's work. Compared to qemu-img the size
parsing is very simplicistic, but I think having a properly numering
object is more useful for non-humand monitor users than having
the units and relative resize parsing.
For SCSI devices the new size can be updated in Linux guests by
doing the following shell command:
echo > /sys/class/scsi_device/0:0:0:0/device/rescan
For ATA devices I don't know of a way to update the block device
size in Linux system, and for virtio-blk the next two patches
will provide an automatic update of the size when this command
is issued on the host.
Signed-off-by: Christoph Hellwig
Signed-off-by: Kevin Wolf
---
blockdev.c | 30 ++
blockdev.h |1 +
hmp-commands.hx | 19 +++
qmp-commands.hx | 28
4 files changed, 78 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index f7f591f..e48d33d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -705,3 +705,33 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject
**ret_data)
return 0;
}
+
+/*
+ * XXX: replace the QERR_UNDEFINED_ERROR errors with real values once the
+ * existing QERR_ macro mess is cleaned up. A good example for better
+ * error reports can be found in the qemu-img resize code.
+ */
+int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+const char *device = qdict_get_str(qdict, "device");
+int64_t size = qdict_get_int(qdict, "size");
+BlockDriverState *bs;
+
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+if (size < 0) {
+qerror_report(QERR_UNDEFINED_ERROR);
+return -1;
+}
+
+if (bdrv_truncate(bs, size)) {
+qerror_report(QERR_UNDEFINED_ERROR);
+return -1;
+}
+
+return 0;
+}
diff --git a/blockdev.h b/blockdev.h
index 4536b5c..b8a88bf 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -53,5 +53,6 @@ int do_change_block(Monitor *mon, const char *device,
const char *filename, const char *fmt);
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
#endif
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1cea572..8df4adf 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -53,6 +53,25 @@ Quit the emulator.
ETEXI
{
+.name = "block_resize",
+.args_type = "device:B,size:o",
+.params = "device size",
+.help = "resize a block image",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_block_resize,
+},
+
+STEXI
+@item block_resize
+@findex block_resize
+Resize a block image while a guest is running. Usually requires guest
+action to see the updated size. Resize to a lower size is supported,
+but should be used with extreme caution. Note that this command only
+resizes image files, it can not resize block devices like LVM volumes.
+ETEXI
+
+
+{
.name = "eject",
.args_type = "force:-f,device:B",
.params = "[-f] device",
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 56c4d8b..9f79f5f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -601,6 +601,34 @@ Example:
-> { "execute": "netdev_del", "arguments": { "id": "netdev1" } }
<- { "return": {} }
+
+EQMP
+
+{
+.name = "block_resize",
+.args_type = "device:B,size:o",
+.params = "device size",
+.help = "resize a block image",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_block_resize,
+},
+
+SQMP
+block_resize
+
+
+Resize a block image while a guest is running.
+
+Arguments:
+
+- "device": the device's ID, must be unique (json-string)
+- "size": new size
+
+Example:
+
+-> { "execute": "block_resize", "arguments": { "device": "scratch", "size":
1073741824 } }
+<- { "return": {} }
+
EQMP
{
--
1.7.2.3
[Qemu-devel] [PATCH 27/28] blockdev: Replace drive_add()'s fmt, ... by optstr parameter
From: Markus Armbruster
Let the callers build the optstr. Only one wants to. All the others
become simpler, because they don't have to worry about escaping '%'.
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c |8 +---
blockdev.h |5 +
vl.c | 23 +--
3 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 3f7b560..4b2145c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -93,17 +93,11 @@ QemuOpts *drive_def(const char *optstr)
}
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
-const char *fmt, ...)
+const char *optstr)
{
-va_list ap;
-char optstr[1024];
QemuOpts *opts;
char buf[32];
-va_start(ap, fmt);
-vsnprintf(optstr, sizeof(optstr), fmt, ap);
-va_end(ap);
-
opts = drive_def(optstr);
if (!opts) {
return NULL;
diff --git a/blockdev.h b/blockdev.h
index 18278cc..e5d8c56 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -47,10 +47,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
QemuOpts *drive_def(const char *optstr);
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
-const char *fmt, ...) /*GCC_FMT_ATTR(4, 5)*/;
-/* GCC_FMT_ATTR() commented out to avoid the (pretty useless)
- * "zero-length gnu_printf format string" warning we insist to
- * enable */
+const char *optstr);
DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
/* device-hotplug */
diff --git a/vl.c b/vl.c
index 99f5bfe..f86724f 100644
--- a/vl.c
+++ b/vl.c
@@ -621,7 +621,6 @@ static int bt_parse(const char *opt)
/***/
/* QEMU Block devices */
-/* Any % in the following strings must be escaped as %% */
#define HD_OPTS "media=disk"
#define CDROM_OPTS "media=cdrom"
#define FD_OPTS ""
@@ -2050,17 +2049,21 @@ int main(int argc, char **argv, char **envp)
initrd_filename = optarg;
break;
case QEMU_OPTION_hda:
-if (cyls == 0)
-hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS);
-else
-hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS
-",cyls=%d,heads=%d,secs=%d%s",
- cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
+{
+char buf[256];
+if (cyls == 0)
+snprintf(buf, sizeof(buf), "%s", HD_OPTS);
+else
+snprintf(buf, sizeof(buf),
+ "%s,cyls=%d,heads=%d,secs=%d%s",
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
",trans=lba" :
- translation == BIOS_ATA_TRANSLATION_NONE ?
+ translation == BIOS_ATA_TRANSLATION_NONE ?
",trans=none" : "");
- break;
+drive_add(IF_DEFAULT, 0, optarg, buf);
+break;
+}
case QEMU_OPTION_hdb:
case QEMU_OPTION_hdc:
case QEMU_OPTION_hdd:
--
1.7.2.3
[Qemu-devel] [PATCH 10/28] ahci: Fix cpu_physical_memory_unmap() argument ordering
From: Stefan Hajnoczi
The len and is_write arguments to cpu_physical_memory_unmap() were
swapped. This patch changes calls to use the correct argument ordering.
Signed-off-by: Stefan Hajnoczi
Signed-off-by: Kevin Wolf
---
hw/ide/ahci.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 968fdce..433171c 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -513,12 +513,12 @@ static void map_page(uint8_t **ptr, uint64_t addr,
uint32_t wanted)
target_phys_addr_t len = wanted;
if (*ptr) {
-cpu_physical_memory_unmap(*ptr, 1, len, len);
+cpu_physical_memory_unmap(*ptr, len, 1, len);
}
*ptr = cpu_physical_memory_map(addr, &len, 1);
if (len < wanted) {
-cpu_physical_memory_unmap(*ptr, 1, len, len);
+cpu_physical_memory_unmap(*ptr, len, 1, len);
*ptr = NULL;
}
}
@@ -956,7 +956,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t
*cmd_fis)
ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
if (cmd_mapped) {
-cpu_physical_memory_unmap(cmd_fis, 0, cmd_len, cmd_len);
+cpu_physical_memory_unmap(cmd_fis, cmd_len, 0, cmd_len);
}
}
@@ -1002,7 +1002,7 @@ static int ahci_populate_sglist(AHCIDevice *ad,
QEMUSGList *sglist)
}
out:
-cpu_physical_memory_unmap(prdt, 0, prdt_len, prdt_len);
+cpu_physical_memory_unmap(prdt, prdt_len, 0, prdt_len);
return r;
}
@@ -1228,7 +1228,7 @@ static int handle_cmd(AHCIState *s, int port, int slot)
}
out:
-cpu_physical_memory_unmap(cmd_fis, 1, cmd_len, cmd_len);
+cpu_physical_memory_unmap(cmd_fis, cmd_len, 1, cmd_len);
if (s->dev[port].port.ifs[0].status & (BUSY_STAT|DRQ_STAT)) {
/* async command, complete later */
--
1.7.2.3
[Qemu-devel] [PATCH 17/28] scsi hotplug: Set DriveInfo member bus correctly
From: Markus Armbruster
drive_init() picks the first free bus and unit number, unless the user
specifies them.
This isn't a good fit for the drive_add monitor command, because there
we specify the controller by PCI address instead of using bus number
set by drive_init().
scsi_hot_add() takes care to replace the unit number set by
drive_init() by the real one, but it neglects to replace the bus
number. Thus, bus/unit in DriveInfo may be bogus. Affects
drive_get() and drive_get_max_bus(). I'm not aware of anything bad
happening because of that; looks like by the time we're hot-plugging,
the two functions aren't used anymore. Fix it anyway.
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
hw/pci-hotplug.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 270a982..b6dcbda 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -90,6 +90,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
* specified).
*/
dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
+dinfo->bus = scsibus->busnr;
scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
false);
if (!scsidev) {
return -1;
--
1.7.2.3
[Qemu-devel] [PATCH 06/28] block: tell drivers about an image resize
From: Christoph Hellwig
Extend the change_cb callback with a reason argument, and use it
to tell drivers about size changes.
Signed-off-by: Christoph Hellwig
Signed-off-by: Kevin Wolf
---
block.c | 12
block.h |3 ++-
block_int.h |5 -
hw/ide/core.c |6 +-
hw/sd.c |7 ++-
5 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index 7ad3ddf..998df1b 100644
--- a/block.c
+++ b/block.c
@@ -645,7 +645,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
/* call the change callback */
bs->media_changed = 1;
if (bs->change_cb)
-bs->change_cb(bs->change_opaque);
+bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
}
return 0;
@@ -684,7 +684,7 @@ void bdrv_close(BlockDriverState *bs)
/* call the change callback */
bs->media_changed = 1;
if (bs->change_cb)
-bs->change_cb(bs->change_opaque);
+bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
}
}
@@ -1135,6 +1135,9 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
ret = drv->bdrv_truncate(bs, offset);
if (ret == 0) {
ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+if (bs->change_cb) {
+bs->change_cb(bs->change_opaque, CHANGE_SIZE);
+}
}
return ret;
}
@@ -1366,7 +1369,8 @@ int bdrv_enable_write_cache(BlockDriverState *bs)
/* XXX: no longer used */
void bdrv_set_change_cb(BlockDriverState *bs,
-void (*change_cb)(void *opaque), void *opaque)
+void (*change_cb)(void *opaque, int reason),
+void *opaque)
{
bs->change_cb = change_cb;
bs->change_opaque = opaque;
@@ -1411,7 +1415,7 @@ int bdrv_set_key(BlockDriverState *bs, const char *key)
/* call the change callback now, we skipped it on open */
bs->media_changed = 1;
if (bs->change_cb)
-bs->change_cb(bs->change_opaque);
+bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
}
return ret;
}
diff --git a/block.h b/block.h
index f923add..239f729 100644
--- a/block.h
+++ b/block.h
@@ -182,7 +182,8 @@ int bdrv_is_locked(BlockDriverState *bs);
void bdrv_set_locked(BlockDriverState *bs, int locked);
int bdrv_eject(BlockDriverState *bs, int eject_flag);
void bdrv_set_change_cb(BlockDriverState *bs,
-void (*change_cb)(void *opaque), void *opaque);
+void (*change_cb)(void *opaque, int reason),
+void *opaque);
void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
BlockDriverState *bdrv_find(const char *name);
BlockDriverState *bdrv_next(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index 12663e8..6ebdc3e 100644
--- a/block_int.h
+++ b/block_int.h
@@ -153,7 +153,7 @@ struct BlockDriverState {
int valid_key; /* if true, a valid encryption key has been set */
int sg;/* if true, the device is a /dev/sg* */
/* event callback when inserting/removing */
-void (*change_cb)(void *opaque);
+void (*change_cb)(void *opaque, int reason);
void *change_opaque;
BlockDriver *drv; /* NULL means no media */
@@ -203,6 +203,9 @@ struct BlockDriverState {
void *private;
};
+#define CHANGE_MEDIA 0x01
+#define CHANGE_SIZE0x02
+
struct BlockDriverAIOCB {
AIOPool *pool;
BlockDriverState *bs;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index e698c13..dd63664 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1584,11 +1584,15 @@ static void ide_cfata_metadata_write(IDEState *s)
}
/* called when the inserted state of the media has changed */
-static void cdrom_change_cb(void *opaque)
+static void cdrom_change_cb(void *opaque, int reason)
{
IDEState *s = opaque;
uint64_t nb_sectors;
+if (!(reason & CHANGE_MEDIA)) {
+return;
+}
+
bdrv_get_geometry(s->bs, &nb_sectors);
s->nb_sectors = nb_sectors;
diff --git a/hw/sd.c b/hw/sd.c
index 601545b..789ca84 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -422,9 +422,14 @@ static void sd_reset(SDState *sd, BlockDriverState *bdrv)
sd->pwd_len = 0;
}
-static void sd_cardchange(void *opaque)
+static void sd_cardchange(void *opaque, int reason)
{
SDState *sd = opaque;
+
+if (!(reason & CHANGE_MEDIA)) {
+return;
+}
+
qemu_set_irq(sd->inserted_cb, bdrv_is_inserted(sd->bdrv));
if (bdrv_is_inserted(sd->bdrv)) {
sd_reset(sd, sd->bdrv);
--
1.7.2.3
[Qemu-devel] [PATCH 16/28] raw-win32: Fix bdrv_flush return value
Signed-off-by: Kevin Wolf
---
block/raw-win32.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 06c9710..c204a80 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -153,7 +153,7 @@ static int raw_flush(BlockDriverState *bs)
int ret;
ret = FlushFileBuffers(s->hfile);
-if (ret != 0) {
+if (ret == 0) {
return -EIO;
}
--
1.7.2.3
[Qemu-devel] [PATCH 11/28] Reorganize struct Qcow2Cache for better struct packing
From: Jes Sorensen
Move size after the two pointers in struct Qcow2Cache to get better
packing of struct elements on 64 bit architectures.
Signed-off-by: Jes Sorensen
Signed-off-by: Kevin Wolf
---
block/qcow2-cache.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 8f2955b..3824739 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -35,9 +35,9 @@ typedef struct Qcow2CachedTable {
} Qcow2CachedTable;
struct Qcow2Cache {
-int size;
Qcow2CachedTable* entries;
struct Qcow2Cache* depends;
+int size;
booldepends_on_flush;
boolwritethrough;
};
--
1.7.2.3
Re: [Qemu-devel] [PATCH] target-arm: Fix Neon vsra instructions.
On 25.01.2011 18:18, Christophe Lyon wrote: > This patch fixes the errors reported by my tests in VSRA. > ping?
[Qemu-devel] [PATCH 21/28] blockdev: Put BlockInterfaceType names and max_devs in tables
From: Markus Armbruster
Turns drive_init()'s lengthy conditional into a concise loop, and
makes the data available elsewhere.
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c | 51 +--
1 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 699f312..9c883ce 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -19,6 +19,23 @@
static QTAILQ_HEAD(drivelist, DriveInfo) drives =
QTAILQ_HEAD_INITIALIZER(drives);
+static const char *const if_name[IF_COUNT] = {
+[IF_NONE] = "none",
+[IF_IDE] = "ide",
+[IF_SCSI] = "scsi",
+[IF_FLOPPY] = "floppy",
+[IF_PFLASH] = "pflash",
+[IF_MTD] = "mtd",
+[IF_SD] = "sd",
+[IF_VIRTIO] = "virtio",
+[IF_XEN] = "xen",
+};
+
+static const int if_max_devs[IF_COUNT] = {
+[IF_IDE] = MAX_IDE_DEVS,
+[IF_SCSI] = MAX_SCSI_DEVS,
+};
+
/*
* We automatically delete the drive when a device using it gets
* unplugged. Questionable feature, but we can't just drop it.
@@ -173,11 +190,9 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi,
int *fatal_error)
if (default_to_scsi) {
type = IF_SCSI;
-max_devs = MAX_SCSI_DEVS;
pstrcpy(devname, sizeof(devname), "scsi");
} else {
type = IF_IDE;
-max_devs = MAX_IDE_DEVS;
pstrcpy(devname, sizeof(devname), "ide");
}
media = MEDIA_DISK;
@@ -199,38 +214,14 @@ DriveInfo *drive_init(QemuOpts *opts, int
default_to_scsi, int *fatal_error)
if ((buf = qemu_opt_get(opts, "if")) != NULL) {
pstrcpy(devname, sizeof(devname), buf);
-if (!strcmp(buf, "ide")) {
- type = IF_IDE;
-max_devs = MAX_IDE_DEVS;
-} else if (!strcmp(buf, "scsi")) {
- type = IF_SCSI;
-max_devs = MAX_SCSI_DEVS;
-} else if (!strcmp(buf, "floppy")) {
- type = IF_FLOPPY;
-max_devs = 0;
-} else if (!strcmp(buf, "pflash")) {
- type = IF_PFLASH;
-max_devs = 0;
- } else if (!strcmp(buf, "mtd")) {
- type = IF_MTD;
-max_devs = 0;
- } else if (!strcmp(buf, "sd")) {
- type = IF_SD;
-max_devs = 0;
-} else if (!strcmp(buf, "virtio")) {
-type = IF_VIRTIO;
-max_devs = 0;
- } else if (!strcmp(buf, "xen")) {
- type = IF_XEN;
-max_devs = 0;
- } else if (!strcmp(buf, "none")) {
- type = IF_NONE;
-max_devs = 0;
- } else {
+for (type = 0; type < IF_COUNT && strcmp(buf, if_name[type]); type++)
+;
+if (type == IF_COUNT) {
error_report("unsupported bus type '%s'", buf);
return NULL;
}
}
+max_devs = if_max_devs[type];
if (cyls || heads || secs) {
if (cyls < 1 || (type == IF_IDE && cyls > 16383)) {
--
1.7.2.3
[Qemu-devel] [PATCH 28/28] blockdev: Fix drive_add for drives without media
From: Markus Armbruster
Watch this:
(qemu) drive_add 0 if=none
(qemu) info block
none0: type=hd removable=0 [not inserted]
(qemu) drive_del none0
Segmentation fault (core dumped)
add_init_drive() is confused about drive_init()'s failure modes, and
cleans up when it shouldn't. This leaves the DriveInfo with member
opts dangling. drive_del attempts to free it, and dies.
drive_init() behaves as follows:
* If it created a drive with media, it returns its DriveInfo.
* If it created a drive without media, it clears *fatal_error and
returns NULL.
* If it couldn't create a drive, it sets *fatal_error and returns
NULL.
Of its three callers:
* drive_init_func() is correct.
* usb_msd_init() assumes drive_init() failed when it returns NULL.
This is correct only because it always passes option "file", and
"drive without media" can't happen then.
* add_init_drive() assumes drive_init() failed when it returns NULL.
This is incorrect.
Clean up drive_init() to return NULL on failure and only on failure.
Drop its parameter fatal_error.
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c |8 ++--
blockdev.h |2 +-
hw/device-hotplug.c |3 +--
hw/usb-msd.c|3 +--
vl.c|9 ++---
5 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 4b2145c..1c56da0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -203,7 +203,7 @@ static int parse_block_error_action(const char *buf, int
is_read)
}
}
-DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi, int *fatal_error)
+DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
{
const char *buf;
const char *file = NULL;
@@ -225,8 +225,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi,
int *fatal_error)
int snapshot = 0;
int ret;
-*fatal_error = 1;
-
translation = BIOS_ATA_TRANSLATION_AUTO;
if (default_to_scsi) {
@@ -499,8 +497,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi,
int *fatal_error)
abort();
}
if (!file || !*file) {
-*fatal_error = 0;
-return NULL;
+return dinfo;
}
if (snapshot) {
/* always use cache=unsafe with snapshot */
@@ -529,7 +526,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi,
int *fatal_error)
if (bdrv_key_required(dinfo->bdrv))
autostart = 0;
-*fatal_error = 0;
return dinfo;
}
diff --git a/blockdev.h b/blockdev.h
index e5d8c56..84e462a 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -48,7 +48,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
QemuOpts *drive_def(const char *optstr);
QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
const char *optstr);
-DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
+DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi);
/* device-hotplug */
diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c
index 95a6372..8b2ed7a 100644
--- a/hw/device-hotplug.c
+++ b/hw/device-hotplug.c
@@ -29,7 +29,6 @@
DriveInfo *add_init_drive(const char *optstr)
{
-int fatal_error;
DriveInfo *dinfo;
QemuOpts *opts;
@@ -37,7 +36,7 @@ DriveInfo *add_init_drive(const char *optstr)
if (!opts)
return NULL;
-dinfo = drive_init(opts, current_machine->use_scsi, &fatal_error);
+dinfo = drive_init(opts, current_machine->use_scsi);
if (!dinfo) {
qemu_opts_del(opts);
return NULL;
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 11722c7..97d1e4a 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -542,7 +542,6 @@ static USBDevice *usb_msd_init(const char *filename)
QemuOpts *opts;
DriveInfo *dinfo;
USBDevice *dev;
-int fatal_error;
const char *p1;
char fmt[32];
@@ -572,7 +571,7 @@ static USBDevice *usb_msd_init(const char *filename)
qemu_opt_set(opts, "if", "none");
/* create host drive */
-dinfo = drive_init(opts, 0, &fatal_error);
+dinfo = drive_init(opts, 0);
if (!dinfo) {
qemu_opts_del(opts);
return NULL;
diff --git a/vl.c b/vl.c
index f86724f..ce5708b 100644
--- a/vl.c
+++ b/vl.c
@@ -631,13 +631,8 @@ static int bt_parse(const char *opt)
static int drive_init_func(QemuOpts *opts, void *opaque)
{
int *use_scsi = opaque;
-int fatal_error = 0;
-if (drive_init(opts, *use_scsi, &fatal_error) == NULL) {
-if (fatal_error)
-return 1;
-}
-return 0;
+return drive_init(opts, *use_scsi) == NULL;
}
static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
@@ -666,7 +661,7 @@ static void default_drive(int enable, int snapshot, int
use_scsi,
if (snapshot) {
drive_enable_snapshot(opts, NULL);
}
-if (drive_init_func(opts, &use_scsi)) {
+if (!drive_init(opts, use_scsi)) {
exit(1);
}
}
--
1.7.2.3
[Qemu-devel] [PATCH 24/28] blockdev: Factor drive_index_to_{bus, unit}_id out of drive_init()
From: Markus Armbruster
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c | 22 ++
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 6d828f2..a42c5e4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -75,6 +75,18 @@ void blockdev_auto_del(BlockDriverState *bs)
}
}
+static int drive_index_to_bus_id(BlockInterfaceType type, int index)
+{
+int max_devs = if_max_devs[type];
+return max_devs ? index / max_devs : 0;
+}
+
+static int drive_index_to_unit_id(BlockInterfaceType type, int index)
+{
+int max_devs = if_max_devs[type];
+return max_devs ? index % max_devs : index;
+}
+
QemuOpts *drive_def(const char *optstr)
{
return qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
@@ -382,14 +394,8 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi,
int *fatal_error)
error_report("index cannot be used with bus and unit");
return NULL;
}
-if (max_devs == 0)
-{
-unit_id = index;
-bus_id = 0;
-} else {
-unit_id = index % max_devs;
-bus_id = index / max_devs;
-}
+bus_id = drive_index_to_bus_id(type, index);
+unit_id = drive_index_to_unit_id(type, index);
}
/* if user doesn't specify a unit_id,
--
1.7.2.3
[Qemu-devel] [PATCH 23/28] blockdev: Make drive_add() take explicit type, index parameters
From: Markus Armbruster
Before, type & index were hidden in printf-like fmt, ... parameters,
which get expanded into an option string. Rather inconvenient for
uses later in this series.
New IF_DEFAULT to ask for the machine's default interface. Before,
that was done by having no option "if" in the option string.
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c | 20 +---
blockdev.h |8 +++-
hw/device-hotplug.c |2 +-
vl.c| 43 +++
4 files changed, 48 insertions(+), 25 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index fba53f9..6d828f2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -75,20 +75,34 @@ void blockdev_auto_del(BlockDriverState *bs)
}
}
-QemuOpts *drive_add(const char *file, const char *fmt, ...)
+QemuOpts *drive_def(const char *optstr)
+{
+return qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
+}
+
+QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
+const char *fmt, ...)
{
va_list ap;
char optstr[1024];
QemuOpts *opts;
+char buf[32];
va_start(ap, fmt);
vsnprintf(optstr, sizeof(optstr), fmt, ap);
va_end(ap);
-opts = qemu_opts_parse(qemu_find_opts("drive"), optstr, 0);
+opts = drive_def(optstr);
if (!opts) {
return NULL;
}
+if (type != IF_DEFAULT) {
+qemu_opt_set(opts, "if", if_name[type]);
+}
+if (index >= 0) {
+snprintf(buf, sizeof(buf), "%d", index);
+qemu_opt_set(opts, "index", buf);
+}
if (file)
qemu_opt_set(opts, "file", file);
return opts;
@@ -473,7 +487,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi,
int *fatal_error)
if (devaddr)
qemu_opt_set(opts, "addr", devaddr);
break;
-case IF_COUNT:
+default:
abort();
}
if (!file || !*file) {
diff --git a/blockdev.h b/blockdev.h
index cf8fc01..0c01e08 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -19,6 +19,7 @@ void blockdev_auto_del(BlockDriverState *bs);
#define BLOCK_SERIAL_STRLEN 20
typedef enum {
+IF_DEFAULT = -1,/* for use with drive_add() only */
IF_NONE,
IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
IF_COUNT
@@ -43,7 +44,12 @@ DriveInfo *drive_get_next(BlockInterfaceType type);
void drive_uninit(DriveInfo *dinfo);
DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
-QemuOpts *drive_add(const char *file, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+QemuOpts *drive_def(const char *optstr);
+QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
+const char *fmt, ...) /*GCC_FMT_ATTR(4, 5)*/;
+/* GCC_FMT_ATTR() commented out to avoid the (pretty useless)
+ * "zero-length gnu_printf format string" warning we insist to
+ * enable */
DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
/* device-hotplug */
diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c
index 9704e2f..95a6372 100644
--- a/hw/device-hotplug.c
+++ b/hw/device-hotplug.c
@@ -33,7 +33,7 @@ DriveInfo *add_init_drive(const char *optstr)
DriveInfo *dinfo;
QemuOpts *opts;
-opts = drive_add(NULL, "%s", optstr);
+opts = drive_def(optstr);
if (!opts)
return NULL;
diff --git a/vl.c b/vl.c
index 14255c4..5399e33 100644
--- a/vl.c
+++ b/vl.c
@@ -621,12 +621,13 @@ static int bt_parse(const char *opt)
/***/
/* QEMU Block devices */
-#define HD_ALIAS "index=%d,media=disk"
-#define CDROM_ALIAS "index=2,media=cdrom"
-#define FD_ALIAS "index=%d,if=floppy"
-#define PFLASH_ALIAS "if=pflash"
-#define MTD_ALIAS "if=mtd"
-#define SD_ALIAS "index=0,if=sd"
+/* Any % in the following strings must be escaped as %% */
+#define HD_OPTS "media=disk"
+#define CDROM_OPTS "media=cdrom"
+#define FD_OPTS ""
+#define PFLASH_OPTS ""
+#define MTD_OPTS ""
+#define SD_OPTS ""
static int drive_init_func(QemuOpts *opts, void *opaque)
{
@@ -1987,7 +1988,7 @@ int main(int argc, char **argv, char **envp)
if (optind >= argc)
break;
if (argv[optind][0] != '-') {
- hda_opts = drive_add(argv[optind++], HD_ALIAS, 0);
+ hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
} else {
const QEMUOption *popt;
@@ -2027,11 +2028,11 @@ int main(int argc, char **argv, char **envp)
break;
case QEMU_OPTION_hda:
if (cyls == 0)
-hda_opts = drive_add(optarg, HD_ALIAS, 0);
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS);
else
-hda_opts = drive_add(optarg, HD_ALIAS
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, HD_OPTS
",cyls=%d,heads=%d,secs=%d%s",
-
[Qemu-devel] [PATCH 09/28] Add documentation for STRTOSZ_DEFSUFFIX_ macros
From: Jes Sorensen Signed-off-by: Jes Sorensen Acked-by: Markus Armbruster Signed-off-by: Kevin Wolf --- qemu-common.h |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/qemu-common.h b/qemu-common.h index c351131..79e1149 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -153,6 +153,13 @@ int qemu_fls(int i); int qemu_fdatasync(int fd); int fcntl_setfl(int fd, int flag); +/* + * strtosz() suffixes used to specify the default treatment of an + * argument passed to strtosz() without an explicit suffix. + * These should be defined using upper case characters in the range + * A-Z, as strtosz() will use qemu_toupper() on the given argument + * prior to comparison. + */ #define STRTOSZ_DEFSUFFIX_TB 'T' #define STRTOSZ_DEFSUFFIX_GB 'G' #define STRTOSZ_DEFSUFFIX_MB 'M' -- 1.7.2.3
[Qemu-devel] [PATCH 25/28] blockdev: New drive_get_by_index()
From: Markus Armbruster
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c |7 +++
blockdev.h |1 +
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a42c5e4..01228f6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -136,6 +136,13 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int
unit)
return NULL;
}
+DriveInfo *drive_get_by_index(BlockInterfaceType type, int index)
+{
+return drive_get(type,
+ drive_index_to_bus_id(type, index),
+ drive_index_to_unit_id(type, index));
+}
+
int drive_get_max_bus(BlockInterfaceType type)
{
int max_bus;
diff --git a/blockdev.h b/blockdev.h
index 0c01e08..18278cc 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -39,6 +39,7 @@ struct DriveInfo {
};
DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
+DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
int drive_get_max_bus(BlockInterfaceType type);
DriveInfo *drive_get_next(BlockInterfaceType type);
void drive_uninit(DriveInfo *dinfo);
--
1.7.2.3
[Qemu-devel] [PATCH 20/28] blockdev: Move BlockInterfaceType from qemu-common.h to blockdev.h
From: Markus Armbruster
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.h|6 ++
qemu-common.h |6 --
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/blockdev.h b/blockdev.h
index 3ed6634..2cbbb87 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -18,6 +18,12 @@ void blockdev_auto_del(BlockDriverState *bs);
#define BLOCK_SERIAL_STRLEN 20
+typedef enum {
+IF_NONE,
+IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+IF_COUNT
+} BlockInterfaceType;
+
struct DriveInfo {
BlockDriverState *bdrv;
char *id;
diff --git a/qemu-common.h b/qemu-common.h
index 79e1149..c7ff280 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -274,12 +274,6 @@ typedef struct VirtIODevice VirtIODevice;
typedef uint64_t pcibus_t;
-typedef enum {
-IF_NONE,
-IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
-IF_COUNT
-} BlockInterfaceType;
-
void cpu_exec_init_all(unsigned long tb_size);
/* CPU save/load. */
--
1.7.2.3
[Qemu-devel] [PATCH 19/28] blockdev: New drive_get_next(), replacing qdev_init_bdrv()
From: Markus Armbruster
qdev_init_bdrv() doesn't belong into qdev.c; it's about drives, not
qdevs. Rename to drive_get_next, move to blockdev.c, drop the bogus
DeviceState argument, and return DriveInfo instead of
BlockDriverState.
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c | 10 ++
blockdev.h |1 +
hw/pl181.c |7 ---
hw/qdev.c | 14 --
hw/qdev.h |2 --
hw/ssi-sd.c |7 ---
6 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index e48d33d..699f312 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -93,6 +93,16 @@ int drive_get_max_bus(BlockInterfaceType type)
return max_bus;
}
+/* Get a block device. This should only be used for single-drive devices
+ (e.g. SD/Floppy/MTD). Multi-disk devices (scsi/ide) should use the
+ appropriate bus. */
+DriveInfo *drive_get_next(BlockInterfaceType type)
+{
+static int next_block_unit[IF_COUNT];
+
+return drive_get(type, 0, next_block_unit[type]++);
+}
+
DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
{
DriveInfo *dinfo;
diff --git a/blockdev.h b/blockdev.h
index b8a88bf..3ed6634 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -36,6 +36,7 @@ struct DriveInfo {
DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
int drive_get_max_bus(BlockInterfaceType type);
+DriveInfo *drive_get_next(BlockInterfaceType type);
void drive_uninit(DriveInfo *dinfo);
DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
diff --git a/hw/pl181.c b/hw/pl181.c
index 3e5f92f..36d9d02 100644
--- a/hw/pl181.c
+++ b/hw/pl181.c
@@ -7,6 +7,7 @@
* This code is licenced under the GPL.
*/
+#include "blockdev.h"
#include "sysbus.h"
#include "sd.h"
@@ -449,15 +450,15 @@ static int pl181_init(SysBusDevice *dev)
{
int iomemtype;
pl181_state *s = FROM_SYSBUS(pl181_state, dev);
-BlockDriverState *bd;
+DriveInfo *dinfo;
iomemtype = cpu_register_io_memory(pl181_readfn, pl181_writefn, s,
DEVICE_NATIVE_ENDIAN);
sysbus_init_mmio(dev, 0x1000, iomemtype);
sysbus_init_irq(dev, &s->irq[0]);
sysbus_init_irq(dev, &s->irq[1]);
-bd = qdev_init_bdrv(&dev->qdev, IF_SD);
-s->card = sd_init(bd, 0);
+dinfo = drive_get_next(IF_SD);
+s->card = sd_init(dinfo ? dinfo->bdrv : NULL, 0);
qemu_register_reset(pl181_reset, s);
pl181_reset(s);
/* ??? Save/restore. */
diff --git a/hw/qdev.c b/hw/qdev.c
index 5b8d374..0c94fb2 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -458,20 +458,6 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
}
}
-static int next_block_unit[IF_COUNT];
-
-/* Get a block device. This should only be used for single-drive devices
- (e.g. SD/Floppy/MTD). Multi-disk devices (scsi/ide) should use the
- appropriate bus. */
-BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type)
-{
-int unit = next_block_unit[type]++;
-DriveInfo *dinfo;
-
-dinfo = drive_get(type, 0, unit);
-return dinfo ? dinfo->bdrv : NULL;
-}
-
BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
{
BusState *bus;
diff --git a/hw/qdev.h b/hw/qdev.h
index e520aaa..9808f85 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -137,8 +137,6 @@ bool qdev_machine_modified(void);
qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
-BlockDriverState *qdev_init_bdrv(DeviceState *dev, BlockInterfaceType type);
-
BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
/*** Device API. ***/
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index a1a63b2..fb4b649 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -7,6 +7,7 @@
* This code is licenced under the GNU GPL v2.
*/
+#include "blockdev.h"
#include "ssi.h"
#include "sd.h"
@@ -231,11 +232,11 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int
version_id)
static int ssi_sd_init(SSISlave *dev)
{
ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev);
-BlockDriverState *bs;
+DriveInfo *dinfo;
s->mode = SSI_SD_CMD;
-bs = qdev_init_bdrv(&dev->qdev, IF_SD);
-s->sd = sd_init(bs, 1);
+dinfo = drive_get_next(IF_SD);
+s->sd = sd_init(dinfo ? dinfo->bdrv : NULL, 1);
register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
return 0;
}
--
1.7.2.3
[Qemu-devel] [PATCH 26/28] blockdev: Reject multiple definitions for the same drive
From: Markus Armbruster
We silently ignore multiple definitions for the same drive:
$ qemu-system-x86_64 -nodefaults -vnc :1 -S -monitor stdio -drive
if=ide,index=1,file=tmp.qcow2 -drive if=ide,index=1,file=nonexistant
QEMU 0.13.50 monitor - type 'help' for more information
(qemu) info block
ide0-hd1: type=hd removable=0 file=tmp.qcow2 backing_file=tmp.img ro=0
drv=qcow2 encrypted=0
With if=none, this can become quite confusing:
$ qemu-system-x86_64 -nodefaults -vnc :1 -S -monitor stdio -drive
if=none,index=1,file=tmp.qcow2,id=eins -drive
if=none,index=1,file=nonexistant,id=zwei -device ide-drive,drive=eins -device
ide-drive,drive=zwei
qemu-system-x86_64: -device ide-drive,drive=zwei: Property
'ide-drive.drive' can't find value 'zwei'
The second -device fails, because it refers to drive zwei, which got
silently ignored.
Make multiple drive definitions fail cleanly.
Unfortunately, there's code that relies on multiple drive definitions
being silently ignored: main() merrily adds default drives even when
the user already defined these drives. Fix that up.
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c |5 +++--
vl.c | 45 ++---
2 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 01228f6..3f7b560 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -429,11 +429,12 @@ DriveInfo *drive_init(QemuOpts *opts, int
default_to_scsi, int *fatal_error)
}
/*
- * ignore multiple definitions
+ * catch multiple definitions
*/
if (drive_get(type, bus_id, unit_id) != NULL) {
-*fatal_error = 0;
+error_report("drive with bus=%d, unit=%d (index=%d) exists",
+ bus_id, unit_id, index);
return NULL;
}
diff --git a/vl.c b/vl.c
index 5399e33..99f5bfe 100644
--- a/vl.c
+++ b/vl.c
@@ -649,6 +649,29 @@ static int drive_enable_snapshot(QemuOpts *opts, void
*opaque)
return 0;
}
+static void default_drive(int enable, int snapshot, int use_scsi,
+ BlockInterfaceType type, int index,
+ const char *optstr)
+{
+QemuOpts *opts;
+
+if (type == IF_DEFAULT) {
+type = use_scsi ? IF_SCSI : IF_IDE;
+}
+
+if (!enable || drive_get_by_index(type, index)) {
+return;
+}
+
+opts = drive_add(type, index, NULL, optstr);
+if (snapshot) {
+drive_enable_snapshot(opts, NULL);
+}
+if (drive_init_func(opts, &use_scsi)) {
+exit(1);
+}
+}
+
void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
{
boot_set_handler = func;
@@ -2893,27 +2916,19 @@ int main(int argc, char **argv, char **envp)
blk_mig_init();
-if (default_cdrom) {
-/* we always create the cdrom drive, even if no disk is there */
-drive_add(IF_DEFAULT, 2, NULL, CDROM_OPTS);
-}
-
-if (default_floppy) {
-/* we always create at least one floppy */
-drive_add(IF_FLOPPY, 0, NULL, FD_OPTS);
-}
-
-if (default_sdcard) {
-/* we always create one sd slot, even if no card is in it */
-drive_add(IF_SD, 0, NULL, SD_OPTS);
-}
-
/* open the virtual block devices */
if (snapshot)
qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
NULL, 0);
if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
&machine->use_scsi, 1) != 0)
exit(1);
+default_drive(default_cdrom, snapshot, machine->use_scsi,
+ IF_DEFAULT, 2, CDROM_OPTS);
+default_drive(default_floppy, snapshot, machine->use_scsi,
+ IF_FLOPPY, 0, FD_OPTS);
+default_drive(default_sdcard, snapshot, machine->use_scsi,
+ IF_SD, 0, SD_OPTS);
+
register_savevm_live(NULL, "ram", 0, 4, NULL, ram_save_live, NULL,
ram_load, NULL);
--
1.7.2.3
[Qemu-devel] [PATCH 13/28] qemu-io: Fix discard command
qemu-io passed bytes where it's supposed to pass sectors, so discard requests
were off.
Signed-off-by: Kevin Wolf
Reviewed-by: Stefan Hajnoczi
---
qemu-io.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 5b24c5e..4470e49 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -1465,7 +1465,7 @@ discard_f(int argc, char **argv)
}
gettimeofday(&t1, NULL);
- ret = bdrv_discard(bs, offset, count);
+ ret = bdrv_discard(bs, offset >> BDRV_SECTOR_BITS, count >>
BDRV_SECTOR_BITS);
gettimeofday(&t2, NULL);
if (ret < 0) {
--
1.7.2.3
Re: [Qemu-devel] [PATCH 03/11] arm: drop unused irq-related part of CPUARMState
On 31 January 2011 15:20, Dmitry Eremin-Solenikov wrote: > These two fields were added as a part of ARMv7 support patch (back in > 2007), were never used by any code, so can be dropped. > > Signed-off-by: Dmitry Eremin-Solenikov Reviewed-by: Peter Maydell -- PMM
Re: [Qemu-devel] [PATCH] target-arm: Set the right overflow bit for neon 32 and 64 bit saturating add/sub.
On 24.01.2011 13:41, Christophe Lyon wrote: > Here is an updated patch with these minor fixes. > ping?
[Qemu-devel] [PATCH 22/28] blockdev: Fix regression in -drive if=scsi, index=N
From: Markus Armbruster
Before commit 622b520f, index=12 meant bus=1,unit=5.
Since the commit, it means bus=0,unit=12. The drive is created, but
not the guest device. That's because the controllers we use with
if=scsi drives (lsi53c895a and esp) support only 7 units, and
scsi_bus_legacy_handle_cmdline() ignores drives with unit numbers
exceeding that limit.
Changing the mapping of index to bus, unit is a regression. Breaking
-drive invocations that used to work just makes it worse.
Revert the part of commit 622b520f that causes this, and clean up
some.
Note that the fix only affects if=scsi. You can still put more than 7
units on a SCSI bus with -device & friends.
Signed-off-by: Markus Armbruster
Signed-off-by: Kevin Wolf
---
blockdev.c| 18 --
blockdev.h|3 ---
hw/ide.h |2 ++
hw/ide/ahci.c |1 -
hw/qdev.c |1 -
hw/scsi.h |3 ++-
savevm.c |1 -
7 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 9c883ce..fba53f9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -32,8 +32,22 @@ static const char *const if_name[IF_COUNT] = {
};
static const int if_max_devs[IF_COUNT] = {
-[IF_IDE] = MAX_IDE_DEVS,
-[IF_SCSI] = MAX_SCSI_DEVS,
+/*
+ * Do not change these numbers! They govern how drive option
+ * index maps to unit and bus. That mapping is ABI.
+ *
+ * All controllers used to imlement if=T drives need to support
+ * if_max_devs[T] units, for any T with if_max_devs[T] != 0.
+ * Otherwise, some index values map to "impossible" bus, unit
+ * values.
+ *
+ * For instance, if you change [IF_SCSI] to 255, -drive
+ * if=scsi,index=12 no longer means bus=1,unit=5, but
+ * bus=0,unit=12. With an lsi53c895a controller (7 units max),
+ * the drive can't be set up. Regression.
+ */
+[IF_IDE] = 2,
+[IF_SCSI] = 7,
};
/*
diff --git a/blockdev.h b/blockdev.h
index 2cbbb87..cf8fc01 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -37,9 +37,6 @@ struct DriveInfo {
QTAILQ_ENTRY(DriveInfo) next;
};
-#define MAX_IDE_DEVS 2
-#define MAX_SCSI_DEVS 255
-
DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
int drive_get_max_bus(BlockInterfaceType type);
DriveInfo *drive_get_next(BlockInterfaceType type);
diff --git a/hw/ide.h b/hw/ide.h
index 2b5ae7c..73fb550 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -4,6 +4,8 @@
#include "isa.h"
#include "pci.h"
+#define MAX_IDE_DEVS 2
+
/* ide-isa.c */
ISADevice *isa_ide_init(int iobase, int iobase2, int isairq,
DriveInfo *hd0, DriveInfo *hd1);
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 433171c..671b4df 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -70,7 +70,6 @@
#include "monitor.h"
#include "dma.h"
#include "cpu-common.h"
-#include "blockdev.h"
#include "internal.h"
#include
diff --git a/hw/qdev.c b/hw/qdev.c
index 0c94fb2..c7fec44 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,7 +29,6 @@
#include "qdev.h"
#include "sysemu.h"
#include "monitor.h"
-#include "blockdev.h"
static int qdev_hotplug = 0;
static bool qdev_hot_added = false;
diff --git a/hw/scsi.h b/hw/scsi.h
index 846fbba..d3b5d56 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -3,9 +3,10 @@
#include "qdev.h"
#include "block.h"
-#include "blockdev.h"
#include "block_int.h"
+#define MAX_SCSI_DEVS 255
+
#define SCSI_CMD_BUF_SIZE 16
/* scsi-disk.c */
diff --git a/savevm.c b/savevm.c
index fcd8db4..4453217 100644
--- a/savevm.c
+++ b/savevm.c
@@ -78,7 +78,6 @@
#include "sysemu.h"
#include "qemu-timer.h"
#include "qemu-char.h"
-#include "blockdev.h"
#include "audio/audio.h"
#include "migration.h"
#include "qemu_socket.h"
--
1.7.2.3
[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop
On 2011-01-31 14:04, Jan Kiszka wrote:
> On 2011-01-31 12:36, Jan Kiszka wrote:
>> On 2011-01-31 11:08, Avi Kivity wrote:
>>> On 01/27/2011 03:10 PM, Jan Kiszka wrote:
Align with qemu-kvm and prepare for IO exit fix: There is no need to run
kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
this service processes will first cause an exit from kvm_cpu_exec
anyway. And we will have to reenter the kernel on IO exits
unconditionally, something that the current logic prevents.
Signed-off-by: Jan Kiszka
---
kvm-all.c | 11 ++-
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index 5bfa8c0..46ecc1c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
DPRINTF("kvm_cpu_exec()\n");
+if (kvm_arch_process_irqchip_events(env)) {
+env->exit_request = 0;
+env->exception_index = EXCP_HLT;
+return 0;
+}
+
do {
#ifndef CONFIG_IOTHREAD
if (env->exit_request) {
@@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
}
>>>
>>> We check for ->exit_request here
>>>
#endif
-if (kvm_arch_process_irqchip_events(env)) {
-ret = 0;
-break;
-}
-
>>>
>>> But this checks for ->interrupt_request. What ensures that we exit when
>>> ->interrupt_request is set?
>>
>> Good question, need to check again. But if that turns out to be an
>> issue, qemu-kvm would be broken as well. I'm just aligning the code here.
>>
>
> The only thing we miss by moving process_irqchip_events is a self-INIT
> of an AP - if such thing exists in real life. In that case, the AP would
> cause a reset of itself, followed by a transition to HALT state.
I checked again with the Intel spec, and a self-INIT is invalid (at
least when specified via shorthand). So I'm under the impression now
that we can safely ignore this case and leave the patch as is.
Any different views?
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH 03/28] strtosz(): Fix name confusion in use of modf()
From: Jes Sorensen
Signed-off-by: Jes Sorensen
Signed-off-by: Kevin Wolf
---
cutils.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/cutils.c b/cutils.c
index 78d35e2..369a016 100644
--- a/cutils.c
+++ b/cutils.c
@@ -304,8 +304,8 @@ int64_t strtosz_suffix(const char *nptr, char **end, const
char default_suffix)
if (isnan(val) || endptr == nptr || errno != 0) {
goto fail;
}
-integral = modf(val, &fraction);
-if (integral != 0) {
+fraction = modf(val, &integral);
+if (fraction != 0) {
mul_required = 1;
}
/*
--
1.7.2.3
Re: [Qemu-devel] [PATCH V9 10/16] xen: Introduce the Xen mapcache
On Wed, 26 Jan 2011, Anthony Liguori wrote:
> > +void qemu_invalidate_map_cache(void)
> > +{
> > +unsigned long i;
> > +MapCacheRev *reventry;
> > +
> > +qemu_aio_flush();
> >
>
> This is bizarre? Why is this needed?
>
Sorry, I forgot to reply to this question.
Xen might send an IOREQ_TYPE_INVALIDATE at any time and we have to
flush the mapcache in response. Of course we cannot flush the mapcache
if there are any in flight AIO's, so we have to make sure they are all
completed before destroying all the mappings.
Maybe there is better API to do that than calling qemu_aio_flush()?
[Qemu-devel] Re: [PATCH v3 14/22] kvm: Fix race between timer signals and vcpu entry under !IOTHREAD
On 01/31/2011 04:31 PM, Jan Kiszka wrote: >> >> And how would you be kicked out of the select() call if it is waiting >> with a timeout? We only have a single thread here. > > If we use signalfd() (either kernel provided or thread+pipe), we kick > out of select by select()ing it (though I don't see how it works without > an iothread, since an fd can't stop a vcpu unless you enable SIGIO on > it, which is silly for signalfd) > > If you leave it as a naked signal, then it can break out of either > pselect() or vcpu. > > Since the goal is to drop !CONFIG_IOTHREAD, the first path seems better, > I just don't understand the problem with emulated signalfd(). > With the emulated signalfd, there won't be any signal for the VCPU while in KVM_RUN. I see it now - with a real signalfd, kvm unmasks the signal, and that takes precedence over signalfd and exits the vcpu. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop
On Mon, Jan 31, 2011 at 04:40:34PM +0100, Jan Kiszka wrote:
> On 2011-01-31 14:04, Jan Kiszka wrote:
> > On 2011-01-31 12:36, Jan Kiszka wrote:
> >> On 2011-01-31 11:08, Avi Kivity wrote:
> >>> On 01/27/2011 03:10 PM, Jan Kiszka wrote:
> Align with qemu-kvm and prepare for IO exit fix: There is no need to run
> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
> this service processes will first cause an exit from kvm_cpu_exec
> anyway. And we will have to reenter the kernel on IO exits
> unconditionally, something that the current logic prevents.
>
> Signed-off-by: Jan Kiszka
> ---
> kvm-all.c | 11 ++-
> 1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 5bfa8c0..46ecc1c 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
>
> DPRINTF("kvm_cpu_exec()\n");
>
> +if (kvm_arch_process_irqchip_events(env)) {
> +env->exit_request = 0;
> +env->exception_index = EXCP_HLT;
> +return 0;
> +}
> +
> do {
> #ifndef CONFIG_IOTHREAD
> if (env->exit_request) {
> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
> }
> >>>
> >>> We check for ->exit_request here
> >>>
> #endif
>
> -if (kvm_arch_process_irqchip_events(env)) {
> -ret = 0;
> -break;
> -}
> -
> >>>
> >>> But this checks for ->interrupt_request. What ensures that we exit when
> >>> ->interrupt_request is set?
> >>
> >> Good question, need to check again. But if that turns out to be an
> >> issue, qemu-kvm would be broken as well. I'm just aligning the code here.
> >>
> >
> > The only thing we miss by moving process_irqchip_events is a self-INIT
> > of an AP - if such thing exists in real life. In that case, the AP would
> > cause a reset of itself, followed by a transition to HALT state.
>
> I checked again with the Intel spec, and a self-INIT is invalid (at
> least when specified via shorthand). So I'm under the impression now
> that we can safely ignore this case and leave the patch as is.
>
> Any different views?
>
IIRC if you don't use shorthand you can send INIT to self.
--
Gleb.
Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
Christophe Lyon a écrit : > On 31.01.2011 10:44, Aurelien Jarno wrote: >> On Mon, Jan 31, 2011 at 10:35:30AM +0100, Christophe Lyon wrote: >>> On 31.01.2011 09:20, Aurelien Jarno wrote: On Fri, Jan 28, 2011 at 04:50:59PM +0100, christophe.l...@st.com wrote: > From: Christophe Lyon > > Handle corner cases where the addition of the rounding constant could > cause overflows. After applying this patch, I get the following gcc warning: CCtranslate.o cc1: warnings being treated as errors qemu/target-arm/translate.c: In function ‘disas_neon_data_insn’: qemu/target-arm/translate.c:4212: error: ‘imm’ may be used uninitialized in this function make: *** [translate.o] Error 1 >>> Which GCC version are you using? I don't have such a warning (using >>> GCC-4.5.1 on x86_64). >>> >> I get this error with GCC 4.3.5, GCC 4.4.5, GCC 4.5.2 and GCC 4.6.0 >> (r169270). This is also on x86_64. >> > > Well, I can't reproduce this error :-( > For the record, I configure with --target-list=arm-softmmu,arm-linux-user > --disable-bluez --enable-debug --disable-sdl and point --host-cc and --cc to > GCC-4.5.1. > It seems the problems come from here, if I add --enable-debug, I am not able to reproduce the problem anymore. I don't understand why though. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCHv2] fix linuxboot.bin and multiboot.bin to not hijack int19
On 31.01.2011, at 14:11, Gleb Natapov wrote: > Currently linuxboot.bin and multiboot.bin option roms override int19 > vector to intercept boot process. No sane option rom should do that. > Provide bev entry instead that will be called by BIOS if option rom > is selected for booting. No idea about the bev standard, but the rest looks good :) Acked-by: Alexander Graf Alex
[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop
On 2011-01-31 17:38, Gleb Natapov wrote:
> On Mon, Jan 31, 2011 at 04:40:34PM +0100, Jan Kiszka wrote:
>> On 2011-01-31 14:04, Jan Kiszka wrote:
>>> On 2011-01-31 12:36, Jan Kiszka wrote:
On 2011-01-31 11:08, Avi Kivity wrote:
> On 01/27/2011 03:10 PM, Jan Kiszka wrote:
>> Align with qemu-kvm and prepare for IO exit fix: There is no need to run
>> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
>> this service processes will first cause an exit from kvm_cpu_exec
>> anyway. And we will have to reenter the kernel on IO exits
>> unconditionally, something that the current logic prevents.
>>
>> Signed-off-by: Jan Kiszka
>> ---
>> kvm-all.c | 11 ++-
>> 1 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 5bfa8c0..46ecc1c 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
>>
>> DPRINTF("kvm_cpu_exec()\n");
>>
>> +if (kvm_arch_process_irqchip_events(env)) {
>> +env->exit_request = 0;
>> +env->exception_index = EXCP_HLT;
>> +return 0;
>> +}
>> +
>> do {
>> #ifndef CONFIG_IOTHREAD
>> if (env->exit_request) {
>> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
>> }
>
> We check for ->exit_request here
>
>> #endif
>>
>> -if (kvm_arch_process_irqchip_events(env)) {
>> -ret = 0;
>> -break;
>> -}
>> -
>
> But this checks for ->interrupt_request. What ensures that we exit when
> ->interrupt_request is set?
Good question, need to check again. But if that turns out to be an
issue, qemu-kvm would be broken as well. I'm just aligning the code here.
>>>
>>> The only thing we miss by moving process_irqchip_events is a self-INIT
>>> of an AP - if such thing exists in real life. In that case, the AP would
>>> cause a reset of itself, followed by a transition to HALT state.
>>
>> I checked again with the Intel spec, and a self-INIT is invalid (at
>> least when specified via shorthand). So I'm under the impression now
>> that we can safely ignore this case and leave the patch as is.
>>
>> Any different views?
>>
> IIRC if you don't use shorthand you can send INIT to self.
We didn't care so far (in qemu-kvm), do you think we should?
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 1/8] target-arm: Fixes for several shift instructions: VRSHL, VRSHR, VRSHRN, VSHLL, VRSRA.
On 31 January 2011 15:59, Aurelien Jarno wrote: > Christophe Lyon a écrit : >> Well, I can't reproduce this error :-( >> For the record, I configure with --target-list=arm-softmmu,arm-linux-user >> --disable-bluez --enable-debug --disable-sdl and point --host-cc and --cc to >> GCC-4.5.1. >> > > It seems the problems come from here, if I add --enable-debug, I am not > able to reproduce the problem anymore. I don't understand why though. --enable-debug turns off optimisation (ie does not pass -O2); a number of gcc's warnings, including this one, are only done in the dataflow analysis pass and so will not be generated unless you have optimisation enabled. -- PMM
[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop
On 2011-01-31 17:41, Jan Kiszka wrote:
> On 2011-01-31 17:38, Gleb Natapov wrote:
>> On Mon, Jan 31, 2011 at 04:40:34PM +0100, Jan Kiszka wrote:
>>> On 2011-01-31 14:04, Jan Kiszka wrote:
On 2011-01-31 12:36, Jan Kiszka wrote:
> On 2011-01-31 11:08, Avi Kivity wrote:
>> On 01/27/2011 03:10 PM, Jan Kiszka wrote:
>>> Align with qemu-kvm and prepare for IO exit fix: There is no need to run
>>> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
>>> this service processes will first cause an exit from kvm_cpu_exec
>>> anyway. And we will have to reenter the kernel on IO exits
>>> unconditionally, something that the current logic prevents.
>>>
>>> Signed-off-by: Jan Kiszka
>>> ---
>>> kvm-all.c | 11 ++-
>>> 1 files changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index 5bfa8c0..46ecc1c 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
>>>
>>> DPRINTF("kvm_cpu_exec()\n");
>>>
>>> +if (kvm_arch_process_irqchip_events(env)) {
>>> +env->exit_request = 0;
>>> +env->exception_index = EXCP_HLT;
>>> +return 0;
>>> +}
>>> +
>>> do {
>>> #ifndef CONFIG_IOTHREAD
>>> if (env->exit_request) {
>>> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
>>> }
>>
>> We check for ->exit_request here
>>
>>> #endif
>>>
>>> -if (kvm_arch_process_irqchip_events(env)) {
>>> -ret = 0;
>>> -break;
>>> -}
>>> -
>>
>> But this checks for ->interrupt_request. What ensures that we exit when
>> ->interrupt_request is set?
>
> Good question, need to check again. But if that turns out to be an
> issue, qemu-kvm would be broken as well. I'm just aligning the code here.
>
The only thing we miss by moving process_irqchip_events is a self-INIT
of an AP - if such thing exists in real life. In that case, the AP would
cause a reset of itself, followed by a transition to HALT state.
>>>
>>> I checked again with the Intel spec, and a self-INIT is invalid (at
>>> least when specified via shorthand). So I'm under the impression now
>>> that we can safely ignore this case and leave the patch as is.
>>>
>>> Any different views?
>>>
>> IIRC if you don't use shorthand you can send INIT to self.
>
> We didn't care so far (in qemu-kvm), do you think we should?
...and the kernel model should have barked "INIT on a runnable vcpu" in
such cases (BTW, that's user triggerable and should likely be rate limited).
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH 17/22] kvm: Move irqchip event processing out of inner loop
On Mon, Jan 31, 2011 at 05:52:13PM +0100, Jan Kiszka wrote:
> On 2011-01-31 17:50, Gleb Natapov wrote:
> > On Mon, Jan 31, 2011 at 05:41:24PM +0100, Jan Kiszka wrote:
> >> On 2011-01-31 17:38, Gleb Natapov wrote:
> >>> On Mon, Jan 31, 2011 at 04:40:34PM +0100, Jan Kiszka wrote:
> On 2011-01-31 14:04, Jan Kiszka wrote:
> > On 2011-01-31 12:36, Jan Kiszka wrote:
> >> On 2011-01-31 11:08, Avi Kivity wrote:
> >>> On 01/27/2011 03:10 PM, Jan Kiszka wrote:
> Align with qemu-kvm and prepare for IO exit fix: There is no need to
> run
> kvm_arch_process_irqchip_events in the inner VCPU loop. Any state
> change
> this service processes will first cause an exit from kvm_cpu_exec
> anyway. And we will have to reenter the kernel on IO exits
> unconditionally, something that the current logic prevents.
>
> Signed-off-by: Jan Kiszka
> ---
> kvm-all.c | 11 ++-
> 1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 5bfa8c0..46ecc1c 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -892,6 +892,12 @@ int kvm_cpu_exec(CPUState *env)
>
> DPRINTF("kvm_cpu_exec()\n");
>
> +if (kvm_arch_process_irqchip_events(env)) {
> +env->exit_request = 0;
> +env->exception_index = EXCP_HLT;
> +return 0;
> +}
> +
> do {
> #ifndef CONFIG_IOTHREAD
> if (env->exit_request) {
> @@ -901,11 +907,6 @@ int kvm_cpu_exec(CPUState *env)
> }
> >>>
> >>> We check for ->exit_request here
> >>>
> #endif
>
> -if (kvm_arch_process_irqchip_events(env)) {
> -ret = 0;
> -break;
> -}
> -
> >>>
> >>> But this checks for ->interrupt_request. What ensures that we exit
> >>> when
> >>> ->interrupt_request is set?
> >>
> >> Good question, need to check again. But if that turns out to be an
> >> issue, qemu-kvm would be broken as well. I'm just aligning the code
> >> here.
> >>
> >
> > The only thing we miss by moving process_irqchip_events is a self-INIT
> > of an AP - if such thing exists in real life. In that case, the AP would
> > cause a reset of itself, followed by a transition to HALT state.
>
> I checked again with the Intel spec, and a self-INIT is invalid (at
> least when specified via shorthand). So I'm under the impression now
> that we can safely ignore this case and leave the patch as is.
>
> Any different views?
>
> >>> IIRC if you don't use shorthand you can send INIT to self.
> >>
> >> We didn't care so far (in qemu-kvm), do you think we should?
> >>
> > Doesn't kernel lapic emulation support this?
>
> See the my other mail: It supports it, but it apparently doesn't expects
> this to happen.
>
I saw it, but I do not understand why do we print this message. May be
it was used for debugging in early stages of KVM development.
--
Gleb.
