[Xen-devel] [linux-4.1 test] 95818: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-amd64 10 guest-startfail REGR. vs. 94729

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl   3 host-install(3)  broken in 95666 pass in 95818
 test-amd64-i386-xl3 host-install(3)  broken in 95666 pass in 95818
 test-amd64-i386-freebsd10-i386 3 host-install(3) broken in 95666 pass in 95818
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken in 
95666 pass in 95818
 test-amd64-i386-qemut-rhel6hvm-amd 3 host-install(3) broken in 95666 pass in 
95818
 test-amd64-i386-xl-raw3 host-install(3)  broken in 95666 pass in 95818
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken in 
95666 pass in 95818
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 3 host-install(3) broken 
in 95666 pass in 95818
 test-armhf-armhf-libvirt-xsm 4 host-ping-check-native fail in 95666 pass in 
95818
 test-armhf-armhf-xl-arndale 15 guest-start/debian.repeat fail in 95666 pass in 
95818
 test-armhf-armhf-xl-xsm  11 guest-startfail in 95666 pass in 95818
 test-armhf-armhf-xl-rtds  6 xen-boot   fail in 95666 pass in 95818
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1  fail pass in 95455
 test-armhf-armhf-xl  15 guest-start/debian.repeat   fail pass in 95666

Regressions which are regarded as allowable (not blocking):
 build-amd64-rumpuserxen   6 xen-buildfail   like 94729
 build-i386-rumpuserxen6 xen-buildfail   like 94729
 test-armhf-armhf-xl-credit2  15 guest-start/debian.repeatfail   like 94729
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeatfail  like 94729
 test-armhf-armhf-xl-cubietruck 15 guest-start/debian.repeatfail like 94729
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 94729
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 94729
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 94729
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 94729
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail   like 94729
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   like 94729

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  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-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 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-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-i386-libvirt-xsm  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 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-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  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-rtds 13 

[Xen-devel] [PATCH v3 1/2] xen/arm: add support for vm_assist hypercall

[Juergen Gross]

Up to now the vm_assist hypercall hasn't been supported on ARM, as
there are only x86 specific features to switch. Add support of
vm_assist on ARM for future use.

Signed-off-by: Juergen Gross 
Reviewed-by: Julien Grall 
---
V2: readded the #ifdef's as requested by Jan Beulich
---
 xen/arch/arm/traps.c | 1 +
 xen/include/asm-arm/config.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index aa3e3c2..44926ca 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1282,6 +1282,7 @@ static arm_hypercall_t arm_hypercall_table[] = {
 HYPERCALL(multicall, 2),
 HYPERCALL(platform_op, 1),
 HYPERCALL_ARM(vcpu_op, 3),
+HYPERCALL(vm_assist, 2),
 };
 
 #ifndef NDEBUG
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 2d11b62..563f49b 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -199,6 +199,8 @@ extern unsigned long frametable_virt_end;
 #define watchdog_disable() ((void)0)
 #define watchdog_enable()  ((void)0)
 
+#define VM_ASSIST_VALID  (0)
+
 #endif /* __ARM_CONFIG_H__ */
 /*
  * Local variables:
-- 
2.6.6


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

[Xen-devel] [PATCH v3 0/2] Support consistent reads of mapped vcpu_runstate_info

[Juergen Gross]

There has been a report about incorrect vruntime accounting in Linux
guests under Xen. A Linux kernel with CONFIG_PARAVIRT_TIME_ACCOUNTING
set is capable to do correct vruntime accounting, but this would
require the kernel to be able to read the runstate data of other cpus.

A guest mapping vcpu_runstate_info into its memory can't read this
information from another cpu but the one the data is referring to.
Reason is there is no reliable way for the guest to detect a concurrent
data update by the hypervisor.

This patch series adds an update flag to the mapped data which can be
used by the guest to detect an update is occurring. As this flag is
modifying the current interface it has to be activated by using a
vm_assist hypercall, which in turn has to be made available for ARM.

Runtime tested on x86 with a modified Linux kernel using the new
feature.
Compile tested only for ARM.

Changes in V3:
- patch 2: simplify update_runstate_area() address calculation as
  requested by Jan Beulich

Changes in V2:
- patch 1: readded the #ifdef's as requested by Jan Beulich
- patch 2: smp_wmb() instead of wmb() as requested by Andrew Cooper
- patch 2: use sizeof() as requested by Jan Beulich
- patch 2: eliminate new variable update_flag as requested by Jan Beulich

Juergen Gross (2):
  xen/arm: add support for vm_assist hypercall
  xen: add update indicator to vcpu_runstate_info

 xen/arch/arm/domain.c| 20 
 xen/arch/arm/traps.c |  1 +
 xen/arch/x86/domain.c| 21 +
 xen/include/asm-arm/config.h |  2 ++
 xen/include/asm-x86/config.h |  1 +
 xen/include/public/vcpu.h|  6 ++
 xen/include/public/xen.h |  7 +++
 7 files changed, 58 insertions(+)

-- 
2.6.6


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

[Xen-devel] [PATCH v9 1/3] vt-d: fix the IOMMU flush issue

[Xu, Quan]

From: Quan Xu 

The propagation value from IOMMU flush interfaces may be positive, which
indicates callers need to flush cache, not one of faliures.

when the propagation value is positive, this patch fixes this flush issue
as follows:
  - call iommu_flush_write_buffer() to flush cache.
  - return zero.

Signed-off-by: Quan Xu 
Acked-by: Kevin Tian 

CC: Kevin Tian 
CC: Feng Wu 
CC: Keir Fraser 
CC: Jan Beulich 
CC: Andrew Cooper 

---
v9: fix naming issue, changing 'iommu_rc / iommu_ret' to 'context_rc / iotlb_rc'
---
 xen/drivers/passthrough/vtd/iommu.c | 150 +---
 1 file changed, 103 insertions(+), 47 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 48edb67..2679ef6 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -388,17 +388,18 @@ static int flush_context_reg(
 return 0;
 }
 
-static int iommu_flush_context_global(
-struct iommu *iommu, int flush_non_present_entry)
+static int __must_check iommu_flush_context_global(struct iommu *iommu,
+   int flush_non_present_entry)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 return flush->context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL,
  flush_non_present_entry);
 }
 
-static int iommu_flush_context_device(
-struct iommu *iommu, u16 did, u16 source_id,
-u8 function_mask, int flush_non_present_entry)
+static int __must_check iommu_flush_context_device(struct iommu *iommu,
+   u16 did, u16 source_id,
+   u8 function_mask,
+   int flush_non_present_entry)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 return flush->context(iommu, did, source_id, function_mask,
@@ -473,8 +474,9 @@ static int flush_iotlb_reg(void *_iommu, u16 did,
 return 0;
 }
 
-static int iommu_flush_iotlb_global(struct iommu *iommu,
-int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check iommu_flush_iotlb_global(struct iommu *iommu,
+ int flush_non_present_entry,
+ int flush_dev_iotlb)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 int status;
@@ -491,8 +493,9 @@ static int iommu_flush_iotlb_global(struct iommu *iommu,
 return status;
 }
 
-static int iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
-int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
+  int flush_non_present_entry,
+  int flush_dev_iotlb)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 int status;
@@ -509,9 +512,10 @@ static int iommu_flush_iotlb_dsi(struct iommu *iommu, u16 
did,
 return status;
 }
 
-static int iommu_flush_iotlb_psi(
-struct iommu *iommu, u16 did, u64 addr, unsigned int order,
-int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check iommu_flush_iotlb_psi(struct iommu *iommu, u16 did,
+  u64 addr, unsigned int order,
+  int flush_non_present_entry,
+  int flush_dev_iotlb)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 int status;
@@ -546,17 +550,37 @@ static int __must_check iommu_flush_all(void)
 struct acpi_drhd_unit *drhd;
 struct iommu *iommu;
 int flush_dev_iotlb;
+int rc = 0;
 
 flush_all_cache();
 for_each_drhd_unit ( drhd )
 {
+int context_rc, iotlb_rc;
+
 iommu = drhd->iommu;
-iommu_flush_context_global(iommu, 0);
+context_rc = iommu_flush_context_global(iommu, 0);
 flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
+iotlb_rc = iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
+
+/*
+ * The current logic for returns:
+ *   - positive  invoke iommu_flush_write_buffer to flush cache.
+ *   - zero  on success.
+ *   - negative  on failure. Continue to flush IOMMU IOTLB on a
+ *   best effort basis.
+ */
+if ( context_rc > 0 || iotlb_rc > 0 )
+iommu_flush_write_buffer(iommu);
+if ( context_rc >= 0 )
+rc = context_rc;
+if ( rc >= 0 )
+rc = iotlb_rc;
 }
 
-return 0;
+if ( rc > 0 )
+rc = 0;
+
+return rc;
 }
 
 static int __must_check 

[Xen-devel] [PATCH v9 3/3] vt-d: add __must_check annotation to IOMMU flush pointers and handlers

[Xu, Quan]

From: Quan Xu 

Signed-off-by: Quan Xu 
Acked-by: Kevin Tian 
Reviewed-by: Jan Beulich 

CC: Jan Beulich 
CC: Kevin Tian 
CC: Feng Wu 
---
 xen/drivers/passthrough/vtd/iommu.c  | 50 ++--
 xen/drivers/passthrough/vtd/iommu.h  | 11 +---
 xen/drivers/passthrough/vtd/qinval.c | 14 +-
 3 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index cc47c30..7d413f1 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -335,10 +335,9 @@ static void iommu_flush_write_buffer(struct iommu *iommu)
 }
 
 /* return value determine if we need a write buffer flush */
-static int flush_context_reg(
-void *_iommu,
-u16 did, u16 source_id, u8 function_mask, u64 type,
-int flush_non_present_entry)
+static int __must_check flush_context_reg(void *_iommu, u16 did, u16 source_id,
+  u8 function_mask, u64 type,
+  bool_t flush_non_present_entry)
 {
 struct iommu *iommu = (struct iommu *) _iommu;
 u64 val = 0;
@@ -389,7 +388,7 @@ static int flush_context_reg(
 }
 
 static int __must_check iommu_flush_context_global(struct iommu *iommu,
-   int flush_non_present_entry)
+   bool_t 
flush_non_present_entry)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 return flush->context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL,
@@ -399,7 +398,7 @@ static int __must_check iommu_flush_context_global(struct 
iommu *iommu,
 static int __must_check iommu_flush_context_device(struct iommu *iommu,
u16 did, u16 source_id,
u8 function_mask,
-   int flush_non_present_entry)
+   bool_t 
flush_non_present_entry)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 return flush->context(iommu, did, source_id, function_mask,
@@ -408,9 +407,10 @@ static int __must_check iommu_flush_context_device(struct 
iommu *iommu,
 }
 
 /* return value determine if we need a write buffer flush */
-static int flush_iotlb_reg(void *_iommu, u16 did,
-   u64 addr, unsigned int size_order, u64 type,
-   int flush_non_present_entry, int flush_dev_iotlb)
+static int __must_check flush_iotlb_reg(void *_iommu, u16 did, u64 addr,
+unsigned int size_order, u64 type,
+bool_t flush_non_present_entry,
+bool_t flush_dev_iotlb)
 {
 struct iommu *iommu = (struct iommu *) _iommu;
 int tlb_offset = ecap_iotlb_offset(iommu->ecap);
@@ -475,8 +475,8 @@ static int flush_iotlb_reg(void *_iommu, u16 did,
 }
 
 static int __must_check iommu_flush_iotlb_global(struct iommu *iommu,
- int flush_non_present_entry,
- int flush_dev_iotlb)
+ bool_t 
flush_non_present_entry,
+ bool_t flush_dev_iotlb)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 int status;
@@ -494,8 +494,8 @@ static int __must_check iommu_flush_iotlb_global(struct 
iommu *iommu,
 }
 
 static int __must_check iommu_flush_iotlb_dsi(struct iommu *iommu, u16 did,
-  int flush_non_present_entry,
-  int flush_dev_iotlb)
+  bool_t flush_non_present_entry,
+  bool_t flush_dev_iotlb)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 int status;
@@ -514,8 +514,8 @@ static int __must_check iommu_flush_iotlb_dsi(struct iommu 
*iommu, u16 did,
 
 static int __must_check iommu_flush_iotlb_psi(struct iommu *iommu, u16 did,
   u64 addr, unsigned int order,
-  int flush_non_present_entry,
-  int flush_dev_iotlb)
+  bool_t flush_non_present_entry,
+  bool_t flush_dev_iotlb)
 {
 struct iommu_flush *flush = iommu_get_flush(iommu);
 int status;
@@ -549,7 +549,7 @@ static int __must_check iommu_flush_all(void)
 {
 struct acpi_drhd_unit *drhd;
 struct iommu *iommu;
-int flush_dev_iotlb;
+bool_t flush_dev_iotlb;
 int rc = 0;
 
 

[Xen-devel] [PATCH v9 0/3] Check VT-d Device-TLB flush error

[Xu, Quan]

From: Quan Xu 

This patch set is a prereq patch set for Patch:'VT-d Device-TLB flush issue'.

While IOMMU Device-TLB flush timed out, xen calls panic() at present. However 
the existing panic()
is going to be eliminated, so we must propagate the IOMMU Device-TLB flush 
error up to the call trees.

This patch set is also based on the discussion of 'abstract model of IOMMU 
unmaping/mapping failures'.

---
Quan Xu (3):
  vt-d: fix the IOMMU flush issue
  vt-d: propagate the IOMMU Device-TLB flush error up to ME phantom
functions
  vt-d: add __must_check annotation to IOMMU flush pointers and handlers

 xen/drivers/passthrough/vtd/extern.h |   3 +-
 xen/drivers/passthrough/vtd/iommu.c  | 184 +++
 xen/drivers/passthrough/vtd/iommu.h  |  11 ++-
 xen/drivers/passthrough/vtd/qinval.c |  14 +--
 xen/drivers/passthrough/vtd/quirks.c |  27 +++--
 5 files changed, 153 insertions(+), 86 deletions(-)

-- 
1.9.1


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

Re: [Xen-devel] [PATCH RESEND 05/14] libxl/arm: Construct ACPI GTDT table

[Shannon Zhao]

On 2016/6/6 19:40, Stefano Stabellini wrote:
> ACPI tables for ARM guests should be user configurable: acpi=1 in the VM
> config file enables them, with default off.
While the configuration "acpi" already exists for HVM guest
configuration, do we plan to reuse it or use a new name, e.g. arm_acpi?

Thanks,
-- 
Shannon


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

[Xen-devel] qemu-traditional build problem for Xen 4.7.0 RC6

[Hao, Xudong]

QEMU_TAG update to 698d6d6f8d095edadb0c23612b552a89dd3eee4c of traditional qemu 
in Config.mk, this commit is a branch stable-4.7 commit for qemu traditional. 
The master commit is 6e20809727261599e8527c456eb078c0e89139a1.
It bring up a build error.

commit 03c716e32912e288929a6e0d9c8729d208462d66
Author: Ian Jackson 
Date:   Fri Jun 10 11:49:24 2016 +0100

QEMU_TAG update

diff --git a/Config.mk b/Config.mk
index bc5c456..1d6522c 100644
--- a/Config.mk
+++ b/Config.mk
@@ -284,9 +284,9 @@ SEABIOS_UPSTREAM_REVISION ?= rel-1.9.2
 ETHERBOOT_NICS ?= rtl8139 8086100e


-QEMU_TRADITIONAL_REVISION ?= df553c056104e3dd8a2bd2e72539a57c4c085bae
-# Thu May 5 11:14:44 2016 +0100
-# Fix build with newer version of GNUTLS
+QEMU_TRADITIONAL_REVISION ?= 698d6d6f8d095edadb0c23612b552a89dd3eee4c
+# Thu May 19 19:38:35 2016 +0100
+# main loop: Big hammer to fix logfile disk DoS in Xen setups

 # Specify which qemu-dm to use. This may be `ioemu' to use the old
 # Mercurial in-tree version, or a local directory, or a git URL.

Best Regards,
Xudong


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

Re: [Xen-devel] [PATCH RESEND 00/14] Xen ARM DomU ACPI support

[Shannon Zhao]

On 2016/6/16 19:20, Wei Liu wrote:
> I've read this sub-thread and the other thread in which Boris replied, I
> think due to the fact that libxl has more information at hand and libxc
> is intended to be simple, I think having the building code in libxl
> makes sense.
> 
> Shannon, Boris and Julien, what do you guys think? Can we agree on
> something so that Shannon can move on with next iteration?
I agree with this and will work on the next version of this series.

Thanks,
-- 
Shannon


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

[Xen-devel] [libvirt test] 95814: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 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-amd64-i386-libvirt-xsm  12 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-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  2c51fa6ec4d34ccdd63f0935fb7530b681b126ac
baseline version:
 libvirt  1a41ed5af5e1704dd9b0bdca40df5c9cacbdabfc

Last test of basis95460  2016-06-09 05:57:15 Z7 days
Failing since 95520  2016-06-10 18:36:19 Z6 days6 attempts
Testing same since95814  2016-06-16 04:24:25 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Chunyan Liu 
  Daniel P. Berrange 
  Guido Günther 
  Jim Fehlig 
  Jingjing Shao 
  Jiri Denemark 
  John Ferlan 
  Ján Tomko 
  Martin Kletzander 
  Maxim Nestratov 
  Michal Privoznik 
  Mikhail Feoktistov 
  Nikolay Shirokovskiy 
  Pavel Hrdina 
  Riku Voipio 
  Roman Bogorodskiy 
  Wang Yufei 
  Wei Liu 

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 fail
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt fail
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-armhf-armhf-libvirt-qcow2   fail
 test-armhf-armhf-libvirt-raw fail
 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

[Xen-devel] [linux-3.18 test] 95809: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumpuserxen  3 host-install(3) broken in 95762 REGR. vs. 94728
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1fail REGR. vs. 94728

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl3 host-install(3)  broken in 95762 pass in 95809
 test-amd64-i386-qemut-rhel6hvm-amd 3 host-install(3) broken in 95762 pass in 
95809
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 3 host-install(3) broken in 
95762 pass in 95809
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken in 
95762 pass in 95809
 test-amd64-i386-xl-qemut-winxpsp3 3 host-install(3) broken in 95762 pass in 
95809
 test-amd64-amd64-xl-qemut-winxpsp3 3 host-install(3) broken in 95762 pass in 
95809
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 15 
guest-localmigrate/x10 fail in 95458 pass in 95809
 test-armhf-armhf-xl-credit2 15 guest-start/debian.repeat fail in 95762 pass in 
95809
 test-amd64-i386-freebsd10-amd64 10 guest-start  fail pass in 95458
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop  fail pass in 95521
 test-armhf-armhf-xl-arndale   6 xen-bootfail pass in 95762

Regressions which are regarded as allowable (not blocking):
 build-i386-rumpuserxen6 xen-buildfail   like 94728
 build-amd64-rumpuserxen   6 xen-buildfail   like 94728
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 94728
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 94728
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 94728

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale  12 migrate-support-check fail in 95762 never pass
 test-armhf-armhf-xl-arndale 13 saverestore-support-check fail in 95762 never 
pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   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-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-xsm  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 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
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-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-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-amd64-amd64-libvirt-vhd 11 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 13 guest-saverestorefail   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
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass

version targeted for testing:
 linuxb5076139991c6b12c62346d9880eec1d4227d99f
baseline version:
 linux3b6aa07b936b09d38c1bfcee1e06845b968df475

Last test of basis

[Xen-devel] [xen-unstable test] 95801: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds  6 xen-boot  fail REGR. vs. 95353
 build-amd64-rumpuserxen   6 xen-buildfail   like 95353
 build-i386-rumpuserxen6 xen-buildfail   like 95353
 test-amd64-amd64-xl-rtds  9 debian-install   fail   like 95353
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 95353
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 95353
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 95353
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 95353

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   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 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 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-xsm 12 migrate-support-checkfail   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-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-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-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-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-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-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 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

version targeted for testing:
 xen  759b9618b8a22ddd87d01c0bff5366814b17eea7
baseline version:
 xen  c2a17869d5dcd845d646bf4db122cad73596a2be

Last test of basis95353  2016-06-07 02:01:22 Z9 days
Failing since 95414  2016-06-08 03:47:23 Z8 days6 attempts
Testing same since95801  2016-06-15 23:00:36 Z1 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anil Madhavapeddy 
  Christoph Egger 
  Daniel De Graaf 
  Daniel Kiper 
  David Scott 
  Dongli Zhang 
  Doug Goldstein 
  Euan Harris 
  George Dunlap 
  George Dunlap 
  Haozhong Zhang 
  Ian Jackson 
  Jan Beulich 
  Jason Andryuk 
  Jiandi An 
  Julien Grall 
  Kevin Tian 
  Konrad Rzeszutek Wilk 
  Konrad Rzeszutek Wilk 
  Len Brown 
  Marek Marczykowski-Górecki 

Re: [Xen-devel] [PATCH 1/1] tools/livepatch: initialise j to 0 to make some versions of gcc happy

[Dongli Zhang]

> I suggest pasting in Olaf's exact error message here.
> 
> To avoid extra round trip, I propose updating the commit message as
> followed:
> 
> Initialise j to 0 to make some versions of gcc (e.g., gcc4.5/4.3)
> happy to
> avoid compilation error by commit
> beba3693f7243e68bbe31fe3794da91068eeea5b.
> 
> Failure manifests with gcc 4.5 as:
> 
> [  153s] cc1: warnings being treated as errors
> [  153s] xen-livepatch.c: In function 'main':
> [  153s] xen-livepatch.c:415:12: error: 'j' may be used
> uninitialized in this function
> [  153s] make[3]: *** [xen-livepatch.o] Error 1
> 
> If you agree with this I will handle the updating while doing my next
> sweep.


Sure. Feel free to update the commit message.

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

Re: [Xen-devel] [PATCH 7/7] vm-event/arm: implement support for control-register write vm-events

[Julien Grall]

On 16/06/2016 20:24, Corneliu ZUZU wrote:

On 6/16/2016 5:26 PM, Julien Grall wrote:
Hi Julien. Yes, I agree that it's complex, I would have preferred to
split it up too and I actually tried, but the changes are tightly
coupled and they don't seem to be 'split-able'.


After I reviewed this patch I think it could be simplified a lot.

Most of the registers not trapped by vm-event could be emulated with 
one-line WRITE_SYSREGxx and does not require specific case depending on 
the architecture.


You can give a look how we handle the domain context switch in C 
(arch/arm/domain.c).


Only few of the registers will require more than one-line (such as 
MAIR*, *FAR) which could be over a macro.


Regards,

--
Julien Grall

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

[Xen-devel] [linux-3.10 test] 95804: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-qemut-rhel6hvm-amd  3 host-install(3)   broken REGR. vs. 86412
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1fail REGR. vs. 86412
 test-amd64-amd64-qemuu-nested-amd 13 xen-boot/l1  fail REGR. vs. 86412

Tests which are failing intermittently (not blocking):
 test-amd64-i386-libvirt-pair 3 host-install/src_host(3) broken in 95747 pass 
in 95804
 test-amd64-i386-pair  3 host-install/src_host(3) broken in 95747 pass in 95804
 test-amd64-amd64-amd64-pvgrub  3 host-install(3) broken in 95747 pass in 95804
 test-amd64-i386-qemuu-rhel6hvm-amd 3 host-install(3) broken in 95747 pass in 
95804
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 3 host-install(3) broken in 95747 
pass in 95804
 test-amd64-i386-xl-qemuu-winxpsp3 3 host-install(3) broken in 95747 pass in 
95804
 test-amd64-amd64-i386-pvgrub  3 host-install(3)   broken pass in 95747

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail in 95747 like 86412
 build-amd64-rumpuserxen   6 xen-buildfail   like 86412
 build-i386-rumpuserxen6 xen-buildfail   like 86412
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 86412
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 86412
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 86412

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 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-amd64-libvirt-xsm 12 migrate-support-checkfail   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  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

version targeted for testing:
 linuxca1199fccf14540e86f6da955333e31d6fec5f3e
baseline version:
 linux326a1b2d50cbe1f890e56ab60b9215cd30053f5a

Last test of basis86412  2016-03-16 16:24:33 Z   92 days
Testing same since95665  2016-06-13 14:51:45 Z3 days3 attempts


People who touched revisions under test:
  Aaro Koskinen 
  Adrian Hunter 
  Al Viro 
  Alan Stern 
  Alex Deucher 
  Alexandre Belloni 
  Alexandre Bounine 
  Alexey Khoroshilov 
  Allain Legacy 
  Andi Kleen 
  Andrew Morton 
  Andrey Gelman 
  Andy Lutomirski 
  Andy Lutomirski  # On a Dell XPS 13 9350
  Anton Blanchard 
  Antonio Quartulli 
  Aristeu Rozanski 
  Arnaldo Carvalho de Melo 
  Arnd Bergmann 
  Artem Bityutskiy 
  Aurelien Jacquiot 
  Behan Webster 
  Ben Hutchings 
  Bill Sommerfeld 
  Bjorn Helgaas 
  Bjørn Mork 
  Bob Moore 
  Borislav Petkov 
  Brian King 
  Brian Norris 
  Chanwoo Choi 
  Chas Williams <3ch...@gmail.com>
  Chris Friesen 
  Dan Carpenter 
  Dan Streetman 
  Daniel Fraga 
  Daniel Lezcano 
  David S. Miller 
  Diego Viola 
  Dinh Nguyen 
  Dmitry Ivanov 
  Dmitry Ivanov 
  Dmitry Torokhov 
  Douglas Gilbert 
  Eric Wheeler 
  Eric Wheeler 
  Eryu Guan 
  Felipe Balbi 
  

Re: [Xen-devel] [PATCH 1/8] x86/time: improve cross-CPU clock monotonicity (and more)

[Joao Martins]

>>> +unsigned long flags;
>>> +u64 tsc;
>>> +
>>> +local_irq_save(flags);
>>> +ap_bringup_ref.master_stime = read_platform_stime();
>>> +tsc = rdtsc();
>>> +local_irq_restore(flags);
>>> +
>>> +ap_bringup_ref.local_stime = get_s_time_fixed(tsc);
>>> +}
>>> +
>>>  void init_percpu_time(void)
>>>  {
>>>  struct cpu_time *t = _cpu(cpu_time);
>>>  unsigned long flags;
>>> +u64 tsc;
>>>  s_time_t now;
>>>  
>>>  /* Initial estimate for TSC rate. */
>>>  t->tsc_scale = per_cpu(cpu_time, 0).tsc_scale;
>>>  
>>>  local_irq_save(flags);
>>> -t->local_tsc_stamp = rdtsc();
>>>  now = read_platform_stime();
>> Do we need to take another read from the platform timer here? We could
>> just reuse the one on ap_bringup_ref.master_stime. See also below.
> 
> Yes, for this model of approximation of the local stime delta by
> master stime delta we obviously need to read the master clock
> here again (or else we have no way to calculate the master
> delta, which we want to use as the correcting value).
Ah, correct.

>> So it would be possible that 
>> get_s_time(S0) on
>> CPU0 immediately followed by a get_s_time(S1) on CPU1 [S1 > S0] ends up 
>> returning a
>> value backwards IOW L0 + scale(S0 - T0) is bigger than L1 + scale(S1 - T1). 
>> That is
>> independently of the deltas being added on every calibration.
>>
>> So how about we do the seeding in another way?
>>
>> 1) Relying on individual CPU TSC like you do on CPU 0:
>>
>>  t->stamp.master_stime = now;
>> +t->stamp.local_tsc = 0;
>> +t->stamp.local_stime = 0;
>> +
>>  if ( boot_cpu_has(X86_FEATURE_CONSTANT_TSC) )
>>  {
>>  now = get_s_time_fixed(tsc);
>>  }
> 
> As said before elsewhere, such a model can only work if all TSCs
> start ticking at the same time. The latest this assumption breaks 
> is when a CPU gets physically hotplugged.
Agreed.

>> Or 2) taking into account the skew between platform timer and tsc we take on
>> init_percpu_time. Diff below based on this series:
>>
>> @@ -1394,11 +1508,14 @@ void init_percpu_time(void)
>>  t->tsc_scale = per_cpu(cpu_time, 0).tsc_scale;
>>
>>  local_irq_save(flags);
>> -now = read_platform_stime();
>> +now = ap_bringup_ref.master_stime;
>>  tsc = rdtsc_ordered();
>>  local_irq_restore(flags);
>>
>>  t->stamp.master_stime = now;
>> +t->stamp.local_tsc = boot_tsc_stamp;
> 
> Again - this is invalid. It indeed works fine on a single socket system
> (where all TSCs start ticking at the same time), and gives really good
> results. But due to hotplug (and to a lesser degree multi-socket, but
> likely to a somewhat higher degree multi-node, systems) we just can't
> use boot_tsc_stamp as reference for APs.
It also gives really good results on my multi-socket system at least on my old 
Intel
multi-socket box (Westmere-based; TSC invariant is introduced on its 
predecessor:
Nehalem). Btw, Linux seems to take Intel boxes as having synchronized TSCs 
across
cores and sockets [0][1]. Though CPU hotplug is the troublesome case.

