Re: [Xen-devel] Inconsistent MSI flags for pure HVM guest in 4.1.3 with the back-ported 4.2.0 msi_retranslate patch. (Was: Issue about domU missing interrupt)

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

Sorry for the delay in the reply

On Thu, 6 Dec 2012, G.R. wrote:
> Sorry, but I have to resend this in a separate thread for better visibility.
> Back ground:
> After backporting this patch to fix my PVHVM interrupt missing issue in 4.1.3:
> http://lists.xen.org/archives/html/xen-devel/2012-06/msg01909.html
> I find side effect for pure HVM guest.
> http://lists.xen.org/archives/html/xen-devel/2012-12/msg00208.html
> 
> I had a follow up thread to analysis the issue and post it again here:
> 
>             Hi, it seems that the patch has some side effect on pure HVM 
> guest.
>             For openelec 2.0 guest, which is based on linux 3.2.x with PVOP 
> disabled, I see such syndrome in
>             qemu-dm-xxx.log:
> 
>             pt_msgaddr64_reg_write: Error: why comes to Upper Address without 
> 64 bit support??
>             pt_pci_write_config: Internal error: Invalid write emulation 
> return value[-1]. I/O emulator exit.
> 
>             The guest dies immediately after the log, I have no way to check 
> guest kernel log.
>             Without the patch, this guest can boot without obvious error log 
> even the VGA passthrough does not
>             quite work.
>             I'll check the code to see what does these log mean...
> 
> 
> I did some analysis and it really looks like a bug to me.
> Since this is a patch back-ported from 4.2.0, I would like to ask is there 
> any follow up patch that would fix this
> issue?
> Please see my analysis below:
> 
> Here is part of the qemu-dm log, with debug log enabled at compile time:
> 
> dm-command: hot insert pass-through pci dev
> register_real_device: Assigning real physical device 00:1b.0 ...
> pt_iomul_init: Error: pt_iomul_init can't open file /dev/xen/pci_iomul: No 
> such file or directory: 0x0:0x1b.0x0
> pt_register_regions: IO region registered (size=0x00004000 
> base_addr=0xf7c10004)
> pt_msi_setup: msi mapped with pirq 36
> pci_intx: intx=1
> register_real_device: Real physical device 00:1b.0 registered successfuly!
> IRQ type = MSI-INTx
> ...
> pt_pci_read_config: [00:06.0]: address=0000 val=0x00008086 len=2
> pt_pci_read_config: [00:06.0]: address=0002 val=0x00001e20 len=2
> ...
> pt_pci_write_config: [00:06.0]: address=0068 val=0x00000000 len=4
> ...
> pt_pci_write_config: [00:06.0]: address=0062 val=0x00000081 len=2
> pt_msgctrl_reg_write: guest enabling MSI, disable MSI-INTx translation
> pci_intx: intx=1
> pt_msi_disable: Unmap msi with pirq 36
> pt_msgctrl_reg_write: setup msi for dev 30
> pt_msi_setup: msi mapped with pirq 36
> pt_msi_update: Update msi with pirq 36 gvec 51 gflags 1303
> pt_pci_read_config: [00:06.0]: address=0062 val=0x00000081 len=2
> pt_pci_write_config: [00:06.0]: address=0062 val=0x00000081 len=2
> pt_pci_write_config: [00:06.0]: address=0064 val=0xfee0300c len=4
> pt_pci_write_config: [00:06.0]: address=0068 val=0x00000000 len=4
> pt_msgaddr64_reg_write: Error: why comes to Upper Address without 64 bit 
> support??
> pt_pci_write_config: Internal error: Invalid write emulation return 
> value[-1]. I/O emulator exit.
> 
> 
> Here the device in question should is the audio controller, 00:1b.0 in the 
> host, which is 64 bit capable:
> 00:1b.0 Audio device: Intel Corporation 7 Series/C210 Series Chipset Family 
> High Definition Audio Controller (rev 04)
> ÂÂÂ Capabilities: [60] MSI: Enable+ Count=1/1 Maskable- 64bit+
> ÂÂÂ ÂÂÂ Address: 00000000fee00378Â Data: 0000

I won't be able to reproduce the bug, so I thank you in advance for any
help you can give me with testing a fix


> And there is also a successful write to offset 0x68 above, while the second 
> write fails the I/O emulator.
> 
> 
> The patch added pt_msi_disable() call into pt_msgctrl_reg_write() function, 
> And the pt_msi_disable() function has these
> lines:
> out:
> ÂÂÂ /* clear msi info */
> ÂÂÂ dev->msi->flags = 0;
> ÂÂÂ dev->msi->pirq = -1;
> ÂÂÂ dev->msi_trans_en = 0;
> 
> As a result, the flags are cleared -- this is new to the patch.
> And I believe this change caused the failure in pt_msgaddr64_write():
> 
> 3882ÂÂÂÂ /* check whether the type is 64 bit or not */
> 3883ÂÂÂÂ if (!(ptdev->msi->flags & PCI_MSI_FLAGS_64BIT))
> 3884ÂÂÂÂ {
> 3885ÂÂÂÂÂÂÂÂ /* exit I/O emulator */
> 3886ÂÂÂÂÂÂÂÂ PT_LOG("Error: why comes to Upper Address without 64 bit 
> support??\n");
> 3887ÂÂÂÂÂÂÂÂ return -1;
> 3888ÂÂÂÂ }
> 
> 
> I only see flags setup in pt_msgctrl_reg_init() function. I guess this is 
> not called this time.

I think you might be right, thanks for the detailed analysis!
If this is really the cause of the problem (as it looks like it is),
then the following should fix it:



diff --git a/hw/pt-msi.c b/hw/pt-msi.c
index 73f737d..b03b989 100644
--- a/hw/pt-msi.c
+++ b/hw/pt-msi.c
@@ -213,7 +213,7 @@ void pt_msi_disable(struct pt_dev *dev)
 
 out:
     /* clear msi info */
-    dev->msi->flags = 0;
+    dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | 
PCI_MSI_FLAGS_ENABLE);
     dev->msi->pirq = -1;
     dev->msi_trans_en = 0;
 }

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel