Re: [Qemu-devel] [PATCH] vmdk: Allow space in file name

2013-01-29 Thread Andreas Färber
Am 29.01.2013 22:50, schrieb Philipp Hahn:
> The previous scanf() format string stopped parsing the file name on the
> first white white space, which seems to be allowed at least by VMware
> Wokrstation.

"Workstation" - hopefully can be fixed when applied.

> 
> Change the format string to collect everything between the first and
> second quote as the file name, disallowing line breaks.
> 
> Signed-off-by: Philipp Hahn 

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 2/2] qga: add guest-set-time command

2013-01-29 Thread Lei Li

On 01/29/2013 04:45 AM, Eric Blake wrote:

On 01/27/2013 11:14 AM, Lei Li wrote:

Signed-off-by: Lei Li 
---
  qga/commands-posix.c |   76 ++
  qga/qapi-schema.json |   42 +++
  2 files changed, 118 insertions(+), 0 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 2fef2b6..5424c50 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -149,6 +149,82 @@ struct TimeInfo *qmp_guest_get_time(Error **errp)
  return time_info;
  }
  
+void qmp_guest_set_time(bool has_seconds, int64_t seconds,

+bool has_microseconds, int64_t microseconds,
+bool has_utc_offset, int64_t utc_offset, Error **errp)
+{
+int ret;
+int status;
+pid_t pid, rpid;
+struct timeval tv;
+TimeInfo *time_info;
+
+/* year-2038 will overflow in case time_t is 32bit */
+if (sizeof(time_t) == 4) {
+return;
+}

What? Silently giving up, without even seeing whether overflow happened?
  That just feels wrong.  Make the change if it is in range, and be vocal
about setting an error if there is overflow.


Yes, this is my stupid mistake...




+
+if (!has_seconds) {
+if (has_microseconds || has_utc_offset) {
+error_setg(errp, "field 'seconds' missing");
+return;
+} else {
+/* Grab time from the host if no arguments given. */
+time_info = get_host_time(errp);

Does this even make sense?  Since this code will be executing in the
guest, you are getting the guest's current notion of time (as I said in
1/2, get_host_time() is misnamed); and then setting the guest's time
right back to the same value.  Why bother?

It seems like if the host is going to bother issuing the guest-set-time
agent command, then the host should have a mandatory argument to tell
the guest what to set its time to.

Then again, reading the rest of your code, I can see one case where it
_might_ make sense to allow the host to not pass in a time - namely, if
the host wants to force the guest to call hwclock to write its notion of
current system time (in RAM) back to the hardware.  But that seems like
a stretch.


+if (!time_info) {
+return;
+}
+
+tv.tv_sec = time_info->seconds;
+tv.tv_usec = time_info->microseconds;
+}
+} else {
+if (abs(utc_offset) > (24 * 60)) {

Won't work.  abs() takes an int argument, so you can pass in int64_t
values that will overflow the range of int and not be detected by this
overflow check.


Yes, you are right.


+error_setg(errp, "'utc_offset' shoud be less than 24 hours");

s/shoud/should/


+return;
+}
+
+if (microseconds > 100) {
+error_setg(errp, "'microseconds' overflow");
+return;
+}
+
+tv.tv_sec = (time_t) seconds + (has_utc_offset ? utc_offset * 60 : 0);
+tv.tv_usec = has_microseconds ? (time_t) microseconds : 0;
+}
+
+
+ret = settimeofday(&tv, NULL);
+if (ret < 0) {
+error_set(errp, QERR_QGA_COMMAND_FAILED, strerror(errno));

Prefer error_setg_errno.


+return;
+}
+
+/* Set the Hardware Clock to the current System Time. */
+pid = fork();
+if (pid == 0) {
+setsid();
+reopen_fd_to_null(0);
+reopen_fd_to_null(1);
+reopen_fd_to_null(2);
+
+execle("/sbin/hwclock", "hwclock", "-w", NULL, environ);
+_exit(EXIT_FAILURE);
+} else if (pid < 0) {
+goto exit_err;
+}
+
+do {
+rpid = waitpid(pid, &status, 0);
+} while (rpid == -1 && errno == EINTR);
+if (rpid == pid && WIFEXITED(status) && !WEXITSTATUS(status)) {
+return;
+}
+
+exit_err:
+error_set(errp, QERR_UNDEFINED_ERROR);

error_setg and a nicer message would be nicer to the end user.


+}
+
  typedef struct GuestFileHandle {
  uint64_t id;
  FILE *fh;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index d067fa5..6eba625 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -121,6 +121,48 @@
'returns': 'TimeInfo' }
  
  ##

+# @guest-set-time:
+#
+# Set guest time. If no arguments were given, will set
+# host time to guest.

I don't see how omitted time arguments can possibly work.  It makes
sense from a management app point of view (tell me a time, and I'll call
guest-set-time with that value on your behalf; or don't tell me a time,
I'll get the host time, and then call guest-set-time with the right


Sorry, I think I am supposed to make it work like this way?


value); but this file is not documenting the management app, but the
JSON interface that the guest sees.  In other words, I think time has to
be a mandatory argument by the time you get this low in the stack.


+#
+# Right now, when a guest is paused or migrated to a file
+# then loaded from that file, the guest OS has no idea that
+# there 

Re: [Qemu-devel] [PATCH for-1.4 1/2] linux-user: Fix cpu_copy() usage

2013-01-29 Thread Andreas Färber
Am 30.01.2013 08:18, schrieb Igor Mammedov:
> On Wed, 30 Jan 2013 01:34:18 +0100
> Andreas Färber  wrote:
> 
>> Commit b4558d7481aefc865b0b52bf9b285ebcf2e8b59f ((x86/Sparc/PPC)-user:
>> fix cpu_copy) added a CPU reset after cpu_copy() inside linux-user code.
>> This reverses the register copying that cpu_copy() does.
>>
>> Clean this up by moving the cpu_reset() call to after cpu_init() but
>> before memcpy(). This matches the initial CPU creation in linux-user.
>>
>> Cc: Blue Swirl 
>> Signed-off-by: Andreas Färber 
>> Cc: Peter Maydell 
>> ---
>>  exec.c   |6 ++
>>  linux-user/syscall.c |3 ---
>>  2 Dateien geändert, 6 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
>>
>> diff --git a/exec.c b/exec.c
>> index b85508b..8dfa458 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -537,6 +537,12 @@ CPUArchState *cpu_copy(CPUArchState *env)
>>  CPUWatchpoint *wp;
>>  #endif
>>  
>> +#ifdef CONFIG_USER_ONLY
> unnecessary ifdef-feniry here, cpu_copy() is linux-user only thing.

It is currently used only by linux-user (and hopefully will stay that
way :)) but I don't see the code limited to CONFIG_USER_ONLY?

Andreas

>> +#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>> +cpu_reset(ENV_GET_CPU(new_env));
>> +#endif
>> +#endif
>> +
>>  memcpy(new_env, env, sizeof(CPUArchState));
>>  
>>  /* Preserve chaining. */
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 693e66f..6c254ba 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4361,9 +4361,6 @@ static int do_fork(CPUArchState *env, unsigned int 
>> flags, abi_ulong newsp,
>>  init_task_state(ts);
>>  /* we create a new CPU instance. */
>>  new_env = cpu_copy(env);
>> -#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>> -cpu_reset(ENV_GET_CPU(new_env));
>> -#endif
>>  /* Init regs that differ from the parent.  */
>>  cpu_clone_regs(new_env, newsp);
>>  new_env->opaque = ts;
>> -- 
>> 1.7.10.4
>>
>>
> Otherwise looks good.

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] vmdk: Allow space in file name

2013-01-29 Thread Philipp Hahn
Hello,

On Tuesday 29 January 2013 22:50:31 Philipp Hahn wrote:
> The previous scanf() format string stopped parsing the file name on the
> first white white space, which seems to be allowed at least by VMware
> Wokrstation.
>
> Change the format string to collect everything between the first and
> second quote as the file name, disallowing line breaks.
>
> Signed-off-by: Philipp Hahn 
Forgot to explicitly cc: the peaople from the first round.

Sincerely
Philipp
-- 
Philipp Hahn   Open Source Software Engineer  h...@univention.de
Univention GmbHbe open.   fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen fax: +49 421 22 232-99
   http://www.univention.de/


signature.asc
Description: This is a digitally signed message part.


Re: [Qemu-devel] [PATCH 1/2] qga: add guest-get-time command

2013-01-29 Thread Lei Li

On 01/29/2013 04:24 AM, Anthony Liguori wrote:

Eric Blake  writes:


On 01/27/2013 11:14 AM, Lei Li wrote:

Signed-off-by: Lei Li 
---
  include/qapi/qmp/qerror.h |3 +++
  qga/commands-posix.c  |   30 ++
  qga/qapi-schema.json  |   38 ++
  3 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..0baf1a4 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -129,6 +129,9 @@ void assert_no_error(Error *err);
  #define QERR_FEATURE_DISABLED \
  ERROR_CLASS_GENERIC_ERROR, "The feature '%s' is not enabled"
  
+#define QERR_GET_TIME_FAILED \

+ERROR_CLASS_GENERIC_ERROR, "Failed to get time"
+

These days, you shouldn't be defining a new QERR wrapper.  Instead,...[1]

  
+static TimeInfo *get_host_time(Error **errp)

This name is unusual...[2]


+{
+int ret;
+qemu_timeval tq;
+TimeInfo *time_info;
+
+ret = qemu_gettimeofday(&tq);
+if (ret < 0) {
+error_set_errno(errp, errno, QERR_GET_TIME_FAILED);

[1]...use the right idiom here:

error_setg_errno(errp, errno, "Failed to get time");


+return NULL;
+}
+
+time_info = g_malloc0(sizeof(TimeInfo));
+time_info->seconds = tq.tv_sec;
+time_info->microseconds = tq.tv_usec;

Is microseconds the right unit to expose through JSON, or are we going
to wish we had exposed nanoseconds down the road (you can still use
qemu_gettimeofday() and scale tv_usec by 1000 before assigning to
time_info->nanoseconds, if we desire the extra granularity).

You aren't setting time_info->utc_offset.  Is that intentional?

Moreover, there's no need to specify microseconds and seconds.  A 64-bit
value for nanoseconds is sufficient.  A signed value can represent
1678 -> 2262.  If anyone is using qemu-ga in 2262, they'll have to
introduce a new command for their quantum emulators :-)

Setting time independent of date is a bit silly because time cannot be
interpreted correctly without a date.

A single value of nanoseconds since the epoch (interpreted as UTC/GMT
time) is really the best strategy.  The host shouldn't be involved in
guest time zones.  In a Cloud environment, it's pretty normal to have
different guests using different time zones.

Regards,

Anthony Liguori


+
+return time_info;
+}
+
+struct TimeInfo *qmp_guest_get_time(Error **errp)
+{
+TimeInfo *time_info = get_host_time(errp);

[2]...given that it is only ever called from qmp_guest_get_time.


+
+if (!time_info) {
+return NULL;
+}

These three lines can be deleted,


+
+return time_info;

since this line will do the same thing if you let NULL time_info get
this far.


+++ b/qga/qapi-schema.json
@@ -83,6 +83,44 @@
  { 'command': 'guest-ping' }
  
  ##

+# @TimeInfo
+#
+# Time Information. It is relative to the Epoch of 1970-01-01.
+#
+# @seconds: "seconds" time unit.
+#
+# @microseconds: "microseconds" time unit.
+#
+# @utc-offset: Information about utc offset. Represented as:
+#  ±[] based at a minimum on minutes, with

s/based at a minimum on//

This still doesn't state whether two hours east of UTC is represented as
120 or as 0200.


It should be 120.
Yeah, I should make it clear.

I am thinking if this 'utc_offset' can be made as a string, represented
like: ±[hh]:[mm], +08:45 (8 hours and 45 minutes) for example.


+#  negative values are west and positive values
+#  are east of UTC. The bounds of @utc-offset is
+#  at most 24 hours away from UTC.
+#
+# Since: 1.4
+##
+{ 'type': 'TimeInfo',
+  'data': { 'seconds': 'int', 'microseconds': 'int',
+'utc-offset': 'int' } }
+
+##
+# @guest-get-time:
+#
+# Get the information about host time in UTC and the

s/host/guest/


+# UTC offset.
+#
+# This command tries to get the host time which is
+# presumably correct, since we need to be able to
+# resynchronize guest clock to host's in guest.

This sentence doesn't make sense.  Better would be:

Get the guest's notion of the current time.


+#
+# Returns: @TimeInfo on success.
+#
+# Since 1.4
+##
+{ 'command': 'guest-get-time',
+  'returns': 'TimeInfo' }
+
+##
  # @GuestAgentCommandInfo:
  #
  # Information about guest agent commands.


--
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



--
Lei




Re: [Qemu-devel] [PATCH 1/2] qga: add guest-get-time command

2013-01-29 Thread Lei Li

On 01/29/2013 02:29 AM, Eric Blake wrote:

On 01/27/2013 11:14 AM, Lei Li wrote:

Signed-off-by: Lei Li 
---
  include/qapi/qmp/qerror.h |3 +++
  qga/commands-posix.c  |   30 ++
  qga/qapi-schema.json  |   38 ++
  3 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..0baf1a4 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -129,6 +129,9 @@ void assert_no_error(Error *err);
  #define QERR_FEATURE_DISABLED \
  ERROR_CLASS_GENERIC_ERROR, "The feature '%s' is not enabled"
  
+#define QERR_GET_TIME_FAILED \

+ERROR_CLASS_GENERIC_ERROR, "Failed to get time"
+

These days, you shouldn't be defining a new QERR wrapper.  Instead,...[1]

  
+static TimeInfo *get_host_time(Error **errp)

This name is unusual...[2]


Okay, I think there might be misunderstanding here.

The current implementation of this 'guest-get-time'
command is trying to get the*  *_host_  time, since the guest
time might be not correct. This qemu_gettimeofday should
return the host time, or am I wrong?


+{
+int ret;
+qemu_timeval tq;
+TimeInfo *time_info;
+
+ret = qemu_gettimeofday(&tq);
+if (ret < 0) {
+error_set_errno(errp, errno, QERR_GET_TIME_FAILED);

[1]...use the right idiom here:

error_setg_errno(errp, errno, "Failed to get time");


+return NULL;
+}
+
+time_info = g_malloc0(sizeof(TimeInfo));
+time_info->seconds = tq.tv_sec;
+time_info->microseconds = tq.tv_usec;

Is microseconds the right unit to expose through JSON, or are we going
to wish we had exposed nanoseconds down the road (you can still use
qemu_gettimeofday() and scale tv_usec by 1000 before assigning to
time_info->nanoseconds, if we desire the extra granularity).

You aren't setting time_info->utc_offset.  Is that intentional?


Yes, since 'guest-get-time' is supposed to get host time in UTC, the utc_offset
is zero. When guest want to set different time zone it can set the
utc_offset through 'guest-set-time'.


+
+return time_info;
+}
+
+struct TimeInfo *qmp_guest_get_time(Error **errp)
+{
+TimeInfo *time_info = get_host_time(errp);

[2]...given that it is only ever called from qmp_guest_get_time.


+
+if (!time_info) {
+return NULL;
+}

These three lines can be deleted,


+
+return time_info;

since this line will do the same thing if you let NULL time_info get
this far.


+++ b/qga/qapi-schema.json
@@ -83,6 +83,44 @@
  { 'command': 'guest-ping' }
  
  ##

+# @TimeInfo
+#
+# Time Information. It is relative to the Epoch of 1970-01-01.
+#
+# @seconds: "seconds" time unit.
+#
+# @microseconds: "microseconds" time unit.
+#
+# @utc-offset: Information about utc offset. Represented as:
+#  ±[] based at a minimum on minutes, with

s/based at a minimum on//

This still doesn't state whether two hours east of UTC is represented as
120 or as 0200.


+#  negative values are west and positive values
+#  are east of UTC. The bounds of @utc-offset is
+#  at most 24 hours away from UTC.
+#
+# Since: 1.4
+##
+{ 'type': 'TimeInfo',
+  'data': { 'seconds': 'int', 'microseconds': 'int',
+'utc-offset': 'int' } }
+
+##
+# @guest-get-time:
+#
+# Get the information about host time in UTC and the

s/host/guest/


+# UTC offset.
+#
+# This command tries to get the host time which is
+# presumably correct, since we need to be able to
+# resynchronize guest clock to host's in guest.

This sentence doesn't make sense.  Better would be:

Get the guest's notion of the current time.


+#
+# Returns: @TimeInfo on success.
+#
+# Since 1.4
+##
+{ 'command': 'guest-get-time',
+  'returns': 'TimeInfo' }
+
+##
  # @GuestAgentCommandInfo:
  #
  # Information about guest agent commands.




--
Lei




Re: [Qemu-devel] [PATCH for-1.4 1/2] linux-user: Fix cpu_copy() usage

2013-01-29 Thread Igor Mammedov
On Wed, 30 Jan 2013 01:34:18 +0100
Andreas Färber  wrote:

> Commit b4558d7481aefc865b0b52bf9b285ebcf2e8b59f ((x86/Sparc/PPC)-user:
> fix cpu_copy) added a CPU reset after cpu_copy() inside linux-user code.
> This reverses the register copying that cpu_copy() does.
> 
> Clean this up by moving the cpu_reset() call to after cpu_init() but
> before memcpy(). This matches the initial CPU creation in linux-user.
> 
> Cc: Blue Swirl 
> Signed-off-by: Andreas Färber 
> Cc: Peter Maydell 
> ---
>  exec.c   |6 ++
>  linux-user/syscall.c |3 ---
>  2 Dateien geändert, 6 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)
> 
> diff --git a/exec.c b/exec.c
> index b85508b..8dfa458 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -537,6 +537,12 @@ CPUArchState *cpu_copy(CPUArchState *env)
>  CPUWatchpoint *wp;
>  #endif
>  
> +#ifdef CONFIG_USER_ONLY
unnecessary ifdef-feniry here, cpu_copy() is linux-user only thing.


> +#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
> +cpu_reset(ENV_GET_CPU(new_env));
> +#endif
> +#endif
> +
>  memcpy(new_env, env, sizeof(CPUArchState));
>  
>  /* Preserve chaining. */
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 693e66f..6c254ba 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4361,9 +4361,6 @@ static int do_fork(CPUArchState *env, unsigned int 
> flags, abi_ulong newsp,
>  init_task_state(ts);
>  /* we create a new CPU instance. */
>  new_env = cpu_copy(env);
> -#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
> -cpu_reset(ENV_GET_CPU(new_env));
> -#endif
>  /* Init regs that differ from the parent.  */
>  cpu_clone_regs(new_env, newsp);
>  new_env->opaque = ts;
> -- 
> 1.7.10.4
> 
> 
Otherwise looks good.

-- 
Regards,
  Igor



Re: [Qemu-devel] [BUG, RFC] block/vmdk.c: File name with space fails to open

2013-01-29 Thread Philipp Hahn
Hello,

On Friday 25 January 2013 09:37:39 Markus Armbruster wrote:
> Suggest to include '\n' in the stop set, like \"%511[^\"\n]\", to better
> detect malformed input.

Will do, also \r.

> > I don't know how portable %[ together with a maximum width is, because
> > the manual page for sscanf() doesn't mention "max width" for "%[", but it
> > works with Debian/GNU Linux Squeeze.
>
> It's fine according to my reading of C89.

Thank you for lloking this up.

> I'm afraid your patch is flawed.  For
>
> RW 1048576 FLAT ""test-f001.vmdk"" 0

You seem to assume " is allowed in the file name, I've assumed that " is not 
allowed, since we don't know the quoting rules for vmdk files, if there are 
any.
That's why I checked our old VMware workstation, which refused to create 
volumes containing !#%^&*><:;'"<>/?
Should we print a warning or error out if a " is detected?

> fname is now "test-f001.vmdk" instead of "\"test-f001.vmdk\"".  That's
> because you change sscanf() to ignore the double-quotes without dropping
> the quote stripping code below.

I'll remove the stripping code.

> Care to post a fixed up patch?

Will do so.

Sincerely
Philipp
-- 
Philipp Hahn   Open Source Software Engineer  h...@univention.de
Univention GmbHbe open.   fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen fax: +49 421 22 232-99
   http://www.univention.de/


signature.asc
Description: This is a digitally signed message part.


[Qemu-devel] [PATCH] vmdk: Allow space in file name

2013-01-29 Thread Philipp Hahn
The previous scanf() format string stopped parsing the file name on the
first white white space, which seems to be allowed at least by VMware
Wokrstation.

Change the format string to collect everything between the first and
second quote as the file name, disallowing line breaks.

Signed-off-by: Philipp Hahn 
---
 block/vmdk.c |   10 +-
 1 files changed, 1 insertions(+), 9 deletions(-)

[V2] Also remove " striping code.
 Add \n\r to stop character set.

diff --git a/block/vmdk.c b/block/vmdk.c
index 19298c2..20ad646 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -641,7 +641,7 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
  * RW [size in sectors] SPARSE "file-name.vmdk"
  */
 flat_offset = -1;
-ret = sscanf(p, "%10s %" SCNd64 " %10s %511s %" SCNd64,
+ret = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64,
 access, §ors, type, fname, &flat_offset);
 if (ret < 4 || strcmp(access, "RW")) {
 goto next_line;
@@ -653,14 +653,6 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 return -EINVAL;
 }
 
-/* trim the quotation marks around */
-if (fname[0] == '"') {
-memmove(fname, fname + 1, strlen(fname));
-if (strlen(fname) <= 1 || fname[strlen(fname) - 1] != '"') {
-return -EINVAL;
-}
-fname[strlen(fname) - 1] = '\0';
-}
 if (sectors <= 0 ||
 (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) ||
 (strcmp(access, "RW"))) {
-- 
1.7.1




Re: [Qemu-devel] [PATCH for-1.4] *-user: Don't reset X86CPU again

2013-01-29 Thread Igor Mammedov
On Wed, 30 Jan 2013 01:38:47 +0100
Andreas Färber  wrote:

> Am 21.01.2013 16:54, schrieb Igor Mammedov:
> > On Sun, 20 Jan 2013 08:39:35 +0100
> > Andreas Färber  wrote:
> > Subj could be more specific, something along the lines:
> >   Fix broken breakpoints duplication for i386-{bds,linux}-user
> > 
> >> Since commit 65dee38052597b6285eb208125369f01b29ba6c1 (target-i386:
> >> move cpu_reset and reset callback to cpu.c) the x86 CPU is reset through
> >> cpu_init() but was still reset immediately after in linux-user and
> >> bsd-user. Similarly it was reset again in linux-user after cpu_copy(),
> >> defeating its very purpose. Clean this up.
> >>
> >> Fixing the ppc and sparc cases of cpu_copy() and overhauling its
> >> implementation is left for another day.
> > Before b55a37c98 cpu_reset() was called at the end of cpu_init() and copying
> > CPUState/duplicating breakpoints afterwards in cpu_copy() worked, BUT commit
> > b4558d7481 breaks it, by positioning cpu_reset() call after copying
> > CPUState/duplicating breakpoints. That should break as minimum breakpoints
> > handling since they all should be duplicated on all CPUs and cpu_reset()
> > deletes them explicitly.
> > 
> > From my POV patch fixes bug introduced by b4558d7481, Perhaps you should
> > update commit message to mention this commit at least and what this patch
> > fixes beside cleanups.
> > 
> > It would be nice though to get confirmation from Blue that all I've said 
> > above
> > is not total nonsense.
> 
> I believe your analysis wrt breakpoints and watchpoints is incorrect
> since the memset() in cpu_reset() handlers only goes up to
> offset(CPUArchState, breakpoints), i.e. not including the breakpoints.
memsetting would lead to leak as minimum, x86_cpu_reset() close to the end
calls cpu_breakpoint_remove_all() and cpu_watchpoint_remove_all() instead. 

> 
> But as discussed with Peter I've posted a v2 that first fixes the reset
> bug and then cleans up the now-superfluous x86 reset.
Cool.

> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 


-- 
Regards,
  Igor



[Qemu-devel] What to do about non-qdevified devices? (was: KVM call minutes 2013-01-29)

2013-01-29 Thread Markus Armbruster
Anthony Liguori  writes:

[...]
> The problems I ran into were (1) this is a lot of work (2) it basically
> requires that all bus children have been qdev/QOM-ified.  Even with
> something like the ISA bus which is where I started, quite a few devices
> were not qdevified still.

So what's the plan to complete the qdevification job?  Lay really low
and quietly hope the problem goes away?  We've tried that for about
three years, doesn't seem to work.

[...]



Re: [Qemu-devel] [PATCH V11 3/4] Use QemuOpts support in block layer

2013-01-29 Thread Dong Xu Wang
于 2013-1-29 17:45, Markus Armbruster 写道:
> Dong Xu Wang  writes:
> 
>> Markus Armbruster  writes:
>>> Dong Xu Wang  writes:
>>>
 Markus Armbruster  writes:
> Dong Xu Wang  writes:
> [...]
>> @@ -264,17 +264,13 @@ static int cow_create(const char *filename, 
>> QEMUOptionParameter *options)
>> int ret;
>> BlockDriverState *cow_bs;
>> 
>> -/* Read out options */
>> -while (options && options->name) {
>> -if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
>> -image_sectors = options->value.n / 512;
>> -} else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
>> -image_filename = options->value.s;
>> -}
>> -options++;
>> +/* Read out opts */
>> +if (opts) {
>
> I suspect you need the if (opts) here and in many other places only
> because you special-cased "both lists empty" in append_opts_list().  I
> suspect things become easier if you drop that.

 No, in this version, if(opt) is needed in "protocol", not needed in
 "format", I want to have the same code style, so I also judged if opts
 is NULL in "format" _create functions. Do you think is it acceptable?
>>>
>>> I still don't get it.  Can you explain how opts can be null here even
>>> when append_opts_list() is changed not to return null?
>>>
>> I mean: When I use protocol, such as gluster:
>> /usr/bin/qemu-img create -f qed -o cluster_size=65536
>> gluster://kvm11/single-volume/12.qed 10M
>>
>> Then opts will be null:
>> qemu_gluster_create (filename=0x55c11930
>> "gluster://kvm11/single-volume/12.qed", opts=0x0) at block/gluster.c:339
>>
>> So it checked if opts is NULL now:
>> 352  if (opts) {
>> 353  total_size =
>> 354  qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) /
>> BDRV_SECTOR_SIZE;
>> 355  }
> 
> I see.
> 
>> I want to make these code in only one kind of style, so I judged if opts
>> is NULL in block format code.
> 
> As far as I can tell, there are just two sources of NULL
> QEMUOptionParameter *:
> 
> 1. parse_option_parameters() returns NULL on error (documented), but
> some callers use it like this without checking for errors:
> 
> param = parse_option_parameters("", create_options, param);
> 
> The only error that can happen then is null create_options.
> Depending on that is slightly unclean.
> 
> Your patch replaces these uses by
> 
> opts = qemu_opts_create_nofail(create_options);
> 
> which cannot return NULL.  I'd call that an improvement.
> 
> 2. qed_create() calls bdrv_create_file(filename, NULL), which passes on
> the null options to bdrv_create().  To get rid of this source, one of
> the two functions needs to create empty options.
> 
> Your patch adds a third source:
> 
> 3. bdrv_img_create() merges drv->create_options and
> proto_drv->create_options (both possibly null) into create_options.
> Unlike the current code, your patch merges two empty lists into null.
> I don't think that's a wise change, and asked you to drop this
> special case in my review of PATCH 2/4.
> 
> [...]

Got it, thank you Markus, I will consider your comments in next version.
> 
> 
> 





Re: [Qemu-devel] [PATCH v4 4/5] sheepdog: use inet_connect to simplify connect code

2013-01-29 Thread MORITA Kazutaka
At Tue, 29 Jan 2013 10:11:44 -0700,
Eric Blake wrote:
> 
> On 01/29/2013 01:56 AM, MORITA Kazutaka wrote:
> > This uses the form ":" for the representation of the
> > sheepdog server to use inet_connect.
> > 
> > Signed-off-by: MORITA Kazutaka 
> > ---
> >  block/sheepdog.c |  111 
> > ++---
> >  1 files changed, 30 insertions(+), 81 deletions(-)
> > 
> 
> >  /* sheepdog[+tcp]://[host:port]/vdiname */
> > -s->addr = g_strdup(uri->server ?: SD_DEFAULT_ADDR);
> > -if (uri->port) {
> > -s->port = g_strdup_printf("%d", uri->port);
> > -} else {
> > -s->port = g_strdup(SD_DEFAULT_PORT);
> > -}
> > +s->host_spec = g_strdup_printf("%s:%d", uri->server ?: SD_DEFAULT_ADDR,
> > +   uri->port ?: SD_DEFAULT_PORT);
> 
> Is this going to properly handle IPv6 addresses, which need to use
> brackets around the host portion, as in:
>  sheepdog+tcp://[::1]:port/vdiname

In such case, uri->server already contains the IPv6 address enclosed
in brackets, so we don't need to enclose it here.

Thanks,

Kazutaka



[Qemu-devel] [Bug 1033727] Re: USB passthrough doesn't work anymore with qemu-kvm 1.1.1

2013-01-29 Thread Serge Hallyn
** Description changed:

+ 
+ SRU Justification:
+ 1. Impact: usb devices which worked with qemu previously stop working
+ 2. Development fix: a patch upstream entitled uhci: Don't queue up packets 
after one with the SPD flag set fixes the problem
+ 3. Stable fix: same patch as development fix.
+ 4. Test case: you must test with certain usb devices - see below for the 
exact kvm arguments to use.
+ 5. Regression potential: this patch is cherrypicked from upstream so should 
be safe.
+ 
+ 
  Hi,
  
  I have a "Bus 006 Device 002: ID 0d46:3003 Kobil Systems GmbH mIDentity Light 
/ KAAN SIM III" (kind of smart card) in an USB port which I make available to a 
Windows XP guest.
  This worked fine with every older qemu-kvm version I've used so far.
  
  But since 1.1.0 it doesn't work anymore.
  The device shows up in the guest, but the software can't access it anymore 
(and the guest is pretty unresponsive).
  
  On the host I get every 2 seconds this message:
  [ 7719.239528] usb 6-1: reset full-speed USB device number 2 using uhci_hcd
  
  Command line options are:
  /usr/bin/kvm
  ...
  -device usb-host,vendorid=0x0d46,productid=0x3003,bus=usb.0,port=3
  ...
  
  When I switch back to qemu-kvm 1.0.1 everything works fine again.
  Any idea what the problem could be?
  
  Thanks
  Klaus

** Changed in: qemu-kvm (Ubuntu Quantal)
   Status: Confirmed => In Progress

** Changed in: qemu-kvm (Ubuntu Quantal)
 Assignee: Serge Hallyn (serge-hallyn) => (unassigned)

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

Title:
  USB passthrough doesn't work anymore with qemu-kvm 1.1.1

Status in QEMU:
  Fix Released
Status in “qemu” package in Ubuntu:
  Fix Released
Status in “qemu-kvm” package in Ubuntu:
  Invalid
Status in “qemu-kvm” source package in Quantal:
  In Progress
Status in “qemu” source package in Raring:
  Fix Released
Status in “qemu-kvm” package in Debian:
  Fix Released

Bug description:
  
  SRU Justification:
  1. Impact: usb devices which worked with qemu previously stop working
  2. Development fix: a patch upstream entitled uhci: Don't queue up packets 
after one with the SPD flag set fixes the problem
  3. Stable fix: same patch as development fix.
  4. Test case: you must test with certain usb devices - see below for the 
exact kvm arguments to use.
  5. Regression potential: this patch is cherrypicked from upstream so should 
be safe.
  

  Hi,

  I have a "Bus 006 Device 002: ID 0d46:3003 Kobil Systems GmbH mIDentity Light 
/ KAAN SIM III" (kind of smart card) in an USB port which I make available to a 
Windows XP guest.
  This worked fine with every older qemu-kvm version I've used so far.

  But since 1.1.0 it doesn't work anymore.
  The device shows up in the guest, but the software can't access it anymore 
(and the guest is pretty unresponsive).

  On the host I get every 2 seconds this message:
  [ 7719.239528] usb 6-1: reset full-speed USB device number 2 using uhci_hcd

  Command line options are:
  /usr/bin/kvm
  ...
  -device usb-host,vendorid=0x0d46,productid=0x3003,bus=usb.0,port=3
  ...

  When I switch back to qemu-kvm 1.0.1 everything works fine again.
  Any idea what the problem could be?

  Thanks
  Klaus

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1033727/+subscriptions



Re: [Qemu-devel] [qemu-devel]The problem of QMP command getfd.

2013-01-29 Thread Li Zhang

On 2013年01月29日 00:30, Eric Blake wrote:

On 01/28/2013 07:16 AM, Li Zhang wrote:

The QMP client must send '{ "execute": "getfd", "arguments": { "fdname":
"fd1" } }' together with SCM_RIGHTS CMSG.  You are getting the error
because you sent the "getfd" command but forgot to include a file
descriptor using SCM_RIGHTS.

SCM_RIGHTS is a feature of UNIX domain sockets.  It allows one process
to pass a file descriptor to another process through the UNIX domain
socket.  See "man 7 unix" and "man 3 cmsg" for details.

You can also look at how libvirt uses this code, for an example of what
it takes to use SCM_RIGHTS.

You are right, I saw that in libvirt.
Thanks for your suggestions.

Look at monitor.c:qmp_getfd() to understand how this works.  The QEMU
code to receive a passed file descriptor is in
qemu-char.c:unix_process_msgfd().

Thanks a lot for your detailed explanation and suggestions.
I will look into the code to understand how it works.

Additionally, using the 'getfd' command is not very robust; new code
should be preferring to use the 'add-fd' command family, as fdsets are
less prone to leaked fds; this also uses SCM_RIGHTS.  However, libvirt
does not yet have example code for using 'add-fd' (I'm slowly working on
adding that).

Nice. Thanks for your information. :)







Re: [Qemu-devel] [RFC PATCH RDMA support v1: 1/5] add openfabrics RDMA libraries and base RDMA code to build

2013-01-29 Thread Anthony Liguori
Andreas Färber  writes:

> Hello,
>
> Am 28.01.2013 23:01, schrieb mrhi...@linux.vnet.ibm.com:
>> From: "Michael R. Hines" 
>> 
>> 
>> Signed-off-by: Michael R. Hines 
>> ---
>>  Makefile.target |5 +-
>>  include/qemu/rdma.h |  249 ++
>>  qemu-rdma.c | 1357 
>> +++
>>  3 files changed, 1609 insertions(+), 2 deletions(-)
>>  create mode 100644 include/qemu/rdma.h
>>  create mode 100644 qemu-rdma.c
>
> This series is missing a cover letter with explanations, starting with
> the acronym and its purpose.

Indeed.  A bit of an introduction of what problem the series solves is
in order.

Can you share performance data with and without RMDA?

> Further, the commit messages are lacking
> descriptions. Any reason that the code is GPLv2 rather than GPLv2+?

I assume copy/paste.  Michael, unless you had an explicit reason to use
GPLv2 only, we now prefer GPLv2+.

Regards,

Anthony Liguori
>
> Regards,
> Andreas
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg




Re: [Qemu-devel] [PATCH for-1.4] libqtest: Wait for the right child PID after killing QEMU

2013-01-29 Thread Anthony Liguori
Andreas Färber  writes:

> Am 28.01.2013 19:15, schrieb Eduardo Habkost:
>> When running "make check" with gcov enabled, we get the following
>> message:
>> 
>>hw/tmp105.gcda:cannot open data file, assuming not executed
>> 
>> The problem happens because:
>> 
>>  * tmp105-test exits before QEMU exits, because waitpid() at
>>qtest_quit() fails;
>>  * waitpid() fails because there's another process already
>>waiting for the QEMU process;
>>  * The process that is already waiting for QEMU is the child created by
>>qtest_init() to run system();
>>  * qtest_quit() is incorrectly waiting for the QEMU PID directly instead
>>of the child created by qtest_init().
>> 
>> This fixes the problem by sending SIGTERM to QEMU, but waiting for the
>> child process created by qtest_init() (that exits immediately after QEMU
>> exits).
>> 
>> Reported-by: Andreas Färber 
>> Signed-off-by: Eduardo Habkost 
>
> Thanks. Still need to test this...
>
> Anthony, might this by any chance fix your sparc 100% CPU issue?

I don't see how.  How would this impact the QEMU process?

Regards,

Anthony Liguori


>
> Andreas
>
>> ---
>>  tests/libqtest.c | 7 +--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index 913fa05..762dec4 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -39,7 +39,8 @@ struct QTestState
>>  int qmp_fd;
>>  bool irq_level[MAX_IRQ];
>>  GString *rx;
>> -gchar *pid_file;
>> +gchar *pid_file; /* QEMU PID file */
>> +int child_pid;   /* Child process created to execute QEMU */
>>  char *socket_path, *qmp_socket_path;
>>  };
>>  
>> @@ -144,6 +145,7 @@ QTestState *qtest_init(const char *extra_args)
>>  
>>  s->rx = g_string_new("");
>>  s->pid_file = pid_file;
>> +s->child_pid = pid;
>>  for (i = 0; i < MAX_IRQ; i++) {
>>  s->irq_level[i] = false;
>>  }
>> @@ -165,8 +167,9 @@ void qtest_quit(QTestState *s)
>>  
>>  pid_t pid = qtest_qemu_pid(s);
>>  if (pid != -1) {
>> +/* kill QEMU, but wait for the child created by us to run system() 
>> */
>>  kill(pid, SIGTERM);
>> -waitpid(pid, &status, 0);
>> +waitpid(s->child_pid, &status, 0);
>>  }
>>  
>>  unlink(s->pid_file);
>> 
>
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] a probem about qemu doesn't support for setting ma

2013-01-29 Thread liequan . che
 Hi  Paolo:
 I recompile the qemu-kvm-0.12.1.2-2.295.el6.10.src.rpm 
with --enable-mixemu.
It makes no effect yet. 
Did the new version 1.3.1 slove this problem?

Best Regards!


- Original Message -
From: Paolo Bonzini 
To: liequan@i-soft.com.cn
Cc: qemu-devel 
Subject: 
Re: [Qemu-devel] a probem about qemu doesn't support for setting master volume/mute when guest is winxp
Date: 13-01-30 02:02:53

Il 29/01/2013 08:35, liequan@i-soft.com.cn ha scritto:

>  Hi all:

>  I found a bug that qemu doesn't support for setting master volume/mute

> when guest is winxp. but the wave volume in the volume control works

> fine.I don't know why it does.

>   Any help is welcome.

>   Best Regards!



You probably need to configure with --enable-mixemu.



Paolo





Re: [Qemu-devel] [RFC PATCH RDMA support v1: 1/5] add openfabrics RDMA libraries and base RDMA code to build

2013-01-29 Thread Andreas Färber
Hello,

Am 28.01.2013 23:01, schrieb mrhi...@linux.vnet.ibm.com:
> From: "Michael R. Hines" 
> 
> 
> Signed-off-by: Michael R. Hines 
> ---
>  Makefile.target |5 +-
>  include/qemu/rdma.h |  249 ++
>  qemu-rdma.c | 1357 
> +++
>  3 files changed, 1609 insertions(+), 2 deletions(-)
>  create mode 100644 include/qemu/rdma.h
>  create mode 100644 qemu-rdma.c

This series is missing a cover letter with explanations, starting with
the acronym and its purpose. Further, the commit messages are lacking
descriptions. Any reason that the code is GPLv2 rather than GPLv2+?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH v2] target-arm: add Faraday ARMv5TE processors support

2013-01-29 Thread Kuo-Jung Su
Hi Peter:
Thanks for the comments.
I'll double if the ARM_FEATURE_TCM is indetical to Faraday's
I/D-scrtachpad,
, add the missing MPU functions for FT606TE, and avoid using CP_ANY in next
patch.

Best Wishes
Dante


2013/1/28 Peter Maydell 

> On 25 January 2013 07:18, Kuo-Jung Su  wrote:
> > +static const ARMCPRegInfo faraday_cp_reginfo[] = {
> > +/*
> > + * Auxiliary Control Register
> > + * - Bit 4 STM aligned transfer for AXI
> > + * - Bit 3 Cache allocation configuration enable
> > + * - Bit 2 Static branch prediction enable
> > + * - Bit 1 Dynamic branch prediction enable
> > + * - Bit 0 Return stack enable
> > + */
> > +{ .name = "AUXCTR", .cp = 15, .crn = 1, .crm = 0,
> > +  .opc1 = 0, .opc2 = 1, .access = PL1_RW, .resetvalue = 0x7,
> > +  .fieldoffset = offsetof(CPUARMState, cp15.c15_aux_control) },
>
> This is just an AUXCR in the usual encoding for one;
> you should use ARM_FEATURE_AUXCR (and set cpu->reset_auxcr
> in the cpu initfns).
>
> > +/*
> > + * I/D-Scratchpad Configuration Register
> > + * - Bits[31:10] Scratchpad base address
> > + * - Bits[5:2]   Scratchpad size
> > + * - Bit 1   Scratchpad self-loading capability
> > + * - Bit 0   Scratchpad enable
> > + */
> > +{ .name = "IDSCFG", .cp = 15, .crn = 9, .crm = 1,
> > +  .opc1 = 0, .opc2 = CP_ANY, .access = PL1_RW, .resetvalue = 0x0,
> > +  .fieldoffset = offsetof(CPUARMState, cp15.c15_ids_config) },
>
> Looking at the documentation you've provided, this encoding
> doesn't seem to be used on the FA606TE.
>
> It only seems to be a scratchpad register for the FA626TE.
> For the FA616TE and FA726TE it is a TCM config register,
> and for the FA606TE it doesn't exist at all. (Possibly
> the "scratchpad" on the 626 is actually a TCM under another
> name; I didn't look too closely but there seems to be some
> similarity...)
>
> The 726TE datasheet also talks about a c9, c1, 0, 1 encoding
> for the data TCM config (with c9, c1, 0, 0 being the insn TCM
> config). This doesn't fit with a CP_ANY value for opc2.
> It also sounds pretty much like the TCM registers for the
> ARM926, ARM946 and ARM1026, for instance.
>
> I think you need to check to what extent these registers
> are different between the four cores you're trying to
> implement, and whether any of them match up with TCM
> implementations in the standard ARM cores. Where there's
> a register set shared between ARM and Faraday implementations
> that probably should have an ARM_FEATURE_TCM flag.
>
> Also you should double check which if any cores really
> have a CP_ANY wildcard decode for opc2: I don't want
> us to underdecode unless we know the hardware really
> underdecodes [ie that working guest code fails unless
> we use CP_ANY]. (Some existing code in qemu uses CP_ANY
> because we have historically done that and don't have
> easy ability to check it won't break currently working
> guests; I would prefer us not to let in new examples
> without scrutiny, though.)
>
> thanks
> -- PMM
>



-- 
Best wishes,
Kuo-Jung Su


Re: [Qemu-devel] [PATCH 2/2] usb-ehci: add Faraday FUSBH200 support

2013-01-29 Thread Kuo-Jung Su
2013/1/29 Andreas Färber 

> Am 29.01.2013 06:43, schrieb Kuo-Jung Su:
> > From: Kuo-Jung Su 
> >
> > Add Faraday FUSBH200 support, which is slightly different from EHCI spec.
> > (Or maybe simply a bad/wrong implementation...)
> >
> > Signed-off-by: Kuo-Jung Su 
> > Cc: Gerd Hoffmann 
> > Cc: Andreas 
> > Cc: Peter Crosthwaite 
> > ---
> >  hw/usb/hcd-ehci-sysbus.c |   66
> ++
> >  hw/usb/hcd-ehci.h|5 
> >  2 files changed, 71 insertions(+)
> >
> > diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
> > index ae2db1a..404a227 100644
> > --- a/hw/usb/hcd-ehci-sysbus.c
> > +++ b/hw/usb/hcd-ehci-sysbus.c
> > @@ -17,6 +17,49 @@
> >
> >  #include "hw/usb/hcd-ehci.h"
> >
> > +/*
> > + * Faraday FUSBH200 USB 2.0 EHCI
> > + */
> > +
> > +static uint64_t
> > +ehci_fusbh200_read(void *ptr, hwaddr addr, unsigned size)
> > +{
> > +hwaddr off = 0x34 + addr;
> > +
> > +switch (off) {
> > +case 0x34:  /* fusbh200: EOF/Async. Sleep Timer Register */
> > +return 0x0041;
> > +case 0x40:  /* fusbh200: Bus Monitor Control/Status Register */
> > +/* High-Speed, VBUS valid, interrupt level-high active */
> > +return (2 << 9) | (1 << 8) | (1 << 3);
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static void
> > +ehci_fusbh200_write(void *ptr, hwaddr addr, uint64_t val, unsigned size)
> > +{
> > +}
> > +
> > +static const MemoryRegionOps ehci_mmio_fusbh200_ops = {
> > +.read = ehci_fusbh200_read,
> > +.write = ehci_fusbh200_write,
> > +.valid.min_access_size = 4,
> > +.valid.max_access_size = 4,
> > +.endianness = DEVICE_LITTLE_ENDIAN,
> > +};
>
> This part looks okay now, except that I'd suggest to name the ops
> ehci_fusbh200_mmio_ops (ehci_fusbh200_ being a consistent prefix).
>
>
Sure, consider it done.


> > +
> > +static void
> > +usb_ehci_fusbh200_initfn(EHCIState *s, DeviceState *dev)
>
> Bad naming, the inconsistent use of "initfn" is what my patch tried to
> clean up for you.
> And this signature is only necessary in common EHCI code because it
> needs to access EHCIState from both PCIDevice and SysBusDevice. This
> change is in SysBus-only code though, so you can access EHCIState
> through the device state.
>
>
My bad, because I didn't know what's the difference between realizefn and
initfn,
so I did it in a wrong way.


> > +{
> > +memory_region_init_io(&s->mem_vendor, &ehci_mmio_fusbh200_ops, s,
> > +  "fusbh200", 0x4c);
>
> This has no dependencies on other fields, so it should go into a
> void ehci_fusbh200_initfn(Object *obj) hooked to your .instance_init.
>
> > +memory_region_add_subregion(&s->mem,
> > +s->opregbase + s->portscbase + 4 *
> s->portnr,
> > +&s->mem_vendor);
> > +}
> > +
> >  static const VMStateDescription vmstate_ehci_sysbus = {
> >  .name= "ehci-sysbus",
> >  .version_id  = 2,
> > @@ -46,6 +89,9 @@ static void usb_ehci_sysbus_realizefn(DeviceState
> *dev, Error **errp)
> >  s->dma = &dma_context_memory;
> >
> >  usb_ehci_initfn(s, dev);
>
> FWIW I notice that I should've renamed this in my patch, too (outside
> the scope of your patch).
>
> > +if (sec->vendor_init) {
> > +sec->vendor_init(s, DEVICE(dev));
> > +}
> >  sysbus_init_irq(d, &s->irq);
> >  sysbus_init_mmio(d, &s->mem);
> >  }
>
> This is not what I had in mind. For one thing your initialization does
> not need to go before sysbus_init_{irq,mmio}.
>
> What I am not too sure about without digging out SysBus and PCI EHCI
> sources is at which point the fields may get modified. In 1/2 your new
> fields are only ever initialized in class_init and then transferred to
> instance state in the realizefn.
>
> There's two better options:
>
> 1) Change 1/2 to set the default values s->... in an instance_init
> initfn directly, then your instance_init can execute "on top" (with QOM
> infrastructure taking care of calling the parent's initfn before yours)
> and overwrite its values.
> You won't need sec->portscbase and sec->portnr then and we avoid
> duplicating "sec->portscbase = 0x44; sec->portnr = NB_PORTS;".
>
> 2) Keep 1/2 as is and override the realizefn in 2/2 via dc->realize =
> your_realizefn. This involves saving the parent's realizefn in a new
> FUSHBH200SysBusEHCIClass::parent_realize and explicitly invoking that
> parent_realize function before adding your MemoryRegion as subregion.
>
> The big underlying question is whether other future devices may want to
> override the values via qdev static properties. Then the assignment
> needs to remain in realizefn, otherwise initfn makes our life easier.
>

No problem,  consider it done.


> > @@ -76,6 +122,7 @@ static void ehci_xlnx_class_init(ObjectClass *oc,
> void *data)
> >  sec->opregbase = 0x140;
> >  sec->portscbase = 0x44;
> >  sec->portnr = NB_PORTS;
> > +sec->vendor_init = NULL;
> >  }

Re: [Qemu-devel] [PATCH for-1.4] libqtest: Wait for the right child PID after killing QEMU

2013-01-29 Thread Andreas Färber
Am 28.01.2013 19:15, schrieb Eduardo Habkost:
> When running "make check" with gcov enabled, we get the following
> message:
> 
>hw/tmp105.gcda:cannot open data file, assuming not executed
> 
> The problem happens because:
> 
>  * tmp105-test exits before QEMU exits, because waitpid() at
>qtest_quit() fails;
>  * waitpid() fails because there's another process already
>waiting for the QEMU process;
>  * The process that is already waiting for QEMU is the child created by
>qtest_init() to run system();
>  * qtest_quit() is incorrectly waiting for the QEMU PID directly instead
>of the child created by qtest_init().
> 
> This fixes the problem by sending SIGTERM to QEMU, but waiting for the
> child process created by qtest_init() (that exits immediately after QEMU
> exits).
> 
> Reported-by: Andreas Färber 
> Signed-off-by: Eduardo Habkost 

Thanks. Still need to test this...

Anthony, might this by any chance fix your sparc 100% CPU issue?

Andreas

> ---
>  tests/libqtest.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 913fa05..762dec4 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -39,7 +39,8 @@ struct QTestState
>  int qmp_fd;
>  bool irq_level[MAX_IRQ];
>  GString *rx;
> -gchar *pid_file;
> +gchar *pid_file; /* QEMU PID file */
> +int child_pid;   /* Child process created to execute QEMU */
>  char *socket_path, *qmp_socket_path;
>  };
>  
> @@ -144,6 +145,7 @@ QTestState *qtest_init(const char *extra_args)
>  
>  s->rx = g_string_new("");
>  s->pid_file = pid_file;
> +s->child_pid = pid;
>  for (i = 0; i < MAX_IRQ; i++) {
>  s->irq_level[i] = false;
>  }
> @@ -165,8 +167,9 @@ void qtest_quit(QTestState *s)
>  
>  pid_t pid = qtest_qemu_pid(s);
>  if (pid != -1) {
> +/* kill QEMU, but wait for the child created by us to run system() */
>  kill(pid, SIGTERM);
> -waitpid(pid, &status, 0);
> +waitpid(s->child_pid, &status, 0);
>  }
>  
>  unlink(s->pid_file);
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH for-1.4] *-user: Don't reset X86CPU again

2013-01-29 Thread Andreas Färber
Am 21.01.2013 16:54, schrieb Igor Mammedov:
> On Sun, 20 Jan 2013 08:39:35 +0100
> Andreas Färber  wrote:
> Subj could be more specific, something along the lines:
>   Fix broken breakpoints duplication for i386-{bds,linux}-user
> 
>> Since commit 65dee38052597b6285eb208125369f01b29ba6c1 (target-i386:
>> move cpu_reset and reset callback to cpu.c) the x86 CPU is reset through
>> cpu_init() but was still reset immediately after in linux-user and
>> bsd-user. Similarly it was reset again in linux-user after cpu_copy(),
>> defeating its very purpose. Clean this up.
>>
>> Fixing the ppc and sparc cases of cpu_copy() and overhauling its
>> implementation is left for another day.
> Before b55a37c98 cpu_reset() was called at the end of cpu_init() and copying
> CPUState/duplicating breakpoints afterwards in cpu_copy() worked, BUT commit
> b4558d7481 breaks it, by positioning cpu_reset() call after copying
> CPUState/duplicating breakpoints. That should break as minimum breakpoints
> handling since they all should be duplicated on all CPUs and cpu_reset()
> deletes them explicitly.
> 
> From my POV patch fixes bug introduced by b4558d7481, Perhaps you should
> update commit message to mention this commit at least and what this patch
> fixes beside cleanups.
> 
> It would be nice though to get confirmation from Blue that all I've said above
> is not total nonsense.

I believe your analysis wrt breakpoints and watchpoints is incorrect
since the memset() in cpu_reset() handlers only goes up to
offset(CPUArchState, breakpoints), i.e. not including the breakpoints.

But as discussed with Peter I've posted a v2 that first fixes the reset
bug and then cleans up the now-superfluous x86 reset.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH for-1.4 2/2] linux-user: bsd-user: Don't reset X86CPU twice

2013-01-29 Thread Andreas Färber
Since commit 65dee38052597b6285eb208125369f01b29ba6c1 (target-i386:
move cpu_reset and reset callback to cpu.c) the x86 CPU is reset through
cpu_init() but was still reset immediately after in linux-user and
bsd-user. Similarly it was reset as part of cpu_copy(). Clean this up.

Cc: Igor Mammedov 
Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 bsd-user/main.c   |2 +-
 exec.c|2 +-
 linux-user/main.c |2 +-
 3 Dateien geändert, 3 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 1dc0330..ae24723 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -917,7 +917,7 @@ int main(int argc, char **argv)
 fprintf(stderr, "Unable to find CPU definition\n");
 exit(1);
 }
-#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
+#if defined(TARGET_SPARC) || defined(TARGET_PPC)
 cpu_reset(ENV_GET_CPU(env));
 #endif
 thread_env = env;
diff --git a/exec.c b/exec.c
index 8dfa458..8ac98f0 100644
--- a/exec.c
+++ b/exec.c
@@ -538,7 +538,7 @@ CPUArchState *cpu_copy(CPUArchState *env)
 #endif
 
 #ifdef CONFIG_USER_ONLY
-#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
+#if defined(TARGET_SPARC) || defined(TARGET_PPC)
 cpu_reset(ENV_GET_CPU(new_env));
 #endif
 #endif
diff --git a/linux-user/main.c b/linux-user/main.c
index 0181bc2..3df8aa2 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3540,7 +3540,7 @@ int main(int argc, char **argv, char **envp)
 fprintf(stderr, "Unable to find CPU definition\n");
 exit(1);
 }
-#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
+#if defined(TARGET_SPARC) || defined(TARGET_PPC)
 cpu_reset(ENV_GET_CPU(env));
 #endif
 
-- 
1.7.10.4




[Qemu-devel] [PATCH for-1.4 1/2] linux-user: Fix cpu_copy() usage

2013-01-29 Thread Andreas Färber
Commit b4558d7481aefc865b0b52bf9b285ebcf2e8b59f ((x86/Sparc/PPC)-user:
fix cpu_copy) added a CPU reset after cpu_copy() inside linux-user code.
This reverses the register copying that cpu_copy() does.

Clean this up by moving the cpu_reset() call to after cpu_init() but
before memcpy(). This matches the initial CPU creation in linux-user.

Cc: Blue Swirl 
Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 exec.c   |6 ++
 linux-user/syscall.c |3 ---
 2 Dateien geändert, 6 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/exec.c b/exec.c
index b85508b..8dfa458 100644
--- a/exec.c
+++ b/exec.c
@@ -537,6 +537,12 @@ CPUArchState *cpu_copy(CPUArchState *env)
 CPUWatchpoint *wp;
 #endif
 
+#ifdef CONFIG_USER_ONLY
+#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
+cpu_reset(ENV_GET_CPU(new_env));
+#endif
+#endif
+
 memcpy(new_env, env, sizeof(CPUArchState));
 
 /* Preserve chaining. */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 693e66f..6c254ba 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4361,9 +4361,6 @@ static int do_fork(CPUArchState *env, unsigned int flags, 
abi_ulong newsp,
 init_task_state(ts);
 /* we create a new CPU instance. */
 new_env = cpu_copy(env);
-#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
-cpu_reset(ENV_GET_CPU(new_env));
-#endif
 /* Init regs that differ from the parent.  */
 cpu_clone_regs(new_env, newsp);
 new_env->opaque = ts;
-- 
1.7.10.4




[Qemu-devel] [PATCH for-1.4 0/2] linux-user/bsd-user cpu_reset() cleanups

2013-01-29 Thread Andreas Färber
Hello,

Here's an extended version of my cpu_reset() cleanup for x86, prompted by a
discussion about the commit message:
v2 first generally cleans up cpu_copy() by moving cpu_reset() into cpu_copy()
before dropping x86 cpu_reset(). Rebased onto master.

Beyond the scope of this 1.4 series is replacing cpu_copy() with linux-user
code copying individual registers according to Linux ABI in place of memcpy().

Regards,
Andreas

v1 -> v2:
* Prepend patch to move cpu_reset() into cpu_copy() to fix ppc and sparc, too.

Cc: Riku Voipio 
Cc: Blue Swirl 
Cc: Igor Mammedov 
Cc: Peter Maydell 

Andreas Färber (2):
  linux-user: Fix cpu_copy() usage
  linux-user: bsd-user: Don't reset X86CPU twice

 bsd-user/main.c  |2 +-
 exec.c   |6 ++
 linux-user/main.c|2 +-
 linux-user/syscall.c |3 ---
 4 Dateien geändert, 8 Zeilen hinzugefügt(+), 5 Zeilen entfernt(-)

-- 
1.7.10.4




Re: [Qemu-devel] [PULL] virtio,make,pci,e1000

2013-01-29 Thread Anthony Liguori
Andreas Färber  writes:

> Am 30.01.2013 00:28, schrieb Michael S. Tsirkin:
>> On Tue, Jan 29, 2013 at 04:54:35PM -0600, Anthony Liguori wrote:
>>> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
>>> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
>>> GTester: last random seed: R02S4b841f7154f9fe3d170e284ce7d3a801
>>> make: *** [check-qtest-i386] Error 1
>> 
>> Nasty turns out for pci branch buildbot does not make check.
>> Anyone knows how to fix this?
>
> The KVM call agenda thread has a link to a GitHub repository with the
> buildbot config (Python file) for us to propose changes.

I looked earlier and it appears that all submaintainer trees are lacking
an addStep(Test(...) command.  Does anyone know if this was a oversight
when test support was added to buildbot or was there a reasonf or this?

Regards,

Anthony Liguori

>
> Andreas
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [Bug 1033727] Re: USB passthrough doesn't work anymore with qemu-kvm 1.1.1

2013-01-29 Thread Serge Hallyn
** No longer affects: qemu-kvm (Ubuntu Raring)

** Changed in: qemu-kvm (Ubuntu Quantal)
 Assignee: (unassigned) => Serge Hallyn (serge-hallyn)

** No longer affects: qemu (Ubuntu Quantal)

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

Title:
  USB passthrough doesn't work anymore with qemu-kvm 1.1.1

Status in QEMU:
  Fix Released
Status in “qemu” package in Ubuntu:
  Fix Released
Status in “qemu-kvm” package in Ubuntu:
  Invalid
Status in “qemu-kvm” source package in Quantal:
  Confirmed
Status in “qemu” source package in Raring:
  Fix Released
Status in “qemu-kvm” package in Debian:
  Fix Released

Bug description:
  Hi,

  I have a "Bus 006 Device 002: ID 0d46:3003 Kobil Systems GmbH mIDentity Light 
/ KAAN SIM III" (kind of smart card) in an USB port which I make available to a 
Windows XP guest.
  This worked fine with every older qemu-kvm version I've used so far.

  But since 1.1.0 it doesn't work anymore.
  The device shows up in the guest, but the software can't access it anymore 
(and the guest is pretty unresponsive).

  On the host I get every 2 seconds this message:
  [ 7719.239528] usb 6-1: reset full-speed USB device number 2 using uhci_hcd

  Command line options are:
  /usr/bin/kvm
  ...
  -device usb-host,vendorid=0x0d46,productid=0x3003,bus=usb.0,port=3
  ...

  When I switch back to qemu-kvm 1.0.1 everything works fine again.
  Any idea what the problem could be?

  Thanks
  Klaus

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1033727/+subscriptions



Re: [Qemu-devel] [PULL] virtio,make,pci,e1000

2013-01-29 Thread Andreas Färber
Am 30.01.2013 00:28, schrieb Michael S. Tsirkin:
> On Tue, Jan 29, 2013 at 04:54:35PM -0600, Anthony Liguori wrote:
>> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
>> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
>> GTester: last random seed: R02S4b841f7154f9fe3d170e284ce7d3a801
>> make: *** [check-qtest-i386] Error 1
> 
> Nasty turns out for pci branch buildbot does not make check.
> Anyone knows how to fix this?

The KVM call agenda thread has a link to a GitHub repository with the
buildbot config (Python file) for us to propose changes.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 4/7 v2] KVM regsync: Add register bitmap parameter to do_kvm_cpu_synchronize_state

2013-01-29 Thread Marcelo Tosatti
On Mon, Jan 28, 2013 at 12:49:26PM -0500, Jason J. Herne wrote:
> On 01/24/2013 07:01 PM, Marcelo Tosatti wrote:
> >On Thu, Jan 24, 2013 at 06:44:50PM -0200, Marcelo Tosatti wrote:
> >
> >What 'subtle errors' are you thinking of?
> >
> >It should be easy to convert as its greppable.
> >
> >>S/390 not synchronizing the env-> copy of the FULL register set is still
> >>a bug, though (because the FULL set is what "cpu_synchronize_state" with
> >>no parameter implies).
> >
> >
> >
> 
> There seems to be a lot of good discussion going on regarding these
> changes.  Have we reached some kind of consensus on how we want to
> proceed?
> If I understand the arguments being made... this patch set, as
> submitted, is wrong because KVM_ARCH_GET_REGS should always retrieve
> runtime regs only?  

Because only registers on the runtime set should be written
(userspace->kernel direction) during runtime.

All registers must be read (kernel->userspace direction) on
cpu_synchronize_state(void).

> Or must it sync everything?  If there is not a
> clear answer here we'll need to decide this before going anywhere
> with this patch, yes?
> 
> Alex wrote:
> """
> I think for now the best choice for get_regs() would be to ignore
> the FULL/RESET bits and always keep the syncing as it happens today
> under the RUNTIME umbrella only. So all of get_regs() only checks
> for RUNTIME.

This makes this 

1. cpu_synchronize_state()
2. read env->env which is part of RESET or FULL set but not RUNTIME set

sequence invalid (such as 'info regs'). Perhaps on S/390 its not a
problem.

> Whenever get_xxx() happens, a bit gets set for set_xxx(). Up to this
> point, only the RUNTIME bit is ever set, because that's what
> cpu_synchronize_registers() sets.
> 
> Then s390 can add special separate bits for "sync GPRs" and "sync
> CRs". SYNC_RUNTIME would include those bits. The kvm hypercall exit
> calls a new synchronize_registers() function with a parameter
> telling it to only sync GPRs. This marks GPRs dirty, but not
> RUNTIME. The set_registers() function in s390 specific code could
> handle this particular case specially.
> 
> That way everything's solved and scalable, no?
> """
> 
> This is the most comprehensive suggestion I've been able to pick out
> of the discussion. 

This is fine.

> However, Marcelo has expressed concern over S390
> not updating everything in env:
> 
> """
> S/390 not synchronizing the env-> copy of the FULL register set is still
> a bug, though (because the FULL set is what "cpu_synchronize_state" with
> no parameter implies).
> """

Since cpu_synchronize_state() does not take a parameter, it is
expected to synchronize all registers (FULL/RESET/RUNTIME sets).

> 
> Also Alex, what did you mean by get_xxx() and set_xxx()?
> 
> -- 
> -- Jason J. Herne (jjhe...@linux.vnet.ibm.com)



[Qemu-devel] [PATCH v2] vmdk: Allow selecting SCSI adapter in image creation

2013-01-29 Thread Othmar Pasteka
Introduce a new option "adapter_type" when converting to vmdk images.
It can be one of the following: ide (default), buslogic, lsilogic
or legacyESX (according to the vmdk spec from vmware).

In case of a non-ide adapter, heads is set to 255 instead of the 16.
The latter is used for "ide".

Also see LP#545089

Signed-off-by: Othmar Pasteka 
---

v2: indentation fix

 block/vmdk.c  |   31 ---
 include/block/block_int.h |1 +
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 8333afb..a8cb5c9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1442,6 +1442,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 int fd, idx = 0;
 char desc[BUF_SIZE];
 int64_t total_size = 0, filesize;
+const char *adapter_type = NULL;
 const char *backing_file = NULL;
 const char *fmt = NULL;
 int flags = 0;
@@ -1453,6 +1454,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 const char *desc_extent_line;
 char parent_desc_line[BUF_SIZE] = "";
 uint32_t parent_cid = 0x;
+uint32_t number_heads = 16;
 const char desc_template[] =
 "# Disk DescriptorFile\n"
 "version=1\n"
@@ -1469,9 +1471,9 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 "\n"
 "ddb.virtualHWVersion = \"%d\"\n"
 "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
-"ddb.geometry.heads = \"16\"\n"
+"ddb.geometry.heads = \"%d\"\n"
 "ddb.geometry.sectors = \"63\"\n"
-"ddb.adapterType = \"ide\"\n";
+"ddb.adapterType = \"%s\"\n";
 
 if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
 return -EINVAL;
@@ -1480,6 +1482,8 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 while (options && options->name) {
 if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
 total_size = options->value.n;
+} else if (!strcmp(options->name, BLOCK_OPT_ADAPTER_TYPE)) {
+adapter_type = options->value.s;
 } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
 backing_file = options->value.s;
 } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) {
@@ -1489,6 +1493,20 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 }
 options++;
 }
+if (!adapter_type) {
+adapter_type = "ide";
+} else if (strcmp(adapter_type, "ide") &&
+   strcmp(adapter_type, "buslogic") &&
+   strcmp(adapter_type, "lsilogic") &&
+   strcmp(adapter_type, "legacyESX")) {
+fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type);
+return -EINVAL;
+}
+if (strcmp(adapter_type, "ide") != 0) {
+/* that's the number of heads with which vmware operates when
+   creating, exporting, etc. vmdk files with a non-ide adapter type */
+number_heads = 255;
+}
 if (!fmt) {
 /* Default format to monolithicSparse */
 fmt = "monolithicSparse";
@@ -1576,7 +1594,8 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 parent_desc_line,
 ext_desc_lines,
 (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
-total_size / (int64_t)(63 * 16 * 512));
+total_size / (int64_t)(63 * number_heads * 512), number_heads,
+adapter_type);
 if (split || flat) {
 fd = qemu_open(filename,
O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
@@ -1661,6 +1680,12 @@ static QEMUOptionParameter vmdk_create_options[] = {
 .help = "Virtual disk size"
 },
 {
+.name = BLOCK_OPT_ADAPTER_TYPE,
+.type = OPT_STRING,
+.help = "Virtual adapter type, can be one of "
+"ide (default), lsilogic, buslogic or legacyESX"
+},
+{
 .name = BLOCK_OPT_BACKING_FILE,
 .type = OPT_STRING,
 .help = "File name of a base image"
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f7279b9..eaad53e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -56,6 +56,7 @@
 #define BLOCK_OPT_SUBFMT"subformat"
 #define BLOCK_OPT_COMPAT_LEVEL  "compat"
 #define BLOCK_OPT_LAZY_REFCOUNTS"lazy_refcounts"
+#define BLOCK_OPT_ADAPTER_TYPE  "adapter_type"
 
 typedef struct BdrvTrackedRequest BdrvTrackedRequest;
 
-- 
1.7.10.4




Re: [Qemu-devel] [PULL] virtio,make,pci,e1000

2013-01-29 Thread Michael S. Tsirkin
On Tue, Jan 29, 2013 at 04:54:35PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin"  writes:
> 
> > The following changes since commit 1356b98d3e95a85071e6bf9a99e8799e1ae1bbee:
> >
> >   sysbus: Drop sysbus_from_qdev() cast macro (2013-01-21 13:52:24 -0600)
> >
> > are available in the git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony
> >
> > for you to fetch changes up to eb023c60c9a1a7fad3058dbb578c69d8ee0e676b:
> >
> >   ich9: add support for pci assignment (2013-01-23 17:47:27 +0200)
> 
> make check fails with the fdc-test qtest:
> 
>   LINK  tests/m48t59-test
> GTESTER check-qtest-i386
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02S38e20b385311b7c6c10b2c92b399dd52
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02S5ebbf3ecb38f1a464703189ceb73fe1a
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02S9f87188247ec150bcd16270f5d4ac8b5
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02Se2b142608b05530b716e46f4a5f80795
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02S50c8584bbcbc65e0591df076ff55d11e
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02Sd89c43672f192301e75412f669fb5565
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02Sfd58f4c52349e36d7bb14e3da6502bea
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02S8b1db835a1ac3717c1e7ea221561e958
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02S60c3bc23909713e5b7ad5883b30472cb
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02S142c378bdcb027ca403c167b2544df1f
> **
> ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion 
> failed ((msr) & (RQM) == (RQM)): (0x == 0x0080)
> GTester: last random seed: R02S4b841f7154f9fe3d170e284ce7d3a801
> make: *** [check-qtest-i386] Error 1
> 
> Regards,
> 
> Anthony Liguori
> 

Nasty turns out for pci branch buildbot does not make check.
Anyone knows how to fix this?


> >
> > 
> > virtio,make,pci,e1000
> >
> > This includes my timestamp generation cleanup,
> > Amos's and my work on virtio net commands,
> > pci and e1000 fixes.
> >
> > Signed-off-by: Michael S. Tsirkin 
> >
> > 
> > Amos Kong (2):
> >   virtio-net: introduce a new macaddr control
> >   virtio-net: rename ctrl rx commands
> >
> > Jason Baron (1):
> >   ich9: add support for pci assignment
> >
> > Michael S. Tsirkin (7):
> >   e1000: document ICS read behaviour
> >   pci: make secondary bus reset explicit
> >   rules.mak: cleanup config generation rules
> >   Makefile: clean timestamp generation rule
> >   rules/mak: make clean should blow away timestamp files
> >   virtio-net: revert mac on reset
> >   virtio-net: remove layout assumptions for ctrl vq
> >
> >  hw/e1000.c  |  10 
> >  hw/ich9.h   |   1 +
> >  hw/lpc_ich9.c   |  33 
> >  hw/pc_piix.c|   4 ++
> >  hw/pc_q35.c |   1 +
> >  hw/pci/pci.c|   2 +-
> >  hw/pci/pci_bridge.c |   6 ++-
> >  hw/virtio-net.c | 143 
> > 
> >  hw/virtio-net.h |  26 ++
> >  rules.mak   |  14 +++--
> >  trace/Makefile.objs |   4 +-
> >  11 files changed, 171 insertions(+), 73 deletions(-)



Re: [Qemu-devel] [PATCH qom-cpu for-1.4] target-openrisc: TYPE_OPENRISC_CPU should be abstract

2013-01-29 Thread Andreas Färber
Am 27.01.2013 22:47, schrieb Andreas Färber:
> A basic assumption of CPU subtypes is that only specific models get
> instantiated. A user is not supposed to instantiate an -cpu.
> Suppress it via abstract = true, which also drops or32-cpu from
> -cpu ? output.
> 
> Cc: qemu-sta...@nongnu.org
> Cc: Jia Liu 
> Signed-off-by: Andreas Färber 

Ping? Can one of you please confirm that this change is okay or explain
why not?

My pending CPU type rename patch depends on this and Hard Freeze is this
Friday.
http://patchwork.ozlabs.org/patch/216066/

Thanks,
Andreas

> ---
>  target-openrisc/cpu.c |2 +-
>  1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)
> 
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> index 54876d9..14f2cbe 100644
> --- a/target-openrisc/cpu.c
> +++ b/target-openrisc/cpu.c
> @@ -159,7 +159,7 @@ static const TypeInfo openrisc_cpu_type_info = {
>  .parent = TYPE_CPU,
>  .instance_size = sizeof(OpenRISCCPU),
>  .instance_init = openrisc_cpu_initfn,
> -.abstract = false,
> +.abstract = true,
>  .class_size = sizeof(OpenRISCCPUClass),
>  .class_init = openrisc_cpu_class_init,
>  };

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH V2 11/20] tap: support enabling or disabling a queue

2013-01-29 Thread Michael S. Tsirkin
On Tue, Jan 29, 2013 at 04:55:25PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin"  writes:
> 
> > On Tue, Jan 29, 2013 at 08:10:26PM +, Blue Swirl wrote:
> >> On Tue, Jan 29, 2013 at 1:50 PM, Jason Wang  wrote:
> >> > On 01/26/2013 03:13 AM, Blue Swirl wrote:
> >> >> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang  
> >> >> wrote:
> >> >>> This patch introduce a new bit - enabled in TAPState which tracks 
> >> >>> whether a
> >> >>> specific queue/fd is enabled. The tap/fd is enabled during 
> >> >>> initialization and
> >> >>> could be enabled/disabled by tap_enalbe() and tap_disable() which 
> >> >>> calls platform
> >> >>> specific helpers to do the real work. Polling of a tap fd can only 
> >> >>> done when
> >> >>> the tap was enabled.
> >> >>>
> >> >>> Signed-off-by: Jason Wang 
> >> >>> ---
> >> >>>  include/net/tap.h |2 ++
> >> >>>  net/tap-win32.c   |   10 ++
> >> >>>  net/tap.c |   43 ---
> >> >>>  3 files changed, 52 insertions(+), 3 deletions(-)
> >> >>>
> >> >>> diff --git a/include/net/tap.h b/include/net/tap.h
> >> >>> index bb7efb5..0caf8c4 100644
> >> >>> --- a/include/net/tap.h
> >> >>> +++ b/include/net/tap.h
> >> >>> @@ -35,6 +35,8 @@ int tap_has_vnet_hdr_len(NetClientState *nc, int 
> >> >>> len);
> >> >>>  void tap_using_vnet_hdr(NetClientState *nc, int using_vnet_hdr);
> >> >>>  void tap_set_offload(NetClientState *nc, int csum, int tso4, int 
> >> >>> tso6, int ecn, int ufo);
> >> >>>  void tap_set_vnet_hdr_len(NetClientState *nc, int len);
> >> >>> +int tap_enable(NetClientState *nc);
> >> >>> +int tap_disable(NetClientState *nc);
> >> >>>
> >> >>>  int tap_get_fd(NetClientState *nc);
> >> >>>
> >> >>> diff --git a/net/tap-win32.c b/net/tap-win32.c
> >> >>> index 265369c..a2cd94b 100644
> >> >>> --- a/net/tap-win32.c
> >> >>> +++ b/net/tap-win32.c
> >> >>> @@ -764,3 +764,13 @@ void tap_set_vnet_hdr_len(NetClientState *nc, int 
> >> >>> len)
> >> >>>  {
> >> >>>  assert(0);
> >> >>>  }
> >> >>> +
> >> >>> +int tap_enable(NetClientState *nc)
> >> >>> +{
> >> >>> +assert(0);
> >> >> abort()
> >> >
> >> > This is just to be consistent with the reset of the helpers in this file.
> >> >>
> >> >>> +}
> >> >>> +
> >> >>> +int tap_disable(NetClientState *nc)
> >> >>> +{
> >> >>> +assert(0);
> >> >>> +}
> >> >>> diff --git a/net/tap.c b/net/tap.c
> >> >>> index 67080f1..95e557b 100644
> >> >>> --- a/net/tap.c
> >> >>> +++ b/net/tap.c
> >> >>> @@ -59,6 +59,7 @@ typedef struct TAPState {
> >> >>>  unsigned int write_poll : 1;
> >> >>>  unsigned int using_vnet_hdr : 1;
> >> >>>  unsigned int has_ufo: 1;
> >> >>> +unsigned int enabled : 1;
> >> >> bool without bit field?
> >> >
> >> > Also to be consistent with other field. If you wish I can send patches
> >> > to convert all those bit field to bool on top of this series.
> >> 
> >> That would be nice, likewise for the assert(0).
> >
> > OK so let's go ahead with this patchset as is,
> > and a cleanup patch will be send after 1.4 then.
> 
> Why?  I'd prefer that we didn't rush things into 1.4 just because.
> There's still ample time to respin a corrected series.
> 
> Regards,
> 
> Anthony Liguori

Confused.  Do you want the coding style rework of net/tap.c
switching it from assert(0)/bitfields to abort()/bool for 1.4?

> >
> >
> >> >
> >> > Thanks
> >> >>>  VHostNetState *vhost_net;
> >> >>>  unsigned host_vnet_hdr_len;
> >> >>>  } TAPState;
> >> >>> @@ -72,9 +73,9 @@ static void tap_writable(void *opaque);
> >> >>>  static void tap_update_fd_handler(TAPState *s)
> >> >>>  {
> >> >>>  qemu_set_fd_handler2(s->fd,
> >> >>> - s->read_poll  ? tap_can_send : NULL,
> >> >>> - s->read_poll  ? tap_send : NULL,
> >> >>> - s->write_poll ? tap_writable : NULL,
> >> >>> + s->read_poll && s->enabled ? tap_can_send : 
> >> >>> NULL,
> >> >>> + s->read_poll && s->enabled ? tap_send : 
> >> >>> NULL,
> >> >>> + s->write_poll && s->enabled ? tap_writable : 
> >> >>> NULL,
> >> >>>   s);
> >> >>>  }
> >> >>>
> >> >>> @@ -339,6 +340,7 @@ static TAPState *net_tap_fd_init(NetClientState 
> >> >>> *peer,
> >> >>>  s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 
> >> >>> 0;
> >> >>>  s->using_vnet_hdr = 0;
> >> >>>  s->has_ufo = tap_probe_has_ufo(s->fd);
> >> >>> +s->enabled = 1;
> >> >>>  tap_set_offload(&s->nc, 0, 0, 0, 0, 0);
> >> >>>  /*
> >> >>>   * Make sure host header length is set correctly in tap:
> >> >>> @@ -737,3 +739,38 @@ VHostNetState *tap_get_vhost_net(NetClientState 
> >> >>> *nc)
> >> >>>  assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
> >> >>>  return s->vhost_net;
> >> >>>  }
> >> >>> +
> >> >>> +int tap_enable(NetClientState *nc)
> >> >>> +{
> >> >>> +TAPState *s = DO_UPCAST(TAPState, nc, nc);
> >> >>> 

Re: [Qemu-devel] [PATCH V2 11/20] tap: support enabling or disabling a queue

2013-01-29 Thread Anthony Liguori
"Michael S. Tsirkin"  writes:

> On Tue, Jan 29, 2013 at 08:10:26PM +, Blue Swirl wrote:
>> On Tue, Jan 29, 2013 at 1:50 PM, Jason Wang  wrote:
>> > On 01/26/2013 03:13 AM, Blue Swirl wrote:
>> >> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang  wrote:
>> >>> This patch introduce a new bit - enabled in TAPState which tracks 
>> >>> whether a
>> >>> specific queue/fd is enabled. The tap/fd is enabled during 
>> >>> initialization and
>> >>> could be enabled/disabled by tap_enalbe() and tap_disable() which calls 
>> >>> platform
>> >>> specific helpers to do the real work. Polling of a tap fd can only done 
>> >>> when
>> >>> the tap was enabled.
>> >>>
>> >>> Signed-off-by: Jason Wang 
>> >>> ---
>> >>>  include/net/tap.h |2 ++
>> >>>  net/tap-win32.c   |   10 ++
>> >>>  net/tap.c |   43 ---
>> >>>  3 files changed, 52 insertions(+), 3 deletions(-)
>> >>>
>> >>> diff --git a/include/net/tap.h b/include/net/tap.h
>> >>> index bb7efb5..0caf8c4 100644
>> >>> --- a/include/net/tap.h
>> >>> +++ b/include/net/tap.h
>> >>> @@ -35,6 +35,8 @@ int tap_has_vnet_hdr_len(NetClientState *nc, int len);
>> >>>  void tap_using_vnet_hdr(NetClientState *nc, int using_vnet_hdr);
>> >>>  void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, 
>> >>> int ecn, int ufo);
>> >>>  void tap_set_vnet_hdr_len(NetClientState *nc, int len);
>> >>> +int tap_enable(NetClientState *nc);
>> >>> +int tap_disable(NetClientState *nc);
>> >>>
>> >>>  int tap_get_fd(NetClientState *nc);
>> >>>
>> >>> diff --git a/net/tap-win32.c b/net/tap-win32.c
>> >>> index 265369c..a2cd94b 100644
>> >>> --- a/net/tap-win32.c
>> >>> +++ b/net/tap-win32.c
>> >>> @@ -764,3 +764,13 @@ void tap_set_vnet_hdr_len(NetClientState *nc, int 
>> >>> len)
>> >>>  {
>> >>>  assert(0);
>> >>>  }
>> >>> +
>> >>> +int tap_enable(NetClientState *nc)
>> >>> +{
>> >>> +assert(0);
>> >> abort()
>> >
>> > This is just to be consistent with the reset of the helpers in this file.
>> >>
>> >>> +}
>> >>> +
>> >>> +int tap_disable(NetClientState *nc)
>> >>> +{
>> >>> +assert(0);
>> >>> +}
>> >>> diff --git a/net/tap.c b/net/tap.c
>> >>> index 67080f1..95e557b 100644
>> >>> --- a/net/tap.c
>> >>> +++ b/net/tap.c
>> >>> @@ -59,6 +59,7 @@ typedef struct TAPState {
>> >>>  unsigned int write_poll : 1;
>> >>>  unsigned int using_vnet_hdr : 1;
>> >>>  unsigned int has_ufo: 1;
>> >>> +unsigned int enabled : 1;
>> >> bool without bit field?
>> >
>> > Also to be consistent with other field. If you wish I can send patches
>> > to convert all those bit field to bool on top of this series.
>> 
>> That would be nice, likewise for the assert(0).
>
> OK so let's go ahead with this patchset as is,
> and a cleanup patch will be send after 1.4 then.

Why?  I'd prefer that we didn't rush things into 1.4 just because.
There's still ample time to respin a corrected series.

Regards,

Anthony Liguori

>
>
>> >
>> > Thanks
>> >>>  VHostNetState *vhost_net;
>> >>>  unsigned host_vnet_hdr_len;
>> >>>  } TAPState;
>> >>> @@ -72,9 +73,9 @@ static void tap_writable(void *opaque);
>> >>>  static void tap_update_fd_handler(TAPState *s)
>> >>>  {
>> >>>  qemu_set_fd_handler2(s->fd,
>> >>> - s->read_poll  ? tap_can_send : NULL,
>> >>> - s->read_poll  ? tap_send : NULL,
>> >>> - s->write_poll ? tap_writable : NULL,
>> >>> + s->read_poll && s->enabled ? tap_can_send : 
>> >>> NULL,
>> >>> + s->read_poll && s->enabled ? tap_send : 
>> >>> NULL,
>> >>> + s->write_poll && s->enabled ? tap_writable : 
>> >>> NULL,
>> >>>   s);
>> >>>  }
>> >>>
>> >>> @@ -339,6 +340,7 @@ static TAPState *net_tap_fd_init(NetClientState 
>> >>> *peer,
>> >>>  s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
>> >>>  s->using_vnet_hdr = 0;
>> >>>  s->has_ufo = tap_probe_has_ufo(s->fd);
>> >>> +s->enabled = 1;
>> >>>  tap_set_offload(&s->nc, 0, 0, 0, 0, 0);
>> >>>  /*
>> >>>   * Make sure host header length is set correctly in tap:
>> >>> @@ -737,3 +739,38 @@ VHostNetState *tap_get_vhost_net(NetClientState *nc)
>> >>>  assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
>> >>>  return s->vhost_net;
>> >>>  }
>> >>> +
>> >>> +int tap_enable(NetClientState *nc)
>> >>> +{
>> >>> +TAPState *s = DO_UPCAST(TAPState, nc, nc);
>> >>> +int ret;
>> >>> +
>> >>> +if (s->enabled) {
>> >>> +return 0;
>> >>> +} else {
>> >>> +ret = tap_fd_enable(s->fd);
>> >>> +if (ret == 0) {
>> >>> +s->enabled = 1;
>> >>> +tap_update_fd_handler(s);
>> >>> +}
>> >>> +return ret;
>> >>> +}
>> >>> +}
>> >>> +
>> >>> +int tap_disable(NetClientState *nc)
>> >>> +{
>> >>> +TAPState *s = DO_UPCAST(TAPState, nc, nc);
>> >>> +int ret;
>> >>> +
>> >

[Qemu-devel] [PATCH 03/19] s390: Add mapping helper functions.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Add s390_cpu_physical_memory_{map,unmap} with special handling
for the lowcore.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 target-s390x/cpu.h|4 
 target-s390x/helper.c |   25 +
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 1f2d942..7951aab 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -302,6 +302,10 @@ int cpu_s390x_handle_mmu_fault (CPUS390XState *env, 
target_ulong address, int rw
 
 
 #ifndef CONFIG_USER_ONLY
+void *s390_cpu_physical_memory_map(CPUS390XState *env, hwaddr addr, hwaddr 
*len,
+   int is_write);
+void s390_cpu_physical_memory_unmap(CPUS390XState *env, void *addr, hwaddr len,
+int is_write);
 void s390x_tod_timer(void *opaque);
 void s390x_cpu_timer(void *opaque);
 
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 023c074..3109c77 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -490,6 +490,31 @@ static void cpu_unmap_lowcore(LowCore *lowcore)
 cpu_physical_memory_unmap(lowcore, sizeof(LowCore), 1, sizeof(LowCore));
 }
 
+void *s390_cpu_physical_memory_map(CPUS390XState *env, hwaddr addr, hwaddr 
*len,
+   int is_write)
+{
+hwaddr start = addr;
+
+/* Mind the prefix area. */
+if (addr < 8192) {
+/* Map the lowcore. */
+start += env->psa;
+*len = MIN(*len, 8192 - addr);
+} else if ((addr >= env->psa) && (addr < env->psa + 8192)) {
+/* Map the 0 page. */
+start -= env->psa;
+*len = MIN(*len, 8192 - start);
+}
+
+return cpu_physical_memory_map(start, len, is_write);
+}
+
+void s390_cpu_physical_memory_unmap(CPUS390XState *env, void *addr, hwaddr len,
+int is_write)
+{
+cpu_physical_memory_unmap(addr, len, is_write, len);
+}
+
 static void do_svc_interrupt(CPUS390XState *env)
 {
 uint64_t mask, addr;
-- 
1.6.0.2




Re: [Qemu-devel] [PULL] virtio,make,pci,e1000

2013-01-29 Thread Anthony Liguori
"Michael S. Tsirkin"  writes:

> The following changes since commit 1356b98d3e95a85071e6bf9a99e8799e1ae1bbee:
>
>   sysbus: Drop sysbus_from_qdev() cast macro (2013-01-21 13:52:24 -0600)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony
>
> for you to fetch changes up to eb023c60c9a1a7fad3058dbb578c69d8ee0e676b:
>
>   ich9: add support for pci assignment (2013-01-23 17:47:27 +0200)

make check fails with the fdc-test qtest:

  LINK  tests/m48t59-test
GTESTER check-qtest-i386
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02S38e20b385311b7c6c10b2c92b399dd52
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02S5ebbf3ecb38f1a464703189ceb73fe1a
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02S9f87188247ec150bcd16270f5d4ac8b5
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02Se2b142608b05530b716e46f4a5f80795
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02S50c8584bbcbc65e0591df076ff55d11e
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02Sd89c43672f192301e75412f669fb5565
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02Sfd58f4c52349e36d7bb14e3da6502bea
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02S8b1db835a1ac3717c1e7ea221561e958
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02S60c3bc23909713e5b7ad5883b30472cb
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02S142c378bdcb027ca403c167b2544df1f
**
ERROR:/home/aliguori/git/qemu/tests/fdc-test.c:84:floppy_send: assertion failed 
((msr) & (RQM) == (RQM)): (0x == 0x0080)
GTester: last random seed: R02S4b841f7154f9fe3d170e284ce7d3a801
make: *** [check-qtest-i386] Error 1

Regards,

Anthony Liguori


>
> 
> virtio,make,pci,e1000
>
> This includes my timestamp generation cleanup,
> Amos's and my work on virtio net commands,
> pci and e1000 fixes.
>
> Signed-off-by: Michael S. Tsirkin 
>
> 
> Amos Kong (2):
>   virtio-net: introduce a new macaddr control
>   virtio-net: rename ctrl rx commands
>
> Jason Baron (1):
>   ich9: add support for pci assignment
>
> Michael S. Tsirkin (7):
>   e1000: document ICS read behaviour
>   pci: make secondary bus reset explicit
>   rules.mak: cleanup config generation rules
>   Makefile: clean timestamp generation rule
>   rules/mak: make clean should blow away timestamp files
>   virtio-net: revert mac on reset
>   virtio-net: remove layout assumptions for ctrl vq
>
>  hw/e1000.c  |  10 
>  hw/ich9.h   |   1 +
>  hw/lpc_ich9.c   |  33 
>  hw/pc_piix.c|   4 ++
>  hw/pc_q35.c |   1 +
>  hw/pci/pci.c|   2 +-
>  hw/pci/pci_bridge.c |   6 ++-
>  hw/virtio-net.c | 143 
> 
>  hw/virtio-net.h |  26 ++
>  rules.mak   |  14 +++--
>  trace/Makefile.objs |   4 +-
>  11 files changed, 171 insertions(+), 73 deletions(-)



[Qemu-devel] [PATCH 14/19] s390-virtio: Check for NULL device in reset hypercall

2013-01-29 Thread Alexander Graf
From: Andreas Färber 

s390_virtio_bus_find_mem() may return a NULL VirtIOS390Device.
If called with, e.g., args[0] == 0, this leads to a segfault.
Fix this by adding error handling as done for other hypercalls.

Present since baf0b55a9e57b909b1f8b0f732c0b10242867418 (Implement
virtio reset).

Signed-off-by: Andreas Färber 
Signed-off-by: Alexander Graf 
---
 hw/s390x/s390-virtio.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index a8a489d..2a1d9ac 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -86,6 +86,9 @@ static int s390_virtio_hcall_reset(const uint64_t *args)
 VirtIOS390Device *dev;
 
 dev = s390_virtio_bus_find_mem(s390_bus, mem);
+if (dev == NULL) {
+return -EINVAL;
+}
 virtio_reset(dev->vdev);
 stb_phys(dev->dev_offs + VIRTIO_DEV_OFFS_STATUS, 0);
 s390_virtio_device_sync(dev);
-- 
1.6.0.2




[Qemu-devel] [PATCH 11/19] s390: Make typeinfo const

2013-01-29 Thread Alexander Graf
All TypeInfo definitions should be const.

Signed-off-by: Alexander Graf 
---
 hw/s390x/ipl.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 7cbbf99..86e8415 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -159,7 +159,7 @@ static void s390_ipl_class_init(ObjectClass *klass, void 
*data)
 dc->no_user = 1;
 }
 
-static TypeInfo s390_ipl_info = {
+static const TypeInfo s390_ipl_info = {
 .class_init = s390_ipl_class_init,
 .parent = TYPE_SYS_BUS_DEVICE,
 .name  = "s390-ipl",
-- 
1.6.0.2




[Qemu-devel] [PATCH 08/19] s390: Wire up channel I/O in kvm.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Trigger the code for our virtual css in case of instruction
intercepts for I/O instructions.

Handle the tsch exit for the subchannel-related part of tsch.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 target-s390x/cpu.h |   11 +++
 target-s390x/kvm.c |  239 +---
 2 files changed, 237 insertions(+), 13 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 778065c..ce12fa4 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -1058,6 +1058,13 @@ void QEMU_NORETURN runtime_exception(CPUS390XState *env, 
int excp,
 
 #include 
 
+#ifdef CONFIG_KVM
+void kvm_s390_io_interrupt(S390CPU *cpu, uint16_t subchannel_id,
+   uint16_t subchannel_nr, uint32_t io_int_parm,
+   uint32_t io_int_word);
+void kvm_s390_crw_mchk(S390CPU *cpu);
+void kvm_s390_enable_css_support(S390CPU *cpu);
+#else
 static inline void kvm_s390_io_interrupt(S390CPU *cpu,
 uint16_t subchannel_id,
 uint16_t subchannel_nr,
@@ -1068,6 +1075,10 @@ static inline void kvm_s390_io_interrupt(S390CPU *cpu,
 static inline void kvm_s390_crw_mchk(S390CPU *cpu)
 {
 }
+static inline void kvm_s390_enable_css_support(S390CPU *cpu)
+{
+}
+#endif
 
 static inline void s390_io_interrupt(S390CPU *cpu,
  uint16_t subchannel_id,
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 99deddf..2c24182 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -47,9 +47,29 @@
 
 #define IPA0_DIAG   0x8300
 #define IPA0_SIGP   0xae00
-#define IPA0_PRIV   0xb200
+#define IPA0_B2 0xb200
+#define IPA0_B9 0xb900
+#define IPA0_EB 0xeb00
 
 #define PRIV_SCLP_CALL  0x20
+#define PRIV_CSCH   0x30
+#define PRIV_HSCH   0x31
+#define PRIV_MSCH   0x32
+#define PRIV_SSCH   0x33
+#define PRIV_STSCH  0x34
+#define PRIV_TSCH   0x35
+#define PRIV_TPI0x36
+#define PRIV_SAL0x37
+#define PRIV_RSCH   0x38
+#define PRIV_STCRW  0x39
+#define PRIV_STCPS  0x3a
+#define PRIV_RCHP   0x3b
+#define PRIV_SCHM   0x3c
+#define PRIV_CHSC   0x5f
+#define PRIV_SIGA   0x74
+#define PRIV_XSCH   0x76
+#define PRIV_SQBS   0x8a
+#define PRIV_EQBS   0x9c
 #define DIAG_KVM_HYPERCALL  0x500
 #define DIAG_KVM_BREAKPOINT 0x501
 
@@ -380,10 +400,123 @@ static int kvm_sclp_service_call(S390CPU *cpu, struct 
kvm_run *run,
 return 0;
 }
 
-static int handle_priv(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
+static int kvm_handle_css_inst(S390CPU *cpu, struct kvm_run *run,
+   uint8_t ipa0, uint8_t ipa1, uint8_t ipb)
+{
+int r = 0;
+int no_cc = 0;
+CPUS390XState *env = &cpu->env;
+
+if (ipa0 != 0xb2) {
+/* Not handled for now. */
+return -1;
+}
+cpu_synchronize_state(env);
+switch (ipa1) {
+case PRIV_XSCH:
+r = ioinst_handle_xsch(env, env->regs[1]);
+break;
+case PRIV_CSCH:
+r = ioinst_handle_csch(env, env->regs[1]);
+break;
+case PRIV_HSCH:
+r = ioinst_handle_hsch(env, env->regs[1]);
+break;
+case PRIV_MSCH:
+r = ioinst_handle_msch(env, env->regs[1], run->s390_sieic.ipb);
+break;
+case PRIV_SSCH:
+r = ioinst_handle_ssch(env, env->regs[1], run->s390_sieic.ipb);
+break;
+case PRIV_STCRW:
+r = ioinst_handle_stcrw(env, run->s390_sieic.ipb);
+break;
+case PRIV_STSCH:
+r = ioinst_handle_stsch(env, env->regs[1], run->s390_sieic.ipb);
+break;
+case PRIV_TSCH:
+/* We should only get tsch via KVM_EXIT_S390_TSCH. */
+fprintf(stderr, "Spurious tsch intercept\n");
+break;
+case PRIV_CHSC:
+r = ioinst_handle_chsc(env, run->s390_sieic.ipb);
+break;
+case PRIV_TPI:
+/* This should have been handled by kvm already. */
+fprintf(stderr, "Spurious tpi intercept\n");
+break;
+case PRIV_SCHM:
+no_cc = 1;
+r = ioinst_handle_schm(env, env->regs[1], env->regs[2],
+   run->s390_sieic.ipb);
+break;
+case PRIV_RSCH:
+r = ioinst_handle_rsch(env, env->regs[1]);
+break;
+case PRIV_RCHP:
+r = ioinst_handle_rchp(env, env->regs[1]);
+break;
+case PRIV_STCPS:
+/* We do not provide this instruction, it is suppressed. */
+no_cc = 1;
+

[Qemu-devel] buildbot failure in qemu on default_x86_64_debian_6_0

2013-01-29 Thread qemu
The Buildbot has detected a new failure on builder default_x86_64_debian_6_0 
while building qemu.
Full details are available at:
 
http://buildbot.b1-systems.de/qemu/builders/default_x86_64_debian_6_0/builds/526

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_default' triggered this build
Build Source Stamp: [branch master] HEAD
Blamelist: 

BUILD FAILED: failed test

sincerely,
 -The Buildbot



[Qemu-devel] [PATCH 09/19] s390-virtio: Factor out some initialization code.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Some of the machine initialization for s390-virtio will be reused
by virtio-ccw.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 hw/s390-virtio.c |  118 ++---
 hw/s390-virtio.h |5 ++
 2 files changed, 72 insertions(+), 51 deletions(-)

diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 5edaabb..6e0f53b 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -147,13 +147,73 @@ unsigned s390_del_running_cpu(CPUS390XState *env)
 return s390_running_cpus;
 }
 
+void s390_init_ipl_dev(const char *kernel_filename,
+   const char *kernel_cmdline,
+   const char *initrd_filename)
+{
+DeviceState *dev;
+
+dev  = qdev_create(NULL, "s390-ipl");
+if (kernel_filename) {
+qdev_prop_set_string(dev, "kernel", kernel_filename);
+}
+if (initrd_filename) {
+qdev_prop_set_string(dev, "initrd", initrd_filename);
+}
+qdev_prop_set_string(dev, "cmdline", kernel_cmdline);
+qdev_init_nofail(dev);
+}
+
+void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
+{
+int i;
+
+if (cpu_model == NULL) {
+cpu_model = "host";
+}
+
+ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
+
+for (i = 0; i < smp_cpus; i++) {
+S390CPU *cpu;
+
+cpu = cpu_s390x_init(cpu_model);
+
+ipi_states[i] = cpu;
+cpu->env.halted = 1;
+cpu->env.exception_index = EXCP_HLT;
+cpu->env.storage_keys = storage_keys;
+}
+}
+
+
+void s390_create_virtio_net(BusState *bus, const char *name)
+{
+int i;
+
+for (i = 0; i < nb_nics; i++) {
+NICInfo *nd = &nd_table[i];
+DeviceState *dev;
+
+if (!nd->model) {
+nd->model = g_strdup("virtio");
+}
+
+if (strcmp(nd->model, "virtio")) {
+fprintf(stderr, "S390 only supports VirtIO nics\n");
+exit(1);
+}
+
+dev = qdev_create(bus, name);
+qdev_set_nic_properties(dev, nd);
+qdev_init_nofail(dev);
+}
+}
+
 /* PC hardware initialisation */
 static void s390_init(QEMUMachineInitArgs *args)
 {
 ram_addr_t my_ram_size = args->ram_size;
-const char *cpu_model = args->cpu_model;
-CPUS390XState *env = NULL;
-DeviceState *dev;
 MemoryRegion *sysmem = get_system_memory();
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 int shift = 0;
@@ -161,7 +221,6 @@ static void s390_init(QEMUMachineInitArgs *args)
 void *virtio_region;
 hwaddr virtio_region_len;
 hwaddr virtio_region_start;
-int i;
 
 /* s390x ram size detection needs a 16bit multiplier + an increment. So
guests > 64GB can be specified in 2MB steps etc. */
@@ -176,15 +235,8 @@ static void s390_init(QEMUMachineInitArgs *args)
 /* get a BUS */
 s390_bus = s390_virtio_bus_init(&my_ram_size);
 s390_sclp_init();
-dev  = qdev_create(NULL, "s390-ipl");
-if (args->kernel_filename) {
-qdev_prop_set_string(dev, "kernel", args->kernel_filename);
-}
-if (args->initrd_filename) {
-qdev_prop_set_string(dev, "initrd", args->initrd_filename);
-}
-qdev_prop_set_string(dev, "cmdline", args->kernel_cmdline);
-qdev_init_nofail(dev);
+s390_init_ipl_dev(args->kernel_filename, args->kernel_cmdline,
+  args->initrd_filename);
 
 /* register hypercalls */
 s390_virtio_register_hcalls();
@@ -207,46 +259,10 @@ static void s390_init(QEMUMachineInitArgs *args)
 storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
 
 /* init CPUs */
-if (cpu_model == NULL) {
-cpu_model = "host";
-}
-
-ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
-
-for (i = 0; i < smp_cpus; i++) {
-S390CPU *cpu;
-CPUS390XState *tmp_env;
-
-cpu = cpu_s390x_init(cpu_model);
-tmp_env = &cpu->env;
-if (!env) {
-env = tmp_env;
-}
-ipi_states[i] = cpu;
-tmp_env->halted = 1;
-tmp_env->exception_index = EXCP_HLT;
-tmp_env->storage_keys = storage_keys;
-}
-
+s390_init_cpus(args->cpu_model, storage_keys);
 
 /* Create VirtIO network adapters */
-for(i = 0; i < nb_nics; i++) {
-NICInfo *nd = &nd_table[i];
-DeviceState *dev;
-
-if (!nd->model) {
-nd->model = g_strdup("virtio");
-}
-
-if (strcmp(nd->model, "virtio")) {
-fprintf(stderr, "S390 only supports VirtIO nics\n");
-exit(1);
-}
-
-dev = qdev_create((BusState *)s390_bus, "virtio-net-s390");
-qdev_set_nic_properties(dev, nd);
-qdev_init_nofail(dev);
-}
+s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390");
 }
 
 static QEMUMachine s390_machine = {
diff --git a/hw/s390-virtio.h b/hw/s390-virtio.h
index 25bb610..67bfd20 100644
--- a/hw/s390-virtio.h
+++ b/hw/s390-virtio.h
@@ -19,4 +19,9 @@
 typedef int (*s390

Re: [Qemu-devel] [PATCH qom-cpu for-1.4] target-m68k: Rename CPU types

2013-01-29 Thread Andreas Färber
Am 27.01.2013 20:18, schrieb Andreas Färber:
> In the initial conversion of CPU models to QOM types, model names were
> mapped 1:1 to type names. As a side effect this gained us a type "any",
> which is now a device.
> 
> To avoid "-device any" silliness and to pave the way for compiling
> multiple targets into one executable, adopt a --cpu scheme.
> 
> No functional changes for -cpu arguments or -cpu ? output.
> 
> Signed-off-by: Andreas Färber 

Applied to qom-cpu (after verifying that env->cpu_model_str is correct):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH 19/19] s390: Drop set_bit usage in virtio_ccw.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

set_bit on indicators doesn't go well on 32 bit targets:

note: expected 'long unsigned int *' but argument is of type 'uint64_t *'

Switch to bit shifts instead.

Signed-off-by: Cornelia Huck 
[agraf: use 1ULL instead]
Signed-off-by: Alexander Graf 
---
 hw/s390x/virtio-ccw.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 7d7f336..231f81e 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -662,12 +662,12 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t 
vector)
 
 if (vector < VIRTIO_PCI_QUEUE_MAX) {
 indicators = ldq_phys(dev->indicators);
-set_bit(vector, &indicators);
+indicators |= 1ULL << vector;
 stq_phys(dev->indicators, indicators);
 } else {
 vector = 0;
 indicators = ldq_phys(dev->indicators2);
-set_bit(vector, &indicators);
+indicators |= 1ULL << vector;
 stq_phys(dev->indicators2, indicators);
 }
 
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH 3/3] tests: Add unit tests for mulu64 and muls64

2013-01-29 Thread Richard Henderson

On 01/29/2013 12:06 PM, Blue Swirl wrote:

+static const Test test_u_data[] = {
+{ 1, 1, 0, 1 },
+{ 1, 1, 0, 1 },
+{ -1ull, 2, 1, -2ull },


Shouldn't '1' be '-1'? How can this test pass?


This is 0x___ * 2 = 0x1____fffe

with the knowledge that -1 = 0xf...f and -2 = 0xf...e.


+{ -1ull, -1ull, -2ull, 1 },


This looks buggy too.


See above.


+{ -10, -10, 0, 100 },
+{ 1, 1, 0, 1 },
+{ -1, 2, -1, -2 },
+{ 0x1122334455667788ll, 0x1122334455667788ull,


Spurious 'll', also below.


Why spurious?


+static void test_u(void)
+{
+int i;
+
+for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) {
+uint64_t rl, rh;
+mulu64(&rl, &rh, test_u_data[i].a, test_u_data[i].b);
+g_assert_cmpuint(rl, ==, test_u_data[i].rl);


This could explain why the test passes: g_assert_cmpuint() uses
unsigned ints so there is truncation from uint64_t.


Does it?  It sure doesn't look like it:


#define g_assert_cmpuint(n1, cmp, n2)   do { guint64 __n1 = (n1), __n2 = (n2); \
 if (__n1 cmp __n2) ; else \
   g_assertion_message_cmpnum 
(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \
 #n1 " " #cmp " " #n2, __n1, 
#cmp, __n2, 'i'); } while (0)


I see guint64 in there, thus no truncation.


+static void test_s(void)
+{
+int i;
+
+for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) {


test_s_data


Good catch.  I wasn't running all of the test_s data points.


r~



[Qemu-devel] [PATCH 15/19] s390: Add s390-ccw-virtio machine.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Add a new machine type, s390-ccw-virtio, making use of the
virtio-ccw transport to present virtio devices as channel
devices.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 hw/s390x/Makefile.objs |1 +
 hw/s390x/s390-virtio-ccw.c |  134 
 hw/s390x/s390-virtio.h |1 +
 3 files changed, 136 insertions(+), 0 deletions(-)
 create mode 100644 hw/s390x/s390-virtio-ccw.c

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index e4ee456..9f2f419 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -5,4 +5,5 @@ obj-y += event-facility.o
 obj-y += sclpquiesce.o sclpconsole.o
 obj-y += ipl.o
 obj-y += css.o
+obj-y += s390-virtio-ccw.o
 obj-y += virtio-ccw.o
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
new file mode 100644
index 000..6549211
--- /dev/null
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -0,0 +1,134 @@
+/*
+ * virtio ccw machine
+ *
+ * Copyright 2012 IBM Corp.
+ * Author(s): Cornelia Huck 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include "hw/boards.h"
+#include "exec/address-spaces.h"
+#include "s390-virtio.h"
+#include "sclp.h"
+#include "ioinst.h"
+#include "css.h"
+#include "virtio-ccw.h"
+
+static int virtio_ccw_hcall_notify(const uint64_t *args)
+{
+uint64_t subch_id = args[0];
+uint64_t queue = args[1];
+SubchDev *sch;
+int cssid, ssid, schid, m;
+
+if (ioinst_disassemble_sch_ident(subch_id, &m, &cssid, &ssid, &schid)) {
+return -EINVAL;
+}
+sch = css_find_subch(m, cssid, ssid, schid);
+if (!sch || !css_subch_visible(sch)) {
+return -EINVAL;
+}
+virtio_queue_notify(virtio_ccw_get_vdev(sch), queue);
+return 0;
+
+}
+
+static int virtio_ccw_hcall_early_printk(const uint64_t *args)
+{
+uint64_t mem = args[0];
+
+if (mem < ram_size) {
+/* Early printk */
+return 0;
+}
+return -EINVAL;
+}
+
+static void virtio_ccw_register_hcalls(void)
+{
+s390_register_virtio_hypercall(KVM_S390_VIRTIO_CCW_NOTIFY,
+   virtio_ccw_hcall_notify);
+/* Tolerate early printk. */
+s390_register_virtio_hypercall(KVM_S390_VIRTIO_NOTIFY,
+   virtio_ccw_hcall_early_printk);
+}
+
+static void ccw_init(QEMUMachineInitArgs *args)
+{
+ram_addr_t my_ram_size = args->ram_size;
+MemoryRegion *sysmem = get_system_memory();
+MemoryRegion *ram = g_new(MemoryRegion, 1);
+int shift = 0;
+uint8_t *storage_keys;
+int ret;
+VirtualCssBus *css_bus;
+
+/* s390x ram size detection needs a 16bit multiplier + an increment. So
+   guests > 64GB can be specified in 2MB steps etc. */
+while ((my_ram_size >> (20 + shift)) > 65535) {
+shift++;
+}
+my_ram_size = my_ram_size >> (20 + shift) << (20 + shift);
+
+/* lets propagate the changed ram size into the global variable. */
+ram_size = my_ram_size;
+
+/* get a BUS */
+css_bus = virtual_css_bus_init();
+s390_sclp_init();
+s390_init_ipl_dev(args->kernel_filename, args->kernel_cmdline,
+  args->initrd_filename);
+
+/* register hypercalls */
+virtio_ccw_register_hcalls();
+
+/* allocate RAM */
+memory_region_init_ram(ram, "s390.ram", my_ram_size);
+vmstate_register_ram_global(ram);
+memory_region_add_subregion(sysmem, 0, ram);
+
+/* allocate storage keys */
+storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
+
+/* init CPUs */
+s390_init_cpus(args->cpu_model, storage_keys);
+
+if (kvm_enabled()) {
+kvm_s390_enable_css_support(s390_cpu_addr2state(0));
+}
+/*
+ * Create virtual css and set it as default so that non mcss-e
+ * enabled guests only see virtio devices.
+ */
+ret = css_create_css_image(VIRTUAL_CSSID, true);
+assert(ret == 0);
+
+/* Create VirtIO network adapters */
+s390_create_virtio_net(BUS(css_bus), "virtio-net-ccw");
+}
+
+static QEMUMachine ccw_machine = {
+.name = "s390-ccw-virtio",
+.alias = "s390-ccw",
+.desc = "VirtIO-ccw based S390 machine",
+.init = ccw_init,
+.block_default_type = IF_VIRTIO,
+.no_cdrom = 1,
+.no_floppy = 1,
+.no_serial = 1,
+.no_parallel = 1,
+.no_sdcard = 1,
+.use_sclp = 1,
+.max_cpus = 255,
+DEFAULT_MACHINE_OPTIONS,
+};
+
+static void ccw_machine_init(void)
+{
+qemu_register_machine(&ccw_machine);
+}
+
+machine_init(ccw_machine_init)
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index 67bfd20..a6c4c19 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -15,6 +15,7 @@
 #define KVM_S390_VIRTIO_NOTIFY  0
 #define KVM_S390_VIRTIO_RESET   1
 #define KVM_S390_VIRTIO_SET_STATUS  2
+#define KVM_S390_VIRTIO_CCW_NOTIFY  3
 
 typedef int (*

Re: [Qemu-devel] [PATCH 0/5] vhost-scsi: Add support for host virtualized target

2013-01-29 Thread Nicholas A. Bellinger
On Tue, 2013-01-29 at 16:03 -0500, Paolo Bonzini wrote:
> > Also, I'm not exactly sure what's involved with a vhost=on/off
> > frontend
> > option discussed above, so if you or Paolo could handle this bit it
> > would be very helpful.
> 
> I can do the forward port, if you take care of testing we have
> enough time to make it into 1.4.
> 

Perfect, thanks for the extra help here.  ;)

I'll look for your forward-port of vhost-scsi, and test as soon as it's
available.

--nab





[Qemu-devel] [PATCH 01/19] s390: Add default support for SCLP console

2013-01-29 Thread Alexander Graf
The current s390 machine uses the virtio console as default console,
but this doesn't mean that we always want to keep it that way for new
machines.

This patch introduces a way for a machine type to specify that it wants
the default console to be an SCLP console, which is a lot closer to what
real hardware does.

Signed-off-by: Alexander Graf 
Reviewed-by: Andreas Färber 
---
 hw/boards.h |1 +
 vl.c|   51 +++
 2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/hw/boards.h b/hw/boards.h
index 3ff9665..3813d4e 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -33,6 +33,7 @@ typedef struct QEMUMachine {
 unsigned int no_serial:1,
 no_parallel:1,
 use_virtcon:1,
+use_sclp:1,
 no_floppy:1,
 no_cdrom:1,
 no_sdcard:1;
diff --git a/vl.c b/vl.c
index 7aab73b..8b0961e 100644
--- a/vl.c
+++ b/vl.c
@@ -176,6 +176,7 @@ int main(int argc, char **argv)
 #define DEFAULT_RAM_SIZE 128
 
 #define MAX_VIRTIO_CONSOLES 1
+#define MAX_SCLP_CONSOLES 1
 
 static const char *data_dir;
 const char *bios_name = NULL;
@@ -203,6 +204,7 @@ int no_quit = 0;
 CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
+CharDriverState *sclp_hds[MAX_SCLP_CONSOLES];
 int win2k_install_hack = 0;
 int singlestep = 0;
 int smp_cpus = 1;
@@ -271,6 +273,7 @@ static int tcg_tb_size;
 static int default_serial = 1;
 static int default_parallel = 1;
 static int default_virtcon = 1;
+static int default_sclp = 1;
 static int default_monitor = 1;
 static int default_floppy = 1;
 static int default_cdrom = 1;
@@ -2340,6 +2343,7 @@ struct device_config {
 DEV_VIRTCON,   /* -virtioconsole */
 DEV_DEBUGCON,  /* -debugcon */
 DEV_GDB,   /* -gdb, -s */
+DEV_SCLP,  /* s390 sclp */
 } type;
 const char *cmdline;
 Location loc;
@@ -2458,6 +2462,39 @@ static int virtcon_parse(const char *devname)
 return 0;
 }
 
+static int sclp_parse(const char *devname)
+{
+QemuOptsList *device = qemu_find_opts("device");
+static int index = 0;
+char label[32];
+QemuOpts *dev_opts;
+
+if (strcmp(devname, "none") == 0) {
+return 0;
+}
+if (index == MAX_SCLP_CONSOLES) {
+fprintf(stderr, "qemu: too many sclp consoles\n");
+exit(1);
+}
+
+assert(arch_type == QEMU_ARCH_S390X);
+
+dev_opts = qemu_opts_create(device, NULL, 0, NULL);
+qemu_opt_set(dev_opts, "driver", "sclpconsole");
+
+snprintf(label, sizeof(label), "sclpcon%d", index);
+sclp_hds[index] = qemu_chr_new(label, devname, NULL);
+if (!sclp_hds[index]) {
+fprintf(stderr, "qemu: could not connect sclp console"
+" to character backend '%s'\n", devname);
+return -1;
+}
+qemu_opt_set(dev_opts, "chardev", label);
+
+index++;
+return 0;
+}
+
 static int debugcon_parse(const char *devname)
 {   
 QemuOpts *opts;
@@ -3832,6 +3869,9 @@ int main(int argc, char **argv, char **envp)
 if (!machine->use_virtcon) {
 default_virtcon = 0;
 }
+if (!machine->use_sclp) {
+default_sclp = 0;
+}
 if (machine->no_floppy) {
 default_floppy = 0;
 }
@@ -3873,11 +3913,16 @@ int main(int argc, char **argv, char **envp)
 add_device_config(DEV_SERIAL, "mon:stdio");
 } else if (default_virtcon && default_monitor) {
 add_device_config(DEV_VIRTCON, "mon:stdio");
+} else if (default_sclp && default_monitor) {
+add_device_config(DEV_SCLP, "mon:stdio");
 } else {
 if (default_serial)
 add_device_config(DEV_SERIAL, "stdio");
 if (default_virtcon)
 add_device_config(DEV_VIRTCON, "stdio");
+if (default_sclp) {
+add_device_config(DEV_SCLP, "stdio");
+}
 if (default_monitor)
 monitor_parse("stdio", "readline");
 }
@@ -3890,6 +3935,9 @@ int main(int argc, char **argv, char **envp)
 monitor_parse("vc:80Cx24C", "readline");
 if (default_virtcon)
 add_device_config(DEV_VIRTCON, "vc:80Cx24C");
+if (default_sclp) {
+add_device_config(DEV_SCLP, "vc:80Cx24C");
+}
 }
 
 socket_init();
@@ -4060,6 +4108,9 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 if (foreach_device_config(DEV_VIRTCON, virtcon_parse) < 0)
 exit(1);
+if (foreach_device_config(DEV_SCLP, sclp_parse) < 0) {
+exit(1);
+}
 if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
 exit(1);
 
-- 
1.6.0.2




Re: [Qemu-devel] KVM call minutes 2013-01-29

2013-01-29 Thread Anthony Liguori
Alexander Graf  writes:

> On 01/29/2013 04:41 PM, Juan Quintela wrote:
>>Alex will fill this
>
> When using -device, we can not specify an IRQ line to attach to the 
> device. This works for some special buses like PCI, but not in the 
> generic case. We need it generically for virtio-mmio and for potential 
> platform assigned vfio devices though.
>
> The conclusion we came up with was that in order to model IRQ lines 
> between arbitrary devices, we should use QOM and the QOM name space. 
> Details are up for Anthony to fill in :).

Oh good :-)  This is how far I got since I last touched this problem.

https://github.com/aliguori/qemu/commits/qom-pin.4

qemu_irq is basically foreign to QOM/qdev.  There are two things I did
to solve this.  The first is to have a stateful Pin object.  Stateful is
important because qemu_irq is totally broken wrt reset and live
migration as it stands today.

It's pretty easy to have a Pin object that can "connect" to a qemu_irq
source or sink which means we can incrementally refactor by first
converting each device under a bus to using Pins (using the qemu_irq
connect interface to maintain compat) until the bus controller can be
converted to export Pins allowing a full switch to using Pins only for
that bus.

The problems I ran into were (1) this is a lot of work (2) it basically
requires that all bus children have been qdev/QOM-ified.  Even with
something like the ISA bus which is where I started, quite a few devices
were not qdevified still.

I'm not going to be able to work on this in the foreseeable future, but
if someone wants to take it over, I'd be happy to provide advice.

I'm also open to other approaches that require less refactoring but I
honestly don't know that there is a way to avoid the heavy lifting.

Regards,

Anthony Liguori

>
>
> Alex
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Qemu-devel] [PATCH 13/19] s390: Move hw files to hw/s390x

2013-01-29 Thread Alexander Graf
This moves all files only used by s390 system emulation to hw/s390x.

Signed-off-by: Alexander Graf 
Acked-by: Christian Borntraeger 
---
 hw/s390x/Makefile.objs   |2 --
 hw/{ => s390x}/s390-virtio-bus.c |8 
 hw/{ => s390x}/s390-virtio-bus.h |   12 ++--
 hw/s390x/s390-virtio-hcall.c |2 +-
 hw/{ => s390x}/s390-virtio.c |   10 +-
 hw/{ => s390x}/s390-virtio.h |0
 6 files changed, 16 insertions(+), 18 deletions(-)
 rename hw/{ => s390x}/s390-virtio-bus.c (99%)
 rename hw/{ => s390x}/s390-virtio-bus.h (95%)
 rename hw/{ => s390x}/s390-virtio.c (98%)
 rename hw/{ => s390x}/s390-virtio.h (100%)

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index f6b461b..e4ee456 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -1,6 +1,4 @@
 obj-y = s390-virtio-bus.o s390-virtio.o
-
-obj-y := $(addprefix ../,$(obj-y))
 obj-y += s390-virtio-hcall.o
 obj-y += sclp.o
 obj-y += event-facility.o
diff --git a/hw/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
similarity index 99%
rename from hw/s390-virtio-bus.c
rename to hw/s390x/s390-virtio-bus.c
index 6858db0..32f63b0 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -17,12 +17,12 @@
  * License along with this library; if not, see .
  */
 
-#include "hw.h"
+#include "hw/hw.h"
 #include "block/block.h"
 #include "sysemu/sysemu.h"
-#include "boards.h"
+#include "hw/boards.h"
 #include "monitor/monitor.h"
-#include "loader.h"
+#include "hw/loader.h"
 #include "elf.h"
 #include "hw/virtio.h"
 #include "hw/virtio-rng.h"
@@ -31,7 +31,7 @@
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
 
-#include "hw/s390-virtio-bus.h"
+#include "hw/s390x/s390-virtio-bus.h"
 #include "hw/virtio-bus.h"
 
 /* #define DEBUG_S390 */
diff --git a/hw/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h
similarity index 95%
rename from hw/s390-virtio-bus.h
rename to hw/s390x/s390-virtio-bus.h
index 438b37f..4aacf83 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390x/s390-virtio-bus.h
@@ -19,12 +19,12 @@
 #ifndef HW_S390_VIRTIO_BUS_H
 #define HW_S390_VIRTIO_BUS_H 1
 
-#include "virtio-blk.h"
-#include "virtio-net.h"
-#include "virtio-rng.h"
-#include "virtio-serial.h"
-#include "virtio-scsi.h"
-#include "virtio-bus.h"
+#include "hw/virtio-blk.h"
+#include "hw/virtio-net.h"
+#include "hw/virtio-rng.h"
+#include "hw/virtio-serial.h"
+#include "hw/virtio-scsi.h"
+#include "hw/virtio-bus.h"
 
 #define VIRTIO_DEV_OFFS_TYPE   0   /* 8 bits */
 #define VIRTIO_DEV_OFFS_NUM_VQ 1   /* 8 bits */
diff --git a/hw/s390x/s390-virtio-hcall.c b/hw/s390x/s390-virtio-hcall.c
index d7938c0..ee62649 100644
--- a/hw/s390x/s390-virtio-hcall.c
+++ b/hw/s390x/s390-virtio-hcall.c
@@ -10,7 +10,7 @@
  */
 
 #include "cpu.h"
-#include "hw/s390-virtio.h"
+#include "hw/s390x/s390-virtio.h"
 
 #define MAX_DIAG_SUBCODES 255
 
diff --git a/hw/s390-virtio.c b/hw/s390x/s390-virtio.c
similarity index 98%
rename from hw/s390-virtio.c
rename to hw/s390x/s390-virtio.c
index 6e0f53b..a8a489d 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -21,22 +21,22 @@
  * License along with this library; if not, see .
  */
 
-#include "hw.h"
+#include "hw/hw.h"
 #include "block/block.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/sysemu.h"
 #include "net/net.h"
-#include "boards.h"
+#include "hw/boards.h"
 #include "monitor/monitor.h"
-#include "loader.h"
+#include "hw/loader.h"
 #include "hw/virtio.h"
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
 #include "exec/address-spaces.h"
 
-#include "hw/s390-virtio-bus.h"
+#include "hw/s390x/s390-virtio-bus.h"
 #include "hw/s390x/sclp.h"
-#include "hw/s390-virtio.h"
+#include "hw/s390x/s390-virtio.h"
 
 //#define DEBUG_S390
 
diff --git a/hw/s390-virtio.h b/hw/s390x/s390-virtio.h
similarity index 100%
rename from hw/s390-virtio.h
rename to hw/s390x/s390-virtio.h
-- 
1.6.0.2




[Qemu-devel] [PATCH for-1.4 v2] target-unicore32: Rename CPU subtypes

2013-01-29 Thread Andreas Färber
In the initial conversion of CPU models to QOM types, model names were
mapped 1:1 to type names. As a side effect this gained us a type "any",
which is now a device.

To avoid "-device any" silliness and to pave the way for compiling
multiple targets into one executable, adopt a --cpu scheme.

No functional changes for -cpu arguments.

Signed-off-by: Andreas Färber 
---
 v1 -> v2:
 * Update env->cpu_model_str to cpu_model rather than the changing
   object_get_typename(). Fixes cpu_copy() used by linux-user.

 target-unicore32/cpu.c|9 ++---
 target-unicore32/helper.c |1 +
 2 Dateien geändert, 7 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
index c120440..9dc7da8 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -25,12 +25,15 @@ static inline void set_feature(CPUUniCore32State *env, int 
feature)
 static ObjectClass *uc32_cpu_class_by_name(const char *cpu_model)
 {
 ObjectClass *oc;
+char *typename;
 
 if (cpu_model == NULL) {
 return NULL;
 }
 
-oc = object_class_by_name(cpu_model);
+typename = g_strdup_printf("%s-" TYPE_UNICORE32_CPU, cpu_model);
+oc = object_class_by_name(typename);
+g_free(typename);
 if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_UNICORE32_CPU) ||
object_class_is_abstract(oc))) {
 oc = NULL;
@@ -83,7 +86,6 @@ static void uc32_cpu_initfn(Object *obj)
 CPUUniCore32State *env = &cpu->env;
 
 cpu_exec_init(env);
-env->cpu_model_str = object_get_typename(obj);
 
 #ifdef CONFIG_USER_ONLY
 env->uncached_asr = ASR_MODE_USER;
@@ -106,12 +108,13 @@ static void uc32_cpu_class_init(ObjectClass *oc, void 
*data)
 static void uc32_register_cpu_type(const UniCore32CPUInfo *info)
 {
 TypeInfo type_info = {
-.name = info->name,
 .parent = TYPE_UNICORE32_CPU,
 .instance_init = info->instance_init,
 };
 
+type_info.name = g_strdup_printf("%s-" TYPE_UNICORE32_CPU, info->name);
 type_register(&type_info);
+g_free((void *)type_info.name);
 }
 
 static const TypeInfo uc32_cpu_type_info = {
diff --git a/target-unicore32/helper.c b/target-unicore32/helper.c
index 183b5b3..3a92232 100644
--- a/target-unicore32/helper.c
+++ b/target-unicore32/helper.c
@@ -38,6 +38,7 @@ CPUUniCore32State *uc32_cpu_init(const char *cpu_model)
 }
 cpu = UNICORE32_CPU(object_new(object_class_get_name(oc)));
 env = &cpu->env;
+env->cpu_model_str = cpu_model;
 
 if (inited) {
 inited = 0;
-- 
1.7.10.4




[Qemu-devel] [PATCH 05/19] s390: I/O interrupt and machine check injection.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

I/O interrupts are queued per isc. Only crw pending machine checks
are supported.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 target-s390x/cpu.h|   69 +++-
 target-s390x/helper.c |  141 +
 2 files changed, 209 insertions(+), 1 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index c1a0040..3e00d38 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -50,6 +50,11 @@
 #define MMU_USER_IDX 1
 
 #define MAX_EXT_QUEUE 16
+#define MAX_IO_QUEUE 16
+#define MAX_MCHK_QUEUE 16
+
+#define PSW_MCHK_MASK 0x0004
+#define PSW_IO_MASK 0x0200
 
 typedef struct PSW {
 uint64_t mask;
@@ -62,6 +67,17 @@ typedef struct ExtQueue {
 uint32_t param64;
 } ExtQueue;
 
+typedef struct IOIntQueue {
+uint16_t id;
+uint16_t nr;
+uint32_t parm;
+uint32_t word;
+} IOIntQueue;
+
+typedef struct MchkQueue {
+uint16_t type;
+} MchkQueue;
+
 typedef struct CPUS390XState {
 uint64_t regs[16]; /* GP registers */
 CPU_DoubleU fregs[16]; /* FP registers */
@@ -93,9 +109,17 @@ typedef struct CPUS390XState {
 uint64_t cregs[16]; /* control registers */
 
 ExtQueue ext_queue[MAX_EXT_QUEUE];
-int pending_int;
+IOIntQueue io_queue[MAX_IO_QUEUE][8];
+MchkQueue mchk_queue[MAX_MCHK_QUEUE];
 
+int pending_int;
 int ext_index;
+int io_index[8];
+int mchk_index;
+
+uint64_t ckc;
+uint64_t cputm;
+uint32_t todpr;
 
 CPU_COMMON
 
@@ -375,10 +399,14 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 #define EXCP_EXT 1 /* external interrupt */
 #define EXCP_SVC 2 /* supervisor call (syscall) */
 #define EXCP_PGM 3 /* program interruption */
+#define EXCP_IO  7 /* I/O interrupt */
+#define EXCP_MCHK 8 /* machine check */
 
 #define INTERRUPT_EXT(1 << 0)
 #define INTERRUPT_TOD(1 << 1)
 #define INTERRUPT_CPUTIMER   (1 << 2)
+#define INTERRUPT_IO (1 << 3)
+#define INTERRUPT_MCHK   (1 << 4)
 
 /* Program Status Word.  */
 #define S390_PSWM_REGNUM 0
@@ -841,6 +869,45 @@ static inline void cpu_inject_ext(CPUS390XState *env, 
uint32_t code, uint32_t pa
 cpu_interrupt(env, CPU_INTERRUPT_HARD);
 }
 
+static inline void cpu_inject_io(CPUS390XState *env, uint16_t subchannel_id,
+ uint16_t subchannel_number,
+ uint32_t io_int_parm, uint32_t io_int_word)
+{
+int isc = ffs(io_int_word << 2) - 1;
+
+if (env->io_index[isc] == MAX_IO_QUEUE - 1) {
+/* ugh - can't queue anymore. Let's drop. */
+return;
+}
+
+env->io_index[isc]++;
+assert(env->io_index[isc] < MAX_IO_QUEUE);
+
+env->io_queue[env->io_index[isc]][isc].id = subchannel_id;
+env->io_queue[env->io_index[isc]][isc].nr = subchannel_number;
+env->io_queue[env->io_index[isc]][isc].parm = io_int_parm;
+env->io_queue[env->io_index[isc]][isc].word = io_int_word;
+
+env->pending_int |= INTERRUPT_IO;
+cpu_interrupt(env, CPU_INTERRUPT_HARD);
+}
+
+static inline void cpu_inject_crw_mchk(CPUS390XState *env)
+{
+if (env->mchk_index == MAX_MCHK_QUEUE - 1) {
+/* ugh - can't queue anymore. Let's drop. */
+return;
+}
+
+env->mchk_index++;
+assert(env->mchk_index < MAX_MCHK_QUEUE);
+
+env->mchk_queue[env->mchk_index].type = 1;
+
+env->pending_int |= INTERRUPT_MCHK;
+cpu_interrupt(env, CPU_INTERRUPT_HARD);
+}
+
 static inline bool cpu_has_work(CPUState *cpu)
 {
 CPUS390XState *env = &S390_CPU(cpu)->env;
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 3109c77..857c897 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -614,12 +614,140 @@ static void do_ext_interrupt(CPUS390XState *env)
 load_psw(env, mask, addr);
 }
 
+static void do_io_interrupt(CPUS390XState *env)
+{
+uint64_t mask, addr;
+LowCore *lowcore;
+IOIntQueue *q;
+uint8_t isc;
+int disable = 1;
+int found = 0;
+
+if (!(env->psw.mask & PSW_MASK_IO)) {
+cpu_abort(env, "I/O int w/o I/O mask\n");
+}
+
+for (isc = 0; isc < ARRAY_SIZE(env->io_index); isc++) {
+if (env->io_index[isc] < 0) {
+continue;
+}
+if (env->io_index[isc] > MAX_IO_QUEUE) {
+cpu_abort(env, "I/O queue overrun for isc %d: %d\n",
+  isc, env->io_index[isc]);
+}
+
+q = &env->io_queue[env->io_index[isc]][isc];
+if (!(env->cregs[6] & q->word)) {
+disable = 0;
+continue;
+}
+found = 1;
+lowcore = cpu_map_lowcore(env);
+
+lowcore->subchannel_id = cpu_to_be16(q->id);
+lowcore->subchannel_nr = cpu_to_be16(q->nr);
+lowcore->io_int_parm = cpu_to_be32(q->parm);
+lowcore->io_int_word = cpu_to_be32(q->word);
+lowcore->io_old_psw.mask = cpu_to_be64(get_psw_mask(env));
+lowcore->io_old_psw.addr = cpu_to

[Qemu-devel] [PATCH 12/19] virtio-s390: add a reset function to virtio-s390 devices

2013-01-29 Thread Alexander Graf
From: Paolo Bonzini 

virtio-s390 devices are not being reset when their bus is.  To fix
this, add a reset method that forwards to virtio_reset.  This is
only needed because of the "strange" modeling of virtio devices;
the ->vdev link is being handled manually rather than through qdev.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Alexander Graf 
---
 hw/s390-virtio-bus.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index b5d1f2b..6858db0 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -508,6 +508,13 @@ static int s390_virtio_busdev_init(DeviceState *dev)
 return _info->init(_dev);
 }
 
+static void s390_virtio_busdev_reset(DeviceState *dev)
+{
+VirtIOS390Device *_dev = (VirtIOS390Device *)dev;
+
+virtio_reset(_dev->vdev);
+}
+
 static void virtio_s390_device_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -515,6 +522,7 @@ static void virtio_s390_device_class_init(ObjectClass 
*klass, void *data)
 dc->init = s390_virtio_busdev_init;
 dc->bus_type = TYPE_S390_VIRTIO_BUS;
 dc->unplug = qdev_simple_unplug_cb;
+dc->reset = s390_virtio_busdev_reset;
 }
 
 static const TypeInfo virtio_s390_device_info = {
-- 
1.6.0.2




[Qemu-devel] [PULL 00/19] s390 patch queue 2013-01-29

2013-01-29 Thread Alexander Graf
Hi Blue / Aurelien,

This is my current patch queue for s390. This time without build breakage
hopefully :).

Please pull.

Alex


The following changes since commit ec9466ff2e50213c8318ffdd7003f345278ab795:
  Anthony Liguori (1):
Merge remote-tracking branch 'afaerber/qom-cpu' into staging

are available in the git repository at:

  git://repo.or.cz/qemu/agraf.git s390-for-upstream

Alexander Graf (3):
  s390: Add default support for SCLP console
  s390: Make typeinfo const
  s390: Move hw files to hw/s390x

Andreas Färber (1):
  s390-virtio: Check for NULL device in reset hypercall

Christian Borntraeger (1):
  sclpconsole: Don't instantiate sclpconsole with -nodefaults

Cornelia Huck (13):
  s390: Lowcore mapping helper.
  s390: Add mapping helper functions.
  s390: Channel I/O basic definitions.
  s390: I/O interrupt and machine check injection.
  s390: Add channel I/O instructions.
  s390: Virtual channel subsystem support.
  s390: Wire up channel I/O in kvm.
  s390-virtio: Factor out some initialization code.
  s390: Add new channel I/O based virtio transport.
  s390: Add s390-ccw-virtio machine.
  s390: Use s390_cpu_physical_memory_map for tpi.
  s390: css error codes.
  s390: Drop set_bit usage in virtio_ccw.

Paolo Bonzini (1):
  virtio-s390: add a reset function to virtio-s390 devices

 hw/boards.h  |1 +
 hw/s390-virtio.h |   22 -
 hw/s390x/Makefile.objs   |5 +-
 hw/s390x/css.c   | 1277 ++
 hw/s390x/css.h   |   99 +++
 hw/s390x/ipl.c   |2 +-
 hw/{ => s390x}/s390-virtio-bus.c |   16 +-
 hw/{ => s390x}/s390-virtio-bus.h |   12 +-
 hw/s390x/s390-virtio-ccw.c   |  134 
 hw/s390x/s390-virtio-hcall.c |2 +-
 hw/{ => s390x}/s390-virtio.c |  131 +++--
 hw/s390x/s390-virtio.h   |   28 +
 hw/s390x/virtio-ccw.c|  960 
 hw/s390x/virtio-ccw.h|   98 +++
 target-s390x/Makefile.objs   |2 +-
 target-s390x/cpu.h   |  247 -
 target-s390x/helper.c|  200 ++-
 target-s390x/ioinst.c|  761 +++
 target-s390x/ioinst.h|  230 +++
 target-s390x/kvm.c   |  239 +++-
 trace-events |   18 +
 vl.c |   52 ++
 22 files changed, 4420 insertions(+), 116 deletions(-)
 delete mode 100644 hw/s390-virtio.h
 create mode 100644 hw/s390x/css.c
 create mode 100644 hw/s390x/css.h
 rename hw/{ => s390x}/s390-virtio-bus.c (98%)
 rename hw/{ => s390x}/s390-virtio-bus.h (95%)
 create mode 100644 hw/s390x/s390-virtio-ccw.c
 rename hw/{ => s390x}/s390-virtio.c (85%)
 create mode 100644 hw/s390x/s390-virtio.h
 create mode 100644 hw/s390x/virtio-ccw.c
 create mode 100644 hw/s390x/virtio-ccw.h
 create mode 100644 target-s390x/ioinst.c
 create mode 100644 target-s390x/ioinst.h



[Qemu-devel] [PATCH 02/19] s390: Lowcore mapping helper.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Create a lowcore mapping helper that includes a check for sufficient
length.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 target-s390x/helper.c |   34 +-
 1 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 9a132e6..023c074 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -471,13 +471,31 @@ static uint64_t get_psw_mask(CPUS390XState *env)
 return r;
 }
 
+static LowCore *cpu_map_lowcore(CPUS390XState *env)
+{
+LowCore *lowcore;
+hwaddr len = sizeof(LowCore);
+
+lowcore = cpu_physical_memory_map(env->psa, &len, 1);
+
+if (len < sizeof(LowCore)) {
+cpu_abort(env, "Could not map lowcore\n");
+}
+
+return lowcore;
+}
+
+static void cpu_unmap_lowcore(LowCore *lowcore)
+{
+cpu_physical_memory_unmap(lowcore, sizeof(LowCore), 1, sizeof(LowCore));
+}
+
 static void do_svc_interrupt(CPUS390XState *env)
 {
 uint64_t mask, addr;
 LowCore *lowcore;
-hwaddr len = TARGET_PAGE_SIZE;
 
-lowcore = cpu_physical_memory_map(env->psa, &len, 1);
+lowcore = cpu_map_lowcore(env);
 
 lowcore->svc_code = cpu_to_be16(env->int_svc_code);
 lowcore->svc_ilen = cpu_to_be16(env->int_svc_ilen);
@@ -486,7 +504,7 @@ static void do_svc_interrupt(CPUS390XState *env)
 mask = be64_to_cpu(lowcore->svc_new_psw.mask);
 addr = be64_to_cpu(lowcore->svc_new_psw.addr);
 
-cpu_physical_memory_unmap(lowcore, len, 1, len);
+cpu_unmap_lowcore(lowcore);
 
 load_psw(env, mask, addr);
 }
@@ -495,7 +513,6 @@ static void do_program_interrupt(CPUS390XState *env)
 {
 uint64_t mask, addr;
 LowCore *lowcore;
-hwaddr len = TARGET_PAGE_SIZE;
 int ilen = env->int_pgm_ilen;
 
 switch (ilen) {
@@ -513,7 +530,7 @@ static void do_program_interrupt(CPUS390XState *env)
 qemu_log_mask(CPU_LOG_INT, "%s: code=0x%x ilen=%d\n",
   __func__, env->int_pgm_code, ilen);
 
-lowcore = cpu_physical_memory_map(env->psa, &len, 1);
+lowcore = cpu_map_lowcore(env);
 
 lowcore->pgm_ilen = cpu_to_be16(ilen);
 lowcore->pgm_code = cpu_to_be16(env->int_pgm_code);
@@ -522,7 +539,7 @@ static void do_program_interrupt(CPUS390XState *env)
 mask = be64_to_cpu(lowcore->program_new_psw.mask);
 addr = be64_to_cpu(lowcore->program_new_psw.addr);
 
-cpu_physical_memory_unmap(lowcore, len, 1, len);
+cpu_unmap_lowcore(lowcore);
 
 DPRINTF("%s: %x %x %" PRIx64 " %" PRIx64 "\n", __func__,
 env->int_pgm_code, ilen, env->psw.mask,
@@ -537,7 +554,6 @@ static void do_ext_interrupt(CPUS390XState *env)
 {
 uint64_t mask, addr;
 LowCore *lowcore;
-hwaddr len = TARGET_PAGE_SIZE;
 ExtQueue *q;
 
 if (!(env->psw.mask & PSW_MASK_EXT)) {
@@ -549,7 +565,7 @@ static void do_ext_interrupt(CPUS390XState *env)
 }
 
 q = &env->ext_queue[env->ext_index];
-lowcore = cpu_physical_memory_map(env->psa, &len, 1);
+lowcore = cpu_map_lowcore(env);
 
 lowcore->ext_int_code = cpu_to_be16(q->code);
 lowcore->ext_params = cpu_to_be32(q->param);
@@ -560,7 +576,7 @@ static void do_ext_interrupt(CPUS390XState *env)
 mask = be64_to_cpu(lowcore->external_new_psw.mask);
 addr = be64_to_cpu(lowcore->external_new_psw.addr);
 
-cpu_physical_memory_unmap(lowcore, len, 1, len);
+cpu_unmap_lowcore(lowcore);
 
 env->ext_index--;
 if (env->ext_index == -1) {
-- 
1.6.0.2




[Qemu-devel] [PATCH] sparc: disable qtest in make check

2013-01-29 Thread Anthony Liguori
We've seen this repeatedly in buildbot but I can now reliably
reproduce it myself too.  With a few hundred runs of 'make check',
qemu-system-sparc will hang consuming 100% CPU.  I've attached GDB
to the hung process and unfortunately, I can't get anything useful
out of GDB (RIP is not a valid simple and there is nothing else on
the stack).

At any rate, since this only manifests in qemu-system-sparc and it
doesn't appear to be a qtest specific problem, I think we should
disable it until the problem is resolved.

Signed-off-by: Anthony Liguori 
---
 tests/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index c681ceb..c9cc24a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -63,8 +63,8 @@ check-qtest-i386-y += tests/rtc-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/mc146818rtc.c
 gcov-files-x86_64-y = $(subst 
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
-check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
-check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
+#check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
+#check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
 gcov-files-sparc-y += hw/m48t59.c
 gcov-files-sparc64-y += hw/m48t59.c
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
-- 
1.8.0




[Qemu-devel] [PATCH 16/19] sclpconsole: Don't instantiate sclpconsole with -nodefaults

2013-01-29 Thread Alexander Graf
From: Christian Borntraeger 

libvirt specifies nodefaults and creates an sclp console with special
parameters. Let qemu follow nodefaults and don't create an sclp
console if nodefaults is specified.

Signed-off-by: Christian Borntraeger 
Signed-off-by: Alexander Graf 
---
 vl.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 8b0961e..910abb6 100644
--- a/vl.c
+++ b/vl.c
@@ -3652,6 +3652,7 @@ int main(int argc, char **argv, char **envp)
 default_serial = 0;
 default_parallel = 0;
 default_virtcon = 0;
+default_sclp = 0;
 default_monitor = 0;
 default_net = 0;
 default_floppy = 0;
-- 
1.6.0.2




[Qemu-devel] [PATCH 04/19] s390: Channel I/O basic definitions.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Basic channel I/O structures and helper function.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 target-s390x/Makefile.objs |2 +-
 target-s390x/cpu.h |1 +
 target-s390x/ioinst.c  |   36 
 target-s390x/ioinst.h  |  207 
 4 files changed, 245 insertions(+), 1 deletions(-)
 create mode 100644 target-s390x/ioinst.c
 create mode 100644 target-s390x/ioinst.h

diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
index e728abf..3afb0b7 100644
--- a/target-s390x/Makefile.objs
+++ b/target-s390x/Makefile.objs
@@ -1,4 +1,4 @@
 obj-y += translate.o helper.o cpu.o interrupt.o
 obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
-obj-$(CONFIG_SOFTMMU) += machine.o
+obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o
 obj-$(CONFIG_KVM) += kvm.o
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 7951aab..c1a0040 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -300,6 +300,7 @@ int cpu_s390x_handle_mmu_fault (CPUS390XState *env, 
target_ulong address, int rw
 int mmu_idx);
 #define cpu_handle_mmu_fault cpu_s390x_handle_mmu_fault
 
+#include "ioinst.h"
 
 #ifndef CONFIG_USER_ONLY
 void *s390_cpu_physical_memory_map(CPUS390XState *env, hwaddr addr, hwaddr 
*len,
diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
new file mode 100644
index 000..06a16ee
--- /dev/null
+++ b/target-s390x/ioinst.c
@@ -0,0 +1,36 @@
+/*
+ * I/O instructions for S/390
+ *
+ * Copyright 2012 IBM Corp.
+ * Author(s): Cornelia Huck 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include 
+
+#include "cpu.h"
+#include "ioinst.h"
+
+int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int *ssid,
+ int *schid)
+{
+if (!IOINST_SCHID_ONE(value)) {
+return -EINVAL;
+}
+if (!IOINST_SCHID_M(value)) {
+if (IOINST_SCHID_CSSID(value)) {
+return -EINVAL;
+}
+*cssid = 0;
+*m = 0;
+} else {
+*cssid = IOINST_SCHID_CSSID(value);
+*m = 1;
+}
+*ssid = IOINST_SCHID_SSID(value);
+*schid = IOINST_SCHID_NR(value);
+return 0;
+}
diff --git a/target-s390x/ioinst.h b/target-s390x/ioinst.h
new file mode 100644
index 000..037aabc
--- /dev/null
+++ b/target-s390x/ioinst.h
@@ -0,0 +1,207 @@
+/*
+ * S/390 channel I/O instructions
+ *
+ * Copyright 2012 IBM Corp.
+ * Author(s): Cornelia Huck 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+*/
+
+#ifndef IOINST_S390X_H
+#define IOINST_S390X_H
+/*
+ * Channel I/O related definitions, as defined in the Principles
+ * Of Operation (and taken from the Linux implementation).
+ */
+
+/* subchannel status word (command mode only) */
+typedef struct SCSW {
+uint16_t flags;
+uint16_t ctrl;
+uint32_t cpa;
+uint8_t dstat;
+uint8_t cstat;
+uint16_t count;
+} QEMU_PACKED SCSW;
+
+#define SCSW_FLAGS_MASK_KEY 0xf000
+#define SCSW_FLAGS_MASK_SCTL 0x0800
+#define SCSW_FLAGS_MASK_ESWF 0x0400
+#define SCSW_FLAGS_MASK_CC 0x0300
+#define SCSW_FLAGS_MASK_FMT 0x0080
+#define SCSW_FLAGS_MASK_PFCH 0x0040
+#define SCSW_FLAGS_MASK_ISIC 0x0020
+#define SCSW_FLAGS_MASK_ALCC 0x0010
+#define SCSW_FLAGS_MASK_SSI 0x0008
+#define SCSW_FLAGS_MASK_ZCC 0x0004
+#define SCSW_FLAGS_MASK_ECTL 0x0002
+#define SCSW_FLAGS_MASK_PNO 0x0001
+
+#define SCSW_CTRL_MASK_FCTL 0x7000
+#define SCSW_CTRL_MASK_ACTL 0x0fe0
+#define SCSW_CTRL_MASK_STCTL 0x001f
+
+#define SCSW_FCTL_CLEAR_FUNC 0x1000
+#define SCSW_FCTL_HALT_FUNC 0x2000
+#define SCSW_FCTL_START_FUNC 0x4000
+
+#define SCSW_ACTL_SUSP 0x0020
+#define SCSW_ACTL_DEVICE_ACTIVE 0x0040
+#define SCSW_ACTL_SUBCH_ACTIVE 0x0080
+#define SCSW_ACTL_CLEAR_PEND 0x0100
+#define SCSW_ACTL_HALT_PEND  0x0200
+#define SCSW_ACTL_START_PEND 0x0400
+#define SCSW_ACTL_RESUME_PEND 0x0800
+
+#define SCSW_STCTL_STATUS_PEND 0x0001
+#define SCSW_STCTL_SECONDARY 0x0002
+#define SCSW_STCTL_PRIMARY 0x0004
+#define SCSW_STCTL_INTERMEDIATE 0x0008
+#define SCSW_STCTL_ALERT 0x0010
+
+#define SCSW_DSTAT_ATTENTION 0x80
+#define SCSW_DSTAT_STAT_MOD  0x40
+#define SCSW_DSTAT_CU_END0x20
+#define SCSW_DSTAT_BUSY  0x10
+#define SCSW_DSTAT_CHANNEL_END   0x08
+#define SCSW_DSTAT_DEVICE_END0x04
+#define SCSW_DSTAT_UNIT_CHECK0x02
+#define SCSW_DSTAT_UNIT_EXCEP0x01
+
+#define SCSW_CSTAT_PCI   0x80
+#define SCSW_CSTAT_INCORR_LEN0x40
+#define SCSW_CSTAT_PROG_CHECK0x20
+#define SCSW_CSTAT_PROT_CHECK0x10
+#define SCSW_CSTAT_DATA_CHECK0x08
+#define SCSW_CSTAT_CHN_CTRL_CHK  0x04
+#define SCSW_CSTAT_INTF_CTRL_CHK 0x02
+#define SCSW_CSTAT_CHAIN_CHECK   0x01
+
+/* path management control word */
+ty

Re: [Qemu-devel] [PATCH V2 11/20] tap: support enabling or disabling a queue

2013-01-29 Thread Michael S. Tsirkin
On Tue, Jan 29, 2013 at 08:10:26PM +, Blue Swirl wrote:
> On Tue, Jan 29, 2013 at 1:50 PM, Jason Wang  wrote:
> > On 01/26/2013 03:13 AM, Blue Swirl wrote:
> >> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang  wrote:
> >>> This patch introduce a new bit - enabled in TAPState which tracks whether 
> >>> a
> >>> specific queue/fd is enabled. The tap/fd is enabled during initialization 
> >>> and
> >>> could be enabled/disabled by tap_enalbe() and tap_disable() which calls 
> >>> platform
> >>> specific helpers to do the real work. Polling of a tap fd can only done 
> >>> when
> >>> the tap was enabled.
> >>>
> >>> Signed-off-by: Jason Wang 
> >>> ---
> >>>  include/net/tap.h |2 ++
> >>>  net/tap-win32.c   |   10 ++
> >>>  net/tap.c |   43 ---
> >>>  3 files changed, 52 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/include/net/tap.h b/include/net/tap.h
> >>> index bb7efb5..0caf8c4 100644
> >>> --- a/include/net/tap.h
> >>> +++ b/include/net/tap.h
> >>> @@ -35,6 +35,8 @@ int tap_has_vnet_hdr_len(NetClientState *nc, int len);
> >>>  void tap_using_vnet_hdr(NetClientState *nc, int using_vnet_hdr);
> >>>  void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, 
> >>> int ecn, int ufo);
> >>>  void tap_set_vnet_hdr_len(NetClientState *nc, int len);
> >>> +int tap_enable(NetClientState *nc);
> >>> +int tap_disable(NetClientState *nc);
> >>>
> >>>  int tap_get_fd(NetClientState *nc);
> >>>
> >>> diff --git a/net/tap-win32.c b/net/tap-win32.c
> >>> index 265369c..a2cd94b 100644
> >>> --- a/net/tap-win32.c
> >>> +++ b/net/tap-win32.c
> >>> @@ -764,3 +764,13 @@ void tap_set_vnet_hdr_len(NetClientState *nc, int 
> >>> len)
> >>>  {
> >>>  assert(0);
> >>>  }
> >>> +
> >>> +int tap_enable(NetClientState *nc)
> >>> +{
> >>> +assert(0);
> >> abort()
> >
> > This is just to be consistent with the reset of the helpers in this file.
> >>
> >>> +}
> >>> +
> >>> +int tap_disable(NetClientState *nc)
> >>> +{
> >>> +assert(0);
> >>> +}
> >>> diff --git a/net/tap.c b/net/tap.c
> >>> index 67080f1..95e557b 100644
> >>> --- a/net/tap.c
> >>> +++ b/net/tap.c
> >>> @@ -59,6 +59,7 @@ typedef struct TAPState {
> >>>  unsigned int write_poll : 1;
> >>>  unsigned int using_vnet_hdr : 1;
> >>>  unsigned int has_ufo: 1;
> >>> +unsigned int enabled : 1;
> >> bool without bit field?
> >
> > Also to be consistent with other field. If you wish I can send patches
> > to convert all those bit field to bool on top of this series.
> 
> That would be nice, likewise for the assert(0).

OK so let's go ahead with this patchset as is,
and a cleanup patch will be send after 1.4 then.


> >
> > Thanks
> >>>  VHostNetState *vhost_net;
> >>>  unsigned host_vnet_hdr_len;
> >>>  } TAPState;
> >>> @@ -72,9 +73,9 @@ static void tap_writable(void *opaque);
> >>>  static void tap_update_fd_handler(TAPState *s)
> >>>  {
> >>>  qemu_set_fd_handler2(s->fd,
> >>> - s->read_poll  ? tap_can_send : NULL,
> >>> - s->read_poll  ? tap_send : NULL,
> >>> - s->write_poll ? tap_writable : NULL,
> >>> + s->read_poll && s->enabled ? tap_can_send : 
> >>> NULL,
> >>> + s->read_poll && s->enabled ? tap_send : 
> >>> NULL,
> >>> + s->write_poll && s->enabled ? tap_writable : 
> >>> NULL,
> >>>   s);
> >>>  }
> >>>
> >>> @@ -339,6 +340,7 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
> >>>  s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
> >>>  s->using_vnet_hdr = 0;
> >>>  s->has_ufo = tap_probe_has_ufo(s->fd);
> >>> +s->enabled = 1;
> >>>  tap_set_offload(&s->nc, 0, 0, 0, 0, 0);
> >>>  /*
> >>>   * Make sure host header length is set correctly in tap:
> >>> @@ -737,3 +739,38 @@ VHostNetState *tap_get_vhost_net(NetClientState *nc)
> >>>  assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
> >>>  return s->vhost_net;
> >>>  }
> >>> +
> >>> +int tap_enable(NetClientState *nc)
> >>> +{
> >>> +TAPState *s = DO_UPCAST(TAPState, nc, nc);
> >>> +int ret;
> >>> +
> >>> +if (s->enabled) {
> >>> +return 0;
> >>> +} else {
> >>> +ret = tap_fd_enable(s->fd);
> >>> +if (ret == 0) {
> >>> +s->enabled = 1;
> >>> +tap_update_fd_handler(s);
> >>> +}
> >>> +return ret;
> >>> +}
> >>> +}
> >>> +
> >>> +int tap_disable(NetClientState *nc)
> >>> +{
> >>> +TAPState *s = DO_UPCAST(TAPState, nc, nc);
> >>> +int ret;
> >>> +
> >>> +if (s->enabled == 0) {
> >>> +return 0;
> >>> +} else {
> >>> +ret = tap_fd_disable(s->fd);
> >>> +if (ret == 0) {
> >>> +qemu_purge_queued_packets(nc);
> >>> +s->enabled = 0;
> >>> +tap_update_fd_handler(s);
> >>> +}
> >>> +return ret;
> >

[Qemu-devel] [Bug 659351] Re: QEMU uses obsolete gethostbyname and inet_aton rather than getaddrinfo

2013-01-29 Thread Jesse Larrew
** Changed in: qemu
 Assignee: (unassigned) => Jesse Larrew (jlarrew)

** Changed in: qemu
   Status: New => In Progress

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

Title:
  QEMU uses obsolete gethostbyname and inet_aton rather than getaddrinfo

Status in QEMU:
  In Progress

Bug description:
  In several places, including the tcp migration code, qemu uses
  gethostbyname and inet_aton to construct a sockaddr_in. These should
  be replaced by the more modern getaddrinfo for both connect and
  accept, along with a loop to try all returned addrinfo structs on the
  connecting side. This would also allow migration over IPv6.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/659351/+subscriptions



[Qemu-devel] [PATCH 06/19] s390: Add channel I/O instructions.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Provide handlers for (most) channel I/O instructions.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 target-s390x/cpu.h|  100 +++
 target-s390x/ioinst.c |  716 +
 target-s390x/ioinst.h |   16 ++
 trace-events  |6 +
 4 files changed, 838 insertions(+), 0 deletions(-)

diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 3e00d38..76a822c 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -147,6 +147,9 @@ static inline void cpu_clone_regs(CPUS390XState *env, 
target_ulong newsp)
 }
 #endif
 
+/* distinguish between 24 bit and 31 bit addressing */
+#define HIGH_ORDER_BIT 0x8000
+
 /* Interrupt Codes */
 /* Program Interrupts */
 #define PGM_OPERATION   0x0001
@@ -331,6 +334,20 @@ void *s390_cpu_physical_memory_map(CPUS390XState *env, 
hwaddr addr, hwaddr *len,
int is_write);
 void s390_cpu_physical_memory_unmap(CPUS390XState *env, void *addr, hwaddr len,
 int is_write);
+static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb)
+{
+hwaddr addr = 0;
+uint8_t reg;
+
+reg = ipb >> 28;
+if (reg > 0) {
+addr = env->regs[reg];
+}
+addr += (ipb >> 16) & 0xfff;
+
+return addr;
+}
+
 void s390x_tod_timer(void *opaque);
 void s390x_cpu_timer(void *opaque);
 
@@ -380,6 +397,89 @@ static inline unsigned s390_del_running_cpu(CPUS390XState 
*env)
 void cpu_lock(void);
 void cpu_unlock(void);
 
+typedef struct SubchDev SubchDev;
+
+static inline SubchDev *css_find_subch(uint8_t m, uint8_t cssid, uint8_t ssid,
+   uint16_t schid)
+{
+return NULL;
+}
+static inline bool css_subch_visible(SubchDev *sch)
+{
+return false;
+}
+static inline void css_conditional_io_interrupt(SubchDev *sch)
+{
+}
+static inline int css_do_stsch(SubchDev *sch, SCHIB *schib)
+{
+return -ENODEV;
+}
+static inline bool css_schid_final(uint8_t cssid, uint8_t ssid, uint16_t schid)
+{
+return true;
+}
+static inline int css_do_msch(SubchDev *sch, SCHIB *schib)
+{
+return -ENODEV;
+}
+static inline int css_do_xsch(SubchDev *sch)
+{
+return -ENODEV;
+}
+static inline int css_do_csch(SubchDev *sch)
+{
+return -ENODEV;
+}
+static inline int css_do_hsch(SubchDev *sch)
+{
+return -ENODEV;
+}
+static inline int css_do_ssch(SubchDev *sch, ORB *orb)
+{
+return -ENODEV;
+}
+static inline int css_do_tsch(SubchDev *sch, IRB *irb)
+{
+return -ENODEV;
+}
+static inline int css_do_stcrw(CRW *crw)
+{
+return 1;
+}
+static inline int css_do_tpi(uint64_t addr, int lowcore)
+{
+return 0;
+}
+static inline int css_collect_chp_desc(int m, uint8_t cssid, uint8_t f_chpid,
+   int rfmt, uint8_t l_chpid, void *buf)
+{
+return 0;
+}
+static inline void css_do_schm(uint8_t mbk, int update, int dct, uint64_t mbo)
+{
+}
+static inline int css_enable_mss(void)
+{
+return -EINVAL;
+}
+static inline int css_enable_mcsse(void)
+{
+return -EINVAL;
+}
+static inline int css_do_rsch(SubchDev *sch)
+{
+return -ENODEV;
+}
+static inline int css_do_rchp(uint8_t cssid, uint8_t chpid)
+{
+return -ENODEV;
+}
+static inline bool css_present(uint8_t cssid)
+{
+return false;
+}
+
 static inline void cpu_set_tls(CPUS390XState *env, target_ulong newtls)
 {
 env->aregs[0] = newtls >> 32;
diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 06a16ee..4ef2d73 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -13,6 +13,7 @@
 
 #include "cpu.h"
 #include "ioinst.h"
+#include "trace.h"
 
 int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int *ssid,
  int *schid)
@@ -34,3 +35,718 @@ int ioinst_disassemble_sch_ident(uint32_t value, int *m, 
int *cssid, int *ssid,
 *schid = IOINST_SCHID_NR(value);
 return 0;
 }
+
+int ioinst_handle_xsch(CPUS390XState *env, uint64_t reg1)
+{
+int cssid, ssid, schid, m;
+SubchDev *sch;
+int ret = -ENODEV;
+int cc;
+
+if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
+program_interrupt(env, PGM_OPERAND, 2);
+return -EIO;
+}
+trace_ioinst_sch_id("xsch", cssid, ssid, schid);
+sch = css_find_subch(m, cssid, ssid, schid);
+if (sch && css_subch_visible(sch)) {
+ret = css_do_xsch(sch);
+}
+switch (ret) {
+case -ENODEV:
+cc = 3;
+break;
+case -EBUSY:
+cc = 2;
+break;
+case 0:
+cc = 0;
+break;
+default:
+cc = 1;
+break;
+}
+
+return cc;
+}
+
+int ioinst_handle_csch(CPUS390XState *env, uint64_t reg1)
+{
+int cssid, ssid, schid, m;
+SubchDev *sch;
+int ret = -ENODEV;
+int cc;
+
+if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
+program_interrupt(env, PGM_OPERAND, 2);
+re

[Qemu-devel] [PATCH 18/19] s390: css error codes.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Changed error codes in the channel subsystem / virtio-ccw code
(-EOPNOTSUPP -> -ENOSYS, -ERESTART -> -EINPROGRESS).

This should hopefully fix building on mingw32.

Signed-off-by: Cornelia Huck 
Reviewed-by: Stefan Weil 
Signed-off-by: Alexander Graf 
---
 hw/s390x/css.c|8 
 hw/s390x/virtio-ccw.c |2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 84efd4a..3244201 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -223,7 +223,7 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr)
 }
 
 if (ccw.flags & CCW_FLAG_SUSPEND) {
-return -ERESTART;
+return -EINPROGRESS;
 }
 
 check_len = !((ccw.flags & CCW_FLAG_SLI) && !(ccw.flags & CCW_FLAG_DC));
@@ -291,7 +291,7 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr)
 /* Handle device specific commands. */
 ret = sch->ccw_cb(sch, ccw);
 } else {
-ret = -EOPNOTSUPP;
+ret = -ENOSYS;
 }
 break;
 }
@@ -347,7 +347,7 @@ static void sch_handle_start_func(SubchDev *sch)
 SCSW_STCTL_STATUS_PEND;
 s->dstat = SCSW_DSTAT_CHANNEL_END | SCSW_DSTAT_DEVICE_END;
 break;
-case -EOPNOTSUPP:
+case -ENOSYS:
 /* unsupported command, generate unit check (command reject) */
 s->ctrl &= ~SCSW_ACTL_START_PEND;
 s->dstat = SCSW_DSTAT_UNIT_CHECK;
@@ -372,7 +372,7 @@ static void sch_handle_start_func(SubchDev *sch)
 s->ctrl &= ~SCSW_CTRL_MASK_STCTL;
 s->ctrl |= SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND;
 break;
-case -ERESTART:
+case -EINPROGRESS:
 /* channel program has been suspended */
 s->ctrl &= ~SCSW_ACTL_START_PEND;
 s->ctrl |= SCSW_ACTL_SUSP;
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 8c9b745..7d7f336 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -384,7 +384,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
 }
 break;
 default:
-ret = -EOPNOTSUPP;
+ret = -ENOSYS;
 break;
 }
 return ret;
-- 
1.6.0.2




[Qemu-devel] [PATCH 17/19] s390: Use s390_cpu_physical_memory_map for tpi.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Map the I/O interruption code before calling into css.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 hw/s390x/css.c|2 +-
 target-s390x/cpu.h|4 ++--
 target-s390x/ioinst.c |   19 ++-
 target-s390x/ioinst.h |7 +++
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 113ac9a..84efd4a 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -852,7 +852,7 @@ int css_do_stcrw(CRW *crw)
 return ret;
 }
 
-int css_do_tpi(uint64_t addr, int lowcore)
+int css_do_tpi(IOIntCode *int_code, int lowcore)
 {
 /* No pending interrupts for !KVM. */
 return 0;
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index ce12fa4..9be4a47 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -413,7 +413,7 @@ int css_do_hsch(SubchDev *sch);
 int css_do_ssch(SubchDev *sch, ORB *orb);
 int css_do_tsch(SubchDev *sch, IRB *irb);
 int css_do_stcrw(CRW *crw);
-int css_do_tpi(uint64_t addr, int lowcore);
+int css_do_tpi(IOIntCode *int_code, int lowcore);
 int css_collect_chp_desc(int m, uint8_t cssid, uint8_t f_chpid, uint8_t 
l_chpid,
  int rfmt, void *buf);
 void css_do_schm(uint8_t mbk, int update, int dct, uint64_t mbo);
@@ -471,7 +471,7 @@ static inline int css_do_stcrw(CRW *crw)
 {
 return 1;
 }
-static inline int css_do_tpi(uint64_t addr, int lowcore)
+static inline int css_do_tpi(IOIntCode *int_code, int lowcore)
 {
 return 0;
 }
diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 4ef2d73..e3531f3 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -619,16 +619,25 @@ int ioinst_handle_tpi(CPUS390XState *env, uint32_t ipb)
 {
 uint64_t addr;
 int lowcore;
+IOIntCode *int_code;
+hwaddr len, orig_len;
+int ret;
 
 trace_ioinst("tpi");
 addr = decode_basedisp_s(env, ipb);
 lowcore = addr ? 0 : 1;
-if (addr < 8192) {
-addr += env->psa;
-} else if ((env->psa <= addr) && (addr < env->psa + 8192)) {
-addr -= env->psa;
+len = lowcore ? 8 /* two words */ : 12 /* three words */;
+orig_len = len;
+int_code = s390_cpu_physical_memory_map(env, addr, &len, 1);
+if (!int_code || (len != orig_len)) {
+program_interrupt(env, PGM_SPECIFICATION, 2);
+ret = -EIO;
+goto out;
 }
-return css_do_tpi(addr, lowcore);
+ret = css_do_tpi(int_code, lowcore);
+out:
+s390_cpu_physical_memory_unmap(env, int_code, len, 1);
+return ret;
 }
 
 #define SCHM_REG1_RES(_reg) (_reg & 0x0ffc)
diff --git a/target-s390x/ioinst.h b/target-s390x/ioinst.h
index a59742c..d5a43f4 100644
--- a/target-s390x/ioinst.h
+++ b/target-s390x/ioinst.h
@@ -195,6 +195,13 @@ typedef struct CRW {
 #define CRW_RSC_SUBCH 0x3
 #define CRW_RSC_CHP   0x4
 
+/* I/O interruption code */
+typedef struct IOIntCode {
+uint32_t subsys_id;
+uint32_t intparm;
+uint32_t interrupt_id;
+} QEMU_PACKED IOIntCode;
+
 /* schid disintegration */
 #define IOINST_SCHID_ONE(_schid)   ((_schid & 0x0001) >> 16)
 #define IOINST_SCHID_M(_schid) ((_schid & 0x0008) >> 19)
-- 
1.6.0.2




[Qemu-devel] [PATCH v3] PPC: Unify dcbzl code path

2013-01-29 Thread Alexander Graf
The bit that makes a dcbz instruction a dcbzl instruction was declared as
reserved in ppc32 ISAs. However, hardware simply ignores the bit, making
code valid if it simply invokes dcbzl instead of dcbz even on 750 and G4.

Thus, mark the bit as unreserved so that we properly emulate a simple dcbz
in case we're running on non-G5s.

While at it, also refactor the code to check the 970 special case during
runtime. This way we don't need to differenciate between a 970 dcbz and
any other dcbz anymore. We also allow for future improvements to add e500mc
dcbz handling.

Reported-by: Amadeusz Sławiński 
Signed-off-by: Alexander Graf 

---

v2 -> v3:

  - fix tcg_temp_free for i32
---
 target-ppc/cpu.h|6 ++
 target-ppc/helper.h |3 +--
 target-ppc/mem_helper.c |   21 -
 target-ppc/translate.c  |   33 -
 target-ppc/translate_init.c |   10 +-
 5 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 953146e..8c081db 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1857,10 +1857,8 @@ enum {
 PPC_CACHE  = 0x0002ULL,
 /*   icbi instruction*/
 PPC_CACHE_ICBI = 0x0004ULL,
-/*   dcbz instruction with fixed cache line size */
+/*   dcbz instruction*/
 PPC_CACHE_DCBZ = 0x0008ULL,
-/*   dcbz instruction with tunable cache line size   */
-PPC_CACHE_DCBZT= 0x0010ULL,
 /*   dcba instruction*/
 PPC_CACHE_DCBA = 0x0020ULL,
 /*   Freescale cache locking instructions*/
@@ -1928,7 +1926,7 @@ enum {
 | PPC_MEM_TLBIE | PPC_MEM_TLBSYNC \
 | PPC_MEM_SYNC | PPC_MEM_EIEIO \
 | PPC_CACHE | PPC_CACHE_ICBI \
-| PPC_CACHE_DCBZ | PPC_CACHE_DCBZT \
+| PPC_CACHE_DCBZ \
 | PPC_CACHE_DCBA | PPC_CACHE_LOCK \
 | PPC_EXTERN | PPC_SEGMENT | PPC_6xx_TLB \
 | PPC_74xx_TLB | PPC_40x_TLB | PPC_SEGMENT_64B \
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 83139d5..18e0394 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -25,8 +25,7 @@ DEF_HELPER_3(stmw, void, env, tl, i32)
 DEF_HELPER_4(lsw, void, env, tl, i32, i32)
 DEF_HELPER_5(lswx, void, env, tl, i32, i32, i32)
 DEF_HELPER_4(stsw, void, env, tl, i32, i32)
-DEF_HELPER_2(dcbz, void, env, tl)
-DEF_HELPER_2(dcbz_970, void, env, tl)
+DEF_HELPER_3(dcbz, void, env, tl, i32)
 DEF_HELPER_2(icbi, void, env, tl)
 DEF_HELPER_5(lscbx, tl, env, tl, i32, i32, i32)
 
diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index 902b1cd..ba383c8 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -136,18 +136,21 @@ static void do_dcbz(CPUPPCState *env, target_ulong addr, 
int dcache_line_size)
 }
 }
 
-void helper_dcbz(CPUPPCState *env, target_ulong addr)
+void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t is_dcbzl)
 {
-do_dcbz(env, addr, env->dcache_line_size);
-}
+int dcbz_size = env->dcache_line_size;
 
-void helper_dcbz_970(CPUPPCState *env, target_ulong addr)
-{
-if (((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) {
-do_dcbz(env, addr, 32);
-} else {
-do_dcbz(env, addr, env->dcache_line_size);
+#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64)
+if (!is_dcbzl &&
+(env->excp_model == POWERPC_EXCP_970) &&
+((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) {
+dcbz_size = 32;
 }
+#endif
+
+/* XXX add e500mc support */
+
+do_dcbz(env, addr, dcbz_size);
 }
 
 void helper_icbi(CPUPPCState *env, target_ulong addr)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 798b7ac..d96d1ed 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4118,29 +4118,21 @@ static void gen_dcbtst(DisasContext *ctx)
 /* dcbz */
 static void gen_dcbz(DisasContext *ctx)
 {
-TCGv t0;
-gen_set_access_type(ctx, ACCESS_CACHE);
-/* NIP cannot be restored if the memory exception comes from an helper */
-gen_update_nip(ctx, ctx->nip - 4);
-t0 = tcg_temp_new();
-gen_addr_reg_index(ctx, t0);
-gen_helper_dcbz(cpu_env, t0);
-tcg_temp_free(t0);
-}
+TCGv tcgv_addr;
+TCGv_i32 tcgv_is_dcbzl;
+int is_dcbzl = ctx->opcode & 0x0020 ? 1 : 0;
 
-static void gen_dcbz_970(DisasContext *ctx)
-{
-TCGv t0;
 gen_set_access_type(ctx, ACCESS_CACHE);
 /* NIP cannot be restored if the memory exception comes from an helper */
 gen_update_nip(ctx, ctx->nip - 4);
-t0 = tcg_temp_new();
-gen_addr_reg_index(ctx, t0);
-if (ctx->opcode & 0x0020)
-   

Re: [Qemu-devel] [PATCH 0/5] vhost-scsi: Add support for host virtualized target

2013-01-29 Thread Paolo Bonzini
> Also, I'm not exactly sure what's involved with a vhost=on/off
> frontend
> option discussed above, so if you or Paolo could handle this bit it
> would be very helpful.

I can do the forward port, if you take care of testing we have
enough time to make it into 1.4.

Paolo



Re: [Qemu-devel] KVM call minutes 2013-01-29

2013-01-29 Thread Alexander Graf

On 01/29/2013 04:41 PM, Juan Quintela wrote:

* Buildbot: discussed on the list (Andreas retired it)

* Replacing select(2) so that we will not hit the 1024 fd_set limit in the
   future. (stefan)

   Add checks for fd's bigger than 1024? multifunction devices uses lot
   of fd's for device.

   Portability?
   Use glib?  and let it use poll underneath.
   slirp is a problem.
   in the end loop: moving to a glib event loop, how we arrive there is the 
discussion.


* Outstanding virtio work for 1.4
   - Multiqueue virtio-net (Amos/Michael)
 version appeared today, problably it is on mergeable state
   - Refactorings (Fred/Peter)
 unlike before the hard freeze
   - virtio-ccw (Cornelia/Alex)
 conflict with multiqueue problably (alex)
 shouldn't (famous last words)
   - Do virtio-ccw used old style virtio API, and make integrating the
 refactorings more difficult?
   - Pushing refactorings to 1.5

* What's the plan for -device and IRQ assignment? (Alex)

   Alex will fill this


When using -device, we can not specify an IRQ line to attach to the 
device. This works for some special buses like PCI, but not in the 
generic case. We need it generically for virtio-mmio and for potential 
platform assigned vfio devices though.


The conclusion we came up with was that in order to model IRQ lines 
between arbitrary devices, we should use QOM and the QOM name space. 
Details are up for Anthony to fill in :).



Alex




[Qemu-devel] [PATCH 07/19] s390: Virtual channel subsystem support.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Provide a mechanism for qemu to provide fully virtual subchannels to
the guest.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 hw/s390x/Makefile.objs |1 +
 hw/s390x/css.c | 1277 
 hw/s390x/css.h |   99 
 target-s390x/cpu.h |   62 +++
 trace-events   |8 +
 5 files changed, 1447 insertions(+), 0 deletions(-)
 create mode 100644 hw/s390x/css.c
 create mode 100644 hw/s390x/css.h

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 1b40c2e..ab99da6 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -6,3 +6,4 @@ obj-y += sclp.o
 obj-y += event-facility.o
 obj-y += sclpquiesce.o sclpconsole.o
 obj-y += ipl.o
+obj-y += css.o
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
new file mode 100644
index 000..113ac9a
--- /dev/null
+++ b/hw/s390x/css.c
@@ -0,0 +1,1277 @@
+/*
+ * Channel subsystem base support.
+ *
+ * Copyright 2012 IBM Corp.
+ * Author(s): Cornelia Huck 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include 
+#include "qemu/bitops.h"
+#include "cpu.h"
+#include "ioinst.h"
+#include "css.h"
+#include "trace.h"
+
+typedef struct CrwContainer {
+CRW crw;
+QTAILQ_ENTRY(CrwContainer) sibling;
+} CrwContainer;
+
+typedef struct ChpInfo {
+uint8_t in_use;
+uint8_t type;
+uint8_t is_virtual;
+} ChpInfo;
+
+typedef struct SubchSet {
+SubchDev *sch[MAX_SCHID + 1];
+unsigned long schids_used[BITS_TO_LONGS(MAX_SCHID + 1)];
+unsigned long devnos_used[BITS_TO_LONGS(MAX_SCHID + 1)];
+} SubchSet;
+
+typedef struct CssImage {
+SubchSet *sch_set[MAX_SSID + 1];
+ChpInfo chpids[MAX_CHPID + 1];
+} CssImage;
+
+typedef struct ChannelSubSys {
+QTAILQ_HEAD(, CrwContainer) pending_crws;
+bool do_crw_mchk;
+bool crws_lost;
+uint8_t max_cssid;
+uint8_t max_ssid;
+bool chnmon_active;
+uint64_t chnmon_area;
+CssImage *css[MAX_CSSID + 1];
+uint8_t default_cssid;
+} ChannelSubSys;
+
+static ChannelSubSys *channel_subsys;
+
+int css_create_css_image(uint8_t cssid, bool default_image)
+{
+trace_css_new_image(cssid, default_image ? "(default)" : "");
+if (cssid > MAX_CSSID) {
+return -EINVAL;
+}
+if (channel_subsys->css[cssid]) {
+return -EBUSY;
+}
+channel_subsys->css[cssid] = g_malloc0(sizeof(CssImage));
+if (default_image) {
+channel_subsys->default_cssid = cssid;
+}
+return 0;
+}
+
+static uint16_t css_build_subchannel_id(SubchDev *sch)
+{
+if (channel_subsys->max_cssid > 0) {
+return (sch->cssid << 8) | (1 << 3) | (sch->ssid << 1) | 1;
+}
+return (sch->ssid << 1) | 1;
+}
+
+static void css_inject_io_interrupt(SubchDev *sch)
+{
+S390CPU *cpu = s390_cpu_addr2state(0);
+uint8_t isc = (sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ISC) >> 11;
+
+trace_css_io_interrupt(sch->cssid, sch->ssid, sch->schid,
+   sch->curr_status.pmcw.intparm, isc, "");
+s390_io_interrupt(cpu,
+  css_build_subchannel_id(sch),
+  sch->schid,
+  sch->curr_status.pmcw.intparm,
+  (0x80 >> isc) << 24);
+}
+
+void css_conditional_io_interrupt(SubchDev *sch)
+{
+/*
+ * If the subchannel is not currently status pending, make it pending
+ * with alert status.
+ */
+if (!(sch->curr_status.scsw.ctrl & SCSW_STCTL_STATUS_PEND)) {
+S390CPU *cpu = s390_cpu_addr2state(0);
+uint8_t isc = (sch->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ISC) >> 
11;
+
+trace_css_io_interrupt(sch->cssid, sch->ssid, sch->schid,
+   sch->curr_status.pmcw.intparm, isc,
+   "(unsolicited)");
+sch->curr_status.scsw.ctrl &= ~SCSW_CTRL_MASK_STCTL;
+sch->curr_status.scsw.ctrl |=
+SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND;
+/* Inject an I/O interrupt. */
+s390_io_interrupt(cpu,
+  css_build_subchannel_id(sch),
+  sch->schid,
+  sch->curr_status.pmcw.intparm,
+  (0x80 >> isc) << 24);
+}
+}
+
+static void sch_handle_clear_func(SubchDev *sch)
+{
+PMCW *p = &sch->curr_status.pmcw;
+SCSW *s = &sch->curr_status.scsw;
+int path;
+
+/* Path management: In our simple css, we always choose the only path. */
+path = 0x80;
+
+/* Reset values prior to 'issueing the clear signal'. */
+p->lpum = 0;
+p->pom = 0xff;
+s->flags &= ~SCSW_FLAGS_MASK_PNO;
+
+/* We always 'attempt to issue the clear signal', and we always succeed. */
+sch->orb = NULL;
+sch->channel_prog = 0x0;
+sch->last_cmd_valid = false;
+s->ctrl &= ~SCSW_ACTL_CLEAR_PEND;
+s->ctrl |= SCSW_STCTL_STATUS

[Qemu-devel] [PATCH 10/19] s390: Add new channel I/O based virtio transport.

2013-01-29 Thread Alexander Graf
From: Cornelia Huck 

Add a new virtio transport that uses channel commands to perform
virtio operations.

Signed-off-by: Cornelia Huck 
Signed-off-by: Alexander Graf 
---
 hw/s390x/Makefile.objs |1 +
 hw/s390x/virtio-ccw.c  |  960 
 hw/s390x/virtio-ccw.h  |   98 +
 trace-events   |4 +
 4 files changed, 1063 insertions(+), 0 deletions(-)
 create mode 100644 hw/s390x/virtio-ccw.c
 create mode 100644 hw/s390x/virtio-ccw.h

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index ab99da6..f6b461b 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -7,3 +7,4 @@ obj-y += event-facility.o
 obj-y += sclpquiesce.o sclpconsole.o
 obj-y += ipl.o
 obj-y += css.o
+obj-y += virtio-ccw.o
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
new file mode 100644
index 000..8c9b745
--- /dev/null
+++ b/hw/s390x/virtio-ccw.c
@@ -0,0 +1,960 @@
+/*
+ * virtio ccw target implementation
+ *
+ * Copyright 2012 IBM Corp.
+ * Author(s): Cornelia Huck 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include "hw/hw.h"
+#include "block/block.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/sysemu.h"
+#include "net/net.h"
+#include "monitor/monitor.h"
+#include "hw/virtio.h"
+#include "hw/virtio-serial.h"
+#include "hw/virtio-net.h"
+#include "hw/sysbus.h"
+#include "qemu/bitops.h"
+#include "hw/virtio-bus.h"
+
+#include "ioinst.h"
+#include "css.h"
+#include "virtio-ccw.h"
+#include "trace.h"
+
+static int virtual_css_bus_reset(BusState *qbus)
+{
+/* This should actually be modelled via the generic css */
+css_reset();
+
+/* we dont traverse ourself, return 0 */
+return 0;
+}
+
+
+static void virtual_css_bus_class_init(ObjectClass *klass, void *data)
+{
+BusClass *k = BUS_CLASS(klass);
+
+k->reset = virtual_css_bus_reset;
+}
+
+static const TypeInfo virtual_css_bus_info = {
+.name = TYPE_VIRTUAL_CSS_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(VirtualCssBus),
+.class_init = virtual_css_bus_class_init,
+};
+
+static const VirtIOBindings virtio_ccw_bindings;
+
+VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch)
+{
+VirtIODevice *vdev = NULL;
+
+if (sch->driver_data) {
+vdev = ((VirtioCcwDevice *)sch->driver_data)->vdev;
+}
+return vdev;
+}
+
+VirtualCssBus *virtual_css_bus_init(void)
+{
+VirtualCssBus *cbus;
+BusState *bus;
+DeviceState *dev;
+
+/* Create bridge device */
+dev = qdev_create(NULL, "virtual-css-bridge");
+qdev_init_nofail(dev);
+
+/* Create bus on bridge device */
+bus = qbus_create(TYPE_VIRTUAL_CSS_BUS, dev, "virtual-css");
+cbus = VIRTUAL_CSS_BUS(bus);
+
+/* Enable hotplugging */
+bus->allow_hotplug = 1;
+
+return cbus;
+}
+
+/* Communication blocks used by several channel commands. */
+typedef struct VqInfoBlock {
+uint64_t queue;
+uint32_t align;
+uint16_t index;
+uint16_t num;
+} QEMU_PACKED VqInfoBlock;
+
+typedef struct VqConfigBlock {
+uint16_t index;
+uint16_t num_max;
+} QEMU_PACKED VqConfigBlock;
+
+typedef struct VirtioFeatDesc {
+uint32_t features;
+uint8_t index;
+} QEMU_PACKED VirtioFeatDesc;
+
+/* Specify where the virtqueues for the subchannel are in guest memory. */
+static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align,
+  uint16_t index, uint16_t num)
+{
+VirtioCcwDevice *dev = sch->driver_data;
+
+if (index > VIRTIO_PCI_QUEUE_MAX) {
+return -EINVAL;
+}
+
+/* Current code in virtio.c relies on 4K alignment. */
+if (addr && (align != 4096)) {
+return -EINVAL;
+}
+
+if (!dev) {
+return -EINVAL;
+}
+
+virtio_queue_set_addr(dev->vdev, index, addr);
+if (!addr) {
+virtio_queue_set_vector(dev->vdev, index, 0);
+} else {
+/* Fail if we don't have a big enough queue. */
+/* TODO: Add interface to handle vring.num changing */
+if (virtio_queue_get_num(dev->vdev, index) > num) {
+return -EINVAL;
+}
+virtio_queue_set_vector(dev->vdev, index, index);
+}
+/* tell notify handler in case of config change */
+dev->vdev->config_vector = VIRTIO_PCI_QUEUE_MAX;
+return 0;
+}
+
+static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
+{
+int ret;
+VqInfoBlock info;
+uint8_t status;
+VirtioFeatDesc features;
+void *config;
+hwaddr indicators;
+VqConfigBlock vq_config;
+VirtioCcwDevice *dev = sch->driver_data;
+bool check_len;
+int len;
+hwaddr hw_len;
+
+if (!dev) {
+return -EINVAL;
+}
+
+trace_virtio_ccw_interpret_ccw(sch->cssid, sch->ssid, sch->schid,
+   ccw.cmd_code);
+check_len = !((ccw.flags & CCW_FLAG_SLI) && !(ccw.flags & CCW_FLAG_DC));
+
+/* Look at the 

Re: [Qemu-devel] [PATCH 06/15] s390: Add channel I/O instructions.

2013-01-29 Thread Alexander Graf

On 01/29/2013 09:12 PM, Blue Swirl wrote:

On Tue, Jan 29, 2013 at 3:45 PM, Cornelia Huck  wrote:

On Tue, 29 Jan 2013 16:09:38 +0100
Alexander Graf  wrote:


On 01/28/2013 10:59 AM, Cornelia Huck wrote:

On Fri, 25 Jan 2013 20:28:31 +0100
Alexander Graf   wrote:


However, I do agree that this duplicates logic. Cornelia, mind to instead call 
our map helper in css_do_tpi?

Well, ioinst_handle_tpi() looks like the better place to do this.

Can you put this into the series, or should I re-send?

It still breaks for 32-bit targets. Could you please replace the set_bit
call by normal bit shift operations?


Here you are:

 From f85a2507c4c5887e308dcd7dfcfebc386d802ea5 Mon Sep 17 00:00:00 2001
From: Cornelia Huck
Date: Tue, 29 Jan 2013 16:33:04 +0100
Subject: [PATCH] s390: Drop set_bit usage in virtio_ccw.

set_bit on indicators doesn't go well on 32 bit targets:

note: expected 'long unsigned int *' but argument is of type 'uint64_t *'

Switch to bit shifts instead.

Signed-off-by: Cornelia Huck
---
  hw/s390x/virtio-ccw.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 7d7f336..77e8f32 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -662,12 +662,12 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t 
vector)

  if (vector<  VIRTIO_PCI_QUEUE_MAX) {
  indicators = ldq_phys(dev->indicators);
-set_bit(vector,&indicators);
+indicators |= 1<<  vector;

Probably 1ULL should be used to avoid truncation on 32 bit hosts.


Thanks, applied to s390-next and changed to 1ULL.


Alex




[Qemu-devel] [PATCH v2] PPC: Unify dcbzl code path

2013-01-29 Thread Alexander Graf
The bit that makes a dcbz instruction a dcbzl instruction was declared as
reserved in ppc32 ISAs. However, hardware simply ignores the bit, making
code valid if it simply invokes dcbzl instead of dcbz even on 750 and G4.

Thus, mark the bit as unreserved so that we properly emulate a simple dcbz
in case we're running on non-G5s.

While at it, also refactor the code to check the 970 special case during
runtime. This way we don't need to differenciate between a 970 dcbz and
any other dcbz anymore. We also allow for future improvements to add e500mc
dcbz handling.

Reported-by: Amadeusz Sławiński 
Signed-off-by: Alexander Graf 
---
 target-ppc/cpu.h|6 ++
 target-ppc/helper.h |3 +--
 target-ppc/mem_helper.c |   21 -
 target-ppc/translate.c  |   33 -
 target-ppc/translate_init.c |   10 +-
 5 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 953146e..8c081db 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1857,10 +1857,8 @@ enum {
 PPC_CACHE  = 0x0002ULL,
 /*   icbi instruction*/
 PPC_CACHE_ICBI = 0x0004ULL,
-/*   dcbz instruction with fixed cache line size */
+/*   dcbz instruction*/
 PPC_CACHE_DCBZ = 0x0008ULL,
-/*   dcbz instruction with tunable cache line size   */
-PPC_CACHE_DCBZT= 0x0010ULL,
 /*   dcba instruction*/
 PPC_CACHE_DCBA = 0x0020ULL,
 /*   Freescale cache locking instructions*/
@@ -1928,7 +1926,7 @@ enum {
 | PPC_MEM_TLBIE | PPC_MEM_TLBSYNC \
 | PPC_MEM_SYNC | PPC_MEM_EIEIO \
 | PPC_CACHE | PPC_CACHE_ICBI \
-| PPC_CACHE_DCBZ | PPC_CACHE_DCBZT \
+| PPC_CACHE_DCBZ \
 | PPC_CACHE_DCBA | PPC_CACHE_LOCK \
 | PPC_EXTERN | PPC_SEGMENT | PPC_6xx_TLB \
 | PPC_74xx_TLB | PPC_40x_TLB | PPC_SEGMENT_64B \
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 83139d5..18e0394 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -25,8 +25,7 @@ DEF_HELPER_3(stmw, void, env, tl, i32)
 DEF_HELPER_4(lsw, void, env, tl, i32, i32)
 DEF_HELPER_5(lswx, void, env, tl, i32, i32, i32)
 DEF_HELPER_4(stsw, void, env, tl, i32, i32)
-DEF_HELPER_2(dcbz, void, env, tl)
-DEF_HELPER_2(dcbz_970, void, env, tl)
+DEF_HELPER_3(dcbz, void, env, tl, i32)
 DEF_HELPER_2(icbi, void, env, tl)
 DEF_HELPER_5(lscbx, tl, env, tl, i32, i32, i32)
 
diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index 902b1cd..ba383c8 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -136,18 +136,21 @@ static void do_dcbz(CPUPPCState *env, target_ulong addr, 
int dcache_line_size)
 }
 }
 
-void helper_dcbz(CPUPPCState *env, target_ulong addr)
+void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t is_dcbzl)
 {
-do_dcbz(env, addr, env->dcache_line_size);
-}
+int dcbz_size = env->dcache_line_size;
 
-void helper_dcbz_970(CPUPPCState *env, target_ulong addr)
-{
-if (((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) {
-do_dcbz(env, addr, 32);
-} else {
-do_dcbz(env, addr, env->dcache_line_size);
+#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64)
+if (!is_dcbzl &&
+(env->excp_model == POWERPC_EXCP_970) &&
+((env->spr[SPR_970_HID5] >> 7) & 0x3) == 1) {
+dcbz_size = 32;
 }
+#endif
+
+/* XXX add e500mc support */
+
+do_dcbz(env, addr, dcbz_size);
 }
 
 void helper_icbi(CPUPPCState *env, target_ulong addr)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 798b7ac..a4506fb 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4118,29 +4118,21 @@ static void gen_dcbtst(DisasContext *ctx)
 /* dcbz */
 static void gen_dcbz(DisasContext *ctx)
 {
-TCGv t0;
-gen_set_access_type(ctx, ACCESS_CACHE);
-/* NIP cannot be restored if the memory exception comes from an helper */
-gen_update_nip(ctx, ctx->nip - 4);
-t0 = tcg_temp_new();
-gen_addr_reg_index(ctx, t0);
-gen_helper_dcbz(cpu_env, t0);
-tcg_temp_free(t0);
-}
+TCGv tcgv_addr;
+TCGv_i32 tcgv_is_dcbzl;
+int is_dcbzl = ctx->opcode & 0x0020 ? 1 : 0;
 
-static void gen_dcbz_970(DisasContext *ctx)
-{
-TCGv t0;
 gen_set_access_type(ctx, ACCESS_CACHE);
 /* NIP cannot be restored if the memory exception comes from an helper */
 gen_update_nip(ctx, ctx->nip - 4);
-t0 = tcg_temp_new();
-gen_addr_reg_index(ctx, t0);
-if (ctx->opcode & 0x0020)
-gen_helper_dcbz(cpu_env, t0);
-else
- 

Re: [Qemu-devel] [PATCH 06/15] s390: Add channel I/O instructions.

2013-01-29 Thread Blue Swirl
On Tue, Jan 29, 2013 at 3:45 PM, Cornelia Huck  wrote:
> On Tue, 29 Jan 2013 16:09:38 +0100
> Alexander Graf  wrote:
>
>> On 01/28/2013 10:59 AM, Cornelia Huck wrote:
>> > On Fri, 25 Jan 2013 20:28:31 +0100
>> > Alexander Graf  wrote:
>> >
>> >> However, I do agree that this duplicates logic. Cornelia, mind to instead 
>> >> call our map helper in css_do_tpi?
>> > Well, ioinst_handle_tpi() looks like the better place to do this.
>> >
>> > Can you put this into the series, or should I re-send?
>>
>> It still breaks for 32-bit targets. Could you please replace the set_bit
>> call by normal bit shift operations?
>>
>
> Here you are:
>
> From f85a2507c4c5887e308dcd7dfcfebc386d802ea5 Mon Sep 17 00:00:00 2001
> From: Cornelia Huck 
> Date: Tue, 29 Jan 2013 16:33:04 +0100
> Subject: [PATCH] s390: Drop set_bit usage in virtio_ccw.
>
> set_bit on indicators doesn't go well on 32 bit targets:
>
> note: expected 'long unsigned int *' but argument is of type 'uint64_t *'
>
> Switch to bit shifts instead.
>
> Signed-off-by: Cornelia Huck 
> ---
>  hw/s390x/virtio-ccw.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 7d7f336..77e8f32 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -662,12 +662,12 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t 
> vector)
>
>  if (vector < VIRTIO_PCI_QUEUE_MAX) {
>  indicators = ldq_phys(dev->indicators);
> -set_bit(vector, &indicators);
> +indicators |= 1 << vector;

Probably 1ULL should be used to avoid truncation on 32 bit hosts.

>  stq_phys(dev->indicators, indicators);
>  } else {
>  vector = 0;
>  indicators = ldq_phys(dev->indicators2);
> -set_bit(vector, &indicators);
> +indicators |= 1 << vector;
>  stq_phys(dev->indicators2, indicators);
>  }
>
> --
> 1.7.6.2
>



Re: [Qemu-devel] [PATCH V2 11/20] tap: support enabling or disabling a queue

2013-01-29 Thread Blue Swirl
On Tue, Jan 29, 2013 at 1:50 PM, Jason Wang  wrote:
> On 01/26/2013 03:13 AM, Blue Swirl wrote:
>> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang  wrote:
>>> This patch introduce a new bit - enabled in TAPState which tracks whether a
>>> specific queue/fd is enabled. The tap/fd is enabled during initialization 
>>> and
>>> could be enabled/disabled by tap_enalbe() and tap_disable() which calls 
>>> platform
>>> specific helpers to do the real work. Polling of a tap fd can only done when
>>> the tap was enabled.
>>>
>>> Signed-off-by: Jason Wang 
>>> ---
>>>  include/net/tap.h |2 ++
>>>  net/tap-win32.c   |   10 ++
>>>  net/tap.c |   43 ---
>>>  3 files changed, 52 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/net/tap.h b/include/net/tap.h
>>> index bb7efb5..0caf8c4 100644
>>> --- a/include/net/tap.h
>>> +++ b/include/net/tap.h
>>> @@ -35,6 +35,8 @@ int tap_has_vnet_hdr_len(NetClientState *nc, int len);
>>>  void tap_using_vnet_hdr(NetClientState *nc, int using_vnet_hdr);
>>>  void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, int 
>>> ecn, int ufo);
>>>  void tap_set_vnet_hdr_len(NetClientState *nc, int len);
>>> +int tap_enable(NetClientState *nc);
>>> +int tap_disable(NetClientState *nc);
>>>
>>>  int tap_get_fd(NetClientState *nc);
>>>
>>> diff --git a/net/tap-win32.c b/net/tap-win32.c
>>> index 265369c..a2cd94b 100644
>>> --- a/net/tap-win32.c
>>> +++ b/net/tap-win32.c
>>> @@ -764,3 +764,13 @@ void tap_set_vnet_hdr_len(NetClientState *nc, int len)
>>>  {
>>>  assert(0);
>>>  }
>>> +
>>> +int tap_enable(NetClientState *nc)
>>> +{
>>> +assert(0);
>> abort()
>
> This is just to be consistent with the reset of the helpers in this file.
>>
>>> +}
>>> +
>>> +int tap_disable(NetClientState *nc)
>>> +{
>>> +assert(0);
>>> +}
>>> diff --git a/net/tap.c b/net/tap.c
>>> index 67080f1..95e557b 100644
>>> --- a/net/tap.c
>>> +++ b/net/tap.c
>>> @@ -59,6 +59,7 @@ typedef struct TAPState {
>>>  unsigned int write_poll : 1;
>>>  unsigned int using_vnet_hdr : 1;
>>>  unsigned int has_ufo: 1;
>>> +unsigned int enabled : 1;
>> bool without bit field?
>
> Also to be consistent with other field. If you wish I can send patches
> to convert all those bit field to bool on top of this series.

That would be nice, likewise for the assert(0).

>
> Thanks
>>>  VHostNetState *vhost_net;
>>>  unsigned host_vnet_hdr_len;
>>>  } TAPState;
>>> @@ -72,9 +73,9 @@ static void tap_writable(void *opaque);
>>>  static void tap_update_fd_handler(TAPState *s)
>>>  {
>>>  qemu_set_fd_handler2(s->fd,
>>> - s->read_poll  ? tap_can_send : NULL,
>>> - s->read_poll  ? tap_send : NULL,
>>> - s->write_poll ? tap_writable : NULL,
>>> + s->read_poll && s->enabled ? tap_can_send : NULL,
>>> + s->read_poll && s->enabled ? tap_send : NULL,
>>> + s->write_poll && s->enabled ? tap_writable : NULL,
>>>   s);
>>>  }
>>>
>>> @@ -339,6 +340,7 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
>>>  s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
>>>  s->using_vnet_hdr = 0;
>>>  s->has_ufo = tap_probe_has_ufo(s->fd);
>>> +s->enabled = 1;
>>>  tap_set_offload(&s->nc, 0, 0, 0, 0, 0);
>>>  /*
>>>   * Make sure host header length is set correctly in tap:
>>> @@ -737,3 +739,38 @@ VHostNetState *tap_get_vhost_net(NetClientState *nc)
>>>  assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP);
>>>  return s->vhost_net;
>>>  }
>>> +
>>> +int tap_enable(NetClientState *nc)
>>> +{
>>> +TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>> +int ret;
>>> +
>>> +if (s->enabled) {
>>> +return 0;
>>> +} else {
>>> +ret = tap_fd_enable(s->fd);
>>> +if (ret == 0) {
>>> +s->enabled = 1;
>>> +tap_update_fd_handler(s);
>>> +}
>>> +return ret;
>>> +}
>>> +}
>>> +
>>> +int tap_disable(NetClientState *nc)
>>> +{
>>> +TAPState *s = DO_UPCAST(TAPState, nc, nc);
>>> +int ret;
>>> +
>>> +if (s->enabled == 0) {
>>> +return 0;
>>> +} else {
>>> +ret = tap_fd_disable(s->fd);
>>> +if (ret == 0) {
>>> +qemu_purge_queued_packets(nc);
>>> +s->enabled = 0;
>>> +tap_update_fd_handler(s);
>>> +}
>>> +return ret;
>>> +}
>>> +}
>>> --
>>> 1.7.1
>>>
>>>
>



Re: [Qemu-devel] [RFC 10/19] target-alpha: Refactor debug output macros

2013-01-29 Thread Blue Swirl
On Tue, Jan 29, 2013 at 9:56 AM, Peter Maydell  wrote:
> On 29 January 2013 08:42, Andreas Färber  wrote:
>> Am 28.01.2013 22:10, schrieb Peter Maydell:
>> Where we disagree is that you suggest that do { ... while (0) is any
>> better than G_STMT_START ... G_STMT_END. I disagree and find *both*
>> obscuring the code. I clearly stated why.P
>>
>> Therefore I am interested in a non-obscured solution!
>
> There is none if you want to use a multistatement macro
> (which presumably you do because that's what your patch does).
> The best you can do is "standard thing that has been done for
> decades and that is easily recognisable".
>
>> Analysing the reasons for the obscured suggestion:
>>
>> a) "if (foo) MACRO1(); else MACRO2();" is forbidden by Coding Style.
>> Thus, if careful review indicates there are no such Coding Style
>> violations, it is entirely possible not to add any
>> fault-that-may-not-happen-obscuring macro statements.
>
> This requires analysis of all the callsites (including any that
> may be incoming in not-yet-applied patchsets) -- that's a lot
> harder and more error prone than just getting the macro right
> in the first place.
>
>> b) Working around an issue resulting from hiding C statements inside a
>> preprocessor macro is totally backwards compared to properly using the C
>> language in the first place. Its mechanism for reuse are functions, and
>> for performance considerations static inline functions.
>>
>> Therefore I disagree with you that b) is not an entirely different
>> disussion as you repeatedly suggest
>
> It clearly is, because *you didn't submit a patch using an inline
> function*. The whole reason we're having this argument is that you
> submitted a patch that uses a multistatement macro. Feel free
> to resubmit something using an inline function if you like. Once
> again:
>
>  C macro with do..while(0): OK
>  inline function: also OK
>  C macro missing do..while(0) protection: not OK
>  C macro using glib obfuscated macros: also not OK

+1

>
> This really doesn't seem particularly complicated to me.
>
> -- PMM
>



Re: [Qemu-devel] [PATCH v2] qmp-commands.hx: s/tray-open/tray_open/ to match qapi schema

2013-01-29 Thread Eric Blake
On 01/29/2013 09:58 AM, Michal Privoznik wrote:
> Currently, we are using 'tray_open' in QMP and 'tray-open' in
> HMP. However, the QMP documentation was mistakenly using the
> HMP version.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> diff to v1:
> -add S-o-b line
> 
>  qmp-commands.hx | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index f58a841..f90efe5 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1659,7 +1659,7 @@ Each json-object contain the following:
>   - Possible values: "unknown"
>  - "removable": true if the device is removable, false otherwise (json-bool)
>  - "locked": true if the device is locked, false otherwise (json-bool)
> -- "tray-open": only present if removable, true if the device has a tray,
> +- "tray_open": only present if removable, true if the device has a tray,
> and it is open (json-bool)
>  - "inserted": only present if the device is inserted, it is a json-object
> containing the following:
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 3/3] tests: Add unit tests for mulu64 and muls64

2013-01-29 Thread Blue Swirl
On Mon, Jan 28, 2013 at 10:09 PM, Richard Henderson  wrote:
> Signed-off-by: Richard Henderson 
> ---
>  tests/Makefile |  7 +-
>  tests/test-mul64.c | 69 
> ++
>  2 files changed, 75 insertions(+), 1 deletion(-)
>  create mode 100644 tests/test-mul64.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 442b286..e695ef6 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -45,6 +45,8 @@ gcov-files-test-aio-$(CONFIG_WIN32) = aio-win32.c
>  gcov-files-test-aio-$(CONFIG_POSIX) = aio-posix.c
>  check-unit-y += tests/test-thread-pool$(EXESUF)
>  gcov-files-test-thread-pool-y = thread-pool.c
> +check-unit-y += tests/test-mul64$(EXESUF)
> +gcov-files-test-mul64-y = util/host-utils.c
>
>  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
>
> @@ -72,7 +74,8 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o 
> tests/check-qdict.o \
> tests/test-coroutine.o tests/test-string-output-visitor.o \
> tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
> tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
> -   tests/test-qmp-commands.o tests/test-visitor-serialization.o
> +   tests/test-qmp-commands.o tests/test-visitor-serialization.o \
> +   tests/test-mul64.o
>
>  test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
>
> @@ -108,6 +111,8 @@ tests/test-qmp-input-strict$(EXESUF): 
> tests/test-qmp-input-strict.o $(test-qapi-
>  tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o 
> tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o 
> libqemuutil.a libqemustub.a
>  tests/test-visitor-serialization$(EXESUF): 
> tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a 
> libqemustub.a
>
> +tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
> +
>  tests/rtc-test$(EXESUF): tests/rtc-test.o
>  tests/m48t59-test$(EXESUF): tests/m48t59-test.o
>  tests/fdc-test$(EXESUF): tests/fdc-test.o
> diff --git a/tests/test-mul64.c b/tests/test-mul64.c
> new file mode 100644
> index 000..6abf57c
> --- /dev/null
> +++ b/tests/test-mul64.c
> @@ -0,0 +1,69 @@
> +/*
> + * Test 64x64 -> 128 multiply subroutines
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include 
> +#include 
> +#include "qemu/host-utils.h"
> +#include "qemu/osdep.h"
> +
> +
> +typedef struct {
> +uint64_t a, b;
> +uint64_t rh, rl;
> +} Test;
> +
> +static const Test test_u_data[] = {
> +{ 1, 1, 0, 1 },
> +{ 1, 1, 0, 1 },
> +{ -1ull, 2, 1, -2ull },

Shouldn't '1' be '-1'? How can this test pass?

It could be clearer to use 'ull' or 'ULL' (stands out better) always.

> +{ -1ull, -1ull, -2ull, 1 },

This looks buggy too.

> +{ 0x1122334455667788ull, 0x8877665544332211ull,
> +  0x092228FB777AE38Full, 0x0A3E963337C60008ull },
> +};
> +
> +static const Test test_s_data[] = {
> +{ 1, 1, 0, 1 },
> +{ 1, -1, -1, -1 },

Negative numbers should probably have ll/LL or ull/ULL.

> +{ -10, -10, 0, 100 },
> +{ 1, 1, 0, 1 },
> +{ -1, 2, -1, -2 },
> +{ 0x1122334455667788ll, 0x1122334455667788ull,

Spurious 'll', also below.

> +  0x01258F60BBC2975Cll, 0x1EACE4A3C82FB840ull },
> +};
> +
> +static void test_u(void)
> +{
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) {
> +uint64_t rl, rh;
> +mulu64(&rl, &rh, test_u_data[i].a, test_u_data[i].b);
> +g_assert_cmpuint(rl, ==, test_u_data[i].rl);

This could explain why the test passes: g_assert_cmpuint() uses
unsigned ints so there is truncation from uint64_t.

> +g_assert_cmpuint(rh, ==, test_u_data[i].rh);
> +}
> +}
> +
> +static void test_s(void)
> +{
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(test_u_data); ++i) {

test_s_data

> +uint64_t rl, rh;
> +muls64(&rl, &rh, test_s_data[i].a, test_s_data[i].b);
> +g_assert_cmpuint(rl, ==, test_s_data[i].rl);
> +g_assert_cmpint(rh, ==, test_s_data[i].rh);
> +}
> +}
> +
> +int main(int argc, char **argv)
> +{
> +g_test_init(&argc, &argv, NULL);
> +g_test_add_func("/host-utils/mulu64", test_u);
> +g_test_add_func("/host-utils/muls64", test_s);
> +return g_test_run();
> +}
> --
> 1.7.11.7
>



Re: [Qemu-devel] [PATCH 0/5] vhost-scsi: Add support for host virtualized target

2013-01-29 Thread Nicholas A. Bellinger
Hi MST, Paolo & Co,

On Mon, 2013-01-28 at 15:39 +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 28, 2013 at 02:33:44PM +0100, Paolo Bonzini wrote:
> > Il 28/01/2013 14:36, Michael S. Tsirkin ha scritto:
> > > On Mon, Jan 28, 2013 at 02:29:23PM +0100, Paolo Bonzini wrote:
> > >> Il 28/01/2013 14:11, Michael S. Tsirkin ha scritto:
> >  I asked for a standalone device because the configuration mechanism
> >  (configfs vs. command-line) and the feature set are completely
> >  different.  Unlike virtio-net, it's not possible to switch one to the
> >  other at run time.
> > >>>
> > >>> Exactly the same applies to any other frontend option.
> > >>> For example if you have two qemu instances with
> > >>> different num_queues values you can not migrate one
> > >>> to the other.
> > >>> So in this sense it is not different from any other
> > >>> frontend option, right?
> > >>
> > >> Indeed, in this sense it is not.
> > >>
> > >> Actually in this case migrating one to the other could succeed, and make
> > >> all disks disappear on the destination (because of the different
> > >> configuration mechanism).  That however could be overcome with vhost=on
> > >> registering a migration blocker.
> > > 
> > > Or better add a subsection if vhost is set: vhost=on to vhost=on
> > > can migrate, right?
> > 
> > I think it's not yet supported by the kernel.  You have no guarantee
> > that I/O is quiescent at the time the VM starts on the destination.
> > You'd need a ioctl to do the equivalent of bdrv_drain_all().
> > 
> > Once you have that, a subsection would do the job, yes.
> > 
> > Paolo
> 
> OK once that's in it would be easy to probe for.
> 
> > >> I won't really block the patch with the vhost=on/off frontend option if
> > >> it is properly done (e.g. the QEMU SCSI bus should not be created for
> > >> vhost=on) and minimally invasive to the non-vhost code.
> > >>

So what's the verdict here..?  Shall I respin the vhost-scsi patches
against qemu.git HEAD to make it in before Friday's cutoff for v1.4..?

Also, I'm not exactly sure what's involved with a vhost=on/off frontend
option discussed above, so if you or Paolo could handle this bit it
would be very helpful.

:)

--nab




Re: [Qemu-devel] [PATCH] PPC: Allow book3s ppc32 to run dcbzl

2013-01-29 Thread Andreas Färber
Am 29.01.2013 19:29, schrieb Alexander Graf:
> On 01/29/2013 07:02 PM, Scott Wood wrote:
>> On 01/29/2013 07:52:58 AM, Alexander Graf wrote:
>>> The bit that makes a dcbz instruction a dcbzl instruction was
>>> declared as
>>> reserved in ppc32 ISAs. However, hardware simply ignores the bit, making
>>> code valid if it simply invokes dcbzl instead of dcbz even on 750 and
>>> G4.
>>>
>>> Thus, mark the bit as unreserved so that we properly emulate a simple
>>> dcbz
>>> in case we're running on non-G5s.
>>
>> FWIW, dcbzl exists on e500mc, which is 32-bit.  If we ever support
>> L1CSR0[DCBZ32] on e500mc, we'll need proper dcbzl support rather than
>> just a dcbz alias.
> 
> Then the e500 machine wouldn't set PPC_CACHE_DCBZ, but a special
> PPC_CACHE_E500MC. Just like 970 sets PPC_CACHE_DCBZT instead today.
> 
> Unless of course we can somehow get to the cpu class from env. Then we
> could also ask for the machine type and implement dcbzl accordingly.

It's possible in two steps:

PowerPCCPU *cpu = ppc_env_get_cpu(env);
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);

That's why it's useful to have PowerPCCPU* (or S390CPU*) wherever
possible to spare that recurring additional step. :)

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] PPC: Allow book3s ppc32 to run dcbzl

2013-01-29 Thread Alexander Graf

On 01/29/2013 07:02 PM, Scott Wood wrote:

On 01/29/2013 07:52:58 AM, Alexander Graf wrote:
The bit that makes a dcbz instruction a dcbzl instruction was 
declared as

reserved in ppc32 ISAs. However, hardware simply ignores the bit, making
code valid if it simply invokes dcbzl instead of dcbz even on 750 and 
G4.


Thus, mark the bit as unreserved so that we properly emulate a simple 
dcbz

in case we're running on non-G5s.


FWIW, dcbzl exists on e500mc, which is 32-bit.  If we ever support 
L1CSR0[DCBZ32] on e500mc, we'll need proper dcbzl support rather than 
just a dcbz alias.


Then the e500 machine wouldn't set PPC_CACHE_DCBZ, but a special 
PPC_CACHE_E500MC. Just like 970 sets PPC_CACHE_DCBZT instead today.


Unless of course we can somehow get to the cpu class from env. Then we 
could also ask for the machine type and implement dcbzl accordingly.



Alex




Re: [Qemu-devel] [PATCH] PPC: Allow book3s ppc32 to run dcbzl

2013-01-29 Thread Scott Wood

On 01/29/2013 07:52:58 AM, Alexander Graf wrote:
The bit that makes a dcbz instruction a dcbzl instruction was  
declared as
reserved in ppc32 ISAs. However, hardware simply ignores the bit,  
making
code valid if it simply invokes dcbzl instead of dcbz even on 750 and  
G4.


Thus, mark the bit as unreserved so that we properly emulate a simple  
dcbz

in case we're running on non-G5s.


FWIW, dcbzl exists on e500mc, which is 32-bit.  If we ever support  
L1CSR0[DCBZ32] on e500mc, we'll need proper dcbzl support rather than  
just a dcbz alias.


-Scott



Re: [Qemu-devel] a probem about qemu doesn't support for setting master volume/mute when guest is winxp

2013-01-29 Thread Paolo Bonzini
Il 29/01/2013 08:35, liequan@i-soft.com.cn ha scritto:
>  Hi all:
>  I found a bug that qemu doesn't support for setting master volume/mute
> when guest is winxp. but the wave volume in the volume control works
> fine.I don't know why it does.
>   Any help is welcome.
>   Best Regards!

You probably need to configure with --enable-mixemu.

Paolo



[Qemu-devel] [PATCH v2] qmp-commands.hx: s/tray-open/tray_open/ to match qapi schema

2013-01-29 Thread Michal Privoznik
Currently, we are using 'tray_open' in QMP and 'tray-open' in
HMP. However, the QMP documentation was mistakenly using the
HMP version.

Signed-off-by: Michal Privoznik 
---

diff to v1:
-add S-o-b line

 qmp-commands.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index f58a841..f90efe5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1659,7 +1659,7 @@ Each json-object contain the following:
  - Possible values: "unknown"
 - "removable": true if the device is removable, false otherwise (json-bool)
 - "locked": true if the device is locked, false otherwise (json-bool)
-- "tray-open": only present if removable, true if the device has a tray,
+- "tray_open": only present if removable, true if the device has a tray,
and it is open (json-bool)
 - "inserted": only present if the device is inserted, it is a json-object
containing the following:
-- 
1.8.0.2




[Qemu-devel] [Bug 1062220] Re: qemu-system-arm crashed with SIGABRT in cpu_abort()

2013-01-29 Thread Peter Maydell
qemu-system-arm -M versatilepb -kernel u-boot.bin
[...]
Trying to execute code outside RAM or ROM

This almost always means that you tried to execute a guest binary which
wasn't for the VersatilePB. Without more info about exactly what this
u-boot.bin file was this bug report can't progress any further.

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

Title:
  qemu-system-arm crashed with SIGABRT in cpu_abort()

Status in QEMU:
  New
Status in “qemu” package in Ubuntu:
  Confirmed
Status in “qemu-linaro” package in Ubuntu:
  Confirmed

Bug description:
  -kernel u-boot.bin

  ProblemType: Crash
  DistroRelease: Ubuntu 12.10
  Package: qemu-system 1.2.0-2012.09-0ubuntu1
  ProcVersionSignature: Ubuntu 3.5.0-10.10-generic 3.5.1
  Uname: Linux 3.5.0-10-generic x86_64
  NonfreeKernelModules: nvidia
  ApportVersion: 2.6.1-0ubuntu1
  Architecture: amd64
  CrashCounter: 1
  Date: Fri Oct  5 19:30:23 2012
  ExecutablePath: /usr/bin/qemu-system-arm
  InstallationMedia: Ubuntu 11.10 "Oneiric Ocelot" - Alpha amd64 (20110804)
  ProcCmdline: qemu-system-arm -M versatilepb -kernel u-boot.bin
  Signal: 6
  SourcePackage: qemu-linaro
  StacktraceTop:
   raise () from /lib/x86_64-linux-gnu/libc.so.6
   abort () from /lib/x86_64-linux-gnu/libc.so.6
   ?? ()
   ?? ()
   ?? ()
  Title: qemu-system-arm crashed with SIGABRT in raise()
  UpgradeStatus: Upgraded to quantal on 2012-08-11 (54 days ago)
  UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare vboxusers

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1062220/+subscriptions



Re: [Qemu-devel] [RFC] qemu snapshot enchancement

2013-01-29 Thread Stefan Hajnoczi
On Tue, Jan 29, 2013 at 2:32 PM, Paolo Bonzini  wrote:
> Il 29/01/2013 14:27, Stefan Hajnoczi ha scritto:
>>> > and in step 5, may need export the delta data, not the whole disk
>>> > data.
>> NBD doesn't have a way to perform bdrv_is_allocated().  Either we need
>> to enhance the protocol or we need to add a QMP command to read
>> the allocation bitmap for an image.  I'm a little hesitant about sending
>> the bitmap or allocation extent information over QMP (JSON) but it might
>> be doable.
>
> I'm planning to add offline mirroring to qemu-img.  If you use an NBD
> server as the destination, it can be used to send only the delta between
> two snapshots via NBD.
>
> I think this is the opposite of what you suggested, which is to run
> qemu-nbd on the image and query the server.

Yep, exactly.  The nice thing about your approach is that we don't
need an explicit API for dirty block tracking, it's implicit in the
writes that we send to the backup application's NBD server.

Stefan



Re: [Qemu-devel] [RFC] qemu snapshot enchancement

2013-01-29 Thread Paolo Bonzini
Il 29/01/2013 17:44, Stefan Hajnoczi ha scritto:
>> >
>> > I'm planning to add offline mirroring to qemu-img.  If you use an NBD
>> > server as the destination, it can be used to send only the delta between
>> > two snapshots via NBD.
>> >
>> > I think this is the opposite of what you suggested, which is to run
>> > qemu-nbd on the image and query the server.
> Yep, exactly.  The nice thing about your approach is that we don't
> need an explicit API for dirty block tracking, it's implicit in the
> writes that we send to the backup application's NBD server.

Of course the ugly thing is that it is much less flexible than letting
the backup application do its own thing.  Also, there is just one dirty
bitmap that you need to use for everything, though in the future the
"block filters" patch will come up magically and will let you have
multiple dirty bitmaps...

Paolo



Re: [Qemu-devel] KVM call minutes 2013-01-29

2013-01-29 Thread Paolo Bonzini
Il 29/01/2013 17:47, Anthony Liguori ha scritto:
> Paolo Bonzini  writes:
> 
>> Il 29/01/2013 16:41, Juan Quintela ha scritto:
>>> * Replacing select(2) so that we will not hit the 1024 fd_set limit in the
>>>   future. (stefan)
>>>
>>>   Add checks for fd's bigger than 1024? multifunction devices uses lot
>>>   of fd's for device.
>>>
>>>   Portability?
>>>   Use glib?  and let it use poll underneath.
>>>   slirp is a problem.
>>>   in the end loop: moving to a glib event loop, how we arrive there is the 
>>> discussion.
>>
>> We can use g_poll while keeping the main-loop.c wrappers around the glib
>> event loop.  Both slirp and iohandler.c access the fd_sets randomly, so
>> we need to remember some state between the fill and poll functions.  We
>> can use two main-loop.c functions:
>>
>> int qemu_add_poll_fd(int fd, int events);
>>
>>   select: writes the events into three fd_sets, returns the file
>>   descriptor itself
>>
>>   poll: writes a GPollFD into a dynamically-sized array (of GPollFDs)
>>   and returns the index in the array.
>>
>> int qemu_get_poll_fd_revents(int index);
>>
>>   select: takes the file descriptor (returned by qemu_add_poll_fd),
>>   makes up revents based on the three fd_sets
>>
>>   poll: takes the index into the array and returns the corresponding
>>   revents
>>
>> iohandler.c can simply store the index into struct IOHandlerRecord, and
>> use it later.  slirp can do the same for struct socket.
>>
>> The select code can be kept for Windows after POSIX switches to poll.
> 
> Doesn't g_poll already do this under the covers for Windows?

No, g_poll is for synchronization objects (like Linux eventfd or
timerfd).  Sockets still require select.  You can tie a socket to a
synchronization object; this way socket events can exit g_poll, and in
fact that's exactly what QEMU does.  But you still need to retrieve the
currently-active events with select, so iohandler.c and slirp (which use
sockets) need to work in terms of select.

Paolo



Re: [Qemu-devel] [PATCH v4 4/5] sheepdog: use inet_connect to simplify connect code

2013-01-29 Thread Eric Blake
On 01/29/2013 01:56 AM, MORITA Kazutaka wrote:
> This uses the form ":" for the representation of the
> sheepdog server to use inet_connect.
> 
> Signed-off-by: MORITA Kazutaka 
> ---
>  block/sheepdog.c |  111 ++---
>  1 files changed, 30 insertions(+), 81 deletions(-)
> 

>  /* sheepdog[+tcp]://[host:port]/vdiname */
> -s->addr = g_strdup(uri->server ?: SD_DEFAULT_ADDR);
> -if (uri->port) {
> -s->port = g_strdup_printf("%d", uri->port);
> -} else {
> -s->port = g_strdup(SD_DEFAULT_PORT);
> -}
> +s->host_spec = g_strdup_printf("%s:%d", uri->server ?: SD_DEFAULT_ADDR,
> +   uri->port ?: SD_DEFAULT_PORT);

Is this going to properly handle IPv6 addresses, which need to use
brackets around the host portion, as in:
 sheepdog+tcp://[::1]:port/vdiname

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.

2013-01-29 Thread Benoît Canet
Le Tuesday 29 Jan 2013 à 16:21:43 (+0100), Kevin Wolf a écrit :
> Am 29.01.2013 16:03, schrieb Benoît Canet:
> >> base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\
> >> base2 [file] --- base2 [qcow2] --- snap2.qcow2 --->-- quorum
> >> base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/
> > 
> > I tried to default snapshot creation to qcow2 in quorum.c and specify
> > quorum as format in the snapshot_blkdev command line.
> > It doesn't work (crash).
> > Do you have any idea on how to open while obtaining the third case ?
> 
> Wait for BlockBackends and then implement proper stay-on-top filters.

Any idea to disable properly snapshot creation for quorum and allowing it
to be merged in a not so far future ?

Benoît

> 
> Kevin
> 



Re: [Qemu-devel] KVM call minutes 2013-01-29

2013-01-29 Thread Anthony Liguori
Paolo Bonzini  writes:

> Il 29/01/2013 16:41, Juan Quintela ha scritto:
>> * Replacing select(2) so that we will not hit the 1024 fd_set limit in the
>>   future. (stefan)
>> 
>>   Add checks for fd's bigger than 1024? multifunction devices uses lot
>>   of fd's for device.
>> 
>>   Portability?
>>   Use glib?  and let it use poll underneath.
>>   slirp is a problem.
>>   in the end loop: moving to a glib event loop, how we arrive there is the 
>> discussion.
>
> We can use g_poll while keeping the main-loop.c wrappers around the glib
> event loop.  Both slirp and iohandler.c access the fd_sets randomly, so
> we need to remember some state between the fill and poll functions.  We
> can use two main-loop.c functions:
>
> int qemu_add_poll_fd(int fd, int events);
>
>   select: writes the events into three fd_sets, returns the file
>   descriptor itself
>
>   poll: writes a GPollFD into a dynamically-sized array (of GPollFDs)
>   and returns the index in the array.
>
> int qemu_get_poll_fd_revents(int index);
>
>   select: takes the file descriptor (returned by qemu_add_poll_fd),
>   makes up revents based on the three fd_sets
>
>   poll: takes the index into the array and returns the corresponding
>   revents
>
> iohandler.c can simply store the index into struct IOHandlerRecord, and
> use it later.  slirp can do the same for struct socket.
>
> The select code can be kept for Windows after POSIX switches to poll.

Doesn't g_poll already do this under the covers for Windows?

Regards,

Anthony Liguori

>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] vnc: Clean up vncws_send_handshake_response()

2013-01-29 Thread Stefan Hajnoczi
On Fri, Jan 25, 2013 at 10:31:16AM +0100, Markus Armbruster wrote:
> Use appropriate types, drop superfluous casts, use sizeof, don't
> exploit that this particular call of gnutls_fingerprint() doesn't
> change its last argument.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  ui/vnc-ws.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan



Re: [Qemu-devel] [PATCH V2 3/4] blockdev: Allow snapshoting of protocols.

2013-01-29 Thread Eric Blake
On 01/29/2013 07:27 AM, Benoît Canet wrote:
>> base1 [file] --- base1 [qcow2] --- snap1.qcow2 --\
>> base2 [file] --- base2 [qcow2] --- snap2.qcow2 --->-- quorum
>> base3 [file] --- base3 [qcow2] --- snap3.qcow2 --/

For that matter, I think it would be useful to have a quorum where the
data is duplicated between different formats, such as:

file1 [raw] --\
base2 [raw] --- snap2.qcow2 [qcow2] -->-- quorum
base3 [qed] --- snap3.qed [qed] --/

In that case, you NEED to be able to pass in the protocol of each
underlying device _as part of opening the quorum_, something like:

quorum:2/3:file1[raw]:snap2.qcow2[qcow2]:snap3.qed[qed]

or some such punctuation.  And of course, that means you'd have to
extend your escaping mechanism to allow escaping of whatever syntax you
use for declaring a per-member format.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC V8 11/13] quorum: Add quorum_snapshot_img_create.

2013-01-29 Thread Eric Blake
On 01/28/2013 10:07 AM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet 

Now that you delayed open() to later in the series, it would be useful
to have this commit document the format of what a valid input string
will be (that is, some of your commit comments on 13/13 are more useful
here, where the parser is actually implemented).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] qemu-iotests: Test qcow2 image creation options

2013-01-29 Thread Kevin Wolf
Am 29.01.2013 17:12, schrieb Eric Blake:
> On 01/29/2013 03:01 AM, Kevin Wolf wrote:
>> Just create lots of images and try out each of the creation options that
>> qcow2 provides (except backing_file/fmt for now)
>>
>> I'm not totally happy with the behaviour of qemu-img in each of the
>> cases, but let's be explicit and update the test when we do change
>> things later.
>>
>> Signed-off-by: Kevin Wolf 
>> ---
> 
>> @@ -0,0 +1,117 @@
>> +#!/bin/bash
> 
> Good, because you definitely used some bash-isms (such as sizes+="more").
> 
>> +# creator
>> +owner=kw...@redhat.com
>> +
>> +seq=`basename $0`
> 
> Since you are already using a capable shell, why not go all the way and
> use $() instead of ``?  

I would have if this wasn't only code copied from other test cases.
(Same thing below: the POSIX function is copied, the bash one is new)

> And in this case, why not:
> 
> seq=${0##*/}
> 
> to avoid a fork?

Because I can't read that and one fork more or less really makes no
difference. :-) (and it's copied as well)

>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
> 
> Likewise, since you are using a capable shell, you can avoid a fork:
> 
> here=$PWD
> 
>> +tmp=/tmp/$$
> 
> And since you are using a capable shell, it would be more secure to use:
> 
> tmp=/tmp/$$.$RANDOM

Both of them are copied.

If you really care about any of them, you should probably submit a patch
that fixes it in all test cases - otherwise I'll again copy one of the
wrong versions next time.

Kevin



Re: [Qemu-devel] [PATCH] qemu-iotests: Test qcow2 image creation options

2013-01-29 Thread Eric Blake
On 01/29/2013 03:01 AM, Kevin Wolf wrote:
> Just create lots of images and try out each of the creation options that
> qcow2 provides (except backing_file/fmt for now)
> 
> I'm not totally happy with the behaviour of qemu-img in each of the
> cases, but let's be explicit and update the test when we do change
> things later.
> 
> Signed-off-by: Kevin Wolf 
> ---

> @@ -0,0 +1,117 @@
> +#!/bin/bash

Good, because you definitely used some bash-isms (such as sizes+="more").

> +# creator
> +owner=kw...@redhat.com
> +
> +seq=`basename $0`

Since you are already using a capable shell, why not go all the way and
use $() instead of ``?  And in this case, why not:

seq=${0##*/}

to avoid a fork?

> +echo "QA output created by $seq"
> +
> +here=`pwd`

Likewise, since you are using a capable shell, you can avoid a fork:

here=$PWD

> +tmp=/tmp/$$

And since you are using a capable shell, it would be more secure to use:

tmp=/tmp/$$.$RANDOM

> +status=1 # failure is the default!
> +
> +_cleanup()
> +{

POSIX-style declaration,...

> + _cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt qcow2
> +_supported_proto file
> +_supported_os Linux
> +
> +function test_qemu_img()

...bash-style declaration.  I generally frown on the use of 'function'
in shell scripts, as it is not portable, and takes up more screen-space
than the POSIX style.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [Bug 1062220] Re: qemu-system-arm crashed with SIGABRT in cpu_abort()

2013-01-29 Thread Serge Hallyn
Marked as affecting Ubuntu's qemu package, as bug 1103405 (a dup of this
one) happened with the new qemu, not qemu-linaro, package.

** Also affects: qemu (Ubuntu)
   Importance: Undecided
   Status: New

** Changed in: qemu (Ubuntu)
   Status: New => Confirmed

** Changed in: qemu (Ubuntu)
   Importance: Undecided => Medium

** Also affects: qemu
   Importance: Undecided
   Status: New

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

Title:
  qemu-system-arm crashed with SIGABRT in cpu_abort()

Status in QEMU:
  New
Status in “qemu” package in Ubuntu:
  Confirmed
Status in “qemu-linaro” package in Ubuntu:
  Confirmed

Bug description:
  -kernel u-boot.bin

  ProblemType: Crash
  DistroRelease: Ubuntu 12.10
  Package: qemu-system 1.2.0-2012.09-0ubuntu1
  ProcVersionSignature: Ubuntu 3.5.0-10.10-generic 3.5.1
  Uname: Linux 3.5.0-10-generic x86_64
  NonfreeKernelModules: nvidia
  ApportVersion: 2.6.1-0ubuntu1
  Architecture: amd64
  CrashCounter: 1
  Date: Fri Oct  5 19:30:23 2012
  ExecutablePath: /usr/bin/qemu-system-arm
  InstallationMedia: Ubuntu 11.10 "Oneiric Ocelot" - Alpha amd64 (20110804)
  ProcCmdline: qemu-system-arm -M versatilepb -kernel u-boot.bin
  Signal: 6
  SourcePackage: qemu-linaro
  StacktraceTop:
   raise () from /lib/x86_64-linux-gnu/libc.so.6
   abort () from /lib/x86_64-linux-gnu/libc.so.6
   ?? ()
   ?? ()
   ?? ()
  Title: qemu-system-arm crashed with SIGABRT in raise()
  UpgradeStatus: Upgraded to quantal on 2012-08-11 (54 days ago)
  UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare vboxusers

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1062220/+subscriptions



Re: [Qemu-devel] QEMU buildbot maintenance state

2013-01-29 Thread Christian Berendt

On 01/28/2013 03:29 PM, Daniel Gollub wrote:

JFYI, the main buildbot configuration which controls everything (beside
buildslave credentials) is accessible to everyone:
http://people.b1-systems.de/~gollub/buildbot/

If you are familiar with buildbot feel free to incorporate your suggested
changes directly on a copy and send me or Christian the diff so we just have
to review and apply it.


I moved the configuration on GitHub 
(https://github.com/b1-systems/buildbot). I'll add a cron job to the 
buildbot system to regular pull and apply the latest configuration. 
Simply open a pull request to modify the configuration.


I'll implement open requests for configuration changes as soon as 
possible (should be done tomorrow).


Christian.

--
Christian Berendt
Solution Architect
Mail: bere...@b1-systems.de

B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537



Re: [Qemu-devel] KVM call minutes 2013-01-29

2013-01-29 Thread Paolo Bonzini
Il 29/01/2013 16:41, Juan Quintela ha scritto:
> * Replacing select(2) so that we will not hit the 1024 fd_set limit in the
>   future. (stefan)
> 
>   Add checks for fd's bigger than 1024? multifunction devices uses lot
>   of fd's for device.
> 
>   Portability?
>   Use glib?  and let it use poll underneath.
>   slirp is a problem.
>   in the end loop: moving to a glib event loop, how we arrive there is the 
> discussion.

We can use g_poll while keeping the main-loop.c wrappers around the glib
event loop.  Both slirp and iohandler.c access the fd_sets randomly, so
we need to remember some state between the fill and poll functions.  We
can use two main-loop.c functions:

int qemu_add_poll_fd(int fd, int events);

  select: writes the events into three fd_sets, returns the file
  descriptor itself

  poll: writes a GPollFD into a dynamically-sized array (of GPollFDs)
  and returns the index in the array.

int qemu_get_poll_fd_revents(int index);

  select: takes the file descriptor (returned by qemu_add_poll_fd),
  makes up revents based on the three fd_sets

  poll: takes the index into the array and returns the corresponding
  revents

iohandler.c can simply store the index into struct IOHandlerRecord, and
use it later.  slirp can do the same for struct socket.

The select code can be kept for Windows after POSIX switches to poll.

Paolo



Re: [Qemu-devel] [PATCH] Add a disk format named iROW, supporting high-efficiency VM snapshot

2013-01-29 Thread Anthony Liguori
Hi,

Thank you for submitting your patch series.  checkpatch.pl has
detected that one or more of the patches in this series violate
the QEMU coding style.

If you believe this message was sent in error, please ignore it
or respond here with an explanation.

Otherwise, please correct the coding style issues and resubmit a
new version of the patch.

For more information about QEMU coding style, see:

http://git.qemu.org/?p=qemu.git;a=blob_plain;f=CODING_STYLE;hb=HEAD

Here is the output from checkpatch.pl:

Subject: Add a disk format named iROW, supporting high-efficiency VM snapshot
WARNING: line over 80 characters
#34: FILE: block/irow.c:5:
+ * iRow (imporved Redirect-on-Write) is a disk format supporting 
high-efficiency VM disk snapshot.

WARNING: line over 80 characters
#35: FILE: block/irow.c:6:
+ * iROW uses bitmap to reduce the amount of metadata, so that both the VM disk 
snapshot key operations

WARNING: line over 80 characters
#36: FILE: block/irow.c:7:
+ * performance and the VM disk I/O performance would be enhanced at the same 
time.

WARNING: line over 80 characters
#40: FILE: block/irow.c:11:
+ *A snapshot consists of 2 files: a bitmap file (btmp file) and a VM disk data 
file (irvd file).

WARNING: line over 80 characters
#43: FILE: block/irow.c:14:
+ *The meta file consists of the meta header and the snapshots information. The 
meta header is used to

WARNING: line over 80 characters
#44: FILE: block/irow.c:15:
+ *store basic information of VM disk image. The snapshots information 
sequentially stores every snapshot???s name,

WARNING: line over 80 characters
#47: FILE: block/irow.c:18:
+ *The btmp file consists of a bitmap and the VM state data. The bitmap is used 
to indicate whether the

WARNING: line over 80 characters
#48: FILE: block/irow.c:19:
+ *clusters exist in corresponding irvd file. Each cluster in the VM disk image 
is mapped to a bit in the bitmap.

WARNING: line over 80 characters
#50: FILE: block/irow.c:21:
+ *The irvd file is used to store the actual data of the VM disk image. The 
smallest unit of storage is cluster.

WARNING: line over 80 characters
#51: FILE: block/irow.c:22:
+ *iROW does not decide the address of the data clusters. It just writes the 
clusters to the same VM disk image

WARNING: line over 80 characters
#52: FILE: block/irow.c:23:
+ *addresses as the virtual addresses of the clusters. Because of host 
machine???s file system support sparse files,

WARNING: line over 80 characters
#53: FILE: block/irow.c:24:
+ *iROW also achieves the gradual growth of the VM disk image size with the 
actual disk usage.

ERROR: do not initialise globals to 0 or NULL
#64: FILE: block/irow.c:35:
+BDRVIrowState **birows_cache = NULL;

ERROR: do not initialise globals to 0 or NULL
#65: FILE: block/irow.c:36:
+ClusterCache *cluster_cache = NULL;

ERROR: code indent should never use tabs
#74: FILE: block/irow.c:45:
+^Iif (size & 1) {$

ERROR: code indent should never use tabs
#75: FILE: block/irow.c:46:
+^I^Ireturn -1;$

ERROR: code indent should never use tabs
#85: FILE: block/irow.c:56:
+^Iconst IRowMeta *irow_meta = (const void *)buf;$

ERROR: space required before the open brace '{'
#89: FILE: block/irow.c:60:
+be32_to_cpu(irow_meta->version) == IROW_VERSION){

ERROR: else should follow close brace '}'
#92: FILE: block/irow.c:63:
+}
+else {

ERROR: open brace '{' following function declarations go on the next line
#97: FILE: block/irow.c:68:
+static void irow_close_btmp(BDRVIrowState *s) {

ERROR: code indent should never use tabs
#98: FILE: block/irow.c:69:
+^Iif(s->bitmap) {$

ERROR: space required before the open parenthesis '('
#98: FILE: block/irow.c:69:
+   if(s->bitmap) {

ERROR: code indent should never use tabs
#99: FILE: block/irow.c:70:
+^I^Ig_free(s->bitmap);$

ERROR: code indent should never use tabs
#100: FILE: block/irow.c:71:
+^I^Is->bitmap = NULL;$

ERROR: code indent should never use tabs
#101: FILE: block/irow.c:72:
+^I}$

ERROR: code indent should never use tabs
#103: FILE: block/irow.c:74:
+^Iif(s->irow_btmp) {$

ERROR: space required before the open parenthesis '('
#103: FILE: block/irow.c:74:
+   if(s->irow_btmp) {

ERROR: code indent should never use tabs
#104: FILE: block/irow.c:75:
+^I^Ibdrv_delete(s->irow_btmp);$

ERROR: code indent should never use tabs
#105: FILE: block/irow.c:76:
+^I^Is->irow_btmp = NULL;$

ERROR: code indent should never use tabs
#106: FILE: block/irow.c:77:
+^I}$

ERROR: open brace '{' following function declarations go on the next line
#109: FILE: block/irow.c:80:
+static void irow_close_irvd(BDRVIrowState *s) {

ERROR: code indent should never use tabs
#110: FILE: block/irow.c:81:
+^Iif(s->irow_irvd) {$

ERROR: space required before the open parenthesis '('
#110: FILE: block/irow.c:81:
+   if(s->irow_irvd) {

ERROR: code indent should never use tabs
#111: FILE: block/irow.c:82:
+^I^Ibdrv_delete(s->irow_irvd);$

ERROR: code indent should never use tabs
#112: FILE: block/irow.c:83:
+^I^Is->

Re: [Qemu-devel] [PATCH v2 0/6] bdrv_open() error return fixes

2013-01-29 Thread Anthony Liguori
Hi,

Thank you for submitting your patch series.  checkpatch.pl has
detected that one or more of the patches in this series violate
the QEMU coding style.

If you believe this message was sent in error, please ignore it
or respond here with an explanation.

Otherwise, please correct the coding style issues and resubmit a
new version of the patch.

For more information about QEMU coding style, see:

http://git.qemu.org/?p=qemu.git;a=blob_plain;f=CODING_STYLE;hb=HEAD

Here is the output from checkpatch.pl:

Subject: parallels: Fix bdrv_open() error handling
Subject: dmg: Use g_free instead of free
Subject: dmg: Fix bdrv_open() error handling
ERROR: space required after that ',' (ctx:VxV)
#68: FILE: block/dmg.c:91:
+uint64_t info_begin,info_end,last_in_offset,last_out_offset;
^

ERROR: space required after that ',' (ctx:VxV)
#68: FILE: block/dmg.c:91:
+uint64_t info_begin,info_end,last_in_offset,last_out_offset;
 ^

ERROR: space required after that ',' (ctx:VxV)
#68: FILE: block/dmg.c:91:
+uint64_t info_begin,info_end,last_in_offset,last_out_offset;
^

ERROR: space required before the open parenthesis '('
#203: FILE: block/dmg.c:225:
+if(inflateInit(&s->zstream) != Z_OK) {

total: 4 errors, 0 warnings, 199 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Subject: vpc: Fix bdrv_open() error handling
Subject: cloop: Fix bdrv_open() error handling
Subject: bochs: Fix bdrv_open() error handling


Regards,

Anthony Liguori




Re: [Qemu-devel] [PATCH] qmp-commands.hx: s/tray-open/tray_open/ to match qapi schema

2013-01-29 Thread Anthony Liguori
Hi,

Thank you for submitting your patch series.  checkpatch.pl has
detected that one or more of the patches in this series violate
the QEMU coding style.

If you believe this message was sent in error, please ignore it
or respond here with an explanation.

Otherwise, please correct the coding style issues and resubmit a
new version of the patch.

For more information about QEMU coding style, see:

http://git.qemu.org/?p=qemu.git;a=blob_plain;f=CODING_STYLE;hb=HEAD

Here is the output from checkpatch.pl:

Subject: qmp-commands.hx: s/tray-open/tray_open/ to match qapi schema
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 8 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.



Regards,

Anthony Liguori




Re: [Qemu-devel] [PATCH V16 0/9] libqblock qemu block layer library

2013-01-29 Thread Anthony Liguori
Hi,

Thank you for submitting your patch series.  This patch breaks the build
by causing make to fail.  Here's the output:

  GEN   i386-softmmu/config-devices.mak
  GEN   x86_64-softmmu/config-devices.mak
  GEN   alpha-softmmu/config-devices.mak
  GEN   arm-softmmu/config-devices.mak
  GEN   cris-softmmu/config-devices.mak
  GEN   lm32-softmmu/config-devices.mak
  GEN   m68k-softmmu/config-devices.mak
  GEN   microblaze-softmmu/config-devices.mak
  GEN   microblazeel-softmmu/config-devices.mak
  GEN   mips-softmmu/config-devices.mak
  GEN   mipsel-softmmu/config-devices.mak
  GEN   mips64-softmmu/config-devices.mak
  GEN   mips64el-softmmu/config-devices.mak
  GEN   or32-softmmu/config-devices.mak
  GEN   ppc-softmmu/config-devices.mak
  GEN   ppcemb-softmmu/config-devices.mak
  GEN   ppc64-softmmu/config-devices.mak
  GEN   sh4eb-softmmu/config-devices.mak
  GEN   sh4-softmmu/config-devices.mak
  GEN   sparc-softmmu/config-devices.mak
  GEN   s390x-softmmu/config-devices.mak
  GEN   sparc64-softmmu/config-devices.mak
  GEN   xtensa-softmmu/config-devices.mak
  GEN   xtensaeb-softmmu/config-devices.mak
  GEN   unicore32-softmmu/config-devices.mak
  GEN   x86_64-linux-user/config-devices.mak
  GEN   alpha-linux-user/config-devices.mak
  GEN   i386-linux-user/config-devices.mak
  GEN   arm-linux-user/config-devices.mak
  GEN   armeb-linux-user/config-devices.mak
  GEN   cris-linux-user/config-devices.mak
  GEN   m68k-linux-user/config-devices.mak
  GEN   microblaze-linux-user/config-devices.mak
  GEN   microblazeel-linux-user/config-devices.mak
  GEN   mips-linux-user/config-devices.mak
  GEN   mipsel-linux-user/config-devices.mak
  GEN   or32-linux-user/config-devices.mak
  GEN   ppc-linux-user/config-devices.mak
  GEN   ppc64-linux-user/config-devices.mak
  GEN   ppc64abi32-linux-user/config-devices.mak
  GEN   sh4-linux-user/config-devices.mak
  GEN   sh4eb-linux-user/config-devices.mak
  GEN   sparc-linux-user/config-devices.mak
  GEN   sparc64-linux-user/config-devices.mak
  GEN   sparc32plus-linux-user/config-devices.mak
  GEN   unicore32-linux-user/config-devices.mak
  GEN   s390x-linux-user/config-devices.mak
  GEN   config-host.h
  GEN   qemu-options.def
  GEN   qmp-commands.h
  GEN   qapi-types.h
  GEN   qapi-visit.h
  GEN   trace/generated-tracers.h
  GEN   tests/test-qapi-types.h
  GEN   tests/test-qapi-visit.h
  GEN   tests/test-qmp-commands.h
  GEN   config-all-devices.mak
  lt CC libqblock/libqblock.lo
  lt CC stubs/arch-query-cpu-def.lo
  lt CC libqblock/libqblock-error.lo
  lt CC stubs/cpu-get-clock.lo
  lt CC stubs/clock-warp.lo
  lt CC stubs/cpu-get-icount.lo
  lt CC stubs/fdset-add-fd.lo
  lt CC stubs/fdset-get-fd.lo
  lt CC stubs/fdset-find-fd.lo
  lt CC stubs/fdset-remove-fd.lo
  lt CC stubs/get-fd.lo
  lt CC stubs/get-vm-name.lo
  lt CC stubs/migr-blocker.lo
  lt CC stubs/iothread-lock.lo
  lt CC stubs/mon-is-qmp.lo
  lt CC stubs/mon-printf.lo
  lt CC stubs/mon-print-filename.lo
  lt CC stubs/mon-protocol-event.lo
  lt CC stubs/mon-set-error.lo
  lt CC stubs/reset.lo
  lt CC stubs/slirp.lo
  lt CC stubs/sysbus.lo
  lt CC stubs/vm-stop.lo
  lt CC stubs/vmstate.lo
  lt CC qapi/qapi-visit-core.lo
  lt CC qapi/qapi-dealloc-visitor.lo
  lt CC qapi/qmp-input-visitor.lo
  lt CC qapi/qmp-output-visitor.lo
  lt CC qapi/qmp-registry.lo
  lt CC qapi/qmp-dispatch.lo
  lt CC qapi/string-input-visitor.lo
  lt CC qapi/string-output-visitor.lo
  lt CC qapi/opts-visitor.lo
  lt CC qobject/qint.lo
  lt CC qobject/qstring.lo
  lt CC qobject/qdict.lo
  lt CC qobject/qlist.lo
  lt CC qobject/qfloat.lo
  lt CC qobject/qbool.lo
  lt CC qobject/qjson.lo
  lt CC qobject/json-lexer.lo
  lt CC qobject/json-streamer.lo
  lt CC qobject/json-parser.lo
  lt CC qobject/qerror.lo
  lt CC trace/default.lo
  lt CC trace/control.lo
  GEN   trace/generated-tracers.c
  lt CC util/osdep.lo
  lt CC util/cutils.lo
  lt CC util/qemu-timer-common.lo
  lt CC util/oslib-posix.lo
  lt CC util/qemu-thread-posix.lo
  lt CC util/event_notifier-posix.lo
  lt CC util/envlist.lo
  lt CC util/path.lo
  lt CC util/host-utils.lo
  lt CC util/cache-utils.lo
  lt CC util/module.lo
  lt CC util/bitmap.lo
  lt CC util/bitops.lo
  lt CC util/hbitmap.lo
  lt CC util/acl.lo
  lt CC util/error.lo
  lt CC util/qemu-error.lo
  lt CC util/compatfd.lo
  lt CC util/iov.lo
  lt CC util/aes.lo
  lt CC util/qemu-config.lo
  lt CC util/qemu-sockets.lo
  lt CC util/uri.lo
  lt CC util/notify.lo
  lt CC util/qemu-option.lo
  lt CC util/qemu-progress.lo
  lt CC async.lo
  lt CC thread-pool.lo
  lt CC nbd.lo
  lt CC block.lo
  lt CC blockjob.lo
  lt CC main-loop.lo
  lt CC iohandler.lo
  lt CC qemu-timer.lo
  lt CC aio-posix.lo
  GEN   qapi-types.c
  GEN   qapi-visit.c
  lt CC qemu-coroutine.lo
  lt CC qemu-coroutine-lock.lo
  lt CC qemu-coroutine-io.lo
  lt CC qemu-coroutine-sleep.lo
  lt CC coroutine-ucontext.lo
  lt CC block/raw.lo
  lt CC block/cow.lo
  lt CC block/qcow.lo
  lt CC block/vdi.lo
  lt CC block/vmdk.lo
  lt CC block/cloop.lo
  lt CC block/dmg

  1   2   3   >