[0] arch/x86/kernel/tsc.c#L1082
[1] arch/x86/kernel/cpu/intel.c#L85

> 
>> +t->stamp.local_stime = 0;
>> +
>>
>>  /*
>>   * To avoid a discontinuity (TSC and platform clock can't be expected
>>   * to be in perfect sync), initialization here needs to match up with
>> @@ -1406,10 +1523,12 @@ void init_percpu_time(void)
>>   */
>>  if ( boot_cpu_has(X86_FEATURE_CONSTANT_TSC) )
>>  {
>> +u64 stime = get_s_time_fixed(tsc);
>> +
>>  if ( system_state < SYS_STATE_smp_boot )
>> -now = get_s_time_fixed(tsc);
>> +now = stime;
>>  else
>> -now += ap_bringup_ref.local_stime - ap_bringup_ref.master_stime;
>> +now += stime - ap_bringup_ref.master_stime;
> 
> That seems to contradict your desire to keep out of all calculations
> the skew between platform timer and TSC.
> 
That's indeed my desire (on my related series about using tsc as clocksource 
and tsc
stable bit), but being aware of TSC limitations: I was trying to see if there 
was
common ground between this seeding with platform timer while accounting to the
potential unreliable TSC of individual CPUs. But of course, for any of these
solutions to have get_s_time return monotonically increasing values, it can 
only work
on a truly invariant TSC box.

>>  }
>>
>> Second option follows a similar thinking of this patch, but on both ways the
>> local_stime will match the tsc on init_percpu_time, thus being a matched 
>> pair. I have
>> a test similar to check_tsc_warp but with get_s_time() and I no longer 
>> observe time
>> going backwards. But without the above I still observe it even on short 
>> periods.
> 
> Right - I'm not claiming the series eliminates all backwards moves.
> But I simply can't see a model to fully eliminate them. 
Totally agree with you.

> I.e. my plan was, once the backwards moves are small enough, to maybe
> indeed 

Re: [Xen-devel] [PATCH 3/7] vm-event: introduce vm_event_vcpu_enter

[Corneliu ZUZU]

On 6/16/2016 5:51 PM, Jan Beulich wrote:

On 16.06.16 at 16:08,  wrote:

@@ -509,6 +508,8 @@ void hvm_do_resume(struct vcpu *v)
  }
  }
  
+vm_event_vcpu_enter(v);

Why here?
Why indeed. It made sense because monitor_write_data handling was 
originally there and then the plan was to move it to vm_event_vcpu_enter 
(which happens in the following commit).
The question is though, why was monitor_write_data handled there in the 
first place? Why was it not put e.g. in vmx_do_resume immediately after 
the call to hvm_do_resume and just before
the reset_stack_and_jump...? And what happens with handling 
monitor_write_data if this:


if ( !handle_hvm_io_completion(v) )
return;

causes a return?


@@ -3874,6 +3875,9 @@ void vmx_vmenter_helper(const struct cpu_user_regs *regs)
  }
  
   out:

+if ( guest_mode(regs) )
+vm_event_vcpu_enter(curr);
+
  HVMTRACE_ND(VMENTRY, 0, 1/*cycles*/, 0, 0, 0, 0, 0, 0, 0);
  
  __vmwrite(GUEST_RIP,regs->rip);

Why with a conditional? The registers can't possibly hold to non-
guest state when you're here.


Right, I must've mistakenly taken that from ARM-side @ some point. There 
it made sense, here it doesn't.

I'll remove it in V2.




--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -36,7 +36,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 

There are way too many of these #include adjustments here. If
you really mean to clean these up, please don't randomly throw
this into various unrelated patches.


I haven't thrown anything "randomly into unrelated patches", please 
first ask for my reasoning and then draw such conclusions.
That was removed because xen/vm_event.h includes asm/vm_event.h with 
this patch (because it calls arch_vm_event_vcpu_enter) and this file 
(p2m.c) already included xen/vm_event.h.



--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -22,8 +22,8 @@
  #include 
  #include 
  #include 
+#include 
  #include 
-#include 

Please retain at least basic grouping (i.e. all xen/ ones together,
meaning the insertion should move up by one line).

Jan




I usually do that, in this case I just didn't notice. Acked.

Corneliu.

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

Re: [Xen-devel] [PATCH 7/7] vm-event/arm: implement support for control-register write vm-events

[Corneliu ZUZU]

On 6/16/2016 5:26 PM, Julien Grall wrote:

Hello Corneliu,

On 16/06/16 15:13, Corneliu ZUZU wrote:
Add ARM support for control-register write monitoring through the 
vm-events

subsystem.

Chosen ARM system control-registers that can be monitored are:
 - VM_EVENT_ARM_SCTLR:  AArch32 SCTLR, AArch64 SCTLR_EL1
 - VM_EVENT_ARM_TTBR{0,1}:  AArch32 TTBR{0,1}, AArch64 TTBR{0,1}_EL1
 - VM_EVENT_ARM_TTBCR:  AArch32 TTBCR, AArch64 TCR_EL1

Trapping of write operations of these registers was attained by 
setting the

HCR_EL2.TVM / HCR.TVM bit.

Signed-off-by: Corneliu ZUZU 
---
  MAINTAINERS|   1 +
  xen/arch/arm/Makefile  |   1 +
  xen/arch/arm/traps.c   | 126 +++-
  xen/arch/arm/vm_event.c| 112 ++
  xen/common/monitor.c   |   2 -
  xen/common/vm_event.c  |   2 -
  xen/include/asm-arm/domain.h   |  30 +
  xen/include/asm-arm/traps.h| 253 
+

  xen/include/asm-arm/vm_event.h |  22 +++-
  xen/include/public/vm_event.h  |   8 +-
  xen/include/xen/monitor.h  |   2 -
  xen/include/xen/vm_event.h |   2 -
  12 files changed, 543 insertions(+), 18 deletions(-)


I think this patch would benefit to be split in multiple patches to 
ease the review and also describe the infrastructure you have 
introduced (TVM_* & co).


I will review in detail later.

Regards,



Hi Julien. Yes, I agree that it's complex, I would have preferred to 
split it up too and I actually tried, but the changes are tightly 
coupled and they don't seem to be 'split-able'.


Corneliu.

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

Re: [Xen-devel] [PATCH 1/7] minor (formatting) fixes

[Corneliu ZUZU]

On 6/16/2016 5:24 PM, Jan Beulich wrote:

On 16.06.16 at 16:06,  wrote:

--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -23,6 +23,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index d950a7c..b30857a 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -22,7 +22,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 

These two adjustments clearly don't fit title / description. I certainly
don't mind unnecessary inclusions to be dropped, but the addition of
one clearly needs explanation (after all thing build fine without it).


Sorry, that was done out of reflex, should have stated the reasoning.
Generally, if:
- event.c includes event.h
- event.c needs paging.h
- event.h -doesn't need- paging.h
then I prefer to include paging.h in event.c, not in event.h (include 
strictly -where- needed).
Also since xen/paging.h included asm/paging.h and event.c only needs the 
asm/paging.h part, I also changed xen/paging.h inclusion -> asm/paging.h 
inclusion (include strictly -what's- needed).
But I can revert that if you want me to (not that important and these 
little things are better done with automatic tools anyway), or should I 
leave the change and update the commit message?



--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -44,6 +44,9 @@ void vm_event_set_registers(struct vcpu *v, 
vm_event_response_t *rsp);
  
  void vm_event_fill_regs(vm_event_request_t *req);
  
+/*

+ * Monitor vm-events.
+ */

This is a single line comment (also elsewhere).


Ack.




--- a/xen/include/xen/vm_event.h
+++ b/xen/include/xen/vm_event.h
@@ -85,7 +85,6 @@ void vm_event_monitor_guest_request(void);
  
  #endif /* __VM_EVENT_H__ */
  
-

  /*
   * Local variables:
   * mode: C

Why don't you remove the other stray blank line at once?

Jan


What stray line? Shouldn't there be -one- blank line between the #endif 
and the local vars block?

Looking @ other files that rule seems to hold...

Corneliu.

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

[Xen-devel] [qemu-mainline test] 95783: regressions - trouble: broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl   3 host-install(3) broken REGR. vs. 94856
 test-amd64-i386-xl3 host-install(3) broken REGR. vs. 94856
 test-amd64-amd64-xl-credit2   3 host-install(3) broken REGR. vs. 94856
 test-amd64-amd64-libvirt-pair 3 host-install/src_host(3) broken REGR. vs. 94856
 test-amd64-i386-libvirt-pair 3 host-install/src_host(3) broken REGR. vs. 94856
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 94856
 test-amd64-i386-freebsd10-amd64 10 guest-startfail REGR. vs. 94856
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1fail REGR. vs. 94856
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 94856
 test-amd64-i386-xl-qemuu-winxpsp3 17 guest-start/win.repeat fail REGR. vs. 
94856

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 94856
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 94856
 test-amd64-amd64-xl-rtds  9 debian-install   fail   like 94856

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

version targeted for testing:
 qemuu49237b856ae58ee7955be0b959c504c51b014f20
baseline version:
 qemuud6550e9ed2e1a60d889dfb721de00d9a4e3bafbe

Last test of basis94856  2016-05-27 20:14:49 Z   19 days
Failing since 94983  2016-05-31 09:40:12 Z   16 days   20 attempts
Testing same since95783  2016-06-15 15:16:41 Z1 days1 attempts


People who touched revisions under test:
  Alberto Garcia 
  Alex Bennée 
  Alex Bligh 
  Alexander Graf 
  Alexey Kardashevskiy 
  Alistair Francis 
  Andrew Jones 
  Anthony PERARD 
  Anton Blanchard 
  Ard Biesheuvel 
  Benjamin Herrenschmidt 
  Bharata B Rao 

Re: [Xen-devel] HEADS UP: Imported Xen 4.7: no blkback

[Marcin Cieslak]

On Thu, 16 Jun 2016, Roger Pau Monné wrote:

> I've just imported Xen 4.7.0-rc6 into the ports tree, could you give it a 
> try when you have a moment?

Yes, of course. A quick test shows no change - Windows get stuck
at the UEFI shell with some block devices listed (I use "hda" for
Windows), SeaBIOS cannot boot FreeBSD with "xvda". FreeBSD
works with "hda", but I have to mount "/dev/ada0p1".

Marcin

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

Re: [Xen-devel] [PATCH 6/7] vm-event/arm: move hvm_event_cr->common vm_event_monitor_cr

[Tamas K Lengyel]

On Thu, Jun 16, 2016 at 8:12 AM, Corneliu ZUZU  wrote:
> Prepare for ARM implementation of control-register write vm-events by moving
> X86-specific hvm_event_cr to the common-side.
>
> Signed-off-by: Corneliu ZUZU 
> ---
>  xen/arch/x86/hvm/event.c| 30 --
>  xen/arch/x86/hvm/hvm.c  |  2 +-
>  xen/arch/x86/monitor.c  | 37 -
>  xen/arch/x86/vm_event.c |  2 +-
>  xen/common/monitor.c| 40 
>  xen/common/vm_event.c   | 31 +++
>  xen/include/asm-x86/hvm/event.h | 13 -
>  xen/include/asm-x86/monitor.h   |  2 --
>  xen/include/xen/monitor.h   |  4 
>  xen/include/xen/vm_event.h  | 10 ++
>  10 files changed, 91 insertions(+), 80 deletions(-)

Some of the code-movement here touches code I cleaned up in
https://patchwork.kernel.org/patch/9151229 to keep vm_event and
monitor code separate as much as possible. The same separation should
be followed when we move code to common.

Tamas

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

Re: [Xen-devel] [PATCH 7/7] vm-event/arm: implement support for control-register write vm-events

[Julien Grall]

Hello Corneliu,

On 16/06/16 15:13, Corneliu ZUZU wrote:

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 8c50685..af61ac3 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -43,6 +43,7 @@
  #include 
  #include 
  #include 
+#include 

  #include "decode.h"
  #include "vtimer.h"
@@ -124,7 +125,12 @@ void init_traps(void)
  WRITE_SYSREG((HCPTR_CP_MASK & ~(HCPTR_CP(10) | HCPTR_CP(11))) | HCPTR_TTA,
   CPTR_EL2);

-/* Setup hypervisor traps */
+/* Setup hypervisor traps
+ *
+ * Note: HCR_TVM bit is also set for system-register write monitoring
+ * purposes (see vm_event_monitor_cr), but (for performance reasons) that's
+ * done selectively (see vcpu_enter_adjust_traps).
+ */
  WRITE_SYSREG(HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
   HCR_TWE|HCR_TWI|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB,
   HCR_EL2);
@@ -1720,6 +1726,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
  const struct hsr_cp32 cp32 = hsr.cp32;
  int regidx = cp32.reg;
  struct vcpu *v = current;
+register_t r = get_user_reg(regs, regidx);

  if ( !check_conditional_instr(regs, hsr) )
  {
@@ -1730,6 +1737,61 @@ static void do_cp15_32(struct cpu_user_regs *regs,
  switch ( hsr.bits & HSR_CP32_REGS_MASK )
  {
  /*
+ * HCR_EL2.TVM / HCR.TVM
+ *
+ * ARMv7 (DDI 0406C.b): B1.14.13


Please try to mention the most recent spec. This was published in 2012, 
the latest release (C.c) was published in 2014.



+ * ARMv8 (DDI 0487A.e): D1-1569 Table D1-34


Ditto. This was published in 2014, whilst the most recent was published 
in 2016.



+ */
+case HSR_CPREG32(SCTLR):
+TVM_EMUL_VMEVT(v, regs, hsr, r, SCTLR);
+break;
+case HSR_CPREG32(TTBR0_32):
+TVM_EMUL_VMEVT(v, regs, hsr, r, TTBR0_32);
+break;
+case HSR_CPREG32(TTBR1_32):
+TVM_EMUL_VMEVT(v, regs, hsr, r, TTBR1_32);
+break;
+case HSR_CPREG32(TTBCR):
+TVM_EMUL_VMEVT(v, regs, hsr, r, TTBCR);
+break;
+case HSR_CPREG32(DACR):
+TVM_EMUL(regs, hsr, r, DACR);
+break;
+case HSR_CPREG32(DFSR):
+TVM_EMUL(regs, hsr, r, DFSR);
+break;
+case HSR_CPREG32(IFSR):
+TVM_EMUL(regs, hsr, r, IFSR);
+break;
+case HSR_CPREG32(DFAR):
+TVM_EMUL(regs, hsr, r, DFAR);
+break;
+case HSR_CPREG32(IFAR):
+TVM_EMUL(regs, hsr, r, IFAR);
+break;

[...]

+case HSR_CPREG32(ADFSR):
+TVM_EMUL(regs, hsr, r, ADFSR);
+break;
+case HSR_CPREG32(AIFSR):
+TVM_EMUL(regs, hsr, r, AIFSR);
+break;
+case HSR_CPREG32(MAIR0):
+TVM_EMUL(regs, hsr, r, MAIR0);
+break;
+case HSR_CPREG32(MAIR1):
+TVM_EMUL(regs, hsr, r, MAIR1);
+break;


Please mention that PRRR and NMRR are aliased to respectively MAIR0 and 
MAIR1. This will avoid to spend time trying to understanding why the 
spec says they are trapped but you don't "handle" them.



+case HSR_CPREG32(AMAIR0):
+TVM_EMUL(regs, hsr, r, AMAIR0);
+break;
+case HSR_CPREG32(AMAIR1):
+TVM_EMUL(regs, hsr, r, AMAIR1);
+break;
+case HSR_CPREG32(CONTEXTIDR):
+TVM_EMUL(regs, hsr, r, CONTEXTIDR);
+break;
+
+/*
   * !CNTHCTL_EL2.EL1PCEN / !CNTHCTL.PL1PCEN
   *
   * ARMv7 (DDI 0406C.b): B4.1.22
@@ -1853,6 +1915,13 @@ static void do_cp15_32(struct cpu_user_regs *regs,
  static void do_cp15_64(struct cpu_user_regs *regs,
 const union hsr hsr)
  {
+struct vcpu *v = current;
+const struct hsr_cp64 cp64 = hsr.cp64;
+sysreg64_t r = {
+.low32 = (uint32_t) get_user_reg(regs, cp64.reg1),


The cast is not necessary.


+.high32 = (uint32_t) get_user_reg(regs, cp64.reg2)


Ditto.


+};
+
  if ( !check_conditional_instr(regs, hsr) )
  {
  advance_pc(regs, hsr);
@@ -1862,6 +1931,19 @@ static void do_cp15_64(struct cpu_user_regs *regs,
  switch ( hsr.bits & HSR_CP64_REGS_MASK )
  {
  /*
+ * HCR_EL2.TVM / HCR.TVM
+ *
+ * ARMv7 (DDI 0406C.b): B1.14.13
+ * ARMv8 (DDI 0487A.e): D1-1569 Table D1-34


Same remarks as above for the spec version.


+ */
+case HSR_CPREG64(TTBR0):
+TVM_EMUL_VMEVT(v, regs, hsr, r.val64, TTBR0_64);
+break;
+case HSR_CPREG64(TTBR1):
+TVM_EMUL_VMEVT(v, regs, hsr, r.val64, TTBR1_64);
+break;
+
+/*
   * !CNTHCTL_EL2.EL1PCEN / !CNTHCTL.PL1PCEN
   *
   * ARMv7 (DDI 0406C.b): B4.1.22
@@ -1891,8 +1973,6 @@ static void do_cp15_64(struct cpu_user_regs *regs,
   */
  default:
  {
-const struct hsr_cp64 cp64 = hsr.cp64;
-
  gdprintk(XENLOG_ERR,
   "%s p15, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n",
   cp64.read ? "mrrc" : "mcrr",
@@ -2128,10 +2208,50 @@ static void 

Re: [Xen-devel] [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters

[Boris Ostrovsky]

On 06/16/2016 05:40 AM, Jan Beulich wrote:
> The IO-APIC address has variable bits determined by the PCI-to-ISA
> bridge, and the IO-APIC version should be read from the IO-APIC. (Note
> that there's still implicit rather than explicit agreement on the
> IO-APIC base address between qemu and the hypervisor.)

It probably doesn't matter now for PVHv2 since we are not going to
initially emulate IOAPIC but when/if we do, how do we query for base
address and version? Should we just rely on the same implicit agreement?

-boris

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

Re: [Xen-devel] [PATCH 4/7] vm-event/x86: use vm_event_vcpu_enter properly

[Tamas K Lengyel]

> diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
> index 1fec412..1e5445f 100644
> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -20,7 +20,6 @@
>   */
>
>  #include 
> -#include 
>
>  int arch_monitor_domctl_event(struct domain *d,
>struct xen_domctl_monitor_op *mop)
> @@ -62,14 +61,6 @@ int arch_monitor_domctl_event(struct domain *d,
>  else
>  ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask;
>
> -if ( VM_EVENT_X86_CR3 == mop->u.mov_to_cr.index )
> -{
> -struct vcpu *v;
> -/* Latches new CR3 mask through CR0 code. */
> -for_each_vcpu ( d, v )
> -hvm_update_guest_cr(v, 0);
> -}
> -

So this block is not really getting relocated as the commit message
suggests as much as being completely reworked at a different location?
It would be better for it to be it's own separate patch as the changes
are not trivial.

>  domain_unpause(d);
>
>  break;

Thanks,
Tamas

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

Re: [Xen-devel] [PATCH 3/7] vm-event: introduce vm_event_vcpu_enter

[Tamas K Lengyel]

On Thu, Jun 16, 2016 at 8:08 AM, Corneliu ZUZU  wrote:
> In an effort to improve on the vm-event interface, we introduce a new function
> called vm_event_vcpu_enter. Its significance is that of a "final touch" vCPU
> function - i.e. it should be called by implementing architectures just before
> re-entering vCPUs.
> On X86 for example, it is called on the scheduling tail (hvm_do_resume) and 
> just
> before reentering the guest world after a hypervisor trap 
> (vmx_vmenter_helper).
>

I don't think this belongs to the vm_event system. It should probably
be in the monitor subsystem as it has nothing to do with passing
messages, which is essentially what vm_event is for.

Tamas

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

Re: [Xen-devel] [PATCH 2/7] vm-event: VM_EVENT_FLAG_DENY requires VM_EVENT_FLAG_VCPU_PAUSED

[Tamas K Lengyel]

On Thu, Jun 16, 2016 at 8:07 AM, Corneliu ZUZU  wrote:
> For VM_EVENT_FLAG_DENY to work, the vcpu must be paused (sync = 1) until the
> vm-event is handled. A vm-event response having VM_EVENT_FLAG_DENY flag set
> should also set the VM_EVENT_FLAG_VCPU_PAUSED flag. Enforce that in
> vm_event_register_write_resume().

Well, the problem with this is that the user can set the VCPU_PAUSED
flag any time it wants. It can happen that Xen hasn't paused the vCPU
but the user still sends that flag, in which case the unpause the flag
induces will not actually do anything. You should also check if the
vCPU is in fact paused rather then just relying on this flag.

Tamas

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

Re: [Xen-devel] [PATCH 1/2] hvmloader: limit CPUs exposed to guests

[Jan Beulich]

>>> On 16.06.16 at 17:37,  wrote:
> On 06/16/2016 11:25 AM, Jan Beulich wrote:
> On 16.06.16 at 17:09,  wrote:
>>> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>>>
>>> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
>>> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
>>> @@ -150,6 +150,14 @@ int main(int argc, char **argv)
>>>  indent(); printf("MSU, 8\n");
>>>  pop_block();
>>>  
>>> +/* Processor object helpers. */
>>> +push_block("Method", "PMAT, 2");
>>> +push_block("If", "LLess(Arg0, NCPU)");
>>> +stmt("Return", "ToBuffer(Arg1)");
>>> +pop_block();
>>> +stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
>>>
>>>
>>> Could you explain what this is? (I suspect  this is related to MAT
>>> object and I don't think I understand what it is).
>> This is a helper routine for _MAT(), helping greatly to reduce
>> overall size of the DSDT. It checks whether the CPU is within the
>> range of available ones (online or offline), and if it isn't returns a
>> static buffer instead of data read from MADT (as it's the purpose
>> of this patch to remove these MADT entries for not present CPUs).
> 
> I meant just the last line (I understand what the routine is --- I am
> not clear where MAT format is defined).

That's a MADT entry of type ACPI_MADT_TYPE_LOCAL_APIC. Basically
an equivalent of the ToBuffer(MAT) used previously (and now still used
for CPU0).

Jan


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

Re: [Xen-devel] [PATCH 1/7] minor (formatting) fixes

[Tamas K Lengyel]

> diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
> index 014d9ba..05c3027 100644
> --- a/xen/include/asm-arm/vm_event.h
> +++ b/xen/include/asm-arm/vm_event.h
> @@ -23,21 +23,18 @@
>  #include 
>  #include 
>
> -static inline
> -int vm_event_init_domain(struct domain *d)
> +static inline int vm_event_init_domain(struct domain *d)
>  {
>  /* Nothing to do. */
>  return 0;
>  }
>
> -static inline
> -void vm_event_cleanup_domain(struct domain *d)
> +static inline void vm_event_cleanup_domain(struct domain *d)
>  {
>  memset(>monitor, 0, sizeof(d->monitor));
>  }
>
> -static inline
> -void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
> +static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu 
> *v)
>  {
>  /* Not supported on ARM. */
>  }
> @@ -59,6 +56,9 @@ static inline void vm_event_fill_regs(vm_event_request_t 
> *req)
>  /* Not supported on ARM. */
>  }
>
> +/*
> + * Monitor vm-events.
> + */

I already have an acked patch that relocates monitor-related functions
from here and the x86 header to the monitor subsystem
(https://patchwork.kernel.org/patch/913/). Generally, I'm trying
to keep monitor-related stuff in the appropriately named files, so if
you encounter things like this in the future the best course of action
is to relocate them. vm_event should be use-case neutral by not having
specific things for the monitor subsystem and just be the i/o system
used for passing messages.

>  static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d)
>  {
>  uint32_t capabilities = 0;

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

Re: [Xen-devel] [PATCH 1/2] hvmloader: limit CPUs exposed to guests

[Boris Ostrovsky]

On 06/16/2016 11:25 AM, Jan Beulich wrote:
 On 16.06.16 at 17:09,  wrote:
>> On 06/16/2016 05:40 AM, Jan Beulich wrote:
>>
>> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
>> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
>> @@ -150,6 +150,14 @@ int main(int argc, char **argv)
>>  indent(); printf("MSU, 8\n");
>>  pop_block();
>>  
>> +/* Processor object helpers. */
>> +push_block("Method", "PMAT, 2");
>> +push_block("If", "LLess(Arg0, NCPU)");
>> +stmt("Return", "ToBuffer(Arg1)");
>> +pop_block();
>> +stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
>>
>>
>> Could you explain what this is? (I suspect  this is related to MAT
>> object and I don't think I understand what it is).
> This is a helper routine for _MAT(), helping greatly to reduce
> overall size of the DSDT. It checks whether the CPU is within the
> range of available ones (online or offline), and if it isn't returns a
> static buffer instead of data read from MADT (as it's the purpose
> of this patch to remove these MADT entries for not present CPUs).

I meant just the last line (I understand what the routine is --- I am
not clear where MAT format is defined).

-boris

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

Re: [Xen-devel] HEADS UP: Imported Xen 4.7: no blkback

[Roger Pau Monné]

On Mon, Jun 13, 2016 at 07:26:30PM +, Marcin Cieslak wrote:
> On Mon, 13 Jun 2016, Roger Pau Monné wrote:
> 
> > On Fri, Jun 10, 2016 at 09:38:59PM +, Marcin Cieslak wrote:
> > > "?" does not work - it mostly causes a panic, the console is slow, but I 
> > > managed
> > > to switch it to /dev/ada0p2, dmesg below:
> > 
> > This has now been reverted, so when I import the new RC this should be 
> > fixed 
> > and you won't need to change anything.
> 
> I am confused now - so with a new Xen kernel (not yet in ports) I can use
> /dev/xbd* devices again? They are in fact missing - xbd driver says 
> "attaching as ada0"
> and I can mount it only as /dev/ada
> 
> > It seems like Windows PV drivers don't attach at all, or are you running 
> > Windows without the PV drivers?
> 
> Yes, I have. Those Windows partitions used to work properly without changes
> under xen 4.5. But we are too early - the problem is that even ovmf
> does not se them drives now, this is before Windows boots.
> 
> > Since you mention that the console is very slow, if you run 'top' on Dom0, 
> > do you see any process (eg: qemu) taking a lot of CPU time?
> 
> Yes,
> 
>  1635 root  7 1000   241M   101M RUN  7:16  91.14% 
> qemu-system-i386
> 
> or more
> 
> > Also, do you see Dom0 consuming a lot of CPU if you run 'xentop'?
> 
>   Domain-0 -r446  100.04194300   25.2   no limit   n/a
>  10000000  0  
> 00
> 

Hello,

I've just imported Xen 4.7.0-rc6 into the ports tree, could you give it a 
try when you have a moment?

FWIW, I'm going on vacations until the 4th of July, and I will be mostly 
AFK.

Roger.

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

Re: [Xen-devel] [PATCH 1/2] hvmloader: limit CPUs exposed to guests

[Jan Beulich]

>>> On 16.06.16 at 17:09,  wrote:
> On 06/16/2016 05:40 AM, Jan Beulich wrote:
> 
> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> @@ -150,6 +150,14 @@ int main(int argc, char **argv)
>  indent(); printf("MSU, 8\n");
>  pop_block();
>  
> +/* Processor object helpers. */
> +push_block("Method", "PMAT, 2");
> +push_block("If", "LLess(Arg0, NCPU)");
> +stmt("Return", "ToBuffer(Arg1)");
> +pop_block();
> +stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
> 
> 
> Could you explain what this is? (I suspect  this is related to MAT
> object and I don't think I understand what it is).

This is a helper routine for _MAT(), helping greatly to reduce
overall size of the DSDT. It checks whether the CPU is within the
range of available ones (online or offline), and if it isn't returns a
static buffer instead of data read from MADT (as it's the purpose
of this patch to remove these MADT entries for not present CPUs).

Jan


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

Re: [Xen-devel] [PATCH 6/7] vm-event/arm: move hvm_event_cr->common vm_event_monitor_cr

[Jan Beulich]

>>> On 16.06.16 at 16:12,  wrote:
> Prepare for ARM implementation of control-register write vm-events by moving
> X86-specific hvm_event_cr to the common-side.
> 
> Signed-off-by: Corneliu ZUZU 
> ---
>  xen/arch/x86/hvm/event.c| 30 --
>  xen/arch/x86/hvm/hvm.c  |  2 +-
>  xen/arch/x86/monitor.c  | 37 -
>  xen/arch/x86/vm_event.c |  2 +-
>  xen/common/monitor.c| 40 
>  xen/common/vm_event.c   | 31 +++
>  xen/include/asm-x86/hvm/event.h | 13 -
>  xen/include/asm-x86/monitor.h   |  2 --
>  xen/include/xen/monitor.h   |  4 
>  xen/include/xen/vm_event.h  | 10 ++
>  10 files changed, 91 insertions(+), 80 deletions(-)

Considering that there's no ARM file getting altered here at all,
mentioning ARM in the subject is a little odd.

> --- a/xen/common/monitor.c
> +++ b/xen/common/monitor.c
> @@ -62,6 +62,46 @@ int monitor_domctl(struct domain *d, struct 
> xen_domctl_monitor_op *mop)
>  
>  switch ( mop->event )
>  {
> +#if CONFIG_X86

#ifdef please.

> +case XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG:
> +{
> +struct arch_domain *ad = >arch;

Peeking into the next patch I see that this stays there. Common code,
however, shouldn't access ->arch sub-structures - respective fields
should be moved out.

And looking at all the uses of this variable I get the impression that
you really want a shorthand for >arch.monitor (if any such
helper variable is worthwhile to have here in the first place).

> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -24,8 +24,6 @@
>  
>  #include 
>  
> -#define monitor_ctrlreg_bitmask(ctrlreg_index) (1U << (ctrlreg_index))
> -
>  static inline
>  int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op 
> *mop)
>  {
> --- a/xen/include/xen/monitor.h
> +++ b/xen/include/xen/monitor.h
> @@ -25,6 +25,10 @@
>  struct domain;
>  struct xen_domctl_monitor_op;
>  
> +#if CONFIG_X86
> +#define monitor_ctrlreg_bitmask(ctrlreg_index) (1U << (ctrlreg_index))
> +#endif

What's the point in removing this from the x86 header if then it
needs to be put in such a conditional? If the conditional gets
dropped in the next patch, then I think you have two options:
Leave it where it is here, and move it there. Or move it here,
but omit the conditional right away - I can't see this definition
being present to harm the ARM build in any way.

> --- a/xen/include/xen/vm_event.h
> +++ b/xen/include/xen/vm_event.h
> @@ -96,6 +96,16 @@ void vm_event_vcpu_unpause(struct vcpu *v);
>  int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
> vm_event_request_t *req);
>  
> +#if CONFIG_X86
> +/*
> + * Called for the current vCPU on control-register changes by guest.
> + * The event might not fire if the client has subscribed to it in 
> onchangeonly
> + * mode, hence the bool_t return type for control register write events.
> + */
> +bool_t vm_event_monitor_cr(unsigned int index, unsigned long value,
> +   unsigned long old);
> +#endif

Same goes for the declaration here.

Jan


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

Re: [Xen-devel] [PATCH 1/2] hvmloader: limit CPUs exposed to guests

[Boris Ostrovsky]

On 06/16/2016 05:40 AM, Jan Beulich wrote:

--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
@@ -150,6 +150,14 @@ int main(int argc, char **argv)
 indent(); printf("MSU, 8\n");
 pop_block();
 
+/* Processor object helpers. */
+push_block("Method", "PMAT, 2");
+push_block("If", "LLess(Arg0, NCPU)");
+stmt("Return", "ToBuffer(Arg1)");
+pop_block();
+stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");


Could you explain what this is? (I suspect  this is related to MAT
object and I don't think I understand what it is).

-boris



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

Re: [Xen-devel] [PATCH 4/7] vm-event/x86: use vm_event_vcpu_enter properly

[Jan Beulich]

>>> On 16.06.16 at 16:09,  wrote:
> @@ -2199,7 +2153,9 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer)
>  
>  if ( hvm_event_crX(CR0, value, old_value) )
>  {
> -/* The actual write will occur in hvm_do_resume(), if permitted. 
> */
> +/* The actual write will occur in vcpu_enter_write_data(), if
> + * permitted.
> + */

Coding style.

> @@ -1432,18 +1430,16 @@ static void vmx_update_guest_cr(struct vcpu *v, 
> unsigned int cr)
>  if ( paging_mode_hap(v->domain) )
>  {
>  /* Manage GUEST_CR3 when CR0.PE=0. */
> +uint32_t old_ctls = v->arch.hvm_vmx.exec_control;
>  uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
>   CPU_BASED_CR3_STORE_EXITING);
> +
>  v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
>  if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
>  v->arch.hvm_vmx.exec_control |= cr3_ctls;
>  
> -/* Trap CR3 updates if CR3 memory events are enabled. */
> -if ( v->domain->arch.monitor.write_ctrlreg_enabled &
> - monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
> -v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;
> -
> -vmx_update_cpu_exec_control(v);
> +if ( old_ctls != v->arch.hvm_vmx.exec_control )
> +vmx_update_cpu_exec_control(v);
>  }

How does this match up with the rest of this patch?

> @@ -179,6 +182,105 @@ void vm_event_fill_regs(vm_event_request_t *req)
>  req->data.regs.x86.cs_arbytes = seg.attr.bytes;
>  }
>  
> +static inline void vcpu_enter_write_data(struct vcpu *v)

Please allow the compiler to decide whether to inline such larger
functions.

> +void arch_vm_event_vcpu_enter(struct vcpu *v)
> +{
> +/* vmx only */
> +ASSERT( cpu_has_vmx );

Stray blanks.

Jan


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

Re: [Xen-devel] [PATCH v2 2/2] xen: add update indicator to vcpu_runstate_info

[Juergen Gross]

On 16/06/16 14:36, Jan Beulich wrote:
 On 16.06.16 at 14:11,  wrote:
>> On 16/06/16 13:15, Jan Beulich wrote:
>> On 15.06.16 at 13:34,  wrote:
 +if ( VM_ASSIST(v->domain, runstate_update_flag) )
 +{
 +off = offsetof(struct vcpu_runstate_info, state_entry_time) +
 +  sizeof(v->runstate.state_entry_time) - 1;
 +if ( has_32bit_shinfo(v->domain) )
 +{
 +guest_handle = v->runstate_guest.compat.p;
 +guest_handle +=
 +offsetof(struct compat_vcpu_runstate_info, 
 state_entry_time) +
 +sizeof(v->runstate.state_entry_time) - 1;
>>>
>>> The sizes of the native and compat fields happen to be the same,
>>> but it would be nice if the right field/type could be used here.
>>
>> Hmm, this will require some ugly type casting, but it is probably
>> cleaner.
> 
> Type casting? I would expect you to be able to use
> v->runstate_guest.compat.p->state_entry_time. In fact I think
> you also could get rid of the offsetof() if you used
> >runstate_guest.compat.p->state_entry_time for initializing
> guest_handle.

Aah, of course. This can then be simplified even more:

if ( VM_ASSIST(v->domain, runstate_update_flag) )
{
guest_handle = has_32bit_shinfo(v->domain)
? >runstate_guest.compat.p->state_entry_time + 1
: >runstate_guest.native.p->state_entry_time + 1;
guest_handle--;
v->runstate.state_entry_time |= XEN_RUNSTATE_UPDATE;
__raw_copy_to_guest(guest_handle,
 (void *)(>runstate.state_entry_time + 1) - 1, 1);
smp_wmb();
}

This will reduce code size by 25 bytes. :-)


Juergen

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

Re: [Xen-devel] [Qemu-devel] [PATCH v7 11/15] qapi: Change Netdev into a flat union

[Eric Blake]

On 06/16/2016 07:15 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> From: Kővágó, Zoltán 
>>
>> Except qapi-schema.json, this patch was generated by:
>>
>> find . -name .git -prune -o -type f \! -name '*~' -print0 | \
>>   xargs -0 sed -i \
>> -e 's/NetClientOptionsKind/NetClientDriver/g' \
>> -e 's/NET_CLIENT_OPTIONS_KIND_/NET_CLIENT_DRIVER_/g' \
>> -e 's/netdev->opts/netdev/g'
> 
> Uh, this is prone to descend into build trees and edit random crap.  I
> used
> 
> $ sed -i -e ... `git-ls-tree -r HEAD | awk '$2 == "blob" { print $4 }'`
> 
> to verify this commit.  Differences noted inline.
> 

> I additionally get:
> 
>   diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
>   index 692283f..825f0ba 100644
>   --- a/hw/net/e1000e.c
>   +++ b/hw/net/e1000e.c
>   @@ -226,7 +226,7 @@ e1000e_set_link_status(NetClientState *nc)
>}
> 
>static NetClientInfo net_e1000e_info = {
>   -.type = NET_CLIENT_OPTIONS_KIND_NIC,
>   +.type = NET_CLIENT_DRIVER_NIC,
>.size = sizeof(NICState),
>.can_receive = e1000e_nc_can_receive,
>.receive = e1000e_nc_receive,
> 
> Rebase needed?

Unfortunately yes. This patch has been under a LOT of churn since it was
first written; it may be better to just redo it from scratch and claim
ownership myself, since it hardly resembles Zoltán's original submission
(but of course, give him credit for the idea).

> 
>> index f8a500f..89a149b 100644
>> --- a/net/dump.c
>> +++ b/net/dump.c
>> @@ -172,7 +172,7 @@ static void dumpclient_cleanup(NetClientState *nc)
>>  }
>>
>>  static NetClientInfo net_dump_info = {
>> -.type = NET_CLIENT_OPTIONS_KIND_DUMP,
>> +.type = NET_CLIENT_DRIVER_DUMP,
>>  .size = sizeof(DumpNetClient),
>>  .receive = dumpclient_receive,
>>  .receive_iov = dumpclient_receive_iov,
>> @@ -189,8 +189,8 @@ int net_init_dump(const Netdev *netdev, const char *name,
>>  NetClientState *nc;
>>  DumpNetClient *dnc;
>>
>> -assert(netdev->opts->type == NET_CLIENT_OPTIONS_KIND_DUMP);
>> -dump = netdev->opts->u.dump.data;
>> +assert(netdev->type == NET_CLIENT_DRIVER_DUMP);
>> +dump = >u.dump;
> 
> sed turns this into
> 
>dump = netdev->u.dump.data;
> 
> Is this part of the manual changes?  More of the same below.
> 

The original sed script is so distant from the actual changes that it's
not worth mentioning the sed script in the commit message any more.

>>
>>  assert(peer);
>>
> 
> Another possible case of "rebase needed":
> 
>   diff --git a/net/filter.c b/net/filter.c
>   index 8ac79f3..888fe6d 100644
>   --- a/net/filter.c
>   +++ b/net/filter.c
>   @@ -201,7 +201,7 @@ static void netfilter_complete(UserCreatable *uc, Error 
> **errp)
>}
> 
>queues = qemu_find_net_clients_except(nf->netdev_id, ncs,
>   -  NET_CLIENT_OPTIONS_KIND_NIC,
>   +  NET_CLIENT_DRIVER_NIC,
>  MAX_QUEUE_NUM);

Yep.  Thanks for researching.

>>
>>  static int net_client_init1(const void *object, int is_netdev, Error **errp)
>>  {
> 
> Multiple differences in this function.  Manual?

Yes.


>> @@ -47,7 +47,7 @@ static void vhost_user_stop(int queues, NetClientState 
>> *ncs[])
>>  int i;
>>
>>  for (i = 0; i < queues; i++) {
>> -assert (ncs[i]->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
>> +assert(ncs[i]->info->type == NET_CLIENT_DRIVER_VHOST_USER);
> 
> Manual whitespace cleanup.  Okay.
> 

And necessary to shut up checkpatch.  I really get to rewrite the commit
message to something better for v8, don't I.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 7/7] vm-event/arm: implement support for control-register write vm-events

[Julien Grall]

Hello Corneliu,

On 16/06/16 15:13, Corneliu ZUZU wrote:

Add ARM support for control-register write monitoring through the vm-events
subsystem.

Chosen ARM system control-registers that can be monitored are:
 - VM_EVENT_ARM_SCTLR:  AArch32 SCTLR, AArch64 SCTLR_EL1
 - VM_EVENT_ARM_TTBR{0,1}:  AArch32 TTBR{0,1}, AArch64 TTBR{0,1}_EL1
 - VM_EVENT_ARM_TTBCR:  AArch32 TTBCR, AArch64 TCR_EL1

Trapping of write operations of these registers was attained by setting the
HCR_EL2.TVM / HCR.TVM bit.

Signed-off-by: Corneliu ZUZU 
---
  MAINTAINERS|   1 +
  xen/arch/arm/Makefile  |   1 +
  xen/arch/arm/traps.c   | 126 +++-
  xen/arch/arm/vm_event.c| 112 ++
  xen/common/monitor.c   |   2 -
  xen/common/vm_event.c  |   2 -
  xen/include/asm-arm/domain.h   |  30 +
  xen/include/asm-arm/traps.h| 253 +
  xen/include/asm-arm/vm_event.h |  22 +++-
  xen/include/public/vm_event.h  |   8 +-
  xen/include/xen/monitor.h  |   2 -
  xen/include/xen/vm_event.h |   2 -
  12 files changed, 543 insertions(+), 18 deletions(-)


I think this patch would benefit to be split in multiple patches to ease 
the review and also describe the infrastructure you have introduced 
(TVM_* & co).


I will review in detail later.

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 1/7] minor (formatting) fixes

[Jan Beulich]

>>> On 16.06.16 at 16:06,  wrote:
> --- a/xen/arch/x86/hvm/event.c
> +++ b/xen/arch/x86/hvm/event.c
> @@ -23,6 +23,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/xen/common/monitor.c b/xen/common/monitor.c
> index d950a7c..b30857a 100644
> --- a/xen/common/monitor.c
> +++ b/xen/common/monitor.c
> @@ -22,7 +22,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 

These two adjustments clearly don't fit title / description. I certainly
don't mind unnecessary inclusions to be dropped, but the addition of
one clearly needs explanation (after all thing build fine without it).

> --- a/xen/include/asm-x86/vm_event.h
> +++ b/xen/include/asm-x86/vm_event.h
> @@ -44,6 +44,9 @@ void vm_event_set_registers(struct vcpu *v, 
> vm_event_response_t *rsp);
>  
>  void vm_event_fill_regs(vm_event_request_t *req);
>  
> +/*
> + * Monitor vm-events.
> + */

This is a single line comment (also elsewhere).

> --- a/xen/include/xen/vm_event.h
> +++ b/xen/include/xen/vm_event.h
> @@ -85,7 +85,6 @@ void vm_event_monitor_guest_request(void);
>  
>  #endif /* __VM_EVENT_H__ */
>  
> -
>  /*
>   * Local variables:
>   * mode: C

Why don't you remove the other stray blank line at once?

Jan


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

[Xen-devel] [PATCH 5/7] x86: replace monitor_write_data.do_write with enum

[Corneliu ZUZU]

After trapping a control-register write vm-event and -until- deciding if that
write is to be permitted or not (VM_EVENT_FLAG_DENY) and doing the actual write,
there cannot and should not be another trapped control-register write event.
That is: currently -only one- of the fields of monitor_write_data.do_write can
be true at any given moment and therefore it would be more appropriate to
replace those fields with an enum value.

Signed-off-by: Corneliu ZUZU 
---
 xen/arch/x86/hvm/hvm.c   | 18 -
 xen/arch/x86/vm_event.c  | 63 ++--
 xen/include/asm-x86/domain.h | 20 +++---
 3 files changed, 41 insertions(+), 60 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 2f48846..4596662 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2156,8 +2156,9 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer)
 /* The actual write will occur in vcpu_enter_write_data(), if
  * permitted.
  */
-v->arch.vm_event->write_data.do_write.cr0 = 1;
-v->arch.vm_event->write_data.cr0 = value;
+ASSERT(MWS_NOWRITE == v->arch.vm_event->write_data.status);
+v->arch.vm_event->write_data.status = MWS_CR0;
+v->arch.vm_event->write_data.value = value;
 
 return X86EMUL_OKAY;
 }
@@ -2260,8 +2261,9 @@ int hvm_set_cr3(unsigned long value, bool_t may_defer)
 /* The actual write will occur in vcpu_enter_write_data(), if
  * permitted.
  */
-v->arch.vm_event->write_data.do_write.cr3 = 1;
-v->arch.vm_event->write_data.cr3 = value;
+ASSERT(MWS_NOWRITE == v->arch.vm_event->write_data.status);
+v->arch.vm_event->write_data.status = MWS_CR3;
+v->arch.vm_event->write_data.value = value;
 
 return X86EMUL_OKAY;
 }
@@ -2342,8 +2344,9 @@ int hvm_set_cr4(unsigned long value, bool_t may_defer)
 /* The actual write will occur in vcpu_enter_write_data(), if
  * permitted.
  */
-v->arch.vm_event->write_data.do_write.cr4 = 1;
-v->arch.vm_event->write_data.cr4 = value;
+ASSERT(MWS_NOWRITE == v->arch.vm_event->write_data.status);
+v->arch.vm_event->write_data.status = MWS_CR4;
+v->arch.vm_event->write_data.value = value;
 
 return X86EMUL_OKAY;
 }
@@ -3724,7 +3727,8 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t 
msr_content,
 /* The actual write will occur in vcpu_enter_write_data(), if
  * permitted.
  */
-v->arch.vm_event->write_data.do_write.msr = 1;
+ASSERT(MWS_NOWRITE == v->arch.vm_event->write_data.status);
+v->arch.vm_event->write_data.status = MWS_MSR;
 v->arch.vm_event->write_data.msr = msr;
 v->arch.vm_event->write_data.value = msr_content;
 
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index 94b50fc..94342d5 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -74,30 +74,8 @@ void vm_event_register_write_resume(struct vcpu *v, 
vm_event_response_t *rsp)
 if ( (rsp->flags & VM_EVENT_FLAG_DENY) &&
  (rsp->flags & VM_EVENT_FLAG_VCPU_PAUSED) )
 {
-struct monitor_write_data *w = >arch.vm_event->write_data;
-
-ASSERT(w);
-
-switch ( rsp->reason )
-{
-case VM_EVENT_REASON_MOV_TO_MSR:
-w->do_write.msr = 0;
-break;
-case VM_EVENT_REASON_WRITE_CTRLREG:
-switch ( rsp->u.write_ctrlreg.index )
-{
-case VM_EVENT_X86_CR0:
-w->do_write.cr0 = 0;
-break;
-case VM_EVENT_X86_CR3:
-w->do_write.cr3 = 0;
-break;
-case VM_EVENT_X86_CR4:
-w->do_write.cr4 = 0;
-break;
-}
-break;
-}
+ASSERT(v->arch.vm_event);
+v->arch.vm_event->write_data.status = MWS_NOWRITE;
 }
 }
 
@@ -208,29 +186,28 @@ static inline void vcpu_enter_write_data(struct vcpu *v)
 v->arch.vm_event->emulate_flags = 0;
 }
 
-if ( w->do_write.msr )
-{
-hvm_msr_write_intercept(w->msr, w->value, 0);
-w->do_write.msr = 0;
-}
-
-if ( w->do_write.cr0 )
-{
-hvm_set_cr0(w->cr0, 0);
-w->do_write.cr0 = 0;
-}
+if ( likely(MWS_NOWRITE == w->status) )
+return;
 
-if ( w->do_write.cr4 )
+switch ( w->status )
 {
-hvm_set_cr4(w->cr4, 0);
-w->do_write.cr4 = 0;
+case MWS_MSR:
+hvm_msr_write_intercept(w->msr, w->value, 0);
+break;
+case MWS_CR0:
+hvm_set_cr0(w->value, 0);
+break;
+case MWS_CR3:
+hvm_set_cr3(w->value, 0);
+break;
+case MWS_CR4:
+hvm_set_cr4(w->value, 0);
+

[Xen-devel] [PATCH 2/7] vm-event: VM_EVENT_FLAG_DENY requires VM_EVENT_FLAG_VCPU_PAUSED

[Corneliu ZUZU]

For VM_EVENT_FLAG_DENY to work, the vcpu must be paused (sync = 1) until the
vm-event is handled. A vm-event response having VM_EVENT_FLAG_DENY flag set
should also set the VM_EVENT_FLAG_VCPU_PAUSED flag. Enforce that in
vm_event_register_write_resume().

Signed-off-by: Corneliu ZUZU 
---
 xen/arch/x86/vm_event.c   | 3 ++-
 xen/include/public/vm_event.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index 5635603..75647c4 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -70,7 +70,8 @@ void vm_event_toggle_singlestep(struct domain *d, struct vcpu 
*v)
 
 void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
 {
-if ( rsp->flags & VM_EVENT_FLAG_DENY )
+if ( (rsp->flags & VM_EVENT_FLAG_DENY) &&
+ (rsp->flags & VM_EVENT_FLAG_VCPU_PAUSED) )
 {
 struct monitor_write_data *w = >arch.vm_event->write_data;
 
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 586f43b..8f94e20 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -77,6 +77,7 @@
 /*
  * Deny completion of the operation that triggered the event.
  * Currently only useful for MSR and control-register write events.
+ * Requires the vCPU to be paused already (synchronous events only).
  */
 #define VM_EVENT_FLAG_DENY   (1 << 6)
 /*
-- 
2.5.0


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

[Xen-devel] [PATCH 7/7] vm-event/arm: implement support for control-register write vm-events

[Corneliu ZUZU]

Add ARM support for control-register write monitoring through the vm-events
subsystem.

Chosen ARM system control-registers that can be monitored are:
- VM_EVENT_ARM_SCTLR:  AArch32 SCTLR, AArch64 SCTLR_EL1
- VM_EVENT_ARM_TTBR{0,1}:  AArch32 TTBR{0,1}, AArch64 TTBR{0,1}_EL1
- VM_EVENT_ARM_TTBCR:  AArch32 TTBCR, AArch64 TCR_EL1

Trapping of write operations of these registers was attained by setting the
HCR_EL2.TVM / HCR.TVM bit.

Signed-off-by: Corneliu ZUZU 
---
 MAINTAINERS|   1 +
 xen/arch/arm/Makefile  |   1 +
 xen/arch/arm/traps.c   | 126 +++-
 xen/arch/arm/vm_event.c| 112 ++
 xen/common/monitor.c   |   2 -
 xen/common/vm_event.c  |   2 -
 xen/include/asm-arm/domain.h   |  30 +
 xen/include/asm-arm/traps.h| 253 +
 xen/include/asm-arm/vm_event.h |  22 +++-
 xen/include/public/vm_event.h  |   8 +-
 xen/include/xen/monitor.h  |   2 -
 xen/include/xen/vm_event.h |   2 -
 12 files changed, 543 insertions(+), 18 deletions(-)
 create mode 100644 xen/arch/arm/vm_event.c
 create mode 100644 xen/include/asm-arm/traps.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 9a224d4..634f359 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -402,6 +402,7 @@ M:  Tamas K Lengyel 
 S: Supported
 F: xen/common/mem_access.c
 F: xen/*/vm_event.c
+F: xen/arch/*/vm_event.c
 F: xen/*/monitor.c
 F: xen/include/*/mem_access.h
 F: xen/include/*/monitor.h
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 9e38da3..390df0a 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -40,6 +40,7 @@ obj-y += device.o
 obj-y += decode.o
 obj-y += processor.o
 obj-y += smc.o
+obj-y += vm_event.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o
 
 #obj-bin-y += o
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 8c50685..af61ac3 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "decode.h"
 #include "vtimer.h"
@@ -124,7 +125,12 @@ void init_traps(void)
 WRITE_SYSREG((HCPTR_CP_MASK & ~(HCPTR_CP(10) | HCPTR_CP(11))) | HCPTR_TTA,
  CPTR_EL2);
 
-/* Setup hypervisor traps */
+/* Setup hypervisor traps
+ *
+ * Note: HCR_TVM bit is also set for system-register write monitoring
+ * purposes (see vm_event_monitor_cr), but (for performance reasons) that's
+ * done selectively (see vcpu_enter_adjust_traps).
+ */
 WRITE_SYSREG(HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
  HCR_TWE|HCR_TWI|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB,
  HCR_EL2);
@@ -1720,6 +1726,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
 const struct hsr_cp32 cp32 = hsr.cp32;
 int regidx = cp32.reg;
 struct vcpu *v = current;
+register_t r = get_user_reg(regs, regidx);
 
 if ( !check_conditional_instr(regs, hsr) )
 {
@@ -1730,6 +1737,61 @@ static void do_cp15_32(struct cpu_user_regs *regs,
 switch ( hsr.bits & HSR_CP32_REGS_MASK )
 {
 /*
+ * HCR_EL2.TVM / HCR.TVM
+ *
+ * ARMv7 (DDI 0406C.b): B1.14.13
+ * ARMv8 (DDI 0487A.e): D1-1569 Table D1-34
+ */
+case HSR_CPREG32(SCTLR):
+TVM_EMUL_VMEVT(v, regs, hsr, r, SCTLR);
+break;
+case HSR_CPREG32(TTBR0_32):
+TVM_EMUL_VMEVT(v, regs, hsr, r, TTBR0_32);
+break;
+case HSR_CPREG32(TTBR1_32):
+TVM_EMUL_VMEVT(v, regs, hsr, r, TTBR1_32);
+break;
+case HSR_CPREG32(TTBCR):
+TVM_EMUL_VMEVT(v, regs, hsr, r, TTBCR);
+break;
+case HSR_CPREG32(DACR):
+TVM_EMUL(regs, hsr, r, DACR);
+break;
+case HSR_CPREG32(DFSR):
+TVM_EMUL(regs, hsr, r, DFSR);
+break;
+case HSR_CPREG32(IFSR):
+TVM_EMUL(regs, hsr, r, IFSR);
+break;
+case HSR_CPREG32(DFAR):
+TVM_EMUL(regs, hsr, r, DFAR);
+break;
+case HSR_CPREG32(IFAR):
+TVM_EMUL(regs, hsr, r, IFAR);
+break;
+case HSR_CPREG32(ADFSR):
+TVM_EMUL(regs, hsr, r, ADFSR);
+break;
+case HSR_CPREG32(AIFSR):
+TVM_EMUL(regs, hsr, r, AIFSR);
+break;
+case HSR_CPREG32(MAIR0):
+TVM_EMUL(regs, hsr, r, MAIR0);
+break;
+case HSR_CPREG32(MAIR1):
+TVM_EMUL(regs, hsr, r, MAIR1);
+break;
+case HSR_CPREG32(AMAIR0):
+TVM_EMUL(regs, hsr, r, AMAIR0);
+break;
+case HSR_CPREG32(AMAIR1):
+TVM_EMUL(regs, hsr, r, AMAIR1);
+break;
+case HSR_CPREG32(CONTEXTIDR):
+TVM_EMUL(regs, hsr, r, CONTEXTIDR);
+break;
+
+/*
  * !CNTHCTL_EL2.EL1PCEN / !CNTHCTL.PL1PCEN
  *
  * ARMv7 (DDI 0406C.b): B4.1.22
@@ -1853,6 +1915,13 @@ static void do_cp15_32(struct cpu_user_regs *regs,
 static void do_cp15_64(struct cpu_user_regs *regs,
 

[Xen-devel] [PATCH 6/7] vm-event/arm: move hvm_event_cr->common vm_event_monitor_cr

[Corneliu ZUZU]

Prepare for ARM implementation of control-register write vm-events by moving
X86-specific hvm_event_cr to the common-side.

Signed-off-by: Corneliu ZUZU 
---
 xen/arch/x86/hvm/event.c| 30 --
 xen/arch/x86/hvm/hvm.c  |  2 +-
 xen/arch/x86/monitor.c  | 37 -
 xen/arch/x86/vm_event.c |  2 +-
 xen/common/monitor.c| 40 
 xen/common/vm_event.c   | 31 +++
 xen/include/asm-x86/hvm/event.h | 13 -
 xen/include/asm-x86/monitor.h   |  2 --
 xen/include/xen/monitor.h   |  4 
 xen/include/xen/vm_event.h  | 10 ++
 10 files changed, 91 insertions(+), 80 deletions(-)

diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 26165b4..e8175e4 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -21,38 +21,8 @@
  * this program; If not, see .
  */
 
-#include 
 #include 
 #include 
-#include 
-#include 
-
-bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
-{
-struct vcpu *curr = current;
-struct arch_domain *ad = >domain->arch;
-unsigned int ctrlreg_bitmask = monitor_ctrlreg_bitmask(index);
-
-if ( (ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask) &&
- (!(ad->monitor.write_ctrlreg_onchangeonly & ctrlreg_bitmask) ||
-  value != old) )
-{
-bool_t sync = !!(ad->monitor.write_ctrlreg_sync & ctrlreg_bitmask);
-
-vm_event_request_t req = {
-.reason = VM_EVENT_REASON_WRITE_CTRLREG,
-.vcpu_id = curr->vcpu_id,
-.u.write_ctrlreg.index = index,
-.u.write_ctrlreg.new_value = value,
-.u.write_ctrlreg.old_value = old
-};
-
-vm_event_monitor_traps(curr, sync, );
-return 1;
-}
-
-return 0;
-}
 
 void hvm_event_msr(unsigned int msr, uint64_t value)
 {
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4596662..26f8625 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -52,7 +53,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 1e5445f..264f0fc 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -29,43 +29,6 @@ int arch_monitor_domctl_event(struct domain *d,
 
 switch ( mop->event )
 {
-case XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG:
-{
-unsigned int ctrlreg_bitmask;
-bool_t old_status;
-
-/* sanity check: avoid left-shift undefined behavior */
-if ( unlikely(mop->u.mov_to_cr.index > 31) )
-return -EINVAL;
-
-ctrlreg_bitmask = monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
-old_status = !!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
-
-if ( unlikely(old_status == requested_status) )
-return -EEXIST;
-
-domain_pause(d);
-
-if ( mop->u.mov_to_cr.sync )
-ad->monitor.write_ctrlreg_sync |= ctrlreg_bitmask;
-else
-ad->monitor.write_ctrlreg_sync &= ~ctrlreg_bitmask;
-
-if ( mop->u.mov_to_cr.onchangeonly )
-ad->monitor.write_ctrlreg_onchangeonly |= ctrlreg_bitmask;
-else
-ad->monitor.write_ctrlreg_onchangeonly &= ~ctrlreg_bitmask;
-
-if ( requested_status )
-ad->monitor.write_ctrlreg_enabled |= ctrlreg_bitmask;
-else
-ad->monitor.write_ctrlreg_enabled &= ~ctrlreg_bitmask;
-
-domain_unpause(d);
-
-break;
-}
-
 case XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR:
 {
 bool_t old_status = ad->monitor.mov_to_msr_enabled;
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index 94342d5..aa65840 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -19,7 +19,7 @@
  */
 
 #include 
-#include 
+#include 
 #include 
 #include 
 
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index c46df5a..2366bae 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -62,6 +62,46 @@ int monitor_domctl(struct domain *d, struct 
xen_domctl_monitor_op *mop)
 
 switch ( mop->event )
 {
+#if CONFIG_X86
+case XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG:
+{
+struct arch_domain *ad = >arch;
+unsigned int ctrlreg_bitmask;
+bool_t old_status;
+
+/* sanity check: avoid left-shift undefined behavior */
+if ( unlikely(mop->u.mov_to_cr.index > 31) )
+return -EINVAL;
+
+ctrlreg_bitmask = monitor_ctrlreg_bitmask(mop->u.mov_to_cr.index);
+old_status = !!(ad->monitor.write_ctrlreg_enabled & ctrlreg_bitmask);
+
+if ( unlikely(old_status == requested_status) )
+return -EEXIST;
+
+

[Xen-devel] [PATCH 4/7] vm-event/x86: use vm_event_vcpu_enter properly

[Corneliu ZUZU]

After introducing vm_event_vcpu_enter, it makes sense to move the following
code there:
- handling of monitor_write_data from hvm_do_resume
- enabling/disabling CPU_BASED_CR3_LOAD_EXITING from vmx_update_guest_cr(v, 0)

Signed-off-by: Corneliu ZUZU 
---
 xen/arch/x86/hvm/hvm.c |  62 +
 xen/arch/x86/hvm/vmx/vmx.c |  12 ++---
 xen/arch/x86/monitor.c |   9 
 xen/arch/x86/vm_event.c| 102 +
 xen/include/asm-x86/vm_event.h |   5 +-
 5 files changed, 119 insertions(+), 71 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 770bb50..2f48846 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -462,52 +462,6 @@ void hvm_do_resume(struct vcpu *v)
 if ( !handle_hvm_io_completion(v) )
 return;
 
-if ( unlikely(v->arch.vm_event) )
-{
-struct monitor_write_data *w = >arch.vm_event->write_data;
-
-if ( v->arch.vm_event->emulate_flags )
-{
-enum emul_kind kind = EMUL_KIND_NORMAL;
-
-if ( v->arch.vm_event->emulate_flags &
- VM_EVENT_FLAG_SET_EMUL_READ_DATA )
-kind = EMUL_KIND_SET_CONTEXT;
-else if ( v->arch.vm_event->emulate_flags &
-  VM_EVENT_FLAG_EMULATE_NOWRITE )
-kind = EMUL_KIND_NOWRITE;
-
-hvm_mem_access_emulate_one(kind, TRAP_invalid_op,
-   HVM_DELIVER_NO_ERROR_CODE);
-
-v->arch.vm_event->emulate_flags = 0;
-}
-
-if ( w->do_write.msr )
-{
-hvm_msr_write_intercept(w->msr, w->value, 0);
-w->do_write.msr = 0;
-}
-
-if ( w->do_write.cr0 )
-{
-hvm_set_cr0(w->cr0, 0);
-w->do_write.cr0 = 0;
-}
-
-if ( w->do_write.cr4 )
-{
-hvm_set_cr4(w->cr4, 0);
-w->do_write.cr4 = 0;
-}
-
-if ( w->do_write.cr3 )
-{
-hvm_set_cr3(w->cr3, 0);
-w->do_write.cr3 = 0;
-}
-}
-
 vm_event_vcpu_enter(v);
 
 /* Inject pending hw/sw trap */
@@ -2199,7 +2153,9 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer)
 
 if ( hvm_event_crX(CR0, value, old_value) )
 {
-/* The actual write will occur in hvm_do_resume(), if permitted. */
+/* The actual write will occur in vcpu_enter_write_data(), if
+ * permitted.
+ */
 v->arch.vm_event->write_data.do_write.cr0 = 1;
 v->arch.vm_event->write_data.cr0 = value;
 
@@ -2301,7 +2257,9 @@ int hvm_set_cr3(unsigned long value, bool_t may_defer)
 
 if ( hvm_event_crX(CR3, value, old) )
 {
-/* The actual write will occur in hvm_do_resume(), if permitted. */
+/* The actual write will occur in vcpu_enter_write_data(), if
+ * permitted.
+ */
 v->arch.vm_event->write_data.do_write.cr3 = 1;
 v->arch.vm_event->write_data.cr3 = value;
 
@@ -2381,7 +2339,9 @@ int hvm_set_cr4(unsigned long value, bool_t may_defer)
 
 if ( hvm_event_crX(CR4, value, old_cr) )
 {
-/* The actual write will occur in hvm_do_resume(), if permitted. */
+/* The actual write will occur in vcpu_enter_write_data(), if
+ * permitted.
+ */
 v->arch.vm_event->write_data.do_write.cr4 = 1;
 v->arch.vm_event->write_data.cr4 = value;
 
@@ -3761,7 +3721,9 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t 
msr_content,
 {
 ASSERT(v->arch.vm_event);
 
-/* The actual write will occur in hvm_do_resume() (if permitted). */
+/* The actual write will occur in vcpu_enter_write_data(), if
+ * permitted.
+ */
 v->arch.vm_event->write_data.do_write.msr = 1;
 v->arch.vm_event->write_data.msr = msr;
 v->arch.vm_event->write_data.value = msr_content;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index b43b94a..8b76ef9 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -35,7 +35,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -58,7 +57,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 static bool_t __initdata opt_force_ept;
@@ -1432,18 +1430,16 @@ static void vmx_update_guest_cr(struct vcpu *v, 
unsigned int cr)
 if ( paging_mode_hap(v->domain) )
 {
 /* Manage GUEST_CR3 when CR0.PE=0. */
+uint32_t old_ctls = v->arch.hvm_vmx.exec_control;
 uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
  CPU_BASED_CR3_STORE_EXITING);
+
 v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
 if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
 

[Xen-devel] [PATCH 3/7] vm-event: introduce vm_event_vcpu_enter

[Corneliu ZUZU]

In an effort to improve on the vm-event interface, we introduce a new function
called vm_event_vcpu_enter. Its significance is that of a "final touch" vCPU
function - i.e. it should be called by implementing architectures just before
re-entering vCPUs.
On X86 for example, it is called on the scheduling tail (hvm_do_resume) and just
before reentering the guest world after a hypervisor trap (vmx_vmenter_helper).

Signed-off-by: Corneliu ZUZU 
---
 xen/arch/arm/domain.c  |  5 -
 xen/arch/arm/traps.c   |  2 ++
 xen/arch/x86/hvm/emulate.c |  2 +-
 xen/arch/x86/hvm/event.c   |  1 -
 xen/arch/x86/hvm/hvm.c |  3 ++-
 xen/arch/x86/hvm/vmx/vmx.c |  4 
 xen/arch/x86/mm/p2m.c  |  1 -
 xen/arch/x86/vm_event.c|  4 +---
 xen/common/monitor.c   |  2 +-
 xen/common/vm_event.c  |  1 -
 xen/include/asm-arm/vm_event.h |  6 +-
 xen/include/asm-x86/vm_event.h |  6 +-
 xen/include/xen/vm_event.h | 15 +++
 13 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index d31f821..ba248c8 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -251,6 +252,8 @@ static void schedule_tail(struct vcpu *prev)
 
 ctxt_switch_to(current);
 
+vm_event_vcpu_enter(current);
+
 local_irq_enable();
 
 context_saved(prev);
@@ -296,7 +299,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
 
 void continue_running(struct vcpu *same)
 {
-/* Nothing to do */
+vm_event_vcpu_enter(same);
 }
 
 void sync_local_execstate(void)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 7fa2ae5..8c50685 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2662,6 +2663,7 @@ asmlinkage void leave_hypervisor_tail(void)
 {
 local_irq_disable();
 if (!softirq_pending(smp_processor_id())) {
+vm_event_vcpu_enter(current);
 gic_inject();
 return;
 }
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 266ed89..9b2872a 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -23,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 
 static void hvmtrace_io_assist(const ioreq_t *p)
 {
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 9c51890..26165b4 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -25,7 +25,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 78db903..770bb50 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -65,7 +65,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -509,6 +508,8 @@ void hvm_do_resume(struct vcpu *v)
 }
 }
 
+vm_event_vcpu_enter(v);
+
 /* Inject pending hw/sw trap */
 if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
 {
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 670d7dc..b43b94a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -3874,6 +3875,9 @@ void vmx_vmenter_helper(const struct cpu_user_regs *regs)
 }
 
  out:
+if ( guest_mode(regs) )
+vm_event_vcpu_enter(curr);
+
 HVMTRACE_ND(VMENTRY, 0, 1/*cycles*/, 0, 0, 0, 0, 0, 0, 0);
 
 __vmwrite(GUEST_RIP,regs->rip);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 89462b2..9d37b12 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -36,7 +36,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include "mm-locks.h"
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index 75647c4..f7eb24a 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -18,9 +18,7 @@
  * License along with this program; If not, see .
  */
 
-#include 
-#include 
-#include 
+#include 
 
 /* Implicitly serialized by the domctl lock. */
 int vm_event_init_domain(struct domain *d)
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index b30857a..c46df5a 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -22,8 +22,8 @@
 #include 
 #include 
 #include 
+#include 
 #include 
-#include 
 
 int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
 {
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 2906407..15152ba 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -27,7 +27,6 @@
 #include 
 

[Xen-devel] [PATCH 1/7] minor (formatting) fixes

[Corneliu ZUZU]

Minor fixes:
- fix 80-columns formatting in some places
- remove some empty lines
- remove some unused includes
- add 2 comments

Signed-off-by: Corneliu ZUZU 
---
 xen/arch/arm/domain.c   |  1 -
 xen/arch/arm/traps.c|  1 -
 xen/arch/x86/hvm/event.c|  1 +
 xen/common/monitor.c|  1 -
 xen/include/asm-arm/vm_event.h  | 12 ++--
 xen/include/asm-x86/hvm/event.h |  2 --
 xen/include/asm-x86/monitor.h   |  3 ---
 xen/include/asm-x86/vm_event.h  |  3 +++
 xen/include/public/vm_event.h   | 36 ++--
 xen/include/xen/vm_event.h  |  1 -
 10 files changed, 28 insertions(+), 33 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 1365b4a..d31f821 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -274,7 +274,6 @@ static void continue_new_vcpu(struct vcpu *prev)
 else
 /* check_wakeup_from_wait(); */
 reset_stack_and_jump(return_to_new_vcpu64);
-
 }
 
 void context_switch(struct vcpu *prev, struct vcpu *next)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index aa3e3c2..7fa2ae5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -439,7 +439,6 @@ static void inject_abt32_exception(struct cpu_user_regs 
*regs,
 far |= addr << 32;
 WRITE_SYSREG(far, FAR_EL1);
 WRITE_SYSREG(fsr, IFSR32_EL2);
-
 #endif
 }
 else
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 56c5514..9c51890 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -23,6 +23,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index d950a7c..b30857a 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -22,7 +22,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index 014d9ba..05c3027 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -23,21 +23,18 @@
 #include 
 #include 
 
-static inline
-int vm_event_init_domain(struct domain *d)
+static inline int vm_event_init_domain(struct domain *d)
 {
 /* Nothing to do. */
 return 0;
 }
 
-static inline
-void vm_event_cleanup_domain(struct domain *d)
+static inline void vm_event_cleanup_domain(struct domain *d)
 {
 memset(>monitor, 0, sizeof(d->monitor));
 }
 
-static inline
-void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
+static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
 {
 /* Not supported on ARM. */
 }
@@ -59,6 +56,9 @@ static inline void vm_event_fill_regs(vm_event_request_t *req)
 /* Not supported on ARM. */
 }
 
+/*
+ * Monitor vm-events.
+ */
 static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d)
 {
 uint32_t capabilities = 0;
diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/event.h
index 03f7fee..504bd66 100644
--- a/xen/include/asm-x86/hvm/event.h
+++ b/xen/include/asm-x86/hvm/event.h
@@ -19,8 +19,6 @@
 #ifndef __ASM_X86_HVM_EVENT_H__
 #define __ASM_X86_HVM_EVENT_H__
 
-#include 
-#include 
 #include 
 
 enum hvm_event_breakpoint_type
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index d367099..7a662f9 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -23,9 +23,6 @@
 #define __ASM_X86_MONITOR_H__
 
 #include 
-#include 
-#include 
-#include 
 
 #define monitor_ctrlreg_bitmask(ctrlreg_index) (1U << (ctrlreg_index))
 
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 0470240..df8e98d 100644
--- a/xen/include/asm-x86/vm_event.h
+++ b/xen/include/asm-x86/vm_event.h
@@ -44,6 +44,9 @@ void vm_event_set_registers(struct vcpu *v, 
vm_event_response_t *rsp);
 
 void vm_event_fill_regs(vm_event_request_t *req);
 
+/*
+ * Monitor vm-events.
+ */
 static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d)
 {
 uint32_t capabilities = 0;
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 9270d52..586f43b 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -74,19 +74,19 @@
  * VM_EVENT_FLAG_SET_EMUL_READ_DATA are set, only the latter will be honored).
  */
 #define VM_EVENT_FLAG_SET_EMUL_READ_DATA (1 << 5)
- /*
-  * Deny completion of the operation that triggered the event.
-  * Currently only useful for MSR, CR0, CR3 and CR4 write events.
-  */
+/*
+ * Deny completion of the operation that triggered the event.
+ * Currently only useful for MSR and control-register write events.
+ */
 #define VM_EVENT_FLAG_DENY   (1 << 6)
 /*
  * This flag can be set in a request or a response
  *
- * On a request, indicates that the event occurred in the alternate p2m 
specified by
- * the altp2m_idx request field.
+ * On a request, indicates that the event 

[Xen-devel] [PATCH 0/7] vm-event: Implement ARM support for control-register writes

[Corneliu ZUZU]

Add ARM support for control-register write vm-events.

Patch-series summary:
1.   [minor] small (formatting & other) fixes
2.   [minor] fix an issue w/ VM_EVENT_FLAG_DENY
3+4. [major] introduce a new vm-event function (vm_event_vcpu_enter) that
 gets executed just before a vCPU is (re)entered and move some
 x86 code there
5.   [minor] conceptual change of x86 monitor_write_data structure
6.   [major] move hvm_event_cr->common vm_event_monitor_cr
7.   [major] actual implementation, use HCR.TVM bit to monitor SCTLR, TTBR0,
 TTBR1 and TTBCR writes

Corneliu ZUZU (7):
  minor (formatting) fixes
  vm-event: VM_EVENT_FLAG_DENY requires VM_EVENT_FLAG_VCPU_PAUSED
  vm-event: introduce vm_event_vcpu_enter
  vm-event/x86: use vm_event_vcpu_enter properly
  x86: replace monitor_write_data.do_write with enum
  vm-event/arm: move hvm_event_cr->common vm_event_monitor_cr
  vm-event/arm: implement support for control-register write vm-events

 MAINTAINERS |   1 +
 xen/arch/arm/Makefile   |   1 +
 xen/arch/arm/domain.c   |   6 +-
 xen/arch/arm/traps.c| 129 +++-
 xen/arch/arm/vm_event.c | 112 ++
 xen/arch/x86/hvm/emulate.c  |   2 +-
 xen/arch/x86/hvm/event.c|  32 +
 xen/arch/x86/hvm/hvm.c  |  83 -
 xen/arch/x86/hvm/vmx/vmx.c  |  16 +--
 xen/arch/x86/mm/p2m.c   |   1 -
 xen/arch/x86/monitor.c  |  46 
 xen/arch/x86/vm_event.c | 134 -
 xen/common/monitor.c|  41 ++-
 xen/common/vm_event.c   |  30 -
 xen/include/asm-arm/domain.h|  30 +
 xen/include/asm-arm/traps.h | 253 
 xen/include/asm-arm/vm_event.h  |  32 +++--
 xen/include/asm-x86/domain.h|  20 ++--
 xen/include/asm-x86/hvm/event.h |  15 +--
 xen/include/asm-x86/monitor.h   |   5 -
 xen/include/asm-x86/vm_event.h  |   6 +-
 xen/include/public/vm_event.h   |  45 ---
 xen/include/xen/monitor.h   |   2 +
 xen/include/xen/vm_event.h  |  24 +++-
 24 files changed, 828 insertions(+), 238 deletions(-)
 create mode 100644 xen/arch/arm/vm_event.c
 create mode 100644 xen/include/asm-arm/traps.h

-- 
2.5.0


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

Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Juergen Gross]

On 16/06/16 15:07, Stefano Stabellini wrote:
> On Thu, 16 Jun 2016, Juergen Gross wrote:
>> On 16/06/16 12:54, Jan Beulich wrote:
>> On 16.06.16 at 12:02,  wrote:
 In case the word size of the domU and qemu running the qdisk backend
 differ BLKIF_OP_DISCARD will not work reliably, as the request
 structure in the ring have different layouts for different word size.

 Correct this by copying the request structure in case of different
 word size element by element in the BLKIF_OP_DISCARD case, too.

 Signed-off-by: Juergen Gross 
>>>
>>> With the indentation (tabs vs blanks) fixed
>>
>> Hmm, qemu coding style is to use blanks. I could:
>> a) leave the patch as is (changed lines indented with blanks)
>> b) use tabs to indent (style of the modified file up to now)
>> c) change the style of the file in this patch
>> d) change the style of the file in a separate patch
>>
>> Any preferences?
> 
> I would go with d), fixing it to conform with QEMU coding style.

As I'm about to resync the header with the Linux one as Paul suggested
it will be more like c). :-)


Juergen


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

Re: [Xen-devel] [PATCH RESEND 00/14] Xen ARM DomU ACPI support

[Boris Ostrovsky]

On 06/16/2016 07:20 AM, Wei Liu wrote:
> On Tue, Jun 07, 2016 at 12:00:26PM +0100, Julien Grall wrote:
>> Hello Shannon,
>>
>> On 07/06/16 02:07, Shannon Zhao wrote:
>>> On 2016/6/6 23:48, Julien Grall wrote:
 On 06/06/16 13:26, Wei Liu wrote:
> On Tue, May 31, 2016 at 01:02:52PM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao 
>>
>> The design of this feature is described as below.
>> Firstly, the toolstack (libxl) generates the ACPI tables according the
>> number of vcpus and gic controller.
>>
>> Then, it copies these ACPI tables to DomU memory space and passes
>> them to UEFI firmware through the "ARM multiboot" protocol.
>>
>> At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
>> and installs these tables like the usual way and passes both ACPI and DT
>> information to the Xen DomU.
>>
>> Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
>> since it's enough now.
>>
>> This has been tested using guest kernel with the Dom0 ACPI support
>> patches which could be fetched from:
>> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
>>
>>
>> Shannon Zhao (14):
>>libxl/arm: Fix the function name in error log
>>libxl/arm: Factor out codes for generating DTB
>>libxc: Add placeholders for ACPI tables blob and size
>>tools: add ACPI tables relevant definitions
>>libxl/arm: Construct ACPI GTDT table
>>libxl/arm: Construct ACPI FADT table
>>libxl/arm: Construct ACPI DSDT table
>>libxl/arm: Construct ACPI MADT table
>>libxl/arm: Construct ACPI XSDT table
>>libxl/arm: Construct ACPI RSDP table
>>libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ
>>libxl/arm: Add ACPI module
>>libxl/arm: initialize memory information of ACPI blob
>>libxc/xc_dom_core: Copy ACPI tables to guest memory space
>>
> After going through this series, I think the code mostly looks good.
>
> There is one higher level question: you seem to have put a lot of the
> table construction code in libxl, while I failed to see why specific
> libxl information is needed. Have you considered moving the code to
> libxc?
 I would rather avoid to have the device tree built by libxl and ACPI
 tables built by libxc.

 I don't remember why we have decided to implement DT building in libxl,
 I guess it is because we need to know which GIC version is used (GICv2
 vs GICv3).

 In the long run, we might also need to have more part of the firmware
 configurable (performance monitor support, memory layout, UART...).

 Most of those information are available easily in libxl, we would to
 export them of libxc.
>>> Yes, and one more reason is that to support ACPI, it also needs to add
>>> the ACPI multiboot module in DT.
>> I don't consider this as a reason for building the ACPI tables in libxl. I
>> am more concerned about building the firmwares in different place.
>>
>> If we decide to build the ACPI tables in libxc, then the code to build the
>> device tree should move in libxc too.
>>
> I've read this sub-thread and the other thread in which Boris replied, I
> think due to the fact that libxl has more information at hand and libxc
> is intended to be simple, I think having the building code in libxl
> makes sense.
>
> Shannon, Boris and Julien, what do you guys think? Can we agree on
> something so that Shannon can move on with next iteration?

I kind of agree that libxl is perhaps a better place but it would be
good to have something more than "libxc is intended to be simple" that
tells us what goes where. In other words -- what libxl is for and what
libxc is for.

For example libxl_x86.c and xc_dom86.c. The latter has a comment at the
top that says this is for arch-specific code. But there is plenty of
arch-specific stuff (e820) in libxl file.

-boris


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

Re: [Xen-devel] [Qemu-devel] [PATCH v7 11/15] qapi: Change Netdev into a flat union

[Markus Armbruster]

Eric Blake  writes:

> From: Kővágó, Zoltán 
>
> Except qapi-schema.json, this patch was generated by:
>
> find . -name .git -prune -o -type f \! -name '*~' -print0 | \
>   xargs -0 sed -i \
> -e 's/NetClientOptionsKind/NetClientDriver/g' \
> -e 's/NET_CLIENT_OPTIONS_KIND_/NET_CLIENT_DRIVER_/g' \
> -e 's/netdev->opts/netdev/g'

Uh, this is prone to descend into build trees and edit random crap.  I
used

$ sed -i -e ... `git-ls-tree -r HEAD | awk '$2 == "blob" { print $4 }'`

to verify this commit.  Differences noted inline.

>
> Signed-off-by: Kővágó, Zoltán 
> Message-Id: 
> <01a527fbf1a5de880091f98cf011616a78ad.1441627176.git.dirty.ice...@gmail.com>
>
> Additional changes:
> Rebase the patch on top of an earlier change from netdev->kind to
> netdev->type, so that tweak is no longer needed here.

I guess this is the "-e 's/netdev->kind/netdev->type/g'" you deleted
from Zoltán's commit message.

>Rebase to
> latest master which enhanced multiqueue.
>
> Rework so that NetdevLegacy doesn't pollute QMP command but is instead
> copied piecewise into the new Netdev, which means that NetClientOptions
> must still remain in qapi. Since legacy previously always rejected
> 'hubport', we can now make that explicit by having the two unions be
> slightly different; but that means we must manually map between the
> two structures.

I suspect this explains most of the differences between the sed script's
results and the actual patch.  Would it be possible to split the patch
into a mechanical and a manual part, for easier review?  If not, could
explain your manual edits in more detail?

> Signed-off-by: Eric Blake 
[...]
> index 8e79b55..490007c 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -1785,7 +1785,7 @@ pci_e1000_uninit(PCIDevice *dev)
>  }
>
>  static NetClientInfo net_e1000_info = {
> -.type = NET_CLIENT_OPTIONS_KIND_NIC,
> +.type = NET_CLIENT_DRIVER_NIC,
>  .size = sizeof(NICState),
>  .can_receive = e1000_can_receive,
>  .receive = e1000_receive,

I additionally get:

  diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
  index 692283f..825f0ba 100644
  --- a/hw/net/e1000e.c
  +++ b/hw/net/e1000e.c
  @@ -226,7 +226,7 @@ e1000e_set_link_status(NetClientState *nc)
   }

   static NetClientInfo net_e1000e_info = {
  -.type = NET_CLIENT_OPTIONS_KIND_NIC,
  +.type = NET_CLIENT_DRIVER_NIC,
   .size = sizeof(NICState),
   .can_receive = e1000e_nc_can_receive,
   .receive = e1000e_nc_receive,

Rebase needed?

> index f8a500f..89a149b 100644
> --- a/net/dump.c
> +++ b/net/dump.c
> @@ -172,7 +172,7 @@ static void dumpclient_cleanup(NetClientState *nc)
>  }
>
>  static NetClientInfo net_dump_info = {
> -.type = NET_CLIENT_OPTIONS_KIND_DUMP,
> +.type = NET_CLIENT_DRIVER_DUMP,
>  .size = sizeof(DumpNetClient),
>  .receive = dumpclient_receive,
>  .receive_iov = dumpclient_receive_iov,
> @@ -189,8 +189,8 @@ int net_init_dump(const Netdev *netdev, const char *name,
>  NetClientState *nc;
>  DumpNetClient *dnc;
>
> -assert(netdev->opts->type == NET_CLIENT_OPTIONS_KIND_DUMP);
> -dump = netdev->opts->u.dump.data;
> +assert(netdev->type == NET_CLIENT_DRIVER_DUMP);
> +dump = >u.dump;

sed turns this into

   dump = netdev->u.dump.data;

Is this part of the manual changes?  More of the same below.

>
>  assert(peer);
>

Another possible case of "rebase needed":

  diff --git a/net/filter.c b/net/filter.c
  index 8ac79f3..888fe6d 100644
  --- a/net/filter.c
  +++ b/net/filter.c
  @@ -201,7 +201,7 @@ static void netfilter_complete(UserCreatable *uc, Error 
**errp)
   }

   queues = qemu_find_net_clients_except(nf->netdev_id, ncs,
  -  NET_CLIENT_OPTIONS_KIND_NIC,
  +  NET_CLIENT_DRIVER_NIC,
 MAX_QUEUE_NUM);
   if (queues < 1) {
   error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "netdev",

> diff --git a/net/hub.c b/net/hub.c
> index ec4626f..32d8cf5 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -131,7 +131,7 @@ static void net_hub_port_cleanup(NetClientState *nc)
>  }
>
>  static NetClientInfo net_hub_port_info = {
> -.type = NET_CLIENT_OPTIONS_KIND_HUBPORT,
> +.type = NET_CLIENT_DRIVER_HUBPORT,
>  .size = sizeof(NetHubPort),
>  .can_receive = net_hub_port_can_receive,
>  .receive = net_hub_port_receive,
> @@ -266,10 +266,10 @@ int net_hub_id_for_client(NetClientState *nc, int *id)
>  {
>  NetHubPort *port;
>
> -if (nc->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) {
> +if (nc->info->type == NET_CLIENT_DRIVER_HUBPORT) {
>  port = DO_UPCAST(NetHubPort, nc, nc);
>  } else if (nc->peer != NULL && nc->peer->info->type ==
> -NET_CLIENT_OPTIONS_KIND_HUBPORT) {
> +

Re: [Xen-devel] [PATCH RFC 18/20] libxc/acpi: Build ACPI tables for HVMlite guests

[Boris Ostrovsky]

On 06/16/2016 04:54 AM, Wei Liu wrote:
> On Mon, Jun 06, 2016 at 11:15:22AM -0400, Boris Ostrovsky wrote:
> [...]
>>> +static int init_acpi_config(struct xc_dom_image *dom,
>>> +struct acpi_config *config)
>>> +{
>>> +xc_interface *xch = dom->xch;
>>> +uint32_t domid = dom->guest_domid;
>>> +xc_dominfo_t info;
>>> +int i, rc;
>>> +
>>> +memset(config, 0, sizeof(*config));
>>> +
>>> +config->dsdt_anycpu = config->dsdt_15cpu = dsdt_empty;
>>> +config->dsdt_anycpu_len = config->dsdt_15cpu_len = dsdt_empty_len;
>>> +
>>> +rc = xc_domain_getinfo(xch, domid, 1, );
>>> +if ( rc < 0 )
>>> +{
>>> +DOMPRINTF("%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc);
>>> +return rc;
>>> +}
>>> +
>>> +config->apic_mode = 1;
>>> +
>>> +if ( dom->nr_vnodes )
>>> +{
>>> +struct acpi_numa *numa = >numa;
>>> +
>>> +numa->vmemrange = calloc(dom->nr_vmemranges,
>>> + sizeof(*numa->vmemrange));
>>> +numa->vdistance = calloc(dom->nr_vnodes,
>>> + sizeof(*numa->vdistance));
>>> +numa->vcpu_to_vnode = calloc(config->nr_vcpus,
>>> + sizeof(*numa->vcpu_to_vnode));
>>> +if ( !numa->vmemrange || !numa->vdistance || !numa->vcpu_to_vnode )
>>> +{
>>> +DOMPRINTF("%s: Out of memory", __FUNCTION__);
>>> +free(numa->vmemrange);
>>> +free(numa->vdistance);
>>> +free(numa->vcpu_to_vnode);
>>> +return -ENOMEM;
>>> +}
>>> +
>>> +rc = xc_domain_getvnuma(xch, domid, >nr_vnodes,
>>> +>nr_vmemranges,
>>> +>nr_vcpus, numa->vmemrange,
>>> +numa->vdistance, numa->vcpu_to_vnode);
>>> +
>>> +   if ( rc )
>>> +{
>>> +DOMPRINTF("%s: xc_domain_getvnuma failed (rc=%d)", 
>>> __FUNCTION__, rc);
>>> +return rc;
>>> +}
>>> +}
>>> +else
>>> +config->nr_vcpus = info.max_vcpu_id + 1;
>>> This looks wrong, at least it is not immediately clear why you would
>>> want to do this.
>>
>> Why is this wrong? If we have one VCPU max_vcpu_id will be zero, won't it?
>>
> I'm not saying it is absolutely wrong, just that I don't quite
> understand why it is coded this way.
>
> It's guarded by dom->nr_vnodes and the code is not immediately clear why
> you want to do that. Perhaps you can add a comment here?

So this actually *is* wrong: I am trying to calloc(config->nr_vcpus,...)
before I call xc_domain_getvnuma(), which sets config->nr_vcpus. And I
suspect the same is true for nr_vmemranges.

I think I lost an earlier call to xc_domain_getvnuma(.., NULL, NULL, NULL).

-boris


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

Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Stefano Stabellini]

On Thu, 16 Jun 2016, Juergen Gross wrote:
> On 16/06/16 12:54, Jan Beulich wrote:
>  On 16.06.16 at 12:02,  wrote:
> >> In case the word size of the domU and qemu running the qdisk backend
> >> differ BLKIF_OP_DISCARD will not work reliably, as the request
> >> structure in the ring have different layouts for different word size.
> >>
> >> Correct this by copying the request structure in case of different
> >> word size element by element in the BLKIF_OP_DISCARD case, too.
> >>
> >> Signed-off-by: Juergen Gross 
> > 
> > With the indentation (tabs vs blanks) fixed
> 
> Hmm, qemu coding style is to use blanks. I could:
> a) leave the patch as is (changed lines indented with blanks)
> b) use tabs to indent (style of the modified file up to now)
> c) change the style of the file in this patch
> d) change the style of the file in a separate patch
> 
> Any preferences?

I would go with d), fixing it to conform with QEMU coding style.

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

Re: [Xen-devel] [PATCH v2 1/2] libs, libxc: Interface for grant copy operation

[Wei Liu]

On Thu, Jun 16, 2016 at 01:36:32PM +0100, David Vrabel wrote:
> On 16/06/16 13:16, Wei Liu wrote:
> > On Mon, Jun 13, 2016 at 11:43:55AM +0200, Paulina Szubarczyk wrote:
> >> Implentation of interface for grant copy operation in libs and
> >> libxc.
> >>
> >> In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
> >> system call is invoked. In mini-os the operation is yet not
> >> implemented. For other OSs there is a dummy implementation.
> >>
> >> Signed-off-by: Paulina Szubarczyk 
> >>
> >> ---
> >> Changes since v1:
> >> - changed the interface to call grant copy operation to match ioctl
> >>   int xengnttab_grant_copy(xengnttab_handle *xgt,
> >>uint32_t count,
> >>xengnttab_grant_copy_segment_t* segs)
> >> - added a struct 'xengnttab_copy_grant_segment' definition to tools/libs  
> >>   /gnttab/private.h, tools/libxc/include/xenctrl_compat.h
> >> - changed the function osdep_gnttab_grant_copy which right now just call
> >>   the ioctl
> >> - added a new VER1.1 to tools/libs/gnttab/libxengnttab.map 
> >>
> >>  tools/include/xen-sys/Linux/gntdev.h  | 21 +
> >>  tools/libs/gnttab/gnttab_core.c   |  6 ++
> >>  tools/libs/gnttab/gnttab_unimp.c  |  6 ++
> >>  tools/libs/gnttab/include/xengnttab.h | 14 ++
> >>  tools/libs/gnttab/libxengnttab.map|  4 
> >>  tools/libs/gnttab/linux.c | 18 ++
> >>  tools/libs/gnttab/minios.c|  6 ++
> >>  tools/libs/gnttab/private.h   | 18 ++
> >>  tools/libxc/include/xenctrl_compat.h  | 23 +++
> >>  tools/libxc/xc_gnttab_compat.c|  7 +++
> >>  10 files changed, 123 insertions(+)
> >>
> >> diff --git a/tools/include/xen-sys/Linux/gntdev.h 
> >> b/tools/include/xen-sys/Linux/gntdev.h
> >> index caf6fb4..0ca07c9 100644
> >> --- a/tools/include/xen-sys/Linux/gntdev.h
> >> +++ b/tools/include/xen-sys/Linux/gntdev.h
> >> @@ -147,4 +147,25 @@ struct ioctl_gntdev_unmap_notify {
> >>  /* Send an interrupt on the indicated event channel */
> >>  #define UNMAP_NOTIFY_SEND_EVENT 0x2
> >>  
> >> +struct ioctl_gntdev_grant_copy_segment {
> >> +union {
> >> +void *virt;
> >> +struct {
> >> +uint32_t ref;
> >> +uint16_t offset;
> >> +uint16_t domid;
> >> +} foreign;
> >> +} source, dest;
> >> +uint16_t len;
> >> +uint16_t flags;
> >> +int16_t status;
> >> +};
> >> +
> >> +#define IOCTL_GNTDEV_GRANT_COPY \
> >> +_IOC(_IOC_NONE, 'G', 8, sizeof(struct ioctl_gntdev_grant_copy))
> >> +struct ioctl_gntdev_grant_copy {
> >> +unsigned int count;
> >> +struct ioctl_gntdev_grant_copy_segment *segments;
> >> +};
> >> +
> > 
> > This is ok.
> > 
> >>  #endif /* __LINUX_PUBLIC_GNTDEV_H__ */
> >> diff --git a/tools/libs/gnttab/gnttab_core.c 
> >> b/tools/libs/gnttab/gnttab_core.c
> >> index 5d0474d..9badc58 100644
> >> --- a/tools/libs/gnttab/gnttab_core.c
> >> +++ b/tools/libs/gnttab/gnttab_core.c
> >> @@ -113,6 +113,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void 
> >> *start_address, uint32_t count)
> >>  return osdep_gnttab_unmap(xgt, start_address, count);
> >>  }
> >>  
> >> +int xengnttab_grant_copy(xengnttab_handle *xgt,
> >> + uint32_t count,
> >> + xengnttab_grant_copy_segment_t* segs)
> >> +{
> >> +return osdep_gnttab_grant_copy(xgt, count, segs);
> >> +}
> >>  /*
> >>   * Local variables:
> >>   * mode: C
> >> diff --git a/tools/libs/gnttab/gnttab_unimp.c 
> >> b/tools/libs/gnttab/gnttab_unimp.c
> >> index b3a4a20..79a5c88 100644
> >> --- a/tools/libs/gnttab/gnttab_unimp.c
> >> +++ b/tools/libs/gnttab/gnttab_unimp.c
> >> @@ -78,6 +78,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void 
> >> *start_address, uint32_t count)
> >>  abort();
> >>  }
> >>  
> >> +int xengnttab_copy_grant(xengnttab_handle *xgt,
> >> + uint32_t count,
> >> + xengnttab_copy_grant_segment_t* segs)
> > 
> > Coding style. Should be " *segs" instead of "* segs".
> > 
> > Please also fix other instances of this nit.
> > 
> >> +{
> >> +return -1;
> > 
> > Please use abort() here instead.
> 
> This function is used to test whether grant copy is supported so cannot
> abort().  It is correctly returning an error.
> 

No. For the "unimplemented" implementation, the code shouldn't call this
function in the first place because the attempt to open a handle would
have already failed.

This is an impossible state for the "unimplemented" implementation. Any
application uses the API like this deserves to be aborted.

> >> \ No newline at end of file
> >> diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
> >> index 7b0fba4..26bfbdc 100644
> >> --- a/tools/libs/gnttab/linux.c
> >> +++ b/tools/libs/gnttab/linux.c
> >> @@ -235,6 +235,24 @@ int 

[Xen-devel] [ovmf test] 95775: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. 
vs. 94748
 test-amd64-amd64-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail 
REGR. vs. 94748

version targeted for testing:
 ovmf 8f88f023fc59961973bb1ef121c7bee7b0a61975
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z   22 days
Failing since 94750  2016-05-25 03:43:08 Z   22 days   39 attempts
Testing same since95775  2016-06-15 11:03:35 Z1 days1 attempts


People who touched revisions under test:
  Chao Zhang 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Darbin Reyes 
  Eugene Cohen 
  Fu Siyuan 
  Fu, Siyuan 
  Gary Li 
  Gary Lin 
  Giri P Mudusuru 
  Hao Wu 
  Hegde Nagaraj P 
  Jeff Fan 
  Jiaxin Wu 
  Jiewen Yao 
  Katie Dellaquila 
  Laszlo Ersek 
  Liming Gao 
  lushifex 
  Marvin H?user 
  Marvin Haeuser 
  Maurice Ma 
  Michael Zimmermann 
  Ruiyu Ni 
  Sriram Subramanian 
  Star Zeng 
  Tapan Shah 
  Thomas Palmer 
  Yonghong Zhu 
  Zhang, Chao B 

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 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail



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


Not pushing.

(No revision log; it would be 1879 lines long.)

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

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Jan Beulich]

>>> On 16.06.16 at 13:18,  wrote:
> On 6/16/2016 6:02 PM, Jan Beulich wrote:
>>> +struct xen_hvm_map_mem_type_to_ioreq_server {
>>> +domid_t domid;  /* IN - domain to be serviced */
>>> +ioservid_t id;  /* IN - ioreq server id */
>>> +uint16_t type;  /* IN - memory type */
>>> +uint16_t pad;
>> This field does not appear to get checked in the handler.
> I am now wondering, how about we remove this pad field and define type
> as uint32_t?
 As above - I think the current layout is fine. But I'm also not heavily
 opposed to using uint32_t here. It's not a stable interface anyway
 (and I already have a series mostly ready to split off all control
 operations from the HVMOP_* ones, into a new HVMCTL_* set,
 which will make all of them interface-versioned).
>>> I'd like to keep this interface. BTW, you mentioned "this field does not
>>> appear to
>>> get checked in the handler", do you mean we need to check the pad in the
>>> handler?
>> Yes.
>>
>>> And why?
>> In order to be able to later assign meaning to it without breaking
>> existing users.
> 
> So the handler need to assure the pad is 0, right?

Correct.

Jan


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

Re: [Xen-devel] [PATCH v2 1/2] libs, libxc: Interface for grant copy operation

[David Vrabel]

On 16/06/16 13:16, Wei Liu wrote:
> On Mon, Jun 13, 2016 at 11:43:55AM +0200, Paulina Szubarczyk wrote:
>> Implentation of interface for grant copy operation in libs and
>> libxc.
>>
>> In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
>> system call is invoked. In mini-os the operation is yet not
>> implemented. For other OSs there is a dummy implementation.
>>
>> Signed-off-by: Paulina Szubarczyk 
>>
>> ---
>> Changes since v1:
>> - changed the interface to call grant copy operation to match ioctl
>>   int xengnttab_grant_copy(xengnttab_handle *xgt,
>>uint32_t count,
>>xengnttab_grant_copy_segment_t* segs)
>> - added a struct 'xengnttab_copy_grant_segment' definition to tools/libs  
>>   /gnttab/private.h, tools/libxc/include/xenctrl_compat.h
>> - changed the function osdep_gnttab_grant_copy which right now just call
>>   the ioctl
>> - added a new VER1.1 to tools/libs/gnttab/libxengnttab.map 
>>
>>  tools/include/xen-sys/Linux/gntdev.h  | 21 +
>>  tools/libs/gnttab/gnttab_core.c   |  6 ++
>>  tools/libs/gnttab/gnttab_unimp.c  |  6 ++
>>  tools/libs/gnttab/include/xengnttab.h | 14 ++
>>  tools/libs/gnttab/libxengnttab.map|  4 
>>  tools/libs/gnttab/linux.c | 18 ++
>>  tools/libs/gnttab/minios.c|  6 ++
>>  tools/libs/gnttab/private.h   | 18 ++
>>  tools/libxc/include/xenctrl_compat.h  | 23 +++
>>  tools/libxc/xc_gnttab_compat.c|  7 +++
>>  10 files changed, 123 insertions(+)
>>
>> diff --git a/tools/include/xen-sys/Linux/gntdev.h 
>> b/tools/include/xen-sys/Linux/gntdev.h
>> index caf6fb4..0ca07c9 100644
>> --- a/tools/include/xen-sys/Linux/gntdev.h
>> +++ b/tools/include/xen-sys/Linux/gntdev.h
>> @@ -147,4 +147,25 @@ struct ioctl_gntdev_unmap_notify {
>>  /* Send an interrupt on the indicated event channel */
>>  #define UNMAP_NOTIFY_SEND_EVENT 0x2
>>  
>> +struct ioctl_gntdev_grant_copy_segment {
>> +union {
>> +void *virt;
>> +struct {
>> +uint32_t ref;
>> +uint16_t offset;
>> +uint16_t domid;
>> +} foreign;
>> +} source, dest;
>> +uint16_t len;
>> +uint16_t flags;
>> +int16_t status;
>> +};
>> +
>> +#define IOCTL_GNTDEV_GRANT_COPY \
>> +_IOC(_IOC_NONE, 'G', 8, sizeof(struct ioctl_gntdev_grant_copy))
>> +struct ioctl_gntdev_grant_copy {
>> +unsigned int count;
>> +struct ioctl_gntdev_grant_copy_segment *segments;
>> +};
>> +
> 
> This is ok.
> 
>>  #endif /* __LINUX_PUBLIC_GNTDEV_H__ */
>> diff --git a/tools/libs/gnttab/gnttab_core.c 
>> b/tools/libs/gnttab/gnttab_core.c
>> index 5d0474d..9badc58 100644
>> --- a/tools/libs/gnttab/gnttab_core.c
>> +++ b/tools/libs/gnttab/gnttab_core.c
>> @@ -113,6 +113,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void 
>> *start_address, uint32_t count)
>>  return osdep_gnttab_unmap(xgt, start_address, count);
>>  }
>>  
>> +int xengnttab_grant_copy(xengnttab_handle *xgt,
>> + uint32_t count,
>> + xengnttab_grant_copy_segment_t* segs)
>> +{
>> +return osdep_gnttab_grant_copy(xgt, count, segs);
>> +}
>>  /*
>>   * Local variables:
>>   * mode: C
>> diff --git a/tools/libs/gnttab/gnttab_unimp.c 
>> b/tools/libs/gnttab/gnttab_unimp.c
>> index b3a4a20..79a5c88 100644
>> --- a/tools/libs/gnttab/gnttab_unimp.c
>> +++ b/tools/libs/gnttab/gnttab_unimp.c
>> @@ -78,6 +78,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void 
>> *start_address, uint32_t count)
>>  abort();
>>  }
>>  
>> +int xengnttab_copy_grant(xengnttab_handle *xgt,
>> + uint32_t count,
>> + xengnttab_copy_grant_segment_t* segs)
> 
> Coding style. Should be " *segs" instead of "* segs".
> 
> Please also fix other instances of this nit.
> 
>> +{
>> +return -1;
> 
> Please use abort() here instead.

This function is used to test whether grant copy is supported so cannot
abort().  It is correctly returning an error.

>> \ No newline at end of file
>> diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
>> index 7b0fba4..26bfbdc 100644
>> --- a/tools/libs/gnttab/linux.c
>> +++ b/tools/libs/gnttab/linux.c
>> @@ -235,6 +235,24 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
>>  return 0;
>>  }
>>  
>> diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
>> index d286c86..22ad53a 100644
>> --- a/tools/libs/gnttab/private.h
>> +++ b/tools/libs/gnttab/private.h
>> @@ -9,6 +9,20 @@ struct xengntdev_handle {
>>  int fd;
>>  };
>>  
>> +struct xengnttab_copy_grant_segment {
>> +union xengnttab_copy_ptr {
>> +void *virt;
>> +struct {
>> +uint32_t ref;
>> +uint16_t offset;
>> +uint16_t domid;
>> +} foreign;
>> +} source, dest;
>> +uint16_t len;
>> +uint16_t 

Re: [Xen-devel] [PATCH v2 2/2] xen: add update indicator to vcpu_runstate_info

[Jan Beulich]

>>> On 16.06.16 at 14:11,  wrote:
> On 16/06/16 13:15, Jan Beulich wrote:
> On 15.06.16 at 13:34,  wrote:
>>> +if ( VM_ASSIST(v->domain, runstate_update_flag) )
>>> +{
>>> +off = offsetof(struct vcpu_runstate_info, state_entry_time) +
>>> +  sizeof(v->runstate.state_entry_time) - 1;
>>> +if ( has_32bit_shinfo(v->domain) )
>>> +{
>>> +guest_handle = v->runstate_guest.compat.p;
>>> +guest_handle +=
>>> +offsetof(struct compat_vcpu_runstate_info, 
>>> state_entry_time) +
>>> +sizeof(v->runstate.state_entry_time) - 1;
>> 
>> The sizes of the native and compat fields happen to be the same,
>> but it would be nice if the right field/type could be used here.
> 
> Hmm, this will require some ugly type casting, but it is probably
> cleaner.

Type casting? I would expect you to be able to use
v->runstate_guest.compat.p->state_entry_time. In fact I think
you also could get rid of the offsetof() if you used
>runstate_guest.compat.p->state_entry_time for initializing
guest_handle.

Jan


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

Re: [Xen-devel] [PATCH RFC 2/2] xen: make available hvm_fep to non-debug build as well

[Jan Beulich]

>>> On 16.06.16 at 13:52,  wrote:
> On Thu, Jun 16, 2016 at 05:37:03AM -0600, Jan Beulich wrote:
>> >>> On 15.06.16 at 16:31,  wrote:
>> > +printk("**\n");
>> > +printk("*** WARNING: HVM FORCED EMULATION PREFIX IS 
>> > PERMITTED\n");
>> > +printk("*** This option is *ONLY* intended to aid debugging "
>> > +   "and testing of Xen\n");
>> > +printk("*** that HVM guest can enter instruction emulator "
>> > +   "with UD instruction.\n");
>> > +printk("*** It has implication on the security of the 
>> > system.\n");
>> > +printk("*** Please *DO NOT* use this in production.\n");
>> > +printk("**\n");
>> > +add_taint(TAINT_HVM_FEP);
>> 
>> Should we perhaps taint the system only the first time a guest
>> makes use of this?
>> 
> 
> Doesn't that add overhead to a potential hot path? Arguably it is only
> setting a bit in a flag, but still...

How can that be a hot path, if it's not even usable without the
option set?

Jan


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

Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Paul Durrant]

> -Original Message-
> From: Juergen Gross [mailto:jgr...@suse.com]
> Sent: 16 June 2016 13:19
> To: Paul Durrant; qemu-de...@nongnu.org; xen-de...@lists.xensource.com
> Cc: Anthony Perard; sstabell...@kernel.org; kra...@redhat.com
> Subject: Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64
> word size mix
> 
> On 16/06/16 13:24, Paul Durrant wrote:
> >> -Original Message-
> >> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
> >> Juergen Gross
> >> Sent: 16 June 2016 11:02
> >> To: qemu-de...@nongnu.org; xen-de...@lists.xensource.com
> >> Cc: Anthony Perard; Juergen Gross; sstabell...@kernel.org;
> >> kra...@redhat.com
> >> Subject: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64
> >> word size mix
> >>
> >> In case the word size of the domU and qemu running the qdisk backend
> >> differ BLKIF_OP_DISCARD will not work reliably, as the request
> >> structure in the ring have different layouts for different word size.
> >>
> >> Correct this by copying the request structure in case of different
> >> word size element by element in the BLKIF_OP_DISCARD case, too.
> >>
> >> Signed-off-by: Juergen Gross 
> >
> > Would it not be better to re-import the canonical blkif header as a whole
> rather than cherry-picking like this? You'd need to post-process to maintain
> style and possibly change some names for compatibility etc. but probably
> nothing beyond what indent and a simple [s]ed script can do.
> > I did broadly the same thing to re-import the netif header into Linux
> recently.
> 
> It's a little bit more than indent/sed, but nothing really scary.
> 
> I'll do it.
>

Cool. The nice thing about bringing the header up-to-date is it pulls in all 
the newer comments and explanations too :-)
 
  Paul

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

Re: [Xen-devel] [PATCH RFC 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

On Thu, Jun 16, 2016 at 01:12:34PM +0100, Andrew Cooper wrote:
> On 16/06/16 12:52, Wei Liu wrote:
> >
> >>> +printk("**\n");
> >>> +printk("*** WARNING: HVM FORCED EMULATION PREFIX IS 
> >>> PERMITTED\n");
> 
> I would say "available" rather than permitted in this case.
> 
> >>> +printk("*** This option is *ONLY* intended to aid debugging "
> >>> +   "and testing of Xen\n");
> 
> Despite the line length, I would keep this string on a single line.  If
> you want it a little shorter, you can drop "debugging and", leaving just
> testing.
> 
> >>> +printk("*** that HVM guest can enter instruction emulator "
> >>> +   "with UD instruction.\n");
> 
> I think this like isn't necessary.  Anyone who is unclear what FEP is
> can look it up.
> 
> >>> +printk("*** It has implication on the security of the 
> >>> system.\n");
> 
> implications.
> 

All fixed.

> >>> +printk("*** Please *DO NOT* use this in production.\n");
> >>> +printk("**\n");
> >>> +add_taint(TAINT_HVM_FEP);
> >> Should we perhaps taint the system only the first time a guest
> >> makes use of this?
> >>
> > Doesn't that add overhead to a potential hot path? Arguably it is only
> > setting a bit in a flag, but still...
> 
> FEP is not a fastpath at all.  It would be fine to defer to
> hvm_ud_intercept().
> 

NP.

Wei.

> ~Andrew

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

Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Juergen Gross]

On 16/06/16 13:24, Paul Durrant wrote:
>> -Original Message-
>> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
>> Juergen Gross
>> Sent: 16 June 2016 11:02
>> To: qemu-de...@nongnu.org; xen-de...@lists.xensource.com
>> Cc: Anthony Perard; Juergen Gross; sstabell...@kernel.org;
>> kra...@redhat.com
>> Subject: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64
>> word size mix
>>
>> In case the word size of the domU and qemu running the qdisk backend
>> differ BLKIF_OP_DISCARD will not work reliably, as the request
>> structure in the ring have different layouts for different word size.
>>
>> Correct this by copying the request structure in case of different
>> word size element by element in the BLKIF_OP_DISCARD case, too.
>>
>> Signed-off-by: Juergen Gross 
> 
> Would it not be better to re-import the canonical blkif header as a whole 
> rather than cherry-picking like this? You'd need to post-process to maintain 
> style and possibly change some names for compatibility etc. but probably 
> nothing beyond what indent and a simple [s]ed script can do.
> I did broadly the same thing to re-import the netif header into Linux 
> recently.

It's a little bit more than indent/sed, but nothing really scary.

I'll do it.


Juergen

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

Re: [Xen-devel] [PATCH v2 1/2] libs, libxc: Interface for grant copy operation

[Wei Liu]

On Mon, Jun 13, 2016 at 11:43:55AM +0200, Paulina Szubarczyk wrote:
> Implentation of interface for grant copy operation in libs and
> libxc.
> 
> In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
> system call is invoked. In mini-os the operation is yet not
> implemented. For other OSs there is a dummy implementation.
> 
> Signed-off-by: Paulina Szubarczyk 
> 
> ---
> Changes since v1:
> - changed the interface to call grant copy operation to match ioctl
>   int xengnttab_grant_copy(xengnttab_handle *xgt,
>uint32_t count,
>xengnttab_grant_copy_segment_t* segs)
> - added a struct 'xengnttab_copy_grant_segment' definition to tools/libs  
>   /gnttab/private.h, tools/libxc/include/xenctrl_compat.h
> - changed the function osdep_gnttab_grant_copy which right now just call
>   the ioctl
> - added a new VER1.1 to tools/libs/gnttab/libxengnttab.map 
> 
>  tools/include/xen-sys/Linux/gntdev.h  | 21 +
>  tools/libs/gnttab/gnttab_core.c   |  6 ++
>  tools/libs/gnttab/gnttab_unimp.c  |  6 ++
>  tools/libs/gnttab/include/xengnttab.h | 14 ++
>  tools/libs/gnttab/libxengnttab.map|  4 
>  tools/libs/gnttab/linux.c | 18 ++
>  tools/libs/gnttab/minios.c|  6 ++
>  tools/libs/gnttab/private.h   | 18 ++
>  tools/libxc/include/xenctrl_compat.h  | 23 +++
>  tools/libxc/xc_gnttab_compat.c|  7 +++
>  10 files changed, 123 insertions(+)
> 
> diff --git a/tools/include/xen-sys/Linux/gntdev.h 
> b/tools/include/xen-sys/Linux/gntdev.h
> index caf6fb4..0ca07c9 100644
> --- a/tools/include/xen-sys/Linux/gntdev.h
> +++ b/tools/include/xen-sys/Linux/gntdev.h
> @@ -147,4 +147,25 @@ struct ioctl_gntdev_unmap_notify {
>  /* Send an interrupt on the indicated event channel */
>  #define UNMAP_NOTIFY_SEND_EVENT 0x2
>  
> +struct ioctl_gntdev_grant_copy_segment {
> +union {
> +void *virt;
> +struct {
> +uint32_t ref;
> +uint16_t offset;
> +uint16_t domid;
> +} foreign;
> +} source, dest;
> +uint16_t len;
> +uint16_t flags;
> +int16_t status;
> +};
> +
> +#define IOCTL_GNTDEV_GRANT_COPY \
> +_IOC(_IOC_NONE, 'G', 8, sizeof(struct ioctl_gntdev_grant_copy))
> +struct ioctl_gntdev_grant_copy {
> +unsigned int count;
> +struct ioctl_gntdev_grant_copy_segment *segments;
> +};
> +

This is ok.

>  #endif /* __LINUX_PUBLIC_GNTDEV_H__ */
> diff --git a/tools/libs/gnttab/gnttab_core.c b/tools/libs/gnttab/gnttab_core.c
> index 5d0474d..9badc58 100644
> --- a/tools/libs/gnttab/gnttab_core.c
> +++ b/tools/libs/gnttab/gnttab_core.c
> @@ -113,6 +113,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void 
> *start_address, uint32_t count)
>  return osdep_gnttab_unmap(xgt, start_address, count);
>  }
>  
> +int xengnttab_grant_copy(xengnttab_handle *xgt,
> + uint32_t count,
> + xengnttab_grant_copy_segment_t* segs)
> +{
> +return osdep_gnttab_grant_copy(xgt, count, segs);
> +}
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libs/gnttab/gnttab_unimp.c 
> b/tools/libs/gnttab/gnttab_unimp.c
> index b3a4a20..79a5c88 100644
> --- a/tools/libs/gnttab/gnttab_unimp.c
> +++ b/tools/libs/gnttab/gnttab_unimp.c
> @@ -78,6 +78,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void 
> *start_address, uint32_t count)
>  abort();
>  }
>  
> +int xengnttab_copy_grant(xengnttab_handle *xgt,
> + uint32_t count,
> + xengnttab_copy_grant_segment_t* segs)

Coding style. Should be " *segs" instead of "* segs".

Please also fix other instances of this nit.

> +{
> +return -1;

Please use abort() here instead.

> +}
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libs/gnttab/include/xengnttab.h 
> b/tools/libs/gnttab/include/xengnttab.h
> index 0431dcf..a9fa1fe 100644
> --- a/tools/libs/gnttab/include/xengnttab.h
> +++ b/tools/libs/gnttab/include/xengnttab.h
> @@ -258,6 +258,20 @@ int xengnttab_unmap(xengnttab_handle *xgt, void 
> *start_address, uint32_t count);
>  int xengnttab_set_max_grants(xengnttab_handle *xgt,
>   uint32_t nr_grants);
>  
> +typedef struct xengnttab_grant_copy_segment xengnttab_grant_copy_segment_t;
> +
> +/**
> + * Copy memory from or to grant references. The information of each 
> operations
> + * are contained in 'xengnttab_grant_copy_segment_t'. The @flag value 
> indicate
> + * the direction of an operation (GNTCOPY_source_gref\GNTCOPY_dest_gref).
> + *
> + * The sum of fields @offset[i] and @len[i] of 
> 'xengnttab_grant_copy_segment_t'
> + * should not exceed XEN_PAGE_SIZE
> + */
> +int xengnttab_grant_copy(xengnttab_handle *xgt,
> + uint32_t count,
> + xengnttab_grant_copy_segment_t* segs);
> +
>  /*
>   

Re: [Xen-devel] [PATCH RFC 2/2] xen: make available hvm_fep to non-debug build as well

[Andrew Cooper]

On 16/06/16 12:52, Wei Liu wrote:
>
>>> +printk("**\n");
>>> +printk("*** WARNING: HVM FORCED EMULATION PREFIX IS 
>>> PERMITTED\n");

I would say "available" rather than permitted in this case.

>>> +printk("*** This option is *ONLY* intended to aid debugging "
>>> +   "and testing of Xen\n");

Despite the line length, I would keep this string on a single line.  If
you want it a little shorter, you can drop "debugging and", leaving just
testing.

>>> +printk("*** that HVM guest can enter instruction emulator "
>>> +   "with UD instruction.\n");

I think this like isn't necessary.  Anyone who is unclear what FEP is
can look it up.

>>> +printk("*** It has implication on the security of the 
>>> system.\n");

implications.

>>> +printk("*** Please *DO NOT* use this in production.\n");
>>> +printk("**\n");
>>> +add_taint(TAINT_HVM_FEP);
>> Should we perhaps taint the system only the first time a guest
>> makes use of this?
>>
> Doesn't that add overhead to a potential hot path? Arguably it is only
> setting a bit in a flag, but still...

FEP is not a fastpath at all.  It would be fine to defer to
hvm_ud_intercept().

~Andrew

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

Re: [Xen-devel] [PATCH v2 2/2] xen: add update indicator to vcpu_runstate_info

[Juergen Gross]

On 16/06/16 13:15, Jan Beulich wrote:
 On 15.06.16 at 13:34,  wrote:
>> In order to support reading another vcpu's mapped vcpu_runstate_info
>> an indicator for an occurring update of the runstate information is
>> needed.
>>
>> Add the possibility to activate setting this indicator in the highest
>> bit of state_entry_time via a vm_assist hypercall. When activated the
>> update indicator will be set before the runstate information is
>> modified in guest memory and it will be reset after modification is
>> done. As state_entry_time is guaranteed to be different after each
>> update the guest can detect any update (either in progress or while
>> reading the runstate data) by comparing state_entry_time before and
>> after reading runstate data: in case the values differ or the update
>> indicator was set the data might be inconsistent and should be reread.
> 
> Neither here nor in the cover letter you mention why this is useful
> to have (and to be honest I did forget what you need this for since
> the time the original discussion happened), yet that's quite relevant
> since for many years we lived happily without it.

I'll add the following in the cover letter:

There has been a report about incorrect vruntime accounting in Linux
guests under Xen. A Linux kernel with CONFIG_PARAVIRT_TIME_ACCOUNTING
set is capable to do correct vruntime accounting, but this would
require the kernel to be able to read the runstate data of other cpus.

> 
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -1926,12 +1926,35 @@ bool_t update_runstate_area(struct vcpu *v)
>>  {
>>  bool_t rc;
>>  smap_check_policy_t smap_policy;
>> +void __user *guest_handle = NULL;
>> +unsigned off = 0;
>>  
>>  if ( guest_handle_is_null(runstate_guest(v)) )
>>  return 1;
>>  
>>  smap_policy = smap_policy_change(v, SMAP_CHECK_ENABLED);
>>  
>> +if ( VM_ASSIST(v->domain, runstate_update_flag) )
>> +{
>> +off = offsetof(struct vcpu_runstate_info, state_entry_time) +
>> +  sizeof(v->runstate.state_entry_time) - 1;
>> +if ( has_32bit_shinfo(v->domain) )
>> +{
>> +guest_handle = v->runstate_guest.compat.p;
>> +guest_handle +=
>> +offsetof(struct compat_vcpu_runstate_info, 
>> state_entry_time) +
>> +sizeof(v->runstate.state_entry_time) - 1;
> 
> The sizes of the native and compat fields happen to be the same,
> but it would be nice if the right field/type could be used here.

Hmm, this will require some ugly type casting, but it is probably
cleaner.

>> --- a/xen/include/public/vcpu.h
>> +++ b/xen/include/public/vcpu.h
>> @@ -84,6 +84,12 @@ struct vcpu_runstate_info {
>>  /* When was current state entered (system time, ns)? */
>>  uint64_t state_entry_time;
>>  /*
>> + * Update indicator set in state_entry_time:
>> + * When activated via VMASST_TYPE_runstate_update_flag, set during
>> + * updates in guest memory mapped copy of vcpu_runstate_info.
>> + */
>> +#define XEN_RUNSTATE_UPDATE  (1ULL << 63)
> 
> I continue to be not particularly happy with the C99ism here, but
> I see we have a few other instances of such already in the public
> headers.

Indeed. And UINT64_C() is used nowhere in the hypervisor and defined in
xen/include/crypto/vmac.h


Juergen

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

[Xen-devel] [qemu-upstream-4.3-testing test] 95788: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

flight 95788 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/95788/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-i386  3 host-install(3)   broken REGR. vs. 80927
 test-amd64-i386-xl-raw3 host-install(3) broken REGR. vs. 80927
 build-amd64-libvirt   5 libvirt-build fail REGR. vs. 80927
 build-i386-libvirt5 libvirt-build fail REGR. vs. 80927

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

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail never pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install  fail never pass

version targeted for testing:
 qemuu12e8fccf5b5460be7aecddc71d27eceaba6e1f15
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  130 days
Failing since 93977  2016-05-10 11:09:16 Z   37 days  125 attempts
Testing same since95534  2016-06-11 00:59:46 Z5 days5 attempts


People who touched revisions under test:
  Anthony PERARD 
  Gerd Hoffmann 
  Ian Jackson 
  Stefano Stabellini 
  Wei Liu 

jobs:
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  fail
 build-i386-libvirt   fail
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-i386-freebsd10-amd64  pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-amd64-xl-credit2  pass
 test-amd64-i386-freebsd10-i386   broken  
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpupass
 test-amd64-amd64-pairpass
 test-amd64-i386-pair pass
 test-amd64-amd64-pv  pass
 test-amd64-i386-pv   pass
 test-amd64-amd64-amd64-pvgrubpass
 test-amd64-amd64-i386-pvgrub pass
 test-amd64-amd64-pygrub  pass
 test-amd64-amd64-xl-qcow2pass
 test-amd64-i386-xl-raw   broken  
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass
 test-amd64-amd64-libvirt-vhd blocked 
 test-amd64-amd64-xl-qemuu-winxpsp3   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

broken-step test-amd64-i386-freebsd10-i386 host-install(3)
broken-step test-amd64-i386-xl-raw host-install(3)

Not pushing.

Re: [Xen-devel] [PATCH RFC 2/2] xen: make available hvm_fep to non-debug build as well

[Wei Liu]

On Thu, Jun 16, 2016 at 05:37:03AM -0600, Jan Beulich wrote:
> >>> On 15.06.16 at 16:31,  wrote:
> > @@ -182,6 +181,32 @@ static int __init hvm_enable(void)
> >  if ( !opt_altp2m_enabled )
> >  hvm_funcs.altp2m_supported = 0;
> >  
> > +if ( opt_hvm_fep )
> > +{
> > +unsigned i, j;
> 
> unsigned int
> 

Ack.

> > +printk("**\n");
> > +printk("*** WARNING: HVM FORCED EMULATION PREFIX IS 
> > PERMITTED\n");
> > +printk("*** This option is *ONLY* intended to aid debugging "
> > +   "and testing of Xen\n");
> > +printk("*** that HVM guest can enter instruction emulator "
> > +   "with UD instruction.\n");
> > +printk("*** It has implication on the security of the 
> > system.\n");
> > +printk("*** Please *DO NOT* use this in production.\n");
> > +printk("**\n");
> > +add_taint(TAINT_HVM_FEP);
> 
> Should we perhaps taint the system only the first time a guest
> makes use of this?
> 

Doesn't that add overhead to a potential hot path? Arguably it is only
setting a bit in a flag, but still...

> > +for ( i = 0; i < 3; i++ )
> > +{
> > +printk("%d... ", 3-i);
> 
> %u and spaces around - please.
> 

Ack.

Wei.

> Jan
> 

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

Re: [Xen-devel] [PATCH RFC 2/2] xen: make available hvm_fep to non-debug build as well

[Jan Beulich]

>>> On 15.06.16 at 16:31,  wrote:
> @@ -182,6 +181,32 @@ static int __init hvm_enable(void)
>  if ( !opt_altp2m_enabled )
>  hvm_funcs.altp2m_supported = 0;
>  
> +if ( opt_hvm_fep )
> +{
> +unsigned i, j;

unsigned int

> +printk("**\n");
> +printk("*** WARNING: HVM FORCED EMULATION PREFIX IS 
> PERMITTED\n");
> +printk("*** This option is *ONLY* intended to aid debugging "
> +   "and testing of Xen\n");
> +printk("*** that HVM guest can enter instruction emulator "
> +   "with UD instruction.\n");
> +printk("*** It has implication on the security of the 
> system.\n");
> +printk("*** Please *DO NOT* use this in production.\n");
> +printk("**\n");
> +add_taint(TAINT_HVM_FEP);

Should we perhaps taint the system only the first time a guest
makes use of this?

> +for ( i = 0; i < 3; i++ )
> +{
> +printk("%d... ", 3-i);

%u and spaces around - please.

Jan


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

Re: [Xen-devel] [PATCH v2] x86: show remote CPU state upon fatal NMI or unknown MCE

[Andrew Cooper]

On 16/06/16 12:26, Jan Beulich wrote:
> @@ -570,6 +589,15 @@ void fatal_trap(const struct cpu_user_re
>  printk("Faulting linear address: %p\n", _p(cr2));
>  show_page_walk(cr2);
>  }
> +if ( show_remote )
> +{
> +cpumask_andnot(_state_mask, _online_map,
> +   cpumask_of(smp_processor_id()));
> +set_nmi_callback(nmi_show_execution_state);
> +smp_send_nmi_allbutself();
> +while ( !cpumask_empty(_state_mask) )
> +cpu_relax();

Sorry about not noticing this before.  We need a maximum timeout here,
just like the nmi shootdown, in case one of the other cpus is already
stuck in an NMI handler, so we block the reboot/kexec.

With that fixed, Reviewed-by: Andrew Cooper 

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

[Xen-devel] [PATCH v2] x86: show remote CPU state upon fatal NMI or unknown MCE

[Jan Beulich]

Quite frequently the watchdog would hit an innocent CPU, e.g. one
trying to acquire a spin lock a remote CPU holds for extended periods
of time, or a random CPU in TSC calbration rendezvous. In such cases
the register and stack dump for that CPU doesn't really help in the
analysis of the problem.

To keep things reasonable on large systems, only log CS:RIP by default.
This can be overridden via a new command line option such that full
register/stack state would get logged.

Signed-off-by: Jan Beulich 
---
v2: Pass flag to fatal_trap().

--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -198,6 +198,14 @@ Permit Xen to use Address Space Identifi
 tags the TLB entries with an ID per vcpu.  This allows for guest TLB flushes
 to be performed without the overhead of a complete TLB flush.
 
+### async-show-all
+> `= `
+
+> Default: `false`
+
+Forces all CPUs' full state to be logged upon certain fatal asynchronous
+exceptions (watchdog NMIs and unexpected MCEs).
+
 ### ats
 > `= `
 
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -77,7 +77,7 @@ static void unexpected_machine_check(con
 {
 console_force_unlock();
 printk("Unexpected Machine Check Exception\n");
-fatal_trap(regs);
+fatal_trap(regs, 1);
 }
 
 
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -488,7 +488,7 @@ bool_t nmi_watchdog_tick(const struct cp
 console_force_unlock();
 printk("Watchdog timer detects that CPU%d is stuck!\n",
smp_processor_id());
-fatal_trap(regs);
+fatal_trap(regs, 1);
 }
 } 
 else 
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -525,6 +525,25 @@ void vcpu_show_execution_state(struct vc
 vcpu_unpause(v);
 }
 
+static cpumask_t show_state_mask;
+static bool_t opt_show_all;
+boolean_param("async-show-all", opt_show_all);
+
+static int nmi_show_execution_state(const struct cpu_user_regs *regs, int cpu)
+{
+if ( !cpumask_test_cpu(cpu, _state_mask) )
+return 0;
+
+if ( opt_show_all )
+show_execution_state(regs);
+else
+printk(XENLOG_ERR "CPU%d @ %04x:%08lx (%pS)\n", cpu, regs->cs, 
regs->rip,
+   guest_mode(regs) ? _p(regs->rip) : NULL);
+cpumask_clear_cpu(cpu, _state_mask);
+
+return 1;
+}
+
 static const char *trapstr(unsigned int trapnr)
 {
 static const char * const strings[] = {
@@ -544,7 +563,7 @@ static const char *trapstr(unsigned int
  * are disabled). In such situations we can't do much that is safe. We try to
  * print out some tracing and then we just spin.
  */
-void fatal_trap(const struct cpu_user_regs *regs)
+void fatal_trap(const struct cpu_user_regs *regs, bool_t show_remote)
 {
 static DEFINE_PER_CPU(char, depth);
 unsigned int trapnr = regs->entry_vector;
@@ -570,6 +589,15 @@ void fatal_trap(const struct cpu_user_re
 printk("Faulting linear address: %p\n", _p(cr2));
 show_page_walk(cr2);
 }
+if ( show_remote )
+{
+cpumask_andnot(_state_mask, _online_map,
+   cpumask_of(smp_processor_id()));
+set_nmi_callback(nmi_show_execution_state);
+smp_send_nmi_allbutself();
+while ( !cpumask_empty(_state_mask) )
+cpu_relax();
+}
 }
 
 panic("FATAL TRAP: vector = %d (%s)\n"
@@ -1711,7 +1739,7 @@ void do_page_fault(struct cpu_user_regs
 {
 console_start_sync();
 printk("Xen SM%cP violation\n", (pf_type == smep_fault) ? 'E' : 
'A');
-fatal_trap(regs);
+fatal_trap(regs, 0);
 }
 
 if ( pf_type != real_fault )
@@ -1782,7 +1810,7 @@ void __init do_early_page_fault(struct c
 console_start_sync();
 printk("Early fatal page fault at %04x:%p (cr2=%p, ec=%04x)\n",
regs->cs, _p(regs->eip), _p(cr2), regs->error_code);
-fatal_trap(regs);
+fatal_trap(regs, 0);
 }
 }
 
@@ -3598,7 +3626,7 @@ static void pci_serr_error(const struct
 default:  /* 'fatal' */
 console_force_unlock();
 printk("\n\nNMI - PCI system error (SERR)\n");
-fatal_trap(regs);
+fatal_trap(regs, 0);
 }
 }
 
@@ -3613,7 +3641,7 @@ static void io_check_error(const struct
 default:  /* 'fatal' */
 console_force_unlock();
 printk("\n\nNMI - I/O ERROR\n");
-fatal_trap(regs);
+fatal_trap(regs, 0);
 }
 
 outb((inb(0x61) & 0x0f) | 0x08, 0x61); /* clear-and-disable IOCK */
@@ -3633,7 +3661,7 @@ static void unknown_nmi_error(const stru
 console_force_unlock();
 printk("Uhhuh. NMI received for unknown reason %02x.\n", reason);
 printk("Do you have a strange power saving mode enabled?\n");
-fatal_trap(regs);
+fatal_trap(regs, 0);
 }
 }
 
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -558,6 +558,7 @@ 

Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Paul Durrant]

> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
> Juergen Gross
> Sent: 16 June 2016 11:02
> To: qemu-de...@nongnu.org; xen-de...@lists.xensource.com
> Cc: Anthony Perard; Juergen Gross; sstabell...@kernel.org;
> kra...@redhat.com
> Subject: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64
> word size mix
> 
> In case the word size of the domU and qemu running the qdisk backend
> differ BLKIF_OP_DISCARD will not work reliably, as the request
> structure in the ring have different layouts for different word size.
> 
> Correct this by copying the request structure in case of different
> word size element by element in the BLKIF_OP_DISCARD case, too.
> 
> Signed-off-by: Juergen Gross 

Would it not be better to re-import the canonical blkif header as a whole 
rather than cherry-picking like this? You'd need to post-process to maintain 
style and possibly change some names for compatibility etc. but probably 
nothing beyond what indent and a simple [s]ed script can do.
I did broadly the same thing to re-import the netif header into Linux recently.

  Paul

> ---
>  hw/block/xen_blkif.h | 20 ++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h
> index e3b133b..969112f 100644
> --- a/hw/block/xen_blkif.h
> +++ b/hw/block/xen_blkif.h
> @@ -26,6 +26,14 @@ struct blkif_x86_32_request {
>   blkif_sector_t sector_number;/* start sector idx on disk (r/w only)
> */
>   struct blkif_request_segment
> seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>  };
> +struct blkif_x86_32_request_discard {
> +uint8_toperation;/* BLKIF_OP_DISCARD */
> +uint8_tflag; /* nr_segments in request struct*/
> +blkif_vdev_t   handle;   /* only for read/write requests */
> +uint64_t   id;   /* private guest value, echoed in resp  */
> +blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
> +uint64_t   nr_sectors;   /* # of contiguous sectors to discard   */
> +};
>  struct blkif_x86_32_response {
>   uint64_tid;  /* copied from request */
>   uint8_t operation;   /* copied from request */
> @@ -44,6 +52,14 @@ struct blkif_x86_64_request {
>   blkif_sector_t sector_number;/* start sector idx on disk (r/w only)
> */
>   struct blkif_request_segment
> seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
>  };
> +struct blkif_x86_64_request_discard {
> +uint8_toperation;/* BLKIF_OP_DISCARD */
> +uint8_tflag; /* nr_segments in request struct*/
> +blkif_vdev_t   handle;   /* only for read/write requests */
> +uint64_t   __attribute__((__aligned__(8))) id;
> +blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
> +uint64_t   nr_sectors;   /* # of contiguous sectors to discard   */
> +};
>  struct blkif_x86_64_response {
>   uint64_t   __attribute__((__aligned__(8))) id;
>   uint8_t operation;   /* copied from request */
> @@ -82,7 +98,7 @@ static inline void blkif_get_x86_32_req(blkif_request_t
> *dst, blkif_x86_32_reque
>   /* Prevent the compiler from using src->... instead. */
>   barrier();
>   if (dst->operation == BLKIF_OP_DISCARD) {
> - struct blkif_request_discard *s = (void *)src;
> +struct blkif_x86_32_request_discard *s = (void *)src;
>   struct blkif_request_discard *d = (void *)dst;
>   d->nr_sectors = s->nr_sectors;
>   return;
> @@ -105,7 +121,7 @@ static inline void
> blkif_get_x86_64_req(blkif_request_t *dst, blkif_x86_64_reque
>   /* Prevent the compiler from using src->... instead. */
>   barrier();
>   if (dst->operation == BLKIF_OP_DISCARD) {
> - struct blkif_request_discard *s = (void *)src;
> +struct blkif_x86_64_request_discard *s = (void *)src;
>   struct blkif_request_discard *d = (void *)dst;
>   d->nr_sectors = s->nr_sectors;
>   return;
> --
> 2.6.6
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Yu Zhang]

On 6/16/2016 6:02 PM, Jan Beulich wrote:

@@ -94,8 +96,16 @@ static unsigned long p2m_type_to_flags(p2m_type_t t, mfn_t 
mfn,
default:
return flags | _PAGE_NX_BIT;
case p2m_grant_map_ro:
-case p2m_ioreq_server:
return flags | P2M_BASE_FLAGS | _PAGE_NX_BIT;
+case p2m_ioreq_server:
+{
+flags |= P2M_BASE_FLAGS | _PAGE_RW;
+
+if ( p2m->ioreq.flags & P2M_IOREQ_HANDLE_WRITE_ACCESS )
+return flags & ~_PAGE_RW;
+else
+return flags;
+}

Same here (for the missing _PAGE_NX) plus no need for braces.

I'll remove the brace. And we do not need to set the _PAGE_NX_BIT, like
the p2m_ram_ro case I guess.

I hope you mean the inverse: You should set _PAGE_NX_BIT here.

Oh, right. I meant the reverse. Thanks for the remind. :)
And I have a question,  here in p2m_type_to_flags(), I saw current code
uses _PAGE_NX_BIT
to disable the executable permission,  and I wonder, why don't we choose
the _PAGE_NX,
which is defined as:

#define _PAGE_NX   (cpu_has_nx ? _PAGE_NX_BIT : 0)

How do we know for sure that bit 63 from pte is not a reserved one
without checking
the cpu capability(the cpu_has_nx)? Is there any other reasons, i.e. the
page tables might
be shared with IOMMU?

Please wait for Andrew to confirm this (or correct me) - there are
some differences between AMD and Intel, and iirc the bit gets
ignored by AMD when NX is off.


+struct xen_hvm_map_mem_type_to_ioreq_server {
+domid_t domid;  /* IN - domain to be serviced */
+ioservid_t id;  /* IN - ioreq server id */
+uint16_t type;  /* IN - memory type */
+uint16_t pad;

This field does not appear to get checked in the handler.

I am now wondering, how about we remove this pad field and define type
as uint32_t?

As above - I think the current layout is fine. But I'm also not heavily
opposed to using uint32_t here. It's not a stable interface anyway
(and I already have a series mostly ready to split off all control
operations from the HVMOP_* ones, into a new HVMCTL_* set,
which will make all of them interface-versioned).

I'd like to keep this interface. BTW, you mentioned "this field does not
appear to
get checked in the handler", do you mean we need to check the pad in the
handler?

Yes.


And why?

In order to be able to later assign meaning to it without breaking
existing users.


So the handler need to assure the pad is 0, right?

Thanks
Yu

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

Re: [Xen-devel] [PATCH RESEND 00/14] Xen ARM DomU ACPI support

[Wei Liu]

On Tue, Jun 07, 2016 at 12:00:26PM +0100, Julien Grall wrote:
> Hello Shannon,
> 
> On 07/06/16 02:07, Shannon Zhao wrote:
> >On 2016/6/6 23:48, Julien Grall wrote:
> >>On 06/06/16 13:26, Wei Liu wrote:
> >>>On Tue, May 31, 2016 at 01:02:52PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> The design of this feature is described as below.
> Firstly, the toolstack (libxl) generates the ACPI tables according the
> number of vcpus and gic controller.
> 
> Then, it copies these ACPI tables to DomU memory space and passes
> them to UEFI firmware through the "ARM multiboot" protocol.
> 
> At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
> and installs these tables like the usual way and passes both ACPI and DT
> information to the Xen DomU.
> 
> Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
> since it's enough now.
> 
> This has been tested using guest kernel with the Dom0 ACPI support
> patches which could be fetched from:
> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
> 
> 
> Shannon Zhao (14):
> libxl/arm: Fix the function name in error log
> libxl/arm: Factor out codes for generating DTB
> libxc: Add placeholders for ACPI tables blob and size
> tools: add ACPI tables relevant definitions
> libxl/arm: Construct ACPI GTDT table
> libxl/arm: Construct ACPI FADT table
> libxl/arm: Construct ACPI DSDT table
> libxl/arm: Construct ACPI MADT table
> libxl/arm: Construct ACPI XSDT table
> libxl/arm: Construct ACPI RSDP table
> libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ
> libxl/arm: Add ACPI module
> libxl/arm: initialize memory information of ACPI blob
> libxc/xc_dom_core: Copy ACPI tables to guest memory space
> 
> >>>
> >>>After going through this series, I think the code mostly looks good.
> >>>
> >>>There is one higher level question: you seem to have put a lot of the
> >>>table construction code in libxl, while I failed to see why specific
> >>>libxl information is needed. Have you considered moving the code to
> >>>libxc?
> >>
> >>I would rather avoid to have the device tree built by libxl and ACPI
> >>tables built by libxc.
> >>
> >>I don't remember why we have decided to implement DT building in libxl,
> >>I guess it is because we need to know which GIC version is used (GICv2
> >>vs GICv3).
> >>
> >>In the long run, we might also need to have more part of the firmware
> >>configurable (performance monitor support, memory layout, UART...).
> >>
> >>Most of those information are available easily in libxl, we would to
> >>export them of libxc.
> >Yes, and one more reason is that to support ACPI, it also needs to add
> >the ACPI multiboot module in DT.
> 
> I don't consider this as a reason for building the ACPI tables in libxl. I
> am more concerned about building the firmwares in different place.
> 
> If we decide to build the ACPI tables in libxc, then the code to build the
> device tree should move in libxc too.
> 

I've read this sub-thread and the other thread in which Boris replied, I
think due to the fact that libxl has more information at hand and libxc
is intended to be simple, I think having the building code in libxl
makes sense.

Shannon, Boris and Julien, what do you guys think? Can we agree on
something so that Shannon can move on with next iteration?

Wei.

> Regards,
> 
> -- 
> Julien Grall

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

Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Jan Beulich]

>>> On 16.06.16 at 13:04,  wrote:
> On 16/06/16 12:54, Jan Beulich wrote:
> On 16.06.16 at 12:02,  wrote:
>>> In case the word size of the domU and qemu running the qdisk backend
>>> differ BLKIF_OP_DISCARD will not work reliably, as the request
>>> structure in the ring have different layouts for different word size.
>>>
>>> Correct this by copying the request structure in case of different
>>> word size element by element in the BLKIF_OP_DISCARD case, too.
>>>
>>> Signed-off-by: Juergen Gross 
>> 
>> With the indentation (tabs vs blanks) fixed
> 
> Hmm, qemu coding style is to use blanks. I could:
> a) leave the patch as is (changed lines indented with blanks)
> b) use tabs to indent (style of the modified file up to now)
> c) change the style of the file in this patch
> d) change the style of the file in a separate patch
> 
> Any preferences?

I'd say a) is not an option, but beyond that I think this needs to be
answered by the qemu maintainers.

Jan


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

Re: [Xen-devel] [PATCH v2 2/2] xen: add update indicator to vcpu_runstate_info

[Jan Beulich]

>>> On 15.06.16 at 13:34,  wrote:
> In order to support reading another vcpu's mapped vcpu_runstate_info
> an indicator for an occurring update of the runstate information is
> needed.
> 
> Add the possibility to activate setting this indicator in the highest
> bit of state_entry_time via a vm_assist hypercall. When activated the
> update indicator will be set before the runstate information is
> modified in guest memory and it will be reset after modification is
> done. As state_entry_time is guaranteed to be different after each
> update the guest can detect any update (either in progress or while
> reading the runstate data) by comparing state_entry_time before and
> after reading runstate data: in case the values differ or the update
> indicator was set the data might be inconsistent and should be reread.

Neither here nor in the cover letter you mention why this is useful
to have (and to be honest I did forget what you need this for since
the time the original discussion happened), yet that's quite relevant
since for many years we lived happily without it.

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1926,12 +1926,35 @@ bool_t update_runstate_area(struct vcpu *v)
>  {
>  bool_t rc;
>  smap_check_policy_t smap_policy;
> +void __user *guest_handle = NULL;
> +unsigned off = 0;
>  
>  if ( guest_handle_is_null(runstate_guest(v)) )
>  return 1;
>  
>  smap_policy = smap_policy_change(v, SMAP_CHECK_ENABLED);
>  
> +if ( VM_ASSIST(v->domain, runstate_update_flag) )
> +{
> +off = offsetof(struct vcpu_runstate_info, state_entry_time) +
> +  sizeof(v->runstate.state_entry_time) - 1;
> +if ( has_32bit_shinfo(v->domain) )
> +{
> +guest_handle = v->runstate_guest.compat.p;
> +guest_handle +=
> +offsetof(struct compat_vcpu_runstate_info, state_entry_time) 
> +
> +sizeof(v->runstate.state_entry_time) - 1;

The sizes of the native and compat fields happen to be the same,
but it would be nice if the right field/type could be used here.

> --- a/xen/include/public/vcpu.h
> +++ b/xen/include/public/vcpu.h
> @@ -84,6 +84,12 @@ struct vcpu_runstate_info {
>  /* When was current state entered (system time, ns)? */
>  uint64_t state_entry_time;
>  /*
> + * Update indicator set in state_entry_time:
> + * When activated via VMASST_TYPE_runstate_update_flag, set during
> + * updates in guest memory mapped copy of vcpu_runstate_info.
> + */
> +#define XEN_RUNSTATE_UPDATE  (1ULL << 63)

I continue to be not particularly happy with the C99ism here, but
I see we have a few other instances of such already in the public
headers.

Jan


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

Re: [Xen-devel] Xen Raspberry Pi3 or Ordoid C2

[Julien Grall]

On 16/06/16 12:04, sarah jones wrote:

Hi,


Hello,


i'm looking for a detailed tutorial on how to install Xen on a Raspberry
Pi3 or .Ordoid C2.


I am not aware of any tutorial of one of these boards.

Regards,

--
Julien Grall

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

Re: [Xen-devel] Xen on Hikey Board

[Julien Grall]

On 16/06/16 11:59, Fadwa Messaoudi wrote:

Hi,


Hello,


i m looking for a detailed tutorial on how to install Xen on a the Hikey
Board.


You can give a look to Xen wiki:

http://wiki.xenproject.org/wiki/HiKey

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RESEND 03/14] libxc: Add placeholders for ACPI tables blob and size

[Wei Liu]

On Thu, Jun 16, 2016 at 06:53:37PM +0800, Shannon Zhao wrote:
> 
> 
> On 2016/6/16 18:49, Wei Liu wrote:
> > On Tue, Jun 07, 2016 at 08:32:26PM +0800, Shannon Zhao wrote:
> > [...]
> >> > That's its own mechanism I think and UEFI wants all the memory maps
> >> > under its control. And it's same for QEMU(x86 and ARM) and also for Xen
> >> > on x86. You can have a look at the OvmfPkg/AcpiPlatformDxe/Xen.c which
> >> > is used for x86 Xen DomU.
> >> > 
> > I don't think Xen relocates ACPI tables in OVMF. The code scans for
> > signature in code.
> Maybe you can have a look at the function InstallXenTables().
> 

Oh, I thought you were talking about Xen specific code. That's my
misunderstanding.

So that function (InstallXenTables) calls InstallAcpiTable, which
eventually calls AcpiProtocol->InstallAcpiTable.

Does it imply "UEFI wants all memory maps under its control"? Does it
imply UEFI will relocate those tables (or keep a copy to itself)?  The
answer "implementation specific" or "not sure" is not good enough to
address Julien's concern.

Maybe this is just due to miscommunication or using the wrong term. I
will refrain from commenting further now because the issue is quite ARM
specific. Don't want to distract you guys further.

Wei.

> Thanks,
> -- 
> Shannon
> 

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

Re: [Xen-devel] [PATCH RESEND 03/14] libxc: Add placeholders for ACPI tables blob and size

[Julien Grall]

Hello Shannon,

On 16/06/16 11:45, Shannon Zhao wrote:

On 2016/6/7 21:06, Julien Grall wrote:

That's its own mechanism I think and UEFI wants all the memory maps
under its control. And it's same for QEMU(x86 and ARM) and also for Xen
on x86. You can have a look at the OvmfPkg/AcpiPlatformDxe/Xen.c which
is used for x86 Xen DomU.


UEFI cannot control the memory map because it is not capable to know
what a region is used for (such as magic pages, grant table...).

In the case of domU for x86, the ACPI table are located in predefine
physical address by hvmloader.

The truth is that hvmloader puts the tables at address
ACPI_PHYSICAL_ADDRESS(0x000EA020), but UEFI will install the tables and
relocate them as well. You can see OvmfPkg/AcpiPlatformDxe/Xen.c in edk2
source code.

So I think it's same with ARM except x86 puts the tables at one fixed
address while ARM dynamically computes the address and passes the
address information to UEFI through dts.

Yeah, of course we can let ARM put the tables at one fixed address and
also it doesn't need the ACPI module to pass the address and let UEFI
find the RSDP table from the fixed address. But what's the difference
between the fixed and dynamicall ones? Because both ways UEFI will
install and relocate the tables.


You seem to think that OVMF is the only UEFI implementation available
and its behavior is set in stone. Can you quote the UEFI spec stating
the ACPI tables will always be relocated?

If not, putting the ACPI module right in the middle of memory is not the
wisest choice because the tables can not be easily relocated like other
modules (DT, Kernel, initramfs).


The ACPI tables are put after the DT. How could you say the ACPI is in
the middle while DT not?


Because the position of the DT in the memory is defined by the Linux 
boot ABI (see section 4.b in Documentation/arm/Booting).





My suggestion to put the ACPI tables at a static address outside the RAM
is to let the choice to the firmware to do whatever it wants without
impacting the performance of a kernel if it ever decides to keep in
place the tables.

However, this static address is from the toolstack point of view. The
firmware should not assume any static address because the guest memory
layout is not part of the ABI for Xen ARM (i.e it can be modified
between two releases). This is to allow us reshuffling the layout to
make space for new features.


Sure? Currently for Xen x86, edk2 uses XEN_ACPI_PHYSICAL_ADDRESS to find
RSDP. If we put the tables at another address, DomU will fail to boot.


I am not sure to follow here. Why do you mention x86 here? My point was 
only for ARM and we are not tight to what x86 does.







I really don't see any reason that would
prevent us to do the same on ARM.

If the UEFI firmware wants to relocate the tables, then fine. But we
should also think about any firmware which may not relocate the tables.

Can you name one firmware except the UEFI?


Sorry I meant any other UEFI implementation (i.e other than OVMF) may
not relocate the tables.

So please tell me the name of other UEFI implementation.


What is the point? All our decision should be based on the specification 
and not ONE specific implementation.


Regards,

--
Julien Grall

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

[Xen-devel] Xen Raspberry Pi3 or Ordoid C2

[sarah jones]

Hi,
i'm looking for a detailed tutorial on how to install Xen on a Raspberry
Pi3 or .Ordoid C2.
Any information can help me a lot
Best Regards.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Juergen Gross]

On 16/06/16 12:54, Jan Beulich wrote:
 On 16.06.16 at 12:02,  wrote:
>> In case the word size of the domU and qemu running the qdisk backend
>> differ BLKIF_OP_DISCARD will not work reliably, as the request
>> structure in the ring have different layouts for different word size.
>>
>> Correct this by copying the request structure in case of different
>> word size element by element in the BLKIF_OP_DISCARD case, too.
>>
>> Signed-off-by: Juergen Gross 
> 
> With the indentation (tabs vs blanks) fixed

Hmm, qemu coding style is to use blanks. I could:
a) leave the patch as is (changed lines indented with blanks)
b) use tabs to indent (style of the modified file up to now)
c) change the style of the file in this patch
d) change the style of the file in a separate patch

Any preferences?

> Reviewed-by: Jan Beulich 
> 
> And maybe ...
> 
>> @@ -82,7 +98,7 @@ static inline void blkif_get_x86_32_req(blkif_request_t 
>> *dst, blkif_x86_32_reque
>>  /* Prevent the compiler from using src->... instead. */
>>  barrier();
>>  if (dst->operation == BLKIF_OP_DISCARD) {
>> -struct blkif_request_discard *s = (void *)src;
>> +struct blkif_x86_32_request_discard *s = (void *)src;
> 
> ... it would also be worth adding const here and ...
> 
>> @@ -105,7 +121,7 @@ static inline void blkif_get_x86_64_req(blkif_request_t 
>> *dst, blkif_x86_64_reque
>>  /* Prevent the compiler from using src->... instead. */
>>  barrier();
>>  if (dst->operation == BLKIF_OP_DISCARD) {
>> -struct blkif_request_discard *s = (void *)src;
>> +struct blkif_x86_64_request_discard *s = (void *)src;
> 
> ... here.

Okay.


Juergen

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

[Xen-devel] Xen on Hikey Board

[Fadwa Messaoudi]

Hi,
i m looking for a detailed tutorial on how to install Xen on a the Hikey
Board.

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

Re: [Xen-devel] [PATCH RESEND 03/14] libxc: Add placeholders for ACPI tables blob and size

[Shannon Zhao]

On 2016/6/16 18:49, Wei Liu wrote:
> On Tue, Jun 07, 2016 at 08:32:26PM +0800, Shannon Zhao wrote:
> [...]
>> > That's its own mechanism I think and UEFI wants all the memory maps
>> > under its control. And it's same for QEMU(x86 and ARM) and also for Xen
>> > on x86. You can have a look at the OvmfPkg/AcpiPlatformDxe/Xen.c which
>> > is used for x86 Xen DomU.
>> > 
> I don't think Xen relocates ACPI tables in OVMF. The code scans for
> signature in code.
Maybe you can have a look at the function InstallXenTables().

Thanks,
-- 
Shannon


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

Re: [Xen-devel] Xen on Dragonboard 410C

[Julien Grall]

On 15/06/16 16:11, Fadwa Messaoudi wrote:

Hi,


Hello,


i'm working on the dragonboard trying to install Xen on it, do you have
any documentation to help me and ease my work.


I am not aware of any tutorial on how to install Xen on this board.

After looking at the hardware specification of this board, it looks like 
Xen does not have the UART driver for this board. So you would need at 
least to write (or port from Linux) for Xen.


You can find more information about Xen ARM on the Xen wiki:

http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions

Feel free to ask more details.

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Jan Beulich]

>>> On 16.06.16 at 12:02,  wrote:
> In case the word size of the domU and qemu running the qdisk backend
> differ BLKIF_OP_DISCARD will not work reliably, as the request
> structure in the ring have different layouts for different word size.
> 
> Correct this by copying the request structure in case of different
> word size element by element in the BLKIF_OP_DISCARD case, too.
> 
> Signed-off-by: Juergen Gross 

With the indentation (tabs vs blanks) fixed
Reviewed-by: Jan Beulich 

And maybe ...

> @@ -82,7 +98,7 @@ static inline void blkif_get_x86_32_req(blkif_request_t 
> *dst, blkif_x86_32_reque
>   /* Prevent the compiler from using src->... instead. */
>   barrier();
>   if (dst->operation == BLKIF_OP_DISCARD) {
> - struct blkif_request_discard *s = (void *)src;
> +struct blkif_x86_32_request_discard *s = (void *)src;

... it would also be worth adding const here and ...

> @@ -105,7 +121,7 @@ static inline void blkif_get_x86_64_req(blkif_request_t 
> *dst, blkif_x86_64_reque
>   /* Prevent the compiler from using src->... instead. */
>   barrier();
>   if (dst->operation == BLKIF_OP_DISCARD) {
> - struct blkif_request_discard *s = (void *)src;
> +struct blkif_x86_64_request_discard *s = (void *)src;

... here.

Jan


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

Re: [Xen-devel] tutorial of Xen on Dragon Board

[Julien Grall]

On 16/06/16 10:12, sarah jones wrote:

Hi,


Hello Sarah,


i couldn't find any tutorials on how to install Xen on a Dragon Board
Could you help me please with the steps to get this done


I am not aware of any tutorial on how to install Xen on this board.

After looking at the hardware specification of this board, it looks like 
Xen does not have the UART driver for this board. So you would need at 
least to write (or port from Linux) for Xen.


You can find more information about Xen ARM on the Xen wiki:

http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions

Feel free to ask more details.

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH RESEND 03/14] libxc: Add placeholders for ACPI tables blob and size

[Wei Liu]

On Tue, Jun 07, 2016 at 08:32:26PM +0800, Shannon Zhao wrote:
[...]
> That's its own mechanism I think and UEFI wants all the memory maps
> under its control. And it's same for QEMU(x86 and ARM) and also for Xen
> on x86. You can have a look at the OvmfPkg/AcpiPlatformDxe/Xen.c which
> is used for x86 Xen DomU.
> 

I don't think Xen relocates ACPI tables in OVMF. The code scans for
signature in code.

I also don't quite get the "control memory map" part. OVMF doesn't
update the memory map, it accepts the memory map provided by hvmloader
as-is. See OvmfPkg/PlatformPei/Xen.c:XenGetE820Map.

Note that I'm talking about this in a pretty out of context way, so I
could be misunderstading what you said.

> > Anyway, you rely on the UEFI firmware to always relocating the tables.
> > What would happen if they decide to remove this behavior?
> Eh, I don't believe this would happen.
> 

I think we can only rely on the spec to be sure.

Wei.

> -- 
> Shannon
> 

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

Re: [Xen-devel] [PATCH RESEND 03/14] libxc: Add placeholders for ACPI tables blob and size

[Shannon Zhao]

On 2016/6/16 18:21, Julien Grall wrote:
> 
> 
> On 16/06/16 07:53, Shannon Zhao wrote:
>> Hi Julien,
> 
> Hi Shannon,
> 
>>
>> On 2016/6/7 21:06, Julien Grall wrote:
 That's its own mechanism I think and UEFI wants all the memory maps
 under its control. And it's same for QEMU(x86 and ARM) and also for Xen
 on x86. You can have a look at the OvmfPkg/AcpiPlatformDxe/Xen.c which
 is used for x86 Xen DomU.
>>>
>>> UEFI cannot control the memory map because it is not capable to know
>>> what a region is used for (such as magic pages, grant table...).
>>>
>>> In the case of domU for x86, the ACPI table are located in predefine
>>> physical address by hvmloader.
>> The truth is that hvmloader puts the tables at address
>> ACPI_PHYSICAL_ADDRESS(0x000EA020), but UEFI will install the tables and
>> relocate them as well. You can see OvmfPkg/AcpiPlatformDxe/Xen.c in edk2
>> source code.
>>
>> So I think it's same with ARM except x86 puts the tables at one fixed
>> address while ARM dynamically computes the address and passes the
>> address information to UEFI through dts.
>>
>> Yeah, of course we can let ARM put the tables at one fixed address and
>> also it doesn't need the ACPI module to pass the address and let UEFI
>> find the RSDP table from the fixed address. But what's the difference
>> between the fixed and dynamicall ones? Because both ways UEFI will
>> install and relocate the tables.
> 
> You seem to think that OVMF is the only UEFI implementation available
> and its behavior is set in stone. Can you quote the UEFI spec stating
> the ACPI tables will always be relocated?
> 
> If not, putting the ACPI module right in the middle of memory is not the
> wisest choice because the tables can not be easily relocated like other
> modules (DT, Kernel, initramfs).
> 
The ACPI tables are put after the DT. How could you say the ACPI is in
the middle while DT not?

> My suggestion to put the ACPI tables at a static address outside the RAM
> is to let the choice to the firmware to do whatever it wants without
> impacting the performance of a kernel if it ever decides to keep in
> place the tables.
> 
> However, this static address is from the toolstack point of view. The
> firmware should not assume any static address because the guest memory
> layout is not part of the ABI for Xen ARM (i.e it can be modified
> between two releases). This is to allow us reshuffling the layout to
> make space for new features.
> 
Sure? Currently for Xen x86, edk2 uses XEN_ACPI_PHYSICAL_ADDRESS to find
RSDP. If we put the tables at another address, DomU will fail to boot.

>>
>>> I really don't see any reason that would
>>> prevent us to do the same on ARM.
>>>
>>> If the UEFI firmware wants to relocate the tables, then fine. But we
>>> should also think about any firmware which may not relocate the tables.
>> Can you name one firmware except the UEFI?
> 
> Sorry I meant any other UEFI implementation (i.e other than OVMF) may
> not relocate the tables.
So please tell me the name of other UEFI implementation.

Thanks,
-- 
Shannon


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

[Xen-devel] [ANNOUNCEMENT] Xen 4.7 RC6

[Wei Liu]

Hi all

Xen 4.7 rc6 is tagged. You can check that out from xen.git:

  git://xenbits.xen.org/xen.git 4.7.0-rc6

For you convenience there is also tarball at:
http://bits.xensource.com/oss-xen/release/4.7.0-rc6/xen-4.7.0-rc6.tar.gz

And the signature is at:
http://bits.xensource.com/oss-xen/release/4.7.0-rc6/xen-4.7.0-rc6.tar.gz.sig

Please send bug reports and test reports to
xen-de...@lists.xenproject.org. When sending bug reports, please CC
relevant maintainers and me (wei.l...@citrix.com).

Thanks
Wei.

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

Re: [Xen-devel] [PATCH RESEND 03/14] libxc: Add placeholders for ACPI tables blob and size

[Julien Grall]

On 16/06/16 07:53, Shannon Zhao wrote:

Hi Julien,


Hi Shannon,



On 2016/6/7 21:06, Julien Grall wrote:

That's its own mechanism I think and UEFI wants all the memory maps
under its control. And it's same for QEMU(x86 and ARM) and also for Xen
on x86. You can have a look at the OvmfPkg/AcpiPlatformDxe/Xen.c which
is used for x86 Xen DomU.


UEFI cannot control the memory map because it is not capable to know
what a region is used for (such as magic pages, grant table...).

In the case of domU for x86, the ACPI table are located in predefine
physical address by hvmloader.

The truth is that hvmloader puts the tables at address
ACPI_PHYSICAL_ADDRESS(0x000EA020), but UEFI will install the tables and
relocate them as well. You can see OvmfPkg/AcpiPlatformDxe/Xen.c in edk2
source code.

So I think it's same with ARM except x86 puts the tables at one fixed
address while ARM dynamically computes the address and passes the
address information to UEFI through dts.

Yeah, of course we can let ARM put the tables at one fixed address and
also it doesn't need the ACPI module to pass the address and let UEFI
find the RSDP table from the fixed address. But what's the difference
between the fixed and dynamicall ones? Because both ways UEFI will
install and relocate the tables.


You seem to think that OVMF is the only UEFI implementation available 
and its behavior is set in stone. Can you quote the UEFI spec stating 
the ACPI tables will always be relocated?


If not, putting the ACPI module right in the middle of memory is not the 
wisest choice because the tables can not be easily relocated like other 
modules (DT, Kernel, initramfs).


My suggestion to put the ACPI tables at a static address outside the RAM 
is to let the choice to the firmware to do whatever it wants without 
impacting the performance of a kernel if it ever decides to keep in 
place the tables.


However, this static address is from the toolstack point of view. The 
firmware should not assume any static address because the guest memory 
layout is not part of the ABI for Xen ARM (i.e it can be modified 
between two releases). This is to allow us reshuffling the layout to 
make space for new features.





I really don't see any reason that would
prevent us to do the same on ARM.

If the UEFI firmware wants to relocate the tables, then fine. But we
should also think about any firmware which may not relocate the tables.

Can you name one firmware except the UEFI?


Sorry I meant any other UEFI implementation (i.e other than OVMF) may 
not relocate the tables.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH raisin v3] Update to 4.7, update qemu and qemu_traditional recipes

[George Dunlap]

On 16/06/16 10:52, Stefano Stabellini wrote:
> On Wed, 15 Jun 2016, George Dunlap wrote:
>> Add a 4.7 config file, make it the default.
>>
>> Also update the qemu and qemu_traditional recipies after Ian Cambell's
>> work to split off separate libraries.
>>
>> Signed-off-by: George Dunlap 
> 
> Acked-by: Stefano Stabellini 

Great, thanks.
 -George


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

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Jan Beulich]

>>> On 16.06.16 at 11:32,  wrote:
>> On 6/14/2016 6:45 PM, Jan Beulich wrote:
>>> On 19.05.16 at 11:05,  wrote:
> @@ -914,6 +916,45 @@ int hvm_unmap_io_range_from_ioreq_server(struct 
> domain
>>> *d, ioservid_t id,
>return rc;
>}
>
> +int hvm_map_mem_type_to_ioreq_server(struct domain *d, ioservid_t id,
> + uint16_t type, uint32_t flags)
 I see no reason why both can't be unsigned int.
>>> Parameter type is passed in from the type field inside struct
>>> xen_hvm_map_mem_type_to_ioreq_server,
>>> which is a uint16_t, followed with a uint16_t pad. Now I am wondering,
>>> may be we can just remove the pad
>>> field in this structure and just define type as uint32_t.
>> I think keeping the interface structure unchanged is the desirable
>> route here. What I dislike is the passing around of non-natural
>> width types, which is more expensive in terms of processing. I.e.
>> as long as a fixed width type (which is necessary to be used in
>> the public interface) fits in "unsigned int", that should be the
>> respective internal type. Otherwise "unsigned long" etc.
>>
>> There are cases where even internally we indeed want to use
>> fixed width types, and admittedly there are likely far more cases
>> where internally fixed width types get used without good reason,
>> but just like everywhere else - let's please not make this worse.
>> IOW please use fixed width types only when you really need them.
> OK. I can keep the interface, and using uint32_t type in the internal 
> routine
> would means a implicit type conversion from uint16_6, which I do not think
> is a problem.

Just to reiterate: Unless there is a specific need, please avoid
fixed width integer types for any internal use.

> @@ -94,8 +96,16 @@ static unsigned long p2m_type_to_flags(p2m_type_t t, 
> mfn_t mfn,
>default:
>return flags | _PAGE_NX_BIT;
>case p2m_grant_map_ro:
> -case p2m_ioreq_server:
>return flags | P2M_BASE_FLAGS | _PAGE_NX_BIT;
> +case p2m_ioreq_server:
> +{
> +flags |= P2M_BASE_FLAGS | _PAGE_RW;
> +
> +if ( p2m->ioreq.flags & P2M_IOREQ_HANDLE_WRITE_ACCESS )
> +return flags & ~_PAGE_RW;
> +else
> +return flags;
> +}
 Same here (for the missing _PAGE_NX) plus no need for braces.
>>> I'll remove the brace. And we do not need to set the _PAGE_NX_BIT, like
>>> the p2m_ram_ro case I guess.
>> I hope you mean the inverse: You should set _PAGE_NX_BIT here.
> Oh, right. I meant the reverse. Thanks for the remind. :)
> And I have a question,  here in p2m_type_to_flags(), I saw current code 
> uses _PAGE_NX_BIT
> to disable the executable permission,  and I wonder, why don't we choose 
> the _PAGE_NX,
> which is defined as:
> 
> #define _PAGE_NX   (cpu_has_nx ? _PAGE_NX_BIT : 0)
> 
> How do we know for sure that bit 63 from pte is not a reserved one 
> without checking
> the cpu capability(the cpu_has_nx)? Is there any other reasons, i.e. the 
> page tables might
> be shared with IOMMU?

Please wait for Andrew to confirm this (or correct me) - there are
some differences between AMD and Intel, and iirc the bit gets
ignored by AMD when NX is off.

> +struct xen_hvm_map_mem_type_to_ioreq_server {
> +domid_t domid;  /* IN - domain to be serviced */
> +ioservid_t id;  /* IN - ioreq server id */
> +uint16_t type;  /* IN - memory type */
> +uint16_t pad;
 This field does not appear to get checked in the handler.
>>> I am now wondering, how about we remove this pad field and define type
>>> as uint32_t?
>> As above - I think the current layout is fine. But I'm also not heavily
>> opposed to using uint32_t here. It's not a stable interface anyway
>> (and I already have a series mostly ready to split off all control
>> operations from the HVMOP_* ones, into a new HVMCTL_* set,
>> which will make all of them interface-versioned).
> 
> I'd like to keep this interface. BTW, you mentioned "this field does not 
> appear to
> get checked in the handler", do you mean we need to check the pad in the 
> handler?

Yes.

> And why?

In order to be able to later assign meaning to it without breaking
existing users.

Jan

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

[Xen-devel] [PATCH] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Juergen Gross]

In case the word size of the domU and qemu running the qdisk backend
differ BLKIF_OP_DISCARD will not work reliably, as the request
structure in the ring have different layouts for different word size.

Correct this by copying the request structure in case of different
word size element by element in the BLKIF_OP_DISCARD case, too.

Signed-off-by: Juergen Gross 
---
 hw/block/xen_blkif.h | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h
index e3b133b..969112f 100644
--- a/hw/block/xen_blkif.h
+++ b/hw/block/xen_blkif.h
@@ -26,6 +26,14 @@ struct blkif_x86_32_request {
blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
 };
+struct blkif_x86_32_request_discard {
+uint8_toperation;/* BLKIF_OP_DISCARD */
+uint8_tflag; /* nr_segments in request struct*/
+blkif_vdev_t   handle;   /* only for read/write requests */
+uint64_t   id;   /* private guest value, echoed in resp  */
+blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
+uint64_t   nr_sectors;   /* # of contiguous sectors to discard   */
+};
 struct blkif_x86_32_response {
uint64_tid;  /* copied from request */
uint8_t operation;   /* copied from request */
@@ -44,6 +52,14 @@ struct blkif_x86_64_request {
blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
 };
+struct blkif_x86_64_request_discard {
+uint8_toperation;/* BLKIF_OP_DISCARD */
+uint8_tflag; /* nr_segments in request struct*/
+blkif_vdev_t   handle;   /* only for read/write requests */
+uint64_t   __attribute__((__aligned__(8))) id;
+blkif_sector_t sector_number;/* start sector idx on disk (r/w only)  */
+uint64_t   nr_sectors;   /* # of contiguous sectors to discard   */
+};
 struct blkif_x86_64_response {
uint64_t   __attribute__((__aligned__(8))) id;
uint8_t operation;   /* copied from request */
@@ -82,7 +98,7 @@ static inline void blkif_get_x86_32_req(blkif_request_t *dst, 
blkif_x86_32_reque
/* Prevent the compiler from using src->... instead. */
barrier();
if (dst->operation == BLKIF_OP_DISCARD) {
-   struct blkif_request_discard *s = (void *)src;
+struct blkif_x86_32_request_discard *s = (void *)src;
struct blkif_request_discard *d = (void *)dst;
d->nr_sectors = s->nr_sectors;
return;
@@ -105,7 +121,7 @@ static inline void blkif_get_x86_64_req(blkif_request_t 
*dst, blkif_x86_64_reque
/* Prevent the compiler from using src->... instead. */
barrier();
if (dst->operation == BLKIF_OP_DISCARD) {
-   struct blkif_request_discard *s = (void *)src;
+struct blkif_x86_64_request_discard *s = (void *)src;
struct blkif_request_discard *d = (void *)dst;
d->nr_sectors = s->nr_sectors;
return;
-- 
2.6.6


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

Re: [Xen-devel] [PATCH RESEND 07/14] libxl/arm: Construct ACPI DSDT table

[Stefano Stabellini]

On Thu, 16 Jun 2016, Julien Grall wrote:
> Hi Shannon,
> 
> On 16/06/16 07:25, Shannon Zhao wrote:
> > 
> > 
> > On 2016/6/7 21:42, Julien Grall wrote:
> > > Hello Shannon,
> > > 
> > > On 31/05/16 06:02, Shannon Zhao wrote:
> > > > From: Shannon Zhao 
> > > > 
> > > > Currently there is only the table header in DSDT table.
> > > > 
> > > > Signed-off-by: Shannon Zhao 
> > > > ---
> > > >tools/libxl/libxl_arm.c | 15 +++
> > > >1 file changed, 15 insertions(+)
> > > > 
> > > > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> > > > index c3b8fb4..7949635 100644
> > > > --- a/tools/libxl/libxl_arm.c
> > > > +++ b/tools/libxl/libxl_arm.c
> > > > @@ -944,6 +944,20 @@ static void make_acpi_fadt(libxl__gc *gc, struct
> > > > xc_dom_image *dom)
> > > >dom->acpitable_size += dom->acpitable_blob->fadt.size;
> > > >}
> > > > 
> > > > +static void make_acpi_dsdt(libxl__gc *gc, struct xc_dom_image *dom)
> > > > +{
> > > > +struct acpi_table_header *dsdt;
> > > > +
> > > > +/* Currently there is only the table header in DSDT table. */
> > > 
> > > What about the processor device objects?
> > 
> > Ah, yes. It should add processor device objects. Since DSDT table is
> > special, what's your suggestion about how to generate DSDT table? Use
> > static table which X86 use(tools/firmware/hvmloader/acpi/mk_dsdt.c) or
> > import what QEMU uses?
> 
> I would do the choice depending on what we need to expose through the DTST. If
> it is only dummy processor containers, then ~1500 lines of code (see
> qemu/hw/acpi/aml-build.c) seems a bit overkill.
> 
> > I prefer the latter, but this will add more codes to libxl.
> 
> What would be the benefits to build dynamically those tables?

Indeed, this is the right question. If we can get away with static
tables like on x86, we should. Less code is always better than more code
:-)

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

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Jan Beulich]

>>> On 16.06.16 at 11:30,  wrote:
> On 6/15/2016 6:21 PM, Jan Beulich wrote:
> On 15.06.16 at 11:50,  wrote:
>>> On 14/06/16 14:31, Jan Beulich wrote:
>>> On 14.06.16 at 15:13,  wrote:
> On 14/06/16 11:45, Jan Beulich wrote:
>> Locking is somewhat strange here: You protect against the "set"
>> counterpart altering state while you retrieve it, but you don't
>> protect against the returned data becoming stale by the time
>> the caller can consume it. Is that not a problem? (The most
>> concerning case would seem to be a race of hvmop_set_mem_type()
>> with de-registration of the type.)
> How is that different than calling set_mem_type() first, and then
> de-registering without first unmapping all the types?
 Didn't we all agree this is something that should be disallowed
 anyway (not that I've seen this implemented, i.e. just being
 reminded of it by your reply)?
>>> I think I suggested it as a good idea, but Paul and Yang both thought it
>>> wasn't necessary.  Do you think it should be a requirement?
>> I think things shouldn't be left in a half-adjusted state.
>>
>>> We could have the de-registering operation fail in those circumstances;
>>> but probably a more robust thing to do would be to have Xen go change
>>> all the ioreq_server entires back to ram_rw (since if the caller just
>>> ignores the failure, things are in an even worse state).
>> If that's reasonable to do without undue delay (e.g. by using
>> the usual "recalculate everything" forced to trickle down through
>> the page table levels, then that's as good.
> 
> Thanks for your advices, Jan & George.
> 
> Previously in the 2nd version, I used p2m_change_entry_type_global() to 
> reset the
> outstanding p2m_ioreq_server entries back to p2m_ram_rw asynchronously after
> the de-registration. But we realized later that this approach means we 
> can not support
> live migration. And to recalculate the whole p2m table forcefully when 
> de-registration
> happens means too much cost.
> 
> And further discussion with Paul was that we can leave the 
> responsibility to reset p2m type
> to the device model side, and even a device model fails to do so, the 
> affected one will only
> be the current VM, neither other VM nor hypervisor will get hurt.
> 
> I thought we have reached agreement in the review process of version 2, 
> so I removed
> this part from version 3.

In which case I would appreciate the commit message to explain
this (in particular I admit I don't recall why live migration would
be affected by the p2m_change_entry_type_global() approach,
but the request is also so that later readers have at least some
source of information other than searching the mailing list).

> Hah, I guess these 2 #defines are just cloned from similar ones, and I 
> did not expected
> they would receive so much comments. Anyway, I admire your preciseness 
> and thanks
> for pointing this out. :)
> 
> Since the bit number #defines have no special meaning, I'd like to just 
> define the flags
> directly:
> 
> #define HVMOP_IOREQ_MEM_ACCESS_READ (1u << 0)
> #define HVMOP_IOREQ_MEM_ACCESS_WRITE (1u << 1)

XEN_HVMOP_* then please.

Jan


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

Re: [Xen-devel] [PATCH raisin v3] Update to 4.7, update qemu and qemu_traditional recipes

[Stefano Stabellini]

On Wed, 15 Jun 2016, George Dunlap wrote:
> Add a 4.7 config file, make it the default.
> 
> Also update the qemu and qemu_traditional recipies after Ian Cambell's
> work to split off separate libraries.
> 
> Signed-off-by: George Dunlap 

Acked-by: Stefano Stabellini 


> Changes in v3:
> - Reword comment
> - Don't use tabs
> 
> Changes in v2:
>  - Only add the extra #defines when building 4.7 or master, and add an
>explanation of what they're for.
> 
> 
> CC: Stefano Stabellini 
> ---
>  components/qemu | 32 +++-
>  components/qemu_traditional |  2 +-
>  configs/config-4.7  |  8 
>  defconfig   |  2 +-
>  4 files changed, 33 insertions(+), 11 deletions(-)
> 
> diff --git a/components/qemu b/components/qemu
> index e0d92a5..b49ed1d 100644
> --- a/components/qemu
> +++ b/components/qemu
> @@ -20,18 +20,32 @@ function qemu_check_package() {
>  }
>  
>  function qemu_build() {
> +local QEMU_EXTRA_CFLAGS
>  cd "$BASEDIR"
>  git-checkout $QEMU_URL $QEMU_REVISION qemu-dir
>  cd qemu-dir
> -./configure --enable-xen --target-list=i386-softmmu --prefix=$PREFIX \
> ---extra-cflags="-I$INST_DIR/$PREFIX/include" \
> ---extra-ldflags="-L$INST_DIR/$PREFIX/lib 
> -Wl,-rpath-link=$INST_DIR/$PREFIX/lib \
> - -L$INST_DIR/$PREFIX/lib64 
> -Wl,-rpath-link=$INST_DIR/$PREFIX/lib64" \
> ---disable-kvm \
> ---disable-docs \
> ---bindir=$PREFIX/lib/xen/bin \
> ---datadir=$PREFIX/share/qemu-xen \
> ---disable-guest-agent
> +
> +QEMU_EXTRA_CFLAGS="-I$INST_DIR/$PREFIX/include"
> +
> +if [[ "$XEN_RELEASE" == "4.7" || "$XEN_RELEASE" == "master" ]] ; then
> +# qemu-xen released with 4.7.0 doesn't use the new libxc api,
> +# nor does it know how to ask for the compat api, so we need
> +# to tell it to do so manually.
> +QEMU_EXTRA_CFLAGS="$QEMU_EXTRA_CFLAGS -DXC_WANT_COMPAT_EVTCHN_API=1 \
> + -DXC_WANT_COMPAT_GNTTAB_API=1 \
> + -DXC_WANT_COMPAT_MAP_FOREIGN_API=1"
> +fi
> +
> +./configure --enable-xen --target-list=i386-softmmu \
> + --prefix=$PREFIX \
> + --extra-cflags="$QEMU_EXTRA_CFLAGS" \
> + --extra-ldflags="-L$INST_DIR/$PREFIX/lib 
> -Wl,-rpath-link=$INST_DIR/$PREFIX/lib \
> + -L$INST_DIR/$PREFIX/lib64 
> -Wl,-rpath-link=$INST_DIR/$PREFIX/lib64" \
> + --bindir=$PREFIX/lib/xen/bin \
> + --datadir=$PREFIX/share/qemu-xen \
> + --disable-kvm \
> + --disable-docs \
> + --disable-guest-agent
>  $RAISIN_MAKE all
>  $RAISIN_MAKE install DESTDIR="$INST_DIR"
>  cd "$BASEDIR"
> diff --git a/components/qemu_traditional b/components/qemu_traditional
> index 3150c3e..8732fe2 100644
> --- a/components/qemu_traditional
> +++ b/components/qemu_traditional
> @@ -30,7 +30,7 @@ function qemu_traditional_build() {
>  
>  export CONFIG_BLKTAP1=n
>  export XEN_ROOT="$BASEDIR"/xen-dir
> -./xen-setup
> +./xen-setup --extra-cflags="-D__XEN_TOOLS__"
>  $RAISIN_MAKE all
>  $RAISIN_MAKE install DESTDIR="$INST_DIR"
>  cd "$BASEDIR"
> diff --git a/configs/config-4.7 b/configs/config-4.7
> new file mode 100644
> index 000..66fe467
> --- /dev/null
> +++ b/configs/config-4.7
> @@ -0,0 +1,8 @@
> +XEN_REVISION="origin/stable-4.7"
> +QEMU_REVISION="origin/stable-4.7"
> +QEMU_TRADITIONAL_REVISION="origin/stable-4.7"
> +SEABIOS_REVISION="rel-1.9.2"
> +GRUB_REVISION="master"
> +LIBVIRT_REVISION="origin/v1.3.3-maint"
> +OVMF_REVISION="52a99493cce88a9d4ec8a02d7f1bd1a1001ce60d"
> +LINUX_REVISION="master"
> diff --git a/defconfig b/defconfig
> index 8e79a43..f8ef398 100644
> --- a/defconfig
> +++ b/defconfig
> @@ -2,7 +2,7 @@
>  # Setup a Xen based system.
>  
>  # Available options: 4.5, 4.6, master (for development branch)
> -XEN_RELEASE="4.6"
> +XEN_RELEASE="4.7"
>  
>  # Components
>  ## All components: seabios ovmf xen qemu qemu_traditional grub libvirt linux
> -- 
> 2.1.4
> 

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

Re: [Xen-devel] [PATCH raisin v2 4/4] Update to 4.7, update qemu and qemu_traditional recipes

[Stefano Stabellini]

On Wed, 15 Jun 2016, George Dunlap wrote:
> On 15/06/16 18:13, Stefano Stabellini wrote:
> > On Wed, 15 Jun 2016, George Dunlap wrote:
> >> Add a 4.7 config file, make it the default.
> >>
> >> Also update the qemu and qemu_traditional recipies after Ian Cambell's
> >> work to split off separate libraries.
> >>
> >> Signed-off-by: George Dunlap 
> >> ---
> >> Changes in v2:
> >>  - Only add the extra #defines when building 4.7 or master, and add an
> >>explanation of what they're for.
> >>
> >>
> >> CC: Stefano Stabellini 
> >> ---
> >>  components/qemu | 30 ++
> >>  components/qemu_traditional |  2 +-
> >>  configs/config-4.7  |  8 
> >>  defconfig   |  2 +-
> >>  4 files changed, 32 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/components/qemu b/components/qemu
> >> index e0d92a5..23e112c 100644
> >> --- a/components/qemu
> >> +++ b/components/qemu
> >> @@ -20,18 +20,32 @@ function qemu_check_package() {
> >>  }
> >>  
> >>  function qemu_build() {
> >> +local QEMU_EXTRA_CFLAGS
> >>  cd "$BASEDIR"
> >>  git-checkout $QEMU_URL $QEMU_REVISION qemu-dir
> >>  cd qemu-dir
> >> -./configure --enable-xen --target-list=i386-softmmu --prefix=$PREFIX \
> >> ---extra-cflags="-I$INST_DIR/$PREFIX/include" \
> >> ---extra-ldflags="-L$INST_DIR/$PREFIX/lib 
> >> -Wl,-rpath-link=$INST_DIR/$PREFIX/lib \
> >> +
> >> +QEMU_EXTRA_CFLAGS="-I$INST_DIR/$PREFIX/include"
> >> +
> >> +if [[ "$XEN_RELEASE" == "4.7" || "$XEN_RELEASE" == "master" ]] ; then
> >> +  # qemu-xen released with 4.7.0 doesn't use the new libxc api,
> >> +  # nor properly detect it so as to use the old version; so we
> > ^ has to use ?
> 
> No, but it is rather an unusual grammar construction so maybe it could
> be reworded. What about:
> 
> "qemu-xen released with 4.7.0 doesn't use the new libxc api, nor does it
> know how to ask for the compat api, so we need to tell it to do so
> manually."

that's fine


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

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Yu Zhang]

On 6/14/2016 6:45 PM, Jan Beulich wrote:

On 19.05.16 at 11:05,  wrote:

A new HVMOP - HVMOP_map_mem_type_to_ioreq_server, is added to
let one ioreq server claim/disclaim its responsibility for the
handling of guest pages with p2m type p2m_ioreq_server. Users
of this HVMOP can specify which kind of operation is supposed
to be emulated in a parameter named flags. Currently, this HVMOP
only support the emulation of write operations. And it can be
easily extended to support the emulation of read ones if an
ioreq server has such requirement in the future.

Didn't we determine that this isn't as easy as everyone first thought?

My understanding is that to emulate read, we need to change the definition
of is_epte_present(), and I do not think this change will cause much
trouble.
But since no one is using the read emulation, I am convinced the more
cautious
way is to only support emulations for write operations for now.

Well, okay. I'd personally drop the "easily", but you know what
to tell people if later they come ask how this "easily" was meant.


OK. Let's drop the word "easily". :)


@@ -914,6 +916,45 @@ int hvm_unmap_io_range_from_ioreq_server(struct domain

*d, ioservid_t id,

   return rc;
   }
   
+int hvm_map_mem_type_to_ioreq_server(struct domain *d, ioservid_t id,

+ uint16_t type, uint32_t flags)

I see no reason why both can't be unsigned int.

Parameter type is passed in from the type field inside struct
xen_hvm_map_mem_type_to_ioreq_server,
which is a uint16_t, followed with a uint16_t pad. Now I am wondering,
may be we can just remove the pad
field in this structure and just define type as uint32_t.

I think keeping the interface structure unchanged is the desirable
route here. What I dislike is the passing around of non-natural
width types, which is more expensive in terms of processing. I.e.
as long as a fixed width type (which is necessary to be used in
the public interface) fits in "unsigned int", that should be the
respective internal type. Otherwise "unsigned long" etc.

There are cases where even internally we indeed want to use
fixed width types, and admittedly there are likely far more cases
where internally fixed width types get used without good reason,
but just like everywhere else - let's please not make this worse.
IOW please use fixed width types only when you really need them.
OK. I can keep the interface, and using uint32_t type in the internal 
routine

would means a implicit type conversion from uint16_6, which I do not think
is a problem.


--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -132,6 +132,12 @@ static void ept_p2m_type_to_flags(struct p2m_domain *p2m, 
ept_entry_t *entry,
   entry->r = entry->w = entry->x = 1;
   entry->a = entry->d = !!cpu_has_vmx_ept_ad;
   break;
+case p2m_ioreq_server:
+entry->r = entry->x = 1;

Why x?

Setting entry->x to 1 is not a must. I can remove it. :)

Please do. We shouldn't grant permissions without reason.


Got it. Thanks.


@@ -94,8 +96,16 @@ static unsigned long p2m_type_to_flags(p2m_type_t t, mfn_t 
mfn,
   default:
   return flags | _PAGE_NX_BIT;
   case p2m_grant_map_ro:
-case p2m_ioreq_server:
   return flags | P2M_BASE_FLAGS | _PAGE_NX_BIT;
+case p2m_ioreq_server:
+{
+flags |= P2M_BASE_FLAGS | _PAGE_RW;
+
+if ( p2m->ioreq.flags & P2M_IOREQ_HANDLE_WRITE_ACCESS )
+return flags & ~_PAGE_RW;
+else
+return flags;
+}

Same here (for the missing _PAGE_NX) plus no need for braces.

I'll remove the brace. And we do not need to set the _PAGE_NX_BIT, like
the p2m_ram_ro case I guess.

I hope you mean the inverse: You should set _PAGE_NX_BIT here.

Oh, right. I meant the reverse. Thanks for the remind. :)
And I have a question,  here in p2m_type_to_flags(), I saw current code 
uses _PAGE_NX_BIT
to disable the executable permission,  and I wonder, why don't we choose 
the _PAGE_NX,

which is defined as:

#define _PAGE_NX   (cpu_has_nx ? _PAGE_NX_BIT : 0)

How do we know for sure that bit 63 from pte is not a reserved one 
without checking
the cpu capability(the cpu_has_nx)? Is there any other reasons, i.e. the 
page tables might

be shared with IOMMU?


+ struct hvm_ioreq_server *s)
+{
+struct p2m_domain *p2m = p2m_get_hostp2m(d);
+int rc;
+
+spin_lock(>ioreq.lock);
+
+if ( flags == 0 )
+{
+rc = -EINVAL;
+if ( p2m->ioreq.server != s )
+goto out;
+
+/* Unmap ioreq server from p2m type by passing flags with 0. */
+p2m->ioreq.server = NULL;
+p2m->ioreq.flags = 0;
+}

What does "passing" refer to in the comment?

It means if this routine is called with flags=0, it will unmap the ioreq
server.

Oh, that's a completely different reading of the comment than I had
implied: With what you say, "flags" here really 

Re: [Xen-devel] [PATCH RESEND 07/14] libxl/arm: Construct ACPI DSDT table

[Julien Grall]

Hi Shannon,

On 16/06/16 07:25, Shannon Zhao wrote:



On 2016/6/7 21:42, Julien Grall wrote:

Hello Shannon,

On 31/05/16 06:02, Shannon Zhao wrote:

From: Shannon Zhao 

Currently there is only the table header in DSDT table.

Signed-off-by: Shannon Zhao 
---
   tools/libxl/libxl_arm.c | 15 +++
   1 file changed, 15 insertions(+)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index c3b8fb4..7949635 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -944,6 +944,20 @@ static void make_acpi_fadt(libxl__gc *gc, struct
xc_dom_image *dom)
   dom->acpitable_size += dom->acpitable_blob->fadt.size;
   }

+static void make_acpi_dsdt(libxl__gc *gc, struct xc_dom_image *dom)
+{
+struct acpi_table_header *dsdt;
+
+/* Currently there is only the table header in DSDT table. */


What about the processor device objects?


Ah, yes. It should add processor device objects. Since DSDT table is
special, what's your suggestion about how to generate DSDT table? Use
static table which X86 use(tools/firmware/hvmloader/acpi/mk_dsdt.c) or
import what QEMU uses?


I would do the choice depending on what we need to expose through the 
DTST. If it is only dummy processor containers, then ~1500 lines of code 
(see qemu/hw/acpi/aml-build.c) seems a bit overkill.



I prefer the latter, but this will add more codes to libxl.


What would be the benefits to build dynamically those tables?

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

[Yu Zhang]

On 6/15/2016 6:21 PM, Jan Beulich wrote:

On 15.06.16 at 11:50,  wrote:

On 14/06/16 14:31, Jan Beulich wrote:

On 14.06.16 at 15:13,  wrote:

On 14/06/16 11:45, Jan Beulich wrote:

Locking is somewhat strange here: You protect against the "set"
counterpart altering state while you retrieve it, but you don't
protect against the returned data becoming stale by the time
the caller can consume it. Is that not a problem? (The most
concerning case would seem to be a race of hvmop_set_mem_type()
with de-registration of the type.)

How is that different than calling set_mem_type() first, and then
de-registering without first unmapping all the types?

Didn't we all agree this is something that should be disallowed
anyway (not that I've seen this implemented, i.e. just being
reminded of it by your reply)?

I think I suggested it as a good idea, but Paul and Yang both thought it
wasn't necessary.  Do you think it should be a requirement?

I think things shouldn't be left in a half-adjusted state.


We could have the de-registering operation fail in those circumstances;
but probably a more robust thing to do would be to have Xen go change
all the ioreq_server entires back to ram_rw (since if the caller just
ignores the failure, things are in an even worse state).

If that's reasonable to do without undue delay (e.g. by using
the usual "recalculate everything" forced to trickle down through
the page table levels, then that's as good.


Thanks for your advices, Jan & George.

Previously in the 2nd version, I used p2m_change_entry_type_global() to 
reset the

outstanding p2m_ioreq_server entries back to p2m_ram_rw asynchronously after
the de-registration. But we realized later that this approach means we 
can not support
live migration. And to recalculate the whole p2m table forcefully when 
de-registration

happens means too much cost.

And further discussion with Paul was that we can leave the 
responsibility to reset p2m type
to the device model side, and even a device model fails to do so, the 
affected one will only

be the current VM, neither other VM nor hypervisor will get hurt.

I thought we have reached agreement in the review process of version 2, 
so I removed

this part from version 3.




+uint32_t flags; /* IN - types of accesses to be forwarded to the
+   ioreq server. flags with 0 means to unmap the
+   ioreq server */
+#define _HVMOP_IOREQ_MEM_ACCESS_READ 0
+#define HVMOP_IOREQ_MEM_ACCESS_READ \
+(1u << _HVMOP_IOREQ_MEM_ACCESS_READ)
+
+#define _HVMOP_IOREQ_MEM_ACCESS_WRITE 1
+#define HVMOP_IOREQ_MEM_ACCESS_WRITE \
+(1u << _HVMOP_IOREQ_MEM_ACCESS_WRITE)

Is there any use for these _HVMOP_* values? The more that they
violate standard C name space rules?

I assume he's just going along with what he sees in params.h.
"Violating standard C name space rules" by having #defines which start
with a single _ seems to be a well-established policy for Xen. :-)

Sadly, and I'm trying to prevent matters becoming worse.
Speaking of which - there are XEN_ prefixes missing here too.

Right, so in that case I think I would have said, "I realize that lots
of other places in the Xen interface use this sort of template for
flags, but I think it's a bad idea and I'm trying to stop it expanding.
  Is there any actual need to have the bit numbers defined separately?
If not, please just define each flag as (1u << 0), "

Actually my coding style related comment wasn't for these two
stage definitions - for those I simply questioned whether they're
needed. My style complaint was for the 
name pattern (which would simply be avoided by not having the
individual bit number #define-s).


I think you've tripped over "changing coding styles" in unfamiliar code
before too, so you know how frustrating it is to try to follow the
existing coding style only to be told that you did it wrong. :-)

Agreed, you caught me on this one. Albeit with the slight
difference that in the public interface we can't as easily correct
old mistakes to aid people who simply clone surrounding code
when adding new bits (the possibility of adding #ifdef-ery doesn't
seem very attractive to me there, unless we got reports of actual
name space collisions).



Hah, I guess these 2 #defines are just cloned from similar ones, and I 
did not expected
they would receive so much comments. Anyway, I admire your preciseness 
and thanks

for pointing this out. :)

Since the bit number #defines have no special meaning, I'd like to just 
define the flags

directly:

#define HVMOP_IOREQ_MEM_ACCESS_READ (1u << 0)
#define HVMOP_IOREQ_MEM_ACCESS_WRITE (1u << 1)


B.R.
Yu

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

[Xen-devel] [PATCH 2/2] hvmloader: don't hard-code IO-APIC parameters

[Jan Beulich]

The IO-APIC address has variable bits determined by the PCI-to-ISA
bridge, and the IO-APIC version should be read from the IO-APIC. (Note
that there's still implicit rather than explicit agreement on the
IO-APIC base address between qemu and the hypervisor.)

Signed-off-by: Jan Beulich 

--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -138,7 +138,7 @@ static struct acpi_20_madt *construct_ma
 io_apic->type= ACPI_IO_APIC;
 io_apic->length  = sizeof(*io_apic);
 io_apic->ioapic_id   = IOAPIC_ID;
-io_apic->ioapic_addr = IOAPIC_BASE_ADDRESS;
+io_apic->ioapic_addr = ioapic_base_address;
 
 lapic = (struct acpi_20_madt_lapic *)(io_apic + 1);
 info->nr_cpus = hvm_info->nr_vcpus;
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -42,9 +42,10 @@ extern struct bios_config ovmf_config;
 #define PAGE_SHIFT 12
 #define PAGE_SIZE  (1ul << PAGE_SHIFT)
 
-#define IOAPIC_BASE_ADDRESS 0xfec0
+extern uint32_t ioapic_base_address;
+extern uint8_t ioapic_version;
+
 #define IOAPIC_ID   0x01
-#define IOAPIC_VERSION  0x11
 
 #define LAPIC_BASE_ADDRESS  0xfee0
 #define LAPIC_ID(vcpu_id)   ((vcpu_id) * 2)
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -108,6 +108,9 @@ asm (
 
 unsigned long scratch_start = SCRATCH_PHYSICAL_ADDRESS;
 
+uint32_t ioapic_base_address = 0xfec0;
+uint8_t ioapic_version;
+
 static void init_hypercalls(void)
 {
 uint32_t eax, ebx, ecx, edx;
@@ -185,6 +188,9 @@ static void init_vm86_tss(void)
 
 static void apic_setup(void)
 {
+ioapic_base_address |= (pci_readb(PCI_ISA_DEVFN, 0x80) & 0x3f) << 10;
+ioapic_version = ioapic_read(0x01) & 0xff;
+
 /* Set the IOAPIC ID to the static value used in the MP/ACPI tables. */
 ioapic_write(0x00, IOAPIC_ID);
 
--- a/tools/firmware/hvmloader/mp_tables.c
+++ b/tools/firmware/hvmloader/mp_tables.c
@@ -227,9 +227,9 @@ static void fill_mp_ioapic_entry(struct
 {
 mpie->type = ENTRY_TYPE_IOAPIC;
 mpie->ioapic_id = IOAPIC_ID;
-mpie->ioapic_version = IOAPIC_VERSION;
+mpie->ioapic_version = ioapic_version;
 mpie->ioapic_flags = 1; /* enabled */
-mpie->ioapic_addr = IOAPIC_BASE_ADDRESS;
+mpie->ioapic_addr = ioapic_base_address;
 }
 
 
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -490,14 +490,14 @@ void *scratch_alloc(uint32_t size, uint3
 
 uint32_t ioapic_read(uint32_t reg)
 {
-*(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg;
-return *(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x10);
+*(volatile uint32_t *)(ioapic_base_address + 0x00) = reg;
+return *(volatile uint32_t *)(ioapic_base_address + 0x10);
 }
 
 void ioapic_write(uint32_t reg, uint32_t val)
 {
-*(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x00) = reg;
-*(volatile uint32_t *)(IOAPIC_BASE_ADDRESS + 0x10) = val;
+*(volatile uint32_t *)(ioapic_base_address + 0x00) = reg;
+*(volatile uint32_t *)(ioapic_base_address + 0x10) = val;
 }
 
 uint32_t lapic_read(uint32_t reg)



hvmloader: don't hard-code IO-APIC parameters

The IO-APIC address has variable bits determined by the PCI-to-ISA
bridge, and the IO-APIC version should be read from the IO-APIC. (Note
that there's still implicit rather than explicit agreement on the
IO-APIC base address between qemu and the hypervisor.)

Signed-off-by: Jan Beulich 

--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -138,7 +138,7 @@ static struct acpi_20_madt *construct_ma
 io_apic->type= ACPI_IO_APIC;
 io_apic->length  = sizeof(*io_apic);
 io_apic->ioapic_id   = IOAPIC_ID;
-io_apic->ioapic_addr = IOAPIC_BASE_ADDRESS;
+io_apic->ioapic_addr = ioapic_base_address;
 
 lapic = (struct acpi_20_madt_lapic *)(io_apic + 1);
 info->nr_cpus = hvm_info->nr_vcpus;
--- a/tools/firmware/hvmloader/config.h
+++ b/tools/firmware/hvmloader/config.h
@@ -42,9 +42,10 @@ extern struct bios_config ovmf_config;
 #define PAGE_SHIFT 12
 #define PAGE_SIZE  (1ul << PAGE_SHIFT)
 
-#define IOAPIC_BASE_ADDRESS 0xfec0
+extern uint32_t ioapic_base_address;
+extern uint8_t ioapic_version;
+
 #define IOAPIC_ID   0x01
-#define IOAPIC_VERSION  0x11
 
 #define LAPIC_BASE_ADDRESS  0xfee0
 #define LAPIC_ID(vcpu_id)   ((vcpu_id) * 2)
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -108,6 +108,9 @@ asm (
 
 unsigned long scratch_start = SCRATCH_PHYSICAL_ADDRESS;
 
+uint32_t ioapic_base_address = 0xfec0;
+uint8_t ioapic_version;
+
 static void init_hypercalls(void)
 {
 uint32_t eax, ebx, ecx, edx;
@@ -185,6 +188,9 @@ static void init_vm86_tss(void)
 
 static void apic_setup(void)
 {
+ioapic_base_address |= (pci_readb(PCI_ISA_DEVFN, 0x80) & 0x3f) << 10;
+ioapic_version = ioapic_read(0x01) & 0xff;
+
 /* Set the IOAPIC ID to the static