Re: [Xen-devel] [PATCH] arm/acpi: Add __acpi_unmap_table function for ARM

[Wei Xu <xuwei5@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel