Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-19 Thread Andrea Bolognani
On Thu, 2017-02-16 at 21:14 +0200, Marcel Apfelbaum wrote:
>  > Wait, actually.. we have two possible directions to go, both of which
>  > have been mentioned in the thread, but I don't think we've settled on
>  > one:
>  >
>  > 1) Have pseries create a PCIe bus (as my first cut draft does).
>  >
>  > That should allow pure PCIe devices to appear either under a port or
>  > (more usually for PAPR) as "integrated endpoints".  In addition we'd
>  > need as suggested above a "pcie_hybrid_type()" function that would
>  > tell hybrid devices to also appear as PCIe rather than PCI.
>  >
>  > 2) Have pseries create a vanilla PCI bus (or a special PAPR PCI
>  >variant)
>  >
>  > Appearing as vanilla PCI would in a number of ways more closely match
>  > the way PCI buses are handled on PAPR.  However, we still need to
>  > connect PCIe devices to it.  So we'd need some 'bus_accepts_pcie()'
>  > hook and use that (in place of pci_bus_is_express()) to determine both
>  > whether we can attach pure PCIe devices and that hybrid devices should
>  > appear as PCIe rather than plain PCI.
>  >
>  >
>  > Based on the immediately preceding discussion, I was leaning towards
>  > (2).  Is that your feeling as well?
> 
> I also like option (2).

After catching up with the thread, I tend to agree.

I've also been thinking about what libvirt would need to
do to adapt to the changes proposed here, and I believe the
answer to be: not much, really.

The current code already places VirtIO devices on the root
bus, even though it does so because it assumes pSeries
guests are not PCIe capable; same for XHCI, and pretty much
all other devices including those assigned through VFIO.

Basically, as far as I can tell, the changes proposed here
would only affect the guest (eg. ability to access the
extended config space), not how the devices would have to
be placed on the various buses. Or did I miss something?

-- 
Andrea Bolognani / Red Hat / Virtualization



Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-16 Thread Marcel Apfelbaum

On 02/16/2017 05:28 AM, David Gibson wrote:

On Thu, Feb 16, 2017 at 01:48:42PM +1100, David Gibson wrote:

On Wed, Feb 15, 2017 at 04:59:33PM +0200, Marcel Apfelbaum wrote:

On 02/15/2017 03:45 AM, David Gibson wrote:

On Tue, Feb 14, 2017 at 02:53:08PM +0200, Marcel Apfelbaum wrote:

On 02/14/2017 06:15 AM, David Gibson wrote:

On Mon, Feb 13, 2017 at 12:14:23PM +0200, Marcel Apfelbaum wrote:

On 02/13/2017 06:33 AM, David Gibson wrote:

On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:

On 02/10/2017 02:37 AM, David Gibson wrote:

On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:

On 02/09/17 05:16, David Gibson wrote:

On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:

On 02/08/17 07:16, David Gibson wrote:

[snip]

  Which means that you can use it to

drive PCIe devices just fine.  "Bus level" PCIe extensions like AER
and PCIe standard hotplug won't work, but PAPR has its own mechanisms
for those (common between PCI and PCIe).

I did float the idea of having the pseries PCI bus remain plain PCI
but with a special flag to allow PCIe devices to be attached to it
anyway.  It wasn't greeted with much enthusiasm..



Can you point me to the discussion please? It seems similar to what I proposed 
above.


Sorry, I was misleading.  I think I just raised that idea with Andrea
and a few other people internally, not on one of the lists at large.


As you properly described it, is much closer to PCI then PCIe, even the only 
characteristic
that makes it "a little" PCIe, the Extended Configuration Space support,
is done with an alternative interface.

I agree the PAPR bus is not PCIe.


Ok, so if we take that direction, the question becomes how do we let
PCIe devices plug into this mostly-not-PCIe bus.  Maybe introduce a
"pci_bus_accepts_express()" function that will replace many, but not
all current uses of "pci_bus_is_express()"?



Sounds good and I think Eduardo is already working on exactly this
idea, however he is on PTO now. It is better to synchronize with him.


Ah, right.  Do you know when he'll be back?  This is semi-urgent for
Power.



Such a helper could maybe simplify the logic in virtio-pci (and XHCI?)
by returning false on an x86 root bus.



The rule would me more complicated. We don't want to completely remove the
possibility to have PCIe devices as part of Root Complex. it seems
like I am contradicting myself, but no).
This is why we have guidelines and  not hard-coded policies.
Also ,the QEMU way is to be more permissive. We provide guidelines and sane
defaults, but we let the user to chose.

Getting back to our problem, the rule would be:
hybrid devices should be PCI or PCIe for a bus?
PAPR bus should return 'PCIe' for hybrid devices.
X86 bus should return 'PCIe' if not root.


Ok.


Wait, actually.. we have two possible directions to go, both of which
have been mentioned in the thread, but I don't think we've settled on
one:

1) Have pseries create a PCIe bus (as my first cut draft does).

That should allow pure PCIe devices to appear either under a port or
(more usually for PAPR) as "integrated endpoints".  In addition we'd
need as suggested above a "pcie_hybrid_type()" function that would
tell hybrid devices to also appear as PCIe rather than PCI.

2) Have pseries create a vanilla PCI bus (or a special PAPR PCI
   variant)

Appearing as vanilla PCI would in a number of ways more closely match
the way PCI buses are handled on PAPR.  However, we still need to
connect PCIe devices to it.  So we'd need some 'bus_accepts_pcie()'
hook and use that (in place of pci_bus_is_express()) to determine both
whether we can attach pure PCIe devices and that hybrid devices should
appear as PCIe rather than plain PCI.


Based on the immediately preceding discussion, I was leaning towards
(2).  Is that your feeling as well?



On 02/16/2017 05:28 AM, David Gibson wrote:
> On Thu, Feb 16, 2017 at 01:48:42PM +1100, David Gibson wrote:
>> On Wed, Feb 15, 2017 at 04:59:33PM +0200, Marcel Apfelbaum wrote:
>>> On 02/15/2017 03:45 AM, David Gibson wrote:
 On Tue, Feb 14, 2017 at 02:53:08PM +0200, Marcel Apfelbaum wrote:
> On 02/14/2017 06:15 AM, David Gibson wrote:
>> On Mon, Feb 13, 2017 at 12:14:23PM +0200, Marcel Apfelbaum wrote:
>>> On 02/13/2017 06:33 AM, David Gibson wrote:
 On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:
> On 02/10/2017 02:37 AM, David Gibson wrote:
>> On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:
>>> On 02/09/17 05:16, David Gibson wrote:
 On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:
> On 02/08/17 07:16, David Gibson wrote:
> [snip]
>   Which means that you can use it to
>> drive PCIe devices just fine.  "Bus level" PCIe extensions like AER
>> and PCIe standard hotplug won't work, but PAPR has its own mechanisms
>> for those (common between PCI and PCIe).
>>
>> I did float the idea of having the pseries 

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-16 Thread Marcel Apfelbaum

On 02/16/2017 04:48 AM, David Gibson wrote:

On Wed, Feb 15, 2017 at 04:59:33PM +0200, Marcel Apfelbaum wrote:


[...]



I did float the idea of having the pseries PCI bus remain plain PCI
but with a special flag to allow PCIe devices to be attached to it
anyway.  It wasn't greeted with much enthusiasm..



Can you point me to the discussion please? It seems similar to what I proposed 
above.


Sorry, I was misleading.  I think I just raised that idea with Andrea
and a few other people internally, not on one of the lists at large.


As you properly described it, is much closer to PCI then PCIe, even the only 
characteristic
that makes it "a little" PCIe, the Extended Configuration Space support,
is done with an alternative interface.

I agree the PAPR bus is not PCIe.


Ok, so if we take that direction, the question becomes how do we let
PCIe devices plug into this mostly-not-PCIe bus.  Maybe introduce a
"pci_bus_accepts_express()" function that will replace many, but not
all current uses of "pci_bus_is_express()"?



