Re: [Qemu-devel] [RESEND][PATCH 1/9] pxa2xx_pcmcia: qdevify

2011-05-03 Thread Dmitry Eremin-Solenikov
Hello, colleagues,

On 4/29/11, Dmitry Eremin-Solenikov  wrote:
> Hello,
>
> Any chance to get any response for this patches serie?

Still no response at all. Is there anything wrong with my approach? Is there
any problem with patch format? Am I just being ignored?

> On 4/25/11, Dmitry Eremin-Solenikov  wrote:
>> Use qdev insfrastructure for managing PXA PCMCIA devices. PCMCIA bus
>> itself isn't converted to QBus (yet). pxa2xx_pcmcia_init() function is
>> moved to pcmcia.h as it will be used by other cores/devices (like
>> StrongARM).
>>
>> Signed-off-by: Dmitry Eremin-Solenikov 
>
> --
> With best wishes
> Dmitry
>


-- 
With best wishes
Dmitry



[Qemu-devel] ­ How to cross compile QEMU

2011-05-03 Thread 李柏舉
Hi, all

 I have built cross toolchain for arm , but I cannot cross compile QEMU

qemu version: qemu-0.14.0
cross-compiler: arm-2007q3 (arm-none-linux-gnueabi)
­The cross compiler I used is downloaded from:
http://www.codesourcery.com/


 The following are my steps:

 1. below is my configure
 #!/bin/bash
 SOURCE_DIR=../qemu-0.14.0

 ZLIB_PREBUILT=/home/poki/hybridQ/qemu/qemu-0.14.0/package/zlib-1.2.3-arm
 TARGET_LIST=arm-linux-user
 CROSS_PREFIX=arm-none-linux-gnueabi-

 CPU=armv4l
 CFLAGS=-I$ZLIB_PREBUILT/include
 LDFLAGS=-L$ZLIB_PREBUILT/lib
 PREFIX=/home/poki/hybridQ/qemu/install
 OPT_FLAGS=" --disable-sdl"
 DEBUG=--disable-strip
 STATIC=--static

 $SOURCE_DIR/configure --target-list=$TARGET_LIST --cpu=$CPU
 --extra-ldflags=$LDFLAGS  --extra- cflags=$CFLAGS
--cross-prefix=$CROSS_PREFIX  --prefix=$PREFIX

 warning message:
 warning: proceeding without arm-none-linux-gnueabi-pkg-config
 warning: using "sdl-config" to detect cross-compiled sdl

  2. make install
  error message:
  .
  .
   LINK  arm-linux-user/qemu-arm

exec.o: In function `tb_set_jmp_target1':
/home/poki/hybridQ/qemu/qemu-0.14.0/exec-all.h:219: undefined reference to
`__builtin___clear_cache'
/home/poki/hybridQ/qemu/qemu-0.14.0/exec-all.h:219: undefined reference to
`__builtin___clear_cache'
/home/poki/hybridQ/qemu/qemu-0.14.0/exec-all.h:219: undefined reference to
`__builtin___clear_cache'
/home/poki/hybridQ/qemu/qemu-0.14.0/exec-all.h:219: undefined reference to
`__builtin___clear_cache'
/home/poki/hybridQ/qemu/qemu-0.14.0/exec-all.h:219: undefined reference to
`__builtin___clear_cache'
cpu-exec.o:/home/poki/hybridQ/qemu/qemu-0.14.0/exec-all.h:219: more
undefined references to `__builtin___clear_cache' follow
collect2: ld returned 1 exit status
make[1]: *** [qemu-arm] Error 1
make: *** [subdir-arm-linux-user] Error 2

Regards,
Poki


Re: [Qemu-devel] ­ How to cross compile QEMU

2011-05-03 Thread Jan-Simon Möller
Am Dienstag, 3. Mai 2011, 11:13:35 schrieb 李柏舉:
> arm-2007q3

Try a newer cross-compiler please and then report back.

Best,
Jan-Simon



Re: [Qemu-devel] [PATCH 2/2 V7] qemu, qmp: add inject-nmi qmp command

2011-05-03 Thread Jamie Lokier
Gleb Natapov wrote:
> On Thu, Apr 07, 2011 at 04:39:58PM -0500, Anthony Liguori wrote:
> > On 04/07/2011 01:51 PM, Gleb Natapov wrote:
> > >NMI does not have to generate crash dump on every guest we support.
> > >Actually even for windows guest it does not generate one without
> > >tweaking registry. For all I know there is a guest that checks mail when
> > >NMI arrives.
> > 
> > And for all we know, a guest can respond to an ACPI poweroff event
> > by tweeting the star spangled banner but we still call the
> > corresponding QMP command system_poweroff.
> > 
> Correct :) But at least system_poweroff implements ACPI poweroff as
> defined by ACPI spec. NMI is not designed as core dump event and is not
> used as such by majority of the guests.

Imho acpi_poweroff or poweroff_button would have been a clearer name.
Or even 'sendkey poweroff' - it's just a button someone on the
keyboard on a lot of systems anyway.  Next to the email button and what
looks, on my laptop, like the play-a-tune button :-)

I put system_poweroff into some QEMU-controlling scripts once, and was
disappointed when several guests ignored it.

But it's done now.

-- Jamie




Re: [Qemu-devel] [PATCH 2/2 V7] qemu, qmp: add inject-nmi qmp command

2011-05-03 Thread Jamie Lokier
Blue Swirl wrote:
> On Fri, Apr 8, 2011 at 9:04 AM, Gleb Natapov  wrote:
> > On Thu, Apr 07, 2011 at 04:41:03PM -0500, Anthony Liguori wrote:
> >> On 04/07/2011 02:17 PM, Gleb Natapov wrote:
> >> >On Thu, Apr 07, 2011 at 10:04:00PM +0300, Blue Swirl wrote:
> >> >>On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov  wrote:
> >> >>
> >> >>I'd prefer something more generic like these:
> >> >>raise /apic@fee0:l1int
> >> >>lower /i44FX-pcihost/e1000@03.0/pinD
> >> >>
> >> >>The clumsier syntax shouldn't be a problem, since this would be a
> >> >>system developer tool.
> >> >>
> >> >>Some kind of IRQ registration would be needed for this to work without
> >> >>lots of changes.
> >> >True. The ability to trigger any interrupt line is very useful for
> >> >debugging. I often re-implement it during debug.
> >>
> >> And it's a good thing to have, but exposing this as the only API to
> >> do something as simple as generating a guest crash dump is not the
> >> friendliest thing in the world to do to users.
> >>
> > Well, this is not intended to be used by regular users directly and
> > management can provide nicer interface for issuing NMI. But really,
> > my point is that NMI actually generates guest core dump in such rare
> > cases (only preconfigured Windows guests) that it doesn't warrant to
> > name command as such. Management is in much better position to implement
> > functionality with such name since it knows what type of guest it runs
> > and can tell agent to configure guest accordingly.
> 
> Does the management need to know about each and every debugging
> oriented interface? For example, "info regs",  "info mem", "info irq"
> and tracepoints?

Linux uses NMI for performance tracing, profiling, watchdog etc. so in
practice, NMI is very similar to the other IRQs.  I.e. highly guest
specific and depending on what's wired up to it.  Injecting NMI to all
CPUs at once does not make any sense for those Linux guests.

For Windows crash dumps, I think it makes sense to have a "button
wired to NMI" device, rather than inject-nmi directly, but I can see
that inject-nmi solves the intended problem quite neatly.

For Linux crash dumps, for example, there are other key combinations,
as well as watchdog devices, that can be used to trigger them.  A
virtual "button wired to GPIO/PCI-IRQ/etc." device might be quite
handy for debugging Linux guests, and would fit comfortably in a
management interface.

-- Jamie



Re: [Qemu-devel] [PATCH v2] MSI: Robust resource release

2011-05-03 Thread Alexander Graf

On 02.05.2011, at 20:00, Jan Kiszka wrote:

> msi_init may fail, so we need to check on uninit if the cap was
> actually installed. This also avoids that the users need to check.
> 
> Signed-off-by: Jan Kiszka 

Looks good to me.

Acked-by: Alexander Graf 


Alex




Re: [Qemu-devel] [PATCH 1/2] ppc64: Fix out-of-tree builds

2011-05-03 Thread Alexander Graf

On 01.05.2011, at 18:31, Andreas Färber wrote:

> Someone on Linux should check if a similar patch is necessary for roms/SLOF/:

Mind to just send a patch I can try out with some instruction on how to trigger 
this? :)


Alex




[Qemu-devel] KVM call agenda for May 03rd

2011-05-03 Thread Juan Quintela

Please send in any agenda items you are interested in covering.

Later, Juan.



Re: [Qemu-devel] [PATCH] kvm: ppc: fixes for KVM_SET_SREGS on init

2011-05-03 Thread Alexander Graf

On 11.04.2011, at 18:15, Scott Wood wrote:

> On Sat, 9 Apr 2011 02:18:34 +0200
> Alexander Graf  wrote:
> 
>>> -int kvm_arch_init_vcpu(CPUState *cenv)
>>> +static int kvm_arch_sync_sregs(CPUState *cenv)
>> 
>> huh? So what about the previous caller of this?
> 
> It's a new function.  kvm_arch_init_vcpu still exists as a public
> function, "introduced" later in the patch.  Diff doesn't know why this line
> is more important than the sregs definition.
> 
>>> {
>>> -int ret = 0;
>>>struct kvm_sregs sregs;
>>> +int ret;
>> 
>> Eh - this makes the patch less readable :)
> 
> I can flip them around in the new function if you want, though having the
> longer declaration first looks a bit nicer to me.
> 
>>> +#ifdef TARGET_PPC
>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>> 
>> This code never gets compiled without TARGET_PPC?
> 
> Hmm, thought I checked that TARGET_PPC wasn't set in a TARGET_PPCEMB build,
> but now I see it is.  Would be nice if we had a define specifically for
> non-PPCEMB.
> 
>>> +if (!kvm_check_extension(cenv->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>> +return 0;
>>> +}
>>> +#else
>>> +return 0;
>> 
>> Doing a simple return 0 might lead to warnings (which become errors with 
>> -Werror) due to unused variables. I'm not sure how to handle this well. 
>> Maybe define KVM_CAP_PPC_SEGSTATE to something invalid when it's not 
>> defined? That way the capability check would fail and we don't need the 
>> duplicate code paths.
> 
> Which variables would be unused?  sregs/ret are used, just in a dead
> portion of the function.  If the rest of the function had been ifdeffed out
> instead, it would be an issue.

I've had gcc be overly clever in situations like this before, where it detects 
the return is unconditional and just removes all the code after it, generating 
unused warnings.

> 
>>> +#endif
>>> +#else /* TARGET_PPCEMB */
>> 
>> I guess you were #ifdefing on PPCEMB before? I don't think you really need 
>> to care about PPCEMB. The only reason it exists is for page size < 4k, which 
>> you don't hit on e500 IIUC.
> 
> PPCEMB is how we've been running this so far... it also involves a larger
> target_phys_addr_t.  I didn't know it was supposed to be supported at all
> under plain PPC.
> 
> If that really is supposed to be supported, then we'll need a dynamic check
> here instead (based on excp_model?), but I don't see the value in
> supporting that.  I did find it odd that all ppc platforms are being built
> for both PPC and PPCEMB.

I don't disagree that it's odd, but that's the way it is. We even support 
32-bit CPUs in the PPC64 target. This patch surely isn't in the scope of 
changing it :). So yes, all the code has to work with qemu-system-ppc.


Alex




Re: [Qemu-devel] [PATCH v2] monitor: add PPC BookE SPRs

2011-05-03 Thread Alexander Graf

On 30.04.2011, at 00:10, Scott Wood wrote:

> Read them via KVM_GET_SREGS in kvm_arch_get_registers(),
> and display them in "info registers".
> 
> Also get CR and PID from the existing KVM_GET_REGS.
> 
> Signed-off-by: Scott Wood 

Nice patch :)


Alex




Re: [Qemu-devel] [PATCH v2] monitor: add PPC BookE SPRs

2011-05-03 Thread Alexander Graf

On 30.04.2011, at 00:10, Scott Wood wrote:

> Read them via KVM_GET_SREGS in kvm_arch_get_registers(),
> and display them in "info registers".
> 
> Also get CR and PID from the existing KVM_GET_REGS.
> 
> Signed-off-by: Scott Wood 

cc1: warnings being treated as errors
/home/agraf/release/qemu/target-ppc/kvm.c:48: error: ‘cap_booke_sregs’ defined 
but not used

I'll wrap the cap with an #ifdef.


Alex




Re: [Qemu-devel] [PATCH 1/2] NBD: Avoid leaking a couple of strings when the NBD device is closed

2011-05-03 Thread Stefan Hajnoczi
On Thu, Apr 28, 2011 at 4:20 PM,   wrote:
> From: Nick Thomas 
>
> Signed-off-by: Nick Thomas 
> ---
>  block/nbd.c |    4 
>  1 files changed, 4 insertions(+), 0 deletions(-)

Reviewed-by: Stefan Hajnoczi 



[Qemu-devel] [PATCH] Fix a number of unused-but-set-variable warnings (new with gcc-4.6)

2011-05-03 Thread Hans de Goede
---
 target-i386/kvm.c |4 ++--
 tcg/tcg.c |4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a13599d..e9e8d54 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -950,7 +950,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);
@@ -966,7 +966,7 @@ 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);
+/* 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 8748c05..11a8daf 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name)
 void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
int sizemask, TCGArg ret, int nargs, TCGArg *args)
 {
-#ifdef TCG_TARGET_I386
+#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 
 int call_type;
 #endif
 int i;
@@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned 
int flags,
 
 *gen_opc_ptr++ = INDEX_op_call;
 nparam = gen_opparam_ptr++;
-#ifdef TCG_TARGET_I386
+#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 
 call_type = (flags & TCG_CALL_TYPE_MASK);
 #endif
 if (ret != TCG_CALL_DUMMY_ARG) {
-- 
1.7.5




Re: [Qemu-devel] [PATCH v2] monitor: add PPC BookE SPRs

2011-05-03 Thread Alexander Graf

On 30.04.2011, at 00:10, Scott Wood wrote:

> Read them via KVM_GET_SREGS in kvm_arch_get_registers(),
> and display them in "info registers".
> 
> Also get CR and PID from the existing KVM_GET_REGS.
> 
> Signed-off-by: Scott Wood 
> ---
> v2: fix a couple style oversights, and cache kvm caps as requested

In fact, I'll just squash this patch with yours, if that's ok for you:


diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index a4ec4bc..5a1b6cb 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -45,7 +45,9 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 static int cap_interrupt_unset = false;
 static int cap_interrupt_level = false;
 static int cap_segstate;
+#ifdef KVM_CAP_PPC_BOOKE_SREGS
 static int cap_booke_sregs;
+#endif
 
 /* XXX We have a race condition where we actually have a level triggered
  * interrupt, but the infrastructure can't expose that yet, so the guest
@@ -94,13 +96,9 @@ static int kvm_arch_sync_sregs(CPUState *cenv)
 if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
 return 0;
 } else {
-#ifdef KVM_CAP_PPC_SEGSTATE
 if (!cap_segstate) {
 return 0;
 }
-#else
-return 0;
-#endif
 }
 
 ret = kvm_vcpu_ioctl(cenv, KVM_GET_SREGS, &sregs);




[Qemu-devel] [PATCH 2/6] atapi: Move comment to proper place

2011-05-03 Thread Kevin Wolf
From: Amit Shah 

Move misplaced comment for media_is_dvd()

Signed-off-by: Amit Shah 
Signed-off-by: Kevin Wolf 
---
 hw/ide/atapi.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 690a0ab..86b18d8 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -71,12 +71,12 @@ static void lba_to_msf(uint8_t *buf, int lba)
 buf[2] = lba % 75;
 }
 
-/* XXX: DVDs that could fit on a CD will be reported as a CD */
 static inline int media_present(IDEState *s)
 {
 return (s->nb_sectors > 0);
 }
 
+/* XXX: DVDs that could fit on a CD will be reported as a CD */
 static inline int media_is_dvd(IDEState *s)
 {
 return (media_present(s) && s->nb_sectors > CD_MAX_SECTORS);
-- 
1.7.2.3




[Qemu-devel] [PATCH 1/6] qemu-img resize: Fix option parsing

2011-05-03 Thread Kevin Wolf
For shrinking images, you're supposed to use a negative size. However, the
leading minus makes getopt think that it's an option and so you get the help
text if you don't use -- like in 'qemu-img resize test.img -- -1G'.

This patch handles the size first and removes it from the argument list so that
getopt won't even try to interpret it and you don't need -- any more.

Signed-off-by: Kevin Wolf 
---
 qemu-img.c |   13 +++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ed5ba91..e825123 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1442,6 +1442,16 @@ static int img_resize(int argc, char **argv)
 { NULL }
 };
 
+/* Remove size from argv manually so that negative numbers are not treated
+ * as options by getopt. */
+if (argc < 3) {
+help();
+return 1;
+}
+
+size = argv[--argc];
+
+/* Parse getopt arguments */
 fmt = NULL;
 for(;;) {
 c = getopt(argc, argv, "f:h");
@@ -1458,11 +1468,10 @@ static int img_resize(int argc, char **argv)
 break;
 }
 }
-if (optind + 1 >= argc) {
+if (optind >= argc) {
 help();
 }
 filename = argv[optind++];
-size = argv[optind++];
 
 /* Choose grow, shrink, or absolute resize mode */
 switch (size[0]) {
-- 
1.7.2.3




[Qemu-devel] [PULL 0/6] Block patches

2011-05-03 Thread Kevin Wolf
The following changes since commit 57aa265d462a64a06268be26d49020729cff56c1:

  lm32: add Milkymist Minimac2 support (2011-05-03 10:48:40 +0200)

are available in the git repository at:
  git://repo.or.cz/qemu/kevin.git for-anthony

Alon Levy (1):
  ide/atapi: fix set but unused

Amit Shah (2):
  atapi: Move comment to proper place
  atapi: Explain why we need a 'media not present' state

Jes Sorensen (1):
  qemu-progress.c: printf isn't signal safe

Kevin Wolf (1):
  qemu-img resize: Fix option parsing

Nick Thomas (1):
  NBD: Avoid leaking a couple of strings when the NBD device is closed

 block/nbd.c |4 
 hw/ide/atapi.c  |   14 +-
 qemu-img.c  |   13 +++--
 qemu-progress.c |7 ++-
 4 files changed, 30 insertions(+), 8 deletions(-)



[Qemu-devel] [PATCH 5/6] qemu-progress.c: printf isn't signal safe

2011-05-03 Thread Kevin Wolf
From: Jes Sorensen 

Change the signal handling to indicate a signal is pending, rather
then printing directly from the signal handler.

In addition make the signal prints go to stderr, rather than stdout.

Signed-off-by: Jes Sorensen 
Signed-off-by: Kevin Wolf 
---
 qemu-progress.c |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/qemu-progress.c b/qemu-progress.c
index e1feb89..a4894c0 100644
--- a/qemu-progress.c
+++ b/qemu-progress.c
@@ -37,6 +37,7 @@ struct progress_state {
 };
 
 static struct progress_state state;
+static volatile sig_atomic_t print_pending;
 
 /*
  * Simple progress print function.
@@ -63,12 +64,16 @@ static void progress_simple_init(void)
 #ifdef CONFIG_POSIX
 static void sigusr_print(int signal)
 {
-printf("(%3.2f/100%%)\n", state.current);
+print_pending = 1;
 }
 #endif
 
 static void progress_dummy_print(void)
 {
+if (print_pending) {
+fprintf(stderr, "(%3.2f/100%%)\n", state.current);
+print_pending = 0;
+}
 }
 
 static void progress_dummy_end(void)
-- 
1.7.2.3




[Qemu-devel] [PATCH 4/6] ide/atapi: fix set but unused

2011-05-03 Thread Kevin Wolf
From: Alon Levy 

Signed-off-by: Alon Levy 
Acked-by: Stefan Weil 
Signed-off-by: Kevin Wolf 
---
 hw/ide/atapi.c |4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 58febc0..fe2fb0b 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1080,17 +1080,15 @@ static const struct {
 
 void ide_atapi_cmd(IDEState *s)
 {
-const uint8_t *packet;
 uint8_t *buf;
 
-packet = s->io_buffer;
 buf = s->io_buffer;
 #ifdef DEBUG_IDE_ATAPI
 {
 int i;
 printf("ATAPI limit=0x%x packet:", s->lcyl | (s->hcyl << 8));
 for(i = 0; i < ATAPI_PACKET_SIZE; i++) {
-printf(" %02x", packet[i]);
+printf(" %02x", buf[i]);
 }
 printf("\n");
 }
-- 
1.7.2.3




[Qemu-devel] [PATCH 3/6] atapi: Explain why we need a 'media not present' state

2011-05-03 Thread Kevin Wolf
From: Amit Shah 

After the re-org of the atapi code, it might not be intuitive for a
reader of the code to understand why we're inserting a 'media not
present' state between cd changes.

Signed-off-by: Amit Shah 
Signed-off-by: Kevin Wolf 
---
 hw/ide/atapi.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 86b18d8..58febc0 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1106,7 +1106,13 @@ void ide_atapi_cmd(IDEState *s)
 ide_atapi_cmd_check_status(s);
 return;
 }
-
+/*
+ * When a CD gets changed, we have to report an ejected state and
+ * then a loaded state to guests so that they detect tray
+ * open/close and media change events.  Guests that do not use
+ * GET_EVENT_STATUS_NOTIFICATION to detect such tray open/close
+ * states rely on this behavior.
+ */
 if (bdrv_is_inserted(s->bs) && s->cdrom_changed) {
 ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
 
-- 
1.7.2.3




[Qemu-devel] [PATCH 6/6] NBD: Avoid leaking a couple of strings when the NBD device is closed

2011-05-03 Thread Kevin Wolf
From: Nick Thomas 

Signed-off-by: Nick Thomas 
Signed-off-by: Kevin Wolf 
---
 block/nbd.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 1d6b225..7a52f62 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -239,6 +239,10 @@ static int nbd_write(BlockDriverState *bs, int64_t 
sector_num,
 
 static void nbd_close(BlockDriverState *bs)
 {
+BDRVNBDState *s = bs->opaque;
+qemu_free(s->export_name);
+qemu_free(s->host_spec);
+
 nbd_teardown_connection(bs);
 }
 
-- 
1.7.2.3




Re: [Qemu-devel] [PATCH] qemu-img resize: Fix option parsing

2011-05-03 Thread Stefan Hajnoczi
On Fri, Apr 29, 2011 at 10:06 AM, Kevin Wolf  wrote:
> For shrinking images, you're supposed to use a negative size. However, the
> leading minus makes getopt think that it's an option and so you get the help
> text if you don't use -- like in 'qemu-img resize test.img -- -1G'.
>
> This patch handles the size first and removes it from the argument list so 
> that
> getopt won't even try to interpret it and you don't need -- any more.
>
> Signed-off-by: Kevin Wolf 
> ---
>  qemu-img.c |   13 +++--
>  1 files changed, 11 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi 



Re: [Qemu-devel] BUG: 0.14.0 -device usb-host supports only one device

2011-05-03 Thread Gerd Hoffmann

  Hi,


When enabling the -device usb-host option support for adding
automatically USB devices from the host to the guest, only one device
gets detected.


Yes.  -device usb-host creates a *single* virtual usb device instance. 
When a matching device on the host is found the virtual device is 
plugged in, otherwise it is plugged out.  In plugged out state qemu 
looks now and then (every two seconds IIRC) whenever a matching device 
was plugged into the host.


Try -device usb-host twice if you need two virtual usb devices.

cheers,
  Gerd



Re: [Qemu-devel] [PATCH 1/2] Support for MIPS64 user mode emulation

2011-05-03 Thread Khansa Butt
I have made following changes
 addr = env->lladdr;
addr &= qemu_host_page_mask;
 page_addr = addr & TARGET_PAGE_MASK;
 start_exclusive();
 mmap_lock();
 flags = page_get_flags(page_addr);
now return to elfload.c
I have a simple hello world mips64 binary for which I have two loadable
segments
so following rounded off address ranges passed to page_set_flags() in
target_mmap()
1) 0x2000 - 0x2008d000
2) 0x2009c000 - 0x200a6000
the last addresses of these ranges are not included in l1_map
because of the for loop condition in page_set_flags()

> for (addr = start, len = end - start;
>
len != 0;
len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
while env->lladdr after rounding off belong to 0x200a6000 so in order to
include last address of above range i made following change
error = target_mmap(vaddr_ps, eppnt->p_memsz + v addr_po,
elf_prot, MAP_PRIVATE | MAP_FIXED,
image_fd, eppnt->p_offset - vaddr_po);
as mem size of a segment is greater than its file size but you told me that
it will cause SIGBUS
please suggest some solution for me in order to avoid target_mmap()
change(i.e. filesz to memsz)
or can I change condition of for loop some how so that one more iteration
will run for the last address.


On Fri, Apr 29, 2011 at 2:01 PM, Aurelien Jarno wrote:

> On Mon, Apr 25, 2011 at 04:54:19PM +0500, Khansa Butt wrote:
> > please see inline comments highlighted in red color.
> >
> > On Wed, Apr 13, 2011 at 2:32 AM, Aurelien Jarno  >wrote:
> >
> > > [I don't know very well linux-user, it would be nice to Cc: Riku
> Voipio,
> > >  the linux-user maintainer for the next version.]
> > >
> > > On Sat, Apr 09, 2011 at 04:02:31PM +0500, Khansa Butt wrote:
> > > > From e96e20e50cada1c9e1b65de5925281cdd5659746 Mon Sep 17 00:00:00
> 2001
> > > > From: Ehsan-ul-Haq & Khansa Butt 
> > > > Date: Sat, 9 Apr 2011 10:51:22 +0500
> > > > Subject: [PATCH 1/2] Support for MIPS64 user mode emulation
> > > >
> > > >
> > > > Signed-off-by: Ehsan-ul-Haq, Abdul Qadeer, Abdul Waheed, Khansa Butt
> <
> > > > kha...@kics.edu.pk>
> > > > ---
> > > >  configure |1 +
> > > >  default-configs/mips64-linux-user.mak |1 +
> > > >  linux-user/elfload.c  |2 +-
> > > >  linux-user/main.c |   29
> > > +++--
> > > >  linux-user/mips64/syscall.h   |3 +++
> > > >  linux-user/signal.c   |3 ++-
> > > >  target-mips/translate.c   |1 +
> > > >  7 files changed, 36 insertions(+), 4 deletions(-)
> > > >  create mode 100644 default-configs/mips64-linux-user.mak
> > > >
> > > > diff --git a/configure b/configure
> > > > index ae97e11..d1f7867 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -1039,6 +1039,7 @@ m68k-linux-user \
> > > >  microblaze-linux-user \
> > > >  microblazeel-linux-user \
> > > >  mips-linux-user \
> > > > +mips64-linux-user \
> > > >  mipsel-linux-user \
> > > >  ppc-linux-user \
> > > >  ppc64-linux-user \
> > > > diff --git a/default-configs/mips64-linux-user.mak
> > > > b/default-configs/mips64-linux-user.mak
> > > > new file mode 100644
> > > > index 000..1598bfc
> > > > --- /dev/null
> > > > +++ b/default-configs/mips64-linux-user.mak
> > > > @@ -0,0 +1 @@
> > > > +# Default configuration for mips64-linux-user
> > > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > > > index fe5410e..2832a33 100644
> > > > --- a/linux-user/elfload.c
> > > > +++ b/linux-user/elfload.c
> > > > @@ -1384,7 +1384,7 @@ static void load_elf_image(const char
> *image_name,
> > > int
> > > > image_fd,
> > > >  vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
> > > >  vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> > > >
> > > > -error = target_mmap(vaddr_ps, eppnt->p_filesz +
> vaddr_po,
> > > > +error = target_mmap(vaddr_ps, eppnt->p_memsz + vaddr_po,
> > >
> > > What is the goal of this change? If the mmapped aread is bigger than
> the
> > > file size rounded up to te page size, it will cause a SIGBUS.
> > >
> > > >  elf_prot, MAP_PRIVATE | MAP_FIXED,
> > > >  image_fd, eppnt->p_offset -
> vaddr_po);
> > > >  if (error == -1) {
> > > > diff --git a/linux-user/main.c b/linux-user/main.c
> > > > index e651bfd..a7f4955 100644
> > > > --- a/linux-user/main.c
> > > > +++ b/linux-user/main.c
> > > > @@ -1937,6 +1937,14 @@ static int do_store_exclusive(CPUMIPSState
> *env)
> > > >  int d;
> > > >
> > > >  addr = env->lladdr;
> > > > +#if defined(TARGET_MIPS64)
> > > > +/* For MIPS64 on 32 bit host there is a need to make
> > > > +* the page accessible to which the above 'addr' is belonged */
> > > > +#if HOST_LONG_BITS == 32
> > > > +int flag = PAGE_VALID | PAGE_READ | PAGE_WRITE | PAGE_WRITE_ORG;
> > > > +page_set_flags(addr, addr + 4096, flag);
> > > > +#endif
> > > > +#endif
> > >
> >

Re: [Qemu-devel] [PATCH v2 1/1] Add QMP bits for blockdev-snapshot-sync.

2011-05-03 Thread Jes Sorensen
On 04/29/11 15:45, Anthony Liguori wrote:
> On 04/29/2011 08:38 AM, Jes Sorensen wrote:
>> It is exactly the same for the management tool:
>> - Creation of the new image either succeeds or fails
>> - Switchover either succeeds or fails
> 
> Creating an image can be treated as an atomic operation.  It either
> fully succeeds or you assume it failed and throw the image away since
> you can always try again.
> 
> When you combine creating an image with another operation, you create a
> a single operation that cannot be treated as atomic which makes recovery
> from failure much more difficult.

As explained in previous emails, you still have the option to do this
with the current implementation, if you so desire. It's a bad idea, but
for those who insist on doing the wrong thing, sure go ahead.

Just create the image first, and then pass it in as the snapshot file
rather than specifying a snapshot file that doesn't exist already.

>>> 3) Begin switch over to new image
>>> 4) Switch over image.
>>> 5) Receive notification when it's complete
>>
>> Sorry but this isn't an accurate description of the process. Once you
>> have a new image ready, you need to halt writes, flush buffers, and then
>> you can do the switch, which has to be atomic.
> 
> You need to queue writes, you can still let the guest run while the
> remaining writes are sent to disk.
> 
> But if this is a background task, then as a management tool, don't I
> want a notification when it's been completed?
> 
> Don't I want to have a little spinning box in my GUI or something like
> that while this is happening?

I agree that having an interface that can run it in the background
asynchronously isn't a bad thing, and it would be a nice feature.
However it really doesn't qualify as anything but an enhancement in my
book. You already have the option to pre-create the snapshot image if
you so desire.

>>> But combining 1-5 in a single interface creates a command that while
>>> convenient on the command line, is not usable for a robust management
>>> tool.
>>
>> As I explained you can already use an externally created image with the
>> current interface, the only issue that may be worth doing async is the
>> buffer flushing. However once you do that, guest writes are going to
>> stall anyway, and eventually the guest applications will all stall.
> 
> The interface shouldn't have the option to create an image... because if
> you have that, the interface is difficult to use.
> 
> Why present an option to do something that we know is broken?  We can't
> blame management tools for not doing a good job managing KVM if we're
> giving them poor interfaces to work with.

Sorry this is utterly bogus. This is *not* broken, it is the right way
to do things. It is clearly a matter of taste whether you prefer it one
way or another, but it is not broken.

There is nothing wrong with it being an option, especially since you
don't have to do anything magic for creating an image in advance. I
realize it doesn't match your image of how you would like the command to
work, but that is not the same as it being broken.

I really see nothing in your above arguments that backs up the argument
that the current implementation is broken in any way. I am totally in
favor or having an async implementation as well, but I really don't see
it being as big an issue as you are making it.

Regards,
Jes




[Qemu-devel] [PATCH] kvm: ppc: detect old headers

2011-05-03 Thread Alexander Graf
When compiling Qemu with older kernel headers, the PVR setting
mechanism isn't available yet. Unfortunately, back then I didn't add
a capability we could check against, so all we can do is add a configure
test to see if we support PVR setting. For BookE, we don't care yet.

This fixes compilation errors with KVM enabled on older kernel headers
(like 2.6.32).

Signed-off-by: Alexander Graf 
---
 configure|   18 ++
 target-ppc/kvm.c |   16 +++-
 2 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index fbf5d5f..adfbb40 100755
--- a/configure
+++ b/configure
@@ -1772,6 +1772,21 @@ recent kvm-kmod from 
http://sourceforge.net/projects/kvm.";
 fi
 
 ##
+# test for ppc kvm pvr setting
+
+if test "$kvm" = "yes" && test "$cpu" = "ppc" -o "$cpu" = "ppc64"; then
+cat > $TMPC <
+int main(void) { struct kvm_sregs s; s.pvr = 0; return 0; }
+EOF
+if compile_prog "$kvm_cflags" "" ; then
+kvm_ppc_pvr=yes
+else
+kvm_ppc_pvr=no
+fi
+fi
+
+##
 # test for vhost net
 
 if test "$vhost_net" != "no"; then
@@ -3257,6 +3272,9 @@ case "$target_arch2" in
   if test $vhost_net = "yes" ; then
 echo "CONFIG_VHOST_NET=y" >> $config_target_mak
   fi
+  if test $kvm_ppc_pvr = "yes" ; then
+echo "CONFIG_KVM_PPC_PVR=y" >> $config_target_mak
+  fi
 fi
 esac
 if test "$target_bigendian" = "yes" ; then
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 5a1b6cb..ccf4668 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -94,19 +94,33 @@ static int kvm_arch_sync_sregs(CPUState *cenv)
 int ret;
 
 if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
+/* What we're really trying to say is "if we're on BookE, we use
+   the native PVR for now". This is the only sane way to check
+   it though, so we potentially confuse users that they can run
+   BookE guests on BookS. Let's hope nobody dares enough :) */
 return 0;
 } else {
 if (!cap_segstate) {
-return 0;
+fprintf(stderr, "kvm error: missing PVR setting capability\n");
+return -ENOSYS;
 }
 }
 
+#if !defined(CONFIG_KVM_PPC_PVR)
+if (1) {
+fprintf(stderr, "kvm error: missing PVR setting capability\n");
+return -ENOSYS;
+}
+#endif
+
 ret = kvm_vcpu_ioctl(cenv, KVM_GET_SREGS, &sregs);
 if (ret) {
 return ret;
 }
 
+#ifdef CONFIG_KVM_PPC_PVR
 sregs.pvr = cenv->spr[SPR_PVR];
+#endif
 return kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, &sregs);
 }
 
-- 
1.6.0.2




[Qemu-devel] [PATCH] kvm: ppc: warn user on PAGE_SIZE mismatch

2011-05-03 Thread Alexander Graf
On PPC, the default PAGE_SIZE is 64kb. Unfortunately, the hardware
alignments don't match here: There are RAM and MMIO regions within
a single page when it's 64kb in size.

So the only way out for now is to tell the user that he should use 4k
PAGE_SIZE.

This patch gives the user a hint on that, telling him that failing to
register a prefix slot is most likely to be caused by mismatching PAGE_SIZE.

This way it's also more future-proof, as bigger PAGE_SIZE can easily be
supported by other machines then, as long as they stick to 64kb granularities.

Signed-off-by: Alexander Graf 
---
 kvm-all.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index d92c20e..a30865e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -590,6 +590,11 @@ static void kvm_set_phys_mem(target_phys_addr_t 
start_addr, ram_addr_t size,
 if (err) {
 fprintf(stderr, "%s: error registering prefix slot: %s\n",
 __func__, strerror(-err));
+#ifdef TARGET_PPC
+fprintf(stderr, "%s: This is probably because your kernel's " \
+"PAGE_SIZE is too big. Please try to use 4k " \
+"PAGE_SIZE!\n", __func__);
+#endif
 abort();
 }
 }
-- 
1.6.0.2




[Qemu-devel] [PATCH 0/3] ide: add TRIM support

2011-05-03 Thread Christoph Hellwig
This patchset resurrects my older patches to add TRIM support.  The
old approach to pass an action handler through the IDE subsystem doesn't
work any more after the refactoring for the AHCI driver, so not we simple
switch on the type of command passed in.



[Qemu-devel] [PATCH 1/3] make dma_bdrv_io available to drivers

2011-05-03 Thread Christoph Hellwig
Make dma_bdrv_io available for drivers, and pass an explicit I/O function
instead of hardcoding bdrv_aio_readv/bdrv_aio_writev.  This is required
to implement non-READ/WRITE dma commands in the ide driver, e.g. the
upcoming TRIM support.

Signed-off-by: Christoph Hellwig 

Index: qemu/dma-helpers.c
===
--- qemu.orig/dma-helpers.c 2010-12-16 17:40:25.422004054 +0100
+++ qemu/dma-helpers.c  2011-05-03 11:12:36.340387288 +0200
@@ -47,6 +47,7 @@ typedef struct {
 target_phys_addr_t sg_cur_byte;
 QEMUIOVector iov;
 QEMUBH *bh;
+DMAIOFunc *io_func;
 } DMAAIOCB;
 
 static void dma_bdrv_cb(void *opaque, int ret);
@@ -116,13 +117,8 @@ static void dma_bdrv_cb(void *opaque, in
 return;
 }
 
-if (dbs->is_write) {
-dbs->acb = bdrv_aio_writev(dbs->bs, dbs->sector_num, &dbs->iov,
-   dbs->iov.size / 512, dma_bdrv_cb, dbs);
-} else {
-dbs->acb = bdrv_aio_readv(dbs->bs, dbs->sector_num, &dbs->iov,
-  dbs->iov.size / 512, dma_bdrv_cb, dbs);
-}
+dbs->acb = dbs->io_func(dbs->bs, dbs->sector_num, &dbs->iov,
+dbs->iov.size / 512, dma_bdrv_cb, dbs);
 if (!dbs->acb) {
 dma_bdrv_unmap(dbs);
 qemu_iovec_destroy(&dbs->iov);
@@ -144,12 +140,12 @@ static AIOPool dma_aio_pool = {
 .cancel = dma_aio_cancel,
 };
 
-static BlockDriverAIOCB *dma_bdrv_io(
+BlockDriverAIOCB *dma_bdrv_io(
 BlockDriverState *bs, QEMUSGList *sg, uint64_t sector_num,
-BlockDriverCompletionFunc *cb, void *opaque,
-int is_write)
+DMAIOFunc *io_func, BlockDriverCompletionFunc *cb,
+void *opaque, int is_write)
 {
-DMAAIOCB *dbs =  qemu_aio_get(&dma_aio_pool, bs, cb, opaque);
+DMAAIOCB *dbs = qemu_aio_get(&dma_aio_pool, bs, cb, opaque);
 
 dbs->acb = NULL;
 dbs->bs = bs;
@@ -158,6 +154,7 @@ static BlockDriverAIOCB *dma_bdrv_io(
 dbs->sg_cur_index = 0;
 dbs->sg_cur_byte = 0;
 dbs->is_write = is_write;
+dbs->io_func = io_func;
 dbs->bh = NULL;
 qemu_iovec_init(&dbs->iov, sg->nsg);
 dma_bdrv_cb(dbs, 0);
@@ -173,12 +170,12 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDri
 QEMUSGList *sg, uint64_t sector,
 void (*cb)(void *opaque, int ret), void 
*opaque)
 {
-return dma_bdrv_io(bs, sg, sector, cb, opaque, 0);
+return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, 0);
 }
 
 BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs,
  QEMUSGList *sg, uint64_t sector,
  void (*cb)(void *opaque, int ret), void 
*opaque)
 {
-return dma_bdrv_io(bs, sg, sector, cb, opaque, 1);
+return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, 1);
 }
Index: qemu/dma.h
===
--- qemu.orig/dma.h 2010-12-16 17:40:25.431007267 +0100
+++ qemu/dma.h  2011-05-03 11:12:36.343720604 +0200
@@ -32,6 +32,14 @@ void qemu_sglist_add(QEMUSGList *qsg, ta
  target_phys_addr_t len);
 void qemu_sglist_destroy(QEMUSGList *qsg);
 
+typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs, int64_t sector_num,
+ QEMUIOVector *iov, int nb_sectors,
+ BlockDriverCompletionFunc *cb, void *opaque);
+
+BlockDriverAIOCB *dma_bdrv_io(BlockDriverState *bs,
+  QEMUSGList *sg, uint64_t sector_num,
+  DMAIOFunc *io_func, BlockDriverCompletionFunc 
*cb,
+  void *opaque, int is_write);
 BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
 QEMUSGList *sg, uint64_t sector,
 BlockDriverCompletionFunc *cb, void *opaque);



[Qemu-devel] [PATCH 2/3] ide: allow other dma comands than read and write

2011-05-03 Thread Christoph Hellwig
Replace the is_read flag with a dma_cmd flag to allow the dma and
restart logic to handler other commands like TRIM.

Signed-off-by: Christoph Hellwig 

Index: qemu/hw/ide/core.c
===
--- qemu.orig/hw/ide/core.c 2011-05-03 11:32:14.727336751 +0200
+++ qemu/hw/ide/core.c  2011-05-03 11:35:56.259469943 +0200
@@ -473,7 +473,7 @@ handle_rw_error:
 if (ret < 0) {
 int op = BM_STATUS_DMA_RETRY;
 
-if (s->is_read)
+if (s->dma_cmd == IDE_DMA_READ)
 op |= BM_STATUS_RETRY_READ;
 if (ide_handle_rw_error(s, -ret, op)) {
 return;
@@ -483,7 +483,7 @@ handle_rw_error:
 n = s->io_buffer_size >> 9;
 sector_num = ide_get_sector(s);
 if (n > 0) {
-dma_buf_commit(s, s->is_read);
+dma_buf_commit(s, ide_cmd_is_read(s));
 sector_num += n;
 ide_set_sector(s, sector_num);
 s->nsector -= n;
@@ -500,20 +500,23 @@ handle_rw_error:
 n = s->nsector;
 s->io_buffer_index = 0;
 s->io_buffer_size = n * 512;
-if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->is_read) == 0)
+if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_read(s)) == 0)
 goto eot;
 
 #ifdef DEBUG_AIO
-printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, is_read=%d\n",
-   sector_num, n, s->is_read);
+printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, cmd_cmd=%d\n",
+   sector_num, n, s->dma_cmd);
 #endif
 
-if (s->is_read) {
+switch (s->dma_cmd) {
+case IDE_DMA_READ:
 s->bus->dma->aiocb = dma_bdrv_read(s->bs, &s->sg, sector_num,
ide_dma_cb, s);
-} else {
+break;
+case IDE_DMA_WRITE:
 s->bus->dma->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
 ide_dma_cb, s);
+break;
 }
 
 if (!s->bus->dma->aiocb) {
@@ -527,12 +530,12 @@ eot:
ide_set_inactive(s);
 }
 
-static void ide_sector_start_dma(IDEState *s, int is_read)
+static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
 s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
 s->io_buffer_index = 0;
 s->io_buffer_size = 0;
-s->is_read = is_read;
+s->dma_cmd = dma_cmd;
 s->bus->dma->ops->start_dma(s->bus->dma, s, ide_dma_cb);
 }
 
@@ -915,7 +918,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t
 if (!s->bs)
 goto abort_cmd;
ide_cmd_lba48_transform(s, lba48);
-ide_sector_start_dma(s, 1);
+ide_sector_start_dma(s, IDE_DMA_READ);
 break;
case WIN_WRITEDMA_EXT:
lba48 = 1;
@@ -924,7 +927,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t
 if (!s->bs)
 goto abort_cmd;
ide_cmd_lba48_transform(s, lba48);
-ide_sector_start_dma(s, 0);
+ide_sector_start_dma(s, IDE_DMA_WRITE);
 s->media_changed = 1;
 break;
 case WIN_READ_NATIVE_MAX_EXT:
Index: qemu/hw/ide/internal.h
===
--- qemu.orig/hw/ide/internal.h 2011-05-03 11:32:13.884007986 +0200
+++ qemu/hw/ide/internal.h  2011-05-03 11:36:05.526086407 +0200
@@ -379,6 +379,14 @@ struct unreported_events {
 bool new_media;
 };
 
+enum ide_dma_cmd {
+IDE_DMA_READ,
+IDE_DMA_WRITE,
+};
+
+#define ide_cmd_is_read(s) \
+   ((s)->dma_cmd == IDE_DMA_READ)
+
 /* NOTE: IDEState represents in fact one drive */
 struct IDEState {
 IDEBus *bus;
@@ -446,7 +454,7 @@ struct IDEState {
 uint32_t mdata_size;
 uint8_t *mdata_storage;
 int media_changed;
-int is_read;
+enum ide_dma_cmd dma_cmd;
 /* SMART */
 uint8_t smart_enabled;
 uint8_t smart_autosave;
Index: qemu/hw/ide/macio.c
===
--- qemu.orig/hw/ide/macio.c2011-05-03 11:32:14.740670013 +0200
+++ qemu/hw/ide/macio.c 2011-05-03 11:35:56.302803041 +0200
@@ -145,12 +145,17 @@ static void pmac_ide_transfer_cb(void *o
 io->addr += io->len;
 io->len = 0;
 
-if (s->is_read)
+switch (s->dma_cmd) {
+case IDE_DMA_READ:
 m->aiocb = dma_bdrv_read(s->bs, &s->sg, sector_num,
 pmac_ide_transfer_cb, io);
-else
+break;
+case IDE_DMA_WRITE:
 m->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
  pmac_ide_transfer_cb, io);
+break;
+}
+
 if (!m->aiocb)
 pmac_ide_transfer_cb(io, -1);
 }
Index: qemu/hw/ide/pci.c
===
--- qemu.orig/hw/ide/pci.c  2011-05-03 11:32:13.854008149 +0200
+++ qemu/hw/ide/pci.c   2011-05-03 11:32:27.877265512 +0200
@@ -169,7 +169,7 @@ static int bmdma_set_inactive(IDEDMA *dm
 return 0;
 }
 
-static void bmdma_restart_dma(BMDMAState *bm, int is_read)
+static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd)
 {

[Qemu-devel] [PATCH 3/3] ide: add TRIM support

2011-05-03 Thread Christoph Hellwig
Add support for TRIM sub function of the data set management command,
and wire it up to the qemu discard infrastructure.

Signed-off-by: Christoph Hellwig 

Index: qemu/hw/ide/core.c
===
--- qemu.orig/hw/ide/core.c 2011-05-03 11:35:56.259469943 +0200
+++ qemu/hw/ide/core.c  2011-05-03 11:39:38.458266188 +0200
@@ -124,6 +124,9 @@ static void ide_identify(IDEState *s)
 put_le16(p + 66, 120);
 put_le16(p + 67, 120);
 put_le16(p + 68, 120);
+if (dev && dev->conf.discard_granularity) {
+put_le16(p + 69, (1 << 14)); /* determinate TRIM behavior */
+}
 
 if (s->ncq_queues) {
 put_le16(p + 75, s->ncq_queues - 1);
@@ -157,6 +160,9 @@ static void ide_identify(IDEState *s)
 dev = s->unit ? s->bus->slave : s->bus->master;
 if (dev && dev->conf.physical_block_size)
 put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf));
+if (dev && dev->conf.discard_granularity) {
+put_le16(p + 169, 1); /* TRIM support */
+}
 
 memcpy(s->identify_data, p, sizeof(s->identify_data));
 s->identify_set = 1;
@@ -299,6 +305,72 @@ static void ide_set_signature(IDEState *
 }
 }
 
+typedef struct TrimAIOCB {
+BlockDriverAIOCB common;
+QEMUBH *bh;
+int ret;
+} TrimAIOCB;
+
+static void trim_aio_cancel(BlockDriverAIOCB *acb)
+{
+TrimAIOCB *iocb = container_of(acb, TrimAIOCB, common);
+
+qemu_bh_delete(iocb->bh);
+iocb->bh = NULL;
+qemu_aio_release(iocb);
+}
+
+static AIOPool trim_aio_pool = {
+.aiocb_size = sizeof(TrimAIOCB),
+.cancel = trim_aio_cancel,
+};
+
+static void ide_trim_bh_cb(void *opaque)
+{
+TrimAIOCB *iocb = opaque;
+
+iocb->common.cb(iocb->common.opaque, iocb->ret);
+
+qemu_bh_delete(iocb->bh);
+iocb->bh = NULL;
+
+qemu_aio_release(iocb);
+}
+
+BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
+int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+TrimAIOCB *iocb;
+int i, j, ret;
+
+iocb = qemu_aio_get(&trim_aio_pool, bs, cb, opaque);
+iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
+iocb->ret = 0;
+
+for (j = 0; j < qiov->niov; j++) {
+uint64_t *buffer = qiov->iov[j].iov_base;
+
+for (i = 0; i < qiov->iov[j].iov_len / 8; i++) {
+/* 6-byte LBA + 2-byte range per entry */
+uint64_t entry = le64_to_cpu(buffer[i]);
+uint64_t sector = entry & 0xULL;
+uint16_t count = entry >> 48;
+
+if (count == 0)
+break;
+
+ret = bdrv_discard(bs, sector * 512, count * 512);
+if (!iocb->ret)
+iocb->ret = ret;
+}
+}
+
+qemu_bh_schedule(iocb->bh);
+
+return &iocb->common;
+}
+
 static inline void ide_abort_command(IDEState *s)
 {
 s->status = READY_STAT | ERR_STAT;
@@ -517,6 +589,10 @@ handle_rw_error:
 s->bus->dma->aiocb = dma_bdrv_write(s->bs, &s->sg, sector_num,
 ide_dma_cb, s);
 break;
+case IDE_DMA_TRIM:
+s->bus->dma->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
+ ide_issue_trim, ide_dma_cb, s, 1);
+break;
 }
 
 if (!s->bus->dma->aiocb) {
@@ -817,6 +893,17 @@ void ide_exec_cmd(IDEBus *bus, uint32_t
 return;
 
 switch(val) {
+case WIN_DSM:
+switch (s->feature) {
+case DSM_TRIM:
+if (!s->bs)
+goto abort_cmd;
+ide_sector_start_dma(s, IDE_DMA_TRIM);
+break;
+default:
+goto abort_cmd;
+}
+break;
 case WIN_IDENTIFY:
 if (s->bs && s->drive_kind != IDE_CD) {
 if (s->drive_kind != IDE_CFATA)
Index: qemu/hw/ide/internal.h
===
--- qemu.orig/hw/ide/internal.h 2011-05-03 11:36:05.526086407 +0200
+++ qemu/hw/ide/internal.h  2011-05-03 11:39:50.264868893 +0200
@@ -62,7 +62,11 @@ typedef struct IDEDMAOps IDEDMAOps;
  */
 #define CFA_REQ_EXT_ERROR_CODE 0x03 /* CFA Request Extended Error Code 
*/
 /*
- * 0x04->0x07 Reserved
+ *  0x04->0x05 Reserved
+ */
+#define WIN_DSM 0x06
+/*
+ *  0x07 Reserved
  */
 #define WIN_SRST   0x08 /* ATAPI soft reset command */
 #define WIN_DEVICE_RESET   0x08
@@ -190,6 +194,9 @@ typedef struct IDEDMAOps IDEDMAOps;
 
 #define IDE_DMA_BUF_SECTORS 256
 
+/* feature values for Data Set Management */
+#define DSM_TRIM0x01
+
 #if (IDE_DMA_BUF_SECTORS < MAX_MULT_SECTORS)
 #error "IDE_DMA_BUF_SECTORS must be bigger or equal to MAX_MULT_SECTORS"
 #endif
@@ -382,6 +389,7 @@ struct unreported_events {
 enum ide_dma_cmd {
 IDE_DMA_READ,
 IDE_DMA_WRITE,
+IDE_DMA_TRIM,
 };
 
 #define ide_cmd_is_read(s) \
@@ -5

Re: [Qemu-devel] [PATCH] scsi-generic: Remove bogus double complete

2011-05-03 Thread Paolo Bonzini

On 04/05/2011 04:00 PM, Kevin Wolf wrote:

I have a hard time each time I try to understand this SCSI stuff without
reading a lot of code and specs. What I would have expected is this:

if (len == 0) {
 scsi_command_complete(r, 0);
} else {
 r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
}

This would fix the problem that both functions are called, but still use
SCSI_REASON_DONE if no data is transferred. This is similar to what
scsi-disk seems to be doing. However, if you can explain to me why your
version is more correct, I'll gladly take it.


I agree with Kevin that his version seems more correct.  The purpose of 
the code is to trigger a phase mismatch for the lsi controller, and that 
would probably not happen with your version.  s->current->dma_len would 
never go down to 0 if it were really a short transfer:


s->current->dma_len -= count;
if (s->current->dma_len == 0) {
s->current->dma_buf = NULL;
if (out) {
/* Write the data.  */
dev->info->write_data(dev, s->current->tag);
} else {
/* Request any remaining data.  */
dev->info->read_data(dev, s->current->tag);
}
} else {
s->current->dma_buf += count;
lsi_resume_script(s);
}

so the SCRIPTS would deadlock, waiting for the full transfer to happen, 
and the SCSI_REASON_DONE callback would never be sent.


Even with spapr_vscsi, however, what would happen is that the 
SCSI_REASON_DATA does nothing except calling sdev->info->read_data (i.e. 
scsi_read_data) again.  This would then call scsi_command_complete and 
send the SCSI_REASON_DONE callback.  So for spapr_vscsi there should be 
no difference between the two cases.


I'll include Kevin's patch into my upcoming SCSI series.

Paolo



Re: [Qemu-devel] [PATCH] kvm: ppc: detect old headers

2011-05-03 Thread Jan Kiszka
On 2011-05-03 13:54, Alexander Graf wrote:
> When compiling Qemu with older kernel headers, the PVR setting
> mechanism isn't available yet. Unfortunately, back then I didn't add
> a capability we could check against, so all we can do is add a configure
> test to see if we support PVR setting. For BookE, we don't care yet.
> 
> This fixes compilation errors with KVM enabled on older kernel headers
> (like 2.6.32).

Why not finally import the latest kvm kernel headers into qemu? Would
save us a lot of current and future configure and #ifdef dances.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: ppc: detect old headers

2011-05-03 Thread Alexander Graf

On 03.05.2011, at 14:14, Jan Kiszka wrote:

> On 2011-05-03 13:54, Alexander Graf wrote:
>> When compiling Qemu with older kernel headers, the PVR setting
>> mechanism isn't available yet. Unfortunately, back then I didn't add
>> a capability we could check against, so all we can do is add a configure
>> test to see if we support PVR setting. For BookE, we don't care yet.
>> 
>> This fixes compilation errors with KVM enabled on older kernel headers
>> (like 2.6.32).
> 
> Why not finally import the latest kvm kernel headers into qemu? Would
> save us a lot of current and future configure and #ifdef dances.

Sure, sounds like a good topic for today's call?

Alex




Re: [Qemu-devel] [PATCH] kvm: ppc: detect old headers

2011-05-03 Thread Jan Kiszka
On 2011-05-03 14:28, Alexander Graf wrote:
> 
> On 03.05.2011, at 14:14, Jan Kiszka wrote:
> 
>> On 2011-05-03 13:54, Alexander Graf wrote:
>>> When compiling Qemu with older kernel headers, the PVR setting
>>> mechanism isn't available yet. Unfortunately, back then I didn't add
>>> a capability we could check against, so all we can do is add a configure
>>> test to see if we support PVR setting. For BookE, we don't care yet.
>>>
>>> This fixes compilation errors with KVM enabled on older kernel headers
>>> (like 2.6.32).
>>
>> Why not finally import the latest kvm kernel headers into qemu? Would
>> save us a lot of current and future configure and #ifdef dances.
> 
> Sure, sounds like a good topic for today's call?

Fine with me. Patch should be done by then as well.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: ppc: detect old headers

2011-05-03 Thread Alexander Graf

On 03.05.2011, at 14:31, Jan Kiszka wrote:

> On 2011-05-03 14:28, Alexander Graf wrote:
>> 
>> On 03.05.2011, at 14:14, Jan Kiszka wrote:
>> 
>>> On 2011-05-03 13:54, Alexander Graf wrote:
 When compiling Qemu with older kernel headers, the PVR setting
 mechanism isn't available yet. Unfortunately, back then I didn't add
 a capability we could check against, so all we can do is add a configure
 test to see if we support PVR setting. For BookE, we don't care yet.
 
 This fixes compilation errors with KVM enabled on older kernel headers
 (like 2.6.32).
>>> 
>>> Why not finally import the latest kvm kernel headers into qemu? Would
>>> save us a lot of current and future configure and #ifdef dances.
>> 
>> Sure, sounds like a good topic for today's call?
> 
> Fine with me. Patch should be done by then as well.

*shrug* I'm fairly indifferent on that topic. It would help users, so they can 
easier compile things, but requires us to keep the headers in sync. Do you have 
any good way of automating the process?


Alex




Re: [Qemu-devel] [PULL 0/6] Block patches

2011-05-03 Thread Anthony Liguori

On 05/03/2011 06:08 AM, Kevin Wolf wrote:

The following changes since commit 57aa265d462a64a06268be26d49020729cff56c1:

   lm32: add Milkymist Minimac2 support (2011-05-03 10:48:40 +0200)

are available in the git repository at:
   git://repo.or.cz/qemu/kevin.git for-anthony


Pulled.  Thanks.

Regards,

Anthony Liguori


Alon Levy (1):
   ide/atapi: fix set but unused

Amit Shah (2):
   atapi: Move comment to proper place
   atapi: Explain why we need a 'media not present' state

Jes Sorensen (1):
   qemu-progress.c: printf isn't signal safe

Kevin Wolf (1):
   qemu-img resize: Fix option parsing

Nick Thomas (1):
   NBD: Avoid leaking a couple of strings when the NBD device is closed

  block/nbd.c |4 
  hw/ide/atapi.c  |   14 +-
  qemu-img.c  |   13 +++--
  qemu-progress.c |7 ++-
  4 files changed, 30 insertions(+), 8 deletions(-)





Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

2011-05-03 Thread Jes Sorensen
On 04/21/11 15:55, Michael Roth wrote:
>> Did you do anything with the fsfreeze patches, or were they dropped in
>> the migration to qapi?
> 
> They were pending some changes required on the agent side that weren't
> really addressed/doable until this patchset, namely:
> 
> 1) server-side timeout mechanism to recover from RPCs that can hang
> indefinitely or take a really long time (fsfreeze, fopen, etc),
> currently it's 30 seconds, may need to bump it to 60 for fsfreeze, or
> potentially add an RPC to change the server-side timeout
> 2) a simple way to temporarily turn off logging so agent doesn't
> deadlock itself
> 3) a way to register a cleanup handler when a timeout occurs.
> 4) disabling RPCs where proper accounting/logging is required
> (guest-open-file, guest-shutdown, etc)
> 
> #4 isn't implemented...I think this could be done fairly in-evasively
> with something like:
> 
> Response important_rpc():
>   if (!ga_log("syslog", LEVEL_CRITICAL, "important stuff happening"))
> return ERROR_LOGGING_CURRENTLY_DISABLED

Either that, or maybe simply disable the full command while the freeze
is in progress? I fear we're more likely to miss a case of checking for
logging than we are to miss command disabling?

It should still be very non evasive, maybe just a flag in the struct
declaring the functions marking it as logging-required and if the
no-logging flag is set, the command is made to wait, or return -EAGAIN

> 
> bool ga_log(log_domain, level, msg):
>   if (log_domain == "syslog")
> if (!logging_enabled && is_critical(log_level))
>   return False;
> syslog(msg, ...)
>   else
> if (logging_enabled)
>   normallog(msg, ...)
>   return True
> 
> With that I think we could actually drop the fsfreeze stuff in. Thoughts?

IMHO it is better to disable the commands rather than just logging, but
either way should allow it to drop in.

Sorry for the late reply, been a bit swamped here.

Cheers,
Jes



Re: [Qemu-devel] implementing ARM926EJ-S support

2011-05-03 Thread Alessandro
> I'm not sure what source tree you're looking at. Code for ARM core
> as a target is in target-arm/. 

Exactly that.


> We don't support Jazelle. We don't implement the TCMs (in the same
> way we don't implement caches). We probably don't get all the
> device-specific cp15 registers right. (None of that is likely to be
> of any practical importance.)

 I have noticed that fully-simulating TCMs could be very difficult too.
My first trouble - for now - is: I can not distinguish beetween _CRITICAL_ 
features(that must be implemented) and _OPTIONAL_ features which can be omitted 
for simplicity.

> What SoC are you planning to model? I assume you have one in
> mind since you were specific about wanting the ARM926 rather
> than a more recent ARM core...

I was originally thinking to TI-DM365: as you can see, this SoC includes 
several 
_inusual_ peripherals, other than main core ARM926.

Linux runs on it: https://linuxlink.timesys.com/dev_center/dm365

IT

Re: [Qemu-devel] implementing ARM926EJ-S support

2011-05-03 Thread Alessandro
Hi, Anton.
Your proposal is also interesting :-)

It is well-documented hardware?

IT





Da: Антон Кочков 
A: Peter Maydell 
Cc: Alessandro ; qemu-devel@nongnu.org
Inviato: Mar 3 maggio 2011, 00:04:13
Oggetto: Re: [Qemu-devel] implementing ARM926EJ-S support

Alessandro, I think you can try add support for one of the Qualcomm
MSM6100, MSM6125, MSM6225, MSM6245, MSM6250, MSM6255A, MSM6260,
MSM6275, MSM6280, MSM6300, MSM6500, MSM6800;
Of course, if you dont know yet which you want.

Best regards,
Anton Kochkov.




On Tue, May 3, 2011 at 01:04, Peter Maydell  wrote:
> On 2 May 2011 13:46, Alessandro  wrote:
>> ARM core-related code appears to be located (mainly)under ARM directory;
>> peripherals-related files should be located under "machine" dir.
>
> I'm not sure what source tree you're looking at. Code for ARM core
> as a target is in target-arm/. Device models are in hw/. (tcg/arm
> is support for emulating other CPUs on an ARM host, so not relevant
> for you.)
>
>> I still have some questions about QEMU ARM926 support:
>>
>> 1- What standard extensions are supported? e.g Jazelle.
>
> We don't support Jazelle. We don't implement the TCMs (in the same
> way we don't implement caches). We probably don't get all the
> device-specific cp15 registers right. (None of that is likely to be
> of any practical importance.) I can't see anything else missing from
> a quick scan.
>
> The rough rule of thumb for support is "if Linux uses it it's almost
> certainly supported; otherwise it might be missing or broken" :-)
>
>> 2- Sometimes, SoC includes "inusual"(and _poorly_ documented) hardware: ISP,
>> video Coproc, and so on.
>> This increase significantly the final complexity of project. What are the
>> "guidelines" to follow? What must be implemented, and what could be safely
>> ignored?
>
> Well, if you're just doing things for your own amusement you can
> implement or leave out what you like. For things to be included
> in QEMU my personal opinion (I don't have a veto or anything) would
> be that the question is whether there's any benefit to the new
> SoC model that the existing ones don't provide -- is it some
> common piece of hardware that a lot of people own and might want
> to use a model of, for example? If you pass that hurdle then there
> is presumably an OS and set of applications that run on the real
> hardware, and the minimum level of peripheral support would be
> "enough to run that".
>
> What SoC are you planning to model? I assume you have one in
> mind since you were specific about wanting the ARM926 rather
> than a more recent ARM core...
>
> -- PMM
>
>

Re: [Qemu-devel] [PATCH] kvm: ppc: detect old headers

2011-05-03 Thread Jan Kiszka
On 2011-05-03 14:33, Alexander Graf wrote:
> 
> On 03.05.2011, at 14:31, Jan Kiszka wrote:
> 
>> On 2011-05-03 14:28, Alexander Graf wrote:
>>>
>>> On 03.05.2011, at 14:14, Jan Kiszka wrote:
>>>
 On 2011-05-03 13:54, Alexander Graf wrote:
> When compiling Qemu with older kernel headers, the PVR setting
> mechanism isn't available yet. Unfortunately, back then I didn't add
> a capability we could check against, so all we can do is add a configure
> test to see if we support PVR setting. For BookE, we don't care yet.
>
> This fixes compilation errors with KVM enabled on older kernel headers
> (like 2.6.32).

 Why not finally import the latest kvm kernel headers into qemu? Would
 save us a lot of current and future configure and #ifdef dances.
>>>
>>> Sure, sounds like a good topic for today's call?
>>
>> Fine with me. Patch should be done by then as well.
> 
> *shrug* I'm fairly indifferent on that topic. It would help users, so they 
> can easier compile things, but requires us to keep the headers in sync. Do 
> you have any good way of automating the process?

There will be a 'make update-kvm-headers' target, imported from
kvm-kmod. Can be run against some recent kernel, and the result will
just have to be committed & posted.

Moreover, I will drop alternative ways of pulling in headers (except via
CFLAGS overwriting). That will typically bite the patch submitter who
requires a header update and make her/him submit latest headers as well.
So far at least for the theory.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] KVM call agenda for May 03rd

2011-05-03 Thread Jan Kiszka
On 2011-05-03 12:21, Juan Quintela wrote:
> 
> Please send in any agenda items you are interested in covering.
> 

Provided there will be more topics:
 - import kvm headers into qemu, drop #ifdef maze

Otherwise, we can also discuss this based on the patch I'm preparing ATM.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] KVM call agenda for May 03rd

2011-05-03 Thread Anthony Liguori

On 05/03/2011 08:04 AM, Jan Kiszka wrote:

On 2011-05-03 12:21, Juan Quintela wrote:


Please send in any agenda items you are interested in covering.



Provided there will be more topics:
  - import kvm headers into qemu, drop #ifdef maze


This has come up a few times in the past.

The opposition tends to be that it would make feature development harder 
because you couldn't easily point QEMU at a different set of kernel headers.


These days, so few things change in the KVM interface though that maybe 
it's not the end of the world.


Regards,

Anthony Liguori



Otherwise, we can also discuss this based on the patch I'm preparing ATM.

Jan






Re: [Qemu-devel] [PATCH 3/3] ide: add TRIM support

2011-05-03 Thread Paolo Bonzini

On 05/03/2011 02:06 PM, Christoph Hellwig wrote:

+case IDE_DMA_TRIM:
+m->aiocb = dma_bdrv_io(s->bs,&s->sg, sector_num,
+   ide_issue_trim,, pmac_ide_transfer_cb, s, 1);
+break;


Not compile-tested?

Paolo



Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-05-03 Thread Markus Armbruster
Alex Williamson  writes:

> When we're trying to get a newly registered phys memory client updated
> with the current page mappings, we end up passing the region offset
> (a ram_addr_t) as the start address rather than the actual guest
> physical memory address (target_phys_addr_t).  If your guest has less
> than 3.5G of memory, these are coincidentally the same thing.  If
> there's more, the region offset for the memory above 4G starts over
> at 0, so the set_memory client will overwrite it's lower memory entries.
>
> Instead, keep track of the guest phsyical address as we're walking the
> tables and pass that to the set_memory client.
>
> Signed-off-by: Alex Williamson 
> ---
>
>  exec.c |   10 ++
>  1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 4752af1..e670929 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1742,7 +1742,7 @@ static int cpu_notify_migration_log(int enable)
>  }
>  
>  static void phys_page_for_each_1(CPUPhysMemoryClient *client,
> - int level, void **lp)
> + int level, void **lp, target_phys_addr_t 
> addr)
>  {
>  int i;
>  

Aren't you abusing target_phys_addr_t here?  It's not a physical
address, it needs to be shifted left to become one.  By how much depends
on level.  Please take pity on future maintainers and spell this out in
a comment.

Perhaps you can code it in a way that makes the parameter an address.
Probably no need for a comment then.

> @@ -1751,16 +1751,18 @@ static void phys_page_for_each_1(CPUPhysMemoryClient 
> *client,
>  }
>  if (level == 0) {
>  PhysPageDesc *pd = *lp;
> +addr <<= L2_BITS + TARGET_PAGE_BITS;
>  for (i = 0; i < L2_SIZE; ++i) {
>  if (pd[i].phys_offset != IO_MEM_UNASSIGNED) {
> -client->set_memory(client, pd[i].region_offset,
> +client->set_memory(client, addr | i << TARGET_PAGE_BITS,
> TARGET_PAGE_SIZE, pd[i].phys_offset);
>  }
>  }
>  } else {
>  void **pp = *lp;
>  for (i = 0; i < L2_SIZE; ++i) {
> -phys_page_for_each_1(client, level - 1, pp + i);
> +phys_page_for_each_1(client, level - 1, pp + i,
> + (addr << L2_BITS) | i);
>  }
>  }
>  }
> @@ -1770,7 +1772,7 @@ static void phys_page_for_each(CPUPhysMemoryClient 
> *client)
>  int i;
>  for (i = 0; i < P_L1_SIZE; ++i) {
>  phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1,
> - l1_phys_map + i);
> + l1_phys_map + i, i);
>  }
>  }
>  

Fix makes sense to me, after some head scratching.  A comment explaining
the phys map data structure would be helpful.  l1_phys_map[] has a
comment, but it's devoid of detail.



Re: [Qemu-devel] [PATCH 2/2] NBD: Convert the NBD driver to use the AIO interface.

2011-05-03 Thread Nicholas Thomas
On Thu, 2011-04-28 at 16:20 +0100, n...@bytemark.co.uk wrote:

[...]

> +static void nbd_unregister_write_request_handler(BDRVNBDState *s)
> +{
> +int sock = s->sock;
> +if (s->sock == -1) { 
> +logout("Unregister write request handler tried when socket
closed\n");
> +return;
> +}
> +
> +qemu_aio_set_fd_handler(s->sock, nbd_aio_read_response, NULL,
> +nbd_aio_flush_request, NULL, s);
> +}

Sorry, I've just realised that this bit of the patch actually causes a
compiler eror (since sock is never used). Last-minute dickering about to
get things to fit into 80 columns :/

If that's the only issue with the patch, I'd be amazed, so I'll leave
resubmitting until there's further feedback, if that's OK?

/Nick




[Qemu-devel] [PATCH] posix-aio-compat: Fix idle_threads counter

2011-05-03 Thread Kevin Wolf
A thread should only be counted as idle when it really is waiting for new
requests. Without this patch, sometimes too few threads are started as busy
threads are counted as idle.

Not sure if it makes a difference in practice outside some artificial
qemu-io/qemu-img tests, but I think the change makes sense in any case.

Signed-off-by: Kevin Wolf 
---
 posix-aio-compat.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 6d4df9d..f3cc868 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -322,7 +322,9 @@ static void *aio_thread(void *unused)
 
 while (QTAILQ_EMPTY(&request_list) &&
!(ret == ETIMEDOUT)) {
+idle_threads++;
 ret = cond_timedwait(&cond, &lock, &ts);
+idle_threads--;
 }
 
 if (QTAILQ_EMPTY(&request_list))
@@ -331,7 +333,6 @@ static void *aio_thread(void *unused)
 aiocb = QTAILQ_FIRST(&request_list);
 QTAILQ_REMOVE(&request_list, aiocb, node);
 aiocb->active = 1;
-idle_threads--;
 mutex_unlock(&lock);
 
 switch (aiocb->aio_type & QEMU_AIO_TYPE_MASK) {
@@ -353,13 +354,11 @@ static void *aio_thread(void *unused)
 
 mutex_lock(&lock);
 aiocb->ret = ret;
-idle_threads++;
 mutex_unlock(&lock);
 
 if (kill(pid, aiocb->ev_signo)) die("kill failed");
 }
 
-idle_threads--;
 cur_threads--;
 mutex_unlock(&lock);
 
@@ -371,7 +370,6 @@ static void spawn_thread(void)
 sigset_t set, oldset;
 
 cur_threads++;
-idle_threads++;
 
 /* block all signals */
 if (sigfillset(&set)) die("sigfillset");
-- 
1.7.2.3




[Qemu-devel] [PATCH v2] configure: List available targets in --help output

2011-05-03 Thread Peter Maydell
Include the list of available targets in the --help output
for the --target-list= option.

Signed-off-by: Peter Maydell 
---
v2: As suggested by Stefan Weil, put the target list in --help
rather than using '--target-list=?'. This patch includes using
fold(1) in configure -- this should be OK because fold is a
standard POSIX utility (and part of GNU coreutils and busybox).
The resulting output looks like this:

Usage: configure [options]
Options: [defaults in brackets after descriptions]

Standard options:
  --help   print this message
  --prefix=PREFIX  install in PREFIX [/usr/local]
  --interp-prefix=PREFIX   where to find shared libraries, etc.
   use %M for cpu name [/usr/gnemul/qemu-%M]
  --target-list=LIST   set target list (default: build everything)
   Available targets: i386-softmmu x86_64-softmmu 
   arm-softmmu cris-softmmu lm32-softmmu m68k-softmmu 
   microblaze-softmmu microblazeel-softmmu mips-softmmu 
   mipsel-softmmu mips64-softmmu mips64el-softmmu 
   ppc-softmmu ppcemb-softmmu ppc64-softmmu sh4-softmmu 
   sh4eb-softmmu sparc-softmmu sparc64-softmmu 
   i386-linux-user x86_64-linux-user alpha-linux-user 
   arm-linux-user armeb-linux-user cris-linux-user 
   m68k-linux-user microblaze-linux-user 
   microblazeel-linux-user mips-linux-user 
   mipsel-linux-user ppc-linux-user ppc64-linux-user 
   ppc64abi32-linux-user sh4-linux-user 
   sh4eb-linux-user sparc-linux-user sparc64-linux-user 
   sparc32plus-linux-user unicore32-linux-user 

Advanced options (experts only):
[etc]


 configure |  134 -
 1 files changed, 70 insertions(+), 64 deletions(-)

diff --git a/configure b/configure
index 6f75e2e..f46f5d4 100755
--- a/configure
+++ b/configure
@@ -822,6 +822,72 @@ esac
 
 [ -z "$guest_base" ] && guest_base="$host_guest_base"
 
+
+default_target_list=""
+
+# these targets are portable
+if [ "$softmmu" = "yes" ] ; then
+default_target_list="\
+i386-softmmu \
+x86_64-softmmu \
+arm-softmmu \
+cris-softmmu \
+lm32-softmmu \
+m68k-softmmu \
+microblaze-softmmu \
+microblazeel-softmmu \
+mips-softmmu \
+mipsel-softmmu \
+mips64-softmmu \
+mips64el-softmmu \
+ppc-softmmu \
+ppcemb-softmmu \
+ppc64-softmmu \
+sh4-softmmu \
+sh4eb-softmmu \
+sparc-softmmu \
+sparc64-softmmu \
+"
+fi
+# the following are Linux specific
+if [ "$linux_user" = "yes" ] ; then
+default_target_list="${default_target_list}\
+i386-linux-user \
+x86_64-linux-user \
+alpha-linux-user \
+arm-linux-user \
+armeb-linux-user \
+cris-linux-user \
+m68k-linux-user \
+microblaze-linux-user \
+microblazeel-linux-user \
+mips-linux-user \
+mipsel-linux-user \
+ppc-linux-user \
+ppc64-linux-user \
+ppc64abi32-linux-user \
+sh4-linux-user \
+sh4eb-linux-user \
+sparc-linux-user \
+sparc64-linux-user \
+sparc32plus-linux-user \
+unicore32-linux-user \
+"
+fi
+# the following are Darwin specific
+if [ "$darwin_user" = "yes" ] ; then
+default_target_list="$default_target_list i386-darwin-user ppc-darwin-user 
"
+fi
+# the following are BSD specific
+if [ "$bsd_user" = "yes" ] ; then
+default_target_list="${default_target_list}\
+i386-bsd-user \
+x86_64-bsd-user \
+sparc-bsd-user \
+sparc64-bsd-user \
+"
+fi
+
 if test x"$show_help" = x"yes" ; then
 cat << EOF
 
@@ -834,7 +900,9 @@ echo "  --help   print this message"
 echo "  --prefix=PREFIX  install in PREFIX [$prefix]"
 echo "  --interp-prefix=PREFIX   where to find shared libraries, etc."
 echo "   use %M for cpu name [$interp_prefix]"
-echo "  --target-list=LIST   set target list [$target_list]"
+echo "  --target-list=LIST   set target list (default: build everything)"
+echo "Available targets: $default_target_list" | \
+fold -s -w 53 | sed -e 's/^/   /'
 echo ""
 echo "Advanced options (experts only):"
 echo "  --source-path=PATH   path of source code [$source_path]"
@@ -1004,70 +1072,8 @@ if test "$solaris" = "yes" ; then
   fi
 fi
 
-
 if test -z "$target_list" ; then
-# these targets are portable
-if [ "$softmmu" = "yes" ] ; then
-target_list="\
-i386-softmmu \
-x86_64-softmmu \
-arm-softmmu \
-cris-softmmu \
-lm32-softmmu \
-m68k-softmmu \
-microblaze-softmmu \
-microblazeel-softmmu \
-mips-softmmu \
-mipsel-softmmu \
-mips64-softmmu \
-mips64el-softmmu \
-ppc-softmmu \
-ppcemb-softmmu \
-ppc64-softmmu \
-sh4-softmmu \
-sh4eb-softmmu \
-sparc-softmmu \
-sparc64-softmmu \
-"
-fi
-# the following are Linux specific
-if [ "$linux_user" = "yes" ] ; then
-target_list="${target_list}\
-i386-linux-user \
-x86_64-linu

Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

2011-05-03 Thread Michael Roth

On 05/03/2011 07:51 AM, Jes Sorensen wrote:

On 04/21/11 15:55, Michael Roth wrote:

Did you do anything with the fsfreeze patches, or were they dropped in
the migration to qapi?


They were pending some changes required on the agent side that weren't
really addressed/doable until this patchset, namely:

1) server-side timeout mechanism to recover from RPCs that can hang
indefinitely or take a really long time (fsfreeze, fopen, etc),
currently it's 30 seconds, may need to bump it to 60 for fsfreeze, or
potentially add an RPC to change the server-side timeout
2) a simple way to temporarily turn off logging so agent doesn't
deadlock itself
3) a way to register a cleanup handler when a timeout occurs.
4) disabling RPCs where proper accounting/logging is required
(guest-open-file, guest-shutdown, etc)

#4 isn't implemented...I think this could be done fairly in-evasively
with something like:

Response important_rpc():
   if (!ga_log("syslog", LEVEL_CRITICAL, "important stuff happening"))
 return ERROR_LOGGING_CURRENTLY_DISABLED


Either that, or maybe simply disable the full command while the freeze
is in progress? I fear we're more likely to miss a case of checking for
logging than we are to miss command disabling?

It should still be very non evasive, maybe just a flag in the struct
declaring the functions marking it as logging-required and if the
no-logging flag is set, the command is made to wait, or return -EAGAIN



Yup when I actually starting dropping it in I realized this was a much 
better approach. Although, for now I just added something like "if 
(!logging_enabled) { error_set(QERR_GA_LOGGING_DISABLED); return }" to 
the start of functions where logging is considered critical, which will 
result in the user getting an error message about logging so it's not 
too much of a surprise to them.


The actual dispatch code closely mirrors Anthony's dispatch stuff for 
QMP so I was hesitant to try to modify it to handle this automatically, 
since it would require some changes to how the schema parsing/handling 
is done (would probably need to add a "requires_logging" flag in the 
schema). Wouldn't take much though. Either way, should be a clean 
conversion if we decide to go that route.




bool ga_log(log_domain, level, msg):
   if (log_domain == "syslog")
 if (!logging_enabled&&  is_critical(log_level))
   return False;
 syslog(msg, ...)
   else
 if (logging_enabled)
   normallog(msg, ...)
   return True

With that I think we could actually drop the fsfreeze stuff in. Thoughts?


IMHO it is better to disable the commands rather than just logging, but
either way should allow it to drop in.


Kinda agree, but logging seems to be the real dependency. With the 
server-side timeouts now in place even doing stuff like fopen/fwrite is 
permitted (it would just timeout if it blocked too long). It's the 
logging stuff that we don't really have a way to recover from, because 
it's not run in a thread we can just nuke after a certain amount of time.


Even when we're not frozen, we can't guarantee an fopen/fwrite/fread 
will succeed, so failures shouldn't be too much of a surprise since they 
need to be handled anyway. And determining whether or not a command 
should be marked as executable during a freeze is somewhat nebulous 
(fopen might work for read-only access, but hang for write access when 
O_CREATE is set, fwrite might succeed if it doesn't require a flush, 
etc), plus internal things like logging need to be taken into account.


So, for now at least I think it's a reasonable way to do it.



Sorry for the late reply, been a bit swamped here.


No problem I have your patches in my tree now. They still need a little 
bit of love and testing but I should be able to get them out on the list 
shortly.




Cheers,
Jes





Re: [Qemu-devel] KVM call agenda for May 03rd

2011-05-03 Thread Jes Sorensen
On 05/03/11 15:04, Jan Kiszka wrote:
> On 2011-05-03 12:21, Juan Quintela wrote:
>>
>> Please send in any agenda items you are interested in covering.
>>
> 
> Provided there will be more topics:
>  - import kvm headers into qemu, drop #ifdef maze
> 
> Otherwise, we can also discuss this based on the patch I'm preparing ATM.

Since this it the only topic for the call, I suggest we cancel and add
it to next week's agenda.

Cheers,
Jes





[Qemu-devel] [RFC][PATCH] Import Linux headers for KVM and vhost

2011-05-03 Thread Jan Kiszka
This helps reducing our build-time checks for feature support in the
available Linux kernel headers. And it helps users that do not have
sufficiently recent headers installed on their build machine.

Header upstate is triggered via 'make update-linux-headers', optionally
specifying a kernel directory as source via 'LINUX='.

---

I'm sure this is not yet perfect from technical POV (e.g. the patch to
Makefile.target looks like a hack to me, better ideas welcome),
therefore RFC and no SOB. But it should demonstrate the plan. The
workflow for adding a new KVM feature support would be first a 'make
update-linux-header LINUX=/my/local/linux' + git commit, and then the
commit of the kvm, vhost, whatever changes.

A patch to actually add the result of the header update would be posted
separately. Same for the now possible massive cleanups of kvm sources.

 Makefile|   17 +++
 Makefile.target |2 +-
 configure   |  127 ---
 3 files changed, 36 insertions(+), 110 deletions(-)

diff --git a/Makefile b/Makefile
index 67c0268..6ca065b 100644
--- a/Makefile
+++ b/Makefile
@@ -341,3 +341,20 @@ tarbin:
 
 # Include automatically generated dependency files
 -include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d ui/*.d)
+
+update-linux-headers:
+   for arch in x86 powerpc s390; do \
+   $(MAKE) -C $(LINUX) INSTALL_HDR_PATH=`pwd`/.tmp-hdrs 
SRCARCH="$$arch" headers_install; \
+   rm -rf $(SRC_PATH)/include/asm-"$$arch"/*; \
+   for header in kvm.h kvm_para.h; do \
+   cp .tmp-hdrs/include/asm/$$header 
$(SRC_PATH)/include/asm-"$$arch"; \
+   done; \
+   if test $$arch == x86; then \
+   cp .tmp-hdrs/include/asm/hyperv.h 
$(SRC_PATH)/include/asm-x86; \
+   fi \
+   done
+   rm -rf $(SRC_PATH)/include/linux/*
+   for header in kvm.h kvm_para.h vhost.h virtio_config.h virtio_ring.h; 
do \
+   cp .tmp-hdrs/include/linux/$$header $(SRC_PATH)/include/linux; \
+   done
+   rm -rf .tmp-hdrs
diff --git a/Makefile.target b/Makefile.target
index 89280c6..b0fe021 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -14,7 +14,7 @@ endif
 
 TARGET_PATH=$(SRC_PATH)/target-$(TARGET_BASE_ARCH)
 $(call set-vpath, $(SRC_PATH):$(TARGET_PATH):$(SRC_PATH)/hw)
-QEMU_CFLAGS+= -I.. -I$(TARGET_PATH) -DNEED_CPU_H
+QEMU_CFLAGS+= -I.. -I../include -I$(TARGET_PATH) -DNEED_CPU_H
 
 include $(SRC_PATH)/Makefile.objs
 
diff --git a/configure b/configure
index 3239fbb..792e4c2 100755
--- a/configure
+++ b/configure
@@ -113,8 +113,7 @@ curl=""
 curses=""
 docs=""
 fdt=""
-kvm=""
-kvm_para=""
+kvm="yes"
 nptl=""
 sdl=""
 vnc="yes"
@@ -129,7 +128,7 @@ vnc_thread="no"
 xen=""
 linux_aio=""
 attr=""
-vhost_net=""
+vhost_net="yes"
 xfs=""
 
 gprof="no"
@@ -1695,109 +1694,6 @@ EOF
 fi
 
 ##
-# kvm probe
-if test "$kvm" != "no" ; then
-cat > $TMPC <
-#if !defined(KVM_API_VERSION) || KVM_API_VERSION < 12 || KVM_API_VERSION > 12
-#error Invalid KVM version
-#endif
-EOF
-must_have_caps="KVM_CAP_USER_MEMORY \
-KVM_CAP_DESTROY_MEMORY_REGION_WORKS \
-KVM_CAP_COALESCED_MMIO \
-KVM_CAP_SYNC_MMU \
-   "
-if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) ; then
-  must_have_caps="$caps \
-  KVM_CAP_SET_TSS_ADDR \
-  KVM_CAP_EXT_CPUID \
-  KVM_CAP_CLOCKSOURCE \
-  KVM_CAP_NOP_IO_DELAY \
-  KVM_CAP_PV_MMU \
-  KVM_CAP_MP_STATE \
-  KVM_CAP_USER_NMI \
- "
-fi
-for c in $must_have_caps ; do
-  cat >> $TMPC <> $TMPC  $TMPC <
-int main(void) { return 0; }
-EOF
-if compile_prog "$kvm_cflags" "" ; then
-  kvm_para=yes
-fi
-  else
-if test "$kvm" = "yes" ; then
-  if has awk && has grep; then
-kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \
-   | grep "error: " \
-   | awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'`
-if test "$kvmerr" != "" ; then
-  echo -e "${kvmerr}\n\
-NOTE: To enable KVM support, update your kernel to 2.6.29+ or install \
-recent kvm-kmod from http://sourceforge.net/projects/kvm.";
-fi
-  fi
-  feature_not_found "kvm"
-fi
-kvm=no
-  fi
-fi
-
-##
-# test for vhost net
-
-if test "$vhost_net" != "no"; then
-if test "$kvm" != "no"; then
-cat > $TMPC <
-int main(void) { return 0; }
-EOF
-if compile_prog "$kvm_cflags" "" ; then
-vhost_net=yes
-else
-if test "$vhost_net" = "yes" ; then
-feature_not_found "vhost-net"
-  

Re: [Qemu-devel] KVM call agenda for May 03rd

2011-05-03 Thread Jan Kiszka
On 2011-05-03 15:07, Anthony Liguori wrote:
> On 05/03/2011 08:04 AM, Jan Kiszka wrote:
>> On 2011-05-03 12:21, Juan Quintela wrote:
>>>
>>> Please send in any agenda items you are interested in covering.
>>>
>>
>> Provided there will be more topics:
>>   - import kvm headers into qemu, drop #ifdef maze
> 
> This has come up a few times in the past.
> 
> The opposition tends to be that it would make feature development harder 
> because you couldn't easily point QEMU at a different set of kernel headers.

There should be no need to worry, even if for those bits that will
continue to change more frequently. I've included a hopefully easy to
use update mechanism.

> 
> These days, so few things change in the KVM interface though that maybe 
> it's not the end of the world.

PowerPC and future KVM arch will go through the same changes that x86
should be through now. So I would prefer to avoid the mistakes we made
for the latter.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

2011-05-03 Thread Jes Sorensen
On 05/03/11 15:53, Michael Roth wrote:
>>
>> IMHO it is better to disable the commands rather than just logging, but
>> either way should allow it to drop in.
> 
> Kinda agree, but logging seems to be the real dependency. With the
> server-side timeouts now in place even doing stuff like fopen/fwrite is
> permitted (it would just timeout if it blocked too long). It's the
> logging stuff that we don't really have a way to recover from, because
> it's not run in a thread we can just nuke after a certain amount of time.
> 
> Even when we're not frozen, we can't guarantee an fopen/fwrite/fread
> will succeed, so failures shouldn't be too much of a surprise since they
> need to be handled anyway. And determining whether or not a command
> should be marked as executable during a freeze is somewhat nebulous
> (fopen might work for read-only access, but hang for write access when
> O_CREATE is set, fwrite might succeed if it doesn't require a flush,
> etc), plus internal things like logging need to be taken into account.
> 
> So, for now at least I think it's a reasonable way to do it.

Please be very careful with any fwrite() calls - it's not just logging.
Any writes to the frozen file systems will result in the caller being
put to sleep until the file system is unfrozen. It won't timeout, and
the agent will be stuck hanging in that call.

It's fun playing with the fsfreeze stuff on your desktop system - doing
it in an xterm has 'interesting' effects. :)

This is why I prefer the 'disable function' rather 'disable logging'
approach.

>> Sorry for the late reply, been a bit swamped here.
> 
> No problem I have your patches in my tree now. They still need a little
> bit of love and testing but I should be able to get them out on the list
> shortly.

Sounds great!

Cheers,
Jes



Re: [Qemu-devel] [PATCH 05/17] kvm: add kvm stub for arch specific stuff

2011-05-03 Thread Alexander Graf

On 18.04.2011, at 20:34, Aurelien Jarno wrote:

> On Fri, Apr 15, 2011 at 05:32:46PM +0200, Alexander Graf wrote:
>> We have a generic stub architecture for kvm calls, but some architectures
>> are different from others. So we do want to be able to have stubs for
>> architecture specific functionality as well.
>> 
>> This patch adds kvm stubs for all architectures.
>> 
>> Signed-off-by: Alexander Graf 
>> ---
>> Makefile.target   |2 +-
>> target-alpha/kvm-arch-stub.c  |   26 +
>> target-arm/kvm-arch-stub.c|   26 +
>> target-cris/kvm-arch-stub.c   |   26 +
>> target-i386/kvm-arch-stub.c   |   26 +
>> target-lm32/kvm-arch-stub.c   |   26 +
>> target-m68k/kvm-arch-stub.c   |   26 +
>> target-microblaze/kvm-arch-stub.c |   26 +
>> target-mips/kvm-arch-stub.c   |   26 +
>> target-ppc/kvm-arch-stub.c|   26 +
>> target-s390x/kvm-arch-stub.c  |   38 
>> +
>> target-sh4/kvm-arch-stub.c|   26 +
>> target-sparc/kvm-arch-stub.c  |   26 +
>> target-unicore32/kvm-arch-stub.c  |   26 +
>> 14 files changed, 351 insertions(+), 1 deletions(-)
>> create mode 100644 target-alpha/kvm-arch-stub.c
>> create mode 100644 target-arm/kvm-arch-stub.c
>> create mode 100644 target-cris/kvm-arch-stub.c
>> create mode 100644 target-i386/kvm-arch-stub.c
>> create mode 100644 target-lm32/kvm-arch-stub.c
>> create mode 100644 target-m68k/kvm-arch-stub.c
>> create mode 100644 target-microblaze/kvm-arch-stub.c
>> create mode 100644 target-mips/kvm-arch-stub.c
>> create mode 100644 target-ppc/kvm-arch-stub.c
>> create mode 100644 target-s390x/kvm-arch-stub.c
>> create mode 100644 target-sh4/kvm-arch-stub.c
>> create mode 100644 target-sparc/kvm-arch-stub.c
>> create mode 100644 target-unicore32/kvm-arch-stub.c
> 
> Do we really want to create so much files on architectures we will never
> see KVM support? Actually I know very few things about KVM, so it would
> be better to have this patch reviewed by someone else. Avi or Anthony
> maybe?

Well, the main idea is to be able to have a unified place to put stub functions 
into. And as it stands with most generalizations, we either make it generic or 
not :).
Maybe there's some Makefile magic to only compile the stub if the file exists? 
I certainly don't know of any.


Alex




Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset

2011-05-03 Thread Alex Williamson
On Tue, 2011-05-03 at 15:15 +0200, Markus Armbruster wrote:
> Alex Williamson  writes:
> 
> > When we're trying to get a newly registered phys memory client updated
> > with the current page mappings, we end up passing the region offset
> > (a ram_addr_t) as the start address rather than the actual guest
> > physical memory address (target_phys_addr_t).  If your guest has less
> > than 3.5G of memory, these are coincidentally the same thing.  If
> > there's more, the region offset for the memory above 4G starts over
> > at 0, so the set_memory client will overwrite it's lower memory entries.
> >
> > Instead, keep track of the guest phsyical address as we're walking the
> > tables and pass that to the set_memory client.
> >
> > Signed-off-by: Alex Williamson 
> > ---
> >
> >  exec.c |   10 ++
> >  1 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index 4752af1..e670929 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1742,7 +1742,7 @@ static int cpu_notify_migration_log(int enable)
> >  }
> >  
> >  static void phys_page_for_each_1(CPUPhysMemoryClient *client,
> > - int level, void **lp)
> > + int level, void **lp, target_phys_addr_t 
> > addr)
> >  {
> >  int i;
> >  
> 
> Aren't you abusing target_phys_addr_t here?  It's not a physical
> address, it needs to be shifted left to become one.  By how much depends
> on level.  Please take pity on future maintainers and spell this out in
> a comment.
> 
> Perhaps you can code it in a way that makes the parameter an address.
> Probably no need for a comment then.

Right, it's not a target_phys_addr_t on passing to the function, but it
becomes one as we work, so it still seemed the appropriate data type.  I
rather like how the shifting works into the recursive-ness of the
function, I think it removes a bit of ugliness for figuring how many
levels are there, where am I, how many multiples of *_BITS do I shift.
I'll add a comment and hope that helps.

> > @@ -1751,16 +1751,18 @@ static void 
> > phys_page_for_each_1(CPUPhysMemoryClient *client,
> >  }
> >  if (level == 0) {
> >  PhysPageDesc *pd = *lp;
> > +addr <<= L2_BITS + TARGET_PAGE_BITS;
> >  for (i = 0; i < L2_SIZE; ++i) {
> >  if (pd[i].phys_offset != IO_MEM_UNASSIGNED) {
> > -client->set_memory(client, pd[i].region_offset,
> > +client->set_memory(client, addr | i << TARGET_PAGE_BITS,
> > TARGET_PAGE_SIZE, pd[i].phys_offset);
> >  }
> >  }
> >  } else {
> >  void **pp = *lp;
> >  for (i = 0; i < L2_SIZE; ++i) {
> > -phys_page_for_each_1(client, level - 1, pp + i);
> > +phys_page_for_each_1(client, level - 1, pp + i,
> > + (addr << L2_BITS) | i);
> >  }
> >  }
> >  }
> > @@ -1770,7 +1772,7 @@ static void phys_page_for_each(CPUPhysMemoryClient 
> > *client)
> >  int i;
> >  for (i = 0; i < P_L1_SIZE; ++i) {
> >  phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1,
> > - l1_phys_map + i);
> > + l1_phys_map + i, i);
> >  }
> >  }
> >  
> 
> Fix makes sense to me, after some head scratching.  A comment explaining
> the phys map data structure would be helpful.  l1_phys_map[] has a
> comment, but it's devoid of detail.

I'll see what I can do, though I'm pretty sure I'm not at the top of the
list for describing the existence and format of these tables.  Thanks,

Alex




Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

2011-05-03 Thread Michael Roth

On 05/03/2011 09:12 AM, Jes Sorensen wrote:

On 05/03/11 15:53, Michael Roth wrote:


IMHO it is better to disable the commands rather than just logging, but
either way should allow it to drop in.


Kinda agree, but logging seems to be the real dependency. With the
server-side timeouts now in place even doing stuff like fopen/fwrite is
permitted (it would just timeout if it blocked too long). It's the
logging stuff that we don't really have a way to recover from, because
it's not run in a thread we can just nuke after a certain amount of time.

Even when we're not frozen, we can't guarantee an fopen/fwrite/fread
will succeed, so failures shouldn't be too much of a surprise since they
need to be handled anyway. And determining whether or not a command
should be marked as executable during a freeze is somewhat nebulous
(fopen might work for read-only access, but hang for write access when
O_CREATE is set, fwrite might succeed if it doesn't require a flush,
etc), plus internal things like logging need to be taken into account.

So, for now at least I think it's a reasonable way to do it.


Please be very careful with any fwrite() calls - it's not just logging.
Any writes to the frozen file systems will result in the caller being
put to sleep until the file system is unfrozen. It won't timeout, and
the agent will be stuck hanging in that call.

It's fun playing with the fsfreeze stuff on your desktop system - doing
it in an xterm has 'interesting' effects. :)

This is why I prefer the 'disable function' rather 'disable logging'
approach.



I'll review the code to make sure, but what I mean is that the actual 
daemon only ever does disk i/o when it's doing logging. All the RPCs are 
handled in a separate worker thread, so if they do something fubar we 
should still be able to recover (the timeouts are done by 
pthread_cancel()'ing the worker thread). So things should be recoverable 
so long as we don't log when frozen. RPCs succeeding or failing we don't 
care too much about, since they'll just result in a nice "command timed 
out" response to the user eventually.


Checking logging_enabled is more to ensure we don't let the user exploit 
the fs_freeze side-effects to do sensitive stuff under the guest owner's 
nose (not that we couldn't anyway from host, but it may important for 
piece of mind about have an externally accessible/privileged agent in 
your guest).


works_under_fsfreeze is harder to determine beforehand, unless maybe you 
flag any command that did i/o other than logging...but there are 
actually use cases where that would be too restrictive. If they're using 
fwrite to pipe output through a character device, for instance, or 
twiddling knobs in sysfs. Better to keep the same semantics as you'd 
have when "physically" on the machine, when possible.


Will need to do some good testing though, fsfreeze is definitely a 
strange state to be in :)



Sorry for the late reply, been a bit swamped here.


No problem I have your patches in my tree now. They still need a little
bit of love and testing but I should be able to get them out on the list
shortly.


Sounds great!

Cheers,
Jes





Re: [Qemu-devel] [PATCH 05/17] kvm: add kvm stub for arch specific stuff

2011-05-03 Thread Jan Kiszka
On 2011-05-03 16:17, Alexander Graf wrote:
> 
> On 18.04.2011, at 20:34, Aurelien Jarno wrote:
> 
>> On Fri, Apr 15, 2011 at 05:32:46PM +0200, Alexander Graf wrote:
>>> We have a generic stub architecture for kvm calls, but some architectures
>>> are different from others. So we do want to be able to have stubs for
>>> architecture specific functionality as well.
>>>
>>> This patch adds kvm stubs for all architectures.
>>>
>>> Signed-off-by: Alexander Graf 
>>> ---
>>> Makefile.target   |2 +-
>>> target-alpha/kvm-arch-stub.c  |   26 +
>>> target-arm/kvm-arch-stub.c|   26 +
>>> target-cris/kvm-arch-stub.c   |   26 +
>>> target-i386/kvm-arch-stub.c   |   26 +
>>> target-lm32/kvm-arch-stub.c   |   26 +
>>> target-m68k/kvm-arch-stub.c   |   26 +
>>> target-microblaze/kvm-arch-stub.c |   26 +
>>> target-mips/kvm-arch-stub.c   |   26 +
>>> target-ppc/kvm-arch-stub.c|   26 +
>>> target-s390x/kvm-arch-stub.c  |   38 
>>> +
>>> target-sh4/kvm-arch-stub.c|   26 +
>>> target-sparc/kvm-arch-stub.c  |   26 +
>>> target-unicore32/kvm-arch-stub.c  |   26 +
>>> 14 files changed, 351 insertions(+), 1 deletions(-)
>>> create mode 100644 target-alpha/kvm-arch-stub.c
>>> create mode 100644 target-arm/kvm-arch-stub.c
>>> create mode 100644 target-cris/kvm-arch-stub.c
>>> create mode 100644 target-i386/kvm-arch-stub.c
>>> create mode 100644 target-lm32/kvm-arch-stub.c
>>> create mode 100644 target-m68k/kvm-arch-stub.c
>>> create mode 100644 target-microblaze/kvm-arch-stub.c
>>> create mode 100644 target-mips/kvm-arch-stub.c
>>> create mode 100644 target-ppc/kvm-arch-stub.c
>>> create mode 100644 target-s390x/kvm-arch-stub.c
>>> create mode 100644 target-sh4/kvm-arch-stub.c
>>> create mode 100644 target-sparc/kvm-arch-stub.c
>>> create mode 100644 target-unicore32/kvm-arch-stub.c
>>
>> Do we really want to create so much files on architectures we will never
>> see KVM support? Actually I know very few things about KVM, so it would
>> be better to have this patch reviewed by someone else. Avi or Anthony
>> maybe?
> 
> Well, the main idea is to be able to have a unified place to put stub 
> functions into. And as it stands with most generalizations, we either make it 
> generic or not :).
> Maybe there's some Makefile magic to only compile the stub if the file 
> exists? I certainly don't know of any.

This approach looks wrong.

The point of kvm stubs is to allow generic components to be built
independently of kvm enabled/disabled. But target-specific callbacks
can't be part of generic components anyway. So there is no need for a
stub, those bits will be built per-target anyway.

The examples you provided with this patch underline it:
s390-virtio-bus.c should be built for s390 but nothing else.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 05/17] kvm: add kvm stub for arch specific stuff

2011-05-03 Thread Peter Maydell
On 3 May 2011 15:17, Alexander Graf  wrote:
> Maybe there's some Makefile magic to only compile the stub if the file
> exists? I certainly don't know of any.

obj-$(CONFIG_NO_KVM) += kvm-stub.o
ifneq ($(wildcard $(TARGET_PATH)/kvm-arch-stub.c),)
obj-$(CONFIG_NO_KVM) += kvm-arch-stub.o
endif

? (not very heavily tested)

-- PMM



[Qemu-devel] [PULL] spice: fix locking

2011-05-03 Thread Gerd Hoffmann
  Hi,

This is the current spice patch queue ready for pull.  Patches have been
posted a few days ago for review.  A minor issue (leftover debug bit,
spotten by Alon) has been fixed and the patch queue has been rebased to
latest master, otherwise it is unmodified.

Please pull.

cheers,
  Gerd

The following changes since commit d2d979c628e4b2c4a3cb71a31841875795c79043:

  NBD: Avoid leaking a couple of strings when the NBD device is closed 
(2011-05-03 11:29:21 +0200)

are available in the git repository at:
  git://anongit.freedesktop.org/spice/qemu spice.v35

Gerd Hoffmann (3):
  spice: don't create updates in spice server context.
  spice: don't call displaystate callbacks from spice server context.
  spice: drop obsolete iothread locking

Jes Sorensen (1):
  Make spice dummy functions inline to fix calls not checking return values

 hw/qxl-render.c|   25 ++---
 hw/qxl.c   |   27 +++
 ui/qemu-spice.h|   12 -
 ui/spice-display.c |   61 +--
 ui/spice-display.h |   24 
 5 files changed, 93 insertions(+), 56 deletions(-)



[Qemu-devel] [PATCH 4/4] spice: drop obsolete iothread locking

2011-05-03 Thread Gerd Hoffmann
We don't use qemu internals from spice server context any more.
Thus we don't also need to grab the iothread mutex from spice
server context.  And we don't have to temporarely release the
lock to avoid deadlocks.  Drop all the calls.

Signed-off-by: Gerd Hoffmann 
---
 hw/qxl.c   |8 
 ui/spice-display.c |6 --
 2 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 4dfddf0..2bb36c6 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -666,10 +666,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
 dprint(d, 1, "%s: start%s\n", __FUNCTION__,
loadvm ? " (loadvm)" : "");
 
-qemu_mutex_unlock_iothread();
 d->ssd.worker->reset_cursor(d->ssd.worker);
 d->ssd.worker->reset_image_cache(d->ssd.worker);
-qemu_mutex_lock_iothread();
 qxl_reset_surfaces(d);
 qxl_reset_memslots(d);
 
@@ -799,9 +797,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
 {
 dprint(d, 1, "%s:\n", __FUNCTION__);
 d->mode = QXL_MODE_UNDEFINED;
-qemu_mutex_unlock_iothread();
 d->ssd.worker->destroy_surfaces(d->ssd.worker);
-qemu_mutex_lock_iothread();
 memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
 }
 
@@ -870,9 +866,7 @@ static void qxl_destroy_primary(PCIQXLDevice *d)
 dprint(d, 1, "%s\n", __FUNCTION__);
 
 d->mode = QXL_MODE_UNDEFINED;
-qemu_mutex_unlock_iothread();
 d->ssd.worker->destroy_primary_surface(d->ssd.worker, 0);
-qemu_mutex_lock_iothread();
 }
 
 static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
@@ -942,10 +936,8 @@ static void ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 case QXL_IO_UPDATE_AREA:
 {
 QXLRect update = d->ram->update_area;
-qemu_mutex_unlock_iothread();
 d->ssd.worker->update_area(d->ssd.worker, d->ram->update_surface,
&update, NULL, 0, 0);
-qemu_mutex_lock_iothread();
 break;
 }
 case QXL_IO_NOTIFY_CMD:
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 8579bfd..15f0704 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -176,18 +176,14 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay 
*ssd)
 surface.mem= (intptr_t)ssd->buf;
 surface.group_id   = MEMSLOT_GROUP_HOST;
 
-qemu_mutex_unlock_iothread();
 ssd->worker->create_primary_surface(ssd->worker, 0, &surface);
-qemu_mutex_lock_iothread();
 }
 
 void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
 {
 dprint(1, "%s:\n", __FUNCTION__);
 
-qemu_mutex_unlock_iothread();
 ssd->worker->destroy_primary_surface(ssd->worker, 0);
-qemu_mutex_lock_iothread();
 }
 
 void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
@@ -197,9 +193,7 @@ void qemu_spice_vm_change_state_handler(void *opaque, int 
running, int reason)
 if (running) {
 ssd->worker->start(ssd->worker);
 } else {
-qemu_mutex_unlock_iothread();
 ssd->worker->stop(ssd->worker);
-qemu_mutex_lock_iothread();
 }
 ssd->running = running;
 }
-- 
1.7.1




[Qemu-devel] [PATCH 1/4] Make spice dummy functions inline to fix calls not checking return values

2011-05-03 Thread Gerd Hoffmann
From: Jes Sorensen 

qemu_spice_set_passwd() and qemu_spice_set_pw_expire() dummy functions
needs to be inline, in order to handle the case where they are called
without checking the return value.

Signed-off-by: Jes Sorensen 
Signed-off-by: Gerd Hoffmann 
---
 ui/qemu-spice.h |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index 916e5dc..3c6f1fe 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -47,8 +47,16 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts);
 #else  /* CONFIG_SPICE */
 
 #define using_spice 0
-#define qemu_spice_set_passwd(_p, _f1, _f2) (-1)
-#define qemu_spice_set_pw_expire(_e) (-1)
+static inline int qemu_spice_set_passwd(const char *passwd,
+bool fail_if_connected,
+bool disconnect_if_connected)
+{
+return -1;
+}
+static inline int qemu_spice_set_pw_expire(time_t expires)
+{
+return -1;
+}
 static inline int qemu_spice_migrate_info(const char *h, int p, int t, const 
char *s)
 { return -1; }
 
-- 
1.7.1




[Qemu-devel] [PATCH 3/4] spice: don't call displaystate callbacks from spice server context.

2011-05-03 Thread Gerd Hoffmann
This patch moves the displaystate callback calls for setting the cursor
and the mouse pointer from spice server to qemu (iothread) context.
This allows us to simplify locking.

Signed-off-by: Gerd Hoffmann 
---
 hw/qxl-render.c|   25 -
 hw/qxl.c   |2 ++
 ui/spice-display.c |   12 
 ui/spice-display.h |3 +++
 4 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 58965e0..1316066 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -185,7 +185,6 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt 
*ext)
 QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
 QXLCursor *cursor;
 QEMUCursor *c;
-int x = -1, y = -1;
 
 if (!qxl->ssd.ds->mouse_set || !qxl->ssd.ds->cursor_define) {
 return;
@@ -198,8 +197,6 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt 
*ext)
 }
 switch (cmd->type) {
 case QXL_CURSOR_SET:
-x = cmd->u.set.position.x;
-y = cmd->u.set.position.y;
 cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id);
 if (cursor->chunk.data_size != cursor->data_size) {
 fprintf(stderr, "%s: multiple chunks\n", __FUNCTION__);
@@ -209,18 +206,20 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt 
*ext)
 if (c == NULL) {
 c = cursor_builtin_left_ptr();
 }
-qemu_mutex_lock_iothread();
-qxl->ssd.ds->cursor_define(c);
-qxl->ssd.ds->mouse_set(x, y, 1);
-qemu_mutex_unlock_iothread();
-cursor_put(c);
+qemu_mutex_lock(&qxl->ssd.lock);
+if (qxl->ssd.cursor) {
+cursor_put(qxl->ssd.cursor);
+}
+qxl->ssd.cursor = c;
+qxl->ssd.mouse_x = cmd->u.set.position.x;
+qxl->ssd.mouse_y = cmd->u.set.position.y;
+qemu_mutex_unlock(&qxl->ssd.lock);
 break;
 case QXL_CURSOR_MOVE:
-x = cmd->u.position.x;
-y = cmd->u.position.y;
-qemu_mutex_lock_iothread();
-qxl->ssd.ds->mouse_set(x, y, 1);
-qemu_mutex_unlock_iothread();
+qemu_mutex_lock(&qxl->ssd.lock);
+qxl->ssd.mouse_x = cmd->u.position.x;
+qxl->ssd.mouse_y = cmd->u.position.y;
+qemu_mutex_unlock(&qxl->ssd.lock);
 break;
 }
 }
diff --git a/hw/qxl.c b/hw/qxl.c
index bd250db..4dfddf0 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1309,6 +1309,8 @@ static int qxl_init_primary(PCIDevice *dev)
qxl_hw_screen_dump, qxl_hw_text_update, 
qxl);
 qxl->ssd.ds = vga->ds;
 qemu_mutex_init(&qxl->ssd.lock);
+qxl->ssd.mouse_x = -1;
+qxl->ssd.mouse_y = -1;
 qxl->ssd.bufsize = (16 * 1024 * 1024);
 qxl->ssd.buf = qemu_malloc(qxl->ssd.bufsize);
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index d56dcfc..8579bfd 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -254,6 +254,16 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 ssd->update = qemu_spice_create_update(ssd);
 ssd->notify++;
 }
+if (ssd->cursor) {
+ssd->ds->cursor_define(ssd->cursor);
+cursor_put(ssd->cursor);
+ssd->cursor = NULL;
+}
+if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
+ssd->ds->mouse_set(ssd->mouse_x, ssd->mouse_y, 1);
+ssd->mouse_x = -1;
+ssd->mouse_y = -1;
+}
 qemu_mutex_unlock(&ssd->lock);
 
 if (ssd->notify) {
@@ -409,6 +419,8 @@ void qemu_spice_display_init(DisplayState *ds)
 assert(sdpy.ds == NULL);
 sdpy.ds = ds;
 qemu_mutex_init(&sdpy.lock);
+sdpy.mouse_x = -1;
+sdpy.mouse_y = -1;
 sdpy.bufsize = (16 * 1024 * 1024);
 sdpy.buf = qemu_malloc(sdpy.bufsize);
 register_displaychangelistener(ds, &display_listener);
diff --git a/ui/spice-display.h b/ui/spice-display.h
index e0cc46e..2f95f68 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -20,6 +20,7 @@
 #include 
 
 #include "qemu-thread.h"
+#include "console.h"
 #include "pflib.h"
 
 #define NUM_MEMSLOTS 8
@@ -55,6 +56,8 @@ struct SimpleSpiceDisplay {
  */
 QemuMutex lock;
 SimpleSpiceUpdate *update;
+QEMUCursor *cursor;
+int mouse_x, mouse_y;
 };
 
 struct SimpleSpiceUpdate {
-- 
1.7.1




[Qemu-devel] [PATCH 2/4] spice: don't create updates in spice server context.

2011-05-03 Thread Gerd Hoffmann
This patch moves the creation of spice screen updates from the spice
server context to qemu iothread context (display refresh timer to be
exact).  This way we avoid accessing qemu internals (display surface)
from spice thread context which in turn allows us to simplify locking.

Signed-off-by: Gerd Hoffmann 
---
 hw/qxl.c   |   17 +++--
 ui/spice-display.c |   43 +++
 ui/spice-display.h |   21 -
 3 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index fe4212b..bd250db 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -343,18 +343,22 @@ static int interface_get_command(QXLInstance *sin, struct 
QXLCommandExt *ext)
 SimpleSpiceUpdate *update;
 QXLCommandRing *ring;
 QXLCommand *cmd;
-int notify;
+int notify, ret;
 
 switch (qxl->mode) {
 case QXL_MODE_VGA:
 dprint(qxl, 2, "%s: vga\n", __FUNCTION__);
-update = qemu_spice_create_update(&qxl->ssd);
-if (update == NULL) {
-return false;
+ret = false;
+qemu_mutex_lock(&qxl->ssd.lock);
+if (qxl->ssd.update != NULL) {
+update = qxl->ssd.update;
+qxl->ssd.update = NULL;
+*ext = update->ext;
+ret = true;
 }
-*ext = update->ext;
+qemu_mutex_unlock(&qxl->ssd.lock);
 qxl_log_command(qxl, "vga", ext);
-return true;
+return ret;
 case QXL_MODE_COMPAT:
 case QXL_MODE_NATIVE:
 case QXL_MODE_UNDEFINED:
@@ -1304,6 +1308,7 @@ static int qxl_init_primary(PCIDevice *dev)
 vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
qxl_hw_screen_dump, qxl_hw_text_update, 
qxl);
 qxl->ssd.ds = vga->ds;
+qemu_mutex_init(&qxl->ssd.lock);
 qxl->ssd.bufsize = (16 * 1024 * 1024);
 qxl->ssd.buf = qemu_malloc(qxl->ssd.bufsize);
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 020b423..d56dcfc 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -62,14 +62,7 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
 dest->right = MAX(dest->right, r->right);
 }
 
-/*
- * Called from spice server thread context (via interface_get_command).
- *
- * We must aquire the global qemu mutex here to make sure the
- * DisplayState (+DisplaySurface) we are accessing doesn't change
- * underneath us.
- */
-SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
 SimpleSpiceUpdate *update;
 QXLDrawable *drawable;
@@ -78,9 +71,7 @@ SimpleSpiceUpdate 
*qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 uint8_t *src, *dst;
 int by, bw, bh;
 
-qemu_mutex_lock_iothread();
 if (qemu_spice_rect_is_empty(&ssd->dirty)) {
-qemu_mutex_unlock_iothread();
 return NULL;
 };
 
@@ -141,7 +132,6 @@ SimpleSpiceUpdate 
*qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 cmd->data = (intptr_t)drawable;
 
 memset(&ssd->dirty, 0, sizeof(ssd->dirty));
-qemu_mutex_unlock_iothread();
 return update;
 }
 
@@ -241,6 +231,12 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
 qemu_pf_conv_put(ssd->conv);
 ssd->conv = NULL;
 
+qemu_mutex_lock(&ssd->lock);
+if (ssd->update != NULL) {
+qemu_spice_destroy_update(ssd, ssd->update);
+ssd->update = NULL;
+}
+qemu_mutex_unlock(&ssd->lock);
 qemu_spice_destroy_host_primary(ssd);
 qemu_spice_create_host_primary(ssd);
 
@@ -252,6 +248,14 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 {
 dprint(3, "%s:\n", __FUNCTION__);
 vga_hw_update();
+
+qemu_mutex_lock(&ssd->lock);
+if (ssd->update == NULL) {
+ssd->update = qemu_spice_create_update(ssd);
+ssd->notify++;
+}
+qemu_mutex_unlock(&ssd->lock);
+
 if (ssd->notify) {
 ssd->notify = 0;
 ssd->worker->wakeup(ssd->worker);
@@ -298,14 +302,20 @@ static int interface_get_command(QXLInstance *sin, struct 
QXLCommandExt *ext)
 {
 SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
 SimpleSpiceUpdate *update;
+int ret = false;
 
 dprint(3, "%s:\n", __FUNCTION__);
-update = qemu_spice_create_update(ssd);
-if (update == NULL) {
-return false;
+
+qemu_mutex_lock(&ssd->lock);
+if (ssd->update != NULL) {
+update = ssd->update;
+ssd->update = NULL;
+*ext = update->ext;
+ret = true;
 }
-*ext = update->ext;
-return true;
+qemu_mutex_unlock(&ssd->lock);
+
+return ret;
 }
 
 static int interface_req_cmd_notification(QXLInstance *sin)
@@ -398,6 +408,7 @@ void qemu_spice_display_init(DisplayState *ds)
 {
 assert(sdpy.ds == NULL);
 sdpy.ds = ds;
+qemu_mutex_init(&sdpy.lock);
 sdpy.bufsize = (16 * 1024 * 1024);
 sdpy.buf = qemu_malloc(sdpy.bufsize);
 register_displaychangelis

Re: [Qemu-devel] [RFC][PATCH] Import Linux headers for KVM and vhost

2011-05-03 Thread Christoph Hellwig
On Tue, May 03, 2011 at 04:05:37PM +0200, Jan Kiszka wrote:
> This helps reducing our build-time checks for feature support in the
> available Linux kernel headers. And it helps users that do not have
> sufficiently recent headers installed on their build machine.
> 
> Header upstate is triggered via 'make update-linux-headers', optionally
> specifying a kernel directory as source via 'LINUX='.

Why not make it a shell scripts?  It's not like it's part of the build
system.  I also think the source tree argument should be mandatory instead
of grabbing random headers.




Re: [Qemu-devel] [PATCH 3/3] ide: add TRIM support

2011-05-03 Thread Christoph Hellwig
On Tue, May 03, 2011 at 03:08:43PM +0200, Paolo Bonzini wrote:
> On 05/03/2011 02:06 PM, Christoph Hellwig wrote:
>> +case IDE_DMA_TRIM:
>> +m->aiocb = dma_bdrv_io(s->bs,&s->sg, sector_num,
>> +   ide_issue_trim,, pmac_ide_transfer_cb, s, 1);
>> +break;
>
> Not compile-tested?

pmac emulation indeed wasn't in my usual compile setup.  Removing the comma
makes pmac compile, but I still can't finish a "full" qemu build:

hch@rocky:~/work/qemu$ make
  CCarm-softmmu/gdbstub-xml.o
 cc1: warnings being treated as errors
 In file included from ../qemu-common.h:5:0,
  from gdbstub-xml.c:2:
../config-host.h:2:0: error: "CONFIG_QEMU_PREFIX" redefined
config.h:2:0: note: this is the location of the previous definition
In file included from gdbstub-xml.c:140:0:
config.h:2:0: error: "CONFIG_QEMU_PREFIX" redefined
../config-host.h:2:0: note: this is the location of the previous definition
make[1]: *** [gdbstub-xml.o] Error 1
make: *** [subdir-arm-softmmu] Error 2



Re: [Qemu-devel] Bug in virtio-9p when fstatting an fd referring to a file that no longer exists

2011-05-03 Thread Sassan Panahinejad
On 2 May 2011 16:47, Venkateswararao Jujjuri wrote:

>  On 04/28/2011 09:51 AM, Sassan Panahinejad wrote:
>
> This thread seems relevant:
> http://www.mail-archive.com/linux-fsdevel@vger.kernel.org/msg09159.html
> Unless things have changed, it looks like the problem is in the client
> kernel (although note that there isn't support in qemu, even if the client
> did send an fid associated with an open file!).
> Any thoughts on a workaround for this?
>
>
> Hrm, I don't see any workaround for this. May be we should add TFSTAT for
> dotl? or add a flag to
> TSTAT?
>

It's definitely a tricky problem. I can't think of how to go about dealing
with it.
Probably worth looking into how nfs deals with it (and also looking into
what happens when communicating between two native plan9 systems). I'll
start reading.

Thanks
Sassan


>
> Copying the v9fs.
>
> Thanks,
> JV
>
>
>
>
>
> Thanks
> Sassan
>
> On 28 April 2011 17:13, Sassan Panahinejad  wrote:
>
>> It should be possible for guest applications to fstat a file for which
>> they have a valid file descriptor, even if the file has been removed.
>> Demonstrated by the code sample below (fstat reports no such file or
>> directory).
>> Strangely it seems that reading from a file in this state works fine (and
>> when both are run, the server receives a different fid for each).
>> On any other filesystem, the code runs correctly. On our 9p filesystem it
>> fails.
>> Many applications (including bash) depend on this working correctly.
>> I will continue investigating, but any thoughts anyone has on the subject
>> would be appreciated.
>>
>>
>> Thanks
>> Sassan
>>
>>
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>>
>>
>> int main(void)
>> {
>> int ret;
>> struct stat statbuf;
>> int fd = open("test.txt", O_RDWR | O_CREAT, 0666);
>> if (fd < 0) {
>> printf("open failed: %m\n");
>> return 1;
>> }
>> ret = write(fd, "test1\n", 6);
>> if (ret < 0) {
>> printf("write1 failed: %m\n");
>> return 1;
>> }
>> ret = unlink("test.txt");
>> if (ret < 0) {
>> printf("unlink failed: %m\n");
>> return 1;
>> }
>> ret = fstat(fd, &statbuf);
>> if (ret < 0) {
>> printf("fstat failed: %m\n");
>> return 1;
>> }
>> return 0;
>> }
>>
>
>
>


Re: [Qemu-devel] [PATCH v2] monitor: add PPC BookE SPRs

2011-05-03 Thread Scott Wood
On Tue, 3 May 2011 13:02:40 +0200
Alexander Graf  wrote:

> 
> On 30.04.2011, at 00:10, Scott Wood wrote:
> 
> > Read them via KVM_GET_SREGS in kvm_arch_get_registers(),
> > and display them in "info registers".
> > 
> > Also get CR and PID from the existing KVM_GET_REGS.
> > 
> > Signed-off-by: Scott Wood 
> > ---
> > v2: fix a couple style oversights, and cache kvm caps as requested
> 
> In fact, I'll just squash this patch with yours, if that's ok for you:

OK.

> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index a4ec4bc..5a1b6cb 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -45,7 +45,9 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>  static int cap_interrupt_unset = false;
>  static int cap_interrupt_level = false;
>  static int cap_segstate;
> +#ifdef KVM_CAP_PPC_BOOKE_SREGS
>  static int cap_booke_sregs;
> +#endif
>  
>  /* XXX We have a race condition where we actually have a level triggered
>   * interrupt, but the infrastructure can't expose that yet, so the guest
> @@ -94,13 +96,9 @@ static int kvm_arch_sync_sregs(CPUState *cenv)
>  if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
>  return 0;
>  } else {
> -#ifdef KVM_CAP_PPC_SEGSTATE
>  if (!cap_segstate) {
>  return 0;
>  }
> -#else
> -return 0;
> -#endif

So there are no headers out there that we need to support building with
that don't have KVM_CAP_PPC_SEGSTATE?

-Scott




Re: [Qemu-devel] [RFC][PATCH] Import Linux headers for KVM and vhost

2011-05-03 Thread Jan Kiszka
On 2011-05-03 17:12, Christoph Hellwig wrote:
> On Tue, May 03, 2011 at 04:05:37PM +0200, Jan Kiszka wrote:
>> This helps reducing our build-time checks for feature support in the
>> available Linux kernel headers. And it helps users that do not have
>> sufficiently recent headers installed on their build machine.
>>
>> Header upstate is triggered via 'make update-linux-headers', optionally
>> specifying a kernel directory as source via 'LINUX='.
> 
> Why not make it a shell scripts?  It's not like it's part of the build
> system.  I also think the source tree argument should be mandatory instead
> of grabbing random headers.

Yes, makes sense. I just adopted the kvm-kmod style here which I'm used to.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [Bug 739785] Re: qemu-i386 user mode on ARMv5 host fails (bash: fork: Invalid argument)

2011-05-03 Thread Ricardo Padilha
> But there's a pretty good chance I'll get to it somewhere in the next
few months.

That would be amazing. I've got a Linkstation Live v2 and a Drobo FS
waiting eagerly for the possibility of running some x86 binaries.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/739785

Title:
  qemu-i386 user mode on ARMv5 host fails (bash: fork: Invalid argument)

Status in QEMU:
  New

Bug description:
  Good time of day everybody,

  I have been trying to make usermode qemu on ARM with plugapps
  (archlinux) with archlinux i386 chroot to work.

  1. I installed arch linux in a virtuabox and created a chroot for it with 
mkarchroot. Transferred it to my pogo plug into /i386/
  2. I comiled qemu-i386 static and put it into /i386/usr/bin/
  ./configure --static --disable-blobs --disable-system 
--target-list=i386-linux-user
  make

  3. I also compiled linux kernel 2.6.38 with CONFIG_BINFMT_MISC=y and 
installed it.
  uname -a
  Linux Plugbox 2.6.38 #4 PREEMPT Fri Mar 18 22:19:10 CDT 2011 armv5tel 
Feroceon 88FR131 rev 1 (v5l) Marvell SheevaPlug Reference Board GNU/Linux

  4. Added the following options into /etc/rc.local
  /sbin/modprobe binfmt_misc
  /bin/mount binfmt_misc -t binfmt_misc /proc/sys/fs/binfmt_misc
  echo 
':qemu-i386:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00:\xff\xff\xff\xff\xff\xfe\xfe\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfb\xff\xff\xff:/usr/bin/qemu-i386:'
 >/proc/sys/fs/binfmt_misc/register

  5. Also copied ld-linux.so.3 (actually ld-2.13.so because ld-
  linux.so.3 is a link to that file) from /lib/ to /i386/lib/

  6.Now i chroot into /i386 and I get this:
  [root@Plugbox i386]# chroot .
  [II aI hnve ao n@P /]# pacman -Suy
  bash: fork: Invalid argument

  7.I also downloaded linux-user-test-0.3 from qemu website and ran the test:
  [root@Plugbox linux-user-test-0.3]# make
  ./qemu-linux-user.sh
  [qemu-i386]
  ../qemu-0.14.0/i386-linux-user/qemu-i386 -L ./gnemul/qemu-i386 i386/ls -l 
dummyfile
  BUG IN DYNAMIC LINKER ld.so: dl-version.c: 210: _dl_check_map_versions: 
Assertion `needed != ((void *)0)' failed!
  make: *** [test] Error 127



Re: [Qemu-devel] [PATCH v2] monitor: add PPC BookE SPRs

2011-05-03 Thread Alexander Graf

On 03.05.2011, at 17:28, Scott Wood wrote:

> On Tue, 3 May 2011 13:02:40 +0200
> Alexander Graf  wrote:
> 
>> 
>> On 30.04.2011, at 00:10, Scott Wood wrote:
>> 
>>> Read them via KVM_GET_SREGS in kvm_arch_get_registers(),
>>> and display them in "info registers".
>>> 
>>> Also get CR and PID from the existing KVM_GET_REGS.
>>> 
>>> Signed-off-by: Scott Wood 
>>> ---
>>> v2: fix a couple style oversights, and cache kvm caps as requested
>> 
>> In fact, I'll just squash this patch with yours, if that's ok for you:
> 
> OK.
> 
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index a4ec4bc..5a1b6cb 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -45,7 +45,9 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = 
>> {
>> static int cap_interrupt_unset = false;
>> static int cap_interrupt_level = false;
>> static int cap_segstate;
>> +#ifdef KVM_CAP_PPC_BOOKE_SREGS
>> static int cap_booke_sregs;
>> +#endif
>> 
>> /* XXX We have a race condition where we actually have a level triggered
>>  * interrupt, but the infrastructure can't expose that yet, so the guest
>> @@ -94,13 +96,9 @@ static int kvm_arch_sync_sregs(CPUState *cenv)
>> if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
>> return 0;
>> } else {
>> -#ifdef KVM_CAP_PPC_SEGSTATE
>> if (!cap_segstate) {
>> return 0;
>> }
>> -#else
>> -return 0;
>> -#endif
> 
> So there are no headers out there that we need to support building with
> that don't have KVM_CAP_PPC_SEGSTATE?

We need to support the headers, but not be able to run there (for Book3S). I 
have another patch on top of yours, enabling building with old headers.
See "[Qemu-devel] [PATCH] kvm: ppc: detect old headers" for it :).

The change above is functionally identical. cap_segstate is false by default.


Alex




Re: [Qemu-devel] [RFC][PATCH] Import Linux headers for KVM and vhost

2011-05-03 Thread Arnd Bergmann
On Tuesday 03 May 2011, Jan Kiszka wrote:
> This helps reducing our build-time checks for feature support in the
> available Linux kernel headers. And it helps users that do not have
> sufficiently recent headers installed on their build machine.
> 
> Header upstate is triggered via 'make update-linux-headers', optionally
> specifying a kernel directory as source via 'LINUX='.
> 
> ---
> 
> I'm sure this is not yet perfect from technical POV (e.g. the patch to
> Makefile.target looks like a hack to me, better ideas welcome),
> therefore RFC and no SOB. But it should demonstrate the plan. The
> workflow for adding a new KVM feature support would be first a 'make
> update-linux-header LINUX=/my/local/linux' + git commit, and then the
> commit of the kvm, vhost, whatever changes.
> 
> Makefile|   17 +++
> Makefile.target |2 +-
> configure   |  127 ---
> 3 files changed, 36 insertions(+), 110 deletions(-)

Very nice!

The old way to poke into the kernel internals was really annoying and confusing.

Arnd



Re: [Qemu-devel] [PATCH] posix-aio-compat: Fix idle_threads counter

2011-05-03 Thread Stefan Hajnoczi
On Tue, May 3, 2011 at 2:26 PM, Kevin Wolf  wrote:
> A thread should only be counted as idle when it really is waiting for new
> requests. Without this patch, sometimes too few threads are started as busy
> threads are counted as idle.
>
> Not sure if it makes a difference in practice outside some artificial
> qemu-io/qemu-img tests, but I think the change makes sense in any case.
>
> Signed-off-by: Kevin Wolf 
> ---
>  posix-aio-compat.c |    6 ++
>  1 files changed, 2 insertions(+), 4 deletions(-)

I think the critical change here is that idle_threads is not being
incremented by spawn_thread().  This means that we will keep spawning
threads as new requests come in and until the first thread goes idle.

Previously you could imagine a scenario where we spawn a thread but
don't schedule it yet.  Then we immediately submit more I/O and since
idle_threads was incremented we don't spawn additional threads to
handle the requests.

Are these the cases you were thinking about?

I like your patch because it reduces the number of places where we
mess with idle_threads.  I'd move the increment/decrement out of the
loop since the mutex is held here anyway so no one will test
idle_threads between loop iterations, but it's up to you if you want
to send a v2 or not.

Stefan



[Qemu-devel] [PATCH 0/4] introduce cpu_physical_memory_map_fast

2011-05-03 Thread Paolo Bonzini
Paravirtualized devices (and also some real devices) can assume they
are going to access RAM.  For this reason, provide a fast-path
function with the following properties:

1) it will never allocate a bounce buffer

2) it can be used for read-modify-write operations

3) unlike qemu_get_ram_ptr, it is safe because it recognizes "short" blocks

Patches 3 and 4 use this function for virtio devices and the milkymist
GPU.  The latter is only compile-tested.

Another function checks if it is possible to split a contiguous physical
address range into multiple subranges, all of which use the fast path.
I will introduce later a use for this function.

Paolo Bonzini (4):
  exec: extract cpu_physical_memory_map_internal
  exec: introduce cpu_physical_memory_map_fast and
cpu_physical_memory_map_check
  virtio: use cpu_physical_memory_map_fast
  milkymist: use cpu_physical_memory_map_fast

 cpu-common.h|4 ++
 exec.c  |  108 +-
 hw/milkymist-tmu2.c |   39 ++
 hw/vhost.c  |   10 ++--
 hw/virtio.c |2 +-
 5 files changed, 111 insertions(+), 52 deletions(-)

-- 
1.7.4.4




[Qemu-devel] [PATCH 1/4] exec: extract cpu_physical_memory_map_internal

2011-05-03 Thread Paolo Bonzini
This function performs all the work on the fast path, and returns
enough information for the slow path to pick up the work.  This
will be used later by other functions that only do the fast path.

Signed-off-by: Paolo Bonzini 
---
 exec.c |   77 ---
 1 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/exec.c b/exec.c
index a718d74..9b2c9e4 100644
--- a/exec.c
+++ b/exec.c
@@ -3898,16 +3898,9 @@ static void cpu_notify_map_clients(void)
 }
 }
 
-/* Map a physical memory region into a host virtual address.
- * May map a subset of the requested range, given by and returned in *plen.
- * May return NULL if resources needed to perform the mapping are exhausted.
- * Use only for reads OR writes - not for read-modify-write operations.
- * Use cpu_register_map_client() to know when retrying the map operation is
- * likely to succeed.
- */
-void *cpu_physical_memory_map(target_phys_addr_t addr,
-  target_phys_addr_t *plen,
-  int is_write)
+static void *cpu_physical_memory_map_internal(target_phys_addr_t addr,
+  target_phys_addr_t *plen,
+  uintptr_t *pd)
 {
 target_phys_addr_t len = *plen;
 target_phys_addr_t done = 0;
@@ -3915,7 +3908,6 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
 uint8_t *ret = NULL;
 uint8_t *ptr;
 target_phys_addr_t page;
-unsigned long pd;
 PhysPageDesc *p;
 unsigned long addr1;
 
@@ -3926,26 +3918,16 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
 l = len;
 p = phys_page_find(page >> TARGET_PAGE_BITS);
 if (!p) {
-pd = IO_MEM_UNASSIGNED;
-} else {
-pd = p->phys_offset;
+*pd = IO_MEM_UNASSIGNED;
+break;
 }
 
-if ((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM) {
-if (done || bounce.buffer) {
-break;
-}
-bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE);
-bounce.addr = addr;
-bounce.len = l;
-if (!is_write) {
-cpu_physical_memory_read(addr, bounce.buffer, l);
-}
-ptr = bounce.buffer;
-} else {
-addr1 = (pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
-ptr = qemu_get_ram_ptr(addr1);
+*pd = p->phys_offset;
+if ((*pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM) {
+break;
 }
+addr1 = (*pd & TARGET_PAGE_MASK) + (addr & ~TARGET_PAGE_MASK);
+ptr = qemu_get_ram_ptr(addr1);
 if (!done) {
 ret = ptr;
 } else if (ret + done != ptr) {
@@ -3960,6 +3942,45 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
 return ret;
 }
 
+/* Map a physical memory region into a host virtual address.
+ * May map a subset of the requested range, given by and returned in *plen.
+ * May return NULL if resources needed to perform the mapping are exhausted.
+ * Use only for reads OR writes - not for read-modify-write operations.
+ * Use cpu_register_map_client() to know when retrying the map operation is
+ * likely to succeed.
+ */
+void *cpu_physical_memory_map(target_phys_addr_t addr,
+  target_phys_addr_t *plen,
+  int is_write)
+{
+target_phys_addr_t page;
+uintptr_t pd = IO_MEM_UNASSIGNED;
+void *ret;
+ret = cpu_physical_memory_map_internal(addr, plen, &pd);
+if (ret) {
+return ret;
+}
+
+assert((pd & ~TARGET_PAGE_MASK) != IO_MEM_RAM);
+if (pd == IO_MEM_UNASSIGNED) {
+return NULL;
+}
+if (bounce.buffer) {
+return NULL;
+}
+
+/* Read at most a page into the temporary buffer.  */
+page = addr & TARGET_PAGE_MASK;
+bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE);
+bounce.addr = addr;
+bounce.len = MIN(page + TARGET_PAGE_SIZE - addr, *plen);
+if (!is_write) {
+cpu_physical_memory_read(addr, bounce.buffer, bounce.len);
+}
+*plen = bounce.len;
+return bounce.buffer;
+}
+
 /* Unmaps a memory region previously mapped by cpu_physical_memory_map().
  * Will also mark the memory as dirty if is_write == 1.  access_len gives
  * the amount of memory that was actually read or written by the caller.
-- 
1.7.4.4





[Qemu-devel] [PATCH 3/4] virtio: use cpu_physical_memory_map_fast

2011-05-03 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hw/vhost.c  |   10 +-
 hw/virtio.c |2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/vhost.c b/hw/vhost.c
index 14b571d..763ee4c 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -283,7 +283,7 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
 continue;
 }
 l = vq->ring_size;
-p = cpu_physical_memory_map(vq->ring_phys, &l, 1);
+p = cpu_physical_memory_map_fast(vq->ring_phys, &l);
 if (!p || l != vq->ring_size) {
 fprintf(stderr, "Unable to map ring buffer for ring %d\n", i);
 return -ENOMEM;
@@ -476,21 +476,21 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
 
 s = l = virtio_queue_get_desc_size(vdev, idx);
 a = virtio_queue_get_desc_addr(vdev, idx);
-vq->desc = cpu_physical_memory_map(a, &l, 0);
+vq->desc = cpu_physical_memory_map_fast(a, &l);
 if (!vq->desc || l != s) {
 r = -ENOMEM;
 goto fail_alloc_desc;
 }
 s = l = virtio_queue_get_avail_size(vdev, idx);
 a = virtio_queue_get_avail_addr(vdev, idx);
-vq->avail = cpu_physical_memory_map(a, &l, 0);
+vq->avail = cpu_physical_memory_map_fast(a, &l);
 if (!vq->avail || l != s) {
 r = -ENOMEM;
 goto fail_alloc_avail;
 }
 vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
 vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
-vq->used = cpu_physical_memory_map(a, &l, 1);
+vq->used = cpu_physical_memory_map_fast(a, &l);
 if (!vq->used || l != s) {
 r = -ENOMEM;
 goto fail_alloc_used;
@@ -498,7 +498,7 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
 
 vq->ring_size = s = l = virtio_queue_get_ring_size(vdev, idx);
 vq->ring_phys = a = virtio_queue_get_ring_addr(vdev, idx);
-vq->ring = cpu_physical_memory_map(a, &l, 1);
+vq->ring = cpu_physical_memory_map_fast(a, &l);
 if (!vq->ring || l != s) {
 r = -ENOMEM;
 goto fail_alloc_ring;
diff --git a/hw/virtio.c b/hw/virtio.c
index 6e8814c..429646a 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -372,7 +372,7 @@ void virtqueue_map_sg(struct iovec *sg, target_phys_addr_t 
*addr,
 
 for (i = 0; i < num_sg; i++) {
 len = sg[i].iov_len;
-sg[i].iov_base = cpu_physical_memory_map(addr[i], &len, is_write);
+sg[i].iov_base = cpu_physical_memory_map_fast(addr[i], &len);
 if (sg[i].iov_base == NULL || len != sg[i].iov_len) {
 error_report("virtio: trying to map MMIO memory");
 exit(1);
-- 
1.7.4.4





[Qemu-devel] [PATCH 2/4] exec: introduce cpu_physical_memory_map_fast and cpu_physical_memory_map_check

2011-05-03 Thread Paolo Bonzini
Paravirtualized devices (and also some real devices) can assume they
are going to access RAM.  For this reason, provide a fast-path
function with the following properties:

1) it will never allocate a bounce buffer

2) it can be used for read-modify-write operations

3) unlike qemu_get_ram_ptr, it is safe because it recognizes "short" blocks

To use cpu_physical_memory_map_fast for RMW, just pass 1 to is_write
when unmapping.

Signed-off-by: Paolo Bonzini 
---
 cpu-common.h |4 
 exec.c   |   31 +++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/cpu-common.h b/cpu-common.h
index 96c02ae..d6e116d 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -80,6 +80,10 @@ static inline void 
cpu_physical_memory_write(target_phys_addr_t addr,
 void *cpu_physical_memory_map(target_phys_addr_t addr,
   target_phys_addr_t *plen,
   int is_write);
+void *cpu_physical_memory_map_fast(target_phys_addr_t addr,
+   target_phys_addr_t *plen);
+bool cpu_physical_memory_map_check(target_phys_addr_t addr,
+   target_phys_addr_t len);
 void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
int is_write, target_phys_addr_t access_len);
 void *cpu_register_map_client(void *opaque, void (*callback)(void *opaque));
diff --git a/exec.c b/exec.c
index 9b2c9e4..2b88c29 100644
--- a/exec.c
+++ b/exec.c
@@ -3944,6 +3944,19 @@ static void 
*cpu_physical_memory_map_internal(target_phys_addr_t addr,
 
 /* Map a physical memory region into a host virtual address.
  * May map a subset of the requested range, given by and returned in *plen.
+ * May return NULL if extra resources are needed to perform the mapping
+ * (i.e. cpu_physical_memory_map is needed).
+ * It may be used for read-modify-write operations.
+ */
+void *cpu_physical_memory_map_fast(target_phys_addr_t addr,
+   target_phys_addr_t *plen)
+{
+uintptr_t pd;
+return cpu_physical_memory_map_internal(addr, plen, &pd);
+}
+
+/* Map a physical memory region into a host virtual address.
+ * May map a subset of the requested range, given by and returned in *plen.
  * May return NULL if resources needed to perform the mapping are exhausted.
  * Use only for reads OR writes - not for read-modify-write operations.
  * Use cpu_register_map_client() to know when retrying the map operation is
@@ -3981,6 +3994,24 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
 return bounce.buffer;
 }
 
+/* Returns true if the entire area between ADDR and ADDR+LEN (inclusive
+ * and exclusive, respectively) can be mapped by cpu_physical_memory_map_fast
+ * (possibly in multiple steps).
+ */
+bool cpu_physical_memory_map_check(target_phys_addr_t addr,
+   target_phys_addr_t len)
+{
+while (len > 0) {
+target_phys_addr_t l = len;
+if (!cpu_physical_memory_map_fast(addr, &l)) {
+return false;
+}
+len -= l;
+addr += l;
+}
+return true;
+}
+
 /* Unmaps a memory region previously mapped by cpu_physical_memory_map().
  * Will also mark the memory as dirty if is_write == 1.  access_len gives
  * the amount of memory that was actually read or written by the caller.
-- 
1.7.4.4





[Qemu-devel] [PATCH 4/4] milkymist: use cpu_physical_memory_map_fast

2011-05-03 Thread Paolo Bonzini
This patch adds proper unmapping of the memory when the addresses
cross multiple memory blocks, and it also uses a single map_fast
operation for the RMW access to the destination frame buffer.

Cc: Michael Walle 
Signed-off-by: Paolo Bonzini 
---
Compile-tested only.

 hw/milkymist-tmu2.c |   39 +--
 1 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/hw/milkymist-tmu2.c b/hw/milkymist-tmu2.c
index 9cebe31..575fa41 100644
--- a/hw/milkymist-tmu2.c
+++ b/hw/milkymist-tmu2.c
@@ -181,9 +181,9 @@ static void tmu2_start(MilkymistTMU2State *s)
 GLXPbuffer pbuffer;
 GLuint texture;
 void *fb;
-target_phys_addr_t fb_len;
+target_phys_addr_t fb_len, fb_len_full;
 void *mesh;
-target_phys_addr_t mesh_len;
+target_phys_addr_t mesh_len, mesh_len_full;
 float m;
 
 trace_milkymist_tmu2_start();
@@ -205,8 +205,12 @@ static void tmu2_start(MilkymistTMU2State *s)
 /* Read the QEMU source framebuffer into an OpenGL texture */
 glGenTextures(1, &texture);
 glBindTexture(GL_TEXTURE_2D, texture);
-fb_len = 2*s->regs[R_TEXHRES]*s->regs[R_TEXVRES];
-fb = cpu_physical_memory_map(s->regs[R_TEXFBUF], &fb_len, 0);
+fb_len_full = fb_len = 2*s->regs[R_TEXHRES]*s->regs[R_TEXVRES];
+fb = cpu_physical_memory_map_fast(s->regs[R_TEXFBUF], &fb_len);
+if (fb_len < fb_len_full) {
+cpu_physical_memory_unmap(fb, fb_len, 0, 0);
+fb = NULL;
+}
 if (fb == NULL) {
 glDeleteTextures(1, &texture);
 glXMakeContextCurrent(s->dpy, None, None, NULL);
@@ -249,8 +253,12 @@ static void tmu2_start(MilkymistTMU2State *s)
 glColor4f(m, m, m, (float)(s->regs[R_ALPHA] + 1) / 64.0f);
 
 /* Read the QEMU dest. framebuffer into the OpenGL framebuffer */
-fb_len = 2 * s->regs[R_DSTHRES] * s->regs[R_DSTVRES];
-fb = cpu_physical_memory_map(s->regs[R_DSTFBUF], &fb_len, 0);
+fb_len_full = fb_len = 2 * s->regs[R_DSTHRES] * s->regs[R_DSTVRES];
+fb = cpu_physical_memory_map_fast(s->regs[R_DSTFBUF], &fb_len);
+if (fb_len < fb_len_full) {
+cpu_physical_memory_unmap(fb, fb_len, 0, 0);
+fb = NULL;
+}
 if (fb == NULL) {
 glDeleteTextures(1, &texture);
 glXMakeContextCurrent(s->dpy, None, None, NULL);
@@ -260,7 +268,6 @@ static void tmu2_start(MilkymistTMU2State *s)
 
 glDrawPixels(s->regs[R_DSTHRES], s->regs[R_DSTVRES], GL_RGB,
 GL_UNSIGNED_SHORT_5_6_5, fb);
-cpu_physical_memory_unmap(fb, fb_len, 0, fb_len);
 glViewport(0, 0, s->regs[R_DSTHRES], s->regs[R_DSTVRES]);
 glMatrixMode(GL_PROJECTION);
 glLoadIdentity();
@@ -268,9 +275,14 @@ static void tmu2_start(MilkymistTMU2State *s)
 glMatrixMode(GL_MODELVIEW);
 
 /* Map the texture */
-mesh_len = MESH_MAXSIZE*MESH_MAXSIZE*sizeof(struct vertex);
-mesh = cpu_physical_memory_map(s->regs[R_VERTICESADDR], &mesh_len, 0);
+mesh_len_full = mesh_len = MESH_MAXSIZE*MESH_MAXSIZE*sizeof(struct vertex);
+mesh = cpu_physical_memory_map_fast(s->regs[R_VERTICESADDR], &mesh_len);
+if (mesh_len < mesh_len_full) {
+cpu_physical_memory_unmap(mesh, mesh_len, 0, 0);
+mesh = NULL;
+}
 if (mesh == NULL) {
+cpu_physical_memory_unmap(fb, fb_len, 0, fb_len);
 glDeleteTextures(1, &texture);
 glXMakeContextCurrent(s->dpy, None, None, NULL);
 glXDestroyPbuffer(s->dpy, pbuffer);
@@ -285,15 +297,6 @@ static void tmu2_start(MilkymistTMU2State *s)
 cpu_physical_memory_unmap(mesh, mesh_len, 0, mesh_len);
 
 /* Write back the OpenGL framebuffer to the QEMU framebuffer */
-fb_len = 2 * s->regs[R_DSTHRES] * s->regs[R_DSTVRES];
-fb = cpu_physical_memory_map(s->regs[R_DSTFBUF], &fb_len, 1);
-if (fb == NULL) {
-glDeleteTextures(1, &texture);
-glXMakeContextCurrent(s->dpy, None, None, NULL);
-glXDestroyPbuffer(s->dpy, pbuffer);
-return;
-}
-
 glReadPixels(0, 0, s->regs[R_DSTHRES], s->regs[R_DSTVRES], GL_RGB,
 GL_UNSIGNED_SHORT_5_6_5, fb);
 cpu_physical_memory_unmap(fb, fb_len, 1, fb_len);
-- 
1.7.4.4




[Qemu-devel] [PATCH 01/20] scsi: add tracing of scsi requests

2011-05-03 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hw/scsi-bus.c |6 ++
 trace-events  |6 ++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ceeb4ec..0fd85fc 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -4,6 +4,7 @@
 #include "scsi-defs.h"
 #include "qdev.h"
 #include "blockdev.h"
+#include "trace.h"
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
 
@@ -141,6 +142,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, 
uint32_t tag, uint32_t l
 req->lun = lun;
 req->status = -1;
 req->enqueued = true;
+trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
 QTAILQ_INSERT_TAIL(&d->requests, req, next);
 return req;
 }
@@ -159,6 +161,7 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
 
 static void scsi_req_dequeue(SCSIRequest *req)
 {
+trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag);
 if (req->enqueued) {
 QTAILQ_REMOVE(&req->dev->requests, req, next);
 req->enqueued = false;
@@ -195,6 +198,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
 req->cmd.len = 12;
 break;
 default:
+trace_scsi_req_parse_bad(req->dev->id, req->lun, req->tag, cmd[0]);
 return -1;
 }
 
@@ -392,6 +396,8 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
 memcpy(req->cmd.buf, buf, req->cmd.len);
 scsi_req_xfer_mode(req);
 req->cmd.lba = scsi_req_lba(req);
+trace_scsi_req_parsed(req->dev->id, req->lun, req->tag, buf[0],
+  req->cmd.mode, req->cmd.xfer, req->cmd.lba);
 return 0;
 }
 
diff --git a/trace-events b/trace-events
index 4f965e2..4e34f66 100644
--- a/trace-events
+++ b/trace-events
@@ -205,6 +205,12 @@ disable usb_set_config(int addr, int config, int ret) "dev 
%d, config %d, ret %d
 disable usb_clear_device_feature(int addr, int feature, int ret) "dev %d, 
feature %d, ret %d"
 disable usb_set_device_feature(int addr, int feature, int ret) "dev %d, 
feature %d, ret %d"
 
+# hw/scsi-bus.c
+disable scsi_req_alloc(int target, int lun, int tag) "target %d lun %d tag %d"
+disable scsi_req_dequeue(int target, int lun, int tag) "target %d lun %d tag 
%d"
+disable scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int 
xfer, uint64_t lba) "target %d lun %d tag %d command %d dir %d length %d lba 
%"PRIu64""
+disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d 
lun %d tag %d command %d"
+
 # vl.c
 disable vm_state_notify(int running, int reason) "running %d reason %d"
 
-- 
1.7.4.4





[Qemu-devel] [PATCH 00/20] SCSI subsystem improvements

2011-05-03 Thread Paolo Bonzini
This series includes the following improvements to the SCSI subsystem:

1) introduction of SCSIBusOps that generalize the existing
command_complete callback;

2) widespread use of the SCSIRequest abstraction, with simpler memory
management (refcounting) and with various common idioms converted into
simple C functions instead of duplicating them all over the place;

3) support for autosense.

Some patches are from Hannes Reinecke's megasas patchset posted last
November, forward ported and applied to the new vSCSI controller as
well.

Testing:
- RHEL6.1 install complete to scsi-disk with lsi, from scsi-generic CD
- iozone run with lsi on scsi-disk target
- RHEL6.1 install to usb-msd from IDE CD hangs (probably) when formatting
  the USB disk, gets I/O errors even earlier without the series
- RHEL6.1 install started with vscsi, from scsi-generic CD including
  playing with opening/closing the tray (to exercise autosense), plan to do
  a complete test later if no one beats me to it

esp is only compile tested.

Hannes Reinecke (4):
  scsi: Use 'SCSIRequest' directly
  scsi: Update sense code handling
  scsi: Implement 'get_sense' callback
  scsi-disk: add data direction checking

Paolo Bonzini (16):
  scsi: add tracing of scsi requests
  scsi-generic: Remove bogus double complete
  scsi: introduce scsi_req_data
  scsi: introduce SCSIBusOps
  scsi: reference-count requests
  lsi: extract lsi_find_by_tag
  scsi: commonize purging requests
  scsi: introduce scsi_req_abort
  scsi: introduce scsi_req_cancel
  scsi: use scsi_req_complete
  scsi: do not call send_command directly
  scsi: introduce scsi_req_new
  scsi: introduce scsi_req_kick
  scsi: introduce scsi_req_get_buf
  scsi: make write_data return void
  scsi-generic: Handle queue full

 hw/esp.c  |   61 -
 hw/lsi53c895a.c   |  158 
 hw/scsi-bus.c |  203 +---
 hw/scsi-disk.c|  264 ++---
 hw/scsi-generic.c |  218 +---
 hw/scsi.h |   84 ++---
 hw/spapr_vscsi.c  |   91 +++
 hw/usb-msd.c  |   55 +++-
 trace-events  |8 ++
 9 files changed, 697 insertions(+), 445 deletions(-)

-- 
1.7.4.4




[Qemu-devel] [PATCH 02/20] scsi-generic: Remove bogus double complete

2011-05-03 Thread Paolo Bonzini
scsi-generic scsi_read_complete() should not -both- call the client
complete callback with SCSI_REASON_DATA -and- call
scsi_command_complete().  The former will cause the client to queue a
new read or write request, while the later will free the request data
structure, thus causing the new read or write request to use a
freed/stale structure when it completes.

This patch fixes the bug, fixing a crash with scsi-generic & RHEL5.5
installer.

Cc: Benjamin Herrenschmidt 
Cc: David Gibson 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi-generic.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 9be1cca..102f1da 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -172,9 +172,11 @@ static void scsi_read_complete(void * opaque, int ret)
 DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, len);
 
 r->len = -1;
-r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
-if (len == 0)
+if (len == 0) {
 scsi_command_complete(r, 0);
+} else {
+r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
+}
 }
 
 /* Read more data from scsi device into buffer.  */
-- 
1.7.4.4





[Qemu-devel] [PATCH 06/20] lsi: extract lsi_find_by_tag

2011-05-03 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hw/lsi53c895a.c |   63 +-
 1 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 56329c2..fc21158 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -652,38 +652,51 @@ static void lsi_reselect(LSIState *s, lsi_request *p)
 }
 }
 
-/* Record that data is available for a queued command.  Returns zero if
-   the device was reselected, nonzero if the IO is deferred.  */
-static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
+static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t tag)
 {
 lsi_request *p;
 
 QTAILQ_FOREACH(p, &s->queue, next) {
 if (p->tag == tag) {
-if (p->pending) {
-BADF("Multiple IO pending for tag %d\n", tag);
-}
-p->pending = arg;
-/* Reselect if waiting for it, or if reselection triggers an IRQ
-   and the bus is free.
-   Since no interrupt stacking is implemented in the emulation, it
-   is also required that there are no pending interrupts waiting
-   for service from the device driver. */
-if (s->waiting == 1 ||
-(lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON) &&
- !(s->istat0 & (LSI_ISTAT0_SIP | LSI_ISTAT0_DIP {
-/* Reselect device.  */
-lsi_reselect(s, p);
-return 0;
-} else {
-DPRINTF("Queueing IO tag=0x%x\n", tag);
-p->pending = arg;
-return 1;
-}
+return p;
 }
 }
-BADF("IO with unknown tag %d\n", tag);
-return 1;
+
+return NULL;
+}
+
+/* Record that data is available for a queued command.  Returns zero if
+   the device was reselected, nonzero if the IO is deferred.  */
+static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
+{
+lsi_request *p;
+
+p = lsi_find_by_tag(s, tag);
+if (!p) {
+BADF("IO with unknown tag %d\n", tag);
+return 1;
+}
+
+if (p->pending) {
+BADF("Multiple IO pending for tag %d\n", tag);
+}
+p->pending = arg;
+/* Reselect if waiting for it, or if reselection triggers an IRQ
+   and the bus is free.
+   Since no interrupt stacking is implemented in the emulation, it
+   is also required that there are no pending interrupts waiting
+   for service from the device driver. */
+if (s->waiting == 1 ||
+(lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON) &&
+ !(s->istat0 & (LSI_ISTAT0_SIP | LSI_ISTAT0_DIP {
+/* Reselect device.  */
+lsi_reselect(s, p);
+return 0;
+} else {
+DPRINTF("Queueing IO tag=0x%x\n", tag);
+p->pending = arg;
+return 1;
+}
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
-- 
1.7.4.4





[Qemu-devel] [PATCH 08/20] scsi: commonize purging requests

2011-05-03 Thread Paolo Bonzini
The code for canceling requests upon reset is already the same.  Clean
it up and move it to scsi-bus.c.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-bus.c |   12 
 hw/scsi-disk.c|   18 ++
 hw/scsi-generic.c |   18 ++
 hw/scsi.h |1 +
 4 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index a175590..e1bb494 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -548,6 +548,18 @@ void scsi_req_complete(SCSIRequest *req)
 scsi_req_unref(req);
 }
 
+void scsi_device_purge_requests(SCSIDevice *sdev)
+{
+SCSIRequest *req;
+
+while (!QTAILQ_EMPTY(&sdev->requests)) {
+req = QTAILQ_FIRST(&sdev->requests);
+sdev->info->cancel_io(req);
+scsi_req_dequeue(req);
+scsi_req_unref(req);
+}
+}
+
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
 {
 SCSIDevice *d = (SCSIDevice*)dev;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 7446115..8962c33 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1145,26 +1145,12 @@ static int32_t scsi_send_command(SCSIRequest *req, 
uint8_t *buf)
 return len;
 }
 
-static void scsi_disk_purge_requests(SCSIDiskState *s)
-{
-SCSIDiskReq *r;
-
-while (!QTAILQ_EMPTY(&s->qdev.requests)) {
-r = DO_UPCAST(SCSIDiskReq, req, QTAILQ_FIRST(&s->qdev.requests));
-if (r->req.aiocb) {
-bdrv_aio_cancel(r->req.aiocb);
-}
-scsi_req_dequeue(&r->req);
-scsi_req_unref(&r->req);
-}
-}
-
 static void scsi_disk_reset(DeviceState *dev)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev.qdev, dev);
 uint64_t nb_sectors;
 
-scsi_disk_purge_requests(s);
+scsi_device_purge_requests(&s->qdev);
 
 bdrv_get_geometry(s->bs, &nb_sectors);
 nb_sectors /= s->cluster_size;
@@ -1178,7 +1164,7 @@ static void scsi_destroy(SCSIDevice *dev)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
 
-scsi_disk_purge_requests(s);
+scsi_device_purge_requests(&s->qdev);
 blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e3e4187..896797c 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -422,32 +422,18 @@ static int get_stream_blocksize(BlockDriverState *bdrv)
 return (buf[9] << 16) | (buf[10] << 8) | buf[11];
 }
 
-static void scsi_generic_purge_requests(SCSIGenericState *s)
-{
-SCSIGenericReq *r;
-
-while (!QTAILQ_EMPTY(&s->qdev.requests)) {
-r = DO_UPCAST(SCSIGenericReq, req, QTAILQ_FIRST(&s->qdev.requests));
-if (r->req.aiocb) {
-bdrv_aio_cancel(r->req.aiocb);
-}
-scsi_req_dequeue(&r->req);
-scsi_req_unref(&r->req);
-}
-}
-
 static void scsi_generic_reset(DeviceState *dev)
 {
 SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev.qdev, dev);
 
-scsi_generic_purge_requests(s);
+scsi_device_purge_requests(&s->qdev);
 }
 
 static void scsi_destroy(SCSIDevice *d)
 {
 SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, d);
 
-scsi_generic_purge_requests(s);
+scsi_device_purge_requests(&s->qdev);
 blockdev_mark_auto_del(s->qdev.conf.bs);
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index e709989..dee8567 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -114,5 +114,6 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
+void scsi_device_purge_requests(SCSIDevice *sdev);
 
 #endif
-- 
1.7.4.4





[Qemu-devel] [PATCH 04/20] scsi: introduce SCSIBusOps

2011-05-03 Thread Paolo Bonzini
There are more operations than a SCSI bus can handle, besides completing
commands.  One example, which this series will introduce, is cleaning up
after a request is cancelled.

More long term, a "SCSI bus" can represent the LUNs attached to a
target; in this case, while all commands will ultimately reach a logical
unit, it is the target who is in charge of answering REPORT LUNs.

Signed-off-by: Paolo Bonzini 
---
 hw/esp.c  |6 +-
 hw/lsi53c895a.c   |6 +-
 hw/scsi-bus.c |   12 ++--
 hw/scsi-generic.c |2 +-
 hw/scsi.h |   13 +++--
 hw/spapr_vscsi.c  |6 +-
 hw/usb-msd.c  |6 +-
 7 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index fa9d2a2..d8bba7a 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -714,6 +714,10 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
 *dma_enable = qdev_get_gpio_in(dev, 1);
 }
 
+static struct SCSIBusOps esp_scsi_ops = {
+.complete = esp_command_complete
+};
+
 static int esp_init1(SysBusDevice *dev)
 {
 ESPState *s = FROM_SYSBUS(ESPState, dev);
@@ -728,7 +732,7 @@ static int esp_init1(SysBusDevice *dev)
 
 qdev_init_gpio_in(&dev->qdev, esp_gpio_demux, 2);
 
-scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, esp_command_complete);
+scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, &esp_scsi_ops);
 return scsi_bus_legacy_handle_cmdline(&s->bus);
 }
 
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index e4b51a8..56329c2 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -2214,6 +2214,10 @@ static int lsi_scsi_uninit(PCIDevice *d)
 return 0;
 }
 
+static struct SCSIBusOps lsi_scsi_ops = {
+.complete = lsi_command_complete
+};
+
 static int lsi_scsi_init(PCIDevice *dev)
 {
 LSIState *s = DO_UPCAST(LSIState, dev, dev);
@@ -2251,7 +2255,7 @@ static int lsi_scsi_init(PCIDevice *dev)
PCI_BASE_ADDRESS_SPACE_MEMORY, lsi_ram_mapfunc);
 QTAILQ_INIT(&s->queue);
 
-scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
+scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, &lsi_scsi_ops);
 if (!dev->qdev.hotplugged) {
 return scsi_bus_legacy_handle_cmdline(&s->bus);
 }
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 0afe3fb..63d9a68 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -21,13 +21,13 @@ static int next_scsi_bus;
 
 /* Create a scsi bus, and attach devices to it.  */
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
-  scsi_completionfn complete)
+  SCSIBusOps *ops)
 {
 qbus_create_inplace(&bus->qbus, &scsi_bus_info, host, NULL);
 bus->busnr = next_scsi_bus++;
 bus->tcq = tcq;
 bus->ndev = ndev;
-bus->complete = complete;
+bus->ops = *ops;
 bus->qbus.allow_hotplug = 1;
 }
 
@@ -498,7 +498,7 @@ static const char *scsi_command_name(uint8_t cmd)
 void scsi_req_data(SCSIRequest *req, int len)
 {
 trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
-req->bus->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
+req->bus->ops.complete(req->bus, SCSI_REASON_DATA, req->tag, len);
 }
 
 void scsi_req_print(SCSIRequest *req)
@@ -533,9 +533,9 @@ void scsi_req_complete(SCSIRequest *req)
 {
 assert(req->status != -1);
 scsi_req_dequeue(req);
-req->bus->complete(req->bus, SCSI_REASON_DONE,
-   req->tag,
-   req->status);
+req->bus->ops.complete(req->bus, SCSI_REASON_DONE,
+   req->tag,
+   req->status);
 }
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e4f1f30..3db734a 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -335,7 +335,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t 
tag,
 s->senselen = 7;
 s->driver_status = SG_ERR_DRIVER_SENSE;
 bus = scsi_bus_from_device(d);
-bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
+bus->ops.complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
 return 0;
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index 7c09f32..d1753f9 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -16,10 +16,9 @@ enum scsi_reason {
 };
 
 typedef struct SCSIBus SCSIBus;
+typedef struct SCSIBusOps SCSIBusOps;
 typedef struct SCSIDevice SCSIDevice;
 typedef struct SCSIDeviceInfo SCSIDeviceInfo;
-typedef void (*scsi_completionfn)(SCSIBus *bus, int reason, uint32_t tag,
-  uint32_t arg);
 
 enum SCSIXferMode {
 SCSI_XFER_NONE,  /*  TEST_UNIT_READY, ...*/
@@ -74,20 +73,22 @@ struct SCSIDeviceInfo {
 uint8_t *(*get_buf)(SCSIDevice *s, uint32_t tag);
 };
 
-typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
-  int unit);
+struct SCSIBusOps {
+void (*complete)(SCSIBus *bus, int reason, uint32_t tag, uint32_t arg);
+};
+
 struct SCSIBus {

[Qemu-devel] [PATCH 03/20] scsi: introduce scsi_req_data

2011-05-03 Thread Paolo Bonzini
This abstracts calling the command_complete callback, reducing churn
in the following patches.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-bus.c |6 ++
 hw/scsi-disk.c|8 
 hw/scsi-generic.c |6 +++---
 hw/scsi.h |1 +
 trace-events  |1 +
 5 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 0fd85fc..0afe3fb 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -495,6 +495,12 @@ static const char *scsi_command_name(uint8_t cmd)
 return names[cmd];
 }
 
+void scsi_req_data(SCSIRequest *req, int len)
+{
+trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
+req->bus->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
+}
+
 void scsi_req_print(SCSIRequest *req)
 {
 FILE *fp = stderr;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b05e654..2b5dc2a 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -167,7 +167,7 @@ static void scsi_read_complete(void * opaque, int ret)
 n = r->iov.iov_len / 512;
 r->sector += n;
 r->sector_count -= n;
-r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, 
r->iov.iov_len);
+scsi_req_data(&r->req, r->iov.iov_len);
 }
 
 
@@ -179,7 +179,7 @@ static void scsi_read_request(SCSIDiskReq *r)
 if (r->sector_count == (uint32_t)-1) {
 DPRINTF("Read buf_len=%zd\n", r->iov.iov_len);
 r->sector_count = 0;
-r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, 
r->iov.iov_len);
+scsi_req_data(&r->req, r->iov.iov_len);
 return;
 }
 DPRINTF("Read sector_count=%d\n", r->sector_count);
@@ -242,7 +242,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, 
int type)
 vm_stop(VMSTOP_DISKFULL);
 } else {
 if (type == SCSI_REQ_STATUS_RETRY_READ) {
-r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, 0);
+scsi_req_data(&r->req, 0);
 }
 scsi_command_complete(r, CHECK_CONDITION,
 HARDWARE_ERROR);
@@ -278,7 +278,7 @@ static void scsi_write_complete(void * opaque, int ret)
 }
 r->iov.iov_len = len;
 DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, len);
-r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
+scsi_req_data(&r->req, len);
 }
 }
 
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 102f1da..e4f1f30 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -175,7 +175,7 @@ static void scsi_read_complete(void * opaque, int ret)
 if (len == 0) {
 scsi_command_complete(r, 0);
 } else {
-r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len);
+scsi_req_data(&r->req, len);
 }
 }
 
@@ -212,7 +212,7 @@ static void scsi_read_data(SCSIDevice *d, uint32_t tag)
 DPRINTF("Sense: %d %d %d %d %d %d %d %d\n",
 r->buf[0], r->buf[1], r->buf[2], r->buf[3],
 r->buf[4], r->buf[5], r->buf[6], r->buf[7]);
-r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, 
s->senselen);
+scsi_req_data(&r->req, s->senselen);
 return;
 }
 
@@ -263,7 +263,7 @@ static int scsi_write_data(SCSIDevice *d, uint32_t tag)
 
 if (r->len == 0) {
 r->len = r->buflen;
-r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, r->len);
+scsi_req_data(&r->req, r->len);
 return 0;
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index d3b5d56..7c09f32 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -105,6 +105,7 @@ void scsi_req_free(SCSIRequest *req);
 
 int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
+void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
 
 #endif
diff --git a/trace-events b/trace-events
index 4e34f66..564c97a 100644
--- a/trace-events
+++ b/trace-events
@@ -207,6 +207,7 @@ disable usb_set_device_feature(int addr, int feature, int 
ret) "dev %d, feature
 
 # hw/scsi-bus.c
 disable scsi_req_alloc(int target, int lun, int tag) "target %d lun %d tag %d"
+disable scsi_req_data(int target, int lun, int tag, int len) "target %d lun %d 
tag %d len %d"
 disable scsi_req_dequeue(int target, int lun, int tag) "target %d lun %d tag 
%d"
 disable scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int 
xfer, uint64_t lba) "target %d lun %d tag %d command %d dir %d length %d lba 
%"PRIu64""
 disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) "target %d 
lun %d tag %d command %d"
-- 
1.7.4.4





[Qemu-devel] [PATCH 09/20] scsi: introduce scsi_req_abort

2011-05-03 Thread Paolo Bonzini
This covers the case of canceling a request's I/O and still
completing it.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-bus.c|9 +
 hw/scsi.h|1 +
 hw/spapr_vscsi.c |8 ++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e1bb494..1d7da9e 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -548,6 +548,15 @@ void scsi_req_complete(SCSIRequest *req)
 scsi_req_unref(req);
 }
 
+void scsi_req_abort(SCSIRequest *req, int status)
+{
+req->status = status;
+if (req->dev && req->dev->info->cancel_io) {
+req->dev->info->cancel_io(req);
+}
+scsi_req_complete(req);
+}
+
 void scsi_device_purge_requests(SCSIDevice *sdev)
 {
 SCSIRequest *req;
diff --git a/hw/scsi.h b/hw/scsi.h
index dee8567..6221fff 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -114,6 +114,7 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
+void scsi_req_abort(SCSIRequest *req, int status);
 void scsi_device_purge_requests(SCSIDevice *sdev);
 
 #endif
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index cf2ed73..678cd00 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -463,10 +463,8 @@ static void vscsi_send_request_sense(VSCSIState *s, 
vscsi_req *req)
 dprintf("VSCSI: Queued request sense tag 0x%x\n", req->qtag);
 if (n < 0) {
 fprintf(stderr, "VSCSI: REQUEST_SENSE wants write data !?!?!?\n");
-sdev->info->cancel_io(req->sreq);
 vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
-vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-vscsi_put_req(s, req);
+scsi_req_abort(req->sreq, CHECK_CONDITION);
 return;
 } else if (n == 0) {
 return;
@@ -547,10 +545,8 @@ static void vscsi_command_complete(SCSIRequest *sreq, int 
reason, uint32_t arg)
 }
 if (rc < 0) {
 fprintf(stderr, "VSCSI: RDMA error rc=%d!\n", rc);
-sdev->info->cancel_io(sreq);
 vscsi_makeup_sense(s, req, HARDWARE_ERROR, 0, 0);
-vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-vscsi_put_req(s, req);
+scsi_req_abort(req->sreq, CHECK_CONDITION);
 return;
 }
 
-- 
1.7.4.4





[Qemu-devel] [PATCH 14/20] scsi: introduce scsi_req_new

2011-05-03 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hw/esp.c |2 +-
 hw/lsi53c895a.c  |3 +--
 hw/scsi-bus.c|5 +
 hw/scsi.h|1 +
 hw/spapr_vscsi.c |2 +-
 hw/usb-msd.c |2 +-
 6 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 3a65aed..ad364b5 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
 
  DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
 lun = busid & 7;
-s->current_req = s->current_dev->info->alloc_req(s->current_dev, 0, lun);
+s->current_req = scsi_req_new(s->current_dev, 0, lun);
 datalen = scsi_req_enqueue(s->current_req, buf);
 s->ti_size = datalen;
 if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 0bc2d73..5319125 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -788,8 +788,7 @@ static void lsi_do_command(LSIState *s)
 assert(s->current == NULL);
 s->current = qemu_mallocz(sizeof(lsi_request));
 s->current->tag = s->select_tag;
-s->current->req = dev->info->alloc_req(dev, s->current->tag,
-   s->current_lun);
+s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
 
 n = scsi_req_enqueue(s->current->req, buf);
 if (n > 0) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e09906a..17bd961 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -146,6 +146,11 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, 
uint32_t tag, uint32_t l
 return req;
 }
 
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
+{
+return d->info->alloc_req(d, tag, lun);
+}
+
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
 {
 assert(!req->enqueued);
diff --git a/hw/scsi.h b/hw/scsi.h
index c36c5cc..e44c194 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -143,6 +143,7 @@ int scsi_build_sense(SCSISense sense, uint8_t *buf, int 
len, int fixed);
 int scsi_sense_valid(SCSISense sense);
 
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t 
lun);
+SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
 void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 307a17f..1bb4bf4 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -653,7 +653,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
 
 req->sdev = sdev;
 req->lun = lun;
-req->sreq = sdev->info->alloc_req(sdev, req->qtag, lun);
+req->sreq = scsi_req_new(sdev, req->qtag, lun);
 n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
 
 dprintf("VSCSI: Queued command tag 0x%x CMD 0x%x ID %d LUN %d ret: %d\n",
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index e833809..f55aef8 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -376,7 +376,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
 DPRINTF("Command tag 0x%x flags %08x len %d data %d\n",
 s->tag, cbw.flags, cbw.cmd_len, s->data_len);
 s->residue = 0;
-s->req = s->scsi_dev->info->alloc_req(s->scsi_dev, s->tag, 0);
+s->req = scsi_req_new(s->scsi_dev, s->tag, 0);
 scsi_req_enqueue(s->req, cbw.cmd);
 /* ??? Should check that USB and SCSI data transfer
directions match.  */
-- 
1.7.4.4





[Qemu-devel] [PATCH 11/20] scsi: use scsi_req_complete

2011-05-03 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hw/scsi-generic.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 5bfbb8a..e1f8a8a 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -288,7 +288,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t 
*cmd)
 {
 SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
-SCSIBus *bus;
 int ret;
 
 scsi_req_enqueue(req);
@@ -305,8 +304,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t 
*cmd)
 s->sensebuf[6] = 0x00;
 s->senselen = 7;
 s->driver_status = SG_ERR_DRIVER_SENSE;
-bus = scsi_bus_from_device(&s->qdev);
-bus->ops.complete(req, SCSI_REASON_DONE, CHECK_CONDITION);
+r->req.status = CHECK_CONDITION;
+scsi_req_complete(&r->req);
 return 0;
 }
 
-- 
1.7.4.4





[Qemu-devel] [PATCH 05/20] scsi: reference-count requests

2011-05-03 Thread Paolo Bonzini
With the next patch, a device may hold SCSIRequest for an indefinite
time.  Split a rather big patch, and protect against access errors,
by reference counting them.

There is some ugliness in scsi_send_command implementation due to
the need to unref the request when it fails.  This will go away
with the next patches, which move the unref'ing to the devices.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-bus.c |   22 --
 hw/scsi-disk.c|   20 +---
 hw/scsi-generic.c |   23 ---
 hw/scsi.h |5 +
 4 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 63d9a68..c0bc275 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -136,6 +136,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, 
uint32_t tag, uint32_t l
 SCSIRequest *req;
 
 req = qemu_mallocz(size);
+req->refcount = 2;
 req->bus = scsi_bus_from_device(d);
 req->dev = d;
 req->tag = tag;
@@ -159,21 +160,23 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
 return NULL;
 }
 
-static void scsi_req_dequeue(SCSIRequest *req)
+void scsi_req_dequeue(SCSIRequest *req)
 {
 trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag);
 if (req->enqueued) {
 QTAILQ_REMOVE(&req->dev->requests, req, next);
 req->enqueued = false;
+scsi_req_unref(req);
 }
 }
 
 void scsi_req_free(SCSIRequest *req)
 {
-scsi_req_dequeue(req);
+assert(req->refcount == 0);
 qemu_free(req);
 }
 
+
 static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
 {
 switch (cmd[0] >> 5) {
@@ -495,6 +498,19 @@ static const char *scsi_command_name(uint8_t cmd)
 return names[cmd];
 }
 
+SCSIRequest *scsi_req_ref(SCSIRequest *req)
+{
+req->refcount++;
+return req;
+}
+
+void scsi_req_unref(SCSIRequest *req)
+{
+if (--req->refcount == 0) {
+req->dev->info->free_req(req);
+}
+}
+
 void scsi_req_data(SCSIRequest *req, int len)
 {
 trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
@@ -532,10 +548,12 @@ void scsi_req_print(SCSIRequest *req)
 void scsi_req_complete(SCSIRequest *req)
 {
 assert(req->status != -1);
+scsi_req_ref(req);
 scsi_req_dequeue(req);
 req->bus->ops.complete(req->bus, SCSI_REASON_DONE,
req->tag,
req->status);
+scsi_req_unref(req);
 }
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2b5dc2a..ba7ffa1 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -95,8 +95,10 @@ static SCSIDiskReq *scsi_new_request(SCSIDiskState *s, 
uint32_t tag,
 return r;
 }
 
-static void scsi_remove_request(SCSIDiskReq *r)
+static void scsi_free_request(SCSIRequest *req)
 {
+SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
+
 qemu_vfree(r->iov.iov_base);
 scsi_req_free(&r->req);
 }
@@ -131,7 +133,6 @@ static void scsi_command_complete(SCSIDiskReq *r, int 
status, int sense)
 r->req.tag, status, sense);
 scsi_req_set_status(r, status, sense);
 scsi_req_complete(&r->req);
-scsi_remove_request(r);
 }
 
 /* Cancel a pending data transfer.  */
@@ -145,7 +146,7 @@ static void scsi_cancel_io(SCSIDevice *d, uint32_t tag)
 if (r->req.aiocb)
 bdrv_aio_cancel(r->req.aiocb);
 r->req.aiocb = NULL;
-scsi_remove_request(r);
+scsi_req_dequeue(&r->req);
 }
 }
 
@@ -1030,7 +1031,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t 
tag,
  uint8_t *buf, int lun)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
-uint32_t len;
+int32_t len;
 int is_write;
 uint8_t command;
 uint8_t *outbuf;
@@ -1092,6 +1093,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t 
tag,
 case REZERO_UNIT:
 rc = scsi_disk_emulate_command(r, outbuf);
 if (rc < 0) {
+scsi_req_unref(&r->req);
 return 0;
 }
 
@@ -1178,9 +1180,11 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t 
tag,
 DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
 fail:
 scsi_command_complete(r, CHECK_CONDITION, ILLEGAL_REQUEST);
+scsi_req_unref(&r->req);
 return 0;
 illegal_lba:
 scsi_command_complete(r, CHECK_CONDITION, HARDWARE_ERROR);
+scsi_req_unref(&r->req);
 return 0;
 }
 if (r->sector_count == 0 && r->iov.iov_len == 0) {
@@ -1188,12 +1192,13 @@ static int32_t scsi_send_command(SCSIDevice *d, 
uint32_t tag,
 }
 len = r->sector_count * 512 + r->iov.iov_len;
 if (is_write) {
-return -len;
+len = -len;
 } else {
 if (!r->sector_count)
 r->sector_count = -1;
-return len;
 }
+scsi_req_unref(&r->req);
+return len;
 }
 
 static void scsi_disk_purge_requests(SCSIDiskState *s)
@@ -1205,7 +1210,7 @@ static void scsi_disk_purge_reques

[Qemu-devel] [PATCH 19/20] scsi: make write_data return void

2011-05-03 Thread Paolo Bonzini
The return value is unused anyway.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-disk.c|6 ++
 hw/scsi-generic.c |7 ++-
 hw/scsi.h |2 +-
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 65744c7..4c7a53e 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -267,7 +267,7 @@ static void scsi_write_complete(void * opaque, int ret)
 }
 }
 
-static int scsi_write_data(SCSIRequest *req)
+static void scsi_write_data(SCSIRequest *req)
 {
 SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
@@ -279,7 +279,7 @@ static int scsi_write_data(SCSIRequest *req)
 if (r->req.cmd.mode != SCSI_XFER_TO_DEV) {
 DPRINTF("Data transfer direction invalid\n");
 scsi_write_complete(r, -EINVAL);
-return 0;
+return;
 }
 
 n = r->iov.iov_len / 512;
@@ -294,8 +294,6 @@ static int scsi_write_data(SCSIRequest *req)
 /* Invoke completion routine to fetch data from host.  */
 scsi_write_complete(r, 0);
 }
-
-return 0;
 }
 
 static void scsi_dma_restart_bh(void *opaque)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index a4de39d..1ea0930 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -275,7 +275,7 @@ static void scsi_write_complete(void * opaque, int ret)
 
 /* Write data to a scsi device.  Returns nonzero on failure.
The transfer may complete asynchronously.  */
-static int scsi_write_data(SCSIRequest *req)
+static void scsi_write_data(SCSIRequest *req)
 {
 SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
@@ -285,16 +285,13 @@ static int scsi_write_data(SCSIRequest *req)
 if (r->len == 0) {
 r->len = r->buflen;
 scsi_req_data(&r->req, r->len);
-return 0;
+return;
 }
 
 ret = execute_command(s->bs, r, SG_DXFER_TO_DEV, scsi_write_complete);
 if (ret < 0) {
 scsi_command_complete(r, ret);
-return 1;
 }
-
-return 0;
 }
 
 /* Return a pointer to the data buffer.  */
diff --git a/hw/scsi.h b/hw/scsi.h
index dbb69ef..7eed475 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -77,7 +77,7 @@ struct SCSIDeviceInfo {
 void (*free_req)(SCSIRequest *req);
 int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
 void (*read_data)(SCSIRequest *req);
-int (*write_data)(SCSIRequest *req);
+void (*write_data)(SCSIRequest *req);
 void (*cancel_io)(SCSIRequest *req);
 uint8_t *(*get_buf)(SCSIRequest *req);
 int (*get_sense)(SCSIRequest *req, uint8_t *buf, int len);
-- 
1.7.4.4





[Qemu-devel] [PATCH 13/20] scsi: do not call send_command directly

2011-05-03 Thread Paolo Bonzini
Move the common part of scsi-disk.c and scsi-generic.c to the SCSI layer.

Signed-off-by: Paolo Bonzini 
---
 hw/esp.c  |2 +-
 hw/lsi53c895a.c   |2 +-
 hw/scsi-bus.c |3 ++-
 hw/scsi-disk.c|1 -
 hw/scsi-generic.c |1 -
 hw/scsi.h |2 +-
 hw/spapr_vscsi.c  |4 ++--
 hw/usb-msd.c  |2 +-
 8 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 46157a8..3a65aed 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -245,7 +245,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t 
busid)
  DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
 lun = busid & 7;
 s->current_req = s->current_dev->info->alloc_req(s->current_dev, 0, lun);
-datalen = s->current_dev->info->send_command(s->current_req, buf);
+datalen = scsi_req_enqueue(s->current_req, buf);
 s->ti_size = datalen;
 if (datalen != 0) {
 s->rregs[ESP_RSTAT] = STAT_TC;
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 822e0e5..0bc2d73 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -791,7 +791,7 @@ static void lsi_do_command(LSIState *s)
 s->current->req = dev->info->alloc_req(dev, s->current->tag,
s->current_lun);
 
-n = dev->info->send_command(s->current->req, buf);
+n = scsi_req_enqueue(s->current->req, buf);
 if (n > 0) {
 lsi_set_phase(s, PHASE_DI);
 dev->info->read_data(s->current->req);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index b83dd88..e09906a 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -146,12 +146,13 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, 
uint32_t tag, uint32_t l
 return req;
 }
 
-void scsi_req_enqueue(SCSIRequest *req)
+int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
 {
 assert(!req->enqueued);
 scsi_req_ref(req);
 req->enqueued = true;
 QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
+return req->dev->info->send_command(req, buf);
 }
 
 static void scsi_req_dequeue(SCSIRequest *req)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a82753f..efb953b 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -982,7 +982,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t 
*buf)
 uint8_t *outbuf;
 int rc;
 
-scsi_req_enqueue(req);
 command = buf[0];
 outbuf = (uint8_t *)r->iov.iov_base;
 is_write = 0;
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index b934ba4..036ab9f 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -318,7 +318,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t 
*cmd)
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
 int ret;
 
-scsi_req_enqueue(req);
 if (cmd[0] != REQUEST_SENSE &&
 (req->lun != s->lun || (cmd[1] >> 5) != s->lun)) {
 DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : cmd[1] >> 5);
diff --git a/hw/scsi.h b/hw/scsi.h
index 61ab7c9..c36c5cc 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -143,7 +143,7 @@ int scsi_build_sense(SCSISense sense, uint8_t *buf, int 
len, int fixed);
 int scsi_sense_valid(SCSISense sense);
 
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t 
lun);
-void scsi_req_enqueue(SCSIRequest *req);
+int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
 void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
 void scsi_req_unref(SCSIRequest *req);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 5b97a83..307a17f 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -459,7 +459,7 @@ static void vscsi_send_request_sense(VSCSIState *s, 
vscsi_req *req)
 cdb[4] = 96;
 cdb[5] = 0;
 req->sensing = 1;
-n = sdev->info->send_command(req->sreq, cdb);
+n = scsi_req_enqueue(req->sreq, cdb);
 dprintf("VSCSI: Queued request sense tag 0x%x\n", req->qtag);
 if (n < 0) {
 fprintf(stderr, "VSCSI: REQUEST_SENSE wants write data !?!?!?\n");
@@ -654,7 +654,7 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
 req->sdev = sdev;
 req->lun = lun;
 req->sreq = sdev->info->alloc_req(sdev, req->qtag, lun);
-n = sdev->info->send_command(req->sreq, srp->cmd.cdb);
+n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
 
 dprintf("VSCSI: Queued command tag 0x%x CMD 0x%x ID %d LUN %d ret: %d\n",
 req->qtag, srp->cmd.cdb[0], id, lun, n);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 4e20c90..e833809 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -377,7 +377,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
 s->tag, cbw.flags, cbw.cmd_len, s->data_len);
 s->residue = 0;
 s->req = s->scsi_dev->info->alloc_req(s->scsi_dev, s->tag, 0);
-s->scsi_dev->info->send_command(s->req, cbw.cmd);
+scsi_req_enqueue(s->req, cbw.cmd);
 /* ??? Should check that USB and SCSI data transfer
directions match.  */
 if (s->resid

[Qemu-devel] [PATCH 12/20] scsi: Update sense code handling

2011-05-03 Thread Paolo Bonzini
From: Hannes Reinecke 

The SCSI spec has a quite detailed list of sense codes available.
It even mandates the use of specific ones for some failure cases.
The current implementation just has one type of generic error
which is actually a violation of the spec in certain cases.
This patch introduces various predefined sense codes to have the
sense code reporting more in line with the spec.

On top of Hannes's patch I fixed the reply to REQUEST SENSE commands
with DESC=0 and a small (<18) length.

Signed-off-by: Hannes Reinecke 
Signed-off-by: Paolo Bonzini 
Cc: Christoph Hellwig 
---
 hw/scsi-bus.c |   91 -
 hw/scsi-disk.c|   82 ++-
 hw/scsi-generic.c |   63 -
 hw/scsi.h |   39 ++-
 4 files changed, 208 insertions(+), 67 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 6a8ea0b..b83dd88 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -154,7 +154,7 @@ void scsi_req_enqueue(SCSIRequest *req)
 QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
 }
 
-void scsi_req_dequeue(SCSIRequest *req)
+static void scsi_req_dequeue(SCSIRequest *req)
 {
 trace_scsi_req_dequeue(req->dev->id, req->lun, req->tag);
 if (req->enqueued) {
@@ -398,6 +398,95 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
 return 0;
 }
 
+/*
+ * Predefined sense codes
+ */
+
+/* No sense data available */
+const struct SCSISense sense_code_NO_SENSE = {
+.key = NO_SENSE , .asc = 0x00 , .ascq = 0x00
+};
+
+/* LUN not ready, Manual intervention required */
+const struct SCSISense sense_code_LUN_NOT_READY = {
+.key = NOT_READY, .asc = 0x04, .ascq = 0x03
+};
+
+/* LUN not ready, Medium not present */
+const struct SCSISense sense_code_NO_MEDIUM = {
+.key = NOT_READY, .asc = 0x3a, .ascq = 0x00
+};
+
+/* Hardware error, internal target failure */
+const struct SCSISense sense_code_TARGET_FAILURE = {
+.key = HARDWARE_ERROR, .asc = 0x44, .ascq = 0x00
+};
+
+/* Illegal request, invalid command operation code */
+const struct SCSISense sense_code_INVALID_OPCODE = {
+.key = ILLEGAL_REQUEST, .asc = 0x20, .ascq = 0x00
+};
+
+/* Illegal request, LBA out of range */
+const struct SCSISense sense_code_LBA_OUT_OF_RANGE = {
+.key = ILLEGAL_REQUEST, .asc = 0x21, .ascq = 0x00
+};
+
+/* Illegal request, Invalid field in CDB */
+const struct SCSISense sense_code_INVALID_FIELD = {
+.key = ILLEGAL_REQUEST, .asc = 0x24, .ascq = 0x00
+};
+
+/* Illegal request, LUN not supported */
+const struct SCSISense sense_code_LUN_NOT_SUPPORTED = {
+.key = ILLEGAL_REQUEST, .asc = 0x25, .ascq = 0x00
+};
+
+/* Command aborted, I/O process terminated */
+const struct SCSISense sense_code_IO_ERROR = {
+.key = ABORTED_COMMAND, .asc = 0x00, .ascq = 0x06
+};
+
+/* Command aborted, I_T Nexus loss occurred */
+const struct SCSISense sense_code_I_T_NEXUS_LOSS = {
+.key = ABORTED_COMMAND, .asc = 0x29, .ascq = 0x07
+};
+
+/* Command aborted, Logical Unit failure */
+const struct SCSISense sense_code_LUN_FAILURE = {
+.key = ABORTED_COMMAND, .asc = 0x3e, .ascq = 0x01
+};
+
+/*
+ * scsi_build_sense
+ *
+ * Build a sense buffer
+ */
+int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed)
+{
+if (!fixed && len < 8) {
+return 0;
+}
+
+memset(buf, 0, len);
+if (fixed) {
+/* Return fixed format sense buffer */
+buf[0] = 0xf0;
+buf[2] = sense.key;
+buf[7] = 7;
+buf[12] = sense.asc;
+buf[13] = sense.ascq;
+return MIN(len, 18);
+} else {
+/* Return descriptor format sense buffer */
+buf[0] = 0x72;
+buf[1] = sense.key;
+buf[2] = sense.asc;
+buf[3] = sense.ascq;
+return 8;
+}
+}
+
 static const char *scsi_command_name(uint8_t cmd)
 {
 static const char *names[] = {
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 0921c62..a82753f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -49,10 +49,6 @@ do { fprintf(stderr, "scsi-disk: " fmt , ## __VA_ARGS__); } 
while (0)
 
 typedef struct SCSIDiskState SCSIDiskState;
 
-typedef struct SCSISense {
-uint8_t key;
-} SCSISense;
-
 typedef struct SCSIDiskReq {
 SCSIRequest req;
 /* ??? We should probably keep track of whether the data transfer is
@@ -109,24 +105,19 @@ static void scsi_disk_clear_sense(SCSIDiskState *s)
 memset(&s->sense, 0, sizeof(s->sense));
 }
 
-static void scsi_disk_set_sense(SCSIDiskState *s, uint8_t key)
-{
-s->sense.key = key;
-}
-
-static void scsi_req_set_status(SCSIDiskReq *r, int status, int sense_code)
+static void scsi_req_set_status(SCSIDiskReq *r, int status, SCSISense sense)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
 r->req.status = status;
-scsi_disk_set_sense(s, sense_code);
+s->sense = sense;
 }
 
 /* Helper function for command completion.  */
-static void scsi_command_com

[Qemu-devel] [PATCH 07/20] scsi: Use 'SCSIRequest' directly

2011-05-03 Thread Paolo Bonzini
From: Hannes Reinecke 

Currently the SCSIRequest structure is abstracted away and cannot accessed
directly from the driver. This requires the handler to do a lookup on
an abstract 'tag' which identifies the SCSIRequest structure.

With this patch the SCSIRequest structure is exposed to the driver. This
allows use to use it directly as an argument to the SCSIDeviceInfo
callback functions and remove the lookup.

A new callback function 'alloc_req' is introduced matching 'free
req'; unref'ing to free up resources after use is moved into the
scsi_command_complete callbacks.

This temporarily introduces a leak of requests that are cancelled,
when they are removed from the queue and not from the driver.  This
is fixed later by introducing scsi_req_cancel.  That patch in turn
depends on this one, because the argument to scsi_req_cancel is a
SCSIRequest.

Signed-off-by: Hannes Reinecke 
Signed-off-by: Paolo Bonzini 
Cc: Christoph Hellwig 
---
 hw/esp.c  |   29 -
 hw/lsi53c895a.c   |   56 +++---
 hw/scsi-bus.c |   24 ---
 hw/scsi-disk.c|  116 ++--
 hw/scsi-generic.c |  107 +++--
 hw/scsi.h |   21 +-
 hw/spapr_vscsi.c  |   44 +++-
 hw/usb-msd.c  |   27 +++-
 8 files changed, 172 insertions(+), 252 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index d8bba7a..096f4dc 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -65,6 +65,7 @@ struct ESPState {
 uint32_t dma;
 SCSIBus bus;
 SCSIDevice *current_dev;
+SCSIRequest *current_req;
 uint8_t cmdbuf[TI_BUFSZ];
 uint32_t cmdlen;
 uint32_t do_cmd;
@@ -209,7 +210,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
 
 if (s->current_dev) {
 /* Started a new command before the old one finished.  Cancel it.  */
-s->current_dev->info->cancel_io(s->current_dev, 0);
+s->current_dev->info->cancel_io(s->current_req);
 s->async_len = 0;
 }
 
@@ -230,9 +231,10 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
uint8_t busid)
 int32_t datalen;
 int lun;
 
-DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
+ DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
 lun = busid & 7;
-datalen = s->current_dev->info->send_command(s->current_dev, 0, buf, lun);
+s->current_req = s->current_dev->info->alloc_req(s->current_dev, 0, lun);
+datalen = s->current_dev->info->send_command(s->current_req, buf);
 s->ti_size = datalen;
 if (datalen != 0) {
 s->rregs[ESP_RSTAT] = STAT_TC;
@@ -240,10 +242,10 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
uint8_t busid)
 s->dma_counter = 0;
 if (datalen > 0) {
 s->rregs[ESP_RSTAT] |= STAT_DI;
-s->current_dev->info->read_data(s->current_dev, 0);
+s->current_dev->info->read_data(s->current_req);
 } else {
 s->rregs[ESP_RSTAT] |= STAT_DO;
-s->current_dev->info->write_data(s->current_dev, 0);
+s->current_dev->info->write_data(s->current_req);
 }
 }
 s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
@@ -372,9 +374,9 @@ static void esp_do_dma(ESPState *s)
 if (s->async_len == 0) {
 if (to_device) {
 // ti_size is negative
-s->current_dev->info->write_data(s->current_dev, 0);
+s->current_dev->info->write_data(s->current_req);
 } else {
-s->current_dev->info->read_data(s->current_dev, 0);
+s->current_dev->info->read_data(s->current_req);
 /* If there is still data to be read from the device then
complete the DMA operation immediately.  Otherwise defer
until the scsi layer has completed.  */
@@ -388,10 +390,9 @@ static void esp_do_dma(ESPState *s)
 }
 }
 
-static void esp_command_complete(SCSIBus *bus, int reason, uint32_t tag,
- uint32_t arg)
+static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
 {
-ESPState *s = DO_UPCAST(ESPState, busdev.qdev, bus->qbus.parent);
+ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
 
 if (reason == SCSI_REASON_DONE) {
 DPRINTF("SCSI Command complete\n");
@@ -405,11 +406,15 @@ static void esp_command_complete(SCSIBus *bus, int 
reason, uint32_t tag,
 s->sense = arg;
 s->rregs[ESP_RSTAT] = STAT_ST;
 esp_dma_done(s);
-s->current_dev = NULL;
+if (s->current_req) {
+scsi_req_unref(s->current_req);
+s->current_req = NULL;
+s->current_dev = NULL;
+}
 } else {
 DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
 s->async_len = arg;
-s->async_buf = s->current_dev->info->get_buf(s->current_dev, 0);
+s->async_buf = s->current_dev->info->get_buf(req);
 if (s->dma_left) {
 esp_do_dma(s)

[Qemu-devel] [PATCH 20/20] scsi-generic: Handle queue full

2011-05-03 Thread Paolo Bonzini
The sg driver currently has a hardcoded limit of commands it
can handle simultaneously. When this limit is reached the
driver will return -EDOM. So we need to capture this to
enable proper return values here.

Signed-off-by: Hannes Reinecke 

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-generic.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 1ea0930..0c04606 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -124,6 +124,9 @@ static void scsi_command_complete(void *opaque, int ret)
 
 if (ret != 0) {
 switch (ret) {
+case -EDOM:
+r->req.status = TASK_SET_FULL;
+break;
 case -EINVAL:
 r->req.status = CHECK_CONDITION;
 scsi_set_sense(s, SENSE_CODE(INVALID_FIELD));
-- 
1.7.4.4




[Qemu-devel] [PATCH 16/20] scsi: introduce scsi_req_get_buf

2011-05-03 Thread Paolo Bonzini
... and remove some SCSIDevice variables or fields that now become unused.

Signed-off-by: Paolo Bonzini 
---
 hw/esp.c |2 +-
 hw/lsi53c895a.c  |2 +-
 hw/scsi-bus.c|5 +
 hw/scsi.h|1 +
 hw/spapr_vscsi.c |8 ++--
 hw/usb-msd.c |2 +-
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 1f342f8..051b0fa 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -419,7 +419,7 @@ static void esp_command_complete(SCSIRequest *req, int 
reason, uint32_t arg)
 } else {
 DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
 s->async_len = arg;
-s->async_buf = s->current_dev->info->get_buf(req);
+s->async_buf = scsi_req_get_buf(req);
 if (s->dma_left) {
 esp_do_dma(s);
 } else if (s->dma_counter != 0 && s->ti_size <= 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index a782904..de0e11b 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -569,7 +569,7 @@ static void lsi_do_dma(LSIState *s, int out)
 s->dnad += count;
 s->dbc -= count;
  if (s->current->dma_buf == NULL) {
-s->current->dma_buf = dev->info->get_buf(s->current->req);
+s->current->dma_buf = scsi_req_get_buf(s->current->req);
 }
 /* ??? Set SFBR to first data byte.  */
 if (out) {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ed2839a..417dc77 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -151,6 +151,11 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, 
uint32_t lun)
 return d->info->alloc_req(d, tag, lun);
 }
 
+uint8_t *scsi_req_get_buf(SCSIRequest *req)
+{
+return req->dev->info->get_buf(req);
+}
+
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
 {
 assert(!req->enqueued);
diff --git a/hw/scsi.h b/hw/scsi.h
index f659503..3af6295 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -154,6 +154,7 @@ void scsi_req_print(SCSIRequest *req);
 void scsi_req_kick(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
+uint8_t *scsi_req_get_buf(SCSIRequest *req);
 void scsi_req_abort(SCSIRequest *req, int status);
 void scsi_req_cancel(SCSIRequest *req);
 void scsi_device_purge_requests(SCSIDevice *sdev);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 27c8e17..46cd1d7 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -74,7 +74,6 @@ typedef struct vscsi_req {
 union viosrp_iu iu;
 
 /* SCSI request tracking */
-SCSIDevice  *sdev;
 SCSIRequest *sreq;
 uint32_tqtag; /* qemu tag != srp tag */
 int lun;
@@ -476,7 +475,6 @@ static void vscsi_command_complete(SCSIRequest *sreq, int 
reason, uint32_t arg)
 {
 VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
 vscsi_req *req = vscsi_find_req(s, sreq);
-SCSIDevice *sdev;
 uint8_t *buf;
 int32_t res_in = 0, res_out = 0;
 int len, rc = 0;
@@ -487,7 +485,6 @@ static void vscsi_command_complete(SCSIRequest *sreq, int 
reason, uint32_t arg)
 fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
 return;
 }
-sdev = req->sdev;
 
 if (req->sensing) {
 if (reason == SCSI_REASON_DONE) {
@@ -495,7 +492,7 @@ static void vscsi_command_complete(SCSIRequest *sreq, int 
reason, uint32_t arg)
 vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
 vscsi_put_req(s, req);
 } else {
-uint8_t *buf = sdev->info->get_buf(sreq);
+uint8_t *buf = scsi_req_get_buf(sreq);
 
 len = MIN(arg, SCSI_SENSE_BUF_SIZE);
 dprintf("VSCSI: Sense data, %d bytes:\n", len);
@@ -539,7 +536,7 @@ static void vscsi_command_complete(SCSIRequest *sreq, int 
reason, uint32_t arg)
  * to write for writes (ie, how much is to be DMA'd)
  */
 if (arg) {
-buf = sdev->info->get_buf(sreq);
+buf = scsi_req_get_buf(sreq);
 rc = vscsi_srp_transfer_data(s, req, req->writing, buf, arg);
 }
 if (rc < 0) {
@@ -646,7 +643,6 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
 } return 1;
 }
 
-req->sdev = sdev;
 req->lun = lun;
 req->sreq = scsi_req_new(sdev, req->qtag, lun);
 n = scsi_req_enqueue(req->sreq, srp->cmd.cdb);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 9b69337..5c429ba 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -247,7 +247,7 @@ static void usb_msd_command_complete(SCSIRequest *req, int 
reason, uint32_t arg)
 }
 assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == 
SCSI_XFER_TO_DEV));
 s->scsi_len = arg;
-s->scsi_buf = s->scsi_dev->info->get_buf(req);
+s->scsi_buf = scsi_req_get_buf(req);
 if (p) {
 usb_msd_copy_data(s);
 if (s->usb_len == 0) {
-- 
1.7.4.4





[Qemu-devel] [PATCH 10/20] scsi: introduce scsi_req_cancel

2011-05-03 Thread Paolo Bonzini
This is for when the request must be dropped in the void,
but still memory should be freed.  To this end, the devices
register a second callback in SCSIBusOps.

Signed-off-by: Paolo Bonzini 
---
 hw/esp.c  |   16 ++--
 hw/lsi53c895a.c   |   30 +-
 hw/scsi-bus.c |   17 ++---
 hw/scsi-disk.c|1 -
 hw/scsi-generic.c |1 -
 hw/scsi.h |2 ++
 hw/spapr_vscsi.c  |   11 ++-
 hw/usb-msd.c  |   19 +++
 8 files changed, 80 insertions(+), 17 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 096f4dc..46157a8 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -188,6 +188,17 @@ static void esp_dma_enable(void *opaque, int irq, int 
level)
 }
 }
 
+static void esp_request_cancelled(SCSIRequest *req)
+{
+ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
+
+if (req == s->current_req) {
+scsi_req_unref(s->current_req);
+s->current_req = NULL;
+s->current_dev = NULL;
+}
+}
+
 static uint32_t get_cmd(ESPState *s, uint8_t *buf)
 {
 uint32_t dmalen;
@@ -210,7 +221,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
 
 if (s->current_dev) {
 /* Started a new command before the old one finished.  Cancel it.  */
-s->current_dev->info->cancel_io(s->current_req);
+scsi_req_cancel(s->current_req);
 s->async_len = 0;
 }
 
@@ -720,7 +731,8 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
 }
 
 static struct SCSIBusOps esp_scsi_ops = {
-.complete = esp_command_complete
+.complete = esp_command_complete,
+.cancel = esp_request_cancelled
 };
 
 static int esp_init1(SysBusDevice *dev)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 81a93b1..822e0e5 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -664,6 +664,26 @@ static lsi_request *lsi_find_by_tag(LSIState *s, uint32_t 
tag)
 return NULL;
 }
 
+static void lsi_request_cancelled(SCSIRequest *req)
+{
+LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
+lsi_request *p;
+
+if (req == s->current->req) {
+scsi_req_unref(req);
+qemu_free(s->current);
+s->current = NULL;
+return;
+}
+
+p = lsi_find_by_tag(s, req->tag);
+if (p) {
+QTAILQ_REMOVE(&s->queue, p, next);
+scsi_req_unref(req);
+qemu_free(p);
+}
+}
+
 /* Record that data is available for a queued command.  Returns zero if
the device was reselected, nonzero if the IO is deferred.  */
 static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
@@ -931,7 +951,7 @@ static void lsi_do_msgout(LSIState *s)
 /* The ABORT TAG message clears the current I/O process only. */
 DPRINTF("MSG: ABORT TAG tag=0x%x\n", current_tag);
 if (current_req) {
-current_dev->info->cancel_io(current_req->req);
+scsi_req_cancel(current_req->req);
 }
 lsi_disconnect(s);
 break;
@@ -956,7 +976,7 @@ static void lsi_do_msgout(LSIState *s)
 
 /* clear the current I/O process */
 if (s->current) {
-current_dev->info->cancel_io(s->current->req);
+scsi_req_cancel(s->current->req);
 }
 
 /* As the current implemented devices scsi_disk and scsi_generic
@@ -969,8 +989,7 @@ static void lsi_do_msgout(LSIState *s)
 id = current_tag & 0xff00;
 QTAILQ_FOREACH_SAFE(p, &s->queue, next, p_next) {
 if ((p->tag & 0xff00) == id) {
-current_dev->info->cancel_io(p->req);
-QTAILQ_REMOVE(&s->queue, p, next);
+scsi_req_cancel(p->req);
 }
 }
 
@@ -2236,7 +2255,8 @@ static int lsi_scsi_uninit(PCIDevice *d)
 }
 
 static struct SCSIBusOps lsi_scsi_ops = {
-.complete = lsi_command_complete
+.complete = lsi_command_complete,
+.cancel = lsi_request_cancelled
 };
 
 static int lsi_scsi_init(PCIDevice *dev)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 1d7da9e..6a8ea0b 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -548,6 +548,19 @@ void scsi_req_complete(SCSIRequest *req)
 scsi_req_unref(req);
 }
 
+void scsi_req_cancel(SCSIRequest *req)
+{
+if (req->dev && req->dev->info->cancel_io) {
+req->dev->info->cancel_io(req);
+}
+scsi_req_ref(req);
+scsi_req_dequeue(req);
+if (req->bus->ops.cancel) {
+req->bus->ops.cancel(req);
+}
+scsi_req_unref(req);
+}
+
 void scsi_req_abort(SCSIRequest *req, int status)
 {
 req->status = status;
@@ -563,9 +576,7 @@ void scsi_device_purge_requests(SCSIDevice *sdev)
 
 while (!QTAILQ_EMPTY(&sdev->requests)) {
 req = QTAILQ_FIRST(&sdev->requests);
-sdev->info->cancel_io(req);
-scsi_req_dequeue(req);
-scsi_req_unref(req);
+scsi_req_cancel(req);
 }
 }
 
diff --git a/hw/scsi-disk.c b/hw/scsi-d

Re: [Qemu-devel] [PATCH] posix-aio-compat: Fix idle_threads counter

2011-05-03 Thread Kevin Wolf
Am 03.05.2011 17:56, schrieb Stefan Hajnoczi:
> On Tue, May 3, 2011 at 2:26 PM, Kevin Wolf  wrote:
>> A thread should only be counted as idle when it really is waiting for new
>> requests. Without this patch, sometimes too few threads are started as busy
>> threads are counted as idle.
>>
>> Not sure if it makes a difference in practice outside some artificial
>> qemu-io/qemu-img tests, but I think the change makes sense in any case.
>>
>> Signed-off-by: Kevin Wolf 
>> ---
>>  posix-aio-compat.c |6 ++
>>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> I think the critical change here is that idle_threads is not being
> incremented by spawn_thread().  This means that we will keep spawning
> threads as new requests come in and until the first thread goes idle.
> 
> Previously you could imagine a scenario where we spawn a thread but
> don't schedule it yet.  Then we immediately submit more I/O and since
> idle_threads was incremented we don't spawn additional threads to
> handle the requests.
> 
> Are these the cases you were thinking about?

Yes, this is the case that I noticed.

However, I'm not sure if this is really the critical change. In this
case, it would take a bit longer until you get your full 64 threads, but
you should get there eventually and then it shouldn't impact performance
any more.

However, what I saw in my test case (qemu-img always running 16
sequential read requests in parallel) was that I only got 16 threads.
This sounds logical, but in fact you seem to need always one thread more
for good performance (I don't fully understand this yet). And with this
patch, you actually get 17 threads. The difference was like 8s vs. 22s
for the same requests, and using more than 17 threads doesn't further
improve it.

This is what makes me a bit nervous about it, I don't really understand
what's going on here. The observation suggests that the race doesn't
only affect thread creation, but also operations afterwards.

> I like your patch because it reduces the number of places where we
> mess with idle_threads.  I'd move the increment/decrement out of the
> loop since the mutex is held here anyway so no one will test
> idle_threads between loop iterations, but it's up to you if you want
> to send a v2 or not.

Probably doesn't really make a difference.

Kevin



[Qemu-devel] [PATCH 17/20] scsi: Implement 'get_sense' callback

2011-05-03 Thread Paolo Bonzini
From: Hannes Reinecke 

The get_sense callback copies existing sense information into
the provided buffer. This is required if sense information
should be transferred together with the command response.

Signed-off-by: Hannes Reinecke 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi-bus.c |9 +
 hw/scsi-disk.c|9 +
 hw/scsi-generic.c |   18 ++
 hw/scsi.h |2 ++
 hw/spapr_vscsi.c  |   10 +-
 5 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 417dc77..e3ab5ca 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -156,6 +156,15 @@ uint8_t *scsi_req_get_buf(SCSIRequest *req)
 return req->dev->info->get_buf(req);
 }
 
+int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len)
+{
+if (req->dev->info->get_sense) {
+return req->dev->info->get_sense(req, buf, len);
+} else {
+return 0;
+}
+}
+
 int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
 {
 assert(!req->enqueued);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index efb953b..4241fad 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -338,6 +338,14 @@ static uint8_t *scsi_get_buf(SCSIRequest *req)
 return (uint8_t *)r->iov.iov_base;
 }
 
+/* Copy sense information into the provided buffer */
+static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
+{
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
+
+return scsi_build_sense(s->sense, outbuf, len, len > 14);
+}
+
 static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
@@ -1225,6 +1233,7 @@ static SCSIDeviceInfo scsi_disk_info = {
 .write_data   = scsi_write_data,
 .cancel_io= scsi_cancel_io,
 .get_buf  = scsi_get_buf,
+.get_sense= scsi_get_sense,
 .qdev.props   = (Property[]) {
 DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
 DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 036ab9f..a4de39d 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -79,6 +79,23 @@ static void scsi_clear_sense(SCSIGenericState *s)
 s->driver_status = 0;
 }
 
+static int scsi_get_sense(SCSIRequest *req, uint8_t *outbuf, int len)
+{
+SCSIGenericState *s = DO_UPCAST(SCSIGenericState, qdev, req->dev);
+int size = SCSI_SENSE_BUF_SIZE;
+
+if (!(s->driver_status & SG_ERR_DRIVER_SENSE)) {
+size = scsi_build_sense(SENSE_CODE(NO_SENSE), s->sensebuf,
+SCSI_SENSE_BUF_SIZE, 0);
+}
+if (size > len) {
+size = len;
+}
+memcpy(outbuf, s->sensebuf, size);
+
+return size;
+}
+
 static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
 {
 SCSIRequest *req;
@@ -533,6 +550,7 @@ static SCSIDeviceInfo scsi_generic_info = {
 .write_data   = scsi_write_data,
 .cancel_io= scsi_cancel_io,
 .get_buf  = scsi_get_buf,
+.get_sense= scsi_get_sense,
 .qdev.props   = (Property[]) {
 DEFINE_BLOCK_PROPERTIES(SCSIGenericState, qdev.conf),
 DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/scsi.h b/hw/scsi.h
index 3af6295..dbb69ef 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -80,6 +80,7 @@ struct SCSIDeviceInfo {
 int (*write_data)(SCSIRequest *req);
 void (*cancel_io)(SCSIRequest *req);
 uint8_t *(*get_buf)(SCSIRequest *req);
+int (*get_sense)(SCSIRequest *req, uint8_t *buf, int len);
 };
 
 struct SCSIBusOps {
@@ -155,6 +156,7 @@ void scsi_req_kick(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
 uint8_t *scsi_req_get_buf(SCSIRequest *req);
+int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
 void scsi_req_abort(SCSIRequest *req, int status);
 void scsi_req_cancel(SCSIRequest *req);
 void scsi_device_purge_requests(SCSIDevice *sdev);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 46cd1d7..8a47de0 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -450,6 +450,15 @@ static void vscsi_send_request_sense(VSCSIState *s, 
vscsi_req *req)
 uint8_t *cdb = req->iu.srp.cmd.cdb;
 int n;
 
+n = scsi_req_get_sense(req->sreq, req->sense, sizeof(req->sense));
+if (n) {
+req->senselen = n;
+vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
+vscsi_put_req(s, req);
+return;
+}
+
+dprintf("VSCSI: Got CHECK_CONDITION, requesting sense...\n");
 cdb[0] = 3;
 cdb[1] = 0;
 cdb[2] = 0;
@@ -522,7 +531,6 @@ static void vscsi_command_complete(SCSIRequest *sreq, int 
reason, uint32_t arg)
 }
 vscsi_send_rsp(s, req, 0, res_in, res_out);
 } else if (arg == CHECK_CONDITION) {
-dprintf("VSCSI: Got CHECK_CONDITION, requesting sense...\n");
 vscsi_send_request_sense(s, req);
 return;
 } else {
-- 
1.7.4.4





[Qemu-devel] [PATCH 15/20] scsi: introduce scsi_req_kick

2011-05-03 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hw/esp.c |   26 ++
 hw/lsi53c895a.c  |   22 --
 hw/scsi-bus.c|   10 ++
 hw/scsi.h|1 +
 hw/spapr_vscsi.c |   26 ++
 hw/usb-msd.c |   15 ---
 trace-events |1 +
 7 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index ad364b5..1f342f8 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -253,11 +253,10 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
uint8_t busid)
 s->dma_counter = 0;
 if (datalen > 0) {
 s->rregs[ESP_RSTAT] |= STAT_DI;
-s->current_dev->info->read_data(s->current_req);
 } else {
 s->rregs[ESP_RSTAT] |= STAT_DO;
-s->current_dev->info->write_data(s->current_req);
 }
+scsi_req_kick(s->current_req);
 }
 s->rregs[ESP_RINTR] = INTR_BS | INTR_FC;
 s->rregs[ESP_RSEQ] = SEQ_CD;
@@ -383,22 +382,17 @@ static void esp_do_dma(ESPState *s)
 else
 s->ti_size -= len;
 if (s->async_len == 0) {
-if (to_device) {
-// ti_size is negative
-s->current_dev->info->write_data(s->current_req);
-} else {
-s->current_dev->info->read_data(s->current_req);
-/* If there is still data to be read from the device then
-   complete the DMA operation immediately.  Otherwise defer
-   until the scsi layer has completed.  */
-if (s->dma_left == 0 && s->ti_size > 0) {
-esp_dma_done(s);
-}
+scsi_req_kick(s->current_req);
+/* If there is still data to be read from the device then
+   complete the DMA operation immediately.  Otherwise defer
+   until the scsi layer has completed.  */
+if (to_device || s->dma_left != 0 || s->ti_size == 0) {
+return;
 }
-} else {
-/* Partially filled a scsi buffer. Complete immediately.  */
-esp_dma_done(s);
 }
+
+/* Partially filled a scsi buffer. Complete immediately.  */
+esp_dma_done(s);
 }
 
 static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 5319125..a782904 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -580,13 +580,7 @@ static void lsi_do_dma(LSIState *s, int out)
 s->current->dma_len -= count;
 if (s->current->dma_len == 0) {
 s->current->dma_buf = NULL;
-if (out) {
-/* Write the data.  */
-dev->info->write_data(s->current->req);
-} else {
-/* Request any remaining data.  */
-dev->info->read_data(s->current->req);
-}
+scsi_req_kick(s->current->req);
 } else {
 s->current->dma_buf += count;
 lsi_resume_script(s);
@@ -791,14 +785,14 @@ static void lsi_do_command(LSIState *s)
 s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
 
 n = scsi_req_enqueue(s->current->req, buf);
-if (n > 0) {
-lsi_set_phase(s, PHASE_DI);
-dev->info->read_data(s->current->req);
-} else if (n < 0) {
-lsi_set_phase(s, PHASE_DO);
-dev->info->write_data(s->current->req);
+if (n) {
+if (n > 0) {
+lsi_set_phase(s, PHASE_DI);
+} else if (n < 0) {
+lsi_set_phase(s, PHASE_DO);
+}
+scsi_req_kick(s->current->req);
 }
-
 if (!s->command_complete) {
 if (n) {
 /* Command did not complete immediately so disconnect.  */
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 17bd961..ed2839a 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -600,6 +600,16 @@ void scsi_req_unref(SCSIRequest *req)
 }
 }
 
+void scsi_req_kick(SCSIRequest *req)
+{
+trace_scsi_req_kick(req->dev->id, req->lun, req->tag);
+if (req->cmd.mode == SCSI_XFER_TO_DEV) {
+req->dev->info->write_data(req);
+} else {
+req->dev->info->read_data(req);
+}
+}
+
 void scsi_req_data(SCSIRequest *req, int len)
 {
 trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
diff --git a/hw/scsi.h b/hw/scsi.h
index e44c194..f659503 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -151,6 +151,7 @@ void scsi_req_unref(SCSIRequest *req);
 
 int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
+void scsi_req_kick(SCSIRequest *req);
 void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
 void scsi_req_abort(SCSIRequest *req, int status);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 1bb4bf4..27c8e17 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -448,7 +448,6 @@ static int vscsi_preprocess_desc(vscsi_req *req)
 
 static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
 {
-SCSIDevice *sdev = req->sdev;
 uint8_t *cdb = req->iu.srp.cmd.cdb;
 int n;
 
@@ -469,7 +468,7 @@ static voi

[Qemu-devel] [PATCH 18/20] scsi-disk: add data direction checking

2011-05-03 Thread Paolo Bonzini
From: Hannes Reinecke 

scsi_req_parse() already provides for a data direction setting,
so we should be using it to check for correct direction.
And we should return the sense code 'INVALID FIELD IN CDB'
in these cases.

Signed-off-by: Hannes Reinecke 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi-disk.c |   37 +
 1 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 4241fad..65744c7 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -49,10 +49,8 @@ do { fprintf(stderr, "scsi-disk: " fmt , ## __VA_ARGS__); } 
while (0)
 
 typedef struct SCSIDiskState SCSIDiskState;
 
-typedef struct SCSIDiskReq {
+ typedef struct SCSIDiskReq {
 SCSIRequest req;
-/* ??? We should probably keep track of whether the data transfer is
-   a read or a write.  Currently we rely on the host getting it right.  */
 /* Both sector and sector_count are in terms of qemu 512 byte blocks.  */
 uint64_t sector;
 uint32_t sector_count;
@@ -178,6 +176,12 @@ static void scsi_read_data(SCSIRequest *req)
 /* No data transfer may already be in progress */
 assert(r->req.aiocb == NULL);
 
+if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
+DPRINTF("Data transfer direction invalid\n");
+scsi_read_complete(r, -EINVAL);
+return;
+}
+
 n = r->sector_count;
 if (n > SCSI_DMA_BUF_SIZE / 512)
 n = SCSI_DMA_BUF_SIZE / 512;
@@ -214,16 +218,22 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int 
error, int type)
 if (type == SCSI_REQ_STATUS_RETRY_READ) {
 scsi_req_data(&r->req, 0);
 }
-if (error == ENOMEM) {
+switch (error) {
+case ENOMEM:
 scsi_command_complete(r, CHECK_CONDITION,
   SENSE_CODE(TARGET_FAILURE));
-} else {
+break;
+case EINVAL:
+scsi_command_complete(r, CHECK_CONDITION,
+  SENSE_CODE(INVALID_FIELD));
+break;
+default:
 scsi_command_complete(r, CHECK_CONDITION,
   SENSE_CODE(IO_ERROR));
+break;
 }
 bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
 }
-
 return 1;
 }
 
@@ -266,6 +276,12 @@ static int scsi_write_data(SCSIRequest *req)
 /* No data transfer may already be in progress */
 assert(r->req.aiocb == NULL);
 
+if (r->req.cmd.mode != SCSI_XFER_TO_DEV) {
+DPRINTF("Data transfer direction invalid\n");
+scsi_write_complete(r, -EINVAL);
+return 0;
+}
+
 n = r->iov.iov_len / 512;
 if (n) {
 qemu_iovec_init_external(&r->qiov, &r->iov, 1);
@@ -985,14 +1001,12 @@ static int32_t scsi_send_command(SCSIRequest *req, 
uint8_t *buf)
 SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
 int32_t len;
-int is_write;
 uint8_t command;
 uint8_t *outbuf;
 int rc;
 
 command = buf[0];
 outbuf = (uint8_t *)r->iov.iov_base;
-is_write = 0;
 DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
 
 if (scsi_req_parse(&r->req, buf) != 0) {
@@ -1072,7 +1086,6 @@ static int32_t scsi_send_command(SCSIRequest *req, 
uint8_t *buf)
 goto illegal_lba;
 r->sector = r->req.cmd.lba * s->cluster_size;
 r->sector_count = len * s->cluster_size;
-is_write = 1;
 break;
 case MODE_SELECT:
 DPRINTF("Mode Select(6) (len %lu)\n", (long)r->req.cmd.xfer);
@@ -1138,13 +1151,13 @@ static int32_t scsi_send_command(SCSIRequest *req, 
uint8_t *buf)
 scsi_command_complete(r, GOOD, SENSE_CODE(NO_SENSE));
 }
 len = r->sector_count * 512 + r->iov.iov_len;
-if (is_write) {
-len = -len;
+if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
+return -len;
 } else {
 if (!r->sector_count)
 r->sector_count = -1;
+return len;
 }
-return len;
 }
 
 static void scsi_disk_reset(DeviceState *dev)
-- 
1.7.4.4





Re: [Qemu-devel] [PATCH v2] Import Linux headers for KVM and vhost

2011-05-03 Thread Avi Kivity

On 05/03/2011 07:48 PM, Jan Kiszka wrote:

This helps reducing our build-time checks for feature support in the
available Linux kernel headers. And it helps users that do not have
sufficiently recent headers installed on their build machine.

Header upstate is triggered via

 scripts/update-linux-headers.sh LINUX_PATH

from the top-level QEMU source directory.

Consequently, the patch removes and build-time checks for kvm and vhost
in configure, and also the --kerneldir switch. Kernel headers are
supposed to be provided by QEMU only.

Kernel headers were automatically imported from current kvm.git,
93c016c8c4. Some are not covered by any license and can be considered
GPLv2 with user space exception. Some are explicitly GPLv2 - at least
for QEMU that's fine in any case. Others are "BSD" which shall to be
considered a compatible variant according to Rusty.


Reluctant ack.

(though for commit log cleanliness, the scripts and the headers should 
be in separate commits)


--
error compiling committee.c: too many arguments to function




  1   2   >