Re: [PATCH v2 3/4] vfio: vfio-pci device assignment driver

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-08-14 21:09, Alex Williamson wrote:
>>> +static void vfio_map_bar(VFIODevice *vdev, int nr)
>>> +{
>>> +    VFIOBAR *bar = &vdev->bars[nr];
>>> +    unsigned size = bar->size;
>>> +    char name[64];
>>> +    uint32_t pci_bar;
>>> +    uint8_t type;
>>> +    int ret;
>>> +
>>> +    /* Skip both unimplemented BARs and the upper half of 64bit BARS. */
>>> +    if (!size) {
>>> +        return;
>>> +    }
>>> +
>>> +    snprintf(name, sizeof(name), "VFIO %04x:%02x:%02x.%x BAR %d",
>>> +             vdev->host.domain, vdev->host.bus, vdev->host.slot,
>>> +             vdev->host.function, nr);
>>> +
>>> +    /* Determine what type of BAR this is for registration */
>>> +    ret = pread(vdev->fd, &pci_bar, sizeof(pci_bar),
>>> +                vdev->config_offset + PCI_BASE_ADDRESS_0 + (4 * nr));
>>> +    if (ret != sizeof(pci_bar)) {
>>> +        error_report("vfio: Failed to read BAR %d (%s)\n", nr, strerror(errno));
>>> +        return;
>>> +    }
>>> +
>>> +    pci_bar = le32_to_cpu(pci_bar);
>>> +    type = pci_bar & (pci_bar & PCI_BASE_ADDRESS_SPACE_IO ?
>>> +           ~PCI_BASE_ADDRESS_IO_MASK : ~PCI_BASE_ADDRESS_MEM_MASK);
>>> +
>>> +    /* A "slow" read/write mapping underlies all BARs */
>>> +    memory_region_init_io(&bar->mem, &vfio_bar_ops, bar, name, size);
>>> +    pci_register_bar(&vdev->pdev, nr, type, &bar->mem);
>>> +
>>> +    /*
>>> +     * We can't mmap areas overlapping the MSIX vector table, so we
>>> +     * potentially insert a direct-mapped subregion before and after it.
>>> +     */
>>> +    if (vdev->msix && vdev->msix->table_bar == nr) {
>>> +        size = vdev->msix->table_offset & TARGET_PAGE_MASK;
>>> +    }
>>> +
>>> +    strncat(name, " mmap", sizeof(name) - strlen(name) - 1);
>>
>> This could generate an unterminated name if we actually have to cut the
>> appended string. You could set name[sizeof(name)-1] = 0.
> 
> strncat adds the terminator, that's why we have the -1 so that there's
> space for it.  strlen does not include the terminator.

Yep, you are right, forget what I said.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux