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

Re: [Xen-devel] [PATCH RFC 11/35] ARM64 / ACPI: Parse MADT to map logical cpu to MPIDR and get cpu_possible/present_map



Hi Parth,

On 04/02/2015 14:01, parth.dixit@xxxxxxxxxx wrote:
From: Naresh Bhat <naresh.bhat@xxxxxxxxxx>

MADT contains the information for MPIDR which is essential for
SMP initialization, parse the GIC cpu interface structures to
get the MPIDR value and map it to cpu_logical_map(), and add
enabled cpu with valid MPIDR into cpu_possible_map and
cpu_present_map.

Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx>
Signed-off-by: Tomasz Nowicki <tomasz.nowicki@xxxxxxxxxx>
Signed-off-by: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
---
  xen/arch/arm/arm64/acpi/arm-core.c | 139 +++++++++++++++++++++++++++++++++++++
  xen/include/asm-arm/acpi.h         |   2 +
  xen/include/xen/acpi.h             |   5 ++
  3 files changed, 146 insertions(+)

diff --git a/xen/arch/arm/arm64/acpi/arm-core.c 
b/xen/arch/arm/arm64/acpi/arm-core.c
index 2b7e2ef..84b0032 100644
--- a/xen/arch/arm/arm64/acpi/arm-core.c
+++ b/xen/arch/arm/arm64/acpi/arm-core.c
@@ -26,7 +26,10 @@
  #include <xen/init.h>
  #include <xen/acpi.h>
  #include <xen/errno.h>
+#include <xen/stdbool.h>
+#include <xen/cpumask.h>

+#include <asm/cputype.h>
  #include <asm/acpi.h>

  /*
@@ -49,10 +52,141 @@ int acpi_psci_present;
  /* 1 to indicate HVC must be used instead of SMC as the PSCI conduit */
  int acpi_psci_use_hvc;

+/* available_cpus means enabled cpu in MADT */
+static int available_cpus;
+

Why adding available_cpus here, and total_cpus a bit below?

  enum acpi_irq_model_id acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM;

  struct acpi_arm_root acpi_arm_rsdp_info;     /* info about RSDP from FDT */

+/* arch-optional setting to enable display of offline cpus >= nr_cpu_ids */
+unsigned int total_cpus = 0;

Set but never used, neither exported...

+/*
+ * acpi_register_gic_cpu_interface - register a gic cpu interface and
+ * generates a logic cpu number
+ * @mpidr: CPU's hardware id to register, MPIDR represented in MADT
+ * @enabled: this cpu is enabled or not
+ *
+ * Returns the logic cpu number which maps to the gic cpu interface
+ */
+static int acpi_register_gic_cpu_interface(u64 mpidr, u8 enabled)
+{
+    int cpu;
+
+    if ( mpidr == INVALID_HWID )
+    {
+        printk("Skip invalid cpu hardware ID\n");
+        return -EINVAL;
+    }
+
+    total_cpus++;
+    if ( !enabled )
+        return -EINVAL;
+
+    if ( available_cpus >=  NR_CPUS )
+    {
+        printk("NR_CPUS limit of %d reached, Processor %d/0x%llx ignored.\n",
+                            NR_CPUS, total_cpus, (long long unsigned 
int)mpidr);

Why the (long long unsigned int) cast? Please use PRIx64.

+        return -EINVAL;
+    }
+
+    /* If it is the first CPU, no need to check duplicate MPIDRs */
+    if ( !available_cpus )
+        goto skip_mpidr_check;

I don't think it's necessary, for_each_present_cpu should not contain the first CPU...

+
+    /*
+    * Duplicate MPIDRs are a recipe for disaster. Scan
+    * all initialized entries and check for
+    * duplicates. If any is found just ignore the CPU.
+    */
+    for_each_present_cpu(cpu)
+    {
+        if ( cpu_logical_map(cpu) == mpidr )
+        {
+            printk("Firmware bug, duplicate CPU MPIDR: 0x%llx in MADT\n",
+                (long long unsigned int)mpidr);

Ditto for the cast.

+            return -EINVAL;
+        }
+    }
+
+skip_mpidr_check:
+    available_cpus++;
+
+    /* allocate a logic cpu id for the new comer */
+    if ( cpu_logical_map(0) == mpidr )
+    {
+        /*
+         * boot_cpu_init() already hold bit 0 in cpu_present_mask
+         * for BSP, no need to allocte again.

allocate

+         */
+        cpu = 0;
+    }
+    else
+        cpu = cpumask_next_zero(-1, &cpu_present_map);
+
+    /* map the logic cpu id to cpu MPIDR */
+    cpu_logical_map(cpu) = mpidr;
+
+    set_cpu_possible(cpu, true);
+    set_cpu_present(cpu, true);
+
+    return cpu;
+}
+
+static int __init
+acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
+                               const unsigned long end)
+{
+    struct acpi_madt_generic_interrupt *processor;
+
+    processor = (struct acpi_madt_generic_interrupt *)header;
+
+    if ( BAD_MADT_ENTRY(processor, end) )
+        return -EINVAL;
+
+    acpi_table_print_madt_entry(header);
+
+    acpi_register_gic_cpu_interface(processor->mpidr,
+        processor->flags & ACPI_MADT_ENABLED);
+
+    return 0;
+}
+
+/*
+ * Parse GIC cpu interface related entries in MADT
+ * returns 0 on success, < 0 on error
+ */
+static int __init acpi_parse_madt_gic_cpu_interface_entries(void)
+{
+    int count;
+
+    /*
+     * do a partial walk of MADT to determine how many CPUs
+     * we have including disabled CPUs, and get information
+     * we need for SMP init
+     */
+    count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
+        acpi_parse_gic_cpu_interface, MAX_GIC_CPU_INTERFACE);
+
+    if ( !count )
+    {
+        printk("No GIC CPU interface entries present\n");
+        return -ENODEV;
+    }
+    else if ( count < 0 )
+    {
+        printk("Error parsing GIC CPU interface entry\n");
+        return count;
+    }
+
+    /* Make boot-up look pretty */
+    printk("%d CPUs available, %d CPUs total\n", available_cpus,
+        total_cpus);
+
+    return 0;
+}
+
  int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
  {
      *irq = -1;
@@ -140,6 +274,11 @@ int __init acpi_boot_init(void)
      if ( err )
          printk("Can't find FADT\n");

If we don't find the FADT, we should give up rather than trying to parse the GIC ACPI table.


+    /* Get the boot CPU's MPIDR before MADT parsing */
+    cpu_logical_map(0) = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
+
+    err = acpi_parse_madt_gic_cpu_interface_entries();
+
      return err;
  }
  #endif
diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
index 03051ef..c2d25db 100644
--- a/xen/include/asm-arm/acpi.h
+++ b/xen/include/asm-arm/acpi.h
@@ -105,4 +105,6 @@ static inline void acpi_disable_pci(void)
  #define acpi_strict 1   /* no ACPI spec workarounds on ARM */
  #endif

+#define MAX_GIC_CPU_INTERFACE 65535
+
  #endif /*_ASM_ARM_ACPI_H*/
diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
index ff96336..9387b36 100644
--- a/xen/include/xen/acpi.h
+++ b/xen/include/xen/acpi.h
@@ -67,6 +67,11 @@ typedef int (*acpi_table_handler) (struct acpi_table_header 
*table);
  typedef int (*acpi_table_entry_handler) (struct acpi_subtable_header *header, 
const unsigned long end);

  unsigned int acpi_get_processor_id (unsigned int cpu);
+
+#define BAD_MADT_ENTRY(entry, end) (                                        \
+                (!entry) || (unsigned long)entry + sizeof(*entry) > end ||  \
+                ((struct acpi_subtable_header *)entry)->length < 
sizeof(*entry))
+

IHMO, this should belong to a separate patch.

  char * __acpi_map_table (paddr_t phys_addr, unsigned long size);
  int acpi_boot_init (void);
  int acpi_boot_table_init (void);


Regards,

--
Julien Grall

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


 


Rackspace

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