Re: [Xen-devel] Removing the PVH assert in arch/x86/hvm/io.c:87

2014-12-05 Thread Jan Beulich
 On 04.12.14 at 17:35, roger@citrix.com wrote:
 I've just stumbled upon this assert while testing PVH on different
 hardware. It was added in 7c4870 as a safe belt, but it turns out INS
 and OUTS go through handle_mmio. So using this instructions from a PVH
 guest basically kills Xen.
 
 I've removed it and everything seems fine, so I'm considering sending a
 patch for 4.5 in order to have it removed. I think the path that could
 trigger the crash because of the missing vioapic stuff is already
 guarded by the other chunk added in the same patch.

Iirc we settled on forbidding paths to handle_mmio() for PVH (hence
the ASSERT()). Sadly you provide way too little detail on what is
actually happening in your case: What's the use case of to-be-
emulated INS/OUTS in a PVH kernel? What's the call tree that gets
you into handle_mmio(), considering that both calls to
handle_mmio_with_translation() from hvm_hap_nested_page_fault()
as well as the one to handle_mmio() ought to be unreachable for PVH?

Jan



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


Re: [Xen-devel] Removing the PVH assert in arch/x86/hvm/io.c:87

2014-12-05 Thread Roger Pau Monné
El 05/12/14 a les 10.15, Jan Beulich ha escrit:
 On 04.12.14 at 17:35, roger@citrix.com wrote:
 I've just stumbled upon this assert while testing PVH on different
 hardware. It was added in 7c4870 as a safe belt, but it turns out INS
 and OUTS go through handle_mmio. So using this instructions from a PVH
 guest basically kills Xen.

 I've removed it and everything seems fine, so I'm considering sending a
 patch for 4.5 in order to have it removed. I think the path that could
 trigger the crash because of the missing vioapic stuff is already
 guarded by the other chunk added in the same patch.
 
 Iirc we settled on forbidding paths to handle_mmio() for PVH (hence
 the ASSERT()). Sadly you provide way too little detail on what is
 actually happening in your case: What's the use case of to-be-
 emulated INS/OUTS in a PVH kernel?

In this specific situation I'm seeing intsw instructions executed by the
FreeBSD ATA layer:

http://fxr.watson.org/fxr/source/dev/ata/ata-lowlevel.c#L740

 What's the call tree that gets
 you into handle_mmio(), considering that both calls to
 handle_mmio_with_translation() from hvm_hap_nested_page_fault()
 as well as the one to handle_mmio() ought to be unreachable for PVH?

You can get there from vmx_vmexit_handler if the exit reason is
EXIT_REASON_IO_INSTRUCTION.

Roger.


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


Re: [Xen-devel] Removing the PVH assert in arch/x86/hvm/io.c:87

2014-12-05 Thread David Vrabel
On 05/12/14 11:07, Roger Pau Monné wrote:
 El 05/12/14 a les 10.15, Jan Beulich ha escrit:
 On 04.12.14 at 17:35, roger@citrix.com wrote:
 I've just stumbled upon this assert while testing PVH on different
 hardware. It was added in 7c4870 as a safe belt, but it turns out INS
 and OUTS go through handle_mmio. So using this instructions from a PVH
 guest basically kills Xen.

 I've removed it and everything seems fine, so I'm considering sending a
 patch for 4.5 in order to have it removed. I think the path that could
 trigger the crash because of the missing vioapic stuff is already
 guarded by the other chunk added in the same patch.

 Iirc we settled on forbidding paths to handle_mmio() for PVH (hence
 the ASSERT()). Sadly you provide way too little detail on what is
 actually happening in your case: What's the use case of to-be-
 emulated INS/OUTS in a PVH kernel?
 
 In this specific situation I'm seeing intsw instructions executed by the
 FreeBSD ATA layer:
 
 http://fxr.watson.org/fxr/source/dev/ata/ata-lowlevel.c#L740

Why are you running this device driver at all in a PVH guest?  It should
only be using PV block devices.

David

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


Re: [Xen-devel] Removing the PVH assert in arch/x86/hvm/io.c:87

2014-12-05 Thread Jan Beulich
 On 05.12.14 at 12:07, roger@citrix.com wrote:
 El 05/12/14 a les 10.15, Jan Beulich ha escrit:
 On 04.12.14 at 17:35, roger@citrix.com wrote:
 I've just stumbled upon this assert while testing PVH on different
 hardware. It was added in 7c4870 as a safe belt, but it turns out INS
 and OUTS go through handle_mmio. So using this instructions from a PVH
 guest basically kills Xen.

 I've removed it and everything seems fine, so I'm considering sending a
 patch for 4.5 in order to have it removed. I think the path that could
 trigger the crash because of the missing vioapic stuff is already
 guarded by the other chunk added in the same patch.
 
 Iirc we settled on forbidding paths to handle_mmio() for PVH (hence
 the ASSERT()). Sadly you provide way too little detail on what is
 actually happening in your case: What's the use case of to-be-
 emulated INS/OUTS in a PVH kernel?
 
 In this specific situation I'm seeing intsw instructions executed by the
 FreeBSD ATA layer:
 
 http://fxr.watson.org/fxr/source/dev/ata/ata-lowlevel.c#L740 
 
 What's the call tree that gets
 you into handle_mmio(), considering that both calls to
 handle_mmio_with_translation() from hvm_hap_nested_page_fault()
 as well as the one to handle_mmio() ought to be unreachable for PVH?
 
 You can get there from vmx_vmexit_handler if the exit reason is
 EXIT_REASON_IO_INSTRUCTION.

A PVH guest without passed through device shouldn't access I/O
ports in the first place. Are you trying to hand an IDE or SATA
controller to the guest? Or is this happening with just Dom0, in
which case I'd suspect the I/O bitmap isn't being set up properly,
thus causing a VM exit when none is needed?

And yes, guarding the EXIT_REASON_IO_INSTRUCTION handling
in vmx_vmexit_handler() against PVH would seem necessary,
directing control flow to exit_and_crash. I'm pretty certain I had
pointed this out while reviewing the original PVH series.

Jan


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


Re: [Xen-devel] Removing the PVH assert in arch/x86/hvm/io.c:87

2014-12-04 Thread Mukesh Rathor
On Thu, 4 Dec 2014 17:35:59 +0100
Roger Pau Monné roger@citrix.com wrote:

 Hello,
 
 I've just stumbled upon this assert while testing PVH on different
 hardware. It was added in 7c4870 as a safe belt, but it turns out INS
 and OUTS go through handle_mmio. So using this instructions from a PVH
 guest basically kills Xen.

Right. Unf CR-moves/lmsw/clts intercepts also go thru handle_mmio, and
the suggestion was to clean it up first with another emulator function
for CR/IO intercepts. I attempted to do that before I left :

http://lists.xen.org/archives/html/xen-devel/2014-08/msg02204.html

See also:

http://lists.xen.org/archives/html/xen-devel/2014-08/msg00391.html

 I've removed it and everything seems fine, so I'm considering sending
 a patch for 4.5 in order to have it removed. I think the path that
 could trigger the crash because of the missing vioapic stuff is
 already guarded by the other chunk added in the same patch.

No, there used to be another path thru hvm_hap_nested_page_fault()
that would walk the back end handlers and crash xen. So you might
wanna check to make sure. I see hvm_hap_nested_page_fault() looks a
little different now, so not sure if its still broken... 

Mukesh


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