Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL

[Markus Armbruster]

Eric Blake  writes:

> On 09/11/2015 02:43 PM, Eric Blake wrote:
>
>>> +++ b/tests/test-qmp-output-visitor.c
>>> @@ -485,7 +485,7 @@ static void 
>>> test_visitor_out_empty(TestOutputVisitorData *data,
>>>  QObject *arg;
>>>  
>>>  arg = qmp_output_get_qobject(data->qov);
>>> -g_assert(!arg);
>>> +g_assert(qobject_type(arg) == QTYPE_QNULL);
>>>  }
>> 
>> Missing
>>  qobject_decref(arg);
>> 
>> Ultimately, the global qnull_ starts life with refcnt 1, and after this
>> test should be back to 1.  But it is coming back as 3, so even with a
>> qobject_decref, that would get it back down to 2.  So we are leaking a
>> reference to qnull somewhere.

Relatively harmless, because the qnull_ singleton is static.  Worth
fixing anyway, of course.

>> I'm still investigating, and may be able to find the patch
>
> Squash this in, and you can have:
> Reviewed-by: Eric Blake 
>
> diff --git i/qapi/qmp-output-visitor.c w/qapi/qmp-output-visitor.c
> index 2d6083e..b96849e 100644
> --- i/qapi/qmp-output-visitor.c
> +++ w/qapi/qmp-output-visitor.c
> @@ -66,11 +66,7 @@ static QObject *qmp_output_first(QmpOutputVisitor *qov)
>  {
>  QStackEntry *e = QTAILQ_LAST(&qov->stack, QStack);
>
> -if (!e) {
> -return qnull();
> -}
> -
> -return e->value;
> +return e ? e->value : NULL;
>  }
>
>  static QObject *qmp_output_last(QmpOutputVisitor *qov)
> @@ -190,6 +186,8 @@ QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
>  QObject *obj = qmp_output_first(qov);
>  if (obj) {
>  qobject_incref(obj);
> +} else {
> +obj = qnull();
>  }
>  return obj;
>  }

The visitor code is disgustingly ambiguous / confused / confusing about
NULL.  The whole thing feels like it was hacked up in a rush.  We've
been paying interest for it not having a written contract ever since.

I put the qnull() in qmp_output_first() to avoid (QObject *)0 entirely.
Also because that's where I found the FIXME :)

You lift it into one of two callers.  Impact:

* qmp_output_get_qobject()

  - master: return NULL when !e || !e->value

  - my patch: return qnull() when !e
  return NULL when !e->value

  - your patch: return qnull() when !e || !e->value

* qmp_output_visitor_cleanup()

  - master: root = NULL when when !e || !e->value

  - my patch: root = qnull() when !e
  root = NULL when !e->value

  - your patch: root = NULL when when !e || !e->value

where e = QTAILQ_LAST(&qov->stack, QStack)

Questions:

* How can e become NULL?

  The only place that pushes onto &qov->stack appears to be
  qmp_output_push_obj().  Can obviously push e with !e->value, but can't
  push null e.

* What about qmp_output_last()?

  Why does qmp_output_first() check for !e, but not qmp_output_last()?

  My patch makes them less symmetric (bad smell).  Yours makes them more
  symmetric, but not quite.

* How does this contraption work?

> diff --git i/tests/test-qmp-output-visitor.c
> w/tests/test-qmp-output-visitor.c
> index 256bffd..8bd48f4 100644
> --- i/tests/test-qmp-output-visitor.c
> +++ w/tests/test-qmp-output-visitor.c
> @@ -486,6 +486,9 @@ static void
> test_visitor_out_empty(TestOutputVisitorData *data,
>
>  arg = qmp_output_get_qobject(data->qov);
>  g_assert(qobject_type(arg) == QTYPE_QNULL);
> +/* Check that qnull reference counting is sane */
> +g_assert(arg->refcnt == 2);
> +qobject_decref(arg);
>  }
>
>  static void init_native_list(UserDefNativeListUnion *cvalue)

Re: [Qemu-devel] [RFC 2/4] hw/i386: Introduce AMD IOMMU

[Valentine Sinitsyn]

Hi David,

On 07.09.2015 17:46, Valentine Sinitsyn wrote:
...snip...

+/* TODO : Mark addresses as Accessed and Dirty */
+static void amd_iommu_do_translate(AMDIOMMUAddressSpace *as, hwaddr
addr, bool is_write, IOMMUTLBEntry *ret)
+{
+AMDIOMMUState *s = as->iommu_state;
+
+int present;
+dma_addr_t pte_addr;
+uint64_t entry[4], pte, pte_perms;
+unsigned level;
+unsigned perms;
+
+if(!amd_iommu_get_dte(s, as->devfn, entry)){
+goto no_translation;
+}
+
+pte = entry[0];
+
+/*
+ * It's okay to check for either read or write permissions
+ * even for memory maps, since we don't support R/W maps.
+ */
+perms = is_write ? IOMMU_PERM_WRITE : IOMMU_PERM_READ;
+
+if(!(level = get_pte_translation_mode(pte))){
+goto no_translation;
+}
+
+while(level > 0){
+/*
+ * check permissions: the bitwise
+ * implication perms -> entry_perms must be true. Pages must
be present
+ * and permissions on all levels must be similar
+ */
+pte_perms = amd_iommu_get_perms(pte);
+present = pte & 1;
+if(!present || perms != (perms & pte_perms)){
+amd_iommu_page_fault(s, as->devfn, entry[1] &
DEV_DOMID_ID_MASK, addr, present,
+ !!(perms & IOMMU_PERM_WRITE));
+return;
+}
+
+/* go to the next lower level */
+pte_addr = pte & DEV_PT_ROOT_MASK;
+pte_addr += ((addr >> ( 9 * level)) & 0xff8);

Does this work for six level page tables? The highest level has
different bit size there IIRC.


+pte = ldq_phys(&address_space_memory, pte_addr);
+level = (pte >> DEV_MODE_RSHIFT) & DEV_MODE_MASK;
+}
+
+ret->iova = addr & IOMMU_PAGE_MASK_4K;
+ret->translated_addr = (pte & DEV_PT_ROOT_MASK) &
IOMMU_PAGE_MASK_4K;
+ret->addr_mask = ~IOMMU_PAGE_MASK_4K;
+ret->perm = IOMMU_RW;
+return;
+
+no_translation:
+ret->iova = addr;
+ret->translated_addr = addr & IOMMU_PAGE_MASK_4K;
+ret->addr_mask = ~IOMMU_PAGE_MASK_4K;
+ret->perm = IOMMU_RW;
+return;

Are you sure these transactions needs to be passed through rather than
target-aborted?

FYI: They are indeed passed-through; see Table 9 in the spec.

Valentine

Re: [Qemu-devel] [PATCH] pulseaudio: reduce 24s recording latency

[Volker Rümelin]

Hi,

Am 12.09.2015 um 01:22 schrieb Marc-André Lureau:


Current code doesn't provide pulseaudio buffer attributes for
recording. Without buffer attributes pulseaudio uses a default
buffer of 4MB. 4MB is approximately 24s 16bit stereo audio
data at 44.1kHz.

Why isn't the buffer processed as soon as some data is available?




On start up qemu opens a connection to pulseaudio in function 
qpa_init_in and pulseaudio immediately starts recording to the 4MB 
ringbuffer. The qemu guest, Windows 8.1 in my case, doesn't consume that 
data if there is no process listening on the audio interface. Now if the 
guest starts recording, it will see audio data which was recorded 24s ago.


Regards,
Volker

Re: [Qemu-devel] [PATCH v2] error: only prepend timestamp on stderr

[Markus Armbruster]

Stefan Hajnoczi  writes:

> The -msg timestamp=on option prepends a timestamp to error messages.
> This is useful on stderr where it allows users to identify when an error
> was raised.
>
> Timestamps do not make sense on the monitor since error_report() is
> called in response to a synchronous monitor command and the user already
> knows "when" the command was issued.  Additionally, the rest of the
> monitor conversation lacks timestamps so the error timestamp cannot be
> correlated with other activity.
>
> Only prepend timestamps on stderr.  This fixes libvirt's 'drive_del'
> processing, which did not expect a timestamp.  Other QEMU monitor
> clients are probably equally confused by timestamps on monitor error
> messages.
>
> Cc: Markus Armbruster 
> Cc: Seiji Aguchi 
> Cc: Frank Schreuder 
> Cc: Daniel P. Berrange 
> Signed-off-by: Stefan Hajnoczi 

I'll take this through my tree.  Thanks!

[Qemu-devel] [PATCH] MAINTAINERS: Add "Error reporting" entry

[Markus Armbruster]

Error reporting work has been flowing through my tree for a while.
Time for MAINTAINERS to catch up.

Signed-off-by: Markus Armbruster 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 688979b..54971a7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -903,6 +903,14 @@ M: Alexander Graf 
 S: Maintained
 F: device_tree.[ch]
 
+Error reporting
+M: Markus Armbruster 
+S: Supported
+F: include/qapi/error.h
+F: include/qemu/error-report.h
+F: util/error.c
+F: util/qemu-error.c
+
 GDB stub
 L: qemu-devel@nongnu.org
 S: Odd Fixes
-- 
2.4.3

Re: [Qemu-devel] [PATCH] pulseaudio: reduce 24s recording latency

[Marc-André Lureau]

Hi

On Fri, Sep 11, 2015 at 9:03 PM, Volker Rümelin  wrote:
> Current code doesn't provide pulseaudio buffer attributes for
> recording. Without buffer attributes pulseaudio uses a default
> buffer of 4MB. 4MB is approximately 24s 16bit stereo audio
> data at 44.1kHz.
>
> This patch fixes bug #1492649
>
> |Signed-off-by: Volker Rümelin |
> ---
>  audio/paaudio.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/audio/paaudio.c b/audio/paaudio.c
> index fea6071..c1169d4 100644
> --- a/audio/paaudio.c
> +++ b/audio/paaudio.c
> @@ -608,6 +608,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings 
> *as, void *drv_opaque)
>  {
>  int error;
>  pa_sample_spec ss;
> +pa_buffer_attr ba;
>  struct audsettings obt_as = *as;
>  PAVoiceIn *pa = (PAVoiceIn *) hw;
>  paaudio *g = pa->g = drv_opaque;
> @@ -616,6 +617,12 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings 
> *as, void *drv_opaque)
>  ss.channels = as->nchannels;
>  ss.rate = as->freq;
>
> +ba.fragsize = pa_frame_size (&ss) * g->conf.samples;
> +ba.maxlength = 5 * ba.fragsize;

This 5 value is a bit arbitrary, I suppose it may increase max latency
up to 5 times the g->conf.samples. (what about using
PA_STREAM_ADJUST_LATENCY?)

Otherwise, the change looks good to me.

> +ba.tlength = -1;
> +ba.prebuf = -1;
> +ba.minreq = -1;
> +
>  obt_as.fmt = pa_to_audfmt (ss.format, &obt_as.endianness);
>
>  pa->stream = qpa_simple_new (
> @@ -625,7 +632,7 @@ static int qpa_init_in(HWVoiceIn *hw, struct audsettings 
> *as, void *drv_opaque)
>  g->conf.source,
>  &ss,
>  NULL,   /* channel map */
> -NULL,   /* buffering attributes */
> +&ba,/* buffering attributes */
>  &error
>  );
>  if (!pa->stream) {
> --
> 1.8.4.5
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 04/29] Introduce QDict

[Programmingkid]

On Sep 12, 2015, at 12:10 AM, Luiz Capitulino wrote:

> On Fri, 11 Sep 2015 20:22:38 -0400
> Programmingkid  wrote:
> 
>> Could you make a tutorial on how to use the QDict type?
> 
> There are several examples in tests/check-qdict.c.

I was hoping for something with a lot more explanation. 

Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL

[Eric Blake]

On 09/12/2015 02:10 AM, Markus Armbruster wrote:

> Relatively harmless, because the qnull_ singleton is static.  Worth
> fixing anyway, of course.
> 
>>> I'm still investigating, and may be able to find the patch
>>
>> Squash this in, and you can have:
>> Reviewed-by: Eric Blake 

Well, your further questions are spot on; my squash proposal isn't quite
right.


> I put the qnull() in qmp_output_first() to avoid (QObject *)0 entirely.
> Also because that's where I found the FIXME :)
> 
> You lift it into one of two callers.  Impact:
> 
> * qmp_output_get_qobject()
> 
>   - master: return NULL when !e || !e->value
> 
>   - my patch: return qnull() when !e
>   return NULL when !e->value
> 
>   - your patch: return qnull() when !e || !e->value

