[Xen-devel] 答复: [PATCH] x86/hpet: add a lock when cpu clear cpumask in hpet_broadcast_exit();

[David Wang]

Hi Jan
    Thank you for pointing out my problem. I will  revise that.  
Answer the following.

发件人: Jan Beulich 
发送时间: 2018年4月16日 19:48
收件人: David Wang
抄送: xen-devel; Fiona Li(BJ-RD)
主题: Re: [PATCH] x86/hpet: add a lock when cpu clear cpumask in 
hpet_broadcast_exit();
    
>>> On 16.04.18 at 10:00,  wrote:
> By the hpet_get_channel(), cpus share an in-use channel somtime.
> So, core shouldn't clear cpumask while others are getting first
> cpumask. If core zero and core one share an channel, the cpumask
> is 0x3. Core zero clear cpumask between core one executing
> cpumask_empty() and cpumask_first(). The return of cpumask_first()
> is nr_cpu_ids. That would lead to ASSERT(cpu < nr_cpu_ids).

I can see your point, but that's in hpet_detach_channel() afaics,
which your description doesn't mention at all. And the assertion
would - afaict - happen through hpet_detach_channel() ->
set_channel_irq_affinity() -> cpumask_of() (as of e8bf5addc9).

Please realize that it helps review quite a bit if you write concise
descriptions for your changes.

[DavidWang]: Ok, revise it and thanks.
 

> --- a/xen/arch/x86/hpet.c
> +++ b/xen/arch/x86/hpet.c
> @@ -740,7 +740,9 @@ void hpet_broadcast_exit(void)
>  if ( !reprogram_timer(deadline) )
>  raise_softirq(TIMER_SOFTIRQ);
>  
> +    spin_lock_irq(>lock);
>  cpumask_clear_cpu(cpu, ch->cpumask);
> +    spin_unlock_irq(>lock);

Rather than this, how about eliminating the cpumask_empty() call
in favor of just the cpumask_first() one in hpet_detach_channel()
(with a local variable storing the intermediate result)? Or if acquiring
the locking can't be avoided here, you would perhaps better not
drop it before calling hpet_detach_channel() (which has only this
single call site and hence would be straightforward to adjust).

[DavidWang]:  The experiment proved that a local variable storing the 
intermediate result  can slove the problem. On one hand a local variable is 
more efficient than adding lock, On the other it is not very clear for reading. 
In fact, In hpet_detach_channel(), a lock for ch->lock will be added.  Can we 
move the lock( in hpet_detach_channel()) backward  to calling 
cpumask_clear_cpu()  and drop it in function (hpet_detach_channel()) ?
Looking forward to your suggestion.
  




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

Re: [Xen-devel] [PATCH] x86/xen: init %gs very early to avoid page faults with stack protector

[Daniel Reichelt]

> Upstream commit 36104cb9012a82e73c32a3b709257766b16bcd1d fixed that. It
> needs to be added to stable as well.

Thanks for the ptr!

Daniel



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 122315
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 122322
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 122322
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 122322
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 122322
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 122322
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 122322
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 122322
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 122322
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 122322
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass

version targeted for testing:
 xen  a6aa678fa380e9369cc44701a181142322b3a4b0
baseline version:
 xen  16fb4b5a9a79f95df17f10ba62e9f44d21cf89b5

Last test of basis   122322  2018-04-16 03:03:13 Z1 days
Testing same since   122332  2018-04-16 15:44:52 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Oleksandr Tyshchenko 

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

[Platform Team regression test user]

This run is configured for baseline tests only.

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf d4ee449d1dabee20fc36650545143a5430fa718f
baseline version:
 ovmf 665bfd41ac32b364201c07dc1c5434432730c034

Last test of basis74626  2018-04-16 11:51:59 Z0 days
Testing same since74627  2018-04-16 22:48:23 Z0 days1 attempts


People who touched revisions under test:
  Dandan Bi 

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



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

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

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


Push not applicable.


commit d4ee449d1dabee20fc36650545143a5430fa718f
Author: Dandan Bi 
Date:   Tue Apr 10 13:51:08 2018 +0800

MdeModulePkg/FPDT: Add error message for unsupported case

We have updated performance infrastructure in previous commits:
between

https://github.com/tianocore/edk2/commit/73fef64f14d1b97ae9bd4705df3becc022391eba
and

https://github.com/tianocore/edk2/commit/115eae650bfd2be2c2bc37360f4a755065e774c4
Update FPDT drivers to collect the performance data reported by
gEdkiiFpdtExtendedFirmwarePerformanceGuid.
The old implementation which collected performance data through
gEfiFirmwarePerformanceGuid is not supported now.
We should add error message to remind user for this unsupported
case in case anyone use it by mistake.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Liming Gao 

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

[Xen-devel] [PATCH v7 7/7] KVM: x86: Allow Qemu/KVM to use PVH entry point

[Maran Wilson]

For certain applications it is desirable to rapidly boot a KVM virtual
machine. In cases where legacy hardware and software support within the
guest is not needed, Qemu should be able to boot directly into the
uncompressed Linux kernel binary without the need to run firmware.

There already exists an ABI to allow this for Xen PVH guests and the ABI
is supported by Linux and FreeBSD:

   https://xenbits.xen.org/docs/unstable/misc/pvh.html

This patch enables Qemu to use that same entry point for booting KVM
guests.

Signed-off-by: Maran Wilson 
Suggested-by: Konrad Rzeszutek Wilk 
Suggested-by: Boris Ostrovsky 
Tested-by: Boris Ostrovsky 
---
 arch/x86/Kbuild   |  2 +-
 arch/x86/Kconfig  |  8 
 arch/x86/platform/pvh/Makefile|  4 ++--
 arch/x86/platform/pvh/enlighten.c | 42 +--
 4 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
index 2089e4414300..c625f57472f7 100644
--- a/arch/x86/Kbuild
+++ b/arch/x86/Kbuild
@@ -7,7 +7,7 @@ obj-$(CONFIG_KVM) += kvm/
 # Xen paravirtualization support
 obj-$(CONFIG_XEN) += xen/
 
-obj-$(CONFIG_XEN_PVH) += platform/pvh/
+obj-$(CONFIG_PVH) += platform/pvh/
 
 # Hyper-V paravirtualization support
 obj-$(subst m,y,$(CONFIG_HYPERV)) += hyperv/
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 8511d419e39f..26fef538d3ef 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -787,6 +787,14 @@ config PVH
  This option enables the PVH entry point for guest virtual machines
  as specified in the x86/HVM direct boot ABI.
 
+config KVM_GUEST_PVH
+   bool "Support for running as a KVM PVH guest"
+   depends on KVM_GUEST
+   select PVH
+   ---help---
+ This option enables starting KVM guests via the PVH entry point as
+ specified in the x86/HVM direct boot ABI.
+
 config KVM_DEBUG_FS
bool "Enable debug information for KVM Guests in debugfs"
depends on KVM_GUEST && DEBUG_FS
diff --git a/arch/x86/platform/pvh/Makefile b/arch/x86/platform/pvh/Makefile
index 9fd25efcd2a3..5dec5067c9fb 100644
--- a/arch/x86/platform/pvh/Makefile
+++ b/arch/x86/platform/pvh/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 OBJECT_FILES_NON_STANDARD_head.o := y
 
-obj-$(CONFIG_XEN_PVH) += enlighten.o
-obj-$(CONFIG_XEN_PVH) += head.o
+obj-$(CONFIG_PVH) += enlighten.o
+obj-$(CONFIG_PVH) += head.o
diff --git a/arch/x86/platform/pvh/enlighten.c 
b/arch/x86/platform/pvh/enlighten.c
index c42a9f36ee9c..0c7f570d3c16 100644
--- a/arch/x86/platform/pvh/enlighten.c
+++ b/arch/x86/platform/pvh/enlighten.c
@@ -8,6 +8,8 @@
 #include 
 #include 
 
+#include 
+
 #include 
 
 /*
@@ -39,11 +41,28 @@ void __init __weak mem_map_via_hcall(struct boot_params 
*ptr __maybe_unused)
BUG();
 }
 
-static void __init init_pvh_bootparams(void)
+static void __init init_pvh_bootparams(bool xen_guest)
 {
memset(_bootparams, 0, sizeof(pvh_bootparams));
 
-   mem_map_via_hcall(_bootparams);
+   if ((pvh_start_info.version > 0) && (pvh_start_info.memmap_entries)) {
+   struct hvm_memmap_table_entry *ep;
+   int i;
+
+   ep = __va(pvh_start_info.memmap_paddr);
+   pvh_bootparams.e820_entries = pvh_start_info.memmap_entries;
+
+   for (i = 0; i < pvh_bootparams.e820_entries ; i++, ep++) {
+   pvh_bootparams.e820_table[i].addr = ep->addr;
+   pvh_bootparams.e820_table[i].size = ep->size;
+   pvh_bootparams.e820_table[i].type = ep->type;
+   }
+   } else if (xen_guest) {
+   mem_map_via_hcall(_bootparams);
+   } else {
+   /* Non-xen guests are not supported by version 0 */
+   BUG();
+   }
 
if (pvh_bootparams.e820_entries < E820_MAX_ENTRIES_ZEROPAGE - 1) {
pvh_bootparams.e820_table[pvh_bootparams.e820_entries].addr =
@@ -74,7 +93,7 @@ static void __init init_pvh_bootparams(void)
 * environment (i.e. hardware_subarch 0).
 */
pvh_bootparams.hdr.version = 0x212;
-   pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */
+   pvh_bootparams.hdr.type_of_loader = ((xen_guest ? 0x9 : 0xb) << 4) | 0;
 
x86_init.acpi.get_root_pointer = pvh_get_root_pointer;
 }
@@ -89,13 +108,10 @@ void __init __weak xen_pvh_init(void)
BUG();
 }
 
-/*
- * When we add support for other hypervisors like Qemu/KVM, this routine can
- * selectively invoke the appropriate initialization based on guest type.
- */
-static void hypervisor_specific_init(void)
+static void hypervisor_specific_init(bool xen_guest)
 {
-   xen_pvh_init();
+   if (xen_guest)
+   xen_pvh_init();
 }
 
 /*
@@ -104,13 +120,17 @@ static void hypervisor_specific_init(void)
  */
 void __init 

[Xen-devel] [PATCH v7 3/7] xen/pvh: Create a new file for Xen specific PVH code

[Maran Wilson]

We need to refactor PVH entry code so that support for other hypervisors
like Qemu/KVM can be added more easily.

The first step in that direction is to create a new file that will
eventually hold the Xen specific routines.

Signed-off-by: Maran Wilson 
---
 arch/x86/platform/pvh/enlighten.c | 5 ++---
 arch/x86/xen/Makefile | 1 +
 arch/x86/xen/enlighten_pvh.c  | 9 +
 3 files changed, 12 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/xen/enlighten_pvh.c

diff --git a/arch/x86/platform/pvh/enlighten.c 
b/arch/x86/platform/pvh/enlighten.c
index aa1c6a6831a9..74ff1c3d2789 100644
--- a/arch/x86/platform/pvh/enlighten.c
+++ b/arch/x86/platform/pvh/enlighten.c
@@ -17,10 +17,9 @@
 /*
  * PVH variables.
  *
- * xen_pvh pvh_bootparams and pvh_start_info need to live in data segment
- * since they are used after startup_{32|64}, which clear .bss, are invoked.
+ * pvh_bootparams and pvh_start_info need to live in the data segment since
+ * they are used after startup_{32|64}, which clear .bss, are invoked.
  */
-bool xen_pvh __attribute__((section(".data"))) = 0;
 struct boot_params pvh_bootparams __attribute__((section(".data")));
 struct hvm_start_info pvh_start_info __attribute__((section(".data")));
 
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index f1b850607212..ae5c6f1f0fe0 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -20,6 +20,7 @@ obj-y := enlighten.o multicalls.o mmu.o irq.o \
 obj-$(CONFIG_XEN_PVHVM)+= enlighten_hvm.o mmu_hvm.o 
suspend_hvm.o
 obj-$(CONFIG_XEN_PV)   += setup.o apic.o pmu.o suspend_pv.o \
p2m.o enlighten_pv.o mmu_pv.o
+obj-$(CONFIG_XEN_PVH)  += enlighten_pvh.o
 
 obj-$(CONFIG_EVENT_TRACING) += trace.o
 
diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
new file mode 100644
index ..313fe499065e
--- /dev/null
+++ b/arch/x86/xen/enlighten_pvh.c
@@ -0,0 +1,9 @@
+#include 
+
+/*
+ * PVH variables.
+ *
+ * The variable xen_pvh needs to live in the data segment since it is used
+ * after startup_{32|64} is invoked, which will clear the .bss segment.
+ */
+bool xen_pvh __attribute__((section(".data"))) = 0;
-- 
2.16.1


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

[Xen-devel] [PATCH v7 4/7] xen/pvh: Move Xen specific PVH VM initialization out of common file

[Maran Wilson]

We need to refactor PVH entry code so that support for other hypervisors
like Qemu/KVM can be added more easily.

This patch moves the small block of code used for initializing Xen PVH
virtual machines into the Xen specific file. This initialization is not
going to be needed for Qemu/KVM guests. Moving it out of the common file
is going to allow us to compile kernels in the future without CONFIG_XEN
that are still capable of being booted as a Qemu/KVM guest via the PVH
entry point.

Signed-off-by: Maran Wilson 
Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Juergen Gross 
---
 arch/x86/platform/pvh/enlighten.c | 28 
 arch/x86/xen/enlighten_pvh.c  | 20 +++-
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/arch/x86/platform/pvh/enlighten.c 
b/arch/x86/platform/pvh/enlighten.c
index 74ff1c3d2789..edcff7de0529 100644
--- a/arch/x86/platform/pvh/enlighten.c
+++ b/arch/x86/platform/pvh/enlighten.c
@@ -80,26 +80,38 @@ static void __init init_pvh_bootparams(void)
x86_init.acpi.get_root_pointer = pvh_get_root_pointer;
 }
 
+/*
+ * If we are trying to boot a Xen PVH guest, it is expected that the kernel
+ * will have been configured to provide the required override for this routine.
+ */
+void __init __weak xen_pvh_init(void)
+{
+   xen_raw_printk("Error: Missing xen PVH initialization\n");
+   BUG();
+}
+
+/*
+ * When we add support for other hypervisors like Qemu/KVM, this routine can
+ * selectively invoke the appropriate initialization based on guest type.
+ */
+static void hypervisor_specific_init(void)
+{
+   xen_pvh_init();
+}
+
 /*
  * This routine (and those that it might call) should not use
  * anything that lives in .bss since that segment will be cleared later.
  */
 void __init xen_prepare_pvh(void)
 {
-   u32 msr;
-   u64 pfn;
-
if (pvh_start_info.magic != XEN_HVM_START_MAGIC_VALUE) {
xen_raw_printk("Error: Unexpected magic value (0x%08x)\n",
pvh_start_info.magic);
BUG();
}
 
-   xen_pvh = 1;
-
-   msr = cpuid_ebx(xen_cpuid_base() + 2);
-   pfn = __pa(hypercall_page);
-   wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32));
+   hypervisor_specific_init();
 
init_pvh_bootparams();
 }
diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/xen/enlighten_pvh.c
index 313fe499065e..bb5784f354b8 100644
--- a/arch/x86/xen/enlighten_pvh.c
+++ b/arch/x86/xen/enlighten_pvh.c
@@ -1,4 +1,10 @@
-#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
 
 /*
  * PVH variables.
@@ -7,3 +13,15 @@
  * after startup_{32|64} is invoked, which will clear the .bss segment.
  */
 bool xen_pvh __attribute__((section(".data"))) = 0;
+
+void __init xen_pvh_init(void)
+{
+   u32 msr;
+   u64 pfn;
+
+   xen_pvh = 1;
+
+   msr = cpuid_ebx(xen_cpuid_base() + 2);
+   pfn = __pa(hypercall_page);
+   wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32));
+}
-- 
2.16.1


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

[Xen-devel] [PATCH v7 2/7] xen/pvh: Move PVH entry code out of Xen specific tree

[Maran Wilson]

Once hypervisors other than Xen start using the PVH entry point for
starting VMs, we would like the option of being able to compile PVH entry
capable kernels without enabling CONFIG_XEN and all the code that comes
along with that. To allow that, we are moving the PVH code out of Xen and
into files sitting at a higher level in the tree.

This patch is not introducing any code or functional changes, just moving
files from one location to another.

Signed-off-by: Maran Wilson 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 MAINTAINERS| 1 +
 arch/x86/Kbuild| 2 ++
 arch/x86/platform/pvh/Makefile | 5 +
 arch/x86/{xen/enlighten_pvh.c => platform/pvh/enlighten.c} | 0
 arch/x86/{xen/xen-pvh.S => platform/pvh/head.S}| 0
 arch/x86/xen/Makefile  | 3 ---
 6 files changed, 8 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/platform/pvh/Makefile
 rename arch/x86/{xen/enlighten_pvh.c => platform/pvh/enlighten.c} (100%)
 rename arch/x86/{xen/xen-pvh.S => platform/pvh/head.S} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7bb2e9595f14..0b816f588fe1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15385,6 +15385,7 @@ L:  xen-devel@lists.xenproject.org (moderated for 
non-subscribers)
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git
 S: Supported
 F: arch/x86/xen/
+F: arch/x86/platform/pvh/
 F: drivers/*/xen-*front.c
 F: drivers/xen/
 F: arch/x86/include/asm/xen/
diff --git a/arch/x86/Kbuild b/arch/x86/Kbuild
index 0038a2d10a7a..2089e4414300 100644
--- a/arch/x86/Kbuild
+++ b/arch/x86/Kbuild
@@ -7,6 +7,8 @@ obj-$(CONFIG_KVM) += kvm/
 # Xen paravirtualization support
 obj-$(CONFIG_XEN) += xen/
 
+obj-$(CONFIG_XEN_PVH) += platform/pvh/
+
 # Hyper-V paravirtualization support
 obj-$(subst m,y,$(CONFIG_HYPERV)) += hyperv/
 
diff --git a/arch/x86/platform/pvh/Makefile b/arch/x86/platform/pvh/Makefile
new file mode 100644
index ..9fd25efcd2a3
--- /dev/null
+++ b/arch/x86/platform/pvh/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+OBJECT_FILES_NON_STANDARD_head.o := y
+
+obj-$(CONFIG_XEN_PVH) += enlighten.o
+obj-$(CONFIG_XEN_PVH) += head.o
diff --git a/arch/x86/xen/enlighten_pvh.c b/arch/x86/platform/pvh/enlighten.c
similarity index 100%
rename from arch/x86/xen/enlighten_pvh.c
rename to arch/x86/platform/pvh/enlighten.c
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/platform/pvh/head.S
similarity index 100%
rename from arch/x86/xen/xen-pvh.S
rename to arch/x86/platform/pvh/head.S
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index d83cb5478f54..f1b850607212 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 OBJECT_FILES_NON_STANDARD_xen-asm_$(BITS).o := y
-OBJECT_FILES_NON_STANDARD_xen-pvh.o := y
 
 ifdef CONFIG_FUNCTION_TRACER
 # Do not profile debug and lowlevel utilities
@@ -21,7 +20,6 @@ obj-y := enlighten.o multicalls.o mmu.o irq.o \
 obj-$(CONFIG_XEN_PVHVM)+= enlighten_hvm.o mmu_hvm.o 
suspend_hvm.o
 obj-$(CONFIG_XEN_PV)   += setup.o apic.o pmu.o suspend_pv.o \
p2m.o enlighten_pv.o mmu_pv.o
-obj-$(CONFIG_XEN_PVH)  += enlighten_pvh.o
 
 obj-$(CONFIG_EVENT_TRACING) += trace.o
 
@@ -33,4 +31,3 @@ obj-$(CONFIG_XEN_DEBUG_FS)+= debugfs.o
 obj-$(CONFIG_XEN_DOM0) += vga.o
 obj-$(CONFIG_SWIOTLB_XEN)  += pci-swiotlb-xen.o
 obj-$(CONFIG_XEN_EFI)  += efi.o
-obj-$(CONFIG_XEN_PVH)  += xen-pvh.o
-- 
2.16.1


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

[Xen-devel] [PATCH v7 1/7] xen/pvh: Split CONFIG_XEN_PVH into CONFIG_PVH and CONFIG_XEN_PVH

[Maran Wilson]

In order to pave the way for hypervisors other than Xen to use the PVH
entry point for VMs, we need to factor the PVH entry code into Xen specific
and hypervisor agnostic components. The first step in doing that, is to
create a new config option for PVH entry that can be enabled
independently from CONFIG_XEN.

Signed-off-by: Maran Wilson 
---
 arch/x86/Kconfig  | 6 ++
 arch/x86/kernel/head_64.S | 2 +-
 arch/x86/xen/Kconfig  | 3 ++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d234cca296db..8511d419e39f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -781,6 +781,12 @@ config KVM_GUEST
  underlying device model, the host provides the guest with
  timing infrastructure such as time of day, and system time
 
+config PVH
+   bool "Support for running PVH guests"
+   ---help---
+ This option enables the PVH entry point for guest virtual machines
+ as specified in the x86/HVM direct boot ABI.
+
 config KVM_DEBUG_FS
bool "Enable debug information for KVM Guests in debugfs"
depends on KVM_GUEST && DEBUG_FS
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 48385c1074a5..d83f2b110b47 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -385,7 +385,7 @@ NEXT_PAGE(early_dynamic_pgts)
 
.data
 
-#if defined(CONFIG_XEN_PV) || defined(CONFIG_XEN_PVH)
+#if defined(CONFIG_XEN_PV) || defined(CONFIG_PVH)
 NEXT_PGD_PAGE(init_top_pgt)
.quad   level3_ident_pgt - __START_KERNEL_map + _KERNPG_TABLE_NOENC
.orginit_top_pgt + L4_PAGE_OFFSET*8, 0
diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index c1f98f32c45f..5fccee76f44d 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -74,6 +74,7 @@ config XEN_DEBUG_FS
  Enabling this option may incur a significant performance overhead.
 
 config XEN_PVH
-   bool "Support for running as a PVH guest"
+   bool "Support for running as a Xen PVH guest"
depends on XEN && XEN_PVHVM && ACPI
+   select PVH
def_bool n
-- 
2.16.1


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

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

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 122212
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 122212
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 122212
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 122212
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 122212
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 122212
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass

version targeted for testing:
 qemuu801bc56336a127d9b351b3a2cc0336e4d0cb2686
baseline version:
 qemuu38e83a71d02e026d4a6d0ab1ef9855c4924c2c68

Last test of basis   122212  2018-04-12 22:53:39 Z3 days
Testing same since   122328  2018-04-16 09:43:42 Z0 days1 attempts


People who touched revisions under test:
  Alex Bennée 
  Bastian Koppelmann 
  Emilio G. Cota 
  Peter Maydell 

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

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

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf d4ee449d1dabee20fc36650545143a5430fa718f
baseline version:
 ovmf 665bfd41ac32b364201c07dc1c5434432730c034

Last test of basis   122324  2018-04-16 06:17:01 Z0 days
Testing same since   122329  2018-04-16 11:50:48 Z0 days1 attempts


People who touched revisions under test:
  Dandan Bi 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   665bfd41ac..d4ee449d1d  d4ee449d1dabee20fc36650545143a5430fa718f -> 
xen-tested-master

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

[Xen-devel] [linux-next test] 122327: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-debianhvm-amd64 15 guest-saverestore.2 fail REGR. vs. 
122317
 test-armhf-armhf-libvirt-xsm 19 leak-check/check fail REGR. vs. 122317
 test-armhf-armhf-xl  10 debian-install   fail REGR. vs. 122317

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 10 debian-install   fail REGR. vs. 122317

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-cubietruck 10 debian-install  fail like 122317
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 122317
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 122317
 test-armhf-armhf-libvirt-raw 10 debian-di-installfail  like 122317
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 122317
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 122317
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 122317
 test-armhf-armhf-xl-arndale  10 debian-install   fail  like 122317
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 122317
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 122317
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 122317
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 122317
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux6e747e25f828d54c4808f64838c0fa9b8c27d909
baseline version:
 linux18b7fd1c93e5204355ddbf2608a097d64df81b88

Last test of basis  (not found) 
Failing since   (not found) 
Testing same since   122327  2018-04-16 09:20:23 Z0 days1 attempts

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

Re: [Xen-devel] [PATCH v3 14/41] hw/display: Use the BYTE-based definitions

[Alistair Francis]

On Sun, Apr 15, 2018 at 4:42 PM, Philippe Mathieu-Daudé  wrote:
> It eases code review, unit is explicit.
>
> Patch generated using:
>
>   $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/
>
> and modified manually.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Gerd Hoffmann 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  include/hw/display/xlnx_dp.h |  5 +++--
>  hw/display/cirrus_vga.c  | 10 +-
>  hw/display/g364fb.c  |  3 ++-
>  hw/display/qxl.c | 27 ---
>  hw/display/sm501.c   |  2 +-
>  hw/display/vga-isa-mm.c  |  5 +++--
>  hw/display/vga.c |  5 +++--
>  hw/display/virtio-gpu.c  |  4 ++--
>  hw/display/vmware_vga.c  |  3 ++-
>  hw/display/xenfb.c   |  3 ++-
>  10 files changed, 35 insertions(+), 32 deletions(-)
>
> diff --git a/include/hw/display/xlnx_dp.h b/include/hw/display/xlnx_dp.h
> index ee046a5fac..8fb604dee0 100644
> --- a/include/hw/display/xlnx_dp.h
> +++ b/include/hw/display/xlnx_dp.h
> @@ -29,14 +29,15 @@
>  #include "hw/display/dpcd.h"
>  #include "hw/i2c/i2c-ddc.h"
>  #include "qemu/fifo8.h"
> +#include "qemu/units.h"
>  #include "hw/dma/xlnx_dpdma.h"
>  #include "audio/audio.h"
>
>  #ifndef XLNX_DP_H
>  #define XLNX_DP_H
>
> -#define AUD_CHBUF_MAX_DEPTH 32768
> -#define MAX_QEMU_BUFFER_SIZE4096
> +#define AUD_CHBUF_MAX_DEPTH (32 * K_BYTE)
> +#define MAX_QEMU_BUFFER_SIZE(4 * K_BYTE)
>
>  #define DP_CORE_REG_ARRAY_SIZE  (0x3AF >> 2)
>  #define DP_AVBUF_REG_ARRAY_SIZE (0x238 >> 2)
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index 138ae961b9..b6d6263297 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -27,6 +27,7 @@
>   *   available at http://home.worldonline.dk/~finth/
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "qapi/error.h"
>  #include "trace.h"
>  #include "hw/hw.h"
> @@ -2218,7 +2219,7 @@ static inline void 
> cirrus_cursor_compute_yrange(CirrusVGAState *s)
>  uint32_t content;
>  int y, y_min, y_max;
>
> -src = s->vga.vram_ptr + s->real_vram_size - 16 * 1024;
> +src = s->vga.vram_ptr + s->real_vram_size - 16 * K_BYTE;
>  if (s->vga.sr[0x12] & CIRRUS_CURSOR_LARGE) {
>  src += (s->vga.sr[0x13] & 0x3c) * 256;
>  y_min = 64;
> @@ -2347,7 +2348,7 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, 
> uint8_t *d1, int scr_y)
>  return;
>  }
>
> -src = s->vga.vram_ptr + s->real_vram_size - 16 * 1024;
> +src = s->vga.vram_ptr + s->real_vram_size - 16 * K_BYTE;
>  if (s->vga.sr[0x12] & CIRRUS_CURSOR_LARGE) {
>  src += (s->vga.sr[0x13] & 0x3c) * 256;
>  src += (scr_y - s->vga.hw_cursor_y) * 16;
> @@ -2995,8 +2996,7 @@ static void cirrus_init_common(CirrusVGAState *s, 
> Object *owner,
>
>  /* I/O handler for LFB */
>  memory_region_init_io(>cirrus_linear_io, owner, 
> _linear_io_ops, s,
> -  "cirrus-linear-io", s->vga.vram_size_mb
> -  * 1024 * 1024);
> +  "cirrus-linear-io", s->vga.vram_size_mb * M_BYTE);
>  memory_region_set_flush_coalesced(>cirrus_linear_io);
>
>  /* I/O handler for LFB */
> @@ -3013,7 +3013,7 @@ static void cirrus_init_common(CirrusVGAState *s, 
> Object *owner,
>  memory_region_set_flush_coalesced(>cirrus_mmio_io);
>
>  s->real_vram_size =
> -(s->device_id == CIRRUS_ID_CLGD5446) ? 4096 * 1024 : 2048 * 1024;
> +(s->device_id == CIRRUS_ID_CLGD5446) ? 4 * M_BYTE : 2 * M_BYTE;
>
>  /* XXX: s->vga.vram_size must be a power of two */
>  s->cirrus_addr_mask = s->real_vram_size - 1;
> diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
> index 3d75394e77..2e7af33427 100644
> --- a/hw/display/g364fb.c
> +++ b/hw/display/g364fb.c
> @@ -18,6 +18,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "hw/hw.h"
>  #include "qemu/error-report.h"
>  #include "ui/console.h"
> @@ -511,7 +512,7 @@ static void g364fb_sysbus_reset(DeviceState *d)
>
>  static Property g364fb_sysbus_properties[] = {
>  DEFINE_PROP_UINT32("vram_size", G364SysBusState, g364.vram_size,
> -8 * 1024 * 1024),
> +   8 * M_BYTE),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index a71714ccb4..f0340ae355 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -19,6 +19,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include 
>
>  #include "qapi/error.h"
> @@ -2012,11 +2013,11 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl)
>  if (qxl->vgamem_size_mb > 256) {
>  qxl->vgamem_size_mb = 256;
>  }
> -qxl->vgamem_size = qxl->vgamem_size_mb * 1024 * 1024;
> +

