Re: [Qemu-devel] [PATCH] ppc/spapr: Allow VIRTIO_VGA

[Gerd Hoffmann]

On Mi, 2015-09-16 at 07:08 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2015-09-15 at 11:19 +0200, Gerd Hoffmann wrote:
> > On Di, 2015-09-15 at 15:51 +1000, Benjamin Herrenschmidt wrote:
> > > It works fine with the Linux driver out of the box
> > 
> > Do you actually want the vga compatibility bits on pseries?
> 
> Yes, our firmware SLOF uses them (via MMIO BARs) for the early boot
> stuff (well, it will use them when the patches I sent are merged).

Fine then, patch queued up.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] target-i386: disable LINT0 after reset

[Gerd Hoffmann]

On Mi, 2015-09-16 at 07:23 +0200, Jan Kiszka wrote:
> On 2015-09-15 23:19, Alex Williamson wrote:
> > On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
> >> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long 
> >> gone
> >> and therefore this hack is no longer needed.  Since it violates the
> >> specifications, it is removed.
> >>
> >> Signed-off-by: Nadav Amit 
> >> ---
> >>  hw/intc/apic_common.c | 9 -
> >>  1 file changed, 9 deletions(-)
> > 
> > Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363
> > 
> > Is this bug perhaps not as long gone as we thought, or is there
> > something else going on here?  Thanks,
> 
> I would say, someone needs to check if the SeaBIOS line that is supposed
> to enable LINT0 is actually executed on one of the broken systems and,
> if not, why not.

There is only one reason (beside miscompiling seabios with
CONFIG_QEMU=n) why seabios would skip acpi initialization, and that is
apic not being present according to cpuid:

cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
if (eax < 1 || !(cpuid_features & CPUID_APIC)) {
// No apic - only the main cpu is present.

https://www.kraxel.org/cgit/seabios/tree/src/fw/smp.c#n79

cheers,
  Gerd

PS: coreboot tripped over this too, fixed just a few days ago.

Re: [Qemu-devel] [PATCH v3 4/5] qmp: add monitor command to add/remove a child

[Wen Congyang]

On 09/15/2015 03:49 PM, Markus Armbruster wrote:
> Wen Congyang  writes:
> 
>> On 09/14/2015 10:36 PM, Markus Armbruster wrote:
>>> Wen Congyang  writes:
>>>
 Signed-off-by: Wen Congyang 
 Signed-off-by: zhanghailiang 
 Signed-off-by: Gonglei 
 ---
  blockdev.c   | 47 ++
  qapi/block-core.json | 34 +
  qmp-commands.hx  | 53 
 
  3 files changed, 134 insertions(+)

 diff --git a/blockdev.c b/blockdev.c
 index bd47756..0a40607 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -3413,6 +3413,53 @@ fail:
  qmp_output_visitor_cleanup(ov);
  }
  
 +void qmp_x_child_add(const char *parent, const char *child,
 + Error **errp)
 +{
 +BlockDriverState *parent_bs, *child_bs;
 +Error *local_err = NULL;
 +
 +parent_bs = bdrv_lookup_bs(parent, parent, &local_err);
 +if (!parent_bs) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +
 +child_bs = bdrv_find_node(child);
 +if (!child_bs) {
 +error_setg(errp, "Node '%s' not found", child);
 +return;
 +}
 +
 +bdrv_add_child(parent_bs, child_bs, &local_err);
 +if (local_err) {
 +error_propagate(errp, local_err);
 +}
 +}
 +
 +void qmp_child_del(const char *parent, const char *child, Error **errp)
 +{
 +BlockDriverState *parent_bs, *child_bs;
 +Error *local_err = NULL;
 +
 +parent_bs = bdrv_lookup_bs(parent, parent, &local_err);
 +if (!parent_bs) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +
 +child_bs = bdrv_find_node(child);
 +if (!child_bs) {
 +error_setg(errp, "Node '%s' not found", child);
 +return;
 +}
 +
 +bdrv_del_child(parent_bs, child_bs, &local_err);
 +if (local_err) {
 +error_propagate(errp, local_err);
 +}
 +}
 +
  BlockJobInfoList *qmp_query_block_jobs(Error **errp)
  {
  BlockJobInfoList *head = NULL, **p_next = &head;
 diff --git a/qapi/block-core.json b/qapi/block-core.json
 index e68a59f..b959577 100644
 --- a/qapi/block-core.json
 +++ b/qapi/block-core.json
 @@ -2272,3 +2272,37 @@
  ##
  { 'command': 'block-set-write-threshold',
'data': { 'node-name': 'str', 'write-threshold': 'uint64' } }
 +
 +##
 +# @x-child-add
 +#
 +# Add a new child to the parent BDS. Currently only the Quorum driver
 +# implements this feature. This is useful to fix a broken quorum child.
 +#
 +# @parent: graph node name or id which the child will be added to.
 +#
 +# @child: graph node name that will be added.
 +#
 +# Note: this command is experimental, and not a stable API.
 +#
 +# Since: 2.5
 +##
 +{ 'command': 'x-child-add',
 +  'data' : { 'parent': 'str', 'child': 'str' } }
 +
 +##
 +# @child-del
 +#
 +# Remove a child from the parent BDS. Currently only the Quorum driver
 +# implements this feature. This is useful to fix a broken quorum child.
 +# Note, you can't remove a child if it would bring the quorum below its
 +# threshold.
 +#
 +# @parent: graph node name or id from which the child will removed.
 +#
 +# @child: graph node name that will be removed.
 +#
 +# Since: 2.5
 +##
 +{ 'command': 'child-del',
 +  'data' : { 'parent': 'str', 'child': 'str' } }
>>>
>>> Why is x-child-add experimental, but child-del isn't?  Please explain
>>> both in the schema and in the commit message.
>>
>> No special reason. Should I put child-del in experimental namespace?
> 
> I found the reason for x-child-add in your v2:
> 
> child-add
> 
> 
> Add a child to a quorum node.
> 
> This command is still a work in progress. It doesn't support all
> block drivers. Stay away from it unless you want it to help with
> its development.
> 
> Eric suggested to rename it to x-child-add, and you did.  Good.  You
> also shortened the "work in progress" note to just "Note: this command
> is experimental, and not a stable API."  I'd like to have a more verbose
> note explaining *why* the command is experimental, both here and in
> qmp-commands.hx.  "It doesn't support all block drivers" is a reason.
> Are the any others?
> 
> Is child-del similarly unfinished?  If yes, make it x-child-del to save
> us from later grief.
> 
> If no: is child-del is only useful together with x-child-add?  Then make
> it x-child-del regardless.

I have another question: if the command is experimental, we have the prefix 
"x-".
Which prefix is used for hmp command?

Thanks
Wen Congyang

> 
 diff --git a/qmp-commands.hx b/

Re: [Qemu-devel] [RFC PATCH] libcacard: move it to a standalone project

[Gerd Hoffmann]

  Hi,

> > Where are we going to host virglrenderer?  Whatever we do for libcacard,
> > we should probably also do for virglrenderer.
> 
> I hadn't considered where to host virglrenderer yet, I was initially 
> considering
> shipping it with mesa but gave up that idea, so I was contemplating just
> getting freedesktop to host it since I've already got a/cs etc there,
> but I suppose
> I could get setup on qemu infrastructure.
> 
> I'm not even sure whether we should have its own mailing list, I'm hoping it
> will be useful to non-qemu projects, but that may be some time down the road.

I think freedesktop.org makes sense.  mesa is there, spice is there too,
and people working on gfx stuff tend to have an account there already.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] target-i386: disable LINT0 after reset

[Nadav Amit]

I'll try to reproduce the problem today.

Nadav
On Sep 16, 2015 8:23 AM, "Jan Kiszka"  wrote:

> On 2015-09-15 23:19, Alex Williamson wrote:
> > On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
> >> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is
> long gone
> >> and therefore this hack is no longer needed.  Since it violates the
> >> specifications, it is removed.
> >>
> >> Signed-off-by: Nadav Amit 
> >> ---
> >>  hw/intc/apic_common.c | 9 -
> >>  1 file changed, 9 deletions(-)
> >
> > Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363
> >
> > Is this bug perhaps not as long gone as we thought, or is there
> > something else going on here?  Thanks,
>
> I would say, someone needs to check if the SeaBIOS line that is supposed
> to enable LINT0 is actually executed on one of the broken systems and,
> if not, why not.
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RTC ITP SES-DE
> Corporate Competence Center Embedded Linux
>

Re: [Qemu-devel] [RFC 00/20] Do away with TB retranslation

[Dennis Luehring]

Am 15.09.2015 um 22:19 schrieb Richard Henderson:

Indeed.  It would indeed be good to add a bunch of bare-metal tests.
Pre-compiled and checked in so that one doesn't have to have a suite of
cross-compilers in order to use them.

OTOH, I don't see myself (or anyone else) really having the time to do that.


im working on that - i've got an clfs (clfs.org) based script that 
generates a sparc64-64 cross-compile suite
and a sparc64-64 linux system with my benchmark tests (prime, stream 
working, g++ pugixml missing)
starting from an initamfs+ramdisk - currently it is still too big (no 
stripping done, too many packages installed)

but i will reduce that

next steps:
-get the base system and benchmarks running
-create an script that startups qemu,run tests,collect results,shutdown
-mips64-64, alpha

Re: [Qemu-devel] [PATCH v2] target-mips: Fix RDHWR on CP0.Count

[Aurelien Jarno]

On 2015-09-08 11:34, Alex Smith wrote:
> For RDHWR on the CP0.Count register, env->CP0_Count was being returned.
> This value is a delta against the QEMU_CLOCK_VIRTUAL clock, not the
> correct current value of CP0.Count. Use cpu_mips_get_count() instead.
> 
> Signed-off-by: Alex Smith 
> Cc: Aurelien Jarno 
> Cc: Leon Alrae 
> ---
> Changes in v2:
>  - Fix build breakage for user builds.
>  - Correct existing code to follow QEMU coding style.
> ---
>  target-mips/op_helper.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 809a061e296b..99574322a29c 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -2184,10 +2184,15 @@ target_ulong helper_rdhwr_synci_step(CPUMIPSState 
> *env)
>  target_ulong helper_rdhwr_cc(CPUMIPSState *env)
>  {
>  if ((env->hflags & MIPS_HFLAG_CP0) ||
> -(env->CP0_HWREna & (1 << 2)))
> +(env->CP0_HWREna & (1 << 2))) {
> +#ifdef CONFIG_USER_ONLY
>  return env->CP0_Count;
> -else
> +#else
> +return (int32_t)cpu_mips_get_count(env);
> +#endif
> +} else {
>  helper_raise_exception(env, EXCP_RI);
> +}
>  
>  return 0;
>  }

Reviewed-by: Aurelien Jarno 

Independently of your patch, I do wonder if we shouldn't change the
return type of cpu_mips_get_count to int32_t. With your patch, there
are now 2 calls to this functions, and both cast the value to int32_t.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH v3 1/5] support nbd driver in blockdev-add

[Wen Congyang]

On 09/15/2015 07:12 PM, Markus Armbruster wrote:
> Wen Congyang  writes:
> 
>> On 09/15/2015 03:37 PM, Markus Armbruster wrote:
>>> Wen Congyang  writes:
>>>
 On 09/14/2015 11:47 PM, Eric Blake wrote:
> On 09/14/2015 08:27 AM, Markus Armbruster wrote:
>> Wen Congyang  writes:
>>
>>> The NBD driver needs: filename, path or (host, port, exportname).
>>> It checks which key exists and decides use unix or inet socket.
>>> It doesn't recognize the key type, so we can't use union, and
>>> can't reuse InetSocketAddress.
>>>
>>> Signed-off-by: Wen Congyang 
>>> Signed-off-by: zhanghailiang 
>>> Signed-off-by: Gonglei 
>>> ---
>>>  qapi/block-core.json | 42 --
>>>  1 file changed, 40 insertions(+), 2 deletions(-)
>>>
>
>>>  ##
>>> +# @BlockdevOptionsNBD
>>> +#
>>> +# Driver specific block device options for NBD
>>> +#
>>> +# @filename: #optional unix or inet path. The format is:
>>> +#unix: nbd+unix:///export?socket=path or
>>> +#  nbd:unix:path:exportname=export
>>> +#inet: nbd[+tcp]://host[:port]/export or
>>> +#  nbd:host[:port]:exportname=export
>
> Yuck. You are passing structured data through a single 'str', when you
> SHOULD be passing it through structured JSON.  Just because we have a
> filename shorthand for convenience does NOT mean that we want to expose
> that convenience in QMP.  Instead, we really want the breakdown of the
> pieces (here, using abbreviated syntax of an inline base, since there
> are pending qapi patches that will allow it):

 Do you mean that: there is no need to support filename?
>>>
>>> Rule of thumb: if the QMP command handler needs to parse a string
>>> argument, that argument should be a complex QAPI type instead.
>>>
>>> Example: @filename needs to be parsed into its components, either
>>>
>>> * protocol unix, socket path, export name, or
>>> * protocol tcp, host, port, export name
>>>
>>> Since there's an either/or, the complex QAPI type should be a union.
>>>
> { 'enum': 'NBDTransport', 'data': ['unix', 'tcp', 'udp'] }

 NBD only uses tcp, it doesn't support udp.

> { 'union': 'BlockdevOptionsNBD',
>   'base': { 'transport': 'NBDTransport', 'export': 'str' },
>   'discriminator': 'transport',
>   'data': { 'unix': 'NBDUnix', 'tcp': 'NBDInet', 'udp': 'NBDInet' } }
> { 'struct': 'NBDUnix', 'data': { 'socket': 'str' } }

 unix socket needs a path, and I think we can use UnixSocketAddress here.
>>>
>>> Yes, we should try to reuse common types like SocketAddress,
>>> InetSocketAddress, UnixSocketAddress.
>>>
>>> Perhaps it could be as simple as
>>>
>>> { 'struct': 'BlockdevOptionsNBD',
>>>   'data': { 'addr: 'SocketAddress', 'export': 'str' } }
>>
>> The problem is that: NBD doesn't use the fd.
> 
> Is that fundamental, or just a matter of implementation?
> 
>> Another question is: what key will we see in nbd_open()? "addr.host"
>> or "host"?
> 
> As long as nbd_config() looks for "host" in the options QDict, we need
> to put "host".

Yes, we need "host" in nbd_config(), but we pass "addr.data.host" to it.

How to avoid this problem?

Thanks
Wen Congyang

> 
>> Thanks
>> Wen Congyang
>>
>>>
>>> Eric, what do you think?
>>>
> { 'struct': 'NBDInet', 'data': { 'host': 'str', '*port': 'int',
>  '*ipv4': 'bool', '*ipv6': 'bool' } }

 Thanks for the above, and I will try it.
>>>
>>> [...]
>>> .
>>>
> .
> 

Re: [Qemu-devel] [PATCH 1/2] target-i386: disable LINT0 after reset

[Jan Kiszka]

On 2015-09-15 23:19, Alex Williamson wrote:
> On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
>> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long 
>> gone
>> and therefore this hack is no longer needed.  Since it violates the
>> specifications, it is removed.
>>
>> Signed-off-by: Nadav Amit 
>> ---
>>  hw/intc/apic_common.c | 9 -
>>  1 file changed, 9 deletions(-)
> 
> Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363
> 
> Is this bug perhaps not as long gone as we thought, or is there
> something else going on here?  Thanks,

I would say, someone needs to check if the SeaBIOS line that is supposed
to enable LINT0 is actually executed on one of the broken systems and,
if not, why not.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 11/12] qapi: add md5 checksum of last dirty bitmap level to query-block

[Markus Armbruster]

John Snow  writes:

> On 09/15/2015 12:29 PM, Eric Blake wrote:
>> On 08/07/2015 03:32 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> From: Vladimir Sementsov-Ogievskiy 
>>>
>>> Reviewed-by: John Snow 
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>>> ---
>>>  block.c| 1 +
>>>  include/qemu/hbitmap.h | 8 
>>>  qapi/block-core.json   | 4 +++-
>>>  util/hbitmap.c | 8 
>>>  4 files changed, 20 insertions(+), 1 deletion(-)
>> 
>>> +++ b/qapi/block-core.json
>>> @@ -359,11 +359,13 @@
>>>  #
>>>  # @status: current status of the dirty bitmap (since 2.4)
>>>  #
>>> +# @md5: md5 checksum of the last bitmap level (since 2.4)
>> 
>> since 2.5, now. Would it help to be explicit that this is a hexadecimal
>> encoding of the checksum (and not the actual binary value)?
>> 
>
> I had reasoned that it was implied so as to maintain a valid QMP
> protocol stream.

There's more ASCII encodings of binary than hex.  Hex is the common one
for MD5, but spelling it out won't hurt.

Re: [Qemu-devel] [PATCH 0/2] target-mips: minor clean up in mtc0 and mfc0

[Aurelien Jarno]

On 2015-09-14 13:45, Leon Alrae wrote:
> This patchset removes the gen_mtc0_store64() which is actually incorrect
> as MTC0 instruction in MIPS64 is supposed to move entire content (if
> dst CP0 register is 64-bit) without sign extending. It also removes the
> gen_mfc0_load64() and replaces the pair of tcg_gen_ld_tl() +
> tcg_gen_ext32s_tl() with single tcg_gen_ld32s_tl().
> 
> Leon Alrae (2):
>   target-mips: correct MTC0 instruction on MIPS64
>   target-mips: remove gen_mfc0_load64() and use tcg_gen_ld32s_tl()

The existence of tcg_gen_ld32s_tl is relatively recent, that's why we
had it opened coded up to now.

Reviewed-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [RFCv2 1/2] spapr: Remove unnecessary owner field from sPAPRDRConnector

[David Gibson]

On Mon, Sep 14, 2015 at 04:24:23PM +0200, Paolo Bonzini wrote:
> 
> 
> On 14/09/2015 16:06, Alexey Kardashevskiy wrote:
> >
> > === * There is no way for a child to determine what its parent
> > is.  It is not * a bidirectional relationship.  This is by
> > design. ===
> >
> > This part always confused me as there is "Object *parent" in
> > the "struct Object". So there is way to determine but it must
> > not be used? Is it debug only?
> >
> > Anyway, all members of the Object class are under /*< private
> >> */ so they should not be accesses in sPAPR code, I believe.
> >>> Ah, good point, I missed that.  I guess we have to keep the owner
> >>> field, redundant though it seems.  Blech.
> >>
> >> I think the comment is wrong or at least inaccurate; it only applies
> >> to the external QOM interface.
> > 
> > Is this case external?
> 
> I meant external as in qom-get, qom-set, qom-list.  There isn't a ".."
> property.
> 
> > Originally I was looking for a object_get_parent() but it is not there
> > so I decided that the comment is correct or I just fail to understand it :)
> 
> Yes, we can add such an API.
> 
> Let's look also at what ->owner is used for.
> 
> > object_property_add_alias(root_container, link_name,
> >   drc->owner, child_name, &err);
> 
> This can be rewritten as
> 
>  object_property_add_const_link(root_container, link_name,
> drc, &err);
> 
> > QTAILQ_FOREACH(prop, &root_container->properties, node) {
> > Object *obj;
> > sPAPRDRConnector *drc;
> > sPAPRDRConnectorClass *drck;
> > uint32_t drc_index, drc_power_domain;
> > 
> > if (!strstart(prop->type, "link<", NULL)) {
> > continue;
> > }
> > 
> > obj = object_property_get_link(root_container, prop->name, NULL);
> > drc = SPAPR_DR_CONNECTOR(obj);
> > drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > 
> > if (owner && (drc->owner != owner)) {
> 
> Could the PCI host bridge instead store the DR connectors when it
> creates them with spapr_dr_connector_new?  Then you can just call
> spapr_drc_populate_dt directly with the right objects, and avoid another
> O(n^2) loop.

So, yes, iterating over the connectors under the owner rather than
going via the global links is another cleanup I've considered but
haven't gotten around to.

Except, I've been thinking further, and I'm not sure it makes sense to
keep these DR connectors around as full QOM objects anyway.  Having
them here at least potentially exposes the DR connector stuff to
users, when it's really an internal of how the hypervisor communicates
with the guest about hotplug.

I'm going to have to talk to more QOM-experienced people to thrash out
the details, but I'm thinking a better approach would be to add a "DR
connector array" QOM interface to the existing PCI host bridge objects
and.. to something for CPU and memory.  That interface would allow the
necessary lookups specifically for DR hotplug events and avoid
creating thousands of extra QOM objects.

Either way, it makes this little cleanup pretty irrelevant, so I'll
drop it for now.

-- 
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


pgpfT66w9jyK7.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH] spapr_pci: fix device tree props for MSI/MSI-X

[David Gibson]

On Tue, Sep 15, 2015 at 04:34:59PM -0500, Michael Roth wrote:
> PAPR requires ibm,req#msi and ibm,req#msi-x to be present in the
> device node to define the number of msi/msi-x interrupts the device
> supports, respectively.
> 
> Currently we have ibm,req#msi-x hardcoded to a non-sensical constant
> that happens to be 2, and are missing ibm,req#msi entirely. The result
> of that is that msi-x capable devices get limited to 2 msi-x
> interrupts (which can impact performance), and msi-only devices likely
> wouldn't work at all. Additionally, if devices expect a minimum that
> exceeds 2, the guest driver may fail to load entirely.
> 
> SLOF still owns the generation of these properties at boot-time
> (although other device properties have since been offloaded to QEMU),
> but for hotplugged devices we rely on the values generated by QEMU
> and thus hit the limitations above.
> 
> Fix this by generating these properties in QEMU as expected by guests.
> 
> In the future it may make sense to modify SLOF to pass through these
> values directly as we do with other props since we're duplicating SLOF
> code.
> 
> Cc: qemu-...@nongnu.org
> Cc: qemu-sta...@nongnu.org
> Cc: David Gibson 
> Cc: Nikunj A Dadhania 
> Signed-off-by: Michael Roth 

Applied to spapr-next, thanks.

> ---
>  hw/ppc/spapr_pci.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2782856..8e9edff 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1012,6 +1012,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
> void *fdt, int offset,
>  int pci_status, err;
>  char *buf = NULL;
>  uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev);
> +uint32_t max_msi, max_msix;
>  
>  if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) ==
>  PCI_HEADER_TYPE_BRIDGE) {
> @@ -1092,8 +1093,15 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
> void *fdt, int offset,
>RESOURCE_CELLS_ADDRESS));
>  _FDT(fdt_setprop_cell(fdt, offset, "#size-cells",
>RESOURCE_CELLS_SIZE));
> -_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x",
> -  RESOURCE_CELLS_SIZE));
> +
> +max_msi = msi_nr_vectors_allocated(dev);
> +if (max_msi) {
> +_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi", max_msi));
> +}
> +max_msix = dev->msix_entries_nr;
> +if (max_msix) {
> +_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x", max_msix));
> +}
>  
>  populate_resource_props(dev, &rp);
>  _FDT(fdt_setprop(fdt, offset, "reg", (uint8_t *)rp.reg, rp.reg_len));

-- 
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


pgptfzH3vF_rO.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH] ppc/spapr: Fix buffer overflow in spapr_populate_drconf_memory()

[David Gibson]

On Tue, Sep 15, 2015 at 09:34:20PM +0200, Thomas Huth wrote:
> The buffer that is allocated in spapr_populate_drconf_memory()
> is used for setting both, the "ibm,dynamic-memory" and the
> "ibm,associativity-lookup-arrays" property. However, only the
> size of the first one is taken into account when allocating the
> memory. So if the length of the second property is larger than
> the length of the first one, we run into a buffer overflow here!
> Fix it by taking the length of the second property into account,
> too.
> 
> Fixes: "spapr: Support ibm,dynamic-reconfiguration-memory" patch
> Signed-off-by: Thomas Huth 

Merged to spapr-next, thanks.

-- 
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


pgp2n5LIq5Txz.pgp
Description: PGP signature

Re: [Qemu-devel] [RFCv2 2/2] spapr: Don't use QOM [*] syntax for DR connectors.

[David Gibson]

On Mon, Sep 14, 2015 at 11:03:00PM -0500, Michael Roth wrote:
> Quoting David Gibson (2015-09-13 20:41:53)
> > The dynamic reconfiguration (hotplug) code for the pseries machine type
> > uses a "DR connector" QOM object for each resource it will be possible
> > to hotplug.  Each of these is added to its owner using
> > object_property_add_child(owner, "dr-connector[*], ...);
> > 
> > That works ok, mostly, but it means that the property indices are
> > arbitrary, depending on the order in which the connectors are constructed.
> > When we have both memory and cpu hotplug, the connectors will be under the
> > same parent (at least in the current drafts), meaning the indices don't
> > correspond to any meaningful ID.
> > 
> > It gets worse when large amounts of hotpluggable RAM is configured.  For
> > RAM, there's a DR connector object for every 256MB of potential memory.  So
> > if maxmem=2T, for example, there are 8192 objects under the same parent.
> > 
> > The QOM interfaces aren't really designed for this.  In particular
> > object_property_add() with [*] has O(n^2) time complexity (in the number of
> > existing children): first it has a linear search through array indices to
> > find a free slot, each of which is attempted to a recursive call to
> > object_property_add() with a specific [N].  Those calls are O(n) because
> > there's a linear search through all properties to check for duplicates.
> > 
> > By using a meaningful index value, which we already know is unique we can
> > avoid the [*] special behaviour.  That lets us reduce the total time for
> > creating the DR objects from O(n^3) to O(n^2).
> > 
> > O(n^2) is still kind of crappy, but it's enough to reduce the startup time
> > of qemu with maxmem=2T from ~20 minutes to ~4 seconds.
> > 
> > Signed-off-by: David Gibson 
> > Cc: Bharata B Rao 
> 
> This is the second patch I've seen that drops use of "[*]" for
> performance reasons, but looking at the code I don't really see
> any reason that logic can't be implemented in object_property_add()
> in O(n) time. For instance I think it can be achieved by
> storing/hashing the base string to an array of auto-incremented
> indicies that we update whenever a child with the corresponding
> [n] format is added.

Oh, there's no question that could be done.  The question is: is
thousands of QOM objects under a single parent a use case that we
actually want to build QOM for?

> I wouldn't hold this real fix up for that though, and in fact the
> use of DRC indexes make for much easier debugging anyway so I'd
> probably still prefer this approach anyway.
> 
> Reviewed-by: Michael Roth 
> 
> > ---
> >  hw/ppc/spapr_drc.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > index 68e0c3e..2f95259 100644
> > --- a/hw/ppc/spapr_drc.c
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -451,13 +451,16 @@ sPAPRDRConnector *spapr_dr_connector_new(Object 
> > *owner,
> >  {
> >  sPAPRDRConnector *drc =
> >  SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR));
> > +char *prop_name;
> > 
> >  g_assert(type);
> > 
> >  drc->type = type;
> >  drc->id = id;
> > -object_property_add_child(owner, "dr-connector[*]", OBJECT(drc), NULL);
> > +prop_name = g_strdup_printf("dr-connector[%"PRIu32"]", get_index(drc));
> > +object_property_add_child(owner, prop_name, OBJECT(drc), NULL);
> >  object_property_set_bool(OBJECT(drc), true, "realized", NULL);
> > +g_free(prop_name);
> > 
> >  /* human-readable name for a DRC to encode into the DT
> >   * description. this is mainly only used within a guest in place
> 

-- 
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


pgpUabBNX9hxs.pgp
Description: PGP signature

[Qemu-devel] [PATCH] MAINTAINERS: Stefan will not maintain net subsystem

[Jason Wang]

Talked with Stefan, he will not maintain net subsystem.

Cc: Stefan Hajnoczi 
Cc: Michael S. Tsirkin 
Signed-off-by: Jason Wang 
---
 MAINTAINERS | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 688979b..2f4e8cf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -955,11 +955,10 @@ F: hmp-commands.hx
 T: git git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
 
 Network device layer
-M: Stefan Hajnoczi 
 M: Jason Wang 
 S: Maintained
 F: net/
-T: git git://github.com/stefanha/qemu.git net
+T: git git://github.com/jasowang/qemu.git net
 
 Netmap network backend
 M: Luigi Rizzo 
-- 
2.1.4

Re: [Qemu-devel] [PATCH 6/7] vhost-user: add multiple queue support

[Yuanhan Liu]

On Wed, Sep 16, 2015 at 10:06:56AM +0800, Yuanhan Liu wrote:
> On Tue, Sep 15, 2015 at 09:02:07AM -0600, Eric Blake wrote:
> > On 09/15/2015 01:10 AM, Yuanhan Liu wrote:
> > > From: Changchun Ouyang 
> > > 
> > > This patch is initially based a patch from Nikolay Nikolaev.
> > > 
> > > Here is the latest version for adding vhost-user multiple queue support,
> > > by creating a nc and vhost_net pair for each queue.
> > 
> > The phrase "Here is the latest version" doesn't make much sense in the
> > long term in git (that is, a year from now, we won't care how many
> > preliminary versions there were, just about the version that got
> > committed; and if anything in git changes after that point, it is no
> > longer the latest version).
> 
> Yeah, good point.
> 
> > 
> > > 
> > > Qemu exits if find that the backend can't support number of requested
> > > queues(by providing queues=# option). The max number is queried by a
> > 
> > space before ( in English.
> 
> Michael reminded me behore, and sorry for making such mistake again.
> 
> And thanks for other typo corrections.
> 
> > 
> > > new message, VHOST_USER_GET_QUEUE_NUM, and is sent only when protocol
> > > feature VHOST_USER_PROTOCOL_F_MQ is present first.
> > > 
> > > The max queue check is done at vhost-user initiation stage. We initiate
> > > one queue first, which, in the meantime, also gets the max_queues the
> > > backend supports.
> > > 
> > > In older version, it was reported that some messages are sent more times
> > > than necessary. Here we came an agreement with Michael that we could
> > > categorize vhost user messages to 2 types: none-vring specific messages,
> > 
> > s/none/non/
> > 
> > > which should be sent only once, and vring specific messages, which should
> > > be sent per queue.
> > > 
> > > Here I introduced a helper function vhost_user_one_time_request(), which
> > > lists following messages as none-vring specific messages:
> > 
> > s/none/non/
> > 
> > > 
> > > VHOST_USER_GET_FEATURES
> > > VHOST_USER_SET_FEATURES
> > > VHOST_USER_GET_PROTOCOL_FEATURES
> > > VHOST_USER_SET_PROTOCOL_FEATURES
> > > VHOST_USER_SET_OWNER
> > > VHOST_USER_RESET_DEVICE
> > > VHOST_USER_SET_MEM_TABLE
> > > VHOST_USER_GET_QUEUE_NUM
> > > 
> > > For above messages, we simply ignore them when they are not sent the first
> > > time.
> > > 
> > 
> > Up to here is mostly fine for the commit message.  Meanwhile...
> > 
> > > v9: per suggested by Jason Wang, we could invoke qemu_chr_add_handlers()
> > > once only, and invoke qemu_find_net_clients_except() at the handler
> > > to gather all related ncs, for initiating all queues. Which addresses
> > > a hidden bug in older verion in a more QEMU-like way.
> > > 
> > > v8: set net->dev.vq_index correctly inside vhost_net_init() based on the
> > > value from net->nc->queue_index.
> > 
> > ...this chunk here is useful only on the mailing list, and not in git,
> > and therefore, should appear...
> 
> Sorry that I may disagree here.
> 
> First of all, it does no harm at all to include few old version
> descriptions. Instead, it may give some hints to the reader how
> the patch evolves. Meanwhile, you will find they are quite common
> in some well known open source projects, such as linux kernel:
> 
> [yliu@yliu-dev ~/linux]$ git log | grep '\<[vV][1-9][0-9]*:' | wc -l
> 10828
> 
> You can find them at qemu project (though not that common), too:
> 
> [yliu@yliu-dev ~/qemu]$ git log | grep '\<[vV][1-9][0-9]*:' | wc -l
> 390
> 
> So, if it's a qemu community specific culture, I could do what
> you suggested.
> 
> If not, I'd like to put them into the commit log, as putting it
> outside the commit log gives unnecessary extra burden to patch
> author when he need update several version change information
> in a patch set: he has to format the patch set first, and add
> them one by one by editing those patches.

Oops, I just found we can do that while editing the commit log (IIRC,
I had a similar try long time ago, and it seems it didn't work).

--yliu

[Qemu-devel] [PATCH] hw/ide/ahci: advance IO buffer offset in multi-sector PIO transfer

[Laszlo Ersek]

The "MdeModulePkg/Bus/Ata/AtaAtapiPassThru" driver in edk2 submits a three
sector long PIO read, when booting off various Fedora installer ISOs in
UEFI mode. With DEBUG_IDE, DEBUG_IDE_ATAPI, DEBUG_AIO and DEBUG_AHCI
enabled, plus a

  DPRINTF(ad->port_no, "offset=%d\n", offset);

at the beginning of ahci_populate_sglist(), we get the following debug
output:

> fis:
> 00:27 80 a0 00 00 fe ff e0 00 00 00 00 00 00 00 00
> 10:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 20:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 30:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 40:28 00 00 00 00 38 00 00 03 00 00 00 00 00 00 00
> 50:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 60:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 70:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> fis:
> 00:28 00 00 00 00 38 00 00 03 00 00 00 00 00 00 00
> ide: CMD=a0
> ATAPI limit=0xfffe packet: 28 00 00 00 00 38 00 00 03 00 00 00
> read pio: LBA=56 nb_sectors=3
> reply: tx_size=6144 elem_tx_size=0 index=2048
> byte_count_limit=65534
> ahci: ahci_populate_sglist: [0] offset=0
> ahci: ahci_dma_prepare_buf: [0] len=0x800
> ahci: ahci_start_transfer: [0] reading 2048 bytes on atapi w/ sglist
> reply: tx_size=4096 elem_tx_size=4096 index=2048
> ahci: ahci_populate_sglist: [0] offset=0
> ahci: ahci_dma_prepare_buf: [0] len=0x800
> ahci: ahci_start_transfer: [0] reading 2048 bytes on atapi w/ sglist
> reply: tx_size=2048 elem_tx_size=2048 index=2048
> ahci: ahci_populate_sglist: [0] offset=0
> ahci: ahci_dma_prepare_buf: [0] len=0x800
> ahci: ahci_start_transfer: [0] reading 2048 bytes on atapi w/ sglist
> reply: tx_size=0 elem_tx_size=0 index=2048
> ahci: ahci_cmd_done: [0] cmd done
> [...]

The following functions play recursive ping-pong, because
ide_atapi_cmd_reply_end() segments the request into individual 2KB
sectors:

ide_transfer_start() <---+
  ahci_start_transfer() via funcptr  |
 |
ahci_dma_prepare_buf()   |
  ahci_populate_sglist() |
 |
dma_buf_read()   |
 |
ahci_commit_buf()|
 |
ide_atapi_cmd_reply_end() via funcptr|
  ide_transfer_start() --+

The ahci_populate_sglist() correctly sets up the scatter-gather list for
dma_buf_read(), based on the Physical Region Descriptors passed in by the
guest. However, the offset into that scatter-gather list remains constant
zero as ide_atapi_cmd_reply_end() wades through every sector of the three
sector long PIO transfer.

The consequence is that the first 2KB of the guest buffer(s), speaking
"linearizedly", is repeatedly overwritten with the next CD-ROM sector. At
the end of the transfer, the sector last read is visible in the first 2KB
of the guest buffer(s), and the rest of the guest buffer(s) remains
unwritten.

Looking at the DMA request path; especially comparing the context of
ahci_commit_buf() between its two callers ahci_dma_rw_buf() and
ahci_start_transfer(), it seems like the latter forgets to advance
"s->io_buffer_offset".

Adding that increment enables the guest to receive valid data.

Cc: John Snow 
Cc: qemu-bl...@nongnu.org
Signed-off-by: Laszlo Ersek 
---

Notes:
I spent the better half of the night on this bug, so please be gentle.
:)

 hw/ide/ahci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 44f6e27..b975c9f 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1291,6 +1291,8 @@ out:
 /* Update number of transferred bytes, destroy sglist */
 ahci_commit_buf(dma, size);
 
+s->io_buffer_offset += size;
+
 s->end_transfer_func(s);
 
 if (!(s->status & DRQ_STAT)) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/2] PCI-e device multi-function hot-add support

