diff --git a/docs/manual.md b/docs/manual.md index 3b4dbb5b..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). @@ -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: diff --git a/tool/microkit/src/sdf.rs b/tool/microkit/src/sdf.rs index e254a2e2..ba53399e 100644 --- a/tool/microkit/src/sdf.rs +++ b/tool/microkit/src/sdf.rs @@ -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. @@ -786,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}]"), )); } @@ -821,44 +831,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(); @@ -873,20 +934,20 @@ 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}]"), )); } 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_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_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/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 02263798..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'", ) } @@ -427,7 +436,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 +445,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 +454,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'", ) } @@ -463,10 +499,33 @@ 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'", ) } + #[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(