Skip to content

Fix x86 MSI PCI parsing + add x86 IRQ vector range check #433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dreamliner787-9 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix x86 MSI PCI parsing + add x86 IRQ vector range check #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc7eeda
tool: fix x86 MSI pcidev parsing + add tests
dreamliner787-9 Mar 10, 2026
c45cf18
manual: fix x86 MSI pcidev notation
dreamliner787-9 Mar 10, 2026
95a05af
tool: add x86 IRQ vector bound check + add tests
dreamliner787-9 Mar 10, 2026
13a72bb
manual: add x86 IRQ vector bound + priority note
dreamliner787-9 Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/manual.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ The `irq` element has the following attributes when registering X86_64 IOAPIC in

* `id`: The channel identifier. Must be at least 0 and less than 63.
* `pin`: IOAPIC pin that generates the interrupt.
* `vector`: CPU vector to deliver the interrupt to.
* `vector`: CPU vector to deliver the interrupt to. Must be at least 0 and less than 108. A high vector equals to a high priority.
* `ioapic`: (optional) Zero based index of the IOAPIC to get the interrupt from. Defaults to 0.
* `level`: (optional) Whether the IRQ is level triggered (1) or edge triggered (0). Defaults to level (1).
* `polarity`: (optional) Whether the line polarity is high (1) or low (0). Defaults to high (1).
Expand All @@ -1030,9 +1030,9 @@ The `irq` element has the following attributes when registering X86_64 IOAPIC in
The `irq` element has the following attributes when registering X86_64 MSI interrupts:

* `id`: The channel identifier. Must be at least 0 and less than 63.
* `pcidev`: The PCI device address of the device that will generate the interrupt, in BUS:DEV:FUNC notation (e.g. 01:1f:2).
* `pcidev`: The PCI device address of the device that will generate the interrupt in hexadecimal, in BUS:DEV.FUNC notation (e.g. 01:1f.2).
* `handle`: Value of the handle programmed into the data portion of the MSI.
* `vector`: CPU vector to deliver the interrupt to.
* `vector`: CPU vector to deliver the interrupt to. Must be at least 0 and less than 108. A high vector equals to a high priority.
* `setvar_id`: (optional) Specifies a symbol in the program image. This symbol will be rewritten with the channel identifier of the IRQ.

The `ioport` element has the following attributes:
Expand Down
133 changes: 97 additions & 36 deletions tool/microkit/src/sdf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ pub const PD_DEFAULT_STACK_SIZE: u64 = 0x2000;
const PD_MIN_STACK_SIZE: u64 = 0x1000;
const PD_MAX_STACK_SIZE: u64 = 1024 * 1024 * 16;

/// Maximum values for PCI bus, device, function numbers. Inclusive.
const PCI_BUS_MAX: i64 = (1 << 8) - 1;
const PCI_DEV_MAX: i64 = (1 << 5) - 1;
const PCI_FUNC_MAX: i64 = (1 << 3) - 1;

/// Maximum x86 IRQ vector value. Inclusive.
/// This value is calculated by the kernel as `irq_user_max - irq_user_min` in
/// `src/arch/x86/object/interrupt.c`
const X86_IRQ_VECTOR_MAX: i64 = 107;

/// The purpose of this function is to parse an integer that could
/// either be in decimal or hex format, unlike the normal parsing
/// functionality that the Rust standard library provides.
Expand Down Expand Up @@ -786,11 +796,11 @@ impl ProtectionDomain {
let vector = checked_lookup(xml_sdf, &child, "vector")?
.parse::<i64>()
.unwrap();
if vector < 0 {
if !(0..=X86_IRQ_VECTOR_MAX).contains(&vector) {
return Err(value_error(
xml_sdf,
&child,
"vector must be >= 0".to_string(),
format!("vector must be within [0..{X86_IRQ_VECTOR_MAX}]"),
));
}

Expand Down Expand Up @@ -821,44 +831,95 @@ impl ProtectionDomain {
&["id", "setvar_id", "pcidev", "handle", "vector"],
)?;

let pci_parts: Vec<i64> = pcidev_str
.split([':', '.'])
.map(str::trim)
.map(|x| {
i64::from_str_radix(x, 16).expect(
"Error: Failed to parse parts of the PCI device address",
)
})
.collect();
if pci_parts.len() != 3 {
// A "pcidev" attribute is in a form of Bus:Dev.Func
// Split by the colon then the dot.

// If the input is valid, index 0 contains "Bus", index 1 contains
// "Dev.Func"
let pci_parts_by_colon: Vec<_> =
pcidev_str.split(':').map(str::trim).collect();

if pci_parts_by_colon.len() != 2 {
return Err(format!(
"Error: failed to parse PCI address '{}' on element '{}'",
pcidev_str,
child.tag_name().name()
));
}
if pci_parts[0] < 0 {
return Err(value_error(
xml_sdf,
&child,
"PCI bus must be >= 0".to_string(),
));
}
if pci_parts[1] < 0 {
return Err(value_error(
xml_sdf,
&child,
"PCI device must be >= 0".to_string(),
));
}
if pci_parts[2] < 0 {
return Err(value_error(
xml_sdf,
&child,
"PCI function must be >= 0".to_string(),

// If the input is valid, index 0 contains "Dev", index 1 contains
// "Func"
let pci_parts_by_dot: Vec<_> =
pci_parts_by_colon[1].split('.').map(str::trim).collect();
if pci_parts_by_dot.len() != 2 {
return Err(format!(
"Error: failed to parse PCI address '{}' on element '{}'",
pcidev_str,
child.tag_name().name()
));
}

let pci_bus_maybe = i64::from_str_radix(pci_parts_by_colon[0], 16);
let pci_dev_maybe = i64::from_str_radix(pci_parts_by_dot[0], 16);
let pci_func_maybe = i64::from_str_radix(pci_parts_by_dot[1], 16);

match pci_bus_maybe {
Ok(pci_bus_unchecked) => {
if !(0..=PCI_BUS_MAX).contains(&pci_bus_unchecked) {
return Err(value_error(
xml_sdf,
&child,
format!("PCI bus must be within [0..{PCI_BUS_MAX}]"),
));
}
}
Err(_) => {
return Err(format!(
"Error: failed to parse PCI bus of '{}' on element '{}'",
pcidev_str,
child.tag_name().name()
))
}
};

match pci_dev_maybe {
Ok(pci_dev_unchecked) => {
if !(0..=PCI_DEV_MAX).contains(&pci_dev_unchecked) {
return Err(value_error(
xml_sdf,
&child,
format!("PCI device must be within [0..{PCI_DEV_MAX}]"),
));
}
}
Err(_) => {
return Err(format!(
"Error: failed to parse PCI device of '{}' on element '{}'",
pcidev_str,
child.tag_name().name()
))
}
};

match pci_func_maybe {
Ok(pci_func_unchecked) => {
if !(0..=PCI_FUNC_MAX).contains(&pci_func_unchecked) {
return Err(value_error(
xml_sdf,
&child,
format!("PCI function must be within [0..{PCI_FUNC_MAX}]"),
));
}
}
Err(_) => {
return Err(format!(
"Error: failed to parse PCI function of '{}' on element '{}'",
pcidev_str,
child.tag_name().name()
))
}
};

let handle = checked_lookup(xml_sdf, &child, "handle")?
.parse::<i64>()
.unwrap();
Expand All @@ -873,20 +934,20 @@ impl ProtectionDomain {
let vector = checked_lookup(xml_sdf, &child, "vector")?
.parse::<i64>()
.unwrap();
if vector < 0 {
if !(0..=X86_IRQ_VECTOR_MAX).contains(&vector) {
return Err(value_error(
xml_sdf,
&child,
"vector must be >= 0".to_string(),
format!("vector must be within [0..{X86_IRQ_VECTOR_MAX}]"),
));
}

let irq = SysIrq {
id: id as u64,
kind: SysIrqKind::MSI {
pci_bus: pci_parts[0] as u64,
pci_dev: pci_parts[1] as u64,
pci_func: pci_parts[2] as u64,
pci_bus: pci_bus_maybe.unwrap() as u64,
pci_dev: pci_dev_maybe.unwrap() as u64,
pci_func: pci_func_maybe.unwrap() as u64,
handle: handle as u64,
vector: vector as u64,
},
Expand Down
12 changes: 12 additions & 0 deletions tool/microkit/tests/sdf/irq_ioapic_vector_greater_than_107.system
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2025, UNSW

SPDX-License-Identifier: BSD-2-Clause
-->
<system>
<protection_domain name="test1">
<program_image path="test" />
<irq pin="1" vector="108" id="16" ioapic="0" />
</protection_domain>
</system>
12 changes: 12 additions & 0 deletions tool/microkit/tests/sdf/irq_msi_pci_bus_greater_than_255.system
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2025, UNSW

SPDX-License-Identifier: BSD-2-Clause
-->
<system>
<protection_domain name="test1">
<program_image path="test" />
<irq pcidev="256:0.0" handle="0" vector="0" id="0" />
</protection_domain>
</system>
12 changes: 12 additions & 0 deletions tool/microkit/tests/sdf/irq_msi_pci_dev_greater_than_31.system
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2025, UNSW

SPDX-License-Identifier: BSD-2-Clause
-->
<system>
<protection_domain name="test1">
<program_image path="test" />
<irq pcidev="0:32.0" handle="0" vector="0" id="0" />
</protection_domain>
</system>
12 changes: 12 additions & 0 deletions tool/microkit/tests/sdf/irq_msi_pci_func_greater_than_7.system
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2025, UNSW

SPDX-License-Identifier: BSD-2-Clause
-->
<system>
<protection_domain name="test1">
<program_image path="test" />
<irq pcidev="0:0.8" handle="0" vector="0" id="0" />
</protection_domain>
</system>
12 changes: 12 additions & 0 deletions tool/microkit/tests/sdf/irq_msi_pci_invalid.system
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2025, UNSW
SPDX-License-Identifier: BSD-2-Clause
-->
<system>
<protection_domain name="test1">
<program_image path="test" />
<irq pcidev="0:0:0" handle="0" vector="0" id="0" />
</protection_domain>
</system>
12 changes: 12 additions & 0 deletions tool/microkit/tests/sdf/irq_msi_pci_valid.system
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2025, UNSW

SPDX-License-Identifier: BSD-2-Clause
-->
<system>
<protection_domain name="test1">
<program_image path="test" />
<irq pcidev="1:2.3" handle="0" vector="0" id="0" />
</protection_domain>
</system>
12 changes: 12 additions & 0 deletions tool/microkit/tests/sdf/irq_msi_vector_greater_than_107.system
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright 2025, UNSW

SPDX-License-Identifier: BSD-2-Clause
-->
<system>
<protection_domain name="test1">
<program_image path="test" />
<irq pcidev="0:0.0" handle="0" vector="108" id="0" />
</protection_domain>
</system>
Loading