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

[Halil Pasic]

On 08/22/2017 11:39 AM, Cornelia Huck 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
nod

> - have a switch/case over byte 8 with only one case (pci)

Let's say some kind of a check for bit 8 is a good idea.

> - move the pci feature check into the pci code(? - not sure)

Don't know. Architecturally I don't see any direct connection
between the pci feature and this command.

The availability of SCLP_CMDW_{,DE}CONFIGURE_IOA is indicated
by the result of the read scp info command read info in
ReadInfo.facilities. I think we should assume that the read scp
info command is always there.

I would appreciate someone with AR access double checking.

> 
> There's still the question of when this sclp command first became
> available...
> 

I would argue that it should not be important for AR
compliance. Currently it operates only on PCI so I doubt it
pre-dates PCI. But I don't think the current implementation
is buggy because it offers the sclp command regardless
of the zPCI facility.

I don't know where should I look for the historical details
which go beyond the AR.

Halil