Re: [Qemu-devel] GSoC mentor summit QEMU users session

[Markus Armbruster]

Anthony Liguori  writes:

> On 11/02/2011 03:24 PM, Blue Swirl wrote:
>> On Wed, Nov 2, 2011 at 19:35, Anthony Liguori  wrote:
>>> For the record, I'm opposed to ever having a stable plugin API.
>>>
>>> We aren't a closed source product.  If people want to have to keep up with
>>> our changing internal interfaces, they can get their code merged upstream.
>>
>> Fully agree. I don't even think there can be any benefit for us from a
>> plugin system, only API/ABI legacy maintenance effort and limitations
>> to architectural changes. All benefits are external.
>>
>> There are other useful paths available for external users, QAPI, QMP,
>> GDBstub, maybe also instrumentation in the future.
>>
>> Perhaps we could add a 'contrib' directory where new stuff could be
>> added easily to make life outside better. It could also be used to
>> clean up bitrotten devices and functionality from core. Marking things
>> with deprecated attributes or something like CONFIG_EXPERIMENTAL in
>> Linux could also help.
>
> I like having dynamic modules for a few reasons:
>
> 1) It makes us start thinking in a more modular fashion.  We're
> getting close to a million lines of code--we really do need to do a
> better job at modularity if we want to keep growing at this rate.

You can turn any collection of .o and .h into .so with the same .h.
Can't see how that will magically make us think in a more modular
fashion.

But once you have clear interfaces, linking modules separately can help
controlling the cross-module references, and thus help defeating
attempts to bypass the interfaces.

Let's start with step 1.  Earth to Anthony, earth to Anthony, please get
the QOM job done before chasing dynamic modules!  ;-P

> 2) It makes life a bit nicer for distributions as we grow optional
> dependencies. Having a single QEMU binary that links against libtpms
> and libspice means that a distro needs to make libtpms/libspice a
> Requires in the package.  libspice has a ton of dependencies which
> makes QEMU have a ton of hard dependencies. Instead, if a distro can
> build a QEMU package and a qemu-spice package, the libspice dependency
> can be isolated to the qemu-spice package.
>
> That means you can install QEMU on your server without pulling in all
> sorts of ffmpeg stuff if you don't care about Spice.  I'm picking on
> Spice here, but there are lots of other examples here (vde2, libbaum,
> SDL, etc.).
>
> 3) It lets people do out-of-tree development without forking the whole
> tree.  As much as I don't like out-of-tree development, it's a
> reality.  If things like the Android SDK could be done just as a set
> of plugins, it would be better for all of us.

If history is any guide, Google forks everything anyway ;)

> 4) We'll hopefully get to a point where most of what we do can be
> demand loaded. This helps us establish a smaller in memory base which
> is important when it comes to Security Certifications[1].
>
> [1] Security Certification has only an arguable relationship with
> actual security but it is, nonetheless, and important consideration.

Re: [Qemu-devel] GSoC mentor summit QEMU users session

[Stefan Hajnoczi]

On Wed, Nov 2, 2011 at 6:51 PM, Anthony Liguori  wrote:
> @@ -77,6 +78,20 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info,
> const ch
>             continue;
>         return info;
>     }
> +
> +    /* try to load an appropriately named module */
> +    {
> +        char *path = g_module_build_path(PREFIX, info->name);
> +        GModule *mod = g_module_open(path, G_MODULE_BIND_LOCAL);
> +        void (*init)(void);
> +
> +        if (g_module_symbol(mod, "mod_init", &init)) {
> +            init();

We need taint marker support from day 1.  Or perhaps a nicer thing:
require each module to declare a support URL string.  QEMU will print
out support URLs when displaying version information or on startup.
This way it's easy to indentify who to ask for help if QEMU appears
broken - we'll know if a third-party plugin is involved.

Stefan

Re: [Qemu-devel] GSoC mentor summit QEMU users session

[Stefan Hajnoczi]

On Wed, Nov 2, 2011 at 5:39 PM, Fabien Chouteau  wrote:
> On 29/10/2011 15:52, Alexander Graf wrote:
>> The RTEMS guys use QEMU to do coverage testing of their kernel code.
>> They run their test-cases and see if all of their code and branches
>> have been hit. Adacore seems to have a patches version of QEMU to
>> provide an easily parsable log file for that sort of thing. Might be a
>> good idea to consolidate upstream. Patches welcome :)
>
> That's right we do have a coverage analysis solution based on Qemu.
> Execution traces generated by Qemu are analyzed by the GNATcoverage
> tool to provide object, statement, decision or Modified
> Condition/Decision coverage analysis.
>
> The main interest of Qemu is to provide execution traces without
> code instrumentation.
>
> Alex, it is of course in our plans to submit patches to the upstream Qemu ;)
>
> I give you some pointers if you want to find more on GNATcoverage (technical 
> stuff :)
>
>  * GNATcoverage is hosted on forge.open-do.org. You can checkout the
>   repository with:
>
>     $ svn checkout 
> svn://scm.forge.open-do.org/scmrepos/svn/couverture/trunk/couverture
>
>   From there, you have
>     * the tool source tree in "tools"
>     * a synthesis article in "publications" : 201005-erts2.pdf
>
>  * GNATcov's documentation attached to this email

Thanks for mentioning Couverture.  Just yesterday I talked to neo_1987
on IRC who is trying to get QEMU /w couverture to work for him.  We
ran into a little bit of confusion because the -trace command-line
option is also used in upstream QEMU but for a different purpose
(http://wiki.qemu.org/Features/Tracing).

I took a quick peak at the qemu-trace.[ch] from couverture and it
looks along the lines of the instrumentation that others have been
doing too.  I hope you have time to propose the coverage
instrumentation for upstream QEMU.

Stefan

Re: [Qemu-devel] GSoC mentor summit QEMU users session

[Markus Armbruster]

Alexander Graf  writes:

> Anthony Liguori wrote:
>> On 11/02/2011 01:17 PM, Jan Kiszka wrote:
>>> On 2011-11-02 18:44, Fabien Chouteau wrote:
 On 31/10/2011 14:12, Peter Maydell wrote:
> On 29 October 2011 14:52, Alexander Graf  wrote:
>> A lot of people seem to also have code that doesn't make sense
>> upstream, for example implementing a one-off device that only
>> really matters for their own devboard which nobody else owns.
>> For such cases, having a plugin framework would be handy. I
>> interestingly enough got into the same discussion on LinuxCon
>> with some QEMU downstreams.
>
> If we get the qdev rework done then I think we're probably in
> a better position to have a plugin framework for devices. (There
> are some issues about API and ABI stability guarantees, of course.)
>

 Interesting, we have a "plug-in" implementation in our Qemu branch. It
>>>
>>> We have a "plugin" model here as well. It's really simple: the plugin is
>>> loaded dynamically into the QEMU process and can access any global
>>> function and variable. Of course, this breaks regularly.
>>
>> Yes, this is the Right Model.
>>
>> All of the work is in making the interfaces not break regularly. 
>> Loading a shared object is easy enough.
>
> I agree. In fact, we could even do it the same way as the kernel and
> build all our internal hw pieces as shared objects.
>
> Then users who want to cut down QEMU can just remove .so files instead
> of messing with the build system or code.

Shared objects have a non-zero cost, both at load time, and at run time.
Whether we can accept that cost just to simplify configuration
management (for a definition of "simplify") isn't obvious.

If removing an optional device model from the build isn't entirely
trivial, we already failed.

Re: [Qemu-devel] GSoC mentor summit QEMU users session

[Markus Armbruster]

Anthony Liguori  writes:

[...]
> For the record, I'm opposed to ever having a stable plugin API.
>
> We aren't a closed source product.  If people want to have to keep up
> with our changing internal interfaces, they can get their code merged
> upstream.

Seconded.

Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?

[Paolo Bonzini]

On 11/02/2011 10:22 PM, Thomas Schmitt wrote:

Hi,

i wrote:

So how is this altered to 0x12 in the further course of processing ?


Paolo Bonzini wrote:

Because you're using an *IDE* (ATAPI) CD-ROM, not SCSI.  See hw/ide/atapi.c.


You convinced me.
But this does not explain yet the difference in behavior of
both groups of IDE DVD-ROMs. Why are those of -drive if=scsi empty ?


They are not "of -drive if=scsi".  They are "of -cdrom", and unlike the 
SCSI ones they are always present, even if no -cdrom is given (you can 
use -nodefaults to remove all default devices, including the IDE 
CD-ROM).  So if you specified "-drive if=scsi" *and* "-cdrom", you'd get 
two non-empty drives.



Ok, in your counting I should have written 20 for IDE (0x12 + page number +
page size) and 22 for SCSI (0x14 + page number + page size).


Why the distinction by transport bus ? Mode page 2Ah is a matter
of the drive alone. MMC is independent of the bus.


It's just an artefact of having two separate implementations for ATAPI 
and SCSI.



I did not yet find out, why the IDE drive emulation emits a Mode Data
Length of 0x22. With Page Length 0x12 it should have been 0x1a.
8 too many.


Yeah, looks like all the

case MODE_PAGE_R_W_ERROR: /* error recovery */
cpu_to_ube16(&buf[0], 16 + 6);

should have "- 2" instead of "+ 6".

Paolo

Re: [Qemu-devel] [PATCH v4 1/4] Add basic version of bridge helper

[Stefan Hajnoczi]

On Wed, Nov 2, 2011 at 3:12 PM, Corey Bryant  wrote:
> On 11/02/2011 06:58 AM, Stefan Hajnoczi wrote:
>>
>> On Tue, Nov 1, 2011 at 5:13 PM, Corey Bryant
>>  wrote:
>>>
>>> +static bool has_vnet_hdr(int fd)
>>> +{
>>> +    unsigned int features = 0;
>>> +    struct ifreq ifreq;
>>> +
>>> +    if (ioctl(fd, TUNGETFEATURES,&features) == -1) {
>>> +        return false;
>>> +    }
>>> +
>>> +    if (!(features&  IFF_VNET_HDR)) {
>>> +        return false;
>>> +    }
>>> +
>>> +    if (ioctl(fd, TUNGETIFF,&ifreq) != -1 || errno != EBADFD) {
>>> +        return false;
>>> +    }
>>
>> I don't understand this expression.  We want TUNGETIFF to fail with
>> EBADFD, otherwise we return false.  What is this trying to do?
>>
>> Why do we even need TUNGETIFF after TUNGETFEATURES succeeded and we
>> were able to check out the IFF_VNET_HDR flag?
>>
>> Stefan
>>
>
> I don't think the TUNGETIFF call is necessary.  It verifies that the tap
> device doesn't already exist.  The ensuing TUNSETIFF call should already
> cover this.

Thanks for explaining.  It would be nice to comment this or drop the
if statement entirely if you are sending another revision of this
patch series.

Stefan

Re: [Qemu-devel] [RFC 1/6] block: add request tracking

[Stefan Hajnoczi]

On Wed, Nov 2, 2011 at 4:30 PM, Kevin Wolf  wrote:
> Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
>> The block layer does not know about pending requests.  This information
>> is necessary for copy-on-read since overlapping requests must be
>> serialized to prevent races that corrupt the image.
>>
>> Add a simple mechanism to enable/disable request tracking.  By default
>> request tracking is disabled.
>>
>> The BlockDriverState gets a new tracked_request list field which
>> contains all pending requests.  Each request is a BdrvTrackedRequest
>> record with sector_num, nb_sectors, and is_write fields.
>>
>> Signed-off-by: Stefan Hajnoczi 
>> ---
>>  block.c     |   83 
>> ++-
>>  block_int.h |    8 +
>>  2 files changed, 90 insertions(+), 1 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 9873b57..2d2c62a 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -978,6 +978,77 @@ void bdrv_commit_all(void)
>>      }
>>  }
>>
>> +struct BdrvTrackedRequest {
>> +    BlockDriverState *bs;
>> +    int64_t sector_num;
>> +    int nb_sectors;
>> +    bool is_write;
>> +    QLIST_ENTRY(BdrvTrackedRequest) list;
>> +};
>> +
>> +/**
>> + * Remove an active request from the tracked requests list
>> + *
>> + * If req is NULL, no operation is performed.
>> + *
>> + * This function should be called when a tracked request is completing.
>> + */
>> +static void tracked_request_remove(BdrvTrackedRequest *req)
>> +{
>> +    if (req) {
>> +        QLIST_REMOVE(req, list);
>> +        g_free(req);
>> +    }
>> +}
>> +
>> +/**
>> + * Add an active request to the tracked requests list
>> + *
>> + * Return NULL if request tracking is disabled, non-NULL otherwise.
>> + */
>> +static BdrvTrackedRequest *tracked_request_add(BlockDriverState *bs,
>> +                                               int64_t sector_num,
>> +                                               int nb_sectors, bool 
>> is_write)
>> +{
>> +    BdrvTrackedRequest *req;
>> +
>> +    if (!bs->request_tracking) {
>> +        return NULL;
>> +    }
>> +
>> +    req = g_malloc(sizeof(*req));
>> +    req->bs = bs;
>> +    req->sector_num = sector_num;
>> +    req->nb_sectors = nb_sectors;
>> +    req->is_write = is_write;
>
> How about using g_malloc0 or a compound literal assignment for
> initialisation, so that we won't get surprises when the struct is extended?

Sure.

>> +
>> +    QLIST_INSERT_HEAD(&bs->tracked_requests, req, list);
>> +
>> +    return req;
>> +}
>> +
>> +/**
>> + * Enable tracking of incoming requests
>> + *
>> + * Request tracking can be safely used by multiple users at the same time,
>> + * there is an internal reference count to match start and stop calls.
>> + */
>> +void bdrv_start_request_tracking(BlockDriverState *bs)
>> +{
>> +    bs->request_tracking++;
>> +}
>> +
>> +/**
>> + * Disable tracking of incoming requests
>> + *
>> + * Note that in-flight requests are still tracked, this function only stops
>> + * tracking incoming requests.
>> + */
>> +void bdrv_stop_request_tracking(BlockDriverState *bs)
>> +{
>> +    bs->request_tracking--;
>> +}
>> +
>>  /*
>>   * Return values:
>>   * 0        - success
>> @@ -1262,6 +1333,8 @@ static int coroutine_fn 
>> bdrv_co_do_readv(BlockDriverState *bs,
>>      int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
>>  {
>>      BlockDriver *drv = bs->drv;
>> +    BdrvTrackedRequest *req;
>> +    int ret;
>>
>>      if (!drv) {
>>          return -ENOMEDIUM;
>> @@ -1270,7 +1343,10 @@ static int coroutine_fn 
>> bdrv_co_do_readv(BlockDriverState *bs,
>>          return -EIO;
>>      }
>>
>> -    return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
>> +    req = tracked_request_add(bs, sector_num, nb_sectors, false);
>> +    ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
>> +    tracked_request_remove(req);
>
> Hm... Do we actually need to malloc the BdrvTrackedRequest? If all users
> are like this (and at least those in this patch are), we can just keep
> it on the coroutine stack.

Okay.

Stefan

Re: [Qemu-devel] [PATCH v11 2/4] block: add I/O throttling algorithm

[Kevin Wolf]

Am 03.11.2011 04:18, schrieb Zhi Yong Wu:
> On Wed, Nov 2, 2011 at 7:51 PM, Kevin Wolf  wrote:
>> Am 02.11.2011 07:01, schrieb Zhi Yong Wu:
>>> Signed-off-by: Zhi Yong Wu 
>>> Signed-off-by: Stefan Hajnoczi 
>>> ---
>>>  block.c   |  233 
>>> +++--
>>>  block.h   |1 +
>>>  block_int.h   |1 +
>>>  qemu-coroutine-lock.c |8 ++
>>>  qemu-coroutine.h  |6 ++
>>>  5 files changed, 242 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/block.c b/block.c
>>> index c70f86d..440e889 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -74,6 +74,13 @@ static BlockDriverAIOCB 
>>> *bdrv_co_aio_rw_vector(BlockDriverState *bs,
>>> bool is_write);
>>>  static void coroutine_fn bdrv_co_do_rw(void *opaque);
>>>
>>> +static bool bdrv_exceed_bps_limits(BlockDriverState *bs, int nb_sectors,
>>> +bool is_write, double elapsed_time, uint64_t *wait);
>>> +static bool bdrv_exceed_iops_limits(BlockDriverState *bs, bool is_write,
>>> +double elapsed_time, uint64_t *wait);
>>> +static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
>>> +bool is_write, int64_t *wait);
>>> +
>>>  static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
>>>  QTAILQ_HEAD_INITIALIZER(bdrv_states);
>>>
>>> @@ -107,6 +114,28 @@ int is_windows_drive(const char *filename)
>>>  #endif
>>>
>>>  /* throttling disk I/O limits */
>>> +void bdrv_io_limits_disable(BlockDriverState *bs)
>>> +{
>>> +bs->io_limits_enabled = false;
>>> +
>>> +if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
>>
>> This if is unnecessary, you can just always run the while loop.
> Good catch. Removed.
>>
>>> +while (qemu_co_queue_next(&bs->throttled_reqs));
>>> +}
>>> +
>>> +qemu_co_queue_init(&bs->throttled_reqs);
>>
>> Why?
> Removed.
>>
>>> +
>>> +if (bs->block_timer) {
>>> +qemu_del_timer(bs->block_timer);
>>> +qemu_free_timer(bs->block_timer);
>>> +bs->block_timer = NULL;
>>> +}
>>> +
>>> +bs->slice_start = 0;
>>> +bs->slice_end   = 0;
>>> +bs->slice_time  = 0;
>>> +memset(&bs->io_disps, 0, sizeof(bs->io_disps));
>>> +}
>>> +
>>>  static void bdrv_block_timer(void *opaque)
>>>  {
>>>  BlockDriverState *bs = opaque;
>>> @@ -116,14 +145,13 @@ static void bdrv_block_timer(void *opaque)
>>>
>>>  void bdrv_io_limits_enable(BlockDriverState *bs)
>>>  {
>>> -bs->io_limits_enabled = true;
>>>  qemu_co_queue_init(&bs->throttled_reqs);
>>> -
>>> -bs->block_timer   = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
>>> -bs->slice_time= 5 * BLOCK_IO_SLICE_TIME;
>>> -bs->slice_start   = qemu_get_clock_ns(vm_clock);
>>> -bs->slice_end = bs->slice_start + bs->slice_time;
>>> +bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
>>> +bs->slice_time  = 5 * BLOCK_IO_SLICE_TIME;
>>> +bs->slice_start = qemu_get_clock_ns(vm_clock);
>>> +bs->slice_end   = bs->slice_start + bs->slice_time;
>>
>> You're only changing whitespace here, right? If so, can you please use
>> the final version in patch 1?
> OK
>>
>>>  memset(&bs->io_disps, 0, sizeof(bs->io_disps));
>>> +bs->io_limits_enabled = true;
>>>  }
>>>
>>>  bool bdrv_io_limits_enabled(BlockDriverState *bs)
>>> @@ -137,6 +165,30 @@ bool bdrv_io_limits_enabled(BlockDriverState *bs)
>>>   || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
>>>  }
>>>
>>> +static void bdrv_io_limits_intercept(BlockDriverState *bs,
>>> + bool is_write, int nb_sectors)
>>> +{
>>> +int64_t wait_time = -1;
>>> +
>>> +if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
>>> +qemu_co_queue_wait(&bs->throttled_reqs);
>>> +goto resume;
>>> +} else if (bdrv_exceed_io_limits(bs, nb_sectors, is_write, 
>>> &wait_time)) {
>>> +qemu_mod_timer(bs->block_timer,
>>> +   wait_time + qemu_get_clock_ns(vm_clock));
>>> +qemu_co_queue_wait(&bs->throttled_reqs);
>>> +
>>> +resume:
>>
>> This goto construct isn't very nice. You could have avoided it with an
>> "else return;"
> else return? no, it can not be returned shortly, i think.
>>
>> But anyway, why do you even need the else if? Can't you directly use the
>> while loop? The difference would be that qemu_co_queue_next() is run
>> even if a request is allowed without going through the queue, but would
>> that hurt?
> Great point. thanks.
>>
>>
>>> +while (bdrv_exceed_io_limits(bs, nb_sectors, is_write, 
>>> &wait_time)) {
>>> +qemu_mod_timer(bs->block_timer,
>>> +   wait_time + qemu_get_clock_ns(vm_clock));
>>> +qemu_co_queue_wait_insert_head(&bs->throttled_reqs);
>>
>> Why do you want to insert at the head? Wouldn't a queue be more
> In fact, we hope to keep each request's timing, in FIFO mode. The next
> throttled requests will not be dequeued until the current request is

Re: [Qemu-devel] [RFC 2/6] block: add bdrv_set_copy_on_read()

[Stefan Hajnoczi]

On Wed, Nov 2, 2011 at 4:36 PM, Kevin Wolf  wrote:
> Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
>> The bdrv_set_copy_on_read() function can be used to programmatically
>> enable or disable copy-on-read for a block device.  Later patches add
>> the actual copy-on-read logic.
>>
>> Signed-off-by: Stefan Hajnoczi 
>> ---
>>  block.c     |   17 +
>>  block.h     |    3 +++
>>  block_int.h |    1 +
>>  3 files changed, 21 insertions(+), 0 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 2d2c62a..e624ac3 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -464,6 +464,18 @@ int bdrv_parse_cache_flags(const char *mode, int *flags)
>>      return 0;
>>  }
>>
>> +void bdrv_set_copy_on_read(BlockDriverState *bs, int enable)
>
> bool enable

Thanks for pointing this out.

>> +{
>> +    if (bs->copy_on_read != enable) {
>> +        if (enable) {
>> +            bdrv_start_request_tracking(bs);
>> +        } else {
>> +            bdrv_stop_request_tracking(bs);
>> +        }
>> +    }
>> +    bs->copy_on_read = enable;
>> +}
>> +
>>  /*
>>   * Common part for opening disk images and files
>>   */
>> @@ -483,6 +495,11 @@ static int bdrv_open_common(BlockDriverState *bs, const 
>> char *filename,
>>      bs->open_flags = flags;
>>      bs->buffer_alignment = 512;
>>
>> +    bs->copy_on_read = 0;
>
> I think it should rather be reset on close. We can assert() it here.

Okay.

Stefan

Re: [Qemu-devel] [PATCH v11 2/4] block: add I/O throttling algorithm

[Zhi Yong Wu]

On Thu, Nov 3, 2011 at 4:03 PM, Kevin Wolf  wrote:
> Am 03.11.2011 04:18, schrieb Zhi Yong Wu:
>> On Wed, Nov 2, 2011 at 7:51 PM, Kevin Wolf  wrote:
>>> Am 02.11.2011 07:01, schrieb Zhi Yong Wu:
 Signed-off-by: Zhi Yong Wu 
 Signed-off-by: Stefan Hajnoczi 
 ---
  block.c               |  233 
 +++--
  block.h               |    1 +
  block_int.h           |    1 +
  qemu-coroutine-lock.c |    8 ++
  qemu-coroutine.h      |    6 ++
  5 files changed, 242 insertions(+), 7 deletions(-)

 diff --git a/block.c b/block.c
 index c70f86d..440e889 100644
 --- a/block.c
 +++ b/block.c
 @@ -74,6 +74,13 @@ static BlockDriverAIOCB 
 *bdrv_co_aio_rw_vector(BlockDriverState *bs,
                                                 bool is_write);
  static void coroutine_fn bdrv_co_do_rw(void *opaque);

 +static bool bdrv_exceed_bps_limits(BlockDriverState *bs, int nb_sectors,
 +        bool is_write, double elapsed_time, uint64_t *wait);
 +static bool bdrv_exceed_iops_limits(BlockDriverState *bs, bool is_write,
 +        double elapsed_time, uint64_t *wait);
 +static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
 +        bool is_write, int64_t *wait);
 +
  static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
      QTAILQ_HEAD_INITIALIZER(bdrv_states);

 @@ -107,6 +114,28 @@ int is_windows_drive(const char *filename)
  #endif

  /* throttling disk I/O limits */
 +void bdrv_io_limits_disable(BlockDriverState *bs)
 +{
 +    bs->io_limits_enabled = false;
 +
 +    if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
>>>
>>> This if is unnecessary, you can just always run the while loop.
>> Good catch. Removed.
>>>
 +        while (qemu_co_queue_next(&bs->throttled_reqs));
 +    }
 +
 +    qemu_co_queue_init(&bs->throttled_reqs);
>>>
>>> Why?
>> Removed.
>>>
 +
 +    if (bs->block_timer) {
 +        qemu_del_timer(bs->block_timer);
 +        qemu_free_timer(bs->block_timer);
 +        bs->block_timer = NULL;
 +    }
 +
 +    bs->slice_start = 0;
 +    bs->slice_end   = 0;
 +    bs->slice_time  = 0;
 +    memset(&bs->io_disps, 0, sizeof(bs->io_disps));
 +}
 +
  static void bdrv_block_timer(void *opaque)
  {
      BlockDriverState *bs = opaque;
 @@ -116,14 +145,13 @@ static void bdrv_block_timer(void *opaque)

  void bdrv_io_limits_enable(BlockDriverState *bs)
  {
 -    bs->io_limits_enabled = true;
      qemu_co_queue_init(&bs->throttled_reqs);
 -
 -    bs->block_timer   = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
 -    bs->slice_time    = 5 * BLOCK_IO_SLICE_TIME;
 -    bs->slice_start   = qemu_get_clock_ns(vm_clock);
 -    bs->slice_end     = bs->slice_start + bs->slice_time;
 +    bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
 +    bs->slice_time  = 5 * BLOCK_IO_SLICE_TIME;
 +    bs->slice_start = qemu_get_clock_ns(vm_clock);
 +    bs->slice_end   = bs->slice_start + bs->slice_time;
>>>
>>> You're only changing whitespace here, right? If so, can you please use
>>> the final version in patch 1?
>> OK
>>>
      memset(&bs->io_disps, 0, sizeof(bs->io_disps));
 +    bs->io_limits_enabled = true;
  }

  bool bdrv_io_limits_enabled(BlockDriverState *bs)
 @@ -137,6 +165,30 @@ bool bdrv_io_limits_enabled(BlockDriverState *bs)
           || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
  }

 +static void bdrv_io_limits_intercept(BlockDriverState *bs,
 +                                     bool is_write, int nb_sectors)
 +{
 +    int64_t wait_time = -1;
 +
 +    if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
 +        qemu_co_queue_wait(&bs->throttled_reqs);
 +        goto resume;
 +    } else if (bdrv_exceed_io_limits(bs, nb_sectors, is_write, 
 &wait_time)) {
 +        qemu_mod_timer(bs->block_timer,
 +                       wait_time + qemu_get_clock_ns(vm_clock));
 +        qemu_co_queue_wait(&bs->throttled_reqs);
 +
 +resume:
>>>
>>> This goto construct isn't very nice. You could have avoided it with an
>>> "else return;"
>> else return? no, it can not be returned shortly, i think.
>>>
>>> But anyway, why do you even need the else if? Can't you directly use the
>>> while loop? The difference would be that qemu_co_queue_next() is run
>>> even if a request is allowed without going through the queue, but would
>>> that hurt?
>> Great point. thanks.
>>>
>>>
 +        while (bdrv_exceed_io_limits(bs, nb_sectors, is_write, 
 &wait_time)) {
 +            qemu_mod_timer(bs->block_timer,
 +                           wait_time + qemu_get_clock_ns(vm_clock));
 +            qemu_co_queue_wait_insert_head(&bs->throttled_reqs);
>>>
>>> Why do you want to ins

Re: [Qemu-devel] CDROM: why can it not be changed to PIO mode?

[Stefan Hajnoczi]

On Wed, Nov 2, 2011 at 8:35 AM, Zhi Yong Wu  wrote:
> HI, guys,
>
> I managed to change one cdrom on guest from DMA mode to PIO mode, but
> failed. Did anyone also met this same issue before? If you have some
> experience to solve this, Can you share with me?

Since no one knows the answer off-hand you can find out the reason for
yourself by looking at the hdparm and Linux driver code to see how it
switches modes.  Then look at QEMU's emulation to see why it's not
working.

Stefan

Re: [Qemu-devel] CDROM: why can it not be changed to PIO mode?

[Zhi Yong Wu]

OK.

On Thu, Nov 3, 2011 at 4:04 PM, Stefan Hajnoczi  wrote:
> On Wed, Nov 2, 2011 at 8:35 AM, Zhi Yong Wu  wrote:
>> HI, guys,
>>
>> I managed to change one cdrom on guest from DMA mode to PIO mode, but
>> failed. Did anyone also met this same issue before? If you have some
>> experience to solve this, Can you share with me?
>
> Since no one knows the answer off-hand you can find out the reason for
> yourself by looking at the hdparm and Linux driver code to see how it
> switches modes.  Then look at QEMU's emulation to see why it's not
> working.
>
> Stefan
>

-- 
Regards,

Zhi Yong Wu

Re: [Qemu-devel] GSoC mentor summit QEMU users session

[Andreas Färber]

Am 03.11.2011 08:46, schrieb Markus Armbruster:
> Anthony Liguori  writes:
> 
>> For the record, I'm opposed to ever having a stable plugin API.
>>
>> We aren't a closed source product.  If people [don't] want to have to keep up
>> with our changing internal interfaces, they can get their code merged
>> upstream.
> 
> Seconded.

Great. Then let's finally get megasas SCSI merged for 1.1. :)

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 1/3] usb-hub: wakeup on attach

[Gerd Hoffmann]

On 11/03/11 07:01, hkran wrote:
> On 11/02/2011 08:56 PM, Gerd Hoffmann wrote:
>>Hi,
>>
static void usb_hub_detach(USBPort *port1)
>>> pulled, In what cases, the usb hub will be suspended? and how to tell it
>>> happened? thanks.
>> The guest enables the remote-wakeup feature.  'lspci -v' (within the
>> guest) shows it.
>>
>> cheers,
>>Gerd
>>
> I use the param "-usb -usbdevice tablet" to start qemu  and then usb_del
> tablet via monitor console after getting qemu up.
> I saw this by entering lsusb -v
> 
> Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Device Descriptor:
>   bLength18
>   bDescriptorType 1
>   bcdUSB   1.10
>   bDeviceClass9 Hub
>   bDeviceSubClass 0 Unused
>   bDeviceProtocol 0 Full speed (or root) hub
>   bMaxPacketSize064
>   idVendor   0x1d6b Linux Foundation
>   idProduct  0x0001 1.1 root hub
>   bcdDevice2.06
>   iManufacturer   3 Linux 2.6.32-131.0.15.el6.x86_64 uhci_hcd
>   iProduct2 UHCI Host Controller
>   iSerial 1 :00:01.2
> ..
> Hub Descriptor:
>   bLength   9
>   bDescriptorType  41
>   nNbrPorts 2
>   wHubCharacteristic 0x000a
> No power switching (usb 1.0)
> Per-port overcurrent protection
>   bPwrOn2PwrGood1 * 2 milli seconds
>   bHubContrCurrent  0 milli Ampere
>   DeviceRemovable0x00
>   PortPwrCtrlMask0xff
>  Hub Port Status:
>Port 1: .0100 power
>Port 2: .0100 power
> Device Status: 0x0003
>   Self Powered
>   Remote Wakeup Enabled
> 
> (It seems that the Remote-wakeup feature has been enabled)
> There are two ports with the status "power",  and I do not know how to
> make it suspended yet. Can you help more ?

That is the root hub created by the linux kernel, not the hub emulated
by qemu.  If you hook up just a single device no hob is needed.

cheers,
  Gerd

[Qemu-devel] [PATCH v12 1/5] block: add the blockio limits command line support

[Zhi Yong Wu]

Signed-off-by: Zhi Yong Wu 
Signed-off-by: Stefan Hajnoczi 
---
 block.c |   39 +++
 block.h |4 
 block_int.h |   29 +
 blockdev.c  |   44 
 qemu-config.c   |   24 
 qemu-options.hx |1 +
 6 files changed, 141 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 9bb236c..79e7f09 100644
--- a/block.c
+++ b/block.c
@@ -30,6 +30,7 @@
 #include "qjson.h"
 #include "qemu-coroutine.h"
 #include "qmp-commands.h"
+#include "qemu-timer.h"
 
 #ifdef CONFIG_BSD
 #include 
@@ -105,6 +106,36 @@ int is_windows_drive(const char *filename)
 }
 #endif
 
+/* throttling disk I/O limits */
+static void bdrv_block_timer(void *opaque)
+{
+BlockDriverState *bs = opaque;
+
+qemu_co_queue_next(&bs->throttled_reqs);
+}
+
+void bdrv_io_limits_enable(BlockDriverState *bs)
+{
+qemu_co_queue_init(&bs->throttled_reqs);
+bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
+bs->slice_time  = 5 * BLOCK_IO_SLICE_TIME;
+bs->slice_start = qemu_get_clock_ns(vm_clock);
+bs->slice_end   = bs->slice_start + bs->slice_time;
+memset(&bs->io_base, 0, sizeof(bs->io_base));
+bs->io_limits_enabled = true;
+}
+
+bool bdrv_io_limits_enabled(BlockDriverState *bs)
+{
+BlockIOLimit *io_limits = &bs->io_limits;
+return io_limits->bps[BLOCK_IO_LIMIT_READ]
+ || io_limits->bps[BLOCK_IO_LIMIT_WRITE]
+ || io_limits->bps[BLOCK_IO_LIMIT_TOTAL]
+ || io_limits->iops[BLOCK_IO_LIMIT_READ]
+ || io_limits->iops[BLOCK_IO_LIMIT_WRITE]
+ || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
+}
+
 /* check if the path starts with ":" */
 static int path_has_protocol(const char *path)
 {
@@ -1519,6 +1550,14 @@ void bdrv_get_geometry_hint(BlockDriverState *bs,
 *psecs = bs->secs;
 }
 
+/* throttling disk io limits */
+void bdrv_set_io_limits(BlockDriverState *bs,
+BlockIOLimit *io_limits)
+{
+bs->io_limits = *io_limits;
+bs->io_limits_enabled = bdrv_io_limits_enabled(bs);
+}
+
 /* Recognize floppy formats */
 typedef struct FDFormat {
 FDriveType drive;
diff --git a/block.h b/block.h
index 38cd748..bc8315d 100644
--- a/block.h
+++ b/block.h
@@ -89,6 +89,10 @@ void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_stats_print(Monitor *mon, const QObject *data);
 void bdrv_info_stats(Monitor *mon, QObject **ret_data);
 
+/* disk I/O throttling */
+void bdrv_io_limits_enable(BlockDriverState *bs);
+bool bdrv_io_limits_enabled(BlockDriverState *bs);
+
 void bdrv_init(void);
 void bdrv_init_with_whitelist(void);
 BlockDriver *bdrv_find_protocol(const char *filename);
diff --git a/block_int.h b/block_int.h
index f4547f6..7315e0d 100644
--- a/block_int.h
+++ b/block_int.h
@@ -34,6 +34,12 @@
 #define BLOCK_FLAG_ENCRYPT 1
 #define BLOCK_FLAG_COMPAT6 4
 
+#define BLOCK_IO_LIMIT_READ 0
+#define BLOCK_IO_LIMIT_WRITE1
+#define BLOCK_IO_LIMIT_TOTAL2
+
+#define BLOCK_IO_SLICE_TIME 1
+
 #define BLOCK_OPT_SIZE  "size"
 #define BLOCK_OPT_ENCRYPT   "encryption"
 #define BLOCK_OPT_COMPAT6   "compat6"
@@ -50,6 +56,16 @@ typedef struct AIOPool {
 BlockDriverAIOCB *free_aiocb;
 } AIOPool;
 
+typedef struct BlockIOLimit {
+int64_t bps[3];
+int64_t iops[3];
+} BlockIOLimit;
+
+typedef struct BlockIOBaseValue {
+uint64_t bytes[2];
+uint64_t ios[2];
+} BlockIOBaseValue;
+
 struct BlockDriver {
 const char *format_name;
 int instance_size;
@@ -184,6 +200,16 @@ struct BlockDriverState {
 
 void *sync_aiocb;
 
+/* the time for latest disk I/O */
+int64_t slice_time;
+int64_t slice_start;
+int64_t slice_end;
+BlockIOLimit io_limits;
+BlockIOBaseValue  io_base;
+CoQueue  throttled_reqs;
+QEMUTimer*block_timer;
+bool io_limits_enabled;
+
 /* I/O stats (display with "info blockstats"). */
 uint64_t nr_bytes[BDRV_MAX_IOTYPE];
 uint64_t nr_ops[BDRV_MAX_IOTYPE];
@@ -227,6 +253,9 @@ void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
 void qemu_aio_release(void *p);
 
+void bdrv_set_io_limits(BlockDriverState *bs,
+BlockIOLimit *io_limits);
+
 #ifdef _WIN32
 int is_windows_drive(const char *filename);
 #endif
diff --git a/blockdev.c b/blockdev.c
index 0827bf7..651828c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -216,6 +216,26 @@ static int parse_block_error_action(const char *buf, int 
is_read)
 }
 }
 
+static bool do_check_io_limits(BlockIOLimit *io_limits)
+{
+bool bps_flag;
+bool iops_flag;
+
+assert(io_limits);
+
+bps_flag  = (io_limits->bps[BLOCK_IO_LIMIT_TOTAL] != 0)
+ && ((io_limits->bps[BLOCK_IO_LIMIT_READ] != 0)
+ || (io_limits->bps[BLOCK_IO_LIMIT_WRITE] != 0));
+iops_flag = (io_limits->iops[BLOCK_IO_LIMIT

[Qemu-devel] [PATCH v12 0/5] The intro to QEMU block I/O throttling

[Zhi Yong Wu]

The main goal of the patch is to effectively cap the disk I/O speed or counts 
of one single VM.It is only one draft, so it unavoidably has some drawbacks, if 
you catch them, please let me know.

The patch will mainly introduce one block I/O throttling algorithm, one timer 
and one block queue for each I/O limits enabled drive.

When a block request is coming in, the throttling algorithm will check if its 
I/O rate or counts exceed the limits; if yes, then it will enqueue to the block 
queue; The timer will handle the I/O requests in it.

Some available features follow as below:
(1) global bps limit.
   -drive bps=xxxin bytes/s
(2) only read bps limit
   -drive bps_rd=xxx in bytes/s
(3) only write bps limit
   -drive bps_wr=xxx in bytes/s
(4) global iops limit
   -drive iops=xxx   in ios/s
(5) only read iops limit
   -drive iops_rd=xxxin ios/s
(6) only write iops limit
   -drive iops_wr=xxxin ios/s
(7) the combination of some limits.
   -drive bps=xxx,iops=xxx

Known Limitations:
(1) #1 can not coexist with #2, #3
(2) #4 can not coexist with #5, #6

Changes since code V11:
   Made some changes based on kevin's comments.

 v11: Made some mininal changes based on stefan and Ryan's comments
  Add one perf report for block I/O throttling

 v10: Greately simply the logic and rebase request queue to CoQueue based on 
Stefan's comments.

 v9: made a lot of changes based on kevin's comments.
slice_time is dynamically adjusted based on wait_time.
rebase the latest qemu upstream.

 v8: fix the build per patch based on stefan's comments.

 v7: Mainly simply the block queue.
 Adjust codes based on stefan's comments.

 v6: Mainly fix the aio callback issue for block queue.
 Adjust codes based on Ram Pai's comments.

 v5: add qmp/hmp support.
 Adjust the codes based on stefan's comments
 qmp/hmp: add block_set_io_throttle

 v4: fix memory leaking based on ryan's feedback.

 v3: Added the code for extending slice time, and modified the method to 
compute wait time for the timer.

 v2: The codes V2 for QEMU disk I/O limits.
 Modified the codes mainly based on stefan's comments.

 v1: Submit the codes for QEMU disk I/O limits.


Zhi Yong Wu (5):
  block: add the blockio limits command line support
  CoQueue: introduce qemu_co_queue_wait_insert_head
  block: add I/O throttling algorithm
  hmp/qmp: add block_set_io_throttle
  block: perf testing report based on block I/O throttling

 10mbps.dat|  310 
 1mbps.dat |  339 +
 block.c   |  274 +++
 block.h   |5 +
 block_int.h   |   30 +
 blockdev.c|  103 +++
 blockdev.h|2 +
 hmp-commands.hx   |   15 ++
 hmp.c |   10 ++
 qapi-schema.json  |   16 ++-
 qemu-config.c |   24 
 qemu-coroutine-lock.c |8 +
 qemu-coroutine.h  |6 +
 qemu-options.hx   |1 +
 qerror.c  |4 +
 qerror.h  |3 +
 qmp-commands.hx   |   53 -
 17 files changed, 1201 insertions(+), 2 deletions(-)
 create mode 100644 10mbps.dat
 create mode 100644 1mbps.dat

-- 
1.7.6

[Qemu-devel] [PATCH v12 2/5] CoQueue: introduce qemu_co_queue_wait_insert_head

[Zhi Yong Wu]

Signed-off-by: Zhi Yong Wu 
Signed-off-by: Stefan Hajnoczi 
---
 qemu-coroutine-lock.c |8 
 qemu-coroutine.h  |6 ++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 6b58160..9549c07 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -61,6 +61,14 @@ void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
 assert(qemu_in_coroutine());
 }
 
+void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue)
+{
+Coroutine *self = qemu_coroutine_self();
+QTAILQ_INSERT_HEAD(&queue->entries, self, co_queue_next);
+qemu_coroutine_yield();
+assert(qemu_in_coroutine());
+}
+
 bool qemu_co_queue_next(CoQueue *queue)
 {
 Coroutine *next;
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index b8fc4f4..8a2e5d2 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -118,6 +118,12 @@ void qemu_co_queue_init(CoQueue *queue);
 void coroutine_fn qemu_co_queue_wait(CoQueue *queue);
 
 /**
+ * Adds the current coroutine to the head of the CoQueue and transfers control 
to the
+ * caller of the coroutine.
+ */
+void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue);
+
+/**
  * Restarts the next coroutine in the CoQueue and removes it from the queue.
  *
  * Returns true if a coroutine was restarted, false if the queue is empty.
-- 
1.7.6

[Qemu-devel] [PATCH v12 3/5] block: add I/O throttling algorithm

[Zhi Yong Wu]

Signed-off-by: Zhi Yong Wu 
Signed-off-by: Stefan Hajnoczi 
---
 block.c |  220 +++
 block.h |1 +
 block_int.h |1 +
 3 files changed, 222 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 79e7f09..b2af48f 100644
--- a/block.c
+++ b/block.c
@@ -74,6 +74,13 @@ static BlockDriverAIOCB 
*bdrv_co_aio_rw_vector(BlockDriverState *bs,
bool is_write);
 static void coroutine_fn bdrv_co_do_rw(void *opaque);
 
+static bool bdrv_exceed_bps_limits(BlockDriverState *bs, int nb_sectors,
+bool is_write, double elapsed_time, uint64_t *wait);
+static bool bdrv_exceed_iops_limits(BlockDriverState *bs, bool is_write,
+double elapsed_time, uint64_t *wait);
+static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
+bool is_write, int64_t *wait);
+
 static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
 QTAILQ_HEAD_INITIALIZER(bdrv_states);
 
@@ -107,6 +114,24 @@ int is_windows_drive(const char *filename)
 #endif
 
 /* throttling disk I/O limits */
+void bdrv_io_limits_disable(BlockDriverState *bs)
+{
+bs->io_limits_enabled = false;
+
+while (qemu_co_queue_next(&bs->throttled_reqs));
+
+if (bs->block_timer) {
+qemu_del_timer(bs->block_timer);
+qemu_free_timer(bs->block_timer);
+bs->block_timer = NULL;
+}
+
+bs->slice_start = 0;
+bs->slice_end   = 0;
+bs->slice_time  = 0;
+memset(&bs->io_base, 0, sizeof(bs->io_base));
+}
+
 static void bdrv_block_timer(void *opaque)
 {
 BlockDriverState *bs = opaque;
@@ -136,6 +161,31 @@ bool bdrv_io_limits_enabled(BlockDriverState *bs)
  || io_limits->iops[BLOCK_IO_LIMIT_TOTAL];
 }
 
+static void bdrv_io_limits_intercept(BlockDriverState *bs,
+ bool is_write, int nb_sectors)
+{
+int64_t wait_time = -1;
+
+if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
+qemu_co_queue_wait(&bs->throttled_reqs);
+}
+
+/* In fact, we hope to keep each request's timing, in FIFO mode. The next
+ * throttled requests will not be dequeued until the current request is
+ * allowed to be serviced. So if the current request still exceeds the
+ * limits, it will be inserted to the head. All requests followed it will
+ * be still in throttled_reqs queue.
+ */
+
+while (bdrv_exceed_io_limits(bs, nb_sectors, is_write, &wait_time)) {
+qemu_mod_timer(bs->block_timer,
+   wait_time + qemu_get_clock_ns(vm_clock));
+qemu_co_queue_wait_insert_head(&bs->throttled_reqs);
+}
+
+qemu_co_queue_next(&bs->throttled_reqs);
+}
+
 /* check if the path starts with ":" */
 static int path_has_protocol(const char *path)
 {
@@ -718,6 +768,11 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
int flags,
 bdrv_dev_change_media_cb(bs, true);
 }
 
+/* throttling disk I/O limits */
+if (bs->io_limits_enabled) {
+bdrv_io_limits_enable(bs);
+}
+
 return 0;
 
 unlink_and_fail:
@@ -753,6 +808,11 @@ void bdrv_close(BlockDriverState *bs)
 
 bdrv_dev_change_media_cb(bs, false);
 }
+
+/*throttling disk I/O limits*/
+if (bs->io_limits_enabled) {
+bdrv_io_limits_disable(bs);
+}
 }
 
 void bdrv_close_all(void)
@@ -1291,6 +1351,11 @@ static int coroutine_fn 
bdrv_co_do_readv(BlockDriverState *bs,
 return -EIO;
 }
 
+/* throttling disk read I/O */
+if (bs->io_limits_enabled) {
+bdrv_io_limits_intercept(bs, false, nb_sectors);
+}
+
 return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
 }
 
@@ -1321,6 +1386,11 @@ static int coroutine_fn 
bdrv_co_do_writev(BlockDriverState *bs,
 return -EIO;
 }
 
+/* throttling disk write I/O */
+if (bs->io_limits_enabled) {
+bdrv_io_limits_intercept(bs, true, nb_sectors);
+}
+
 ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
 
 if (bs->dirty_bitmap) {
@@ -2512,6 +2582,156 @@ void bdrv_aio_cancel(BlockDriverAIOCB *acb)
 acb->pool->cancel(acb);
 }
 
+/* block I/O throttling */
+static bool bdrv_exceed_bps_limits(BlockDriverState *bs, int nb_sectors,
+ bool is_write, double elapsed_time, uint64_t *wait) {
+uint64_t bps_limit = 0;
+double   bytes_limit, bytes_base, bytes_res;
+double   slice_time, wait_time;
+
+if (bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL]) {
+bps_limit = bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL];
+} else if (bs->io_limits.bps[is_write]) {
+bps_limit = bs->io_limits.bps[is_write];
+} else {
+if (wait) {
+*wait = 0;
+}
+
+return false;
+}
+
+slice_time = bs->slice_end - bs->slice_start;
+slice_time /= (NANOSECONDS_PER_SECOND);
+bytes_limit = bps_limit * slice_time;
+bytes_base  = bs->nr_bytes[is_write] - bs->io_base.bytes[is_write];
+if (bs->io_limits.bps[BLOCK_IO_L

[Qemu-devel] [PATCH v12 4/5] hmp/qmp: add block_set_io_throttle

[Zhi Yong Wu]

Signed-off-by: Zhi Yong Wu 
Signed-off-by: Stefan Hajnoczi 
---
 block.c  |   15 +
 blockdev.c   |   59 ++
 blockdev.h   |2 +
 hmp-commands.hx  |   15 +
 hmp.c|   10 +
 qapi-schema.json |   16 +-
 qerror.c |4 +++
 qerror.h |3 ++
 qmp-commands.hx  |   53 +++-
 9 files changed, 175 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index b2af48f..ed6fe20 100644
--- a/block.c
+++ b/block.c
@@ -1971,6 +1971,21 @@ BlockInfoList *qmp_query_block(Error **errp)
 info->value->inserted->has_backing_file = true;
 info->value->inserted->backing_file = 
g_strdup(bs->backing_file);
 }
+
+if (bs->io_limits_enabled) {
+info->value->inserted->bps =
+   bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL];
+info->value->inserted->bps_rd =
+   bs->io_limits.bps[BLOCK_IO_LIMIT_READ];
+info->value->inserted->bps_wr =
+   bs->io_limits.bps[BLOCK_IO_LIMIT_WRITE];
+info->value->inserted->iops =
+   bs->io_limits.iops[BLOCK_IO_LIMIT_TOTAL];
+info->value->inserted->iops_rd =
+   bs->io_limits.iops[BLOCK_IO_LIMIT_READ];
+info->value->inserted->iops_wr =
+   bs->io_limits.iops[BLOCK_IO_LIMIT_WRITE];
+}
 }
 
 /* XXX: waiting for the qapi to support GSList */
diff --git a/blockdev.c b/blockdev.c
index 651828c..95d1faa 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -757,6 +757,65 @@ int do_change_block(Monitor *mon, const char *device,
 return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
 }
 
+/* throttling disk I/O limits */
+int do_block_set_io_throttle(Monitor *mon,
+   const QDict *qdict, QObject **ret_data)
+{
+BlockIOLimit io_limits;
+const char *devname = qdict_get_str(qdict, "device");
+BlockDriverState *bs;
+
+io_limits.bps[BLOCK_IO_LIMIT_TOTAL]
+= qdict_get_try_int(qdict, "bps", -1);
+io_limits.bps[BLOCK_IO_LIMIT_READ]
+= qdict_get_try_int(qdict, "bps_rd", -1);
+io_limits.bps[BLOCK_IO_LIMIT_WRITE]
+= qdict_get_try_int(qdict, "bps_wr", -1);
+io_limits.iops[BLOCK_IO_LIMIT_TOTAL]
+= qdict_get_try_int(qdict, "iops", -1);
+io_limits.iops[BLOCK_IO_LIMIT_READ]
+= qdict_get_try_int(qdict, "iops_rd", -1);
+io_limits.iops[BLOCK_IO_LIMIT_WRITE]
+= qdict_get_try_int(qdict, "iops_wr", -1);
+
+bs = bdrv_find(devname);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, devname);
+return -1;
+}
+
+if ((io_limits.bps[BLOCK_IO_LIMIT_TOTAL] == -1)
+|| (io_limits.bps[BLOCK_IO_LIMIT_READ] == -1)
+|| (io_limits.bps[BLOCK_IO_LIMIT_WRITE] == -1)
+|| (io_limits.iops[BLOCK_IO_LIMIT_TOTAL] == -1)
+|| (io_limits.iops[BLOCK_IO_LIMIT_READ] == -1)
+|| (io_limits.iops[BLOCK_IO_LIMIT_WRITE] == -1)) {
+qerror_report(QERR_MISSING_PARAMETER,
+  "bps/bps_rd/bps_wr/iops/iops_rd/iops_wr");
+return -1;
+}
+
+if (!do_check_io_limits(&io_limits)) {
+qerror_report(QERR_INVALID_PARAMETER_COMBINATION);
+return -1;
+}
+
+bs->io_limits = io_limits;
+bs->slice_time = BLOCK_IO_SLICE_TIME;
+
+if (!bs->io_limits_enabled && bdrv_io_limits_enabled(bs)) {
+bdrv_io_limits_enable(bs);
+} else if (bs->io_limits_enabled && !bdrv_io_limits_enabled(bs)) {
+bdrv_io_limits_disable(bs);
+} else {
+if (bs->block_timer) {
+qemu_mod_timer(bs->block_timer, qemu_get_clock_ns(vm_clock));
+}
+}
+
+return 0;
+}
+
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 const char *id = qdict_get_str(qdict, "id");
diff --git a/blockdev.h b/blockdev.h
index 3587786..1b48a75 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -63,6 +63,8 @@ int do_block_set_passwd(Monitor *mon, const QDict *qdict, 
QObject **ret_data);
 int do_change_block(Monitor *mon, const char *device,
 const char *filename, const char *fmt);
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_block_set_io_throttle(Monitor *mon,
+ const QDict *qdict, QObject **ret_data);
 int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 089c1ac..48f3c21 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1207,6 +1207,21 @@ ETEXI
 },
 
 STEXI
+@item block_

[Qemu-devel] [PATCH v12 5/5] block: perf testing report based on block I/O throttling

[Zhi Yong Wu]

The file 1mbps.dat is based on bps=1024*1024 I/O throttling; and the file 
10mbps.dat is based on bps=10*1024*1024 I/O throttling.

Signed-off-by: Zhi Yong Wu 
---
 10mbps.dat |  310 ++
 1mbps.dat  |  339 
 2 files changed, 649 insertions(+), 0 deletions(-)
 create mode 100644 10mbps.dat
 create mode 100644 1mbps.dat

diff --git a/10mbps.dat b/10mbps.dat
new file mode 100644
index 000..2ef419b
--- /dev/null
+++ b/10mbps.dat
@@ -0,0 +1,310 @@
+test: (g=0): rw=write, bs=512-512/512-512, ioengine=libaio, iodepth=1
+Starting 1 process
+Jobs: 1 (f=1): [W] [100.0% done] [0K/1,618K /s] [0/3K iops] [eta 00m:00s]
+test: (groupid=0, jobs=1): err= 0: pid=3552
+  write: io=51,200KB, bw=1,739KB/s, iops=3,478, runt= 29441msec
+slat (usec): min=14, max=9,047, avg=22.07, stdev=66.92
+clat (usec): min=1, max=120K, avg=262.82, stdev=555.45
+ lat (usec): min=213, max=120K, avg=285.49, stdev=571.78
+bw (KB/s) : min= 1223, max= 1847, per=100.11%, avg=1740.90, stdev=120.92
+  cpu  : usr=1.45%, sys=9.67%, ctx=102648, majf=0, minf=23
+  IO depths: 1=100.0%, 2=0.0%, 4=0.0%, 8=0.0%, 16=0.0%, 32=0.0%, >=64=0.0%
+ submit: 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
+ complete  : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
+ issued r/w: total=0/102400, short=0/0
+ lat (usec): 2=0.01%, 4=0.01%, 50=0.01%, 100=0.05%, 250=78.05%
+ lat (usec): 500=20.75%, 750=0.19%, 1000=0.36%
+ lat (msec): 2=0.53%, 4=0.02%, 10=0.02%, 20=0.01%, 50=0.02%
+ lat (msec): 250=0.01%
+
+Run status group 0 (all jobs):
+  WRITE: io=51,200KB, aggrb=1,739KB/s, minb=1,780KB/s, maxb=1,780KB/s, 
mint=29441msec, maxt=29441msec
+
+Disk stats (read/write):
+  dm-0: ios=0/102409, merge=0/0, ticks=0/30908, in_queue=30908, util=88.72%, 
aggrios=0/102435, aggrmerge=0/37, aggrticks=0/29427, aggrin_queue=29420, 
aggrutil=88.58%
+vda: ios=0/102435, merge=0/37, ticks=0/29427, in_queue=29420, util=88.58%
+test: (g=0): rw=write, bs=1K-1K/1K-1K, ioengine=libaio, iodepth=1
+Starting 1 process
+Jobs: 1 (f=1): [W] [100.0% done] [0K/3,289K /s] [0/3K iops] [eta 00m:00s]
+test: (groupid=0, jobs=1): err= 0: pid=3560
+  write: io=51,200KB, bw=3,482KB/s, iops=3,481, runt= 14705msec
+slat (usec): min=14, max=16,837, avg=22.62, stdev=92.91
+clat (usec): min=1, max=36,828, avg=261.91, stdev=418.40
+ lat (usec): min=214, max=38,143, avg=285.13, stdev=454.17
+bw (KB/s) : min= 3038, max= 3722, per=100.00%, avg=3481.03, stdev=219.52
+  cpu  : usr=1.86%, sys=9.47%, ctx=51325, majf=0, minf=23
+  IO depths: 1=100.0%, 2=0.0%, 4=0.0%, 8=0.0%, 16=0.0%, 32=0.0%, >=64=0.0%
+ submit: 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
+ complete  : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
+ issued r/w: total=0/51200, short=0/0
+ lat (usec): 2=0.01%, 4=0.02%, 100=0.06%, 250=78.20%, 500=20.52%
+ lat (usec): 750=0.23%, 1000=0.27%
+ lat (msec): 2=0.61%, 4=0.01%, 10=0.04%, 20=0.01%, 50=0.02%
+
+Run status group 0 (all jobs):
+  WRITE: io=51,200KB, aggrb=3,481KB/s, minb=3,565KB/s, maxb=3,565KB/s, 
mint=14705msec, maxt=14705msec
+
+Disk stats (read/write):
+  dm-0: ios=0/51037, merge=0/0, ticks=0/13460, in_queue=13460, util=88.34%, 
aggrios=0/51210, aggrmerge=0/17, aggrticks=0/13221, aggrin_queue=13219, 
aggrutil=88.14%
+vda: ios=0/51210, merge=0/17, ticks=0/13221, in_queue=13219, util=88.14%
+test: (g=0): rw=write, bs=2K-2K/2K-2K, ioengine=libaio, iodepth=1
+Starting 1 process
+Jobs: 1 (f=1): [W] [100.0% done] [0K/7,353K /s] [0/4K iops] [eta 00m:00s]
+test: (groupid=0, jobs=1): err= 0: pid=3567
+  write: io=51,200KB, bw=6,847KB/s, iops=3,423, runt=  7478msec
+slat (usec): min=14, max=16,906, avg=23.00, stdev=108.13
+clat (usec): min=1, max=34,159, avg=266.41, stdev=442.61
+ lat (usec): min=217, max=37,680, avg=290.00, stdev=484.19
+bw (KB/s) : min= 5948, max= 7280, per=99.65%, avg=6821.93, stdev=441.31
+  cpu  : usr=1.78%, sys=9.44%, ctx=25657, majf=0, minf=23
+  IO depths: 1=100.0%, 2=0.0%, 4=0.0%, 8=0.0%, 16=0.0%, 32=0.0%, >=64=0.0%
+ submit: 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
+ complete  : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
+ issued r/w: total=0/25600, short=0/0
+ lat (usec): 2=0.02%, 4=0.01%, 100=0.10%, 250=73.29%, 500=25.28%
+ lat (usec): 750=0.30%, 1000=0.29%
+ lat (msec): 2=0.60%, 4=0.02%, 10=0.06%, 50=0.02%
+
+Run status group 0 (all jobs):
+  WRITE: io=51,200KB, aggrb=6,846KB/s, minb=7,011KB/s, maxb=7,011KB/s, 
mint=7478msec, maxt=7478msec
+
+Disk stats (read/write):
+  dm-0: ios=0/25300, merge=0/0, ticks=0/6863, in_queue=6863, util=87.37%, 
aggrios=0/25604, aggrmerge=0/15, aggrticks=0/6678, aggrin_queue=6677, 
aggrutil=87.10%
+vda: ios=0/25604, merge=0/15, ticks=0/6678, in_queue=6677, 

Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?

[Thomas Schmitt]

Hi,

first i want to thank Paolo for his patience.

> So if you specified "-drive if=scsi" *and* "-cdrom", you'd get two non-empty
> drives.

Indeed. With
   -drive file=/dev/sr0,if=scsi,media=cdrom -cdrom /dvdbuffer/pseudo_drive
i get two drives with media:

  0  -dev '/dev/sr0' rwrw-- :  'QEMU' 'QEMU DVD-ROM' 
  1  -dev '/dev/sr1' rwrw-- :  'QEMU' 'QEMU CD-ROM' 

/dev/sr0 is from hw/ide/atapi.c:cmd_mode_sense()
  MODE SENSE
  5a 00 2a 00 00 00 00 00 1c 00 
  From drive: 28b
  00 22 70 00 00 00 00 00 2a 12 3b 00 71 60 2b 00 02 c0 00 02
  02 00 02 c0 00 00 00 00 

/dev/sr1 indeed differs in behavior
  MODE SENSE
  5a 00 2a 00 00 00 00 00 2d 00 
  From drive: 45b
  00 24 00 80 00 00 00 08 00 23 05 40 00 00 08 00 2a 14 3b 00
  7f ff 2f 00 22 60 00 02 08 00 0b 00 00 00 0b 00 0b 00 00 00
  00 00 00 00 00 

Which gives me something to work on. libburn does not take into
respect the Block Descriptor of 8 bytes which sits between
Mode Data Header and mode page.
So it misinterpets the result and demands the wrong Allocation
Length.

This error is explainable by my reading of MMC-5 which prescribes
in table 653:
  "Block Descriptor Length = 0"
MMC-1 on the other hand has in Annex B.4.1 :
  "The Mode Parameter Block Descriptor does not apply to ATAPI
   devices, and the Block Descriptor Length in the Mode
   Parameter Header shall be set to 0."

My thanks to qemu and the developers of its SCSI CD-ROM for showing
me this misconception in libburn.



If i do only -drive file=/dev/sr0,if=scsi,medium=cdrom i get
one empty drive, obviously from hw/ide/atapi.c:cmd_mode_sense():

  MODE SENSE
  5a 00 2a 00 00 00 00 00 1c 00 
  From drive: 28b
  00 22 70 00 00 00 00 00 2a 12 3b 00 71 60 29 00 02 c0 00 02
  02 00 02 c0 00 00 00 00 

So i mistook the default DVD-ROM drive, which has no source data and
thus is empty, for the CD-ROM drive which i expected from -drive if=scsi
which would not be empty, but does not show up at all.

The question remains, why the CD-ROM drive is missing if i do -drive
but not -cdrom.



I will repeat my experiments with
   -drive file=/dev/sg2,if=scsi,media=cdrom -cdrom /dvdbuffer/pseudo_drive

Maybe the presence of -cdrom cures the problems i had with passthrough.
(The bug in libburn is not to blame for that. The passthrough drive did not
 deliver a block descriptor.)



> Yeah, looks like all the
> 
> case MODE_PAGE_R_W_ERROR: /* error recovery */
> cpu_to_ube16(&buf[0], 16 + 6);
> should have "- 2" instead of "+ 6".

I came to the same conclusion.


The implementation of MODE SENSE(6) in hw/ide/atapi.c:cmd_mode_sense()
is wrong.

SPC-1, table 239 prescribes only 4 bytes of Mode Parameter Header,
with only one byte of Mode Data Length.

cmd_mode_sense() prepends 8 bytes unconditionally. Suitable only
for MODE SENSE(10).

Obviously nobody ever really needed a correct MODE SENSE(6) with
the emulated IDE CD-ROM of qemu.
MMC-1 allows MODE SENSE(6), MMC-3 implicitely assumes MODE SENSE(10) only,
MMC-5 prescribes to use MODE SENSE(10).

MMC-2 has a list of required ATAPI commands. MODE SENSE(10) is listed.
MODE SENSE(6) is not listed.


I compared this with the SCSI counterpart of cmd_mode_sense():
hw/scsi-disk.c:scsi_disk_emulate_mode_sense()

It distinguishes between MODE_SENSE, which gets 4 bytes of header,
and MODE_SENSE_10, which gets 8. This looks correct.



Have a nice day :)

Thomas

Re: [Qemu-devel] [PATCH 1/3] usb-hub: wakeup on attach

[hkran]

On 11/03/2011 04:41 PM, Gerd Hoffmann wrote:

On 11/03/11 07:01, hkran wrote:

On 11/02/2011 08:56 PM, Gerd Hoffmann wrote:

Hi,


static void usb_hub_detach(USBPort *port1)

pulled, In what cases, the usb hub will be suspended? and how to tell it
happened? thanks.

The guest enables the remote-wakeup feature.  'lspci -v' (within the
guest) shows it.

cheers,
Gerd


I use the param "-usb -usbdevice tablet" to start qemu  and then usb_del
tablet via monitor console after getting qemu up.
I saw this by entering lsusb -v

Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Device Descriptor:
   bLength18
   bDescriptorType 1
   bcdUSB   1.10
   bDeviceClass9 Hub
   bDeviceSubClass 0 Unused
   bDeviceProtocol 0 Full speed (or root) hub
   bMaxPacketSize064
   idVendor   0x1d6b Linux Foundation
   idProduct  0x0001 1.1 root hub
   bcdDevice2.06
   iManufacturer   3 Linux 2.6.32-131.0.15.el6.x86_64 uhci_hcd
   iProduct2 UHCI Host Controller
   iSerial 1 :00:01.2
..
Hub Descriptor:
   bLength   9
   bDescriptorType  41
   nNbrPorts 2
   wHubCharacteristic 0x000a
 No power switching (usb 1.0)
 Per-port overcurrent protection
   bPwrOn2PwrGood1 * 2 milli seconds
   bHubContrCurrent  0 milli Ampere
   DeviceRemovable0x00
   PortPwrCtrlMask0xff
  Hub Port Status:
Port 1: .0100 power
Port 2: .0100 power
Device Status: 0x0003
   Self Powered
   Remote Wakeup Enabled

(It seems that the Remote-wakeup feature has been enabled)
There are two ports with the status "power",  and I do not know how to
make it suspended yet. Can you help more ?

That is the root hub created by the linux kernel, not the hub emulated
by qemu.  If you hook up just a single device no hob is needed.

cheers,
   Gerd


Yes, it is. After I usb_add a storage a 8-port hub emulated is added :

lsusb
Bus 001 Device 004: ID :
Bus 001 Device 003: ID :
Bus 001 Device 002: ID 0627:0001 Adomax Technology Co., Ltd
Bus 001 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub

Device 003 is the hub and Device 004 is my storage

lsusb -s 3 -v:

Bus 001 Device 003: ID :
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   1.10
  bDeviceClass9 Hub
  bDeviceSubClass 0 Unused
  bDeviceProtocol 0 Full speed (or root) hub
  bMaxPacketSize0 8
  idVendor   0x
  idProduct  0x
  bcdDevice1.01
  iManufacturer   1 QEMU 0.15.50
  iProduct2 QEMU USB Hub
  iSerial 3 314159
  bNumConfigurations  1
...
iConfiguration  0
bmAttributes 0xe0
  Self Powered
  Remote Wakeup
MaxPower0mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 9 Hub
  bInterfaceSubClass  0 Unused
  bInterfaceProtocol  0 Full speed (or root) hub
  iInterface  0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0002  1x 2 bytes
bInterval 255
Hub Descriptor:
  bLength  10
  bDescriptorType  41
  nNbrPorts 8
  wHubCharacteristic 0x000a
No power switching (usb 1.0)
Per-port overcurrent protection
  bPwrOn2PwrGood1 * 2 milli seconds
  bHubContrCurrent  0 milli Ampere
  DeviceRemovable0x00 0x00
  PortPwrCtrlMask0xff 0x00
 Hub Port Status:
   Port 1: .0103 power enable connect
   Port 2: .0100 power
   Port 3: .0100 power
   Port 4: .0100 power
   Port 5: .0100 power
   Port 6: .0100 power
   Port 7: .0100 power
   Port 8: .0100 power
Device Status: 0x0001
  Self Powered

my storage is connected to the port 1.

after usb_del, all the hub port status will turn to power. how should i 
do next to get it to be suspended?thanks.

[Qemu-devel] [PATCH V3] Introduce a new bus "ICC" to connect APIC

[pingfank]

From: Liu Ping Fan 

Introduce a new structure CPUS as the controller of ICC (INTERRUPT
CONTROLLER COMMUNICATIONS), and new bus "ICC" to hold APIC,instead
of sysbus. So we can support APIC hot-plug feature.

Signed-off-by: liu ping fan 
---
 Makefile.target |1 +
 hw/apic.c   |   24 +
 hw/apic.h   |1 +
 hw/icc_bus.c|   92 +++
 hw/icc_bus.h|   61 +
 hw/pc.c |9 +++--
 hw/pc_piix.c|   14 +++-
 target-i386/cpu.h   |1 +
 target-i386/cpuid.c |   16 +
 9 files changed, 207 insertions(+), 12 deletions(-)
 create mode 100644 hw/icc_bus.c
 create mode 100644 hw/icc_bus.h

diff --git a/Makefile.target b/Makefile.target
index 9011f28..5607c6d 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 obj-i386-y += testdev.o
 obj-i386-y += acpi.o acpi_piix4.o
+obj-i386-y += icc_bus.o
 
 obj-i386-y += pcspk.o i8254.o
 obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
diff --git a/hw/apic.c b/hw/apic.c
index 69d6ac5..34fa1dd 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -21,9 +21,10 @@
 #include "ioapic.h"
 #include "qemu-timer.h"
 #include "host-utils.h"
-#include "sysbus.h"
+#include "icc_bus.h"
 #include "trace.h"
 #include "kvm.h"
+#include "exec-memory.h"
 
 /* APIC Local Vector Table */
 #define APIC_LVT_TIMER   0
@@ -80,7 +81,7 @@
 typedef struct APICState APICState;
 
 struct APICState {
-SysBusDevice busdev;
+ICCBusDevice busdev;
 MemoryRegion io_memory;
 void *cpu_env;
 uint32_t apicbase;
@@ -1104,9 +1105,19 @@ static const MemoryRegionOps apic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int apic_init1(SysBusDevice *dev)
+int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
 {
-APICState *s = FROM_SYSBUS(APICState, dev);
+APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
+
+memory_region_add_subregion(get_system_memory(),
+base,
+&s->io_memory);
+return 0;
+}
+
+static int apic_init1(ICCBusDevice *dev)
+{
+APICState *s = DO_UPCAST(APICState, busdev, dev);
 static int last_apic_idx;
 
 if (last_apic_idx >= MAX_APICS) {
@@ -1114,7 +1125,6 @@ static int apic_init1(SysBusDevice *dev)
 }
 memory_region_init_io(&s->io_memory, &apic_io_ops, s, "apic",
   MSI_ADDR_SIZE);
-sysbus_init_mmio_region(dev, &s->io_memory);
 
 s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
 s->idx = last_apic_idx++;
@@ -1122,7 +1132,7 @@ static int apic_init1(SysBusDevice *dev)
 return 0;
 }
 
-static SysBusDeviceInfo apic_info = {
+static ICCBusDeviceInfo apic_info = {
 .init = apic_init1,
 .qdev.name = "apic",
 .qdev.size = sizeof(APICState),
@@ -1138,7 +1148,7 @@ static SysBusDeviceInfo apic_info = {
 
 static void apic_register_devices(void)
 {
-sysbus_register_withprop(&apic_info);
+iccbus_register_devinfo(&apic_info);
 }
 
 device_init(apic_register_devices)
diff --git a/hw/apic.h b/hw/apic.h
index c857d52..e2c0af5 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
 uint8_t cpu_get_apic_tpr(DeviceState *s);
 void apic_init_reset(DeviceState *s);
 void apic_sipi(DeviceState *s);
+int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
 
 /* pc.c */
 int cpu_is_bsp(CPUState *env);
diff --git a/hw/icc_bus.c b/hw/icc_bus.c
new file mode 100644
index 000..ac88f2e
--- /dev/null
+++ b/hw/icc_bus.c
@@ -0,0 +1,92 @@
+/* icc_bus.c
+ * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+#include "icc_bus.h"
+
+static CPUSockets *cpu_sockets;
+
+static ICCBusInfo icc_bus_info = {
+.qinfo.name = "icc",
+.qinfo.size = sizeof(ICCBus),
+.qinfo.props = (Property[]) {
+DEFINE_PROP_END_OF_LIST(),
+}
+};
+
+static int iccbus_device_init(DeviceState *dev, DeviceInfo *base)
+{
+ICCBusDeviceInfo *info = container_of(base, ICCBusDeviceInfo, qdev);
+ICCBusDevice *idev = DO_UPCAST(ICCBusDevice, qdev, dev);
+
+return info->init(idev);
+}
+
+void iccbus_register_d

Re: [Qemu-devel] [PATCH 2/5] linux-user: add open() hijack infrastructure

[David Gilbert]

On 2 November 2011 19:23, Alexander Graf  wrote:
> There are a number of files in /proc that expose host information
> to the guest program. This patch adds infrastructure to override
> the open() syscall for guest programs to enable us to on the fly
> generate guest sensible files.
>
> Signed-off-by: Alexander Graf 
> ---
>  linux-user/syscall.c |   52 +++--
>  1 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9f5da36..38953ba 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4600,6 +4600,52 @@ int get_osversion(void)
>     return osversion;
>  }
>
> +static int do_open(void *cpu_env, const char *pathname, int flags, mode_t 
> mode)

Once you open the pandoras-box that is emulating /proc, I think you'll probably
need to hook it in more places and be more general; although you may
well get away
with it for this particular case.

Isn't it better to put the filename interception code somewhere more general
so that it can also be misused by other calls - e.g. stat().

I guess you're also going to need to be able to do /proc/pid/* instead
of /proc/self;
something is bound to use that.

Dave

Re: [Qemu-devel] GSoC mentor summit QEMU users session

[Fabien Chouteau]

On 03/11/2011 08:44, Stefan Hajnoczi wrote:
> On Wed, Nov 2, 2011 at 5:39 PM, Fabien Chouteau  wrote:
>> On 29/10/2011 15:52, Alexander Graf wrote:
>>> The RTEMS guys use QEMU to do coverage testing of their kernel code.
>>> They run their test-cases and see if all of their code and branches
>>> have been hit. Adacore seems to have a patches version of QEMU to
>>> provide an easily parsable log file for that sort of thing. Might be a
>>> good idea to consolidate upstream. Patches welcome :)
>>
>> That's right we do have a coverage analysis solution based on Qemu.
>> Execution traces generated by Qemu are analyzed by the GNATcoverage
>> tool to provide object, statement, decision or Modified
>> Condition/Decision coverage analysis.
>>
>> The main interest of Qemu is to provide execution traces without
>> code instrumentation.
>>
>> Alex, it is of course in our plans to submit patches to the upstream Qemu ;)
>>
>> I give you some pointers if you want to find more on GNATcoverage (technical 
>> stuff :)
>>
>>  * GNATcoverage is hosted on forge.open-do.org. You can checkout the
>>   repository with:
>>
>> $ svn checkout 
>> svn://scm.forge.open-do.org/scmrepos/svn/couverture/trunk/couverture
>>
>>   From there, you have
>> * the tool source tree in "tools"
>> * a synthesis article in "publications" : 201005-erts2.pdf
>>
>>  * GNATcov's documentation attached to this email
>
> Thanks for mentioning Couverture.  Just yesterday I talked to neo_1987
> on IRC who is trying to get QEMU /w couverture to work for him.

Great, good to know!

>  We ran into a little bit of confusion because the -trace command-line
>  option is also used in upstream QEMU but for a different purpose
>  (http://wiki.qemu.org/Features/Tracing).
>

We had the same problem, the option is now -exec-trace. Checkout the
qemu-head branch of 
https://forge.open-do.org/anonscm/git/couverture-qemu/couverture-qemu.git

> I took a quick peak at the qemu-trace.[ch] from couverture and it
> looks along the lines of the instrumentation that others have been
> doing too.  I hope you have time to propose the coverage
> instrumentation for upstream QEMU.
>

I don't know much about other instrumentations in Qemu (pointers are
welcome :), but what we have in couverture-qemu is not trivial,
especially when it comes to MC/DC analysis. You should take a look at
201005-erts2.pdf if you want technical details.

-- 
Fabien Chouteau

Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?

[Paolo Bonzini]

On 11/03/2011 10:15 AM, Thomas Schmitt wrote:

Which gives me something to work on. libburn does not take into
respect the Block Descriptor of 8 bytes which sits between
Mode Data Header and mode page.
So it misinterpets the result and demands the wrong Allocation
Length.

This error is explainable by my reading of MMC-5 which prescribes
in table 653:
   "Block Descriptor Length = 0"
MMC-1 on the other hand has in Annex B.4.1 :
   "The Mode Parameter Block Descriptor does not apply to ATAPI
devices, and the Block Descriptor Length in the Mode
Parameter Header shall be set to 0."

My thanks to qemu and the developers of its SCSI CD-ROM for showing
me this misconception in libburn.


MMC-3 also has some text saying that for compatibility you can have 
block descriptors.





If i do only -drive file=/dev/sr0,if=scsi,medium=cdrom i get
one empty drive, obviously from hw/ide/atapi.c:cmd_mode_sense():

   MODE SENSE
   5a 00 2a 00 00 00 00 00 1c 00
   From drive: 28b
   00 22 70 00 00 00 00 00 2a 12 3b 00 71 60 29 00 02 c0 00 02
   02 00 02 c0 00 00 00 00

So i mistook the default DVD-ROM drive, which has no source data and
thus is empty, for the CD-ROM drive which i expected from -drive if=scsi
which would not be empty, but does not show up at all.

The question remains, why the CD-ROM drive is missing if i do -drive
but not -cdrom.


Ok, so we have something to test now. :)


I will repeat my experiments with
-drive file=/dev/sg2,if=scsi,media=cdrom -cdrom /dvdbuffer/pseudo_drive

Maybe the presence of -cdrom cures the problems i had with passthrough.
(The bug in libburn is not to blame for that. The passthrough drive did not
  deliver a block descriptor.)


Yes, otherwise it wouldn't be passthrough. :)





Yeah, looks like all the

 case MODE_PAGE_R_W_ERROR: /* error recovery */
 cpu_to_ube16(&buf[0], 16 + 6);
should have "- 2" instead of "+ 6".


I came to the same conclusion.


Good.


MMC-1 allows MODE SENSE(6), MMC-3 implicitely assumes MODE SENSE(10) only,
MMC-5 prescribes to use MODE SENSE(10).

MMC-2 has a list of required ATAPI commands. MODE SENSE(10) is listed.
MODE SENSE(6) is not listed.


I compared this with the SCSI counterpart of cmd_mode_sense():
hw/scsi-disk.c:scsi_disk_emulate_mode_sense()

It distinguishes between MODE_SENSE, which gets 4 bytes of header,
and MODE_SENSE_10, which gets 8. This looks correct.


Yep.  Seems easiest to just drop MODE SENSE(6) from ATAPI.

Paolo

Re: [Qemu-devel] Multiple instances of Qemu on Windows multicore

[Fabien Chouteau]

On 02/11/2011 20:52, Paolo Bonzini wrote:
> On 11/02/2011 07:01 PM, Peter Maydell wrote:
>> On 2 November 2011 17:45, Paolo Bonzini  wrote:
>>> The rest is always done in the iothread.  The iothread will then
>>> suspend/resume the VCPU thread around the unchaining, so what matters is (in
>>> Unix parlance) signal-safety of the unchaining, not thread-safety.
>>
>> The unchaining is neither signal-safe nor thread-safe...
> 
> Yeah, but there's nothing Windows-specific in that.

That's very important, I don't see why it is different between Linux and
Windows here. Also, why running all the threads on the same CPU would
make the code thread-safe?

-- 
Fabien Chouteau

Re: [Qemu-devel] Multiple instances of Qemu on Windows multicore

[Fabien Chouteau]

On 02/11/2011 20:57, Peter Maydell wrote:
> On 2 November 2011 19:52, Paolo Bonzini  wrote:
>> (Also, the unchaining is safer, or even completely safe in system mode than
>> it is with pthreads).
> 
> I don't think it's completely safe, you're just a bit less likely
> to get bitten than if you're trying to run a linux-user-mode
> multithreaded guest binary.

Do you think it's possible to make the code safe?

-- 
Fabien Chouteau

Re: [Qemu-devel] Multiple instances of Qemu on Windows multicore

[Paolo Bonzini]

On 11/03/2011 10:54 AM, Fabien Chouteau wrote:

>>  The unchaining is neither signal-safe nor thread-safe...

>
>  Yeah, but there's nothing Windows-specific in that.

That's very important, I don't see why it is different between Linux and
Windows here.


Yep, perhaps for timers it was the case a while ago, but with 
iothread+dynticks it should not be a problem anymore.  For unchaining, 
Linux and Windows should have never been different.



Also, why running all the threads on the same CPU would
make the code thread-safe?


It would ensure that two mutators wouldn't run concurrently.  In some 
sense, signal-safe code could then be considered thread-safe too.


Paolo

[Qemu-devel] [PULL v3 00/29] VMState port of all cpus

[Juan Quintela]

The following changes since commit 932eacc158c064935c7bab920c88a93a629e1ca4:

  Merge branch 'xtensa' of git://jcmvbkbc.spb.ru/dumb/qemu-xtensa (2011-11-02 
20:52:23 +)

are available in the git repository at:

  git://repo.or.cz/qemu/quintela.git vmstate-cpus-for-anthony

[v3]
- rebase to top
- fix sparc/arm/i386 changes in upstream
- all reviews were positive, Anthony, please pull

[v2] Changes since v1

- preserve arm comment that was missing (pbrook)
- add copyright notice to the files that were empty
- new patches:
  * fix formating for i386
  * remove unneeded includes
  * rename machine.c to vmstate.c

Later, Juan.

[v1]

This series port all cpus to use vmstate.
- 1st patch is a fix of vmstate.
- I discussed the arm changes over irc with Peter, he agreed that some
  simplification could be good, but he didn't saw the patches O:-)
- mips: no pci chipset has been ported, so migration don't work there.
  I have embedded a couple of structs to improve vmstate checking.  Notice
  that they were always allocated, so there shouldn't be any problem.
- sparc: I changed the format a little bit to be able to use normal arrays.
- sparc: If we always send the whole register windows, we don't need
  VMSTATE_VARRAY_MULTIPLY.  As that array is quite big (520 elements), I am not
  sure what is best.
- cpsr_vmstate on arm: I am not sure if I could "abuse" uncached_cpsr for that
  purpose?

I have only tested on x86, for the rest, I double checked, but it is
possible that I missed something.  I expect all patches to be
integrated by Anthony in one go.  Architecture maintainers are CC'd
for an ACK/NACK/comments.

Please, review.

PD. Is there an easy way of creating this "CC" list of mail addresses,
or the only way is to edit comments and write it by hand as I did?


Juan Quintela (29):
  vmstate: Fix VMSTATE_VARRAY_UINT32
  vmstate: Simplify test for CPU_SAVE_VERSION
  vmstate: make all architectures export a way to migrate cpu's
  vmstate: unicore32 don't support cpu migration
  vmstate: use new cpu style for x86
  vmstate: use new style for lm32 cpus
  vmstate: make microblaze cpus not migrateable
  vmstate: port cris cpu to vmstate
  vmstate: machine.c is only compiled for !CONFIG_USER_ONLY
  vmstate: introduce float32 arrays
  vmstate: introduce float64 arrays
  vmstate: introduce CPU_DoubleU arrays
  vmstate: Introduce VMSTATE_STRUCT_VARRAY_INT32_TEST
  vmstate: port ppc cpu
  vmstate: introduce VMSTATE_VARRAY_MULTIPLY
  vmstate: define vmstate_info_uinttls
  vmstate: port sparc cpu
  vmstate: make incompatible change for sparc
  mips_fulong2e: cpu vmstate already registered in cpu_exec_init
  mips: make mvp an embedded struct instead of pointer
  mips: make tlb an embedded struct instead of a pointer
  mips: bump migration version to 4
  vmstate: port mips cpu
  arm: save always 32 fpu registers
  vmstate: port arm cpu
  vmstate: all cpus converted
  vmstate: fix vmstate formating for i386
  vmstate: remove unneeded includes from target-*/machine.c
  vmstate: rename machine.c to vmstate-cpu.c

 Makefile.target|3 +-
 exec.c |7 +-
 hw/hw.h|   62 +-
 hw/mips_fulong2e.c |1 -
 hw/mips_malta.c|4 +-
 hw/mips_timer.c|2 +-
 hw/sun4u.c |   20 --
 qemu-common.h  |4 -
 savevm.c   |   92 +
 target-alpha/{machine.c => vmstate-cpu.c}  |   13 +-
 target-arm/cpu.h   |5 +-
 target-arm/machine.c   |  225 
 target-arm/vmstate-cpu.c   |  173 
 target-cris/cpu.h  |   13 +-
 target-cris/machine.c  |   90 
 target-cris/vmstate-cpu.c  |   59 ++
 target-i386/cpu.h  |2 -
 target-i386/{machine.c => vmstate-cpu.c}   |   36 +---
 target-lm32/cpu.h  |2 -
 target-lm32/{machine.c => vmstate-cpu.c}   |   17 +--
 target-m68k/vmstate-cpu.c  |   21 ++
 target-microblaze/cpu.h|2 -
 target-microblaze/machine.c|   11 -
 target-microblaze/vmstate-cpu.c|   21 ++
 target-mips/cpu.h  |   11 +-
 target-mips/helper.c   |   30 ++-
 target-mips/machine.c  |  308 
 target-mips/op_helper.c|   70 ---
 target-mips/translate.c|   22 ++-
 target-mips/translate_init.c   |   36 ++--
 target-mips/vmstate-cpu.c  |  196 ++
 target-ppc/cpu.h   |5 +-
 t

Re: [Qemu-devel] Multiple instances of Qemu on Windows multicore

[Avi Kivity]

On 11/03/2011 12:10 PM, Paolo Bonzini wrote:
>
>> Also, why running all the threads on the same CPU would
>> make the code thread-safe?
>
> It would ensure that two mutators wouldn't run concurrently.  In some
> sense, signal-safe code could then be considered thread-safe too.
>

How so?  The scheduler can switch between the two threads on every
instruction.

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

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Avi Kivity]

On 11/02/2011 12:20 AM, Anthony Liguori wrote:
>
> Seems harmless for QEMU, so applied.  You should update the virtio-pci
> spec too.

Should be the other way round.

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

Re: [Qemu-devel] [PATCH 0/5] linux-user: fake some /proc/self entries

[Riku Voipio]

On Wed, Nov 02, 2011 at 08:23:21PM +0100, Alexander Graf wrote:
> When running linux-user programs in QEMU, the guest program can examine
> itself by checking /proc/self/ files. And some libraries really do use
> this!
> 
> Unfortunately, when checking /proc/self/ today, the guest program sees
> the QEMU files, exposing wrong information to the guest program.
 
> This patch set fakes auxv, maps and stat to make gtk, pthread and boehm
> gc happy.

This is awesome stuff, but unfortunately just a day after 1.0 hard freeze :(
David pointed out that more files in proc could be done, but this is a good 
start.

Riku
 
> Alex
> 
> Alexander Graf (5):
>   linux-user: save auxv length
>   linux-user: add open() hijack infrastructure
>   linux-user: fake /proc/self/maps
>   linux-user: fake /proc/self/stat
>   linux-user: fake /proc/self/auxv
> 
>  linux-user/elfload.c |   15 ++-
>  linux-user/qemu.h|1 +
>  linux-user/syscall.c |  123 -
>  3 files changed, 125 insertions(+), 14 deletions(-)
> 

Re: [Qemu-devel] [PATCH (repost) RFC 2/2] virtio-pci: recall and return msix notifications on ISR read

[Jan Kiszka]

On 2011-11-02 21:11, Michael S. Tsirkin wrote:
> MSIX spec requires that device can be operated with
> all vectors masked, by polling pending bits.
> Add APIs to recall an msix notification, and make polling
> mode possible in virtio-pci by clearing the
> pending bits and setting ISR appropriately on ISR read.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/msix.c   |   26 ++
>  hw/msix.h   |3 +++
>  hw/virtio-pci.c |   11 ++-
>  3 files changed, 39 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 63b41b9..fe967c9 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -349,6 +349,32 @@ void msix_notify(PCIDevice *dev, unsigned vector)
>  stl_le_phys(address, data);
>  }
>  
> +/* Recall outstanding MSI-X notifications for a vector, if possible.
> + * Return true if any were outstanding. */
> +bool msix_recall(PCIDevice *dev, unsigned vector)
> +{
> +bool ret;
> +if (vector >= dev->msix_entries_nr)
> +return false;
> +ret = msix_is_pending(dev, vector);
> +msix_clr_pending(dev, vector);
> +return ret;
> +}

I would prefer to have a single API instead to clarify the tight relation:

bool msi[x]_set_notify(PCIDevice *dev, unsigned vector, unsigned level)

Would return true for level=1 if the message was either sent directly or
queued (we could deliver false if it was already queued, but I see no
use case for this yet).

Also, I don't see the generic value of some msix_recall_all. I think
it's better handled in a single loop over all vectors at caller site,
clearing the individual interrupt reason bits on a per-vector basis
there. msix_recall_all is only useful in the virtio case where you have
one vector of reason A and all the rest of B. Once you had multiple
reason C vectors as well, it would not help anymore.

Jan

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

Re: [Qemu-devel] Multiple instances of Qemu on Windows multicore

[Paolo Bonzini]

On 11/03/2011 11:29 AM, Avi Kivity wrote:

>  It would ensure that two mutators wouldn't run concurrently.  In some
>  sense, signal-safe code could then be considered thread-safe too.

How so?  The scheduler can switch between the two threads on every
instruction.


In general signal-safe is more stringent than thread-safe, but with two 
exceptions: memory barriers and locked memory access.  On x86 (implied 
by Windows...) you might also assume that the compiler will generate 
arithmetic operations with a memory destination, which makes code like


void cpu_interrupt(CPUState *env, int mask)
{
env->interrupt_request |= mask;  /* <--- this */
cpu_unlink_tb(env);
}

signal-safe in practice---and even "thread-safe" on non-SMP systems. 
It's a huge assumption though, and I don't think it should be assumed 
anymore.  With iothread the architecture of the QEMU main loop is anyway 
completely different.


Paolo

Re: [Qemu-devel] [PATCH (repost) RFC 2/2] virtio-pci: recall and return msix notifications on ISR read

[Michael S. Tsirkin]

On Thu, Nov 03, 2011 at 12:42:55PM +0100, Jan Kiszka wrote:
> On 2011-11-02 21:11, Michael S. Tsirkin wrote:
> > MSIX spec requires that device can be operated with
> > all vectors masked, by polling pending bits.
> > Add APIs to recall an msix notification, and make polling
> > mode possible in virtio-pci by clearing the
> > pending bits and setting ISR appropriately on ISR read.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  hw/msix.c   |   26 ++
> >  hw/msix.h   |3 +++
> >  hw/virtio-pci.c |   11 ++-
> >  3 files changed, 39 insertions(+), 1 deletions(-)
> > 
> > diff --git a/hw/msix.c b/hw/msix.c
> > index 63b41b9..fe967c9 100644
> > --- a/hw/msix.c
> > +++ b/hw/msix.c
> > @@ -349,6 +349,32 @@ void msix_notify(PCIDevice *dev, unsigned vector)
> >  stl_le_phys(address, data);
> >  }
> >  
> > +/* Recall outstanding MSI-X notifications for a vector, if possible.
> > + * Return true if any were outstanding. */
> > +bool msix_recall(PCIDevice *dev, unsigned vector)
> > +{
> > +bool ret;
> > +if (vector >= dev->msix_entries_nr)
> > +return false;
> > +ret = msix_is_pending(dev, vector);
> > +msix_clr_pending(dev, vector);
> > +return ret;
> > +}
> 
> I would prefer to have a single API instead to clarify the tight relation:
> 
> bool msi[x]_set_notify(PCIDevice *dev, unsigned vector, unsigned level)
> 
> Would return true for level=1 if the message was either sent directly or
> queued (we could deliver false if it was already queued, but I see no
> use case for this yet).

It's a matter of taste: some people like functions with flags, some
prefer separate functions.  I really prefer two functions.

But I agree it woulkd be better to have a name that makes it clear that
what we recall is a notification.
msix_notify_queue/msix_notify_dequeue?


> Also, I don't see the generic value of some msix_recall_all. I think
> it's better handled in a single loop over all vectors at caller site,
> clearing the individual interrupt reason bits on a per-vector basis
> there. msix_recall_all is only useful in the virtio case where you have
> one vector of reason A and all the rest of B. Once you had multiple
> reason C vectors as well, it would not help anymore.
> 
> Jan

The reason I wanted to have it is to reduce the overhead this adds:
since PBA is packed, it's much faster to check whether any bits are set
than by going through them all, one by one. Typically all PBA
bits are clear ...

I agree it might not help non-virtio devices, but to me it looks like a
harmless little helper - what's the issue with it?

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

Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions

[Michael S. Tsirkin]

On Thu, Nov 03, 2011 at 04:16:34PM +1100, David Gibson wrote:
> On Wed, Nov 02, 2011 at 09:16:34AM +0200, Michael S. Tsirkin wrote:
> > On Mon, Oct 31, 2011 at 05:06:49PM +1100, David Gibson wrote:
> > > From: Eduard - Gabriel Munteanu 
> [snip]
> > > @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
> > >   * values which really matter.
> > >   * Number of data should check configuration!!!
> > >   */
> > > -cpu_physical_memory_write(s->statsaddr, &s->statistics, 
> > > s->stats_size);
> > > -e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
> > > -e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
> > > -e100_stl_le_phys(s->statsaddr + 48, 
> > > s->statistics.rx_resource_errors);
> > > -e100_stl_le_phys(s->statsaddr + 60, 
> > > s->statistics.rx_short_frame_errors);
> > > +pci_dma_write(&s->dev, s->statsaddr,
> > > +  (uint8_t *) &s->statistics, s->stats_size);
> > > +stl_le_pci_dma(&s->dev, s->statsaddr + 0,
> > > +   s->statistics.tx_good_frames);
> > > +stl_le_pci_dma(&s->dev, s->statsaddr + 36,
> > > +   s->statistics.rx_good_frames);
> > > +stl_le_pci_dma(&s->dev, s->statsaddr + 48,
> > > +   s->statistics.rx_resource_errors);
> > > +stl_le_pci_dma(&s->dev, s->statsaddr + 60,
> > > +   s->statistics.rx_short_frame_errors);
> > 
> > This might introduce a bug: stlXX APIs assume aligned addresses,
> > an address in statsaddr is user-controlled so I'm not sure
> > it's always aligned.
> > 
> > Why isn't the patch simply replacing cpu_physical_memory_read
> > with pci_XXX ? Any cleanups should be done separately.
> 
> Because it seemed like a good idea at the time.  When I first wrote
> this, the possibility of unaligned addresses wasn't obvious to me.
> So, I'm working on fixing this now.  I can take one of two approaches:
> 
>  - Simply revert this part of the change, reinstate the e100_stl
> functions as calling into dma_write().
> 
>  - Alter the stX_dma() functions to work for unaligned addresses (by
> falling back to dma_rw() in that case).  This is a little more
> involved but might make device writing safer in future.

Yes but then we lose the atomicity guarantee. So this might
still result in subtle emulation bugs.

> Anthony, Michael, any preferred direction here?

For 1.0 I'd go for option 1 as the simplest.

> -- 
> David Gibson  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au| minimalist, thank you.  NOT _the_ 
> _other_
>   | _way_ _around_!
> http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [PULL v3 00/29] VMState port of all cpus

[Anthony Liguori]

On 11/03/2011 05:26 AM, Juan Quintela wrote:

The following changes since commit 932eacc158c064935c7bab920c88a93a629e1ca4:

   Merge branch 'xtensa' of git://jcmvbkbc.spb.ru/dumb/qemu-xtensa (2011-11-02 
20:52:23 +)