The leak in your patch was that qnull() increments the count, then
qmp_output_get_object() also increments the count.

I guess I'll have to investigate where e->value can be set to NULL to
see if my idea was safe, or if we'd have to do something even different.

If this were the only caller, then I guess we could always do something
like this in qmp_output_first(), leaving the possibility of returning
NULL for e->value:

if (!e) {
obj = qnull();
qobject_decref(obj); /* Caller will again increment refcount */
return obj;
}

But it's not the only caller.

> 
> * qmp_output_visitor_cleanup()
> 
>   - master: root = NULL when when !e || !e->value
> 
>   - my patch: root = qnull() when !e
>   root = NULL when !e->value
> 
>   - your patch: root = NULL when when !e || !e->value

And calls qobject_decref(root), but that is safe on NULL.

Here, your patch ends up with a net 0 refcnt on qnull() (incremented in
qmp_output_first(), decremented in the cleanup), but my idea above to
pre-decrement would be wrong.

Another option would be to keep your patch to qmp_output_first(), but
then fix qmp_output_get_object() to special case it it has an instance
of QNull (no refcnt change) vs. anything else (qobject_incref).  But
that feels gross.

> 
> where e = QTAILQ_LAST(&qov->stack, QStack)
> 
> Questions:
> 
> * How can e become NULL?
> 
>   The only place that pushes onto &qov->stack appears to be
>   qmp_output_push_obj().  Can obviously push e with !e->value, but can't
>   push null e.

My understanding is that qov->stack corresponds to nesting levels of {}
or [] in the JSON code.  The testsuite shows a case where the stack is
empty as one case where e is NULL, but if e is non-NULL, I'm not sure
whether e->value can ever be NULL.  I'll have to read the code more closely.

> 
> * What about qmp_output_last()?
> 
>   Why does qmp_output_first() check for !e, but not qmp_output_last()?
> 
>   My patch makes them less symmetric (bad smell).  Yours makes them more
>   symmetric, but not quite.

Which is awkward in its own right.

> 
> * How does this contraption work?

Indeed. Without reading further, we're both shooting in the dark for
something that makes tests pass, but without being a clean interface.

How about this: go ahead with your series as proposed, with the squash
hunk to tests/ to avoid the leak in the testsuite, but leaving the leak
in qmp_output_get_object(), and we address the leak in a followup patch.
 refcnt is size_t, so on 32-bit platforms, it could roll over after 4G
repeats of the leak and cause catastrophe, but I don't think we are
outputting qnull() frequently enough for the leak to bite while we wait
for a followup patch.

The followup patch(es) will then have to include some contract
documentation, so that we track what we learned while investigating the
code, and so that the next reader has more than just code to start from.


-- 
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] MAINTAINERS: Add "Error reporting" entry

[Eric Blake]

On 09/12/2015 05:29 AM, Markus Armbruster wrote:
> Error reporting work has been flowing through my tree for a while.
> Time for MAINTAINERS to catch up.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  MAINTAINERS | 8 
>  1 file changed, 8 insertions(+)

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/3] pci: Update pci_regs header

[Knut Omang]

On Thu, 2015-09-10 at 22:27 +0200, Knut Omang wrote:
> On Thu, 2015-09-10 at 21:11 +0300, Michael S. Tsirkin wrote:
> > On Tue, Sep 01, 2015 at 05:59:49PM +0200, Knut Omang wrote:
> > > Pull new version from kernel v4.1
> > 
> > Paolo sent what I consider a better version.

So looking at the details of the differences between the kernel and
qemu's version, I agree that Paolo's version are better, however there
were also a few cases where the consistency of naming and quality of
comments had improved on the kernel side.

> > Can you send just the patch removing duplicate symbols?

I have now hand merged what I considered improvements 
in the kernel version, will post a new version of the patch set
shortly.

Knut

> 
> Ok, I think I just misunderstood your intentions 
> - I thought you wanted to eventually arrive at "sharing" 
> the code with the kernel to simplify future updates,
> so I "helped" you with the last bit of work :-)
> 
> > > msi: Removed local definitions now in pci_regs.h
> > > Adds definitions necessary to support emulated SR/IOV.
> > > 
> > > Replace usage of PCI_MSIX_FLAGS_BIRMASK with PCI_MSIX_TABLE_BIR
> > > to keep in sync with the kernel for future updates.
> > 
> > I fixed kernel to keep the old name around.
> 
> Yes, I noticed just after I had submitted the patch set.
> I can rebase the patch set using a minimal patch for this one,
> 
> Knut
> 
> > > Replaced PCI_ERR_UNC_UND with _TRAIN for bit 0x0001
> > > from the kernel header to support aer code.
> > > Should probably be suggested as an upstream kernel fix.
> > 
> > Do we really need to generate these errors?
> > They were only present on old 1.0 hardware.
> > 
> > 
> > > 
> > > Signed-off-by: Knut Omang 
> > > ---
> > >  hw/i386/kvm/pci-assign.c  |   4 +-
> > >  hw/pci/msix.c |   2 +-
> > >  hw/vfio/pci.c |   9 +-
> > >  include/standard-headers/linux/pci_regs.h | 380
> > > ++
> > >  4 files changed, 291 insertions(+), 104 deletions(-)
> > > 
> > > diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
> > > index 74d22f4..36b66ea 100644
> > > --- a/hw/i386/kvm/pci-assign.c
> > > +++ b/hw/i386/kvm/pci-assign.c
> > > @@ -1320,8 +1320,8 @@ static int
> > > assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
> > >   PCI_MSIX_FLAGS_ENABLE |
> > > PCI_MSIX_FLAGS_MASKALL);
> > >  
> > >  msix_table_entry = pci_get_long(pci_dev->config + pos +
> > > PCI_MSIX_TABLE);
> > > -bar_nr = msix_table_entry & PCI_MSIX_FLAGS_BIRMASK;
> > > -msix_table_entry &= ~PCI_MSIX_FLAGS_BIRMASK;
> > > +bar_nr = msix_table_entry & PCI_MSIX_TABLE_BIR;
> > > +msix_table_entry &= ~PCI_MSIX_TABLE_BIR;
> > >  dev->msix_table_addr = pci_region[bar_nr].base_addr +
> > > msix_table_entry;
> > >  dev->msix_max = msix_max;
> > >  }
> > > diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> > > index 7716bf3..2788be2 100644
> > > --- a/hw/pci/msix.c
> > > +++ b/hw/pci/msix.c
> > > @@ -250,7 +250,7 @@ int msix_init(struct PCIDevice *dev, unsigned
> > > short nentries,
> > >   ranges_overlap(table_offset, table_size, pba_offset,
> > > pba_size)) ||
> > >  table_offset + table_size >
> > > memory_region_size(table_bar)
> > > > > 
> > >  pba_offset + pba_size > memory_region_size(pba_bar) ||
> > > -(table_offset | pba_offset) & PCI_MSIX_FLAGS_BIRMASK) {
> > > +(table_offset | pba_offset) & PCI_MSIX_TABLE_BIR) {
> > >  return -EINVAL;
> > >  }
> > >  
> > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > > index 4023d8e..b940df6 100644
> > > --- a/hw/vfio/pci.c
> > > +++ b/hw/vfio/pci.c
> > > @@ -2244,10 +2244,10 @@ static int
> > > vfio_early_setup_msix(VFIOPCIDevice *vdev)
> > >  pba = le32_to_cpu(pba);
> > >  
> > >  vdev->msix = g_malloc0(sizeof(*(vdev->msix)));
> > > -vdev->msix->table_bar = table & PCI_MSIX_FLAGS_BIRMASK;
> > > -vdev->msix->table_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
> > > -vdev->msix->pba_bar = pba & PCI_MSIX_FLAGS_BIRMASK;
> > > -vdev->msix->pba_offset = pba & ~PCI_MSIX_FLAGS_BIRMASK;
> > > +vdev->msix->table_bar = table & PCI_MSIX_TABLE_BIR;
> > > +vdev->msix->table_offset = table & ~PCI_MSIX_TABLE_BIR;
> > > +vdev->msix->pba_bar = pba & PCI_MSIX_TABLE_BIR;
> > > +vdev->msix->pba_offset = pba & ~PCI_MSIX_TABLE_BIR;
> > >  vdev->msix->entries = (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
> > >  
> > >  /*
> > > @@ -2281,7 +2281,6 @@ static int
> > > vfio_early_setup_msix(VFIOPCIDevice *vdev)
> > >  vdev->msix->table_bar,
> > >  vdev->msix->table_offset,
> > >  vdev->msix->entries);
> > > -
> > >  return 0;
> > >  }
> > >  
> > > diff --git a/include/standard-headers/linux/pci_regs.h
> > > b/include/standard-headers/linux/pci_regs.h
> > > index 57e8c80..4414a81 10064

[Qemu-devel] [PATCH v4 1/3] pci: Make use of the devfn property when registering new devices

[Knut Omang]

Without this, the devfn argument to pci_create_*()
does not affect the assigned devfn.

Needed to support (VF_STRIDE,VF_OFFSET) values other than (1,1)
for SR/IOV.

Signed-off-by: Knut Omang 
---
 hw/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ccea628..a5cc015 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1840,7 +1840,7 @@ static void pci_qdev_realize(DeviceState *qdev, Error 
**errp)
 bus = PCI_BUS(qdev_get_parent_bus(qdev));
 pci_dev = do_pci_register_device(pci_dev, bus,
  object_get_typename(OBJECT(qdev)),
- pci_dev->devfn, errp);
+ object_property_get_int(OBJECT(qdev), 
"addr", NULL), errp);
 if (pci_dev == NULL)
 return;
 
-- 
2.4.3

[Qemu-devel] [PATCH v4 2/3] pci: Update pci_regs header

[Knut Omang]

Merge in new definitions from kernel v4.2
Adds definition necessary to support emulated SR/IOV.

Signed-off-by: Knut Omang 
---
 include/standard-headers/linux/pci_regs.h | 142 +-
 1 file changed, 118 insertions(+), 24 deletions(-)

diff --git a/include/standard-headers/linux/pci_regs.h 
b/include/standard-headers/linux/pci_regs.h
index 57e8c80..3d56c1e 100644
--- a/include/standard-headers/linux/pci_regs.h
+++ b/include/standard-headers/linux/pci_regs.h
@@ -304,13 +304,14 @@
 #define PCI_MSI_PENDING_64 20  /* Pending bits register for 32-bit 
devices */
 
 /* MSI-X registers */
-#define PCI_MSIX_FLAGS 2
-#define  PCI_MSIX_FLAGS_QSIZE  0x7FF
-#define  PCI_MSIX_FLAGS_ENABLE (1 << 15)
-#define  PCI_MSIX_FLAGS_MASKALL(1 << 14)
-#define PCI_MSIX_TABLE 4
-#define PCI_MSIX_PBA   8
+#define PCI_MSIX_FLAGS 2   /* Message Control */
+#define  PCI_MSIX_FLAGS_QSIZE  0x7FF   /* Table size */
+#define  PCI_MSIX_FLAGS_ENABLE (1 << 15) /* MSI-X enable */
+#define  PCI_MSIX_FLAGS_MASKALL(1 << 14) /* Mask all vectors for this 
function */
+#define PCI_MSIX_TABLE 4   /* Table offset */
+#define PCI_MSIX_PBA   8   /* Pending Bit Array offset */
 #define  PCI_MSIX_FLAGS_BIRMASK(7 << 0)
+#define PCI_CAP_MSIX_SIZEOF12  /* size of MSIX capability structure */
 
 /* MSI-X entry's format */
 #define PCI_MSIX_ENTRY_SIZE16
@@ -517,31 +518,63 @@
 #define  PCI_EXP_OBFF_MSG  0x4 /* New message signaling */
 #define  PCI_EXP_OBFF_WAKE 0x8 /* Re-use WAKE# for OBFF */
 #define PCI_EXP_DEVCTL240  /* Device Control 2 */
-#define  PCI_EXP_DEVCTL2_ARI   0x20/* Alternative Routing-ID */
-#define  PCI_EXP_IDO_REQ_EN0x100   /* ID-based ordering request enable */
-#define  PCI_EXP_IDO_CMP_EN0x200   /* ID-based ordering completion enable 
*/
-#define  PCI_EXP_LTR_EN0x400   /* Latency tolerance reporting 
*/
-#define  PCI_EXP_OBFF_MSGA_EN  0x2000  /* OBFF enable with Message type A */
-#define  PCI_EXP_OBFF_MSGB_EN  0x4000  /* OBFF enable with Message type B */
-#define  PCI_EXP_OBFF_WAKE_EN  0x6000  /* OBFF using WAKE# signaling */
+#define  PCI_EXP_DEVCTL2_COMP_TIMEOUT  0x000f  /* Completion Timeout Value */
+#define  PCI_EXP_DEVCTL2_ARI   0x0020  /* Alternative Routing-ID */
+#define  PCI_EXP_DEVCTL2_IDO_REQ_EN0x0100  /* Allow IDO for requests */
+#define  PCI_EXP_DEVCTL2_IDO_CMP_EN0x0200  /* Allow IDO for completions */
+#define  PCI_EXP_DEVCTL2_LTR_EN0x0400  /* Enable LTR mechanism 
*/
+#define  PCI_EXP_DEVCTL2_OBFF_MSGA_EN  0x2000  /* Enable OBFF Message type A */
+#define  PCI_EXP_DEVCTL2_OBFF_MSGB_EN  0x4000  /* Enable OBFF Message type B */
+#define  PCI_EXP_DEVCTL2_OBFF_WAKE_EN  0x6000  /* OBFF using WAKE# signaling */
+#define PCI_EXP_DEVSTA242  /* Device Status 2 */
+#define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2 44  /* v2 endpoints end here */
+#define PCI_EXP_LNKCAP244  /* Link Capabilities 2 */
+#define  PCI_EXP_LNKCAP2_SLS_2_5GB 0x0002 /* Supported Speed 2.5GT/s */
+#define  PCI_EXP_LNKCAP2_SLS_5_0GB 0x0004 /* Supported Speed 5.0GT/s */
+#define  PCI_EXP_LNKCAP2_SLS_8_0GB 0x0008 /* Supported Speed 8.0GT/s */
+#define  PCI_EXP_LNKCAP2_CROSSLINK 0x0100 /* Crosslink supported */
 #define PCI_EXP_LNKCTL248  /* Link Control 2 */
-#define PCI_EXP_SLTCTL256  /* Slot Control 2 */
+#define PCI_EXP_LNKSTA250  /* Link Status 2 */
+#define PCI_EXP_SLTCAP252  /* Slot Capabilities 2 */
+#define PCI_EXP_SLTCTL256  /* Slot Control 2 */
+#define PCI_EXP_SLTSTA258  /* Slot Status 2 */
 
 /* Extended Capabilities (PCI-X 2.0 and Express) */
 #define PCI_EXT_CAP_ID(header) (header & 0x)
 #define PCI_EXT_CAP_VER(header)((header >> 16) & 0xf)
 #define PCI_EXT_CAP_NEXT(header)   ((header >> 20) & 0xffc)
 
-#define PCI_EXT_CAP_ID_ERR 1
-#define PCI_EXT_CAP_ID_VC  2
-#define PCI_EXT_CAP_ID_DSN 3
-#define PCI_EXT_CAP_ID_PWR 4
-#define PCI_EXT_CAP_ID_VNDR11
-#define PCI_EXT_CAP_ID_ACS 13
-#define PCI_EXT_CAP_ID_ARI 14
-#define PCI_EXT_CAP_ID_ATS 15
-#define PCI_EXT_CAP_ID_SRIOV   16
-#define PCI_EXT_CAP_ID_LTR 24
+#define PCI_EXT_CAP_ID_ERR 0x01/* Advanced Error Reporting */
+#define PCI_EXT_CAP_ID_VC  0x02/* Virtual Channel Capability */
+#define PCI_EXT_CAP_ID_DSN 0x03/* Device Serial Number */
+#define PCI_EXT_CAP_ID_PWR 0x04/* Power Budgeting */
+#define PCI_EXT_CAP_ID_RCLD0x05/* Root Complex Link Declaration */
+#define PCI_EXT_CAP_ID_RCILC   0x06/* Root Complex Internal Link Control */
+#define PCI_EXT_CAP_ID_RCEC0x07/* Root Complex Event Collector */
+#define PCI_EXT_CAP_ID_MFVC0x08/* Multi-Function VC Capability */

[Qemu-devel] [PATCH v4 0/3] pcie: Add support for Single Root I/O Virtualization

[Knut Omang]

This patch set implements generic support for SR/IOV as an extension to the
core PCIe functionality, similar to the way other capabilities such as AER
is implemented.

There is no implementation of any device that provides
SR/IOV support included, but I have implemented a test
example which can be found together with this patch set here:

  git://github.com/knuto/qemu.git sriov_patches_v4

Testing with the example device was documented here:

  http://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg05110.html

Changes since v3:
  - Reworked 'pci: Update pci_regs header' to merge kernel version improvements
with the current qemu version instead of copying from the kernel version.

Changes since v2:
  - Rebased onto 090d0bfd
  - Un-qdev'ified - avoids issues when resetting NUM_VFS
  - Fixed handling of vf_offset/vf_stride

Changes since v1:
  - Rebased on top of latest master, eliminating prereqs.
  - Implement proper support for VF_STRIDE, VF_OFFSET and SUP_PGSIZE
Time better spent fixing it than explaining what the previous
limitations were.
- Added new first patch to fix pci bug related to this
  - Split out patch to pci_default_config_write to a separate patch 2
to highlight bug fix.
  - Refactored out logic into new source files
hw/pci/pcie_sriov.c include/hw/pci/pcie_sriov.h
similar to pcie_aer.c/h.
  - Rename functions and introduce structs to better separate
pf and vf functionality.
  - Replaced is_vf member with pci_is_vf() function abstraction
  - Fix numerous syntax, whitespace and comment issues
according to Michael's review.
  - Fix memory leaks.
  - Removed igb example device - a rebased version available
on github instead.

Knut Omang (3):
  pci: Make use of the devfn property when registering new devices
  pci: Update pci_regs header
  pcie: Add support for Single Root I/O Virtualization (SR/IOV)

 hw/pci/Makefile.objs  |   2 +-
 hw/pci/pci.c  | 101 ---
 hw/pci/pcie.c |   9 +-
 hw/pci/pcie_sriov.c   | 271 ++
 include/hw/pci/pci.h  |  11 +-
 include/hw/pci/pcie.h |   6 +
 include/hw/pci/pcie_sriov.h   |  55 ++
 include/qemu/typedefs.h   |   2 +
 include/standard-headers/linux/pci_regs.h | 142 +---
 9 files changed, 545 insertions(+), 54 deletions(-)
 create mode 100644 hw/pci/pcie_sriov.c
 create mode 100644 include/hw/pci/pcie_sriov.h

--
2.4.3

[Qemu-devel] [PATCH v4 3/3] pcie: Add support for Single Root I/O Virtualization (SR/IOV)

[Knut Omang]

This patch provides the building blocks for creating an SR/IOV
PCIe Extended Capability header and register/unregister
SR/IOV Virtual Functions.

Signed-off-by: Knut Omang 
---
 hw/pci/Makefile.objs|   2 +-
 hw/pci/pci.c|  99 
 hw/pci/pcie.c   |   9 +-
 hw/pci/pcie_sriov.c | 271 
 include/hw/pci/pci.h|  11 +-
 include/hw/pci/pcie.h   |   6 +
 include/hw/pci/pcie_sriov.h |  55 +
 include/qemu/typedefs.h |   2 +
 8 files changed, 426 insertions(+), 29 deletions(-)
 create mode 100644 hw/pci/pcie_sriov.c
 create mode 100644 include/hw/pci/pcie_sriov.h

diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
index 9f905e6..2226980 100644
--- a/hw/pci/Makefile.objs
+++ b/hw/pci/Makefile.objs
@@ -3,7 +3,7 @@ common-obj-$(CONFIG_PCI) += msix.o msi.o
 common-obj-$(CONFIG_PCI) += shpc.o
 common-obj-$(CONFIG_PCI) += slotid_cap.o
 common-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
-common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o
+common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o pcie_sriov.o
 
 common-obj-$(call lnot,$(CONFIG_PCI)) += pci-stub.o
 common-obj-$(CONFIG_ALL) += pci-stub.o
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index a5cc015..9c0eba1 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -153,6 +153,9 @@ int pci_bar(PCIDevice *d, int reg)
 {
 uint8_t type;
 
+/* PCIe virtual functions do not have their own BARs */
+assert(!pci_is_vf(d));
+
 if (reg != PCI_ROM_SLOT)
 return PCI_BASE_ADDRESS_0 + reg * 4;
 
@@ -211,22 +214,13 @@ void pci_device_deassert_intx(PCIDevice *dev)
 }
 }
 
-static void pci_do_device_reset(PCIDevice *dev)
+static void pci_reset_regions(PCIDevice *dev)
 {
 int r;
+if (pci_is_vf(dev)) {
+return;
+}
 
-pci_device_deassert_intx(dev);
-assert(dev->irq_state == 0);
-
-/* Clear all writable bits */
-pci_word_test_and_clear_mask(dev->config + PCI_COMMAND,
- pci_get_word(dev->wmask + PCI_COMMAND) |
- pci_get_word(dev->w1cmask + PCI_COMMAND));
-pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
- pci_get_word(dev->wmask + PCI_STATUS) |
- pci_get_word(dev->w1cmask + PCI_STATUS));
-dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
-dev->config[PCI_INTERRUPT_LINE] = 0x0;
 for (r = 0; r < PCI_NUM_REGIONS; ++r) {
 PCIIORegion *region = &dev->io_regions[r];
 if (!region->size) {
@@ -240,6 +234,27 @@ static void pci_do_device_reset(PCIDevice *dev)
 pci_set_long(dev->config + pci_bar(dev, r), region->type);
 }
 }
+}
+
+static void pci_do_device_reset(PCIDevice *dev)
+{
+qdev_reset_all(&dev->qdev);
+
+dev->irq_state = 0;
+pci_update_irq_status(dev);
+pci_device_deassert_intx(dev);
+assert(dev->irq_state == 0);
+
+/* Clear all writable bits */
+pci_word_test_and_clear_mask(dev->config + PCI_COMMAND,
+ pci_get_word(dev->wmask + PCI_COMMAND) |
+ pci_get_word(dev->w1cmask + PCI_COMMAND));
+pci_word_test_and_clear_mask(dev->config + PCI_STATUS,
+ pci_get_word(dev->wmask + PCI_STATUS) |
+ pci_get_word(dev->w1cmask + PCI_STATUS));
+dev->config[PCI_CACHE_LINE_SIZE] = 0x0;
+dev->config[PCI_INTERRUPT_LINE] = 0x0;
+pci_reset_regions(dev);
 pci_update_mappings(dev);
 
 msi_reset(dev);
@@ -771,6 +786,15 @@ static void pci_init_multifunction(PCIBus *bus, PCIDevice 
*dev, Error **errp)
 dev->config[PCI_HEADER_TYPE] |= PCI_HEADER_TYPE_MULTI_FUNCTION;
 }
 
+/* With SR/IOV and ARI, a device at function 0 need not be a multifunction
+ * device, as it may just be a VF that ended up with function 0 in
+ * the legacy PCI interpretation. Avoid failing in such cases:
+ */
+if (pci_is_vf(dev) &&
+dev->exp.sriov_vf.pf->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
+return;
+}
+
 /*
  * multifunction bit is interpreted in two ways as follows.
  *   - all functions must set the bit to 1.
@@ -962,6 +986,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
 uint64_t wmask;
 pcibus_t size = memory_region_size(memory);
 
+assert(!pci_is_vf(pci_dev)); /* VFs must use pcie_sriov_vf_register_bar */
 assert(region_num >= 0);
 assert(region_num < PCI_NUM_REGIONS);
 if (size & (size-1)) {
@@ -1060,11 +1085,44 @@ pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int 
region_num)
 return pci_dev->io_regions[region_num].addr;
 }
 
-static pcibus_t pci_bar_address(PCIDevice *d,
-   int reg, uint8_t type, pcibus_t size)
+
+static pcibus_t pci_config_get_bar_addr(PCIDevice *d, int reg,
+uint8_t type, pcibus_t size)
+{
+pcibus_t

Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL

[Eric Blake]

On 09/12/2015 06:16 AM, Eric Blake wrote:

>>
>> where e = QTAILQ_LAST(&qov->stack, QStack)
>>
>> Questions:
>>
>> * How can e become NULL?
>>
>>   The only place that pushes onto &qov->stack appears to be
>>   qmp_output_push_obj().  Can obviously push e with !e->value, but can't
>>   push null e.
> 
> My understanding is that qov->stack corresponds to nesting levels of {}
> or [] in the JSON code.  The testsuite shows a case where the stack is
> empty as one case where e is NULL, but if e is non-NULL, I'm not sure
> whether e->value can ever be NULL.  I'll have to read the code more closely.

I agree that qmp_object_push_obj() is the only place that pushes; and it
promptly calls qobject_type(e->value) which blindly dereferences
value->type.  So it sounds like e->value can never be NULL (or we would
segfault), although a well-placed assert in qmp-output-visitor.c would
be nicer than having to chase through qobject.h.

So the only way for qmp_object_first() to return NULL is if e is NULL
(the stack was empty), since e->value would be non-NULL.  Your patch
fixed it to never return NULL, mine fixed it to handle a NULL return in
callers that care (only 1 of the 2 callers).

> 
>>
>> * What about qmp_output_last()?
>>
>>   Why does qmp_output_first() check for !e, but not qmp_output_last()?
>>
>>   My patch makes them less symmetric (bad smell).  Yours makes them more
>>   symmetric, but not quite.
> 
> Which is awkward in its own right.

qmp_output_last() is only used by qmp_output_add_obj(), which first
checks for an empty queue; so the queue cannot be empty.  Therefore,
qmp_output_last() could assert that e != NULL. At any rate,
qmp_output_last() never returns NULL; pre-patch, only qmp_output_first()
could do so.

And qmp_output_add_obj() blindly calls qobject_type() on the result of
qmp_output_last(), which as argued before would segfault if e->value
could ever be NULL.

It looks like the role of qmp_output_last() is to determine the last
thing that was being output; if it is a QDict or QList, then add the
current visit on to that existing object; otherwise, the last thing
output is a scalar and is complete, so we are replacing the root with
the new object being output.  Meanwhile, the role of qmp_output_first()
appears to be to grab the root of the output tree - either to give the
caller the overall QObject* created by visiting a structure
(qmp_output_get_qobject, which must not return NULL), or to clean up all
references still stored in the stack.  It also looks like
qmp_output_get_qobject() can be called multiple times before cleanup (to
grab copies of the current root).

> 
>>
>> * How does this contraption work?
> 
> Indeed. Without reading further, we're both shooting in the dark for
> something that makes tests pass, but without being a clean interface.

It looks like the stack is actually tracking two things at once: the
oldest member of the stack (qmp_output_first(), or QTAILQ_LAST) is the
root (can be any QObject), and all other members of the stack hold any
open-ended container QObject (necessarily QDict or QList) that is still
having contents added (the newest member is qmp_output_last(), or
QTAILQ_FIRST).  It's a bit confusing that QTAILQ works in the opposite
direction of our terminology.

Hmm, qmp_output_add_obj() is still a bit odd.  If, on a brand new
visitor, we try to visit two scalars in a row (via consecutive
visit_type_int()), the first scalar will become the stack root, then the
second scalar will replace the first as the root.  But if we try to
visit two QDict in a row (via consecutive
visit_start_struct(),visit_type_FOO()...,visit_end_struct() sequences),
the first QDict becomes the stack root, gets pushed onto the stack a
second time to be the open-ended container for the visit_type_FOO()
calls, then gets popped only once from the stack when complete.  That
means the second QDict will attempt to add itself into the existing
root, instead of replacing the root.  A better behavior would be to
blindly replace the root node if the stack has exactly one element (so
that we are consistent on behavior when using a single visitor on two
top-level visits in a row), but that should be a separate patch - in
particular, I don't know how often we ever use a visitor on consecutive
top-level items to need to replace the root (our testsuite allocates a
new visitor every time, instead of reusing visitors).  More in another mail.

For the sake of our current issue, I think that adding comments to the
existing behavior is sufficient to explain why my approach works.  So
how about squashing this:


diff --git c/qapi/qmp-output-visitor.c w/qapi/qmp-output-visitor.c
index 2d6083e..d42ca0e 100644
--- c/qapi/qmp-output-visitor.c
+++ w/qapi/qmp-output-visitor.c
@@ -41,10 +41,12 @@ static QmpOutputVisitor *to_qov(Visitor *v)
 return container_of(v, QmpOutputVisitor, visitor);
 }

+/* Push @value onto the stack of current QObjects being built */
 static void qmp_output_push_obj(QmpOutputVisitor *qov, QO

Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL

[Eric Blake]

On 09/12/2015 07:42 AM, Eric Blake wrote:

> It looks like the stack is actually tracking two things at once: the
> oldest member of the stack (qmp_output_first(), or QTAILQ_LAST) is the
> root (can be any QObject), and all other members of the stack hold any
> open-ended container QObject (necessarily QDict or QList) that is still
> having contents added (the newest member is qmp_output_last(), or
> QTAILQ_FIRST).  It's a bit confusing that QTAILQ works in the opposite
> direction of our terminology.
> 
> Hmm, qmp_output_add_obj() is still a bit odd.  If, on a brand new
> visitor, we try to visit two scalars in a row (via consecutive
> visit_type_int()), the first scalar will become the stack root, then the
> second scalar will replace the first as the root.  But if we try to
> visit two QDict in a row (via consecutive
> visit_start_struct(),visit_type_FOO()...,visit_end_struct() sequences),
> the first QDict becomes the stack root, gets pushed onto the stack a
> second time to be the open-ended container for the visit_type_FOO()
> calls, then gets popped only once from the stack when complete.  That
> means the second QDict will attempt to add itself into the existing
> root, instead of replacing the root.  A better behavior would be to
> blindly replace the root node if the stack has exactly one element (so
> that we are consistent on behavior when using a single visitor on two
> top-level visits in a row), but that should be a separate patch - in
> particular, I don't know how often we ever use a visitor on consecutive
> top-level items to need to replace the root (our testsuite allocates a
> new visitor every time, instead of reusing visitors).  More in another mail.

As in this - instead of abusing the stack to track two things, make it
explicit that we have two things (to be applied on top of the previous
suggestion, but as a separate patch):

diff --git i/qapi/qmp-output-visitor.c w/qapi/qmp-output-visitor.c
index cddbdb6..093ffe4 100644
--- i/qapi/qmp-output-visitor.c
+++ w/qapi/qmp-output-visitor.c
@@ -30,6 +30,7 @@ struct QmpOutputVisitor
 {
 Visitor visitor;
 QStack stack;
+QObject *root;
 };

 #define qmp_output_add(qov, name, value) \
@@ -46,6 +47,7 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov,
QObject *value)
 {
 QStackEntry *e = g_malloc0(sizeof(*e));

+assert(qov->root);
 assert(value);
 e->value = value;
 if (qobject_type(e->value) == QTYPE_QLIST) {
@@ -70,23 +72,15 @@ static QObject *qmp_output_pop(QmpOutputVisitor *qov)
 /* Grab the root QObject, if any, in preparation to empty the stack */
 static QObject *qmp_output_first(QmpOutputVisitor *qov)
 {
-QStackEntry *e = QTAILQ_LAST(&qov->stack, QStack);
-
-if (!e) {
-/* No root */
-return NULL;
-}
-assert(e->value);
-return e->value;
+return qov->root;
 }

-/* Grab the most recent QObject from the stack, which must exist */
+/* Grab the most recent QObject from the stack, if any */
 static QObject *qmp_output_last(QmpOutputVisitor *qov)
 {
 QStackEntry *e = QTAILQ_FIRST(&qov->stack);

-assert(e);
-return e->value;
+return e ? e->value : NULL;
 }

 /* Add @value to the current QObject being built.
@@ -97,28 +91,23 @@ static void qmp_output_add_obj(QmpOutputVisitor
*qov, const char *name,
 {
 QObject *cur;

-if (QTAILQ_EMPTY(&qov->stack)) {
-/* Stack was empty, track this object as root */
-qmp_output_push_obj(qov, value);
-return;
-}
-
 cur = qmp_output_last(qov);

-switch (qobject_type(cur)) {
-case QTYPE_QDICT:
-assert(name);
-qdict_put_obj(qobject_to_qdict(cur), name, value);
-break;
-case QTYPE_QLIST:
-qlist_append_obj(qobject_to_qlist(cur), value);
-break;
-default:
-/* The previous root was a scalar, replace it with a new root */
-qobject_decref(qmp_output_pop(qov));
-assert(QTAILQ_EMPTY(&qov->stack));
-qmp_output_push_obj(qov, value);
-break;
+if (!cur) {
+qobject_decref(qov->root);
+qov->root = value;
+} else {
+switch (qobject_type(cur)) {
+case QTYPE_QDICT:
+assert(name);
+qdict_put_obj(qobject_to_qdict(cur), name, value);
+break;
+case QTYPE_QLIST:
+qlist_append_obj(qobject_to_qlist(cur), value);
+break;
+default:
+g_assert_not_reached();
+}
 }
 }

@@ -223,16 +212,12 @@ void qmp_output_visitor_cleanup(QmpOutputVisitor *v)
 {
 QStackEntry *e, *tmp;

-/* The bottom QStackEntry, if any, owns the root QObject. See the
- * qmp_output_push_obj() invocations in qmp_output_add_obj(). */
-QObject *root = QTAILQ_EMPTY(&v->stack) ? NULL : qmp_output_first(v);
-
 QTAILQ_FOREACH_SAFE(e, &v->stack, node, tmp) {
 QTAILQ_REMOVE(&v->stack, e, node);
 g_free(e);
 }

-qobject_decref(root);
+qobject_decref(v->root);
 g_free(v);
 

[Qemu-devel] Invitation: Dr Velano @ Wed Sep 16, 2015 19:50 - 20:50 (shlomopongr...@gmail.com)

[Shlomo Pongratz]

BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:REQUEST
BEGIN:VEVENT
DTSTART:20150916T165000Z
DTEND:20150916T175000Z
DTSTAMP:20150912T140307Z
ORGANIZER;CN=Shlomo Pongratz:mailto:shlomopongr...@gmail.com
UID:mhfgd22vmap7kbtja2d8p70...@google.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;RSVP=TRUE
 ;CN=Shlomo Pongratz;X-NUM-GUESTS=0:mailto:shlomopongr...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=lav...@bezeqint.net;X-NUM-GUESTS=0:mailto:lav...@bezeqint.net
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=qemu-devel@nongnu.org;X-NUM-GUESTS=0:mailto:qemu-devel@nongnu.org
CREATED:20150912T140307Z
DESCRIPTION:View your event at https://www.google.com/calendar/event?action
 =VIEW&eid=bWhmZ2QyMnZtYXA3a2J0amEyZDhwNzA4bGcgcWVtdS1kZXZlbEBub25nbnUub3Jn&
 tok=MjQjc2hsb21vcG9uZ3JhdHpAZ21haWwuY29tMjIyYzRhMzI5OGRlYzdjZTIxNGRmNWVkZmJ
 mMTY5MDFlZmY0ZDkyOQ&ctz=Asia/Jerusalem&hl=en.
LAST-MODIFIED:20150912T140307Z
LOCATION:Uri Tsvi Grinberg Street\, Tel Aviv-Yafo\, Israel 25
SEQUENCE:0
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 v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL

[Markus Armbruster]

Eric Blake  writes:

> On 09/12/2015 02:10 AM, Markus Armbruster wrote:
>
>> Relatively harmless, because the qnull_ singleton is static.  Worth
>> fixing anyway, of course.
>> 
 I'm still investigating, and may be able to find the patch
>>>
>>> Squash this in, and you can have:
>>> Reviewed-by: Eric Blake 
>
> Well, your further questions are spot on; my squash proposal isn't quite
> right.
>
>
>> I put the qnull() in qmp_output_first() to avoid (QObject *)0 entirely.
>> Also because that's where I found the FIXME :)
>> 
>> You lift it into one of two callers.  Impact:
>> 
>> * qmp_output_get_qobject()
>> 
>>   - master: return NULL when !e || !e->value
>> 
>>   - my patch: return qnull() when !e
>>   return NULL when !e->value
>> 
>>   - your patch: return qnull() when !e || !e->value
>
> The leak in your patch was that qnull() increments the count, then
> qmp_output_get_object() also increments the count.
>
> I guess I'll have to investigate where e->value can be set to NULL to
> see if my idea was safe, or if we'd have to do something even different.
>
> If this were the only caller, then I guess we could always do something
> like this in qmp_output_first(), leaving the possibility of returning
> NULL for e->value:
>
> if (!e) {
> obj = qnull();
> qobject_decref(obj); /* Caller will again increment refcount */
> return obj;
> }
>
> But it's not the only caller.
>
>> 
>> * qmp_output_visitor_cleanup()
>> 
>>   - master: root = NULL when when !e || !e->value
>> 
>>   - my patch: root = qnull() when !e
>>   root = NULL when !e->value
>> 
>>   - your patch: root = NULL when when !e || !e->value
>
> And calls qobject_decref(root), but that is safe on NULL.
>
> Here, your patch ends up with a net 0 refcnt on qnull() (incremented in
> qmp_output_first(), decremented in the cleanup), but my idea above to
> pre-decrement would be wrong.
>
> Another option would be to keep your patch to qmp_output_first(), but
> then fix qmp_output_get_object() to special case it it has an instance
> of QNull (no refcnt change) vs. anything else (qobject_incref).  But
> that feels gross.

It does.

>> where e = QTAILQ_LAST(&qov->stack, QStack)
>> 
>> Questions:
>> 
>> * How can e become NULL?
>> 
>>   The only place that pushes onto &qov->stack appears to be
>>   qmp_output_push_obj().  Can obviously push e with !e->value, but can't
>>   push null e.
>
> My understanding is that qov->stack corresponds to nesting levels of {}
> or [] in the JSON code.  The testsuite shows a case where the stack is
> empty as one case where e is NULL, but if e is non-NULL, I'm not sure
> whether e->value can ever be NULL.  I'll have to read the code more closely.
>
>> 
>> * What about qmp_output_last()?
>> 
>>   Why does qmp_output_first() check for !e, but not qmp_output_last()?
>> 
>>   My patch makes them less symmetric (bad smell).  Yours makes them more
>>   symmetric, but not quite.
>
> Which is awkward in its own right.
>
>> 
>> * How does this contraption work?
>
> Indeed. Without reading further, we're both shooting in the dark for
> something that makes tests pass, but without being a clean interface.
>
> How about this: go ahead with your series as proposed, with the squash
> hunk to tests/ to avoid the leak in the testsuite, but leaving the leak
> in qmp_output_get_object(), and we address the leak in a followup patch.

I'll add a FIXME explaining the reference counting bug, and the wider
problem.

What exactly do you want changed in tests?

>  refcnt is size_t, so on 32-bit platforms, it could roll over after 4G
> repeats of the leak and cause catastrophe,

Assertion failure in qnull_destroy_obj(), to be exact.

>but I don't think we are
> outputting qnull() frequently enough for the leak to bite while we wait
> for a followup patch.

Agree.

> The followup patch(es) will then have to include some contract
> documentation, so that we track what we learned while investigating the
> code, and so that the next reader has more than just code to start from.

It's time to retrofit visitors with a proper contract.

Retrofitting a contract is much harder than starting with one, but we
got to play the hand we've been dealt.

Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL

[Eric Blake]

On 09/12/2015 08:11 AM, Markus Armbruster wrote:

>> Indeed. Without reading further, we're both shooting in the dark for
>> something that makes tests pass, but without being a clean interface.
>>
>> How about this: go ahead with your series as proposed, with the squash
>> hunk to tests/ to avoid the leak in the testsuite, but leaving the leak
>> in qmp_output_get_object(), and we address the leak in a followup patch.

I've given a couple more responses with my analysis, but up to you how
much you want to take now or defer to later.

> 
> I'll add a FIXME explaining the reference counting bug, and the wider
> problem.
> 
> What exactly do you want changed in tests?

Definitely add the qobject_decref(arg). But the g_assert(refcnt...)
should not be added unless we go with a solution that doesn't leak, and
instead should be replaced with a FIXME, if you don't want my followup
mails now.


>> The followup patch(es) will then have to include some contract
>> documentation, so that we track what we learned while investigating the
>> code, and so that the next reader has more than just code to start from.
> 
> It's time to retrofit visitors with a proper contract.
> 
> Retrofitting a contract is much harder than starting with one, but we
> got to play the hand we've been dealt.

I've already started that work in some of my pending patches:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02597.html

but it could indeed use further documentation in each visitor
implementation.

-- 
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] pulseaudio: reduce 24s recording latency

[Kővágó Zoltán]

2015-09-12 13:23 keltezéssel, Volker Rümelin írta:

Hi,

Am 12.09.2015 um 01:22 schrieb Marc-André Lureau:


Current code doesn't provide pulseaudio buffer attributes for
recording. Without buffer attributes pulseaudio uses a default
buffer of 4MB. 4MB is approximately 24s 16bit stereo audio
data at 44.1kHz.

Why isn't the buffer processed as soon as some data is available?




On start up qemu opens a connection to pulseaudio in function
qpa_init_in and pulseaudio immediately starts recording to the 4MB
ringbuffer. The qemu guest, Windows 8.1 in my case, doesn't consume that
data if there is no process listening on the audio interface. Now if the
guest starts recording, it will see audio data which was recorded 24s ago.


Weird, pulseaudio shouldn't delay the input more than 2 seconds in the 
default config.  Maybe PA_STREAM_EARLY_REQUESTS help.  See my patch at 
[1].  Alternatively we should maybe call pa_stream_flush when enabling 
the input to tell pulseaudio to drop previously recorded samples.


[1]: https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02455.html

Zoltan

[Qemu-devel] [PATCH] linux-user/signal.c: Use setup_rt_frame() instead of setup_frame() for target openrisc

[gang . chen . 5i5j]

From: Chen Gang 

qemu has already considered about some targets may have no traditional
signals. And openrisc's setup_frame() is dummy, but it can be supported
by setup_rt_frame().

Signed-off-by: Chen Gang 
---
 linux-user/signal.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 502efd9..ac82baa 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -3900,12 +3900,6 @@ static inline abi_ulong get_sigframe(struct 
target_sigaction *ka,
 return sp;
 }
 
-static void setup_frame(int sig, struct target_sigaction *ka,
-target_sigset_t *set, CPUOpenRISCState *env)
-{
-qemu_log("Not implement.\n");
-}
-
 static void setup_rt_frame(int sig, struct target_sigaction *ka,
target_siginfo_t *info,
target_sigset_t *set, CPUOpenRISCState *env)
@@ -5662,7 +5656,8 @@ void process_pending_signals(CPUArchState *cpu_env)
 }
 #endif
 /* prepare the stack frame of the virtual CPU */
-#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
+#if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64) \
+|| defined(TARGET_OPENRISC)
 /* These targets do not have traditional signals.  */
 setup_rt_frame(sig, sa, &q->info, &target_old_set, cpu_env);
 #else
-- 
1.9.3

Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use setup_rt_frame() instead of setup_frame() for target openrisc

[Peter Maydell]

On 12 September 2015 at 16:32,   wrote:
> From: Chen Gang 
>
> qemu has already considered about some targets may have no traditional
> signals. And openrisc's setup_frame() is dummy, but it can be supported
> by setup_rt_frame().
>
> Signed-off-by: Chen Gang 
> ---
>  linux-user/signal.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add "Error reporting" entry

[Peter Crosthwaite]

On Sat, Sep 12, 2015 at 4:29 AM, Markus Armbruster  wrote:
> Error reporting work has been flowing through my tree for a while.
> Time for MAINTAINERS to catch up.
>
> Signed-off-by: Markus Armbruster 

Reviewed-by: Peter Crosthwaite 

> ---
>  MAINTAINERS | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 688979b..54971a7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -903,6 +903,14 @@ M: Alexander Graf 
>  S: Maintained
>  F: device_tree.[ch]
>
> +Error reporting
> +M: Markus Armbruster 
> +S: Supported
> +F: include/qapi/error.h
> +F: include/qemu/error-report.h
> +F: util/error.c
> +F: util/qemu-error.c
> +
>  GDB stub
>  L: qemu-devel@nongnu.org
>  S: Odd Fixes
> --
> 2.4.3
>
>

Re: [Qemu-devel] [PATCH v2 0/2] intel_iommu: Add support for translation for devices behind bridges

[Knut Omang]

On Thu, 2015-09-03 at 07:26 +0200, Knut Omang wrote:
> On Thu, 2015-09-03 at 08:33 +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2015-09-02 at 10:12 -0600, Alex Williamson wrote:
> > 
> > > There are very specific rules for translating requester IDs across
> > > bridges.  Bus numbers can change during enumeration, devfn cannot.
> 
> Thanks for clarifying that point, Alex, I realize I was a bit imprecise
> in my last mail,
> 
> > > devfn can however be masked by topology changes from PCIe to PCI.  If 
> > > we pretend that the IOMMU can distinguish requester IDs where it 
> > > can't on real hardware, we're going to break the guest.  Thanks,
> > 
> > Note that whether a PCI / PCI-X bridge will mask devfn, bus# or both or
> > even mask it partially (number of bits) or replace some transfers with
> > its own RID ... depends on a given bridge implementation.
> > 
> > Another thing is while I agree that the bus number is problematic,
> > since it changes, it is still what the HW actually uses to match the
> > requester in practice, at least on PHB and I would think on Intel.
> > 
> > The problem is more fundamental. qemu is trying to bind devices to
> > address spaces in a fixed way at device creation time, while this is
> > lazily resolved in HW at the point of the DMA occurring.
> 
> So let me try to sum up my understanding in context of the patch in
> terms of these two approaches,
> 
> > One way to fix it is to effectively have an address space per device,
> > and have the iommu translate function figure out the binding
> > dynamically and flush things if it detects a change. But that is tricky
> > for vfio and it means invalidations will have to iterate all address
> > spaces.
> 
> So my patch is along these lines by actually moving the address space
> pointer into the device struct.
> The benefit is that:
>   * The data structure for the DMA address space can be reused across 
> IOMMUs, and the address spaces can be set up before bus numbers are
> 
> assigned, and the implementation is fairly simple.
>   * The IOMMU does not have to be notified of bus changes, except for
> invalidation purposes (but wouldn't a new enumeration cause a full 
> IOMMU invalidate anyway?)
> 
> The drawbacks are:
>   * The IOMMUs get to know explicitly about devices behind a bridge, 
> which logically deviates from how hardware works and 
> complicates future attempts to implement bridges that 
> translate RIDs.
>   * Each device can have only one DMA address space mapping associated 
> with it (I suppose it might be possible to have a topology that 
> would allow multiple paths to a device, but do we care at this 
> stage?)
> 
> > The other option is to create Address Spaces on the fly as we lookup
> > domains, and bind them to devices lazily, but again, we need to deal
> > with changes/invalidations and that can be nasty with VFIO.
> 
> We could get here without changing the interfaces, by refining the
> current implementation to just cache bus pointers at setup, then lazily
> add address spaces for each device. This approach would yield IOMMU
> device specific implementations, but would still in practice associate
> devices with address spaces.

As the thread went silent after our conclusions, I have made a second
implementation for the Intel IOMMU according to this alternate scheme,
It keeps the current API and handles the bus number resolution lazily
within the IOMMU implementation, I will post the (single) patch as v3
of this. 

Hopefully this is acceptable and can be leveraged to do a similar
rework, or be abstracted as generic functionality (?) for the other
architectures,..

Thanks,

Knut

[Qemu-devel] [PATCH v3 0/1] intel_iommu: Add support for translation for devices behind bridges

[Knut Omang]

This patch set has been completely reimplemented according to ideas from the
discussion of v2.

It still solves the same problem, but does so only within the Intel IOMMU code 
and Q35,
without changing the IOMMU interface. This eliminates the need for any separate
interface change patch.

This is the thread following v2 of the patch set:

  http://thread.gmane.org/gmane.comp.emulators.qemu/358525

This is the thread following the initial patch set:

  http://thread.gmane.org/gmane.comp.emulators.qemu/302246

The patch set was also discussed in this thread:

  http://thread.gmane.org/gmane.comp.emulators.qemu/316949

Changes from v2:
  - Completely reimplemented fix to avoid API change and further
logical deviation from how hardware works.
API change no longer necessary, so just a single patch.

Changes from v1:
  - Rebased to current master
  - Fixed minor syntax issues

Knut Omang (1):
  intel_iommu: Add support for translation for devices behind bridges

 hw/i386/intel_iommu.c | 90 +++
 hw/pci-host/q35.c | 25 ++--
 include/hw/i386/intel_iommu.h | 16 +++-
 3 files changed, 91 insertions(+), 40 deletions(-)

--
2.4.3

[Qemu-devel] [PATCH v3 1/1] intel_iommu: Add support for translation for devices behind bridges

[Knut Omang]

- Use a hash table indexed on bus pointers to store information about buses
  instead of using the bus numbers.
  Bus pointers are stored in a new VTDBus struct together with the vector
  of device address space pointers indexed by devfn.
- The bus number is still used for lookup for selective SID based invalidate,
  in which case the bus number is lazily resolved from the bus hash table and
  cached in a separate index.

Signed-off-by: Knut Omang 
---
 hw/i386/intel_iommu.c | 90 +++
 hw/pci-host/q35.c | 25 ++--
 include/hw/i386/intel_iommu.h | 16 +++-
 3 files changed, 91 insertions(+), 40 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 08055a8..da67c36 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -22,6 +22,7 @@
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
 #include "intel_iommu_internal.h"
+#include "hw/pci/pci.h"
 
 /*#define DEBUG_INTEL_IOMMU*/
 #ifdef DEBUG_INTEL_IOMMU
@@ -166,19 +167,17 @@ static gboolean vtd_hash_remove_by_page(gpointer key, 
gpointer value,
  */
 static void vtd_reset_context_cache(IntelIOMMUState *s)
 {
-VTDAddressSpace **pvtd_as;
 VTDAddressSpace *vtd_as;
-uint32_t bus_it;
+VTDBus *vtd_bus;
+GHashTableIter bus_it;
 uint32_t devfn_it;
 
+g_hash_table_iter_init(&bus_it, s->vtd_as_by_busptr);
+
 VTD_DPRINTF(CACHE, "global context_cache_gen=1");
-for (bus_it = 0; bus_it < VTD_PCI_BUS_MAX; ++bus_it) {
-pvtd_as = s->address_spaces[bus_it];
-if (!pvtd_as) {
-continue;
-}
+while (g_hash_table_iter_next (&bus_it, NULL, (void**)&vtd_bus)) {
 for (devfn_it = 0; devfn_it < VTD_PCI_DEVFN_MAX; ++devfn_it) {
-vtd_as = pvtd_as[devfn_it];
+vtd_as = vtd_bus->dev_as[devfn_it];
 if (!vtd_as) {
 continue;
 }
@@ -754,12 +753,13 @@ static inline bool vtd_is_interrupt_addr(hwaddr addr)
  * @is_write: The access is a write operation
  * @entry: IOMMUTLBEntry that contain the addr to be translated and result
  */
-static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, uint8_t bus_num,
+static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,
uint8_t devfn, hwaddr addr, bool is_write,
IOMMUTLBEntry *entry)
 {
 IntelIOMMUState *s = vtd_as->iommu_state;
 VTDContextEntry ce;
+uint8_t bus_num = pci_bus_num(bus);
 VTDContextCacheEntry *cc_entry = &vtd_as->context_cache_entry;
 uint64_t slpte;
 uint32_t level;
@@ -874,6 +874,30 @@ static void vtd_context_global_invalidate(IntelIOMMUState 
*s)
 }
 }
 
+
+/* Find the VTD address space currently associated with a given bus number,
+ */
+static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState *s, uint8_t bus_num)
+{
+VTDBus *vtd_bus = s->vtd_as_by_bus_num[bus_num];
+if (!vtd_bus) {
+/* Iterate over the registered buses to find the one
+ * which currently hold this bus number, and update the bus_num lookup 
table:
+ */
+GHashTableIter iter;
+uint64_t key;
+
+g_hash_table_iter_init(&iter, s->vtd_as_by_busptr);
+while (g_hash_table_iter_next (&iter, (void**)&key, (void**)&vtd_bus)) 
{
+if (pci_bus_num(vtd_bus->bus) == bus_num) {
+s->vtd_as_by_bus_num[bus_num] = vtd_bus;
+return vtd_bus;
+}
+}
+}
+return vtd_bus;
+}
+
 /* Do a context-cache device-selective invalidation.
  * @func_mask: FM field after shifting
  */
@@ -882,7 +906,7 @@ static void vtd_context_device_invalidate(IntelIOMMUState 
*s,
   uint16_t func_mask)
 {
 uint16_t mask;
-VTDAddressSpace **pvtd_as;
+VTDBus *vtd_bus;
 VTDAddressSpace *vtd_as;
 uint16_t devfn;
 uint16_t devfn_it;
@@ -903,11 +927,11 @@ static void vtd_context_device_invalidate(IntelIOMMUState 
*s,
 }
 VTD_DPRINTF(INV, "device-selective invalidation source 0x%"PRIx16
 " mask %"PRIu16, source_id, mask);
-pvtd_as = s->address_spaces[VTD_SID_TO_BUS(source_id)];
-if (pvtd_as) {
+vtd_bus = vtd_find_as_from_bus_num(s, VTD_SID_TO_BUS(source_id));
+if (vtd_bus) {
 devfn = VTD_SID_TO_DEVFN(source_id);
 for (devfn_it = 0; devfn_it < VTD_PCI_DEVFN_MAX; ++devfn_it) {
-vtd_as = pvtd_as[devfn_it];
+vtd_as = vtd_bus->dev_as[devfn_it];
 if (vtd_as && ((devfn_it & mask) == (devfn & mask))) {
 VTD_DPRINTF(INV, "invalidate context-cahce of devfn 0x%"PRIx16,
 devfn_it);
@@ -1805,11 +1829,11 @@ static IOMMUTLBEntry vtd_iommu_translate(MemoryRegion 
*iommu, hwaddr addr,
 return ret;
 }
 
-vtd_do_iommu_translate(vtd_as, vtd_as->bus_num, vtd_as->devfn, addr,
+vtd_do_iommu_translate(vtd_as, vtd_as->bus, vtd_as->de

Re: [Qemu-devel] [PATCH] pulseaudio: reduce 24s recording latency

[Volker Rümelin]

Am 12.09.2015 um 16:35 schrieb Kővágó Zoltán:

2015-09-12 13:23 keltezéssel, Volker Rümelin írta:


On start up qemu opens a connection to pulseaudio in function
qpa_init_in and pulseaudio immediately starts recording to the 4MB
ringbuffer. The qemu guest, Windows 8.1 in my case, doesn't consume that
data if there is no process listening on the audio interface. Now if the
guest starts recording, it will see audio data which was recorded 24s 
ago.


Weird, pulseaudio shouldn't delay the input more than 2 seconds in the 
default config.  Maybe PA_STREAM_EARLY_REQUESTS help.  See my patch at 
[1].  Alternatively we should maybe call pa_stream_flush when enabling 
the input to tell pulseaudio to drop previously recorded samples.


[1]: https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02455.html



Hi Zoltan,

I tested your suggestions.

There was the same 24s latency when I replaced PA_STREAM_ADJUST_LATENCY 
with PA_STREAM_EARLY_REQUESTS in function pa_stream_connect_record. This 
was to be expected, because the pulseaudio documentation only mentions 
playback there.


A call to pa_stream_flush in qpa_ctl_in in case VOICE_ENABLE: also 
didn't solve the problem.


But when I replaced pa_stream_flush with a repeated call to 
pa_stream_peek and pa_stream_drop things started to improve. Now I 
experience the 1-2s recording latency mentioned in the pulseaudio 
documentation. But this is still much higher than the latency I see with 
my old patch.


Regards,
Volker

Re: [Qemu-devel] [PATCH 0/4] target-i386: Don't try to enable unsupported TCG features by default

[Paolo Bonzini]

On 11/09/2015 21:58, Eduardo Habkost wrote:
> About implementing DE in TCG: I really don't think it is easier, but if
> somebody wants to implement it, it would be welcome.

Actually I agree that it's easier, and even a partial implementation
(e.g. no I/O port breakpoints) would be nice to have because recent
Windows IIRC requires DE.

Paolo

[Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots

[Guenter Roeck]

The Linux kernel only accepts 34 Khz and 67 Khz clock rates, and
may crash if the actual clock rate is too low. The clock rate used to be
(ps-clk-frequency * 26 / 4), which resulted in a CPU frequency of
21 Khz if ps-clk-frequency was set to  Hz. Change it to
(ps-clk-frequency * 20 / 2) = 33 Khz for to make Linux happy.
Limit the change to Linux boots only.

Signed-off-by: Guenter Roeck 

---
v2: Limit scope of change to Linux boots.

 hw/misc/zynq_slcr.c | 29 +++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index 964f253..ed510fb 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -14,6 +14,7 @@
  * with this program; if not, see .
  */
 
+#include "hw/arm/linux-boot-if.h"
 #include "hw/hw.h"
 #include "qemu/timer.h"
 #include "hw/sysbus.h"
@@ -177,6 +178,8 @@ typedef struct ZynqSLCRState {
 
 MemoryRegion iomem;
 
+bool is_linux;
+
 uint32_t regs[ZYNQ_SLCR_NUM_REGS];
 } ZynqSLCRState;
 
@@ -189,7 +192,11 @@ static void zynq_slcr_reset(DeviceState *d)
 
 s->regs[LOCKSTA] = 1;
 /* 0x100 - 0x11C */
-s->regs[ARM_PLL_CTRL]   = 0x0001A008;
+if (!s->is_linux) {
+s->regs[ARM_PLL_CTRL]   = 0x0001A008;
+} else {
+s->regs[ARM_PLL_CTRL]   = 0x00014008;
+}
 s->regs[DDR_PLL_CTRL]   = 0x0001A008;
 s->regs[IO_PLL_CTRL]= 0x0001A008;
 s->regs[PLL_STATUS] = 0x003F;
@@ -198,7 +205,11 @@ static void zynq_slcr_reset(DeviceState *d)
 s->regs[IO_PLL_CFG] = 0x00014000;
 
 /* 0x120 - 0x16C */
-s->regs[ARM_CLK_CTRL]   = 0x1F000400;
+if (!s->is_linux) {
+s->regs[ARM_CLK_CTRL]   = 0x1F000400;
+} else {
+s->regs[ARM_CLK_CTRL]   = 0x1F000200;
+}
 s->regs[DDR_CLK_CTRL]   = 0x1843;
 s->regs[DCI_CLK_CTRL]   = 0x01E03201;
 s->regs[APER_CLK_CTRL]  = 0x01FFCCCD;
@@ -429,17 +440,27 @@ static const VMStateDescription vmstate_zynq_slcr = {
 .version_id = 2,
 .minimum_version_id = 2,
 .fields = (VMStateField[]) {
+VMSTATE_BOOL(is_linux, ZynqSLCRState),
 VMSTATE_UINT32_ARRAY(regs, ZynqSLCRState, ZYNQ_SLCR_NUM_REGS),
 VMSTATE_END_OF_LIST()
 }
 };
 
+static void zynq_sclr_linux_init(ARMLinuxBootIf *obj, bool secure_boot)
+{
+ZynqSLCRState *s = ZYNQ_SLCR(obj);
+
+s->is_linux = true;
+}
+
 static void zynq_slcr_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
+ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_CLASS(klass);
 
 dc->vmsd = &vmstate_zynq_slcr;
 dc->reset = zynq_slcr_reset;
+albifc->arm_linux_init = zynq_sclr_linux_init;
 }
 
 static const TypeInfo zynq_slcr_info = {
@@ -448,6 +469,10 @@ static const TypeInfo zynq_slcr_info = {
 .parent = TYPE_SYS_BUS_DEVICE,
 .instance_size  = sizeof(ZynqSLCRState),
 .instance_init = zynq_slcr_init,
+.interfaces = (InterfaceInfo []) {
+{ TYPE_ARM_LINUX_BOOT_IF },
+{ },
+},
 };
 
 static void zynq_slcr_register_types(void)
-- 
2.1.4

[Qemu-devel] [PATCH v2] hw/misc: Add support for ADC controller in Xilinx Zynq 7000

[Guenter Roeck]

Add support for the Xilinx XADC core used in Zynq 7000.

References:
- Zynq-7000 All Programmable SoC Technical Reference Manual
- 7 Series FPGAs and Zynq-7000 All Programmable SoC XADC
  Dual 12-Bit 1 MSPS Analog-to-Digital Converter

Tested with Linux using qemu machine xilinx-zynq-a9 with devicetree
files zynq-zc702.dtb and zynq-zc706.dtb, and kernel configuration
multi_v7_defconfig.

Signed-off-by: Guenter Roeck 

---
v2: Use extract32()
Merge zynq_xadc_reset() and _zynq_xadc_reset() into one function
Use "xlnx,zynq_xadc"
Move device model to include/hw/misc/zynq_xadc.h
irq -> qemu_irq
xadc_dfifo_depth -> xadc_dfifo_entries
Dropped unnecessary comments
Merged zynq_xadc_realize() into zynq_xadc_init()

 hw/arm/xilinx_zynq.c|   6 +
 hw/misc/Makefile.objs   |   1 +
 hw/misc/zynq_xadc.c | 270 
 include/hw/misc/zynq_xadc.h |  49 
 4 files changed, 326 insertions(+)
 create mode 100644 hw/misc/zynq_xadc.c
 create mode 100644 include/hw/misc/zynq_xadc.h

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index a4e7b5c..f933f81 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -24,6 +24,7 @@
 #include "hw/block/flash.h"
 #include "sysemu/block-backend.h"
 #include "hw/loader.h"
+#include "hw/misc/zynq_xadc.h"
 #include "hw/ssi.h"
 #include "qemu/error-report.h"
 
@@ -225,6 +226,11 @@ static void zynq_init(MachineState *machine)
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
 sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
 
+dev = qdev_create(NULL, TYPE_ZYNQ_XADC);
+qdev_init_nofail(dev);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]);
+
 dev = qdev_create(NULL, "pl330");
 qdev_prop_set_uint8(dev, "num_chnls",  8);
 qdev_prop_set_uint8(dev, "num_periph_req",  4);
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 4aa76ff..5f76f05 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -36,6 +36,7 @@ obj-$(CONFIG_OMAP) += omap_sdrc.o
 obj-$(CONFIG_OMAP) += omap_tap.o
 obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
+obj-$(CONFIG_ZYNQ) += zynq_xadc.o
 obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
diff --git a/hw/misc/zynq_xadc.c b/hw/misc/zynq_xadc.c
new file mode 100644
index 000..dc67b73
--- /dev/null
+++ b/hw/misc/zynq_xadc.c
@@ -0,0 +1,270 @@
+/*
+ * ADC registers for Xilinx Zynq Platform
+ *
+ * Copyright (c) 2015 Guenter Roeck
+ * Based on hw/misc/zynq_slcr.c, written by Michal Simek
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "hw/hw.h"
+#include "hw/misc/zynq_xadc.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+
+enum {
+CFG= 0x000 / 4,
+INTSTS,
+INTMSK,
+STATUS,
+CFIFO,
+DFIFO,
+CTL,
+};
+
+#define XADC_ZYNQ_CFG_ENABLEBIT(31)
+#define XADC_ZYNQ_CFG_CFIFOTH_RD(x) (((x) >> 20) & 0x0f)
+#define XADC_ZYNQ_CFG_DFIFOTH_RD(x) (((x) >> 16) & 0x0f)
+#define XADC_ZYNQ_CFG_WEDGE BIT(13)
+#define XADC_ZYNQ_CFG_REDGE BIT(12)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV2  (0x0 << 8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV4  (0x1 << 8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV8  (0x2 << 8)
+#define XADC_ZYNQ_CFG_TCKRATE_DIV16 (0x3 << 8)
+#define XADC_ZYNQ_CFG_IGAP_MASK 0x1f
+#define XADC_ZYNQ_CFG_IGAP(x)   ((x) & XADC_ZYNQ_CFG_IGAP_MASK)
+
+#define XADC_ZYNQ_INT_CFIFO_LTH BIT(9)
+#define XADC_ZYNQ_INT_DFIFO_GTH BIT(8)
+#define XADC_ZYNQ_INT_ALARM_MASK0xff
+#define XADC_ZYNQ_INT_ALARM_OFFSET  0
+
+#define XADC_ZYNQ_STATUS_DFIFO_LVL(x)   (((x) & 0x0f) << 12)
+#define XADC_ZYNQ_STATUS_CFIFOF BIT(11)
+#define XADC_ZYNQ_STATUS_CFIFOE BIT(10)
+#define XADC_ZYNQ_STATUS_DFIFOF BIT(9)
+#define XADC_ZYNQ_STATUS_DFIFOE BIT(8)
+#define XADC_ZYNQ_STATUS_OT BIT(7)
+#define XADC_ZYNQ_STATUS_ALM(x) BIT(x)
+
+#define XADC_ZYNQ_CTL_RESET BIT(4)
+
+#define XADC_ZYNQ_CMD_NOP   0x00
+#define XADC_ZYNQ_CMD_READ  0x01
+#define XADC_ZYNQ_CMD_WRITE 0x02
+
+static void zynq_xadc_reset(DeviceState *d)
+{
+ZynqXADCState *s = ZYNQ_XADC(d);
+int i;
+
+s->regs[CFG] = XADC_ZYNQ_CFG_IGAP(0x14) | XADC_ZYNQ_CFG_TCKRATE_DIV4 |
+XADC_ZYNQ_CFG_REDGE;
+s->regs[INTSTS] = XADC_ZYNQ_INT_CFIFO_LTH;
+s->regs[INTMSK] = 0x;
+s->regs[STATUS] = 0;
+s->regs[CFIFO] = 0;
+s->re

Re: [Qemu-devel] [PATCH] linux-user/signal.c: Use setup_rt_frame() instead of setup_frame() for target openrisc

[Chen Gang]

On 9/13/15 00:23, Peter Maydell wrote:
> On 12 September 2015 at 16:32,   wrote:
>> From: Chen Gang 
>>
>> qemu has already considered about some targets may have no traditional
>> signals. And openrisc's setup_frame() is dummy, but it can be supported
>> by setup_rt_frame().
>>
>> Signed-off-by: Chen Gang 
>> ---
>>  linux-user/signal.c | 9 ++---
>>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Peter Maydell 
> 

Thank you very much.

I am just analyzing the raise insn about tilegx, and shall implement the
related code in signal.c, your Reviewed-by is very helpful for it. :-)


Thanks.
-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

[Qemu-devel] [RFC PATCH 3/3] acpi: arm: add fw_cfg device node to dsdt

[Gabriel L. Somlo]

Add a fw_cfg device node to the ACPI DSDT. This is mostly
informational, as the authoritative fw_cfg MMIO region(s)
are listed in the Device Tree. However, since we are building
ACPI tables, we might as well be thorough while at it...

Signed-off-by: Gabriel Somlo 
---

I used surrounding examples to create acpi_dsdt_add_fw_cfg(), and
noticed that none add a _STA method, and many include a 0 _UID even
for nodes with a single instance. I wonder whether 1. I really need
the _UID, and 2. why would we be OK not including a _STA method ?

Is the #2 answer "because no exisging arm OSPM does in fact check,
and/or care about the absence of _STA" ? :)

Thanks,
 --Gabriel

 hw/arm/virt-acpi-build.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9088248..150c9f9 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -110,6 +110,23 @@ static void acpi_dsdt_add_rtc(Aml *scope, const 
MemMapEntry *rtc_memmap,
 aml_append(scope, dev);
 }
 
+static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
+{
+Aml *dev = aml_device("FWCF");
+aml_append(dev, aml_name_decl("_HID", aml_string("FWCF0001")));
+
+/* FIXME: is this necessary ? */
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+/* FIXME: why doesn't a _STA get added to any other node ? */
+aml_append(dev, aml_name_decl("_STA", aml_int(0x0B)));
+
+Aml *crs = aml_resource_template();
+aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
+   fw_cfg_memmap->size, AML_READ_WRITE));
+aml_append(dev, aml_name_decl("_CRS", crs));
+aml_append(scope, dev);
+}
+
 static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
 {
 Aml *dev, *crs;
@@ -519,6 +536,7 @@ build_dsdt(GArray *table_data, GArray *linker, 
VirtGuestInfo *guest_info)
(irqmap[VIRT_UART] + ARM_SPI_BASE));
 acpi_dsdt_add_rtc(scope, &memmap[VIRT_RTC],
   (irqmap[VIRT_RTC] + ARM_SPI_BASE));
+acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
 acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
 acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
 (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
-- 
2.4.3

[Qemu-devel] [RFC PATCH 0/3] adding acpi node for fw_cfg on pc and arm

[Gabriel L. Somlo]

This series adds a fw_cfg device node to the SSDT (on pc), or to the
DSDT (on arm).

- Patch 1/3 moves (and renames) the BIOS_CFG_IOPORT (0x510)
  define from pc.c to pc.h, so that it could be used from
  acpi-build.c in patch 2/3.

- Patch 2/3 adds a fw_cfg node to the pc SSDT.

- Patch 3/3 adds a fw_cfg node to the arm DSDT.

I made up some names - "FWCF" for the node name, and "FWCF0001"
for _HID; no idea whether that's appropriate, or how else I should
figure out what to use instead...

Also, using scope "\\_SB", based on where fw_cfg shows up in the
output of "info qtree". Again, if that's wrong, please point me in
the right direction.

Re. 3/3 (also mentioned after the commit blurb in the patch itself),
I noticed none of the other DSDT entries contain a _STA field, wondering
why it would (not) make sense to include that, same as on the PC.

TIA for any feedback, comments, reviews, etc.
 --Gabriel

Gabriel L. Somlo (3):
  pc: fw_cfg: move ioport base constant to pc.h
  acpi: pc: add fw_cfg device node to ssdt
  acpi: arm: add fw_cfg device node to dsdt

 hw/arm/virt-acpi-build.c | 18 ++
 hw/i386/acpi-build.c | 19 +++
 hw/i386/pc.c |  5 ++---
 include/hw/i386/pc.h |  3 +++
 4 files changed, 42 insertions(+), 3 deletions(-)

-- 
2.4.3

[Qemu-devel] [RFC PATCH 2/3] acpi: pc: add fw_cfg device node to ssdt

[Gabriel L. Somlo]

Add a fw_cfg device node to the ACPI SSDT. While the guest-side
BIOS can't utilize this information (since it has to access the
hard-coded fw_cfg device to extract ACPI tables to begin with),
having fw_cfg listed in ACPI will help the guest kernel keep a
more accurate inventory of in-use IO port regions.

Signed-off-by: Gabriel Somlo 
---
 hw/i386/acpi-build.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 95e0c65..9d0ec22 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1071,6 +1071,25 @@ build_ssdt(GArray *table_data, GArray *linker,
 aml_append(scope, aml_name_decl("_S5", pkg));
 aml_append(ssdt, scope);
 
+if (guest_info->fw_cfg) {
+scope = aml_scope("\\_SB");
+dev = aml_device("FWCF");
+
+aml_append(dev, aml_name_decl("_HID", aml_string("FWCF0001")));
+/* device present, functioning, decoding, not shown in UI */
+aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+
+crs = aml_resource_template();
+aml_append(crs,
+aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE,
+   0x01, FW_CFG_IO_SIZE)
+);
+aml_append(dev, aml_name_decl("_CRS", crs));
+
+aml_append(scope, dev);
+aml_append(ssdt, scope);
+}
+
 if (misc->applesmc_io_base) {
 scope = aml_scope("\\_SB.PCI0.ISA");
 dev = aml_device("SMC");
-- 
2.4.3

[Qemu-devel] [RFC PATCH 1/3] pc: fw_cfg: move ioport base constant to pc.h

[Gabriel L. Somlo]

Move BIOS_CFG_IOPORT define from pc.c to pc.h, and rename
it to FW_CFG_IO_BASE. Also, add FW_CFG_IO_SIZE define (set
to 0x02, to cover the overlapping 16-bit control and 8-bit
data ports).

Signed-off-by: Gabriel Somlo 
---
 hw/i386/pc.c | 5 ++---
 include/hw/i386/pc.h | 3 +++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b5107f7..1a92b4f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -86,7 +86,6 @@ void pc_set_legacy_acpi_data_size(void)
 acpi_data_size = 0x1;
 }
 
-#define BIOS_CFG_IOPORT 0x510
 #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
 #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
 #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
@@ -760,7 +759,7 @@ static FWCfgState *bochs_bios_init(void)
 int i, j;
 unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
-fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+fw_cfg = fw_cfg_init_io(FW_CFG_IO_BASE);
 /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
  *
  * SeaBIOS needs FW_CFG_MAX_CPUS for CPU hotplug, but the CPU hotplug
@@ -1292,7 +1291,7 @@ FWCfgState *xen_load_linux(PCMachineState *pcms,
 
 assert(MACHINE(pcms)->kernel_filename != NULL);
 
-fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+fw_cfg = fw_cfg_init_io(FW_CFG_IO_BASE);
 rom_set_fw(fw_cfg);
 
 load_linux(pcms, fw_cfg);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 3e002c9..0cab3c5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -206,6 +206,9 @@ typedef void (*cpu_set_smm_t)(int smm, void *arg);
 
 void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
+#define FW_CFG_IO_BASE 0x510
+#define FW_CFG_IO_SIZE  0x02
+
 /* acpi_piix.c */
 
 I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 0/2] intel_iommu: Add support for translation for devices behind bridges

[Benjamin Herrenschmidt]

On Sat, 2015-09-12 at 20:37 +0200, Knut Omang wrote:
> As the thread went silent after our conclusions, I have made a second
> implementation for the Intel IOMMU according to this alternate scheme,
> It keeps the current API and handles the bus number resolution lazily
> within the IOMMU implementation, I will post the (single) patch as v3
> of this. 
> 
> Hopefully this is acceptable and can be leveraged to do a similar
> rework, or be abstracted as generic functionality (?) for the other
> architectures,..

Ah sorry, I meant to look at your email in more details and respond but
it fell through the cracks.

I'm happy to have a look at your work and see how it applies to me, you
can see my powernv code which also supports translation for devices
behind bridges here (but doesn't do as much caching as q35 does):

https://github.com/ozbenh/qemu/commit/4e0ed1002f98fd97aa7ca3a48c74933d0343dd42

Which depends on:

https://github.com/ozbenh/qemu/commit/facedeba8811985ca20ac3dbad5d07e1a10ea9b2

(Which I think Michael merged recently, I haven't checked).

Cheers,
Ben.

[Qemu-devel] ping: [PATCH v5] raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Programmingkid]

> Mac OS X can be picky when it comes to allowing the user to use physical 
> devices
> in QEMU. Most mounted volumes appear to be off limits to QEMU. If an issue is
> detected, a message is displayed showing the user how to unmount a volume.
> 
> Signed-off-by: John Arbuckle 
> 
> ---
> Removed changes to GetBSDPath() to a separate patch.
> This patch now depends on the GetBSDPath patch.
> 
>  block/raw-posix.c |   90 +---
>  1 files changed, 64 insertions(+), 26 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 67d1d48..a41006f 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -42,9 +42,8 @@
>  #include 
>  #include 
>  #include 
> -//#include 
>  #include 
> -#endif
> +#endif /* (__APPLE__) && (__MACH__) */
>  
>  #ifdef __sun__
>  #define _POSIX_PTHREAD_SEMANTICS 1
> @@ -1972,7 +1971,7 @@ BlockDriver bdrv_file = {
>  /* host device */
>  
>  #if defined(__APPLE__) && defined(__MACH__)
> -static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator );
> +static kern_return_t FindEjectableCDMedia(io_iterator_t *mediaIterator);
>  static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
>  CFIndex maxPathSize, int flags);
>  kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator )
> @@ -2030,7 +2029,34 @@ kern_return_t GetBSDPath(io_iterator_t mediaIterator, 
> char *bsdPath,
>  return kernResult;
>  }
>  
> -#endif
> +/* Sets up a real cdrom for use in QEMU */
> +static bool setupCDROM(char *bsdPath)
> +{
> +int index, numOfTestPartitions = 2, fd;
> +char testPartition[MAXPATHLEN];
> +bool partitionFound = false;
> +
> +/* look for a working partition */
> +for (index = 0; index < numOfTestPartitions; index++) {
> +snprintf(testPartition, sizeof(testPartition), "%ss%d", bsdPath, 
> index);
> +fd = qemu_open(testPartition, O_RDONLY | O_BINARY | O_LARGEFILE);
> +if (fd >= 0) {
> +partitionFound = true;
> +qemu_close(fd);
> +break;
> +}
> +}
> +
> +/* if a working partition on the device was not found */
> +if (partitionFound == false) {
> +printf("Error: Failed to find a working partition on disc!\n");
> +} else {
> +DPRINTF("Using %s as optical disc\n", testPartition);
> +pstrcpy(bsdPath, MAXPATHLEN, testPartition);
> +}
> +return partitionFound;
> +}
> +#endif /* defined(__APPLE__) && defined(__MACH__) */
>  
>  static int hdev_probe_device(const char *filename)
>  {
> @@ -2118,34 +2144,32 @@ static int hdev_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  BDRVRawState *s = bs->opaque;
>  Error *local_err = NULL;
>  int ret;
> +bool cdromOK = true;
>  
>  #if defined(__APPLE__) && defined(__MACH__)
>  const char *filename = qdict_get_str(options, "filename");
>  
> -if (strstart(filename, "/dev/cdrom", NULL)) {
> -kern_return_t kernResult;
> -io_iterator_t mediaIterator;
> -char bsdPath[ MAXPATHLEN ];
> -int fd;
> -
> -kernResult = FindEjectableCDMedia( &mediaIterator );
> -kernResult = GetBSDPath(mediaIterator, bsdPath, sizeof(bsdPath), 
> -
> flags);
> -if ( bsdPath[ 0 ] != '\0' ) {
> -strcat(bsdPath,"s0");
> -/* some CDs don't have a partition 0 */
> -fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
> -if (fd < 0) {
> -bsdPath[strlen(bsdPath)-1] = '1';
> +/* If using a physical device */
> +if (strstart(filename, "/dev/", NULL)) {
> +char bsdPath[MAXPATHLEN];
> +
> +/* If the physical device is a cdrom */
> +if (strcmp(filename, "/dev/cdrom") == 0) {
> +io_iterator_t mediaIterator;
> +FindEjectableCDMedia(&mediaIterator);
> +GetBSDPath(mediaIterator, bsdPath, sizeof(bsdPath), flags);
> +if (bsdPath[0] == '\0') {
> +printf("Error: failed to obtain bsd path for optical 
> drive!\n");
>  } else {
> -qemu_close(fd);
> +cdromOK = setupCDROM(bsdPath);
> +filename = bsdPath;
>  }
> -filename = bsdPath;
> -qdict_put(options, "filename", qstring_from_str(filename));
> -}
>  
> -if ( mediaIterator )
> -IOObjectRelease( mediaIterator );
> +if (mediaIterator) {
> +IOObjectRelease(mediaIterator);
> +}
> +}
> +qdict_put(options, "filename", qstring_from_str(filename));
>  }
>  #endif
>  
> @@ -2156,7 +2180,21 @@ static int hdev_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  if (local_err) {
>  error_propagate(errp, local_err);
>  }
> -return ret;
> +}
> +
> +#

[Qemu-devel] Python problem

[Programmingkid]

After a 'git pull' on 9/12/2015, I noticed that QEMU no longer builds on Mac OS 
10.6. I think there is a problem with a python script. This is the error 
message: 

  GEN   qmp-commands.h
Traceback (most recent call last):
  File "/scripts/qapi-commands.py", line 352, in 
ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + 
"\n"

I'm using Python 2.6.1. 

Re: [Qemu-devel] Python problem

[Eric Blake]

On 09/12/2015 07:57 PM, Programmingkid wrote:
> After a 'git pull' on 9/12/2015, I noticed that QEMU no longer builds on Mac 
> OS 10.6. I think there is a problem with a python script. This is the error 
> message: 
> 
>   GEN   qmp-commands.h
> Traceback (most recent call last):
>   File "/scripts/qapi-commands.py", line 352, in 
> ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + 
> "\n"
> 
> I'm using Python 2.6.1. 

This issue has been reported multiple times:

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02329.html

and the patch will hit the tree soon.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Confused by QOM: /machine/unattached/device[5]/dr-connector[255]/fdt

[Eric Blake]

On 09/09/2015 10:16 AM, Paolo Bonzini wrote:

>>> Is returning NULL without setting an error okay?
>>>
>>> Should it return qnull() instead?  Then the QMP return value would be
>>> JSON null.
> 
> JSON null support in QObject is new, it should be the result of
> object_property_get_qobject() or even qmp_output_get_qobject().  Needs
> auditing the code.

Yes, returning QNull rather than NULL seems like the right approach.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Python problem

[Programmingkid]

On Sep 12, 2015, at 10:04 PM, Eric Blake wrote:

> On 09/12/2015 07:57 PM, Programmingkid wrote:
>> After a 'git pull' on 9/12/2015, I noticed that QEMU no longer builds on Mac 
>> OS 10.6. I think there is a problem with a python script. This is the error 
>> message: 
>> 
>>  GEN   qmp-commands.h
>> Traceback (most recent call last):
>>  File "/scripts/qapi-commands.py", line 352, in 
>>ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + 
>> "\n"
>> 
>> I'm using Python 2.6.1. 
> 
> This issue has been reported multiple times:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02329.html
> 
> and the patch will hit the tree soon.
> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

Excellent. This fixed the problem. Thank you very much. The minimum version of 
python QEMU
supports is 2.6?

[Qemu-devel] [PATCH] linux-user/signal.c: Skip calling unlock_user_struct() when lock_user_struct() failed for target m68k

[gang . chen . 5i5j]

From: Chen Gang 

For target m68k, setup_rt_frame() and do_rt_sigreturn() have this issue.

Signed-off-by: Chen Gang 
---
 linux-user/signal.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index cead97b..0265c46 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -5160,7 +5160,7 @@ static void setup_rt_frame(int sig, struct 
target_sigaction *ka,
 
 frame_addr = get_sigframe(ka, env, sizeof *frame);
 if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0))
-   goto give_sigsegv;
+goto err;
 
 __put_user(sig, &frame->sig);
 
@@ -5215,6 +5215,7 @@ static void setup_rt_frame(int sig, struct 
target_sigaction *ka,
 
 give_sigsegv:
 unlock_user_struct(frame, frame_addr, 1);
+err:
 force_sig(TARGET_SIGSEGV);
 }
 
@@ -5261,7 +5262,7 @@ long do_rt_sigreturn(CPUM68KState *env)
 int d0;
 
 if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
-goto badframe;
+goto err;
 
 target_to_host_sigset_internal(&set, &target_set);
 do_sigprocmask(SIG_SETMASK, &set, NULL);
@@ -5281,6 +5282,7 @@ long do_rt_sigreturn(CPUM68KState *env)
 
 badframe:
 unlock_user_struct(frame, frame_addr, 0);
+err:
 force_sig(TARGET_SIGSEGV);
 return 0;
 }
-- 
1.9.3

[Qemu-devel] [PATCH] linux-user/signal.c: Skip calling unlock_user_struct() when lock_user_struct() failed for target ppc and ppc64

[gang . chen . 5i5j]

From: Chen Gang 

For target ppc and ppc64, all related funcitons have this issue.

Signed-off-by: Chen Gang 
---
 linux-user/signal.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 0265c46..61f98e7 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -4666,7 +4666,7 @@ static void setup_frame(int sig, struct target_sigaction 
*ka,
 
 frame_addr = get_sigframe(ka, env, sizeof(*frame));
 if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 1))
-goto sigsegv;
+goto err;
 sc = &frame->sctx;
 
 __put_user(ka->_sa_handler, &sc->handler);
@@ -4729,6 +4729,7 @@ static void setup_frame(int sig, struct target_sigaction 
*ka,
 
 sigsegv:
 unlock_user_struct(frame, frame_addr, 1);
+err:
 qemu_log("segfaulting from setup_frame\n");
 force_sig(TARGET_SIGSEGV);
 }
@@ -4748,7 +4749,7 @@ static void setup_rt_frame(int sig, struct 
target_sigaction *ka,
 
 rt_sf_addr = get_sigframe(ka, env, sizeof(*rt_sf));
 if (!lock_user_struct(VERIFY_WRITE, rt_sf, rt_sf_addr, 1))
-goto sigsegv;
+goto err;
 
 tswap_siginfo(&rt_sf->info, info);
 
@@ -4825,6 +4826,7 @@ static void setup_rt_frame(int sig, struct 
target_sigaction *ka,
 
 sigsegv:
 unlock_user_struct(rt_sf, rt_sf_addr, 1);
+err:
 qemu_log("segfaulting from setup_rt_frame\n");
 force_sig(TARGET_SIGSEGV);
 
@@ -4840,7 +4842,7 @@ long do_sigreturn(CPUPPCState *env)
 
 sc_addr = env->gpr[1] + SIGNAL_FRAMESIZE;
 if (!lock_user_struct(VERIFY_READ, sc, sc_addr, 1))
-goto sigsegv;
+goto err;
 
 #if defined(TARGET_PPC64)
 set.sig[0] = sc->oldmask + ((uint64_t)(sc->_unused[3]) << 32);
@@ -4861,8 +4863,8 @@ long do_sigreturn(CPUPPCState *env)
 return -TARGET_QEMU_ESIGRETURN;
 
 sigsegv:
-unlock_user_struct(sr, sr_addr, 1);
 unlock_user_struct(sc, sc_addr, 1);
+err:
 qemu_log("segfaulting from do_sigreturn\n");
 force_sig(TARGET_SIGSEGV);
 return 0;
@@ -4905,7 +4907,7 @@ long do_rt_sigreturn(CPUPPCState *env)
 
 rt_sf_addr = env->gpr[1] + SIGNAL_FRAMESIZE + 16;
 if (!lock_user_struct(VERIFY_READ, rt_sf, rt_sf_addr, 1))
-goto sigsegv;
+goto err;
 
 if (do_setcontext(&rt_sf->uc, env, 1))
 goto sigsegv;
@@ -4919,6 +4921,7 @@ long do_rt_sigreturn(CPUPPCState *env)
 
 sigsegv:
 unlock_user_struct(rt_sf, rt_sf_addr, 1);
+err:
 qemu_log("segfaulting from do_rt_sigreturn\n");
 force_sig(TARGET_SIGSEGV);
 return 0;
-- 
1.9.3