uACPI - a portable and easy-to-integrate ACPI implementation

[fiery turtle]

Np

[torpid root]

unfortunately this passes

assert(!memcmp(reinterpret_cast<uint8_t *>(vaddr) + (addr - paddr), reinterpret_cast<uint8_t *>(tohh(addr)), len));

[fiery turtle]

@torpid root can you try instead disabling the huge page optimization and always map as small?

[torpid root]

sure

same result :(

if I identity map instead of allocating a new virtual address then it works

these are all for dereferencing addresses returned by uacpi_kernel_map whose return value has the same alignment as the physical address parameter passed to it

I don't think it matters too much though

[fiery turtle]

yeah i have no idea

id happy if u figure it out tho

[fiery turtle]

this is strange because the structs are marked as packed

oh acpi_sdt_hdr is not

thats a bug, ill mark it as packed

but thats an unrelated thing

[fiery turtle]

i dont understand why that would matter tho

[torpid root]

me neither

[torpid root]

adding uacpi_packed to it removes the ubsan warnings

[fiery turtle]

makes sense

[fiery turtle]

fixed in upstream

[torpid root]

has uacpi been tested on virtualbox?

[fiery turtle]

not by me

ive tested vmware and qemu

but we have like ~25 real computers tested already

[torpid root]

at least on my kernel, uacpi seems to get stuck somewhere in do_sta_ini if booted with ich9 chipset
it could also be virtualbox being virtualbox as the line where it gets stuck changes on almost every run, but it's mostly uacpi.c:431. it also freezes if the kernel draws too fast on the framebuffer lol

[fiery turtle]

when in doubt try managarm

i dont think virtual box aml is more difficult or quirky than a real computer lol

[fiery turtle]

    is_sb = node == uacpi_namespace_get_predefined(
        UACPI_PREDEFINED_NAMESPACE_SB
    );

This can't freeze lol

it returns a constant pointer from a static table

[torpid root]

yes I don't understand why it doesn't work

[mortal yoke]

it does use indexfields so its different from what you get in eg. qemu but yeah I also doubt that

[torpid root]

i put two printfs around it
it works first few times and then it gets stuck on the first one

[fiery turtle]

memory corruption?

u never figured out the root cause for that mapping thing

[torpid root]

yes but I have uacpi_kernel_map replaced with return reinterpret_cast<uint8_t *>(tohh(addr)) so it should be working

in theory

[fiery turtle]

unless its some other memory that gets corrupted now

[torpid root]

it works on qemu and vmware

[fiery turtle]

that doesnt mean much

but give mangarm iso a try

[torpid root]

[torpid root]

ok

[fiery turtle]

make sure its not the one with LAI tho

[torpid root]

uacpi is merged on master right?

[fiery turtle]

yea

[torpid root]

oof 900mb xz

[fiery turtle]

don't forget to log into some ssh server so that we can steal your data via xz backdoor

[torpid root]

lmao

[fiery turtle]

if u just clcked on the link in the readme it might be some insanely old image

https://builds.managarm.org/ this should have newer stuff

[torpid root]

it says nightly

[fiery turtle]

well no idea maybe its new

[torpid root]

It's stuck on Waiting for /dev/sda0

[fiery turtle]

yeah that means it booted succesfully

try shutting down

via power button

[torpid root]

that works

[fiery turtle]

did u see the logs?

try redirecting serial to a file

[hasty fossil]

sorry if i'm interrupting, but will uACPI support being compiled as a static library out of the box in the future? it'd help out a ton for kernels in languages other than C/C++

[fiery turtle]

anyway if its an image with uacpi then it booted succesfully and already past that init stage where it stuck for u @torpid root

but look at logs to be sure

[fiery turtle]

it already supports that?

it gives u a list of sources and you're free to do anything with them

[torpid root]

so it's my kernel's fault

[fiery turtle]

well if there's a word "uacpi" in logs then yes

[torpid root]

doesn't managarm log to serial?

[hasty fossil]

but, i mean, if one can just compile it out-of-the-box like LAI, without any modifications, so that a submodule can be used that points to upstream instead of a fork

[fiery turtle]

it does yea

[fiery turtle]

right

[torpid root]

well nothing is sent there

[fiery turtle]

did u select the right grub entry?

or limine

[torpid root]

weston, e9 output, plainfb

[fiery turtle]

what prevents you from compiling it out-of-the-box right now?

[fiery turtle]

hmm @hollow elm how to log to serial in managarm

[torpid root]

ah there is serial output in physical hardware submenu

[hollow elm]

yeah in the bootloader theres a boot option for serial

which just changes the cmdline arg from bochs to serial

[hasty fossil]

i'm having a bit of trouble building it at all at the moment

i did meson ../uACPI-master then ninja

ninja reports ninja: no work to do.

[fiery turtle]

well thats not how u build it

meson & cmake files just export the sources

its up to u to attach them to a library or an executable

[torpid root]

works fine, no uacpi errors or warnigns

[jaunty fox]

thanks for doing that instead of trying to build it in the same

that means i won't have to fork the repo to modify them to only do that

[fiery turtle]

i cant dictate how to build them lol, its up to your environment

ikr

[jaunty fox]

this was my big problem with the LAI

[fiery turtle]

yeah unfortunately your kernel skill issue

[jaunty fox]

its mesonfile tries to build it

[fiery turtle]

like i cant know every compiler or architecture you will build this for

its not my responsibility

[jaunty fox]

maybe i will get around to the uacpi integration in keyronex this week

i have to redo the kernel wired memory junk first

[fiery turtle]

nice

[jaunty fox]

one thing i am looking forward to

i implemented the windows storport driver framework

and i have some drivers which want to call acpi methods

[fiery turtle]

oh thats cool

[jaunty fox]

should be fun to hook that up to uacpi

[fiery turtle]

i have no idea what that even is

[hasty fossil]

im a dum dum, i just compiled it all w/ gcc and ar'd it into a static library and it seems to work lol

is there a list of symbols that uACPI expects to be exported by the kernel?

[fiery turtle]

yeah that should work

[fiery turtle]

include/kernel_api.h

but read the readme

[hasty fossil]

thanks!

[jaunty fox]

it's a driver framework for modern HBAs, they derived StorPort from the older ScsiPort and made it more scalable, and encourage it for all modern HBAs not just scsi ones

[fiery turtle]

interesting

[jaunty fox]

i decided to adopt it because it wasn't too much effort to implement and it's fun

i haven't tested it on fancy proprietary HBAs yet only on the official NVMe, AHCI drivers and on the virtio-disk/virtio-scsi drivers from red hate

[fiery turtle]

red hate lol

[torpid root]

this is with piix3 chipset btw

[fiery turtle]

looks like cpu bringup code dies

not the first time

[wind fiber]

Hmm

[wind fiber]

@hollow elm @flat badge I believe this is in VMWare?

[fiery turtle]

this is vbox

[flat badge]

hm?

[fiery turtle]

i think qookie had a similar problem or someone else

[flat badge]

did somebody use gdb to see where it faults?

[hollow elm]

which image did you use?

the one linked in the readme?

[torpid root]

yep

[hollow elm]

wtf what kind of superpowered strip is meson using

the kernel file has no symbol table at all

i'm surprised it even has section headers

[hollow elm]

this is vbox right?

[torpid root]

yes

[flat badge]

thor should not be striped, should it?

we compile in debugoptimized mode

[hollow elm]

the unstripped version is still in pkg-builds/managarm-kernel/thor/thor

but in the sysroot there's a stripped binary

[flat badge]

we only strip it when copying it to the initrd/boot dir

[hollow elm]

i changed it iirc

i think the reason was that update-image cannot strip it on it's own because it's a containerless tool

so it can't use our target strip

and as such may not have an appropriate strip available (e.g. when compiling aarch64 managarm)

[flat badge]

we have prefix=/usr/managarm and there's install: true in thor's meson.build

so unstriped thor should be at /usr/managarm/bin/thor (?)

[mortal yoke]

the version there is also stripped

because meson is passed -Dstrip=true in bootstrap.yml

[hollow elm]

we configure managarm-kernel with -Dstrip=true

yeah

so ninja install strips executables on it's own

[mortal yoke]

is there really a reason why it needs to be stripped?

[hollow elm]

minimizing the space taken up in the esp

[mortal yoke]

its 17M with debug info but that doesn't sound too bad to me

considering the esp is 256M

[flat badge]

oh

I didn't see -Dstrip=true

we could add a custom_target to provide the striped binary

then we could pass -Dstrip=false

btw if you check out the right commit that xbbs used + use the same compiler, you should get exactly the same thor binary

if you compile it locally in cbuildrt

i tested that it's reproducible

[hollow elm]

oh it's dying in thor::pollSleepNano(unsigned long) at hpet.cpp:207

iirc you need to enable the hpet with a vboxcmd command :^)

oh it even says No HPET table!

@torpid root for piix3 you need to vboxmanage modifyvm Managarm --hpet=on

maybe we should add support for calibrating the timer with the pit

[fiery turtle]

or the acpi timer

[hollow elm]

or that yeah

we could probably support all 3

anyway after enabling the hpet and switching from ide to ahci, it boots fine

now i can uninstall vbox :^)

[fiery turtle]

lol

[hollow elm]

also vbox is as slow as i remember :^)

every ui action has a small but noticeable delay

[flat badge]

that still sounds like a bug

[mortal yoke]

virtualbox doesn't actually provide a hpet table by default

[flat badge]

we should not try to use the hpet if it's not present

we should panic with a more reasonable message instead of triggering a fault

[hollow elm]

i mean yeah

[fiery turtle]

finally implemented bounds checking for opregions

[hollow elm]

nice

[fiery turtle]

What's interesting is fuzzer generated some 0 length opregions

i should probably add special handling for these somehow

to not call e.g. kernel_map for them

Okay well all qookie crashes no longer crash

i should rework is_physical_address for the test runner now

also some overflow checks at opregion creation time

[hollow elm]

lol

[north holly]

@fiery turtle Not an immediate problem, but:

/mnt/c/Code/obos/dependencies/uACPI/source/interpreter.c:2826:9: warning: 'arg1_buf.ptr' may be used uninitialized [-Wmaybe-uninitialized]
 2826 |         uacpi_memcpy(dst_buf + arg0_buf->size, arg1_buf.ptr, arg1_buf.len);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/mnt/c/Code/obos/dependencies/uACPI/source/interpreter.c:2816:41: note: 'arg1_buf.ptr' was declared here
 2816 |         struct object_storage_as_buffer arg1_buf;
      |                                         ^~~~~~~~
/mnt/c/Code/obos/dependencies/uACPI/source/interpreter.c:2819:18: warning: 'arg1_buf.len' may be used uninitialized [-Wmaybe-uninitialized]
 2819 |         buf_size = arg0_buf->size + arg1_buf.len;
      |         ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/mnt/c/Code/obos/dependencies/uACPI/source/interpreter.c:2816:41: note: 'arg1_buf.len' was declared here
 2816 |         struct object_storage_as_buffer arg1_buf;
      |                                         ^~~~~~~~

and that

[fiery turtle]

I think a lot of those are BS but ill take a look

[north holly]

also 4500 line static analyzer output from gcc if you care enough to check

[lapis hawk]

bruh

[fiery turtle]

bruh

yeah ill take a look, some of those are probably valid

[fiery turtle]

this is a false positive

the arguments to Concatenate are parsed asUACPI_PARSE_OP_COMPUTATIONAL_DATA

idk how to tell gcc that tho

[north holly]

there is probably some attribute for variables that tells gcc that it is initialized

or to not warn if it's used uninitialized

[fiery turtle]

static uacpi_status typecheck_computational_data(
    const struct op_context *op_ctx,
    const uacpi_object *obj
)
{
    switch (obj->type) {
    case UACPI_OBJECT_STRING:
    case UACPI_OBJECT_BUFFER:
    case UACPI_OBJECT_INTEGER:
        return UACPI_STATUS_OK;
    default:
        EXEC_OP_WARN_2(
            "invalid argument type: %s, expected a %s",
            uacpi_object_type_to_string(obj->type),
            SPEC_COMPUTATIONAL_DATA
        );
        return UACPI_STATUS_AML_INCOMPATIBLE_OBJECT_TYPE;
    }
}

its typechecked specifically before that

[fiery turtle]

hmm

[fiery turtle]

i decided to bite the bullet and add __attribute__((format)) to uACPI log callbacks

soo many warnings

some are actual sort-of bugs so its good i guess

[rain nimbus]

Isn't there a C23 and C++ attribute for it? Through gnu::format and msvc::format something

[fiery turtle]

idk, im not gonna enforce C23

[rain nimbus]

Would make it more portable without the need for macros, but oh well

[fiery turtle]

what

the macros is not mandatory

its defined to nothing on msvc

[rain nimbus]

I mean a macro is still a macro...

[fiery turtle]

and..?

[jaunty fox]

what's wrong with a macro

C is full of them

and even your name is only one letter away from being "Macro"

[fiery turtle]

lmao

[rain nimbus]

😳

But with the attributes you don't need macros at all

Altho compilers warn about unknown and unused attributes...

[fiery turtle]

soo hardcoding attributes is more portable than an optional macro??

[jaunty fox]

i think it's alright to have macros like UACPI_PRINTF_FORMAT(3, 2), they are a little annoying when you have loads of permutations of them, but nothing compared to the olden days when every project had its very own random and deficient reimplementation of <stdint.h> with arbitrary names

[rain nimbus]

Oof

[fiery turtle]

close

[rain nimbus]

Portable in the sense of always being in the same place, some places have the problem of __attribute__ and __declspec being in different places for the same thing

[jaunty fox]

thanks, still working on cultivating my telepathy

[fiery turtle]

hmmm im not sure what that means

[rain nimbus]

The [[]] always appear in the same place regardless of compiler since it's a language feature, meanwhile __attribute__ and __declspec could be wherever the compiler decided

[fiery turtle]

[jaunty fox]

i assume the way some compilers want the attributes in different positions and with different behaviours so you'd have to e.g. for struct packing perhaps have

PACKED_PUSH_MACRO()
struct PACKED_STRUCT { whatever };
PACKED_POP_MACRO()

[fiery turtle]

just use the LCD of all compilers, which is pragma

[rain nimbus]

Probably before the ; ...

[rain nimbus]

#pragma dllexport
Where this at

[north holly]

Solution: Only support compiling with one compiler

[rain nimbus]

[fiery turtle]

    #define UACPI_PACKED(decl)  \
        __pragma(pack(push, 1)) \
        decl;                   \
        __pragma(pack(pop))

[rain nimbus]

Dafuq

[fiery turtle]

this works on all compilers

[north holly]

GCC and Clang basically support every architecture and platform you can think of

[fiery turtle]

clang, apple clang, msvc, gcc

[jaunty fox]

how about Open Watcom

[rain nimbus]

Clang is bae :>

[fiery turtle]

touch uacpi_compiler.h + UACPI_OVERRIDE_COMPILER=1

then define your own helpers

[jaunty fox]

thanks, i will keep it in mind if i ever do something i am occasionally tempted to do, which would be some kind of ridiculous kernel (something like windows 9x, or netware, or some other funny style) for x86 built with watcom

[fiery turtle]

yeah np, I made all platform-specific headers overridable for this reason

[vast kestrel]

Isn't declspec placed at the same position of an attribute that can define visibility

[rain nimbus]

For funcs yes

[left orbit]

i just tried embedding uacpi in my zig kernel and found out that some stdlib functions are not implemented, is that intentional? should i provide them for uacpi? or is this something to be fixed upstream? 😅

* building package: kernel
install
└─ install kernel
   └─ zig build-exe kernel Debug x86_64-freestanding-none 5 errors
error: ld.lld: undefined symbol: strnlen
    note: referenced by interpreter.c:726 (/base_dir/kernel/./uacpi/source/interpreter.c:726)
    note:               /build_dir/builds/kernel/o/644c5082b06446141c05097fed656836/interpreter.o:(handle_string)
    note: referenced by interpreter.c:2728 (/base_dir/kernel/./uacpi/source/interpreter.c:2728)
    note:               /build_dir/builds/kernel/o/644c5082b06446141c05097fed656836/interpreter.o:(handle_to_string)
error: ld.lld: undefined symbol: strncmp
    note: referenced by interpreter.c:5712 (/base_dir/kernel/./uacpi/source/interpreter.c:5712)
    note:               /build_dir/builds/kernel/o/644c5082b06446141c05097fed656836/interpreter.o:(uacpi_osi)
error: ld.lld: undefined symbol: snprintf
    note: referenced by interpreter.c:2849 (/base_dir/kernel/./uacpi/source/interpreter.c:2849)
    note:               /build_dir/builds/kernel/o/644c5082b06446141c05097fed656836/interpreter.o:(handle_concatenate)
    note: referenced by interpreter.c:2605 (/base_dir/kernel/./uacpi/source/interpreter.c:2605)
    note:               /build_dir/builds/kernel/o/644c5082b06446141c05097fed656836/interpreter.o:(buffer_to_string)
    note: referenced by interpreter.c:2541 (/base_dir/kernel/./uacpi/source/interpreter.c:2541)
    note:               /build_dir/builds/kernel/o/644c5082b06446141c05097fed656836/interpreter.o:(integer_to_string)
error: ld.lld: undefined symbol: strlen
    note: referenced by namespace.c:337 (/base_dir/kernel/./uacpi/source/namespace.c:337)
    note:               /build_dir/builds/kernel/o/9f77beff4ec03d5634ff53d6356406be/namespace.o:(uacpi_namespace_node_do_find)
error: ld.lld: undefined symbol: strcmp
    note: referenced by utilities.c:575 (/base_dir/kernel/./uacpi/source/utilities.c:575)
    note:               /build_dir/builds/kernel/o/c1e892bbce2b4070b34e81d8fcf39b49/utilities.o:(matches_any)```

i think it would be very trivial and nice to implement them in stdlib.c just so that non-C/C++ kernels don't have to provide these, but that's not my call obviously

[fiery turtle]

Well if only you looked at the readme

These are implemented by your kernel

[left orbit]

well the only thing i did find is "stdlib.h - exports a minimal subset of libc helpers that uACPI utilizes. This should only be overriden if your kernel's standard library is fundumentally different from libc.", but it wasn't immediately clear to me that i should provide implementations for these

it makes sense though, no big deal

[fiery turtle]

Well fair enough, ill make it more clearer

[left orbit]

ugh it wants snprintf

[fiery turtle]

Yea

[left orbit]

export fn snprintf(buf: *u8, buf_size: usize, fmt: *const u8, ...) callconv(.C) usize {
    _ = buf;
    _ = buf_size;
    _ = fmt;
    unreachable;
}```

that'll do for now

[torpid root]

https://github.com/eyalroz/printf

[left orbit]

yeah yeah

i know

it's just a bit of pain to embed another C library

[fiery turtle]

Well its used internally for some conversion opcodes sooo

Besides logging

Might break

They aren't used by hardware often tho

[left orbit]

yeah well, i don't exactly expect it to work given all of the kernel api functions are just unreachable :^)

i'm just trying to get it to build atm

[fiery turtle]

I see

Zig bindings are welcome as well

[left orbit]

yay it tried to call uacpi_kernel_map

that means it all linked properly

[fiery turtle]

Nice

[left orbit]

given that zig has @cImport and @cInclude it's really as simple as zig const C = @cImport({ @cInclude("uacpi/uacpi.h"); });

:^)

[fiery turtle]

I see

[torpid root]

When you implement that function and it works, can you post it here? I'm interested in something

[left orbit]

sure, but i'm expecting it to be as simple as return virt.asHigherHalfUncached(addr); :p

[fiery turtle]

on x86 yea

[left orbit]

unless returning hhdm pointers is not a very good practice

[fiery turtle]

nah its ok

[left orbit]

epic

i'll return to work in a bit tho as i gotta go to the store lol

hopefully integrating uacpi is as smooth as it seems so far

[fiery turtle]

snprintf is the biggest annoyance

For non c kernels

[left orbit]

true, but i think i'll just slam eyalroz printf in

[fiery turtle]

oberrow just took the stb header

[left orbit]

it should be as simple as adding a submodule and adding the C files to the build process

and adding the right compile flags so it doesn't try using floating point or whatever

i remember there's some configuration to do

i think lyre uses eyalroz printf so i'll look at that

[fiery turtle]

ok nice

[torpid root]

I meant if you were going to map it manually (I have been experiencing some problems with it), but thanks

[left orbit]

does uacpi_kernel_alloc expect any specific alignment?

[left orbit]

hitting a ud1 inside uacpi_install_address_space_handler... fun!

opregion.c:432

my bad, that's just a return address

opregion.c:145 according to addr2line

[fiery turtle]

suitable for any base type

aka at least 8

[verbal herald]

well my approach is to have two zig bindings, one is a direct @cImport while the other is a more zig-friendly bindings

[rain nimbus]

Would've assumed 16 bytes, since that's what basically every allocator uses

[fiery turtle]

doesnt really matter

[fiery turtle]

Starting to implement support for global lock finally

...and new kernel api has to be added aka uacpi_kernel_get_thread_id

[rain nimbus]

What kind of lock is it?

[fiery turtle]

firmware vs kernel

for accessing shared resources\

https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#global-lock

[rain nimbus]

Ah ok, but wouldn't it only need to be a boolean based lock?

Oh it is xD

[fiery turtle]

basically it is a boolean lock

the only problem is the kernel can acquire it from multiple threads

[rain nimbus]

Then why do you need the thread id?

Or is that for something else?

[fiery turtle]

and AML can acquire it from multiple threads

this mechanism is tied with a \_GL object inside aml

so basically its a low level lock + high level mutex combined

i need thread id to support nested acquires

[rain nimbus]

Ah right recursive mutex

[fiery turtle]

ye

i used to not need it because only AML could acquire mutexes

but this mutex is shared with the kernel

[rain nimbus]

Hmm

[fiery turtle]

For aml I would just use execution context address as thread id

[rain nimbus]

So you're going to use a 64 bit mutex then, to store the thread id and the base global lock structure they have in the spec?

[rain nimbus]

O_O

[fiery turtle]

Basically when acquiring this mutex I will check if its the global lock and if it is I will also acquire that lock structure

[rain nimbus]

So basically a double mutex

[fiery turtle]

Ye

The second part is for the firmware

The first part is for the kernel

[rain nimbus]

Smart I guess

Since it seems like AML requires that specific GlobalLock structure

[fiery turtle]

Well im just stealing the acpica ideas

[rain nimbus]

oof

[winter orbit]

Oh man this project looks super cool,
Let's see about integrating it into ReactOS for fun..

[north holly]

It indeed is

My kernel and previous kernel used it

Managarm uses it

uhhhh

Yeah

[winter orbit]

👀 well lets see what happens

[fiery turtle]

Well thank you

I think reactos uses acpica

[winter orbit]

we do, but i am thinking of switching it over

I recently redesigned our HALs, PCI, ACPI, etc to conform to what windows 2003->win8.x do

so could be fun to try :)

[fiery turtle]

Cool stuff

[winter orbit]

ill let you know how it goes

[fiery turtle]

Well uACPI is definitely closer to reactos spirit because nt conformance

[winter orbit]

the result will likely be an acpi.sys that uses uACPI that runs on windows XP

[fiery turtle]

Cool cool

[winter orbit]

yeah we've talked about your project quite a bit now :)

[fiery turtle]

Nice!

[deft canopy]

has there actually been any work on this? if so is there any repository I can look at

[winter orbit]

im actively working on it atm

ill post when it's slightly functional :)

[deft canopy]

alrighty! I can't wait to have a look, been meaning to work on my plan to port various systems to Itanium, maybe that'll be how I eventually test out uACPI on Itanium ;P

[sterile egret]

Gonna be a race between the two of us for that last part 😛

[deft canopy]

oh its on >:P...
as soon as I can get my unit to actually boot

https://tenor.com/view/elmo-hello-elmo-rise-elmo-hell-gif-9112913

[jaunty fox]

i will see about transitioning keyronex to uACPI today

[fiery turtle]

Very well

[jaunty fox]

there isn't much to deal with

iterating over the device tree, pci intx routing, that sort of thing

[fiery turtle]

Yeah should be easy

[jaunty fox]

../../../../platform/amd64/meson.build:1:0: ERROR: Source item is <IncludeDirs subprojects/uacpi/['include']> instead of string or File-type object
fuck off meson

[fiery turtle]

Lol

[jaunty fox]

oh how handy

you have a function for doing depth first iteration of the tree

[jaunty fox]

this looks frightful, investigating now

oh i see, you want true from the acquire mutex glue if you acquired the mutex

[fiery turtle]

Yea

[fiery turtle]

Indeed

There's also find_devices and stuff

[jaunty fox]

it's done

total transition

not that there was much needing done in any case

i will try to find a storport driver that uses StorPortInvokeAcpiMethod function and shim that, that ought to be fun

[robust sable]

How is it going with the new memory manager and dispatcher in Keyronex?

[jaunty fox]

i'm in the process of integrating the former, the latter i hope to do soon after - both i prototyped and tested as user programs

i am fairly happy with both, they both meet their intended purpose (giving me working set based paging, and giving me more finely locked dispatching, respectively)

it was especially nice to copy what Kishan says in his video about sorting dispatcher objects by pointer address to establish a lock ordering for these siblings so that WaitAll can be done, while the recent proposed patch to add NT-like dispatcher objects to Linux (for WINE compatibility) just used a blobal WaitAll lock

[robust sable]

It's pretty funny that his 15-year-old video still lives on

[fiery turtle]

That was quick

What are your thoughts on the api and stuff? Did it work okay for you?

Also do you actually map stuff or just use higher half mappings? @torpid root had some problems with that

Also you can now try reducing stack size

[jaunty fox]

very happy with it, no friction to report, i still object to unsized frees though

i just give it pointers into the direct map at present, but that will change prior to running on any real machine

[fiery turtle]

I see

[fiery turtle]

Interesting, yeah sized frees is a direction im considering

[fiery turtle]

Decided to finally figure out the 32-bit build (it was previously untested entirely)

Looks like the linux 32 build passed all tests first try, including ACPICA arithmetic test suite

❯ file ./runner/build-linux-32/test-runner
./runner/build-linux-32/test-runner: ELF 32-bit LSB pie executable, Intel 80386, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux.so.2, BuildID[sha1]=b2e4d83e0c8cd37eef557867290b4c4a12e8bf15, for GNU/Linux 3.2.0, with debug_info, not stripped

❯ ./runner/build-linux-32/test-runner /mnt/d/acpica/tests/aslts/tmp/aml/20230628/aslplus/arithmetic.aml int 0
...
[uACPI][TRACE] [AML DEBUG] String => "Run time (in seconds): 0x0"
[uACPI][TRACE] [AML DEBUG] String => "The total number of exceptions handled: 0x0"
[uACPI][TRACE] [AML DEBUG] String => "========= ROOT METHODS SUMMARY (max 600):"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:ADD0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:SUB0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:MTP0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:DVD0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:ICR0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:DCR0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:AND0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:NAN0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:NOR0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:NOT0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:OR00:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:XOR0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:MOD0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:SHL0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:SHR0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:FSL0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => ":STST:functional:arithmetic:FSR0:PASS:"
[uACPI][TRACE] [AML DEBUG] String => "========= END."
[uACPI][TRACE] [AML DEBUG] String => "TEST ACPICA: 64-bit : PASS"

On windows I had to do this

#ifdef _WIN64
    ret = _BitScanReverse64(&index, value);
    if (ret == 0)
        return 0;

    return (uacpi_u8)index + 1;
#else
    ret = _BitScanReverse(&index, value >> 32);
    if (ret == 0) {
        ret = _BitScanReverse(&index, value);
        if (ret == 0)
            return 0;

        return (uacpi_u8)index + 1;
    }

    return (uacpi_u8)index + 33;
#endif

but seems to work as well

I'll add proper CI for 32 bit builds

[left orbit]

zig allocator api does not actually offer a way for unsized frees... so i decided to leak all allocations lol

[fiery turtle]

damn lol

[left orbit]

though i have not really tested uacpi in my kernel yet, i'm hitting the same stupid ud1 as i was a few days ago

i'll try looking into it in a bit, it seems pretty strange

[fiery turtle]

❯ git grep uacpi_kernel_free | wc -l
73

quite a lot of stuff to refactor for this

[mortal yoke]

I mean couldn't you still just put the size at the start of the allocation and allocate that much more? not ideal but it should work

[fiery turtle]

yup, that seems easy to do

[left orbit]

that is not a bad idea at all, thanks!

i'll do it once it is all up and running

[fiery turtle]

probably some kernel api implemented incorrectly

[left orbit]

right now my kernel api very closely resembles the test runner's one

[hasty plinth]

That's been my hack while playing around with uacpi

[fiery turtle]

you use zig as well?

[hasty plinth]

Nah c++

[fiery turtle]

test runner works pretty well tbh

ive tested it against hundreds of real DSDTs

[hasty plinth]

But I like sized frees

[fiery turtle]

I see

[left orbit]

[  0] (phys) warn: uacpi_kernel_map is a stub, mapping f52c0 with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe22fc with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe22fc with len 38
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe20f4 with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe20f4 with len f4
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe0040 with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe0040 with len 20b4
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe0000 with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe0000 with len 40
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe21e8 with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe21e8 with len 78
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe2260 with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe2260 with len 38
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe2298 with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe2298 with len 3c
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe22d4 with len 24
[  0] (phys) warn: uacpi_kernel_map is a stub, mapping 7ffe22d4 with len 28
[  0] (interrupts) err: An exception #6 occurred```

that is what currently happens, ill try just providing a hhdm pointer instead of an uncached one

[fiery turtle]

wait, is a stub in what way

[left orbit]

provides a higher half pointer

[fiery turtle]

Okay well since so many people are interested in sized frees ill see what i can do

[left orbit]

not that much of a stub but i'd like to do it properly

[fiery turtle]

i see

[left orbit]

the stack trace inside uacpi is:
uacpi_initialize (uacpi.c:302)
uacpi_install_default_address_space_handlers (default_handlers.c:382)
uacpi_install_address_space_handler (opregion.c:432)
with the crash occuring in find_handler (opregion.c:145)

[fiery turtle]

yeah this doesnt help much unfortunately

the bug is likely kernel api somewhere

[left orbit]

ill try to put some debug logs in the kernel api

already running with uacpi trace logging

[fiery turtle]

is your calloc correct?

[left orbit]

export fn uacpi_kernel_calloc(count: C.uacpi_size, size: C.uacpi_size) callconv(.C) *anyopaque {
    return uacpi_kernel_alloc(count * size);
}```

:D

[fiery turtle]

thats not how that works..

[left orbit]

damn

i thought it just allocated an array of count elemnts of size size

[fiery turtle]

calloc means the memory is zeroed

[left orbit]

oh im pretty sure the zig allocator zeroes the memory

[fiery turtle]

lol

[left orbit]

but i will zero it out

oh wow that was it

[ 0] (phys) debug: acpi mode already enabled

ah right everything is a stub

[fiery turtle]

i guess it does not

[left orbit]

yeah i thought it did, that's my fault

[left orbit]

after implementing the raw_memory and raw_io functions i was still getting the same message, and it seems that it was due to me passing smm=off on qemu command line

now im getting some port read and writes and uacpi enters acpi mode

time to rip out all the old acpi code i had and replace it with uacpi calls :^)

[fiery turtle]

i mean this isnt necessarily an error lol

some hw has acpi mode enabled by default

for example surface stuff i think

[left orbit]

i wasn't getting any port/memory reads at all though

so i figured it was failing early

[fiery turtle]

    if (!fadt->smi_cmd)
        return HW_MODE_ACPI;

[left orbit]

then i saw there was some smi_something check in read_mode

yeah

that

[fiery turtle]

since u disabled smm there's no way to call into SMI

[left orbit]

is that in any way related to smm? lol

ahj

that makes sense

[fiery turtle]

SMI is system managamenet interrupt

[left orbit]

that makes a lot of sense actually

[fiery turtle]

@hasty plinth @jaunty fox okay so help me understand here

and @left orbit

I rewrote 95% of frees to be sized

5% is freeing strings of size that I just don't know at free site

How am I supposed to do this?

Do I just strlen every time for every free?

Seems wasteful as this hint won't be used by a lot of allocators

example:

    absolute_path = uacpi_namespace_node_generate_absolute_path(node);
    uacpi_warn(
        "aborted execution of '%s.%s' due to an error: %s\n",
        absolute_path, method, uacpi_status_to_string(ret)
    );
    uacpi_kernel_free((void*)absolute_path);

[left orbit]

are the 5% of cases error handling?

if that's the case i would say it doesn't matter that much the overhead of strlen on the cold path

[fiery turtle]

most callers of uacpi_namespace_node_generate_absolute_path mostly are, but not all of them

there's a method that returns an array of strings of arbitrary length

[left orbit]

technically, but idk how good of a solution that would be, is to let the library consumer decide whether the frees are sized or not

using some preprocessor directive

like UACPI_USE_SIZED_FREE

you could hide the sized frees of statically known size behind a macro

[fiery turtle]

hmm yeah that could be a solution definitely

[left orbit]

and for the dynamic sized ones just strlen for example

[fiery turtle]

like this definitely won't be used by a lot of allocators, e.g. on linux querying object size is an O(1) operation

u just get the struct page of the allocation and then the link back to slab and that has object size

[jaunty fox]

Could expose a strfree deo

Let people choose

[fiery turtle]

/*
 * Free a previously allocated memory block.
 *
 * An optionally enabled 'size_hint' parameter contains the size of the original
 * allocation. Note that in some scenarios this incures additional cost to
 * calculate the object size, so a non-sized version should be preferred if
 * possible.
 */
#ifdef UACPI_SIZED_FREES
void uacpi_kernel_free(void *mem, uacpi_size size_hint);
#else
void uacpi_kernel_free(void *mem);
#define uacpi_kernel_free(ptr, _) uacpi_kernel_free(ptr)
#endif

how about this

[left orbit]

how does that #define work?

[fiery turtle]

the second parameter gets preprocessed into nothing

there

[left orbit]

yep i was a little bit confused

[fiery turtle]

ye

[left orbit]

can you have "recursive" macros like that?

as in

[fiery turtle]

yeah that works

[left orbit]

that is interesting

[fiery turtle]

the preprocessor is not recursive

[left orbit]

i've never seen that used before honestly, but it's really cool

yeah i know

i just didn't know how that would behave

[jaunty fox]

Is string length not stored anywhere else you can use at free time?

[left orbit]

yeah basically my other solution was to create a uacpi_string struct that holds ptr and size

[fiery turtle]

the most "difficult" user of this is uacpi_pnp_id_array, which is a list of strings returned by the firmware

the strings are arbitrary size

or could be integers that got transformed into strings

actually you know what would be a good solution

[left orbit]

but that would be a muuuch bigger refactor

[fiery turtle]

precalculate the array size and put all strings into one large buffer with a known size

i guess

[fiery turtle]

yeah i dont really want to do stuff that will hurt the general user for no reason

like storing this size i dont need

[left orbit]

yeah that's why i didn't really suggest it at first

it's not that good of an idea

but i think this opt-in sized free is a nice idea

people who don't care aren't affected

and people who need that can enjoy it

win-win

[fiery turtle]

for (i = 0; i < out_list->num_entries; ++i) {
    uacpi_object *object = objects[i];

    switch (object->type) {
    case UACPI_OBJECT_STRING:
        out_list->ids[i] = steal_or_copy_string(object);
        if (uacpi_unlikely(out_list->ids[i] == UACPI_NULL))
            ret = UACPI_STATUS_OUT_OF_MEMORY;
        break;
    case UACPI_OBJECT_INTEGER:
        out_list->ids[i] = uacpi_kernel_alloc(8);
        if (uacpi_unlikely(out_list->ids[i] == UACPI_NULL)) {
            ret = UACPI_STATUS_OUT_OF_MEMORY;
            goto out_late_error;
        }

        uacpi_eisa_id_to_string(object->integer, out_list->ids[i]);
        break;
    default:
        ret = UACPI_STATUS_AML_INCOMPATIBLE_OBJECT_TYPE;
        goto out_late_error;
    }
}

idk maybe its worth changing this to be a preallocated array of fixed known size

ACPICA does that i think

[left orbit]

off topic but ive been meaning to ask, how do you test windows with uacpi? lol

also what windows version do you test it on

[fiery turtle]

/* Structures used for device/processor HID, UID, CID */

typedef struct acpi_pnp_device_id
{
    UINT32                          Length;             /* Length of string + null */
    char                            *String;

} ACPI_PNP_DEVICE_ID;

typedef struct acpi_pnp_device_id_list
{
    UINT32                          Count;              /* Number of IDs in Ids array */
    UINT32                          ListSize;           /* Size of list, including ID strings */
    ACPI_PNP_DEVICE_ID              Ids[];              /* ID array */

} ACPI_PNP_DEVICE_ID_LIST;

this is how ACPICA does it

is this better than just list of strings? idk

[fiery turtle]

you mean how I test AML against windows?

[left orbit]

yep

[fiery turtle]

#1217009725711847465 message

i feed it AML blobs via QEMU and check the output via a debugger

[fiery turtle]

some windows10 image i found online

in qcow2 format

[left orbit]

that is pretty smart, i thought it was a bit more complicated ngl

[fiery turtle]

qemu-system-x86_64 -M q35 -hda virtioa.qcow2 -serial tcp::4445,server,nowait -m 4G -smp 8 -acpitable file="..\..\uACPI\tests\reftest.aml" -accel whpx -L "C:\Program Files\qemu\share"

the table has to be called SSDT or windows won't load it and a few more annoying things but it mostly just works

[fiery turtle]

anyway RE: sized frees

ill just steal this design

one large allocation is better than 10 small ones anyway

for error handling ill just bite the bullet and store the size somewhere

so this won't be opt-in as no extra cost there

[hasty plinth]

Sorry I'm late to the party, was sleeping and working. Seems like you figured it all out?

I suppose you could always prefix the string length to the actual data, but that'd be kind of a pain to adopt this late into a codebase.

[fiery turtle]

Yeah I also figured out it was mostly design skill issue in some of those places and knowing the length will make the code better anyway

[hasty plinth]

sounds like everyone wins 😄

[fiery turtle]

Okay looks like sized frees work

I added some code to the test runner to verify that the amount of byte being freed matches the original allocation

[rustic compass]

FYI i said that i will create a wrapper crate for uacpi in the future, but as of now it looks like that wont really be possible as uacpi needs a lot of bidirectional interconnection with the rust code and getting that to work as a importable crate currently vastly exceeds my knowledge

but a short documentation on how to use uacpi in a rust project directly should work out

[fiery turtle]

bidirectional interconnection..?

[rustic compass]

the stdlib stub thing i saw in the source probably needs to access some custom rust code and thats exceeds my knowlegde on how to passthrou that when wrapping it in a crate

[fiery turtle]

like a memcpy?

i mean the crate could ship its own implementation

[mortal yoke]

default impls of mem*/strlen functions are also provided by the compiler builtins so you wouldn't need to do anything about them in a crate like that

basically the stdlib functions that would have to be implemented are strncmp/strcmp/strnlen/snprintf, you could implement those as weak symbols in the crate and then if an user wants to provide an alternate impl they can just have a non-weak function

[fiery turtle]

yeah

i might actually do that at least for snprintf because its annoying

[mortal yoke]

I guess you could also do it for the kernel printfs at the same time if you do it for snprintf? it should be pretty easy to make them share the impl

it would be nice if the kernel doesn't otherwise need a printf (eg. uses c++ overloading or whatever for printing)

[fiery turtle]

yeah that's what i meant

[verbal herald]

I'm currently porting uACPI to my OS

It's written in Zig

How do you do free shit in uACPI?

[north holly]

look at kernel api

[verbal herald]

i know

i did that

its just that i don't know how to translate free to zig

yes i did use zig alloc but i keep getting errors

[fiery turtle]

What's the problem exactly

I know at least two people I think that ported it to zig and it worked for them

The only problem is unsized frees but thats coming soon

Meanwhile you can just insert a header with size for each allocation

[verbal herald]

i'll going to wait for sized free support for now

[fiery turtle]

Sure

[fiery turtle]

Sized frees are done

ill do a bit of testing and then it should be good for merge

(fixed msg)

This PR is getting a bit too large unfortunately

[fiery turtle]

Also CI now tests 32-bit builds as well

[fiery turtle]

sized frees are available (on an experimental branch), you can see if that works for you

[kindred beacon]

hmm I just had a look at the meson.build

it might be nice to provide a library and a dependency there

I'll take a look at this later once I have some time

[fiery turtle]

Why tho? But im not good at meson so no idea

[kindred beacon]

for wrap files esp, providing a dependency is expected

[fiery turtle]

I see

[kindred beacon]

that has the benefit of allowing you to rearrange internals

like a downstream user does not need to rely on knowing that you provide source files under var x or include dirs under y

when it can just use the dependency

I think there's a deprecation warning in meson when poking into subprojects for stuff like that

[fiery turtle]

Cool sounds useful

[fiery turtle]

Found a bug with a 32-bit build

handle is 32 bits obviously so the upper half of the 64-bit member was not getting zeroed

none of the existing tests seem to have triggered it

only found out by feeding it my thinkpad t14s blob

[fiery turtle]

any progress there btw?

[hollow elm]

hmm maybe it's time for another round of fuzzing

[winter orbit]

A little bit I think uACPI might be too good

Ros doesn’t have arbiter so using windows ACPI although I got it working.. on actual hardware leads to some wild lock ups

I think I might be stuck until the core issue in the kernel is solved

[fiery turtle]

arbiter?

[fiery turtle]

wait until i merge this PR

ill get it merged tmrw probably

[winter orbit]

NT has a concept for IO conflicts and how to handle them

[hollow elm]

alright

[fiery turtle]

it has tons of fixes

[winter orbit]

ROS has a lot of old dos era hardware detection I’ve been removing and removing

So the result is a lot of crappy hardware resource conflicts

[fiery turtle]

How is this related to acpi exactly?

[winter orbit]

I can’t do much with the enumeration I find

So I.e ros doesn’t get to desktop

[fiery turtle]

wdym

is some api missing?

[winter orbit]

Our ACPI is super hacked to ignore 90% of hardware atm

No it’s nothing to do with uACPI lol

[fiery turtle]

how do you set it up to ignore hw?

[winter orbit]

Like I’m having more progress getting it to work on windows then ros itself

[winter orbit]

Wanna see lol? This is actually a source of about 70% of the hardware driver BSoDs

[fiery turtle]

yea

[fiery turtle]

im a bit confused tbh about what exactly it is that you did lol

[winter orbit]

ah tldr:
I started this, got some basic stuff working like getting resources for devices etc,
And realized oh crap I can’t progress farther than this until I fix soemthing else

[hollow elm]

btw i just remembered, the iasl version check in run_tests.py is not sufficient if "20200925" in out: vs ASL+ Optimizing Compiler/Disassembler version 20200717

[fiery turtle]

as far as i understand:
on reactos - works in some places, locks up on real hw
on windows(?) - sort of works?

[winter orbit]

https://github.com/reactos/reactos/blob/master/drivers/bus/acpi/pnp.c

https://github.com/reactos/reactos/blob/master/ntoskrnl/io/pnpmgr/pnpres.c

Anyway if you wanna see how awfully ros handles this stuff atm:

[winter orbit]

yes

But I haven’t finished implementing everything yet :) it’s only like 40% done

[fiery turtle]

so what breaks exactly?

[winter orbit]

On ros? Uhh hang on im on my phone

Ping me again in like 4 hours ~

And I can give a better show case

[fiery turtle]

because u mentioned resource conflicts and like e.g. managarm couldnt care less about resource conflicts and it works

[mortal yoke]

is that the gentoo iasl? if yes I had that same issue but I just decided to update it to a version that is not many years out of date like the current one in the gentoo repo is

[fiery turtle]

like its a way less advanced kernel and it still works on real hw

so im not sure what exactly u mean

[hollow elm]

yea it is, did you submit a pr to gentoo/gentoo or only updated it locally

[fiery turtle]

yeah it might need a smarter < or wahtever

[mortal yoke]

no I just updated it locally, I don't really know how the gentoo upstreaming process works

[winter orbit]

Managram doesn’t let drivers take control of resources and be the only one accessing it last I saw though

[hollow elm]

you should be able to get away with parsing it as an int and just comparing (although idk what the first non-broken version is)

[fiery turtle]

oh right its just an int

ill just set wahtever ubuntu 22 ships

[hollow elm]

you can just make a pr on the github gentoo/gentoo repo

[mortal yoke]

ah nice, Ill make one

[fiery turtle]

and with ACPICA it does work because?

i mean reactos

[winter orbit]

It doesn’t we hard code only loading like 6 specific drivers

I am removing all the hacks that do this as I rewrite ACPI .sys

It’s crap in pnpmgr and ACPI

[fiery turtle]

oh so u removed some old hacks while switching to uacpi?

[winter orbit]

That’s what I’m saying yes

[fiery turtle]

okay i see then

[winter orbit]

sorry I wasn’t clear

I actually was removing hacks rewriting Hal to make it work with windows ACPI

[fiery turtle]

couldn't u just keep them in to at least eliminate uacpi being broken

[winter orbit]

probably but i just kinda took the mentality of doing it right If I’m touching something

[fiery turtle]

i see

[winter orbit]

I’m the first core dev to touch this stuff in awhile

[fiery turtle]

makes sense

also reactos is the reason i decided to make 32-bit builds "first class citizen"

it was completely untested before

it was on the todo list anyway but still

[winter orbit]

That’s super neat I hope we find some cool stuff for you

I’m pretty much 100% sure we’re going to fully swap to this anyway

[fiery turtle]

cool, well if u find any problems or missing api let me know

and looking forward to seeing progress reports too

[winter orbit]

I’ll have more time to dedicate to it after this weird movement thing is done

[fiery turtle]

movement?

[winter orbit]

We’re synching all our wine code and tests so we can pursue compiling the whole OS as windows 7

[fiery turtle]

ohh damn thats cool

good luck there

[winter orbit]

Thanks!!

[fiery turtle]

@winter orbit btw why is the latest reactos release 3 years old?

[winter orbit]

It’s taken awhile to get to the point we’re at right now, I think we’re ready to branch off to a new RC

But yeah we’ll see when that happens

The jump from 0.4.14 -> .15 is massive

[fiery turtle]

so its been too unstable for 3 years to make a release?

[winter orbit]

We’ve hit a lot of the marks on the checklist for 0.5.0

[winter orbit]

That’s a good way to describe it

[fiery turtle]

nice

lol

[winter orbit]

one of these next few releases will likely be the beta. Depends on how we’re feeling

We’ve kinda just decided to solve every issue every review of ros ran into before jumping the ship

I mean that last Michael MJD video

That GMA GPU driver issue’s fix STILL isn’t in master branch

[fiery turtle]

What branch is it in?

[winter orbit]

Right now it’s just a .patch file floating around till we fix one of the core problems that made the fix necessessry it’s slightly a work around

:D which gets back into needing arbiter / and a new ACPI

[fiery turtle]

ohhh i see

cool stuff

Every review ran into this issue?

[winter orbit]

Here’s the same era of GMA driver from his video hardware accelerating YouTube

Idk if I can post video clips here

[fiery turtle]

you can i think

[winter orbit]

Most of the issues people ran into are fixed now

[fiery turtle]

like driver compatibility?

[fiery turtle]

amazing

[winter orbit]

Driver compatibility is the last one that we’re fixing

The random bricking of installs is gone

We can survive BSoDs now perfectly

Registry can self heal etc

Random Mm crashes when running browsers is gone

etc

[fiery turtle]

wow, cool stuff

looks like you guys made a lot of progress fast

[winter orbit]

Hell even Audio drivers semi work now! We have YouTube audio play back at 4K

[fiery turtle]

daamn

[winter orbit]

Yeah it’s not bad, stuff to do still but I’m excited

This core Hal/ACPI/PnP stuff is the last thing really bugging me

[mortal yoke]

I tried the last reactos release a few days ago and when I uncleanly shut it down then it just didn't boot anymore (well it did boot to the bootloader but then it didn't go further)

[winter orbit]

And ntfs support

[winter orbit]

Yeah that’s the issue

The last release

That’s been fixed for about 2 years now

[fiery turtle]

yeah it says 2001 on that file u sent

[winter orbit]

exactly

When this is fixed we can load pretty much all drivers for XP/2k3 and if the retarget goes well

We’ll load even windows 7 video drivers

[fiery turtle]

daaamn

so u can even run modern nvidia cards technically?

[winter orbit]

I did already I just have a lot of problems with the code base

WDDM is.. interesting

It needs to be rethought out in some spots

[fiery turtle]

the windows DRM?

[winter orbit]

WDDM is the windows vista -> 11video driver and acceleration stack

[fiery turtle]

thats really cool

[fiery turtle]

i see

well thats super exciting!

[winter orbit]

It’s still far from enter master tbh

Thanks :)

[sterile egret]

@winter orbit You should create a thread for your work! I for one would follow along

[winter orbit]

Hmm alright next time I got something interesting going on I’ll make a thread

[fiery turtle]

Added a new log level to uACPI called DEBUG, this will hopefully allow to get rid of compile time defines guarding certain extra tracing

also improved op tracing and added micro op tracing as well

this is the full trace of the ToHexString opcode

here's an interesting case of CondRefOf where the null case is implemented entirely in microcode

[hollow elm]

nice

how's the uacpi pr going btw?

i see it was merged

ig it's time for another fuzzing campaign then

[fiery turtle]

indeed

[fiery turtle]

Yes, the PR is finally in after like two weeks of being open i think!

@jaunty fox @hasty plinth @verbal herald sized frees mode is now available on master if u wanna try that

alongside tiny public api breakage

[hasty plinth]

cheers for the ping, keen to try it!

[fiery turtle]

@left orbit you might be interested as well i think(?)

[fiery turtle]

is your kernel available on github btw?

[fiery turtle]

btw set the runner log level to DEBUG for better logs

[hollow elm]

qookie@selenium /tmp/uacpi λ _build/test-runner fuzz-out/default/crashes/id\:000000\,sig\:06\,src\:000334\,time\:98340\,execs\:63717\,op\:havoc\,rep\:1 
[uACPI][WARN] invalid table 'DSDT' checksum!
[uACPI][TRACE] load of 'DSDT' (OEM ID '' OEM Table ID '')
invalid free size: originally allocated 0 bytes, freeing as 1
Aborted

interesting ```
[uACPI][TRACE] fixed event 0 enabled successfully
[uACPI][TRACE] activated all 'TableData' opregions controlled by '', 0 _REG() calls (0 errors)
/tmp/uacpi/source/uacpi.c:610:9: runtime error: store to null pointer of type 'uacpi_object *' (aka 'struct uacpi_object *')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/uacpi/source/uacpi.c:610:9 in
AddressSanitizer:DEADLYSIGNAL

==133951==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x55b4c23226e7 bp 0x55b4c24a94d8 sp 0x7ffed3c1e9a0 T0)
==133951==The signal is caused by a WRITE memory access.
==133951==Hint: address points to the zero page.
#0 0x55b4c23226e7 in uacpi_eval /tmp/uacpi/source/uacpi.c:610:14
#1 0x55b4c23bcc3a in region_run_reg /tmp/uacpi/source/opregion.c:104:11
#2 0x55b4c23bd765 in do_run_reg /tmp/uacpi/source/opregion.c:373:11
#3 0x55b4c23b8100 in uacpi_namespace_for_each_node_depth_first /tmp/uacpi/source/namespace.c:476:17
#4 0x55b4c23bdf0e in uacpi_reg_all_opregions /tmp/uacpi/source/opregion.c:411:5
#5 0x55b4c2321788 in uacpi_namespace_initialize /tmp/uacpi/source/uacpi.c:566:13

[hollow elm]

looks like that's the only 2 things so far

[hollow elm]

okay after 2 hours it's only found those two crashes

[north holly]

Time to test my kernel with the new uACPI updates

[fiery turtle]

Interesting

[north holly]

Can confirm nothing obvious changed

[fiery turtle]

Can you send those over?

[north holly]

ie. it worked

[fiery turtle]

Not bad

[hollow elm]

the invalid free is coming from free_buffer

[fiery turtle]

I didn't know there was a code path that allocated 0 length but I guess I was wrong

[hollow elm]

[uACPI][DEBUG] RESUME OP 'ToBufferOp' (0x0096)
[uACPI][DEBUG]     pOP: OBJECT_ALLOC_TYPED (0x19)
[uACPI][DEBUG]     pOP: INVOKE_HANDLER (0x14)
[uACPI][DEBUG]     pOP: STORE_TO_TARGET (0x27)
[uACPI][DEBUG]     pOP: OBJECT_TRANSFER_TO_PREV (0x25)
[uACPI][DEBUG]     pOP: <END-OF-OP> (0x00)
[uACPI][DEBUG] END OP 'ToBufferOp' (0x0096)
invalid free size: originally allocated 0 bytes, freeing as 1

unfortunately i can't disassemble it with iasl

because it's convinced it's not even a table

[fiery turtle]

Lol

[fiery turtle]

Hmm to buffer can allocate empty buffers I guess

This is not really a crash just the free code would get the wrong value

But should be fixed ofc

[fiery turtle]

This one is very interesting, not at my pc right now so cant check

[fiery turtle]

Some public api did change

I guess you weren't using it

I had to change a bit of managarm code

[north holly]

My kernel doesn't make use of uACPI API yet

After my VFS it should because of the driver loading code

[hollow elm]

    If ((\PCI1.STAT == 0x01))
    {
        0x01 = (Local0 - \PCI1.XHCI.WRIT) /* External reference */
        Local0
    }
    Else
    {
         = "PCI1.XHCI._REG was never called"
    }

love the assignment to nothing

[fiery turtle]

Lol

Its probably a store to 0

[hollow elm]

okay with enough patching with a hex editor i got one of the free abort inputs to disassemble

https://uwu.foundation/4wsh5Dr.png

and i think iasl is stuck in an infinite loop

Disassembly completed
ASL Output:    foo.dsl - 32699 bytes
Pass 1 parse of [R, g]
ACPI Warning: Invalid character(s) in name (0x43501044), repaired: [D*PC] (20200717/utstring-337)
ACPI Warning: Invalid character(s) in name (0x53083149), repaired: [I1*S] (20200717/utstring-337)
ACPI Warning: Invalid character(s) in name (0x0A544154), repaired: [TAT*] (20200717/utstring-337)

[fiery turtle]

Lmao

[hollow elm]

okay this is insanee

[fiery turtle]

okay i found it

handle_mid

MidOp allocates 0 bytes in some edge cases

and i treat it as a null buffer in free (aka 0 bytes long but in reality 1)

static void free_buffer(uacpi_handle handle)
{
    uacpi_buffer *buf = handle;

    if (buf->data != UACPI_NULL)
        /*
         * If buffer has a size of 0 but a valid data pointer it's probably an
         * "empty" buffer allocated by the interpreter in make_null_buffer
         * and its real size is actually 1.
         */
        uacpi_free(buf->data, UACPI_MAX(buf->size, 1));

    uacpi_free(buf, sizeof(*buf));
}

this UACPI_MAX is the reason it fails

anyway this is a trivial fix

lets see about the store to null

[fiery turtle]

ohh damn this one found an actual bug

uacpi_eval unconditionally assumes that if the object being evaluated is not a method we want to retrieve the object

but the caller might not have provided a return pointer

as was the case for this _REG method

[fiery turtle]

lmao

guaranteed

yeah right

okay that was relatively easy

[hollow elm]

nice

[fiery turtle]

alright fixes merged

thanks qookie

[hollow elm]

np

ig i'll do another round of fuzzing

[fiery turtle]

yeee

[north holly]

shouldn't break anything in my kernel

but might as well test

[fiery turtle]

this would only break on insane aml

like the one this fuzzer produces

[hollow elm]

fuzzing bytecode has to be one of my new favorite activities

the decompiled results are just insane

[fiery turtle]

lmao

[hollow elm]

also that's nice of afl ```
[*] Setting up output directories...

[-] The job output directory already exists and contains the results of more
than 25 minutes worth of fuzzing. To avoid data loss, afl-fuzz will NOT
automatically delete this data for you.

[fiery turtle]

I doubt this code was there from the start

[north holly]

as expected everything works

[fiery turtle]

cool

[north holly]

ikr

[hollow elm]

hm so far no crashes found

i wonder if it might be worth it to add dsdts from real machines into the fuzzer input pool

since every time the number of edges seems to bottom out at about 32%

[fiery turtle]

yeah ideally u acpixtract every dsdt blob from https://github.com/linuxhw/ACPI

[north holly]

I should put my computer on there

[hollow elm]

good thing you mentioned acpixtract i was about to start writing my own parser :^)

also what about the ssdts?

[fiery turtle]

well

you could load them but they contain references to other tables

so on their own they're useless

[hollow elm]

i was thinking of somehow combining them with the dsdts

[fiery turtle]

yeah u can just cat them technically

if you strip the header

or extend the test runner to support multiple blobs

if you cat make sure to update the length in the dsdt

[hollow elm]

https://uwu.foundation/5it2qk9.png

[fiery turtle]

wait 4575 input files..?

[hollow elm]

yeah i didn't delete the ssdts

figured it wouldn't hurt to mix them in

[fiery turtle]

yeah

thats nice

[hollow elm]

although it looks like it might hurt actually

[fiery turtle]

why is that?

[hollow elm]

well it significantly increases the corpus size

with not particularly interesting inputs

unless the ssdts have _INIs or table-level code ig

[fiery turtle]

meh

some do

like SSDT2/3 is usually intel gpu stuff

[north holly]

didn't you say at one point you wanted to do some sort of intel gpu driver stuff

[hollow elm]

[+] Found 33483 unique tuples across 4575 files.
[+] Narrowed down to 244 files, saved in 'new-corpus-unique'.

[fiery turtle]

that's my next project, yes

or ill focus on the kernel, not sure

[fiery turtle]

cool cool

[hollow elm]

it also found a bunch of crashing inputs :^)

[fiery turtle]

like instantly?

[hollow elm]

oh nvm, it says crashing but that also includes timeouts

[fiery turtle]

i really need to implement hanging aml aborts

[hollow elm]

one of the files just gets into an infinite loop

reading from memory constantly, so ig waiting on mmio or something

[fiery turtle]

yeah imagine implementing a timeout

wouldnt be a good firmware dev

i know HP blobs often are guilty of that

they just infinitely poll some memory address

[hollow elm]

wtf

why does afl-fuzz default to a timeout of 1000ms but afl-cmin defaults to a timeout of 5000ms

this means that even if afl-cmin let it through afl-fuzz might complain that the input corpus times out

[fiery turtle]

1000ms is definitely too little

especially with asan

[hollow elm]

[!] WARNING: Test case 'id:000080,time:0,execs:0,orig:2067241125710dsdt.dat.bin' results in a crash, skipping

huh it is a use-after-free

[fiery turtle]

daamn

[hollow elm]

not even fuzzer output

just a plain dsdt from linuxhw/ACPI

[fiery turtle]

lol

can u show it

or the log

[fiery turtle]

did the physical address happen to match some malloc'ed memory?

[hollow elm]

right above it is ```
[uACPI][TRACE] read from [_SB_.PCI0.SDC_.SCGP] (4 bytes) SystemMemory[0x00000000000006B0] = 0x0
[uACPI][TRACE] write to [_SB_.PCI0.IPCR] (4 bytes) SystemMemory[0x0000000000000080] = 0x4

[fiery turtle]

the virtual backing is what's interesting here

[hollow elm]

i can just send you the dsdt

[fiery turtle]

yea pls

[hollow elm]

don't mind the filename too much :^)

[fiery turtle]

lol

yeah im afraid its some is_physical_address bullshit again

lemme double check tho

[hollow elm]

i did make is_physical_address use mincore locally

    unsigned char vec[512];
    assert((size + 4091) / 4092 < 512);
    int res = mincore((void*)(addr & ~0xFFF), size + (addr & 0xFFF), vec);

    return res < 0 || !vec[0];

[fiery turtle]

nahh, its just some previous region access happened to be a valid usersapce address

im guessing

because it doesnt repro under a debugger

[hollow elm]

hm

btw, in what case would map be called with a non-physical address here?

[fiery turtle]

if some opregion address happened to collide with a memory returned from malloc previously

in that case mincore/write would both succeed

and the mapper would assume its a virtual address

[hollow elm]

hm

[fiery turtle]

early table mapping

is the only use case

because DSDT is obviously virtual

[hollow elm]

maybe you could just tag the pointers in the first 16 bits (doesn't work on 32-bit though) if they're virtual

[fiery turtle]

yeah

the current mechanism is just broken beyond repair

like it literally says

    path = uacpi_namespace_node_generate_absolute_path(node);

    uacpi_trace(
        "%s [%s] (%d bytes) %s[0x%016"UACPI_PRIX64"] = 0x%"PRIX64"\n",
        type_str, path, byte_size,
        uacpi_address_space_to_string(op_region->space),
        offset, ret
    );

    uacpi_free_dynamic_string(path);

use after free for this path

which is a local variable

    case 4:
        *(volatile uacpi_u32*)ptr = in;

used-after-free here

[hollow elm]

hm yeah seems unlikely :^)

[fiery turtle]

yeah basically a tag seems like the way to go

[hollow elm]

and the fuzzer seems to have reinvented this crash even though the crashing input was discarded :^)

[fiery turtle]

although the problem is

how do u make this tag work on 32-bit

[hollow elm]

i suppose you only really need 1 bit of information

if you can figure out a way to limit allocations to the first 2GB of the addr space on 32-bit builds that would work

[fiery turtle]

maybe another way would be to explicitly tell it like here's a list of valid pointers to be treated as virtual

because its only a few

like that DSDT and other tables are generated at startup

[hollow elm]

that could work as well

ig there still is a risk of collision but at that point it's very small

[fiery turtle]

yeah

❯ ./runner/build-linux-64bits/test-runner /mnt/d/2067241125710dsdt.dat.bin
0x00007FFDB46130A0 is NOT a physical address!
0x00007FFDB46130F0 is NOT a physical address!
0x00007FFDB46130F0 is NOT a physical address!
0x0000612000000340 is NOT a physical address!
0x0000612000000340 is NOT a physical address!
0x0000630000000400 is NOT a physical address!
0x0000630000000400 is NOT a physical address!
[uACPI][TRACE] load of 'DSDT' (OEM ID 'ALASKA' OEM Table ID 'A M I ')

so its basically like 4 addresses that really need to be checked

[north holly]

The obvious solution is to just give the dsdt blobs to my kernel /s

Then you have no problem with physical address

I mean I could use with the free bug reports...

related to my allocator or whatever shits itself in my kernel with the dsdt blobs

[fiery turtle]

@hollow elm

i found the issue

it's actually a different problem

Mapping 0x00000000000006B0 with size 44
[uACPI][TRACE] read from [\_SB_.PCI0.SDC_.SCGP] (4 bytes) SystemMemory[0x00000000000006B0] = 0x0
Mapping 0x0000000000000000 with size 160
[uACPI][TRACE] write to [\_SB_.PCI0.IPCR] (4 bytes) SystemMemory[0x0000000000000080] = 0x4

basically...

since 0x000000 is already mapped

it just takes that mapping

but it was mapped previously with a smaller size

do u see how this can be an issue?

the mapper needs to save the size of the mapping and realloc i guess

[hollow elm]

ah lol

[fiery turtle]

earlier in the log there's this

Mapping 0x0000000000000000 with size 8
[uACPI][TRACE] read from [\_SB_.ILBR] (1 bytes) SystemMemory[0x0000000000000007] = 0x0
[uACPI][TRACE] read from [\_SB_.ILBR] (1 bytes) SystemMemory[0x0000000000000006] = 0x0
[uACPI][TRACE] read from [\_SB_.ILBR] (1 bytes) SystemMemory[0x0000000000000005] = 0x0
[uACPI][TRACE] read from [\_SB_.ILBR] (1 bytes) SystemMemory[0x0000000000000004] = 0x0
[uACPI][TRACE] read from [\_SB_.ILBR] (1 bytes) SystemMemory[0x0000000000000003] = 0x0
[uACPI][TRACE] read from [\_SB_.ILBR] (1 bytes) SystemMemory[0x0000000000000002] = 0x0
[uACPI][TRACE] read from [\_SB_.ILBR] (1 bytes) SystemMemory[0x0000000000000001] = 0x0
[uACPI][TRACE] read from [\_SB_.ILBR] (1 bytes) SystemMemory[0x0000000000000000] = 0x0

why would they even read IVT lol

ohh it probably maps the memory it retrieves from some register dynamically

and siince that's stubbed out it just maps 0

yeah this userspace implementation of map/unmap is wack

[hollow elm]

not much you can do

[fiery turtle]

its possible to work around every edge case

will need a bit of work to correctly realloc the existing dsdt if a collision happens

but its doable

ill definitely do it tho because it unlocks proper fuzzing

[north holly]

you wouldn't have this problem if you just use a my kernel as the test runner

[fiery turtle]

your kernel doesnt have ASAN or UBSAN

[north holly]

not yet

[fiery turtle]

so we wouldnt be able to see problems instantly

[north holly]

Well too much work to do that so I'm just going to make some script to run qemu with all those dsdt blobs

and hope nothing in qemu blows up

so how do I add an acpi table to qemu

I think there was a command line option

[fiery turtle]

-acpitable file=...

[hollow elm]

https://uwu.foundation/4te7Q6M.png lol discord moment

also so far no crashes were found other than the one i posted

although it's running significantly slower than on just the tests

[fiery turtle]

looks like we fuzzed it quite well

[hollow elm]

but the coverage is also a bit better at least, 34%

[fiery turtle]

how is it so tiny damn

[hollow elm]

a chunk of that is the public apis that go unused ig

[fiery turtle]

maybe we could also make the runner attempt to retrive pci routings and _CRS of everything

[hollow elm]

yeah that'd probably exercise it a bit more

it's also possible i'm misunderstanding the output

[fiery turtle]

gcov showed like 84% last time i tried

but its possible i have no idea what im looking at as well

[hollow elm]

i think this is coverage of paths through the code, not just % of code executed in total

[fiery turtle]

yeah maybe

[hollow elm]

so paths foo() -> bar() and baz() -> bar() would be separate, but for gcov bar() would be covered if only one of the two paths was triggered by tests

[fiery turtle]

makes sense

[hollow elm]

now i wonder what gcov would say the coverage was for all the fuzzer inputs

[fiery turtle]

tests + all DSDT blobs would be interesting

[wind fiber]

Work on lil

[hollow elm]

okay the fuzzer found what appears to be an actual use-after-free

[fiery turtle]

Interesting, ill take a look tomorrow

If you send the blob

[fiery turtle]

Its more modest focused, I dont really care about that for now

[hollow elm]

that particular stack trace is from this blob

[fiery turtle]

Thanks

[hollow elm]

there's a few other but they differ in the where the object was allocated/freed

[fiery turtle]

Uhhh

You sure it isnt a repro of that thing earlier?

[hollow elm]

no memory_write anywhere

    if ((*ret) != UACPI_NULL)
        type_mask = 1 << (*ret)->type;

[fiery turtle]

Oh well, ill take a look

[hollow elm]

dying heree

[fiery turtle]

Maybe its a bug in package free code

Although its extensively tested

Maybe some case is missing

[hollow elm]

there's multiple blobs that crash like this

and they have different call stacks for the previously allocated/freed parts