[RFC,v2,2/2] xen/vpci: header: avoid cast for value passed to vpci_read_val

Commit Message

Stewart Hildebrand Aug. 16, 2023, 6:50 p.m. UTC

The only purpose of this is to give an idea of what it might look like to
introduce a new memory allocation in order to get rid of the casts for the value
passed to vpci_read_val. If this is deemed preferable vs the casts, I will
squash it for the next version of the series.

Signed-off-by: Stewart Hildebrand <[e-mail]>
---
v1->v2:
* new patch
---
 xen/drivers/vpci/header.c | 13 ++++++++++---
 xen/drivers/vpci/vpci.c   |  8 +++++++-
 xen/include/xen/vpci.h    |  3 +++
 3 files changed, 20 insertions(+), 4 deletions(-)

Comments

Jan Beulich Aug. 17, 2023, 7:29 a.m. UTC | #1

On 16.08.2023 20:50, Stewart Hildebrand wrote:
> The only purpose of this is to give an idea of what it might look like to
> introduce a new memory allocation in order to get rid of the casts for the value
> passed to vpci_read_val. If this is deemed preferable vs the casts, I will
> squash it for the next version of the series.

I would say this isn't worth it, but it'll be Roger to ultimately judge.

Jan

Patch

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index ec5947300198..ae269a0b4bbc 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -571,14 +571,19 @@  static int cf_check init_bars(struct pci_dev *pdev)
         {
             /* Only expose capabilities to the guest that vPCI can handle. */
             uint8_t next;
-            int ttl = 48;
+            int ttl = 48, i = 0;
+
+            header->caps_list = xzalloc_array(uint8_t, ttl);
+            if ( !header->caps_list )
+                return -ENOMEM;
 
             next = pci_find_next_cap(pdev->sbdf, PCI_CAPABILITY_LIST,
                                      vpci_cap_supported, &ttl);
 
+            header->caps_list[i] = next;
             rc = vpci_add_register(pdev->vpci, vpci_read_val, NULL,
                                    PCI_CAPABILITY_LIST, 1,
-                                   (void *)(uintptr_t)next);
+                                   &header->caps_list[i]);
             if ( rc )
                 return rc;
 
@@ -594,9 +599,11 @@  static int cf_check init_bars(struct pci_dev *pdev)
                 if ( rc )
                     return rc;
 
+                i++;
+                header->caps_list[i] = next;
                 rc = vpci_add_register(pdev->vpci, vpci_read_val, NULL,
                                        pos + PCI_CAP_LIST_NEXT, 1,
-                                       (void *)(uintptr_t)next);
+                                       &header->caps_list[i]);
                 if ( rc )
                     return rc;
 
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 4a96aa50494d..717340b8fbe8 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -61,6 +61,7 @@  void vpci_remove_device(struct pci_dev *pdev)
             if ( pdev->vpci->msix->table[i] )
                 iounmap(pdev->vpci->msix->table[i]);
     }
+    xfree(pdev->vpci->header.caps_list);
     xfree(pdev->vpci->msix);
     xfree(pdev->vpci->msi);
     xfree(pdev->vpci);
@@ -136,7 +137,12 @@  static void cf_check vpci_ignored_write(
 uint32_t cf_check vpci_read_val(
     const struct pci_dev *pdev, unsigned int reg, void *data)
 {
-    return (uintptr_t)data;
+    uint8_t *val = data;
+
+    if ( val )
+        return *val;
+
+    return 0;
 }
 
 uint32_t cf_check vpci_hw_read8(
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 17fd252746ec..7cdee84b5123 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -88,6 +88,9 @@  struct vpci {
         } bars[PCI_HEADER_NORMAL_NR_BARS + 1];
         /* At most 6 BARS + 1 expansion ROM BAR. */
 
+        /* Guest view of capabilities next pointers. */
+        uint8_t *caps_list;
+
         /*
          * Store whether the ROM enable bit is set (doesn't imply ROM BAR
          * is mapped into guest p2m) if there's a ROM BAR on the device.