Bug 1410577

Summary: [RFE] PCIe: Add an option to PCIe ports to disable IO port space support (depends on libvirt bug 1408810 )
Product: Red Hat Enterprise Virtualization Manager Reporter: Marcel Apfelbaum <marcel>
Component: ovirt-engineAssignee: Nobody <nobody>
Status: CLOSED WONTFIX QA Contact: meital avital <mavital>
Severity: low Docs Contact:
Priority: medium    
Version: unspecifiedCC: ailan, chayang, dfediuck, dyuan, hhan, jinzhao, juzhang, laine, lersek, libvirt-maint, lmen, lsurette, marcel, michal.skrivanek, mtessun, rbarry, Rhev-m-bugs, srevivo, virt-maint, xfu, xuzhang
Target Milestone: ---Keywords: FutureFeature
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: 1408810 Environment:
Last Closed: 2020-03-18 15:52:13 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Virt RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1344299, 1408810, 1410578    
Bug Blocks:    

Description Marcel Apfelbaum 2017-01-05 19:15:54 UTC 
+++ This bug was initially created as a clone of Bug #1408810 +++

+++ This bug was initially created as a clone of Bug #1344299 +++

Even if the firmware skips assigning IO ranges to PCIe ports (root
ports/downstream ports), Linux guests will still try to assign them IO.

We can to add a parameter "disable-io" to PCIe ports to disable IO support.
It will work by making IO base/limit registers read-only so both firmware
and guest OSes will comply.

--- Additional comment from RHEL Product and Program Management on 2016-06-09 08:04:38 EDT ---

Since this bug report was entered in bugzilla, the release flag has been
set to ? to ensure that it is properly evaluated for this release.

--- Additional comment from Marcel Apfelbaum on 2016-06-23 06:07:07 EDT ---

We intend to provide a 'thin' version of Q35 for 7.3 to be used mainly with
virtio devices which are PCIe, the IO limitation will not be an issue.


Add libvirt support for the device command line parameters.
Comment 1 Marcel Apfelbaum 2017-01-05 19:24:45 UTC 
By default libvirt will disable IO space support for PCI Express Root Ports since all PCI Express devices should work just fine using only MMIO.

However there are cases when we do want to enable IO space support:
1. We might want to assign a phys device (using vfio) and the device is not spec
   compliant and requires IO ports to function properly.
2. We might have a guest firmware with an ancient driver knowing only IO ports.

Decisions like this cannot be done by libvirt, we need the upper layer to decide which PCI Express Root Ports need to have IO ports enabled.
Comment 2 Martin Tessun 2017-03-16 08:20:43 UTC 
Is there any reason to skip the IO regions for 7.4? If I read comment 1 there are still quite some scenarios where you need the IO space.

As such: What would be the consequences if we don't disable IO space support for all other devices?
Comment 3 Marcel Apfelbaum 2017-03-29 13:55:26 UTC 
(In reply to Martin Tessun from comment #2)
> Is there any reason to skip the IO regions for 7.4? If I read comment 1
> there are still quite some scenarios where you need the IO space.
> 
> As such: What would be the consequences if we don't disable IO space support
> for all other devices?

If we attach too many legacy PCI devices (10+) to PCIe Root Ports (we are not supposed to do that, we are supposed to add PCI devices only to the Root Complex) the SeaBIOS will panic and the VM will halt.

Since we are not encouraging users to use legacy PCI devices on Q35, it is not a very interesting scenario, but it would improve QEMU's resiliency.

Thanks,
Marcel
Comment 4 Martin Tessun 2017-04-04 07:34:42 UTC 
(In reply to Marcel Apfelbaum from comment #3)
> (In reply to Martin Tessun from comment #2)
> > Is there any reason to skip the IO regions for 7.4? If I read comment 1
> > there are still quite some scenarios where you need the IO space.
> > 
> > As such: What would be the consequences if we don't disable IO space support
> > for all other devices?
> 
> If we attach too many legacy PCI devices (10+) to PCIe Root Ports (we are
> not supposed to do that, we are supposed to add PCI devices only to the Root
> Complex) the SeaBIOS will panic and the VM will halt.
> 
> Since we are not encouraging users to use legacy PCI devices on Q35, it is
> not a very interesting scenario, but it would improve QEMU's resiliency.
> 
> Thanks,
> Marcel

Ack.
So to sum up: Using "disable-io" to the PCIe ports would improve QEMU's resiliency and it wouldn't halt the VM in case too many PCI devices are attached to the PCIe root ports, correct?
If so, and there are no further drawbacks using this, I agree on doing this.
Comment 7 Marcel Apfelbaum 2017-04-04 12:53:19 UTC 
(In reply to Martin Tessun from comment #4)
> (In reply to Marcel Apfelbaum from comment #3)
> > (In reply to Martin Tessun from comment #2)
> > > Is there any reason to skip the IO regions for 7.4? If I read comment 1
> > > there are still quite some scenarios where you need the IO space.
> > > 
> > > As such: What would be the consequences if we don't disable IO space support
> > > for all other devices?
> > 
> > If we attach too many legacy PCI devices (10+) to PCIe Root Ports (we are
> > not supposed to do that, we are supposed to add PCI devices only to the Root
> > Complex) the SeaBIOS will panic and the VM will halt.
> > 
> > Since we are not encouraging users to use legacy PCI devices on Q35, it is
> > not a very interesting scenario, but it would improve QEMU's resiliency.
> > 
> > Thanks,
> > Marcel
> 
> Ack.
> So to sum up: Using "disable-io" to the PCIe ports would improve QEMU's
> resiliency and it wouldn't halt the VM in case too many PCI devices are
> attached to the PCIe root ports, correct?
> If so, and there are no further drawbacks using this, I agree on doing this.

Thanks! I hope I will be ready in time for 7.4 ...

I have another BZ doing something similar, but from the SeaBIOS point of view. It will not halt when out of IO, instead it will not reserve IO ports and continue "as usual".

Thanks,
Marcel
Comment 8 Martin Tessun 2017-06-21 11:46:47 UTC 
(In reply to Marcel Apfelbaum from comment #7)
> (In reply to Martin Tessun from comment #4)
> > (In reply to Marcel Apfelbaum from comment #3)
> > > (In reply to Martin Tessun from comment #2)
> > > > Is there any reason to skip the IO regions for 7.4? If I read comment 1
> > > > there are still quite some scenarios where you need the IO space.
> > > > 
> > > > As such: What would be the consequences if we don't disable IO space support
> > > > for all other devices?
> > > 
> > > If we attach too many legacy PCI devices (10+) to PCIe Root Ports (we are
> > > not supposed to do that, we are supposed to add PCI devices only to the Root
> > > Complex) the SeaBIOS will panic and the VM will halt.
> > > 
> > > Since we are not encouraging users to use legacy PCI devices on Q35, it is
> > > not a very interesting scenario, but it would improve QEMU's resiliency.
> > > 
> > > Thanks,
> > > Marcel
> > 
> > Ack.
> > So to sum up: Using "disable-io" to the PCIe ports would improve QEMU's
> > resiliency and it wouldn't halt the VM in case too many PCI devices are
> > attached to the PCIe root ports, correct?
> > If so, and there are no further drawbacks using this, I agree on doing this.
> 
> Thanks! I hope I will be ready in time for 7.4 ...
> 

Are you ready? :)

> I have another BZ doing something similar, but from the SeaBIOS point of
> view. It will not halt when out of IO, instead it will not reserve IO ports
> and continue "as usual".
> 
> Thanks,
> Marcel
Comment 9 Marcel Apfelbaum 2017-06-23 04:21:46 UTC 
(In reply to Martin Tessun from comment #8)
> (In reply to Marcel Apfelbaum from comment #7)
> > (In reply to Martin Tessun from comment #4)
> > > (In reply to Marcel Apfelbaum from comment #3)
> > > > (In reply to Martin Tessun from comment #2)
> > > > > Is there any reason to skip the IO regions for 7.4? If I read comment 1
> > > > > there are still quite some scenarios where you need the IO space.
> > > > > 
> > > > > As such: What would be the consequences if we don't disable IO space support
> > > > > for all other devices?
> > > > 
> > > > If we attach too many legacy PCI devices (10+) to PCIe Root Ports (we are
> > > > not supposed to do that, we are supposed to add PCI devices only to the Root
> > > > Complex) the SeaBIOS will panic and the VM will halt.
> > > > 
> > > > Since we are not encouraging users to use legacy PCI devices on Q35, it is
> > > > not a very interesting scenario, but it would improve QEMU's resiliency.
> > > > 
> > > > Thanks,
> > > > Marcel
> > > 
> > > Ack.
> > > So to sum up: Using "disable-io" to the PCIe ports would improve QEMU's
> > > resiliency and it wouldn't halt the VM in case too many PCI devices are
> > > attached to the PCIe root ports, correct?
> > > If so, and there are no further drawbacks using this, I agree on doing this.
> > 
> > Thanks! I hope I will be ready in time for 7.4 ...
> > 
> 
> Are you ready? :)
> 

Not  yet, the code is kind of ready but the guests don't "behave" yet.
Since is too late for 7.4 I moved the BZ to 7.5.

Thanks,
Marcel

> > I have another BZ doing something similar, but from the SeaBIOS point of
> > view. It will not halt when out of IO, instead it will not reserve IO ports
> > and continue "as usual".
> > 
> > Thanks,
> > Marcel
Comment 10 Michal Skrivanek 2020-03-18 15:37:02 UTC 
can you please doublecheck the latest state in AV 8.2?
Comment 11 Ryan Barry 2020-03-18 15:52:13 UTC 
The bug in AV is still ASSIGNED with no commits visible in tree. It's also not a very interesting use case