[Cao jin]

Enable pcie device multifunction hot, just ensure the function 0
added last, then driver will got the notification to scan all the
function in the slot.

Signed-off-by: Cao jin 
---
 hw/pci/pcie.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 6e28985..89bf61b 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -249,16 +249,16 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 return;
 }
 
-/* TODO: multifunction hot-plug.
- * Right now, only a device of function = 0 is allowed to be
- * hot plugged/unplugged.
+/* To enable multifunction hot-plug, we just ensure the function
+ * 0 added last. Until function 0 added, we set the sltsta and
+ * inform OS via event notification.
  */
-assert(PCI_FUNC(pci_dev->devfn) == 0);
-
-pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
-   PCI_EXP_SLTSTA_PDS);
-pcie_cap_slot_event(PCI_DEVICE(hotplug_dev),
-PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
+if (PCI_FUNC(pci_dev->devfn) == 0) {
+pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
+   PCI_EXP_SLTSTA_PDS);
+pcie_cap_slot_event(PCI_DEVICE(hotplug_dev),
+PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
+}
 }
 
 void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/2] PCI-e device multi-function hot-add support

[Cao jin]

In case user regret when hot-add multi-function, we should roll back,
device_del the function added but still not worked.

Signed-off-by: Cao jin 
---
 hw/pci/pcie.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 89bf61b..497f390 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -265,9 +265,27 @@ void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler 
*hotplug_dev,
  DeviceState *dev, Error **errp)
 {
 uint8_t *exp_cap;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+PCIBus *bus = pci_dev->bus;
 
 pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp);
 
+/* handle the condition: user hot-add multi function, but regret before
+ * finish it, and want to delete the added but not worked function. Fake
+ * the condition: the slot is polulated, power indicator is off and power
+ * controller is off, so device can be detached when OS write config space.
+ */
+if (PCI_FUNC(pci_dev->devfn) > 0 &&
+bus->devices[PCI_DEVFN(0, 0)] == NULL) {
+pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
+PCI_EXP_SLTSTA_PDS);
+
+pcie_cap_slot_event(PCI_DEVICE(hotplug_dev),
+PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
+
+return;
+}
+
 pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
 }
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH 2/2] PCI-e device multi-function hot-add support

[Cao jin]

Hi Igor,
sorry I missed you mail.

On 09/11/2015 10:35 PM, Igor Mammedov wrote:

On Thu, 10 Sep 2015 20:12:23 +0800
Cao jin  wrote:


In case user regret when hot-add multi-function, we should roll back,
device_del the function added but still not worked.

Signed-off-by: Cao jin 
---
  hw/pci/pcie.c | 28 +++-
  1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 61ebefd..b83a244 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -265,10 +265,33 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
  }
  }

+static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
+{
+object_unparent(OBJECT(dev));
+}
+
  void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
   DeviceState *dev, Error **errp)
  {
  uint8_t *exp_cap;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+PCIBus *bus = pci_dev->bus;
+
+/* handle the condition: user want to hot-add multi function, but regret
+ * before finish it, and want to delete the added but not worked function.
+ */

sorry, I couldn't parse this comment and commit message as well. Please 
rephrase it.



Ok, I send v2 before saw your mail, sorry. Maybe I can rephrase it in v3.


+if (PCI_FUNC(pci_dev->devfn) > 0 &&
+bus->devices[PCI_DEVFN(0,0)] == NULL) {
+pci_for_each_device(bus, pci_bus_num(bus),
+pcie_unplug_device, NULL);

*_unplug_request_cb is a way to communicate to guest that it should free
and eject device. So you are not allowed to destroy device from this path,
it's upto guest to decide what do on this request.

Look where pcie_unplug_device() is actually used, that's the place where
guest voluntary ejects device.



Agree. In v2 patch, I changed the delete process, fake the condition, 
let the Guest voluntary ejects device



+
+pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
+ PCI_EXP_SLTSTA_PDS);
+pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
+   PCI_EXP_SLTSTA_PDC);
+
+return;
+}

  pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, 
errp);

@@ -382,11 +405,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
  hotplug_event_update_event_status(dev);
  }

-static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
-{
-object_unparent(OBJECT(dev));
-}
-
  void pcie_cap_slot_write_config(PCIDevice *dev,
  uint32_t addr, uint32_t val, int len)
  {


.



--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH 6/7] vhost-user: add multiple queue support

[Yuanhan Liu]

On Tue, Sep 15, 2015 at 09:02:07AM -0600, Eric Blake wrote:
> On 09/15/2015 01:10 AM, Yuanhan Liu wrote:
> > From: Changchun Ouyang 
> > 
> > This patch is initially based a patch from Nikolay Nikolaev.
> > 
> > Here is the latest version for adding vhost-user multiple queue support,
> > by creating a nc and vhost_net pair for each queue.
> 
> The phrase "Here is the latest version" doesn't make much sense in the
> long term in git (that is, a year from now, we won't care how many
> preliminary versions there were, just about the version that got
> committed; and if anything in git changes after that point, it is no
> longer the latest version).

Yeah, good point.

