[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 2/4] xen/riscv: introduce setup_initial_pages
Hi Jan, On Mon, 2023-04-24 at 17:35 +0200, Jan Beulich wrote: > On 24.04.2023 17:16, Oleksii wrote: > > On Mon, 2023-04-24 at 12:18 +0200, Jan Beulich wrote: > > > On 21.04.2023 18:01, Oleksii wrote: > > > > On Thu, 2023-04-20 at 16:36 +0200, Jan Beulich wrote: > > > > > On 19.04.2023 17:42, Oleksii Kurochko wrote: > > > > > > + csr_write(CSR_SATP, > > > > > > + ((unsigned long)stage1_pgtbl_root >> > > > > > > PAGE_SHIFT) > > > > > > > > > > > > > + satp_mode << SATP_MODE_SHIFT); > > > > > > + > > > > > > + if ( (csr_read(CSR_SATP) >> SATP_MODE_SHIFT) == > > > > > > satp_mode > > > > > > ) > > > > > > + is_mode_supported = true; > > > > > > + > > > > > > + /* Clean MMU root page table and disable MMU */ > > > > > > + stage1_pgtbl_root[index] = paddr_to_pte(0x0, 0x0); > > > > > > + > > > > > > + csr_write(CSR_SATP, 0); > > > > > > + asm volatile("sfence.vma"); > > > > > > > > > > I guess what you do in this function could do with some more > > > > > comments. > > > > > Looks like you're briefly enabling the MMU to check that what > > > > > you > > > > > wrote > > > > > to SATP you can also read back. (Isn't there a register > > > > > reporting > > > > > whether the feature is available?) > > > > I supposed that it has to be but I couldn't find a register in > > > > docs. > > > > > > Well, yes, interestingly the register is marked WARL, so > > > apparently > > > intended to by used for probing like you do. (I find the > > > definition > > > of > > > WARL a little odd though, as such writes supposedly aren't > > > necessarily > > > value preserving. For SATP this might mean that translation is > > > enabled > > > by a write of an unsupported mode, with a different number of > > > levels. > > > This isn't going to work very well, I'm afraid.) > > Agree. It will be an issue in case of a different number of levels. > > > > Then it looks there is no way to check if SATP mode is supported. > > > > So we have to rely on the fact that the developer specified > > RV_STAGE1_MODE correctly in the config file. > > Well, maybe the spec could be clarified in this regard. That WARL > behavior may be okay for some registers, but as said I think it isn't > enough of a guarantee for SATP probing. Alistair, Bob - any thoughts? I've re-read the manual regarding CSR_SATP and the code of detecting SATP mode will work fine. >From the manual ( 4.1.11 Supervisor Address Translation and Protection (satp) Register ): “Implementations are not required to support all MODE settings, and if satp is written with an unsupported MODE, the entire write has no effect; no fields in satp are modified.” So there is no leave any open question so I'll provide new patch series. ~ Oleksii
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |