[Qemu-devel] [PATCH] ignore reads to the EOI register.

[Qemu-devel] [PATCH] ignore reads to the EOI register.

[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 <gcosta@redhat.com>
---
 qemu/hw/apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/apic.c b/qemu/hw/apic.c
index 92248dd..4102493 100644
--- a/qemu/hw/apic.c
+++ b/qemu/hw/apic.c
@@ -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;
-- 
1.5.0.6

[Qemu-devel] Re: [PATCH] ignore reads to the EOI register.

[Avi Kivity]

Glauber Costa wrote:
> 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.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH] ignore reads to the EOI register.

[Maciej W. Rozycki]

On Thu, 27 Mar 2008, Avi Kivity wrote:

> 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

[Qemu-devel] Re: [PATCH] ignore reads to the EOI register.

[Avi Kivity]

Andi Kleen wrote:
> "Maciej W. Rozycki" <macro@linux-mips.org> writes:
>   
>>  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.
>>     
>
> 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?

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH] ignore reads to the EOI register.

[Maciej W. Rozycki]

On Thu, 27 Mar 2008, Andi Kleen wrote:

> 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

Re: [Qemu-devel] [PATCH] ignore reads to the EOI register.

[Aurelien Jarno]

On Wed, Mar 26, 2008 at 09:57:16PM -0300, Glauber Costa wrote:
> 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 <gcosta@redhat.com>
> ---
>  qemu/hw/apic.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu/hw/apic.c b/qemu/hw/apic.c
> index 92248dd..4102493 100644
> --- a/qemu/hw/apic.c
> +++ b/qemu/hw/apic.c
> @@ -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?

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] ignore reads to the EOI register.

[Glauber Costa]

Aurelien Jarno wrote:
> On Wed, Mar 26, 2008 at 09:57:16PM -0300, Glauber Costa wrote:
>> 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 <gcosta@redhat.com>
>> ---
>>  qemu/hw/apic.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu/hw/apic.c b/qemu/hw/apic.c
>> index 92248dd..4102493 100644
>> --- a/qemu/hw/apic.c
>> +++ b/qemu/hw/apic.c
>> @@ -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.