Re: [PATCH v4 00/12] GDB support for more powerpc registers on linux

Pedro Alves
2018-08-16T17:00:56Z

--
Thanks,
Pedro Alves
On 08/16/2018 05:44 PM, Pedro Franco de Carvalho wrote:
>
> Pedro Franco de Carvalho <pedromfc@linux.ibm.com> writes:
>
>> These are all the changes compared to V2, following the suggestions
>> there, and following the changes to linux-tdep.c:
>>
>> * Adapted "Zero-initialize linux note sections" due to recent changes
>> from commit a616bb9450.
>>
>> * Added a comment to "Zero-initialize linux note sections"
>>
>> * Adapted all the four main patches that enable the registers
>> (PPR/DSCR, TAR, EBB/PMU and HTM) to account for the changes from
>> commit a616bb9450.
>>
>> * De-duplicated testcase names. Changed a few of the test names so
>> that they are environment-independent. Fixed a few other minor
>> issues with the testcases.
>>
>> * Changed one of the testcases for the HTM registers so that it
>> XFAILs. This testcase occasionally failed, possibly due to a kernel
>> bug.
>>
>> * Fixed some of the formatting and const-correctness in "[PowerPC] Add
>> support for HTM registers", in gdbserver/linux-ppc-low.c.
>>
>> * Added an item in the NEWS file on the last commit, indicating all
>> the newly supported registers from this series.
>
> Considering Ulrich had already approved the V2 patches, do these changes
> need to be re-approved, or can they be considered as obvious, and can I
> commit them?
>

I was reading through the series as you sent this. I sent a couple
comments to patches #12 and #14. The remarks in #12 is something that can
always be addressed with this series already merged. Re. the comments for #14,
it would be nice to go over the series and add missing intro comments to
functions, etc. Otherwise, it all LGTM. I only really skimmed
it for mechanics, since as you say Ulrich already reviewed it, and I'm
far from a Power expert anyway.

> I don't know if a NEWS item counts as obvious, but if not, I can submit
> it in a separate patch.

It's usually reviewed like other patches. gdb/NEWS falls under
Eli's documentation maintainership.

Thanks,
Pedro Alves