> 
> > 
> > Qemu exits if find that the backend can't support number of requested
> > queues(by providing queues=# option). The max number is queried by a
> 
> space before ( in English.

Michael reminded me behore, and sorry for making such mistake again.

And thanks for other typo corrections.

> 
> > new message, VHOST_USER_GET_QUEUE_NUM, and is sent only when protocol
> > feature VHOST_USER_PROTOCOL_F_MQ is present first.
> > 
> > The max queue check is done at vhost-user initiation stage. We initiate
> > one queue first, which, in the meantime, also gets the max_queues the
> > backend supports.
> > 
> > In older version, it was reported that some messages are sent more times
> > than necessary. Here we came an agreement with Michael that we could
> > categorize vhost user messages to 2 types: none-vring specific messages,
> 
> s/none/non/
> 
> > which should be sent only once, and vring specific messages, which should
> > be sent per queue.
> > 
> > Here I introduced a helper function vhost_user_one_time_request(), which
> > lists following messages as none-vring specific messages:
> 
> s/none/non/
> 
> > 
> > VHOST_USER_GET_FEATURES
> > VHOST_USER_SET_FEATURES
> > VHOST_USER_GET_PROTOCOL_FEATURES
> > VHOST_USER_SET_PROTOCOL_FEATURES
> > VHOST_USER_SET_OWNER
> > VHOST_USER_RESET_DEVICE
> > VHOST_USER_SET_MEM_TABLE
> > VHOST_USER_GET_QUEUE_NUM
> > 
> > For above messages, we simply ignore them when they are not sent the first
> > time.
> > 
> 
> Up to here is mostly fine for the commit message.  Meanwhile...
> 
> > v9: per suggested by Jason Wang, we could invoke qemu_chr_add_handlers()
> > once only, and invoke qemu_find_net_clients_except() at the handler
> > to gather all related ncs, for initiating all queues. Which addresses
> > a hidden bug in older verion in a more QEMU-like way.
> > 
> > v8: set net->dev.vq_index correctly inside vhost_net_init() based on the
> > value from net->nc->queue_index.
> 
> ...this chunk here is useful only on the mailing list, and not in git,
> and therefore, should appear...

Sorry that I may disagree here.

First of all, it does no harm at all to include few old version
descriptions. Instead, it may give some hints to the reader how
the patch evolves. Meanwhile, you will find they are quite common
in some well known open source projects, such as linux kernel:

[yliu@yliu-dev ~/linux]$ git log | grep '\<[vV][1-9][0-9]*:' | wc -l
10828

You can find them at qemu project (though not that common), too:

[yliu@yliu-dev ~/qemu]$ git log | grep '\<[vV][1-9][0-9]*:' | wc -l
390

So, if it's a qemu community specific culture, I could do what
you suggested.

If not, I'd like to put them into the commit log, as putting it
outside the commit log gives unnecessary extra burden to patch
author when he need update several version change information
in a patch set: he has to format the patch set first, and add
them one by one by editing those patches.

Thanks.

--yliu

Re: [Qemu-devel] [PATCH 1/2] vhost-user: add multiple queue support

[Yuanhan Liu]

On Tue, Sep 15, 2015 at 08:56:05AM -0600, Eric Blake wrote:
> On 09/15/2015 01:10 AM, Yuanhan Liu wrote:
> > From: Changchun Ouyang 
> > 
> > This patch is initially based a patch from Nikolay Nikolaev.
> 
> Subject line confusion - this is titled 1/2, but in reply to a cover
> letter titled 0/7, and where there is also a 1/7 patch, and where the
> 6/7 looks similar to this mail. I'll review that one instead, assuming
> this one is a sending glitch.

Oops, a careless mistake, sorry for that. Please ignore this patch.

--yliu

[Qemu-devel] [PATCH v2 0/2] PCI-e device multi-function hot-add support

[Cao jin]

Support PCI-e device hot-add multi-function via device_add, just ensure
add the function 0 is added last. While allow user to roll back in the
middle via device_del, in case user regret.

According to Alex`s comment, v2 has following update:
1. patch 1/2 remove if() which seems breaks the assumptions elsewhere
2. patch 2/2 change device_del process: qemu fake the device detach condition,
   make OS trigger the action

Cao jin (2):
  PCI-e device multi-function hot-add support
  PCI-e device multi-function hot-add support

 hw/pci/pcie.c | 36 +++-
 1 file changed, 27 insertions(+), 9 deletions(-)

-- 
2.1.0

Re: [Qemu-devel] [PULL 0/19] xen-2015-09-08-tag

[Chen, Tiejun]

On 9/15/2015 7:00 PM, Paolo Bonzini wrote:



On 15/09/2015 11:55, Stefano Stabellini wrote:

On Mon, 14 Sep 2015, Paolo Bonzini wrote:

> On 10/09/2015 12:29, Stefano Stabellini wrote:

> > +if (lseek(config_fd, pos, SEEK_SET) != pos) {
> > +return -errno;
> > +}
> >  do {
> > -rc = pread(config_fd, (uint8_t *)&val, len, pos);
> > +rc = read(config_fd, (uint8_t *)&val, len);
> >  } while (rc < 0 && (errno == EINTR || errno == EAGAIN));

>
> This leaks config_fd.

I don't follow, it leaks config_fd where?


Where lseek returns -errno (and IIRC in other places in the same function).


Do you mean we need this change?

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 1fb71c8..7d44228 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -775,15 +775,18 @@ static int host_pci_config_read(int pos, int len, 
uint32_t val)

 }

 if (lseek(config_fd, pos, SEEK_SET) != pos) {
+close(config_fd);
 return -errno;
 }
 do {
 rc = read(config_fd, (uint8_t *)&val, len);
 } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
 if (rc != len) {
+close(config_fd);
 return -errno;
 }

+close(config_fd);
 return 0;
 }


Thanks
Tiejun

Re: [Qemu-devel] [PATCH V1] sdhci: Fix hostctl2 write logic.

[Alistair Francis]

On Sun, Sep 13, 2015 at 1:36 PM, Peter Crosthwaite
 wrote:
> On Fri, Sep 11, 2015 at 3:30 AM, Sai Pavan Boddu
>  wrote:
>> From: Peter Crosthwaite 
>>
>> This should be a shifted MASKED_WRITE like all other instances of
>> non-word aligned registers.
>>
>> Signed-off-by: Peter Crosthwaite 

Looks good to me

Reviewed-by: Alistair Francis 

Thanks,

Alistair

>
>
> As the sender, this requires your signed-off-by line (in addition to
> any originals). git commit --amend -s should do it.
>
> Your own RB might help as well (I can't do review as author).
>
> Regards,
> Peter
>
>> ---
>>  hw/sd/sdhci.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> index 8fd75f7..fd354e3 100644
>> --- a/hw/sd/sdhci.c
>> +++ b/hw/sd/sdhci.c
>> @@ -1059,7 +1059,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
>> unsigned size)
>>  value |= SDHC_CTRL2_SAMPLING_CLKSEL;
>>  }
>>  s->acmd12errsts = value;
>> -s->hostctl2 = value >> 16;
>> +MASKED_WRITE(s->hostctl2, mask >> 16, value >> 16);
>>  break;
>>  case SDHC_CLKCON:
>>  if (!(mask & 0xFF00)) {
>> --
>> 2.1.1
>>
>

Re: [Qemu-devel] [RFC PATCH] libcacard: move it to a standalone project

[Dave Airlie]

On 15 September 2015 at 23:37, Paolo Bonzini  wrote:
> On 15/09/2015 15:28, Daniel P. Berrange wrote:
>> I have looked through the new libcacard git repository you created
>> from QEMU history, and reviewed the extra patches you added on top
>> for the build system and it all looks sane to me. So this this a
>>
>> Reviewed-by: Daniel P. Berrange 
>>
>> for both the new GIT repo and also your proposed patch to switch
>> QEMU to use the new lib.
>>
>> I agree that it could make sense to host libcacard.git on git.qemu.org
>> if we're going to continue to use qemu-devel and a QEMU-like workflow,
>> but equally don't see any problem with it being a totally standalone
>> project with its own infra & practices.
>
> Where are we going to host virglrenderer?  Whatever we do for libcacard,
> we should probably also do for virglrenderer.

I hadn't considered where to host virglrenderer yet, I was initially considering
shipping it with mesa but gave up that idea, so I was contemplating just
getting freedesktop to host it since I've already got a/cs etc there,
but I suppose
I could get setup on qemu infrastructure.

I'm not even sure whether we should have its own mailing list, I'm hoping it
will be useful to non-qemu projects, but that may be some time down the road.

Dave.

[Qemu-devel] [PATCH] spapr_pci: fix device tree props for MSI/MSI-X

[Michael Roth]

PAPR requires ibm,req#msi and ibm,req#msi-x to be present in the
device node to define the number of msi/msi-x interrupts the device
supports, respectively.

Currently we have ibm,req#msi-x hardcoded to a non-sensical constant
that happens to be 2, and are missing ibm,req#msi entirely. The result
of that is that msi-x capable devices get limited to 2 msi-x
interrupts (which can impact performance), and msi-only devices likely
wouldn't work at all. Additionally, if devices expect a minimum that
exceeds 2, the guest driver may fail to load entirely.

SLOF still owns the generation of these properties at boot-time
(although other device properties have since been offloaded to QEMU),
but for hotplugged devices we rely on the values generated by QEMU
and thus hit the limitations above.

Fix this by generating these properties in QEMU as expected by guests.

In the future it may make sense to modify SLOF to pass through these
values directly as we do with other props since we're duplicating SLOF
code.

Cc: qemu-...@nongnu.org
Cc: qemu-sta...@nongnu.org
Cc: David Gibson 
Cc: Nikunj A Dadhania 
Signed-off-by: Michael Roth 
---
 hw/ppc/spapr_pci.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 2782856..8e9edff 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1012,6 +1012,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
void *fdt, int offset,
 int pci_status, err;
 char *buf = NULL;
 uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev);
+uint32_t max_msi, max_msix;
 
 if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) ==
 PCI_HEADER_TYPE_BRIDGE) {
@@ -1092,8 +1093,15 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
void *fdt, int offset,
   RESOURCE_CELLS_ADDRESS));
 _FDT(fdt_setprop_cell(fdt, offset, "#size-cells",
   RESOURCE_CELLS_SIZE));
-_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x",
-  RESOURCE_CELLS_SIZE));
+
+max_msi = msi_nr_vectors_allocated(dev);
+if (max_msi) {
+_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi", max_msi));
+}
+max_msix = dev->msix_entries_nr;
+if (max_msix) {
+_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x", max_msix));
+}
 
 populate_resource_props(dev, &rp);
 _FDT(fdt_setprop(fdt, offset, "reg", (uint8_t *)rp.reg, rp.reg_len));
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/2] target-i386: disable LINT0 after reset

[Alex Williamson]

On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long gone
> and therefore this hack is no longer needed.  Since it violates the
> specifications, it is removed.
> 
> Signed-off-by: Nadav Amit 
> ---
>  hw/intc/apic_common.c | 9 -
>  1 file changed, 9 deletions(-)

Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363

Is this bug perhaps not as long gone as we thought, or is there
something else going on here?  Thanks,

Alex

> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 042e960..d38d24b 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -243,15 +243,6 @@ static void apic_reset_common(DeviceState *dev)
>  info->vapic_base_update(s);
>  
>  apic_init_reset(dev);
> -
> -if (bsp) {
> -/*
> - * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
> - * time typically by BIOS, so PIC interrupt can be delivered to the
> - * processor when local APIC is enabled.
> - */
> -s->lvt[APIC_LVT_LINT0] = 0x700;
> -}
>  }
>  
>  /* This function is only used for old state version 1 and 2 */

Re: [Qemu-devel] [RFC PATCH 2/2] target-ppc: add CPU IRQ state to PPC VMStateDescription

[Mark Cave-Ayland]

On 15/09/15 00:10, Alexey Kardashevskiy wrote:

> On 09/15/2015 05:30 AM, Mark Cave-Ayland wrote:
>> Commit a90db15 "target-ppc: Convert ppc cpu savevm to VMStateDescription"
>> appears to drop the internal CPU IRQ state from the migration stream.
>> Whilst
>> testing migration on g3beige/mac99 machines, test images would
>> randomly fail to
>> resume unless a key was pressed on the VGA console.
>>
>> Further investigation suggests that internal CPU IRQ state isn't being
>> preserved and so interrupts asserted at the time of migration are
>> lost. Adding
>> the pending_interrupts and irq_input_state fields back into the migration
>> stream appears to fix the problem here during local tests.
>  
> On spapr, interrupt state migrates with XICS interrupt controller and it
> resets the CPU bits you are adding to the migration descriptor. I'd
> expect openpic (this one is used for mac99?) to do the same.

Interesting. I wrote the patch that converted openpic to
VMStateDescription at the end of last year, and my understanding from
the feedback was that ideally interrupt state should be maintained so
that no post_load function was required. I guess spapr is very different
from the basic Mac machines though.

Also I see that you also removed the reference to cpu_write_xer() which
appears to set some related internal state variables. Is this now not
necessary either?

>>
>> Signed-off-by: Mark Cave-Ayland 
>> ---
>>   target-ppc/machine.c |2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>> index bd99844..968a7d6 100644
>> --- a/target-ppc/machine.c
>> +++ b/target-ppc/machine.c
>> @@ -528,6 +528,8 @@ const VMStateDescription vmstate_ppc_cpu = {
>>
>>   /* Internal state */
>>   VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
>> +VMSTATE_UINT32(env.pending_interrupts, PowerPCCPU),
>> +VMSTATE_UINT32(env.irq_input_state, PowerPCCPU),
> 
> This update requires a "version" increment for vmstate_ppc_cpu and
> VMSTATE_UINT32_V instead of VMSTATE_UINT32.

So this means you're happy with the basic patch if I go ahead and make
the version changes too?


ATB,

Mark.

Re: [Qemu-devel] [PATCH] ppc/spapr: Allow VIRTIO_VGA

[Benjamin Herrenschmidt]

On Tue, 2015-09-15 at 11:19 +0200, Gerd Hoffmann wrote:
> On Di, 2015-09-15 at 15:51 +1000, Benjamin Herrenschmidt wrote:
> > It works fine with the Linux driver out of the box
> 
> Do you actually want the vga compatibility bits on pseries?

Yes, our firmware SLOF uses them (via MMIO BARs) for the early boot
stuff (well, it will use them when the patches I sent are merged).

> There also is virtio-gpu-pci (same thing as virtio-vga but without 
> vga compatibility), which should already be enabled on ppc64.
> 
> Thel inux kvm driver certainly doesn't need vga compatibility.
> 
> For slof support it might be useful though, the vga compatibility
> bits
> in virtio-vga are fully compatible to stdvga, so slof support should
> be
> as simple as adding a PCI ID ...

Right. Well, I first had to update SLOF to use the MMIO BARs rather
than the legacy IO crap, but yes, that's pretty much what I did.

> cheers,
>   Gerd
> 

Re: [Qemu-devel] Compiling Qemu from Cygwin

[Eric Blake]

On 09/15/2015 02:10 PM, Stefan Weil wrote:
>>
>>   CCqga/commands-win32.o
>>> qga/commands-win32.c: In function ‘qmp_guest_set_user_password’:
>>> qga/commands-win32.c:1254:55: warning: passing argument 2 of
>>> ‘g_base64_decode’ from incompatible pointer type
>>>  rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
>>>^

Ugh. Compiling for cygwin should favor POSIX interfaces, not
commands-win32.  The build is failing because it isn't even correctly
deciding  which files it should be compiling.


> compiling with cygwin is unsupported. I suggest using MinGW-w64
> (which also works for cross compilations under Linux).

Remember, cygwin is itself an emulation layer - you are emulating POSIX
interfaces on top of Windows; which, while making the environment easier
to port to, also makes the environment slower.  qemu targets mingw and
not cygwin because qemu is an emulator and already slow enough, without
needing another layer of emulation thrown in the mix.

That said, if you want to port qemu to cygwin and supply patches, I'm
more than willing to help review them. It's just that right now, it's
not my personal itch to write such patches, and that the qemu community
currently favors mingw rather than cygwin.

Also, cygwin comes with cross-compilers, so you can still use your
cygwin setup to compile qemu for mingw (although doing the cross-compile
on a true Linux box will be faster).

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] Compiling Qemu from Cygwin

[Mike Ladouceur]

Stefan,

 I got your email from the #qemu channel on OFTC. I was told you may be
able to assist me. I'm trying to compile Qemu from within Cygwin. At first
I was getting an error about not being able to find zlib.h even though it
is in fact installed on the "system". I commented out the code that errors
out the make command. Once I did that, it does go further, however, I get
the following errors:

  CCqga/commands-win32.o
> qga/commands-win32.c: In function ‘qmp_guest_set_user_password’:
> qga/commands-win32.c:1254:55: warning: passing argument 2 of
> ‘g_base64_decode’ from incompatible pointer type
>  rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
>^
> In file included from /usr/include/glib-2.0/glib.h:35:0,
>  from qga/commands-win32.c:14:
> /usr/include/glib-2.0/glib/gbase64.h:52:9: note: expected ‘gsize *’ but
> argument is of type ‘size_t *’
>  guchar *g_base64_decode (const gchar  *text,
>  ^
>   CCqga/channel-win32.o
> qga/channel-win32.c: In function ‘ga_channel_create_watch’:
> qga/channel-win32.c:199:24: warning: cast from pointer to integer of
> different size [-Wpointer-to-int-cast]
>  watch->pollfd.fd = (gintptr) c->rstate.ov.hEvent;
> ^
> qga/channel-win32.c: At top level:
> qga/channel-win32.c:205:11: error: conflicting types for ‘ga_channel_read’
>  GIOStatus ga_channel_read(GAChannel *c, char *buf, size_t size, gsize
> *count)
>^
> In file included from qga/channel-win32.c:9:0:
> ./qga/channel.h:30:11: note: previous declaration of ‘ga_channel_read’ was
> here
>  GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize
> *count);
>^
> qga/channel-win32.c:269:11: error: conflicting types for
> ‘ga_channel_write_all’
>  GIOStatus ga_channel_write_all(GAChannel *c, const char *buf, size_t size)
>^
> In file included from qga/channel-win32.c:9:0:
> ./qga/channel.h:31:11: note: previous declaration of
> ‘ga_channel_write_all’ was here
>  GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize
> size);
>^
> /build/qemu/rules.mak:57: recipe for target 'qga/channel-win32.o' failed
> make: *** [qga/channel-win32.o] Error 1
>

I have tried your builds, however, I am only returned to the prompt without
any output whatsoever. I appreciate any help you can give me.


Thanks,

Mike.

[Qemu-devel] [PATCH: 1/2] rdma: changed companies, use an email that doesn't change

[mhines]

From: "Michael R. Hines" 

Is change this kosher --- changing the header like this? I often see really
outdated emails in other projects sometimes, so I'm not sure if that's
to preserve some kind of providence or something.

Signed-off-by: Michael R. Hines 
---
 migration/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 9424834..d9925c4 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4,7 +4,7 @@
  * Copyright IBM, Corp. 2010-2013
  *
  * Authors:
- *  Michael R. Hines 
+ *  Michael R. Hines 
  *  Jiuxing Liu 
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or
-- 
1.9.1

[Qemu-devel] [Bug 1488363] Re: qemu 2.4.0 hangs using vfio for pci passthrough of graphics card

[Martin]

** Also affects: qemu (Gentoo Linux)
   Importance: Undecided
   Status: New

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

Title:
  qemu 2.4.0 hangs using vfio for pci passthrough of graphics card

Status in QEMU:
  New
Status in qemu package in Gentoo Linux:
  New

Bug description:
  2.3.0 (manjaro distro package) works fine. 2.4.0 (manjaro or the arch
  vanilla one) hangs on the SeaBIOS screen when saying "Press F12 for
  boot menu". All tested with the same hardware, OS, command and
  configuration. It also starts without the GPU passed through, even
  with the USB passed through. I am using the latest SeaBIOS 1.8.2.

  The release notes say:
   VFIO
  Support for resetting AMD Bonaire and Hawaii GPUs
  Platform device passthrough support for Calxeda xgmac devices 
  
  So maybe something there broke it.
  
  I am using the arch qemu 2.4.0 PKGBUILD (modified to have make -j8 and 
removed iscsi, gluster, ceph, etc.), which uses vanilla sources and no patches. 
https://projects.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/qemu

  I am not using a frontend. I am using a script I wrote that generates
  the command below.

  Guest OS here would be 64 bit windows 7, but it didn't start so that's
  not relevant. Also a Manjaro Linux VM won't start.

  CPU is AMD FX-8150; board is Gigabyte GA-990FXA-UD5 (990FX chipset).

  full command line (without the \ after each line) is:

  qemu-system-x86_64
  -enable-kvm
  -M q35
  -m 3584
  -cpu host
  -boot c
  -smp 7,sockets=1,cores=7,threads=1
  -vga none
  -device ioh3420,bus=pcie.0,addr=1c.0,port=1,chassis=1,id=root.1
  -device 
vfio-pci,host=04:00.0,bus=root.1,multifunction=on,x-vga=on,addr=0.0,romfile=Sapphire.R7260X.1024.131106.rom
  -device vfio-pci,host=00:14.2,bus=pcie.0
  -device vfio-pci,host=00:16.0,bus=root.1
  -device vfio-pci,host=00:16.2,bus=root.1
  -usb
  -device ahci,bus=pcie.0,id=ahci
  -drive 
file=/dev/data/vm1,id=disk1,format=raw,if=virtio,index=0,media=disk,discard=on
  -drive media=cdrom,id=cdrom,index=5,media=cdrom
  -netdev type=tap,id=net0,ifname=tap-vm1
  -device virtio-net-pci,netdev=net0,mac=00:01:02:03:04:05
  -monitor stdio
  -boot menu=on

  
  $ lspci -nn | grep -E "04:00.0|00:14.2|00:16.0|00:16.2"
  00:14.2 Audio device [0403]: Advanced Micro Devices, Inc. [AMD/ATI] SBx00 
Azalia (Intel HDA) [1002:4383] (rev 40)
  00:16.0 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD/ATI] 
SB7x0/SB8x0/SB9x0 USB OHCI0 Controller [1002:4397]
  00:16.2 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD/ATI] 
SB7x0/SB8x0/SB9x0 USB EHCI Controller [1002:4396]
  04:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. 
[AMD/ATI] Bonaire XTX [Radeon R7 260X] [1002:6658]

  
  Also I have this one that also hangs:
  05:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. 
[AMD/ATI] Juniper XT [Radeon HD 6770] [1002:68ba]

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

[Qemu-devel] [PATCH: 2/2] maintainers: changed companies, use an email that doesn't change

[mhines]

From: "Michael R. Hines" 

Is there a procedure for updating this file besides just updating the
file?

Signed-off-by: Michael R. Hines 
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 688979b..c455533 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1066,6 +1066,12 @@ F: migration/
 F: scripts/vmstate-static-checker.py
 F: tests/vmstate-static-checker-data/
 
+RDMA Migration
+M: Michael R. Hines 
+M: Michael R. Hines 
+S: Maintained
+F: migration/rdma.c 
+
 Seccomp
 M: Eduardo Otubo 
 S: Supported
-- 
1.9.1

Re: [Qemu-devel] [RFC 00/20] Do away with TB retranslation

[Richard Henderson]

On 09/10/2015 11:55 AM, Alex Bennée wrote:
> I've only had a quick glance so far but I'm fairly familiar with the
> concept from a previous life. I'll aim to do a full review later once
> I've gotten through my MTTCG review backlog.
> 
> Anyway some quick points:
> 
>  * You can save data by only marking faulting instructions
> 
> Assuming that all asynchronous instructions trigger at the end/prologue
> of basic blocks you only actually need to record the address of
> potentially faulting instructions. In fact only a few backend
> instructions will actually synchronously fault.
> 
> Of course this does have the downside of having to mark all those
> instructions in the front end.

We have that.  The only tcg opcodes that can fault are qemu_ld, qemu_st, and
call.  So, yes, I could do exactly this.  Perhaps not for round 1, however?

>  * This method can also be used for additional rectification data
> 
> AIUI we currently ensure all load/stores are barriers and ensure the CPU
> register file is updated before the occur. However if you wanted to you
> could drop that requirement and mark the target-host register pair and
> only fish it out when required on a fault.

Maybe.  I'd have to think about this.  We'd probably want to study how many
flushes to the register file this could elide.  My off-the-cuff guess is not
enough to make the extra overhead useful.

>  * Test suites are essential if your going to get clever
> 
> Last time I went through this I built a SPARC test suite to cover all
> faulting instructions in all the various addressing modes. It flushed
> out a lot of bugs.

Indeed.  It would indeed be good to add a bunch of bare-metal tests.
Pre-compiled and checked in so that one doesn't have to have a suite of
cross-compilers in order to use them.

OTOH, I don't see myself (or anyone else) really having the time to do that.


r~

Re: [Qemu-devel] Compiling Qemu from Cygwin

[Stefan Weil]

Am 15.09.2015 um 20:48 schrieb Mike Ladouceur:
> Stefan,
> 
>  I got your email from the #qemu channel on OFTC. I was told you may be
> able to assist me. I'm trying to compile Qemu from within Cygwin. At first
> I was getting an error about not being able to find zlib.h even though it
> is in fact installed on the "system". I commented out the code that errors
> out the make command. Once I did that, it does go further, however, I get
> the following errors:
> 
>   CCqga/commands-win32.o
>> qga/commands-win32.c: In function ‘qmp_guest_set_user_password’:
>> qga/commands-win32.c:1254:55: warning: passing argument 2 of
>> ‘g_base64_decode’ from incompatible pointer type
>>  rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
>>^
>> In file included from /usr/include/glib-2.0/glib.h:35:0,
>>  from qga/commands-win32.c:14:
>> /usr/include/glib-2.0/glib/gbase64.h:52:9: note: expected ‘gsize *’ but
>> argument is of type ‘size_t *’
>>  guchar *g_base64_decode (const gchar  *text,
>>  ^
>>   CCqga/channel-win32.o
>> qga/channel-win32.c: In function ‘ga_channel_create_watch’:
>> qga/channel-win32.c:199:24: warning: cast from pointer to integer of
>> different size [-Wpointer-to-int-cast]
>>  watch->pollfd.fd = (gintptr) c->rstate.ov.hEvent;
>> ^
>> qga/channel-win32.c: At top level:
>> qga/channel-win32.c:205:11: error: conflicting types for ‘ga_channel_read’
>>  GIOStatus ga_channel_read(GAChannel *c, char *buf, size_t size, gsize
>> *count)
>>^
>> In file included from qga/channel-win32.c:9:0:
>> ./qga/channel.h:30:11: note: previous declaration of ‘ga_channel_read’ was
>> here
>>  GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize
>> *count);
>>^
>> qga/channel-win32.c:269:11: error: conflicting types for
>> ‘ga_channel_write_all’
>>  GIOStatus ga_channel_write_all(GAChannel *c, const char *buf, size_t size)
>>^
>> In file included from qga/channel-win32.c:9:0:
>> ./qga/channel.h:31:11: note: previous declaration of
>> ‘ga_channel_write_all’ was here
>>  GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize
>> size);
>>^
>> /build/qemu/rules.mak:57: recipe for target 'qga/channel-win32.o' failed
>> make: *** [qga/channel-win32.o] Error 1
>>
> 
> I have tried your builds, however, I am only returned to the prompt without
> any output whatsoever. I appreciate any help you can give me.
> 
> 
> Thanks,
> 
> Mike.

Hi Mike,

compiling with cygwin is unsupported. I suggest using MinGW-w64
(which also works for cross compilations under Linux).

See http://qemu.weilnetz.de/FAQ for your last question.

Cheers
Stefan

Re: [Qemu-devel] [PATCH 18/20] tcg: Save insn data and use it in cpu_restore_state_from_tb

[Richard Henderson]

On 09/10/2015 06:49 AM, Peter Maydell wrote:
>> +tcg_debug_assert(num_insns >= 0);
> 
> This is claiming that every TB will have at least one insn_start,
> right? I think that most targets will violate that in the breakpoint
> case, because the "if we have a bp for this insn then generate a
> debug insn and break out of the loop" code is before the call
> to tcg_gen_insn_start().
> 
> We should probably assert that num_insns < TCG_MAX_INSNS while
> we're here.

True.  I wonder if we shouldn't fix bp placement while I'm at it.  And the
assertion should really be num_insns == tb->icount.

>> +static target_long decode_sleb128(uint8_t **pp)
>> +{
>> +uint8_t *p = *pp;
>> +target_long val = 0;
>> +int byte, shift = 0;
>> +
>> +do {
>> +byte = *p++;
>> +val |= (target_ulong)(byte & 0x7f) << shift;
>> +shift += 7;
>> +} while (byte & 0x80);
>> +if (shift < TARGET_LONG_BITS && (byte & 0x40)) {
>> +val |= -(target_ulong)1 << shift;
>> +}
>> +
>> +*pp = p;
>> +return val;
>> +}
> 
> Are the encode/decode sleb128 functions known-good ones
> borrowed from somewhere else?

Yes, from libgcc.

> (PS: checkpatch complains about missing braces.)

Ho hum...

>> +static int encode_search(TranslationBlock *tb, uint8_t *block)
>> +{
> 
> I think this function would benefit from a brief comment
> describing the compressed format we're creating here.

Yes.

>>  gen_code_size = tcg_gen_code(&tcg_ctx, gen_code_buf);
>> +search_size = encode_search(tb, (void *)gen_code_buf + gen_code_size);
> 
> Now we're putting the encoded search info in the codegen buffer,
> don't we need to adjust the calculation of code_gen_buffer_max_size
> to avoid falling off the end if the last TB in the buffer has a very
> large set of generated TCG code and also a big encoded search buffer?

Dunno.  It's not that we've ever checked for this before; I'm not sure what
factor I would actually apply.

> It would also be nice to assert if we do fall off the end of the
> buffer somehow.

Given that we generally use a very large mmap to allocate it, perhaps simply
adding a guard page would be best.

> How much extra space does the encoded search typically take (as a
> % of the gen_code_size, say)?

Dunno; I'll have to have a look at that.  Probably easiest to just enhance info
jit...


r~

Re: [Qemu-devel] [RFT PATCH v1 0/3] net: smc91c111 can_receive fixes

[Richard Purdie]

On Tue, 2015-09-15 at 11:02 -0700, Peter Crosthwaite wrote:
> On Mon, Sep 14, 2015 at 2:09 PM, Richard Purdie
>  wrote:
> > Hi Peter,
> >
> > On Thu, 2015-09-10 at 21:23 -0700, Peter Crosthwaite wrote:
> >> This should hopefully fix your bug, while addressing the extra concern
> >> I raised.
> >>
> >> There was also inconsistent behaviour with corking packets through a
> >> soft reset which I notice and fixed.
> >>
> >> Please let me know if this works for you.
> >
> > I've run it through a few builds/tests in place of my own patches and so
> > far it seems to be working, thanks!
> >
> 
> Can we take that as a formal Tested-by? :)

Tested-by: Richard Purdie 

if that helps :)

Cheers,

Richard

[Qemu-devel] [PULL 0/6] target-i386 exception improvments

[Richard Henderson]

Quite a few fixes to icounting and missed updates to cc_op,
all obviated by using the restore state path.


r~


The following changes since commit 619622424dba749feef752d76d79ef2569f7f250:

  Merge remote-tracking branch 
'remotes/berrange/tags/vnc-crypto-v9-for-upstream' into staging (2015-09-15 
15:42:58 +0100)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-target-i386-20150915

for you to fetch changes up to 4054cdec0423c7190bfc733c27c303d513d531ab:

  target-i386: exception handling for other helper functions (2015-09-15 
12:31:59 -0700)


Exception handling improvments from Pavel Dovgalyuk.


Pavel Dovgalyuk (6):
  target-i386: introduce new raise_exception functions
  target-i386: exception handling for FPU instructions
  target-i386: exception handling for div instructions
  target-i386: exception handling for memory helpers
  target-i386: exception handling for seg_helper functions
  target-i386: exception handling for other helper functions

 target-i386/cc_helper.c   |   2 +-
 target-i386/cpu.h |   4 +
 target-i386/excp_helper.c |  30 ++-
 target-i386/fpu_helper.c  | 164 ++--
 target-i386/helper.h  |   4 +-
 target-i386/int_helper.c  |  32 +--
 target-i386/mem_helper.c  |  39 ++-
 target-i386/misc_helper.c |   8 +-
 target-i386/ops_sse.h |   2 +-
 target-i386/seg_helper.c  | 616 --
 target-i386/translate.c   |  79 +-
 11 files changed, 491 insertions(+), 489 deletions(-)

[Qemu-devel] [PULL 2/6] target-i386: exception handling for FPU instructions

[Richard Henderson]

From: Pavel Dovgalyuk 

This patch fixes exception handling for FPU instructions
and removes obsolete PC update from translate.c.

Reviewed-by: Aurelien Jarno 
Reviewed-by: Richard Henderson 

Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Richard Henderson 
---
 target-i386/fpu_helper.c | 164 +++
 target-i386/translate.c  |  24 ---
 2 files changed, 95 insertions(+), 93 deletions(-)

diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
index 1f954e0..d421a47 100644
--- a/target-i386/fpu_helper.c
+++ b/target-i386/fpu_helper.c
@@ -67,22 +67,24 @@ static inline void fpop(CPUX86State *env)
 env->fpstt = (env->fpstt + 1) & 7;
 }
 
-static inline floatx80 helper_fldt(CPUX86State *env, target_ulong ptr)
+static inline floatx80 helper_fldt(CPUX86State *env, target_ulong ptr,
+   uintptr_t retaddr)
 {
 CPU_LDoubleU temp;
 
-temp.l.lower = cpu_ldq_data(env, ptr);
-temp.l.upper = cpu_lduw_data(env, ptr + 8);
+temp.l.lower = cpu_ldq_data_ra(env, ptr, retaddr);
+temp.l.upper = cpu_lduw_data_ra(env, ptr + 8, retaddr);
 return temp.d;
 }
 
-static inline void helper_fstt(CPUX86State *env, floatx80 f, target_ulong ptr)
+static inline void helper_fstt(CPUX86State *env, floatx80 f, target_ulong ptr,
+   uintptr_t retaddr)
 {
 CPU_LDoubleU temp;
 
 temp.d = f;
-cpu_stq_data(env, ptr, temp.l.lower);
-cpu_stw_data(env, ptr + 8, temp.l.upper);
+cpu_stq_data_ra(env, ptr, temp.l.lower, retaddr);
+cpu_stw_data_ra(env, ptr + 8, temp.l.upper, retaddr);
 }
 
 /* x87 FPU helpers */
@@ -125,10 +127,10 @@ static inline floatx80 helper_fdiv(CPUX86State *env, 
floatx80 a, floatx80 b)
 return floatx80_div(a, b, &env->fp_status);
 }
 
-static void fpu_raise_exception(CPUX86State *env)
+static void fpu_raise_exception(CPUX86State *env, uintptr_t retaddr)
 {
 if (env->cr[0] & CR0_NE_MASK) {
-raise_exception(env, EXCP10_COPR);
+raise_exception_ra(env, EXCP10_COPR, retaddr);
 }
 #if !defined(CONFIG_USER_ONLY)
 else {
@@ -313,14 +315,14 @@ void helper_fldt_ST0(CPUX86State *env, target_ulong ptr)
 int new_fpstt;
 
 new_fpstt = (env->fpstt - 1) & 7;
-env->fpregs[new_fpstt].d = helper_fldt(env, ptr);
+env->fpregs[new_fpstt].d = helper_fldt(env, ptr, GETPC());
 env->fpstt = new_fpstt;
 env->fptags[new_fpstt] = 0; /* validate stack entry */
 }
 
 void helper_fstt_ST0(CPUX86State *env, target_ulong ptr)
 {
-helper_fstt(env, ST0, ptr);
+helper_fstt(env, ST0, ptr, GETPC());
 }
 
 void helper_fpush(CPUX86State *env)
@@ -603,7 +605,7 @@ void helper_fclex(CPUX86State *env)
 void helper_fwait(CPUX86State *env)
 {
 if (env->fpus & FPUS_SE) {
-fpu_raise_exception(env);
+fpu_raise_exception(env, GETPC());
 }
 }
 
@@ -633,11 +635,11 @@ void helper_fbld_ST0(CPUX86State *env, target_ulong ptr)
 
 val = 0;
 for (i = 8; i >= 0; i--) {
-v = cpu_ldub_data(env, ptr + i);
+v = cpu_ldub_data_ra(env, ptr + i, GETPC());
 val = (val * 100) + ((v >> 4) * 10) + (v & 0xf);
 }
 tmp = int64_to_floatx80(val, &env->fp_status);
-if (cpu_ldub_data(env, ptr + 9) & 0x80) {
+if (cpu_ldub_data_ra(env, ptr + 9, GETPC()) & 0x80) {
 tmp = floatx80_chs(tmp);
 }
 fpush(env);
@@ -654,10 +656,10 @@ void helper_fbst_ST0(CPUX86State *env, target_ulong ptr)
 mem_ref = ptr;
 mem_end = mem_ref + 9;
 if (val < 0) {
-cpu_stb_data(env, mem_end, 0x80);
+cpu_stb_data_ra(env, mem_end, 0x80, GETPC());
 val = -val;
 } else {
-cpu_stb_data(env, mem_end, 0x00);
+cpu_stb_data_ra(env, mem_end, 0x00, GETPC());
 }
 while (mem_ref < mem_end) {
 if (val == 0) {
@@ -666,10 +668,10 @@ void helper_fbst_ST0(CPUX86State *env, target_ulong ptr)
 v = val % 100;
 val = val / 100;
 v = ((v / 10) << 4) | (v % 10);
-cpu_stb_data(env, mem_ref++, v);
+cpu_stb_data_ra(env, mem_ref++, v, GETPC());
 }
 while (mem_ref < mem_end) {
-cpu_stb_data(env, mem_ref++, 0);
+cpu_stb_data_ra(env, mem_ref++, 0, GETPC());
 }
 }
 
@@ -977,7 +979,8 @@ void helper_fxam_ST0(CPUX86State *env)
 }
 }
 
-void helper_fstenv(CPUX86State *env, target_ulong ptr, int data32)
+static void do_fstenv(CPUX86State *env, target_ulong ptr, int data32,
+  uintptr_t retaddr)
 {
 int fpus, fptag, exp, i;
 uint64_t mant;
@@ -1005,37 +1008,43 @@ void helper_fstenv(CPUX86State *env, target_ulong ptr, 
int data32)
 }
 if (data32) {
 /* 32 bit */
-cpu_stl_data(env, ptr, env->fpuc);
-cpu_stl_data(env, ptr + 4, fpus);
-cpu_stl_data(env, ptr + 8, fptag);
-cpu_stl_data(env, ptr + 12, 0); /* fpip */
-cpu_stl_data(env, ptr + 16, 0); /* fpcs */
-cpu_stl_data(env, ptr + 20, 0); /* fpoo */
-cpu_stl_data(env, ptr + 24, 0

Re: [Qemu-devel] [PATCH 11/12] qapi: add md5 checksum of last dirty bitmap level to query-block

[John Snow]

On 09/15/2015 12:29 PM, Eric Blake wrote:
> On 08/07/2015 03:32 AM, Vladimir Sementsov-Ogievskiy wrote:
>> From: Vladimir Sementsov-Ogievskiy 
>>
>> Reviewed-by: John Snow 
>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> ---
>>  block.c| 1 +
>>  include/qemu/hbitmap.h | 8 
>>  qapi/block-core.json   | 4 +++-
>>  util/hbitmap.c | 8 
>>  4 files changed, 20 insertions(+), 1 deletion(-)
> 
>> +++ b/qapi/block-core.json
>> @@ -359,11 +359,13 @@
>>  #
>>  # @status: current status of the dirty bitmap (since 2.4)
>>  #
>> +# @md5: md5 checksum of the last bitmap level (since 2.4)
> 
> since 2.5, now. Would it help to be explicit that this is a hexadecimal
> encoding of the checksum (and not the actual binary value)?
> 

I had reasoned that it was implied so as to maintain a valid QMP
protocol stream.

[Qemu-devel] [PULL 5/6] target-i386: exception handling for seg_helper functions

[Richard Henderson]

From: Pavel Dovgalyuk 

This patch fixes exception handling for seg_helper functions.

Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Richard Henderson 
---
 target-i386/helper.h |   4 +-
 target-i386/seg_helper.c | 616 +--
 target-i386/translate.c  |  42 +---
 3 files changed, 335 insertions(+), 327 deletions(-)

diff --git a/target-i386/helper.h b/target-i386/helper.h
index 74308f4..8454a04 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -30,9 +30,9 @@ DEF_HELPER_2(verw, void, env, tl)
 DEF_HELPER_2(lldt, void, env, int)
 DEF_HELPER_2(ltr, void, env, int)
 DEF_HELPER_3(load_seg, void, env, int, int)
-DEF_HELPER_4(ljmp_protected, void, env, int, tl, int)
+DEF_HELPER_4(ljmp_protected, void, env, int, tl, tl)
 DEF_HELPER_5(lcall_real, void, env, int, tl, int, int)
-DEF_HELPER_5(lcall_protected, void, env, int, tl, int, int)
+DEF_HELPER_5(lcall_protected, void, env, int, tl, int, tl)
 DEF_HELPER_2(iret_real, void, env, int)
 DEF_HELPER_3(iret_protected, void, env, int, int)
 DEF_HELPER_3(lret_protected, void, env, int, int)
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 8a4271e..1a3a2e7 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -67,8 +67,9 @@
 #endif
 
 /* return non zero if error */
-static inline int load_segment(CPUX86State *env, uint32_t *e1_ptr,
-   uint32_t *e2_ptr, int selector)
+static inline int load_segment_ra(CPUX86State *env, uint32_t *e1_ptr,
+   uint32_t *e2_ptr, int selector,
+   uintptr_t retaddr)
 {
 SegmentCache *dt;
 int index;
@@ -84,11 +85,17 @@ static inline int load_segment(CPUX86State *env, uint32_t 
*e1_ptr,
 return -1;
 }
 ptr = dt->base + index;
-*e1_ptr = cpu_ldl_kernel(env, ptr);
-*e2_ptr = cpu_ldl_kernel(env, ptr + 4);
+*e1_ptr = cpu_ldl_kernel_ra(env, ptr, retaddr);
+*e2_ptr = cpu_ldl_kernel_ra(env, ptr + 4, retaddr);
 return 0;
 }
 
+static inline int load_segment(CPUX86State *env, uint32_t *e1_ptr,
+   uint32_t *e2_ptr, int selector)
+{
+return load_segment_ra(env, e1_ptr, e2_ptr, selector, 0);
+}
+
 static inline unsigned int get_seg_limit(uint32_t e1, uint32_t e2)
 {
 unsigned int limit;
@@ -124,7 +131,8 @@ static inline void load_seg_vm(CPUX86State *env, int seg, 
int selector)
 }
 
 static inline void get_ss_esp_from_tss(CPUX86State *env, uint32_t *ss_ptr,
-   uint32_t *esp_ptr, int dpl)
+   uint32_t *esp_ptr, int dpl,
+   uintptr_t retaddr)
 {
 X86CPU *cpu = x86_env_get_cpu(env);
 int type, index, shift;
@@ -153,60 +161,61 @@ static inline void get_ss_esp_from_tss(CPUX86State *env, 
uint32_t *ss_ptr,
 shift = type >> 3;
 index = (dpl * 4 + 2) << shift;
 if (index + (4 << shift) - 1 > env->tr.limit) {
-raise_exception_err(env, EXCP0A_TSS, env->tr.selector & 0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, env->tr.selector & 0xfffc, 
retaddr);
 }
 if (shift == 0) {
-*esp_ptr = cpu_lduw_kernel(env, env->tr.base + index);
-*ss_ptr = cpu_lduw_kernel(env, env->tr.base + index + 2);
+*esp_ptr = cpu_lduw_kernel_ra(env, env->tr.base + index, retaddr);
+*ss_ptr = cpu_lduw_kernel_ra(env, env->tr.base + index + 2, retaddr);
 } else {
-*esp_ptr = cpu_ldl_kernel(env, env->tr.base + index);
-*ss_ptr = cpu_lduw_kernel(env, env->tr.base + index + 4);
+*esp_ptr = cpu_ldl_kernel_ra(env, env->tr.base + index, retaddr);
+*ss_ptr = cpu_lduw_kernel_ra(env, env->tr.base + index + 4, retaddr);
 }
 }
 
-static void tss_load_seg(CPUX86State *env, int seg_reg, int selector, int cpl)
+static void tss_load_seg(CPUX86State *env, int seg_reg, int selector, int cpl,
+ uintptr_t retaddr)
 {
 uint32_t e1, e2;
 int rpl, dpl;
 
 if ((selector & 0xfffc) != 0) {
-if (load_segment(env, &e1, &e2, selector) != 0) {
-raise_exception_err(env, EXCP0A_TSS, selector & 0xfffc);
+if (load_segment_ra(env, &e1, &e2, selector, retaddr) != 0) {
+raise_exception_err_ra(env, EXCP0A_TSS, selector & 0xfffc, 
retaddr);
 }
 if (!(e2 & DESC_S_MASK)) {
-raise_exception_err(env, EXCP0A_TSS, selector & 0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector & 0xfffc, 
retaddr);
 }
 rpl = selector & 3;
 dpl = (e2 >> DESC_DPL_SHIFT) & 3;
 if (seg_reg == R_CS) {
 if (!(e2 & DESC_CS_MASK)) {
-raise_exception_err(env, EXCP0A_TSS, selector & 0xfffc);
+raise_exception_err_ra(env, EXCP0A_TSS, selector & 0xfffc, 
retaddr);
 }
 if (dpl != rpl) {
-raise_exception_err(env, EXCP0A_TSS, selector & 0xfffc);
+   

[Qemu-devel] [PULL 3/6] target-i386: exception handling for div instructions

[Richard Henderson]

From: Pavel Dovgalyuk 

This patch fixes exception handling for div instructions
and removes obsolete PC update from translate.c.

Reviewed-by: Richard Henderson 
Reviewed-by: Aurelien Jarno 

Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Richard Henderson 
---
 target-i386/int_helper.c | 32 
 target-i386/translate.c  |  8 
 2 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/target-i386/int_helper.c b/target-i386/int_helper.c
index b0d78e6..3dcd25f 100644
--- a/target-i386/int_helper.c
+++ b/target-i386/int_helper.c
@@ -48,11 +48,11 @@ void helper_divb_AL(CPUX86State *env, target_ulong t0)
 num = (env->regs[R_EAX] & 0x);
 den = (t0 & 0xff);
 if (den == 0) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q = (num / den);
 if (q > 0xff) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q &= 0xff;
 r = (num % den) & 0xff;
@@ -66,11 +66,11 @@ void helper_idivb_AL(CPUX86State *env, target_ulong t0)
 num = (int16_t)env->regs[R_EAX];
 den = (int8_t)t0;
 if (den == 0) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q = (num / den);
 if (q != (int8_t)q) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q &= 0xff;
 r = (num % den) & 0xff;
@@ -84,11 +84,11 @@ void helper_divw_AX(CPUX86State *env, target_ulong t0)
 num = (env->regs[R_EAX] & 0x) | ((env->regs[R_EDX] & 0x) << 16);
 den = (t0 & 0x);
 if (den == 0) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q = (num / den);
 if (q > 0x) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q &= 0x;
 r = (num % den) & 0x;
@@ -103,11 +103,11 @@ void helper_idivw_AX(CPUX86State *env, target_ulong t0)
 num = (env->regs[R_EAX] & 0x) | ((env->regs[R_EDX] & 0x) << 16);
 den = (int16_t)t0;
 if (den == 0) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q = (num / den);
 if (q != (int16_t)q) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q &= 0x;
 r = (num % den) & 0x;
@@ -123,12 +123,12 @@ void helper_divl_EAX(CPUX86State *env, target_ulong t0)
 num = ((uint32_t)env->regs[R_EAX]) | 
((uint64_t)((uint32_t)env->regs[R_EDX]) << 32);
 den = t0;
 if (den == 0) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q = (num / den);
 r = (num % den);
 if (q > 0x) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 env->regs[R_EAX] = (uint32_t)q;
 env->regs[R_EDX] = (uint32_t)r;
@@ -142,12 +142,12 @@ void helper_idivl_EAX(CPUX86State *env, target_ulong t0)
 num = ((uint32_t)env->regs[R_EAX]) | 
((uint64_t)((uint32_t)env->regs[R_EDX]) << 32);
 den = t0;
 if (den == 0) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 q = (num / den);
 r = (num % den);
 if (q != (int32_t)q) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 env->regs[R_EAX] = (uint32_t)q;
 env->regs[R_EDX] = (uint32_t)r;
@@ -379,12 +379,12 @@ void helper_divq_EAX(CPUX86State *env, target_ulong t0)
 uint64_t r0, r1;
 
 if (t0 == 0) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 r0 = env->regs[R_EAX];
 r1 = env->regs[R_EDX];
 if (div64(&r0, &r1, t0)) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 env->regs[R_EAX] = r0;
 env->regs[R_EDX] = r1;
@@ -395,12 +395,12 @@ void helper_idivq_EAX(CPUX86State *env, target_ulong t0)
 uint64_t r0, r1;
 
 if (t0 == 0) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 r0 = env->regs[R_EAX];
 r1 = env->regs[R_EDX];
 if (idiv64(&r0, &r1, t0)) {
-raise_exception(env, EXCP00_DIVZ);
+raise_exception_ra(env, EXCP00_DIVZ, GETPC());
 }
 env->regs[R_EAX] = r0;
 env->regs[R_EDX] = r1;
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 3b3335f..827e85c 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -4841,21 +4841,17 @@ static target_ulong disas_insn(CPUX86State *env, 
DisasContext *s,
 case 6: /* div */
 switch(ot) {
 case MO_8:
-gen_jmp_im(pc_start - s->cs_base);
   

[Qemu-devel] [PULL 6/6] target-i386: exception handling for other helper functions

[Richard Henderson]

From: Pavel Dovgalyuk 

This patch fixes exception handling for other helper functions.

Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Richard Henderson 
---
 target-i386/cc_helper.c   | 2 +-
 target-i386/misc_helper.c | 8 
 target-i386/ops_sse.h | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target-i386/cc_helper.c b/target-i386/cc_helper.c
index ecbf0ec..d5b7c7b 100644
--- a/target-i386/cc_helper.c
+++ b/target-i386/cc_helper.c
@@ -378,7 +378,7 @@ void helper_sti_vm(CPUX86State *env)
 {
 env->eflags |= VIF_MASK;
 if (env->eflags & VIP_MASK) {
-raise_exception(env, EXCP0D_GPF);
+raise_exception_ra(env, EXCP0D_GPF, GETPC());
 }
 }
 #endif
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 52c5d65..6bfc7dd 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -220,7 +220,7 @@ void helper_rdtsc(CPUX86State *env)
 uint64_t val;
 
 if ((env->cr[4] & CR4_TSD_MASK) && ((env->hflags & HF_CPL_MASK) != 0)) {
-raise_exception(env, EXCP0D_GPF);
+raise_exception_ra(env, EXCP0D_GPF, GETPC());
 }
 cpu_svm_check_intercept_param(env, SVM_EXIT_RDTSC, 0);
 
@@ -238,7 +238,7 @@ void helper_rdtscp(CPUX86State *env)
 void helper_rdpmc(CPUX86State *env)
 {
 if ((env->cr[4] & CR4_PCE_MASK) && ((env->hflags & HF_CPL_MASK) != 0)) {
-raise_exception(env, EXCP0D_GPF);
+raise_exception_ra(env, EXCP0D_GPF, GETPC());
 }
 cpu_svm_check_intercept_param(env, SVM_EXIT_RDPMC, 0);
 
@@ -589,7 +589,7 @@ void helper_hlt(CPUX86State *env, int next_eip_addend)
 void helper_monitor(CPUX86State *env, target_ulong ptr)
 {
 if ((uint32_t)env->regs[R_ECX] != 0) {
-raise_exception(env, EXCP0D_GPF);
+raise_exception_ra(env, EXCP0D_GPF, GETPC());
 }
 /* XXX: store address? */
 cpu_svm_check_intercept_param(env, SVM_EXIT_MONITOR, 0);
@@ -601,7 +601,7 @@ void helper_mwait(CPUX86State *env, int next_eip_addend)
 X86CPU *cpu;
 
 if ((uint32_t)env->regs[R_ECX] != 0) {
-raise_exception(env, EXCP0D_GPF);
+raise_exception_ra(env, EXCP0D_GPF, GETPC());
 }
 cpu_svm_check_intercept_param(env, SVM_EXIT_MWAIT, 0);
 env->eip += next_eip_addend;
diff --git a/target-i386/ops_sse.h b/target-i386/ops_sse.h
index bee134b..7aa693a 100644
--- a/target-i386/ops_sse.h
+++ b/target-i386/ops_sse.h
@@ -483,7 +483,7 @@ void glue(helper_maskmov, SUFFIX)(CPUX86State *env, Reg *d, 
Reg *s,
 
 for (i = 0; i < (8 << SHIFT); i++) {
 if (s->B(i) & 0x80) {
-cpu_stb_data(env, a0 + i, d->B(i));
+cpu_stb_data_ra(env, a0 + i, d->B(i), GETPC());
 }
 }
 }
-- 
2.1.0

[Qemu-devel] [PULL 1/6] target-i386: introduce new raise_exception functions

[Richard Henderson]

From: Pavel Dovgalyuk 

This patch introduces new versions of raise_exception functions
that receive TB return address as an argument.

Reviewed-by: Aurelien Jarno 
Reviewed-by: Richard Henderson 

Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Richard Henderson 
---
 target-i386/cpu.h |  4 
 target-i386/excp_helper.c | 30 +-
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index af97772..3bcf2f6 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1267,8 +1267,12 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int 
bank,
 
 /* excp_helper.c */
 void QEMU_NORETURN raise_exception(CPUX86State *env, int exception_index);
+void QEMU_NORETURN raise_exception_ra(CPUX86State *env, int exception_index,
+  uintptr_t retaddr);
 void QEMU_NORETURN raise_exception_err(CPUX86State *env, int exception_index,
int error_code);
+void QEMU_NORETURN raise_exception_err_ra(CPUX86State *env, int 
exception_index,
+  int error_code, uintptr_t retaddr);
 void QEMU_NORETURN raise_interrupt(CPUX86State *nenv, int intno, int is_int,
int error_code, int next_eip_addend);
 
diff --git a/target-i386/excp_helper.c b/target-i386/excp_helper.c
index 99fca84..5e347bc 100644
--- a/target-i386/excp_helper.c
+++ b/target-i386/excp_helper.c
@@ -22,14 +22,6 @@
 #include "sysemu/sysemu.h"
 #include "exec/helper-proto.h"
 
-#if 0
-#define raise_exception_err(env, a, b)  \
-do {\
-qemu_log("raise_exception line=%d\n", __LINE__);\
-(raise_exception_err)(env, a, b);   \
-} while (0)
-#endif
-
 void helper_raise_interrupt(CPUX86State *env, int intno, int next_eip_addend)
 {
 raise_interrupt(env, intno, 1, 0, next_eip_addend);
@@ -92,7 +84,8 @@ static int check_exception(CPUX86State *env, int intno, int 
*error_code)
  */
 static void QEMU_NORETURN raise_interrupt2(CPUX86State *env, int intno,
int is_int, int error_code,
-   int next_eip_addend)
+   int next_eip_addend,
+   uintptr_t retaddr)
 {
 CPUState *cs = CPU(x86_env_get_cpu(env));
 
@@ -108,7 +101,7 @@ static void QEMU_NORETURN raise_interrupt2(CPUX86State 
*env, int intno,
 env->error_code = error_code;
 env->exception_is_int = is_int;
 env->exception_next_eip = env->eip + next_eip_addend;
-cpu_loop_exit(cs);
+cpu_loop_exit_restore(cs, retaddr);
 }
 
 /* shortcuts to generate exceptions */
@@ -116,16 +109,27 @@ static void QEMU_NORETURN raise_interrupt2(CPUX86State 
*env, int intno,
 void QEMU_NORETURN raise_interrupt(CPUX86State *env, int intno, int is_int,
int error_code, int next_eip_addend)
 {
-raise_interrupt2(env, intno, is_int, error_code, next_eip_addend);
+raise_interrupt2(env, intno, is_int, error_code, next_eip_addend, 0);
 }
 
 void raise_exception_err(CPUX86State *env, int exception_index,
  int error_code)
 {
-raise_interrupt2(env, exception_index, 0, error_code, 0);
+raise_interrupt2(env, exception_index, 0, error_code, 0, 0);
+}
+
+void raise_exception_err_ra(CPUX86State *env, int exception_index,
+int error_code, uintptr_t retaddr)
+{
+raise_interrupt2(env, exception_index, 0, error_code, 0, retaddr);
 }
 
 void raise_exception(CPUX86State *env, int exception_index)
 {
-raise_interrupt2(env, exception_index, 0, 0, 0);
+raise_interrupt2(env, exception_index, 0, 0, 0, 0);
+}
+
+void raise_exception_ra(CPUX86State *env, int exception_index, uintptr_t 
retaddr)
+{
+raise_interrupt2(env, exception_index, 0, 0, 0, retaddr);
 }
-- 
2.1.0

Re: [Qemu-devel] [PATCH v4 03/11] qemu-log: correct help text for -d cpu

[Richard Henderson]

On 09/15/2015 12:28 PM, Peter Maydell wrote:
> On 4 August 2015 at 20:16, Richard Henderson  wrote:
>> On 08/04/2015 12:08 PM, Alex Bennée wrote:
> Would that make sense as a debug option or should we have a specific set
> of TCG options to alter its behaviour?


 That's what I'm saying -- probably a separate debug option is better.
>>>
>>> Sorry I meant should we add it to -d (as in -d nochain) or have some tcg
>>> opts (--tcg nochain,blah)
>>
>> I was suggesting the former: -d nochain.
> 
> Did anybody ever write the patch to add 'nochain' support?

Nope.  ;-)


r~

[Qemu-devel] [PULL 4/6] target-i386: exception handling for memory helpers

[Richard Henderson]

From: Pavel Dovgalyuk 

This patch fixes exception handling for memory helpers
and removes obsolete PC update from translate.c.

Reviewed-by: Aurelien Jarno 
Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Richard Henderson 
---
 target-i386/mem_helper.c | 39 ++-
 target-i386/translate.c  |  5 -
 2 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/target-i386/mem_helper.c b/target-i386/mem_helper.c
index 8bf0da2..0e20df2 100644
--- a/target-i386/mem_helper.c
+++ b/target-i386/mem_helper.c
@@ -60,13 +60,14 @@ void helper_cmpxchg8b(CPUX86State *env, target_ulong a0)
 int eflags;
 
 eflags = cpu_cc_compute_all(env, CC_OP);
-d = cpu_ldq_data(env, a0);
+d = cpu_ldq_data_ra(env, a0, GETPC());
 if (d == (((uint64_t)env->regs[R_EDX] << 32) | 
(uint32_t)env->regs[R_EAX])) {
-cpu_stq_data(env, a0, ((uint64_t)env->regs[R_ECX] << 32) | 
(uint32_t)env->regs[R_EBX]);
+cpu_stq_data_ra(env, a0, ((uint64_t)env->regs[R_ECX] << 32)
+  | (uint32_t)env->regs[R_EBX], GETPC());
 eflags |= CC_Z;
 } else {
 /* always do the store */
-cpu_stq_data(env, a0, d);
+cpu_stq_data_ra(env, a0, d, GETPC());
 env->regs[R_EDX] = (uint32_t)(d >> 32);
 env->regs[R_EAX] = (uint32_t)d;
 eflags &= ~CC_Z;
@@ -81,19 +82,19 @@ void helper_cmpxchg16b(CPUX86State *env, target_ulong a0)
 int eflags;
 
 if ((a0 & 0xf) != 0) {
-raise_exception(env, EXCP0D_GPF);
+raise_exception_ra(env, EXCP0D_GPF, GETPC());
 }
 eflags = cpu_cc_compute_all(env, CC_OP);
-d0 = cpu_ldq_data(env, a0);
-d1 = cpu_ldq_data(env, a0 + 8);
+d0 = cpu_ldq_data_ra(env, a0, GETPC());
+d1 = cpu_ldq_data_ra(env, a0 + 8, GETPC());
 if (d0 == env->regs[R_EAX] && d1 == env->regs[R_EDX]) {
-cpu_stq_data(env, a0, env->regs[R_EBX]);
-cpu_stq_data(env, a0 + 8, env->regs[R_ECX]);
+cpu_stq_data_ra(env, a0, env->regs[R_EBX], GETPC());
+cpu_stq_data_ra(env, a0 + 8, env->regs[R_ECX], GETPC());
 eflags |= CC_Z;
 } else {
 /* always do the store */
-cpu_stq_data(env, a0, d0);
-cpu_stq_data(env, a0 + 8, d1);
+cpu_stq_data_ra(env, a0, d0, GETPC());
+cpu_stq_data_ra(env, a0 + 8, d1, GETPC());
 env->regs[R_EDX] = d1;
 env->regs[R_EAX] = d0;
 eflags &= ~CC_Z;
@@ -106,11 +107,11 @@ void helper_boundw(CPUX86State *env, target_ulong a0, int 
v)
 {
 int low, high;
 
-low = cpu_ldsw_data(env, a0);
-high = cpu_ldsw_data(env, a0 + 2);
+low = cpu_ldsw_data_ra(env, a0, GETPC());
+high = cpu_ldsw_data_ra(env, a0 + 2, GETPC());
 v = (int16_t)v;
 if (v < low || v > high) {
-raise_exception(env, EXCP05_BOUND);
+raise_exception_ra(env, EXCP05_BOUND, GETPC());
 }
 }
 
@@ -118,10 +119,10 @@ void helper_boundl(CPUX86State *env, target_ulong a0, int 
v)
 {
 int low, high;
 
-low = cpu_ldl_data(env, a0);
-high = cpu_ldl_data(env, a0 + 4);
+low = cpu_ldl_data_ra(env, a0, GETPC());
+high = cpu_ldl_data_ra(env, a0 + 4, GETPC());
 if (v < low || v > high) {
-raise_exception(env, EXCP05_BOUND);
+raise_exception_ra(env, EXCP05_BOUND, GETPC());
 }
 }
 
@@ -141,11 +142,7 @@ void tlb_fill(CPUState *cs, target_ulong addr, int 
is_write, int mmu_idx,
 X86CPU *cpu = X86_CPU(cs);
 CPUX86State *env = &cpu->env;
 
-if (retaddr) {
-/* now we have a real cpu fault */
-cpu_restore_state(cs, retaddr);
-}
-raise_exception_err(env, cs->exception_index, env->error_code);
+raise_exception_err_ra(env, cs->exception_index, env->error_code, 
retaddr);
 }
 }
 #endif
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 827e85c..17e4ef3 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -5203,8 +5203,6 @@ static target_ulong disas_insn(CPUX86State *env, 
DisasContext *s,
 if (dflag == MO_64) {
 if (!(s->cpuid_ext_features & CPUID_EXT_CX16))
 goto illegal_op;
-gen_jmp_im(pc_start - s->cs_base);
-gen_update_cc_op(s);
 gen_lea_modrm(env, s, modrm);
 gen_helper_cmpxchg16b(cpu_env, cpu_A0);
 } else
@@ -5212,8 +5210,6 @@ static target_ulong disas_insn(CPUX86State *env, 
DisasContext *s,
 {
 if (!(s->cpuid_features & CPUID_CX8))
 goto illegal_op;
-gen_jmp_im(pc_start - s->cs_base);
-gen_update_cc_op(s);
 gen_lea_modrm(env, s, modrm);
 gen_helper_cmpxchg8b(cpu_env, cpu_A0);
 }
@@ -6951,7 +6947,6 @@ static target_ulong disas_insn(CPUX86State *env, 
DisasContext *s,
 goto illegal_op;
 gen_op_mov_v_reg(ot, cpu_T[0], reg);
 gen_lea_modrm(env, s, modrm);
-gen_jmp_im(pc_start - s->cs_base);
 tcg_gen_trunc_tl_i32

[Qemu-devel] [PATCH] ppc/spapr: Fix buffer overflow in spapr_populate_drconf_memory()

[Thomas Huth]

The buffer that is allocated in spapr_populate_drconf_memory()
is used for setting both, the "ibm,dynamic-memory" and the
"ibm,associativity-lookup-arrays" property. However, only the
size of the first one is taken into account when allocating the
memory. So if the length of the second property is larger than
the length of the first one, we run into a buffer overflow here!
Fix it by taking the length of the second property into account,
too.

Fixes: "spapr: Support ibm,dynamic-reconfiguration-memory" patch
Signed-off-by: Thomas Huth 
---
 Note: This is for the spapr-next branch only, the patch
 which introduces this problem is not on master yet.

 hw/ppc/spapr.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f22db12..e4177fb 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -725,9 +725,12 @@ static int spapr_populate_drconf_memory(sPAPRMachineState 
*spapr, void *fdt)
 uint32_t *int_buf, *cur_index, buf_len;
 int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
 
-/* Allocate enough buffer size to fit in ibm,dynamic-memory */
-buf_len = nr_lmbs * SPAPR_DR_LMB_LIST_ENTRY_SIZE * sizeof(uint32_t) +
-sizeof(uint32_t);
+/*
+ * Allocate enough buffer size to fit in ibm,dynamic-memory
+ * or ibm,associativity-lookup-arrays
+ */
+buf_len = MAX(nr_lmbs * SPAPR_DR_LMB_LIST_ENTRY_SIZE + 1, nr_nodes * 4 + 2)
+  * sizeof(uint32_t);
 cur_index = int_buf = g_malloc0(buf_len);
 
 offset = fdt_add_subnode(fdt, 0, "ibm,dynamic-reconfiguration-memory");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v4 03/11] qemu-log: correct help text for -d cpu

[Peter Maydell]

On 4 August 2015 at 20:16, Richard Henderson  wrote:
> On 08/04/2015 12:08 PM, Alex Bennée wrote:
 Would that make sense as a debug option or should we have a specific set
 of TCG options to alter its behaviour?
>>>
>>>
>>> That's what I'm saying -- probably a separate debug option is better.
>>
>> Sorry I meant should we add it to -d (as in -d nochain) or have some tcg
>> opts (--tcg nochain,blah)
>
> I was suggesting the former: -d nochain.

Did anybody ever write the patch to add 'nochain' support?

thanks
-- PMM

Re: [Qemu-devel] [Xen-devel] [Patch V1 2/3] xen/usb: add capability for passing through isoc jobs to host devices

[Konrad Rzeszutek Wilk]

On Thu, Sep 03, 2015 at 12:45:12PM +0200, Juergen Gross wrote:
> When Xen is using the qemu usb framework for pure passthrough of I/Os
> to host devices the handling of isoc jobs is rather complicated if
> multiple isoc frames are transferred with one call.
> 
> Instead of calling the framework with each frame individually, using
> timers to avoid polling in a loop and sampling all responses to
> construct a sum response for the user, just add a capability to
> use the libusb isoc framework instead. This capability is selected
> via a device specific property.
> 
> When the property is selected the host usb driver will use xen specific

You mean it will use generic callbacks - but call Xen specific code?

> callbacks to signal the end of isoc I/Os. For now these callbacks will
> just be nops, they'll be filled with sensible actions when the xen
> pv-usb backend is being added.

The PV USB backend being in the QEMU driver?

But this patch by itself will avoid some complex code right?
> 
> Signed-off-by: Juergen Gross 
> ---
>  hw/usb/core.c| 11 ++
>  hw/usb/host-libusb.c | 51 
> ++--
>  include/hw/xen/xen_backend.h |  3 +++
>  3 files changed, 55 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/usb/core.c b/hw/usb/core.c
> index cf34755..ed2255c 100644
> --- a/hw/usb/core.c
> +++ b/hw/usb/core.c
> @@ -427,10 +427,13 @@ void usb_handle_packet(USBDevice *dev, USBPacket *p)
>  if (QTAILQ_EMPTY(&p->ep->queue) || p->ep->pipeline || p->stream) {
>  usb_process_one(p);
>  if (p->status == USB_RET_ASYNC) {
> -/* hcd drivers cannot handle async for isoc */
> -assert(p->ep->type != USB_ENDPOINT_XFER_ISOC);
> -/* using async for interrupt packets breaks migration */
> -assert(p->ep->type != USB_ENDPOINT_XFER_INT ||
> +/*
> + * hcd drivers cannot handle async for isoc.
> + * Using async for interrupt packets breaks migration.
> + * Host devices are okay in any case.
> + */
> +assert((p->ep->type != USB_ENDPOINT_XFER_ISOC &&
> +p->ep->type != USB_ENDPOINT_XFER_INT) ||
> (dev->flags & (1 << USB_DEV_FLAG_IS_HOST)));
>  usb_packet_set_state(p, USB_PACKET_ASYNC);
>  QTAILQ_INSERT_TAIL(&p->ep->queue, p, queue);
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index a5f9dab..ce644c3 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -42,6 +42,7 @@
>  #include "trace.h"
>  
>  #include "hw/usb.h"
> +#include "hw/xen/xen_backend.h"
>  
>  /*  
> */
>  
> @@ -64,6 +65,7 @@ struct USBAutoFilter {
>  
>  enum USBHostDeviceOptions {
>  USB_HOST_OPT_PIPELINE,
> +USB_HOST_XEN_ISO_PASSTHROUGH,
>  };
>  
>  struct USBHostDevice {
> @@ -152,6 +154,7 @@ static void usb_host_attach_kernel(USBHostDevice *s);
>  #define CONTROL_TIMEOUT  1/* 10 sec*/
>  #define BULK_TIMEOUT 0/* unlimited */
>  #define INTR_TIMEOUT 0/* unlimited */
> +#define ISO_TIMEOUT  0/* unlimited */
>  
>  #if LIBUSBX_API_VERSION >= 0x01000103
>  # define HAVE_STREAMS 1
> @@ -306,14 +309,14 @@ static bool usb_host_use_combining(USBEndpoint *ep)
>  /*  
> */
>  
>  static USBHostRequest *usb_host_req_alloc(USBHostDevice *s, USBPacket *p,
> -  bool in, size_t bufsize)
> +  bool in, size_t bufsize, int 
> packets)
>  {
>  USBHostRequest *r = g_new0(USBHostRequest, 1);
>  
>  r->host = s;
>  r->p = p;
>  r->in = in;
> -r->xfer = libusb_alloc_transfer(0);
> +r->xfer = libusb_alloc_transfer(packets);
>  if (bufsize) {
>  r->buffer = g_malloc(bufsize);
>  }
> @@ -376,6 +379,29 @@ out:
>  }
>  }
>  
> +static void usb_host_req_complete_iso_xen(struct libusb_transfer *xfer)
> +{
> +USBHostRequest *r = xfer->user_data;
> +USBHostDevice  *s = r->host;
> +bool disconnect = (xfer->status == LIBUSB_TRANSFER_NO_DEVICE);
> +
> +if (r->p == NULL) {
> +goto out; /* request was canceled */
> +}
> +
> +r->p->status = status_map[xfer->status];
> +trace_usb_host_req_complete(s->bus_num, s->addr, r->p,
> +r->p->status, r->p->actual_length);
> +/* copying of buffer is done in complete callback of xen */
> +usb_packet_complete(USB_DEVICE(s), r->p);
> +
> +out:
> +usb_host_req_free(r);
> +if (disconnect) {
> +usb_host_nodev(s);
> +}
> +}
> +
>  static void usb_host_req_complete_data(struct libusb_transfer *xfer)
>  {
>  USBHostRequest *r = xfer->user_data;
> @@ -1226,7 +1252,7 @@ static void usb_host_handle_control(USBDevice *udev, 
> USBPacket *p,
>  

Re: [Qemu-devel] [PATCH] ide: fix ATAPI command permissions

[John Snow]

On 09/15/2015 02:50 PM, Markus Armbruster wrote:
> John Snow  writes:
> 
>> On 09/15/2015 02:11 PM, Markus Armbruster wrote:
>>> John Snow  writes:
>>>
 On 09/15/2015 02:53 AM, Markus Armbruster wrote:
> John Snow  writes:
>
>> We're a little too lenient with what we'll let an ATAPI drive handle.
>> Clamp down on the IDE command execution table to remove CD_OK permissions
>> from commands that are not and have never been ATAPI commands.
>>
>> For ATAPI command validity, please see:
>> - ATA4 Section 6.5 ("PACKET Command feature set")
>> - ATA8/ACS Section 4.3 ("The PACKET feature set")
>> - ACS3 Section 4.3 ("The PACKET feature set")
>>
>> ACS3 has a historical command validity table in Table B.4
>> ("Historical Command Assignments") that can be referenced to find when
>> a command was introduced, deprecated, obsoleted, etc.
>>
>> The only reference for ATAPI command validity is by checking that
>> version's PACKET feature set section.
>>
>> ATAPI was introduced by T13 into ATA4, all commands retired prior to ATA4
>> therefore are assumed to have never been ATAPI commands.
>>
>> Mandatory commands, as listed in ATA8-ACS3, are:
>>
>> - DEVICE RESET
>> - EXECUTE DEVICE DIAGNOSTIC
>> - IDENTIFY DEVICE
>> - IDENTIFY PACKET DEVICE
>> - NOP
>> - PACKET
>> - READ SECTOR(S)
>> - SET FEATURES
>>
>> Optional commands as listed in ATA8-ACS3, are:
>>
>> - FLUSH CACHE
>> - READ LOG DMA EXT
>> - READ LOG EXT
>> - WRITE LOG DMA EXT
>> - WRITE LOG EXT
>>
>> All other commands are illegal to send to an ATAPI device and should
>> be rejected by the device.
>
> We could perhaps argue about "should be rejected by the device", but I
> think the weaker "a device is free to reject it" still suffices to
> support your patch.
>

 Sure -- I suppose drives CAN support a superset if they want to. In my
 mind, anything above the ATAPI spec should be justified directly with
 "Guest X breaks without it."

>> CD_OK removal justifications:
>>
>> 0x06 WIN_DSM  Defined in ACS2. Not valid for ATAPI.
>> 0x21 WIN_READ_ONCERetired in ATA5. Not ATAPI in ATA4.
>> 0x94 WIN_STANDBYNOW2  Retired in ATA4. Did not coexist with ATAPI.
>> 0x95 WIN_IDLEIMMEDIATE2   Retired in ATA4. Did not coexist with ATAPI.
>> 0x96 WIN_STANDBY2 Retired in ATA4. Did not coexist with ATAPI.
>> 0x97 WIN_SETIDLE2 Retired in ATA4. Did not coexist with ATAPI.
>> 0x98 WIN_CHECKPOWERMODE2  Retired in ATA4. Did not coexist with ATAPI.
>> 0x99 WIN_SLEEPNOW2Retired in ATA4. Did not coexist with ATAPI.
>> 0xE0 WIN_STANDBYNOW1  Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE1 WIN_IDLEIMMDIATE Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE2 WIN_STANDBY  Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE3 WIN_SETIDLE1 Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE4 WIN_CHECKPOWERMODE1  Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE5 WIN_SLEEPNOW1Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xF8 WIN_READ_NATIVE_MAX  Obsoleted in ACS3. Not ATAPI in ATA4 or ACS.
>
> Actual patch matches this list.
>
>> This patch fixes a divide by zero fault that can be caused by sending
>> the WIN_READ_NATIVE_MAX command to an empty ATAPI drive, which causes
>> it to attempt to use zeroed CHS values to perform sector arithmetic.
>>
>> Reported-by: Qinghao Tang 
>> Signed-off-by: John Snow 
>
> I appreciate you going to the root of the problem instead of merely
> fixing the narrow bug.
>
> Could a similar argument be made for dropping CFA_OK from some commands?
>

 Very likely, but that's another patch. I didn't audit that yet.

> Do we still need this conditional in cmd_read_native_max()?
>
> /* Refuse if no sectors are addressable (e.g. medium not inserted) */
> if (s->nb_sectors == 0) {
> ide_abort_command(s);
> return true;
> }
>
> Why does it fail at guarding the CHS use from empty ATAPI drives before
> your patch?
>

 Because I misunderstood the real reason myself, and my POC test was a
 little bananas. This works *with* a CDROM inserted, not without.

 So s->nb_sectors can be non-zero, and ide_set_sector then triggers e.g.
 SIGFPE.

 If you'll save me the re-spin, I can fix that part of the commit message
 in my staging branch.
>>>
>>> Let me paraphrase to make sure I got you.
>>>
>>> If the drive is empty, the guard aborts the command correctly.
>>>
>>> If the drive isn't empty, the guard doesn't abort.  The code then goes
>>> on and happily uses CHS.  However, ATAPI devices don't have CHS
>>> geometry, see ide_dev_initfn():
>>>
>>> if (kind != IDE_CD) {
>>> 

Re: [Qemu-devel] [PATCH] ide: fix ATAPI command permissions

[Markus Armbruster]

John Snow  writes:

> On 09/15/2015 02:11 PM, Markus Armbruster wrote:
>> John Snow  writes:
>> 
>>> On 09/15/2015 02:53 AM, Markus Armbruster wrote:
 John Snow  writes:

> We're a little too lenient with what we'll let an ATAPI drive handle.
> Clamp down on the IDE command execution table to remove CD_OK permissions
> from commands that are not and have never been ATAPI commands.
>
> For ATAPI command validity, please see:
> - ATA4 Section 6.5 ("PACKET Command feature set")
> - ATA8/ACS Section 4.3 ("The PACKET feature set")
> - ACS3 Section 4.3 ("The PACKET feature set")
>
> ACS3 has a historical command validity table in Table B.4
> ("Historical Command Assignments") that can be referenced to find when
> a command was introduced, deprecated, obsoleted, etc.
>
> The only reference for ATAPI command validity is by checking that
> version's PACKET feature set section.
>
> ATAPI was introduced by T13 into ATA4, all commands retired prior to ATA4
> therefore are assumed to have never been ATAPI commands.
>
> Mandatory commands, as listed in ATA8-ACS3, are:
>
> - DEVICE RESET
> - EXECUTE DEVICE DIAGNOSTIC
> - IDENTIFY DEVICE
> - IDENTIFY PACKET DEVICE
> - NOP
> - PACKET
> - READ SECTOR(S)
> - SET FEATURES
>
> Optional commands as listed in ATA8-ACS3, are:
>
> - FLUSH CACHE
> - READ LOG DMA EXT
> - READ LOG EXT
> - WRITE LOG DMA EXT
> - WRITE LOG EXT
>
> All other commands are illegal to send to an ATAPI device and should
> be rejected by the device.

 We could perhaps argue about "should be rejected by the device", but I
 think the weaker "a device is free to reject it" still suffices to
 support your patch.

>>>
>>> Sure -- I suppose drives CAN support a superset if they want to. In my
>>> mind, anything above the ATAPI spec should be justified directly with
>>> "Guest X breaks without it."
>>>
> CD_OK removal justifications:
>
> 0x06 WIN_DSM  Defined in ACS2. Not valid for ATAPI.
> 0x21 WIN_READ_ONCERetired in ATA5. Not ATAPI in ATA4.
> 0x94 WIN_STANDBYNOW2  Retired in ATA4. Did not coexist with ATAPI.
> 0x95 WIN_IDLEIMMEDIATE2   Retired in ATA4. Did not coexist with ATAPI.
> 0x96 WIN_STANDBY2 Retired in ATA4. Did not coexist with ATAPI.
> 0x97 WIN_SETIDLE2 Retired in ATA4. Did not coexist with ATAPI.
> 0x98 WIN_CHECKPOWERMODE2  Retired in ATA4. Did not coexist with ATAPI.
> 0x99 WIN_SLEEPNOW2Retired in ATA4. Did not coexist with ATAPI.
> 0xE0 WIN_STANDBYNOW1  Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE1 WIN_IDLEIMMDIATE Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE2 WIN_STANDBY  Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE3 WIN_SETIDLE1 Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE4 WIN_CHECKPOWERMODE1  Not part of ATAPI in ATA4, ACS or ACS3.
> 0xE5 WIN_SLEEPNOW1Not part of ATAPI in ATA4, ACS or ACS3.
> 0xF8 WIN_READ_NATIVE_MAX  Obsoleted in ACS3. Not ATAPI in ATA4 or ACS.

 Actual patch matches this list.

> This patch fixes a divide by zero fault that can be caused by sending
> the WIN_READ_NATIVE_MAX command to an empty ATAPI drive, which causes
> it to attempt to use zeroed CHS values to perform sector arithmetic.
>
> Reported-by: Qinghao Tang 
> Signed-off-by: John Snow 

 I appreciate you going to the root of the problem instead of merely
 fixing the narrow bug.

 Could a similar argument be made for dropping CFA_OK from some commands?

>>>
>>> Very likely, but that's another patch. I didn't audit that yet.
>>>
 Do we still need this conditional in cmd_read_native_max()?

 /* Refuse if no sectors are addressable (e.g. medium not inserted) */
 if (s->nb_sectors == 0) {
 ide_abort_command(s);
 return true;
 }

 Why does it fail at guarding the CHS use from empty ATAPI drives before
 your patch?

>>>
>>> Because I misunderstood the real reason myself, and my POC test was a
>>> little bananas. This works *with* a CDROM inserted, not without.
>>>
>>> So s->nb_sectors can be non-zero, and ide_set_sector then triggers e.g.
>>> SIGFPE.
>>>
>>> If you'll save me the re-spin, I can fix that part of the commit message
>>> in my staging branch.
>> 
>> Let me paraphrase to make sure I got you.
>> 
>> If the drive is empty, the guard aborts the command correctly.
>> 
>> If the drive isn't empty, the guard doesn't abort.  The code then goes
>> on and happily uses CHS.  However, ATAPI devices don't have CHS
>> geometry, see ide_dev_initfn():
>> 
>> if (kind != IDE_CD) {
>> blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
>> if (err) {
>> error_report_err(err);
>> return -1;
>> }
>>  

[Qemu-devel] [PATCH 7/8] target-i386: Handle I/O breakpoints

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-i386/bpt_helper.c | 99 +++-
 target-i386/cpu.h|  2 +
 target-i386/helper.h |  1 +
 target-i386/translate.c  | 20 +-
 4 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index f872387..9050006 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -49,60 +49,72 @@ static inline int hw_breakpoint_len(unsigned long dr7, int 
index)
 return (len == 2) ? 8 : len + 1;
 }
 
-static void hw_breakpoint_insert(CPUX86State *env, int index)
+static int hw_breakpoint_insert(CPUX86State *env, int index)
 {
 CPUState *cs = CPU(x86_env_get_cpu(env));
-int type = 0, err = 0;
+target_ulong dr7 = env->dr[7];
+target_ulong drN = env->dr[index];
+int err = 0;
 
-switch (hw_breakpoint_type(env->dr[7], index)) {
+switch (hw_breakpoint_type(dr7, index)) {
 case DR7_TYPE_BP_INST:
-if (hw_breakpoint_enabled(env->dr[7], index)) {
-err = cpu_breakpoint_insert(cs, env->dr[index], BP_CPU,
+if (hw_breakpoint_enabled(dr7, index)) {
+err = cpu_breakpoint_insert(cs, drN, BP_CPU,
 &env->cpu_breakpoint[index]);
 }
 break;
-case DR7_TYPE_DATA_WR:
-type = BP_CPU | BP_MEM_WRITE;
-break;
+
 case DR7_TYPE_IO_RW:
-/* No support for I/O watchpoints yet */
+/* Notice when we should enable calls to bpt_io.  */
+return (hw_breakpoint_enabled(env->dr[7], index)
+? HF_IOBPT_MASK : 0);
+
+case DR7_TYPE_DATA_WR:
+if (hw_breakpoint_enabled(dr7, index)) {
+err = cpu_watchpoint_insert(cs, drN,
+hw_breakpoint_len(dr7, index),
+BP_CPU | BP_MEM_WRITE,
+&env->cpu_watchpoint[index]);
+}
 break;
+
 case DR7_TYPE_DATA_RW:
-type = BP_CPU | BP_MEM_ACCESS;
+if (hw_breakpoint_enabled(dr7, index)) {
+err = cpu_watchpoint_insert(cs, drN,
+hw_breakpoint_len(dr7, index),
+BP_CPU | BP_MEM_ACCESS,
+&env->cpu_watchpoint[index]);
+}
 break;
 }
-
-if (type != 0) {
-err = cpu_watchpoint_insert(cs, env->dr[index],
-hw_breakpoint_len(env->dr[7], index),
-type, &env->cpu_watchpoint[index]);
-}
-
 if (err) {
 env->cpu_breakpoint[index] = NULL;
 }
+return 0;
 }
 
 static void hw_breakpoint_remove(CPUX86State *env, int index)
 {
-CPUState *cs;
+CPUState *cs = CPU(x86_env_get_cpu(env));
 
-if (!env->cpu_breakpoint[index]) {
-return;
-}
-cs = CPU(x86_env_get_cpu(env));
 switch (hw_breakpoint_type(env->dr[7], index)) {
 case DR7_TYPE_BP_INST:
-if (hw_breakpoint_enabled(env->dr[7], index)) {
+if (env->cpu_breakpoint[index]) {
 cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
+env->cpu_breakpoint[index] = NULL;
 }
 break;
+
 case DR7_TYPE_DATA_WR:
 case DR7_TYPE_DATA_RW:
-cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+if (env->cpu_breakpoint[index]) {
+cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+env->cpu_breakpoint[index] = NULL;
+}
 break;
+
 case DR7_TYPE_IO_RW:
-/* No support for I/O watchpoints yet */
+/* HF_IOBPT_MASK cleared elsewhere.  */
 break;
 }
 }
@@ -110,6 +122,7 @@ static void hw_breakpoint_remove(CPUX86State *env, int 
index)
 void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
 {
 target_ulong old_dr7 = env->dr[7];
+int iobpt = 0;
 int i;
 
 /* If nothing is changing except the global/local enable bits,
@@ -125,10 +138,13 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t 
new_dr7)
 /* We know that register i has changed enable state;
recheck what that state should be and apply.  */
 if (hw_breakpoint_enabled(new_dr7, i)) {
-hw_breakpoint_insert(env, i);
+iobpt |= hw_breakpoint_insert(env, i);
 } else {
 hw_breakpoint_remove(env, i);
 }
+} else if (hw_breakpoint_type(new_dr7, i) == DR7_TYPE_IO_RW
+   && hw_breakpoint_enabled(new_dr7, i)) {
+iobpt |= HF_IOBPT_MASK;
 }
 }
 } else {
@@ -137,9 +153,11 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
 }
 env->dr[7] = new_dr7;
 for (i = 0; i < DR7_MAX_BP; i++) {
-hw_breakpoint_insert(env, i);
+ 

[Qemu-devel] [PATCH 8/8] target-i386: Check CR4[DE] for processing DR4/DR5

[Richard Henderson]

Introduce helper_get_dr so that we don't have to put CR4[DE]
into the scarce HFLAGS resource.  At the same time, rename
helper_movl_drN_T0 to helper_set_dr and set the helper flags.

Signed-off-by: Richard Henderson 
---
 target-i386/bpt_helper.c | 46 +-
 target-i386/cpu.h|  2 +-
 target-i386/helper.h |  3 ++-
 target-i386/translate.c  | 10 ++
 4 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 9050006..c258598 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -240,10 +240,11 @@ void helper_single_step(CPUX86State *env)
 raise_exception(env, EXCP01_DB);
 }
 
-void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
+void helper_set_dr(CPUX86State *env, int reg, target_ulong t0)
 {
 #ifndef CONFIG_USER_ONLY
-if (reg < 4) {
+switch (reg) {
+case 0: case 1: case 2: case 3:
 if (hw_breakpoint_enabled(env->dr[7], reg)
 && hw_breakpoint_type(env->dr[7], reg) != DR7_TYPE_IO_RW) {
 hw_breakpoint_remove(env, reg);
@@ -252,14 +253,49 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, 
target_ulong t0)
 } else {
 env->dr[reg] = t0;
 }
-} else if (reg == 7) {
+return;
+case 4:
+if (env->cr[4] & CR4_DE_MASK) {
+break;
+}
+/* fallthru */
+case 6:
+env->dr[6] = t0;
+return;
+case 5:
+if (env->cr[4] & CR4_DE_MASK) {
+break;
+}
+/* fallthru */
+case 7:
 cpu_x86_update_dr7(env, t0);
-} else {
-env->dr[reg] = t0;
+return;
 }
+raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
 #endif
 }
 
+target_ulong helper_get_dr(CPUX86State *env, int reg)
+{
+switch (reg) {
+case 0: case 1: case 2: case 3: case 6: case 7:
+return env->dr[reg];
+case 4:
+if (env->cr[4] & CR4_DE_MASK) {
+break;
+} else {
+return env->dr[6];
+}
+case 5:
+if (env->cr[4] & CR4_DE_MASK) {
+break;
+} else {
+return env->dr[7];
+}
+}
+raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+}
+
 /* Check if Port I/O is trapped by a breakpoint.  */
 void helper_bpt_io(CPUX86State *env, uint32_t port,
uint32_t size, target_ulong next_eip)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 74914f0..e71fb1b 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -916,7 +916,7 @@ typedef struct CPUX86State {
 int error_code;
 int exception_is_int;
 target_ulong exception_next_eip;
-target_ulong dr[8]; /* debug registers */
+target_ulong dr[8]; /* debug registers; note dr4 and dr5 are unused */
 union {
 struct CPUBreakpoint *cpu_breakpoint[4];
 struct CPUWatchpoint *cpu_watchpoint[4];
diff --git a/target-i386/helper.h b/target-i386/helper.h
index e9858c0..ecfcfd1 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -40,7 +40,8 @@ DEF_HELPER_2(read_crN, tl, env, int)
 DEF_HELPER_3(write_crN, void, env, int, tl)
 DEF_HELPER_2(lmsw, void, env, tl)
 DEF_HELPER_1(clts, void, env)
-DEF_HELPER_3(movl_drN_T0, void, env, int, tl)
+DEF_HELPER_FLAGS_3(set_dr, TCG_CALL_NO_WG, void, env, int, tl)
+DEF_HELPER_FLAGS_2(get_dr, TCG_CALL_NO_WG, tl, env, int)
 DEF_HELPER_2(invlpg, void, env, tl)
 
 DEF_HELPER_4(enter_level, void, env, int, int, tl)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 2c06f01..fbef312 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7634,18 +7634,20 @@ static target_ulong disas_insn(CPUX86State *env, 
DisasContext *s,
 ot = MO_64;
 else
 ot = MO_32;
-/* XXX: do it dynamically with CR4.DE bit */
-if (reg == 4 || reg == 5 || reg >= 8)
+if (reg >= 8) {
 goto illegal_op;
+}
 if (b & 2) {
 gen_svm_check_intercept(s, pc_start, SVM_EXIT_WRITE_DR0 + reg);
 gen_op_mov_v_reg(ot, cpu_T[0], rm);
-gen_helper_movl_drN_T0(cpu_env, tcg_const_i32(reg), cpu_T[0]);
+tcg_gen_movi_i32(cpu_tmp2_i32, reg);
+gen_helper_set_dr(cpu_env, cpu_tmp2_i32, cpu_T[0]);
 gen_jmp_im(s->pc - s->cs_base);
 gen_eob(s);
 } else {
 gen_svm_check_intercept(s, pc_start, SVM_EXIT_READ_DR0 + reg);
-tcg_gen_ld_tl(cpu_T[0], cpu_env, 
offsetof(CPUX86State,dr[reg]));
+tcg_gen_movi_i32(cpu_tmp2_i32, reg);
+gen_helper_get_dr(cpu_T[0], cpu_env, cpu_tmp2_i32);
 gen_op_mov_reg_v(ot, rm, cpu_T[0]);
 }
 }
-- 
2.1.0

[Qemu-devel] [PATCH 6/8] target-i386: Optimize setting dr[0-3]

[Richard Henderson]

If the debug register is not enabled, we need
do nothing besides update the register.

Signed-off-by: Richard Henderson 
---
 target-i386/bpt_helper.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 10bbf9f..f872387 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -226,9 +226,14 @@ void helper_movl_drN_T0(CPUX86State *env, int reg, 
target_ulong t0)
 {
 #ifndef CONFIG_USER_ONLY
 if (reg < 4) {
-hw_breakpoint_remove(env, reg);
-env->dr[reg] = t0;
-hw_breakpoint_insert(env, reg);
+if (hw_breakpoint_enabled(env->dr[7], reg)
+&& hw_breakpoint_type(env->dr[7], reg) != DR7_TYPE_IO_RW) {
+hw_breakpoint_remove(env, reg);
+env->dr[reg] = t0;
+hw_breakpoint_insert(env, reg);
+} else {
+env->dr[reg] = t0;
+}
 } else if (reg == 7) {
 cpu_x86_update_dr7(env, t0);
 } else {
-- 
2.1.0

[Qemu-devel] [PATCH 5/8] target-i386: Move hw_*breakpoint_* functions

[Richard Henderson]

They're only used from bpt_helper.c now.

Signed-off-by: Richard Henderson 
---
 target-i386/bpt_helper.c | 29 -
 target-i386/cpu.h| 27 ---
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 911d9f8..10bbf9f 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -22,6 +22,33 @@
 
 
 #ifndef CONFIG_USER_ONLY
+static inline bool hw_local_breakpoint_enabled(unsigned long dr7, int index)
+{
+return (dr7 >> (index * 2)) & 1;
+}
+
+static inline bool hw_global_breakpoint_enabled(unsigned long dr7, int index)
+{
+return (dr7 >> (index * 2)) & 2;
+
+}
+static inline bool hw_breakpoint_enabled(unsigned long dr7, int index)
+{
+return hw_global_breakpoint_enabled(dr7, index) ||
+   hw_local_breakpoint_enabled(dr7, index);
+}
+
+static inline int hw_breakpoint_type(unsigned long dr7, int index)
+{
+return (dr7 >> (DR7_TYPE_SHIFT + (index * 4))) & 3;
+}
+
+static inline int hw_breakpoint_len(unsigned long dr7, int index)
+{
+int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 4))) & 3);
+return (len == 2) ? 8 : len + 1;
+}
+
 static void hw_breakpoint_insert(CPUX86State *env, int index)
 {
 CPUState *cs = CPU(x86_env_get_cpu(env));
@@ -114,7 +141,6 @@ void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
 }
 }
 }
-#endif
 
 static bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
 {
@@ -185,6 +211,7 @@ void breakpoint_handler(CPUState *cs)
 }
 }
 }
+#endif
 
 void helper_single_step(CPUX86State *env)
 {
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 6d3e4f9..8f07db1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1124,33 +1124,6 @@ void x86_stl_phys(CPUState *cs, hwaddr addr, uint32_t 
val);
 void x86_stq_phys(CPUState *cs, hwaddr addr, uint64_t val);
 #endif
 
-static inline bool hw_local_breakpoint_enabled(unsigned long dr7, int index)
-{
-return (dr7 >> (index * 2)) & 1;
-}
-
-static inline bool hw_global_breakpoint_enabled(unsigned long dr7, int index)
-{
-return (dr7 >> (index * 2)) & 2;
-
-}
-static inline bool hw_breakpoint_enabled(unsigned long dr7, int index)
-{
-return hw_global_breakpoint_enabled(dr7, index) ||
-   hw_local_breakpoint_enabled(dr7, index);
-}
-
-static inline int hw_breakpoint_type(unsigned long dr7, int index)
-{
-return (dr7 >> (DR7_TYPE_SHIFT + (index * 4))) & 3;
-}
-
-static inline int hw_breakpoint_len(unsigned long dr7, int index)
-{
-int len = ((dr7 >> (DR7_LEN_SHIFT + (index * 4))) & 3);
-return (len == 2) ? 8 : len + 1;
-}
-
 void breakpoint_handler(CPUState *cs);
 
 /* will be suppressed */
-- 
2.1.0

[Qemu-devel] [PATCH 4/8] target-i386: Re-introduce optimal breakpoint removal

[Richard Henderson]

Before the last patch, we had an efficient loop that disabled
local breakpoints on task switch.  Re-add that, but in a more
general way that handles changes to the global enable bits too.

Signed-off-by: Richard Henderson 
---
 target-i386/bpt_helper.c | 34 --
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index f14788a..911d9f8 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -82,14 +82,36 @@ static void hw_breakpoint_remove(CPUX86State *env, int 
index)
 
 void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
 {
+target_ulong old_dr7 = env->dr[7];
 int i;
 
-for (i = 0; i < DR7_MAX_BP; i++) {
-hw_breakpoint_remove(env, i);
-}
-env->dr[7] = new_dr7;
-for (i = 0; i < DR7_MAX_BP; i++) {
-hw_breakpoint_insert(env, i);
+/* If nothing is changing except the global/local enable bits,
+   then we can make the change more efficient.  */
+if (((old_dr7 ^ new_dr7) & ~0xff) == 0) {
+/* Fold the global and local enable bits together into the
+   global fields, then xor to show which registers have
+   changed collective enable state.  */
+int mod = ((old_dr7 | old_dr7 * 2) ^ (new_dr7 | new_dr7 * 2)) & 0xff;
+
+for (i = 0; i < DR7_MAX_BP; i++) {
+if (mod & (2 << i * 2)) {
+/* We know that register i has changed enable state;
+   recheck what that state should be and apply.  */
+if (hw_breakpoint_enabled(new_dr7, i)) {
+hw_breakpoint_insert(env, i);
+} else {
+hw_breakpoint_remove(env, i);
+}
+}
+}
+} else {
+for (i = 0; i < DR7_MAX_BP; i++) {
+hw_breakpoint_remove(env, i);
+}
+env->dr[7] = new_dr7;
+for (i = 0; i < DR7_MAX_BP; i++) {
+hw_breakpoint_insert(env, i);
+}
 }
 }
 #endif
-- 
2.1.0

[Qemu-devel] [PATCH 3/8] target-i386: Introduce cpu_x86_update_dr7

[Richard Henderson]

This moves the last of the iteration over breakpoints into
the bpt_helper.c file.  This also allows us to make several
breakpoint functions static.

Signed-off-by: Richard Henderson 
---
 target-i386/bpt_helper.c | 29 ++---
 target-i386/cpu.h|  4 ++--
 target-i386/machine.c|  8 ++--
 target-i386/seg_helper.c |  8 +---
 4 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index c071c24..f14788a 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -21,7 +21,8 @@
 #include "exec/helper-proto.h"
 
 
-void hw_breakpoint_insert(CPUX86State *env, int index)
+#ifndef CONFIG_USER_ONLY
+static void hw_breakpoint_insert(CPUX86State *env, int index)
 {
 CPUState *cs = CPU(x86_env_get_cpu(env));
 int type = 0, err = 0;
@@ -55,7 +56,7 @@ void hw_breakpoint_insert(CPUX86State *env, int index)
 }
 }
 
-void hw_breakpoint_remove(CPUX86State *env, int index)
+static void hw_breakpoint_remove(CPUX86State *env, int index)
 {
 CPUState *cs;
 
@@ -79,6 +80,20 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 }
 }
 
+void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7)
+{
+int i;
+
+for (i = 0; i < DR7_MAX_BP; i++) {
+hw_breakpoint_remove(env, i);
+}
+env->dr[7] = new_dr7;
+for (i = 0; i < DR7_MAX_BP; i++) {
+hw_breakpoint_insert(env, i);
+}
+}
+#endif
+
 static bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
 {
 target_ulong dr6;
@@ -161,20 +176,12 @@ void helper_single_step(CPUX86State *env)
 void helper_movl_drN_T0(CPUX86State *env, int reg, target_ulong t0)
 {
 #ifndef CONFIG_USER_ONLY
-int i;
-
 if (reg < 4) {
 hw_breakpoint_remove(env, reg);
 env->dr[reg] = t0;
 hw_breakpoint_insert(env, reg);
 } else if (reg == 7) {
-for (i = 0; i < DR7_MAX_BP; i++) {
-hw_breakpoint_remove(env, i);
-}
-env->dr[7] = t0;
-for (i = 0; i < DR7_MAX_BP; i++) {
-hw_breakpoint_insert(env, i);
-}
+cpu_x86_update_dr7(env, t0);
 } else {
 env->dr[reg] = t0;
 }
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9189a63..6d3e4f9 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -234,6 +234,7 @@
 #define DR7_TYPE_SHIFT  16
 #define DR7_LEN_SHIFT   18
 #define DR7_FIXED_1 0x0400
+#define DR7_GLOBAL_BP_MASK   0xaa
 #define DR7_LOCAL_BP_MASK0x55
 #define DR7_MAX_BP   4
 #define DR7_TYPE_BP_INST 0x0
@@ -1150,14 +1151,13 @@ static inline int hw_breakpoint_len(unsigned long dr7, 
int index)
 return (len == 2) ? 8 : len + 1;
 }
 
-void hw_breakpoint_insert(CPUX86State *env, int index);
-void hw_breakpoint_remove(CPUX86State *env, int index);
 void breakpoint_handler(CPUState *cs);
 
 /* will be suppressed */
 void cpu_x86_update_cr0(CPUX86State *env, uint32_t new_cr0);
 void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3);
 void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4);
+void cpu_x86_update_dr7(CPUX86State *env, uint32_t new_dr7);
 
 /* hw/pc.c */
 uint64_t cpu_get_tsc(CPUX86State *env);
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a0df64b..782d057 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -367,8 +367,12 @@ static int cpu_post_load(void *opaque, int version_id)
 
 cpu_breakpoint_remove_all(cs, BP_CPU);
 cpu_watchpoint_remove_all(cs, BP_CPU);
-for (i = 0; i < DR7_MAX_BP; i++) {
-hw_breakpoint_insert(env, i);
+{
+/* Indicate all breakpoints disabled, as they are, then
+   let the helper re-enable them.  */
+target_ulong dr7 = env->dr[7];
+env->dr[7] = dr7 & ~(DR7_GLOBAL_BP_MASK | DR7_LOCAL_BP_MASK);
+cpu_x86_update_dr7(env, dr7);
 }
 tlb_flush(cs, 1);
 
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 1a3a2e7..0257706 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -501,13 +501,7 @@ static void switch_tss_ra(CPUX86State *env, int 
tss_selector,
 #ifndef CONFIG_USER_ONLY
 /* reset local breakpoints */
 if (env->dr[7] & DR7_LOCAL_BP_MASK) {
-for (i = 0; i < DR7_MAX_BP; i++) {
-if (hw_local_breakpoint_enabled(env->dr[7], i) &&
-!hw_global_breakpoint_enabled(env->dr[7], i)) {
-hw_breakpoint_remove(env, i);
-}
-}
-env->dr[7] &= ~DR7_LOCAL_BP_MASK;
+cpu_x86_update_dr7(env, env->dr[7] & ~DR7_LOCAL_BP_MASK);
 }
 #endif
 }
-- 
2.1.0

[Qemu-devel] [PATCH 1/8] target-i386: Move breakpoint related functions to new file

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-i386/Makefile.objs |   2 +-
 target-i386/bpt_helper.c  | 182 ++
 target-i386/helper.c  | 128 
 target-i386/misc_helper.c |  34 -
 4 files changed, 183 insertions(+), 163 deletions(-)
 create mode 100644 target-i386/bpt_helper.c

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 7a1df2c..df51db5 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += translate.o helper.o cpu.o
+obj-y += translate.o helper.o cpu.o bpt_helper.o
 obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o
 obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o
 obj-y += gdbstub.o
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
new file mode 100644
index 000..6f6537d
--- /dev/null
+++ b/target-i386/bpt_helper.c
@@ -0,0 +1,182 @@
+/*
+ *  i386 breakpoint helpers
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * 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 "cpu.h"
+#include "exec/helper-proto.h"
+
+
+void hw_breakpoint_insert(CPUX86State *env, int index)
+{
+CPUState *cs = CPU(x86_env_get_cpu(env));
+int type = 0, err = 0;
+
+switch (hw_breakpoint_type(env->dr[7], index)) {
+case DR7_TYPE_BP_INST:
+if (hw_breakpoint_enabled(env->dr[7], index)) {
+err = cpu_breakpoint_insert(cs, env->dr[index], BP_CPU,
+&env->cpu_breakpoint[index]);
+}
+break;
+case DR7_TYPE_DATA_WR:
+type = BP_CPU | BP_MEM_WRITE;
+break;
+case DR7_TYPE_IO_RW:
+/* No support for I/O watchpoints yet */
+break;
+case DR7_TYPE_DATA_RW:
+type = BP_CPU | BP_MEM_ACCESS;
+break;
+}
+
+if (type != 0) {
+err = cpu_watchpoint_insert(cs, env->dr[index],
+hw_breakpoint_len(env->dr[7], index),
+type, &env->cpu_watchpoint[index]);
+}
+
+if (err) {
+env->cpu_breakpoint[index] = NULL;
+}
+}
+
+void hw_breakpoint_remove(CPUX86State *env, int index)
+{
+CPUState *cs;
+
+if (!env->cpu_breakpoint[index]) {
+return;
+}
+cs = CPU(x86_env_get_cpu(env));
+switch (hw_breakpoint_type(env->dr[7], index)) {
+case DR7_TYPE_BP_INST:
+if (hw_breakpoint_enabled(env->dr[7], index)) {
+cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
+}
+break;
+case DR7_TYPE_DATA_WR:
+case DR7_TYPE_DATA_RW:
+cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+break;
+case DR7_TYPE_IO_RW:
+/* No support for I/O watchpoints yet */
+break;
+}
+}
+
+bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
+{
+target_ulong dr6;
+int reg;
+bool hit_enabled = false;
+
+dr6 = env->dr[6] & ~0xf;
+for (reg = 0; reg < DR7_MAX_BP; reg++) {
+bool bp_match = false;
+bool wp_match = false;
+
+switch (hw_breakpoint_type(env->dr[7], reg)) {
+case DR7_TYPE_BP_INST:
+if (env->dr[reg] == env->eip) {
+bp_match = true;
+}
+break;
+case DR7_TYPE_DATA_WR:
+case DR7_TYPE_DATA_RW:
+if (env->cpu_watchpoint[reg] &&
+env->cpu_watchpoint[reg]->flags & BP_WATCHPOINT_HIT) {
+wp_match = true;
+}
+break;
+case DR7_TYPE_IO_RW:
+break;
+}
+if (bp_match || wp_match) {
+dr6 |= 1 << reg;
+if (hw_breakpoint_enabled(env->dr[7], reg)) {
+hit_enabled = true;
+}
+}
+}
+
+if (hit_enabled || force_dr6_update) {
+env->dr[6] = dr6;
+}
+
+return hit_enabled;
+}
+
+void breakpoint_handler(CPUState *cs)
+{
+X86CPU *cpu = X86_CPU(cs);
+CPUX86State *env = &cpu->env;
+CPUBreakpoint *bp;
+
+if (cs->watchpoint_hit) {
+if (cs->watchpoint_hit->flags & BP_CPU) {
+cs->watchpoint_hit = NULL;
+if (check_hw_breakpoints(env, false)) {
+raise_exception(env, EXCP01_DB);
+} else {
+cpu_resume_from_signal(cs, NULL)

[Qemu-devel] [PATCH 0/8] target-i386: Implement debug extensions

[Richard Henderson]

Best guess, since I can't find any code that actually uses them.
Linux actively turns them off at boot...


r~


Richard Henderson (8):
  target-i386: Move breakpoint related functions to new file
  target-i386: Make check_hw_breakpoints static
  target-i386: Introduce cpu_x86_update_dr7
  target-i386: Re-introduce optimal breakpoint removal
  target-i386: Move hw_*breakpoint_* functions
  target-i386: Optimize setting dr[0-3]
  target-i386: Handle I/O breakpoints
  target-i386: Check CR4[DE] for processing DR4/DR5

 target-i386/Makefile.objs |   2 +-
 target-i386/bpt_helper.c  | 324 ++
 target-i386/cpu.h |  36 +-
 target-i386/helper.c  | 128 --
 target-i386/helper.h  |   4 +-
 target-i386/machine.c |   8 +-
 target-i386/misc_helper.c |  34 -
 target-i386/seg_helper.c  |   8 +-
 target-i386/translate.c   |  30 -
 9 files changed, 365 insertions(+), 209 deletions(-)
 create mode 100644 target-i386/bpt_helper.c

-- 
2.1.0

[Qemu-devel] [PATCH 2/8] target-i386: Make check_hw_breakpoints static

[Richard Henderson]

The function is now only used from within a single file.

Signed-off-by: Richard Henderson 
---
 target-i386/bpt_helper.c | 2 +-
 target-i386/cpu.h| 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index 6f6537d..c071c24 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -79,7 +79,7 @@ void hw_breakpoint_remove(CPUX86State *env, int index)
 }
 }
 
-bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
+static bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update)
 {
 target_ulong dr6;
 int reg;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 3bcf2f6..9189a63 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1152,7 +1152,6 @@ static inline int hw_breakpoint_len(unsigned long dr7, 
int index)
 
 void hw_breakpoint_insert(CPUX86State *env, int index);
 void hw_breakpoint_remove(CPUX86State *env, int index);
-bool check_hw_breakpoints(CPUX86State *env, bool force_dr6_update);
 void breakpoint_handler(CPUState *cs);
 
 /* will be suppressed */
-- 
2.1.0

[Qemu-devel] Canceled Event: Dr Velano @ Wed Sep 16, 2015 17:50 - 18:50 (shlomopongr...@gmail.com)

[shlomopongratz]

BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:CANCEL
BEGIN:VEVENT
DTSTART:20150916T145000Z
DTEND:20150916T155000Z
DTSTAMP:20150915T184155Z
ORGANIZER;CN=Shlomo Pongratz:mailto:shlomopongr...@gmail.com
UID:mhfgd22vmap7kbtja2d8p70...@google.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=DECLINED;CN=qemu-d
 e...@nongnu.org;X-NUM-GUESTS=0:mailto:qemu-devel@nongnu.org
CREATED:20150912T140307Z
DESCRIPTION:
LAST-MODIFIED:20150915T184154Z
LOCATION:Uri Tsvi Grinberg Street\, Tel Aviv-Yafo\, Israel 25
SEQUENCE:1
STATUS:CONFIRMED
SUMMARY:Dr Velano
TRANSP:OPAQUE
BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:This is an event reminder
TRIGGER:-P1D
END:VALARM
BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:This is an event reminder
TRIGGER:-P0DT1H0M0S
END:VALARM
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

Re: [Qemu-devel] [PATCH] ide: fix ATAPI command permissions

[John Snow]

On 09/15/2015 02:11 PM, Markus Armbruster wrote:
> John Snow  writes:
> 
>> On 09/15/2015 02:53 AM, Markus Armbruster wrote:
>>> John Snow  writes:
>>>
 We're a little too lenient with what we'll let an ATAPI drive handle.
 Clamp down on the IDE command execution table to remove CD_OK permissions
 from commands that are not and have never been ATAPI commands.

 For ATAPI command validity, please see:
 - ATA4 Section 6.5 ("PACKET Command feature set")
 - ATA8/ACS Section 4.3 ("The PACKET feature set")
 - ACS3 Section 4.3 ("The PACKET feature set")

 ACS3 has a historical command validity table in Table B.4
 ("Historical Command Assignments") that can be referenced to find when
 a command was introduced, deprecated, obsoleted, etc.

 The only reference for ATAPI command validity is by checking that
 version's PACKET feature set section.

 ATAPI was introduced by T13 into ATA4, all commands retired prior to ATA4
 therefore are assumed to have never been ATAPI commands.

 Mandatory commands, as listed in ATA8-ACS3, are:

 - DEVICE RESET
 - EXECUTE DEVICE DIAGNOSTIC
 - IDENTIFY DEVICE
 - IDENTIFY PACKET DEVICE
 - NOP
 - PACKET
 - READ SECTOR(S)
 - SET FEATURES

 Optional commands as listed in ATA8-ACS3, are:

 - FLUSH CACHE
 - READ LOG DMA EXT
 - READ LOG EXT
 - WRITE LOG DMA EXT
 - WRITE LOG EXT

 All other commands are illegal to send to an ATAPI device and should
 be rejected by the device.
>>>
>>> We could perhaps argue about "should be rejected by the device", but I
>>> think the weaker "a device is free to reject it" still suffices to
>>> support your patch.
>>>
>>
>> Sure -- I suppose drives CAN support a superset if they want to. In my
>> mind, anything above the ATAPI spec should be justified directly with
>> "Guest X breaks without it."
>>
 CD_OK removal justifications:

 0x06 WIN_DSM  Defined in ACS2. Not valid for ATAPI.
 0x21 WIN_READ_ONCERetired in ATA5. Not ATAPI in ATA4.
 0x94 WIN_STANDBYNOW2  Retired in ATA4. Did not coexist with ATAPI.
 0x95 WIN_IDLEIMMEDIATE2   Retired in ATA4. Did not coexist with ATAPI.
 0x96 WIN_STANDBY2 Retired in ATA4. Did not coexist with ATAPI.
 0x97 WIN_SETIDLE2 Retired in ATA4. Did not coexist with ATAPI.
 0x98 WIN_CHECKPOWERMODE2  Retired in ATA4. Did not coexist with ATAPI.
 0x99 WIN_SLEEPNOW2Retired in ATA4. Did not coexist with ATAPI.
 0xE0 WIN_STANDBYNOW1  Not part of ATAPI in ATA4, ACS or ACS3.
 0xE1 WIN_IDLEIMMDIATE Not part of ATAPI in ATA4, ACS or ACS3.
 0xE2 WIN_STANDBY  Not part of ATAPI in ATA4, ACS or ACS3.
 0xE3 WIN_SETIDLE1 Not part of ATAPI in ATA4, ACS or ACS3.
 0xE4 WIN_CHECKPOWERMODE1  Not part of ATAPI in ATA4, ACS or ACS3.
 0xE5 WIN_SLEEPNOW1Not part of ATAPI in ATA4, ACS or ACS3.
 0xF8 WIN_READ_NATIVE_MAX  Obsoleted in ACS3. Not ATAPI in ATA4 or ACS.
>>>
>>> Actual patch matches this list.
>>>
 This patch fixes a divide by zero fault that can be caused by sending
 the WIN_READ_NATIVE_MAX command to an empty ATAPI drive, which causes
 it to attempt to use zeroed CHS values to perform sector arithmetic.

 Reported-by: Qinghao Tang 
 Signed-off-by: John Snow 
>>>
>>> I appreciate you going to the root of the problem instead of merely
>>> fixing the narrow bug.
>>>
>>> Could a similar argument be made for dropping CFA_OK from some commands?
>>>
>>
>> Very likely, but that's another patch. I didn't audit that yet.
>>
>>> Do we still need this conditional in cmd_read_native_max()?
>>>
>>> /* Refuse if no sectors are addressable (e.g. medium not inserted) */
>>> if (s->nb_sectors == 0) {
>>> ide_abort_command(s);
>>> return true;
>>> }
>>>
>>> Why does it fail at guarding the CHS use from empty ATAPI drives before
>>> your patch?
>>>
>>
>> Because I misunderstood the real reason myself, and my POC test was a
>> little bananas. This works *with* a CDROM inserted, not without.
>>
>> So s->nb_sectors can be non-zero, and ide_set_sector then triggers e.g.
>> SIGFPE.
>>
>> If you'll save me the re-spin, I can fix that part of the commit message
>> in my staging branch.
> 
> Let me paraphrase to make sure I got you.
> 
> If the drive is empty, the guard aborts the command correctly.
> 
> If the drive isn't empty, the guard doesn't abort.  The code then goes
> on and happily uses CHS.  However, ATAPI devices don't have CHS
> geometry, see ide_dev_initfn():
> 
> if (kind != IDE_CD) {
> blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
> if (err) {
> error_report_err(err);
> return -1;
> }
> }
> 
> Therefore, CHS is all zero, and the code using it blows up.
> 
> Correct?
> 

Indeed. I had wrongly assumed previously that the CHS valu

Re: [Qemu-devel] [PATCH v7 00/26] qapi: QMP introspection

[Markus Armbruster]

Commit 351d36e "qapi: allow override of default enum prefix naming" got
in first.  The rebase is non-trivial, so a full respin is necessary.
Clock for pull request will be reset...

Re: [Qemu-devel] [PATCH RFC v5 00/32] qapi: QMP introspection

[Markus Armbruster]

Commit 351d36e "qapi: allow override of default enum prefix naming" got
in first.  The rebase is non-trivial, so a full respin is necessary.
Clock for pull request will be reset...

Re: [Qemu-devel] [PATCH] ide: fix ATAPI command permissions

[Markus Armbruster]

John Snow  writes:

> On 09/15/2015 02:53 AM, Markus Armbruster wrote:
>> John Snow  writes:
>> 
>>> We're a little too lenient with what we'll let an ATAPI drive handle.
>>> Clamp down on the IDE command execution table to remove CD_OK permissions
>>> from commands that are not and have never been ATAPI commands.
>>>
>>> For ATAPI command validity, please see:
>>> - ATA4 Section 6.5 ("PACKET Command feature set")
>>> - ATA8/ACS Section 4.3 ("The PACKET feature set")
>>> - ACS3 Section 4.3 ("The PACKET feature set")
>>>
>>> ACS3 has a historical command validity table in Table B.4
>>> ("Historical Command Assignments") that can be referenced to find when
>>> a command was introduced, deprecated, obsoleted, etc.
>>>
>>> The only reference for ATAPI command validity is by checking that
>>> version's PACKET feature set section.
>>>
>>> ATAPI was introduced by T13 into ATA4, all commands retired prior to ATA4
>>> therefore are assumed to have never been ATAPI commands.
>>>
>>> Mandatory commands, as listed in ATA8-ACS3, are:
>>>
>>> - DEVICE RESET
>>> - EXECUTE DEVICE DIAGNOSTIC
>>> - IDENTIFY DEVICE
>>> - IDENTIFY PACKET DEVICE
>>> - NOP
>>> - PACKET
>>> - READ SECTOR(S)
>>> - SET FEATURES
>>>
>>> Optional commands as listed in ATA8-ACS3, are:
>>>
>>> - FLUSH CACHE
>>> - READ LOG DMA EXT
>>> - READ LOG EXT
>>> - WRITE LOG DMA EXT
>>> - WRITE LOG EXT
>>>
>>> All other commands are illegal to send to an ATAPI device and should
>>> be rejected by the device.
>> 
>> We could perhaps argue about "should be rejected by the device", but I
>> think the weaker "a device is free to reject it" still suffices to
>> support your patch.
>> 
>
> Sure -- I suppose drives CAN support a superset if they want to. In my
> mind, anything above the ATAPI spec should be justified directly with
> "Guest X breaks without it."
>
>>> CD_OK removal justifications:
>>>
>>> 0x06 WIN_DSM  Defined in ACS2. Not valid for ATAPI.
>>> 0x21 WIN_READ_ONCERetired in ATA5. Not ATAPI in ATA4.
>>> 0x94 WIN_STANDBYNOW2  Retired in ATA4. Did not coexist with ATAPI.
>>> 0x95 WIN_IDLEIMMEDIATE2   Retired in ATA4. Did not coexist with ATAPI.
>>> 0x96 WIN_STANDBY2 Retired in ATA4. Did not coexist with ATAPI.
>>> 0x97 WIN_SETIDLE2 Retired in ATA4. Did not coexist with ATAPI.
>>> 0x98 WIN_CHECKPOWERMODE2  Retired in ATA4. Did not coexist with ATAPI.
>>> 0x99 WIN_SLEEPNOW2Retired in ATA4. Did not coexist with ATAPI.
>>> 0xE0 WIN_STANDBYNOW1  Not part of ATAPI in ATA4, ACS or ACS3.
>>> 0xE1 WIN_IDLEIMMDIATE Not part of ATAPI in ATA4, ACS or ACS3.
>>> 0xE2 WIN_STANDBY  Not part of ATAPI in ATA4, ACS or ACS3.
>>> 0xE3 WIN_SETIDLE1 Not part of ATAPI in ATA4, ACS or ACS3.
>>> 0xE4 WIN_CHECKPOWERMODE1  Not part of ATAPI in ATA4, ACS or ACS3.
>>> 0xE5 WIN_SLEEPNOW1Not part of ATAPI in ATA4, ACS or ACS3.
>>> 0xF8 WIN_READ_NATIVE_MAX  Obsoleted in ACS3. Not ATAPI in ATA4 or ACS.
>> 
>> Actual patch matches this list.
>> 
>>> This patch fixes a divide by zero fault that can be caused by sending
>>> the WIN_READ_NATIVE_MAX command to an empty ATAPI drive, which causes
>>> it to attempt to use zeroed CHS values to perform sector arithmetic.
>>>
>>> Reported-by: Qinghao Tang 
>>> Signed-off-by: John Snow 
>> 
>> I appreciate you going to the root of the problem instead of merely
>> fixing the narrow bug.
>> 
>> Could a similar argument be made for dropping CFA_OK from some commands?
>> 
>
> Very likely, but that's another patch. I didn't audit that yet.
>
>> Do we still need this conditional in cmd_read_native_max()?
>> 
>> /* Refuse if no sectors are addressable (e.g. medium not inserted) */
>> if (s->nb_sectors == 0) {
>> ide_abort_command(s);
>> return true;
>> }
>> 
>> Why does it fail at guarding the CHS use from empty ATAPI drives before
>> your patch?
>> 
>
> Because I misunderstood the real reason myself, and my POC test was a
> little bananas. This works *with* a CDROM inserted, not without.
>
> So s->nb_sectors can be non-zero, and ide_set_sector then triggers e.g.
> SIGFPE.
>
> If you'll save me the re-spin, I can fix that part of the commit message
> in my staging branch.

Let me paraphrase to make sure I got you.

If the drive is empty, the guard aborts the command correctly.

If the drive isn't empty, the guard doesn't abort.  The code then goes
on and happily uses CHS.  However, ATAPI devices don't have CHS
geometry, see ide_dev_initfn():

if (kind != IDE_CD) {
blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
if (err) {
error_report_err(err);
return -1;
}
}

Therefore, CHS is all zero, and the code using it blows up.

Correct?

Re: [Qemu-devel] [RFT PATCH v1 0/3] net: smc91c111 can_receive fixes

[Peter Crosthwaite]

On Mon, Sep 14, 2015 at 2:09 PM, Richard Purdie
 wrote:
> Hi Peter,
>
> On Thu, 2015-09-10 at 21:23 -0700, Peter Crosthwaite wrote:
>> This should hopefully fix your bug, while addressing the extra concern
>> I raised.
>>
>> There was also inconsistent behaviour with corking packets through a
>> soft reset which I notice and fixed.
>>
>> Please let me know if this works for you.
>
> I've run it through a few builds/tests in place of my own patches and so
> far it seems to be working, thanks!
>

Can we take that as a formal Tested-by? :)

Regards,
Peter

> Cheers,
>
> Richard
>

Re: [Qemu-devel] [PATCH v2] add macro file for coccinelle

[Michael Tokarev]

07.09.2015 13:21, Paolo Bonzini wrote:
> Coccinelle chokes on some idioms from compiler.h and queue.h.
> Extract those in a macro file, to be used with "--macro-file
> scripts/cocci-macro-file.h".

Applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [PATCH v2] Add .dir-locals.el file to configure emacs coding style

[Michael Tokarev]

15.09.2015 19:22, Markus Armbruster wrote:
> "Daniel P. Berrange"  writes:
[]
>> Personally I be fine with both the minimal approach or the more
>> comprehensive approach of Peter's, but I'd probably tend towards
>> the minimal approach to avoid the warnings problem.
> 
> Seconded.
> 
> The "minimal approach" has been on the list since June, and it has my
> R-by.  It's clearly better than nothing.  Let's commit it now.  We can
> always improve on it later.

Ok, let's apply it.  BTW, Marcus, your R-By wasn't actually clear,
to which version it applies.  Now I see that after your comment.

Applied, finally, to -trivial now, thank you all!

Should I add some more r-by or s-by? :)

Thanks!

/mjt

Re: [Qemu-devel] [PATCH v2 09/18] nvdimm: build ACPI NFIT table

[Igor Mammedov]

On Tue, 15 Sep 2015 18:12:43 +0200
Paolo Bonzini  wrote:

> 
> 
> On 14/08/2015 16:52, Xiao Guangrong wrote:
> > NFIT is defined in ACPI 6.0: 5.2.25 NVDIMM Firmware Interface Table (NFIT)
> > 
> > Currently, we only support PMEM mode. Each device has 3 tables:
> > - SPA table, define the PMEM region info
> > 
> > - MEM DEV table, it has the @handle which is used to associate specified
> >   ACPI NVDIMM  device we will introduce in later patch.
> >   Also we can happily ignored the memory device's interleave, the real
> >   nvdimm hardware access is hidden behind host
> > 
> > - DCR table, it defines Vendor ID used to associate specified vendor
> >   nvdimm driver. Since we only implement PMEM mode this time, Command
> >   window and Data window are not needed
> > 
> > Signed-off-by: Xiao Guangrong 
> > ---
> >  hw/i386/acpi-build.c   |   3 +
> >  hw/mem/Makefile.objs   |   2 +-
> >  hw/mem/nvdimm/acpi.c   | 285 
> > +
> >  hw/mem/nvdimm/internal.h   |  29 +
> >  hw/mem/nvdimm/pc-nvdimm.c  |  27 -
> >  include/hw/mem/pc-nvdimm.h |   2 +
> >  6 files changed, 346 insertions(+), 2 deletions(-)
> >  create mode 100644 hw/mem/nvdimm/acpi.c
> >  create mode 100644 hw/mem/nvdimm/internal.h
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 8ead1c1..092ed2f 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -39,6 +39,7 @@
> >  #include "hw/loader.h"
> >  #include "hw/isa/isa.h"
> >  #include "hw/acpi/memory_hotplug.h"
> > +#include "hw/mem/pc-nvdimm.h"
> >  #include "sysemu/tpm.h"
> >  #include "hw/acpi/tpm.h"
> >  #include "sysemu/tpm_backend.h"
> > @@ -1741,6 +1742,8 @@ void acpi_build(PcGuestInfo *guest_info, 
> > AcpiBuildTables *tables)
> >  build_dmar_q35(tables_blob, tables->linker);
> >  }
> >  
> > +pc_nvdimm_build_nfit_table(table_offsets, tables_blob, tables->linker);
> > +
> >  /* Add tables supplied by user (if any) */
> >  for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
> >  unsigned len = acpi_table_len(u);
> > diff --git a/hw/mem/Makefile.objs b/hw/mem/Makefile.objs
> > index 4df7482..7a6948d 100644
> > --- a/hw/mem/Makefile.objs
> > +++ b/hw/mem/Makefile.objs
> > @@ -1,2 +1,2 @@
> >  common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o
> > -common-obj-$(CONFIG_NVDIMM) += nvdimm/pc-nvdimm.o
> > +common-obj-$(CONFIG_NVDIMM) += nvdimm/pc-nvdimm.o nvdimm/acpi.o
> > diff --git a/hw/mem/nvdimm/acpi.c b/hw/mem/nvdimm/acpi.c
> > new file mode 100644
> > index 000..f28752f
> > --- /dev/null
> > +++ b/hw/mem/nvdimm/acpi.c
> > @@ -0,0 +1,285 @@
> > +/*
> > + * NVDIMM (A Non-Volatile Dual In-line Memory Module) NFIT Implement
> > + *
> > + * Copyright(C) 2015 Intel Corporation.
> > + *
> > + * Author:
> > + *  Xiao Guangrong 
> > + *
> > + * NFIT is defined in ACPI 6.0: 5.2.25 NVDIMM Firmware Interface Table 
> > (NFIT)
> > + * and the DSM specfication can be found at:
> > + *   http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
> > + *
> > + * Currently, it only supports PMEM Virtualization.
> > + *
> > + * 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 "qemu-common.h"
> > +
> > +#include "hw/acpi/aml-build.h"
> > +#include "hw/mem/pc-nvdimm.h"
> > +
> > +#include "internal.h"
> > +
> > +static void nfit_spa_uuid_pm(void *uuid)
> > +{
> > +uuid_le uuid_pm = UUID_LE(0x66f0d379, 0xb4f3, 0x4074, 0xac, 0x43, 0x0d,
> > +  0x33, 0x18, 0xb7, 0x8c, 0xdb);
> > +memcpy(uuid, &uuid_pm, sizeof(uuid_pm));
> > +}
> > +
> > +enum {
> > +NFIT_TABLE_SPA = 0,
> > +NFIT_TABLE_MEM = 1,
> > +NFIT_TABLE_IDT = 2,
> > +NFIT_TABLE_SMBIOS = 3,
> > +NFIT_TABLE_DCR = 4,
> > +NFIT_TABLE_BDW = 5,
> > +NFIT_TABLE_FLUSH = 6,
> > +};
> > +
> > +enum {
> > +EFI_MEMORY_UC = 0x1ULL,
> > +EFI_MEMORY_WC = 0x2ULL,
> > +EFI_MEMORY_WT = 0x4ULL,
> > +EFI_MEMORY_WB = 0x8ULL,
> > +EFI_MEMORY_UCE = 0x10ULL,
> > +EFI_MEMORY_WP = 0x1000ULL,
> > +EFI_MEMORY_RP = 0x2000ULL,
> > +EFI_MEMORY_XP = 0x4000ULL,
> > +EFI_MEMORY_NV = 0x8000ULL,
> > +EFI_MEMORY_MORE_RELIABLE = 0x1ULL,
> > +};
> > +
> > +/*
> > + * struct nfit - Nvdimm Firmware Interface Table
> > + * @signature: "NFIT"
> > + */
> > +struct n

Re: [Qemu-devel] [RFC PATCH] os-android: Add support to android platform, built by ndk-r10

[Houcheng Lin]

Hi Paolo,

(Please ignore the previous mail that did not include "qemu-devel")

Thanks for your review and suggestions. I'll fix this patch
accordingly and please see my replies below.

best regards,
Houcheng Lin

2015-09-15 17:41 GMT+08:00 Paolo Bonzini :

> This is okay and can be done unconditionally (introduce a new
> qemu_getdtablesize function that is defined in util/oslib-posix.c).

Will fix it.
>
>
>> - sigtimewait(): call __rt_sigtimewait() instead.
>> - lockf(): not see this feature in android, directly return -1.
>> - shm_open(): not see this feature in android, directly return -1.
>
> This is not okay.  Please fix your libc instead.

I'll modify the bionic C library to support these functions and feedback
to google's AOSP project. But the android kernel does not support shmem,
so I will prevent compile the ivshmem.c when in android config. The config
for ivshmem in pci.mak will be:

CONFIG_IVSHMEM=$(call land, $(call lnot,$(CONFIG_ANDROID)),$(CONFIG_KVM))

> For sys/io.h, we can just remove the inclusion.  It is not necessary.
>
> scsi/sg.h should be exported by the Linux kernel, so that we can use
> scripts/update-linux-headers.sh to copy it from QEMU.  I've sent a Linux
> kernel patch and CCed you.

It's better to put headers on kernel user headers. Thanks.

>
> You should instead disable the guest agent on your configure command line.
>

Okay.

>
> If you have CONFIG_ANDROID, you do not need -DANDROID.
>

Okay.

>> +  LIBS="-lglib-2.0 -lgthread-2.0 -lz -lpixman-1 -lintl -liconv -lc $LIBS"
>> +  libs_qga="-lglib-2.0 -lgthread-2.0 -lz -lpixman-1 -lintl -liconv -lc"
>
> This should not be necessary, QEMU uses pkg-config.
>
>> +fi
>>  if [ "$bsd" = "yes" ] ; then
>>if [ "$darwin" != "yes" ] ; then
>>  bsd_user="yes"
>> @@ -1736,7 +1749,14 @@ fi
>>  # pkg-config probe
>>
>>  if ! has "$pkg_config_exe"; then
>> -  error_exit "pkg-config binary '$pkg_config_exe' not found"
>> +  case $cross_prefix in
>> +*android*)
>> + pkg_config_exe=scripts/android-pkg-config
>
> Neither should this.  Your cross-compilation environment is not
> correctly set up if you do not have a pkg-config executable.  If you
> want to use a wrapper, you can specify it with the PKG_CONFIG
> environment variable.  But it need not be maintained in the QEMU
> repository, because QEMU assumes a complete cross-compilation environment.

I'll use wrapper in next release and specify with environment variable.
Later, I may generate pkg-config data file while building library and install
it into cross-compilation environment.

>
>> + ;;
>> +*)
>> + error_exit "pkg-config binary '$pkg_config_exe' not found"
>> + ;;
>> +  esac
>>  fi
>>
>>  ##
>> @@ -3764,7 +3784,7 @@ elif compile_prog "" "$pthread_lib -lrt" ; then
>>  fi
>>
>>  if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
>> -"$aix" != "yes" -a "$haiku" != "yes" ; then
>> +"$aix" != "yes" -a "$haiku" != "yes" -a "$android" != "yes" ; then
>>  libs_softmmu="-lutil $libs_softmmu"
>>  fi
>>
>> @@ -4709,6 +4729,10 @@ if test "$linux" = "yes" ; then
>>echo "CONFIG_LINUX=y" >> $config_host_mak
>>  fi
>>
>> +if test "$android" = "yes" ; then
>> +  echo "CONFIG_ANDROID=y" >> $config_host_mak
>> +fi
>> +
>>  if test "$darwin" = "yes" ; then
>>echo "CONFIG_DARWIN=y" >> $config_host_mak
>>  fi
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index ab3c876..1ba22be 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -59,6 +59,10 @@
>>  #define WEXITSTATUS(x) (x)
>>  #endif
>>
>> +#ifdef ANDROID
>> + #include "sysemu/os-android.h"
>> +#endif
>> +
>>  #ifdef _WIN32
>>  #include "sysemu/os-win32.h"
>>  #endif
>> diff --git a/include/sysemu/os-android.h b/include/sysemu/os-android.h
>> new file mode 100644
>> index 000..7f73777
>> --- /dev/null
>> +++ b/include/sysemu/os-android.h
>> @@ -0,0 +1,35 @@
>> +#ifndef QEMU_OS_ANDROID_H
>> +#define QEMU_OS_ANDROID_H
>> +
>> +#include 
>> +
>> +/*
>> + * For include the basename prototyping in android.
>> + */
>> +#include 
>
> These two includes can be added to sysemu/os-posix.h.
>
>> +extern int __rt_sigtimedwait(const sigset_t *uthese, siginfo_t *uinfo, \
>> + const struct timespec *uts, size_t sigsetsize);
>> +
>> +int getdtablesize(void);
>> +int lockf(int fd, int cmd, off_t len);
>> +int sigtimedwait(const sigset_t *set, siginfo_t *info, \
>> + const struct timespec *ts);
>> +int shm_open(const char *name, int oflag, mode_t mode);
>> +char *a_ptsname(int fd);
>> +
>> +#define F_TLOCK 0
>> +#define IOV_MAX 256
>
> libc should really be fixed for all of these (except getdtablesize and
> a_ptsname).  The way you're working around it is introducing subtle
> bugs, for example the pidfile is _not_ locked.

Okay, I will fix it.

>> +#define I_LOOK  (__SID | 4) /* Retrieve the name of the module just 
>> below
>> +   

Re: [Qemu-devel] [PATCH v7 25/42] Postcopy: Maintain sentmap and calculate discard

[Dr. David Alan Gilbert]

* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)"  wrote:
> > From: "Dr. David Alan Gilbert" 
> >
> > Where postcopy is preceeded by a period of precopy, the destination will
> > have received pages that may have been dirtied on the source after the
> > page was sent.  The destination must throw these pages away before
> > starting it's CPUs.
> >
> > Maintain a 'sentmap' of pages that have already been sent.
> > Calculate list of sent & dirty pages
> > Provide helpers on the destination side to discard these.
> >
> > Signed-off-by: Dr. David Alan Gilbert 
> 
> Not a patch without a suggestion O:-)
> 
> > ---
> >  include/migration/migration.h|  12 +++
> >  include/migration/postcopy-ram.h |  35 +++
> >  include/qemu/typedefs.h  |   1 +
> >  migration/migration.c|   1 +
> >  migration/postcopy-ram.c | 108 +
> >  migration/ram.c  | 203 
> > ++-
> >  migration/savevm.c   |   2 -
> >  trace-events |   5 +
> >  8 files changed, 363 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 2a22381..4c6cf95 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -114,6 +114,13 @@ struct MigrationState
> >  
> >  /* Flag set once the migration has been asked to enter postcopy */
> >  bool start_postcopy;
> > +
> > +/* bitmap of pages that have been sent at least once
> > + * only maintained and used in postcopy at the moment
> > + * where it's used to send the dirtymap at the start
> > + * of the postcopy phase
> > + */
> > +unsigned long *sentmap;
> >  };
> 
> We can use this sentmap for zero page optimization.  If page is on
> sentmap, we need to sent a zero page, otherwise, just sent sentmap at
> the end of migration and clean everything not there?

Just as a compact way of sending zero pages? I'm not sure it would help.

> > +/*
> > + * Discard the contents of memory start..end inclusive.
> > + * We can assume that if we've been called postcopy_ram_hosttest returned 
> > true
> > + */
> > +int postcopy_ram_discard_range(MigrationIncomingState *mis, uint8_t *start,
> > +   uint8_t *end)
> > +{
> > +trace_postcopy_ram_discard_range(start, end);
> > +if (madvise(start, (end-start)+1, MADV_DONTNEED)) {
> 
> Can we s/end/lenght/ and adjust everywhere?

Done - partially; everything that works in bytes is now start & length,
everything that works in indexes into the RAM bitmap is still start/end,
since generally that's what they're working with already.

> Not here, but putting a comment explaining where magic 12 cames from on
> definition of constant?

Done.

> I think that the sentbitmap bits could we used without the rest.

Possibly, I can kind of see it's useful - but I'm not convinced what else for
yet.

Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 3/5] acpi: pc: add fw_cfg device node to ssdt

[Eduardo Habkost]

On Mon, Sep 14, 2015 at 04:54:25PM -0400, Gabriel L. Somlo wrote:
> On Mon, Sep 14, 2015 at 04:34:02PM -0400, Gabriel L. Somlo wrote:
> > > > So I'll replace the "if (guest_info->fw_cfg)" check with
> > > > "if machine-type >= (pc-q35-2.5 or pc-i440fx-2.5))", in v3,
> > > > as soon as the patches for the 2.5 machine type make it into
> > > > git master (I remember seeing a reviewed-by fly by for that
> > > > earlier today :)
> > > 
> > > Sounds good, assuming you are going to implement the "machine-type >= 
> > > pc-2.5"
> > > check with something like: 
> > > PC_MACHINE_GET_CLASS(machine)->acpi_no_fw_cfg_node.
> > 
> > Thanks, that gives me something to grep for ;)
> > 
> > I was going to mimic how other acpi related decisions are made on pc
> > (piix or q35), something like below. Might even be the same thing,
> > once I learn about PC_MACHINE_GET_CLASS :)
> 
> OK, so not exactly the same thing. What's the trade-off between
> adding a boolean field to PCMachineClass vs. PcGuestInfo? Either
> would work technically, and PcGuestInfo already has a bunch of
> acpi related booleans. But if the new canonical place for this
> kind of thing is PCMachineClass rather than PcGuestInfo, it's OK
> with me...

PCGuestInfo is initialized only when pc_init1() is called and we are
already initializing a machine. PCMachineClass is initialized when the
corresponding machine_options() is called at class_init time. Keeping
machine-specific compat info at PCMachineClass will allow us to move
that information somewhere else more easily in the future (and allow us
to avoid needing new global variables and new pc_compat_*() functions).

See how PCMachineClass::broken_reserved_end gets used, for reference.

-- 
Eduardo

Re: [Qemu-devel] [Question] QEMU 2.3 Assertion with `existing->mr->subpage || existing->mr == &io_mem_unassigned' failed

[Paolo Bonzini]

On 15/09/2015 11:20, Gonglei wrote:
> On 2015/9/15 14:33, Gonglei wrote:
>> On 2015/9/15 9:16, Gonglei wrote:
>>> On 2015/9/14 17:28, Paolo Bonzini wrote:


 On 14/09/2015 10:01, Gonglei (Arei) wrote:
> [2015-09-11 13:42:44] domain is rebooting 
> qemu-kvm: /home/abuild/rpmbuild/BUILD/qemu-kvm-2.3.0/exec.c:1188: 
> register_subpage: Assertion `existing->mr->subpage || existing->mr == 
> &io_mem_unassigned' failed. 
> [2015-09-11 13:42:58]: shutting down
>
> Or
> qemu-kvm: 
> /home/abuild/rpmbuild/BUILD/qemu-kvm-2.3.0/include/qemu/int128.h:22: 
> int128_get64: Assertion `!a.hi' failed.

 You need to provide a backtrace.

 Paolo

>>> Yup, I noticed that, but when I sent this email yesterday, I didn't get a 
>>> backtrace :(
>>> Fortunately,   I get a backtrace now:
>>>
>>> #0 int128_get64 (a=...) at /home/qemu/include/qemu/int128.h:27
>>> #1 0x7f17ad7a7f1a in register_multipage (d=0x7f179c4f8480, 
>>> section=0x7f17a323c3f0) at /home/qemu/exec.c:1215
>>> #2 0x7f17ad7a8266 in mem_add (listener=0x7f17ae043968 
>>> , section=0x7f17a323c730) at /home/qemu/exec.c:1250
>>> #3 0x7f17ad7f843a in address_space_update_topology_pass 
>>> (as=0x7f17ae043920 , old_view=0x7f179c1f8b50, 
>>> new_view=0x7f179c523620, adding=true)
>>> at /home/qemu/memory.c:739
>>> #4 0x7f17ad7f8520 in address_space_update_topology (as=0x7f17ae043920 
>>> ) at /home/qemu/memory.c:754
>>> #5 0x7f17ad7f8660 in memory_region_transaction_commit () at 
>>> /home/qemu/memory.c:794
>>> #6 0x7f17ad9a690c in cirrus_update_memory_access (s=0x7f17b12873c0) at 
>>> hw/display/cirrus_vga.c:2485
>>> #7 0x7f17ad9a4dac in cirrus_vga_write_gr (s=0x7f17b12873c0, 
>>> reg_index=9, reg_value=163) at hw/display/cirrus_vga.c:1524
>>> #8 0x7f17ad9a6e47 in cirrus_vga_ioport_write (opaque=0x7f17b12873c0, 
>>> addr=975, val=163, size=1) at hw/display/cirrus_vga.c:2672
>>> #9 0x7f17ad7f6882 in memory_region_write_accessor (mr=0x7f17b1297d88, 
>>> addr=31, value=0x7f17a323c968, size=1, shift=8, mask=255) at 
>>> /home/qemu/memory.c:430
>>> #10 0x7f17ad7f698b in access_with_adjusted_size (addr=30, 
>>> value=0x7f17a323c968, size=2, access_size_min=1, access_size_max=1, 
>>> access=0x7f17ad7f67fd , mr=0x7f17b1297d88)
>>> at /home/qemu/memory.c:467
>>> #11 0x7f17ad7f9311 in memory_region_dispatch_write (mr=0x7f17b1297d88, 
>>> addr=30, data=41737, size=2) at /home/qemu/memory.c:1103
>>> #12 0x7f17ad7fc22e in io_mem_write (mr=0x7f17b1297d88, addr=30, 
>>> val=41737, size=2) at /home/qemu/memory.c:2003
>>> #13 0x7f17ad7aafe4 in address_space_rw (as=0x7f17ae043920 
>>> , addr=974, buf=0x7f17ad6f6000 "\t\243\320", len=2, 
>>> is_write=true) at /home/qemu/exec.c:2533
>>> #14 0x7f17ad7f3acf in kvm_handle_io (port=974, data=0x7f17ad6f6000, 
>>> direction=1, size=2, count=1) at /home/qemu/kvm-all.c:1707
>>> #15 0x7f17ad7f3fb5 in kvm_cpu_exec (cpu=0x7f17b05b7a20) at 
>>> /home/qemu/kvm-all.c:1864
>>> #16 0x7f17ad7db416 in qemu_kvm_cpu_thread_fn (arg=0x7f17b05b7a20) at 
>>> /home/qemu/cpus.c:972
>>> #17 0x7f17ac2cbdf5 in start_thread () from /lib64/libpthread.so.0
>>> #18 0x7f17a73e31ad in clone () from /lib64/libc.so.6
>>>
>>> It seems that something wrong happened in vga memory updating.
>>>
>>
>> Another backtrace:
>>
>> (gdb) bt
>> #0 int128_get64 (a=...) at /home/qemu/include/qemu/int128.h:27
>> #1 0x7f4cdefc1f6a in register_multipage (d=0x7f4cd012f1c0, 
>> section=0x7f4cd4a562c0) at /home/qemu/exec.c:1215
>> #2 0x7f4cdefc22b6 in mem_add (listener=0x7f4cdf85d968 
>> , section=0x7f4cd4a56600) at /home/qemu/exec.c:1250
>> #3 0x7f4cdf01248a in address_space_update_topology_pass 
>> (as=0x7f4cdf85d920 , old_view=0x7f4cd0028d40, 
>> new_view=0x7f4cd015f5f0, adding=true)
>> at /home/qemu/memory.c:739
>> #4 0x7f4cdf012570 in address_space_update_topology (as=0x7f4cdf85d920 
>> ) at /home/qemu/memory.c:754
>> #5 0x7f4cdf0126b0 in memory_region_transaction_commit () at 
>> /home/qemu/memory.c:794
>> #6 0x7f4cdf0151f0 in memory_region_del_subregion (mr=0x7f4ce01034e0, 
>> subregion=0x7f4ce13873a0) at /home/qemu/memory.c:1698
>> #7 0x7f4cdf21761d in pci_update_mappings (d=0x7f4ce1386f70) at 
>> hw/pci/pci.c:1120
>> #8 0x7f4cdf2179b0 in pci_default_write_config (d=0x7f4ce1386f70, addr=4, 
>> val_in=256, l=2) at hw/pci/pci.c:1180
>> #9 0x7f4cdf28d2d6 in virtio_write_config (pci_dev=0x7f4ce1386f70, 
>> address=4, val=256, len=2) at hw/virtio/virtio-pci.c:430
>> #10 0x7f4cdf220746 in pci_host_config_write_common 
>> (pci_dev=0x7f4ce1386f70, addr=4, limit=256, val=256, len=2) at 
>> hw/pci/pci_host.c:57
>> #11 0x7f4cdf22084a in pci_data_write (s=0x7f4ce008afc0, addr=2147489796, 
>> val=256, len=2) at hw/pci/pci_host.c:84
>> #12 0x7f4cdf22096c in pci_host_data_write (opaque=0x7f4ce00896b0, 
>> addr=0, val=256, len=2) at hw/pci/pci_host.c:137
>> #13 0x7f4cdf0108d2 in memory_region_write_accessor (mr=

