Re: [RISC-V] Switch to H-mode

[Bobby Eshleman <bobbyeshleman@xxxxxxxxx>]

On Mon, Jan 23, 2023 at 11:09:13PM +0000, Andrew Cooper wrote:
> On 19/01/2023 1:05 pm, Bobby Eshleman wrote:
> > On Mon, Jan 23, 2023 at 06:56:19PM +0200, Oleksii wrote:
> >> Hi Alistair and community,
> >>
> >> I am working on RISC-V support upstream for Xen based on your and Bobby
> >> patches.
> >>
> >> Adding the RISC-V support I realized that Xen is ran in S-mode. Output
> >> of OpenSBI:
> >>     ...
> >>     Domain0 Next Mode         : S-mode
> >>     ...
> >> So the first my question is shouldn't it be in H-mode?
> >>
> >> If I am right than it looks like we have to do a patch to OpenSBI to
> >> add support of H-mode as it is not supported now:
> >> [1]
> >> https://github.com/riscv-software-src/opensbi/blob/master/lib/sbi/sbi_domain.c#L380
> >> [2]
> >> https://github.com/riscv-software-src/opensbi/blob/master/include/sbi/riscv_encoding.h#L110
> >> Please correct me if I am wrong.
> >>
> >> The other option I see is to switch to H-mode in U-boot as I understand
> >> the classical boot flow is:
> >>     OpenSBI -> U-boot -> Xen -> Domain{0,...}
> >> If it is at all possible since U-boot will be in S mode after OpenSBI.
> >>
> >> Thanks in advance.
> >>
> >> ~ Oleksii
> >>
> > Ah, what you are seeing there is that the openSBI's Next Mode excludes
> > the virtualization mode (it treats HS and S synonymously) and it is only
> > used for setting the mstatus MPP. The code also has next_virt for
> > setting the MPV but I don't think that is exposed via the device tree
> > yet. For Xen, you'd want next_mode = PRIV_S and next_virt = 0 (HS mode,
> > not VS mode). The relevant setup prior to mret is here for interested
> > readers:
> > https://github.com/riscv-software-src/opensbi/blob/001106d19b21cd6443ae7f7f6d4d048d80e9ecac/lib/sbi/sbi_hart.c#L759
> >
> > As long as the next_mode and next_virt are set correctly, then Xen
> > should be launching in HS mode. I do believe this should be default for
> > the stock build too for Domain0, unless something has changed.
> 
> Ok, so everything ought to be doing the right thing, even if it doesn't
> show up clearly in the logging.
> 

Right.

> At some point, Xen is going to need a `if ( !hs-mode ) panic();`,
> because we can't operate dom0 properly if Xen is in plan S-mode.
> 
> I suggested that we try and make csr_read_safe() work, then try and read
> `hstatus` to probe if the H extension is active.
> 
> Does this sound reasonable, or is there a better option?
> 
> ~Andrew

That sounds reasonable to me.

The alternative is parsing the isa string from the dtb, which also seems
fine, but I'm not sure if it is better per se.

Best,
Bobby