hpet: Clean up initial hpet counter

PATCH In review Jan Kiszka 2 participants 21 messages Jun 15, 2010 – Jun 17, 2010

lore

Share & tools

lore

Jan Kiszka

author

2010-06-15 22:40 UTC #0 lore raw

PATCH 1/1 hpet: Clean up initial hpet counter

From: Jan Kiszka <[email protected]>

There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/hpet.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

hw/hpet.c +1 -6

index d5c406c..ed4e995 100644
@@ -74,7 +74,7 @@ typedef struct HPETState {
     uint8_t  hpet_id;           /* instance id */
 } HPETState;
 
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
+struct hpet_fw_config hpet_cfg;
 
 static uint32_t hpet_in_legacy_mode(HPETState *s)
 {

@@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
     int i, iomemtype;
     HPETTimer *timer;
 
-    if (hpet_cfg.count == UINT8_MAX) {
-        /* first instance */
-        hpet_cfg.count = 0;
-    }
-
     if (hpet_cfg.count == 8) {
         fprintf(stderr, "Only 8 instances of HPET is allowed\n");
         return -1;

Signature · 1 line

1.6.0.2

Gleb Natapov

2010-06-16 04:40 UTC #1 lore raw

Quoted 6 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.

I want seabios to be able to distinguish between old qemu and new one. Hence special value. I used it incorrectly in may v2 seabios patch. Will resend asap. Will teach me to not change logic at the last minute :( I removed "valid" field between v1 and v2 of the patches and introduces special value for count instead. As a result I made one bug in qemu and one is seabios. Heh.

Quoted 32 lines

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/hpet.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

hw/hpet.c +1 -6

index d5c406c..ed4e995 100644
@@ -74,7 +74,7 @@ typedef struct HPETState {
     uint8_t  hpet_id;           /* instance id */
 } HPETState;
 
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
+struct hpet_fw_config hpet_cfg;
 
 static uint32_t hpet_in_legacy_mode(HPETState *s)
 {

@@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
     int i, iomemtype;
     HPETTimer *timer;
 
-    if (hpet_cfg.count == UINT8_MAX) {
-        /* first instance */
-        hpet_cfg.count = 0;
-    }
-
     if (hpet_cfg.count == 8) {
         fprintf(stderr, "Only 8 instances of HPET is allowed\n");
         return -1;

Signature · 1 line

1.6.0.2

--
Gleb.

Jan Kiszka

author

2010-06-16 07:03 UTC #2 lore raw

Quoted 8 lines · Gleb Natapov

Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.

I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?

Jan

Quoted 41 lines

Hence special value. I used it incorrectly in may v2 seabios patch. Will resend asap. Will teach me to not change logic at the last minute :( I removed "valid" field between v1 and v2 of the patches and introduces special value for count instead. As a result I made one bug in qemu and one is seabios. Heh.

Quoted 32 lines

Signed-off-by Jan Kiszka <[email protected]>

---
 hw/hpet.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

hw/hpet.c +1 -6

index d5c406c..ed4e995 100644
@@ -74,7 +74,7 @@ typedef struct HPETState {
     uint8_t  hpet_id;           /* instance id */
 } HPETState;
 
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
+struct hpet_fw_config hpet_cfg;
 
 static uint32_t hpet_in_legacy_mode(HPETState *s)
 {

@@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
     int i, iomemtype;
     HPETTimer *timer;
 
-    if (hpet_cfg.count == UINT8_MAX) {
-        /* first instance */
-        hpet_cfg.count = 0;
-    }
-
     if (hpet_cfg.count == 8) {
         fprintf(stderr, "Only 8 instances of HPET is allowed\n");
         return -1;

Signature · 1 line

1.6.0.2

--
Gleb.

Gleb Natapov

2010-06-16 07:33 UTC #3 lore raw

Quoted 4 lines · Jan Kiszka

Quoted 3 lines · Gleb Natapov
Quoted 2 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>

Quoted 9 lines

Quoted 5 lines
Quoted 4 lines
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?

Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)

Quoted 45 lines

Jan
Quoted 41 lines
Hence special value. I used it incorrectly in may v2 seabios patch. Will resend asap. Will teach me to not change logic at the last minute :( I removed "valid" field between v1 and v2 of the patches and introduces special value for count instead. As a result I made one bug in qemu and one is seabios. Heh.
Quoted 32 lines
Signed-off-by Jan Kiszka <[email protected]>
---
 hw/hpet.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)
hw/hpet.c +1 -6
index d5c406c..ed4e995 100644
@@ -74,7 +74,7 @@ typedef struct HPETState {
     uint8_t  hpet_id;           /* instance id */
 } HPETState;
 
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
+struct hpet_fw_config hpet_cfg;
 
 static uint32_t hpet_in_legacy_mode(HPETState *s)
 {
@@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
     int i, iomemtype;
     HPETTimer *timer;
 
-    if (hpet_cfg.count == UINT8_MAX) {
-        /* first instance */
-        hpet_cfg.count = 0;
-    }
-
     if (hpet_cfg.count == 8) {
         fprintf(stderr, "Only 8 instances of HPET is allowed\n");
         return -1;
Signature · 1 line
1.6.0.2
--
Gleb.

--
Gleb.

Jan Kiszka

author

2010-06-16 07:51 UTC #4 lore raw

Quoted 9 lines · Gleb Natapov

Quoted 8 lines · Jan Kiszka
Quoted 7 lines · Gleb Natapov
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)

That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.

Jan

Gleb Natapov

2010-06-16 07:53 UTC #5 lore raw

Quoted 14 lines · Jan Kiszka

Quoted 13 lines · Gleb Natapov
Quoted 12 lines · Jan Kiszka
Quoted 8 lines · Gleb Natapov
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?

Quoted 8 lines

Quoted 3 lines
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.

That will not support holes in key space, and our key space is already sparse.

--
Gleb.

Jan Kiszka

author

2010-06-16 07:57 UTC #6 lore raw

Quoted 18 lines · Gleb Natapov

Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Gleb Natapov
Quoted 4 lines · Jan Kiszka
Quoted 3 lines · Gleb Natapov
Quoted 2 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)

Quoted 6 lines

Quoted 4 lines
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.

Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...

Jan

Gleb Natapov

2010-06-16 09:07 UTC #7 lore raw

Quoted 23 lines · Jan Kiszka

Quoted 22 lines · Gleb Natapov
Quoted 21 lines · Jan Kiszka
Quoted 9 lines · Gleb Natapov
Quoted 8 lines · Jan Kiszka
Quoted 7 lines · Gleb Natapov
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.

Quoted 6 lines

Quoted 2 lines
That will not support holes in key space, and our key space is already sparse.
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...

Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?

--
Gleb.

Jan Kiszka

author

2010-06-16 09:34 UTC #8 lore raw

Quoted 26 lines · Gleb Natapov

Quoted 25 lines · Jan Kiszka
Quoted 24 lines · Gleb Natapov
Quoted 14 lines · Jan Kiszka
Quoted 13 lines · Gleb Natapov
Quoted 12 lines · Jan Kiszka
Quoted 8 lines · Gleb Natapov
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Quoted 7 lines
Quoted 3 lines
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.

Quoted 7 lines

Quoted 3 lines
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...
Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?

Isn't there some key that was always there and will always be?

Jan

Gleb Natapov

2010-06-16 09:35 UTC #9 lore raw

Quoted 30 lines · Jan Kiszka

Quoted 29 lines · Gleb Natapov
Quoted 28 lines · Jan Kiszka
Quoted 18 lines · Gleb Natapov
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Gleb Natapov
Quoted 4 lines · Jan Kiszka
Quoted 3 lines · Gleb Natapov
Quoted 2 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
Quoted 6 lines
Quoted 4 lines
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...

Quoted 7 lines

Quoted 4 lines
Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?
Isn't there some key that was always there and will always be?

FW_CFG_SIGNATURE

--
Gleb.

Gleb Natapov

2010-06-16 15:36 UTC #10 lore raw

Quoted 31 lines · Gleb Natapov

Quoted 30 lines · Jan Kiszka
Quoted 29 lines · Gleb Natapov
Quoted 28 lines · Jan Kiszka
Quoted 18 lines · Gleb Natapov
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Gleb Natapov
Quoted 4 lines · Jan Kiszka
Quoted 3 lines · Gleb Natapov
Quoted 2 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
Quoted 6 lines
Quoted 4 lines
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...

Quoted 9 lines

Quoted 7 lines
Quoted 4 lines
Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?
Isn't there some key that was always there and will always be?
FW_CFG_SIGNATURE

So any ideas? Or did I misunderstood your hint? ;)

--
Gleb.

Jan Kiszka

author

2010-06-16 16:01 UTC #11 lore raw

Quoted 36 lines · Gleb Natapov

Quoted 35 lines · Gleb Natapov
Quoted 34 lines · Jan Kiszka
Quoted 33 lines · Gleb Natapov
Quoted 19 lines · Jan Kiszka
Quoted 18 lines · Gleb Natapov
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Gleb Natapov
Quoted 4 lines · Jan Kiszka
Quoted 3 lines · Gleb Natapov
Quoted 2 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...
Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?

Quoted 5 lines

Quoted 4 lines
Quoted 2 lines
Isn't there some key that was always there and will always be?
FW_CFG_SIGNATURE
So any ideas? Or did I misunderstood your hint? ;)

I thought you found the answer yourself:

Seabios could select FW_CFG_SIGNATURE and then perform a read-back on the control register. Older QEMUs will return -1, versions that support the read-back 0. Problem solved, no?

Jan

Gleb Natapov

2010-06-17 05:49 UTC #12 lore raw

Quoted 41 lines · Jan Kiszka

Quoted 40 lines · Gleb Natapov
Quoted 39 lines · Gleb Natapov
Quoted 27 lines · Jan Kiszka
Quoted 26 lines · Gleb Natapov
Quoted 25 lines · Jan Kiszka
Quoted 24 lines · Gleb Natapov
Quoted 14 lines · Jan Kiszka
Quoted 13 lines · Gleb Natapov
Quoted 12 lines · Jan Kiszka
Quoted 8 lines · Gleb Natapov
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Quoted 7 lines
Quoted 3 lines
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.
Quoted 9 lines
Quoted 7 lines
Quoted 3 lines
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...
Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?
Isn't there some key that was always there and will always be?
FW_CFG_SIGNATURE

Quoted 8 lines

Quoted 1 line
So any ideas? Or did I misunderstood your hint? ;)
I thought you found the answer yourself:
Seabios could select FW_CFG_SIGNATURE and then perform a read-back on the control register. Older QEMUs will return -1, versions that support the read-back 0. Problem solved, no?

AFAIK QEMU returns 0 if io read was done from non-used port or mmio address, but can we rely on this? If we can then problem solved, if we can't then no.

--
Gleb.

Jan Kiszka

author

2010-06-17 07:17 UTC #13 lore raw

Quoted 45 lines · Gleb Natapov

Quoted 44 lines · Jan Kiszka
Quoted 36 lines · Gleb Natapov
Quoted 35 lines · Gleb Natapov
Quoted 34 lines · Jan Kiszka
Quoted 33 lines · Gleb Natapov
Quoted 19 lines · Jan Kiszka
Quoted 18 lines · Gleb Natapov
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Gleb Natapov
Quoted 4 lines · Jan Kiszka
Quoted 3 lines · Gleb Natapov
Quoted 2 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines
There is no need starting with the special value for hpet_cfg.count.
Either Seabios is aware of the new firmware interface and properly
interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...
Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Isn't there some key that was always there and will always be?
FW_CFG_SIGNATURE
So any ideas? Or did I misunderstood your hint? ;)
I thought you found the answer yourself:

Quoted 7 lines

Quoted 4 lines
Seabios could select FW_CFG_SIGNATURE and then perform a read-back on the control register. Older QEMUs will return -1, versions that support the read-back 0. Problem solved, no?
AFAIK QEMU returns 0 if io read was done from non-used port or mmio address, but can we rely on this? If we can then problem solved, if we can't then no.

It works for IO-based fw-cfg, but not for MMIO-based. So the firmware should probably pick a non-zero key for this check, e.g. FW_CFG_ID.

Jan

Gleb Natapov

2010-06-17 08:08 UTC #14 lore raw

Quoted 50 lines · Jan Kiszka

Quoted 49 lines · Gleb Natapov
Quoted 48 lines · Jan Kiszka
Quoted 36 lines · Gleb Natapov
Quoted 35 lines · Gleb Natapov
Quoted 34 lines · Jan Kiszka
Quoted 33 lines · Gleb Natapov
Quoted 19 lines · Jan Kiszka
Quoted 18 lines · Gleb Natapov
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Gleb Natapov
Quoted 4 lines · Jan Kiszka
Quoted 3 lines · Gleb Natapov
On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote:
From: Jan Kiszka <[email protected]>
Quoted 8 lines
Quoted 5 lines
There is no need starting with the special value for hpet_cfg.count.
Either Seabios is aware of the new firmware interface and properly
interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...
Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Isn't there some key that was always there and will always be?
FW_CFG_SIGNATURE
So any ideas? Or did I misunderstood your hint? ;)
I thought you found the answer yourself:
Seabios could select FW_CFG_SIGNATURE and then perform a read-back on the control register. Older QEMUs will return -1, versions that support the read-back 0. Problem solved, no?

Quoted 7 lines

Quoted 3 lines
AFAIK QEMU returns 0 if io read was done from non-used port or mmio address, but can we rely on this? If we can then problem solved, if we can't then no.
It works for IO-based fw-cfg, but not for MMIO-based. So the firmware should probably pick a non-zero key for this check, e.g. FW_CFG_ID.

Sorry, I lost you here. What "works for IO-based fw-cfg, but not for MMIO-based". Can you write pseudo logic of how you think it all should work?

--
Gleb.

Jan Kiszka

author

2010-06-17 08:30 UTC #15 lore raw

Quoted 51 lines · Gleb Natapov

Quoted 50 lines · Jan Kiszka
Quoted 49 lines · Gleb Natapov
Quoted 48 lines · Jan Kiszka
Quoted 36 lines · Gleb Natapov
Quoted 35 lines · Gleb Natapov
Quoted 34 lines · Jan Kiszka
Quoted 33 lines · Gleb Natapov
Quoted 19 lines · Jan Kiszka
Quoted 18 lines · Gleb Natapov
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Gleb Natapov
Quoted 12 lines · Jan Kiszka
Gleb Natapov wrote:
On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote:
From: Jan Kiszka <[email protected]>
There is no need starting with the special value for hpet_cfg.count.
Either Seabios is aware of the new firmware interface and properly
interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic
interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;)
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0.
That will not support holes in key space, and our key space is already sparse.
Then add a service to obtain a bitmap of supported keys. If that bitmap is empty...
Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that?
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Isn't there some key that was always there and will always be?
FW_CFG_SIGNATURE
So any ideas? Or did I misunderstood your hint? ;)
I thought you found the answer yourself:
Seabios could select FW_CFG_SIGNATURE and then perform a read-back on the control register. Older QEMUs will return -1, versions that support the read-back 0. Problem solved, no?

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
AFAIK QEMU returns 0 if io read was done from non-used port or mmio address, but can we rely on this? If we can then problem solved, if we can't then no.
It works for IO-based fw-cfg, but not for MMIO-based. So the firmware should probably pick a non-zero key for this check, e.g. FW_CFG_ID.
Sorry, I lost you here. What "works for IO-based fw-cfg, but not for MMIO-based".

Undefined IO ports return -1, undefined (/wrt read access) MMIO 0. So you need to select a key that is different from both.

Quoted 2 lines

Can you write pseudo logic of how you think it
all should work?

The firmware should do this:

write(CTL_BASE, FW_CFG_ID);
if (read(CTL_BASE) != FW_CFG_ID)

	deal_with_old_qemu();
else
	check_for_supported_keys();

Jan

Gleb Natapov

2010-06-17 08:36 UTC #16 lore raw

Quoted 6 lines · Jan Kiszka

Quoted 2 lines
Sorry, I lost you here. What "works for IO-based fw-cfg, but not for MMIO-based".
Undefined IO ports return -1, undefined (/wrt read access) MMIO 0. So you need to select a key that is different from both.

But can we rely on it? Is this defined somewhere or if it happens to be the case in current qemu for x86 arch.

Quoted 11 lines

Quoted 2 lines
Can you write pseudo logic of how you think it
all should work?
The firmware should do this:
write(CTL_BASE, FW_CFG_ID);
if (read(CTL_BASE) != FW_CFG_ID)
	deal_with_old_qemu();
else
	check_for_supported_keys();

Ah, I thought about read() returning 0/1, not key itself, so any key that always existed would do.

--
Gleb.

Jan Kiszka

author

2010-06-17 08:42 UTC #17 lore raw

Quoted 8 lines · Gleb Natapov

Quoted 5 lines · Jan Kiszka
Quoted 2 lines
Sorry, I lost you here. What "works for IO-based fw-cfg, but not for MMIO-based".
Undefined IO ports return -1, undefined (/wrt read access) MMIO 0. So you need to select a key that is different from both.
But can we rely on it? Is this defined somewhere or if it happens to be the case in current qemu for x86 arch.

For x86 with its port-based access, we are on the safe side as (pre-pnp) device probing used to work this way. Can't tell for the other archs that support fw-cfg.

Quoted 5 lines

Quoted 5 lines
Quoted 3 lines
Can you write pseudo logic of how you think it
all should work?
The firmware should do this:

Quoted 8 lines

Quoted 6 lines
write(CTL_BASE, FW_CFG_ID);
if (read(CTL_BASE) != FW_CFG_ID)
	deal_with_old_qemu();
else
	check_for_supported_keys();
Ah, I thought about read() returning 0/1, not key itself, so any key that always existed would do.

Yes, read-back would mean returning FWCfgState::cur_entry. And that will be -1 when selected an invalid one.

Jan

Gleb Natapov

2010-06-17 08:56 UTC #18 lore raw

Quoted 25 lines · Jan Kiszka

Quoted 8 lines · Gleb Natapov
Quoted 5 lines · Jan Kiszka
Quoted 2 lines
Sorry, I lost you here. What "works for IO-based fw-cfg, but not for MMIO-based".
Undefined IO ports return -1, undefined (/wrt read access) MMIO 0. So you need to select a key that is different from both.
But can we rely on it? Is this defined somewhere or if it happens to be the case in current qemu for x86 arch.
For x86 with its port-based access, we are on the safe side as (pre-pnp) device probing used to work this way. Can't tell for the other archs that support fw-cfg.
Quoted 11 lines
Quoted 11 lines
Quoted 3 lines
Can you write pseudo logic of how you think it
all should work?
The firmware should do this:
write(CTL_BASE, FW_CFG_ID);
if (read(CTL_BASE) != FW_CFG_ID)
	deal_with_old_qemu();
else
	check_for_supported_keys();

Quoted 6 lines

Quoted 2 lines
Ah, I thought about read() returning 0/1, not key itself, so any key that always existed would do.
Yes, read-back would mean returning FWCfgState::cur_entry. And that will be -1 when selected an invalid one.

Heh, actually I have better idea. Why not advance FW_CFG_ID to version 2.

--
Gleb.

Jan Kiszka

author

2010-06-17 08:59 UTC #19 lore raw

Quoted 24 lines · Gleb Natapov

Quoted 23 lines · Jan Kiszka
Quoted 8 lines · Gleb Natapov
Quoted 5 lines · Jan Kiszka
Quoted 2 lines
Sorry, I lost you here. What "works for IO-based fw-cfg, but not for MMIO-based".
Undefined IO ports return -1, undefined (/wrt read access) MMIO 0. So you need to select a key that is different from both.
But can we rely on it? Is this defined somewhere or if it happens to be the case in current qemu for x86 arch.
For x86 with its port-based access, we are on the safe side as (pre-pnp) device probing used to work this way. Can't tell for the other archs that support fw-cfg.
Quoted 10 lines
Quoted 10 lines
Quoted 2 lines
Can you write pseudo logic of how you think it
all should work?
The firmware should do this:
write(CTL_BASE, FW_CFG_ID);
if (read(CTL_BASE) != FW_CFG_ID)
	deal_with_old_qemu();
else
	check_for_supported_keys();

Quoted 6 lines

Quoted 5 lines
Quoted 2 lines
Ah, I thought about read() returning 0/1, not key itself, so any key that always existed would do.
Yes, read-back would mean returning FWCfgState::cur_entry. And that will be -1 when selected an invalid one.
Heh, actually I have better idea. Why not advance FW_CFG_ID to version 2.

If that is supposed to be a version number - yeah, good idea.

Jan

Gleb Natapov

2010-06-17 09:01 UTC #20 lore raw

Quoted 25 lines · Jan Kiszka

Quoted 24 lines · Gleb Natapov
Quoted 23 lines · Jan Kiszka
Quoted 8 lines · Gleb Natapov
Quoted 5 lines · Jan Kiszka
Quoted 2 lines
Sorry, I lost you here. What "works for IO-based fw-cfg, but not for MMIO-based".
Undefined IO ports return -1, undefined (/wrt read access) MMIO 0. So you need to select a key that is different from both.
But can we rely on it? Is this defined somewhere or if it happens to be the case in current qemu for x86 arch.
For x86 with its port-based access, we are on the safe side as (pre-pnp) device probing used to work this way. Can't tell for the other archs that support fw-cfg.
Quoted 10 lines
Quoted 10 lines
Quoted 2 lines
Can you write pseudo logic of how you think it
all should work?
The firmware should do this:
write(CTL_BASE, FW_CFG_ID);
if (read(CTL_BASE) != FW_CFG_ID)
	deal_with_old_qemu();
else
	check_for_supported_keys();

Quoted 9 lines

Quoted 6 lines
Quoted 5 lines
Quoted 2 lines
Ah, I thought about read() returning 0/1, not key itself, so any key that always existed would do.
Yes, read-back would mean returning FWCfgState::cur_entry. And that will be -1 when selected an invalid one.
Heh, actually I have better idea. Why not advance FW_CFG_ID to version 2.
If that is supposed to be a version number - yeah, good idea.

That was the idea behind it. I just forgot it exists.

--
Gleb.