Page MenuHomeFreeBSD

arm64: Add support to vchiq and bcm2835_audio (plus some fixes)
Needs ReviewPublic

Authored by devesas.campos_gmail.com on Dec 26 2022, 8:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 8, 5:40 PM
Unknown Object (File)
Sat, Nov 8, 3:37 PM
Unknown Object (File)
Sun, Nov 2, 5:34 AM
Unknown Object (File)
Sun, Nov 2, 5:34 AM
Unknown Object (File)
Fri, Oct 31, 12:20 AM
Unknown Object (File)
Thu, Oct 30, 11:19 PM
Unknown Object (File)
Thu, Oct 30, 10:53 PM
Unknown Object (File)
Thu, Oct 30, 12:51 AM

Details

Reviewers
emaste
andrew
manu
Summary

Add 64 bit support to vchiq:

  • update fields to the appropriate fixed bit-size variants (everywhere [cf. e.g., ref:sizes and ref:sizes2])
  • refer to event semaphores (that go into the very 32 bit VC) by offset instead of pointers [ref:sems]
  • dsb() is dsb(sy) in arm64 (vchiq_{core.c,core.h,kmod.c}) [ref:dsb]
  • comment out some unneeded code in parse_rx_slots around VCHIQ_MSG_BULK_RX (cf. [ref:deadcode])
  • adapt remote_event_signal to arm64 caching behaviours (vchiq_kmod.c)
  • refactor synchronization around remote_event_signal, forcing a wmb to be on the safe side; thereby make it look more like what linux does [ref:sync] (vchiq_{core,kmod}.c); and make a comment in vchiq_core.c true (wasn't before)
  • add a few more syncs to be on the safe side (vchiq_2835_arm.c)
  • use arm64 dcache invalidation mechanisms (vchiq_2835_arm.c)
  • explicitly invalidate pages on arm64 post bulk-read (vchiq_2835_arm.c)
  • support bulk transfers on rpi-4 (aka "long address space" transfers), by hard-coding their vc offset (0) and different bit-shift [ref:longbulk] (vchiq_2835_arm.c)
  • refactor a loop-of-constant-test (vchiq_2835_arm.c)
  • use the correct (hard-coded) cache-line size on arm64
  • rework the handling of chipset "features" to account for the extra behaviours with 64 bit chipsets. (vchiq_kmod.c)
  • add sysctl-s (log, arm_log) to control debug (vchiq_kmod.c)
  • add example kernel config (GENERIC-VCHIQ)

Fixes:

  • Rework error handling in create_pagelist, avoiding a potential panic when freeing memory that had been dmamem_alloc, a potential null dereference, and a leak when having problems pinning pages (vchiq_2835_arm.c)
  • fix a confusion about the behaviour cv_wait_sig that lead to uninterruptible looping (vchiq_bsd.c)
  • implement detection of fatal signals (vchiq_bsd.c)
  • fix a confusion with the name of a variable introduced by #a0b8746 that could lead to a panic when closing the cdev file (vchiq_arm.c)
  • release user connection when destructing cdevpriv and avoid user processes sharing connection data, which lead to stalls and data corruption. (vchiq_arm.c)

Update bcm2835_audio to work on 64bit systems:

  • update VC audio fields (vc_vchi_audioserv_defs.h, bcm2835_audio.c)
  • repurpose the hitherto unused callback field to help push a 64 bit pointer in (bcm2835_audio.c)
  • increase (hopefully) the robustness of the code that shifts data to VC (bcm2835_audio.c)
  • add a sysctl to control the amount of debugging info output by bcm2835_audio.c

Tested on zero, zero2 and 4+ with ping, functional, bulk
and control vchiq_test-s, and omxplayer

[ref:dsb]: https://github.com/raspberrypi/linux/commit/35b7ebda57affcfd3616d39d5a727a4495b31123
[ref:sems]: https://github.com/raspberrypi/linux/commit/24a4262afb10907fce3cdbc3ae336fcf4cdaece5
[ref:sizes]: https://github.com/raspberrypi/linux/commit/e64568b8ea6c04e747e432c17ce2452652075216
[ref:sizes2]: https://github.com/raspberrypi/linux/commit/f9bee6dd24addfa00c2c8d50c25b73efbfbb28ba
[ref:deadcode]: https://github.com/raspberrypi/linux/commit/14f4d72fb799a9b3170a45ab80d4a3ddad541960
[ref:sync]: https://github.com/raspberrypi/linux/commit/51c071265079319583e4c6e8c61e09660300d0bf
[ref:longbulk]: https://github.com/raspberrypi/linux/commit/37f6f19a83722c9b866cecb5e455b2e16e5bbc6b

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 68222
Build 65105: arc lint + arc unit

Event Timeline

adrian edited the summary of this revision. (Show Details)
adrian added a subscriber: adrian.

Updates after refactoring work and testing it on a recent -HEAD on a RPI4.

I've pushed an update and fiddled with the diff stack a bunch. This works fine on -HEAD from this week on a RPI4 (yes I verified by playing Miley Cyrus for a good couple of hours.)

I'm happy to pull smaller pieces of this out to get it into a larger diff stack and get those pieces reviewed.

  • update now that some of the work has been moved into D53372
  • some style(9) cleanup

I've pushed an update and fiddled with the diff stack a bunch. This works fine on -HEAD from this week on a RPI4 (yes I verified by playing Miley Cyrus for a good couple of hours.)

I'm happy to pull smaller pieces of this out to get it into a larger diff stack and get those pieces reviewed.

Thanks! I originally wanted to do the same, but due to university commitments, I couldn’t. I’m very happy to see you helping tidy up the patch. I also have an RPI4, so if you need any testing on your new patch D53372, I’d be glad to help.

sys/contrib/vchiq/interface/vchiq_arm/vchiq_core.c
1101

why are the wmb()s removed here and elsewhere?

3528

and whys this barrier being removed?

sys/contrib/vchiq/interface/vchiq_arm/vchiq_core.c
1101

It's the first thing remote_event_signal does now— if I'm not mistaken all the calls to it were preceded by wmb.

This is explained in the commit note:“refactor synchronization around remote_event_signal, forcing a wmb to be on the safe side; thereby make it look more like what linux does [ref:sync] (vchiq_{core,kmod}.c); and make a comment in vchiq_core.c true (wasn't before)”