Re: [Qemu-devel] [PATCH v7 04/11] target-mips: improve exception handling

[Leon Alrae]

On 28/08/2015 10:08, Pavel Dovgaluk wrote:
>> From: Aurelien Jarno [mailto:aurel...@aurel32.net]
>> On 2015-08-13 14:12, Leon Alrae wrote:
>>> On 10/07/2015 10:57, Pavel Dovgalyuk wrote:
 @@ -2364,14 +2363,12 @@ static void gen_st_cond (DisasContext *ctx, 
 uint32_t opc, int rt,
  #if defined(TARGET_MIPS64)
  case OPC_SCD:
  case R6_OPC_SCD:
 -save_cpu_state(ctx, 1);
  op_st_scd(t1, t0, rt, ctx);
  opn = "scd";
  break;
  #endif
  case OPC_SC:
  case R6_OPC_SC:
 -save_cpu_state(ctx, 1);
  op_st_sc(t1, t0, rt, ctx);
  opn = "sc";
  break;
>>>
>>> Wouldn't we be better off assuming that conditional stores in linux-user
>>> always take an exception (we generate fake EXCP_SC exception) and avoid
>>> retranslation? After applying these changes I observed significant impact on
>>> performance in linux-user multithreaded apps, for instance c11-atomic-exec
>>> test before the change took just 2 seconds to finish, whereas now more than 
>>> 30...
>>
>> This really show the impact of retranslation and why we should avoid
>> it when not necessary. Coming back to the issue here, the fact that we
>> go through retranslation is actually due to the fact that
>> helper_raise_exception has been changed to go through retranslation.
>>
>> Given the code path between user-mode and softmmu is quite different,
>> we definitely need a different code path wrt exception and retranslation
>> for the two cases. That said if we want deterministic code execution
>> (the original purpose of this patch), I don't see how we can do without
>> forcing retranslation. Pavel, do you have an idea for that?
> 
> There is only one case when we can execute without retranslation -
> when the instruction is the last instruction in translation block.
> Then we can setup PC and flags before this last instruction.
> If the exception happens, we can just break the execution.
> The drawback of this method is breaking translation blocks into
> the smaller parts.

c11-atomic-exec.4 test execution time in linux-user:

* no changes:
real0m3.039s
user0m2.976s
sys 0m1.908s

* tb_lock + patch:
real1m1.167s
user0m57.240s
sys 0m36.678s

* tb_lock + patch + SC-without-retranslation:
real0m3.016s
user0m2.988s
sys 0m1.848s

I had to add tb_lock() to cpu_restore_state() in the first place, otherwise
all of my multithreaded user mode tests crash QEMU with this patch.

SC-without-retranslation (the diff below) seems to improve the situation,
and if I understand correctly we retain deterministic code execution.
Therefore if there are no objections I'll apply this patch + SC correction
to mips-next.

Thanks,
Leon

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 99b99c5..006cb96 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -2060,7 +2060,7 @@ static inline void op_st_##insn(TCGv arg1, TCGv arg2, int 
rt, DisasContext *ctx)
 tcg_gen_movi_tl(t0, rt | ((almask << 3) & 0x20));\
 tcg_gen_st_tl(t0, cpu_env, offsetof(CPUMIPSState, llreg)); 
  \
 tcg_gen_st_tl(arg1, cpu_env, offsetof(CPUMIPSState, llnewval));
  \
-gen_helper_0e0i(raise_exception, EXCP_SC);   \
+generate_exception_end(ctx, EXCP_SC);\
 gen_set_label(l2);   \
 tcg_gen_movi_tl(t0, 0);  \
 gen_store_gpr(t0, rt);   \

Re: [Qemu-devel] [PATCH 06/12] qapi: add dirty-bitmaps migration capability

[Eric Blake]

On 08/07/2015 03:32 AM, Vladimir Sementsov-Ogievskiy wrote:
> From: Vladimir Sementsov-Ogievskiy 
> 
> Reviewed-by: John Snow 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  include/migration/migration.h | 1 +
>  migration/migration.c | 9 +
>  qapi-schema.json  | 4 +++-
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 

> +++ b/qapi-schema.json
> @@ -529,11 +529,13 @@
>  # @auto-converge: If enabled, QEMU will automatically throttle down the guest
>  #  to speed up convergence of RAM migration. (since 1.6)
>  #
> +# @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps. (since 
> 2.4)

2.5 now

With that fixed,
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/2] hw/arm/virt-acpi-build: Add DBG2 table

[Peter Maydell]

On 15 September 2015 at 17:42, Andrew Jones  wrote:
> On Tue, Sep 15, 2015 at 03:44:41PM +0100, Leif Lindholm wrote:
>> Add a DBG2 table, describing the pl011 UART.
>>
>> Signed-off-by: Leif Lindholm 
>> ---
>>  hw/arm/virt-acpi-build.c | 88 
>> +++-
>>  1 file changed, 87 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 9088248..763d531 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -351,6 +351,89 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned 
>> rsdt)
>>  return rsdp_table;
>>  }
>>
>> +static int
>> +dbg2_addresses_size(int num_addr)
>> +{
>> +return num_addr * (sizeof(struct AcpiGenericAddress) + 
>> sizeof(uint32_t));
>> +}
>> +
>> +static int
>> +dbg2_dev_length(int num_addr, const char *namepath, int oemdata_length)
>> +{
>> +int size;
>> +
>> +size = sizeof(AcpiDebugPort2Device);
>> +size += dbg2_addresses_size(num_addr);
>> +size += strlen(namepath) + 1;
>> +size += oemdata_length;
>> +
>> +return size;
>> +}
>
> I think macros should suffice for the above helpers.

...but if you can write them as functions then why not do so?
The compiler's smart enough to inline as appropriate, and it's
not like performance is critical with one-time ACPI table
building anyhow.

Incidentally putting a linebreak before the function name is
not the usual QEMU style for function definitions.

thanks
-- PMM

Re: [Qemu-devel] I want to recover my QEMU password.

[SkyKiDS]

SkyKiDS is mine, and Skykids is maybe mine, too, but could be removed.

Any helps?


2015-09-12 14:09 GMT+09:00 Stefan Weil :

> Am 11.09.2015 um 17:38 schrieb SkyKiDS:
> > Hi, there!
> >
> > Long long time ago, in 2010, I am a member of QEMU wiki, just had an
> > account, and contributed some news or corrected mis-spells.
> > But now, I forgot my password of the site, so hope to recover my
> password.
> > Anyone can help me?
> >
> > My account is SkyKiDS . my registered email maybe nub...@gmail.com or
> this.
> >
> > Any help will be appreciated. Thx. :)
>
> There are two accounts: SkyKIDS and Skykids. Are both yours?
> Skykids has no contributions, so I think that account could be
> removed.
>
> It looks like there is no mailing function enabled for the wiki,
> so I cannot send an e-mail to you using the wiki, nor
> can you request a new password per e-mail.
>
> You'll need help from an administrator...
>
> Stefan
>
>


-- 
하늘을 향해 쏴라, SkyKiDS!

Re: [Qemu-devel] [PATCH] ide: fix ATAPI command permissions

[John Snow]

On 09/15/2015 02:53 AM, Markus Armbruster wrote:
> John Snow  writes:
> 
>> We're a little too lenient with what we'll let an ATAPI drive handle.
>> Clamp down on the IDE command execution table to remove CD_OK permissions
>> from commands that are not and have never been ATAPI commands.
>>
>> For ATAPI command validity, please see:
>> - ATA4 Section 6.5 ("PACKET Command feature set")
>> - ATA8/ACS Section 4.3 ("The PACKET feature set")
>> - ACS3 Section 4.3 ("The PACKET feature set")
>>
>> ACS3 has a historical command validity table in Table B.4
>> ("Historical Command Assignments") that can be referenced to find when
>> a command was introduced, deprecated, obsoleted, etc.
>>
>> The only reference for ATAPI command validity is by checking that
>> version's PACKET feature set section.
>>
>> ATAPI was introduced by T13 into ATA4, all commands retired prior to ATA4
>> therefore are assumed to have never been ATAPI commands.
>>
>> Mandatory commands, as listed in ATA8-ACS3, are:
>>
>> - DEVICE RESET
>> - EXECUTE DEVICE DIAGNOSTIC
>> - IDENTIFY DEVICE
>> - IDENTIFY PACKET DEVICE
>> - NOP
>> - PACKET
>> - READ SECTOR(S)
>> - SET FEATURES
>>
>> Optional commands as listed in ATA8-ACS3, are:
>>
>> - FLUSH CACHE
>> - READ LOG DMA EXT
>> - READ LOG EXT
>> - WRITE LOG DMA EXT
>> - WRITE LOG EXT
>>
>> All other commands are illegal to send to an ATAPI device and should
>> be rejected by the device.
> 
> We could perhaps argue about "should be rejected by the device", but I
> think the weaker "a device is free to reject it" still suffices to
> support your patch.
> 

Sure -- I suppose drives CAN support a superset if they want to. In my
mind, anything above the ATAPI spec should be justified directly with
"Guest X breaks without it."

>> CD_OK removal justifications:
>>
>> 0x06 WIN_DSM  Defined in ACS2. Not valid for ATAPI.
>> 0x21 WIN_READ_ONCERetired in ATA5. Not ATAPI in ATA4.
>> 0x94 WIN_STANDBYNOW2  Retired in ATA4. Did not coexist with ATAPI.
>> 0x95 WIN_IDLEIMMEDIATE2   Retired in ATA4. Did not coexist with ATAPI.
>> 0x96 WIN_STANDBY2 Retired in ATA4. Did not coexist with ATAPI.
>> 0x97 WIN_SETIDLE2 Retired in ATA4. Did not coexist with ATAPI.
>> 0x98 WIN_CHECKPOWERMODE2  Retired in ATA4. Did not coexist with ATAPI.
>> 0x99 WIN_SLEEPNOW2Retired in ATA4. Did not coexist with ATAPI.
>> 0xE0 WIN_STANDBYNOW1  Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE1 WIN_IDLEIMMDIATE Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE2 WIN_STANDBY  Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE3 WIN_SETIDLE1 Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE4 WIN_CHECKPOWERMODE1  Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xE5 WIN_SLEEPNOW1Not part of ATAPI in ATA4, ACS or ACS3.
>> 0xF8 WIN_READ_NATIVE_MAX  Obsoleted in ACS3. Not ATAPI in ATA4 or ACS.
> 
> Actual patch matches this list.
> 
>> This patch fixes a divide by zero fault that can be caused by sending
>> the WIN_READ_NATIVE_MAX command to an empty ATAPI drive, which causes
>> it to attempt to use zeroed CHS values to perform sector arithmetic.
>>
>> Reported-by: Qinghao Tang 
>> Signed-off-by: John Snow 
> 
> I appreciate you going to the root of the problem instead of merely
> fixing the narrow bug.
> 
> Could a similar argument be made for dropping CFA_OK from some commands?
> 

Very likely, but that's another patch. I didn't audit that yet.

> Do we still need this conditional in cmd_read_native_max()?
> 
> /* Refuse if no sectors are addressable (e.g. medium not inserted) */
> if (s->nb_sectors == 0) {
> ide_abort_command(s);
> return true;
> }
> 
> Why does it fail at guarding the CHS use from empty ATAPI drives before
> your patch?
> 

Because I misunderstood the real reason myself, and my POC test was a
little bananas. This works *with* a CDROM inserted, not without.

So s->nb_sectors can be non-zero, and ide_set_sector then triggers e.g.
SIGFPE.

If you'll save me the re-spin, I can fix that part of the commit message
in my staging branch.

--js

[Qemu-devel] [PATCH v3 43/46] ivshmem: add hostmem backend

[marcandre . lureau]

From: Marc-André Lureau 

Instead of handling allocation, teach ivshmem to use a memory backend.
This allows to use hugetlbfs backed memory now.

Signed-off-by: Marc-André Lureau 
---
 hw/misc/ivshmem.c| 84 
 tests/ivshmem-test.c | 12 
 2 files changed, 77 insertions(+), 19 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 5fb2123..ac90f0a 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -26,6 +26,8 @@
 #include "qemu/event_notifier.h"
 #include "qemu/fifo8.h"
 #include "sysemu/char.h"
+#include "sysemu/hostmem.h"
+#include "qapi/visitor.h"
 
 #include "hw/misc/ivshmem.h"
 
@@ -57,6 +59,8 @@
 #define IVSHMEM(obj) \
 OBJECT_CHECK(IVShmemState, (obj), TYPE_IVSHMEM)
 
+#define IVSHMEM_MEMDEV_PROP "memdev"
+
 typedef struct Peer {
 int nb_eventfds;
 EventNotifier *eventfds;
@@ -72,6 +76,7 @@ typedef struct IVShmemState {
 PCIDevice parent_obj;
 /*< public >*/
 
+HostMemoryBackend *hostmem;
 uint32_t intrmask;
 uint32_t intrstatus;
 
@@ -699,9 +704,22 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error 
**errp)
 PCI_BASE_ADDRESS_MEM_PREFETCH;;
 Error *local_err = NULL;
 
-s->shm_fd = -1;
+if (!!s->server_chr + !!s->shmobj + !!s->hostmem != 1) {
+error_setg(errp, "You must specify either a shmobj, a chardev"
+   " or a hostmem");
+return;
+}
+
+if (s->hostmem) {
+MemoryRegion *mr;
 
-if (s->sizearg == NULL) {
+if (s->sizearg) {
+g_warning("size argument ignored with hostmem");
+}
+
+mr = host_memory_backend_get_memory(s->hostmem, errp);
+s->ivshmem_size = memory_region_size(mr);
+} else if (s->sizearg == NULL) {
 s->ivshmem_size = 4 << 20; /* 4 MB default */
 } else {
 s->ivshmem_size = parse_mem_size(s->sizearg, &local_err);
@@ -757,7 +775,16 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error 
**errp)
 attr |= PCI_BASE_ADDRESS_MEM_TYPE_64;
 }
 
-if (s->server_chr != NULL) {
+if (s->hostmem != NULL) {
+MemoryRegion *mr;
+
+IVSHMEM_DPRINTF("using hostmem\n");
+
+mr = host_memory_backend_get_memory(MEMORY_BACKEND(s->hostmem), errp);
+vmstate_register_ram(mr, DEVICE(s));
+memory_region_add_subregion(&s->bar, 0, mr);
+pci_register_bar(PCI_DEVICE(s), 2, attr, &s->bar);
+} else if (s->server_chr != NULL) {
 if (strncmp(s->server_chr->filename, "unix:", 5)) {
 error_setg(errp, "chardev is not a unix client socket");
 return;
@@ -766,12 +793,6 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error 
**errp)
 /* if we get a UNIX socket as the parameter we will talk
  * to the ivshmem server to receive the memory region */
 
-if (s->shmobj != NULL) {
-error_setg(errp, "do not specify both 'chardev' "
-   "and 'shm' with ivshmem");
-return;
-}
-
 IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
 s->server_chr->filename);
 
@@ -795,11 +816,6 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error 
**errp)
 /* just map the file immediately, we're not using a server */
 int fd;
 
-if (s->shmobj == NULL) {
-error_setg(errp, "Must specify 'chardev' or 'shm' to ivshmem");
-return;
-}
-
 IVSHMEM_DPRINTF("using shm_open (shm object = %s)\n", s->shmobj);
 
 /* try opening with O_EXCL and if it succeeds zero the memory
@@ -839,14 +855,17 @@ static void pci_ivshmem_exit(PCIDevice *dev)
 }
 
 if (memory_region_is_mapped(&s->ivshmem)) {
-void *addr = memory_region_get_ram_ptr(&s->ivshmem);
+if (!s->hostmem) {
+void *addr = memory_region_get_ram_ptr(&s->ivshmem);
+
+if (munmap(addr, s->ivshmem_size) == -1) {
+error_report("Failed to munmap shared memory %s",
+ strerror(errno));
+}
+}
 
 vmstate_unregister_ram(&s->ivshmem, DEVICE(dev));
 memory_region_del_subregion(&s->bar, &s->ivshmem);
-
-if (munmap(addr, s->ivshmem_size) == -1) {
-error_report("Failed to munmap shared memory %s", strerror(errno));
-}
 }
 
 if (s->eventfd_chr) {
@@ -988,10 +1007,37 @@ static void ivshmem_class_init(ObjectClass *klass, void 
*data)
 dc->desc = "Inter-VM shared memory";
 }
 
+static void ivshmem_check_memdev_is_busy(Object *obj, const char *name,
+ Object *val, Error **errp)
+{
+MemoryRegion *mr;
+
+mr = host_memory_backend_get_memory(MEMORY_BACKEND(val), errp);
+if (memory_region_is_mapped(mr)) {
+char *path = object_get_canonical_path_component(val);
+error_setg(errp, "can't use already busy memdev: %s", path);
+g_free(path);
+} else {
+   

Re: [Qemu-devel] [PATCH v3 2/2] hw/arm/virt-acpi-build: Add DBG2 table

[Andrew Jones]

On Tue, Sep 15, 2015 at 03:44:41PM +0100, Leif Lindholm wrote:
> Add a DBG2 table, describing the pl011 UART.
> 
> Signed-off-by: Leif Lindholm 
> ---
>  hw/arm/virt-acpi-build.c | 88 
> +++-
>  1 file changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 9088248..763d531 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -351,6 +351,89 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned 
> rsdt)
>  return rsdp_table;
>  }
>  
> +static int
> +dbg2_addresses_size(int num_addr)
> +{
> +return num_addr * (sizeof(struct AcpiGenericAddress) + sizeof(uint32_t));
> +}
> +
> +static int
> +dbg2_dev_length(int num_addr, const char *namepath, int oemdata_length)
> +{
> +int size;
> +
> +size = sizeof(AcpiDebugPort2Device);
> +size += dbg2_addresses_size(num_addr);
> +size += strlen(namepath) + 1;
> +size += oemdata_length;
> +
> +return size;
> +}

I think macros should suffice for the above helpers.

> +
> +static void
> +build_dbg2(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> +{
> +const MemMapEntry *uart_memmap = &guest_info->memmap[VIRT_UART];
> +AcpiDebugPort2Header *dbg2;
> +AcpiDebugPort2Device *dev;
> +struct AcpiGenericAddress *addr;
> +uint32_t *addr_size;
> +char *data;
> +const char namepath[] = ".";
> +int address_count, oem_length, table_revision, table_size;
> +
> +address_count = 1;
> +oem_length = 0;
> +table_revision = 0;
> +table_size = sizeof(*dbg2) + dbg2_dev_length(address_count, namepath,
> + oem_length);
> +
> +dbg2 = acpi_data_push(table_data, sizeof(*dbg2));
> +dbg2->devices_offset = sizeof(*dbg2);
> +dbg2->devices_count = 1;
> +
> +dev = acpi_data_push(table_data, sizeof(*dev));
> +dev->revision = table_revision;

dev->revision and table_revision are presumably independent. I think
they should both get their own explicit '= 0'. Doing so allows us to get
rid of the local variable. I actually find the local variables, which
are constants, a bit crufty, and would prefer to just see the numbers.

> +dev->length = cpu_to_le16(dbg2_dev_length(address_count, namepath,
> +  oem_length));
> +dev->address_count = address_count;
> +dev->namepath_length = cpu_to_le16(strlen(namepath));

what happened to the strlen + 1

> +dev->namepath_offset = cpu_to_le16(sizeof(*dev) +
> +   dbg2_addresses_size(address_count));
> +dev->oem_data_length = cpu_to_le16(oem_length);
> +if (oem_length) {
> +dev->oem_data_offset = cpu_to_le16(dbg2_dev_length(address_count,
> +   namepath, 0));
> +} else {
> +dev->oem_data_offset = 0;
> +}

I wouldn't bother with the special oem_data handling now, since we don't
plan to use it. If somebody extends this function for nonzero oem_length
sometime, then they can deal with it.

> +dev->port_type = cpu_to_le16(0x8000);/* Serial */
> +dev->port_subtype = cpu_to_le16(0x3);/* ARM PL011 UART */
> +dev->base_address_offset = cpu_to_le16(sizeof(*dev));
> +dev->address_size_offset = cpu_to_le16(sizeof(*dev) +
> +   address_count * sizeof(*addr));

Could create another macro for this offset calculation. Actually could add
a macro for each for consistency.

> +
> +addr = acpi_data_push(table_data, sizeof(*addr) * address_count);
> +addr->space_id = AML_SYSTEM_MEMORY;
> +addr->bit_width = 8;
> +addr->bit_offset = 0;
> +addr->access_width = 1;
> +addr->address = cpu_to_le64(uart_memmap->base);
> +
> +addr_size = acpi_data_push(table_data, sizeof(*addr_size) * 
> address_count);
> +*addr_size = cpu_to_le32(sizeof(*addr));
> +
> +data = acpi_data_push(table_data, strlen(namepath) + 1);

After dropping the oem data handling code, then we can use a better name
than 'data' for this.

> +strcpy(data, namepath);
> +
> +if (oem_length) {
> +data = acpi_data_push(table_data, oem_length);
> +}
> +
> +build_header(linker, table_data, (void *)dbg2, "DBG2", table_size,
> + table_revision);
> +}
> +
>  static void
>  build_spcr(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>  {
> @@ -577,7 +660,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  dsdt = tables_blob->len;
>  build_dsdt(tables_blob, tables->linker, guest_info);
>  
> -/* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
> +/* FADT MADT GTDT MCFG DBG2 SPCR pointed to by RSDT */
>  acpi_add_table(table_offsets, tables_blob);
>  build_fadt(tables_blob, tables->linker, dsdt);
>  
> @@ -591,6 +674,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> A

Re: [Qemu-devel] [PATCH PULL v3 00/11] Extract TLS handling code from VNC server

[Peter Maydell]

On 15 September 2015 at 15:36, Daniel P. Berrange  wrote:
> The following changes since commit 007e620a7576e4ce2ea6955541e87d8ae8ed32ae:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
> (2015-09-14 18:51:09 +0100)
>
> are available in the git repository at:
>
>   git://github.com/berrange/qemu.git tags/vnc-crypto-v9-for-upstream
>
> for you to fetch changes up to 3e305e4a4752f70c0b5c3cf5b43ec957881714f7:
>
>   ui: convert VNC server to use QCryptoTLSSession (2015-09-15 15:20:55 +0100)
>
> 
> Merge vnc-crypto-v9
>
> 
> Daniel P. Berrange (11):
>   qapi: allow override of default enum prefix naming
>   tests: remove repetition in unit test object deps
>   crypto: move crypto objects out of libqemuutil.la
>   qom: allow QOM to be linked into tools binaries
>   crypto: introduce new base module for TLS credentials
>   crypto: introduce new module for TLS anonymous credentials
>   crypto: introduce new module for TLS x509 credentials
>   crypto: add sanity checking of TLS x509 credentials
>   crypto: introduce new module for handling TLS sessions
>   ui: fix return type for VNC I/O functions to be ssize_t
>   ui: convert VNC server to use QCryptoTLSSession

Applied, thanks.

-- PMM

[Qemu-devel] [PATCH v3 42/46] ivshmem: make ivshmem_get_size() more generic

[marcandre . lureau]

From: Marc-André Lureau 

Use a more explicit function name parse_mem_size(). I guess such
function could be common (or exists already somewhere).

Signed-off-by: Marc-André Lureau 
---
 hw/misc/ivshmem.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index f9ac955..5fb2123 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -643,12 +643,12 @@ static void ivshmem_reset(DeviceState *d)
 ivshmem_use_msix(s);
 }
 
-static uint64_t ivshmem_get_size(IVShmemState * s, Error **errp) {
-
+static uint64_t parse_mem_size(const char *sizearg, Error **errp)
+{
 uint64_t value;
 char *ptr;
 
-value = strtoull(s->sizearg, &ptr, 10);
+value = strtoull(sizearg, &ptr, 10);
 switch (*ptr) {
 case 0: case 'M': case 'm':
 value <<= 20;
@@ -657,7 +657,7 @@ static uint64_t ivshmem_get_size(IVShmemState * s, Error 
**errp) {
 value <<= 30;
 break;
 default:
-error_setg(errp, "invalid ram size: %s", s->sizearg);
+error_setg(errp, "invalid ram size: %s", sizearg);
 return 0;
 }
 
@@ -704,7 +704,7 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error 
**errp)
 if (s->sizearg == NULL) {
 s->ivshmem_size = 4 << 20; /* 4 MB default */
 } else {
-s->ivshmem_size = ivshmem_get_size(s, &local_err);
+s->ivshmem_size = parse_mem_size(s->sizearg, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2] Add .dir-locals.el file to configure emacs coding style

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Tue, Sep 15, 2015 at 09:31:08AM -0600, Eric Blake wrote:
>> ping
>> 
>> On 08/24/2015 06:15 PM, John Snow wrote:
>> > 
>> > 
>> > On 06/18/2015 10:05 AM, Markus Armbruster wrote:
>> >> Peter Maydell  writes:
>> >>
>> >>> On 18 June 2015 at 10:28, Markus Armbruster  wrote:
>>  However, I can't see how I could define a new C style there without
>>  pushing the "local variables" feature well beyond its intended use, and
>>  triggering the confirmation prompts.
>> >>>
>> >>> We wouldn't want to define a new C style, but in general the items
>> >>> I have in my config over the Stroustrup defaults are going to be there
>> >>> because I've noticed something where Stroustrup doesn't indent right...
>> >>
>> >> As far as I can see, the difference bwteen stroustrup style and yours is
>> >> a few tweaks to c-offsets-alist and c-hanging-braces-alist.  I'm not
>> >> sure how to do that from .dir-locals.el.
>> >>
>>  If we take Dan's patch, every Emacs user who hasn't already configured a
>>  suitable style profits.  Users who have may have to adjust their
>>  configuration to work with or around Dan's patch.
>> >>>
>> >>> Is there some way to tell whether your emacs has picked up the local
>> >>> style info rather than the one you have in your .emacs ?
>> >>
>> >> The obvious way is to check the buffer-local variables.  Buffer-local
>> >> variable c-indentation-style is the name of the style in use.  Without
>> >> anything in .emacs or local variables, it's "gnu".  With your
>> >> (c-add-style "qemu" qemu-c-style) in .emacs, it should be "qemu".  With
>> >> Dan's .dir-locals.el, it should be "stroustrup".
>> >>
>> >> When you can't or don't want to create or modify a .dir-locals.el, you
>> >> can do something like this instead:
>> >>
>> >> (dir-locals-set-class-variables 'qemu '((c-mode
>> >> . ((c-file-style . "qemu")
>> >> (dir-locals-set-directory-class "~/work/qemu" 'qemu)
>> >>
>> >> This *overrides* .dir-locals.el in my testing.
>> >>
>> > 
>> > ping -- any love for this for 2.5?
>
> So just to re-cap.
>
> My original proposed .dir-locals.el was pretty short:
>
>   ((c-mode . ((c-file-style . "stroustrup")
>  (indent-tabs-mode . nil
>
> IIUC, the quote above says it is possible to override this with more
> developer custom styles if desired, so it shouldn't cause any obvious
> disadvantage / problem to contributors to have this set by default.
>
> I have tried an alternative .dir-locals.el that contains all of
> Peter's rules (from http://wiki.linaro.org/PeterMaydell/QemuEmacsStyle)
> which looks like this:
>
>
>   ;; Based on https://wiki.linaro.org/PeterMaydell/QemuEmacsStyle
>   ;; which aims to apply CODING_STYLE guidelines
>   ((c-mode . (
>   (c-file-style . "stroustrup")
>   (indent-tabs-mode . nil)
>   (tab-width . 8)
>   (c-comment-only-line-offset . 1)
>   (c-hanging-braces-alist . ((substatement-open before after)))
>   (c-offsets-alist . (
>   (statement-block-intro . +)
>   (substatement-open . 0)
>   (label . 0)
>   (statement-cont . +)
>   (innamespace . 0)
>   (inline-open . 0)
>   ))
>   (c-hanging-braces-alist . (
>   (brace-list-open)
>   (brace-list-intro)
>   (brace-list-entry)
>   (brace-list-close)
>   (brace-entry-open)
>   (block-close . c-snug-do-while)
>   ;; structs have hanging braces on open
>   (class-open . (after))
>   ;; ditto if statements
>   (substatement-open . (after))
>   ;; and no auto newline at the end
>   (class-close)
>   ))
>   )))
>
> The main downside/problem I found with having this more comprehensive
> .dir-locals.el file, is that emacs will raise a warning at launch time:
>
> "The local variables list in /home/berrange/src/virt/qemu
>  contains vlaues that may not be safe (*).
>
>  Do you want to apply it? You can type
>   y -- to apply the local variables list.
>   n -- to ignore the local variables list.
>   ! -- to apply the local variables list and permanently
>mark these values (*) as safe (in the future, they
>will be set automatically)"
>
> It will ask this every time it launches unless you say "!"
> whereupon it modifies your $HOME/.emacs to disable the warning
>
> Personally I be fine with both the minimal approach or the more
> comprehensive approach of Peter's, but I'd probably tend towards
> the minimal approach to avoid the warnings problem.

Seconded.

The "minimal approach" has been on the list since June, and it has my
R-by.  It's clearly better than nothing.  Let's commit it now.  We can
always improve on it later.

[Qemu-devel] [PATCH v3 40/46] tests: add ivshmem qtest

[marcandre . lureau]

From: Marc-André Lureau 

Adds 4 ivshmemtests:
- single qemu instance and basic IO
- pair of instances, check memory sharing
- pair of instances with server, and MSIX
- hot plug/unplug

A temporary shm is created as well as a directory to place server
socket, both should be clear on exit and abort.

Cc: Cam Macdonell 
CC: Andreas Färber 
Signed-off-by: Marc-André Lureau 
---
 tests/Makefile   |   3 +
 tests/ivshmem-test.c | 474 +++
 2 files changed, 477 insertions(+)
 create mode 100644 tests/ivshmem-test.c

diff --git a/tests/Makefile b/tests/Makefile
index 34c6136..e7a50e7 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -142,6 +142,8 @@ gcov-files-pci-y += hw/display/virtio-gpu-pci.c
 gcov-files-pci-$(CONFIG_VIRTIO_VGA) += hw/display/virtio-vga.c
 check-qtest-pci-y += tests/intel-hda-test$(EXESUF)
 gcov-files-pci-y += hw/audio/intel-hda.c hw/audio/hda-codec.c
+check-qtest-pci-$(CONFIG_LINUX) += tests/ivshmem-test$(EXESUF)
+gcov-files-pci-y += hw/misc/ivshmem.c
 
 check-qtest-i386-y = tests/endianness-test$(EXESUF)
 check-qtest-i386-y += tests/fdc-test$(EXESUF)
@@ -416,6 +418,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o 
qemu-char.o qemu-timer.o
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a 
libqemustub.a
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o 
$(block-obj-y) libqemuutil.a libqemustub.a
+tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o 
contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
 
 ifeq ($(CONFIG_POSIX),y)
 LIBS += -lutil
diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
new file mode 100644
index 000..03f44c1
--- /dev/null
+++ b/tests/ivshmem-test.c
@@ -0,0 +1,474 @@
+/*
+ * QTest testcase for ivshmem
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "contrib/ivshmem-server/ivshmem-server.h"
+#include "libqos/pci-pc.h"
+#include "libqtest.h"
+#include "qemu/osdep.h"
+#include 
+
+#if GLIB_CHECK_VERSION(2, 32, 0)
+#define HAVE_THREAD_NEW
+#endif
+
+#define TMPSHMSIZE (1 << 20)
+static char *tmpshm;
+static void *tmpshmem;
+static char *tmpdir;
+static char *tmpserver;
+
+static void save_fn(QPCIDevice *dev, int devfn, void *data)
+{
+QPCIDevice **pdev = (QPCIDevice **) data;
+
+*pdev = dev;
+}
+
+static QPCIDevice *get_device(void)
+{
+QPCIDevice *dev;
+QPCIBus *pcibus;
+
+pcibus = qpci_init_pc();
+qpci_device_foreach(pcibus, 0x1af4, 0x1110, save_fn, &dev);
+g_assert(dev != NULL);
+
+return dev;
+}
+
+typedef struct _IVState {
+QTestState *qtest;
+void *reg_base, *mem_base;
+QPCIDevice *dev;
+} IVState;
+
+#define REG(name, len, val) \
+static inline unsigned in_##name(IVState *s)\
+{   \
+QTestState *qtest = global_qtest;   \
+unsigned res;   \
+global_qtest = s->qtest;\
+res = qpci_io_read##len(s->dev, s->reg_base+(val)); \
+g_test_message("*%s -> %x\n", #name, res);  \
+global_qtest = qtest;   \
+return res; \
+}   \
+static inline void out_##name(IVState *s, unsigned v)   \
+{   \
+QTestState *qtest = global_qtest;   \
+global_qtest = s->qtest;\
+g_test_message("%x -> *%s\n", v, #name);\
+qpci_io_write##len(s->dev, s->reg_base+(val), v);   \
+global_qtest = qtest;   \
+}
+
+REG(IntrMask, l, 0)
+REG(IntrStatus, l, 4)
+REG(IVPosition, l, 8)
+REG(DoorBell, l, 12)
+
+#if 0
+static void info_qtree(void)
+{
+QDict *response;
+
+response = qmp("{'execute': 'human-monitor-command',"
+   " 'arguments': {"
+   "   'command-line': 'info qtree'"
+   "}}");
+g_assert(response);
+g_debug(qdict_get_try_str(response, "return"));
+QDECREF(response);
+}
+#endif
+
+static void setup_vm_cmd(IVState *s, const char *cmd, bool msix)
+{
+uint64_t barsize;
+
+s->qtest = qtest_start(cmd);
+
+s->dev = get_device();
+
+/* FIXME: other bar order fails, mappings changes */
+s->mem_base = qpci_iomap(s->dev, 2, &barsize);
+g_assert_nonnull(s->mem_base);
+g_assert_cmpuint(barsize, ==, TMPSHMSIZE);
+
+if (msix) {
+q

[Qemu-devel] [PATCH v3 38/46] msix: implement pba write (but read-only)

[marcandre . lureau]

From: Marc-André Lureau 

qpci_msix_pending() writes on pba region, causing qemu to SEGV:

  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 0x77fba8c0 (LWP 25882)]
  0x in ?? ()
  (gdb) bt
  #0  0x in  ()
  #1  0x556556c5 in memory_region_oldmmio_write_accessor 
(mr=0x579f3f80, addr=0, value=0x7fffbf68, size=4, shift=0, 
mask=4294967295, attrs=...) at /home/elmarco/src/qemu/memory.c:434
  #2  0x556558e1 in access_with_adjusted_size (addr=0, 
value=0x7fffbf68, size=4, access_size_min=1, access_size_max=4, 
access=0x5565563e , 
mr=0x579f3f80, attrs=...) at /home/elmarco/src/qemu/memory.c:506
  #3  0x556581eb in memory_region_dispatch_write (mr=0x579f3f80, 
addr=0, data=0, size=4, attrs=...) at /home/elmarco/src/qemu/memory.c:1176
  #4  0x5560b6f9 in address_space_rw (as=0x55eff4e0 
, addr=3759147008, attrs=..., buf=0x7fffc1b0 "", 
len=4, is_write=true) at /home/elmarco/src/qemu/exec.c:2439
  #5  0x5560baa2 in cpu_physical_memory_rw (addr=3759147008, 
buf=0x7fffc1b0 "", len=4, is_write=1) at /home/elmarco/src/qemu/exec.c:2534
  #6  0x5564c005 in cpu_physical_memory_write (addr=3759147008, 
buf=0x7fffc1b0, len=4) at 
/home/elmarco/src/qemu/include/exec/cpu-common.h:80
  #7  0x5564cd9c in qtest_process_command (chr=0x5642b890, 
words=0x578de4b0) at /home/elmarco/src/qemu/qtest.c:378
  #8  0x5564db77 in qtest_process_inbuf (chr=0x5642b890, 
inbuf=0x5641b340) at /home/elmarco/src/qemu/qtest.c:569
  #9  0x5564dc07 in qtest_read (opaque=0x5642b890, 
buf=0x7fffc2e0 "writel 0xe0100800 0x0\n", size=22) at 
/home/elmarco/src/qemu/qtest.c:581
  #10 0x5574ce3e in qemu_chr_be_write (s=0x5642b890, 
buf=0x7fffc2e0 "writel 0xe0100800 0x0\n", len=22) at qemu-char.c:306
  #11 0x55751263 in tcp_chr_read (chan=0x5642bcf0, cond=G_IO_IN, 
opaque=0x5642b890) at qemu-char.c:2876
  #12 0x764c9a8a in g_main_context_dispatch (context=0x5641c400) at 
gmain.c:3122

(without this patch, this can be reproduced with the ivshmem qtest)

Implement an empty mmio write to avoid the crash.

Signed-off-by: Marc-André Lureau 
---
 hw/pci/msix.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 2fdada4..64c93d8 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -200,8 +200,14 @@ static uint64_t msix_pba_mmio_read(void *opaque, hwaddr 
addr,
 return pci_get_long(dev->msix_pba + addr);
 }
 
+static void msix_pba_mmio_write(void *opaque, hwaddr addr,
+uint64_t val, unsigned size)
+{
+}
+
 static const MemoryRegionOps msix_pba_mmio_ops = {
 .read = msix_pba_mmio_read,
+.write = msix_pba_mmio_write,
 .endianness = DEVICE_LITTLE_ENDIAN,
 .valid = {
 .min_access_size = 4,
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 08/18] nvdimm: init backend memory mapping and config data area

[Paolo Bonzini]

On 07/09/2015 16:11, Igor Mammedov wrote:
> 
> here is common concepts that could be reused.
>   - on physical system both DIMM and NVDIMM devices use
> the same slots. We could share QEMU's '-m slots' option between
> both devices. An alternative to not sharing would be to introduce
> '-machine nvdimm_slots' option.
> And yes, we need to know number of NVDIMMs to describe
> them all in ACPI table (taking in amount future hotplug
> include in this possible NVDIMM devices)
> I'd go the same way as on real hardware on make them share the same slots.
>   - they share the same physical address space and limits
> on how much memory system can handle. So I'd suggest sharing existing
> '-m maxmem' option and reuse hotplug_memory address space.

I agree, and the slot number also provide a nice way to build a
consistent memory region name across multiple systems.

Paolo

[Qemu-devel] [PATCH v3 45/46] ivshmem: rename MSI eventfd_table

[marcandre . lureau]

From: Marc-André Lureau 

The array is used to have vector specific data, so use a more
descriptive name.

Signed-off-by: Marc-André Lureau 
---
 hw/misc/ivshmem.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 976fbea..f4a2ea2 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -66,9 +66,9 @@ typedef struct Peer {
 EventNotifier *eventfds;
 } Peer;
 
-typedef struct EventfdEntry {
+typedef struct MSIVector {
 PCIDevice *pdev;
-} EventfdEntry;
+} MSIVector;
 
 typedef struct IVShmemState {
 /*< private >*/
@@ -99,7 +99,7 @@ typedef struct IVShmemState {
 int vm_id;
 uint32_t vectors;
 uint32_t features;
-EventfdEntry *eventfd_table;
+MSIVector *msi_vectors;
 
 Error *migration_blocker;
 
@@ -284,10 +284,10 @@ static void ivshmem_event(void *opaque, int event)
 
 static void fake_irqfd(void *opaque, const uint8_t *buf, int size) {
 
-EventfdEntry *entry = opaque;
+MSIVector *entry = opaque;
 PCIDevice *pdev = entry->pdev;
 IVShmemState *s = IVSHMEM(pdev);
-int vector = entry - s->eventfd_table;
+int vector = entry - s->msi_vectors;
 
 IVSHMEM_DPRINTF("interrupt on vector %p %d\n", pdev, vector);
 msix_notify(pdev, vector);
@@ -311,10 +311,10 @@ static CharDriverState* create_eventfd_chr_device(void * 
opaque, EventNotifier *
 
 /* if MSI is supported we need multiple interrupts */
 if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
-s->eventfd_table[vector].pdev = PCI_DEVICE(s);
+s->msi_vectors[vector].pdev = PCI_DEVICE(s);
 
 qemu_chr_add_handlers(chr, ivshmem_can_receive, fake_irqfd,
-  ivshmem_event, &s->eventfd_table[vector]);
+  ivshmem_event, &s->msi_vectors[vector]);
 } else {
 qemu_chr_add_handlers(chr, ivshmem_can_receive, ivshmem_receive,
   ivshmem_event, s);
@@ -684,7 +684,7 @@ static int ivshmem_setup_msi(IVShmemState * s)
 IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors);
 
 /* allocate QEMU char devices for receiving interrupts */
-s->eventfd_table = g_malloc0(s->vectors * sizeof(EventfdEntry));
+s->msi_vectors = g_malloc0(s->vectors * sizeof(MSIVector));
 
 ivshmem_use_msix(s);
 return 0;
@@ -888,7 +888,7 @@ static void pci_ivshmem_exit(PCIDevice *dev)
 msix_uninit_exclusive_bar(dev);
 }
 
-g_free(s->eventfd_table);
+g_free(s->msi_vectors);
 }
 
 static bool test_msix(void *opaque, int version_id)
-- 
2.4.3

[Qemu-devel] [PATCH v3 36/46] ivshmem: add check on protocol version in QEMU

[marcandre . lureau]

From: David Marchand 

Send a protocol version as the first message from server, clients must
close communication if they don't support this protocol version.  Older
QEMUs should be fine with this change in the protocol since they
overrides their own vm_id on reception of an id associated to no
eventfd.

Signed-off-by: David Marchand 
Signed-off-by: Marc-André Lureau 
[use fifo_update_and_get()]
---
 contrib/ivshmem-client/ivshmem-client.c | 13 ++---
 contrib/ivshmem-client/ivshmem-client.h |  1 +
 contrib/ivshmem-server/ivshmem-server.c |  9 +
 contrib/ivshmem-server/ivshmem-server.h |  1 +
 docs/specs/ivshmem_device_spec.txt  |  9 ++---
 hw/misc/ivshmem.c   | 31 +--
 include/hw/misc/ivshmem.h   | 25 +
 7 files changed, 81 insertions(+), 8 deletions(-)
 create mode 100644 include/hw/misc/ivshmem.h

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index 01e24a7..a8477d8 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -205,10 +205,17 @@ ivshmem_client_connect(IvshmemClient *client)
 goto err_close;
 }
 
-/* first, we expect our index + a fd == -1 */
+/* first, we expect a protocol version */
+if (ivshmem_client_read_one_msg(client, &tmp, &fd) < 0 ||
+(tmp != IVSHMEM_PROTOCOL_VERSION) || fd != -1) {
+IVSHMEM_CLIENT_DEBUG(client, "cannot read from server\n");
+goto err_close;
+}
+
+/* then, we expect our index + a fd == -1 */
 if (ivshmem_client_read_one_msg(client, &client->local.id, &fd) < 0 ||
 client->local.id < 0 || fd != -1) {
-IVSHMEM_CLIENT_DEBUG(client, "cannot read from server\n");
+IVSHMEM_CLIENT_DEBUG(client, "cannot read from server (2)\n");
 goto err_close;
 }
 IVSHMEM_CLIENT_DEBUG(client, "our_id=%ld\n", client->local.id);
@@ -220,7 +227,7 @@ ivshmem_client_connect(IvshmemClient *client)
 if (fd >= 0) {
 close(fd);
 }
-IVSHMEM_CLIENT_DEBUG(client, "cannot read from server (2)\n");
+IVSHMEM_CLIENT_DEBUG(client, "cannot read from server (3)\n");
 goto err_close;
 }
 client->shm_fd = fd;
diff --git a/contrib/ivshmem-client/ivshmem-client.h 
b/contrib/ivshmem-client/ivshmem-client.h
index 284c4a3..9215f34 100644
--- a/contrib/ivshmem-client/ivshmem-client.h
+++ b/contrib/ivshmem-client/ivshmem-client.h
@@ -23,6 +23,7 @@
 #include 
 
 #include "qemu/queue.h"
+#include "hw/misc/ivshmem.h"
 
 /**
  * Maximum number of notification vectors supported by the client
diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index 51264b4..d917161 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -101,6 +101,15 @@ ivshmem_server_send_initial_info(IvshmemServer *server, 
IvshmemServerPeer *peer)
 {
 int ret;
 
+/* send our protocol version first */
+ret = ivshmem_server_send_one_msg(peer->sock_fd, IVSHMEM_PROTOCOL_VERSION,
+  -1);
+if (ret < 0) {
+IVSHMEM_SERVER_DEBUG(server, "cannot send version: %s\n",
+ strerror(errno));
+return -1;
+}
+
 /* send the peer id to the client */
 ret = ivshmem_server_send_one_msg(peer->sock_fd, peer->id, -1);
 if (ret < 0) {
diff --git a/contrib/ivshmem-server/ivshmem-server.h 
b/contrib/ivshmem-server/ivshmem-server.h
index e9b0e7a..65b3c2d 100644
--- a/contrib/ivshmem-server/ivshmem-server.h
+++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -32,6 +32,7 @@
 #include 
 
 #include "qemu/queue.h"
+#include "hw/misc/ivshmem.h"
 
 /**
  * Maximum number of notification vectors supported by the server
diff --git a/docs/specs/ivshmem_device_spec.txt 
b/docs/specs/ivshmem_device_spec.txt
index 12f338e..3435116 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -64,6 +64,8 @@ It creates a shared memory object then waits for clients to 
connect on a unix
 socket.
 
 For each client (QEMU process) that connects to the server:
+- the server sends a protocol version, if client does not support it, the 
client
+  closes the communication,
 - the server assigns an ID for this client and sends this ID to him as the 
first
   message,
 - the server sends a fd to the shared memory object to this client,
@@ -86,9 +88,10 @@ been provided in qemu.git/contrib/ivshmem-client for debug.
 
 *QEMU as an ivshmem client*
 
-At initialisation, when creating the ivshmem device, QEMU gets its ID from the
-server then makes it available through BAR0 IVPosition register for the VM to
-use (see 'PCI device registers' subsection).
+At initialisation, when creating the ivshmem device, QEMU first receives a
+protocol version and closes communication with server if it does not match.
+Then, QEMU gets its ID from 

Re: [Qemu-devel] [PATCH 11/12] qapi: add md5 checksum of last dirty bitmap level to query-block

[Eric Blake]

On 08/07/2015 03:32 AM, Vladimir Sementsov-Ogievskiy wrote:
> From: Vladimir Sementsov-Ogievskiy 
> 
> Reviewed-by: John Snow 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block.c| 1 +
>  include/qemu/hbitmap.h | 8 
>  qapi/block-core.json   | 4 +++-
>  util/hbitmap.c | 8 
>  4 files changed, 20 insertions(+), 1 deletion(-)

> +++ b/qapi/block-core.json
> @@ -359,11 +359,13 @@
>  #
>  # @status: current status of the dirty bitmap (since 2.4)
>  #
> +# @md5: md5 checksum of the last bitmap level (since 2.4)

since 2.5, now. Would it help to be explicit that this is a hexadecimal
encoding of the checksum (and not the actual binary value)?

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 44/46] ivshmem: remove EventfdEntry.vector

[marcandre . lureau]

From: Marc-André Lureau 

No need to store an extra int for the vector number when it can be
computed easily by looking at the position in the array.

Signed-off-by: Marc-André Lureau 
---
 hw/misc/ivshmem.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index ac90f0a..976fbea 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -68,7 +68,6 @@ typedef struct Peer {
 
 typedef struct EventfdEntry {
 PCIDevice *pdev;
-int vector;
 } EventfdEntry;
 
 typedef struct IVShmemState {
@@ -287,9 +286,11 @@ static void fake_irqfd(void *opaque, const uint8_t *buf, 
int size) {
 
 EventfdEntry *entry = opaque;
 PCIDevice *pdev = entry->pdev;
+IVShmemState *s = IVSHMEM(pdev);
+int vector = entry - s->eventfd_table;
 
-IVSHMEM_DPRINTF("interrupt on vector %p %d\n", pdev, entry->vector);
-msix_notify(pdev, entry->vector);
+IVSHMEM_DPRINTF("interrupt on vector %p %d\n", pdev, vector);
+msix_notify(pdev, vector);
 }
 
 static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier 
*n,
@@ -311,7 +312,6 @@ static CharDriverState* create_eventfd_chr_device(void * 
opaque, EventNotifier *
 /* if MSI is supported we need multiple interrupts */
 if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
 s->eventfd_table[vector].pdev = PCI_DEVICE(s);
-s->eventfd_table[vector].vector = vector;
 
 qemu_chr_add_handlers(chr, ivshmem_can_receive, fake_irqfd,
   ivshmem_event, &s->eventfd_table[vector]);
-- 
2.4.3

[Qemu-devel] [PATCH v3 06/46] ivshmem: remove unnecessary dup()

[marcandre . lureau]

From: Marc-André Lureau 

qemu_chr_fe_get_msgfd() transfers ownership, there is no need to dup the
fd.

Signed-off-by: Marc-André Lureau 
---
 hw/misc/ivshmem.c | 21 ++---
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index dd15f0e..fbeb731 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -480,7 +480,7 @@ static bool fifo_update_and_get(IVShmemState *s, const 
uint8_t *buf, int size,
 static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
 {
 IVShmemState *s = opaque;
-int incoming_fd, tmp_fd;
+int incoming_fd;
 int guest_max_eventfd;
 long incoming_posn;
 
@@ -495,21 +495,21 @@ static void ivshmem_read(void *opaque, const uint8_t 
*buf, int size)
 }
 
 /* pick off s->server_chr->msgfd and store it, posn should accompany msg */
-tmp_fd = qemu_chr_fe_get_msgfd(s->server_chr);
-IVSHMEM_DPRINTF("posn is %ld, fd is %d\n", incoming_posn, tmp_fd);
+incoming_fd = qemu_chr_fe_get_msgfd(s->server_chr);
+IVSHMEM_DPRINTF("posn is %ld, fd is %d\n", incoming_posn, incoming_fd);
 
 /* make sure we have enough space for this guest */
 if (incoming_posn >= s->nb_peers) {
 if (increase_dynamic_storage(s, incoming_posn) < 0) {
 error_report("increase_dynamic_storage() failed");
-if (tmp_fd != -1) {
-close(tmp_fd);
+if (incoming_fd != -1) {
+close(incoming_fd);
 }
 return;
 }
 }
 
-if (tmp_fd == -1) {
+if (incoming_fd == -1) {
 /* if posn is positive and unseen before then this is our posn*/
 if ((incoming_posn >= 0) &&
 (s->peers[incoming_posn].eventfds == NULL)) {
@@ -524,15 +524,6 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, 
int size)
 }
 }
 
-/* because of the implementation of get_msgfd, we need a dup */
-incoming_fd = dup(tmp_fd);
-
-if (incoming_fd == -1) {
-error_report("could not allocate file descriptor %s", strerror(errno));
-close(tmp_fd);
-return;
-}
-
 /* if the position is -1, then it's shared memory region fd */
 if (incoming_posn == -1) {
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 33/46] ivshmem-server: use a uint16 for client ID

[marcandre . lureau]

From: Marc-André Lureau 

In practice, the number of VM is limited to MAXUINT16 in ivshmem, so use
the same limit on the server (removes a theorical infinite loop)

Signed-off-by: Marc-André Lureau 
---
 contrib/ivshmem-server/ivshmem-server.c | 11 ++-
 contrib/ivshmem-server/ivshmem-server.h |  2 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/contrib/ivshmem-server/ivshmem-server.c 
b/contrib/ivshmem-server/ivshmem-server.c
index 16ee583..972fda2 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -145,9 +145,18 @@ ivshmem_server_handle_new_conn(IvshmemServer *server)
 peer->sock_fd = newfd;
 
 /* get an unused peer id */
-while (ivshmem_server_search_peer(server, server->cur_id) != NULL) {
+/* XXX: this could use id allocation such as Linux IDA, or simply
+ * a free-list */
+for (i = 0; i < G_MAXUINT16; i++) {
+if (ivshmem_server_search_peer(server, server->cur_id) == NULL) {
+break;
+}
 server->cur_id++;
 }
+if (i == G_MAXUINT16) {
+IVSHMEM_SERVER_DEBUG(server, "cannot allocate new client id\n");
+goto fail;
+}
 peer->id = server->cur_id++;
 
 /* create eventfd, one per vector */
diff --git a/contrib/ivshmem-server/ivshmem-server.h 
b/contrib/ivshmem-server/ivshmem-server.h
index cd584fc..2176d5e 100644
--- a/contrib/ivshmem-server/ivshmem-server.h
+++ b/contrib/ivshmem-server/ivshmem-server.h
@@ -70,7 +70,7 @@ typedef struct IvshmemServer {
 size_t shm_size; /**< size of shm */
 int shm_fd;  /**< shm file descriptor */
 unsigned n_vectors;  /**< number of vectors */
-long cur_id; /**< id to be given to next client */
+uint16_t cur_id; /**< id to be given to next client */
 bool verbose;/**< true in verbose mode */
 IvshmemServerPeerList peer_list; /**< list of peers */
 } IvshmemServer;
-- 
2.4.3

Re: [Qemu-devel] [PATCH 03/17] spec: add qcow2-dirty-bitmaps specification

[Eric Blake]

On 09/05/2015 10:43 AM, Vladimir Sementsov-Ogievskiy wrote:
> Persistent dirty bitmaps will be saved into qcow2 files. It may be used
> as 'internal' bitmaps (for qcow2 drives) or as 'external' bitmaps for
> other drives (there may be qcow2 file with zero disk size but with
> several dirty bitmaps for other drives).
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  docs/specs/qcow2.txt | 127 
> ++-
>  1 file changed, 126 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 121dfc8..5fc0365 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -103,7 +103,13 @@ in the description of a field.
>  write to an image with unknown auto-clear features if it
>  clears the respective bits from this field first.
>  
> -Bits 0-63:  Reserved (set to 0)
> +Bit 0:  Dirty bitmaps bit. If this bit is set then
> +there is a _consistent_ Dirty bitmaps 
> extension
> +in the image. If it is not set, but there is 
> a
> +Dirty bitmaps extension, its data should be
> +considered as inconsistent.

Thanks for documenting this. I don't know that we use underscore for
_emphasis_ anywhere else in the file, but I don't have any better
suggestions.  Should you also require that it is an error if this bit is
set but no Dirty bitmap extension header is present?

> +
> +Bits 1-63:  Reserved (set to 0)
>  
>   96 -  99:  refcount_order
>  Describes the width of a reference count block entry 
> (width
> @@ -123,6 +129,7 @@ be stored. Each extension has a structure like the 
> following:
>  0x - End of the header extension area
>  0xE2792ACA - Backing file format name
>  0x6803f857 - Feature name table
> +0x23852875 - Dirty bitmaps
>  other  - Unknown header extension, can be safely
>   ignored
>  
> @@ -166,6 +173,24 @@ the header extension data. Each entry look like this:
>  terminated if it has full length)
>  
>  
> +== Dirty bitmaps ==
> +
> +Dirty bitmaps is an optional header extension. It provides an ability to 
> store
> +dirty bitmaps in a qcow2 image. The fields are:

Might not hurt to remind the reader about the auto-clear feature bit
mentioned earlier controlling whether this extension can be trusted as
consistent.

> +
> +  0 -  3:  nb_dirty_bitmaps
> +   The number of dirty bitmaps contained in the image. Valid
> +   values: 0 - 65535.
> +
> +  4 -  7:  dirty_bitmap_directory_size
> +   Size of the Dirty Bitmap Directory in bytes. Valid values:
> +   0 - 67108864 (= 1024 * nb_dirty_bitmaps).

Is it always going to be 1024 * nb_dirty_bitmaps? If so, why do we need
a redundant field?  If not, then this wording needs help; from the rest
of this text, it looks like you want "at most 1024 * nb_dirty_bitmaps".
 Also, while Dirty Bitmap Directory entries are variable length (and
thus a variable maximum), they do have a minimum size (so the minimum
value for dirty_bitmap_directory_size must be larger than 0 unless
nb_dirty_bitmaps is 0, in which case why would we have this header
extension)

> +
> +  8 - 15:  dirty_bitmap_directory_offset
> +   Offset into the image file at which the Dirty Bitmap
> +   Directory starts. Must be aligned to a cluster boundary.
> +
> +
>  == Host cluster management ==
>  
>  qcow2 manages the allocation of host clusters by maintaining a reference 
> count
> @@ -360,3 +385,103 @@ Snapshot table entry:
>  
>  variable:   Padding to round up the snapshot table entry size to the
>  next multiple of 8.
> +
> +
> +== Dirty bitmaps ==
> +
> +The feature supports storing dirty bitmaps in a qcow2 image.
> +
> +=== Cluster mapping ===
> +
> +Dirty bitmaps are stored using a ONE-level structure for the mapping of
> +bitmaps to host clusters. It is called Dirty Bitmap Table.

s/ONE/one/ (I didn't see the reason for the emphasis)

> +
> +The Dirty Bitmap Table has a variable size (stored in the Dirty Bitmap

s/The/Each/

> +Directory Entry) and may use multiple clusters, however it must be contiguous
> +in the image file.
> +
> +Given an offset (in bytes) into the bitmap, the offset into the image file 
> can
> +be obtained as follows:
> +
> +byte_offset =
> +dirty_bitmap_table[offset / cluster_size] + (offset % cluster_size)
> +
> +Taking into accout the granularity of the bitmap, an offset in bits into the

s/accout/account/

> +image file can be obtained like this:
> +
> +bit_offset =
> +

[Qemu-devel] [PATCH v3 37/46] contrib: remove unnecessary strdup()

[marcandre . lureau]

From: Marc-André Lureau 

getopt() optarg points to argv memory, no need to dup those values,
fixes small leaks detected by clang-analyzer.

Signed-off-by: Marc-André Lureau 
---
 contrib/ivshmem-client/main.c | 2 +-
 contrib/ivshmem-server/main.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/ivshmem-client/main.c b/contrib/ivshmem-client/main.c
index 5d85ae7..bd7cbfc 100644
--- a/contrib/ivshmem-client/main.c
+++ b/contrib/ivshmem-client/main.c
@@ -53,7 +53,7 @@ ivshmem_client_parse_args(IvshmemClientArgs *args, int argc, 
char *argv[])
 break;
 
 case 'S': /* unix_sock_path */
-args->unix_sock_path = strdup(optarg);
+args->unix_sock_path = optarg;
 break;
 
 default:
diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
index cd8d9ed..71e87ea 100644
--- a/contrib/ivshmem-server/main.c
+++ b/contrib/ivshmem-server/main.c
@@ -92,15 +92,15 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int 
argc, char *argv[])
 break;
 
 case 'p': /* pid_file */
-args->pid_file = strdup(optarg);
+args->pid_file = optarg;
 break;
 
 case 'S': /* unix_socket_path */
-args->unix_socket_path = strdup(optarg);
+args->unix_socket_path = optarg;
 break;
 
 case 'm': /* shm_path */
-args->shm_path = strdup(optarg);
+args->shm_path = optarg;
 break;
 
 case 'l': /* shm_size */
-- 
2.4.3

[Qemu-devel] [PATCH v3 32/46] ivshmem-client: check the number of vectors

[marcandre . lureau]

From: Marc-André Lureau 

Check the number of vectors received from the server, to avoid
out of bound array access.

Signed-off-by: Marc-André Lureau 
---
 contrib/ivshmem-client/ivshmem-client.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/contrib/ivshmem-client/ivshmem-client.c 
b/contrib/ivshmem-client/ivshmem-client.c
index 11c805c..01e24a7 100644
--- a/contrib/ivshmem-client/ivshmem-client.c
+++ b/contrib/ivshmem-client/ivshmem-client.c
@@ -128,6 +128,10 @@ ivshmem_client_handle_server_msg(IvshmemClient *client)
 /* new vector */
 IVSHMEM_CLIENT_DEBUG(client, "  new vector %d (fd=%d) for peer id %ld\n",
  peer->vectors_count, fd, peer->id);
+if (peer->vectors_count >= G_N_ELEMENTS(peer->vectors)) {
+return -1;
+}
+
 peer->vectors[peer->vectors_count] = fd;
 peer->vectors_count++;
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 29/46] ivshmem: error on too many eventfd received

[marcandre . lureau]

From: Marc-André Lureau 

The number of eventfd that can be handled per peer is limited by the
number of vectors. Return an error when receiving too many of them.

Signed-off-by: Marc-André Lureau 
---
 hw/misc/ivshmem.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index b9c78cd..63e4c4f 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -569,6 +569,13 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, 
int size)
 }
 
 /* get a new eventfd */
+if (peer->nb_eventfds >= s->vectors) {
+error_report("Too many eventfd received, device has %d vectors",
+ s->vectors);
+close(incoming_fd);
+return;
+}
+
 nth_eventfd = peer->nb_eventfds++;
 
 /* this is an eventfd for a particular peer VM */
-- 
2.4.3

[Qemu-devel] [PATCH v3 39/46] qtest: add qtest_add_abrt_handler()

[marcandre . lureau]

From: Marc-André Lureau 

Allow a test to add abort handlers, use GHook for all handlers.

There is currently no way to remove a handler, but it could be
later added if needed.

Signed-off-by: Marc-André Lureau 
---
 tests/libqtest.c | 37 -
 tests/libqtest.h |  2 ++
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index e5188e0..4a3a6ad 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -49,6 +49,7 @@ struct QTestState
 struct sigaction sigact_old; /* restored on exit */
 };
 
+static GHookList abrt_hooks;
 static GList *qtest_instances;
 static struct sigaction sigact_old;
 
@@ -112,10 +113,7 @@ static void kill_qemu(QTestState *s)
 
 static void sigabrt_handler(int signo)
 {
-GList *elem;
-for (elem = qtest_instances; elem; elem = elem->next) {
-kill_qemu(elem->data);
-}
+g_hook_list_invoke(&abrt_hooks, FALSE);
 }
 
 static void setup_sigabrt_handler(void)
@@ -136,6 +134,23 @@ static void cleanup_sigabrt_handler(void)
 sigaction(SIGABRT, &sigact_old, NULL);
 }
 
+void qtest_add_abrt_handler(void (*fn), const void *data)
+{
+GHook *hook;
+
+/* Only install SIGABRT handler once */
+if (!abrt_hooks.is_setup) {
+g_hook_list_init(&abrt_hooks, sizeof(GHook));
+setup_sigabrt_handler();
+}
+
+hook = g_hook_alloc(&abrt_hooks);
+hook->func = fn;
+hook->data = (void *)data;
+
+g_hook_prepend(&abrt_hooks, hook);
+}
+
 QTestState *qtest_init(const char *extra_args)
 {
 QTestState *s;
@@ -156,12 +171,7 @@ QTestState *qtest_init(const char *extra_args)
 sock = init_socket(socket_path);
 qmpsock = init_socket(qmp_socket_path);
 
-/* Only install SIGABRT handler once */
-if (!qtest_instances) {
-setup_sigabrt_handler();
-}
-
-qtest_instances = g_list_prepend(qtest_instances, s);
+qtest_add_abrt_handler(kill_qemu, s);
 
 s->qemu_pid = fork();
 if (s->qemu_pid == 0) {
@@ -209,13 +219,14 @@ QTestState *qtest_init(const char *extra_args)
 
 void qtest_quit(QTestState *s)
 {
+qtest_instances = g_list_remove(qtest_instances, s);
+g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
+
 /* Uninstall SIGABRT handler on last instance */
-if (qtest_instances && !qtest_instances->next) {
+if (!qtest_instances) {
 cleanup_sigabrt_handler();
 }
 
-qtest_instances = g_list_remove(qtest_instances, s);
-
 kill_qemu(s);
 close(s->fd);
 close(s->qmp_fd);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index ec42031..f02c07c 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -427,6 +427,8 @@ void qtest_add_data_func(const char *str, const void *data, 
void (*fn));
 g_free(path); \
 } while (0)
 
+void qtest_add_abrt_handler(void (*fn), const void *data);
+
 /**
  * qtest_start:
  * @args: other arguments to pass to QEMU
-- 
2.4.3