| Message ID | 20240808082030.25940-1-yongxuan.wang@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] hw/intc/riscv_aplic: Fix APLIC in clrip and clripnum write emulation | expand |
Ccing Anup On 8/8/24 5:20 AM, Yong-Xuan Wang wrote: > In the section "4.7 Precise effects on interrupt-pending bits" > of the RISC-V AIA specification defines that: > > If the source mode is Level1 or Level0 and the interrupt domain > is configured in MSI delivery mode (domaincfg.DM = 1): > The pending bit is cleared whenever the rectified input value is > low, when the interrupt is forwarded by MSI, or by a relevant > write to an in clrip register or to clripnum. > > Update the riscv_aplic_set_pending() to match the spec. > This would need a Fixes: bf31cf06eb ("hw/intc/riscv_aplic: Fix setipnum_le write emulation for APLIC MSI-mode") > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > --- > hw/intc/riscv_aplic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c > index c1748c2d17d1..45d8b4089229 100644 > --- a/hw/intc/riscv_aplic.c > +++ b/hw/intc/riscv_aplic.c > @@ -247,7 +247,7 @@ static void riscv_aplic_set_pending(RISCVAPLICState *aplic, > > if ((sm == APLIC_SOURCECFG_SM_LEVEL_HIGH) || > (sm == APLIC_SOURCECFG_SM_LEVEL_LOW)) { > - if (!aplic->msimode || (aplic->msimode && !pending)) { > + if (!aplic->msimode) { The change you're doing here seems sensible to me but I'd rather have Anup take a look to see if this somehow undo what was made here in commit bf31cf06. In particular w.r.t this paragraph from section 4.9.2 of AIA: "A second option is for the interrupt service routine to write the APLIC’s source identity number for the interrupt to the domain’s setipnum register just before exiting. This will cause the interrupt’s pending bit to be set to one again if the source is still asserting an interrupt, but not if the source is not asserting an interrupt." Thanks, Daniel > return; > } > if ((aplic->state[irq] & APLIC_ISTATE_INPUT) &&
Hi Daniel, On Fri, Aug 9, 2024 at 5:40 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > Ccing Anup > > On 8/8/24 5:20 AM, Yong-Xuan Wang wrote: > > In the section "4.7 Precise effects on interrupt-pending bits" > > of the RISC-V AIA specification defines that: > > > > If the source mode is Level1 or Level0 and the interrupt domain > > is configured in MSI delivery mode (domaincfg.DM = 1): > > The pending bit is cleared whenever the rectified input value is > > low, when the interrupt is forwarded by MSI, or by a relevant > > write to an in clrip register or to clripnum. > > > > Update the riscv_aplic_set_pending() to match the spec. > > > > This would need a > > Fixes: bf31cf06eb ("hw/intc/riscv_aplic: Fix setipnum_le write emulation for APLIC MSI-mode") > I will add it into next version. Thank you! > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > > --- > > hw/intc/riscv_aplic.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c > > index c1748c2d17d1..45d8b4089229 100644 > > --- a/hw/intc/riscv_aplic.c > > +++ b/hw/intc/riscv_aplic.c > > @@ -247,7 +247,7 @@ static void riscv_aplic_set_pending(RISCVAPLICState *aplic, > > > > if ((sm == APLIC_SOURCECFG_SM_LEVEL_HIGH) || > > (sm == APLIC_SOURCECFG_SM_LEVEL_LOW)) { > > - if (!aplic->msimode || (aplic->msimode && !pending)) { > > + if (!aplic->msimode) { > > The change you're doing here seems sensible to me but I'd rather have Anup take > a look to see if this somehow undo what was made here in commit bf31cf06. > > In particular w.r.t this paragraph from section 4.9.2 of AIA: > > "A second option is for the interrupt service routine to write the > APLIC’s source identity number for the interrupt to the domain’s > setipnum register just before exiting. This will cause the interrupt’s > pending bit to be set to one again if the source is still asserting > an interrupt, but not if the source is not asserting an interrupt." > I think that this patch won't affect setipnum. For the setipnum case, the pending value would be true. And it is handled by the 2 if conditions below. Regards, Yong-Xuan > > Thanks, > > Daniel > > > > return; > > } > > if ((aplic->state[irq] & APLIC_ISTATE_INPUT) &&
ping On Fri, Aug 9, 2024 at 11:28 AM Yong-Xuan Wang <yongxuan.wang@sifive.com> wrote: > > Hi Daniel, > > On Fri, Aug 9, 2024 at 5:40 AM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: > > > > Ccing Anup > > > > On 8/8/24 5:20 AM, Yong-Xuan Wang wrote: > > > In the section "4.7 Precise effects on interrupt-pending bits" > > > of the RISC-V AIA specification defines that: > > > > > > If the source mode is Level1 or Level0 and the interrupt domain > > > is configured in MSI delivery mode (domaincfg.DM = 1): > > > The pending bit is cleared whenever the rectified input value is > > > low, when the interrupt is forwarded by MSI, or by a relevant > > > write to an in clrip register or to clripnum. > > > > > > Update the riscv_aplic_set_pending() to match the spec. > > > > > > > This would need a > > > > Fixes: bf31cf06eb ("hw/intc/riscv_aplic: Fix setipnum_le write emulation for APLIC MSI-mode") > > > > I will add it into next version. Thank you! > > > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > > > --- > > > hw/intc/riscv_aplic.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c > > > index c1748c2d17d1..45d8b4089229 100644 > > > --- a/hw/intc/riscv_aplic.c > > > +++ b/hw/intc/riscv_aplic.c > > > @@ -247,7 +247,7 @@ static void riscv_aplic_set_pending(RISCVAPLICState *aplic, > > > > > > if ((sm == APLIC_SOURCECFG_SM_LEVEL_HIGH) || > > > (sm == APLIC_SOURCECFG_SM_LEVEL_LOW)) { > > > - if (!aplic->msimode || (aplic->msimode && !pending)) { > > > + if (!aplic->msimode) { > > > > The change you're doing here seems sensible to me but I'd rather have Anup take > > a look to see if this somehow undo what was made here in commit bf31cf06. > > > > In particular w.r.t this paragraph from section 4.9.2 of AIA: > > > > "A second option is for the interrupt service routine to write the > > APLIC’s source identity number for the interrupt to the domain’s > > setipnum register just before exiting. This will cause the interrupt’s > > pending bit to be set to one again if the source is still asserting > > an interrupt, but not if the source is not asserting an interrupt." > > > > I think that this patch won't affect setipnum. For the setipnum case, > the pending value would > be true. And it is handled by the 2 if conditions below. > > Regards, > Yong-Xuan > > > > > Thanks, > > > > Daniel > > > > > > > return; > > > } > > > if ((aplic->state[irq] & APLIC_ISTATE_INPUT) &&
On Thu, Aug 8, 2024 at 1:51 PM Yong-Xuan Wang <yongxuan.wang@sifive.com> wrote: > > In the section "4.7 Precise effects on interrupt-pending bits" > of the RISC-V AIA specification defines that: > > If the source mode is Level1 or Level0 and the interrupt domain > is configured in MSI delivery mode (domaincfg.DM = 1): > The pending bit is cleared whenever the rectified input value is > low, when the interrupt is forwarded by MSI, or by a relevant > write to an in clrip register or to clripnum. > > Update the riscv_aplic_set_pending() to match the spec. Same comments at the kernel patch. https://lore.kernel.org/kvm/CAAhSdy3NmwbHY9Qef9LUeXfr0iE7wC-u0d_fHzC47PXk-MzmRg@mail.gmail.com/ Regards, Anup > > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> > --- > hw/intc/riscv_aplic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c > index c1748c2d17d1..45d8b4089229 100644 > --- a/hw/intc/riscv_aplic.c > +++ b/hw/intc/riscv_aplic.c > @@ -247,7 +247,7 @@ static void riscv_aplic_set_pending(RISCVAPLICState *aplic, > > if ((sm == APLIC_SOURCECFG_SM_LEVEL_HIGH) || > (sm == APLIC_SOURCECFG_SM_LEVEL_LOW)) { > - if (!aplic->msimode || (aplic->msimode && !pending)) { > + if (!aplic->msimode) { > return; > } > if ((aplic->state[irq] & APLIC_ISTATE_INPUT) && > -- > 2.17.1 > >
diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c index c1748c2d17d1..45d8b4089229 100644 --- a/hw/intc/riscv_aplic.c +++ b/hw/intc/riscv_aplic.c @@ -247,7 +247,7 @@ static void riscv_aplic_set_pending(RISCVAPLICState *aplic, if ((sm == APLIC_SOURCECFG_SM_LEVEL_HIGH) || (sm == APLIC_SOURCECFG_SM_LEVEL_LOW)) { - if (!aplic->msimode || (aplic->msimode && !pending)) { + if (!aplic->msimode) { return; } if ((aplic->state[irq] & APLIC_ISTATE_INPUT) &&
In the section "4.7 Precise effects on interrupt-pending bits" of the RISC-V AIA specification defines that: If the source mode is Level1 or Level0 and the interrupt domain is configured in MSI delivery mode (domaincfg.DM = 1): The pending bit is cleared whenever the rectified input value is low, when the interrupt is forwarded by MSI, or by a relevant write to an in clrip register or to clripnum. Update the riscv_aplic_set_pending() to match the spec. Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com> --- hw/intc/riscv_aplic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)