are available in the git repository at:

   git://repo.or.cz/qemu/quintela.git vmstate-cpus-for-anthony

[v3]
- rebase to top
- fix sparc/arm/i386 changes in upstream
- all reviews were positive, Anthony, please pull

[v2] Changes since v1

- preserve arm comment that was missing (pbrook)
- add copyright notice to the files that were empty
- new patches:
   * fix formating for i386
   * remove unneeded includes
   * rename machine.c to vmstate.c

Later, Juan.


Hi Juan,

This will need to wait until 1.1 opens up.

Regards,

Anthony Liguori



[v1]

This series port all cpus to use vmstate.
- 1st patch is a fix of vmstate.
- I discussed the arm changes over irc with Peter, he agreed that some
   simplification could be good, but he didn't saw the patches O:-)
- mips: no pci chipset has been ported, so migration don't work there.
   I have embedded a couple of structs to improve vmstate checking.  Notice
   that they were always allocated, so there shouldn't be any problem.
- sparc: I changed the format a little bit to be able to use normal arrays.
- sparc: If we always send the whole register windows, we don't need
   VMSTATE_VARRAY_MULTIPLY.  As that array is quite big (520 elements), I am not
   sure what is best.
- cpsr_vmstate on arm: I am not sure if I could "abuse" uncached_cpsr for that
   purpose?

I have only tested on x86, for the rest, I double checked, but it is
possible that I missed something.  I expect all patches to be
integrated by Anthony in one go.  Architecture maintainers are CC'd
for an ACK/NACK/comments.

Please, review.

PD. Is there an easy way of creating this "CC" list of mail addresses,
 or the only way is to edit comments and write it by hand as I did?


Juan Quintela (29):
   vmstate: Fix VMSTATE_VARRAY_UINT32
   vmstate: Simplify test for CPU_SAVE_VERSION
   vmstate: make all architectures export a way to migrate cpu's
   vmstate: unicore32 don't support cpu migration
   vmstate: use new cpu style for x86
   vmstate: use new style for lm32 cpus
   vmstate: make microblaze cpus not migrateable
   vmstate: port cris cpu to vmstate
   vmstate: machine.c is only compiled for !CONFIG_USER_ONLY
   vmstate: introduce float32 arrays
   vmstate: introduce float64 arrays
   vmstate: introduce CPU_DoubleU arrays
   vmstate: Introduce VMSTATE_STRUCT_VARRAY_INT32_TEST
   vmstate: port ppc cpu
   vmstate: introduce VMSTATE_VARRAY_MULTIPLY
   vmstate: define vmstate_info_uinttls
   vmstate: port sparc cpu
   vmstate: make incompatible change for sparc
   mips_fulong2e: cpu vmstate already registered in cpu_exec_init
   mips: make mvp an embedded struct instead of pointer
   mips: make tlb an embedded struct instead of a pointer
   mips: bump migration version to 4
   vmstate: port mips cpu
   arm: save always 32 fpu registers
   vmstate: port arm cpu
   vmstate: all cpus converted
   vmstate: fix vmstate formating for i386
   vmstate: remove unneeded includes from target-*/machine.c
   vmstate: rename machine.c to vmstate-cpu.c

  Makefile.target|3 +-
  exec.c |7 +-
  hw/hw.h|   62 +-
  hw/mips_fulong2e.c |1 -
  hw/mips_malta.c|4 +-
  hw/mips_timer.c|2 +-
  hw/sun4u.c |   20 --
  qemu-common.h  |4 -
  savevm.c   |   92 +
  target-alpha/{machine.c =>  vmstate-cpu.c}  |   13 +-
  target-arm/cpu.h   |5 +-
  target-arm/machine.c   |  225 
  target-arm/vmstate-cpu.c   |  173 
  target-cris/cpu.h  |   13 +-
  target-cris/machine.c  |   90 
  target-cris/vmstate-cpu.c  |   59 ++
  target-i386/cpu.h  |2 -
  target-i386/{machine.c =>  vmstate-cpu.c}   |   36 +---
  target-lm32/cpu.h  |2 -
  target-lm32/{machine.c =>  vmstate-cpu.c}   |   17 +--
  target-m68k/vmstate-cpu.c  |   21 ++
  target-microblaze/cpu.h|2 -
  target-microblaze/machine.c|   11 -
  target-microblaze/vmstate-cpu.c|   21 ++
  target-mips/cpu.h  |   11 +-
  target-mips/helper.c   |   30 ++-
  target-mips/machine.c  |  308 
  target-mips/op_helper.c|   70 ---
  target-mips/translate.c 

[Qemu-devel] [PATCH] os-posix: call system_powerdown on SIGTERM, enabling clean guest shutdown

[Bjørn Mork]

Allow well behaved guests to shutdown cleanly when we receive SIGTERM,
e.g. when the host reboots.

The host may be powered down or rebooted while guests are running
without any outer supervision scripts.  These guests will be stopped
by the generic host "sendsigs" script, usually by sending SIGTERM to all
processes still running and then SIGKILL if that didn't work.  Let the
guest power down instead of just killing i instantly.

Signed-off-by: Bjørn Mork 
---
I wonder if I miss something obvious here... So this patch should be
considered a RFC.  It certainly fixes a small problem for me:  I often
run a number of guests which are started manually, or by simple "runme"
scripts, without any libvirt wrapper or similar.  If/when the host is
rebooted, these guests will be shut down abrubtly with possible guest
file system corruption and other data loss as a consequence. Powering
down on SIGTERM seems like an obious win-win.

I also wonder if it would be useful to do a reboot on HUP, but that
wouldn't solve any real problem so I'm not including it in this 
patch.



Bjørn

 os-posix.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index dbf3b24..167fd3b 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -65,7 +65,10 @@ void os_setup_early_signal_handling(void)
 
 static void termsig_handler(int signal, siginfo_t *info, void *c)
 {
-qemu_system_killed(info->si_signo, info->si_pid);
+if (info->si_signo == SIGTERM)
+   qemu_system_powerdown_request();
+else
+   qemu_system_killed(info->si_signo, info->si_pid);
 }
 
 void os_setup_signal_handling(void)
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Anthony Liguori]

On 11/03/2011 05:36 AM, Avi Kivity wrote:

On 11/02/2011 12:20 AM, Anthony Liguori wrote:


Seems harmless for QEMU, so applied.  You should update the virtio-pci
spec too.


Should be the other way round.


Am not entirely sure.  Having worked code that's been reviewed will make for a 
better spec.  Writing the spec and committing to the spec change before getting 
either side of the implementation merged seems to be asking for trouble to me.


We could use a better agreement on the processor for making virtio changes. 
Should it go (1) virtio spec (2) kernel (3) qemu, or should it go (2), (1), (3)?


Regards,

Anthony Liguori

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Anthony Liguori]

On 11/02/2011 05:10 AM, Avi Kivity wrote:

On 11/01/2011 08:08 PM, Anthony Liguori wrote:

On 10/30/2011 09:02 AM, Avi Kivity wrote:

This somewhat controversial patchset converts internal arithmetic in the
memory API to 128 bits.

It has been argued that with careful coding we can make 64-bit work as
well.  I don't think this is true in general - a memory router can
adjust
addresses either forwards or backwards, and some buses (PCIe) need the
full 64-bit space - though it's probably the case for all the
configurations
we support today.  Regardless, the need for careful coding means
subtle bugs,
which I don't want in a core API that is driven by guest supplied
values.

Avi Kivity (3):
Add support for 128-bit arithmetic
memory: use 128-bit integers for sizes and intermediates
Adjust system and pci address spaces to full 64-bit


I now notice that this is not a PULL request...  Did you mess up the
subject or the pull request?



I forgot to supply the pull info, which is:

   git://github.com/avikivity/qemu.git memory/int128


Pulled.  Thanks.

Regards,

Anthony Liguori

Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?

[Thomas Schmitt]

Hi,

i repeated my tests with -drive and -cdrom in the same qemu run:

  ...absolute.path.../x86_64-softmmu/qemu-system-x86_64 \
 -enable-kvm \
 -L ...absolute.path.../pc-bios \
 -nographic \
 -m 512 \
 -net nic,model=ne2k_pci \
 -net user,hostfwd=tcp::5557-:22 \
 -hda /dvdbuffer/i386-install.qemu \
 -drive file=/dev/sg1,if=scsi,media=cdrom \
 -cdrom /dvdbuffer/pseudo_drive

xorriso lists two drives

  0  -dev '/dev/sr0' rwrw-- :  'QEMU' 'QEMU DVD-ROM' 
  1  -dev '/dev/sr1' rwrw-- :  'TSSTcorp' 'CDDVDW SH-S223B' 

Both hold a medium now

  Drive current: -dev '/dev/sr0'
  Drive type   : vendor 'QEMU' product 'QEMU DVD-ROM' revision '0.15'
  Media current: CD-ROM
  Media status : is written , is closed
  Media summary: 1 session, 109597 data blocks,  214m data, 0 free

  Drive current: -dev '/dev/sr1'
  Drive type   : vendor 'TSSTcorp' product 'CDDVDW SH-S223B' revision 'SB02'
  Media current: DVD+RW
  Media status : is written , is appendable
  Media summary: 1 session, 16771 data blocks, 32.8m data, 4450m free

(The status "appendable" of DVD+RW is an emulation of xorriso,
 not the result of drive replies.)

The other failures remain, i fear:

Timeouts with
  READ DISC STRUCTURE
  ad 00 00 00 00 00 00 04 00 04 00 00 
  READ DISC STRUCTURE
  ad 00 00 00 00 00 00 11 00 04 00 00
but not with
  READ DISC STRUCTURE 
  ad 00 00 00 00 00 00 00 00 04 00 00  
 
I still get sense code B 00 06 I/O PROCESS TERMINATED with
  PREVENT/ALLOW MEDIA REMOVAL
  1e 00 00 00 01 00
  MODE SELECT 
  55 10 00 00 00 00 00 00 3c 00
  SET STREAMING
  b6 00 00 00 00 00 00 00 00 00 1c 00 
  SET CD SPEED
  bb 00 ff ff 06 e4 00 00 00 00 00 00
  WRITE(10)
  2a 00 00 00 99 0f 00 00 10 00
  SYNCHRONIZE CACHE
  35 02 00 00 00 00 00 00 00 00
  CLOSE TRACK/SESSION
  5b 01 02 00 00 00 00 00 00 00

Good news:
Other than with qemu-0.12.5, the command
  SET STREAMING
  b6 00 00 00 00 00 00 00 00 00 1c 00 
  To drive: 28b
  00 00 00 00 00 00 00 00 00 23 04 88 10 00 00 00 00 00 03 e8
  10 00 00 00 00 00 03 e8 
does no crash qemu but only throws B 00 06.
I am using it with -enable-kvm now, to accelerate booting and login.


Do you have a proposal what i should try next ?
(Else i will try to find the code which throws B 00 06.)

Please give me a note, when there are improvements to test.
(Do i get it right, that "git pull" will update my local clone ?
 In my few encounters with git, "git clone" was all i needed.)


---

I have a (weak) argument for making the ATAPI mode page 2A compliant
with MMC-1:
'SanDisk' 'Cruzer', an emulated CD-ROM in a memory stick, throws
random errors if i ask it for 28 bytes rather than the 30 of MMC-1.

Its U3 CD-ROM emulation is not of much importance, as it only
serves to store MS-Windows auto-executables.
Nevertheless, it serves me as example of a poorly programmed
firmware.

I now have to bet on the fact that qemu guest operating systems
tolerate with ATAPI that i request 30 bytes and the drive delivers
only 28.
The only occasion, where i am aware of this assumption to fail, is
Linux 2.4 with USB attached drives.
ATAPI via ide-scsi kernel module did tolerate oversized requests
back in 2007. Since then i did not challenge OSes that way.

---

Have a nice day :)

Thomas

Re: [Qemu-devel] [PATCH v4 1/4] Add basic version of bridge helper

[Corey Bryant]

On 11/03/2011 03:54 AM, Stefan Hajnoczi wrote:

On Wed, Nov 2, 2011 at 3:12 PM, Corey Bryant  wrote:

On 11/02/2011 06:58 AM, Stefan Hajnoczi wrote:


On Tue, Nov 1, 2011 at 5:13 PM, Corey Bryant
  wrote:


+static bool has_vnet_hdr(int fd)
+{
+unsigned int features = 0;
+struct ifreq ifreq;
+
+if (ioctl(fd, TUNGETFEATURES,&features) == -1) {
+return false;
+}
+
+if (!(features&IFF_VNET_HDR)) {
+return false;
+}
+
+if (ioctl(fd, TUNGETIFF,&ifreq) != -1 || errno != EBADFD) {
+return false;
+}


I don't understand this expression.  We want TUNGETIFF to fail with
EBADFD, otherwise we return false.  What is this trying to do?

Why do we even need TUNGETIFF after TUNGETFEATURES succeeded and we
were able to check out the IFF_VNET_HDR flag?

Stefan



I don't think the TUNGETIFF call is necessary.  It verifies that the tap
device doesn't already exist.  The ensuing TUNSETIFF call should already
cover this.


Thanks for explaining.  It would be nice to comment this or drop the
if statement entirely if you are sending another revision of this
patch series.

Stefan



You're welcome, and thanks for the comment.  I'll drop this if statement 
in the next version of the patch series.


Regards,
Corey

Re: [Qemu-devel] [PULL] Virtfs update

[Anthony Liguori]

On 11/02/2011 09:09 AM, Aneesh Kumar K.V wrote:

On Wed, 02 Nov 2011 07:42:16 -0500, Anthony Liguori  wrote:

On 11/02/2011 05:22 AM, Aneesh Kumar K.V wrote:


The following changes since commit e072ea2fd8fdceef64159b9596d3c15ce01bea91:

Bump version to 1.0-rc0 (2011-11-01 19:37:01 -0500)

are available in the git repository at:
git://repo.or.cz/qemu/v9fs.git for-upstream-8

Aneesh Kumar K.V (1):
hw/9pfs: Move opt validation to FsDriver callback

Stefan Hajnoczi (1):
hw/9pfs: use g_vasprintf() instead of rolling our own


Neither of these look like bug fixes to me.

Regards,



The related discussion for the first commit can be found at

http://article.gmane.org/gmane.comp.emulators.qemu/122891

I dropped the second commit. updated details below.


Pulled.  Thanks.

Regards,

Anthony Liguori




The following changes since commit e072ea2fd8fdceef64159b9596d3c15ce01bea91:

   Bump version to 1.0-rc0 (2011-11-01 19:37:01 -0500)

are available in the git repository at:
   git://repo.or.cz/qemu/v9fs.git for-upstream-8

Stefan Hajnoczi (1):
   hw/9pfs: use g_vasprintf() instead of rolling our own

  hw/9pfs/virtio-9p.c |  103 ++-
  1 files changed, 4 insertions(+), 99 deletions(-)

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Avi Kivity]

On 11/03/2011 03:07 PM, Anthony Liguori wrote:
> On 11/03/2011 05:36 AM, Avi Kivity wrote:
>> On 11/02/2011 12:20 AM, Anthony Liguori wrote:
>>>
>>> Seems harmless for QEMU, so applied.  You should update the virtio-pci
>>> spec too.
>>
>> Should be the other way round.
>
> Am not entirely sure.  Having worked code that's been reviewed will
> make for a better spec.  Writing the spec and committing to the spec
> change before getting either side of the implementation merged seems
> to be asking for trouble to me.

Maybe.  But committed code before a spec proposal?

I can see the spec maintainer requiring prototype code (for both guest
and host).  But committing code before a spec change is even seen is not
the right way to do this.

> We could use a better agreement on the processor for making virtio
> changes. Should it go (1) virtio spec (2) kernel (3) qemu, or should
> it go (2), (1), (3)?

1. Informal discussion
2. Proposed spec patch, kernel change, qemu change
3. Buy-ins from spec maintainer, kernel driver maintainer, qemu device
maintainer (only regarding the ABI, not the code)
4. Spec change committed
5. kernel and qemu code submitted and committed through the normal process

Note there are multiple implementations of the device code (qemu virtio,
linux vhost) and driver code (linux, windows).  The only real
synchronization point is the spec.

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

Re: [Qemu-devel] GSoC mentor summit session "how to bind students long-term"

[Luiz Capitulino]

On Mon, 31 Oct 2011 08:55:56 +
Stefan Hajnoczi  wrote:

> On Sat, Oct 29, 2011 at 04:00:34PM +0200, Alexander Graf wrote:
> > During the GSoC mentor summit there was a pretty interesting session on how 
> > to get students to stick with your project even after GSoC has ended. So 
> > far we haven't really been exactly successful in that respect :). I'll just 
> > post my notes below:
> > 
> >   - send successful students to conferences
> >   - set expectations on what we expect from students after gsoc, lay out 
> > the achievement plan for students to times beyond gsoc
> >   - give students responsibility, make them maintain parts (makes it harder 
> > for them to just leave, because they feel obliged)
> >   - shove students to community, no sidechannel communication, make them do 
> > A&Os on the public list
> 
> My personal experience being a GSoC student was that responsibility and
> fellowship matters most - it's what makes contributing addictive.  It's
> one thing to do an interesting project for 12 weeks but another to stick
> around because the group of developers have become your friends and you
> feel responsibility and satisfaction from supporting users on
> IRC/mailing lists.
> 
> The easiest way to give students responsibility is to get them actively
> involved in supporting users on IRC/mailing lists and fixing bugs.
> Doing this in addition to the official GSoC project is more likely to
> keep them hooked.  It helps turn them into a QEMU expert and someone who
> can help others - and hopefully they'll want to continue using this
> skill once the summer is over.

Please, let's create a http://wiki.qemu.org/GSoCMentors or something.

> 
> Stefan
> 

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Anthony Liguori]

On 11/03/2011 08:13 AM, Avi Kivity wrote:

On 11/03/2011 03:07 PM, Anthony Liguori wrote:

On 11/03/2011 05:36 AM, Avi Kivity wrote:

On 11/02/2011 12:20 AM, Anthony Liguori wrote:


Seems harmless for QEMU, so applied.  You should update the virtio-pci
spec too.


Should be the other way round.


Am not entirely sure.  Having worked code that's been reviewed will
make for a better spec.  Writing the spec and committing to the spec
change before getting either side of the implementation merged seems
to be asking for trouble to me.


Maybe.  But committed code before a spec proposal?

I can see the spec maintainer requiring prototype code (for both guest
and host).  But committing code before a spec change is even seen is not
the right way to do this.


We could use a better agreement on the processor for making virtio
changes. Should it go (1) virtio spec (2) kernel (3) qemu, or should
it go (2), (1), (3)?


1. Informal discussion


Where?  Is this lkml?  There were a number of virtio changes recently that never 
involved qemu-devel.



2. Proposed spec patch, kernel change, qemu change
3. Buy-ins from spec maintainer, kernel driver maintainer, qemu device
maintainer (only regarding the ABI, not the code)


I don't think this is how it's working today.  I would be happy with a flow like 
this.


Regards,

Anthony Liguori


4. Spec change committed
5. kernel and qemu code submitted and committed through the normal process

Note there are multiple implementations of the device code (qemu virtio,
linux vhost) and driver code (linux, windows).  The only real
synchronization point is the spec.

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Avi Kivity]

On 11/03/2011 03:38 PM, Anthony Liguori wrote:
>>
>>> We could use a better agreement on the processor for making virtio
>>> changes. Should it go (1) virtio spec (2) kernel (3) qemu, or should
>>> it go (2), (1), (3)?
>>
>> 1. Informal discussion
>
>
> Where?  Is this lkml?  There were a number of virtio changes recently
> that never involved qemu-devel.

Theoretically, virtualizat...@lists.linux-foundation.org, if it still
exists.  Maybe we need a virtio list. qemu-devel@, kvm@, lkml could be
copied.

The point is that we can't drive virtio from either qemu or the kernel
any more.  The spec represents the "virtual hardware manufacturer",
which qemu and linux/vhost (and others) emulate, and which linux (and
others) write drivers for.

>
>> 2. Proposed spec patch, kernel change, qemu change
>> 3. Buy-ins from spec maintainer, kernel driver maintainer, qemu device
>> maintainer (only regarding the ABI, not the code)
>
> I don't think this is how it's working today.  I would be happy with a
> flow like this.

If Michael and Rusty agree, we can adopt it immediately.

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

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Anthony Liguori]

On 11/03/2011 08:45 AM, Avi Kivity wrote:

On 11/03/2011 03:38 PM, Anthony Liguori wrote:



We could use a better agreement on the processor for making virtio
changes. Should it go (1) virtio spec (2) kernel (3) qemu, or should
it go (2), (1), (3)?


1. Informal discussion



Where?  Is this lkml?  There were a number of virtio changes recently
that never involved qemu-devel.


Theoretically, virtualizat...@lists.linux-foundation.org, if it still
exists.  Maybe we need a virtio list. qemu-devel@, kvm@, lkml could be
copied.


Perhaps it's time to create a virtio@vger?  Just have a simple process that all 
spec changes to there the appropriate kernel, QEMU, virtio-win, or NKT 
maintainers can require any virtio change to also have a committed spec change 
first.



The point is that we can't drive virtio from either qemu or the kernel
any more.  The spec represents the "virtual hardware manufacturer",
which qemu and linux/vhost (and others) emulate, and which linux (and
others) write drivers for.


Yup.  We need to be more rigorous about using the spec for that as we've not 
done a great job historically here.


Regards,

Anthony Liguori






2. Proposed spec patch, kernel change, qemu change
3. Buy-ins from spec maintainer, kernel driver maintainer, qemu device
maintainer (only regarding the ABI, not the code)


I don't think this is how it's working today.  I would be happy with a
flow like this.


If Michael and Rusty agree, we can adopt it immediately.

Re: [Qemu-devel] [v8 Patch 1/6]Qemu: Enhance "info block" to display host cache setting

[Luiz Capitulino]

On Sun, 30 Oct 2011 16:03:54 +0530
Supriya Kannery  wrote:

> Enhance "info block" to display hostcache setting for each
> block device.
> 
> Example:
> (qemu) info block
> ide0-hd0: removable=0 file=../sles11-32.raw ro=0 drv=raw encrypted=0
> 
> Enhanced to display "hostcache" setting:
> (qemu) info block
> ide0-hd0: removable=0 hostcache=1 file=../sles11-32.raw ro=0 drv=raw 
> encrypted=0
> 
> Signed-off-by: Supriya Kannery 
> 
> ---
>  block.c |   20 
>  qmp-commands.hx |2 ++
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> Index: qemu/qmp-commands.hx
> ===
> --- qemu.orig/qmp-commands.hx
> +++ qemu/qmp-commands.hx
> @@ -1142,6 +1142,7 @@ Each json-object contain the following:
>  - "locked": true if the device is locked, false otherwise (json-bool)
>  - "tray-open": only present if removable, true if the device has a tray,
> and it is open (json-bool)
> +- "hostcache": true if host pagecache enabled, false otherwise (json-bool)
>  - "inserted": only present if the device is inserted, it is a json-object
> containing the following:
>   - "file": device file name (json-string)
> @@ -1168,6 +1169,7 @@ Example:
>  "io-status": "ok",
>  "device":"ide0-hd0",
>  "locked":false,
> +"hostcache":false,
>  "removable":false,
>  "inserted":{
> "ro":false,
> Index: qemu/block.c
> ===
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -1841,6 +1841,11 @@ static void bdrv_print_dict(QObject *obj
> qdict_get_bool(bs_dict, "tray-open"));
>  }
>  
> +if (qdict_haskey(bs_dict, "hostcache")) {
> +monitor_printf(mon, " hostcache=%d",
> +   qdict_get_bool(bs_dict, "hostcache"));
> +}

This series needs to be rebased, as the info block command has been
converted to the QAPI. Please, see the following commit for details:
b202381800d81fbff9978abbdea95760dd363bb6.

Also note that if you're adding new commands (I haven't reviewed the
series) you should use the QAPI. A document on how to use it is coming soon.

