[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 05/10] acpi: Refactor acpi_os_map_memory to be architecturally independent
>>> On 22.01.16 at 09:38, <zhaoshenglong@xxxxxxxxxx> wrote: > > On 2016/1/18 21:33, Jan Beulich wrote: >>>>> On 16.01.16 at 06:01, <zhaoshenglong@xxxxxxxxxx> 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; > +} Well, I had clearly said the vmap() part is generic; if there's anything architecture dependent here, then the mapping attributes (and hence only _those_ should be factored out, not the entire function invocation). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |