On 08/22/2017 03:24 PM, Cornelia Huck wrote: > On Tue, 22 Aug 2017 14:58:37 +0200 > Halil Pasic <pa...@linux.vnet.ibm.com> wrote: > >> On 08/22/2017 11:39 AM, Cornelia Huck wrote: >>> On Tue, 22 Aug 2017 11:20:51 +0200 >>> Halil Pasic <pa...@linux.vnet.ibm.com> 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. > > Yes. > >> >>> - 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. > > I'd either have the check in the pci case for the adapter type, or in > the called function. It's probably cleaner to do the check before > calling the pci function. >
nod >> >> 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. > > Sure. But see the question in my other mail regarding the sclp > facilities bit (does it cover pci or adapter reconfiguration?) It (SCLP_HAS_PCI_RECONFIG) exactly covers adapter reconfiguration. That's what I tried to say with the paragraph above. > >> >> I would appreciate someone with AR access double checking. > > I'd have hoped you had AR access :p Yes, my statements are based solely on my reading of the AR (me still lacks druid-knowledge). What I've asked for is a second opinion (because AR-s are a twisty maze). > >> >>> >>> 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'm just wondering if there's another facility we're missing. The zpci > facility might imply presence of adapter reconfiguration, but are there > other facilities implying that as well, or even a dedicated facility? Yes. The SCLP facility with the facility code 33 (aka SCLP_HAS_PCI_RECONFIG). It is the dedicated facility. I don't think zPCI architecturally implies the presence of this SCLP command. And logically I would say it's either the other way around adapter reconfiguration implies zPCI (because otherwise adapter reconfiguration would be completely useless) or bidirectional. > >> >> I don't know where should I look for the historical details >> which go beyond the AR. > > If there is no independent facility, it is probably best to just always > provide the command, but fail for pci adapter type if the zpci facility > is off. IMHO we should SCLP_RC_INVALID_SCLP_COMMAND iff we don't provide SCLP_HAS_PCI_RECONFIG (which has bad name again s/PCI/IOA). I don't know of any facility except basic SCLP on which SCLP_HAS_PCI_RECONFIG depends on. For me both presenting and not presenting SCLP_HAS_PCI_RECONFIG to the guest (via read SCP info SCLP command) in the absence of the zPCI feature makes sense. The late because of the likely historical reasons, the former because I think the AR allows it and it gives us more flexibility. >