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

Re: [Minios-devel] [UNIKRAFT PATCHv5 3/6] plat/common: Implement gic-v2 library for Arm





On 7/9/19 7:43 PM, Julien Grall wrote:
Hi,

On 7/9/19 6:31 PM, Sharan Santhanam wrote:
+static void gic_init_dist(void)
+{
+    uint32_t val, cpuif_number, irq_number;
+    uint32_t i;
+
+    /* Turn down distributor */
+    gic_disable_dist();
+
+    /* Get GIC CPU interface */
+    val = read_gicd32(GICD_TYPER);
+    cpuif_number = GICD_TYPER_CPUI_NUM(val);
+    if (cpuif_number > GIC_MAX_CPUIF)
+        cpuif_number = GIC_MAX_CPUIF;
+    uk_pr_info("GICv2 Max CPU interface:%d\n", cpuif_number);
+
+    /* Get the maximum number of interrupts that the GIC supports */
+    irq_number = GICD_TYPER_LINE_NUM(val);
+    if (irq_number > GIC_MAX_IRQ)
+        irq_number = GIC_MAX_IRQ;
+    uk_pr_info("GICv2 Max interrupt lines:%d\n", irq_number);
+    /*
+     * Set all SPI interrupts targets to all CPU.
+     */
+    for (i = GIC_SPI_BASE; i < irq_number; i += GICD_I_PER_ITARGETSRn)
+        write_gicd32(GICD_ITARGETSR(i), GICD_ITARGETSR_DEF);
+
+    /*
+     * Set all SPI interrupts type to be level triggered
+     */
+    for (i = GIC_SPI_BASE; i < irq_number; i += GICD_I_PER_ICFGRn)
+        write_gicd32(GICD_ICFGR(i), GICD_ICFGR_DEF_TYPE);
+
+    /*
+     * Set all SPI priority to a default value.
+     */
+    for (i = GIC_SPI_BASE; i < irq_number; i += GICD_I_PER_IPRIORITYn)
+        write_gicd32(GICD_IPRIORITYR(i), GICD_IPRIORITY_DEF);
+
+    /*
+     * Deactivate and disable all SPIs.
+     */
+    for (i = GIC_SPI_BASE; i < irq_number; i += GICD_I_PER_ICACTIVERn) {
+        write_gicd32(GICD_ICACTIVER(i), GICD_DEF_ICACTIVERn);
+        write_gicd32(GICD_ICENABLER(i), GICD_DEF_ICENABLERn);
+    }
+
+    /* turn on distributor */
+    gic_enable_dist();
+}
+



This function definition assume single CPU which is fine for now. Maybe, we could add TODO stating we need to extend the function cpu interface.

The function gic_init_dist() is initializing information shared between all the CPUs.

For gic_init_cpuif() this needs to be called by the CPU itself as the interface is banked.
I misread the spec, my bad. I agree.


+static void gic_init_cpuif(void)
+{
+    uint32_t i;
+    /*
+     * set priority mask to the lowest priority to let all irq
+     * visible to cpu interface
+     */
+    gic_set_threshold_priority(GICC_PMR_PRIO_MAX);
+


Is this operation valid? From the GIC specification v2 these


register are read only register and PPI register


are implementation defined. Atleast the GICD_ICFGR0 is readonly.

I am a bit confused... Is this comment applied to the code below? If so, it is common code comment below the code rather than above as this is counter-intuitive.

Yes, it applies to the code below. pg. 109 of [1], under usage constraints.

Sorry, I will add the comment below the code from now on.


[1] https://static.docs.arm.com/ihi0048/bb/IHI0048B_b_gic_architecture_specification.pdf?_ga=2.72292506.847879194.1562677153-1998449976.1551706994
Cheers,


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

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