[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
Date: Thu, 11 Feb 2010 08:20:16 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20091209 Fedora/3.0-4.fc12 Lightning/1.0pre Thunderbird/3.0

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

    return stuff

    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.


Anthony Liguori

reply via email to

[Prev in Thread] Current Thread [Next in Thread]