On 01/12/2017 05:56 PM, Michael S. Tsirkin wrote:
On Wed, Jan 11, 2017 at 02:18:53PM +0200, Marcel Apfelbaum wrote:
v1 -> v2:
- Rebased on master.
The Generic Root Port behaves the same as the
Intel's IOH device with id 3420, without having
Intel specific attributes.
The device has two purposes:
(1) Can be used on both X86 and ARM machines.
(2) It will allow us to tweak the behaviour
(e.g add vendor-specific PCI capabilities)
- something that obviously cannot be done
on a known device.
Patch 1/3: Introduce a base class for Root Ports - most of the code
is migrated from IOH3420 implementation.
Patch 2/3: Derives the IOH3420 from the new base class
Patch 3/3: Introduces the generic Root Port.
Tested with Linux and Windows guests only on x86 hosts.
So I understand how it's helpful to share code
with an existing bridge, but I worries me that
you picked up some arbitrary values from the intel chip
and promote them as a standard pcie thing.
I suppose you are referring to the macros, I can duplicate
them, of course.
You want to have e.g. base-pcie-root-port and
inherit that. Only put really common stuff in there.
Similar to how we do it in hw/pci/pci_bridge.c
So you prefer a different code file for the generic device.
I currently put them both in the same file, I'll split.
Now we do not want to over-engineer, so I understand
how you might want to say hey I use this option
and intel does the same so we do not need a flag
for that now. And that's fine but please add a comment
to that end.
Indeed, I wanted to keep things simple. I'll add a comment
on the functions that are not generic but are the same on
both implementations 'by chance'.
And when you do you will discover there's some stuff you
might be able to simplify, e.g. I don't yet see why
you would want AER support there.
I am not sure *why not*.
I am 'afraid' people will not use the new port
because of missing features and they can see the AER as one of them.
Do you see a special issue in imitating Intel's behavior on AER?