Re: [PATCH v5 07/10] PCI/IDE: Add IDE establishment helpers

[Alexey Kardashevskiy <aik@amd.com>]

On 17/10/25 15:40, dan.j.williams@intel.com wrote:
> Alexey Kardashevskiy wrote:
> [..]
>>> +	/*
>>> +	 * Now that in use ids are known, grab and assign a free id for idle
>>> +	 * streams to remove ambiguity of which key slot is being activated by a
>>> +	 * K_SET_GO event (PCIe r7.0 section 6.33.3 IDE Key Management (IDE_KM))
>>> +	 */
>>> +	reserved_id = find_first_zero_bit(pdev->ide_stream_map, U8_MAX);
>>
>>
>> pci_ide_init() is called when PCI is probing and at that point no
>> stream is enabled so reserved_id ends up as 0.
>>
>> Since my platform only wants 1 stream, should I have called
>> pci_ide_init_nr_streams() with "2", not "1"?
> 
> Hmm, no I would expect that number only needs to reflect the assignable
> id maps.
> 
>> pci_ide_stream_alloc() fails to find a stream - fails in:
>>
>>    struct stream_index *ep_stream __free(free_stream) = alloc_stream_index(
>>            pdev->ide_stream_map, pdev->nr_sel_ide, &__stream[PCI_IDE_EP]);
>>
>> as nr_sel_ide==1 (my EP has just one stream). If I go with
>> reserved_id==0 and set ide->stream_id=1 in the PSP TSM driver, then
>> streamindex#1 gets programmed (which is beyond the end of IDE cap)
>> with streamid=0. As if reserved_id is actually reserved_index but the
>> device wants me to reserve an ID.
> 
> The intent was ID reservation, not index. Will take a look, but first...
> 
>> We could simplify it by assigning something like "255" to all
>> unused+disabled streams. Thanks,
> 
> I thought we were still waiting to to determine if this behavior is a
> quirk of a test device, or something that Linux is going to see in
> production?

Well, the justification (about scarce resources in devices) remains so I suspect this behavior will continue.

And I tried blindly write "255" to all unused streams and that works fine too so the IDs do not have to be unique even for devices with that uncommon interpretation of the PCIe spec.

> Yes, I want the reserved id to start at 255 (unless BIOS already stole
> that id).
>
> In any event I will send out v7 without this, but include it in an
> incremental update along with the address association support from
> Yilun.

Sure. Thanks,


-- 
Alexey