On 02/09/2010 06:24 PM, malc wrote:
On Tue, 9 Feb 2010, Anthony Liguori wrote:
On 02/09/2010 05:36 PM, malc wrote:
Let's see:
Currently we have this
readb(...):
dostuff
return stuff
readw(...):
dostuff
return stuff
And this is completely wrong.
It's completely right, until some future C standard implements proper
algebraic data types.
I've been thinking about what to do here and I think what I've come up
with is to change the callbacks to take be a separate type. So instead
of just:
void (PCIOWriteFunc)(PCIDevice *, pcibus_t addr, int size, uint32_t value);
uint32_t (PCIOReadFunc)(PCIDevice *, pcibus_t addr, int size);
We would have:
typedef struct PCIIOHandler
{
void (*write)(PCIIOHandler *, pcibus_t addr, int size, uint32_t);
uint32_t (*read)(PCIIOHandler *, pcibus_t addr, int size);
} PCIIOHandler;
And then we would have:
pci_register_io_region(PCIDevice *, ... PCIIOHandler *handler);
We can then introduce a mechanism to dispatch to individual functions
based on size. That way, instead of pushing an if (size == 1) else if
(size == 2) check to each device, we let the device decide how it wants
to be invoked.
For the most part, device models don't consistently handle access to registers
via their non native sizes. Some devices return the lower part of a dword
register when accessed with a word accessor. Some devices only return the
register when there's a dword access.
That's up to device to register an accessor and return what's appropriate.
I think the point though is to make it possible for common mechanisms to
be formalized into common code. For instance, a common set of rules
will be, all registers can be accessed in byte, word, or dword size but
most be accessed at natural size boundaries. We should make it simple
for a device to essentially choose that pattern and not have to worry
about explicitly coding it themselves.
We can even take it further, and do something like:
pci_regs = {
PCI_REG_DWORD(REG0, DeviceState, reg0),
PCI_REG_BYTE(REG1, DeviceState, reg1),
...
}
In which case, we only need to handle the case in switch() if we need to
implement a side effect of the register operation.
But none of this is possible if we completely rely on every device to
implement non-dword access in it's own broken way.
Lovely, we are heading full speed into fanatsy land, where PCI BUS itself
accesses device specific stuff.
I never said the PCI BUS is the one that actually got this info. It
could just be common code that implements this logic.
But there are good reasons to normalize device emulation at this level.
For instance, it allows us to implement consistent tracing for device
models using symbolic names for registers as opposed to only being able
to trace IO operations at addresses.
Regards,
Anthony Liguori
