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

[Pierre Morel]

On 21/08/2017 11:16, Cornelia Huck wrote:
> If we do not provide zpci, pci reconfiguration via sclp is not available
> either. Don't indicate it in the sclp facilities and return an invalid
> command if the guest tries to issue pci configure/deconfigure.
>
> Reviewed-by: Thomas Huth <address@hidden>
> Signed-off-by: Cornelia Huck <address@hidden>
> ---
>   hw/s390x/sclp.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 9253dbbc64..d0104cd784 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -59,6 +59,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>       int rnsize, rnmax;
>       int slots = MIN(machine->ram_slots, s390_get_memslot_count(kvm_state));
>       IplParameterBlock *ipib = s390_ipl_get_iplb();
> +    uint64_t sclp_facilities = SCLP_HAS_CPU_INFO;
>
>       CPU_FOREACH(cpu) {
>           cpu_count++;
> @@ -79,8 +80,10 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>
>       prepare_cpu_entries(sclp, read_info->entries, cpu_count);
>
> -    read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
> -                                        SCLP_HAS_PCI_RECONFIG);
> +    if (s390_has_feat(S390_FEAT_ZPCI)) {
> +        sclp_facilities |= SCLP_HAS_PCI_RECONFIG;
> +    }
> +    read_info->facilities = cpu_to_be64(sclp_facilities);
>
>       /* Memory Hotplug is only supported for the ccw machine type */
>       if (mhd) {
> @@ -385,10 +388,18 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, 
> uint32_t code)
>           sclp_c->unassign_storage(sclp, sccb);
>           break;
>       case SCLP_CMDW_CONFIGURE_PCI:
> -        s390_pci_sclp_configure(sccb);
> +        if (s390_has_feat(S390_FEAT_ZPCI)) {
> +            s390_pci_sclp_configure(sccb);
> +        } else {
> +            sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);

Hello Conny,

I find more logical if the response would be 0x06F0 : "Adapter type in 
SCCB not recognized"
Since we could have more than one adapter type... someday.
then the SCLP command would be valid but not the adapter type.

However, I will try to find a real hardware to test this since I noticed 
that my logic sometime... :) .

Another point is that don't you think that this test on S390_FEAT_ZPCI 
better belong to the dedicated PCI code inside of the 
s390_pci_sclp_configure().

best regards,

Pierre


> +        }
>           break;
>       case SCLP_CMDW_DECONFIGURE_PCI:
> -        s390_pci_sclp_deconfigure(sccb);
> +        if (s390_has_feat(S390_FEAT_ZPCI)) {
> +            s390_pci_sclp_deconfigure(sccb);
> +        } else {
> +            sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> +        }
>           break;
>       default:
>           efc->command_handler(ef, sccb, code);


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany