On Tue, Nov 16, 2010 at 05:26:05PM +0900, Isaku Yamahata wrote:
> This patch cleans up command register initialization with
> comments. It also fixes the initialization of io/memory bit of command
> register.
> Those bits for type 1 device is RW.
> Those bits for type 0 device is
> RO = 0 if it has no io/memory BAR
> RW if it has io/memory BAR
>
> Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp>
There's a bug here: you can not assume that device that has
no io BAR claims no io transactions.
Another bug is that migrating from qemu where a bit is writeable to one
where it's RO creates a situation where a RW bit becomes RO, or the
reverse, which might confuse guests. So we will need a compatibility
flag and set it for old machine types.
> ---
> Changes v8 -> v9
> - patch squash
> ---
> hw/pci.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 57 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 962886e..2fc8ab1 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -544,8 +544,53 @@ static void pci_init_wmask(PCIDevice *dev)
>
> dev->wmask[PCI_CACHE_LINE_SIZE] = 0xff;
> dev->wmask[PCI_INTERRUPT_LINE] = 0xff;
> +
> + /*
> + * bit 0: PCI_COMMAND_IO
> + * type 0: if IO BAR is used, RW
> + * This is handled by pci_register_bar()
> + * type 1: RW:
> + * This is fixed by pci_init_wmask_bridge()
> + * bit 1: PCI_COMMAND_MEMORY
> + * type 0: if IO BAR is used, RW
> + * This is handled by pci_register_bar()
> + * type 1: RW
> + * This is fixed by pci_init_wmask_bridge()
> + * bit 2: PCI_COMMAND_MASTER
> + * type 0: RW if bus master
> + * type 1: RW
> + * bit 3: PCI_COMMAND_SPECIAL
> + * RO=0, optionally RW: Such device should set this bit itself
> + * bit 4: PCI_COMMAND_INVALIDATE
> + * RO=0, optionally RW: Such device should set this bit itself
> + * bit 5: PCI_COMMAND_VGA_PALETTE
> + * RO=0, optionally RW: Such device should set this bit itself
> + * bit 6: PCI_COMMAND_PARITY
> + * RW with exceptions: Such device should clear this bit itself
> + * Given that qemu doesn't emulate pci bus cycles, so that there
> + * is no place to generate parity error. So just making this
> + * register RW is okay because there is no place which refers
> + * this bit.
> + * TODO: When device assignment tried to inject PERR# into qemu,
> + * some extra work would be needed.
> + * bit 7: PCI_COMMAND_WAIT: reserved (PCI 3.0)
> + * RO=0
> + * bit 8: PCI_COMMAND_SERR
> + * RW with exceptions: Such device should clear this bit itself
> + * Given that qemu doesn't emulate pci bus cycles, so that there
> + * is no place to generate system error. So just making this
> + * register RW is okay because there is no place which refers
> + * this bit.
> + * TODO: When device assignment tried to inject SERR# into qemu,
> + * some extra work would be needed.
> + * bit 9: PCI_COMMAND_FAST_BACK
> + * RO=0, optionally RW: Such device should set this bit itself
> + * bit 10: PCI_COMMAND_INTX_DISABLE
> + * RW
> + * bit 11-15: reserved
> + */
Let's document non-obvious things, like maybe COMMAND_PARITY/COMMAND_SERR.
I don't cherish writing each bit up in two places.
> pci_set_word(dev->wmask + PCI_COMMAND,
> - PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
> + PCI_COMMAND_MASTER | PCI_COMMAND_PARITY | PCI_COMMAND_SERR |
> PCI_COMMAND_INTX_DISABLE);
>
> memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
> @@ -554,6 +599,9 @@ static void pci_init_wmask(PCIDevice *dev)
>
> static void pci_init_wmask_bridge(PCIDevice *d)
> {
> + pci_word_test_and_set_mask(d->wmask + PCI_COMMAND,
> + PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
> +
> /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
> PCI_SEC_LETENCY_TIMER */
> memset(d->wmask + PCI_PRIMARY_BUS, 0xff, 4);
> @@ -791,6 +839,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> if (region_num == PCI_ROM_SLOT) {
> /* ROM enable bit is writeable */
> wmask |= PCI_ROM_ADDRESS_ENABLE;
> + } else {
> + if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> + pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
> + PCI_COMMAND_IO);
> + } else {
> + pci_word_test_and_set_mask(pci_dev->wmask + PCI_COMMAND,
> + PCI_COMMAND_MEMORY);
> + }
> }
> pci_set_long(pci_dev->config + addr, type);
> if (!(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> --
> 1.7.1.1
