Hi Jan, On 2020/1/22 16:24, Jan Beulich wrote: > On 22.01.2020 06:57, Wei Xu wrote: >> On 2020/1/21 19:02, Jan Beulich wrote: >>> On 21.01.2020 10:49, Wei Xu wrote: >>>> --- a/xen/drivers/acpi/osl.c >>>> +++ b/xen/drivers/acpi/osl.c >>>> @@ -114,6 +114,8 @@ void acpi_os_unmap_memory(void __iomem * virt, >>>> acpi_size size) >>>> return; >>>> } >>>> >>>> + __acpi_unmap_table(virt, size); >>>> + >>>> if (system_state >= SYS_STATE_boot) >>>> vunmap((void *)((unsigned long)virt & PAGE_MASK)); >>> >>> How can it possibly be correct to call both vunmap() and your new >>> function? And how is this, having jsut an Arm implementation, >>> going to compile for x86? Seeing that x86 gets away without this, >>> may I suggest that you look at the x86 code to see why that is, >>> and then consider whether the same model makes sense for Arm? And >>> if it doesn't, check whether the new Arm model would make sense >>> to also use on x86? >>> >> >> Sorry, I thought acpi_os_unmap_memory is specific for ARM. >> Just now I checked map_pages_to_xen in arch/x86/mm.c and did not find any >> place >> to forbid the modification of a mapping. Maybe clearing mapping before >> modification >> is not necessary for X86. Do you think is it OK to add a empty stub function >> for the other cases except ARM and invoke it after vunmap as following? > > No. This is still doing two unmaps when system_state >= SYS_STATE_boot. > At the very least this need to go in an "else" block to the existing > if(). There also shouldn't be a blanket empty stub function. Even on > x86 it would be _better_ (albeit not strictly needed) if the unmap > indeed zapped the fixmap mappings. And for potential future ports it > would be outright dangerous to have such an empty stub.
Got it. I will check how to clear the fixmap on X86. Thanks! Best Regards, Wei > > Jan > > . > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
