Re: [PATCH v4 05/10] acpi: Refactor acpi_os_map_memory to be architecturally independent

[Shannon Zhao <zhaoshenglong@huawei.com>]

On 2016/1/18 21:33, Jan Beulich wrote:
>>>> On 16.01.16 at 06:01, <zhaoshenglong@huawei.com> wrote:
>> > --- a/xen/drivers/acpi/osl.c
>> > +++ b/xen/drivers/acpi/osl.c
>> > @@ -86,17 +86,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>> >  void __iomem *
>> >  acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
>> >  {
>> > -	if (system_state >= SYS_STATE_active) {
>> > -		mfn_t mfn = _mfn(PFN_DOWN(phys));
>> > -		unsigned int offs = phys & (PAGE_SIZE - 1);
>> > -
>> > -		/* The low first Mb is always mapped. */
>> > -		if ( !((phys + size - 1) >> 20) )
>> > -			return __va(phys);
>> > -		return __vmap(&mfn, PFN_UP(offs + size), 1, 1,
>> > -			      PAGE_HYPERVISOR_NOCACHE) + offs;
>> > -	}
>> > -	return __acpi_map_table(phys, size);
>> > +	return arch_acpi_os_map_memory(phys, size);
>> >  }
> I'm quite sure I've said before that this goes too far: The __vmap()
> part and likely also the early-boot __acpi_map_table() one already
> are architecture independent and hence should stay. The factoring
> hence should only concern the first Mb handling and maybe the
> the mapping attributes passed to __vmap().

Yes, the first MB handling and __vmap() should refactor. So if it only
moves them to an architecture function, how about below patch?

diff --git a/xen/arch/x86/acpi/lib.c b/xen/arch/x86/acpi/lib.c
index cc15ea3..5885a3a 100644
--- a/xen/arch/x86/acpi/lib.c
+++ b/xen/arch/x86/acpi/lib.c
@@ -33,6 +33,19 @@ u8 __read_mostly acpi_disable_value;
 u32 __read_mostly x86_acpiid_to_apicid[MAX_MADT_ENTRIES] =
     {[0 ... MAX_MADT_ENTRIES - 1] = BAD_APICID };

+void __iomem *
+arch_acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
+{
+       mfn_t mfn = _mfn(PFN_DOWN(phys));
+       unsigned int offs = phys & (PAGE_SIZE - 1);
+
+       /* The low first Mb is always mapped. */
+       if ( !((phys + size - 1) >> 20) )
+               return __va(phys);
+       return __vmap(&mfn, PFN_UP(offs + size), 1, 1,
+                     PAGE_HYPERVISOR_NOCACHE) + offs;
+}
+
 /*
  * Important Safety Note:  The fixed ACPI page numbers are *subtracted*
  * from the fixed base.  That's why we start at FIX_ACPI_END and
diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
index a2fc8c4..e2dda2e 100644
--- a/xen/drivers/acpi/osl.c
+++ b/xen/drivers/acpi/osl.c
@@ -88,16 +88,8 @@ acpi_physical_address __init
acpi_os_get_root_pointer(void)
 void __iomem *
 acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
 {
-       if (system_state >= SYS_STATE_active) {
-               mfn_t mfn = _mfn(PFN_DOWN(phys));
-               unsigned int offs = phys & (PAGE_SIZE - 1);
-
-               /* The low first Mb is always mapped. */
-               if ( !((phys + size - 1) >> 20) )
-                       return __va(phys);
-               return __vmap(&mfn, PFN_UP(offs + size), 1, 1,
-                             PAGE_HYPERVISOR_NOCACHE) + offs;
-       }
+       if (system_state >= SYS_STATE_active)
+               return arch_acpi_os_map_memory(phys, size);
        return __acpi_map_table(phys, size);
 }


-- 
Shannon