Skip to contentSkip to conversation

ignore reads to the EOI register.

PATCH Applied Glauber Costa 4 participants 7 messages Mar 27, 2008 – Mar 28, 2008 1 message not in the archive KVM

lore
Share & tools
lore

Glauber Costa

author
#0 lore raw

PATCH 1/1 ignore reads to the EOI register.

They seem legal in real hardware, even though the EOI is a write-only register. By "legal" I mean they are completely ignored, but at least, don't cause any bits to be set at ESR.

Without this patch, some (very recent) linux git trees will fail to boot in i386.

This is generated from kvm-userspace, but should apply well to plain qemu too.

Signed-off-by Glauber Costa <[email protected]>
---
 qemu/hw/apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
qemu/hw/apic.c +2 -0
index 92248dd..4102493 100644
@@ -615,6 +615,8 @@ static uint32_t apic_mem_readl(void *opaque, target_phys_addr_t addr)
         /* ppr */
         val = apic_get_ppr(s);
         break;
+    case 0x0b:
+        break;
     case 0x0d:
         val = s->log_dest << 24;
         break;
Signature · 1 line
1.5.0.6
Applied
#1 lore raw
Quoted 11 lines · Glauber Costa

They seem legal in real hardware, even though the EOI is a write-only register. By "legal" I mean they are completely ignored, but at least, don't cause any bits to be set at ESR.

Without this patch, some (very recent) linux git trees will fail to boot in i386.

This is generated from kvm-userspace, but should apply well to plain qemu too.

Applied, but perhaps a patch to linux to avoid reads to write-only registers is needed as well.

Signature · 1 line
error compiling committee.c: too many arguments to function
#2 lore raw
Quoted 2 lines · Avi Kivity

Applied, but perhaps a patch to linux to avoid reads to write-only registers is needed as well.

Linux performs reads to all registers written including this one
deliberately using an RMW cycle to avoid triggering an erratum in some early Pentium integrated APICs. Obviously it does not matter for most of the world as the workaround is build-time conditional, but you'll get it if you build a generic "runs everywhere" kernel.

Maciej

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw
Quoted 9 lines · Andi Kleen
Quoted 6 lines · Maciej W. Rozycki

Linux performs reads to all registers written including this one
deliberately using an RMW cycle to avoid triggering an erratum in some early Pentium integrated APICs. Obviously it does not matter for most of the world as the workaround is build-time conditional, but you'll get it if you build a generic "runs everywhere" kernel.

Quoted 4 lines

It would be quite possible to make the cycle conditional on a cpufeatures.h quirk flag that is only set on P5. Just would need to out of line a few functions to avoid code bloat.

Or, alternatives? xchg vs mov?

Signature · 8 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
#5 lore raw
In reply to a message not in the archive
Quoted 3 lines · Andi Kleen

It would be quite possible to make the cycle conditional on a cpufeatures.h quirk flag that is only set on P5. Just would need to out of line a few functions to avoid code bloat.

Exactly what I suggested at the LKML -- you could even go down to a list
of steppings affected as an RMW cycle is unnecessarily expensive (we only need interrupt atomicity, but there is no way to disable the LOCK# implication of "xchg").

Maciej

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

#6 lore raw
In reply to Glauber Costa
Quoted 20 lines · Glauber Costa

They seem legal in real hardware, even though the EOI is a write-only register. By "legal" I mean they are completely ignored, but at least, don't cause any bits to be set at ESR.

Without this patch, some (very recent) linux git trees will fail to boot in i386.

This is generated from kvm-userspace, but should apply well to plain qemu too.

Signed-off-by Glauber Costa <[email protected]>
---
 qemu/hw/apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
qemu/hw/apic.c +0 -0
index 92248dd..4102493 100644

qemu/hw/apic.c static uint32_t apic_mem_readl(void *opaque, target_phys_addr_t addr) · line 619

Quoted 5 lines
qemu/hw/apic.c +2 -0
@@ -615,6 +615,8 @@ static uint32_t apic_mem_readl(void *opaque, target_phys_addr_t addr)
         /* ppr */
         val = apic_get_ppr(s);
         break;
+    case 0x0b:
+        break;

While I agree the guest should not care of the value (it should actually not read it), wouldn't it be safer to return a default value (0 ?) instead of an initialized value?

Signature · 10 lines
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   [email protected]         | [email protected]
   `-    people.debian.org/~aurel32 | www.aurel32.net

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
author
#7 lore raw
Quoted 16 lines · Aurelien Jarno
Quoted 15 lines · Glauber Costa

They seem legal in real hardware, even though the EOI is a write-only register. By "legal" I mean they are completely ignored, but at least, don't cause any bits to be set at ESR.

Without this patch, some (very recent) linux git trees will fail to boot in i386.

This is generated from kvm-userspace, but should apply well to plain qemu too.

Signed-off-by Glauber Costa <[email protected]>
---
 qemu/hw/apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Quoted 15 lines

qemu/hw/apic.c static uint32_t apic_mem_readl(void *opaque, target_phys_addr_t addr) · line 619

Quoted 10 lines
qemu/hw/apic.c +2 -0
index 92248dd..4102493 100644
@@ -615,6 +615,8 @@ static uint32_t apic_mem_readl(void *opaque, target_phys_addr_t addr)
         /* ppr */
         val = apic_get_ppr(s);
         break;
+    case 0x0b:
+        break;

While I agree the guest should not care of the value (it should actually not read it), wouldn't it be safer to return a default value (0 ?) instead of an initialized value?

I don't think it would really make any difference, but is not opposed to this solution either.

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace