On Thu, Nov 2, 2017 at 3:52 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
> On Thu, Nov 02, 2017 at 02:31:15PM +0100, Ladi Prosek wrote:
>> The statement being removed doesn't change anything as virtio PCI devices
>> already
>> have Subsystem Vendor ID set to pci_default_sub_vendor_id (0x1af4), same as
>> Vendor
>> ID. And the Virtio spec does not require the two to be equal, either:
>>
>> "The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY reflect
>> the PCI
>> Vendor and Device ID of the environment (for informational purposes by the
>> driver)."
>>
>> Background:
>>
>> Following the recent virtio-win licensing change, several vendors are
>> planning to
>> ship their own certified version of Windows guest Virtio drivers,
>> potentially taking
>> advantage of Windows Update as a distribution channel. It is therefore
>> critical that
>> each vendor uses their own PCI Subsystem Vendor ID for Virtio devices to
>> prevent
>> drivers from other vendors binding to it.
>>
>> This would be trivially done by adding:
>>
>> k->subsystem_vendor_id = ...
>>
>> to virtio_pci_class_init(). Except for the problematic statement deleted by
>> this
>> patch, which reverts the Subsystem Vendor ID back to 0x1af4 for legacy
>> devices for
>> no good reason.
>>
>> Signed-off-by: Ladi Prosek <lpro...@redhat.com>
>
> I wonder whether it's a problem that legacy devices ignore
> the subsystem ID (that's part of spec).
I don't understand this comment. I don't see anything in the spec
related to ignoring the subsystem ID.
>> ---
>> hw/virtio/virtio-pci.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index e92837c42b..cb74aa3d85 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -1589,8 +1589,6 @@ static void virtio_pci_device_plugged(DeviceState *d,
>> Error **errp)
>> return ;
>> }
>> /* legacy and transitional */
>> - pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
>> - pci_get_word(config + PCI_VENDOR_ID));
>
>
> This happens to work because the default subsystem vendor id
> within qemu is 0x1af4. Let's add a comment to that end.
Sure, will do.
>> pci_set_word(config + PCI_SUBSYSTEM_ID,
>> virtio_bus_get_vdev_id(bus));
>> } else {
>> /* pure virtio-1.0 */
>> --
>> 2.13.5
