re-work API for adding capabilities (#200)

Allow to add capabilities individually, including extended capabilities, and
those to be handled via the region callback.

As a side effect, rework config space accesses to handle reads that straddle
capabilities and non-standard areas and use callbacks as needed.

Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>

2 files changed, 136 insertions, 52 deletions

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index cf7c5e3..50f58f2 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -27,12 +27,12 @@
 #
 
 add_executable(unit-tests unit-tests.c mocks.c
-		../lib/cap.c
 		../lib/dma.c
 		../lib/irq.c
 		../lib/libvfio-user.c
 		../lib/migration.c
 		../lib/pci.c
+		../lib/pci_caps.c
 		../lib/tran_sock.c)
 
 target_link_libraries(unit-tests PUBLIC cmocka json-c)
diff --git a/test/unit-tests.c b/test/unit-tests.c
index 2a62a3a..c03a37a 100644
--- a/test/unit-tests.c
+++ b/test/unit-tests.c
@@ -42,10 +42,10 @@
 
 #include "dma.h"
 #include "libvfio-user.h"
+#include "pci.h"
 #include "private.h"
 #include "migration.h"
 #include "tran_sock.h"
-#include "cap.h"
 
 static void
 test_dma_map_without_dma(void **state __attribute__((unused)))
@@ -389,7 +389,7 @@ test_realize_ctx(void **state __attribute__((unused)))
     assert_int_equal(PCI_CFG_SPACE_SIZE, cfg_reg->size);
     assert_non_null(vfu_ctx.pci.config_space);
     assert_non_null(vfu_ctx.irqs);
-    assert_null(vfu_ctx.pci.caps);
+    assert_int_equal(0, vfu_ctx.pci.nr_caps);
 }
 
 static int
@@ -531,8 +531,10 @@ static void
 test_vfu_ctx_create(void **state __attribute__((unused)))
 {
     vfu_ctx_t *vfu_ctx = NULL;
-    vfu_cap_t pm = {.pm = {.hdr.id = PCI_CAP_ID_PM}};
-    vfu_cap_t *caps[] = { &pm };
+    struct pmcap pm = { { 0 } };
+
+    pm.hdr.id = PCI_CAP_ID_PM;
+    pm.pmcs.nsfrst = 0x1;
 
     vfu_ctx = vfu_create_ctx(VFU_TRANS_SOCK, "", LIBVFIO_USER_FLAG_ATTACH_NB,
                              NULL, VFU_DEV_TYPE_PCI);
@@ -541,10 +543,26 @@ test_vfu_ctx_create(void **state __attribute__((unused)))
     assert_int_equal(1, vfu_ctx->irq_count[VFU_DEV_REQ_IRQ]);
     assert_int_equal(0, vfu_pci_init(vfu_ctx, VFU_PCI_TYPE_CONVENTIONAL,
                         PCI_HEADER_TYPE_NORMAL, 0));
-    assert_int_equal(0, vfu_pci_setup_caps(vfu_ctx, caps, 1));
+    assert_int_equal(PCI_STD_HEADER_SIZEOF,
+                     vfu_pci_add_capability(vfu_ctx, 0, 0, &pm));
     assert_int_equal(0, vfu_realize_ctx(vfu_ctx));
 }
 
+static ssize_t
+test_pci_caps_region_cb(vfu_ctx_t *vfu_ctx, char *buf, size_t count,
+                        loff_t offset, bool is_write)
+{
+    uint8_t *ptr = pci_config_space_ptr(vfu_ctx, offset);
+
+    assert_int_equal(is_write, true);
+    assert_int_equal(offset, PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF + 8 +
+                             offsetof(struct vsc, data));
+    assert_int_equal(count, 10);
+    assert_memory_equal(ptr, "Hello world.", 10);
+    memcpy(ptr, buf, count);
+    return count;
+}
+
 static void
 test_pci_caps(void **state __attribute__((unused)))
 {
@@ -555,35 +573,72 @@ test_pci_caps(void **state __attribute__((unused)))
     vfu_ctx_t vfu_ctx = { .pci.config_space = &config_space,
                           .reg_info = reg_info,
     };
-    vfu_cap_t pm = {.pm = {.hdr.id = PCI_CAP_ID_PM, .pmcs.raw = 0xabcd }};
-    vfu_cap_t *vsc[2] = {
-        alloca(sizeof(struct vsc) + 5),
-        alloca(sizeof(struct vsc) + 13)
+    struct vsc *vsc1 = alloca(sizeof (*vsc1) + 3);
+    struct vsc *vsc2 = alloca(sizeof (*vsc2) + 13);
+    struct vsc *vsc3 = alloca(sizeof (*vsc3) + 13);
+    struct vsc *vsc4 = alloca(sizeof (*vsc4) + 13);
+    struct pmcap pm = { { 0 } };
+    size_t expoffsets[] = {
+        PCI_STD_HEADER_SIZEOF,
+        PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF,
+        /* NB: note 4-byte alignment of vsc2. */
+        PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF + 8,
+        0x80,
+        0x90
     };
-    vfu_cap_t *vfu_caps[] = { &pm, vsc[0], vsc[1] };
-    struct caps *caps;
-    int err;
-    struct pmcap pmcap = { .pmcs.raw = 0xef01 };
     size_t offset;
-    off_t off;
+    ssize_t ret;
+
+    memset(&config_space, 0, sizeof (config_space));
+
+    vfu_ctx.reg_info = calloc(VFU_PCI_DEV_NUM_REGIONS,
+                              sizeof (*vfu_ctx.reg_info));
+
+    pm.hdr.id = PCI_CAP_ID_PM;
+    pm.pmcs.raw = 0xef01;
+
+    vsc1->hdr.id = PCI_CAP_ID_VNDR;
+    vsc1->size = 6;
+    memcpy(vsc1->data, "abc", 3);
+
+    vsc2->hdr.id = PCI_CAP_ID_VNDR;
+    vsc2->size = 16;
+    memcpy(vsc2->data, "Hello world.", 12);
 
-    vsc[0]->vsc.hdr.id = PCI_CAP_ID_VNDR;
-    vsc[0]->vsc.size = 8;
-    memcpy(vsc[0]->vsc.data, "abcde", 5);
+    vsc3->hdr.id = PCI_CAP_ID_VNDR;
+    vsc3->size = 16;
+    memcpy(vsc3->data, "Hello world.", 12);
 
-    vsc[1]->vsc.hdr.id = PCI_CAP_ID_VNDR;
-    vsc[1]->vsc.size = 16;
-    memcpy(vsc[1]->vsc.data, "Hello world.", 12);
+    vsc4->hdr.id = PCI_CAP_ID_VNDR;
+    vsc4->size = 16;
+    memcpy(vsc4->data, "Hello world.", 12);
 
-    caps = caps_create(&vfu_ctx, vfu_caps, 3, &err);
-    assert_non_null(caps);
+    offset = vfu_pci_add_capability(&vfu_ctx, 0, VFU_CAP_FLAG_CALLBACK, &pm);
+    assert_int_equal(-1, offset);
+    assert_int_equal(EINVAL, errno);
+
+    offset = vfu_pci_add_capability(&vfu_ctx, 256, 0, &pm);
+    assert_int_equal(-1, offset);
+    assert_int_equal(EINVAL, errno);
 
-    /* check that capability list is placed correctly */
+    vfu_ctx.reg_info[VFU_PCI_DEV_CFG_REGION_IDX].cb = test_pci_caps_region_cb;
+    vfu_ctx.reg_info[VFU_PCI_DEV_CFG_REGION_IDX].size = PCI_CFG_SPACE_SIZE;
+
+    offset = vfu_pci_add_capability(&vfu_ctx, 0, 0, &pm);
+    assert_int_equal(expoffsets[0], offset);
+    offset = vfu_pci_add_capability(&vfu_ctx, 0, VFU_CAP_FLAG_READONLY, vsc1);
+    assert_int_equal(expoffsets[1], offset);
+    offset = vfu_pci_add_capability(&vfu_ctx, 0, VFU_CAP_FLAG_CALLBACK, vsc2);
+    assert_int_equal(expoffsets[2], offset);
+    offset = vfu_pci_add_capability(&vfu_ctx, expoffsets[3], 0, vsc3);
+    assert_int_equal(expoffsets[3], offset);
+    offset = vfu_pci_add_capability(&vfu_ctx, expoffsets[4], 0, vsc4);
+    assert_int_equal(expoffsets[4], offset);
 
     offset = vfu_pci_find_capability(&vfu_ctx, false, PCI_CAP_ID_PM);
-    assert_int_equal(PCI_STD_HEADER_SIZEOF, offset);
+    assert_int_equal(expoffsets[0], offset);
     assert_int_equal(PCI_CAP_ID_PM, config_space.raw[offset]);
-    assert_int_equal(PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF,
+    assert_int_equal(expoffsets[1],
                      config_space.raw[offset + PCI_CAP_LIST_NEXT]);
 
     offset = vfu_pci_find_next_capability(&vfu_ctx, false, offset,
@@ -591,31 +646,40 @@ test_pci_caps(void **state __attribute__((unused)))
     assert_int_equal(0, offset);
 
     offset = vfu_pci_find_capability(&vfu_ctx, false, PCI_CAP_ID_VNDR);
-    assert_int_equal(PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF, offset);
+    assert_int_equal(expoffsets[1], offset);
     assert_int_equal(PCI_CAP_ID_VNDR, config_space.raw[offset]);
+    assert_int_equal(expoffsets[2],
+                     config_space.raw[offset + PCI_CAP_LIST_NEXT]);
 
     offset = vfu_pci_find_next_capability(&vfu_ctx, false, offset,
                                           PCI_CAP_ID_PM);
     assert_int_equal(0, offset);
 
     offset = vfu_pci_find_next_capability(&vfu_ctx, false, 0, PCI_CAP_ID_VNDR);
-    assert_int_equal(PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF, offset);
+    assert_int_equal(expoffsets[1], offset);
     assert_int_equal(PCI_CAP_ID_VNDR, config_space.raw[offset]);
-    assert_int_equal(PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF + vsc[0]->vsc.size,
+    assert_int_equal(expoffsets[2],
                      config_space.raw[offset + PCI_CAP_LIST_NEXT]);
 
-    offset = vfu_pci_find_next_capability(&vfu_ctx, false, offset,
-                                          PCI_CAP_ID_VNDR);
-    assert_int_equal(PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF + vsc[0]->vsc.size,
-                     offset);
+    offset = vfu_pci_find_next_capability(&vfu_ctx, false,
+                                          offset, PCI_CAP_ID_VNDR);
+    assert_int_equal(expoffsets[2], offset);
     assert_int_equal(PCI_CAP_ID_VNDR, config_space.raw[offset]);
-    assert_int_equal(0, config_space.raw[offset + PCI_CAP_LIST_NEXT]);
+    assert_int_equal(expoffsets[3],
+                     config_space.raw[offset + PCI_CAP_LIST_NEXT]);
 
-    offset = vfu_pci_find_next_capability(&vfu_ctx, false, offset,
-                                          PCI_CAP_ID_VNDR);
+    offset = vfu_pci_find_next_capability(&vfu_ctx, false,
+                                          offset, PCI_CAP_ID_VNDR);
+    assert_int_equal(expoffsets[3], offset);
+    offset = vfu_pci_find_next_capability(&vfu_ctx, false,
+                                          offset, PCI_CAP_ID_VNDR);
+    assert_int_equal(expoffsets[4], offset);
+    offset = vfu_pci_find_next_capability(&vfu_ctx, false,
+                                          offset, PCI_CAP_ID_VNDR);
     assert_int_equal(0, offset);
 
     /* check for invalid offsets */
+
     offset = vfu_pci_find_next_capability(&vfu_ctx, false, 8192, PCI_CAP_ID_PM);
     assert_int_equal(0, offset);
     assert_int_equal(EINVAL, errno);
@@ -634,22 +698,42 @@ test_pci_caps(void **state __attribute__((unused)))
     assert_int_equal(ENOENT, errno);
 
     /* check writing PMCS */
-    assert_int_equal(0,
-        cap_maybe_access(&vfu_ctx, caps, (char*)&pmcap.pmcs,
-                         sizeof(struct pmcs),
-                         PCI_STD_HEADER_SIZEOF + offsetof(struct pmcap, pmcs)));
-    assert_memory_equal(
-        &config_space.raw[PCI_STD_HEADER_SIZEOF + offsetof(struct pmcap, pmcs)],
-        &pmcap.pmcs.raw, sizeof(struct pmcs));
-
-    /*
-     * Check that pci_cap_access returns 0 when reading a non-vendor-specific
-     * capability which doesn't have a callback.
-     */
-    off = PCI_STD_HEADER_SIZEOF + PCI_PM_SIZEOF + PCI_CAP_FLAGS + 1;
-    assert_int_equal(5,
-        cap_maybe_access(&vfu_ctx, caps, (char*)vsc[0]->vsc.data, 5, off));
-    assert_memory_equal(&config_space.raw[off], vsc[0]->vsc.data, 5);
+
+    pm.pmcs.raw = 0xffff;
+
+    ret = pci_config_space_access(&vfu_ctx, (char *)&pm.pmcs,
+                                  sizeof (struct pmcs), expoffsets[0] +
+                                  offsetof(struct pmcap, pmcs), true);
+
+    assert_int_equal(sizeof (struct pmcs), ret);
+
+    assert_memory_equal(pci_config_space_ptr(&vfu_ctx, expoffsets[0] +
+                                             offsetof(struct pmcap, pmcs)),
+                        &pm.pmcs, sizeof (struct pmcs));
+
+    /* check read only capability */
+
+    ret = pci_config_space_access(&vfu_ctx, (char *)vsc1->data, 3,
+                                  expoffsets[1] + offsetof(struct vsc, data),
+                                  false);
+    assert_int_equal(ret, 3);
+    assert_memory_equal(vsc1->data, "abc", 3);
+
+    ret = pci_config_space_access(&vfu_ctx, "ced", 3,
+                                  expoffsets[1] + offsetof(struct vsc, data),
+                                  true);
+    assert_int_equal(ret, -EPERM);
+
+    /* check capability callback */
+
+    ret = pci_config_space_access(&vfu_ctx, "Bye world.", 10,
+                                  expoffsets[2] + offsetof(struct vsc, data),
+                                  true);
+
+    assert_int_equal(ret, 10);
+    assert_memory_equal(pci_config_space_ptr(&vfu_ctx,
+                        expoffsets[2] + offsetof(struct vsc, data)),
+                        "Bye world.", 10);
 }
 
 static void