[PATCH v2] x86/HVM: fix forwarding of internally cached requests

[PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 5652 bytes --]

Forwarding entire batches to the device model when an individual
iteration of them got rejected by internal device emulation handlers
with X86EMUL_UNHANDLEABLE is wrong: The device model would then handle
all iterations, without the internal handler getting to see any past
the one it returned failure for. This causes misbehavior in at least
the MSI-X and VGA code, which want to see all such requests for
internal tracking/caching purposes. But note that this does not apply
to buffered I/O requests.

This in turn means that the condition in hvm_process_io_intercept() of
when to crash the domain was wrong: Since X86EMUL_UNHANDLEABLE can
validly be returned by the individual device handlers, we mustn't
blindly crash the domain if such occurs on other than the initial
iteration. Instead we need to distinguish hvm_copy_*_guest_phys()
failures from device specific ones, and then the former need to always
be fatal to the domain (i.e. also on the first iteration), since
otherwise we again would end up forwarding a request to qemu which the
internal handler didn't get to see.

The adjustment should be okay even for stdvga's MMIO handling:
- if it is not caching then the accept function would have failed so we
  won't get into hvm_process_io_intercept(),
- if it issued the buffered ioreq then we only get to the p->count
  reduction if hvm_send_ioreq() actually encountered an error (in which
  we don't care about the request getting split up).

Also commit 4faffc41d ("x86/hvm: limit reps to avoid the need to handle
retry") went too far in removing code from hvm_process_io_intercept():
When there were successfully handled iterations, the function should
continue to return success with a clipped repeat count.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Chang Jianzhong <changjzh@gmail.com>
---
v2: Make hvm_process_io_intercept() always return a valid X86EMUL_*
    code by moving the domain_crash() invocations up. Add a stdvga
    related paragraph to the commit message.
---
I assume this also addresses the issue which
http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html 
attempted to deal with in a not really acceptable way.

--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -95,7 +95,7 @@ static const struct hvm_io_handler null_
 };
 
 static int hvmemul_do_io(
-    bool_t is_mmio, paddr_t addr, unsigned long reps, unsigned int size,
+    bool_t is_mmio, paddr_t addr, unsigned long *reps, unsigned int size,
     uint8_t dir, bool_t df, bool_t data_is_addr, uintptr_t data)
 {
     struct vcpu *curr = current;
@@ -104,7 +104,7 @@ static int hvmemul_do_io(
         .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
         .addr = addr,
         .size = size,
-        .count = reps,
+        .count = *reps,
         .dir = dir,
         .df = df,
         .data = data,
@@ -136,7 +136,7 @@ static int hvmemul_do_io(
         if ( (p.type != is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO) ||
              (p.addr != addr) ||
              (p.size != size) ||
-             (p.count != reps) ||
+             (p.count != *reps) ||
              (p.dir != dir) ||
              (p.df != df) ||
              (p.data_is_ptr != data_is_addr) )
@@ -214,7 +214,7 @@ static int hvmemul_do_io_buffer(
 
     BUG_ON(buffer == NULL);
 
-    rc = hvmemul_do_io(is_mmio, addr, *reps, size, dir, df, 0,
+    rc = hvmemul_do_io(is_mmio, addr, reps, size, dir, df, 0,
                        (uintptr_t)buffer);
     if ( rc == X86EMUL_UNHANDLEABLE && dir == IOREQ_READ )
         memset(buffer, 0xff, size);
@@ -305,13 +305,13 @@ static int hvmemul_do_io_addr(
         count = 1;
     }
 
-    rc = hvmemul_do_io(is_mmio, addr, count, size, dir, df, 1,
+    rc = hvmemul_do_io(is_mmio, addr, &count, size, dir, df, 1,
                        ram_gpa);
+
     if ( rc == X86EMUL_OKAY )
-    {
         v->arch.hvm_vcpu.hvm_io.mmio_retry = (count < *reps);
-        *reps = count;
-    }
+
+    *reps = count;
 
  out:
     while ( nr_pages )
--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -148,8 +148,8 @@ int hvm_process_io_intercept(const struc
                     ASSERT_UNREACHABLE();
                     /* fall through */
                 default:
-                    rc = X86EMUL_UNHANDLEABLE;
-                    break;
+                    domain_crash(current->domain);
+                    return X86EMUL_UNHANDLEABLE;
                 }
                 if ( rc != X86EMUL_OKAY )
                     break;
@@ -178,8 +178,8 @@ int hvm_process_io_intercept(const struc
                     ASSERT_UNREACHABLE();
                     /* fall through */
                 default:
-                    rc = X86EMUL_UNHANDLEABLE;
-                    break;
+                    domain_crash(current->domain);
+                    return X86EMUL_UNHANDLEABLE;
                 }
                 if ( rc != X86EMUL_OKAY )
                     break;
@@ -196,8 +196,20 @@ int hvm_process_io_intercept(const struc
         }
     }
 
-    if ( i != 0 && rc == X86EMUL_UNHANDLEABLE )
-        domain_crash(current->domain);
+    if ( i )
+    {
+        p->count = i;
+        rc = X86EMUL_OKAY;
+    }
+    else if ( rc == X86EMUL_UNHANDLEABLE )
+    {
+        /*
+         * Don't forward entire batches to the device model: This would
+         * prevent the internal handlers to see subsequent iterations of
+         * the request.
+         */
+        p->count = 1;
+    }
 
     return rc;
 }



[-- Attachment #2: x86-HVM-internal-IO-with-REP.patch --]
[-- Type: text/plain, Size: 5704 bytes --]

x86/HVM: fix forwarding of internally cached requests

Forwarding entire batches to the device model when an individual
iteration of them got rejected by internal device emulation handlers
with X86EMUL_UNHANDLEABLE is wrong: The device model would then handle
all iterations, without the internal handler getting to see any past
the one it returned failure for. This causes misbehavior in at least
the MSI-X and VGA code, which want to see all such requests for
internal tracking/caching purposes. But note that this does not apply
to buffered I/O requests.

This in turn means that the condition in hvm_process_io_intercept() of
when to crash the domain was wrong: Since X86EMUL_UNHANDLEABLE can
validly be returned by the individual device handlers, we mustn't
blindly crash the domain if such occurs on other than the initial
iteration. Instead we need to distinguish hvm_copy_*_guest_phys()
failures from device specific ones, and then the former need to always
be fatal to the domain (i.e. also on the first iteration), since
otherwise we again would end up forwarding a request to qemu which the
internal handler didn't get to see.

The adjustment should be okay even for stdvga's MMIO handling:
- if it is not caching then the accept function would have failed so we
  won't get into hvm_process_io_intercept(),
- if it issued the buffered ioreq then we only get to the p->count
  reduction if hvm_send_ioreq() actually encountered an error (in which
  we don't care about the request getting split up).

Also commit 4faffc41d ("x86/hvm: limit reps to avoid the need to handle
retry") went too far in removing code from hvm_process_io_intercept():
When there were successfully handled iterations, the function should
continue to return success with a clipped repeat count.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Chang Jianzhong <changjzh@gmail.com>
---
v2: Make hvm_process_io_intercept() always return a valid X86EMUL_*
    code by moving the domain_crash() invocations up. Add a stdvga
    related paragraph to the commit message.
---
I assume this also addresses the issue which
http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html
attempted to deal with in a not really acceptable way.

--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -95,7 +95,7 @@ static const struct hvm_io_handler null_
 };
 
 static int hvmemul_do_io(
-    bool_t is_mmio, paddr_t addr, unsigned long reps, unsigned int size,
+    bool_t is_mmio, paddr_t addr, unsigned long *reps, unsigned int size,
     uint8_t dir, bool_t df, bool_t data_is_addr, uintptr_t data)
 {
     struct vcpu *curr = current;
@@ -104,7 +104,7 @@ static int hvmemul_do_io(
         .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
         .addr = addr,
         .size = size,
-        .count = reps,
+        .count = *reps,
         .dir = dir,
         .df = df,
         .data = data,
@@ -136,7 +136,7 @@ static int hvmemul_do_io(
         if ( (p.type != is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO) ||
              (p.addr != addr) ||
              (p.size != size) ||
-             (p.count != reps) ||
+             (p.count != *reps) ||
              (p.dir != dir) ||
              (p.df != df) ||
              (p.data_is_ptr != data_is_addr) )
@@ -214,7 +214,7 @@ static int hvmemul_do_io_buffer(
 
     BUG_ON(buffer == NULL);
 
-    rc = hvmemul_do_io(is_mmio, addr, *reps, size, dir, df, 0,
+    rc = hvmemul_do_io(is_mmio, addr, reps, size, dir, df, 0,
                        (uintptr_t)buffer);
     if ( rc == X86EMUL_UNHANDLEABLE && dir == IOREQ_READ )
         memset(buffer, 0xff, size);
@@ -305,13 +305,13 @@ static int hvmemul_do_io_addr(
         count = 1;
     }
 
-    rc = hvmemul_do_io(is_mmio, addr, count, size, dir, df, 1,
+    rc = hvmemul_do_io(is_mmio, addr, &count, size, dir, df, 1,
                        ram_gpa);
+
     if ( rc == X86EMUL_OKAY )
-    {
         v->arch.hvm_vcpu.hvm_io.mmio_retry = (count < *reps);
-        *reps = count;
-    }
+
+    *reps = count;
 
  out:
     while ( nr_pages )
--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -148,8 +148,8 @@ int hvm_process_io_intercept(const struc
                     ASSERT_UNREACHABLE();
                     /* fall through */
                 default:
-                    rc = X86EMUL_UNHANDLEABLE;
-                    break;
+                    domain_crash(current->domain);
+                    return X86EMUL_UNHANDLEABLE;
                 }
                 if ( rc != X86EMUL_OKAY )
                     break;
@@ -178,8 +178,8 @@ int hvm_process_io_intercept(const struc
                     ASSERT_UNREACHABLE();
                     /* fall through */
                 default:
-                    rc = X86EMUL_UNHANDLEABLE;
-                    break;
+                    domain_crash(current->domain);
+                    return X86EMUL_UNHANDLEABLE;
                 }
                 if ( rc != X86EMUL_OKAY )
                     break;
@@ -196,8 +196,20 @@ int hvm_process_io_intercept(const struc
         }
     }
 
-    if ( i != 0 && rc == X86EMUL_UNHANDLEABLE )
-        domain_crash(current->domain);
+    if ( i )
+    {
+        p->count = i;
+        rc = X86EMUL_OKAY;
+    }
+    else if ( rc == X86EMUL_UNHANDLEABLE )
+    {
+        /*
+         * Don't forward entire batches to the device model: This would
+         * prevent the internal handlers to see subsequent iterations of
+         * the request.
+         */
+        p->count = 1;
+    }
 
     return rc;
 }

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 29 March 2016 11:40
> To: xen-devel
> Cc: Andrew Cooper; Paul Durrant; Chang Jianzhong; Keir (Xen.org)
> Subject: [PATCH v2] x86/HVM: fix forwarding of internally cached requests
> 
> Forwarding entire batches to the device model when an individual
> iteration of them got rejected by internal device emulation handlers
> with X86EMUL_UNHANDLEABLE is wrong: The device model would then
> handle
> all iterations, without the internal handler getting to see any past
> the one it returned failure for. This causes misbehavior in at least
> the MSI-X and VGA code, which want to see all such requests for
> internal tracking/caching purposes. But note that this does not apply
> to buffered I/O requests.
> 
> This in turn means that the condition in hvm_process_io_intercept() of
> when to crash the domain was wrong: Since X86EMUL_UNHANDLEABLE can
> validly be returned by the individual device handlers, we mustn't
> blindly crash the domain if such occurs on other than the initial
> iteration. Instead we need to distinguish hvm_copy_*_guest_phys()
> failures from device specific ones, and then the former need to always
> be fatal to the domain (i.e. also on the first iteration), since
> otherwise we again would end up forwarding a request to qemu which the
> internal handler didn't get to see.
> 
> The adjustment should be okay even for stdvga's MMIO handling:
> - if it is not caching then the accept function would have failed so we
>   won't get into hvm_process_io_intercept(),
> - if it issued the buffered ioreq then we only get to the p->count
>   reduction if hvm_send_ioreq() actually encountered an error (in which
>   we don't care about the request getting split up).
> 
> Also commit 4faffc41d ("x86/hvm: limit reps to avoid the need to handle
> retry") went too far in removing code from hvm_process_io_intercept():
> When there were successfully handled iterations, the function should
> continue to return success with a clipped repeat count.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> Cc: Chang Jianzhong <changjzh@gmail.com>
> ---
> v2: Make hvm_process_io_intercept() always return a valid X86EMUL_*
>     code by moving the domain_crash() invocations up. Add a stdvga
>     related paragraph to the commit message.
> ---
> I assume this also addresses the issue which
> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html
> attempted to deal with in a not really acceptable way.
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -95,7 +95,7 @@ static const struct hvm_io_handler null_
>  };
> 
>  static int hvmemul_do_io(
> -    bool_t is_mmio, paddr_t addr, unsigned long reps, unsigned int size,
> +    bool_t is_mmio, paddr_t addr, unsigned long *reps, unsigned int size,
>      uint8_t dir, bool_t df, bool_t data_is_addr, uintptr_t data)
>  {
>      struct vcpu *curr = current;
> @@ -104,7 +104,7 @@ static int hvmemul_do_io(
>          .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
>          .addr = addr,
>          .size = size,
> -        .count = reps,
> +        .count = *reps,
>          .dir = dir,
>          .df = df,
>          .data = data,
> @@ -136,7 +136,7 @@ static int hvmemul_do_io(
>          if ( (p.type != is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO) ||
>               (p.addr != addr) ||
>               (p.size != size) ||
> -             (p.count != reps) ||
> +             (p.count != *reps) ||
>               (p.dir != dir) ||
>               (p.df != df) ||
>               (p.data_is_ptr != data_is_addr) )
> @@ -214,7 +214,7 @@ static int hvmemul_do_io_buffer(
> 
>      BUG_ON(buffer == NULL);
> 
> -    rc = hvmemul_do_io(is_mmio, addr, *reps, size, dir, df, 0,
> +    rc = hvmemul_do_io(is_mmio, addr, reps, size, dir, df, 0,
>                         (uintptr_t)buffer);
>      if ( rc == X86EMUL_UNHANDLEABLE && dir == IOREQ_READ )
>          memset(buffer, 0xff, size);
> @@ -305,13 +305,13 @@ static int hvmemul_do_io_addr(
>          count = 1;
>      }
> 
> -    rc = hvmemul_do_io(is_mmio, addr, count, size, dir, df, 1,
> +    rc = hvmemul_do_io(is_mmio, addr, &count, size, dir, df, 1,
>                         ram_gpa);
> +
>      if ( rc == X86EMUL_OKAY )
> -    {
>          v->arch.hvm_vcpu.hvm_io.mmio_retry = (count < *reps);
> -        *reps = count;
> -    }
> +
> +    *reps = count;
> 
>   out:
>      while ( nr_pages )
> --- a/xen/arch/x86/hvm/intercept.c
> +++ b/xen/arch/x86/hvm/intercept.c
> @@ -148,8 +148,8 @@ int hvm_process_io_intercept(const struc
>                      ASSERT_UNREACHABLE();
>                      /* fall through */
>                  default:
> -                    rc = X86EMUL_UNHANDLEABLE;
> -                    break;
> +                    domain_crash(current->domain);
> +                    return X86EMUL_UNHANDLEABLE;
>                  }
>                  if ( rc != X86EMUL_OKAY )
>                      break;
> @@ -178,8 +178,8 @@ int hvm_process_io_intercept(const struc
>                      ASSERT_UNREACHABLE();
>                      /* fall through */
>                  default:
> -                    rc = X86EMUL_UNHANDLEABLE;
> -                    break;
> +                    domain_crash(current->domain);
> +                    return X86EMUL_UNHANDLEABLE;
>                  }
>                  if ( rc != X86EMUL_OKAY )
>                      break;
> @@ -196,8 +196,20 @@ int hvm_process_io_intercept(const struc
>          }
>      }
> 
> -    if ( i != 0 && rc == X86EMUL_UNHANDLEABLE )
> -        domain_crash(current->domain);
> +    if ( i )
> +    {
> +        p->count = i;
> +        rc = X86EMUL_OKAY;
> +    }
> +    else if ( rc == X86EMUL_UNHANDLEABLE )
> +    {
> +        /*
> +         * Don't forward entire batches to the device model: This would
> +         * prevent the internal handlers to see subsequent iterations of
> +         * the request.
> +         */
> +        p->count = 1;
> +    }
> 
>      return rc;
>  }
> 


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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[jzh Chang]

[-- Attachment #1.1: Type: text/plain, Size: 5917 bytes --]

2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@suse.com>:

> Forwarding entire batches to the device model when an individual
> iteration of them got rejected by internal device emulation handlers
> with X86EMUL_UNHANDLEABLE is wrong: The device model would then handle
> all iterations, without the internal handler getting to see any past
> the one it returned failure for. This causes misbehavior in at least
> the MSI-X and VGA code, which want to see all such requests for
> internal tracking/caching purposes. But note that this does not apply
> to buffered I/O requests.
>
> This in turn means that the condition in hvm_process_io_intercept() of
> when to crash the domain was wrong: Since X86EMUL_UNHANDLEABLE can
> validly be returned by the individual device handlers, we mustn't
> blindly crash the domain if such occurs on other than the initial
> iteration. Instead we need to distinguish hvm_copy_*_guest_phys()
> failures from device specific ones, and then the former need to always
> be fatal to the domain (i.e. also on the first iteration), since
> otherwise we again would end up forwarding a request to qemu which the
> internal handler didn't get to see.
>
> The adjustment should be okay even for stdvga's MMIO handling:
> - if it is not caching then the accept function would have failed so we
>   won't get into hvm_process_io_intercept(),
> - if it issued the buffered ioreq then we only get to the p->count
>   reduction if hvm_send_ioreq() actually encountered an error (in which
>   we don't care about the request getting split up).
>
> Also commit 4faffc41d ("x86/hvm: limit reps to avoid the need to handle
> retry") went too far in removing code from hvm_process_io_intercept():
> When there were successfully handled iterations, the function should
> continue to return success with a clipped repeat count.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Chang Jianzhong <changjzh@gmail.com>
> ---
> v2: Make hvm_process_io_intercept() always return a valid X86EMUL_*
>     code by moving the domain_crash() invocations up. Add a stdvga
>     related paragraph to the commit message.
> ---
> I assume this also addresses the issue which
> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html
> attempted to deal with in a not really acceptable way.
>
> I hope this issue is resolved, but it still exists.

> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -95,7 +95,7 @@ static const struct hvm_io_handler null_
>  };
>
>  static int hvmemul_do_io(
> -    bool_t is_mmio, paddr_t addr, unsigned long reps, unsigned int size,
> +    bool_t is_mmio, paddr_t addr, unsigned long *reps, unsigned int size,
>      uint8_t dir, bool_t df, bool_t data_is_addr, uintptr_t data)
>  {
>      struct vcpu *curr = current;
> @@ -104,7 +104,7 @@ static int hvmemul_do_io(
>          .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO,
>          .addr = addr,
>          .size = size,
> -        .count = reps,
> +        .count = *reps,
>          .dir = dir,
>          .df = df,
>          .data = data,
> @@ -136,7 +136,7 @@ static int hvmemul_do_io(
>          if ( (p.type != is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO) ||
>               (p.addr != addr) ||
>               (p.size != size) ||
> -             (p.count != reps) ||
> +             (p.count != *reps) ||
>               (p.dir != dir) ||
>               (p.df != df) ||
>               (p.data_is_ptr != data_is_addr) )
> @@ -214,7 +214,7 @@ static int hvmemul_do_io_buffer(
>
>      BUG_ON(buffer == NULL);
>
> -    rc = hvmemul_do_io(is_mmio, addr, *reps, size, dir, df, 0,
> +    rc = hvmemul_do_io(is_mmio, addr, reps, size, dir, df, 0,
>                         (uintptr_t)buffer);
>      if ( rc == X86EMUL_UNHANDLEABLE && dir == IOREQ_READ )
>          memset(buffer, 0xff, size);
> @@ -305,13 +305,13 @@ static int hvmemul_do_io_addr(
>          count = 1;
>      }
>
> -    rc = hvmemul_do_io(is_mmio, addr, count, size, dir, df, 1,
> +    rc = hvmemul_do_io(is_mmio, addr, &count, size, dir, df, 1,
>                         ram_gpa);
> +
>      if ( rc == X86EMUL_OKAY )
> -    {
>          v->arch.hvm_vcpu.hvm_io.mmio_retry = (count < *reps);
> -        *reps = count;
> -    }
> +
> +    *reps = count;
>
>   out:
>      while ( nr_pages )
> --- a/xen/arch/x86/hvm/intercept.c
> +++ b/xen/arch/x86/hvm/intercept.c
> @@ -148,8 +148,8 @@ int hvm_process_io_intercept(const struc
>                      ASSERT_UNREACHABLE();
>                      /* fall through */
>                  default:
> -                    rc = X86EMUL_UNHANDLEABLE;
> -                    break;
> +                    domain_crash(current->domain);
> +                    return X86EMUL_UNHANDLEABLE;
>                  }
>                  if ( rc != X86EMUL_OKAY )
>                      break;
> @@ -178,8 +178,8 @@ int hvm_process_io_intercept(const struc
>                      ASSERT_UNREACHABLE();
>                      /* fall through */
>                  default:
> -                    rc = X86EMUL_UNHANDLEABLE;
> -                    break;
> +                    domain_crash(current->domain);
> +                    return X86EMUL_UNHANDLEABLE;
>                  }
>                  if ( rc != X86EMUL_OKAY )
>                      break;
> @@ -196,8 +196,20 @@ int hvm_process_io_intercept(const struc
>          }
>      }
>
> -    if ( i != 0 && rc == X86EMUL_UNHANDLEABLE )
> -        domain_crash(current->domain);
> +    if ( i )
> +    {
> +        p->count = i;
> +        rc = X86EMUL_OKAY;
> +    }
> +    else if ( rc == X86EMUL_UNHANDLEABLE )
> +    {
> +        /*
> +         * Don't forward entire batches to the device model: This would
> +         * prevent the internal handlers to see subsequent iterations of
> +         * the request.
> +         */
> +        p->count = 1;
> +    }
>
>      return rc;
>  }
>
>
>


-- 
Jianzhong,Chang

[-- Attachment #1.2: Type: text/html, Size: 7746 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Jan Beulich]

>>> On 30.03.16 at 09:28, <changjzh@gmail.com> wrote:
> 2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>> Forwarding entire batches to the device model when an individual
>> iteration of them got rejected by internal device emulation handlers
>> with X86EMUL_UNHANDLEABLE is wrong: The device model would then handle
>> all iterations, without the internal handler getting to see any past
>> the one it returned failure for. This causes misbehavior in at least
>> the MSI-X and VGA code, which want to see all such requests for
>> internal tracking/caching purposes. But note that this does not apply
>> to buffered I/O requests.
>>
>> This in turn means that the condition in hvm_process_io_intercept() of
>> when to crash the domain was wrong: Since X86EMUL_UNHANDLEABLE can
>> validly be returned by the individual device handlers, we mustn't
>> blindly crash the domain if such occurs on other than the initial
>> iteration. Instead we need to distinguish hvm_copy_*_guest_phys()
>> failures from device specific ones, and then the former need to always
>> be fatal to the domain (i.e. also on the first iteration), since
>> otherwise we again would end up forwarding a request to qemu which the
>> internal handler didn't get to see.
>>
>> The adjustment should be okay even for stdvga's MMIO handling:
>> - if it is not caching then the accept function would have failed so we
>>   won't get into hvm_process_io_intercept(),
>> - if it issued the buffered ioreq then we only get to the p->count
>>   reduction if hvm_send_ioreq() actually encountered an error (in which
>>   we don't care about the request getting split up).
>>
>> Also commit 4faffc41d ("x86/hvm: limit reps to avoid the need to handle
>> retry") went too far in removing code from hvm_process_io_intercept():
>> When there were successfully handled iterations, the function should
>> continue to return success with a clipped repeat count.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Chang Jianzhong <changjzh@gmail.com>
>> ---
>> v2: Make hvm_process_io_intercept() always return a valid X86EMUL_*
>>     code by moving the domain_crash() invocations up. Add a stdvga
>>     related paragraph to the commit message.
>> ---
>> I assume this also addresses the issue which
>> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html 
>> attempted to deal with in a not really acceptable way.
>>
>> I hope this issue is resolved, but it still exists.

Coming back to this: Have you made any progress with the analysis?

Jan


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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Jan Beulich]

(re-adding xen-devel)

>>> On 21.04.16 at 09:48, <changjzh@gmail.com> wrote:
> 2016-04-21 14:24 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
> 
>> >>> On 30.03.16 at 09:28, <changjzh@gmail.com> wrote:
>> > 2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>> >> Forwarding entire batches to the device model when an individual
>> >> iteration of them got rejected by internal device emulation handlers
>> >> with X86EMUL_UNHANDLEABLE is wrong: The device model would then handle
>> >> all iterations, without the internal handler getting to see any past
>> >> the one it returned failure for. This causes misbehavior in at least
>> >> the MSI-X and VGA code, which want to see all such requests for
>> >> internal tracking/caching purposes. But note that this does not apply
>> >> to buffered I/O requests.
>> >>
>> >> This in turn means that the condition in hvm_process_io_intercept() of
>> >> when to crash the domain was wrong: Since X86EMUL_UNHANDLEABLE can
>> >> validly be returned by the individual device handlers, we mustn't
>> >> blindly crash the domain if such occurs on other than the initial
>> >> iteration. Instead we need to distinguish hvm_copy_*_guest_phys()
>> >> failures from device specific ones, and then the former need to always
>> >> be fatal to the domain (i.e. also on the first iteration), since
>> >> otherwise we again would end up forwarding a request to qemu which the
>> >> internal handler didn't get to see.
>> >>
>> >> The adjustment should be okay even for stdvga's MMIO handling:
>> >> - if it is not caching then the accept function would have failed so we
>> >>   won't get into hvm_process_io_intercept(),
>> >> - if it issued the buffered ioreq then we only get to the p->count
>> >>   reduction if hvm_send_ioreq() actually encountered an error (in which
>> >>   we don't care about the request getting split up).
>> >>
>> >> Also commit 4faffc41d ("x86/hvm: limit reps to avoid the need to handle
>> >> retry") went too far in removing code from hvm_process_io_intercept():
>> >> When there were successfully handled iterations, the function should
>> >> continue to return success with a clipped repeat count.
>> >>
>> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> >> Cc: Chang Jianzhong <changjzh@gmail.com>
>> >> ---
>> >> v2: Make hvm_process_io_intercept() always return a valid X86EMUL_*
>> >>     code by moving the domain_crash() invocations up. Add a stdvga
>> >>     related paragraph to the commit message.
>> >> ---
>> >> I assume this also addresses the issue which
>> >>
>> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html 
>> >> attempted to deal with in a not really acceptable way.
>> >>
>> >> I hope this issue is resolved, but it still exists.
>>
>> Coming back to this: Have you made any progress with the analysis?
>>
> Win-guest xen log message：
> hvmemul_do_io() {
>   ...
>   rc = hvm_io_intercept(&p);
>   // rc == 1 is printed in log
>   ...
> }
> 
>  int hvm_io_intercept(ioreq_t *p){
>  ...
>    handler = hvm_find_io_handler(p);// handler == NULL is printed in log
> 
>    if ( handler == NULL ){
>          return X86EMUL_UNHANDLEABLE;
>      }
> 
>     rc = hvm_process_io_intercept(handler, p);// This function is not
> called.
>  ...
> }
> 
> hvm_find_io_handler(ioreq_t *p)
> -> hvm_mmio_accept(...)
> -> msixtbl_range(...)
> msixtbl_entry can not be found.
> 
> Windows guest try to write msixtbl before it is registed
>  (msixtbl_pt_register()).
> Win-guest do not write msixtbl after masixtble is registed.
> This situation may be the root cause.

Ah, but that's again an issue a patch was posted for already
http://lists.xenproject.org/archives/html/xen-devel/2016-04/msg00041.html
(albeit, as said there, not taking care of all possible cases yet).
And it's a pre-existing issue, not one introduced by any half
way recent change.

Jan

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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Jan Beulich]

>>> On 30.03.16 at 09:28, <changjzh@gmail.com> wrote:
> 2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>> ---
>> I assume this also addresses the issue which
>> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html 
>> attempted to deal with in a not really acceptable way.
>
> I hope this issue is resolved, but it still exists.

Mind giving the small below patch a try?

Jan

--- unstable.orig/xen/arch/x86/msi.c
+++ unstable/xen/arch/x86/msi.c
@@ -430,8 +430,13 @@ static bool_t msi_set_mask_bit(struct ir
         {
             writel(flag, entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
             readl(entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
+
             if ( likely(control & PCI_MSIX_FLAGS_ENABLE) )
                 break;
+
+            entry->msi_attrib.host_masked = host;
+            entry->msi_attrib.guest_masked = guest;
+
             flag = 1;
         }
         else if ( flag && !(control & PCI_MSIX_FLAGS_MASKALL) )



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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[jzh Chang]

[-- Attachment #1.1: Type: text/plain, Size: 1885 bytes --]

2016-04-21 22:19 GMT+08:00 Jan Beulich <JBeulich@suse.com>:

> >>> On 30.03.16 at 09:28, <changjzh@gmail.com> wrote:
> > 2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
> >> ---
> >> I assume this also addresses the issue which
> >>
> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html
> >> attempted to deal with in a not really acceptable way.
> >
> > I hope this issue is resolved, but it still exists.
>
> Mind giving the small below patch a try?
>
> Jan
>
> --- unstable.orig/xen/arch/x86/msi.c
> +++ unstable/xen/arch/x86/msi.c
> @@ -430,8 +430,13 @@ static bool_t msi_set_mask_bit(struct ir
>          {
>              writel(flag, entry->mask_base +
> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
>              readl(entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> +
>              if ( likely(control & PCI_MSIX_FLAGS_ENABLE) )
>                  break;
> +
> +            entry->msi_attrib.host_masked = host;
> +            entry->msi_attrib.guest_masked = guest;
> +
>              flag = 1;
>          }
>          else if ( flag && !(control & PCI_MSIX_FLAGS_MASKALL) )
>
>
> The issue still exist. But, the host_masked is changed.
guest_masked can be changed by guest_mask_msi_irq() function.
The function is not called as previous e-mail analysis.

No patch xen log message:
(XEN)  MSI-X  114 vec=73 lowest  edge   assert  log lowest dest=00000555
mask=1/HG/1
(XEN)  MSI-X  115 vec=7b lowest  edge   assert  log lowest dest=00000555
mask=1/HG/1
(XEN)  MSI-X  116 vec=83 lowest  edge   assert  log lowest dest=00000555
mask=1/HG/1

Patched-xen log message :
(XEN)  MSI-X  114 vec=76 lowest  edge   assert  log lowest dest=00000555
mask=1/ G/1
(XEN)  MSI-X  115 vec=7e lowest  edge   assert  log lowest dest=00000555
mask=1/ G/1
(XEN)  MSI-X  116 vec=86 lowest  edge   assert  log lowest dest=00000555
mask=1/ G/1

-- 
Jianzhong,Chang

[-- Attachment #1.2: Type: text/html, Size: 3190 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Jan Beulich]

>>> On 22.04.16 at 04:02, <changjzh@gmail.com> wrote:
> 2016-04-21 22:19 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
> 
>> >>> On 30.03.16 at 09:28, <changjzh@gmail.com> wrote:
>> > 2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
>> >> ---
>> >> I assume this also addresses the issue which
>> >>
>> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.html 
>> >> attempted to deal with in a not really acceptable way.
>> >
>> > I hope this issue is resolved, but it still exists.
>>
>> Mind giving the small below patch a try?
>>
>> Jan
>>
>> --- unstable.orig/xen/arch/x86/msi.c
>> +++ unstable/xen/arch/x86/msi.c
>> @@ -430,8 +430,13 @@ static bool_t msi_set_mask_bit(struct ir
>>          {
>>              writel(flag, entry->mask_base +
>> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
>>              readl(entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
>> +
>>              if ( likely(control & PCI_MSIX_FLAGS_ENABLE) )
>>                  break;
>> +
>> +            entry->msi_attrib.host_masked = host;
>> +            entry->msi_attrib.guest_masked = guest;
>> +
>>              flag = 1;
>>          }
>>          else if ( flag && !(control & PCI_MSIX_FLAGS_MASKALL) )
>>
>>
>> The issue still exist. But, the host_masked is changed.

At least something.

> guest_masked can be changed by guest_mask_msi_irq() function.
> The function is not called as previous e-mail analysis.

I have to admit that I had quite a bit of trouble understanding that
previous patch of yours. The function not being called of course
needs to be understood, which requires a trace of the writes of the
guest to the vector control field(s), including the ones before the
MSI-X region gets registered. Just to double check - was this latest
try with the other patch also in place, or just the small one I had
sent yesterday?

I do have a debugging patch around for the necessary logging to
get added, but that's against quite a bit older a hypervisor version,
so likely would serve only as a reference. Let me know if you would
still like me to hand that to you.

Jan

> No patch xen log message:
> (XEN)  MSI-X  114 vec=73 lowest  edge   assert  log lowest dest=00000555
> mask=1/HG/1
> (XEN)  MSI-X  115 vec=7b lowest  edge   assert  log lowest dest=00000555
> mask=1/HG/1
> (XEN)  MSI-X  116 vec=83 lowest  edge   assert  log lowest dest=00000555
> mask=1/HG/1
> 
> Patched-xen log message :
> (XEN)  MSI-X  114 vec=76 lowest  edge   assert  log lowest dest=00000555
> mask=1/ G/1
> (XEN)  MSI-X  115 vec=7e lowest  edge   assert  log lowest dest=00000555
> mask=1/ G/1
> (XEN)  MSI-X  116 vec=86 lowest  edge   assert  log lowest dest=00000555
> mask=1/ G/1
> 
> -- 
> Jianzhong,Chang




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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Li, Liang Z]

> >> >>> On 30.03.16 at 09:28, <changjzh@gmail.com> wrote:
> >> > 2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
> >> >> ---
> >> >> I assume this also addresses the issue which
> >> >>
> >> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg03189.
> >> html
> >> >> attempted to deal with in a not really acceptable way.
> >> >
> >> > I hope this issue is resolved, but it still exists.
> >>
> >> Mind giving the small below patch a try?
> >>
> >> Jan
> >>
> >> --- unstable.orig/xen/arch/x86/msi.c
> >> +++ unstable/xen/arch/x86/msi.c
> >> @@ -430,8 +430,13 @@ static bool_t msi_set_mask_bit(struct ir
> >>          {
> >>              writel(flag, entry->mask_base +
> >> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> >>              readl(entry->mask_base +
> >> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> >> +
> >>              if ( likely(control & PCI_MSIX_FLAGS_ENABLE) )
> >>                  break;
> >> +
> >> +            entry->msi_attrib.host_masked = host;
> >> +            entry->msi_attrib.guest_masked = guest;
> >> +
> >>              flag = 1;
> >>          }
> >>          else if ( flag && !(control & PCI_MSIX_FLAGS_MASKALL) )
> >>
> >>
> >> The issue still exist. But, the host_masked is changed.
> 
> At least something.
> 
> > guest_masked can be changed by guest_mask_msi_irq() function.
> > The function is not called as previous e-mail analysis.
> 
> I have to admit that I had quite a bit of trouble understanding that previous
> patch of yours. The function not being called of course needs to be
> understood, which requires a trace of the writes of the guest to the vector
> control field(s), including the ones before the MSI-X region gets registered.
> Just to double check - was this latest try with the other patch also in place, or
> just the small one I had sent yesterday?
> 

With other patches also in place, still not work. 
Jianzhong  has been left and Quan will take over the task.

Liang

> I do have a debugging patch around for the necessary logging to get added,
> but that's against quite a bit older a hypervisor version, so likely would serve
> only as a reference. Let me know if you would still like me to hand that to you.
> 
> Jan
> 
> > No patch xen log message:
> > (XEN)  MSI-X  114 vec=73 lowest  edge   assert  log lowest dest=00000555
> > mask=1/HG/1
> > (XEN)  MSI-X  115 vec=7b lowest  edge   assert  log lowest dest=00000555
> > mask=1/HG/1
> > (XEN)  MSI-X  116 vec=83 lowest  edge   assert  log lowest dest=00000555
> > mask=1/HG/1
> >
> > Patched-xen log message :
> > (XEN)  MSI-X  114 vec=76 lowest  edge   assert  log lowest dest=00000555
> > mask=1/ G/1
> > (XEN)  MSI-X  115 vec=7e lowest  edge   assert  log lowest dest=00000555
> > mask=1/ G/1
> > (XEN)  MSI-X  116 vec=86 lowest  edge   assert  log lowest dest=00000555
> > mask=1/ G/1
> >
> > --
> > Jianzhong,Chang
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Xu, Quan]

On April 25, 2016 4:41pm, Li, Liang Z <liang.z.li@intel.com> wrote:
> > >> >>> On 30.03.16 at 09:28, <changjzh@gmail.com> wrote:
> > >> > 2016-03-29 18:39 GMT+08:00 Jan Beulich <JBeulich@suse.com>:
> > >> >> ---
> > >> >> I assume this also addresses the issue which
> > >> >>
> > >> http://lists.xenproject.org/archives/html/xen-devel/2016-
> 01/msg03189.
> > >> html
> > >> >> attempted to deal with in a not really acceptable way.
> > >> >
> > >> > I hope this issue is resolved, but it still exists.
> > >>
> > >> Mind giving the small below patch a try?
> > >>
> > >> Jan
> > >>
> > >> --- unstable.orig/xen/arch/x86/msi.c
> > >> +++ unstable/xen/arch/x86/msi.c
> > >> @@ -430,8 +430,13 @@ static bool_t msi_set_mask_bit(struct ir
> > >>          {
> > >>              writel(flag, entry->mask_base +
> > >> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> > >>              readl(entry->mask_base +
> > >> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> > >> +
> > >>              if ( likely(control & PCI_MSIX_FLAGS_ENABLE) )
> > >>                  break;
> > >> +
> > >> +            entry->msi_attrib.host_masked = host;
> > >> +            entry->msi_attrib.guest_masked = guest;
> > >> +
> > >>              flag = 1;
> > >>          }
> > >>          else if ( flag && !(control & PCI_MSIX_FLAGS_MASKALL) )
> > >>
> > >>
> > >> The issue still exist. But, the host_masked is changed.
> >
> > At least something.
> >
> > > guest_masked can be changed by guest_mask_msi_irq() function.
> > > The function is not called as previous e-mail analysis.
> >
> > I have to admit that I had quite a bit of trouble understanding that
> > previous patch of yours. The function not being called of course needs
> > to be understood, which requires a trace of the writes of the guest to
> > the vector control field(s), including the ones before the MSI-X region gets
> registered.
> > Just to double check - was this latest try with the other patch also
> > in place, or just the small one I had sent yesterday?
> >
> 
> With other patches also in place, still not work.
> Jianzhong  has been left and Quan will take over the task.
>

Yes, I am just getting warmed up..:)
Quan


> Liang
> 
> > I do have a debugging patch around for the necessary logging to get
> > added, but that's against quite a bit older a hypervisor version, so
> > likely would serve only as a reference. Let me know if you would still like me
> to hand that to you.
> >
> > Jan
> >
> > > No patch xen log message:
> > > (XEN)  MSI-X  114 vec=73 lowest  edge   assert  log lowest dest=00000555
> > > mask=1/HG/1
> > > (XEN)  MSI-X  115 vec=7b lowest  edge   assert  log lowest dest=00000555
> > > mask=1/HG/1
> > > (XEN)  MSI-X  116 vec=83 lowest  edge   assert  log lowest dest=00000555
> > > mask=1/HG/1
> > >
> > > Patched-xen log message :
> > > (XEN)  MSI-X  114 vec=76 lowest  edge   assert  log lowest dest=00000555
> > > mask=1/ G/1
> > > (XEN)  MSI-X  115 vec=7e lowest  edge   assert  log lowest dest=00000555
> > > mask=1/ G/1
> > > (XEN)  MSI-X  116 vec=86 lowest  edge   assert  log lowest dest=00000555
> > > mask=1/ G/1
> > >
> > > --
> > > Jianzhong,Chang

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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Jan Beulich]

>>> On 25.04.16 at 10:40, <liang.z.li@intel.com> wrote:
> With other patches also in place, still not work. 
> Jianzhong  has been left and Quan will take over the task.

May I ask for another try, with current tip of staging plus
http://lists.xenproject.org/archives/html/xen-devel/2016-04/msg03661.html
?

Thanks, Jan


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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Xu, Quan]

On April 28, 2016 11:13 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 25.04.16 at 10:40, <liang.z.li@intel.com> wrote:
> > With other patches also in place, still not work.
> > Jianzhong  has been left and Quan will take over the task.
> 
> May I ask for another try, with current tip of staging plus
> http://lists.xenproject.org/archives/html/xen-devel/2016-
> 04/msg03661.html
> ?


Sure, I will try it on tomorrow afternoon or next Tuesday.
Quan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Xu, Quan]

On April 29, 2016 12:11 AM, <quan.xu@intel.com> wrote:
> On April 28, 2016 11:13 PM, Jan Beulich <JBeulich@suse.com> wrote:
> > >>> On 25.04.16 at 10:40, <liang.z.li@intel.com> wrote:
> > > With other patches also in place, still not work.
> > > Jianzhong  has been left and Quan will take over the task.
> >
> > May I ask for another try, with current tip of staging plus
> > http://lists.xenproject.org/archives/html/xen-devel/2016-
> > 04/msg03661.html
> > ?
> 
> 
> Sure, I will try it on tomorrow afternoon or next Tuesday.

Jan,
Sorry, this is still working in progress.
 The original env is broken. I am rebuilding a new env, with some problems of Windows image.
I checked the staging branch, and I found your patches are in staging branch now,
 so I am just going to test staging branch. Right?

Quan


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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Jan Beulich]

>>> On 03.05.16 at 14:50, <quan.xu@intel.com> wrote:
> On April 29, 2016 12:11 AM, <quan.xu@intel.com> wrote:
>> On April 28, 2016 11:13 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> > >>> On 25.04.16 at 10:40, <liang.z.li@intel.com> wrote:
>> > > With other patches also in place, still not work.
>> > > Jianzhong  has been left and Quan will take over the task.
>> >
>> > May I ask for another try, with current tip of staging plus
>> > http://lists.xenproject.org/archives/html/xen-devel/2016- 
>> > 04/msg03661.html
>> > ?
>> 
>> 
>> Sure, I will try it on tomorrow afternoon or next Tuesday.
> 
> Jan,
> Sorry, this is still working in progress.
>  The original env is broken. I am rebuilding a new env, with some problems 
> of Windows image.
> I checked the staging branch, and I found your patches are in staging branch 
> now,
>  so I am just going to test staging branch. Right?

Yes.


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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Xu, Quan]

On May 03, 2016 9:39 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 03.05.16 at 14:50, <quan.xu@intel.com> wrote:
> > On April 29, 2016 12:11 AM, <quan.xu@intel.com> wrote:
> >> On April 28, 2016 11:13 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >> > >>> On 25.04.16 at 10:40, <liang.z.li@intel.com> wrote:
> >> > > With other patches also in place, still not work.
> >> > > Jianzhong  has been left and Quan will take over the task.
> >> >
> >> > May I ask for another try, with current tip of staging plus
> >> > http://lists.xenproject.org/archives/html/xen-devel/2016-
> >> > 04/msg03661.html
> >> > ?
> >>
> >>
> >> Sure, I will try it on tomorrow afternoon or next Tuesday.
> >
> > Jan,
> > Sorry, this is still working in progress.
> >  The original env is broken. I am rebuilding a new env, with some
> > problems of Windows image.
> > I checked the staging branch, and I found your patches are in staging
> > branch now,  so I am just going to test staging branch. Right?
> 
> Yes.

Jan,
The VF is still _not_ working for Windows_Server2008, but the PF does.
My nic is  'Intel Corporation I350 Gigabit Network Connection',
and the last commit is 'c2994f86'.
I also tested the RHEL6 VM, the same result as Windows_Server2008.

I'll also go into this issue.
Any update, let me help you test it.

Quan

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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Xu, Quan]

On April 28, 2016 11:13 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 25.04.16 at 10:40, <liang.z.li@intel.com> wrote:
> > With other patches also in place, still not work.
> > Jianzhong  has been left and Quan will take over the task.
> 
> May I ask for another try, with current tip of staging plus
> http://lists.xenproject.org/archives/html/xen-devel/2016-
> 04/msg03661.html
> ?

Jan,

The same issue for rhel 6.4VM,  which cannot initialize VF driver too.. the below is log  of rhel 6.4 VM:
             .. 
             igbvf: 0000:00.04.0: Invalid MAC Address: 00:00:00:00:00:00
             igbvf: probe of 0000:00.04.0 failed with error -5
             ..

But when I tried to initialize VF MAC in Dom0 with e.g.:
                  $ip link set eth0 vf 0 mac 00:1E:67:65:93:01

Then to create rhel 6.4 VM again, the VF is working for rhel 6.4 VM.

I will ask QA team to help me verify it on win2k8 guest (I'm not sure whether the win2k8 network driver is working or not).

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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Jan Beulich]

>>> On 19.05.16 at 10:30, <quan.xu@intel.com> wrote:
> On April 28, 2016 11:13 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> >>> On 25.04.16 at 10:40, <liang.z.li@intel.com> wrote:
>> > With other patches also in place, still not work.
>> > Jianzhong  has been left and Quan will take over the task.
>> 
>> May I ask for another try, with current tip of staging plus
>> http://lists.xenproject.org/archives/html/xen-devel/2016- 
>> 04/msg03661.html
>> ?
> 
> The same issue for rhel 6.4VM,  which cannot initialize VF driver too.. the 
> below is log  of rhel 6.4 VM:
>              .. 
>              igbvf: 0000:00.04.0: Invalid MAC Address: 00:00:00:00:00:00
>              igbvf: probe of 0000:00.04.0 failed with error -5
>              ..
> 
> But when I tried to initialize VF MAC in Dom0 with e.g.:
>                   $ip link set eth0 vf 0 mac 00:1E:67:65:93:01

Makes things even more strange, as things work fine for me with
SLE11 and SLE12 guests. But I have to admit that the "Invalid MAC
Address" looks quite unrelated, i.e. is this perhaps some completely
different problem you have?

Again, considering that you have a repro (while I don't), the
easiest would be to simply log all MSI-X related actions (there
shouldn't be too many), to see where things go wrong. Such a
log alone would maybe already allow me to do further
investigation.

Jan


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

Re: [PATCH v2] x86/HVM: fix forwarding of internally cached requests

[Xu, Quan]

On May 19, 2016 5:15 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 19.05.16 at 10:30, <quan.xu@intel.com> wrote:
> > On April 28, 2016 11:13 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >> >>> On 25.04.16 at 10:40, <liang.z.li@intel.com> wrote:
> >> > With other patches also in place, still not work.
> >> > Jianzhong  has been left and Quan will take over the task.
> >>
> >> May I ask for another try, with current tip of staging plus
> >> http://lists.xenproject.org/archives/html/xen-devel/2016-
> >> 04/msg03661.html
> >> ?
> >
> > The same issue for rhel 6.4VM,  which cannot initialize VF driver
> > too.. the below is log  of rhel 6.4 VM:
> >              ..
> >              igbvf: 0000:00.04.0: Invalid MAC Address: 00:00:00:00:00:00
> >              igbvf: probe of 0000:00.04.0 failed with error -5
> >              ..
> >
> > But when I tried to initialize VF MAC in Dom0 with e.g.:
> >                   $ip link set eth0 vf 0 mac 00:1E:67:65:93:01
> 
> Makes things even more strange, as things work fine for me with
> SLE11 and SLE12 guests. But I have to admit that the "Invalid MAC Address"
> looks quite unrelated, i.e. is this perhaps some completely different problem
> you have?

I tried to run SLE12 guest. Things are really working fine for me too..
But not for rhel 6.4 guest..

So far, I think the bug is not from xen hypervisor, just from vf driver.

Look at this bug, even from KVM --- igb VF can't work in KVM guest, https://bugzilla.kernel.org/show_bug.cgi?id=55421
 
> 
> Again, considering that you have a repro (while I don't), the easiest would be
> to simply log all MSI-X related actions (there shouldn't be too many), to see
> where things go wrong. Such a log alone would maybe already allow me to do
> further investigation.
> 

I need more time to read this part, but sure I will follow it.

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