Re: [Qemu-block] [Qemu-devel] [PATCH V4] pci: removed the is_express field since a uniform interface was inserted

2017-12-18 Thread Eduardo Habkost
On Tue, Dec 12, 2017 at 07:36:49AM +0200, Yoni Bettan wrote:
> * according to Eduardo Habkost's commit
>   fd3b02c8896d597dd8b9e053dec579cf0386aee1
> 
> * since all PCIEs now implement INTERFACE_PCIE_DEVICE we
>   don't need this field anymore
> 
> * Devices that where only INTERFACE_PCIE_DEVICE (is_express == 1)
>   or
>   devices that where only INTERFACE_CONVENTIONAL_PCI_DEVICE 
> (is_express == 0)
>   where not affected by the change
> 
>   The only devices that were affected are those that are hybrid and 
> also
>   had (is_express == 1) - therefor only:
> - hw/vfio/pci.c
> - hw/usb/hcd-xhci.c

Oops, we now have xen-pt too.  See:

  From: Simon Gaiser 
  Date: Sat, 28 Oct 2017 04:53:15 +0200
  Message-Id: <20171028025315.13500-1-h...@ipsumj.de>
  Subject: [PATCH] xen/pt: Set is_express to avoid out-of-bounds write

Which was included in a pull request sent on last Thursday.

> 
>   For both I made sure that QEMU_PCI_CAP_EXPRESS is on
> 
> Signed-off-by: Yoni Bettan 
[...]

-- 
Eduardo



Re: [Qemu-block] [Qemu-devel] [PATCH V4] pci: removed the is_express field since a uniform interface was inserted

2017-12-18 Thread Eduardo Habkost
On Tue, Dec 12, 2017 at 07:36:49AM +0200, Yoni Bettan wrote:
> * according to Eduardo Habkost's commit
>   fd3b02c8896d597dd8b9e053dec579cf0386aee1
> 
> * since all PCIEs now implement INTERFACE_PCIE_DEVICE we
>   don't need this field anymore
> 
> * Devices that where only INTERFACE_PCIE_DEVICE (is_express == 1)
>   or
>   devices that where only INTERFACE_CONVENTIONAL_PCI_DEVICE 
> (is_express == 0)
>   where not affected by the change
> 
>   The only devices that were affected are those that are hybrid and 
> also
>   had (is_express == 1) - therefor only:
> - hw/vfio/pci.c
> - hw/usb/hcd-xhci.c
> 
>   For both I made sure that QEMU_PCI_CAP_EXPRESS is on
> 
> Signed-off-by: Yoni Bettan 

The code looks good.

Reviewed-by: Eduardo Habkost 

But I suggest reformatting the commit message before committing
this.


> ---
>  docs/pcie_pci_bridge.txt   | 2 +-
>  hw/block/nvme.c| 1 -
>  hw/net/e1000e.c| 1 -
>  hw/pci-bridge/pcie_pci_bridge.c| 1 -
>  hw/pci-bridge/pcie_root_port.c | 1 -
>  hw/pci-bridge/xio3130_downstream.c | 1 -
>  hw/pci-bridge/xio3130_upstream.c   | 1 -
>  hw/pci-host/xilinx-pcie.c  | 1 -
>  hw/pci/pci.c   | 8 ++--
>  hw/scsi/megasas.c  | 4 
>  hw/usb/hcd-xhci.c  | 9 -
>  hw/vfio/pci.c  | 5 -
>  include/hw/pci/pci.h   | 3 ---
>  13 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/docs/pcie_pci_bridge.txt b/docs/pcie_pci_bridge.txt
> index 5a4203f97c..ab35ebf3ca 100644
> --- a/docs/pcie_pci_bridge.txt
> +++ b/docs/pcie_pci_bridge.txt
> @@ -110,5 +110,5 @@ To enable device hot-plug into the bridge on Linux 
> there're 3 ways:
>  Implementation
>  ==
>  The PCIE-PCI bridge is based on PCI-PCI bridge, but also accumulates PCI 
> Express
> -features as a PCI Express device (is_express=1).
> +features as a PCI Express device.
>  
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 441e21ed1f..9325bc0911 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1087,7 +1087,6 @@ static void nvme_class_init(ObjectClass *oc, void *data)
>  pc->vendor_id = PCI_VENDOR_ID_INTEL;
>  pc->device_id = 0x5845;
>  pc->revision = 2;
> -pc->is_express = 1;
>  
>  set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>  dc->desc = "Non-Volatile Memory Express";
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index f1af279e8d..c360f0d8c9 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -675,7 +675,6 @@ static void e1000e_class_init(ObjectClass *class, void 
> *data)
>  c->revision = 0;
>  c->romfile = "efi-e1000e.rom";
>  c->class_id = PCI_CLASS_NETWORK_ETHERNET;
> -c->is_express = 1;
>  
>  dc->desc = "Intel 82574L GbE Controller";
>  dc->reset = e1000e_qdev_reset;
> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
> index a4d827c99d..b7d9ebbec2 100644
> --- a/hw/pci-bridge/pcie_pci_bridge.c
> +++ b/hw/pci-bridge/pcie_pci_bridge.c
> @@ -169,7 +169,6 @@ static void pcie_pci_bridge_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>  
> -k->is_express = 1;
>  k->is_bridge = 1;
>  k->vendor_id = PCI_VENDOR_ID_REDHAT;
>  k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE;
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> index 9b6e4ce512..45f9e8cd4a 100644
> --- a/hw/pci-bridge/pcie_root_port.c
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -145,7 +145,6 @@ static void rp_class_init(ObjectClass *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -k->is_express = 1;
>  k->is_bridge = 1;
>  k->config_write = rp_write_config;
>  k->realize = rp_realize;
> diff --git a/hw/pci-bridge/xio3130_downstream.c 
> b/hw/pci-bridge/xio3130_downstream.c
> index 1e09d2afb7..613a0d6bb7 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -177,7 +177,6 @@ static void xio3130_downstream_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>  
> -k->is_express = 1;
>  k->is_bridge = 1;
>  k->config_write = xio3130_downstream_write_config;
>  k->realize = xio3130_downstream_realize;
> diff --git a/hw/pci-bridge/xio3130_upstream.c 
> b/hw/pci-bridge/xio3130_upstream.c
> index 227997ce46..d4645bddee 100644
> --- a/hw/pci-bridge/xio3130_upstream.c
> +++ b/hw/pci-bridge/xio3130_upstream.c
> @@ -148,7 +148,6 @@ static void xio3130_upstream_class_init(ObjectClass 
> *klass, void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PCIDeviceClass *k = PCI_DEVIC