[SLOF] [PATCH] Correctly set ibm,my-drc-index

[Thomas Huth]

On 14/09/15 13:02, Laurent Vivier wrote:
> 
> 
> On 14/09/2015 12:28, Nikunj A Dadhania wrote:
>> Laurent Vivier <lvivier at redhat.com> writes:
>>
>>> On 14/09/2015 10:35, Alexey Kardashevskiy wrote:
>>>> On 09/14/2015 01:57 PM, David Gibson wrote:
>>>>> On Sun, 13 Sep 2015 21:25:17 -0500
>>>>> Michael Roth <mdroth at linux.vnet.ibm.com> wrote:
>>>>>
>>>>>> Quoting David Gibson (2015-09-13 18:45:00)
>>>>>>> On Fri, 11 Sep 2015 18:48:19 +0200
>>>>>>> Laurent Vivier <lvivier at redhat.com> wrote:
>>>>>>>
>>>>>>>> The current calculation of DRC index for PCI card doesn't take
>>>>>>>> into account the PHB bridge index, so with several bridges we
>>>>>>>> can have several devices with the same DRC index.
>>>>>>>>
>>>>>>>> This is a problem when we want to unplug a PCI card with
>>>>>>>> drmgr which is based on the DRC index.
>>>>>>>>
>>>>>>>> Signed-off-by: Laurent Vivier <lvivier at redhat.com>
>>>>>>>
>>>>>>> Actually, I think the safer fix is just for SLOF not to touch the
>>>>>>> ibm,my-drc-index property if it's already present.  QEMU is already
>>>>>>> setting it correctly, and SLOF is overwriting it  :(.
>>>>>>
>>>>>> This patch makes sense and fixes a real bug, but my understanding is
>>>>>> that
>>>>>> SLOF only fills in properties missing from device node rather than
>>>>>> overwriting ones provided by QEMU. On my end, on the second PHB, I see
>>>>>> this for a hotplugged device in slot 1 (where prop comes from QEMU via
>>>>>> RTAS):
>>>>>>
>>>>>> mdroth at ubuntu:/proc/device-tree/pci at 800000020000001/pci at 2$ od -x
>>>>>> ibm,my-drc-index
>>>>>> 0000000 0140 1000
>>>>>> 0000004
>>>>>>
>>>>>> And after reboot (where prop comes from QEMU via SLOF):
>>>>>>
>>>>>> mdroth at ubuntu:/proc/device-tree/pci at 800000020000001/ethernet at 2$ od -x
>>>>>> ibm,my-drc-index
>>>>>> 0000000 0140 1000
>>>>>> 0000004
>>>>>>
>>>>>> If you're getting the buggy values (0040 1000, i.e. 40000010 be32 value)
>>>>>> it makes me think you're not using the QEMU-generated DTs for device
>>>>>> nodes. Older SLOF that doesn't have Nikunj's updates possibly? I think
>>>>>> in that case values might get overwritten/thrown out.
>>>>>
>>>>>
>>>>> Ah.  Good point, yes, the downstream qemu doesn't have the code to set
>>>>> my-drc-index.
>>>>>
>>>>> So the question is, do we fix this in SLOF, or do we pull down the qemu
>>>>> changes to move more device node creation into qemu, making it moot.
>>>>
>>>>
>>>> I vote for the latter and do not touch DRC index in SLOF.
>>>
>>> But code in SLOF is wrong, we can at least fix it and do whatever we
>>> want in QEMU. This will allow older versions of QEMU to work
>>> correctly.
>>
>> We have moved the PCI enumeration code to QEMU and have provided
>> backward compatibility.
>>
>>> Moreover, it is one line fix...
>>
>> So the newer QEMU will have newer SLOF.  Are you suggesting to submit
>> slof.bin for stable qemu branches to fix this issue?
> 
> No, what I suggest is just to fix a buggy code, or if this code is
> useless, you should remove it.

+1

Either fix it, or remove it, but having unused, buggy code hanging
around sounds like a bad idea to me, too.

 Thomas