Re: [PATCH v8 2/2] hw/intc: Fix LoongArch extioi coreisr accessing

[yangxiaojuan]

在 2022/10/21 下午5:11, Philippe Mathieu-Daudé 写道:
> On 21/10/22 03:53, Xiaojuan Yang wrote:
> > 1. When cpu read or write extioi COREISR reg, it should access
> > the reg belonged to itself, so the cpu index of 's->coreisr'
> > is current cpu number. Using MemTxAttrs' requester_id to get
> > the cpu index.
> > 2. it need not to mask 0x1f when calculate the coreisr array index.
> >
> > Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> > ---
> >   hw/intc/loongarch_extioi.c      | 10 ++++++----
> >   target/loongarch/iocsr_helper.c | 19 +++++++++++--------
> >   2 files changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
> > index 72f4b0cde5..4b8ec3f28a 100644
> > --- a/hw/intc/loongarch_extioi.c
> > +++ b/hw/intc/loongarch_extioi.c
> > @@ -93,8 +93,9 @@ static MemTxResult extioi_readw(void *opaque, 
> > hwaddr addr, uint64_t *data,
> >           *data = s->bounce[index];
> >           break;
> >       case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
> > -        index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
> > -        cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
> > +        index = (offset - EXTIOI_COREISR_START) >> 2;
> > +        /* using attrs to get current cpu index */
> > +        cpu = attrs.requester_id;
> >           *data = s->coreisr[cpu][index];
> >           break;
> >       case EXTIOI_COREMAP_START ... EXTIOI_COREMAP_END - 1:
> > @@ -185,8 +186,9 @@ static MemTxResult extioi_writew(void *opaque, 
> > hwaddr addr,
> >           s->bounce[index] = val;
> >           break;
> >       case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
> > -        index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
> > -        cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
> > +        index = (offset - EXTIOI_COREISR_START) >> 2;
> > +        /* using attrs to get current cpu index */
> > +        cpu = attrs.requester_id;
> >           old_data = s->coreisr[cpu][index];
> >           s->coreisr[cpu][index] = old_data & ~val;
> >           /* write 1 to clear interrrupt */
> > diff --git a/target/loongarch/iocsr_helper.c 
> > b/target/loongarch/iocsr_helper.c
> > index 0e9c537dc7..505853e17b 100644
> > --- a/target/loongarch/iocsr_helper.c
> > +++ b/target/loongarch/iocsr_helper.c
> > @@ -14,54 +14,57 @@
> >   #include "exec/cpu_ldst.h"
> >   #include "tcg/tcg-ldst.h"
> >   +#define GET_MEMTXATTRS(cas) \
> > +        ((MemTxAttrs){.requester_id = env_cpu(cas)->cpu_index})
>
> The suggestion from v7 is incomplete, I apologize for missing it.
>
> #define GET_MEMTXATTRS(cas) ((MemTxAttrs) {\
>                                .requester_type = MTRT_CPU,\
>                                .requester_id = env_cpu(cas)->cpu_index,\
>                             })
>
> Also see from v6, add in the read/write handlers:
>
>             assert(attrs.requester_type == MTRT_CPU);
>
> f7c4f7ca-cbf9-87d6-4d8c-5957c36ae23c@linaro.org/">https://lore.kernel.org/qemu-devel/f7c4f7ca-cbf9-87d6-4d8c-5957c36ae23c@linaro.org/ 
hi,
we do not based on the 'MemTxAttrs requester_type patch' so far, and 
when that
patch merged we will apply it quickly.

Thanks.
Xiaojuan.