[Xen-devel] [PATCH v7 06/11] x86, paravirt: Add interface to support kvm/xen vcpu preempted check

[Pan Xinhui]

This is to fix some lock holder preemption issues. Some other locks
implementation do a spin loop before acquiring the lock itself.
Currently kernel has an interface of bool vcpu_is_preempted(int cpu). It
takes the cpu as parameter and return true if the cpu is preempted.
Then kernel can break the spin loops upon the retval of
vcpu_is_preempted.

As kernel has used this interface, So lets support it.

To deal with kernel and kvm/xen, add vcpu_is_preempted into struct
pv_lock_ops.

Then kvm or xen could provide their own implementation to support
vcpu_is_preempted.

Signed-off-by: Pan Xinhui 
Acked-by: Paolo Bonzini 
---
 arch/x86/include/asm/paravirt_types.h | 2 ++
 arch/x86/include/asm/spinlock.h   | 8 
 arch/x86/kernel/paravirt-spinlocks.c  | 6 ++
 3 files changed, 16 insertions(+)

diff --git a/arch/x86/include/asm/paravirt_types.h 
b/arch/x86/include/asm/paravirt_types.h
index 0f400c0..38c3bb7 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -310,6 +310,8 @@ struct pv_lock_ops {
 
void (*wait)(u8 *ptr, u8 val);
void (*kick)(int cpu);
+
+   bool (*vcpu_is_preempted)(int cpu);
 };
 
 /* This contains all the paravirt structures: we get a convenient
diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 921bea7..0526f59 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -26,6 +26,14 @@
 extern struct static_key paravirt_ticketlocks_enabled;
 static __always_inline bool static_key_false(struct static_key *key);
 
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+#define vcpu_is_preempted vcpu_is_preempted
+static inline bool vcpu_is_preempted(int cpu)
+{
+   return pv_lock_ops.vcpu_is_preempted(cpu);
+}
+#endif
+
 #include 
 
 /*
diff --git a/arch/x86/kernel/paravirt-spinlocks.c 
b/arch/x86/kernel/paravirt-spinlocks.c
index 2c55a00..2f204dd 100644
--- a/arch/x86/kernel/paravirt-spinlocks.c
+++ b/arch/x86/kernel/paravirt-spinlocks.c
@@ -21,12 +21,18 @@ bool pv_is_native_spin_unlock(void)
__raw_callee_save___native_queued_spin_unlock;
 }
 
+static bool native_vcpu_is_preempted(int cpu)
+{
+   return 0;
+}
+
 struct pv_lock_ops pv_lock_ops = {
 #ifdef CONFIG_SMP
.queued_spin_lock_slowpath = native_queued_spin_lock_slowpath,
.queued_spin_unlock = PV_CALLEE_SAVE(__native_queued_spin_unlock),
.wait = paravirt_nop,
.kick = paravirt_nop,
+   .vcpu_is_preempted = native_vcpu_is_preempted,
 #endif /* SMP */
 };
 EXPORT_SYMBOL(pv_lock_ops);
-- 
2.4.11


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

[Xen-devel] [PATCH v7 07/11] KVM: Introduce kvm_write_guest_offset_cached

[Pan Xinhui]

It allows us to update some status or field of one struct partially.

We can also save one kvm_read_guest_cached if we just update one filed
of the struct regardless of its current value.

Signed-off-by: Pan Xinhui 
Acked-by: Paolo Bonzini 
---
 include/linux/kvm_host.h |  2 ++
 virt/kvm/kvm_main.c  | 20 ++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 01c0b9c..6f00237 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -645,6 +645,8 @@ int kvm_write_guest(struct kvm *kvm, gpa_t gpa, const void 
*data,
unsigned long len);
 int kvm_write_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
   void *data, unsigned long len);
+int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache 
*ghc,
+  void *data, int offset, unsigned long len);
 int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
  gpa_t gpa, unsigned long len);
 int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2907b7b..95308ee 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1972,30 +1972,38 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct 
gfn_to_hva_cache *ghc,
 }
 EXPORT_SYMBOL_GPL(kvm_gfn_to_hva_cache_init);
 
-int kvm_write_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
-  void *data, unsigned long len)
+int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache 
*ghc,
+  void *data, int offset, unsigned long len)
 {
struct kvm_memslots *slots = kvm_memslots(kvm);
int r;
+   gpa_t gpa = ghc->gpa + offset;
 
-   BUG_ON(len > ghc->len);
+   BUG_ON(len + offset > ghc->len);
 
if (slots->generation != ghc->generation)
kvm_gfn_to_hva_cache_init(kvm, ghc, ghc->gpa, ghc->len);
 
if (unlikely(!ghc->memslot))
-   return kvm_write_guest(kvm, ghc->gpa, data, len);
+   return kvm_write_guest(kvm, gpa, data, len);
 
if (kvm_is_error_hva(ghc->hva))
return -EFAULT;
 
-   r = __copy_to_user((void __user *)ghc->hva, data, len);
+   r = __copy_to_user((void __user *)ghc->hva + offset, data, len);
if (r)
return -EFAULT;
-   mark_page_dirty_in_slot(ghc->memslot, ghc->gpa >> PAGE_SHIFT);
+   mark_page_dirty_in_slot(ghc->memslot, gpa >> PAGE_SHIFT);
 
return 0;
 }
+EXPORT_SYMBOL_GPL(kvm_write_guest_offset_cached);
+
+int kvm_write_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
+  void *data, unsigned long len)
+{
+   return kvm_write_guest_offset_cached(kvm, ghc, data, 0, len);
+}
 EXPORT_SYMBOL_GPL(kvm_write_guest_cached);
 
 int kvm_read_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
-- 
2.4.11


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

[Xen-devel] [PATCH v7 11/11] Documentation: virtual: kvm: Support vcpu preempted check

[Pan Xinhui]

Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
preempted" into struct kvm_steal_time. This field tells if one vcpu is
running or not.

It is zero if 1) some old KVM deos not support this filed. 2) the vcpu
is not preempted. Other values means the vcpu has been preempted.

Signed-off-by: Pan Xinhui 
Acked-by: Radim Krčmář 
Acked-by: Paolo Bonzini 
---
 Documentation/virtual/kvm/msr.txt | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/msr.txt 
b/Documentation/virtual/kvm/msr.txt
index 2a71c8f..ab2ab76 100644
--- a/Documentation/virtual/kvm/msr.txt
+++ b/Documentation/virtual/kvm/msr.txt
@@ -208,7 +208,9 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
__u64 steal;
__u32 version;
__u32 flags;
-   __u32 pad[12];
+   __u8  preempted;
+   __u8  u8_pad[3];
+   __u32 pad[11];
}
 
whose data will be filled in by the hypervisor periodically. Only one
@@ -232,6 +234,11 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
nanoseconds. Time during which the vcpu is idle, will not be
reported as steal time.
 
+   preempted: indicate the VCPU who owns this struct is running or
+   not. Non-zero values mean the VCPU has been preempted. Zero
+   means the VCPU is not preempted. NOTE, it is always zero if the
+   the hypervisor doesn't support this field.
+
 MSR_KVM_EOI_EN: 0x4b564d04
data: Bit 0 is 1 when PV end of interrupt is enabled on the vcpu; 0
when disabled.  Bit 1 is reserved and must be zero.  When PV end of
-- 
2.4.11


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

[Xen-devel] [PATCH v7 09/11] x86, kernel/kvm.c: support vcpu preempted check

[Pan Xinhui]

Support the vcpu_is_preempted() functionality under KVM. This will
enhance lock performance on overcommitted hosts (more runnable vcpus
than physical cpus in the system) as doing busy waits for preempted
vcpus will hurt system performance far worse than early yielding.

struct kvm_steal_time::preempted indicate that if one vcpu is running or
not after commit("x86, kvm/x86.c: support vcpu preempted check").

unix benchmark result:
host:  kernel 4.8.1, i5-4570, 4 cpus
guest: kernel 4.8.1, 8 vcpus

test-case   after-patch   before-patch
Execl Throughput   |18307.9 lps  |11701.6 lps
File Copy 1024 bufsize 2000 maxblocks  |  1352407.3 KBps |   790418.9 KBps
File Copy 256 bufsize 500 maxblocks|   367555.6 KBps |   222867.7 KBps
File Copy 4096 bufsize 8000 maxblocks  |  3675649.7 KBps |  1780614.4 KBps
Pipe Throughput| 11872208.7 lps  | 11855628.9 lps
Pipe-based Context Switching   |  1495126.5 lps  |  1490533.9 lps
Process Creation   |29881.2 lps  |28572.8 lps
Shell Scripts (1 concurrent)   |23224.3 lpm  |22607.4 lpm
Shell Scripts (8 concurrent)   | 3531.4 lpm  | 3211.9 lpm
System Call Overhead   | 10385653.0 lps  | 10419979.0 lps

Signed-off-by: Pan Xinhui 
Acked-by: Paolo Bonzini 
---
 arch/x86/kernel/kvm.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index edbbfc8..0b48dd2 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -415,6 +415,15 @@ void kvm_disable_steal_time(void)
wrmsr(MSR_KVM_STEAL_TIME, 0, 0);
 }
 
+static bool kvm_vcpu_is_preempted(int cpu)
+{
+   struct kvm_steal_time *src;
+
+   src = _cpu(steal_time, cpu);
+
+   return !!src->preempted;
+}
+
 #ifdef CONFIG_SMP
 static void __init kvm_smp_prepare_boot_cpu(void)
 {
@@ -471,6 +480,9 @@ void __init kvm_guest_init(void)
if (kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
has_steal_clock = 1;
pv_time_ops.steal_clock = kvm_steal_clock;
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+   pv_lock_ops.vcpu_is_preempted = kvm_vcpu_is_preempted;
+#endif
}
 
if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
-- 
2.4.11


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

[Xen-devel] [PATCH v7 08/11] x86, kvm/x86.c: support vcpu preempted check

[Pan Xinhui]

Support the vcpu_is_preempted() functionality under KVM. This will
enhance lock performance on overcommitted hosts (more runnable vcpus
than physical cpus in the system) as doing busy waits for preempted
vcpus will hurt system performance far worse than early yielding.

Use one field of struct kvm_steal_time ::preempted to indicate that if
one vcpu is running or not.

Signed-off-by: Pan Xinhui 
Acked-by: Paolo Bonzini 
---
 arch/x86/include/uapi/asm/kvm_para.h |  4 +++-
 arch/x86/kvm/x86.c   | 16 
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/kvm_para.h 
b/arch/x86/include/uapi/asm/kvm_para.h
index 94dc8ca..1421a65 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -45,7 +45,9 @@ struct kvm_steal_time {
__u64 steal;
__u32 version;
__u32 flags;
-   __u32 pad[12];
+   __u8  preempted;
+   __u8  u8_pad[3];
+   __u32 pad[11];
 };
 
 #define KVM_STEAL_ALIGNMENT_BITS 5
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e375235..f06e115 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2057,6 +2057,8 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
>arch.st.steal, sizeof(struct kvm_steal_time
return;
 
+   vcpu->arch.st.steal.preempted = 0;
+
if (vcpu->arch.st.steal.version & 1)
vcpu->arch.st.steal.version += 1;  /* first time write, random 
junk */
 
@@ -2810,8 +2812,22 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
kvm_make_request(KVM_REQ_STEAL_UPDATE, vcpu);
 }
 
+static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu)
+{
+   if (!(vcpu->arch.st.msr_val & KVM_MSR_ENABLED))
+   return;
+
+   vcpu->arch.st.steal.preempted = 1;
+
+   kvm_write_guest_offset_cached(vcpu->kvm, >arch.st.stime,
+   >arch.st.steal.preempted,
+   offsetof(struct kvm_steal_time, preempted),
+   sizeof(vcpu->arch.st.steal.preempted));
+}
+
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
+   kvm_steal_time_set_preempted(vcpu);
kvm_x86_ops->vcpu_put(vcpu);
kvm_put_guest_fpu(vcpu);
vcpu->arch.last_host_tsc = rdtsc();
-- 
2.4.11


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

[Xen-devel] [PATCH v7 10/11] x86, xen: support vcpu preempted check

[Pan Xinhui]

From: Juergen Gross 

Support the vcpu_is_preempted() functionality under Xen. This will
enhance lock performance on overcommitted hosts (more runnable vcpus
than physical cpus in the system) as doing busy waits for preempted
vcpus will hurt system performance far worse than early yielding.

A quick test (4 vcpus on 1 physical cpu doing a parallel build job
with "make -j 8") reduced system time by about 5% with this patch.

Signed-off-by: Juergen Gross 
Signed-off-by: Pan Xinhui 
---
 arch/x86/xen/spinlock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 3d6e006..74756bb 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -114,7 +114,6 @@ void xen_uninit_lock_cpu(int cpu)
per_cpu(irq_name, cpu) = NULL;
 }
 
-
 /*
  * Our init of PV spinlocks is split in two init functions due to us
  * using paravirt patching and jump labels patching and having to do
@@ -137,6 +136,8 @@ void __init xen_init_spinlocks(void)
pv_lock_ops.queued_spin_unlock = 
PV_CALLEE_SAVE(__pv_queued_spin_unlock);
pv_lock_ops.wait = xen_qlock_wait;
pv_lock_ops.kick = xen_qlock_kick;
+
+   pv_lock_ops.vcpu_is_preempted = xen_vcpu_stolen;
 }
 
 /*
-- 
2.4.11


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

[Xen-devel] [PATCH v7 02/11] locking/osq: Drop the overload of osq_lock()

[Pan Xinhui]

An over-committed guest with more vCPUs than pCPUs has a heavy overload
in osq_lock().

This is because vCPU A hold the osq lock and yield out, vCPU B wait
per_cpu node->locked to be set. IOW, vCPU B wait vCPU A to run and
unlock the osq lock.

Kernel has an interface bool vcpu_is_preempted(int cpu) to detect if a
vCPU is currently running or not. So break the spin loops on true
condition.

test case:
perf record -a perf bench sched messaging -g 400 -p && perf report

before patch:
18.09%  sched-messaging  [kernel.vmlinux]  [k] osq_lock
12.28%  sched-messaging  [kernel.vmlinux]  [k] rwsem_spin_on_owner
 5.27%  sched-messaging  [kernel.vmlinux]  [k] mutex_unlock
 3.89%  sched-messaging  [kernel.vmlinux]  [k] wait_consider_task
 3.64%  sched-messaging  [kernel.vmlinux]  [k] _raw_write_lock_irq
 3.41%  sched-messaging  [kernel.vmlinux]  [k] mutex_spin_on_owner.is
 2.49%  sched-messaging  [kernel.vmlinux]  [k] system_call

after patch:
20.68%  sched-messaging  [kernel.vmlinux]  [k] mutex_spin_on_owner
 8.45%  sched-messaging  [kernel.vmlinux]  [k] mutex_unlock
 4.12%  sched-messaging  [kernel.vmlinux]  [k] system_call
 3.01%  sched-messaging  [kernel.vmlinux]  [k] system_call_common
 2.83%  sched-messaging  [kernel.vmlinux]  [k] copypage_power7
 2.64%  sched-messaging  [kernel.vmlinux]  [k] rwsem_spin_on_owner
 2.00%  sched-messaging  [kernel.vmlinux]  [k] osq_lock

Suggested-by: Boqun Feng 
Signed-off-by: Pan Xinhui 
Acked-by: Christian Borntraeger 
Acked-by: Paolo Bonzini 
Tested-by: Juergen Gross 
---
 kernel/locking/osq_lock.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
index 05a3785..091f97f 100644
--- a/kernel/locking/osq_lock.c
+++ b/kernel/locking/osq_lock.c
@@ -21,6 +21,11 @@ static inline int encode_cpu(int cpu_nr)
return cpu_nr + 1;
 }
 
+static inline int node_cpu(struct optimistic_spin_node *node)
+{
+   return node->cpu - 1;
+}
+
 static inline struct optimistic_spin_node *decode_cpu(int encoded_cpu_val)
 {
int cpu_nr = encoded_cpu_val - 1;
@@ -118,8 +123,9 @@ bool osq_lock(struct optimistic_spin_queue *lock)
while (!READ_ONCE(node->locked)) {
/*
 * If we need to reschedule bail... so we can block.
+* Use vcpu_is_preempted to detect lock holder preemption issue.
 */
-   if (need_resched())
+   if (need_resched() || vcpu_is_preempted(node_cpu(node->prev)))
goto unqueue;
 
cpu_relax_lowlatency();
-- 
2.4.11


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

[Xen-devel] [PATCH v7 00/11] implement vcpu preempted check

[Pan Xinhui]

change from v6:
fix typos and remove uncessary comments.
change from v5:
spilt x86/kvm patch into guest/host part.
introduce kvm_write_guest_offset_cached.
fix some typos.
rebase patch onto 4.9.2
change from v4:
spilt x86 kvm vcpu preempted check into two patches.
add documentation patch.
add x86 vcpu preempted check patch under xen
add s390 vcpu preempted check patch 
change from v3:
add x86 vcpu preempted check patch
change from v2:
no code change, fix typos, update some comments
change from v1:
a simplier definition of default vcpu_is_preempted
skip mahcine type check on ppc, and add config. remove dedicated macro.
add one patch to drop overload of rwsem_spin_on_owner and 
mutex_spin_on_owner. 
add more comments
thanks boqun and Peter's suggestion.

This patch set aims to fix lock holder preemption issues.

test-case:
perf record -a perf bench sched messaging -g 400 -p && perf report

18.09%  sched-messaging  [kernel.vmlinux]  [k] osq_lock
12.28%  sched-messaging  [kernel.vmlinux]  [k] rwsem_spin_on_owner
 5.27%  sched-messaging  [kernel.vmlinux]  [k] mutex_unlock
 3.89%  sched-messaging  [kernel.vmlinux]  [k] wait_consider_task
 3.64%  sched-messaging  [kernel.vmlinux]  [k] _raw_write_lock_irq
 3.41%  sched-messaging  [kernel.vmlinux]  [k] mutex_spin_on_owner.is
 2.49%  sched-messaging  [kernel.vmlinux]  [k] system_call

We introduce interface bool vcpu_is_preempted(int cpu) and use it in some spin
loops of osq_lock, rwsem_spin_on_owner and mutex_spin_on_owner.
These spin_on_onwer variant also cause rcu stall before we apply this patch set

We also have observed some performace improvements in uninx benchmark tests.

PPC test result:
1 copy - 0.94%
2 copy - 7.17%
4 copy - 11.9%
8 copy -  3.04%
16 copy - 15.11%

details below:
Without patch:

1 copy - File Write 4096 bufsize 8000 maxblocks  2188223.0 KBps  (30.0 s, 1 
samples)
2 copy - File Write 4096 bufsize 8000 maxblocks  1804433.0 KBps  (30.0 s, 1 
samples)
4 copy - File Write 4096 bufsize 8000 maxblocks  1237257.0 KBps  (30.0 s, 1 
samples)
8 copy - File Write 4096 bufsize 8000 maxblocks  1032658.0 KBps  (30.0 s, 1 
samples)
16 copy - File Write 4096 bufsize 8000 maxblocks   768000.0 KBps  (30.1 s, 
1 samples)

With patch: 

1 copy - File Write 4096 bufsize 8000 maxblocks  2209189.0 KBps  (30.0 s, 1 
samples)
2 copy - File Write 4096 bufsize 8000 maxblocks  1943816.0 KBps  (30.0 s, 1 
samples)
4 copy - File Write 4096 bufsize 8000 maxblocks  1405591.0 KBps  (30.0 s, 1 
samples)
8 copy - File Write 4096 bufsize 8000 maxblocks  1065080.0 KBps  (30.0 s, 1 
samples)
16 copy - File Write 4096 bufsize 8000 maxblocks   904762.0 KBps  (30.0 s, 
1 samples)

X86 test result:
test-case   after-patch   before-patch
Execl Throughput   |18307.9 lps  |11701.6 lps 
File Copy 1024 bufsize 2000 maxblocks  |  1352407.3 KBps |   790418.9 KBps
File Copy 256 bufsize 500 maxblocks|   367555.6 KBps |   222867.7 KBps
File Copy 4096 bufsize 8000 maxblocks  |  3675649.7 KBps |  1780614.4 KBps
Pipe Throughput| 11872208.7 lps  | 11855628.9 lps 
Pipe-based Context Switching   |  1495126.5 lps  |  1490533.9 lps 
Process Creation   |29881.2 lps  |28572.8 lps 
Shell Scripts (1 concurrent)   |23224.3 lpm  |22607.4 lpm 
Shell Scripts (8 concurrent)   | 3531.4 lpm  | 3211.9 lpm 
System Call Overhead   | 10385653.0 lps  | 10419979.0 lps 


Christian Borntraeger (1):
  s390/spinlock: Provide vcpu_is_preempted

Juergen Gross (1):
  x86, xen: support vcpu preempted check

Pan Xinhui (9):
  kernel/sched: introduce vcpu preempted check interface
  locking/osq: Drop the overload of osq_lock()
  kernel/locking: Drop the overload of {mutex,rwsem}_spin_on_owner
  powerpc/spinlock: support vcpu preempted check
  x86, paravirt: Add interface to support kvm/xen vcpu preempted check
  KVM: Introduce kvm_write_guest_offset_cached
  x86, kvm/x86.c: support vcpu preempted check
  x86, kernel/kvm.c: support vcpu preempted check
  Documentation: virtual: kvm: Support vcpu preempted check

 Documentation/virtual/kvm/msr.txt |  9 -
 arch/powerpc/include/asm/spinlock.h   |  8 
 arch/s390/include/asm/spinlock.h  |  8 
 arch/s390/kernel/smp.c|  9 +++--
 arch/s390/lib/spinlock.c  | 25 -
 arch/x86/include/asm/paravirt_types.h |  2 ++
 arch/x86/include/asm/spinlock.h   |  8 
 arch/x86/include/uapi/asm/kvm_para.h  |  4 +++-
 arch/x86/kernel/kvm.c | 12 
 arch/x86/kernel/paravirt-spinlocks.c  |  6 ++
 arch/x86/kvm/x86.c| 16 
 arch/x86/xen/spinlock.c   |  3 ++-
 include/linux/kvm_host.h  |  

[Xen-devel] [PATCH v7 04/11] powerpc/spinlock: support vcpu preempted check

[Pan Xinhui]

This is to fix some lock holder preemption issues. Some other locks
implementation do a spin loop before acquiring the lock itself.
Currently kernel has an interface of bool vcpu_is_preempted(int cpu). It
takes the cpu as parameter and return true if the cpu is preempted. Then
kernel can break the spin loops upon the retval of vcpu_is_preempted.

As kernel has used this interface, So lets support it.

Only pSeries need support it. And the fact is PowerNV is built into same
kernel image with pSeries. So we need return false if we are runnig as
PowerNV. The another fact is that lppaca->yiled_count keeps zero on
PowerNV. So we can just skip the machine type check.

Suggested-by: Boqun Feng 
Suggested-by: Peter Zijlstra (Intel) 
Signed-off-by: Pan Xinhui 
---
 arch/powerpc/include/asm/spinlock.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/powerpc/include/asm/spinlock.h 
b/arch/powerpc/include/asm/spinlock.h
index fa37fe9..8c1b913 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -52,6 +52,14 @@
 #define SYNC_IO
 #endif
 
+#ifdef CONFIG_PPC_PSERIES
+#define vcpu_is_preempted vcpu_is_preempted
+static inline bool vcpu_is_preempted(int cpu)
+{
+   return !!(be32_to_cpu(lppaca_of(cpu).yield_count) & 1);
+}
+#endif
+
 static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
 {
return lock.slock == 0;
-- 
2.4.11


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

[Xen-devel] [PATCH v7 01/11] kernel/sched: introduce vcpu preempted check interface

[Pan Xinhui]

This patch support to fix lock holder preemption issue.

For kernel users, we could use bool vcpu_is_preempted(int cpu) to detect
if one vcpu is preempted or not.

The default implementation is a macro defined by false. So compiler can
wrap it out if arch dose not support such vcpu preempted check.

Suggested-by: Peter Zijlstra (Intel) 
Signed-off-by: Pan Xinhui 
Acked-by: Christian Borntraeger 
Acked-by: Paolo Bonzini 
Tested-by: Juergen Gross 
---
 include/linux/sched.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 348f51b..44c1ce7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -3506,6 +3506,18 @@ static inline void set_task_cpu(struct task_struct *p, 
unsigned int cpu)
 
 #endif /* CONFIG_SMP */
 
+/*
+ * In order to deal with a various lock holder preemption issues provide an
+ * interface to see if a vCPU is currently running or not.
+ *
+ * This allows us to terminate optimistic spin loops and block, analogous to
+ * the native optimistic spin heuristic of testing if the lock owner task is
+ * running or not.
+ */
+#ifndef vcpu_is_preempted
+#define vcpu_is_preempted(cpu) false
+#endif
+
 extern long sched_setaffinity(pid_t pid, const struct cpumask *new_mask);
 extern long sched_getaffinity(pid_t pid, struct cpumask *mask);
 
-- 
2.4.11


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

[Xen-devel] [PATCH v7 05/11] s390/spinlock: Provide vcpu_is_preempted

[Pan Xinhui]

From: Christian Borntraeger 

this implements the s390 backend for commit
"kernel/sched: introduce vcpu preempted check interface"
by reworking the existing smp_vcpu_scheduled into
arch_vcpu_is_preempted. We can then also get rid of the
local cpu_is_preempted function by moving the
CIF_ENABLED_WAIT test into arch_vcpu_is_preempted.

Signed-off-by: Christian Borntraeger 
Acked-by: Heiko Carstens 
---
 arch/s390/include/asm/spinlock.h |  8 
 arch/s390/kernel/smp.c   |  9 +++--
 arch/s390/lib/spinlock.c | 25 -
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/arch/s390/include/asm/spinlock.h b/arch/s390/include/asm/spinlock.h
index 7e9e09f..7ecd890 100644
--- a/arch/s390/include/asm/spinlock.h
+++ b/arch/s390/include/asm/spinlock.h
@@ -23,6 +23,14 @@ _raw_compare_and_swap(unsigned int *lock, unsigned int old, 
unsigned int new)
return __sync_bool_compare_and_swap(lock, old, new);
 }
 
+#ifndef CONFIG_SMP
+static inline bool arch_vcpu_is_preempted(int cpu) { return false; }
+#else
+bool arch_vcpu_is_preempted(int cpu);
+#endif
+
+#define vcpu_is_preempted arch_vcpu_is_preempted
+
 /*
  * Simple spin lock operations.  There are two variants, one clears IRQ's
  * on the local processor, one does not.
diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
index 35531fe..b988ed1 100644
--- a/arch/s390/kernel/smp.c
+++ b/arch/s390/kernel/smp.c
@@ -368,10 +368,15 @@ int smp_find_processor_id(u16 address)
return -1;
 }
 
-int smp_vcpu_scheduled(int cpu)
+bool arch_vcpu_is_preempted(int cpu)
 {
-   return pcpu_running(pcpu_devices + cpu);
+   if (test_cpu_flag_of(CIF_ENABLED_WAIT, cpu))
+   return false;
+   if (pcpu_running(pcpu_devices + cpu))
+   return false;
+   return true;
 }
+EXPORT_SYMBOL(arch_vcpu_is_preempted);
 
 void smp_yield_cpu(int cpu)
 {
diff --git a/arch/s390/lib/spinlock.c b/arch/s390/lib/spinlock.c
index e5f50a7..e48a48e 100644
--- a/arch/s390/lib/spinlock.c
+++ b/arch/s390/lib/spinlock.c
@@ -37,15 +37,6 @@ static inline void _raw_compare_and_delay(unsigned int 
*lock, unsigned int old)
asm(".insn rsy,0xeb22,%0,0,%1" : : "d" (old), "Q" (*lock));
 }
 
-static inline int cpu_is_preempted(int cpu)
-{
-   if (test_cpu_flag_of(CIF_ENABLED_WAIT, cpu))
-   return 0;
-   if (smp_vcpu_scheduled(cpu))
-   return 0;
-   return 1;
-}
-
 void arch_spin_lock_wait(arch_spinlock_t *lp)
 {
unsigned int cpu = SPINLOCK_LOCKVAL;
@@ -62,7 +53,7 @@ void arch_spin_lock_wait(arch_spinlock_t *lp)
continue;
}
/* First iteration: check if the lock owner is running. */
-   if (first_diag && cpu_is_preempted(~owner)) {
+   if (first_diag && arch_vcpu_is_preempted(~owner)) {
smp_yield_cpu(~owner);
first_diag = 0;
continue;
@@ -81,7 +72,7 @@ void arch_spin_lock_wait(arch_spinlock_t *lp)
 * yield the CPU unconditionally. For LPAR rely on the
 * sense running status.
 */
-   if (!MACHINE_IS_LPAR || cpu_is_preempted(~owner)) {
+   if (!MACHINE_IS_LPAR || arch_vcpu_is_preempted(~owner)) {
smp_yield_cpu(~owner);
first_diag = 0;
}
@@ -108,7 +99,7 @@ void arch_spin_lock_wait_flags(arch_spinlock_t *lp, unsigned 
long flags)
continue;
}
/* Check if the lock owner is running. */
-   if (first_diag && cpu_is_preempted(~owner)) {
+   if (first_diag && arch_vcpu_is_preempted(~owner)) {
smp_yield_cpu(~owner);
first_diag = 0;
continue;
@@ -127,7 +118,7 @@ void arch_spin_lock_wait_flags(arch_spinlock_t *lp, 
unsigned long flags)
 * yield the CPU unconditionally. For LPAR rely on the
 * sense running status.
 */
-   if (!MACHINE_IS_LPAR || cpu_is_preempted(~owner)) {
+   if (!MACHINE_IS_LPAR || arch_vcpu_is_preempted(~owner)) {
smp_yield_cpu(~owner);
first_diag = 0;
}
@@ -165,7 +156,7 @@ void _raw_read_lock_wait(arch_rwlock_t *rw)
owner = 0;
while (1) {
if (count-- <= 0) {
-   if (owner && cpu_is_preempted(~owner))
+   if (owner && arch_vcpu_is_preempted(~owner))
smp_yield_cpu(~owner);
count = spin_retry;
}
@@ -211,7 +202,7 @@ void _raw_write_lock_wait(arch_rwlock_t *rw, unsigned int 
prev)
owner = 0;
while (1) {
if (count-- <= 0) {
- 

[Xen-devel] [PATCH v7 03/11] kernel/locking: Drop the overload of {mutex, rwsem}_spin_on_owner

[Pan Xinhui]

An over-committed guest with more vCPUs than pCPUs has a heavy overload
in the two spin_on_owner. This blames on the lock holder preemption
issue.

Kernel has an interface bool vcpu_is_preempted(int cpu) to see if a vCPU
is currently running or not. So break the spin loops on true condition.

test-case:
perf record -a perf bench sched messaging -g 400 -p && perf report

before patch:
20.68%  sched-messaging  [kernel.vmlinux]  [k] mutex_spin_on_owner
 8.45%  sched-messaging  [kernel.vmlinux]  [k] mutex_unlock
 4.12%  sched-messaging  [kernel.vmlinux]  [k] system_call
 3.01%  sched-messaging  [kernel.vmlinux]  [k] system_call_common
 2.83%  sched-messaging  [kernel.vmlinux]  [k] copypage_power7
 2.64%  sched-messaging  [kernel.vmlinux]  [k] rwsem_spin_on_owner
 2.00%  sched-messaging  [kernel.vmlinux]  [k] osq_lock

after patch:
 9.99%  sched-messaging  [kernel.vmlinux]  [k] mutex_unlock
 5.28%  sched-messaging  [unknown] [H] 0xc00768e0
 4.27%  sched-messaging  [kernel.vmlinux]  [k] __copy_tofrom_user_power7
 3.77%  sched-messaging  [kernel.vmlinux]  [k] copypage_power7
 3.24%  sched-messaging  [kernel.vmlinux]  [k] _raw_write_lock_irq
 3.02%  sched-messaging  [kernel.vmlinux]  [k] system_call
 2.69%  sched-messaging  [kernel.vmlinux]  [k] wait_consider_task

Signed-off-by: Pan Xinhui 
Acked-by: Christian Borntraeger 
Acked-by: Paolo Bonzini 
Tested-by: Juergen Gross 
---
 kernel/locking/mutex.c  | 13 +++--
 kernel/locking/rwsem-xadd.c | 14 +++---
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index a70b90d..24face6 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -236,7 +236,11 @@ bool mutex_spin_on_owner(struct mutex *lock, struct 
task_struct *owner)
 */
barrier();
 
-   if (!owner->on_cpu || need_resched()) {
+   /*
+* Use vcpu_is_preempted to detect lock holder preemption issue.
+*/
+   if (!owner->on_cpu || need_resched() ||
+   vcpu_is_preempted(task_cpu(owner))) {
ret = false;
break;
}
@@ -261,8 +265,13 @@ static inline int mutex_can_spin_on_owner(struct mutex 
*lock)
 
rcu_read_lock();
owner = READ_ONCE(lock->owner);
+
+   /*
+* As lock holder preemption issue, we both skip spinning if task is not
+* on cpu or its cpu is preempted
+*/
if (owner)
-   retval = owner->on_cpu;
+   retval = owner->on_cpu && !vcpu_is_preempted(task_cpu(owner));
rcu_read_unlock();
/*
 * if lock->owner is not set, the mutex owner may have just acquired
diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index 2337b4b..b664ce1 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -336,7 +336,11 @@ static inline bool rwsem_can_spin_on_owner(struct 
rw_semaphore *sem)
goto done;
}
 
-   ret = owner->on_cpu;
+   /*
+* As lock holder preemption issue, we both skip spinning if task is not
+* on cpu or its cpu is preempted
+*/
+   ret = owner->on_cpu && !vcpu_is_preempted(task_cpu(owner));
 done:
rcu_read_unlock();
return ret;
@@ -362,8 +366,12 @@ static noinline bool rwsem_spin_on_owner(struct 
rw_semaphore *sem)
 */
barrier();
 
-   /* abort spinning when need_resched or owner is not running */
-   if (!owner->on_cpu || need_resched()) {
+   /*
+* abort spinning when need_resched or owner is not running or
+* owner's cpu is preempted.
+*/
+   if (!owner->on_cpu || need_resched() ||
+   vcpu_is_preempted(task_cpu(owner))) {
rcu_read_unlock();
return false;
}
-- 
2.4.11


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

[Xen-devel] [ovmf baseline-only test] 67977: all pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 67977 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/67977/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 63998d7cd4f573c41fe0e018257f6d7012218cc2
baseline version:
 ovmf 90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e

Last test of basis67974  2016-11-01 07:50:55 Z0 days
Testing same since67977  2016-11-01 21:49:55 Z0 days1 attempts


People who touched revisions under test:
  Giri P Mudusuru 
  Mudusuru, Giri P 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

Test harness code can be found at
http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Push not applicable.


commit 63998d7cd4f573c41fe0e018257f6d7012218cc2
Author: Mudusuru, Giri P 
Date:   Tue Nov 1 00:56:35 2016 +0800

IntelSiliconPkg: Add SMBIOS data HOB GUID

Add gIntelSmbiosDataHobGuid used to publish SMBIOS data from PEI phase.
The HOB data format will be same as SMBIOS spec define formats for
Types 0 to 127 and OEM defined types for 128 to 255.

Generic library or DXE driver can add SMBIOS records using this HOB(s).

Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Michael Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Giri P Mudusuru 
Reviewed-by: Jiewen Yao 
Reviewed-by: Star Zeng 

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

[Xen-devel] [PATCH 1/1] xen-netfront: cast grant table reference first to type int

[Dongli Zhang]

IS_ERR_VALUE() in commit 87557efc27f6a50140fb20df06a917f368ce3c66
("xen-netfront: do not cast grant table reference to signed short") would
not return true for error code unless we cast ref first to type int.

Signed-off-by: Dongli Zhang 
---
 drivers/net/xen-netfront.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 189a28d..bf2744e 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -304,7 +304,7 @@ static void xennet_alloc_rx_buffers(struct netfront_queue 
*queue)
queue->rx_skbs[id] = skb;
 
ref = gnttab_claim_grant_reference(>gref_rx_head);
-   WARN_ON_ONCE(IS_ERR_VALUE((unsigned long)ref));
+   WARN_ON_ONCE(IS_ERR_VALUE((unsigned long)(int)ref));
queue->grant_rx_ref[id] = ref;
 
page = skb_frag_page(_shinfo(skb)->frags[0]);
@@ -428,7 +428,7 @@ static void xennet_tx_setup_grant(unsigned long gfn, 
unsigned int offset,
id = get_id_from_freelist(>tx_skb_freelist, queue->tx_skbs);
tx = RING_GET_REQUEST(>tx, queue->tx.req_prod_pvt++);
ref = gnttab_claim_grant_reference(>gref_tx_head);
-   WARN_ON_ONCE(IS_ERR_VALUE((unsigned long)ref));
+   WARN_ON_ONCE(IS_ERR_VALUE((unsigned long)(int)ref));
 
gnttab_grant_foreign_access_ref(ref, queue->info->xbdev->otherend_id,
gfn, GNTMAP_readonly);
-- 
2.7.4


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

Re: [Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Xuquan (Quan Xu)]

On November 01, 2016 9:21 PM, Andrew Cooper wrote:
>On 01/11/16 12:07, Xuquan (Quan Xu) wrote:
>> On November 01, 2016 8:00 PM, Andrew Cooper wrote:
>>> On 01/11/16 11:57, Xuquan (Quan Xu) wrote:
 On November 01, 2016 7:41 PM, Andrew Cooper
>>>  wrote:
> On 01/11/16 11:23, Xuquan (Quan Xu) wrote:
>> On November 01, 2016 7:16 PM, Andrew Cooper <
> andrew.coop...@citrix.com > wrote:
>>> On 01/11/16 11:01, Xuquan (Quan Xu) wrote:
 Hi Andrew,

 When I run some application with Xen, I encounter a Panic with
 log as the
>>> bottom of this email.
 I find this panic is as similar as your fix e4e9d2d4e76bd8fe22
>>> 'x86/p2m-ept:
>>> don't unmap the EPT pagetable while it is still in use'.
>>>
>>> Its not the same.  My fix was for a pagefault.  Your crash is
>>> hitting a
>>> BUG()
>>>
>>> You need to investigate which BUG() is being hit (you are clearly
>>> not using staging, as page_alloc.c:429 is outside of a function), and 
>>> why.
>>>
>> It is not using staging, a internal version, but the
>> __do_update_va_mapping() is
> the same..
>> So I think it is similar to staging.
> __do_update_va_mapping() is not relevant.  Your problem is in
> alloc_heap_pages(), not __do_update_va_mapping().
>
 Andrew, thanks for your help..

 Check it,
 the BUG() is from
 do_memory_op() -- alloc_domheap_pages() -- alloc_heap_pages()..
 , not from  __do_update_va_mapping().

 If so, I think the information is too limited to debug..

 the BUG() is being hit at:
 ...
 alloc_heap_pages()
 for ( i = 0; i < (1 << order); i++ )
 {
 /* Reference count must continuously be zero for free pages. */
 BUG_ON(pg[i].count_info != PGC_state_free); ...


 I guess I hit a 'count_info == -1', more put_page() is called to return the
>page..
>>> Yes.  This means that you have a reference counting error elsewhere
>>> in the hypervisor, and something is dropping too many references.
>>>
>> :(:(..
>> Any method to address it?
>
>Not specifically.  It is similar to chasing a double free bug.
>
>You should review whatever local additions you have in your tree extra 
>carefully.
>It might be that there is a setup path which doesn't take a ref, or a buggy 
>error
>path which cleans up too many refs.
>

Andrew, thank you very much!!
I think it is likely a buggy error of local additions..

Quan

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

[Xen-devel] [qemu-mainline baseline-only test] 67975: tolerable FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 67975 qemu-mainline real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/67975/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail like 67970
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  9 windows-installfail like 67970

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass

version targeted for testing:
 qemuue80b4b8fb6babce7dcc91ea9ddeecbc351fd4646
baseline version:
 qemuu2dfe5113b11ce0ddb08176ebb54ab7ac4104b413

Last test of basis67970  2016-10-31 22:18:16 Z1 days
Testing same since67975  2016-11-01 09:46:14 Z0 days1 attempts


People who touched revisions under test:
  Akanksha Srivastava 
  Alberto Garcia 
  Alex Bennée 
  Alex Williamson 
  Amit Shah 
  Anand J 
  Anthony PERARD 
  Ashijeet Acharya 
  Changlong Xie 
  Cornelia Huck 
  Daniel Shahaf 
  Emil Condrea 
  Emilio G. Cota 
  Eric Blake 
  Fam Zheng 
  Gonglei 
  Guenter Roeck 
  Ido Yariv 
  Jean-Christophe Dubois 
  Kevin Wolf 
  KONRAD Frederic 
  Laurent Vivier 
  Li Zhijian 
  Marc-André Lureau 
  Michael Tokarev 
  Michael Walle 
  Paolo Bonzini 
  Peter Maydell 
  Pierre Morel 
  Prasad J Pandit 
  Quan Xu 
  Quan Xu 
  Stefan Weil 
  Stefano Stabellini 
  Thomas Huth 
  Tomáš 

Re: [Xen-devel] xc_hvm_inject_trap() races

[Andrew Cooper]

On 01/11/2016 22:17, Andrei Vlad LUTAS wrote:
>>> First of all, to answer your original question: the injection decision
>>> is made when the introspection logic needs to inspect a page that is
>>> not present in the physical memory. We don't really care if the current
>>> instruction triggers multiple faults or not (and here I'm not sure what
>>> you mean by that - multiple exceptions, or multiple EPT violations -
>>> but the answer is still the same), and removing the page restrictions
>>> after the #PF injection is introspection specific logic - the address
>>> for which we inject the #PF doesn't have to be related in any way to the 
>>> current instruction.
>> Ah, that's this no-architectural behavior again.
> I don't think the HVI #PF injection internals or how the #PF is handled by 
> the OS are relevant here. We are using an existing API that seems to not work 
> quite correct under certain circumstances and we were curious if any of you 
> can shed some light in this regard, and maybe point us to the right direction 
> for cooking up a fix.

Just because there is an API like this, doesn't necessarily mean it ever
worked.  This one clearly doesn't, and it got introduced before we as a
community took a rather harder stance towards code review.

Architecturally speaking, faults are always raised as a direct
consequence of the current state.  Therefore, having in introspection
agent interposing on the instruction stream and causing faults as a side
effect of EPT permissions/etc, is quite natural and in line with
architectural expectation.

You also have a second usecase for this API, which is to trick Windows
into paging in a frame you care about looking at.  Overall, using this
#PF method to get Windows to page content back in clearly the rational
way of making that happen, but it is very definitely a non-architectural
usecase; if windows were to double check the instruction stream on
getting this pagefault, it would get very confused, as the pagefault it
received has nothing to do with the code pointed at in the exception frame.

It is quite likely that these different usecases might have different
solutions.  IMO, the former should probably be controlled by responses
in the vm_event ring, but this latter issue probably shouldn't.

When it comes to injecting exceptions, there are some restrictions which
limit what can legitimately be done.  We can only inject a single thing
at once.  Stacking a #PF on top of a plain interrupt could be resolved
by leaving the interrupt pending in the vLAPIC and injecting the #PF
instead.  Stacking a #PF on top of a different fault is going to cause
hvm_combine_exceptions() to turn it into something more nasty.  OTOH,
deferring the #PF by even a single instruction could result in it being
sent in an unsafe context, which should also be avoided.

What hard propertied do you need for this usecase, and are there any
properties can afford to be flexible?

~Andrew

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

Re: [Xen-devel] [PATCH v2] PCMachineState: introduce acpi_build_enabled field

[Stefano Stabellini]

On Tue, 1 Nov 2016, Eduardo Habkost wrote:
> On Tue, Nov 01, 2016 at 05:44:16PM +, Wei Liu wrote:
> > Introduce this field to control whether ACPI build is enabled by a
> > particular machine or accelerator.
> > 
> > It defaults to true if the machine itself supports ACPI build. Xen
> > accelerator will disable it because Xen is in charge of building ACPI
> > tables for the guest.
> > 
> > Signed-off-by: Wei Liu 
> 
> Reviewed-by: Eduardo Habkost 

Thank you Eduardo. I can queue it up unless you have other stuff in
progress.

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

Re: [Xen-devel] xc_hvm_inject_trap() races

[Andrei Vlad LUTAS]

-Original Message-
From: Jan Beulich [mailto:jbeul...@suse.com]
Sent: 1 November, 2016 18:40
To: rcojoc...@bitdefender.com; Andrei Vlad LUTAS 
Cc: andrew.coop...@citrix.com; xen-de...@lists.xenproject.org; 
ta...@tklengyel.com
Subject: Re: RE: [Xen-devel] xc_hvm_inject_trap() races

>>> Andrei Vlad LUTAS  11/01/16 5:13 PM >>>

>First of all, please don't top post.

>>First of all, to answer your original question: the injection decision
>>is made when the introspection logic needs to inspect a page that is
>>not present in the physical memory. We don't really care if the current
>>instruction triggers multiple faults or not (and here I'm not sure what
>>you mean by that - multiple exceptions, or multiple EPT violations -
>>but the answer is still the same), and removing the page restrictions
>>after the #PF injection is introspection specific logic - the address
>>for which we inject the #PF doesn't have to be related in any way to the 
>>current instruction.

>Ah, that's this no-architectural behavior again.

I don't think the HVI #PF injection internals or how the #PF is handled by the 
OS are relevant here. We are using an existing API that seems to not work quite 
correct under certain circumstances and we were curious if any of you can shed 
some light in this regard, and maybe point us to the right direction for 
cooking up a fix.

>What if the OS doesn't fully carry out the page-in, relying on the #PF to 
>retrigger once the insn for which it got reported has been restarted?

Can you be more specific?

> Or what if the page gets paged out again before the insn actually gets to 
> execute (e.g. because a re-schedule happened inside the guest on the way out 
> of the #PF handler)? All of this suggests that you really can't lift >any 
> restrictions _before_ seeing what you need to see.

We don't really care when and how the #PF is handled. We don't care if the page 
is paged out at some random point. What we do know is that at a certain point 
in the future, the page will be swapped in; how do we know when? The OS will 
write the guest page tables, at which point we can inspect the physical page 
itself (so you can see here why we don't care about the page being swapped out 
sometime in the future). So we really _can_ lift any restriction we want at 
that point.

>>Assuming that we wouldn't remove the restrictions and we would rely on
>>re-generating the event - that is not acceptable: first of all because
>>the instruction would normally be emulated anyway before re-entering
>>the guest,

>How would that be a problem?

I thought it was obvious without further clarification: how can we expect the 
exact same event to be generated, if the instruction that triggered it in the 
first place was emulated or single stepped?

>>and secondly because that is not a normal CPU behavior

>This really is the main problem here, afaict.

Best regards,
Andrei.



This email was scanned by Bitdefender

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

Re: [Xen-devel] [PATCH v2] PCMachineState: introduce acpi_build_enabled field

[Eduardo Habkost]

On Tue, Nov 01, 2016 at 05:44:16PM +, Wei Liu wrote:
> Introduce this field to control whether ACPI build is enabled by a
> particular machine or accelerator.
> 
> It defaults to true if the machine itself supports ACPI build. Xen
> accelerator will disable it because Xen is in charge of building ACPI
> tables for the guest.
> 
> Signed-off-by: Wei Liu 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo

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

[Xen-devel] [qemu-mainline test] 101842: regressions - FAIL

[osstest service owner]

flight 101842 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101842/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-xsm 15 guest-start/debian.repeat fail REGR. vs. 101835

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  9 debian-install  fail blocked in 101835
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 101835
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 101835
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 101835
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 101835
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 101835
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 101835

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 qemuub90da81d9f2d3d974db13bc69f961974741b0e2e
baseline version:
 qemuue80b4b8fb6babce7dcc91ea9ddeecbc351fd4646

Last test of basis   101835  2016-10-31 22:21:49 Z0 days
Testing same since   101842  2016-11-01 09:47:34 Z0 days1 attempts


People who touched revisions under test:
  Corey Minyard 
  Eduardo Habkost 
  Kevin Wolf 
  Peter Maydell 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl  

[Xen-devel] [ovmf test] 101843: all pass - PUSHED

[osstest service owner]

flight 101843 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101843/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 63998d7cd4f573c41fe0e018257f6d7012218cc2
baseline version:
 ovmf 90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e

Last test of basis   101838  2016-11-01 02:17:57 Z0 days
Testing same since   101843  2016-11-01 12:47:07 Z0 days1 attempts


People who touched revisions under test:
  Giri P Mudusuru 
  Mudusuru, Giri P 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=ovmf
+ revision=63998d7cd4f573c41fe0e018257f6d7012218cc2
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
63998d7cd4f573c41fe0e018257f6d7012218cc2
+ branch=ovmf
+ revision=63998d7cd4f573c41fe0e018257f6d7012218cc2
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.7-testing
+ '[' x63998d7cd4f573c41fe0e018257f6d7012218cc2 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git
++ : 

[Xen-devel] [xen-unstable test] 101841: regressions - FAIL

[osstest service owner]

flight 101841 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101841/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-xsm   9 debian-install   fail REGR. vs. 101673

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 101673
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 101673
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 101673
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 101673
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 101673
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 101673
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 101673
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 101673
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 101673

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 build-i386-rumprun7 xen-buildfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 build-amd64-rumprun   7 xen-buildfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass

version targeted for testing:
 xen  496673a2ada93c201fbe1cc83146c8bd8e79169d
baseline version:
 xen  6f9b62ca57322197e26d3b22ff11b629697142bd

Last test of basis   101673  2016-10-26 02:01:16 Z6 days
Failing since101698  2016-10-26 19:50:48 Z6 days   10 attempts
Testing same since   101841  2016-11-01 07:29:46 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Dario Faggioli 
  David Scott 
  Ian Jackson 
  Jan Beulich 
  Juergen Gross 
  Meng Xu 
  Roger Pau Monne 
  Roger Pau Monné 
  Samuel Thibault 
  Wei Liu 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64-xtf  pass
 build-amd64  

[Xen-devel] [linux-3.4 test] 101840: regressions - FAIL

[osstest service owner]

flight 101840 linux-3.4 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101840/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl   6 xen-boot  fail REGR. vs. 92983
 test-amd64-i386-qemut-rhel6hvm-intel  6 xen-boot  fail REGR. vs. 92983
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 
92983
 test-amd64-amd64-libvirt-vhd  6 xen-boot  fail REGR. vs. 92983
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  6 xen-boot  fail REGR. vs. 92983
 test-amd64-i386-xl-qemuu-debianhvm-amd64  6 xen-boot  fail REGR. vs. 92983
 test-amd64-amd64-xl-qcow2 6 xen-boot  fail REGR. vs. 92983
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-bootfail REGR. vs. 92983
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  6 xen-boot  fail REGR. vs. 92983
 test-amd64-i386-xl-qemuu-winxpsp3  6 xen-boot fail REGR. vs. 92983
 test-amd64-amd64-qemuu-nested-intel  6 xen-boot   fail REGR. vs. 92983
 test-amd64-i386-xl6 xen-boot  fail REGR. vs. 92983
 test-amd64-amd64-xl-xsm   6 xen-boot  fail REGR. vs. 92983

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail in 101695 pass 
in 101840
 test-amd64-amd64-i386-pvgrub  6 xen-boot   fail pass in 101695
 test-amd64-amd64-xl-rtds  6 xen-boot   fail pass in 101695
 test-amd64-i386-freebsd10-amd64  6 xen-bootfail pass in 101695
 test-amd64-i386-pair  9 xen-boot/src_host  fail pass in 101720
 test-amd64-i386-pair 10 xen-boot/dst_host  fail pass in 101720
 test-amd64-i386-qemuu-rhel6hvm-intel  6 xen-boot   fail pass in 101822

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 92983
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 92983
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 92983

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 build-amd64-rumprun   7 xen-buildfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 build-i386-rumprun7 xen-buildfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail never pass

version targeted for testing:
 linux8d1988f838a95e836342b505398d38b223181f17
baseline version:
 linux343a5fbeef08baf2097b8cf4e26137cebe3cfef4

Last test of basis92983  2016-04-27 16:21:44 Z  188 days
Testing same since   101695  2016-10-26 18:26:23 Z6 days   10 attempts


People who touched revisions under test:
  "Suzuki K. Poulose" 
  Aaro Koskinen 
  Al Viro 
  Alan Stern 
  Aleksander Morgado 
  Alex Thorlton 
  Alexandru Cornea 
  Alexey Khoroshilov 
  Amitkumar Karwar 
  Andrew Banman 
  Andrew Morton 
  Andrey Ryabinin 
  Anson Huang 
  Arnaldo Carvalho de Melo 
  Arnaldo Carvalho de Melo 
  Arnd Bergmann 
  Ben Hutchings 
  Bjørn Mork 
  Boris Brezillon 
  Borislav Petkov 
  Brian Norris 
  Charles Keepax 
  Chen Yu 
  Christoph Hellwig 
  Chunfeng Yun 
  Clemens Ladisch 
  Colin Ian King 
  Cong Wang 
  Daeho Jeong 
  Dan Carpenter 
  Darren Hart 
  Dave Airlie 
  

[Xen-devel] [distros-debian-snapshot test] 67973: tolerable FAIL

[Platform Team regression test user]

flight 67973 distros-debian-snapshot real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/67973/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-armhf-daily-netboot-pygrub 9 debian-di-install fail like 67930
 test-amd64-i386-i386-weekly-netinst-pygrub 9 debian-di-install fail like 67930
 test-amd64-amd64-i386-daily-netboot-pygrub 9 debian-di-install fail like 67930
 test-amd64-i386-amd64-weekly-netinst-pygrub 9 debian-di-install fail like 67930
 test-amd64-i386-i386-daily-netboot-pvgrub  9 debian-di-install fail like 67930
 test-amd64-amd64-amd64-daily-netboot-pvgrub 9 debian-di-install fail like 67930
 test-amd64-i386-amd64-daily-netboot-pygrub 9 debian-di-install fail like 67930
 test-amd64-amd64-i386-weekly-netinst-pygrub 9 debian-di-install fail like 67930
 test-amd64-amd64-amd64-weekly-netinst-pygrub 9 debian-di-install fail like 
67930

baseline version:
 flight   67930

jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-daily-netboot-pvgrub  fail
 test-amd64-i386-i386-daily-netboot-pvgrubfail
 test-amd64-i386-amd64-daily-netboot-pygrub   fail
 test-armhf-armhf-armhf-daily-netboot-pygrub  fail
 test-amd64-amd64-i386-daily-netboot-pygrub   fail
 test-amd64-amd64-amd64-current-netinst-pygrubpass
 test-amd64-i386-amd64-current-netinst-pygrub pass
 test-amd64-amd64-i386-current-netinst-pygrub pass
 test-amd64-i386-i386-current-netinst-pygrub  pass
 test-amd64-amd64-amd64-weekly-netinst-pygrub fail
 test-amd64-i386-amd64-weekly-netinst-pygrub  fail
 test-amd64-amd64-i386-weekly-netinst-pygrub  fail
 test-amd64-i386-i386-weekly-netinst-pygrub   fail



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

Test harness code can be found at
http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Push not applicable.


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

Re: [Xen-devel] [PATCH v2] PCMachineState: introduce acpi_build_enabled field

[Stefano Stabellini]

On Tue, 1 Nov 2016, Wei Liu wrote:
> Introduce this field to control whether ACPI build is enabled by a
> particular machine or accelerator.
> 
> It defaults to true if the machine itself supports ACPI build. Xen
> accelerator will disable it because Xen is in charge of building ACPI
> tables for the guest.
> 
> Signed-off-by: Wei Liu 

Reviewed-by: Stefano Stabellini 


> Cc: Igor Mammedov 
> Cc: Eduardo Habkost 
> Cc: Anthony PERARD 
> Cc: Stefano Stabellini 
> Cc: Sander Eikelenboom 
> 
> v2:
> 1. drop acpi-build property
> 2. set acpi_build_enabled to acpi_has_build
> 3. replace acpi_has_build check in acpi_build()
> ---
>  hw/i386/acpi-build.c | 2 +-
>  hw/i386/pc.c | 2 ++
>  include/hw/i386/pc.h | 2 ++
>  xen-common.c | 6 ++
>  4 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5cd1da9..13cbbde 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2953,7 +2953,7 @@ void acpi_setup(void)
>  return;
>  }
>  
> -if (!pcmc->has_acpi_build) {
> +if (!pcms->acpi_build_enabled) {
>  ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
>  return;
>  }
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f56ea0f..fbd9aed 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2159,6 +2159,8 @@ static void pc_machine_initfn(Object *obj)
>  pcms->vmport = ON_OFF_AUTO_AUTO;
>  /* nvdimm is disabled on default. */
>  pcms->acpi_nvdimm_state.is_enabled = false;
> +/* acpi build is enabled by default if machine supports it */
> +pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
>  }
>  
>  static void pc_machine_reset(void)
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 98dc772..8eb517f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -62,6 +62,8 @@ struct PCMachineState {
>  
>  AcpiNVDIMMState acpi_nvdimm_state;
>  
> +bool acpi_build_enabled;
> +
>  /* RAM information (sizes, addresses, configuration): */
>  ram_addr_t below_4g_mem_size, above_4g_mem_size;
>  
> diff --git a/xen-common.c b/xen-common.c
> index 9099760..bacf962 100644
> --- a/xen-common.c
> +++ b/xen-common.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "hw/i386/pc.h"
>  #include "hw/xen/xen_backend.h"
>  #include "qmp-commands.h"
>  #include "sysemu/char.h"
> @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int 
> running,
>  
>  static int xen_init(MachineState *ms)
>  {
> +PCMachineState *pcms = PC_MACHINE(ms);
> +
> +/* Disable ACPI build because Xen handles it */
> +pcms->acpi_build_enabled = false;
> +
>  xen_xc = xc_interface_open(0, 0, 0);
>  if (xen_xc == NULL) {
>  xen_pv_printf(NULL, 0, "can't open xen interface\n");
> -- 
> 2.1.4
> 

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

Re: [Xen-devel] [PATCH 0/2] Fix Xen boot on XGene

[Stefano Stabellini]

On Mon, 31 Oct 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 26/10/16 23:12, Stefano Stabellini wrote:
> > On Wed, 26 Oct 2016, Julien Grall wrote:
> > > Hi,
> > > Apologize for not respecting the netiquette.
> > > 
> > > On Tue, 25 Oct 2016, 5:49 p.m. Stefano Stabellini,
> > >  wrote:
> > >   Hi all,
> > > 
> > >   the following commit:
> > > 
> > >   commit 21550029f709072aacf3b90edd574e7d3021b400
> > >   Author: Julien Grall 
> > >   Date:   Thu Oct 8 19:23:53 2015 +0100
> > > 
> > >   xen/arm: gic-v2: Automatically detect aliased GIC400
> > > 
> > > 
> > >   removed PLATFORM_QUIRK_GIC_64K_STRIDE and introduced an heuristic to
> > >   check whether the two GICC pages have a 64K stride. However the
> > >   heuristic needs device tree to report a GICC region size of 128K to
> > > work
> > >   properly. That is not the case for some versions of XGene (including
> > > the
> > >   one I am using, kindly provided by CloudLab.us).
> > > 
> > >   The patch series fixes the issue by reintroducing platform quirks,
> > >   PLATFORM_QUIRK_GIC_64K_STRIDE, and forcing GICC size to 128K if
> > >   PLATFORM_QUIRK_GIC_64K_STRIDE.
> > > 
> > >   We should consider this series for 4.8.
> > > 
> > > 
> > > The platform you are using has likely a buggy firmware (I cannot find the
> > > mail from
> > > APM stating that).
> > > 
> > > I am not convinced that we should support a such case. IIRC unmodified
> > > Linux will
> > > not work properly on those platform (e.g the GIC will be drived as a
> > > GICv1).
> > 
> > I agree with you that it is buggy firmware, but unfortunately it is
> > still out there, deployed. I am using a regular unmodified old Linux
> > kernel (4.0) which works fine (and should still work fine with modern
> > Xen hypervisors). I believe this DTS comes from upstream Linux 4.0.
> > 
> > The question is: should we carry this workaround to make our users' life
> > easier? Or should we push back the burden of fixing the firmware to the
> > users? There are valid arguments for both, eventually it comes down to
> > finding the right compromise.
> 
> In general it is better to get the newest firmware as other unrelated bugs may
> be present on older version or there is missing workaround for broken hardware
> (e.g enabling chicken bits). For instance, we have already decided to not
> support some version of the X-gene firmware (see commit 784c2d1 "xen/arm: Drop
> support of platform where GICH_LR_HW is not working correctly").
> 
> In this specific case, you assume that the GIC device tree node will always
> point to the beginning of the first 64K page. It might be possible in the
> future to have an updated firmware pointing to the last alias of the first
> page, so the second 4K page will follow directly.

Such firmware could have a version number we could check to disable
PLATFORM_QUIRK_GIC_64K_STRIDE.


> > To be clear I don't feel strongly either way, but I think it is easier
> > for us to carry this workaround than for our users to update the
> > firmware. So in this case I would take these patches. But it might not
> > be the case in the future for other, more invasive, workarounds. Let me
> > know what you think.
> 
> I would much prefer something in Xen to check whether the user has to upgrade
> his firmware based on known broken features.

Yes, we should warn the user in any case, that would be helpful. I can
produce a patch for that.

Regarding firmware updates, for example in this case I cannot actually
perform one, as I don't control the environment, I am just a cloud user
like any other. I can ask u-boot to use a different DTB, but then I
cannot provide a good one because the device tree for m400 has not been
properly pushed upstream in Linux. In other words, there isn't really a
good update path on this platform.

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

[Xen-devel] [PATCH v2] build: make debug option affect tools only

[Wei Liu]

The debug option in Config.mk affects hypervisor, tools and stubdom by
appending different flags to CFLAGS.  Mini-os under extra is not
affected because it already has its own build system when it is
separated from xen.git.

It is undesirable because now hypervisor build is affected by both
Kconfig and debug.

Disentangle the semantics of debug by pushing relevant options to
individual sub-systems.

For hypervisor, the flags previously added by debug option is now
controlled by CONFIG_DEBUG.

For tools, flags are moved from config/*.mk into tools/Rules.mk.

For stubdom, because it unilaterally sets debug=y before including
top-level Config.mk, we only need to move the debug build set of flags
into stubdom Makefile.

Specifically there are some considerations on what flags are picked:

1. we don't need -fno-optimize-sibling-calls anymore because gcc doc
   indicates that it is not enabled for -O1, which we already set in the
   debug build.
2. for tools we use -O0 -g3 in Rules.mk because they already take
   precedence over the flags set in config/*.mk.
3. for hypervisor we don't add -fno-omit-frame-pointer to debug build
   because that's controlled by CONFIG_FRAME_POINTER.

This patch doesn't intend to tune those flags, but to provide identical
set of effective flags as before.  The debug option in Config.mk will
only affect tools components after this patch is applied.

Signed-off-by: Wei Liu 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
Cc: Samuel Thibault 
---
 config/StdGNU.mk | 8 
 config/SunOS.mk  | 7 ---
 stubdom/Makefile | 2 ++
 tools/Rules.mk   | 4 +++-
 xen/Rules.mk | 6 ++
 5 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 39d36b2..6be8233 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -35,14 +35,6 @@ UTIL_LIBS = -lutil
 SONAME_LDFLAG = -soname
 SHLIB_LDFLAGS = -shared
 
-ifneq ($(debug),y)
-CFLAGS += -O2 -fomit-frame-pointer
-else
-# Less than -O1 produces bad code and large stack frames
-CFLAGS += -O1 -fno-omit-frame-pointer
-CFLAGS-$(gcc) += -fno-optimize-sibling-calls
-endif
-
 ifeq ($(lto),y)
 CFLAGS += -flto
 LDFLAGS-$(clang) += -plugin LLVMgold.so
diff --git a/config/SunOS.mk b/config/SunOS.mk
index 86a384d..0fe5f45 100644
--- a/config/SunOS.mk
+++ b/config/SunOS.mk
@@ -31,12 +31,5 @@ UTIL_LIBS =
 SONAME_LDFLAG = -h
 SHLIB_LDFLAGS = -R $(SunOS_LIBDIR) -shared
 
-ifneq ($(debug),y)
-CFLAGS += -O2 -fno-omit-frame-pointer
-else
-# Less than -O1 produces bad code and large stack frames
-CFLAGS += -O1 -fno-omit-frame-pointer
-endif
-
 CFLAGS += -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__
 
diff --git a/stubdom/Makefile b/stubdom/Makefile
index 571704d..2921f30 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -6,6 +6,8 @@ export XEN_OS=MiniOS
 
 export stubdom=y
 export debug=y
+# Moved from config/StdGNU.mk
+CFLAGS += -O1 -fno-omit-frame-pointer
 
 ifeq (,$(findstring clean,$(MAKECMDGOALS)))
   ifeq ($(wildcard $(MINI_OS)/Config.mk),)
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 5a80fec..0e73690 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -138,9 +138,11 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) 
-Wl,-rpath-link=$(XEN_LIBVCHAN)
 
 ifeq ($(debug),y)
 # Disable optimizations and enable debugging information for macros
-CFLAGS += -O0 -g3
+CFLAGS += -O0 -g3 -fno-omit-frame-pointer
 # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=.
 PY_CFLAGS += $(PY_NOOPT_CFLAGS)
+else
+CFLAGS += -O2 -fomit-frame-pointer
 endif
 
 LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
diff --git a/xen/Rules.mk b/xen/Rules.mk
index a9fda71..08cc776 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -46,6 +46,12 @@ ALL_OBJS-y   += $(BASEDIR)/xsm/built_in.o
 ALL_OBJS-y   += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
+ifeq ($(CONFIG_DEBUG),y)
+CFLAGS += -O1
+else
+CFLAGS += -O2 -fomit-frame-pointer
+endif
+
 CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
 CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v2] PCMachineState: introduce acpi_build_enabled field

[Sander Eikelenboom]

On 2016-11-01 18:44, Wei Liu wrote:

Introduce this field to control whether ACPI build is enabled by a
particular machine or accelerator.

It defaults to true if the machine itself supports ACPI build. Xen
accelerator will disable it because Xen is in charge of building ACPI
tables for the guest.

Signed-off-by: Wei Liu 


Hi Wei,

Just gave it a spin (backporting for xen-unstable wasn't too hard),
and this also works for me !

--
Sander


---
Cc: Igor Mammedov 
Cc: Eduardo Habkost 
Cc: Anthony PERARD 
Cc: Stefano Stabellini 
Cc: Sander Eikelenboom 

v2:
1. drop acpi-build property
2. set acpi_build_enabled to acpi_has_build
3. replace acpi_has_build check in acpi_build()
---
 hw/i386/acpi-build.c | 2 +-
 hw/i386/pc.c | 2 ++
 include/hw/i386/pc.h | 2 ++
 xen-common.c | 6 ++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5cd1da9..13cbbde 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2953,7 +2953,7 @@ void acpi_setup(void)
 return;
 }

-if (!pcmc->has_acpi_build) {
+if (!pcms->acpi_build_enabled) {
 ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
 return;
 }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f56ea0f..fbd9aed 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2159,6 +2159,8 @@ static void pc_machine_initfn(Object *obj)
 pcms->vmport = ON_OFF_AUTO_AUTO;
 /* nvdimm is disabled on default. */
 pcms->acpi_nvdimm_state.is_enabled = false;
+/* acpi build is enabled by default if machine supports it */
+pcms->acpi_build_enabled = 
PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;

 }

 static void pc_machine_reset(void)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 98dc772..8eb517f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -62,6 +62,8 @@ struct PCMachineState {

 AcpiNVDIMMState acpi_nvdimm_state;

+bool acpi_build_enabled;
+
 /* RAM information (sizes, addresses, configuration): */
 ram_addr_t below_4g_mem_size, above_4g_mem_size;

diff --git a/xen-common.c b/xen-common.c
index 9099760..bacf962 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -9,6 +9,7 @@
  */

 #include "qemu/osdep.h"
+#include "hw/i386/pc.h"
 #include "hw/xen/xen_backend.h"
 #include "qmp-commands.h"
 #include "sysemu/char.h"
@@ -114,6 +115,11 @@ static void xen_change_state_handler(void
*opaque, int running,

 static int xen_init(MachineState *ms)
 {
+PCMachineState *pcms = PC_MACHINE(ms);
+
+/* Disable ACPI build because Xen handles it */
+pcms->acpi_build_enabled = false;
+
 xen_xc = xc_interface_open(0, 0, 0);
 if (xen_xc == NULL) {
 xen_pv_printf(NULL, 0, "can't open xen interface\n");


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

Re: [Xen-devel] [RFC PATCH 03/24] ARM: GICv3 ITS: allocate device and collection table

[Julien Grall]

Hi Andre,

On 28/09/2016 19:24, Andre Przywara wrote:

Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and
an EventID (the MSI payload or interrupt ID) to a pair of LPI number
and collection ID, which points to the target CPU.
This mapping is stored in the device and collection tables, which software
has to provide for the ITS to use.
Allocate the required memory and hand it the ITS.
We limit the number of devices to cover 4 PCI busses for now.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/gic-its.c| 114 ++
 xen/arch/arm/gic-v3.c |   5 ++
 xen/include/asm-arm/gic-its.h |  49 +-
 3 files changed, 167 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c
index b52dff3..40238a2 100644
--- a/xen/arch/arm/gic-its.c
+++ b/xen/arch/arm/gic-its.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table);
 min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS)
 #define MAX_HOST_LPIS   (BIT(MAX_HOST_LPI_BITS) - 8192)

+#define BASER_ATTR_MASK   \
+((0x3UL << GITS_BASER_SHAREABILITY_SHIFT)   | \
+ (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \
+ (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT))
+#define BASER_RO_MASK   (GENMASK(52, 48) | GENMASK(58, 56))


I did not notice in the previous patch. But I would rather introduce 
GENMASK_ULL to avoid any issue the day we decide to port on 32-bits (for 
similar reason as BIT).



+
+static uint64_t encode_phys_addr(paddr_t addr, int page_bits)


Please make page_bits unsigned int.


+{
+uint64_t ret;
+
+if ( page_bits < 16)


Coding style.


+return (uint64_t)addr & GENMASK(47, page_bits);
+
+ret = addr & GENMASK(47, 16);
+return ret | (addr & GENMASK(51, 48)) >> (48 - 12);
+}
+
+static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items)


unsigned int nr_items

Also can you find a better name for regc and/or reg. The naming is very 
similar and could be error-prone.



+{
+uint64_t attr;
+int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f;


unsigned int here.


+int pagesz;


Same.


+int order;


Same.


+void *buffer = NULL;
+
+attr  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
+attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
+attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT;
+
+/*
+ * Loop over the page sizes (4K, 16K, 64K) to find out what the host
+ * supports.
+ */
+for (pagesz = 0; pagesz < 3; pagesz++)


Coding style: for ( ... )


+{
+uint64_t reg;
+int nr_bytes;


unsigned int nr_bytes.


+
+nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12));
+order = get_order_from_bytes(nr_bytes);
+
+if ( !buffer )
+buffer = alloc_xenheap_pages(order, 0);
+if ( !buffer )
+return -ENOMEM;
+
+reg  = attr;
+reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT);
+reg |= nr_bytes >> (pagesz * 2 + 12);
+reg |= regc & BASER_RO_MASK;
+reg |= GITS_BASER_VALID;
+reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12);
+
+writeq_relaxed(reg, basereg);
+regc = readl_relaxed(basereg);


Should not it be readq_relaxed?


+
+/* The host didn't like our attributes, just use what it returned. */
+if ( (regc & BASER_ATTR_MASK) != attr )
+attr = regc & BASER_ATTR_MASK;


Should we flush the cache if the GIC does not support shareability?



+
+/* If the host accepted our page size, we are done. */
+if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz )


You probably want a define for 3UL << GITS_BASER_PAGE_SIZE_SHIFT.


+return 0;
+
+/* Check whether our buffer is aligned to the next page size already. 
*/
+if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) )


Well the buffer could be aligned to the next page size but the size not 
page aligned. So you would provide the GIC memory that it cannot touch.


So you have to always reallocate the buffer from scratch to avoid this 
issue.



+{
+free_xenheap_pages(buffer, order);
+buffer = NULL;
+}
+}
+
+if ( buffer )
+free_xenheap_pages(buffer, order);
+
+return -EINVAL;
+}
+
+int gicv3_its_init(struct host_its *hw_its)
+{
+uint64_t reg;
+int i;
+
+hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
+if ( !hw_its->its_base )
+return -ENOMEM;
+
+for (i = 0; i < 8; i++)


Coding style: for ( ... )


+{
+void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8;
+

Re: [Xen-devel] [PATCH V4] xen/arm: domain_build: allocate lowmem for dom0 as much as possible

[Stefano Stabellini]

On Tue, 1 Nov 2016, Julien Grall wrote:
> Hi Peng,
> 
> Sorry for the late answer.
> 
> On 23/09/2016 03:55, Peng Fan wrote:
> > On AArch64 SoCs, some IPs may only have the capability to access
> > 32 bits address space. The physical memory assigned for Dom0 maybe
> > not in 4GB address space, then the IPs will not work properly.
> > So need to allocate memory under 4GB for Dom0.
> > 
> > There is no restriction that how much lowmem needs to be allocated for
> > Dom0 ,so allocate lowmem as much as possible for Dom0.
> > 
> > This patch does not affect 32-bit domain, because Variable "lowmem" is
> > set to true at the beginning. If failed to allocate bank0 under 4GB,
> > need to panic for 32-bit domain, because 32-bit domain requires bank0
> > be allocated under 4GB.
> > 
> > For 64-bit domain, set "lowmem" to false, and continue allocating
> > memory from above 4GB.
> > 
> > Signed-off-by: Peng Fan 
> > Cc: Stefano Stabellini 
> > Cc: Julien Grall 
> 
> Reviewed-by: Julien Grall 
> 
> I am undecided whether this should be considered as a bug fix for Xen 4.8. Are
> you aware of any ARM64 platform we currently support requiring allocation of
> memory below 4GB?

I am more comfortable having this in 4.9 (I queued it up in xen-arm-next
for now), unless we have a regression, a concrete problem, with an
existing supported platform, like you wrote.

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

Re: [Xen-devel] [PATCH] Don't clear HCR_VM bit when updating VTTBR.

[Stefano Stabellini]

On Tue, 1 Nov 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 31/10/2016 22:18, Stefano Stabellini wrote:
> > On Mon, 31 Oct 2016, Steve Capper wrote:
> > > On Wed, Oct 19, 2016 at 12:59:45PM -0700, Stefano Stabellini wrote:
> > > > On Mon, 10 Oct 2016, Jun Sun wrote:
> > > > > Currently function p2m_restore_state() would clear HCR_VM bit, i.e.,
> > > > > disabling stage2 translation, before updating VTTBR register. After
> > > > > some research and talking to ARM support, I got confirmed that this is
> > > > > not
> > > > > necessary. We are currently working on a new platform that would need
> > > > > this
> > > > > to be removed.
> > > > > 
> > > > > The patch is tested on FVP foundation model.
> > > > > 
> > > > > Signed-off-by: Jun Sun 
> > > > 
> > > > Hello Jun,
> > > > 
> > > > thanks for the patch and sorry for the late reply. I would appreciate
> > > > feedback from Julien, but he is current AFK.
> > > > 
> > > > Steve,
> > > > can I have your Acked-by on this patch?
> > > > 
> > > 
> > > Apologies for my very late reply on this.
> > > 
> > > I've taken a look and I think that this is okay, so please feel free to
> > > add:
> > > Acked-by: Steve Capper 
> > 
> > Thanks Steve. Given the state of the release, I am going to push the fix
> > to Xen 4.9 unless you or Jun have a strong argument for committing this
> > sooner.
> 
> Not related to this patch. Do you plan to have a branch for queuing all Xen
> 4.9 patches?

Hi Julien, thanks a good suggestion (I am already starting to lose
patches). I created:

git://xenbits.xen.org/people/sstabellini/xen-unstable.git xen-arm-next

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

Re: [Xen-devel] [OSSTEST PATCH] ts-xen-build: set CONFIG_DEBUG for KConfig

[Ian Jackson]

Wei Liu writes ("[OSSTEST PATCH] ts-xen-build: set CONFIG_DEBUG for KConfig"):
> Starting from Xen 4.8 the hypervisor debug build is controlled by
> Kconfig. Set it correctly in xen/.config.
> 
> Signed-off-by: Wei Liu 

Acked-by: Ian Jackson 

Thanks.  Pushed to osstest pretest.

Ian.

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

[Xen-devel] [OSSTEST PATCH] ts-xen-build: set CONFIG_DEBUG for KConfig

[Wei Liu]

Starting from Xen 4.8 the hypervisor debug build is controlled by
Kconfig. Set it correctly in xen/.config.

Signed-off-by: Wei Liu 
---
 ts-xen-build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ts-xen-build b/ts-xen-build
index 07a69ec..3e53d74 100755
--- a/ts-xen-build
+++ b/ts-xen-build
@@ -86,6 +86,7 @@ END
if test -f xen/Kconfig; then
echo >>xen/.config CONFIG_XSM='${build_xsm}'
echo >>xen/.config CONFIG_FLASK='${build_xsm}'
+   echo >>xen/.config CONFIG_DEBUG=$debug_build
fi
echo >>.config XSM_ENABLE='${build_xsm}'
 END
-- 
2.1.4


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

[Xen-devel] [PATCH for-4.8 v2] tools/tests/x86_emulator: Pass -no-pie -fno-pic to gcc on x86_32

[Ian Jackson]

The current build fails with GCC6 on Debian sid i386 (unstable):

 /tmp/ccqjaueF.s: Assembler messages:
 /tmp/ccqjaueF.s:3713: Error: missing or invalid displacement expression 
`vmovd_to_reg_len@GOT'

This is due to the combination of GCC6, and Debian's decision to
enable some hardening flags by default (to try to make runtime
addresses less predictable):
  https://wiki.debian.org/Hardening/PIEByDefaultTransition

This is of no benefit for the x86 instruction emulator test, which is
a rebuild of the emulator code for testing purposes only.  So pass
options to disable this.

These options will be no-ops if they are the same as the compiler
default.

On amd64, the -fno-pic breaks the build in a different way.  So do
this only on i386.

Signed-off-by: Ian Jackson 
CC: Jan Beulich 
CC: Andrew Cooper 

squash! tools/tests/x86_emulator: Pass -no-pie -fno-pic to gcc

Signed-off-by: Ian Jackson 
---
 tools/tests/x86_emulator/Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/tools/tests/x86_emulator/Makefile 
b/tools/tests/x86_emulator/Makefile
index 13ace9a..d349c0f 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -45,6 +45,10 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 
 HOSTCFLAGS += $(CFLAGS_xeninclude)
 
+ifeq ($(XEN_TARGET_ARCH),x86_32)
+HOSTCFLAGS += -no-pie -fno-pic
+endif
+
 x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c 
x86_emulate/x86_emulate.h
$(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
 
-- 
2.10.1


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

Re: [Xen-devel] [DOC v7] PV Calls protocol design

[Stefano Stabellini]

ping?

On Thu, 13 Oct 2016, Stefano Stabellini wrote:
> Hi all,
> 
> This is the design document of the PV Calls protocol. You can find
> prototypes of the Linux frontend and backend drivers here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git pvcalls-7
> 
> To use them, make sure to enable CONFIG_XEN_PVCALLS in your kernel
> config and add "pvcalls=1" to the command line of your DomU Linux
> kernel. You also need the toolstack to create the initial xenstore nodes
> for the protocol. To do that, please apply the attached patch to libxl
> (the patch is based on Xen 4.7.0-rc3) and add "pvcalls=1" to your DomU
> config file.
> 
> Please review!
> 
> Cheers,
> 
> Stefano
> 
> 
> Changes in v7:
> - add a glossary of Xen terms 
> - add a paragraph on why Xen was chosen 
> - wording improvements
> - add links to xenstore documents and headers
> - specify that the current version is "1"
> - rename max-dataring-page-order to max-page-order
> - rename networking-calls to function-calls
> - add links to [Data ring] throughout the document
> - explain the difference between req_id and id
> - mention that future commands larger than 56 bytes will require a
>   version increase
> - mention that the list of commands is in calling order
> - clarify that reuse data rings are found by ref
> - rename ENOTSUPP to ENOTSUP
> - add padding in struct pvcalls_data_intf for cachelining
> - rename pvcalls_ring_queued to pvcalls_ring_unconsumed
> 
> 
> Changes in v6:
> - add reuse field in release command
> - add "networking-calls" backend node on xenstore
> - fixed tab/whitespace indentation
> 
> Changes in v5:
> - clarify text
> - rename id to req_id
> - rename sockid to id
> - move id to request and response specific fields
> - add version node to xenstore
> 
> Changes in v4:
> - rename xensock to pvcalls
> 
> Changes in v3:
> - add a dummy element to struct xen_xensock_request to make sure the
>   size of the struct is the same on both x86_32 and x86_64
> 
> Changes in v2:
> - add max-dataring-page-order
> - add "Publish backend features and transport parameters" to backend
>   xenbus workflow
> - update new cmd values
> - update xen_xensock_request
> - add backlog parameter to listen and binary layout
> - add description of new data ring format (interface+data)
> - modify connect and accept to reflect new data ring format
> - add link to POSIX docs
> - add error numbers
> - add address format section and relevant numeric definitions
> - add explicit mention of unimplemented commands
> - add protocol node name
> - add xenbus shutdown diagram
> - add socket operation
> 
> ---
> 
> 
> # PV Calls Protocol version 1
> 
> ## Glossary
> 
> The following is a list of terms and definitions used in the Xen
> community. If you are a Xen contributor you can skip this section.
> 
> * PV
> 
>   Short for paravirtualized.
> 
> * Dom0
> 
>   First virtual machine that boots. In most configurations Dom0 is
>   privileged and has control over hardware devices, such as network
>   cards, graphic cards, etc.
> 
> * DomU
> 
>   Regular unprivileged Xen virtual machine.
> 
> * Domain
> 
>   A Xen virtual machine. Dom0 and all DomUs are all separate Xen
>   domains.
> 
> * Guest
> 
>   Same as domain: a Xen virtual machine.
> 
> * Frontend
> 
>   Each DomU has one or more paravirtualized frontend drivers to access
>   disks, network, console, graphics, etc. The presence of PV devices is
>   advertized on XenStore, a cross domain key-value database. Frontends
>   are similar in intent to the virtio drivers in Linux.
> 
> * Backend
> 
>   A Xen paravirtualized backend typically runs in Dom0 and it is used to
>   export disks, network, console, graphics, etcs, to DomUs. Backends can
>   live both in kernel space and in userspace. For example xen-blkback
>   lives under drivers/block in the Linux kernel and xen_disk lives under
>   hw/block in QEMU. Paravirtualized backends are similar in intent to
>   virtio device emulators.
> 
> * VMX and SVM
>   
>   On Intel processors, VMX is the CPU flag for VT-x, hardware
>   virtualization support. It corresponds to SVM on AMD processors.
> 
> 
> 
> ## Rationale
> 
> PV Calls is a paravirtualized protocol that allows the implementation of
> a set of POSIX functions in a different domain. The PV Calls frontend
> sends POSIX function calls to the backend, which implements them and
> returns a value to the frontend.
> 
> This version of the document covers networking function calls, such as
> connect, accept, bind, release, listen, poll, recvmsg and sendmsg; but
> the protocol is meant to be easily extended to cover different sets of
> calls. Unimplemented commands return ENOTSUP.
> 
> PV Calls provide the following benefits:
> * full visibility of the guest behavior on the backend domain, allowing
>   for inexpensive filtering and manipulation of any guest calls
> * excellent performance
> 
> Specifically, PV Calls for networking offer these advantages:
> * guest networking works out of the 

Re: [Xen-devel] [PATCH for-4.8] tools/tests/x86_emulator: Pass -no-pie -fno-pic to gcc

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH for-4.8] tools/tests/x86_emulator: Pass -no-pie 
-fno-pic to gcc"):
> Ian Jackson writes ("[PATCH] tools/tests/x86_emulator: Pass -no-pie -fno-pic 
> to gcc"):
> > The current build fails with GCC6 on Debian sid (unstable):
> 
> Something seems wrong with this patch.  I will get back to you.

It breaks the build on amd64.

Ian.

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

Re: [Xen-devel] [PATCH for-4.8] tools/tests/x86_emulator: Pass -no-pie -fno-pic to gcc

[Ian Jackson]

Ian Jackson writes ("[PATCH] tools/tests/x86_emulator: Pass -no-pie -fno-pic to 
gcc"):
> The current build fails with GCC6 on Debian sid (unstable):
> 

Something seems wrong with this patch.  I will get back to you.

Ian.

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

[Xen-devel] [PATCH v2] PCMachineState: introduce acpi_build_enabled field

[Wei Liu]

Introduce this field to control whether ACPI build is enabled by a
particular machine or accelerator.

It defaults to true if the machine itself supports ACPI build. Xen
accelerator will disable it because Xen is in charge of building ACPI
tables for the guest.

Signed-off-by: Wei Liu 
---
Cc: Igor Mammedov 
Cc: Eduardo Habkost 
Cc: Anthony PERARD 
Cc: Stefano Stabellini 
Cc: Sander Eikelenboom 

v2:
1. drop acpi-build property
2. set acpi_build_enabled to acpi_has_build
3. replace acpi_has_build check in acpi_build()
---
 hw/i386/acpi-build.c | 2 +-
 hw/i386/pc.c | 2 ++
 include/hw/i386/pc.h | 2 ++
 xen-common.c | 6 ++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5cd1da9..13cbbde 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2953,7 +2953,7 @@ void acpi_setup(void)
 return;
 }
 
-if (!pcmc->has_acpi_build) {
+if (!pcms->acpi_build_enabled) {
 ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
 return;
 }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f56ea0f..fbd9aed 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2159,6 +2159,8 @@ static void pc_machine_initfn(Object *obj)
 pcms->vmport = ON_OFF_AUTO_AUTO;
 /* nvdimm is disabled on default. */
 pcms->acpi_nvdimm_state.is_enabled = false;
+/* acpi build is enabled by default if machine supports it */
+pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
 }
 
 static void pc_machine_reset(void)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 98dc772..8eb517f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -62,6 +62,8 @@ struct PCMachineState {
 
 AcpiNVDIMMState acpi_nvdimm_state;
 
+bool acpi_build_enabled;
+
 /* RAM information (sizes, addresses, configuration): */
 ram_addr_t below_4g_mem_size, above_4g_mem_size;
 
diff --git a/xen-common.c b/xen-common.c
index 9099760..bacf962 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -9,6 +9,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/i386/pc.h"
 #include "hw/xen/xen_backend.h"
 #include "qmp-commands.h"
 #include "sysemu/char.h"
@@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int 
running,
 
 static int xen_init(MachineState *ms)
 {
+PCMachineState *pcms = PC_MACHINE(ms);
+
+/* Disable ACPI build because Xen handles it */
+pcms->acpi_build_enabled = false;
+
 xen_xc = xc_interface_open(0, 0, 0);
 if (xen_xc == NULL) {
 xen_pv_printf(NULL, 0, "can't open xen interface\n");
-- 
2.1.4


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

Re: [Xen-devel] [RFC PATCH 03/24] ARM: GICv3 ITS: allocate device and collection table

[Julien Grall]

Hi Stefano,

On 26/10/2016 23:57, Stefano Stabellini wrote:

+int pagesz;
+int order;
+void *buffer = NULL;
+
+attr  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
+attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
+attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT;
+
+/*
+ * Loop over the page sizes (4K, 16K, 64K) to find out what the host
+ * supports.
+ */


Is this really the best way to do it? Can't we assume ITS supports 4K,
given that Xen requires 4K pages at the moment? Is it actually possible
to find hardware that supports 4K but with an ITS that only support 64K
or 16K pages? It seems insane to me. Otherwise can't we probe the page
size somehow?


By reading the spec (8.19.1 in IHI 0069C):

"If the GIC implementation supports only a single, fixed page size, this 
field might be RO.
When this register has an architecturally-defined reset value, if this 
field is implemented as an RW

field, it resets to a value that is architecturally UNKNOWN."

As the reset value is architecturally unknown the only way to find out 
the correct page size is to try them one by one.


The GIC is a separate component of the platform and will be programed 
using physical address (and not virtual one). It would be fine to have a 
BASE registers supporting only 64K to save few lines in the GIC.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RFC] PCMachineState: introduce acpi_build_enabled field

[Eduardo Habkost]

On Tue, Nov 01, 2016 at 05:12:37PM +, Wei Liu wrote:
> On Tue, Nov 01, 2016 at 03:02:31PM -0200, Eduardo Habkost wrote:
> > On Tue, Nov 01, 2016 at 04:43:11PM +, Wei Liu wrote:
> > [...]
> > > @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, 
> > > int running,
> > >  
> > >  static int xen_init(MachineState *ms)
> > >  {
> > > +PCMachineState *pcms = PC_MACHINE(ms);
> > > +
> > > +/* Disable ACPI build because Xen handles it */
> > > +pcms->acpi_build_enabled = false;
> > 
> > I just noticed that I don't see any code to disable ACPI build in
> > the case of "-machine pc,accel=xen". I suggest a xen_enabled()
> 
> Hmm... I think this code snippet does exactly that -- xen_init is the
> initialization function for Xen accelerator. So this covers -m xenfv
> (because it sets accelerator to xen) and -m whatever,accel=xen.
> 
> Did I miss anything?

Nevermind, I confused it with pc_xen_hvm_init() (which is
specific for xenfv). The patch looks correct.

-- 
Eduardo

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

Re: [Xen-devel] [RFC PATCH 02/24] ARM: GICv3: allocate LPI pending and property table

[Julien Grall]

Hi Andre,

On 28/09/2016 19:24, Andre Przywara wrote:

The ARM GICv3 ITS provides a new kind of interrupt called LPIs.
The pending bits and the configuration data (priority, enable bits) for
those LPIs are stored in tables in normal memory, which software has to
provide to the hardware.
Allocate the required memory, initialize it and hand it over to each
ITS. We limit the number of LPIs we use with a compile time constant to
avoid wasting memory.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/Kconfig  |  6 
 xen/arch/arm/efi/efi-boot.h   |  1 -
 xen/arch/arm/gic-its.c| 76 +++
 xen/arch/arm/gic-v3.c | 27 ++
 xen/include/asm-arm/cache.h   |  4 +++
 xen/include/asm-arm/gic-its.h | 22 +++-
 xen/include/asm-arm/gic_v3_defs.h | 48 -
 7 files changed, 181 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 9fe3b8e..66e2bb8 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -50,6 +50,12 @@ config HAS_ITS
 depends on ARM_64
 depends on HAS_GICV3

+config HOST_LPI_BITS
+depends on HAS_ITS
+int "Maximum bits for GICv3 host LPIs (14-32)"
+range 14 32
+default "20"
+


This would be better to be defined as a parameter command line. So the 
user does not need to rebuild Xen in order to increase the number of 
bits supported. It would also be useful to get a rational behind the 
default number in the commit message.



 config ALTERNATIVE
bool

diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index 045d6ce..dc64aec 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -10,7 +10,6 @@
 #include "efi-dom0.h"

 void noreturn efi_xen_start(void *fdt_ptr, uint32_t fdt_size);
-void __flush_dcache_area(const void *vaddr, unsigned long size);

 #define DEVICE_TREE_GUID \
 {0xb1b621d5, 0xf19c, 0x41a5, {0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0}}
diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c
index 0f42a77..b52dff3 100644
--- a/xen/arch/arm/gic-its.c
+++ b/xen/arch/arm/gic-its.c


Please rename this file gic-v3-its to make clear ITS is only GICv3.


@@ -20,10 +20,86 @@
 #include 
 #include 
 #include 
+#include 


Why did you include p2m.h? This header contains stage-2 page table 
functions but I don't see any use of them within this patch.



 #include 
 #include 
 #include 

+/* Global state */
+static struct {
+uint8_t *lpi_property;
+int host_lpi_bits;


Please use unsigned int.


+} lpi_data;
+
+/* Pending table for each redistributor */
+static DEFINE_PER_CPU(void *, pending_table);
+
+#define MAX_HOST_LPI_BITS\
+min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS)


Why don't you directly initialize host_lpi_bits to the correct value? 
This would avoid to compute the min every time you use MAX_HOST_LPI_BITS 
and save few instructions.



+#define MAX_HOST_LPIS   (BIT(MAX_HOST_LPI_BITS) - 8192)


I know that we don't support ITS for 32-bits, but I would rather avoid 
to use BIT as this macro is working on unsigned long. I would prefer if 
you introduce BIT_ULL or open-code.



+
+uint64_t gicv3_lpi_allocate_pendtable(void)
+{
+uint64_t reg, attr;
+void *pendtable;
+
+attr  = GIC_BASER_CACHE_RaWaWb << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT;
+attr |= GIC_BASER_CACHE_SameAsInner << 
GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT;
+attr |= GIC_BASER_InnerShareable << GICR_PENDBASER_SHAREABILITY_SHIFT;


From the spec (8.11.18 in ARM IHI 0069C) the cacheability and 
shareability could be fixed (though it marked as deprecated). Should we 
check whether the value stick?


Also the variable attr sounds pointless as you will directly assign the 
value to reg with no more computation.



+
+/*
+ * The pending table holds one bit per LPI, so we need three bits less
+ * than the number of LPI_BITs. But the alignment requirement from the
+ * ITS is 64K, so make order at least 16 (-12).
+ */
+pendtable = alloc_xenheap_pages(MAX(lpi_data.host_lpi_bits - 3, 16) - 12, 
0);
+if ( !pendtable )
+return 0;
+
+memset(pendtable, 0, BIT(lpi_data.host_lpi_bits - 3));


Same remark for BIT here.


+this_cpu(pending_table) = pendtable;
+
+reg  = attr | GICR_PENDBASER_PTZ;
+reg |= virt_to_maddr(pendtable) & GENMASK(51, 16);


I don't think the mask is useful and would need to be changed if the 
physical address bits increased as it was done in ARMv8.2.



+
+return reg;
+}
+
+uint64_t gicv3_lpi_get_proptable()
+{
+uint64_t attr;
+static uint64_t reg = 0;
+
+/* The property table is shared across all redistributors. */
+if ( reg )
+return reg;
+
+attr  = GIC_BASER_CACHE_RaWaWb << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT;
+attr |= 

Re: [Xen-devel] [PATCH RFC] PCMachineState: introduce acpi_build_enabled field

[Wei Liu]

On Tue, Nov 01, 2016 at 03:02:31PM -0200, Eduardo Habkost wrote:
> On Tue, Nov 01, 2016 at 04:43:11PM +, Wei Liu wrote:
> [...]
> > @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int 
> > running,
> >  
> >  static int xen_init(MachineState *ms)
> >  {
> > +PCMachineState *pcms = PC_MACHINE(ms);
> > +
> > +/* Disable ACPI build because Xen handles it */
> > +pcms->acpi_build_enabled = false;
> 
> I just noticed that I don't see any code to disable ACPI build in
> the case of "-machine pc,accel=xen". I suggest a xen_enabled()

Hmm... I think this code snippet does exactly that -- xen_init is the
initialization function for Xen accelerator. So this covers -m xenfv
(because it sets accelerator to xen) and -m whatever,accel=xen.

Did I miss anything?

Wei.

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

Re: [Xen-devel] [PATCH] tools/tests/x86_emulator: Pass -no-pie -fno-pic to gcc

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH] tools/tests/x86_emulator: Pass -no-pie 
-fno-pic to gcc"):
> On 01/11/16 16:58, Ian Jackson wrote:
> > Jan Beulich writes ("Re: [PATCH] tools/tests/x86_emulator: Pass -no-pie 
> > -fno-pic to gcc"):
> >> Ian Jackson  11/01/16 5:46 PM >>>
> >>> --- a/tools/tests/x86_emulator/Makefile
> >>> +++ b/tools/tests/x86_emulator/Makefile
> >>> @@ -44,6 +44,7 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> >>> [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
> >>  >
> >>> HOSTCFLAGS += $(CFLAGS_xeninclude)
> >>> +HOSTCFLAGS += -no-pie -fno-pic
> >>  
> >> -fno-pic surely has been supported forever, but what about -no-pie? Do all
> >> gcc versions we support / the tools subtree can be built with understand 
> >> it?
> > TBH, I don't know.  You're the one with the ancient compilers, can you
> > check ? :-)  I don't have a gcc that ancient here and checking will be
> > easier than trying to divine the answer from changelogs.
> 
> Also, any reason why this isn't on-list?

My original mail was mistakenly not on-list.  Jan replied to that
rather than the followup.

Ian.

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

Re: [Xen-devel] [PATCH RFC] PCMachineState: introduce acpi_build_enabled field

[Eduardo Habkost]

On Tue, Nov 01, 2016 at 04:43:11PM +, Wei Liu wrote:
[...]
> @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int 
> running,
>  
>  static int xen_init(MachineState *ms)
>  {
> +PCMachineState *pcms = PC_MACHINE(ms);
> +
> +/* Disable ACPI build because Xen handles it */
> +pcms->acpi_build_enabled = false;

I just noticed that I don't see any code to disable ACPI build in
the case of "-machine pc,accel=xen". I suggest a xen_enabled()
check in pc_init1() and pc_q35_init().

-- 
Eduardo

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

Re: [Xen-devel] [PATCH RFC] PCMachineState: introduce acpi_build_enabled field

[Wei Liu]

On Tue, Nov 01, 2016 at 02:59:25PM -0200, Eduardo Habkost wrote:
> On Tue, Nov 01, 2016 at 04:53:17PM +, Wei Liu wrote:
> > On Tue, Nov 01, 2016 at 02:48:27PM -0200, Eduardo Habkost wrote:
> > [...]
> > > >  static void pc_machine_set_nvdimm(Object *obj, bool value, Error 
> > > > **errp)
> > > >  {
> > > >  PCMachineState *pcms = PC_MACHINE(obj);
> > > > @@ -2159,6 +2173,8 @@ static void pc_machine_initfn(Object *obj)
> > > >  pcms->vmport = ON_OFF_AUTO_AUTO;
> > > >  /* nvdimm is disabled on default. */
> > > >  pcms->acpi_nvdimm_state.is_enabled = false;
> > > > +/* acpi build is enabled by default. */
> > > > +pcms->acpi_build_enabled = true;
> > > 
> > > If you set:
> > >   pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
> > > the default value will be more consistent with the actual results
> > > (where pc-1.6 and older don't have ACPI build). Then you would
> > > probably be able to remove the pcmc->has_acpi_build check from
> > > acpi_setup() and only check pcms->acpi_build_enabled.
> > > 
> > 
> > Thank you for your good advice.
> > 
> > I take it that you're ok with the name of the field and the code in
> > general? If so I will drop RFC tag in my next submission.
> 
> The rest looks good to me, except that I don't see a reason to
> add a "acpi-build" property if it's not used for anything (all
> code is using the struct field directly).
> 

OK. I'm fine with deleting that. I was just following existing examples.

Wei.

> -- 
> Eduardo

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

Re: [Xen-devel] [PATCH RFC] PCMachineState: introduce acpi_build_enabled field

[Eduardo Habkost]

On Tue, Nov 01, 2016 at 04:53:17PM +, Wei Liu wrote:
> On Tue, Nov 01, 2016 at 02:48:27PM -0200, Eduardo Habkost wrote:
> [...]
> > >  static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
> > >  {
> > >  PCMachineState *pcms = PC_MACHINE(obj);
> > > @@ -2159,6 +2173,8 @@ static void pc_machine_initfn(Object *obj)
> > >  pcms->vmport = ON_OFF_AUTO_AUTO;
> > >  /* nvdimm is disabled on default. */
> > >  pcms->acpi_nvdimm_state.is_enabled = false;
> > > +/* acpi build is enabled by default. */
> > > +pcms->acpi_build_enabled = true;
> > 
> > If you set:
> >   pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
> > the default value will be more consistent with the actual results
> > (where pc-1.6 and older don't have ACPI build). Then you would
> > probably be able to remove the pcmc->has_acpi_build check from
> > acpi_setup() and only check pcms->acpi_build_enabled.
> > 
> 
> Thank you for your good advice.
> 
> I take it that you're ok with the name of the field and the code in
> general? If so I will drop RFC tag in my next submission.

The rest looks good to me, except that I don't see a reason to
add a "acpi-build" property if it's not used for anything (all
code is using the struct field directly).

-- 
Eduardo

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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Andrew Cooper]

On 01/11/16 16:56, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug 
> option"):
>> On Mon, Oct 31, 2016 at 05:09:46PM +, Wei Liu wrote:
>>> -# A debug build of Xen and tools?
>>> +# A debug build of tools?
>> For this to hold true, a patch like this is needed:
>>
>> Please let me know what you think.
> ...
>> For hypervisor, the flags previously added by debug option is now
>> controlled by CONFIG_DEBUG.
> So there will no longer be one place where debug is enabled.  That's a
> shame.  It would need to come with an osstest patch...

It hasn't been the case for almost all of 4.8

Attempting to build Xen with an explicit debug= set yields "You must use
'make menuconfig' to enable/disable debug now."

I personally would prefer if debug= still worked, but apparently that is
prohibitively difficult to do.

~Andrew

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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Tue, Nov 01, 2016 at 10:42:37AM -0600, Jan Beulich wrote:
> >>> Wei Liu  11/01/16 5:38 PM >>>
> >On Tue, Nov 01, 2016 at 10:33:54AM -0600, Jan Beulich wrote:
> >> >>> Wei Liu  11/01/16 5:20 PM >>>
> >> >On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
> >> >> >>> Wei Liu  11/01/16 2:48 PM >>>
> >> >> >config/StdGNU.mk | 8 
> >> >> >config/SunOS.mk  | 7 ---
> >> >> >tools/Rules.mk   | 4 +++-
> >> >> >xen/Rules.mk | 6 ++
> >> >> >4 files changed, 9 insertions(+), 16 deletions(-)
> >> >> 
> >> >> Considering this diffstat - did the original global settings not get 
> >> >> inherited
> >> >> by any of the other subtrees, namely stubdom/ and/or extras/ ?
> >> > 
> >> >I mentioned this in commit message and deliberately made the decision to
> >> >not inherit these flags for stubdom build.
> >> 
> >> Indeed, I've overlooked that part, but I'm not fully convinced. And then
> >
> >Not fully convinced because? Can you be more specific?
> 
> I don't know very much about the stubdom build machinery, but I could
> easily imaging some top level setting to have made it through to some
> of the sub-components, in which case this change would have a
> functional impact.
> 

I don't feel strongly about this. I can move some flags into stubdom
build system as well, just to err on the safe side.

Wei.

> Jan
> 

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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Ian Jackson]

Wei Liu writes ("Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug 
option"):
> On Mon, Oct 31, 2016 at 05:09:46PM +, Wei Liu wrote:
> > -# A debug build of Xen and tools?
> > +# A debug build of tools?
> 
> For this to hold true, a patch like this is needed:
> 
> Please let me know what you think.
...
> For hypervisor, the flags previously added by debug option is now
> controlled by CONFIG_DEBUG.

So there will no longer be one place where debug is enabled.  That's a
shame.  It would need to come with an osstest patch...

Ian.

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

Re: [Xen-devel] [PATCH RFC] PCMachineState: introduce acpi_build_enabled field

[Wei Liu]

On Tue, Nov 01, 2016 at 02:48:27PM -0200, Eduardo Habkost wrote:
[...]
> >  static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
> >  {
> >  PCMachineState *pcms = PC_MACHINE(obj);
> > @@ -2159,6 +2173,8 @@ static void pc_machine_initfn(Object *obj)
> >  pcms->vmport = ON_OFF_AUTO_AUTO;
> >  /* nvdimm is disabled on default. */
> >  pcms->acpi_nvdimm_state.is_enabled = false;
> > +/* acpi build is enabled by default. */
> > +pcms->acpi_build_enabled = true;
> 
> If you set:
>   pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
> the default value will be more consistent with the actual results
> (where pc-1.6 and older don't have ACPI build). Then you would
> probably be able to remove the pcmc->has_acpi_build check from
> acpi_setup() and only check pcms->acpi_build_enabled.
> 

Thank you for your good advice.

I take it that you're ok with the name of the field and the code in
general? If so I will drop RFC tag in my next submission.

Wei.

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

Re: [Xen-devel] [PATCH RFC] PCMachineState: introduce acpi_build_enabled field

[Eduardo Habkost]

On Tue, Nov 01, 2016 at 04:43:11PM +, Wei Liu wrote:
> Introduce this field to control whether ACPI build is enabled by a
> particular machine or accelerator.
> 
> It defaults to true so that PC machine has ACPI build by default. Xen
> accelerator will disable it because Xen is in charge of building ACPI
> tables for the guest.
> 
> Signed-off-by: Wei Liu 
> ---
> Cc: Igor Mammedov 
> Cc: Eduardo Habkost 
> Cc: Anthony PERARD 
> Cc: Stefano Stabellini 
> Cc: Sander Eikelenboom 
> 
> Tested a backport version which only involves trivial code movement. It
> worked with both -m xenfv and -m pc,accel=xen.
> 
> Sander, if you want the backported patch please let me know.
> ---
>  hw/i386/acpi-build.c |  2 +-
>  hw/i386/pc.c | 19 +++
>  include/hw/i386/pc.h |  3 +++
>  xen-common.c |  6 ++
>  4 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 93be96f..a5cd2fd 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2954,7 +2954,7 @@ void acpi_setup(void)
>  return;
>  }
>  
> -if (!pcmc->has_acpi_build) {
> +if (!pcmc->has_acpi_build || !pcms->acpi_build_enabled) {
>  ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
>  return;
>  }
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f56ea0f..3e7982f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2143,6 +2143,20 @@ static bool pc_machine_get_nvdimm(Object *obj, Error 
> **errp)
>  return pcms->acpi_nvdimm_state.is_enabled;
>  }
>  
> +static bool pc_machine_get_acpi_build(Object *obj, Error **errp)
> +{
> +PCMachineState *pcms = PC_MACHINE(obj);
> +
> +return pcms->acpi_build_enabled;
> +}
> +
> +static void pc_machine_set_acpi_build(Object *obj, bool value, Error **errp)
> +{
> +PCMachineState *pcms = PC_MACHINE(obj);
> +
> +pcms->acpi_build_enabled = value;
> +}
> +
>  static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
>  {
>  PCMachineState *pcms = PC_MACHINE(obj);
> @@ -2159,6 +2173,8 @@ static void pc_machine_initfn(Object *obj)
>  pcms->vmport = ON_OFF_AUTO_AUTO;
>  /* nvdimm is disabled on default. */
>  pcms->acpi_nvdimm_state.is_enabled = false;
> +/* acpi build is enabled by default. */
> +pcms->acpi_build_enabled = true;

If you set:
  pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
the default value will be more consistent with the actual results
(where pc-1.6 and older don't have ACPI build). Then you would
probably be able to remove the pcmc->has_acpi_build check from
acpi_setup() and only check pcms->acpi_build_enabled.

>  }
>  
>  static void pc_machine_reset(void)
> @@ -2319,6 +2335,9 @@ static void pc_machine_class_init(ObjectClass *oc, void 
> *data)
>  
>  object_class_property_add_bool(oc, PC_MACHINE_NVDIMM,
>  pc_machine_get_nvdimm, pc_machine_set_nvdimm, _abort);
> +
> +object_class_property_add_bool(oc, PC_MACHINE_ACPI_BUILD,
> +pc_machine_get_acpi_build, pc_machine_set_acpi_build, _abort);
>  }
>  
>  static const TypeInfo pc_machine_info = {
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 17fff80..ec8cd0c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -63,6 +63,8 @@ struct PCMachineState {
>  
>  AcpiNVDIMMState acpi_nvdimm_state;
>  
> +bool acpi_build_enabled;
> +
>  /* RAM information (sizes, addresses, configuration): */
>  ram_addr_t below_4g_mem_size, above_4g_mem_size;
>  
> @@ -87,6 +89,7 @@ struct PCMachineState {
>  #define PC_MACHINE_VMPORT   "vmport"
>  #define PC_MACHINE_SMM  "smm"
>  #define PC_MACHINE_NVDIMM   "nvdimm"
> +#define PC_MACHINE_ACPI_BUILD   "acpi-build"
>  
>  /**
>   * PCMachineClass:
> diff --git a/xen-common.c b/xen-common.c
> index e641ad1..b1858d7 100644
> --- a/xen-common.c
> +++ b/xen-common.c
> @@ -9,6 +9,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "hw/i386/pc.h"
>  #include "hw/xen/xen_backend.h"
>  #include "qmp-commands.h"
>  #include "sysemu/char.h"
> @@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int 
> running,
>  
>  static int xen_init(MachineState *ms)
>  {
> +PCMachineState *pcms = PC_MACHINE(ms);
> +
> +/* Disable ACPI build because Xen handles it */
> +pcms->acpi_build_enabled = false;
> +
>  xen_xc = xc_interface_open(0, 0, 0);
>  if (xen_xc == NULL) {
>  xen_be_printf(NULL, 0, "can't open xen interface\n");
> -- 
> 2.1.4
> 

-- 
Eduardo

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

[Xen-devel] [PATCH RFC] PCMachineState: introduce acpi_build_enabled field

[Wei Liu]

Introduce this field to control whether ACPI build is enabled by a
particular machine or accelerator.

It defaults to true so that PC machine has ACPI build by default. Xen
accelerator will disable it because Xen is in charge of building ACPI
tables for the guest.

Signed-off-by: Wei Liu 
---
Cc: Igor Mammedov 
Cc: Eduardo Habkost 
Cc: Anthony PERARD 
Cc: Stefano Stabellini 
Cc: Sander Eikelenboom 

Tested a backport version which only involves trivial code movement. It
worked with both -m xenfv and -m pc,accel=xen.

Sander, if you want the backported patch please let me know.
---
 hw/i386/acpi-build.c |  2 +-
 hw/i386/pc.c | 19 +++
 include/hw/i386/pc.h |  3 +++
 xen-common.c |  6 ++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 93be96f..a5cd2fd 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2954,7 +2954,7 @@ void acpi_setup(void)
 return;
 }
 
-if (!pcmc->has_acpi_build) {
+if (!pcmc->has_acpi_build || !pcms->acpi_build_enabled) {
 ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
 return;
 }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f56ea0f..3e7982f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2143,6 +2143,20 @@ static bool pc_machine_get_nvdimm(Object *obj, Error 
**errp)
 return pcms->acpi_nvdimm_state.is_enabled;
 }
 
+static bool pc_machine_get_acpi_build(Object *obj, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+return pcms->acpi_build_enabled;
+}
+
+static void pc_machine_set_acpi_build(Object *obj, bool value, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+pcms->acpi_build_enabled = value;
+}
+
 static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
 {
 PCMachineState *pcms = PC_MACHINE(obj);
@@ -2159,6 +2173,8 @@ static void pc_machine_initfn(Object *obj)
 pcms->vmport = ON_OFF_AUTO_AUTO;
 /* nvdimm is disabled on default. */
 pcms->acpi_nvdimm_state.is_enabled = false;
+/* acpi build is enabled by default. */
+pcms->acpi_build_enabled = true;
 }
 
 static void pc_machine_reset(void)
@@ -2319,6 +2335,9 @@ static void pc_machine_class_init(ObjectClass *oc, void 
*data)
 
 object_class_property_add_bool(oc, PC_MACHINE_NVDIMM,
 pc_machine_get_nvdimm, pc_machine_set_nvdimm, _abort);
+
+object_class_property_add_bool(oc, PC_MACHINE_ACPI_BUILD,
+pc_machine_get_acpi_build, pc_machine_set_acpi_build, _abort);
 }
 
 static const TypeInfo pc_machine_info = {
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 17fff80..ec8cd0c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -63,6 +63,8 @@ struct PCMachineState {
 
 AcpiNVDIMMState acpi_nvdimm_state;
 
+bool acpi_build_enabled;
+
 /* RAM information (sizes, addresses, configuration): */
 ram_addr_t below_4g_mem_size, above_4g_mem_size;
 
@@ -87,6 +89,7 @@ struct PCMachineState {
 #define PC_MACHINE_VMPORT   "vmport"
 #define PC_MACHINE_SMM  "smm"
 #define PC_MACHINE_NVDIMM   "nvdimm"
+#define PC_MACHINE_ACPI_BUILD   "acpi-build"
 
 /**
  * PCMachineClass:
diff --git a/xen-common.c b/xen-common.c
index e641ad1..b1858d7 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -9,6 +9,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/i386/pc.h"
 #include "hw/xen/xen_backend.h"
 #include "qmp-commands.h"
 #include "sysemu/char.h"
@@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int 
running,
 
 static int xen_init(MachineState *ms)
 {
+PCMachineState *pcms = PC_MACHINE(ms);
+
+/* Disable ACPI build because Xen handles it */
+pcms->acpi_build_enabled = false;
+
 xen_xc = xc_interface_open(0, 0, 0);
 if (xen_xc == NULL) {
 xen_be_printf(NULL, 0, "can't open xen interface\n");
-- 
2.1.4


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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Jan Beulich]

>>> Wei Liu  11/01/16 5:38 PM >>>
>On Tue, Nov 01, 2016 at 10:33:54AM -0600, Jan Beulich wrote:
>> >>> Wei Liu  11/01/16 5:20 PM >>>
>> >On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
>> >> >>> Wei Liu  11/01/16 2:48 PM >>>
>> >> >config/StdGNU.mk | 8 
>> >> >config/SunOS.mk  | 7 ---
>> >> >tools/Rules.mk   | 4 +++-
>> >> >xen/Rules.mk | 6 ++
>> >> >4 files changed, 9 insertions(+), 16 deletions(-)
>> >> 
>> >> Considering this diffstat - did the original global settings not get 
>> >> inherited
>> >> by any of the other subtrees, namely stubdom/ and/or extras/ ?
>> > 
>> >I mentioned this in commit message and deliberately made the decision to
>> >not inherit these flags for stubdom build.
>> 
>> Indeed, I've overlooked that part, but I'm not fully convinced. And then
>
>Not fully convinced because? Can you be more specific?

I don't know very much about the stubdom build machinery, but I could
easily imaging some top level setting to have made it through to some
of the sub-components, in which case this change would have a
functional impact.

Jan


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

Re: [Xen-devel] xc_hvm_inject_trap() races

[Jan Beulich]

>>> Andrei Vlad LUTAS  11/01/16 5:13 PM >>>

First of all, please don't top post.

>First of all, to answer your original question: the injection decision is made
>when the introspection logic needs to inspect a page that is not present in
>the physical memory. We don't really care if the current instruction triggers
>multiple faults or not (and here I'm not sure what you mean by that - multiple
>exceptions, or multiple EPT violations - but the answer is still the same),
>and removing the page restrictions after the #PF injection is introspection
>specific logic - the address for which we inject the #PF doesn't have to be
>related in any way to the current instruction.

Ah, that's this no-architectural behavior again. What if the OS doesn't fully
carry out the page-in, relying on the #PF to retrigger once the insn for which
it got reported has been restarted? Or what if the page gets paged out again
before the insn actually gets to execute (e.g. because a re-schedule
happened inside the guest on the way out of the #PF handler)? All of this
suggests that you really can't lift any restrictions _before_ seeing what you
need to see.

>Assuming that we wouldn't remove the restrictions and we would rely on
>re-generating the event - that is not acceptable: first of all because the
>instruction would normally be emulated anyway before re-entering the guest,

How would that be a problem?

>and secondly because that is not a normal CPU behavior

This really is the main problem here, afaict.

Jan

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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Tue, Nov 01, 2016 at 10:33:54AM -0600, Jan Beulich wrote:
> >>> Wei Liu  11/01/16 5:20 PM >>>
> >On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
> >> >>> Wei Liu  11/01/16 2:48 PM >>>
> >> >config/StdGNU.mk | 8 
> >> >config/SunOS.mk  | 7 ---
> >> >tools/Rules.mk   | 4 +++-
> >> >xen/Rules.mk | 6 ++
> >> >4 files changed, 9 insertions(+), 16 deletions(-)
> >> 
> >> Considering this diffstat - did the original global settings not get 
> >> inherited
> >> by any of the other subtrees, namely stubdom/ and/or extras/ ?
> > 
> >I mentioned this in commit message and deliberately made the decision to
> >not inherit these flags for stubdom build.
> 
> Indeed, I've overlooked that part, but I'm not fully convinced. And then

Not fully convinced because? Can you be more specific?

> extras/ and possible other subtrees don't get mentioned (at the very
> least I'd expect a sentence clarifying that no other subtrees are
> affected, but as said at least for extras/ I'm not sure: That's where
> mini-os gets linked in, and I don't recall whether its build machinery
> got fully separated).
> 

Mini-os got its own build system after I separated it out from Xen.git.
It should be dealt with separately if necessary.  I can mention that in
commit message as well.

Wei.

> Jan
> 

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

[Xen-devel] [PATCH for-4.8] tools/tests/x86_emulator: Pass -no-pie -fno-pic to gcc

[Ian Jackson]

[resending with slightly improved headers]

The current build fails with GCC6 on Debian sid (unstable):

 /tmp/ccqjaueF.s: Assembler messages:
 /tmp/ccqjaueF.s:3713: Error: missing or invalid displacement expression 
`vmovd_to_reg_len@GOT'

This is due to the combination of GCC6, and Debian's decision to
enable some hardening flags by default (to try to make runtime
addresses less predictable):
  https://wiki.debian.org/Hardening/PIEByDefaultTransition

This is of no benefit for the x86 instruction emulator test, which is
a rebuild of the emulator code for testing purposes only.  So pass
options to disable this.

These options will be no-ops if they are the same as the compiler
default.

Signed-off-by: Ian Jackson 
CC: Jan Beulich 
CC: Andrew Cooper 
---
 tools/tests/x86_emulator/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/tests/x86_emulator/Makefile 
b/tools/tests/x86_emulator/Makefile
index 13ace9a..6281fdf 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -44,6 +44,7 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
 HOSTCFLAGS += $(CFLAGS_xeninclude)
+HOSTCFLAGS += -no-pie -fno-pic
 
 x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c 
x86_emulate/x86_emulate.h
$(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
-- 
2.10.1


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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Jan Beulich]

>>> Wei Liu  11/01/16 5:20 PM >>>
>On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
>> >>> Wei Liu  11/01/16 2:48 PM >>>
>> >config/StdGNU.mk | 8 
>> >config/SunOS.mk  | 7 ---
>> >tools/Rules.mk   | 4 +++-
>> >xen/Rules.mk | 6 ++
>> >4 files changed, 9 insertions(+), 16 deletions(-)
>> 
>> Considering this diffstat - did the original global settings not get 
>> inherited
>> by any of the other subtrees, namely stubdom/ and/or extras/ ?
> 
>I mentioned this in commit message and deliberately made the decision to
>not inherit these flags for stubdom build.

Indeed, I've overlooked that part, but I'm not fully convinced. And then
extras/ and possible other subtrees don't get mentioned (at the very
least I'd expect a sentence clarifying that no other subtrees are
affected, but as said at least for extras/ I'm not sure: That's where
mini-os gets linked in, and I don't recall whether its build machinery
got fully separated).

Jan

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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
> >>> Wei Liu  11/01/16 2:48 PM >>>
> >config/StdGNU.mk | 8 
> >config/SunOS.mk  | 7 ---
> >tools/Rules.mk   | 4 +++-
> >xen/Rules.mk | 6 ++
> >4 files changed, 9 insertions(+), 16 deletions(-)
> 
> Considering this diffstat - did the original global settings not get inherited
> by any of the other subtrees, namely stubdom/ and/or extras/ ?
> 

I mentioned this in commit message and deliberately made the decision to
not inherit these flags for stubdom build.

Wei.

> Jan
> 

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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Jan Beulich]

>>> Wei Liu  11/01/16 2:48 PM >>>
>config/StdGNU.mk | 8 
>config/SunOS.mk  | 7 ---
>tools/Rules.mk   | 4 +++-
>xen/Rules.mk | 6 ++
>4 files changed, 9 insertions(+), 16 deletions(-)

Considering this diffstat - did the original global settings not get inherited
by any of the other subtrees, namely stubdom/ and/or extras/ ?

Jan

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

[Xen-devel] [linux-3.10 test] 101837: regressions - FAIL

[osstest service owner]

flight 101837 linux-3.10 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101837/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-amd64-pvgrub  6 xen-bootfail REGR. vs. 100648
 test-amd64-i386-xl-xsm6 xen-boot fail REGR. vs. 100648
 test-amd64-i386-qemut-rhel6hvm-intel  6 xen-boot fail REGR. vs. 100648
 test-amd64-amd64-xl   6 xen-boot fail REGR. vs. 100648
 test-amd64-amd64-xl-credit2   6 xen-boot fail REGR. vs. 100648
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 100648
 test-amd64-i386-xl-qemuu-ovmf-amd64  6 xen-boot  fail REGR. vs. 100648
 test-amd64-i386-xl6 xen-boot fail REGR. vs. 100648

Tests which are failing intermittently (not blocking):
 test-amd64-i386-libvirt-xsm   9 debian-install   fail in 101783 pass in 101837
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail pass in 101576
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail pass in 101594
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 6 xen-boot fail pass in 
101663
 test-amd64-i386-libvirt   6 xen-boot   fail pass in 101680
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  6 xen-boot  fail pass in 101680
 test-amd64-i386-xl-qemuu-debianhvm-amd64  6 xen-boot   fail pass in 101731
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail pass in 
101731
 test-amd64-amd64-libvirt-pair  9 xen-boot/src_host fail pass in 101731
 test-amd64-amd64-libvirt-pair 10 xen-boot/dst_host fail pass in 101731
 test-amd64-amd64-xl-qemut-winxpsp3  6 xen-boot fail pass in 101783
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-boot   fail pass in 101783
 test-amd64-i386-libvirt-pair  9 xen-boot/src_host  fail pass in 101800
 test-amd64-i386-libvirt-pair 10 xen-boot/dst_host  fail pass in 101800
 test-amd64-i386-pair  9 xen-boot/src_host  fail pass in 101800
 test-amd64-i386-pair 10 xen-boot/dst_host  fail pass in 101800
 test-amd64-amd64-qemuu-nested-intel  6 xen-bootfail pass in 101814
 test-amd64-amd64-pair 9 xen-boot/src_host  fail pass in 101828
 test-amd64-amd64-pair10 xen-boot/dst_host  fail pass in 101828
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail pass in 
101828
 test-amd64-i386-qemuu-rhel6hvm-intel  6 xen-boot   fail pass in 101828

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 15 
guest-localmigrate/x10 fail in 101594 like 100646
 build-i386-rumprun5 rumprun-build fail in 101663 baseline untested
 build-amd64-rumprun   5 rumprun-build fail in 101663 baseline untested
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 100648
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 100648

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt 12 migrate-support-check fail in 101680 never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail in 101680 never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail in 101783 never pass
 build-i386-rumprun7 xen-buildfail   never pass
 build-amd64-rumprun   7 xen-buildfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass

version targeted for testing:
 linux7828a9658951301a3fd83daa4ed0a607d370399e
baseline version:
 linux2ecaf1d025af0f481d00b3701ffbcc600dcab076

Last test of basis   100648  2016-08-28 23:14:14 Z   64 days
Testing same since   101576  2016-10-21 10:51:14 Z   11 days   18 attempts


People who touched revisions under test:
  Andrea Arcangeli 
  Andrew Morton 
  Bjorn Helgaas 
  Casey Schaufler 
  Dan Carpenter 
  Dave Carroll 
  Greg Kroah-Hartman 

Re: [Xen-devel] xc_hvm_inject_trap() races

[Andrei Vlad LUTAS]

Hello,

First of all, to answer your original question: the injection decision is made 
when the introspection logic needs to inspect a page that is not present in the 
physical memory. We don't really care if the current instruction triggers 
multiple faults or not (and here I'm not sure what you mean by that - multiple 
exceptions, or multiple EPT violations - but the answer is still the same), and 
removing the page restrictions after the #PF injection is introspection 
specific logic - the address for which we inject the #PF doesn't have to be 
related in any way to the current instruction. Assuming that we wouldn't remove 
the restrictions and we would rely on re-generating the event - that is not 
acceptable: first of all because the instruction would normally be emulated 
anyway before re-entering the guest, and secondly because that is not a normal 
CPU behavior (and it would break internal introspection logic) - once an 
instruction triggered an exit, it should be emulated or single-stepped.

Best regards,
Andrei.
 
-Original Message-
From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of Jan 
Beulich
Sent: 1 November, 2016 17:54
To: rcojoc...@bitdefender.com
Cc: andrew.coop...@citrix.com; ta...@tklengyel.com; 
xen-de...@lists.xenproject.org
Subject: Re: [Xen-devel] xc_hvm_inject_trap() races

>>> Razvan Cojocaru  11/01/16 11:53 AM >>>
>On 11/01/2016 12:30 PM, Jan Beulich wrote:
> Razvan Cojocaru  11/01/16 10:04 AM >>>
>>> We've stumbled across the following scenario: we're injecting a #PF 
>>> to try to bring a swapped page back, but Xen already have a pending 
>>> interrupt, and the two collide.
>>>
>>> I've logged what happens in hvm_do_resume() at the point of 
>>> injection, and stumbled across this:
>>>
>>> (XEN) [  252.878389] vector: 14, type: 3, error_code: 0,
>>> VM_ENTRY_INTR_INFO: 0x80e1
>>>
>>> VM_ENTRY_INTR_INFO does have INTR_INFO_VALID_MASK set here.
>> 
>> So a first question I have is this: What are the criteria that made 
>> your application decide it needs to inject a trap? Obviously there 
>> must have been some kind of event in the guest that triggered this. 
>> Hence the question is whether this same event wouldn't re-trigger at 
>> the end of the hardware interrupt (or could be made re-trigger reasonably 
>> easily).
>> Because in the end what you're trying to do here is something that's 
>> architecturally impossible: There can't be a (non-nested) exception 
>> once an external interrupt has been accepted (i.e. any subsequent 
>> exception can only be related to the delivery of that interrupt 
>> vector, not to the code which was running when the interrupt was signaled).
>
>Unfortunately there are two main reasons why relying on the conditions 
>for injecting the page fault repeating is problematic:
>
>1. We'd need to be able differentiate between a failed run (where 
>injection doesn't work) and a succesful run, with no real possibility 
>to know the difference for sure. So we don't know how to adapt the 
>application's internal state based on some events: is the event the 
>"final" one, or just a duplicate? xc_hvm_inject_trap() does not tell us 
>(as indeed it cannot know) if the injection succeeded, and there's no 
>other way to know.
>
>2. More importantly (although working around 1. is far from trivial), 
>the event may not be repeatable. As an example, #PF injection can occur 
>as part of handling an EPT event, but during handling the event the 
>introspection engine can decide to lift the restrictions on said page 
>after injecting the #PF. So the application relied on the #PF being 
>delivered, and with the restrictions lifted from the page there will be 
>no further EPT events for that page, therefore the main condition for 
>triggering the #PF is lost forever.

Isn't this a problem you also have under other circumstances, e.g.
multiple faults occurring for a single instruction? Which gets us to the fact 
that you didn't answer at all the initial question I did raise. Apart from that 
I'm also not really understanding the model you describe:
What good does injecting #PF alongside lifting restrictions? I'd normally 
expect just one of the two to occur to deal with whatever caused the original 
event.

Jan

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


This email was scanned by Bitdefender
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] xc_hvm_inject_trap() races

[Jan Beulich]

>>> Razvan Cojocaru  11/01/16 11:53 AM >>>
>On 11/01/2016 12:30 PM, Jan Beulich wrote:
> Razvan Cojocaru  11/01/16 10:04 AM >>>
>>> We've stumbled across the following scenario: we're injecting a #PF to
>>> try to bring a swapped page back, but Xen already have a pending
>>> interrupt, and the two collide.
>>>
>>> I've logged what happens in hvm_do_resume() at the point of injection,
>>> and stumbled across this:
>>>
>>> (XEN) [  252.878389] vector: 14, type: 3, error_code: 0,
>>> VM_ENTRY_INTR_INFO: 0x80e1
>>>
>>> VM_ENTRY_INTR_INFO does have INTR_INFO_VALID_MASK set here.
>> 
>> So a first question I have is this: What are the criteria that made your
>> application decide it needs to inject a trap? Obviously there must have
>> been some kind of event in the guest that triggered this. Hence the
>> question is whether this same event wouldn't re-trigger at the end of the
>> hardware interrupt (or could be made re-trigger reasonably easily).
>> Because in the end what you're trying to do here is something that's
>> architecturally impossible: There can't be a (non-nested) exception once
>> an external interrupt has been accepted (i.e. any subsequent exception
>> can only be related to the delivery of that interrupt vector, not to the code
>> which was running when the interrupt was signaled).
>
>Unfortunately there are two main reasons why relying on the conditions
>for injecting the page fault repeating is problematic:
>
>1. We'd need to be able differentiate between a failed run (where
>injection doesn't work) and a succesful run, with no real possibility to
>know the difference for sure. So we don't know how to adapt the
>application's internal state based on some events: is the event the
>"final" one, or just a duplicate? xc_hvm_inject_trap() does not tell us
>(as indeed it cannot know) if the injection succeeded, and there's no
>other way to know.
>
>2. More importantly (although working around 1. is far from trivial),
>the event may not be repeatable. As an example, #PF injection can occur
>as part of handling an EPT event, but during handling the event the
>introspection engine can decide to lift the restrictions on said page
>after injecting the #PF. So the application relied on the #PF being
>delivered, and with the restrictions lifted from the page there will be
>no further EPT events for that page, therefore the main condition for
>triggering the #PF is lost forever.

Isn't this a problem you also have under other circumstances, e.g.
multiple faults occurring for a single instruction? Which gets us to the
fact that you didn't answer at all the initial question I did raise. Apart
from that I'm also not really understanding the model you describe:
What good does injecting #PF alongside lifting restrictions? I'd normally
expect just one of the two to occur to deal with whatever caused the
original event.

Jan

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

Re: [Xen-devel] [RFC PATCH 01/24] ARM: GICv3 ITS: parse and store ITS subnodes from hardware DT

[Julien Grall]

Hi Andre,

On 28/09/2016 19:24, Andre Przywara wrote:

Parse the DT GIC subnodes to find every ITS MSI controller the hardware
offers. Store that information in a list to both propagate all of them
later to Dom0, but also to be able to iterate over all ITSes.
This introduces an ITS Kconfig option.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/Kconfig  |  5 
 xen/arch/arm/Makefile |  1 +
 xen/arch/arm/gic-its.c| 67 +++
 xen/arch/arm/gic-v3.c |  6 
 xen/include/asm-arm/gic-its.h | 57 
 5 files changed, 136 insertions(+)
 create mode 100644 xen/arch/arm/gic-its.c
 create mode 100644 xen/include/asm-arm/gic-its.h

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 797c91f..9fe3b8e 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -45,6 +45,11 @@ config ACPI
 config HAS_GICV3
bool

+config HAS_ITS
+bool "GICv3 ITS MSI controller support"
+depends on ARM_64


HAS_GICV3 will only be selected for 64-bit. It would need some rework to 
be supported on 32-bit. So I would drop this dependency.



+depends on HAS_GICV3
+


I am not convinced that we should (currently) let the user selecting the 
ITS support. It increases the test coverage (we have to test with and 
without). Do we expect people using GICv3 without ITS?


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Tue, Nov 01, 2016 at 02:58:22PM +, Andrew Cooper wrote:
> On 01/11/16 13:47, Wei Liu wrote:
> > On Mon, Oct 31, 2016 at 05:09:46PM +, Wei Liu wrote:
> >> Signed-off-by: Wei Liu 
> >> ---
> >> Cc: Andrew Cooper 
> >> Cc: George Dunlap 
> >> Cc: Ian Jackson 
> >> Cc: Jan Beulich 
> >> Cc: Konrad Rzeszutek Wilk 
> >> Cc: Stefano Stabellini 
> >> Cc: Tim Deegan 
> >> Cc: Wei Liu 
> >> ---
> >>  Config.mk | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Config.mk b/Config.mk
> >> index ebbd9c0..fb836a4 100644
> >> --- a/Config.mk
> >> +++ b/Config.mk
> >> @@ -16,7 +16,8 @@ or   = $(if $(strip $(1)),$(1),$(if $(strip 
> >> $(2)),$(2),$(if $(strip $(3)),$(
> >>  
> >>  -include $(XEN_ROOT)/.config
> >>  
> >> -# A debug build of Xen and tools?
> >> +# A debug build of tools?
> > For this to hold true, a patch like this is needed:
> >
> > Please let me know what you think.
> 
> Looks like another swamp :s
> 

Indeed. This is something we overlooked early in the release.

> >
> > ---8<---
> > From 0a96ff9f3610622bc4f7114d6e094bf45ca9305f Mon Sep 17 00:00:00 2001
> > From: Wei Liu 
> > Date: Mon, 31 Oct 2016 17:42:25 +
> > Subject: [PATCH] build: make debug option affect tools only
> >
> > The debug option in Config.mk affects hypervisor, tools and stubdom by
> > appending different flags to CFLAGS.
> >
> > It is undesirable because now hypervisor build is affect by both Kconfig
> > and debug.
> >

^
Specifically because of this, I really want to fix this whole thing
properly. Otherwise it is going to be rather confusing to downstream.

> > Disentangle the semantics of debug by pushing relevant options to
> > individual sub-systems.
> >
> > For hypervisor, the flags previously added by debug option is now
> > controlled by CONFIG_DEBUG.
> >
> > For tools, flags are moved from config/*.mk into tools/Rules.mk.
> >
[...]
> > diff --git a/tools/Rules.mk b/tools/Rules.mk
> > index 5a80fec..0e73690 100644
> > --- a/tools/Rules.mk
> > +++ b/tools/Rules.mk
> > @@ -138,9 +138,11 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) 
> > -Wl,-rpath-link=$(XEN_LIBVCHAN)
> >  
> >  ifeq ($(debug),y)
> >  # Disable optimizations and enable debugging information for macros
> > -CFLAGS += -O0 -g3
> > +CFLAGS += -O0 -g3 -fno-omit-frame-pointer
> 
> Perhaps this a suggestion better left for a later patch, but the use of
> -O0 is still bad here.
> 
> Debug builds should use -Og if available, and -O1 otherwise.  As
> identified immediately below, a number of options are incompatible with -O0.
> 
> >  # But allow an override to -O0 in case Python enforces 
> > -D_FORTIFY_SOURCE=.
> >  PY_CFLAGS += $(PY_NOOPT_CFLAGS)
> > +else
> > +CFLAGS += -O2 -fomit-frame-pointer
> >  endif
> >  
> >  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> > diff --git a/xen/Rules.mk b/xen/Rules.mk
> > index a9fda71..08cc776 100644
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -46,6 +46,12 @@ ALL_OBJS-y   += $(BASEDIR)/xsm/built_in.o
> >  ALL_OBJS-y   += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
> >  ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
> >  
> > +ifeq ($(CONFIG_DEBUG),y)
> > +CFLAGS += -O1
> > +else
> > +CFLAGS += -O2 -fomit-frame-pointer
> 
> The frame pointer option should be omitted entirely.  A user should be
> able to control debug and frame pointer entirely independently with Kconfig.
> 
> There have been two times where I have specifically needed a release
> build with frame pointers to track down bugs.
> 

I think both are fine suggestions.

I would like to (hopefully) not introduce noticeable changes of the
effect of all combined flags in this patch and adjust the flags in a
later patch.

Wei.

> ~Andrew

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

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Andrew Cooper]

On 01/11/16 13:47, Wei Liu wrote:
> On Mon, Oct 31, 2016 at 05:09:46PM +, Wei Liu wrote:
>> Signed-off-by: Wei Liu 
>> ---
>> Cc: Andrew Cooper 
>> Cc: George Dunlap 
>> Cc: Ian Jackson 
>> Cc: Jan Beulich 
>> Cc: Konrad Rzeszutek Wilk 
>> Cc: Stefano Stabellini 
>> Cc: Tim Deegan 
>> Cc: Wei Liu 
>> ---
>>  Config.mk | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Config.mk b/Config.mk
>> index ebbd9c0..fb836a4 100644
>> --- a/Config.mk
>> +++ b/Config.mk
>> @@ -16,7 +16,8 @@ or   = $(if $(strip $(1)),$(1),$(if $(strip 
>> $(2)),$(2),$(if $(strip $(3)),$(
>>  
>>  -include $(XEN_ROOT)/.config
>>  
>> -# A debug build of Xen and tools?
>> +# A debug build of tools?
> For this to hold true, a patch like this is needed:
>
> Please let me know what you think.

Looks like another swamp :s

>
> ---8<---
> From 0a96ff9f3610622bc4f7114d6e094bf45ca9305f Mon Sep 17 00:00:00 2001
> From: Wei Liu 
> Date: Mon, 31 Oct 2016 17:42:25 +
> Subject: [PATCH] build: make debug option affect tools only
>
> The debug option in Config.mk affects hypervisor, tools and stubdom by
> appending different flags to CFLAGS.
>
> It is undesirable because now hypervisor build is affect by both Kconfig
> and debug.
>
> Disentangle the semantics of debug by pushing relevant options to
> individual sub-systems.
>
> For hypervisor, the flags previously added by debug option is now
> controlled by CONFIG_DEBUG.
>
> For tools, flags are moved from config/*.mk into tools/Rules.mk.
>
> For stubdom, it is a bit special because it unilaterally sets debug all
> the time, and it also inherits CFLAGS from the source package it tries
> to build. It should be fine to not inherit any flags from Xen build
> system because they will be overridden by source packages anyway.
>
> Specifically there are some considerations on how the flags are picked:
>
> 1. we don't need -fno-optimize-sibling-calls anymore because gcc doc
>indicates that it is not enabled for -O1, which we already set in the
>debug build.
> 2. for tools we use -O0 -g3 in Rules.mk because they already take
>precedence over the flags set in config/*.mk.
> 3. for hypervisor we don't add -fno-omit-frame-pointer to debug build
>because that's controlled by CONFIG_FRAME_POINTER.
>
> The debug option in Config.mk will only affect tools components after
> this patch is applied.
>
> Signed-off-by: Wei Liu 
> ---
>  config/StdGNU.mk | 8 
>  config/SunOS.mk  | 7 ---
>  tools/Rules.mk   | 4 +++-
>  xen/Rules.mk | 6 ++
>  4 files changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 39d36b2..6be8233 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -35,14 +35,6 @@ UTIL_LIBS = -lutil
>  SONAME_LDFLAG = -soname
>  SHLIB_LDFLAGS = -shared
>  
> -ifneq ($(debug),y)
> -CFLAGS += -O2 -fomit-frame-pointer
> -else
> -# Less than -O1 produces bad code and large stack frames
> -CFLAGS += -O1 -fno-omit-frame-pointer
> -CFLAGS-$(gcc) += -fno-optimize-sibling-calls
> -endif
> -
>  ifeq ($(lto),y)
>  CFLAGS += -flto
>  LDFLAGS-$(clang) += -plugin LLVMgold.so
> diff --git a/config/SunOS.mk b/config/SunOS.mk
> index 86a384d..0fe5f45 100644
> --- a/config/SunOS.mk
> +++ b/config/SunOS.mk
> @@ -31,12 +31,5 @@ UTIL_LIBS =
>  SONAME_LDFLAG = -h
>  SHLIB_LDFLAGS = -R $(SunOS_LIBDIR) -shared
>  
> -ifneq ($(debug),y)
> -CFLAGS += -O2 -fno-omit-frame-pointer
> -else
> -# Less than -O1 produces bad code and large stack frames
> -CFLAGS += -O1 -fno-omit-frame-pointer
> -endif
> -
>  CFLAGS += -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__
>  
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 5a80fec..0e73690 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -138,9 +138,11 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) 
> -Wl,-rpath-link=$(XEN_LIBVCHAN)
>  
>  ifeq ($(debug),y)
>  # Disable optimizations and enable debugging information for macros
> -CFLAGS += -O0 -g3
> +CFLAGS += -O0 -g3 -fno-omit-frame-pointer

Perhaps this a suggestion better left for a later patch, but the use of
-O0 is still bad here.

Debug builds should use -Og if available, and -O1 otherwise.  As
identified immediately below, a number of options are incompatible with -O0.

>  # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=.
>  PY_CFLAGS += $(PY_NOOPT_CFLAGS)
> +else
> +CFLAGS += -O2 -fomit-frame-pointer
>  endif
>  
>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index a9fda71..08cc776 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -46,6 +46,12 @@ ALL_OBJS-y   += $(BASEDIR)/xsm/built_in.o
>  ALL_OBJS-y   += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
>  

Re: [Xen-devel] PCIe devices that are hotplugged after MMIO has been setup fail due to _CRS not covering 64-bit area

[Konrad Rzeszutek Wilk]

. snip..
> I modified it be subtractive, and got it to start with
> large areas and then smaller and smaller:
> 
> (d2)  - CPU0 ... 36-bit phys ... fixed MTRRs ... Cover @04344(MB) to 
> 65536(M
> (d2) B) with 7 MTRRs.
> (d2) MTRR 1 @04344(MB)  37112(MB)
> (d2) MTRR 2 @37112(MB)  53496(MB)
> (d2) MTRR 3 @53496(MB)  61688(MB)
> (d2) MTRR 4 @61688(MB)  63736(MB)
> (d2) MTRR 5 @63736(MB)  64760(MB)
> (d2) MTRR 6 @64760(MB)  65272(MB)
> (d2) MTRR 7 @65272(MB)  65528(MB)
> (d2) var MTRRs [8/8] ... done.
> 
> But of course on 48-bit hardware, even with this we ran out of MTRRs:
> (d1)  - CPU0 ... 48-bit phys ... fixed MTRRs ... Cover @04344(MB) to 
> 0268435456(
> (d1) MB) with 7 MTRRs.
> (d1) MTRR 1 @04344(MB)  0134222072(MB)
> (d1) MTRR 2 @0134222072(MB) 0201330936(MB)
> (d1) MTRR 3 @0201330936(MB) 0234885368(MB)
> (d1) MTRR 4 @0234885368(MB) 0251662584(MB)
> (d1) MTRR 5 @0251662584(MB) 0260051192(MB)
> (d1) MTRR 6 @0260051192(MB) 0264245496(MB)
> (d1) MTRR 7 @0264245496(MB) 0266342648(MB)
> (d1) var MTRRs [8/8] ... done.

For comparison here is what the existing code does (pls ignore the 'MTRR 1'):

(d35) MB) with 7 MTRRs.
(d35) MTRR 1@04344(MB)  04352(MB)[8(MB)]
(d35) MTRR 1@04352(MB)  04608(MB)[00256(MB)]
(d35) MTRR 1@04608(MB)  05120(MB)[00512(MB)]
(d35) MTRR 1@05120(MB)  06144(MB)[01024(MB)]
(d35) MTRR 1@06144(MB)  08192(MB)[02048(MB)]
(d35) MTRR 1@08192(MB)  16384(MB)[08192(MB)]
(d35) MTRR 1@16384(MB)  32768(MB)[16384(MB)]

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

Re: [Xen-devel] [PATCH RFC 3/6] xen/arm: Allow platforms to hook IRQ routing.

[Julien Grall]

Hello Kyle,

On 05/09/2016 11:13, Kyle Temkin wrote:

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 52c9a01..402c766 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c


[...]


+
+bool_t platform_irq_is_routable(struct dt_raw_irq * rirq)


We are trying to move the code base to bool rather than bool_t.


+{
+/*
+ * If we have a platform-specific method to determine if an IRQ is 
routable,
+ * check that; otherwise fall back to checking to see if an IRQ belongs to
+ * the GIC.
+ */
+if ( platform && platform->irq_is_routable )
+return platform->irq_is_routable(rirq);
+else
+return (rirq->controller == dt_interrupt_controller);
+}
+
+int platform_irq_for_device(const struct dt_device_node *dev, int index)
+{
+if ( platform && platform->irq_for_device )
+return platform->irq_for_device(dev, index);
+else
+return platform_get_irq(dev, index);


IHMO the naming of this new function will confuse the user. Without any 
documentation it is not clear whether the user should call 
platform_irq_for_device or platform_get_irq.


However, I am not sure to understand why you need to override 
platform_get_irq as the code is exactly the same and dt_irq_xlate could 
be overidden to return the IRQ.



+}
+
+
 /*
  * Local variables:
  * mode: C


[...]


diff --git a/xen/drivers/passthrough/arm/smmu.c 
b/xen/drivers/passthrough/arm/smmu.c
index cf8b8b8..94d035a 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -103,7 +103,7 @@ static struct resource *platform_get_resource(struct 
platform_device *pdev,
return ((ret) ? NULL : );

case IORESOURCE_IRQ:
-   ret = platform_get_irq(pdev, num);
+   ret = platform_irq_for_device(pdev, num);
if (ret < 0)
return NULL;

@@ -2349,7 +2349,7 @@ static int arm_smmu_device_dt_probe(struct 
platform_device *pdev)
}

for (i = 0; i < num_irqs; ++i) {
-   int irq = platform_get_irq(pdev, i);
+   int irq = platform_irq_for_device(pdev, i);

if (irq < 0) {
dev_err(dev, "failed to get irq index %d\n", i);
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index f97315d..4ea278b 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -26,6 +26,13 @@ struct platform_desc {
 void (*reset)(void);
 /* Platform power-off */
 void (*poweroff)(void);
+/* Platform-specific IRQ routing */
+int (*route_irq_to_guest)(struct domain *d, unsigned int virq,
+   struct irq_desc *desc, unsigned int priority);
+void (*route_irq_to_xen)(struct irq_desc *desc, unsigned int priority);
+bool_t (*irq_is_routable)(struct dt_raw_irq * rirq);


Please use bool here.


+int (*irq_for_device)(const struct dt_device_node *dev, int index);
+
 /*
  * Platform blacklist devices
  * List of devices which must not pass-through to a guest
@@ -42,6 +49,13 @@ int platform_cpu_up(int cpu);
 #endif
 void platform_reset(void);
 void platform_poweroff(void);
+
+int platform_route_irq_to_guest(struct domain *d, unsigned int virq,
+ struct irq_desc *desc, unsigned int priority);
+void platform_route_irq_to_xen(struct irq_desc *desc, unsigned int priority);
+bool_t platform_irq_is_routable(struct dt_raw_irq *rirq);


Ditto.


+int platform_irq_for_device(const struct dt_device_node *dev, int index);
+
 bool_t platform_device_is_blacklisted(const struct dt_device_node *node);

 #define PLATFORM_START(_name, _namestr) \



Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/1] xen-netfront: do not cast grant table reference to signed short

[David Miller]

From: Dongli Zhang 
Date: Mon, 31 Oct 2016 21:46:09 -0700 (PDT)

> David, I am very sorry for this error and I will be careful the next time.
> Would you please let me know if I should resend a new patch or an incremental
> based on previous one at 
> https://git.kernel.org/cgit/linux/kernel/git/davem/net.git?

Incremental, please.

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

Re: [Xen-devel] [PATCH V4] xen/arm: domain_build: allocate lowmem for dom0 as much as possible

[Julien Grall]

Hi Peng,

Sorry for the late answer.

On 23/09/2016 03:55, Peng Fan wrote:

On AArch64 SoCs, some IPs may only have the capability to access
32 bits address space. The physical memory assigned for Dom0 maybe
not in 4GB address space, then the IPs will not work properly.
So need to allocate memory under 4GB for Dom0.

There is no restriction that how much lowmem needs to be allocated for
Dom0 ,so allocate lowmem as much as possible for Dom0.

This patch does not affect 32-bit domain, because Variable "lowmem" is
set to true at the beginning. If failed to allocate bank0 under 4GB,
need to panic for 32-bit domain, because 32-bit domain requires bank0
be allocated under 4GB.

For 64-bit domain, set "lowmem" to false, and continue allocating
memory from above 4GB.

Signed-off-by: Peng Fan 
Cc: Stefano Stabellini 
Cc: Julien Grall 


Reviewed-by: Julien Grall 

I am undecided whether this should be considered as a bug fix for Xen 
4.8. Are you aware of any ARM64 platform we currently support requiring 
allocation of memory below 4GB?


Regards,

--
Julien Grall

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

Re: [Xen-devel] PCIe devices that are hotplugged after MMIO has been setup fail due to _CRS not covering 64-bit area

[Konrad Rzeszutek Wilk]

On Thu, Oct 13, 2016 at 03:20:24AM -0600, Jan Beulich wrote:
> >>> On 12.10.16 at 23:15,  wrote:
> > On Wed, Sep 28, 2016 at 03:21:08AM -0600, Jan Beulich wrote:
> >> >>> On 27.09.16 at 16:43,  wrote:
> >> > If the guest is booted with 'pci' we nicely expand the MMIO region below
> >> > 4GB and try to fit in the BARs in there. If that fails (not enough
> >> > space) we move it above the memory (64-bit). And throughout all of this
> >> > we also update the _CRS field to cover these ranges.
> >> > 
> >> > (Note, I need to check if the 64-bit area is also set, I think it is).
> >> > 
> >> > But the situation is different if we hot-plug a device that has too big
> >> > BAR to fit in the MMIO region. We move it in the 64-bit area but we
> >> > don't update the _CRS. Which means that Linux will complain (unless
> >> > booted with pci=nocrs)). Not sure about Windows but I would assume so
> >> > to.
> >> > 
> >> > I was wondering what would be a good way to solve this? I looked at some
> >> > Dell machines to see how they deal with hotplug PCIe devices and they
> >> > just declared all the memory in the _CRS (including RAM).
> >> > 
> >> > We could do a hybrid - during bootup make the _CRS region have entry from
> >> > end of RAM to .. end of memory?
> >> 
> >> End of physical address space you mean? Generally yes, but we
> >> need to be a little careful there: For one, on AMD we'd better not
> >> overlap with the HT area. And then there's this MTRR related
> >> comment next to the setting of pci_hi_mem_end (albeit both HT
> >> area start and end of PA space should be aligned well enough).

This got interesting. The existing code that sets the variable
MTRR ran out of MTRRs to cover say 1<<36 of space. The reason
is that it starts at low granularity sizes (4KB) and then builds up
from there. To cover say from 4GB to 64GB we ran out of MTRRs.
I modified it be subtractive, and got it to start with
large areas and then smaller and smaller:

(d2)  - CPU0 ... 36-bit phys ... fixed MTRRs ... Cover @04344(MB) to 
65536(M
(d2) B) with 7 MTRRs.
(d2) MTRR 1 @04344(MB)  37112(MB)
(d2) MTRR 2 @37112(MB)  53496(MB)
(d2) MTRR 3 @53496(MB)  61688(MB)
(d2) MTRR 4 @61688(MB)  63736(MB)
(d2) MTRR 5 @63736(MB)  64760(MB)
(d2) MTRR 6 @64760(MB)  65272(MB)
(d2) MTRR 7 @65272(MB)  65528(MB)
(d2) var MTRRs [8/8] ... done.

But of course on 48-bit hardware, even with this we ran out of MTRRs:
(d1)  - CPU0 ... 48-bit phys ... fixed MTRRs ... Cover @04344(MB) to 
0268435456(
(d1) MB) with 7 MTRRs.
(d1) MTRR 1 @04344(MB)  0134222072(MB)
(d1) MTRR 2 @0134222072(MB) 0201330936(MB)
(d1) MTRR 3 @0201330936(MB) 0234885368(MB)
(d1) MTRR 4 @0234885368(MB) 0251662584(MB)
(d1) MTRR 5 @0251662584(MB) 0260051192(MB)
(d1) MTRR 6 @0260051192(MB) 0264245496(MB)
(d1) MTRR 7 @0264245496(MB) 0266342648(MB)
(d1) var MTRRs [8/8] ... done.

[I figured that it would be OK to set the UC MTRR even for the
HT region: FC   -> FF   as you surely don't want WB there?]

Then it ocurred to me that maybe I am overthinking it and
should just pick the biggest one:

(d32) Multiprocessor initialisation:
(d32)  - CPU0 ... 48-bit phys ... fixed MTRRs ... Cover @04344(MB) to 
0268435456(
(d32) MB) with 7 MTRRs.
(d32) MTRR 1@04344(MB)  0268439800(MB)
(d32) var MTRRs [1/8] ... done.

Which would cover _past_ the CPU end, but that surely won't be healthy
to the CPU? The Intel SDM doesn't mention what happens then.

Also I realized that "Range Size and Alignment Requirement" aren't meet
with the code I wrote - as the size (2^n) must be aligned on the
2^n boundary, and that is certainly not meet.

Anyhow the point here is that with modifications here I will
still run in the variable MTRR limit if I am to cover most of the
space. I can do up to a certain value. And that 'value' could
become the pci_high_mem_end?

Or perhaps revisit a6a822324:
Author: Keir Fraser 
Date:   Wed Apr 16 13:36:44 2008 +0100

x86, hvm: Lots of MTRR/PAT emulation cleanup.

 - Move MTRR MSR initialisation into hvmloader.
 - Simplify initialisation logic by overlaying UC on default WB rather
   than vice versa.
 - Clean up hypervisor HVM MTRR/PAE code's interface with rest of
   hypervisor.


As the default MTRR is WB. If that was UC we could just set MTRRs
for RAM regions and have the type be WB for those regions?

I am not sure thought if that is a good direction either?

> >> 
> >> > Or perhaps add some extra logic between QEMU and ACPI AML to expand (or
> >> > perhaps modify the last _CRS entry) when PCIe devices are hotplugged?
> >> 
> >> While that would be the most flexible variant, I'd be afraid of this
> >> getting rather complicated. Or have you already got some
> >> reasonable layout of how this would look like?
> > 
> > I did this and while 

Re: [Xen-devel] [PATCH] Don't clear HCR_VM bit when updating VTTBR.

[Julien Grall]

Hi Stefano,

On 31/10/2016 22:18, Stefano Stabellini wrote:

On Mon, 31 Oct 2016, Steve Capper wrote:

On Wed, Oct 19, 2016 at 12:59:45PM -0700, Stefano Stabellini wrote:

On Mon, 10 Oct 2016, Jun Sun wrote:

Currently function p2m_restore_state() would clear HCR_VM bit, i.e.,
disabling stage2 translation, before updating VTTBR register. After
some research and talking to ARM support, I got confirmed that this is not
necessary. We are currently working on a new platform that would need this
to be removed.

The patch is tested on FVP foundation model.

Signed-off-by: Jun Sun 


Hello Jun,

thanks for the patch and sorry for the late reply. I would appreciate
feedback from Julien, but he is current AFK.

Steve,
can I have your Acked-by on this patch?



Apologies for my very late reply on this.

I've taken a look and I think that this is okay, so please feel free to
add:
Acked-by: Steve Capper 


Thanks Steve. Given the state of the release, I am going to push the fix
to Xen 4.9 unless you or Jun have a strong argument for committing this
sooner.


Not related to this patch. Do you plan to have a branch for queuing all 
Xen 4.9 patches?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [xen-unstable test] 101833: regressions - trouble: blocked/broken/fail/pass

[Wei Liu]

On Tue, Nov 01, 2016 at 09:48:38AM +, Andrew Cooper wrote:
> On 01/11/2016 07:22, osstest service owner wrote:
> > flight 101833 xen-unstable real [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/101833/
> >
> > Regressions :-(
> >
> > Tests which did not succeed and are blocking,
> > including tests which could not be run:
> >  test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install 
> > fail REGR. vs. 101673
> 
> This run was without my fix for 32bit oxenstored transactions, so this
> failure isn't unexpected.
> 
> >  test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 17 
> > guest-start/debianhvm.repeat fail REGR. vs. 101673
> 
> This attempted to ssh to dom0 and got Destination Host Unreachable.  I
> notice from the serial log
> 
> (XEN) Assertion '!in_irq() && local_irq_is_enabled()' failed at softirq.c:57
> ...
> (XEN) Xen call trace:
> (XEN)[] process_pending_softirqs+0x29/0x37
> (XEN)[] irq.c#dump_irqs+0x43/0x309
> (XEN)[] handle_keypress+0x8c/0xac
> (XEN)[] console.c#__serial_rx+0x38/0x73
> (XEN)[] console.c#serial_rx+0x8a/0x8f
> (XEN)[] serial_rx_interrupt+0x90/0xac
> (XEN)[] ns16550.c#ns16550_interrupt+0x57/0x71
> (XEN)[] do_IRQ+0x56e/0x60f
> (XEN)[] common_interrupt+0x67/0x70
> (XEN)[] domain.c#default_idle+0x6d/0x72
> (XEN)[] domain.c#idle_loop+0x4b/0x62
> 

Note that osstest doesn't rotate the serial log -- this could be from an
earlier run.

Looking at the time stamp, the last error message is

  Nov  1 00:16:55.129992 (XEN) Assertion '!in_irq() && local_irq_is_enabled()' 
failed 

which is way before the test job started -- the allocate host step is
to allocate a host from the pool.

Wei.

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

Re: [Xen-devel] [RFC] Shared coprocessor framework

[Artem Mygaiev]

Let me explain a bit background of this work.

We see growing amount of use cases for different "co-processors" like
 - GPUs (inside of most modern SoCs)
 - low-power side CPU cores (like ARM Cortex M or R on board with
Cortex A cores to handle PM or other tasks)
 - DSPs (for example, TI C6000 family DSP core inside of Jacinto 6 SoC)
 - FPGAs (Altera or Xilinx SoCs = ARM+FPGA)

These cores most often used standalone for some specific function, but
quite often there is a need to "virtualize" such co-processor together
with the main CPU cores. For example, they may be used in some
virtualized heterogeneous computing environment so there shall be some
sort of an independent "context" of a co-processor associated with a
VM which interacts with it. In some cases, addressing security and
stability requirements, "context" means not only "data" but also
"code" (firmware); i.e. when VMs switch on main CPU, both code and
data memory shall switch on co-processor.

Couple examples when VMs run on same SoC, both want to use some
co-processor in data-intense tasks with different data sets and with
different firmware images and ensure isolation (no data is leaked or
code corrupted through co-processor's memory access) and stability
(restart of one system does not lead to crash of another):
1. use GPU for GL rendering in one VM, another for NN state computing
2. use DSP for HW-accelerated media decoding, another for video image
processing (object recognition, etc.)

We already see a need for enabling such cases in Embedded/Automotive
space (mostly dominated by ARM now), but also this might fit generic
computing in heterogeneous environments - different co-processors are
now deployed alongside generic CPUs in server environments (Google use
own tensor processors for NN computing acceleration, Microsoft used
Altera's FPGAs in project Catapult, there are deployments of GPU
computing nodes in some clouds, etc.)

Hope that makes sense

Best regards,
Artem Mygaiev


On Mon, Oct 31, 2016 at 9:31 PM, Andrii Anisov  wrote:
>> Thankyou for the design doc.  An immediate +1 from me, simply for the
>> doc existing :)
>
> Thank you for you interest and comments.
>
>> Forgive my ignorance (I am an x86 person, and given the CC list, I guess
>> this is talking about ARM systems), but what are coprocessors and what
>> might I do with one?
>
> Well coprocessor could be a some processing unit inside a SoC which is
> running some firmware and supplementing primary processor functions.
> F.e. GPU, DSP, some FPGA inside a SoC.
> The living example is a GPU sharing implemented for the ARM based SoC.
> BTW, the xengt implements pretty close approach and is a pure x86
> world solution.
>
>> > It is targeted capability of different domains to run concurrently 
>> > different
>> > firmware on the coproc.
>> I cant parse this sentence.  I presume you mean that the purpose of this
>> framework is to provide a mechanism for Xen to share a coprocessors
>> resource between multiple domains?
>
> Maybe it should be reworded. I mean that coprocessors are entities
> which are running some firmware to perform their tasks. So different
> domains in their time slice could run different versions of firmware
> on the same coprocessor.
> It is mentioned here to stress that domains contexts are totally
> independent (both for processed data and for firmware code).
>
>> Grammar nit.  Either "Provide coprocessor resource sharing between..."
>> or "Provide sharing of coprocessor resources between..."
>
> Will take "Provide sharing of coprocessor resources between...".
>
>
>> Does it need to only be configurable at system startup?  There is often
>> more flexibility by having a default configuration at system start (so
>> dom0 can use the resources), which can later be altered by toolstack policy.
>
> I did mean that hypervisor, what starts first, checks what
> coprocessors within the system would be shared, own them and provide
> to a framework. Providing some of those resources to dom0 would not a
> big deal: just assign a vcoproc to the domain. And yes, it could be a
> default configuration.
>
>>
>> Considering the latter option, even if you don't implement support at
>> first, tends to lead to a cleaner design, but of course it does depend
>> heavily on the details of the situation.
>
> Definitely we would tailor the design along with an implementation.
>
>> > * MMU-enabled SoC with MMU-protected coprocessor(s)
>> Right - definitely ARM then, but it took me until half way through the
>> document to work this out.
>
> You know, it was specified ARM based SoC here. At some point it was
> removed such a dependency. Inspired by the already mentioned xengt.
>
>> I would be tempted to extend this slightly, and specify that there
>> should be a mechanism for the toolstack to query all of this information
>> at arbitrary points in time.
>
> It should be covered here:
>>
>> > Runtime configuration of scheduling algorithm per instance of shared
>> > 

Re: [Xen-devel] [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Mon, Oct 31, 2016 at 05:09:46PM +, Wei Liu wrote:
> Signed-off-by: Wei Liu 
> ---
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> ---
>  Config.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Config.mk b/Config.mk
> index ebbd9c0..fb836a4 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -16,7 +16,8 @@ or   = $(if $(strip $(1)),$(1),$(if $(strip 
> $(2)),$(2),$(if $(strip $(3)),$(
>  
>  -include $(XEN_ROOT)/.config
>  
> -# A debug build of Xen and tools?
> +# A debug build of tools?

For this to hold true, a patch like this is needed:

Please let me know what you think.

---8<---
From 0a96ff9f3610622bc4f7114d6e094bf45ca9305f Mon Sep 17 00:00:00 2001
From: Wei Liu 
Date: Mon, 31 Oct 2016 17:42:25 +
Subject: [PATCH] build: make debug option affect tools only

The debug option in Config.mk affects hypervisor, tools and stubdom by
appending different flags to CFLAGS.

It is undesirable because now hypervisor build is affect by both Kconfig
and debug.

Disentangle the semantics of debug by pushing relevant options to
individual sub-systems.

For hypervisor, the flags previously added by debug option is now
controlled by CONFIG_DEBUG.

For tools, flags are moved from config/*.mk into tools/Rules.mk.

For stubdom, it is a bit special because it unilaterally sets debug all
the time, and it also inherits CFLAGS from the source package it tries
to build. It should be fine to not inherit any flags from Xen build
system because they will be overridden by source packages anyway.

Specifically there are some considerations on how the flags are picked:

1. we don't need -fno-optimize-sibling-calls anymore because gcc doc
   indicates that it is not enabled for -O1, which we already set in the
   debug build.
2. for tools we use -O0 -g3 in Rules.mk because they already take
   precedence over the flags set in config/*.mk.
3. for hypervisor we don't add -fno-omit-frame-pointer to debug build
   because that's controlled by CONFIG_FRAME_POINTER.

The debug option in Config.mk will only affect tools components after
this patch is applied.

Signed-off-by: Wei Liu 
---
 config/StdGNU.mk | 8 
 config/SunOS.mk  | 7 ---
 tools/Rules.mk   | 4 +++-
 xen/Rules.mk | 6 ++
 4 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 39d36b2..6be8233 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -35,14 +35,6 @@ UTIL_LIBS = -lutil
 SONAME_LDFLAG = -soname
 SHLIB_LDFLAGS = -shared
 
-ifneq ($(debug),y)
-CFLAGS += -O2 -fomit-frame-pointer
-else
-# Less than -O1 produces bad code and large stack frames
-CFLAGS += -O1 -fno-omit-frame-pointer
-CFLAGS-$(gcc) += -fno-optimize-sibling-calls
-endif
-
 ifeq ($(lto),y)
 CFLAGS += -flto
 LDFLAGS-$(clang) += -plugin LLVMgold.so
diff --git a/config/SunOS.mk b/config/SunOS.mk
index 86a384d..0fe5f45 100644
--- a/config/SunOS.mk
+++ b/config/SunOS.mk
@@ -31,12 +31,5 @@ UTIL_LIBS =
 SONAME_LDFLAG = -h
 SHLIB_LDFLAGS = -R $(SunOS_LIBDIR) -shared
 
-ifneq ($(debug),y)
-CFLAGS += -O2 -fno-omit-frame-pointer
-else
-# Less than -O1 produces bad code and large stack frames
-CFLAGS += -O1 -fno-omit-frame-pointer
-endif
-
 CFLAGS += -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__
 
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 5a80fec..0e73690 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -138,9 +138,11 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) 
-Wl,-rpath-link=$(XEN_LIBVCHAN)
 
 ifeq ($(debug),y)
 # Disable optimizations and enable debugging information for macros
-CFLAGS += -O0 -g3
+CFLAGS += -O0 -g3 -fno-omit-frame-pointer
 # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=.
 PY_CFLAGS += $(PY_NOOPT_CFLAGS)
+else
+CFLAGS += -O2 -fomit-frame-pointer
 endif
 
 LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
diff --git a/xen/Rules.mk b/xen/Rules.mk
index a9fda71..08cc776 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -46,6 +46,12 @@ ALL_OBJS-y   += $(BASEDIR)/xsm/built_in.o
 ALL_OBJS-y   += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
+ifeq ($(CONFIG_DEBUG),y)
+CFLAGS += -O1
+else
+CFLAGS += -O2 -fomit-frame-pointer
+endif
+
 CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
 CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
-- 
2.1.4


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

Re: [Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Andrew Cooper]

On 01/11/16 12:07, Xuquan (Quan Xu) wrote:
> On November 01, 2016 8:00 PM, Andrew Cooper wrote:
>> On 01/11/16 11:57, Xuquan (Quan Xu) wrote:
>>> On November 01, 2016 7:41 PM, Andrew Cooper
>>  wrote:
 On 01/11/16 11:23, Xuquan (Quan Xu) wrote:
> On November 01, 2016 7:16 PM, Andrew Cooper <
 andrew.coop...@citrix.com > wrote:
>> On 01/11/16 11:01, Xuquan (Quan Xu) wrote:
>>> Hi Andrew,
>>>
>>> When I run some application with Xen, I encounter a Panic with log
>>> as the
>> bottom of this email.
>>> I find this panic is as similar as your fix e4e9d2d4e76bd8fe22
>> 'x86/p2m-ept:
>> don't unmap the EPT pagetable while it is still in use'.
>>
>> Its not the same.  My fix was for a pagefault.  Your crash is
>> hitting a
>> BUG()
>>
>> You need to investigate which BUG() is being hit (you are clearly
>> not using staging, as page_alloc.c:429 is outside of a function), and 
>> why.
>>
> It is not using staging, a internal version, but the
> __do_update_va_mapping() is
 the same..
> So I think it is similar to staging.
 __do_update_va_mapping() is not relevant.  Your problem is in
 alloc_heap_pages(), not __do_update_va_mapping().

>>> Andrew, thanks for your help..
>>>
>>> Check it,
>>> the BUG() is from
>>> do_memory_op() -- alloc_domheap_pages() -- alloc_heap_pages()..
>>> , not from  __do_update_va_mapping().
>>>
>>> If so, I think the information is too limited to debug..
>>>
>>> the BUG() is being hit at:
>>> ...
>>> alloc_heap_pages()
>>> for ( i = 0; i < (1 << order); i++ )
>>> {
>>> /* Reference count must continuously be zero for free pages. */
>>> BUG_ON(pg[i].count_info != PGC_state_free); ...
>>>
>>>
>>> I guess I hit a 'count_info == -1', more put_page() is called to return the 
>>> page..
>> Yes.  This means that you have a reference counting error elsewhere in
>> the hypervisor, and something is dropping too many references.
>>
> :(:(..
> Any method to address it?

Not specifically.  It is similar to chasing a double free bug.

You should review whatever local additions you have in your tree extra
carefully.  It might be that there is a setup path which doesn't take a
ref, or a buggy error path which cleans up too many refs.

~Andrew

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

[Xen-devel] [ovmf baseline-only test] 67974: all pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 67974 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/67974/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e
baseline version:
 ovmf ac55b925548f3b33f2bc93e603ecffe4a6cb191a

Last test of basis67971  2016-11-01 02:20:11 Z0 days
Testing same since67974  2016-11-01 07:50:55 Z0 days1 attempts


People who touched revisions under test:
  Feng Tian 
  Laszlo Ersek 
  Leo Duran  
  Leo Duran 
  Michael Kinney 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

Test harness code can be found at
http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Push not applicable.


commit 90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e
Author: Feng Tian 
Date:   Mon Oct 31 13:20:05 2016 +0800

MdeModulePkg/Xhci: Change short packet debug message to verbose level

Short Packet case is a normal case, we shouldn't print it as an error

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
Reviewed-by: Star Zeng 

commit a7b3f90f8a081d90bc4287990e53f367228d53b2
Author: Feng Tian 
Date:   Mon Oct 31 13:46:08 2016 +0800

MdeModulePkg/AtaAtapiPassThru: update AtaStatusBlock after cmd exec

AhciDumpPortStatus doesn't fully populate all the fields of
AtaStatusBlock after completing command execution, which may bring
issue if someone depends on the return status.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
Reviewed-by: Star Zeng 

commit 73152f19c0be7f31ee05f32878b515a296c487fa
Author: Leo Duran 
Date:   Tue Nov 1 03:42:57 2016 +0800

UefiCpuPkg: Move GetProcessorLocation() to LocalApicLib library

1) Remove SmmGetProcessorLocation() from PiSmmCpuDxeSmm driver.
2) Remove ExtractProcessorLocation() from MpInitLib library.
3) Add GetProcessorLocation() to BaseXApicLib and BaseXApicX2ApicLib.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran  
Signed-off-by: Michael Kinney 
Tested-by: Laszlo Ersek 
Reviewed-by: Michael Kinney 
Reviewed-by: Jeff Fan 

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

Re: [Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Xuquan (Quan Xu)]

On November 01, 2016 8:00 PM, Andrew Cooper wrote:
>On 01/11/16 11:57, Xuquan (Quan Xu) wrote:
>> On November 01, 2016 7:41 PM, Andrew Cooper
> wrote:
>>> On 01/11/16 11:23, Xuquan (Quan Xu) wrote:
 On November 01, 2016 7:16 PM, Andrew Cooper <
>>> andrew.coop...@citrix.com > wrote:
> On 01/11/16 11:01, Xuquan (Quan Xu) wrote:
>> Hi Andrew,
>>
>> When I run some application with Xen, I encounter a Panic with log
>> as the
> bottom of this email.
>> I find this panic is as similar as your fix e4e9d2d4e76bd8fe22
>'x86/p2m-ept:
> don't unmap the EPT pagetable while it is still in use'.
>
> Its not the same.  My fix was for a pagefault.  Your crash is
> hitting a
> BUG()
>
> You need to investigate which BUG() is being hit (you are clearly
> not using staging, as page_alloc.c:429 is outside of a function), and why.
>
 It is not using staging, a internal version, but the
 __do_update_va_mapping() is
>>> the same..
 So I think it is similar to staging.
>>> __do_update_va_mapping() is not relevant.  Your problem is in
>>> alloc_heap_pages(), not __do_update_va_mapping().
>>>
>>
>> Andrew, thanks for your help..
>>
>> Check it,
>> the BUG() is from
>> do_memory_op() -- alloc_domheap_pages() -- alloc_heap_pages()..
>> , not from  __do_update_va_mapping().
>>
>> If so, I think the information is too limited to debug..
>>
>> the BUG() is being hit at:
>> ...
>> alloc_heap_pages()
>> for ( i = 0; i < (1 << order); i++ )
>> {
>> /* Reference count must continuously be zero for free pages. */
>> BUG_ON(pg[i].count_info != PGC_state_free); ...
>>
>>
>> I guess I hit a 'count_info == -1', more put_page() is called to return the 
>> page..
>
>Yes.  This means that you have a reference counting error elsewhere in
>the hypervisor, and something is dropping too many references.
>

:(:(..
Any method to address it? 

Quan

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

Re: [Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Andrew Cooper]

On 01/11/16 11:57, Xuquan (Quan Xu) wrote:
> On November 01, 2016 7:41 PM, Andrew Cooper  
> wrote:
>> On 01/11/16 11:23, Xuquan (Quan Xu) wrote:
>>> On November 01, 2016 7:16 PM, Andrew Cooper <
>> andrew.coop...@citrix.com > wrote:
 On 01/11/16 11:01, Xuquan (Quan Xu) wrote:
> Hi Andrew,
>
> When I run some application with Xen, I encounter a Panic with log
> as the
 bottom of this email.
> I find this panic is as similar as your fix e4e9d2d4e76bd8fe22 
> 'x86/p2m-ept:
 don't unmap the EPT pagetable while it is still in use'.

 Its not the same.  My fix was for a pagefault.  Your crash is hitting
 a
 BUG()

 You need to investigate which BUG() is being hit (you are clearly not
 using staging, as page_alloc.c:429 is outside of a function), and why.

>>> It is not using staging, a internal version, but the 
>>> __do_update_va_mapping() is
>> the same..
>>> So I think it is similar to staging.
>> __do_update_va_mapping() is not relevant.  Your problem is in
>> alloc_heap_pages(), not __do_update_va_mapping().
>>
>
> Andrew, thanks for your help.. 
>
> Check it, 
> the BUG() is from
> do_memory_op() -- alloc_domheap_pages() -- alloc_heap_pages()..
> , not from  __do_update_va_mapping().
>
> If so, I think the information is too limited to debug..
>
> the BUG() is being hit at:
> ...
> alloc_heap_pages()
> for ( i = 0; i < (1 << order); i++ )
> {
> /* Reference count must continuously be zero for free pages. */
> BUG_ON(pg[i].count_info != PGC_state_free);
> ...
>
>
> I guess I hit a 'count_info == -1', more put_page() is called to return the 
> page..

Yes.  This means that you have a reference counting error elsewhere in
the hypervisor, and something is dropping too many references.

~Andrew

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

Re: [Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Xuquan (Quan Xu)]

On November 01, 2016 7:41 PM, Andrew Cooper  wrote:
>On 01/11/16 11:23, Xuquan (Quan Xu) wrote:
>> On November 01, 2016 7:16 PM, Andrew Cooper <
>andrew.coop...@citrix.com > wrote:
>>> On 01/11/16 11:01, Xuquan (Quan Xu) wrote:
 Hi Andrew,

 When I run some application with Xen, I encounter a Panic with log
 as the
>>> bottom of this email.
 I find this panic is as similar as your fix e4e9d2d4e76bd8fe22 
 'x86/p2m-ept:
>>> don't unmap the EPT pagetable while it is still in use'.
>>>
>>> Its not the same.  My fix was for a pagefault.  Your crash is hitting
>>> a
>>> BUG()
>>>
>>> You need to investigate which BUG() is being hit (you are clearly not
>>> using staging, as page_alloc.c:429 is outside of a function), and why.
>>>
>> It is not using staging, a internal version, but the 
>> __do_update_va_mapping() is
>the same..
>> So I think it is similar to staging.
>
>__do_update_va_mapping() is not relevant.  Your problem is in
>alloc_heap_pages(), not __do_update_va_mapping().
>


Andrew, thanks for your help.. 

Check it, 
the BUG() is from
do_memory_op() -- alloc_domheap_pages() -- alloc_heap_pages()..
, not from  __do_update_va_mapping().

If so, I think the information is too limited to debug..

the BUG() is being hit at:
...
alloc_heap_pages()
for ( i = 0; i < (1 << order); i++ )
{
/* Reference count must continuously be zero for free pages. */
BUG_ON(pg[i].count_info != PGC_state_free);
...


I guess I hit a 'count_info == -1', more put_page() is called to return the 
page..

Quan

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

Re: [Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Andrew Cooper]

On 01/11/16 11:23, Xuquan (Quan Xu) wrote:
> On November 01, 2016 7:16 PM, Andrew Cooper < andrew.coop...@citrix.com > 
> wrote:
>> On 01/11/16 11:01, Xuquan (Quan Xu) wrote:
>>> Hi Andrew,
>>>
>>> When I run some application with Xen, I encounter a Panic with log as the
>> bottom of this email.
>>> I find this panic is as similar as your fix e4e9d2d4e76bd8fe22 'x86/p2m-ept:
>> don't unmap the EPT pagetable while it is still in use'.
>>
>> Its not the same.  My fix was for a pagefault.  Your crash is hitting a
>> BUG()
>>
>> You need to investigate which BUG() is being hit (you are clearly not using 
>> staging,
>> as page_alloc.c:429 is outside of a function), and why.
>>
> It is not using staging, a internal version, but the __do_update_va_mapping() 
> is the same..
> So I think it is similar to staging.

__do_update_va_mapping() is not relevant.  Your problem is in
alloc_heap_pages(), not __do_update_va_mapping().

~Andrew

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

Re: [Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Xuquan (Quan Xu)]

On November 01, 2016 7:16 PM, Andrew Cooper < andrew.coop...@citrix.com > wrote:
>On 01/11/16 11:01, Xuquan (Quan Xu) wrote:
>> Hi Andrew,
>>
>> When I run some application with Xen, I encounter a Panic with log as the
>bottom of this email.
>> I find this panic is as similar as your fix e4e9d2d4e76bd8fe22 'x86/p2m-ept:
>don't unmap the EPT pagetable while it is still in use'.
>
>Its not the same.  My fix was for a pagefault.  Your crash is hitting a
>BUG()
>
>You need to investigate which BUG() is being hit (you are clearly not using 
>staging,
>as page_alloc.c:429 is outside of a function), and why.
>

It is not using staging, a internal version, but the __do_update_va_mapping() 
is the same..
So I think it is similar to staging.

Quan


>~Andrew
>
>>
>> _iiuc_, in __do_update_va_mapping(),
>>
>> 'va' is unmapped before to call flush_tlb_one_mask(.., va).
>>
>> Specifically,
>> va is related to pl1e, pl1e = guest_map_l1e(va, )...
>>
>> but va is unmapped here:
>> out:
>> if ( pl1e )
>> guest_unmap_l1e(pl1e);
>>
>> ...
>>
>> Before to call flush_tlb_one_mask(..., va);
>>
>> Is it right?
>> Also, I wonder whether I need to move 'put_page(gl1pg)' to the bottom of
>__do_update_va_mapping().
>>
>>
>> (XEN) Xen call trace:
>> (XEN)[] panic+0xc3/0x1a0
>> (XEN)[] invalidate_interrupt+0x2a/0x30
>> (XEN)[] symbols_lookup+0x22/0x2a0
>> (XEN)[] syscall_enter+0x88/0x8d
>> (XEN)[] syscall_enter+0x88/0x8d
>> (XEN)[] __print_symbol+0x8a/0xc0
>> (XEN)[] syscall_enter+0x88/0x8d
>> (XEN)[] show_stack+0x110/0x180
>> (XEN)[] alloc_heap_pages+0x5f6/0x600
>> (XEN)[] alloc_heap_pages+0x5f6/0x600
>> (XEN)[] do_invalid_op+0x394/0x420
>> (XEN)[] ept_set_entry+0x4d3/0x820
>> (XEN)[] handle_exception_saved+0x30/0x6e
>>
>> (XEN)[] alloc_heap_pages+0x5f4/0x600
>> (XEN)[] alloc_heap_pages+0x70/0x600
>> (XEN)[] alloc_domheap_pages+0x74/0x120
>> (XEN)[] do_memory_op+0x11db/0x2a40
>> (XEN)[] get_page_type+0xb/0x20
>> (XEN)[] get_page_from_l1e+0x1a2/0x680
>> (XEN)[] is_iomem_page+0x9/0x80
>> (XEN)[] mod_l1_entry+0x1f5/0x720
>> (XEN)[] flush_area_mask+0x7c/0x140
>> (XEN)[] __do_update_va_mapping+0x453/0x7a0
>> (XEN)[] do_iret+0xc1/0x1a0
>> (XEN)[] syscall_enter+0x88/0x8d
>> (XEN)
>> (XEN)
>> (XEN) 
>> (XEN) Panic on CPU 30:
>> (XEN) Xen BUG at page_alloc.c:429
>> (XEN) 
>> (XEN)
>> (XEN) Reboot in five seconds...
>>
>>
>>
>> Quan
>>
>>

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

Re: [Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Andrew Cooper]

On 01/11/16 11:01, Xuquan (Quan Xu) wrote:
> Hi Andrew, 
>
> When I run some application with Xen, I encounter a Panic with log as the 
> bottom of this email.
> I find this panic is as similar as your fix e4e9d2d4e76bd8fe22 'x86/p2m-ept: 
> don't unmap the EPT pagetable while it is still in use'.

Its not the same.  My fix was for a pagefault.  Your crash is hitting a
BUG()

You need to investigate which BUG() is being hit (you are clearly not
using staging, as page_alloc.c:429 is outside of a function), and why.

~Andrew

>
> _iiuc_, in __do_update_va_mapping(),
>
> 'va' is unmapped before to call flush_tlb_one_mask(.., va).
>
> Specifically, 
> va is related to pl1e, pl1e = guest_map_l1e(va, )...
>
> but va is unmapped here:
> out:
> if ( pl1e )
> guest_unmap_l1e(pl1e);
>
> ...
>
> Before to call flush_tlb_one_mask(..., va);
>
> Is it right?
> Also, I wonder whether I need to move 'put_page(gl1pg)' to the bottom of 
> __do_update_va_mapping().
>
>
> (XEN) Xen call trace:
> (XEN)[] panic+0xc3/0x1a0
> (XEN)[] invalidate_interrupt+0x2a/0x30
> (XEN)[] symbols_lookup+0x22/0x2a0
> (XEN)[] syscall_enter+0x88/0x8d
> (XEN)[] syscall_enter+0x88/0x8d
> (XEN)[] __print_symbol+0x8a/0xc0
> (XEN)[] syscall_enter+0x88/0x8d
> (XEN)[] show_stack+0x110/0x180
> (XEN)[] alloc_heap_pages+0x5f6/0x600
> (XEN)[] alloc_heap_pages+0x5f6/0x600
> (XEN)[] do_invalid_op+0x394/0x420
> (XEN)[] ept_set_entry+0x4d3/0x820
> (XEN)[] handle_exception_saved+0x30/0x6e
>
> (XEN)[] alloc_heap_pages+0x5f4/0x600
> (XEN)[] alloc_heap_pages+0x70/0x600
> (XEN)[] alloc_domheap_pages+0x74/0x120
> (XEN)[] do_memory_op+0x11db/0x2a40
> (XEN)[] get_page_type+0xb/0x20
> (XEN)[] get_page_from_l1e+0x1a2/0x680
> (XEN)[] is_iomem_page+0x9/0x80
> (XEN)[] mod_l1_entry+0x1f5/0x720
> (XEN)[] flush_area_mask+0x7c/0x140
> (XEN)[] __do_update_va_mapping+0x453/0x7a0
> (XEN)[] do_iret+0xc1/0x1a0
> (XEN)[] syscall_enter+0x88/0x8d
> (XEN)
> (XEN)
> (XEN) 
> (XEN) Panic on CPU 30:
> (XEN) Xen BUG at page_alloc.c:429
> (XEN) 
> (XEN)
> (XEN) Reboot in five seconds...
>
>
>
> Quan
>
>


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

[Xen-devel] qeustion: a panic in __do_update_va_mapping()

[Xuquan (Quan Xu)]

Hi Andrew, 

When I run some application with Xen, I encounter a Panic with log as the 
bottom of this email.
I find this panic is as similar as your fix e4e9d2d4e76bd8fe22 'x86/p2m-ept: 
don't unmap the EPT pagetable while it is still in use'.

_iiuc_, in __do_update_va_mapping(),

'va' is unmapped before to call flush_tlb_one_mask(.., va).

Specifically, 
va is related to pl1e, pl1e = guest_map_l1e(va, )...

but va is unmapped here:
out:
if ( pl1e )
guest_unmap_l1e(pl1e);

...

Before to call flush_tlb_one_mask(..., va);

Is it right?
Also, I wonder whether I need to move 'put_page(gl1pg)' to the bottom of 
__do_update_va_mapping().


(XEN) Xen call trace:
(XEN)[] panic+0xc3/0x1a0
(XEN)[] invalidate_interrupt+0x2a/0x30
(XEN)[] symbols_lookup+0x22/0x2a0
(XEN)[] syscall_enter+0x88/0x8d
(XEN)[] syscall_enter+0x88/0x8d
(XEN)[] __print_symbol+0x8a/0xc0
(XEN)[] syscall_enter+0x88/0x8d
(XEN)[] show_stack+0x110/0x180
(XEN)[] alloc_heap_pages+0x5f6/0x600
(XEN)[] alloc_heap_pages+0x5f6/0x600
(XEN)[] do_invalid_op+0x394/0x420
(XEN)[] ept_set_entry+0x4d3/0x820
(XEN)[] handle_exception_saved+0x30/0x6e

(XEN)[] alloc_heap_pages+0x5f4/0x600
(XEN)[] alloc_heap_pages+0x70/0x600
(XEN)[] alloc_domheap_pages+0x74/0x120
(XEN)[] do_memory_op+0x11db/0x2a40
(XEN)[] get_page_type+0xb/0x20
(XEN)[] get_page_from_l1e+0x1a2/0x680
(XEN)[] is_iomem_page+0x9/0x80
(XEN)[] mod_l1_entry+0x1f5/0x720
(XEN)[] flush_area_mask+0x7c/0x140
(XEN)[] __do_update_va_mapping+0x453/0x7a0
(XEN)[] do_iret+0xc1/0x1a0
(XEN)[] syscall_enter+0x88/0x8d
(XEN)
(XEN)
(XEN) 
(XEN) Panic on CPU 30:
(XEN) Xen BUG at page_alloc.c:429
(XEN) 
(XEN)
(XEN) Reboot in five seconds...



Quan


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

Re: [Xen-devel] xc_hvm_inject_trap() races

[Razvan Cojocaru]

On 11/01/2016 12:30 PM, Jan Beulich wrote:
 Razvan Cojocaru  11/01/16 10:04 AM >>>
>> We've stumbled across the following scenario: we're injecting a #PF to
>> try to bring a swapped page back, but Xen already have a pending
>> interrupt, and the two collide.
>>
>> I've logged what happens in hvm_do_resume() at the point of injection,
>> and stumbled across this:
>>
>> (XEN) [  252.878389] vector: 14, type: 3, error_code: 0,
>> VM_ENTRY_INTR_INFO: 0x80e1
>>
>> VM_ENTRY_INTR_INFO does have INTR_INFO_VALID_MASK set here.
> 
> So a first question I have is this: What are the criteria that made your
> application decide it needs to inject a trap? Obviously there must have
> been some kind of event in the guest that triggered this. Hence the
> question is whether this same event wouldn't re-trigger at the end of the
> hardware interrupt (or could be made re-trigger reasonably easily).
> Because in the end what you're trying to do here is something that's
> architecturally impossible: There can't be a (non-nested) exception once
> an external interrupt has been accepted (i.e. any subsequent exception
> can only be related to the delivery of that interrupt vector, not to the code
> which was running when the interrupt was signaled).

Unfortunately there are two main reasons why relying on the conditions
for injecting the page fault repeating is problematic:

1. We'd need to be able differentiate between a failed run (where
injection doesn't work) and a succesful run, with no real possibility to
know the difference for sure. So we don't know how to adapt the
application's internal state based on some events: is the event the
"final" one, or just a duplicate? xc_hvm_inject_trap() does not tell us
(as indeed it cannot know) if the injection succeeded, and there's no
other way to know.

2. More importantly (although working around 1. is far from trivial),
the event may not be repeatable. As an example, #PF injection can occur
as part of handling an EPT event, but during handling the event the
introspection engine can decide to lift the restrictions on said page
after injecting the #PF. So the application relied on the #PF being
delivered, and with the restrictions lifted from the page there will be
no further EPT events for that page, therefore the main condition for
triggering the #PF is lost forever.


Thanks,
Razvan

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

[Xen-devel] [PATCH 2/3] xen/err: Support an optional per-arch slide for the error region

[Andrew Cooper]

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/include/xen/err.h | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/xen/include/xen/err.h b/xen/include/xen/err.h
index ef77992..bea7fa3 100644
--- a/xen/include/xen/err.h
+++ b/xen/include/xen/err.h
@@ -15,19 +15,28 @@
  */
 #define MAX_ERRNO  4095
 
+/*
+ * Individual architectures may wish to slide the error region away from its
+ * default position at the very top of virtual address space.  (e.g. if Xen
+ * doesn't own the range).
+ */
+#ifndef ARCH_ERR_PTR_SLIDE
+#define ARCH_ERR_PTR_SLIDE 0
+#endif
+
 static inline bool IS_ERR_VALUE(unsigned long x)
 {
-   return x >= (unsigned long)-MAX_ERRNO;
+   return (x + ARCH_ERR_PTR_SLIDE) >= (unsigned long)-MAX_ERRNO;
 }
 
 static inline void *__must_check ERR_PTR(long error)
 {
-   return (void *)error;
+   return (void *)((unsigned long)error - ARCH_ERR_PTR_SLIDE);
 }
 
 static inline long __must_check PTR_ERR(const void *ptr)
 {
-   return (long)ptr;
+   return (long)((unsigned long)ptr + ARCH_ERR_PTR_SLIDE);
 }
 
 static inline bool __must_check IS_ERR(const void *ptr)
-- 
2.1.4


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

[Xen-devel] [PATCH for-4.9 0/3] Improvements to ERR_PTR() infrastructure

[Andrew Cooper]

One XenServer issue I am looking in to requires improvements to the memory
allocation logic, mostly to avoid losing error information internally.

As a result, I am choosing to make these ERR_PTR() improvements a prerequisite
to avoid accidentally introducing further issues.

Andrew Cooper (3):
  xen/err: Use static inlines and boolean types
  xen/err: Support an optional per-arch slide for the error region
  xen/x86: Use non-canonical pointers for ERR_PTR() errors

 xen/arch/x86/Kconfig | 18 ++
 xen/include/asm-x86/config.h |  8 
 xen/include/xen/err.h| 23 ++-
 3 files changed, 44 insertions(+), 5 deletions(-)

-- 
2.1.4


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

[Xen-devel] [PATCH 3/3] xen/x86: Use non-canonical pointers for ERR_PTR() errors

[Andrew Cooper]

The top of the virutal address space is owned by 64bit PV kernels.  Code which
fails to correctly check an ERR_PTR() value might follow the pointer into
guest space.

Mitigate this risk by sliding the ERR_PTR() error range into the non-canonical
region.

As this comes with a small overhead, and isn't necessary if 64bit PV guests
aren't used, provide a Kconfig opt-out for power users.

As an example, ERR_PTR(-EINVAL) has the value 0xbad0eee0ffea.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

Ideally, the constant would read "bad err", but r's are hard in hex.
Alternative constant suggestions welcome.
---
 xen/arch/x86/Kconfig | 18 ++
 xen/include/asm-x86/config.h |  8 
 2 files changed, 26 insertions(+)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 96ca2bf..9aa9d3f 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -89,6 +89,24 @@ config TBOOT
  Technology (TXT)
 
  If unsure, say Y.
+
+config UNSAFE_ERRPTR
+   def_bool n
+   prompt "Unsafe ERR_PTR() constants" if EXPERT = "y"
+   ---help---
+ The ERR_PTR() infrastructure by default uses the top of virtual
+ address space for error information, but with 64bit PV guests, this
+ range belongs to the guest kernel.
+
+ Xen by default slides the error range elsewhere in virtual address
+ space, but this comes at the expense of a few extra instructions of
+ calculations.
+
+ If that overhead is too much, this option is safe to enable if no
+ 64bit PV guests are in use, or all 64bit PV guests are fully trusted.
+
+ If unsure, say N.
+
 endmenu
 
 source "common/Kconfig"
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index 6fd84e7..2997ee3 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -87,6 +87,14 @@
 #define LIST_POISON1  ((void *)0x0100100100100100UL)
 #define LIST_POISON2  ((void *)0x0200200200200200UL)
 
+#if !defined(NDEBUG) || !defined(CONFIG_UNSAFE_ERRPTR)
+/*
+ * Always use safe pointers in debug builds.  Use safe pointers in release
+ * builds unless the user explicitly opts out.
+ */
+#define ARCH_ERR_PTR_SLIDE (-(unsigned long)0xbad0eee1ull)
+#endif
+
 #ifndef __ASSEMBLY__
 extern unsigned long trampoline_phys;
 #define bootsym_phys(sym) \
-- 
2.1.4


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

[Xen-devel] [PATCH 1/3] xen/err: Use static inlines and boolean types

[Andrew Cooper]

IS_ERR() and IS_ERR_OR_NULL() both return boolean values.

No functional change.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
---
 xen/include/xen/err.h | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/include/xen/err.h b/xen/include/xen/err.h
index 2f29b57..ef77992 100644
--- a/xen/include/xen/err.h
+++ b/xen/include/xen/err.h
@@ -2,6 +2,7 @@
 #define __XEN_ERR_H__
 
 #include 
+#include 
 #include 
 
 /*
@@ -14,7 +15,10 @@
  */
 #define MAX_ERRNO  4095
 
-#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
+static inline bool IS_ERR_VALUE(unsigned long x)
+{
+   return x >= (unsigned long)-MAX_ERRNO;
+}
 
 static inline void *__must_check ERR_PTR(long error)
 {
@@ -26,12 +30,12 @@ static inline long __must_check PTR_ERR(const void *ptr)
return (long)ptr;
 }
 
-static inline long __must_check IS_ERR(const void *ptr)
+static inline bool __must_check IS_ERR(const void *ptr)
 {
return IS_ERR_VALUE((unsigned long)ptr);
 }
 
-static inline long __must_check IS_ERR_OR_NULL(const void *ptr)
+static inline bool __must_check IS_ERR_OR_NULL(const void *ptr)
 {
return !ptr || IS_ERR_VALUE((unsigned long)ptr);
 }
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v3.1 03/15] xen/x86: allow calling {sh/hap}_set_allocation with the idle domain

[Tim Deegan]

At 10:34 -0600 on 31 Oct (1477910088), Jan Beulich wrote:
> >>> On 29.10.16 at 10:59,  wrote:
> > ... and using the "preempted" parameter. The solution relies on just calling
> > softirq_pending if the current domain is the idle domain. If such preemption
> > happens, the caller should then call process_pending_softirqs in order to
> > drain the pending softirqs, and then call {sh/hap}_set_allocation again to
> > continue with it's execution.
> > 
> > This allows us to call *_set_allocation() when building domain 0.
> > 
> > Signed-off-by: Roger Pau Monné 
> > Acked-by: George Dunlap 
> > ---
> > Cc: George Dunlap 
> 
> Cc: Tim
> 
> > Cc: Jan Beulich 
> > Cc: Andrew Cooper 
> > ---
> > Changes since v2:
> >  - Fix commit message.
> > ---
> >  xen/arch/x86/mm/hap/hap.c   | 4 +++-
> >  xen/arch/x86/mm/shadow/common.c | 4 +++-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> > index f099e94..0645521 100644
> > --- a/xen/arch/x86/mm/hap/hap.c
> > +++ b/xen/arch/x86/mm/hap/hap.c
> > @@ -379,7 +379,9 @@ hap_set_allocation(struct domain *d, unsigned int 
> > pages, 
> > int *preempted)
> >  break;
> >  
> >  /* Check to see if we need to yield and try again */
> > -if ( preempted && hypercall_preempt_check() )
> > +if ( preempted &&
> > + (is_idle_vcpu(current) ? softirq_pending(smp_processor_id()) :
> > +  hypercall_preempt_check()) )

This is a bit clunky, and is open-coded twice.  Can the existing checks in
hypercall_preempt_check() be made safe to run on the idle vcpu?
If not, please make a wrapper to call here that DTRT on idle and
non-idle, e.g. something like:

 /*
  * For long-running operations that may be in hypercall context or on
  * the idle vcpu (e.g. during dom0 construction), check if there is
  * background work to be done that should interrupt this operation.
  */
 static inline bool general_preempt_check(void)
 {
 return unlikely(softirq_pending(smp_processor_id()) ||
 (!is_idle_vcpu(current) && local_events_need_delivery()));
 }

If you're feeling keen, you could convert hypercall_preempt_check() to
an inline function and comment it too. :)

Apart from that, ack.

Cheers,

Tim.

> >  {
> >  *preempted = 1;
> >  return 0;
> > diff --git a/xen/arch/x86/mm/shadow/common.c 
> > b/xen/arch/x86/mm/shadow/common.c
> > index 065bdc7..b2e99c2 100644
> > --- a/xen/arch/x86/mm/shadow/common.c
> > +++ b/xen/arch/x86/mm/shadow/common.c
> > @@ -1679,7 +1679,9 @@ static int sh_set_allocation(struct domain *d,
> >  break;
> >  
> >  /* Check to see if we need to yield and try again */
> > -if ( preempted && hypercall_preempt_check() )
> > +if ( preempted &&
> > + (is_idle_vcpu(current) ? softirq_pending(smp_processor_id()) :
> > +  hypercall_preempt_check()) )
> >  {
> >  *preempted = 1;
> >  return 0;
> > -- 
> > 2.7.4 (Apple Git-66)
> 
> 

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

Re: [Xen-devel] xc_hvm_inject_trap() races

[Jan Beulich]

>>> Razvan Cojocaru  11/01/16 10:04 AM >>>
>We've stumbled across the following scenario: we're injecting a #PF to
>try to bring a swapped page back, but Xen already have a pending
>interrupt, and the two collide.
>
>I've logged what happens in hvm_do_resume() at the point of injection,
>and stumbled across this:
>
>(XEN) [  252.878389] vector: 14, type: 3, error_code: 0,
>VM_ENTRY_INTR_INFO: 0x80e1
>
>VM_ENTRY_INTR_INFO does have INTR_INFO_VALID_MASK set here.

So a first question I have is this: What are the criteria that made your
application decide it needs to inject a trap? Obviously there must have
been some kind of event in the guest that triggered this. Hence the
question is whether this same event wouldn't re-trigger at the end of the
hardware interrupt (or could be made re-trigger reasonably easily).
Because in the end what you're trying to do here is something that's
architecturally impossible: There can't be a (non-nested) exception once
an external interrupt has been accepted (i.e. any subsequent exception
can only be related to the delivery of that interrupt vector, not to the code
which was running when the interrupt was signaled).

Jan

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

Re: [Xen-devel] [PATCH v3.1 05/15] x86/paging: introduce paging_set_allocation

[Tim Deegan]

At 10:42 -0600 on 31 Oct (1477910532), Jan Beulich wrote:
> >>> On 29.10.16 at 10:59,  wrote:
> > --- a/xen/arch/x86/mm/shadow/common.c
> > +++ b/xen/arch/x86/mm/shadow/common.c
> > @@ -1609,13 +1609,7 @@ shadow_free_p2m_page(struct domain *d, struct 
> > page_info *pg)
> >  paging_unlock(d);
> >  }
> >  
> > -/* Set the pool of shadow pages to the required number of pages.
> > - * Input will be rounded up to at least shadow_min_acceptable_pages(),
> > - * plus space for the p2m table.
> > - * Returns 0 for success, non-zero for failure. */
> > -static int sh_set_allocation(struct domain *d,
> > - unsigned int pages,
> > - int *preempted)
> > +int sh_set_allocation(struct domain *d, unsigned int pages, bool 
> > *preempted)
> 
> Iirc functions with a name starting with sh_ are shadow code internal,
> so you may better switch to shadow_set_allocation(). Tim?

Yep.  That naming convention is not very faithfully followed,
especially since the introduction of hap* and paging*, but it would be
nice.

Cheers,

Tim.

> With that taken care of (unless Tim indicates there's no need),
> Reviewed-by: Jan Beulich 
> 
> Jan

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

Re: [Xen-devel] [xen-unstable test] 101833: regressions - trouble: blocked/broken/fail/pass

[Jan Beulich]

>>> Andrew Cooper  11/01/16 10:50 AM >>>
>On 01/11/2016 07:22, osstest service owner wrote:
>>  test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 17 
>> guest-start/debianhvm.repeat fail REGR. vs. 101673
>
>This attempted to ssh to dom0 and got Destination Host Unreachable.  I
>notice from the serial log
>
>(XEN) Assertion '!in_irq() && local_irq_is_enabled()' failed at softirq.c:57
>...
>(XEN) Xen call trace:
>(XEN)[] process_pending_softirqs+0x29/0x37
>(XEN)[] irq.c#dump_irqs+0x43/0x309
>(XEN)[] handle_keypress+0x8c/0xac
>(XEN)[] console.c#__serial_rx+0x38/0x73
>(XEN)[] console.c#serial_rx+0x8a/0x8f
>(XEN)[] serial_rx_interrupt+0x90/0xac
>(XEN)[] ns16550.c#ns16550_interrupt+0x57/0x71
>(XEN)[] do_IRQ+0x56e/0x60f
>(XEN)[] common_interrupt+0x67/0x70
>(XEN)[] domain.c#default_idle+0x6d/0x72
>(XEN)[] domain.c#idle_loop+0x4b/0x62

Considering that handle_keypress() invokes non-IRQ-handler functions directly
only when !in_irq(), I can't help getting the impression that in_irq() returns 
the
wrong value here. This, however, would then not be a new problem, as nothing
here really changed recently (the only recent change got reverted), or at least
I'm unaware of any change which might have affected this.

Jan

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

Re: [Xen-devel] [xen-unstable test] 101833: regressions - trouble: blocked/broken/fail/pass

[Andrew Cooper]

On 01/11/2016 07:22, osstest service owner wrote:
> flight 101833 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/101833/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install 
> fail REGR. vs. 101673

This run was without my fix for 32bit oxenstored transactions, so this
failure isn't unexpected.

>  test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 17 guest-start/debianhvm.repeat 
> fail REGR. vs. 101673

This attempted to ssh to dom0 and got Destination Host Unreachable.  I
notice from the serial log

(XEN) Assertion '!in_irq() && local_irq_is_enabled()' failed at softirq.c:57
...
(XEN) Xen call trace:
(XEN)[] process_pending_softirqs+0x29/0x37
(XEN)[] irq.c#dump_irqs+0x43/0x309
(XEN)[] handle_keypress+0x8c/0xac
(XEN)[] console.c#__serial_rx+0x38/0x73
(XEN)[] console.c#serial_rx+0x8a/0x8f
(XEN)[] serial_rx_interrupt+0x90/0xac
(XEN)[] ns16550.c#ns16550_interrupt+0x57/0x71
(XEN)[] do_IRQ+0x56e/0x60f
(XEN)[] common_interrupt+0x67/0x70
(XEN)[] domain.c#default_idle+0x6d/0x72
(XEN)[] domain.c#idle_loop+0x4b/0x62

>  test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail REGR. 
> vs. 101673

I really can't fathom why this one failed.  The guest was asked to
reboot and didn't, but I can't spot anything obviously wrong.

>  test-armhf-armhf-libvirt-raw 14 guest-start/debian.repeat fail REGR. vs. 
> 101673

This was also Destination Host Unreachable.

~Andrew

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

Re: [Xen-devel] [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-netback

[Paul Durrant]

> -Original Message-
> From: Juergen Gross [mailto:jgr...@suse.com]
> Sent: 31 October 2016 16:48
> To: linux-ker...@vger.kernel.org; xen-devel@lists.xen.org
> Cc: David Vrabel ; boris.ostrov...@oracle.com;
> Juergen Gross ; Wei Liu ; Paul
> Durrant ; net...@vger.kernel.org
> Subject: [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-
> netback
> 
> Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
> This requires to change the type of some reads from int to unsigned,
> but these cases have been wrong before: negative values are not allowed
> for the modified cases.
> 
> Cc: wei.l...@citrix.com
> Cc: paul.durr...@citrix.com

Reviewed-by: Paul Durrant 

> Cc: net...@vger.kernel.org
> 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/net/xen-netback/xenbus.c | 50 +++---
> --
>  1 file changed, 14 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> index 8674e18..7356e00 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -785,12 +785,9 @@ static void xen_mcast_ctrl_changed(struct
> xenbus_watch *watch,
>   struct xenvif *vif = container_of(watch, struct xenvif,
> mcast_ctrl_watch);
>   struct xenbus_device *dev = xenvif_to_xenbus_device(vif);
> - int val;
> 
> - if (xenbus_scanf(XBT_NIL, dev->otherend,
> -  "request-multicast-control", "%d", ) < 0)
> - val = 0;
> - vif->multicast_control = !!val;
> + vif->multicast_control = !!xenbus_read_unsigned(dev->otherend,
> + "request-multicast-control", 0);
>  }
> 
>  static int xen_register_mcast_ctrl_watch(struct xenbus_device *dev,
> @@ -934,12 +931,9 @@ static void connect(struct backend_info *be)
>   /* Check whether the frontend requested multiple queues
>* and read the number requested.
>*/
> - err = xenbus_scanf(XBT_NIL, dev->otherend,
> -"multi-queue-num-queues",
> -"%u", _num_queues);
> - if (err < 0) {
> - requested_num_queues = 1; /* Fall back to single queue */
> - } else if (requested_num_queues > xenvif_max_queues) {
> + requested_num_queues = xenbus_read_unsigned(dev->otherend,
> + "multi-queue-num-queues", 1);
> + if (requested_num_queues > xenvif_max_queues) {
>   /* buggy or malicious guest */
>   xenbus_dev_fatal(dev, err,
>"guest requested %u queues, exceeding the
> maximum of %u.",
> @@ -1134,7 +1128,7 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>   struct xenvif *vif = be->vif;
>   struct xenbus_device *dev = be->dev;
>   unsigned int rx_copy;
> - int err, val;
> + int err;
> 
>   err = xenbus_scanf(XBT_NIL, dev->otherend, "request-rx-copy",
> "%u",
>  _copy);
> @@ -1150,10 +1144,7 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>   if (!rx_copy)
>   return -EOPNOTSUPP;
> 
> - if (xenbus_scanf(XBT_NIL, dev->otherend,
> -  "feature-rx-notify", "%d", ) < 0)
> - val = 0;
> - if (!val) {
> + if (!xenbus_read_unsigned(dev->otherend, "feature-rx-notify", 0)) {
>   /* - Reduce drain timeout to poll more frequently for
>*   Rx requests.
>* - Disable Rx stall detection.
> @@ -1162,34 +1153,21 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>   be->vif->stall_timeout = 0;
>   }
> 
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
> -  "%d", ) < 0)
> - val = 0;
> - vif->can_sg = !!val;
> + vif->can_sg = !!xenbus_read_unsigned(dev->otherend, "feature-
> sg", 0);
> 
>   vif->gso_mask = 0;
> 
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
> -  "%d", ) < 0)
> - val = 0;
> - if (val)
> + if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv4", 0))
>   vif->gso_mask |= GSO_BIT(TCPV4);
> 
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv6",
> -  "%d", ) < 0)
> - val = 0;
> - if (val)
> + if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv6", 0))
>   vif->gso_mask |= GSO_BIT(TCPV6);
> 
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-
> offload",
> -  "%d", ) < 0)
> - val = 0;
> - vif->ip_csum = !val;
> + vif->ip_csum = !xenbus_read_unsigned(dev->otherend,
> +  "feature-no-csum-offload", 0);
> 
> - if 

[Xen-devel] [qemu-mainline test] 101835: tolerable FAIL - PUSHED

[osstest service owner]

flight 101835 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101835/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  6 xen-boot fail REGR. vs. 101826
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 101826
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 101826
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 101826
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 101826
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 101826
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 101826
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail  like 101826

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 qemuue80b4b8fb6babce7dcc91ea9ddeecbc351fd4646
baseline version:
 qemuu2dfe5113b11ce0ddb08176ebb54ab7ac4104b413

Last test of basis   101826  2016-10-31 10:18:57 Z0 days
Testing same since   101835  2016-10-31 22:21:49 Z0 days1 attempts


People who touched revisions under test:
  Akanksha Srivastava 
  Alberto Garcia 
  Alex Bennée 
  Alex Williamson 
  Amit Shah 
  Anand J 
  Anthony PERARD 
  Ashijeet Acharya 
  Changlong Xie 
  Cornelia Huck 
  Daniel Shahaf 
  Emil Condrea 
  Emilio G. Cota 
  Eric Blake 
  Fam Zheng 
  Gonglei 
  Guenter Roeck 
  Ido Yariv 
  Jean-Christophe Dubois 
  Kevin Wolf 
  KONRAD Frederic 
  Laurent Vivier 
  Li Zhijian 
  Marc-André Lureau 
  Michael Tokarev 
  Michael Walle 
  Paolo Bonzini 
  Peter Maydell 
  Pierre Morel 
  Prasad J Pandit 
  Quan Xu 
  Quan Xu 
  Stefan Weil 

[Xen-devel] xc_hvm_inject_trap() races

[Razvan Cojocaru]

Hello,

We've stumbled across the following scenario: we're injecting a #PF to
try to bring a swapped page back, but Xen already have a pending
interrupt, and the two collide.

I've logged what happens in hvm_do_resume() at the point of injection,
and stumbled across this:

(XEN) [  252.878389] vector: 14, type: 3, error_code: 0,
VM_ENTRY_INTR_INFO: 0x80e1

VM_ENTRY_INTR_INFO does have INTR_INFO_VALID_MASK set here.

This obviously leads to all sorts of unpleasantness with the guest.
Ideally, the injected #PF should take precedence, and the other
interrupt should not be lost as well. Suggestions on how best to solve
this are welcome and appreciated.

This problem would also occur with reinjected breakpoint events (please
see xen-access.c), which also use xc_hvm_inject_trap().

The easiest approach would be to simply ignore the injected trap while
VM_ENTRY_INTR_INFO & INTR_INFO_VALID_MASK, like this:

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 2d78e45..cfe04b4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -532,8 +532,16 @@ void hvm_do_resume(struct vcpu *v)
 /* Inject pending hw/sw trap */
 if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
 {
-hvm_inject_trap(>arch.hvm_vcpu.inject_trap);
-v->arch.hvm_vcpu.inject_trap.vector = -1;
+unsigned long ev;
+
+__vmread(VM_ENTRY_INTR_INFO, );
+
+/* Check for already pending interrupts (races). */
+if ( !(ev & INTR_INFO_VALID_MASK) )
+{
+hvm_inject_trap(>arch.hvm_vcpu.inject_trap);
+v->arch.hvm_vcpu.inject_trap.vector = -1;
+}
 }
 }

However, this does not even guarantee delayed delivery of the trap,
since by the time the next hvm_do_resume() call happens where there's no
other pending interrupt we could have called xc_hvm_inject_trap() with
different values, and have overwritten v->arch.hvm_vcpu.inject_trap.
More importantly, the context might be different then and the injection
would fail (or mess things up) because of that.

So we'd be better off simply discarding the trap in this case (as has
been suggested by Andrew), but here again the xc_hvm_inject_trap()
caller has no clue whether the call succeeded or failed. This is
inefficient (and thus comes with a performance impact) since the guest
would be put in a loop until injection finally succeeds, and finally
makes client applications significantly more cumbersome to implement,
because they would need to guess when the injection failed (since
xc_hvm_inject_trap() won't tell us), and handle duplicates for these
corner cases.

Is there an elegant way to fix this that keeps both interrupts but makes
the injected one higher priority?


Thanks,
Razvan

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

[Xen-devel] [libvirt test] 101839: tolerable all pass - PUSHED

[osstest service owner]

flight 101839 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101839/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 101773
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 101773
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 101773
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 101773

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass

version targeted for testing:
 libvirt  06a7b1ff4dbb1ed6a69e09765bef1f67a75a86eb
baseline version:
 libvirt  011935457a3b5d911e10dc60c681778e78c8fdf9

Last test of basis   101773  2016-10-29 04:20:39 Z3 days
Testing same since   101839  2016-11-01 04:20:30 Z0 days1 attempts


People who touched revisions under test:
  John Ferlan 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-armhf-armhf-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=libvirt
+ revision=06a7b1ff4dbb1ed6a69e09765bef1f67a75a86eb
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push libvirt 

[Xen-devel] [ovmf test] 101838: all pass - PUSHED

[osstest service owner]

flight 101838 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101838/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e
baseline version:
 ovmf ac55b925548f3b33f2bc93e603ecffe4a6cb191a

Last test of basis   101832  2016-10-31 19:30:22 Z0 days
Testing same since   101838  2016-11-01 02:17:57 Z0 days1 attempts


People who touched revisions under test:
  Feng Tian 
  Laszlo Ersek 
  Leo Duran  
  Leo Duran 
  Michael Kinney 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=ovmf
+ revision=90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push ovmf 
90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e
+ branch=ovmf
+ revision=90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=ovmf
+ xenbranch=xen-unstable
+ '[' xovmf = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.7-testing
+ '[' x90d6dfb9871bcf7e73b8884b1c1b1fc0029fcb2e = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/xtf.git
++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git
++ : git://xenbits.xen.org/xtf.git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : git
++ : git://xenbits.xen.org/osstest/rumprun.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git
++ : git://git.seabios.org/seabios.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git
++ : git://xenbits.xen.org/osstest/seabios.git
++ : https://github.com/tianocore/edk2.git
++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/ovmf.git
++ : git://xenbits.xen.org/osstest/linux-firmware.git
++ : 

[Xen-devel] [xen-unstable test] 101833: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

flight 101833 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/101833/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 101673
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 17 guest-start/debianhvm.repeat 
fail REGR. vs. 101673
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail REGR. vs. 
101673
 test-armhf-armhf-libvirt-raw 14 guest-start/debian.repeat fail REGR. vs. 101673

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds  3 host-install(3)broken REGR. vs. 101673
 test-armhf-armhf-libvirt 13 saverestore-support-checkfail  like 101673
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 101673
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 101673
 test-armhf-armhf-libvirt-qcow2 12 saverestore-support-check   fail like 101673
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 101673
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 101673
 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail  like 101673
 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail  like 101673
 test-amd64-amd64-xl-rtds  9 debian-install   fail  like 101673

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 build-i386-rumprun7 xen-buildfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 build-amd64-rumprun   7 xen-buildfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass

version targeted for testing:
 xen  26c4f0b8a4cf233f600f4163fc3aeeb7f70b3021
baseline version:
 xen  6f9b62ca57322197e26d3b22ff11b629697142bd

Last test of basis   101673  2016-10-26 02:01:16 Z6 days
Failing since101698  2016-10-26 19:50:48 Z5 days9 attempts
Testing same since   101833  2016-10-31 19:45:33 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Dario Faggioli 
  David Scott 
  Ian Jackson 
  Jan Beulich 
  Juergen Gross 
  Meng Xu 
  Roger Pau Monne 
  Roger Pau Monné 
  Samuel Thibault 
  Wei Liu 

jobs: