[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] arm/acpi: Add __acpi_unmap_table function for ARM
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. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |