[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specifi
From: |
Halil Pasic |
Subject: |
Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions |
Date: |
Tue, 22 Aug 2017 15:54:32 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 08/22/2017 03:24 PM, Cornelia Huck wrote:
> On Tue, 22 Aug 2017 14:58:37 +0200
> Halil Pasic <address@hidden> wrote:
>
>> 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.
>
> 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.
>
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, (continued)
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Pierre Morel, 2017/08/21
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Halil Pasic, 2017/08/21
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Halil Pasic, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Halil Pasic, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Halil Pasic, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions,
Halil Pasic <=
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Halil Pasic, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Halil Pasic, 2017/08/22
- Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions, Cornelia Huck, 2017/08/22
[Qemu-devel] [PATCH v4 09/10] s390x/kvm: msi route fixup for non-pci, Cornelia Huck, 2017/08/21