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

Re: [Xen-devel] [PATCH v2 05/15] xen/arm: segregate GIC low level functionality




Hello Vijaya,

On 04/04/14 12:56, vijay.kilari@xxxxxxxxx wrote:
+static void gic_send_sgi(const cpumask_t *cpumask, enum gic_sgi sgi)
  {
      unsigned int mask = 0;
      cpumask_t online_mask;
@@ -498,30 +606,26 @@ void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi 
sgi)
          | sgi;
  }

-void send_SGI_one(unsigned int cpu, enum gic_sgi sgi)
+void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi)
  {
-    ASSERT(cpu < NR_GIC_CPU_IF);  /* Targets bitmap only supports 8 CPUs */
-    send_SGI_mask(cpumask_of(cpu), sgi);
+   gic_hw_ops->send_sgi(cpumask, sgi);
  }

-void send_SGI_self(enum gic_sgi sgi)
+void send_SGI_one(unsigned int cpu, enum gic_sgi sgi)
  {
-    ASSERT(sgi < 16); /* There are only 16 SGIs */
-
-    dsb(sy);
-
-    GICD[GICD_SGIR] = GICD_SGI_TARGET_SELF
-        | sgi;
+    ASSERT(cpu < NR_GIC_CPU_IF);  /* Targets bitmap only supports 8 CPUs */
+    send_SGI_mask(cpumask_of(cpu), sgi);
  }

  void send_SGI_allbutself(enum gic_sgi sgi)
  {
-   ASSERT(sgi < 16); /* There are only 16 SGIs */
+    cpumask_t all_others_mask;
+    ASSERT(sgi < 16); /* There are only 16 SGIs */

-   dsb(sy);
+    cpumask_andnot(&all_others_mask, &cpu_possible_map, 
cpumask_of(smp_processor_id()));

-   GICD[GICD_SGIR] = GICD_SGI_TARGET_OTHERS
-       | sgi;
+    dsb(sy);
+    send_SGI_mask(&all_others_mask, sgi);
  }

As I said in V1, this change breaks SMP boot with GICv2...

GICD_SGI_TARGERT_OTHERS will send an SGI to every CPUs even if the CPU is not yet online (i.e. not registered by Xen). It's used during secondary boot (cpu_up_send_sgi).

Your solution won't work because send_SGI_mask only deal with online CPU.

All the changes of send_sgi is more than splitting functions... this should be moved on another patch and you should justify the modifications.

[..]

  int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
@@ -921,10 +1046,10 @@ out:
      return retval;
  }

-static void do_sgi(struct cpu_user_regs *regs, int othercpu, enum gic_sgi sgi)
+static void do_sgi(struct cpu_user_regs *regs, enum gic_sgi sgi)

Why did you drop the othercpu here?

Again, please justify every change on the prototype of every functions. If it's not trivial, split in small patches...

--
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®.