[Xen-devel] [qemu-upstream-4.3-testing test] 94782: trouble: blocked/broken

2016-05-25 Thread osstest service owner
flight 94782 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94782/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   3 host-install(3) broken REGR. vs. 80927
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927
 build-i386-pvops  3 host-install(3) broken REGR. vs. 80927
 build-i3863 host-install(3) broken REGR. vs. 80927

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-pv1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-pv   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a

version targeted for testing:
 qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  109 days
Testing same since93977  2016-05-10 11:09:16 Z   15 days   59 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Stefano Stabellini 

jobs:
 build-amd64  broken  
 build-i386   broken  
 build-amd64-libvirt  blocked 
 build-i386-libvirt   blocked 
 build-amd64-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-xl  blocked 
 test-amd64-i386-xl   blocked 
 test-amd64-i386-qemuu-rhel6hvm-amd   blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked 
 test-amd64-i386-freebsd10-amd64  blocked 
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 
 test-amd64-amd64-xl-qemuu-win7-amd64 blocked 
 test-amd64-i386-xl-qemuu-win7-amd64  blocked 
 test-amd64-amd64-xl-credit2  blocked 
 test-amd64-i386-freebsd10-i386   blocked 
 test-amd64-i386-qemuu-rhel6hvm-intel blocked 
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpublocked 
 

[Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

2016-05-25 Thread Tamas K Lengyel
Move sharing locks above altp2m to avoid locking order violation. Allow
applying altp2m mem_access settings when the hostp2m entries are shared.
Also, do not trigger PoD for hostp2m when setting altp2m mem_access to be
in-line with non-altp2m mem_access path. Also allow gfn remapping with
p2m_ram_shared type gfns in altp2m views.

When using mem_sharing in combination with altp2m, unsharing events overwrite
altp2m entries with that of the hostp2m (both remapped entries and mem_access
settings). User should take precaution to not share pages where this behavior
is undesired.

Signed-off-by: Tamas K Lengyel 
---
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 

v4: Resolve locking problem by moving sharing locks above altp2m locks
---
 xen/arch/x86/mm/mm-locks.h | 30 +++---
 xen/arch/x86/mm/p2m.c  | 10 +-
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/mm/mm-locks.h b/xen/arch/x86/mm/mm-locks.h
index 086c8bb..74fdfc1 100644
--- a/xen/arch/x86/mm/mm-locks.h
+++ b/xen/arch/x86/mm/mm-locks.h
@@ -242,6 +242,21 @@ declare_mm_lock(nestedp2m)
 
 declare_mm_rwlock(p2m);
 
+/* Sharing per page lock
+ *
+ * This is an external lock, not represented by an mm_lock_t. The memory
+ * sharing lock uses it to protect addition and removal of (gfn,domain)
+ * tuples to a shared page. We enforce order here against the p2m lock,
+ * which is taken after the page_lock to change the gfn's p2m entry.
+ *
+ * The lock is recursive because during share we lock two pages. */
+
+declare_mm_order_constraint(per_page_sharing)
+#define page_sharing_mm_pre_lock()   
mm_enforce_order_lock_pre_per_page_sharing()
+#define page_sharing_mm_post_lock(l, r) \
+mm_enforce_order_lock_post_per_page_sharing((l), (r))
+#define page_sharing_mm_unlock(l, r) mm_enforce_order_unlock((l), (r))
+
 /* Alternate P2M list lock (per-domain)
  *
  * A per-domain lock that protects the list of alternate p2m's.
@@ -287,21 +302,6 @@ declare_mm_rwlock(altp2m);
 #define p2m_locked_by_me(p)   mm_write_locked_by_me(&(p)->lock)
 #define gfn_locked_by_me(p,g) p2m_locked_by_me(p)
 
-/* Sharing per page lock
- *
- * This is an external lock, not represented by an mm_lock_t. The memory
- * sharing lock uses it to protect addition and removal of (gfn,domain)
- * tuples to a shared page. We enforce order here against the p2m lock,
- * which is taken after the page_lock to change the gfn's p2m entry.
- *
- * The lock is recursive because during share we lock two pages. */
-
-declare_mm_order_constraint(per_page_sharing)
-#define page_sharing_mm_pre_lock()   
mm_enforce_order_lock_pre_per_page_sharing()
-#define page_sharing_mm_post_lock(l, r) \
-mm_enforce_order_lock_post_per_page_sharing((l), (r))
-#define page_sharing_mm_unlock(l, r) mm_enforce_order_unlock((l), (r))
-
 /* PoD lock (per-p2m-table)
  * 
  * Protects private PoD data structs: entry and cache
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 9b19769..dc97082 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1768,10 +1768,10 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct 
p2m_domain *hp2m,
 if ( !mfn_valid(mfn) )
 {
 mfn = hp2m->get_entry(hp2m, gfn_l, , _a,
-  P2M_ALLOC | P2M_UNSHARE, _order, NULL);
+  0, _order, NULL);
 
 rc = -ESRCH;
-if ( !mfn_valid(mfn) || t != p2m_ram_rw )
+if ( !mfn_valid(mfn) || (t != p2m_ram_rw && t != p2m_ram_shared) )
 return rc;
 
 /* If this is a superpage, copy that first */
@@ -2542,9 +2542,9 @@ int p2m_change_altp2m_gfn(struct domain *d, unsigned int 
idx,
 if ( !mfn_valid(mfn) )
 {
 mfn = hp2m->get_entry(hp2m, gfn_x(old_gfn), , ,
-  P2M_ALLOC | P2M_UNSHARE, _order, NULL);
+  P2M_ALLOC, _order, NULL);
 
-if ( !mfn_valid(mfn) || t != p2m_ram_rw )
+if ( !mfn_valid(mfn) || (t != p2m_ram_rw && t != p2m_ram_shared) )
 goto out;
 
 /* If this is a superpage, copy that first */
@@ -2567,7 +2567,7 @@ int p2m_change_altp2m_gfn(struct domain *d, unsigned int 
idx,
 if ( !mfn_valid(mfn) )
 mfn = hp2m->get_entry(hp2m, gfn_x(new_gfn), , , 0, NULL, NULL);
 
-if ( !mfn_valid(mfn) || (t != p2m_ram_rw) )
+if ( !mfn_valid(mfn) || (t != p2m_ram_rw && t != p2m_ram_shared) )
 goto out;
 
 if ( !ap2m->set_entry(ap2m, gfn_x(old_gfn), mfn, PAGE_ORDER_4K, t, a,
-- 
2.8.1


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


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

2016-05-25 Thread osstest service owner
flight 94773 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94773/

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 855743f7177459bea95798e59b6b18dab867710c
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z1 days
Failing since 94750  2016-05-25 03:43:08 Z0 days4 attempts
Testing same since94773  2016-05-25 19:44:42 Z0 days1 attempts


People who touched revisions under test:
  Dandan Bi 
  Hao Wu 
  Laszlo Ersek 
  Marvin H?user 
  Marvin Haeuser 
  Yonghong Zhu 

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.


commit 855743f7177459bea95798e59b6b18dab867710c
Author: Laszlo Ersek 
Date:   Wed May 18 20:13:41 2016 +0200

OvmfPkg: prevent 64-bit MMIO BAR degradation if there is no CSM

According to edk2 commit

  "MdeModulePkg/PciBus: do not improperly degrade resource"

and to the EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL definition in the
Platform Init 1.4a specification, a platform can provide such a protocol
in order to influence the PCI resource allocation performed by the PCI Bus
driver.

In particular it is possible instruct the PCI Bus driver, with a
"wildcard" hint, to allocate the 64-bit MMIO BARs of a device in 64-bit
address space, regardless of whether the device features an option ROM.

(By default, the PCI Bus driver considers an option ROM reason enough for
allocating the 64-bit MMIO BARs in 32-bit address space. It cannot know if
BDS will launch a legacy boot option, and under legacy boot, a legacy BIOS
binary from a combined option ROM could be dispatched, and fail to access
MMIO BARs in 64-bit address space.)

In platform code we can ascertain whether a CSM is present or not. If not,
then legacy BIOS binaries in option ROMs can't be dispatched, hence the
BAR degradation is detrimental, and we should prevent it. This is expected
to conserve the 32-bit address space for 32-bit MMIO BARs.

The driver added in this patch could be simplified based on the following
facts:

- In the Ia32 build, the 64-bit MMIO aperture is always zero-size, hence
  the driver will exit immediately. Therefore the driver could be omitted
  from the Ia32 build.

- In the Ia32X64 and X64 builds, the driver could be omitted if CSM_ENABLE
  was defined (because in that case the degradation would be justified).
  On the other hand, if CSM_ENABLE was undefined, then the driver could be
  included, and it could provide the hint unconditionally (without looking
  for the Legacy BIOS protocol).

These short-cuts are not taken because they would increase the differences
between the OVMF DSC/FDF files. If we can manage without extreme
complexity, we should use dynamic logic (vs. build time configuration),
plus keep conditional compilation to a minimum.

Cc: Jordan Justen 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo 

[Xen-devel] [PATCH v3] AMD IOMMU: Introduce support for IVHD block type 11h

2016-05-25 Thread suravee.suthikulpanit
From: Suravee Suthikulpanit 

Along with the IVHD block type 10h, newer AMD platforms also come with
types 11h, which is a superset of the older one. Having multiple IVHD
block types in the same platform allows backward compatibility of newer
systems to work with existing drivers.  The driver should only parse
the highest-level (newest) type of IVHD block that it can support.
However, the current driver returns error when encounters with unknown
IVHD block type. This causes existing driver to unnecessarily fail IOMMU
initialization on new systems.

This patch introduces a new logic, which scans through IVRS table looking
for the highest-level supporsted IVHD block type. It also adds support
for the new IVHD block type 11h. More information about the IVHD type 11h
can be found in the AMD I/O Virtualization Technology (IOMMU) Specification
rev 2.62.

http://support.amd.com/TechDocs/48882_IOMMU.pdf

Signed-off-by: Suravee Suthikulpanit 
---
Changes from V2:
  * Remove redundant IVHD_HIGHEST_SUPPORT_TYPE.
  * Misc clean up suggestions from Andrew.

 xen/drivers/passthrough/amd/iommu_acpi.c  | 136 ++
 xen/drivers/passthrough/amd/iommu_init.c  |  10 +-
 xen/include/acpi/actbl2.h |   5 +-
 xen/include/asm-x86/amd-iommu.h   |   1 +
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |   1 +
 5 files changed, 108 insertions(+), 45 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c 
b/xen/drivers/passthrough/amd/iommu_acpi.c
index 79c1f8c..8bad2bc 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -821,13 +821,29 @@ static u16 __init parse_ivhd_device_special(
 return dev_length;
 }
 
+static inline size_t
+get_ivhd_header_size(const struct acpi_ivrs_hardware *ivhd_block)
+{
+switch ( ivhd_block->header.type )
+{
+case ACPI_IVRS_TYPE_HARDWARE:
+return offsetof(struct acpi_ivrs_hardware, efr_image);
+case ACPI_IVRS_TYPE_HARDWARE_11H:
+return sizeof(struct acpi_ivrs_hardware);
+default:
+break;
+}
+return 0;
+}
+
 static int __init parse_ivhd_block(const struct acpi_ivrs_hardware *ivhd_block)
 {
 const union acpi_ivhd_device *ivhd_device;
 u16 block_length, dev_length;
+size_t hdr_size = get_ivhd_header_size(ivhd_block) ;
 struct amd_iommu *iommu;
 
-if ( ivhd_block->header.length < sizeof(*ivhd_block) )
+if ( ivhd_block->header.length < hdr_size )
 {
 AMD_IOMMU_DEBUG("IVHD Error: Invalid Block Length!\n");
 return -ENODEV;
@@ -845,7 +861,7 @@ static int __init parse_ivhd_block(const struct 
acpi_ivrs_hardware *ivhd_block)
 }
 
 /* parse Device Entries */
-block_length = sizeof(*ivhd_block);
+block_length = hdr_size;
 while ( ivhd_block->header.length >=
 (block_length + sizeof(struct acpi_ivrs_de_header)) )
 {
@@ -914,34 +930,6 @@ static int __init parse_ivhd_block(const struct 
acpi_ivrs_hardware *ivhd_block)
 return 0;
 }
 
-static int __init parse_ivrs_block(const struct acpi_ivrs_header *ivrs_block)
-{
-const struct acpi_ivrs_hardware *ivhd_block;
-const struct acpi_ivrs_memory *ivmd_block;
-
-switch ( ivrs_block->type )
-{
-case ACPI_IVRS_TYPE_HARDWARE:
-ivhd_block = container_of(ivrs_block, const struct acpi_ivrs_hardware,
-  header);
-return parse_ivhd_block(ivhd_block);
-
-case ACPI_IVRS_TYPE_MEMORY_ALL:
-case ACPI_IVRS_TYPE_MEMORY_ONE:
-case ACPI_IVRS_TYPE_MEMORY_RANGE:
-case ACPI_IVRS_TYPE_MEMORY_IOMMU:
-ivmd_block = container_of(ivrs_block, const struct acpi_ivrs_memory,
-  header);
-return parse_ivmd_block(ivmd_block);
-
-default:
-AMD_IOMMU_DEBUG("IVRS Error: Invalid Block Type!\n");
-return -ENODEV;
-}
-
-return 0;
-}
-
 static void __init dump_acpi_table_header(struct acpi_table_header *table)
 {
 int i;
@@ -978,6 +966,25 @@ static void __init dump_acpi_table_header(struct 
acpi_table_header *table)
 
 }
 
+#define to_ivhd_block(hdr) \
+container_of(hdr, const struct acpi_ivrs_hardware, header)
+#define to_ivmd_block(hdr) \
+container_of(hdr, const struct acpi_ivrs_memory, header)
+
+static inline bool_t is_ivhd_block(u8 type)
+{
+return ( type == ACPI_IVRS_TYPE_HARDWARE ||
+ type == ACPI_IVRS_TYPE_HARDWARE_11H );
+}
+
+static inline bool_t is_ivmd_block(u8 type) \
+{
+return ( type == ACPI_IVRS_TYPE_MEMORY_ALL ||
+ type == ACPI_IVRS_TYPE_MEMORY_ONE ||
+ type == ACPI_IVRS_TYPE_MEMORY_RANGE ||
+ type == ACPI_IVRS_TYPE_MEMORY_IOMMU );
+}
+
 static int __init parse_ivrs_table(struct acpi_table_header *table)
 {
 const struct acpi_ivrs_header *ivrs_block;
@@ -1010,7 +1017,10 @@ static int __init parse_ivrs_table(struct 
acpi_table_header *table)

[Xen-devel] [PATCH] x86/psr: make opt_psr persistent

2016-05-25 Thread Chao Peng
opt_psr is now not only used at booting time but also at runtime.
More specifically, it is used to check CDP switch in psr_cpu_init()
which can potentially be called in CPU hotplug case.

Signed-off-by: Chao Peng 
---
 xen/arch/x86/psr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index f796552..0b5073c 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -52,7 +52,7 @@ static unsigned long *__read_mostly cat_socket_enable;
 static struct psr_cat_socket_info *__read_mostly cat_socket_info;
 static unsigned long *__read_mostly cdp_socket_enable;
 
-static unsigned int __initdata opt_psr;
+static unsigned int opt_psr;
 static unsigned int __initdata opt_rmid_max = 255;
 static unsigned int __read_mostly opt_cos_max = 255;
 static uint64_t rmid_mask;
-- 
1.9.1


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


Re: [Xen-devel] [BUG] Xen panic with VT-d PI

2016-05-25 Thread Wu, Feng
The patch fixing this issue has already been sent to upstream.

" [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list "

v2 will be sent out soon.

Thanks,
Feng

> -Original Message-
> From: Hao, Xudong
> Sent: Thursday, May 26, 2016 9:45 AM
> To: Xen-devel 
> Cc: Wu, Feng ; Wei Liu 
> Subject: [BUG] Xen panic with VT-d PI
> 
> Xen 4.7 RC3 enabled VT-d PI(iommu=on,intpost), and created HVM guest with VF
> NIC assigned repeatedly, it will cause Xen panic(maybe 2 times ,5 times ,10
> times).
> It's not a RC3 regression, at least this Xen commit 1bd52e1f got failure as 
> well.
> 
> HW: Intel Broadwell server, Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz.
> Xen: Xen 4.7.0 RC3
> Dom0: Linux 4.4.4
> VF: Intel 82599 NIC
> 
> ...
> (XEN) irq.c:275: Dom8 PCI link 3 changed 5 -> 0
> [ 1319.177226] ixgbe :05:00.0 eth30: VF Reset msg received from vf 1
> [ 1321.949952] ixgbe :05:00.0 eth30: 1 Spoofed packets detected
> (XEN) [ Xen-4.7-unstable  x86_64  debug=y  Tainted:C ]
> (XEN) CPU:3
> (XEN) RIP:e008:[] vmx.c#pi_wakeup_interrupt+0xcf/0x15b
> (XEN) RFLAGS: 00010002   CONTEXT: hypervisor
> (XEN) rax: 830078a71760   rbx: 01001001000fff60   rcx: 0003
> (XEN) rdx: 0100100100100100   rsi: 0046   rdi: 830078a715c0
> (XEN) rbp: 83043c477d18   rsp: 83043c477ce8   r8:  0003
> (XEN) r9:  830425ae2710   r10: 830425ae28e0   r11: 01354d22b2fb
> (XEN) r12: 83043c47dde0   r13: 83043c47ddf0   r14: 0100100100100100
> (XEN) r15: 0200200200200200   cr0: 8005003b   cr4:
> 003526e0
> (XEN) cr3: 000424276000   cr2: 003abfaaca80
> (XEN) ds:    es:    fs:    gs:    ss:    cs: e008
> (XEN) Xen code around 
> (vmx.c#pi_wakeup_interrupt+0xcf/0x15b):
> (XEN)  00 5a 48 8b 97 a0 01 00 <48> 3b 42 08 74 04 0f 0b eb fe 48 8b 8f a8 01 
> 00
> (XEN) Xen stack trace from rsp=83043c477ce8:
> (XEN)83043c477d08 00f5 83043c477db8
> 00f5
> (XEN)83043c47f788  83043c477da8 82d080171c7a
> (XEN)  deadbeefdeadf00d
> 8300782fe000
> (XEN)0003 830422e7 
> 80003c477d90
> (XEN)82d0801949fa 8c7adb3d0002 83043c477e18
> 0135394f6d83
> (XEN)83043c47f6d0 82d080807280 83043c47f788 
> (XEN)7cfbc3b88227 82d08023feb2  83043c47f788
> (XEN)82d080807280 83043c47f6d0 83043c477ee0
> 0135394f6d83
> (XEN)01354d22b2fb 830425ae28e0 830425ae2710
> 0001
> (XEN)0008 20c49ba5e353f7cf 83043c47
> 0135394f6d83
> (XEN)83043c47f700 00f5 82d0801ba6d2
> e008
> (XEN)0286 83043c477e60 
> 00080020
> (XEN)013534e2391b  830078a71000 81c17dc0
> (XEN)83043c477f08  
> 82d080832980
> (XEN)1eba6a07  83043c477f18 82d080807280
> (XEN)83043c477f18 830078a71000 83043c423000 83043c477f10
> (XEN)82d0801660fd 81df8000 8300782fe000 0003
> (XEN)830422e7 83043c477d78 81df8000 
> (XEN) 81c17dc0 81a03ea8
> 
> (XEN) 0002d788742b 
> 
> (XEN) Xen call trace:
> (XEN)[] vmx.c#pi_wakeup_interrupt+0xcf/0x15b
> (XEN)[] do_IRQ+0x9d/0x65e
> (XEN)[] common_interrupt+0x62/0x70
> (XEN)[] mwait-idle.c#mwait_idle+0x2b3/0x30a
> (XEN)[] domain.c#idle_loop+0x6c/0x89
> (XEN)
> (XEN)
> (XEN) 
> (XEN) Panic on CPU 3:
> (XEN) GENERAL PROTECTION FAULT
> (XEN) [error_code=]
> (XEN) 
> (XEN)
> (XEN) Reboot in five seconds...
> 
> 
> Best Regards,
> Xudong


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


[Xen-devel] [BUG] Xen panic with VT-d PI

2016-05-25 Thread Hao, Xudong
Xen 4.7 RC3 enabled VT-d PI(iommu=on,intpost), and created HVM guest with VF 
NIC assigned repeatedly, it will cause Xen panic(maybe 2 times ,5 times ,10 
times).
It's not a RC3 regression, at least this Xen commit 1bd52e1f got failure as 
well.

HW: Intel Broadwell server, Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz. 
Xen: Xen 4.7.0 RC3
Dom0: Linux 4.4.4
VF: Intel 82599 NIC

...
(XEN) irq.c:275: Dom8 PCI link 3 changed 5 -> 0
[ 1319.177226] ixgbe :05:00.0 eth30: VF Reset msg received from vf 1
[ 1321.949952] ixgbe :05:00.0 eth30: 1 Spoofed packets detected
(XEN) [ Xen-4.7-unstable  x86_64  debug=y  Tainted:C ]
(XEN) CPU:3
(XEN) RIP:e008:[] vmx.c#pi_wakeup_interrupt+0xcf/0x15b
(XEN) RFLAGS: 00010002   CONTEXT: hypervisor
(XEN) rax: 830078a71760   rbx: 01001001000fff60   rcx: 0003
(XEN) rdx: 0100100100100100   rsi: 0046   rdi: 830078a715c0
(XEN) rbp: 83043c477d18   rsp: 83043c477ce8   r8:  0003
(XEN) r9:  830425ae2710   r10: 830425ae28e0   r11: 01354d22b2fb
(XEN) r12: 83043c47dde0   r13: 83043c47ddf0   r14: 0100100100100100
(XEN) r15: 0200200200200200   cr0: 8005003b   cr4: 003526e0
(XEN) cr3: 000424276000   cr2: 003abfaaca80
(XEN) ds:    es:    fs:    gs:    ss:    cs: e008
(XEN) Xen code around  (vmx.c#pi_wakeup_interrupt+0xcf/0x15b):
(XEN)  00 5a 48 8b 97 a0 01 00 <48> 3b 42 08 74 04 0f 0b eb fe 48 8b 8f a8 01 00
(XEN) Xen stack trace from rsp=83043c477ce8:
(XEN)83043c477d08 00f5 83043c477db8 00f5
(XEN)83043c47f788  83043c477da8 82d080171c7a
(XEN)  deadbeefdeadf00d 8300782fe000
(XEN)0003 830422e7  80003c477d90
(XEN)82d0801949fa 8c7adb3d0002 83043c477e18 0135394f6d83
(XEN)83043c47f6d0 82d080807280 83043c47f788 
(XEN)7cfbc3b88227 82d08023feb2  83043c47f788
(XEN)82d080807280 83043c47f6d0 83043c477ee0 0135394f6d83
(XEN)01354d22b2fb 830425ae28e0 830425ae2710 0001
(XEN)0008 20c49ba5e353f7cf 83043c47 0135394f6d83
(XEN)83043c47f700 00f5 82d0801ba6d2 e008
(XEN)0286 83043c477e60  00080020
(XEN)013534e2391b  830078a71000 81c17dc0
(XEN)83043c477f08   82d080832980
(XEN)1eba6a07  83043c477f18 82d080807280
(XEN)83043c477f18 830078a71000 83043c423000 83043c477f10
(XEN)82d0801660fd 81df8000 8300782fe000 0003
(XEN)830422e7 83043c477d78 81df8000 
(XEN) 81c17dc0 81a03ea8 
(XEN) 0002d788742b  
(XEN) Xen call trace:
(XEN)[] vmx.c#pi_wakeup_interrupt+0xcf/0x15b
(XEN)[] do_IRQ+0x9d/0x65e
(XEN)[] common_interrupt+0x62/0x70
(XEN)[] mwait-idle.c#mwait_idle+0x2b3/0x30a
(XEN)[] domain.c#idle_loop+0x6c/0x89
(XEN)
(XEN)
(XEN) 
(XEN) Panic on CPU 3:
(XEN) GENERAL PROTECTION FAULT
(XEN) [error_code=]
(XEN) 
(XEN)
(XEN) Reboot in five seconds...


Best Regards,
Xudong



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


Re: [Xen-devel] [PATCH v5 03/10] IOMMU/MMU: enhance the call trees of IOMMU unmapping and mapping

2016-05-25 Thread Xu, Quan
On May 26, 2016 12:02 AM, Jan Beulich  wrote:
> >>> On 25.05.16 at 17:34,  wrote:
> > On May 23, 2016 10:19 PM, Jan Beulich  wrote:
> >> >>> On 18.05.16 at 10:08,  wrote:
> >> > +unsigned long type,
> >> > +int preemptible)
> >> >  {
> >> >  unsigned long nx, x, y = page->u.inuse.type_info;
> >> > -int rc = 0;
> >> > +int rc = 0, ret = 0;
> >> >
> >> >  ASSERT(!(type & ~(PGT_type_mask | PGT_pae_xen_l2)));
> >> >
> >> > @@ -2578,11 +2579,11 @@ static int __get_page_type(struct page_info
> >> *page, unsigned long type,
> >> >  if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
> >> >  {
> >> >  if ( (x & PGT_type_mask) == PGT_writable_page )
> >> > -iommu_unmap_page(d, mfn_to_gmfn(d, page_to_mfn(page)));
> >> > +ret = iommu_unmap_page(d, mfn_to_gmfn(d,
> >> > + page_to_mfn(page)));
> >> >  else if ( type == PGT_writable_page )
> >> > -iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)),
> >> > -   page_to_mfn(page),
> >> > -   IOMMUF_readable|IOMMUF_writable);
> >> > +ret = iommu_map_page(d, mfn_to_gmfn(d,
> page_to_mfn(page)),
> >> > + page_to_mfn(page),
> >> > +
> >> > + IOMMUF_readable|IOMMUF_writable);
> >> >  }
> >> >  }
> >> >
> >> > @@ -2599,6 +2600,9 @@ static int __get_page_type(struct page_info
> >> *page, unsigned long type,
> >> >  if ( (x & PGT_partial) && !(nx & PGT_partial) )
> >> >  put_page(page);
> >> >
> >> > +if ( !rc )
> >> > +rc = ret;
> >>
> >> I know I've seen this a couple of time already, but with the special
> >> purpose of "ret" I really wonder whether a more specific name
> >> wouldn't be warranted - e.g. "iommu_rc" or "iommu_ret".
> >
> >
> > rc is return value for this function, and no directly association with
> > IOMMU related code ( rc is only for alloc_page_type() ).
> > So the rc cannot be "iommu_rc"..
> >
> > ret can be "iommu_ret", but I think the pair 'rc' / 'ret' may look good.
> 
> Well, I'm not entirely opposed to keeping the names, but I continue to think
> that while at the call sites the shorter name is reasonable, it is quite a 
> bit less
> clear at the point where you conditionally update rc.
> 

I am open to it. What about 'rc' / 'iommu_ret' ?

Quan

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


[Xen-devel] [qemu-upstream-4.3-testing test] 94776: trouble: blocked/broken

2016-05-25 Thread osstest service owner
flight 94776 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94776/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-pvops  3 host-install(3) broken REGR. vs. 80927
 build-amd64   3 host-install(3) broken REGR. vs. 80927
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927
 build-i3863 host-install(3) broken REGR. vs. 80927

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-pv1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-pv   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a

version targeted for testing:
 qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  109 days
Testing same since93977  2016-05-10 11:09:16 Z   15 days   58 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Stefano Stabellini 

jobs:
 build-amd64  broken  
 build-i386   broken  
 build-amd64-libvirt  blocked 
 build-i386-libvirt   blocked 
 build-amd64-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-xl  blocked 
 test-amd64-i386-xl   blocked 
 test-amd64-i386-qemuu-rhel6hvm-amd   blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked 
 test-amd64-i386-freebsd10-amd64  blocked 
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 
 test-amd64-amd64-xl-qemuu-win7-amd64 blocked 
 test-amd64-i386-xl-qemuu-win7-amd64  blocked 
 test-amd64-amd64-xl-credit2  blocked 
 test-amd64-i386-freebsd10-i386   blocked 
 test-amd64-i386-qemuu-rhel6hvm-intel blocked 
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpublocked 
 

[Xen-devel] [PATCH] x86/mce: handle DOMID_XEN properly in XEN_MC_msrinject

2016-05-25 Thread Haozhong Zhang
Commit 26646f3 "x86/mce: translate passed-in GPA to host machine
address" forgot to consider dom_xen, which fails tools/xen-mceinj when
it's going to inject into domain DOMID_XEN (e.g. when -d option is not
used) via XEN_MC_msrinject. Use dom_xen when the domain id DOMID_XEN is
passed in.

Signed-off-by: Haozhong Zhang 
---
 xen/arch/x86/cpu/mcheck/mce.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index cc446eb..65de627 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1427,17 +1427,18 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
 
 if ( mc_msrinject->mcinj_flags & MC_MSRINJ_F_GPADDR )
 {
-struct domain *d;
+domid_t domid = mc_msrinject->mcinj_domid;
+struct domain *d = (domid == DOMID_XEN) ?
+   dom_xen : get_domain_by_id(domid);
 struct mcinfo_msr *msr;
 unsigned int i;
 paddr_t gaddr;
 unsigned long gfn, mfn;
 p2m_type_t t;
 
-d = get_domain_by_id(mc_msrinject->mcinj_domid);
 if ( d == NULL )
 return x86_mcerr("do_mca inject: bad domain id %d",
- -EINVAL, mc_msrinject->mcinj_domid);
+ -EINVAL, domid);
 
 for ( i = 0, msr = _msrinject->mcinj_msr[0];
   i < mc_msrinject->mcinj_count;
@@ -1452,7 +1453,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
 put_gfn(d, gfn);
 put_domain(d);
 return x86_mcerr("do_mca inject: bad gfn %#lx of domain 
%d",
- -EINVAL, gfn, mc_msrinject->mcinj_domid);
+ -EINVAL, gfn, domid);
 }
 
 msr->value = pfn_to_paddr(mfn) | (gaddr & (PAGE_SIZE - 1));
-- 
2.8.3


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


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

2016-05-25 Thread osstest service owner
flight 94770 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94770/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt-qcow2  9 debian-di-install   fail REGR. vs. 94756

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeatfail   like 94746
 build-amd64-rumpuserxen   6 xen-buildfail   like 94756
 build-i386-rumpuserxen6 xen-buildfail   like 94756
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 94756
 test-amd64-amd64-xl-rtds  9 debian-install   fail   like 94756
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 94756
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 94756
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 94756

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-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail 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-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-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-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-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-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-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

version targeted for testing:
 xen  1260c7fdb91dfe0033d6eef0e94b93607be020a9
baseline version:
 xen  4504b7d1a6594c8ad4b1ecdab15d30feca7eaa51

Last test of basis94756  2016-05-25 06:32:18 Z0 days
Testing same since94770  2016-05-25 17:11:38 Z0 days1 attempts


People who touched revisions under test:
  Dario Faggioli 
  Ian Jackson 
  Julien Grall 
  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
 

[Xen-devel] [xen-unstable-smoke test] 94774: tolerable all pass - PUSHED

2016-05-25 Thread osstest service owner
flight 94774 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94774/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-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

version targeted for testing:
 xen  8478c9409a2c6726208e8dbc9f3e455b76725a33
baseline version:
 xen  1260c7fdb91dfe0033d6eef0e94b93607be020a9

Last test of basis94767  2016-05-25 15:01:35 Z0 days
Testing same since94769  2016-05-25 17:01:46 Z0 days2 attempts


People who touched revisions under test:
  Ian Jackson 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=8478c9409a2c6726208e8dbc9f3e455b76725a33
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
8478c9409a2c6726208e8dbc9f3e455b76725a33
+ branch=xen-unstable-smoke
+ revision=8478c9409a2c6726208e8dbc9f3e455b76725a33
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.6-testing
+ '[' x8478c9409a2c6726208e8dbc9f3e455b76725a33 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src 
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local 'options=[fetch=try]'
 getconfig GitCacheProxy
 perl -e '
use Osstest;
readglobalconfig();
print $c{"GitCacheProxy"} or die $!;
'
+++ local cache=git://cache:9419/
+++ '[' 

[Xen-devel] [qemu-upstream-4.3-testing test] 94772: trouble: blocked/broken

2016-05-25 Thread osstest service owner
flight 94772 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94772/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-pvops  3 host-install(3) broken REGR. vs. 80927
 build-amd64   3 host-install(3) broken REGR. vs. 80927
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927
 build-i3863 host-install(3) broken REGR. vs. 80927

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-pv1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-pv   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a

version targeted for testing:
 qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  109 days
Testing same since93977  2016-05-10 11:09:16 Z   15 days   57 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Stefano Stabellini 

jobs:
 build-amd64  broken  
 build-i386   broken  
 build-amd64-libvirt  blocked 
 build-i386-libvirt   blocked 
 build-amd64-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-xl  blocked 
 test-amd64-i386-xl   blocked 
 test-amd64-i386-qemuu-rhel6hvm-amd   blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked 
 test-amd64-i386-freebsd10-amd64  blocked 
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 
 test-amd64-amd64-xl-qemuu-win7-amd64 blocked 
 test-amd64-i386-xl-qemuu-win7-amd64  blocked 
 test-amd64-amd64-xl-credit2  blocked 
 test-amd64-i386-freebsd10-i386   blocked 
 test-amd64-i386-qemuu-rhel6hvm-intel blocked 
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpublocked 
 

Re: [Xen-devel] [PATCH v3 14/16] x86/boot: implement early command line parser in C

2016-05-25 Thread Daniel Kiper
On Wed, May 25, 2016 at 04:33:54AM -0600, Jan Beulich wrote:
> >>> On 15.04.16 at 14:33,  wrote:
> > --- a/xen/arch/x86/boot/build32.lds
> > +++ b/xen/arch/x86/boot/build32.lds
> > @@ -25,6 +25,7 @@ SECTIONS
> >  *(.text)
> >  *(.text.*)
> >  *(.rodata)
> > +*(.rodata.*)
> >}
>
> Interesting - didn't you say you don't want this for now?

Yep, however, I also said that we should add "*(.rodata.*)"
if it is needed. And it is needed here.

> > --- /dev/null
> > +++ b/xen/arch/x86/boot/cmdline.c
> > @@ -0,0 +1,357 @@
> > +/*
> > + * Copyright (c) 2015, 2016 Oracle and/or its affiliates. All rights 
> > reserved.
> > + *  Daniel Kiper 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program.  If not, see .
> > + *
> > + * strlen(), strncmp(), strspn() and strcspn() were copied from
> > + * Linux kernel source (linux/lib/string.c).
>
> Any reason you can't just #include ".../common/string.c" here?

I am confused. Sometimes you request to reduce number of such strange
includes and sometimes you ask me to do something contrary? So, what
is the rule behind these requests?

Additionally, there is a chance that final xen ELF file will be
unnecessary larger because it will contain unused functions. Wait,
maybe there is a linker option which allows us to remove unreferenced
objects from final output.

> > +/*
> > + * Space and TAB are obvious delimiters. However, I am
> > + * adding "\n" and "\r" here too. Just in case when
> > + * crazy bootloader/user put them somewhere.
> > + */
> > +#define DELIM_CHARS" \n\r\t"
> > +#define DELIM_CHARS_COMMA  DELIM_CHARS ","
>
> static const char[] variables (or really just one, with the comma put
> first and the non-comma variant indexing into that variable by 1)?

OK.

> > +#define __packed   __attribute__((__packed__))
>
> No way to include compiler.h here?

Ditto.

> > +/*
> > + * Compiler is not able to optimize regular strlen()
> > + * if argument is well known string during build.
> > + * Hence, introduce optimized strlen_opt().
> > + */
> > +#define strlen_opt(s) (sizeof(s) - 1)
>
> Do we really care in this code?

Not to strongly but why not?

> > +/* Keep in sync with trampoline.S:early_boot_opts label! */
> > +typedef struct __packed {
> > +u8 skip_realmode;
> > +u8 opt_edd;
> > +u8 opt_edid;
> > +u16 boot_vid_mode;
> > +u16 vesa_width;
> > +u16 vesa_height;
> > +u16 vesa_depth;
> > +} early_boot_opts_t;
>
> This "keeping in sync" should be automated in some way, e.g. via
> a new header and suitable macroization.

OK.

> > +static int strtoi(const char *s, const char *stop, const char **next)
> > +{
> > +int base = 10, i, ores = 0, res = 0;
>
> You don't even handle a '-' on the numbers here, so all the variables

Yep, however, ...

> and the function return type should be unsigned int afaict. And the
> function name perhaps be strtoui().

... we return -1 in case of error.

> > +if ( *s == '0' )
> > +  base = (tolower(*++s) == 'x') ? (++s, 16) : 8;
> > +
> > +for ( ; *s != '\0'; ++s )
> > +{
> > +for ( i = 0; stop && stop[i] != '\0'; ++i )
> > +if ( *s == stop[i] )
> > +goto out;
>
> strchr()?

Could be.

> > +if ( *s < '0' || (*s > '7' && base == 8) )
> > +{
> > +res = -1;
> > +goto out;
> > +}
> > +
> > +if ( *s > '9' && (base != 16 || tolower(*s) < 'a' || tolower(*s) > 
> > 'f') )
> > +{
> > +res = -1;
> > +goto out;
> > +}
> > +
> > +res *= base;
> > +res += (tolower(*s) >= 'a') ? (tolower(*s) - 'a' + 10) : (*s - 
> > '0');
>
> With the four instances, how about latching tolower(*s) into a local
> variable?

Make sense.

> > +static u8 skip_realmode(const char *cmdline)
> > +{
> > +return !!find_opt(cmdline, "no-real-mode", 0) || !!find_opt(cmdline, 
> > "tboot=", 1);
>
> The || makes the two !! pointless.
>
> Also please settle on which type you want to use for boolean
> (find_opt()'s last parameter is "int", yet here you use "u8"), and

Could be u8.

> perhaps make yourself a bool_t.

I do not think it make sense here.

> > +static void vga_parse(const char *cmdline, early_boot_opts_t *ebo)
> > +{
> > +const char *c;
> > +int 

Re: [Xen-devel] [PATCH v3 13/16 - RFC] x86: add multiboot2 protocol support for EFI platforms

2016-05-25 Thread Daniel Kiper
On Wed, May 25, 2016 at 03:32:37AM -0600, Jan Beulich wrote:
> >>> On 15.04.16 at 14:33,  wrote:
> > @@ -100,19 +107,29 @@ multiboot2_header_end:
> >  gdt_boot_descr:
> >  .word   6*8-1
> >  .long   sym_phys(trampoline_gdt)
> > +.long   0 /* Needed for 64-bit lgdt */
> > +
> > +cs32_switch_addr:
> > +.long   sym_phys(cs32_switch)
> > +.word   BOOT_CS32
> >
> >  .Lbad_cpu_msg: .asciz "ERR: Not a 64-bit CPU!"
> >  .Lbad_ldr_msg: .asciz "ERR: Not a Multiboot bootloader!"
> > +.Lbad_ldr_mb2: .asciz "ERR: On EFI platform use latest Multiboot2 
> > compatible bootloader!"
>
> What is "latest" going to mean 5 years from now?

:-))) I will try to fix it.

> >  .section .init.text, "ax", @progbits
> >
> >  bad_cpu:
> >  mov $(sym_phys(.Lbad_cpu_msg)),%esi # Error message
> > -jmp print_err
> > +mov $0xB8000,%edi   # VGA framebuffer
> > +jmp 1f
> >  not_multiboot:
> >  mov $(sym_phys(.Lbad_ldr_msg)),%esi # Error message
> > -print_err:
> > -mov $0xB8000,%edi  # VGA framebuffer
> > +mov $0xB8000,%edi   # VGA framebuffer
> > +jmp 1f
> > +mb2_too_old:
> > +mov $(sym_phys(.Lbad_ldr_mb2)),%esi # Error message
> > +xor %edi,%edi   # No VGA framebuffer
>
> Leaving aside that "framebuffer" really is a bad term here (we're
> talking of text mode output after all), limiting the output to serial

Yep, but then we should change this in other places too. Maybe in separate 
patch.

> isn't going to be very helpful in the field, I'm afraid. Even more so
> that there's no guarantee for a UART to be at port 0x3f8. That's

Right but we do not have big choice here at very early boot stage... :-(((

> not much of a problem for the other two messages as people are
> unlikely to try to boot Xen on an unsuitable system, but I view it
> as quite possible for Xen to be tried to get booted with an
> unsuitable grub2.
>
> IOW - this needs a better solution, presumably using EFI boot
> service output functions.

No way, here boot services are dead. GRUB2 (or other loader)
shutdown them... :-(((

> > @@ -130,6 +149,130 @@ print_err:
> >  .Lhalt: hlt
> >  jmp .Lhalt
> >
> > +.code64
> > +
> > +__efi64_start:
>
> As long as we have split files under boot/, I think I'd prefer 64-bit
> code to only go into x86_64.S.
>
> > +cld
> > +
> > +/* Check for Multiboot2 bootloader. */
> > +cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
> > +je  efi_multiboot2_proto
> > +
> > +/* Jump to not_multiboot after switching CPU to x86_32 mode. */
> > +lea not_multiboot(%rip),%rdi
> > +jmp x86_32_switch
>
> What I've said above would also eliminate the need to switch to
> 32-bit mode just for emitting an error message and halting the
> system.

It is not possible. We just know that we run on EFI platform here.
However, we are not able to get EFI SystemTable pointer.

> > +efi_multiboot2_proto:
>
> .Lefi_multiboot2_proto

OK if you insist. However, I think that we are loosing helpful
debug information this way.

> > +/*
> > + * Multiboot2 information address is 32-bit,
> > + * so, zero higher half of %rbx.
> > + */
> > +mov %ebx,%ebx
>
> Wait, no - that's a protocol bug then. We're being entered in 64-bit
> mode here, so registers should be in 64-bit clean state.

You mean higher half cleared. Right? This is not guaranteed.
Please check this: 
http://lists.gnu.org/archive/html/grub-devel/2016-03/msg00304.html

> > +/* Skip Multiboot2 information fixed part. */
> > +lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%rbx),%rcx
> > +and $~(MULTIBOOT2_TAG_ALIGN-1),%rcx
>
> Or if there really was a reason to do the above, and if there is a
> reason not to assume this data is located below 4Gb, then
> calculations like this could avoid the REX64 prefix by using %ecx.

Here you said something different:
  http://lists.xen.org/archives/html/xen-devel/2015-03/msg03557.html

So?

> > +0:
> > +/* Get EFI SystemTable address from Multiboot2 information. */
> > +cmpl$MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%rcx)
> > +jne 1f
> > +
> > +mov MB2_efi64_st(%rcx),%rsi
> > +
> > +/* Do not clear BSS twice and do not go into real mode. */
> > +movb$1,skip_realmode(%rip)
>
> How is the setting of skip_realmode related to the clearing of BSS?
> Oh, I've found the connection below (albeit see there for its
> validity), but I think mentioning this here is more confusing than
> clarifying.

In general I have a problem skip_realmode. The name itself is confusing
here and there. Probably it should be changed. However, I do not have
idea for good compact name describing all skip_realmode usages. And I do
not think that we should add another 

Re: [Xen-devel] live migrating hvm from 4.4 to 4.7 fails in ioreq server

2016-05-25 Thread Konrad Rzeszutek Wilk
On Thu, May 12, 2016 at 02:13:21PM +, Paul Durrant wrote:
> > -Original Message-
> [snip]
> > >
> > > Ok. Do you regard this as a critical issue for 4.7?
> > >
> > 
> > Our general support statement is to support N->N+1 migration, so it is
> > not really critical for me. On the other hand, if the fix is not overly
> > complex, it would be nice to have for 4.7.
> > 
> > Note that the fix will need to be in upstream QEMU first before it can
> > be cherry-picked to our tree, so there is risk that it might just be
> > blocked on QEMU side (I haven't checked their schedule). So I wouldn't
> > really block xen release just for that.
> > 
> 
> Ok.
> 
> > If for some reason (either you don't have time or the patch is blocked
> > on QEMU side) the fix doesn't make 4.7.0 I would suggest QEMU maintainer
> > to backport to 4.7.1 etc.
> > 
> 
> I'll try to get to it as soon as I can, but my guess is that it will miss 
> 4.7.0.

+CC Zhigang.

Any ideas on the timeline for this fix? Thanks!
> 
>   Paul
> 
> ___
> 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] Question about the best practice to install two versions of Xen toolstack on the same machine

2016-05-25 Thread Meng Xu
On Tue, May 24, 2016 at 11:16 AM, Dario Faggioli
 wrote:
> [trimmed To/Cc]
>
> On Fri, 2016-05-20 at 13:56 -0400, Meng Xu wrote:
>> On Fri, May 20, 2016 at 6:20 AM, Jan Beulich 
>> wrote:
>> > Or, as an alternative to Olaf's reply, don't install the tools at
>> > all, but
>> > instead run everything right out of the build trees. That requires
>> > some
>> > script wrappers to get things like the library search path set up
>> > correctly, but with that in place it has been working fine for me
>> > for
>> > years.
>> >
>> Thank you so much for your suggestions!  I tried to add the library
>> and bins in xen/dist/install into the PATH and LD_LIBRARY_PATH, but
>> failed to have the system work properly.
>>
>> I'm wondering if it's convenient for you, would you mind sharing your
>> script wrapper? I can learn from it and customize it for my machine.
>>
>> Once I have it set up successfully, I can write a xen wiki to
>> describe
>> how to do it.
>>
> Hey Meng,
>
> Now that you've got a couple of alternatives, feel free to go and write
> the wiki page! :-P

I have added a separate wiki page for this.
I finally took Olaf and Donglin's approach. So I just documented that
approach. :-)

Please feel free to modify that page. I'm thinking if I should put a
link to the new page?


Thanks and Best Regards,

Meng
---
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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


Re: [Xen-devel] Question about the best practice to install two versions of Xen toolstack on the same machine

2016-05-25 Thread Meng Xu
Hi Wei,

On Wed, May 25, 2016 at 6:53 AM, Wei Liu  wrote:
> On Tue, May 24, 2016 at 04:47:38PM -0400, Meng Xu wrote:
>> Hi Olaf,
>>
>> Thank you very much for your suggestion!
>>
>> On Fri, May 20, 2016 at 4:52 AM, Olaf Hering  wrote:
>> > On Thu, May 19, Meng Xu wrote:
>> >
>> >> Does anyone try to install two version of Xen toolstack on the same 
>> >> machine?
>> >
>> > I do that. See the INSTALL file which has examples at the end:
>> >
>> > * To build a private copy of tools and xen:
>> > configure --prefix=/odd/path --sysconfdir=/odd/path/etc --enable-rpath
>> > make
>> > sudo make install BOOT_DIR=/ood/path/boot EFI_DIR=/odd/path/efi
>> >
>>
>> I'm wondering if  BOOT_DIR=/ood/path/boot and EFI_DIR=/odd/path/efi is a 
>> must?
>> I'm using the grub2 to boot the kernel (so that I can remotely control
>> which grub entry I will get into).
>>
>> I put the xen image to /boot and the system can boot up. The xl
>> toolstack seems working well.
>>
>> However, the dom0's name is null.
>>
>> When I run /ood/path/etc/init.d/xencommons restart , it reports the
>> following message:
>>
>> Stopping xenconsoled
>> Stopping QEMU
>> WARNING: Not stopping xenstored, as it cannot be restarted.
>> Starting xenconsoled...
>> Starting QEMU as disk backend for dom0
>>
>> I guess there is something wrong with the xenstored's configuration?
>> Did you happen to experience this issue before?
>>
>>
>
> I think xenstored is running fine.
>
> If you're using systemd, there should be a service called xen-init-dom0
> that needs to be started. That small program does a bunch of things
> including setting the name for Dom0 in xenstore.

Yes, you are right! After running xen-init-dom0, everything works now.

BTW, I followed Olaf and Dongli's approach. I'm writing the wiki now...

-- 
Best Regards,

Meng
---
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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


Re: [Xen-devel] [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator

2016-05-25 Thread Daniel Kiper
On Wed, May 25, 2016 at 02:39:57AM -0600, Jan Beulich wrote:
> >>> On 15.04.16 at 14:33,  wrote:
> > There is a problem with place_string() which is used as early memory
> > allocator. It gets memory chunks starting from start symbol and
> > going down. Sadly this does not work when Xen is loaded using multiboot2
> > protocol because start lives on 1 MiB address. So, I tried to use
> > mem_lower address calculated by GRUB2. However, it works only on some
> > machines. There are machines in the wild (e.g. Dell PowerEdge R820)
> > which uses first ~640 KiB for boot services code or data... :-(((
> >
> > In case of multiboot2 protocol we need that place_string() only allocate
> > memory chunk for EFI memory map. However, I think that it should be fixed
> > instead of making another function used just in one case. I thought about
> > two solutions.
> >
> > 1) We could use native EFI allocation functions (e.g. AllocatePool()
> >or AllocatePages()) to get memory chunk. However, later (somewhere
> >in __start_xen()) we must copy its contents to safe place or reserve
> >this in e820 memory map and map it in Xen virtual address space.
> >In later case we must also care about conflicts with e.g. crash
> >kernel regions which could be quite difficult.
>
> I don't see why that would be: Simply use an allocation type that
> doesn't lead to the area getting consumed as normal RAM. Nor do
> I see the kexec collision potential. Furthermore (and I think I've
> said so before) ARM is already using AllocatePool() - just with an
> unsuitable memory type -, so doing so on x86 too would allow for

Nope, they are using standard EfiLoaderData.

> efi_arch_allocate_mmap_buffer() to go away.

That would be great, so, I will think how to solve this issue.

> > Jan Beulich added 1b) Do away with efi_arch_allocate_mmap_buffer() and use
> >AllocatePages() uniformly, perhaps with a per-arch specified memory type
> >(by means of which you can control whether the memory contents will 
> > remain
> >preserved until the time you want to look at it). That will eliminate the
> >only place_string() you're concerned about, with a patch with better
> >diffstat (largely due to the questionable arch hook gone).
> >
> > However, this solution does not solve conflicts problem described in #1
> > because EFI memory map is needed during Xen runtime after init phase.
> > So, finally we would get back to #1. Hmmm... Should I check how Linux
> > and others cope with that problem?
>
> Ah, here you mention it actually. Yet you don't explain what conflict
> potential you see once using EfiRuntimeServicesData for the allocation.

Good point! IMO, if crash kernel region conflicts with EfiRuntimeServices*
then we should display warning that it cannot be allocated. By the way,
once you mentioned that you have in your queue (I suppose that it is
extremely long) kdump patch which adds functionality to automatically
establish crash kernel region placement. I think that could solve (at
least partially) problem with conflicts. Could you post it?

Daniel

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


[Xen-devel] Basic bare metal ARM domain interface

2016-05-25 Thread Ivan Pavić2
Hello,

I'm working on bare metal application for ARM Cortex A7/15 on Odroid XU3 
platform. I'm using Xen 4.6.

After successfully creating bare metal example(by successful I mean I've stuck 
processor in while loop in main function), I probably should initialize memory
management unit and similiar. I've found FreeRTOS project on Github where 
similiar stuff was done for ARM Cortex A15. Project is done for Xen 4.4. 
Xen FreeRTOS project: https://github.com/GaloisInc/FreeRTOS-Xen/ .

1) I'm using 4.6. so I don't know if code from  is fully compatible? (I had to 
comment out lot of things because they block program, printing for example) 

2) Additionally , I would like to implement basic serial output to dom0 from my 
bare metal domU. What is the minimum one should do for implementing console 
output?  

3) Furthermore, as this should be bare metal application, I would like at least 
to be able to toggle LED. How can interface hardware from domU? My first guess
would be to directly address Exynos 5422 GPIO registers but I don't know if
that would work beacause application is running on VCPU (probably not??) ??

Any answer, example of implementation, specific documentation or advice would
be very helpful.

Thank you in advance...

Regards,
Ivan Pavić
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3] altp2m: Allow the hostp2m to be shared

2016-05-25 Thread Tamas K Lengyel
On Wed, May 25, 2016 at 12:25 PM, George Dunlap
 wrote:
> On Wed, May 25, 2016 at 5:58 PM, Tamas K Lengyel  wrote:
>> On Wed, May 25, 2016 at 10:08 AM, George Dunlap
>>  wrote:
>>> On Wed, May 25, 2016 at 4:31 PM, Tamas K Lengyel  
>>> wrote:

 On May 25, 2016 05:27, "George Dunlap"  wrote:
>
> On Fri, Apr 29, 2016 at 6:42 PM, Tamas K Lengyel 
> wrote:
> > Don't propagate altp2m changes from ept_set_entry for memshare as
> > memshare
> > already has the lock. We call altp2m propagate changes once memshare
> > successfully finishes. Allow the hostp2m entries to be of type
> > p2m_ram_shared when applying mem_access. Also, do not trigger PoD for
> > hostp2m
> > when setting altp2m mem_access to be in-line with non-altp2m mem_access
> > path.
>
> Hey Tamas,
>
> Sorry for the long delay in getting back to you on this.

 No problem, thanks for taking a closer look!

>
> So the main issue here (correct me if I'm wrong) is the locking
> discipline: namely, men_sharing_share_pages():
> - Grabs the hostp2m lock
> - Grabs the appropriate domain memsharing locks
> - Calls set_shared_p2m_entry(), which ends up calling ept_set_entry(),
> which (when altp2m is active) grabs the altp2mlist and altp2m locks.
>
> This causes an ASSERT(), since the altp2mlist lock is ahead of the
> memsharing locks in the list.
>
> But having taken a closer look at the code, I'm not sure the change is
> quite correct.  Please correct me if I've misread something:
>
> mem_sharing_share_pages() is passed two  pairs -- the
>  (which I assume stands for "shared gfn") and 
> (which I assume stands for "copy"); and it

 Here s/c stands for source/client.

> 1) Looks up smfn and cmfn, which back sgfn and cmfn respectively
> 2) Looks up cmfn, which backs cgfn then replaces all gfn entries which
> point to cmfn with smfn (updating accounting as appropriate)

 Hm, I might have missed that. Where does it do the lookup for all other
 cgfns backed by this cmfn?
>>>
>>> I was looking at the loop in the middle of the function:
>>>
>>> while ( (gfn = rmap_iterate(cpage, )) != NULL) {
>>>  ...
>>> }
>>>
>>> I haven't chased it down, but it looks like this walks the reverse map
>>> of all gfns which map cpage; and for each such gfn it finds it:
>>> * removes the cpage -> gfn rmap
>>> * Adds an spage -> gfn map
>>> * Reduces the type count of cpage
>>> * Sets the p2m entry for that gfn to the smfn (rather than cmfn).
>>>
>>> Obviously the common case is that the number of mappings is exactly 1;
>>> but we need to either ensure that this is always true, or we need to
>>> handle the case where it's not true. :-)
>>>
> But this change will only call p2m_altp2m_propagate_change() for the
> original cgfn -- any other gfns which are backed by cmfn will not have
> the corresponding altp2m entries propagated properly.

 Right, if there is some other place where it does sharing in the back we
 would have to propagate that change.

> This sort of mistake is easy to make, which is why I think we should
> try to always update the altp2ms in ept_set_entry() if we can, to
> minimize the opportunity for making this sort of mistake.
>
> Is there ever a reason to grab the altp2m lock and *then* grab the
> sharing lock?  Could we just move the sharing lock up between the p2m
> lock and the altp2mlist lock instead?
>

 I can't think of a scenario where we would get to sharing from altp2m with
 altp2m locking first. Not sure what you mean by moving the sharing lock up
 though. The problem is that sharing already has the lock by the time altp2m
 tries to lock, so we could pass that info down to make altp2m aware it 
 needs
 no locking. It would require extending a bunch of functions though with an
 extra input that is barely ever used..
>>>
>>> If you have altp2m there are three locks.  There's one p2m lock for
>>> the "host" p2m (that is, Xen's idea of what the mapping should look
>>> like).  Then there's the altp2mlist lock, which protects the *list* of
>>> altp2ms; then each altp2m itself has its own lock.  These are defined
>>> in mm-lock.h and  must be grabbed in that order: p2m before
>>> altp2mlist, altp2mlist before altp2m.
>>>
>>> I assume that the memsharing code is grabbing the hostp2m lock (it
>>> should be anyway), then grabbing the memsharing locks. This is allowed
>>> because the memsharing locks are defined after the p2m lock in
>>> mm-lock.h.  But then when updating the p2m entry, if you have an
>>> altp2m active, it then tries to grab the altp2mlist lock so it can
>>> iterate over the altp2ms.  Since the altp2mlist lock is 

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

2016-05-25 Thread osstest service owner
flight 94758 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94758/

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

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

Last test of basis94748  2016-05-24 22:43:25 Z0 days
Failing since 94750  2016-05-25 03:43:08 Z0 days3 attempts
Testing same since94758  2016-05-25 09:14:19 Z0 days1 attempts


People who touched revisions under test:
  Dandan Bi 
  Hao Wu 
  Marvin H?user 
  Marvin Haeuser 
  Yonghong Zhu 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  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.


commit 8caa3caaed4b32d699b79c6d5aaa606b52d740e7
Author: Dandan Bi 
Date:   Mon May 23 14:54:46 2016 +0800

MdeModulePkg: Make function comments and function match in UI codes

Cc: Qiu Shumin 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
Reviewed-by: Qiu Shumin 

commit 4b7345a7dd71bdc99a824facf55066838ec240da
Author: Dandan Bi 
Date:   Thu May 19 14:17:34 2016 +0800

MdeModulePkg/DisplayEngine: Fix memory leak issues in DisplayEngine

The following codes are useless and cause memory leak issues.
So now remove them.

Cc: Cecil Sheng 
Cc: Qiu Shumin 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 

commit e4979beee9e5d334d97fd8e2c79670ad08587bc6
Author: Yonghong Zhu 
Date:   Fri May 6 15:20:23 2016 +0800

BaseTools/GenFds: enhance to get TOOL_CHAIN_TAG and TARGET value

when user don't set TOOL_CHAIN_TAG and TARGET by –D Flag, then GenFds
would report failure for format:
FILE DATA = $(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/testfile
so this patch enhance to get the TOOL_CHAIN_TAG and TARGET value by
following priority (high to low): 1. the Macro value set by -D Flag;
2. Get the value by the -t/-b option. 3. get the value from target.txt
file. Besides, this patch also remove the error checking for missing
-t/-b option.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
Reviewed-by: Liming Gao 

commit 07fb9c264400d7ca2c14d3d8076102584038eb96
Author: Hao Wu 
Date:   Mon May 23 11:40:30 2016 +0800

MdeModulePkg RamDiskDxe: VALID_ARCH cleanup to list supported options

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
Reviewed-by: Feng Tian 

commit bd3fc8133b2b17ad2e0427d1bf6b44b08cf2f3b2
Author: Marvin H?user 
Date:   Fri May 20 03:04:02 2016 +0800

ShellPkg/App: Fix memory leak and save resources.

1) RunSplitCommand() allocates the initial SplitStdOut via
   CreateFileInterfaceMem(). Free SplitStdIn after the swap to fix
   the memory 

Re: [Xen-devel] [PATCH v2 08/13] ACPICA: Hardware: Add optimized access bit width support

2016-05-25 Thread Boris Ostrovsky
On 05/05/2016 12:58 AM, Lv Zheng wrote:
> ACPICA commit c49a751b4dae7baec1790748a2b4b6e8ab599f51
>
> For Access Size = 0, it actually can use user expected access bit width.
> This patch implements this.
>
> Besides of the ACPICA upstream commit, this patch also includes a fix fixing
> the issue reported by the FreeBSD community.
> The old register descriptors are translated in acpi_tb_init_generic_address()
> with access_width being filled with 0. This breaks code in
> acpi_hw_get_access_bit_width() when the registers are 16-bit IO ports and 
> their
> bit_width fields are filled with 16. The rapid fix is meant to make code
> written for acpi_hw_get_access_bit_width() regression safer before the issue 
> is
> correctly fixed from acpi_tb_init_generic_address(). Reported by
> John Baldwin , fixed by Lv Zheng , 
> tested
> by Jung-uk Kim .
>
> Link: https://github.com/acpica/acpica/commit/c49a751b
> Reported-by: John Baldwin 
> Tested-by Jung-uk Kim .
> Signed-off-by: Lv Zheng 
> Signed-off-by: Bob Moore 
> ---
>  drivers/acpi/acpica/hwregs.c |   49 
> --
>  1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/acpica/hwregs.c b/drivers/acpi/acpica/hwregs.c
> index 035fb52..892e677 100644
> --- a/drivers/acpi/acpica/hwregs.c
> +++ b/drivers/acpi/acpica/hwregs.c
> @@ -51,6 +51,10 @@ ACPI_MODULE_NAME("hwregs")
>  
>  #if (!ACPI_REDUCED_HARDWARE)
>  /* Local Prototypes */
> +static u8
> +acpi_hw_get_access_bit_width(struct acpi_generic_address *reg,
> +  u8 max_bit_width);
> +
>  static acpi_status
>  acpi_hw_read_multiple(u32 *value,
> struct acpi_generic_address *register_a,
> @@ -65,6 +69,48 @@ acpi_hw_write_multiple(u32 value,
>  
>  
> /**
>   *
> + * FUNCTION:acpi_hw_get_access_bit_width
> + *
> + * PARAMETERS:  reg - GAS register structure
> + *  max_bit_width   - Max bit_width supported (32 or 64)
> + *
> + * RETURN:  Status
> + *
> + * DESCRIPTION: Obtain optimal access bit width
> + *
> + 
> **/
> +
> +static u8
> +acpi_hw_get_access_bit_width(struct acpi_generic_address *reg, u8 
> max_bit_width)
> +{
> + u64 address;
> +
> + if (!reg->access_width) {
> + /*
> +  * Detect old register descriptors where only the bit_width 
> field
> +  * makes senses. The target address is copied to handle possible
> +  * alignment issues.
> +  */
> + ACPI_MOVE_64_TO_64(, >address);
> + if (!reg->bit_offset && reg->bit_width &&
> + ACPI_IS_POWER_OF_TWO(reg->bit_width) &&
> + ACPI_IS_ALIGNED(reg->bit_width, 8) &&
> + ACPI_IS_ALIGNED(address, reg->bit_width)) {
> + return (reg->bit_width);
> + } else {
> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> + return (32);

This (together with "... Add access_width/bit_offset support in
acpi_hw_write") breaks Xen guests using older QEMU which doesn't support
4-byte IO accesses.

Why not return "reg->bit_width?:max_bit_width" ? This will preserve
original behavior.

-boris

> + } else {
> + return (max_bit_width);
> + }
> + }
> + } else {
> + return (1 << (reg->access_width + 2));
> + }
> +}
> +
> +/**
> + *
>   * FUNCTION:acpi_hw_validate_register
>   *
>   * PARAMETERS:  reg - GAS register structure
> @@ -122,8 +168,7 @@ acpi_hw_validate_register(struct acpi_generic_address 
> *reg,
>  
>   /* Validate the bit_width, convert access_width into number of bits */
>  
> - access_width = reg->access_width ? reg->access_width : 1;
> - access_width = 1 << (access_width + 2);
> + access_width = acpi_hw_get_access_bit_width(reg, max_bit_width);
>   bit_width =
>   ACPI_ROUND_UP(reg->bit_offset + reg->bit_width, access_width);
>   if (max_bit_width < bit_width) {



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


Re: [Xen-devel] [PATCH v3 11/16] efi: build xen.gz with EFI code

2016-05-25 Thread Daniel Kiper
On Wed, May 25, 2016 at 01:53:31AM -0600, Jan Beulich wrote:
> >>> On 15.04.16 at 14:33,  wrote:
> > --- a/xen/arch/x86/efi/Makefile
> > +++ b/xen/arch/x86/efi/Makefile
> > @@ -1,14 +1,9 @@
> >  CFLAGS += -fshort-wchar
> >
> > -obj-y += stub.o
> > -
> > -create = test -e $(1) || touch -t 19990101 $(1)
> > -
> >  efi := y$(shell rm -f disabled)
> >  efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) 
> > -c check.c 2>disabled && echo y))
> >  efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi 
> > check.o 2>disabled && echo y))
> > -efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call 
> > create,boot.init.o); $(call create,runtime.o)))
> > +efi := $(if $(efi),$(shell rm disabled)y)
> >
> > -extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o
> > -
> > -stub.o: $(extra-y)
> > +obj-y := stub.o
> > +obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o
>
> I assume/hope all these adjustments work for all intended cases, but

I have done some tests and it looks that everything works.

> they quite clearly leave stale bits in xen/arch/x86/Rules.mk: Its

I suppose that you were thinking about xen/arch/x86/Makefile.

> references to efi/*.o should all go away now afaict.

OK.

> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -1244,6 +1244,9 @@ void __init efi_init_memory(void)
> >  } *extra, *extra_head = NULL;
> >  #endif
> >
> > +if ( !efi_enabled(EFI_PLATFORM) )
> > +return;
>
> Arguably such checks would then better be put at the call site,
> allowing the respective stubs to just BUG().

Ugh... I am confused. Here 
http://lists.xen.org/archives/html/xen-devel/2015-08/msg01790.html
you asked for what is done above. So, what is your final decision?

> Also - what's your rule for where to put such efi_enabled() checks?
> I would have expected them to get added to everything that has
> a counterpart in stubs.c, but things like efi_get_time() or
> efi_{halt,reset}_system() don't get any added. If those are
> unreachable, I'd at least expect respective ASSERT()s to get added
> there.

I have added checks to functions which are called from common EFI/BIOS code.

> > --- a/xen/common/efi/runtime.c
> > +++ b/xen/common/efi/runtime.c
> > @@ -167,6 +167,9 @@ int efi_get_info(uint32_t idx, union xenpf_efi_info 
> > *info)
> >  {
> >  unsigned int i, n;
> >
> > +if ( !efi_enabled(EFI_PLATFORM) )
> > +return -EOPNOTSUPP;
>
> Please do not introduce behavioral differences to the current stub
> implementations: This and ...
>
> > @@ -301,6 +304,9 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
> >  EFI_STATUS status = EFI_NOT_STARTED;
> >  int rc = 0;
> >
> > +if ( !efi_enabled(EFI_PLATFORM) )
> > +return -EOPNOTSUPP;
>
> ... this return -ENOSYS there.

OK.

Daniel

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


[Xen-devel] [xen-unstable-smoke test] 94769: regressions - FAIL

2016-05-25 Thread osstest service owner
flight 94769 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94769/

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-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

version targeted for testing:
 xen  8478c9409a2c6726208e8dbc9f3e455b76725a33
baseline version:
 xen  1260c7fdb91dfe0033d6eef0e94b93607be020a9

Last test of basis94767  2016-05-25 15:01:35 Z0 days
Testing same since94769  2016-05-25 17:01:46 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  fail
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt 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


Not pushing.


commit 8478c9409a2c6726208e8dbc9f3e455b76725a33
Author: Wei Liu 
Date:   Mon May 23 12:07:20 2016 +0100

docs: update xl manpage about {block,network}-attach command

State that only attaching PV interface is supported.

Signed-off-by: Wei Liu 
Signed-off-by: Ian Jackson 
Release-acked-by: Wei Liu 
(qemu changes not included)

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


Re: [Xen-devel] [PATCH v3 1/3] x86/hvm: Add check when register io handler

2016-05-25 Thread Suravee Suthikulanit

On 5/23/2016 6:46 AM, Jan Beulich wrote:

On 22.05.16 at 01:42,  wrote:

--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -258,6 +258,8 @@ struct hvm_io_handler *hvm_next_io_handler(struct domain *d)
 {
 unsigned int i = d->arch.hvm_domain.io_handler_count++;

+ASSERT( d->arch.hvm_domain.io_handler );


Am I wrong in understanding that without patch 2 this ASSERT() will
actually trigger? In which case the patch order would be wrong (but
that could be taken care of during commit).

Jan



Right, I can fix this in V4 if we decide to change the 
iommu_domain_initialise() ordering.


Suravee

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


Re: [Xen-devel] [PATCH v3 2/3] svm: iommu: Only call guest_iommu_init() after initialized HVM domain

2016-05-25 Thread Suravee Suthikulanit

On 5/23/2016 6:54 AM, Jan Beulich wrote:

On 22.05.16 at 01:42,  wrote:

From: Suravee Suthikulpanit 

The guest_iommu_init() is currently called by the following code path:

arch/x86/domain.c: arch_domain_create()
  ]- drivers/passthrough/iommu.c: iommu_domain_init()
|- drivers/passthrough/amd/pci_amd_iommu.c: amd_iommu_domain_init();
  |- drivers/passthrough/amd/iommu_guest.c: guest_iommu_init()

At this point, the hvm_domain_initialised() has not been called.
So register_mmio_handler() in guest_iommu_init() silently fails.
This patch moves the iommu_domain_init() to a later point after the
hvm_domain_intialise() instead.


That's one possible approach, which I continue to be not really
happy with. guest_iommu_init() really is HVM-specific, so maybe
no longer calling it from amd_iommu_domain_init() would be the
better solution (instead calling it from hvm_domain_initialise()
would then seem to be the better option). Thoughts?


Then, this goes back to the approach I proposed in the v1 of this patch 
series, where I call guest_iommu_init/destroy() in the 
svm_domain_initialise/destroy().


However, I'm still not quite clear in why the iommu_domain_init() is 
needed before hvm_domain_initialise().




In any event is the choice of ...


@@ -675,6 +675,9 @@ int arch_domain_create(struct domain *d, unsigned int 
domcr_flags,

 return 0;

+ fail_1:
+if ( has_hvm_container_domain(d) )
+hvm_domain_destroy(d);
  fail:
 d->is_dying = DOMDYING_dead;
 psr_domain_free(d);


... the new label name sub-optimal. Please pick something more
descriptive, e.g. "iommu_fail", if the current approach is to be
retained.

Jan



In case we are going with this approach, I will make this change.

Thanks,
Suravee

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


Re: [Xen-devel] [PATCH v3 3/3] AMD IOMMU: Check io_handler before registering mmio handler

2016-05-25 Thread Suravee Suthikulanit

On 5/23/2016 3:23 AM, Paul Durrant wrote:

-Original Message-
> From: suravee.suthikulpa...@amd.com
> [mailto:suravee.suthikulpa...@amd.com]
> Sent: 22 May 2016 00:43
> To: xen-devel@lists.xen.org; Paul Durrant; jbeul...@suse.com; George
> Dunlap
> Cc: Keir (Xen.org); Suravee Suthikulpanit; Suravee Suthikulapanit
> Subject: [PATCH v3 3/3] AMD IOMMU: Check io_handler before registering
> mmio handler
>
> From: Suravee Suthikulpanit 
>
> guest_iommu_init tries to register mmio handler before HVM domain
> is initialized. This cause registration to silently failing.
> This patch adds a sanitiy check and puts out error message.
>
> Signed-off-by: Suravee Suthikulapanit 

This patch is now defunct isn't it?

  Paul



It is no longer required, but I think this is a good sanity check in 
case something changes in the future and causing this to be called 
before the HVM I/O handler initialized.


Thanks,
Suravee

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


[Xen-devel] [qemu-upstream-unstable test] 94765: tolerable FAIL - PUSHED

2016-05-25 Thread osstest service owner
flight 94765 qemu-upstream-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94765/

Failures :-/ but no regressions.

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-amd64-i386-libvirt-xsm  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-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail 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-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-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-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  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-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-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass

version targeted for testing:
 qemuuc2092c20d6ad5e26375ec680c504fe584ae5d4b5
baseline version:
 qemuu62b3d206425c245ed0a020390a64640d40d97471

Last test of basis93947  2016-05-10 02:21:26 Z   15 days
Testing same since94765  2016-05-25 13:11:16 Z0 days1 attempts


People who touched revisions under test:
  Gerd Hoffmann 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 test-amd64-amd64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-xl-xsm  

Re: [Xen-devel] [PATCH v3] altp2m: Allow the hostp2m to be shared

2016-05-25 Thread George Dunlap
On Wed, May 25, 2016 at 5:58 PM, Tamas K Lengyel  wrote:
> On Wed, May 25, 2016 at 10:08 AM, George Dunlap
>  wrote:
>> On Wed, May 25, 2016 at 4:31 PM, Tamas K Lengyel  wrote:
>>>
>>> On May 25, 2016 05:27, "George Dunlap"  wrote:

 On Fri, Apr 29, 2016 at 6:42 PM, Tamas K Lengyel 
 wrote:
 > Don't propagate altp2m changes from ept_set_entry for memshare as
 > memshare
 > already has the lock. We call altp2m propagate changes once memshare
 > successfully finishes. Allow the hostp2m entries to be of type
 > p2m_ram_shared when applying mem_access. Also, do not trigger PoD for
 > hostp2m
 > when setting altp2m mem_access to be in-line with non-altp2m mem_access
 > path.

 Hey Tamas,

 Sorry for the long delay in getting back to you on this.
>>>
>>> No problem, thanks for taking a closer look!
>>>

 So the main issue here (correct me if I'm wrong) is the locking
 discipline: namely, men_sharing_share_pages():
 - Grabs the hostp2m lock
 - Grabs the appropriate domain memsharing locks
 - Calls set_shared_p2m_entry(), which ends up calling ept_set_entry(),
 which (when altp2m is active) grabs the altp2mlist and altp2m locks.

 This causes an ASSERT(), since the altp2mlist lock is ahead of the
 memsharing locks in the list.

 But having taken a closer look at the code, I'm not sure the change is
 quite correct.  Please correct me if I've misread something:

 mem_sharing_share_pages() is passed two  pairs -- the
  (which I assume stands for "shared gfn") and 
 (which I assume stands for "copy"); and it
>>>
>>> Here s/c stands for source/client.
>>>
 1) Looks up smfn and cmfn, which back sgfn and cmfn respectively
 2) Looks up cmfn, which backs cgfn then replaces all gfn entries which
 point to cmfn with smfn (updating accounting as appropriate)
>>>
>>> Hm, I might have missed that. Where does it do the lookup for all other
>>> cgfns backed by this cmfn?
>>
>> I was looking at the loop in the middle of the function:
>>
>> while ( (gfn = rmap_iterate(cpage, )) != NULL) {
>>  ...
>> }
>>
>> I haven't chased it down, but it looks like this walks the reverse map
>> of all gfns which map cpage; and for each such gfn it finds it:
>> * removes the cpage -> gfn rmap
>> * Adds an spage -> gfn map
>> * Reduces the type count of cpage
>> * Sets the p2m entry for that gfn to the smfn (rather than cmfn).
>>
>> Obviously the common case is that the number of mappings is exactly 1;
>> but we need to either ensure that this is always true, or we need to
>> handle the case where it's not true. :-)
>>
 But this change will only call p2m_altp2m_propagate_change() for the
 original cgfn -- any other gfns which are backed by cmfn will not have
 the corresponding altp2m entries propagated properly.
>>>
>>> Right, if there is some other place where it does sharing in the back we
>>> would have to propagate that change.
>>>
 This sort of mistake is easy to make, which is why I think we should
 try to always update the altp2ms in ept_set_entry() if we can, to
 minimize the opportunity for making this sort of mistake.

 Is there ever a reason to grab the altp2m lock and *then* grab the
 sharing lock?  Could we just move the sharing lock up between the p2m
 lock and the altp2mlist lock instead?

>>>
>>> I can't think of a scenario where we would get to sharing from altp2m with
>>> altp2m locking first. Not sure what you mean by moving the sharing lock up
>>> though. The problem is that sharing already has the lock by the time altp2m
>>> tries to lock, so we could pass that info down to make altp2m aware it needs
>>> no locking. It would require extending a bunch of functions though with an
>>> extra input that is barely ever used..
>>
>> If you have altp2m there are three locks.  There's one p2m lock for
>> the "host" p2m (that is, Xen's idea of what the mapping should look
>> like).  Then there's the altp2mlist lock, which protects the *list* of
>> altp2ms; then each altp2m itself has its own lock.  These are defined
>> in mm-lock.h and  must be grabbed in that order: p2m before
>> altp2mlist, altp2mlist before altp2m.
>>
>> I assume that the memsharing code is grabbing the hostp2m lock (it
>> should be anyway), then grabbing the memsharing locks. This is allowed
>> because the memsharing locks are defined after the p2m lock in
>> mm-lock.h.  But then when updating the p2m entry, if you have an
>> altp2m active, it then tries to grab the altp2mlist lock so it can
>> iterate over the altp2ms.  Since the altp2mlist lock is *before* the
>> sharing lock in mm-lock.h, this triggers an assert.
>>
>> Is that not what your issue is?
>
> Ahh, I see! Let me give that a try - TBH this locking order
> enforcement based on 

Re: [Xen-devel] [PATCH v2] AMD IOMMU: Introduce support for IVHD block type 11h

2016-05-25 Thread Suravee Suthikulanit

Thanks for these review comment. I'll update this patch and send out V3

Suravee

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


Re: [Xen-devel] [PATCH v3] xen-hvm: ignore background I/O sections

2016-05-25 Thread Paolo Bonzini


- Original Message -
> From: "Anthony PERARD" 
> To: "Paul Durrant" 
> Cc: qemu-de...@nongnu.org, xen-de...@lists.xenproject.org, "Stefano 
> Stabellini" , "Paolo
> Bonzini" 
> Sent: Wednesday, May 25, 2016 5:52:32 PM
> Subject: Re: [PATCH v3] xen-hvm: ignore background I/O sections
> 
> On Mon, May 09, 2016 at 05:31:20PM +0100, Paul Durrant wrote:
> > Since Xen will correctly handle accesses to unimplemented I/O ports (by
> > returning all 1's for reads and ignoring writes) there is no need for
> > QEMU to register backgroud I/O sections.
> > 
> > This patch therefore adds checks to xen_io_add/del so that sections with
> > memory-region ops pointing at 'unassigned_io_ops' are ignored.
> > 
> > Signed-off-by: Paul Durrant 
> > Cc: Stefano Stabellini 
> > Cc: Anthony Perard 
> > Cc: Paolo Bonzini 
> 
> Acked-by: Anthony PERARD 

Do you have a signed GPG key or do I need to apply this for you?

Thanks,

Paolo

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


[Xen-devel] [qemu-upstream-4.3-testing test] 94766: trouble: blocked/broken

2016-05-25 Thread osstest service owner
flight 94766 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94766/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-pvops  3 host-install(3) broken REGR. vs. 80927
 build-amd64   3 host-install(3) broken REGR. vs. 80927
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927
 build-i3863 host-install(3) broken REGR. vs. 80927

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-pv1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-pv   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a

version targeted for testing:
 qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  109 days
Testing same since93977  2016-05-10 11:09:16 Z   15 days   56 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Stefano Stabellini 

jobs:
 build-amd64  broken  
 build-i386   broken  
 build-amd64-libvirt  blocked 
 build-i386-libvirt   blocked 
 build-amd64-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-xl  blocked 
 test-amd64-i386-xl   blocked 
 test-amd64-i386-qemuu-rhel6hvm-amd   blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked 
 test-amd64-i386-freebsd10-amd64  blocked 
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 
 test-amd64-amd64-xl-qemuu-win7-amd64 blocked 
 test-amd64-i386-xl-qemuu-win7-amd64  blocked 
 test-amd64-amd64-xl-credit2  blocked 
 test-amd64-i386-freebsd10-i386   blocked 
 test-amd64-i386-qemuu-rhel6hvm-intel blocked 
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpublocked 
 

Re: [Xen-devel] [PATCH v3 10/16] efi: create efi_enabled()

2016-05-25 Thread Daniel Kiper
On Wed, May 25, 2016 at 01:20:23AM -0600, Jan Beulich wrote:
> >>> On 15.04.16 at 14:33,  wrote:
> > --- a/xen/arch/x86/efi/stub.c
> > +++ b/xen/arch/x86/efi/stub.c
> > @@ -4,11 +4,8 @@
> >  #include 
> >  #include 
> >
> > -#ifndef efi_enabled
> > -const bool_t efi_enabled = 0;
> > -#endif
> > -
> >  struct efi __read_mostly efi = {
> > +   .flags   = 0, /* Initialized later. */
>
> This is pointless to add - the field will get zero-initialized anyway.

