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

[Laurent Vivier]

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.

For instance, imagine someone using and older QEMU because he has
implemented some proprietary devices in it and doesn't want to port his
changes to a newer version of QEMU. He can fix the DRC index problem
only by updating the "firmware".

Laurent

Laurent