> 
>  if (qdict_haskey(bs_dict, "io-status")) {
>  monitor_printf(mon, " io-status=%s", qdict_get_str(bs_dict, 
> "io-status"));
>  }
> @@ -1888,10 +1893,12 @@ void bdrv_info(Monitor *mon, QObject **r
>  QDict *bs_dict;
>  
>  bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', "
> -"'removable': %i, 'locked': %i }",
> +"'removable': %i, 'locked': %i, "
> +"'hostcache': %i }",
>  bs->device_name,
>  bdrv_dev_has_removable_media(bs),
> -bdrv_dev_is_medium_locked(bs));
> +bdrv_dev_is_medium_locked(bs),
> +!(bs->open_flags & BDRV_O_NOCACHE));
>  bs_dict = qobject_to_qdict(bs_obj);
>  
>  if (bdrv_dev_has_removable_media(bs)) {
> 
> 

[Qemu-devel] [PATCH 1.0] add sgabios blob and submodule

[Paolo Bonzini]

The rom was not added together with the sgabios device and is
not installed.

Signed-off-by: Paolo Bonzini 
---
The sgabios.git mirror repository can be fetched from
http://people.redhat.com/pbonzini/sgabios-git.tgz

 .gitmodules |3 +++
 Makefile|2 +-
 pc-bios/README  |6 ++
 pc-bios/sgabios.bin |  Bin 0 -> 4096 bytes
 roms/sgabios|1 +
 5 files changed, 11 insertions(+), 1 deletions(-)
 create mode 100755 pc-bios/sgabios.bin
 create mode 16 roms/sgabios

diff --git a/.gitmodules b/.gitmodules
index 2a43dbc..eca876f 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -16,3 +16,6 @@
 [submodule "roms/qemu-palcode"]
path = roms/qemu-palcode
url = git://repo.or.cz/qemu-palcode.git
+[submodule "roms/sgabios"]
+   path = roms/sgabios
+   url = git://git.qemu.org/sgabios.git
diff --git a/Makefile b/Makefile
index 4f6eaa4..168093c 100644
--- a/Makefile
+++ b/Makefile
@@ -253,7 +253,7 @@ ar  de en-us  fi  fr-be  hr it  lv  nl 
pl  ru th \
 common  de-ch  es fo  fr-ca  hu ja  mk  nl-be  pt  sl tr
 
 ifdef INSTALL_BLOBS
-BLOBS=bios.bin vgabios.bin vgabios-cirrus.bin \
+BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
 vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
 ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \
 pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
diff --git a/pc-bios/README b/pc-bios/README
index 0668559..1cebbbc 100644
--- a/pc-bios/README
+++ b/pc-bios/README
@@ -19,6 +19,12 @@
   https://github.com/dgibson/SLOF, and the image currently in qemu is
   built from git tag qemu-slof-20111013.
 
+- sgabios (the Serial Graphics Adapter option ROM) provides a means for
+  legacy x86 software to communicate with an attached serial console as
+  if a video card were attached.  The master sources reside in a subversion
+  repository at http://sgabios.googlecode.com/svn/trunk.  A git mirror is
+  available at git://git.qemu.org/sgabios.git.
+
 - The PXE roms come from the iPXE project. Built with BANNER_TIME 0.
   Sources available at http://ipxe.org.  Vendor:Device ID -> ROM mapping:
 
diff --git a/pc-bios/sgabios.bin b/pc-bios/sgabios.bin
new file mode 100755
index 
..c3da4c3d0a34e4eca9b0cda38d0538b332f7b09e
GIT binary patch
literal 4096
zcmeHJeRNah8Gmw(K=NZ!#YiO_$Hpho3C>|*k=364AM}*OQ+`H$d;Erd1?wtL_
zdrscx_dM_O{NCq%?)%)_<*#K9x2T8LsLwZz!svOLz$lClLt13p+N?4PpsPn6{(7CgLT_|Gwe#g&&%N@y-~Z{Y=U;g8
z>EAx{?3=%M{g=D%kmB#V#y>;iFGT)H-|_$IkH7wd2FlAf&4
z(K9!L)zW+!O&LAG>I~YmE02WU@_&ZX!e`URplc_v)&RTzVJbtb^y
zKF2cAH_MXAswSND$*AVA7@giGOh@BQ&zsL_)_w&O$}D71UB{0iB=(`uU6m?*QDyz~kk)R`zx
zRE2jGq6-zEh%Uq{j~62G1{rfZ@mh`wT@r?XUYW%G@}l}aPm2{P@T$dN(PozWQu
zN)|Qpw^ZR_WGrPqnT|5@E0yUo@aF~=XyCs&8^7bMC$5Z?#Ji+j(HQ|5f8xmCI4ggc
z|7=M8&70D3=Vth(rkOjVf*dP!-OAq4c@Zc&4>LV-k(Q&tSedaagIl1H5}3y@6)?|g
zgL#bE+yNGbsRWC)4uQF_14a+C}WuUP-b-?Fwg3M2NtQQXh^$+!F4)Et
zBB7nZJsk|%mRRS*65MPdgRTpiMPOxc-O4P0Mb-*fggM)+m0-hUYZ%xV3du$omosuL
zKI3bP7_mFZMbT&Mk@@aUo4@YyvlqC2okrgavg7*f&D?(Ouw^q#g)+B>8CRsveBkT_
z@}YQotUj{bQD^hCfcXG_j{H;fm2nF|*jMczQ}{B-ry@Jr`>mWmM|a&@m3h`To$i`h
zm3hH;^PE;-<<+s#ah<&y`@$6F&+!Wsf&B_+J+m@mv)5q`Q@H)_j*7$~_83l&af9jH
zM(Qzn2d38J_bIbC_jb#UTMX@rUOy(vL@X+w){^PF$)fhDu&d3}iDe{pRQ<8g9
z`F3HGw$}*8vWFGb9XNG+_#E~SAix{fI&@2p{grSp!{+I?m)WX4-QW%I)rlWMp
z)L!ta=q?n(LAs=PZ(2hk{DZVnI_w&LY8L2S5!;I7?k!=gK(QFR(%?+%q(eLtm#_mD
z$&ba|T!eVx%(o4UWILwjBiz+vYA(tRN|9M65*IR)J{5Nr;YEQvfr1EJxZ1fF2=|qiBqMIycl046@k2wvP@&;WGEHX-3h!Z5TOEzGOxT>2Rs_2cg*mL
zv0>-fD*jyHgGqQXGWeic!!tdpciTyR%5>Is$UU4fX0sE0%BRBnEGI3Z%Q-ydDjqnU+9(8d
zS&;Mbm6JLDx^gP>D67J9KAAF|^ougYYO*C|68#C$r}ifVHY=1#+EX~SN*)Qx!$4N2
zy+u|{{6eULI4}TT+;Z}aF#h!M89Clok!J;l1XB^RLx`y)eH-dtAnQ|sG1&%rO)N$%
z5Fs=0&8XD#BRv$GMH+>z2axD+Dkdk#Q#~@Qp&Z}I5u22{c26{0b$C!=<0Jg7dS^uE
zKx0?gY9OaXMvpC#_BrJrQXL-hM{L1cgPK)r+yH3Bh>kBq=DM|j4
z3@sy(1TN8?Wmy1WRvy?-MOI-e*_ZR@f-5vD*FB`(a+Z}3&lLSh3gs@rpOm8FDaW#_-nRi+9QP?-3y2T1mrY@mPVYl~Hh8^s
z8pAqFCwE&+-damZyEDU5r*I|-7PUeoBwOJnQ^S&--}2XZz}N)vrcft-w!S&6<@AV=dQ
zFL9lwzV8^2a2%7SyL;i~#V~-U1oCA(^cXomMtAQ8Zolw%&bS3QV;zuzIJp=fP}YEa
zK-mQMMU{k(>ojNV1){$~-oMg!
z5=d>F-f5z?4U+aa-nY&8Z$p~nn+a

literal 0
HcmV?d1

diff --git a/roms/sgabios b/roms/sgabios
new file mode 16
index 000..23d4749
--- /dev/null
+++ b/roms/sgabios
@@ -0,0 +1 @@
+Subproject commit 23d474943dcd55d0550a3d20b3d30e9040a4f15b
-- 
1.7.6.4

Re: [Qemu-devel] [RFC 3/6] block: wait for overlapping requests

[Kevin Wolf]

Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
> When copy-on-read is enabled it is necessary to wait for overlapping
> requests before issuing new requests.  This prevents races between the
> copy-on-read and a write request.
> 
> Signed-off-by: Stefan Hajnoczi 

This doesn't only order guest request against COR requests, but also
makes guest requests wait on each other. It's probably not a big
problem, but if we had to optimise performance with COR later, this is
something to remember.

Doing an optimisation that only requests of different type are ordered
wouldn't be too hard, though maybe avoiding starvation of the other type
could get a bit harder.

Let's leave it as it is for now.

Kevin

Re: [Qemu-devel] [RFC 5/6] block: core copy-on-read logic

[Kevin Wolf]

Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
> Signed-off-by: Stefan Hajnoczi 
> ---
>  block.c  |   69 
> ++
>  trace-events |1 +
>  2 files changed, 70 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 0c22741..2aec6b4 100644
> --- a/block.c
> +++ b/block.c
> @@ -1409,6 +1409,55 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t 
> offset,
>  return 0;
>  }
>  
> +static int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
> +int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
> +{
> +void *bounce_buffer;

I think the bounce buffer deserves a comment. It may not be obvious for
everyone why it's needed.

> +struct iovec iov;
> +QEMUIOVector bounce_qiov;
> +int64_t cluster_sector_num;
> +int cluster_nb_sectors;
> +size_t skip_bytes;
> +int ret;
> +
> +/* Cover entire cluster so no additional backing file I/O is required 
> when
> + * allocating cluster in the image file.
> + */
> +round_to_clusters(bs, sector_num, nb_sectors,
> +  &cluster_sector_num, &cluster_nb_sectors);
> +
> +trace_bdrv_co_copy_on_readv(bs, sector_num, nb_sectors,
> +cluster_sector_num, cluster_nb_sectors);
> +
> +iov.iov_len = cluster_nb_sectors * BDRV_SECTOR_SIZE;
> +iov.iov_base = bounce_buffer = qemu_blockalign(bs, iov.iov_len);
> +qemu_iovec_init_external(&bounce_qiov, &iov, 1);
> +
> +ret = bs->drv->bdrv_co_readv(bs, cluster_sector_num, cluster_nb_sectors,
> + &bounce_qiov);
> +if (ret < 0) {
> +goto err;
> +}
> +
> +ret = bs->drv->bdrv_co_writev(bs, cluster_sector_num, cluster_nb_sectors,
> +  &bounce_qiov);
> +if (ret < 0) {
> +/* It might be okay to ignore write errors for guest requests.  If 
> this
> + * is a deliberate copy-on-read then we don't want to ignore the 
> error.
> + * Simply report it in all cases.
> + */
> +goto err;
> +}
> +
> +skip_bytes = (sector_num - cluster_sector_num) * BDRV_SECTOR_SIZE;
> +qemu_iovec_from_buffer(qiov, bounce_buffer + skip_bytes,
> +   nb_sectors * BDRV_SECTOR_SIZE);
> +
> +err:
> +qemu_vfree(bounce_buffer);
> +return ret;
> +}
> +
>  /*
>   * Handle a read request in coroutine context
>   */
> @@ -1431,7 +1480,27 @@ static int coroutine_fn 
> bdrv_co_do_readv(BlockDriverState *bs,
>  }
>  
>  req = tracked_request_add(bs, sector_num, nb_sectors, false);
> +
> +if (bs->copy_on_read) {
> +int pnum;
> +
> +/* TODO it is not safe to call bdrv_is_allocated() in coroutine 
> context
> + * because it's a synchronous interface.  We probably want a
> + * bdrv_co_is_allocated(). */
> +ret = bdrv_is_allocated(bs, sector_num, nb_sectors, &pnum);

I think I already said it in a reply to another patch, but for the
record: We need bdrv_co_is_allocated() before this can be merged.

Kevin

Re: [Qemu-devel] [RFC 6/6] block: add -drive copy-on-read=on|off

[Kevin Wolf]

Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
> This patch adds the -drive copy-on-read=on|off command-line option:
> 
>   copy-on-read=on|off
>   copy-on-read is "on" or "off" and enables whether to copy read backing
>   file sectors into the image file.  Copy-on-read avoids accessing the
>   same backing file sectors repeatedly and is useful when the backing
>   file is over a slow network.  By default copy-on-read is off.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  blockdev.c  |6 ++
>  hmp-commands.hx |5 +++--
>  qemu-config.c   |4 
>  qemu-options.hx |9 -
>  4 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 0827bf7..1dd0f23 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -236,6 +236,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>  const char *devaddr;
>  DriveInfo *dinfo;
>  int snapshot = 0;
> +int copy_on_read;

Another s/int/bool/ :-)

Kevin

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Michael S. Tsirkin]

On Thu, Nov 03, 2011 at 08:49:31AM -0500, Anthony Liguori wrote:
> On 11/03/2011 08:45 AM, Avi Kivity wrote:
> >On 11/03/2011 03:38 PM, Anthony Liguori wrote:
> >>>
> We could use a better agreement on the processor for making virtio
> changes. Should it go (1) virtio spec (2) kernel (3) qemu, or should
> it go (2), (1), (3)?
> >>>
> >>>1. Informal discussion
> >>
> >>
> >>Where?  Is this lkml?  There were a number of virtio changes recently
> >>that never involved qemu-devel.
> >
> >Theoretically, virtualizat...@lists.linux-foundation.org, if it still
> >exists.  Maybe we need a virtio list. qemu-devel@, kvm@, lkml could be
> >copied.
> 
> Perhaps it's time to create a virtio@vger?  Just have a simple
> process that all spec changes to there the appropriate kernel, QEMU,
> virtio-win, or NKT maintainers can require any virtio change to also
> have a committed spec change first.

Historically virtualizat...@lists.linux-foundation.org was used,
that is still low enough volume. It was down but seems to be
up now. It's easy enough for me to subscribe to yet
another list but my concern is that if we move we might loose some
people that don't notice the change.

Anything wrong with just using the old list?
It's on the MAINTAINERS file, I'm not sure why it was bypassed in this case.
Maybe linux-foundation.org was still down when the patch was posted?

> >The point is that we can't drive virtio from either qemu or the kernel
> >any more.  The spec represents the "virtual hardware manufacturer",
> >which qemu and linux/vhost (and others) emulate, and which linux (and
> >others) write drivers for.
> 
> Yup.  We need to be more rigorous about using the spec for that as
> we've not done a great job historically here.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >>
> >>>2. Proposed spec patch, kernel change, qemu change
> >>>3. Buy-ins from spec maintainer, kernel driver maintainer, qemu device
> >>>maintainer (only regarding the ABI, not the code)
> >>
> >>I don't think this is how it's working today.  I would be happy with a
> >>flow like this.
> >
> >If Michael and Rusty agree, we can adopt it immediately.
> >

If I understand the proposal, what is suggested is that
all of spec, kvm and virtio patches are posted on list and
acked before merging any one of them?

Sure, this makes sense.

-- 
MST

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Anthony Liguori]

On 11/03/2011 09:31 AM, Michael S. Tsirkin wrote:

On Thu, Nov 03, 2011 at 08:49:31AM -0500, Anthony Liguori wrote:

On 11/03/2011 08:45 AM, Avi Kivity wrote:

On 11/03/2011 03:38 PM, Anthony Liguori wrote:



We could use a better agreement on the processor for making virtio
changes. Should it go (1) virtio spec (2) kernel (3) qemu, or should
it go (2), (1), (3)?


1. Informal discussion



Where?  Is this lkml?  There were a number of virtio changes recently
that never involved qemu-devel.


Theoretically, virtualizat...@lists.linux-foundation.org, if it still
exists.  Maybe we need a virtio list. qemu-devel@, kvm@, lkml could be
copied.


Perhaps it's time to create a virtio@vger?  Just have a simple
process that all spec changes to there the appropriate kernel, QEMU,
virtio-win, or NKT maintainers can require any virtio change to also
have a committed spec change first.


Historically virtualizat...@lists.linux-foundation.org was used,
that is still low enough volume. It was down but seems to be
up now. It's easy enough for me to subscribe to yet
another list but my concern is that if we move we might loose some
people that don't notice the change.

Anything wrong with just using the old list?


No, but it's so old, I have no idea who admin's it these days.


It's on the MAINTAINERS file, I'm not sure why it was bypassed in this case.
Maybe linux-foundation.org was still down when the patch was posted?


The list has been down for a while.


2. Proposed spec patch, kernel change, qemu change
3. Buy-ins from spec maintainer, kernel driver maintainer, qemu device
maintainer (only regarding the ABI, not the code)


I don't think this is how it's working today.  I would be happy with a
flow like this.


If Michael and Rusty agree, we can adopt it immediately.



If I understand the proposal, what is suggested is that
all of spec, kvm and virtio patches are posted on list and
acked before merging any one of them?

Sure, this makes sense.


Well, what's needed before the spec is changed is an interesting question, but I 
think the main thing is, don't commit any virtio ABI changes to vhost, QEMU, 
NKT, or the kernel until the spec for the change has been committed.


It would be nice to have a working implementation before committing a spec 
change.  Even nicer would be to have Acked-by's a maintainer in each area affected.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] virtio: Add PCI memory BAR in addition to PIO BAR

[Avi Kivity]

On 11/03/2011 04:37 PM, Anthony Liguori wrote:
>
>> 2. Proposed spec patch, kernel change, qemu change
>> 3. Buy-ins from spec maintainer, kernel driver maintainer, qemu
>> device
>> maintainer (only regarding the ABI, not the code)
>
> I don't think this is how it's working today.  I would be happy
> with a
> flow like this.

 If Michael and Rusty agree, we can adopt it immediately.

>>
>> If I understand the proposal, what is suggested is that
>> all of spec, kvm and virtio patches are posted on list and
>> acked before merging any one of them?
>>
>> Sure, this makes sense.
>
> Well, what's needed before the spec is changed is an interesting
> question, but I think the main thing is, don't commit any virtio ABI
> changes to vhost, QEMU, NKT, or the kernel until the spec for the
> change has been committed.
>
> It would be nice to have a working implementation before committing a
> spec change.  Even nicer would be to have Acked-by's a maintainer in
> each area affected.
>

Those are steps 2 and 3 of the proposed process.

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

[Qemu-devel] [PATCH 1.0] disable automatic loading of sgabios when -nographic

[Paolo Bonzini]

sgabios hasn't gotten a lot of coverage since it was not shipped.  For 1.0,
let's disable the automatic loading of the option ROM in -nographic
mode.  We can put it back for 1.1.

Signed-off-by: Paolo Bonzini 
---
Requested by Anthony on IRC.

 hw/pc.c |9 -
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index eb4c2d8..1024465 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1104,15 +1104,6 @@ void pc_vga_init(PCIBus *pci_bus)
 isa_vga_init();
 }
 }
-
-/*
- * sga does not suppress normal vga output. So a machine can have both a
- * vga card and sga manually enabled. Output will be seen on both.
- * For nographic case, sga is enabled at all times
- */
-if (display_type == DT_NOGRAPHIC) {
-isa_create_simple("sga");
-}
 }
 
 static void cpu_request_exit(void *opaque, int irq, int level)
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 1/2] Allow 1366x768 as a valid VGA resolution

[John Baboval]

This is a good idea. I'm going to re-work the patch, but I have a lot of 
other stuff going on too, so it may be a week or so before I get back to it.


On 11/01/2011 12:57 PM, Gerd Hoffmann wrote:

On 11/01/11 14:39, John Baboval wrote:

I don't know of any reason for it.

I'd guess it is alignment, probably not important for all color depts.

Maybe it is a good idea to do all sanity checks in the
VBE_DISPI_INDEX_ENABLE branch where the actual mode switch happens. Then
you already know xres, yres and depth when applying the checks.  You can
calculate the scanline length, then check the scanline alignment instead
of being overly strict on xres in high color modes to satisfy alignment
requirements in low color modes.

You can also simply calculate how much memory the video mode needs and
check that against the configured video ram instead of pulling xres and
yres limits out of thin air.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH] os-posix: call system_powerdown on SIGTERM, enabling clean guest shutdown

[Anthony Liguori]

On 11/03/2011 08:08 AM, Bjørn Mork wrote:

Allow well behaved guests to shutdown cleanly when we receive SIGTERM,
e.g. when the host reboots.

The host may be powered down or rebooted while guests are running
without any outer supervision scripts.  These guests will be stopped
by the generic host "sendsigs" script, usually by sending SIGTERM to all
processes still running and then SIGKILL if that didn't work.  Let the
guest power down instead of just killing i instantly.

Signed-off-by: Bjørn Mork


I don't think this is such a good idea.  SIGTERM shouldn't be subject to the 
guest's interpretations.


Instead of doing a killall qemu, just send a system_powerdown command to each 
qemu's monitor instance.


Regards,

Anthony Liguori


---
I wonder if I miss something obvious here... So this patch should be
considered a RFC.  It certainly fixes a small problem for me:  I often
run a number of guests which are started manually, or by simple "runme"
scripts, without any libvirt wrapper or similar.  If/when the host is
rebooted, these guests will be shut down abrubtly with possible guest
file system corruption and other data loss as a consequence. Powering
down on SIGTERM seems like an obious win-win.

I also wonder if it would be useful to do a reboot on HUP, but that
wouldn't solve any real problem so I'm not including it in this
patch.



Bjørn

  os-posix.c |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index dbf3b24..167fd3b 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -65,7 +65,10 @@ void os_setup_early_signal_handling(void)

  static void termsig_handler(int signal, siginfo_t *info, void *c)
  {
-qemu_system_killed(info->si_signo, info->si_pid);
+if (info->si_signo == SIGTERM)
+   qemu_system_powerdown_request();
+else
+   qemu_system_killed(info->si_signo, info->si_pid);
  }

  void os_setup_signal_handling(void)

[Qemu-devel] [PATCH] [SPARC] Improve "ta 0" shutdown

[Fabien Chouteau]

This patch replace the previous implementation with this simplified and
more complete version (no shutdown when psret == 1).

Signed-off-by: Fabien Chouteau 
---
 target-sparc/helper.c   |7 ---
 target-sparc/helper.h   |1 -
 target-sparc/int32_helper.c |   10 --
 target-sparc/translate.c|9 +
 4 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 18609c4..037a72c 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -34,13 +34,6 @@ void helper_debug(CPUState *env)
 cpu_loop_exit(env);
 }
 
-void helper_shutdown(void)
-{
-#if !defined(CONFIG_USER_ONLY)
-qemu_system_shutdown_request();
-#endif
-}
-
 #ifdef TARGET_SPARC64
 target_ulong helper_popc(target_ulong val)
 {
diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index faaf8dc..1f67b08 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -79,7 +79,6 @@ DEF_HELPER_1(fcmpeq_fcc2, void, env)
 DEF_HELPER_1(fcmpeq_fcc3, void, env)
 #endif
 DEF_HELPER_2(raise_exception, void, env, int)
-DEF_HELPER_0(shutdown, void)
 #define F_HELPER_0_1(name) DEF_HELPER_1(f ## name, void, env)
 
 DEF_HELPER_3(faddd, f64, env, f64, f64)
diff --git a/target-sparc/int32_helper.c b/target-sparc/int32_helper.c
index 3a749bf..ac9d01e 100644
--- a/target-sparc/int32_helper.c
+++ b/target-sparc/int32_helper.c
@@ -19,6 +19,7 @@
 
 #include "cpu.h"
 #include "trace.h"
+#include "sysemu.h"
 
 //#define DEBUG_PCALL
 
@@ -100,8 +101,13 @@ void do_interrupt(CPUState *env)
 #endif
 #if !defined(CONFIG_USER_ONLY)
 if (env->psret == 0) {
-cpu_abort(env, "Trap 0x%02x while interrupts disabled, Error state",
-  env->exception_index);
+if (env->exception_index == 0x80 &&
+env->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
+qemu_system_shutdown_request();
+} else {
+cpu_abort(env, "Trap 0x%02x while interrupts disabled, Error 
state",
+  env->exception_index);
+}
 return;
 }
 #endif
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 9318540..d261112 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2518,15 +2518,8 @@ static void disas_sparc_insn(DisasContext * dc)
 tcg_gen_andi_tl(cpu_dst, cpu_dst, V8_TRAP_MASK);
 tcg_gen_addi_tl(cpu_dst, cpu_dst, TT_TRAP);
 tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_dst);
+gen_helper_raise_exception(cpu_env, cpu_tmp32);
 
-if (rs2 == 0 &&
-dc->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
-
-gen_helper_shutdown();
-
-} else {
-gen_helper_raise_exception(cpu_env, cpu_tmp32);
-}
 } else if (cond != 0) {
 TCGv r_cond = tcg_temp_new();
 int l1;
-- 
1.7.4.1

[Qemu-devel] [PATCH] [SPARC] Improve "ta 0" shutdown

[Fabien Chouteau]

This patch replace the previous implementation with this simplified and
more complete version (no shutdown when psret == 1).

Signed-off-by: Fabien Chouteau 
---
 target-sparc/helper.c   |7 ---
 target-sparc/helper.h   |1 -
 target-sparc/int32_helper.c |   10 --
 target-sparc/translate.c|9 +
 4 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 18609c4..037a72c 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -34,13 +34,6 @@ void helper_debug(CPUState *env)
 cpu_loop_exit(env);
 }
 
-void helper_shutdown(void)
-{
-#if !defined(CONFIG_USER_ONLY)
-qemu_system_shutdown_request();
-#endif
-}
-
 #ifdef TARGET_SPARC64
 target_ulong helper_popc(target_ulong val)
 {
diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index faaf8dc..1f67b08 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -79,7 +79,6 @@ DEF_HELPER_1(fcmpeq_fcc2, void, env)
 DEF_HELPER_1(fcmpeq_fcc3, void, env)
 #endif
 DEF_HELPER_2(raise_exception, void, env, int)
-DEF_HELPER_0(shutdown, void)
 #define F_HELPER_0_1(name) DEF_HELPER_1(f ## name, void, env)
 
 DEF_HELPER_3(faddd, f64, env, f64, f64)
diff --git a/target-sparc/int32_helper.c b/target-sparc/int32_helper.c
index 3a749bf..ac9d01e 100644
--- a/target-sparc/int32_helper.c
+++ b/target-sparc/int32_helper.c
@@ -19,6 +19,7 @@
 
 #include "cpu.h"
 #include "trace.h"
+#include "sysemu.h"
 
 //#define DEBUG_PCALL
 
@@ -100,8 +101,13 @@ void do_interrupt(CPUState *env)
 #endif
 #if !defined(CONFIG_USER_ONLY)
 if (env->psret == 0) {
-cpu_abort(env, "Trap 0x%02x while interrupts disabled, Error state",
-  env->exception_index);
+if (env->exception_index == 0x80 &&
+env->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
+qemu_system_shutdown_request();
+} else {
+cpu_abort(env, "Trap 0x%02x while interrupts disabled, Error 
state",
+  env->exception_index);
+}
 return;
 }
 #endif
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 9318540..d261112 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2518,15 +2518,8 @@ static void disas_sparc_insn(DisasContext * dc)
 tcg_gen_andi_tl(cpu_dst, cpu_dst, V8_TRAP_MASK);
 tcg_gen_addi_tl(cpu_dst, cpu_dst, TT_TRAP);
 tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_dst);
+gen_helper_raise_exception(cpu_env, cpu_tmp32);
 
-if (rs2 == 0 &&
-dc->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
-
-gen_helper_shutdown();
-
-} else {
-gen_helper_raise_exception(cpu_env, cpu_tmp32);
-}
 } else if (cond != 0) {
 TCGv r_cond = tcg_temp_new();
 int l1;
-- 
1.7.4.1

[Qemu-devel] [PATCH] [SPARC] Improve "ta 0" shutdown

[Fabien Chouteau]

This patch replace the previous implementation with this simplified and
more complete version (no shutdown when psret == 1).

Signed-off-by: Fabien Chouteau 
---
 target-sparc/helper.c   |7 ---
 target-sparc/helper.h   |1 -
 target-sparc/int32_helper.c |   10 --
 target-sparc/translate.c|9 +
 4 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 18609c4..037a72c 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -34,13 +34,6 @@ void helper_debug(CPUState *env)
 cpu_loop_exit(env);
 }
 
-void helper_shutdown(void)
-{
-#if !defined(CONFIG_USER_ONLY)
-qemu_system_shutdown_request();
-#endif
-}
-
 #ifdef TARGET_SPARC64
 target_ulong helper_popc(target_ulong val)
 {
diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index faaf8dc..1f67b08 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -79,7 +79,6 @@ DEF_HELPER_1(fcmpeq_fcc2, void, env)
 DEF_HELPER_1(fcmpeq_fcc3, void, env)
 #endif
 DEF_HELPER_2(raise_exception, void, env, int)
-DEF_HELPER_0(shutdown, void)
 #define F_HELPER_0_1(name) DEF_HELPER_1(f ## name, void, env)
 
 DEF_HELPER_3(faddd, f64, env, f64, f64)
diff --git a/target-sparc/int32_helper.c b/target-sparc/int32_helper.c
index 3a749bf..ac9d01e 100644
--- a/target-sparc/int32_helper.c
+++ b/target-sparc/int32_helper.c
@@ -19,6 +19,7 @@
 
 #include "cpu.h"
 #include "trace.h"
+#include "sysemu.h"
 
 //#define DEBUG_PCALL
 
@@ -100,8 +101,13 @@ void do_interrupt(CPUState *env)
 #endif
 #if !defined(CONFIG_USER_ONLY)
 if (env->psret == 0) {
-cpu_abort(env, "Trap 0x%02x while interrupts disabled, Error state",
-  env->exception_index);
+if (env->exception_index == 0x80 &&
+env->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
+qemu_system_shutdown_request();
+} else {
+cpu_abort(env, "Trap 0x%02x while interrupts disabled, Error 
state",
+  env->exception_index);
+}
 return;
 }
 #endif
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 9318540..d261112 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2518,15 +2518,8 @@ static void disas_sparc_insn(DisasContext * dc)
 tcg_gen_andi_tl(cpu_dst, cpu_dst, V8_TRAP_MASK);
 tcg_gen_addi_tl(cpu_dst, cpu_dst, TT_TRAP);
 tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_dst);
+gen_helper_raise_exception(cpu_env, cpu_tmp32);
 
-if (rs2 == 0 &&
-dc->def->features & CPU_FEATURE_TA0_SHUTDOWN) {
-
-gen_helper_shutdown();
-
-} else {
-gen_helper_raise_exception(cpu_env, cpu_tmp32);
-}
 } else if (cond != 0) {
 TCGv r_cond = tcg_temp_new();
 int l1;
-- 
1.7.4.1

Re: [Qemu-devel] [RFC] block: convert block_stream command to QAPI

[Luiz Capitulino]

On Tue,  1 Nov 2011 18:08:16 +
Stefan Hajnoczi  wrote:

> Move from plain HMP/QMP command implementation to QAPI.  The
> block_stream command takes arguments, returns nothing, may raise errors,
> and will raise a QMP event when the operation completes.
> ---
> Hi Luiz,
> I converted block_stream to QAPI.  The in-tree examples are quite simple so 
> far
> so I hope I've converted this command in a reasonable way.  Please let me know
> what you think:

In general it looks good. The fact that the diff is against your latest series
(vs. against master) didn't help me, so I couldn't test the command.

>  - HMP wrapper does QError/Error housekeeping, is there a nicer way?

Yes. Usually, the HMP command should print the error or do something
about it. More comments below.

>  - To avoid duplicating documentation I move it to qapi-schema.json.

That's ok, the qmp-commands.hx file will die soon.

>  - QMP events seem independent of QAPI, of course raising a monitor event
>precludes this QAPI code from really being used as an internal C API today.
>Thoughts?

We'll add an event type soon, so for now it's ok to use the current interface
for that.

Some review comments below.

> 
> CCed qemu-devel because I suspect others are figuring out QAPI/monitor
> integration too.
> 
>  blockdev.c   |   25 -
>  hmp-commands.hx  |3 +--
>  hmp.c|   14 ++
>  hmp.h|1 +
>  qapi-schema.json |   53 +
>  qmp-commands.hx  |   49 ++---
>  trace-events |2 +-
>  7 files changed, 84 insertions(+), 63 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 6d78597..380de39 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -16,6 +16,7 @@
>  #include "qemu-objects.h"
>  #include "sysemu.h"
>  #include "block_int.h"
> +#include "qmp-commands.h"
>  #include "trace.h"
>  
>  static QTAILQ_HEAD(drivelist, DriveInfo) drives = 
> QTAILQ_HEAD_INITIALIZER(drives);
> @@ -817,39 +818,37 @@ static void block_stream_cb(void *opaque, int ret)
>  qobject_decref(obj);
>  }
>  
> -int do_block_stream(Monitor *mon, const QDict *params, QObject **ret_data)
> +void qmp_block_stream(const char *device, bool has_base,
> +  const char *base, Error **errp)
>  {
> -const char *device = qdict_get_str(params, "device");
> -const char *base = qdict_get_try_str(params, "base");
>  BlockDriverState *bs;
>  int ret;
>  
>  bs = bdrv_find(device);
>  if (!bs) {
> -qerror_report(QERR_DEVICE_NOT_FOUND, device);
> -return -1;
> +error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +return;
>  }
>  
>  /* Base device not supported */
>  if (base) {
> -qerror_report(QERR_NOT_SUPPORTED);
> -return -1;
> +error_set(errp, QERR_NOT_SUPPORTED);
> +return;
>  }
>  
>  ret = stream_start(bs, NULL, block_stream_cb, bs);
>  if (ret < 0) {
>  switch (ret) {
>  case -EBUSY:
> -qerror_report(QERR_DEVICE_IN_USE, device);
> -return -1;
> +error_set(errp, QERR_DEVICE_IN_USE, device);
> +return;
>  default:
> -qerror_report(QERR_NOT_SUPPORTED);
> -return -1;
> +error_set(errp, QERR_NOT_SUPPORTED);
> +return;
>  }
>  }
>  
> -trace_do_block_stream(bs, bs->job);
> -return 0;
> +trace_qmp_block_stream(bs, bs->job);
>  }
>  
>  static BlockJob *find_block_job(const char *device)
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index a8b83f0..b2af867 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -75,8 +75,7 @@ ETEXI
>  .args_type  = "device:B,base:s?",
>  .params = "device [base]",
>  .help   = "copy data from a backing file into a block device",
> -.user_print = monitor_user_noop,
> -.mhandler.cmd_new = do_block_stream,
> +.mhandler.cmd = hmp_block_stream,
>  },
>  
>  STEXI
> diff --git a/hmp.c b/hmp.c
> index 34416fc..e65510d 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -114,3 +114,17 @@ void hmp_system_powerdown(Monitor *mon, const QDict 
> *qdict)
>  {
>  qmp_system_powerdown(NULL);
>  }
> +
> +void hmp_block_stream(Monitor *mon, const QDict *qdict)
> +{
> +Error *error = NULL;
> +const char *device = qdict_get_str(qdict, "device");
> +const char *base = qdict_get_try_str(qdict, "base");
> +
> +qmp_block_stream(device, base != NULL, base, &error);
> +
> +if (error_is_set(&error)) {
> +qerror_report_err(error);
> +}
> +error_free(error);

If all you want here is to inform users about the error, you could do
something like this:

if (error_is_set(&error)) {
monitor_printf(mon, "%s\n", error_get_pretty(error));
error_free(error);
return;
}

> +}
> diff --git a/hmp.h b/hmp.h
> index 92433cf..779a601 100644
> --- a/h

Re: [Qemu-devel] [RFC 6/6] block: add -drive copy-on-read=on|off

[Stefan Hajnoczi]

On Thu, Nov 03, 2011 at 03:32:17PM +0100, Kevin Wolf wrote:
> Am 17.10.2011 17:47, schrieb Stefan Hajnoczi:
> > This patch adds the -drive copy-on-read=on|off command-line option:
> > 
> >   copy-on-read=on|off
> >   copy-on-read is "on" or "off" and enables whether to copy read backing
> >   file sectors into the image file.  Copy-on-read avoids accessing the
> >   same backing file sectors repeatedly and is useful when the backing
> >   file is over a slow network.  By default copy-on-read is off.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  blockdev.c  |6 ++
> >  hmp-commands.hx |5 +++--
> >  qemu-config.c   |4 
> >  qemu-options.hx |9 -
> >  4 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/blockdev.c b/blockdev.c
> > index 0827bf7..1dd0f23 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -236,6 +236,7 @@ DriveInfo *drive_init(QemuOpts *opts, int 
> > default_to_scsi)
> >  const char *devaddr;
> >  DriveInfo *dinfo;
> >  int snapshot = 0;
> > +int copy_on_read;
> 
> Another s/int/bool/ :-)

In general I use bool but in this case I hesitated since the other
variables are int I tried to stick with the style.  I'm happy to change
it if you prefer bool.

Stefan

Re: [Qemu-devel] [PATCH 3/8] block: add image streaming block job

[Marcelo Tosatti]

On Wed, Nov 02, 2011 at 03:43:49PM +, Stefan Hajnoczi wrote:
> On Tue, Nov 1, 2011 at 6:06 PM, Marcelo Tosatti  wrote:
> > On Thu, Oct 27, 2011 at 04:22:50PM +0100, Stefan Hajnoczi wrote:
> >> +static int stream_one_iteration(StreamBlockJob *s, int64_t sector_num,
> >> +                                void *buf, int max_sectors, int *n)
> >> +{
> >> +    BlockDriverState *bs = s->common.bs;
> >> +    int ret;
> >> +
> >> +    trace_stream_one_iteration(s, sector_num, max_sectors);
> >> +
> >> +    ret = bdrv_is_allocated(bs, sector_num, max_sectors, n);
> >> +    if (ret < 0) {
> >> +        return ret;
> >> +    }
> >
> > bdrv_is_allocated is still synchronous? If so, there should be at least
> > a plan to make it asynchronous.
> 
> Yes, that's a good discussion to have.  My thoughts are that
> bdrv_is_allocated() should be executed in coroutine context.  The
> semantics are a little tricky because of parallel requests:
> 
> 1. If a write request is in progress when we do bdrv_is_allocated() we
> might get back "unallocated" even though clusters are just being
> allocated.
> 2. If a TRIM request is in progress when we do bdrv_is_allocated() we
> might get back "allocated" even though clusters are just being
> deallocated.
> 
> In order to be reliable the caller needs to be aware of parallel
> requests.  I think it's correct to defer this problem to the caller.
> 
> In the case of image streaming we're not TRIM-safe, I haven't really
> thought about it yet.  But we are safe against parallel write requests
> because there is serialization to prevent copy-on-read requests from
> racing with write requests.
> 
> >> +    if (!ret) {
> >> +        ret = stream_populate(bs, sector_num, *n, buf);
> >> +    }
> >> +    return ret;
> >> +}
> >> +
> >> +static void coroutine_fn stream_run(void *opaque)
> >> +{
> >> +    StreamBlockJob *s = opaque;
> >> +    BlockDriverState *bs = s->common.bs;
> >> +    int64_t sector_num, end;
> >> +    int ret = 0;
> >> +    int n;
> >> +    void *buf;
> >> +
> >> +    buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE);
> >> +    s->common.len = bdrv_getlength(bs);
> >> +    bdrv_get_geometry(bs, (uint64_t *)&end);
> >> +
> >> +    bdrv_set_zero_detection(bs, true);
> >> +    bdrv_set_copy_on_read(bs, true);
> >
> > Should distinguish between stream initiated and user initiated setting
> > of zero detection and cor (so that unsetting below does not clear
> > user settings).
> 
> For zero detection I agree.
> 
> For copy-on-read it is questionable since once streaming is complete
> it does not make sense to have copy-on-read enabled.
> 
> I will address this in the next revision and think more about the
> copy-on-read case.
> 
> >> +
> >> +    for (sector_num = 0; sector_num < end; sector_num += n) {
> >> +        if (block_job_is_cancelled(&s->common)) {
> >> +            break;
> >> +        }
> >
> > If cancellation is seen here in the last loop iteration,
> > bdrv_change_backing_file below should not be executed.
> 
> I documented this case in the QMP API.  I'm not sure if it's possible
> to guarantee that the operation isn't just completing as you cancel
> it.  Any blocking point between completion of the last iteration and
> completing the operation is vulnerable to missing the cancel.  It's
> easier to explicitly say the operation might just have completed when
> you canceled, rather than trying to protect the completion path.  Do
> you think it's a problem to have these loose semantics that I
> described?

No, that is ok. I'm referring to bdrv_change_backing_file() being
executed without the entire image being streamed.

"if (sector_num == end && ret == 0)" includes both all sectors being 
streamed and all sectors except the last iteration being streamed (due
to job cancelled break).

> >> +
> >> +        /* TODO rate-limit */
> >> +        /* Note that even when no rate limit is applied we need to yield 
> >> with
> >> +         * no pending I/O here so that qemu_aio_flush() is able to return.
> >> +         */
> >> +        co_sleep_ns(rt_clock, 0);
> >
> > How do you plan to implement rate limit?
> 
> It was implemented in the QED-specific image streaming series:
> 
> http://repo.or.cz/w/qemu/stefanha.git/commitdiff/22f2c09d2fcfe5e49ac4604fd23e4744f549a476
> 
> That implementation works fine and is small but I'd like to reuse the
> migration speed limit, if possible.  That way we don't have 3
> different rate-limiting implementations in QEMU :).

One possibility would be to create a "virtual" block device for
streaming, sitting on top of the real block device. Then enforce block
I/O limits on the virtual block device, the guest would remain accessing
the real block device.

Re: [Qemu-devel] [PATCH (repost) RFC 2/2] virtio-pci: recall and return msix notifications on ISR read

[Jan Kiszka]

On 2011-11-03 13:07, Michael S. Tsirkin wrote:
> On Thu, Nov 03, 2011 at 12:42:55PM +0100, Jan Kiszka wrote:
>> On 2011-11-02 21:11, Michael S. Tsirkin wrote:
>>> MSIX spec requires that device can be operated with
>>> all vectors masked, by polling pending bits.
>>> Add APIs to recall an msix notification, and make polling
>>> mode possible in virtio-pci by clearing the
>>> pending bits and setting ISR appropriately on ISR read.
>>>
>>> Signed-off-by: Michael S. Tsirkin 
>>> ---
>>>  hw/msix.c   |   26 ++
>>>  hw/msix.h   |3 +++
>>>  hw/virtio-pci.c |   11 ++-
>>>  3 files changed, 39 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/msix.c b/hw/msix.c
>>> index 63b41b9..fe967c9 100644
>>> --- a/hw/msix.c
>>> +++ b/hw/msix.c
>>> @@ -349,6 +349,32 @@ void msix_notify(PCIDevice *dev, unsigned vector)
>>>  stl_le_phys(address, data);
>>>  }
>>>  
>>> +/* Recall outstanding MSI-X notifications for a vector, if possible.
>>> + * Return true if any were outstanding. */
>>> +bool msix_recall(PCIDevice *dev, unsigned vector)
>>> +{
>>> +bool ret;
>>> +if (vector >= dev->msix_entries_nr)
>>> +return false;
>>> +ret = msix_is_pending(dev, vector);
>>> +msix_clr_pending(dev, vector);
>>> +return ret;
>>> +}
>>
>> I would prefer to have a single API instead to clarify the tight relation:
>>
>> bool msi[x]_set_notify(PCIDevice *dev, unsigned vector, unsigned level)
>>
>> Would return true for level=1 if the message was either sent directly or
>> queued (we could deliver false if it was already queued, but I see no
>> use case for this yet).
> 
> It's a matter of taste: some people like functions with flags, some
> prefer separate functions.  I really prefer two functions.
> 
> But I agree it woulkd be better to have a name that makes it clear that
> what we recall is a notification.
> msix_notify_queue/msix_notify_dequeue?

OK, that doesn't sound bad.

> 
> 
>> Also, I don't see the generic value of some msix_recall_all. I think
>> it's better handled in a single loop over all vectors at caller site,
>> clearing the individual interrupt reason bits on a per-vector basis
>> there. msix_recall_all is only useful in the virtio case where you have
>> one vector of reason A and all the rest of B. Once you had multiple
>> reason C vectors as well, it would not help anymore.
>>
>> Jan
> 
> The reason I wanted to have it is to reduce the overhead this adds:
> since PBA is packed, it's much faster to check whether any bits are set
> than by going through them all, one by one. Typically all PBA
> bits are clear ...
> 
> I agree it might not help non-virtio devices, but to me it looks like a
> harmless little helper - what's the issue with it?

*If* there is a noticeable performance gain, I'm fine with
msix_notify_dequeue_all (about how may vectors are we talking in the
vitio case?). But the code would be more regular the other way around.

Jan

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

[Qemu-devel] [PATCH 1.0] qxl: fix vga port initialization.

[Gerd Hoffmann]

Commit 0a039dc70096b768d3810afa50ba1d214768aaf4 broke vga modes for
qxl-vga by loosing vga_ioport_read windup.  qxl needs to hook into
vga port writes only and used to realize that by letting vga_init() do
the work for both reads and writes, then overwrite the write function.
That little detail was missed while doing the conversion ...

This patch fixes it.  It also switch qxl vga ioport registration to
portio lists while being at it.

Cc: Hans de Goede 
Signed-off-by: Gerd Hoffmann 
---
 hw/qxl.c |   22 +-
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 84ffd45..41500e9 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -896,6 +896,20 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t 
addr, uint32_t val)
 vga_ioport_write(opaque, addr, val);
 }
 
+static const MemoryRegionPortio qxl_vga_portio_list[] = {
+{ 0x04,  2, 1, .read  = vga_ioport_read,
+   .write = qxl_vga_ioport_write }, /* 3b4 */
+{ 0x0a,  1, 1, .read  = vga_ioport_read,
+   .write = qxl_vga_ioport_write }, /* 3ba */
+{ 0x10, 16, 1, .read  = vga_ioport_read,
+   .write = qxl_vga_ioport_write }, /* 3c0 */
+{ 0x24,  2, 1, .read  = vga_ioport_read,
+   .write = qxl_vga_ioport_write }, /* 3d4 */
+{ 0x2a,  1, 1, .read  = vga_ioport_read,
+   .write = qxl_vga_ioport_write }, /* 3da */
+PORTIO_END_OF_LIST(),
+};
+
 static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
 qxl_async_io async)
 {
@@ -1595,6 +1609,7 @@ static int qxl_init_primary(PCIDevice *dev)
 PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev);
 VGACommonState *vga = &qxl->vga;
 ram_addr_t ram_size = msb_mask(qxl->vga.vram_size * 2 - 1);
+PortioList *qxl_vga_port_list = g_new(PortioList, 1);
 
 qxl->id = 0;
 
@@ -1603,11 +1618,8 @@ static int qxl_init_primary(PCIDevice *dev)
 }
 vga_common_init(vga, ram_size);
 vga_init(vga, pci_address_space(dev), pci_address_space_io(dev), false);
-register_ioport_write(0x3c0, 16, 1, qxl_vga_ioport_write, vga);
-register_ioport_write(0x3b4,  2, 1, qxl_vga_ioport_write, vga);
-register_ioport_write(0x3d4,  2, 1, qxl_vga_ioport_write, vga);
-register_ioport_write(0x3ba,  1, 1, qxl_vga_ioport_write, vga);
-register_ioport_write(0x3da,  1, 1, qxl_vga_ioport_write, vga);
+portio_list_init(qxl_vga_port_list, qxl_vga_portio_list, vga, "vga");
+portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0);
 
 vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
qxl_hw_screen_dump, qxl_hw_text_update, 
qxl);
-- 
1.7.1

Re: [Qemu-devel] [PATCH 2/5] linux-user: add open() hijack infrastructure

[Alexander Graf]

On 03.11.2011, at 02:34, David Gilbert  wrote:

> On 2 November 2011 19:23, Alexander Graf  wrote:
>> There are a number of files in /proc that expose host information
>> to the guest program. This patch adds infrastructure to override
>> the open() syscall for guest programs to enable us to on the fly
>> generate guest sensible files.
>> 
>> Signed-off-by: Alexander Graf 
>> ---
>>  linux-user/syscall.c |   52 
>> +++--
>>  1 files changed, 49 insertions(+), 3 deletions(-)
>> 
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 9f5da36..38953ba 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4600,6 +4600,52 @@ int get_osversion(void)
>> return osversion;
>>  }
>> 
>> +static int do_open(void *cpu_env, const char *pathname, int flags, mode_t 
>> mode)
> 
> Once you open the pandoras-box that is emulating /proc, I think you'll 
> probably
> need to hook it in more places and be more general; although you may
> well get away
> with it for this particular case.
> 
> Isn't it better to put the filename interception code somewhere more general
> so that it can also be misused by other calls - e.g. stat().

We can move it when we get there :).

> 
> I guess you're also going to need to be able to do /proc/pid/* instead
> of /proc/self;
> something is bound to use that.

I was hoping to get away with a bit less involving. Sure, we could actually 
open the pandora box of emulating all of /proc, but I really don't want to go 
down that route.

Since all these files already do exist on the host view, I don't think we have 
to fake stat(). The file size will be wrong either way.

So yes, eventually we might have to use the big sledge hammer of emulating all 
of /proc, but as long as I can refrain from it I will :). It's just a _lot_ of 
work.

Alex

> 
> Dave

Re: [Qemu-devel] [PATCH 0/5] linux-user: fake some /proc/self entries

[Alexander Graf]

On 03.11.2011, at 03:47, Riku Voipio  wrote:

> On Wed, Nov 02, 2011 at 08:23:21PM +0100, Alexander Graf wrote:
>> When running linux-user programs in QEMU, the guest program can examine
>> itself by checking /proc/self/ files. And some libraries really do use
>> this!
>> 
>> Unfortunately, when checking /proc/self/ today, the guest program sees
>> the QEMU files, exposing wrong information to the guest program.
> 
>> This patch set fakes auxv, maps and stat to make gtk, pthread and boehm
>> gc happy.
> 
> This is awesome stuff, but unfortunately just a day after 1.0 hard freeze :(
> David pointed out that more files in proc could be done, but this is a good 
> start.

I'm sure we'll find more nice little issues later, so no worries. There is a 
world post-1.0 :)

Alex

> 
> Riku
> 
>> Alex
>> 
>> Alexander Graf (5):
>>  linux-user: save auxv length
>>  linux-user: add open() hijack infrastructure
>>  linux-user: fake /proc/self/maps
>>  linux-user: fake /proc/self/stat
>>  linux-user: fake /proc/self/auxv
>> 
>> linux-user/elfload.c |   15 ++-
>> linux-user/qemu.h|1 +
>> linux-user/syscall.c |  123 -
>> 3 files changed, 125 insertions(+), 14 deletions(-)
>> 

[Qemu-devel] [RFC] docs: Add writing-qmp-commands.txt

[Luiz Capitulino]

Explains how to write QMP commands using the QAPI.

TODO:
- write "returning lists" chapter
- review it

Signed-off-by: Luiz Capitulino 
---

This is incomplete, but I figured I should send it anyway as there are people
who want to add new QMP commands but are still using the old interface. Review
is really appreciated.

 docs/writing-qmp-commands.txt |  488 +
 1 files changed, 488 insertions(+), 0 deletions(-)
 create mode 100644 docs/writing-qmp-commands.txt

diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
new file mode 100644
index 000..26c8d15
--- /dev/null
+++ b/docs/writing-qmp-commands.txt
@@ -0,0 +1,488 @@
+= How to write QMP commands using the QAPI framework =
+
+This document is a step-by-step guide on how to write new QMP commands using
+the QAPI framework. It also shows how to implement new style HMP commands,
+which do QMP calls.
+
+This document doesn't discuss QMP protocol level details, nor does it dive
+into the QAPI framework implementation.
+
+For an in-depth introduction to the QAPI framework, please refer to
+docs/qapi-code-gen.txt. For documentation about the QMP protocol, please
+check the files in QMP/.
+
+== Overview ==
+
+Generally speaking, the following steps should be taken in order to write a
+new QMP command.
+
+1. Write the command and type(s) specification in the QAPI schema file
+   (qapi-schema.json in the root directory)
+
+2. Write the QMP command itself, which is a regular C function. Preferably,
+   the command should be exported by some QEMU subsystem. But it can also be
+   added to the qmp.c file
+
+3. At this point the command can be tested under the QMP protocol
+
+4. Write the HMP command equivalent. This is not required and should only be
+   done if it does make sense to have the functionality in HMP. The HMP command
+   is implemented in terms of the QMP command
+
+The following sections will demonstrate each of the steps above. We will start
+very simple and get more complex as we progress.
+
+=== Testing ===
+
+For all the commands implementations in the next sections, the test setup is
+the same and is shown here.
+
+First, QEMU should be started as:
+
+# /path/to/your/source/qemu [...] \
+-chardev socket,id=qmp,port=,host=localhost,server \
+-mon chardev=qmp,mode=control,pretty=on
+
+Then, in a different terminal:
+
+$ telnet localhost 
+Trying 127.0.0.1...
+Connected to localhost.
+Escape character is '^]'.
+{
+"QMP": {
+"version": {
+"qemu": {
+"micro": 50, 
+"minor": 15, 
+"major": 0
+}, 
+"package": ""
+}, 
+"capabilities": [
+]
+}
+}
+
+The above output is the QMP server saying you're connected. The server is
+actually in capabilities negotiation mode. To enter in command mode type:
+
+{ "execute": "qmp_capabilities" }
+
+Then the server should respond:
+
+{
+"return": {
+}
+}
+
+Which is QMP way of saying "the latest command executed OK and didn't return
+any data". Now you're ready to enter the QMP example commands as suggested
+in the following sections.
+
+== Writing a command that doesn't return data ==
+
+That's the most simple QMP command that can be written. Usually, this kind of
+command carries some meaningful action in QEMU but here it will just print
+'Hello, world' to the standard output.
+
+Our command will be called 'hello-world'. It takes no arguments, nor does it
+return any data.
+
+The first step is to add the following line to the bottom of the
+qapi-schema.json file:
+
+{ 'command': 'hello-world' }
+
+This will instruct the QAPI to generate any prototypes and the necessary code
+to marshal and unmarshal protocol data.
+
+The next step is to write the 'hello-world' implementation. As explained
+earlier, it's preferable for commands to live in QEMU subsystems. But
+'hello-world' doesn't pertain to any, so we add this to qmp.c:
+
+void qmp_hello_world(Error **errp)
+{
+printf("Hello, world!\n");
+}
+
+There are a few things to be noted:
+
+1. QMP command implementation functions must be prefixed with "qmp_"
+2. qmp_hello_world() returns void, this is in accordance with the fact that the
+   command doesn't return any data
+3. It takes an 'Error **' argument. This is required. Later we will see how to
+   return errors and take additional arguments. The Error argument should not
+   be touched if the command doesn't return errors
+4. We won't add the function's prototype. That's automatically done by the QAPI
+5. Printing to the terminal is discouraged for QMP commands, we do it here
+   because it's the easiest way to demonstrate a QMP command
+
+Now a little hack is needed. As we're still using the old QMP server we need
+to add the new command to its internal dispatch table. This step won't be
+required in the near future. Open the qmp-commands.hx file and add the
+following in the botton:
+
+{
+

[Qemu-devel] QEMU 0.15.1 Linux

[Gus Zernial]

I have Kubuntu 11.10 Oneiric with standard 3.0.0-12-generic kernel on an AMD 
Phenom II X6 box.
I've been running Windows 7 guest under qemu 

$ /usr/bin/qemu-system-x86_64 -version
QEMU emulator version 0.14.1 (qemu-kvm-0.14.1), Copyright (c) 2003-2008 Fabrice 
Bellard

$ sudo /usr/bin/qemu-system-x86_64 -boot c -drive 
file=~/Win7.img,if=virtio -m 4096 -smp 1,cores-6 -net 
nic,vlan=0,macaddr=52:54:db:7f:0c:3d,model=virtio  -net tap -vga cirrus 
-soundhw hda -cdrom /dev/sr0

This boots the Windows 7 client, no problems

Then I installed qemu-0.15.1, compiled and installed from source and ran the 
same qemu startup command 
line with the new version

$ /usr/local/bin/qemu-system-x86_64 -version
QEMU emulator version 0.15.1, Copyright (c) 2003-2008 Fabrice Bellard

$ sudo /usr/local/bin/qemu-system-x86_64 -boot c -drive 
file=~/Win7.img,if=virtio -m 4096 -smp 1,cores-6 -net 
nic,vlan=0,macaddr=52:54:db:7f:0c:3d,model=virtio  -net tap -vga cirrus 
-soundhw hda -cdrom /dev/sr0

This results in Windows BSOD. I tried installing 
libvirt-bin_0.9.6~oneiric-2ubuntu5_amd64 to see if that would
help, but it didn't. It's possible there's some prerequisite or compile option 
for the new version, not sure?

Can someone tell me what's wrong and/or how to fix?


Thx, Gus

Re: [Qemu-devel] State of KVM guest debugging support on Power

[Stuart Yoder]

On Tue, Nov 1, 2011 at 9:22 AM, Jan Kiszka  wrote:
> Hi there,
>
> I'm generating some slides on guest debugging via kvm. What's the
> current state for Book-E and Book-S? Works out of box, mostly usable, or
> to be implemented? Is anyone using it?

Are you talking about guest debug using the QEMU stub or using
the virtual CPU's debug registers and interrupts?For Freescale
Book E we have both approaches working, and patches to be sent upstream
soon.

Stuart

Re: [Qemu-devel] State of KVM guest debugging support on Power

[Alexander Graf]

On 03.11.2011, at 11:59, Stuart Yoder  wrote:

> On Tue, Nov 1, 2011 at 9:22 AM, Jan Kiszka  wrote:
>> Hi there,
>> 
>> I'm generating some slides on guest debugging via kvm. What's the
>> current state for Book-E and Book-S? Works out of box, mostly usable, or
>> to be implemented? Is anyone using it?
> 
> Are you talking about guest debug using the QEMU stub or using
> the virtual CPU's debug registers and interrupts?For Freescale
> Book E we have both approaches working, and patches to be sent upstream
> soon.

For BookS I had played around with soft breakpoints for a bit, but I don't 
think I ever sent that upstream.

Alex

> 
> Stuart

Re: [Qemu-devel] State of KVM guest debugging support on Power

[Jan Kiszka]

On 2011-11-03 19:59, Stuart Yoder wrote:
> On Tue, Nov 1, 2011 at 9:22 AM, Jan Kiszka  wrote:
>> Hi there,
>>
>> I'm generating some slides on guest debugging via kvm. What's the
>> current state for Book-E and Book-S? Works out of box, mostly usable, or
>> to be implemented? Is anyone using it?
> 
> Are you talking about guest debug using the QEMU stub or using
> the virtual CPU's debug registers and interrupts?For Freescale
> Book E we have both approaches working, and patches to be sent upstream
> soon.

It's good to see both features coming into mainline.

I'll talk about the former, ie. debugging without guest help/awareness
(virtual hardware debugger). Is gdb well prepared for it (all registers
available, real-mode support, etc.)?

Thanks,
Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 3/5] linux-user: fake /proc/self/maps

[Alexander Graf]

Alexander Graf wrote:
> glibc's pthread_attr_getstack tries to find the stack range from
> /proc/self/maps. Unfortunately, /proc is usually the host's /proc
> which means linux-user guests see qemu's stack there.
>
> Fake the file with a constructed maps entry that exposes the guest's
> stack range.
>
> Signed-off-by: Alexander Graf 
> ---
>  linux-user/syscall.c |   15 +++
>  1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 38953ba..a51b457 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4600,6 +4600,20 @@ int get_osversion(void)
>  return osversion;
>  }
>  
> +
> +static int open_self_maps(void *cpu_env, int fd)
> +{
> +TaskState *ts = ((CPUState *)cpu_env)->opaque;
> +
> +dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0  [stack]\n",
> +(unsigned long long)ts->info->stack_limit,
> +(unsigned long long)(ts->stack_base + (TARGET_PAGE_SIZE - 1))
> + & TARGET_PAGE_MASK,
>   

This needs to be guarded by:

#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)

Any idea how to fetch the stack range from somewhere else for other archs?

Alex

Re: [Qemu-devel] [PATCH] os-posix: call system_powerdown on SIGTERM, enabling clean guest shutdown

[Bjørn Mork]

Anthony Liguori  writes:

> I don't think this is such a good idea.  SIGTERM shouldn't be subject
> to the guest's interpretations.

OK.  Just an idea.  Thanks for the feedback.

> Instead of doing a killall qemu, just send a system_powerdown command
> to each qemu's monitor instance.

Yes, I know I should.  I am just too lazy to create such scripts for
some of the more occasional guests.  This does of course reflect their
relative importance to me.



Bjørn

Re: [Qemu-devel] [PATCH v3 4/4] Add support for net bridge

[Corey Bryant]

On 11/02/2011 11:10 PM, Mark Wu wrote:

On 11/01/2011 02:36 AM, Corey Bryant wrote:

The default bridge that we attach to is br0. The thinking is that a
distro
could preconfigure such an interface to allow out-of-the-box bridged
networking.

Alternatively, if a user wants to use a different bridge, they can say:

qemu linux.img -net bridge,br=qemubr0 -net nic,model=virtio

or:

qemu linux.img -net
tap,helper=/usr/local/libexec/qemu-bridge-helper,br=qemubr0
-net nic,model=virtio


With this series of patches, I can also use "bridge" with the option
"-netdev". So do you think we need update the doc for 'netdev' too?
qemu-system-x86_64 ../rhel61.img --enable-kvm -m 1024 -netdev
bridge,br=virbr0,helper=/usr/local/libexec/qemu-bridge-helper,id=port1
-device virtio-net-pci,netdev=port1,id=nic1

Forgot to point out this in last post. Sorry for that.



Yes, I agree.  I'll do that in version 5 of the patch series.  Thanks 
for pointing that out.


Regards,
Corey

Re: [Qemu-devel] [RFC] docs: Add writing-qmp-commands.txt

[Alon Levy]

On Thu, Nov 03, 2011 at 04:36:03PM -0200, Luiz Capitulino wrote:
> Explains how to write QMP commands using the QAPI.
> 
> TODO:
> - write "returning lists" chapter
> - review it
> 
> Signed-off-by: Luiz Capitulino 
> ---
> 
> This is incomplete, but I figured I should send it anyway as there are people
> who want to add new QMP commands but are still using the old interface. Review
> is really appreciated.
> 
>  docs/writing-qmp-commands.txt |  488 
> +
>  1 files changed, 488 insertions(+), 0 deletions(-)
>  create mode 100644 docs/writing-qmp-commands.txt
> 
> diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
> new file mode 100644
> index 000..26c8d15
> --- /dev/null
> +++ b/docs/writing-qmp-commands.txt
> @@ -0,0 +1,488 @@
> += How to write QMP commands using the QAPI framework =
> +
> +This document is a step-by-step guide on how to write new QMP commands using
> +the QAPI framework. It also shows how to implement new style HMP commands,
> +which do QMP calls.
> +
> +This document doesn't discuss QMP protocol level details, nor does it dive
> +into the QAPI framework implementation.
> +
> +For an in-depth introduction to the QAPI framework, please refer to
> +docs/qapi-code-gen.txt. For documentation about the QMP protocol, please
> +check the files in QMP/.
> +
> +== Overview ==
> +
> +Generally speaking, the following steps should be taken in order to write a
> +new QMP command.
> +
> +1. Write the command and type(s) specification in the QAPI schema file
> +   (qapi-schema.json in the root directory)
> +
> +2. Write the QMP command itself, which is a regular C function. Preferably,
> +   the command should be exported by some QEMU subsystem. But it can also be
> +   added to the qmp.c file
> +
> +3. At this point the command can be tested under the QMP protocol
> +
> +4. Write the HMP command equivalent. This is not required and should only be
> +   done if it does make sense to have the functionality in HMP. The HMP 
> command
> +   is implemented in terms of the QMP command
> +
> +The following sections will demonstrate each of the steps above. We will 
> start
> +very simple and get more complex as we progress.
> +
> +=== Testing ===
> +
> +For all the commands implementations in the next sections, the test setup is
> +the same and is shown here.
> +
> +First, QEMU should be started as:
> +
> +# /path/to/your/source/qemu [...] \
> +-chardev socket,id=qmp,port=,host=localhost,server \
> +-mon chardev=qmp,mode=control,pretty=on
> +
> +Then, in a different terminal:
> +
> +$ telnet localhost 
> +Trying 127.0.0.1...
> +Connected to localhost.
> +Escape character is '^]'.
> +{
> +"QMP": {
> +"version": {
> +"qemu": {
> +"micro": 50, 
> +"minor": 15, 
> +"major": 0
> +}, 
> +"package": ""
> +}, 
> +"capabilities": [
> +]
> +}
> +}
> +
> +The above output is the QMP server saying you're connected. The server is
> +actually in capabilities negotiation mode. To enter in command mode type:
> +
> +{ "execute": "qmp_capabilities" }
> +
> +Then the server should respond:
> +
> +{
> +"return": {
> +}
> +}
> +
> +Which is QMP way of saying "the latest command executed OK and didn't return
> +any data". Now you're ready to enter the QMP example commands as suggested
> +in the following sections.
> +
> +== Writing a command that doesn't return data ==
> +
> +That's the most simple QMP command that can be written. Usually, this kind of
> +command carries some meaningful action in QEMU but here it will just print
> +'Hello, world' to the standard output.
> +
> +Our command will be called 'hello-world'. It takes no arguments, nor does it
> +return any data.
> +
> +The first step is to add the following line to the bottom of the
> +qapi-schema.json file:
> +
> +{ 'command': 'hello-world' }
> +
> +This will instruct the QAPI to generate any prototypes and the necessary code
> +to marshal and unmarshal protocol data.
> +
> +The next step is to write the 'hello-world' implementation. As explained
> +earlier, it's preferable for commands to live in QEMU subsystems. But
> +'hello-world' doesn't pertain to any, so we add this to qmp.c:
> +
> +void qmp_hello_world(Error **errp)
> +{
> +printf("Hello, world!\n");
> +}
> +
> +There are a few things to be noted:
> +
> +1. QMP command implementation functions must be prefixed with "qmp_"
> +2. qmp_hello_world() returns void, this is in accordance with the fact that 
> the
> +   command doesn't return any data
> +3. It takes an 'Error **' argument. This is required. Later we will see how 
> to
> +   return errors and take additional arguments. The Error argument should not
> +   be touched if the command doesn't return errors
> +4. We won't add the function's prototype. That's automatically done by the 
> QAPI
> +5. Printing to the terminal is discoura

Re: [Qemu-devel] [RFC] docs: Add writing-qmp-commands.txt

[Michael Roth]

On 11/03/2011 01:36 PM, Luiz Capitulino wrote:

Explains how to write QMP commands using the QAPI.

TODO:
 - write "returning lists" chapter
 - review it

Signed-off-by: Luiz Capitulino
---

This is incomplete, but I figured I should send it anyway as there are people
who want to add new QMP commands but are still using the old interface. Review
is really appreciated.

  docs/writing-qmp-commands.txt |  488 +
  1 files changed, 488 insertions(+), 0 deletions(-)
  create mode 100644 docs/writing-qmp-commands.txt

diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
new file mode 100644
index 000..26c8d15
--- /dev/null
+++ b/docs/writing-qmp-commands.txt
@@ -0,0 +1,488 @@
+= How to write QMP commands using the QAPI framework =
+
+This document is a step-by-step guide on how to write new QMP commands using
+the QAPI framework. It also shows how to implement new style HMP commands,
+which do QMP calls.
+
+This document doesn't discuss QMP protocol level details, nor does it dive
+into the QAPI framework implementation.
+
+For an in-depth introduction to the QAPI framework, please refer to
+docs/qapi-code-gen.txt. For documentation about the QMP protocol, please
+check the files in QMP/.
+
+== Overview ==
+
+Generally speaking, the following steps should be taken in order to write a
+new QMP command.
+
+1. Write the command and type(s) specification in the QAPI schema file
+   (qapi-schema.json in the root directory)
+
+2. Write the QMP command itself, which is a regular C function. Preferably,
+   the command should be exported by some QEMU subsystem. But it can also be
+   added to the qmp.c file
+
+3. At this point the command can be tested under the QMP protocol
+
+4. Write the HMP command equivalent. This is not required and should only be
+   done if it does make sense to have the functionality in HMP. The HMP command
+   is implemented in terms of the QMP command
+
+The following sections will demonstrate each of the steps above. We will start
+very simple and get more complex as we progress.
+
+=== Testing ===
+
+For all the commands implementations in the next sections, the test setup is
+the same and is shown here.
+
+First, QEMU should be started as:
+
+# /path/to/your/source/qemu [...] \
+-chardev socket,id=qmp,port=,host=localhost,server \
+-mon chardev=qmp,mode=control,pretty=on
+
+Then, in a different terminal:
+
+$ telnet localhost 
+Trying 127.0.0.1...
+Connected to localhost.
+Escape character is '^]'.
+{
+"QMP": {
+"version": {
+"qemu": {
+"micro": 50,
+"minor": 15,
+"major": 0
+},
+"package": ""
+},
+"capabilities": [
+]
+}
+}
+
+The above output is the QMP server saying you're connected. The server is
+actually in capabilities negotiation mode. To enter in command mode type:
+
+{ "execute": "qmp_capabilities" }
+
+Then the server should respond:
+
+{
+"return": {
+}
+}
+
+Which is QMP way of saying "the latest command executed OK and didn't return
+any data". Now you're ready to enter the QMP example commands as suggested
+in the following sections.
+
+== Writing a command that doesn't return data ==
+
+That's the most simple QMP command that can be written. Usually, this kind of
+command carries some meaningful action in QEMU but here it will just print
+'Hello, world' to the standard output.
+
+Our command will be called 'hello-world'. It takes no arguments, nor does it
+return any data.
+
+The first step is to add the following line to the bottom of the
+qapi-schema.json file:
+
+{ 'command': 'hello-world' }
+
+This will instruct the QAPI to generate any prototypes and the necessary code
+to marshal and unmarshal protocol data.
+
+The next step is to write the 'hello-world' implementation. As explained
+earlier, it's preferable for commands to live in QEMU subsystems. But
+'hello-world' doesn't pertain to any, so we add this to qmp.c:
+
+void qmp_hello_world(Error **errp)
+{
+printf("Hello, world!\n");
+}
+
+There are a few things to be noted:
+
+1. QMP command implementation functions must be prefixed with "qmp_"
+2. qmp_hello_world() returns void, this is in accordance with the fact that the
+   command doesn't return any data
+3. It takes an 'Error **' argument. This is required. Later we will see how to
+   return errors and take additional arguments. The Error argument should not
+   be touched if the command doesn't return errors
+4. We won't add the function's prototype. That's automatically done by the QAPI
+5. Printing to the terminal is discouraged for QMP commands, we do it here
+   because it's the easiest way to demonstrate a QMP command
+
+Now a little hack is needed. As we're still using the old QMP server we need
+to add the new command to its internal dispatch table. This step won't be
+required in the near future. Open the qmp-commands.hx file and

Re: [Qemu-devel] QEMU 0.15.1 Linux

[Jernej Simončič]

On Thursday, November 3, 2011, 19:59:37, Gus Zernial wrote:

> $ /usr/bin/qemu-system-x86_64 -version
> QEMU emulator version 0.14.1 (qemu-kvm-0.14.1), Copyright (c) 2003-2008 
> Fabrice Bellard
 ^^^
This part is important - you had qemu-kvm, which is slightly different
from...

> $ /usr/local/bin/qemu-system-x86_64 -version
> QEMU emulator version 0.15.1, Copyright (c) 2003-2008 Fabrice Bellard

...plain qemu you compiled. Download qemu-kvm from


-- 
< Jernej Simončič ><><><><>< http://eternallybored.org/ >

If it should exist, it doesn't.
   -- Arnold's First Law of Documentation

Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?

[Thomas Schmitt]

Hi,

i could track down the reason for sense code B 00 06 to an ioctl(SG_IO)
which returns -1.  errno is 1.
The host system has in /usr/include/asm-generic/errno-base.h
  #define EPERM1  /* Operation not permitted */

The user who runs qemu is able to perform e.g. 
  PREVENT/ALLOW MEDIA REMOVAL
  1e 00 00 00 00 00 
via ioctl(SG_IO) on /dev/sg1.
So it can hardly be the permissions of the device file:
  crw-rw 1 root cdrom 21, 1 Nov  2 12:44 /dev/sg1
The user is member of group cdrom.

For today, i close my shop. Maybe i get an idea tomorrow, how
to further investigate the problem.

-

B 00 06 comes from hw/scsi-generic.c:scsi_command_complete()

  static void scsi_command_complete(void *opaque, int ret)
  {
  ...
  if (ret != 0) {
  switch (ret) {
  ...
  default:
  status = CHECK_CONDITION;
  scsi_req_build_sense(&r->req, SENSE_CODE(IO_ERROR));

SENSE_CODE is defined in hw/scsi.h
  #define SENSE_CODE(x) sense_code_ ## x

so that SENSE_CODE(IO_ERROR) yields this struct from hw/scsi-bus.c
  /* Command aborted, I/O process terminated */
  const struct SCSISense sense_code_IO_ERROR = {
  .key = ABORTED_COMMAND, .asc = 0x00, .ascq = 0x06
  };


I tried to activate DPRINTF in hw/scsi-generic.c by removing the "//"
before
  #define DEBUG_SCSI

make yields:
  .../hw/scsi-generic.c: In function 'scsi_send_command':
  .../hw/scsi-generic.c:286: error: 'lun' undeclared (first use in this 
function)
  .../hw/scsi-generic.c:286: error: 'tag' undeclared (first use in this 
function)

So i had to change in scsi_send_command():

  - DPRINTF("Command: lun=%d tag=0x%x len %zd data=0x%02x", lun, tag,
  + DPRINTF("Command: len %zd data=0x%02x",

Further i added DPRINTFs to the suspected thrower of B 00 06:

if (ret < 0) {
DPRINTF("scsi_send_command: calling scsi_command_complete(%d)\n", 
ret);
scsi_command_complete(r, ret);
return 0;
}
DPRINTF("scsi_send_command: returning 0\n");

This yields with boot-time command
  1e 00 00 00 00 00 
in the qemu start terminal:

  scsi-generic: Command: len 0 data=0x1e 0x00 0x00 0x00 0x00 0x00
  scsi-generic: scsi_send_command: returning 0
  scsi-generic: scsi_command_complete: ret= -1

I.e. it went through scsi_send_command() and got 0 as return value from
  ret = execute_command(s->conf.bs, r, SG_DXFER_NONE, scsi_command_complete);
Nevertheless there happens a call to scsi_command_complete() with ret==-1.

I added more DPRINTF and printf. 
It turns out that the offending call comes from
  posix-aio-compat.c: posix_aio_process_queue
after the ioctl(SG_IO) failed.

My print points report:
  scsi-generic: Command: len 0 data=0x1e 0x00 0x00 0x00 0x00 0x00
  block.c: bdrv_aio_ioctl: drv= 0x961f00 , drv->bdrv_aio_ioctl= 0x42bc30
  block.c: bdrv_aio_ioctl: drv= 0x9619e0 , drv->bdrv_aio_ioctl= 0x42b2d0
  block/raw-posix.c: hdev_aio_ioctl: calling paio_ioctl()
  scsi-generic: scsi_send_command: returning 0
  posix-aio-compat.c: handle_aiocb_ioctl: ioctl(12, 0x2285, ), ret= -1 , errno= 
1
  posix-aio-compat.c: qemu_paio_error: ret= -1
  posix-aio-compat.c: qemu_paio_error: returning 1
  posix-aio-compat.c: posix_aio_process_queue: ret= 1 -> -1
  posix-aio-compat.c: posix_aio_process_queue: calling acb->common.cb(,-1)
  scsi-generic: scsi_command_complete: ret= -1
  scsi-generic: Command complete 0x0x16c0e60 tag=0x0 status=2

ioctl command 0x2285 is defined in /usr/include/scsi/sg.h of host:
  #define SG_IO 0x2285

-

Since i inserted the last few of the new printf() calls, i occasionaly
suffer qemu abort by SIGSEGV during guest boot. It happens before the
first DPRINTF happens in scsi_send_command(). 

But "git diff" reveils no changes towards the original state, which
would be to blame.
The printf format codes and the types match good enough to cause
no warnings at compile time.

Is it a known problem, that adding DPRINTF() or printf() can cause this ?
Or am i just too tired to see my fault ?

+printf("block.c: bdrv_aio_ioctl: drv= 0x%lx , drv->bdrv_aio_ioctl= 0x%lx\n"
,
+   (unsigned long) drv,
+   (unsigned long) (drv ? drv->bdrv_aio_ioctl : NULL));

+printf("block/raw-posix.c: hdev_aio_ioctl: calling paio_ioctl()\n");

+DPRINTF("scsi_command_complete: ret= %d\n", ret);

+DPRINTF("scsi_read_data: calling scsi_command_complete(%d)\n", ret);

+DPRINTF("scsi_write_complete: calling scsi_command_complete(%d)\n", ret);

+DPRINTF("scsi_write_data: calling scsi_command_complete(%d)\n",
+ret);

+DPRINTF("Command: len %zd data=0x%02x",
 r->req.cmd.xfer, cmd[0]);

+DPRINTF("scsi_send_command: calling scsi_command_complete(%d)\n",
+ret);

+DPRINTF("scsi_send_command: returning 0\n");

+

Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions

[David Gibson]

On Thu, Nov 03, 2011 at 02:25:45PM +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 03, 2011 at 04:16:34PM +1100, David Gibson wrote:
> > On Wed, Nov 02, 2011 at 09:16:34AM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Oct 31, 2011 at 05:06:49PM +1100, David Gibson wrote:
> > > > From: Eduard - Gabriel Munteanu 
> > [snip]
> > > > @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
> > > >   * values which really matter.
> > > >   * Number of data should check configuration!!!
> > > >   */
> > > > -cpu_physical_memory_write(s->statsaddr, &s->statistics, 
> > > > s->stats_size);
> > > > -e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
> > > > -e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
> > > > -e100_stl_le_phys(s->statsaddr + 48, 
> > > > s->statistics.rx_resource_errors);
> > > > -e100_stl_le_phys(s->statsaddr + 60, 
> > > > s->statistics.rx_short_frame_errors);
> > > > +pci_dma_write(&s->dev, s->statsaddr,
> > > > +  (uint8_t *) &s->statistics, s->stats_size);
> > > > +stl_le_pci_dma(&s->dev, s->statsaddr + 0,
> > > > +   s->statistics.tx_good_frames);
> > > > +stl_le_pci_dma(&s->dev, s->statsaddr + 36,
> > > > +   s->statistics.rx_good_frames);
> > > > +stl_le_pci_dma(&s->dev, s->statsaddr + 48,
> > > > +   s->statistics.rx_resource_errors);
> > > > +stl_le_pci_dma(&s->dev, s->statsaddr + 60,
> > > > +   s->statistics.rx_short_frame_errors);
> > > 
> > > This might introduce a bug: stlXX APIs assume aligned addresses,
> > > an address in statsaddr is user-controlled so I'm not sure
> > > it's always aligned.
> > > 
> > > Why isn't the patch simply replacing cpu_physical_memory_read
> > > with pci_XXX ? Any cleanups should be done separately.
> > 
> > Because it seemed like a good idea at the time.  When I first wrote
> > this, the possibility of unaligned addresses wasn't obvious to me.
> > So, I'm working on fixing this now.  I can take one of two approaches:
> > 
> >  - Simply revert this part of the change, reinstate the e100_stl
> > functions as calling into dma_write().
> > 
> >  - Alter the stX_dma() functions to work for unaligned addresses (by
> > falling back to dma_rw() in that case).  This is a little more
> > involved but might make device writing safer in future.
> 
> Yes but then we lose the atomicity guarantee. So this might
> still result in subtle emulation bugs.

Sorry, I should have been clearer - I was planning to fall back to
dma_rw() *only* for unaligned addresses.  Aligned addresses would
still have the atomicity guarantee.

> > Anthony, Michael, any preferred direction here?
> 
> For 1.0 I'd go for option 1 as the simplest.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

[Qemu-devel] [PATCH][ARM] Fix C15 FSR (C5) domain setting

[Jean-Christophe DUBOIS]

During Xvisor development, it was noted that qemu did not return
the correct domain value in the C15 [Data] FSR register (C5).

This patch is a proposal to fix it.

Signed-off-by: Jean-Christophe DUBOIS
---

--- qemu-0.15.1.org/target-arm/helper.c2011-10-12 18:41:43.0 
+0200

+++ qemu-0.15.1/target-arm/helper.c2011-11-04 00:49:03.247062518 +0100
@@ -924,12 +924,12 @@
 /* Check section/page access permissions.
Returns the page protection flags, or zero if the access is not
permitted.  */
-static inline int check_ap(CPUState *env, int ap, int domain, int 
access_type,
+static inline int check_ap(CPUState *env, int ap, int domain_prot, int 
access_type,

int is_user)
 {
   int prot_ro;

-  if (domain == 3)
+  if (domain_prot == 3)
 return PAGE_READ | PAGE_WRITE;

   if (access_type == 1)
@@ -996,6 +996,7 @@
 int type;
 int ap;
 int domain;
+int domain_prot;
 uint32_t phys_addr;

 /* Pagetable walk.  */
@@ -1003,13 +1004,14 @@
 table = get_level1_table_address(env, address);
 desc = ldl_phys(table);
 type = (desc & 3);
-domain = (env->cp15.c3 >> ((desc >> 4) & 0x1e)) & 3;
+domain = (desc >> 5) & 0x0f;
+domain_prot = (env->cp15.c3 >> (domain * 2)) & 3;
 if (type == 0) {
 /* Section translation fault.  */
 code = 5;
 goto do_fault;
 }
-if (domain == 0 || domain == 2) {
+if (domain_prot == 0 || domain_prot == 2) {
 if (type == 2)
 code = 9; /* Section domain fault.  */
 else
@@ -1067,7 +1069,7 @@
 }
 code = 15;
 }
-*prot = check_ap(env, ap, domain, access_type, is_user);
+*prot = check_ap(env, ap, domain_prot, access_type, is_user);
 if (!*prot) {
 /* Access permission fault.  */
 goto do_fault;
@@ -1090,6 +1092,7 @@
 int type;
 int ap;
 int domain;
+int domain_prot;
 uint32_t phys_addr;

 /* Pagetable walk.  */
@@ -1107,10 +1110,10 @@
 domain = 0;
 } else {
 /* Section or page.  */
-domain = (desc >> 4) & 0x1e;
+domain = (desc >> 5) & 0x0f;
 }
-domain = (env->cp15.c3 >> domain) & 3;
-if (domain == 0 || domain == 2) {
+domain_prot = (env->cp15.c3 >> (domain * 2)) & 3;
+if (domain_prot == 0 || domain_prot == 2) {
 if (type == 2)
 code = 9; /* Section domain fault.  */
 else
@@ -1155,7 +1158,7 @@
 }
 code = 15;
 }
-if (domain == 3) {
+if (domain_prot == 3) {
 *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 } else {
 if (xn && access_type == 2)
@@ -1167,7 +1170,7 @@
 code = (code == 15) ? 6 : 3;
 goto do_fault;
 }
-*prot = check_ap(env, ap, domain, access_type, is_user);
+*prot = check_ap(env, ap, domain_prot, access_type, is_user);
 if (!*prot) {
 /* Access permission fault.  */
 goto do_fault;

[Qemu-devel] [PATCH 2/7] Remove unnecessary casts from PCI DMA code in e1000

[David Gibson]

This patch removes some unnecessary casts in the e1000 device,
introduced by commit 62ecbd353d25e62c4a6c327ea88ba5404e13507a 'e1000:
Use PCI DMA stub functions'.

Signed-off-by: David Gibson 
---
 hw/e1000.c |   11 +--
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 986ed9c..a29c944 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -507,7 +507,7 @@ txdesc_writeback(E1000State *s, dma_addr_t base, struct 
e1000_tx_desc *dp)
 ~(E1000_TXD_STAT_EC | E1000_TXD_STAT_LC | E1000_TXD_STAT_TU);
 dp->upper.data = cpu_to_le32(txd_upper);
 pci_dma_write(&s->dev, base + ((char *)&dp->upper - (char *)dp),
-  (void *)&dp->upper, sizeof(dp->upper));
+  &dp->upper, sizeof(dp->upper));
 return E1000_ICR_TXDW;
 }
 
@@ -534,7 +534,7 @@ start_xmit(E1000State *s)
 while (s->mac_reg[TDH] != s->mac_reg[TDT]) {
 base = tx_desc_base(s) +
sizeof(struct e1000_tx_desc) * s->mac_reg[TDH];
-pci_dma_read(&s->dev, base, (void *)&desc, sizeof(desc));
+pci_dma_read(&s->dev, base, &desc, sizeof(desc));
 
 DBGOUT(TX, "index %d: %p : %x %x\n", s->mac_reg[TDH],
(void *)(intptr_t)desc.buffer_addr, desc.lower.data,
@@ -714,7 +714,7 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, 
size_t size)
 desc_size = s->rxbuf_size;
 }
 base = rx_desc_base(s) + sizeof(desc) * s->mac_reg[RDH];
-pci_dma_read(&s->dev, base, (void *)&desc, sizeof(desc));
+pci_dma_read(&s->dev, base, &desc, sizeof(desc));
 desc.special = vlan_special;
 desc.status |= (vlan_status | E1000_RXD_STAT_DD);
 if (desc.buffer_addr) {
@@ -724,8 +724,7 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, 
size_t size)
 copy_size = s->rxbuf_size;
 }
 pci_dma_write(&s->dev, le64_to_cpu(desc.buffer_addr),
- (void *)(buf + desc_offset + vlan_offset),
- copy_size);
+  buf + desc_offset + vlan_offset, copy_size);
 }
 desc_offset += desc_size;
 desc.length = cpu_to_le16(desc_size);
@@ -739,7 +738,7 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, 
size_t size)
 } else { // as per intel docs; skip descriptors with null buf addr
 DBGOUT(RX, "Null RX descriptor!!\n");
 }
-pci_dma_write(&s->dev, base, (void *)&desc, sizeof(desc));
+pci_dma_write(&s->dev, base, &desc, sizeof(desc));
 
 if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN])
 s->mac_reg[RDH] = 0;
-- 
1.7.7

[Qemu-devel] Novatis, agence web innovatrice

[Votre agence Web]

Agence web multidisciplinaire spécialisée en solutions interactives,
conception de site Internet, e-commerce, multimédia, animation flash,
stratégie marketing Internet et design graphique en tous genres.

Basée sur les valeurs de marques et parce qu'elle intègre les dimensions
d'images et d'opinions, la démarche de NOVATIS donne à la communication
des entreprises et des institutions davantage de sens et de cohérence
pour, au final, leur permettre de construire une célébrité plus
pérenne.

Pendant une période très courte, NOVATIS a réussi de faire une bonne
réputation en Tunisie ainsi qu'en France et de gagner vos confiances.
Avec nos compétences, notre dynamisme et notre créativité, nous serons
toujours votre soutien pour mettre vos rêves en réalité.

De la réservation de votre nom de domaine à la gestion de votre site, de
l'hébergement mutualisé aux serveurs 


Plus d'informations, visitez  notre site



www.novatis.org

[Qemu-devel] [PATCH 4/7] Remove unnecessary casts from PCI DMA code in lsi53c895a

[David Gibson]

This patch removes some unnecessary casts in the lsi53c895a device,
introduced by commit 9ba4524cda1348cbe741535f77815dca6a57da05
'lsi53c895a: Use PCI DMA stub functions'.

Signed-off-by: David Gibson 
---
 hw/lsi53c895a.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index fcc27d7..0d3a101 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -392,7 +392,7 @@ static inline uint32_t read_dword(LSIState *s, uint32_t 
addr)
 {
 uint32_t buf;
 
-pci_dma_read(&s->dev, addr, (uint8_t *)&buf, 4);
+pci_dma_read(&s->dev, addr, &buf, 4);
 return cpu_to_le32(buf);
 }
 
@@ -1079,7 +1079,7 @@ again:
 
 /* 32-bit Table indirect */
 offset = sxt24(addr);
-pci_dma_read(&s->dev, s->dsa + offset, (uint8_t *)buf, 8);
+pci_dma_read(&s->dev, s->dsa + offset, buf, 8);
 /* byte count is stored in bits 0:23 only */
 s->dbc = cpu_to_le32(buf[0]) & 0xff;
 s->rbc = s->dbc;
-- 
1.7.7

[Qemu-devel] [PATCH 1/7] Remove unnecessary casts from PCI DMA code in eepro100

[David Gibson]

This patch removes some unnecessary casts in the eepro100 device,
introduced by commit 16ef60c9a8269f7cbc95219a431b1d7cbf29
'eepro100: Use PCI DMA stub functions'.

Signed-off-by: David Gibson 
---
 hw/eepro100.c |7 +++
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 7d59e71..8769e33 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -713,8 +713,7 @@ static void dump_statistics(EEPRO100State * s)
  * values which really matter.
  * Number of data should check configuration!!!
  */
-pci_dma_write(&s->dev, s->statsaddr,
-  (uint8_t *) &s->statistics, s->stats_size);
+pci_dma_write(&s->dev, s->statsaddr, &s->statistics, s->stats_size);
 stl_le_pci_dma(&s->dev, s->statsaddr + 0,
s->statistics.tx_good_frames);
 stl_le_pci_dma(&s->dev, s->statsaddr + 36,
@@ -732,7 +731,7 @@ static void dump_statistics(EEPRO100State * s)
 
 static void read_cb(EEPRO100State *s)
 {
-pci_dma_read(&s->dev, s->cb_address, (uint8_t *) &s->tx, sizeof(s->tx));
+pci_dma_read(&s->dev, s->cb_address, &s->tx, sizeof(s->tx));
 s->tx.status = le16_to_cpu(s->tx.status);
 s->tx.command = le16_to_cpu(s->tx.command);
 s->tx.link = le32_to_cpu(s->tx.link);
@@ -1707,7 +1706,7 @@ static ssize_t nic_receive(VLANClientState *nc, const 
uint8_t * buf, size_t size
 /* !!! */
 eepro100_rx_t rx;
 pci_dma_read(&s->dev, s->ru_base + s->ru_offset,
- (uint8_t *) &rx, sizeof(eepro100_rx_t));
+ &rx, sizeof(eepro100_rx_t));
 uint16_t rfd_command = le16_to_cpu(rx.command);
 uint16_t rfd_size = le16_to_cpu(rx.size);
 
-- 
1.7.7

[Qemu-devel] [PATCH 5/7] Remove unnecessary casts from PCI DMA code in rtl8139

[David Gibson]

This patch removes some unnecessary casts in the rtl8139 device,
introduced by commit 3ada003aee2004d24f23b9cd6f4eda87d9601ddb
'rtl8139: Use PCI DMA stub functions'.

Signed-off-by: David Gibson 
---
 hw/rtl8139.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 4c37993..30a7eb6 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -991,13 +991,13 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, 
const uint8_t *buf, size_
 
 uint32_t val, rxdw0,rxdw1,rxbufLO,rxbufHI;
 
-pci_dma_read(&s->dev, cplus_rx_ring_desc, (uint8_t *)&val, 4);
+pci_dma_read(&s->dev, cplus_rx_ring_desc, &val, 4);
 rxdw0 = le32_to_cpu(val);
-pci_dma_read(&s->dev, cplus_rx_ring_desc+4, (uint8_t *)&val, 4);
+pci_dma_read(&s->dev, cplus_rx_ring_desc+4, &val, 4);
 rxdw1 = le32_to_cpu(val);
-pci_dma_read(&s->dev, cplus_rx_ring_desc+8, (uint8_t *)&val, 4);
+pci_dma_read(&s->dev, cplus_rx_ring_desc+8, &val, 4);
 rxbufLO = le32_to_cpu(val);
-pci_dma_read(&s->dev, cplus_rx_ring_desc+12, (uint8_t *)&val, 4);
+pci_dma_read(&s->dev, cplus_rx_ring_desc+12, &val, 4);
 rxbufHI = le32_to_cpu(val);
 
 DPRINTF("+++ C+ mode RX descriptor %d %08x %08x %08x %08x\n",
-- 
1.7.7

Re: [Qemu-devel] [PATCH 1.0] add sgabios blob and submodule

[Zhi Yong Wu]

On Thu, Nov 3, 2011 at 10:14 PM, Paolo Bonzini  wrote:
> The rom was not added together with the sgabios device and is
> not installed.
>
> Signed-off-by: Paolo Bonzini 
> ---
>        The sgabios.git mirror repository can be fetched from
>        http://people.redhat.com/pbonzini/sgabios-git.tgz
>
>  .gitmodules         |    3 +++
>  Makefile            |    2 +-
>  pc-bios/README      |    6 ++
>  pc-bios/sgabios.bin |  Bin 0 -> 4096 bytes
>  roms/sgabios        |    1 +
Sorry, what are sgabios and sgabios.bin separately? What is the relationship?

>  5 files changed, 11 insertions(+), 1 deletions(-)
>  create mode 100755 pc-bios/sgabios.bin
>  create mode 16 roms/sgabios
>
> diff --git a/.gitmodules b/.gitmodules
> index 2a43dbc..eca876f 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -16,3 +16,6 @@
>  [submodule "roms/qemu-palcode"]
>        path = roms/qemu-palcode
>        url = git://repo.or.cz/qemu-palcode.git
> +[submodule "roms/sgabios"]
> +       path = roms/sgabios
> +       url = git://git.qemu.org/sgabios.git
> diff --git a/Makefile b/Makefile
> index 4f6eaa4..168093c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -253,7 +253,7 @@ ar      de     en-us  fi  fr-be  hr     it  lv  nl        
>  pl  ru     th \
>  common  de-ch  es     fo  fr-ca  hu     ja  mk  nl-be      pt  sl     tr
>
>  ifdef INSTALL_BLOBS
> -BLOBS=bios.bin vgabios.bin vgabios-cirrus.bin \
> +BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
>  vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
>  ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \
>  pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
> diff --git a/pc-bios/README b/pc-bios/README
> index 0668559..1cebbbc 100644
> --- a/pc-bios/README
> +++ b/pc-bios/README
> @@ -19,6 +19,12 @@
>   https://github.com/dgibson/SLOF, and the image currently in qemu is
>   built from git tag qemu-slof-20111013.
>
> +- sgabios (the Serial Graphics Adapter option ROM) provides a means for
> +  legacy x86 software to communicate with an attached serial console as
> +  if a video card were attached.  The master sources reside in a subversion
> +  repository at http://sgabios.googlecode.com/svn/trunk.  A git mirror is
> +  available at git://git.qemu.org/sgabios.git.
> +
>  - The PXE roms come from the iPXE project. Built with BANNER_TIME 0.
>   Sources available at http://ipxe.org.  Vendor:Device ID -> ROM mapping:
>
> diff --git a/pc-bios/sgabios.bin b/pc-bios/sgabios.bin
> new file mode 100755
> index 
> ..c3da4c3d0a34e4eca9b0cda38d0538b332f7b09e
> GIT binary patch
> literal 4096
> zcmeHJeRNah8Gmw z(goPP4Wd)$$L3rSH#>(K=NZ!#YiO_$Hpho3C>|*k=364AM}*OQ+`H$d;Erd1?wtL_
> zdrscx_dM_O{NCq%?)%)_<*#K9x2T8LsLwZz!svOLz$lClLt13p+N?4Pp z{vin~r3n_cwXJ=mDc?}j+DL<=$=mFH#IVroUcaunvE5Ly#=YLx ztf;K9ItsPn6{ z+h8yh8O%i`MMWjE&4%U6stkoDB(vQ%jWN$FoL%gl#^5ZWMbnt#R#?{LeYC^?)s^!r
> zP@KOsf(J`abH9R7@%saGCfacG^t&~;X5VAH(|p_Q+R6GUQ)m95;Ff$@-n5@@pZl|(
> zEnBy7IbA{i@uI9*vu~p0iVUSHbAtNSd*7Zo>(7CgLT_|Gwe#g&&%N@y-~Z{Y=U;g8
> z>EAx{?3=%M{g=D%kmB#V#y>;iFGT)H-|_$IkH7wd2FlAf zhB8ZSLBobBw#kj7v8k!4xw*NbqS{^2*4D zGiJ=#gd;Cc`bndpU{gU_NrM6$ z%x6PuVXgrblv0)&4
> z(K9!L)zW+!O&LAG>I~YmE02WU@_&ZX!e`URplc_v)&RTzVJbtb^y
> zKF2cAH_MXAswSND zQ`+do@f>$*AVA7@giGOh@BQ&zsL_)_w&O$}D71UB{0iB=(`uU6m?*QDyz~kk)R`zx
> zRE2jGq6-zEh%Uq{j~62G1{rfZ@mh`wT@r?XUYW%G@}l}aPm2{P@T$dN(PozWQu
> zN)|Qpw^ZR_WGrPqnT|5@E0yUo@aF~=XyCs&8^7bMC$5Z?#Ji+j(HQ|5f8xmCI4ggc
> z|7=M8&70D3=Vth(rkOjVf*dP!-OAq4c@Zc&4>LV-k(Q&tSedaagIl1H5}3y@6)?|g
> zgL#bE+yNGbsRWC)4uQF_14a+C}WuUP-b-?Fwg3M2N znD6g^a_e%aV9awmpu)NWDjDtQQXh^$+!F4)Et
> zBB7nZJsk|%mRRS*65MPdgRTpiMPOxc-O4P0Mb-*fggM)+m0-hUYZ% ziur=g?x2HHbErVS%oH98^i!dyN79B~8E_{?xT}@pYM@^pnvCnRko>xV3du$omosuL
> zKI3bP7_mFZMbT&Mk@@aUo4@YyvlqC2okrgavg7*f&D?(Ouw^q#g)+B>8CRsveBkT_
> z@}YQotUj{bQD^hCfcXG_j{H;fm2nF|*jMczQ}{B-ry@Jr`>mWmM|a&@m3h`To$i`h
> zm3hH;^PE;-<<+s#ah<&y`@$6F&+!Wsf&B_+J+m@mv)5q`Q@H)_j*7$~_83l&af9jH
> zM(Qzn2d38J_bIbC_jb#UTMX@rUOy(vL@X+w){^PF$)fhDu&d3}iDe{pRQ<8g9
> z`F3HGw$}*8vWFGb9XNG+_#E~SAix{fI&@2p{grSp!{+I?m)WX4-QW%I)rlWMp
> z)L!ta=q?n(LAs=PZ(2hk{DZVnI_w&LY8L2S5!;I7?k!=gK(QFR(%?+%q(eLtm#_mD
> z$&ba|T!eVx%(o4UWILwjBiz+vYA(tRN|9M65*IR)J{5Nr;YEQvfr1E zb3sz3k)uM0As z(&&vPIuH0P{)CFnz`$4m_eHOP;s&!-$GLuPFiaH3xq;4pT@KkPv91-uXBFD#!;ev+
> zsiZ~>JxZ1fF2=|qiBqMIycl046@k2wvP@&;WGEHX-3h!Z5TOEzGOxT>2Rs_2cg*mL
> zv0>-fD*jyHgGqQXGW znHr`l*{lKO%>eic!!tdpciTyR%5>Is$UU4fX0sE0%BRBnEGI3Z%Q-ydDjqnU+9(8d
> zS&;Mbm6JLDx^gP>D67J9KAAF|^ougYYO*C|68#C$r}ifVHY=1#+EX~SN*)Qx!$4N2
> zy+u|{{6eULI4}TT+;Z}aF#h!M89Clok!J;l1XB^RLx`y)eH-dtAnQ|sG1&%rO)N$%
> z5Fs=0&8XD#BRv$GMH+>z2axD+Dkdk#Q#~@Qp&Z}I5u22{c26{0b$C!=<0Jg7dS^uE
> zKx0?gY9OaXMvpC#_BrJrQXL-hM{L1cgPK) zDYVPD7`kgfBGexueg83gfRx1bhp-(dk|$E3Lb5s;T1-|Z@knpu(alGP9~M{G$#WwA
> zsmlUBt&KFNu;oGyGB@dJh}gEB!ZK_-CG9cc06WDms#q;qAQ9az5lu znJ!%U42}r<=n@S!S_a*P!Y7q{Uh+($Uie_nr+yH3Bh>kBq=DM|j4
> z3@sy(1TN8?Wmy1WRvy?-MOI-e*_ZR@f-5vD*FB`(a+Z}3&lLSh3gs@rpOm8 z3+*bFitX|zGf};a<_Re0pj4xrjZ%a1E|gl7eJCfREEb%u@Aky!=pKIY@;e7XY6xWu
> zNR8&%nFF3(Lx3P30g*

[Qemu-devel] [PATCH 6/7] Remove unnecessary casts from PCI DMA code in usb-ehci

[David Gibson]

This patch removes some unnecessary casts in the usb-ehci device,
introduced by commit 68d553587c0aa271c3eb2902921b503740d775b6
'usb-ehci: Use PCI DMA stub functions'.

Signed-off-by: David Gibson 
---
 hw/usb-ehci.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index cdd5aae..9e0e0b1 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1107,7 +1107,7 @@ static inline int get_dwords(EHCIState *ehci, uint32_t 
addr,
 int i;
 
 for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
-pci_dma_read(&ehci->dev, addr, (uint8_t *)buf, sizeof(*buf));
+pci_dma_read(&ehci->dev, addr, buf, sizeof(*buf));
 *buf = le32_to_cpu(*buf);
 }
 
@@ -1122,7 +1122,7 @@ static inline int put_dwords(EHCIState *ehci, uint32_t 
addr,
 
 for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
 uint32_t tmp = cpu_to_le32(*buf);
-pci_dma_write(&ehci->dev, addr, (uint8_t *)&tmp, sizeof(tmp));
+pci_dma_write(&ehci->dev, addr, &tmp, sizeof(tmp));
 }
 
 return 1;
@@ -2154,7 +2154,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
 }
 list |= ((ehci->frindex & 0x1ff8) >> 1);
 
-pci_dma_read(&ehci->dev, list, (uint8_t *) &entry, sizeof entry);
+pci_dma_read(&ehci->dev, list, &entry, sizeof entry);
 entry = le32_to_cpu(entry);
 
 DPRINTF("PERIODIC state adv fr=%d.  [%08X] -> %08X\n",
-- 
1.7.7

[Qemu-devel] [0/7] Remove unnecessary casts introduced by PCI DMA stubs series

[David Gibson]

As a few people pointed out, my recent patch series introducing a new
(stub) PCI DMA API added unnecessary casts in quite a few places.  I
think this was a hangover from early days of the patchese where the
casts were necessary for some reason I've now forgotten.

Thanks for applying the series anyway, here is the requested followup
series which removes those unnecessary casts.

This still leaves the bug I introduced in eepro100 where
stl_le_pci_dma() is now used on a possibly unaligned address.  I'll be
sending another patch to fix that.

Re: [Qemu-devel] [PATCH 1.0] add sgabios blob and submodule

[Anthony Liguori]

On 11/03/2011 08:39 PM, Zhi Yong Wu wrote:

On Thu, Nov 3, 2011 at 10:14 PM, Paolo Bonzini  wrote:

The rom was not added together with the sgabios device and is
not installed.

Signed-off-by: Paolo Bonzini
---
The sgabios.git mirror repository can be fetched from
http://people.redhat.com/pbonzini/sgabios-git.tgz

  .gitmodules |3 +++
  Makefile|2 +-
  pc-bios/README  |6 ++
  pc-bios/sgabios.bin |  Bin 0 ->  4096 bytes
  roms/sgabios|1 +

Sorry, what are sgabios and sgabios.bin separately? What is the relationship?


The way git does submodules is that it stores a file in the tree that contains 
the commit hash of the submodule along with metadata indicating that its a 
submodule.  So in a git diff, you see a commit hash change, but when you check 
out or commit, it's really a directory that contains an external git tree.


We use submodules for all of the different firmware blobs that you need to run a 
guest.  You can look in roms/* for the full list.


We also include the binary blobs in the tree because they often require special 
build tools (or at least, target specific cross compilers).  That means that 
most people don't have the necessary tools available to build those blobs.


Regards,

Anthony Liguori




  5 files changed, 11 insertions(+), 1 deletions(-)
  create mode 100755 pc-bios/sgabios.bin
  create mode 16 roms/sgabios

diff --git a/.gitmodules b/.gitmodules
index 2a43dbc..eca876f 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -16,3 +16,6 @@
  [submodule "roms/qemu-palcode"]
path = roms/qemu-palcode
url = git://repo.or.cz/qemu-palcode.git
+[submodule "roms/sgabios"]
+   path = roms/sgabios
+   url = git://git.qemu.org/sgabios.git
diff --git a/Makefile b/Makefile
index 4f6eaa4..168093c 100644
--- a/Makefile
+++ b/Makefile
@@ -253,7 +253,7 @@ ar  de en-us  fi  fr-be  hr it  lv  nl 
pl  ru th \
  common  de-ch  es fo  fr-ca  hu ja  mk  nl-be  pt  sl tr

  ifdef INSTALL_BLOBS
-BLOBS=bios.bin vgabios.bin vgabios-cirrus.bin \
+BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
  vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
  ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \
  pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
diff --git a/pc-bios/README b/pc-bios/README
index 0668559..1cebbbc 100644
--- a/pc-bios/README
+++ b/pc-bios/README
@@ -19,6 +19,12 @@
   https://github.com/dgibson/SLOF, and the image currently in qemu is
   built from git tag qemu-slof-20111013.

+- sgabios (the Serial Graphics Adapter option ROM) provides a means for
+  legacy x86 software to communicate with an attached serial console as
+  if a video card were attached.  The master sources reside in a subversion
+  repository at http://sgabios.googlecode.com/svn/trunk.  A git mirror is
+  available at git://git.qemu.org/sgabios.git.
+
  - The PXE roms come from the iPXE project. Built with BANNER_TIME 0.
   Sources available at http://ipxe.org.  Vendor:Device ID ->  ROM mapping:

diff --git a/pc-bios/sgabios.bin b/pc-bios/sgabios.bin
new file mode 100755
index 
..c3da4c3d0a34e4eca9b0cda38d0538b332f7b09e
GIT binary patch
literal 4096
zcmeHJeRNah8Gmw(K=NZ!#YiO_$Hpho3C>|*k=364AM}*OQ+`H$d;Erd1?wtL_
zdrscx_dM_O{NCq%?)%)_<*#K9x2T8LsLwZz!svOLz$lClLt13p+N?4PpsPn6{(7CgLT_|Gwe#g&&%N@y-~Z{Y=U;g8
z>EAx{?3=%M{g=D%kmB#V#y>;iFGT)H-|_$IkH7wd2FlAf&4
z(K9!L)zW+!O&LAG>I~YmE02WU@_&ZX!e`URplc_v)&RTzVJbtb^y
zKF2cAH_MXAswSND$*AVA7@giGOh@BQ&zsL_)_w&O$}D71UB{0iB=(`uU6m?*QDyz~kk)R`zx
zRE2jGq6-zEh%Uq{j~62G1{rfZ@mh`wT@r?XUYW%G@}l}aPm2{P@T$dN(PozWQu
zN)|Qpw^ZR_WGrPqnT|5@E0yUo@aF~=XyCs&8^7bMC$5Z?#Ji+j(HQ|5f8xmCI4ggc
z|7=M8&70D3=Vth(rkOjVf*dP!-OAq4c@Zc&4>LV-k(Q&tSedaagIl1H5}3y@6)?|g
zgL#bE+yNGbsRWC)4uQF_14a+C}WuUP-b-?Fwg3M2NtQQXh^$+!F4)Et
zBB7nZJsk|%mRRS*65MPdgRTpiMPOxc-O4P0Mb-*fggM)+m0-hUYZ%xV3du$omosuL
zKI3bP7_mFZMbT&Mk@@aUo4@YyvlqC2okrgavg7*f&D?(Ouw^q#g)+B>8CRsveBkT_
z@}YQotUj{bQD^hCfcXG_j{H;fm2nF|*jMczQ}{B-ry@Jr`>mWmM|a&@m3h`To$i`h
zm3hH;^PE;-<<+s#ah<&y`@$6F&+!Wsf&B_+J+m@mv)5q`Q@H)_j*7$~_83l&af9jH
zM(Qzn2d38J_bIbC_jb#UTMX@rUOy(vL@X+w){^PF$)fhDu&d3}iDe{pRQ<8g9
z`F3HGw$}*8vWFGb9XNG+_#E~SAix{fI&@2p{grSp!{+I?m)WX4-QW%I)rlWMp
z)L!ta=q?n(LAs=PZ(2hk{DZVnI_w&LY8L2S5!;I7?k!=gK(QFR(%?+%q(eLtm#_mD
z$&ba|T!eVx%(o4UWILwjBiz+vYA(tRN|9M65*IR)J{5Nr;YEQvfr1EJxZ1fF2=|qiBqMIycl046@k2wvP@&;WGEHX-3h!Z5TOEzGOxT>2Rs_2cg*mL
zv0>-fD*jyHgGqQXGWeic!!tdpciTyR%5>Is$UU4fX0sE0%BRBnEGI3Z%Q-ydDjqnU+9(8d
zS&;Mbm6JLDx^gP>D67J9KAAF|^ougYYO*C|68#C$r}ifVHY=1#+EX~SN*)Qx!$4N2
zy+u|{{6eULI4}TT+;Z}aF#h!M89Clok!J;l1XB^RLx`y)eH-dtAnQ|sG1&%rO)N$%
z5Fs=0&8XD#BRv$GMH+>z2axD+Dkdk#Q#~@Qp&Z}I5u22{c26{0b$C!=<0Jg7dS^uE
zKx0?gY9OaXMvpC#_BrJrQXL-hM{L1cgPK)r+yH3Bh>kBq=DM|j4
z3@sy(1TN8?Wmy1WRvy?-MOI-e*_ZR@f-5vD*FB`(a+Z}3&lLSh3gs@rpOm8FDaW#_-nRi+9QP?-3y2T1mrY@mPVYl~Hh8^s
z8pAqFCwE&+-damZyEDU5r*I|-7PUeoBwOJnQ^S&--}2XZz}N)vrcft-w!S&6<@AV=dQ
zFL9lwzV8^2a2%7SyL;i~#V~-U1oCA(^cXomMtAQ8Zolw%&bS3QV;zuzIJp

[Qemu-devel] [PATCH 7/7] Remove unnecessary casts from PCI DMA code in usb-uhci

[David Gibson]

This patch removes some unnecessary casts in the usb-uhci device,
introduced by commit fff23ee9a5de74ab111b3cea9eec56782e7d7c50
'usb-uhci: Use PCI DMA stub functions'.

Signed-off-by: David Gibson 
---
 hw/usb-uhci.c |   17 +++--
 1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index f9e3ea5..f8912e2 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -876,7 +876,7 @@ static void uhci_async_complete(USBPort *port, USBPacket 
*packet)
 uint32_t link = async->td;
 uint32_t int_mask = 0, val;
 
-pci_dma_read(&s->dev, link & ~0xf, (uint8_t *) &td, sizeof(td));
+pci_dma_read(&s->dev, link & ~0xf, &td, sizeof(td));
 le32_to_cpus(&td.link);
 le32_to_cpus(&td.ctrl);
 le32_to_cpus(&td.token);
@@ -888,8 +888,7 @@ static void uhci_async_complete(USBPort *port, USBPacket 
*packet)
 
 /* update the status bits of the TD */
 val = cpu_to_le32(td.ctrl);
-pci_dma_write(&s->dev, (link & ~0xf) + 4,
-  (const uint8_t *)&val, sizeof(val));
+pci_dma_write(&s->dev, (link & ~0xf) + 4, &val, sizeof(val));
 uhci_async_free(s, async);
 } else {
 async->done = 1;
@@ -952,7 +951,7 @@ static void uhci_process_frame(UHCIState *s)
 
 DPRINTF("uhci: processing frame %d addr 0x%x\n" , s->frnum, frame_addr);
 
-pci_dma_read(&s->dev, frame_addr, (uint8_t *)&link, 4);
+pci_dma_read(&s->dev, frame_addr, &link, 4);
 le32_to_cpus(&link);
 
 int_mask = 0;
@@ -976,7 +975,7 @@ static void uhci_process_frame(UHCIState *s)
 break;
 }
 
-pci_dma_read(&s->dev, link & ~0xf, (uint8_t *) &qh, sizeof(qh));
+pci_dma_read(&s->dev, link & ~0xf, &qh, sizeof(qh));
 le32_to_cpus(&qh.link);
 le32_to_cpus(&qh.el_link);
 
@@ -996,7 +995,7 @@ static void uhci_process_frame(UHCIState *s)
 }
 
 /* TD */
-pci_dma_read(&s->dev, link & ~0xf, (uint8_t *) &td, sizeof(td));
+pci_dma_read(&s->dev, link & ~0xf, &td, sizeof(td));
 le32_to_cpus(&td.link);
 le32_to_cpus(&td.ctrl);
 le32_to_cpus(&td.token);
@@ -1010,8 +1009,7 @@ static void uhci_process_frame(UHCIState *s)
 if (old_td_ctrl != td.ctrl) {
 /* update the status bits of the TD */
 val = cpu_to_le32(td.ctrl);
-pci_dma_write(&s->dev, (link & ~0xf) + 4,
-  (const uint8_t *)&val, sizeof(val));
+pci_dma_write(&s->dev, (link & ~0xf) + 4, &val, sizeof(val));
 }
 
 if (ret < 0) {
@@ -1039,8 +1037,7 @@ static void uhci_process_frame(UHCIState *s)
/* update QH element link */
 qh.el_link = link;
 val = cpu_to_le32(qh.el_link);
-pci_dma_write(&s->dev, (curr_qh & ~0xf) + 4,
-  (const uint8_t *)&val, sizeof(val));
+pci_dma_write(&s->dev, (curr_qh & ~0xf) + 4, &val, sizeof(val));
 
 if (!depth_first(link)) {
/* done with this QH */
-- 
1.7.7

[Qemu-devel] [PATCH 3/7] Remove unnecessary casts from PCI DMA code in PCI IDE

[David Gibson]

This patch removes some unnecessary casts in the PCI IDE device,
introduced by commit 552908fef5b67ad9d96b76d7cb8371ebc26c9bc8
'PCI IDE: Use PCI DMA stub functions'.

Signed-off-by: David Gibson 
---
 hw/ide/pci.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 49b823d..07d2cfa 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -71,7 +71,7 @@ static int bmdma_prepare_buf(IDEDMA *dma, int is_write)
 if (bm->cur_prd_last ||
 (bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE)
 return s->io_buffer_size != 0;
-pci_dma_read(&bm->pci_dev->dev, bm->cur_addr, (uint8_t *)&prd, 8);
+pci_dma_read(&bm->pci_dev->dev, bm->cur_addr, &prd, 8);
 bm->cur_addr += 8;
 prd.addr = le32_to_cpu(prd.addr);
 prd.size = le32_to_cpu(prd.size);
@@ -113,7 +113,7 @@ static int bmdma_rw_buf(IDEDMA *dma, int is_write)
 if (bm->cur_prd_last ||
 (bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE)
 return 0;
-pci_dma_read(&bm->pci_dev->dev, bm->cur_addr, (uint8_t *)&prd, 8);
+pci_dma_read(&bm->pci_dev->dev, bm->cur_addr, &prd, 8);
 bm->cur_addr += 8;
 prd.addr = le32_to_cpu(prd.addr);
 prd.size = le32_to_cpu(prd.size);
-- 
1.7.7

[Qemu-devel] qemu ARM host support likely to be broken in 1.0

[Peter Maydell]

This is just a quick email to summarise a discussion on IRC.

QEMU on ARM hosts (not ARM guests!) is currently broken in
at least the following ways:
 * configure will select the ucontext coroutine implementation
but on ARM makecontext returns ENOSYS and we will abort() on
startup
 * the fixed address we mmap the code gen buffer to in system
mode tends to overlap the libc heap (at least on Ubuntu oneiric);
when this happens you typically get a sysmalloc abort
 * there is a bug I have not investigated which causes an
i386 guest image to loop infinitely resetting before it brings
up the grub menu
 * the TCG_AREG0 is r7, which clashes with the Thumb frame
pointer, with the effect that --enable-debug fails to compile

ARM host support for TCG is not a very high priority for me and
I don't expect to fix any of these before 1.0 is released. I'm
happy to test patches if anybody else wishes to write them :-)

-- PMM

Re: [Qemu-devel] [PATCH 1.0] add sgabios blob and submodule

[Zhi Yong Wu]

On Fri, Nov 4, 2011 at 9:44 AM, Anthony Liguori  wrote:
> On 11/03/2011 08:39 PM, Zhi Yong Wu wrote:
>>
>> On Thu, Nov 3, 2011 at 10:14 PM, Paolo Bonzini
>>  wrote:
>>>
>>> The rom was not added together with the sgabios device and is
>>> not installed.
>>>
>>> Signed-off-by: Paolo Bonzini
>>> ---
>>>        The sgabios.git mirror repository can be fetched from
>>>        http://people.redhat.com/pbonzini/sgabios-git.tgz
>>>
>>>  .gitmodules         |    3 +++
>>>  Makefile            |    2 +-
>>>  pc-bios/README      |    6 ++
>>>  pc-bios/sgabios.bin |  Bin 0 ->  4096 bytes
>>>  roms/sgabios        |    1 +
>>
>> Sorry, what are sgabios and sgabios.bin separately? What is the
>> relationship?
>
> The way git does submodules is that it stores a file in the tree that
> contains the commit hash of the submodule along with metadata indicating
> that its a submodule.  So in a git diff, you see a commit hash change, but
> when you check out or commit, it's really a directory that contains an
> external git tree.
>
> We use submodules for all of the different firmware blobs that you need to
> run a guest.  You can look in roms/* for the full list.
>
> We also include the binary blobs in the tree because they often require
> special build tools (or at least, target specific cross compilers).  That
> means that most people don't have the necessary tools available to build
> those blobs.
Got it. thanks, anthony.

>
> Regards,
>
> Anthony Liguori
>
>>
>>>  5 files changed, 11 insertions(+), 1 deletions(-)
>>>  create mode 100755 pc-bios/sgabios.bin
>>>  create mode 16 roms/sgabios
>>>
>>> diff --git a/.gitmodules b/.gitmodules
>>> index 2a43dbc..eca876f 100644
>>> --- a/.gitmodules
>>> +++ b/.gitmodules
>>> @@ -16,3 +16,6 @@
>>>  [submodule "roms/qemu-palcode"]
>>>        path = roms/qemu-palcode
>>>        url = git://repo.or.cz/qemu-palcode.git
>>> +[submodule "roms/sgabios"]
>>> +       path = roms/sgabios
>>> +       url = git://git.qemu.org/sgabios.git
>>> diff --git a/Makefile b/Makefile
>>> index 4f6eaa4..168093c 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -253,7 +253,7 @@ ar      de     en-us  fi  fr-be  hr     it  lv  nl
>>>       pl  ru     th \
>>>  common  de-ch  es     fo  fr-ca  hu     ja  mk  nl-be      pt  sl     tr
>>>
>>>  ifdef INSTALL_BLOBS
>>> -BLOBS=bios.bin vgabios.bin vgabios-cirrus.bin \
>>> +BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
>>>  vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \
>>>  ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \
>>>  pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
>>> diff --git a/pc-bios/README b/pc-bios/README
>>> index 0668559..1cebbbc 100644
>>> --- a/pc-bios/README
>>> +++ b/pc-bios/README
>>> @@ -19,6 +19,12 @@
>>>   https://github.com/dgibson/SLOF, and the image currently in qemu is
>>>   built from git tag qemu-slof-20111013.
>>>
>>> +- sgabios (the Serial Graphics Adapter option ROM) provides a means for
>>> +  legacy x86 software to communicate with an attached serial console as
>>> +  if a video card were attached.  The master sources reside in a
>>> subversion
>>> +  repository at http://sgabios.googlecode.com/svn/trunk.  A git mirror
>>> is
>>> +  available at git://git.qemu.org/sgabios.git.
>>> +
>>>  - The PXE roms come from the iPXE project. Built with BANNER_TIME 0.
>>>   Sources available at http://ipxe.org.  Vendor:Device ID ->  ROM
>>> mapping:
>>>
>>> diff --git a/pc-bios/sgabios.bin b/pc-bios/sgabios.bin
>>> new file mode 100755
>>> index
>>> ..c3da4c3d0a34e4eca9b0cda38d0538b332f7b09e
>>> GIT binary patch
>>> literal 4096
>>> zcmeHJeRNah8Gmw>> z(goPP4Wd)$$L3rSH#>(K=NZ!#YiO_$Hpho3C>|*k=364AM}*OQ+`H$d;Erd1?wtL_
>>> zdrscx_dM_O{NCq%?)%)_<*#K9x2T8LsLwZz!svOLz$lClLt13p+N?4Pp>> z{vin~r3n_cwXJ=mDc?}j+DL<=$=mFH#IVroUcaunvE5Ly#=YLx>> ztf;K9ItsPn6{>> z+h8yh8O%i`MMWjE&4%U6stkoDB(vQ%jWN$FoL%gl#^5ZWMbnt#R#?{LeYC^?)s^!r
>>> zP@KOsf(J`abH9R7@%saGCfacG^t&~;X5VAH(|p_Q+R6GUQ)m95;Ff$@-n5@@pZl|(
>>> zEnBy7IbA{i@uI9*vu~p0iVUSHbAtNSd*7Zo>(7CgLT_|Gwe#g&&%N@y-~Z{Y=U;g8
>>> z>EAx{?3=%M{g=D%kmB#V#y>;iFGT)H-|_$IkH7wd2FlAf>> zhB8ZSLBobBw#kj7v8k!4xw*NbqS{^2*4D>> zGiJ=#gd;Cc`bndpU{gU_NrM6$>> z%x6PuVXgrblv0)&4
>>> z(K9!L)zW+!O&LAG>I~YmE02WU@_&ZX!e`URplc_v)&RTzVJbtb^y
>>> zKF2cAH_MXAswSND>> zQ`+do@f>$*AVA7@giGOh@BQ&zsL_)_w&O$}D71UB{0iB=(`uU6m?*QDyz~kk)R`zx
>>> zRE2jGq6-zEh%Uq{j~62G1{rfZ@mh`wT@r?XUYW%G@}l}aPm2{P@T$dN(PozWQu
>>> zN)|Qpw^ZR_WGrPqnT|5@E0yUo@aF~=XyCs&8^7bMC$5Z?#Ji+j(HQ|5f8xmCI4ggc
>>> z|7=M8&70D3=Vth(rkOjVf*dP!-OAq4c@Zc&4>LV-k(Q&tSedaagIl1H5}3y@6)?|g
>>> zgL#bE+yNGbsRWC)4uQF_14a+C}WuUP-b-?Fwg3M2N>> znD6g^a_e%aV9awmpu)NWDjDtQQXh^$+!F4)Et
>>> zBB7nZJsk|%mRRS*65MPdgRTpiMPOxc-O4P0Mb-*fggM)+m0-hUYZ%>> ziur=g?x2HHbErVS%oH98^i!dyN79B~8E_{?xT}@pYM@^pnvCnRko>xV3du$omosuL
>>> zKI3bP7_mFZMbT&Mk@@aUo4@YyvlqC2okrgavg7*f&D?(Ouw^q#g)+B>8CRsveBkT_
>>> z@}YQotUj{bQD^hCfcXG_j{H;fm2nF|*jMczQ}{B-ry@Jr`>mWmM|a&@m3h`To$i`h
>>> zm3hH;^PE;-<<+s#ah<&y`@$6F&+!Wsf&B_+J+m@mv)5q`Q@H)_j*7$~_83l&a

Re: [Qemu-devel] [v8 Patch 0/6]Qemu: Host pagecache setting from cmdline and monitor

[Zhi Yong Wu]

On Sun, Oct 30, 2011 at 6:33 PM, Supriya Kannery
 wrote:
> Currently cache setting of a block device cannot be changed
> without restarting a running VM. Following patchset is for
> enabling dynamic change of cache setting for block devices
> through qemu monitor. Code changes are based on patches
> from Christoph Hellwig and Prerna Saxena.
>
> This patchset introduces
> a. monitor command 'block_set_hostcache' using which host
>   pagecache setting for a block device can be changed
>   dynamically.
I got a bit confusion. Is it used to change host pagecache setting on
hyperviser or on guest?
This block device said by you is for guest, right?

> b. a new option for setting host cache from qemu
>   commandline option -drive  "hostcache=on/off".
> c. BDRVReopenState, a generic structure which can be
>   extended by each of the block drivers to reopen
>   respective image files.
>   Extension of this structure for drivers raw-posix
>   is done here.
> d. 'hostcache and 'cache' options when used together,
>   cache=xx will override hostcache=yy.
>
> v8:
>  1. Mandate implementation of all three reopen
>    related functions by block drivers.
>  2. If 'cache=xx' and 'hostcache=yy' specified
>    in cmdline, 'cache=' overrides 'hostcache='.
>
>
> v7:
>  1. Added structure BDRVReopenState to support safe
>    reopening of image files.
>  2. Implemented reopen functions for raw-posix driver
>
> v6:
>  1. "block_set_hostcache" to replace "block_set" command
>
> v5:
>  1. Defined qerror class for incorrect command syntax.
>  2. Changed error_report() calls to qerror_report()
>
> v4:
>    Added 'hostcache' option to '-drive' commandline option.
>
> v3:
>  1. Command "block_set" for changing various block params
>  2. Enhanced info-block to display hostcache setting
>  3. Added qmp interfaces for setting and querying hostcache
>
> v2:
>  1. Support of dynamic cache change only for hostcache.
>  2. Monitor command "hostcache_get" added to display cache setting
>  3. Backed off the changes for display of cache setting in "info block"
>
> v1:
>     Dynamic cache change through monitor
>
> New block command added:
> "block_set_hostcache"
>    -- Sets hostcache parameter for block device  while guest is running.
>
> Usage:
>  block_set_hostcache   
>    = block device
>     = on/off
>
>
> New 'hostcache' option added to -drive:
>  -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
>        
> "       [,readonly=on|off][,hostcache=on|off]\n"
>
>  qemu/block.c           |   79 +++
>  qemu/block.h           |    3 ++
>  qemu/block/raw-posix.c |   57 +
>  qemu/block/raw.c       |   23 +++-
>  qemu/block_int.h       |   16 +++
>  qemu/blockdev.c        |    7 +
>  qemu/qemu-common.h     |    1
>  qemu/qemu-config.c     |    4 ++
>  qemu/qemu-options.hx   |    2 -
>  qemu/qerror.c          |    8 +
>  qemu/qerror.h          |    6 
>  qemu/qmp-commands.hx   |    4 ++
>  14 files changed, 194 insertions(+), 16 deletions(-)
> ~
> ~
> ~
> ~
> ~
> ~
> ~
> ~
> ~
> ~
> "txt" 13L, 574C
>
>
>
>
>
>
>



-- 
Regards,

Zhi Yong Wu

[Qemu-devel] [PATCH] reenable vm_clock when resuming all vcpus

[Wen Congyang]

We disable vm_clock when pausing all vcpus, but we forget to
reenable it when resuming all vcpus. It will cause that the
guest can not be rebooted.

Signed-off-by: Wen Congyang 

---
 cpus.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index 6aff425..82530c4 100644
--- a/cpus.c
+++ b/cpus.c
@@ -891,6 +891,7 @@ void resume_all_vcpus(void)
 {
 CPUState *penv = first_cpu;
 
+qemu_clock_enable(vm_clock, true);
 while (penv) {
 penv->stop = 0;
 penv->stopped = 0;
-- 
1.7.1