Re: [PATCH 1/4] s390: pci: Exporting access to CLP PCI function and PCI group

Robin Murphy
2019-05-10T10:21:33Z

On 10/05/2019 09:22, Pierre Morel wrote:
> For the generic implementation of VFIO PCI we need to retrieve
> the hardware configuration for the PCI functions and the
> PCI function groups.
>
> We modify the internal function using CLP Query PCI function and
> CLP query PCI function group so that they can be called from
> outside the S390 architecture PCI code and prefix the two
> functions with "zdev" to make clear that they can be called
> knowing only the associated zdevice.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> Reviewed-by: Sebastian Ott <sebott@linux.ibm.com>
> ---
> arch/s390/include/asm/pci.h | 3 ++
> arch/s390/pci/pci_clp.c | 72 ++++++++++++++++++++++++---------------------
> 2 files changed, 41 insertions(+), 34 deletions(-)
>
> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> index 305befd..e66b246 100644
> --- a/arch/s390/include/asm/pci.h
> +++ b/arch/s390/include/asm/pci.h
> @@ -261,4 +261,7 @@ cpumask_of_pcibus(const struct pci_bus *bus)
>
> #endif /* CONFIG_NUMA */
>
> +int zdev_query_pci_fngrp(struct zpci_dev *zdev,
> + struct clp_req_rsp_query_pci_grp *rrb);
> +int zdev_query_pci_fn(struct zpci_dev *zdev, struct clp_req_rsp_query_pci *rrb);
> #endif
> diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
> index 3a36b07..4ae5d77 100644
> --- a/arch/s390/pci/pci_clp.c
> +++ b/arch/s390/pci/pci_clp.c
> @@ -113,32 +113,18 @@ static void clp_store_query_pci_fngrp(struct zpci_dev *zdev,
> }
> }
>
> -static int clp_query_pci_fngrp(struct zpci_dev *zdev, u8 pfgid)
> +int zdev_query_pci_fngrp(struct zpci_dev *zdev,
> + struct clp_req_rsp_query_pci_grp *rrb)
> {
> - struct clp_req_rsp_query_pci_grp *rrb;
> - int rc;
> -
> - rrb = clp_alloc_block(GFP_KERNEL);
> - if (!rrb)
> - return -ENOMEM;
> -
> memset(rrb, 0, sizeof(*rrb));
> rrb->request.hdr.len = sizeof(rrb->request);
> rrb->request.hdr.cmd = CLP_QUERY_PCI_FNGRP;
> rrb->response.hdr.len = sizeof(rrb->response);
> - rrb->request.pfgid = pfgid;
> + rrb->request.pfgid = zdev->pfgid;
>
> - rc = clp_req(rrb, CLP_LPS_PCI);
> - if (!rc && rrb->response.hdr.rsp == CLP_RC_OK)
> - clp_store_query_pci_fngrp(zdev, &rrb->response);
> - else {
> - zpci_err("Q PCI FGRP:\n");
> - zpci_err_clp(rrb->response.hdr.rsp, rc);
> - rc = -EIO;
> - }
> - clp_free_block(rrb);
> - return rc;
> + return clp_req(rrb, CLP_LPS_PCI);
> }
> +EXPORT_SYMBOL(zdev_query_pci_fngrp);

AFAICS it's only the IOMMU driver itself which needs to call these. That
can't be built as a module, so you shouldn't need explicit exports - the
header declaration is enough.

Robin.

> static int clp_store_query_pci_fn(struct zpci_dev *zdev,
> struct clp_rsp_query_pci *response)
> @@ -174,32 +160,50 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev,
> return 0;
> }
>
> -static int clp_query_pci_fn(struct zpci_dev *zdev, u32 fh)
> +int zdev_query_pci_fn(struct zpci_dev *zdev, struct clp_req_rsp_query_pci *rrb)
> +{
> +
> + memset(rrb, 0, sizeof(*rrb));
> + rrb->request.hdr.len = sizeof(rrb->request);
> + rrb->request.hdr.cmd = CLP_QUERY_PCI_FN;
> + rrb->response.hdr.len = sizeof(rrb->response);
> + rrb->request.fh = zdev->fh;
> +
> + return clp_req(rrb, CLP_LPS_PCI);
> +}
> +EXPORT_SYMBOL(zdev_query_pci_fn);
> +
> +static int clp_query_pci(struct zpci_dev *zdev)
> {
> struct clp_req_rsp_query_pci *rrb;
> + struct clp_req_rsp_query_pci_grp *grrb;
> int rc;
>
> rrb = clp_alloc_block(GFP_KERNEL);
> if (!rrb)
> return -ENOMEM;
>
> - memset(rrb, 0, sizeof(*rrb));
> - rrb->request.hdr.len = sizeof(rrb->request);
> - rrb->request.hdr.cmd = CLP_QUERY_PCI_FN;
> - rrb->response.hdr.len = sizeof(rrb->response);
> - rrb->request.fh = fh;
> -
> - rc = clp_req(rrb, CLP_LPS_PCI);
> - if (!rc && rrb->response.hdr.rsp == CLP_RC_OK) {
> - rc = clp_store_query_pci_fn(zdev, &rrb->response);
> - if (rc)
> - goto out;
> - rc = clp_query_pci_fngrp(zdev, rrb->response.pfgid);
> - } else {
> + rc = zdev_query_pci_fn(zdev, rrb);
> + if (rc || rrb->response.hdr.rsp != CLP_RC_OK) {
> zpci_err("Q PCI FN:\n");
> zpci_err_clp(rrb->response.hdr.rsp, rc);
> rc = -EIO;
> + goto out;
> }
> + rc = clp_store_query_pci_fn(zdev, &rrb->response);
> + if (rc)
> + goto out;
> +
> + grrb = (struct clp_req_rsp_query_pci_grp *)rrb;
> + rc = zdev_query_pci_fngrp(zdev, grrb);
> + if (rc || grrb->response.hdr.rsp != CLP_RC_OK) {
> + zpci_err("Q PCI FGRP:\n");
> + zpci_err_clp(grrb->response.hdr.rsp, rc);
> + rc = -EIO;
> + goto out;
> + }
> + clp_store_query_pci_fngrp(zdev, &grrb->response);
> +
> out:
> clp_free_block(rrb);
> return rc;
> @@ -219,7 +223,7 @@ int clp_add_pci_device(u32 fid, u32 fh, int configured)
> zdev->fid = fid;
>
> /* Query function properties and update zdev */
> - rc = clp_query_pci_fn(zdev, fh);
> + rc = clp_query_pci(zdev);
> if (rc)
> goto error;
>
>