[Qemu-devel] [PULL 0/6] target-arm queue

[Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

The following changes since commit fc8ead74674b7129e8f31c2595c76658e5622197:

  Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2013-10-18 10:03:24 -0700)

are available in the git repository at:


  git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20131025

for you to fetch changes up to 71c903cc3b78fc563122fe40c5cadd050068b91a:

  integrator: fix Linux boot failure by emulating dbg region (2013-10-25 18:27:07 +0100)

----------------------------------------------------------------
target-arm queue: a couple of trivial features to improve support
for some guest emulation cases, notably running UEFI images:
 * support VBAR (vector base address register)
 * allow running without specifying a kernel (ie just running
   an image from flash)
Plus some bugfixes.

----------------------------------------------------------------
Alex Bennée (1):
      integrator: fix Linux boot failure by emulating dbg region

Alvise Rigo (2):
      target-arm: sort TCG cpreg list by KVM-style 64 bit ID number
      target-arm: fix sorting issue of KVM cpreg list

Nathan Rossi (1):
      target-arm: Add CP15 VBAR support

Peter Maydell (2):
      hw/arm/boot: Make user not specifying a kernel not an error
      hw/arm: Tidy up conditional calls to arm_load_kernel

 default-configs/arm-softmmu.mak        |    1 +
 hw/arm/boot.c                          |    6 +-
 hw/arm/integratorcp.c                  |    2 +
 hw/arm/omap_sx1.c                      |   10 ++--
 hw/arm/palm.c                          |   10 ++--
 hw/arm/z2.c                            |   12 ++--
 hw/misc/Makefile.objs                  |    1 +
 hw/misc/arm_integrator_debug.c         |   99 ++++++++++++++++++++++++++++++++
 include/hw/misc/arm_integrator_debug.h |   18 ++++++
 target-arm/cpu.h                       |    1 +
 target-arm/helper.c                    |   33 ++++++++++-
 target-arm/kvm.c                       |    8 ++-
 12 files changed, 176 insertions(+), 25 deletions(-)
 create mode 100644 hw/misc/arm_integrator_debug.c
 create mode 100644 include/hw/misc/arm_integrator_debug.h

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Edgar E. Iglesias]

On Fri, Oct 25, 2013 at 07:07:23PM +0100, Peter Maydell wrote:
> The following changes since commit fc8ead74674b7129e8f31c2595c76658e5622197:
> 
>   Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2013-10-18 10:03:24 -0700)
> 
> are available in the git repository at:
> 
> 
>   git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20131025
> 
> for you to fetch changes up to 71c903cc3b78fc563122fe40c5cadd050068b91a:
> 
>   integrator: fix Linux boot failure by emulating dbg region (2013-10-25 18:27:07 +0100)


Applied, thanks all.

Cheers,
Edgar


> 
> ----------------------------------------------------------------
> target-arm queue: a couple of trivial features to improve support
> for some guest emulation cases, notably running UEFI images:
>  * support VBAR (vector base address register)
>  * allow running without specifying a kernel (ie just running
>    an image from flash)
> Plus some bugfixes.
> 
> ----------------------------------------------------------------
> Alex Bennée (1):
>       integrator: fix Linux boot failure by emulating dbg region
> 
> Alvise Rigo (2):
>       target-arm: sort TCG cpreg list by KVM-style 64 bit ID number
>       target-arm: fix sorting issue of KVM cpreg list
> 
> Nathan Rossi (1):
>       target-arm: Add CP15 VBAR support
> 
> Peter Maydell (2):
>       hw/arm/boot: Make user not specifying a kernel not an error
>       hw/arm: Tidy up conditional calls to arm_load_kernel
> 
>  default-configs/arm-softmmu.mak        |    1 +
>  hw/arm/boot.c                          |    6 +-
>  hw/arm/integratorcp.c                  |    2 +
>  hw/arm/omap_sx1.c                      |   10 ++--
>  hw/arm/palm.c                          |   10 ++--
>  hw/arm/z2.c                            |   12 ++--
>  hw/misc/Makefile.objs                  |    1 +
>  hw/misc/arm_integrator_debug.c         |   99 ++++++++++++++++++++++++++++++++
>  include/hw/misc/arm_integrator_debug.h |   18 ++++++
>  target-arm/cpu.h                       |    1 +
>  target-arm/helper.c                    |   33 ++++++++++-
>  target-arm/kvm.c                       |    8 ++-
>  12 files changed, 176 insertions(+), 25 deletions(-)
>  create mode 100644 hw/misc/arm_integrator_debug.c
>  create mode 100644 include/hw/misc/arm_integrator_debug.h
> 

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Andreas Färber]

Hi,

Am 31.10.2013 15:02, schrieb Edgar E. Iglesias:
> On Fri, Oct 25, 2013 at 07:07:23PM +0100, Peter Maydell wrote:
>> The following changes since commit fc8ead74674b7129e8f31c2595c76658e5622197:
>>
>>   Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2013-10-18 10:03:24 -0700)
>>
>> are available in the git repository at:
>>
>>
>>   git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20131025
>>
>> for you to fetch changes up to 71c903cc3b78fc563122fe40c5cadd050068b91a:
>>
>>   integrator: fix Linux boot failure by emulating dbg region (2013-10-25 18:27:07 +0100)
> 
> 
> Applied, thanks all.

Edgar, there is no merge commit in qemu.git despite this being a signed
pull. Do you maybe need to upgrade your version of git?

Peter, since I had picked up the first two patches into my still pending
qom-next pull, as per the QEMU Summit discussion those patches should've
gotten an Acked-by.

Regards,
Andreas

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Anthony Liguori]

On Thu, Oct 31, 2013 at 3:18 PM, Andreas Färber <afaerber@suse.de> wrote:
> Hi,
>
> Am 31.10.2013 15:02, schrieb Edgar E. Iglesias:
>> On Fri, Oct 25, 2013 at 07:07:23PM +0100, Peter Maydell wrote:
>>> The following changes since commit fc8ead74674b7129e8f31c2595c76658e5622197:
>>>
>>>   Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2013-10-18 10:03:24 -0700)
>>>
>>> are available in the git repository at:
>>>
>>>
>>>   git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20131025
>>>
>>> for you to fetch changes up to 71c903cc3b78fc563122fe40c5cadd050068b91a:
>>>
>>>   integrator: fix Linux boot failure by emulating dbg region (2013-10-25 18:27:07 +0100)
>>
>>
>> Applied, thanks all.
>
> Edgar, there is no merge commit in qemu.git despite this being a signed
> pull. Do you maybe need to upgrade your version of git?

Need to add:

[merge]
 ff = false

To your git config to prevent fast forwards on merging.

Regards,

Anthony Liguori

> Peter, since I had picked up the first two patches into my still pending
> qom-next pull, as per the QEMU Summit discussion those patches should've
> gotten an Acked-by.
>
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
> Peter, since I had picked up the first two patches into my still pending
> qom-next pull, as per the QEMU Summit discussion those patches should've
> gotten an Acked-by.

Hmm? I don't recall this part of the discussion. If you want the
patches to have an Acked-by from you you need to send mail
to the list with an Acked-by line.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Andreas Färber]

Am 31.10.2013 15:31, schrieb Peter Maydell:
> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>> Peter, since I had picked up the first two patches into my still pending
>> qom-next pull, as per the QEMU Summit discussion those patches should've
>> gotten an Acked-by.
> 
> Hmm? I don't recall this part of the discussion. If you want the
> patches to have an Acked-by from you you need to send mail
> to the list with an Acked-by line.

No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
needs to be explicitly sent as reply but that "looks okay" should in
exactly such a case where sender=submaintainer should be recorded as
Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.

Andreas

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Anthony Liguori]

On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 15:31, schrieb Peter Maydell:
>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>> Peter, since I had picked up the first two patches into my still pending
>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>> gotten an Acked-by.
>>
>> Hmm? I don't recall this part of the discussion. If you want the
>> patches to have an Acked-by from you you need to send mail
>> to the list with an Acked-by line.
>
> No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
> needs to be explicitly sent as reply but that "looks okay" should in
> exactly such a case where sender=submaintainer should be recorded as
> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.

Nope.  If you want there to be an Acked-by, say "Acked-by:".  Don't
make people infer your Acked-bys.

And adding tags is a nice-to-have.  There is no "rule" stating that
you must include everyone that appears on the mailing list.  But I
expect that maintainers try to

Regards,

Anthony Liguori

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Andreas Färber]

Am 31.10.2013 15:39, schrieb Anthony Liguori:
> On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>>> Peter, since I had picked up the first two patches into my still pending
>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>> gotten an Acked-by.
>>>
>>> Hmm? I don't recall this part of the discussion. If you want the
>>> patches to have an Acked-by from you you need to send mail
>>> to the list with an Acked-by line.
>>
>> No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
>> needs to be explicitly sent as reply but that "looks okay" should in
>> exactly such a case where sender=submaintainer should be recorded as
>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
> 
> Nope.  If you want there to be an Acked-by, say "Acked-by:".  Don't
> make people infer your Acked-bys.

Yes, that's in the minutes. And yes, that's what I got as answer there.
Please reply to the minutes if you think otherwise.

I brought up exactly this situation where I am contributor to CPU and
submaintainer of CPU and often not getting Reviewed-bys but if at all,
such as from Paolo recently, some verbal "looks OK" for a series. I was
told that that should be turned into an Acked-by on the patches to
satisfy your criteria that contributors may not just send patches as
pull without Reviewed-by.

> And adding tags is a nice-to-have.  There is no "rule" stating that
> you must include everyone that appears on the mailing list.  But I
> expect that maintainers try to

Again, at QEMU Summit you pushed for making Reviewed-by a must-have and
we discussed whether a submaintainer must add a Reviewed-by then and
what to do if author==submaintainer. If you dropped that thought, then
fine with me.

Regards,
Andreas

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Anthony Liguori]

On Thu, Oct 31, 2013 at 3:45 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 15:39, schrieb Anthony Liguori:
>> On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>>>> Peter, since I had picked up the first two patches into my still pending
>>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>>> gotten an Acked-by.
>>>>
>>>> Hmm? I don't recall this part of the discussion. If you want the
>>>> patches to have an Acked-by from you you need to send mail
>>>> to the list with an Acked-by line.
>>>
>>> No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
>>> needs to be explicitly sent as reply but that "looks okay" should in
>>> exactly such a case where sender=submaintainer should be recorded as
>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>
>> Nope.  If you want there to be an Acked-by, say "Acked-by:".  Don't
>> make people infer your Acked-bys.
>
> Yes, that's in the minutes. And yes, that's what I got as answer there.
> Please reply to the minutes if you think otherwise.

I

> I brought up exactly this situation where I am contributor to CPU and
> submaintainer of CPU and often not getting Reviewed-bys but if at all,
> such as from Paolo recently, some verbal "looks OK" for a series. I was
> told that that should be turned into an Acked-by on the patches to
> satisfy your criteria that contributors may not just send patches as
> pull without Reviewed-by.
>
>> And adding tags is a nice-to-have.  There is no "rule" stating that
>> you must include everyone that appears on the mailing list.  But I
>> expect that maintainers try to
>
> Again, at QEMU Summit you pushed for making Reviewed-by a must-have and
> we discussed whether a submaintainer must add a Reviewed-by then and
> what to do if author==submaintainer. If you dropped that thought, then
> fine with me.
>
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Anthony Liguori]

On Thu, Oct 31, 2013 at 3:45 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 15:39, schrieb Anthony Liguori:
>> On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>>>> Peter, since I had picked up the first two patches into my still pending
>>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>>> gotten an Acked-by.
>>>>
>>>> Hmm? I don't recall this part of the discussion. If you want the
>>>> patches to have an Acked-by from you you need to send mail
>>>> to the list with an Acked-by line.
>>>
>>> No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
>>> needs to be explicitly sent as reply but that "looks okay" should in
>>> exactly such a case where sender=submaintainer should be recorded as
>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>
>> Nope.  If you want there to be an Acked-by, say "Acked-by:".  Don't
>> make people infer your Acked-bys.
>
> Yes, that's in the minutes. And yes, that's what I got as answer there.
> Please reply to the minutes if you think otherwise.

I explicitly said that Acked-bys are useless too.

The minutes say that you said the kernel treats "Acked-bys" as "looks
good".  You did say that.  At no point did a "rule" get made though.

> I brought up exactly this situation where I am contributor to CPU and
> submaintainer of CPU and often not getting Reviewed-bys but if at all,
> such as from Paolo recently, some verbal "looks OK" for a series. I was
> told that that should be turned into an Acked-by on the patches to
> satisfy your criteria that contributors may not just send patches as
> pull without Reviewed-by.

I think you misunderstood.

I don't care about Acked-bys.  They are useless.

A third of patches are being committed with Reviewed-bys.  There are
certainly many cases where patches are going in from submaintainers
that have been reviewed which comes implicitly with Signed-off-by.

But I worry that we're not reviewing enough on list and that there are
patches from maintainers going in through maintainer trees that aren't
getting outside review.

There's no immediate action for this other than we should all try to
review more patches on list to prevent the above situation.

>> And adding tags is a nice-to-have.  There is no "rule" stating that
>> you must include everyone that appears on the mailing list.  But I
>> expect that maintainers try to
>
> Again, at QEMU Summit you pushed for making Reviewed-by a must-have and
> we discussed whether a submaintainer must add a Reviewed-by then and
> what to do if author==submaintainer. If you dropped that thought, then
> fine with me.

Yes, patches should get reviewed.  I hope this is obvious to all of us :-)

I also suggested that I have tooling that people can use to simplify
adding collected Reviewed-bys on the list.

But none of this has anything to do with inferred Acked-bys.  I'll go
a step further and say that I would be very unhappy if anyone every
added any kind of tag to a patch with my name on it that I didn't send
myself.

Regards,

Anthony Liguori

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 31 October 2013 14:36, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 15:31, schrieb Peter Maydell:
>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>> Peter, since I had picked up the first two patches into my still pending
>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>> gotten an Acked-by.
>>
>> Hmm? I don't recall this part of the discussion. If you want the
>> patches to have an Acked-by from you you need to send mail
>> to the list with an Acked-by line.
>
> No, I added a Signed-off-by.

I checked my mail and the only thing I can find in reply to those
patches is a note from you saying you added them to your queue.

> It was clearly stated that a Reviewed-by
> needs to be explicitly sent as reply but that "looks okay" should in
> exactly such a case where sender=submaintainer should be recorded as
> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.

...but you're not the submaintainer here so I don't think this applies.

The point about the kernel practice as I understood it was that
the kernel folks treat acked-by at about the same level of review as
"looks ok to me" (ie, very little), not that there's some obligation to
treat any informal 'looks ok' note as an acked-by. I'm in full agreement
with Anthony that if you want a tag to appear you should send it
properly.

-- PMM

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Andreas Färber]

Am 31.10.2013 16:04, schrieb Anthony Liguori:
> On Thu, Oct 31, 2013 at 3:45 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 31.10.2013 15:39, schrieb Anthony Liguori:
>>> On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>>>>> Peter, since I had picked up the first two patches into my still pending
>>>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>>>> gotten an Acked-by.
>>>>>
>>>>> Hmm? I don't recall this part of the discussion. If you want the
>>>>> patches to have an Acked-by from you you need to send mail
>>>>> to the list with an Acked-by line.
>>>>
>>>> No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
>>>> needs to be explicitly sent as reply but that "looks okay" should in
>>>> exactly such a case where sender=submaintainer should be recorded as
>>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>>
>>> Nope.  If you want there to be an Acked-by, say "Acked-by:".  Don't
>>> make people infer your Acked-bys.
>>
>> Yes, that's in the minutes. And yes, that's what I got as answer there.
>> Please reply to the minutes if you think otherwise.
> 
> I explicitly said that Acked-bys are useless too.
> 
> The minutes say that you said the kernel treats "Acked-bys" as "looks
> good".  You did say that.

I *asked* about what to do with my QEMU CPU patches that only get a
"looks okay" and got only positive answers for whether that should be an
Acked-by and no objection, including none from you.
I certainly said nothing at all about the kernel.

>  At no point did a "rule" get made though.

The new rule you made was: no patch without Reviewed-by.
Peter sending that PULL and Edgar merging it both violate that rule.
No objection against a particular patch function-wise.

Point is, had Peter ping'ed me before sending out that pull, he would've
actually gotten a Reviewed-by from me, thereby satisfying your rule! He
didn't, ignoring that he himself had actually told me to queue the
patches before his vacation, for which obviously I reviewed and tested them.

Maybe there's no obligation for picking up tags, but then again you
can't go ahead and do statistics over incompletely recorded tags.

Regards,
Andreas

>> I brought up exactly this situation where I am contributor to CPU and
>> submaintainer of CPU and often not getting Reviewed-bys but if at all,
>> such as from Paolo recently, some verbal "looks OK" for a series. I was
>> told that that should be turned into an Acked-by on the patches to
>> satisfy your criteria that contributors may not just send patches as
>> pull without Reviewed-by.
> 
> I think you misunderstood.
> 
> I don't care about Acked-bys.  They are useless.
> 
> A third of patches are being committed with Reviewed-bys.  There are
> certainly many cases where patches are going in from submaintainers
> that have been reviewed which comes implicitly with Signed-off-by.
> 
> But I worry that we're not reviewing enough on list and that there are
> patches from maintainers going in through maintainer trees that aren't
> getting outside review.
> 
> There's no immediate action for this other than we should all try to
> review more patches on list to prevent the above situation.
> 
>>> And adding tags is a nice-to-have.  There is no "rule" stating that
>>> you must include everyone that appears on the mailing list.  But I
>>> expect that maintainers try to
>>
>> Again, at QEMU Summit you pushed for making Reviewed-by a must-have and
>> we discussed whether a submaintainer must add a Reviewed-by then and
>> what to do if author==submaintainer. If you dropped that thought, then
>> fine with me.
> 
> Yes, patches should get reviewed.  I hope this is obvious to all of us :-)
> 
> I also suggested that I have tooling that people can use to simplify
> adding collected Reviewed-bys on the list.
> 
> But none of this has anything to do with inferred Acked-bys.  I'll go
> a step further and say that I would be very unhappy if anyone every
> added any kind of tag to a patch with my name on it that I didn't send
> myself.
> 
> Regards,
> 
> Anthony Liguori
> 
>>
>> Regards,
>> Andreas
>>
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Anthony Liguori]

On Thu, Oct 31, 2013 at 5:52 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 16:04, schrieb Anthony Liguori:
>> On Thu, Oct 31, 2013 at 3:45 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 31.10.2013 15:39, schrieb Anthony Liguori:
>>>> On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>>>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>>>>>> Peter, since I had picked up the first two patches into my still pending
>>>>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>>>>> gotten an Acked-by.
>>>>>>
>>>>>> Hmm? I don't recall this part of the discussion. If you want the
>>>>>> patches to have an Acked-by from you you need to send mail
>>>>>> to the list with an Acked-by line.
>>>>>
>>>>> No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
>>>>> needs to be explicitly sent as reply but that "looks okay" should in
>>>>> exactly such a case where sender=submaintainer should be recorded as
>>>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>>>
>>>> Nope.  If you want there to be an Acked-by, say "Acked-by:".  Don't
>>>> make people infer your Acked-bys.
>>>
>>> Yes, that's in the minutes. And yes, that's what I got as answer there.
>>> Please reply to the minutes if you think otherwise.
>>
>> I explicitly said that Acked-bys are useless too.
>>
>> The minutes say that you said the kernel treats "Acked-bys" as "looks
>> good".  You did say that.
>
> I *asked* about what to do with my QEMU CPU patches that only get a
> "looks okay" and got only positive answers for whether that should be an
> Acked-by and no objection, including none from you.
> I certainly said nothing at all about the kernel.
>
>>  At no point did a "rule" get made though.
>
> The new rule you made was: no patch without Reviewed-by.
> Peter sending that PULL and Edgar merging it both violate that rule.

I never said anything like that.

Regards,

Anthony Liguori

> No objection against a particular patch function-wise.
>
> Point is, had Peter ping'ed me before sending out that pull, he would've
> actually gotten a Reviewed-by from me, thereby satisfying your rule! He
> didn't, ignoring that he himself had actually told me to queue the
> patches before his vacation, for which obviously I reviewed and tested them.
>
> Maybe there's no obligation for picking up tags, but then again you
> can't go ahead and do statistics over incompletely recorded tags.
>
> Regards,
> Andreas
>
>>> I brought up exactly this situation where I am contributor to CPU and
>>> submaintainer of CPU and often not getting Reviewed-bys but if at all,
>>> such as from Paolo recently, some verbal "looks OK" for a series. I was
>>> told that that should be turned into an Acked-by on the patches to
>>> satisfy your criteria that contributors may not just send patches as
>>> pull without Reviewed-by.
>>
>> I think you misunderstood.
>>
>> I don't care about Acked-bys.  They are useless.
>>
>> A third of patches are being committed with Reviewed-bys.  There are
>> certainly many cases where patches are going in from submaintainers
>> that have been reviewed which comes implicitly with Signed-off-by.
>>
>> But I worry that we're not reviewing enough on list and that there are
>> patches from maintainers going in through maintainer trees that aren't
>> getting outside review.
>>
>> There's no immediate action for this other than we should all try to
>> review more patches on list to prevent the above situation.
>>
>>>> And adding tags is a nice-to-have.  There is no "rule" stating that
>>>> you must include everyone that appears on the mailing list.  But I
>>>> expect that maintainers try to
>>>
>>> Again, at QEMU Summit you pushed for making Reviewed-by a must-have and
>>> we discussed whether a submaintainer must add a Reviewed-by then and
>>> what to do if author==submaintainer. If you dropped that thought, then
>>> fine with me.
>>
>> Yes, patches should get reviewed.  I hope this is obvious to all of us :-)
>>
>> I also suggested that I have tooling that people can use to simplify
>> adding collected Reviewed-bys on the list.
>>
>> But none of this has anything to do with inferred Acked-bys.  I'll go
>> a step further and say that I would be very unhappy if anyone every
>> added any kind of tag to a patch with my name on it that I didn't send
>> myself.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>> Regards,
>>> Andreas
>>>
>>> --
>>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 31 October 2013 16:52, Andreas Färber <afaerber@suse.de> wrote:
> I *asked* about what to do with my QEMU CPU patches that only get a
> "looks okay" and got only positive answers for whether that should be an
> Acked-by and no objection, including none from you.

I agreed with that because IMHO you may treat a "looks ok" from
a relevant subsystem maintainer like an acked-by. There is no
*obligation* to do so -- it's merely that if you think it's worth
noting and it will help get your patches upstream you can.

> Point is, had Peter ping'ed me before sending out that pull, he would've
> actually gotten a Reviewed-by from me, thereby satisfying your rule! He
> didn't, ignoring that he himself had actually told me to queue the
> patches before his vacation, for which obviously I reviewed and tested them.

I told you to queue the patches because you needed them as prereqs
and I was expecting the timing to work out such that you'd get a pullreq
taken so they'd get upstream while I was away.
Since it didn't and I wanted them in 1.7 I put them in my pullreq (which
is technically the better place for them since they're ARM patches, not
QOM ones). I don't see this as a big deal.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Andreas Färber]

Am 31.10.2013 17:54, schrieb Anthony Liguori:
> On Thu, Oct 31, 2013 at 5:52 PM, Andreas Färber <afaerber@suse.de> wrote:
>> The new rule you made was: no patch without Reviewed-by.
>> Peter sending that PULL and Edgar merging it both violate that rule.
> 
> I never said anything like that.

I could've sworn you did and that prompted Peter(?) to ask whether
submaintainers taking a patch from someone else should add a
Reviewed-by, too...

Then this whole discussion is moot and we just need to fix the minutes:

http://www.mail-archive.com/qemu-devel@nongnu.org/msg199693.html

Regards,
Andreas

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Andreas Färber]

Am 31.10.2013 16:16, schrieb Peter Maydell:
> On 31 October 2013 14:36, Andreas Färber <afaerber@suse.de> wrote:
>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>>> Peter, since I had picked up the first two patches into my still pending
>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>> gotten an Acked-by.
>>>
>>> Hmm? I don't recall this part of the discussion. If you want the
>>> patches to have an Acked-by from you you need to send mail
>>> to the list with an Acked-by line.
>>
>> No, I added a Signed-off-by.
> 
> I checked my mail and the only thing I can find in reply to those
> patches is a note from you saying you added them to your queue.

Right, and as such they got a Signed-off-by, which should've been
visible in the link I usually add. Here's the pull messages you
should've been cc'ed on:

http://patchwork.ozlabs.org/patch/281630/
http://patchwork.ozlabs.org/patch/281575/

I don't see why I should reply with a Reviewed-by when I pick up patches
- again, same discussion as at QEMU Summit.

>> It was clearly stated that a Reviewed-by
>> needs to be explicitly sent as reply but that "looks okay" should in
>> exactly such a case where sender=submaintainer should be recorded as
>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
> 
> ...but you're not the submaintainer here so I don't think this applies.

It does, because you are the patch author and the ARM submaintainer
sending the pull.

> The point about the kernel practice as I understood it was that
> the kernel folks treat acked-by at about the same level of review as
> "looks ok to me" (ie, very little), not that there's some obligation to
> treat any informal 'looks ok' note as an acked-by. I'm in full agreement
> with Anthony that if you want a tag to appear you should send it
> properly.

If Anthony had been and would be more responsive as to why he didn't
pull the queue containing these patches with two different Sobs, we
wouldn't be having this conversation in the first place. Or had you not
gone on vacation or sent another pull before etc. etc.

Andreas

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 31 October 2013 17:02, Peter Maydell <peter.maydell@linaro.org> wrote:
> I told you to queue the patches because you needed them as prereqs
> and I was expecting the timing to work out such that you'd get a pullreq
> taken so they'd get upstream while I was away.
> Since it didn't and I wanted them in 1.7 I put them in my pullreq (which
> is technically the better place for them since they're ARM patches, not
> QOM ones). I don't see this as a big deal.

...also, to be honest, by the time I got back from holiday I'd pretty
much forgotten about this and they were just another set of patches
in my list of "this should go in and isn't in upstream yet". Sorry for
any confusion.

-- PMM

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 31 October 2013 17:14, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 16:16, schrieb Peter Maydell:
>> On 31 October 2013 14:36, Andreas Färber <afaerber@suse.de> wrote:
>>> It was clearly stated that a Reviewed-by
>>> needs to be explicitly sent as reply but that "looks okay" should in
>>> exactly such a case where sender=submaintainer should be recorded as
>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>
>> ...but you're not the submaintainer here so I don't think this applies.
>
> It does, because you are the patch author and the ARM submaintainer
> sending the pull.

Er, no, because they're ARM subsystem patches. If they'd gone through
your queue and been written by somebody other than me and I'd given
them an acked-by, that would be worth noting (maybe) because it tells
the person applying your queue that I'm happy with these ARM related
patches even though they're not coming through the ARM queue.
Similarly if there were some QOM patches coming through my queue
that might make an acked-by from you useful. But these aren't QOM
patches, they're plain ARM patches, so the only person whose "ack"
is important is mine. Basically an 'ack' says "I have some kind of veto
over these patches and I'm not exercising it".

-- PMM

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Andreas Färber]

Am 31.10.2013 18:18, schrieb Peter Maydell:
> On 31 October 2013 17:14, Andreas Färber <afaerber@suse.de> wrote:
>> Am 31.10.2013 16:16, schrieb Peter Maydell:
>>> On 31 October 2013 14:36, Andreas Färber <afaerber@suse.de> wrote:
>>>> It was clearly stated that a Reviewed-by
>>>> needs to be explicitly sent as reply but that "looks okay" should in
>>>> exactly such a case where sender=submaintainer should be recorded as
>>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>>
>>> ...but you're not the submaintainer here so I don't think this applies.
>>
>> It does, because you are the patch author and the ARM submaintainer
>> sending the pull.
> 
> Er, no, because they're ARM subsystem patches.

You misunderstand. You sending an ARM patch in your ARM PULL with just
your Sob is the same as me sending a CPU patch with just my Sob in my
CPU PULL. That's what I was saying.

It is NOT about whether someone can veto something, it's about getting
external review and formally recognizing that review.
If Anthony is apparently making a retreat on that front, then we don't
necessarily need external review on our own subsystems, but if we want
to evaluate which or how many patches have been reviewed by someone else
then we need to record that in the commit message in *some* way. I don't
care what -by it is as long as we have and respect a clear rule.

Andreas

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 31 October 2013 17:27, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 18:18, schrieb Peter Maydell:
>> On 31 October 2013 17:14, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 31.10.2013 16:16, schrieb Peter Maydell:
>>>> On 31 October 2013 14:36, Andreas Färber <afaerber@suse.de> wrote:
>>>>> It was clearly stated that a Reviewed-by
>>>>> needs to be explicitly sent as reply but that "looks okay" should in
>>>>> exactly such a case where sender=submaintainer should be recorded as
>>>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>>>
>>>> ...but you're not the submaintainer here so I don't think this applies.
>>>
>>> It does, because you are the patch author and the ARM submaintainer
>>> sending the pull.
>>
>> Er, no, because they're ARM subsystem patches.
>
> You misunderstand. You sending an ARM patch in your ARM PULL with just
> your Sob is the same as me sending a CPU patch with just my Sob in my
> CPU PULL.

I agree with this...

> That's what I was saying.

...it's just not at all what you seemed to be saying. I think this is
related to a disagreement about whether acked-by is at all meaningful
for anybody who's not the relevant subsystem maintainer or otherwise
an "authoritative person".

> It is NOT about whether someone can veto something, it's about getting
> external review and formally recognizing that review.

No, that's what Reviewed-by is for. Acked-by is exactly a statement
that "I think this looks OK and my opinion matters", which is implicitly
making the statement that it's not a NAK, ie not a veto. It's a handy
way to avoid somebody further upstream having to make an explicit
query of that person about whether they'd seen this stuff and were
happy with it, nothing more.

So, to be clear:
 * I welcome external review
 * If I get review and people send emails to the list with reviewed-by:
   tags I'll do my best (and my workflow generally helps) to pick up
   and reflect those tags in the pull requests
 * I'm not going to attempt to infer reviewed-by tags from anything
   other than a specific reply to the list with a tag in the proper format
 * pragmatically speaking there are some patches for ARM which do
   not get any third-party review and where patches have been on list
   for a reasonable period of time I'm going to put them in pull requests,
   since we can't stop the world just because we don't have enough
   people willing to code review things
 * acked-by doesn't imply (to me) any kind of level of review beyond "I don't
   object to this", so it is irrelevant for the purposes of "try to make sure
   patches get review" (which is a goal I agree with)
 * nonetheless I'll generally reflect specifically sent acked-by tags
   where I get them, simply because my usual workflow tends to
   result in that
 * I think a general rule that all tags should be sent to the list explicitly
   and nobody should infer them will be simpler and less confusing
   for all concerned

> If Anthony is apparently making a retreat on that front

I don't recall Anthony ever saying that external review was going
to be mandatory. I think it's certainly something we should try to
do better with, but pragmatically speaking we're not going to get
to 100% reviewed overnight. I'd definitely object to any proposal
to enforce full code review by simply refusing to apply nonreviewed
patches now (and I don't think anybody's proposed that).

>, then we don't
> necessarily need external review on our own subsystems, but if we want
> to evaluate which or how many patches have been reviewed by someone else
> then we need to record that in the commit message in *some* way. I don't
> care what -by it is as long as we have and respect a clear rule.

I don't think the rules have ever changed here; they've been broadly
described in the kernel doc that our wiki page points to for at least a
year. If you've reviewed a patch you mark that with Reviewed-by.

-- PMM

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Anthony Liguori]

On Thu, Oct 31, 2013 at 5:52 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 16:04, schrieb Anthony Liguori:
>> On Thu, Oct 31, 2013 at 3:45 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 31.10.2013 15:39, schrieb Anthony Liguori:
>>>> On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>>>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>>>>>> Peter, since I had picked up the first two patches into my still pending
>>>>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>>>>> gotten an Acked-by.
>>>>>>
>>>>>> Hmm? I don't recall this part of the discussion. If you want the
>>>>>> patches to have an Acked-by from you you need to send mail
>>>>>> to the list with an Acked-by line.
>>>>>
>>>>> No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
>>>>> needs to be explicitly sent as reply but that "looks okay" should in
>>>>> exactly such a case where sender=submaintainer should be recorded as
>>>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>>>
>>>> Nope.  If you want there to be an Acked-by, say "Acked-by:".  Don't
>>>> make people infer your Acked-bys.
>>>
>>> Yes, that's in the minutes. And yes, that's what I got as answer there.
>>> Please reply to the minutes if you think otherwise.
>>
>> I explicitly said that Acked-bys are useless too.
>>
>> The minutes say that you said the kernel treats "Acked-bys" as "looks
>> good".  You did say that.
>
> I *asked* about what to do with my QEMU CPU patches that only get a
> "looks okay" and got only positive answers for whether that should be an
> Acked-by and no objection, including none from you.
> I certainly said nothing at all about the kernel.
>
>>  At no point did a "rule" get made though.
>
> The new rule you made was: no patch without Reviewed-by.

Andreas, I have no idea where you're getting this from.  I think you
misunderstood what was discussed at the QEMU Summit.  Again, there are
no new rules.  I spoke about encouraging more reviews on list because
it's something we need to focus on as a community.

I think you need to step back a bit and give folks the benefit of the
doubt.  No one is doing anything malicious here.

Regards,

Anthony Liguori

> Peter sending that PULL and Edgar merging it both violate that rule.
> No objection against a particular patch function-wise.
>
> Point is, had Peter ping'ed me before sending out that pull, he would've
> actually gotten a Reviewed-by from me, thereby satisfying your rule! He
> didn't, ignoring that he himself had actually told me to queue the
> patches before his vacation, for which obviously I reviewed and tested them.
>
> Maybe there's no obligation for picking up tags, but then again you
> can't go ahead and do statistics over incompletely recorded tags.
>
> Regards,
> Andreas
>
>>> I brought up exactly this situation where I am contributor to CPU and
>>> submaintainer of CPU and often not getting Reviewed-bys but if at all,
>>> such as from Paolo recently, some verbal "looks OK" for a series. I was
>>> told that that should be turned into an Acked-by on the patches to
>>> satisfy your criteria that contributors may not just send patches as
>>> pull without Reviewed-by.
>>
>> I think you misunderstood.
>>
>> I don't care about Acked-bys.  They are useless.
>>
>> A third of patches are being committed with Reviewed-bys.  There are
>> certainly many cases where patches are going in from submaintainers
>> that have been reviewed which comes implicitly with Signed-off-by.
>>
>> But I worry that we're not reviewing enough on list and that there are
>> patches from maintainers going in through maintainer trees that aren't
>> getting outside review.
>>
>> There's no immediate action for this other than we should all try to
>> review more patches on list to prevent the above situation.
>>
>>>> And adding tags is a nice-to-have.  There is no "rule" stating that
>>>> you must include everyone that appears on the mailing list.  But I
>>>> expect that maintainers try to
>>>
>>> Again, at QEMU Summit you pushed for making Reviewed-by a must-have and
>>> we discussed whether a submaintainer must add a Reviewed-by then and
>>> what to do if author==submaintainer. If you dropped that thought, then
>>> fine with me.
>>
>> Yes, patches should get reviewed.  I hope this is obvious to all of us :-)
>>
>> I also suggested that I have tooling that people can use to simplify
>> adding collected Reviewed-bys on the list.
>>
>> But none of this has anything to do with inferred Acked-bys.  I'll go
>> a step further and say that I would be very unhappy if anyone every
>> added any kind of tag to a patch with my name on it that I didn't send
>> myself.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>> Regards,
>>> Andreas
>>>
>>> --
>>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Anthony Liguori]

On Thu, Oct 31, 2013 at 6:14 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 31.10.2013 16:16, schrieb Peter Maydell:
>> On 31 October 2013 14:36, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> wrote:
>>>>> Peter, since I had picked up the first two patches into my still pending
>>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>>> gotten an Acked-by.
>>>>
>>>> Hmm? I don't recall this part of the discussion. If you want the
>>>> patches to have an Acked-by from you you need to send mail
>>>> to the list with an Acked-by line.
>>>
>>> No, I added a Signed-off-by.
>>
>> I checked my mail and the only thing I can find in reply to those
>> patches is a note from you saying you added them to your queue.
>
> Right, and as such they got a Signed-off-by, which should've been
> visible in the link I usually add. Here's the pull messages you
> should've been cc'ed on:
>
> http://patchwork.ozlabs.org/patch/281630/
> http://patchwork.ozlabs.org/patch/281575/
>
> I don't see why I should reply with a Reviewed-by when I pick up patches
> - again, same discussion as at QEMU Summit.
>
>>> It was clearly stated that a Reviewed-by
>>> needs to be explicitly sent as reply but that "looks okay" should in
>>> exactly such a case where sender=submaintainer should be recorded as
>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>
>> ...but you're not the submaintainer here so I don't think this applies.
>
> It does, because you are the patch author and the ARM submaintainer
> sending the pull.
>
>> The point about the kernel practice as I understood it was that
>> the kernel folks treat acked-by at about the same level of review as
>> "looks ok to me" (ie, very little), not that there's some obligation to
>> treat any informal 'looks ok' note as an acked-by. I'm in full agreement
>> with Anthony that if you want a tag to appear you should send it
>> properly.
>
> If Anthony had been and would be more responsive as to why he didn't
> pull the queue containing these patches with two different Sobs, we
> wouldn't be having this conversation in the first place. Or had you not
> gone on vacation or sent another pull before etc. etc.

Your tree is broken.  I gave you the errors that it produced.  You
were able to produce your own errors.  It's your responsibility, as a
subsystem maintainer, to test (and fix) your own tree.

Regards,

Anthony Liguori

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Edgar E. Iglesias]

On Thu, Oct 31, 2013 at 03:18:41PM +0100, Andreas Färber wrote:
> Hi,
> 
> Am 31.10.2013 15:02, schrieb Edgar E. Iglesias:
> > On Fri, Oct 25, 2013 at 07:07:23PM +0100, Peter Maydell wrote:
> >> The following changes since commit fc8ead74674b7129e8f31c2595c76658e5622197:
> >>
> >>   Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2013-10-18 10:03:24 -0700)
> >>
> >> are available in the git repository at:
> >>
> >>
> >>   git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20131025
> >>
> >> for you to fetch changes up to 71c903cc3b78fc563122fe40c5cadd050068b91a:
> >>
> >>   integrator: fix Linux boot failure by emulating dbg region (2013-10-25 18:27:07 +0100)
> > 
> > 
> > Applied, thanks all.
> 
> Edgar, there is no merge commit in qemu.git despite this being a signed
> pull. Do you maybe need to upgrade your version of git?

Hi, thanks for letting me know, I'll make sure to keep the merge commit
next time.

Cheers,
Edgar

[Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

Last target-arm pull before rc1. I don't know of any further outstanding
ARM related issues which would need to be fixed for 2.0 so barring any
late-breaking bug reports I think this should be it until release.

thanks
-- PMM

The following changes since commit 059b3527f0229f4d60fd77a317503d42abd5e50f:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-vnc-2' into staging (2014-03-18 16:39:29 +0000)

are available in the git repository at:


  git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20140319

for you to fetch changes up to 09e037354b6f940c18f417f23355cffd23f4fde5:

  target-arm: A64: Add saturating accumulate ops (USQADD/SUQADD) (2014-03-18 23:10:06 +0000)

----------------------------------------------------------------
target-arm queue:
 * last few A64 Neon instructions
 * fix some PL011 UART bugs causing occasional serial lockups
 * fix the non-PCI AHCI device

----------------------------------------------------------------
Alex Bennée (2):
      target-arm: A64: Add saturating int ops (SQNEG/SQABS)
      target-arm: A64: Add saturating accumulate ops (USQADD/SUQADD)

Rob Herring (4):
      ahci: fix sysbus support
      pl011: reset the fifo when enabled or disabled
      pl011: fix UARTRSR accesses corrupting the UARTCR value
      pl011: fix incorrect logic to set the RXFF flag

 hw/char/pl011.c            |  24 ++++--
 hw/ide/ahci.c              |  13 ++--
 target-arm/helper.h        |  34 ++++++---
 target-arm/neon_helper.c   | 187 +++++++++++++++++++++++++++++++++++++++++++++
 target-arm/translate-a64.c | 160 +++++++++++++++++++++++++++++++++++---
 5 files changed, 383 insertions(+), 35 deletions(-)

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 19 March 2014 12:05, Peter Maydell <peter.maydell@linaro.org> wrote:
> Last target-arm pull before rc1. I don't know of any further outstanding
> ARM related issues which would need to be fixed for 2.0 so barring any
> late-breaking bug reports I think this should be it until release.

Applied, thanks.

-- PMM

[Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

This week's collection of target-arm bugfixes...

thanks
-- PMM


The following changes since commit 5563168c530e2cde8e000ee7aa4afc0ea4d0b42e:

  Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2016-07-07 10:29:05 +0100)

are available in the git repository at:


  git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20160707

for you to fetch changes up to 66542f639927bd1420db38a969d5fa8ad1c89ae1:

  i.MX: split the GPT timer implementation into per SOC definitions (2016-07-07 13:47:01 +0100)

----------------------------------------------------------------
target-arm queue:
 * fix a wrong variable type for A64 SYS_HEAPINFO semihosting call
 * xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo
 * aux: fix break that wanted to break two levels out
 * aux: Rename aux.[ch] to auxbus.[ch] for the benefit of Windows
 * hw/block/m25p80: fix resource leak
 * i.MX: split the GPT timer implementation into per SOC definitions

----------------------------------------------------------------
Jean-Christophe Dubois (1):
      i.MX: split the GPT timer implementation into per SOC definitions

Paolo Bonzini (2):
      xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo
      aux: fix break that wanted to break two levels out

Peter Maydell (2):
      target-arm/arm-semi.c: In SYS_HEAPINFO use correct type for 'limit'
      aux: Rename aux.[ch] to auxbus.[ch] for the benefit of Windows

Shannon Zhao (1):
      hw/block/m25p80: fix resource leak

 hw/arm/fsl-imx25.c                  |  2 +-
 hw/arm/fsl-imx31.c                  |  2 +-
 hw/arm/fsl-imx6.c                   |  2 +-
 hw/block/m25p80.c                   |  6 ++--
 hw/display/dpcd.c                   |  2 +-
 hw/display/xlnx_dp.c                | 10 +++---
 hw/misc/Makefile.objs               |  2 +-
 hw/misc/{aux.c => auxbus.c}         | 16 ++++-----
 hw/misc/imx6_ccm.c                  |  6 ++++
 hw/timer/imx_gpt.c                  | 69 +++++++++++++++++++++++++++++++++----
 include/hw/display/xlnx_dp.h        |  2 +-
 include/hw/misc/{aux.h => auxbus.h} |  2 +-
 include/hw/misc/imx_ccm.h           |  5 ++-
 include/hw/timer/imx_gpt.h          |  9 ++++-
 target-arm/arm-semi.c               |  2 +-
 15 files changed, 107 insertions(+), 30 deletions(-)
 rename hw/misc/{aux.c => auxbus.c} (97%)
 rename include/hw/misc/{aux.h => auxbus.h} (99%)

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 7 July 2016 at 14:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This week's collection of target-arm bugfixes...
>
> thanks
> -- PMM
>
>
> The following changes since commit 5563168c530e2cde8e000ee7aa4afc0ea4d0b42e:
>
>   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2016-07-07 10:29:05 +0100)
>
> are available in the git repository at:
>
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20160707
>
> for you to fetch changes up to 66542f639927bd1420db38a969d5fa8ad1c89ae1:
>
>   i.MX: split the GPT timer implementation into per SOC definitions (2016-07-07 13:47:01 +0100)
>
> ----------------------------------------------------------------
> target-arm queue:
>  * fix a wrong variable type for A64 SYS_HEAPINFO semihosting call
>  * xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo
>  * aux: fix break that wanted to break two levels out
>  * aux: Rename aux.[ch] to auxbus.[ch] for the benefit of Windows
>  * hw/block/m25p80: fix resource leak
>  * i.MX: split the GPT timer implementation into per SOC definitions
>

Applied, thanks.

-- PMM

[Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

A set of small bugfixes for arm for 3.0; the "migration was
broken" fixes for SMMUv3 and v7M NVIC with security extensions
are the most significant.

thanks
-- PMM

The following changes since commit 6d9dd5fb9d0e9f4a174f53a0e20a39fbe809c71e:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qobject-2018-07-27-v2' into staging (2018-07-30 09:55:47 +0100)

are available in the Git repository at:

  git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180730

for you to fetch changes up to 0261fb805c00a6f97d143235e7b06b0906bdf898:

  target/arm: Remove duplicate 'host' entry in '-cpu ?' output (2018-07-30 15:07:08 +0100)

----------------------------------------------------------------
target-arm queue:
 * arm/smmuv3: Fix broken VM state migration
 * armv7m_nvic: Fix broken VM state migration
 * hw/arm/sysbus-fdt: Fix assertion in copy_properties_from_host()
 * hw/arm/iotkit: Fix IRQ number for timer1
 * hw/misc/tz-mpc: Zero the LUT on initialization, not just reset
 * target/arm: Remove duplicate 'host' entry in '-cpu ?' output

----------------------------------------------------------------
Dr. David Alan Gilbert (1):
      arm/smmuv3: Fix missing VMSD terminator

Geert Uytterhoeven (1):
      hw/arm/sysbus-fdt: Fix assertion in copy_properties_from_host()

Peter Maydell (3):
      armv7m_nvic: Fix m-security subsection name
      hw/arm/iotkit: Fix IRQ number for timer1
      hw/misc/tz-mpc: Zero the LUT on initialization, not just reset

Philippe Mathieu-Daudé (1):
      target/arm: Remove duplicate 'host' entry in '-cpu ?' output

 hw/arm/iotkit.c       | 2 +-
 hw/arm/smmuv3.c       | 1 +
 hw/arm/sysbus-fdt.c   | 1 +
 hw/intc/armv7m_nvic.c | 2 +-
 hw/misc/tz-mpc.c      | 2 +-
 target/arm/helper.c   | 6 ------
 6 files changed, 5 insertions(+), 9 deletions(-)

Re: [Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

On 30 July 2018 at 15:17, Peter Maydell <peter.maydell@linaro.org> wrote:
> A set of small bugfixes for arm for 3.0; the "migration was
> broken" fixes for SMMUv3 and v7M NVIC with security extensions
> are the most significant.
>
> thanks
> -- PMM
>
> The following changes since commit 6d9dd5fb9d0e9f4a174f53a0e20a39fbe809c71e:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qobject-2018-07-27-v2' into staging (2018-07-30 09:55:47 +0100)
>
> are available in the Git repository at:
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180730
>
> for you to fetch changes up to 0261fb805c00a6f97d143235e7b06b0906bdf898:
>
>   target/arm: Remove duplicate 'host' entry in '-cpu ?' output (2018-07-30 15:07:08 +0100)
>
> ----------------------------------------------------------------
> target-arm queue:
>  * arm/smmuv3: Fix broken VM state migration
>  * armv7m_nvic: Fix broken VM state migration
>  * hw/arm/sysbus-fdt: Fix assertion in copy_properties_from_host()
>  * hw/arm/iotkit: Fix IRQ number for timer1
>  * hw/misc/tz-mpc: Zero the LUT on initialization, not just reset
>  * target/arm: Remove duplicate 'host' entry in '-cpu ?' output
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

[Qemu-devel] [PULL 0/6] target-arm queue

[Peter Maydell]

Last lot of patches for arm before softfreeze tomorrow...

thanks
-- PMM

The following changes since commit ef3a6af5e789ff078d1fef880f9dfb6adf18e8f1:

  Merge remote-tracking branch 'remotes/kraxel/tags/vga-20181029-pull-request' into staging (2018-10-29 12:59:15 +0000)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20181029

for you to fetch changes up to 20cf5663734310a282e27b7389bc9f53ffe227e6:

  tests/boot-serial-test: Add microbit board testcase (2018-10-29 15:19:48 +0000)

----------------------------------------------------------------
target-arm queue:
 * microbit: Add the UART to our nRF51 SoC model
 * Add a virtual Xilinx Versal board "xlnx-versal-virt"
 * hw/arm/virt: Set VIRT_COMPAT_3_0 compat

----------------------------------------------------------------
Edgar E. Iglesias (2):
      hw/arm: versal: Add a model of Xilinx Versal SoC
      hw/arm: versal: Add a virtual Xilinx Versal board

Eric Auger (1):
      hw/arm/virt: Set VIRT_COMPAT_3_0 compat

Julia Suvorova (3):
      hw/char: Implement nRF51 SoC UART
      hw/arm/nrf51_soc: Connect UART to nRF51 SoC
      tests/boot-serial-test: Add microbit board testcase

 hw/arm/Makefile.objs                |   1 +
 hw/char/Makefile.objs               |   1 +
 include/hw/arm/nrf51_soc.h          |   3 +
 include/hw/arm/xlnx-versal.h        | 122 +++++++++
 include/hw/char/nrf51_uart.h        |  78 ++++++
 hw/arm/microbit.c                   |   2 +
 hw/arm/nrf51_soc.c                  |  20 ++
 hw/arm/virt.c                       |   4 +
 hw/arm/xlnx-versal-virt.c           | 493 ++++++++++++++++++++++++++++++++++++
 hw/arm/xlnx-versal.c                | 323 +++++++++++++++++++++++
 hw/char/nrf51_uart.c                | 330 ++++++++++++++++++++++++
 tests/boot-serial-test.c            |  19 ++
 default-configs/aarch64-softmmu.mak |   1 +
 hw/char/trace-events                |   4 +
 14 files changed, 1401 insertions(+)
 create mode 100644 include/hw/arm/xlnx-versal.h
 create mode 100644 include/hw/char/nrf51_uart.h
 create mode 100644 hw/arm/xlnx-versal-virt.c
 create mode 100644 hw/arm/xlnx-versal.c
 create mode 100644 hw/char/nrf51_uart.c

[Qemu-devel] [PULL 1/6] hw/arm/virt: Set VIRT_COMPAT_3_0 compat

[Peter Maydell]

From: Eric Auger <eric.auger@redhat.com>

We are missing the VIRT_COMPAT_3_0 definition and setting.
Let's add them.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-id: 20181024085602.16611-1-eric.auger@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9f677825f9f..a2b8d8f7c2c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1871,6 +1871,9 @@ static void virt_machine_3_1_options(MachineClass *mc)
 }
 DEFINE_VIRT_MACHINE_AS_LATEST(3, 1)
 
+#define VIRT_COMPAT_3_0 \
+    HW_COMPAT_3_0
+
 static void virt_3_0_instance_init(Object *obj)
 {
     virt_3_1_instance_init(obj);
@@ -1879,6 +1882,7 @@ static void virt_3_0_instance_init(Object *obj)
 static void virt_machine_3_0_options(MachineClass *mc)
 {
     virt_machine_3_1_options(mc);
+    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_3_0);
 }
 DEFINE_VIRT_MACHINE(3, 0)
 
-- 
2.19.1

[Qemu-devel] [PULL 2/6] hw/arm: versal: Add a model of Xilinx Versal SoC

[Peter Maydell]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add a model of Xilinx Versal SoC.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/Makefile.objs                |   1 +
 include/hw/arm/xlnx-versal.h        | 122 +++++++++++
 hw/arm/xlnx-versal.c                | 323 ++++++++++++++++++++++++++++
 default-configs/aarch64-softmmu.mak |   1 +
 4 files changed, 447 insertions(+)
 create mode 100644 include/hw/arm/xlnx-versal.h
 create mode 100644 hw/arm/xlnx-versal.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 5f88062c666..ec21d9bc1f0 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -26,6 +26,7 @@ obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
 obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
+obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.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
diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
new file mode 100644
index 00000000000..9da621e4b68
--- /dev/null
+++ b/include/hw/arm/xlnx-versal.h
@@ -0,0 +1,122 @@
+/*
+ * Model of the Xilinx Versal
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ * Written by Edgar E. Iglesias
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#ifndef XLNX_VERSAL_H
+#define XLNX_VERSAL_H
+
+#include "hw/sysbus.h"
+#include "hw/arm/arm.h"
+#include "hw/intc/arm_gicv3.h"
+
+#define TYPE_XLNX_VERSAL "xlnx-versal"
+#define XLNX_VERSAL(obj) OBJECT_CHECK(Versal, (obj), TYPE_XLNX_VERSAL)
+
+#define XLNX_VERSAL_NR_ACPUS   2
+#define XLNX_VERSAL_NR_UARTS   2
+#define XLNX_VERSAL_NR_GEMS    2
+#define XLNX_VERSAL_NR_IRQS    256
+
+typedef struct Versal {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    struct {
+        struct {
+            MemoryRegion mr;
+            ARMCPU *cpu[XLNX_VERSAL_NR_ACPUS];
+            GICv3State gic;
+        } apu;
+    } fpd;
+
+    MemoryRegion mr_ps;
+
+    struct {
+        /* 4 ranges to access DDR.  */
+        MemoryRegion mr_ddr_ranges[4];
+    } noc;
+
+    struct {
+        MemoryRegion mr_ocm;
+
+        struct {
+            SysBusDevice *uart[XLNX_VERSAL_NR_UARTS];
+            SysBusDevice *gem[XLNX_VERSAL_NR_GEMS];
+        } iou;
+    } lpd;
+
+    struct {
+        MemoryRegion *mr_ddr;
+        uint32_t psci_conduit;
+    } cfg;
+} Versal;
+
+/* Memory-map and IRQ definitions. Copied a subset from
+ * auto-generated files.  */
+
+#define VERSAL_GIC_MAINT_IRQ        9
+#define VERSAL_TIMER_VIRT_IRQ       11
+#define VERSAL_TIMER_S_EL1_IRQ      13
+#define VERSAL_TIMER_NS_EL1_IRQ     14
+#define VERSAL_TIMER_NS_EL2_IRQ     10
+
+#define VERSAL_UART0_IRQ_0         18
+#define VERSAL_UART1_IRQ_0         19
+#define VERSAL_GEM0_IRQ_0          56
+#define VERSAL_GEM0_WAKE_IRQ_0     57
+#define VERSAL_GEM1_IRQ_0          58
+#define VERSAL_GEM1_WAKE_IRQ_0     59
+
+/* Architecturally eserved IRQs suitable for virtualization.  */
+#define VERSAL_RSVD_HIGH_IRQ_FIRST 160
+#define VERSAL_RSVD_HIGH_IRQ_LAST  255
+
+#define MM_TOP_RSVD                 0xa0000000U
+#define MM_TOP_RSVD_SIZE            0x4000000
+#define MM_GIC_APU_DIST_MAIN        0xf9000000U
+#define MM_GIC_APU_DIST_MAIN_SIZE   0x10000
+#define MM_GIC_APU_REDIST_0         0xf9080000U
+#define MM_GIC_APU_REDIST_0_SIZE    0x80000
+
+#define MM_UART0                    0xff000000U
+#define MM_UART0_SIZE               0x10000
+#define MM_UART1                    0xff010000U
+#define MM_UART1_SIZE               0x10000
+
+#define MM_GEM0                     0xff0c0000U
+#define MM_GEM0_SIZE                0x10000
+#define MM_GEM1                     0xff0d0000U
+#define MM_GEM1_SIZE                0x10000
+
+#define MM_OCM                      0xfffc0000U
+#define MM_OCM_SIZE                 0x40000
+
+#define MM_TOP_DDR                  0x0
+#define MM_TOP_DDR_SIZE             0x80000000U
+#define MM_TOP_DDR_2                0x800000000ULL
+#define MM_TOP_DDR_2_SIZE           0x800000000ULL
+#define MM_TOP_DDR_3                0xc000000000ULL
+#define MM_TOP_DDR_3_SIZE           0x4000000000ULL
+#define MM_TOP_DDR_4                0x10000000000ULL
+#define MM_TOP_DDR_4_SIZE           0xb780000000ULL
+
+#define MM_PSM_START                0xffc80000U
+#define MM_PSM_END                  0xffcf0000U
+
+#define MM_CRL                      0xff5e0000U
+#define MM_CRL_SIZE                 0x300000
+#define MM_IOU_SCNTR                0xff130000U
+#define MM_IOU_SCNTR_SIZE           0x10000
+#define MM_IOU_SCNTRS               0xff140000U
+#define MM_IOU_SCNTRS_SIZE          0x10000
+#define MM_FPD_CRF                  0xfd1a0000U
+#define MM_FPD_CRF_SIZE             0x140000
+#endif
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
new file mode 100644
index 00000000000..5ee58c09be8
--- /dev/null
+++ b/hw/arm/xlnx-versal.c
@@ -0,0 +1,323 @@
+/*
+ * Xilinx Versal SoC model.
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ * Written by Edgar E. Iglesias
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "qemu/log.h"
+#include "hw/sysbus.h"
+#include "net/net.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/kvm.h"
+#include "hw/arm/arm.h"
+#include "kvm_arm.h"
+#include "hw/misc/unimp.h"
+#include "hw/intc/arm_gicv3_common.h"
+#include "hw/arm/xlnx-versal.h"
+
+#define XLNX_VERSAL_ACPU_TYPE ARM_CPU_TYPE_NAME("cortex-a72")
+#define GEM_REVISION        0x40070106
+
+static void versal_create_apu_cpus(Versal *s)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->fpd.apu.cpu); i++) {
+        Object *obj;
+        char *name;
+
+        obj = object_new(XLNX_VERSAL_ACPU_TYPE);
+        if (!obj) {
+            /* Secondary CPUs start in PSCI powered-down state */
+            error_report("Unable to create apu.cpu[%d] of type %s",
+                         i, XLNX_VERSAL_ACPU_TYPE);
+            exit(EXIT_FAILURE);
+        }
+
+        name = g_strdup_printf("apu-cpu[%d]", i);
+        object_property_add_child(OBJECT(s), name, obj, &error_fatal);
+        g_free(name);
+
+        object_property_set_int(obj, s->cfg.psci_conduit,
+                                "psci-conduit", &error_abort);
+        if (i) {
+            object_property_set_bool(obj, true,
+                                     "start-powered-off", &error_abort);
+        }
+
+        object_property_set_int(obj, ARRAY_SIZE(s->fpd.apu.cpu),
+                                "core-count", &error_abort);
+        object_property_set_link(obj, OBJECT(&s->fpd.apu.mr), "memory",
+                                 &error_abort);
+        object_property_set_bool(obj, true, "realized", &error_fatal);
+        s->fpd.apu.cpu[i] = ARM_CPU(obj);
+    }
+}
+
+static void versal_create_apu_gic(Versal *s, qemu_irq *pic)
+{
+    static const uint64_t addrs[] = {
+        MM_GIC_APU_DIST_MAIN,
+        MM_GIC_APU_REDIST_0
+    };
+    SysBusDevice *gicbusdev;
+    DeviceState *gicdev;
+    int nr_apu_cpus = ARRAY_SIZE(s->fpd.apu.cpu);
+    int i;
+
+    sysbus_init_child_obj(OBJECT(s), "apu-gic",
+                          &s->fpd.apu.gic, sizeof(s->fpd.apu.gic),
+                          gicv3_class_name());
+    gicbusdev = SYS_BUS_DEVICE(&s->fpd.apu.gic);
+    gicdev = DEVICE(&s->fpd.apu.gic);
+    qdev_prop_set_uint32(gicdev, "revision", 3);
+    qdev_prop_set_uint32(gicdev, "num-cpu", 2);
+    qdev_prop_set_uint32(gicdev, "num-irq", XLNX_VERSAL_NR_IRQS + 32);
+    qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
+    qdev_prop_set_uint32(gicdev, "redist-region-count[0]", 2);
+    qdev_prop_set_bit(gicdev, "has-security-extensions", true);
+
+    object_property_set_bool(OBJECT(&s->fpd.apu.gic), true, "realized",
+                                    &error_fatal);
+
+    for (i = 0; i < ARRAY_SIZE(addrs); i++) {
+        MemoryRegion *mr;
+
+        mr = sysbus_mmio_get_region(gicbusdev, i);
+        memory_region_add_subregion(&s->fpd.apu.mr, addrs[i], mr);
+    }
+
+    for (i = 0; i < nr_apu_cpus; i++) {
+        DeviceState *cpudev = DEVICE(s->fpd.apu.cpu[i]);
+        int ppibase = XLNX_VERSAL_NR_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
+        qemu_irq maint_irq;
+        int ti;
+        /* Mapping from the output timer irq lines from the CPU to the
+         * GIC PPI inputs.
+         */
+        const int timer_irq[] = {
+            [GTIMER_PHYS] = VERSAL_TIMER_NS_EL1_IRQ,
+            [GTIMER_VIRT] = VERSAL_TIMER_VIRT_IRQ,
+            [GTIMER_HYP]  = VERSAL_TIMER_NS_EL2_IRQ,
+            [GTIMER_SEC]  = VERSAL_TIMER_S_EL1_IRQ,
+        };
+
+        for (ti = 0; ti < ARRAY_SIZE(timer_irq); ti++) {
+            qdev_connect_gpio_out(cpudev, ti,
+                                  qdev_get_gpio_in(gicdev,
+                                                   ppibase + timer_irq[ti]));
+        }
+        maint_irq = qdev_get_gpio_in(gicdev,
+                                        ppibase + VERSAL_GIC_MAINT_IRQ);
+        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
+                                    0, maint_irq);
+        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
+        sysbus_connect_irq(gicbusdev, i + nr_apu_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
+        sysbus_connect_irq(gicbusdev, i + 2 * nr_apu_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
+        sysbus_connect_irq(gicbusdev, i + 3 * nr_apu_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+    }
+
+    for (i = 0; i < XLNX_VERSAL_NR_IRQS; i++) {
+        pic[i] = qdev_get_gpio_in(gicdev, i);
+    }
+}
+
+static void versal_create_uarts(Versal *s, qemu_irq *pic)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->lpd.iou.uart); i++) {
+        static const int irqs[] = { VERSAL_UART0_IRQ_0, VERSAL_UART1_IRQ_0};
+        static const uint64_t addrs[] = { MM_UART0, MM_UART1 };
+        char *name = g_strdup_printf("uart%d", i);
+        DeviceState *dev;
+        MemoryRegion *mr;
+
+        dev = qdev_create(NULL, "pl011");
+        s->lpd.iou.uart[i] = SYS_BUS_DEVICE(dev);
+        qdev_prop_set_chr(dev, "chardev", serial_hd(i));
+        object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
+        qdev_init_nofail(dev);
+
+        mr = sysbus_mmio_get_region(s->lpd.iou.uart[i], 0);
+        memory_region_add_subregion(&s->mr_ps, addrs[i], mr);
+
+        sysbus_connect_irq(s->lpd.iou.uart[i], 0, pic[irqs[i]]);
+        g_free(name);
+    }
+}
+
+static void versal_create_gems(Versal *s, qemu_irq *pic)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->lpd.iou.gem); i++) {
+        static const int irqs[] = { VERSAL_GEM0_IRQ_0, VERSAL_GEM1_IRQ_0};
+        static const uint64_t addrs[] = { MM_GEM0, MM_GEM1 };
+        char *name = g_strdup_printf("gem%d", i);
+        NICInfo *nd = &nd_table[i];
+        DeviceState *dev;
+        MemoryRegion *mr;
+
+        dev = qdev_create(NULL, "cadence_gem");
+        s->lpd.iou.gem[i] = SYS_BUS_DEVICE(dev);
+        object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
+        if (nd->used) {
+            qemu_check_nic_model(nd, "cadence_gem");
+            qdev_set_nic_properties(dev, nd);
+        }
+        object_property_set_int(OBJECT(s->lpd.iou.gem[i]),
+                                2, "num-priority-queues",
+                                &error_abort);
+        object_property_set_link(OBJECT(s->lpd.iou.gem[i]),
+                                 OBJECT(&s->mr_ps), "dma",
+                                 &error_abort);
+        qdev_init_nofail(dev);
+
+        mr = sysbus_mmio_get_region(s->lpd.iou.gem[i], 0);
+        memory_region_add_subregion(&s->mr_ps, addrs[i], mr);
+
+        sysbus_connect_irq(s->lpd.iou.gem[i], 0, pic[irqs[i]]);
+        g_free(name);
+    }
+}
+
+/* This takes the board allocated linear DDR memory and creates aliases
+ * for each split DDR range/aperture on the Versal address map.
+ */
+static void versal_map_ddr(Versal *s)
+{
+    uint64_t size = memory_region_size(s->cfg.mr_ddr);
+    /* Describes the various split DDR access regions.  */
+    static const struct {
+        uint64_t base;
+        uint64_t size;
+    } addr_ranges[] = {
+        { MM_TOP_DDR, MM_TOP_DDR_SIZE },
+        { MM_TOP_DDR_2, MM_TOP_DDR_2_SIZE },
+        { MM_TOP_DDR_3, MM_TOP_DDR_3_SIZE },
+        { MM_TOP_DDR_4, MM_TOP_DDR_4_SIZE }
+    };
+    uint64_t offset = 0;
+    int i;
+
+    assert(ARRAY_SIZE(addr_ranges) == ARRAY_SIZE(s->noc.mr_ddr_ranges));
+    for (i = 0; i < ARRAY_SIZE(addr_ranges) && size; i++) {
+        char *name;
+        uint64_t mapsize;
+
+        mapsize = size < addr_ranges[i].size ? size : addr_ranges[i].size;
+        name = g_strdup_printf("noc-ddr-range%d", i);
+        /* Create the MR alias.  */
+        memory_region_init_alias(&s->noc.mr_ddr_ranges[i], OBJECT(s),
+                                 name, s->cfg.mr_ddr,
+                                 offset, mapsize);
+
+        /* Map it onto the NoC MR.  */
+        memory_region_add_subregion(&s->mr_ps, addr_ranges[i].base,
+                                    &s->noc.mr_ddr_ranges[i]);
+        offset += mapsize;
+        size -= mapsize;
+        g_free(name);
+    }
+}
+
+static void versal_unimp_area(Versal *s, const char *name,
+                                MemoryRegion *mr,
+                                hwaddr base, hwaddr size)
+{
+    DeviceState *dev = qdev_create(NULL, TYPE_UNIMPLEMENTED_DEVICE);
+    MemoryRegion *mr_dev;
+
+    qdev_prop_set_string(dev, "name", name);
+    qdev_prop_set_uint64(dev, "size", size);
+    object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal);
+    qdev_init_nofail(dev);
+
+    mr_dev = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+    memory_region_add_subregion(mr, base, mr_dev);
+}
+
+static void versal_unimp(Versal *s)
+{
+    versal_unimp_area(s, "psm", &s->mr_ps,
+                        MM_PSM_START, MM_PSM_END - MM_PSM_START);
+    versal_unimp_area(s, "crl", &s->mr_ps,
+                        MM_CRL, MM_CRL_SIZE);
+    versal_unimp_area(s, "crf", &s->mr_ps,
+                        MM_FPD_CRF, MM_FPD_CRF_SIZE);
+    versal_unimp_area(s, "iou-scntr", &s->mr_ps,
+                        MM_IOU_SCNTR, MM_IOU_SCNTR_SIZE);
+    versal_unimp_area(s, "iou-scntr-seucre", &s->mr_ps,
+                        MM_IOU_SCNTRS, MM_IOU_SCNTRS_SIZE);
+}
+
+static void versal_realize(DeviceState *dev, Error **errp)
+{
+    Versal *s = XLNX_VERSAL(dev);
+    qemu_irq pic[XLNX_VERSAL_NR_IRQS];
+
+    versal_create_apu_cpus(s);
+    versal_create_apu_gic(s, pic);
+    versal_create_uarts(s, pic);
+    versal_create_gems(s, pic);
+    versal_map_ddr(s);
+    versal_unimp(s);
+
+    /* Create the On Chip Memory (OCM).  */
+    memory_region_init_ram(&s->lpd.mr_ocm, OBJECT(s), "ocm",
+                           MM_OCM_SIZE, &error_fatal);
+
+    memory_region_add_subregion_overlap(&s->mr_ps, MM_OCM, &s->lpd.mr_ocm, 0);
+    memory_region_add_subregion_overlap(&s->fpd.apu.mr, 0, &s->mr_ps, 0);
+}
+
+static void versal_init(Object *obj)
+{
+    Versal *s = XLNX_VERSAL(obj);
+
+    memory_region_init(&s->fpd.apu.mr, obj, "mr-apu", UINT64_MAX);
+    memory_region_init(&s->mr_ps, obj, "mr-ps-switch", UINT64_MAX);
+}
+
+static Property versal_properties[] = {
+    DEFINE_PROP_LINK("ddr", Versal, cfg.mr_ddr, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
+    DEFINE_PROP_UINT32("psci-conduit", Versal, cfg.psci_conduit, 0),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void versal_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = versal_realize;
+    dc->props = versal_properties;
+    /* No VMSD since we haven't got any top-level SoC state to save.  */
+}
+
+static const TypeInfo versal_info = {
+    .name = TYPE_XLNX_VERSAL,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(Versal),
+    .instance_init = versal_init,
+    .class_init = versal_class_init,
+};
+
+static void versal_register_types(void)
+{
+    type_register_static(&versal_info);
+}
+
+type_init(versal_register_types);
diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak
index 6f790f061a1..4ea9add0034 100644
--- a/default-configs/aarch64-softmmu.mak
+++ b/default-configs/aarch64-softmmu.mak
@@ -8,4 +8,5 @@ CONFIG_DDC=y
 CONFIG_DPCD=y
 CONFIG_XLNX_ZYNQMP=y
 CONFIG_XLNX_ZYNQMP_ARM=y
+CONFIG_XLNX_VERSAL=y
 CONFIG_ARM_SMMUV3=y
-- 
2.19.1

[Qemu-devel] [PULL 3/6] hw/arm: versal: Add a virtual Xilinx Versal board

[Peter Maydell]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add a virtual Xilinx Versal board.

This board is based on the Xilinx Versal SoC. The exact
details of what peripherals are attached to this board
will remain in control of QEMU. QEMU will generate an
FDT on the fly for Linux and other software to auto-discover
peripherals.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
[PMM: removed stray blank line at EOF]
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/Makefile.objs      |   2 +-
 hw/arm/xlnx-versal-virt.c | 493 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 494 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/xlnx-versal-virt.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index ec21d9bc1f0..50c7b4a927d 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -26,7 +26,7 @@ obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
 obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
 obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
-obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.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
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
new file mode 100644
index 00000000000..63083888188
--- /dev/null
+++ b/hw/arm/xlnx-versal-virt.c
@@ -0,0 +1,493 @@
+/*
+ * Xilinx Versal Virtual board.
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ * Written by Edgar E. Iglesias
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "sysemu/device_tree.h"
+#include "exec/address-spaces.h"
+#include "hw/boards.h"
+#include "hw/sysbus.h"
+#include "hw/arm/sysbus-fdt.h"
+#include "hw/arm/fdt.h"
+#include "cpu.h"
+#include "hw/arm/xlnx-versal.h"
+
+#define TYPE_XLNX_VERSAL_VIRT_MACHINE MACHINE_TYPE_NAME("xlnx-versal-virt")
+#define XLNX_VERSAL_VIRT_MACHINE(obj) \
+    OBJECT_CHECK(VersalVirt, (obj), TYPE_XLNX_VERSAL_VIRT_MACHINE)
+
+typedef struct VersalVirt {
+    MachineState parent_obj;
+
+    Versal soc;
+    MemoryRegion mr_ddr;
+
+    void *fdt;
+    int fdt_size;
+    struct {
+        uint32_t gic;
+        uint32_t ethernet_phy[2];
+        uint32_t clk_125Mhz;
+        uint32_t clk_25Mhz;
+    } phandle;
+    struct arm_boot_info binfo;
+
+    struct {
+        bool secure;
+    } cfg;
+} VersalVirt;
+
+static void fdt_create(VersalVirt *s)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(s);
+    int i;
+
+    s->fdt = create_device_tree(&s->fdt_size);
+    if (!s->fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
+    }
+
+    /* Allocate all phandles.  */
+    s->phandle.gic = qemu_fdt_alloc_phandle(s->fdt);
+    for (i = 0; i < ARRAY_SIZE(s->phandle.ethernet_phy); i++) {
+        s->phandle.ethernet_phy[i] = qemu_fdt_alloc_phandle(s->fdt);
+    }
+    s->phandle.clk_25Mhz = qemu_fdt_alloc_phandle(s->fdt);
+    s->phandle.clk_125Mhz = qemu_fdt_alloc_phandle(s->fdt);
+
+    /* Create /chosen node for load_dtb.  */
+    qemu_fdt_add_subnode(s->fdt, "/chosen");
+
+    /* Header */
+    qemu_fdt_setprop_cell(s->fdt, "/", "interrupt-parent", s->phandle.gic);
+    qemu_fdt_setprop_cell(s->fdt, "/", "#size-cells", 0x2);
+    qemu_fdt_setprop_cell(s->fdt, "/", "#address-cells", 0x2);
+    qemu_fdt_setprop_string(s->fdt, "/", "model", mc->desc);
+    qemu_fdt_setprop_string(s->fdt, "/", "compatible", "xlnx-versal-virt");
+}
+
+static void fdt_add_clk_node(VersalVirt *s, const char *name,
+                             unsigned int freq_hz, uint32_t phandle)
+{
+    qemu_fdt_add_subnode(s->fdt, name);
+    qemu_fdt_setprop_cell(s->fdt, name, "phandle", phandle);
+    qemu_fdt_setprop_cell(s->fdt, name, "clock-frequency", freq_hz);
+    qemu_fdt_setprop_cell(s->fdt, name, "#clock-cells", 0x0);
+    qemu_fdt_setprop_string(s->fdt, name, "compatible", "fixed-clock");
+    qemu_fdt_setprop(s->fdt, name, "u-boot,dm-pre-reloc", NULL, 0);
+}
+
+static void fdt_add_cpu_nodes(VersalVirt *s, uint32_t psci_conduit)
+{
+    int i;
+
+    qemu_fdt_add_subnode(s->fdt, "/cpus");
+    qemu_fdt_setprop_cell(s->fdt, "/cpus", "#size-cells", 0x0);
+    qemu_fdt_setprop_cell(s->fdt, "/cpus", "#address-cells", 1);
+
+    for (i = XLNX_VERSAL_NR_ACPUS - 1; i >= 0; i--) {
+        char *name = g_strdup_printf("/cpus/cpu@%d", i);
+        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i));
+
+        qemu_fdt_add_subnode(s->fdt, name);
+        qemu_fdt_setprop_cell(s->fdt, name, "reg", armcpu->mp_affinity);
+        if (psci_conduit != QEMU_PSCI_CONDUIT_DISABLED) {
+            qemu_fdt_setprop_string(s->fdt, name, "enable-method", "psci");
+        }
+        qemu_fdt_setprop_string(s->fdt, name, "device_type", "cpu");
+        qemu_fdt_setprop_string(s->fdt, name, "compatible",
+                                armcpu->dtb_compatible);
+        g_free(name);
+    }
+}
+
+static void fdt_add_gic_nodes(VersalVirt *s)
+{
+    char *nodename;
+
+    nodename = g_strdup_printf("/gic@%x", MM_GIC_APU_DIST_MAIN);
+    qemu_fdt_add_subnode(s->fdt, nodename);
+    qemu_fdt_setprop_cell(s->fdt, nodename, "phandle", s->phandle.gic);
+    qemu_fdt_setprop_cells(s->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_PPI, VERSAL_GIC_MAINT_IRQ,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    qemu_fdt_setprop(s->fdt, nodename, "interrupt-controller", NULL, 0);
+    qemu_fdt_setprop_sized_cells(s->fdt, nodename, "reg",
+                                 2, MM_GIC_APU_DIST_MAIN,
+                                 2, MM_GIC_APU_DIST_MAIN_SIZE,
+                                 2, MM_GIC_APU_REDIST_0,
+                                 2, MM_GIC_APU_REDIST_0_SIZE);
+    qemu_fdt_setprop_cell(s->fdt, nodename, "#interrupt-cells", 3);
+    qemu_fdt_setprop_string(s->fdt, nodename, "compatible", "arm,gic-v3");
+}
+
+static void fdt_add_timer_nodes(VersalVirt *s)
+{
+    const char compat[] = "arm,armv8-timer";
+    uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
+
+    qemu_fdt_add_subnode(s->fdt, "/timer");
+    qemu_fdt_setprop_cells(s->fdt, "/timer", "interrupts",
+            GIC_FDT_IRQ_TYPE_PPI, VERSAL_TIMER_S_EL1_IRQ, irqflags,
+            GIC_FDT_IRQ_TYPE_PPI, VERSAL_TIMER_NS_EL1_IRQ, irqflags,
+            GIC_FDT_IRQ_TYPE_PPI, VERSAL_TIMER_VIRT_IRQ, irqflags,
+            GIC_FDT_IRQ_TYPE_PPI, VERSAL_TIMER_NS_EL2_IRQ, irqflags);
+    qemu_fdt_setprop(s->fdt, "/timer", "compatible",
+                     compat, sizeof(compat));
+}
+
+static void fdt_add_uart_nodes(VersalVirt *s)
+{
+    uint64_t addrs[] = { MM_UART1, MM_UART0 };
+    unsigned int irqs[] = { VERSAL_UART1_IRQ_0, VERSAL_UART0_IRQ_0 };
+    const char compat[] = "arm,pl011\0arm,sbsa-uart";
+    const char clocknames[] = "uartclk\0apb_pclk";
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(addrs); i++) {
+        char *name = g_strdup_printf("/uart@%" PRIx64, addrs[i]);
+        qemu_fdt_add_subnode(s->fdt, name);
+        qemu_fdt_setprop_cell(s->fdt, name, "current-speed", 115200);
+        qemu_fdt_setprop_cells(s->fdt, name, "clocks",
+                               s->phandle.clk_125Mhz, s->phandle.clk_125Mhz);
+        qemu_fdt_setprop(s->fdt, name, "clock-names",
+                         clocknames, sizeof(clocknames));
+
+        qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
+                               GIC_FDT_IRQ_TYPE_SPI, irqs[i],
+                               GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+        qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
+                                     2, addrs[i], 2, 0x1000);
+        qemu_fdt_setprop(s->fdt, name, "compatible",
+                         compat, sizeof(compat));
+        qemu_fdt_setprop(s->fdt, name, "u-boot,dm-pre-reloc", NULL, 0);
+
+        if (addrs[i] == MM_UART0) {
+            /* Select UART0.  */
+            qemu_fdt_setprop_string(s->fdt, "/chosen", "stdout-path", name);
+        }
+        g_free(name);
+    }
+}
+
+static void fdt_add_fixed_link_nodes(VersalVirt *s, char *gemname,
+                                     uint32_t phandle)
+{
+    char *name = g_strdup_printf("%s/fixed-link", gemname);
+
+    qemu_fdt_add_subnode(s->fdt, name);
+    qemu_fdt_setprop_cell(s->fdt, name, "phandle", phandle);
+    qemu_fdt_setprop_cells(s->fdt, name, "full-duplex");
+    qemu_fdt_setprop_cell(s->fdt, name, "speed", 1000);
+    g_free(name);
+}
+
+static void fdt_add_gem_nodes(VersalVirt *s)
+{
+    uint64_t addrs[] = { MM_GEM1, MM_GEM0 };
+    unsigned int irqs[] = { VERSAL_GEM1_IRQ_0, VERSAL_GEM0_IRQ_0 };
+    const char clocknames[] = "pclk\0hclk\0tx_clk\0rx_clk";
+    const char compat_gem[] = "cdns,zynqmp-gem\0cdns,gem";
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(addrs); i++) {
+        char *name = g_strdup_printf("/ethernet@%" PRIx64, addrs[i]);
+        qemu_fdt_add_subnode(s->fdt, name);
+
+        fdt_add_fixed_link_nodes(s, name, s->phandle.ethernet_phy[i]);
+        qemu_fdt_setprop_string(s->fdt, name, "phy-mode", "rgmii-id");
+        qemu_fdt_setprop_cell(s->fdt, name, "phy-handle",
+                              s->phandle.ethernet_phy[i]);
+        qemu_fdt_setprop_cells(s->fdt, name, "clocks",
+                               s->phandle.clk_25Mhz, s->phandle.clk_25Mhz,
+                               s->phandle.clk_25Mhz, s->phandle.clk_25Mhz);
+        qemu_fdt_setprop(s->fdt, name, "clock-names",
+                         clocknames, sizeof(clocknames));
+        qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
+                               GIC_FDT_IRQ_TYPE_SPI, irqs[i],
+                               GIC_FDT_IRQ_FLAGS_LEVEL_HI,
+                               GIC_FDT_IRQ_TYPE_SPI, irqs[i],
+                               GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+        qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
+                                     2, addrs[i], 2, 0x1000);
+        qemu_fdt_setprop(s->fdt, name, "compatible",
+                         compat_gem, sizeof(compat_gem));
+        qemu_fdt_setprop_cell(s->fdt, name, "#address-cells", 1);
+        qemu_fdt_setprop_cell(s->fdt, name, "#size-cells", 0);
+        g_free(name);
+    }
+}
+
+static void fdt_nop_memory_nodes(void *fdt, Error **errp)
+{
+    Error *err = NULL;
+    char **node_path;
+    int n = 0;
+
+    node_path = qemu_fdt_node_unit_path(fdt, "memory", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    while (node_path[n]) {
+        if (g_str_has_prefix(node_path[n], "/memory")) {
+            qemu_fdt_nop_node(fdt, node_path[n]);
+        }
+        n++;
+    }
+    g_strfreev(node_path);
+}
+
+static void fdt_add_memory_nodes(VersalVirt *s, void *fdt, uint64_t ram_size)
+{
+    /* Describes the various split DDR access regions.  */
+    static const struct {
+        uint64_t base;
+        uint64_t size;
+    } addr_ranges[] = {
+        { MM_TOP_DDR, MM_TOP_DDR_SIZE },
+        { MM_TOP_DDR_2, MM_TOP_DDR_2_SIZE },
+        { MM_TOP_DDR_3, MM_TOP_DDR_3_SIZE },
+        { MM_TOP_DDR_4, MM_TOP_DDR_4_SIZE }
+    };
+    uint64_t mem_reg_prop[8] = {0};
+    uint64_t size = ram_size;
+    Error *err = NULL;
+    char *name;
+    int i;
+
+    fdt_nop_memory_nodes(fdt, &err);
+    if (err) {
+        error_report_err(err);
+        return;
+    }
+
+    name = g_strdup_printf("/memory@%x", MM_TOP_DDR);
+    for (i = 0; i < ARRAY_SIZE(addr_ranges) && size; i++) {
+        uint64_t mapsize;
+
+        mapsize = size < addr_ranges[i].size ? size : addr_ranges[i].size;
+
+        mem_reg_prop[i * 2] = addr_ranges[i].base;
+        mem_reg_prop[i * 2 + 1] = mapsize;
+        size -= mapsize;
+    }
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "device_type", "memory");
+
+    switch (i) {
+    case 1:
+        qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                     2, mem_reg_prop[0],
+                                     2, mem_reg_prop[1]);
+        break;
+    case 2:
+        qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                     2, mem_reg_prop[0],
+                                     2, mem_reg_prop[1],
+                                     2, mem_reg_prop[2],
+                                     2, mem_reg_prop[3]);
+        break;
+    case 3:
+        qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                     2, mem_reg_prop[0],
+                                     2, mem_reg_prop[1],
+                                     2, mem_reg_prop[2],
+                                     2, mem_reg_prop[3],
+                                     2, mem_reg_prop[4],
+                                     2, mem_reg_prop[5]);
+        break;
+    case 4:
+        qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                     2, mem_reg_prop[0],
+                                     2, mem_reg_prop[1],
+                                     2, mem_reg_prop[2],
+                                     2, mem_reg_prop[3],
+                                     2, mem_reg_prop[4],
+                                     2, mem_reg_prop[5],
+                                     2, mem_reg_prop[6],
+                                     2, mem_reg_prop[7]);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    g_free(name);
+}
+
+static void versal_virt_modify_dtb(const struct arm_boot_info *binfo,
+                                    void *fdt)
+{
+    VersalVirt *s = container_of(binfo, VersalVirt, binfo);
+
+    fdt_add_memory_nodes(s, fdt, binfo->ram_size);
+}
+
+static void *versal_virt_get_dtb(const struct arm_boot_info *binfo,
+                                  int *fdt_size)
+{
+    const VersalVirt *board = container_of(binfo, VersalVirt, binfo);
+
+    *fdt_size = board->fdt_size;
+    return board->fdt;
+}
+
+#define NUM_VIRTIO_TRANSPORT 32
+static void create_virtio_regions(VersalVirt *s)
+{
+    int virtio_mmio_size = 0x200;
+    int i;
+
+    for (i = 0; i < NUM_VIRTIO_TRANSPORT; i++) {
+        char *name = g_strdup_printf("virtio%d", i);;
+        hwaddr base = MM_TOP_RSVD + i * virtio_mmio_size;
+        int irq = VERSAL_RSVD_HIGH_IRQ_FIRST + i;
+        MemoryRegion *mr;
+        DeviceState *dev;
+        qemu_irq pic_irq;
+
+        pic_irq = qdev_get_gpio_in(DEVICE(&s->soc.fpd.apu.gic), irq);
+        dev = qdev_create(NULL, "virtio-mmio");
+        object_property_add_child(OBJECT(&s->soc), name, OBJECT(dev),
+                                  &error_fatal);
+        qdev_init_nofail(dev);
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic_irq);
+        mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+        memory_region_add_subregion(&s->soc.mr_ps, base, mr);
+        sysbus_create_simple("virtio-mmio", base, pic_irq);
+    }
+
+    for (i = 0; i < NUM_VIRTIO_TRANSPORT; i++) {
+        hwaddr base = MM_TOP_RSVD + i * virtio_mmio_size;
+        int irq = VERSAL_RSVD_HIGH_IRQ_FIRST + i;
+        char *name = g_strdup_printf("/virtio_mmio@%" PRIx64, base);
+
+        qemu_fdt_add_subnode(s->fdt, name);
+        qemu_fdt_setprop(s->fdt, name, "dma-coherent", NULL, 0);
+        qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
+                               GIC_FDT_IRQ_TYPE_SPI, irq,
+                               GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+        qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
+                                     2, base, 2, virtio_mmio_size);
+        qemu_fdt_setprop_string(s->fdt, name, "compatible", "virtio,mmio");
+        g_free(name);
+    }
+}
+
+static void versal_virt_init(MachineState *machine)
+{
+    VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine);
+    int psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
+
+    /*
+     * If the user provides an Operating System to be loaded, we expect them
+     * to use the -kernel command line option.
+     *
+     * Users can load firmware or boot-loaders with the -device loader options.
+     *
+     * When loading an OS, we generate a dtb and let arm_load_kernel() select
+     * where it gets loaded. This dtb will be passed to the kernel in x0.
+     *
+     * If there's no -kernel option, we generate a DTB and place it at 0x1000
+     * for the bootloaders or firmware to pick up.
+     *
+     * If users want to provide their own DTB, they can use the -dtb option.
+     * These dtb's will have their memory nodes modified to match QEMU's
+     * selected ram_size option before they get passed to the kernel or fw.
+     *
+     * When loading an OS, we turn on QEMU's PSCI implementation with SMC
+     * as the PSCI conduit. When there's no -kernel, we assume the user
+     * provides EL3 firmware to handle PSCI.
+     */
+    if (machine->kernel_filename) {
+        psci_conduit = QEMU_PSCI_CONDUIT_SMC;
+    }
+
+    memory_region_allocate_system_memory(&s->mr_ddr, NULL, "ddr",
+                                         machine->ram_size);
+
+    sysbus_init_child_obj(OBJECT(machine), "xlnx-ve", &s->soc,
+                          sizeof(s->soc), TYPE_XLNX_VERSAL);
+    object_property_set_link(OBJECT(&s->soc), OBJECT(&s->mr_ddr),
+                             "ddr", &error_abort);
+    object_property_set_int(OBJECT(&s->soc), psci_conduit,
+                            "psci-conduit", &error_abort);
+    object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
+
+    fdt_create(s);
+    create_virtio_regions(s);
+    fdt_add_gem_nodes(s);
+    fdt_add_uart_nodes(s);
+    fdt_add_gic_nodes(s);
+    fdt_add_timer_nodes(s);
+    fdt_add_cpu_nodes(s, psci_conduit);
+    fdt_add_clk_node(s, "/clk125", 125000000, s->phandle.clk_125Mhz);
+    fdt_add_clk_node(s, "/clk25", 25000000, s->phandle.clk_25Mhz);
+
+    /* Make the APU cpu address space visible to virtio and other
+     * modules unaware of muliple address-spaces.  */
+    memory_region_add_subregion_overlap(get_system_memory(),
+                                        0, &s->soc.fpd.apu.mr, 0);
+
+    s->binfo.ram_size = machine->ram_size;
+    s->binfo.kernel_filename = machine->kernel_filename;
+    s->binfo.kernel_cmdline = machine->kernel_cmdline;
+    s->binfo.initrd_filename = machine->initrd_filename;
+    s->binfo.loader_start = 0x0;
+    s->binfo.get_dtb = versal_virt_get_dtb;
+    s->binfo.modify_dtb = versal_virt_modify_dtb;
+    if (machine->kernel_filename) {
+        arm_load_kernel(s->soc.fpd.apu.cpu[0], &s->binfo);
+    } else {
+        AddressSpace *as = arm_boot_address_space(s->soc.fpd.apu.cpu[0],
+                                                  &s->binfo);
+        /* Some boot-loaders (e.g u-boot) don't like blobs at address 0 (NULL).
+         * Offset things by 4K.  */
+        s->binfo.loader_start = 0x1000;
+        s->binfo.dtb_limit = 0x1000000;
+        if (arm_load_dtb(s->binfo.loader_start,
+                         &s->binfo, s->binfo.dtb_limit, as) < 0) {
+            exit(EXIT_FAILURE);
+        }
+    }
+}
+
+static void versal_virt_machine_instance_init(Object *obj)
+{
+}
+
+static void versal_virt_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "Xilinx Versal Virtual development board";
+    mc->init = versal_virt_init;
+    mc->max_cpus = XLNX_VERSAL_NR_ACPUS;
+    mc->default_cpus = XLNX_VERSAL_NR_ACPUS;
+    mc->no_cdrom = true;
+}
+
+static const TypeInfo versal_virt_machine_init_typeinfo = {
+    .name       = TYPE_XLNX_VERSAL_VIRT_MACHINE,
+    .parent     = TYPE_MACHINE,
+    .class_init = versal_virt_machine_class_init,
+    .instance_init = versal_virt_machine_instance_init,
+    .instance_size = sizeof(VersalVirt),
+};
+
+static void versal_virt_machine_init_register_types(void)
+{
+    type_register_static(&versal_virt_machine_init_typeinfo);
+}
+
+type_init(versal_virt_machine_init_register_types)
-- 
2.19.1

[Qemu-devel] [PULL 4/6] hw/char: Implement nRF51 SoC UART

[Peter Maydell]

From: Julia Suvorova via Qemu-devel <qemu-devel@nongnu.org>

Not implemented: CTS/NCTS, PSEL*.

Signed-off-by: Julia Suvorova <jusual@mail.ru>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/char/Makefile.objs        |   1 +
 include/hw/char/nrf51_uart.h |  78 +++++++++
 hw/char/nrf51_uart.c         | 330 +++++++++++++++++++++++++++++++++++
 hw/char/trace-events         |   4 +
 4 files changed, 413 insertions(+)
 create mode 100644 include/hw/char/nrf51_uart.h
 create mode 100644 hw/char/nrf51_uart.c

diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index b5705312910..c4947d7ae7b 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -1,5 +1,6 @@
 common-obj-$(CONFIG_IPACK) += ipoctal232.o
 common-obj-$(CONFIG_ESCC) += escc.o
+common-obj-$(CONFIG_NRF51_SOC) += nrf51_uart.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_PARALLEL) += parallel-isa.o
 common-obj-$(CONFIG_PL011) += pl011.o
diff --git a/include/hw/char/nrf51_uart.h b/include/hw/char/nrf51_uart.h
new file mode 100644
index 00000000000..e3ecb7c81c2
--- /dev/null
+++ b/include/hw/char/nrf51_uart.h
@@ -0,0 +1,78 @@
+/*
+ * nRF51 SoC UART emulation
+ *
+ * Copyright (c) 2018 Julia Suvorova <jusual@mail.ru>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#ifndef NRF51_UART_H
+#define NRF51_UART_H
+
+#include "hw/sysbus.h"
+#include "chardev/char-fe.h"
+#include "hw/registerfields.h"
+
+#define UART_FIFO_LENGTH 6
+#define UART_BASE 0x40002000
+#define UART_SIZE 0x1000
+
+#define TYPE_NRF51_UART "nrf51_soc.uart"
+#define NRF51_UART(obj) OBJECT_CHECK(NRF51UARTState, (obj), TYPE_NRF51_UART)
+
+REG32(UART_STARTRX, 0x000)
+REG32(UART_STOPRX, 0x004)
+REG32(UART_STARTTX, 0x008)
+REG32(UART_STOPTX, 0x00C)
+REG32(UART_SUSPEND, 0x01C)
+
+REG32(UART_CTS, 0x100)
+REG32(UART_NCTS, 0x104)
+REG32(UART_RXDRDY, 0x108)
+REG32(UART_TXDRDY, 0x11C)
+REG32(UART_ERROR, 0x124)
+REG32(UART_RXTO, 0x144)
+
+REG32(UART_INTEN, 0x300)
+    FIELD(UART_INTEN, CTS, 0, 1)
+    FIELD(UART_INTEN, NCTS, 1, 1)
+    FIELD(UART_INTEN, RXDRDY, 2, 1)
+    FIELD(UART_INTEN, TXDRDY, 7, 1)
+    FIELD(UART_INTEN, ERROR, 9, 1)
+    FIELD(UART_INTEN, RXTO, 17, 1)
+REG32(UART_INTENSET, 0x304)
+REG32(UART_INTENCLR, 0x308)
+REG32(UART_ERRORSRC, 0x480)
+REG32(UART_ENABLE, 0x500)
+REG32(UART_PSELRTS, 0x508)
+REG32(UART_PSELTXD, 0x50C)
+REG32(UART_PSELCTS, 0x510)
+REG32(UART_PSELRXD, 0x514)
+REG32(UART_RXD, 0x518)
+REG32(UART_TXD, 0x51C)
+REG32(UART_BAUDRATE, 0x524)
+REG32(UART_CONFIG, 0x56C)
+
+typedef struct NRF51UARTState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    CharBackend chr;
+    qemu_irq irq;
+    guint watch_tag;
+
+    uint8_t rx_fifo[UART_FIFO_LENGTH];
+    unsigned int rx_fifo_pos;
+    unsigned int rx_fifo_len;
+
+    uint32_t reg[0x56C];
+
+    bool rx_started;
+    bool tx_started;
+    bool pending_tx_byte;
+    bool enabled;
+} NRF51UARTState;
+
+#endif
diff --git a/hw/char/nrf51_uart.c b/hw/char/nrf51_uart.c
new file mode 100644
index 00000000000..2f5fae61671
--- /dev/null
+++ b/hw/char/nrf51_uart.c
@@ -0,0 +1,330 @@
+/*
+ * nRF51 SoC UART emulation
+ *
+ * See nRF51 Series Reference Manual, "29 Universal Asynchronous
+ * Receiver/Transmitter" for hardware specifications:
+ * http://infocenter.nordicsemi.com/pdf/nRF51_RM_v3.0.pdf
+ *
+ * Copyright (c) 2018 Julia Suvorova <jusual@mail.ru>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/char/nrf51_uart.h"
+#include "trace.h"
+
+static void nrf51_uart_update_irq(NRF51UARTState *s)
+{
+    bool irq = false;
+
+    irq |= (s->reg[R_UART_RXDRDY] &&
+            (s->reg[R_UART_INTEN] & R_UART_INTEN_RXDRDY_MASK));
+    irq |= (s->reg[R_UART_TXDRDY] &&
+            (s->reg[R_UART_INTEN] & R_UART_INTEN_TXDRDY_MASK));
+    irq |= (s->reg[R_UART_ERROR]  &&
+            (s->reg[R_UART_INTEN] & R_UART_INTEN_ERROR_MASK));
+    irq |= (s->reg[R_UART_RXTO]   &&
+            (s->reg[R_UART_INTEN] & R_UART_INTEN_RXTO_MASK));
+
+    qemu_set_irq(s->irq, irq);
+}
+
+static uint64_t uart_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    NRF51UARTState *s = NRF51_UART(opaque);
+    uint64_t r;
+
+    if (!s->enabled) {
+        return 0;
+    }
+
+    switch (addr) {
+    case A_UART_RXD:
+        r = s->rx_fifo[s->rx_fifo_pos];
+        if (s->rx_started && s->rx_fifo_len) {
+            s->rx_fifo_pos = (s->rx_fifo_pos + 1) % UART_FIFO_LENGTH;
+            s->rx_fifo_len--;
+            if (s->rx_fifo_len) {
+                s->reg[R_UART_RXDRDY] = 1;
+                nrf51_uart_update_irq(s);
+            }
+            qemu_chr_fe_accept_input(&s->chr);
+        }
+        break;
+    case A_UART_INTENSET:
+    case A_UART_INTENCLR:
+    case A_UART_INTEN:
+        r = s->reg[R_UART_INTEN];
+        break;
+    default:
+        r = s->reg[addr / 4];
+        break;
+    }
+
+    trace_nrf51_uart_read(addr, r, size);
+
+    return r;
+}
+
+static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void *opaque)
+{
+    NRF51UARTState *s = NRF51_UART(opaque);
+    int r;
+    uint8_t c = s->reg[R_UART_TXD];
+
+    s->watch_tag = 0;
+
+    r = qemu_chr_fe_write(&s->chr, &c, 1);
+    if (r <= 0) {
+        s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
+                                             uart_transmit, s);
+        if (!s->watch_tag) {
+            /* The hardware has no transmit error reporting,
+             * so silently drop the byte
+             */
+            goto buffer_drained;
+        }
+        return FALSE;
+    }
+
+buffer_drained:
+    s->reg[R_UART_TXDRDY] = 1;
+    s->pending_tx_byte = false;
+    return FALSE;
+}
+
+static void uart_cancel_transmit(NRF51UARTState *s)
+{
+    if (s->watch_tag) {
+        g_source_remove(s->watch_tag);
+        s->watch_tag = 0;
+    }
+}
+
+static void uart_write(void *opaque, hwaddr addr,
+                       uint64_t value, unsigned int size)
+{
+    NRF51UARTState *s = NRF51_UART(opaque);
+
+    trace_nrf51_uart_write(addr, value, size);
+
+    if (!s->enabled && (addr != A_UART_ENABLE)) {
+        return;
+    }
+
+    switch (addr) {
+    case A_UART_TXD:
+        if (!s->pending_tx_byte && s->tx_started) {
+            s->reg[R_UART_TXD] = value;
+            s->pending_tx_byte = true;
+            uart_transmit(NULL, G_IO_OUT, s);
+        }
+        break;
+    case A_UART_INTEN:
+        s->reg[R_UART_INTEN] = value;
+        break;
+    case A_UART_INTENSET:
+        s->reg[R_UART_INTEN] |= value;
+        break;
+    case A_UART_INTENCLR:
+        s->reg[R_UART_INTEN] &= ~value;
+        break;
+    case A_UART_TXDRDY ... A_UART_RXTO:
+        s->reg[addr / 4] = value;
+        break;
+    case A_UART_ERRORSRC:
+        s->reg[addr / 4] &= ~value;
+        break;
+    case A_UART_RXD:
+        break;
+    case A_UART_RXDRDY:
+        if (value == 0) {
+            s->reg[R_UART_RXDRDY] = 0;
+        }
+        break;
+    case A_UART_STARTTX:
+        if (value == 1) {
+            s->tx_started = true;
+        }
+        break;
+    case A_UART_STARTRX:
+        if (value == 1) {
+            s->rx_started = true;
+        }
+        break;
+    case A_UART_ENABLE:
+        if (value) {
+            if (value == 4) {
+                s->enabled = true;
+            }
+            break;
+        }
+        s->enabled = false;
+        value = 1;
+        /* fall through */
+    case A_UART_SUSPEND:
+    case A_UART_STOPTX:
+        if (value == 1) {
+            s->tx_started = false;
+        }
+        /* fall through */
+    case A_UART_STOPRX:
+        if (addr != A_UART_STOPTX && value == 1) {
+            s->rx_started = false;
+            s->reg[R_UART_RXTO] = 1;
+        }
+        break;
+    default:
+        s->reg[addr / 4] = value;
+        break;
+    }
+    nrf51_uart_update_irq(s);
+}
+
+static const MemoryRegionOps uart_ops = {
+    .read =  uart_read,
+    .write = uart_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void nrf51_uart_reset(DeviceState *dev)
+{
+    NRF51UARTState *s = NRF51_UART(dev);
+
+    s->pending_tx_byte = 0;
+
+    uart_cancel_transmit(s);
+
+    memset(s->reg, 0, sizeof(s->reg));
+
+    s->reg[R_UART_PSELRTS] = 0xFFFFFFFF;
+    s->reg[R_UART_PSELTXD] = 0xFFFFFFFF;
+    s->reg[R_UART_PSELCTS] = 0xFFFFFFFF;
+    s->reg[R_UART_PSELRXD] = 0xFFFFFFFF;
+    s->reg[R_UART_BAUDRATE] = 0x4000000;
+
+    s->rx_fifo_len = 0;
+    s->rx_fifo_pos = 0;
+    s->rx_started = false;
+    s->tx_started = false;
+    s->enabled = false;
+}
+
+static void uart_receive(void *opaque, const uint8_t *buf, int size)
+{
+
+    NRF51UARTState *s = NRF51_UART(opaque);
+    int i;
+
+    if (size == 0 || s->rx_fifo_len >= UART_FIFO_LENGTH) {
+        return;
+    }
+
+    for (i = 0; i < size; i++) {
+        uint32_t pos = (s->rx_fifo_pos + s->rx_fifo_len) % UART_FIFO_LENGTH;
+        s->rx_fifo[pos] = buf[i];
+        s->rx_fifo_len++;
+    }
+
+    s->reg[R_UART_RXDRDY] = 1;
+    nrf51_uart_update_irq(s);
+}
+
+static int uart_can_receive(void *opaque)
+{
+    NRF51UARTState *s = NRF51_UART(opaque);
+
+    return s->rx_started ? (UART_FIFO_LENGTH - s->rx_fifo_len) : 0;
+}
+
+static void uart_event(void *opaque, int event)
+{
+    NRF51UARTState *s = NRF51_UART(opaque);
+
+    if (event == CHR_EVENT_BREAK) {
+        s->reg[R_UART_ERRORSRC] |= 3;
+        s->reg[R_UART_ERROR] = 1;
+        nrf51_uart_update_irq(s);
+    }
+}
+
+static void nrf51_uart_realize(DeviceState *dev, Error **errp)
+{
+    NRF51UARTState *s = NRF51_UART(dev);
+
+    qemu_chr_fe_set_handlers(&s->chr, uart_can_receive, uart_receive,
+                             uart_event, NULL, s, NULL, true);
+}
+
+static void nrf51_uart_init(Object *obj)
+{
+    NRF51UARTState *s = NRF51_UART(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+    memory_region_init_io(&s->iomem, obj, &uart_ops, s,
+                          "nrf51_soc.uart", UART_SIZE);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);
+}
+
+static int nrf51_uart_post_load(void *opaque, int version_id)
+{
+    NRF51UARTState *s = NRF51_UART(opaque);
+
+    if (s->pending_tx_byte) {
+        s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
+                                             uart_transmit, s);
+    }
+
+    return 0;
+}
+
+static const VMStateDescription nrf51_uart_vmstate = {
+    .name = "nrf51_soc.uart",
+    .post_load = nrf51_uart_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(reg, NRF51UARTState, 0x56C),
+        VMSTATE_UINT8_ARRAY(rx_fifo, NRF51UARTState, UART_FIFO_LENGTH),
+        VMSTATE_UINT32(rx_fifo_pos, NRF51UARTState),
+        VMSTATE_UINT32(rx_fifo_len, NRF51UARTState),
+        VMSTATE_BOOL(rx_started, NRF51UARTState),
+        VMSTATE_BOOL(tx_started, NRF51UARTState),
+        VMSTATE_BOOL(pending_tx_byte, NRF51UARTState),
+        VMSTATE_BOOL(enabled, NRF51UARTState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property nrf51_uart_properties[] = {
+    DEFINE_PROP_CHR("chardev", NRF51UARTState, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void nrf51_uart_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = nrf51_uart_reset;
+    dc->realize = nrf51_uart_realize;
+    dc->props = nrf51_uart_properties;
+    dc->vmsd = &nrf51_uart_vmstate;
+}
+
+static const TypeInfo nrf51_uart_info = {
+    .name = TYPE_NRF51_UART,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(NRF51UARTState),
+    .instance_init = nrf51_uart_init,
+    .class_init = nrf51_uart_class_init
+};
+
+static void nrf51_uart_register_types(void)
+{
+    type_register_static(&nrf51_uart_info);
+}
+
+type_init(nrf51_uart_register_types)
diff --git a/hw/char/trace-events b/hw/char/trace-events
index b64213d4dd1..de34a74399b 100644
--- a/hw/char/trace-events
+++ b/hw/char/trace-events
@@ -73,3 +73,7 @@ cmsdk_apb_uart_receive(uint8_t c) "CMSDK APB UART: got character 0x%x from backe
 cmsdk_apb_uart_tx_pending(void) "CMSDK APB UART: character send to backend pending"
 cmsdk_apb_uart_tx(uint8_t c) "CMSDK APB UART: character 0x%x sent to backend"
 cmsdk_apb_uart_set_params(int speed) "CMSDK APB UART: params set to %d 8N1"
+
+# hw/char/nrf51_uart.c
+nrf51_uart_read(uint64_t addr, uint64_t r, unsigned int size) "addr 0x%" PRIx64 " value 0x%" PRIx64 " size %u"
+nrf51_uart_write(uint64_t addr, uint64_t value, unsigned int size) "addr 0x%" PRIx64 " value 0x%" PRIx64 " size %u"
-- 
2.19.1

[Qemu-devel] [PULL 5/6] hw/arm/nrf51_soc: Connect UART to nRF51 SoC

[Peter Maydell]

From: Julia Suvorova via Qemu-devel <qemu-devel@nongnu.org>

Wire up nRF51 UART in the corresponding SoC.

Signed-off-by: Julia Suvorova <jusual@mail.ru>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/nrf51_soc.h |  3 +++
 hw/arm/microbit.c          |  2 ++
 hw/arm/nrf51_soc.c         | 20 ++++++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/include/hw/arm/nrf51_soc.h b/include/hw/arm/nrf51_soc.h
index f4e092b554e..73fc92e9a8d 100644
--- a/include/hw/arm/nrf51_soc.h
+++ b/include/hw/arm/nrf51_soc.h
@@ -12,6 +12,7 @@
 
 #include "hw/sysbus.h"
 #include "hw/arm/armv7m.h"
+#include "hw/char/nrf51_uart.h"
 
 #define TYPE_NRF51_SOC "nrf51-soc"
 #define NRF51_SOC(obj) \
@@ -24,6 +25,8 @@ typedef struct NRF51State {
     /*< public >*/
     ARMv7MState cpu;
 
+    NRF51UARTState uart;
+
     MemoryRegion iomem;
     MemoryRegion sram;
     MemoryRegion flash;
diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
index e7d74116a50..a734e7f650e 100644
--- a/hw/arm/microbit.c
+++ b/hw/arm/microbit.c
@@ -12,6 +12,7 @@
 #include "qapi/error.h"
 #include "hw/boards.h"
 #include "hw/arm/arm.h"
+#include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 
 #include "hw/arm/nrf51_soc.h"
@@ -35,6 +36,7 @@ static void microbit_init(MachineState *machine)
 
     sysbus_init_child_obj(OBJECT(machine), "nrf51", soc, sizeof(s->nrf51),
                           TYPE_NRF51_SOC);
+    qdev_prop_set_chr(DEVICE(&s->nrf51), "serial0", serial_hd(0));
     object_property_set_link(soc, OBJECT(system_memory), "memory",
                              &error_fatal);
     object_property_set_bool(soc, true, "realized", &error_fatal);
diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
index 1a59ef45525..b89c1bdea08 100644
--- a/hw/arm/nrf51_soc.c
+++ b/hw/arm/nrf51_soc.c
@@ -43,9 +43,12 @@
 #define NRF51822_FLASH_SIZE     (256 * 1024)
 #define NRF51822_SRAM_SIZE      (16 * 1024)
 
+#define BASE_TO_IRQ(base) ((base >> 12) & 0x1F)
+
 static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
 {
     NRF51State *s = NRF51_SOC(dev_soc);
+    MemoryRegion *mr;
     Error *err = NULL;
 
     if (!s->board_memory) {
@@ -82,6 +85,18 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
     }
     memory_region_add_subregion(&s->container, SRAM_BASE, &s->sram);
 
+    /* UART */
+    object_property_set_bool(OBJECT(&s->uart), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->uart), 0);
+    memory_region_add_subregion_overlap(&s->container, UART_BASE, mr, 0);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart), 0,
+                       qdev_get_gpio_in(DEVICE(&s->cpu),
+                       BASE_TO_IRQ(UART_BASE)));
+
     create_unimplemented_device("nrf51_soc.io", IOMEM_BASE, IOMEM_SIZE);
     create_unimplemented_device("nrf51_soc.ficr", FICR_BASE, FICR_SIZE);
     create_unimplemented_device("nrf51_soc.private",
@@ -99,6 +114,11 @@ static void nrf51_soc_init(Object *obj)
     qdev_prop_set_string(DEVICE(&s->cpu), "cpu-type",
                          ARM_CPU_TYPE_NAME("cortex-m0"));
     qdev_prop_set_uint32(DEVICE(&s->cpu), "num-irq", 32);
+
+    sysbus_init_child_obj(obj, "uart", &s->uart, sizeof(s->uart),
+                           TYPE_NRF51_UART);
+    object_property_add_alias(obj, "serial0", OBJECT(&s->uart), "chardev",
+                              &error_abort);
 }
 
 static Property nrf51_soc_properties[] = {
-- 
2.19.1

[Qemu-devel] [PULL 6/6] tests/boot-serial-test: Add microbit board testcase

[Peter Maydell]

From: Julia Suvorova via Qemu-devel <qemu-devel@nongnu.org>

New mini-kernel test for nRF51 SoC UART.

Signed-off-by: Julia Suvorova <jusual@mail.ru>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/boot-serial-test.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index f865822e32f..8ec6aed35d2 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -62,6 +62,24 @@ static const uint8_t kernel_aarch64[] = {
     0xfd, 0xff, 0xff, 0x17,                 /* b       -12 (loop) */
 };
 
+static const uint8_t kernel_nrf51[] = {
+    0x00, 0x00, 0x00, 0x00,                 /* Stack top address */
+    0x09, 0x00, 0x00, 0x00,                 /* Reset handler address */
+    0x04, 0x4a,                             /* ldr  r2, [pc, #16] Get ENABLE */
+    0x04, 0x21,                             /* movs r1, #4 */
+    0x11, 0x60,                             /* str  r1, [r2] */
+    0x04, 0x4a,                             /* ldr  r2, [pc, #16] Get STARTTX */
+    0x01, 0x21,                             /* movs r1, #1 */
+    0x11, 0x60,                             /* str  r1, [r2] */
+    0x03, 0x4a,                             /* ldr  r2, [pc, #12] Get TXD */
+    0x54, 0x21,                             /* movs r1, 'T' */
+    0x11, 0x60,                             /* str  r1, [r2] */
+    0xfe, 0xe7,                             /* b    . */
+    0x00, 0x25, 0x00, 0x40,                 /* 0x40002500 = UART ENABLE */
+    0x08, 0x20, 0x00, 0x40,                 /* 0x40002008 = UART STARTTX */
+    0x1c, 0x25, 0x00, 0x40                  /* 0x4000251c = UART TXD */
+};
+
 typedef struct testdef {
     const char *arch;       /* Target architecture */
     const char *machine;    /* Name of the machine */
@@ -105,6 +123,7 @@ static testdef_t tests[] = {
     { "hppa", "hppa", "", "SeaBIOS wants SYSTEM HALT" },
     { "aarch64", "virt", "-cpu cortex-a57", "TT", sizeof(kernel_aarch64),
       kernel_aarch64 },
+    { "arm", "microbit", "", "T", sizeof(kernel_nrf51), kernel_nrf51 },
 
     { NULL }
 };
-- 
2.19.1