Re: [Xen-devel] Weird altp2m behaviour when switching early to a new view

[George Dunlap]

On Mon, Apr 16, 2018 at 7:46 PM, Razvan Cojocaru
 wrote:
> On 04/16/2018 08:47 PM, George Dunlap wrote:
>> On 04/13/2018 03:44 PM, Razvan Cojocaru wrote:
>>> On 04/11/2018 11:04 AM, Razvan Cojocaru wrote:
 Debugging continues.
>>>
>>> Finally, the attached patch seems to get the display unstuck in my
>>> scenario, although for one guest I get:
>>>
>>> (XEN) d2v0 Unexpected vmexit: reason 49
>>> (XEN) domain_crash called from vmx.c:4120
>>> (XEN) Domain 2 (vcpu#0) crashed on cpu#1:
>>> (XEN) [ Xen-4.11-unstable  x86_64  debug=y   Not tainted ]
>>> (XEN) CPU:1
>>> (XEN) RIP:0010:[]
>>> (XEN) RFLAGS: 00010246   CONTEXT: hvm guest (d2v0)
>>> (XEN) rax: f8800300   rbx: f900c0083db0   rcx: aa55aa55
>>> (XEN) rdx: fa80041bdc41   rsi: f900c00c69a0   rdi: 0001
>>> (XEN) rbp:    rsp: f88002ee9ef0   r8:  fa80041bdc40
>>> (XEN) r9:  f80001810e80   r10: fa800342aa70   r11: f88002ee9e80
>>> (XEN) r12: 0005   r13: 0001   r14: f900c00c08b0
>>> (XEN) r15: 0001   cr0: 80050031   cr4: 000406f8
>>> (XEN) cr3: ef771000   cr2: f900c00c8000
>>> (XEN) fsb: fffde000   gsb: f80001810d00   gss: 07fdc000
>>> (XEN) ds: 002b   es: 002b   fs: 0053   gs: 002b   ss: 0018   cs: 0010
>>>
>>> i.e. EXIT_REASON_EPT_MISCONFIG - so not of the woods yet. I am hoping
>>> somebody more familiar with the code can point to a more elegant
>>> solution if one exists.
>>
>> I think I have an idea what's going on, but it's complicated. :-)
>>
>> Basically, the logdirty functionality isn't simple, and needs careful
>> thought on how to integrate it.  I'll write some more tomorrow, and see
>> if I can come up with a solution.
>
> I think I know why this happens for the one guest - the other guests
> start at a certain resolution display-wise and stay that way until shutdown.
>
> This particular guest starts with a larger screen, then goes to roughly
> 2/3rds of it, then tries to go back to the initial larger one - at which
> point the above happens. I assume this corresponds to some pages being
> removed and/or added. I'll test this theory more tomorrow - if it's
> correct I should be able to reproduce the crash (with the patch) by
> simply resetting the screen resolution (increasing it).

The trick is that p2m_change_type doesn't actually iterate over the
entire p2m range, individually changing entries as it goes.  Instead
it misconfigures the entries at the top-level, which causes the kinds
of faults shown above.  As it gets faults for each entry, it checks
the current type, the logdirty ranges, and the global logdirty bit to
determine what the new types should be.

Your patch makes it so that all the altp2ms now get the
misconfiguration when the logdirty range is changed; but clearly
handling the misconfiguration isn't integrated properly with the
altp2m system yet.  Doing it right may take some thought.

 -George

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

[Xen-devel] [linux-linus test] 122325: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 118324
 test-armhf-armhf-xl-credit2   6 xen-install  fail REGR. vs. 118324
 test-armhf-armhf-libvirt-xsm 12 guest-start  fail REGR. vs. 118324
 test-armhf-armhf-xl-xsm  10 debian-install   fail REGR. vs. 118324
 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 118324
 test-armhf-armhf-xl-cubietruck 10 debian-install fail REGR. vs. 118324
 test-armhf-armhf-libvirt 10 debian-install   fail REGR. vs. 118324

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 10 debian-install   fail REGR. vs. 118324

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 118324
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 118324
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 118324
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 118324
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 118324
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 118324
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux60cc43fc888428bb2f18f08997432d426a243338
baseline version:
 linux5b7d27967dabfb17c21b0d98b29153b9e3ee71e5

Last test of basis   118324  2018-01-25 07:31:24 Z   81 days
Failing since118362  2018-01-26 16:56:17 Z   80 days   69 attempts
Testing same since   122325  2018-04-16 07:29:07 Z0 days1 attempts


3303 people touched revisions under test,
not listing them all

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

Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver

[Dongwon Kim]

Yeah, I definitely agree on the idea of expanding the use case to the 
general domain where dmabuf sharing is used. However, what you are
targetting with proposed changes is identical to the core design of
hyper_dmabuf.

On top of this basic functionalities, hyper_dmabuf has driver level
inter-domain communication, that is needed for dma-buf remote tracking
(no fence forwarding though), event triggering and event handling, extra
meta data exchange and hyper_dmabuf_id that represents grefs
(grefs are shared implicitly on driver level)

Also it is designed with frontend (common core framework) + backend
(hyper visor specific comm and memory sharing) structure for portability.
We just can't limit this feature to Xen because we want to use the same
uapis not only for Xen but also other applicable hypervisor, like ACORN.

So I am wondering we can start with this hyper_dmabuf then modify it for
your use-case if needed and polish and fix any glitches if we want to 
to use this for all general dma-buf usecases.

Also, I still have one unresolved question regarding the export/import flow
in both of hyper_dmabuf and xen-zcopy.

@danvet: Would this flow (guest1->import existing dmabuf->share underlying
pages->guest2->map shared pages->create/export dmabuf) be acceptable now?

Regards,
DW
 
On Mon, Apr 16, 2018 at 05:33:46PM +0300, Oleksandr Andrushchenko wrote:
> Hello, all!
> 
> After discussing xen-zcopy and hyper-dmabuf [1] approaches
> 
> it seems that xen-zcopy can be made not depend on DRM core any more
> 
> and be dma-buf centric (which it in fact is).
> 
> The DRM code was mostly there for dma-buf's FD import/export
> 
> with DRM PRIME UAPI and with DRM use-cases in mind, but it comes out that if
> 
> the proposed 2 IOCTLs (DRM_XEN_ZCOPY_DUMB_FROM_REFS and
> DRM_XEN_ZCOPY_DUMB_TO_REFS)
> 
> are extended to also provide a file descriptor of the corresponding dma-buf,
> then
> 
> PRIME stuff in the driver is not needed anymore.
> 
> That being said, xen-zcopy can safely be detached from DRM and moved from
> 
> drivers/gpu/drm/xen into drivers/xen/dma-buf-backend(?).
> 
> This driver then becomes a universal way to turn any shared buffer between
> Dom0/DomD
> 
> and DomU(s) into a dma-buf, e.g. one can create a dma-buf from any grant
> references
> 
> or represent a dma-buf as grant-references for export.
> 
> This way the driver can be used not only for DRM use-cases, but also for
> other
> 
> use-cases which may require zero copying between domains.
> 
> For example, the use-cases we are about to work in the nearest future will
> use
> 
> V4L, e.g. we plan to support cameras, codecs etc. and all these will benefit
> 
> from zero copying much. Potentially, even block/net devices may benefit,
> 
> but this needs some evaluation.
> 
> 
> I would love to hear comments for authors of the hyper-dmabuf
> 
> and Xen community, as well as DRI-Devel and other interested parties.
> 
> 
> Thank you,
> 
> Oleksandr
> 
> 
> On 03/29/2018 04:19 PM, Oleksandr Andrushchenko wrote:
> >From: Oleksandr Andrushchenko 
> >
> >Hello!
> >
> >When using Xen PV DRM frontend driver then on backend side one will need
> >to do copying of display buffers' contents (filled by the
> >frontend's user-space) into buffers allocated at the backend side.
> >Taking into account the size of display buffers and frames per seconds
> >it may result in unneeded huge data bus occupation and performance loss.
> >
> >This helper driver allows implementing zero-copying use-cases
> >when using Xen para-virtualized frontend display driver by
> >implementing a DRM/KMS helper driver running on backend's side.
> >It utilizes PRIME buffers API to share frontend's buffers with
> >physical device drivers on backend's side:
> >
> >  - a dumb buffer created on backend's side can be shared
> >with the Xen PV frontend driver, so it directly writes
> >into backend's domain memory (into the buffer exported from
> >DRM/KMS driver of a physical display device)
> >  - a dumb buffer allocated by the frontend can be imported
> >into physical device DRM/KMS driver, thus allowing to
> >achieve no copying as well
> >
> >For that reason number of IOCTLs are introduced:
> >  -  DRM_XEN_ZCOPY_DUMB_FROM_REFS
> > This will create a DRM dumb buffer from grant references provided
> > by the frontend
> >  - DRM_XEN_ZCOPY_DUMB_TO_REFS
> >This will grant references to a dumb/display buffer's memory provided
> >by the backend
> >  - DRM_XEN_ZCOPY_DUMB_WAIT_FREE
> >This will block until the dumb buffer with the wait handle provided
> >be freed
> >
> >With this helper driver I was able to drop CPU usage from 17% to 3%
> >on Renesas R-Car M3 board.
> >
> >This was tested with Renesas' Wayland-KMS and backend running as DRM master.
> >
> >Thank you,
> >Oleksandr
> >
> >Oleksandr Andrushchenko (1):
> >   drm/xen-zcopy: Add Xen zero-copy helper DRM driver
> >
> >  Documentation/gpu/drivers.rst   |   1 +
> 

Re: [Xen-devel] Weird altp2m behaviour when switching early to a new view

[Razvan Cojocaru]

On 04/16/2018 08:47 PM, George Dunlap wrote:
> On 04/13/2018 03:44 PM, Razvan Cojocaru wrote:
>> On 04/11/2018 11:04 AM, Razvan Cojocaru wrote:
>>> Debugging continues.
>>
>> Finally, the attached patch seems to get the display unstuck in my
>> scenario, although for one guest I get:
>>
>> (XEN) d2v0 Unexpected vmexit: reason 49
>> (XEN) domain_crash called from vmx.c:4120
>> (XEN) Domain 2 (vcpu#0) crashed on cpu#1:
>> (XEN) [ Xen-4.11-unstable  x86_64  debug=y   Not tainted ]
>> (XEN) CPU:1
>> (XEN) RIP:0010:[]
>> (XEN) RFLAGS: 00010246   CONTEXT: hvm guest (d2v0)
>> (XEN) rax: f8800300   rbx: f900c0083db0   rcx: aa55aa55
>> (XEN) rdx: fa80041bdc41   rsi: f900c00c69a0   rdi: 0001
>> (XEN) rbp:    rsp: f88002ee9ef0   r8:  fa80041bdc40
>> (XEN) r9:  f80001810e80   r10: fa800342aa70   r11: f88002ee9e80
>> (XEN) r12: 0005   r13: 0001   r14: f900c00c08b0
>> (XEN) r15: 0001   cr0: 80050031   cr4: 000406f8
>> (XEN) cr3: ef771000   cr2: f900c00c8000
>> (XEN) fsb: fffde000   gsb: f80001810d00   gss: 07fdc000
>> (XEN) ds: 002b   es: 002b   fs: 0053   gs: 002b   ss: 0018   cs: 0010
>>
>> i.e. EXIT_REASON_EPT_MISCONFIG - so not of the woods yet. I am hoping
>> somebody more familiar with the code can point to a more elegant
>> solution if one exists.
> 
> I think I have an idea what's going on, but it's complicated. :-)
> 
> Basically, the logdirty functionality isn't simple, and needs careful
> thought on how to integrate it.  I'll write some more tomorrow, and see
> if I can come up with a solution.

I think I know why this happens for the one guest - the other guests
start at a certain resolution display-wise and stay that way until shutdown.

This particular guest starts with a larger screen, then goes to roughly
2/3rds of it, then tries to go back to the initial larger one - at which
point the above happens. I assume this corresponds to some pages being
removed and/or added. I'll test this theory more tomorrow - if it's
correct I should be able to reproduce the crash (with the patch) by
simply resetting the screen resolution (increasing it).


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH v7 1/9] x86/xpti: avoid copying L4 page table contents when possible

[Juergen Gross]

On 16/04/18 17:34, Tim Deegan wrote:
> Hi,
> 
> At 02:43 -0600 on 13 Apr (1523587395), Jan Beulich wrote:
> On 12.04.18 at 20:09,  wrote:
>>> For mitigation of Meltdown the current L4 page table is copied to the
>>> cpu local root page table each time a 64 bit pv guest is entered.
>>>
>>> Copying can be avoided in cases where the guest L4 page table hasn't
>>> been modified while running the hypervisor, e.g. when handling
>>> interrupts or any hypercall not modifying the L4 page table or %cr3.
>>>
>>> So add a per-cpu flag indicating whether the copying should be
>>> performed and set that flag only when loading a new %cr3 or modifying
>>> the L4 page table.  This includes synchronization of the cpu local
>>> root page table with other cpus, so add a special synchronization flag
>>> for that case.
>>>
>>> A simple performance check (compiling the hypervisor via "make -j 4")
>>> in dom0 with 4 vcpus shows a significant improvement:
>>>
>>> - real time drops from 112 seconds to 103 seconds
>>> - system time drops from 142 seconds to 131 seconds
>>>
>>> Signed-off-by: Juergen Gross 
>>> Reviewed-by: Jan Beulich 
>>> ---
>>> V7:
>>> - add missing flag setting in shadow code
>>
>> This now needs an ack from Tim (now Cc-ed).
> 
> I may be misunderstanding how this flag is supposed to work, but this
> seems at first glance to do both too much and too little.  The sl4
> table that's being modified may be in use on any number of other
> pcpus, and might not be in use on the current pcpu.

Okay.

> It looks like the do_mmu_update path issues a flush IPI; I think that
> the equivalent IPI would be a better place to hook if you can.

I want to catch only cases where the L4 table is being modified.

> Also I'm not sure why the flag needs to be set in
> l4e_propagate_from_guest() as well as shadow_set_l4e().  Can you
> elaborate?

I narrowed down iteratively where the setting of the flag is mandatory.
Doing it in shadow_set_l4e() seemed to look right, but it wasn't enough.
Setting it also in l4e_propagate_from_guest() showed no further problems
migrating the guest.

So the latter is strictly required, while the former might be not
necessary.

What you are telling me is I need to set the flag on the other cpus,
too. I didn't experience any problems omitting that, but maybe this was
only because the flag would be set eventually some time later (e.g. in
case of a vcpu scheduling event or a scheduling in the guest leading to
the flag being set), resulting in a short hang instead of crash.


Juergen

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

Re: [Xen-devel] [RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Anthony PERARD]

On Mon, Apr 16, 2018 at 06:32:26PM +0100, Anthony PERARD wrote:
> -add-fd, to create a fdset which is use later.
> -chardev 'file,path=/dev/fdset/1,append=true', this open a char device
> on the write-end of the pipe, tell qemu that the FD is write-only, and
> not to run truncate on it.
> -mon, just start the QMP server on this new chardev.

There might be a better way than this. But the new way only exist in
2.12, and is not documented... (in --help).

With QEMU commit 0935700f8544033ebbd41e1f13cd528f8a58d24d (char: allow
passing pre-opened socket file descriptor at startup).

-- 
Anthony PERARD

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

Re: [Xen-devel] [Qemu-devel] [PATCH 08/12] os-posix: Provide new -runas : facility

[Markus Armbruster]

Ian Jackson  writes:

> Markus Armbruster writes ("Re: [Qemu-devel] [PATCH 08/12] os-posix: Provide 
> new -runas : facility"):
>> Ian Jackson  writes:
>> > That would defer the getpwnam from argument parsing to os_setup_post.
>> > I think that's undesriable.
>> 
>> No argument.  But why can't os_parse_cmd_args() call getpwnam() as it
>> does now, then store user_pwd->pw_uid, ->pw_gid and ->pw_name instead of
>> user_pwd?  Store a null name when it parses the argument as UID:GID.
>
> Oh, I see.  It seems less obvious to me than what I have done, but I
> can do it like that if you like.

I just wanted to toss out the idea.  Please use your judgenment and do
it the way you like better.

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

[Xen-devel] [RFC v2 9/9] libxl_qmp: Add a warning to not trust QEMU

[Anthony PERARD]

... even if it is not the case for the current code.

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_qmp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 83bd804219..3b167f24dd 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -16,6 +16,9 @@
 /*
  * This file implement a client for QMP (QEMU Monitor Protocol). For the
  * Specification, see in the QEMU repository.
+ *
+ * WARNING - Do not trust QEMU when writing codes for new commands or when
+ *   improving the client code.
  */
 
 /*
-- 
Anthony PERARD


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

[Xen-devel] [RFC v2 0/9] libxl: Enable save/restore/migration of a restricted QEMU

[Anthony PERARD]

Patches 1 and 3 fix debug build.
Patch 2 add some documentation to libxl_qmp.c.
Patch 4 fix an issue with qmp_next().

Patches 5 and 6 fix save of a VM with a restricted QEMU.
And the last two patchs try to fix the restore path, but it is still WIP.

Checkout the last patch comments for more information.

Anthony PERARD (9):
  libxl_event: Fix DEBUG prints
  libxl_qmp: Documentation of the logic of the QMP client
  libxl_qmp: Fix use of DEBUG_RECEIVED
  libxl_qmp: Move the buffer realloc to the same scope level as read
  libxl: Learned to send FD through QMP to QEMU
  libxl: Have QEMU save its state to a file descriptor
  libxl_qmp: Implement query-status command
  HACK libxl_exec: Check QEMU status via QMP instead of xenstore
  libxl_qmp: Add a warning to not trust QEMU

 tools/libxl/libxl_aoutils.c  | 156 
 tools/libxl/libxl_dm.c   |  44 +++--
 tools/libxl/libxl_event.c|   8 +-
 tools/libxl/libxl_exec.c |  28 --
 tools/libxl/libxl_internal.h |  33 +++
 tools/libxl/libxl_qmp.c  | 167 ++-
 6 files changed, 403 insertions(+), 33 deletions(-)

-- 
Anthony PERARD


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

[Xen-devel] [RFC v2 1/9] libxl_event: Fix DEBUG prints

[Anthony PERARD]

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_event.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 484f9bab4d..0370b6acdd 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -248,6 +248,7 @@ void libxl__ev_fd_deregister(libxl__gc *gc, libxl__ev_fd 
*ev)
 short libxl__fd_poll_recheck(libxl__egc *egc, int fd, short events) {
 struct pollfd check;
 int r;
+EGC_GC;
 
 for (;;) {
 check.fd = fd;
@@ -336,7 +337,7 @@ static void time_done_debug(libxl__gc *gc, const char *func,
 libxl__ev_time *ev, int rc)
 {
 #ifdef DEBUG
-libxl__log(CTX, XTL_DEBUG, -1,__FILE__,0,func,
+libxl__log(CTX, XTL_DEBUG, -1, __FILE__, 0, func, INVALID_DOMID,
"ev_time=%p done rc=%d .func=%p infinite=%d abs=%lu.%06lu",
ev, rc, ev->func, ev->infinite,
(unsigned long)ev->abs.tv_sec, (unsigned long)ev->abs.tv_usec);
@@ -445,6 +446,8 @@ void libxl__ev_time_deregister(libxl__gc *gc, 
libxl__ev_time *ev)
 
 static void time_occurs(libxl__egc *egc, libxl__ev_time *etime, int rc)
 {
+EGC_GC;
+
 DBG("ev_time=%p occurs abs=%lu.%06lu",
 etime, (unsigned long)etime->abs.tv_sec,
 (unsigned long)etime->abs.tv_usec);
@@ -1192,6 +1195,7 @@ static int afterpoll_check_fd(libxl__poller *poller,
 static void fd_occurs(libxl__egc *egc, libxl__ev_fd *efd, short revents_ign)
 {
 short revents_current = libxl__fd_poll_recheck(egc, efd->fd, efd->events);
+EGC_GC;
 
 DBG("ev_fd=%p occurs fd=%d events=%x revents_ign=%x revents_current=%x",
 efd, efd->fd, efd->events, revents_ign, revents_current);
@@ -2117,6 +2121,8 @@ int libxl_ao_abort(libxl_ctx *ctx, const 
libxl_asyncop_how *how)
 int libxl__ao_aborting(libxl__ao *ao)
 {
 libxl__ao *root = ao_nested_root(ao);
+AO_GC;
+
 if (root->aborting) {
 DBG("ao=%p: aborting at explicit check (root=%p)", ao, root);
 return ERROR_ABORTED;
-- 
Anthony PERARD


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

[Xen-devel] [RFC v2 7/9] libxl_qmp: Implement query-status command

[Anthony PERARD]

It check via QMP if QEMU as reach the intended status.

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_internal.h |  3 +++
 tools/libxl/libxl_qmp.c  | 39 
 2 files changed, 42 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 6352380644..3764d26463 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1857,6 +1857,9 @@ _hidden int libxl__qmp_nbd_server_stop(libxl__gc *gc, int 
domid);
 _hidden int libxl__qmp_x_blockdev_change(libxl__gc *gc, int domid,
  const char *parant,
  const char *child, const char *node);
+_hidden int libxl__qmp_query_status(libxl__gc *gc,
+int domid,
+const char *intended_status);
 /* run a hmp command in qmp mode */
 _hidden int libxl__qmp_hmp(libxl__gc *gc, int domid, const char *command_line,
char **out);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 3bb0f28bea..83bd804219 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1246,6 +1246,45 @@ int libxl__qmp_x_blockdev_change(libxl__gc *gc, int 
domid, const char *parent,
 return qmp_run_command(gc, domid, "x-blockdev-change", args, NULL, NULL);
 }
 
+static int qmp_check_status(libxl__qmp_handler *qmp,
+const libxl__json_object *response,
+void *opaque)
+{
+char **status = opaque;
+GC_INIT(qmp->ctx);
+const libxl__json_object *o;
+
+o = libxl__json_map_get("status", response, JSON_STRING);
+if (!o)
+return 1;
+*status = libxl__strdup(gc, libxl__json_object_get_string(o));
+return 0;
+}
+
+int libxl__qmp_query_status(libxl__gc *gc,
+int domid,
+const char *intended_status)
+{
+char *status = NULL;
+int rc;
+
+rc = qmp_run_command(gc, domid, "query-status", NULL,
+ qmp_check_status, );
+if (rc < 0)
+return rc;
+if (rc == 1)
+/* QMP command returned unexpected result */
+return ERROR_FAIL;
+
+LOGD(DEBUG, domid, "query-status result: %s", status);
+if (!strcmp(intended_status, status))
+/* success and ready */
+return 0;
+
+/* command success, status != intended_status */
+return ERROR_NOT_READY;
+}
+
 static int hmp_callback(libxl__qmp_handler *qmp,
 const libxl__json_object *response,
 void *opaque)
-- 
Anthony PERARD


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

[Xen-devel] [RFC v2 2/9] libxl_qmp: Documentation of the logic of the QMP client

[Anthony PERARD]

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_qmp.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index d03cb51668..be23ffea6a 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -18,6 +18,42 @@
  * Specification, see in the QEMU repository.
  */
 
+/*
+ * Logic used to send command to QEMU
+ *
+ * qmp_open():
+ *  Will open a socket and connect to QEMU.
+ *
+ * qmp_next():
+ *  Will read data sent by QEMU and then call qmp_handle_response() once a
+ *  complete QMP message is received.
+ *  The function return on timeout/error or once every data received as been
+ *  processed.
+ *
+ * qmp_handle_response()
+ *  This process json messages received from QEMU and update different list and
+ *  may call callback function.
+ *  `libxl__qmp_handler.wait_for_id` is reset once a message with this ID is
+ *processed.
+ *  `libxl__qmp_handler.callback_list`: list with ID of command sent and
+ *optional assotiated callback function. The return value of a callback is
+ *set in context.
+ *
+ * qmp_send():
+ *  Simply prepare a QMP command and send it to QEMU.
+ *  It also add a `struct callback_id_pair` on the
+ *  `libxl__qmp_handler.callback_list` via qmp_send_prepare().
+ *
+ * qmp_synchronous_send():
+ *  This function calls qmp_send(), then wait for QEMU to reply to the command.
+ *  The wait is done by calling qmp_next() over and over again until either
+ *  there is a responce for the command or there is an error.
+ *
+ *  An ID can be set for each QMP command, this is set into
+ *  `libxl__qmp_handler.wait_for_id`. qmp_next will check every response's ID
+ *  again this field and change the value of the field once the ID is found.
+ */
+
 #include "libxl_osdeps.h" /* must come before any other headers */
 
 #include 
-- 
Anthony PERARD


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

[Xen-devel] [RFC v2 6/9] libxl: Have QEMU save its state to a file descriptor

[Anthony PERARD]

In case QEMU have restricted access to the system, open the file for it,
and QEMU will save its state to this file descritor.

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_qmp.c | 38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 28bc2cabcd..3bb0f28bea 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -989,25 +989,61 @@ int libxl__qmp_system_wakeup(libxl__gc *gc, int domid)
 return qmp_run_command(gc, domid, "system_wakeup", NULL, NULL, NULL);
 }
 
