* [Qemu-devel] [PULL 0/6] target-arm queue
@ 2013-10-25 18:07 Peter Maydell
2013-10-31 14:02 ` Edgar E. Iglesias
0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2013-10-25 18:07 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-25 18:07 Peter Maydell
@ 2013-10-31 14:02 ` Edgar E. Iglesias
2013-10-31 14:18 ` Andreas Färber
0 siblings, 1 reply; 36+ messages in thread
From: Edgar E. Iglesias @ 2013-10-31 14:02 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, Anthony Liguori
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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:02 ` Edgar E. Iglesias
@ 2013-10-31 14:18 ` Andreas Färber
2013-10-31 14:21 ` Anthony Liguori
` (2 more replies)
0 siblings, 3 replies; 36+ messages in thread
From: Andreas Färber @ 2013-10-31 14:18 UTC (permalink / raw)
To: Edgar E. Iglesias, Peter Maydell; +Cc: qemu-devel, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:18 ` Andreas Färber
@ 2013-10-31 14:21 ` Anthony Liguori
2013-10-31 14:31 ` Peter Maydell
2013-10-31 22:13 ` Edgar E. Iglesias
2 siblings, 0 replies; 36+ messages in thread
From: Anthony Liguori @ 2013-10-31 14:21 UTC (permalink / raw)
To: Andreas Färber
Cc: Edgar E. Iglesias, qemu-devel, Anthony Liguori, Peter Maydell
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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:18 ` Andreas Färber
2013-10-31 14:21 ` Anthony Liguori
@ 2013-10-31 14:31 ` Peter Maydell
2013-10-31 14:36 ` Andreas Färber
2013-10-31 22:13 ` Edgar E. Iglesias
2 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2013-10-31 14:31 UTC (permalink / raw)
To: Andreas Färber; +Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:31 ` Peter Maydell
@ 2013-10-31 14:36 ` Andreas Färber
2013-10-31 14:39 ` Anthony Liguori
2013-10-31 15:16 ` Peter Maydell
0 siblings, 2 replies; 36+ messages in thread
From: Andreas Färber @ 2013-10-31 14:36 UTC (permalink / raw)
To: Peter Maydell; +Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:36 ` Andreas Färber
@ 2013-10-31 14:39 ` Anthony Liguori
2013-10-31 14:45 ` Andreas Färber
2013-10-31 15:16 ` Peter Maydell
1 sibling, 1 reply; 36+ messages in thread
From: Anthony Liguori @ 2013-10-31 14:39 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:39 ` Anthony Liguori
@ 2013-10-31 14:45 ` Andreas Färber
2013-10-31 14:54 ` Anthony Liguori
2013-10-31 15:04 ` Anthony Liguori
0 siblings, 2 replies; 36+ messages in thread
From: Andreas Färber @ 2013-10-31 14:45 UTC (permalink / raw)
To: Anthony Liguori
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:45 ` Andreas Färber
@ 2013-10-31 14:54 ` Anthony Liguori
2013-10-31 15:04 ` Anthony Liguori
1 sibling, 0 replies; 36+ messages in thread
From: Anthony Liguori @ 2013-10-31 14:54 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:45 ` Andreas Färber
2013-10-31 14:54 ` Anthony Liguori
@ 2013-10-31 15:04 ` Anthony Liguori
2013-10-31 16:52 ` Andreas Färber
1 sibling, 1 reply; 36+ messages in thread
From: Anthony Liguori @ 2013-10-31 15:04 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:36 ` Andreas Färber
2013-10-31 14:39 ` Anthony Liguori
@ 2013-10-31 15:16 ` Peter Maydell
2013-10-31 17:14 ` Andreas Färber
1 sibling, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2013-10-31 15:16 UTC (permalink / raw)
To: Andreas Färber; +Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 15:04 ` Anthony Liguori
@ 2013-10-31 16:52 ` Andreas Färber
2013-10-31 16:54 ` Anthony Liguori
` (2 more replies)
0 siblings, 3 replies; 36+ messages in thread
From: Andreas Färber @ 2013-10-31 16:52 UTC (permalink / raw)
To: Anthony Liguori
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 16:52 ` Andreas Färber
@ 2013-10-31 16:54 ` Anthony Liguori
2013-10-31 17:10 ` Andreas Färber
2013-10-31 17:02 ` Peter Maydell
2013-10-31 18:55 ` Anthony Liguori
2 siblings, 1 reply; 36+ messages in thread
From: Anthony Liguori @ 2013-10-31 16:54 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 16:52 ` Andreas Färber
2013-10-31 16:54 ` Anthony Liguori
@ 2013-10-31 17:02 ` Peter Maydell
2013-10-31 17:15 ` Peter Maydell
2013-10-31 18:55 ` Anthony Liguori
2 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2013-10-31 17:02 UTC (permalink / raw)
To: Andreas Färber
Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori,
Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 16:54 ` Anthony Liguori
@ 2013-10-31 17:10 ` Andreas Färber
0 siblings, 0 replies; 36+ messages in thread
From: Andreas Färber @ 2013-10-31 17:10 UTC (permalink / raw)
To: Anthony Liguori
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 15:16 ` Peter Maydell
@ 2013-10-31 17:14 ` Andreas Färber
2013-10-31 17:18 ` Peter Maydell
2013-10-31 18:58 ` Anthony Liguori
0 siblings, 2 replies; 36+ messages in thread
From: Andreas Färber @ 2013-10-31 17:14 UTC (permalink / raw)
To: Peter Maydell; +Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 17:02 ` Peter Maydell
@ 2013-10-31 17:15 ` Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2013-10-31 17:15 UTC (permalink / raw)
To: Andreas Färber
Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori,
Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 17:14 ` Andreas Färber
@ 2013-10-31 17:18 ` Peter Maydell
2013-10-31 17:27 ` Andreas Färber
2013-10-31 18:58 ` Anthony Liguori
1 sibling, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2013-10-31 17:18 UTC (permalink / raw)
To: Andreas Färber; +Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 17:18 ` Peter Maydell
@ 2013-10-31 17:27 ` Andreas Färber
2013-10-31 17:51 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Andreas Färber @ 2013-10-31 17:27 UTC (permalink / raw)
To: Peter Maydell; +Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 17:27 ` Andreas Färber
@ 2013-10-31 17:51 ` Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2013-10-31 17:51 UTC (permalink / raw)
To: Andreas Färber; +Cc: Edgar E. Iglesias, QEMU Developers, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 16:52 ` Andreas Färber
2013-10-31 16:54 ` Anthony Liguori
2013-10-31 17:02 ` Peter Maydell
@ 2013-10-31 18:55 ` Anthony Liguori
2 siblings, 0 replies; 36+ messages in thread
From: Anthony Liguori @ 2013-10-31 18:55 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 17:14 ` Andreas Färber
2013-10-31 17:18 ` Peter Maydell
@ 2013-10-31 18:58 ` Anthony Liguori
1 sibling, 0 replies; 36+ messages in thread
From: Anthony Liguori @ 2013-10-31 18:58 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, QEMU Developers, Anthony Liguori,
Edgar E. Iglesias
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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2013-10-31 14:18 ` Andreas Färber
2013-10-31 14:21 ` Anthony Liguori
2013-10-31 14:31 ` Peter Maydell
@ 2013-10-31 22:13 ` Edgar E. Iglesias
2 siblings, 0 replies; 36+ messages in thread
From: Edgar E. Iglesias @ 2013-10-31 22:13 UTC (permalink / raw)
To: Andreas Färber; +Cc: Peter Maydell, qemu-devel, Anthony Liguori
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 0/6] target-arm queue
@ 2014-03-19 12:05 Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 1/6] ahci: fix sysbus support Peter Maydell
` (6 more replies)
0 siblings, 7 replies; 36+ messages in thread
From: Peter Maydell @ 2014-03-19 12:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Blue Swirl, Andreas Färber, qemu-devel, Aurelien Jarno
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(-)
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 1/6] ahci: fix sysbus support
2014-03-19 12:05 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
@ 2014-03-19 12:05 ` Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 2/6] pl011: reset the fifo when enabled or disabled Peter Maydell
` (5 subsequent siblings)
6 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2014-03-19 12:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Blue Swirl, Andreas Färber, qemu-devel, Aurelien Jarno
From: Rob Herring <rob.herring@linaro.org>
Non-PCI AHCI support is broken due to assertion failures when trying
to convert AHCIState to a PCIDevice pointer as AHCIState can have
different container structs. Fix this by using the non-asserting object
cast and checking the returned pointer is not NULL.
The AddressSpace pointer is also being initialized to NULL and causing
dma_memory_map call to fail. Fix this by initializing to
address_space_memory for sysbus instances.
Also correct AHCI_VMSTATE to use the correct container SysbusAHCIState
for sysbus instances.
Signed-off-by: Rob Herring <rob.herring@linaro.org>
Message-id: 1392073373-3295-1-git-send-email-robherring2@gmail.com
[PMM: added linebreaks to fix overlong lines]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/ide/ahci.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index fbea9e8..bfe633f 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -118,11 +118,12 @@ static uint32_t ahci_port_read(AHCIState *s, int port, int offset)
static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
{
AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
- PCIDevice *pci_dev = PCI_DEVICE(d);
+ PCIDevice *pci_dev =
+ (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
DPRINTF(0, "raise irq\n");
- if (msi_enabled(pci_dev)) {
+ if (pci_dev && msi_enabled(pci_dev)) {
msi_notify(pci_dev, 0);
} else {
qemu_irq_raise(s->irq);
@@ -132,10 +133,12 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
{
AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
+ PCIDevice *pci_dev =
+ (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
DPRINTF(0, "lower irq\n");
- if (!msi_enabled(PCI_DEVICE(d))) {
+ if (!pci_dev || !msi_enabled(pci_dev)) {
qemu_irq_lower(s->irq);
}
}
@@ -1311,7 +1314,7 @@ static const VMStateDescription vmstate_sysbus_ahci = {
.name = "sysbus-ahci",
.unmigratable = 1, /* Still buggy under I/O load */
.fields = (VMStateField []) {
- VMSTATE_AHCI(ahci, AHCIPCIState),
+ VMSTATE_AHCI(ahci, SysbusAHCIState),
VMSTATE_END_OF_LIST()
},
};
@@ -1328,7 +1331,7 @@ static void sysbus_ahci_realize(DeviceState *dev, Error **errp)
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
SysbusAHCIState *s = SYSBUS_AHCI(dev);
- ahci_init(&s->ahci, dev, NULL, s->num_ports);
+ ahci_init(&s->ahci, dev, &address_space_memory, s->num_ports);
sysbus_init_mmio(sbd, &s->ahci.mem);
sysbus_init_irq(sbd, &s->ahci.irq);
--
1.9.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 2/6] pl011: reset the fifo when enabled or disabled
2014-03-19 12:05 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 1/6] ahci: fix sysbus support Peter Maydell
@ 2014-03-19 12:05 ` Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 3/6] pl011: fix UARTRSR accesses corrupting the UARTCR value Peter Maydell
` (4 subsequent siblings)
6 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2014-03-19 12:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Blue Swirl, Andreas Färber, qemu-devel, Aurelien Jarno
From: Rob Herring <rob.herring@linaro.org>
Intermittent issues have been seen where no serial input occurs. It
appears the pl011 gets in a state where the rx interrupt never fires
because the rx interrupt only asserts when crossing the fifo trigger
level. The fifo state appears to get out of sync when the pl011 is
re-configured. This combined with the rx timeout interrupt not being
modeled results in no more rx interrupts.
Disabling the fifo is the recommended way to clear the tx fifo in the
TRM (section 3.3.8). The behavior in this case for the rx fifo is
undefined in the TRM, but having fifo contents to be maintained during
configuration changes is not likely expected behavior. Reseting the
fifo state when the fifo size is changed is the simplest solution.
Signed-off-by: Rob Herring <rob.herring@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1395166721-15716-2-git-send-email-robherring2@gmail.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/char/pl011.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index a8ae6f4..8103e2e 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -162,6 +162,11 @@ static void pl011_write(void *opaque, hwaddr offset,
s->fbrd = value;
break;
case 11: /* UARTLCR_H */
+ /* Reset the FIFO state on FIFO enable or disable */
+ if ((s->lcr ^ value) & 0x10) {
+ s->read_count = 0;
+ s->read_pos = 0;
+ }
s->lcr = value;
pl011_set_read_trigger(s);
break;
--
1.9.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 3/6] pl011: fix UARTRSR accesses corrupting the UARTCR value
2014-03-19 12:05 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 1/6] ahci: fix sysbus support Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 2/6] pl011: reset the fifo when enabled or disabled Peter Maydell
@ 2014-03-19 12:05 ` Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 4/6] pl011: fix incorrect logic to set the RXFF flag Peter Maydell
` (3 subsequent siblings)
6 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2014-03-19 12:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Blue Swirl, Andreas Färber, qemu-devel, Aurelien Jarno
From: Rob Herring <rob.herring@linaro.org>
Offset 4 is UARTRSR/UARTECR, not the UARTCR. The UARTCR would be
corrupted if the UARTRSR is ever written. Fix by implementing a correct
model of the UARTRSR/UARTECR register. Reads of this register simply
reflect the error bits in data register. Only breaks can be triggered in
QEMU. With the pl011_can_receive function, we effectively have flow
control between the host and the model. Framing and parity errors simply
don't make sense in the model and will never occur.
Signed-off-by: Rob Herring <rob.herring@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1395166721-15716-3-git-send-email-robherring2@gmail.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/char/pl011.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 8103e2e..11c3a75 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -20,6 +20,7 @@ typedef struct PL011State {
uint32_t readbuff;
uint32_t flags;
uint32_t lcr;
+ uint32_t rsr;
uint32_t cr;
uint32_t dmacr;
uint32_t int_enabled;
@@ -81,13 +82,14 @@ static uint64_t pl011_read(void *opaque, hwaddr offset,
}
if (s->read_count == s->read_trigger - 1)
s->int_level &= ~ PL011_INT_RX;
+ s->rsr = c >> 8;
pl011_update(s);
if (s->chr) {
qemu_chr_accept_input(s->chr);
}
return c;
- case 1: /* UARTCR */
- return 0;
+ case 1: /* UARTRSR */
+ return s->rsr;
case 6: /* UARTFR */
return s->flags;
case 8: /* UARTILPR */
@@ -146,8 +148,8 @@ static void pl011_write(void *opaque, hwaddr offset,
s->int_level |= PL011_INT_TX;
pl011_update(s);
break;
- case 1: /* UARTCR */
- s->cr = value;
+ case 1: /* UARTRSR/UARTECR */
+ s->rsr = 0;
break;
case 6: /* UARTFR */
/* Writes to Flag register are ignored. */
@@ -247,13 +249,14 @@ static const MemoryRegionOps pl011_ops = {
static const VMStateDescription vmstate_pl011 = {
.name = "pl011",
- .version_id = 1,
- .minimum_version_id = 1,
- .minimum_version_id_old = 1,
+ .version_id = 2,
+ .minimum_version_id = 2,
+ .minimum_version_id_old = 2,
.fields = (VMStateField[]) {
VMSTATE_UINT32(readbuff, PL011State),
VMSTATE_UINT32(flags, PL011State),
VMSTATE_UINT32(lcr, PL011State),
+ VMSTATE_UINT32(rsr, PL011State),
VMSTATE_UINT32(cr, PL011State),
VMSTATE_UINT32(dmacr, PL011State),
VMSTATE_UINT32(int_enabled, PL011State),
--
1.9.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 4/6] pl011: fix incorrect logic to set the RXFF flag
2014-03-19 12:05 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
` (2 preceding siblings ...)
2014-03-19 12:05 ` [Qemu-devel] [PULL 3/6] pl011: fix UARTRSR accesses corrupting the UARTCR value Peter Maydell
@ 2014-03-19 12:05 ` Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 5/6] target-arm: A64: Add saturating int ops (SQNEG/SQABS) Peter Maydell
` (2 subsequent siblings)
6 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2014-03-19 12:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Blue Swirl, Andreas Färber, qemu-devel, Aurelien Jarno
From: Rob Herring <rob.herring@linaro.org>
The receive fifo full bit should be set when 1 character is received and
the fifo is disabled or when 16 characters are in the fifo.
Signed-off-by: Rob Herring <rob.herring@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1395166721-15716-4-git-send-email-robherring2@gmail.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/char/pl011.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 11c3a75..644aad7 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -221,7 +221,7 @@ static void pl011_put_fifo(void *opaque, uint32_t value)
s->read_fifo[slot] = value;
s->read_count++;
s->flags &= ~PL011_FLAG_RXFE;
- if (s->cr & 0x10 || s->read_count == 16) {
+ if (!(s->lcr & 0x10) || s->read_count == 16) {
s->flags |= PL011_FLAG_RXFF;
}
if (s->read_count == s->read_trigger) {
--
1.9.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 5/6] target-arm: A64: Add saturating int ops (SQNEG/SQABS)
2014-03-19 12:05 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
` (3 preceding siblings ...)
2014-03-19 12:05 ` [Qemu-devel] [PULL 4/6] pl011: fix incorrect logic to set the RXFF flag Peter Maydell
@ 2014-03-19 12:05 ` Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 6/6] target-arm: A64: Add saturating accumulate ops (USQADD/SUQADD) Peter Maydell
2014-03-19 13:33 ` [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
6 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2014-03-19 12:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Blue Swirl, Andreas Färber, qemu-devel, Aurelien Jarno
From: Alex Bennée <alex.bennee@linaro.org>
This mostly re-uses the existing NEON helpers with an additional two for
the 64 bit case. I also took the opportunity to add TCG_CALL_NO_RWG
options to the helpers as they don't modify globals (saturation flags
are in the CPU Environment).
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
target-arm/helper.h | 14 +++++++------
target-arm/neon_helper.c | 22 ++++++++++++++++++++
target-arm/translate-a64.c | 51 ++++++++++++++++++++++++++++++++++++++++------
3 files changed, 75 insertions(+), 12 deletions(-)
diff --git a/target-arm/helper.h b/target-arm/helper.h
index a3d6f32..b006fd5 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -375,12 +375,14 @@ DEF_HELPER_2(neon_mull_s16, i64, i32, i32)
DEF_HELPER_1(neon_negl_u16, i64, i64)
DEF_HELPER_1(neon_negl_u32, i64, i64)
-DEF_HELPER_2(neon_qabs_s8, i32, env, i32)
-DEF_HELPER_2(neon_qabs_s16, i32, env, i32)
-DEF_HELPER_2(neon_qabs_s32, i32, env, i32)
-DEF_HELPER_2(neon_qneg_s8, i32, env, i32)
-DEF_HELPER_2(neon_qneg_s16, i32, env, i32)
-DEF_HELPER_2(neon_qneg_s32, i32, env, i32)
+DEF_HELPER_FLAGS_2(neon_qabs_s8, TCG_CALL_NO_RWG, i32, env, i32)
+DEF_HELPER_FLAGS_2(neon_qabs_s16, TCG_CALL_NO_RWG, i32, env, i32)
+DEF_HELPER_FLAGS_2(neon_qabs_s32, TCG_CALL_NO_RWG, i32, env, i32)
+DEF_HELPER_FLAGS_2(neon_qabs_s64, TCG_CALL_NO_RWG, i64, env, i64)
+DEF_HELPER_FLAGS_2(neon_qneg_s8, TCG_CALL_NO_RWG, i32, env, i32)
+DEF_HELPER_FLAGS_2(neon_qneg_s16, TCG_CALL_NO_RWG, i32, env, i32)
+DEF_HELPER_FLAGS_2(neon_qneg_s32, TCG_CALL_NO_RWG, i32, env, i32)
+DEF_HELPER_FLAGS_2(neon_qneg_s64, TCG_CALL_NO_RWG, i64, env, i64)
DEF_HELPER_3(neon_abd_f32, i32, i32, i32, ptr)
DEF_HELPER_3(neon_ceq_f32, i32, i32, i32, ptr)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 13752ba..e23f224 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -1776,6 +1776,28 @@ uint32_t HELPER(neon_qneg_s32)(CPUARMState *env, uint32_t x)
return x;
}
+uint64_t HELPER(neon_qabs_s64)(CPUARMState *env, uint64_t x)
+{
+ if (x == SIGNBIT64) {
+ SET_QC();
+ x = ~SIGNBIT64;
+ } else if ((int64_t)x < 0) {
+ x = -x;
+ }
+ return x;
+}
+
+uint64_t HELPER(neon_qneg_s64)(CPUARMState *env, uint64_t x)
+{
+ if (x == SIGNBIT64) {
+ SET_QC();
+ x = ~SIGNBIT64;
+ } else {
+ x = -x;
+ }
+ return x;
+}
+
/* NEON Float helpers. */
uint32_t HELPER(neon_abd_f32)(uint32_t a, uint32_t b, void *fpstp)
{
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index befffac..18659d7 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -73,6 +73,7 @@ typedef struct AArch64DecodeTable {
} AArch64DecodeTable;
/* Function prototype for gen_ functions for calling Neon helpers */
+typedef void NeonGenOneOpEnvFn(TCGv_i32, TCGv_ptr, TCGv_i32);
typedef void NeonGenTwoOpFn(TCGv_i32, TCGv_i32, TCGv_i32);
typedef void NeonGenTwoOpEnvFn(TCGv_i32, TCGv_ptr, TCGv_i32, TCGv_i32);
typedef void NeonGenTwo64OpFn(TCGv_i64, TCGv_i64, TCGv_i64);
@@ -6942,6 +6943,13 @@ static void handle_2misc_64(DisasContext *s, int opcode, bool u,
*/
tcg_gen_not_i64(tcg_rd, tcg_rn);
break;
+ case 0x7: /* SQABS, SQNEG */
+ if (u) {
+ gen_helper_neon_qneg_s64(tcg_rd, cpu_env, tcg_rn);
+ } else {
+ gen_helper_neon_qabs_s64(tcg_rd, cpu_env, tcg_rn);
+ }
+ break;
case 0xa: /* CMLT */
/* 64 bit integer comparison against zero, result is
* test ? (2^64 - 1) : 0. We implement via setcond(!test) and
@@ -7332,6 +7340,8 @@ static void disas_simd_scalar_two_reg_misc(DisasContext *s, uint32_t insn)
TCGv_ptr tcg_fpstatus;
switch (opcode) {
+ case 0x7: /* SQABS / SQNEG */
+ break;
case 0xa: /* CMLT */
if (u) {
unallocated_encoding(s);
@@ -7441,11 +7451,25 @@ static void disas_simd_scalar_two_reg_misc(DisasContext *s, uint32_t insn)
write_fp_dreg(s, rd, tcg_rd);
tcg_temp_free_i64(tcg_rd);
tcg_temp_free_i64(tcg_rn);
- } else if (size == 2) {
- TCGv_i32 tcg_rn = read_fp_sreg(s, rn);
+ } else {
+ TCGv_i32 tcg_rn = tcg_temp_new_i32();
TCGv_i32 tcg_rd = tcg_temp_new_i32();
+ read_vec_element_i32(s, tcg_rn, rn, 0, size);
+
switch (opcode) {
+ case 0x7: /* SQABS, SQNEG */
+ {
+ NeonGenOneOpEnvFn *genfn;
+ static NeonGenOneOpEnvFn * const fns[3][2] = {
+ { gen_helper_neon_qabs_s8, gen_helper_neon_qneg_s8 },
+ { gen_helper_neon_qabs_s16, gen_helper_neon_qneg_s16 },
+ { gen_helper_neon_qabs_s32, gen_helper_neon_qneg_s32 },
+ };
+ genfn = fns[size][u];
+ genfn(tcg_rd, cpu_env, tcg_rn);
+ break;
+ }
case 0x1a: /* FCVTNS */
case 0x1b: /* FCVTMS */
case 0x1c: /* FCVTAS */
@@ -7475,8 +7499,6 @@ static void disas_simd_scalar_two_reg_misc(DisasContext *s, uint32_t insn)
write_fp_sreg(s, rd, tcg_rd);
tcg_temp_free_i32(tcg_rd);
tcg_temp_free_i32(tcg_rn);
- } else {
- g_assert_not_reached();
}
if (is_fcvt) {
@@ -9177,8 +9199,7 @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn)
unallocated_encoding(s);
return;
}
- unsupported_encoding(s, insn);
- return;
+ break;
case 0xc ... 0xf:
case 0x16 ... 0x1d:
case 0x1f:
@@ -9389,6 +9410,13 @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn)
gen_helper_cls32(tcg_res, tcg_op);
}
break;
+ case 0x7: /* SQABS, SQNEG */
+ if (u) {
+ gen_helper_neon_qneg_s32(tcg_res, cpu_env, tcg_op);
+ } else {
+ gen_helper_neon_qabs_s32(tcg_res, cpu_env, tcg_op);
+ }
+ break;
case 0xb: /* ABS, NEG */
if (u) {
tcg_gen_neg_i32(tcg_res, tcg_op);
@@ -9463,6 +9491,17 @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn)
gen_helper_neon_cnt_u8(tcg_res, tcg_op);
}
break;
+ case 0x7: /* SQABS, SQNEG */
+ {
+ NeonGenOneOpEnvFn *genfn;
+ static NeonGenOneOpEnvFn * const fns[2][2] = {
+ { gen_helper_neon_qabs_s8, gen_helper_neon_qneg_s8 },
+ { gen_helper_neon_qabs_s16, gen_helper_neon_qneg_s16 },
+ };
+ genfn = fns[size][u];
+ genfn(tcg_res, cpu_env, tcg_op);
+ break;
+ }
case 0x8: /* CMGT, CMGE */
case 0x9: /* CMEQ, CMLE */
case 0xa: /* CMLT */
--
1.9.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 6/6] target-arm: A64: Add saturating accumulate ops (USQADD/SUQADD)
2014-03-19 12:05 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
` (4 preceding siblings ...)
2014-03-19 12:05 ` [Qemu-devel] [PULL 5/6] target-arm: A64: Add saturating int ops (SQNEG/SQABS) Peter Maydell
@ 2014-03-19 12:05 ` Peter Maydell
2014-03-19 13:33 ` [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
6 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2014-03-19 12:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Blue Swirl, Andreas Färber, qemu-devel, Aurelien Jarno
From: Alex Bennée <alex.bennee@linaro.org>
Add the saturating accumulate operations USQADD and SUQADD
to the A64 instruction set. This completes coverage of A64 Neon.
These operations (which are unsigned + signed -> signed and
signed + unsigned -> unsigned) don't exist in the A32/T32
instruction set, so require a complete new set of helper functions.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
target-arm/helper.h | 20 ++++--
target-arm/neon_helper.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
target-arm/translate-a64.c | 109 ++++++++++++++++++++++++++++--
3 files changed, 284 insertions(+), 10 deletions(-)
diff --git a/target-arm/helper.h b/target-arm/helper.h
index b006fd5..366c1b3 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -186,12 +186,20 @@ DEF_HELPER_FLAGS_2(rints, TCG_CALL_NO_RWG, f32, f32, ptr)
DEF_HELPER_FLAGS_2(rintd, TCG_CALL_NO_RWG, f64, f64, ptr)
/* neon_helper.c */
-DEF_HELPER_3(neon_qadd_u8, i32, env, i32, i32)
-DEF_HELPER_3(neon_qadd_s8, i32, env, i32, i32)
-DEF_HELPER_3(neon_qadd_u16, i32, env, i32, i32)
-DEF_HELPER_3(neon_qadd_s16, i32, env, i32, i32)
-DEF_HELPER_3(neon_qadd_u32, i32, env, i32, i32)
-DEF_HELPER_3(neon_qadd_s32, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_qadd_u8, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_qadd_s8, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_qadd_u16, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_qadd_s16, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_qadd_u32, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_qadd_s32, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_uqadd_s8, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_uqadd_s16, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_uqadd_s32, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_uqadd_s64, TCG_CALL_NO_RWG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(neon_sqadd_u8, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_sqadd_u16, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_sqadd_u32, TCG_CALL_NO_RWG, i32, env, i32, i32)
+DEF_HELPER_FLAGS_3(neon_sqadd_u64, TCG_CALL_NO_RWG, i64, env, i64, i64)
DEF_HELPER_3(neon_qsub_u8, i32, env, i32, i32)
DEF_HELPER_3(neon_qsub_s8, i32, env, i32, i32)
DEF_HELPER_3(neon_qsub_u16, i32, env, i32, i32)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index e23f224..8d6f9a9 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -236,6 +236,171 @@ uint64_t HELPER(neon_qadd_s64)(CPUARMState *env, uint64_t src1, uint64_t src2)
return res;
}
+/* Unsigned saturating accumulate of signed value
+ *
+ * Op1/Rn is treated as signed
+ * Op2/Rd is treated as unsigned
+ *
+ * Explicit casting is used to ensure the correct sign extension of
+ * inputs. The result is treated as a unsigned value and saturated as such.
+ *
+ * We use a macro for the 8/16 bit cases which expects signed integers of va,
+ * vb, and vr for interim calculation and an unsigned 32 bit result value r.
+ */
+
+#define USATACC(bits, shift) \
+ do { \
+ va = sextract32(a, shift, bits); \
+ vb = extract32(b, shift, bits); \
+ vr = va + vb; \
+ if (vr > UINT##bits##_MAX) { \
+ SET_QC(); \
+ vr = UINT##bits##_MAX; \
+ } else if (vr < 0) { \
+ SET_QC(); \
+ vr = 0; \
+ } \
+ r = deposit32(r, shift, bits, vr); \
+ } while (0)
+
+uint32_t HELPER(neon_uqadd_s8)(CPUARMState *env, uint32_t a, uint32_t b)
+{
+ int16_t va, vb, vr;
+ uint32_t r = 0;
+
+ USATACC(8, 0);
+ USATACC(8, 8);
+ USATACC(8, 16);
+ USATACC(8, 24);
+ return r;
+}
+
+uint32_t HELPER(neon_uqadd_s16)(CPUARMState *env, uint32_t a, uint32_t b)
+{
+ int32_t va, vb, vr;
+ uint64_t r = 0;
+
+ USATACC(16, 0);
+ USATACC(16, 16);
+ return r;
+}
+
+#undef USATACC
+
+uint32_t HELPER(neon_uqadd_s32)(CPUARMState *env, uint32_t a, uint32_t b)
+{
+ int64_t va = (int32_t)a;
+ int64_t vb = (uint32_t)b;
+ int64_t vr = va + vb;
+ if (vr > UINT32_MAX) {
+ SET_QC();
+ vr = UINT32_MAX;
+ } else if (vr < 0) {
+ SET_QC();
+ vr = 0;
+ }
+ return vr;
+}
+
+uint64_t HELPER(neon_uqadd_s64)(CPUARMState *env, uint64_t a, uint64_t b)
+{
+ uint64_t res;
+ res = a + b;
+ /* We only need to look at the pattern of SIGN bits to detect
+ * +ve/-ve saturation
+ */
+ if (~a & b & ~res & SIGNBIT64) {
+ SET_QC();
+ res = UINT64_MAX;
+ } else if (a & ~b & res & SIGNBIT64) {
+ SET_QC();
+ res = 0;
+ }
+ return res;
+}
+
+/* Signed saturating accumulate of unsigned value
+ *
+ * Op1/Rn is treated as unsigned
+ * Op2/Rd is treated as signed
+ *
+ * The result is treated as a signed value and saturated as such
+ *
+ * We use a macro for the 8/16 bit cases which expects signed integers of va,
+ * vb, and vr for interim calculation and an unsigned 32 bit result value r.
+ */
+
+#define SSATACC(bits, shift) \
+ do { \
+ va = extract32(a, shift, bits); \
+ vb = sextract32(b, shift, bits); \
+ vr = va + vb; \
+ if (vr > INT##bits##_MAX) { \
+ SET_QC(); \
+ vr = INT##bits##_MAX; \
+ } else if (vr < INT##bits##_MIN) { \
+ SET_QC(); \
+ vr = INT##bits##_MIN; \
+ } \
+ r = deposit32(r, shift, bits, vr); \
+ } while (0)
+
+uint32_t HELPER(neon_sqadd_u8)(CPUARMState *env, uint32_t a, uint32_t b)
+{
+ int16_t va, vb, vr;
+ uint32_t r = 0;
+
+ SSATACC(8, 0);
+ SSATACC(8, 8);
+ SSATACC(8, 16);
+ SSATACC(8, 24);
+ return r;
+}
+
+uint32_t HELPER(neon_sqadd_u16)(CPUARMState *env, uint32_t a, uint32_t b)
+{
+ int32_t va, vb, vr;
+ uint32_t r = 0;
+
+ SSATACC(16, 0);
+ SSATACC(16, 16);
+
+ return r;
+}
+
+#undef SSATACC
+
+uint32_t HELPER(neon_sqadd_u32)(CPUARMState *env, uint32_t a, uint32_t b)
+{
+ int64_t res;
+ int64_t op1 = (uint32_t)a;
+ int64_t op2 = (int32_t)b;
+ res = op1 + op2;
+ if (res > INT32_MAX) {
+ SET_QC();
+ res = INT32_MAX;
+ } else if (res < INT32_MIN) {
+ SET_QC();
+ res = INT32_MIN;
+ }
+ return res;
+}
+
+uint64_t HELPER(neon_sqadd_u64)(CPUARMState *env, uint64_t a, uint64_t b)
+{
+ uint64_t res;
+ res = a + b;
+ /* We only need to look at the pattern of SIGN bits to detect an overflow */
+ if (((a & res)
+ | (~b & res)
+ | (a & ~b)) & SIGNBIT64) {
+ SET_QC();
+ res = INT64_MAX;
+ }
+ return res;
+}
+
+
#define NEON_USAT(dest, src1, src2, type) do { \
uint32_t tmp = (uint32_t)src1 - (uint32_t)src2; \
if (tmp != (type)tmp) { \
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 18659d7..9f06450 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -7321,6 +7321,101 @@ static void handle_2misc_narrow(DisasContext *s, bool scalar,
}
}
+/* Remaining saturating accumulating ops */
+static void handle_2misc_satacc(DisasContext *s, bool is_scalar, bool is_u,
+ bool is_q, int size, int rn, int rd)
+{
+ bool is_double = (size == 3);
+
+ if (is_double) {
+ TCGv_i64 tcg_rn = tcg_temp_new_i64();
+ TCGv_i64 tcg_rd = tcg_temp_new_i64();
+ int pass;
+
+ for (pass = 0; pass < (is_scalar ? 1 : 2); pass++) {
+ read_vec_element(s, tcg_rn, rn, pass, MO_64);
+ read_vec_element(s, tcg_rd, rd, pass, MO_64);
+
+ if (is_u) { /* USQADD */
+ gen_helper_neon_uqadd_s64(tcg_rd, cpu_env, tcg_rn, tcg_rd);
+ } else { /* SUQADD */
+ gen_helper_neon_sqadd_u64(tcg_rd, cpu_env, tcg_rn, tcg_rd);
+ }
+ write_vec_element(s, tcg_rd, rd, pass, MO_64);
+ }
+ if (is_scalar) {
+ clear_vec_high(s, rd);
+ }
+
+ tcg_temp_free_i64(tcg_rd);
+ tcg_temp_free_i64(tcg_rn);
+ } else {
+ TCGv_i32 tcg_rn = tcg_temp_new_i32();
+ TCGv_i32 tcg_rd = tcg_temp_new_i32();
+ int pass, maxpasses;
+
+ if (is_scalar) {
+ maxpasses = 1;
+ } else {
+ maxpasses = is_q ? 4 : 2;
+ }
+
+ for (pass = 0; pass < maxpasses; pass++) {
+ if (is_scalar) {
+ read_vec_element_i32(s, tcg_rn, rn, pass, size);
+ read_vec_element_i32(s, tcg_rd, rd, pass, size);
+ } else {
+ read_vec_element_i32(s, tcg_rn, rn, pass, MO_32);
+ read_vec_element_i32(s, tcg_rd, rd, pass, MO_32);
+ }
+
+ if (is_u) { /* USQADD */
+ switch (size) {
+ case 0:
+ gen_helper_neon_uqadd_s8(tcg_rd, cpu_env, tcg_rn, tcg_rd);
+ break;
+ case 1:
+ gen_helper_neon_uqadd_s16(tcg_rd, cpu_env, tcg_rn, tcg_rd);
+ break;
+ case 2:
+ gen_helper_neon_uqadd_s32(tcg_rd, cpu_env, tcg_rn, tcg_rd);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+ } else { /* SUQADD */
+ switch (size) {
+ case 0:
+ gen_helper_neon_sqadd_u8(tcg_rd, cpu_env, tcg_rn, tcg_rd);
+ break;
+ case 1:
+ gen_helper_neon_sqadd_u16(tcg_rd, cpu_env, tcg_rn, tcg_rd);
+ break;
+ case 2:
+ gen_helper_neon_sqadd_u32(tcg_rd, cpu_env, tcg_rn, tcg_rd);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+ }
+
+ if (is_scalar) {
+ TCGv_i64 tcg_zero = tcg_const_i64(0);
+ write_vec_element(s, tcg_zero, rd, 0, MO_64);
+ tcg_temp_free_i64(tcg_zero);
+ }
+ write_vec_element_i32(s, tcg_rd, rd, pass, MO_32);
+ }
+
+ if (!is_q) {
+ clear_vec_high(s, rd);
+ }
+
+ tcg_temp_free_i32(tcg_rd);
+ tcg_temp_free_i32(tcg_rn);
+ }
+}
+
/* C3.6.12 AdvSIMD scalar two reg misc
* 31 30 29 28 24 23 22 21 17 16 12 11 10 9 5 4 0
* +-----+---+-----------+------+-----------+--------+-----+------+------+
@@ -7340,6 +7435,9 @@ static void disas_simd_scalar_two_reg_misc(DisasContext *s, uint32_t insn)
TCGv_ptr tcg_fpstatus;
switch (opcode) {
+ case 0x3: /* USQADD / SUQADD*/
+ handle_2misc_satacc(s, true, u, false, size, rn, rd);
+ return;
case 0x7: /* SQABS / SQNEG */
break;
case 0xa: /* CMLT */
@@ -7427,10 +7525,7 @@ static void disas_simd_scalar_two_reg_misc(DisasContext *s, uint32_t insn)
}
break;
default:
- /* Other categories of encoding in this class:
- * + SUQADD/USQADD/SQABS/SQNEG : size 8, 16, 32 or 64
- */
- unsupported_encoding(s, insn);
+ unallocated_encoding(s);
return;
}
@@ -9194,6 +9289,12 @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn)
}
break;
case 0x3: /* SUQADD, USQADD */
+ if (size == 3 && !is_q) {
+ unallocated_encoding(s);
+ return;
+ }
+ handle_2misc_satacc(s, false, u, is_q, size, rn, rd);
+ return;
case 0x7: /* SQABS, SQNEG */
if (size == 3 && !is_q) {
unallocated_encoding(s);
--
1.9.0
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2014-03-19 12:05 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
` (5 preceding siblings ...)
2014-03-19 12:05 ` [Qemu-devel] [PULL 6/6] target-arm: A64: Add saturating accumulate ops (USQADD/SUQADD) Peter Maydell
@ 2014-03-19 13:33 ` Peter Maydell
6 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2014-03-19 13:33 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Blue Swirl, QEMU Developers, Aurelien Jarno
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 0/6] target-arm queue
@ 2016-07-07 13:48 Peter Maydell
2016-07-11 10:16 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2016-07-07 13:48 UTC (permalink / raw)
To: qemu-devel
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%)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2016-07-07 13:48 Peter Maydell
@ 2016-07-11 10:16 ` Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2016-07-11 10:16 UTC (permalink / raw)
To: QEMU Developers
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 0/6] target-arm queue
@ 2018-07-30 14:17 Peter Maydell
2018-07-30 18:11 ` Peter Maydell
0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2018-07-30 14:17 UTC (permalink / raw)
To: qemu-devel
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(-)
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] target-arm queue
2018-07-30 14:17 Peter Maydell
@ 2018-07-30 18:11 ` Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-07-30 18:11 UTC (permalink / raw)
To: QEMU Developers
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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 0/6] target-arm queue
@ 2018-10-29 15:34 Peter Maydell
0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2018-10-29 15:34 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2018-10-29 15:35 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-19 12:05 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 1/6] ahci: fix sysbus support Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 2/6] pl011: reset the fifo when enabled or disabled Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 3/6] pl011: fix UARTRSR accesses corrupting the UARTCR value Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 4/6] pl011: fix incorrect logic to set the RXFF flag Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 5/6] target-arm: A64: Add saturating int ops (SQNEG/SQABS) Peter Maydell
2014-03-19 12:05 ` [Qemu-devel] [PULL 6/6] target-arm: A64: Add saturating accumulate ops (USQADD/SUQADD) Peter Maydell
2014-03-19 13:33 ` [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
-- strict thread matches above, loose matches on Subject: below --
2018-10-29 15:34 Peter Maydell
2018-07-30 14:17 Peter Maydell
2018-07-30 18:11 ` Peter Maydell
2016-07-07 13:48 Peter Maydell
2016-07-11 10:16 ` Peter Maydell
2013-10-25 18:07 Peter Maydell
2013-10-31 14:02 ` Edgar E. Iglesias
2013-10-31 14:18 ` Andreas Färber
2013-10-31 14:21 ` Anthony Liguori
2013-10-31 14:31 ` Peter Maydell
2013-10-31 14:36 ` Andreas Färber
2013-10-31 14:39 ` Anthony Liguori
2013-10-31 14:45 ` Andreas Färber
2013-10-31 14:54 ` Anthony Liguori
2013-10-31 15:04 ` Anthony Liguori
2013-10-31 16:52 ` Andreas Färber
2013-10-31 16:54 ` Anthony Liguori
2013-10-31 17:10 ` Andreas Färber
2013-10-31 17:02 ` Peter Maydell
2013-10-31 17:15 ` Peter Maydell
2013-10-31 18:55 ` Anthony Liguori
2013-10-31 15:16 ` Peter Maydell
2013-10-31 17:14 ` Andreas Färber
2013-10-31 17:18 ` Peter Maydell
2013-10-31 17:27 ` Andreas Färber
2013-10-31 17:51 ` Peter Maydell
2013-10-31 18:58 ` Anthony Liguori
2013-10-31 22:13 ` Edgar E. Iglesias
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).