[PATCH v2 00/22] ADB: fix autopoll issues and rework mac_via state machine

Mark Cave-Ayland posted 22 patches 3 years, 10 months ago
Test FreeBSD passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200623204936.24064-1-mark.cave-ayland@ilande.co.uk
Maintainers: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, David Gibson <david@gibson.dropbear.id.au>, Laurent Vivier <laurent@vivier.eu>
hw/input/adb-kbd.c           |  42 ++--
hw/input/adb-mouse.c         |  65 ++++--
hw/input/adb.c               | 210 ++++++++++++++++--
hw/input/trace-events        |  27 ++-
hw/misc/mac_via.c            | 411 +++++++++++++++++++++++------------
hw/misc/macio/cuda.c         |  60 +++--
hw/misc/macio/pmu.c          |  47 ++--
hw/misc/trace-events         |   3 +
hw/ppc/mac_newworld.c        |   2 -
include/hw/input/adb.h       |  26 ++-
include/hw/misc/mac_via.h    |   2 +-
include/hw/misc/macio/cuda.h |   4 -
include/hw/misc/macio/pmu.h  |   4 -
13 files changed, 620 insertions(+), 283 deletions(-)
[PATCH v2 00/22] ADB: fix autopoll issues and rework mac_via state machine
Posted by Mark Cave-Ayland 3 years, 10 months ago
This patchset is something I have been chipping away at for a while since
spending some time over the Christmas holidays trying to boot the MacOS
toolbox ROM on the new q800 machine.

Initially I discovered that there were some problems when the MacOS ROM was
enumerating ADB devices due to multiple meanings of the vADBInt bit. After
fixing this there were still issues with keys being dropped during autopoll
which were eventually traced back to the autopoll timer re-firing before
the host had managed to read back the previous response.

At this point I noticed that CUDA/PMU/mac_via all had their own implementations
of ADB autopoll, and that it would make sense to consolidate the autopoll timer,
mask, interval and locking into the ADB bus. This would allow the logic to be
removed from each separate device and managed in just one place.

Finally I updated the trace-events to allow separate tracing of bus requests
and device responses which makes it easier to follow the ADB enumeration process.

The breakdown of the patchset is as follows:

- Patch 1 keeps checkpatch happy for the remainder of the patchset whilst patch
  2 is the proper fix for a spurious ADB register 3 write during enumeration
  caused by ignoring the request length which I had tried to work around earlier.

- Patches 3 to 10 are part of the autopoll consolidation process which moves the
  separate autopoll implementations into a single implementation within
  ADBBusState.

- Patches 11 to 13 update the ADB implementation to hold a status variable
  indicating the result of the last request and allow devices to indicate
  whether they have data to send. This extra information is required by the
  upcoming mac_via state machine changes.

- Patches 14 to 17 add a variable and functions to block and unblock ADB
  autopoll at bus level, adding the functions at the correct places within
  CUDA and PMU.

- Patches 18 and 19 rework the mac_via ADB state machine so that the bus
  can be enumerated correctly, and both explicit and autopoll requests work
  under both MacOS and Linux.

- Patch 20 enforces the blocking and unblocking of autopoll at the ADB
  level, including adding an assert() to prevent developers from trying to
  make an ADB request whilst autopoll is in progress.
  
- Patches 21 and 22 update the trace-events to separate out ADB device and
  ADB bus events.

The patch has been tested by myself and a couple of others during the development
process across the PPC g3beige/mac99 and 68K q800 machine so it should be quite
solid.

One thing to indicate is that the patchset bumps the VMState versions for the
affected devices but does not allow older versions to load. This is a conscious
decision given that for the mac_via device used in the q800 machine it would be
just about impossible to map this in a way that would work for all cases. Similarly
for the Mac PPC machines migration is already hit/miss due to timebase issues so
I don't see this as being a big loss.

To finish off I'd also like to say a big thank-you to both Laurent Vivier and
Finn Thain who both took time to answer my questions, dump information from a
real q800, and analyse it in very fine detail. Without them this patchset would
still be several months away.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


v2:
- Rebased onto master
- Added R-B tags from Philippe
- Fixed byte discrepency at end of bus timeout spotted by Finn
- Added Tested-by tag from Finn


Mark Cave-Ayland (22):
  adb: coding style update to fix checkpatch errors
  adb: fix adb-mouse read length and revert disable-reg3-direct-writes
    workaround
  cuda: convert ADB autopoll timer from ns to ms
  pmu: fix duplicate autopoll mask variable
  pmu: honour autopoll_rate_ms when rearming the ADB autopoll timer
  adb: introduce realize/unrealize and VMStateDescription for ADB bus
  adb: create autopoll variables directly within ADBBusState
  cuda: convert to use ADBBusState internal autopoll variables
  pmu: convert to use ADBBusState internal autopoll variables
  mac_via: convert to use ADBBusState internal autopoll variables
  adb: introduce new ADBDeviceHasData method to ADBDeviceClass
  adb: keep track of devices with pending data
  adb: add status field for holding information about the last ADB
    request
  adb: use adb_request() only for explicit requests
  adb: add autopoll_blocked variable to block autopoll
  cuda: add adb_autopoll_block() and adb_autopoll_unblock() functions
  pmu: add adb_autopoll_block() and adb_autopoll_unblock() functions
  mac_via: move VIA1 portB write logic into mos6522_q800_via1_write()
  mac_via: rework ADB state machine to be compatible with both MacOS and
    Linux
  adb: only call autopoll callbacks when autopoll is not blocked
  adb: use adb_device prefix for ADB device trace events
  adb: add ADB bus trace events

 hw/input/adb-kbd.c           |  42 ++--
 hw/input/adb-mouse.c         |  65 ++++--
 hw/input/adb.c               | 210 ++++++++++++++++--
 hw/input/trace-events        |  27 ++-
 hw/misc/mac_via.c            | 411 +++++++++++++++++++++++------------
 hw/misc/macio/cuda.c         |  60 +++--
 hw/misc/macio/pmu.c          |  47 ++--
 hw/misc/trace-events         |   3 +
 hw/ppc/mac_newworld.c        |   2 -
 include/hw/input/adb.h       |  26 ++-
 include/hw/misc/mac_via.h    |   2 +-
 include/hw/misc/macio/cuda.h |   4 -
 include/hw/misc/macio/pmu.h  |   4 -
 13 files changed, 620 insertions(+), 283 deletions(-)

-- 
2.20.1


Re: [PATCH v2 00/22] ADB: fix autopoll issues and rework mac_via state machine
Posted by Laurent Vivier 3 years, 10 months ago
Le 23/06/2020 à 22:49, Mark Cave-Ayland a écrit :
> This patchset is something I have been chipping away at for a while since
> spending some time over the Christmas holidays trying to boot the MacOS
> toolbox ROM on the new q800 machine.
> 
> Initially I discovered that there were some problems when the MacOS ROM was
> enumerating ADB devices due to multiple meanings of the vADBInt bit. After
> fixing this there were still issues with keys being dropped during autopoll
> which were eventually traced back to the autopoll timer re-firing before
> the host had managed to read back the previous response.
> 
> At this point I noticed that CUDA/PMU/mac_via all had their own implementations
> of ADB autopoll, and that it would make sense to consolidate the autopoll timer,
> mask, interval and locking into the ADB bus. This would allow the logic to be
> removed from each separate device and managed in just one place.
> 
> Finally I updated the trace-events to allow separate tracing of bus requests
> and device responses which makes it easier to follow the ADB enumeration process.
> 
> The breakdown of the patchset is as follows:
> 
> - Patch 1 keeps checkpatch happy for the remainder of the patchset whilst patch
>   2 is the proper fix for a spurious ADB register 3 write during enumeration
>   caused by ignoring the request length which I had tried to work around earlier.
> 
> - Patches 3 to 10 are part of the autopoll consolidation process which moves the
>   separate autopoll implementations into a single implementation within
>   ADBBusState.
> 
> - Patches 11 to 13 update the ADB implementation to hold a status variable
>   indicating the result of the last request and allow devices to indicate
>   whether they have data to send. This extra information is required by the
>   upcoming mac_via state machine changes.
> 
> - Patches 14 to 17 add a variable and functions to block and unblock ADB
>   autopoll at bus level, adding the functions at the correct places within
>   CUDA and PMU.
> 
> - Patches 18 and 19 rework the mac_via ADB state machine so that the bus
>   can be enumerated correctly, and both explicit and autopoll requests work
>   under both MacOS and Linux.
> 
> - Patch 20 enforces the blocking and unblocking of autopoll at the ADB
>   level, including adding an assert() to prevent developers from trying to
>   make an ADB request whilst autopoll is in progress.
>   
> - Patches 21 and 22 update the trace-events to separate out ADB device and
>   ADB bus events.
> 
> The patch has been tested by myself and a couple of others during the development
> process across the PPC g3beige/mac99 and 68K q800 machine so it should be quite
> solid.
> 
> One thing to indicate is that the patchset bumps the VMState versions for the
> affected devices but does not allow older versions to load. This is a conscious
> decision given that for the mac_via device used in the q800 machine it would be
> just about impossible to map this in a way that would work for all cases. Similarly
> for the Mac PPC machines migration is already hit/miss due to timebase issues so
> I don't see this as being a big loss.
> 
> To finish off I'd also like to say a big thank-you to both Laurent Vivier and
> Finn Thain who both took time to answer my questions, dump information from a
> real q800, and analyse it in very fine detail. Without them this patchset would
> still be several months away.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> 
> v2:
> - Rebased onto master
> - Added R-B tags from Philippe
> - Fixed byte discrepency at end of bus timeout spotted by Finn
> - Added Tested-by tag from Finn
> 
> 
> Mark Cave-Ayland (22):
>   adb: coding style update to fix checkpatch errors
>   adb: fix adb-mouse read length and revert disable-reg3-direct-writes
>     workaround
>   cuda: convert ADB autopoll timer from ns to ms
>   pmu: fix duplicate autopoll mask variable
>   pmu: honour autopoll_rate_ms when rearming the ADB autopoll timer
>   adb: introduce realize/unrealize and VMStateDescription for ADB bus
>   adb: create autopoll variables directly within ADBBusState
>   cuda: convert to use ADBBusState internal autopoll variables
>   pmu: convert to use ADBBusState internal autopoll variables
>   mac_via: convert to use ADBBusState internal autopoll variables
>   adb: introduce new ADBDeviceHasData method to ADBDeviceClass
>   adb: keep track of devices with pending data
>   adb: add status field for holding information about the last ADB
>     request
>   adb: use adb_request() only for explicit requests
>   adb: add autopoll_blocked variable to block autopoll
>   cuda: add adb_autopoll_block() and adb_autopoll_unblock() functions
>   pmu: add adb_autopoll_block() and adb_autopoll_unblock() functions
>   mac_via: move VIA1 portB write logic into mos6522_q800_via1_write()
>   mac_via: rework ADB state machine to be compatible with both MacOS and
>     Linux
>   adb: only call autopoll callbacks when autopoll is not blocked
>   adb: use adb_device prefix for ADB device trace events
>   adb: add ADB bus trace events
> 
>  hw/input/adb-kbd.c           |  42 ++--
>  hw/input/adb-mouse.c         |  65 ++++--
>  hw/input/adb.c               | 210 ++++++++++++++++--
>  hw/input/trace-events        |  27 ++-
>  hw/misc/mac_via.c            | 411 +++++++++++++++++++++++------------
>  hw/misc/macio/cuda.c         |  60 +++--
>  hw/misc/macio/pmu.c          |  47 ++--
>  hw/misc/trace-events         |   3 +
>  hw/ppc/mac_newworld.c        |   2 -
>  include/hw/input/adb.h       |  26 ++-
>  include/hw/misc/mac_via.h    |   2 +-
>  include/hw/misc/macio/cuda.h |   4 -
>  include/hw/misc/macio/pmu.h  |   4 -
>  13 files changed, 620 insertions(+), 283 deletions(-)
> 

Acked-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v2 00/22] ADB: fix autopoll issues and rework mac_via state machine
Posted by Mark Cave-Ayland 3 years, 10 months ago
On 26/06/2020 08:14, Laurent Vivier wrote:

