Hi, Richard

On 2022/4/18 上午11:15, Richard Henderson wrote:
On 4/15/22 02:40, Xiaojuan Yang wrote:
+static void pch_pic_update_irq(LoongArchPCHPIC *s, uint32_t mask,
+                               int level, int hi)
+{
+    uint32_t val, irq;
+
+    if (level == 1) {
+        if (hi) {
+            val = mask & s->intirr_hi & (~s->int_mask_hi);
+            irq = find_first_bit((unsigned long *)&val, 32);

This does not work -- you're accessing beyond the end of the uint32_t for all LP64 hosts.  I think you just want ctz32()...


+            if (irq != 32) {
+                s->intisr_hi |= 1ULL << irq;
+ qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 1);
+            }

... which should in fact only be tested if val != 0, which is to what this IF equates.

Is there a good reason that this function is treating hi and lo separately, as opposed to simply doing all of the computation on uint64_t?


In the part of linux kernel, pch pic driver use 32 bits for reading and writing. e.g in the drivers/irqchip/irq-loongson-pch-pic.c, pch_pic_mask_irq() function use writel() to write pch_pic mask reg.

Thanks.
Xiaojuan

r~

Reply via email to