I've made this an assert. If I read the man page correctly, all of the ways that it can fail depend on passing flags in mda_flags. We pass MAKEDEV_CHECKNAME, which enables some possible failures, but they should not actually be possible (or should happen all of the time).

There was an effort to allow for oneliners, but prohibit them for multi-line comments. I'll have to go try to find what happened to it. I'd follow that here, in general, though the / clearly has a meaning to doxygen and shouldn't be eliminated.

Why not store all that junk in the softc rather than recompute it every time? seems like overkill.

same comment here. Just store these in softc, compute it once and then use the softc data members directly.

mulit-line comment.

This is out of place here. It should be centralized so people can find it in debuggers.

sc will be freed entirely after we return. We're single threaded there, so there's no reason to set this to NULL.

So if it's really 4 characters, why the indirection an obfuscation with the different const stuff?
Also, printing characters here are a bad idea. You should print the hex values since if they aren't ASCII, they are hard to dig out of the dmesg (especially if they are NULs).

blank line between decl and write_selecctor()

This needs to be conditional... It can only exist on x86 and arm64...

You can omit the rest of the boiler plate.

/// and /* are doxygen comments and, importantly, are picked up by LSP things so if you're editing in a modern editor (vim, vs code, and so on) they can pop up the docs. I generally prefer /* for multi-line comments but missed some where they stated as single-line comments and were then wrapped. Please flag any where I missed them.

I'm not sure I follow. We need to 'recompute' the index here because this is where we're setting the index.

I'm not sure what you see is being computed here. We have one offset for MMIO, one for device port things. We could store two fields, but you'll end up with basically the same number of instructions, more memory, and three things that have to be in sync when one is trivially computable from the other.

You have exactly one offset at any time. since sc->io_type is invariant after attach, you can precompute all the offsets when you set that type in attach. There's no need to check each time you go to use it.
So you'd have sc->data_offset and sc->write_offset that you'd use here and elsewhere.

You only ever set io_type here. Store the data and write offsets here too in new softc members. Then you can use them directly everywhere rather than having different ifs.

Aha, thank you! That explains why the kernel kept rejecting my error codes. I should have realised it used the Linux syscall convention. I've now made the types consistent and ensured that positive values are negated here (after doing Linux things for a bit, I have a habit of accidentally adding a - in front of any errno value I'm returning from a function, so it's nice to have those fixed automatically).

To be honest, I quite like the mount_fusefs bits, and it also avoids having to parse all of the default arguments (noatime and so on). One extra exec at mount time isn't too bad.

Does that mean open / opendir are there just to support filesystems that want to track some per-descriptor state? I've updated the code to set those flags if the defaults are not overridden (which should mean that this function is never called and exists just for type inference / documentation). This let me delete a load of code, thanks!

More code deleted, yay!

Thanks. I did that originally, but it turned out I had two bugs in the logic that produced error returns. This is now done.

Now that I've fixed the error returns, ENOSYS works fine here.

Yup, copy-and-paste error.

This is taken from share/examples/etc/bsd-style-copyright, is there a newer template I should be using?

The thing I'm not understand is why, on a code path that does a VM exit that takes (conservatively) thousands of cycles, you would prefer to shave off a single conditional move and maybe one other instruction to materialise offsets and grow a structure by two fields.

So if it's really 4 characters, why the indirection an obfuscation with the different const stuff?

I don't know what this means.

Also, printing characters here are a bad idea. You should print the hex values since if they aren't ASCII, they are hard to dig out of the dmesg (especially if they are NULs).

On a valid platform, they will be the string "QEMU". Printing hex values makes this less clear. I would imagine that anything else that looks like this device will also provide a string. This is roughly the same interface as x86 CPUID, which we print as characters.

The MMIO interface is generic. Linux enables this device on Arm PA RISC, PowerPC, SPARC, x86, and RISC-V. I think that's a superset of architectures that we support and there's no reason that it wouldn't be expected to work on others.

I keep changing my mind on this. On the one hand, the underlying device doesn’t support seeking and requires a VM exit to read 1-8 bytes (depending on the architecture), so when someone reads a 100 KiB file in a bunch of small reads, it adds a lot of overhead. On the other hand, I doubt anyone actually cares about the performance.

Then again, the total amount of data typically exposed here is < 1 MiB, so reading each file the first time it’s accessed is not adding much memory overhead and it simplifies the logic quite a bit to just read the entire file into memory once and service requests by returning a sub range into that buffer.

If I didn’t return -1, I saw infinite loops where the kernel kept issuing readdir requests at offset 0.

If I set the inode to the file’s inode, I got other failures.

If I set the offset to anything other than the offset of the next one as a concrete value, I also got failures.

I traced the NetBSD version with trace to figure out what it was doing and found the values that it used. Nothing else worked for me.

Copy and paste error. When I factored this out into a separate function, I left the comment from the copy in opendir.

It doesn’t seem to cause errors in the kernel. Possibly it should include the number of .. links, I’m not sure if anything actually cares?

Every secure coding guide I've ever read says (for good reasons) that it's a bad idea to leave dangling pointers, even if you know that they won't be read. Unless the free is in the same function, leaving a dangling pointer is just asking for someone to introduce a UAF bug in a subsequent change.

It's useless work and never or rarely done for error åpaths in the kernel. It's just as likely to lead to kernel panics as it is to uaf issues. It's very out of place. Both are bad since you also assume it is non null in many places, It's a jump ball which one you'd theoretically see,

Better to have useful data.. the error message can say expected vs actual to know what is expected. Having a hex value that happens to be QEMU is fine. There is no need to make that clear its ascii value.

For similar reasons, its a magic 4 byte value. Why not just use uint32_t and do simple compares. The strinh stuff is more error prone.

I've made it a compile-time option. It does improve performance a little bit, but not enough to be on by default.

The following line fails. I'd like to set this to the largest allowed timeout, but the kernel rejects the maximum value of this type. I guess this is an unsigned type that is interpreted as signed in the kernel somewhere? Dividing it by two works.

"simplifies the logic quite a bit" sounds like a good enough reason to me.

What do you mean by "the kernel rejects the maximum value of this type"? The fusefs test suite contains a test that sets attr_valid to UINT64_MAX, and that test works.

Infinite loops probably happened because your readdir implementation kept returning the same dirent over and over. You should be returning something unique and sorted for off, and on entry you should skip over any entries whose offset is less than or equal to the offset provided by the kernel in readIn.

To be POSIX-compliant it should include the number of .. links. But I'm not aware of anything that cares, either.