On 2011-06-28 00:48, Alex Williamson wrote:
>> - case PCI_CAP_ID_VPD:
>> - case PCI_CAP_ID_VNDR:
>> - assigned_dev_pci_write(pci_dev, address, val, len);
>> + switch (len) {
>> + case 4:
>> + direct_mask =
>> + pci_get_long(assigned_dev->direct_config_write + address);
>> + full_access = 0xffffffff;
>> + break;
>> + case 2:
>> + direct_mask =
>> + pci_get_word(assigned_dev->direct_config_write + address);
>> + full_access = 0xffff;
>> break;
>> + case 1:
>> + direct_mask =
>> + pci_get_byte(assigned_dev->direct_config_write + address);
>> + full_access = 0xff;
>> + break;
>> + default:
>> + abort();
>> + }
>
> Shouldn't there be a if (!direct_mask) return; here?
Yep.
>
>> + if (direct_mask != full_access) {
>> + val &= direct_mask;
>> + val |= assigned_dev_pci_read(pci_dev, address, len) & ~direct_mask;
>> }
>> + assigned_dev_pci_write(pci_dev, address, val, len);
>> }
>>
>> static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>> @@ -1404,6 +1247,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>> return ret;
>> }
>>
>> + /* direct read except for next cap pointer */
>> + memset(dev->direct_config_read + pos, 0xff, PCI_PM_SIZEOF);
>> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] = 0;
>> +
>> pmc = pci_get_word(pci_dev->config + pos + PCI_CAP_FLAGS);
>> pmc &= (PCI_PM_CAP_VER_MASK | PCI_PM_CAP_DSI);
>> pci_set_word(pci_dev->config + pos + PCI_CAP_FLAGS, pmc);
>> @@ -1438,6 +1285,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>> return ret;
>> }
>>
>> + /* direct read except for next cap pointer */
>> + memset(dev->direct_config_read + pos, 0xff, size);
>> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] = 0;
>> +
>> type = pci_get_word(pci_dev->config + pos + PCI_EXP_FLAGS);
>> type = (type & PCI_EXP_FLAGS_TYPE) >> 8;
>> if (type != PCI_EXP_TYPE_ENDPOINT &&
>> @@ -1513,6 +1364,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>> return ret;
>> }
>>
>> + /* direct read except for next cap pointer */
>> + memset(dev->direct_config_read + pos, 0xff, 8);
>> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] = 0;
>> +
>> /* Command register, clear upper bits, including extended modes */
>> cmd = pci_get_word(pci_dev->config + pos + PCI_X_CMD);
>> cmd &= (PCI_X_CMD_DPERR_E | PCI_X_CMD_ERO | PCI_X_CMD_MAX_READ |
>> @@ -1534,6 +1389,13 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>> if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8)) < 0) {
>> return ret;
>> }
>> +
>> + /* direct read except for next cap pointer */
>> + memset(dev->direct_config_read + pos, 0xff, 8);
>> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] = 0;
>> +
>> + /* direct write for cap content */
>> + memset(dev->direct_config_write + pos + 2, 0xff, 6);
>> }
>>
>> /* Devices can have multiple vendor capabilities, get them all */
>> @@ -1545,6 +1407,21 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>> pos, len)) < 0) {
>> return ret;
>> }
>> +
>> + /* direct read except for next cap pointer */
>> + memset(dev->direct_config_read + pos, 0xff, len);
>> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] = 0;
>> +
>> + /* direct write for cap content */
>> + memset(dev->direct_config_write + pos + 2, 0xff, len - 2);
>> + }
>> +
>> + /* If real and virtual capability list status bits differ, virtualize the
>> + * access. */
>> + if ((pci_get_word(pci_dev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST) !=
>> + (assigned_dev_pci_read_byte(pci_dev, PCI_STATUS) &
>> + PCI_STATUS_CAP_LIST)) {
>> + dev->direct_config_read[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
>> }
>>
>> return 0;
>> @@ -1694,6 +1571,26 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
>> return -1;
>> }
>>
>> + /*
>> + * Set up basic config space access control. Will be further refined during
>> + * device initialization.
>> + *
>> + * Direct read/write access is grangted for:
>> + * - status & command registers, revision ID & class code, cacheline size,
>> + * latency timer, header type, BIST
>> + * - cardbus CIS pointer, subsystem vendor & ID
>> + * - reserved fields
>> + * - Min_Gnt & Max_Lat
>> + */
>> + memset(dev->direct_config_read + PCI_COMMAND, 0xff, 12);
>
> This is more efficient, but maybe we should memset each field for
> grep-ability.
Will play with that. May save some comments.
>
> Nice change overall. Thanks,
Thanks,
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
