Re: [Qemu-devel] [PATCH] riscv: Add proper alignment check and pending 'C' extension upon misa writes

[Amed Magdy]

Thank you for your review and feedback, Richard.
As Eric mentioned, this is the first time contribution. I have been
exploring Qemu for some time and try to understand main flow, internals,
..etc.

>  You cannot manipulate env like this during translation.

>  Neither the write to env->pc_next nor the read from env->pending_rvc
here will
>  be in any synchronization with the execution of write_misa.

Does this applies for translated code in a single translation block only or
for different TBs also ?

So should I manipulate "env" from translation context through helpers only
? for example:

TCGv temp;
tcg_gen_movi_tl(temp, ctx->pc_succ_insn);
gen_helper_next_pc(cpu_env, temp);

while the helper function definition like that:
void helper_next_pc(CPURISCVState *env, target_ulong pc_next)
{
env->pc_next = pc_next;
}

and the same to read "env->pending_rvc"

I'm thinking of a way to add 'C' extension at run time through waiting the
correct aligned instruction, which I believe it might be after branch
something quite similar to switching between ARM and THUMB states in ARM,
for misa 'RVC' enable to take effect since it will be no possibility to
check alignment with 'C' extension.

Thanks,
Ahmed


On Sat, 23 Feb 2019 at 01:57, Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 2/22/19 8:25 AM, amagdy.af...@gmail.com wrote:
> > @@ -373,9 +373,10 @@ static int write_misa(CPURISCVState *env, int
> csrno, target_ulong val)
> >  }
> >
> >  /* Suppress 'C' if next instruction is not aligned
> > -   TODO: this should check next_pc */
> > -if ((val & RVC) && (GETPC() & ~3) != 0) {
> > +   check next target pc */
> > +if ((val & RVC) && (env->pc_next & 3) != 0) {
> >  val &= ~RVC;
> > +env->pending_rvc = 1;
> >  }
> >
> >  /* misa.MXL writes are not supported by QEMU */
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index 2321bba..c9d84ea 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -1999,20 +1999,26 @@ static void decode_RV32_64G(DisasContext *ctx)
> >  }
> >  }
> >
> > -static void decode_opc(DisasContext *ctx)
> > +static void decode_opc(DisasContext *ctx, CPUState *cpu)
> >  {
> > +CPURISCVState *env = cpu->env_ptr;
> >  /* check for compressed insn */
> >  if (extract32(ctx->opcode, 0, 2) != 3) {
> >  if (!has_ext(ctx, RVC)) {
> >  gen_exception_illegal(ctx);
> >  } else {
> > -ctx->pc_succ_insn = ctx->base.pc_next + 2;
> > +env->pc_next = ctx->pc_succ_insn = ctx->base.pc_next + 2;
> >  decode_RV32_64C(ctx);
> >  }
> >  } else {
> > -ctx->pc_succ_insn = ctx->base.pc_next + 4;
> > +env->pc_next = ctx->pc_succ_insn = ctx->base.pc_next + 4;
> >  decode_RV32_64G(ctx);
> >  }
> > +/* check pending RVC */
> > +if (env->pending_rvc && ((env->pc_next & 3) != 0)) {
> > +env->misa |= RVC;
> > +env->pending_rvc = 0;
>
> You cannot manipulate env like this during translation.
>
> Neither the write to env->pc_next nor the read from env->pending_rvc here
> will
> be in any synchronization with the execution of write_misa.
>
> What semantics are you attempting to implement wrt setting/clearing RVC
> from MISA?
>
> > @@ -2061,7 +2067,7 @@ static void riscv_tr_translate_insn
> >  CPURISCVState *env = cpu->env_ptr;
> >
> >  ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next);
> > -decode_opc(ctx);
> > +decode_opc(ctx, cpu);
>
> This is exactly the reason why cpu is *not* passed down to decode_opc, so
> that
> you cannot make this kind of mistake.
>
>
> r~
>

Re: [Qemu-devel] [PULL 00/26] pci, pc, virtio: fixes, cleanups, tests

[Michael S. Tsirkin]

On Fri, Feb 22, 2019 at 10:53:54AM -0500, Michael S. Tsirkin wrote:
> On Fri, Feb 22, 2019 at 03:47:36PM +, Peter Maydell wrote:
> > On Fri, 22 Feb 2019 at 02:40, Michael S. Tsirkin  wrote:
> > >
> > > The following changes since commit 
> > > fc3dbb90f2eb069801bfb4cfe9cbc83cf9c5f4a9:
> > >
> > >   Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' 
> > > into staging (2019-02-21 13:09:33 +)
> > >
> > > are available in the Git repository at:
> > >
> > >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> > >
> > > for you to fetch changes up to 1f8c04f18d2ee2f6ec88217dfd547ab38d2be5c5:
> > >
> > >   pci: Sanity test minimum downstream LNKSTA (2019-02-21 12:28:41 -0500)
> > >
> > > 
> > > pci, pc, virtio: fixes, cleanups, tests
> > >
> > > Lots of work on tests: BiosTablesTest UEFI app,
> > > vhost-user testing for non-Linux hosts.
> > > Misc cleanups and fixes all over the place
> > >
> > > Signed-off-by: Michael S. Tsirkin 
> > >
> > > 
> > 
> > Compile failure on clang:
> > 
> > /home/petmay01/linaro/qemu-for-merges/hw/virtio/virtio-balloon.c:40:3:
> > error: redefinition of typedef 'PartiallyBalloonedPage' is a C11
> > feature [-Werror,-Wtypedef-redefinition]
> > } PartiallyBalloonedPage;
> >   ^
> > /home/petmay01/linaro/qemu-for-merges/include/hw/virtio/virtio-balloon.h:33:39:
> > note: previous definition is here
> > typedef struct PartiallyBalloonedPage PartiallyBalloonedPage;
> >   ^
> > 1 error generated.
> > /home/petmay01/linaro/qemu-for-merges/rules.mak:69: recipe for target
> > 'hw/virtio/virtio-balloon.o' failed
> > 
> > thanks
> > -- PMM
> 
> Fixed up and re-pushed.

Peter, can you merge for_upstream now pls? Don't want to spam
the list with a trivial change like that ...

> David, pls note above and don't add duplicate typedefs in the future.
> There's always include/qemu/typedefs.h if you don't know where
> to put a typedef.
> 
> -- 
> MST

Re: [Qemu-devel] [PATCH v6 1/7] vhost-user: Support transferring inflight buffer between qemu and backend

[Michael S. Tsirkin]

On Sat, Feb 23, 2019 at 09:10:01PM +0800, Yongji Xie wrote:
> On Fri, 22 Feb 2019 at 22:54, Michael S. Tsirkin  wrote:
> >
> > On Fri, Feb 22, 2019 at 03:05:23PM +0800, Yongji Xie wrote:
> > > On Fri, 22 Feb 2019 at 14:21, Michael S. Tsirkin  wrote:
> > > >
> > > > On Fri, Feb 22, 2019 at 10:47:03AM +0800, Yongji Xie wrote:
> > > > > > > +
> > > > > > > +To track inflight I/O, the queue region should be processed as 
> > > > > > > follows:
> > > > > > > +
> > > > > > > +When receiving available buffers from the driver:
> > > > > > > +
> > > > > > > +1. Get the next available head-descriptor index from 
> > > > > > > available ring, i
> > > > > > > +
> > > > > > > +2. Set desc[i].inflight to 1
> > > > > > > +
> > > > > > > +When supplying used buffers to the driver:
> > > > > > > +
> > > > > > > +1. Get corresponding used head-descriptor index, i
> > > > > > > +
> > > > > > > +2. Set desc[i].next to process_head
> > > > > > > +
> > > > > > > +3. Set process_head to i
> > > > > > > +
> > > > > > > +4. Steps 1,2,3 may be performed repeatedly if batching is 
> > > > > > > possible
> > > > > > > +
> > > > > > > +5. Increase the idx value of used ring by the size of the 
> > > > > > > batch
> > > > > > > +
> > > > > > > +6. Set the inflight field of each DescStateSplit entry in 
> > > > > > > the batch to 0
> > > > > > > +
> > > > > > > +7. Set used_idx to the idx value of used ring
> > > > > > > +
> > > > > > > +When reconnecting:
> > > > > > > +
> > > > > > > +1. If the value of used_idx does not match the idx value of 
> > > > > > > used ring,
> > > > > > > +
> > > > > > > +(a) Subtract the value of used_idx from the idx value of 
> > > > > > > used ring to get
> > > > > > > +the number of in-progress DescStateSplit entries
> > > > > > > +
> > > > > > > +(b) Set the inflight field of the in-progress 
> > > > > > > DescStateSplit entries which
> > > > > > > +start from process_head to 0
> > > > > > > +
> > > > > > > +(c) Set used_idx to the idx value of used ring
> > > > > > > +
> > > > > > > +2. Resubmit each inflight DescStateSplit entry
> > > > > >
> > > > > > I re-read a couple of time and I still don't understand what it 
> > > > > > says.
> > > > > >
> > > > > > For simplicity consider split ring. So we want a list of heads that 
> > > > > > are
> > > > > > outstanding. Fair enough. Now device finishes a head. What now? I 
> > > > > > needs
> > > > > > to drop head from the list. But list is unidirectional (just next, 
> > > > > > no
> > > > > > prev). So how can you drop an entry from the middle?
> > > > > >
> > > > >
> > > > > The process_head is only used when slave crash between increasing the
> > > > > idx value of used ring and updating used_idx. We use it to find the
> > > > > in-progress DescStateSplit entries before the crash and complete them
> > > > > when reconnecting. Make sure guest and slave have the same view for
> > > > > inflight I/Os.
> > > > >
> > > >
> > > > But I don't understand how does the described process help do it?
> > > >
> > >
> > > For example, we need to submit descriptors A, B, C to driver in a batch.
> > >
> > > Firstly, we will link those descriptors like:
> > >
> > > process_head->A->B->C(A)
> > >
> > > Then, we need to update idx value of used vring to mark those
> > > descriptors as used:
> > >
> > > _vring.used->idx += 3(B)
> > >
> > > At last, clear the inflight field of those descriptors and update
> > > used_idx field:
> > >
> > > A.inflight = 0; B.inflight = 0; C.inflight = 0;(C)
> > >
> > > used_idx = _vring.used->idx;(D)
> > >
> > > After (B), guest can consume the descriptors A,B,C. So we must make
> > > sure the inflight field of A,B,C is cleared when reconnecting to avoid
> > > re-submitting used descriptor. If slave crash during (C), the inflight
> > > field of A,B,C may be incorrect. To detect that case, we can see
> > > whether used_idx matches _vring.used->idx. And through process_head,
> > > we can get the in-progress descriptors A,B,C and clear their inflight
> > > field again when reconnecting.
> > >
> > > >
> > > > > In other case, the inflight field is enough to track inflight I/O.
> > > > > When reconnecting, we go through all DescStateSplit entries and
> > > > > re-submit the entry whose inflight field is equal to 1.
> > > >
> > > > What I don't understand is how do we know the order
> > > > in which they have to be resubmitted. Reordering
> > > > operations would be a big problem, won't it?
> > > >
> > >
> > > In previous patch, I record avail_idx for each DescStateSplit entry to
> > > preserve the order. Is it useful to fix this?
> > >
> > > >
> > > > Let's say I fetch descriptors A, B, C and start
> > > > processing. how does memory look?
> > >
> > > A.inflight = 1, C.inflight = 1, B.inflight = 1
> > >
> > > > Now I finished B and marked it used. How does
> > > > memory look?
> > > >
> > >
> > > A.inflight = 1, C.inflight = 1, B.inflight = 0, 

Re: [Qemu-devel] [PATCH 5/5] decodetree: Allow grouping of overlapping patterns

[Philippe Mathieu-Daudé]

On 2/24/19 12:29 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
> ---
>  docs/decodetree.rst   |  58 +
>  scripts/decodetree.py | 144 ++
>  2 files changed, 191 insertions(+), 11 deletions(-)
> 
> diff --git a/docs/decodetree.rst b/docs/decodetree.rst
> index d9be30b2db..391069c105 100644
> --- a/docs/decodetree.rst
> +++ b/docs/decodetree.rst
> @@ -154,3 +154,61 @@ which will, in part, invoke::
>  and::
>  
>trans_addl_i(ctx, _opi, insn)
> +
> +Pattern Groups
> +==
> +
> +Syntax::
> +
> +  group:= '{' ( pat_def | group )+ '}'
> +
> +A *group* begins with a lone open-brace, with all subsequent lines
> +indented two spaces, and ending with a lone close-brace.  Groups
> +may be nested, increasing the required indentation of the lines
> +within the nested group to two spaces per nesting level.
> +
> +Unlike ungrouped patterns, grouped patterns are allowed to overlap.
> +Conflicts are resolved by selecting the patterns in order.  If all
> +of the fixedbits for a pattern match, its translate function will
> +be called.  If the translate function returns false, then subsequent
> +patterns within the group will be matched.

Excellent! The tibetan cuisine inspired you :P

> +
> +The following example from PA-RISC shows specialization of the *or*
> +instruction::
> +
> +  {
> +{
> +  nop   10 - -  001001 0 0
> +  copy  10 0 r1:5   001001 0 rt:5
> +}
> +or  10 r2:5  r1:5  cf:4 001001 0 rt:5
> +  }
> +
> +When the *cf* field is zero, the instruction has no side effects,
> +and may be specialized.  When the *rt* field is zero, the output
> +is discarded and so the instruction has no effect.  When the *rt2*

*r2*?

> +field is zero, the operation is ``reg[rt] | 0`` and so encodes
> +the canonical register copy operation.
> +
> +The output from the generator might look like::
> +
> +  switch (insn & 0xfc000fe0) {
> +  case 0x08000240:
> +/* 10..  0010 010. */
> +if ((insn & 0xf000) == 0x) {
> +/* 10..  0010 010. */
> +if ((insn & 0x001f) == 0x) {
> +/* 10..  0010 0100 */
> +extract_decode_Fmt_0(_decode0, insn);
> +if (trans_nop(ctx, _decode0)) return true;
> +}
> +if ((insn & 0x03e0) == 0x) {
> +/* 1000 000. 0010 010. */
> +extract_decode_Fmt_1(_decode1, insn);
> +if (trans_copy(ctx, _decode1)) return true;
> +}
> +}
> +extract_decode_Fmt_2(_decode2, insn);
> +if (trans_or(ctx, _decode2)) return true;
> +return false;
> +  }
> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
> index dd495096fc..abce58ed8f 100755
> --- a/scripts/decodetree.py
> +++ b/scripts/decodetree.py
> @@ -31,6 +31,7 @@ fields = {}
>  arguments = {}
>  formats = {}
>  patterns = []
> +allpatterns = []
>  
>  translate_prefix = 'trans'
>  translate_scope = 'static '
> @@ -353,6 +354,46 @@ class Pattern(General):
>  # end Pattern
>  
>  
> +class MultiPattern(General):
> +"""Class representing an overlapping set of instruction patterns"""
> +
> +def __init__(self, lineno, pats, fixb, fixm, udfm):
> +self.lineno = lineno
> +self.pats = pats
> +self.base = None
> +self.fixedbits = fixb
> +self.fixedmask = fixm
> +self.undefmask = udfm
> +
> +def __str__(self):
> +r = "{"
> +for p in self.pats:
> +   r = r + ' ' + str(p)
> +return r + "}"
> +
> +def output_decl(self):
> +for p in self.pats:
> +p.output_decl()
> +
> +def output_code(self, i, extracted, outerbits, outermask):
> +global translate_prefix
> +ind = str_indent(i)
> +for p in self.pats:
> +if outermask != p.fixedmask:
> +innermask = p.fixedmask & ~outermask
> +innerbits = p.fixedbits & ~outermask
> +output(ind, 'if ((insn & ',
> +   '0x{0:08x}) == 0x{1:08x}'.format(innermask, 
> innerbits),
> +   ') {\n')
> +output(ind, '/* ',
> +   str_match_bits(p.fixedbits, p.fixedmask), ' */\n')
> +p.output_code(i + 4, extracted, p.fixedbits, p.fixedmask)
> +output(ind, '}\n')
> +else:
> +p.output_code(i, extracted, p.fixedbits, p.fixedmask)
> +#end MultiPattern
> +
> +
>  def parse_field(lineno, name, toks):
>  """Parse one instruction field from TOKS at LINENO"""
>  global fields
> @@ -505,6 +546,7 @@ def parse_generic(lineno, is_format, name, toks):
>  global arguments
>  global formats
>  global patterns
> +global allpatterns
>  global re_ident
>  global insnwidth
>  global insnmask
> @@ -649,6 +691,7 @@ def 

Re: [Qemu-devel] [PATCH 4/5] decodetree: Do not unconditionaly return from Pattern.output_code

[Philippe Mathieu-Daudé]

On 2/24/19 12:29 AM, Richard Henderson wrote:
> As a consequence, the 'return false' gets pushed up one level.
> 
> This will allow us to perform some other action when the
> translator returns failure.
> 
> Signed-off-by: Richard Henderson 

Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  scripts/decodetree.py | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
> index fb9a0ab3ad..dd495096fc 100755
> --- a/scripts/decodetree.py
> +++ b/scripts/decodetree.py
> @@ -348,8 +348,8 @@ class Pattern(General):
>  output(ind, self.base.extract_name(), '(_', arg, ', 
> insn);\n')
>  for n, f in self.fields.items():
>  output(ind, 'u.f_', arg, '.', n, ' = ', f.str_extract(), ';\n')
> -output(ind, 'return ', translate_prefix, '_', self.name,
> -   '(ctx, _', arg, ');\n')
> +output(ind, 'if (', translate_prefix, '_', self.name,
> +   '(ctx, _', arg, ')) return true;\n')
>  # end Pattern
>  
>  
> @@ -777,8 +777,8 @@ class Tree:
>  output(ind, '/* ',
> str_match_bits(innerbits, innermask), ' */\n')
>  s.output_code(i + 4, extracted, innerbits, innermask)
> +output(ind, 'return false;\n')
>  output(ind, '}\n')
> -output(ind, 'return false;\n')
>  # end Tree
>  
>  
> @@ -932,6 +932,7 @@ def main():
>  output(i4, '} u;\n\n')
>  
>  t.output_code(4, False, 0, 0)
> +output(i4, 'return false;\n')
>  
>  output('}\n')
>  
> 

[Qemu-devel] [PATCH 5/5] decodetree: Allow grouping of overlapping patterns

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 docs/decodetree.rst   |  58 +
 scripts/decodetree.py | 144 ++
 2 files changed, 191 insertions(+), 11 deletions(-)

diff --git a/docs/decodetree.rst b/docs/decodetree.rst
index d9be30b2db..391069c105 100644
--- a/docs/decodetree.rst
+++ b/docs/decodetree.rst
@@ -154,3 +154,61 @@ which will, in part, invoke::
 and::
 
   trans_addl_i(ctx, _opi, insn)
+
+Pattern Groups
+==
+
+Syntax::
+
+  group:= '{' ( pat_def | group )+ '}'
+
+A *group* begins with a lone open-brace, with all subsequent lines
+indented two spaces, and ending with a lone close-brace.  Groups
+may be nested, increasing the required indentation of the lines
+within the nested group to two spaces per nesting level.
+
+Unlike ungrouped patterns, grouped patterns are allowed to overlap.
+Conflicts are resolved by selecting the patterns in order.  If all
+of the fixedbits for a pattern match, its translate function will
+be called.  If the translate function returns false, then subsequent
+patterns within the group will be matched.
+
+The following example from PA-RISC shows specialization of the *or*
+instruction::
+
+  {
+{
+  nop   10 - -  001001 0 0
+  copy  10 0 r1:5   001001 0 rt:5
+}
+or  10 r2:5  r1:5  cf:4 001001 0 rt:5
+  }
+
+When the *cf* field is zero, the instruction has no side effects,
+and may be specialized.  When the *rt* field is zero, the output
+is discarded and so the instruction has no effect.  When the *rt2*
+field is zero, the operation is ``reg[rt] | 0`` and so encodes
+the canonical register copy operation.
+
+The output from the generator might look like::
+
+  switch (insn & 0xfc000fe0) {
+  case 0x08000240:
+/* 10..  0010 010. */
+if ((insn & 0xf000) == 0x) {
+/* 10..  0010 010. */
+if ((insn & 0x001f) == 0x) {
+/* 10..  0010 0100 */
+extract_decode_Fmt_0(_decode0, insn);
+if (trans_nop(ctx, _decode0)) return true;
+}
+if ((insn & 0x03e0) == 0x) {
+/* 1000 000. 0010 010. */
+extract_decode_Fmt_1(_decode1, insn);
+if (trans_copy(ctx, _decode1)) return true;
+}
+}
+extract_decode_Fmt_2(_decode2, insn);
+if (trans_or(ctx, _decode2)) return true;
+return false;
+  }
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index dd495096fc..abce58ed8f 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -31,6 +31,7 @@ fields = {}
 arguments = {}
 formats = {}
 patterns = []
+allpatterns = []
 
 translate_prefix = 'trans'
 translate_scope = 'static '
@@ -353,6 +354,46 @@ class Pattern(General):
 # end Pattern
 
 
+class MultiPattern(General):
+"""Class representing an overlapping set of instruction patterns"""
+
+def __init__(self, lineno, pats, fixb, fixm, udfm):
+self.lineno = lineno
+self.pats = pats
+self.base = None
+self.fixedbits = fixb
+self.fixedmask = fixm
+self.undefmask = udfm
+
+def __str__(self):
+r = "{"
+for p in self.pats:
+   r = r + ' ' + str(p)
+return r + "}"
+
+def output_decl(self):
+for p in self.pats:
+p.output_decl()
+
+def output_code(self, i, extracted, outerbits, outermask):
+global translate_prefix
+ind = str_indent(i)
+for p in self.pats:
+if outermask != p.fixedmask:
+innermask = p.fixedmask & ~outermask
+innerbits = p.fixedbits & ~outermask
+output(ind, 'if ((insn & ',
+   '0x{0:08x}) == 0x{1:08x}'.format(innermask, innerbits),
+   ') {\n')
+output(ind, '/* ',
+   str_match_bits(p.fixedbits, p.fixedmask), ' */\n')
+p.output_code(i + 4, extracted, p.fixedbits, p.fixedmask)
+output(ind, '}\n')
+else:
+p.output_code(i, extracted, p.fixedbits, p.fixedmask)
+#end MultiPattern
+
+
 def parse_field(lineno, name, toks):
 """Parse one instruction field from TOKS at LINENO"""
 global fields
@@ -505,6 +546,7 @@ def parse_generic(lineno, is_format, name, toks):
 global arguments
 global formats
 global patterns
+global allpatterns
 global re_ident
 global insnwidth
 global insnmask
@@ -649,6 +691,7 @@ def parse_generic(lineno, is_format, name, toks):
 pat = Pattern(name, lineno, fmt, fixedbits, fixedmask,
   undefmask, fieldmask, flds)
 patterns.append(pat)
+allpatterns.append(pat)
 
 # Validate the masks that we have assembled.
 if fieldmask & fixedmask:
@@ -667,17 +710,61 @@ def parse_generic(lineno, is_format, name, toks):
  

[Qemu-devel] [PATCH 4/5] decodetree: Do not unconditionaly return from Pattern.output_code

[Richard Henderson]

As a consequence, the 'return false' gets pushed up one level.

This will allow us to perform some other action when the
translator returns failure.

Signed-off-by: Richard Henderson 
---
 scripts/decodetree.py | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index fb9a0ab3ad..dd495096fc 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -348,8 +348,8 @@ class Pattern(General):
 output(ind, self.base.extract_name(), '(_', arg, ', insn);\n')
 for n, f in self.fields.items():
 output(ind, 'u.f_', arg, '.', n, ' = ', f.str_extract(), ';\n')
-output(ind, 'return ', translate_prefix, '_', self.name,
-   '(ctx, _', arg, ');\n')
+output(ind, 'if (', translate_prefix, '_', self.name,
+   '(ctx, _', arg, ')) return true;\n')
 # end Pattern
 
 
@@ -777,8 +777,8 @@ class Tree:
 output(ind, '/* ',
str_match_bits(innerbits, innermask), ' */\n')
 s.output_code(i + 4, extracted, innerbits, innermask)
+output(ind, 'return false;\n')
 output(ind, '}\n')
-output(ind, 'return false;\n')
 # end Tree
 
 
@@ -932,6 +932,7 @@ def main():
 output(i4, '} u;\n\n')
 
 t.output_code(4, False, 0, 0)
+output(i4, 'return false;\n')
 
 output('}\n')
 
-- 
2.17.2

[Qemu-devel] [PATCH 3/5] decodetree: Ensure build_tree does not include values outside insnmask

[Richard Henderson]

From: Philippe Mathieu-Daudé 

Reproduced with "scripts/decodetree.py /dev/null".

Reviewed-by: Eduardo Habkost 
Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 scripts/decodetree.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index ba203aeccd..fb9a0ab3ad 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -784,7 +784,7 @@ class Tree:
 
 def build_tree(pats, outerbits, outermask):
 # Find the intersection of all remaining fixedmask.
-innermask = ~outermask
+innermask = ~outermask & insnmask
 for i in pats:
 innermask &= i.fixedmask
 
-- 
2.17.2

[Qemu-devel] [PATCH 2/5] decodetree: Move documentation to docs/decodetree.rst

[Richard Henderson]

One great big block comment isn't the best way to document
the syntax of a language.

Signed-off-by: Richard Henderson 
---
 MAINTAINERS   |   1 +
 docs/decodetree.rst   | 156 ++
 scripts/decodetree.py | 134 +---
 3 files changed, 158 insertions(+), 133 deletions(-)
 create mode 100644 docs/decodetree.rst

diff --git a/MAINTAINERS b/MAINTAINERS
index ad007748b9..fc7cddb873 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -118,6 +118,7 @@ F: exec.c
 F: accel/tcg/
 F: accel/stubs/tcg-stub.c
 F: scripts/decodetree.py
+F: docs/decodetree.rst
 F: include/exec/cpu*.h
 F: include/exec/exec-all.h
 F: include/exec/helper*.h
diff --git a/docs/decodetree.rst b/docs/decodetree.rst
new file mode 100644
index 00..d9be30b2db
--- /dev/null
+++ b/docs/decodetree.rst
@@ -0,0 +1,156 @@
+
+Decodetree Specification
+
+
+A *decodetree* is built from instruction *patterns*.  A pattern may
+represent a single architectural instruction or a group of same, depending
+on what is convenient for further processing.
+
+Each pattern has both *fixedbits* and *fixedmask*, the combination of which
+describes the condition under which the pattern is matched::
+
+  (insn & fixedmask) == fixedbits
+
+Each pattern may have *fields*, which are extracted from the insn and
+passed along to the translator.  Examples of such are registers,
+immediates, and sub-opcodes.
+
+In support of patterns, one may declare *fields*, *argument sets*, and
+*formats*, each of which may be re-used to simplify further definitions.
+
+Fields
+==
+
+Syntax::
+
+  field_def := '%' identifier ( unnamed_field )+ ( !function=identifier )?
+  unnamed_field := number ':' ( 's' ) number
+
+For *unnamed_field*, the first number is the least-significant bit position
+of the field and the second number is the length of the field.  If the 's' is
+present, the field is considered signed.  If multiple ``unnamed_fields`` are
+present, they are concatenated.  In this way one can define disjoint fields.
+
+If ``!function`` is specified, the concatenated result is passed through the
+named function, taking and returning an integral value.
+
+FIXME: the fields of the structure into which this result will be stored
+is restricted to ``int``.  Which means that we cannot expand 64-bit items.
+
+Field examples:
+
++---+-+
+| Input | Generated code  |
++===+=+
+| %disp   0:s16 | sextract(i, 0, 16)  |
++---+-+
+| %imm9   16:6 10:3 | extract(i, 16, 6) << 3 | extract(i, 10, 3)  |
++---+-+
+| %disp12 0:s1 1:1 2:10 | sextract(i, 0, 1) << 11 |   |
+|   |extract(i, 1, 1) << 10 | |
+|   |extract(i, 2, 10)|
++---+-+
+| %shimm8 5:s8 13:1 | expand_shimm8(sextract(i, 5, 8) << 1 |  |
+|   !function=expand_shimm8 |   extract(i, 13, 1))|
++---+-+
+
+Argument Sets
+=
+
+Syntax::
+
+  args_def:= '&' identifier ( args_elt )+ ( !extern )?
+  args_elt:= identifier
+
+Each *args_elt* defines an argument within the argument set.
+Each argument set will be rendered as a C structure "arg_$name"
+with each of the fields being one of the member arguments.
+
+If ``!extern`` is specified, the backing structure is assumed
+to have been already declared, typically via a second decoder.
+
+Argument set examples::
+
+ ra rb rc
+reg base offset
+
+
+Formats
+===
+
+Syntax::
+
+  fmt_def  := '@' identifier ( fmt_elt )+
+  fmt_elt  := fixedbit_elt | field_elt | field_ref | args_ref
+  fixedbit_elt := [01.-]+
+  field_elt:= identifier ':' 's'? number
+  field_ref:= '%' identifier | identifier '=' '%' identifier
+  args_ref := '&' identifier
+
+Defining a format is a handy way to avoid replicating groups of fields
+across many instruction patterns.
+
+A *fixedbit_elt* describes a contiguous sequence of bits that must
+be 1, 0, or don't care.  The difference between '.' and '-'
+is that '.' means that the bit will be covered with a field or a
+final 0 or 1 from the pattern, and '-' means that the bit is really
+ignored by the cpu and will not be specified.
+
+A *field_elt* describes a simple field only given a width; the position of
+the field is implied by its position with respect to other *fixedbit_elt*
+and *field_elt*.
+
+If any *fixedbit_elt* or *field_elt* appear, then all bits must be 

[Qemu-devel] [RFC 6/7] target/hppa: Use pattern groups to decode OR

[Richard Henderson]

It seems clearer to decode specializations of OR within decodetree
instead of by hand within the translation function.

Signed-off-by: Richard Henderson 
---
 target/hppa/translate.c  | 106 ---
 target/hppa/insns.decode |  10 +++-
 2 files changed, 64 insertions(+), 52 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index b4fd307b77..8f5010b633 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2619,59 +2619,63 @@ static bool trans_and(DisasContext *ctx, arg_rrr_cf *a)
 return do_log_reg(ctx, a, tcg_gen_and_reg);
 }
 
+static bool trans_copy(DisasContext *ctx, arg_copy *a)
+{
+unsigned r = a->r;
+unsigned t = a->t;
+
+if (r == 0) {
+TCGv_reg dest = dest_gpr(ctx, t);
+tcg_gen_movi_reg(dest, 0);
+save_gpr(ctx, t, dest);
+} else {
+save_gpr(ctx, t, cpu_gr[r]);
+}
+cond_free(>null_cond);
+return true;
+}
+
+static bool trans_pause(DisasContext *ctx, arg_pause *a)
+{
+#ifndef CONFIG_USER_ONLY
+/*
+ * These are QEMU extensions and are nops in the real architecture:
+ *
+ * or %r10,%r10,%r10 -- idle loop; wait for interrupt
+ * or %r31,%r31,%r31 -- death loop; offline cpu
+ *  currently implemented as idle.
+ */
+TCGv_i32 tmp;
+
+/*
+ * No need to check for supervisor, as userland can only pause
+ * until the next timer interrupt.
+ */
+nullify_over(ctx);
+
+/* Advance the instruction queue.  */
+copy_iaoq_entry(cpu_iaoq_f, ctx->iaoq_b, cpu_iaoq_b);
+copy_iaoq_entry(cpu_iaoq_b, ctx->iaoq_n, ctx->iaoq_n_var);
+nullify_set(ctx, 0);
+
+/* Tell the qemu main loop to halt until this cpu has work.  */
+tmp = tcg_const_i32(1);
+tcg_gen_st_i32(tmp, cpu_env, -offsetof(HPPACPU, env) +
+   offsetof(CPUState, halted));
+tcg_temp_free_i32(tmp);
+gen_excp_1(EXCP_HALTED);
+ctx->base.is_jmp = DISAS_NORETURN;
+
+return nullify_end(ctx);
+#else
+/* For user-only, don't pause but treat as nop.  */
+cond_free(>null_cond);
+return true;
+#endif
+}
+
 static bool trans_or(DisasContext *ctx, arg_rrr_cf *a)
 {
-if (a->cf == 0) {
-unsigned r2 = a->r2;
-unsigned r1 = a->r1;
-unsigned rt = a->t;
-
-if (rt == 0) { /* NOP */
-cond_free(>null_cond);
-return true;
-}
-if (r2 == 0) { /* COPY */
-if (r1 == 0) {
-TCGv_reg dest = dest_gpr(ctx, rt);
-tcg_gen_movi_reg(dest, 0);
-save_gpr(ctx, rt, dest);
-} else {
-save_gpr(ctx, rt, cpu_gr[r1]);
-}
-cond_free(>null_cond);
-return true;
-}
-#ifndef CONFIG_USER_ONLY
-/* These are QEMU extensions and are nops in the real architecture:
- *
- * or %r10,%r10,%r10 -- idle loop; wait for interrupt
- * or %r31,%r31,%r31 -- death loop; offline cpu
- *  currently implemented as idle.
- */
-if ((rt == 10 || rt == 31) && r1 == rt && r2 == rt) { /* PAUSE */
-TCGv_i32 tmp;
-
-/* No need to check for supervisor, as userland can only pause
-   until the next timer interrupt.  */
-nullify_over(ctx);
-
-/* Advance the instruction queue.  */
-copy_iaoq_entry(cpu_iaoq_f, ctx->iaoq_b, cpu_iaoq_b);
-copy_iaoq_entry(cpu_iaoq_b, ctx->iaoq_n, ctx->iaoq_n_var);
-nullify_set(ctx, 0);
-
-/* Tell the qemu main loop to halt until this cpu has work.  */
-tmp = tcg_const_i32(1);
-tcg_gen_st_i32(tmp, cpu_env, -offsetof(HPPACPU, env) +
- offsetof(CPUState, halted));
-tcg_temp_free_i32(tmp);
-gen_excp_1(EXCP_HALTED);
-ctx->base.is_jmp = DISAS_NORETURN;
-
-return nullify_end(ctx);
-}
-#endif
-}
 return do_log_reg(ctx, a, tcg_gen_or_reg);
 }
 
diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 55ff39dd05..46c64334d1 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -148,7 +148,15 @@ lci 01 - - -- 01001100 0 t:5
 
 andcm   10 . .  00 0 .  @rrr_cf
 and 10 . .  001000 0 .  @rrr_cf
-or  10 . .  001001 0 .  @rrr_cf
+{
+  {
+nop 10 - -  001001 0 0
+copy10 0 r:5    001001 0 t:5
+pause   10 01010 01010  001001 0 01010
+pause   10 1 1  001001 0 1
+  }
+  or10 . .  001001 0 .  @rrr_cf
+}
 xor 10 . .  001010 0 .  @rrr_cf
 uxor10 . .  001110 0 .  @rrr_cf
 ds  10 . .  

[Qemu-devel] [RFC 7/7] target/riscv: Use pattern groups for RVC

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target/riscv/insn_trans/trans_rvc.inc.c | 60 +
 target/riscv/insn16.decode  | 23 +++---
 target/riscv/insn32.decode  | 18 
 3 files changed, 48 insertions(+), 53 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvc.inc.c 
b/target/riscv/insn_trans/trans_rvc.inc.c
index 631e72c8b5..a81da2f107 100644
--- a/target/riscv/insn_trans/trans_rvc.inc.c
+++ b/target/riscv/insn_trans/trans_rvc.inc.c
@@ -48,24 +48,6 @@ static bool trans_c_li(DisasContext *ctx, arg_c_li *a)
 return trans_addi(ctx, );
 }
 
-static bool trans_c_addi16sp_lui(DisasContext *ctx, arg_c_addi16sp_lui *a)
-{
-if (a->rd == 2) {
-/* C.ADDI16SP */
-arg_addi arg = { .rd = 2, .rs1 = 2, .imm = a->imm_addi16sp };
-return trans_addi(ctx, );
-} else if (a->imm_lui != 0) {
-/* C.LUI */
-if (a->rd == 0) {
-/* Hint: insn is valid but does not affect state */
-return true;
-}
-arg_lui arg = { .rd = a->rd, .imm = a->imm_lui };
-return trans_lui(ctx, );
-}
-return false;
-}
-
 static bool trans_c_srli(DisasContext *ctx, arg_c_srli *a)
 {
 int shamt = a->shamt;
@@ -114,36 +96,38 @@ static bool trans_c_slli(DisasContext *ctx, arg_c_slli *a)
 return trans_slli(ctx, );
 }
 
-static bool trans_c_jr_mv(DisasContext *ctx, arg_c_jr_mv *a)
+static bool trans_c_jr(DisasContext *ctx, arg_c_jr *a)
 {
-if (a->rd != 0 && a->rs2 == 0) {
-/* C.JR */
+if (a->rd != 0) {
 arg_jalr arg = { .rd = 0, .rs1 = a->rd, .imm = 0 };
 return trans_jalr(ctx, );
-} else if (a->rd != 0 && a->rs2 != 0) {
-/* C.MV */
+}
+return false;
+}
+
+static bool trans_c_mv(DisasContext *ctx, arg_c_mv *a)
+{
+if (a->rd != 0 && a->rs2 != 0) {
 arg_add arg = { .rd = a->rd, .rs1 = 0, .rs2 = a->rs2 };
 return trans_add(ctx, );
 }
 return false;
 }
 
-static bool trans_c_ebreak_jalr_add(DisasContext *ctx, arg_c_ebreak_jalr_add 
*a)
+static bool trans_c_jalr(DisasContext *ctx, arg_c_jalr *a)
 {
-if (a->rd == 0 && a->rs2 == 0) {
-/* C.EBREAK */
-arg_ebreak arg = { };
-return trans_ebreak(ctx, );
-} else if (a->rd != 0) {
-if (a->rs2 == 0) {
-/* C.JALR */
-arg_jalr arg = { .rd = 1, .rs1 = a->rd, .imm = 0 };
-return trans_jalr(ctx, );
-} else {
-/* C.ADD */
-arg_add arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
-return trans_add(ctx, );
-}
+if (a->rd != 0) {
+arg_jalr arg = { .rd = 1, .rs1 = a->rd, .imm = 0 };
+return trans_jalr(ctx, );
+}
+return false;
+}
+
+static bool trans_c_add(DisasContext *ctx, arg_c_add *a)
+{
+if (a->rd != 0 && a->rs2 != 0) {
+arg_add arg = { .rd = a->rd, .rs1 = a->rd, .rs2 = a->rs2 };
+return trans_add(ctx, );
 }
 return false;
 }
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index d88a0c78ab..5b93051a19 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -42,11 +42,13 @@
 
 
 # Argument sets imported from insn32.decode:
+  !extern
  rd rs1 rs2   !extern
  imm rs1 rd   !extern
  imm rs1 rs2  !extern
  imm rd   !extern
  imm rs2 rs1  !extern
+ imm rd   !extern
 
 # Argument sets:
 immrd
@@ -55,8 +57,6 @@
rd  rs2
 _shift   shamt  rd
 
-_addi16sp_lui  imm_lui imm_addi16sp rd
-
 # Formats 16:
 @cr  . .  ..   rs2=%rs2_5  %rd
 @ci... . . .  ..   imm=%imm_ci %rd rs1=%rd
@@ -74,8 +74,6 @@
 @c_sd  ... . .  . ..   imm=%uimm_6bit_sd  rs1=2 rs2=%rs2_5
 @c_sw  ... . .  . ..   imm=%uimm_6bit_sw  rs1=2 rs2=%rs2_5
 
-@c_addi16sp_lui ... .  . . .. _addi16sp_lui %imm_lui %imm_addi16sp 
%rd
-
 @c_shift... . .. ... . .. _shift rd=%rs1_3 shamt=%nzuimm_6bit
 @c_shift2   ... . .. ... . .. _shift rd=%rdshamt=%nzuimm_6bit
 
@@ -92,7 +90,11 @@ sw110  ... ... .. ... 00 @cs_w
 # *** RV64C Standard Extension (Quadrant 1) ***
 c_addi000 .  .  . 01 @ci
 c_li  010 .  .  . 01 @ci
-c_addi16sp_lui011 .  .  . 01 @c_addi16sp_lui # shares opc with 
C.LUI
+{
+  # addi16sp
+  addi011 .  00010  . 01  rd=2 rs1=2 imm=%imm_addi16sp
+  lui 011 .  .  . 01  %rd imm=%imm_lui
+}
 c_srli100 . 00 ...  . 01 @c_shift
 c_srai100 . 01 ...  . 01 @c_shift
 andi  100 . 10 ...  . 01 @c_andi
@@ -108,7 +110,14 @@ bne   111  ... ...  . 01 @cb # c_bnez
 c_slli000 .  .  . 10 @c_shift2
 fld   001 .  .  . 10 @c_ld # fldsp
 lw010 .  .  . 10 @c_lw # lwsp
-c_jr_mv   100 0 

[Qemu-devel] [PATCH 1/5] MAINTAINERS: Add scripts/decodetree.py to the TCG section

[Richard Henderson]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20181110211313.6922-2-f4...@amsat.org>
Signed-off-by: Richard Henderson 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b1d786cfd8..ad007748b9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -117,6 +117,7 @@ F: cpus.c
 F: exec.c
 F: accel/tcg/
 F: accel/stubs/tcg-stub.c
+F: scripts/decodetree.py
 F: include/exec/cpu*.h
 F: include/exec/exec-all.h
 F: include/exec/helper*.h
-- 
2.17.2

[Qemu-devel] [PATCH 0/5] decodetree enhancements

[Richard Henderson]

Two of these have been sitting around on a branch for too long.
Then, split out the documentation to a new file.

Finally, support overlapping patterns.  To demonstrate, I convert two
decodetree users: hppa and riscv, rfc patches appended to the series.


r~


Philippe Mathieu-Daudé (2):
  MAINTAINERS: Add scripts/decodetree.py to the TCG section
  decodetree: Ensure build_tree does not include values outside insnmask

Richard Henderson (3):
  decodetree: Move documentation to docs/decodetree.rst
  decodetree: Do not unconditionaly return from Pattern.output_code
  decodetree: Allow grouping of overlapping patterns

 MAINTAINERS   |   2 +
 docs/decodetree.rst   | 214 +++
 scripts/decodetree.py | 287 --
 3 files changed, 355 insertions(+), 148 deletions(-)
 create mode 100644 docs/decodetree.rst

-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 6/8] qemu-doc: Add section on MIPS' Boston board

[Philippe Mathieu-Daudé]

On 2/22/19 8:26 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Add section on MIPS' Boston board in QEMU user documentation.
> 
> Signed-off-by: Aleksandar Markovic 
> ---
>  qemu-doc.texi | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 18e383c..ae3c3f9 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2049,6 +2049,15 @@ Malta FPGA serial device
>  Cirrus (default) or any other PCI VGA graphics card
>  @end itemize
>  
> +The Boston board emulation supports the following devices:
> +
> +@itemize @minus
> +@item
> +Xilinx FPGA, which includes a PCIe root port and an UART

and a LCD display?

Reviewed-by: Philippe Mathieu-Daudé 

> +@item
> +Intel EG20T PCH connects the I/O peripherals, but only the SATA bus is 
> emulated
> +@end itemize
> +
>  The ACER Pica emulation supports:
>  
>  @itemize @minus
> 

Re: [Qemu-devel] [PATCH v3 4/8] qemu-doc: Move section on MIPS' mipssim pseudo board

[Philippe Mathieu-Daudé]

On 2/22/19 8:26 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Move section on MIPS' mipssim pseudo board to the more
> appropriate place.
> 
> Signed-off-by: Aleksandar Markovic 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  qemu-doc.texi | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 83be010..4c5577f 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2062,19 +2062,6 @@ PC Keyboard
>  IDE controller
>  @end itemize
>  
> -The mipssim pseudo board emulation provides an environment similar
> -to what the proprietary MIPS emulator uses for running Linux.
> -It supports:
> -
> -@itemize @minus
> -@item
> -A range of MIPS CPUs, default is the 24Kf
> -@item
> -PC style serial port
> -@item
> -MIPSnet network emulation
> -@end itemize
> -
>  The MIPS Magnum R4000 emulation supports:
>  
>  @itemize @minus
> @@ -2090,6 +2077,19 @@ SCSI controller
>  G364 framebuffer
>  @end itemize
>  
> +The mipssim pseudo board emulation provides an environment similar
> +to what the proprietary MIPS emulator uses for running Linux.
> +It supports:
> +
> +@itemize @minus
> +@item
> +A range of MIPS CPUs, default is the 24Kf
> +@item
> +PC style serial port
> +@item
> +MIPSnet network emulation
> +@end itemize
> +
>  @node nanoMIPS System emulator
>  @subsection nanoMIPS System emulator
>  @cindex system emulation (nanoMIPS)
> 

Re: [Qemu-devel] [PATCH v3 5/8] qemu-doc: Add section on MIPS' Fulong 2E board

[Philippe Mathieu-Daudé]

On 2/22/19 8:26 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> Add section on MIPS' Fulong 2E board in QEMU user documentation.
> 
> Signed-off-by: Aleksandar Markovic 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  qemu-doc.texi | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 4c5577f..18e383c 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2077,6 +2077,19 @@ SCSI controller
>  G364 framebuffer
>  @end itemize
>  
> +The Fulong 2E emulation supports:
> +
> +@itemize @minus
> +@item
> +Loongson 2E CPU
> +@item
> +Bonito64 system controller as North Bridge
> +@item
> +VT82C686 chipset as South Bridge
> +@item
> +RTL8139D as a network card chipset
> +@end itemize
> +
>  The mipssim pseudo board emulation provides an environment similar
>  to what the proprietary MIPS emulator uses for running Linux.
>  It supports:
> 

Re: [Qemu-devel] [PATCH v3 10/10] iotests: add busy/recording bit test to 124

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> This adds a simple test that ensures the busy bit works for push backups,
> as well as doubling as bonus test for incremental backups that get interrupted
> by EIO errors.

This makes 124 longer to run, but it is not in the 'quick' group, so
that's okay.

The easy part: I compiled the series, and validated that ./check still
passes with this applied, so:

Tested-by: Eric Blake 

Now for the fun part (I know from IRC that you had some interesting
challenges coming up with scenarios to even provoke the states you
wanted shown in the output):

> 
> Recording bit tests are already handled sufficiently by 236.
> 
> Signed-off-by: John Snow 
> ---
>  tests/qemu-iotests/124 | 110 +
>  tests/qemu-iotests/124.out |   4 +-
>  2 files changed, 112 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
> index 5aa1bf1bd6..30f12a2202 100755
> --- a/tests/qemu-iotests/124
> +++ b/tests/qemu-iotests/124
> @@ -634,6 +634,116 @@ class 
> TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
>  self.vm.shutdown()
>  self.check_backups()
>  
> +def test_incremental_pause(self):
> +"""
> +Test an incremental backup that errors into a pause and is resumed.
> +"""
> +
> +drive0 = self.drives[0]
> +result = self.vm.qmp('blockdev-add',
> + node_name=drive0['id'],
> + driver=drive0['fmt'],
> + file={
> + 'driver': 'blkdebug',
> + 'image': {
> + 'driver': 'file',
> + 'filename': drive0['file']
> + },
> + 'set-state': [{
> + 'event': 'flush_to_disk',
> + 'state': 1,
> + 'new_state': 2
> + },{
> + 'event': 'read_aio',
> + 'state': 2,
> + 'new_state': 3
> + }],
> + 'inject-error': [{
> + 'event': 'read_aio',
> + 'errno': 5,
> + 'state': 3,
> + 'immediately': False,
> + 'once': True
> + }],

Yeah, it's hairy to come up with sequences that will pause at the right
place, and this may be sensitive enough that we have to revisit it in
the future, but for now it works and I don't have any better suggestions.

> + })
> +self.assert_qmp(result, 'return', {})
> +self.create_anchor_backup(drive0)
> +bitmap = self.add_bitmap('bitmap0', drive0)
> +
> +# Emulate guest activity
> +self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
> +  ('0xfe', '16M', '256k'),
> +  ('0x64', '32736k', '64k')))
> +
> +# For the purposes of query-block visibility of bitmaps, add a drive
> +# frontend after we've written data; otherwise we can't use hmp-io
> +result = self.vm.qmp("device_add",
> + id="device0",
> + drive=drive0['id'],
> + driver="virtio-blk")
> +self.assert_qmp(result, 'return', {})
> +
> +# Bitmap Status Check
> +query = self.vm.qmp('query-block')
> +ret = [bmap for bmap in query['return'][0]['dirty-bitmaps']
> +   if bmap.get('name') == bitmap.name][0]
> +self.assert_qmp(ret, 'count', 458752)
> +self.assert_qmp(ret, 'granularity', 65536)
> +self.assert_qmp(ret, 'status', 'active')
> +self.assert_qmp(ret, 'busy', False)
> +self.assert_qmp(ret, 'recording', True)

So far, nothing too different from we've seen elsewhere, but then we get
to the fun part:

> +
> +# Start backup
> +parent, _ = bitmap.last_target()
> +target = self.prepare_backup(bitmap, parent)
> +res = self.vm.qmp('drive-backup',
> +  job_id=bitmap.drive['id'],
> +  device=bitmap.drive['id'],
> +  sync='incremental',
> +  bitmap=bitmap.name,
> +  format=bitmap.drive['fmt'],
> +  target=target,
> +  mode='existing',
> +  on_source_error='stop')
> +self.assert_qmp(res, 'return', {})
> +
> +# Wait for the error
> +event = 

Re: [Qemu-devel] [PATCH] hw/arm/stellaris: Implement watchdog timer

[Eric Blake]

On 2/22/19 5:38 PM, michelhe...@gmail.com wrote:
> From: Michel Heily 
> 
> Signed-off-by: Michel Heily 

This appears to be your first contribution to qemu. Welcome to the
community!

Your commit message doesn't give any details beyond the "what" in the
subject line. The body of the commit message should explain the "why"
(what bug are you fixing, how to reproduce it), so that a reviewer
stands a chance of determining if the code matches the description you
gave, and if the issue you describe really does warrant the inclusion of
your patch.  In particular, when implementing a particular piece of
hardware, giving a URL to the datasheet you used as your reference will
let reviewers check if your software appears to match what the actual
hardware would do.

For more patch submission hints, see:
https://wiki.qemu.org/Contribute/SubmitAPatch

I'm not an expert on this hardware, but will at least give a cursory
glance for style issues:

> +case WDT_O_LOCK:
> +return s->locked ? 1 : 0;
> +case WDT_O_PeriphID4: /* fallthrough */
> +case WDT_O_PeriphID5:

I don't think that particular /* fallthrough */ comment is needed. It
matters when you have a case with code, but not for consecutive case labels.


> +
> +static void wdtimer_write(void *opaque, hwaddr offset,
> +   uint64_t value, unsigned size)

Alignment looks off.

> +{
> +wdtimer_state *s = (wdtimer_state *)opaque;

This is C, where void* implicitly converts to other types; you don't
need the cast.


> @@ -1243,7 +1478,7 @@ static void stellaris_init(MachineState *ms, 
> stellaris_board_info *board)
>   * Stellaris LM3S6965 Microcontroller Data Sheet (rev I)
>   * http://www.ti.com/lit/ds/symlink/lm3s6965.pdf
>   *
> - * 4000 wdtimer (unimplemented)
> + * 4000 wdtimer

Okay, so there is a datasheet. But it may still help to mention it in
the commit message in addition to the comment already in the code.

>   * 40002000 i2c (unimplemented)
>   * 40004000 GPIO
>   * 40005000 GPIO
> @@ -1335,6 +1570,12 @@ static void stellaris_init(MachineState *ms, 
> stellaris_board_info *board)
>  }
>  }
>  
> +if (board->dc1 & (1  << 3)) { /* watchdog present */

Spacing looks off.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] riscv: Add proper alignment check and pending 'C' extension upon misa writes

[Eric Blake]

On 2/22/19 10:25 AM, amagdy.af...@gmail.com wrote:
> From: ahmed_magdy 
> 
> Signed-off-by: ahmed_magdy 

This appears to be your first contribution to qemu. Welcome to the
community!

Typically, a Signed-off-by designation should be a proper name (what you
would sign a legal document with, as it has a legal significance on your
right to contribute the code).  Using all lowercase and _ instead of
space looks like a username, and while I am not one to tell you it can't
be a legal name, it is unusual enough to at least raise my suspicions.

Furthermore, your commit message doesn't give any details beyond the
"what" in the subject line. The body of the commit message should
explain the "why" (what bug are you fixing, how to reproduce it), so
that a reviewer stands a chance of determining if the code matches the
description you gave, and if the issue you describe really does warrant
the inclusion of your patch.  You gave a brief "why" in your cover letter:

"I'm submiting this patch to properly check the next instruction
alignment and scheduale compression extenstion enable upon 'MISA'
register writes to later aligned instruction through exporting next
instruction 'pc' to riscv cpu state"

where it would be wise to include an improved version of that text with
this commit proper (since the cover letter does not get applied to git).
 For that matter, when sending a single patch, a cover letter is
optional (it is only mandatory when sending a multi-patch series).

For more patch submission hints, see:
https://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 08/10] block/dirty-bitmaps: move comment block

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> Simply move the big status enum comment block to above the status
> function, and document it as being deprecated. The whole confusing
> block can get deleted in three releases time.
> 
> Signed-off-by: John Snow 
> ---
>  block/dirty-bitmap.c | 36 +++-
>  1 file changed, 19 insertions(+), 17 deletions(-)
> 
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 09/10] blockdev: remove unused paio parameter documentation

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> This field isn't present anymore.
> 
> Signed-off-by: John Snow 
> ---
>  blockdev.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/blockdev.c b/blockdev.c
> index 1aaadb6128..cbce44877d 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1255,7 +1255,6 @@ out_aio_context:
>   * @node: The name of the BDS node to search for bitmaps
>   * @name: The name of the bitmap to search for
>   * @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
> - * @paio: Output pointer for aio_context acquisition, if desired. Can be 
> NULL.
>   * @errp: Output pointer for error information. Can be NULL.
>   *
>   * @return: A bitmap object on success, or NULL on failure.
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 07/10] block/dirty-bitmaps: unify qmp_locked and user_locked calls

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> These mean the same thing now. Unify them and rename the merged call
> bdrv_dirty_bitmap_busy to indicate semantically what we are describing,
> as well as help disambiguate from the various _locked and _unlocked
> versions of bitmap helpers that refer to mutex locks.
> 
> Signed-off-by: John Snow 
> ---

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 05/10] nbd: change error checking order for bitmaps

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> Check that the bitmap is not in use prior to it checking if it is
> not enabled/recording guest writes. The bitmap being busy was likely
> at the behest of the user, so this error has a greater chance of being
> understood by the user.
> 
> Signed-off-by: John Snow 
> ---
>  nbd/server.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 04/10] block/dirty-bitmap: change semantics of enabled predicate

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> Currently, the enabled predicate means something like:
> "the QAPI status of the bitmap is ACTIVE."
> After this patch, it should mean exclusively:
> "This bitmap is recording guest writes, and is allowed to do so."
> 
> In many places, this is how this predicate was already used.
> Internal usages of the bitmap QPI can call user_locked to find out if
> the bitmap is in use by an operation.
> 
> To accommodate this, modify the create_successor routine to now
> explicitly disable the parent bitmap at creation time.
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 03/10] block/dirty-bitmap: remove set/reset assertions against enabled bit

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> bdrv_set_dirty_bitmap and bdrv_reset_dirty_bitmap are only used as an
> internal API by the mirror and migration areas of our code. These
> calls modify the bitmap, but do so at the behest of QEMU and not the
> guest.
> 
> Presently, these bitmaps are always "enabled" anyway, but there's no
> reason they have to be.
> 
> Modify these internal APIs to drop this assertion.
> 
> Signed-off-by: John Snow 
> ---
>  block/dirty-bitmap.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 02/10] block/dirty-bitmaps: rename frozen predicate helper

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> "Frozen" was a good description a long time ago, but it isn't adequate now.
> Rename the frozen predicate to has_successor to make the semantics of the
> predicate more clear to outside callers.
> 
> In the process, remove some calls to frozen() that no longer semantically
> make sense. For bdrv_enable_dirty_bitmap_locked and
> bdrv_disable_dirty_bitmap_locked, it doesn't make sense to prohibit QEMU
> internals from performing this action when we only wished to prohibit QMP
> users from issuing these commands. All of the QMP API commands for bitmap
> manipulation already check against user_locked() to prohibit these actions.
> 
> Several other assertions really want to check that the bitmap isn't in-use
> by another operation -- use the bitmap_user_locked function for this instead,
> which presently also checks for has_successor. This leaves some redundant
> checks of has_sucessor through different helpers that are addressed in

successor (maintainer can fix, if we don't need v4)

> forthcoming patches.
> 
> Signed-off-by: John Snow 
> ---
>  block/dirty-bitmap.c   | 32 +---
>  include/block/dirty-bitmap.h   |  2 +-
>  migration/block-dirty-bitmap.c |  2 +-
>  3 files changed, 19 insertions(+), 17 deletions(-)
> 
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 01/10] block/dirty-bitmap: add recording and busy properties

[Eric Blake]

On 2/22/19 6:06 PM, John Snow wrote:
> The current API allows us to report a single status, which we've defined as:
> 
> Frozen: has a successor, treated as qmp_locked, may or may not be enabled.
> Locked: no successor, qmp_locked. may or may not be enabled.
> Disabled: Not frozen or locked, disabled.
> Active: Not frozen, locked, or disabled.
> 
> The problem is that both "Frozen" and "Locked" mean nearly the same thing,
> and that both of them do not intuit whether they are recording guest writes
> or not.
> 
> This patch deprecates that status field and introduces two orthogonal
> properties instead to replace it.
> 
> Signed-off-by: John Snow 
> ---

> +++ b/qemu-deprecated.texi
> @@ -67,6 +67,12 @@ topologies described with -smp include all possible cpus, 
> i.e.
>  "autoload" parameter is now ignored. All bitmaps are automatically loaded
>  from qcow2 images.
>  
> +@subsection query-block result field(s) dirty-bitmaps[i].status (since 4.0)

I suspect you wrote "field(s)" since there can be more than one
dirty-bitmaps[i]. But it still sound funny; within a single
dirty-bitmaps[i], there is only one field being deprecated. Dropping the
'(s)' makes this subsection read better to me.

That's minor enough that a maintainer could fix it, if you don't have
any other reason to spin v4.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH 0/3] block: fix graph modification

[Vladimir Sementsov-Ogievskiy]

Hi all.

I've found two problems during my developing of backup-top filter.
The first one was already discussed in context of backup-top series,
so, here is patch 01 taken from it, previously it was
03 of [PATCH v5 00/11] backup-top filter driver for backup
(https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg06218.html)
so, here it is marked v6, and it was changed a lot:
 v6: rewrite using GHashTable and GQueue for efficient BFS.

02 is a new fix of bug I've found today, and 03 is a test for both
fixed bugs.

Vladimir Sementsov-Ogievskiy (3):
  block: improve should_update_child
  block: fix bdrv_check_perm for non-tree subgraph
  tests: add test-bdrv-graph-mod

 include/block/block_int.h   |   5 +
 block.c |  70 +++--
 tests/test-bdrv-graph-mod.c | 198 
 tests/Makefile.include  |   2 +
 4 files changed, 268 insertions(+), 7 deletions(-)
 create mode 100644 tests/test-bdrv-graph-mod.c

-- 
2.18.0

[Qemu-devel] [PATCH 2/3] block: fix bdrv_check_perm for non-tree subgraph

[Vladimir Sementsov-Ogievskiy]

bdrv_check_perm in it's recursion checks each node in context of new
permissions for one parent, because of nature of DFS. It works well,
while children subgraph of top-most updated node is a tree, i.e. it
doesn't have any kind of loops. But if we have a loop (not oriented,
of course), i.e. we have two different ways from top-node to some
child-node, then bdrv_check_perm will do wrong thing:

  top
  | \
  |  |
  v  v
  A  B
  |  |
  v  v
  node

It will once check new permissions of node in context of new A
permissions and old B permissions and once visa-versa. It's a wrong way
and may lead to corruption of permission system. We may start with
no-permissions and all-shared for both A->node and B->node relations
and finish up with non shared write permission for both ways.

The following commit will add a test, which shows this bug.

To fix this situation, let's really set BdrvChild permissions during
bdrv_check_perm procedure. And we are happy here, as check-perm is
already written in transaction manner, so we just need to restore
backed-up permissions in _abort.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Thing to discuss:
in bdrv_child_abort_perm_update(), should we include
"bdrv_abort_perm_update(c->bs);" into "if"? Or not? Anyway, if yes,
it'd better be additional patch, as it may be other bug. Or, all
things are prepared for abort even if check was not called for this
particular child?

 include/block/block_int.h |  5 +
 block.c   | 27 ++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0075bafd10..8437df85a2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -662,6 +662,11 @@ struct BdrvChild {
  */
 uint64_t shared_perm;
 
+/* backup of permissions during permission update procedure */
+bool has_backup_perm;
+uint64_t backup_perm;
+uint64_t backup_shared_perm;
+
 QLIST_ENTRY(BdrvChild) next;
 QLIST_ENTRY(BdrvChild) next_parent;
 };
diff --git a/block.c b/block.c
index d7c2ff655c..ab98e64305 100644
--- a/block.c
+++ b/block.c
@@ -1954,13 +1954,32 @@ static int bdrv_child_check_perm(BdrvChild *c, 
BlockReopenQueue *q,
 ret = bdrv_check_update_perm(c->bs, q, perm, shared, ignore_children, 
errp);
 g_slist_free(ignore_children);
 
-return ret;
+if (ret < 0) {
+return ret;
+}
+
+if (!c->has_backup_perm) {
+c->has_backup_perm = true;
+c->backup_perm = c->perm;
+c->backup_shared_perm = c->shared_perm;
+}
+/*
+ * Note: it's OK if c->has_backup_perm was already set, as we can find the
+ * same child twice during check_perm procedure
+ */
+
+c->perm = perm;
+c->shared_perm = shared;
+
+return 0;
 }
 
 static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared)
 {
 uint64_t cumulative_perms, cumulative_shared_perms;
 
+c->has_backup_perm = false;
+
 c->perm = perm;
 c->shared_perm = shared;
 
@@ -1971,6 +1990,12 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t 
perm, uint64_t shared)
 
 static void bdrv_child_abort_perm_update(BdrvChild *c)
 {
+if (c->has_backup_perm) {
+c->perm = c->backup_perm;
+c->shared_perm = c->backup_shared_perm;
+c->has_backup_perm = false;
+}
+
 bdrv_abort_perm_update(c->bs);
 }
 
-- 
2.18.0

[Qemu-devel] [PATCH 3/3] tests: add test-bdrv-graph-mod

[Vladimir Sementsov-Ogievskiy]

Add two tests of node graph modification.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/test-bdrv-graph-mod.c | 198 
 tests/Makefile.include  |   2 +
 2 files changed, 200 insertions(+)
 create mode 100644 tests/test-bdrv-graph-mod.c

diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c
new file mode 100644
index 00..458dfa6661
--- /dev/null
+++ b/tests/test-bdrv-graph-mod.c
@@ -0,0 +1,198 @@
+/*
+ * Block node graph modifications tests
+ *
+ * Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved.
+ *
+ * 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.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "block/block_int.h"
+#include "sysemu/block-backend.h"
+
+static BlockDriver bdrv_pass_through = {
+.format_name = "pass-through",
+.bdrv_child_perm = bdrv_filter_default_perms,
+};
+
+static void no_perm_default_perms(BlockDriverState *bs, BdrvChild *c,
+ const BdrvChildRole *role,
+ BlockReopenQueue *reopen_queue,
+ uint64_t perm, uint64_t shared,
+ uint64_t *nperm, uint64_t *nshared)
+{
+*nperm = 0;
+*nshared = BLK_PERM_ALL;
+}
+
+static BlockDriver bdrv_no_perm = {
+.format_name = "no-perm",
+.bdrv_child_perm = no_perm_default_perms,
+};
+
+static BlockDriverState *no_perm_node(const char *name)
+{
+return bdrv_new_open_driver(_no_perm, name, BDRV_O_RDWR, 
_abort);
+}
+
+static BlockDriverState *pass_through_node(const char *name)
+{
+return bdrv_new_open_driver(_pass_through, name,
+BDRV_O_RDWR, _abort);
+}
+
+/*
+ * test_update_perm_tree
+ *
+ * When checking node for a possibility to update permissions, it's subtree
+ * should be correctly checked too. New permissions for each node should be
+ * calculated and checked in context of permissions of other nodes. If we
+ * check new permissions of the node only in context of old permissions of
+ * its neighbors, we can finish up with wrong permission graph.
+ *
+ * This test firstly create the following graph:
+ *++
+ *|  root  |
+ *++
+ *|
+ *| perm: write, read
+ *| shared: except write
+ *v
+ *  +---+   ++
+ *  | passtrough filter |-->|  null-co node  |
+ *  +---+   ++
+ *
+ *
+ * and then, tries to append filter under node. Expected behavior: fail.
+ * Otherwise we'll get the following picture, with two BdrvChild'ren, having
+ * write permission to one node, without actually sharing it.
+ *
+ * ++
+ * |  root  |
+ * ++
+ * |
+ * | perm: write, read
+ * | shared: except write
+ * v
+ *+---+
+ *| passtrough filter |
+ *+---+
+ *   |   |
+ * perm: write, read |   | perm: write, read
+ *  shared: except write |   | shared: except write
+ *   v   v
+ *++
+ *|  null co node  |
+ *++
+ */
+static void test_update_perm_tree(void)
+{
+Error *local_err = NULL;
+
+BlockBackend *root = blk_new(BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ,
+ BLK_PERM_ALL & ~BLK_PERM_WRITE);
+BlockDriverState *bs = no_perm_node("node");
+BlockDriverState *filter = pass_through_node("filter");
+
+blk_insert_bs(root, bs, _abort);
+
+bdrv_attach_child(filter, bs, "child", _file, _abort);
+
+bdrv_append(filter, bs, _err);
+
+g_assert_nonnull(local_err);
+
+bdrv_unref(bs);
+blk_unref(root);
+}
+
+/*
+ * test_should_update_child
+ *
+ * Test that bdrv_replace_node, and concretely should_update_child
+ * do the right thing, i.e. not creating loops on the graph.
+ *
+ * The test does the 

[Qemu-devel] [PATCH v6 1/3] block: improve should_update_child

[Vladimir Sementsov-Ogievskiy]

As it already said in the comment, we don't want to create loops in
parent->child relations. So, when we try to append @to to @c, we should
check that @c is not in @to children subtree, and we should check it
recursively, not only the first level. The patch provides BFS-based
search, to check the relations.

This is needed for further fleecing-hook filter usage: we need to
append it to source, when the hook is already a parent of target, and
source may be in a backing chain of target (fleecing-scheme). So, on
appending, the hook should not became a child (direct or through
children subtree) of the target.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

v6: rewrite using GHashTable and GQueue for efficient BFS.

 block.c | 43 +--
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 4ad0e90d7e..d7c2ff655c 100644
--- a/block.c
+++ b/block.c
@@ -3542,7 +3542,9 @@ void bdrv_close_all(void)
 
 static bool should_update_child(BdrvChild *c, BlockDriverState *to)
 {
-BdrvChild *to_c;
+GQueue *queue;
+GHashTable *found;
+bool ret;
 
 if (c->role->stay_at_node) {
 return false;
@@ -3578,14 +3580,43 @@ static bool should_update_child(BdrvChild *c, 
BlockDriverState *to)
  * if A is a child of B, that means we cannot replace A by B there
  * because that would create a loop.  Silently detaching A from B
  * is also not really an option.  So overall just leaving A in
- * place there is the most sensible choice. */
-QLIST_FOREACH(to_c, >children, next) {
-if (to_c == c) {
-return false;
+ * place there is the most sensible choice.
+ *
+ * We would also create a loop in any cases where @c is only
+ * indirectly referenced by @to. Prevent this by returning false
+ * if @c is found (by breadth-first search) anywhere in the whole
+ * subtree of @to.
+ */
+
+ret = true;
+found = g_hash_table_new(NULL, NULL);
+g_hash_table_add(found, to);
+queue = g_queue_new();
+g_queue_push_tail(queue, to);
+
+while (!g_queue_is_empty(queue)) {
+BlockDriverState *v = g_queue_pop_head(queue);
+BdrvChild *c2;
+
+QLIST_FOREACH(c2, >children, next) {
+if (c2 == c) {
+ret = false;
+break;
+}
+
+if (g_hash_table_contains(found, c2->bs)) {
+continue;
+}
+
+g_queue_push_tail(queue, c2->bs);
+g_hash_table_add(found, c2->bs);
 }
 }
 
-return true;
+g_queue_free(queue);
+g_hash_table_destroy(found);
+
+return ret;
 }
 
 void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
-- 
2.18.0

Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error

[Peter Maydell]

On Fri, 22 Feb 2019 at 14:07, Stefan Hajnoczi  wrote:
> Richard: Commit 7db2145a6826b14efceb8dd64bfe6ad8647072eb ("bswap: Add
> host endian unaligned access functions") introduced the unaligned
> memory access functions in question here.  Please see below for
> details on the bug - basically QEMU code assumes they are atomic, but
> that is not guaranteed :(.  Any ideas for how to fix this?

I suspect we want a separate family of access functions for
"I guarantee this will be an aligned access and I need the
atomicity". (The other place where we've talked about needing
the atomicity is in emulation of page-table-walk, where you
need the page table loads to be atomic w.r.t. other CPU
threads, especially in the case where you need to emulate
a hardware update of a dirty/access bit in the page table entry.)

Mostly this hasn't bitten us before because any sensible compiler
will turn the memcpy into a straight load on most common hosts,
which will be atomic (but accidentally rather than on purpose).

thanks
-- PMM

Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error

[Fernando Casas Schössow]

Hi Natanael,

The package information is the following:

https://pkgs.alpinelinux.org/package/v3.9/main/x86_64/qemu-system-x86_64

The binary is for x86_64 architecture and dynamically linked:

fernando@vmsvr01:~$ file /usr/bin/qemu-system-x86_64
/usr/bin/qemu-system-x86_64: ELF 64-bit LSB shared object, x86-64, version 1 
(SYSV), dynamically linked, interpreter /lib/ld-musl-x86_64.so.1, stripped

Thanks.

On sáb, feb 23, 2019 at 4:55 PM, Natanael Copa  wrote:
On Fri, 22 Feb 2019 14:04:20 + Stefan Hajnoczi 
mailto:stefa...@gmail.com>> wrote:
On Fri, Feb 22, 2019 at 12:57 PM Fernando Casas Schössow 
mailto:casasferna...@outlook.com>> wrote: I have 
CCed Natanael Copa, qemu package maintainer in Alpine Linux. Fernando: Can you 
confirm that the bug occurs with an unmodified Alpine Linux qemu binary?
I wonder exactly which binary it is. What arcitecture (x86 or x86_64) and what 
binary. Is it ddynamic or statically linked qemu-system-x86_64?

Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error

[Peter Maydell]

On Sat, 23 Feb 2019 at 16:05, Natanael Copa  wrote:
> I was thinking of something in the lines of:
>
> typedef volatile uint16_t __attribute__((__may_alias__)) volatile_uint16_t;
> static inline int lduw_he_p(const void *ptr)
> {
>  volatile_uint16_t r = *(volatile_uint16_t*)ptr;
>  return r;
> }

This won't correctly handle accesses with unaligned pointers,
I'm afraid. We rely on these functions correctly working
with pointers that are potentially unaligned.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error

[Natanael Copa]

On Fri, 22 Feb 2019 14:04:20 +
Stefan Hajnoczi  wrote:

> On Fri, Feb 22, 2019 at 12:57 PM Fernando Casas Schössow
>  wrote:
> 
> I have CCed Natanael Copa, qemu package maintainer in Alpine Linux.
> 
> Fernando: Can you confirm that the bug occurs with an unmodified
> Alpine Linux qemu binary?

I wonder exactly which binary it is. What arcitecture (x86 or x86_64)
and what binary. Is it ddynamic or statically linked qemu-system-x86_64?
 
> Richard: Commit 7db2145a6826b14efceb8dd64bfe6ad8647072eb ("bswap: Add
> host endian unaligned access functions") introduced the unaligned
> memory access functions in question here.  Please see below for
> details on the bug - basically QEMU code assumes they are atomic, but
> that is not guaranteed :(.  Any ideas for how to fix this?
> 
> Natanael: It seems likely that the qemu package in Alpine Linux
> suffers from a compilation issue resulting in a broken QEMU.  It may
> be necessary to leave the compiler optimization flag alone in APKBUILD
> to work around this problem.
> 
> Here are the details.  QEMU relies on the compiler turning
> memcpy(, , 2) turning into a load instruction in
> include/qemu/bswap.h:lduw_he_p() (explanation below):
> 
> /* Any compiler worth its salt will turn these memcpy into native unaligned
>operations.  Thus we don't need to play games with packed attributes, or
>inline byte-by-byte stores.  */
> 
> static inline int lduw_he_p(const void *ptr)
> {
> uint16_t r;
> memcpy(, ptr, sizeof(r));
> return r;
> }
> 
> Here is the disassembly snippet of virtqueue_pop() from Fedora 29 that
> shows the load instruction:
> 
>   398166:   0f b7 42 02 movzwl 0x2(%rdx),%eax
>   39816a:   66 89 43 32 mov%ax,0x32(%rbx)
> 
> Here is the instruction sequence in the Alpine Linux binary:
> 
>   455562:ba 02 00 00 00   mov$0x2,%edx
>   455567:e8 74 24 f3 ff   callq  3879e0 
>   45556c:0f b7 44 24 42   movzwl 0x42(%rsp),%eax
>   455571:66 41 89 47 32   mov%ax,0x32(%r15)
> 
> It's calling memcpy instead of using a load instruction.

I tried to find this section. How do you get the assembly listing of
relevant secion? I tried to do "disas virtio_pop" from
`gdb /usr/bin/qemu-system-x86_64` from the binary in alpine edge. I
could find 2 memcpy but none of them look like a 16 bit operation after:

   0x004551f1 <+353>:   mov0x10(%rsp),%rdi
   0x004551f6 <+358>:   mov$0x10,%edx
   0x004551fb <+363>:   callq  0x3879e0 
   0x00455200 <+368>:   movzwl 0x5c(%rsp),%eax
   0x00455205 <+373>:   test   $0x4,%al
   0x00455207 <+375>:   je 0x4552aa 



   0x00455291 <+513>:   mov0x10(%rsp),%rdi
   0x00455296 <+518>:   mov$0x10,%edx
   0x0045529b <+523>:   callq  0x3879e0 
   0x004552a0 <+528>:   mov%rbp,0x20(%rsp)
   0x004552a5 <+533>:   movzwl 0x5c(%rsp),%eax
   0x004552aa <+538>:   lea0x20e0(%rsp),%rdi
   0x004552b2 <+546>:   xor%r11d,%r11d
   0x004552b5 <+549>:   mov%r15,0x38(%rsp)



> 
> Fernando found that QEMU's virtqueue_pop() function sees bogus values
> when loading a 16-bit guest RAM location.  Paolo figured out that the
> bogus value can be produced by memcpy() when another thread is
> updating the 16-bit memory location simultaneously.  This is a race
> condition between one thread loading the 16-bit value and another
> thread storing it (in this case a guest vcpu thread).  Sometimes
> memcpy() may load one old byte and one new byte, resulting in a bogus
> value.
> 
> The symptom that Fernando experienced is a "Virtqueue size exceeded"
> error message from QEMU and then the virtio-blk or virtio-scsi device
> stops working.  This issue potentially affects other device emulation
> code in QEMU as well, not just virtio devices.
> 
> For the time being, I suggest tweaking the APKBUILD so the memcpy() is
> not generated.  Hopefully QEMU can make the code more portable in the
> future so the compiler always does the expected thing, but this may
> not be easily possible.

I was thinking of something in the lines of:

typedef volatile uint16_t __attribute__((__may_alias__)) volatile_uint16_t;
static inline int lduw_he_p(const void *ptr)
{
 volatile_uint16_t r = *(volatile_uint16_t*)ptr;
 return r;
}

I can test different CFLAGS with and without the _FORTIFY_SOURCE and
with different variants of memcpy (like __builtint_memcpy etc) but i
need find a way to get the correct assembly output so I know if/when I
have found something that works.

Thanks!

-nc


> 
> Stefan

[Qemu-devel] [PATCH] spapr-rtas: add ibm,get-vpd RTAS interface

[Maxiwell S. Garcia]

This adds a handler for ibm,get-vpd RTAS calls, allowing pseries
guest to collect host information. It is disabled by default to
avoid unwanted information leakage. To enable it, use:
‘-M pseries,vpd-export=on’

Only the SE and TM keywords are returned at the moment:
SE for Machine or Cabinet Serial Number and
TM for Machine Type and Model.

Powerpc-utils tools can dispatch RTAS calls to retrieve host
information using this ibm,get-vpd interface. The 'host-serial'
and 'host-model' nodes of device-tree hold the same information but
in a static manner, which is useless after a migration operation.

Signed-off-by: Maxiwell S. Garcia 
---
 hw/ppc/spapr.c | 21 ++
 hw/ppc/spapr_rtas.c| 93 ++
 include/hw/ppc/spapr.h | 17 +++-
 3 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index abf9ebce59..09fd9e2ebb 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3026,6 +3026,20 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, 
BusState *bus,
 return NULL;
 }
 
+static bool spapr_get_vpd_export(Object *obj, Error **errp)
+{
+sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+
+return spapr->vpd_export;
+}
+
+static void spapr_set_vpd_export(Object *obj, bool value, Error **errp)
+{
+sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
+
+spapr->vpd_export = value;
+}
+
 static char *spapr_get_kvm_type(Object *obj, Error **errp)
 {
 sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
@@ -3150,6 +3164,7 @@ static void spapr_instance_init(Object *obj)
 sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
 
 spapr->htab_fd = -1;
+spapr->vpd_export = false;
 spapr->use_hotplug_event_source = true;
 object_property_add_str(obj, "kvm-type",
 spapr_get_kvm_type, spapr_set_kvm_type, NULL);
@@ -3182,6 +3197,12 @@ static void spapr_instance_init(Object *obj)
 object_property_add_bool(obj, "vfio-no-msix-emulation",
  spapr_get_msix_emulation, NULL, NULL);
 
+object_property_add_bool(obj, "vpd-export", spapr_get_vpd_export,
+ spapr_set_vpd_export, NULL);
+object_property_set_description(obj, "vpd-export",
+"Export Host's VPD information to guest",
+_abort);
+
 /* The machine class defines the default interrupt controller mode */
 spapr->irq = smc->irq;
 object_property_add_str(obj, "ic-mode", spapr_get_ic_mode,
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index d6a0952154..214e0edfc5 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -287,6 +287,97 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
 rtas_st(rets, 0, ret);
 }
 
+static inline int vpd_st(target_ulong addr, target_ulong len,
+ const void *val, uint16_t val_len)
+{
+hwaddr phys = ppc64_phys_to_real(addr);
+if (len < val_len) {
+return RTAS_OUT_PARAM_ERROR;
+}
+cpu_physical_memory_write(phys, val, val_len);
+return RTAS_OUT_SUCCESS;
+}
+
+static inline void vpd_ret(target_ulong rets, const int status,
+   const int next_seq_number, const int bytes_returned)
+{
+rtas_st(rets, 0, status);
+rtas_st(rets, 1, next_seq_number);
+rtas_st(rets, 2, bytes_returned);
+}
+
+static void rtas_ibm_get_vpd(PowerPCCPU *cpu,
+ sPAPRMachineState *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+sPAPRMachineState *sm = SPAPR_MACHINE(spapr);
+target_ulong loc_code_addr;
+target_ulong work_area_addr;
+target_ulong work_area_size;
+target_ulong seq_number;
+unsigned char loc_code = 0;
+unsigned int next_seq_number = 0;
+int status = RTAS_IBM_GET_VPD_PARAMETER_ERROR;
+int ret = 0;
+char *field = '\0';
+
+if (!sm->vpd_export) {
+vpd_ret(rets, RTAS_OUT_NOT_AUTHORIZED, 1, 0);
+return;
+}
+
+/* Specific Location Code is not supported */
+loc_code_addr = rtas_ld(args, 0);
+cpu_physical_memory_read(loc_code_addr, _code, 1);
+if (loc_code != 0) {
+vpd_ret(rets, RTAS_IBM_GET_VPD_PARAMETER_ERROR, 1, 0);
+return;
+}
+
+work_area_addr = rtas_ld(args, 1);
+work_area_size = rtas_ld(args, 2);
+seq_number = rtas_ld(args, 3);
+switch (seq_number) {
+case RTAS_IBM_VPD_KEYWORD_SE: {
+char *host_serial;
+if (kvmppc_get_host_serial(_serial)) {
+/* LoPAPR: SE for Machine or Cabinet Serial Number */
+field = g_strdup_printf("SE %s", host_serial);
+ret = vpd_st(work_area_addr, work_area_size,
+ field, strlen(field) + 1);
+g_free(host_serial);
+}
+

[Qemu-devel] [PATCH PULL 10/11] hw/pvrdma: Unregister from shutdown notifier when device goes down

[Marcel Apfelbaum]

From: Yuval Shaia 

This hook was installed to close the device when VM is going down.
After the device is closed there is no need to be informed on VM
shutdown.

Signed-off-by: Yuval Shaia 
Reviewed-by: Marcel Apfelbaum 
Message-Id: <20190213065357.16076-11-yuval.sh...@oracle.com>
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/vmw/pvrdma_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 8b379c6435..1177c0822f 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -359,6 +359,8 @@ static void pvrdma_fini(PCIDevice *pdev)
 {
 PVRDMADev *dev = PVRDMA_DEV(pdev);
 
+notifier_remove(>shutdown_notifier);
+
 pvrdma_qp_ops_fini();
 
 rdma_backend_stop(>backend_dev);
-- 
2.17.1

[Qemu-devel] [PATCH PULL 11/11] hw/rdma: another clang compilation fix

[Marcel Apfelbaum]

Configuring QEMU with:
   configure --target-list="x86_64-softmmu" --cc=clang --enable-pvrdma
Results in:
   qemu/hw/rdma/rdma_rm_defs.h:108:3: error: redefinition of typedef 
'RdmaDeviceResources' is a C11 feature [-Werror,-Wtypedef-redefinition]
   } RdmaDeviceResources;
 ^
   qemu/hw/rdma/rdma_backend_defs.h:24:36: note: previous definition is here
   typedef struct RdmaDeviceResources RdmaDeviceResources;

Fix by removing one of the 'typedef' definitions.

Signed-off-by: Marcel Apfelbaum 
Message-Id: <20190214154053.15050-1-marcel.apfelb...@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Kamal Heib 
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/rdma_rm_defs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
index 325bbf58ec..e00091e730 100644
--- a/hw/rdma/rdma_rm_defs.h
+++ b/hw/rdma/rdma_rm_defs.h
@@ -120,7 +120,7 @@ typedef struct RdmaRmStats {
 uint64_t missing_cqe;
 } RdmaRmStats;
 
-typedef struct RdmaDeviceResources {
+struct RdmaDeviceResources {
 RdmaRmPort port;
 RdmaRmResTbl pd_tbl;
 RdmaRmResTbl mr_tbl;
@@ -131,6 +131,6 @@ typedef struct RdmaDeviceResources {
 GHashTable *qp_hash; /* Keeps mapping between real and emulated */
 QemuMutex lock;
 RdmaRmStats stats;
-} RdmaDeviceResources;
+};
 
 #endif
-- 
2.17.1

[Qemu-devel] [PATCH PULL 02/11] hw/rdma: Switch to generic error reporting way

[Marcel Apfelbaum]

From: Yuval Shaia 

Utilize error_report for all pr_err calls and some pr_dbg that are
considered as errors.
For the remaining pr_dbg calls, the important ones were replaced by
trace points while other deleted.

Signed-off-by: Yuval Shaia 
Reviewed-by: Marcel Apfelbaum 
Message-Id: <20190213065357.16076-2-yuval.sh...@oracle.com>
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/rdma_backend.c| 336 ++
 hw/rdma/rdma_rm.c | 121 +---
 hw/rdma/rdma_utils.c  |  11 +-
 hw/rdma/rdma_utils.h  |  45 +
 hw/rdma/trace-events  |  32 +++-
 hw/rdma/vmw/pvrdma.h  |   2 +-
 hw/rdma/vmw/pvrdma_cmd.c  | 113 +++-
 hw/rdma/vmw/pvrdma_dev_ring.c |  26 +--
 hw/rdma/vmw/pvrdma_main.c | 132 +
 hw/rdma/vmw/pvrdma_qp_ops.c   |  49 ++---
 hw/rdma/vmw/trace-events  |  16 +-
 11 files changed, 337 insertions(+), 546 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index fd571f21e5..5f60856d19 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -14,7 +14,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qlist.h"
@@ -39,7 +38,6 @@
 
 typedef struct BackendCtx {
 void *up_ctx;
-bool is_tx_req;
 struct ibv_sge sge; /* Used to save MAD recv buffer */
 } BackendCtx;
 
@@ -52,7 +50,7 @@ static void (*comp_handler)(void *ctx, struct ibv_wc *wc);
 
 static void dummy_comp_handler(void *ctx, struct ibv_wc *wc)
 {
-pr_err("No completion handler is registered\n");
+rdma_error_report("No completion handler is registered");
 }
 
 static inline void complete_work(enum ibv_wc_status status, uint32_t 
vendor_err,
@@ -66,29 +64,24 @@ static inline void complete_work(enum ibv_wc_status status, 
uint32_t vendor_err,
 comp_handler(ctx, );
 }
 
-static void poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
+static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq 
*ibcq)
 {
 int i, ne;
 BackendCtx *bctx;
 struct ibv_wc wc[2];
 
-pr_dbg("Entering poll_cq loop on cq %p\n", ibcq);
 do {
 ne = ibv_poll_cq(ibcq, ARRAY_SIZE(wc), wc);
 
-pr_dbg("Got %d completion(s) from cq %p\n", ne, ibcq);
+trace_rdma_poll_cq(ne, ibcq);
 
 for (i = 0; i < ne; i++) {
-pr_dbg("wr_id=0x%" PRIx64 "\n", wc[i].wr_id);
-pr_dbg("status=%d\n", wc[i].status);
-
 bctx = rdma_rm_get_cqe_ctx(rdma_dev_res, wc[i].wr_id);
 if (unlikely(!bctx)) {
-pr_dbg("Error: Failed to find ctx for req %" PRId64 "\n",
-   wc[i].wr_id);
+rdma_error_report("No matching ctx for req %"PRId64,
+  wc[i].wr_id);
 continue;
 }
-pr_dbg("Processing %s CQE\n", bctx->is_tx_req ? "send" : "recv");
 
 comp_handler(bctx->up_ctx, [i]);
 
@@ -98,7 +91,7 @@ static void poll_cq(RdmaDeviceResources *rdma_dev_res, struct 
ibv_cq *ibcq)
 } while (ne > 0);
 
 if (ne < 0) {
-pr_dbg("Got error %d from ibv_poll_cq\n", ne);
+rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
 }
 }
 
@@ -115,12 +108,10 @@ static void *comp_handler_thread(void *arg)
 flags = fcntl(backend_dev->channel->fd, F_GETFL);
 rc = fcntl(backend_dev->channel->fd, F_SETFL, flags | O_NONBLOCK);
 if (rc < 0) {
-pr_dbg("Fail to change to non-blocking mode\n");
+rdma_error_report("Failed to change backend channel FD to 
non-blocking");
 return NULL;
 }
 
-pr_dbg("Starting\n");
-
 pfds[0].fd = backend_dev->channel->fd;
 pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
 
@@ -132,27 +123,25 @@ static void *comp_handler_thread(void *arg)
 } while (!rc && backend_dev->comp_thread.run);
 
 if (backend_dev->comp_thread.run) {
-pr_dbg("Waiting for completion on channel %p\n", 
backend_dev->channel);
 rc = ibv_get_cq_event(backend_dev->channel, _cq, _ctx);
-pr_dbg("ibv_get_cq_event=%d\n", rc);
 if (unlikely(rc)) {
-pr_dbg("---> ibv_get_cq_event (%d)\n", rc);
+rdma_error_report("ibv_get_cq_event fail, rc=%d, errno=%d", rc,
+  errno);
 continue;
 }
 
 rc = ibv_req_notify_cq(ev_cq, 0);
 if (unlikely(rc)) {
-pr_dbg("Error %d from ibv_req_notify_cq\n", rc);
+rdma_error_report("ibv_req_notify_cq fail, rc=%d, errno=%d", 
rc,
+  errno);
 }
 
-poll_cq(backend_dev->rdma_dev_res, ev_cq);
+rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
 
 ibv_ack_cq_events(ev_cq, 1);
 }
 }
 
-pr_dbg("Going down\n");
-
 /* TODO: Post cqe for all remaining buffs that 

[Qemu-devel] [PATCH PULL 05/11] {monitor, hw/pvrdma}: Expose device internals via monitor interface

[Marcel Apfelbaum]

From: Yuval Shaia 

Allow interrogating device internals through HMP interface.
The exposed indicators can be used for troubleshooting by developers or
sysadmin.
There is no need to expose these attributes to a management system (e.x.
libvirt) because (1) most of them are not "device-management' related
info and (2) there is no guarantee the interface is stable.

Signed-off-by: Yuval Shaia 
Acked-by: Dr. David Alan Gilbert 
Message-Id: <20190213065357.16076-6-yuval.sh...@oracle.com>
Reviewed-by: Marcel Apfelbaum 
Signed-off-by: Marcel Apfelbaum 
---
 hmp-commands-info.hx  | 16 
 hw/rdma/rdma_backend.c| 70 ++-
 hw/rdma/rdma_rm.c |  7 
 hw/rdma/rdma_rm_defs.h| 27 +-
 hw/rdma/vmw/pvrdma.h  |  5 +++
 hw/rdma/vmw/pvrdma_hmp.h  | 21 +++
 hw/rdma/vmw/pvrdma_main.c | 77 +++
 monitor.c | 10 +
 8 files changed, 215 insertions(+), 18 deletions(-)
 create mode 100644 hw/rdma/vmw/pvrdma_hmp.h

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index cbee8b944d..9153c33974 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -524,6 +524,22 @@ STEXI
 Show CPU statistics.
 ETEXI
 
+#if defined(CONFIG_PVRDMA)
+{
+.name   = "pvrdmacounters",
+.args_type  = "",
+.params = "",
+.help   = "show pvrdma device counters",
+.cmd= hmp_info_pvrdmacounters,
+},
+
+STEXI
+@item info pvrdmacounters
+@findex info pvrdmacounters
+Show pvrdma device counters.
+ETEXI
+#endif
+
 #if defined(CONFIG_SLIRP)
 {
 .name   = "usernet",
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 3a2913facf..0fb4842970 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -64,9 +64,9 @@ static inline void complete_work(enum ibv_wc_status status, 
uint32_t vendor_err,
 comp_handler(ctx, );
 }
 
-static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq 
*ibcq)
+static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
 {
-int i, ne;
+int i, ne, total_ne = 0;
 BackendCtx *bctx;
 struct ibv_wc wc[2];
 
@@ -89,12 +89,18 @@ static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, 
struct ibv_cq *ibcq)
 rdma_rm_dealloc_cqe_ctx(rdma_dev_res, wc[i].wr_id);
 g_free(bctx);
 }
+total_ne += ne;
 } while (ne > 0);
+atomic_sub(_dev_res->stats.missing_cqe, total_ne);
 qemu_mutex_unlock(_dev_res->lock);
 
 if (ne < 0) {
 rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
 }
+
+rdma_dev_res->stats.completions += total_ne;
+
+return total_ne;
 }
 
 static void *comp_handler_thread(void *arg)
@@ -122,6 +128,9 @@ static void *comp_handler_thread(void *arg)
 while (backend_dev->comp_thread.run) {
 do {
 rc = qemu_poll_ns(pfds, 1, THR_POLL_TO * (int64_t)SCALE_MS);
+if (!rc) {
+backend_dev->rdma_dev_res->stats.poll_cq_ppoll_to++;
+}
 } while (!rc && backend_dev->comp_thread.run);
 
 if (backend_dev->comp_thread.run) {
@@ -138,6 +147,7 @@ static void *comp_handler_thread(void *arg)
   errno);
 }
 
+backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;
 rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
 
 ibv_ack_cq_events(ev_cq, 1);
@@ -271,7 +281,13 @@ int rdma_backend_query_port(RdmaBackendDev *backend_dev,
 
 void rdma_backend_poll_cq(RdmaDeviceResources *rdma_dev_res, RdmaBackendCQ *cq)
 {
-rdma_poll_cq(rdma_dev_res, cq->ibcq);
+int polled;
+
+rdma_dev_res->stats.poll_cq_from_guest++;
+polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
+if (!polled) {
+rdma_dev_res->stats.poll_cq_from_guest_empty++;
+}
 }
 
 static GHashTable *ah_hash;
@@ -333,7 +349,7 @@ static void ah_cache_init(void)
 
 static int build_host_sge_array(RdmaDeviceResources *rdma_dev_res,
 struct ibv_sge *dsge, struct ibv_sge *ssge,
-uint8_t num_sge)
+uint8_t num_sge, uint64_t *total_length)
 {
 RdmaRmMR *mr;
 int ssge_idx;
@@ -349,6 +365,8 @@ static int build_host_sge_array(RdmaDeviceResources 
*rdma_dev_res,
 dsge->length = ssge[ssge_idx].length;
 dsge->lkey = rdma_backend_mr_lkey(>backend_mr);
 
+*total_length += dsge->length;
+
 dsge++;
 }
 
@@ -445,8 +463,10 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
 rc = mad_send(backend_dev, sgid_idx, sgid, sge, num_sge);
 if (rc) {
 complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_MAD_SEND, ctx);
+backend_dev->rdma_dev_res->stats.mad_tx_err++;
 } else {
 complete_work(IBV_WC_SUCCESS, 0, ctx);
+

[Qemu-devel] [PATCH PULL 04/11] hw/rdma: Protect against concurrent execution of poll_cq

[Marcel Apfelbaum]

From: Yuval Shaia 

The function rdma_poll_cq is called from two contexts - completion
handler thread which sense new completion on backend channel and
explicitly as result of guest issuing poll_cq command.

Add lock to protect against concurrent executions.

Signed-off-by: Yuval Shaia 
Reviewed-by: Marcel Apfelbaum 
Message-Id: <20190213065357.16076-5-yuval.sh...@oracle.com>
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/rdma_backend.c | 2 ++
 hw/rdma/rdma_rm.c  | 4 
 hw/rdma/rdma_rm_defs.h | 1 +
 3 files changed, 7 insertions(+)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 6e9c4617da..3a2913facf 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -70,6 +70,7 @@ static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, 
struct ibv_cq *ibcq)
 BackendCtx *bctx;
 struct ibv_wc wc[2];
 
+qemu_mutex_lock(_dev_res->lock);
 do {
 ne = ibv_poll_cq(ibcq, ARRAY_SIZE(wc), wc);
 
@@ -89,6 +90,7 @@ static void rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, 
struct ibv_cq *ibcq)
 g_free(bctx);
 }
 } while (ne > 0);
+qemu_mutex_unlock(_dev_res->lock);
 
 if (ne < 0) {
 rdma_error_report("ibv_poll_cq fail, rc=%d, errno=%d", ne, errno);
diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 64c6ea1a4e..7cc597cdc8 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -618,12 +618,16 @@ int rdma_rm_init(RdmaDeviceResources *dev_res, struct 
ibv_device_attr *dev_attr,
 
 init_ports(dev_res);
 
+qemu_mutex_init(_res->lock);
+
 return 0;
 }
 
 void rdma_rm_fini(RdmaDeviceResources *dev_res, RdmaBackendDev *backend_dev,
   const char *ifname)
 {
+qemu_mutex_destroy(_res->lock);
+
 fini_ports(dev_res, backend_dev, ifname);
 
 res_tbl_free(_res->uc_tbl);
diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
index 0ba61d1838..f0ee1f3072 100644
--- a/hw/rdma/rdma_rm_defs.h
+++ b/hw/rdma/rdma_rm_defs.h
@@ -105,6 +105,7 @@ typedef struct RdmaDeviceResources {
 RdmaRmResTbl cq_tbl;
 RdmaRmResTbl cqe_ctx_tbl;
 GHashTable *qp_hash; /* Keeps mapping between real and emulated */
+QemuMutex lock;
 } RdmaDeviceResources;
 
 #endif
-- 
2.17.1

[Qemu-devel] [PATCH PULL 08/11] hw/pvrdma: Delete unneeded function argument

[Marcel Apfelbaum]

From: Yuval Shaia 

The function argument rdma_dev_res is not needed as it is stored in the
backend_dev object at init.

Signed-off-by: Yuval Shaia 
Reviewed-by: Marcel Apfelbaum 
Message-Id: <20190213065357.16076-9-yuval.sh...@oracle.com>
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/rdma_backend.c  | 13 ++---
 hw/rdma/rdma_backend.h  |  1 -
 hw/rdma/vmw/pvrdma_qp_ops.c |  3 +--
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 83a68057a7..4c994e9a39 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -594,7 +594,6 @@ static unsigned int save_mad_recv_buffer(RdmaBackendDev 
*backend_dev,
 }
 
 void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
-RdmaDeviceResources *rdma_dev_res,
 RdmaBackendQP *qp, uint8_t qp_type,
 struct ibv_sge *sge, uint32_t num_sge, void *ctx)
 {
@@ -613,9 +612,9 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
 rc = save_mad_recv_buffer(backend_dev, sge, num_sge, ctx);
 if (rc) {
 complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
-rdma_dev_res->stats.mad_rx_bufs_err++;
+backend_dev->rdma_dev_res->stats.mad_rx_bufs_err++;
 } else {
-rdma_dev_res->stats.mad_rx_bufs++;
+backend_dev->rdma_dev_res->stats.mad_rx_bufs++;
 }
 }
 return;
@@ -625,7 +624,7 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
 bctx->up_ctx = ctx;
 bctx->backend_qp = qp;
 
-rc = rdma_rm_alloc_cqe_ctx(rdma_dev_res, _id, bctx);
+rc = rdma_rm_alloc_cqe_ctx(backend_dev->rdma_dev_res, _id, bctx);
 if (unlikely(rc)) {
 complete_work(IBV_WC_GENERAL_ERR, VENDOR_ERR_NOMEM, ctx);
 goto err_free_bctx;
@@ -633,7 +632,7 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
 
 rdma_protected_gslist_append_int32(>cqe_ctx_list, bctx_id);
 
-rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge,
+rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge,
   _dev->rdma_dev_res->stats.rx_bufs_len);
 if (rc) {
 complete_work(IBV_WC_GENERAL_ERR, rc, ctx);
@@ -652,13 +651,13 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
 }
 
 atomic_inc(_dev->rdma_dev_res->stats.missing_cqe);
-rdma_dev_res->stats.rx_bufs++;
+backend_dev->rdma_dev_res->stats.rx_bufs++;
 
 return;
 
 err_dealloc_cqe_ctx:
 backend_dev->rdma_dev_res->stats.rx_bufs_err++;
-rdma_rm_dealloc_cqe_ctx(rdma_dev_res, bctx_id);
+rdma_rm_dealloc_cqe_ctx(backend_dev->rdma_dev_res, bctx_id);
 
 err_free_bctx:
 g_free(bctx);
diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h
index cb5efa2a3a..5d507a1c41 100644
--- a/hw/rdma/rdma_backend.h
+++ b/hw/rdma/rdma_backend.h
@@ -111,7 +111,6 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
 union ibv_gid *dgid, uint32_t dqpn, uint32_t dqkey,
 void *ctx);
 void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
-RdmaDeviceResources *rdma_dev_res,
 RdmaBackendQP *qp, uint8_t qp_type,
 struct ibv_sge *sge, uint32_t num_sge, void *ctx);
 
diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
index 16db726dac..508d8fca3c 100644
--- a/hw/rdma/vmw/pvrdma_qp_ops.c
+++ b/hw/rdma/vmw/pvrdma_qp_ops.c
@@ -231,8 +231,7 @@ void pvrdma_qp_recv(PVRDMADev *dev, uint32_t qp_handle)
 continue;
 }
 
-rdma_backend_post_recv(>backend_dev, >rdma_dev_res,
-   >backend_qp, qp->qp_type,
+rdma_backend_post_recv(>backend_dev, >backend_qp, qp->qp_type,
(struct ibv_sge *)>sge[0], 
wqe->hdr.num_sge,
comp_ctx);
 
-- 
2.17.1

[Qemu-devel] [PATCH PULL 07/11] hw/rdma: Free all receive buffers when QP is destroyed

[Marcel Apfelbaum]

From: Yuval Shaia 

When QP is destroyed the backend QP is destroyed as well. This ensures
we clean all received buffer we posted to it.
However, a contexts of these buffers are still remain in the device.
Fix it by maintaining a list of buffer's context and free them when QP
is destroyed.

Signed-off-by: Yuval Shaia 
Message-Id: <20190213065357.16076-8-yuval.sh...@oracle.com>
Reviewed-by: Marcel Apfelbaum 
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/rdma_backend.c  | 26 --
 hw/rdma/rdma_backend.h  |  2 +-
 hw/rdma/rdma_backend_defs.h |  2 +-
 hw/rdma/rdma_rm.c   |  2 +-
 hw/rdma/rdma_utils.c| 29 +
 hw/rdma/rdma_utils.h| 11 +++
 6 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index e8ee205b5f..83a68057a7 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -39,6 +39,7 @@
 typedef struct BackendCtx {
 void *up_ctx;
 struct ibv_sge sge; /* Used to save MAD recv buffer */
+RdmaBackendQP *backend_qp; /* To maintain recv buffers */
 } BackendCtx;
 
 struct backend_umad {
@@ -73,6 +74,7 @@ static void free_cqe_ctx(gpointer data, gpointer user_data)
 bctx = rdma_rm_get_cqe_ctx(rdma_dev_res, cqe_ctx_id);
 if (bctx) {
 rdma_rm_dealloc_cqe_ctx(rdma_dev_res, cqe_ctx_id);
+atomic_dec(_dev_res->stats.missing_cqe);
 }
 g_free(bctx);
 }
@@ -85,13 +87,15 @@ static void clean_recv_mads(RdmaBackendDev *backend_dev)
 cqe_ctx_id = rdma_protected_qlist_pop_int64(_dev->
 recv_mads_list);
 if (cqe_ctx_id != -ENOENT) {
+atomic_inc(_dev->rdma_dev_res->stats.missing_cqe);
 free_cqe_ctx(GINT_TO_POINTER(cqe_ctx_id),
  backend_dev->rdma_dev_res);
 }
 } while (cqe_ctx_id != -ENOENT);
 }
 
-static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
+static int rdma_poll_cq(RdmaBackendDev *backend_dev,
+RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
 {
 int i, ne, total_ne = 0;
 BackendCtx *bctx;
@@ -113,6 +117,8 @@ static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, 
struct ibv_cq *ibcq)
 
 comp_handler(bctx->up_ctx, [i]);
 
+rdma_protected_gslist_remove_int32(>backend_qp->cqe_ctx_list,
+   wc[i].wr_id);
 rdma_rm_dealloc_cqe_ctx(rdma_dev_res, wc[i].wr_id);
 g_free(bctx);
 }
@@ -175,14 +181,12 @@ static void *comp_handler_thread(void *arg)
 }
 
 backend_dev->rdma_dev_res->stats.poll_cq_from_bk++;
-rdma_poll_cq(backend_dev->rdma_dev_res, ev_cq);
+rdma_poll_cq(backend_dev, backend_dev->rdma_dev_res, ev_cq);
 
 ibv_ack_cq_events(ev_cq, 1);
 }
 }
 
-/* TODO: Post cqe for all remaining buffs that were posted */
-
 backend_dev->comp_thread.is_running = false;
 
 qemu_thread_exit(0);
@@ -311,7 +315,7 @@ void rdma_backend_poll_cq(RdmaDeviceResources 
*rdma_dev_res, RdmaBackendCQ *cq)
 int polled;
 
 rdma_dev_res->stats.poll_cq_from_guest++;
-polled = rdma_poll_cq(rdma_dev_res, cq->ibcq);
+polled = rdma_poll_cq(cq->backend_dev, rdma_dev_res, cq->ibcq);
 if (!polled) {
 rdma_dev_res->stats.poll_cq_from_guest_empty++;
 }
@@ -501,6 +505,7 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
 
 bctx = g_malloc0(sizeof(*bctx));
 bctx->up_ctx = ctx;
+bctx->backend_qp = qp;
 
 rc = rdma_rm_alloc_cqe_ctx(backend_dev->rdma_dev_res, _id, bctx);
 if (unlikely(rc)) {
@@ -508,6 +513,8 @@ void rdma_backend_post_send(RdmaBackendDev *backend_dev,
 goto err_free_bctx;
 }
 
+rdma_protected_gslist_append_int32(>cqe_ctx_list, bctx_id);
+
 rc = build_host_sge_array(backend_dev->rdma_dev_res, new_sge, sge, num_sge,
   _dev->rdma_dev_res->stats.tx_len);
 if (rc) {
@@ -616,6 +623,7 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
 
 bctx = g_malloc0(sizeof(*bctx));
 bctx->up_ctx = ctx;
+bctx->backend_qp = qp;
 
 rc = rdma_rm_alloc_cqe_ctx(rdma_dev_res, _id, bctx);
 if (unlikely(rc)) {
@@ -623,6 +631,8 @@ void rdma_backend_post_recv(RdmaBackendDev *backend_dev,
 goto err_free_bctx;
 }
 
+rdma_protected_gslist_append_int32(>cqe_ctx_list, bctx_id);
+
 rc = build_host_sge_array(rdma_dev_res, new_sge, sge, num_sge,
   _dev->rdma_dev_res->stats.rx_bufs_len);
 if (rc) {
@@ -762,6 +772,8 @@ int rdma_backend_create_qp(RdmaBackendQP *qp, uint8_t 
qp_type,
 return -EIO;
 }
 
+rdma_protected_gslist_init(>cqe_ctx_list);
+
 qp->ibpd = pd->ibpd;
 
 /* TODO: Query QP to get max_inline_data and save it to be used in send */
@@ -919,11 +931,13 @@ int rdma_backend_query_qp(RdmaBackendQP 

[Qemu-devel] [PATCH PULL 01/11] contrib/rdmacm-mux: Fix out-of-bounds risk

[Marcel Apfelbaum]

From: Yuval Shaia 

The function get_fd extract context from the received MAD message and
uses it as a key to fetch the destination fd from the mapping table.
A context can be dgid in case of CM request message or comm_id in case
of CM SIDR response message.

When MAD message with a smaller size as expected for the message type
received we are hitting out-of-bounds where we are looking for the
context out of message boundaries.

Fix it by validating the message size.

Reported-by Sam Smith 
Signed-off-by: Yuval Shaia 
Message-Id: <20190212112347.1605-1-yuval.sh...@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Marcel Apfelbaum 
---
 contrib/rdmacm-mux/main.c | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/contrib/rdmacm-mux/main.c b/contrib/rdmacm-mux/main.c
index ae88c77a1e..21cc804367 100644
--- a/contrib/rdmacm-mux/main.c
+++ b/contrib/rdmacm-mux/main.c
@@ -300,7 +300,7 @@ static void hash_tbl_remove_fd_ifid_pair(int fd)
 pthread_rwlock_unlock();
 }
 
-static int get_fd(const char *mad, int *fd, __be64 *gid_ifid)
+static int get_fd(const char *mad, int umad_len, int *fd, __be64 *gid_ifid)
 {
 struct umad_hdr *hdr = (struct umad_hdr *)mad;
 char *data = (char *)hdr + sizeof(*hdr);
@@ -308,13 +308,35 @@ static int get_fd(const char *mad, int *fd, __be64 
*gid_ifid)
 uint16_t attr_id = be16toh(hdr->attr_id);
 int rc = 0;
 
+if (umad_len <= sizeof(*hdr)) {
+rc = -EINVAL;
+syslog(LOG_DEBUG, "Ignoring MAD packets with header only\n");
+goto out;
+}
+
 switch (attr_id) {
 case UMAD_CM_ATTR_REQ:
+if (unlikely(umad_len < sizeof(*hdr) + CM_REQ_DGID_POS +
+sizeof(*gid_ifid))) {
+rc = -EINVAL;
+syslog(LOG_WARNING,
+   "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+attr_id);
+goto out;
+}
 memcpy(gid_ifid, data + CM_REQ_DGID_POS, sizeof(*gid_ifid));
 rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
 break;
 
 case UMAD_CM_ATTR_SIDR_REQ:
+if (unlikely(umad_len < sizeof(*hdr) + CM_SIDR_REQ_DGID_POS +
+sizeof(*gid_ifid))) {
+rc = -EINVAL;
+syslog(LOG_WARNING,
+   "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+attr_id);
+goto out;
+}
 memcpy(gid_ifid, data + CM_SIDR_REQ_DGID_POS, sizeof(*gid_ifid));
 rc = hash_tbl_search_fd_by_ifid(fd, gid_ifid);
 break;
@@ -331,6 +353,13 @@ static int get_fd(const char *mad, int *fd, __be64 
*gid_ifid)
 data += sizeof(comm_id);
 /* Fall through */
 case UMAD_CM_ATTR_SIDR_REP:
+if (unlikely(umad_len < sizeof(*hdr) + sizeof(comm_id))) {
+rc = -EINVAL;
+syslog(LOG_WARNING,
+   "Invalid MAD packet size (%d) for attr_id 0x%x\n", umad_len,
+   attr_id);
+goto out;
+}
 memcpy(_id, data, sizeof(comm_id));
 if (comm_id) {
 rc = hash_tbl_search_fd_by_comm_id(comm_id, fd, gid_ifid);
@@ -344,6 +373,7 @@ static int get_fd(const char *mad, int *fd, __be64 
*gid_ifid)
 
 syslog(LOG_DEBUG, "mad_to_vm: %d 0x%x 0x%x\n", *fd, attr_id, comm_id);
 
+out:
 return rc;
 }
 
@@ -372,7 +402,8 @@ static void *umad_recv_thread_func(void *args)
 } while (rc && server.run);
 
 if (server.run) {
-rc = get_fd(msg.umad.mad, , _id);
+rc = get_fd(msg.umad.mad, msg.umad_len, ,
+_id);
 if (rc) {
 continue;
 }
-- 
2.17.1

[Qemu-devel] [PATCH PULL 06/11] hw/rdma: Free all MAD receive buffers when device is closed

[Marcel Apfelbaum]

From: Yuval Shaia 

When device is going down free all saved MAD buffers.

Signed-off-by: Yuval Shaia 
Reviewed-by: Marcel Apfelbaum 
Message-Id: <20190213065357.16076-7-yuval.sh...@oracle.com>
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/rdma_backend.c| 34 +-
 hw/rdma/vmw/pvrdma_main.c |  2 ++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 0fb4842970..e8ee205b5f 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -64,6 +64,33 @@ static inline void complete_work(enum ibv_wc_status status, 
uint32_t vendor_err,
 comp_handler(ctx, );
 }
 
+static void free_cqe_ctx(gpointer data, gpointer user_data)
+{
+BackendCtx *bctx;
+RdmaDeviceResources *rdma_dev_res = user_data;
+unsigned long cqe_ctx_id = GPOINTER_TO_INT(data);
+
+bctx = rdma_rm_get_cqe_ctx(rdma_dev_res, cqe_ctx_id);
+if (bctx) {
+rdma_rm_dealloc_cqe_ctx(rdma_dev_res, cqe_ctx_id);
+}
+g_free(bctx);
+}
+
+static void clean_recv_mads(RdmaBackendDev *backend_dev)
+{
+unsigned long cqe_ctx_id;
+
+do {
+cqe_ctx_id = rdma_protected_qlist_pop_int64(_dev->
+recv_mads_list);
+if (cqe_ctx_id != -ENOENT) {
+free_cqe_ctx(GINT_TO_POINTER(cqe_ctx_id),
+ backend_dev->rdma_dev_res);
+}
+} while (cqe_ctx_id != -ENOENT);
+}
+
 static int rdma_poll_cq(RdmaDeviceResources *rdma_dev_res, struct ibv_cq *ibcq)
 {
 int i, ne, total_ne = 0;
@@ -1037,6 +1064,11 @@ static int mad_init(RdmaBackendDev *backend_dev, 
CharBackend *mad_chr_be)
 return 0;
 }
 
+static void mad_stop(RdmaBackendDev *backend_dev)
+{
+clean_recv_mads(backend_dev);
+}
+
 static void mad_fini(RdmaBackendDev *backend_dev)
 {
 disable_rdmacm_mux_async(backend_dev);
@@ -1224,12 +1256,12 @@ void rdma_backend_start(RdmaBackendDev *backend_dev)
 
 void rdma_backend_stop(RdmaBackendDev *backend_dev)
 {
+mad_stop(backend_dev);
 stop_backend_thread(_dev->comp_thread);
 }
 
 void rdma_backend_fini(RdmaBackendDev *backend_dev)
 {
-rdma_backend_stop(backend_dev);
 mad_fini(backend_dev);
 g_hash_table_destroy(ah_hash);
 ibv_destroy_comp_channel(backend_dev->channel);
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 8ffe79ceca..90dc9b191b 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -361,6 +361,8 @@ static void pvrdma_fini(PCIDevice *pdev)
 
 pvrdma_qp_ops_fini();
 
+rdma_backend_stop(>backend_dev);
+
 rdma_rm_fini(>rdma_dev_res, >backend_dev,
  dev->backend_eth_device_name);
 
-- 
2.17.1

[Qemu-devel] [PATCH PULL 03/11] hw/rdma: Introduce protected qlist

[Marcel Apfelbaum]

From: Yuval Shaia 

To make code more readable move handling of protected list to a
rdma_utils

Signed-off-by: Yuval Shaia 
Message-Id: <20190213065357.16076-4-yuval.sh...@oracle.com>
Reviewed-by: Marcel Apfelbaum 
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/rdma_backend.c  | 20 +--
 hw/rdma/rdma_backend_defs.h |  8 ++--
 hw/rdma/rdma_utils.c| 39 +
 hw/rdma/rdma_utils.h|  9 +
 4 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 5f60856d19..6e9c4617da 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -527,9 +527,7 @@ static unsigned int save_mad_recv_buffer(RdmaBackendDev 
*backend_dev,
 bctx->up_ctx = ctx;
 bctx->sge = *sge;
 
-qemu_mutex_lock(_dev->recv_mads_list.lock);
-qlist_append_int(backend_dev->recv_mads_list.list, bctx_id);
-qemu_mutex_unlock(_dev->recv_mads_list.lock);
+rdma_protected_qlist_append_int64(_dev->recv_mads_list, bctx_id);
 
 return 0;
 }
@@ -913,23 +911,19 @@ static inline void build_mad_hdr(struct ibv_grh *grh, 
union ibv_gid *sgid,
 static void process_incoming_mad_req(RdmaBackendDev *backend_dev,
  RdmaCmMuxMsg *msg)
 {
-QObject *o_ctx_id;
 unsigned long cqe_ctx_id;
 BackendCtx *bctx;
 char *mad;
 
 trace_mad_message("recv", msg->umad.mad, msg->umad_len);
 
-qemu_mutex_lock(_dev->recv_mads_list.lock);
-o_ctx_id = qlist_pop(backend_dev->recv_mads_list.list);
-qemu_mutex_unlock(_dev->recv_mads_list.lock);
-if (!o_ctx_id) {
+cqe_ctx_id = rdma_protected_qlist_pop_int64(_dev->recv_mads_list);
+if (cqe_ctx_id == -ENOENT) {
 rdma_warn_report("No more free MADs buffers, waiting for a while");
 sleep(THR_POLL_TO);
 return;
 }
 
-cqe_ctx_id = qnum_get_uint(qobject_to(QNum, o_ctx_id));
 bctx = rdma_rm_get_cqe_ctx(backend_dev->rdma_dev_res, cqe_ctx_id);
 if (unlikely(!bctx)) {
 rdma_error_report("No matching ctx for req %ld", cqe_ctx_id);
@@ -994,8 +988,7 @@ static int mad_init(RdmaBackendDev *backend_dev, 
CharBackend *mad_chr_be)
 return -EIO;
 }
 
-qemu_mutex_init(_dev->recv_mads_list.lock);
-backend_dev->recv_mads_list.list = qlist_new();
+rdma_protected_qlist_init(_dev->recv_mads_list);
 
 enable_rdmacm_mux_async(backend_dev);
 
@@ -1010,10 +1003,7 @@ static void mad_fini(RdmaBackendDev *backend_dev)
 {
 disable_rdmacm_mux_async(backend_dev);
 qemu_chr_fe_disconnect(backend_dev->rdmacm_mux.chr_be);
-if (backend_dev->recv_mads_list.list) {
-qlist_destroy_obj(QOBJECT(backend_dev->recv_mads_list.list));
-qemu_mutex_destroy(_dev->recv_mads_list.lock);
-}
+rdma_protected_qlist_destroy(_dev->recv_mads_list);
 }
 
 int rdma_backend_get_gid_index(RdmaBackendDev *backend_dev,
diff --git a/hw/rdma/rdma_backend_defs.h b/hw/rdma/rdma_backend_defs.h
index 15ae8b970e..a8c15b09ab 100644
--- a/hw/rdma/rdma_backend_defs.h
+++ b/hw/rdma/rdma_backend_defs.h
@@ -20,6 +20,7 @@
 #include "chardev/char-fe.h"
 #include 
 #include "contrib/rdmacm-mux/rdmacm-mux.h"
+#include "rdma_utils.h"
 
 typedef struct RdmaDeviceResources RdmaDeviceResources;
 
@@ -30,11 +31,6 @@ typedef struct RdmaBackendThread {
 bool is_running; /* Set by the thread to report its status */
 } RdmaBackendThread;
 
-typedef struct RecvMadList {
-QemuMutex lock;
-QList *list;
-} RecvMadList;
-
 typedef struct RdmaCmMux {
 CharBackend *chr_be;
 int can_receive;
@@ -48,7 +44,7 @@ typedef struct RdmaBackendDev {
 struct ibv_context *context;
 struct ibv_comp_channel *channel;
 uint8_t port_num;
-RecvMadList recv_mads_list;
+RdmaProtectedQList recv_mads_list;
 RdmaCmMux rdmacm_mux;
 } RdmaBackendDev;
 
diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c
index f1c980c6be..672a09079a 100644
--- a/hw/rdma/rdma_utils.c
+++ b/hw/rdma/rdma_utils.c
@@ -14,6 +14,8 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qnum.h"
 #include "trace.h"
 #include "rdma_utils.h"
 
@@ -55,3 +57,40 @@ void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, 
dma_addr_t len)
 pci_dma_unmap(dev, buffer, len, DMA_DIRECTION_TO_DEVICE, 0);
 }
 }
+
+void rdma_protected_qlist_init(RdmaProtectedQList *list)
+{
+qemu_mutex_init(>lock);
+list->list = qlist_new();
+}
+
+void rdma_protected_qlist_destroy(RdmaProtectedQList *list)
+{
+if (list->list) {
+qlist_destroy_obj(QOBJECT(list->list));
+qemu_mutex_destroy(>lock);
+list->list = NULL;
+}
+}
+
+void rdma_protected_qlist_append_int64(RdmaProtectedQList *list, int64_t value)
+{
+qemu_mutex_lock(>lock);
+qlist_append_int(list->list, value);
+qemu_mutex_unlock(>lock);
+}
+
+int64_t rdma_protected_qlist_pop_int64(RdmaProtectedQList *list)
+{
+QObject *obj;
+
+qemu_mutex_lock(>lock);
+obj = 

[Qemu-devel] [PATCH PULL 09/11] hw/pvrdma: Delete pvrdma_exit function

[Marcel Apfelbaum]

From: Yuval Shaia 

This hook is not called and was implemented by mistake.

Signed-off-by: Yuval Shaia 
Reviewed-by: Marcel Apfelbaum 
Message-Id: <20190213065357.16076-10-yuval.sh...@oracle.com>
Signed-off-by: Marcel Apfelbaum 
---
 hw/rdma/vmw/pvrdma_main.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 90dc9b191b..8b379c6435 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -701,18 +701,12 @@ out:
 }
 }
 
-static void pvrdma_exit(PCIDevice *pdev)
-{
-pvrdma_fini(pdev);
-}
-
 static void pvrdma_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
 k->realize = pvrdma_realize;
-k->exit = pvrdma_exit;
 k->vendor_id = PCI_VENDOR_ID_VMWARE;
 k->device_id = PCI_DEVICE_ID_VMWARE_PVRDMA;
 k->revision = 0x00;
-- 
2.17.1

[Qemu-devel] [PATCH PULL 00/11] RDMA queue

[Marcel Apfelbaum]

The following changes since commit 8eb29f1bf5a974dc4c11d2d1f5e7c7f7a62be116:

  Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20190221.0' 
into staging (2019-02-22 15:48:04 +)

are available in the Git repository at:

  https://github.com/marcel-apf/qemu tags/rdma-pull-request

for you to fetch changes up to e1038e22f7b3ade2e46cf168fa7486414c424e94:

  hw/rdma: another clang compilation fix (2019-02-23 15:28:25 +0200)


RDMA queue

 * Another Clang compilation fix
 * Various fixes for the pvrdma device


Marcel Apfelbaum (1):
  hw/rdma: another clang compilation fix

Yuval Shaia (10):
  contrib/rdmacm-mux: Fix out-of-bounds risk
  hw/rdma: Switch to generic error reporting way
  hw/rdma: Introduce protected qlist
  hw/rdma: Protect against concurrent execution of poll_cq
  {monitor, hw/pvrdma}: Expose device internals via monitor interface
  hw/rdma: Free all MAD receive buffers when device is closed
  hw/rdma: Free all receive buffers when QP is destroyed
  hw/pvrdma: Delete unneeded function argument
  hw/pvrdma: Delete pvrdma_exit function
  hw/pvrdma: Unregister from shutdown notifier when device goes down

 contrib/rdmacm-mux/main.c |  35 ++-
 hmp-commands-info.hx  |  16 ++
 hw/rdma/rdma_backend.c| 483 +-
 hw/rdma/rdma_backend.h|   3 +-
 hw/rdma/rdma_backend_defs.h   |  10 +-
 hw/rdma/rdma_rm.c | 134 ++--
 hw/rdma/rdma_rm_defs.h|  32 ++-
 hw/rdma/rdma_utils.c  |  79 ++-
 hw/rdma/rdma_utils.h  |  61 +++---
 hw/rdma/trace-events  |  32 ++-
 hw/rdma/vmw/pvrdma.h  |   7 +-
 hw/rdma/vmw/pvrdma_cmd.c  | 113 +++---
 hw/rdma/vmw/pvrdma_dev_ring.c |  26 +--
 hw/rdma/vmw/pvrdma_hmp.h  |  21 ++
 hw/rdma/vmw/pvrdma_main.c | 217 +++
 hw/rdma/vmw/pvrdma_qp_ops.c   |  52 ++---
 hw/rdma/vmw/trace-events  |  16 +-
 monitor.c |  10 +
 18 files changed, 744 insertions(+), 603 deletions(-)
 create mode 100644 hw/rdma/vmw/pvrdma_hmp.h

-- 
2.17.1

Re: [Qemu-devel] [PATCH v6 1/7] vhost-user: Support transferring inflight buffer between qemu and backend

[Yongji Xie]

On Fri, 22 Feb 2019 at 22:54, Michael S. Tsirkin  wrote:
>
> On Fri, Feb 22, 2019 at 03:05:23PM +0800, Yongji Xie wrote:
> > On Fri, 22 Feb 2019 at 14:21, Michael S. Tsirkin  wrote:
> > >
> > > On Fri, Feb 22, 2019 at 10:47:03AM +0800, Yongji Xie wrote:
> > > > > > +
> > > > > > +To track inflight I/O, the queue region should be processed as 
> > > > > > follows:
> > > > > > +
> > > > > > +When receiving available buffers from the driver:
> > > > > > +
> > > > > > +1. Get the next available head-descriptor index from available 
> > > > > > ring, i
> > > > > > +
> > > > > > +2. Set desc[i].inflight to 1
> > > > > > +
> > > > > > +When supplying used buffers to the driver:
> > > > > > +
> > > > > > +1. Get corresponding used head-descriptor index, i
> > > > > > +
> > > > > > +2. Set desc[i].next to process_head
> > > > > > +
> > > > > > +3. Set process_head to i
> > > > > > +
> > > > > > +4. Steps 1,2,3 may be performed repeatedly if batching is 
> > > > > > possible
> > > > > > +
> > > > > > +5. Increase the idx value of used ring by the size of the batch
> > > > > > +
> > > > > > +6. Set the inflight field of each DescStateSplit entry in the 
> > > > > > batch to 0
> > > > > > +
> > > > > > +7. Set used_idx to the idx value of used ring
> > > > > > +
> > > > > > +When reconnecting:
> > > > > > +
> > > > > > +1. If the value of used_idx does not match the idx value of 
> > > > > > used ring,
> > > > > > +
> > > > > > +(a) Subtract the value of used_idx from the idx value of 
> > > > > > used ring to get
> > > > > > +the number of in-progress DescStateSplit entries
> > > > > > +
> > > > > > +(b) Set the inflight field of the in-progress 
> > > > > > DescStateSplit entries which
> > > > > > +start from process_head to 0
> > > > > > +
> > > > > > +(c) Set used_idx to the idx value of used ring
> > > > > > +
> > > > > > +2. Resubmit each inflight DescStateSplit entry
> > > > >
> > > > > I re-read a couple of time and I still don't understand what it says.
> > > > >
> > > > > For simplicity consider split ring. So we want a list of heads that 
> > > > > are
> > > > > outstanding. Fair enough. Now device finishes a head. What now? I 
> > > > > needs
> > > > > to drop head from the list. But list is unidirectional (just next, no
> > > > > prev). So how can you drop an entry from the middle?
> > > > >
> > > >
> > > > The process_head is only used when slave crash between increasing the
> > > > idx value of used ring and updating used_idx. We use it to find the
> > > > in-progress DescStateSplit entries before the crash and complete them
> > > > when reconnecting. Make sure guest and slave have the same view for
> > > > inflight I/Os.
> > > >
> > >
> > > But I don't understand how does the described process help do it?
> > >
> >
> > For example, we need to submit descriptors A, B, C to driver in a batch.
> >
> > Firstly, we will link those descriptors like:
> >
> > process_head->A->B->C(A)
> >
> > Then, we need to update idx value of used vring to mark those
> > descriptors as used:
> >
> > _vring.used->idx += 3(B)
> >
> > At last, clear the inflight field of those descriptors and update
> > used_idx field:
> >
> > A.inflight = 0; B.inflight = 0; C.inflight = 0;(C)
> >
> > used_idx = _vring.used->idx;(D)
> >
> > After (B), guest can consume the descriptors A,B,C. So we must make
> > sure the inflight field of A,B,C is cleared when reconnecting to avoid
> > re-submitting used descriptor. If slave crash during (C), the inflight
> > field of A,B,C may be incorrect. To detect that case, we can see
> > whether used_idx matches _vring.used->idx. And through process_head,
> > we can get the in-progress descriptors A,B,C and clear their inflight
> > field again when reconnecting.
> >
> > >
> > > > In other case, the inflight field is enough to track inflight I/O.
> > > > When reconnecting, we go through all DescStateSplit entries and
> > > > re-submit the entry whose inflight field is equal to 1.
> > >
> > > What I don't understand is how do we know the order
> > > in which they have to be resubmitted. Reordering
> > > operations would be a big problem, won't it?
> > >
> >
> > In previous patch, I record avail_idx for each DescStateSplit entry to
> > preserve the order. Is it useful to fix this?
> >
> > >
> > > Let's say I fetch descriptors A, B, C and start
> > > processing. how does memory look?
> >
> > A.inflight = 1, C.inflight = 1, B.inflight = 1
> >
> > > Now I finished B and marked it used. How does
> > > memory look?
> > >
> >
> > A.inflight = 1, C.inflight = 1, B.inflight = 0, process_head = B
>
> OK. And we have
>
> process_head->B->process_head
>
> ?
>
> Now if there is a reconnect, I want to submit A and then C,
> correct? How do I know that from this picture? How do I
> know to start with A? It's not on the list anymore...
>

We can go through all DescStateSplit entries (track all descriptors in
Descriptor 

Re: [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU

[Peter Maydell]

On Fri, 22 Feb 2019 at 21:48, Roman Bolshakov  wrote:
>
> On Fri, Feb 22, 2019 at 03:41:05PM +, Peter Maydell wrote:
> > On Fri, 22 Feb 2019 at 15:19, Roman Bolshakov  wrote:
> > >
> > > On Thu, Feb 14, 2019 at 10:28:10AM +, Peter Maydell wrote:
> > > > The Cocoa UI should run on the main thread; this is enforced
> > > > in OSX Mojave. In order to be able to run on the main thread,
> > > > we need to make sure we hold the iothread lock whenever we
> > > > call into various QEMU UI midlayer functions.
> > > >
> > >
> > > I also think it's better to clarify that the reason of the commit is not
> > > Mojave enforcing usage of event loop in main thread but an improvement
> > > of event processing in Cocoa UI, because Cocoa UI works on Mojave.
> >
> > Hmm? The point of this patchset is exactly that Mojave enforces
> > that things go on the main thread, where previous OSX versions
> > did not, and so in some situations QEMU will crash on Mojave
> > where it did not on older versions. So I'm not sure what you're
> > suggesting should be clarified here.
> >
>
> I'm not exactly sure there's an issue with QEMU on Mojave. But I lean
> towards the opinion because I haven't seen it :)

It only happens for some guest workloads. The "usual" case
is that the cocoa_refresh callback is called from the QEMU
main loop, which happens to be on the OSX main thread, which
means OSX is still happy. But in some cases cocoa_refresh can
be called from a guest vCPU thread -- I think we've seen this
when a guest initiates a screen resolution change: the call
from the guest vCPU thread goes into the model of the graphics
device, which makes a call into the UI midlayer to say
"resolution changed", which immediately triggers a
refresh callback to the UI frontend layer from that thread.
In Mojave this causes OSX to terminate QEMU. I think in
older OSX versions it would probably be a race condition,
so it's technically a bug but not one that usually has
any visible bad effects; it's only surfaced as a problem
now that Mojave actively checks for this condition and
kills the process.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-block] Guest unresponsive after Virtqueue size exceeded error

[Natanael Copa]

On Fri, 22 Feb 2019 14:04:20 +
Stefan Hajnoczi  wrote:

> On Fri, Feb 22, 2019 at 12:57 PM Fernando Casas Schössow
>  wrote:
> 
> I have CCed Natanael Copa, qemu package maintainer in Alpine Linux.

Hi!

...

> Richard: Commit 7db2145a6826b14efceb8dd64bfe6ad8647072eb ("bswap: Add
> host endian unaligned access functions") introduced the unaligned
> memory access functions in question here.  Please see below for
> details on the bug - basically QEMU code assumes they are atomic, but
> that is not guaranteed :(.  Any ideas for how to fix this?
> 
> Natanael: It seems likely that the qemu package in Alpine Linux
> suffers from a compilation issue resulting in a broken QEMU.  It may
> be necessary to leave the compiler optimization flag alone in APKBUILD
> to work around this problem.
> 
> Here are the details.  QEMU relies on the compiler turning
> memcpy(, , 2) turning into a load instruction in
> include/qemu/bswap.h:lduw_he_p() (explanation below):
> 
> /* Any compiler worth its salt will turn these memcpy into native unaligned
>operations.  Thus we don't need to play games with packed attributes, or
>inline byte-by-byte stores.  */
> 
> static inline int lduw_he_p(const void *ptr)
> {
> uint16_t r;
> memcpy(, ptr, sizeof(r));
> return r;
> }
> 
> Here is the disassembly snippet of virtqueue_pop() from Fedora 29 that
> shows the load instruction:
> 
>   398166:   0f b7 42 02 movzwl 0x2(%rdx),%eax
>   39816a:   66 89 43 32 mov%ax,0x32(%rbx)
> 
> Here is the instruction sequence in the Alpine Linux binary:
> 
>   455562:ba 02 00 00 00   mov$0x2,%edx
>   455567:e8 74 24 f3 ff   callq  3879e0 
>   45556c:0f b7 44 24 42   movzwl 0x42(%rsp),%eax
>   455571:66 41 89 47 32   mov%ax,0x32(%r15)
> 
> It's calling memcpy instead of using a load instruction.

My first reaction to this is: If the intention is to not actually call
memcpy function, then maybe memcpy should not be used in the C code in
first place?
 
> Fernando found that QEMU's virtqueue_pop() function sees bogus values
> when loading a 16-bit guest RAM location.  Paolo figured out that the
> bogus value can be produced by memcpy() when another thread is
> updating the 16-bit memory location simultaneously.  This is a race
> condition between one thread loading the 16-bit value and another
> thread storing it (in this case a guest vcpu thread).  Sometimes
> memcpy() may load one old byte and one new byte, resulting in a bogus
> value.
> 
> The symptom that Fernando experienced is a "Virtqueue size exceeded"
> error message from QEMU and then the virtio-blk or virtio-scsi device
> stops working.  This issue potentially affects other device emulation
> code in QEMU as well, not just virtio devices.
> 
> For the time being, I suggest tweaking the APKBUILD so the memcpy() is
> not generated.  Hopefully QEMU can make the code more portable in the
> future so the compiler always does the expected thing, but this may
> not be easily possible.

I suspect this happens due to the Alpine toolchain will enable
_FORTIFY_SOURCE=2 by default and the way this is implemented via 
fortify-headers:
http://git.2f30.org/fortify-headers/file/include/string.h.html#l39

Try build with -U_FORTIFY_SOURCE

> 
> Stefan

Re: [Qemu-devel] [PATCH v2 02/25] hw/arm: Express dependencies of the highbank machines with Kconfig

[Philippe Mathieu-Daudé]

On 2/15/19 1:15 PM, Thomas Huth wrote:
> On 14/02/2019 21.35, Paolo Bonzini wrote:
>> On 14/02/19 20:17, Peter Maydell wrote:
>>> On Wed, 13 Feb 2019 at 08:38, Thomas Huth  wrote:

 Add Kconfig dependencies for the highbank machine (and the midway
 machine).
 This patch is slightly based on earlier work by Ákos Kovács (i.e.
 his "hw/arm/Kconfig: Add ARM Kconfig" patch).

 Signed-off-by: Thomas Huth 
 ---
  default-configs/arm-softmmu.mak |  4 +---
  hw/arm/Kconfig  | 12 
  2 files changed, 13 insertions(+), 3 deletions(-)

 diff --git a/default-configs/arm-softmmu.mak 
 b/default-configs/arm-softmmu.mak
 index 3baafc4..59734ee 100644
 --- a/default-configs/arm-softmmu.mak
 +++ b/default-configs/arm-softmmu.mak
 @@ -6,6 +6,7 @@ CONFIG_ARM_V7M=y
  CONFIG_PCI_DEVICES=y

  CONFIG_EXYNOS4=y
 +CONFIG_HIGHBANK=y

  CONFIG_VGA=y
  CONFIG_NAND=y
 @@ -54,14 +55,12 @@ CONFIG_PL022=y
  CONFIG_PL031=y
  CONFIG_PL041=y
  CONFIG_PL050=y
 -CONFIG_PL061=y
  CONFIG_PL080=y
  CONFIG_PL110=y
  CONFIG_PL181=y
  CONFIG_PL190=y
  CONFIG_PL330=y
  CONFIG_CADENCE=y
 -CONFIG_XGMAC=y
>>>
>>> Could you explain the logic for when CONFIG_*=y
>>> lines get deleted from the arm-softmmu.mak?
>>> In this patch PL061 has been deleted, but PL011,
>>> PL022, PL031 have not, though all these devices are
>>> used in both Highbank and in other not-yet-converted
>>> machine types. What's the difference ?
> 
> My plan was indeed to remove the switches from the default-configs file
> as soon as they are enabled in the Kconfig file. Looks like I missed
> these here in this patch, sorry!
> 
> (but unless there are other reasons for respinning, I think it should
> also be OK if the switches get removed in later patches instead)
> 
>> I think it's a mistake.  I'd not remove any CONFIG_*=y except in a final
>> separate patch.
> 
> Really? In that case it's way harder to see which options from the
> default-configs are really handled in the Kconfig files - and there's a
> risk that some options would get removed that are not enabled in any of
> the preceeding Kconfig patches. So I think it's better this way, bit by bit.

Agreed. This way you can build/test at each step.

[Qemu-devel] [PATCH 0/1] update copyright notice

[David Kiarie]

update copyright notice to reflect my full legal name. looks better to
me that way.

also, that way people are not under the impression i *own* qemu AMD
IOMMU.

thanks.

David Kiarie (1):
  hw/i386: update copyright notice

 hw/i386/amd_iommu.c | 2 +-
 hw/i386/amd_iommu.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/1] hw/i386: update copyright notice

[David Kiarie]

Signed-off-by: David Kiarie 
---
 hw/i386/amd_iommu.c | 2 +-
 hw/i386/amd_iommu.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 8ad707a..4f179da 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -2,7 +2,7 @@
  * QEMU emulation of AMD IOMMU (AMD-Vi)
  *
  * Copyright (C) 2011 Eduard - Gabriel Munteanu
- * Copyright (C) 2015 David Kiarie, 
+ * Copyright (C) 2016 David Kiarie Kahurani
  *
  * 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
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index c52886f..cb258a7 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -2,7 +2,7 @@
  * QEMU emulation of an AMD IOMMU (AMD-Vi)
  *
  * Copyright (C) 2011 Eduard - Gabriel Munteanu
- * Copyright (C) 2015 David Kiarie, 
+ * Copyright (C) 2016 David Kiarie Kahurani
  *
  * 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
-- 
2.20.1

Re: [Qemu-devel] [PATCH v1 6/6] softfloat: Support float_round_to_odd more places

[Alex Bennée]

Richard Henderson  writes:

> On 2/22/19 12:45 PM, Alex Bennée wrote:
>> @@ -3526,6 +3551,8 @@ static float32 roundAndPackFloat32(flag zSign, int 
>> zExp, uint32_t zSig,
>>  case float_round_down:
>>  roundIncrement = zSign ? 0x7f : 0;
>>  break;
>> +case float_round_to_odd:
>> +roundIncrement = zSig & 0x80 ? 0 : 0x7f;
>>  default:
>>  abort();
>
> I clearly missed a break here.
>
> Since this didn't kill fp-test, are we missing a float128_to_float32
> test?

So I've not got -r all against all tests but adding it to make mulAdd
take even longer to run so maybe I'll not bother for mulAdd.

>
>
> r~


--
Alex Bennée

Re: [Qemu-devel] [PATCH v1 6/6] softfloat: Support float_round_to_odd more places

[Alex Bennée]

Richard Henderson  writes:

> On 2/22/19 12:45 PM, Alex Bennée wrote:
>> @@ -3526,6 +3551,8 @@ static float32 roundAndPackFloat32(flag zSign, int 
>> zExp, uint32_t zSig,
>>  case float_round_down:
>>  roundIncrement = zSign ? 0x7f : 0;
>>  break;
>> +case float_round_to_odd:
>> +roundIncrement = zSig & 0x80 ? 0 : 0x7f;
>>  default:
>>  abort();
>
> I clearly missed a break here.
>
> Since this didn't kill fp-test, are we missing a float128_to_float32
> test?

Ahh yes missing that:

10:11:17 [alex@zen:~/l/q/b/a/t/fp] fpu/next + ./fp-test f16_to_f32 f64_to_f32 
f128_to_f32 -r odd
>> Testing f16_to_f32
408 tests total.
408 tests performed.
In 408 tests, no errors found in f16_to_f32.
>> Testing f64_to_f32, rounding odd, tininess before rounding
768 tests total.
768 tests performed.
In 768 tests, no errors found in f64_to_f32, rounding odd, tininess before 
rounding.
>> Testing f64_to_f32, rounding odd, tininess after rounding
768 tests total.
768 tests performed.
In 768 tests, no errors found in f64_to_f32, rounding odd, tininess after 
rounding.
>> Testing f128_to_f32, rounding odd, tininess before rounding
936 tests total.
fish: “./fp-test f16_to_f32 f64_to_f32…” terminated by signal SIGABRT (Abort)

I'll patch it up and fix.

>
>
> r~


--
Alex Bennée

Re: [Qemu-devel] [PATCH v1 4/6] tests/fp: add wrapping for f128_to_ui32

[Alex Bennée]

Richard Henderson  writes:

> On 2/22/19 12:44 PM, Alex Bennée wrote:
>> Needed to test: softfloat: add float128_is_{normal,denormal}

Obviously I was reading the title of the other patch.. will fix.

>
> Eh?
>
>>
>> Signed-off-by: Alex Bennée 
>> ---
>>  tests/fp/fp-test.c  | 3 ++-
>>  tests/fp/wrap.inc.c | 1 +
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> But the rest of the patch matches $SUBJECT, so
> Reviewed-by: Richard Henderson 
>
>
> r~


--
Alex Bennée

Re: [Qemu-devel] [PATCH v2] iotests: handle TypeError for Python3 in test 242

[Vladimir Sementsov-Ogievskiy]

22.02.2019 14:26, Andrey Shinkevich wrote:
> The data type for bytes in Python3 differs from the one in Python2.
> Those cases should be managed separately.
> 
> v1:
> In the first version, the TypeError in Python3 was handled as the
> exception.
> Discussed in the e-mail thread with the Message ID:
> <1550519997-253534-1-git-send-email-andrey.shinkev...@virtuozzo.com>
> 
> Signed-off-by: Andrey Shinkevich 
> Reported-by: Kevin Wolf 
> ---
>   tests/qemu-iotests/242 | 8 ++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242
> index 16c65ed..446fbf8 100755
> --- a/tests/qemu-iotests/242
> +++ b/tests/qemu-iotests/242
> @@ -20,6 +20,7 @@
>   
>   import iotests
>   import json
> +import sys
>   from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
>   file_path, img_info_log, log, filter_qemu_io
>   
> @@ -65,9 +66,12 @@ def toggle_flag(offset):
>   with open(disk, "r+b") as f:
>   f.seek(offset, 0)
>   c = f.read(1)
> -toggled = chr(ord(c) ^ bitmap_flag_unknown)
> +toggled = ord(c) ^ bitmap_flag_unknown
>   f.seek(-1, 1)
> -f.write(toggled)
> +if sys.version_info.major >= 3:
> +f.write(bytes([toggled]))
> +else:
> +f.write(chr(toggled))
>   
>   
>   qemu_img_create('-f', iotests.imgfmt, disk, '1M')
> 

Reviewed-by: Vladimir Sementsov-Ogievskiy 

-- 
Best regards,
Vladimir

[Qemu-devel] [PATCH v3 30/30] hw/arm: Remove hard-enablement of the remaining PCI devices

[Thomas Huth]

The PCI devices should be pulled in by default if PCI_DEVICES
is set, so there is no need anymore to enforce them in the configs
file.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 8 
 1 file changed, 8 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index ee95cc0..6b1f6a8 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -33,14 +33,6 @@ CONFIG_DIGIC=y
 CONFIG_SABRELITE=y
 CONFIG_EMCRAFT_SF2=y
 CONFIG_MICROBIT=y
-
-CONFIG_VGA=y
-
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
 CONFIG_FSL_IMX6UL=y
-
-CONFIG_PCIE_PORT=y
-CONFIG_XIO3130=y
-CONFIG_IOH3420=y
-CONFIG_I82801B11=y
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 26/30] hw/arm: Express dependencies of the microbit / nrf51 machine with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the NRF51 / microbit machine.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 3 +--
 hw/arm/Kconfig  | 6 ++
 hw/arm/Makefile.objs| 3 ++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 88e4e8e..ee95cc0 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -32,11 +32,10 @@ CONFIG_RASPI=y
 CONFIG_DIGIC=y
 CONFIG_SABRELITE=y
 CONFIG_EMCRAFT_SF2=y
+CONFIG_MICROBIT=y
 
 CONFIG_VGA=y
 
-CONFIG_NRF51_SOC=y
-
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
 CONFIG_FSL_IMX6UL=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 3eee2bd..5ce6245 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -362,8 +362,14 @@ config FSL_IMX6UL
 select IMX_I2C
 select SDHCI
 
+config MICROBIT
+bool
+select NRF51_SOC
+
 config NRF51_SOC
 bool
+select I2C
+select ARM_V7M
 
 config EMCRAFT_SF2
 bool
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index eae9f6c..994e67d 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -8,6 +8,7 @@ obj-$(CONFIG_EMCRAFT_SF2) += msf2-som.o
 obj-$(CONFIG_HIGHBANK) += highbank.o
 obj-$(CONFIG_INTEGRATOR) += integratorcp.o
 obj-$(CONFIG_MAINSTONE) += mainstone.o
+obj-$(CONFIG_MICROBIT) += microbit.o
 obj-$(CONFIG_MUSICPAL) += musicpal.o
 obj-$(CONFIG_NETDUINO2) += netduino2.o
 obj-$(CONFIG_NSERIES) += nseries.o
@@ -48,4 +49,4 @@ obj-$(CONFIG_ARMSSE) += armsse.o
 obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
 obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o smmuv3.o
 obj-$(CONFIG_FSL_IMX6UL) += fsl-imx6ul.o mcimx6ul-evk.o
-obj-$(CONFIG_NRF51_SOC) += nrf51_soc.o microbit.o
+obj-$(CONFIG_NRF51_SOC) += nrf51_soc.o
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 27/30] hw/arm: Express dependencies of the ZynqMP zcu102 machine with Kconfig

[Thomas Huth]

This cleans up most settings in default-configs/aarch64-softmmu.mak.

Signed-off-by: Thomas Huth 
---
 default-configs/aarch64-softmmu.mak |  4 
 hw/arm/Kconfig  | 11 +++
 hw/display/Kconfig  |  1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/default-configs/aarch64-softmmu.mak 
b/default-configs/aarch64-softmmu.mak
index 4ea9add..3a4b15e 100644
--- a/default-configs/aarch64-softmmu.mak
+++ b/default-configs/aarch64-softmmu.mak
@@ -3,10 +3,6 @@
 # We support all the 32 bit boards so need all their config
 include arm-softmmu.mak
 
-CONFIG_AUX=y
-CONFIG_DDC=y
-CONFIG_DPCD=y
-CONFIG_XLNX_ZYNQMP=y
 CONFIG_XLNX_ZYNQMP_ARM=y
 CONFIG_XLNX_VERSAL=y
 CONFIG_ARM_SMMUV3=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 5ce6245..744b5ff 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -289,6 +289,17 @@ config STM32F205_SOC
 
 config XLNX_ZYNQMP_ARM
 bool
+select AHCI
+select ARM_GIC
+select CADENCE
+select DDC
+select DPCD
+select SDHCI
+select SSI
+select SSI_M25P80
+select XILINX_AXI
+select XILINX_SPIPS
+select XLNX_ZYNQMP
 
 config XLNX_VERSAL
 bool
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index a5f3fc0..d2ae2f1 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -108,3 +108,4 @@ config VIRTIO_VGA
 
 config DPCD
 bool
+select AUX
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 28/30] hw/arm: Express dependencies of the xlnx-versal-virt machine with Kconfig

[Thomas Huth]

Dependencies have been determined with trial-and-error and by
looking at the xlnx-versal.c source file.

Signed-off-by: Thomas Huth 
---
 hw/arm/Kconfig | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 744b5ff..06e9e9a 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -303,6 +303,10 @@ config XLNX_ZYNQMP_ARM
 
 config XLNX_VERSAL
 bool
+select ARM_GIC
+select PL011
+select CADENCE
+select VIRTIO_MMIO
 
 config FSL_IMX25
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 24/30] hw/arm: Express dependencies of the MSF2 / EMCRAFT_SF2 machine with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the emcraft-sf2 machine - we also
distinguish between the machine (CONFIG_EMCRAFT_SF2) and the SoC
(CONFIG_MSF2) now.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak |  3 +--
 hw/arm/Kconfig  | 12 ++--
 hw/arm/Makefile.objs|  3 ++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 9150858..a01e407 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -31,9 +31,9 @@ CONFIG_MPS2=y
 CONFIG_RASPI=y
 CONFIG_DIGIC=y
 CONFIG_SABRELITE=y
+CONFIG_EMCRAFT_SF2=y
 
 CONFIG_VGA=y
-CONFIG_SSI_M25P80=y
 CONFIG_IMX_FEC=y
 
 CONFIG_NRF51_SOC=y
@@ -46,5 +46,4 @@ CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
-CONFIG_MSF2=y
 CONFIG_PCI_EXPRESS_DESIGNWARE=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 8418047..9495e7f 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -331,7 +331,6 @@ config MPS2
 select MPS2_SCC
 select PL022# Serial port
 select PL080# DMA controller
-select TZ_MPC
 
 config IOTKIT
 bool
@@ -348,9 +347,17 @@ config FSL_IMX6UL
 config NRF51_SOC
 bool
 
+config EMCRAFT_SF2
+bool
+select MSF2
+select SSI_M25P80
+
 config MSF2
 bool
+select ARM_V7M
 select PTIMER
+select SERIAL
+select SSI
 
 config ZAURUS
 bool
@@ -375,13 +382,14 @@ config ARM11MPCORE
 config ARMSSE
 bool
 select ARMSSE_CPUID
-select CMSDK_APB_TIMER
 select CMSDK_APB_DUALTIMER
+select CMSDK_APB_TIMER
 select CMSDK_APB_UART
 select CMSDK_APB_WATCHDOG
 select IOTKIT_SECCTL
 select IOTKIT_SYSCTL
 select IOTKIT_SYSINFO
+select TZ_MPC
 select TZ_MSC
 select TZ_PPC
 
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index fadd698..eae9f6c 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -4,6 +4,7 @@ obj-$(CONFIG_ARM_VIRT) += virt.o
 obj-$(CONFIG_ACPI) += virt-acpi-build.o
 obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
+obj-$(CONFIG_EMCRAFT_SF2) += msf2-som.o
 obj-$(CONFIG_HIGHBANK) += highbank.o
 obj-$(CONFIG_INTEGRATOR) += integratorcp.o
 obj-$(CONFIG_MAINSTONE) += mainstone.o
@@ -41,7 +42,7 @@ obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
 obj-$(CONFIG_MPS2) += mps2.o
 obj-$(CONFIG_MPS2) += mps2-tz.o
-obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
+obj-$(CONFIG_MSF2) += msf2-soc.o
 obj-$(CONFIG_MUSCA) += musca.o
 obj-$(CONFIG_ARMSSE) += armsse.o
 obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 29/30] hw/arm: Express dependencies of the musca machines with Kconfig

[Thomas Huth]

Dependencies have been determined with trial-and-error and by
looking at the musca.c source file.

Signed-off-by: Thomas Huth 
---
 hw/arm/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 06e9e9a..8105587 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -79,6 +79,9 @@ config MAINSTONE
 
 config MUSCA
 bool
+select ARMSSE
+select PL011
+select PL031
 
 config MUSICPAL
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 21/30] hw/arm: Express dependencies of the raspi machines with Kconfig

[Thomas Huth]

Most of the code is directly controlled by the CONFIG_RASPI switch,
so not much to add here additionally.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 4 +---
 hw/arm/Kconfig  | 3 +++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index b8509fd..290023c 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -28,15 +28,13 @@ CONFIG_COLLIE=y
 CONFIG_ASPEED_SOC=y
 CONFIG_NETDUINO2=y
 CONFIG_MPS2=y
+CONFIG_RASPI=y
 
 CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
 CONFIG_IMX_FEC=y
 
-CONFIG_FRAMEBUFFER=y
-
 CONFIG_DIGIC=y
-CONFIG_RASPI=y
 CONFIG_NRF51_SOC=y
 
 CONFIG_FSL_IMX6=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index fa69f1b..32495e6 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -269,6 +269,9 @@ config ALLWINNER_A10
 
 config RASPI
 bool
+select FRAMEBUFFER
+select PL011 # UART
+select SDHCI
 
 config STM32F205_SOC
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 19/30] hw/arm: Express dependencies of allwinner / cubieboard with Kconfig

[Thomas Huth]

Add dependencies for the Cubitech Cubieboard.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 6 +-
 hw/arm/Kconfig  | 9 +
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index b0c6c99..badb190 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -4,6 +4,7 @@
 CONFIG_ARM_V7M=y
 
 CONFIG_ARM_VIRT=y
+CONFIG_CUBIEBOARD=y
 CONFIG_EXYNOS4=y
 CONFIG_HIGHBANK=y
 CONFIG_INTEGRATOR=y
@@ -29,7 +30,6 @@ CONFIG_NETDUINO2=y
 
 CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
-CONFIG_ALLWINNER_EMAC=y
 CONFIG_IMX_FEC=y
 
 CONFIG_FRAMEBUFFER=y
@@ -56,10 +56,6 @@ CONFIG_IOTKIT_SYSCTL=y
 CONFIG_IOTKIT_SYSINFO=y
 CONFIG_ARMSSE_CPUID=y
 
-CONFIG_ALLWINNER_A10_PIT=y
-CONFIG_ALLWINNER_A10_PIC=y
-CONFIG_ALLWINNER_A10=y
-
 CONFIG_FSL_IMX6=y
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 34243b4..73382b6 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -24,6 +24,10 @@ config CHEETAH
 select OMAP
 select TSC210X
 
+config CUBIEBOARD
+bool
+select ALLWINNER_A10
+
 config DIGIC
 bool
 select PTIMER
@@ -257,6 +261,11 @@ config EXYNOS4
 
 config ALLWINNER_A10
 bool
+select AHCI
+select ALLWINNER_A10_PIT
+select ALLWINNER_A10_PIC
+select ALLWINNER_EMAC
+select SERIAL
 
 config RASPI
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 25/30] hw/arm: Express dependencies for remaining IMX boards with Kconfig

[Thomas Huth]

IMX25, IMX7 and IMX6UL were still missing the Kconfig dependencies.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak |  2 --
 hw/arm/Kconfig  | 18 ++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index a01e407..88e4e8e 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -34,7 +34,6 @@ CONFIG_SABRELITE=y
 CONFIG_EMCRAFT_SF2=y
 
 CONFIG_VGA=y
-CONFIG_IMX_FEC=y
 
 CONFIG_NRF51_SOC=y
 
@@ -46,4 +45,3 @@ CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
-CONFIG_PCI_EXPRESS_DESIGNWARE=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 9495e7f..3eee2bd 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -295,6 +295,10 @@ config XLNX_VERSAL
 
 config FSL_IMX25
 bool
+select IMX
+select IMX_FEC
+select IMX_I2C
+select DS1338
 
 config FSL_IMX31
 bool
@@ -309,6 +313,7 @@ config FSL_IMX6
 select A9MPCORE
 select IMX
 select IMX_I2C
+select SDHCI
 
 config ASPEED_SOC
 bool
@@ -337,12 +342,25 @@ config IOTKIT
 
 config FSL_IMX7
 bool
+imply PCI_DEVICES
+select A15MPCORE
+select PCI
+select IMX
+select IMX_FEC
+select IMX_I2C
+select PCI_EXPRESS_DESIGNWARE
+select SDHCI
 
 config ARM_SMMUV3
 bool
 
 config FSL_IMX6UL
 bool
+select A15MPCORE
+select IMX
+select IMX_FEC
+select IMX_I2C
+select SDHCI
 
 config NRF51_SOC
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 18/30] hw/arm: Express dependencies of netduino / stm32f2xx with Kconfig

[Thomas Huth]

Netduino only depends on the stm32f205 SoC which in turn depends on
its components.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 9 +
 hw/arm/Kconfig  | 7 +++
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 62d6b96..b0c6c99 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -25,25 +25,18 @@ CONFIG_TOSA=y
 CONFIG_Z2=y
 CONFIG_COLLIE=y
 CONFIG_ASPEED_SOC=y
+CONFIG_NETDUINO2=y
 
 CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
 CONFIG_ALLWINNER_EMAC=y
 CONFIG_IMX_FEC=y
 
-CONFIG_NETDUINO2=y
-
 CONFIG_FRAMEBUFFER=y
 
 CONFIG_DIGIC=y
 CONFIG_MPS2=y
 CONFIG_RASPI=y
-CONFIG_STM32F2XX_TIMER=y
-CONFIG_STM32F2XX_USART=y
-CONFIG_STM32F2XX_SYSCFG=y
-CONFIG_STM32F2XX_ADC=y
-CONFIG_STM32F2XX_SPI=y
-CONFIG_STM32F205_SOC=y
 CONFIG_NRF51_SOC=y
 
 CONFIG_CMSDK_APB_TIMER=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 487fb03..34243b4 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -87,6 +87,7 @@ config MUSICPAL
 
 config NETDUINO2
 bool
+select STM32F205_SOC
 
 config NSERIES
 bool
@@ -262,6 +263,12 @@ config RASPI
 
 config STM32F205_SOC
 bool
+select ARM_V7M
+select STM32F2XX_TIMER
+select STM32F2XX_USART
+select STM32F2XX_SYSCFG
+select STM32F2XX_ADC
+select STM32F2XX_SPI
 
 config XLNX_ZYNQMP_ARM
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 16/30] hw/arm: Express dependencies of the aspeed boards with Kconfig

[Thomas Huth]

Dependencies have been determined by looking at hw/arm/aspeed.c

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak |  7 +--
 hw/arm/Kconfig  | 10 ++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 34725e1..476fe05 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -23,15 +23,12 @@ CONFIG_SPITZ=y
 CONFIG_TOSA=y
 CONFIG_Z2=y
 CONFIG_COLLIE=y
+CONFIG_ASPEED_SOC=y
 
 CONFIG_VGA=y
-CONFIG_TMP421=y
-CONFIG_PCA9552=y
 CONFIG_SSI_M25P80=y
 CONFIG_ALLWINNER_EMAC=y
 CONFIG_IMX_FEC=y
-CONFIG_FTGMAC100=y
-CONFIG_DS1338=y
 CONFIG_PLATFORM_BUS=y
 CONFIG_VIRTIO_MMIO=y
 
@@ -91,8 +88,6 @@ CONFIG_I82801B11=y
 CONFIG_ACPI=y
 CONFIG_ARM_VIRT=y
 CONFIG_SMBIOS=y
-CONFIG_ASPEED_SOC=y
-CONFIG_SMBUS_EEPROM=y
 CONFIG_GPIO_KEY=y
 CONFIG_MSF2=y
 CONFIG_FW_CFG_DMA=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index f15e433..32b3785 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -267,6 +267,16 @@ config FSL_IMX6
 
 config ASPEED_SOC
 bool
+select DS1338
+select FTGMAC100
+select I2C
+select PCA9552
+select SERIAL
+select SMBUS_EEPROM
+select SSI
+select SSI_M25P80
+select TMP105
+select TMP421
 
 config MPS2
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 22/30] hw/arm: Express dependencies of canon-a1100 with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the DIGIC / canon-a1100 machine.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 2 +-
 hw/arm/Kconfig  | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 290023c..53609ea 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -29,12 +29,12 @@ CONFIG_ASPEED_SOC=y
 CONFIG_NETDUINO2=y
 CONFIG_MPS2=y
 CONFIG_RASPI=y
+CONFIG_DIGIC=y
 
 CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
 CONFIG_IMX_FEC=y
 
-CONFIG_DIGIC=y
 CONFIG_NRF51_SOC=y
 
 CONFIG_FSL_IMX6=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 32495e6..dbdc02c 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -31,6 +31,7 @@ config CUBIEBOARD
 config DIGIC
 bool
 select PTIMER
+select PFLASH_CFI02
 
 config EXYNOS4
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 23/30] hw/arm: Express dependencies of sabrelite with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the Sabrelite / iMX6 machine.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 4 +---
 hw/arm/Kconfig  | 7 +++
 hw/arm/Makefile.objs| 3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 53609ea..9150858 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -30,6 +30,7 @@ CONFIG_NETDUINO2=y
 CONFIG_MPS2=y
 CONFIG_RASPI=y
 CONFIG_DIGIC=y
+CONFIG_SABRELITE=y
 
 CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
@@ -37,13 +38,10 @@ CONFIG_IMX_FEC=y
 
 CONFIG_NRF51_SOC=y
 
-CONFIG_FSL_IMX6=y
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
 CONFIG_FSL_IMX6UL=y
 
-CONFIG_IMX_I2C=y
-
 CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index dbdc02c..8418047 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -184,6 +184,10 @@ config REALVIEW
 select DS1338 # I2C RTC+NVRAM
 select USB_OHCI
 
+config SABRELITE
+bool
+select FSL_IMX6
+
 config STELLARIS
 bool
 select ARM_V7M
@@ -302,6 +306,9 @@ config FSL_IMX31
 
 config FSL_IMX6
 bool
+select A9MPCORE
+select IMX
+select IMX_I2C
 
 config ASPEED_SOC
 bool
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 4f591ca..fadd698 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -22,6 +22,7 @@ obj-$(CONFIG_COLLIE) += collie.o
 obj-$(CONFIG_VERSATILE) += versatilepb.o
 obj-$(CONFIG_VEXPRESS) += vexpress.o
 obj-$(CONFIG_ZYNQ) += xilinx_zynq.o
+obj-$(CONFIG_SABRELITE) += sabrelite.o
 
 obj-$(CONFIG_ARM_V7M) += armv7m.o
 obj-$(CONFIG_EXYNOS4) += exynos4210.o
@@ -36,7 +37,7 @@ obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
 obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o xlnx-versal-virt.o
 obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
 obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
-obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
+obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
 obj-$(CONFIG_MPS2) += mps2.o
 obj-$(CONFIG_MPS2) += mps2-tz.o
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 14/30] hw/arm: Express dependencies of xilinx-zynq with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the xilinx-zynq-a9 board.
This patch is based on earlier work by Ákos Kovács (i.e.
his "hw/arm/Kconfig: Add ARM Kconfig" patch).

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak |  7 +--
 hw/arm/Kconfig  | 14 ++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index cba62ba..b9fce47 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -16,6 +16,7 @@ CONFIG_STELLARIS=y
 CONFIG_REALVIEW=y
 CONFIG_VERSATILE=y
 CONFIG_VEXPRESS=y
+CONFIG_ZYNQ=y
 CONFIG_MAINSTONE=y
 CONFIG_GUMSTIX=y
 CONFIG_SPITZ=y
@@ -35,16 +36,11 @@ CONFIG_VIRTIO_MMIO=y
 
 CONFIG_NETDUINO2=y
 
-CONFIG_PL330=y
-CONFIG_CADENCE=y
 CONFIG_FRAMEBUFFER=y
-CONFIG_XILINX_SPIPS=y
-CONFIG_ZYNQ_DEVCFG=y
 
 CONFIG_DIGIC=y
 CONFIG_MPS2=y
 CONFIG_RASPI=y
-CONFIG_ZYNQ=y
 CONFIG_STM32F2XX_TIMER=y
 CONFIG_STM32F2XX_USART=y
 CONFIG_STM32F2XX_SYSCFG=y
@@ -99,7 +95,6 @@ CONFIG_SMBUS_EEPROM=y
 CONFIG_GPIO_KEY=y
 CONFIG_MSF2=y
 CONFIG_FW_CFG_DMA=y
-CONFIG_XILINX_AXI=y
 CONFIG_PCI_EXPRESS_DESIGNWARE=y
 
 CONFIG_STRONGARM=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index fd6b92c..78e694b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -208,6 +208,20 @@ config VEXPRESS
 
 config ZYNQ
 bool
+select A9MPCORE
+select CADENCE # UART
+select PCI
+select PFLASH_CFI02
+select PL330
+select SDHCI
+select SSI_M25P80
+select USB_EHCI
+select USB_EHCI_SYSBUS
+select XILINX # UART
+select XILINX_AXI
+select XILINX_SPI
+select XILINX_SPIPS
+select ZYNQ_DEVCFG
 
 config ARM_V7M
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 12/30] hw/arm: Express dependencies of realview, versatile and vexpress with Kconfig

[Thomas Huth]

This patch is slightly based on earlier work by Ákos Kovács (i.e.
his "hw/arm/Kconfig: Add ARM Kconfig" patch).

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 21 +++---
 hw/arm/Kconfig  | 47 +
 hw/arm/Makefile.objs|  3 ++-
 hw/display/Kconfig  |  1 +
 hw/i2c/Kconfig  |  2 +-
 5 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 4ca7b63..d6858c3 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -3,8 +3,6 @@
 # TODO: ARM_V7M is currently always required - make this more flexible!
 CONFIG_ARM_V7M=y
 
-CONFIG_PCI_DEVICES=y
-
 CONFIG_EXYNOS4=y
 CONFIG_HIGHBANK=y
 CONFIG_INTEGRATOR=y
@@ -15,6 +13,9 @@ CONFIG_CHEETAH=y
 CONFIG_SX1=y
 CONFIG_NSERIES=y
 CONFIG_STELLARIS=y
+CONFIG_REALVIEW=y
+CONFIG_VERSATILE=y
+CONFIG_VEXPRESS=y
 
 CONFIG_VGA=y
 CONFIG_NAND=y
@@ -23,8 +24,6 @@ CONFIG_SERIAL=y
 CONFIG_MAX7310=y
 CONFIG_TMP421=y
 CONFIG_PCA9552=y
-CONFIG_DDC=y
-CONFIG_SII9022=y
 CONFIG_ADS7846=y
 CONFIG_MAX111X=y
 CONFIG_SSI_M25P80=y
@@ -36,13 +35,8 @@ CONFIG_MICRODRIVE=y
 CONFIG_PLATFORM_BUS=y
 CONFIG_VIRTIO_MMIO=y
 
-CONFIG_ARM11MPCORE=y
-
 CONFIG_NETDUINO2=y
 
-CONFIG_PL041=y
-CONFIG_PL080=y
-CONFIG_PL190=y
 CONFIG_PL330=y
 CONFIG_CADENCE=y
 CONFIG_PXA2XX=y
@@ -50,12 +44,10 @@ CONFIG_FRAMEBUFFER=y
 CONFIG_XILINX_SPIPS=y
 CONFIG_ZYNQ_DEVCFG=y
 
-CONFIG_ARM11SCU=y
 CONFIG_DIGIC=y
 CONFIG_MAINSTONE=y
 CONFIG_MPS2=y
 CONFIG_RASPI=y
-CONFIG_REALVIEW=y
 CONFIG_ZAURUS=y
 CONFIG_ZYNQ=y
 CONFIG_STM32F2XX_TIMER=y
@@ -83,10 +75,6 @@ CONFIG_IOTKIT_SYSCTL=y
 CONFIG_IOTKIT_SYSINFO=y
 CONFIG_ARMSSE_CPUID=y
 
-CONFIG_VERSATILE=y
-CONFIG_VERSATILE_PCI=y
-CONFIG_VERSATILE_I2C=y
-
 CONFIG_PCI_EXPRESS=y
 CONFIG_PCI_EXPRESS_GENERIC_BRIDGE=y
 CONFIG_VFIO_PLATFORM=y
@@ -120,6 +108,3 @@ CONFIG_XILINX_AXI=y
 CONFIG_PCI_EXPRESS_DESIGNWARE=y
 
 CONFIG_STRONGARM=y
-
-# for realview and versatilepb
-CONFIG_LSI_SCSI_PCI=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index db48b9b..62ab487 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -95,6 +95,29 @@ config PXA2XX
 
 config REALVIEW
 bool
+imply PCI_DEVICES
+select SMC91C111
+select LAN9118
+select A9MPCORE
+select A15MPCORE
+select ARM11MPCORE
+select ARM_TIMER
+select VERSATILE_PCI
+select WM8750 # audio codec
+select LSI_SCSI_PCI
+select PCI
+select PL011  # UART
+select PL031  # RTC
+select PL041  # audio codec
+select PL050  # keyboard/mouse
+select PL061  # GPIO
+select PL080  # DMA controller
+select PL110
+select PL181  # display
+select PL310  # cache controller
+select VERSATILE_I2C
+select DS1338 # I2C RTC+NVRAM
+select USB_OHCI
 
 config STELLARIS
 bool
@@ -118,6 +141,29 @@ config SX1
 
 config VERSATILE
 bool
+select ARM_TIMER # sp804
+select PFLASH_CFI01
+select LSI_SCSI_PCI
+select PL050  # keyboard/mouse
+select PL080  # DMA controller
+select PL190  # Vector PIC
+select REALVIEW
+select USB_OHCI
+
+config VEXPRESS
+bool
+select A9MPCORE
+select A15MPCORE
+select ARM_MPTIMER
+select ARM_TIMER # sp804
+select LAN9118
+select PFLASH_CFI01
+select PL011 # UART
+select PL041 # audio codec
+select PL181  # display
+select REALVIEW
+select SII9022
+select VIRTIO_MMIO
 
 config ZYNQ
 bool
@@ -197,6 +243,7 @@ config A15MPCORE
 
 config ARM11MPCORE
 bool
+select ARM11SCU
 
 config ARMSSE
 bool
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 8302b8d..bd0b45a 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -15,7 +15,8 @@ obj-$(CONFIG_PXA2XX) += gumstix.o spitz.o tosa.o z2.o
 obj-$(CONFIG_REALVIEW) += realview.o
 obj-$(CONFIG_STELLARIS) += stellaris.o
 obj-$(CONFIG_STRONGARM) += collie.o
-obj-$(CONFIG_VERSATILE) += vexpress.o versatilepb.o
+obj-$(CONFIG_VERSATILE) += versatilepb.o
+obj-$(CONFIG_VEXPRESS) += vexpress.o
 obj-$(CONFIG_ZYNQ) += xilinx_zynq.o
 
 obj-$(CONFIG_ARM_V7M) += armv7m.o
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 34c8c12..a5f3fc0 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -26,6 +26,7 @@ config PL110
 config SII9022
 bool
 depends on I2C
+select DDC
 
 config SSD0303
 bool
diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
index ef1caa6..0004893 100644
--- a/hw/i2c/Kconfig
+++ b/hw/i2c/Kconfig
@@ -12,7 +12,7 @@ config DDC
 
 config VERSATILE_I2C
 bool
-select I2C
+select BITBANG_I2C
 
 config ACPI_SMBUS
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 17/30] hw/arm: Express dependencies of the virt machine with Kconfig

[Thomas Huth]

Dependencies have been determined by looking at hw/arm/virt.c

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 14 +-
 hw/arm/Kconfig  | 19 +++
 hw/arm/Makefile.objs|  3 ++-
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 476fe05..62d6b96 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -3,6 +3,7 @@
 # TODO: ARM_V7M is currently always required - make this more flexible!
 CONFIG_ARM_V7M=y
 
+CONFIG_ARM_VIRT=y
 CONFIG_EXYNOS4=y
 CONFIG_HIGHBANK=y
 CONFIG_INTEGRATOR=y
@@ -29,8 +30,6 @@ CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
 CONFIG_ALLWINNER_EMAC=y
 CONFIG_IMX_FEC=y
-CONFIG_PLATFORM_BUS=y
-CONFIG_VIRTIO_MMIO=y
 
 CONFIG_NETDUINO2=y
 
@@ -64,12 +63,6 @@ CONFIG_IOTKIT_SYSCTL=y
 CONFIG_IOTKIT_SYSINFO=y
 CONFIG_ARMSSE_CPUID=y
 
-CONFIG_PCI_EXPRESS=y
-CONFIG_PCI_EXPRESS_GENERIC_BRIDGE=y
-CONFIG_VFIO_PLATFORM=y
-CONFIG_VFIO_XGMAC=y
-CONFIG_VFIO_AMD_XGBE=y
-
 CONFIG_ALLWINNER_A10_PIT=y
 CONFIG_ALLWINNER_A10_PIC=y
 CONFIG_ALLWINNER_A10=y
@@ -85,10 +78,5 @@ CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
-CONFIG_ACPI=y
-CONFIG_ARM_VIRT=y
-CONFIG_SMBIOS=y
-CONFIG_GPIO_KEY=y
 CONFIG_MSF2=y
-CONFIG_FW_CFG_DMA=y
 CONFIG_PCI_EXPRESS_DESIGNWARE=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 32b3785..487fb03 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -1,5 +1,23 @@
 config ARM_VIRT
 bool
+imply PCI_DEVICES
+imply VFIO_AMD_XGBE
+imply VFIO_XGMAC
+select A15MPCORE
+select ACPI
+select ARM_SMMUV3
+select GPIO_KEY
+select FW_CFG_DMA
+select PCI_EXPRESS
+select PCI_EXPRESS_GENERIC_BRIDGE
+select PFLASH_CFI01
+select PL011 # UART
+select PL031 # RTC
+select PL061 # GPIO
+select PLATFORM_BUS
+select SMBIOS
+select VIRTIO_MMIO
+select VFIO_PLATFORM
 
 config CHEETAH
 bool
@@ -314,6 +332,7 @@ config A9MPCORE
 
 config A15MPCORE
 bool
+select ARM_GIC
 
 config ARM11MPCORE
 bool
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 729e711..4f591ca 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -1,4 +1,5 @@
-obj-y += boot.o sysbus-fdt.o
+obj-y += boot.o
+obj-$(CONFIG_PLATFORM_BUS) += sysbus-fdt.o
 obj-$(CONFIG_ARM_VIRT) += virt.o
 obj-$(CONFIG_ACPI) += virt-acpi-build.o
 obj-$(CONFIG_DIGIC) += digic_boards.o
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 11/30] hw/arm: Express dependencies of stellaris with Kconfig

[Thomas Huth]

This patch is slightly based on earlier work by Ákos Kovács (i.e.
his "hw/arm/Kconfig: Add ARM Kconfig" patch).

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak |  7 +--
 hw/arm/Kconfig  | 10 ++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index e0818f1..4ca7b63 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -14,6 +14,7 @@ CONFIG_MUSCA=y
 CONFIG_CHEETAH=y
 CONFIG_SX1=y
 CONFIG_NSERIES=y
+CONFIG_STELLARIS=y
 
 CONFIG_VGA=y
 CONFIG_NAND=y
@@ -22,16 +23,10 @@ CONFIG_SERIAL=y
 CONFIG_MAX7310=y
 CONFIG_TMP421=y
 CONFIG_PCA9552=y
-CONFIG_STELLARIS=y
-CONFIG_STELLARIS_INPUT=y
-CONFIG_STELLARIS_ENET=y
-CONFIG_SSD0303=y
-CONFIG_SSD0323=y
 CONFIG_DDC=y
 CONFIG_SII9022=y
 CONFIG_ADS7846=y
 CONFIG_MAX111X=y
-CONFIG_SSI_SD=y
 CONFIG_SSI_M25P80=y
 CONFIG_ALLWINNER_EMAC=y
 CONFIG_IMX_FEC=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index af559be..db48b9b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -98,6 +98,16 @@ config REALVIEW
 
 config STELLARIS
 bool
+select ARM_V7M
+select I2C
+select PL011 # UART
+select PL022 # Serial port
+select PL061 # GPIO
+select SSD0303 # OLED display
+select SSD0323 # OLED display
+select SSI_SD
+select STELLARIS_INPUT
+select STELLARIS_ENET # ethernet
 
 config STRONGARM
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 20/30] hw/arm: Express dependencies of the MPS2 boards with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the mps2-an* machines.

Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 19 +--
 hw/arm/Kconfig  | 17 +
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index badb190..b8509fd 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -27,6 +27,7 @@ CONFIG_Z2=y
 CONFIG_COLLIE=y
 CONFIG_ASPEED_SOC=y
 CONFIG_NETDUINO2=y
+CONFIG_MPS2=y
 
 CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
@@ -35,27 +36,9 @@ CONFIG_IMX_FEC=y
 CONFIG_FRAMEBUFFER=y
 
 CONFIG_DIGIC=y
-CONFIG_MPS2=y
 CONFIG_RASPI=y
 CONFIG_NRF51_SOC=y
 
-CONFIG_CMSDK_APB_TIMER=y
-CONFIG_CMSDK_APB_DUALTIMER=y
-CONFIG_CMSDK_APB_UART=y
-CONFIG_CMSDK_APB_WATCHDOG=y
-
-CONFIG_MPS2_FPGAIO=y
-CONFIG_MPS2_SCC=y
-
-CONFIG_TZ_MPC=y
-CONFIG_TZ_MSC=y
-CONFIG_TZ_PPC=y
-CONFIG_ARMSSE=y
-CONFIG_IOTKIT_SECCTL=y
-CONFIG_IOTKIT_SYSCTL=y
-CONFIG_IOTKIT_SYSINFO=y
-CONFIG_ARMSSE_CPUID=y
-
 CONFIG_FSL_IMX6=y
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 73382b6..fa69f1b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -314,6 +314,13 @@ config ASPEED_SOC
 
 config MPS2
 bool
+select ARMSSE
+select LAN9118
+select MPS2_FPGAIO
+select MPS2_SCC
+select PL022# Serial port
+select PL080# DMA controller
+select TZ_MPC
 
 config IOTKIT
 bool
@@ -356,6 +363,16 @@ config ARM11MPCORE
 
 config ARMSSE
 bool
+select ARMSSE_CPUID
+select CMSDK_APB_TIMER
+select CMSDK_APB_DUALTIMER
+select CMSDK_APB_UART
+select CMSDK_APB_WATCHDOG
+select IOTKIT_SECCTL
+select IOTKIT_SYSCTL
+select IOTKIT_SYSINFO
+select TZ_MSC
+select TZ_PPC
 
 config ARMSSE_CPUID
 bool
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 04/30] hw/sd/sdhci: Move PCI-related code into a separate file

[Thomas Huth]

Some machines have an SDHCI device, but no PCI. To be able to
compile hw/sd/sdhci.c without CONFIG_PCI, we must not call functions
like pci_get_address_space() and pci_allocate_irq() there. Thus
move the PCI-related code into a separate file.

This is required for the new Kconfig-like build system, e.g. it is
needed if a user wants to compile a QEMU binary with just one machine
that has SDHCI, but no PCI, like the ARM "raspi" machines for example.

Signed-off-by: Thomas Huth 
---
 Philippe, I dropped your "Reviewed-by" since I modified the patch for
 Kconfig now. Please have another look at the Kconfig and Makefile.objs
 changes, the rest is still the same.

 hw/sd/Kconfig  |  6 +++-
 hw/sd/Makefile.objs|  1 +
 hw/sd/sdhci-internal.h | 34 ++
 hw/sd/sdhci-pci.c  | 87 
 hw/sd/sdhci.c  | 98 +++---
 5 files changed, 132 insertions(+), 94 deletions(-)
 create mode 100644 hw/sd/sdhci-pci.c

diff --git a/hw/sd/Kconfig b/hw/sd/Kconfig
index 864f535..c5e1e55 100644
--- a/hw/sd/Kconfig
+++ b/hw/sd/Kconfig
@@ -12,6 +12,10 @@ config SD
 
 config SDHCI
 bool
+select SD
+
+config SDHCI_PCI
+bool
 default y if PCI_DEVICES
 depends on PCI
-select SD
+select SDHCI
diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
index a99d9fb..0665727 100644
--- a/hw/sd/Makefile.objs
+++ b/hw/sd/Makefile.objs
@@ -2,6 +2,7 @@ common-obj-$(CONFIG_PL181) += pl181.o
 common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
 common-obj-$(CONFIG_SD) += sd.o core.o sdmmc-internal.o
 common-obj-$(CONFIG_SDHCI) += sdhci.o
+common-obj-$(CONFIG_SDHCI_PCI) += sdhci-pci.o
 
 obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
 obj-$(CONFIG_OMAP) += omap_mmc.o
diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 19665fd..3414140 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -304,4 +304,38 @@ extern const VMStateDescription sdhci_vmstate;
 
 #define ESDHC_PRNSTS_SDSTB  (1 << 3)
 
+/*
+ * Default SD/MMC host controller features information, which will be
+ * presented in CAPABILITIES register of generic SD host controller at reset.
+ *
+ * support:
+ * - 3.3v and 1.8v voltages
+ * - SDMA/ADMA1/ADMA2
+ * - high-speed
+ * max host controller R/W buffers size: 512B
+ * max clock frequency for SDclock: 52 MHz
+ * timeout clock frequency: 52 MHz
+ *
+ * does not support:
+ * - 3.0v voltage
+ * - 64-bit system bus
+ * - suspend/resume
+ */
+#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
+
+#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
+DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
+DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
+\
+/* Capabilities registers provide information on supported
+ * features of this specific host controller implementation */ \
+DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
+DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
+
+void sdhci_initfn(SDHCIState *s);
+void sdhci_uninitfn(SDHCIState *s);
+void sdhci_common_realize(SDHCIState *s, Error **errp);
+void sdhci_common_unrealize(SDHCIState *s, Error **errp);
+void sdhci_common_class_init(ObjectClass *klass, void *data);
+
 #endif
diff --git a/hw/sd/sdhci-pci.c b/hw/sd/sdhci-pci.c
new file mode 100644
index 000..f884661
--- /dev/null
+++ b/hw/sd/sdhci-pci.c
@@ -0,0 +1,87 @@
+/*
+ * SDHCI device on PCI
+ *
+ * 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.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "hw/sd/sdhci.h"
+#include "sdhci-internal.h"
+
+static Property sdhci_pci_properties[] = {
+DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
+{
+SDHCIState *s = PCI_SDHCI(dev);
+Error *local_err = NULL;
+
+sdhci_initfn(s);
+sdhci_common_realize(s, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
+dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
+s->irq = pci_allocate_irq(dev);
+s->dma_as = pci_get_address_space(dev);
+pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, >iomem);
+}
+
+static void