IRC channel logs

<damo22>i instrumented rump and found that bus_space_map() is failing

<damo22>pci_device_map_range(pci_devices[i], addr, len, 1, &ret); is setting ret to 0

<damo22>its a bug in libpciaccess

<damo22>cant find it though

<damo22>printf("success devmem 0x%p\n", *dest);

<damo22>that prints nonzero in pciaccess

<damo22>some function is aliased

<damo22>something is getting called twice

<damo22>pci_device_map_range

<damo22>libpciaccess is calling pci_device_map_range twice somehow before returning

<damo22>s/calling/executing

<damo22>youpi: theres a bug with pci_device_hurd_map_range

<damo22>io_map() calls into x86 map range and that seems to work

<damo22>but when it calls vm_map that fails

<damo22>EINVAL

<damo22>solid_black: ping

<damo22>maybe the size being mapped is not a whole number of pages

<solid_black>pong!

<solid_black>morning

<solid_black>vm_map cannot fail with einval

<solid_black>that's a unix errno, not a mach error

<damo22>it fails with 4

<solid_black>that's KERN_INVALID_ARGUMENT, ok

<damo22>i fixed the rounding to page size

<damo22>but it still fails

<solid_black>that should not be an issue, mach does the rounding itself

<solid_black> https://gitlab.freedesktop.org/xorg/lib/libpciaccess/-/blob/master/src/hurd_pci.c#L166 this is the code to look at, right?

<damo22>yeah

<damo22>now that we are using io_map() why do we need an offset

<solid_black>does io_map succeed?

<damo22>yes

<damo22>and i have logs for inside x86 version of map_range

<damo22>during io_map()

<solid_black>huh, why does it return eperm is robj != wobj

<solid_black>it should just always map wobj

<solid_black>but that's not the issue in question either

<damo22>the vm_map() call fails

<solid_black>is this the thing using vm_region memobj proxy thing?

<solid_black>that I have vague recollection of

<damo22>yeah

<solid_black>is the relevant code in pci-arbiter?

<solid_black>and how do I reproduce this?

<damo22>no, its further down in the file you showed me

<damo22>line 252

<solid_black>yes, looking at that

<solid_black>but the memobj we try to map, where does it come from?

<solid_black>is it created with vm_region_create_proxy?

<damo22>no

<damo22>actually i dont know

<damo22>it would be in the pci arbiter?

<damo22>io_map()

<damo22>we are using that

<damo22>object

<solid_black>yes, but io_map is just the call to get from pci-arbiter

<solid_black>what does pci-arbiter do to create it

<solid_black>oh and also

<solid_black>why are we passing VM_PROT_ALL for max protection with copy=0

<solid_black>that's not gonna work

<solid_black>please change that VM_PROT_ALL to prot

<damo22>uh

<damo22>so prot, prot

<solid_black>yes

<solid_black>let me check if getting that wrong will result in KERNL_INVALID_ARGUMENT, I'd expect KERN_PROTECTION_FAILUREor something

<solid_black>we you could actually be smarter about it

<solid_black>I think mmap in glibc is

<solid_black>if you got returned a wobj, you can pass VM_PROT_WRITE for max_prot even if writing is not being requested at the moment

<solid_black>why isn't this just using mmap btw?

<damo22>can u mmap a pager?

<solid_black>yes

<damo22>uh

<solid_black>file_name_lookup_under -> openat, io_map+vm_map -> mmap

<solid_black>glibc already has all this logic

<damo22>oh man

<damo22>i dont know

<damo22>so what do i mmap ?

<solid_black>is Joan around?

<damo22>the port from file_name_lookup_under?

<solid_black>you can replace the file_name_lookup_under call with openat()

<solid_black>which would give you an fd

<solid_black>then you mmap that

<solid_black>and close it afterwards

<solid_black>but I'm not necesarily saying you should change it like that

<solid_black>just wondering why it wasn't written like that in the first palce

<solid_black>yes, mmap in glibc is smart to notice if robj == wobj, and add the other protection to max_prot in both branches

<solid_black>cool

<solid_black>actually I see I added that, heh

<damo22>so is pci_sys_hurd->root equivalent to a dirfd?

<solid_black>ah, you have a port, not a dirfd

<solid_black>well, you could convert it, but don't bother

<solid_black>this Hurd-level version should work as well

<damo22>yeah i have a port

<solid_black>size == 0 would result in KERN_INVALID_ARGUMENT

<solid_black>mapping outside of the allowed range of the proxy would, too (!)

<solid_black>so this might be it

<damo22>i dont understand why there is an offset

<damo22>isnt the io_map mapping the exact file

<solid_black>io_map retrieves the memory object (aka pager) that can be used to map the file

<damo22>i think offset should be 0

<solid_black>offset = map->base - region->base_addr; <- does that make sense?

<damo22>no

<damo22>offset = 0 would make more sense

<solid_black>looking at pci_device_freebsd_map_range, they map /dev/mem with offset=map->base

<solid_black>whereas map->base is already factored in by us opening a specific pci-arbiter file in my understanding

<solid_black>the linux sysfs version:

<solid_black>off_t offset = map->base - dev->regions[map->region].base_addr;

<solid_black>hm that looks the same as the Hurd version?

<damo22>if we are opening a specific pci-arbiter file, surely the offset is zero?

<damo22>from the start of the file

<solid_black>I don't know

<solid_black>(I don't understand what pci is, exactly, or what kind of offset we want and why)

<damo22>pci region is a MMIO at a physical address

<damo22>thats all

<solid_black>is it always MMIO? so it could be reachable via /dev/mem, as the freebsd version does it?

<damo22>we use dev/mem

<solid_black>ah and pci-arbiter also returns a proxy to /dev/mem

<damo22>device_open mem

<solid_black>so is map->base the start of the whole PCI region, or the speciic part we want?

<damo22>??

<solid_black>also what is it that you're doing overall again? haven't pci-arbiter and libpciaccess been working great this far?

<damo22>its broken on native hw

<solid_black>ah

<damo22>[start, start + size] is a pci region

<damo22>its just a slice of memory

<damo22>that is backed by hardware

<damo22>when i hexdump -C /servers/bus/pci/0000/.../regionX it dumps the region

<damo22>so if i mmap the region, it should let me use it

<damo22>we are just doing that

<damo22>but something is wrong in libpciaccess

<damo22>the hurdish map_range breaks on vm_map() call

<damo22>maybe it just needs to be map->base ?

<damo22>if io_map is giving a proxy to dev/mem @ 0

<damo22>?

<damo22> vm_region_create_proxy (mach_task_self (),

<damo22> (vm_address_t) node->nn->ln->region_maps[reg_num],

<damo22> max_prot, region->size, &proxy);

<damo22>i think its supposed to be 0

<damo22>which works in qemu but fails on hw

<damo22>ok i think i know what is wrong.... the size is 0x800 of the whole region

<damo22>but as thats smaller than a page, mach rounds it up to a whole page but cant map it because the underlying region is truncated?

<damo22>so the original mapping should round up to a whole page first?

<solid_black>ok, so it's creating a proxy for the *region*, of the region->size

<solid_black>what's max_prot that is passes?

<solid_black>oh the size thing would make perfect sense

<solid_black>let me check

<damo22>it needs to create a proxy for the region rounded up to a whole number of pages right?

<damo22>or can we get mach to do that

<damo22>why do we need to care in userspace

<p4r4D0xum>hey, do you guys use uppermem on real machines?

<damo22>uppermem?

<solid_black>indeed, so Mach doesn't round the size when creating the proxy

<solid_black>perhaps it should

<solid_black>but it would be wise for pci-arbiter to do that in userland

<solid_black>is that a security issue though?

<damo22>yes

<solid_black>like if what foolows is a different device, won't we expose a part of its space to a task who's not supposed to have access?

<damo22>indeed

<damo22>how do we fix it properly?

<p4r4D0xum>damo22: it's in the faq - probably outdated. uppermem 786432 should limit mem on systems with more RAM

<solid_black>I don't think there'a proper way

<damo22>solid_black: can we test that the length fits exactly in mach when creating the proxy, but round up the proxy?

<solid_black>wdym round up the proxy?

<damo22>rpc_len

<solid_black>at the end of the day, you're still either mapping the last page, or not

<solid_black>if you do, you expose a part of another region

<solid_black>if yo don't, you don't fully expose this region

<damo22>i see

<solid_black>how is access regulated in pci-arbiter?

<solid_black>all seemingly owned by root:root

<solid_black>-rw-rw----

<damo22>so what do we do

<damo22>probably map the last page

<damo22>we need it

<solid_black>yes, if everything is root:root rw-rw----, it should be fine

<solid_black>so pci-arbiter should round up the size when creating the proxy

<damo22>ok

<youpi>one of the goals was to make the thing possibly exposed to non-root, such as non-trusted driver and such

<youpi>if hardware exposes pieces that are non-pages, that's really a concern for that goal...

<solid_black>hi youpi

<youpi>I wonder though if by construction BARs are limited to some rounding of their base

<youpi>in which case there cannot be anything else in the same page

<youpi>p4r4D0xum: that comment is not really outdated, it's saying "If you have an older version"

<youpi>that older version would still have issues, sure

<p4r4D0xum>youpi: good point

<p4r4D0xum>youpi: btw. do you still run hurd on dell latitude e6420?

<youpi>it's been a long time I didn't try

<p4r4D0xum>question, the one I found at work e6440 has an original sata 80GB and crashes in AHCI mode on kmem

<p4r4D0xum>and in IDE boots with hd0 and then rumpdisk looks for wd0

<youpi> 91346000-913467ff : 0000:00:17.0

<youpi> 91346000-913467ff : ahci

<youpi> 91347000-913470ff : 0000:00:17.0

<youpi>that's what happens on my machine

<youpi>the area is 2k, but the next is 4k away

<damo22>we are fixing a long standing bug shortly that will fix ahci

<damo22>with rumpdisk

<youpi> 91347000-913470ff : 0000:00:17.0

<youpi> 91347000-913470ff : ahci

<youpi> 91348000-91348fff : 0000:00:16.0

<youpi>same for that one

<p4r4D0xum>damo22: good to know, I have loads of old machines at home

<damo22>so do i

<p4r4D0xum>I mean, I can assist with testing

<p4r4D0xum>have 2 T4xxx

<p4r4D0xum>On one medion MT it works - slow but boots just fine

<p4r4D0xum>with IDE PATA HDD

<p4r4D0xum>oh E6400

<p4r4D0xum>Medion MD 8822 to be exact

<solid_black>I'm trying to get a better understanding of how copying objects works in mach vm

<solid_black>it evidently does work, but a bunch of the things about the code are suprising and seemingly wrong

<damo22>fixed, and it probes the controller now, but somehow wont probe the disk

<damo22>new problem

<damo22>maybe we should compile for single threaded locking in rump

<damo22> https://paste.debian.net/plain/1341113

<damo22>gets further on AMD

<damo22>all PCI bases are 16 byte aligned

<damo22>solid_black: https://paste.debian.net/plain/1341113

<solid_black>"irqhelp: device_intr_register failed with 2503"?

<damo22>yeah

<youpi1>ED_ALREADY_OPEN

<damo22>the _CRS is empty for the IRQ _PRT setting though

<damo22>meaning, it doesnt know the interrupt number

<solid_black>either "Cannot share irq" or "You can't have this interrupt" should've been printed for D_ALREADY_OPEN

<damo22> Enter evaluation : _SB.PCI0.PBR5._ADR (Integeirq handler [-1]: new delivery port f67fda58 entry f54a1ee0 You rcan't have this )interrupt

<damo22>its difficult to read these logs when they are interleaved between acpi and rumpdisk

<solid_black>ok, so something in the kernel already handles this irq

<damo22>i think it tries to open -1

<damo22>we should reject that

<damo22>irq -1

<solid_black>that might be unrelated

<solid_black>since there's no "irq handler [%d]" in this printf

<damo22>irq handler [-1]: new delivery port f67fda58 entry f54a1ee0

<damo22>i think the acpi server can return -1 for some get_irq rpc

<damo22>that should not be happening

<damo22>what do we do if the ACPI table does not have the information??

<damo22>maybe we are not parsing it 100% correctly

<damo22>*irq = acpi_get_irq_number(bus, dev, func);

<damo22>it cannot fail!

<damo22>so either we make it bulletproof or we need to allow it to return a failure state and handle the case where we dont get a value

<damo22> // TODO: may need to compare prt_func

<damo22> // prt_func = entry->address & 0xffff;

<damo22>this is probably it

<damo22>(10:14:42 PM) jmcneill: damo22: _CRS will have irq resources for the pci root port, and if they are missing, it means "use MSI"

<damo22>i think we are not matching on the function number

<damo22>so finding the wrong resources

<damo22>The PCI function number in the Address field of the _PRT packages must be 0xFFFF, indicating “any” function number or “all functions”.

<damo22>hmm so not matching on function number is the same thing

<damo22>i added better error handling to acpi server and now i get:

<damo22>Empty _CRS IRQ resource

<damo22>irqhelp: tried acpi to get pci gsi and failed for 00:11.0

IRC channel logs

2024-12-24.log