public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Fix GPE registers read/write handling.
@ 2009-02-04  9:58 Gleb Natapov
  2009-02-04  9:58 ` [PATCH 2/3] Fix CPU hotplug Gleb Natapov
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Gleb Natapov @ 2009-02-04  9:58 UTC (permalink / raw)
  To: avi; +Cc: kvm

For STS register bit are cleared by writing 1 into it.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---

 qemu/hw/acpi.c |   42 ++++++++++++++++++++++++++++++------------
 1 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/qemu/hw/acpi.c b/qemu/hw/acpi.c
index b998225..68513c0 100644
--- a/qemu/hw/acpi.c
+++ b/qemu/hw/acpi.c
@@ -590,6 +590,13 @@ struct pci_status {
 static struct gpe_regs gpe;
 static struct pci_status pci0_status;
 
+static uint32_t gpe_read_val(uint16_t val, uint32_t addr)
+{
+    if (addr & 1)
+        return (val >> 8) & 0xff;
+    return val & 0xff;
+}
+
 static uint32_t gpe_readb(void *opaque, uint32_t addr)
 {
     uint32_t val = 0;
@@ -603,16 +610,12 @@ static uint32_t gpe_readb(void *opaque, uint32_t addr)
             break;
 
         case GPE_BASE:
-            val = g->sts & 0xFF;
-            break;
         case GPE_BASE + 1:
-            val =  (g->sts >> 8) & 0xFF;
+            val = gpe_read_val(g->sts, addr);
             break;
         case GPE_BASE + 2:
-            val =  g->en & 0xFF;
-            break;
         case GPE_BASE + 3:
-            val =  (g->en >> 8) & 0xFF;
+            val = gpe_read_val(g->en, addr);
             break;
         default:
             break;
@@ -624,6 +627,25 @@ static uint32_t gpe_readb(void *opaque, uint32_t addr)
     return val;
 }
 
+static uint16_t gpe_write_val(uint16_t cur, int addr, uint32_t val)
+{
+    if (addr & 1)
+        return (cur & 0xff) | (val << 8);
+    return (cur & 0xff00) | (val & 0xff);
+}
+
+static uint16_t gpe_reset_val(uint16_t cur, int addr, uint32_t val)
+{
+    uint16_t x1, x0 = val & 0xff;
+    int shift = (addr & 1) ? 8 : 0;
+
+    x1 = (cur >> shift) & 0xff;
+
+    x1 = x1 & ~x0;
+
+    return (cur & (0xff << (8 - shift))) | (x1 << shift);
+}
+
 static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val)
 {
     struct gpe_regs *g = opaque;
@@ -636,16 +658,12 @@ static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val)
             break;
 
         case GPE_BASE:
-            g->sts = (g->sts & ~0xFFFF) | (val & 0xFFFF);
-            break;
         case GPE_BASE + 1:
-            g->sts = (g->sts & 0xFFFF) | (val << 8);
+            g->sts = gpe_reset_val(g->sts, addr, val);
             break;
         case GPE_BASE + 2:
-            g->en = (g->en & ~0xFFFF) | (val & 0xFFFF);
-            break;
         case GPE_BASE + 3:
-            g->en = (g->en & 0xFFFF) | (val << 8);
+            g->en = gpe_write_val(g->en, addr, val);
             break;
         default:
             break;


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] Fix CPU hotplug
  2009-02-04  9:58 [PATCH 1/3] Fix GPE registers read/write handling Gleb Natapov
@ 2009-02-04  9:58 ` Gleb Natapov
  2009-02-04 12:45   ` Avi Kivity
  2009-02-04  9:58 ` [PATCH 3/3] PCI hotplug fixes Gleb Natapov
  2009-02-04 12:29 ` [PATCH 1/3] Fix GPE registers read/write handling Avi Kivity
  2 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2009-02-04  9:58 UTC (permalink / raw)
  To: avi; +Cc: kvm

1) Disabled processor's _STA method should return 0 (this fixes Vista's
   BSOD on resuming after hibernate problem)
2) Disabled processor's _MAT method should return disabled MADT entry
   instead of 0
3) Extend bitmask of hot pluggable CPUs to be 16 bit long
4) Generate interrupt only if corespondent EN bit is set
5) Use reserved STS bits from PIIX4 chipset to avoid clash in the
   future.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---

 qemu/hw/acpi.c |   39 +++++++++++++++++++++------------------
 1 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/bios/acpi-dsdt.dsl b/bios/acpi-dsdt.dsl
index d67616d..c400382 100755
--- a/bios/acpi-dsdt.dsl
+++ b/bios/acpi-dsdt.dsl
@@ -27,28 +27,29 @@ DefinitionBlock (
 {
    Scope (\_PR)
    {
-	OperationRegion( PRST, SystemIO, 0xaf00, 0x02)
-	Field (PRST, ByteAcc, NoLock, WriteAsZeros)
+	OperationRegion( PRST, SystemIO, 0xaf00, 0x04)
+	Field (PRST, WordAcc, NoLock, Preserve)
 	{
-		PRU, 8,
-		PRD, 8,
+		PRU, 16,
+		PRD, 16,
 	}
 
 #define gen_processor(nr, name) 				            \
 	Processor (CPU##name, nr, 0x0000b010, 0x06) {                       \
-            Name (TMP, Buffer(0x8) {0x0, 0x8, nr, nr, 0x1, 0x0, 0x0, 0x0})  \
+            Name (PREN, Buffer(0x8) {0x0, 0x8, nr, nr, 0x1, 0x0, 0x0, 0x0}) \
+            Name (PRDS, Buffer(0x8) {0x0, 0x8, nr, nr, 0x0, 0x0, 0x0, 0x0}) \
             Method(_MAT, 0) {                                               \
-                If (And(\_PR.PRU, ShiftLeft(1, nr))) { Return(TMP) }        \
-                Else { Return(0x0) }                                        \
+                If (And(\_PR.PRU, ShiftLeft(1, nr))) { Return(PREN) }       \
+                Else { Return(PRDS) }                                       \
             }                                                               \
             Method (_STA) {                                                 \
-                Return(0xF)                                                 \
+                If (And(\_PR.PRU, ShiftLeft(1, nr))) { Return(0xF) }        \
+                Else { Return(0x0) }                                        \
             }                                                               \
         }                                                                   \
 
 
-
-        Processor (CPU0, 0x00, 0x0000b010, 0x06) {Method (_STA) { Return(0xF)}}
+	gen_processor(0, 0)
 	gen_processor(1, 1)
 	gen_processor(2, 2)
 	gen_processor(3, 3)
@@ -63,6 +64,28 @@ DefinitionBlock (
 	gen_processor(12, C)
 	gen_processor(13, D)
 	gen_processor(14, E)
+
+	Method (NTFY, 2, NotSerialized) {
+#define gen_ntfy(nr)                              \
+	If (LEqual(Arg0, 0x##nr)) {               \
+		Notify(CPU##nr, Arg1)             \
+	}
+		gen_ntfy(0)
+		gen_ntfy(1)
+		gen_ntfy(2)
+		gen_ntfy(3)
+		gen_ntfy(4)
+		gen_ntfy(5)
+		gen_ntfy(6)
+		gen_ntfy(7)
+		gen_ntfy(8)
+		gen_ntfy(9)
+		gen_ntfy(A)
+		gen_ntfy(B)
+		gen_ntfy(C)
+		gen_ntfy(D)
+		gen_ntfy(E)
+	}
     }
 
     Scope (\)
@@ -666,33 +689,12 @@ DefinitionBlock (
         Zero,  /* reserved */
         Zero   /* reserved */
     })
+
     Scope (\_GPE)
     {
-
-#define gen_cpu_hotplug(name, nr)                      \
-	If (And(\_PR.PRU, ShiftLeft(1, nr))) {     \
-	    Notify(\_PR.CPU##name, 1)              \
-        }                                          \
-	If (And(\_PR.PRD, ShiftLeft(1, nr))) {     \
-	    Notify(\_PR.CPU##name, 3)              \
-        }
+	Name(_HID, "ACPI0006")
 
         Method(_L00) {
-	    gen_cpu_hotplug(1, 1)
-	    gen_cpu_hotplug(2, 2)
-	    gen_cpu_hotplug(3, 3)
-	    gen_cpu_hotplug(4, 4)
-	    gen_cpu_hotplug(5, 5)
-	    gen_cpu_hotplug(6, 6)
-	    gen_cpu_hotplug(7, 7)
-	    gen_cpu_hotplug(8, 8)
-	    gen_cpu_hotplug(9, 9)
-	    gen_cpu_hotplug(A, 10)
-	    gen_cpu_hotplug(B, 11)
-	    gen_cpu_hotplug(C, 12)
-	    gen_cpu_hotplug(D, 13)
-	    gen_cpu_hotplug(E, 14)
-
             Return(0x01)
         }
 
@@ -739,9 +741,29 @@ DefinitionBlock (
 
             Return(0x01)
         }
+
         Method(_L02) {
+	    Store(Zero, Local3)
+	    Store(\_PR.PRU, Local2)
+	    Xor(Local2, \_PR.PRD, Local0)
+	    Store(Local2, \_PR.PRD)
+	    Store(\_PR.PRD, Local1)
+            While (LNotEqual (Local0, Zero)) {
+		Store(ShiftLeft(1, Local3), Local1)
+		If (And(Local0, Local1)) {
+			Store(And(Local0, Not(Local1)), Local0)
+			If (And(Local2, Local1)) {
+	                	Store(1, Local4)
+			} Else {
+	                	Store(3, Local4)
+			}
+	                \_PR.NTFY(Local3, Local4)
+		}
+		Increment(Local3)
+	    }
             Return(0x01)
         }
+
         Method(_L03) {
             Return(0x01)
         }
diff --git a/qemu/hw/acpi.c b/qemu/hw/acpi.c
index 68513c0..83079fa 100644
--- a/qemu/hw/acpi.c
+++ b/qemu/hw/acpi.c
@@ -578,8 +578,8 @@ void qemu_system_powerdown(void)
 struct gpe_regs {
     uint16_t sts; /* status */
     uint16_t en;  /* enabled */
-    uint8_t up;
-    uint8_t down;
+    uint16_t cpus_sts;
+    uint16_t bios_cpus_sts;
 };
 
 struct pci_status {
@@ -603,10 +603,12 @@ static uint32_t gpe_readb(void *opaque, uint32_t addr)
     struct gpe_regs *g = opaque;
     switch (addr) {
         case PROC_BASE:
-            val = g->up;
-            break;
         case PROC_BASE + 1:
-            val = g->down;
+            val = gpe_read_val(g->cpus_sts, addr);
+            break;
+        case PROC_BASE + 2:
+        case PROC_BASE + 3:
+            val = gpe_read_val(g->bios_cpus_sts, addr);
             break;
 
         case GPE_BASE:
@@ -651,10 +653,12 @@ static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val)
     struct gpe_regs *g = opaque;
     switch (addr) {
         case PROC_BASE:
-            g->up = val;
-            break;
         case PROC_BASE + 1:
-            g->down = val;
+            /* don't allow to change cpu_sts from inside a guest */
+            break;
+        case PROC_BASE + 2:
+        case PROC_BASE + 3:
+            g->bios_cpus_sts = gpe_write_val(g->bios_cpus_sts, addr, val);
             break;
 
         case GPE_BASE:
@@ -735,6 +739,7 @@ static const char *model;
 
 void qemu_system_hot_add_init(const char *cpu_model)
 {
+    gpe.bios_cpus_sts = gpe.cpus_sts = (1 << smp_cpus) - 1;
     register_ioport_write(GPE_BASE, 4, 1, gpe_writeb, &gpe);
     register_ioport_read(GPE_BASE, 4, 1,  gpe_readb, &gpe);
 
@@ -752,16 +757,14 @@ void qemu_system_hot_add_init(const char *cpu_model)
 
 static void enable_processor(struct gpe_regs *g, int cpu)
 {
-    g->sts |= 1;
-    g->en |= 1;
-    g->up |= (1 << cpu);
+    g->sts |= 4;
+    g->cpus_sts |= (1 << cpu);
 }
 
 static void disable_processor(struct gpe_regs *g, int cpu)
 {
-    g->sts |= 1;
-    g->en |= 1;
-    g->down |= (1 << cpu);
+    g->sts |= 4;
+    g->cpus_sts &= ~(1 << cpu);
 }
 
 #if defined(TARGET_I386) || defined(TARGET_X86_64)
@@ -802,14 +805,14 @@ void qemu_system_cpu_hot_add(int cpu, int state)
 #endif
     }
 
-    qemu_set_irq(pm_state->irq, 1);
-    gpe.up = 0;
-    gpe.down = 0;
     if (state)
         enable_processor(&gpe, cpu);
     else
         disable_processor(&gpe, cpu);
-    qemu_set_irq(pm_state->irq, 0);
+    if (gpe.en & 4) {
+        qemu_set_irq(pm_state->irq, 1);
+        qemu_set_irq(pm_state->irq, 0);
+    }
 }
 #endif
 


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] PCI hotplug fixes
  2009-02-04  9:58 [PATCH 1/3] Fix GPE registers read/write handling Gleb Natapov
  2009-02-04  9:58 ` [PATCH 2/3] Fix CPU hotplug Gleb Natapov
