[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/4] xen/acpi: Rework acpi_os_map_memory() and acpi_os_unmap_memory()



Hi,

On 28/09/2020 11:39, Julien Grall wrote:


On 28/09/2020 11:09, Jan Beulich wrote:
On 28.09.2020 11:58, Julien Grall wrote:
On 28/09/2020 09:18, Jan Beulich wrote:
On 26.09.2020 22:55, Julien Grall wrote:
--- a/xen/arch/x86/acpi/lib.c
+++ b/xen/arch/x86/acpi/lib.c
@@ -46,6 +46,10 @@ char *__acpi_map_table(paddr_t phys, unsigned long size)
       if ((phys + size) <= (1 * 1024 * 1024))
           return __va(phys);
+    /* No arch specific implementation after early boot */
+    if (system_state >= SYS_STATE_boot)
+        return NULL;

Considering the code in context above, the comment isn't entirely
correct.

How about "No arch specific implementation after early boot but for the
first 1MB"?

That or simply "No further ...".

I will do that.

+{
+    unsigned long vaddr = (unsigned long)ptr;
+
+    if (vaddr >= DIRECTMAP_VIRT_START &&
+        vaddr < DIRECTMAP_VIRT_END) {
+        ASSERT(!((__pa(ptr) + size - 1) >> 20));
+        return true;
+    }
+
+    return (vaddr >= __fix_to_virt(FIX_ACPI_END)) &&
+        (vaddr < (__fix_to_virt(FIX_ACPI_BEGIN) + PAGE_SIZE));

Indentation is slightly wrong here.

This is Linux coding style and therefore is using hard tab. Care to
explain the problem?

The two opening parentheses should align with one another,
shouldn't they?

Hmmm... somehow vim wants to indent this way. I am not entirely sure why...

Looking at the Linux codebase this is the expected indentation. This is because the first ( on the first line is not closed and until the last ) on the second line.

So I will stick with this code.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.