On Thu, Feb 2, 2023 at 3:34 AM Bobby Eshleman <bobbyeshle...@gmail.com> wrote: > > On Wed, Feb 01, 2023 at 09:06:21AM +0000, Julien Grall wrote: > > Hi Andrew, > > > > On 01/02/2023 00:21, Andrew Cooper wrote: > > > On 31/01/2023 11:17 pm, Alistair Francis wrote: > > > > On Tue, Jan 31, 2023 at 10:03 PM Julien Grall <jul...@xen.org> wrote: > > > > > On 31/01/2023 11:44, Alistair Francis wrote: > > > > > > On Sat, Jan 28, 2023 at 12:15 AM Oleksii > > > > > > <oleksii.kuroc...@gmail.com> wrote: > > > > > > > > > > > From my understanding, on RISC-V, the use of PC-relative address is > > > > > only guaranteed with medany. So if you were going to change the cmodel > > > > > (Andrew suggested you would), then early_*() may end up to be broken. > > > > > > > > > > This check serve as a documentation of the assumption and also help > > > > > the > > > > > developer any change in the model and take the appropriate action to > > > > > remediate it. > > > > > > > > > > > I think this is safe to remove. > > > > > Based on what I wrote above, do you still think this is safe? > > > > With that in mind it's probably worth leaving in then. Maybe the > > > > comment should be updated to make it explicit why we want this check > > > > (I find the current comment not very helpful). > > > > > > The presence of this check pre-supposes that Xen will always relocate > > > itself, and this simply not true. XIP for example typically does not > > > > > > Furthermore it's not checking the property wanted. The way C is > > > compiled has no bearing on what relocation head.S uses to call it. > > > > I think we are still cross-talking each other because this is not my point. > > I am not sure how to explain differently... > > > > This check is not about how head.S call early_*() but making sure the C > > function can be executed with the MMU off. In which case, you will likely > > have VA != PA. > > > > In theory head.S could apply some relocations before hand, but it may be too > > complicated to do it before calling early_*(). > > > > > > > > It is a given that we compile the hypervisor with a consistent code > > > model, meaning that the properly actually making the check do what is > > > wanted is also the property that means it is not needed in the first > > > place. > > > > See above. > > > > > > > > This check is literally not worth the bytes it's taking up on disk, and > > > not worth the added compiler time, nor the wasted time of whoever comes > > > to support something like XIP in the future finds it to be in the way. > > > Xen as a whole will really genuinely function as intended in models > > > other than medany, and it demonstrates a misunderstanding of the topic > > > to put in such a check to begin with. > > > > Then enlight me :). So far it looks more like you are not understanding my > > point: I am talking about C function call with MMU off (e.g. VA != PA) and > > you are talking about Xen as a whole. > > > > I guess the only way to really know is to implement a different model. At > > which point there are three possible outcome: > > 1) We had the compiler check, it fired and the developper will take action > > to fix it (if needed). > > 2) We have no compiler check, the developper knew what to do it and fixed > > it. > > 3) We have no compiler check, the developper where not aware of the > > problem and we will waste time. > > > > I tend to favor the check because outcome #1 is so desirable, and I do > think that checking for medany is sufficient for the bulk of future > work. But that said, I do see Andrew's point now. > > If Xen were to a) not relocate itself, b) be executed under the 2GB > range, c) be compiled under medlow, and d) the MMU is off or on but Xen > is identity mapped, then C functions should still be safe to call in > early boot. Checking medany does protect developers from accidental bad > configuration now, but it is a somewhat imperfect proxy.
Yeah, I agree. It probably is worth leaving the check in for now, even if it's imperfect. We can always remove it in the future if required. Alistair > > One alternative that may be more long term is for the self relocation > code to be Kconfig-able and to require/force medany. Anyone wanting to > develop something like XIP could deselect relocation, which would allow > them to use medlow if they wanted/needed. The early C functions would > still be callable under both in this case. The help strings could offer > explanation too. > > Thanks, > Bobby > > > Even if you disagree with the check, then I think 1 is still the best > > because it would explain our assumption. Yes it may waste a few minutes to > > the developer switching the model. But that probably nothing compare to the > > time you could waste if you don't notice it. > > > > Anyway, Alistair has now all the information to take an informative > > decision. > > > > Cheers, > > > > -- > > Julien Grall > >
