[PATCH] x86: Incorrect vmgexit entry in prefix_table

[H.J. Lu]

On Wed, Jun 17, 2020 at 8:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 17.06.2020 16:32, H.J. Lu wrote:
> > On Wed, Jun 17, 2020 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 17.06.2020 16:11, H.J. Lu wrote:
> >>> On Wed, Jun 17, 2020 at 7:10 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 17.06.2020 16:02, H.J. Lu wrote:
> >>>>> On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils
> >>>>> <binutils@sourceware.org> wrote:
> >>>>>>
> >>>>>> From: "Cui,Lili" <lili.cui@intel.com>
> >>>>>>
> >>>>>>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.
> >>>>>> ---
> >>>>>>  opcodes/i386-dis.c | 2 +-
> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
> >>>>>> index 441866d6c97..55e595a4be0 100644
> >>>>>> --- a/opcodes/i386-dis.c
> >>>>>> +++ b/opcodes/i386-dis.c
> >>>>>> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {
> >>>>>>      { "vmmcall",       { Skip_MODRM }, 0 },
> >>>>>>      { "vmgexit",       { Skip_MODRM }, 0 },
> >>>>>>      { Bad_Opcode },
> >>>>>> -    { "vmgexit",       { Skip_MODRM }, 0 },
> >>>>>> +    { Bad_Opcode },
> >>>>>>    },
> >>>>>>
> >>>>>>    /* PREFIX_0F01_REG_5_MOD_0 */
> >>>>>> --
> >>>>>> 2.26.2
> >>>>>>
> >>>>>
> >>>>> This is the patch I am checking in.
> >>>>
> >>>> But both patches are wrong - as per the PM there are two encodings,
> >>>> i.e. either prefix is valid. Afaics from the quoted text above (I
> >>>> didn't get to see the original mail, presumably due to email issues
> >>>> over here) there's no justification here at all why the second
> >>>> entry would be invalid. Please revert.
> >>>
> >>> Please add a testcase to verify it.
> >>
> >> I didn't at the time I added support for it primarily because
> >> there's no way to sensibly express this to gas. And I'm very
> >> hesitant to add .byte-based tests (except when wanting to test
> >> the disassembler against invalid encodings) ... In any event
> >> the bad commit needs reverting first, or any such test added
> >> would fail. And also in any event it would have been prudent
> >> to check why things are the way they are, before "fixing" them.
> >> At that point one of the two of you could have decided to add
> >> such a test to your liking.
> >
> > All entries in dis-i386.c should have at least one test.  Currently
> > "make check" is clean.
>
> And a test like what you ask for can't be added without breaking
> it. Please undo the breakage, independent of the possible test
> case addition (to be honest, I have better uses of my time).
>

Testcase does 2 things:

1. Verify implementation.
2. Prevent accidental change.

Without a testcase indicates it isn't important.  You can take your time.

-- 
H.J.