From: Michael Ellerman <mpe@ellerman.id.au>
To: Tyrel Datwyler <tyreld@linux.ibm.com>,
Nathan Lynch <nathanl@linux.ibm.com>
Cc: mwb@linux.vnet.ibm.com, msuchanek@suse.de, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc/pseries: fix of_read_drc_info_cell() to point at next record
Date: Fri, 13 Mar 2020 13:16:16 +1100 [thread overview]
Message-ID: <87k13pxamn.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <3a3950ed-a7fb-3458-dce0-0dd6e45f93eb@linux.ibm.com>
Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> On 3/11/20 10:43 PM, Michael Ellerman wrote:
>> Tyrel Datwyler <tyreld@linux.ibm.com> writes:
>>> On 3/10/20 10:25 AM, Nathan Lynch wrote:
>>>> Tyrel Datwyler <tyreld@linux.ibm.com> writes:
>>>>> The expectation is that when calling of_read_drc_info_cell()
>>>>> repeatedly to parse multiple drc-info records that the in/out curval
>>>>> parameter points at the start of the next record on return. However,
>>>>> the current behavior has curval still pointing at the final value of
>>>>> the record just parsed. The result of which is that if the
>>>>> ibm,drc-info property contains multiple properties the parsed value
>>>>> of the drc_type for any record after the first has the power_domain
>>>>> value of the previous record appended to the type string.
>>>>>
>>>>> Ex: observed the following 0xffffffff prepended to PHB
>>>>>
>>>>> [ 69.485037] drc-info: type: \xff\xff\xff\xffPHB, prefix: PHB , index_start: 0x20000001
>>>>> [ 69.485038] drc-info: suffix_start: 1, sequential_elems: 3072, sequential_inc: 1
>>>>> [ 69.485038] drc-info: power-domain: 0xffffffff, last_index: 0x20000c00
>>>>>
>>>>> Fix by incrementing curval past the power_domain value to point at
>>>>> drc_type string of next record.
>>>>>
>>>>> Fixes: a29396653b8bf ("pseries/drc-info: Search DRC properties for CPU indexes")
>>>>
>>>> I have a different commit hash for that:
>>>> e83636ac3334 pseries/drc-info: Search DRC properties for CPU indexes
>>>
>>> Oof, looks like I grabbed the commit hash from the SLES 15 SP1 branch. You have
>>> the correct upstream commit.
>>>
>>> Fixes: e83636ac3334 ("pseries/drc-info: Search DRC properties for CPU indexes")
>>>
>>> Michael, let me know if you want me to resubmit, or if you will fixup the Fixes
>>> tag on your end?
>>
>> I can update the Fixes tag.
>>
>> What's the practical effect of this bug? It seems like it should break
>> all the hotplug stuff, but presumably it doesn't in practice for some
>> reason?
>>
>> It would also be *really* nice if we had some unit tests for this
>> parsing code, it's demonstrably very bug prone.
>
> In practice PHBs are the only type of connector in the ibm,drc-info property
> that has multiple records. So, it breaks PHB hotplug, but by chance not pci,
> cpu, slot, or mem because they happen to only ever be a single record.
Thanks. I folded that into the change log.
cheers
next prev parent reply other threads:[~2020-03-13 2:17 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-07 2:45 [PATCH] powerpc/pseries: fix of_read_drc_info_cell() to point at next record Tyrel Datwyler
2020-03-10 17:25 ` Nathan Lynch
2020-03-10 18:18 ` Tyrel Datwyler
2020-03-12 5:43 ` Michael Ellerman
2020-03-12 15:56 ` Nathan Lynch
2020-03-13 2:29 ` Michael Ellerman
2020-03-12 21:34 ` Tyrel Datwyler
2020-03-13 2:16 ` Michael Ellerman [this message]
2020-03-26 12:06 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87k13pxamn.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman.id.au \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=msuchanek@suse.de \
--cc=mwb@linux.vnet.ibm.com \
--cc=nathanl@linux.ibm.com \
--cc=tyreld@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).