Sounds good and I think Eduardo is already working on exactly this
idea, however he is on PTO now. It is better to synchronize with him.


Ah, right.  Do you know when he'll be back?  This is semi-urgent for
Power.



In a week or so.





[...]



Ok.. I'm still missing something.  Are you saying that instead of the
legacy/modern status determining PCIe support, that PCIe status is
determining legacy/modern support by default?


Is more a guideline. We want QEMU to behave like this *by default*.
The modern spec does not require virtio 1.0 devices to be PCIe.


Ok.  But I'm trying to get a handle on how - even by default - the
linkage between PCI/PCIe and modern/legacy is created:  it's not
obvious to me.



There is no direct connection between modern/legacy <-> PCI/PCIe.
However the defaults work like that:
I440FX - (pc): devices are PCI, transitional
   created by: disable-legacy=auto,disable-modern=off
   - in virtio_pci_realize disable-legacy: auto => off
Q35 - on root bus - devices are PCI, transitional
- other buses - devices are PCIe, modern
   created by: disable-legacy=auto,disable-modern=off
   - in virtio_pci_dc_realize:
if disable-modern=off -> pcie
   - in virtio_pci_realize:
if on pcie root port -> disable-legacy: auto => on
otherwise ->disable-legacy: auto => off



  That would actually

seem to simplify things: whatever method we end up allowing PCIe
virtio on PAPR, that should automatically enable modern mode, which is
fine.



Agreed.


Although.. I do wonder about legacy/modern for PAPR in general.
Current kernels will support virtio 1.0 fine, but we have older
released versions which only support legacy.


So, do you want legacy virtio devices to be PCIe ?


TBH, we probably don't care one way or another.

I'm not clear here if you're making a distinction between legacy-only
and legacy+modern virtio instances.



Legacy devices and the modern ones are different devices (product ID).
I think the 'transitional' ones have the ID of the modern virtio.


I think we need to keep legacy support on for Power machines, because
we don't have the machine type switchover to let us handle it.  But we
certainly would like to enable modern support so that newer guests can
exploit it.



"transitional" is the right way to go.

Thanks,
Marcel


  Unlike PC we won't (I

hope) have a whole machine type switch to handle this.


Good for you!

  At this stage

I think we want virtio devices (whatever their bus type) on PAPR to
default to allowing both legacy and modern for the forseeable future.
How does that affect the matrix?



It adds a legacy virtio PCIe device to the matrix. For X86 we don't need
this combination because PC machines are PCI and Q35 machines are to be used
only on modern setups.

Since PARP does not have separate machines for PCi and PCIe, we need to live 
with it.



XHCI being PCIe on Root Complex is an unintended exception, but we want it 
connected to a
Root Port anyway, we don't have anything to gain from having it as Integrated 
Endpoint.
We only loose a slot that can be used by 8 Root Ports assembled into one 
multi-function device.

PAPR bus should not be considered PCIe and should have a different set of rules 
allowing PCIe
devices to be plugged into Root Complex.


Alright, I can work with that.  Michael, Andrea, how does this idea
seem to you?



We should definitely get more opinions.
Thanks,
Marcel








Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-15 Thread David Gibson
On Thu, Feb 16, 2017 at 01:48:42PM +1100, David Gibson wrote:
> On Wed, Feb 15, 2017 at 04:59:33PM +0200, Marcel Apfelbaum wrote:
> > On 02/15/2017 03:45 AM, David Gibson wrote:
> > > On Tue, Feb 14, 2017 at 02:53:08PM +0200, Marcel Apfelbaum wrote:
> > > > On 02/14/2017 06:15 AM, David Gibson wrote:
> > > > > On Mon, Feb 13, 2017 at 12:14:23PM +0200, Marcel Apfelbaum wrote:
> > > > > > On 02/13/2017 06:33 AM, David Gibson wrote:
> > > > > > > On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:
> > > > > > > > On 02/10/2017 02:37 AM, David Gibson wrote:
> > > > > > > > > On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:
> > > > > > > > > > On 02/09/17 05:16, David Gibson wrote:
> > > > > > > > > > > On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek 
> > > > > > > > > > > wrote:
> > > > > > > > > > > > On 02/08/17 07:16, David Gibson wrote:
[snip]
> > > >   Which means that you can use it to
> > > > > drive PCIe devices just fine.  "Bus level" PCIe extensions like AER
> > > > > and PCIe standard hotplug won't work, but PAPR has its own mechanisms
> > > > > for those (common between PCI and PCIe).
> > > > > 
> > > > > I did float the idea of having the pseries PCI bus remain plain PCI
> > > > > but with a special flag to allow PCIe devices to be attached to it
> > > > > anyway.  It wasn't greeted with much enthusiasm..
> > > > > 
> > > > 
> > > > Can you point me to the discussion please? It seems similar to what I 
> > > > proposed above.
> > > 
> > > Sorry, I was misleading.  I think I just raised that idea with Andrea
> > > and a few other people internally, not on one of the lists at large.
> > > 
> > > > As you properly described it, is much closer to PCI then PCIe, even the 
> > > > only characteristic
> > > > that makes it "a little" PCIe, the Extended Configuration Space support,
> > > > is done with an alternative interface.
> > > > 
> > > > I agree the PAPR bus is not PCIe.
> > > 
> > > Ok, so if we take that direction, the question becomes how do we let
> > > PCIe devices plug into this mostly-not-PCIe bus.  Maybe introduce a
> > > "pci_bus_accepts_express()" function that will replace many, but not
> > > all current uses of "pci_bus_is_express()"?
> > > 
> > 
> > Sounds good and I think Eduardo is already working on exactly this
> > idea, however he is on PTO now. It is better to synchronize with him.
> 
> Ah, right.  Do you know when he'll be back?  This is semi-urgent for
> Power.
> 
> 
> > > Such a helper could maybe simplify the logic in virtio-pci (and XHCI?)
> > > by returning false on an x86 root bus.
> > > 
> > 
> > The rule would me more complicated. We don't want to completely remove the
> > possibility to have PCIe devices as part of Root Complex. it seems
> > like I am contradicting myself, but no).
> > This is why we have guidelines and  not hard-coded policies.
> > Also ,the QEMU way is to be more permissive. We provide guidelines and sane
> > defaults, but we let the user to chose.
> > 
> > Getting back to our problem, the rule would be:
> > hybrid devices should be PCI or PCIe for a bus?
> > PAPR bus should return 'PCIe' for hybrid devices.
> > X86 bus should return 'PCIe' if not root.
> 
> Ok.

Wait, actually.. we have two possible directions to go, both of which
have been mentioned in the thread, but I don't think we've settled on
one:

1) Have pseries create a PCIe bus (as my first cut draft does).

That should allow pure PCIe devices to appear either under a port or
(more usually for PAPR) as "integrated endpoints".  In addition we'd
need as suggested above a "pcie_hybrid_type()" function that would
tell hybrid devices to also appear as PCIe rather than PCI.

2) Have pseries create a vanilla PCI bus (or a special PAPR PCI
   variant)

Appearing as vanilla PCI would in a number of ways more closely match
the way PCI buses are handled on PAPR.  However, we still need to
connect PCIe devices to it.  So we'd need some 'bus_accepts_pcie()'
hook and use that (in place of pci_bus_is_express()) to determine both
whether we can attach pure PCIe devices and that hybrid devices should
appear as PCIe rather than plain PCI.


Based on the immediately preceding discussion, I was leaning towards
(2).  Is that your feeling as well?

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-15 Thread David Gibson
On Wed, Feb 15, 2017 at 04:59:33PM +0200, Marcel Apfelbaum wrote:
> On 02/15/2017 03:45 AM, David Gibson wrote:
> > On Tue, Feb 14, 2017 at 02:53:08PM +0200, Marcel Apfelbaum wrote:
> > > On 02/14/2017 06:15 AM, David Gibson wrote:
> > > > On Mon, Feb 13, 2017 at 12:14:23PM +0200, Marcel Apfelbaum wrote:
> > > > > On 02/13/2017 06:33 AM, David Gibson wrote:
> > > > > > On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:
> > > > > > > On 02/10/2017 02:37 AM, David Gibson wrote:
> > > > > > > > On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:
> > > > > > > > > On 02/09/17 05:16, David Gibson wrote:
> > > > > > > > > > On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek 
> > > > > > > > > > wrote:
> > > > > > > > > > > On 02/08/17 07:16, David Gibson wrote:
> > > > > > > > > > > > Marcel,
> > > > > > > > > > > > 
> > > > > > > > > > > > Your original patch adding PCIe support to virtio-pci.c 
> > > > > > > > > > > > has the
> > > > > > > > > > > > limitation noted below that PCIe won't be enabled if 
> > > > > > > > > > > > the device is on
> > > > > > > > > > > > the root bus (rather than under a root or downstream 
> > > > > > > > > > > > port).  As
> > > > > > > > > > > > reasoned below, I think removing the check is correct, 
> > > > > > > > > > > > even for x86
> > > > > > > > > > > > (though it would rarely be useful there).  But I could 
> > > > > > > > > > > > well have
> > > > > > > > > > > > missed something.  Let me know if so...
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > Virtio devices can appear as either vanilla PCI or 
> > > > > > > > > > > > PCI-Express devices
> > > > > > > > > > > > depending on the bus they're connected to.  At the 
> > > > > > > > > > > > moment it will only
> > > > > > > > > > > > appear as vanilla PCI if connected to the root bus of a 
> > > > > > > > > > > > PCIe host bridge.
> > > > > > > > > > > > 
> > > > > > > > > > > > Presumably this is to reflect the fact that PCIe 
> > > > > > > > > > > > devices usually need to
> > > > > > > > > > > > be connected to a root (or further downstream) port 
> > > > > > > > > > > > rather than directly
> > > > > > > > > > > > on the root bus.  However, due to the odd requirements 
> > > > > > > > > > > > of the PAPR spec on the 'pseries'
> > > > > > > > > > > > machine type, it's normal for PCIe devices to appear on 
> > > > > > > > > > > > the root bus
> > > > > > > > > > > > without root ports.
> > > > > > > > > > > > 
> > > > > > > > > > > > Further, even on x86, there's no inherent reason we 
> > > > > > > > > > > > couldn't present a
> > > > > > > > > > > > virtio device as an "integrated device" (typically used 
> > > > > > > > > > > > for things built
> > > > > > > > > > > > into the PCI chipset), and those devices *do* typically 
> > > > > > > > > > > > appear on the root
> > > > > > > > > > > > bus.
> > > > > > > > > > > 
> > > > > > > > > > > I'm not personally making a counter-argument, just 
> > > > > > > > > > > qouting some of
> > > > > > > > > > > the relevant parts of "docs/pcie.txt" ("PCI EXPRESS 
> > > > > > > > > > > GUIDELINES"):
> > > > > > > > > > 
> > > > > > > > > > So, an earlier discussion more or less concluded that the 
> > > > > > > > > > PCIe
> > > > > > > > > > guidelines don't really work with PAPR guests.  That comes 
> > > > > > > > > > because
> > > > > > > > > > PAPR was designed with PowerVM in mind which allows PCI 
> > > > > > > > > > passthrough
> > > > > > > > > > but doesn't do any emulated PCI devices.  So they wanted to 
> > > > > > > > > > present
> > > > > > > > > > passed through devices (virtual or phyical) to the guest 
> > > > > > > > > > without
> > > > > > > > > > inserting virtual root ports.
> > > > > > > > > > 
> > > > > > > > > > Now, you can argue that this was a silly decision in PAPR, 
> > > > > > > > > > and you
> > > > > > > > > > could well be right, but there it is.
> > > > > > > > > 
> > > > > > > > > I can totally accept this, but then we should state it as a 
> > > > > > > > > fact near
> > > > > > > > > the top of "docs/pcie.txt".
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > > > Place only the following kinds of devices directly on 
> > > > > > > > > > > > the Root Complex:
> > > > > > > > > > > > (1) PCI Devices (e.g. network card, graphics card, 
> > > > > > > > > > > > IDE controller),
> > > > > > > > > > > > not controllers. Place only legacy PCI devices 
> > > > > > > > > > > > on
> > > > > > > > > > > > the Root Complex. These will be considered 
> > > > > > > > > > > > Integrated Endpoints.
> > > > > > > > > > > > Note: Integrated Endpoints are not 
> > > > > > > > > > > > hot-pluggable.
> > > > > > > > > > > > 
> > > > > > > > > > > > Although the PCI Express spec does not forbid 
> > > > > > > > > > > > PCI Express devices as
> > > > > > > > > > > > Integrated Endpoints, existing hardware mostly 
> > > > > > > > > 

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-15 Thread Marcel Apfelbaum

On 02/15/2017 03:45 AM, David Gibson wrote:

On Tue, Feb 14, 2017 at 02:53:08PM +0200, Marcel Apfelbaum wrote:

On 02/14/2017 06:15 AM, David Gibson wrote:

On Mon, Feb 13, 2017 at 12:14:23PM +0200, Marcel Apfelbaum wrote:

On 02/13/2017 06:33 AM, David Gibson wrote:

On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:

On 02/10/2017 02:37 AM, David Gibson wrote:

On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:

On 02/09/17 05:16, David Gibson wrote:

On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:

On 02/08/17 07:16, David Gibson wrote:

Marcel,

Your original patch adding PCIe support to virtio-pci.c has the
limitation noted below that PCIe won't be enabled if the device is on
the root bus (rather than under a root or downstream port).  As
reasoned below, I think removing the check is correct, even for x86
(though it would rarely be useful there).  But I could well have
missed something.  Let me know if so...



Virtio devices can appear as either vanilla PCI or PCI-Express devices
depending on the bus they're connected to.  At the moment it will only
appear as vanilla PCI if connected to the root bus of a PCIe host bridge.

Presumably this is to reflect the fact that PCIe devices usually need to
be connected to a root (or further downstream) port rather than directly
on the root bus.  However, due to the odd requirements of the PAPR spec on the 
'pseries'
machine type, it's normal for PCIe devices to appear on the root bus
without root ports.

Further, even on x86, there's no inherent reason we couldn't present a
virtio device as an "integrated device" (typically used for things built
into the PCI chipset), and those devices *do* typically appear on the root
bus.


I'm not personally making a counter-argument, just qouting some of
the relevant parts of "docs/pcie.txt" ("PCI EXPRESS GUIDELINES"):


So, an earlier discussion more or less concluded that the PCIe
guidelines don't really work with PAPR guests.  That comes because
PAPR was designed with PowerVM in mind which allows PCI passthrough
but doesn't do any emulated PCI devices.  So they wanted to present
passed through devices (virtual or phyical) to the guest without
inserting virtual root ports.

Now, you can argue that this was a silly decision in PAPR, and you
could well be right, but there it is.


I can totally accept this, but then we should state it as a fact near
the top of "docs/pcie.txt".




Place only the following kinds of devices directly on the Root Complex:
(1) PCI Devices (e.g. network card, graphics card, IDE controller),
not controllers. Place only legacy PCI devices on
the Root Complex. These will be considered Integrated Endpoints.
Note: Integrated Endpoints are not hot-pluggable.

Although the PCI Express spec does not forbid PCI Express devices as
Integrated Endpoints, existing hardware mostly integrates legacy PCI
devices with the Root Complex.


"Mostly".. on my laptop at least the GPU shows up as an integrated PCI
Express endpoint, so it's certainly not the case that *all* root bus
devices are legacy.


Guest OSes are suspected to behave

strangely when PCI Express devices are integrated
with the Root Complex.


Clearly not that strangely, that often, since my laptop works just fine.



[...]

2.2 PCI Express only hierarchy
==
Always use PCI Express Root Ports to start PCI Express hierarchies.


Above you mention "it's normal for PCIe devices to appear on the root bus without 
root ports".


Well "normal" perhaps wasn't the right word.  Let's say precedented,
if uncommon.


Let me turn the question around: is it a *problem* for "pseries" if
we require root ports? If so, why exactly?


That's.. a complex question.  At least Linux guests (and we don't
support any others yet) might cope with the addition of root ports.
Maybe.  I have discussed this option with BenH and others.

However it is gratuitously different from how PCIe devices will
typically appear for the same guest running under PowerVM.  Although I
suspect Linux would cope with the "normal standard" rather than "PAPR
standard" presentation, I'm not as confident about it as I would like.

Another consideration here is that other PCIe capable qemu emulated
devices, such as XHCI, will present fine as PCIe integrated endpoints
when attached to the root bus.  Libvirt won't do that usually, of
course, and it may not be the recommended way of doing things (on PC)
but it's possible.  I don't see any particular reason that virtio-pci
should enforce the root port requirement more so than any other
device.


On 02/08/17 07:16, David Gibson wrote:


pcie_endpoint_cap_init() already automatically adjusts to advertise as
an integrated device rather than a "normal" PCIe endpoint when attached to
a root bus.  So we can remove the check for root bus within virtio and
allow (at the user's discretion) a PCIe virtio bus to be attached to a

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-14 Thread David Gibson
On Tue, Feb 14, 2017 at 02:53:08PM +0200, Marcel Apfelbaum wrote:
> On 02/14/2017 06:15 AM, David Gibson wrote:
> > On Mon, Feb 13, 2017 at 12:14:23PM +0200, Marcel Apfelbaum wrote:
> > > On 02/13/2017 06:33 AM, David Gibson wrote:
> > > > On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:
> > > > > On 02/10/2017 02:37 AM, David Gibson wrote:
> > > > > > On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:
> > > > > > > On 02/09/17 05:16, David Gibson wrote:
> > > > > > > > On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:
> > > > > > > > > On 02/08/17 07:16, David Gibson wrote:
> > > > > > > > > > Marcel,
> > > > > > > > > > 
> > > > > > > > > > Your original patch adding PCIe support to virtio-pci.c has 
> > > > > > > > > > the
> > > > > > > > > > limitation noted below that PCIe won't be enabled if the 
> > > > > > > > > > device is on
> > > > > > > > > > the root bus (rather than under a root or downstream port). 
> > > > > > > > > >  As
> > > > > > > > > > reasoned below, I think removing the check is correct, even 
> > > > > > > > > > for x86
> > > > > > > > > > (though it would rarely be useful there).  But I could well 
> > > > > > > > > > have
> > > > > > > > > > missed something.  Let me know if so...
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Virtio devices can appear as either vanilla PCI or 
> > > > > > > > > > PCI-Express devices
> > > > > > > > > > depending on the bus they're connected to.  At the moment 
> > > > > > > > > > it will only
> > > > > > > > > > appear as vanilla PCI if connected to the root bus of a 
> > > > > > > > > > PCIe host bridge.
> > > > > > > > > > 
> > > > > > > > > > Presumably this is to reflect the fact that PCIe devices 
> > > > > > > > > > usually need to
> > > > > > > > > > be connected to a root (or further downstream) port rather 
> > > > > > > > > > than directly
> > > > > > > > > > on the root bus.  However, due to the odd requirements of 
> > > > > > > > > > the PAPR spec on the 'pseries'
> > > > > > > > > > machine type, it's normal for PCIe devices to appear on the 
> > > > > > > > > > root bus
> > > > > > > > > > without root ports.
> > > > > > > > > > 
> > > > > > > > > > Further, even on x86, there's no inherent reason we 
> > > > > > > > > > couldn't present a
> > > > > > > > > > virtio device as an "integrated device" (typically used for 
> > > > > > > > > > things built
> > > > > > > > > > into the PCI chipset), and those devices *do* typically 
> > > > > > > > > > appear on the root
> > > > > > > > > > bus.
> > > > > > > > > 
> > > > > > > > > I'm not personally making a counter-argument, just qouting 
> > > > > > > > > some of
> > > > > > > > > the relevant parts of "docs/pcie.txt" ("PCI EXPRESS 
> > > > > > > > > GUIDELINES"):
> > > > > > > > 
> > > > > > > > So, an earlier discussion more or less concluded that the PCIe
> > > > > > > > guidelines don't really work with PAPR guests.  That comes 
> > > > > > > > because
> > > > > > > > PAPR was designed with PowerVM in mind which allows PCI 
> > > > > > > > passthrough
> > > > > > > > but doesn't do any emulated PCI devices.  So they wanted to 
> > > > > > > > present
> > > > > > > > passed through devices (virtual or phyical) to the guest without
> > > > > > > > inserting virtual root ports.
> > > > > > > > 
> > > > > > > > Now, you can argue that this was a silly decision in PAPR, and 
> > > > > > > > you
> > > > > > > > could well be right, but there it is.
> > > > > > > 
> > > > > > > I can totally accept this, but then we should state it as a fact 
> > > > > > > near
> > > > > > > the top of "docs/pcie.txt".
> > > > > > > 
> > > > > > > > 
> > > > > > > > > > Place only the following kinds of devices directly on the 
> > > > > > > > > > Root Complex:
> > > > > > > > > > (1) PCI Devices (e.g. network card, graphics card, IDE 
> > > > > > > > > > controller),
> > > > > > > > > > not controllers. Place only legacy PCI devices on
> > > > > > > > > > the Root Complex. These will be considered 
> > > > > > > > > > Integrated Endpoints.
> > > > > > > > > > Note: Integrated Endpoints are not hot-pluggable.
> > > > > > > > > > 
> > > > > > > > > > Although the PCI Express spec does not forbid PCI 
> > > > > > > > > > Express devices as
> > > > > > > > > > Integrated Endpoints, existing hardware mostly 
> > > > > > > > > > integrates legacy PCI
> > > > > > > > > > devices with the Root Complex.
> > > > > > > > 
> > > > > > > > "Mostly".. on my laptop at least the GPU shows up as an 
> > > > > > > > integrated PCI
> > > > > > > > Express endpoint, so it's certainly not the case that *all* 
> > > > > > > > root bus
> > > > > > > > devices are legacy.
> > > > > > > > 
> > > > > > > > > Guest OSes are suspected to behave
> > > > > > > > > > strangely when PCI Express devices are integrated
> > > > > > > > > > with the Root Complex.
> > > > > > > > 
> > > > > > > > 

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-13 Thread Marcel Apfelbaum

On 02/13/2017 06:33 AM, David Gibson wrote:

On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:

On 02/10/2017 02:37 AM, David Gibson wrote:

On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:

On 02/09/17 05:16, David Gibson wrote:

On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:

On 02/08/17 07:16, David Gibson wrote:

Marcel,

Your original patch adding PCIe support to virtio-pci.c has the
limitation noted below that PCIe won't be enabled if the device is on
the root bus (rather than under a root or downstream port).  As
reasoned below, I think removing the check is correct, even for x86
(though it would rarely be useful there).  But I could well have
missed something.  Let me know if so...



Virtio devices can appear as either vanilla PCI or PCI-Express devices
depending on the bus they're connected to.  At the moment it will only
appear as vanilla PCI if connected to the root bus of a PCIe host bridge.

Presumably this is to reflect the fact that PCIe devices usually need to
be connected to a root (or further downstream) port rather than directly
on the root bus.  However, due to the odd requirements of the PAPR spec on the 
'pseries'
machine type, it's normal for PCIe devices to appear on the root bus
without root ports.

Further, even on x86, there's no inherent reason we couldn't present a
virtio device as an "integrated device" (typically used for things built
into the PCI chipset), and those devices *do* typically appear on the root
bus.


I'm not personally making a counter-argument, just qouting some of
the relevant parts of "docs/pcie.txt" ("PCI EXPRESS GUIDELINES"):


So, an earlier discussion more or less concluded that the PCIe
guidelines don't really work with PAPR guests.  That comes because
PAPR was designed with PowerVM in mind which allows PCI passthrough
but doesn't do any emulated PCI devices.  So they wanted to present
passed through devices (virtual or phyical) to the guest without
inserting virtual root ports.

Now, you can argue that this was a silly decision in PAPR, and you
could well be right, but there it is.


I can totally accept this, but then we should state it as a fact near
the top of "docs/pcie.txt".




Place only the following kinds of devices directly on the Root Complex:
(1) PCI Devices (e.g. network card, graphics card, IDE controller),
not controllers. Place only legacy PCI devices on
the Root Complex. These will be considered Integrated Endpoints.
Note: Integrated Endpoints are not hot-pluggable.

Although the PCI Express spec does not forbid PCI Express devices as
Integrated Endpoints, existing hardware mostly integrates legacy PCI
devices with the Root Complex.


"Mostly".. on my laptop at least the GPU shows up as an integrated PCI
Express endpoint, so it's certainly not the case that *all* root bus
devices are legacy.


Guest OSes are suspected to behave

strangely when PCI Express devices are integrated
with the Root Complex.


Clearly not that strangely, that often, since my laptop works just fine.



[...]

2.2 PCI Express only hierarchy
==
Always use PCI Express Root Ports to start PCI Express hierarchies.


Above you mention "it's normal for PCIe devices to appear on the root bus without 
root ports".


Well "normal" perhaps wasn't the right word.  Let's say precedented,
if uncommon.


Let me turn the question around: is it a *problem* for "pseries" if
we require root ports? If so, why exactly?


That's.. a complex question.  At least Linux guests (and we don't
support any others yet) might cope with the addition of root ports.
Maybe.  I have discussed this option with BenH and others.

However it is gratuitously different from how PCIe devices will
typically appear for the same guest running under PowerVM.  Although I
suspect Linux would cope with the "normal standard" rather than "PAPR
standard" presentation, I'm not as confident about it as I would like.

Another consideration here is that other PCIe capable qemu emulated
devices, such as XHCI, will present fine as PCIe integrated endpoints
when attached to the root bus.  Libvirt won't do that usually, of
course, and it may not be the recommended way of doing things (on PC)
but it's possible.  I don't see any particular reason that virtio-pci
should enforce the root port requirement more so than any other
device.


On 02/08/17 07:16, David Gibson wrote:


pcie_endpoint_cap_init() already automatically adjusts to advertise as
an integrated device rather than a "normal" PCIe endpoint when attached to
a root bus.  So we can remove the check for root bus within virtio and
allow (at the user's discretion) a PCIe virtio bus to be attached to a
root bus.


If Marcel thinks this is a good change, then I think we should go
through "docs/pcie.txt" with a fine-toothed comb, and update all
relevant spots. (If Marcel agrees, perhaps you can include such
hunks in your patch 

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-12 Thread David Gibson
On Sun, Feb 12, 2017 at 09:05:46PM +0200, Marcel Apfelbaum wrote:
> On 02/10/2017 02:37 AM, David Gibson wrote:
> > On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:
> > > On 02/09/17 05:16, David Gibson wrote:
> > > > On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:
> > > > > On 02/08/17 07:16, David Gibson wrote:
> > > > > > Marcel,
> > > > > > 
> > > > > > Your original patch adding PCIe support to virtio-pci.c has the
> > > > > > limitation noted below that PCIe won't be enabled if the device is 
> > > > > > on
> > > > > > the root bus (rather than under a root or downstream port).  As
> > > > > > reasoned below, I think removing the check is correct, even for x86
> > > > > > (though it would rarely be useful there).  But I could well have
> > > > > > missed something.  Let me know if so...
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Virtio devices can appear as either vanilla PCI or PCI-Express 
> > > > > > devices
> > > > > > depending on the bus they're connected to.  At the moment it will 
> > > > > > only
> > > > > > appear as vanilla PCI if connected to the root bus of a PCIe host 
> > > > > > bridge.
> > > > > > 
> > > > > > Presumably this is to reflect the fact that PCIe devices usually 
> > > > > > need to
> > > > > > be connected to a root (or further downstream) port rather than 
> > > > > > directly
> > > > > > on the root bus.  However, due to the odd requirements of the PAPR 
> > > > > > spec on the 'pseries'
> > > > > > machine type, it's normal for PCIe devices to appear on the root bus
> > > > > > without root ports.
> > > > > > 
> > > > > > Further, even on x86, there's no inherent reason we couldn't 
> > > > > > present a
> > > > > > virtio device as an "integrated device" (typically used for things 
> > > > > > built
> > > > > > into the PCI chipset), and those devices *do* typically appear on 
> > > > > > the root
> > > > > > bus.
> > > > > 
> > > > > I'm not personally making a counter-argument, just qouting some of
> > > > > the relevant parts of "docs/pcie.txt" ("PCI EXPRESS GUIDELINES"):
> > > > 
> > > > So, an earlier discussion more or less concluded that the PCIe
> > > > guidelines don't really work with PAPR guests.  That comes because
> > > > PAPR was designed with PowerVM in mind which allows PCI passthrough
> > > > but doesn't do any emulated PCI devices.  So they wanted to present
> > > > passed through devices (virtual or phyical) to the guest without
> > > > inserting virtual root ports.
> > > > 
> > > > Now, you can argue that this was a silly decision in PAPR, and you
> > > > could well be right, but there it is.
> > > 
> > > I can totally accept this, but then we should state it as a fact near
> > > the top of "docs/pcie.txt".
> > > 
> > > > 
> > > > > > Place only the following kinds of devices directly on the Root 
> > > > > > Complex:
> > > > > > (1) PCI Devices (e.g. network card, graphics card, IDE 
> > > > > > controller),
> > > > > > not controllers. Place only legacy PCI devices on
> > > > > > the Root Complex. These will be considered Integrated 
> > > > > > Endpoints.
> > > > > > Note: Integrated Endpoints are not hot-pluggable.
> > > > > > 
> > > > > > Although the PCI Express spec does not forbid PCI Express 
> > > > > > devices as
> > > > > > Integrated Endpoints, existing hardware mostly integrates 
> > > > > > legacy PCI
> > > > > > devices with the Root Complex.
> > > > 
> > > > "Mostly".. on my laptop at least the GPU shows up as an integrated PCI
> > > > Express endpoint, so it's certainly not the case that *all* root bus
> > > > devices are legacy.
> > > > 
> > > > > Guest OSes are suspected to behave
> > > > > > strangely when PCI Express devices are integrated
> > > > > > with the Root Complex.
> > > > 
> > > > Clearly not that strangely, that often, since my laptop works just fine.
> > > > 
> > > > > > 
> > > > > > [...]
> > > > > > 
> > > > > > 2.2 PCI Express only hierarchy
> > > > > > ==
> > > > > > Always use PCI Express Root Ports to start PCI Express hierarchies.
> > > > > 
> > > > > Above you mention "it's normal for PCIe devices to appear on the root 
> > > > > bus without root ports".
> > > > 
> > > > Well "normal" perhaps wasn't the right word.  Let's say precedented,
> > > > if uncommon.
> > > > 
> > > > > Let me turn the question around: is it a *problem* for "pseries" if
> > > > > we require root ports? If so, why exactly?
> > > > 
> > > > That's.. a complex question.  At least Linux guests (and we don't
> > > > support any others yet) might cope with the addition of root ports.
> > > > Maybe.  I have discussed this option with BenH and others.
> > > > 
> > > > However it is gratuitously different from how PCIe devices will
> > > > typically appear for the same guest running under PowerVM.  Although I
> > > > suspect Linux would cope with the "normal standard" rather than "PAPR
> > > > standard" 

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-12 Thread Marcel Apfelbaum

On 02/10/2017 02:37 AM, David Gibson wrote:

On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:

On 02/09/17 05:16, David Gibson wrote:

On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:

On 02/08/17 07:16, David Gibson wrote:

Marcel,

Your original patch adding PCIe support to virtio-pci.c has the
limitation noted below that PCIe won't be enabled if the device is on
the root bus (rather than under a root or downstream port).  As
reasoned below, I think removing the check is correct, even for x86
(though it would rarely be useful there).  But I could well have
missed something.  Let me know if so...



Virtio devices can appear as either vanilla PCI or PCI-Express devices
depending on the bus they're connected to.  At the moment it will only
appear as vanilla PCI if connected to the root bus of a PCIe host bridge.

Presumably this is to reflect the fact that PCIe devices usually need to
be connected to a root (or further downstream) port rather than directly
on the root bus.  However, due to the odd requirements of the PAPR spec on the 
'pseries'
machine type, it's normal for PCIe devices to appear on the root bus
without root ports.

Further, even on x86, there's no inherent reason we couldn't present a
virtio device as an "integrated device" (typically used for things built
into the PCI chipset), and those devices *do* typically appear on the root
bus.


I'm not personally making a counter-argument, just qouting some of
the relevant parts of "docs/pcie.txt" ("PCI EXPRESS GUIDELINES"):


So, an earlier discussion more or less concluded that the PCIe
guidelines don't really work with PAPR guests.  That comes because
PAPR was designed with PowerVM in mind which allows PCI passthrough
but doesn't do any emulated PCI devices.  So they wanted to present
passed through devices (virtual or phyical) to the guest without
inserting virtual root ports.

Now, you can argue that this was a silly decision in PAPR, and you
could well be right, but there it is.


I can totally accept this, but then we should state it as a fact near
the top of "docs/pcie.txt".




Place only the following kinds of devices directly on the Root Complex:
(1) PCI Devices (e.g. network card, graphics card, IDE controller),
not controllers. Place only legacy PCI devices on
the Root Complex. These will be considered Integrated Endpoints.
Note: Integrated Endpoints are not hot-pluggable.

Although the PCI Express spec does not forbid PCI Express devices as
Integrated Endpoints, existing hardware mostly integrates legacy PCI
devices with the Root Complex.


"Mostly".. on my laptop at least the GPU shows up as an integrated PCI
Express endpoint, so it's certainly not the case that *all* root bus
devices are legacy.


Guest OSes are suspected to behave

strangely when PCI Express devices are integrated
with the Root Complex.


Clearly not that strangely, that often, since my laptop works just fine.



[...]

2.2 PCI Express only hierarchy
==
Always use PCI Express Root Ports to start PCI Express hierarchies.


Above you mention "it's normal for PCIe devices to appear on the root bus without 
root ports".


Well "normal" perhaps wasn't the right word.  Let's say precedented,
if uncommon.


Let me turn the question around: is it a *problem* for "pseries" if
we require root ports? If so, why exactly?


That's.. a complex question.  At least Linux guests (and we don't
support any others yet) might cope with the addition of root ports.
Maybe.  I have discussed this option with BenH and others.

However it is gratuitously different from how PCIe devices will
typically appear for the same guest running under PowerVM.  Although I
suspect Linux would cope with the "normal standard" rather than "PAPR
standard" presentation, I'm not as confident about it as I would like.

Another consideration here is that other PCIe capable qemu emulated
devices, such as XHCI, will present fine as PCIe integrated endpoints
when attached to the root bus.  Libvirt won't do that usually, of
course, and it may not be the recommended way of doing things (on PC)
but it's possible.  I don't see any particular reason that virtio-pci
should enforce the root port requirement more so than any other
device.


On 02/08/17 07:16, David Gibson wrote:


pcie_endpoint_cap_init() already automatically adjusts to advertise as
an integrated device rather than a "normal" PCIe endpoint when attached to
a root bus.  So we can remove the check for root bus within virtio and
allow (at the user's discretion) a PCIe virtio bus to be attached to a
root bus.


If Marcel thinks this is a good change, then I think we should go
through "docs/pcie.txt" with a fine-toothed comb, and update all
relevant spots. (If Marcel agrees, perhaps you can include such
hunks in your patch at once.)


Actually, I think that would be a neverending process.  Maybe better
to put in a whole different 

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-09 Thread David Gibson
On Thu, Feb 09, 2017 at 10:04:47AM +0100, Laszlo Ersek wrote:
> On 02/09/17 05:16, David Gibson wrote:
> > On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:
> >> On 02/08/17 07:16, David Gibson wrote:
> >>> Marcel,
> >>>
> >>> Your original patch adding PCIe support to virtio-pci.c has the
> >>> limitation noted below that PCIe won't be enabled if the device is on
> >>> the root bus (rather than under a root or downstream port).  As
> >>> reasoned below, I think removing the check is correct, even for x86
> >>> (though it would rarely be useful there).  But I could well have
> >>> missed something.  Let me know if so...
> >>>
> >>>
> >>>
> >>> Virtio devices can appear as either vanilla PCI or PCI-Express devices
> >>> depending on the bus they're connected to.  At the moment it will only
> >>> appear as vanilla PCI if connected to the root bus of a PCIe host bridge.
> >>>
> >>> Presumably this is to reflect the fact that PCIe devices usually need to
> >>> be connected to a root (or further downstream) port rather than directly
> >>> on the root bus.  However, due to the odd requirements of the PAPR spec 
> >>> on the 'pseries'
> >>> machine type, it's normal for PCIe devices to appear on the root bus
> >>> without root ports.
> >>>
> >>> Further, even on x86, there's no inherent reason we couldn't present a
> >>> virtio device as an "integrated device" (typically used for things built
> >>> into the PCI chipset), and those devices *do* typically appear on the root
> >>> bus.
> >>
> >> I'm not personally making a counter-argument, just qouting some of
> >> the relevant parts of "docs/pcie.txt" ("PCI EXPRESS GUIDELINES"):
> > 
> > So, an earlier discussion more or less concluded that the PCIe
> > guidelines don't really work with PAPR guests.  That comes because
> > PAPR was designed with PowerVM in mind which allows PCI passthrough
> > but doesn't do any emulated PCI devices.  So they wanted to present
> > passed through devices (virtual or phyical) to the guest without
> > inserting virtual root ports.
> > 
> > Now, you can argue that this was a silly decision in PAPR, and you
> > could well be right, but there it is.
> 
> I can totally accept this, but then we should state it as a fact near
> the top of "docs/pcie.txt".
> 
> > 
> >>> Place only the following kinds of devices directly on the Root Complex:
> >>> (1) PCI Devices (e.g. network card, graphics card, IDE controller),
> >>> not controllers. Place only legacy PCI devices on
> >>> the Root Complex. These will be considered Integrated Endpoints.
> >>> Note: Integrated Endpoints are not hot-pluggable.
> >>>
> >>> Although the PCI Express spec does not forbid PCI Express devices 
> >>> as
> >>> Integrated Endpoints, existing hardware mostly integrates legacy 
> >>> PCI
> >>> devices with the Root Complex.
> > 
> > "Mostly".. on my laptop at least the GPU shows up as an integrated PCI
> > Express endpoint, so it's certainly not the case that *all* root bus
> > devices are legacy.
> > 
> >> Guest OSes are suspected to behave
> >>> strangely when PCI Express devices are integrated
> >>> with the Root Complex.
> > 
> > Clearly not that strangely, that often, since my laptop works just fine.
> > 
> >>>
> >>> [...]
> >>>
> >>> 2.2 PCI Express only hierarchy
> >>> ==
> >>> Always use PCI Express Root Ports to start PCI Express hierarchies.
> >>
> >> Above you mention "it's normal for PCIe devices to appear on the root bus 
> >> without root ports".
> > 
> > Well "normal" perhaps wasn't the right word.  Let's say precedented,
> > if uncommon.
> > 
> >> Let me turn the question around: is it a *problem* for "pseries" if
> >> we require root ports? If so, why exactly?
> > 
> > That's.. a complex question.  At least Linux guests (and we don't
> > support any others yet) might cope with the addition of root ports.
> > Maybe.  I have discussed this option with BenH and others.
> > 
> > However it is gratuitously different from how PCIe devices will
> > typically appear for the same guest running under PowerVM.  Although I
> > suspect Linux would cope with the "normal standard" rather than "PAPR
> > standard" presentation, I'm not as confident about it as I would like.
> > 
> > Another consideration here is that other PCIe capable qemu emulated
> > devices, such as XHCI, will present fine as PCIe integrated endpoints
> > when attached to the root bus.  Libvirt won't do that usually, of
> > course, and it may not be the recommended way of doing things (on PC)
> > but it's possible.  I don't see any particular reason that virtio-pci
> > should enforce the root port requirement more so than any other
> > device.
> > 
> >> On 02/08/17 07:16, David Gibson wrote:
> >>>
> >>> pcie_endpoint_cap_init() already automatically adjusts to advertise as
> >>> an integrated device rather than a "normal" PCIe endpoint when attached to
> >>> a root bus.  So we can remove 

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-09 Thread Laszlo Ersek
On 02/09/17 05:16, David Gibson wrote:
> On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:
>> On 02/08/17 07:16, David Gibson wrote:
>>> Marcel,
>>>
>>> Your original patch adding PCIe support to virtio-pci.c has the
>>> limitation noted below that PCIe won't be enabled if the device is on
>>> the root bus (rather than under a root or downstream port).  As
>>> reasoned below, I think removing the check is correct, even for x86
>>> (though it would rarely be useful there).  But I could well have
>>> missed something.  Let me know if so...
>>>
>>>
>>>
>>> Virtio devices can appear as either vanilla PCI or PCI-Express devices
>>> depending on the bus they're connected to.  At the moment it will only
>>> appear as vanilla PCI if connected to the root bus of a PCIe host bridge.
>>>
>>> Presumably this is to reflect the fact that PCIe devices usually need to
>>> be connected to a root (or further downstream) port rather than directly
>>> on the root bus.  However, due to the odd requirements of the PAPR spec on 
>>> the 'pseries'
>>> machine type, it's normal for PCIe devices to appear on the root bus
>>> without root ports.
>>>
>>> Further, even on x86, there's no inherent reason we couldn't present a
>>> virtio device as an "integrated device" (typically used for things built
>>> into the PCI chipset), and those devices *do* typically appear on the root
>>> bus.
>>
>> I'm not personally making a counter-argument, just qouting some of
>> the relevant parts of "docs/pcie.txt" ("PCI EXPRESS GUIDELINES"):
> 
> So, an earlier discussion more or less concluded that the PCIe
> guidelines don't really work with PAPR guests.  That comes because
> PAPR was designed with PowerVM in mind which allows PCI passthrough
> but doesn't do any emulated PCI devices.  So they wanted to present
> passed through devices (virtual or phyical) to the guest without
> inserting virtual root ports.
> 
> Now, you can argue that this was a silly decision in PAPR, and you
> could well be right, but there it is.

I can totally accept this, but then we should state it as a fact near
the top of "docs/pcie.txt".

> 
>>> Place only the following kinds of devices directly on the Root Complex:
>>> (1) PCI Devices (e.g. network card, graphics card, IDE controller),
>>> not controllers. Place only legacy PCI devices on
>>> the Root Complex. These will be considered Integrated Endpoints.
>>> Note: Integrated Endpoints are not hot-pluggable.
>>>
>>> Although the PCI Express spec does not forbid PCI Express devices as
>>> Integrated Endpoints, existing hardware mostly integrates legacy PCI
>>> devices with the Root Complex.
> 
> "Mostly".. on my laptop at least the GPU shows up as an integrated PCI
> Express endpoint, so it's certainly not the case that *all* root bus
> devices are legacy.
> 
>> Guest OSes are suspected to behave
>>> strangely when PCI Express devices are integrated
>>> with the Root Complex.
> 
> Clearly not that strangely, that often, since my laptop works just fine.
> 
>>>
>>> [...]
>>>
>>> 2.2 PCI Express only hierarchy
>>> ==
>>> Always use PCI Express Root Ports to start PCI Express hierarchies.
>>
>> Above you mention "it's normal for PCIe devices to appear on the root bus 
>> without root ports".
> 
> Well "normal" perhaps wasn't the right word.  Let's say precedented,
> if uncommon.
> 
>> Let me turn the question around: is it a *problem* for "pseries" if
>> we require root ports? If so, why exactly?
> 
> That's.. a complex question.  At least Linux guests (and we don't
> support any others yet) might cope with the addition of root ports.
> Maybe.  I have discussed this option with BenH and others.
> 
> However it is gratuitously different from how PCIe devices will
> typically appear for the same guest running under PowerVM.  Although I
> suspect Linux would cope with the "normal standard" rather than "PAPR
> standard" presentation, I'm not as confident about it as I would like.
> 
> Another consideration here is that other PCIe capable qemu emulated
> devices, such as XHCI, will present fine as PCIe integrated endpoints
> when attached to the root bus.  Libvirt won't do that usually, of
> course, and it may not be the recommended way of doing things (on PC)
> but it's possible.  I don't see any particular reason that virtio-pci
> should enforce the root port requirement more so than any other
> device.
> 
>> On 02/08/17 07:16, David Gibson wrote:
>>>
>>> pcie_endpoint_cap_init() already automatically adjusts to advertise as
>>> an integrated device rather than a "normal" PCIe endpoint when attached to
>>> a root bus.  So we can remove the check for root bus within virtio and
>>> allow (at the user's discretion) a PCIe virtio bus to be attached to a
>>> root bus.
>>
>> If Marcel thinks this is a good change, then I think we should go
>> through "docs/pcie.txt" with a fine-toothed comb, and update all
>> relevant 

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-08 Thread David Gibson
On Wed, Feb 08, 2017 at 11:40:50AM +0100, Laszlo Ersek wrote:
> On 02/08/17 07:16, David Gibson wrote:
> > Marcel,
> > 
> > Your original patch adding PCIe support to virtio-pci.c has the
> > limitation noted below that PCIe won't be enabled if the device is on
> > the root bus (rather than under a root or downstream port).  As
> > reasoned below, I think removing the check is correct, even for x86
> > (though it would rarely be useful there).  But I could well have
> > missed something.  Let me know if so...
> > 
> > 
> > 
> > Virtio devices can appear as either vanilla PCI or PCI-Express devices
> > depending on the bus they're connected to.  At the moment it will only
> > appear as vanilla PCI if connected to the root bus of a PCIe host bridge.
> > 
> > Presumably this is to reflect the fact that PCIe devices usually need to
> > be connected to a root (or further downstream) port rather than directly
> > on the root bus.  However, due to the odd requirements of the PAPR spec on 
> > the 'pseries'
> > machine type, it's normal for PCIe devices to appear on the root bus
> > without root ports.
> > 
> > Further, even on x86, there's no inherent reason we couldn't present a
> > virtio device as an "integrated device" (typically used for things built
> > into the PCI chipset), and those devices *do* typically appear on the root
> > bus.
> 
> I'm not personally making a counter-argument, just qouting some of
> the relevant parts of "docs/pcie.txt" ("PCI EXPRESS GUIDELINES"):

So, an earlier discussion more or less concluded that the PCIe
guidelines don't really work with PAPR guests.  That comes because
PAPR was designed with PowerVM in mind which allows PCI passthrough
but doesn't do any emulated PCI devices.  So they wanted to present
passed through devices (virtual or phyical) to the guest without
inserting virtual root ports.

Now, you can argue that this was a silly decision in PAPR, and you
could well be right, but there it is.

> > Place only the following kinds of devices directly on the Root Complex:
> > (1) PCI Devices (e.g. network card, graphics card, IDE controller),
> > not controllers. Place only legacy PCI devices on
> > the Root Complex. These will be considered Integrated Endpoints.
> > Note: Integrated Endpoints are not hot-pluggable.
> >
> > Although the PCI Express spec does not forbid PCI Express devices as
> > Integrated Endpoints, existing hardware mostly integrates legacy PCI
> > devices with the Root Complex.

"Mostly".. on my laptop at least the GPU shows up as an integrated PCI
Express endpoint, so it's certainly not the case that *all* root bus
devices are legacy.

> Guest OSes are suspected to behave
> > strangely when PCI Express devices are integrated
> > with the Root Complex.

Clearly not that strangely, that often, since my laptop works just fine.

> >
> > [...]
> >
> > 2.2 PCI Express only hierarchy
> > ==
> > Always use PCI Express Root Ports to start PCI Express hierarchies.
> 
> Above you mention "it's normal for PCIe devices to appear on the root bus 
> without root ports".

Well "normal" perhaps wasn't the right word.  Let's say precedented,
if uncommon.

> Let me turn the question around: is it a *problem* for "pseries" if
> we require root ports? If so, why exactly?

That's.. a complex question.  At least Linux guests (and we don't
support any others yet) might cope with the addition of root ports.
Maybe.  I have discussed this option with BenH and others.

However it is gratuitously different from how PCIe devices will
typically appear for the same guest running under PowerVM.  Although I
suspect Linux would cope with the "normal standard" rather than "PAPR
standard" presentation, I'm not as confident about it as I would like.

Another consideration here is that other PCIe capable qemu emulated
devices, such as XHCI, will present fine as PCIe integrated endpoints
when attached to the root bus.  Libvirt won't do that usually, of
course, and it may not be the recommended way of doing things (on PC)
but it's possible.  I don't see any particular reason that virtio-pci
should enforce the root port requirement more so than any other
device.

> On 02/08/17 07:16, David Gibson wrote:
> > 
> > pcie_endpoint_cap_init() already automatically adjusts to advertise as
> > an integrated device rather than a "normal" PCIe endpoint when attached to
> > a root bus.  So we can remove the check for root bus within virtio and
> > allow (at the user's discretion) a PCIe virtio bus to be attached to a
> > root bus.
> 
> If Marcel thinks this is a good change, then I think we should go
> through "docs/pcie.txt" with a fine-toothed comb, and update all
> relevant spots. (If Marcel agrees, perhaps you can include such
> hunks in your patch at once.)

Actually, I think that would be a neverending process.  Maybe better
to put in a whole different spapr-pcie.txt with the assorted ways that

Re: [Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-08 Thread Laszlo Ersek
On 02/08/17 07:16, David Gibson wrote:
> Marcel,
> 
> Your original patch adding PCIe support to virtio-pci.c has the
> limitation noted below that PCIe won't be enabled if the device is on
> the root bus (rather than under a root or downstream port).  As
> reasoned below, I think removing the check is correct, even for x86
> (though it would rarely be useful there).  But I could well have
> missed something.  Let me know if so...
> 
> 
> 
> Virtio devices can appear as either vanilla PCI or PCI-Express devices
> depending on the bus they're connected to.  At the moment it will only
> appear as vanilla PCI if connected to the root bus of a PCIe host bridge.
> 
> Presumably this is to reflect the fact that PCIe devices usually need to
> be connected to a root (or further downstream) port rather than directly
> on the root bus.  However, due to the odd requirements of the PAPR spec on 
> the 'pseries'
> machine type, it's normal for PCIe devices to appear on the root bus
> without root ports.
> 
> Further, even on x86, there's no inherent reason we couldn't present a
> virtio device as an "integrated device" (typically used for things built
> into the PCI chipset), and those devices *do* typically appear on the root
> bus.

I'm not personally making a counter-argument, just qouting some of the relevant 
parts of "docs/pcie.txt" ("PCI EXPRESS GUIDELINES"):

> Place only the following kinds of devices directly on the Root Complex:
> (1) PCI Devices (e.g. network card, graphics card, IDE controller),
> not controllers. Place only legacy PCI devices on
> the Root Complex. These will be considered Integrated Endpoints.
> Note: Integrated Endpoints are not hot-pluggable.
>
> Although the PCI Express spec does not forbid PCI Express devices as
> Integrated Endpoints, existing hardware mostly integrates legacy PCI
> devices with the Root Complex. Guest OSes are suspected to behave
> strangely when PCI Express devices are integrated
> with the Root Complex.
>
> [...]
>
> 2.2 PCI Express only hierarchy
> ==
> Always use PCI Express Root Ports to start PCI Express hierarchies.

Above you mention "it's normal for PCIe devices to appear on the root bus 
without root ports".

Let me turn the question around: is it a *problem* for "pseries" if we require 
root ports? If so, why exactly?

On 02/08/17 07:16, David Gibson wrote:
> 
> pcie_endpoint_cap_init() already automatically adjusts to advertise as
> an integrated device rather than a "normal" PCIe endpoint when attached to
> a root bus.  So we can remove the check for root bus within virtio and
> allow (at the user's discretion) a PCIe virtio bus to be attached to a
> root bus.

If Marcel thinks this is a good change, then I think we should go through 
"docs/pcie.txt" with a fine-toothed comb, and update all relevant spots. (If 
Marcel agrees, perhaps you can include such hunks in your patch at once.)

It also may have consequences for libvirt (but I see you addressed Andrea at 
once, which is great).

Thanks,
Laszlo

> 
> Signed-off-by: David Gibson 
> ---
>  hw/virtio/virtio-pci.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 5ce42af..caea44c 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1737,8 +1737,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, 
> Error **errp)
>  {
>  VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
>  VirtioPCIClass *k = VIRTIO_PCI_GET_CLASS(pci_dev);
> -bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
> - !pci_bus_is_root(pci_dev->bus);
> +bool pcie_port = pci_bus_is_express(pci_dev->bus);
>  
>  if (!kvm_has_many_ioeventfds()) {
>  proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
> 




[Qemu-devel] [RFC] virtio-pci: Allow PCIe virtio devices on root bus

2017-02-07 Thread David Gibson
Marcel,

Your original patch adding PCIe support to virtio-pci.c has the
limitation noted below that PCIe won't be enabled if the device is on
the root bus (rather than under a root or downstream port).  As
reasoned below, I think removing the check is correct, even for x86
(though it would rarely be useful there).  But I could well have
missed something.  Let me know if so...



Virtio devices can appear as either vanilla PCI or PCI-Express devices
depending on the bus they're connected to.  At the moment it will only
appear as vanilla PCI if connected to the root bus of a PCIe host bridge.

Presumably this is to reflect the fact that PCIe devices usually need to
be connected to a root (or further downstream) port rather than directly
on the root bus.  However, due to the odd requirements of the PAPR spec on the 
'pseries'
machine type, it's normal for PCIe devices to appear on the root bus
without root ports.

Further, even on x86, there's no inherent reason we couldn't present a
virtio device as an "integrated device" (typically used for things built
into the PCI chipset), and those devices *do* typically appear on the root
bus.

pcie_endpoint_cap_init() already automatically adjusts to advertise as
an integrated device rather than a "normal" PCIe endpoint when attached to
a root bus.  So we can remove the check for root bus within virtio and
allow (at the user's discretion) a PCIe virtio bus to be attached to a
root bus.

Signed-off-by: David Gibson 
---
 hw/virtio/virtio-pci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 5ce42af..caea44c 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1737,8 +1737,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error 
**errp)
 {
 VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
 VirtioPCIClass *k = VIRTIO_PCI_GET_CLASS(pci_dev);
-bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
- !pci_bus_is_root(pci_dev->bus);
+bool pcie_port = pci_bus_is_express(pci_dev->bus);
 
 if (!kvm_has_many_ioeventfds()) {
 proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
-- 
2.9.3