Re: [Qemu-devel] [RFC PATCH 0/8] Add Generic PCI host device update

[Claudio Fontana]

Hi Alvise,

On 11.07.2014 09:21, Alvise Rigo wrote:
> This patch series is based on the previous work [1] and [2] by Rob
> Herring and it tries to enhance this work on these points:

do your patches need to be applied on top of Rob's?

I ask because I cannot see the patch "hw/arm/virt: Add generic PCI host device" 
and among these,
as wll as the "hw/pci-host: add a generic PCI host".

I think those two need modifications as well, as they hardcode addresses, and 
also try to register
the PCI bus as a PCIE bus: does it really provide a PCI-Express? (probably 
harmless but still would benefit from review).

> 
> - Some of the hardcoded values have been moved to an header file.  This
>   header file is also used to share some device structures with the
>   mach-virt machine.

Some additional hardcoded addresses have been also introduced with your changes 
though.
(I'll post comments to the the patches in the series momentarily).

> - The interrupt-map dt node generation has been revisited; it is now
>   done after the generic devices init so that it's possible to attach
>   PCI devices by mean of the qdev infrastructure. This allows to have
>   several devices in the PCI bus, with the current limitation of one
>   interrupt per PCI slot.
> 
> Probably the most objectionable part of these patches regards the way
> some data and definitions have been shared between the machine and the
> device code; a better solution is still under evaluation. Any advice on
> this and on the rest of the work is highly appreciated.
> 
> This work has been tested attaching several PCI devices to the mach-virt
> platform.  The tested devices are: virtio-blk-pci, virtio-net-pci,
> lsi53c895a and pci-ohci (all attached at the same time).
> Even if the original work was not changed in its core functionalities, I
> couldn't reproduce the malfunctioning of the LSI SCSI mentioned in [1].
> After attaching several qcow2 images, formatting and filling them, I
> didn't notice anything wrong. Am I missing something?
> 
> Thank you,
> alvise
> 
> [1]
> "[Qemu-devel] [RFC PATCH 1/2] hw/pci-host: add a generic PCI host"
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
> [2]
> "[Qemu-devel] [RFC PATCH 2/2] hw/arm/virt: Add generic PCI host device"
> http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03483.html
> 
> Alvise Rigo (8):
>   mach-virt: move GIC inside mach-virt structure
>   mach-virt: improve PCI memory topology definition
>   QEMUMachine: finalize_dt function
>   generic_pci: create header file
>   generic_pci: create own map irq function
>   generic_pci: generate dt node after devices init
>   generic_pci: realize device with machine data
>   generic_pci: add interrupt map structures
> 
>  hw/arm/virt.c                     | 110 +++++++++++++++---------
>  hw/pci-host/generic-pci.c         | 173 
> +++++++++++++++++++++++++++++---------
>  include/hw/boards.h               |   4 +
>  include/hw/pci-host/pci_generic.h |  66 +++++++++++++++
>  vl.c                              |   5 ++
>  5 files changed, 277 insertions(+), 81 deletions(-)
>  create mode 100644 include/hw/pci-host/pci_generic.h
>