diff mbox

[FYI,4/4] prep: Quickfix for ioport

Message ID 1292287758-8009-5-git-send-email-andreas.faerber@web.de
State New
Headers show

Commit Message

Andreas Färber Dec. 14, 2010, 12:49 a.m. UTC
Workaround the following error:

qemu: hardware error: register_ioport_read: invalid opaque

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/ppc_prep.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Comments

Andreas Färber Dec. 26, 2010, 11:28 p.m. UTC | #1
Am 14.12.2010 um 01:49 schrieb Andreas Färber:

> Workaround the following error:
>
> qemu: hardware error: register_ioport_read: invalid opaque

I found out that this is a conflict with i8042 registering I/O port  
0x0092 in pckbd.c, too.
Not sure what the proper fix should look like - add some qdev property  
to ISAKBDState to disable the port?

Andreas

> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
> hw/ppc_prep.c |    2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 3073870..0c9183e 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -721,8 +721,10 @@ static void ppc_prep_init (ram_addr_t ram_size,
>   register_ioport_read(0x398, 2, 1, &PREP_io_read, sysctrl);
>   register_ioport_write(0x398, 2, 1, &PREP_io_write, sysctrl);
>   /* System control ports */
> +#if 0
>   register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
>   register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb,  
> sysctrl);
> +#endif
>   register_ioport_read(0x0800, 0x52, 1, &PREP_io_800_readb, sysctrl);
>   register_ioport_write(0x0800, 0x52, 1, &PREP_io_800_writeb,  
> sysctrl);
>   /* PCI intack location */
> -- 
> 1.7.3
>
>
Alexander Graf Dec. 27, 2010, 12:11 a.m. UTC | #2
Am 27.12.2010 um 00:28 schrieb Andreas Färber <andreas.faerber@web.de>:

> Am 14.12.2010 um 01:49 schrieb Andreas Färber:
> 
>> Workaround the following error:
>> 
>> qemu: hardware error: register_ioport_read: invalid opaque
> 
> I found out that this is a conflict with i8042 registering I/O port 0x0092 in pckbd.c, too.
> Not sure what the proper fix should look like - add some qdev property to ISAKBDState to disable the port?

Well, why does the PREP board have its own mapping there in the first place? It should be the same as a PC from a port pov, no?

Alex


> 
> Andreas
> 
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>> ---
>> hw/ppc_prep.c |    2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>> 
>> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
>> index 3073870..0c9183e 100644
>> --- a/hw/ppc_prep.c
>> +++ b/hw/ppc_prep.c
>> @@ -721,8 +721,10 @@ static void ppc_prep_init (ram_addr_t ram_size,
>>  register_ioport_read(0x398, 2, 1, &PREP_io_read, sysctrl);
>>  register_ioport_write(0x398, 2, 1, &PREP_io_write, sysctrl);
>>  /* System control ports */
>> +#if 0
>>  register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
>>  register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb, sysctrl);
>> +#endif
>>  register_ioport_read(0x0800, 0x52, 1, &PREP_io_800_readb, sysctrl);
>>  register_ioport_write(0x0800, 0x52, 1, &PREP_io_800_writeb, sysctrl);
>>  /* PCI intack location */
>> -- 
>> 1.7.3
>> 
>> 
>
Andreas Färber Dec. 27, 2010, 12:25 a.m. UTC | #3
Am 27.12.2010 um 01:11 schrieb Alexander Graf:

> Am 27.12.2010 um 00:28 schrieb Andreas Färber  
> <andreas.faerber@web.de>:
>
>> Am 14.12.2010 um 01:49 schrieb Andreas Färber:
>>
>>> Workaround the following error:
>>>
>>> qemu: hardware error: register_ioport_read: invalid opaque
>>
>> I found out that this is a conflict with i8042 registering I/O port  
>> 0x0092 in pckbd.c, too.
>> Not sure what the proper fix should look like - add some qdev  
>> property to ISAKBDState to disable the port?
>
> Well, why does the PREP board have its own mapping there in the  
> first place? It should be the same as a PC from a port pov, no?

Why does the i8042 controller have a System Control Port in the first  
place rather than the 'pc' machine? :)

The PReP spec v1.1 has a Special Port 0x0092 that includes two bits of  
significence - softreset and endian mode. So wherever the port is  
registered, the latter behavior is board-specific and does not match  
pckbd.c (A20 gate or HDD activity depending on bit numbering).

A similar issue arises between PReP machines: The ioport currently  
does not allow using a different opaque for reads and writes to the  
same address. So either we need to change ioport or create some  
inheritence/override mechanism for port behavior.

Andreas
Alexander Graf Jan. 4, 2011, 8:57 p.m. UTC | #4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On 27.12.2010, at 01:25, Andreas Färber wrote:

> Am 27.12.2010 um 01:11 schrieb Alexander Graf:
> 
>> Am 27.12.2010 um 00:28 schrieb Andreas Färber <andreas.faerber@web.de>:
>> 
>>> Am 14.12.2010 um 01:49 schrieb Andreas Färber:
>>> 
>>>> Workaround the following error:
>>>> 
>>>> qemu: hardware error: register_ioport_read: invalid opaque
>>> 
>>> I found out that this is a conflict with i8042 registering I/O port 0x0092 in pckbd.c, too.
>>> Not sure what the proper fix should look like - add some qdev property to ISAKBDState to disable the port?
>> 
>> Well, why does the PREP board have its own mapping there in the first place? It should be the same as a PC from a port pov, no?
> 
> Why does the i8042 controller have a System Control Port in the first place rather than the 'pc' machine? :)
> 
> The PReP spec v1.1 has a Special Port 0x0092 that includes two bits of significence - softreset and endian mode. So wherever the port is registered, the latter behavior is board-specific and does not match pckbd.c (A20 gate or HDD activity depending on bit numbering).
> 
> A similar issue arises between PReP machines: The ioport currently does not allow using a different opaque for reads and writes to the same address. So either we need to change ioport or create some inheritence/override mechanism for port behavior.

Hmm - sounds to me like that register simply doesn't belong there then, so a split would be the correct solution. The alternative would be to have the keyboard controller create a bus that those bits send signals to. An x86 system could then implement its A20 gate in x86 specific code while PREP could do its specific stuff.


Alex

-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.16 (Darwin)

iEYEARECAAYFAk0jibgACgkQq7Wi27wfN1PtUACfdx6L4Sk/aR5/sC3QEg1Hciyy
Zm0AnAxaD2jguRq+RcaI8TLsaWGE1gN7
=VioX
-----END PGP SIGNATURE-----
Andreas Färber Jan. 4, 2011, 9:36 p.m. UTC | #5
Am 04.01.2011 um 21:57 schrieb Alexander Graf:

> On 27.12.2010, at 01:25, Andreas Färber wrote:
>
>> Am 27.12.2010 um 01:11 schrieb Alexander Graf:
>>
>>> Am 27.12.2010 um 00:28 schrieb Andreas Färber <andreas.faerber@web.de 
>>> >:
>>>
>>>> Am 14.12.2010 um 01:49 schrieb Andreas Färber:
>>>>
>>>>> Workaround the following error:
>>>>>
>>>>> qemu: hardware error: register_ioport_read: invalid opaque
>>>>
>>>> I found out that this is a conflict with i8042 registering I/O  
>>>> port 0x0092 in pckbd.c, too.
>>>> Not sure what the proper fix should look like - add some qdev  
>>>> property to ISAKBDState to disable the port?
>>>
>>> Well, why does the PREP board have its own mapping there in the  
>>> first place? It should be the same as a PC from a port pov, no?
>>
>> Why does the i8042 controller have a System Control Port in the  
>> first place rather than the 'pc' machine? :)
>>
>> The PReP spec v1.1 has a Special Port 0x0092 that includes two bits  
>> of significence - softreset and endian mode. So wherever the port  
>> is registered, the latter behavior is board-specific and does not  
>> match pckbd.c (A20 gate or HDD activity depending on bit numbering).
>>
>> A similar issue arises between PReP machines: The ioport currently  
>> does not allow using a different opaque for reads and writes to the  
>> same address. So either we need to change ioport or create some  
>> inheritence/override mechanism for port behavior.
>
> Hmm - sounds to me like that register simply doesn't belong there  
> then, so a split would be the correct solution. The alternative  
> would be to have the keyboard controller create a bus that those  
> bits send signals to. An x86 system could then implement its A20  
> gate in x86 specific code while PREP could do its specific stuff.

Coincidentally there's a recent pckbd patch from Blue that resolves  
the conflict. Feel free to review. :)

Any thoughts on the read vs. write port issue? Do we need to register  
both ports in the base device and add hooks to the device state?  
Sounds problematic VMState-wise, just like some of the qemu_irq  
constructs in the ppc devices. (My prep_pci changes are still far from  
perfect, UniNorth has similar issues I noticed.)

Andreas
Alexander Graf Jan. 4, 2011, 9:43 p.m. UTC | #6
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On 04.01.2011, at 22:36, Andreas Färber wrote:

> Am 04.01.2011 um 21:57 schrieb Alexander Graf:
> 
>> On 27.12.2010, at 01:25, Andreas Färber wrote:
>> 
>>> Am 27.12.2010 um 01:11 schrieb Alexander Graf:
>>> 
>>>> Am 27.12.2010 um 00:28 schrieb Andreas Färber <andreas.faerber@web.de>:
>>>> 
>>>>> Am 14.12.2010 um 01:49 schrieb Andreas Färber:
>>>>> 
>>>>>> Workaround the following error:
>>>>>> 
>>>>>> qemu: hardware error: register_ioport_read: invalid opaque
>>>>> 
>>>>> I found out that this is a conflict with i8042 registering I/O port 0x0092 in pckbd.c, too.
>>>>> Not sure what the proper fix should look like - add some qdev property to ISAKBDState to disable the port?
>>>> 
>>>> Well, why does the PREP board have its own mapping there in the first place? It should be the same as a PC from a port pov, no?
>>> 
>>> Why does the i8042 controller have a System Control Port in the first place rather than the 'pc' machine? :)
>>> 
>>> The PReP spec v1.1 has a Special Port 0x0092 that includes two bits of significence - softreset and endian mode. So wherever the port is registered, the latter behavior is board-specific and does not match pckbd.c (A20 gate or HDD activity depending on bit numbering).
>>> 
>>> A similar issue arises between PReP machines: The ioport currently does not allow using a different opaque for reads and writes to the same address. So either we need to change ioport or create some inheritence/override mechanism for port behavior.
>> 
>> Hmm - sounds to me like that register simply doesn't belong there then, so a split would be the correct solution. The alternative would be to have the keyboard controller create a bus that those bits send signals to. An x86 system could then implement its A20 gate in x86 specific code while PREP could do its specific stuff.
> 
> Coincidentally there's a recent pckbd patch from Blue that resolves the conflict. Feel free to review. :)

Ah, sorry :). Still catching up. Just now finished to watch news of the last 1 1/2 weeks :).

> Any thoughts on the read vs. write port issue? Do we need to register both ports in the base device and add hooks to the device state? Sounds problematic VMState-wise, just like some of the qemu_irq constructs in the ppc devices. (My prep_pci changes are still far from perfect, UniNorth has similar issues I noticed.)

Well, the general idea is that a single IO always gets routed to the same device. I don't fully understand why you need separate handlers there tbh. Mind to explain this?


Alex

-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.16 (Darwin)

iEYEARECAAYFAk0jlJcACgkQq7Wi27wfN1N/agCeL6hOC6c+DHjLQ7Q3uXsp1Clo
x94An0lSzs/kvXGynRy1y2UMmLwpltp1
=NkaJ
-----END PGP SIGNATURE-----
Andreas Färber Jan. 4, 2011, 9:59 p.m. UTC | #7
Am 04.01.2011 um 22:43 schrieb Alexander Graf:

> On 04.01.2011, at 22:36, Andreas Färber wrote:
>
>> Am 04.01.2011 um 21:57 schrieb Alexander Graf:
>>
>>> On 27.12.2010, at 01:25, Andreas Färber wrote:
>>>
>>>> Am 27.12.2010 um 01:11 schrieb Alexander Graf:
>>>>
>>>>> Am 27.12.2010 um 00:28 schrieb Andreas Färber <andreas.faerber@web.de 
>>>>> >:
>>>>>
>>>>>> Am 14.12.2010 um 01:49 schrieb Andreas Färber:
>>>>>>
>>>>>>> Workaround the following error:
>>>>>>>
>>>>>>> qemu: hardware error: register_ioport_read: invalid opaque
>>>>>>
>>>>>> I found out that this is a conflict with i8042 registering I/O  
>>>>>> port 0x0092 in pckbd.c, too.
>>>>>> Not sure what the proper fix should look like - add some qdev  
>>>>>> property to ISAKBDState to disable the port?
>>>>>
>>>>> Well, why does the PREP board have its own mapping there in the  
>>>>> first place? It should be the same as a PC from a port pov, no?
>>>>
>>>> Why does the i8042 controller have a System Control Port in the  
>>>> first place rather than the 'pc' machine? :)
>>>>
>>>> The PReP spec v1.1 has a Special Port 0x0092 that includes two  
>>>> bits of significence - softreset and endian mode. So wherever the  
>>>> port is registered, the latter behavior is board-specific and  
>>>> does not match pckbd.c (A20 gate or HDD activity depending on bit  
>>>> numbering).
>>>>
>>>> A similar issue arises between PReP machines: The ioport  
>>>> currently does not allow using a different opaque for reads and  
>>>> writes to the same address. So either we need to change ioport or  
>>>> create some inheritence/override mechanism for port behavior.
>>>
>>> Hmm - sounds to me like that register simply doesn't belong there  
>>> then, so a split would be the correct solution. The alternative  
>>> would be to have the keyboard controller create a bus that those  
>>> bits send signals to. An x86 system could then implement its A20  
>>> gate in x86 specific code while PREP could do its specific stuff.
>>
>> Coincidentally there's a recent pckbd patch from Blue that resolves  
>> the conflict. Feel free to review. :)
>
> Ah, sorry :). Still catching up. Just now finished to watch news of  
> the last 1 1/2 weeks :).

Welcome back. Did they have to pay much? ;)

>> Any thoughts on the read vs. write port issue? Do we need to  
>> register both ports in the base device and add hooks to the device  
>> state? Sounds problematic VMState-wise, just like some of the  
>> qemu_irq constructs in the ppc devices. (My prep_pci changes are  
>> still far from perfect, UniNorth has similar issues I noticed.)
>
> Well, the general idea is that a single IO always gets routed to the  
> same device. I don't fully understand why you need separate handlers  
> there tbh. Mind to explain this?

In short: Inheritance.

The actual issue appears to be snip'ed above, it's about apparent  
subtle machine differences concerning non-standard register uses  
(here, "Motorola ... register" in -M prep vs. nothing in PReP 1.1 spec).

Andreas
Alexander Graf Jan. 4, 2011, 10:02 p.m. UTC | #8
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On 04.01.2011, at 22:59, Andreas Färber wrote:

> Am 04.01.2011 um 22:43 schrieb Alexander Graf:
> 
>> On 04.01.2011, at 22:36, Andreas Färber wrote:
>> 
>>> Am 04.01.2011 um 21:57 schrieb Alexander Graf:
>>> 
>>>> On 27.12.2010, at 01:25, Andreas Färber wrote:
>>>> 
>>>>> Am 27.12.2010 um 01:11 schrieb Alexander Graf:
>>>>> 
>>>>>> Am 27.12.2010 um 00:28 schrieb Andreas Färber <andreas.faerber@web.de>:
>>>>>> 
>>>>>>> Am 14.12.2010 um 01:49 schrieb Andreas Färber:
>>>>>>> 
>>>>>>>> Workaround the following error:
>>>>>>>> 
>>>>>>>> qemu: hardware error: register_ioport_read: invalid opaque
>>>>>>> 
>>>>>>> I found out that this is a conflict with i8042 registering I/O port 0x0092 in pckbd.c, too.
>>>>>>> Not sure what the proper fix should look like - add some qdev property to ISAKBDState to disable the port?
>>>>>> 
>>>>>> Well, why does the PREP board have its own mapping there in the first place? It should be the same as a PC from a port pov, no?
>>>>> 
>>>>> Why does the i8042 controller have a System Control Port in the first place rather than the 'pc' machine? :)
>>>>> 
>>>>> The PReP spec v1.1 has a Special Port 0x0092 that includes two bits of significence - softreset and endian mode. So wherever the port is registered, the latter behavior is board-specific and does not match pckbd.c (A20 gate or HDD activity depending on bit numbering).
>>>>> 
>>>>> A similar issue arises between PReP machines: The ioport currently does not allow using a different opaque for reads and writes to the same address. So either we need to change ioport or create some inheritence/override mechanism for port behavior.
>>>> 
>>>> Hmm - sounds to me like that register simply doesn't belong there then, so a split would be the correct solution. The alternative would be to have the keyboard controller create a bus that those bits send signals to. An x86 system could then implement its A20 gate in x86 specific code while PREP could do its specific stuff.
>>> 
>>> Coincidentally there's a recent pckbd patch from Blue that resolves the conflict. Feel free to review. :)
>> 
>> Ah, sorry :). Still catching up. Just now finished to watch news of the last 1 1/2 weeks :).
> 
> Welcome back. Did they have to pay much? ;)

The only one ending up paying was me to get in for the 27c3 ;).

>>> Any thoughts on the read vs. write port issue? Do we need to register both ports in the base device and add hooks to the device state? Sounds problematic VMState-wise, just like some of the qemu_irq constructs in the ppc devices. (My prep_pci changes are still far from perfect, UniNorth has similar issues I noticed.)
>> 
>> Well, the general idea is that a single IO always gets routed to the same device. I don't fully understand why you need separate handlers there tbh. Mind to explain this?
> 
> In short: Inheritance.
> 
> The actual issue appears to be snip'ed above, it's about apparent subtle machine differences concerning non-standard register uses (here, "Motorola ... register" in -M prep vs. nothing in PReP 1.1 spec).

Hrm. So the port belongs to machine specific code then and should only call helper functions for other generic bits, no?


Alex

-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.16 (Darwin)

iEYEARECAAYFAk0jmOYACgkQq7Wi27wfN1Pp2ACdFf7aa8y/tNC7fx9Z4zFheNnU
1RQAnjGNJqAGhYNvEOKuNihLVJKsVeG9
=QdiF
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 3073870..0c9183e 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -721,8 +721,10 @@  static void ppc_prep_init (ram_addr_t ram_size,
     register_ioport_read(0x398, 2, 1, &PREP_io_read, sysctrl);
     register_ioport_write(0x398, 2, 1, &PREP_io_write, sysctrl);
     /* System control ports */
+#if 0
     register_ioport_read(0x0092, 0x01, 1, &PREP_io_800_readb, sysctrl);
     register_ioport_write(0x0092, 0x01, 1, &PREP_io_800_writeb, sysctrl);
+#endif
     register_ioport_read(0x0800, 0x52, 1, &PREP_io_800_readb, sysctrl);
     register_ioport_write(0x0800, 0x52, 1, &PREP_io_800_writeb, sysctrl);
     /* PCI intack location */