Re: [Qemu-devel] [PATCH 3/5] s390: vfio_pci: Use a PCI Group structure

[Cornelia Huck <cohuck@redhat.com>]

On Fri, 10 May 2019 16:38:51 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:

> We use a S390PCIGroup structure to hold the information
> related to zPCI Function group.
> 
> This allows us to be ready to support multiple groups and to retrieve
> the group information from the host.

What if there is no host to retrieve information from?

> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  hw/s390x/s390-pci-bus.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  hw/s390x/s390-pci-bus.h  | 11 ++++++++++-
>  hw/s390x/s390-pci-inst.c | 22 +++++++++++++---------
>  3 files changed, 65 insertions(+), 10 deletions(-)

> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index be28962..8147847 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -284,21 +284,25 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
>          stq_p(&resquery->edma, ZPCI_EDMA_ADDR);
>          stl_p(&resquery->fid, pbdev->fid);
>          stw_p(&resquery->pchid, 0);
> -        stw_p(&resquery->ug, 1);
> +        stw_p(&resquery->ug, ZPCI_DEFAULT_FN_GRP);
>          stl_p(&resquery->uid, pbdev->uid);
>          stw_p(&resquery->hdr.rsp, CLP_RC_OK);
>          break;
>      }
>      case CLP_QUERY_PCI_FNGRP: {
>          ClpRspQueryPciGrp *resgrp = (ClpRspQueryPciGrp *)resh;
> -        resgrp->fr = 1;
> -        stq_p(&resgrp->dasm, 0);
> -        stq_p(&resgrp->msia, ZPCI_MSI_ADDR);
> -        stw_p(&resgrp->mui, DEFAULT_MUI);
> -        stw_p(&resgrp->i, 128);
> -        stw_p(&resgrp->maxstbl, 128);
> -        resgrp->version = 0;
>  
> +        ClpReqQueryPciGrp *reqgrp = (ClpReqQueryPciGrp *)reqh;
> +        S390PCIGroup *grp;
> +
> +        grp = s390_grp_find(reqgrp->g);
> +        if (!grp) {
> +            /* We do not allow access to unknown groups */
> +            /* The group must have been obtained with a vfio device */

What about non-vfio devices? How does this whole feature work for
emulated devices?

> +            stw_p(&resgrp->hdr.rsp, CLP_RC_QUERYPCIFG_PFGID);
> +            goto out;
> +        }
> +        memcpy(resgrp, &grp->zpci_grp, sizeof(ClpRspQueryPciGrp));
>          stw_p(&resgrp->hdr.rsp, CLP_RC_OK);
>          break;
>      }
> @@ -752,7 +756,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
>      }
>      /* Length must be greater than 8, a multiple of 8 */
>      /* and not greater than maxstbl */
> -    if ((len <= 8) || (len % 8) || (len > pbdev->maxstbl)) {
> +    if ((len <= 8) || (len % 8) || (len > pbdev->pci_grp->zpci_grp.maxstbl)) {
>          goto specification_error;
>      }
>      /* Do not cross a 4K-byte boundary */