| Message ID | 20221021015307.2570844-3-yangxiaojuan@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | Fix LoongArch extioi coreisr accessing | expand |
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); https://lore.kernel.org/qemu-devel/f7c4f7ca-cbf9-87d6-4d8c-5957c36ae23c@linaro.org/ Regards, Phil.
在 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); > > 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.
在 2022/10/21 下午5:28, 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); >> >> 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. > Hi, Can we merge this patch ? or after Alex's series[1]? [1] https://lore.kernel.org/qemu-devel/20220927141504.3886314-2-alex.bennee@linaro.org/ Thanks. Song Gao
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}) + uint64_t helper_iocsrrd_b(CPULoongArchState *env, target_ulong r_addr) { return address_space_ldub(&env->address_space_iocsr, r_addr, - MEMTXATTRS_UNSPECIFIED, NULL); + GET_MEMTXATTRS(env), NULL); } uint64_t helper_iocsrrd_h(CPULoongArchState *env, target_ulong r_addr) { return address_space_lduw(&env->address_space_iocsr, r_addr, - MEMTXATTRS_UNSPECIFIED, NULL); + GET_MEMTXATTRS(env), NULL); } uint64_t helper_iocsrrd_w(CPULoongArchState *env, target_ulong r_addr) { return address_space_ldl(&env->address_space_iocsr, r_addr, - MEMTXATTRS_UNSPECIFIED, NULL); + GET_MEMTXATTRS(env), NULL); } uint64_t helper_iocsrrd_d(CPULoongArchState *env, target_ulong r_addr) { return address_space_ldq(&env->address_space_iocsr, r_addr, - MEMTXATTRS_UNSPECIFIED, NULL); + GET_MEMTXATTRS(env), NULL); } void helper_iocsrwr_b(CPULoongArchState *env, target_ulong w_addr, target_ulong val) { address_space_stb(&env->address_space_iocsr, w_addr, - val, MEMTXATTRS_UNSPECIFIED, NULL); + val, GET_MEMTXATTRS(env), NULL); } void helper_iocsrwr_h(CPULoongArchState *env, target_ulong w_addr, target_ulong val) { address_space_stw(&env->address_space_iocsr, w_addr, - val, MEMTXATTRS_UNSPECIFIED, NULL); + val, GET_MEMTXATTRS(env), NULL); } void helper_iocsrwr_w(CPULoongArchState *env, target_ulong w_addr, target_ulong val) { address_space_stl(&env->address_space_iocsr, w_addr, - val, MEMTXATTRS_UNSPECIFIED, NULL); + val, GET_MEMTXATTRS(env), NULL); } void helper_iocsrwr_d(CPULoongArchState *env, target_ulong w_addr, target_ulong val) { address_space_stq(&env->address_space_iocsr, w_addr, - val, MEMTXATTRS_UNSPECIFIED, NULL); + val, GET_MEMTXATTRS(env), NULL); }
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(-)