rivosinc / salus

Risc-V hypervisor for TEE development
92 stars 25 forks source link

Vm: Add support to probe Nacl features. #295

Closed rajnesh-kanwal closed 1 year ago

rajnesh-kanwal commented 1 year ago

In the recent updates to Nacl extension [0], feature probing was introduced and allows guest hypervisor to probe which features are supported by the host hypervisor.

Also, the SetShmem FID argument has changed from pfn to full address of the shared memory page.

[0] https://lists.riscv.org/g/sig-hypervisors/message/256

rajnesh-kanwal commented 1 year ago

probably need to update tellus too?

tellus uses the register_shmem function in sbi-rs which takes in shmem_ptr and converts it to pfn, with register_shmem updated tellus doesn't need any changes.

I can add a test for ProbeFeature if that's what you are suggesting.

abrestic-rivos commented 1 year ago

Ah, I was looking at the CI failure (which is because Atish's PR isn't in yet).

atishp04 commented 1 year ago

I am thinking to add CoVE related features which can be probed as well.

  1. SBI_NACL_FEAT_COVE_GPR -- This will describe the the scratch layout for GPRs

  2. SBI_NACL_FEAT_COVE_CSR

This will align both specs more cleanly and clearly describe where CoVE supersedes NACL description. It will also allow other TSM/host hypervisor implementations to not use the NACL features if they wish to. It allows extensibility for any possible future use case as well.

Any thoughts ? This can be done a separate PR as spec change also need to drafted.

abrestic-rivos commented 1 year ago

I am thinking to add CoVE related features which can be probed as well.

Doesn't probe just say if those features are present? i.e. if we want to know whether or not the shmem scratch area has the COVE layout, don't we need just one "feature" that indicates we have COVE?

rajnesh-kanwal commented 1 year ago

I have pointed sbi-rs to https://github.com/rivosinc/sbi-rs/commit/9b83bc49608177d092848f41880064feef5886fd.

atishp04 commented 1 year ago

I am thinking to add CoVE related features which can be probed as well.

Doesn't probe just say if those features are present? i.e. if we want to know whether or not the shmem scratch area has the COVE layout, don't we need just one "feature" that indicates we have COVE?

One will work too. I was suggesting two that aligns better in terms of current spec alignment (scratch area/ CSR area). We can describe the scratch & CSR access exception under the same feature.