On 11.01.2024 11:47, Elias El Yandouzi wrote: > On 22/12/2022 13:24, Jan Beulich wrote: >> That said, I think this change comes too early in the series, or there is >> something missing. > > At first, I had the same feeling but looking at the rest of the series, > I can see that the option is needed in follow-up patches. > >> As said in reply to patch 10, while there the mapcache >> is being initialized for the idle domain, I don't think it can be used >> just yet. Read through mapcache_current_vcpu() to understand why I think >> that way, paying particular attention to the ASSERT() near the end. > > Would be able to elaborate a bit more why you think that? I haven't been > able to get your point.
Why exactly I referred to the ASSERT() there I can't reconstruct. The function as a whole looks problematic though when suddenly the idle domain also gains a mapcache. I'm sorry, too much context was lost from over a year ago; all of this will need looking at from scratch again whenever a new version was posted. >> In preparation of this patch here I think the mfn_to_virt() uses have to all >> disappear from map_domain_page(). Perhaps yet more strongly all >> ..._to_virt() (except fix_to_virt() and friends) and __va() have to >> disappear up front from x86 and any code path which can be taken on x86 >> (which may simply mean purging all respective x86 #define-s, without >> breaking the build in any way). > > I agree with you on that one. I think it is what we're aiming for in the > long term. However, as mentioned by Julien in the cover letter, the > series's name is a misnomer and I am afraid we won't be able to remove > all of them with this series. These helpers would still be used for > xenheap pages or when the direct map is enabled. Leaving a hazard of certain uses not having been converted, or even overlooked in patches going in at around the same time as this series? I view this as pretty "adventurous". Jan
