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

[Cornelia Huck]

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.]