2014-07-18 07:14:50

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCHv4 0/2] parport: parport_pc: Fix false-positives at checking for Intel bug

From: "Matwey V. Kornilov" <[email protected]>

Hi,

The following patch series is to deal with the issue on false-positives
of Intel EPP bug check [1].

More than a decade ago, the check was introduced in order to prevent EPP
operation on the some Intel LPT chipsets. The main issue to defence from
was CPU hang at EPP operation on broken chipsets. It is mentioned that
affected chipsets are Intel 82091. However, it is not known whether
there are others.

The check was implemented in strange manner. Now, there is no explanations
why. The check itself now leads to the false-positives, disabling EPP on
many PC-s (Dell OptiPlex series for instance). The latter is an issue.

As it was suggested by One Thousand Gnomes we implement the additional check of CPU model.
If CPU is Intel 80486 or Pentium, then it is considered as potentially affected by the initial problem, so the initial check is applied.

The patches organized as following:

1. Introduce-intel_bug_present-function.

The transparent refactoring of the check is performed.
Make the check be immutable regarding to ECR register.

2. Implement CPU model check to cut off false-positives

Based on CPU model, disable the check. The check is enabled only for Intel 80486 or Pentium.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=630593

Suggested-by: One Thousand Gnomes <[email protected]>
Tested-by: Heiko Andreas Sommer <[email protected]>
Signed-off-by: Matwey V. Kornilov <[email protected]>

---
Changes from v3:
- Do not introduce the additional option, rely on CPU model instead.

Changes from v2:
- The module option has more clear description

Changes from v1:
- Patch 1 fetched from right branch and now compiled


2014-07-18 07:14:51

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCHv4 2/2] parport: parport_pc: Implement CPU model check to cut off false-positives

From: "Matwey V. Kornilov" <[email protected]>

The code in intel_bug_present is known to produce much false-positives.
It is believed that the affected by the bug hardware are used with either Intel 80486 or Pentium.

Perform the check only when the kernel configured as CONFIG_X86_32,
then use cpuinfo_x86 of the first available CPU to check the model
and run initial check code.

Suggested-by: One Thousand Gnomes <[email protected]>
Tested-by: Heiko Andreas Sommer <[email protected]>
Signed-off-by: Matwey V. Kornilov <[email protected]>
---
drivers/parport/parport_pc.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index a6eaafb..6b28f9f 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -65,6 +65,7 @@
#include <linux/parport_pc.h>
#include <linux/via.h>
#include <asm/parport.h>
+#include <asm/processor.h>

#define PARPORT_PC_MAX_PORTS PARPORT_MAX

@@ -1702,7 +1703,11 @@ static int parport_ECP_supported(struct parport *pb)
}
#endif

-static int intel_bug_present(struct parport *pb)
+/* It is believed that CPU model correlates with buggy LPT chipset model.
+ Here we check that or CPU is elder than Pentium Pro: either 80486 or Pentium.
+ If it is then we perform The Check. */
+#ifdef CONFIG_X86_32
+static int intel_bug_present_check_epp(struct parport *pb)
{
const struct parport_pc_private *priv = pb->private_data;
int bug_present = 0;
@@ -1725,6 +1730,20 @@ static int intel_bug_present(struct parport *pb)

return bug_present;
}
+static int intel_bug_present(struct parport *pb)
+{
+ struct cpuinfo_x86 *c = &cpu_data(0);
+
+ if (c->x86_vendor == X86_VENDOR_INTEL && (c->x86 == 4 || c->x86 == 5))
+ return intel_bug_present_check_epp(pb);
+ return 0;
+}
+#else
+static int intel_bug_present(struct parport *pb)
+{
+ return 0;
+}
+#endif

static int parport_ECPPS2_supported(struct parport *pb)
{
--
1.8.1.4

2014-07-18 07:14:48

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCHv4 1/2] parport: parport_pc: Introduce intel_bug_present function.

From: "Matwey V. Kornilov" <[email protected]>

Put the code to check present of the Intel bug from parport_EPP_supported
into new intel_bug_present function. The later also return ECR register
to the state it has before function call.

Suggested-by: One Thousand Gnomes <[email protected]>
Tested-by: Heiko Andreas Sommer <[email protected]>
Signed-off-by: Matwey V. Kornilov <[email protected]>
---
drivers/parport/parport_pc.c | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
index 76ee775..a6eaafb 100644
--- a/drivers/parport/parport_pc.c
+++ b/drivers/parport/parport_pc.c
@@ -1702,6 +1702,30 @@ static int parport_ECP_supported(struct parport *pb)
}
#endif

+static int intel_bug_present(struct parport *pb)
+{
+ const struct parport_pc_private *priv = pb->private_data;
+ int bug_present = 0;
+
+ if (priv->ecr) {
+ /* store value of ECR */
+ unsigned char ecr = inb(ECONTROL(pb));
+ unsigned char i;
+ for (i = 0x00; i < 0x80; i += 0x20) {
+ ECR_WRITE(pb, i);
+ if (clear_epp_timeout(pb)) {
+ /* Phony EPP in ECP. */
+ bug_present = 1;
+ break;
+ }
+ }
+ /* return ECR into the inital state */
+ ECR_WRITE(pb, ecr);
+ }
+
+ return bug_present;
+}
+
static int parport_ECPPS2_supported(struct parport *pb)
{
const struct parport_pc_private *priv = pb->private_data;
@@ -1722,8 +1746,6 @@ static int parport_ECPPS2_supported(struct parport *pb)

static int parport_EPP_supported(struct parport *pb)
{
- const struct parport_pc_private *priv = pb->private_data;
-
/*
* Theory:
* Bit 0 of STR is the EPP timeout bit, this bit is 0
@@ -1742,16 +1764,8 @@ static int parport_EPP_supported(struct parport *pb)
return 0; /* No way to clear timeout */

/* Check for Intel bug. */
- if (priv->ecr) {
- unsigned char i;
- for (i = 0x00; i < 0x80; i += 0x20) {
- ECR_WRITE(pb, i);
- if (clear_epp_timeout(pb)) {
- /* Phony EPP in ECP. */
- return 0;
- }
- }
- }
+ if (intel_bug_present(pb))
+ return 0;

pb->modes |= PARPORT_MODE_EPP;

--
1.8.1.4

2014-07-18 07:32:23

by Ondrej Zary

[permalink] [raw]
Subject: Re: [PATCHv4 2/2] parport: parport_pc: Implement CPU model check to cut off false-positives

On Friday 18 July 2014, [email protected] wrote:
> From: "Matwey V. Kornilov" <[email protected]>
>
> The code in intel_bug_present is known to produce much false-positives.
> It is believed that the affected by the bug hardware are used with either
> Intel 80486 or Pentium.
>
> Perform the check only when the kernel configured as CONFIG_X86_32,
> then use cpuinfo_x86 of the first available CPU to check the model
> and run initial check code.
>
> Suggested-by: One Thousand Gnomes <[email protected]>
> Tested-by: Heiko Andreas Sommer <[email protected]>
> Signed-off-by: Matwey V. Kornilov <[email protected]>
> ---
> drivers/parport/parport_pc.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/parport/parport_pc.c b/drivers/parport/parport_pc.c
> index a6eaafb..6b28f9f 100644
> --- a/drivers/parport/parport_pc.c
> +++ b/drivers/parport/parport_pc.c
> @@ -65,6 +65,7 @@
> #include <linux/parport_pc.h>
> #include <linux/via.h>
> #include <asm/parport.h>
> +#include <asm/processor.h>
>
> #define PARPORT_PC_MAX_PORTS PARPORT_MAX
>
> @@ -1702,7 +1703,11 @@ static int parport_ECP_supported(struct parport *pb)
> }
> #endif
>
> -static int intel_bug_present(struct parport *pb)
> +/* It is believed that CPU model correlates with buggy LPT chipset model.
> + Here we check that or CPU is elder than Pentium Pro: either 80486 or
> Pentium. + If it is then we perform The Check. */
> +#ifdef CONFIG_X86_32
> +static int intel_bug_present_check_epp(struct parport *pb)
> {
> const struct parport_pc_private *priv = pb->private_data;
> int bug_present = 0;
> @@ -1725,6 +1730,20 @@ static int intel_bug_present(struct parport *pb)
>
> return bug_present;
> }
> +static int intel_bug_present(struct parport *pb)
> +{
> + struct cpuinfo_x86 *c = &cpu_data(0);
> +
> + if (c->x86_vendor == X86_VENDOR_INTEL && (c->x86 == 4 || c->x86 == 5))
> + return intel_bug_present_check_epp(pb);

You can have a non-Intel CPU in a 486 or Pentium board.

> + return 0;
> +}
> +#else
> +static int intel_bug_present(struct parport *pb)
> +{
> + return 0;
> +}
> +#endif
>
> static int parport_ECPPS2_supported(struct parport *pb)
> {



--
Ondrej Zary