Add support for a peripheral attached to the user port #355

catharinejm · 2025-08-12T22:28:02Z

This adds support for connecting emulated peripherals to the user port via a dynamically linked library which exposes an x16_user_port_init function. read, write and step functions are also provided by the library, via a struct, and they are dispatched to by the via2_read, etc. functions. If the peripheral fails to load or initialize, an error is logged, and the X16 continues to boot without it.

Usage is mostly documented in user_pins.h, which must be included by a user peripheral lib. The shared lib is passed via a new -user-peripheral command-line flag.

I don't know if this kind of thing is wanted by the project, but there was a murmur of interest on discord, and I found it quite helpful for debugging a bitbang uart I'm running on my userport in hardware. The interface is not exactly user friendly, the implementation is by no means complete, and I've only really tested in my one case, and only on Linux... but it seems like a decent start.

This is a lot of commits for not a lot of code, it churned a lot. If you prefer I can come back with a single squashed commit. Or if you're not interested at all, I'll take no offense.

If you'd like an example of it being used, the emulated FTDI I wrote is here: https://git.sr.ht/~cjm/x16-ft232rl/tree/main/item/userport.c. It's not a very straightforward example perhaps, but it is the only one at the moment 😅

sending only PORTA pins makes it look like PORTB output pins are being pulled low, and vice versa. send all, regardless of which register was written. still no CA1/CA2/CB1/CB2 though

allowing the libraries to verify they were built against the same version as the x16emu executable

in particular, if the user perhipheral connected pins don't include CA1 or CB1, don't check them for level changes. Old logic would treat unconnected as 'low' which could, in principle, override a value set by other means. In practice, there is no code path for those values to be set elsewhere, but no sense in leaving in a future bug.

rather than in the userport library

Fulgen301

I'm a bit early with the review, so please wait for @mooinglemur 's opinion, but my two cents:

The API needs a cleanup function so that the shared object can clean itself up properly. This should be called before a reset and before the emulator exits.
Has this been tested with the WASM build? The JS wrapper has to specify which WASM plugin to load, if any.

And my main gripe:

This will result in people uploading precompiled binaries to somewhere and telling people to download it to try out new hardware in the emulator, binaries that can then do anything because they're native code like any other. (I'm not implying that hardware manufacturers are evil, just that the potential is there.) So at the minimum this needs to communicate to the user, in the README and the command line help, that only trusted plugins should be loaded.
In fact, this plugin API would actually be a good candidate for WASM plugins. This would add a runtime as a dependency for all builds except the WASM one (and I'd volunteer for that part), but we could shield plugins from doing anything nefarious without the user's consent. I am aware that a plugin communicating with real hardware will need some sort of system access, but I'd rather see that opt-in than loading a shared object that may return an error and download a Bitcoin miner in another thread.

Fulgen301 · 2025-08-12T22:57:14Z

src/dylib.h

+    #define LOAD_LIBRARY(name) LoadLibrary(name)
+    #define GET_FUNCTION(lib, name) (void *)GetProcAddress(lib, name)
+    #define CLOSE_LIBRARY(lib) FreeLibrary(lib)
+    #define LIBRARY_ERROR() "[ unknown error ]"


GetLastError + FormatMessage would be the way to get the error message; if you don't want to do the FormatMessage wrapping in this PR, I'd go with just printing GetLastError() and switching the format specifier between %u and %s depending on the platform.

I can add that but I can't test it. I just pulled these macros out of midi.c, there just wasn't one for errors.

Fulgen301 · 2025-08-12T23:05:25Z

src/user_pins.h

+	user_pin_t (*read)();
+
+	// New pin values pushed from the via to the peripheral. Pins not in the
+	// [connected_pins] mask do not contain meaningful values (but should be zeroes).


We should just guarantee that they're zero here.

As implemented that is guaranteed, I could make the comment stronger. Mostly I meant to imply that a zero in a bit not connected does not mean the pin is low, it doesn't mean anything.

Fulgen301 · 2025-08-12T23:06:56Z

src/user_pins.h

+// manually. However, unless the data direction of the pins on the via stays constant it
+// will be quite difficult to keep the via's and peripheral's states in sync, since there
+// is no signal that a via pin has switched from being actively driven to hi-Z, or vice
+// versa.


Is this relevant to implementers?

I think it's easy enough to overlook that I think it's worth calling out, especially since the emulator does actually do I2C, but it's a special case. That said, I think I could add pull-up/pull-down masks to the user_port initialization, and treat output->input changes in the ddrs as a 'write' of the pulled value, and input->output as a write of the register value.

Fulgen301 · 2025-08-12T23:07:40Z

src/via.c

+		if (user_port_init(&user_port) < 0) {
+			fprintf(stderr, "error initializing user peripheral\n");
+			error = true;


This should return a proper error message. e.g. via const char *user_port_error() that is valid until the next call.

Do you mean that the peripheral should retain its error, or the via code should retain userport errors, or both?

I've added a set_error callback to user_port_init_args_t which a peripheral can call to pass an error string back to the emulator, which retains a copy.

Fulgen301 · 2025-08-12T23:18:14Z

src/user_pins.h

+// A peripheral library's exposed [user_port_init_t] function MUST be named "x16_user_port_init".
+// 
+// Returns 0 on success and <0 on error.
+typedef int (*user_port_init_t)(user_port_t *);


You made the struct API extensible, but not the init function - and the plugin has no idea what version you want from it. I'd recommend adding another struct:

typedef struct __attribute__((aligned(void *))) user_port_init_args { int api_version; } user_port_init_args;

and modifying user_port_init to take a pointer to that struct. That way the plugin can determine whether it actually supports the given API version, and we can pass additional arguments in the future by extending the struct. (Force pointer alignment so that adding a pointer later doesn't cause headaches.)

Fulgen301 · 2025-08-12T23:21:53Z

src/user_pins.h

+// Include this header in a user peripheral library.
+//
+// A user peripheral library must export a [user_port_init_t x16_user_port_init] to be
+// consumed at runtime by the emulator during initialization of via2. [x16_user_port_init]


Suggested change

// consumed at runtime by the emulator during initialization of via2. [x16_user_port_init]

// called at runtime by the emulator during initialization of via2. [x16_user_port_init]

Fulgen301 · 2025-08-12T23:22:28Z

src/user_pins.h

+
+	// Return the values of the connected pins based on the peripheral's internal
+	// state. Any pin values not in [connected_pins] will be ignored.
+	user_pin_t (*read)();


Suggested change

user_pin_t (*read)();

user_pin_t (*read)(void);

I forgot which C standard dropped () declarations meaning "variable number of arguments", but I don't want to find out the hard way...

Fulgen301 · 2025-08-12T23:23:34Z

src/user_pins.h

+// library so that version mismatches can be detected.
+#define X16_USER_PORT_API_VERSION 1
+
+// Bit assignments for each 65c22 pin exposed to the user port, for use with


Suggested change

// Bit assignments for each 65c22 pin exposed to the user port, for use with

// Bit assignments for each 65C22 pin exposed to the user port, for use with

Fulgen301 · 2025-08-12T23:24:10Z

src/user_pins.h

+// Populates the provided [user_port_t *]. If any of [read], [write] or [step] is NULL, it
+// will be ignored.


This check isn't implemented.

It is actually, via2_read, via2_write and via2_step check that user_port.whatever != NULL before invoking it. I figure it could be valid for a device not to need some subset of the functions.

Fulgen301 · 2025-08-12T23:24:51Z

README.md

 * `-sound <device>` can be used to specify the output sound device. If 'none', no audio is generated.
 * `-abufs` can be used to specify the number of audio buffers (defaults to 8 when using the SD card, 32 when using HostFS). If you're experiencing stuttering in the audio, try increasing this number. This will result in additional audio latency though.
 * `-via2` installs the second VIA chip expansion at $9F10.
+* `-user-peripheral <my_peripheral.{so|dll}>` connects an emulated user peripheral built as a dynamically linked library. implies `-via2`


macOS uses .dylib. The extension also isn't included in main.c (and I'm not sure whether it should be).

- unconnected pin values will be zero on write - x16_user_port_init error must be -1

easier peripheral state management

catharinejm · 2025-08-17T00:25:49Z

I agree that it's a security hole, but then so is running any binary downloaded from the internet. I don't know much about WASM, I've played with it a little, but nothing too serious. I've only tested this as a regular linux shared library. A WASM plugin could be an interesting alternative, but I just don't know much about it.

called on reset and poweroff so peripherals can clean up state, close files, etc

catharinejm added 18 commits May 6, 2025 10:17

Add basic support for providing user perhipherals via shared library

7e746fb

ensure all output pins are sent to user_port on via2_write

de038af

sending only PORTA pins makes it look like PORTB output pins are being pulled low, and vice versa. send all, regardless of which register was written. still no CA1/CA2/CB1/CB2 though

clear any flag when writing to via2's IFR

e88430a

pass X16_USER_PORT_API_VERSION to user_port_init

0f5a281

allowing the libraries to verify they were built against the same version as the x16emu executable

minor readability tweaks

a2a07ff

Handle CA1 & CB1 interrupts from user pins

306fbb6

comment out via2 ifr state logging

5a5bd77

remove ifr debugging statements

1a53e35

Merge branch 'master' into userport

cd6b1a9

via2: remove special case for setting IFR, it's handled elsewhere

d0c6a02

via2 userport: handle api version mismatches in emulator

93e5de7

rather than in the userport library

userport: extract dylib macros from midi.c for userport handler

050b3e6

userport: log errors to stderr

7f5efe3

userport: rename expected init function to x16_user_port_init

2eb9604

userport: comment user_pins.h

a073871

userport: add [-user-perhipheral] flag to README

6056aac

userport: spell 'peripheral' correctly...

f015c22

Fulgen301 requested changes Aug 12, 2025

View reviewed changes

catharinejm added 8 commits August 16, 2025 15:25

userport: Add 'dylib' extension to README

e6f857d

userport: rename user_peripheral_path -> user_peripheral_plugin_path

41e42cd

userport: express tighter value guarantees in user_port.h comments

efe08b5

- unconnected pin values will be zero on write - x16_user_port_init error must be -1

userport: add extensible user port init args

f93da6b

userport: explicit empty arglist in user_port.read()

7e8be97

userport: one more user_perhipheral_plugin_path

f492c4a

userport: one more return -1 comment

ebbabaa

userport: allow plugins to set an error message on init

f5ffd64

catharinejm force-pushed the userport branch from 42c5f95 to f5ffd64 Compare August 16, 2025 20:49

userport: thread userdata through callbacks

78c2a92

easier peripheral state management

userport: add cleanup function to user peripheral

0e0b09c

called on reset and poweroff so peripherals can clean up state, close files, etc

catharinejm requested a review from Fulgen301 September 9, 2025 15:03

Fulgen301 requested a review from mooinglemur September 12, 2025 17:10

Merge branch 'master' into userport

7bb9768

	// consumed at runtime by the emulator during initialization of via2. [x16_user_port_init]
	// called at runtime by the emulator during initialization of via2. [x16_user_port_init]

	// Bit assignments for each 65c22 pin exposed to the user port, for use with
	// Bit assignments for each 65C22 pin exposed to the user port, for use with

		// Populates the provided [user_port_t *]. If any of [read], [write] or [step] is NULL, it
		// will be ignored.

Add support for a peripheral attached to the user port #355

Are you sure you want to change the base?

Add support for a peripheral attached to the user port #355

Uh oh!

Conversation

catharinejm commented Aug 12, 2025

Uh oh!

Fulgen301 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

catharinejm commented Aug 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fulgen301 left a comment •

edited

Loading