| Message ID | 4C18015C.1070306@web.de |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > 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. > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/hpet.c | 7 +------ > 1 files changed, 1 insertions(+), 6 deletions(-) > > diff --git a/hw/hpet.c b/hw/hpet.c > index d5c406c..ed4e995 100644 > --- a/hw/hpet.c > +++ b/hw/hpet.c > @@ -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; > -- > 1.6.0.2 -- Gleb.
Gleb Natapov wrote: > On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> 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 > 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. > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> hw/hpet.c | 7 +------ >> 1 files changed, 1 insertions(+), 6 deletions(-) >> >> diff --git a/hw/hpet.c b/hw/hpet.c >> index d5c406c..ed4e995 100644 >> --- a/hw/hpet.c >> +++ b/hw/hpet.c >> @@ -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; >> -- >> 1.6.0.2 > > -- > Gleb.
On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: > >> From: Jan Kiszka <jan.kiszka@siemens.com> > >> > >> 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 ;) > Jan > > > 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. > > > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > >> --- > >> hw/hpet.c | 7 +------ > >> 1 files changed, 1 insertions(+), 6 deletions(-) > >> > >> diff --git a/hw/hpet.c b/hw/hpet.c > >> index d5c406c..ed4e995 100644 > >> --- a/hw/hpet.c > >> +++ b/hw/hpet.c > >> @@ -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; > >> -- > >> 1.6.0.2 > > > > -- > > Gleb. > > -- Gleb.
Gleb Natapov wrote: > On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: >> Gleb Natapov wrote: >>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: >>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> 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 ;) 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
On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: > >> Gleb Natapov wrote: > >>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: > >>>> From: Jan Kiszka <jan.kiszka@siemens.com> > >>>> > >>>> 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 ;) > > 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.
Gleb Natapov wrote: > On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: >> Gleb Natapov wrote: >>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: >>>> Gleb Natapov wrote: >>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: >>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>>>> >>>>>> 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 ;) >> 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
On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: > >> Gleb Natapov wrote: > >>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: > >>>> Gleb Natapov wrote: > >>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: > >>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> > >>>>>> > >>>>>> 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 ;) > >> 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? -- Gleb.
Gleb Natapov wrote: > On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: >> Gleb Natapov wrote: >>> On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: >>>> Gleb Natapov wrote: >>>>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: >>>>>> Gleb Natapov wrote: >>>>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: >>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>>>>>> >>>>>>>> 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 ;) >>>> 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? Isn't there some key that was always there and will always be? Jan
On Wed, Jun 16, 2010 at 11:33:13AM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: > >> Gleb Natapov wrote: > >>> On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: > >>>> Gleb Natapov wrote: > >>>>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: > >>>>>> Gleb Natapov wrote: > >>>>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: > >>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> > >>>>>>>> > >>>>>>>> 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 ;) > >>>> 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? > > Isn't there some key that was always there and will always be? > FW_CFG_SIGNATURE -- Gleb.
On Wed, Jun 16, 2010 at 12:35:16PM +0300, Gleb Natapov wrote: > On Wed, Jun 16, 2010 at 11:33:13AM +0200, Jan Kiszka wrote: > > Gleb Natapov wrote: > > > On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: > > >> Gleb Natapov wrote: > > >>> On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: > > >>>> Gleb Natapov wrote: > > >>>>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: > > >>>>>> Gleb Natapov wrote: > > >>>>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: > > >>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> > > >>>>>>>> > > >>>>>>>> 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 ;) > > >>>> 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? > > > > 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.
Gleb Natapov wrote: > On Wed, Jun 16, 2010 at 12:35:16PM +0300, Gleb Natapov wrote: >> On Wed, Jun 16, 2010 at 11:33:13AM +0200, Jan Kiszka wrote: >>> Gleb Natapov wrote: >>>> On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: >>>>> Gleb Natapov wrote: >>>>>> On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: >>>>>>> Gleb Natapov wrote: >>>>>>>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: >>>>>>>>> Gleb Natapov wrote: >>>>>>>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: >>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>>>>>>>>> >>>>>>>>>>> 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 ;) >>>>>>> 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? >>> 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
On Wed, Jun 16, 2010 at 06:00:56PM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Wed, Jun 16, 2010 at 12:35:16PM +0300, Gleb Natapov wrote: > >> On Wed, Jun 16, 2010 at 11:33:13AM +0200, Jan Kiszka wrote: > >>> Gleb Natapov wrote: > >>>> On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: > >>>>> Gleb Natapov wrote: > >>>>>> On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: > >>>>>>> Gleb Natapov wrote: > >>>>>>>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: > >>>>>>>>> Gleb Natapov wrote: > >>>>>>>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: > >>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> > >>>>>>>>>>> > >>>>>>>>>>> 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 ;) > >>>>>>> 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? > >>> 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? > 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.
Gleb Natapov wrote: > On Wed, Jun 16, 2010 at 06:00:56PM +0200, Jan Kiszka wrote: >> Gleb Natapov wrote: >>> On Wed, Jun 16, 2010 at 12:35:16PM +0300, Gleb Natapov wrote: >>>> On Wed, Jun 16, 2010 at 11:33:13AM +0200, Jan Kiszka wrote: >>>>> Gleb Natapov wrote: >>>>>> On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: >>>>>>> Gleb Natapov wrote: >>>>>>>> On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: >>>>>>>>> Gleb Natapov wrote: >>>>>>>>>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: >>>>>>>>>>> Gleb Natapov wrote: >>>>>>>>>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: >>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>>>>>>>>>>> >>>>>>>>>>>>> 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 ;) >>>>>>>>> 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? >>>>> 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? >> > 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
On Thu, Jun 17, 2010 at 09:17:51AM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Wed, Jun 16, 2010 at 06:00:56PM +0200, Jan Kiszka wrote: > >> Gleb Natapov wrote: > >>> On Wed, Jun 16, 2010 at 12:35:16PM +0300, Gleb Natapov wrote: > >>>> On Wed, Jun 16, 2010 at 11:33:13AM +0200, Jan Kiszka wrote: > >>>>> Gleb Natapov wrote: > >>>>>> On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: > >>>>>>> Gleb Natapov wrote: > >>>>>>>> On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: > >>>>>>>>> Gleb Natapov wrote: > >>>>>>>>>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: > >>>>>>>>>>> Gleb Natapov wrote: > >>>>>>>>>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: > >>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> > >>>>>>>>>>>>> > >>>>>>>>>>>>> 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 ;) > >>>>>>>>> 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? > >>>>> 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? > >> > > 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.
Gleb Natapov wrote: > On Thu, Jun 17, 2010 at 09:17:51AM +0200, Jan Kiszka wrote: >> Gleb Natapov wrote: >>> On Wed, Jun 16, 2010 at 06:00:56PM +0200, Jan Kiszka wrote: >>>> Gleb Natapov wrote: >>>>> On Wed, Jun 16, 2010 at 12:35:16PM +0300, Gleb Natapov wrote: >>>>>> On Wed, Jun 16, 2010 at 11:33:13AM +0200, Jan Kiszka wrote: >>>>>>> Gleb Natapov wrote: >>>>>>>> On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: >>>>>>>>> Gleb Natapov wrote: >>>>>>>>>> On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: >>>>>>>>>>> Gleb Natapov wrote: >>>>>>>>>>>> On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: >>>>>>>>>>>>> Gleb Natapov wrote: >>>>>>>>>>>>>> On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: >>>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> 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 ;) >>>>>>>>>>> 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? >>>>>>> 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? >>>> >>> 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. > 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
On Thu, Jun 17, 2010 at 10:30:15AM +0200, Jan Kiszka wrote: > > 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. > > 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.
Gleb Natapov wrote: > On Thu, Jun 17, 2010 at 10:30:15AM +0200, Jan Kiszka wrote: >>> 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. > >>> 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. Yes, read-back would mean returning FWCfgState::cur_entry. And that will be -1 when selected an invalid one. Jan
On Thu, Jun 17, 2010 at 10:42:34AM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Thu, Jun 17, 2010 at 10:30:15AM +0200, Jan Kiszka wrote: > >>> 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. > > > > >>> 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. > > 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.
Gleb Natapov wrote: > On Thu, Jun 17, 2010 at 10:42:34AM +0200, Jan Kiszka wrote: >> Gleb Natapov wrote: >>> On Thu, Jun 17, 2010 at 10:30:15AM +0200, Jan Kiszka wrote: >>>>> 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. >> >>>>> 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. >> 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
On Thu, Jun 17, 2010 at 10:59:01AM +0200, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Thu, Jun 17, 2010 at 10:42:34AM +0200, Jan Kiszka wrote: > >> Gleb Natapov wrote: > >>> On Thu, Jun 17, 2010 at 10:30:15AM +0200, Jan Kiszka wrote: > >>>>> 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. > >> > >>>>> 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. > >> 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.
diff --git a/hw/hpet.c b/hw/hpet.c index d5c406c..ed4e995 100644 --- a/hw/hpet.c +++ b/hw/hpet.c @@ -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;