On 19/04/2022 02:50, yangxiaojuan wrote:
On 2022/4/18 下午4:57, Mark Cave-Ayland wrote:
On 18/04/2022 04:48, Richard Henderson wrote:
On 4/15/22 02:40, Xiaojuan Yang wrote:
+ memory_region_init(&s->mmio[cpu], OBJECT(s),
+ "loongarch_extioi", EXTIOI_SIZE);
+
+ memory_region_init_io(&s->mmio_nodetype[cpu], OBJECT(s),
+ &extioi_nodetype_ops, s,
+ EXTIOI_LINKNAME(.nodetype),
+ IPMAP_OFFSET - APIC_BASE);
+ memory_region_add_subregion(&s->mmio[cpu], 0, &s->mmio_nodetype[cpu]);
+
+ memory_region_init_io(&s->mmio_ipmap_enable[cpu], OBJECT(s),
+ &extioi_ipmap_enable_ops, s,
+ EXTIOI_LINKNAME(.ipmap_enable),
+ BOUNCE_OFFSET - IPMAP_OFFSET);
+ memory_region_add_subregion(&s->mmio[cpu], IPMAP_OFFSET - APIC_BASE,
+ &s->mmio_ipmap_enable[cpu]);
+
+ memory_region_init_io(&s->mmio_bounce_coreisr[cpu], OBJECT(s),
+ &extioi_bounce_coreisr_ops, s,
+ EXTIOI_LINKNAME(.bounce_coreisr),
+ COREMAP_OFFSET - BOUNCE_OFFSET);
+ memory_region_add_subregion(&s->mmio[cpu], BOUNCE_OFFSET - APIC_BASE,
+ &s->mmio_bounce_coreisr[cpu]);
+
+ memory_region_init_io(&s->mmio_coremap[cpu], OBJECT(s),
+ &extioi_coremap_ops, s,
+ EXTIOI_LINKNAME(.coremap),
+ EXTIOI_COREMAP_END);
+ memory_region_add_subregion(&s->mmio[cpu], COREMAP_OFFSET - APIC_BASE,
+ &s->mmio_coremap[cpu]);
Why are these separate memory regions, instead of one region? They're certainly
described in a single table in section 11.2 of the 3A5000 manual...
The reason it was done like this is because there were different access sizes in
the relevant _ops definitions. Certainly when I looked at the patches previously I
wasn't able to easily see how these could be consolidated without digging deeper
into the documentation.
Would it be better to keep consistent with the content of the 3A5000 manual
document? And only one memory region is used to represent the extioi iocsr region
The reason that different memory regions are required is because you've specified
different access size requirements for each region:
static const MemoryRegionOps extioi_nodetype_ops = {
.read = extioi_nodetype_readw,
.write = extioi_nodetype_writew,
.impl.min_access_size = 4,
.impl.max_access_size = 4,
.valid.min_access_size = 4,
.valid.max_access_size = 8,
.endianness = DEVICE_LITTLE_ENDIAN,
};
static const MemoryRegionOps extioi_ipmap_enable_ops = {
.read = extioi_ipmap_enable_read,
.write = extioi_ipmap_enable_write,
.impl.min_access_size = 1,
.impl.max_access_size = 1,
.valid.min_access_size = 1,
.valid.max_access_size = 8,
.endianness = DEVICE_LITTLE_ENDIAN,
};
static const MemoryRegionOps extioi_bounce_coreisr_ops = {
.read = extioi_bounce_coreisr_readw,
.write = extioi_bounce_coreisr_writew,
.impl.min_access_size = 4,
.impl.max_access_size = 4,
.valid.min_access_size = 4,
.valid.max_access_size = 8,
.endianness = DEVICE_LITTLE_ENDIAN,
};
static const MemoryRegionOps extioi_coremap_ops = {
.read = extioi_coremap_read,
.write = extioi_coremap_write,
.impl.min_access_size = 1,
.impl.max_access_size = 1,
.valid.min_access_size = 1,
.valid.max_access_size = 8,
.endianness = DEVICE_LITTLE_ENDIAN,
};
Certainly this is unusual (and for a given device I'd expect that its registers would
all have the same access requirements), but it's not something that can be tested
without access to real hardware.
As Richard suggests though, if it is found that all of the device registers have the
same access requirements then they could potentially be collapsed into a single
memory region.
ATB,
Mark.