Skip to content
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

hvdef: make HvStatus type distinct from HvError #754

Merged
merged 4 commits into from
Jan 31, 2025
Merged

hvdef: make HvStatus type distinct from HvError #754

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
eef9af4
hvdef: make `HvStatus` type distinct from `HvError`
jstarks Jan 28, 2025
e68a20a
tweak
jstarks Jan 31, 2025
bd5200f
fixes
jstarks Jan 31, 2025
4992ba2
aarch
jstarks Jan 31, 2025
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
10 changes: 5 additions & 5 deletions openhcl/hcl/src/ioctl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ use hvdef::HvMessage;
use hvdef::HvRegisterName;
use hvdef::HvRegisterValue;
use hvdef::HvRegisterVsmPartitionConfig;
use hvdef::HvStatus;
use hvdef::HvX64RegisterName;
use hvdef::HvX64RegisterPage;
use hvdef::HypercallCode;
Expand Down Expand Up @@ -171,10 +172,9 @@ pub enum HypercallError {
impl HypercallError {
pub(crate) fn check(r: Result<i32, nix::Error>) -> Result<(), Self> {
match r {
Ok(0) => Ok(()),
Ok(n) => Err(Self::Hypervisor(HvError(
n.try_into().expect("hypervisor result out of range"),
))),
Ok(n) => HvStatus(n.try_into().expect("hypervisor result out of range"))
.result()
.map_err(Self::Hypervisor),
Err(err) => Err(Self::Ioctl(IoctlError(err))),
}
}
Expand Down Expand Up @@ -1040,7 +1040,7 @@ impl MshvHvcall {
.map_err(HvcallError::HypercallIoctlFailed)?;
}

if call_object.status.call_status() == HvError::Timeout.0 {
if call_object.status.call_status() == Err(HvError::Timeout).into() {
// Any hypercall can timeout, even one that doesn't have reps. Continue processing
// from wherever the hypervisor left off. The rep start index isn't checked for
// validity, since it is only being used as an input to the untrusted hypervisor.
Expand Down
31 changes: 15 additions & 16 deletions openhcl/sidecar/src/arch/x86_64/vp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use core::sync::atomic::Ordering::Release;
use hvdef::hypercall::HvInputVtl;
use hvdef::hypercall::HvRegisterAssoc;
use hvdef::hypercall::TranslateVirtualAddressX64;
use hvdef::HvError;
use hvdef::HvStatus;
use hvdef::HvVtlEntryReason;
use hvdef::HvX64RegisterName;
use hvdef::HypercallCode;
Expand Down Expand Up @@ -400,7 +400,7 @@ fn get_vp_registers(command_page: &mut CommandPage) {
count,
target_vtl,
rsvd: _,
ref mut result,
ref mut status,
rsvd2: _,
regs: [],
} = FromBytes::mut_from(request).unwrap();
Expand All @@ -413,7 +413,7 @@ fn get_vp_registers(command_page: &mut CommandPage) {
return;
};

*result = HvError(0);
*status = HvStatus::SUCCESS;
for &mut HvRegisterAssoc {
name,
pad: _,
Expand All @@ -434,7 +434,7 @@ fn get_vp_registers(command_page: &mut CommandPage) {
match r {
Ok(v) => *value = v,
Err(err) => {
*result = err;
*status = Err(err).into();
break;
}
};
Expand All @@ -450,7 +450,7 @@ fn set_vp_registers(command_page: &mut CommandPage) {
count,
target_vtl,
rsvd: _,
ref mut result,
ref mut status,
rsvd2: _,
regs: [],
} = FromBytes::mut_from(request).unwrap();
Expand All @@ -463,7 +463,7 @@ fn set_vp_registers(command_page: &mut CommandPage) {
return;
};

*result = HvError(0);
*status = HvStatus::SUCCESS;
for &HvRegisterAssoc {
name,
value,
Expand All @@ -482,8 +482,8 @@ fn set_vp_registers(command_page: &mut CommandPage) {
set_hv_vp_register(target_vtl, name, value)
};

if let Err(err) = r {
*result = err;
if r.is_err() {
*status = r.into();
break;
}
}
Expand All @@ -509,17 +509,16 @@ fn translate_gva(command_page: &mut CommandPage) {
}

let result = hypercall(HypercallCode::HvCallTranslateVirtualAddressEx, 0);
let (result, output) = match result {
Ok(()) => {
// SAFETY: the output is not concurrently accessed
let output = unsafe { &*addr_space::hypercall_output() };
(HvError(0), FromBytes::read_from_prefix(output).unwrap())
}
Err(err) => (err, FromZeroes::new_zeroed()),
let output = if result.is_ok() {
// SAFETY: the output is not concurrently accessed
let output = unsafe { &*addr_space::hypercall_output() };
FromBytes::read_from_prefix(output).unwrap()
} else {
FromZeroes::new_zeroed()
};

TranslateGvaResponse {
result,
status: result.into(),
rsvd: [0; 7],
output,
}
Expand Down
23 changes: 9 additions & 14 deletions openhcl/sidecar_client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use hvdef::hypercall::HvRegisterAssoc;
use hvdef::hypercall::TranslateVirtualAddressExOutputX64;
use hvdef::HvError;
use hvdef::HvMessage;
use hvdef::HvStatus;
use pal_async::driver::PollImpl;
use pal_async::driver::SpawnDriver;
use pal_async::fd::PollFdReady;
Expand Down Expand Up @@ -434,19 +435,17 @@ impl<'a> SidecarVp<'a> {
count: regs.len() as u16,
target_vtl,
rsvd: 0,
result: HvError(0),
status: HvStatus::SUCCESS,
rsvd2: [0; 10],
regs: [],
},
regs.len(),
);
buf.copy_from_slice(regs);
self.run_sync()?;
let (&GetSetVpRegisterRequest { result, .. }, buf) =
let (&GetSetVpRegisterRequest { status, .. }, buf) =
self.command_result::<_, HvRegisterAssoc>(regs.len())?;
if result != HvError(0) {
return Err(SidecarError::Hypervisor(result));
}
status.result().map_err(SidecarError::Hypervisor)?;
regs.copy_from_slice(buf);
}
Ok(())
Expand All @@ -466,18 +465,16 @@ impl<'a> SidecarVp<'a> {
count: regs.len() as u16,
target_vtl,
rsvd: 0,
result: HvError(0),
status: HvStatus::SUCCESS,
rsvd2: [0; 10],
regs: [],
},
regs.len(),
);
buf.copy_from_slice(regs);
self.run_sync()?;
let &GetSetVpRegisterRequest { result, .. } = self.command_result::<_, u8>(0)?.0;
if result != HvError(0) {
return Err(SidecarError::Hypervisor(result));
}
let &GetSetVpRegisterRequest { status, .. } = self.command_result::<_, u8>(0)?.0;
status.result().map_err(SidecarError::Hypervisor)?;
}
Ok(())
}
Expand All @@ -491,16 +488,14 @@ impl<'a> SidecarVp<'a> {
) -> Result<TranslateVirtualAddressExOutputX64, SidecarError> {
tracing::trace!("translate gva");
let &TranslateGvaResponse {
result,
status,
rsvd: _,
output,
} = self.dispatch_sync(
SidecarCommand::TRANSLATE_GVA,
TranslateGvaRequest { gvn, control_flags },
)?;
if result != HvError(0) {
return Err(SidecarError::Hypervisor(result));
}
status.result().map_err(SidecarError::Hypervisor)?;
Ok(output)
}

Expand Down
6 changes: 3 additions & 3 deletions openhcl/sidecar_defs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
use core::sync::atomic::AtomicU32;
use core::sync::atomic::AtomicU8;
use hvdef::hypercall::HvInputVtl;
use hvdef::HvError;
use hvdef::HvMessage;
use hvdef::HvStatus;
use open_enum::open_enum;
use zerocopy::AsBytes;
use zerocopy::FromBytes;
Expand Down Expand Up @@ -214,7 +214,7 @@ pub struct GetSetVpRegisterRequest {
/// Reserved.
pub rsvd: u8,
/// The hypervisor result.
pub result: HvError,
pub status: HvStatus,
/// Reserved.
pub rsvd2: [u8; 10],
/// Alignment field.
Expand Down Expand Up @@ -243,7 +243,7 @@ pub struct TranslateGvaRequest {
#[derive(Debug, Copy, Clone, AsBytes, FromBytes, FromZeroes)]
pub struct TranslateGvaResponse {
/// The hypervisor result.
pub result: HvError,
pub status: HvStatus,
/// Reserved.
pub rsvd: [u16; 7],
/// The output of the translation.
Expand Down
8 changes: 4 additions & 4 deletions vm/hv1/hv1_hypercall/src/support.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl<'a, T: HypercallIo> InnerDispatcher<'a, T> {
/// Complete hypercall handling.
fn complete(&mut self, output: Option<HypercallOutput>) {
if let Some(output) = output {
if output.call_status() == HvError::Timeout.0 {
if output.call_status() == Err(HvError::Timeout).into() {
self.handler.retry(
self.control
.with_rep_start(output.elements_processed())
Expand Down Expand Up @@ -282,7 +282,7 @@ impl<'a, T: HypercallIo> InnerDispatcher<'a, T> {
// which is handled as a failure), nothing is written back.
let output_end = if out_elem_size > 0 {
out_elem_size * ret.elements_processed()
} else if ret.call_status() == 0 {
} else if ret.call_status().is_ok() {
output_len
} else {
0
Expand Down Expand Up @@ -338,7 +338,7 @@ impl<'a, T: HypercallIo> InnerDispatcher<'a, T> {
// which is handled as a failure), nothing is written back.
let output_end = if out_elem_size > 0 {
out_elem_size * ret.elements_processed()
} else if ret.call_status() == 0 {
} else if ret.call_status().is_ok() {
output_len
} else {
0
Expand All @@ -354,7 +354,7 @@ impl<'a, T: HypercallIo> InnerDispatcher<'a, T> {
ret
};

if ret.call_status() == 0 {
if ret.call_status().is_ok() {
debug_assert_eq!(ret.elements_processed(), control.rep_count());
}

Expand Down
14 changes: 7 additions & 7 deletions vm/hv1/hv1_hypercall/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,13 +302,13 @@ impl From<TestResult> for HypercallOutput {
match result {
TestResult::Simple(SimpleResult::Success) => HypercallOutput::new(),
TestResult::Simple(SimpleResult::Failure(err)) => {
HypercallOutput::new().with_call_status(err.0)
HypercallOutput::new().with_call_status(Err(err).into())
}
TestResult::Rep(RepResult::Success(rep_count)) => {
HypercallOutput::new().with_elements_processed(rep_count)
}
TestResult::Rep(RepResult::Failure(err, rep_count)) => HypercallOutput::new()
.with_call_status(err.0)
.with_call_status(Err(err).into())
.with_elements_processed(rep_count),
_ => panic!("Should not be invoked for VTL"),
}
Expand Down Expand Up @@ -1371,7 +1371,7 @@ where
let mut io = io_gen(&mut handler);
let result = HypercallOutput::from(io.get_result());
let control = Control::from(io.control());
let call_status = HvError(result.call_status());
let call_status = result.call_status();

// Copy the output back. Note that in the case of errors the hypercall parser may not have
// actually modified the output buffers/registers, and it is the responsibility of the test
Expand All @@ -1382,8 +1382,8 @@ where
// that in cases where this routine does not modify the output, the hypercall parser does
// not do so either.
if is_timeout
|| (call_status != HvError::InvalidHypercallInput
&& call_status != HvError::InvalidAlignment)
|| (call_status != Err(HvError::InvalidHypercallInput).into()
&& call_status != Err(HvError::InvalidAlignment).into())
{
let mut output_buffer;
let (hdr, reps) = if !params.fast {
Expand Down Expand Up @@ -1559,7 +1559,7 @@ fn hypercall_simple(test_params: TestParams) {

check_test_result(&test_params, result, control);

let expected_output_size = if result.call_status() == 0 {
let expected_output_size = if result.call_status().is_ok() {
assert_eq!(
output.as_bytes(),
TestController::generate_test_output::<TestOutput>().as_bytes()
Expand Down Expand Up @@ -1779,7 +1779,7 @@ fn hypercall_variable(test_params: TestParams) {

check_test_result(&test_params, result, control);

let expected_output_size = if result.call_status() == 0 {
let expected_output_size = if result.call_status().is_ok() {
assert_eq!(
output.as_bytes(),
TestController::generate_test_output::<TestOutput>().as_bytes()
Expand Down
Loading