@ 2009-02-04  9:58 ` Gleb Natapov
  2009-02-04 12:29 ` [PATCH 1/3] Fix GPE registers read/write handling Avi Kivity
  2 siblings, 0 replies; 12+ messages in thread
From: Gleb Natapov @ 2009-02-04  9:58 UTC (permalink / raw)
  To: avi; +Cc: kvm

Generate interrupt only if corespondent EN bit is set.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---

 qemu/hw/acpi.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/qemu/hw/acpi.c b/qemu/hw/acpi.c
index 83079fa..337432a 100644
--- a/qemu/hw/acpi.c
+++ b/qemu/hw/acpi.c
@@ -819,25 +819,26 @@ void qemu_system_cpu_hot_add(int cpu, int state)
 static void enable_device(struct pci_status *p, struct gpe_regs *g, int slot)
 {
     g->sts |= 2;
-    g->en |= 2;
     p->up |= (1 << slot);
 }
 
 static void disable_device(struct pci_status *p, struct gpe_regs *g, int slot)
 {
     g->sts |= 2;
-    g->en |= 2;
     p->down |= (1 << slot);
 }
 
 void qemu_system_device_hot_add(int pcibus, int slot, int state)
 {
-    qemu_set_irq(pm_state->irq, 1);
     pci0_status.up = 0;
     pci0_status.down = 0;
     if (state)
         enable_device(&pci0_status, &gpe, slot);
     else
         disable_device(&pci0_status, &gpe, slot);
-    qemu_set_irq(pm_state->irq, 0);
+
+    if (gpe.en & 2) {
+        qemu_set_irq(pm_state->irq, 1);
+        qemu_set_irq(pm_state->irq, 0);
+    }
 }


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] Fix GPE registers read/write handling.
  2009-02-04  9:58 [PATCH 1/3] Fix GPE registers read/write handling Gleb Natapov
  2009-02-04  9:58 ` [PATCH 2/3] Fix CPU hotplug Gleb Natapov
  2009-02-04  9:58 ` [PATCH 3/3] PCI hotplug fixes Gleb Natapov
