On 27/4/25 09:09, bibo mao wrote:
On 2025/4/25 下午5:53, Philippe Mathieu-Daudé wrote:
On 24/3/25 10:37, Bibo Mao wrote:
For memory region iomem32_low, generic read callback is used.
Signed-off-by: Bibo Mao <maob...@loongson.cn>
---
hw/intc/loongarch_pch_pic.c | 71 +++++++++++++++++++++++++------------
1 file changed, 48 insertions(+), 23 deletions(-)
diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
index 10b4231464..b495bd3a4d 100644
--- a/hw/intc/loongarch_pch_pic.c
+++ b/hw/intc/loongarch_pch_pic.c
@@ -7,6 +7,7 @@
#include "qemu/osdep.h"
#include "qemu/bitops.h"
+#include "qemu/log.h"
#include "hw/irq.h"
#include "hw/intc/loongarch_pch_pic.h"
#include "trace.h"
@@ -71,47 +72,71 @@ static void pch_pic_irq_handler(void *opaque, int
irq, int level)
pch_pic_update_irq(s, mask, level);
}
-static uint64_t loongarch_pch_pic_low_readw(void *opaque, hwaddr addr,
- unsigned size)
+static uint64_t pch_pic_read(void *opaque, hwaddr addr, uint64_t
field_mask)
{
LoongArchPICCommonState *s = LOONGARCH_PIC_COMMON(opaque);
uint64_t val = 0;
+ uint32_t offset = addr & 7;
switch (addr) {
- case PCH_PIC_INT_ID:
- val = s->id.data & UINT_MAX;
+ case PCH_PIC_INT_ID ... PCH_PIC_INT_ID + 7:
+ val = s->id.data;
break;
- case PCH_PIC_INT_ID + 4:
- val = s->id.data >> 32;
+ case PCH_PIC_INT_MASK ... PCH_PIC_INT_MASK + 7:
+ val = s->int_mask;
break;
- case PCH_PIC_INT_MASK:
- val = (uint32_t)s->int_mask;
+ case PCH_PIC_INT_EDGE ... PCH_PIC_INT_EDGE + 7:
+ val = s->intedge;
break;
- case PCH_PIC_INT_MASK + 4:
- val = s->int_mask >> 32;
+ case PCH_PIC_HTMSI_EN ... PCH_PIC_HTMSI_EN + 7:
+ val = s->htmsi_en;
break;
- case PCH_PIC_INT_EDGE:
- val = (uint32_t)s->intedge;
+ case PCH_PIC_AUTO_CTRL0 ... PCH_PIC_AUTO_CTRL0 + 7:
+ case PCH_PIC_AUTO_CTRL1 ... PCH_PIC_AUTO_CTRL1 + 7:
+ /* PCH PIC connect to EXTIOI always, discard auto_ctrl
access */
break;
- case PCH_PIC_INT_EDGE + 4:
- val = s->intedge >> 32;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "pch_pic_read: Bad address 0x%"PRIx64"\n", addr);
break;
- case PCH_PIC_HTMSI_EN:
- val = (uint32_t)s->htmsi_en;
+ }
+
+ return (val >> (offset * 8)) & field_mask;
Maybe you want to simplify from a different angle:
--- a/hw/intc/loongarch_pch_pic.c
+++ b/hw/intc/loongarch_pch_pic.c
@@ -320,8 +320,7 @@ static const MemoryRegionOps
loongarch_pch_pic_reg32_low_ops = {
.max_access_size = 8,
},
.impl = {
- .min_access_size = 4,
- .max_access_size = 4,
+ .min_access_size = 8,
},
.endianness = DEVICE_LITTLE_ENDIAN,
};
I do not follow this, do you mean something like this?
.impl = {
- .min_access_size = 4,
- .max_access_size = 4,
+ .min_access_size = 1,
+ .max_access_size = 8,
},
Since this driver is used by KVM, performance issue need be considered.
We are in slow path anyway, I doubt performance is measurable, you are
doing the same maths manually before returning.
Anyway I'm not insisting.
For normal aligned 1/2/4/8 bytes access, it had better be accessed once
rather than concatenated with 1 byte access for many times.
Regards
Bibo Mao
