| Message ID | 1458643613-22906-1-git-send-email-felix@linux.vnet.ibm.com |
|---|---|
| State | Rejected |
| Headers | show |
On Tue, 2016-03-22 at 11:46 +0100, Philippe Bergheaud wrote: > From: Vaibhav Jain <vaibhav@linux.vnet.ibm.com> > > Adds a 5ms wait to phb3_msi_set_xive after the interrupt is masked so > that the kernel delays cleanup until an irq if its in-flight is > handled. The value 5ms is the worst case time needed by an irq to be > presented to the host after its generated. I don't think we can do this here. We can't have firmware take a CPU for 5ms. I think we need to do this workaround in Linux. Mikey > > Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com> > --- > This patch requires the following patches: > https://patchwork.ozlabs.org/patch/581764/ > https://patchwork.ozlabs.org/patch/581765/ > > hw/phb3.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/phb3.c b/hw/phb3.c > index fbdcb9e..e5d49b2 100644 > --- a/hw/phb3.c > +++ b/hw/phb3.c > @@ -1751,6 +1751,8 @@ static int64_t phb3_msi_set_xive(void *data, > PHB_IVC_UPDATE_ENABLE_Q | > PHB_IVC_UPDATE_ENABLE_GEN; > out_be64(p->regs + PHB_IVC_UPDATE, ivc); > + /* wait for 5ms before signalling the interrupt is masked */ This would need a longer explanation as to why. > + time_wait_ms(5); > } > > return OPAL_SUCCESS;
Michael Neuling <mikey@neuling.org> writes: > On Tue, 2016-03-22 at 11:46 +0100, Philippe Bergheaud wrote: >> From: Vaibhav Jain <vaibhav@linux.vnet.ibm.com> >> >> Adds a 5ms wait to phb3_msi_set_xive after the interrupt is masked so >> that the kernel delays cleanup until an irq if its in-flight is >> handled. The value 5ms is the worst case time needed by an irq to be >> presented to the host after its generated. > > I don't think we can do this here. We can't have firmware > take a CPU for 5ms. > > I think we need to do this workaround in Linux. 100% agree. Before we'd go and do something as drastic as this (OPAL calls should/must not block), we'd need a detailed reason as to why literally every other option is a worse idea. A 5ms sleep at 4Ghz is about 20 million cycles, and at 1IPC that's about half the number of instructions it takes to boot skiboot+Linux in a single core simulator. Wasting that much time doing nothing in firmware is a last resort, as we can't go and do any useful work while spinning doing nothing.
diff --git a/hw/phb3.c b/hw/phb3.c index fbdcb9e..e5d49b2 100644 --- a/hw/phb3.c +++ b/hw/phb3.c @@ -1751,6 +1751,8 @@ static int64_t phb3_msi_set_xive(void *data, PHB_IVC_UPDATE_ENABLE_Q | PHB_IVC_UPDATE_ENABLE_GEN; out_be64(p->regs + PHB_IVC_UPDATE, ivc); + /* wait for 5ms before signalling the interrupt is masked */ + time_wait_ms(5); } return OPAL_SUCCESS;