Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions

[Halil Pasic]

On 08/22/2017 02:57 PM, Cornelia Huck wrote:
> On Tue, 22 Aug 2017 11:39:14 +0200
> Cornelia Huck <address@hidden> wrote:
> 
>> On Tue, 22 Aug 2017 11:20:51 +0200
>> Halil Pasic <address@hidden> wrote:
>>
>>> On 08/22/2017 10:39 AM, Cornelia Huck wrote:  
>>>>> I'm fine either way. If I imagine having a lots of adapter types, then I
>>>>> would expect a switch or a jumptable on the type before handling control
>>>>> to the pci specific function. In this case statically not supported types
>>>>> would probably get caught by the default branch of the switch and for a
>>>>> jumptable it could even handle the dynamic case (based on the facilities)
>>>>> trivially. In short both approaches can make sense.    
>>>> I'm also wondering at the naming (the command sounds very
>>>> pci-specific). I'd just stick with this approach (modulo a possible
>>>> change of the response code, for which I need to rely on you guys).
>>>>     
>>>
>>>
>>> Well, the QEMU name of the command is misleading misleading. In the AR
>>> it's called 'Configure I/O Adapter'. The PCI comes into the picture via
>>> byte 8 of the SCCB, the so called adapter type. Valid values for the
>>> adapter type are: 00-01 reserved; 02 PCI function; 03-FF reserved. So
>>> at this point we only have PCI.  
>>
>> OK, misleading naming combined with missing documentation leads to
>> confusion...
>>
>> So:
>>
>> - s/PCI/IOA/ for SCLP_CMDW_{CONFIGURE,DECONFIGURE}_PCI
>> - have a switch/case over byte 8 with only one case (pci)
> 
> - switch to response code 0x06f0 instead of invalid command
> 
>> - move the pci feature check into the pci code(? - not sure)
>>
>> There's still the question of when this sclp command first became
>> available...
> 
> ...because we probably want to indicate invalid command for older
> machine types (or is there another facility bit?)
> 
> Another question: There's the sclp facilities bit SCLP_HAS_PCI_RECONFIG
> - is that really pci, or I/O adapter as for the actual commands?
> 
> [The Linux kernel uses the _PCI naming scheme, so I can't derive
> anything from that.]
> 

Ah, we to each other almost simultaneously... See my other mail.