Bug 754565

http://www.spinics.net/lists/kvm/msg63852.html
http://www.spinics.net/lists/kvm/msg63853.html
http://www.spinics.net/lists/kvm/msg63854.html
http://www.spinics.net/lists/kvm/msg63855.html

(Patches 1 & 2 aren't applicable since they were introduced with the new memory API)

Are you sure the first two aren't applicable?

(In reply to comment #2)
> Are you sure the first two aren't applicable?

Patch #1 fixes de-registering memory sub-regions before destroying the memory container.  This was a bug introduced by the memory API port of device assignment.  If we don't backport the memory API, we don't need this fix.

Patch #2 fixes the ioport access functions.  The memory API port attempted to make use of the existing functions, but it doesn't work.  This patch refreshed the access function to the new style API.  If we don't backport the memory API, we don't need this fix.

Off-by two bug: I read "patch #2" as "second of the four links in comment#1" instead of "upstream PATCH 2/6".  Sorry for the noise.

Hi,Alex

   Would you please provide an efficient way for qe verify this kind(found by Coverity) of issue? thanks

(In reply to comment #5)
> Hi,Alex
> 
>    Would you please provide an efficient way for qe verify this kind(found by
> Coverity) of issue? thanks

There is no simple way to test most of this.  The first patch in comment 1 might allow a PCIe non-endpoint device to be assigned, so we could maybe try assigning a PCIe root port (by qemu command line, not libvirt).  That shouldn't work after the patch.  The 2nd patch is a masking of PCI link capability bits, which you might notice in lspci for the device, but you'd have to be using a device that exposes an L1 exit latency.  Patch 3 is preventing bits in the PCIe link capabilities from being writable.  I guess we could try to write 0 or ~0 to the link capability register and see that there was a change and with the patch the write has no effect.  There's no way to test the last patch w/o modifying a kernel.

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
No documentation needed

(In reply to comment #6)
> (In reply to comment #5)
> > Hi,Alex
> > 
> >    Would you please provide an efficient way for qe verify this kind(found by
> > Coverity) of issue? thanks
> 
> There is no simple way to test most of this.  The first patch in comment 1
> might allow a PCIe non-endpoint device to be assigned, so we could maybe try
> assigning a PCIe root port (by qemu command line, not libvirt).  That 
Hi Alex, 
 I was trying to verify the first patch according to your method - try assigning a PCIe root port, seems it doesn't work on qemu-209, can you please take a look and tell my any step missing or offer me another way to verify? Thanks.

# /usr/libexec/qemu-kvm -M rhel6.2.0 -enable-kvm -m 2048 -smp 2,sockets=1,cores=2,threads=1 -name test -uuid `uuidgen` -rtc base=utc,clock=host,driftfix=slew -boot menu=on -drive file=/home/rhel6.3-64.qcow2,if=none,id=drive-virtio-0-0,media=disk,format=qcow2,cache=none,werror=stop,rerror=stop -device virtio-blk-pci,drive=drive-virtio-0-0,id=virt0-0-0,scsi=off -netdev tap,id=net -device virtio-net-pci,netdev=net,id=net0,mac=64:31:59:63:79:23 -usb -spice port=8000,disable-ticketing -vga qxl -global qxl-vga.vram_size=67108864 -monitor stdio -balloon none -device pci-assign,host=00:1c.0,id=root-port
do_spice_init: starting 0.10.1
spice_server_add_interface: SPICE_INTERFACE_MIGRATION
spice_server_add_interface: SPICE_INTERFACE_KEYBOARD
spice_server_add_interface: SPICE_INTERFACE_MOUSE
spice_server_add_interface: SPICE_INTERFACE_QXL
red_worker_main: begin
display_channel_create: create display channel
cursor_channel_create: create cursor channel
Failed to assign device "root-port" : Operation not permitted
qemu-kvm: -device pci-assign,host=00:1c.0,id=root-port: Device 'pci-assign' could not be initialized

# lspci -vvv -s 00:1c.0
00:1c.0 PCI bridge: Intel Corporation 82801JI (ICH10 Family) PCI Express Root Port 1 (prog-if 00 [Normal decode])
	Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Bus: primary=00, secondary=1c, subordinate=1c, sec-latency=0
	I/O behind bridge: 00003000-00003fff
	Memory behind bridge: fff00000-000fffff
	Prefetchable memory behind bridge: 00000000fff00000-00000000000fffff
	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR-
	BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
	Capabilities: [40] Express (v1) Root Port (Slot+), MSI 00
		DevCap:	MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us
			ExtTag- RBE+ FLReset-
		DevCtl:	Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
			RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
			MaxPayload 128 bytes, MaxReadReq 128 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
		LnkCap:	Port #1, Speed 2.5GT/s, Width x4, ASPM L0s L1, Latency L0 <1us, L1 <4us
			ClockPM- Surprise- LLActRep+ BwNot-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x0, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
		SltCap:	AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surpise+
			Slot #  3, PowerLimit 10.000000; Interlock- NoCompl-
		SltCtl:	Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg-
			Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock-
		SltSta:	Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock-
			Changed: MRL- PresDet- LinkState-
		RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna- CRSVisible-
		RootCap: CRSVisible-
		RootSta: PME ReqID 0000, PMEStatus- PMEPending-
	Capabilities: [80] MSI: Enable- Count=1/1 Maskable- 64bit-
		Address: fee00000  Data: 4069
	Capabilities: [90] Subsystem: Hewlett-Packard Company Device 130b
	Capabilities: [a0] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [100] Virtual Channel <?>
	Capabilities: [180] Root Complex Link <?>
	Kernel driver in use: pci-stub
	Kernel modules: shpchp

(In reply to comment #14)
>  I was trying to verify the first patch according to your method - try
> assigning a PCIe root port, seems it doesn't work on qemu-209, can you please
> take a look and tell my any step missing or offer me another way to verify?
> Thanks.
> 
> # /usr/libexec/qemu-kvm -M rhel6.2.0 -enable-kvm -m 2048 -smp
> 2,sockets=1,cores=2,threads=1 -name test -uuid `uuidgen` -rtc
> base=utc,clock=host,driftfix=slew -boot menu=on -drive
> file=/home/rhel6.3-64.qcow2,if=none,id=drive-virtio-0-0,media=disk,format=qcow2,cache=none,werror=stop,rerror=stop
> -device virtio-blk-pci,drive=drive-virtio-0-0,id=virt0-0-0,scsi=off -netdev
> tap,id=net -device virtio-net-pci,netdev=net,id=net0,mac=64:31:59:63:79:23 -usb
> -spice port=8000,disable-ticketing -vga qxl -global qxl-vga.vram_size=67108864
> -monitor stdio -balloon none -device pci-assign,host=00:1c.0,id=root-port
> do_spice_init: starting 0.10.1
> spice_server_add_interface: SPICE_INTERFACE_MIGRATION
> spice_server_add_interface: SPICE_INTERFACE_KEYBOARD
> spice_server_add_interface: SPICE_INTERFACE_MOUSE
> spice_server_add_interface: SPICE_INTERFACE_QXL
> red_worker_main: begin
> display_channel_create: create display channel
> cursor_channel_create: create cursor channel
> Failed to assign device "root-port" : Operation not permitted

It looks like you got a -EPERM from kvm, which is from Bug 756093 where the kernel will no longer allow such devices to be assigned.  IIRC, with this change qemu-kvm won't even attempt to assign the device and you should see this error:

    fprintf(stderr,
            "Device assignment only supports endpoint assignment, "
            "device type %d\n", type);

So as long as the test above is qemu-kvm without these fixes, this could still be a valid test case.

Thanks Alex. But still have no idea how to reproduce the issue patch 2&3 fixes.

Reproduction:
------------

------- Patch 1: a PCIe root port could be assigned to guest on:
2.6.32-220.el6.x86_64, qemu-kvm-0.12.1.2-2.209.el6.x86_64
logs from dmesg:
pci-stub 0000:00:1c.0: restoring config space at offset 0x1 (was 0x100400, writing 0x100103)
assign device: host bdf = 0:1c:0
pci-stub 0000:00:1c.0: restoring config space at offset 0x1 (was 0x100400, writing 0x100103)
tap0: no IPv6 routers present
kvm: 9375: cpu0 unimplemented perfctr wrmsr: 0xc1 data 0xabcd
switch: port 2(tap0) entering disabled state
device tap0 left promiscuous mode
switch: port 2(tap0) entering disabled state
pci-stub 0000:00:1c.0: restoring config space at offset 0x1 (was 0x100400, writing 0x100007)

------- Patch 2 & 3: I don't see any difference by lspci in guest between unfixed version and fixed version. So I used Coverity to scan qemu-kvm-209. If there is any efficient way to reproduce, I'd like to retest.

A slice of qemu-kvm-0.12.1.2-2.209.el6/run1/qemu-kvm-0.12.1.2-2.209.el6.err:
...
Error: CONSTANT_EXPRESSION_RESULT:
/builddir/build/BUILD/qemu-kvm-0.12.1.2/hw/device-assignment.c:1510: extra_high_bits: In lnkcap &= 262143 /* (((0xf | 0x3f0) | 0xc00) | 0x7000) | 0x38000 */, wider 262143 /* (((0xf | 0x3f0) | 0xc00) | 0x7000) | 0x38000 */ has high-order bits (0x30000) that don't affect the narrower left-hand side.

Error: CONSTANT_EXPRESSION_RESULT:
/builddir/build/BUILD/qemu-kvm-0.12.1.2/hw/device-assignment.c:1477: result_independent_of_operands: (type & 0xf0) >> 8 is 0 regardless of the values of its operands. This occurs as the non-specific operand of assignment.


Verification:
------------

------- Patch 1, a PCIe root port could not be assigned to guest on qemu-kvm-0.12.1.2-2.293.el6.x86_64 on error:
Device assignment only supports endpoint assignment, device type 4
qemu-kvm: -device pci-assign,host=00:1c.0,id=root-port: Device 'pci-assign' could not be initialized

------- Patch 2 & 3, rescanned on qemu-kvm-0.12.1.2-2.293, no such defects found.

(In reply to comment #16)
> 
> A slice of qemu-kvm-0.12.1.2-2.209.el6/run1/qemu-kvm-0.12.1.2-2.209.el6.err:
> ...
> Error: CONSTANT_EXPRESSION_RESULT:
> /builddir/build/BUILD/qemu-kvm-0.12.1.2/hw/device-assignment.c:1510:
> extra_high_bits: In lnkcap &= 262143 /* (((0xf | 0x3f0) | 0xc00) | 0x7000) |
> 0x38000 */, wider 262143 /* (((0xf | 0x3f0) | 0xc00) | 0x7000) | 0x38000 */
> has high-order bits (0x30000) that don't affect the narrower left-hand side.
> 
> Error: CONSTANT_EXPRESSION_RESULT:
> /builddir/build/BUILD/qemu-kvm-0.12.1.2/hw/device-assignment.c:1477:
> result_independent_of_operands: (type & 0xf0) >> 8 is 0 regardless of the
> values of its operands. This occurs as the non-specific operand of
> assignment.
> 
> 
> Verification:
> ------------

> ------- Patch 2 & 3, rescanned on qemu-kvm-0.12.1.2-2.293, no such defects
> found.

I think we should count the coverity fixes as verification.  These are very minut fixes that would require just the right device to verify otherwise.  Thanks.

Created attachment 586246 [details]
Coverity scan on qemu-kvm-293

I think this issue has been fixed according to Comment #16

(In reply to comment #18)
> Created attachment 586246 [details]
> Coverity scan on qemu-kvm-293
> 
> I think this issue has been fixed according to Comment #16

This issue has been fixed according to Comment #16, Comment #17

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2012-0746.html