| Message ID | 1442178225-26780-3-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On Sun, Sep 13, 2015 at 11:03:45PM +0200, Aurelien Jarno wrote: > The xscmpodp and xscmpudp instructions only have the AX, BX bits in > there encoding, the lowest bit (usually TX) is marked as an invalid > bit. We therefore can't decode them with GEN_XX2FORM, which decodes > the two lowest bit. > > Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark > the lowest bit as invalid. > > Cc: Tom Musta <tommusta@gmail.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: qemu-stable@nongnu.org > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > target-ppc/translate.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index 84c5cea..c0eed13 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \ > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \ > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2) > > +#undef GEN_XX2IFORM > +#define GEN_XX2IFORM(name, opc2, opc3, fl2) \ > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \ > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \ > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \ > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2) > + > #undef GEN_XX3_RC_FORM > #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2) \ > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \ > @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX), > GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX), > GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX), > GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX), > -GEN_XX2FORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > -GEN_XX2FORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), > +GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > +GEN_XX2IFORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), > GEN_XX3FORM(xsmaxdp, 0x00, 0x14, PPC2_VSX), > GEN_XX3FORM(xsmindp, 0x00, 0x15, PPC2_VSX), > GEN_XX2FORM(xscvdpsp, 0x12, 0x10, PPC2_VSX), This particular patch fixed a number of crashes I was experiencing in libm in Fedora 22 (ppc64), which I traced to the xscmpudp instruction causing SIGILL. So: Tested-by: Richard W.M. Jones <rjones@redhat.com> Cole: I think we should add this small patch series to Fedora, even though it's not upstream yet, since it required for running F22/ppc64 guests reliably (on non-ppc hosts). Rich.
On 13/09/15 23:03, Aurelien Jarno wrote: > The xscmpodp and xscmpudp instructions only have the AX, BX bits in > there encoding, the lowest bit (usually TX) is marked as an invalid > bit. We therefore can't decode them with GEN_XX2FORM, which decodes > the two lowest bit. > > Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark > the lowest bit as invalid. > > Cc: Tom Musta <tommusta@gmail.com> > Cc: Alexander Graf <agraf@suse.de> > Cc: qemu-stable@nongnu.org > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > target-ppc/translate.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index 84c5cea..c0eed13 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \ > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \ > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2) > > +#undef GEN_XX2IFORM > +#define GEN_XX2IFORM(name, opc2, opc3, fl2) \ > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \ > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \ > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \ > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2) > + > #undef GEN_XX3_RC_FORM > #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2) \ > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \ > @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX), > GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX), > GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX), > GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX), > -GEN_XX2FORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > -GEN_XX2FORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), > +GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > +GEN_XX2IFORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), According to PowerISA 2.07, xscmpodp and xscmpudp are of type XX3, not of type XX2 ... so should this macro maybe rather be named XX3IFORM instead? Thomas
On 2015-09-22 12:26, Thomas Huth wrote: > On 13/09/15 23:03, Aurelien Jarno wrote: > > The xscmpodp and xscmpudp instructions only have the AX, BX bits in > > there encoding, the lowest bit (usually TX) is marked as an invalid > > bit. We therefore can't decode them with GEN_XX2FORM, which decodes > > the two lowest bit. > > > > Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark > > the lowest bit as invalid. > > > > Cc: Tom Musta <tommusta@gmail.com> > > Cc: Alexander Graf <agraf@suse.de> > > Cc: qemu-stable@nongnu.org > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > --- > > target-ppc/translate.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > > index 84c5cea..c0eed13 100644 > > --- a/target-ppc/translate.c > > +++ b/target-ppc/translate.c > > @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \ > > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \ > > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2) > > > > +#undef GEN_XX2IFORM > > +#define GEN_XX2IFORM(name, opc2, opc3, fl2) \ > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \ > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \ > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \ > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2) > > + > > #undef GEN_XX3_RC_FORM > > #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2) \ > > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \ > > @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX), > > GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX), > > GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX), > > GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX), > > -GEN_XX2FORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > > -GEN_XX2FORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), > > +GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > > +GEN_XX2IFORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), > > According to PowerISA 2.07, xscmpodp and xscmpudp are of type XX3, not > of type XX2 ... so should this macro maybe rather be named XX3IFORM instead? Indeed, I have chosen the name without actually realizing the manual also give names. Then I do wonder if the lower bit is really decoded as invalid, I wouldn't be surprised it is actually just ignored. I'll try to do some tests on real hardware and come up with a fixup patch. Aurelien
The modern versions of the ISA require that reserved instruction bits be ignored for server class processors (see Book I, section 1.3.3). I believe older versions of the ISA allowed for Illegal Instruction Interrupt or "Boundedly Undefined", which is, of course, much less specific. QEMU supports implementations from both eras and, as a general rule, flags this situation as an illegal instruction. So I would expect that real hardware will ignore the bit. You will still be left with the choice of making this decoder consistent with the hardware or consistent with the rest of QEMU :) When I added support for VSX, the intent was the latter. On Tue, Sep 22, 2015 at 4:28 PM, Aurelien Jarno <aurelien@aurel32.net> wrote: > On 2015-09-22 12:26, Thomas Huth wrote: > > On 13/09/15 23:03, Aurelien Jarno wrote: > > > The xscmpodp and xscmpudp instructions only have the AX, BX bits in > > > there encoding, the lowest bit (usually TX) is marked as an invalid > > > bit. We therefore can't decode them with GEN_XX2FORM, which decodes > > > the two lowest bit. > > > > > > Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark > > > the lowest bit as invalid. > > > > > > Cc: Tom Musta <tommusta@gmail.com> > > > Cc: Alexander Graf <agraf@suse.de> > > > Cc: qemu-stable@nongnu.org > > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > > --- > > > target-ppc/translate.c | 11 +++++++++-- > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > > > index 84c5cea..c0eed13 100644 > > > --- a/target-ppc/translate.c > > > +++ b/target-ppc/translate.c > > > @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, > opc3, 0, PPC_NONE, fl2), \ > > > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \ > > > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2) > > > > > > +#undef GEN_XX2IFORM > > > +#define GEN_XX2IFORM(name, opc2, opc3, fl2) > \ > > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \ > > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \ > > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \ > > > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2) > > > + > > > #undef GEN_XX3_RC_FORM > > > #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2) > \ > > > GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, > PPC_NONE, fl2), \ > > > @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX), > > > GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX), > > > GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX), > > > GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX), > > > -GEN_XX2FORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > > > -GEN_XX2FORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), > > > +GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), > > > +GEN_XX2IFORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), > > > > According to PowerISA 2.07, xscmpodp and xscmpudp are of type XX3, not > > of type XX2 ... so should this macro maybe rather be named XX3IFORM > instead? > > Indeed, I have chosen the name without actually realizing the manual > also give names. Then I do wonder if the lower bit is really decoded as > invalid, I wouldn't be surprised it is actually just ignored. > > I'll try to do some tests on real hardware and come up with a fixup > patch. > > Aurelien > > -- > Aurelien Jarno GPG: 4096R/1DDD8C9B > aurelien@aurel32.net http://www.aurel32.net >
diff --git a/target-ppc/translate.c b/target-ppc/translate.c index 84c5cea..c0eed13 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2) +#undef GEN_XX2IFORM +#define GEN_XX2IFORM(name, opc2, opc3, fl2) \ +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \ +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \ +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \ +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2) + #undef GEN_XX3_RC_FORM #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2) \ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, fl2), \ @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX), GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX), GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX), GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX), -GEN_XX2FORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), -GEN_XX2FORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), +GEN_XX2IFORM(xscmpodp, 0x0C, 0x05, PPC2_VSX), +GEN_XX2IFORM(xscmpudp, 0x0C, 0x04, PPC2_VSX), GEN_XX3FORM(xsmaxdp, 0x00, 0x14, PPC2_VSX), GEN_XX3FORM(xsmindp, 0x00, 0x15, PPC2_VSX), GEN_XX2FORM(xscvdpsp, 0x12, 0x10, PPC2_VSX),
The xscmpodp and xscmpudp instructions only have the AX, BX bits in there encoding, the lowest bit (usually TX) is marked as an invalid bit. We therefore can't decode them with GEN_XX2FORM, which decodes the two lowest bit. Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark the lowest bit as invalid. Cc: Tom Musta <tommusta@gmail.com> Cc: Alexander Graf <agraf@suse.de> Cc: qemu-stable@nongnu.org Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- target-ppc/translate.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)