Kevin Wolf <kw...@redhat.com> writes:
> Am 05.03.2013 um 14:53 hat Anthony Liguori geschrieben:
>> This includes basic PCI support.
>>
>> Signed-off-by: Anthony Liguori <aligu...@us.ibm.com>
>
>> +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
