Adjust the pci format and add new example for qemu with pci #51

dallasxy · 2025-09-11T05:40:02Z

No description provided.

ZhongkaiXu

revoke approval

revoke approval, still something need to change

ZZJJWarth · 2025-12-03T03:27:04Z

include/zone_config.h


+struct hv_pci_dev_config {
+    __u64 bdf;
+    __u64 vbdf;


There are no corresponding items in the new_pcie branch of hvisor:
https://github.com/syswonder/hvisor/tree/new_pcie

Perhaps this config needs to be updated.

See the following link for reference:
https://github.com/syswonder/hvisor/blob/new_pcie/src/config.rs#L212

ZZJJWarth · 2025-12-03T03:41:03Z

include/zone_config.h

+
+typedef struct hv_pci_dev_config hv_pci_dev_config_t;
+
 struct pci_config {


Since we added bus_range_begin and bus_range_end to the pci_config of hvisor,
we should probably add the same fields to the pci_config of hvisor-tools as well.
It would also be better to implement the corresponding JSON parsing logic.
Or the hvisor would panic at https://github.com/syswonder/hvisor/blob/new_pcie/src/hypercall/mod.rs#L200
See the following link for reference: https://github.com/syswonder/hvisor/blob/new_pcie/src/config.rs#L58

Some problems in code

enkerewpo · 2026-01-03T16:08:36Z

tools/hvisor.c

+            SAFE_CJSON_GET_ARRAY_ITEM(alloc_pci_devs_json, i);
+        hv_pci_dev_config_t *dev_config = &config->alloc_pci_devs[i];
+        dev_config->domain = strtoull(
+            SAFE_CJSON_GET_OBJECT_ITEM(dev_config_json, "domain")->valuestring,


the example jsons in this PR use bdf instead of domain-bus-device-function, which is inconsistent

Copilot

Pull request overview

This PR refactors the PCI configuration format to support multiple PCI buses and introduces a more detailed device configuration structure. The changes update the configuration schema from a single PCI bus to an array of PCI buses and change PCI device configuration from simple integer values to structured objects with domain, bus, device, and function fields.

Refactored PCI configuration from single object to array supporting up to 4 buses
Changed PCI device configuration from integer BDF values to structured objects with separate domain/bus/device/function fields
Added bus_range_begin, bus_range_end, and domain fields to pci_config structure

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
tools/hvisor.c	Updated PCI parsing logic to handle array of PCI configs and structured device configs with domain/bus/device/function fields
include/zone_config.h	Added hv_pci_dev_config struct, changed pci_config to array, added new fields (bus_range, domain), incremented config version to 0x05
examples/qemu-riscv64/linux2.json	Updated to use array format for pci_config with new bus_range fields, changed alloc_pci_devs to structured objects with bdf field
examples/qemu-riscv64/linux2-aia.json	Updated to use array format for pci_config with new bus_range fields, changed alloc_pci_devs to structured objects with bdf field
examples/qemu-aarch64/with_virtio_blk_console/zone1_linux_with_pci.json	New example file demonstrating PCI configuration with array format and structured device configs using bdf field
examples/qemu-aarch64/with_virtio_blk_console/qemu_aarch64.rs	Updated PCI device array to use hv_pci_dev_config_t struct with bdf field, added bus_range fields to pci_config
examples/qemu-aarch64/with_virtio_gpu/qemu_aarch64.rs	Updated PCI device array to use hv_pci_dev_config struct with bdf field, added bus_range fields to pci_config

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-03T16:16:58Z

tools/hvisor.c

+        pci_config->ecam_base =
+            strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "ecam_base")
+                         ->valuestring,
+                     NULL, 16);
+        pci_config->ecam_size =
+            strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "ecam_size")
+                         ->valuestring,
+                     NULL, 16);
+        pci_config->io_base = strtoull(
+            SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "io_base")->valuestring,
+            NULL, 16);
+        pci_config->io_size = strtoull(
+            SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "io_size")->valuestring,
+            NULL, 16);
+        pci_config->pci_io_base =
+            strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_io_base")
+                         ->valuestring,
+                     NULL, 16);
+        pci_config->mem32_base =
+            strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem32_base")
+                         ->valuestring,
+                     NULL, 16);
+        pci_config->mem32_size =
+            strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem32_size")
+                         ->valuestring,
+                     NULL, 16);
+        pci_config->pci_mem32_base = strtoull(
+            SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_mem32_base")
+                ->valuestring,
+            NULL, 16);
+        pci_config->mem64_base =
+            strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem64_base")
+                         ->valuestring,
+                     NULL, 16);
+        pci_config->mem64_size =
+            strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem64_size")
+                         ->valuestring,
+                     NULL, 16);
+        pci_config->pci_mem64_base = strtoull(
+            SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_mem64_base")
+                ->valuestring,
+            NULL, 16);
+        pci_config->bus_range_begin = strtoull(
+            SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "bus_range_begin")
+                ->valuestring,
+            NULL, 16);
+        pci_config->bus_range_end = strtoull(
+            SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "bus_range_end")
+                ->valuestring,
+            NULL, 16);
+        pci_config->domain = strtoull(
+            SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "domain")->valuestring,
+            NULL, 16);
+


The result of SAFE_CJSON_GET_OBJECT_ITEM is directly dereferenced without null checking. If any of the required fields (ecam_base, ecam_size, io_base, io_size, pci_io_base, mem32_base, mem32_size, pci_mem32_base, mem64_base, mem64_size, pci_mem64_base, bus_range_begin, bus_range_end, domain) are missing from the JSON, this will cause a null pointer dereference and crash. Add null checks before dereferencing the JSON objects, similar to how it's done elsewhere in the codebase with CHECK_JSON_NULL or CHECK_JSON_NULL_ERR_OUT macros.

Suggested change

pci_config->ecam_base =

strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "ecam_base")

->valuestring,

NULL, 16);

pci_config->ecam_size =

strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "ecam_size")

->valuestring,

NULL, 16);

pci_config->io_base = strtoull(

SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "io_base")->valuestring,

NULL, 16);

pci_config->io_size = strtoull(

SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "io_size")->valuestring,

NULL, 16);

pci_config->pci_io_base =

strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_io_base")

->valuestring,

NULL, 16);

pci_config->mem32_base =

strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem32_base")

->valuestring,

NULL, 16);

pci_config->mem32_size =

strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem32_size")

->valuestring,

NULL, 16);

pci_config->pci_mem32_base = strtoull(

SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_mem32_base")

->valuestring,

NULL, 16);

pci_config->mem64_base =

strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem64_base")

->valuestring,

NULL, 16);

pci_config->mem64_size =

strtoull(SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem64_size")

->valuestring,

NULL, 16);

pci_config->pci_mem64_base = strtoull(

SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_mem64_base")

->valuestring,

NULL, 16);

pci_config->bus_range_begin = strtoull(

SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "bus_range_begin")

->valuestring,

NULL, 16);

pci_config->bus_range_end = strtoull(

SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "bus_range_end")

->valuestring,

NULL, 16);

pci_config->domain = strtoull(

SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "domain")->valuestring,

NULL, 16);

cJSON *ecam_base_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "ecam_base");

if (ecam_base_item && ecam_base_item->valuestring) {

pci_config->ecam_base = strtoull(ecam_base_item->valuestring, NULL, 16);

} else {

pci_config->ecam_base = 0;

}

cJSON *ecam_size_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "ecam_size");

if (ecam_size_item && ecam_size_item->valuestring) {

pci_config->ecam_size = strtoull(ecam_size_item->valuestring, NULL, 16);

} else {

pci_config->ecam_size = 0;

}

cJSON *io_base_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "io_base");

if (io_base_item && io_base_item->valuestring) {

pci_config->io_base = strtoull(io_base_item->valuestring, NULL, 16);

} else {

pci_config->io_base = 0;

}

cJSON *io_size_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "io_size");

if (io_size_item && io_size_item->valuestring) {

pci_config->io_size = strtoull(io_size_item->valuestring, NULL, 16);

} else {

pci_config->io_size = 0;

}

cJSON *pci_io_base_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_io_base");

if (pci_io_base_item && pci_io_base_item->valuestring) {

pci_config->pci_io_base = strtoull(pci_io_base_item->valuestring, NULL, 16);

} else {

pci_config->pci_io_base = 0;

}

cJSON *mem32_base_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem32_base");

if (mem32_base_item && mem32_base_item->valuestring) {

pci_config->mem32_base = strtoull(mem32_base_item->valuestring, NULL, 16);

} else {

pci_config->mem32_base = 0;

}

cJSON *mem32_size_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem32_size");

if (mem32_size_item && mem32_size_item->valuestring) {

pci_config->mem32_size = strtoull(mem32_size_item->valuestring, NULL, 16);

} else {

pci_config->mem32_size = 0;

}

cJSON *pci_mem32_base_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_mem32_base");

if (pci_mem32_base_item && pci_mem32_base_item->valuestring) {

pci_config->pci_mem32_base = strtoull(pci_mem32_base_item->valuestring, NULL, 16);

} else {

pci_config->pci_mem32_base = 0;

}

cJSON *mem64_base_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem64_base");

if (mem64_base_item && mem64_base_item->valuestring) {

pci_config->mem64_base = strtoull(mem64_base_item->valuestring, NULL, 16);

} else {

pci_config->mem64_base = 0;

}

cJSON *mem64_size_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "mem64_size");

if (mem64_size_item && mem64_size_item->valuestring) {

pci_config->mem64_size = strtoull(mem64_size_item->valuestring, NULL, 16);

} else {

pci_config->mem64_size = 0;

}

cJSON *pci_mem64_base_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "pci_mem64_base");

if (pci_mem64_base_item && pci_mem64_base_item->valuestring) {

pci_config->pci_mem64_base = strtoull(pci_mem64_base_item->valuestring, NULL, 16);

} else {

pci_config->pci_mem64_base = 0;

}

cJSON *bus_range_begin_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "bus_range_begin");

if (bus_range_begin_item && bus_range_begin_item->valuestring) {

pci_config->bus_range_begin = strtoull(bus_range_begin_item->valuestring, NULL, 16);

} else {

pci_config->bus_range_begin = 0;

}

cJSON *bus_range_end_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "bus_range_end");

if (bus_range_end_item && bus_range_end_item->valuestring) {

pci_config->bus_range_end = strtoull(bus_range_end_item->valuestring, NULL, 16);

} else {

pci_config->bus_range_end = 0;

}

cJSON *domain_item = SAFE_CJSON_GET_OBJECT_ITEM(pci_config_json, "domain");

if (domain_item && domain_item->valuestring) {

pci_config->domain = strtoull(domain_item->valuestring, NULL, 16);

} else {

pci_config->domain = 0;

}

Copilot · 2026-01-03T16:16:58Z

tools/hvisor.c

-        printf("pci_config field found.\n");
-    }
+    cJSON *pci_configs_json = SAFE_CJSON_GET_OBJECT_ITEM(root, "pci_config");
+    CHECK_JSON_NULL_ERR_OUT(pci_configs_json, "pci_configs")


The error message says "pci_configs" but the actual JSON field being checked is "pci_config" (singular). This inconsistency could confuse users debugging missing configuration. The error message should match the actual field name.

Suggested change

CHECK_JSON_NULL_ERR_OUT(pci_configs_json, "pci_configs")

CHECK_JSON_NULL_ERR_OUT(pci_configs_json, "pci_config")

Copilot · 2026-01-03T16:16:58Z

examples/qemu-riscv64/linux2-aia.json

+            "bdf": "0x0",
+            "dev_type": "0x0"
+        },
+        {
+            "bdf": "0x10",
+            "dev_type": "0x0"
+        },
+        {
+            "bdf": "0x20",


The JSON uses "bdf" field, but the C struct hv_pci_dev_config expects separate "domain", "bus", "device", and "function" fields. The JSON configuration should be updated to use these separate fields to match the struct definition in zone_config.h, or the parsing code needs to extract these components from the bdf value.

Suggested change

"bdf": "0x0",

"dev_type": "0x0"

},

{

"bdf": "0x10",

"dev_type": "0x0"

},

{

"bdf": "0x20",

"domain": "0x0",

"bus": "0x0",

"device": "0x0",

"function": "0x0",

"dev_type": "0x0"

},

{

"domain": "0x0",

"bus": "0x0",

"device": "0x2",

"function": "0x0",

"dev_type": "0x0"

},

{

"domain": "0x0",

"bus": "0x0",

"device": "0x4",

"function": "0x0",

Copilot · 2026-01-03T16:16:59Z

examples/qemu-riscv64/linux2.json

+        "bus_range_begin": "0x0",
+        "bus_range_end": "0x1f"


The C parsing code expects a "domain" field in the pci_config object, but none of the JSON example files include this field. This will cause a null pointer dereference when parsing. Add the "domain" field to the pci_config object.

Copilot · 2026-01-03T16:16:59Z

examples/qemu-aarch64/with_virtio_blk_console/qemu_aarch64.rs

+pub const ROOT_PCI_DEVS: [hv_pci_dev_config_t; 2] = [
+    hv_pci_dev_config_t {
+        bdf: 0,
+        dev_type: 0,
+    },
+    hv_pci_dev_config_t {


The type name uses the underscore suffix convention (hv_pci_dev_config_t) which is inconsistent with the type name used in the other Rust file (hv_pci_dev_config without _t suffix). For consistency, both files should use the same type name.

Suggested change

pub const ROOT_PCI_DEVS: [hv_pci_dev_config_t; 2] = [

hv_pci_dev_config_t {

bdf: 0,

dev_type: 0,

},

hv_pci_dev_config_t {

pub const ROOT_PCI_DEVS: [hv_pci_dev_config; 2] = [

hv_pci_dev_config {

bdf: 0,

dev_type: 0,

},

hv_pci_dev_config {

Copilot · 2026-01-03T16:17:00Z

examples/qemu-riscv64/linux2.json

+            "bdf": "0x0",
+            "dev_type": "0x0"
+        },
+        {
+            "bdf": "0x10",
+            "dev_type": "0x0"
+        },
+        {
+            "bdf": "0x20",


The JSON uses "bdf" field, but the C struct hv_pci_dev_config expects separate "domain", "bus", "device", and "function" fields. The JSON configuration should be updated to use these separate fields to match the struct definition in zone_config.h, or the parsing code needs to extract these components from the bdf value.

Suggested change

"bdf": "0x0",

"dev_type": "0x0"

},

{

"bdf": "0x10",

"dev_type": "0x0"

},

{

"bdf": "0x20",

"domain": "0x0",

"bus": "0x0",

"device": "0x0",

"function": "0x0",

"dev_type": "0x0"

},

{

"domain": "0x0",

"bus": "0x0",

"device": "0x2",

"function": "0x0",

"dev_type": "0x0"

},

{

"domain": "0x0",

"bus": "0x0",

"device": "0x4",

"function": "0x0",

Copilot · 2026-01-03T16:17:00Z

examples/qemu-aarch64/with_virtio_blk_console/zone1_linux_with_pci.json

+            "bdf": "0x0",
+            "dev_type": "0x0"
+        },
+        {
+            "bdf": "0x18",


The JSON uses "bdf" field, but the C struct hv_pci_dev_config expects separate "domain", "bus", "device", and "function" fields. The JSON configuration should be updated to use these separate fields to match the struct definition in zone_config.h, or the parsing code needs to extract these components from the bdf value.

Suggested change

"bdf": "0x0",

"dev_type": "0x0"

},

{

"bdf": "0x18",

"domain": "0x0",

"bus": "0x0",

"device": "0x0",

"function": "0x0",

"dev_type": "0x0"

},

{

"domain": "0x0",

"bus": "0x0",

"device": "0x3",

"function": "0x0",

Copilot · 2026-01-03T16:17:00Z

examples/qemu-riscv64/linux2-aia.json

+        "bus_range_begin": "0x0",
+        "bus_range_end": "0x1f"


The C parsing code expects a "domain" field in the pci_config object, but none of the JSON example files include this field. This will cause a null pointer dereference when parsing. Add the "domain" field to the pci_config object.

Copilot · 2026-01-03T16:17:00Z

examples/qemu-aarch64/with_virtio_blk_console/zone1_linux_with_pci.json

+        "bus_range_begin": "0x0",
+        "bus_range_end": "0x1f" 


The C parsing code expects a "domain" field in the pci_config object, but none of the JSON example files include this field. This will cause a null pointer dereference when parsing. Add the "domain" field to the pci_config object.

Copilot · 2026-01-03T16:17:00Z

tools/hvisor.c

+    }
+
+    config->num_pci_bus = num_pci_bus;
+    log_info("num pci bus %llx", num_pci_bus);


The format specifier %llx is used for num_pci_bus which is of type int. This is incorrect and should use %d instead. Using %llx for an int can lead to undefined behavior and incorrect output.

Suggested change

log_info("num pci bus %llx", num_pci_bus);

log_info("num pci bus %d", num_pci_bus);

dallasxy added 2 commits September 11, 2025 13:31

Adjust the pci format and add new example for qemu with pci

7b5cf7c

fmt

9f5073e

caodg requested review from Inquisitor-201 and ZhongkaiXu October 31, 2025 02:48

ZhongkaiXu previously approved these changes Dec 3, 2025

View reviewed changes

ZhongkaiXu reviewed Dec 3, 2025

View reviewed changes

ZZJJWarth reviewed Dec 3, 2025

View reviewed changes

dallasxy added 6 commits December 3, 2025 12:49

update for new pcie

7b6ff70

Merge remote-tracking branch 'upstream/main'

febd052

Merge remote-tracking branch 'upstream/main'

b3ff20f

update pcie config

9fd2351

update magic version

b38e455

fmt

23d3311

dallasxy requested a review from ZhongkaiXu December 31, 2025 16:41

caodg requested review from Solicey and enkerewpo December 31, 2025 21:42

enkerewpo previously approved these changes Jan 3, 2026

View reviewed changes

enkerewpo reviewed Jan 3, 2026

View reviewed changes

enkerewpo requested a review from Copilot January 3, 2026 16:12

Copilot started reviewing on behalf of enkerewpo January 3, 2026 16:13 View session

Copilot AI reviewed Jan 3, 2026

View reviewed changes


		typedef struct hv_pci_dev_config hv_pci_dev_config_t;

		struct pci_config {

	CHECK_JSON_NULL_ERR_OUT(pci_configs_json, "pci_configs")
	CHECK_JSON_NULL_ERR_OUT(pci_configs_json, "pci_config")

-            "bdf": "0x0",
-            "dev_type": "0x0"
-        },
-        {
-            "bdf": "0x10",
-            "dev_type": "0x0"
-        },
-        {
-            "bdf": "0x20",
+            "domain": "0x0",
+            "bus": "0x0",
+            "device": "0x0",
+            "function": "0x0",
+            "dev_type": "0x0"
+        },
+        {
+            "domain": "0x0",
+            "bus": "0x0",
+            "device": "0x2",
+            "function": "0x0",
+            "dev_type": "0x0"
+        },
+        {
+            "domain": "0x0",
+            "bus": "0x0",
+            "device": "0x4",
+            "function": "0x0",

	log_info("num pci bus %llx", num_pci_bus);
	log_info("num pci bus %d", num_pci_bus);

Adjust the pci format and add new example for qemu with pci #51

Are you sure you want to change the base?

Adjust the pci format and add new example for qemu with pci #51

Uh oh!

Conversation

dallasxy commented Sep 11, 2025

Uh oh!

ZhongkaiXu left a comment

Choose a reason for hiding this comment

Uh oh!

ZZJJWarth Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

ZZJJWarth Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

enkerewpo Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants