Re: [Qemu-devel] [PATCH] virtio-pci: Don't force Subsystem Vendor ID = Vendor ID

[Michael S. Tsirkin]

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 <address@hidden>

I wonder whether it's a problem that legacy devices ignore
the subsystem ID (that's part of spec).

> ---
>  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.

>          pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus));
>      } else {
>          /* pure virtio-1.0 */
> -- 
> 2.13.5