> Le 23/06/2020 à 22:49, Mark Cave-Ayland a écrit :
>> This patchset is something I have been chipping away at for a while since
>> spending some time over the Christmas holidays trying to boot the MacOS
>> toolbox ROM on the new q800 machine.
>>
>> Initially I discovered that there were some problems when the MacOS ROM was
>> enumerating ADB devices due to multiple meanings of the vADBInt bit. After
>> fixing this there were still issues with keys being dropped during autopoll
>> which were eventually traced back to the autopoll timer re-firing before
>> the host had managed to read back the previous response.
>>
>> At this point I noticed that CUDA/PMU/mac_via all had their own implementations
>> of ADB autopoll, and that it would make sense to consolidate the autopoll timer,
>> mask, interval and locking into the ADB bus. This would allow the logic to be
>> removed from each separate device and managed in just one place.
>>
>> Finally I updated the trace-events to allow separate tracing of bus requests
>> and device responses which makes it easier to follow the ADB enumeration process.
>>
>> The breakdown of the patchset is as follows:
>>
>> - Patch 1 keeps checkpatch happy for the remainder of the patchset whilst patch
>>   2 is the proper fix for a spurious ADB register 3 write during enumeration
>>   caused by ignoring the request length which I had tried to work around earlier.
>>
>> - Patches 3 to 10 are part of the autopoll consolidation process which moves the
>>   separate autopoll implementations into a single implementation within
>>   ADBBusState.
>>
>> - Patches 11 to 13 update the ADB implementation to hold a status variable
>>   indicating the result of the last request and allow devices to indicate
>>   whether they have data to send. This extra information is required by the
>>   upcoming mac_via state machine changes.
>>
>> - Patches 14 to 17 add a variable and functions to block and unblock ADB
>>   autopoll at bus level, adding the functions at the correct places within
>>   CUDA and PMU.
>>
>> - Patches 18 and 19 rework the mac_via ADB state machine so that the bus
>>   can be enumerated correctly, and both explicit and autopoll requests work
>>   under both MacOS and Linux.
>>
>> - Patch 20 enforces the blocking and unblocking of autopoll at the ADB
>>   level, including adding an assert() to prevent developers from trying to
>>   make an ADB request whilst autopoll is in progress.
>>   
>> - Patches 21 and 22 update the trace-events to separate out ADB device and
>>   ADB bus events.
>>
>> The patch has been tested by myself and a couple of others during the development
>> process across the PPC g3beige/mac99 and 68K q800 machine so it should be quite
>> solid.
>>
>> One thing to indicate is that the patchset bumps the VMState versions for the
>> affected devices but does not allow older versions to load. This is a conscious
>> decision given that for the mac_via device used in the q800 machine it would be
>> just about impossible to map this in a way that would work for all cases. Similarly
>> for the Mac PPC machines migration is already hit/miss due to timebase issues so
>> I don't see this as being a big loss.
>>
>> To finish off I'd also like to say a big thank-you to both Laurent Vivier and
>> Finn Thain who both took time to answer my questions, dump information from a
>> real q800, and analyse it in very fine detail. Without them this patchset would
>> still be several months away.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>
>>
>> v2:
>> - Rebased onto master
>> - Added R-B tags from Philippe
>> - Fixed byte discrepency at end of bus timeout spotted by Finn
>> - Added Tested-by tag from Finn
>>
>>
>> Mark Cave-Ayland (22):
>>   adb: coding style update to fix checkpatch errors
>>   adb: fix adb-mouse read length and revert disable-reg3-direct-writes
>>     workaround
>>   cuda: convert ADB autopoll timer from ns to ms
>>   pmu: fix duplicate autopoll mask variable
>>   pmu: honour autopoll_rate_ms when rearming the ADB autopoll timer
>>   adb: introduce realize/unrealize and VMStateDescription for ADB bus
>>   adb: create autopoll variables directly within ADBBusState
>>   cuda: convert to use ADBBusState internal autopoll variables
>>   pmu: convert to use ADBBusState internal autopoll variables
>>   mac_via: convert to use ADBBusState internal autopoll variables
>>   adb: introduce new ADBDeviceHasData method to ADBDeviceClass
>>   adb: keep track of devices with pending data
>>   adb: add status field for holding information about the last ADB
>>     request
>>   adb: use adb_request() only for explicit requests
>>   adb: add autopoll_blocked variable to block autopoll
>>   cuda: add adb_autopoll_block() and adb_autopoll_unblock() functions
>>   pmu: add adb_autopoll_block() and adb_autopoll_unblock() functions
>>   mac_via: move VIA1 portB write logic into mos6522_q800_via1_write()
>>   mac_via: rework ADB state machine to be compatible with both MacOS and
>>     Linux
>>   adb: only call autopoll callbacks when autopoll is not blocked
>>   adb: use adb_device prefix for ADB device trace events
>>   adb: add ADB bus trace events
>>
>>  hw/input/adb-kbd.c           |  42 ++--
>>  hw/input/adb-mouse.c         |  65 ++++--
>>  hw/input/adb.c               | 210 ++++++++++++++++--
>>  hw/input/trace-events        |  27 ++-
>>  hw/misc/mac_via.c            | 411 +++++++++++++++++++++++------------
>>  hw/misc/macio/cuda.c         |  60 +++--
>>  hw/misc/macio/pmu.c          |  47 ++--
>>  hw/misc/trace-events         |   3 +
>>  hw/ppc/mac_newworld.c        |   2 -
>>  include/hw/input/adb.h       |  26 ++-
>>  include/hw/misc/mac_via.h    |   2 +-
>>  include/hw/misc/macio/cuda.h |   4 -
>>  include/hw/misc/macio/pmu.h  |   4 -
>>  13 files changed, 620 insertions(+), 283 deletions(-)
>>
> 
> Acked-by: Laurent Vivier <laurent@vivier.eu>

Thanks Laurent! I know David is busy at the moment, but he's fine with the Mac PPC
changes so I'll add your A-B tag and send a PR shortly.


ATB,

Mark.