Skip to content

Commit f9d86a0

Browse files
Sarah ChanCQ Bot
Sarah Chan
authored and
CQ Bot
committed
[fdf] Refactor driver index composite code
Move helper functions into a separate file. Add test functions to make creating fake test data easier. Change-Id: Id366dca5e0ed8c2ee4c2302e91a46cff553e7247 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1178033 Reviewed-by: Nick Eaton <[email protected]> Commit-Queue: Sarah Chan <[email protected]>
1 parent ea41741 commit f9d86a0

File tree

7 files changed

+443
-488
lines changed

7 files changed

+443
-488
lines changed

src/devices/bin/driver-index/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ rustc_binary("bin") {
9090
]
9191

9292
sources = [
93+
"src/composite_helper.rs",
9394
"src/composite_node_spec_manager.rs",
9495
"src/driver_loading_fuzzer.rs",
9596
"src/escrow_support.rs",
Lines changed: 300 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,300 @@
1+
// Copyright 2024 The Fuchsia Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
use crate::match_common::{get_composite_rules_from_composite_driver, node_to_device_property};
6+
use crate::resolved_driver::ResolvedDriver;
7+
use crate::serde_ext::ConditionDef;
8+
use bind::compiler::symbol_table::{get_deprecated_key_identifier, get_deprecated_key_value};
9+
use bind::compiler::Symbol;
10+
use bind::interpreter::match_bind::{match_bind, DeviceProperties, MatchBindData, PropertyKey};
11+
use fidl_fuchsia_driver_framework as fdf;
12+
use serde::{Deserialize, Serialize};
13+
use std::collections::{BTreeMap, HashMap, HashSet};
14+
use zx::sys::zx_status_t;
15+
use zx::Status;
16+
17+
pub type BindRules = BTreeMap<PropertyKey, BindRuleCondition>;
18+
19+
#[derive(Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)]
20+
pub struct BindRuleCondition {
21+
#[serde(with = "ConditionDef")]
22+
pub condition: fdf::Condition,
23+
pub values: Vec<Symbol>,
24+
}
25+
26+
pub fn find_composite_driver_match<'a>(
27+
parents: &'a Vec<fdf::ParentSpec>,
28+
composite_drivers: &Vec<&ResolvedDriver>,
29+
) -> Option<fdf::CompositeDriverMatch> {
30+
for composite_driver in composite_drivers {
31+
let matched_composite = match_composite_properties(composite_driver, parents);
32+
if let Ok(Some(matched_composite)) = matched_composite {
33+
return Some(matched_composite);
34+
}
35+
}
36+
None
37+
}
38+
39+
pub fn node_matches_composite_driver(
40+
node: &fdf::ParentSpec,
41+
bind_rules_node: &Vec<u8>,
42+
symbol_table: &HashMap<u32, String>,
43+
) -> bool {
44+
match node_to_device_property(&node.properties) {
45+
Err(_) => false,
46+
Ok(props) => {
47+
match_bind(MatchBindData { symbol_table, instructions: bind_rules_node }, &props)
48+
.unwrap_or(false)
49+
}
50+
}
51+
}
52+
53+
pub fn convert_fidl_to_bind_rules(
54+
fidl_bind_rules: &Vec<fdf::BindRule>,
55+
) -> Result<BindRules, zx_status_t> {
56+
if fidl_bind_rules.is_empty() {
57+
return Err(Status::INVALID_ARGS.into_raw());
58+
}
59+
60+
let mut bind_rules = BTreeMap::new();
61+
for fidl_rule in fidl_bind_rules {
62+
let key = match &fidl_rule.key {
63+
fdf::NodePropertyKey::IntValue(i) => PropertyKey::NumberKey(i.clone().into()),
64+
fdf::NodePropertyKey::StringValue(s) => PropertyKey::StringKey(s.clone()),
65+
};
66+
67+
// Check if the properties contain duplicate keys.
68+
if bind_rules.contains_key(&key) {
69+
return Err(Status::INVALID_ARGS.into_raw());
70+
}
71+
72+
let first_val = fidl_rule.values.first().ok_or_else(|| Status::INVALID_ARGS.into_raw())?;
73+
let values = fidl_rule
74+
.values
75+
.iter()
76+
.map(|val| {
77+
// Check that the properties are all the same type.
78+
if std::mem::discriminant(first_val) != std::mem::discriminant(val) {
79+
return Err(Status::INVALID_ARGS.into_raw());
80+
}
81+
Ok(node_property_to_symbol(val)?)
82+
})
83+
.collect::<Result<Vec<Symbol>, zx_status_t>>()?;
84+
85+
bind_rules
86+
.insert(key, BindRuleCondition { condition: fidl_rule.condition, values: values });
87+
}
88+
Ok(bind_rules)
89+
}
90+
91+
pub fn node_property_to_symbol(value: &fdf::NodePropertyValue) -> Result<Symbol, zx_status_t> {
92+
match value {
93+
fdf::NodePropertyValue::IntValue(i) => {
94+
Ok(bind::compiler::Symbol::NumberValue(i.clone().into()))
95+
}
96+
fdf::NodePropertyValue::StringValue(s) => {
97+
Ok(bind::compiler::Symbol::StringValue(s.clone()))
98+
}
99+
fdf::NodePropertyValue::EnumValue(s) => Ok(bind::compiler::Symbol::EnumValue(s.clone())),
100+
fdf::NodePropertyValue::BoolValue(b) => Ok(bind::compiler::Symbol::BoolValue(b.clone())),
101+
_ => Err(Status::INVALID_ARGS.into_raw()),
102+
}
103+
}
104+
105+
pub fn get_driver_url(composite: &fdf::CompositeDriverMatch) -> String {
106+
return composite
107+
.composite_driver
108+
.as_ref()
109+
.and_then(|driver| driver.driver_info.as_ref())
110+
.and_then(|driver_info| driver_info.url.clone())
111+
.unwrap_or_else(|| "".to_string());
112+
}
113+
114+
pub fn match_node(bind_rules: &BindRules, device_properties: &DeviceProperties) -> bool {
115+
for (key, node_prop_values) in bind_rules.iter() {
116+
let mut dev_prop_contains_value = match device_properties.get(key) {
117+
Some(val) => node_prop_values.values.contains(val),
118+
None => false,
119+
};
120+
121+
// If the properties don't contain the key, try to convert it to a deprecated
122+
// key and check the properties with it.
123+
if !dev_prop_contains_value && !device_properties.contains_key(key) {
124+
let deprecated_key = match key {
125+
PropertyKey::NumberKey(int_key) => get_deprecated_key_identifier(*int_key as u32)
126+
.map(|key| PropertyKey::StringKey(key)),
127+
PropertyKey::StringKey(str_key) => {
128+
get_deprecated_key_value(str_key).map(|key| PropertyKey::NumberKey(key as u64))
129+
}
130+
};
131+
132+
if let Some(key) = deprecated_key {
133+
dev_prop_contains_value = match device_properties.get(&key) {
134+
Some(val) => node_prop_values.values.contains(val),
135+
None => false,
136+
};
137+
}
138+
}
139+
140+
let evaluate_condition = match node_prop_values.condition {
141+
fdf::Condition::Accept => {
142+
// If the node property accepts a false boolean value and the property is
143+
// missing from the device properties, then we should evaluate the condition
144+
// as true.
145+
dev_prop_contains_value
146+
|| node_prop_values.values.contains(&Symbol::BoolValue(false))
147+
}
148+
fdf::Condition::Reject => !dev_prop_contains_value,
149+
fdf::Condition::Unknown => {
150+
tracing::error!("Invalid condition type in bind rules.");
151+
return false;
152+
}
153+
};
154+
155+
if !evaluate_condition {
156+
return false;
157+
}
158+
}
159+
160+
true
161+
}
162+
163+
pub fn match_composite_properties<'a>(
164+
composite_driver: &'a ResolvedDriver,
165+
parents: &'a Vec<fdf::ParentSpec>,
166+
) -> Result<Option<fdf::CompositeDriverMatch>, i32> {
167+
// The spec must have at least 1 node to match a composite driver.
168+
if parents.len() < 1 {
169+
return Ok(None);
170+
}
171+
172+
let composite = get_composite_rules_from_composite_driver(composite_driver)?;
173+
174+
// The composite driver bind rules should have a total node count of more than or equal to the
175+
// total node count of the spec. This is to account for optional nodes in the
176+
// composite driver bind rules.
177+
if composite.optional_nodes.len() + composite.additional_nodes.len() + 1 < parents.len() {
178+
return Ok(None);
179+
}
180+
181+
// First find a matching primary node.
182+
let mut primary_parent_index = 0;
183+
let mut primary_matches = false;
184+
for i in 0..parents.len() {
185+
primary_matches = node_matches_composite_driver(
186+
&parents[i],
187+
&composite.primary_node.instructions,
188+
&composite.symbol_table,
189+
);
190+
if primary_matches {
191+
primary_parent_index = i as u32;
192+
break;
193+
}
194+
}
195+
196+
if !primary_matches {
197+
return Ok(None);
198+
}
199+
200+
// The remaining nodes in the properties can match the
201+
// additional nodes in the bind rules in any order.
202+
//
203+
// This logic has one issue that we are accepting as a tradeoff for simplicity:
204+
// If a properties node can match to multiple bind rule
205+
// additional nodes, it is going to take the first one, even if there is a less strict
206+
// node that it can take. This can lead to false negative matches.
207+
//
208+
// Example:
209+
// properties[1] can match both additional_nodes[0] and additional_nodes[1]
210+
// properties[2] can only match additional_nodes[0]
211+
//
212+
// This algorithm will return false because it matches up properties[1] with
213+
// additional_nodes[0], and so properties[2] can't match the remaining nodes
214+
// [additional_nodes[1]].
215+
//
216+
// If we were smarter here we could match up properties[1] with additional_nodes[1]
217+
// and properties[2] with additional_nodes[0] to return a positive match.
218+
// TODO(https://fxbug.dev/42058532): Disallow ambiguity with spec matching. We should log
219+
// a warning and return false if a spec node matches with multiple composite
220+
// driver nodes, and vice versa.
221+
let mut unmatched_additional_indices =
222+
(0..composite.additional_nodes.len()).collect::<HashSet<_>>();
223+
let mut unmatched_optional_indices =
224+
(0..composite.optional_nodes.len()).collect::<HashSet<_>>();
225+
226+
let mut parent_names = vec![];
227+
228+
for i in 0..parents.len() {
229+
if i == primary_parent_index as usize {
230+
parent_names.push(composite.symbol_table[&composite.primary_node.name_id].clone());
231+
continue;
232+
}
233+
234+
let mut matched = None;
235+
let mut matched_name: Option<String> = None;
236+
let mut from_optional = false;
237+
238+
// First check if any of the additional nodes match it.
239+
for &j in &unmatched_additional_indices {
240+
let matches = node_matches_composite_driver(
241+
&parents[i],
242+
&composite.additional_nodes[j].instructions,
243+
&composite.symbol_table,
244+
);
245+
if matches {
246+
matched = Some(j);
247+
matched_name =
248+
Some(composite.symbol_table[&composite.additional_nodes[j].name_id].clone());
249+
break;
250+
}
251+
}
252+
253+
// If no additional nodes matched it, then look in the optional nodes.
254+
if matched.is_none() {
255+
for &j in &unmatched_optional_indices {
256+
let matches = node_matches_composite_driver(
257+
&parents[i],
258+
&composite.optional_nodes[j].instructions,
259+
&composite.symbol_table,
260+
);
261+
if matches {
262+
from_optional = true;
263+
matched = Some(j);
264+
matched_name =
265+
Some(composite.symbol_table[&composite.optional_nodes[j].name_id].clone());
266+
break;
267+
}
268+
}
269+
}
270+
271+
if matched.is_none() {
272+
return Ok(None);
273+
}
274+
275+
if from_optional {
276+
unmatched_optional_indices.remove(&matched.unwrap());
277+
} else {
278+
unmatched_additional_indices.remove(&matched.unwrap());
279+
}
280+
281+
parent_names.push(matched_name.unwrap());
282+
}
283+
284+
// If we didn't consume all of the additional nodes in the bind rules then this is not a match.
285+
if !unmatched_additional_indices.is_empty() {
286+
return Ok(None);
287+
}
288+
289+
let driver = fdf::CompositeDriverInfo {
290+
composite_name: Some(composite.symbol_table[&composite.device_name_id].clone()),
291+
driver_info: Some(composite_driver.create_driver_info(false)),
292+
..Default::default()
293+
};
294+
return Ok(Some(fdf::CompositeDriverMatch {
295+
composite_driver: Some(driver),
296+
parent_names: Some(parent_names),
297+
primary_parent_index: Some(primary_parent_index),
298+
..Default::default()
299+
}));
300+
}

0 commit comments

Comments
 (0)