[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 1/8] qtest: add libqos
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [RFC PATCH 1/8] qtest: add libqos |
Date: |
Wed, 13 Mar 2013 09:03:33 -0500 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Kevin Wolf <address@hidden> writes:
> Am 05.03.2013 um 14:53 hat Anthony Liguori geschrieben:
>> This includes basic PCI support.
>>
>> Signed-off-by: Anthony Liguori <address@hidden>
>
>> +static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno)
>> +{
>> + QPCIBusPC *s = container_of(bus, QPCIBusPC, bus);
>> + static const int bar_reg_map[] = {
>> + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_1, PCI_BASE_ADDRESS_2,
>> + PCI_BASE_ADDRESS_3, PCI_BASE_ADDRESS_4, PCI_BASE_ADDRESS_5,
>> + };
>> + int bar_reg;
>> + uint32_t addr;
>> + uint64_t size;
>> +
>> + g_assert(barno >= 0 && barno <= 5);
>> + bar_reg = bar_reg_map[barno];
>> +
>> + qpci_config_writel(dev, bar_reg, 0xFFFFFFFF);
>> + addr = qpci_config_readl(dev, bar_reg);
>> +
>> + size = (1ULL << ctol(addr));
>
> This doesn't look right. It should be something like:
>
> size = (1ULL << ctzl(addr & ~0x3));
>
> In fact, what must be masked out differs for I/O (0x3) and memory
> (0xf).
You are correct, it should look something like:
if (addr & PCI_BASE_ADDRESS_SPACE_IO) {
size = (1ULL << ctzl(addr & PCI_BASE_ADDRESS_IO_MASK));
} else {
size = (1ULL << ctzl(addr & PCI_BASE_ADDRESS_MEM_MASK));
}
This doesn't deal with 64-bit bars though.
I'll update in the next round. I think this has worked for me because I
have only done single device testing. I did switch from ffz when I
rebased but I think ffz would still have this problem.
>> +QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn)
>> +{
>> + QPCIDevice *dev;
>> +
>> + dev = g_malloc0(sizeof(*dev));
>
> Where is the matching free? I can't seem to destroy a device I
> once queried.
It's missing, I'll add it in the next round.
>> + dev->bus = bus;
>> + dev->devfn = devfn;
>> +
>> + if (qpci_config_readw(dev, PCI_VENDOR_ID) == 0xFFFF) {
>> + printf("vendor id is %x\n", qpci_config_readw(dev, PCI_VENDOR_ID));
>> + g_free(dev);
>> + return NULL;
>> + }
>> +
>> + return dev;
>> +}
>> +
>> +void qpci_device_enable(QPCIDevice *dev)
>> +{
>> + uint16_t cmd;
>> +
>> + /* FIXME -- does this need to be a bus callout? */
>> + cmd = qpci_config_readw(dev, PCI_COMMAND);
>> + cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
>> + qpci_config_writew(dev, PCI_COMMAND, cmd);
>> +}
>
> Wouldn't it make sense to enable bus mastering here as well? Forgetting
> to do this manually is a trap that's easy to fall in...
Indeed, thanks for pointing it out.
Regards,
Anthony Liguori
>
> Kevin
- [Qemu-devel] [RFC PATCH 7/8] libqos: add fw_cfg-pc, (continued)
- [Qemu-devel] [RFC PATCH 7/8] libqos: add fw_cfg-pc, Anthony Liguori, 2013/03/05
- [Qemu-devel] [RFC PATCH 4/8] pci: foreach, Anthony Liguori, 2013/03/05
- [Qemu-devel] [RFC PATCH 8/8] libqos: add malloc, Anthony Liguori, 2013/03/05
- [Qemu-devel] [RFC PATCH 2/8] i440fx-test: add test to compare default register values against spec, Anthony Liguori, 2013/03/05
- [Qemu-devel] [RFC PATCH 3/8] i440fx-test: add test for PAM functionality, Anthony Liguori, 2013/03/05
- [Qemu-devel] [RFC PATCH 6/8] libqos: fw_cfg, Anthony Liguori, 2013/03/05
- [Qemu-devel] [RFC PATCH 1/8] qtest: add libqos, Anthony Liguori, 2013/03/05
- Re: [Qemu-devel] [RFC PATCH 1/8] qtest: add libqos, Andreas Färber, 2013/03/13
[Qemu-devel] [RFC PATCH 5/8] fw_cfg: add qtest test harness, Anthony Liguori, 2013/03/05
Re: [Qemu-devel] [RFC PATCH 0/8] libqos support, Stefan Hajnoczi, 2013/03/05