On Sun, 21 Jul 2019, Mark Cave-Ayland wrote:
Right, moving the detection and handling of the space into map-in rather than relying on the caller to do it seems to be the right thing here. It shouldn't take you too long to make the relevant changes:
- Change pci_bus_addr_to_host_addr() to return arch->cfg_addr if the space code
indicates CONFIGURATION_SPACE (a switch statement is probably better here)
- Create pci_decode_phys_addr() as the opposite to pci_encode_phys_addr() that takes
an array of 3 u32s and sets flags, space_code, dev and addr
I gave it a try and got to this point without problems.
- Switch pci_decode_pci_addr() in ob_pci_map() to use your new pci_decode_phys_addr()
function
- ...at which point it probably makes sense to merge ob_pci_map() into
ob_pci_bus_map_in(). There are a couple of callers that you'll need to fix up, and possibly vga.fs too.
But I got stuck here, there are too many callers and they have different input than the proposed phys_addr[3] so this I don't see how to adjust them. ob_pci_map currently only takes an address which callers get from config->assigned[1] (the BAR if I got that right) but proposed new function would take phys_addr[3] of which only the low 64 bits are in config->assigned. Where to get the phys[0] part? Should I try to encode that at every caller when it's not really needed? That looks inefficient so unless it's already available somewhere we probably need to leave ob_pci_map as it is.
Then ob_pci_bus_map_in takes forth parameters while ob_pci_map takes C ones so merging the two means that all callers could just use the forth map-in word as they would need to push their parameters to stach to pass to ob_pci_bus_map_in(). Either there's a problem with this design or I need more details on how should this work to go further.
Regards, BALATON Zoltan