From bc7eeda063527c6eeb29dcebb1554e3c02daa5a1 Mon Sep 17 00:00:00 2001 From: Bill Nguyen Date: Tue, 10 Mar 2026 14:56:04 +1100 Subject: [PATCH 1/4] tool: fix x86 MSI pcidev parsing + add tests Signed-off-by: Bill Nguyen --- tool/microkit/src/sdf.rs | 120 +++++++++++++----- .../irq_msi_pci_bus_greater_than_255.system | 12 ++ .../irq_msi_pci_dev_greater_than_31.system | 12 ++ .../irq_msi_pci_func_greater_than_7.system | 12 ++ .../tests/sdf/irq_msi_pci_invalid.system | 12 ++ .../tests/sdf/irq_msi_pci_valid.system | 12 ++ tool/microkit/tests/test.rs | 47 ++++++- 7 files changed, 192 insertions(+), 35 deletions(-) create mode 100644 tool/microkit/tests/sdf/irq_msi_pci_bus_greater_than_255.system create mode 100644 tool/microkit/tests/sdf/irq_msi_pci_dev_greater_than_31.system create mode 100644 tool/microkit/tests/sdf/irq_msi_pci_func_greater_than_7.system create mode 100644 tool/microkit/tests/sdf/irq_msi_pci_invalid.system create mode 100644 tool/microkit/tests/sdf/irq_msi_pci_valid.system diff --git a/tool/microkit/src/sdf.rs b/tool/microkit/src/sdf.rs index e254a2e2..2f777d30 100644 --- a/tool/microkit/src/sdf.rs +++ b/tool/microkit/src/sdf.rs @@ -49,6 +49,11 @@ 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; + /// 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. @@ -821,44 +826,95 @@ impl ProtectionDomain { &["id", "setvar_id", "pcidev", "handle", "vector"], )?; - let pci_parts: Vec = 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::() .unwrap(); @@ -884,9 +940,9 @@ impl ProtectionDomain { 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, }, diff --git a/tool/microkit/tests/sdf/irq_msi_pci_bus_greater_than_255.system b/tool/microkit/tests/sdf/irq_msi_pci_bus_greater_than_255.system new file mode 100644 index 00000000..328ee1dd --- /dev/null +++ b/tool/microkit/tests/sdf/irq_msi_pci_bus_greater_than_255.system @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/tool/microkit/tests/sdf/irq_msi_pci_dev_greater_than_31.system b/tool/microkit/tests/sdf/irq_msi_pci_dev_greater_than_31.system new file mode 100644 index 00000000..28c39e85 --- /dev/null +++ b/tool/microkit/tests/sdf/irq_msi_pci_dev_greater_than_31.system @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/tool/microkit/tests/sdf/irq_msi_pci_func_greater_than_7.system b/tool/microkit/tests/sdf/irq_msi_pci_func_greater_than_7.system new file mode 100644 index 00000000..bd27ccd3 --- /dev/null +++ b/tool/microkit/tests/sdf/irq_msi_pci_func_greater_than_7.system @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/tool/microkit/tests/sdf/irq_msi_pci_invalid.system b/tool/microkit/tests/sdf/irq_msi_pci_invalid.system new file mode 100644 index 00000000..8689d418 --- /dev/null +++ b/tool/microkit/tests/sdf/irq_msi_pci_invalid.system @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/tool/microkit/tests/sdf/irq_msi_pci_valid.system b/tool/microkit/tests/sdf/irq_msi_pci_valid.system new file mode 100644 index 00000000..e994bc1d --- /dev/null +++ b/tool/microkit/tests/sdf/irq_msi_pci_valid.system @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/tool/microkit/tests/test.rs b/tool/microkit/tests/test.rs index 02263798..a6d187e6 100644 --- a/tool/microkit/tests/test.rs +++ b/tool/microkit/tests/test.rs @@ -427,7 +427,7 @@ mod protection_domain { check_error( &DEFAULT_X86_64_KERNEL_CONFIG, "irq_msi_pci_bus_less_than_0.system", - "Error: PCI bus must be >= 0 on element 'irq'", + "Error: PCI bus must be within [0..255] on element 'irq'", ) } @@ -436,7 +436,7 @@ mod protection_domain { check_error( &DEFAULT_X86_64_KERNEL_CONFIG, "irq_msi_pci_dev_less_than_0.system", - "Error: PCI device must be >= 0 on element 'irq'", + "Error: PCI device must be within [0..31] on element 'irq'", ) } @@ -445,7 +445,34 @@ mod protection_domain { check_error( &DEFAULT_X86_64_KERNEL_CONFIG, "irq_msi_pci_func_less_than_0.system", - "Error: PCI function must be >= 0 on element 'irq'", + "Error: PCI function must be within [0..7] on element 'irq'", + ) + } + + #[test] + fn test_irq_msi_pci_bus_greater_than_255() { + check_error( + &DEFAULT_X86_64_KERNEL_CONFIG, + "irq_msi_pci_bus_greater_than_255.system", + "Error: PCI bus must be within [0..255] on element 'irq'", + ) + } + + #[test] + fn test_irq_msi_pci_dev_greater_than_31() { + check_error( + &DEFAULT_X86_64_KERNEL_CONFIG, + "irq_msi_pci_dev_greater_than_31.system", + "Error: PCI device must be within [0..31] on element 'irq'", + ) + } + + #[test] + fn test_irq_msi_pci_func_greater_than_7() { + check_error( + &DEFAULT_X86_64_KERNEL_CONFIG, + "irq_msi_pci_func_greater_than_7.system", + "Error: PCI function must be within [0..7] on element 'irq'", ) } @@ -467,6 +494,20 @@ mod protection_domain { ) } + #[test] + fn test_irq_msi_msi_pci_invalid() { + check_error( + &DEFAULT_X86_64_KERNEL_CONFIG, + "irq_msi_pci_invalid.system", + "Error: failed to parse PCI address '0:0:0' on element 'irq'", + ) + } + + #[test] + fn test_irq_msi_msi_pci_valid() { + check_success(&DEFAULT_X86_64_KERNEL_CONFIG, "irq_msi_pci_valid.system") + } + #[test] fn test_parent_has_id() { check_error( From c45cf182a6385487223ae62e7fe09aa96eb4913e Mon Sep 17 00:00:00 2001 From: Bill Nguyen Date: Tue, 10 Mar 2026 14:56:44 +1100 Subject: [PATCH 2/4] manual: fix x86 MSI pcidev notation Signed-off-by: Bill Nguyen --- docs/manual.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/manual.md b/docs/manual.md index 3b4dbb5b..af25fbd0 100644 --- a/docs/manual.md +++ b/docs/manual.md @@ -1030,7 +1030,7 @@ 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. * `setvar_id`: (optional) Specifies a symbol in the program image. This symbol will be rewritten with the channel identifier of the IRQ. From 95a05af67f7a9f1d5cfe14c93380f4df9fb67e61 Mon Sep 17 00:00:00 2001 From: Bill Nguyen Date: Tue, 10 Mar 2026 15:11:57 +1100 Subject: [PATCH 3/4] tool: add x86 IRQ vector bound check + add tests Signed-off-by: Bill Nguyen --- tool/microkit/src/sdf.rs | 13 +++++++---- .../irq_ioapic_vector_greater_than_107.system | 12 ++++++++++ .../irq_msi_vector_greater_than_107.system | 12 ++++++++++ tool/microkit/tests/test.rs | 22 +++++++++++++++++-- 4 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 tool/microkit/tests/sdf/irq_ioapic_vector_greater_than_107.system create mode 100644 tool/microkit/tests/sdf/irq_msi_vector_greater_than_107.system diff --git a/tool/microkit/src/sdf.rs b/tool/microkit/src/sdf.rs index 2f777d30..ba53399e 100644 --- a/tool/microkit/src/sdf.rs +++ b/tool/microkit/src/sdf.rs @@ -54,6 +54,11 @@ 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. @@ -791,11 +796,11 @@ impl ProtectionDomain { let vector = checked_lookup(xml_sdf, &child, "vector")? .parse::() .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}]"), )); } @@ -929,11 +934,11 @@ impl ProtectionDomain { let vector = checked_lookup(xml_sdf, &child, "vector")? .parse::() .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}]"), )); } diff --git a/tool/microkit/tests/sdf/irq_ioapic_vector_greater_than_107.system b/tool/microkit/tests/sdf/irq_ioapic_vector_greater_than_107.system new file mode 100644 index 00000000..b1756ec6 --- /dev/null +++ b/tool/microkit/tests/sdf/irq_ioapic_vector_greater_than_107.system @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/tool/microkit/tests/sdf/irq_msi_vector_greater_than_107.system b/tool/microkit/tests/sdf/irq_msi_vector_greater_than_107.system new file mode 100644 index 00000000..de520e61 --- /dev/null +++ b/tool/microkit/tests/sdf/irq_msi_vector_greater_than_107.system @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/tool/microkit/tests/test.rs b/tool/microkit/tests/test.rs index a6d187e6..36872712 100644 --- a/tool/microkit/tests/test.rs +++ b/tool/microkit/tests/test.rs @@ -418,7 +418,16 @@ mod protection_domain { check_error( &DEFAULT_X86_64_KERNEL_CONFIG, "irq_ioapic_vector_less_than_0.system", - "Error: vector must be >= 0 on element 'irq'", + "Error: vector must be within [0..107] on element 'irq'", + ) + } + + #[test] + fn test_irq_ioapic_vector_greater_than_107() { + check_error( + &DEFAULT_X86_64_KERNEL_CONFIG, + "irq_ioapic_vector_greater_than_107.system", + "Error: vector must be within [0..107] on element 'irq'", ) } @@ -490,7 +499,16 @@ mod protection_domain { check_error( &DEFAULT_X86_64_KERNEL_CONFIG, "irq_msi_vector_less_than_0.system", - "Error: vector must be >= 0 on element 'irq'", + "Error: vector must be within [0..107] on element 'irq'", + ) + } + + #[test] + fn test_irq_msi_vector_greater_than_107() { + check_error( + &DEFAULT_X86_64_KERNEL_CONFIG, + "irq_msi_vector_greater_than_107.system", + "Error: vector must be within [0..107] on element 'irq'", ) } From 13a72bb8adb4dec159a48dab54143d6f45c289c8 Mon Sep 17 00:00:00 2001 From: Bill Nguyen Date: Tue, 10 Mar 2026 15:13:40 +1100 Subject: [PATCH 4/4] manual: add x86 IRQ vector bound + priority note Signed-off-by: Bill Nguyen --- docs/manual.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/manual.md b/docs/manual.md index af25fbd0..96cf1071 100644 --- a/docs/manual.md +++ b/docs/manual.md @@ -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). @@ -1032,7 +1032,7 @@ The `irq` element has the following attributes when registering X86_64 MSI inter * `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 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: