* [PATCH] crypto: sun4i-ss: prevent deadlock on emulated hardware @ 2018-06-14 19:36 Corentin Labbe 2018-06-15 7:57 ` Maxime Ripard 0 siblings, 1 reply; 6+ messages in thread From: Corentin Labbe @ 2018-06-14 19:36 UTC (permalink / raw) To: davem, herbert, maxime.ripard, wens Cc: linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi, Corentin Labbe Running a qemu emulated cubieboard with sun4i-ss driver enabled led to a never ending boot. This is due to sun4i-ss deadlocked and taking all cpu in an infinite loop. Since the crypto hardware is not implemented, all registers are read as 0. So sun4i-ss will never progress in any operations. (TX_CNT being always 0) The first idea is to add a "TX_CNT always zero timeout" but this made cipher/hash loops more complex and prevent a case that never happen on real hardware. The best way to fix is to check at probe time if we run on a virtual machine with hardware emulated but non-implemented and prevent sun4i-ss to be loaded in that case. Letting sun4i-ss to load is useless anyway since all crypto algorithm will be disabled since they will fail crypto selftests. Tested-on: qemu-cubieboard Tested-on: cubieboard2 Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> --- drivers/crypto/sunxi-ss/sun4i-ss-core.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c index a81d89b3b7d8..a178e80adcf3 100644 --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c @@ -341,9 +341,18 @@ static int sun4i_ss_probe(struct platform_device *pdev) * I expect to be a sort of Security System Revision number. * Since the A80 seems to have an other version of SS * this info could be useful + * Detect virtual machine with non-implemented hardware + * (qemu-cubieboard) by checking the register value after a write to it. + * On non-implemented hardware, all registers are read as 0. + * On real hardware we should have a value > 0. */ writel(SS_ENABLED, ss->base + SS_CTL); v = readl(ss->base + SS_CTL); + if (!v) { + dev_err(&pdev->dev, "Qemu with non-implemented SS detected.\n"); + err = -ENODEV; + goto error_rst; + } v >>= 16; v &= 0x07; dev_info(&pdev->dev, "Die ID %d\n", v); @@ -398,6 +407,7 @@ static int sun4i_ss_probe(struct platform_device *pdev) break; } } +error_rst: if (ss->reset) reset_control_assert(ss->reset); error_clk: -- 2.16.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: sun4i-ss: prevent deadlock on emulated hardware 2018-06-14 19:36 [PATCH] crypto: sun4i-ss: prevent deadlock on emulated hardware Corentin Labbe @ 2018-06-15 7:57 ` Maxime Ripard 2018-06-15 8:15 ` Corentin Labbe 0 siblings, 1 reply; 6+ messages in thread From: Maxime Ripard @ 2018-06-15 7:57 UTC (permalink / raw) To: Corentin Labbe Cc: davem, herbert, wens, linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 2759 bytes --] On Thu, Jun 14, 2018 at 09:36:59PM +0200, Corentin Labbe wrote: > Running a qemu emulated cubieboard with sun4i-ss driver enabled led to a never > ending boot. > This is due to sun4i-ss deadlocked and taking all cpu in an infinite loop. > Since the crypto hardware is not implemented, all registers are read as 0. > So sun4i-ss will never progress in any operations. (TX_CNT being always 0) > > The first idea is to add a "TX_CNT always zero timeout" but this made cipher/hash loops > more complex and prevent a case that never happen on real hardware. > > The best way to fix is to check at probe time if we run on a virtual > machine with hardware emulated but non-implemented and prevent > sun4i-ss to be loaded in that case. > Letting sun4i-ss to load is useless anyway since all crypto algorithm will be > disabled since they will fail crypto selftests. > > Tested-on: qemu-cubieboard > Tested-on: cubieboard2 > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > --- > drivers/crypto/sunxi-ss/sun4i-ss-core.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > index a81d89b3b7d8..a178e80adcf3 100644 > --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > @@ -341,9 +341,18 @@ static int sun4i_ss_probe(struct platform_device *pdev) > * I expect to be a sort of Security System Revision number. > * Since the A80 seems to have an other version of SS > * this info could be useful > + * Detect virtual machine with non-implemented hardware > + * (qemu-cubieboard) by checking the register value after a write to it. > + * On non-implemented hardware, all registers are read as 0. > + * On real hardware we should have a value > 0. > */ > writel(SS_ENABLED, ss->base + SS_CTL); > v = readl(ss->base + SS_CTL); > + if (!v) { > + dev_err(&pdev->dev, "Qemu with non-implemented SS detected.\n"); > + err = -ENODEV; > + goto error_rst; > + } This is wrong way to tackle the issue. There's multiple reason why this could happen (for example the device not being clocked, or maintained in reset). There's nothing specific about qemu here, and the fundamental issue isn't that the device isn't functional in qemu, it's that qemu lies about which hardware it can emulate in the DT it passes to the kernel. There's no way this can scale, alone from the fact that qemu should patch the DT according to what it can do. Not trying to chase after each and every device that is broken in qemu. NAK. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: sun4i-ss: prevent deadlock on emulated hardware 2018-06-15 7:57 ` Maxime Ripard @ 2018-06-15 8:15 ` Corentin Labbe 2018-06-15 9:04 ` Maxime Ripard 0 siblings, 1 reply; 6+ messages in thread From: Corentin Labbe @ 2018-06-15 8:15 UTC (permalink / raw) To: Maxime Ripard Cc: davem, herbert, wens, linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi On Fri, Jun 15, 2018 at 09:57:54AM +0200, Maxime Ripard wrote: > On Thu, Jun 14, 2018 at 09:36:59PM +0200, Corentin Labbe wrote: > > Running a qemu emulated cubieboard with sun4i-ss driver enabled led to a never > > ending boot. > > This is due to sun4i-ss deadlocked and taking all cpu in an infinite loop. > > Since the crypto hardware is not implemented, all registers are read as 0. > > So sun4i-ss will never progress in any operations. (TX_CNT being always 0) > > > > The first idea is to add a "TX_CNT always zero timeout" but this made cipher/hash loops > > more complex and prevent a case that never happen on real hardware. > > > > The best way to fix is to check at probe time if we run on a virtual > > machine with hardware emulated but non-implemented and prevent > > sun4i-ss to be loaded in that case. > > Letting sun4i-ss to load is useless anyway since all crypto algorithm will be > > disabled since they will fail crypto selftests. > > > > Tested-on: qemu-cubieboard > > Tested-on: cubieboard2 > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > --- > > drivers/crypto/sunxi-ss/sun4i-ss-core.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > index a81d89b3b7d8..a178e80adcf3 100644 > > --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > @@ -341,9 +341,18 @@ static int sun4i_ss_probe(struct platform_device *pdev) > > * I expect to be a sort of Security System Revision number. > > * Since the A80 seems to have an other version of SS > > * this info could be useful > > + * Detect virtual machine with non-implemented hardware > > + * (qemu-cubieboard) by checking the register value after a write to it. > > + * On non-implemented hardware, all registers are read as 0. > > + * On real hardware we should have a value > 0. > > */ > > writel(SS_ENABLED, ss->base + SS_CTL); > > v = readl(ss->base + SS_CTL); > > + if (!v) { > > + dev_err(&pdev->dev, "Qemu with non-implemented SS detected.\n"); > > + err = -ENODEV; > > + goto error_rst; > > + } > > This is wrong way to tackle the issue. There's multiple reason why > this could happen (for example the device not being clocked, or > maintained in reset). There's nothing specific about qemu here, and > the fundamental issue isn't that the device isn't functional in qemu, > it's that qemu lies about which hardware it can emulate in the DT it > passes to the kernel. > > There's no way this can scale, alone from the fact that qemu should > patch the DT according to what it can do. Not trying to chase after > each and every device that is broken in qemu. > > NAK. > My fix detect also when the device is badly clocked. So since it could fix problem unrelated to qemu, I will send a V2 with updated comment. Regards ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: sun4i-ss: prevent deadlock on emulated hardware 2018-06-15 8:15 ` Corentin Labbe @ 2018-06-15 9:04 ` Maxime Ripard 2018-06-15 9:16 ` [linux-sunxi] " Corentin Labbe 0 siblings, 1 reply; 6+ messages in thread From: Maxime Ripard @ 2018-06-15 9:04 UTC (permalink / raw) To: Corentin Labbe Cc: davem, herbert, wens, linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 3284 bytes --] On Fri, Jun 15, 2018 at 10:15:54AM +0200, Corentin Labbe wrote: > On Fri, Jun 15, 2018 at 09:57:54AM +0200, Maxime Ripard wrote: > > On Thu, Jun 14, 2018 at 09:36:59PM +0200, Corentin Labbe wrote: > > > Running a qemu emulated cubieboard with sun4i-ss driver enabled led to a never > > > ending boot. > > > This is due to sun4i-ss deadlocked and taking all cpu in an infinite loop. > > > Since the crypto hardware is not implemented, all registers are read as 0. > > > So sun4i-ss will never progress in any operations. (TX_CNT being always 0) > > > > > > The first idea is to add a "TX_CNT always zero timeout" but this made cipher/hash loops > > > more complex and prevent a case that never happen on real hardware. > > > > > > The best way to fix is to check at probe time if we run on a virtual > > > machine with hardware emulated but non-implemented and prevent > > > sun4i-ss to be loaded in that case. > > > Letting sun4i-ss to load is useless anyway since all crypto algorithm will be > > > disabled since they will fail crypto selftests. > > > > > > Tested-on: qemu-cubieboard > > > Tested-on: cubieboard2 > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > --- > > > drivers/crypto/sunxi-ss/sun4i-ss-core.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > index a81d89b3b7d8..a178e80adcf3 100644 > > > --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > @@ -341,9 +341,18 @@ static int sun4i_ss_probe(struct platform_device *pdev) > > > * I expect to be a sort of Security System Revision number. > > > * Since the A80 seems to have an other version of SS > > > * this info could be useful > > > + * Detect virtual machine with non-implemented hardware > > > + * (qemu-cubieboard) by checking the register value after a write to it. > > > + * On non-implemented hardware, all registers are read as 0. > > > + * On real hardware we should have a value > 0. > > > */ > > > writel(SS_ENABLED, ss->base + SS_CTL); > > > v = readl(ss->base + SS_CTL); > > > + if (!v) { > > > + dev_err(&pdev->dev, "Qemu with non-implemented SS detected.\n"); > > > + err = -ENODEV; > > > + goto error_rst; > > > + } > > > > This is wrong way to tackle the issue. There's multiple reason why > > this could happen (for example the device not being clocked, or > > maintained in reset). There's nothing specific about qemu here, and > > the fundamental issue isn't that the device isn't functional in qemu, > > it's that qemu lies about which hardware it can emulate in the DT it > > passes to the kernel. > > > > There's no way this can scale, alone from the fact that qemu should > > patch the DT according to what it can do. Not trying to chase after > > each and every device that is broken in qemu. > > > > NAK. > > > > My fix detect also when the device is badly clocked. In which case, the proper fix is to enable the clock, not throw the kernel's arm up in the air. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-sunxi] Re: [PATCH] crypto: sun4i-ss: prevent deadlock on emulated hardware 2018-06-15 9:04 ` Maxime Ripard @ 2018-06-15 9:16 ` Corentin Labbe 2018-06-15 11:08 ` Maxime Ripard 0 siblings, 1 reply; 6+ messages in thread From: Corentin Labbe @ 2018-06-15 9:16 UTC (permalink / raw) To: Maxime Ripard Cc: davem, herbert, wens, linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi On Fri, Jun 15, 2018 at 11:04:12AM +0200, Maxime Ripard wrote: > On Fri, Jun 15, 2018 at 10:15:54AM +0200, Corentin Labbe wrote: > > On Fri, Jun 15, 2018 at 09:57:54AM +0200, Maxime Ripard wrote: > > > On Thu, Jun 14, 2018 at 09:36:59PM +0200, Corentin Labbe wrote: > > > > Running a qemu emulated cubieboard with sun4i-ss driver enabled led to a never > > > > ending boot. > > > > This is due to sun4i-ss deadlocked and taking all cpu in an infinite loop. > > > > Since the crypto hardware is not implemented, all registers are read as 0. > > > > So sun4i-ss will never progress in any operations. (TX_CNT being always 0) > > > > > > > > The first idea is to add a "TX_CNT always zero timeout" but this made cipher/hash loops > > > > more complex and prevent a case that never happen on real hardware. > > > > > > > > The best way to fix is to check at probe time if we run on a virtual > > > > machine with hardware emulated but non-implemented and prevent > > > > sun4i-ss to be loaded in that case. > > > > Letting sun4i-ss to load is useless anyway since all crypto algorithm will be > > > > disabled since they will fail crypto selftests. > > > > > > > > Tested-on: qemu-cubieboard > > > > Tested-on: cubieboard2 > > > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > --- > > > > drivers/crypto/sunxi-ss/sun4i-ss-core.c | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > > index a81d89b3b7d8..a178e80adcf3 100644 > > > > --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > > @@ -341,9 +341,18 @@ static int sun4i_ss_probe(struct platform_device *pdev) > > > > * I expect to be a sort of Security System Revision number. > > > > * Since the A80 seems to have an other version of SS > > > > * this info could be useful > > > > + * Detect virtual machine with non-implemented hardware > > > > + * (qemu-cubieboard) by checking the register value after a write to it. > > > > + * On non-implemented hardware, all registers are read as 0. > > > > + * On real hardware we should have a value > 0. > > > > */ > > > > writel(SS_ENABLED, ss->base + SS_CTL); > > > > v = readl(ss->base + SS_CTL); > > > > + if (!v) { > > > > + dev_err(&pdev->dev, "Qemu with non-implemented SS detected.\n"); > > > > + err = -ENODEV; > > > > + goto error_rst; > > > > + } > > > > > > This is wrong way to tackle the issue. There's multiple reason why > > > this could happen (for example the device not being clocked, or > > > maintained in reset). There's nothing specific about qemu here, and > > > the fundamental issue isn't that the device isn't functional in qemu, > > > it's that qemu lies about which hardware it can emulate in the DT it > > > passes to the kernel. > > > > > > There's no way this can scale, alone from the fact that qemu should > > > patch the DT according to what it can do. Not trying to chase after > > > each and every device that is broken in qemu. > > > > > > NAK. > > > > > > > My fix detect also when the device is badly clocked. > > In which case, the proper fix is to enable the clock, not throw the > kernel's arm up in the air. > By badly I mean "not clocked" or "with the wrong frequencies". I could change the clock rate range test to exit (it issue only a warning for now). But I think this fix detect all cases and still permit someone to play with overclocking/downclocking. Regards ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [linux-sunxi] Re: [PATCH] crypto: sun4i-ss: prevent deadlock on emulated hardware 2018-06-15 9:16 ` [linux-sunxi] " Corentin Labbe @ 2018-06-15 11:08 ` Maxime Ripard 0 siblings, 0 replies; 6+ messages in thread From: Maxime Ripard @ 2018-06-15 11:08 UTC (permalink / raw) To: Corentin Labbe Cc: davem, herbert, wens, linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi On Fri, Jun 15, 2018 at 11:16:50AM +0200, Corentin Labbe wrote: > On Fri, Jun 15, 2018 at 11:04:12AM +0200, Maxime Ripard wrote: > > On Fri, Jun 15, 2018 at 10:15:54AM +0200, Corentin Labbe wrote: > > > On Fri, Jun 15, 2018 at 09:57:54AM +0200, Maxime Ripard wrote: > > > > On Thu, Jun 14, 2018 at 09:36:59PM +0200, Corentin Labbe wrote: > > > > > Running a qemu emulated cubieboard with sun4i-ss driver enabled led to a never > > > > > ending boot. > > > > > This is due to sun4i-ss deadlocked and taking all cpu in an infinite loop. > > > > > Since the crypto hardware is not implemented, all registers are read as 0. > > > > > So sun4i-ss will never progress in any operations. (TX_CNT being always 0) > > > > > > > > > > The first idea is to add a "TX_CNT always zero timeout" but this made cipher/hash loops > > > > > more complex and prevent a case that never happen on real hardware. > > > > > > > > > > The best way to fix is to check at probe time if we run on a virtual > > > > > machine with hardware emulated but non-implemented and prevent > > > > > sun4i-ss to be loaded in that case. > > > > > Letting sun4i-ss to load is useless anyway since all crypto algorithm will be > > > > > disabled since they will fail crypto selftests. > > > > > > > > > > Tested-on: qemu-cubieboard > > > > > Tested-on: cubieboard2 > > > > > > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com> > > > > > --- > > > > > drivers/crypto/sunxi-ss/sun4i-ss-core.c | 10 ++++++++++ > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > > > index a81d89b3b7d8..a178e80adcf3 100644 > > > > > --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > > > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > > > > > @@ -341,9 +341,18 @@ static int sun4i_ss_probe(struct platform_device *pdev) > > > > > * I expect to be a sort of Security System Revision number. > > > > > * Since the A80 seems to have an other version of SS > > > > > * this info could be useful > > > > > + * Detect virtual machine with non-implemented hardware > > > > > + * (qemu-cubieboard) by checking the register value after a write to it. > > > > > + * On non-implemented hardware, all registers are read as 0. > > > > > + * On real hardware we should have a value > 0. > > > > > */ > > > > > writel(SS_ENABLED, ss->base + SS_CTL); > > > > > v = readl(ss->base + SS_CTL); > > > > > + if (!v) { > > > > > + dev_err(&pdev->dev, "Qemu with non-implemented SS detected.\n"); > > > > > + err = -ENODEV; > > > > > + goto error_rst; > > > > > + } > > > > > > > > This is wrong way to tackle the issue. There's multiple reason why > > > > this could happen (for example the device not being clocked, or > > > > maintained in reset). There's nothing specific about qemu here, and > > > > the fundamental issue isn't that the device isn't functional in qemu, > > > > it's that qemu lies about which hardware it can emulate in the DT it > > > > passes to the kernel. > > > > > > > > There's no way this can scale, alone from the fact that qemu should > > > > patch the DT according to what it can do. Not trying to chase after > > > > each and every device that is broken in qemu. > > > > > > > > NAK. > > > > > > > > > > My fix detect also when the device is badly clocked. > > > > In which case, the proper fix is to enable the clock, not throw the > > kernel's arm up in the air. > > > > By badly I mean "not clocked" or "with the wrong frequencies". > > I could change the clock rate range test to exit (it issue only a > warning for now). But I think this fix detect all cases and still > permit someone to play with overclocking/downclocking. You're still trying to fix the consequence when you should be fixing the cause. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-15 11:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-14 19:36 [PATCH] crypto: sun4i-ss: prevent deadlock on emulated hardware Corentin Labbe 2018-06-15 7:57 ` Maxime Ripard 2018-06-15 8:15 ` Corentin Labbe 2018-06-15 9:04 ` Maxime Ripard 2018-06-15 9:16 ` [linux-sunxi] " Corentin Labbe 2018-06-15 11:08 ` Maxime Ripard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).