[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v1 3/3] vpci/msi: Remove registers when init_msi() fails


  • To: <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jiqian Chen <Jiqian.Chen@xxxxxxx>
  • Date: Thu, 27 Mar 2025 15:32:14 +0800
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=xYEeEBUWQ7pVi/5cONRapUgnYozVHriILuKaiULbW5w=; b=ctu84xR8wzKV9Ozy6n2QPlXcQb3cVKPALvYOo7cTfUrBukdJmJqgW0FIHdKmmSCRxRbL/nGH57Lbkixm3TKtVF413gMFMkWApTj0+pnxI8QprSejF6Vrre5f4CWPAf0b/M9tF2RQUvvzio+xlAKPwjwoeNDjhUx69vzq6veWX2HhzPVgHbbwHGejhvPbG1J3JAdbmBHdULqnGt/gdKN2NH+yEReSVJlATuaH/Ud1/UYWXHEZgkJ1cz88FQ8TMTwuZ8tc2FGbmBxfk6lBnREma0+9Yjbc9vF3teGTf6AV9e5o2FiyZLHvy41gdYM2k1CQPUegWyzN6e3L74tIcwIKYw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=lxQteUsH5vHrukHWOXL7jnRxfO9wslEmCrC3FAU7C1tE3iKVgmt8rHA7FgdqbI4ddx2kWwK4CzJQnWi9td1RmXZj8OtuiFZw4Vi7xXmUDUqsrjx9MTsdNkYoLVn/bXHfceSPjjz3e59Ywli1pOAeprSegbPlLQKNv/bQI4j0X6No2vgM5WvTjEw5tZO3aQSd6dolMo9rOigUFNsulOvQXlWRd1H6kLXvTEB5ruo4T/YbVsupnaDFDd8LVJauhkGcIMOYHcepCOAXZt7dA66QZd7+CUu/L6cgq3RP2rsLjCihunDvOaBrWPJSCqVj74XuqF9DB0xjVEUbGK5B+9hv3Q==
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Huang Rui <ray.huang@xxxxxxx>, Jiqian Chen <Jiqian.Chen@xxxxxxx>
  • Delivery-date: Thu, 27 Mar 2025 07:33:22 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

When init_msi() fails, the new codes will try to hide MSI
capability, so it can't rely on vpci_deassign_device() to
remove all MSI related registers anymore, those registers
must be cleaned up in failure path of init_msi.

To do that, use a vpci_register array to record all MSI
registers and call vpci_remove_register() to remove registers.

Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
---
 xen/drivers/vpci/msi.c | 57 +++++++++++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 18 deletions(-)

diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
index 9d7a9fd8dba1..30ef97efb7b0 100644
--- a/xen/drivers/vpci/msi.c
+++ b/xen/drivers/vpci/msi.c
@@ -195,6 +195,9 @@ static void cf_check mask_write(
 
 static int cf_check init_msi(struct pci_dev *pdev)
 {
+    unsigned int offset;
+    unsigned int reg_index = 0;
+    struct vpci_register registers[VPCI_CAP_MAX_REGISTER];
     unsigned int pos = pdev->msi_pos;
     uint16_t control;
     int ret;
@@ -206,15 +209,13 @@ static int cf_check init_msi(struct pci_dev *pdev)
     if ( !pdev->vpci->msi )
         return -ENOMEM;
 
+    offset = msi_control_reg(pos);
     ret = vpci_add_register(pdev->vpci, control_read, control_write,
-                            msi_control_reg(pos), 2, pdev->vpci->msi);
+                            offset, 2, pdev->vpci->msi);
     if ( ret )
-        /*
-         * NB: there's no need to free the msi struct or remove the register
-         * handlers form the config space, the caller will take care of the
-         * cleanup.
-         */
-        return ret;
+        goto fail;
+    registers[reg_index].offset = offset;
+    registers[reg_index++].size = 2;
 
     /* Get the maximum number of vectors the device supports. */
     control = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
@@ -234,33 +235,42 @@ static int cf_check init_msi(struct pci_dev *pdev)
     pdev->vpci->msi->address64 = is_64bit_address(control);
     pdev->vpci->msi->masking = is_mask_bit_support(control);
 
+    offset = msi_lower_address_reg(pos);
     ret = vpci_add_register(pdev->vpci, address_read, address_write,
-                            msi_lower_address_reg(pos), 4, pdev->vpci->msi);
+                            offset, 4, pdev->vpci->msi);
     if ( ret )
-        return ret;
+        goto fail;
+    registers[reg_index].offset = offset;
+    registers[reg_index++].size = 4;
 
+    offset = msi_data_reg(pos, pdev->vpci->msi->address64);
     ret = vpci_add_register(pdev->vpci, data_read, data_write,
-                            msi_data_reg(pos, pdev->vpci->msi->address64), 2,
-                            pdev->vpci->msi);
+                            offset, 2, pdev->vpci->msi);
     if ( ret )
-        return ret;
+        goto fail;
+    registers[reg_index].offset = offset;
+    registers[reg_index++].size = 2;
 
     if ( pdev->vpci->msi->address64 )
     {
+        offset = msi_upper_address_reg(pos);
         ret = vpci_add_register(pdev->vpci, address_hi_read, address_hi_write,
-                                msi_upper_address_reg(pos), 4, 
pdev->vpci->msi);
+                                offset, 4, pdev->vpci->msi);
         if ( ret )
-            return ret;
+            goto fail;
+        registers[reg_index].offset = offset;
+        registers[reg_index++].size = 4;
     }
 
     if ( pdev->vpci->msi->masking )
     {
+        offset = msi_mask_bits_reg(pos, pdev->vpci->msi->address64);
         ret = vpci_add_register(pdev->vpci, mask_read, mask_write,
-                                msi_mask_bits_reg(pos,
-                                                  pdev->vpci->msi->address64),
-                                4, pdev->vpci->msi);
+                                offset, 4, pdev->vpci->msi);
         if ( ret )
-            return ret;
+            goto fail;
+        registers[reg_index].offset = offset;
+        registers[reg_index++].size = 4;
         /*
          * FIXME: do not add any handler for the pending bits for the hardware
          * domain, which means direct access. This will be revisited when
@@ -269,6 +279,17 @@ static int cf_check init_msi(struct pci_dev *pdev)
     }
 
     return 0;
+
+ fail:
+    for ( unsigned int i = 0; i < reg_index; i++ )
+        if ( vpci_remove_register(pdev->vpci,
+                                  registers[i].offset,
+                                  registers[i].size) )
+            continue;
+    xfree(pdev->vpci->msi);
+    pdev->vpci->msi = NULL;
+
+    return ret;
 }
 REGISTER_VPCI_LEGACY_CAP(PCI_CAP_ID_MSI, init_msi, VPCI_PRIORITY_LOW);
 
-- 
2.34.1




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.