[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 7/7] libqos: add VIRTIO PCI 1.0 support
From: |
Thomas Huth |
Subject: |
Re: [PATCH v2 7/7] libqos: add VIRTIO PCI 1.0 support |
Date: |
Fri, 18 Oct 2019 08:51:09 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 |
On 18/10/2019 08.48, Thomas Huth wrote:
> On 17/10/2019 18.18, Thomas Huth wrote:
>> On 17/10/2019 18.07, Stefan Hajnoczi wrote:
>>> On Thu, Oct 17, 2019 at 04:52:54PM +0200, Thomas Huth wrote:
>>>> On 11/10/2019 10.56, Stefan Hajnoczi wrote:
>>>>> Implement the VIRTIO 1.0 virtio-pci interface. The main change here is
>>>>> that the register layout is no longer a fixed layout in BAR 0. Instead
>>>>> we have to iterate of PCI Capabilities to find descriptions of where
>>>>> various registers are located. The vring registers are also more
>>>>> fine-grained, allowing for more flexible vring layouts, but we don't
>>>>> take advantage of that.
>>>>>
>>>>> Note that test cases do not negotiate VIRTIO_F_VERSION_1 yet and are
>>>>> therefore not running in VIRTIO 1.0 mode.
>>>>>
>>>>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>>>>> ---
>>>>> tests/Makefile.include | 1 +
>>>>> tests/libqos/virtio-pci-modern.h | 17 ++
>>>>> tests/libqos/virtio-pci.h | 10 +
>>>>> tests/libqos/virtio-pci-modern.c | 412 +++++++++++++++++++++++++++++++
>>>>> tests/libqos/virtio-pci.c | 6 +-
>>>>> 5 files changed, 445 insertions(+), 1 deletion(-)
>>>>> create mode 100644 tests/libqos/virtio-pci-modern.h
>>>>> create mode 100644 tests/libqos/virtio-pci-modern.c
>>>> [...]
>>>>> +static bool probe_device_type(QVirtioPCIDevice *dev)
>>>>> +{
>>>>> + uint16_t vendor_id;
>>>>> + uint16_t device_id;
>>>>> +
>>>>> + /* "Drivers MUST match devices with the PCI Vendor ID 0x1AF4" */
>>>>> + vendor_id = qpci_config_readw(dev->pdev, PCI_VENDOR_ID);
>>>>> + if (vendor_id != 0x1af4) {
>>>>> + return false;
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * "Any PCI device with ... PCI Device ID 0x1000 through 0x107F
>>>>> inclusive
>>>>> + * is a virtio device"
>>>>> + */
>>>>> + device_id = qpci_config_readw(dev->pdev, PCI_DEVICE_ID);
>>>>> + if (device_id < 0x1000 || device_id > 0x107f) {
>>>>> + return false;
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * "Devices MAY utilize a Transitional PCI Device ID range, 0x1000 to
>>>>> + * 0x103F depending on the device type"
>>>>> + */
>>>>> + if (device_id < 0x1040) {
>>>>> + /*
>>>>> + * "Transitional devices MUST have the PCI Subsystem Device ID
>>>>> matching
>>>>> + * the Virtio Device ID"
>>>>> + */
>>>>> + dev->vdev.device_type = qpci_config_readw(dev->pdev,
>>>>> PCI_SUBSYSTEM_ID);
>>>>
>>>> Shouldn't you return "false" here in case the device_type is 0 ? Which
>>>> likely means that it is a legacy or broken device ...?
>>>
>>> The real decision whether to use this PCI device or not happens in
>>> probe_device_layout(). If it's broken or a legacy device then that
>>> function will fail.
>>
>> Ok, fair.
>>
>> I've added the patches to my qtest-next branch:
>>
>> https://gitlab.com/huth/qemu/tree/qtest-next
>
> Hi Stephan,
>
> looks like this is breaking the virtio-blk-test in certain configurations:
>
> https://gitlab.com/huth/qemu/-/jobs/324085741
>
> and:
>
> https://cirrus-ci.com/task/4511314474434560
>
> Could you please have a look?
FWIW, the
assertion failed (capacity == TEST_IMAGE_SIZE / 512): (2199023255552 ==
131072)
looks like an endianess bug to me. 2199023255552 is 131072 byte-swapped.
Thomas
signature.asc
Description: OpenPGP digital signature
[PATCH v2 7/7] libqos: add VIRTIO PCI 1.0 support, Stefan Hajnoczi, 2019/10/11