Tyrel Datwyler <tyr...@linux.ibm.com> writes: > On 3/11/20 10:43 PM, Michael Ellerman wrote: >> Tyrel Datwyler <tyr...@linux.ibm.com> writes: >>> On 3/10/20 10:25 AM, Nathan Lynch wrote: >>>> Tyrel Datwyler <tyr...@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