+/* Find out which fdset have been allocated */
+static int qmp_fdset_add_fd_callback(libxl__qmp_handler *qmp,
+ const libxl__json_object *ret,
+ void *opaque)
+{
+const libxl__json_object *o;
+int fdset;
+
+o = libxl__json_map_get("fdset-id", ret, JSON_INTEGER);
+if (!o)
+return 1;
+
+fdset = libxl__json_object_get_integer(o);
+*(int*)opaque = fdset;
+return 0;
+}
+
 int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename, bool live)
 {
 libxl__json_object *args = NULL;
 libxl__qmp_handler *qmp = NULL;
 int rc;
+int state_fd;
+int new_fdset;
 
 qmp = libxl__qmp_initialize(gc, domid);
 if (!qmp)
 return ERROR_FAIL;
 
-qmp_parameters_add_string(gc, , "filename", (char *)filename);
+state_fd = open(filename, O_WRONLY | O_CREAT, 0600);
+if (state_fd < 0) {
+LOGED(ERROR, domid,
+  "Failed to open file %s for QEMU", filename);
+goto out;
+}
+
+qmp->fd_to_send = state_fd;
+
+rc = qmp_synchronous_send(qmp, "add-fd", NULL,
+  qmp_fdset_add_fd_callback, _fdset,
+  qmp->timeout);
+if (rc)
+goto out;
 
 /* live parameter was added to QEMU 2.11. It signal QEMU that the save
  * operation is for a live migration rather that for taking a snapshot. */
 if (qmp_qemu_check_version(qmp, 2, 11, 0))
 qmp_parameters_add_bool(gc, , "live", live);
 
+QMP_PARAMETERS_SPRINTF(, "filename", "/dev/fdset/%d", new_fdset);
 rc = qmp_synchronous_send(qmp, "xen-save-devices-state", args,
   NULL, NULL, qmp->timeout);
+out:
+if (state_fd >= 0)
+close(state_fd);
 libxl__qmp_close(qmp);
 return rc;
 }
-- 
Anthony PERARD


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

[Xen-devel] [RFC v2 5/9] libxl: Learned to send FD through QMP to QEMU

[Anthony PERARD]

Adding the ability to send a file descriptor from libxl to QEMU via the
QMP interface. This will be use with the "add-fd" QMP command.

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_qmp.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index a25f445fb6..28bc2cabcd 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -116,6 +116,9 @@ struct libxl__qmp_handler {
 int minor;
 int micro;
 } version;
+
+/* File descriptor to send to QEMU on the next command */
+int fd_to_send;
 };
 
 static int qmp_send(libxl__qmp_handler *qmp,
@@ -414,6 +417,8 @@ static libxl__qmp_handler *qmp_init_handler(libxl__gc *gc, 
uint32_t domid)
 
 LIBXL_STAILQ_INIT(>callback_list);
 
+qmp->fd_to_send = -1;
+
 return qmp;
 }
 
@@ -639,9 +644,16 @@ static int qmp_send(libxl__qmp_handler *qmp,
 goto out;
 }
 
-if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, strlen(buf),
-"QMP command", "QMP socket"))
-goto out;
+if (qmp->fd_to_send >= 0) {
+if (libxl__sendmsg_fds(gc, qmp->qmp_fd, buf, strlen(buf),
+   1, >fd_to_send, "QMP socket"))
+goto out;
+qmp->fd_to_send = -1;
+} else {
+if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, strlen(buf),
+"QMP command", "QMP socket"))
+goto out;
+}
 if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, "\r\n", 2,
 "CRLF", "QMP socket"))
 goto out;
-- 
Anthony PERARD


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

[Xen-devel] [RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Anthony PERARD]

When QEMU is restricted, the qemu on the receiving side cann't write
anything to xenstore once the migration is started. So it cann't tell
libxl that it is ready to continue running the guest.

In order to find out if QEMU is ready, we can issue QMP commands and
check if it respond.

But there is no QMP socket to connect to before qemu is started. But we
can uses different facility from qemu in order to setup some kind of
callback before starting QEMU. For that, we open a file descriptor and
give it to qemu.

This patch creates a pipe, give the write-end to qemu, and wait for
something to be written to it. (We could check if it is actually the QMP
greeting message.)

QEMU is asked to setup a QMP server on this pipe, but even if it is a
one-way only, qemu will write the QMP greeting message to the pipe.
This is done with:
-add-fd, to create a fdset which is use later.
-chardev 'file,path=/dev/fdset/1,append=true', this open a char device
on the write-end of the pipe, tell qemu that the FD is write-only, and
not to run truncate on it.
-mon, just start the QMP server on this new chardev.

With that, qemu will start the QMP server on the write-only fd, which is
enough to have the QMP greeting message. At this point, the QMP socket
is ready, and I think qemu is in the main-loop and ready to start the
emulation and respond to QMP commands.

This patch calls 'query-status', any response to that without error
means that QEMU is ready. If the status is "running", QEMU would already
have written the xenstore node if it could and is doing emulation.
(Any subsequent QMP command 'cont', libxl__qmp_resume(),  is not
changing anything.  If one adds '-S' to the QEMU command line, QEMU will
have the status "prelaunch" as a response to 'query-status', then QEMU
can be asked to start emulation with 'cont' via QMP.)

This patch copies most of "xswait" and call it "qmpwait". This is
probably not the best way forward due to duplication.

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_aoutils.c  | 156 +++
 tools/libxl/libxl_dm.c   |  44 --
 tools/libxl/libxl_exec.c |  28 +--
 tools/libxl/libxl_internal.h |  30 +++
 4 files changed, 246 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 9e493cd487..7d654996c3 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -626,6 +626,162 @@ void libxl__kill(libxl__gc *gc, pid_t pid, int sig, const 
char *what)
 what, (unsigned long)pid, sig);
 }
 
+/*- qmpwait -*/
+
+static libxl__ev_fd_callback qmpwait_fd_read_callback;
+static libxl__ev_time_callback qmpwait_timeout_callback;
+static void qmpwait_report_error(libxl__egc*, libxl__qmpwait_state*, int rc);
+
+void libxl__qmpwait_init(libxl__qmpwait_state *qmpwa)
+{
+libxl__ev_time_init(>time_ev);
+libxl__ev_fd_init(>notify_efd);
+}
+
+void libxl__qmpwait_stop(libxl__gc *gc, libxl__qmpwait_state *qmpwa)
+{
+libxl__ev_time_deregister(gc, >time_ev);
+libxl__ev_fd_deregister(gc, >notify_efd);
+libxl__carefd_close(qmpwa->notify_cfd);
+qmpwa->notify_cfd = NULL;
+}
+
+bool libxl__qmpwait_inuse(const libxl__qmpwait_state *qmpwa)
+{
+bool time_inuse = libxl__ev_time_isregistered(>time_ev);
+bool watch_inuse = libxl__ev_fd_isregistered(>notify_efd);
+assert(time_inuse == watch_inuse);
+return time_inuse;
+}
+
+int libxl__qmpwait_start(libxl__gc *gc, libxl__qmpwait_state *qmpwa)
+{
+int rc;
+
+rc = libxl__ev_time_register_rel(qmpwa->ao, >time_ev,
+ qmpwait_timeout_callback, 
qmpwa->timeout_ms);
+if (rc) goto err;
+
+rc = libxl__ev_fd_register(gc, >notify_efd,
+   qmpwait_fd_read_callback,
+   libxl__carefd_fd(qmpwa->notify_cfd),
+   POLLIN);
+if (rc) goto err;
+
+return 0;
+
+ err:
+libxl__qmpwait_stop(gc, qmpwa);
+return rc;
+}
+
+static void qmpwait_fd_read_callback(libxl__egc *egc, libxl__ev_fd *ev,
+ int fd, short events, short revents)
+{
+// checkout:
+// - datacopier_readable
+// - watchfd_callback
+
+libxl__qmpwait_state *qmpwa = CONTAINER_OF(ev, *qmpwa, notify_efd);
+libxl__spawn_state *ss = CONTAINER_OF(ev, *ss, qmpwait.notify_efd);
+STATE_AO_GC(qmpwa->ao);
+
+// need to handle POLLERR
+// POLLHUP and POLLIN might be both set.
+if (revents & POLLHUP && !(revents & POLLIN)) {
+LOG(DEBUG, "received POLLHUP on fd %d: read for %s",
+fd, qmpwa->what);
+libxl__ev_fd_deregister(gc, >notify_efd);
+if (!qmpwa->life_time_read)
+ss->failure_cb(egc, ss, ERROR_FAIL);
+return;
+}
+if (revents & ~(POLLIN|POLLHUP)) {
+LOG(ERROR, "unexpected poll event 0x%x on fd %d (expected POLLIN "
+"and/or POLLHUP) reading %s",
+

[Xen-devel] [RFC v2 3/9] libxl_qmp: Fix use of DEBUG_RECEIVED

[Anthony PERARD]

This patch fix complilation error with #define DEBUG_RECEIVED of the
macro DEBUG_REPORT_RECEIVED.

  error: field precision specifier ‘.*’ expects argument of type ‘int’, but 
argument 9 has type ‘ssize_t {aka long int}’

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_qmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index be23ffea6a..4d207c3842 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -522,7 +522,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
 }
 qmp->buffer[rd] = '\0';
 
-DEBUG_REPORT_RECEIVED(qmp->domid, qmp->buffer, rd);
+DEBUG_REPORT_RECEIVED(qmp->domid, qmp->buffer, (int)rd);
 
 do {
 char *end = NULL;
-- 
Anthony PERARD


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

Re: [Xen-devel] [PATCH 3/7] xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)

[Julien Grall]

On 16/04/18 17:52, Mirela Simonovic wrote:

On Mon, Apr 16, 2018 at 4:26 PM, Julien Grall  wrote:

Hi Mirela,


On 16/04/18 11:02, Mirela Simonovic wrote:


On Thu, Apr 12, 2018 at 3:31 PM, Julien Grall 
wrote:

On 12/04/18 12:33, Mirela Simonovic wrote:

On Wed, Apr 11, 2018 at 4:46 PM, Julien Grall 
wrote:

On 11/04/18 14:19, Mirela Simonovic wrote:


I disagree here, if you are unable to turn off a CPU via PSCI then something
is definitely wrong. This means that CPU will forever spin in Xen code with
no way to exit. This could bring interesting issue with anything potentially
modifying Xen code (i.e livepatching).

IHMO, the forever sleep in stop_cpu() is just a temporary solution to cater
shutdown of the platform. The state of secondary CPU does not much matter at
that time. In case of suspend/resume you want really want to be able to turn
off those CPUs correctly otherwise they are not going to come up again.



If we follow that logic the CPU will not be able to exit WFI state
either. So we should raise panic in that case as well and in cases
where the system is suspending + the CPU is stopped + cpu off doesn't
work so the CPU cannot be enabled again.


In case of suspend/resume you can check before hand whether we cpu_off 
is implemented for that platform. If not, you deny suspend.


This will avoid people using suspend/resume on those platforms.


However, raising panic makes no sense for shutdown scenario.


I agree and that's why I suggested to move the panic in one of my 
previous e-mail:


"Furthermore, now on platform without CPU off support (e.g non-PSCI 
platform and PSCI 0.1) you will log an error message that may worry 
people. In reality, PSCI cpu_off will unlikely fail, so you probably 
want to add a panic in call_psci_cpu_off instead."



How about we do it something like this:

void stop_cpu(void)
{
 ...
 if ( system_state == SYS_STATE_suspend )
 call_psci_cpu_off();

 while ( 1 )
 wfi();
}

void call_psci_cpu_off(void)
{
 int errno;

 /* If successfull the cpu_off call doesn't return */
 errno = call_smc(PSCI_0_2_FN32_CPU_OFF, 0, 0, 0);
 if ( errno )
 panic("PSCI cpu off failed for CPU%d err=%d\n", get_processor_id(),
 errno);
}

call_psci_cpu_off should not check for PSCI version because we need to
panic regardless.


Yes. There are no need for the check because it will never return on 
success.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [Qemu-devel] [PATCH 08/12] os-posix: Provide new -runas : facility

[Ian Jackson]

Markus Armbruster writes ("Re: [Qemu-devel] [PATCH 08/12] os-posix: Provide new 
-runas : facility"):
> Ian Jackson  writes:
> > That would defer the getpwnam from argument parsing to os_setup_post.
> > I think that's undesriable.
> 
> No argument.  But why can't os_parse_cmd_args() call getpwnam() as it
> does now, then store user_pwd->pw_uid, ->pw_gid and ->pw_name instead of
> user_pwd?  Store a null name when it parses the argument as UID:GID.

Oh, I see.  It seems less obvious to me than what I have done, but I
can do it like that if you like.

Ian.

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

Re: [Xen-devel] [PATCH 3/7] xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)

[Mirela Simonovic]

Hi Julien,

On Mon, Apr 16, 2018 at 4:26 PM, Julien Grall  wrote:
> Hi Mirela,
>
>
> On 16/04/18 11:02, Mirela Simonovic wrote:
>>
>> On Thu, Apr 12, 2018 at 3:31 PM, Julien Grall 
>> wrote:
>>>
>>>
>>>
>>> On 12/04/18 12:33, Mirela Simonovic wrote:


 On Wed, Apr 11, 2018 at 4:46 PM, Julien Grall 
 wrote:
>
>
> On 11/04/18 14:19, Mirela Simonovic wrote:
>
>> local_irq_disable();
>> cpu_is_dead = true;
>> /* Make sure the write happens before we sleep forever */
>> dsb(sy);
>> isb();
>> +/* PSCI cpu off call will return only in case of an error */
>> +errno = call_psci_cpu_off();
>> +printk(XENLOG_DEBUG "PSCI cpu off call failed for CPU#%d
>> err=%d\n",
>> +   get_processor_id(), errno);
>> +isb();
>
>
>
>
> What are you trying to achieve with the isb() here?
>

 I use to have a problem that the wfi below gets executed before the
 call_psci_cpu_off(). Adding isb() fixed the issue. However, I tried
 now to reproduce the problem and it doesn't show up. I still believe
 isb() should be here, please let me know if you disagree (I obviously
 can't prove the claim now).
>>>
>>>
>>>
>>> The problem you describe can't be possible with the code you have because
>>> call_psci_cpu_off() is issuing a SMC. SMC will lead to change exception
>>> level and therefore have a context-synchronization barrier.
>>>
>>> This is obviously based on the assumption you don't have an errata on
>>> your
>>> CPU exposing the behavior you describe. For that you would need to check
>>> errata notice for your CPU and/or try to reproduce.
>>>
>>> However, what you would need is a dsb(sy); isb(); to drain the write
>>> buffer
>>> if you print a message.
>>>
>>> Furthermore, now on platform without CPU off support (e.g non-PSCI
>>> platform
>>> and PSCI 0.1) you will log an error message that may worry people. In
>>> reality, PSCI cpu_off will unlikely fail, so you probably want to add a
>>> panic in call_psci_cpu_off instead.
>>>
>>
>> Even if PSCI cpu_off call fails, what is unlikely to happen, the
>> system is still functional.
>
>
> I disagree here, if you are unable to turn off a CPU via PSCI then something
> is definitely wrong. This means that CPU will forever spin in Xen code with
> no way to exit. This could bring interesting issue with anything potentially
> modifying Xen code (i.e livepatching).
>
> IHMO, the forever sleep in stop_cpu() is just a temporary solution to cater
> shutdown of the platform. The state of secondary CPU does not much matter at
> that time. In case of suspend/resume you want really want to be able to turn
> off those CPUs correctly otherwise they are not going to come up again.
>

If we follow that logic the CPU will not be able to exit WFI state
either. So we should raise panic in that case as well and in cases
where the system is suspending + the CPU is stopped + cpu off doesn't
work so the CPU cannot be enabled again.
However, raising panic makes no sense for shutdown scenario.
How about we do it something like this:

void stop_cpu(void)
{
...
if ( system_state == SYS_STATE_suspend )
call_psci_cpu_off();

while ( 1 )
wfi();
}

void call_psci_cpu_off(void)
{
int errno;

/* If successfull the cpu_off call doesn't return */
errno = call_smc(PSCI_0_2_FN32_CPU_OFF, 0, 0, 0);
if ( errno )
panic("PSCI cpu off failed for CPU%d err=%d\n", get_processor_id(),
errno);
}

call_psci_cpu_off should not check for PSCI version because we need to
panic regardless.

Thanks,
Mirela


>> Enabling that pCPU later will fail, but
>> Xen can handle this error and continue running properly on the boot
>> pCPU (I've tested this in 2 pCPUs config).
>
>
> I don't consider that as xen running properly. You lost a pCPU so your
> workload is completely different. Imagine you are using the NULL scheduler
> (e.g only one vCPU is pinned to a specific pCPU), what are you going to do
> with the vCPU?
>
>> Therefore, I believe panic may not be necessary in this case. I
>> suggest that we dump the error message and continue to run. Please let
>> me know if you disagree.
>
>
> This is a bad idea, a failure should at least be logged to show something
> gone wrong.
>
> PSCI CPU off will, as you said, unlikely failed. Looking at the spec, the
> only possible reason is your are trying to turn off a CPU where Trusted OS
> is resident. This means something far more wrong is happening in Xen code
> and I don't think it would be safe to continue to run.
>
> Hence why I suggested a BUG_ON/panic because this is something that is not
> meant to happen.
>
> Cheers,
>
> --
> Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org

Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()

[Andrew Cooper]

On 16/04/18 13:32, Jan Beulich wrote:
 On 16.04.18 at 14:18,  wrote:
>> On 13/04/18 12:39, Jan Beulich wrote:
>> On 13.04.18 at 13:17,  wrote:
 On 13/04/18 09:31, Jan Beulich wrote:
 On 12.04.18 at 18:55,  wrote:
>> do_get_debugreg() has several bugs:
>>
>>  * The %cr4.de condition is inverted.  %dr4/5 should be accessible only 
>> when
>>%cr4.de is disabled.
>>  * When %cr4.de is disabled, emulation should yield #UD rather than 
>> complete
>>with zero.
>>  * Using -EINVAL for errors is a broken ABI, as it overlaps with valid 
>> values
>>near the top of the address space.
>>
>> Introduce a common x86emul_read_dr() handler (as we will eventually want 
>> to
>> add HVM support) which separates its success/failure indication from the 
>> data
>> value, and have do_get_debugreg() call into the handler.
> The HVM part here is sort of questionable because of your use of
> curr->arch.pv_vcpu.ctrlreg[4].
 That is what the "needs further plumbing" refers to, as well as needing
 hooks to get/modify %dr6/7 from the VMCB/VMCS.

 However, we are gaining an increasing amount of common x86 code which
 needs to read control register values, and I've got a plan to refactor
 across the board to v->arch.cr4 (and similar).  There is no point having
 identical information in different parts of sub-unions.
>>> I agree.
>>>
> This is appropriate for the NULL ctxt case,
> but it's already a layering violation for the use of the function in
> priv_op_ops, where the read_cr() hook should be used instead.
 Hmm - doing this, while probably the better long temr course of action,
 would require passing the ops structures down into the callbacks.
>>> That doesn't sound like a problem, though - the hypercall path would
>>> pass NULL there as well.
> This ...
>
>> +int x86emul_read_dr(unsigned int reg, unsigned long *val,
>> +struct x86_emulate_ctxt *ctxt)
>> +{
>> +struct vcpu *curr = current;
>> +
>> +/* HVM support requires a bit more plumbing before it will work. */
>> +ASSERT(is_pv_vcpu(curr));
>> +
>> +switch ( reg )
>> +{
>> +case 0 ... 3:
>> +case 6:
>> +*val = curr->arch.debugreg[reg];
>> +break;
>> +
>> +case 7:
>> +*val = (curr->arch.debugreg[7] |
>> +curr->arch.debugreg[5]);
>> +break;
>> +
>> +case 4 ... 5:
>> +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) )
>> +{
>> +*val = curr->arch.debugreg[reg + 2];
>> +break;
> Once at it, wouldn't you better also fix the missing ORing of [5] into 
> the DR7 (really
> DR5) value here?
 [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent
 patch), as IO breakpoints are only valid to use when %cr4.de is enabled.
>>> Oh, right you are.
>> So, are your comments suitably addressed?  It is unclear whether you
>> want any changes to be made.
> ... is what I'd prefer to be taken care of without delaying to the time when
> we make this work for HVM as well. Unless you feel really strongly about it
> being better the way you have it, in which case you may feel free to add
> my ack.

In all PV cases (hypercall and emulation), the current code functions
correctly, because DE is active in context.

In principle, the emulation case would be better if it used the
read_cr() hook, but that is invasive to arrange (which is why I chose
not to at this point), and still needs special casing for the hypercall
case anyway.

~Andrew

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

Re: [Xen-devel] [Qemu-devel] [PATCH 08/12] os-posix: Provide new -runas : facility

[Markus Armbruster]

Ian Jackson  writes:

> Thanks for the review.  Taking your comments out of order slightly:
>
> Markus Armbruster writes ("Re: [Qemu-devel] [PATCH 08/12] os-posix: Provide 
> new -runas : facility"):
>> [change_process_uid] is the only user of @user_pwd, @user_uid, @user_gid.
>> 
>> Have you considered replacing global @user_pwd by @user_uid, @user_gid
>> and @user_name?  --runas with numeric uid and gid would leave @user_name
>> null.
>
> That would defer the getpwnam from argument parsing to os_setup_post.
> I think that's undesriable.

No argument.  But why can't os_parse_cmd_args() call getpwnam() as it
does now, then store user_pwd->pw_uid, ->pw_gid and ->pw_name instead of
user_pwd?  Store a null name when it parses the argument as UID:GID.

>> Ian Jackson  writes:
>> >  static struct passwd *user_pwd;
>> > +static uid_t user_uid = (uid_t)-1;
>> > +static gid_t user_gid = (gid_t)-1;
>> 
>> As we'll see below, @user_pwd->pw_uid, @user_pwd_pw_gid take precedence
>> over @user_uid, @user_gid.  Awkward.
>
> My patch has the right behaviour: each -runas completely overrides the
> previous one.  -runas that sets user_{uid,gid} always clears user_pwd
> on the way.  So user_pwd can only be set if the most recent -runas was
> a name, and then we should honour the name.
>
> This is rather obscure.  I think you are right that this is confusing.
> It ought to be clearer.
>
> I will
>   - add a comment next to these three variables saying they must
> all be set at the same time
>   - explicitly (redundantly) clear user_pwd in os_parse_runas_uid_gid
>   - explicitly set user_{uid,gid} to -1 when -runas gets a
> success from getpwnam
>   - assert in change_process_uid that the combination is legal

Yes, that's better.  But perhaps you like my idea above.

[...]

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

Re: [Xen-devel] [RFC 1/4] libxl: Learned to send FD through QMP to QEMU

[Anthony PERARD]

On Tue, Mar 27, 2018 at 11:29:35AM +0100, Ian Jackson wrote:
> (George, CC'ing you wrt your depriv doc patch - see below.)
> 
> Anthony PERARD writes ("[RFC 1/4] libxl: Learned to send FD through QMP to 
> QEMU"):
> > +/* File descriptor to send to QEMU on the next command */
> > +int fd_to_send;
> 
> I did wonder if this was a layering violation, or a poor API in some
> other sense.  AFAICT it isn't, and libxl__qmp_handler is completely
> transparent to everything in libxl_qmp.c.
> 
> I think this whole file would benefit from some doc comments about the
> internal interfaces.  Particularly, something describing the boundary
> between operation-specific code and the generic qmp_send machinery
> would help review of both (i) new operations and (ii) extensions of
> the generic machinery.

I'll try to write some documentation.

> Looking at this and the next patch, I think (almost?) every user of
> this new feature will need to tell qmp_send to call
> qmp_fdset_add_fd_callback.  Is that right ?  Maybe this means we want
> to provide a more cooked version.

Not exactly. We can add a parameter to "add-fd" to use a specific
"fdset-id".

> Anthony PERARD writes ("[RFC 2/4] libxl: Have QEMU save its state to a file 
> descriptor"):
> > In case QEMU have restricted access to the system, open the file for it,
> > and QEMU will save its state to this file descritor.
> 
> This 2nd patch looks reasonable, but it prompted to notice two new
> kinds of hazard introduced by the deprivileging design goal:
> 
> >  int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename, bool 
> > live)
> >  {
> ...
> > +rc = qmp_synchronous_send(qmp, "add-fd", NULL,
> > +  qmp_fdset_add_fd_callback, _fdset,
> > +  qmp->timeout);
> > +if (rc)
> > +goto out;
> 
> By this point, a depriv'd qemu must be assumed to be compromised by
> its guest - ie we must treat it as hostile.
> 
> This is not consistent with use of qmp_synchronous_send, because
> qmp_synchronous_send will block with both the domain and ctx locks
> held.  That is, a malicious qemu can deny service; it even has the
> ability to prevent its serviced domain from being destroyed.
> 
> Secondly, the point about qemu now being malicious means that we need
> to audit the code which handles communications with qemu for safety.
> 
> I think this means that:
> 
>  * George's todo list patch for the depriv doc should mention
>the need to replace qmp_synchronous_send with qemp_send.
> 
>  * Likewise it should mention the need for this audit.
> 
>  * We should write a comment somewhere (near the top of libxl_qmp.c
>perhaps) warning developers not to treat qemu as trusted.  That
>would usefully fit into your own series.
> 
> I volunteer to do the audit.  Some internal commentary about the
> internal interfaces (as I discuss above) would be helpful for that.

I think we would need to rewrite part of libxl_qmp.c to use the
libxl__ev_*, so QEMU would not be able to block libxl.

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH 2/8] x86/HVM/SVM: Add AVIC initialization code

[Jan Beulich]

>>> On 04.04.18 at 01:01,  wrote:
> From: Suravee Suthikulpanit 
> 
> Introduce AVIC base initialization code. This includes:
> * Setting up per-VM data structures.
> * Setting up per-vCPU data structure.
> * Initializing AVIC-related VMCB bit fields.
> 
> This patch also introduces a new Xen parameter (svm-avic),
> which can be used to enable/disable AVIC support.

This sentence looks stale/misplaced now.

> --- /dev/null
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -0,0 +1,191 @@
> +/*
> + * avic.c: implements AMD Advanced Virtual Interrupt Controller (AVIC) 
> support
> + * Copyright (c) 2016, Advanced Micro Devices, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * Note: Current max index allowed for physical APIC ID table is 255.
> + */

This is a single line comment.

> +#define AVIC_PHY_APIC_ID_MAX0xFF

Is this really an AVIC-specific constant, rather than e.g.
GET_xAPIC_ID(APIC_ID_MASK)?

> +#define AVIC_VAPIC_BAR_MASK (((1ULL << 40) - 1) << PAGE_SHIFT)
> +
> +/*
> + * Note:
> + * Currently, svm-avic mode is not supported with nested virtualization.
> + * Therefore, it is not yet currently enabled by default. Once the support
> + * is in-place, this should be enabled by default.
> + */
> +bool svm_avic = 0;

false (or simply omit the intializer)

> +int svm_avic_dom_init(struct domain *d)
> +{
> +int ret = 0;
> +struct page_info *pg;
> +
> +if ( !svm_avic || !has_vlapic(d) )
> +return 0;
> +
> +/*
> + * Note:
> + * AVIC hardware walks the nested page table to check permissions,
> + * but does not use the SPA address specified in the leaf page
> + * table entry since it uses  address in the AVIC_BACKING_PAGE pointer
> + * field of the VMCB. Therefore, we set up a dummy page for APIC _mfn(0).
> + */
> +set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE),
> +   _mfn(0), PAGE_ORDER_4K,
> +   p2m_get_hostp2m(d)->default_access);

The use of MFN 0 here looks risky to me: How do you guarantee nothing else
might ever use that P2M entry?

> +/* Init AVIC logical APIC ID table */
> +pg = alloc_domheap_page(d, MEMF_no_owner);
> +if ( !pg )
> +{
> +ret = -ENOMEM;
> +goto err_out;
> +}
> +clear_domain_page(_mfn(page_to_mfn(pg)));
> +d->arch.hvm_domain.svm.avic_logical_id_table_pg = pg;
> +d->arch.hvm_domain.svm.avic_logical_id_table = 
> __map_domain_page_global(pg);

Both here and ...

> +/* Init AVIC physical APIC ID table */
> +pg = alloc_domheap_page(d, MEMF_no_owner);
> +if ( !pg )
> +{
> +ret = -ENOMEM;
> +goto err_out;
> +}
> +clear_domain_page(_mfn(page_to_mfn(pg)));
> +d->arch.hvm_domain.svm.avic_physical_id_table_pg = pg;
> +d->arch.hvm_domain.svm.avic_physical_id_table = 
> __map_domain_page_global(pg);

... here: Do you really need to store both page pointer and map address?

> +void svm_avic_dom_destroy(struct domain *d)
> +{
> +if ( !svm_avic || !has_vlapic(d) )
> +return;
> +
> +if ( d->arch.hvm_domain.svm.avic_physical_id_table )
> +{
> +
> unmap_domain_page_global(d->arch.hvm_domain.svm.avic_physical_id_table);
> +free_domheap_page(d->arch.hvm_domain.svm.avic_physical_id_table_pg);
> +d->arch.hvm_domain.svm.avic_physical_id_table_pg = 0;
> +d->arch.hvm_domain.svm.avic_physical_id_table = 0;

DYM NULL?

> +int svm_avic_init_vmcb(struct vcpu *v)
> +{
> +u32 apic_id;
> +unsigned long tmp;
> +struct arch_svm_struct *s = >arch.hvm_svm;
> +struct vmcb_struct *vmcb = s->vmcb;
> +struct svm_domain *d = >domain->arch.hvm_domain.svm;
> +const struct vlapic *vlapic = vcpu_vlapic(v);
> +struct avic_physical_id_entry *entry = (struct avic_physical_id_entry 
> *)
> +
> +if ( !svm_avic || !has_vlapic(v->domain) )
> +return 0;
> +
> +if ( !vlapic || !vlapic->regs_page )
> +return -EINVAL;
> +
> +apic_id = vlapic_read_aligned(vcpu_vlapic(v), APIC_ID);
> +s->avic_last_phy_id = avic_get_physical_id_entry(d, 
> GET_xAPIC_ID(apic_id));
> +if ( 

Re: [Xen-devel] [PATCH v7 1/9] x86/xpti: avoid copying L4 page table contents when possible

[Tim Deegan]

Hi,

At 02:43 -0600 on 13 Apr (1523587395), Jan Beulich wrote:
> >>> On 12.04.18 at 20:09,  wrote:
> > For mitigation of Meltdown the current L4 page table is copied to the
> > cpu local root page table each time a 64 bit pv guest is entered.
> > 
> > Copying can be avoided in cases where the guest L4 page table hasn't
> > been modified while running the hypervisor, e.g. when handling
> > interrupts or any hypercall not modifying the L4 page table or %cr3.
> > 
> > So add a per-cpu flag indicating whether the copying should be
> > performed and set that flag only when loading a new %cr3 or modifying
> > the L4 page table.  This includes synchronization of the cpu local
> > root page table with other cpus, so add a special synchronization flag
> > for that case.
> > 
> > A simple performance check (compiling the hypervisor via "make -j 4")
> > in dom0 with 4 vcpus shows a significant improvement:
> > 
> > - real time drops from 112 seconds to 103 seconds
> > - system time drops from 142 seconds to 131 seconds
> > 
> > Signed-off-by: Juergen Gross 
> > Reviewed-by: Jan Beulich 
> > ---
> > V7:
> > - add missing flag setting in shadow code
> 
> This now needs an ack from Tim (now Cc-ed).

I may be misunderstanding how this flag is supposed to work, but this
seems at first glance to do both too much and too little.  The sl4
table that's being modified may be in use on any number of other
pcpus, and might not be in use on the current pcpu.

It looks like the do_mmu_update path issues a flush IPI; I think that
the equivalent IPI would be a better place to hook if you can.

Also I'm not sure why the flag needs to be set in
l4e_propagate_from_guest() as well as shadow_set_l4e().  Can you
elaborate?

Cheers,

Tim.

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

Re: [Xen-devel] [PATCH 4/7] xen/arm: When CPU dies, free percpu area immediatelly

[Julien Grall]

On 16/04/18 14:41, Mirela Simonovic wrote:

On Mon, Apr 16, 2018 at 3:14 PM, Julien Grall  wrote:

On 12/04/18 22:31, Stefano Stabellini wrote:

On Thu, 12 Apr 2018, Julien Grall wrote:

On 12/04/18 00:46, Stefano Stabellini wrote:

On Wed, 11 Apr 2018, Julien Grall wrote:

On 11/04/18 14:19, Mirela Simonovic wrote:

I guess the rcu_barrier() in the function handling suspend/resume works. But
that doesn't cover the hotplug case. Looking at x86, suspend/resume case.
For the hotplug case, there are an rcu_barrier in cpu_{up,down}_helper but
they are only present in the case of cpu_{up,down} failed. I am not entirely
sure how this is handled in x86

Andrew, Jan, do you know when the percpu will be free on hotplug? It is call
to call_rcu(...) but I am not sure when this is going to be executed.



AFAIK disable/enable_nonboot_cpus() is the only way to do the hotplug
and rcu_barrier() is not included in the flow.


That's not the only way. I clearly specified one in my previous answer 
(see cpu_{up,down}_helper) and there are other place (look for cpu_up).


Cheers,

--
Julien Grall

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  a6aa678fa380e9369cc44701a181142322b3a4b0
baseline version:
 xen  16fb4b5a9a79f95df17f10ba62e9f44d21cf89b5

Last test of basis   122264  2018-04-13 19:58:45 Z2 days
Testing same since   122331  2018-04-16 13:10:31 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Oleksandr Tyshchenko 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  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 :

To xenbits.xen.org:/home/xen/git/xen.git
   16fb4b5a9a..a6aa678fa3  a6aa678fa380e9369cc44701a181142322b3a4b0 -> smoke

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

[Xen-devel] Xen inside KVM on AMD: Linux HVM/PVH crashes on AP bring up

[Marek Marczykowski-Górecki]

Hi,

I' trying to boot Linux PVH on Xen, which is running inside KVM on AMD
hardware. As soon as secondary CPU is starting, domain crashes.
Strangely, without printing any related messages on the console. The
last message is "x86: Booting SMP configuration:".
This happens for both PVH and HVM with 2 vcpus. PVH/HVM domains with 1
vcpu works fine(*), as well as PV domains with multiple vcpus.

Using gdbsx I've managed to get the point where it crashes:

(gdb) f 12
#12 0x81025101 in do_error_trap (regs=0xc937fe78, 
error_code=-2401053088876204019, 
str=0x40  , trapnr=6, signr=-2)
at arch/x86/kernel/traps.c:302
302 arch/x86/kernel/traps.c: No such file or directory.
(gdb) p/x *regs
$8 = {r15 = 0x0, r14 = 0x0, r13 = 0x0, r12 = 0x0, bp = 0x1, bx = 
0x88007fd0f040, r11 = 0x0, 
  r10 = 0x0, r9 = 0x38, r8 = 0x0, ax = 0xffe4, cx = 0x82251e68, 
dx = 0x0, si = 0x96, 
  di = 0x82, orig_ax = 0x, ip = 0x81036bd3, cs = 
0x10, flags = 0x10086, 
  sp = 0xc937ff20, ss = 0x0}
(gdb) info symbol 0x81036bd3
identify_secondary_cpu + 83 in section .text

It is BUG_ON(c == _cpu_data). If I read it correctly, "c" is 0x82,
which indeed isn't _cpu_data (0x8234fe00).

Any idea?

Version info:
Linux (L0, KVM): 4.4.114-42 (OpenSUSE Leap 42.3)
Xen (L1): 4.8.3
Linux dom0 (L1): 4.14.18
Linux guest: 4.14.18

(*) besides some 20s+ delay on flush_work in deferred_probe_initcall,
before actually calling deferred_probe_work_func.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Julien Grall]

Hi Mirela,

On 16/04/18 15:06, Mirela Simonovic wrote:

On Mon, Apr 16, 2018 at 1:33 PM, Julien Grall  wrote:

On 13/04/18 11:19, Mirela Simonovic wrote:

On Thu, Apr 12, 2018 at 10:43 AM, Julien Grall 

On 11/04/18 17:37, Mirela Simonovic wrote:

On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall > wrote:

So this cover interrupt routed to a virtual CPU. However, this does not
handle interrupts used by Xen. How do you handle them?

For instance SMMUs IRQ might be routed to other interrupt than CPU #0.



Interrupts used by Xen should not wake-up the system and will be
disabled when we suspend the devices used by Xen.


Here you only speak about the suspend use case. While I understand your
ultimate goal is suspend/resume, this series is about CPU hotplug.



AFAIK, the only way and occasion to hotplug a CPU is using > 
disable/enable_nonboot_cpus() within the Xen suspend/resume procedure.


As I mention in a previous e-mail, suspend/resume is not the only way to 
hotplug a CPU. There are an interface (see XEN_SYSCTL_cpu_hotplug) to 
allow that from the userland.


But I agree that it is not implemented on Arm, so suspend/resume would 
be the only way to so far. To be clear, I am not asking you to implement 
XEN_SYSCTL_cpu_hotplug. Althougth it should be easy to do it and easy 
for testing CPU offline/online.



We are implementing CPU hotplug only to enable Xen suspend/resume.

>

This is how it is also done for x86 and we wanted to implement the
equivalent behavior for ARM.
If the cover-letter is misleading please let me know what would be
more appropriate title.


We have multiple places in Xen that could use cpu_up/cpu_down helpers. 
From the cover letter, it is quite unclear that you are only looking 
after suspend/resume.


The SYSCTL interface is one of the other use case. While most of the 
code can be shared, some of them may not make sense.


For instance, in the suspend/resume case routing the interrupt assigned 
to Xen to CPU#0 may not be necessary because all CPUs should come back 
later on. However, this should be necessary when offlining a vCPU with 
SYSCTL interface.


That's why I asked it and I think it should be clarified in the cover 
letter.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 5/5] ALSA: xen-front: Implement ALSA virtual sound driver

[kbuild test robot]

Hi Oleksandr,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sound/for-next]
[also build test ERROR on v4.17-rc1 next-20180416]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Oleksandr-Andrushchenko/ALSA-xen-front-Add-Xen-para-virtualized-frontend-driver/20180416-143123
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   sound/xen/xen_snd_front.c:70:46: sparse: undefined identifier 
'XENSND_OP_HW_PARAM_QUERY'
   sound/xen/xen_snd_front.c:71:16: sparse: no member 'hw_param' in union 

   sound/xen/xen_snd_front.c:105:21: sparse: no member 'period_sz' in struct 
xensnd_open_req
   sound/xen/xen_snd_front.c:201:46: sparse: undefined identifier 
'XENSND_OP_TRIGGER'
   sound/xen/xen_snd_front.c:202:16: sparse: no member 'trigger' in union 

   sound/xen/xen_snd_front.c:70:36: sparse: call with no type!
   sound/xen/xen_snd_front.c:71:16: sparse: generating address of non-lvalue (8)
   sound/xen/xen_snd_front.c:105:21: sparse: generating address of non-lvalue 
(8)
   sound/xen/xen_snd_front.c:201:36: sparse: call with no type!
   In file included from sound/xen/xen_snd_front.c:23:0:
   sound/xen/xen_snd_front_evtchnl.h:62:34: error: field 'hw_param' has 
incomplete type
struct xensnd_query_hw_param hw_param;
 ^~~~
   sound/xen/xen_snd_front.c: In function 'xen_snd_front_stream_query_hw_param':
>> sound/xen/xen_snd_front.c:70:39: error: 'XENSND_OP_HW_PARAM_QUERY' 
>> undeclared (first use in this function); did you mean 'XENSND_OP_WRITE'?
 req = be_stream_prepare_req(evtchnl, XENSND_OP_HW_PARAM_QUERY);
  ^~~~
  XENSND_OP_WRITE
   sound/xen/xen_snd_front.c:70:39: note: each undeclared identifier is 
reported only once for each function it appears in
>> sound/xen/xen_snd_front.c:71:9: error: 'union ' has no member 
>> named 'hw_param'
 req->op.hw_param = *hw_param_req;
^
>> sound/xen/xen_snd_front.c:71:21: error: dereferencing pointer to incomplete 
>> type 'struct xensnd_query_hw_param'
 req->op.hw_param = *hw_param_req;
^
   sound/xen/xen_snd_front.c: In function 'xen_snd_front_stream_prepare':
>> sound/xen/xen_snd_front.c:105:14: error: 'struct xensnd_open_req' has no 
>> member named 'period_sz'
 req->op.open.period_sz = period_sz;
 ^
   sound/xen/xen_snd_front.c: In function 'xen_snd_front_stream_trigger':
>> sound/xen/xen_snd_front.c:201:39: error: 'XENSND_OP_TRIGGER' undeclared 
>> (first use in this function); did you mean 'XENSND_OP_WRITE'?
 req = be_stream_prepare_req(evtchnl, XENSND_OP_TRIGGER);
  ^
  XENSND_OP_WRITE
>> sound/xen/xen_snd_front.c:202:9: error: 'union ' has no member 
>> named 'trigger'
 req->op.trigger.type = type;
^
--
   sound/xen/xen_snd_front_alsa.c:191:47: sparse: restricted snd_pcm_format_t 
degrades to integer
   sound/xen/xen_snd_front_alsa.c:205:59: sparse: restricted snd_pcm_format_t 
degrades to integer
   sound/xen/xen_snd_front_alsa.c:266:12: sparse: using member 'formats' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:269:12: sparse: using member 'rates' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:270:12: sparse: using member 'rates' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:272:12: sparse: using member 'channels' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:273:12: sparse: using member 'channels' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:275:12: sparse: using member 'buffer' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:276:12: sparse: using member 'buffer' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:278:12: sparse: using member 'period' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:279:12: sparse: using member 'period' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:293:50: sparse: using member 'formats' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:306:28: sparse: using member 'rates' in 
incomplete struct xensnd_query_hw_param
   sound/xen/xen_snd_front_alsa.c:307:28: sparse: using member 'rates' in 
incomplete struct xensnd_query_hw_para

Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver

[Oleksandr Andrushchenko]

Hello, all!

After discussing xen-zcopy and hyper-dmabuf [1] approaches

it seems that xen-zcopy can be made not depend on DRM core any more

and be dma-buf centric (which it in fact is).

The DRM code was mostly there for dma-buf's FD import/export

with DRM PRIME UAPI and with DRM use-cases in mind, but it comes out that if

the proposed 2 IOCTLs (DRM_XEN_ZCOPY_DUMB_FROM_REFS and 
DRM_XEN_ZCOPY_DUMB_TO_REFS)


are extended to also provide a file descriptor of the corresponding 
dma-buf, then


PRIME stuff in the driver is not needed anymore.

That being said, xen-zcopy can safely be detached from DRM and moved from

drivers/gpu/drm/xen into drivers/xen/dma-buf-backend(?).

This driver then becomes a universal way to turn any shared buffer 
between Dom0/DomD


and DomU(s) into a dma-buf, e.g. one can create a dma-buf from any grant 
references


or represent a dma-buf as grant-references for export.

This way the driver can be used not only for DRM use-cases, but also for 
other


use-cases which may require zero copying between domains.

For example, the use-cases we are about to work in the nearest future 
will use


V4L, e.g. we plan to support cameras, codecs etc. and all these will benefit

from zero copying much. Potentially, even block/net devices may benefit,

but this needs some evaluation.


I would love to hear comments for authors of the hyper-dmabuf

and Xen community, as well as DRI-Devel and other interested parties.


Thank you,

Oleksandr


On 03/29/2018 04:19 PM, Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

Hello!

When using Xen PV DRM frontend driver then on backend side one will need
to do copying of display buffers' contents (filled by the
frontend's user-space) into buffers allocated at the backend side.
Taking into account the size of display buffers and frames per seconds
it may result in unneeded huge data bus occupation and performance loss.

This helper driver allows implementing zero-copying use-cases
when using Xen para-virtualized frontend display driver by
implementing a DRM/KMS helper driver running on backend's side.
It utilizes PRIME buffers API to share frontend's buffers with
physical device drivers on backend's side:

  - a dumb buffer created on backend's side can be shared
with the Xen PV frontend driver, so it directly writes
into backend's domain memory (into the buffer exported from
DRM/KMS driver of a physical display device)
  - a dumb buffer allocated by the frontend can be imported
into physical device DRM/KMS driver, thus allowing to
achieve no copying as well

For that reason number of IOCTLs are introduced:
  -  DRM_XEN_ZCOPY_DUMB_FROM_REFS
 This will create a DRM dumb buffer from grant references provided
 by the frontend
  - DRM_XEN_ZCOPY_DUMB_TO_REFS
This will grant references to a dumb/display buffer's memory provided
by the backend
  - DRM_XEN_ZCOPY_DUMB_WAIT_FREE
This will block until the dumb buffer with the wait handle provided
be freed

With this helper driver I was able to drop CPU usage from 17% to 3%
on Renesas R-Car M3 board.

This was tested with Renesas' Wayland-KMS and backend running as DRM master.

Thank you,
Oleksandr

Oleksandr Andrushchenko (1):
   drm/xen-zcopy: Add Xen zero-copy helper DRM driver

  Documentation/gpu/drivers.rst   |   1 +
  Documentation/gpu/xen-zcopy.rst |  32 +
  drivers/gpu/drm/xen/Kconfig |  25 +
  drivers/gpu/drm/xen/Makefile|   5 +
  drivers/gpu/drm/xen/xen_drm_zcopy.c | 880 
  drivers/gpu/drm/xen/xen_drm_zcopy_balloon.c | 154 +
  drivers/gpu/drm/xen/xen_drm_zcopy_balloon.h |  38 ++
  include/uapi/drm/xen_zcopy_drm.h| 129 
  8 files changed, 1264 insertions(+)
  create mode 100644 Documentation/gpu/xen-zcopy.rst
  create mode 100644 drivers/gpu/drm/xen/xen_drm_zcopy.c
  create mode 100644 drivers/gpu/drm/xen/xen_drm_zcopy_balloon.c
  create mode 100644 drivers/gpu/drm/xen/xen_drm_zcopy_balloon.h
  create mode 100644 include/uapi/drm/xen_zcopy_drm.h

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2018-02/msg01202.html


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

Re: [Xen-devel] [PATCH 3/7] xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)

[Julien Grall]

Hi Mirela,

On 16/04/18 11:02, Mirela Simonovic wrote:

On Thu, Apr 12, 2018 at 3:31 PM, Julien Grall  wrote:



On 12/04/18 12:33, Mirela Simonovic wrote:


On Wed, Apr 11, 2018 at 4:46 PM, Julien Grall 
wrote:


On 11/04/18 14:19, Mirela Simonovic wrote:


local_irq_disable();
cpu_is_dead = true;
/* Make sure the write happens before we sleep forever */
dsb(sy);
isb();
+/* PSCI cpu off call will return only in case of an error */
+errno = call_psci_cpu_off();
+printk(XENLOG_DEBUG "PSCI cpu off call failed for CPU#%d err=%d\n",
+   get_processor_id(), errno);
+isb();




What are you trying to achieve with the isb() here?



I use to have a problem that the wfi below gets executed before the
call_psci_cpu_off(). Adding isb() fixed the issue. However, I tried
now to reproduce the problem and it doesn't show up. I still believe
isb() should be here, please let me know if you disagree (I obviously
can't prove the claim now).



The problem you describe can't be possible with the code you have because
call_psci_cpu_off() is issuing a SMC. SMC will lead to change exception
level and therefore have a context-synchronization barrier.

This is obviously based on the assumption you don't have an errata on your
CPU exposing the behavior you describe. For that you would need to check
errata notice for your CPU and/or try to reproduce.

However, what you would need is a dsb(sy); isb(); to drain the write buffer
if you print a message.

Furthermore, now on platform without CPU off support (e.g non-PSCI platform
and PSCI 0.1) you will log an error message that may worry people. In
reality, PSCI cpu_off will unlikely fail, so you probably want to add a
panic in call_psci_cpu_off instead.



Even if PSCI cpu_off call fails, what is unlikely to happen, the
system is still functional.


I disagree here, if you are unable to turn off a CPU via PSCI then 
something is definitely wrong. This means that CPU will forever spin in 
Xen code with no way to exit. This could bring interesting issue with 
anything potentially modifying Xen code (i.e livepatching).


IHMO, the forever sleep in stop_cpu() is just a temporary solution to 
cater shutdown of the platform. The state of secondary CPU does not much 
matter at that time. In case of suspend/resume you want really want to 
be able to turn off those CPUs correctly otherwise they are not going to 
come up again.



Enabling that pCPU later will fail, but
Xen can handle this error and continue running properly on the boot
pCPU (I've tested this in 2 pCPUs config).


I don't consider that as xen running properly. You lost a pCPU so your 
workload is completely different. Imagine you are using the NULL 
scheduler (e.g only one vCPU is pinned to a specific pCPU), what are you 
going to do with the vCPU?



Therefore, I believe panic may not be necessary in this case. I
suggest that we dump the error message and continue to run. Please let
me know if you disagree.


This is a bad idea, a failure should at least be logged to show 
something gone wrong.


PSCI CPU off will, as you said, unlikely failed. Looking at the spec, 
the only possible reason is your are trying to turn off a CPU where 
Trusted OS is resident. This means something far more wrong is happening 
in Xen code and I don't think it would be safe to continue to run.


Hence why I suggested a BUG_ON/panic because this is something that is 
not meant to happen.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 5/5] ALSA: xen-front: Implement ALSA virtual sound driver

[Juergen Gross]

On 16/04/18 08:24, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Implement essential initialization of the sound driver:
>   - introduce required data structures
>   - handle driver registration
>   - handle sound card registration
>   - register sound driver on backend connection
>   - remove sound driver on backend disconnect
> 
> Initialize virtual sound card with streams according to the
> Xen store configuration.
> 
> Implement ALSA driver operations including:
> - manage frontend/backend shared buffers
> - manage Xen bus event channel states
> 
> Implement requests from front to back for ALSA
> PCM operations.
>  - report ALSA period elapsed event: handle XENSND_EVT_CUR_POS
>notifications from the backend when stream position advances
>during playback/capture. The event carries a value of how
>many octets were played/captured at the time of the event.
>  - implement explicit stream parameter negotiation between
>backend and frontend: handle XENSND_OP_HW_PARAM_QUERY request
>to read/update configuration space for the parameter given:
>request passes desired parameter interval and the response to
>this request returns min/max interval for the parameter to be used.
> 
> Signed-off-by: Oleksandr Andrushchenko 
> ---
>  sound/xen/Makefile|   3 +-
>  sound/xen/xen_snd_front.c | 193 -
>  sound/xen/xen_snd_front.h |  28 ++
>  sound/xen/xen_snd_front_alsa.c| 830 
> ++
>  sound/xen/xen_snd_front_alsa.h|  23 ++
>  sound/xen/xen_snd_front_evtchnl.c |   6 +-
>  6 files changed, 1080 insertions(+), 3 deletions(-)
>  create mode 100644 sound/xen/xen_snd_front_alsa.c
>  create mode 100644 sound/xen/xen_snd_front_alsa.h
> 
> diff --git a/sound/xen/Makefile b/sound/xen/Makefile
> index f028bc30af5d..1e6470ecc2f2 100644
> --- a/sound/xen/Makefile
> +++ b/sound/xen/Makefile
> @@ -3,6 +3,7 @@
>  snd_xen_front-objs := xen_snd_front.o \
> xen_snd_front_cfg.o \
> xen_snd_front_evtchnl.o \
> -   xen_snd_front_shbuf.o
> +   xen_snd_front_shbuf.o \
> +   xen_snd_front_alsa.o
>  
>  obj-$(CONFIG_SND_XEN_FRONTEND) += snd_xen_front.o
> diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c
> index 0569c6c596a3..1fef253ea21a 100644
> --- a/sound/xen/xen_snd_front.c
> +++ b/sound/xen/xen_snd_front.c
> @@ -19,10 +19,201 @@
>  #include 
>  
>  #include "xen_snd_front.h"
> +#include "xen_snd_front_alsa.h"
>  #include "xen_snd_front_evtchnl.h"
> +#include "xen_snd_front_shbuf.h"
> +
> +static struct xensnd_req *
> +be_stream_prepare_req(struct xen_snd_front_evtchnl *evtchnl, u8 operation)
> +{
> + struct xensnd_req *req;
> +
> + req = RING_GET_REQUEST(>u.req.ring,
> +evtchnl->u.req.ring.req_prod_pvt);
> + req->operation = operation;
> + req->id = evtchnl->evt_next_id++;
> + evtchnl->evt_id = req->id;
> + return req;
> +}
> +
> +static int be_stream_do_io(struct xen_snd_front_evtchnl *evtchnl)
> +{
> + if (unlikely(evtchnl->state != EVTCHNL_STATE_CONNECTED))
> + return -EIO;
> +
> + reinit_completion(>u.req.completion);
> + xen_snd_front_evtchnl_flush(evtchnl);
> + return 0;
> +}
> +
> +static int be_stream_wait_io(struct xen_snd_front_evtchnl *evtchnl)
> +{
> + if (wait_for_completion_timeout(>u.req.completion,
> + msecs_to_jiffies(VSND_WAIT_BACK_MS)) <= 0)
> + return -ETIMEDOUT;
> +
> + return evtchnl->u.req.resp_status;
> +}
> +
> +int xen_snd_front_stream_query_hw_param(struct xen_snd_front_evtchnl 
> *evtchnl,
> + struct xensnd_query_hw_param 
> *hw_param_req,
> + struct xensnd_query_hw_param 
> *hw_param_resp)
> +{
> + struct xen_snd_front_info *front_info = evtchnl->front_info;
> + struct xensnd_req *req;
> + unsigned long flags;
> + int ret;
> +
> + mutex_lock(>u.req.req_io_lock);
> +
> + spin_lock_irqsave(_info->io_lock, flags);
> + req = be_stream_prepare_req(evtchnl, XENSND_OP_HW_PARAM_QUERY);
> + req->op.hw_param = *hw_param_req;
> +
> + ret = be_stream_do_io(evtchnl);
> + spin_unlock_irqrestore(_info->io_lock, flags);
> +
> + if (ret == 0)
> + ret = be_stream_wait_io(evtchnl);
> +
> + if (ret == 0)
> + *hw_param_resp = evtchnl->u.req.resp.hw_param;
> +
> + mutex_unlock(>u.req.req_io_lock);
> + return ret;
> +}
> +
> +int xen_snd_front_stream_prepare(struct xen_snd_front_evtchnl *evtchnl,
> +  struct xen_snd_front_shbuf *sh_buf,
> +  u8 format, unsigned int channels,
> +  unsigned int rate, u32 buffer_sz,
> +  u32 period_sz)
> +{
> + struct 

Re: [Xen-devel] [Qemu-devel] [PATCH 08/12] os-posix: Provide new -runas : facility

[Ian Jackson]

Thanks for the review.  Taking your comments out of order slightly:

Markus Armbruster writes ("Re: [Qemu-devel] [PATCH 08/12] os-posix: Provide new 
-runas : facility"):
> [change_process_uid] is the only user of @user_pwd, @user_uid, @user_gid.
> 
> Have you considered replacing global @user_pwd by @user_uid, @user_gid
> and @user_name?  --runas with numeric uid and gid would leave @user_name
> null.

That would defer the getpwnam from argument parsing to os_setup_post.
I think that's undesriable.

> Ian Jackson  writes:
> >  static struct passwd *user_pwd;
> > +static uid_t user_uid = (uid_t)-1;
> > +static gid_t user_gid = (gid_t)-1;
> 
> As we'll see below, @user_pwd->pw_uid, @user_pwd_pw_gid take precedence
> over @user_uid, @user_gid.  Awkward.

My patch has the right behaviour: each -runas completely overrides the
previous one.  -runas that sets user_{uid,gid} always clears user_pwd
on the way.  So user_pwd can only be set if the most recent -runas was
a name, and then we should honour the name.

This is rather obscure.  I think you are right that this is confusing.
It ought to be clearer.

I will
  - add a comment next to these three variables saying they must
all be set at the same time
  - explicitly (redundantly) clear user_pwd in os_parse_runas_uid_gid
  - explicitly set user_{uid,gid} to -1 when -runas gets a
success from getpwnam
  - assert in change_process_uid that the combination is legal

> > +errno = 0;
> > +rc = qemu_strtoul(optarg, , 0, );
...
> > +lv = 0;
> 
> Either zero lv before both qemu_strtoul() or neither one.

This is a hangover from the previous version which used raw strtoul.
The assignments to both lv and errno are redundant.

> > -if (!user_pwd) {
> > -fprintf(stderr, "User \"%s\" doesn't exist\n", optarg);
> > +if (!user_pwd && !os_parse_runas_uid_gid(optarg)) {
> > +error_report("User doesn't exist (and is not :)");
> 
> The error message no longer includes the offending value.  Intentional?

I was in two minds.  I will put it back.

> > +if (user_pwd || user_uid != (uid_t)-1) {
> > +gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid;
> > +uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid;
> > +if (setgid(intended_gid) < 0) {
> > +fprintf(stderr, "Failed to setgid(%d)\n", intended_gid);
> 
> error_report(), please.  More of the same below.

I was following the existing code in this function.  I'll add a
pre-patch to fix this up here, and maybe a post-patch to fix the rest
of this file.

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Mirela Simonovic]

Hi Julien,

On Mon, Apr 16, 2018 at 1:33 PM, Julien Grall  wrote:
> Hi,
>
>
> On 13/04/18 11:19, Mirela Simonovic wrote:
>>
>> On Thu, Apr 12, 2018 at 10:43 AM, Julien Grall 
>> wrote:
>>>
>>>
>>>
>>> On 11/04/18 17:37, Mirela Simonovic wrote:


 Hi Julien,
>>>
>>>
>>>
>>> Hi,
>>>
>>> May I ask you to configure your mail client to use > for quoting and use
>>> plain text? Otherwise, this is going to be really difficult to follow the
>>> discussion after few round (see already below).
>>>
 On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall > wrote:

  Hi,

  On 11/04/18 16:58, Mirela Simonovic wrote:

  On 04/11/2018 05:07 PM, Julien Grall wrote:

  On 11/04/18 14:19, Mirela Simonovic wrote:

  Migrating interrupts when turning off a CPU already works.
  However, when a CPU is turned back on there is no interrupt
  migration back to the hotplugged CPU - all interrupts will
  remain routed to the CPU#0.
  Patch 7/7 fixes this


  What do you mean by all interrupts? Interrupts routed to guest will
  always follow the vCPU. So are you sure they are going to be
  migrated when that vCPU is paused/off?


 Just to make sure we're on the same page - this is about hotplugging
 physical CPUs. Hotplugging vCPUs using virtual PSCI CPU_OFF interface is
 already implemented and unrelated to this series.
>>>
>>>
>>>
>>> Yes, we are on the same page :). I was just wondering what happen to
>>> interrupt routed to that pCPU.
>>>

 Assuming that system has 2 pCPUs by 'all interrupts' I mean interrupts
 that were targeted to the pCPU#0 and pCPU#1 prior to doing any hotplug.

 For example, if a guest is pinned to pCPU#1 an interrupt of a device it
 owns will be targeted to pCPU#1.
 When pCPU#1 is turned off that interrupt will be migrated to pCPU#0.
 pCPU#0 finalizes the suspend and receives wake-up interrupts. However,
 when
 CPU#1 is turned back on that interrupt will remain targeted to the
 CPU#0,
 which I assumed is wrong.
 The scenario described here is also how I tested this.

  Can you give the path in Xen doing that?


 Sure, here is a backtrace (dumped on the CPU being turned off):
   0  0x2603dc arch_move_irqs(): vgic.c, line 309
   1  0x22ee58 sched_move_irqs()+20: schedule.c, line 303
   2  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
   3  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
   4  0x25aff8 __cpu_disable()+96: smpboot.c, line 386
   5  0x201608 take_cpu_down()+52: cpu.c, line 75
   6  0x23426c stopmachine_action()+188: stop_machine.c, line 159
   7  0x235858 do_tasklet_work()+176: tasklet.c, line 94
   8  0x235c80 do_tasklet()+104: tasklet.c, line 126
   9  0x24daec idle_loop()+144: domain.c, line 72
  10  0x25b1f8 start_secondary()+404: smpboot.c, line 368
>>>
>>>
>>>
>>>
>>> So this cover interrupt routed to a virtual CPU. However, this does not
>>> handle interrupts used by Xen. How do you handle them?
>>>
>>> For instance SMMUs IRQ might be routed to other interrupt than CPU #0.
>>
>>
>> Interrupts used by Xen should not wake-up the system and will be
>> disabled when we suspend the devices used by Xen.
>
> Here you only speak about the suspend use case. While I understand your
> ultimate goal is suspend/resume, this series is about CPU hotplug.
>

AFAIK, the only way and occasion to hotplug a CPU is using
disable/enable_nonboot_cpus() within the Xen suspend/resume procedure.
We are implementing CPU hotplug only to enable Xen suspend/resume.
This is how it is also done for x86 and we wanted to implement the
equivalent behavior for ARM.
If the cover-letter is misleading please let me know what would be
more appropriate title.

However, I absolutely agree that the interrupt routing before and
after the hotplug has to be the same.

> IHMO, the suspend/resume case is no more than a superset of CPU up/down. If
> you solve the problem for up/down, likely you are going to solve it for
> suspend/resume.
>
> So, what would happen to interrupts routed to the CPU going offline?
>
>> However, I need to double check that such interrupts get enabled on
>> the right CPU on resume. Could you please tell me which mechanism in
>> Xen is used to target such an interrupt to a secondary CPU only? Is
>> that even possible and why would that be used?
>
>
> SPIs will be routed to the CPU calling setup_irq. It may not always be
> CPU#0. For instance, this is the case context interrupt for the SMMU because
> they are setup when the device is assigned.
>
> I guess this decision is arguable. If you move all the interrupts to CPU#0
> it will potentially 

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

[Platform Team regression test user]

This run is configured for baseline tests only.

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 665bfd41ac32b364201c07dc1c5434432730c034
baseline version:
 ovmf b85b20fba42e25ff658ed1a470250d530c189027

Last test of basis74623  2018-04-15 16:28:19 Z0 days
Testing same since74626  2018-04-16 11:51:59 Z0 days1 attempts


People who touched revisions under test:
  Star Zeng 

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



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

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

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


Push not applicable.


commit 665bfd41ac32b364201c07dc1c5434432730c034
Author: Star Zeng 
Date:   Fri Apr 13 17:55:14 2018 +0800

SignedCapsulePkg SystemFirmwareUpdateDxe: Fix failure caused by d69d922

d69d9227d046211265de1fab5580c50a65944614 caused system firmware update
failure. It is because FindMatchingFmpHandles() is expected to return
handles matched, but the function returns all handles found.

This patch is to fix the issue.
This patch also assigns mSystemFmpPrivate->Handle for "case 1:" path
in case the Handle is needed by other place in future.

Cc: Michael D Kinney 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Jiewen Yao 
Reviewed-by: Michael D Kinney 

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

Re: [Xen-devel] [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling

[kbuild test robot]

Hi Oleksandr,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sound/for-next]
[also build test ERROR on v4.17-rc1 next-20180416]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Oleksandr-Andrushchenko/ALSA-xen-front-Add-Xen-para-virtualized-frontend-driver/20180416-143123
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from sound/xen/xen_snd_front.c:21:0:
>> sound/xen/xen_snd_front_evtchnl.h:62:34: error: field 'hw_param' has 
>> incomplete type
struct xensnd_query_hw_param hw_param;
 ^~~~
--
   sound/xen/xen_snd_front_evtchnl.c:51:22: sparse: undefined identifier 
'XENSND_OP_TRIGGER'
   sound/xen/xen_snd_front_evtchnl.c:55:22: sparse: undefined identifier 
'XENSND_OP_HW_PARAM_QUERY'
   sound/xen/xen_snd_front_evtchnl.c:58:45: sparse: no member 'resp' in struct 
xensnd_resp
   sound/xen/xen_snd_front_evtchnl.c:51:22: sparse: incompatible types for 
'case' statement
   sound/xen/xen_snd_front_evtchnl.c:55:22: sparse: incompatible types for 
'case' statement
   sound/xen/xen_snd_front_evtchnl.c:99:20: sparse: using member 'in_prod' in 
incomplete struct xensnd_event_page
   sound/xen/xen_snd_front_evtchnl.c:102:25: sparse: using member 'in_cons' in 
incomplete struct xensnd_event_page
   sound/xen/xen_snd_front_evtchnl.c:105:25: sparse: using member 'in_cons' in 
incomplete struct xensnd_event_page
   sound/xen/xen_snd_front_evtchnl.c:108:26: sparse: undefined identifier 
'XENSND_IN_RING_REF'
   sound/xen/xen_snd_front_evtchnl.c:109:21: sparse: using member 'id' in 
incomplete struct xensnd_evt
   sound/xen/xen_snd_front_evtchnl.c:112:30: sparse: using member 'type' in 
incomplete struct xensnd_evt
   sound/xen/xen_snd_front_evtchnl.c:113:22: sparse: undefined identifier 
'XENSND_EVT_CUR_POS'
   sound/xen/xen_snd_front_evtchnl.c:113:22: sparse: incompatible types for 
'case' statement
   sound/xen/xen_snd_front_evtchnl.c:119:13: sparse: using member 'in_cons' in 
incomplete struct xensnd_event_page
   sound/xen/xen_snd_front_evtchnl.c:213:34: sparse: undefined identifier 
'XENSND_FIELD_EVT_RING_REF'
   sound/xen/xen_snd_front_evtchnl.c:412:47: sparse: undefined identifier 
'XENSND_FIELD_EVT_RING_REF'
   sound/xen/xen_snd_front_evtchnl.c:432:47: sparse: undefined identifier 
'XENSND_FIELD_EVT_RING_REF'
   sound/xen/xen_snd_front_evtchnl.c:51:22: sparse: Expected constant 
expression in case statement
   sound/xen/xen_snd_front_evtchnl.c:55:22: sparse: Expected constant 
expression in case statement
   sound/xen/xen_snd_front_evtchnl.c:113:22: sparse: Expected constant 
expression in case statement
   In file included from sound/xen/xen_snd_front_evtchnl.c:18:0:
>> sound/xen/xen_snd_front_evtchnl.h:62:34: error: field 'hw_param' has 
>> incomplete type
struct xensnd_query_hw_param hw_param;
 ^~~~
   sound/xen/xen_snd_front_evtchnl.c: In function 'evtchnl_interrupt_req':
>> sound/xen/xen_snd_front_evtchnl.c:51:8: error: 'XENSND_OP_TRIGGER' 
>> undeclared (first use in this function); did you mean 'XENSND_OP_WRITE'?
  case XENSND_OP_TRIGGER:
   ^
   XENSND_OP_WRITE
   sound/xen/xen_snd_front_evtchnl.c:51:8: note: each undeclared identifier is 
reported only once for each function it appears in
>> sound/xen/xen_snd_front_evtchnl.c:55:8: error: 'XENSND_OP_HW_PARAM_QUERY' 
>> undeclared (first use in this function); did you mean 'XENSND_OP_TRIGGER'?
  case XENSND_OP_HW_PARAM_QUERY:
   ^~~~
   XENSND_OP_TRIGGER
>> sound/xen/xen_snd_front_evtchnl.c:58:10: error: 'struct xensnd_resp' has no 
>> member named 'resp'
 resp->resp.hw_param;
 ^~
   sound/xen/xen_snd_front_evtchnl.c: In function 'evtchnl_interrupt_evt':
>> sound/xen/xen_snd_front_evtchnl.c:99:13: error: dereferencing pointer to 
>> incomplete type 'struct xensnd_event_page'
 prod = page->in_prod;
^~
>> sound/xen/xen_snd_front_evtchnl.c:108:12: error: implicit declaration of 
>> function 'XENSND_IN_RING_REF'; did you mean 'XENSND_FIELD_RING_REF'? 
>> [-Werror=implicit-function-declaration]
  event = _IN_RING_REF(page, cons);
   ^~
   XENSND_FIELD_RING_REF
>> sound/xen/xen_snd_front_evtchnl.c:108:11: error: lvalue required as unary 
>> '&' operand
  event = _IN_RING_REF(page, cons);
  ^
   In file included from include/linux/kernel.h:10:0,
   

Re: [Xen-devel] [PATCH 4/7] xen/arm: When CPU dies, free percpu area immediatelly

[Mirela Simonovic]

Hi Julien, Stefano,

Thanks for the feedback and suggestions.

On Mon, Apr 16, 2018 at 3:14 PM, Julien Grall  wrote:
> Hi,
>
>
> On 12/04/18 22:31, Stefano Stabellini wrote:
>>
>> On Thu, 12 Apr 2018, Julien Grall wrote:
>>>
>>> On 12/04/18 00:46, Stefano Stabellini wrote:

 On Wed, 11 Apr 2018, Julien Grall wrote:
>
> On 11/04/18 14:19, Mirela Simonovic wrote:
>>
>> Freeing percpu area is done when a non-boot CPU is disabled upon
>> suspend.
>> This use to be scheduled for execution after a period of time, what
>> caused
>> the following racing issues. If CPU is enabled after it is disabled
>> and
>> before the freeing of percpu area is performed, Xen would crash upon
>> initializing percpu area because per cpu offset is not marked as
>> INVALID_PERCPU_AREA (this suppose to happen when cpu area is freed).
>> To resolve the racing issue, free percpu area right away instead
>> scheduling it for later.
>
>
> The reason of using the RCU is you want to make sure that none of the
> other
> CPUs will access that percpu data before freeing it. So I don't think
> this
> patch is valid.
>
> It looks like to me a rcu barrier is missing after calling cpu_down
> somewhere
> in the CPU off path. I am not entirely sure where.


 We need a rcu_barrier(). Perhaps, it could be added on cpu_on before
 initializing the percpu area?
>>>
>>>
>>> Do you mind giving a bit more details on your thought? cpu_up looks a
>>> strange
>>> place as no one should access the percpu area after the CPU is down. So
>>> it
>>> feels the rcu_barrier should be somewhere before PSCI_cpu_off is called.
>>
>>
>> Yes, it feels strange to do it on cpu_on, it would be more obvious on
>> cpu_off, but we don't actually need to _free_percpu_area on cpu_off,
>> right? We only need to make sure it is done before cpu_percpu_callback
>> is called on cpu_on.
>>
>> My suggestion would be to evaluate if it is possible to introduce the
>> rcu_barrier() on the resume path before cpu_percpu_callback, maybe in
>> start_secondary.
>
>
> Well, cpu_percpu_callback is not called by start_secondary. It is called
> when preparing the CPU from another CPU. So anything in start_secondary will
> not work.
>

I have also confirmed that in start_secondary it doesn't work, it's too late.

>>
>> I was also looking at xen/arch/x86/acpi/power.c:enter_state and noticed
>> that they chose to call rcu_barrier() on enable_cpu before
>> enable_nonboot_cpus().
>

Before the enable_nonboot_cpus() gets invoked seems to be a good place
for rcu_barrier(), as it's done for x86.

>
> I guess the rcu_barrier() in the function handling suspend/resume works. But
> that doesn't cover the hotplug case. Looking at x86, suspend/resume case.
> For the hotplug case, there are an rcu_barrier in cpu_{up,down}_helper but
> they are only present in the case of cpu_{up,down} failed. I am not entirely
> sure how this is handled in x86
>
> Andrew, Jan, do you know when the percpu will be free on hotplug? It is call
> to call_rcu(...) but I am not sure when this is going to be executed.
>

AFAIK disable/enable_nonboot_cpus() is the only way to do the hotplug
and rcu_barrier() is not included in the flow.
I suggest to invoke rcu_barrier() before enable_nonboot_cpus() and
eventually this could be moved into enable_nonboot_cpus() itself.

Thanks,
Mirela

> Cheers,
>
> --
> Julien Grall

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

Re: [Xen-devel] [PATCH v2 4/5] ALSA: xen-front: Implement handling of shared buffers

[Juergen Gross]

On 16/04/18 08:24, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Implement shared buffer handling according to the
> para-virtualized sound device protocol at xen/interface/io/sndif.h:
>   - manage buffer memory
>   - handle granted references
>   - handle page directories
> 
> Signed-off-by: Oleksandr Andrushchenko 
> ---
>  sound/xen/Makefile  |   3 +-
>  sound/xen/xen_snd_front.c   |   8 ++
>  sound/xen/xen_snd_front_shbuf.c | 193 
> 
>  sound/xen/xen_snd_front_shbuf.h |  36 
>  4 files changed, 239 insertions(+), 1 deletion(-)
>  create mode 100644 sound/xen/xen_snd_front_shbuf.c
>  create mode 100644 sound/xen/xen_snd_front_shbuf.h
> 
> diff --git a/sound/xen/Makefile b/sound/xen/Makefile
> index 03c669984000..f028bc30af5d 100644
> --- a/sound/xen/Makefile
> +++ b/sound/xen/Makefile
> @@ -2,6 +2,7 @@
>  
>  snd_xen_front-objs := xen_snd_front.o \
> xen_snd_front_cfg.o \
> -   xen_snd_front_evtchnl.o
> +   xen_snd_front_evtchnl.o \
> +   xen_snd_front_shbuf.o
>  
>  obj-$(CONFIG_SND_XEN_FRONTEND) += snd_xen_front.o
> diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c
> index eb46bf4070f9..0569c6c596a3 100644
> --- a/sound/xen/xen_snd_front.c
> +++ b/sound/xen/xen_snd_front.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -186,6 +187,13 @@ static struct xenbus_driver xen_driver = {
>  
>  static int __init xen_drv_init(void)
>  {
> + /* At the moment we only support case with XEN_PAGE_SIZE == PAGE_SIZE */
> + if (XEN_PAGE_SIZE != PAGE_SIZE) {
> + pr_err(XENSND_DRIVER_NAME ": different kernel and Xen page 
> sizes are not supported: XEN_PAGE_SIZE (%lu) != PAGE_SIZE (%lu)\n",
> +XEN_PAGE_SIZE, PAGE_SIZE);
> + return -ENODEV;
> + }

Do you really want to print that error message on bare metal?

> +
>   if (!xen_domain())
>   return -ENODEV;
>  
> diff --git a/sound/xen/xen_snd_front_shbuf.c b/sound/xen/xen_snd_front_shbuf.c
> new file mode 100644
> index ..6845dbc7fdf5
> --- /dev/null
> +++ b/sound/xen/xen_snd_front_shbuf.c
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +/*
> + * Xen para-virtual sound device
> + *
> + * Copyright (C) 2016-2018 EPAM Systems Inc.
> + *
> + * Author: Oleksandr Andrushchenko 
> + */
> +
> +#include 
> +#include 
> +
> +#include "xen_snd_front_shbuf.h"
> +
> +grant_ref_t xen_snd_front_shbuf_get_dir_start(struct xen_snd_front_shbuf 
> *buf)
> +{
> + if (!buf->grefs)
> + return GRANT_INVALID_REF;
> +
> + return buf->grefs[0];
> +}
> +
> +void xen_snd_front_shbuf_clear(struct xen_snd_front_shbuf *buf)
> +{
> + memset(buf, 0, sizeof(*buf));
> +}
> +
> +void xen_snd_front_shbuf_free(struct xen_snd_front_shbuf *buf)
> +{
> + int i;
> +
> + if (buf->grefs) {
> + for (i = 0; i < buf->num_grefs; i++)
> + if (buf->grefs[i] != GRANT_INVALID_REF)
> + gnttab_end_foreign_access(buf->grefs[i],
> +   0, 0UL);
> + kfree(buf->grefs);
> + }
> + kfree(buf->directory);
> + free_pages_exact(buf->buffer, buf->buffer_sz);
> + xen_snd_front_shbuf_clear(buf);
> +}
> +
> +/*
> + * number of grant references a page can hold with respect to the
> + * xensnd_page_directory header
> + */
> +#define XENSND_NUM_GREFS_PER_PAGE ((XEN_PAGE_SIZE - \
> + offsetof(struct xensnd_page_directory, gref)) / \
> + sizeof(grant_ref_t))
> +
> +static void fill_page_dir(struct xen_snd_front_shbuf *buf,
> +   int num_pages_dir)
> +{
> + struct xensnd_page_directory *page_dir;
> + unsigned char *ptr;
> + int i, cur_gref, grefs_left, to_copy;
> +
> + ptr = buf->directory;
> + grefs_left = buf->num_grefs - num_pages_dir;
> + /*
> +  * skip grant references at the beginning, they are for pages granted
> +  * for the page directory itself
> +  */
> + cur_gref = num_pages_dir;
> + for (i = 0; i < num_pages_dir; i++) {
> + page_dir = (struct xensnd_page_directory *)ptr;
> + if (grefs_left <= XENSND_NUM_GREFS_PER_PAGE) {
> + to_copy = grefs_left;
> + page_dir->gref_dir_next_page = GRANT_INVALID_REF;
> + } else {
> + to_copy = XENSND_NUM_GREFS_PER_PAGE;
> + page_dir->gref_dir_next_page = buf->grefs[i + 1];
> + }
> +
> + memcpy(_dir->gref, >grefs[cur_gref],
> +to_copy * sizeof(grant_ref_t));
> +
> + ptr += XEN_PAGE_SIZE;
> + grefs_left -= to_copy;
> + 

[Xen-devel] [xen-unstable test] 122322: FAIL

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-win7-amd64   broken in 122315

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemut-win7-amd64 4 host-install(4) broken in 122315 pass in 
122322
 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail pass in 
122315

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop  fail in 122315 like 122294
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 122294
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 122315
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 122315
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 122315
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 122315
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 122315
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 122315
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 122315
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 122315
 test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 xen  16fb4b5a9a79f95df17f10ba62e9f44d21cf89b5
baseline version:
 xen  

Re: [Xen-devel] [PATCH] tools/kdd: silence gcc 8 warning a different way

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH] tools/kdd: silence gcc 8 warning a different 
way"):
> And I'm not overly happy about it, but couldn't think of a better way
> without disabling said warning (or -Werror) altogether for the CU. If
> Ian's sketched out approach worked, I'd be quite happy to drop the
> patch here.

I did not sketch out an approach.  Implicit, though, is the suggestion
that the memcpy should be made conditional.

Ian.

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

Re: [Xen-devel] [PATCH] x86/xen: Remove use of VLAs

[Boris Ostrovsky]

On 04/13/2018 06:11 PM, Laura Abbott wrote:
> There's an ongoing effort to remove VLAs[1] from the kernel to eventually
> turn on -Wvla. The few VLAs in use have an upper bound based on a size
> of 64K. This doesn't produce an excessively large stack so just switch
> the upper bound.
>
> [1] https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Laura Abbott 
> ---
>  arch/x86/xen/enlighten_pv.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index c36d23aa6c35..d96a5a535cbb 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -421,8 +421,7 @@ static void xen_load_gdt(const struct desc_ptr *dtr)
>  {
>   unsigned long va = dtr->address;
>   unsigned int size = dtr->size + 1;
> - unsigned pages = DIV_ROUND_UP(size, PAGE_SIZE);



Isn't dtr->size always either GDT_SIZE or 0?

-boris




> - unsigned long frames[pages];
> + unsigned long frames[DIV_ROUND_UP(SZ_64K, PAGE_SIZE)];
>   int f;
>  
>   /*
> @@ -470,8 +469,7 @@ static void __init xen_load_gdt_boot(const struct 
> desc_ptr *dtr)
>  {
>   unsigned long va = dtr->address;
>   unsigned int size = dtr->size + 1;
> - unsigned pages = DIV_ROUND_UP(size, PAGE_SIZE);
> - unsigned long frames[pages];
> + unsigned long frames[DIV_ROUND_UP(SZ_64K, PAGE_SIZE)];
>   int f;
>  
>   /*


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

Re: [Xen-devel] [PATCH v4] x86/setup: disallow any src/dst overlaps when relocating Xen image

[Jan Beulich]

>>> On 03.04.18 at 17:54,  wrote:
> Commit 0d31d16 (x86/setup: do not relocate Xen over current Xen image
> placement) disallowed src/dst images overlaps when relocating Xen image.
> Though it deliberately allowed destination region between __image_base__
> and (__image_base__ + XEN_IMG_OFFSET) overlaps with the end of source
> image. And here is the problem. If anything between __page_tables_start
> and __page_tables_end in source image lands in the overlap then some or
> even all page table entries may not be updated. This usually means boom
> in early boot which will be difficult to the investigate. So, I think
> that we have three choices to fix the issue:
>   - drop XEN_IMG_OFFSET from
> if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= __pa(_end)) )
>   - add XEN_IMG_OFFSET to xen_phys_start in PFN_DOWN(xen_phys_start)
> used in loops as one of conditions,
>   - change PFN_DOWN(xen_phys_start) to PFN_DOWN(xen_remap_end_pfn)
> proposed in earlier version of this patch.
> 
> This patch implements the first option. This way we will avoid all kinds
> of overlaps which are always full can of worms.

Personally I'd like option 2 better, as there's nothing of interest in the
[0,XEN_IMG_OFFSET) range. Instead of modifying every
PFN_DOWN(xen_phys_start), perhaps simply introduce a local variable,
accompanied by a suitable comment.

Jan



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

Re: [Xen-devel] [RFC PATCH 1/3] xen: Introduce vcpu_sleep_nosync_locked()

[Dario Faggioli]

On Wed, 2018-04-11 at 13:25 +0100, George Dunlap wrote:
> There are a lot of places which release a lock before calling
> vcpu_sleep_nosync(), which then just grabs the lock again.  This is
> not only a waste of time, but leads to more code duplication (since
> you have to copy-and-paste recipes rather than calling a unified
> function), which in turn leads to an increased chance of bugs.
> 
> Introduce vcpu_sleep_nosync_locked(), which can be called if you
> already hold the schedule lock.
> 
> Signed-off-by: George Dunlap 
>
Reviewed-by: Dario Faggioli 

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 4/7] xen/arm: When CPU dies, free percpu area immediatelly

[Julien Grall]

Hi,

On 12/04/18 22:31, Stefano Stabellini wrote:

On Thu, 12 Apr 2018, Julien Grall wrote:

On 12/04/18 00:46, Stefano Stabellini wrote:

On Wed, 11 Apr 2018, Julien Grall wrote:

On 11/04/18 14:19, Mirela Simonovic wrote:

Freeing percpu area is done when a non-boot CPU is disabled upon
suspend.
This use to be scheduled for execution after a period of time, what
caused
the following racing issues. If CPU is enabled after it is disabled and
before the freeing of percpu area is performed, Xen would crash upon
initializing percpu area because per cpu offset is not marked as
INVALID_PERCPU_AREA (this suppose to happen when cpu area is freed).
To resolve the racing issue, free percpu area right away instead
scheduling it for later.


The reason of using the RCU is you want to make sure that none of the
other
CPUs will access that percpu data before freeing it. So I don't think this
patch is valid.

It looks like to me a rcu barrier is missing after calling cpu_down
somewhere
in the CPU off path. I am not entirely sure where.


We need a rcu_barrier(). Perhaps, it could be added on cpu_on before
initializing the percpu area?


Do you mind giving a bit more details on your thought? cpu_up looks a strange
place as no one should access the percpu area after the CPU is down. So it
feels the rcu_barrier should be somewhere before PSCI_cpu_off is called.


Yes, it feels strange to do it on cpu_on, it would be more obvious on
cpu_off, but we don't actually need to _free_percpu_area on cpu_off,
right? We only need to make sure it is done before cpu_percpu_callback
is called on cpu_on.

My suggestion would be to evaluate if it is possible to introduce the
rcu_barrier() on the resume path before cpu_percpu_callback, maybe in
start_secondary.


Well, cpu_percpu_callback is not called by start_secondary. It is called 
when preparing the CPU from another CPU. So anything in start_secondary 
will not work.




I was also looking at xen/arch/x86/acpi/power.c:enter_state and noticed
that they chose to call rcu_barrier() on enable_cpu before
enable_nonboot_cpus().


I guess the rcu_barrier() in the function handling suspend/resume works. 
But that doesn't cover the hotplug case. Looking at x86, suspend/resume 
case. For the hotplug case, there are an rcu_barrier in 
cpu_{up,down}_helper but they are only present in the case of 
cpu_{up,down} failed. I am not entirely sure how this is handled in x86


Andrew, Jan, do you know when the percpu will be free on hotplug? It is 
call to call_rcu(...) but I am not sure when this is going to be executed.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling

[Juergen Gross]

On 16/04/18 08:24, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Handle Xen event channels:
>   - create for all configured streams and publish
> corresponding ring references and event channels in Xen store,
> so backend can connect
>   - implement event channels interrupt handlers
>   - create and destroy event channels with respect to Xen bus state
> 
> Signed-off-by: Oleksandr Andrushchenko 
> ---
>  sound/xen/Makefile|   3 +-
>  sound/xen/xen_snd_front.c |  10 +-
>  sound/xen/xen_snd_front.h |   7 +
>  sound/xen/xen_snd_front_evtchnl.c | 474 
> ++
>  sound/xen/xen_snd_front_evtchnl.h |  92 
>  5 files changed, 584 insertions(+), 2 deletions(-)
>  create mode 100644 sound/xen/xen_snd_front_evtchnl.c
>  create mode 100644 sound/xen/xen_snd_front_evtchnl.h
> 
> diff --git a/sound/xen/Makefile b/sound/xen/Makefile
> index 06705bef61fa..03c669984000 100644
> --- a/sound/xen/Makefile
> +++ b/sound/xen/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0 OR MIT
>  
>  snd_xen_front-objs := xen_snd_front.o \
> -   xen_snd_front_cfg.o
> +   xen_snd_front_cfg.o \
> +   xen_snd_front_evtchnl.o
>  
>  obj-$(CONFIG_SND_XEN_FRONTEND) += snd_xen_front.o
> diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c
> index 65d2494a9d14..eb46bf4070f9 100644
> --- a/sound/xen/xen_snd_front.c
> +++ b/sound/xen/xen_snd_front.c
> @@ -18,9 +18,11 @@
>  #include 
>  
>  #include "xen_snd_front.h"
> +#include "xen_snd_front_evtchnl.h"

Does it really make sense to have multiple driver-private headers?

I think those can be merged.

>  
>  static void xen_snd_drv_fini(struct xen_snd_front_info *front_info)
>  {
> + xen_snd_front_evtchnl_free_all(front_info);
>  }
>  
>  static int sndback_initwait(struct xen_snd_front_info *front_info)
> @@ -32,7 +34,12 @@ static int sndback_initwait(struct xen_snd_front_info 
> *front_info)
>   if (ret < 0)
>   return ret;
>  
> - return 0;
> + /* create event channels for all streams and publish */
> + ret = xen_snd_front_evtchnl_create_all(front_info, num_streams);
> + if (ret < 0)
> + return ret;
> +
> + return xen_snd_front_evtchnl_publish_all(front_info);
>  }
>  
>  static int sndback_connect(struct xen_snd_front_info *front_info)
> @@ -122,6 +129,7 @@ static int xen_drv_probe(struct xenbus_device *xb_dev,
>   return -ENOMEM;
>  
>   front_info->xb_dev = xb_dev;
> + spin_lock_init(_info->io_lock);
>   dev_set_drvdata(_dev->dev, front_info);
>  
>   return xenbus_switch_state(xb_dev, XenbusStateInitialising);
> diff --git a/sound/xen/xen_snd_front.h b/sound/xen/xen_snd_front.h
> index b52226cb30bc..9c2ffbb4e4b8 100644
> --- a/sound/xen/xen_snd_front.h
> +++ b/sound/xen/xen_snd_front.h
> @@ -13,9 +13,16 @@
>  
>  #include "xen_snd_front_cfg.h"
>  
> +struct xen_snd_front_evtchnl_pair;
> +
>  struct xen_snd_front_info {
>   struct xenbus_device *xb_dev;
>  
> + /* serializer for backend IO: request/response */
> + spinlock_t io_lock;
> + int num_evt_pairs;
> + struct xen_snd_front_evtchnl_pair *evt_pairs;
> +
>   struct xen_front_cfg_card cfg;
>  };
>  
> diff --git a/sound/xen/xen_snd_front_evtchnl.c 
> b/sound/xen/xen_snd_front_evtchnl.c
> new file mode 100644
> index ..9ece39f938f8
> --- /dev/null
> +++ b/sound/xen/xen_snd_front_evtchnl.c
> @@ -0,0 +1,474 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +/*
> + * Xen para-virtual sound device
> + *
> + * Copyright (C) 2016-2018 EPAM Systems Inc.
> + *
> + * Author: Oleksandr Andrushchenko 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "xen_snd_front.h"
> +#include "xen_snd_front_cfg.h"
> +#include "xen_snd_front_evtchnl.h"
> +
> +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
> +{
> + struct xen_snd_front_evtchnl *channel = dev_id;
> + struct xen_snd_front_info *front_info = channel->front_info;
> + struct xensnd_resp *resp;
> + RING_IDX i, rp;
> + unsigned long flags;
> +
> + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(_info->io_lock, flags);
> +
> +again:
> + rp = channel->u.req.ring.sring->rsp_prod;
> + /* ensure we see queued responses up to rp */
> + rmb();
> +
> + for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
> + resp = RING_GET_RESPONSE(>u.req.ring, i);
> + if (resp->id != channel->evt_id)
> + continue;
> + switch (resp->operation) {
> + case XENSND_OP_OPEN:
> + /* fall through */
> + case XENSND_OP_CLOSE:
> + /* fall through */
> + 

Re: [Xen-devel] [PATCH 0/3] x86: S3 resume adjustments

[Jan Beulich]

>>> On 15.04.18 at 22:15,  wrote:
> (XEN) *** DOUBLE FAULT ***
> (XEN) [ Xen-4.11-unstable  x86_64  debug=y   Not tainted ]
> (XEN) CPU:0
> (XEN) RIP:e008:[] search_pre_exception_table+0/0x54
> (XEN) RFLAGS: 00010046   CONTEXT: hypervisor
> (XEN) rax:    rbx:    rcx: 
> (XEN) rdx:    rsi:    rdi: c90040cd4028
> (XEN) rbp: 36ffbf32bfb7   rsp: c90040cd4020   r8:  
> (XEN) r9:     r10:    r11: 
> (XEN) r12:    r13:    r14: c90040cd7fff
> (XEN) r15:    cr0: 8005003b   cr4: 000426e0
> (XEN) cr3: 00022200a000   cr2: c90040cd3ff8
> (XEN) fsb: 7fd74515e740   gsb: 88021e6c   gss: 
> (XEN) ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
> (XEN) Current stack base c90040cd differs from expected 
> 8300cec88000
> (XEN) Valid stack range: c90040cd6000-c90040cd8000, 
> sp=c90040cd4020, tss.rsp0=8300cec8ffa0

The fact that the exact location varies where the #DF triggers is of no big
interest - it all depends on when exactly the stack overflow occurs. What
I note though: c90040cd4020 is a guest (presumably Dom0) kernel
address, far outside the Xen range. I guess we'd need to see all of that
(wrong) stack's contents logged up to the original entry into Xen to
understand how that could have happened.

Jan



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

Re: [Xen-devel] [PATCH v2 2/5] ALSA: xen-front: Read sound driver configuration from Xen store

[Juergen Gross]

On 16/04/18 08:24, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Read configuration values from Xen store according
> to xen/interface/io/sndif.h protocol:
>   - introduce configuration structures for different
> components, e.g. sound card, device, stream
>   - read PCM HW parameters, e.g rate, format etc.
>   - detect stream type (capture/playback)
>   - read device and card parameters
> 
> Signed-off-by: Oleksandr Andrushchenko 
> ---
>  sound/xen/Makefile|   3 +-
>  sound/xen/xen_snd_front.c |   7 +
>  sound/xen/xen_snd_front.h |   4 +
>  sound/xen/xen_snd_front_cfg.c | 517 
> ++
>  sound/xen/xen_snd_front_cfg.h |  46 
>  5 files changed, 576 insertions(+), 1 deletion(-)
>  create mode 100644 sound/xen/xen_snd_front_cfg.c
>  create mode 100644 sound/xen/xen_snd_front_cfg.h
> 
> diff --git a/sound/xen/Makefile b/sound/xen/Makefile
> index 4507ef3c27fd..06705bef61fa 100644
> --- a/sound/xen/Makefile
> +++ b/sound/xen/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0 OR MIT
>  
> -snd_xen_front-objs := xen_snd_front.o
> +snd_xen_front-objs := xen_snd_front.o \
> +   xen_snd_front_cfg.o
>  
>  obj-$(CONFIG_SND_XEN_FRONTEND) += snd_xen_front.o
> diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c
> index f406a8f52c51..65d2494a9d14 100644
> --- a/sound/xen/xen_snd_front.c
> +++ b/sound/xen/xen_snd_front.c
> @@ -25,6 +25,13 @@ static void xen_snd_drv_fini(struct xen_snd_front_info 
> *front_info)
>  
>  static int sndback_initwait(struct xen_snd_front_info *front_info)
>  {
> + int num_streams;
> + int ret;
> +
> + ret = xen_snd_front_cfg_card(front_info, _streams);
> + if (ret < 0)
> + return ret;
> +
>   return 0;
>  }
>  
> diff --git a/sound/xen/xen_snd_front.h b/sound/xen/xen_snd_front.h
> index 4ae204b23d32..b52226cb30bc 100644
> --- a/sound/xen/xen_snd_front.h
> +++ b/sound/xen/xen_snd_front.h
> @@ -11,8 +11,12 @@
>  #ifndef __XEN_SND_FRONT_H
>  #define __XEN_SND_FRONT_H
>  
> +#include "xen_snd_front_cfg.h"
> +
>  struct xen_snd_front_info {
>   struct xenbus_device *xb_dev;
> +
> + struct xen_front_cfg_card cfg;
>  };
>  
>  #endif /* __XEN_SND_FRONT_H */
> diff --git a/sound/xen/xen_snd_front_cfg.c b/sound/xen/xen_snd_front_cfg.c
> new file mode 100644
> index ..d461985afffa
> --- /dev/null
> +++ b/sound/xen/xen_snd_front_cfg.c
> @@ -0,0 +1,517 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +/*
> + * Xen para-virtual sound device
> + *
> + * Copyright (C) 2016-2018 EPAM Systems Inc.
> + *
> + * Author: Oleksandr Andrushchenko 
> + */
> +
> +#include 
> +
> +#include 
> +
> +#include "xen_snd_front.h"
> +#include "xen_snd_front_cfg.h"
> +
> +/* maximum number of supported streams */

Comment style (multiple times below, too):
Start with a capital letter and end sentences with a full stop.

> +#define VSND_MAX_STREAM  8
> +
> +struct cfg_hw_sample_rate {
> + const char *name;
> + unsigned int mask;
> + unsigned int value;
> +};
> +
> +static const struct cfg_hw_sample_rate CFG_HW_SUPPORTED_RATES[] = {
> + { .name = "5512",   .mask = SNDRV_PCM_RATE_5512,   .value = 5512 },
> + { .name = "8000",   .mask = SNDRV_PCM_RATE_8000,   .value = 8000 },
> + { .name = "11025",  .mask = SNDRV_PCM_RATE_11025,  .value = 11025 },
> + { .name = "16000",  .mask = SNDRV_PCM_RATE_16000,  .value = 16000 },
> + { .name = "22050",  .mask = SNDRV_PCM_RATE_22050,  .value = 22050 },
> + { .name = "32000",  .mask = SNDRV_PCM_RATE_32000,  .value = 32000 },
> + { .name = "44100",  .mask = SNDRV_PCM_RATE_44100,  .value = 44100 },
> + { .name = "48000",  .mask = SNDRV_PCM_RATE_48000,  .value = 48000 },
> + { .name = "64000",  .mask = SNDRV_PCM_RATE_64000,  .value = 64000 },
> + { .name = "96000",  .mask = SNDRV_PCM_RATE_96000,  .value = 96000 },
> + { .name = "176400", .mask = SNDRV_PCM_RATE_176400, .value = 176400 },
> + { .name = "192000", .mask = SNDRV_PCM_RATE_192000, .value = 192000 },
> +};
> +
> +struct cfg_hw_sample_format {
> + const char *name;
> + u64 mask;
> +};
> +
> +static const struct cfg_hw_sample_format CFG_HW_SUPPORTED_FORMATS[] = {
> + {
> + .name = XENSND_PCM_FORMAT_U8_STR,
> + .mask = SNDRV_PCM_FMTBIT_U8
> + },
> + {
> + .name = XENSND_PCM_FORMAT_S8_STR,
> + .mask = SNDRV_PCM_FMTBIT_S8
> + },
> + {
> + .name = XENSND_PCM_FORMAT_U16_LE_STR,
> + .mask = SNDRV_PCM_FMTBIT_U16_LE
> + },
> + {
> + .name = XENSND_PCM_FORMAT_U16_BE_STR,
> + .mask = SNDRV_PCM_FMTBIT_U16_BE
> + },
> + {
> + .name = XENSND_PCM_FORMAT_S16_LE_STR,
> + .mask = SNDRV_PCM_FMTBIT_S16_LE
> + },
> + {
> +

Re: [Xen-devel] [PATCH] tools/kdd: silence gcc 8 warning a different way

[Marek Marczykowski]

On Mon, Apr 16, 2018 at 06:00:37AM -0600, Jan Beulich wrote:
> >>> On 16.04.18 at 12:33,  wrote:
> > On Thu, Apr 12, 2018 at 06:04:49AM -0600, Jan Beulich wrote:
> >> Older gcc doesn't like "#pragma GCC diagnostic" inside functions.
> >> 
> >> Signed-off-by: Jan Beulich 
> >> 
> >> --- a/tools/debugger/kdd/kdd.c
> >> +++ b/tools/debugger/kdd/kdd.c
> >> @@ -695,10 +695,10 @@ static void kdd_handle_read_ctrl(kdd_sta
> >>  KDD_LOG(s, "Request outside of known control space\n");
> >>  len = 0;
> >>  } else {
> >> -#pragma GCC diagnostic push
> >> -#pragma GCC diagnostic ignored "-Warray-bounds"
> >> -memcpy(buf, ((uint8_t *)) + offset, len);
> >> -#pragma GCC diagnostic pop
> >> +/* Suppress bogus gcc 8 "out of bounds" warning. */
> >> +const uint8_t *src;
> >> +asm ("" : "=g" (src) : "0" ((uint8_t *) + offset));
> >> +memcpy(buf, src, len);
> > 
> > The code looks correct to me:
> > 
> > Reviewed-by: Wei Liu 
> > 
> > This will hopefully also fix the issue Boris reported that some older
> > gcc (<4.6) doesn't support push and pop.
> > 
> > This is the first time I see inline assembly is used to silence gcc.
> > ;-)
> 
> And I'm not overly happy about it, but couldn't think of a better way
> without disabling said warning (or -Werror) altogether for the CU. If
> Ian's sketched out approach worked, I'd be quite happy to drop the
> patch here.

What about changing offset type to uint32_t (or similar), which also
mute the warning?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 7/7] xen/arm: Restore IRQ affinity after hotplugging a CPU

[Dario Faggioli]

On Fri, 2018-04-13 at 12:11 +0200, Mirela Simonovic wrote:
> Hi Dario,
> 
Hi,

> On Thu, Apr 12, 2018 at 6:49 PM, Dario Faggioli 
> wrote:
> No worries, my commit message should be more understandable and your
> answer helps me identify what's unclear. I'll try to explain below.
> 
> This is about suspend/resume implementation for ARM that is based on
> PSCI and targeted for embedded systems as well. Each guest could have
> its own wake-up devices/interrupts (passthrough) that could trigger
> the resume. So the wake-up interrupt in this context triggers the
> resume. By 'all interrupts' I meant interrupts that are left enabled
> by guests upon suspend (those interrupts could wake-up the system).
> Here is the PSCI-based suspend to RAM design spec:
> https://lists.xenproject.org/archives/html/xen-devel/2017-12/msg01574
> .html
> 
Ah, so you are talking about suspend/resume of _the_guest_! This was
actually the bit of information that I was missing. :-)

This was, probbly, partly due to me not being familiar with ARM, and to
the fact that I haven't really looked at the rest of the series
probably.

Still, I think it would not hurt for the changelog to be a bit more
explitic and clear.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()

[Jan Beulich]

>>> On 16.04.18 at 14:18,  wrote:
> On 13/04/18 12:39, Jan Beulich wrote:
> On 13.04.18 at 13:17,  wrote:
>>> On 13/04/18 09:31, Jan Beulich wrote:
>>> On 12.04.18 at 18:55,  wrote:
> do_get_debugreg() has several bugs:
>
>  * The %cr4.de condition is inverted.  %dr4/5 should be accessible only 
> when
>%cr4.de is disabled.
>  * When %cr4.de is disabled, emulation should yield #UD rather than 
> complete
>with zero.
>  * Using -EINVAL for errors is a broken ABI, as it overlaps with valid 
> values
>near the top of the address space.
>
> Introduce a common x86emul_read_dr() handler (as we will eventually want 
> to
> add HVM support) which separates its success/failure indication from the 
> data
> value, and have do_get_debugreg() call into the handler.
 The HVM part here is sort of questionable because of your use of
 curr->arch.pv_vcpu.ctrlreg[4].
>>> That is what the "needs further plumbing" refers to, as well as needing
>>> hooks to get/modify %dr6/7 from the VMCB/VMCS.
>>>
>>> However, we are gaining an increasing amount of common x86 code which
>>> needs to read control register values, and I've got a plan to refactor
>>> across the board to v->arch.cr4 (and similar).  There is no point having
>>> identical information in different parts of sub-unions.
>> I agree.
>>
 This is appropriate for the NULL ctxt case,
 but it's already a layering violation for the use of the function in
 priv_op_ops, where the read_cr() hook should be used instead.
>>> Hmm - doing this, while probably the better long temr course of action,
>>> would require passing the ops structures down into the callbacks.
>> That doesn't sound like a problem, though - the hypercall path would
>> pass NULL there as well.

This ...

> +int x86emul_read_dr(unsigned int reg, unsigned long *val,
> +struct x86_emulate_ctxt *ctxt)
> +{
> +struct vcpu *curr = current;
> +
> +/* HVM support requires a bit more plumbing before it will work. */
> +ASSERT(is_pv_vcpu(curr));
> +
> +switch ( reg )
> +{
> +case 0 ... 3:
> +case 6:
> +*val = curr->arch.debugreg[reg];
> +break;
> +
> +case 7:
> +*val = (curr->arch.debugreg[7] |
> +curr->arch.debugreg[5]);
> +break;
> +
> +case 4 ... 5:
> +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) )
> +{
> +*val = curr->arch.debugreg[reg + 2];
> +break;
 Once at it, wouldn't you better also fix the missing ORing of [5] into the 
 DR7 (really
 DR5) value here?
>>> [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent
>>> patch), as IO breakpoints are only valid to use when %cr4.de is enabled.
>> Oh, right you are.
> 
> So, are your comments suitably addressed?  It is unclear whether you
> want any changes to be made.

... is what I'd prefer to be taken care of without delaying to the time when
we make this work for HVM as well. Unless you feel really strongly about it
being better the way you have it, in which case you may feel free to add
my ack.

Jan



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

Re: [Xen-devel] [PATCH v2 1/5] ALSA: xen-front: Introduce Xen para-virtualized sound frontend driver

[Juergen Gross]

On 16/04/18 08:24, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko 
> 
> Introduce skeleton of the para-virtualized Xen sound
> frontend driver.
> 
> Initial handling for Xen bus states: implement
> Xen bus state machine for the frontend driver according to
> the state diagram and recovery flow from sound para-virtualized
> protocol: xen/interface/io/sndif.h.
> 
> Signed-off-by: Oleksandr Andrushchenko 

Only one minor nit (see below). With that addressed (or fixed when
committing):

Reviewed-by: Juergen Gross 


Juergen

> ---
>  sound/Kconfig |   2 +
>  sound/Makefile|   2 +-
>  sound/xen/Kconfig |  10 +++
>  sound/xen/Makefile|   5 ++
>  sound/xen/xen_snd_front.c | 196 
> ++
>  sound/xen/xen_snd_front.h |  18 +
>  6 files changed, 232 insertions(+), 1 deletion(-)
>  create mode 100644 sound/xen/Kconfig
>  create mode 100644 sound/xen/Makefile
>  create mode 100644 sound/xen/xen_snd_front.c
>  create mode 100644 sound/xen/xen_snd_front.h
> 
> diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c
> new file mode 100644
> index ..f406a8f52c51
> --- /dev/null
> +++ b/sound/xen/xen_snd_front.c
> @@ -0,0 +1,196 @@
> +static void sndback_changed(struct xenbus_device *xb_dev,
> + enum xenbus_state backend_state)
> +{
> + struct xen_snd_front_info *front_info = dev_get_drvdata(_dev->dev);
> + int ret;
> +
> + dev_dbg(_dev->dev, "Backend state is %s, front is %s\n",
> + xenbus_strstate(backend_state),
> + xenbus_strstate(xb_dev->state));
> +
> + switch (backend_state) {
> + case XenbusStateReconfiguring:
> + /* fall through */
> + case XenbusStateReconfigured:
> + /* fall through */
> + case XenbusStateInitialised:
> + /* fall through */
> + break;
> +
> + case XenbusStateInitialising:
> + /* recovering after backend unexpected closure */
> + sndback_disconnect(front_info);
> + break;
> +
> + case XenbusStateInitWait:
> + /* recovering after backend unexpected closure */
> + sndback_disconnect(front_info);
> +
> + ret = sndback_initwait(front_info);
> + if (ret < 0)
> + xenbus_dev_fatal(xb_dev, ret, "initializing frontend");
> + else
> + xenbus_switch_state(xb_dev, XenbusStateInitialised);
> + break;
> +
> + case XenbusStateConnected:
> + if (xb_dev->state != XenbusStateInitialised)
> + break;
> +
> + ret = sndback_connect(front_info);
> + if (ret < 0)
> + xenbus_dev_fatal(xb_dev, ret, "initializing frontend");
> + else
> + xenbus_switch_state(xb_dev, XenbusStateConnected);
> + break;
> +
> + case XenbusStateClosing:
> + /*
> +  * in this state backend starts freeing resources,
> +  * so let it go into closed state first, so we can also
> +  * remove ours
> +  */

Please start the sentence with a capital letter and end it with a
full stop.


Juergen

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

Re: [Xen-devel] [PATCH v1] xen: Fix emfn calculation in init_domheap_pages()

[Oleksandr Tyshchenko]

On Mon, Apr 16, 2018 at 2:56 PM, Juergen Gross  wrote:
> On 16/04/18 13:54, Jan Beulich wrote:
> On 16.04.18 at 13:52,  wrote:
>> On 16.04.18 at 13:29,  wrote:
 From: Oleksandr Tyshchenko 

 The "end" address must be rounded down before shifting,
 otherwise we will insert wrong page range to a heap if address isn't
 page aligned.

 It seems that a copy-paste mistake took place in the following commit:
 0c12972e34b20a26f2b42044b98bf12db7ed62b6
 xen/mm: Switch some of page_alloc.c to typesafe MFN

 Signed-off-by: Oleksandr Tyshchenko 
>>>
>>> Reviewed-by: Jan Beulich 
>>
>> Oh, and Cc Jürgen.
>
> Thanks, just saw it myself. :-)
>
> Release-acked-by: Juergen Gross 
>
>
> Juergen

Thank you.


-- 
Regards,

Oleksandr Tyshchenko

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

Re: [Xen-devel] [PATCH 1/3] x86/pv: Introduce and use x86emul_read_dr()

[Andrew Cooper]

On 13/04/18 12:39, Jan Beulich wrote:
 On 13.04.18 at 13:17,  wrote:
>> On 13/04/18 09:31, Jan Beulich wrote:
>> On 12.04.18 at 18:55,  wrote:
 do_get_debugreg() has several bugs:

  * The %cr4.de condition is inverted.  %dr4/5 should be accessible only 
 when
%cr4.de is disabled.
  * When %cr4.de is disabled, emulation should yield #UD rather than 
 complete
with zero.
  * Using -EINVAL for errors is a broken ABI, as it overlaps with valid 
 values
near the top of the address space.

 Introduce a common x86emul_read_dr() handler (as we will eventually want to
 add HVM support) which separates its success/failure indication from the 
 data
 value, and have do_get_debugreg() call into the handler.
>>> The HVM part here is sort of questionable because of your use of
>>> curr->arch.pv_vcpu.ctrlreg[4].
>> That is what the "needs further plumbing" refers to, as well as needing
>> hooks to get/modify %dr6/7 from the VMCB/VMCS.
>>
>> However, we are gaining an increasing amount of common x86 code which
>> needs to read control register values, and I've got a plan to refactor
>> across the board to v->arch.cr4 (and similar).  There is no point having
>> identical information in different parts of sub-unions.
> I agree.
>
>>> This is appropriate for the NULL ctxt case,
>>> but it's already a layering violation for the use of the function in
>>> priv_op_ops, where the read_cr() hook should be used instead.
>> Hmm - doing this, while probably the better long temr course of action,
>> would require passing the ops structures down into the callbacks.
> That doesn't sound like a problem, though - the hypercall path would
> pass NULL there as well.
>
 +int x86emul_read_dr(unsigned int reg, unsigned long *val,
 +struct x86_emulate_ctxt *ctxt)
 +{
 +struct vcpu *curr = current;
 +
 +/* HVM support requires a bit more plumbing before it will work. */
 +ASSERT(is_pv_vcpu(curr));
 +
 +switch ( reg )
 +{
 +case 0 ... 3:
 +case 6:
 +*val = curr->arch.debugreg[reg];
 +break;
 +
 +case 7:
 +*val = (curr->arch.debugreg[7] |
 +curr->arch.debugreg[5]);
 +break;
 +
 +case 4 ... 5:
 +if ( !(curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_DE) )
 +{
 +*val = curr->arch.debugreg[reg + 2];
 +break;
>>> Once at it, wouldn't you better also fix the missing ORing of [5] into the 
>>> DR7 (really
>>> DR5) value here?
>> [5] is zero when %cr4.de is clear (subject to a bugfix in the subsequent
>> patch), as IO breakpoints are only valid to use when %cr4.de is enabled.
> Oh, right you are.

So, are your comments suitably addressed?  It is unclear whether you
want any changes to be made.

~Andrew

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

[Xen-devel] [distros-debian-sid test] 74625: trouble: blocked/broken/fail/pass

[Platform Team regression test user]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64  broken
 build-arm64-pvopsbroken

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-armhf-sid-netboot-pygrub  1 build-check(1)blocked n/a
 build-arm64   2 hosts-allocate   broken like 74563
 build-arm64-pvops 2 hosts-allocate   broken like 74563
 build-arm64   3 capture-logs broken like 74563
 build-arm64-pvops 3 capture-logs broken like 74563
 test-amd64-i386-i386-sid-netboot-pvgrub 10 debian-di-install   fail like 74563
 test-amd64-i386-amd64-sid-netboot-pygrub 10 debian-di-install  fail like 74563
 test-amd64-amd64-amd64-sid-netboot-pvgrub 10 debian-di-install fail like 74563
 test-amd64-amd64-i386-sid-netboot-pygrub 10 debian-di-install  fail like 74563
 test-armhf-armhf-armhf-sid-netboot-pygrub 10 debian-di-install fail like 74563

baseline version:
 flight   74563

jobs:
 build-amd64  pass
 build-arm64  broken  
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-arm64-pvopsbroken  
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-sid-netboot-pvgrubfail
 test-amd64-i386-i386-sid-netboot-pvgrub  fail
 test-amd64-i386-amd64-sid-netboot-pygrub fail
 test-arm64-arm64-armhf-sid-netboot-pygrubblocked 
 test-armhf-armhf-armhf-sid-netboot-pygrubfail
 test-amd64-amd64-i386-sid-netboot-pygrub fail



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

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

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


Push not applicable.


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

Re: [Xen-devel] [PATCH] tools/kdd: silence gcc 8 warning a different way

[Jan Beulich]

>>> On 16.04.18 at 12:33,  wrote:
> On Thu, Apr 12, 2018 at 06:04:49AM -0600, Jan Beulich wrote:
>> Older gcc doesn't like "#pragma GCC diagnostic" inside functions.
>> 
>> Signed-off-by: Jan Beulich 
>> 
>> --- a/tools/debugger/kdd/kdd.c
>> +++ b/tools/debugger/kdd/kdd.c
>> @@ -695,10 +695,10 @@ static void kdd_handle_read_ctrl(kdd_sta
>>  KDD_LOG(s, "Request outside of known control space\n");
>>  len = 0;
>>  } else {
>> -#pragma GCC diagnostic push
>> -#pragma GCC diagnostic ignored "-Warray-bounds"
>> -memcpy(buf, ((uint8_t *)) + offset, len);
>> -#pragma GCC diagnostic pop
>> +/* Suppress bogus gcc 8 "out of bounds" warning. */
>> +const uint8_t *src;
>> +asm ("" : "=g" (src) : "0" ((uint8_t *) + offset));
>> +memcpy(buf, src, len);
> 
> The code looks correct to me:
> 
> Reviewed-by: Wei Liu 
> 
> This will hopefully also fix the issue Boris reported that some older
> gcc (<4.6) doesn't support push and pop.
> 
> This is the first time I see inline assembly is used to silence gcc.
> ;-)

And I'm not overly happy about it, but couldn't think of a better way
without disabling said warning (or -Werror) altogether for the CU. If
Ian's sketched out approach worked, I'd be quite happy to drop the
patch here.

Jan



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

Re: [Xen-devel] [PATCH 0/3] x86: S3 resume adjustments

[Juergen Gross]

On 13/04/18 14:01, Andrew Cooper wrote:
> On 13/04/18 12:49, Jan Beulich wrote:
>> 1: correct ordering of operations during S3 resume
>> 2: suppress BTI mitigations around S3 suspend/resume
>> 3: check feature flags after resume
>>
>> Signed-off-by: Jan Beulich 
> 
> Acked-by: Andrew Cooper 
> 

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH v1] xen: Fix emfn calculation in init_domheap_pages()

[Juergen Gross]

On 16/04/18 13:54, Jan Beulich wrote:
 On 16.04.18 at 13:52,  wrote:
> On 16.04.18 at 13:29,  wrote:
>>> From: Oleksandr Tyshchenko 
>>>
>>> The "end" address must be rounded down before shifting,
>>> otherwise we will insert wrong page range to a heap if address isn't
>>> page aligned.
>>>
>>> It seems that a copy-paste mistake took place in the following commit:
>>> 0c12972e34b20a26f2b42044b98bf12db7ed62b6
>>> xen/mm: Switch some of page_alloc.c to typesafe MFN
>>>
>>> Signed-off-by: Oleksandr Tyshchenko 
>>
>> Reviewed-by: Jan Beulich 
> 
> Oh, and Cc Jürgen.

Thanks, just saw it myself. :-)

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH v1] xen: Fix emfn calculation in init_domheap_pages()

[Jan Beulich]

>>> On 16.04.18 at 13:52,  wrote:
 On 16.04.18 at 13:29,  wrote:
>> From: Oleksandr Tyshchenko 
>> 
>> The "end" address must be rounded down before shifting,
>> otherwise we will insert wrong page range to a heap if address isn't
>> page aligned.
>> 
>> It seems that a copy-paste mistake took place in the following commit:
>> 0c12972e34b20a26f2b42044b98bf12db7ed62b6
>> xen/mm: Switch some of page_alloc.c to typesafe MFN
>> 
>> Signed-off-by: Oleksandr Tyshchenko 
> 
> Reviewed-by: Jan Beulich 

Oh, and Cc Jürgen.

Jan


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

Re: [Xen-devel] [PATCH v1] xen: Fix emfn calculation in init_domheap_pages()

[Jan Beulich]

>>> On 16.04.18 at 13:29,  wrote:
> From: Oleksandr Tyshchenko 
> 
> The "end" address must be rounded down before shifting,
> otherwise we will insert wrong page range to a heap if address isn't
> page aligned.
> 
> It seems that a copy-paste mistake took place in the following commit:
> 0c12972e34b20a26f2b42044b98bf12db7ed62b6
> xen/mm: Switch some of page_alloc.c to typesafe MFN
> 
> Signed-off-by: Oleksandr Tyshchenko 

Reviewed-by: Jan Beulich 



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

Re: [Xen-devel] [PATCH] x86/hpet: add a lock when cpu clear cpumask in hpet_broadcast_exit();

[Jan Beulich]

>>> On 16.04.18 at 10:00,  wrote:
> By the hpet_get_channel(), cpus share an in-use channel somtime.
> So, core shouldn't clear cpumask while others are getting first
> cpumask. If core zero and core one share an channel, the cpumask
> is 0x3. Core zero clear cpumask between core one executing
> cpumask_empty() and cpumask_first(). The return of cpumask_first()
> is nr_cpu_ids. That would lead to ASSERT(cpu < nr_cpu_ids).

I can see your point, but that's in hpet_detach_channel() afaics,
which your description doesn't mention at all. And the assertion
would - afaict - happen through hpet_detach_channel() ->
set_channel_irq_affinity() -> cpumask_of() (as of e8bf5addc9).

Please realize that it helps review quite a bit if you write concise
descriptions for your changes.

> --- a/xen/arch/x86/hpet.c
> +++ b/xen/arch/x86/hpet.c
> @@ -740,7 +740,9 @@ void hpet_broadcast_exit(void)
>  if ( !reprogram_timer(deadline) )
>  raise_softirq(TIMER_SOFTIRQ);
>  
> +spin_lock_irq(>lock);
>  cpumask_clear_cpu(cpu, ch->cpumask);
> +spin_unlock_irq(>lock);

Rather than this, how about eliminating the cpumask_empty() call
in favor of just the cpumask_first() one in hpet_detach_channel()
(with a local variable storing the intermediate result)? Or if acquiring
the locking can't be avoided here, you would perhaps better not
drop it before calling hpet_detach_channel() (which has only this
single call site and hence would be straightforward to adjust).

Jan



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

Re: [Xen-devel] [PATCH for-4.11 0/3] x86/pv: Fixes to debug register handling

[Juergen Gross]

On 12/04/18 18:55, Andrew Cooper wrote:
> At this point, I think these patches are plausible candidates for inclusion
> into 4.11.  Whether they want backporting is a slightly harder matter, as
> these patches necesserily alter some error values for the get/set_debug_reg()
> hypercalls.
> 
> If however there is objection to these going into 4.11, they can sit in
> x86-next until the 4.12 window opens.
> 
> Andrew Cooper (3):
>   x86/pv: Introduce and use x86emul_read_dr()
>   x86/pv: Introduce and use x86emul_write_dr()
>   x86/traps: Misc non-functional improvements to set_debugreg()

The complete series:

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH for-4.11] x86/msr: Correct the emulation behaviour of MSR_PRED_CMD

[Juergen Gross]

On 16/04/18 12:56, Andrew Cooper wrote:
> The behaviour of reserved bits in MSR_PRED_CMD changed between beta and
> production microcode, and now raises a #GP fault for set reserved bits. The
> AMD spec for future hardware also specifies this behaviour.
> 
> Signed-off-by: Andrew Cooper 

Release-acked-by: Juergen Gross 


Juergen

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

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

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 665bfd41ac32b364201c07dc1c5434432730c034
baseline version:
 ovmf b85b20fba42e25ff658ed1a470250d530c189027

Last test of basis   122311  2018-04-15 13:17:20 Z0 days
Testing same since   122324  2018-04-16 06:17:01 Z0 days1 attempts


People who touched revisions under test:
  Star Zeng 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   b85b20fba4..665bfd41ac  665bfd41ac32b364201c07dc1c5434432730c034 -> 
xen-tested-master

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

Re: [Xen-devel] [PATCH 0/7] xen/arm: CPU hotplug fixes

[Julien Grall]

Hi,

On 13/04/18 11:19, Mirela Simonovic wrote:

On Thu, Apr 12, 2018 at 10:43 AM, Julien Grall  wrote:



On 11/04/18 17:37, Mirela Simonovic wrote:


Hi Julien,



Hi,

May I ask you to configure your mail client to use > for quoting and use
plain text? Otherwise, this is going to be really difficult to follow the
discussion after few round (see already below).


On Wed, Apr 11, 2018 at 6:02 PM, Julien Grall > wrote:

 Hi,

 On 11/04/18 16:58, Mirela Simonovic wrote:

 On 04/11/2018 05:07 PM, Julien Grall wrote:

 On 11/04/18 14:19, Mirela Simonovic wrote:

 Migrating interrupts when turning off a CPU already works.
 However, when a CPU is turned back on there is no interrupt
 migration back to the hotplugged CPU - all interrupts will
 remain routed to the CPU#0.
 Patch 7/7 fixes this


 What do you mean by all interrupts? Interrupts routed to guest will
 always follow the vCPU. So are you sure they are going to be
 migrated when that vCPU is paused/off?


Just to make sure we're on the same page - this is about hotplugging
physical CPUs. Hotplugging vCPUs using virtual PSCI CPU_OFF interface is
already implemented and unrelated to this series.



Yes, we are on the same page :). I was just wondering what happen to
interrupt routed to that pCPU.



Assuming that system has 2 pCPUs by 'all interrupts' I mean interrupts
that were targeted to the pCPU#0 and pCPU#1 prior to doing any hotplug.

For example, if a guest is pinned to pCPU#1 an interrupt of a device it
owns will be targeted to pCPU#1.
When pCPU#1 is turned off that interrupt will be migrated to pCPU#0.
pCPU#0 finalizes the suspend and receives wake-up interrupts. However, when
CPU#1 is turned back on that interrupt will remain targeted to the CPU#0,
which I assumed is wrong.
The scenario described here is also how I tested this.

 Can you give the path in Xen doing that?


Sure, here is a backtrace (dumped on the CPU being turned off):
  0  0x2603dc arch_move_irqs(): vgic.c, line 309
  1  0x22ee58 sched_move_irqs()+20: schedule.c, line 303
  2  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
  3  0x2318e8 cpu_disable_scheduler()+1000: schedule.c, line 586
  4  0x25aff8 __cpu_disable()+96: smpboot.c, line 386
  5  0x201608 take_cpu_down()+52: cpu.c, line 75
  6  0x23426c stopmachine_action()+188: stop_machine.c, line 159
  7  0x235858 do_tasklet_work()+176: tasklet.c, line 94
  8  0x235c80 do_tasklet()+104: tasklet.c, line 126
  9  0x24daec idle_loop()+144: domain.c, line 72
 10  0x25b1f8 start_secondary()+404: smpboot.c, line 368




So this cover interrupt routed to a virtual CPU. However, this does not
handle interrupts used by Xen. How do you handle them?

For instance SMMUs IRQ might be routed to other interrupt than CPU #0.


Interrupts used by Xen should not wake-up the system and will be
disabled when we suspend the devices used by Xen.
Here you only speak about the suspend use case. While I understand your 
ultimate goal is suspend/resume, this series is about CPU hotplug.


IHMO, the suspend/resume case is no more than a superset of CPU up/down. 
If you solve the problem for up/down, likely you are going to solve it 
for suspend/resume.


So, what would happen to interrupts routed to the CPU going offline?


However, I need to double check that such interrupts get enabled on
the right CPU on resume. Could you please tell me which mechanism in
Xen is used to target such an interrupt to a secondary CPU only? Is
that even possible and why would that be used?


SPIs will be routed to the CPU calling setup_irq. It may not always be 
CPU#0. For instance, this is the case context interrupt for the SMMU 
because they are setup when the device is assigned.


I guess this decision is arguable. If you move all the interrupts to 
CPU#0 it will potentially disrupt vCPU running on it. I am thinking in 
the case of SMMU fault that could be triggered easily by another domain.


Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH v1] xen: Fix emfn calculation in init_domheap_pages()

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko 

The "end" address must be rounded down before shifting,
otherwise we will insert wrong page range to a heap if address isn't
page aligned.

It seems that a copy-paste mistake took place in the following commit:
0c12972e34b20a26f2b42044b98bf12db7ed62b6
xen/mm: Switch some of page_alloc.c to typesafe MFN

Signed-off-by: Oleksandr Tyshchenko 
CC: Andrew Cooper 
CC: George Dunlap 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Julien Grall 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
CC: xen-de...@lists.xen.org
---
 xen/common/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 186b39a..20ee1e4 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2166,7 +2166,7 @@ void init_domheap_pages(paddr_t ps, paddr_t pe)
 ASSERT(!in_irq());
 
 smfn = maddr_to_mfn(round_pgup(ps));
-emfn = maddr_to_mfn(round_pgup(pe));
+emfn = maddr_to_mfn(round_pgdown(pe));
 
 if ( mfn_x(emfn) <= mfn_x(smfn) )
 return;
-- 
2.7.4


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

Re: [Xen-devel] [PATCH for-4.11] x86/msr: Correct the emulation behaviour of MSR_PRED_CMD

[Jan Beulich]

>>> On 16.04.18 at 13:19,  wrote:
> On 16/04/18 12:12, Jan Beulich wrote:
> On 16.04.18 at 12:56,  wrote:
>>> The behaviour of reserved bits in MSR_PRED_CMD changed between beta and
>>> production microcode, and now raises a #GP fault for set reserved bits.
>> Interesting - quite unfortunate a change. Plus - I can't find where this is 
>> being
>> said.
> 
> Its not, but the new behaviour can be demonstrated easily.  Those with
> the beta microcode can confirm that the old behaviour was to ignore.
> 
> FWIW, ignoring reserved bits was always dubious, and I only implemented
> it like that to match how hardware behaved.  This new behaviour is far
> more sane.

Fully agree - should have been that way from the beginning.

Jan



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

Re: [Xen-devel] [PATCH for-4.11] x86/msr: Correct the emulation behaviour of MSR_PRED_CMD

[Andrew Cooper]

On 16/04/18 12:12, Jan Beulich wrote:
 On 16.04.18 at 12:56,  wrote:
>> The behaviour of reserved bits in MSR_PRED_CMD changed between beta and
>> production microcode, and now raises a #GP fault for set reserved bits.
> Interesting - quite unfortunate a change. Plus - I can't find where this is 
> being
> said.

Its not, but the new behaviour can be demonstrated easily.  Those with
the beta microcode can confirm that the old behaviour was to ignore.

FWIW, ignoring reserved bits was always dubious, and I only implemented
it like that to match how hardware behaved.  This new behaviour is far
more sane.

>
>> The AMD spec for future hardware also specifies this behaviour.
> I can find this one (albeit not in the PRM).

Its still in whitepaper form. 

https://developer.amd.com/wp-content/resources/Architecture_Guidelines_Update_Indirect_Branch_Control.pdf

"Indirect branch prediction barrier (IBPB) exists at MSR 0x49 (PRED_CMD)
bit 0. This is a write only MSR that both GP faults when software reads
it or if software tries to write any of the bits in 63:1"

~Andrew

>
>> Signed-off-by: Andrew Cooper 
> With some clarification on the origin of the Intel related information
> Reviewed-by: Jan Beulich 
>
> Jan
>
>


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

Re: [Xen-devel] [PATCH] tools/kdd: silence gcc 8 warning a different way

[Ian Jackson]

Wei Liu writes ("Re: [PATCH] tools/kdd: silence gcc 8 warning a different way"):
> On Thu, Apr 12, 2018 at 06:04:49AM -0600, Jan Beulich wrote:
> > Older gcc doesn't like "#pragma GCC diagnostic" inside functions.

Surely this commit message should refer to the previous commit ?


I reviewed the previous code to check that the validation was correct:

   if (offset > sizeof ctrl.c32 || offset + len > sizeof ctrl.c32) {
 len = 0;
   } else {
 memcpy(buf, ((uint8_t *)) + offset, len);
   }

I see two problems:

1. Adversarial optimissation hazard:

   The compiler may reason as follows:
   It is not legal to compute an out-of-bounds pointer.
   Doing so is UB.
   Therefore  ((uint8_t *)) + offset
   (which is caclulated unconditionally)
   is within the stack object `ctrl'.
   Therefore offset <= sizeof(ctrl).

1a. The compiler can continue to reason:
   Suppose offset > sizeof(ctrl.c32) but offset + len <=
   sizeof(ctrl.c32).  Because len is limited to 32-bit
   that can only happen if offset is large enough to
   wrap when len is added.  But I know that offset <= 216.
   So this situation cannot exist.
   Therefore I can remove the check for offset and
   rely only on the check for offset + len.

1b. The compiler can continue to reason:
   So offset <= 216.  I can therefore check that
   offset <= sizeof(ctrl.c32) using an instruction sequence
   that only looks at the bottom byte of offset (which on
   some architectures might be faster).

1c. If sizeof(ctrl.c32) ever becomes the same as sizeof(ctrl),
   the compiler can remove both checks entirely.

2. Style problem:

   Suppose  len = (uint64_t)-1   offset = 1
   The check passes, but the memcpy gets len = bazillion-1.
   In fact, len is a uint32_t so this is not possible but it is
   not possible to review these lines in isolation and see that
   they are correct.  A future programmer might increase the size
   of len, introducing a bug.  You should compile-time assert
   that len is 32-bit, somehow, right next to that check, IMO.

> > +/* Suppress bogus gcc 8 "out of bounds" warning. */
> > +const uint8_t *src;
> > +asm ("" : "=g" (src) : "0" ((uint8_t *) + offset));
> > +memcpy(buf, src, len);
> 
> The code looks correct to me:

IMO the new code is very hard to follow.  Are we really expecting
every reader of this to know w3hat the asm does ?

At the very least it should be accompanied with an explanation of what
the asm does, for the benefit of readers who don't speak asm.

I think that rather than introducing an asm, we should disable -Werror
for known-buggy compilers.  But it is possible that fixing the
problems I identify above will make the warning go away anyway.

Ian.

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

Re: [Xen-devel] [PATCH for-4.11] x86/msr: Correct the emulation behaviour of MSR_PRED_CMD

[Jan Beulich]

>>> On 16.04.18 at 12:56,  wrote:
> The behaviour of reserved bits in MSR_PRED_CMD changed between beta and
> production microcode, and now raises a #GP fault for set reserved bits.

Interesting - quite unfortunate a change. Plus - I can't find where this is 
being
said.

> The AMD spec for future hardware also specifies this behaviour.

I can find this one (albeit not in the PRM).

> Signed-off-by: Andrew Cooper 

With some clarification on the origin of the Intel related information
Reviewed-by: Jan Beulich 

Jan



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

[Xen-devel] [PATCH for-4.11] x86/msr: Correct the emulation behaviour of MSR_PRED_CMD

[Andrew Cooper]

The behaviour of reserved bits in MSR_PRED_CMD changed between beta and
production microcode, and now raises a #GP fault for set reserved bits. The
AMD spec for future hardware also specifies this behaviour.

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

This wants backporting to all trees which gained Spectre workarounds, and
therefore wants including in 4.11 at this point.
---
 xen/arch/x86/msr.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 369b475..d034561 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -243,11 +243,10 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t 
val)
 if ( !cp->feat.ibrsb && !cp->extd.ibpb )
 goto gp_fault; /* MSR available? */
 
-/*
- * The only defined behaviour is when writing PRED_CMD_IBPB.  In
- * practice, real hardware accepts any value without faulting.
- */
-if ( v == curr && (val & PRED_CMD_IBPB) )
+if ( val & ~PRED_CMD_IBPB )
+goto gp_fault; /* Rsvd bit set? */
+
+if ( v == curr )
 wrmsrl(MSR_PRED_CMD, PRED_CMD_IBPB);
 break;
 
-- 
2.1.4


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

Re: [Xen-devel] [PATCH] tools/kdd: silence gcc 8 warning a different way

[Wei Liu]

On Thu, Apr 12, 2018 at 06:04:49AM -0600, Jan Beulich wrote:
> Older gcc doesn't like "#pragma GCC diagnostic" inside functions.
> 
> Signed-off-by: Jan Beulich 
> 
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -695,10 +695,10 @@ static void kdd_handle_read_ctrl(kdd_sta
>  KDD_LOG(s, "Request outside of known control space\n");
>  len = 0;
>  } else {
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Warray-bounds"
> -memcpy(buf, ((uint8_t *)) + offset, len);
> -#pragma GCC diagnostic pop
> +/* Suppress bogus gcc 8 "out of bounds" warning. */
> +const uint8_t *src;
> +asm ("" : "=g" (src) : "0" ((uint8_t *) + offset));
> +memcpy(buf, src, len);

The code looks correct to me:

Reviewed-by: Wei Liu 

This will hopefully also fix the issue Boris reported that some older
gcc (<4.6) doesn't support push and pop.

This is the first time I see inline assembly is used to silence gcc.
;-)

>  }
>  }
>  
> 
> 
> 

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

Re: [Xen-devel] [PATCH 2/2] x86/microcode: Do not upload microcode if CPUs are offline

[Jan Beulich]

>>> On 13.04.18 at 19:57,  wrote:
> On Fri, Apr 13, 2018 at 09:57:25AM -0600, Jan Beulich wrote:
>> >>> On 30.03.18 at 08:59,  wrote:
>> > This patch is to backport the patch below from linux kernel.
>> > 
>> > commit 30ec26da9967d0d785abc24073129a34c3211777
>> > Author: Ashok Raj 
>> > Date:   Wed Feb 28 11:28:43 2018 +0100
>> > 
>> > x86/microcode: Do not upload microcode if CPUs are offline
>> > 
>> > Avoid loading microcode if any of the CPUs are offline, and issue a
>> > warning. Having different microcode revisions on the system at any 
>> > time
>> > is outright dangerous.
>> 
>> I'm afraid I don't fully agree - not applying an ucode update to the online
>> CPUs because some are offline isn't any better. Plus (while updating)
>> there's always going to be some discrepancy between ucode versions.
>> As long as we apply updates while bringing a CPU online, I think we're fine.
> 
> This is the safest option. Microcode is considered part of the cpu. We don't
> allow cpus with different capabilities in the same system.. yes they might
> work, but not something we allow. 

Please compare with the boot time CPU bringup cases: Just like when onlining
a CPU at run time, the new microcode is put in place early during bringing up
the CPU. I don't see the difference in risk, yet from what you say we should
disallow boot time applying ucode to the BSP (much earlier than the APs) as
well.

> In general we recommend early update. Earliest the best. 
> 
> - BIOS update (difficult to deploy, but some microcodes have to be done
>   this way.)
> - early update from initrd.. almost same as #1, since we apply at earliest
>   chance that's the closest and most recommended method.
> - late update. Before this procedure of stopping all cpus, we did have a 
>   time when some are updated and some werent uptodate yet. This synchronized
>   update is precicely to get as close as possible to updating all of them.
>> 
>> Also please consider valid cases you make not work anymore, like someone
>> having brought offline all sibling hyperthreads, with each core still having
>> one thread active. In that case an ucode update will implicitly update all
>> offline threads as well.
> 
> Of cource we can tweak this to be much better, there are other ideas, but
> this is an effort to keep this simple, and also address microcode requirements
> post spectre for some processors.  In the grand scheme of things although its
> interesting to allow such updates we think it may not be best practice.
> 
> We want to get this working right first before getting fancy.

Simplicity and correctness are very worthwhile goals, but please without
regressing valid scenarios.

Jan



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

Re: [Xen-devel] [PATCH 1/2] x86/microcode: Synchronize late microcode loading

[Jan Beulich]

>>> On 16.04.18 at 08:20,  wrote:
> On Fri, Apr 13, 2018 at 09:49:17AM -0600, Jan Beulich wrote:
> On 30.03.18 at 08:59,  wrote:
>>> +static int do_microcode_update(void *_info)
>>> +{
>>> +struct microcode_info *info = _info;
>>> +int error, ret = 0;
>>> +
>>> +error = __wait_for_cpus(>cpu_in, USEC_PER_SEC);
>>
>>Why this long a timeout here?
> 
> I just use the same timeout as the patch on linux kernel side. As we
> know if the timeout is too small, updating microcode may be likely to
> failed even if other CPUs disabled interrupt temporally.
> 
> If you object to such a long timeout (for Xen may need much smaller
> time to rendezvous all CPUs compared to linux kernel because Xen doesn't
> have device drivers which may malfunction), how about just use the
> default timeout, 30ms, used by live patching? if it is also not
> good enough, then we make it an option which comes from callers.

Yes, 30ms is likely to be acceptable.

>>> +if ( error )
>>> +{
>>> +ret = -EBUSY;
>>> +return ret;
>>> +}
>>> +
>>> +error = microcode_update_cpu(info->buffer, info->buffer_size);
>>> +if ( error && !ret )
>>> +ret = error;
>>> +/*
>>> + * Increase the wait timeout to a safe value here since we're 
>>> serializing
>>> + * the microcode update and that could take a while on a large number 
>>> of
>>> + * CPUs. And that is fine as the *actual* timeout will be determined by
>>> + * the last CPU finished updating and thus cut short
>>> + */
>>> +error = __wait_for_cpus(>cpu_out, USEC_PER_SEC * 
>>> num_online_cpus());
>>
>>And this one's even worse, in particular on huge systems. I'm afraid such a 
>>long
>>period of time in stop-machine context is going to confuse most of the running
>>domains (including Dom0). There's nothing inherently wrong with e.g. 
>>processing
>>the updates on distinct cores (and even more so on distinct sockets) in 
>>parallel.
> 
> I cannot say for sure. But the original patch does want updating
> microcode be performed one-by-one.

And they don't restrict the "when" aspect in any way? I.e. all sorts of
applications (and perhaps even KVM guests) may be running?

>>Therefore revising the locking in microcode_update_cpu() might be a necessary
>>prereq step.
> 
> Do you mean changing it to a per-core or per-socket lock?

Either changing to such, or introducing a second, more fine grained lock.

>>Or alternatively you may need to demand that no other running
>>domains exist besides Dom0 (and hope the best for Dom0 itself).
>>
>>I also don't think there's any point invoking the operation on all HT threads 
>>on a
>>core, but I realize stop_machine_run() isn't flexible enough to allow such.
> 
> Only one thread in a core will do the actual update. Other threads only
> check the microcode version and find the version is already
> update-to-date, then exit the critical region. Considering the check may
> don't need much time, I wonder whether it can significantly benefit the
> overall time to invoking the operation on all HT threads on a core?  

With better parallelism this would be less of an issue. Plus, as said - the
infrastructure we have at present wouldn't allow anyway what I've
described above, and hand-rolling another "flavor" of the stop-machine
logic is quite certainly out of question. To answer your question though:
Taking as the example a 4-threads-per-core system with sufficiently
many cores, I'm afraid the overall time spent in handling the
"uninteresting" threads may become measurable. But of course, if actual
numbers said otherwise, I'd be fine.

Jan



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

[Xen-devel] osstest outage,

[Ian Jackson]

Ian Jackson writes ("Re: osstest planned outage consultation"):
> We are now planning to do this work on April 19th-21st.  osstest will
> be shut down some time on the 17th/18th to let it drain its queue.

Reminder: I will stop it staring on new flights, some time tomorrow.

Ian.

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

Re: [Xen-devel] [PATCH v4 1/7] introduce time managment in xtf

[Paul Semel]

On 04/13/2018 02:05 PM, Roger Pau Monné wrote:

this file is introduce to be able to implement an inter domain
communication protocol over xenstore. For synchronization purpose, we do
really want to be able to "control" time

common/time.c: since_boot_time gets the time in nanoseconds from the
moment the VM has booted

Signed-off-by: Paul Semel 
---

Notes:
 v4:
 - moved rdtsc to arch/x86/include/arch/lib.h
 - added a rdtsc_ordered implementation to serialize rdtsc
 - simplified since_boot_time function
 - still need to have Andrew's scale_delta version

  arch/x86/include/arch/lib.h | 18 ++
  build/files.mk  |  1 +
  common/time.c   | 81 +
  include/xtf/time.h  | 24 ++
  4 files changed, 124 insertions(+)
  create mode 100644 common/time.c
  create mode 100644 include/xtf/time.h

diff --git a/arch/x86/include/arch/lib.h b/arch/x86/include/arch/lib.h
index 0045902..510cdb1 100644
--- a/arch/x86/include/arch/lib.h
+++ b/arch/x86/include/arch/lib.h
@@ -6,6 +6,7 @@
  #include 
  #include 
  #include 
+#include 


This include list is sorted alphabetically.

  
  static inline void cpuid(uint32_t leaf,

   uint32_t *eax, uint32_t *ebx,
@@ -374,6 +375,23 @@ static inline void write_xcr0(uint64_t xcr0)
  xsetbv(0, xcr0);
  }
  
+static inline uint64_t rdtsc(void)

+{
+uint32_t lo, hi;
+
+asm volatile("rdtsc": "=a"(lo), "=d"(hi));
+
+return ((uint64_t)hi << 32) | lo;
+}
+
+static inline uint64_t rdtsc_ordered(void)
+{
+rmb();
+mb();
+
+return rdtsc();
+}


I would likely just add a single function like:

static inline uint64_t rdtsc_ordered(void)
{
 uint32_t lo, hi;

 asm volatile("lfence; mfence; rdtsc" : "=a" (lo), "=d" (hi));

 return ((uint64_t)hi << 32) | lo;
}

Because there's no external caller of rdtsc, but I will leave that to
Andrew to decide. In any case this should work fine.



I think you're right for this one. What do you think about it @Andrew ?


+
  #endif /* XTF_X86_LIB_H */
  
  /*

diff --git a/build/files.mk b/build/files.mk
index 46b42d6..55ed1ca 100644
--- a/build/files.mk
+++ b/build/files.mk
@@ -16,6 +16,7 @@ obj-perarch += $(ROOT)/common/libc/vsnprintf.o
  obj-perarch += $(ROOT)/common/report.o
  obj-perarch += $(ROOT)/common/setup.o
  obj-perarch += $(ROOT)/common/xenbus.o
+obj-perarch += $(ROOT)/common/time.o
  
  obj-perenv += $(ROOT)/arch/x86/decode.o

  obj-perenv += $(ROOT)/arch/x86/desc.o
diff --git a/common/time.c b/common/time.c
new file mode 100644
index 000..79abc7e
--- /dev/null
+++ b/common/time.c
@@ -0,0 +1,81 @@
+#include 
+#include 
+#include 


Alphabetic order please.


+#include 
+#include 
+
+/* This function was taken from mini-os source code */
+/* It returns ((delta << shift) * mul_frac) >> 32 */


Comment has wrong style, please check CODING_STYLE.


+static inline uint64_t scale_delta(uint64_t delta, uint32_t mul_frac, int 
shift)
+{
+uint64_t product;
+#ifdef __i386__
+uint32_t tmp1, tmp2;
+#endif
+
+if ( shift < 0 )
+delta >>= -shift;
+else
+delta <<= shift;
+
+#ifdef __i386__
+__asm__ (
+"mul  %5   ; "
+"mov  %4,%%eax ; "
+"mov  %%edx,%4 ; "
+"mul  %5   ; "
+"add  %4,%%eax ; "
+"xor  %5,%5; "
+"adc  %5,%%edx ; "
+: "=A" (product), "=r" (tmp1), "=r" (tmp2)
+: "a" ((uint32_t)delta), "1" ((uint32_t)(delta >> 32)), "2" 
(mul_frac) );


This line is too long.


+#else
+__asm__ (
+"mul %%rdx ; shrd $32,%%rdx,%%rax"
+: "=a" (product) : "0" (delta), "d" ((uint64_t)mul_frac) );


Not sure whether you need to add a ': "d"' clobber here, since the d
register is used but it's not in the list of output operands.


+#endif
+
+return product;
+}
+


Actually, I'm not sure that I have to make that much changes to this function, 
as @Andrew wanted to use another version of it as far as I recall.



+
+uint64_t since_boot_time(void)
+{
+uint32_t ver1, ver2;
+uint64_t tsc_timestamp, system_time, tsc;
+uint32_t tsc_to_system_mul;
+int8_t tsc_shift;
+
+do
+{
+ver1 = ACCESS_ONCE(shared_info.vcpu_info[0].time.version);
+smp_rmb();
+
+system_time = shared_info.vcpu_info[0].time.system_time;
+tsc_timestamp = shared_info.vcpu_info[0].time.tsc_timestamp;
+tsc_to_system_mul = shared_info.vcpu_info[0].time.tsc_to_system_mul;
+tsc_shift = shared_info.vcpu_info[0].time.tsc_shift;
+tsc = rdtsc_ordered();
+smp_rmb();


I don't think you need the barrier here if you use rdtsc_ordered, but
I would like confirmation from someone else.


+
+ver2 = ACCESS_ONCE(shared_info.vcpu_info[0].time.version);
+} while ( ver2 & 1 || ver1 != ver2 );
+
+
+system_time += scale_delta(tsc - tsc_timestamp,
+   

Re: [Xen-devel] [PATCH v4 7/7] add sleep, msleep and NOW() macros to time manager

[Paul Semel]

On 04/13/2018 03:55 PM, Roger Pau Monné wrote:

those are helpful macro to use the time manager correctly

Signed-off-by: Paul Semel 
---

Notes:
 v4:
 - new patch version

  common/time.c  | 10 ++
  include/xtf/time.h | 12 
  2 files changed, 22 insertions(+)

diff --git a/common/time.c b/common/time.c
index 7515eb0..e2779b9 100644
--- a/common/time.c
+++ b/common/time.c
@@ -163,6 +163,16 @@ static inline void mspin_sleep(uint64_t t)
  nspin_sleep(nsec);
  }
  
+void sleep(uint64_t t)

+{
+spin_sleep(t);
+}
+
+void msleep(uint64_t t)
+{
+mspin_sleep(t);


Why can you just call mspin_sleep msleep directly?

The same applies to spin_sleep.

Also I was expecting to see some kind of test appear at the end of the
series. You are basically adding a bunch of dead code, since there's
no user of any of the newly introduced functions.


Actually, I won't be able to add a real XSA test using this feature. Anyway, I 
do think that having the sleep functions will be really useful in the future.
Anyway, I was thinking about adding a test that is calling the gettimeofday 
function or something similar.


What do you think about it ?

--
Paul Semel

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

Re: [Xen-devel] [PATCH 3/3] x86: check feature flags after resume

[Jan Beulich]

>>> On 13.04.18 at 20:56,  wrote:
> Simon Gaiser:
>> Jan Beulich:
>>> Make sure no previously present features are missing after resume (and
>>> the re-loading of microcode), to avoid later crashes or (likely silent)
>>> hangs / live locks. This doesn't go beyond checking x86_capability[],
>>> but this should be good enough for the immediate need of making sure
>>> that the BIT mitigation MSRs are still available.
>>>
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/xen/arch/x86/acpi/power.c
>>> +++ b/xen/arch/x86/acpi/power.c
>>> @@ -254,6 +254,9 @@ static int enter_state(u32 state)
>>>  
>>>  microcode_resume_cpu(0);
>>>  
>>> +if ( !recheck_cpu_features(0) )
>>> +panic("Missing previously available feature(s).");
>>> +
>>>  ci->bti_ist_info = default_bti_ist_info;
>>>  asm volatile (ALTERNATIVE("", "wrmsr", X86_FEATURE_XEN_IBRS_SET)
>>>:: "a" (SPEC_CTRL_IBRS), "c" (MSR_SPEC_CTRL), "d" (0)
>>> --- a/xen/arch/x86/cpu/common.c
>>> +++ b/xen/arch/x86/cpu/common.c
>>> @@ -501,6 +501,9 @@ void identify_cpu(struct cpuinfo_x86 *c)
>>> printk("\n");
>>>  #endif
>>>  
>>> +   if (system_state == SYS_STATE_resume)
>>> +   return;
>>> +
>>> /*
>>>  * On SMP, boot_cpu_data holds the common feature set between
>>>  * all CPUs; so make sure that we indicate which features are
>>> --- a/xen/arch/x86/cpuid.c
>>> +++ b/xen/arch/x86/cpuid.c
>>> @@ -473,6 +473,28 @@ void __init init_guest_cpuid(void)
>>>  calculate_hvm_max_policy();
>>>  }
>>>  
>>> +bool recheck_cpu_features(unsigned int cpu)
>>> +{
>>> +bool okay = true;
>>> +struct cpuinfo_x86 c;
>>> +const struct cpuinfo_x86 *bsp = _cpu_data;
>>> +unsigned int i;
>>> +
>>> +identify_cpu();
>> 
>> This runs into a bug in identify_cpu(). x86_vendor_id does not get
>> zeroed, so the x86_vendor_id is not null terminated and the vendor
>> identification fails.
>> 
>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>> index 4feaa2ceb6..5750d26216 100644
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -366,8 +366,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
>> c->x86_vendor = X86_VENDOR_UNKNOWN;
>> c->cpuid_level = -1;/* CPUID not detected */
>> c->x86_model = c->x86_mask = 0; /* So far unknown... */
>> -   c->x86_vendor_id[0] = '\0'; /* Unset */
>> -   c->x86_model_id[0] = '\0';  /* Unset */
>> +   memset(>x86_vendor_id, 0, sizeof(c->x86_vendor_id));
>> +   memset(>x86_model_id, 0, sizeof(c->x86_model_id));
>> c->x86_max_cores = 1;
>> c->x86_num_siblings = 1;
>> c->x86_clflush_size = 0;
>> 
>> With this patch it works for me.
> 
> Meh, also a backport failure from me. Since e34bc403c3c7 this problem
> should not appear since it does not assume a null terminated string.

NP - it's good to be aware of such issues in case we as well decide to
backport this.

Thanks for the feedback,
Jan



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

Re: [Xen-devel] [PATCH 3/7] xen/arm/psci: Implement CPU_OFF PSCI call (physical interface)

[Mirela Simonovic]

Hi Julien,


On Thu, Apr 12, 2018 at 3:31 PM, Julien Grall  wrote:
>
>
> On 12/04/18 12:33, Mirela Simonovic wrote:
>>
>> On Wed, Apr 11, 2018 at 4:46 PM, Julien Grall 
>> wrote:
>>>
>>> On 11/04/18 14:19, Mirela Simonovic wrote:
>>>
local_irq_disable();
cpu_is_dead = true;
/* Make sure the write happens before we sleep forever */
dsb(sy);
isb();
 +/* PSCI cpu off call will return only in case of an error */
 +errno = call_psci_cpu_off();
 +printk(XENLOG_DEBUG "PSCI cpu off call failed for CPU#%d err=%d\n",
 +   get_processor_id(), errno);
 +isb();
>>>
>>>
>>>
>>> What are you trying to achieve with the isb() here?
>>>
>>
>> I use to have a problem that the wfi below gets executed before the
>> call_psci_cpu_off(). Adding isb() fixed the issue. However, I tried
>> now to reproduce the problem and it doesn't show up. I still believe
>> isb() should be here, please let me know if you disagree (I obviously
>> can't prove the claim now).
>
>
> The problem you describe can't be possible with the code you have because
> call_psci_cpu_off() is issuing a SMC. SMC will lead to change exception
> level and therefore have a context-synchronization barrier.
>
> This is obviously based on the assumption you don't have an errata on your
> CPU exposing the behavior you describe. For that you would need to check
> errata notice for your CPU and/or try to reproduce.
>
> However, what you would need is a dsb(sy); isb(); to drain the write buffer
> if you print a message.
>
> Furthermore, now on platform without CPU off support (e.g non-PSCI platform
> and PSCI 0.1) you will log an error message that may worry people. In
> reality, PSCI cpu_off will unlikely fail, so you probably want to add a
> panic in call_psci_cpu_off instead.
>

Even if PSCI cpu_off call fails, what is unlikely to happen, the
system is still functional. Enabling that pCPU later will fail, but
Xen can handle this error and continue running properly on the boot
pCPU (I've tested this in 2 pCPUs config).
Therefore, I believe panic may not be necessary in this case. I
suggest that we dump the error message and continue to run. Please let
me know if you disagree.

Thanks,
Mirela

> Cheers,
>
> --
> Julien Grall

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

Re: [Xen-devel] [PATCH v4 3/7] add gettimeofday function to time managment

[Paul Semel]

Hi !

Thanks a lot for reviewing !

On 04/13/2018 03:39 PM, Roger Pau Monné wrote:

On Tue, Apr 10, 2018 at 09:16:57PM +0200, Paul Semel wrote:

this function acts as the POSIX gettimeofday function

Signed-off-by: Paul Semel 
---

Notes:
 v4:
 - new patch version

  common/time.c  | 30 ++
  include/xtf/time.h |  8 
  2 files changed, 38 insertions(+)

diff --git a/common/time.c b/common/time.c
index c1b7cd1..8489f3b 100644
--- a/common/time.c
+++ b/common/time.c
@@ -1,6 +1,7 @@
  #include 
  #include 
  #include 
+#include 


Sorting.

  
  #include 

  #include 
@@ -109,6 +110,35 @@ uint64_t current_time(void)
  return sec + boot_time;
  }
  
+/* The POSIX gettimeofday syscall normally takes a second argument, which is

+ * the timezone (struct timezone). However, it sould be NULL because linux
+ * doesn't use it anymore. So we need for us to add it in this function
+ */
+int gettimeofday(struct timeval *tp, void *restrict tzp)
+{
+uint64_t boot_time, sec;
+uint32_t mod, nsec;
+
+if ( tzp != NULL )
+return -EOPNOTSUPP;
+
+if ( tp == NULL )
+return -EINVAL;
+
+get_time_info(_time, , );


Why are you using get_time_info here? Shouldn't you use the
current_time function introduced in the previous patch?

Or else I don't see the need to introduce current_time in the previous
patch.



Actually, I can't use *only* the current_time function here, because I won't be 
able to get the nanoseconds if so.


Anyway, in the last patch, I am using current_time function for the NOW() macro, 
which I think is really helpful.


Do you think I should drop all of those ?


+#if defined(__i386__)
+mod = divmod64(_time, SEC_TO_NSEC(1));
+#else
+mod = boot_time % SEC_TO_NSEC(1);
+boot_time /= SEC_TO_NSEC(1);
+#endif


Please use divmod64 unconditionally.


+
+tp->sec = sec + boot_time;
+tp->nsec = nsec + mod;
+return 0;
+}
+
  /*
   * Local variables:
   * mode: C
diff --git a/include/xtf/time.h b/include/xtf/time.h
index e33dc8a..ce4d6db 100644
--- a/include/xtf/time.h
+++ b/include/xtf/time.h
@@ -8,6 +8,12 @@
  
  #include 
  
+struct timeval {

+uint64_t sec;
+uint64_t nsec;
+};
+
+


Extra newline.


Thanks,

--
Paul

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

Re: [Xen-devel] [PATCH] x86/xen: Remove use of VLAs

[Ingo Molnar]

* Laura Abbott  wrote:

> There's an ongoing effort to remove VLAs[1] from the kernel to eventually
> turn on -Wvla. The few VLAs in use have an upper bound based on a size
> of 64K. This doesn't produce an excessively large stack so just switch
> the upper bound.
> 
> [1] https://lkml.org/lkml/2018/3/7/621
> 
> Signed-off-by: Laura Abbott 
> ---
>  arch/x86/xen/enlighten_pv.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index c36d23aa6c35..d96a5a535cbb 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -421,8 +421,7 @@ static void xen_load_gdt(const struct desc_ptr *dtr)
>  {
>   unsigned long va = dtr->address;
>   unsigned int size = dtr->size + 1;
> - unsigned pages = DIV_ROUND_UP(size, PAGE_SIZE);
> - unsigned long frames[pages];
> + unsigned long frames[DIV_ROUND_UP(SZ_64K, PAGE_SIZE)];
>   int f;
>  
>   /*
> @@ -470,8 +469,7 @@ static void __init xen_load_gdt_boot(const struct 
> desc_ptr *dtr)
>  {
>   unsigned long va = dtr->address;
>   unsigned int size = dtr->size + 1;
> - unsigned pages = DIV_ROUND_UP(size, PAGE_SIZE);
> - unsigned long frames[pages];
> + unsigned long frames[DIV_ROUND_UP(SZ_64K, PAGE_SIZE)];
>   int f;

Reviewed-by: Ingo Molnar 

Thanks,

Ingo

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

Re: [Xen-devel] [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot

[Daniel Kiper]

On Mon, Apr 16, 2018 at 10:15:15AM +0200, Ard Biesheuvel wrote:
> On 11 April 2018 at 10:56, Daniel Kiper  wrote:
> > On Wed, Apr 04, 2018 at 12:38:24PM +0200, Daniel Kiper wrote:
> >> On Tue, Apr 03, 2018 at 10:00:52AM -0700, James Bottomley wrote:
> >> > On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote:
> >> > > On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote:
> >>
> >> [...]
> >>
> >> > > > This looks like a bad idea: you're duplicating the secure boot
> >> > > > check in
> >> > > >
> >> > > > drivers/firmware/efi/libstub/secureboot.c
> >> > > >
> >> > > > Which is an implementation of policy.  If we have to have policy in
> >> > > > the kernel, it should really only be in one place to prevent drift;
> >> > > > why can't you simply use the libstub efi_get_secureboot() so we're
> >> > > > not duplicating the implementation of policy?
> >> > >
> >> > > Well, here is the first version of this patch:
> >> > > https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not
> >> > > happy too. In general both approaches are not perfect. More you can
> >> > > find in the discussion around this patchset. If you have better idea
> >> > > how to do that I am happy to implement it.
> >> >
> >> > One way might be simply to have the pre exit-boot-services code lay
> >> > down a variable containing the state which you pick up, rather than you
> >>
> >> Do you mean variable in kernel proper or something like that? If yes this
> >> is not possible. EFI Linux stub is not executed in Xen dom0. All UEFI
> >> infrastructure is owned and operated by Xen. Dom0 kernel can access some
> >> stuff in UEFI, including variables, via hypercall. However, when dom0
> >> runs only UEFI runtime services are available.
> >>
> >> > calling efi code separately and trying to use the insecure RT
> >>
> >> I am not sure why they are insecure.
> >>
> >> > variables.  That way there's a uniform view of the internal kernel
> >> > secure boot state that everyone can use.
> >>
> >> That would be perfect but I have a feeling that in form proposed above
> >> it is not possible.
> >
> > Ping?
> >
>
> (apologies if this is a duplicate email - I thought I had replied
> already but I don't see it in my sent folder)
>
> Queued in efi/next - thanks.

Thanks a lot!

Daniel

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

Re: [Xen-devel] [PATCH] tools/kdd: silence gcc 8 warning a different way

[Wei Liu]

Cc Tim

On Thu, Apr 12, 2018 at 06:04:49AM -0600, Jan Beulich wrote:
> Older gcc doesn't like "#pragma GCC diagnostic" inside functions.
> 
> Signed-off-by: Jan Beulich 
> 
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -695,10 +695,10 @@ static void kdd_handle_read_ctrl(kdd_sta
>  KDD_LOG(s, "Request outside of known control space\n");
>  len = 0;
>  } else {
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Warray-bounds"
> -memcpy(buf, ((uint8_t *)) + offset, len);
> -#pragma GCC diagnostic pop
> +/* Suppress bogus gcc 8 "out of bounds" warning. */
> +const uint8_t *src;
> +asm ("" : "=g" (src) : "0" ((uint8_t *) + offset));
> +memcpy(buf, src, len);
>  }
>  }
>  
> 
> 
> 

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

[Xen-devel] [PATCH] x86/hpet: add a lock when cpu clear cpumask in hpet_broadcast_exit();

[Davidwang]

From: David Wang 

By the hpet_get_channel(), cpus share an in-use channel somtime.
So, core shouldn't clear cpumask while others are getting first
cpumask. If core zero and core one share an channel, the cpumask
is 0x3. Core zero clear cpumask between core one executing
cpumask_empty() and cpumask_first(). The return of cpumask_first()
is nr_cpu_ids. That would lead to ASSERT(cpu < nr_cpu_ids).

Signed-off-by: David Wang 
---
 xen/arch/x86/hpet.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index bc7a851..69a7b3a 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -740,7 +740,9 @@ void hpet_broadcast_exit(void)
 if ( !reprogram_timer(deadline) )
 raise_softirq(TIMER_SOFTIRQ);
 
+spin_lock_irq(>lock);
 cpumask_clear_cpu(cpu, ch->cpumask);
+spin_unlock_irq(>lock);
 
 if ( !(ch->flags & HPET_EVT_LEGACY) )
 hpet_detach_channel(cpu, ch);
-- 
2.7.4


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

Re: [Xen-devel] [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot

[Ard Biesheuvel]

On 11 April 2018 at 10:56, Daniel Kiper  wrote:
> On Wed, Apr 04, 2018 at 12:38:24PM +0200, Daniel Kiper wrote:
>> On Tue, Apr 03, 2018 at 10:00:52AM -0700, James Bottomley wrote:
>> > On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote:
>> > > On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote:
>>
>> [...]
>>
>> > > > This looks like a bad idea: you're duplicating the secure boot
>> > > > check in
>> > > >
>> > > > drivers/firmware/efi/libstub/secureboot.c
>> > > >
>> > > > Which is an implementation of policy.  If we have to have policy in
>> > > > the kernel, it should really only be in one place to prevent drift;
>> > > > why can't you simply use the libstub efi_get_secureboot() so we're
>> > > > not duplicating the implementation of policy?
>> > >
>> > > Well, here is the first version of this patch:
>> > > https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not
>> > > happy too. In general both approaches are not perfect. More you can
>> > > find in the discussion around this patchset. If you have better idea
>> > > how to do that I am happy to implement it.
>> >
>> > One way might be simply to have the pre exit-boot-services code lay
>> > down a variable containing the state which you pick up, rather than you
>>
>> Do you mean variable in kernel proper or something like that? If yes this
>> is not possible. EFI Linux stub is not executed in Xen dom0. All UEFI
>> infrastructure is owned and operated by Xen. Dom0 kernel can access some
>> stuff in UEFI, including variables, via hypercall. However, when dom0
>> runs only UEFI runtime services are available.
>>
>> > calling efi code separately and trying to use the insecure RT
>>
>> I am not sure why they are insecure.
>>
>> > variables.  That way there's a uniform view of the internal kernel
>> > secure boot state that everyone can use.
>>
>> That would be perfect but I have a feeling that in form proposed above
>> it is not possible.
>
> Ping?
>

(apologies if this is a duplicate email - I thought I had replied
already but I don't see it in my sent folder)

Queued in efi/next - thanks.

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

Re: [Xen-devel] [PATCH] x86/xen: Remove use of VLAs

[Juergen Gross]

On 14/04/18 00:11, Laura Abbott wrote:
> There's an ongoing effort to remove VLAs[1] from the kernel to eventually
> turn on -Wvla. The few VLAs in use have an upper bound based on a size
> of 64K. This doesn't produce an excessively large stack so just switch
> the upper bound.
> 
> [1] https://lkml.org/lkml/2018/3/7/621
> 
> Signed-off-by: Laura Abbott 

Reviewed-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [PATCH] xen: Change return type to vm_fault_t

[Juergen Gross]

On 14/04/18 21:15, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler
> in struct vm_operations_struct.
> 
> Signed-off-by: Souptick Joarder 
> Reviewed-by: Matthew Wilcox 

Reviewed-by: Juergen Gross 


Juergen


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

[Xen-devel] [linux-linus test] 122317: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-multivcpu 10 debian-install  fail REGR. vs. 118324
 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 118324
 test-armhf-armhf-xl-cubietruck 10 debian-install fail REGR. vs. 118324
 test-armhf-armhf-xl-credit2  10 debian-install fail in 122301 REGR. vs. 118324

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-xl   7 xen-boot fail in 122301 pass in 122317
 test-armhf-armhf-xl  11 debian-fixup fail in 122301 pass in 122317
 test-armhf-armhf-libvirt-xsm 10 debian-install   fail in 122301 pass in 122317
 test-armhf-armhf-xl-credit2   7 xen-boot   fail pass in 122301
 test-armhf-armhf-xl-arndale  10 debian-install fail pass in 122301

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-arndale 13 migrate-support-check fail in 122301 never pass
 test-armhf-armhf-xl-arndale 14 saverestore-support-check fail in 122301 never 
pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 118324
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 118324
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 118324
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 118324
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 118324
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 118324
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 118324
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 118324
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux18b7fd1c93e5204355ddbf2608a097d64df81b88
baseline version:
 linux5b7d27967dabfb17c21b0d98b29153b9e3ee71e5

Last test of basis   118324  2018-01-25 07:31:24 Z   80 days
Failing since118362  2018-01-26 16:56:17 Z   79 days   68 attempts
Testing same since   122301  2018-04-15 05:37:18 Z1 days2 attempts


3296 people touched revisions under test,
not