Sure thing. However, I think that we should be clear here that
there is no default value for .flags (well, it is 0). Though if
you wish I can remove that.

> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -934,6 +934,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> > *SystemTable)
> >  char *option_str;
> >  bool_t use_cfg_file;
> >
> > +#ifndef CONFIG_ARM /* Disabled until runtime services implemented. */
> > +set_bit(EFI_PLATFORM, );
> > +#endif
>
> Surely this can be __set_bit()? It's also hard to see what setting this

OK.

> flag has got to do with runtime services. But more on this below.

Well, comment is not the best one here... I will fix it.

> > @@ -42,11 +38,12 @@ UINT64 __read_mostly efi_boot_remain_var_store_size;
> >  UINT64 __read_mostly efi_boot_max_var_size;
> >
> >  struct efi __read_mostly efi = {
> > -   .acpi   = EFI_INVALID_TABLE_ADDR,
> > -   .acpi20 = EFI_INVALID_TABLE_ADDR,
> > -   .mps= EFI_INVALID_TABLE_ADDR,
> > -   .smbios = EFI_INVALID_TABLE_ADDR,
> > -   .smbios3 = EFI_INVALID_TABLE_ADDR,
> > +   .flags   = 0, /* Initialized later. */
> > +   .acpi= EFI_INVALID_TABLE_ADDR,
> > +   .acpi20  = EFI_INVALID_TABLE_ADDR,
> > +   .mps = EFI_INVALID_TABLE_ADDR,
> > +   .smbios  = EFI_INVALID_TABLE_ADDR,
> > +   .smbios3 = EFI_INVALID_TABLE_ADDR
> >  };
>
> This, again, is an unnecessary hunk. And in no case should you drop

Ditto.

> the trailing comma - that's there for a reason.

What is the reason for trailing comma?

> > --- a/xen/include/xen/efi.h
> > +++ b/xen/include/xen/efi.h
> > @@ -2,15 +2,17 @@
> >  #define __XEN_EFI_H__
> >
> >  #ifndef __ASSEMBLY__
> > +#include 
> >  #include 
> >  #endif
> >
> > -extern const bool_t efi_enabled;
> > -
> >  #define EFI_INVALID_TABLE_ADDR (~0UL)
> >
> > +#define EFI_PLATFORM   0
>
> So what does "platform" mean? Did you consider using the more fine

It means "EFI platform". It differentiates from "legacy BIOS platform".

> grained set of flags Linux uses nowadays? That would also eliminate

I wish to use just basic idea. However, I am not going to copy all
stuff from Linux. We do not need that.

> the odd connection to runtime services mentioned earlier.

That is good point. I will think how to solve that in good way.

> And please add a comment making clear that these values are bit
> positions to be used in the flags field below. I might also help to
> move this right next to the structure field.

OK.

Daniel

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


Re: [Xen-devel] [PATCH v3] altp2m: Allow the hostp2m to be shared

2016-05-25 Thread Tamas K Lengyel
On Wed, May 25, 2016 at 10:08 AM, George Dunlap
 wrote:
> On Wed, May 25, 2016 at 4:31 PM, Tamas K Lengyel  wrote:
>>
>> On May 25, 2016 05:27, "George Dunlap"  wrote:
>>>
>>> On Fri, Apr 29, 2016 at 6:42 PM, Tamas K Lengyel 
>>> wrote:
>>> > Don't propagate altp2m changes from ept_set_entry for memshare as
>>> > memshare
>>> > already has the lock. We call altp2m propagate changes once memshare
>>> > successfully finishes. Allow the hostp2m entries to be of type
>>> > p2m_ram_shared when applying mem_access. Also, do not trigger PoD for
>>> > hostp2m
>>> > when setting altp2m mem_access to be in-line with non-altp2m mem_access
>>> > path.
>>>
>>> Hey Tamas,
>>>
>>> Sorry for the long delay in getting back to you on this.
>>
>> No problem, thanks for taking a closer look!
>>
>>>
>>> So the main issue here (correct me if I'm wrong) is the locking
>>> discipline: namely, men_sharing_share_pages():
>>> - Grabs the hostp2m lock
>>> - Grabs the appropriate domain memsharing locks
>>> - Calls set_shared_p2m_entry(), which ends up calling ept_set_entry(),
>>> which (when altp2m is active) grabs the altp2mlist and altp2m locks.
>>>
>>> This causes an ASSERT(), since the altp2mlist lock is ahead of the
>>> memsharing locks in the list.
>>>
>>> But having taken a closer look at the code, I'm not sure the change is
>>> quite correct.  Please correct me if I've misread something:
>>>
>>> mem_sharing_share_pages() is passed two  pairs -- the
>>>  (which I assume stands for "shared gfn") and 
>>> (which I assume stands for "copy"); and it
>>
>> Here s/c stands for source/client.
>>
>>> 1) Looks up smfn and cmfn, which back sgfn and cmfn respectively
>>> 2) Looks up cmfn, which backs cgfn then replaces all gfn entries which
>>> point to cmfn with smfn (updating accounting as appropriate)
>>
>> Hm, I might have missed that. Where does it do the lookup for all other
>> cgfns backed by this cmfn?
>
> I was looking at the loop in the middle of the function:
>
> while ( (gfn = rmap_iterate(cpage, )) != NULL) {
>  ...
> }
>
> I haven't chased it down, but it looks like this walks the reverse map
> of all gfns which map cpage; and for each such gfn it finds it:
> * removes the cpage -> gfn rmap
> * Adds an spage -> gfn map
> * Reduces the type count of cpage
> * Sets the p2m entry for that gfn to the smfn (rather than cmfn).
>
> Obviously the common case is that the number of mappings is exactly 1;
> but we need to either ensure that this is always true, or we need to
> handle the case where it's not true. :-)
>
>>> But this change will only call p2m_altp2m_propagate_change() for the
>>> original cgfn -- any other gfns which are backed by cmfn will not have
>>> the corresponding altp2m entries propagated properly.
>>
>> Right, if there is some other place where it does sharing in the back we
>> would have to propagate that change.
>>
>>> This sort of mistake is easy to make, which is why I think we should
>>> try to always update the altp2ms in ept_set_entry() if we can, to
>>> minimize the opportunity for making this sort of mistake.
>>>
>>> Is there ever a reason to grab the altp2m lock and *then* grab the
>>> sharing lock?  Could we just move the sharing lock up between the p2m
>>> lock and the altp2mlist lock instead?
>>>
>>
>> I can't think of a scenario where we would get to sharing from altp2m with
>> altp2m locking first. Not sure what you mean by moving the sharing lock up
>> though. The problem is that sharing already has the lock by the time altp2m
>> tries to lock, so we could pass that info down to make altp2m aware it needs
>> no locking. It would require extending a bunch of functions though with an
>> extra input that is barely ever used..
>
> If you have altp2m there are three locks.  There's one p2m lock for
> the "host" p2m (that is, Xen's idea of what the mapping should look
> like).  Then there's the altp2mlist lock, which protects the *list* of
> altp2ms; then each altp2m itself has its own lock.  These are defined
> in mm-lock.h and  must be grabbed in that order: p2m before
> altp2mlist, altp2mlist before altp2m.
>
> I assume that the memsharing code is grabbing the hostp2m lock (it
> should be anyway), then grabbing the memsharing locks. This is allowed
> because the memsharing locks are defined after the p2m lock in
> mm-lock.h.  But then when updating the p2m entry, if you have an
> altp2m active, it then tries to grab the altp2mlist lock so it can
> iterate over the altp2ms.  Since the altp2mlist lock is *before* the
> sharing lock in mm-lock.h, this triggers an assert.
>
> Is that not what your issue is?

Ahh, I see! Let me give that a try - TBH this locking order
enforcement based on position in mm-lock.h was not entirely clear to
me =)

Tamas

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


Re: [Xen-devel] balloon_mutex lockdep complaint at HVM domain destroy

2016-05-25 Thread David Vrabel
On 25/05/16 15:30, Ed Swierk wrote:
> The following lockdep dump occurs whenever I destroy an HVM domain, on
> Linux 4.4 Dom0 with CONFIG_XEN_BALLOON=n on recent stable Xen 4.5.

This occurs in dom0?  Or the guest that's being destroyed?

> Any clues whether this is a real potential deadlock, or how to silence
> it if not?

It's a bug but...

> ==
> [ INFO: RECLAIM_FS-safe -> RECLAIM_FS-unsafe lock order detected ]
> 4.4.11-grsec #1 Not tainted
  
...this isn't a vanilla kernel?  Can you try vanilla 4.6?

Because:

>IN-RECLAIM_FS-W at:
>[<__lock_acquire at lockdep.c:2839>] 810becc5
>[] 810c0ac9
>[] 816d1b4c
>[] 
> 8143c3d4
>[] 8143c450
>[<__mmu_notifier_invalidate_page at 
> mmu_notifier.c:183>] 8119de42
>[] 
> 811840c2
>[] 81185051
>[] 81185497
>[] 811599b7
>[] 
> 8115a489
>[] 8115af3a
>[] 8115b1bb
>[] 8115c1e4
>[] 8108eccc
>[] 816d706e

We should not be reclaiming pages from a gntdev VMA since it's special
(marked as VM_IO).

David

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


Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Boris Ostrovsky
On 05/25/2016 12:51 PM, Boris Ostrovsky wrote:
> Let me see whether which path we take.


That is "Let me see which code path we are taking in Linux"

-boris

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


[Xen-devel] Intel hosts OpenXT Summit on Xen Project based Client Virtualization, June 7-8 in Fairfax, VA, USA

2016-05-25 Thread Rich Persaud
The inaugural OpenXT Summit brings together developers and ecosystem 
participants for a 2-day conference in Fairfax, VA, USA on June 7-8, 2016.  The 
audience for this event includes kernel and application developers, hardware 
designers, system integrators and security architects.

Released as open-source software in 2014, OpenXT is a development toolkit for 
hardware-assisted security research and appliance integration. It includes 
hardened Linux VMs that can be configured as a user-facing software appliance 
for client devices, with virtualization of storage, network, input, sound, 
display and USB devices.  Hardware targets include laptops, desktops and 
workstations.  

OpenXT stands on the shoulders of the Xen Project, OpenEmbedded Linux and 
Citrix XenClient XT.  It is optimized for hardware-assisted virtualization with 
an IOMMU and a TPM.  It configures Xen network driver domains, Linux stub 
domains, Xen Security Modules, GPU passthrough, Intel TXT, SE Linux, and VPNs.  
Guest operating systems include Windows, Linux and FreeBSD.  VM storage options 
include encrypted VHD files with boot-time measurement and non-persistence.  

The 2016 OpenXT Summit will chart the evolution of OpenXT from cross-domain 
endpoint virtualization to an extensible systems innovation platform, enabling 
derivative products to make security assurances for diverse hardware, markets 
and use cases.  

There is no cost to attend, but registration is required.  For more 
information, please see the event website at http://openxt.org/summit.   For 
presentations and papers related to OpenXT, please see 
http://openxt.org/history.

Regards,
Rich___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

2016-05-25 Thread osstest service owner
flight 94756 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94756/

Failures :-/ but no regressions.

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

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-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail 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-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-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-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-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-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail 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-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-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

version targeted for testing:
 xen  4504b7d1a6594c8ad4b1ecdab15d30feca7eaa51
baseline version:
 xen  983aae0b591458c6cf2e3166c4e1170fd0404e7d

Last test of basis94740  2016-05-24 12:47:10 Z1 days
Testing same since94746  2016-05-24 21:42:24 Z0 days2 attempts


People who touched revisions under test:
  Anthony PERARD 
  Ian Jackson 
  Jan Beulich 
  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-oldkern  pass
 

Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Boris Ostrovsky
On 05/25/2016 12:09 PM, Ian Jackson wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 
> 32-bit default_ioport_* accesses"):
>> On 25.05.16 at 17:36,  wrote:
>>> AccesSize parameter is optional when invoking the Register macro. If the
>>> AccessSize parameter is
>>> not supplied then the AccessSize field will be set to zero. In this
>>> case, OSPM will assume the access
>>> size.
>>>
>>> I don't think I understand what the last sentence means. Does it imply
>>> that SW can do whatever it thinks is appropriate?
>> I think so, yes.
> I think this question can only be resolved de jure by looking at what
> previous ACPI specifications (before this AccessSize field) said.

It's been around since 3.0 (which is 2004). Prior to that --- my cursory
read of 2.0 suggests that accesses were 8-bit.

>
> But, I think: de facto, what is going on here is that ACPICA and hence
> Linux have changed their behaviour in a way that is not compatible
> with at least some existing "hardware".  Is this not arguably a
> compatibility defect Linux ?
>
> It would surely be better to make Linux do whatever it did before,
> when AccessSize is not supplied.  That will avoid breaking any other
> things (whether or not those other things are de jure broken according
> to previous specs).  It will also avoid us having to make changes our
> ACPI tables which themselves come with a risk of compatibility
> problems.

ACPICA will use 32-bit accesses for access_size=0:
https://github.com/acpica/acpica/commit/c49a751b

However, Linux appears to have some sort of workaround for FreeBSD,
which *appears* as it should be applicable to hvmloader's tables as
well. But it clearly does not as we are failing on Linux.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/acpi/acpica/hwregs.c?id=b314a172ee968d45f72dffea68ab8af38aa80ded

Let me see whether which path we take.

-boris



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


Re: [Xen-devel] [PATCH v3 09/16] efi: explicitly define efi struct in xen/arch/x86/efi/stub.c

2016-05-25 Thread Daniel Kiper
On Wed, May 25, 2016 at 01:03:42AM -0600, Jan Beulich wrote:
> >>> On 15.04.16 at 14:33,  wrote:
> > Existing solution does not allocate space for this symbol and any
> > references to acpi20, etc. does not make sense. As I saw any efi.*
> > references are protected by relevant ifs but we should not do that
> > because it makes code very fragile. If somebody does not know how
> > efi symbol is created he/she may assume that it always represent
> > valid structure and do invalid references somewhere.
>
> I do not view this as a valid reason for the change.

Why?

> > Additionally, following patch adds efi struct flags member which
> > is used during runtime to differentiate between legacy BIOS and
> > EFI platforms and multiboot2 and EFI native loader. So, efi symbol
> > have to proper representation in ELF and PE Xen image. Hence,
> > define efi struct in xen/arch/x86/efi/stub.c and remove efi
> > symbol from ld script.
>
> Only this one is, afaic. The only request here would be to replace
> "following" by e.g. "a subsequent", to make the description
> independent of whether the two patches get committed together.

OK.

> > --- a/xen/arch/x86/efi/stub.c
> > +++ b/xen/arch/x86/efi/stub.c
> > @@ -8,6 +8,14 @@
> >  const bool_t efi_enabled = 0;
> >  #endif
> >
> > +struct efi __read_mostly efi = {
> > +   .acpi= EFI_INVALID_TABLE_ADDR,
> > +   .acpi20  = EFI_INVALID_TABLE_ADDR,
> > +   .mps = EFI_INVALID_TABLE_ADDR,
> > +   .smbios  = EFI_INVALID_TABLE_ADDR,
> > +   .smbios3 = EFI_INVALID_TABLE_ADDR
> > +};
>
> I don't view duplicating this here as a good approach - you'd better
> move the existing instance elsewhere. If this was a temporary thing
> (until a later patch), it might be acceptable, but since building without
> EFI support will need to remain an option (for people using older tool
> chains), I don't expect a later patch to remove this.

Do you think about separate C file which should contain efi struct
and should be included in stub.c and runtime.c? Or anything else?

Daniel

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


[Xen-devel] [xen-unstable-smoke test] 94767: tolerable all pass - PUSHED

2016-05-25 Thread osstest service owner
flight 94767 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94767/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-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

version targeted for testing:
 xen  1260c7fdb91dfe0033d6eef0e94b93607be020a9
baseline version:
 xen  4074e4ebe9115ac4986f963a13feada3e0560460

Last test of basis94764  2016-05-25 13:00:57 Z0 days
Testing same since94767  2016-05-25 15:01:35 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 
  Julien Grall 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=1260c7fdb91dfe0033d6eef0e94b93607be020a9
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
1260c7fdb91dfe0033d6eef0e94b93607be020a9
+ branch=xen-unstable-smoke
+ revision=1260c7fdb91dfe0033d6eef0e94b93607be020a9
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.6-testing
+ '[' x1260c7fdb91dfe0033d6eef0e94b93607be020a9 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src 
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local 'options=[fetch=try]'
 getconfig GitCacheProxy
 perl -e '
use Osstest;
readglobalconfig();
print $c{"GitCacheProxy"} or die $!;
'
+++ local 

Re: [Xen-devel] [PATCH v3 08/16] x86: add multiboot2 protocol support

2016-05-25 Thread Daniel Kiper
On Tue, May 24, 2016 at 09:46:13AM -0600, Jan Beulich wrote:
> >>> On 15.04.16 at 14:33,  wrote:
> > @@ -19,6 +20,28 @@
> >  #define BOOT_PSEUDORM_CS 0x0020
> >  #define BOOT_PSEUDORM_DS 0x0028
> >
> > +#define MB2_HT(name)  (MULTIBOOT2_HEADER_TAG_##name)
> > +#define MB2_TT(name)  (MULTIBOOT2_TAG_TYPE_##name)
> > +
> > +.macro mb2ht_args arg, args:vararg
> > +.long \arg
> > +.ifnb \args
> > +mb2ht_args \args
> > +.endif
> > +.endm
> > +
> > +.macro mb2ht_init type, req, args:vararg
>
> If you already use :vararg here and above, please also use :req on
> the other macro arguments.

Why?

> > @@ -34,6 +57,42 @@ multiboot1_header_start:   /*** MULTIBOOT1 HEADER 
> > /
> >  .long   -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
> >  multiboot1_header_end:
> >
> > +/*** MULTIBOOT2 HEADER /
> > +/* Some ideas are taken from grub-2.00/grub-core/tests/boot/kernel-i386.S 
> > file. */
> > +.align  MULTIBOOT2_HEADER_ALIGN
> > +
> > +multiboot2_header_start:
> > +/* Magic number indicating a Multiboot2 header. */
> > +.long   MULTIBOOT2_HEADER_MAGIC
> > +/* Architecture: i386. */
> > +.long   MULTIBOOT2_ARCHITECTURE_I386
> > +/* Multiboot2 header length. */
> > +.long   multiboot2_header_end - multiboot2_header_start
> > +/* Multiboot2 header checksum. */
> > +.long   -(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 + 
> > \
> > +(multiboot2_header_end - multiboot2_header_start))
> > +
> > +/* Multiboot2 information request tag. */
> > +mb2ht_init MB2_HT(INFORMATION_REQUEST), MB2_HT(REQUIRED), \
> > +   MB2_TT(BASIC_MEMINFO), MB2_TT(MMAP)
> > +
> > +/* Align modules at page boundry. */
> > +mb2ht_init MB2_HT(MODULE_ALIGN), MB2_HT(REQUIRED)
> > +
> > +/* Console flags tag. */
> > +mb2ht_init MB2_HT(CONSOLE_FLAGS), MB2_HT(OPTIONAL), \
> > +   MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED
> > +
> > +/* Framebuffer tag. */
> > +mb2ht_init MB2_HT(FRAMEBUFFER), MB2_HT(OPTIONAL), \
> > +   0, /* Number of the columns - no preference. */ \
> > +   0, /* Number of the lines - no preference. */ \
> > +   0  /* Number of bits per pixel - no preference. */
> > +
> > +/* Multiboot2 header end tag. */
> > +mb2ht_init MB2_HT(END), MB2_HT(REQUIRED)
> > +multiboot2_header_end:
>
> Imo "end" labels should always preferably be .L-prefixed, to avoid
> them getting used by a consumer instead of another "proper" label
> starting whatever comes next.

Make sense, however, I am in line with multiboot1_header_end label here.
So, if we wish .L here then we should change multiboot1_header_end label
above too. Of course in separate patch.

> > @@ -82,10 +141,49 @@ __start:
> >  mov %ecx,%es
> >  mov %ecx,%ss
> >
> > -/* Check for Multiboot bootloader */
> > +/* Bootloaders may set multiboot{1,2}.mem_lower to a nonzero 
> > value. */
> > +xor %edx,%edx
> > +
> > +/* Check for Multiboot2 bootloader. */
> > +cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax
> > +je  multiboot2_proto
> > +
> > +/* Check for Multiboot bootloader. */
> >  cmp $MULTIBOOT_BOOTLOADER_MAGIC,%eax
> >  jne not_multiboot
> >
> > +/* Get mem_lower from Multiboot information. */
> > +testb   $MBI_MEMLIMITS,MB_flags(%ebx)
> > +
> > +/* Not available? BDA value will be fine. */
> > +cmovnz  MB_mem_lower(%ebx),%edx
> > +jmp trampoline_setup
> > +
> > +multiboot2_proto:
> > +/* Skip Multiboot2 information fixed part. */
> > +lea (MB2_fixed_sizeof+MULTIBOOT2_TAG_ALIGN-1)(%ebx),%ecx
> > +and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
> > +
> > +0:
> > +/* Get mem_lower from Multiboot2 information. */
> > +cmpl$MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,MB2_tag_type(%ecx)
> > +jne 1f
> > +
> > +mov MB2_mem_lower(%ecx),%edx
> > +jmp trampoline_setup
> > +
> > +1:
> > +/* Is it the end of Multiboot2 information? */
> > +cmpl$MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%ecx)
> > +je  trampoline_setup
> > +
> > +/* Go to next Multiboot2 information tag. */
> > +add MB2_tag_size(%ecx),%ecx
> > +add $(MULTIBOOT2_TAG_ALIGN-1),%ecx
> > +and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx
> > +jmp 0b
>
> I'm missing a total size check, matching what meanwhile got added
> to the C equivalent(s) of this loop. There's little point in doing it
> there if it doesn't also get done here.

OK.

> > @@ -41,7 +45,16 @@ asm (
> >  );
> >
> >  typedef unsigned int u32;
> > +typedef unsigned long long u64;
> > +
> >  #include 

Re: [Xen-devel] [PATCH v5 0/6] Kconfig debug options

2016-05-25 Thread Wei Liu
Series:

Reviewed-by: Wei Liu 

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


Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Ian Jackson
Jan Beulich writes ("Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 
32-bit default_ioport_* accesses"):
> On 25.05.16 at 17:36,  wrote:
> > AccesSize parameter is optional when invoking the Register macro. If the
> > AccessSize parameter is
> > not supplied then the AccessSize field will be set to zero. In this
> > case, OSPM will assume the access
> > size.
> > 
> > I don't think I understand what the last sentence means. Does it imply
> > that SW can do whatever it thinks is appropriate?
> 
> I think so, yes.

I think this question can only be resolved de jure by looking at what
previous ACPI specifications (before this AccessSize field) said.

But, I think: de facto, what is going on here is that ACPICA and hence
Linux have changed their behaviour in a way that is not compatible
with at least some existing "hardware".  Is this not arguably a
compatibility defect Linux ?

It would surely be better to make Linux do whatever it did before,
when AccessSize is not supplied.  That will avoid breaking any other
things (whether or not those other things are de jure broken according
to previous specs).  It will also avoid us having to make changes our
ACPI tables which themselves come with a risk of compatibility
problems.

Ian.

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


Re: [Xen-devel] [PATCH v3] altp2m: Allow the hostp2m to be shared

2016-05-25 Thread George Dunlap
On Wed, May 25, 2016 at 4:31 PM, Tamas K Lengyel  wrote:
>
> On May 25, 2016 05:27, "George Dunlap"  wrote:
>>
>> On Fri, Apr 29, 2016 at 6:42 PM, Tamas K Lengyel 
>> wrote:
>> > Don't propagate altp2m changes from ept_set_entry for memshare as
>> > memshare
>> > already has the lock. We call altp2m propagate changes once memshare
>> > successfully finishes. Allow the hostp2m entries to be of type
>> > p2m_ram_shared when applying mem_access. Also, do not trigger PoD for
>> > hostp2m
>> > when setting altp2m mem_access to be in-line with non-altp2m mem_access
>> > path.
>>
>> Hey Tamas,
>>
>> Sorry for the long delay in getting back to you on this.
>
> No problem, thanks for taking a closer look!
>
>>
>> So the main issue here (correct me if I'm wrong) is the locking
>> discipline: namely, men_sharing_share_pages():
>> - Grabs the hostp2m lock
>> - Grabs the appropriate domain memsharing locks
>> - Calls set_shared_p2m_entry(), which ends up calling ept_set_entry(),
>> which (when altp2m is active) grabs the altp2mlist and altp2m locks.
>>
>> This causes an ASSERT(), since the altp2mlist lock is ahead of the
>> memsharing locks in the list.
>>
>> But having taken a closer look at the code, I'm not sure the change is
>> quite correct.  Please correct me if I've misread something:
>>
>> mem_sharing_share_pages() is passed two  pairs -- the
>>  (which I assume stands for "shared gfn") and 
>> (which I assume stands for "copy"); and it
>
> Here s/c stands for source/client.
>
>> 1) Looks up smfn and cmfn, which back sgfn and cmfn respectively
>> 2) Looks up cmfn, which backs cgfn then replaces all gfn entries which
>> point to cmfn with smfn (updating accounting as appropriate)
>
> Hm, I might have missed that. Where does it do the lookup for all other
> cgfns backed by this cmfn?

I was looking at the loop in the middle of the function:

while ( (gfn = rmap_iterate(cpage, )) != NULL) {
 ...
}

I haven't chased it down, but it looks like this walks the reverse map
of all gfns which map cpage; and for each such gfn it finds it:
* removes the cpage -> gfn rmap
* Adds an spage -> gfn map
* Reduces the type count of cpage
* Sets the p2m entry for that gfn to the smfn (rather than cmfn).

Obviously the common case is that the number of mappings is exactly 1;
but we need to either ensure that this is always true, or we need to
handle the case where it's not true. :-)

>> But this change will only call p2m_altp2m_propagate_change() for the
>> original cgfn -- any other gfns which are backed by cmfn will not have
>> the corresponding altp2m entries propagated properly.
>
> Right, if there is some other place where it does sharing in the back we
> would have to propagate that change.
>
>> This sort of mistake is easy to make, which is why I think we should
>> try to always update the altp2ms in ept_set_entry() if we can, to
>> minimize the opportunity for making this sort of mistake.
>>
>> Is there ever a reason to grab the altp2m lock and *then* grab the
>> sharing lock?  Could we just move the sharing lock up between the p2m
>> lock and the altp2mlist lock instead?
>>
>
> I can't think of a scenario where we would get to sharing from altp2m with
> altp2m locking first. Not sure what you mean by moving the sharing lock up
> though. The problem is that sharing already has the lock by the time altp2m
> tries to lock, so we could pass that info down to make altp2m aware it needs
> no locking. It would require extending a bunch of functions though with an
> extra input that is barely ever used..

If you have altp2m there are three locks.  There's one p2m lock for
the "host" p2m (that is, Xen's idea of what the mapping should look
like).  Then there's the altp2mlist lock, which protects the *list* of
altp2ms; then each altp2m itself has its own lock.  These are defined
in mm-lock.h and  must be grabbed in that order: p2m before
altp2mlist, altp2mlist before altp2m.

I assume that the memsharing code is grabbing the hostp2m lock (it
should be anyway), then grabbing the memsharing locks. This is allowed
because the memsharing locks are defined after the p2m lock in
mm-lock.h.  But then when updating the p2m entry, if you have an
altp2m active, it then tries to grab the altp2mlist lock so it can
iterate over the altp2ms.  Since the altp2mlist lock is *before* the
sharing lock in mm-lock.h, this triggers an assert.

Is that not what your issue is?

 -George

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


Re: [Xen-devel] [PATCH] xen: Clean up includes

2016-05-25 Thread Anthony PERARD
On Tue, May 24, 2016 at 04:27:18PM +0100, Peter Maydell wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Peter Maydell 

Acked-by: Anthony PERARD 

-- 
Anthony PERARD

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


Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Jan Beulich
>>> On 25.05.16 at 17:36,  wrote:
> This is what the spec says:
> 
> AccessSize evaluates to an 8-bit integer that specifies the size of data
> values used when accessing the
> address space as follows:
> 0 - Undefined (legacy)
> 1 - Byte access
> 2 - Word access
> 3 - DWord access
> 4 - QWord access
> The 8-bit field DescriptorName . _ASZ is automatically created in order
> to refer to this portion of the
> resource descriptor. See _ASZ (page 368) for more information. For
> backwards compatibility, the
> AccesSize parameter is optional when invoking the Register macro. If the
> AccessSize parameter is
> not supplied then the AccessSize field will be set to zero. In this
> case, OSPM will assume the access
> size.
> 
> I don't think I understand what the last sentence means. Does it imply
> that SW can do whatever it thinks is appropriate?

I think so, yes.

Jan


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


Re: [Xen-devel] [PATCH] x86/compat: correct SMEP/SMAP NOPs patching

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 09:00:49AM -0600, Jan Beulich wrote:
> Correct the number of single byte NOPs we want to be replaced in case
> neither SMEP nor SMAP are available.
> 
> Also simplify the expression adding these NOPs - at that location .
> equals .Lcr4_orig, and removing that part of the expression fixes a
> bogus ".space or fill with negative value, ignored" warning by very old
> gas (which actually is what made me look at those constructs again).
> 
> Signed-off-by: Jan Beulich 
> 
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -175,7 +175,7 @@ compat_bad_hypercall:
>  ENTRY(compat_restore_all_guest)
>  ASSERT_INTERRUPTS_DISABLED
>  .Lcr4_orig:
> -.skip (.Lcr4_alt_end - .Lcr4_alt) - (. - .Lcr4_orig), 0x90
> +.skip .Lcr4_alt_end - .Lcr4_alt, 0x90
>  .Lcr4_orig_end:
>  .pushsection .altinstr_replacement, "ax"
>  .Lcr4_alt:
> @@ -200,7 +200,8 @@ ENTRY(compat_restore_all_guest)
>  jne   1b
>  .Lcr4_alt_end:
>  .section .altinstructions, "a"
> -altinstruction_entry .Lcr4_orig, .Lcr4_orig, X86_FEATURE_ALWAYS, 12, > 0
> +altinstruction_entry .Lcr4_orig, .Lcr4_orig, X86_FEATURE_ALWAYS, \
> + (.Lcr4_orig_end - .Lcr4_orig), 0
>  altinstruction_entry .Lcr4_orig, .Lcr4_alt, X86_FEATURE_SMEP, \
>   (.Lcr4_orig_end - .Lcr4_orig), \
>   (.Lcr4_alt_end - .Lcr4_alt)
> 
> 
> 

FWIW:
Reviewed-by: Wei Liu 

And subject to review from Andrew:

Release-acked-by: Wei Liu 


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


[Xen-devel] [for-4.7 v2 1/2] xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id' as reserved for dev_mmio

2016-05-25 Thread Julien Grall
The field 'foreign_id' is not used when the space is dev_mmio. As the
space is not yet part of the stable ABI, the field is marked as reserved
for future use.

The value should always be 0, other values will return -EOPNOTSUPP.

Signed-off-by: Julien Grall 

---
Changes in v2:
- Return -EOPNOTSUPP rather than -ENOSYS
- Introduce a union in the sturcutre xenmem_add_to_physmap_batch
---
 xen/arch/arm/mm.c   |  9 +++--
 xen/arch/x86/mm.c   |  4 ++--
 xen/common/compat/memory.c  |  2 ++
 xen/common/memory.c | 12 +---
 xen/include/public/memory.h | 10 +-
 xen/include/xen/mm.h|  3 ++-
 6 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index b46e23e..79f4310 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1047,7 +1047,7 @@ void share_xen_page_with_privileged_guests(
 int xenmem_add_to_physmap_one(
 struct domain *d,
 unsigned int space,
-domid_t foreign_domid,
+union add_to_physmap_batch_extra extra,
 unsigned long idx,
 xen_pfn_t gpfn)
 {
@@ -1103,7 +1103,8 @@ int xenmem_add_to_physmap_one(
 {
 struct domain *od;
 p2m_type_t p2mt;
-od = rcu_lock_domain_by_any_id(foreign_domid);
+
+od = rcu_lock_domain_by_any_id(extra.foreign_domid);
 if ( od == NULL )
 return -ESRCH;
 
@@ -1143,6 +1144,10 @@ int xenmem_add_to_physmap_one(
 break;
 }
 case XENMAPSPACE_dev_mmio:
+/* extra should be 0. Reserved for future use. */
+if ( extra.res0 )
+return -EOPNOTSUPP;
+
 rc = map_dev_mmio_region(d, gpfn, 1, idx);
 return rc;
 
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 03a4d5b..0b67103 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4769,7 +4769,7 @@ static int handle_iomem_range(unsigned long s, unsigned 
long e, void *p)
 int xenmem_add_to_physmap_one(
 struct domain *d,
 unsigned int space,
-domid_t foreign_domid,
+union add_to_physmap_batch_extra extra,
 unsigned long idx,
 xen_pfn_t gpfn)
 {
@@ -4830,7 +4830,7 @@ int xenmem_add_to_physmap_one(
 break;
 }
 case XENMAPSPACE_gmfn_foreign:
-return p2m_add_foreign(d, idx, gpfn, foreign_domid);
+return p2m_add_foreign(d, idx, gpfn, extra.foreign_domid);
 default:
 break;
 }
diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
index 19a914d..e56e187 100644
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -253,6 +253,8 @@ int compat_memory_op(unsigned int cmd, 
XEN_GUEST_HANDLE_PARAM(void) compat)
 unsigned int size = cmp.atpb.size;
 xen_ulong_t *idxs = (void *)(nat.atpb + 1);
 xen_pfn_t *gpfns = (void *)(idxs + limit);
+enum XLAT_add_to_physmap_batch_u u =
+XLAT_add_to_physmap_batch_u_res0;
 
 if ( copy_from_guest(, compat, 1) ||
  !compat_handle_okay(cmp.atpb.idxs, size) ||
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 644f81a..b1ac8b9 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -639,9 +639,15 @@ static int xenmem_add_to_physmap(struct domain *d,
 {
 unsigned int done = 0;
 long rc = 0;
+union add_to_physmap_batch_extra extra;
+
+if ( xatp->space != XENMAPSPACE_gmfn_foreign )
+extra.res0 = 0;
+else
+extra.foreign_domid = DOMID_INVALID;
 
 if ( xatp->space != XENMAPSPACE_gmfn_range )
-return xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID,
+return xenmem_add_to_physmap_one(d, xatp->space, extra,
  xatp->idx, xatp->gpfn);
 
 if ( xatp->size < start )
@@ -658,7 +664,7 @@ static int xenmem_add_to_physmap(struct domain *d,
 
 while ( xatp->size > done )
 {
-rc = xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID,
+rc = xenmem_add_to_physmap_one(d, xatp->space, extra,
xatp->idx, xatp->gpfn);
 if ( rc < 0 )
 break;
@@ -719,7 +725,7 @@ static int xenmem_add_to_physmap_batch(struct domain *d,
 }
 
 rc = xenmem_add_to_physmap_one(d, xatpb->space,
-   xatpb->foreign_domid,
+   xatpb->u,
idx, gpfn);
 
 if ( unlikely(__copy_to_guest_offset(xatpb->errs, 0, , 1)) )
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index fe52ee1..a5ab139 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -259,7 +259,15 @@ struct xen_add_to_physmap_batch {
 
 /* Number of pages to go through */
 uint16_t size;
-domid_t foreign_domid; /* IFF gmfn_foreign */
+
+#if __XEN_INTERFACE_VERSION__ < 0x00040700
+domid_t foreign_domid; /* IFF gmfn_foreign. Should 

Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Jan Beulich
>>> On 25.05.16 at 17:08,  wrote:
> On 05/25/2016 10:35 AM, Ian Jackson wrote:
>> Ian Jackson writes ("Re: [PATCH qemu-traditional] ioreq: Support 32-bit 
> default_ioport_* accesses"):
>>> Boris Ostrovsky writes ("[PATCH qemu-traditional] ioreq: Support 32-bit 
> default_ioport_* accesses"):
 Recent changes in ACPICA (specifically, Linux commit 66b1ed5aa8dd ("ACPICA:
 ACPI 2.0, Hardware: Add access_width/bit_offset support for
 acpi_hw_write()") result in guests issuing 32-bit accesses to IO space.

 QEMU needs to be able to handle them.
>>> I'm kind of missing something here.  If the specification has recently
>>> been updated to permit this, why should old hardware support it ?
>>>
>>> (I tried to find the Linux upstream git commit you're referring to but
>>> my linux.git is up to date and it seems not to be fetching within a
>>> reasonable time, so I thought I would reply now.)
>> I have looked at this commit now and I am none the wiser.
>>
>> It says just "This patch adds access_width/bit_offset support in
>> acpi_hw_write()".  I also looked at the two linked messages:
>>   https://github.com/acpica/acpica/commit/48eea5e7 
>>   https://bugs.acpica.org/show_bug.cgi?id=1240 
>> and none of this explains why this supported is needed in a
>> our deep-frozen ancient branch.
> 
> IIUIC, the Linux/ACPICA patch makes ACPICA use correct field in ACPI's
> Generic Address Structure (section 5.2.3.2 in the 6.0 spec). Before the
> patch it used register's bit_width and now it will use access_size.
> According to the spec access_size 0 means undefined/legacy access.
> 
> I just looked at what hvmloader provides and at least for FADT
> address_size is 0. And I wonder whether ACPICA uses 4-byte-access for
> these cases.
> 
> So maybe instead of trying to patch qemu-trad I should see if I can make
> hvmloader provide proper access size. Let me poke at that.

But don't forget about the risk of breaking other OSes with any
kind of change like this one.

Jan


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


Re: [Xen-devel] [PATCH v5 03/10] IOMMU/MMU: enhance the call trees of IOMMU unmapping and mapping

2016-05-25 Thread Jan Beulich
>>> On 25.05.16 at 17:34,  wrote:
> On May 23, 2016 10:19 PM, Jan Beulich  wrote:
>> >>> On 18.05.16 at 10:08,  wrote:
>> > +unsigned long type,
>> > +int preemptible)
>> >  {
>> >  unsigned long nx, x, y = page->u.inuse.type_info;
>> > -int rc = 0;
>> > +int rc = 0, ret = 0;
>> >
>> >  ASSERT(!(type & ~(PGT_type_mask | PGT_pae_xen_l2)));
>> >
>> > @@ -2578,11 +2579,11 @@ static int __get_page_type(struct page_info
>> *page, unsigned long type,
>> >  if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
>> >  {
>> >  if ( (x & PGT_type_mask) == PGT_writable_page )
>> > -iommu_unmap_page(d, mfn_to_gmfn(d, page_to_mfn(page)));
>> > +ret = iommu_unmap_page(d, mfn_to_gmfn(d,
>> > + page_to_mfn(page)));
>> >  else if ( type == PGT_writable_page )
>> > -iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)),
>> > -   page_to_mfn(page),
>> > -   IOMMUF_readable|IOMMUF_writable);
>> > +ret = iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)),
>> > + page_to_mfn(page),
>> > +
>> > + IOMMUF_readable|IOMMUF_writable);
>> >  }
>> >  }
>> >
>> > @@ -2599,6 +2600,9 @@ static int __get_page_type(struct page_info
>> *page, unsigned long type,
>> >  if ( (x & PGT_partial) && !(nx & PGT_partial) )
>> >  put_page(page);
>> >
>> > +if ( !rc )
>> > +rc = ret;
>> 
>> I know I've seen this a couple of time already, but with the special purpose 
>> of
>> "ret" I really wonder whether a more specific name wouldn't be warranted -
>> e.g. "iommu_rc" or "iommu_ret".
> 
> 
> rc is return value for this function, and no directly association with IOMMU 
> related code ( rc is only for alloc_page_type() ).
> So the rc cannot be "iommu_rc"..
> 
> ret can be "iommu_ret", but I think the pair 'rc' / 'ret' may look good.

Well, I'm not entirely opposed to keeping the names, but I continue
to think that while at the call sites the shorter name is reasonable, it
is quite a bit less clear at the point where you conditionally update rc.

Jan


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


[Xen-devel] [for-4.7 v2 2/2] xen/arm: Document the behavior of XENMAPSPACE_dev_mmio

2016-05-25 Thread Julien Grall
Currently, XENMAPSPACE_dev_mmio maps MMIO regions using one of the most
restrictive memory attribute (Device-nGnRE).

Signed-off-by: Julien Grall 

---
Changes in v2:
- s/Device_nGnRE/Device-nGnRE/ to match the spec
- Update the comment to mention the name for ARMv7.
---
 xen/include/public/memory.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index a5ab139..cfb427f 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -220,7 +220,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
 #define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. 
*/
 #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
 * XENMEM_add_to_physmap_batch only. */
-#define XENMAPSPACE_dev_mmio 5 /* device mmio region */
+#define XENMAPSPACE_dev_mmio 5 /* device mmio region
+  ARM only; the region will be mapped
+  in Stage-2 using the memory attribute
+  "Device-nGnRE" (previously named
+  "Device" on ARMv7) */
 /* ` } */
 
 /*
-- 
1.9.1


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


[Xen-devel] [for-4.7 v2 0/2] xen: XENMEM_add_to_phymap_batch: Mark 'foreign_id' as reserved for dev_mmio

2016-05-25 Thread Julien Grall
Hello all,

This series is based on the thread [1]. To allow future extension, the new
space dev_mmio should have all unused fields marked as reserved.

This series is candidate for Xen 4.7, the first patch ensure a clean ABI
for the new space which will allow future extension.

See in each patch for all the changes.

Regards,

[1] http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg02328.html

Julien Grall (2):
  xen: XENMEM_add_to_physmap_batch: Mark 'foreign_id' as reserved for
dev_mmio
  xen/arm: Document the behavior of XENMAPSPACE_dev_mmio

 xen/arch/arm/mm.c   |  9 +++--
 xen/arch/x86/mm.c   |  4 ++--
 xen/common/compat/memory.c  |  2 ++
 xen/common/memory.c | 12 +---
 xen/include/public/memory.h | 16 ++--
 xen/include/xen/mm.h|  3 ++-
 6 files changed, 36 insertions(+), 10 deletions(-)

-- 
1.9.1


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


Re: [Xen-devel] [PATCH v3] xen-hvm: ignore background I/O sections

2016-05-25 Thread Anthony PERARD
On Mon, May 09, 2016 at 05:31:20PM +0100, Paul Durrant wrote:
> Since Xen will correctly handle accesses to unimplemented I/O ports (by
> returning all 1's for reads and ignoring writes) there is no need for
> QEMU to register backgroud I/O sections.
> 
> This patch therefore adds checks to xen_io_add/del so that sections with
> memory-region ops pointing at 'unassigned_io_ops' are ignored.
> 
> Signed-off-by: Paul Durrant 
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: Paolo Bonzini 

Acked-by: Anthony PERARD 

-- 
Anthony PERARD

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


Re: [Xen-devel] Xen Security Advisory 180 (CVE-2014-3672) - Unrestricted qemu logging

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 03:51:23PM +0100, Wei Liu wrote:
> On Wed, May 25, 2016 at 03:04:40PM +0100, George Dunlap wrote:
> > On Mon, May 23, 2016 at 6:09 PM, Xen.org security team  
> > wrote:
> > > RESOLUTION
> > > ==
> > >
> > > Applying the appropriate attached patch resolves this issue.
> > >
> > > The patches adopt a simple and rather crude approach which is
> > > effective at resolving the security issue in the context of a Xen
> > > device model.  They may not be appropriate for adoption upstream or in
> > > other contexts.
> > 
> > This is indeed a rather crude approach; but for our upcoming 4.7
> > release, what's the plan?  Do we have time to generalize xenconsoled
> > to handle this sort of logging, and if so, who is going to do that
> > work?
> > 
> 
> I this it's going to be a bit intrusive at this point to change
> xenconsoled to do that. However it should be too hard to test.
> We also need people to test and review it. All in all it seems time is
> very tight.
> 

I just read the code of virtlogd and xenconsoled.

I think xenconsoled is missing at least things.

From a higher level:

1. Abstraction of rotating file.
2. Abstraction of client.
3. IPC interface to libxl -- presumably we need to create a socket.

Then we need to write code in libxl to use it. That then involves
inventing a protocol to pass the file name to xenconsoled (assuming we
still want one file per qemu).

Wei.

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


Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Boris Ostrovsky
On 05/25/2016 11:22 AM, Ian Jackson wrote:
> Boris Ostrovsky writes ("Re: [PATCH qemu-traditional] ioreq: Support 32-bit 
> default_ioport_* accesses"):
>> IIUIC, the Linux/ACPICA patch makes ACPICA use correct field in ACPI's
>> Generic Address Structure (section 5.2.3.2 in the 6.0 spec). Before the
>> patch it used register's bit_width and now it will use access_size.
>> According to the spec access_size 0 means undefined/legacy access.
> I see.  (Well, sort of.)
>
>> I just looked at what hvmloader provides and at least for FADT
>> address_size is 0. And I wonder whether ACPICA uses 4-byte-access for
>> these cases.
> If 0 is "undefined/legacy access", shouldn't it be using the
> register's bit width ?  Ie, isn't this then a bug in ACPICA ?
>
>> So maybe instead of trying to patch qemu-trad I should see if I can make
>> hvmloader provide proper access size. Let me poke at that.
> If this "access size" attribute is new, things should work without it,
> surely ?

This is what the spec says:

AccessSize evaluates to an 8-bit integer that specifies the size of data
values used when accessing the
address space as follows:
0 - Undefined (legacy)
1 - Byte access
2 - Word access
3 - DWord access
4 - QWord access
The 8-bit field DescriptorName . _ASZ is automatically created in order
to refer to this portion of the
resource descriptor. See _ASZ (page 368) for more information. For
backwards compatibility, the
AccesSize parameter is optional when invoking the Register macro. If the
AccessSize parameter is
not supplied then the AccessSize field will be set to zero. In this
case, OSPM will assume the access
size.

I don't think I understand what the last sentence means. Does it imply
that SW can do whatever it thinks is appropriate?


-boris

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


Re: [Xen-devel] [PATCH v3] altp2m: Allow the hostp2m to be shared

2016-05-25 Thread Tamas K Lengyel
On May 25, 2016 05:27, "George Dunlap"  wrote:
>
> On Fri, Apr 29, 2016 at 6:42 PM, Tamas K Lengyel 
wrote:
> > Don't propagate altp2m changes from ept_set_entry for memshare as
memshare
> > already has the lock. We call altp2m propagate changes once memshare
> > successfully finishes. Allow the hostp2m entries to be of type
> > p2m_ram_shared when applying mem_access. Also, do not trigger PoD for
hostp2m
> > when setting altp2m mem_access to be in-line with non-altp2m mem_access
path.
>
> Hey Tamas,
>
> Sorry for the long delay in getting back to you on this.

No problem, thanks for taking a closer look!

>
> So the main issue here (correct me if I'm wrong) is the locking
> discipline: namely, men_sharing_share_pages():
> - Grabs the hostp2m lock
> - Grabs the appropriate domain memsharing locks
> - Calls set_shared_p2m_entry(), which ends up calling ept_set_entry(),
> which (when altp2m is active) grabs the altp2mlist and altp2m locks.
>
> This causes an ASSERT(), since the altp2mlist lock is ahead of the
> memsharing locks in the list.
>
> But having taken a closer look at the code, I'm not sure the change is
> quite correct.  Please correct me if I've misread something:
>
> mem_sharing_share_pages() is passed two  pairs -- the
>  (which I assume stands for "shared gfn") and 
> (which I assume stands for "copy"); and it

Here s/c stands for source/client.

> 1) Looks up smfn and cmfn, which back sgfn and cmfn respectively
> 2) Looks up cmfn, which backs cgfn then replaces all gfn entries which
> point to cmfn with smfn (updating accounting as appropriate)

Hm, I might have missed that. Where does it do the lookup for all other
cgfns backed by this cmfn?

> But this change will only call p2m_altp2m_propagate_change() for the
> original cgfn -- any other gfns which are backed by cmfn will not have
> the corresponding altp2m entries propagated properly.

Right, if there is some other place where it does sharing in the back we
would have to propagate that change.

> This sort of mistake is easy to make, which is why I think we should
> try to always update the altp2ms in ept_set_entry() if we can, to
> minimize the opportunity for making this sort of mistake.
>
> Is there ever a reason to grab the altp2m lock and *then* grab the
> sharing lock?  Could we just move the sharing lock up between the p2m
> lock and the altp2mlist lock instead?
>

I can't think of a scenario where we would get to sharing from altp2m with
altp2m locking first. Not sure what you mean by moving the sharing lock up
though. The problem is that sharing already has the lock by the time altp2m
tries to lock, so we could pass that info down to make altp2m aware it
needs no locking. It would require extending a bunch of functions though
with an extra input that is barely ever used..

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


Re: [Xen-devel] [PATCH v5 03/10] IOMMU/MMU: enhance the call trees of IOMMU unmapping and mapping

2016-05-25 Thread Xu, Quan
On May 23, 2016 10:19 PM, Jan Beulich  wrote:
> >>> On 18.05.16 at 10:08,  wrote:
> 
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -2463,11 +2463,12 @@ static int __put_page_type(struct page_info
> > *page,  }
> >
> >
> > -static int __get_page_type(struct page_info *page, unsigned long type,
> > -   int preemptible)
> > +static int __must_check __get_page_type(struct page_info *page,
> 
> Such a __must_check is relatively pointless when all existing callers already
> check the return value (and surely all code inside mm.c knows it needs to), 
> and
> you don't at the same time make the non-static functions propagating its
> return value also __must_check.

I will drop this __must_check annotation.

> Hence I think best is to limit your effort to IOMMU functions for this patch 
> set.
>

Good idea.
 
> > +unsigned long type,
> > +int preemptible)
> >  {
> >  unsigned long nx, x, y = page->u.inuse.type_info;
> > -int rc = 0;
> > +int rc = 0, ret = 0;
> >
> >  ASSERT(!(type & ~(PGT_type_mask | PGT_pae_xen_l2)));
> >
> > @@ -2578,11 +2579,11 @@ static int __get_page_type(struct page_info
> *page, unsigned long type,
> >  if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
> >  {
> >  if ( (x & PGT_type_mask) == PGT_writable_page )
> > -iommu_unmap_page(d, mfn_to_gmfn(d, page_to_mfn(page)));
> > +ret = iommu_unmap_page(d, mfn_to_gmfn(d,
> > + page_to_mfn(page)));
> >  else if ( type == PGT_writable_page )
> > -iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)),
> > -   page_to_mfn(page),
> > -   IOMMUF_readable|IOMMUF_writable);
> > +ret = iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)),
> > + page_to_mfn(page),
> > +
> > + IOMMUF_readable|IOMMUF_writable);
> >  }
> >  }
> >
> > @@ -2599,6 +2600,9 @@ static int __get_page_type(struct page_info
> *page, unsigned long type,
> >  if ( (x & PGT_partial) && !(nx & PGT_partial) )
> >  put_page(page);
> >
> > +if ( !rc )
> > +rc = ret;
> 
> I know I've seen this a couple of time already, but with the special purpose 
> of
> "ret" I really wonder whether a more specific name wouldn't be warranted -
> e.g. "iommu_rc" or "iommu_ret".


rc is return value for this function, and no directly association with IOMMU 
related code ( rc is only for alloc_page_type() ).
So the rc cannot be "iommu_rc"..

ret can be "iommu_ret", but I think the pair 'rc' / 'ret' may look good.


> 
> > --- a/xen/arch/x86/mm/p2m-ept.c
> > +++ b/xen/arch/x86/mm/p2m-ept.c
> > @@ -658,7 +658,7 @@ bool_t ept_handle_misconfig(uint64_t gpa)
> >   *
> >   * Returns: 0 for success, -errno for failure
> >   */
> > -static int
> > +static int __must_check
> >  ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn,
> 
> Now adding the annotation here, without also (first) adding it to the p2m
> method which this gets used for (and which is this function's sole purpose), 
> is
> not useful at all. Please remember - we don't want this annotation because it
> looks good, but in order to make sure errors don't get wrongly ignored. That's
> why, from the beginning, I have been telling you that adding such annotations
> to other than new code means doing it top down (which you clearly don't do
> here).
> 

I will drop this __must_check annotation.

> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -171,20 +171,33 @@ void __hwdom_init iommu_hwdom_init(struct
> domain *d)
> >  {
> >  struct page_info *page;
> >  unsigned int i = 0;
> > +int rc = 0;
> > +
> >  page_list_for_each ( page, >page_list )
> >  {
> >  unsigned long mfn = page_to_mfn(page);
> >  unsigned long gfn = mfn_to_gmfn(d, mfn);
> >  unsigned int mapping = IOMMUF_readable;
> > +int ret;
> >
> >  if ( ((page->u.inuse.type_info & PGT_count_mask) == 0) ||
> >   ((page->u.inuse.type_info & PGT_type_mask)
> >== PGT_writable_page) )
> >  mapping |= IOMMUF_writable;
> > -hd->platform_ops->map_page(d, gfn, mfn, mapping);
> > +
> > +ret = hd->platform_ops->map_page(d, gfn, mfn, mapping);
> > +
> > +if ( unlikely(ret) )
> > +rc = ret;
> 
> This should be good enough, but I think it would be better if, just like
> elsewhere, you returned the first error instead of the last one.
> 

I will also apply it to this series.


(Jan, I know It is not an easy work to review these little things. I very 
appreciate it. Thank you!! )
Quan

___
Xen-devel mailing list

Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Ian Jackson
Boris Ostrovsky writes ("Re: [PATCH qemu-traditional] ioreq: Support 32-bit 
default_ioport_* accesses"):
> IIUIC, the Linux/ACPICA patch makes ACPICA use correct field in ACPI's
> Generic Address Structure (section 5.2.3.2 in the 6.0 spec). Before the
> patch it used register's bit_width and now it will use access_size.
> According to the spec access_size 0 means undefined/legacy access.

I see.  (Well, sort of.)

> I just looked at what hvmloader provides and at least for FADT
> address_size is 0. And I wonder whether ACPICA uses 4-byte-access for
> these cases.

If 0 is "undefined/legacy access", shouldn't it be using the
register's bit width ?  Ie, isn't this then a bug in ACPICA ?

> So maybe instead of trying to patch qemu-trad I should see if I can make
> hvmloader provide proper access size. Let me poke at that.

If this "access size" attribute is new, things should work without it,
surely ?

Ian.

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


Re: [Xen-devel] x86 PAT fixes for stable

2016-05-25 Thread Ed Swierk
...along with 
https://git.kernel.org/cgit/linux/kernel/git/xen/tip.git/commit/?id=702f9260
which should also go into v4.4, IMO.


On Wed, May 25, 2016 at 4:17 AM, Ed Swierk  wrote:
> On Tue, May 24, 2016 at 11:29 PM, Ingo Molnar  wrote:
>> Do they apply, build and boot cleanly in that order on top of v4.4, v4.5 and 
>> v4.6?
>> If yes then:
>>
>>   Acked-by: Ingo Molnar 
>
> I confirm that they do so on top of v4.4.
>
> Acked-by: Ed Swierk 

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


Re: [Xen-devel] ARM Xen Bug #45: Is there a solution?

2016-05-25 Thread Dirk Behme

Hi Julien,

On 24.05.2016 22:05, Julien Grall wrote:



On 24/05/2016 14:39, Dirk Behme wrote:

Hi Julien,


Hello Dirk,


On 23.05.2016 22:15, Julien Grall wrote:

Hello Dirk,


is there a solution for

arm: domain 0 disables clocks which are in fact being used
http://bugs.xenproject.org/xen/bug/45

?

On an ARM based board I have to use 'clk_ignore_unused' preventing
that
Dom0 disables the UART clock for the console UART configured with
console=hvc0.


There is no better solution than passing "clk_ignore_unused" on the
kernel command line so far.



What would be the solution for this issue? The

"propagate any clock related properties from the UART
node into the Xen hypervisor node"

mentioned in the ticket?


That is correct. Xen would copy the property "clocks" of the UART into
the hypervisor node.

DOM0 would then parse the clocks associated to this node and mark them
as used by Xen (I think CLK_IGNORE_UNUSED could do the job for us).



I've started to look into this:

I'd think in arm_uart.c in dt_uart_init() after

if ( !dev )

we know the UART node we are looking for. From this we have to read 
the clock configuration.


To be clarified: How to read the clock configuration? I couldn't find 
any convenient function dt_device_get_clock() or similar for that.


Now, we have the clocks we are looking for.

These are needed in domain_build.c in make_hypervisor_node(), then.

To be clarified: How to pass the clock configuration from arm_uart.c 
to domain_build.c (and not break the non-dt / non-ARM platforms)?


Any ideas or comments?


Best regards

Dirk


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


Re: [Xen-devel] [PATCH v5 09/10] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU suspending

2016-05-25 Thread Xu, Quan
On May 25, 2016 4:07 PM, Jan Beulich  wrote:
> >>> On 25.05.16 at 08:41,  wrote:
> > On May 24, 2016 4:22 PM, Jan Beulich  wrote:
> >> >>> On 18.05.16 at 10:08,  wrote:
> >> >  static int device_power_down(void)  {
> >> > -console_suspend();
> >> > +if ( console_suspend() )
> >> > +return SAVED_CONSOLE;
> >>
> >> I said so on the previous round, and I need to repeat it now: If
> >> console_suspend() fails, you saved _nothing_.
> >>
> >
> > Ah, we may have some different views for SAVED_*, which I mean has
> > been saved and we are no need to resume.
> >
> > e.g. if console_suspend() fails, I did return SAVED_CONSOLE, reading
> > my patch again, and I really resume nothing at all.
> >
> > device_power_up()
> > {
> >  ...
> > +case SAVED_CONSOLE:
> > +break;
> > ...
> > }
> >
> >
> > I know we can also propagate SAVED_NONE for console_suspend() failure,
> > then we need adjust device_power_up() relevantly.
> 
> My main point is that the names of these enumerators should reflect their
> purpose. If one reads "SAVED_CONSOLE", then (s)he should be allowed this
> to mean that console state was saved (and hence needs to be restored upon
> error / resume).
> 


I'll follow this.. it is much better than what I understand.


> >> > -time_suspend();
> >> > +if ( time_suspend() )
> >> > +return SAVED_TIME;
> >> >
> >> > -i8259A_suspend();
> >> > +if ( i8259A_suspend() )
> >> > +return SAVED_I8259A;
> >> >
> >> > +/* ioapic_suspend cannot fail */
> >> >  ioapic_suspend();
> >> >
> >> > -iommu_suspend();
> >> > +if ( iommu_suspend() )
> >> > +return SAVED_IOMMU;
> >> >
> >> > -lapic_suspend();
> >> > +if ( lapic_suspend() )
> >> > +return SAVED_LAPIC;
> >> >
> >> >  return 0;
> >>
> >> And this silently means SAVED_NONE, whereas here you saved everything.
> >> Yielding clearly bogus code ...
> >>
> >
> >
> >  '0' is just on success here.  Look at the condition where we call
> > device_power_up():
> >
> > +if ( error > 0 )
> > +device_power_up(error);
> >
> > Then, it is not bogus code.
> 
> See above: Zero should not mean both "nothing saved" and "saved
> everything".
> 
> >> Also, having come here - did I miss iommu_flush_iotlb_global()
> >> gaining a __must_check annotation somewhere?
> >
> > I will add __must_check annotation to iommu_flush_iotlb_global().
> >
> >> And the struct iommu_flush pointers
> >> and handlers? And, by analogy, iommu_flush_context_*()?
> >
> > I am better only add __must_check annotation to flush->iotlb and
> > handlers, but leaving flush->context/handers and
> > iommu_flush_context_*() as are in current patch set..
> > the coming patch set will fix them.
> 
> I don't follow the logic behind this: The purpose of this series is to make 
> sure
> flushing errors get properly bubbled up, which includes adding __must_check
> annotations. I'm not saying this needs to happen in this patch, but it should
> happen in this series

I will add a patch..

> (and please following the same basic model: A caller or a
> __must_check function should either already be __must_check, or should
> become so at the same time).
> 

Agreed.

Quan


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


Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Boris Ostrovsky
On 05/25/2016 10:35 AM, Ian Jackson wrote:
> Ian Jackson writes ("Re: [PATCH qemu-traditional] ioreq: Support 32-bit 
> default_ioport_* accesses"):
>> Boris Ostrovsky writes ("[PATCH qemu-traditional] ioreq: Support 32-bit 
>> default_ioport_* accesses"):
>>> Recent changes in ACPICA (specifically, Linux commit 66b1ed5aa8dd ("ACPICA:
>>> ACPI 2.0, Hardware: Add access_width/bit_offset support for
>>> acpi_hw_write()") result in guests issuing 32-bit accesses to IO space.
>>>
>>> QEMU needs to be able to handle them.
>> I'm kind of missing something here.  If the specification has recently
>> been updated to permit this, why should old hardware support it ?
>>
>> (I tried to find the Linux upstream git commit you're referring to but
>> my linux.git is up to date and it seems not to be fetching within a
>> reasonable time, so I thought I would reply now.)
> I have looked at this commit now and I am none the wiser.
>
> It says just "This patch adds access_width/bit_offset support in
> acpi_hw_write()".  I also looked at the two linked messages:
>   https://github.com/acpica/acpica/commit/48eea5e7
>   https://bugs.acpica.org/show_bug.cgi?id=1240
> and none of this explains why this supported is needed in a
> our deep-frozen ancient branch.

IIUIC, the Linux/ACPICA patch makes ACPICA use correct field in ACPI's
Generic Address Structure (section 5.2.3.2 in the 6.0 spec). Before the
patch it used register's bit_width and now it will use access_size.
According to the spec access_size 0 means undefined/legacy access.

I just looked at what hvmloader provides and at least for FADT
address_size is 0. And I wonder whether ACPICA uses 4-byte-access for
these cases.

So maybe instead of trying to patch qemu-trad I should see if I can make
hvmloader provide proper access size. Let me poke at that.

-boris


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


[Xen-devel] [PATCH] x86/compat: correct SMEP/SMAP NOPs patching

2016-05-25 Thread Jan Beulich
Correct the number of single byte NOPs we want to be replaced in case
neither SMEP nor SMAP are available.

Also simplify the expression adding these NOPs - at that location .
equals .Lcr4_orig, and removing that part of the expression fixes a
bogus ".space or fill with negative value, ignored" warning by very old
gas (which actually is what made me look at those constructs again).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -175,7 +175,7 @@ compat_bad_hypercall:
 ENTRY(compat_restore_all_guest)
 ASSERT_INTERRUPTS_DISABLED
 .Lcr4_orig:
-.skip (.Lcr4_alt_end - .Lcr4_alt) - (. - .Lcr4_orig), 0x90
+.skip .Lcr4_alt_end - .Lcr4_alt, 0x90
 .Lcr4_orig_end:
 .pushsection .altinstr_replacement, "ax"
 .Lcr4_alt:
@@ -200,7 +200,8 @@ ENTRY(compat_restore_all_guest)
 jne   1b
 .Lcr4_alt_end:
 .section .altinstructions, "a"
-altinstruction_entry .Lcr4_orig, .Lcr4_orig, X86_FEATURE_ALWAYS, 12, 0
+altinstruction_entry .Lcr4_orig, .Lcr4_orig, X86_FEATURE_ALWAYS, \
+ (.Lcr4_orig_end - .Lcr4_orig), 0
 altinstruction_entry .Lcr4_orig, .Lcr4_alt, X86_FEATURE_SMEP, \
  (.Lcr4_orig_end - .Lcr4_orig), \
  (.Lcr4_alt_end - .Lcr4_alt)



x86/compat: correct SMEP/SMAP NOPs patching

Correct the number of single byte NOPs we want to be replaced in case
neither SMEP nor SMAP are available.

Also simplify the expression adding these NOPs - at that location .
equals .Lcr4_orig, and removing that part of the expression fixes a
bogus ".space or fill with negative value, ignored" warning by very old
gas (which actually is what made me look at those constructs again).

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -175,7 +175,7 @@ compat_bad_hypercall:
 ENTRY(compat_restore_all_guest)
 ASSERT_INTERRUPTS_DISABLED
 .Lcr4_orig:
-.skip (.Lcr4_alt_end - .Lcr4_alt) - (. - .Lcr4_orig), 0x90
+.skip .Lcr4_alt_end - .Lcr4_alt, 0x90
 .Lcr4_orig_end:
 .pushsection .altinstr_replacement, "ax"
 .Lcr4_alt:
@@ -200,7 +200,8 @@ ENTRY(compat_restore_all_guest)
 jne   1b
 .Lcr4_alt_end:
 .section .altinstructions, "a"
-altinstruction_entry .Lcr4_orig, .Lcr4_orig, X86_FEATURE_ALWAYS, 12, 0
+altinstruction_entry .Lcr4_orig, .Lcr4_orig, X86_FEATURE_ALWAYS, \
+ (.Lcr4_orig_end - .Lcr4_orig), 0
 altinstruction_entry .Lcr4_orig, .Lcr4_alt, X86_FEATURE_SMEP, \
  (.Lcr4_orig_end - .Lcr4_orig), \
  (.Lcr4_alt_end - .Lcr4_alt)
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [for-4.7 2/2] xen/arm: Document the behavior of XENMAPSPACE_dev_mmio

2016-05-25 Thread Jan Beulich
>>> On 25.05.16 at 16:43,  wrote:
> I could say:
> 
> "ARM only; the region will be mapped in Stage-2 using the memory 
> attribute 'Device-nGnRE' (previously named 'Device' on ARMv7)".

SGTM

Jan


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


Re: [Xen-devel] Xen Security Advisory 180 (CVE-2014-3672) - Unrestricted qemu logging

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 03:04:40PM +0100, George Dunlap wrote:
> On Mon, May 23, 2016 at 6:09 PM, Xen.org security team  
> wrote:
> > RESOLUTION
> > ==
> >
> > Applying the appropriate attached patch resolves this issue.
> >
> > The patches adopt a simple and rather crude approach which is
> > effective at resolving the security issue in the context of a Xen
> > device model.  They may not be appropriate for adoption upstream or in
> > other contexts.
> 
> This is indeed a rather crude approach; but for our upcoming 4.7
> release, what's the plan?  Do we have time to generalize xenconsoled
> to handle this sort of logging, and if so, who is going to do that
> work?
> 

I this it's going to be a bit intrusive at this point to change
xenconsoled to do that. However it should be too hard to test.
We also need people to test and review it. All in all it seems time is
very tight.

We can at least apply this patch to our own tree.

Wei.

>  -George
> 
> ___
> 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


[Xen-devel] [xen-unstable-smoke test] 94764: tolerable all pass - PUSHED

2016-05-25 Thread osstest service owner
flight 94764 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94764/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-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

version targeted for testing:
 xen  4074e4ebe9115ac4986f963a13feada3e0560460
baseline version:
 xen  4504b7d1a6594c8ad4b1ecdab15d30feca7eaa51

Last test of basis94744  2016-05-24 17:01:59 Z0 days
Testing same since94764  2016-05-25 13:00:57 Z0 days1 attempts


People who touched revisions under test:
  Dario Faggioli 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=4074e4ebe9115ac4986f963a13feada3e0560460
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
4074e4ebe9115ac4986f963a13feada3e0560460
+ branch=xen-unstable-smoke
+ revision=4074e4ebe9115ac4986f963a13feada3e0560460
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.6-testing
+ '[' x4074e4ebe9115ac4986f963a13feada3e0560460 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src 
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local 'options=[fetch=try]'
 getconfig GitCacheProxy
 perl -e '
use Osstest;
readglobalconfig();
print $c{"GitCacheProxy"} or die $!;
'
+++ local cache=git://cache:9419/
+++ '[' xgit://cache:9419/ '!=' x ']'
+++ echo 

Re: [Xen-devel] [for-4.7 2/2] xen/arm: Document the behavior of XENMAPSPACE_dev_mmio

2016-05-25 Thread Julien Grall

Hi Jan,

On 25/05/16 14:14, Jan Beulich wrote:

On 25.05.16 at 13:41,  wrote:

--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -220,7 +220,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
  #define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. 
*/
  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
  * XENMEM_add_to_physmap_batch only. */
-#define XENMAPSPACE_dev_mmio 5 /* device mmio region */
+#define XENMAPSPACE_dev_mmio 5 /* device mmio region
+  On ARM, the region will be mapped
+  in stage-2 using the memory attribute
+  Device_nGnRE. */


Since this is unimplemented on x86, may I suggest to make this "ARM
only; the region will be ..."?


Sounds good.



Also - is Device_nGnRE a term uniform between ARM32 and ARM64?


Actually it should be Device-nGnRE to match with the spec. This has been 
introduced on Armv8. Previously for Armv7, the name was "Device", 
although the behavior is exactly the same.


I could say:

"ARM only; the region will be mapped in Stage-2 using the memory 
attribute 'Device-nGnRE' (previously named 'Device' on ARMv7)".


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Ian Jackson
Ian Jackson writes ("Re: [PATCH qemu-traditional] ioreq: Support 32-bit 
default_ioport_* accesses"):
> Boris Ostrovsky writes ("[PATCH qemu-traditional] ioreq: Support 32-bit 
> default_ioport_* accesses"):
> > Recent changes in ACPICA (specifically, Linux commit 66b1ed5aa8dd ("ACPICA:
> > ACPI 2.0, Hardware: Add access_width/bit_offset support for
> > acpi_hw_write()") result in guests issuing 32-bit accesses to IO space.
> > 
> > QEMU needs to be able to handle them.
> 
> I'm kind of missing something here.  If the specification has recently
> been updated to permit this, why should old hardware support it ?
> 
> (I tried to find the Linux upstream git commit you're referring to but
> my linux.git is up to date and it seems not to be fetching within a
> reasonable time, so I thought I would reply now.)

I have looked at this commit now and I am none the wiser.

It says just "This patch adds access_width/bit_offset support in
acpi_hw_write()".  I also looked at the two linked messages:
  https://github.com/acpica/acpica/commit/48eea5e7
  https://bugs.acpica.org/show_bug.cgi?id=1240
and none of this explains why this supported is needed in a
our deep-frozen ancient branch.

Thanks,
Ian.

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


Re: [Xen-devel] [PATCH for-4.7] docs: update xl manpage about {block, network}-attach command

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 03:31:19PM +0100, Wei Liu wrote:
> On Wed, May 25, 2016 at 03:28:50PM +0100, Ian Jackson wrote:
> > Wei Liu writes ("Re: [PATCH for-4.7] docs: update xl manpage about 
> > {block,network}-attach command"):
> > > On Wed, May 25, 2016 at 03:15:23PM +0100, Ian Jackson wrote:
> > ...
> > > > But the vdev field specifies more than just the device type.  So I
> > > > think this is wrong.
> > > 
> > > I guess to make things simpler I can just delete the "" part?
> > > 
> > > "Note that only attaching PV block device is supported"
> > 
> > How about
> > 
> >   Note that only PV block devices are supported by block-attach.
> >   Requests to attach emulated devices (eg, vdev=hdc) will result in
> >   only the PV view being available to the guest.
> > 
> 
> LGTM. I will update the patch.
> 

This is the updated patch.

I will commit it soon.

---8<--
From 1bca93e2326e5a644f00de80a72bcab181587489 Mon Sep 17 00:00:00 2001
From: Wei Liu 
Date: Mon, 23 May 2016 12:07:20 +0100
Subject: [PATCH for-4.7 v2] docs: update xl manpage about 
{block,network}-attach command

State that only attaching PV interface is supported.

Signed-off-by: Wei Liu 
Signed-off-by: Ian Jackson 
---
Use Ian's wording about block-attach and add his S-o-B.
---
 docs/man/xl.pod.1 | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 9887f1b..f4dc32c 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1286,6 +1286,10 @@ effect to the guest OS is much the same as any hotplug 
event.
 Create a new virtual block device.  This will trigger a hotplug event
 for the guest.
 
+Note that only PV block devices are supported by block-attach.
+Requests to attach emulated devices (eg, vdev=hdc) will result in only
+the PV view being available to the guest.
+
 B
 
 =over 4
@@ -1369,6 +1373,8 @@ B string in the domain config file. See L and
 L
 for more informations.
 
+Note that only attaching PV network interface is supported.
+
 =item B I I
 
 Removes the network device from the domain specified by I.
-- 
2.1.4


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


[Xen-devel] balloon_mutex lockdep complaint at HVM domain destroy

2016-05-25 Thread Ed Swierk
The following lockdep dump occurs whenever I destroy an HVM domain, on
Linux 4.4 Dom0 with CONFIG_XEN_BALLOON=n on recent stable Xen 4.5.

Any clues whether this is a real potential deadlock, or how to silence
it if not?

==
[ INFO: RECLAIM_FS-safe -> RECLAIM_FS-unsafe lock order detected ]
4.4.11-grsec #1 Not tainted
--
qemu-system-i38/3338 [HC0[0]:SC0[0]:HE1:SE1] is trying to acquire:
 (balloon_mutex){+.+.+.}, at: [] 
81430ac3

and this task is already holding:
 (>lock){+.+.-.}, at: [] 8143c77f
which would create a new lock dependency:
 (>lock){+.+.-.} -> (balloon_mutex){+.+.+.}

but this new dependency connects a RECLAIM_FS-irq-safe lock:
 (>lock){+.+.-.}
... which became RECLAIM_FS-irq-safe at:
  [<__lock_acquire at lockdep.c:2839>] 810becc5
  [] 810c0ac9
  [] 816d1b4c
  [] 8143c3d4
  [] 8143c450
  [<__mmu_notifier_invalidate_page at mmu_notifier.c:183>] 8119de42
  [] 811840c2
  [] 81185051
  [] 81185497
  [] 811599b7
  [] 8115a489
  [] 8115af3a
  [] 8115b1bb
  [] 8115c1e4
  [] 8108eccc
  [] 816d706e

to a RECLAIM_FS-irq-unsafe lock:
 (balloon_mutex){+.+.+.}
... which became RECLAIM_FS-irq-unsafe at:
...  [] 810bdd69
  [] 810c12f9
  [<__alloc_pages_nodemask at page_alloc.c:3248>] 8114e0d1
  [] 81199b36
  [] 8143030e
  [] 81430c94
  [] 8142f362
  [] 8143d208
  [] 811da630
  [] 811daa64
  [] 816d6cba

other info that might help us debug this:

 Possible interrupt unsafe locking scenario:

   CPU0CPU1
   
  lock(balloon_mutex);
   local_irq_disable();
   lock(>lock);
   lock(balloon_mutex);
  
lock(>lock);

 *** DEADLOCK ***

1 lock held by qemu-system-i38/3338:
 #0:  (>lock){+.+.-.}, at: [] 
8143c77f

the dependencies between RECLAIM_FS-irq-safe lock and the holding lock:
-> (>lock){+.+.-.} ops: 8996 {
   HARDIRQ-ON-W at:
[<__lock_acquire at lockdep.c:2818>] 810bec71
[] 810c0ac9
[] 816d1b4c
[] 8143c3d4
[<__mmu_notifier_invalidate_range_start at 
mmu_notifier.c:197>] 8119d72a
[] 
81172051
[] 81173e55
[] 81175f26
[<__do_page_fault at fault.c:1491>] 8105676f
[] 81056a49
[] 816d87e8
[] 811c4074
[] 
816d6cba
   SOFTIRQ-ON-W at:
[<__lock_acquire at lockdep.c:2822>] 810bec9e
[] 810c0ac9
[] 816d1b4c
[] 8143c3d4
[<__mmu_notifier_invalidate_range_start at 
mmu_notifier.c:197>] 8119d72a
[] 
81172051
[] 81173e55
[] 81175f26
[<__do_page_fault at fault.c:1491>] 8105676f
[] 81056a49
[] 816d87e8
[] 811c4074
[] 
816d6cba
   IN-RECLAIM_FS-W at:
   [<__lock_acquire at lockdep.c:2839>] 810becc5
   [] 810c0ac9
   [] 816d1b4c
   [] 8143c3d4
   [] 8143c450
   [<__mmu_notifier_invalidate_page at mmu_notifier.c:183>] 
8119de42
   [] 
811840c2
   [] 81185051
   [] 81185497
   [] 811599b7
   [] 
8115a489
   [] 8115af3a
   [] 8115b1bb
   [] 8115c1e4
   [] 8108eccc
   [] 816d706e
   INITIAL USE at:
   [<__lock_acquire at lockdep.c:3171>] 810be85c
   [] 810c0ac9
   [] 816d1b4c
   [] 8143c3d4
   [<__mmu_notifier_invalidate_range_start at 
mmu_notifier.c:197>] 8119d72a
   [] 
81172051
   [] 81173e55
   [] 81175f26
   [<__do_page_fault at fault.c:1491>] 8105676f
   [] 81056a49
   [] 816d87e8
   [] 811c4074

Re: [Xen-devel] [PATCH qemu-traditional] ioreq: Support 32-bit default_ioport_* accesses

2016-05-25 Thread Ian Jackson
Boris Ostrovsky writes ("[PATCH qemu-traditional] ioreq: Support 32-bit 
default_ioport_* accesses"):
> Recent changes in ACPICA (specifically, Linux commit 66b1ed5aa8dd ("ACPICA:
> ACPI 2.0, Hardware: Add access_width/bit_offset support for
> acpi_hw_write()") result in guests issuing 32-bit accesses to IO space.
> 
> QEMU needs to be able to handle them.

I'm kind of missing something here.  If the specification has recently
been updated to permit this, why should old hardware support it ?

(I tried to find the Linux upstream git commit you're referring to but
my linux.git is up to date and it seems not to be fetching within a
reasonable time, so I thought I would reply now.)

Ian.

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


Re: [Xen-devel] [PATCH for-4.7] docs: update xl manpage about {block, network}-attach command

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 03:28:50PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH for-4.7] docs: update xl manpage about 
> {block,network}-attach command"):
> > On Wed, May 25, 2016 at 03:15:23PM +0100, Ian Jackson wrote:
> ...
> > > But the vdev field specifies more than just the device type.  So I
> > > think this is wrong.
> > 
> > I guess to make things simpler I can just delete the "" part?
> > 
> > "Note that only attaching PV block device is supported"
> 
> How about
> 
>   Note that only PV block devices are supported by block-attach.
>   Requests to attach emulated devices (eg, vdev=hdc) will result in
>   only the PV view being available to the guest.
> 

LGTM. I will update the patch.

Wei.

> Ian.

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


Re: [Xen-devel] [PATCH for-4.7] docs: update xl manpage about {block, network}-attach command

2016-05-25 Thread Ian Jackson
Wei Liu writes ("Re: [PATCH for-4.7] docs: update xl manpage about 
{block,network}-attach command"):
> On Wed, May 25, 2016 at 03:15:23PM +0100, Ian Jackson wrote:
...
> > But the vdev field specifies more than just the device type.  So I
> > think this is wrong.
> 
> I guess to make things simpler I can just delete the "" part?
> 
> "Note that only attaching PV block device is supported"

How about

  Note that only PV block devices are supported by block-attach.
  Requests to attach emulated devices (eg, vdev=hdc) will result in
  only the PV view being available to the guest.

Ian.

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


Re: [Xen-devel] [for-4.7 1/2] xen: XENMEM_add_physmap_batch: Mark 'foreign_id' as reserved for dev_mmio

2016-05-25 Thread Julien Grall

Hi Jan,

On 25/05/16 14:12, Jan Beulich wrote:

On 25.05.16 at 13:41,  wrote:

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1143,6 +1143,10 @@ int xenmem_add_to_physmap_one(
  break;
  }
  case XENMAPSPACE_dev_mmio:
+/* The field 'foreign_domid' is reserved for future use */
+if ( foreign_domid )
+return -ENOSYS;


This should return -EINVAL or maybe -EOPNOTSUPP, but
definitely not -ENOSYS.


I will use -EOPNOTSUPP.




--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -639,9 +639,11 @@ static int xenmem_add_to_physmap(struct domain *d,
  {
  unsigned int done = 0;
  long rc = 0;
+/* The field 'foreign_id' should be 0 when mapping MMIO. */
+domid_t inv = (xatp->space != XENMAPSPACE_dev_mmio) ? DOMID_INVALID : 0;


This is a bad type for something that now isn't a domain ID anymore.
Please use u16 or even better unsigned int. Eventually we should
fix xenmem_add_to_physmap_one()'s respective parameter type
accordingly.

Also I think the condition would better be space == gmfn_foreign.


Ok.




@@ -658,7 +660,7 @@ static int xenmem_add_to_physmap(struct domain *d,

  while ( xatp->size > done )
  {
-rc = xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID,
+rc = xenmem_add_to_physmap_one(d, xatp->space, inv,
 xatp->idx, xatp->gpfn);


This instance you could actually leave alone (as it's dealing with
XENMAPSPACE_gmfn_range only).


--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -259,7 +259,7 @@ struct xen_add_to_physmap_batch {

  /* Number of pages to go through */
  uint16_t size;
-domid_t foreign_domid; /* IFF gmfn_foreign */
+domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other spaces. 
*/


I wonder whether we shouldn't fix up the structure here right away,
instead of deferring that to after 4.7. After all, as above, we don't
really want a domain ID here generally anymore, so this should
either become "u16 aux" (or some such) or a union (all of course only
for new enough __XEN_INTERFACE_VERSION__).

Plus I think we will want this to be IN/OUT, such that if the
implementation, rather than failing, uses a replacement attribute,
that could be communicated back. Of course that would matter
only if we don't go the union route mentioned above.


I will give a look to use an union.

Regards,
--
Julien Grall

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


Re: [Xen-devel] [RFC for-4.8 0/6] xen/arm: Add support for mapping mmio-sram nodes into dom0

2016-05-25 Thread Julien Grall

Hi Edgar,

On 25/05/16 14:29, Edgar E. Iglesias wrote:

On Tue, May 24, 2016 at 08:44:41PM +0100, Julien Grall wrote:
Looking a little closer, the place where the generic list of matches and
attributes doesn't work well is when trying to deal with the no-memory-wc
property available only in mmio-sram nodes.

We'd really need an mmio-sram specific check in that case. Either
explicitely open coded in domain_build.c or something along the lines
f the .map method. Or did you have other ideas in mind?


How about extending the function dt_match_node and the structure 
dt_device_match to check the existence (or not) of a property?


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH for-4.7] docs: update xl manpage about {block, network}-attach command

2016-05-25 Thread Ian Jackson
Wei Liu writes ("[PATCH for-4.7] docs: update xl manpage about 
{block,network}-attach command"):
> State that only attaching PV interface is supported.
...
>  Create a new virtual block device.  This will trigger a hotplug event
> -for the guest.
> +for the guest. Note that only attaching PV block device is supported,
> +the B field is ignored.

But the vdev field specifies more than just the device type.  So I
think this is wrong.

Ian.

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


Re: [Xen-devel] [PATCH for-4.7] docs: update xl manpage about {block, network}-attach command

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 03:15:23PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH for-4.7] docs: update xl manpage about 
> {block,network}-attach command"):
> > State that only attaching PV interface is supported.
> ...
> >  Create a new virtual block device.  This will trigger a hotplug event
> > -for the guest.
> > +for the guest. Note that only attaching PV block device is supported,
> > +the B field is ignored.
> 
> But the vdev field specifies more than just the device type.  So I
> think this is wrong.

I guess to make things simpler I can just delete the "" part?

"Note that only attaching PV block device is supported"

> 
> Ian.

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


Re: [Xen-devel] [PATCH 4/4] xen/arm: arm64: Remove MPIDR multiprocessing extensions check

2016-05-25 Thread Julien Grall

Hi Wei,

On 25/05/16 03:10, Wei Chen wrote:

In ARM64, the MPIDR multiprocessing extensions bit is reserved to 1.


Well, technically the bit is unamed for ARM64. So I would make clear 
that the name is from AArch32. Something along:


"The bit 31 (former bit Multiprocessing extensions bit in AArch32) is 
always RES1 for AArch64."



So, the value check for this bit is no longer necessary on ARM64.

Signed-off-by: Wei Chen 
---
  xen/arch/arm/arm64/head.S | 1 -
  1 file changed, 1 deletion(-)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 3090beb..91e2817 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -267,7 +267,6 @@ common_start:
* find that multiprocessor extensions 
are
* present and the system is SMP  */
  mrs   x0, mpidr_el1
-tbz   x0, _MPIDR_SMP, 1f /* Multiprocessor extension not 
supported? */
  tbnz  x0, _MPIDR_UP, 1f  /* Uniprocessor system? */

  ldr   x13, =(~MPIDR_HWID_MASK)



Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH 3/4] xen:arm: arm64: Add correct MPIDR_HWID_MASK value for ARM64

2016-05-25 Thread Julien Grall

Hi Wei,

Title: "Fix the MPIDR_HWID_MASK value for ARM64".

On 25/05/16 03:09, Wei Chen wrote:

Current MPIDR_HWID_MASK is using the bit definition of ARM32 MPIDR.


s/Current/Currently/


This value is not correct while Xen is running on ARM64.


I think s/while/when/


Now, we add a correct value for this marco on ARM64. But this value


s/marco/macro/


is not a valid 64-bit immediate which can be encoded in mov instruction.
So we have to use ldr to load this value to register.


You need to explain what is the valid value, i.e there is 4 level of 
affinity whilst AArch32 has only 3. It would be good to mention the spec 
too.




Signed-off-by: Wei Chen 
---
  xen/arch/arm/arm64/head.S   | 2 +-
  xen/include/asm-arm/processor.h | 4 
  2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index d5831f2..3090beb 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -270,7 +270,7 @@ common_start:
  tbz   x0, _MPIDR_SMP, 1f /* Multiprocessor extension not 
supported? */
  tbnz  x0, _MPIDR_UP, 1f  /* Uniprocessor system? */

-mov   x13, #(~MPIDR_HWID_MASK)
+ldr   x13, =(~MPIDR_HWID_MASK)
  bic   x24, x0, x13   /* Mask out flags to get CPU ID */
  1:

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index b4cce7e..284ad6a 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -18,7 +18,11 @@
  #define MPIDR_SMP   (_AC(1,U) << _MPIDR_SMP)
  #define MPIDR_AFF0_SHIFT(0)
  #define MPIDR_AFF0_MASK (_AC(0xff,U) << MPIDR_AFF0_SHIFT)
+#ifdef CONFIG_ARM_64
+#define MPIDR_HWID_MASK _AC(0xff00ff,UL)
+#else
  #define MPIDR_HWID_MASK _AC(0xff,U)
+#endif
  #define MPIDR_INVALID   (~MPIDR_HWID_MASK)
  #define MPIDR_LEVEL_BITS(8)




Regards,

--
Julien Grall

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


Re: [Xen-devel] [for-4.7 1/2] xen: XENMEM_add_physmap_batch: Mark 'foreign_id' as reserved for dev_mmio

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 07:12:30AM -0600, Jan Beulich wrote:
> >>> On 25.05.16 at 13:41,  wrote:
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -1143,6 +1143,10 @@ int xenmem_add_to_physmap_one(
> >  break;
> >  }
> >  case XENMAPSPACE_dev_mmio:
> > +/* The field 'foreign_domid' is reserved for future use */
> > +if ( foreign_domid )
> > +return -ENOSYS;
> 
> This should return -EINVAL or maybe -EOPNOTSUPP, but
> definitely not -ENOSYS.
> 
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -639,9 +639,11 @@ static int xenmem_add_to_physmap(struct domain *d,
> >  {
> >  unsigned int done = 0;
> >  long rc = 0;
> > +/* The field 'foreign_id' should be 0 when mapping MMIO. */
> > +domid_t inv = (xatp->space != XENMAPSPACE_dev_mmio) ? DOMID_INVALID : 
> > 0;
> 
> This is a bad type for something that now isn't a domain ID anymore.
> Please use u16 or even better unsigned int. Eventually we should
> fix xenmem_add_to_physmap_one()'s respective parameter type
> accordingly.
> 
> Also I think the condition would better be space == gmfn_foreign.
> 
> > @@ -658,7 +660,7 @@ static int xenmem_add_to_physmap(struct domain *d,
> >  
> >  while ( xatp->size > done )
> >  {
> > -rc = xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID,
> > +rc = xenmem_add_to_physmap_one(d, xatp->space, inv,
> > xatp->idx, xatp->gpfn);
> 
> This instance you could actually leave alone (as it's dealing with
> XENMAPSPACE_gmfn_range only).
> 
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -259,7 +259,7 @@ struct xen_add_to_physmap_batch {
> >  
> >  /* Number of pages to go through */
> >  uint16_t size;
> > -domid_t foreign_domid; /* IFF gmfn_foreign */
> > +domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other 
> > spaces. */
> 
> I wonder whether we shouldn't fix up the structure here right away,
> instead of deferring that to after 4.7. After all, as above, we don't
> really want a domain ID here generally anymore, so this should
> either become "u16 aux" (or some such) or a union (all of course only
> for new enough __XEN_INTERFACE_VERSION__).
> 
> Plus I think we will want this to be IN/OUT, such that if the
> implementation, rather than failing, uses a replacement attribute,
> that could be communicated back. Of course that would matter
> only if we don't go the union route mentioned above.
> 
> Wei, would that be still acceptable for 4.7?
> 

Sure. It's a simple in term of code change and should be fairly easy to
review.

Wei.

> Jan
> 

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


Re: [Xen-devel] Xen Security Advisory 180 (CVE-2014-3672) - Unrestricted qemu logging

2016-05-25 Thread George Dunlap
On Mon, May 23, 2016 at 6:09 PM, Xen.org security team  wrote:
> RESOLUTION
> ==
>
> Applying the appropriate attached patch resolves this issue.
>
> The patches adopt a simple and rather crude approach which is
> effective at resolving the security issue in the context of a Xen
> device model.  They may not be appropriate for adoption upstream or in
> other contexts.

This is indeed a rather crude approach; but for our upcoming 4.7
release, what's the plan?  Do we have time to generalize xenconsoled
to handle this sort of logging, and if so, who is going to do that
work?

 -George

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


Re: [Xen-devel] [PATCH 2/4] xen/arm: Make AFFINITY_MASK generate correct mask for level3

2016-05-25 Thread Julien Grall

Hi Wei,

On 25/05/16 03:09, Wei Chen wrote:

The original affinity shift bits algorithm in AFFINITY_MASK is buggy,
it could not generate correct affinity shift bits of level3.
The macro MPIDR_LEVEL_SHIFT can calculate level3 affinity shift bits
correctly. We use this macro in AFFINITY_MASK to generate correct
mask for level3.
Signed-off-by: Wei Chen 


Reviewed-by: Julien Grall 

Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH 1/4] xen/arm: Change the variable type of cpu_logical_map to register_t

2016-05-25 Thread Julien Grall

Hi Wei,

On 25/05/16 03:09, Wei Chen wrote:

The cpu_logical_map is used to store CPU hardware ID from MPIDR_EL1 or
from CPU node of DT. Currently, the cpu_logical_map is using the u32 as
its variable type. It can work properly while Xen is running on ARM32,
because the hardware ID is 32-bits. While Xen is running on ARM64, the
hardware ID expands to 64-bits and then the cpu_logical_map will overflow.

Change the the variable type of cpu_logical_map to register_t will make


NIT: s/the the/the/


cpu_logical_map to store hardware ID correctly on ARM32 and ARM64.


s/ID/IDs/



Signed-off-by: Wei Chen 


The code looks good to me. With the typos fixed:

Acked-by: Julien Grall 

Regards,

--
Julien Grall

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


[Xen-devel] [qemu-upstream-4.3-testing test] 94761: trouble: blocked/broken

2016-05-25 Thread osstest service owner
flight 94761 qemu-upstream-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/94761/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-pvops  3 host-install(3) broken REGR. vs. 80927
 build-amd64   3 host-install(3) broken REGR. vs. 80927
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 80927
 build-i3863 host-install(3) broken REGR. vs. 80927

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-pv1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-pv   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a

version targeted for testing:
 qemuuc97c20f71240a538a19cb6b0e598bc1bbd5168f1
baseline version:
 qemuu10c1b763c26feb645627a1639e722515f3e1e876

Last test of basis80927  2016-02-06 13:30:02 Z  109 days
Testing same since93977  2016-05-10 11:09:16 Z   15 days   55 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Stefano Stabellini 

jobs:
 build-amd64  broken  
 build-i386   broken  
 build-amd64-libvirt  blocked 
 build-i386-libvirt   blocked 
 build-amd64-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-xl  blocked 
 test-amd64-i386-xl   blocked 
 test-amd64-i386-qemuu-rhel6hvm-amd   blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-i386-xl-qemuu-debianhvm-amd64 blocked 
 test-amd64-i386-freebsd10-amd64  blocked 
 test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked 
 test-amd64-i386-xl-qemuu-ovmf-amd64  blocked 
 test-amd64-amd64-xl-qemuu-win7-amd64 blocked 
 test-amd64-i386-xl-qemuu-win7-amd64  blocked 
 test-amd64-amd64-xl-credit2  blocked 
 test-amd64-i386-freebsd10-i386   blocked 
 test-amd64-i386-qemuu-rhel6hvm-intel blocked 
 test-amd64-amd64-libvirt blocked 
 test-amd64-i386-libvirt  blocked 
 test-amd64-amd64-xl-multivcpublocked 
 

Re: [Xen-devel] [for-4.7] xen/arm: Don't call setup_virtual_regions multiple time

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 09:37:09AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, May 25, 2016 at 02:14:06PM +0100, Julien Grall wrote:
> > The commit 2aa925be84293b44ad587ed117184ace61b41dd6 "arm/x86: Use struct
> > virtual_region to do bug, symbol, and (x86) exception tables lookup."
> > has introduced virtual_region. The call to initialize those regions is
> > made in init_traps which is called during each CPU bring up.
> > 
> > This will result to register multiple time the same region and Xen crash
> > when an address is looked up.
> 
> AAh, and that would explain why I didn't see it when I ran it under
> the emulator - I couldn't boot it with more than one CPU (the TIMER bug)!
> 
> > 
> > This can be fixed by moving the call to setup_virtual_region directly in
> > start_xen.
> > 
> > Signed-off-by: Julien Grall 
> > Reported-by: Chenxia Zhao 
> 
> Reviewed-by: Konrad Rzeszutek Wilk 

Pushed. Thanks everyone.

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


Re: [Xen-devel] [PATCH for-4.7] xl: use xstrdup in cpurange_parse

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 02:23:56PM +0100, Wei Liu wrote:
> This ensures buf is always valid when it is passed to strtok_r.
> 
> CID: 1291936
> 
> Signed-off-by: Wei Liu 

Pushed. Thanks everyone.

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


Re: [Xen-devel] [PATCH 0/4] xen/arm: arm64: Widen register access to mpidr to 64-bits

2016-05-25 Thread Julien Grall

Hi Wei,

Please try to send all the patches with the cover letter as references. 
git send-email should do it if you send all the patches at once.


This is really helpful to with mail client supporting thread.

Regards,

On 25/05/16 03:08, Wei Chen wrote:

In ARM64 the MPIDR register is mapped to MPIDR_EL1, and the register
bits are expanded to 64-bits. But Xen 64-bit ARM code treats this it
as 32-bit register.
We have to provide correct accessing to this register to avoid
unexpected issues that is caused by incorrect MPIDR value.

Wei Chen (4):
   xen/arm: Change the variable type of cpu_logical_map to register_t
   xen/arm: Make AFFINITY_MASK generate correct mask for level3
   xen:arm: arm64: Add correct MPIDR_HWID_MASK value for ARM64
   xen/arm: arm64: Remove MPIDR multiprocessing extensions check

  xen/arch/arm/arm64/head.S   |  3 +--
  xen/arch/arm/gic-v3.c   |  2 +-
  xen/arch/arm/smpboot.c  | 13 +++--
  xen/include/asm-arm/processor.h |  9 +++--
  4 files changed, 16 insertions(+), 11 deletions(-)



--
Julien Grall

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


Re: [Xen-devel] [for-4.7] xen/arm: Don't call setup_virtual_regions multiple time

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 02:14:06PM +0100, Julien Grall wrote:
> The commit 2aa925be84293b44ad587ed117184ace61b41dd6 "arm/x86: Use struct
> virtual_region to do bug, symbol, and (x86) exception tables lookup."
> has introduced virtual_region. The call to initialize those regions is
> made in init_traps which is called during each CPU bring up.
> 
> This will result to register multiple time the same region and Xen crash
> when an address is looked up.
> 
> This can be fixed by moving the call to setup_virtual_region directly in
> start_xen.
> 
> Signed-off-by: Julien Grall 
> Reported-by: Chenxia Zhao 
> 

Also fwiw:
Reviewed-by: Wei Liu 

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


Re: [Xen-devel] [for-4.7] xen/arm: Don't call setup_virtual_regions multiple time

2016-05-25 Thread Konrad Rzeszutek Wilk
On Wed, May 25, 2016 at 02:14:06PM +0100, Julien Grall wrote:
> The commit 2aa925be84293b44ad587ed117184ace61b41dd6 "arm/x86: Use struct
> virtual_region to do bug, symbol, and (x86) exception tables lookup."
> has introduced virtual_region. The call to initialize those regions is
> made in init_traps which is called during each CPU bring up.
> 
> This will result to register multiple time the same region and Xen crash
> when an address is looked up.

AAh, and that would explain why I didn't see it when I ran it under
the emulator - I couldn't boot it with more than one CPU (the TIMER bug)!

> 
> This can be fixed by moving the call to setup_virtual_region directly in
> start_xen.
> 
> Signed-off-by: Julien Grall 
> Reported-by: Chenxia Zhao 

Reviewed-by: Konrad Rzeszutek Wilk 
> 
> ---
> 
> Cc: Konrad Rzeszutek Wilk 
> 
> This is a bug fix for Xen 4.7. Without this change, any use of
> virtual_region (printing a symbol) could lead to a crash in Xen.
> ---
>  xen/arch/arm/setup.c | 1 +
>  xen/arch/arm/traps.c | 2 --
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 09ff1ea..9bc11c4 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -722,6 +722,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>  set_current((struct vcpu *)0xf000); /* debug sanity */
>  idle_vcpu[0] = current;
>  
> +setup_virtual_regions(NULL, NULL);
>  /* Initialize traps early allow us to get backtrace when an error 
> occurred */
>  init_traps();
>  
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 1828ea1..aa3e3c2 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -102,8 +102,6 @@ integer_param("debug_stack_lines", debug_stack_lines);
>  
>  void init_traps(void)
>  {
> -setup_virtual_regions(NULL, NULL);
> -
>  /* Setup Hyp vector base */
>  WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
>  
> -- 
> 1.9.1
> 

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


Re: [Xen-devel] [PATCH for-4.7] xl: use xstrdup in cpurange_parse

2016-05-25 Thread Wei Liu
On Wed, May 25, 2016 at 02:33:41PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH for-4.7] xl: use xstrdup in cpurange_parse"):
> > This ensures buf is always valid when it is passed to strtok_r.
> > 
> > CID: 1291936
> > 
> > Signed-off-by: Wei Liu 
> 
> Acked-by: Ian Jackson 
> 
> > This is a backport candidate.
> 
> Really ?  malloc failing is vanishingly rare in practice nowadays and
> the consequence is a crash (which is what xstrdup achieves anyway).
> 

This is a fair argument. I will leave the judgement to you. If you don't
think this is worth backporting I won't insist. :-)

> Ian.

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


Re: [Xen-devel] [PATCH for-4.7] xl: use xstrdup in cpurange_parse

2016-05-25 Thread Ian Jackson
Wei Liu writes ("[PATCH for-4.7] xl: use xstrdup in cpurange_parse"):
> This ensures buf is always valid when it is passed to strtok_r.
> 
> CID: 1291936
> 
> Signed-off-by: Wei Liu 

Acked-by: Ian Jackson 

> This is a backport candidate.

Really ?  malloc failing is vanishingly rare in practice nowadays and
the consequence is a crash (which is what xstrdup achieves anyway).

Ian.

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


Re: [Xen-devel] [PATCH for-4.7] xl: use xstrdup in cpurange_parse

2016-05-25 Thread Andrew Cooper
On 25/05/16 14:23, Wei Liu wrote:
> This ensures buf is always valid when it is passed to strtok_r.
>
> CID: 1291936
>
> Signed-off-by: Wei Liu 
> ---
> Cc: Ian Jackson 

Reviewed-by: Andrew Cooper 

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


Re: [Xen-devel] [RFC for-4.8 0/6] xen/arm: Add support for mapping mmio-sram nodes into dom0

2016-05-25 Thread Edgar E. Iglesias
On Tue, May 24, 2016 at 08:44:41PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 23/05/2016 16:42, Edgar E. Iglesias wrote:
> >On Mon, May 23, 2016 at 04:13:53PM +0100, Julien Grall wrote:
> >>On 23/05/16 15:02, Edgar E. Iglesias wrote:
> >>>On Mon, May 23, 2016 at 02:02:39PM +0100, Julien Grall wrote:
> (CC Wei Liu)
> 
> On 23/05/16 12:56, Edgar E. Iglesias wrote:
> >On Mon, May 23, 2016 at 11:29:31AM +0100, Julien Grall wrote:
> >>On 20/05/16 16:51, Edgar E. Iglesias wrote:
> >>>From: "Edgar E. Iglesias" 
> >>>
> >>>This series adds support for mapping mmio-sram nodes into dom0
> >>>as MEMORY, cached and with RWX perms.
> >>
> >>Can you explain why you chose to map those nodes as MEMORY, cached and 
> >>with
> >>RWX perms?
> >
> >My understanding is that these mmio-sram nodes are allowed to be treated 
> >as
> >Normal memory by the guest OS.
> >Guests could potentially do any kind of memory like operations on them.
> >
> >In our specific case, dom0 won't execute code from these regions but
> >Linux/dom0 ends up using standard memcpy/memset/x functions (not
> >memcpy_fromio and friends) on the regions.
> 
> I looked at the generic sram driver in Linux (drivers/misc/sram.c) which
> actually use memcpy_{to,from}io. So how you driver differs from the 
> generic
> one? What the SRAM will contain?
> >>>
> >>>We intend to use that same driver to map the memory but mmio-sram
> >>>nodes allow you to assign the regions to other device-drivers.
> >>>
> >>>Some examples:
> >>>Documentation/devicetree/bindings/crypto/marvell-cesa.txt
> >>>arch/arm/boot/dts/orion5x.dtsi
> >>>drivers/crypto/mv_cesa.c
> >>>
> >>>The cover letter for the sram driver had an example aswell allthough
> >>>the function names have changed since (it's of_gen_pool_get now):
> >>>https://lwn.net/Articles/537728/
> >>>
> >>>Nothing explicitely says that the regions can be assumed to be mapped
> >>>as Normal memory, but on Linux they do get mapped as Mormal WC mem
> >>>(unless the no-memory-wc prop is set on the node).
> >>>The marvell-cesa example also uses plain memset on the sram.
> >>
> >>I am a bit confused with this example. From my understanding of
> >>mv_cesa_get_ram, cp->sram can point either to a normal memory (?) area (see
> >>gen_pool_dma_alloc) or a Device_nGnRE area (see devm_ioremap_resource).
> >>
> >>However, memcpy_{from,to}io should be used when dealing with MMIO (the field
> >>sram has the __iomem attribute). See the commit 0f3304dc from Russel King
> >>related to marvell/cesa.
> >
> >
> >Yeah, I'm started to get confused too. Maybe they just forgot the memset
> >in drivers/crypto/mv_cesa.c.
> >
> >There are other examples though, that don't do fromio/toio at all.
> >Documentation/devicetree/bindings/media/coda.txt
> >drivers/media/platform/coda/coda-common.c
> >
> >Allthough ofcourse, these could also be wrong. Maybe I've missunderstood how
> >mmio-sram is supposed to be used.
> 
> I have talked about the memory attribute around me and the consensus is we
> should use the most relaxed mode that does not have any security implication
> or undefined behavior for a given device.
> 
> For SRAM it would be normal memory uncached (?) when the property
> "no-memory-wc" is not present, else TBD.
> 
> I suspect we would have to relax more MMIOs in the future. Rather than
> providing a function to map, the code is very similar except the memory
> attribute, I suggest to provide a list of compatible with the memory
> attribute to use.
> 
> All the children node would inherit the memory attribute of the parent.
> 
> What do you think?

Hi again,

Looking a little closer, the place where the generic list of matches and
attributes doesn't work well is when trying to deal with the no-memory-wc
property available only in mmio-sram nodes.

We'd really need an mmio-sram specific check in that case. Either
explicitely open coded in domain_build.c or something along the lines
f the .map method. Or did you have other ideas in mind?

Best regards,
Edgar

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


Re: [Xen-devel] [PATCH 4/4] xen/arm: arm64: Remove MPIDR multiprocessing extensions check

2016-05-25 Thread Julien Grall

Hello Peng,

On 25/05/16 13:37, Peng Fan wrote:

On Wed, May 25, 2016 at 10:10:11AM +0800, Wei Chen wrote:

In ARM64, the MPIDR multiprocessing extensions bit is reserved to 1.
So, the value check for this bit is no longer necessary on ARM64.


 From ARM DDI0487A.G, I found the U bit for MPIDR_EL1:
"
Indicates a Uniprocessor system, as distinct from PE 0 in a multiprocessor 
system. The possible
values of this bit are:
0 Processor is part of a multiprocessor system.
1 Processor is part of a uniprocessor system.
"

It's not reserved to 1. Which doc are you refering to?


Please read carefully the patch. The check on MPDIR_EL1.U is kept, only 
the check to the RES1 bit ('M' for Aarch32) is removed.


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list

2016-05-25 Thread Wu, Feng


> -Original Message-
> From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> Sent: Tuesday, May 24, 2016 10:47 PM
> To: Wu, Feng ; Jan Beulich 
> Cc: andrew.coop...@citrix.com; george.dun...@eu.citrix.com; Tian, Kevin
> ; xen-devel@lists.xen.org; konrad.w...@oracle.com;
> k...@xen.org
> Subject: Re: [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu
> blocking list
> 
> On Tue, 2016-05-24 at 13:33 +, Wu, Feng wrote:
> > > From: Wu, Feng
> > > > From: Dario Faggioli [mailto:dario.faggi...@citrix.com]
> > > >
> > > > If a
> > > > vCPU is blocker, there is nothing to do, and in fact, nothing
> > > > happens
> > > > (as vcpu_sleep_nosync() and vcpu_wake() are NOP in that case).
> > > What do you mean by saying ' vcpu_sleep_nosync() and vcpu_wake()
> > > are NOP '?
> >
> > I think I understand what you meant above now.
> >
> Right. In any case, see the email I've just sent, with a detailed
> breakdown of the situation and of what actually happens.
> 
> > Do you think the following idea makes sense?
> >
> > When a pCPU is unplugged, we can just remove the vcpus on the
> > associated per-cpu blocking list, then we can choose another online
> > cpu, set the right NDST value, and put the vCPU the new per-cpu list?
> >
> Well, this does make sense to me, but the point is how you do that. I
> mean, how do you get to execute some PI adjustment code, from cpu-
> teardown code?
> 
> Right now, for what seemed to be necessary until now, we have the
> arch_vcpu_block(). Do we need more? If yes where?
> 
> From looking only at schedule.c, we already have arch_move_irqs(), can
> we take advantage of it?

I think we can add the logic in vmx_cpu_dead(). I've already have a draft
patch, and I will send it out to your guys to have a review later!

Thanks,
Feng

> 
> Regards,
> Dario
> --
> <> (Raistlin Majere)
> -
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)

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


Re: [Xen-devel] [for-4.7] xen/arm: Don't call setup_virtual_regions multiple time

2016-05-25 Thread Wei Liu
Should be "multiple times" in title.

On Wed, May 25, 2016 at 02:14:06PM +0100, Julien Grall wrote:
> The commit 2aa925be84293b44ad587ed117184ace61b41dd6 "arm/x86: Use struct
> virtual_region to do bug, symbol, and (x86) exception tables lookup."
> has introduced virtual_region. The call to initialize those regions is
> made in init_traps which is called during each CPU bring up.
> 
> This will result to register multiple time the same region and Xen crash
> when an address is looked up.
> 
> This can be fixed by moving the call to setup_virtual_region directly in
> start_xen.
> 
> Signed-off-by: Julien Grall 
> Reported-by: Chenxia Zhao 
> 
> ---
> 
> Cc: Konrad Rzeszutek Wilk 
> 
> This is a bug fix for Xen 4.7. Without this change, any use of
> virtual_region (printing a symbol) could lead to a crash in Xen.

Yes, this needs fixing.

And of course this is all ARM code and you're the maintainer so I'm fine
with this going in:

Release-acked-by: Wei Liu 

> ---
>  xen/arch/arm/setup.c | 1 +
>  xen/arch/arm/traps.c | 2 --
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 09ff1ea..9bc11c4 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -722,6 +722,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>  set_current((struct vcpu *)0xf000); /* debug sanity */
>  idle_vcpu[0] = current;
>  
> +setup_virtual_regions(NULL, NULL);
>  /* Initialize traps early allow us to get backtrace when an error 
> occurred */
>  init_traps();
>  
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 1828ea1..aa3e3c2 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -102,8 +102,6 @@ integer_param("debug_stack_lines", debug_stack_lines);
>  
>  void init_traps(void)
>  {
> -setup_virtual_regions(NULL, NULL);
> -
>  /* Setup Hyp vector base */
>  WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
>  
> -- 
> 1.9.1
> 

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


Re: [Xen-devel] [PATCH OSSTEST] make-flight: don't create ovmf tests for seabios branch

2016-05-25 Thread Ian Jackson
Wei Liu writes ("[PATCH OSSTEST] make-flight: don't create ovmf tests for 
seabios branch"):
> Signed-off-by: Wei Liu 

Acked-by: Ian Jackson 

I have queued this in a branch that will see it pushed at some point.

Thanks,
Ian.

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


[Xen-devel] [PATCH for-4.7] xl: use xstrdup in cpurange_parse

2016-05-25 Thread Wei Liu
This ensures buf is always valid when it is passed to strtok_r.

CID: 1291936

Signed-off-by: Wei Liu 
---
Cc: Ian Jackson 

This is a backport candidate.
---
 tools/libxl/xl_cmdimpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 03ab644..d8530f0 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -847,7 +847,7 @@ static int update_cpumap_range(const char *str, 
libxl_bitmap *cpumap)
  */
 static int cpurange_parse(const char *cpu, libxl_bitmap *cpumap)
 {
-char *ptr, *saveptr = NULL, *buf = strdup(cpu);
+char *ptr, *saveptr = NULL, *buf = xstrdup(cpu);
 int rc = 0;
 
 for (ptr = strtok_r(buf, ",", ); ptr;
-- 
2.1.4


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


  1   2   >