Re: [Xen-devel] [PATCH RFC 02/35] xen: arm64: ACPI: Support common ACPI drivers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.02.15 at 18:36, <julien.grall@xxxxxxxxxx> wrote:
>> --- a/xen/drivers/acpi/osl.c
>> +++ b/xen/drivers/acpi/osl.c
>> @@ -96,7 +96,11 @@ acpi_os_map_memory(acpi_physical_address phys, acpi_size 
>> size)
>>                      return __va(phys);
>>              return __vmap(&pfn, PFN_UP(offs + size), 1, 1, 
>> PAGE_HYPERVISOR_NOCACHE) + offs;
>>      }
>> +#ifdef CONFIG_X86
>>      return __acpi_map_table(phys, size);
>> +#else
>> +    return __va(phys);
> 
> I remembered to have a discussion about this change with Naresh few 
> month ago.
> 
> __va should only be used when the memory is direct-mapped to Xen (i.e 
> accessible directly). On ARM64, this only the case for the RAM. Can you 
> confirm that ACPI will always reside to the RAM?
> 
> Futhermore, the code of this function seems x86-specific. The low 1MB 
> may not be mapped on ARM64.
> 
> I would move the whole function (acpi_os_map_memory) per-architecture.

You mean __acpi_map_table() I suppose. acpi_os_map_memory()
should remain here.

>> @@ -140,6 +145,7 @@ acpi_status acpi_os_write_port(acpi_io_address port, u32 
>> value, u32 width)
>>
>>      return AE_OK;
>>   }
>> +#endif
> 
> Why only x86? Linux seems to define it also for ARM64.

What is a port on ARM?

>> +#define ACPI_DISABLE_IRQS() local_irq_disable()
>> +#define ACPI_ENABLE_IRQS()  local_irq_enable()
>> +#define ACPI_FLUSH_CPU_CACHE() flush_cache_all()
>> +
>> +/* Blob handling macros */
>> +#define ACPI_BLOB_HEADER_SIZE   8
>> +
>> +/* Basic configuration for ACPI */
>> +#ifdef  CONFIG_ACPI
>> +extern int acpi_disabled;
>> +extern int acpi_noirq;
>> +extern int acpi_pci_disabled;
>> +extern int acpi_strict;
> 
> I think it would be better to define common external variable in a 
> common headers. Although I'm an ACPI maintainers...

I fully agree. I any such cleanup opportunities are noticed, they
should go into a prereq patch.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel