Re: [PATCH v1 35/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[yangxiaojuan]

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
    
    Thanks.
    Xiaojuan
    
>       
>       ATB,
>       
>       
>       Mark.
>       
>