@ 2009-02-04 12:29 ` Avi Kivity
  2009-02-04 12:39   ` Gleb Natapov
  2 siblings, 1 reply; 12+ messages in thread
From: Avi Kivity @ 2009-02-04 12:29 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: kvm

Gleb Natapov wrote:
> For STS register bit are cleared by writing 1 into it.
>
>  
> +static uint16_t gpe_write_val(uint16_t cur, int addr, uint32_t val)
> +{
> +    if (addr & 1)
> +        return (cur & 0xff) | (val << 8);
> +    return (cur & 0xff00) | (val & 0xff);
> +}
> +
> +static uint16_t gpe_reset_val(uint16_t cur, int addr, uint32_t val)
> +{
> +    uint16_t x1, x0 = val & 0xff;
> +    int shift = (addr & 1) ? 8 : 0;
> +
> +    x1 = (cur >> shift) & 0xff;
> +
> +    x1 = x1 & ~x0;
> +
> +    return (cur & (0xff << (8 - shift))) | (x1 << shift);
> +}
>   

It's strange that write_val() and reset_val() return a value.  Won't it 
be cleaner to make val a pointer?


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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] Fix GPE registers read/write handling.
  2009-02-04 12:29 ` [PATCH 1/3] Fix GPE registers read/write handling Avi Kivity
@ 2009-02-04 12:39   ` Gleb Natapov
  2009-02-04 12:46     ` Avi Kivity
  0 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2009-02-04 12:39 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

On Wed, Feb 04, 2009 at 02:29:07PM +0200, Avi Kivity wrote:
> Gleb Natapov wrote:
>> For STS register bit are cleared by writing 1 into it.
>>
>>  +static uint16_t gpe_write_val(uint16_t cur, int addr, uint32_t val)
>> +{
>> +    if (addr & 1)
>> +        return (cur & 0xff) | (val << 8);
>> +    return (cur & 0xff00) | (val & 0xff);
>> +}
>> +
>> +static uint16_t gpe_reset_val(uint16_t cur, int addr, uint32_t val)
>> +{
>> +    uint16_t x1, x0 = val & 0xff;
>> +    int shift = (addr & 1) ? 8 : 0;
>> +
>> +    x1 = (cur >> shift) & 0xff;
>> +
>> +    x1 = x1 & ~x0;
>> +
>> +    return (cur & (0xff << (8 - shift))) | (x1 << shift);
>> +}
>>   
>
> It's strange that write_val() and reset_val() return a value.  Won't it  
> be cleaner to make val a pointer?
>
>From functional programing POV no, it is not cleaner. But I don't really
care where to do assignment.

--
			Gleb.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] Fix CPU hotplug
  2009-02-04  9:58 ` [PATCH 2/3] Fix CPU hotplug Gleb Natapov
