From 3322ba0d7bea1e24ae464418626f6a15b69533ab Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Thu, 8 Jul 2021 14:55:42 +0200 Subject: s390: make PCI mio support a machine flag Kernel support for the newer PCI mio instructions can be toggled off with the pci=nomio command line option which needs to integrate with common code PCI option parsing. However this option then toggles static branches which can't be toggled yet in an early_param() call. Thus commit 9964f396f1d0 ("s390: fix setting of mio addressing control") moved toggling the static branches to the PCI init routine. With this setup however we can't check for mio support outside the PCI code during early boot, i.e. before switching the static branches, which we need to be able to export this as an ELF HWCAP. Improve on this by turning mio availability into a machine flag that gets initially set based on CONFIG_PCI and the facility bit and gets toggled off if pci=nomio is found during PCI option parsing allowing simple access to this machine flag after early init. Reviewed-by: Heiko Carstens Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens --- arch/s390/pci/pci.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'arch/s390/pci') diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index b0993e05affe..a678b9db56e5 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -886,7 +886,6 @@ static void zpci_mem_exit(void) } static unsigned int s390_pci_probe __initdata = 1; -static unsigned int s390_pci_no_mio __initdata; unsigned int s390_pci_force_floating __initdata; static unsigned int s390_pci_initialized; @@ -897,7 +896,7 @@ char * __init pcibios_setup(char *str) return NULL; } if (!strcmp(str, "nomio")) { - s390_pci_no_mio = 1; + S390_lowcore.machine_flags &= ~MACHINE_FLAG_PCI_MIO; return NULL; } if (!strcmp(str, "force_floating")) { @@ -928,7 +927,7 @@ static int __init pci_base_init(void) return 0; } - if (test_facility(153) && !s390_pci_no_mio) { + if (MACHINE_HAS_PCI_MIO) { static_branch_enable(&have_mio); ctl_set_bit(2, 5); } -- cgit v1.2.3 From 02368b7cf6c7badefa13741aed7a8b91d9a11b19 Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Fri, 6 Aug 2021 10:28:40 +0200 Subject: s390/pci: cleanup resources only if necessary It's currently safe to call zpci_cleanup_bus_resources() even if the resources were never created but it makes no sense so check zdev->has_resources before we call zpci_cleanup_bus_resources() in zpci_release_device(). Reviewed-by: Matthew Rosato Acked-by: Pierre Morel Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens --- arch/s390/pci/pci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'arch/s390/pci') diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index a678b9db56e5..67dc5b1379c1 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -822,7 +822,8 @@ void zpci_release_device(struct kref *kref) case ZPCI_FN_STATE_STANDBY: if (zdev->has_hp_slot) zpci_exit_slot(zdev); - zpci_cleanup_bus_resources(zdev); + if (zdev->has_resources) + zpci_cleanup_bus_resources(zdev); zpci_bus_device_unregister(zdev); zpci_destroy_iommu(zdev); fallthrough; -- cgit v1.2.3 From 81a076171e72dcb6545a8a508b800aec59d6e82b Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Fri, 6 Aug 2021 12:12:11 +0200 Subject: s390/pci: reset zdev->zbus on registration failure On failure to register a struct zpci_dev with a struct zpci_bus we left a dangling pointer in zdev->zbus. As zpci_create_device() bails if zpci_bus_device_register() fails this is of no consequence but still bad practice. Reviewed-by: Matthew Rosato Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens --- arch/s390/pci/pci_bus.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'arch/s390/pci') diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c index 9629f9779c79..3c9ad518712e 100644 --- a/arch/s390/pci/pci_bus.c +++ b/arch/s390/pci/pci_bus.c @@ -343,11 +343,11 @@ static int zpci_bus_add_device(struct zpci_bus *zbus, struct zpci_dev *zdev) { int rc = -EINVAL; - zdev->zbus = zbus; if (zbus->function[zdev->devfn]) { pr_err("devfn %04x is already assigned\n", zdev->devfn); return rc; } + zdev->zbus = zbus; zbus->function[zdev->devfn] = zdev; zpci_nb_devices++; @@ -367,6 +367,7 @@ static int zpci_bus_add_device(struct zpci_bus *zbus, struct zpci_dev *zdev) error: zbus->function[zdev->devfn] = NULL; + zdev->zbus = NULL; zpci_nb_devices--; return rc; } -- cgit v1.2.3 From f7addcdd527a6dddfebe20c358b87bdb95624612 Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Wed, 21 Jul 2021 19:58:54 +0200 Subject: s390/pci: fix misleading rc in clp_set_pci_fn() Currently clp_set_pci_fn() always returns 0 as long as the CLP request itself succeeds even if the operation itself returns a response code other than CLP_RC_OK or CLP_RC_SETPCIFN_ALRDY. This is highly misleading because calling code assumes that a zero rc means that the operation was successful. Fix this by returning the response code or cc on failure with the exception of the special handling for CLP_RC_SETPCIFN_ALRDY. Also let's not assume that the returned function handle for CLP_RC_SETPCIFN_ALRDY is 0, we don't need it anyway. Reviewed-by: Matthew Rosato Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens --- arch/s390/pci/pci.c | 7 ++++--- arch/s390/pci/pci_clp.c | 33 ++++++++++++++++----------------- 2 files changed, 20 insertions(+), 20 deletions(-) (limited to 'arch/s390/pci') diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index 67dc5b1379c1..4eafa8160184 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -655,9 +655,10 @@ int zpci_enable_device(struct zpci_dev *zdev) { int rc; - rc = clp_enable_fh(zdev, ZPCI_NR_DMA_SPACES); - if (rc) + if (clp_enable_fh(zdev, ZPCI_NR_DMA_SPACES)) { + rc = -EIO; goto out; + } rc = zpci_dma_init_device(zdev); if (rc) @@ -678,7 +679,7 @@ int zpci_disable_device(struct zpci_dev *zdev) * The zPCI function may already be disabled by the platform, this is * detected in clp_disable_fh() which becomes a no-op. */ - return clp_disable_fh(zdev); + return clp_disable_fh(zdev) ? -EIO : 0; } /** diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c index d3331596ddbe..0a0e8b8293be 100644 --- a/arch/s390/pci/pci_clp.c +++ b/arch/s390/pci/pci_clp.c @@ -213,15 +213,19 @@ out: } static int clp_refresh_fh(u32 fid); -/* - * Enable/Disable a given PCI function and update its function handle if - * necessary +/** + * clp_set_pci_fn() - Execute a command on a PCI function + * @zdev: Function that will be affected + * @nr_dma_as: DMA address space number + * @command: The command code to execute + * + * Returns: 0 on success, < 0 for Linux errors (e.g. -ENOMEM), and + * > 0 for non-success platform responses */ static int clp_set_pci_fn(struct zpci_dev *zdev, u8 nr_dma_as, u8 command) { struct clp_req_rsp_set_pci *rrb; int rc, retries = 100; - u32 fid = zdev->fid; rrb = clp_alloc_block(GFP_KERNEL); if (!rrb) @@ -245,17 +249,16 @@ static int clp_set_pci_fn(struct zpci_dev *zdev, u8 nr_dma_as, u8 command) } } while (rrb->response.hdr.rsp == CLP_RC_SETPCIFN_BUSY); - if (rc || rrb->response.hdr.rsp != CLP_RC_OK) { - zpci_err("Set PCI FN:\n"); - zpci_err_clp(rrb->response.hdr.rsp, rc); - } - if (!rc && rrb->response.hdr.rsp == CLP_RC_OK) { zdev->fh = rrb->response.fh; - } else if (!rc && rrb->response.hdr.rsp == CLP_RC_SETPCIFN_ALRDY && - rrb->response.fh == 0) { + } else if (!rc && rrb->response.hdr.rsp == CLP_RC_SETPCIFN_ALRDY) { /* Function is already in desired state - update handle */ - rc = clp_refresh_fh(fid); + rc = clp_refresh_fh(zdev->fid); + } else { + zpci_err("Set PCI FN:\n"); + zpci_err_clp(rrb->response.hdr.rsp, rc); + if (!rc) + rc = rrb->response.hdr.rsp; } clp_free_block(rrb); return rc; @@ -301,17 +304,13 @@ int clp_enable_fh(struct zpci_dev *zdev, u8 nr_dma_as) rc = clp_set_pci_fn(zdev, nr_dma_as, CLP_SET_ENABLE_PCI_FN); zpci_dbg(3, "ena fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc); - if (rc) - goto out; - - if (zpci_use_mio(zdev)) { + if (!rc && zpci_use_mio(zdev)) { rc = clp_set_pci_fn(zdev, nr_dma_as, CLP_SET_ENABLE_MIO); zpci_dbg(3, "ena mio fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc); if (rc) clp_disable_fh(zdev); } -out: return rc; } -- cgit v1.2.3 From 8256adda1f44ea1ec763711aefcd25f8c0cf93f3 Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Thu, 22 Jul 2021 12:38:29 +0200 Subject: s390/pci: handle FH state mismatch only on disable Instead of always treating CLP_RC_SETPCIFN_ALRDY as success and blindly updating the function handle restrict this special handling to the disable case by moving it into zpci_disable_device() and still treating it as an error while also updating the function handle such that a subsequent zpci_disable_device() succeeds or the caller can ignore the error when aborting is not an option such as for zPCI event 0x304. Also print this occurrence to the log such that an admin can tell why a disable operation returned an error. A mismatch between the state of the underlying device and our view of it can naturally happen when the device suddenly enters the error state but we haven't gotten the error notification yet, it must not happen on enable though. Reviewed-by: Matthew Rosato Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens --- arch/s390/pci/pci.c | 15 ++++++++++++++- arch/s390/pci/pci_clp.c | 6 +----- 2 files changed, 15 insertions(+), 6 deletions(-) (limited to 'arch/s390/pci') diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index 4eafa8160184..6be5ee320194 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -674,12 +674,25 @@ out: int zpci_disable_device(struct zpci_dev *zdev) { + int cc, rc = 0; + zpci_dma_exit_device(zdev); /* * The zPCI function may already be disabled by the platform, this is * detected in clp_disable_fh() which becomes a no-op. */ - return clp_disable_fh(zdev) ? -EIO : 0; + cc = clp_disable_fh(zdev); + if (cc == CLP_RC_SETPCIFN_ALRDY) { + pr_info("Disabling PCI function %08x had no effect as it was already disabled\n", + zdev->fid); + /* Function is already disabled - update handle */ + rc = clp_refresh_fh(zdev->fid); + if (!rc) + rc = -EINVAL; + } else if (cc) { + rc = -EIO; + } + return rc; } /** diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c index 0a0e8b8293be..04147f28d159 100644 --- a/arch/s390/pci/pci_clp.c +++ b/arch/s390/pci/pci_clp.c @@ -212,7 +212,6 @@ out: return rc; } -static int clp_refresh_fh(u32 fid); /** * clp_set_pci_fn() - Execute a command on a PCI function * @zdev: Function that will be affected @@ -251,9 +250,6 @@ static int clp_set_pci_fn(struct zpci_dev *zdev, u8 nr_dma_as, u8 command) if (!rc && rrb->response.hdr.rsp == CLP_RC_OK) { zdev->fh = rrb->response.fh; - } else if (!rc && rrb->response.hdr.rsp == CLP_RC_SETPCIFN_ALRDY) { - /* Function is already in desired state - update handle */ - rc = clp_refresh_fh(zdev->fid); } else { zpci_err("Set PCI FN:\n"); zpci_err_clp(rrb->response.hdr.rsp, rc); @@ -409,7 +405,7 @@ static void __clp_refresh_fh(struct clp_fh_list_entry *entry, void *data) /* * Refresh the function handle of the function matching @fid */ -static int clp_refresh_fh(u32 fid) +int clp_refresh_fh(u32 fid) { struct clp_req_rsp_list_pci *rrb; int rc; -- cgit v1.2.3 From cc049eecfb7adc4bfecd05eb25e425d8def96fce Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Thu, 22 Jul 2021 11:44:08 +0200 Subject: s390/pci: simplify CLP List PCI handling Currently clp_get_state() and clp_refresh_fh() awkwardly use the clp_list_pci() callback mechanism to find the entry for a specific FID and update its zdev, respectively return its state. This is both needlessly complex and means we are always going through the entire PCI function list even if the FID has already been found. Instead lets introduce a clp_find_pci() function to find a specific entry and share the CLP List PCI request handling code with clp_list_pci(). With that in place we can also easily make the function handle a simple out parameter instead of directly altering the zdev allowing easier access to the updated function handle by the caller. Reviewed-by: Matthew Rosato Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens --- arch/s390/pci/pci.c | 28 +++++---- arch/s390/pci/pci_clp.c | 155 +++++++++++++++++++++++++----------------------- 2 files changed, 99 insertions(+), 84 deletions(-) (limited to 'arch/s390/pci') diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index 6be5ee320194..b05b86e2d2c0 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -653,12 +653,14 @@ void zpci_free_domain(int domain) int zpci_enable_device(struct zpci_dev *zdev) { + u32 fh = zdev->fh; int rc; - if (clp_enable_fh(zdev, ZPCI_NR_DMA_SPACES)) { + if (clp_enable_fh(zdev, &fh, ZPCI_NR_DMA_SPACES)) { rc = -EIO; goto out; } + zdev->fh = fh; rc = zpci_dma_init_device(zdev); if (rc) @@ -667,29 +669,33 @@ int zpci_enable_device(struct zpci_dev *zdev) return 0; out_dma: - clp_disable_fh(zdev); + clp_disable_fh(zdev, &fh); out: + zdev->fh = fh; return rc; } int zpci_disable_device(struct zpci_dev *zdev) { + u32 fh = zdev->fh; int cc, rc = 0; zpci_dma_exit_device(zdev); - /* - * The zPCI function may already be disabled by the platform, this is - * detected in clp_disable_fh() which becomes a no-op. - */ - cc = clp_disable_fh(zdev); - if (cc == CLP_RC_SETPCIFN_ALRDY) { + if (!zdev_enabled(zdev)) + return 0; + cc = clp_disable_fh(zdev, &fh); + if (!cc) { + zdev->fh = fh; + } else if (cc == CLP_RC_SETPCIFN_ALRDY) { pr_info("Disabling PCI function %08x had no effect as it was already disabled\n", zdev->fid); /* Function is already disabled - update handle */ - rc = clp_refresh_fh(zdev->fid); - if (!rc) + rc = clp_refresh_fh(zdev->fid, &fh); + if (!rc) { + zdev->fh = fh; rc = -EINVAL; - } else if (cc) { + } + } else { rc = -EIO; } return rc; diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c index 04147f28d159..51dc2215a2b7 100644 --- a/arch/s390/pci/pci_clp.c +++ b/arch/s390/pci/pci_clp.c @@ -215,17 +215,19 @@ out: /** * clp_set_pci_fn() - Execute a command on a PCI function * @zdev: Function that will be affected + * @fh: Out parameter for updated function handle * @nr_dma_as: DMA address space number * @command: The command code to execute * * Returns: 0 on success, < 0 for Linux errors (e.g. -ENOMEM), and * > 0 for non-success platform responses */ -static int clp_set_pci_fn(struct zpci_dev *zdev, u8 nr_dma_as, u8 command) +static int clp_set_pci_fn(struct zpci_dev *zdev, u32 *fh, u8 nr_dma_as, u8 command) { struct clp_req_rsp_set_pci *rrb; int rc, retries = 100; + *fh = 0; rrb = clp_alloc_block(GFP_KERNEL); if (!rrb) return -ENOMEM; @@ -249,7 +251,7 @@ static int clp_set_pci_fn(struct zpci_dev *zdev, u8 nr_dma_as, u8 command) } while (rrb->response.hdr.rsp == CLP_RC_SETPCIFN_BUSY); if (!rc && rrb->response.hdr.rsp == CLP_RC_OK) { - zdev->fh = rrb->response.fh; + *fh = rrb->response.fh; } else { zpci_err("Set PCI FN:\n"); zpci_err_clp(rrb->response.hdr.rsp, rc); @@ -294,31 +296,62 @@ int clp_setup_writeback_mio(void) return rc; } -int clp_enable_fh(struct zpci_dev *zdev, u8 nr_dma_as) +int clp_enable_fh(struct zpci_dev *zdev, u32 *fh, u8 nr_dma_as) { int rc; - rc = clp_set_pci_fn(zdev, nr_dma_as, CLP_SET_ENABLE_PCI_FN); - zpci_dbg(3, "ena fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc); + rc = clp_set_pci_fn(zdev, fh, nr_dma_as, CLP_SET_ENABLE_PCI_FN); + zpci_dbg(3, "ena fid:%x, fh:%x, rc:%d\n", zdev->fid, *fh, rc); if (!rc && zpci_use_mio(zdev)) { - rc = clp_set_pci_fn(zdev, nr_dma_as, CLP_SET_ENABLE_MIO); + rc = clp_set_pci_fn(zdev, fh, nr_dma_as, CLP_SET_ENABLE_MIO); zpci_dbg(3, "ena mio fid:%x, fh:%x, rc:%d\n", - zdev->fid, zdev->fh, rc); + zdev->fid, *fh, rc); if (rc) - clp_disable_fh(zdev); + clp_disable_fh(zdev, fh); } return rc; } -int clp_disable_fh(struct zpci_dev *zdev) +int clp_disable_fh(struct zpci_dev *zdev, u32 *fh) { int rc; if (!zdev_enabled(zdev)) return 0; - rc = clp_set_pci_fn(zdev, 0, CLP_SET_DISABLE_PCI_FN); - zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, zdev->fh, rc); + rc = clp_set_pci_fn(zdev, fh, 0, CLP_SET_DISABLE_PCI_FN); + zpci_dbg(3, "dis fid:%x, fh:%x, rc:%d\n", zdev->fid, *fh, rc); + return rc; +} + +static int clp_list_pci_req(struct clp_req_rsp_list_pci *rrb, + u64 *resume_token, int *nentries) +{ + int rc; + + memset(rrb, 0, sizeof(*rrb)); + rrb->request.hdr.len = sizeof(rrb->request); + rrb->request.hdr.cmd = CLP_LIST_PCI; + /* store as many entries as possible */ + rrb->response.hdr.len = CLP_BLK_SIZE - LIST_PCI_HDR_LEN; + rrb->request.resume_token = *resume_token; + + /* Get PCI function handle list */ + rc = clp_req(rrb, CLP_LPS_PCI); + if (rc || rrb->response.hdr.rsp != CLP_RC_OK) { + zpci_err("List PCI FN:\n"); + zpci_err_clp(rrb->response.hdr.rsp, rc); + return -EIO; + } + + update_uid_checking(rrb->response.uid_checking); + WARN_ON_ONCE(rrb->response.entry_size != + sizeof(struct clp_fh_list_entry)); + + *nentries = (rrb->response.hdr.len - LIST_PCI_HDR_LEN) / + rrb->response.entry_size; + *resume_token = rrb->response.resume_token; + return rc; } @@ -326,38 +359,40 @@ static int clp_list_pci(struct clp_req_rsp_list_pci *rrb, void *data, void (*cb)(struct clp_fh_list_entry *, void *)) { u64 resume_token = 0; - int entries, i, rc; + int nentries, i, rc; do { - memset(rrb, 0, sizeof(*rrb)); - rrb->request.hdr.len = sizeof(rrb->request); - rrb->request.hdr.cmd = CLP_LIST_PCI; - /* store as many entries as possible */ - rrb->response.hdr.len = CLP_BLK_SIZE - LIST_PCI_HDR_LEN; - rrb->request.resume_token = resume_token; - - /* Get PCI function handle list */ - rc = clp_req(rrb, CLP_LPS_PCI); - if (rc || rrb->response.hdr.rsp != CLP_RC_OK) { - zpci_err("List PCI FN:\n"); - zpci_err_clp(rrb->response.hdr.rsp, rc); - rc = -EIO; - goto out; - } + rc = clp_list_pci_req(rrb, &resume_token, &nentries); + if (rc) + return rc; + for (i = 0; i < nentries; i++) + cb(&rrb->response.fh_list[i], data); + } while (resume_token); - update_uid_checking(rrb->response.uid_checking); - WARN_ON_ONCE(rrb->response.entry_size != - sizeof(struct clp_fh_list_entry)); + return rc; +} - entries = (rrb->response.hdr.len - LIST_PCI_HDR_LEN) / - rrb->response.entry_size; +static int clp_find_pci(struct clp_req_rsp_list_pci *rrb, u32 fid, + struct clp_fh_list_entry *entry) +{ + struct clp_fh_list_entry *fh_list; + u64 resume_token = 0; + int nentries, i, rc; - resume_token = rrb->response.resume_token; - for (i = 0; i < entries; i++) - cb(&rrb->response.fh_list[i], data); + do { + rc = clp_list_pci_req(rrb, &resume_token, &nentries); + if (rc) + return rc; + for (i = 0; i < nentries; i++) { + fh_list = rrb->response.fh_list; + if (fh_list[i].fid == fid) { + *entry = fh_list[i]; + return 0; + } + } } while (resume_token); -out: - return rc; + + return -ENODEV; } static void __clp_add(struct clp_fh_list_entry *entry, void *data) @@ -387,67 +422,41 @@ int clp_scan_pci_devices(void) return rc; } -static void __clp_refresh_fh(struct clp_fh_list_entry *entry, void *data) -{ - struct zpci_dev *zdev; - u32 fid = *((u32 *)data); - - if (!entry->vendor_id || fid != entry->fid) - return; - - zdev = get_zdev_by_fid(fid); - if (!zdev) - return; - - zdev->fh = entry->fh; -} - /* - * Refresh the function handle of the function matching @fid + * Get the current function handle of the function matching @fid */ -int clp_refresh_fh(u32 fid) +int clp_refresh_fh(u32 fid, u32 *fh) { struct clp_req_rsp_list_pci *rrb; + struct clp_fh_list_entry entry; int rc; rrb = clp_alloc_block(GFP_NOWAIT); if (!rrb) return -ENOMEM; - rc = clp_list_pci(rrb, &fid, __clp_refresh_fh); + rc = clp_find_pci(rrb, fid, &entry); + if (!rc) + *fh = entry.fh; clp_free_block(rrb); return rc; } -struct clp_state_data { - u32 fid; - enum zpci_state state; -}; - -static void __clp_get_state(struct clp_fh_list_entry *entry, void *data) -{ - struct clp_state_data *sd = data; - - if (entry->fid != sd->fid) - return; - - sd->state = entry->config_state; -} - int clp_get_state(u32 fid, enum zpci_state *state) { struct clp_req_rsp_list_pci *rrb; - struct clp_state_data sd = {fid, ZPCI_FN_STATE_RESERVED}; + struct clp_fh_list_entry entry; int rc; + *state = ZPCI_FN_STATE_RESERVED; rrb = clp_alloc_block(GFP_ATOMIC); if (!rrb) return -ENOMEM; - rc = clp_list_pci(rrb, &sd, __clp_get_state); + rc = clp_find_pci(rrb, fid, &entry); if (!rc) - *state = sd.state; + *state = entry.config_state; clp_free_block(rrb); return rc; -- cgit v1.2.3 From 1f3f76812d5dfc791193b39c2140a8bd09962c0e Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Fri, 16 Jul 2021 11:53:37 +0200 Subject: s390/pci: improve DMA translation init and exit Currently zpci_dma_init_device()/zpci_dma_exit_device() is called as part of zpci_enable_device()/zpci_disable_device() and errors for zpci_dma_exit_device() are always ignored even if we could abort. Improve upon this by moving zpci_dma_exit_device() out of zpci_disable_device() and check for errors whenever we have a way to abort the current operation. Note that for example in zpci_event_hard_deconfigured() the device is expected to be gone so we really can't abort and proceed even in case of error. Similarly move the cc == 3 special case out of zpci_unregister_ioat() and into the callers allowing to abort when finding an already disabled devices precludes proceeding with the operation. While we are at it log IOAT register/unregister errors in the s390 debugfs log, Reviewed-by: Matthew Rosato Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens --- arch/s390/pci/pci.c | 43 +++++++++++++++++++------------------------ arch/s390/pci/pci_bus.c | 5 +++++ arch/s390/pci/pci_dma.c | 25 +++++++++++++++++-------- arch/s390/pci/pci_event.c | 5 ++++- arch/s390/pci/pci_sysfs.c | 19 ++++++++++++++++--- 5 files changed, 61 insertions(+), 36 deletions(-) (limited to 'arch/s390/pci') diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index b05b86e2d2c0..728508da999d 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -113,13 +113,16 @@ int zpci_register_ioat(struct zpci_dev *zdev, u8 dmaas, { u64 req = ZPCI_CREATE_REQ(zdev->fh, dmaas, ZPCI_MOD_FC_REG_IOAT); struct zpci_fib fib = {0}; - u8 status; + u8 cc, status; WARN_ON_ONCE(iota & 0x3fff); fib.pba = base; fib.pal = limit; fib.iota = iota | ZPCI_IOTA_RTTO_FLAG; - return zpci_mod_fc(req, &fib, &status) ? -EIO : 0; + cc = zpci_mod_fc(req, &fib, &status); + if (cc) + zpci_dbg(3, "reg ioat fid:%x, cc:%d, status:%d\n", zdev->fid, cc, status); + return cc; } /* Modify PCI: Unregister I/O address translation parameters */ @@ -130,9 +133,9 @@ int zpci_unregister_ioat(struct zpci_dev *zdev, u8 dmaas) u8 cc, status; cc = zpci_mod_fc(req, &fib, &status); - if (cc == 3) /* Function already gone. */ - cc = 0; - return cc ? -EIO : 0; + if (cc) + zpci_dbg(3, "unreg ioat fid:%x, cc:%d, status:%d\n", zdev->fid, cc, status); + return cc; } /* Modify PCI: Set PCI function measurement parameters */ @@ -654,24 +657,12 @@ void zpci_free_domain(int domain) int zpci_enable_device(struct zpci_dev *zdev) { u32 fh = zdev->fh; - int rc; + int rc = 0; - if (clp_enable_fh(zdev, &fh, ZPCI_NR_DMA_SPACES)) { + if (clp_enable_fh(zdev, &fh, ZPCI_NR_DMA_SPACES)) rc = -EIO; - goto out; - } - zdev->fh = fh; - - rc = zpci_dma_init_device(zdev); - if (rc) - goto out_dma; - - return 0; - -out_dma: - clp_disable_fh(zdev, &fh); -out: - zdev->fh = fh; + else + zdev->fh = fh; return rc; } @@ -680,9 +671,6 @@ int zpci_disable_device(struct zpci_dev *zdev) u32 fh = zdev->fh; int cc, rc = 0; - zpci_dma_exit_device(zdev); - if (!zdev_enabled(zdev)) - return 0; cc = clp_disable_fh(zdev, &fh); if (!cc) { zdev->fh = fh; @@ -808,6 +796,11 @@ int zpci_deconfigure_device(struct zpci_dev *zdev) if (zdev->zbus->bus) zpci_bus_remove_device(zdev, false); + if (zdev->dma_table) { + rc = zpci_dma_exit_device(zdev); + if (rc) + return rc; + } if (zdev_enabled(zdev)) { rc = zpci_disable_device(zdev); if (rc) @@ -831,6 +824,8 @@ void zpci_release_device(struct kref *kref) if (zdev->zbus->bus) zpci_bus_remove_device(zdev, false); + if (zdev->dma_table) + zpci_dma_exit_device(zdev); if (zdev_enabled(zdev)) zpci_disable_device(zdev); diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c index 3c9ad518712e..5d77acbd1c87 100644 --- a/arch/s390/pci/pci_bus.c +++ b/arch/s390/pci/pci_bus.c @@ -49,6 +49,11 @@ static int zpci_bus_prepare_device(struct zpci_dev *zdev) rc = zpci_enable_device(zdev); if (rc) return rc; + rc = zpci_dma_init_device(zdev); + if (rc) { + zpci_disable_device(zdev); + return rc; + } } if (!zdev->has_resources) { diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c index ebc9a49523aa..58f2f7abea96 100644 --- a/arch/s390/pci/pci_dma.c +++ b/arch/s390/pci/pci_dma.c @@ -590,10 +590,11 @@ int zpci_dma_init_device(struct zpci_dev *zdev) } } - rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, - (u64) zdev->dma_table); - if (rc) + if (zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, + (u64)zdev->dma_table)) { + rc = -EIO; goto free_bitmap; + } return 0; free_bitmap: @@ -608,17 +609,25 @@ out: return rc; } -void zpci_dma_exit_device(struct zpci_dev *zdev) +int zpci_dma_exit_device(struct zpci_dev *zdev) { + int cc = 0; + /* * At this point, if the device is part of an IOMMU domain, this would * be a strong hint towards a bug in the IOMMU API (common) code and/or * simultaneous access via IOMMU and DMA API. So let's issue a warning. */ WARN_ON(zdev->s390_domain); - - if (zpci_unregister_ioat(zdev, 0)) - return; + if (zdev_enabled(zdev)) + cc = zpci_unregister_ioat(zdev, 0); + /* + * cc == 3 indicates the function is gone already. This can happen + * if the function was deconfigured/disabled suddenly and we have not + * received a new handle yet. + */ + if (cc && cc != 3) + return -EIO; dma_cleanup_tables(zdev->dma_table); zdev->dma_table = NULL; @@ -626,8 +635,8 @@ void zpci_dma_exit_device(struct zpci_dev *zdev) zdev->iommu_bitmap = NULL; vfree(zdev->lazy_bitmap); zdev->lazy_bitmap = NULL; - zdev->next_bit = 0; + return 0; } static int __init dma_alloc_cpu_table_caches(void) diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c index cd447b96b4b1..c856f80cb21b 100644 --- a/arch/s390/pci/pci_event.c +++ b/arch/s390/pci/pci_event.c @@ -84,7 +84,10 @@ static void zpci_event_hard_deconfigured(struct zpci_dev *zdev, u32 fh) /* Even though the device is already gone we still * need to free zPCI resources as part of the disable. */ - zpci_disable_device(zdev); + if (zdev->dma_table) + zpci_dma_exit_device(zdev); + if (zdev_enabled(zdev)) + zpci_disable_device(zdev); zdev->state = ZPCI_FN_STATE_STANDBY; } diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c index 6e2450c2b9c1..335c281811c7 100644 --- a/arch/s390/pci/pci_sysfs.c +++ b/arch/s390/pci/pci_sysfs.c @@ -82,13 +82,26 @@ static ssize_t recover_store(struct device *dev, struct device_attribute *attr, pci_lock_rescan_remove(); if (pci_dev_is_added(pdev)) { pci_stop_and_remove_bus_device(pdev); - ret = zpci_disable_device(zdev); - if (ret) - goto out; + if (zdev->dma_table) { + ret = zpci_dma_exit_device(zdev); + if (ret) + goto out; + } + + if (zdev_enabled(zdev)) { + ret = zpci_disable_device(zdev); + if (ret) + goto out; + } ret = zpci_enable_device(zdev); if (ret) goto out; + ret = zpci_dma_init_device(zdev); + if (ret) { + zpci_disable_device(zdev); + goto out; + } pci_rescan_bus(zdev->zbus->bus); } out: -- cgit v1.2.3