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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.01.2020 10:49, Wei Xu wrote:
> Add __acpi_unmap_table function for ARM and invoke it at acpi_os_unmap_memory
> to make sure the related fixmap has been cleared before using it for a
> different mapping.

How can it possibly be that this is needed for Arm only?

> --- a/xen/arch/arm/acpi/lib.c
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -49,6 +49,31 @@ char *__acpi_map_table(paddr_t phys, unsigned long size)
>       return ((char *) base + offset);
>   }
>   
> +void __acpi_unmap_table(void __iomem * virt, unsigned long size)
> +{
> +    unsigned long base, end;
> +    int idx;

unsigned int

> +    base = FIXMAP_ADDR(FIXMAP_ACPI_BEGIN);
> +    end = FIXMAP_ADDR(FIXMAP_ACPI_END);
> +
> +    if ( (unsigned long)virt < base || (unsigned long)virt > end )
> +    {
> +        return;
> +    }

Stray braces.

> --- 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?

> --- a/xen/include/xen/acpi.h
> +++ b/xen/include/xen/acpi.h
> @@ -68,6 +68,7 @@ typedef int (*acpi_table_entry_handler) (struct 
> acpi_subtable_header *header, co
>   
>   unsigned int acpi_get_processor_id (unsigned int cpu);
>   char * __acpi_map_table (paddr_t phys_addr, unsigned long size);
> +void __acpi_unmap_table(void __iomem * virt, unsigned long size);
>   int acpi_boot_init (void);
>   int acpi_boot_table_init (void);
>   int acpi_numa_init (void);

Best noticable here, your mailer has mangled the patch. The way
of this mangling makes me guess you used Thunderbird to send the
patch, in which case you'll need to adjust its settings (iirc it
was mailnews.wraplength which needed setting to zero).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel