[v1] Fixup exclusive store logic

[Qemu-devel] [PATCH v1 0/3] Fixup exclusive store logic

Posted by Alistair Francis 6 years, 8 months ago

I found some issues with the way exclusive store was working. This patch
series seems to fix the test cases that were failing for me.

The first patch is just a simple adjustment.

The third patch fixes the main bug I was seeing.

The second patch is left over from the RFC that seems like it is still a
good idea.

Changes from RFC:
 - Rewrite the third patch to correctly fix the issue.

Alistair Francis (3):
  target/arm: Update the memops for exclusive load
  tcg/tcg-op: Expose the tcg_gen_ext_i* functions
  target/arm: Correct exclusive store cmpxchg memop mask

 target/arm/translate-a64.c | 4 ++--
 tcg/tcg-op.c               | 4 ++--
 tcg/tcg-op.h               | 2 ++
 3 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.11.0

Re: [Qemu-devel] [PATCH v1 0/3] Fixup exclusive store logic

Posted by Alistair Francis 6 years, 8 months ago

On Fri, Aug 11, 2017 at 3:17 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> I found some issues with the way exclusive store was working. This patch
> series seems to fix the test cases that were failing for me.
>
> The first patch is just a simple adjustment.
>
> The third patch fixes the main bug I was seeing.
>
> The second patch is left over from the RFC that seems like it is still a
> good idea.

After working with the internal fuzzy testing team I have a test case
where exclusive operations are failing on master but working on top of
this patch series.

Thanks,
Alistair

>
> Changes from RFC:
>  - Rewrite the third patch to correctly fix the issue.
>
> Alistair Francis (3):
>   target/arm: Update the memops for exclusive load
>   tcg/tcg-op: Expose the tcg_gen_ext_i* functions
>   target/arm: Correct exclusive store cmpxchg memop mask
>
>  target/arm/translate-a64.c | 4 ++--
>  tcg/tcg-op.c               | 4 ++--
>  tcg/tcg-op.h               | 2 ++
>  3 files changed, 6 insertions(+), 4 deletions(-)
>
> --
> 2.11.0
>

Re: [Qemu-devel] [PATCH v1 0/3] Fixup exclusive store logic

Posted by Alistair Francis 6 years, 8 months ago

On Fri, Aug 11, 2017 at 4:21 PM, Alistair Francis <alistair23@gmail.com> wrote:
> On Fri, Aug 11, 2017 at 3:17 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> I found some issues with the way exclusive store was working. This patch
>> series seems to fix the test cases that were failing for me.
>>
>> The first patch is just a simple adjustment.
>>
>> The third patch fixes the main bug I was seeing.
>>
>> The second patch is left over from the RFC that seems like it is still a
>> good idea.

+ Portia from fuzzy testing team.

>
> After working with the internal fuzzy testing team I have a test case
> where exclusive operations are failing on master but working on top of
> this patch series.
>
> Thanks,
> Alistair
>
>>
>> Changes from RFC:
>>  - Rewrite the third patch to correctly fix the issue.
>>
>> Alistair Francis (3):
>>   target/arm: Update the memops for exclusive load
>>   tcg/tcg-op: Expose the tcg_gen_ext_i* functions
>>   target/arm: Correct exclusive store cmpxchg memop mask
>>
>>  target/arm/translate-a64.c | 4 ++--
>>  tcg/tcg-op.c               | 4 ++--
>>  tcg/tcg-op.h               | 2 ++
>>  3 files changed, 6 insertions(+), 4 deletions(-)
>>
>> --
>> 2.11.0
>>

Re: [Qemu-devel] [PATCH v1 0/3] Fixup exclusive store logic

Posted by Portia Stephens 6 years, 8 months ago

> -----Original Message-----
> From: alistair23@gmail.com [mailto:alistair23@gmail.com] On Behalf Of
> Alistair Francis
> Sent: Friday, August 11, 2017 4:23 PM
> To: Alistair Francis <alistai@xilinx.com>; Portia Stephens
> <portias@xilinx.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>;
> Peter Maydell <peter.maydell@linaro.org>; Edgar Iglesias
> <edgari@xilinx.com>; Edgar Iglesias <edgar.iglesias@gmail.com>; qemu-arm
> <qemu-arm@nongnu.org>
> Subject: Re: [PATCH v1 0/3] Fixup exclusive store logic
>
> On Fri, Aug 11, 2017 at 4:21 PM, Alistair Francis <alistair23@gmail.com> wrote:
> > On Fri, Aug 11, 2017 at 3:17 PM, Alistair Francis
> > <alistair.francis@xilinx.com> wrote:
> >> I found some issues with the way exclusive store was working. This patch
> >> series seems to fix the test cases that were failing for me.
> >>
> >> The first patch is just a simple adjustment.
> >>
> >> The third patch fixes the main bug I was seeing.
> >>
> >> The second patch is left over from the RFC that seems like it is still a
> >> good idea.
>
> + Portia from fuzzy testing team.
>
> >
> > After working with the internal fuzzy testing team I have a test case
> > where exclusive operations are failing on master but working on top of
> > this patch series.
> >

This patch series fixes the failures previously seen with exclusive stores
by our internal tests on AArch64.

Tested-by: Portia Stephens <portia.stephens@xilinx.com>

> > Thanks,
> > Alistair
> >
> >>
> >> Changes from RFC:
> >>  - Rewrite the third patch to correctly fix the issue.
> >>
> >> Alistair Francis (3):
> >>   target/arm: Update the memops for exclusive load
> >>   tcg/tcg-op: Expose the tcg_gen_ext_i* functions
> >>   target/arm: Correct exclusive store cmpxchg memop mask
> >>
> >>  target/arm/translate-a64.c | 4 ++--
> >>  tcg/tcg-op.c               | 4 ++--
> >>  tcg/tcg-op.h               | 2 ++
> >>  3 files changed, 6 insertions(+), 4 deletions(-)
> >>
> >> --
> >> 2.11.0
> >>


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: [Qemu-devel] [PATCH v1 0/3] Fixup exclusive store logic

Posted by Peter Maydell 6 years, 8 months ago

On 11 August 2017 at 23:17, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> I found some issues with the way exclusive store was working. This patch
> series seems to fix the test cases that were failing for me.
>
> The first patch is just a simple adjustment.
>
> The third patch fixes the main bug I was seeing.
>
> The second patch is left over from the RFC that seems like it is still a
> good idea.
>
> Changes from RFC:
>  - Rewrite the third patch to correctly fix the issue.
>
> Alistair Francis (3):
>   target/arm: Update the memops for exclusive load
>   tcg/tcg-op: Expose the tcg_gen_ext_i* functions
>   target/arm: Correct exclusive store cmpxchg memop mask
>
>  target/arm/translate-a64.c | 4 ++--
>  tcg/tcg-op.c               | 4 ++--
>  tcg/tcg-op.h               | 2 ++
>  3 files changed, 6 insertions(+), 4 deletions(-)

Is this series (or at least patches 1 and 3) worth putting
into 2.10 ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 0/3] Fixup exclusive store logic

Posted by Edgar E. Iglesias 6 years, 8 months ago

On Sat, Aug 12, 2017 at 11:24:30AM +0100, Peter Maydell wrote:
> On 11 August 2017 at 23:17, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
> > I found some issues with the way exclusive store was working. This patch
> > series seems to fix the test cases that were failing for me.
> >
> > The first patch is just a simple adjustment.
> >
> > The third patch fixes the main bug I was seeing.
> >
> > The second patch is left over from the RFC that seems like it is still a
> > good idea.
> >
> > Changes from RFC:
> >  - Rewrite the third patch to correctly fix the issue.
> >
> > Alistair Francis (3):
> >   target/arm: Update the memops for exclusive load
> >   tcg/tcg-op: Expose the tcg_gen_ext_i* functions
> >   target/arm: Correct exclusive store cmpxchg memop mask
> >
> >  target/arm/translate-a64.c | 4 ++--
> >  tcg/tcg-op.c               | 4 ++--
> >  tcg/tcg-op.h               | 2 ++
> >  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> Is this series (or at least patches 1 and 3) worth putting
> into 2.10 ?


I would vote for including it...

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 0/3] Fixup exclusive store logic

Posted by Alistair Francis 6 years, 8 months ago

On Sat, Aug 12, 2017 at 4:42 AM, Edgar E. Iglesias
<edgar.iglesias@xilinx.com> wrote:
> On Sat, Aug 12, 2017 at 11:24:30AM +0100, Peter Maydell wrote:
>> On 11 August 2017 at 23:17, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>> > I found some issues with the way exclusive store was working. This patch
>> > series seems to fix the test cases that were failing for me.
>> >
>> > The first patch is just a simple adjustment.
>> >
>> > The third patch fixes the main bug I was seeing.
>> >
>> > The second patch is left over from the RFC that seems like it is still a
>> > good idea.
>> >
>> > Changes from RFC:
>> >  - Rewrite the third patch to correctly fix the issue.
>> >
>> > Alistair Francis (3):
>> >   target/arm: Update the memops for exclusive load
>> >   tcg/tcg-op: Expose the tcg_gen_ext_i* functions
>> >   target/arm: Correct exclusive store cmpxchg memop mask
>> >
>> >  target/arm/translate-a64.c | 4 ++--
>> >  tcg/tcg-op.c               | 4 ++--
>> >  tcg/tcg-op.h               | 2 ++
>> >  3 files changed, 6 insertions(+), 4 deletions(-)
>>
>> Is this series (or at least patches 1 and 3) worth putting
>> into 2.10 ?
>
>
> I would vote for including it...

The only reason not to is because this bug has been in QEMU for a
while, so it obviously isn't hit very often. In saying that it is a
pretty big issue (32-bit pair stores are completely broken) which
might become an issue during the 2.10 support window and I don't see
many complications from including the series.

I agree with Edgar, probably worth including.

Thanks,
Alistair

>
> Cheers,
> Edgar