@ 2009-02-04 12:45   ` Avi Kivity
  2009-02-04 13:03     ` Gleb Natapov
  0 siblings, 1 reply; 12+ messages in thread
From: Avi Kivity @ 2009-02-04 12:45 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: kvm

Gleb Natapov wrote:
> 1) Disabled processor's _STA method should return 0 (this fixes Vista's
>    BSOD on resuming after hibernate problem)
> 2) Disabled processor's _MAT method should return disabled MADT entry
>    instead of 0
> 3) Extend bitmask of hot pluggable CPUs to be 16 bit long
> 4) Generate interrupt only if corespondent EN bit is set
>   

Looks like a good idea, but it is really necessary?  The guest should be 
able to deal with null notifies.
(I'd like to apply this, just want to understand).

> 5) Use reserved STS bits from PIIX4 chipset to avoid clash in the
>    future.
>   

Please split into separate patches.

> @@ -739,9 +741,29 @@ DefinitionBlock (
>  
>              Return(0x01)
>          }
> +
>          Method(_L02) {
> +	    Store(Zero, Local3)
> +	    Store(\_PR.PRU, Local2)
> +	    Xor(Local2, \_PR.PRD, Local0)
> +	    Store(Local2, \_PR.PRD)
> +	    Store(\_PR.PRD, Local1)
> +            While (LNotEqual (Local0, Zero)) {
> +		Store(ShiftLeft(1, Local3), Local1)
> +		If (And(Local0, Local1)) {
> +			Store(And(Local0, Not(Local1)), Local0)
> +			If (And(Local2, Local1)) {
> +	                	Store(1, Local4)
> +			} Else {
> +	                	Store(3, Local4)
> +			}
> +	                \_PR.NTFY(Local3, Local4)
> +		}
> +		Increment(Local3)
> +	    }
>              Return(0x01)
>          }
>   

Please document this.

> --- a/qemu/hw/acpi.c
> +++ b/qemu/hw/acpi.c
> @@ -578,8 +578,8 @@ void qemu_system_powerdown(void)
>  struct gpe_regs {
>      uint16_t sts; /* status */
>      uint16_t en;  /* enabled */
> -    uint8_t up;
> -    uint8_t down;
> +    uint16_t cpus_sts;
> +    uint16_t bios_cpus_sts;
>  };
>   

We'll need to scale this soon.

>  
>  struct pci_status {
> @@ -603,10 +603,12 @@ static uint32_t gpe_readb(void *opaque, uint32_t addr)
>      struct gpe_regs *g = opaque;
>      switch (addr) {
>          case PROC_BASE:
> -            val = g->up;
> -            break;
>          case PROC_BASE + 1:
> -            val = g->down;
> +            val = gpe_read_val(g->cpus_sts, addr);
> +            break;
> +        case PROC_BASE + 2:
> +        case PROC_BASE + 3:
> +            val = gpe_read_val(g->bios_cpus_sts, addr);
>              break;
>   

Why can't the bios maintain bios_cpu_sts in RAM?


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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] Fix GPE registers read/write handling.
  2009-02-04 12:39   ` Gleb Natapov
@ 2009-02-04 12:46     ` Avi Kivity
  0 siblings, 0 replies; 12+ messages in thread
From: Avi Kivity @ 2009-02-04 12:46 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: kvm

Gleb Natapov wrote:

  

>> It's strange that write_val() and reset_val() return a value.  Won't it  
>> be cleaner to make val a pointer?
>>
>>     
> From functional programing POV no, it is not cleaner. But I don't really
> care where to do assignment.
>   

We're not doing functional programming:

   a = f(a, x)

is just duplication, but still has an assignment.

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] Fix CPU hotplug
  2009-02-04 12:45   ` Avi Kivity
@ 2009-02-04 13:03     ` Gleb Natapov
  2009-02-04 13:24       ` Avi Kivity
  0 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2009-02-04 13:03 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

On Wed, Feb 04, 2009 at 02:45:12PM +0200, Avi Kivity wrote:
> Gleb Natapov wrote:
>> 1) Disabled processor's _STA method should return 0 (this fixes Vista's
>>    BSOD on resuming after hibernate problem)
>> 2) Disabled processor's _MAT method should return disabled MADT entry
>>    instead of 0
>> 3) Extend bitmask of hot pluggable CPUs to be 16 bit long
>> 4) Generate interrupt only if corespondent EN bit is set
>>   
>
> Looks like a good idea, but it is really necessary?  The guest should be  
> able to deal with null notifies.
> (I'd like to apply this, just want to understand).
>
I don't really know what different OSes will do with null notifiers. But
if one of them will not handle them properly I don't want to be the one
who'll debug it :)
 
>> --- a/qemu/hw/acpi.c
>> +++ b/qemu/hw/acpi.c
>> @@ -578,8 +578,8 @@ void qemu_system_powerdown(void)
>>  struct gpe_regs {
>>      uint16_t sts; /* status */
>>      uint16_t en;  /* enabled */
>> -    uint8_t up;
>> -    uint8_t down;
>> +    uint16_t cpus_sts;
>> +    uint16_t bios_cpus_sts;
>>  };
>>   
>
> We'll need to scale this soon.
>
We should think how to handle Windows 2000 15 CPU limit.

>>   struct pci_status {
>> @@ -603,10 +603,12 @@ static uint32_t gpe_readb(void *opaque, uint32_t addr)
>>      struct gpe_regs *g = opaque;
>>      switch (addr) {
>>          case PROC_BASE:
>> -            val = g->up;
>> -            break;
>>          case PROC_BASE + 1:
>> -            val = g->down;
>> +            val = gpe_read_val(g->cpus_sts, addr);
>> +            break;
>> +        case PROC_BASE + 2:
>> +        case PROC_BASE + 3:
>> +            val = gpe_read_val(g->bios_cpus_sts, addr);
>>              break;
>>   
>
> Why can't the bios maintain bios_cpu_sts in RAM?
>
It can, just need to find a place for it. Currently our AML does not use
RAM at all.

--
			Gleb.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] Fix CPU hotplug
  2009-02-04 13:03     ` Gleb Natapov
@ 2009-02-04 13:24       ` Avi Kivity
  2009-02-04 13:31         ` Gleb Natapov
  0 siblings, 1 reply; 12+ messages in thread
From: Avi Kivity @ 2009-02-04 13:24 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: kvm

Gleb Natapov wrote:
>>>   struct pci_status {
>>> @@ -603,10 +603,12 @@ static uint32_t gpe_readb(void *opaque, uint32_t addr)
>>>      struct gpe_regs *g = opaque;
>>>      switch (addr) {
>>>          case PROC_BASE:
>>> -            val = g->up;
>>> -            break;
>>>          case PROC_BASE + 1:
>>> -            val = g->down;
>>> +            val = gpe_read_val(g->cpus_sts, addr);
>>> +            break;
>>> +        case PROC_BASE + 2:
>>> +        case PROC_BASE + 3:
>>> +            val = gpe_read_val(g->bios_cpus_sts, addr);
>>>              break;
>>>   
>>>       
>> Why can't the bios maintain bios_cpu_sts in RAM?
>>
>>     
> It can, just need to find a place for it. Currently our AML does not use
> RAM at all.

OperationRegion(..., SystemMemory, ...) should work.  It's better to 
avoid introducing unnecessary virtual hardware.

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] Fix CPU hotplug
  2009-02-04 13:24       ` Avi Kivity
@ 2009-02-04 13:31         ` Gleb Natapov
  2009-02-04 13:35           ` Avi Kivity
  0 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2009-02-04 13:31 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

On Wed, Feb 04, 2009 at 03:24:59PM +0200, Avi Kivity wrote:
> Gleb Natapov wrote:
>>>>   struct pci_status {
>>>> @@ -603,10 +603,12 @@ static uint32_t gpe_readb(void *opaque, uint32_t addr)
>>>>      struct gpe_regs *g = opaque;
>>>>      switch (addr) {
>>>>          case PROC_BASE:
>>>> -            val = g->up;
>>>> -            break;
>>>>          case PROC_BASE + 1:
>>>> -            val = g->down;
>>>> +            val = gpe_read_val(g->cpus_sts, addr);
>>>> +            break;
>>>> +        case PROC_BASE + 2:
>>>> +        case PROC_BASE + 3:
>>>> +            val = gpe_read_val(g->bios_cpus_sts, addr);
>>>>              break;
>>>>         
>>> Why can't the bios maintain bios_cpu_sts in RAM?
>>>
>>>     
>> It can, just need to find a place for it. Currently our AML does not use
>> RAM at all.
>
> OperationRegion(..., SystemMemory, ...) should work.  It's better to  
> avoid introducing unnecessary virtual hardware.
>
But what address to chose. It needs to be reserved in e820 map and S3
resume should not touch it.

--
			Gleb.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] Fix CPU hotplug
  2009-02-04 13:31         ` Gleb Natapov
@ 2009-02-04 13:35           ` Avi Kivity
  2009-02-04 13:36             ` Gleb Natapov
  0 siblings, 1 reply; 12+ messages in thread
From: Avi Kivity @ 2009-02-04 13:35 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: kvm

Gleb Natapov wrote:

    

>>> It can, just need to find a place for it. Currently our AML does not use
>>> RAM at all.
>>>       
>> OperationRegion(..., SystemMemory, ...) should work.  It's better to  
>> avoid introducing unnecessary virtual hardware.
>>
>>     
> But what address to chose. It needs to be reserved in e820 map and S3
> resume should not touch it.
>
>   

The Extended BIOS Data Area, at 640K - epsilon.

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] Fix CPU hotplug
  2009-02-04 13:35           ` Avi Kivity
@ 2009-02-04 13:36             ` Gleb Natapov
  0 siblings, 0 replies; 12+ messages in thread
From: Gleb Natapov @ 2009-02-04 13:36 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

On Wed, Feb 04, 2009 at 03:35:16PM +0200, Avi Kivity wrote:
> Gleb Natapov wrote:
>
>    
>
>>>> It can, just need to find a place for it. Currently our AML does not use
>>>> RAM at all.
>>>>       
>>> OperationRegion(..., SystemMemory, ...) should work.  It's better to  
>>> avoid introducing unnecessary virtual hardware.
>>>
>>>     
>> But what address to chose. It needs to be reserved in e820 map and S3
>> resume should not touch it.
>>
>>   
>
> The Extended BIOS Data Area, at 640K - epsilon.
>
EBDA can be moved by add-on BIOS so epsilon may change depending on your
HW.

--
			Gleb.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2009-02-04 13:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-04  9:58 [PATCH 1/3] Fix GPE registers read/write handling Gleb Natapov
2009-02-04  9:58 ` [PATCH 2/3] Fix CPU hotplug Gleb Natapov
2009-02-04 12:45   ` Avi Kivity
2009-02-04 13:03     ` Gleb Natapov
2009-02-04 13:24       ` Avi Kivity
2009-02-04 13:31         ` Gleb Natapov
2009-02-04 13:35           ` Avi Kivity
2009-02-04 13:36             ` Gleb Natapov
2009-02-04  9:58 ` [PATCH 3/3] PCI hotplug fixes Gleb Natapov
2009-02-04 12:29 ` [PATCH 1/3] Fix GPE registers read/write handling Avi Kivity
2009-02-04 12:39   ` Gleb Natapov
2009-02-04 12:46     ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox