On 08.10.2024 12:26, [email protected] wrote: > On Fri, 2024-10-04 at 18:04 +0200, Oleksii Kurochko wrote: >> @@ -28,7 +29,21 @@ static inline void *maddr_to_virt(paddr_t ma) >> return NULL; >> } >> >> -#define virt_to_maddr(va) ({ BUG_ON("unimplemented"); 0; }) >> +static inline unsigned long virt_to_maddr(unsigned long va) >> +{ >> + ASSERT(va >= (DIRECTMAP_VIRT_START + DIRECTMAP_SIZE)); > It should be ASSERT(va < (...)). > > Then I can't use virt_to_maddr() instead of LINK_TO_LOAD() as > address from Xen's VA space ( XEN_VIRT_START ) are higher then > (DIRECTMAP_VIRT_START + DIRECTMAP_SIZE). > > Or as an option we could consider to drop this ASSERT() as if > VA is from directmap range the if below will catch that; otherwise > we have another one ASSERT() which checks that VA is from Xen VA range > where it is sage to use (phys_offset + va). > > Could we consider just dropping "ASSERT(va < (DIRECTMAP_VIRT_START + > DIRECTMAP_SIZE))" or I am missing something?
Counter question: Why did you put the ASSERT() there when - once corrected - it's actually pointless? What you want to make sure is that virt_to_maddr() can't be invoked with bad argument (without noticing). If that's achieved with just the other assertion (as it looks to be), then leaving out this assertion ought to be fine. Jan
