From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/HVM: fix hvmemul_rep_outs_set_context() Date: Thu, 23 Nov 2017 18:37:17 +0000 Message-ID: <0717d4d8-72a4-a332-796e-56b468cc95d3@citrix.com> References: <5A16F2B202000078001916CD@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------A8E70809883346E95BF84525" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eHwNL-0007RX-2A for xen-devel@lists.xenproject.org; Thu, 23 Nov 2017 18:37:27 +0000 In-Reply-To: <5A16F2B202000078001916CD@prv-mh.provo.novell.com> Content-Language: en-GB List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: Jan Beulich , xen-devel Cc: Julien Grall , Paul Durrant , Razvan Cojocaru List-Id: xen-devel@lists.xenproject.org --------------A8E70809883346E95BF84525 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit On 23/11/17 15:09, Jan Beulich wrote: > There were two issues with this function: Its use of > hvmemul_do_pio_buffer() was wrong (the function deals only with > individual port accesses, not repeated ones, i.e. passing it > "*reps * bytes_per_rep" does not have the intended effect). And it > could have processed a larger set of operations in one go than was > probably intended (limited just by the size that xmalloc() can hand > back). > > By converting to proper use of hvmemul_do_pio_buffer(), no intermediate > buffer is needed at all. As a result a preemption check is being added. > > Also drop unused parameters from the function. > > Signed-off-by: Jan Beulich While this does look like real bug, and bugfix, it isn't the issue I'm hitting.  I've distilled the repro scenario down to a tiny XTF test, which is just a `rep outsb` with a buffer which crosses a page boundary. The results are reliably: (d1) --- Xen Test Framework --- (d1) Environment: HVM 32bit (No paging) (d1) Test hvm-print (d1) String crossing a page boundary (XEN) MMIO emulation failed (1): d1v0 32bit @ 0010:001032b0 -> 5e c3 8d b4 26 00 00 00 00 8d bc 27 00 00 00 00 (d1) Test result: SUCCESS The Port IO hits a retry because of hitting the page boundary, and the retry logic successes, as evident by all data hitting hvm_print_line().  Somewhere however, the PIO turns into MMIO, and a failure is reported after the PIO completed successfully.  %rip in the failure message points after the `rep outsb`, rather than at it. If anyone has any ideas, I'm all ears.  If not, I will try to find some time to look deeper into the issue. ~Andrew --------------A8E70809883346E95BF84525 Content-Type: text/x-patch; name="0001-MMIO-failure-trigger.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-MMIO-failure-trigger.patch" >>From 9141a36374f52434a291e3be41bd259cfb9bda72 Mon Sep 17 00:00:00 2001 From: Andrew Cooper Date: Thu, 23 Nov 2017 18:31:40 +0000 Subject: [PATCH] MMIO failure trigger --- tests/hvm-print/Makefile | 9 +++++++++ tests/hvm-print/main.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 tests/hvm-print/Makefile create mode 100644 tests/hvm-print/main.c diff --git a/tests/hvm-print/Makefile b/tests/hvm-print/Makefile new file mode 100644 index 0000000..c70bede --- /dev/null +++ b/tests/hvm-print/Makefile @@ -0,0 +1,9 @@ +include $(ROOT)/build/common.mk + +NAME := hvm-print +CATEGORY := utility +TEST-ENVS := hvm32 + +obj-perenv += main.o + +include $(ROOT)/build/gen.mk diff --git a/tests/hvm-print/main.c b/tests/hvm-print/main.c new file mode 100644 index 0000000..882b716 --- /dev/null +++ b/tests/hvm-print/main.c @@ -0,0 +1,40 @@ +/** + * @file tests/hvm-print/main.c + * @ref test-hvm-print + * + * @page test-hvm-print hvm-print + * + * @todo Docs for test-hvm-print + * + * @see tests/hvm-print/main.c + */ +#include + +const char test_title[] = "Test hvm-print"; + +static char buf[2 * PAGE_SIZE] __page_aligned_bss; + +void test_main(void) +{ + char *ptr = &buf[4090]; + size_t len; + + strcpy(ptr, "String crossing a page boundary\n"); + len = strlen(ptr); + + asm volatile("rep; outsb" + : "+S" (ptr), "+c" (len) + : "d" (0xe9)); + + xtf_success(NULL); +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ -- 2.1.4 --------------A8E70809883346E95BF84525 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVucHJvamVjdC5vcmcKaHR0cHM6Ly9saXN0 cy54ZW5wcm9qZWN0Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL3hlbi1kZXZlbA== --------------A8E70809883346E95BF84525--