Re: [Xen-devel] [PATCH v3 0/5] sndif: add explicit back and front synchronization

[Oleksandr Andrushchenko]

Hi, Konrad!

Could you please review?

Thank you,
Oleksandr

On 03/21/2018 09:25 AM, Oleksandr Andrushchenko wrote:

On 03/21/2018 09:20 AM, Takashi Iwai wrote:

On Wed, 21 Mar 2018 08:15:36 +0100,
Oleksandr Andrushchenko wrote:

On 03/20/2018 10:22 PM, Takashi Iwai wrote:

On Mon, 19 Mar 2018 08:22:19 +0100,
Oleksandr Andrushchenko wrote:

From: Oleksandr Andrushchenko 

Hello, all!

In order to provide explicit synchronization between backend and
frontend the following changes are introduced in the protocol:
   - bump protocol version to 2
   - add new ring buffer for sending asynchronous events from
 backend to frontend to report number of bytes played by the
 frontend (XENSND_EVT_CUR_POS)
   - introduce trigger events for playback control: 
start/stop/pause/resume

   - add "req-" prefix to event-channel and ring-ref to unify naming
 of the Xen event channels for requests and events
   - add XENSND_OP_HW_PARAM_QUERY request to read/update
 stream configuration space: request passes desired 
intervals/formats for
 the stream parameters and the response returns allowed 
intervals and

 formats mask that can be used.

Changes since v2:
1. Konrad's r-b tag for version patch
2. MAJOR: changed req/resp/evt packet sizes from 32 to 64 octets
3. Reworked XENSND_OP_HW_PARAM_QUERY so it now sends all
 parameters at once, allowing to check all the configuration
 space.
4. Minor documentation cleanup (added missed "reserved" fields)

Changes since v1:

1. Changed protocol version definition from string to integer,
so it can easily be used in comparisons.
Konrad, I have removed your r-b tag for the reason of this change.

2. In order to provide explicit stream parameter negotiation between
backend and frontend the following changes are introduced in the 
protocol:

add XENSND_OP_HW_PARAM_QUERY request to read/update
configuration space for the parameter given: request passes
desired parameter interval (mask) and the response to this request
returns min/max interval (mask) for the parameter to be used.

Parameters supported by this request/response:
   - format mask
   - sample rate interval
   - number of channels interval
   - buffer size, interval, frames
   - period size, interval, frames

I can't judge exactly about the protocol without the actual FE/BE
implementations, but the change looks good to me, especially if you've
already tested something.

Thank you, I have tested the changes and need them to start upstreaming
the frontend driver used to test the protocol.
Do you mind if I put your Acked-by (or you prefer Reviewed-by?) tag to
these patches:

[PATCH v3 4/5] sndif: Add explicit back and front synchronization
[PATCH v3 5/5] sndif: Add explicit back and front parameter negotiation

Sure, feel free to take my ack:
   Reviewed-by: Takashi Iwai 

Thank you


Takashi


Please note, that the changes first to be merged into Xen and then
I'll prepare
the same, but for the kernel

If other people have no concern, let's go ahead with FE/BE stuff.

Konrad, are you ok with the changes?

thanks,

Takashi

Thank you,
Oleksandr






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

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

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf dd190645eb43424706eb1709d0032c69a1935d9f
baseline version:
 ovmf 65e0e10d23323b18f31bf9aa9eef3c2434f53780

Last test of basis   121170  2018-03-24 12:09:46 Z2 days
Testing same since   121281  2018-03-26 01:44:33 Z1 days1 attempts


People who touched revisions under test:
  Ruiyu Ni 

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
   65e0e10d23..dd190645eb  dd190645eb43424706eb1709d0032c69a1935d9f -> 
xen-tested-master

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

[Xen-devel] [PATCH v2] x86/boot: Disable IBRS in intr/nmi exit path at bootup stage

[Zhenzhong Duan]

After reset, IBRS is disabled by processor, but a coming intr/nmi leave IBRS
enabled after their exit. It's not necessory for bootup code to run in low
performance with IBRS enabled.

On ORACLE X6-2(500GB/88 cpus, dom0 11GB/20 vcpus), we observed an 200s+ delay
in construct_dom0.

By initializing use_shadow_spec_ctrl with the result of (system_state <
SYS_STATE_active), IBRS is disabled in intr/nmi exit path at bootup stage.
Then delay in construct_dom0 is ~50s.

When hot-onlining a CPU, we initialize IBRS early and set use_shadow_spec_ctrl
to false to avoid Branch Target Injection from sibling threads.

v2: Use (system_state < SYS_STATE_active) to initialize use_shadow_spec_ctrl
instead of literal 1 per Jan.

Signed-off-by: Zhenzhong Duan 
---
 xen/include/asm-x86/spec_ctrl.h |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 5ab4ff3..1672317 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -32,8 +32,22 @@ extern uint8_t default_bti_ist_info;
 static inline void init_shadow_spec_ctrl_state(void)
 {
 struct cpu_info *info = get_cpu_info();
+uint32_t val = SPEC_CTRL_IBRS;
+
+/* Initialize IA32_SPEC_CTRL MSR for hotplugging cpu early */
+if ( system_state >= SYS_STATE_active )
+asm volatile (ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
+  :: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory");
 
-info->shadow_spec_ctrl = info->use_shadow_spec_ctrl = 0;
+info->shadow_spec_ctrl = 0;
+/*
+ * We want to make sure we clear IBRS in interrupt exit path
+ * (DO_SPEC_CTRL_EXIT_TO_XEN) while dom0 is still booting to
+ * avoid unnecessary performance impact. As soon as dom0 has
+ * booted use_shadow_spec_ctrl will be cleared, for example,
+ * in idle routine.
+ */
+info->use_shadow_spec_ctrl = system_state < SYS_STATE_active;
 info->bti_ist_info = default_bti_ist_info;
 }
 
-- 
1.7.3

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-qemuu-nested-intel 14 xen-boot/l1   fail REGR. vs. 120095
 test-amd64-amd64-qemuu-nested-amd 14 xen-boot/l1 fail REGR. vs. 120095

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 120095
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 120095
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 120095
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 120095
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 120095
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 120095
 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-amd64-libvirt 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-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-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-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-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-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
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-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-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-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-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-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 qemuu7b1db0908d88f0c9cfac24e214ff72a860692e23
baseline version:
 qemuu6697439794f72b3501ee16bb95d16854f9981421

Last test of basis   120095  2018-02-28 13:46:33 Z   26 days
Failing since120146  2018-03-02 10:10:57 Z   24 days   14 attempts
Testing same since   121275  2018-03-25 19:43:31 Z1 days1 attempts


People who touched revisions under test:
  Alberto Garcia 
  Alex Bennée 
  Alex Bennée 
  Alex Williamson 
  Alexey Kardashevskiy 
  Alistair Francis 
  Alistair Francis 
  Andrew Jones 
  Andrey Smirnov 
  Anton 

Re: [Xen-devel] [PATCH] x86/boot: Disable IBRS in intr/nmi exit path at bootup stage

[Zhenzhong Duan]

在 2018/3/26 21:39, Jan Beulich 写道:

On 21.03.18 at 03:58,  wrote:

After reset, IBRS is disabled by processor, but a coming intr/nmi leave IBRS
enabled after their exit. It's not necessory for bootup code to run in low
performance with IBRS enabled.

On ORACLE X6-2(500GB/88 cpus, dom0 11GB/20 vcpus), we observed an 200s+ delay
in construct_dom0.

By initializing use_shadow_spec_ctrl with 1, IBRS is disabled in intr/nmi exit
path at bootup stage. Then delay in construct_dom0 is ~50s.


While I can certainly follow the argumentation, did you pay
attention to Andrew also modifying what you would call "bootup
code" in commit 7c508612f7 ("x86: Support indirect thunks from
assembly code")? That wasn't just a random change - we
specifically want it for the case of bringing up CPUs at runtime.
You'll need to be equally careful here, I think: Rather than
storing literal 1 (which should have been "true" anyway), you'll
want to store (system_state < SYS_STATE_active) or maybe
(system_state != SYS_STATE_active), at least when the CPU
being booted is a hyperthread of a CPU which is active already.

Make sense for me, thank you for explanation. I'll send a new patch.

Regards
Zhenzhong

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 120859
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 120943
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 120943
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 120943
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 120943
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 120943
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 120943
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 120943
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 120943
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 120943
 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-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-amd64-i386-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
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-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-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  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
 test-armhf-armhf-libvirt-xsm 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 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-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 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  eabb83121226d5a6a5a68da3a913ac0b5bb1e0cf
baseline version:
 xen  0012ae8afb4a6e76f2847119f2c6850fbf41d9b7

Last test of basis   120943  2018-03-18 21:56:54 Z8 days
Failing since120988  2018-03-20 10:55:25 Z6 days4 attempts
Testing same since   121145  2018-03-24 07:52:53 Z2 days2 attempts


People who touched revisions under test:
  Amit Singh Tomar 
  Andre Przywara 

Re: [Xen-devel] [PATCH v3 24/39] ARM: new VGIC: Add TARGET registers handlers

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> The target register handlers are v2 emulation specific, so their
> implementation lives entirely in vgic-mmio-v2.c.
> We copy the old VGIC behaviour of assigning an IRQ to the first VCPU
> set in the target mask instead of making it possibly pending on
> multiple VCPUs.
> We update the physical affinity of a hardware mapped vIRQ on the way.
> 
> This is based on Linux commit 2c234d6f1826, written by Andre Przywara.
> 
> Signed-off-by: Andre Przywara 
> Reviewed-by: Julien Grall 

Acked-by: Stefano Stabellini 


> ---
>  xen/arch/arm/vgic/vgic-mmio-v2.c | 59 
> +++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vgic/vgic-mmio-v2.c 
> b/xen/arch/arm/vgic/vgic-mmio-v2.c
> index a28d0e459b..b333de9ed7 100644
> --- a/xen/arch/arm/vgic/vgic-mmio-v2.c
> +++ b/xen/arch/arm/vgic/vgic-mmio-v2.c
> @@ -81,6 +81,63 @@ static void vgic_mmio_write_v2_misc(struct vcpu *vcpu,
>  }
>  }
>  
> +static unsigned long vgic_mmio_read_target(struct vcpu *vcpu,
> +   paddr_t addr, unsigned int len)
> +{
> +uint32_t intid = VGIC_ADDR_TO_INTID(addr, 8);
> +uint32_t val = 0;
> +unsigned int i;
> +
> +for ( i = 0; i < len; i++ )
> +{
> +struct vgic_irq *irq = vgic_get_irq(vcpu->domain, vcpu, intid + i);
> +
> +val |= (uint32_t)irq->targets << (i * 8);
> +
> +vgic_put_irq(vcpu->domain, irq);
> +}
> +
> +return val;
> +}
> +
> +static void vgic_mmio_write_target(struct vcpu *vcpu,
> +   paddr_t addr, unsigned int len,
> +   unsigned long val)
> +{
> +uint32_t intid = VGIC_ADDR_TO_INTID(addr, 8);
> +uint8_t cpu_mask = GENMASK(vcpu->domain->max_vcpus - 1, 0);
> +unsigned int i;
> +unsigned long flags;
> +
> +/* GICD_ITARGETSR[0-7] are read-only */
> +if ( intid < VGIC_NR_PRIVATE_IRQS )
> +return;
> +
> +for ( i = 0; i < len; i++ )
> +{
> +struct vgic_irq *irq = vgic_get_irq(vcpu->domain, NULL, intid + i);
> +
> +spin_lock_irqsave(>irq_lock, flags);
> +
> +irq->targets = (val >> (i * 8)) & cpu_mask;
> +if ( irq->targets )
> +{
> +irq->target_vcpu = vcpu->domain->vcpu[ffs(irq->targets) - 1];
> +if ( irq->hw )
> +{
> +struct irq_desc *desc = irq_to_desc(irq->hwintid);
> +
> +irq_set_affinity(desc, 
> cpumask_of(irq->target_vcpu->processor));
> +}
> +}
> +else
> +irq->target_vcpu = NULL;
> +
> +spin_unlock_irqrestore(>irq_lock, flags);
> +vgic_put_irq(vcpu->domain, irq);
> +}
> +}
> +
>  static const struct vgic_register_region vgic_v2_dist_registers[] = {
>  REGISTER_DESC_WITH_LENGTH(GICD_CTLR,
>  vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12,
> @@ -110,7 +167,7 @@ static const struct vgic_register_region 
> vgic_v2_dist_registers[] = {
>  vgic_mmio_read_priority, vgic_mmio_write_priority, 8,
>  VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>  REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_ITARGETSR,
> -vgic_mmio_read_raz, vgic_mmio_write_wi, 8,
> +vgic_mmio_read_target, vgic_mmio_write_target, 8,
>  VGIC_ACCESS_32bit | VGIC_ACCESS_8bit),
>  REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_ICFGR,
>  vgic_mmio_read_config, vgic_mmio_write_config, 2,
> -- 
> 2.14.1
> 

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

Re: [Xen-devel] [PATCH v3a 14/39] ARM: new VGIC: Add GICv2 world switch backend

[Stefano Stabellini]

On Tue, 27 Mar 2018, Julien Grall wrote:
> Hi,
> Sorry for the formatting.
> 
> On Tue, 27 Mar 2018, 07:25 Stefano Stabellini,  wrote:
>   On Thu, 22 Mar 2018, Andre Przywara wrote:
>   > Processing maintenance interrupts and accessing the list registers
>   > are dependent on the host's GIC version.
>   > Introduce vgic-v2.c to contain GICv2 specific functions.
>   > Implement the GICv2 specific code for syncing the emulation state
>   > into the VGIC registers.
>   > This also adds the hook to let Xen setup the host GIC addresses.
>   >
>   > This is based on Linux commit 140b086dd197, written by Marc Zyngier.
>   >
>   > Signed-off-by: Andre Przywara 
>   > ---
>   > Changelog v3 ... v3a:
>   > - take hardware IRQ lock in vgic_v2_fold_lr_state()
>   > - fix last remaining u32 usage
>   > - print message when using new VGIC
>   > - add TODO about racy _IRQ_INPROGRESS setting
>   >
>   > Changelog v2 ... v3:
>   > - remove no longer needed asm/io.h header
>   > - replace 0/1 with false/true for bool's
>   > - clear _IRQ_INPROGRESS bit when retiring hardware mapped IRQ
>   > - fix indentation and w/s issues
>   >
>   > Changelog v1 ... v2:
>   > - remove v2 specific underflow function (now generic)
>   > - re-add Linux code to properly handle acked level IRQs
>   >
>   >  xen/arch/arm/vgic/vgic-v2.c | 259 
> 
>   >  xen/arch/arm/vgic/vgic.c    |   6 +
>   >  xen/arch/arm/vgic/vgic.h    |   9 ++
>   >  3 files changed, 274 insertions(+)
>   >  create mode 100644 xen/arch/arm/vgic/vgic-v2.c
>   >
>   > diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
>   > new file mode 100644
>   > index 00..1773503cfb
>   > --- /dev/null
>   > +++ b/xen/arch/arm/vgic/vgic-v2.c
>   > @@ -0,0 +1,259 @@
>   > +/*
>   > + * Copyright (C) 2015, 2016 ARM Ltd.
>   > + * Imported from Linux ("new" KVM VGIC) and heavily adapted to Xen.
>   > + *
>   > + * This program is free software; you can redistribute it and/or 
> modify
>   > + * it under the terms of the GNU General Public License version 2 as
>   > + * published by the Free Software Foundation.
>   > + *
>   > + * This program is distributed in the hope that it will be useful,
>   > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>   > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>   > + * GNU General Public License for more details.
>   > + *
>   > + * You should have received a copy of the GNU General Public License
>   > + * along with this program.  If not, see 
> .
>   > + */
>   > +
>   > +#include 
>   > +#include 
>   > +#include 
>   > +#include 
>   > +#include 
>   > +
>   > +#include "vgic.h"
>   > +
>   > +static struct {
>   > +    bool enabled;
>   > +    paddr_t dbase;          /* Distributor interface address */
>   > +    paddr_t cbase;          /* CPU interface address & size */
>   > +    paddr_t csize;
>   > +    paddr_t vbase;          /* Virtual CPU interface address */
>   > +
>   > +    /* Offset to add to get an 8kB contiguous region if GIC is 
> aliased */
>   > +    uint32_t aliased_offset;
>   > +} gic_v2_hw_data;
>   > +
>   > +void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t csize,
>   > +                      paddr_t vbase, uint32_t aliased_offset)
>   > +{
>   > +    gic_v2_hw_data.enabled = true;
>   > +    gic_v2_hw_data.dbase = dbase;
>   > +    gic_v2_hw_data.cbase = cbase;
>   > +    gic_v2_hw_data.csize = csize;
>   > +    gic_v2_hw_data.vbase = vbase;
>   > +    gic_v2_hw_data.aliased_offset = aliased_offset;
>   > +
>   > +    printk("Using the new VGIC implementation.\n");
>   > +}
>   > +
>   > +/*
>   > + * transfer the content of the LRs back into the corresponding 
> ap_list:
>   > + * - active bit is transferred as is
>   > + * - pending bit is
>   > + *   - transferred as is in case of edge sensitive IRQs
>   > + *   - set to the line-level (resample time) for level sensitive IRQs
>   > + */
>   > +void vgic_v2_fold_lr_state(struct vcpu *vcpu)
>   > +{
>   > +    struct vgic_cpu *vgic_cpu = >arch.vgic;
>   > +    unsigned int used_lrs = vcpu->arch.vgic.used_lrs;
>   > +    unsigned long flags;
>   > +    unsigned int lr;
>   > +
>   > +    if ( !used_lrs )    /* No LRs used, so nothing to sync back 
> here. */
>   > +        return;
>   > +
>   > +    gic_hw_ops->update_hcr_status(GICH_HCR_UIE, false);
>   > +
>   > +    for ( lr = 0; lr < used_lrs; lr++ )
>   > +    {
>   > +        

Re: [Xen-devel] [PATCH v3a 14/39] ARM: new VGIC: Add GICv2 world switch backend

[Stefano Stabellini]

On Mon, 26 Mar 2018, Stefano Stabellini wrote:
> On Thu, 22 Mar 2018, Andre Przywara wrote:
> > Processing maintenance interrupts and accessing the list registers
> > are dependent on the host's GIC version.
> > Introduce vgic-v2.c to contain GICv2 specific functions.
> > Implement the GICv2 specific code for syncing the emulation state
> > into the VGIC registers.
> > This also adds the hook to let Xen setup the host GIC addresses.
> > 
> > This is based on Linux commit 140b086dd197, written by Marc Zyngier.
> > 
> > Signed-off-by: Andre Przywara 
> > ---
> > Changelog v3 ... v3a:
> > - take hardware IRQ lock in vgic_v2_fold_lr_state()
> > - fix last remaining u32 usage
> > - print message when using new VGIC
> > - add TODO about racy _IRQ_INPROGRESS setting
> > 
> > Changelog v2 ... v3:
> > - remove no longer needed asm/io.h header
> > - replace 0/1 with false/true for bool's
> > - clear _IRQ_INPROGRESS bit when retiring hardware mapped IRQ
> > - fix indentation and w/s issues
> > 
> > Changelog v1 ... v2:
> > - remove v2 specific underflow function (now generic)
> > - re-add Linux code to properly handle acked level IRQs
> > 
> >  xen/arch/arm/vgic/vgic-v2.c | 259 
> > 
> >  xen/arch/arm/vgic/vgic.c|   6 +
> >  xen/arch/arm/vgic/vgic.h|   9 ++
> >  3 files changed, 274 insertions(+)
> >  create mode 100644 xen/arch/arm/vgic/vgic-v2.c
> > 
> > diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
> > new file mode 100644
> > index 00..1773503cfb
> > --- /dev/null
> > +++ b/xen/arch/arm/vgic/vgic-v2.c
> > @@ -0,0 +1,259 @@
> > +/*
> > + * Copyright (C) 2015, 2016 ARM Ltd.
> > + * Imported from Linux ("new" KVM VGIC) and heavily adapted to Xen.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see .
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "vgic.h"
> > +
> > +static struct {
> > +bool enabled;
> > +paddr_t dbase;  /* Distributor interface address */
> > +paddr_t cbase;  /* CPU interface address & size */
> > +paddr_t csize;
> > +paddr_t vbase;  /* Virtual CPU interface address */
> > +
> > +/* Offset to add to get an 8kB contiguous region if GIC is aliased */
> > +uint32_t aliased_offset;
> > +} gic_v2_hw_data;
> > +
> > +void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t csize,
> > +  paddr_t vbase, uint32_t aliased_offset)
> > +{
> > +gic_v2_hw_data.enabled = true;
> > +gic_v2_hw_data.dbase = dbase;
> > +gic_v2_hw_data.cbase = cbase;
> > +gic_v2_hw_data.csize = csize;
> > +gic_v2_hw_data.vbase = vbase;
> > +gic_v2_hw_data.aliased_offset = aliased_offset;
> > +
> > +printk("Using the new VGIC implementation.\n");
> > +}
> > +
> > +/*
> > + * transfer the content of the LRs back into the corresponding ap_list:
> > + * - active bit is transferred as is
> > + * - pending bit is
> > + *   - transferred as is in case of edge sensitive IRQs
> > + *   - set to the line-level (resample time) for level sensitive IRQs
> > + */
> > +void vgic_v2_fold_lr_state(struct vcpu *vcpu)
> > +{
> > +struct vgic_cpu *vgic_cpu = >arch.vgic;
> > +unsigned int used_lrs = vcpu->arch.vgic.used_lrs;
> > +unsigned long flags;
> > +unsigned int lr;
> > +
> > +if ( !used_lrs )/* No LRs used, so nothing to sync back here. */
> > +return;
> > +
> > +gic_hw_ops->update_hcr_status(GICH_HCR_UIE, false);
> > +
> > +for ( lr = 0; lr < used_lrs; lr++ )
> > +{
> > +struct gic_lr lr_val;
> > +uint32_t intid;
> > +struct vgic_irq *irq;
> > +struct irq_desc *desc = NULL;
> > +bool have_desc_lock = false;
> > +
> > +gic_hw_ops->read_lr(lr, _val);
> > +
> > +/*
> > + * TODO: Possible optimization to avoid reading LRs:
> > + * Read the ELRSR to find out which of our LRs have been cleared
> > + * by the guest. We just need to know the IRQ number for those, 
> > which
> > + * we could save in an array when populating the LRs.
> > + * This trades one MMIO access (ELRSR) for possibly more than one 
> > (LRs),
> > + * but requires some more code to save the IRQ number and to handle
> > + * those finished IRQs according to the algorithm below.
> > + 

Re: [Xen-devel] [PATCH v3a 14/39] ARM: new VGIC: Add GICv2 world switch backend

[Julien Grall]

Hi,

Sorry for the formatting.

On Tue, 27 Mar 2018, 07:25 Stefano Stabellini, 
wrote:

> On Thu, 22 Mar 2018, Andre Przywara wrote:
> > Processing maintenance interrupts and accessing the list registers
> > are dependent on the host's GIC version.
> > Introduce vgic-v2.c to contain GICv2 specific functions.
> > Implement the GICv2 specific code for syncing the emulation state
> > into the VGIC registers.
> > This also adds the hook to let Xen setup the host GIC addresses.
> >
> > This is based on Linux commit 140b086dd197, written by Marc Zyngier.
> >
> > Signed-off-by: Andre Przywara 
> > ---
> > Changelog v3 ... v3a:
> > - take hardware IRQ lock in vgic_v2_fold_lr_state()
> > - fix last remaining u32 usage
> > - print message when using new VGIC
> > - add TODO about racy _IRQ_INPROGRESS setting
> >
> > Changelog v2 ... v3:
> > - remove no longer needed asm/io.h header
> > - replace 0/1 with false/true for bool's
> > - clear _IRQ_INPROGRESS bit when retiring hardware mapped IRQ
> > - fix indentation and w/s issues
> >
> > Changelog v1 ... v2:
> > - remove v2 specific underflow function (now generic)
> > - re-add Linux code to properly handle acked level IRQs
> >
> >  xen/arch/arm/vgic/vgic-v2.c | 259
> 
> >  xen/arch/arm/vgic/vgic.c|   6 +
> >  xen/arch/arm/vgic/vgic.h|   9 ++
> >  3 files changed, 274 insertions(+)
> >  create mode 100644 xen/arch/arm/vgic/vgic-v2.c
> >
> > diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
> > new file mode 100644
> > index 00..1773503cfb
> > --- /dev/null
> > +++ b/xen/arch/arm/vgic/vgic-v2.c
> > @@ -0,0 +1,259 @@
> > +/*
> > + * Copyright (C) 2015, 2016 ARM Ltd.
> > + * Imported from Linux ("new" KVM VGIC) and heavily adapted to Xen.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see  >.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "vgic.h"
> > +
> > +static struct {
> > +bool enabled;
> > +paddr_t dbase;  /* Distributor interface address */
> > +paddr_t cbase;  /* CPU interface address & size */
> > +paddr_t csize;
> > +paddr_t vbase;  /* Virtual CPU interface address */
> > +
> > +/* Offset to add to get an 8kB contiguous region if GIC is aliased
> */
> > +uint32_t aliased_offset;
> > +} gic_v2_hw_data;
> > +
> > +void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t csize,
> > +  paddr_t vbase, uint32_t aliased_offset)
> > +{
> > +gic_v2_hw_data.enabled = true;
> > +gic_v2_hw_data.dbase = dbase;
> > +gic_v2_hw_data.cbase = cbase;
> > +gic_v2_hw_data.csize = csize;
> > +gic_v2_hw_data.vbase = vbase;
> > +gic_v2_hw_data.aliased_offset = aliased_offset;
> > +
> > +printk("Using the new VGIC implementation.\n");
> > +}
> > +
> > +/*
> > + * transfer the content of the LRs back into the corresponding ap_list:
> > + * - active bit is transferred as is
> > + * - pending bit is
> > + *   - transferred as is in case of edge sensitive IRQs
> > + *   - set to the line-level (resample time) for level sensitive IRQs
> > + */
> > +void vgic_v2_fold_lr_state(struct vcpu *vcpu)
> > +{
> > +struct vgic_cpu *vgic_cpu = >arch.vgic;
> > +unsigned int used_lrs = vcpu->arch.vgic.used_lrs;
> > +unsigned long flags;
> > +unsigned int lr;
> > +
> > +if ( !used_lrs )/* No LRs used, so nothing to sync back here. */
> > +return;
> > +
> > +gic_hw_ops->update_hcr_status(GICH_HCR_UIE, false);
> > +
> > +for ( lr = 0; lr < used_lrs; lr++ )
> > +{
> > +struct gic_lr lr_val;
> > +uint32_t intid;
> > +struct vgic_irq *irq;
> > +struct irq_desc *desc = NULL;
> > +bool have_desc_lock = false;
> > +
> > +gic_hw_ops->read_lr(lr, _val);
> > +
> > +/*
> > + * TODO: Possible optimization to avoid reading LRs:
> > + * Read the ELRSR to find out which of our LRs have been cleared
> > + * by the guest. We just need to know the IRQ number for those,
> which
> > + * we could save in an array when populating the LRs.
> > + * This trades one MMIO access (ELRSR) for possibly more than
> one (LRs),
> > + * but requires some more code to save the IRQ number and to
> handle
> > + * 

Re: [Xen-devel] [PATCH v6] hvm/svm: Implement Debug events

[Boris Ostrovsky]

On 03/26/2018 05:43 AM, Alexandru Isaila wrote:

At this moment the Debug events for the AMD architecture are not
forwarded to the monitor layer.

This patch adds the Debug event to the common capabilities, adds
the VMEXIT_ICEBP then forwards the event to the monitor layer.

Chapter 2: SVM Processor and Platform Extensions: "Note: A vector 1
exception generated by the single byte INT1
instruction (also known as ICEBP) does not trigger the #DB
intercept. Software should use the dedicated ICEBP
intercept to intercept ICEBP"

Signed-off-by: Alexandru Isaila 


SVM bits:

Reviewed-by: Boris Ostrovsky 



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

Re: [Xen-devel] [PATCH v3 32/39] ARM: new VGIC: Implement arch_move_irqs()

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> When a VCPU moves to another CPU, we need to adjust the target affinity
> of any hardware mapped vIRQs, to observe our "physical-follows-virtual"
> policy.
> Implement arch_move_irqs() to adjust the physical affinity of all hardware
> mapped vIRQs targetting this VCPU.
> 
> Signed-off-by: Andre Przywara 
> Reviewed-by: Julien Grall 

Acked-by: Stefano Stabellini 

> ---
>  xen/arch/arm/vgic/vgic.c | 39 +++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> index ffab0b2635..23b8abfc5e 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -791,6 +791,45 @@ void gic_dump_vgic_info(struct vcpu *v)
>  spin_unlock_irqrestore(>arch.vgic.ap_list_lock, flags);
>  }
>  
> +/**
> + * arch_move_irqs() - migrate the physical affinity of hardware mapped vIRQs
> + * @v:  the vCPU, already assigned to the new pCPU
> + *
> + * arch_move_irqs() updates the physical affinity of all virtual IRQs
> + * targetting this given vCPU. This only affects hardware mapped IRQs. The
> + * new pCPU to target is already set in v->processor.
> + * This is called by the core code after a vCPU has been migrated to a new
> + * physical CPU.
> + */
> +void arch_move_irqs(struct vcpu *v)
> +{
> +struct domain *d = v->domain;
> +unsigned int i;
> +
> +/* We only target SPIs with this function */
> +for ( i = 0; i < d->arch.vgic.nr_spis; i++ )
> +{
> +struct vgic_irq *irq = vgic_get_irq(d, NULL, i + 
> VGIC_NR_PRIVATE_IRQS);
> +unsigned long flags;
> +
> +if ( !irq )
> +continue;
> +
> +spin_lock_irqsave(>irq_lock, flags);
> +
> +/* only vIRQs that are not on a vCPU yet , but targetting this vCPU 
> */
> +if ( irq->hw && !irq->vcpu && irq->target_vcpu == v)
> +{
> +irq_desc_t *desc = irq_to_desc(irq->hwintid);
> +
> +irq_set_affinity(desc, cpumask_of(v->processor));
> +}
> +
> +spin_unlock_irqrestore(>irq_lock, flags);
> +vgic_put_irq(d, irq);
> +}
> +}
> +
>  struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v,
>unsigned int virq)
>  {
> -- 
> 2.14.1
> 

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

Re: [Xen-devel] [PATCH v3 15/39] ARM: new VGIC: Implement vgic_vcpu_pending_irq

[Stefano Stabellini]

On Thu, 22 Mar 2018, Andre Przywara wrote:
> Hi,
> 
> On 22/03/18 03:52, Julien Grall wrote:
> > Hi Andre,
> > 
> > On 03/21/2018 04:32 PM, Andre Przywara wrote:
> >> Tell Xen whether a particular VCPU has an IRQ that needs handling
> >> in the guest. This is used to decide whether a VCPU is runnable or
> >> if a hypercall should be preempted to let the guest handle the IRQ.
> >>
> >> This is based on Linux commit 90eee56c5f90, written by Eric Auger.
> >>
> >> Signed-off-by: Andre Przywara 
> >> ---
> >> Changelog v2 ... v3:
> >> - adjust vgic_vcpu_pending_irq() to return integers, not false/true
> > 
> > I would have preferred to have the return switch to bool instead. I
> > guess this can be done on a follow-up. With one comment below:
> 
> I did that originally, but then you meanwhile merged that first patch
> already. So I didn't want to add another patch to this series.
> I am fine with changing this afterwards, probably as part of a fixup series.
> 
> > Reviewed-by: Julien Grall 
> 
> Thanks!

Acked-by: Stefano Stabellini 


> Cheers,
> Andre.
> 
> >>
> >> Changelog v1 ... v2:
> >> - adjust to new vgic_vcpu_pending_irq() prototype, drop wrapper
> >>
> >>   xen/arch/arm/vgic/vgic.c | 37 +
> >>   1 file changed, 37 insertions(+)
> >>
> >> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> >> index 2fa595f4f7..925cda4580 100644
> >> --- a/xen/arch/arm/vgic/vgic.c
> >> +++ b/xen/arch/arm/vgic/vgic.c
> >> @@ -647,6 +647,43 @@ void vgic_sync_to_lrs(void)
> >>   gic_hw_ops->update_hcr_status(GICH_HCR_EN, 1);
> >>   }
> >>   +/**
> >> + * vgic_vcpu_pending_irq() - determine if interrupts need to be injected
> >> + * @vcpu: The vCPU on which to check for interrupts.
> >> + *
> >> + * Checks whether there is an interrupt on the given VCPU which needs
> >> + * handling in the guest. This requires at least one IRQ to be pending
> >> + * and enabled.
> >> + *
> >> + * Returns: 1 if the guest should run to handle interrupts, 0 otherwise.
> > 
> > NIT: Because of "ret = irq_is_pending(irq) && irq->enabled", you will
> > return a non-zero value if the guest should run to handle interrupts.
> > 
> >> + */
> >> +int vgic_vcpu_pending_irq(struct vcpu *vcpu)
> >> +{
> >> +    struct vgic_cpu *vgic_cpu = >arch.vgic;
> >> +    struct vgic_irq *irq;
> >> +    unsigned long flags;
> >> +    int ret = 0;
> >> +
> >> +    if ( !vcpu->domain->arch.vgic.enabled )
> >> +    return 0;
> >> +
> >> +    spin_lock_irqsave(_cpu->ap_list_lock, flags);
> >> +
> >> +    list_for_each_entry(irq, _cpu->ap_list_head, ap_list)
> >> +    {
> >> +    spin_lock(>irq_lock);
> >> +    ret = irq_is_pending(irq) && irq->enabled;
> >> +    spin_unlock(>irq_lock);
> >> +
> >> +    if ( ret )
> >> +    break;
> >> +    }
> >> +
> >> +    spin_unlock_irqrestore(_cpu->ap_list_lock, flags);
> >> +
> >> +    return ret;
> >> +}
> >> +
> >>   /*
> >>    * Local variables:
> >>    * mode: C
> >>
> > 
> > Cheers,
> > 
> ___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[osstest service owner]

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

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  7356011ec2242f12caae461ed694a6f2796621f4
baseline version:
 xen  9f5b0ce10b2895b4136c9e5c5ebd0aebac31ea98

Last test of basis   121301  2018-03-26 16:10:15 Z0 days
Testing same since   121306  2018-03-26 21:02:33 Z0 days1 attempts


People who touched revisions under test:
  Andre Przywara 
  Julien Grall 

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
   9f5b0ce10b..7356011ec2  7356011ec2242f12caae461ed694a6f2796621f4 -> smoke

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

Re: [Xen-devel] [PATCH v3a 14/39] ARM: new VGIC: Add GICv2 world switch backend

[Stefano Stabellini]

On Thu, 22 Mar 2018, Andre Przywara wrote:
> Processing maintenance interrupts and accessing the list registers
> are dependent on the host's GIC version.
> Introduce vgic-v2.c to contain GICv2 specific functions.
> Implement the GICv2 specific code for syncing the emulation state
> into the VGIC registers.
> This also adds the hook to let Xen setup the host GIC addresses.
> 
> This is based on Linux commit 140b086dd197, written by Marc Zyngier.
> 
> Signed-off-by: Andre Przywara 
> ---
> Changelog v3 ... v3a:
> - take hardware IRQ lock in vgic_v2_fold_lr_state()
> - fix last remaining u32 usage
> - print message when using new VGIC
> - add TODO about racy _IRQ_INPROGRESS setting
> 
> Changelog v2 ... v3:
> - remove no longer needed asm/io.h header
> - replace 0/1 with false/true for bool's
> - clear _IRQ_INPROGRESS bit when retiring hardware mapped IRQ
> - fix indentation and w/s issues
> 
> Changelog v1 ... v2:
> - remove v2 specific underflow function (now generic)
> - re-add Linux code to properly handle acked level IRQs
> 
>  xen/arch/arm/vgic/vgic-v2.c | 259 
> 
>  xen/arch/arm/vgic/vgic.c|   6 +
>  xen/arch/arm/vgic/vgic.h|   9 ++
>  3 files changed, 274 insertions(+)
>  create mode 100644 xen/arch/arm/vgic/vgic-v2.c
> 
> diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
> new file mode 100644
> index 00..1773503cfb
> --- /dev/null
> +++ b/xen/arch/arm/vgic/vgic-v2.c
> @@ -0,0 +1,259 @@
> +/*
> + * Copyright (C) 2015, 2016 ARM Ltd.
> + * Imported from Linux ("new" KVM VGIC) and heavily adapted to Xen.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "vgic.h"
> +
> +static struct {
> +bool enabled;
> +paddr_t dbase;  /* Distributor interface address */
> +paddr_t cbase;  /* CPU interface address & size */
> +paddr_t csize;
> +paddr_t vbase;  /* Virtual CPU interface address */
> +
> +/* Offset to add to get an 8kB contiguous region if GIC is aliased */
> +uint32_t aliased_offset;
> +} gic_v2_hw_data;
> +
> +void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t csize,
> +  paddr_t vbase, uint32_t aliased_offset)
> +{
> +gic_v2_hw_data.enabled = true;
> +gic_v2_hw_data.dbase = dbase;
> +gic_v2_hw_data.cbase = cbase;
> +gic_v2_hw_data.csize = csize;
> +gic_v2_hw_data.vbase = vbase;
> +gic_v2_hw_data.aliased_offset = aliased_offset;
> +
> +printk("Using the new VGIC implementation.\n");
> +}
> +
> +/*
> + * transfer the content of the LRs back into the corresponding ap_list:
> + * - active bit is transferred as is
> + * - pending bit is
> + *   - transferred as is in case of edge sensitive IRQs
> + *   - set to the line-level (resample time) for level sensitive IRQs
> + */
> +void vgic_v2_fold_lr_state(struct vcpu *vcpu)
> +{
> +struct vgic_cpu *vgic_cpu = >arch.vgic;
> +unsigned int used_lrs = vcpu->arch.vgic.used_lrs;
> +unsigned long flags;
> +unsigned int lr;
> +
> +if ( !used_lrs )/* No LRs used, so nothing to sync back here. */
> +return;
> +
> +gic_hw_ops->update_hcr_status(GICH_HCR_UIE, false);
> +
> +for ( lr = 0; lr < used_lrs; lr++ )
> +{
> +struct gic_lr lr_val;
> +uint32_t intid;
> +struct vgic_irq *irq;
> +struct irq_desc *desc = NULL;
> +bool have_desc_lock = false;
> +
> +gic_hw_ops->read_lr(lr, _val);
> +
> +/*
> + * TODO: Possible optimization to avoid reading LRs:
> + * Read the ELRSR to find out which of our LRs have been cleared
> + * by the guest. We just need to know the IRQ number for those, which
> + * we could save in an array when populating the LRs.
> + * This trades one MMIO access (ELRSR) for possibly more than one 
> (LRs),
> + * but requires some more code to save the IRQ number and to handle
> + * those finished IRQs according to the algorithm below.
> + * We need some numbers to justify this: chances are that we don't
> + * have many LRs in use most of the time, so we might not save much.
> + */
> +gic_hw_ops->clear_lr(lr);
> +
> +intid = lr_val.virq;
> +irq = vgic_get_irq(vcpu->domain, vcpu, intid);
> +
> +   

Re: [Xen-devel] [PATCH v3 13/39] ARM: new VGIC: Add IRQ sync/flush framework

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> Implement the framework for syncing IRQs between our emulation and the
> list registers, which represent the guest's view of IRQs.
> This is done in vgic_sync_from_lrs() and vgic_sync_to_lrs(), which
> get called on guest entry and exit, respectively.
> The code talking to the actual GICv2/v3 hardware is added in the
> following patches.
> 
> This is based on Linux commit 0919e84c0fc1, written by Marc Zyngier.
> 
> Signed-off-by: Andre Przywara 

Just one question below, but the code looks nice


> ---
> Changelog v2 ... v3:
> - replace "true" instead of "1" for the boolean parameter
> 
> Changelog v1 ... v2:
> - make functions void
> - do underflow setting directly (no v2/v3 indirection)
> - fix multiple SGIs injections (as the late Linux bugfix)
> 
>  xen/arch/arm/vgic/vgic.c | 232 
> +++
>  xen/arch/arm/vgic/vgic.h |   2 +
>  2 files changed, 234 insertions(+)
> 
> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> index ee0de8d2e0..52e1669888 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -409,6 +409,238 @@ void vgic_inject_irq(struct domain *d, struct vcpu 
> *vcpu, unsigned int intid,
>  return;
>  }
>  
> +/**
> + * vgic_prune_ap_list() - Remove non-relevant interrupts from the ap_list
> + *
> + * @vcpu:   The VCPU of which the ap_list should be pruned.
> + *
> + * Go over the list of interrupts on a VCPU's ap_list, and prune those that
> + * we won't have to consider in the near future.
> + * This removes interrupts that have been successfully handled by the guest,
> + * or that have otherwise became obsolete (not pending anymore).
> + * Also this moves interrupts between VCPUs, if their affinity has changed.
> + */
> +static void vgic_prune_ap_list(struct vcpu *vcpu)
> +{
> +struct vgic_cpu *vgic_cpu = >arch.vgic;
> +struct vgic_irq *irq, *tmp;
> +unsigned long flags;
> +
> +retry:
> +spin_lock_irqsave(_cpu->ap_list_lock, flags);
> +
> +list_for_each_entry_safe( irq, tmp, _cpu->ap_list_head, ap_list )
> +{
> +struct vcpu *target_vcpu, *vcpuA, *vcpuB;
> +
> +spin_lock(>irq_lock);
> +
> +BUG_ON(vcpu != irq->vcpu);
> +
> +target_vcpu = vgic_target_oracle(irq);
> +
> +if ( !target_vcpu )
> +{
> +/*
> + * We don't need to process this interrupt any
> + * further, move it off the list.
> + */
> +list_del(>ap_list);
> +irq->vcpu = NULL;
> +spin_unlock(>irq_lock);
> +
> +/*
> + * This vgic_put_irq call matches the
> + * vgic_get_irq_kref in vgic_queue_irq_unlock,
> + * where we added the LPI to the ap_list. As
> + * we remove the irq from the list, we drop
> + * also drop the refcount.
> + */
> +vgic_put_irq(vcpu->domain, irq);
> +continue;
> +}
> +
> +if ( target_vcpu == vcpu )
> +{
> +/* We're on the right CPU */
> +spin_unlock(>irq_lock);
> +continue;
> +}
> +
> +/* This interrupt looks like it has to be migrated. */
> +
> +spin_unlock(>irq_lock);
> +spin_unlock_irqrestore(_cpu->ap_list_lock, flags);
> +
> +/*
> + * Ensure locking order by always locking the smallest
> + * ID first.
> + */
> +if ( vcpu->vcpu_id < target_vcpu->vcpu_id )
> +{
> +vcpuA = vcpu;
> +vcpuB = target_vcpu;
> +}
> +else
> +{
> +vcpuA = target_vcpu;
> +vcpuB = vcpu;
> +}
> +
> +spin_lock_irqsave(>arch.vgic.ap_list_lock, flags);
> +spin_lock(>arch.vgic.ap_list_lock);
> +spin_lock(>irq_lock);
> +
> +/*
> + * If the affinity has been preserved, move the
> + * interrupt around. Otherwise, it means things have
> + * changed while the interrupt was unlocked, and we
> + * need to replay this.
> + *
> + * In all cases, we cannot trust the list not to have
> + * changed, so we restart from the beginning.
> + */
> +if ( target_vcpu == vgic_target_oracle(irq) )
> +{
> +struct vgic_cpu *new_cpu = _vcpu->arch.vgic;
> +
> +list_del(>ap_list);
> +irq->vcpu = target_vcpu;
> +list_add_tail(>ap_list, _cpu->ap_list_head);
> +}
> +
> +spin_unlock(>irq_lock);
> +spin_unlock(>arch.vgic.ap_list_lock);
> +spin_unlock_irqrestore(>arch.vgic.ap_list_lock, flags);
> +goto retry;
> +}
> +
> +spin_unlock_irqrestore(_cpu->ap_list_lock, flags);
> +}
> +
> +static void vgic_fold_lr_state(struct vcpu *vcpu)
> +{
> +}
> +
> +/* Requires the irq_lock to be held. */
> +static void vgic_populate_lr(struct vcpu *vcpu,
> 

Re: [Xen-devel] [PATCH v3 12/39] ARM: new VGIC: Add IRQ sorting

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> Adds the sorting function to cover the case where you have more IRQs
> to consider than you have LRs. We consider their priorities.
> This uses the new sort_list() implementation imported from Linux.
> 
> This is based on Linux commit 8e4447457965, written by Christoffer Dall.
> 
> Signed-off-by: Andre Przywara 
> Reviewed-by: Julien Grall 

Acked-by: Stefano Stabellini 

> ---
>  xen/arch/arm/vgic/vgic.c | 59 
> 
>  1 file changed, 59 insertions(+)
> 
> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> index f7dfd01c1d..ee0de8d2e0 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -15,6 +15,7 @@
>   * along with this program.  If not, see .
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -193,6 +194,64 @@ static struct vcpu *vgic_target_oracle(struct vgic_irq 
> *irq)
>  return NULL;
>  }
>  
> +/*
> + * The order of items in the ap_lists defines how we'll pack things in LRs as
> + * well, the first items in the list being the first things populated in the
> + * LRs.
> + *
> + * A hard rule is that active interrupts can never be pushed out of the LRs
> + * (and therefore take priority) since we cannot reliably trap on 
> deactivation
> + * of IRQs and therefore they have to be present in the LRs.
> + *
> + * Otherwise things should be sorted by the priority field and the GIC
> + * hardware support will take care of preemption of priority groups etc.
> + *
> + * Return negative if "a" sorts before "b", 0 to preserve order, and positive
> + * to sort "b" before "a".
> + */
> +static int vgic_irq_cmp(void *priv, struct list_head *a, struct list_head *b)
> +{
> +struct vgic_irq *irqa = container_of(a, struct vgic_irq, ap_list);
> +struct vgic_irq *irqb = container_of(b, struct vgic_irq, ap_list);
> +bool penda, pendb;
> +int ret;
> +
> +spin_lock(>irq_lock);
> +spin_lock(>irq_lock);
> +
> +if ( irqa->active || irqb->active )
> +{
> +ret = (int)irqb->active - (int)irqa->active;
> +goto out;
> +}
> +
> +penda = irqa->enabled && irq_is_pending(irqa);
> +pendb = irqb->enabled && irq_is_pending(irqb);
> +
> +if ( !penda || !pendb )
> +{
> +ret = (int)pendb - (int)penda;
> +goto out;
> +}
> +
> +/* Both pending and enabled, sort by priority */
> +ret = irqa->priority - irqb->priority;
> +out:
> +spin_unlock(>irq_lock);
> +spin_unlock(>irq_lock);
> +return ret;
> +}
> +
> +/* Must be called with the ap_list_lock held */
> +static void vgic_sort_ap_list(struct vcpu *vcpu)
> +{
> +struct vgic_cpu *vgic_cpu = >arch.vgic;
> +
> +ASSERT(spin_is_locked(_cpu->ap_list_lock));
> +
> +list_sort(NULL, _cpu->ap_list_head, vgic_irq_cmp);
> +}
> +
>  /*
>   * Only valid injection if changing level for level-triggered IRQs or for a
>   * rising edge.
> -- 
> 2.14.1
> 

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

Re: [Xen-devel] [PATCH v3 10/39] ARM: new VGIC: Implement virtual IRQ injection

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> Provide a vgic_queue_irq_unlock() function which decides whether a
> given IRQ needs to be queued to a VCPU's ap_list.
> This should be called whenever an IRQ becomes pending or enabled,
> either as a result of a hardware IRQ injection, from devices emulated by
> Xen (like the architected timer) or from MMIO accesses to the distributor
> emulation.
> Also provides the necessary functions to allow to inject an IRQ to a guest.
> Since this is the first code that starts using our locking mechanism,
> we add some (hopefully) clear documentation of our locking strategy and
> requirements along with this patch.
> 
> This is based on Linux commit 81eeb95ddbab, written by Christoffer Dall.
> 
> Signed-off-by: Andre Przywara 
> Reviewed-by: Julien Grall 

Reviewed-by: Stefano Stabellini 


> ---
>  xen/arch/arm/vgic/vgic.c | 226 
> +++
>  xen/arch/arm/vgic/vgic.h |  10 +++
>  2 files changed, 236 insertions(+)
> 
> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> index a818e382b1..f7dfd01c1d 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -17,10 +17,36 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "vgic.h"
>  
> +/*
> + * Locking order is always:
> + *   vgic->lock
> + * vgic_cpu->ap_list_lock
> + *   vgic->lpi_list_lock
> + * desc->lock
> + *   vgic_irq->irq_lock
> + *
> + * If you need to take multiple locks, always take the upper lock first,
> + * then the lower ones, e.g. first take the ap_list_lock, then the irq_lock.
> + * If you are already holding a lock and need to take a higher one, you
> + * have to drop the lower ranking lock first and re-acquire it after having
> + * taken the upper one.
> + *
> + * When taking more than one ap_list_lock at the same time, always take the
> + * lowest numbered VCPU's ap_list_lock first, so:
> + *   vcpuX->vcpu_id < vcpuY->vcpu_id:
> + * spin_lock(vcpuX->arch.vgic.ap_list_lock);
> + * spin_lock(vcpuY->arch.vgic.ap_list_lock);
> + *
> + * Since the VGIC must support injecting virtual interrupts from ISRs, we 
> have
> + * to use the spin_lock_irqsave/spin_unlock_irqrestore versions of outer
> + * spinlocks for any lock that may be taken while injecting an interrupt.
> + */
> +
>  /*
>   * Iterate over the VM's list of mapped LPIs to find the one with a
>   * matching interrupt ID and return a reference to the IRQ structure.
> @@ -124,6 +150,206 @@ void vgic_put_irq(struct domain *d, struct vgic_irq 
> *irq)
>  xfree(irq);
>  }
>  
> +/**
> + * vgic_target_oracle() - compute the target vcpu for an irq
> + * @irq:The irq to route. Must be already locked.
> + *
> + * Based on the current state of the interrupt (enabled, pending,
> + * active, vcpu and target_vcpu), compute the next vcpu this should be
> + * given to. Return NULL if this shouldn't be injected at all.
> + *
> + * Requires the IRQ lock to be held.
> + *
> + * Returns: The pointer to the virtual CPU this interrupt should be injected
> + *  to. Will be NULL if this IRQ does not need to be injected.
> + */
> +static struct vcpu *vgic_target_oracle(struct vgic_irq *irq)
> +{
> +ASSERT(spin_is_locked(>irq_lock));
> +
> +/* If the interrupt is active, it must stay on the current vcpu */
> +if ( irq->active )
> +return irq->vcpu ? : irq->target_vcpu;
> +
> +/*
> + * If the IRQ is not active but enabled and pending, we should direct
> + * it to its configured target VCPU.
> + * If the distributor is disabled, pending interrupts shouldn't be
> + * forwarded.
> + */
> +if ( irq->enabled && irq_is_pending(irq) )
> +{
> +if ( unlikely(irq->target_vcpu &&
> +  !irq->target_vcpu->domain->arch.vgic.enabled) )
> +return NULL;
> +
> +return irq->target_vcpu;
> +}
> +
> +/*
> + * If neither active nor pending and enabled, then this IRQ should not
> + * be queued to any VCPU.
> + */
> +return NULL;
> +}
> +
> +/*
> + * Only valid injection if changing level for level-triggered IRQs or for a
> + * rising edge.
> + */
> +static bool vgic_validate_injection(struct vgic_irq *irq, bool level)
> +{
> +/* For edge interrupts we only care about a rising edge. */
> +if ( irq->config == VGIC_CONFIG_EDGE )
> +return level;
> +
> +/* For level interrupts we have to act when the line level changes. */
> +return irq->line_level != level;
> +}
> +
> +/**
> + * vgic_queue_irq_unlock() - Queue an IRQ to a VCPU, to be injected to a 
> guest.
> + * @d:The domain the virtual IRQ belongs to.
> + * @irq:  A pointer to the vgic_irq of the virtual IRQ, with the lock 
> held.
> + * @flags:The flags used when having grabbed the IRQ lock.
> + *
> + * Check whether an IRQ needs to (and can) be queued to a VCPU's 

Re: [Xen-devel] [PATCH v3 09/39] ARM: new VGIC: Add accessor to new struct vgic_irq instance

[Stefano Stabellini]

On Thu, 22 Mar 2018, Julien Grall wrote:
> Hi Andre,
> 
> On 03/21/2018 04:32 PM, Andre Przywara wrote:
> > The new VGIC implementation centers around a struct vgic_irq instance
> > per virtual IRQ.
> > Provide a function to retrieve the right instance for a given IRQ
> > number and (in case of private interrupts) the right VCPU.
> > This also includes the corresponding put function, which does nothing
> > for private interrupts and SPIs, but handles the ref-counting for LPIs.
> > 
> > This is based on Linux commit 64a959d66e47, written by Christoffer Dall.
> > 
> > Signed-off-by: Andre Przywara 
> > ---
> > Changelog v2 ... v3:
> > - extend comments to note preliminary nature of vgic_get_lpi()
> 
> Thank you for the update.
> 
> > 
> > Changelog v1 ... v2:
> > - reorder header file inclusion
> > 
> >   xen/arch/arm/vgic/vgic.c | 134
> > +++
> >   xen/arch/arm/vgic/vgic.h |  41 +++
> >   2 files changed, 175 insertions(+)
> >   create mode 100644 xen/arch/arm/vgic/vgic.c
> >   create mode 100644 xen/arch/arm/vgic/vgic.h
> > 
> > diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> > new file mode 100644
> > index 00..a818e382b1
> > --- /dev/null
> > +++ b/xen/arch/arm/vgic/vgic.c
> > @@ -0,0 +1,134 @@
> > +/*
> > + * Copyright (C) 2015, 2016 ARM Ltd.
> > + * Imported from Linux ("new" KVM VGIC) and heavily adapted to Xen.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see .
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "vgic.h"
> > +
> > +/*
> > + * Iterate over the VM's list of mapped LPIs to find the one with a
> > + * matching interrupt ID and return a reference to the IRQ structure.
> > + *
> > + * TODO: This is more documentation of how it should be done. A list is
> > + * not a good data structure for Dom0's LPIs, it merely serves as an
> > + * example here how to properly do the locking, allocation and refcounting.
> > + * So lpi_list_head should be replaced with something more appropriate.
> > + */
> > +static struct vgic_irq *vgic_get_lpi(struct domain *d, u32 intid)
> 
> It looks like I forgot to mention it on previous version. Please replace u32
> with uint32_t.
> 
> [...]
> 
> > +struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu,
> > +  u32 intid)
> 
> Here too.
> 
> > +{
> > +/* SGIs and PPIs */
> > +if ( intid <= VGIC_MAX_PRIVATE )
> > +return >arch.vgic.private_irqs[intid];
> > +
> > +/* SPIs */
> > +if ( intid <= VGIC_MAX_SPI )
> > +return >arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];
> > +
> > +/* LPIs */
> > +if ( intid >= VGIC_MIN_LPI )
> > +return vgic_get_lpi(d, intid);
> > +
> > +ASSERT_UNREACHABLE();
> > +
> > +return NULL;
> > +}
> > +
> 
> [...]
> 
> > diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
> > new file mode 100644
> > index 00..a3befd386b
> > --- /dev/null
> > +++ b/xen/arch/arm/vgic/vgic.h
> 
> [...]
> 
> > +struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu,
> > +  u32 intid);
> 
> And here too.
> 
> With that:
> 
> Acked-by: Julien Grall 

same here:
Acked-by: Stefano Stabellini 

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

Re: [Xen-devel] [PATCH v3 08/39] ARM: new VGIC: Add data structure definitions

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> Add a new header file for the new and improved GIC implementation.
> The big change is that we now have a struct vgic_irq per IRQ instead
> of spreading all the information over various bitmaps in the ranks.
> 
> We include this new header conditionally from within the old header
> file for the time being to avoid touching all the users.
> 
> This is based on Linux commit b18b57787f5e, written by Christoffer Dall.
> 
> Signed-off-by: Andre Przywara 
> Reviewed-by: Julien Grall 

Acked-by: Stefano Stabellini 

> ---
>  xen/include/asm-arm/new_vgic.h | 198 
> +
>  xen/include/asm-arm/vgic.h |   6 ++
>  2 files changed, 204 insertions(+)
>  create mode 100644 xen/include/asm-arm/new_vgic.h
> 
> diff --git a/xen/include/asm-arm/new_vgic.h b/xen/include/asm-arm/new_vgic.h
> new file mode 100644
> index 00..97d622bff6
> --- /dev/null
> +++ b/xen/include/asm-arm/new_vgic.h
> @@ -0,0 +1,198 @@
> +/*
> + * Copyright (C) 2015, 2016 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +#ifndef __ASM_ARM_NEW_VGIC_H
> +#define __ASM_ARM_NEW_VGIC_H
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define VGIC_V3_MAX_CPUS255
> +#define VGIC_V2_MAX_CPUS8
> +#define VGIC_NR_SGIS16
> +#define VGIC_NR_PPIS16
> +#define VGIC_NR_PRIVATE_IRQS(VGIC_NR_SGIS + VGIC_NR_PPIS)
> +#define VGIC_MAX_PRIVATE(VGIC_NR_PRIVATE_IRQS - 1)
> +#define VGIC_MAX_SPI1019
> +#define VGIC_MAX_RESERVED   1023
> +#define VGIC_MIN_LPI8192
> +
> +#define irq_is_ppi(irq) ((irq) >= VGIC_NR_SGIS && (irq) < 
> VGIC_NR_PRIVATE_IRQS)
> +#define irq_is_spi(irq) ((irq) >= VGIC_NR_PRIVATE_IRQS && \
> + (irq) <= VGIC_MAX_SPI)
> +
> +enum vgic_type {
> +VGIC_V2,/* Good ol' GICv2 */
> +VGIC_V3,/* New fancy GICv3 */
> +};
> +
> +#define VGIC_V2_MAX_LRS (1 << 6)
> +#define VGIC_V3_MAX_LRS 16
> +#define VGIC_V3_LR_INDEX(lr)(VGIC_V3_MAX_LRS - 1 - lr)
> +
> +#define VGIC_CONFIG_EDGEfalse
> +#define VGIC_CONFIG_LEVEL   true
> +
> +struct vgic_irq {
> +struct list_head ap_list;
> +
> +struct vcpu *vcpu;  /*
> + * SGIs and PPIs: The VCPU
> + * SPIs and LPIs: The VCPU whose ap_list
> + * this is queued on.
> + */
> +
> +struct vcpu *target_vcpu;   /*
> + * The VCPU that this interrupt should
> + * be sent to, as a result of the
> + * targets reg (v2) or the affinity reg (v3).
> + */
> +
> +spinlock_t irq_lock;/* Protects the content of the struct */
> +uint32_t intid; /* Guest visible INTID */
> +atomic_t refcount;  /* Used for LPIs */
> +uint32_t hwintid;   /* HW INTID number */
> +union
> +{
> +struct {
> +uint8_t targets;/* GICv2 target VCPUs mask */
> +uint8_t source; /* GICv2 SGIs only */
> +};
> +uint32_t mpidr; /* GICv3 target VCPU */
> +};
> +uint8_t priority;
> +bool line_level:1;  /* Level only */
> +bool pending_latch:1;   /*
> + * The pending latch state used to
> + * calculate the pending state for both
> + * level and edge triggered IRQs.
> + */
> +bool active:1;  /* not used for LPIs */
> +bool enabled:1;
> +bool hw:1;  /* Tied to HW IRQ */
> +bool config:1;  /* Level or edge */
> +struct list_head lpi_list;  /* Used to link all LPIs together */
> +};
> +
> +enum iodev_type {
> +IODEV_DIST,
> +IODEV_REDIST,
> +};
> +
> +struct vgic_io_device {
> +gfn_t base_fn;
> +struct vcpu *redist_vcpu;
> +const struct vgic_register_region *regions;
> +enum iodev_type iodev_type;
> +unsigned int nr_regions;
> +};
> +
> +struct vgic_dist {
> +boolready;
> +boolinitialized;
> +
> 

Re: [Xen-devel] [PATCH v3 05/39] ARM: timer: Handle level triggered IRQs correctly

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> The ARM Generic Timer uses a level-sensitive interrupt semantic. We
> easily catch when the line goes high, as this triggers the hardware IRQ.
> However we also have to keep track of when the line lowers, as the
> emulation depends on it: Upon entering the guest, the new VGIC will
> *clear* the virtual interrupt line, so it needs to re-sample the actual
> state after returning from the guest.
> So we have to sync the state of the interrupt condition at certain
> points to catch when the line goes low and we can remove the vtimer vIRQ
> from the vGIC (and the LR).
> The VGIC in Xen so far only implemented edge triggered vIRQs, really, so
> we need to add new functionality to re-sample the interrupt state.
> Do this only when the new VGIC is in use.
> 
> Signed-off-by: Andre Przywara 
> ---
> Changelog v2 ... v3:
> - move vtimer_sync() from time.c into vtimer.c
> - rename function to vtimer_update_irqs()
> - refactor functionality into new static function, to ...
> - handle physical timer as well
> - extending comments
> 
> Changelog v1 ... v2:
> - restrict to new VGIC
> - add TODO: comment
> 
>  xen/arch/arm/traps.c | 11 ++
>  xen/arch/arm/vtimer.c| 49 
> 
>  xen/include/asm-arm/vtimer.h |  1 +
>  3 files changed, 61 insertions(+)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 7411bff7a7..2638446693 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2024,6 +2024,17 @@ static void enter_hypervisor_head(struct cpu_user_regs 
> *regs)
>  if ( current->arch.hcr_el2 & HCR_VA )
>  current->arch.hcr_el2 = READ_SYSREG(HCR_EL2);
>  
> +#ifdef CONFIG_NEW_VGIC
> +/*
> + * We need to update the state of our emulated devices using level
> + * triggered interrupts before syncing back the VGIC state.
> + *
> + * TODO: Investigate whether this is necessary to do on every
> + * trap and how it can be optimised.
> + */
> +vtimer_update_irqs(current);
> +#endif
> +
>  vgic_sync_from_lrs(current);
>  }
>  }
> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> index 8164f6c7f1..c99dd237d1 100644
> --- a/xen/arch/arm/vtimer.c
> +++ b/xen/arch/arm/vtimer.c
> @@ -334,6 +334,55 @@ bool vtimer_emulate(struct cpu_user_regs *regs, union 
> hsr hsr)
>  }
>  }
>  
> +static void vtimer_update_irq(struct vcpu *v, struct vtimer *vtimer,
> +  uint32_t vtimer_ctl)
> +{
> +bool level;
> +
> +/* Filter for the three bits that determine the status of the timer */
> +vtimer_ctl &= (CNTx_CTL_ENABLE | CNTx_CTL_PENDING | CNTx_CTL_MASK);
> +
> +/* The level is high if the timer is pending and enabled, but not 
> masked. */
> +level = (vtimer_ctl == (CNTx_CTL_ENABLE | CNTx_CTL_PENDING));
> +
> +/*
> + * This is mostly here to *lower* the virtual interrupt line if the timer
> + * is no longer pending.
> + * We would have injected an IRQ already via SOFTIRQ when the timer 
> expired.
> + * Doing it here again is basically a NOP if the line was already high.
> + */
> +vgic_inject_irq(v->domain, v, vtimer->irq, level);
> +}
> +
> +/**
> + * vtimer_update_irqs() - update the virtual timers' IRQ lines after a guest 
> run
> + * @vcpu: The VCPU to sync the timer state
> + *
> + * After returning from a guest, update the state of the timers' virtual
> + * interrupt lines, to model the level triggered interrupts correctly.
> + * If the guest has handled a timer interrupt, the virtual interrupt line
> + * needs to be lowered explicitly. vgic_inject_irq() takes care of that.
> + */
> +void vtimer_update_irqs(struct vcpu *v)
> +{
> +/*
> + * For the virtual timer we read the current state from the hardware.
> + * Technically we should keep the CNTx_CTL_MASK bit here, to catch if
> + * the timer interrupt is masked. However Xen *always* masks the timer
> + * upon entering the hypervisor, leaving it up to the guest to un-mask 
> it.
> + * So we would always read a "low" level, despite the condition being
> + * actually "high".  Ignoring the mask bit solves this (for now).
> + *
> + * TODO: The proper fix for this is to make vtimer vIRQ hardware mapped,
> + * but this requires reworking the arch timer to implement this.
> + */
> +vtimer_update_irq(v, >arch.virt_timer,
> +  READ_SYSREG32(CNTV_CTL_EL0) & ~CNTx_CTL_MASK);

Yes, but won't this have the opposite effect? Meaning that it will
always read as "high" for the virtual timer (because we remove the MASK
and that is the only thing that can cause a "low" read in
vtimer_update_irq if it's enabled and pending)?

It seems to me that it would be better to remove the update of the
virtual timer -- this seems to have the potential of causing problems.


> +/* For the physical timer we 

Re: [Xen-devel] [PATCH v3 07/39] ARM: vPL011: Use the VGIC's level triggered IRQs handling if available

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> The emulated ARM SBSA UART is using level triggered IRQ semantics,
> however the current VGIC can only handle edge triggered IRQs, really.
> Disable the existing workaround for this problem in case we have the
> new VGIC in place, which can properly handle level triggered IRQs.
> 
> Signed-off-by: Andre Przywara 
> Reviewed-by: Julien Grall 

Acked-by: Stefano Stabellini 

> ---
>  xen/arch/arm/vpl011.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 5dcf4bec18..a281eabd7e 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -54,6 +54,7 @@ static void vpl011_update_interrupt_status(struct domain *d)
>   */
>  ASSERT(spin_is_locked(>lock));
>  
> +#ifndef CONFIG_NEW_VGIC
>  /*
>   * TODO: PL011 interrupts are level triggered which means
>   * that interrupt needs to be set/clear instead of being
> @@ -71,6 +72,9 @@ static void vpl011_update_interrupt_status(struct domain *d)
>  vgic_inject_irq(d, NULL, GUEST_VPL011_SPI, true);
>  
>  vpl011->shadow_uartmis = uartmis;
> +#else
> +vgic_inject_irq(d, NULL, GUEST_VPL011_SPI, uartmis);
> +#endif
>  }
>  
>  static uint8_t vpl011_read_data(struct domain *d)
> -- 
> 2.14.1
> 

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

[Xen-devel] [linux-4.9 test] 121270: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 120913
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 120913
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 120913
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 120913
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 120913
 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-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-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-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  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail 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-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-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 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  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-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-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail 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-ws16-amd64 17 guest-stop  fail 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-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-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass

version targeted for testing:
 linux24f70aa804cd7f8fee4353cf4990997d1c8375ae
baseline version:
 linux46e076f0dad02f5c445a5c27adbd3f06147e33ed

Last test of basis   120913  2018-03-18 11:32:38 Z8 days
Failing since121052  2018-03-22 08:44:51 Z4 days3 attempts
Testing same since   121270  2018-03-25 13:00:14 Z1 days1 attempts


488 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm   

Re: [Xen-devel] [PATCH v3a 03/39] ARM: GIC: Allow tweaking the active and pending state of an IRQ

[Stefano Stabellini]

On Thu, 22 Mar 2018, Andre Przywara wrote:
> When playing around with hardware mapped, level triggered virtual IRQs,
> there is the need to explicitly set the active or pending state of an
> interrupt at some point.
> To prepare the GIC for that, we introduce a set_active_state() and a
> set_pending_state() function to let the VGIC manipulate the state of
> an associated hardware IRQ.
> This takes care of properly setting the _IRQ_INPROGRESS bit.
> 
> Signed-off-by: Andre Przywara 

Acked-by: Stefano Stabellini 

> ---
> Changelog v3 ... v3a:
> - always set/clear _IRQ_INPROGRESS bit (not only for guest IRQs)
> - add comments
> 
> Changelog v2 ... v3:
> - extend comments to note preliminary nature of vgic_get_lpi()
> 
> Changelog v1 ... v2:
> - reorder header file inclusion
> 
>  xen/arch/arm/gic-v2.c | 41 +
>  xen/arch/arm/gic-v3.c | 37 +
>  xen/include/asm-arm/gic.h | 24 
>  3 files changed, 102 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index aa0fc6c1a1..7374686235 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -243,6 +243,45 @@ static void gicv2_poke_irq(struct irq_desc *irqd, 
> uint32_t offset)
>  writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4);
>  }
>  
> +/*
> + * This is forcing the active state of an interrupt, somewhat circumventing
> + * the normal interrupt flow and the GIC state machine. So use with care
> + * and only if you know what you are doing. For this reason we also have to
> + * tinker with the _IRQ_INPROGRESS bit here, since the normal IRQ handler
> + * will not be involved.
> + */
> +static void gicv2_set_active_state(struct irq_desc *irqd, bool active)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +if ( active )
> +{
> +set_bit(_IRQ_INPROGRESS, >status);
> +gicv2_poke_irq(irqd, GICD_ISACTIVER);
> +}
> +else
> +{
> +clear_bit(_IRQ_INPROGRESS, >status);
> +gicv2_poke_irq(irqd, GICD_ICACTIVER);
> +}
> +}
> +
> +static void gicv2_set_pending_state(struct irq_desc *irqd, bool pending)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +if ( pending )
> +{
> +/* The _IRQ_INPROGRESS bit will be set when the interrupt fires. */
> +gicv2_poke_irq(irqd, GICD_ISPENDR);
> +}
> +else
> +{
> +/* The _IRQ_INPROGRESS remains unchanged. */
> +gicv2_poke_irq(irqd, GICD_ICPENDR);
> +}
> +}
> +
>  static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>  uint32_t cfg, actual, edgebit;
> @@ -1278,6 +1317,8 @@ const static struct gic_hw_operations gicv2_ops = {
>  .eoi_irq = gicv2_eoi_irq,
>  .deactivate_irq  = gicv2_dir_irq,
>  .read_irq= gicv2_read_irq,
> +.set_active_state= gicv2_set_active_state,
> +.set_pending_state   = gicv2_set_pending_state,
>  .set_irq_type= gicv2_set_irq_type,
>  .set_irq_priority= gicv2_set_irq_priority,
>  .send_SGI= gicv2_send_SGI,
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index cb41844af2..a5105ac9e7 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -477,6 +477,41 @@ static unsigned int gicv3_read_irq(void)
>  return irq;
>  }
>  
> +/*
> + * This is forcing the active state of an interrupt, somewhat circumventing
> + * the normal interrupt flow and the GIC state machine. So use with care
> + * and only if you know what you are doing. For this reason we also have to
> + * tinker with the _IRQ_INPROGRESS bit here, since the normal IRQ handler
> + * will not be involved.
> + */
> +static void gicv3_set_active_state(struct irq_desc *irqd, bool active)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +if ( active )
> +{
> +set_bit(_IRQ_INPROGRESS, >status);
> +gicv3_poke_irq(irqd, GICD_ISACTIVER, false);
> +}
> +else
> +{
> +clear_bit(_IRQ_INPROGRESS, >status);
> +gicv3_poke_irq(irqd, GICD_ICACTIVER, false);
> +}
> +}
> +
> +static void gicv3_set_pending_state(struct irq_desc *irqd, bool pending)
> +{
> +ASSERT(spin_is_locked(>lock));
> +
> +if ( pending )
> +/* The _IRQ_INPROGRESS bit will be set when the interrupt fires. */
> +gicv3_poke_irq(irqd, GICD_ISPENDR, false);
> +else
> +/* The _IRQ_INPROGRESS bit will remain unchanged. */
> +gicv3_poke_irq(irqd, GICD_ICPENDR, false);
> +}
> +
>  static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
>  {
>   uint64_t mpidr = cpu_logical_map(cpu);
> @@ -1769,6 +1804,8 @@ static const struct gic_hw_operations gicv3_ops = {
>  .eoi_irq = gicv3_eoi_irq,
>  .deactivate_irq  = gicv3_dir_irq,
>  .read_irq= gicv3_read_irq,
> +.set_active_state= gicv3_set_active_state,
> +

Re: [Xen-devel] [PATCH v3 04/39] ARM: GIC: Allow reading pending state of a hardware IRQ

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> To synchronize level triggered interrupts which are mapped into a guest,
> we need to update the virtual line level at certain points in time.
> For a hardware mapped interrupt the GIC is the only place where we can
> easily access this information.
> Implement a gic_hw_operations member to return the pending state of a
> particular interrupt. Due to hardware limitations this only works for
> private interrupts of the current CPU, so there is no CPU field in the
> prototype.
> This adds gicv2/3_peek_irq() helper functions, to read a bit in a bitmap
> spread over several MMIO registers.
> 
> Signed-off-by: Andre Przywara 
> Reviewed-by: Julien Grall 

Acked-by: Stefano Stabellini 


> ---
> Changelog v2 ... v3:
> - introduce gicv[23]_peek_irq() (moved from patch before)
> 
>  xen/arch/arm/gic-v2.c | 15 +++
>  xen/arch/arm/gic-v3.c | 19 +++
>  xen/include/asm-arm/gic.h | 11 +++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index d1f1578c05..b440a45e8e 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -243,6 +243,15 @@ static void gicv2_poke_irq(struct irq_desc *irqd, 
> uint32_t offset)
>  writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4);
>  }
>  
> +static bool gicv2_peek_irq(struct irq_desc *irqd, uint32_t offset)
> +{
> +uint32_t reg;
> +
> +reg = readl_gicd(offset + (irqd->irq / 32) * 4) & (1U << (irqd->irq % 
> 32));
> +
> +return reg;
> +}
> +
>  static void gicv2_set_active_state(struct irq_desc *irqd, bool active)
>  {
>  ASSERT(spin_is_locked(>lock));
> @@ -580,6 +589,11 @@ static unsigned int gicv2_read_apr(int apr_reg)
> return readl_gich(GICH_APR);
>  }
>  
> +static bool gicv2_read_pending_state(struct irq_desc *irqd)
> +{
> +return gicv2_peek_irq(irqd, GICD_ISPENDR);
> +}
> +
>  static void gicv2_irq_enable(struct irq_desc *desc)
>  {
>  unsigned long flags;
> @@ -1325,6 +1339,7 @@ const static struct gic_hw_operations gicv2_ops = {
>  .write_lr= gicv2_write_lr,
>  .read_vmcr_priority  = gicv2_read_vmcr_priority,
>  .read_apr= gicv2_read_apr,
> +.read_pending_state  = gicv2_read_pending_state,
>  .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
>  .make_hwdom_madt = gicv2_make_hwdom_madt,
>  .get_hwdom_extra_madt_size = gicv2_get_hwdom_extra_madt_size,
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index f244d51661..5c9a783968 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -444,6 +444,19 @@ static void gicv3_poke_irq(struct irq_desc *irqd, u32 
> offset, bool wait_for_rwp)
>  gicv3_wait_for_rwp(irqd->irq);
>  }
>  
> +static bool gicv3_peek_irq(struct irq_desc *irqd, u32 offset)
> +{
> +void __iomem *base;
> +unsigned int irq = irqd->irq;
> +
> +if ( irq >= NR_GIC_LOCAL_IRQS)
> +base = GICD + (irq / 32) * 4;
> +else
> +base = GICD_RDIST_SGI_BASE;
> +
> +return !!(readl(base + offset) & (1U << (irq % 32)));
> +}
> +
>  static void gicv3_unmask_irq(struct irq_desc *irqd)
>  {
>  gicv3_poke_irq(irqd, GICD_ISENABLER, false);
> @@ -1144,6 +1157,11 @@ static unsigned int gicv3_read_apr(int apr_reg)
>  }
>  }
>  
> +static bool gicv3_read_pending_state(struct irq_desc *irqd)
> +{
> +return gicv3_peek_irq(irqd, GICD_ISPENDR);
> +}
> +
>  static void gicv3_irq_enable(struct irq_desc *desc)
>  {
>  unsigned long flags;
> @@ -1812,6 +1830,7 @@ static const struct gic_hw_operations gicv3_ops = {
>  .write_lr= gicv3_write_lr,
>  .read_vmcr_priority  = gicv3_read_vmcr_priority,
>  .read_apr= gicv3_read_apr,
> +.read_pending_state  = gicv3_read_pending_state,
>  .secondary_init  = gicv3_secondary_cpu_init,
>  .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
>  .make_hwdom_madt = gicv3_make_hwdom_madt,
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 2aca243ac3..58b910fe6a 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -373,6 +373,8 @@ struct gic_hw_operations {
>  unsigned int (*read_vmcr_priority)(void);
>  /* Read APRn register */
>  unsigned int (*read_apr)(int apr_reg);
> +/* Query the pending state of an interrupt at the distributor level. */
> +bool (*read_pending_state)(struct irq_desc *irqd);
>  /* Secondary CPU init */
>  int (*secondary_init)(void);
>  /* Create GIC node for the hardware domain */
> @@ -417,6 +419,15 @@ static inline void gic_set_pending_state(struct irq_desc 
> *irqd, bool state)
>  gic_hw_ops->set_pending_state(irqd, state);
>  }
>  
> +/*
> + * Read the pending state of an interrupt from the distributor.
> + * For private IRQs this only works for those of the current CPU.
> + */
> 

Re: [Xen-devel] [PATCH v3 02/39] ARM: GIC: add GIC_INVALID to enum gic_version

[Stefano Stabellini]

On Wed, 21 Mar 2018, Andre Przywara wrote:
> The enum gic_version at the moment just contains GIC_V2 and GIC_V3,
> where GIC_V2 happens to map to 0. So without having initialised a
> variable of that type, we will read back GIC_V2 (when allocated with zeroing
> the memory).
> To prevent ambiguities and to give an explicitly uninitialised state, add
> a new first member: GIC_INVALID. Also make it obvious that this has a
> "0" encoding.
> 
> Signed-off-by: Andre Przywara 

Acked-by: Stefano Stabellini 


> ---
>  xen/include/asm-arm/gic.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 565b0875ca..3079387e06 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -227,6 +227,7 @@ struct gic_lr {
>  };
>  
>  enum gic_version {
> +GIC_INVALID = 0,/* the default until explicitly set up */
>  GIC_V2,
>  GIC_V3,
>  };
> -- 
> 2.14.1
> 

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

Re: [Xen-devel] [RFC PATCH 07/12] hvmloader: allocate MMCONFIG area in the MMIO hole + minor code refactoring

[Alexey G]

On Mon, 26 Mar 2018 10:24:38 +0100
Roger Pau Monné  wrote:

>On Sat, Mar 24, 2018 at 08:32:44AM +1000, Alexey G wrote:
[...]
>> In fact, the emulated chipset (NB+SB combo without supplemental
>> devices) itself is a small part of required emulation. It's
>> relatively easy to provide own analogs of for eg. 'mch' and
>> 'ICH9-LPC' QEMU PCIDevice's, the problem is to glue all remaining
>> parts together.
>> 
>> I assume the final goal in this case is to have only a set of
>> necessary QEMU PCIDevice's for which we will be providing I/O, MMIO
>> and PCI conf trapping facilities. Only devices such as rtl8139,
>> ich9-ahci and few others.
>> 
>> Basically, this means a new, chipset-less QEMU machine type.
>> Well, in theory it is possible with a bit of effort I think. The main
>> question is where will be the NB/SB/PCIbus emulating part reside in
>> this case.  
>
>Mostly inside of Xen. Of course the IDE/SATA/USB/Ethernet... part of
>the southbrigde will be emulated by a device model (ie: QEMU).
>
>As you mention above, I also took a look and it seems like the amount
>of registers that we should emulate for Q35 DRAM controller (D0:F0) is
>fairly minimal based on current QEMU implementation. We could even
>possibly get away by just emulating PCIEXBAR.

MCH emulation alone might be not an option. Besides, some
southbridge-specific features like emulating ACPI PM facilities for
domain power management (basically, anything at PMBASE) will be
preferable to implement on Xen side, especially considering the fact
that ACPI tables are already provided by Xen's libacpi/hvmloader, not
the device model.
I think the feature may require to cover at least the NB+SB
combination, at least Q35 MCH + ICH9 for start, ideally 82441FX+PIIX4
as well. Also, Xen should control emulated/PT PCI device placement.

Before going this way, it would be good to measure all risks.
Looks like there are two main directions currently:

I. (conservative) Let the main device model (QEMU) to inform Xen about
the current chipset-specific MMCONFIG location, to allow Xen to know
that some MMIO accesses to this area must be forwarded to other ioreq
servers (device emulators) in a form of PCI config read/write ioreqs,
if BDF corresponding to a MMCONFIG offset will point to the PCI device
owned by a device emulator.
In case of device emulators the conversion of MMIO accesses to PCI
config ones is a mandatory step, while the owner of the MMCONFIG MMIO
range may receive MMIO accesses in a native form without conversion
(a strongly preferable option for QEMU).

This approach assumes introducing of the new dmop/hypercall (something
like XEN_DMOP_mmcfg_location_change) to pass to Xen basic MMCONFIG
information -- address, enabled/disabled status (or simply address=0
instead) and size of the MMCONFIG area, eg. as a number of buses.
This information is enough to select a proper ioreq server in Xen and
allow multiple device emulators to function properly.
For future compatibility we can also provide the segment and
start/end bus range as arguments.

What this approach will require:


- new notification-style dmop/hypercall to tell Xen about the current
  emulated MMCONFIG location

- trivial changes in QEMU to use this dmop in Q35 PCIEXBAR handling code

- relatively simple Xen changes in ioreq.c to use the provided range
  for ioreq server selection. Also, to provide MMIO -> PCI config ioreq
  translation for supplemental ioreq servers which don't know anything
  about the emulated system

Risks:
--

Risk to break anything is minimal in this case.

If QEMU will not provide this information (eg. due to an outdated
version installed), only basic PCI config space accesses via CF8/CFC
will be forwarded to a distinct ioreq server. This means the extended
PCI config space accesses won't be forwarded to specific device
emulators. Other than these device emulators, anything else will
continue to work properly in this case. No differences will be for
guest OSes without PCIe ECAM support in either case.

In general, no breakthrough improvements, no negative side-effects.
Just PCIe ECAM working as expected and compatibility with multiple
ioreq servers is retained.


II. (a new feature) Move chipset emulation to Xen directly.

In this case no separate notification necessary as Xen will be
emulating the chosen chipset itself. MMCONFIG location will be known
from own PCIEXBAR emulation.

QEMU will be used only to emulate a minimal set of unrelated devices
(eg. storage/network/vga). Less dependency on QEMU overall.

More freedom to implement some specific features in the future like
smram support for EFI firmware needs. Chipset remapping (aka reclaim)
functionality for memory relocation may be implemented under complete
Xen control, avoiding usage of unsafe add_to_physmap hypercalls.

In future this will allow to move passthrough-supporting code from QEMU
(hw/xen/xen-pt*.c) to Xen, merging it with Roger's vpci series.

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

[Anthony PERARD]

The first two patchs fix save of a VM with a restricted QEMU, and the later two
patchs try to fix the restore path, but it is still WIP.

Checkout the last patch comments for more information.

Anthony PERARD (4):
  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

 tools/libxl/libxl_dm.c   |  3 ++
 tools/libxl/libxl_exec.c | 95 
 tools/libxl/libxl_internal.h | 17 
 tools/libxl/libxl_qmp.c  | 95 ++--
 4 files changed, 199 insertions(+), 11 deletions(-)

-- 
Anthony PERARD


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

[Xen-devel] [RFC 1/4] 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 d03cb51668..5bf5236240 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -80,6 +80,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,
@@ -378,6 +381,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;
 }
 
@@ -602,9 +607,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 3/4] 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 8dd63319fc..d5e98114d6 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1856,6 +1856,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 7b69d6a4de..9f4ba12ff0 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1209,6 +1209,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 4/4] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Anthony PERARD]

This path is more of a prof of concept reather than a patch as this
would break qemu-trad.

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.

For libxl, the only way to find out if qemu is ready on migrate/restore,
it is to connect to the QMP socket and run "query-status".

This patch succeed in implementing that, but QMP doesn't fit well with
the libxl__ev_* infrastructure. One main issue would be qmp_open(), it
tries to connect to the QMP socket during 5 seconds without ever giving
back the hand to libxl.

Also right now, xswait is disabled, but libxl could check both
xenstore and QMP at the same time.

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_dm.c   |  3 ++
 tools/libxl/libxl_exec.c | 95 
 tools/libxl/libxl_internal.h | 14 +++
 3 files changed, 105 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a3cddce8b7..43314e3309 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2350,6 +2350,9 @@ retry_transaction:
 spawn->failure_cb = device_model_startup_failed;
 spawn->detached_cb = device_model_detached;
 
+// HACK, disable xenstore watch, will instead use QMP
+spawn->xspath = NULL;
+
 rc = libxl__spawn_spawn(egc, spawn);
 if (rc < 0)
 goto out_close;
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 02e6c917f0..2b5db5197a 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -274,6 +274,58 @@ void libxl__spawn_init(libxl__spawn_state *ss)
 libxl__xswait_init(>xswait);
 }
 
+static void qmpwait_callback(libxl__egc *egc,
+  libxl__ev_time *ev,
+  const struct timeval *requested_abs,
+  int rc)
+{
+libxl__spawn_state *ss = CONTAINER_OF(ev, *ss, qmpwait.qmp_ev);
+libxl__dm_spawn_state *dmss = CONTAINER_OF(ss, *dmss, spawn);
+STATE_AO_GC(ss->ao);
+
+if (rc == ERROR_TIMEDOUT) /* As intended */
+rc = 0;
+else
+goto out_err;
+
+rc = libxl__qmp_query_status(gc, dmss->guest_domid, "running");
+
+if (rc) {
+/* retry QMP connection later */
+libxl__ev_time_register_rel(ss->ao,
+ev,
+qmpwait_callback,
+100);
+return;
+}
+
+libxl__spawn_initiate_detach(gc, ss);
+return;
+out_err:
+LOG(DEBUG, "qmpwait failure: %d", rc);
+ss->failure_cb(egc, ss, rc);
+}
+
+static void qmpwait_report_error(libxl__egc *egc, libxl__qmpwait_state *qmpwa,
+int rc)
+{
+libxl__spawn_state *ss = CONTAINER_OF(qmpwa, *ss, qmpwait);
+EGC_GC;
+libxl__ev_time_deregister(gc, >time_ev);
+libxl__ev_time_deregister(gc, >qmp_ev);
+qmpwa->callback(egc, >xswait, rc, 0);
+}
+
+static void qmpwait_timeout_callback(libxl__egc *egc, libxl__ev_time *ev,
+ const struct timeval *requested_abs,
+ int rc)
+{
+EGC_GC;
+libxl__qmpwait_state *qmpwa = CONTAINER_OF(ev, *qmpwa, time_ev);
+LOG(DEBUG, "%s: qmpwait timeout", qmpwa->what);
+qmpwait_report_error(egc, qmpwa, rc);
+}
+
 int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
 {
 STATE_AO_GC(ss->ao);
@@ -284,13 +336,35 @@ int libxl__spawn_spawn(libxl__egc *egc, 
libxl__spawn_state *ss)
 libxl__spawn_init(ss);
 ss->rc = ss->detaching = 0;
 
-ss->xswait.ao = ao;
-ss->xswait.what = GCSPRINTF("%s startup", ss->what);
-ss->xswait.path = ss->xspath;
-ss->xswait.timeout_ms = ss->timeout_ms;
-ss->xswait.callback = spawn_watch_event;
-rc = libxl__xswait_start(gc, >xswait);
-if (rc) goto out_err;
+if (ss->xspath) {
+ss->xswait.ao = ao;
+ss->xswait.what = GCSPRINTF("%s startup", ss->what);
+ss->xswait.path = ss->xspath;
+ss->xswait.timeout_ms = ss->timeout_ms;
+ss->xswait.callback = spawn_watch_event;
+rc = libxl__xswait_start(gc, >xswait);
+if (rc) goto out_err;
+} else {
+libxl__qmpwait_state *qmpwa = >qmpwait;
+
+ss->qmpwait.ao = ao;
+ss->qmpwait.what = GCSPRINTF("%s startup (QMP)", ss->what);
+/*ss->qmpwait.guest_domid = ;*/
+ss->qmpwait.timeout_ms = ss->timeout_ms;
+ss->qmpwait.callback = spawn_watch_event;
+
+libxl__ev_time_init(>time_ev);
+libxl__ev_time_init(>qmp_ev);
+
+rc = libxl__ev_time_register_rel(qmpwa->ao, >time_ev,
+ qmpwait_timeout_callback,
+ qmpwa->timeout_ms);
+if (rc) goto out_err;
+
+rc = libxl__ev_time_register_rel(ss->ao, >qmp_ev,
+ 

[Xen-devel] [RFC 2/4] 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 5bf5236240..7b69d6a4de 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -952,25 +952,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] [xen-unstable-smoke test] 121301: tolerable all pass - PUSHED

[osstest service owner]

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

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  9f5b0ce10b2895b4136c9e5c5ebd0aebac31ea98
baseline version:
 xen  eabb83121226d5a6a5a68da3a913ac0b5bb1e0cf

Last test of basis   121090  2018-03-23 17:09:54 Z3 days
Testing same since   121297  2018-03-26 14:01:21 Z0 days2 attempts


People who touched revisions under test:
  Roger Pau Monné 

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
   eabb831212..9f5b0ce10b  9f5b0ce10b2895b4136c9e5c5ebd0aebac31ea98 -> smoke

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

[Xen-devel] [linux-3.18 test] 121268: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-libvirt  broken
 build-armhf-libvirt   5 host-build-prep  fail REGR. vs. 121099

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 121099
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 121099
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 121099
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 121099
 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-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 build-arm64-pvops 6 kernel-build fail   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  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-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 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-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-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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-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-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-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-i386-xl-qemut-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

version targeted for testing:
 linux9764536dc592144beee43c987fef45d2e91ca55c
baseline version:
 linux44ec71c0cd728e8cbd346e135eef9b43b03654ab

Last test of basis   121099  2018-03-23 23:27:33 Z2 days
Testing same since   121268  2018-03-25 10:19:05 Z1 days1 attempts


People who touched revisions under test:
  Aaron Brown 
  Alexandre Belloni 
  Alexey Kardashevskiy 
  Alexey Khoroshilov 
  Anthony Brandon 
  Bartlomiej Zolnierkiewicz 
  Bernd Faust 
  Bjorn Helgaas 
  

Re: [Xen-devel] [PATCH] docs/qemu-deprivilege: Revise and update with status and future plans

[Ian Jackson]

Thanks for this update!

George Dunlap writes ("[PATCH] docs/qemu-deprivilege: Revise and update with 
status and future plans"):
...
> +# Technical details
> +
> +## Restrictions done

This makes this doc into a mixture of a design doc and a user doc, I
think.

It might be worth stating the design intent, which I think is this:

 * Even if there is a bug (for example in qemu) which permits a domain
   to compromise the device model, the compromised device model
   process is prevented from violating the system's overall security
   properties.  Ie, a guest cannot "escape" from the virtualisation by
   using a qemu bug.

This design intent is not yet achieved.  Right now an attacker is
impeded and their attack is complicated; in some circumstances the
will be limited to denial of service.

I'm not sure the individual restrictions need to be in a user-facing
doc.

Maybe the user-facing wording from your patch should be moved to
xl.cfg.doc.5 ?

> +'''Description''': Close and restrict Xen-related file descriptors.
> +Specifically, make sure that only one `privcmd` instance is open, and
> +that the IOCTL_EVTCHN_RESTRICT_DOMID ioctl has been called.
> +
> +XXX Also, make sure that only one `xenstore` fd remains open, and that
> +it's restricted.

No.  Firstly, in each case, all relevant descriptors are restricted.
This is the purpose of the xentoolcore__restrict_* stuff.  Secondly,
xenstore *is* covered - but the xs fd is squashed so as to be totally
unuseable: xs.c uses xentoolcore__restrict_by_dup2_null.

> +### Namespaces for unused functionality
> +
> +'''Descripiton''': Enter QEMU into its own mount & IPC namespaces.
> +This means that even if other restrictions fail, the process won't be
> +able to even name system mount points or exsting non-file-based IPC
> +descriptors to attempt to attack them.
> +
> +'''Implementation''':
> +
> +In theory this could be done in QEMU (similar to -sandbox, -runas,
> +-chroot, and so on), but a patch doing this in QEMU was NAKed
> +upstream. They preferred that this was done as a setup step by
> +whatever executes QEMU; i.e., have the process which exec's QEMU first
> +call:
> +
> +unshare(CLONE_NEWNS | CLONE_NEWIPC)

This would mean we would have to pass qemu fds for both the network
tap devices and any vnc consoles.  That makes life considerably more
complicated.  I think we should perhaps revisit this upstream.

> +'''Implementation''': Enable from the command-line:
> +
> +-sandbox 
> on,obsolete=deny,elevateprivileges=allow,spawn=deny,resourcecontrol=deny
> +
> +`elevateprivileges` is currently required to allow `-runas` to work.
> +Removing this requirement would mean making sure that the uid change
> +happened before the seccomp2 call, perhaps by changing the uid before
> +executing QEMU.  (But this would then require other changes to create
> +the QMP socket, VNC socket, and so on).

See what I say above.

> +### Further RLIMITs
> +
> +RLIMIT_AS limits the total amount of memory; but this includes the
> +virtual memory which QEMU uses as a mapcache.  xen-mapcache.c already
> +fiddles with this; it would be straightforward to make it *set* the
> +rlimit to what it thinks a sensible limit is.
> +
> +Other things that would take some cleverness / changes to QEMU to
> +utilize due to ordering constrants:
> + - RLIMIT_NPROC (after uid changes to a unique uid)
> + - RLIMIT_NOFILES (after all necessary files are opened)

I think there is little difficulty with RLIMIT_NPROC since our qemu
does not fork.  I think we can set it to a value which is currently
violated for the current uid ?

> +### libxl UID cleanup
...
> +kill(-1,sig) sends a signal to "every process to which the calling
> +process has permission to send a signal".  So in theory:
> +  setuid(X)
> +  kill(-1,KILL)
> +should do the trick.

We need to check whether a malicious qemu process could kill this
one.

> +### Disks
> +
> +The chroot (and seccomp?) happens late enough such that QEMU can
> +initialize itself and open its disks. If you want to add a disk at run
> +time via or insert a CD, you can't pass a path because QEMU is
> +chrooted. Instead use the add-fd QMP command and use
> +/dev/fdset/ as the path.

I don't think we (Xen) really support hotplug of emulated disks right
now.  So it's just cd insert that's a problem.

> +### Network
>  
> +If QEMU runs in its own network namespace, it can't open the tap
> +device itself because the interface won't be visible outside of its
> +own namespace. So instead, have the toolstack open the device and pass
> +it as an fd on the command-line:

I think this could be solved by doing these things in a different
order.

Thanks,
Ian.

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

[Xen-devel] [PATCH v2 for-4.11 2/2] vpci: make sure handlers can deal with size == 0

[Roger Pau Monne]

The code is not prepared to handle such case, so just return early. In
the debug case add an assert.

Reported-by: Coverity
Coverity ID: 1430809
Signed-off-by: Roger Pau Monné 
---
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 
---
 xen/drivers/vpci/vpci.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 2913b56500..82607bdb9a 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -320,6 +320,12 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg, 
unsigned int size)
 unsigned int data_offset = 0;
 uint32_t data = ~(uint32_t)0;
 
+if ( !size )
+{
+ASSERT_UNREACHABLE();
+return data;
+}
+
 /* Find the PCI dev matching the address. */
 pdev = pci_get_pdev_by_domain(d, sbdf.seg, sbdf.bus, sbdf.extfunc);
 if ( !pdev )
@@ -416,6 +422,12 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, 
unsigned int size,
 const struct vpci_register *r;
 unsigned int data_offset = 0;
 
+if ( !size )
+{
+ASSERT_UNREACHABLE();
+return;
+}
+
 /*
  * Find the PCI dev matching the address.
  * Passthrough everything that's not trapped.
-- 
2.16.2


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

Re: [Xen-devel] [PATCH 4/4] drivers/net: Use octal not symbolic permissions

[David Miller]

From: Joe Perches 
Date: Fri, 23 Mar 2018 15:54:39 -0700

> Prefer the direct use of octal for permissions.
> 
> Done with checkpatch -f --types=SYMBOLIC_PERMS --fix-inplace
> and some typing.
> 
> Miscellanea:
> 
> o Whitespace neatening around these conversions.
> 
> Signed-off-by: Joe Perches 

Applied.

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

Re: [Xen-devel] [PATCH 19/20] xen/domain: Call arch_domain_create() as early as possible in domain_create()

[Jan Beulich]

>>> On 19.03.18 at 20:13,  wrote:
> This is in preparation to set up d->max_cpus and d->vcpu[] in
> arch_domain_create(), and allow later parts of domain construction to have
> access to the values.

I'm not convinced of the allocation to be done in arch_domain_create()
(in the next patch), and hence I'm not sure we need what you do here.
I can see that you want arch code to audit the vCPU count, but why
does arch code need to set up the array and store ->max_vcpus? You
could simply call a per-arch auditing function early (even the lower
bound check could be done in common code), set up both fields early in
domain_create(), and leave the call to arch_domain_create() where it is
now, couldn't you? As the ARM auditing may need to do more than just
auditing (to select the vGIC variant), perhaps this could be
arch_domain_configure(), doing the auditing and all necessary setup
needed for the auditing to be possible.

Jan


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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf   6 xen-buildfail REGR. vs. 121090

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 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

version targeted for testing:
 xen  9f5b0ce10b2895b4136c9e5c5ebd0aebac31ea98
baseline version:
 xen  eabb83121226d5a6a5a68da3a913ac0b5bb1e0cf

Last test of basis   121090  2018-03-23 17:09:54 Z2 days
Testing same since   121297  2018-03-26 14:01:21 Z0 days1 attempts


People who touched revisions under test:
  Roger Pau Monné 

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


Not pushing.


commit 9f5b0ce10b2895b4136c9e5c5ebd0aebac31ea98
Author: Roger Pau Monné 
Date:   Mon Mar 26 15:17:12 2018 +0200

vpci/msix: fix incorrect usage of bitmask

The bitmask to clear the low bits of the address field should be
~0xull, the current mask clears both the low and the high bits
of the address field, which is a bug.

Reported-by: Coverity
Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 

commit 91a8ffe3d1bc43498b76abb63d2dfb078dd5bd01
Author: Roger Pau Monné 
Date:   Mon Mar 26 15:16:14 2018 +0200

vpci/bars: fix error message

Error message is incorrectly using map when it should be using
map->map instead.

Coverity ID: 1430811

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
(qemu changes not included)

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

Re: [Xen-devel] [PATCH 2/5] tools/firmware: #define IPXE_PATH

[Anoob Soman]

On 21/03/18 15:18, Wei Liu wrote:

On Thu, Mar 15, 2018 at 05:31:50PM +, Anoob Soman wrote:

--with-system-ipxe allows the user to specify ipxe rom. If this option
is given, use system supplied ipxe instead of building and installing
our own version

Plumbing for using iPXE roms, specified with --with-system-ipxe, doesn't
exist and will be added in future commits.

Re-run of autoconf is needed.

Signed-off-by: Anoob Soman 

This looks fine. But it should probably be squashed to one of the other


Sure, let me see what I can do.


patches.

Wei.




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

Re: [Xen-devel] [PATCH 4/5] libxl: Load iPXE ROM from a file

[Anoob Soman]

On 21/03/18 15:25, Wei Liu wrote:



+LOGE(ERROR, "xc_dom_kernel_file failed");
+goto out;
+}
+if ((ipxe_filename = libxl__ipxe_path())) {
+rc = xc_dom_module_file(dom, ipxe_filename, "ipxe");
+if (rc) {
+LOGE(ERROR, "xc_dom_ipxe_module_file failed");
+goto out;

This is the wrong place. Being an HVM guest doesn't mean ipxe should be
loaded. You probably need to look a few lines down and add code in
appropriate places like when ROMBIOS is picked.


Sure, I will fix it up.



Wei.




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

Re: [Xen-devel] [PATCH 3/5] libxc: Allow loading of firmware modules for HVM guest

[Anoob Soman]

On 21/03/18 15:17, Wei Liu wrote:

On Thu, Mar 15, 2018 at 05:31:51PM +, Anoob Soman wrote:

 modlist, start_info);
+for ( i = 0; i < dom->num_modules; i++ )
+{
+struct xc_hvm_firmware_module mod;
+
+DOMPRINTF("Adding module %u", i);
+mod.guest_addr_out =
+dom->modules[i].seg.vstart;

Shouldn't this be dom->modules[i].seg.vstart - dom->parms.virt_base?


It didn't work when guest_addr_out = seg.vstart - virt_base. I will try 
to figure out why.





+mod.length =
+dom->modules[i].seg.vend - dom->modules[i].seg.vstart;
+
+add_module_to_list(dom, , dom->modules[i].cmdline,
+   modlist, start_info);
+}

Now that both paths of the if ... else ... structure contain the same
code it should be lifted outside of the loop.


Sure, I can do that.

-Anoob

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

Re: [Xen-devel] [PATCH v1.1 for-4.11 0/3] vpci bugfixes

[Roger Pau Monné]

On Mon, Mar 26, 2018 at 06:39:21AM -0600, Jan Beulich wrote:
> >>> On 26.03.18 at 13:28,  wrote:
> > This tree patches are bugfixes for the vPCI code merged last week. They
> > where spotted by Coverity.
> 
> Thanks for dealing with them. You having omitted Coverity IDs I
> suppose the report you've looked at was from the XenServer internal
> instance. That would also explain why you have a fix for an issue the
> open source instance didn't spot. It spotted another issue though:
> 
> *** CID 1430809:(BAD_SHIFT)
> /xen/drivers/vpci/vpci.c: 382 in vpci_read()
> 376  size - data_offset);
> 377 
> 378 data = merge_result(data, tmp_data, size - data_offset, 
> data_offset);
> 379 }
> 380 spin_unlock(>vpci->lock);
> 381 
> >>> CID 1430809:(BAD_SHIFT)
> >>> In expression "0xU >> 32U - 8U * size", right shifting by 
> >>> more than 31 bits has undefined behavior.  The shift amount, "32U - 8U * 
> >>> size", is 32.
> 382 return data & (0x >> (32 - 8 * size));
> 383 }
> 
> And indeed there's no way I can see that it could prove size to
> only ever be 1, 2, or 4. I can't figure whether they've actually
> found a code path where size could end up being zero here. I
> think/hope a suitable ASSERT() would help.

I've also seen that one, but was wondering whether this should be
fixed in the handler dispatcher code instead. But seeing that
vpci_read/write can be called from both the IO or the MMIO handlers, I
guess it's best to just add an ASSERT(size); to both the read and the
write handlers.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH 18/20] xen/dom0: Arrange for dom0_cfg to contain the real max_vcpus value

[Jan Beulich]

>>> On 19.03.18 at 20:13,  wrote:
> Make dom0_max_vcpus() a common interface, and implement it on ARM by splitting
> the existing alloc_dom0_vcpu0() function in half.
> 
> As domain_create() doesn't yet set up the vcpu array, the max value is also
> passed into alloc_dom0_vcpu0().  This is temporary for bisectibility and
> removed in the following patch.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 



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

[Xen-devel] [SVM]NPF page mapping

[Alexandru Stefan ISAILA]

Hello,

We are trying to introduce the Pace Access functionality on SVM. Right
now, on the p2m_pt_set_entry() the p2m_access_t is not set and in the
p2m_pt_get_entry() it is returned as p2m_access_rwx by default.

Could you please suggest some up-to-date documentation on NPT,
especially illustrating the mapping of the u64 intpte_t type? Also, any
suggestions on how to approach this are appreciated.


Thanks,
Alex


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

Re: [Xen-devel] [PATCH v1 06/15] arm64: Add accessors for the ICH_APxRn_EL2 registers

[Marc Zyngier]

On 26/03/18 14:19, Manish Jaggi wrote:
> Hi Marc,
> 
> I have a query on this patch. The original patch was using these 
> functions so it was ok to make them static.
> But this patch is not touching the xen vgic code similar to what your 
> patch did.
> 
> Will it be ok to merge this patch with 
> https://www.spinics.net/lists/arm-kernel/msg587089.html

I can only repeat the argument I try to convey earlier. By changing the
structure of the series, you're making it harder to review it, as it is
not possible to look at two patches side by side and work out what changed.

In the end, that's your call. If you want to change the shape of the
series, go for it. But also appreciate the consequences of doing so.

M.
-- 
Jazz is not dead. It just smells funny...

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

Re: [Xen-devel] [PATCH 17/20] xen/gnttab: Fold grant_table_{create, set_limits}() into grant_table_init()

[Jan Beulich]

>>> On 19.03.18 at 20:13,  wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1808,22 +1808,28 @@ gnttab_grow_table(struct domain *d, unsigned int 
> req_nr_frames)
>  return -ENOMEM;
>  }
>  
> -static int
> -grant_table_init(struct domain *d, struct grant_table *gt,
> - unsigned int grant_frames, unsigned int maptrack_frames)
> +int grant_table_init(struct domain *d, unsigned int max_grant_frames,
> + unsigned int max_maptrack_frames)
>  {
> +struct grant_table *gt;
>  int ret = -ENOMEM;
>  
> -grant_write_lock(gt);
> +if ( max_grant_frames < INITIAL_NR_GRANT_FRAMES ||
> + max_grant_frames > opt_max_grant_frames ||
> + max_maptrack_frames > opt_max_maptrack_frames )
> +return -EINVAL;
>  
> -if ( gt->active )
> -{
> -ret = -EBUSY;
> -goto out_no_cleanup;
> -}
> +if ( (gt = xzalloc(struct grant_table)) == NULL )
> +return -ENOMEM;
> +
> +/* Simple stuff. */
> +percpu_rwlock_resource_init(>lock, grant_rwlock);
> +spin_lock_init(>maptrack_lock);
> +
> +grant_write_lock(gt);

This was sort of sensible with the old (split) code structure, but
without having stored gt anywhere yet I think you want to
acquire this later. Of course otoh there's not going to be any
contention on the lock here, and it doesn't look like you can
avoid acquiring the lock altogether.

> @@ -1871,7 +1881,6 @@ grant_table_init(struct domain *d, struct grant_table 
> *gt,
>  gt->active = NULL;
>  }
>  
> - out_no_cleanup:

You now want to free gt upwards from here (where the closing
brace is visible in context - the caller won't call grant_table_destroy()
when an error comes back from here. Or wait - that's a bug in patch
16 already. So I'm afraid I have to withdraw my R-b given there.

Jan


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

Re: [Xen-devel] [PATCH 16/20] xen/gnttab: Pass max_{grant, maptrack}_frames into grant_table_create()

[Jan Beulich]

>>> On 19.03.18 at 20:13,  wrote:
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2078,7 +2078,8 @@ static void __init find_gnttab_region(struct domain *d,
>   * enough space for a large grant table
>   */
>  kinfo->gnttab_start = __pa(_stext);
> -kinfo->gnttab_size = gnttab_dom0_frames() << PAGE_SHIFT;
> +kinfo->gnttab_size = min_t(unsigned int, opt_max_grant_frames,
> +   PFN_DOWN(_etext - _stext)) << PAGE_SHIFT;

ARM folks will know whether having the same expression here
and ...

> @@ -695,6 +696,17 @@ void __init start_xen(unsigned long boot_phys_offset,
>  struct domain *dom0;
>  struct xen_domctl_createdomain dom0_cfg = {
>  .max_evtchn_port = -1,
> +
> +/*
> + * The region used by Xen on the memory will never be mapped in DOM0
> + * memory layout. Therefore it can be used for the grant table.
> + *
> + * Only use the text section as it's always present and will contain
> + * enough space for a large grant table
> + */
> +.max_grant_frames = min_t(unsigned int, opt_max_grant_frames,
> +  PFN_DOWN(_etext - _stext)),

... here isn't risking someone updating one but not the other.

> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -547,11 +547,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>  op->domain = d->domain_id;
>  copyback = true;
>  
> -ret = grant_table_set_limits(d, op->u.createdomain.max_grant_frames,
> - op->u.createdomain.max_maptrack_frames);
> -if ( !ret )
> -goto createdomain_fail_late;

With this grant_table_set_limits() can become static, at which point
it becomes questionable whether it wouldn't better be expanded
into its only caller. Oh, looks like that's the subject of patch 17. In
which case non-ARM bits
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 15/20] xen/gnttab: Export opt_max_{grant, maptrack}_frames

[Jan Beulich]

>>> On 19.03.18 at 20:13,  wrote:
> This is to facilitate the values being passed in via domain_create(), at which
> point the dom0 construction code needs to know them.
> 
> While cleaning up, drop the DEFAULT_* defines, which are only used immediately
> adjacent in a context which makes it obvious that they are the defaults, and
> drop the (unused) logic to allow a per-arch override of
> DEFAULT_MAX_NR_GRANT_FRAMES.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 



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

Re: [Xen-devel] [PATCH 4/4] drivers/net: Use octal not symbolic permissions

[Wei Liu]

On Fri, Mar 23, 2018 at 03:54:39PM -0700, Joe Perches wrote:
> Prefer the direct use of octal for permissions.
> 
> Done with checkpatch -f --types=SYMBOLIC_PERMS --fix-inplace
> and some typing.
> 
> Miscellanea:
> 
> o Whitespace neatening around these conversions.
> 
> Signed-off-by: Joe Perches 
> ---
>  drivers/net/xen-netback/xenbus.c   |  4 +-
>  drivers/net/xen-netfront.c |  6 +--

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH 14/20] xen/gnttab: Remove replace_grant_supported()

[Jan Beulich]

>>> On 19.03.18 at 20:13,  wrote:
> It is identical on all architecture, and this is a better overall than fixing
> it up to have a proper boolean return value.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 



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

Re: [Xen-devel] [PATCH 13/20] xen/evtchn: Pass max_evtchn_port into evtchn_init()

[Jan Beulich]

>>> On 19.03.18 at 20:13,  wrote:
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -693,7 +693,9 @@ void __init start_xen(unsigned long boot_phys_offset,
>  const char *cmdline;
>  struct bootmodule *xen_bootmodule;
>  struct domain *dom0;
> -struct xen_domctl_createdomain dom0_cfg = {};
> +struct xen_domctl_createdomain dom0_cfg = {
> +.max_evtchn_port = -1,
> +};
>  
>  dcache_line_bytes = read_dcache_line_bytes();
>  
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -673,6 +673,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  };
>  struct xen_domctl_createdomain dom0_cfg = {
>  .flags = XEN_DOMCTL_CDF_s3_integrity,
> +.max_evtchn_port = -1,
>  };

Any chance I can talk you into using INT_MAX, UINT_MAX, or ~0u
in both cases above, seeing that max_evtchn_port's type is
unsigned int? Other than this
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 01/12] checkpatch: Add xendevicemodel_handle to the list of types

[Ian Jackson]

Paolo Bonzini writes ("Re: [PATCH 01/12] checkpatch: Add xendevicemodel_handle 
to the list of types"):
> On 08/03/2018 20:02, Ian Jackson wrote:
> > This avoids checkpatch misparsing (as statements) long function
> > definitions or declarations, which sometimes start with constructs
> > like this:
> > 
> >   static inline int xendevicemodel_relocate_memory(
> >   xendevicemodel_handle *dmod, domid_t domid, ...
...
> 
> Or just rename it so that it is CamelCase.  Then checkpatch will be happy.

I can do that if you want, although it seems a bit like pointless
churn.  If I do that it ought to be renamed as well as re-spelled
because xendevicemodel_handle is actually a struct, not a "handle"
(ie, not a scalar).[1]  So
   s/xendevicemodel_handle/XendevicemodelInterface/
I guess ?

[1] If it were a scalar CODING_STYLE says it should be in lowercase
with _t.  This is a violation of the C standard, which reserves names
ending _t for the implementation...

Ian.

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

Re: [Xen-devel] Sunseting mercurial

[Doug Goldstein]

On 3/26/18 5:35 AM, Jan Beulich wrote:
 On 25.03.18 at 04:46,  wrote:
>> Its been officially 5+ years since Xen has moved to git so I propose we
>> start thinking about when to retire the mercurial mirrors. At this point
>> the last stable version to be tracked in mercurial is 4.4 which is long
>> out of any form of support. I know some vendors still have support for
>> versions of Xen down to 4.1 but let's be realistic, there's not a flurry
>> of development happening in those old versions. The mercurial mirror is
>> often out of date (I know someone that's tried to use it) and in fact as
>> of this email its several weeks out of date.
>>
>> So maybe its time we start thinking about sunsetting the mercurial
>> mirrors and use those resources for more practical uses.
> 
> This was brought up before, and I continue to agree _as long_ as
> our web representation of the tree gains something similar to
> hg's "annotate" functionality. Without that I find it quite hard to
> locate commits most recently changing a line or an area of code.
> Of course aiui this can be done from the command line, but only if
> one happens to have a repo on the particular machine (which for
> example I don't have or intend to have at home).
> 

While I agree that gitweb should be changed to support showing that,
there exist a few options out there as well that work today.

https://gitlab.com/xen-project/xen/blame/staging/xen/common/Makefile
https://github.com/xen-project/xen/blame/staging/xen/common/Makefile

Browse to any file and click the Blame button or remember the URL and go
to it manually or here's a 10 second script.

#!/bin/bash -e

url="http://gitlab.com/xen-project/xen/blame;
thefile=$(readlink -f $1)
repo=$(git rev-parse --show-toplevel)
relfile=${thefile##${repo}}
branch=$(git rev-parse --abbrev-ref HEAD)
xdg-open "${url}/${branch}${relfile}"


-- 
Doug Goldstein

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

Re: [Xen-devel] [PATCH 4/3] README: document that qemu-trad wants libpci

[Jan Beulich]

>>> On 26.03.18 at 15:22,  wrote:
> On 3/26/18 5:18 AM, Jan Beulich wrote:
> On 25.03.18 at 16:26,  wrote:
>>> qemu-traditional wants libpci from pciutils for supporting PCI
>>> passthrough.
>> 
>> Iirc it builds fine without, disabling respective code. Hence I don't
>> think this fully fits the other (strict) requirements.
> 
> I put it under the section that reads:
> 
> In addition to the above there are a number of optional build
> prerequisites. Omitting these will cause the related features to be
> disabled at compile time:
> 
> I figured with that caveat and mentioning PCI passthrough that it was
> good enough but I can happily reword further.

The immediately preceding item is

* 16-bit x86 assembler, loader and compiler for qemu-traditional / rombios
  (dev86 rpm or bin86 & bcc debs)

Aiui qemu-trad won't build and/or work without these. As mentioned
I believe qemu-trad both builds and works fine without libpci, just
that PCI passthrough won't be possible. How about

* libpci from pciutils for qemu-traditional to support PCI passthrough

? That's still not as explicitly optional as I would want it, but I can't
think of any better wording (without the entry getting undesirably
long).

Jan


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

Re: [Xen-devel] Sunseting mercurial

[Jan Beulich]

>>> On 26.03.18 at 15:18,  wrote:
> On Mon, Mar 26, 2018 at 11:56:49AM +0100, Anthony PERARD wrote:
>> I'll fix the mirror.
> 
> It's fixed now.

Thanks a lot!

Jan


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

Re: [Xen-devel] [PATCH] x86/boot: Disable IBRS in intr/nmi exit path at bootup stage

[Jan Beulich]

>>> On 21.03.18 at 03:58,  wrote:
> After reset, IBRS is disabled by processor, but a coming intr/nmi leave IBRS
> enabled after their exit. It's not necessory for bootup code to run in low
> performance with IBRS enabled.
> 
> On ORACLE X6-2(500GB/88 cpus, dom0 11GB/20 vcpus), we observed an 200s+ delay
> in construct_dom0.
> 
> By initializing use_shadow_spec_ctrl with 1, IBRS is disabled in intr/nmi exit
> path at bootup stage. Then delay in construct_dom0 is ~50s.

While I can certainly follow the argumentation, did you pay
attention to Andrew also modifying what you would call "bootup
code" in commit 7c508612f7 ("x86: Support indirect thunks from
assembly code")? That wasn't just a random change - we
specifically want it for the case of bringing up CPUs at runtime.
You'll need to be equally careful here, I think: Rather than
storing literal 1 (which should have been "true" anyway), you'll
want to store (system_state < SYS_STATE_active) or maybe
(system_state != SYS_STATE_active), at least when the CPU
being booted is a hyperthread of a CPU which is active already.

Jan


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

Re: [Xen-devel] broken build

[Wei Liu]

On Sat, Mar 24, 2018 at 04:20:14PM -0400, Jacob Embree wrote:
> This is a followup to https://xen.markmail.org/thread/6bzbtbbprexrli2f
> 
> According to 
> https://stackoverflow.com/questions/20369672/undefined-reference-to-dlsym
> a couple more flags are needed.
> 
> commit 22c7e6d2204114c25625558e1f9634f264f9ac7c
> Author: Jacob Embree 
> Date:   Sat Mar 24 15:51:31 2018 -0400
> 
> Fix LDFLAGS for libxenstore.so
> 
> Signed-off-by: Jacob Embree 
> 
> diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
> index 69e55e7..1a636ee 100644
> --- a/tools/xenstore/Makefile
> +++ b/tools/xenstore/Makefile
> @@ -104,7 +104,7 @@ libxenstore.so.$(MAJOR): libxenstore.so.$(MAJOR).$(MINOR)
>  xs.opic: CFLAGS += -DUSE_PTHREAD
>  ifeq ($(CONFIG_Linux),y)
>  xs.opic: CFLAGS += -DUSE_DLSYM
> -libxenstore.so.$(MAJOR).$(MINOR): LDFLAGS += -ldl
> +libxenstore.so.$(MAJOR).$(MINOR): LDFLAGS += -Wl,--no-as-needed,-ldl
>  else
>  PKG_CONFIG_REMOVE += -ldl
>  endif
> 

I think this is fixed by 1a3731949 with another method.

Wei.

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

[Xen-devel] [xen-4.7-testing test] 121247: tolerable FAIL - PUSHED

[osstest service owner]

flight 121247 xen-4.7-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/121247/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-xtf-amd64-amd64-2 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 121093 pass 
in 121247
 test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 121093 pass 
in 121247
 test-armhf-armhf-libvirt-raw 15 guest-start/debian.repeat fail in 121093 pass 
in 121247
 test-xtf-amd64-amd64-4   96 leak-check/check   fail pass in 121093
 test-armhf-armhf-xl-rtds 12 guest-startfail pass in 121093

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 121093 like 
121047
 test-armhf-armhf-xl-rtds13 migrate-support-check fail in 121093 never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 121093 never pass
 test-xtf-amd64-amd64-1  50 xtf/test-hvm64-lbr-tsx-vmentry fail like 121047
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 121047
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 121047
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 121047
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 121047
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 121047
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 121047
 test-armhf-armhf-libvirt-xsm 14 saverestore-support-checkfail  like 121047
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 121047
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 121047
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 121047
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 121047
 test-xtf-amd64-amd64-3   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-1   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-2   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-4   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-5   52 xtf/test-hvm64-memop-seg 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-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-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  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-cubietruck 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-cubietruck 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 13 migrate-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-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-amd64-i386-xl-qemuu-win10-i386 10 windows-install

Re: [Xen-devel] [PATCH 4/3] README: document that qemu-trad wants libpci

[Doug Goldstein]

On 3/26/18 5:18 AM, Jan Beulich wrote:
 On 25.03.18 at 16:26,  wrote:
>> qemu-traditional wants libpci from pciutils for supporting PCI
>> passthrough.
> 
> Iirc it builds fine without, disabling respective code. Hence I don't
> think this fully fits the other (strict) requirements.
> 
> Jan
> 

I put it under the section that reads:

In addition to the above there are a number of optional build
prerequisites. Omitting these will cause the related features to be
disabled at compile time:

I figured with that caveat and mentioning PCI passthrough that it was
good enough but I can happily reword further.

-- 
Doug Goldstein

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

Re: [Xen-devel] [PATCH v1 06/15] arm64: Add accessors for the ICH_APxRn_EL2 registers

[Manish Jaggi]

Hi Marc,

I have a query on this patch. The original patch was using these 
functions so it was ok to make them static.
But this patch is not touching the xen vgic code similar to what your 
patch did.


Will it be ok to merge this patch with 
https://www.spinics.net/lists/arm-kernel/msg587089.html


-manish


On 03/16/2018 05:28 PM, Manish Jaggi wrote:

This patch is ported to xen from linux commit
63000dd8006dc987db31ba670edc23142ea91e01

As we're about to access the Active Priority registers a lot more,
let's define accessors that take the register number as a parameter.

This patch only has accessors, another patch will have register trap handlers

Signed-off-by: Manish Jaggi 

diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 114d5107a9..1aaade40dc 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -114,6 +114,98 @@ void handle_igrpen1(struct cpu_user_regs *regs, int regidx,
  __vgic_v3_write_igrpen1(regs, regidx);
  }
  
+void  __vgic_v3_write_ap0rn(uint32_t val, int n)

+{
+switch (n)
+{
+case 0:
+WRITE_SYSREG32(val, ICH_AP0R0_EL2);
+break;
+case 1:
+WRITE_SYSREG32(val, ICH_AP0R1_EL2);
+break;
+case 2:
+WRITE_SYSREG32(val, ICH_AP0R2_EL2);
+break;
+case 3:
+WRITE_SYSREG32(val, ICH_AP0R3_EL2);
+break;
+default:
+unreachable();
+}
+}
+
+void __vgic_v3_write_ap1rn(uint32_t val, int n)
+{
+switch (n)
+{
+case 0:
+WRITE_SYSREG32(val, ICH_AP1R0_EL2);
+break;
+case 1:
+WRITE_SYSREG32(val, ICH_AP1R1_EL2);
+break;
+case 2:
+WRITE_SYSREG32(val, ICH_AP1R2_EL2);
+break;
+case 3:
+WRITE_SYSREG32(val, ICH_AP1R3_EL2);
+break;
+default:
+unreachable();
+}
+}
+
+uint32_t  __vgic_v3_read_ap0rn(int n)
+{
+uint32_t val;
+
+switch (n)
+{
+case 0:
+val = READ_SYSREG32(ICH_AP0R0_EL2);
+break;
+case 1:
+val = READ_SYSREG32(ICH_AP0R1_EL2);
+break;
+case 2:
+val = READ_SYSREG32(ICH_AP0R2_EL2);
+break;
+case 3:
+val = READ_SYSREG32(ICH_AP0R3_EL2);
+break;
+default:
+unreachable();
+}
+
+return val;
+}
+
+uint32_t  __vgic_v3_read_ap1rn(int n)
+{
+uint32_t val;
+
+switch (n)
+{
+case 0:
+val = READ_SYSREG32(ICH_AP1R0_EL2);
+break;
+case 1:
+val = READ_SYSREG32(ICH_AP1R1_EL2);
+break;
+case 2:
+val = READ_SYSREG32(ICH_AP1R2_EL2);
+break;
+case 3:
+val = READ_SYSREG32(ICH_AP1R3_EL2);
+break;
+default:
+unreachable();
+}
+
+return val;
+}
+
  bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr 
hsr)
  {
  bool ret = true;



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

Re: [Xen-devel] Sunseting mercurial

[Anthony PERARD]

On Mon, Mar 26, 2018 at 11:56:49AM +0100, Anthony PERARD wrote:
> I'll fix the mirror.

It's fixed now.

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH v1 04/15] arm64: vgic-v3: Add ICV_BPR1_EL1 handler

[Manish Jaggi]

On 03/21/2018 01:41 PM, Julien Grall wrote:

Hi Manish,

On 03/16/2018 11:58 AM, Manish Jaggi wrote:

This patch is ported to xen from linux commit
d70c7b31a60f2458f35c226131f2a01a7a98b6cf

Add a handler for reading/writing the guest's view of the ICC_BPR1_EL1
register, which is located in the ICH_VMCR_EL2.BPR1 field.

Signed-off-by: Manish Jaggi 


As you port commit by commit, please at least mention Marc as he is 
the original author of that patch.
I have a little confusion, should a signoff be added once it is reviewed 
or even if it is ported?

I thought once it is acked, we add signoff.

Please suggest.



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

Re: [Xen-devel] [PATCH v1 05/15] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler

[Manish Jaggi]

Hi Julien,


On 03/21/2018 02:08 PM, Julien Grall wrote:



On 03/16/2018 11:58 AM, Manish Jaggi wrote:

This patch is ported to xen from linux commit:
f8b630bc542e0368886ae193d3519c832b270359

Add a handler for reading/writing the guest's view of the
ICC_IGRPEN1_EL1


The wrapping looks wrong.


register, which is located in the ICH_VMCR_EL2.VENG1 field.

Signed-off-by: Manish Jaggi 

diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c 
b/xen/arch/arm/arm64/vgic-v3-sr.c

index 364785d3ac..114d5107a9 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -86,6 +86,34 @@ void handle_bpr1(struct cpu_user_regs *regs, int 
regidx, const union hsr hsr)

  __vgic_v3_write_bpr1(regs, regidx);
  }
  +static void  __vgic_v3_read_igrpen1(struct cpu_user_regs *regs, 
int regidx)


Please remove the __ for all the functions.

ok



+{
+    uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);


Newline.

For 2 line function should there be a newline?
Will add it.



+    set_user_reg(regs, regidx, !!(vmcr & ICH_VMCR_ENG1_MASK));
+}
+
+static void  __vgic_v3_write_igrpen1(struct cpu_user_regs *regs, int 
regidx)

+{
+    register_t val = get_user_reg(regs, regidx);
+    uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+
+    if ( val & 1 )
+    vmcr |= ICH_VMCR_ENG1_MASK;
+    else
+    vmcr &= ~ICH_VMCR_ENG1_MASK;
+
+    WRITE_SYSREG32(vmcr, ICH_VMCR_EL2);


So basically, you ported commit without even looking if there were 
change on top. For instance the latest code has a specific has an 
helper to update vmcr. Can you please make sure that all the code is 
ported?



Well julien, I did that and it was in original code as well.
__vgic_v3_write_vmcr was called which was calling write_gicreg(vmcr, 
ICH_VMCR_EL2).

Xen aleady has a macro so it is better to use xen macro.

I can write that in commit log if that helps in easier review

+}
+
+void handle_igrpen1(struct cpu_user_regs *regs, int regidx,
+    const union hsr hsr)
+{
+    if ( hsr.sysreg.read )
+    __vgic_v3_read_igrpen1(regs, regidx);
+    else
+    __vgic_v3_write_igrpen1(regs, regidx);
+}
+
  bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const 
union hsr hsr)

  {
  bool ret = true;
@@ -105,6 +133,10 @@ bool vgic_v3_handle_cpuif_access(struct 
cpu_user_regs *regs, const union hsr hsr

   handle_bpr1(regs, regidx, hsr);
   break;
  +    case HSR_SYSREG_ICC_IGRPEN1_EL1:
+    handle_igrpen1(regs, regidx, hsr);
+    break;
+
  default:
  ret = false;
  break;
diff --git a/xen/include/asm-arm/arm64/sysregs.h 
b/xen/include/asm-arm/arm64/sysregs.h

index 025a27b0b4..731cabc74a 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -90,6 +90,7 @@
  #define HSR_SYSREG_ICC_SGI0R_EL1  HSR_SYSREG(3,2,c12,c11,7)
  #define HSR_SYSREG_ICC_SRE_EL1    HSR_SYSREG(3,0,c12,c12,5)
  #define HSR_SYSREG_ICC_BPR1_EL1   HSR_SYSREG(3,0,c12,c12,3)
+#define HSR_SYSREG_ICC_IGRPEN1_EL1 HSR_SYSREG(3,0,c12,c12,7)
  #define HSR_SYSREG_CONTEXTIDR_EL1 HSR_SYSREG(3,0,c13,c0,1)
    #define HSR_SYSREG_PMCR_EL0   HSR_SYSREG(3,3,c9,c12,0)
diff --git a/xen/include/asm-arm/gic_v3_defs.h 
b/xen/include/asm-arm/gic_v3_defs.h

index 68a34cc353..ff8bda37d1 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -163,6 +163,8 @@
  #define ICH_VMCR_BPR0_MASK   (7 << ICH_VMCR_BPR0_SHIFT)
  #define ICH_VMCR_BPR1_SHIFT  18
  #define ICH_VMCR_BPR1_MASK   (7 << ICH_VMCR_BPR1_SHIFT)
+#define ICH_VMCR_ENG1_SHIFT  1
+#define ICH_VMCR_ENG1_MASK   (1 << ICH_VMCR_ENG1_SHIFT)
    #define GICH_LR_VIRTUAL_MASK 0x
  #define GICH_LR_VIRTUAL_SHIFT    0



Cheers,



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

Re: [Xen-devel] [PATCH v18 10/11] common: add a new mappable resource type: XENMEM_resource_grant_table

[Jan Beulich]

>>> On 26.03.18 at 14:16,  wrote:
 On 22.03.18 at 12:55,  wrote:
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -3863,6 +3863,35 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, 
>> grant_ref_t ref,
>>  }
>>  #endif
>>  
>> +/* caller must hold read or write lock */
>> +static int gnttab_get_status_frame_mfn(struct domain *d,
>> +   unsigned long idx, mfn_t *mfn)
>> +{
>> +struct grant_table *gt = d->grant_table;
>> +
>> +if ( idx >= nr_status_frames(gt) )
>> +return -EINVAL;
>> +
>> +*mfn = _mfn(virt_to_mfn(gt->status[idx]));
>> +return 0;
>> +}
>> +
>> +/* caller must hold write lock */
>> +static int gnttab_get_shared_frame_mfn(struct domain *d,
>> +   unsigned long idx, mfn_t *mfn)
>> +{
>> +struct grant_table *gt = d->grant_table;
>> +
>> +if ( (idx >= nr_grant_frames(gt)) && (idx < gt->max_grant_frames) )
>> +gnttab_grow_table(d, idx + 1);
>> +
>> +if ( idx >= nr_grant_frames(gt) )
>> +return -EINVAL;
>> +
>> +*mfn = _mfn(virt_to_mfn(gt->shared_raw[idx]));
>> +return 0;
>> +}
> 
> I realize the anomaly was there already before, but imo it becomes
> more pronounced with the two functions differing in more than just
> the shared vs status naming (IOW I find it strange that one grows
> the grant table while the other doesn't). This extends to ...
> 
>> +int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
>> +mfn_t *mfn)
>> +{
>> +struct grant_table *gt = d->grant_table;
>> +int rc;
>> +
>> +grant_write_lock(gt);
>> +rc = gnttab_get_shared_frame_mfn(d, idx, mfn);
>> +grant_write_unlock(gt);
>> +
>> +return rc;
>> +}
>> +
>> +int gnttab_get_status_frame(struct domain *d, unsigned long idx,
>> +mfn_t *mfn)
>> +{
>> +struct grant_table *gt = d->grant_table;
>> +int rc;
>> +
>> +grant_read_lock(gt);
>> +rc = gnttab_get_status_frame_mfn(d, idx, mfn);
>> +grant_read_unlock(gt);
>> +
>> +return rc;
>> +}
> 
> ... these two acquiring the lock in different ways.
> 
> And then I'm completely missing the interaction with
> gnttab_unpopulate_status_frames(). While this might not be a
> practical problem at this point in time, we're liable to forget to
> address this later on if there's no stop gap measure. A PV guest
> mapping the obtained MFNs is going to be fine, but a HVM/PVH
> one isn't, since neither x86 nor ARM refcount pages inserted into
> or removed from a domain's p2m. I therefore think you need to
> add a is_hvm_domain() check to acquire_grant_table(), with a
> suitable fixme comment attached to it.

Or perhaps better paging_mode_translate(current->domain).

Jan


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

Re: [Xen-devel] [PATCH v18 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Jan Beulich]

>>> On 26.03.18 at 13:41,  wrote:
 On 22.03.18 at 12:55,  wrote:
>> --- a/xen/include/public/memory.h
>> +++ b/xen/include/public/memory.h
>> @@ -599,6 +599,59 @@ struct xen_reserved_device_memory_map {
>>  typedef struct xen_reserved_device_memory_map 
>> xen_reserved_device_memory_map_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t);
>>  
>> +/*
>> + * Get the pages for a particular guest resource, so that they can be
>> + * mapped directly by a tools domain.
>> + */
>> +#define XENMEM_acquire_resource 28
>> +struct xen_mem_acquire_resource {
>> +/* IN - The domain whose resource is to be mapped */
>> +domid_t domid;
>> +/* IN - the type of resource */
>> +uint16_t type;
>> +/*
>> + * IN - a type-specific resource identifier, which must be zero
>> + *  unless stated otherwise.
>> + */
>> +uint32_t id;
>> +/*
>> + * IN/OUT - As an IN parameter number of frames of the resource
>> + *  to be mapped. However, if the specified value is 0 and
>> + *  frame_list is NULL then this field will be set to the
>> + *  maximum value supported by the implementation on return.
>> + */
>> +uint32_t nr_frames;
>> +/*
>> + * OUT - Must be zero on entry. On return this may contain a bitwise
>> + *   OR of the following values.
>> + */
>> +uint32_t flags;
>> +
>> +/* The resource pages have been assigned to the tools domain */
>> +#define _XENMEM_resource_flag_tools_owned 0
>> +#define XENMEM_resource_flag_tools_owned (1u << 
> _XENMEM_resource_flag_tools_owned)
> 
> Is "tools" really an appropriate (and "flag" a necessary) name
> component here? How about e.g. XENMEM_res_acq_caller_owned?

Or maybe XENMEM_rsrc_acq_caller_owned.

Jan


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

Re: [Xen-devel] [PATCH v3] drm/xen-front: Add support for Xen PV display frontend

[Oleksandr Andrushchenko]

On 03/26/2018 11:18 AM, Daniel Vetter wrote:

On Fri, Mar 23, 2018 at 05:54:49PM +0200, Oleksandr Andrushchenko wrote:

My apologies, but I found a few more things that look strange and should
be cleaned up. Sorry for this iterative review approach, but I think we're
slowly getting there.

Thank you for reviewing!

Cheers, Daniel


---
+static int xen_drm_drv_dumb_create(struct drm_file *filp,
+   struct drm_device *dev, struct drm_mode_create_dumb *args)
+{
+   struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+   struct drm_gem_object *obj;
+   int ret;
+
+   ret = xen_drm_front_gem_dumb_create(filp, dev, args);
+   if (ret)
+   goto fail;
+
+   obj = drm_gem_object_lookup(filp, args->handle);
+   if (!obj) {
+   ret = -ENOENT;
+   goto fail_destroy;
+   }
+
+   drm_gem_object_unreference_unlocked(obj);

You can't drop the reference while you keep using the object, someone else
might sneak in and destroy your object. The unreference always must be
last.

Will fix, thank you

+
+   /*
+* In case of CONFIG_DRM_XEN_FRONTEND_CMA gem_obj is constructed
+* via DRM CMA helpers and doesn't have ->pages allocated
+* (xendrm_gem_get_pages will return NULL), but instead can provide
+* sg table
+*/
+   if (xen_drm_front_gem_get_pages(obj))
+   ret = xen_drm_front_dbuf_create_from_pages(
+   drm_info->front_info,
+   xen_drm_front_dbuf_to_cookie(obj),
+   args->width, args->height, args->bpp,
+   args->size,
+   xen_drm_front_gem_get_pages(obj));
+   else
+   ret = xen_drm_front_dbuf_create_from_sgt(
+   drm_info->front_info,
+   xen_drm_front_dbuf_to_cookie(obj),
+   args->width, args->height, args->bpp,
+   args->size,
+   xen_drm_front_gem_get_sg_table(obj));
+   if (ret)
+   goto fail_destroy;
+

The above also has another race: If you construct an object, then it must
be fully constructed by the time you publish it to the wider world. In gem
this is done by calling drm_gem_handle_create() - after that userspace can
get at your object and do nasty things with it in a separate thread,
forcing your driver to Oops if the object isn't fully constructed yet.

That means you need to redo this code here to make sure that the gem
object is fully set up (including pages and sg tables) _before_ anything
calls drm_gem_handle_create().

You are correct, I need to rework this code

This probably means you also need to open-code the cma side, by first
calling drm_gem_cma_create(), then doing any additional setup, and finally
doing the registration to userspace with drm_gem_handle_create as the very
last thing.

Although I tend to avoid open-coding, but this seems the necessary measure
here

Alternativet is to do the pages/sg setup only when you create an fb (and
drop the pages again when the fb is destroyed), but that requires some
refcounting/locking in the driver.

Not sure this will work: nothing prevents you from attaching multiple FBs to
a single dumb handle
So, not only ref-counting should be done here, but I also need to check if
the dumb buffer,
we are attaching to, has been created already

No, you must make sure that no dumb buffer can be seen by anyone else
before it's fully created. If you don't register it in the file_priv idr
using drm_gem_handle_create, no one else can get at your buffer. Trying to
paper over this race from all the other places breaks the gem core code
design, and is also much more fragile.

Yes, this is what I implement now, e.g. I do not create
any dumb handle until GEM is fully created. I was just
saying that alternative way when we do pages/sgt on FB
attach will not work in my case

So, I will rework with open-coding some stuff from CMA helpers


Aside: There's still a lot of indirection and jumping around which makes
the code a bit hard to follow.

Probably I am not sure of which indirection we are talking about, could you
please
specifically mark those annoying you?

I think it's the same indirection we talked about last time, it still
annoys me. But it's still ok if you prefer this way I think :-)

Ok, probably this is because I'm looking at the driver
from an editor, but you are from your mail client ;)

+
+static void xen_drm_drv_release(struct drm_device *dev)
+{
+   struct xen_drm_front_drm_info *drm_info = dev->dev_private;
+   struct xen_drm_front_info *front_info = drm_info->front_info;
+
+   drm_atomic_helper_shutdown(dev);
+   drm_mode_config_cleanup(dev);
+
+   xen_drm_front_evtchnl_free_all(front_info);
+   dbuf_free_all(_info->dbuf_list);
+
+   drm_dev_fini(dev);
+   kfree(dev);
+
+ 

Re: [Xen-devel] [PATCH v1.1 for-4.11 0/3] vpci bugfixes

[Jan Beulich]

>>> On 26.03.18 at 13:28,  wrote:
> This tree patches are bugfixes for the vPCI code merged last week. They
> where spotted by Coverity.

Thanks for dealing with them. You having omitted Coverity IDs I
suppose the report you've looked at was from the XenServer internal
instance. That would also explain why you have a fix for an issue the
open source instance didn't spot. It spotted another issue though:

*** CID 1430809:(BAD_SHIFT)
/xen/drivers/vpci/vpci.c: 382 in vpci_read()
376  size - data_offset);
377 
378 data = merge_result(data, tmp_data, size - data_offset, 
data_offset);
379 }
380 spin_unlock(>vpci->lock);
381 
>>> CID 1430809:(BAD_SHIFT)
>>> In expression "0xU >> 32U - 8U * size", right shifting by more 
>>> than 31 bits has undefined behavior.  The shift amount, "32U - 8U * size", 
>>> is 32.
382 return data & (0x >> (32 - 8 * size));
383 }

And indeed there's no way I can see that it could prove size to
only ever be 1, 2, or 4. I can't figure whether they've actually
found a code path where size could end up being zero here. I
think/hope a suitable ASSERT() would help.

Jan


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

Re: [Xen-devel] [PATCH v1.1 for-4.11 3/3] vpci/msi: fix size of the vectors fields

[Jan Beulich]

>>> On 26.03.18 at 13:28,  wrote:
> The current size (5bits) is not enough to store the maximum number of
> vectors (32), bump it by one bit.
> 
> Note that the size of the struct is still the same.

Coverity ID: 1430810

> --- a/xen/include/xen/vpci.h
> +++ b/xen/include/xen/vpci.h
> @@ -100,7 +100,7 @@ struct vpci {
>  /* Data. */
>  uint16_t data;
>  /* Maximum number of vectors supported by the device. */
> -uint8_t max_vectors : 5;
> +uint8_t max_vectors : 6;
>  /* Enabled? */
>  bool enabled: 1;
>  /* Supports per-vector masking? */

To aid simplicity of generated code, I had specifically asked for the
current 5-1-1-1-5 arrangement of bit field members. Now that the
5s need bumping to 6, we'll want 6-1-1-6-1, so please move
"enabled" down (also resulting in all feature flags coming before
all state ones). With that
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 v1.1 for-4.11 2/3] vpci/msix: fix incorrect usage of bitmask

[Jan Beulich]

>>> On 26.03.18 at 13:28,  wrote:
> The bitmask to clear the low bits of the address field should be
> ~0xull, the current mask clears both the low and the high bits
> of the address field, which is a bug.
> 
> Reported-by: Coverity
> Signed-off-by: Roger Pau Monné 

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 v1.1 for-4.11 1/3] vpci/bars: fix error message

[Jan Beulich]

>>> On 26.03.18 at 13:28,  wrote:
> Error message is incorrectly using map when it should be using
> map->map instead.
> 
> Reported-by: Coverity

Coverity ID: 1430811

> Signed-off-by: Roger Pau Monné 

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 v3 7/7] xen/x86: use PCID feature

[Juergen Gross]

On 26/03/18 14:19, Jan Beulich wrote:
 On 26.03.18 at 14:04,  wrote:
>> On 26/03/18 12:43, Jan Beulich wrote:
>> On 26.03.18 at 12:29,  wrote:
 On 26/03/18 12:13, Jan Beulich wrote:
 On 26.03.18 at 10:55,  wrote:
>> I can change the scheme to use different values for guest PCIDs
>> with XPTI on, of course. Are you fine with:
>>
>> - XPTI off: PCID 0 = kernel, PCID 1 = user
>> - XPTI on:  PCID 0 = kernel/Xen, PCID 1 = user/Xen,
>> PCID 2 = kernel/guest, PCID 3 = user/guest
>
> Yes, that would fit the first variant I've described. I take it you
> prefer not to avoid PCID 0 altogether when PCIDs are enabled -
> is there a particular reason?

 Yes. As written in the comment in flush_area_local() I can't be sure
 whether the current address space is that of a domain with XPTI
 enabled (the idle domain could be "current"). So I need to always
 flush with PCID 0 and with the possible PCID values for a XPTI domain.
 When using PCID 0 for XPTI as well I'll need 4 INVPCIDs, while when
 avoiding it I'd need 5 (at least when current == idle).
>>>
>>> I see. Which makes me wonder whether a suitable combination
>>> of INVLPG (to get rid of global entries) and INVPCID couldn't be
>>> used instead. For example, you may be able to replace the
>>> INVPCID for the active PCID by INVLPG (without needing to
>>> know who "current" is).
>>
>> INVLPG has the disadvantage to clear all paging-structure cache
>> entries associated with the current PCID.
>>
>> And I thought we were finally on the same page not to use global
>> pages with PCID enabled?
> 
> Yes, we are. If no global TLB entries can ever survive a CR4.PCIDE
> 0 -> 1 transition, all would be fine without INVLPG of course.

Okay, I'll add an ASSERT() to make sure this is true:

ASSERT(!cr4.pge || !cr4.pcide)


Juergen


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

Re: [Xen-devel] [PATCH v6] hvm/svm: Implement Debug events

[Jan Beulich]

>>> On 26.03.18 at 11:43,  wrote:
> At this moment the Debug events for the AMD architecture are not
> forwarded to the monitor layer.
> 
> This patch adds the Debug event to the common capabilities, adds
> the VMEXIT_ICEBP then forwards the event to the monitor layer.
> 
> Chapter 2: SVM Processor and Platform Extensions: "Note: A vector 1
> exception generated by the single byte INT1
> instruction (also known as ICEBP) does not trigger the #DB
> intercept. Software should use the dedicated ICEBP
> intercept to intercept ICEBP"
> 
> Signed-off-by: Alexandru Isaila 

Applicable parts
Acked-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 v3 7/7] xen/x86: use PCID feature

[Jan Beulich]

>>> On 26.03.18 at 14:04,  wrote:
> On 26/03/18 12:43, Jan Beulich wrote:
> On 26.03.18 at 12:29,  wrote:
>>> On 26/03/18 12:13, Jan Beulich wrote:
>>> On 26.03.18 at 10:55,  wrote:
> I can change the scheme to use different values for guest PCIDs
> with XPTI on, of course. Are you fine with:
>
> - XPTI off: PCID 0 = kernel, PCID 1 = user
> - XPTI on:  PCID 0 = kernel/Xen, PCID 1 = user/Xen,
> PCID 2 = kernel/guest, PCID 3 = user/guest

 Yes, that would fit the first variant I've described. I take it you
 prefer not to avoid PCID 0 altogether when PCIDs are enabled -
 is there a particular reason?
>>>
>>> Yes. As written in the comment in flush_area_local() I can't be sure
>>> whether the current address space is that of a domain with XPTI
>>> enabled (the idle domain could be "current"). So I need to always
>>> flush with PCID 0 and with the possible PCID values for a XPTI domain.
>>> When using PCID 0 for XPTI as well I'll need 4 INVPCIDs, while when
>>> avoiding it I'd need 5 (at least when current == idle).
>> 
>> I see. Which makes me wonder whether a suitable combination
>> of INVLPG (to get rid of global entries) and INVPCID couldn't be
>> used instead. For example, you may be able to replace the
>> INVPCID for the active PCID by INVLPG (without needing to
>> know who "current" is).
> 
> INVLPG has the disadvantage to clear all paging-structure cache
> entries associated with the current PCID.
> 
> And I thought we were finally on the same page not to use global
> pages with PCID enabled?

Yes, we are. If no global TLB entries can ever survive a CR4.PCIDE
0 -> 1 transition, all would be fine without INVLPG of course.

Jan


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

Re: [Xen-devel] [PATCH v18 10/11] common: add a new mappable resource type: XENMEM_resource_grant_table

[Jan Beulich]

>>> On 22.03.18 at 12:55,  wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3863,6 +3863,35 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, 
> grant_ref_t ref,
>  }
>  #endif
>  
> +/* caller must hold read or write lock */
> +static int gnttab_get_status_frame_mfn(struct domain *d,
> +   unsigned long idx, mfn_t *mfn)
> +{
> +struct grant_table *gt = d->grant_table;
> +
> +if ( idx >= nr_status_frames(gt) )
> +return -EINVAL;
> +
> +*mfn = _mfn(virt_to_mfn(gt->status[idx]));
> +return 0;
> +}
> +
> +/* caller must hold write lock */
> +static int gnttab_get_shared_frame_mfn(struct domain *d,
> +   unsigned long idx, mfn_t *mfn)
> +{
> +struct grant_table *gt = d->grant_table;
> +
> +if ( (idx >= nr_grant_frames(gt)) && (idx < gt->max_grant_frames) )
> +gnttab_grow_table(d, idx + 1);
> +
> +if ( idx >= nr_grant_frames(gt) )
> +return -EINVAL;
> +
> +*mfn = _mfn(virt_to_mfn(gt->shared_raw[idx]));
> +return 0;
> +}

I realize the anomaly was there already before, but imo it becomes
more pronounced with the two functions differing in more than just
the shared vs status naming (IOW I find it strange that one grows
the grant table while the other doesn't). This extends to ...

> +int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
> +mfn_t *mfn)
> +{
> +struct grant_table *gt = d->grant_table;
> +int rc;
> +
> +grant_write_lock(gt);
> +rc = gnttab_get_shared_frame_mfn(d, idx, mfn);
> +grant_write_unlock(gt);
> +
> +return rc;
> +}
> +
> +int gnttab_get_status_frame(struct domain *d, unsigned long idx,
> +mfn_t *mfn)
> +{
> +struct grant_table *gt = d->grant_table;
> +int rc;
> +
> +grant_read_lock(gt);
> +rc = gnttab_get_status_frame_mfn(d, idx, mfn);
> +grant_read_unlock(gt);
> +
> +return rc;
> +}

... these two acquiring the lock in different ways.

And then I'm completely missing the interaction with
gnttab_unpopulate_status_frames(). While this might not be a
practical problem at this point in time, we're liable to forget to
address this later on if there's no stop gap measure. A PV guest
mapping the obtained MFNs is going to be fine, but a HVM/PVH
one isn't, since neither x86 nor ARM refcount pages inserted into
or removed from a domain's p2m. I therefore think you need to
add a is_hvm_domain() check to acquire_grant_table(), with a
suitable fixme comment attached to it.

With that (and despite the noticed anomaly)
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 v3 7/7] xen/x86: use PCID feature

[Juergen Gross]

On 26/03/18 12:43, Jan Beulich wrote:
 On 26.03.18 at 12:29,  wrote:
>> On 26/03/18 12:13, Jan Beulich wrote:
>> On 26.03.18 at 10:55,  wrote:
 I can change the scheme to use different values for guest PCIDs
 with XPTI on, of course. Are you fine with:

 - XPTI off: PCID 0 = kernel, PCID 1 = user
 - XPTI on:  PCID 0 = kernel/Xen, PCID 1 = user/Xen,
 PCID 2 = kernel/guest, PCID 3 = user/guest
>>>
>>> Yes, that would fit the first variant I've described. I take it you
>>> prefer not to avoid PCID 0 altogether when PCIDs are enabled -
>>> is there a particular reason?
>>
>> Yes. As written in the comment in flush_area_local() I can't be sure
>> whether the current address space is that of a domain with XPTI
>> enabled (the idle domain could be "current"). So I need to always
>> flush with PCID 0 and with the possible PCID values for a XPTI domain.
>> When using PCID 0 for XPTI as well I'll need 4 INVPCIDs, while when
>> avoiding it I'd need 5 (at least when current == idle).
> 
> I see. Which makes me wonder whether a suitable combination
> of INVLPG (to get rid of global entries) and INVPCID couldn't be
> used instead. For example, you may be able to replace the
> INVPCID for the active PCID by INVLPG (without needing to
> know who "current" is).

INVLPG has the disadvantage to clear all paging-structure cache
entries associated with the current PCID.

And I thought we were finally on the same page not to use global
pages with PCID enabled?


Juergen

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

Re: [Xen-devel] [PATCH v18 06/11] x86/hvm/ioreq: add a new mappable resource type...

[Jan Beulich]

>>> On 22.03.18 at 12:55,  wrote:
> ... XENMEM_resource_ioreq_server
> 
> This patch adds support for a new resource type that can be mapped using
> the XENMEM_acquire_resource memory op.
> 
> If an emulator makes use of this resource type then, instead of mapping
> gfns, the IOREQ server will allocate pages from the emulating domain's
> heap. These pages will never be present in the P2M of the guest at any
> point (and are not even shared with the guest) and so are not vulnerable to
> any direct attack by the guest.

"allocate pages from the emulating domain's heap" is a sub-optimal
(at least slightly misleading) description, due to your use of
MEMF_no_refcount together with the fact that domain's don't
really have their own heaps.

> +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> +{
> +struct hvm_ioreq_page *iorp = buf ? >bufioreq : >ioreq;
> +
> +if ( iorp->page )
> +{
> +/*
> + * If a guest frame has already been mapped (which may happen
> + * on demand if hvm_get_ioreq_server_info() is called), then
> + * allocating a page is not permitted.
> + */
> +if ( !gfn_eq(iorp->gfn, INVALID_GFN) )
> +return -EPERM;
> +
> +return 0;
> +}
> +
> +/*
> + * Allocated IOREQ server pages are assigned to the emulating
> + * domain, not the target domain. This is safe because the emulating
> + * domain cannot be destroyed until the ioreq server is destroyed.
> + * Also we must use MEMF_no_refcount otherwise page allocation
> + * could fail if the emulating domain has already reached its
> + * maximum allocation.
> + */
> +iorp->page = alloc_domheap_page(s->emulator, MEMF_no_refcount);
> +
> +if ( !iorp->page )
> +return -ENOMEM;
> +
> +if ( !get_page_type(iorp->page, PGT_writable_page) )
> +goto fail;
> +
> +iorp->va = __map_domain_page_global(iorp->page);
> +if ( !iorp->va )
> +goto fail;
> +
> +clear_page(iorp->va);
> +return 0;
> +
> + fail:
> +put_page_and_type(iorp->page);

This is wrong in case it's the get_page_type() which failed.

> +int arch_acquire_resource(struct domain *d, unsigned int type,
> +  unsigned int id, unsigned long frame,
> +  unsigned int nr_frames, xen_pfn_t mfn_list[],
> +  unsigned int *flags)
> +{
> +int rc;
> +
> +switch ( type )
> +{
> +case XENMEM_resource_ioreq_server:
> +{
> +ioservid_t ioservid = id;
> +unsigned int i;
> +
> +rc = -EINVAL;
> +if ( id != (unsigned int)ioservid )
> +break;
> +
> +rc = 0;
> +for ( i = 0; i < nr_frames; i++ )
> +{
> +mfn_t mfn;
> +
> +rc = hvm_get_ioreq_server_frame(d, id, frame + i, );
> +if ( rc )
> +break;
> +
> +mfn_list[i] = mfn_x(mfn);
> +}
> +
> +/*
> + * The frames will be assigned to the tools domain that created
> + * the ioreq server.
> + */

s/will be/have been/ and perhaps drop "tools"?

> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -374,6 +374,14 @@ static inline void put_page_and_type(struct page_info 
> *page)
>  
>  void clear_and_clean_page(struct page_info *page);
>  
> +static inline int arch_acquire_resource(
> +struct domain *d, unsigned int type, unsigned int id,
> +unsigned long frame,unsigned int nr_frames, xen_pfn_t mfn_list[],

Missing blank.

Jan


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

Re: [Xen-devel] [RFC v2] xen/arm: Suspend to RAM Support in Xen for ARM

[Edgar E. Iglesias]

On Mon, Mar 26, 2018 at 09:51:40AM +, Peng Fan wrote:
> Hi Mirela,
> 
> Good to know that you are working suspend/resume support. Currently we are 
> also trying
> to support this on i.MX8, just wonder do you have any open source available to
> support suspend to ram?
> 
> > +
> > +Suspend to RAM (in the following text 'suspend') for ARM in Xen should
> > +be coordinated using ARM PSCI standard [1].
> > +
> > +Ideally, EL1/2 should suspend in the following order:
> > +1) Unprivileged guests (DomUs) suspend
> > +2) Privileged guest (Dom0) suspends
> > +3) Xen suspends
> > +
> > +However, suspending unprivileged guests is not mandatory for suspending
> > +Dom0 and Xen. System suspend initiated by Dom0 (step 2) is considered
> > +to be an ultimate decision to suspend the physical machine. Suspending
> > +of Xen (step 3) is triggered whenever Dom0 completes suspend. Xen
> > +suspend leads to the full suspend of EL2.
> > +
> > +If an unprivileged guest is not suspended at the moment when Dom0
> > +initiates its own suspend, the guest will be paused on Xen's suspend
> > +and unpaused on Xen's resume. That way, a guest which doesn't have
> > +power management support cannot prevent the physical system from
> > +suspending when the decision to suspend is made by privileged software
> > (Dom0).
> > +
> > +Each guest in the system is responsible for suspending the devices it owns.
> > +If a guest does not suspend a device, the device will continue to
> > +operate as it is configured at the moment when the system suspends. If
> > +a device triggers an interrupt while the physical system is suspended, the
> > system will resume.
> > +
> > +It is recommended for an unprivileged guest to participate in power
> > +management in the following scenario:
> > +Assume unprivileged guest owns a device which will trigger interrupt at
> > +some point. This interrupt will wake-up the system. If such a behavior
> > +is not wanted, coordination between Dom0 and the guest is required in
> > +order to inform the guest about the intended physical system suspend.
> > +Then, the guest will have a chance to suspend the device or respond to the
> > request in an abort fashion.
> > +
> > +Since this proposal is focused on implementing PSCI-based suspend
> > +mechanisms in Xen, communication with or among the guests is not covered by
> > this document.
> > +The order of suspending the guests is assumed to be guaranteed by the
> > +software running in EL1.
> > +
> > +This document covers the following:
> > +1) Mechanism for suspending/resuming a guest:
> > +   1.1) Suspend is initiated by the guest
> > +   1.2) Resume is initiated by a device interrupt
> > +2) Mechanism for pausing/unpausing running guests when Dom0 suspends
> 
> Will this take care of passthroughed devices for DomU?

Hi Peng,

The ZynqMP uses the EEMI Firmware interface to do power-management.
https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf

In our case, we've implemented an EEMI mediator in Xen that traps EEMI
requests from domU's and makes sure that the guest owns the device it
is trying to operate on.
https://github.com/Xilinx/xen/blob/xilinx/stable-4.9/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c

So domU will first issue the usual EEMI calls as it would in a non-virtualized
case to suspend all it's devices. Once that has happened, the guest will issue
PSCI calls to suspend the VM. So, Mirela please shim in if I missed something.

The EEMI mediator has been posted to the ML but is currently sitting in our
tree waiting for us to go through the upstreaming effort.

Cheers,
Edgar




> 
> Thanks,
> Peng.
> 
> > +3) Mechanism for suspending/resuming Xen when Dom0 completes suspend
> > +4) Resuming from any state on a wake-up event (device interrupt):
> > +   4.1) Resume DomU on wake-up event when Dom0 is still running
> > +   4.2) Resume DomU on wake-up event when Xen is suspended
> > +   4.3) Resume Dom0 on wake-up event
> > +
> > +Mechanisms enumerated above will allow different kind of policies and
> > +coordination among guests to be implemented in EL1. That is out of the
> > +scope of this document.
> > +
> > +-
> > +Suspending Guests
> > +-
> > +
> > +Suspend procedure for a guest consists of the following:
> > +1) Suspending devices
> > +2) Suspending non-boot CPUs (based on hotplug/PSCI)
> > +3) System suspend, performed by the boot CPU
> > +
> > +Each guest should suspend the devices it owns just like it would when
> > +running without Xen.
> > +
> > +Guests should suspend their non-boot vCPUs using the hotplug mechanism.
> > +Virtual CPUs should be put offline using the already implemented PSCI
> > +vCPU_OFF call (prefix 'v' is added to distinguish PSCI calls made by
> > +guests to Xen, which affect virtual machines; as opposed to PSCI calls
> > +made by Xen to the EL3, which can affect power state of the physical
> > machine).
> > +
> > +After suspending its non-boot vCPUs a guest should finalize 

Re: [Xen-devel] [PATCH v18 05/11] x86/mm: add HYPERVISOR_memory_op to acquire guest resources

[Jan Beulich]

>>> On 22.03.18 at 12:55,  wrote:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -967,6 +967,94 @@ static long xatp_permission_check(struct domain *d, 
> unsigned int space)
>  return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
>  }
>  
> +static int acquire_resource(
> +XEN_GUEST_HANDLE_PARAM(xen_mem_acquire_resource_t) arg)
> +{
> +struct domain *d, *currd = current->domain;
> +xen_mem_acquire_resource_t xmar;
> +/*
> + * The mfn_list and gfn_list (below) arrays are ok on stack for the
> + * moment since they are small, but if they need to grow in future
> + * use-cases then per-CPU arrays or heap allocations may be required.
> + */
> +xen_pfn_t mfn_list[2];
> +int rc;
> +
> +if ( copy_from_guest(, arg, 1) )
> +return -EFAULT;
> +
> +if ( xmar.flags != 0 )
> +return -EINVAL;
> +
> +if ( guest_handle_is_null(xmar.frame_list) )
> +{
> +if ( xmar.nr_frames )
> +return -EINVAL;
> +
> +xmar.nr_frames = ARRAY_SIZE(mfn_list);
> +
> +if ( __copy_field_to_guest(arg, , nr_frames) )
> +return -EFAULT;
> +
> +return 0;
> +}
> +
> +if ( xmar.nr_frames > ARRAY_SIZE(mfn_list) )
> +return -E2BIG;
> +
> +rc = rcu_lock_remote_domain_by_id(xmar.domid, );
> +if ( rc )
> +return rc;
> +
> +rc = xsm_domain_resource_map(XSM_DM_PRIV, d);
> +if ( rc )
> +goto out;
> +
> +switch ( xmar.type )
> +{
> +default:
> +rc = -EOPNOTSUPP;
> +break;
> +}
> +
> +if ( rc )
> +goto out;
> +
> +if ( !paging_mode_translate(currd) )
> +{
> +if ( copy_to_guest(xmar.frame_list, mfn_list, xmar.nr_frames) )
> +rc = -EFAULT;
> +}
> +else
> +{
> +xen_pfn_t gfn_list[ARRAY_SIZE(mfn_list)];
> +unsigned int i;
> +
> +if ( copy_from_guest(gfn_list, xmar.frame_list, xmar.nr_frames) )
> +rc = -EFAULT;
> +
> +for ( i = 0; !rc && i < xmar.nr_frames; i++ )
> +{
> +rc = set_foreign_p2m_entry(currd, gfn_list[i],
> +   _mfn(mfn_list[i]));
> +if ( rc )
> +/*
> + * Make sure rc is -EIO for any iteration other than
> + * the first.
> + */
> +rc = i ? -EIO : rc;

Perhaps easier as

/*
 * Make sure rc is -EIO for any iteration other than
 * the first.
 */
if ( rc && i )
rc = -EIO;

? Looks like the comment could then also be a single line one.

> +}
> +}
> +
> +if ( xmar.flags != 0 &&
> + __copy_field_to_guest(arg, , flags) )
> +rc = -EFAULT;
> +
> + out:
> +rcu_unlock_domain(d);
> +return rc;
> +}

Blank line please ahead of main "return".

> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -599,6 +599,59 @@ struct xen_reserved_device_memory_map {
>  typedef struct xen_reserved_device_memory_map 
> xen_reserved_device_memory_map_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t);
>  
> +/*
> + * Get the pages for a particular guest resource, so that they can be
> + * mapped directly by a tools domain.
> + */
> +#define XENMEM_acquire_resource 28
> +struct xen_mem_acquire_resource {
> +/* IN - The domain whose resource is to be mapped */
> +domid_t domid;
> +/* IN - the type of resource */
> +uint16_t type;
> +/*
> + * IN - a type-specific resource identifier, which must be zero
> + *  unless stated otherwise.
> + */
> +uint32_t id;
> +/*
> + * IN/OUT - As an IN parameter number of frames of the resource
> + *  to be mapped. However, if the specified value is 0 and
> + *  frame_list is NULL then this field will be set to the
> + *  maximum value supported by the implementation on return.
> + */
> +uint32_t nr_frames;
> +/*
> + * OUT - Must be zero on entry. On return this may contain a bitwise
> + *   OR of the following values.
> + */
> +uint32_t flags;
> +
> +/* The resource pages have been assigned to the tools domain */
> +#define _XENMEM_resource_flag_tools_owned 0
> +#define XENMEM_resource_flag_tools_owned (1u << 
> _XENMEM_resource_flag_tools_owned)

Is "tools" really an appropriate (and "flag" a necessary) name
component here? How about e.g. XENMEM_res_acq_caller_owned?

> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -86,6 +86,7 @@
>  !memory_map  memory.h
>  !memory_reservation  memory.h
>  !mem_access_op   memory.h
> +!mem_acquire_resourcememory.h

Why ! ? The layout doesn't appear to differ between native and
compat. Or wait, the handle does, but why is that not
XEN_GUEST_HANDLE_64()? (I've skipped the 

[Xen-devel] [rumprun test] 121264: regressions - FAIL

[osstest service owner]

flight 121264 rumprun real [real]
http://logs.test-lab.xenproject.org/osstest/logs/121264/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumprun   6 rumprun-buildfail REGR. vs. 106754
 build-i386-rumprun6 rumprun-buildfail REGR. vs. 106754

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumprun-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumprun-amd64  1 build-check(1)   blocked  n/a

version targeted for testing:
 rumprun  94bdf32ac57b84c1b42150d21f0ad79b3b5dd99c
baseline version:
 rumprun  c7f2f016becc1cd0e85da6e1b25a8e7f9fb2aa74

Last test of basis   106754  2017-03-18 04:21:25 Z  373 days
Testing same since   120360  2018-03-09 04:19:20 Z   17 days   13 attempts


People who touched revisions under test:
  Kent McLeod 
  Kent McLeod 
  Naja Melan 
  Sebastian Wicki 
  Wei Liu 

jobs:
 build-amd64  pass
 build-i386   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 build-amd64-rumprun  fail
 build-i386-rumprun   fail
 test-amd64-amd64-rumprun-amd64   blocked 
 test-amd64-i386-rumprun-i386 blocked 



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

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

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

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


Not pushing.


commit 94bdf32ac57b84c1b42150d21f0ad79b3b5dd99c
Merge: 8fe40c8 b3c1033
Author: Kent McLeod 
Date:   Fri Feb 16 09:15:45 2018 +1100

Merge pull request #118 from kent-mcleod/stretch-linking-defaultpie

Fix linking on Debian Stretch (gcc-6)

commit b3c1033b090b65e8e86999ddd063c174502aa3f0
Author: Kent McLeod 
Date:   Wed Feb 14 16:43:16 2018 +1100

Add further -no-pie checks to Rumprun build tools

This builds upon the previous commit to add -no-pie anywhere the
relocatable flag (-Wl,-r) is used to handle compilers that enable -pie
by default (Such as Debian Stretch).

commit 8fe40c84edddfbf472b4a7cce960df749701174c
Merge: c7f2f01 685f4ab
Author: Sebastian Wicki 
Date:   Fri Jan 5 15:04:18 2018 +0100

Merge pull request #112 from najamelan/bugfix/gcc7-fallthrough

Add the -Wimplicit-fallthrough=0 flag to allow compiling with GCC7

commit 685f4ab3b74b6f1e1b40bdd3d2c42efa44bf385d
Author: Naja Melan 
Date:   Thu Jan 4 16:07:46 2018 +

Make the disabling of the fallthrough warning dependent on GCC version

This should prevent older gcc versions from choking on unknown argument.

I have not tested this, just wrote the code directly on github. Use with 
caution.

commit 34056451174e8722b972229fefc1bf9e0b89a7da
Author: Naja Melan 
Date:   Wed Jan 3 18:57:50 2018 +

Add the -Wimplicit-fallthrough=0 flag to allow compiling with GCC7

GCC7 comes with a new warning "implicit-fallthrough" which will prevent 
building the netbsd-src.

For more information: 
https://dzone.com/articles/implicit-fallthrough-in-gcc-7

commit 35d81194b7feb75d20af3ba4fdb45ea76230852f
Author: Wei Liu 
Date:   Wed Jun 7 16:30:00 2017 +0100

Fix linking on Debian Stretch

Provide cc-option. Use that to check if -no-pie is available and
append it when necessary.

Signed-off-by: Wei Liu 

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

[Xen-devel] [PATCH v1.1 for-4.11 0/3] vpci bugfixes

[Roger Pau Monne]

Hello,

This tree patches are bugfixes for the vPCI code merged last week. They
where spotted by Coverity.

Version v1.1 is due to the fact that I've failed to Cc the maintainers
in v1, sorry for the spam.

Thanks, Roger.

Roger Pau Monne (3):
  vpci/bars: fix error message
  vpci/msix: fix incorrect usage of bitmask
  vpci/msi: fix size of the vectors fields

 xen/drivers/vpci/header.c | 2 +-
 xen/drivers/vpci/msix.c   | 2 +-
 xen/include/xen/vpci.h| 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.16.2


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

[Xen-devel] [PATCH v1.1 for-4.11 2/3] vpci/msix: fix incorrect usage of bitmask

[Roger Pau Monne]

The bitmask to clear the low bits of the address field should be
~0xull, the current mask clears both the low and the high bits
of the address field, which is a bug.

Reported-by: Coverity
Signed-off-by: Roger Pau Monné 
---
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 
---
 xen/drivers/vpci/msix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 3b378c2e51..bcf63256f6 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -328,7 +328,7 @@ static int msix_write(struct vcpu *v, unsigned long addr, 
unsigned int len,
 entry->addr = data;
 break;
 }
-entry->addr &= ~0x;
+entry->addr &= ~0xull;
 entry->addr |= data;
 break;
 
-- 
2.16.2


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

[Xen-devel] [PATCH v1.1 for-4.11 3/3] vpci/msi: fix size of the vectors fields

[Roger Pau Monne]

The current size (5bits) is not enough to store the maximum number of
vectors (32), bump it by one bit.

Note that the size of the struct is still the same.

Reported-by: Coverity
Signed-off-by: Roger Pau Monné 
---
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 
---
 xen/include/xen/vpci.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index cb39e0ebea..fac12a1c42 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -100,7 +100,7 @@ struct vpci {
 /* Data. */
 uint16_t data;
 /* Maximum number of vectors supported by the device. */
-uint8_t max_vectors : 5;
+uint8_t max_vectors : 6;
 /* Enabled? */
 bool enabled: 1;
 /* Supports per-vector masking? */
@@ -108,7 +108,7 @@ struct vpci {
 /* 64-bit address capable? */
 bool address64  : 1;
 /* Number of vectors configured. */
-uint8_t vectors : 5;
+uint8_t vectors : 6;
 /* Arch-specific data. */
 struct vpci_arch_msi arch;
 } *msi;
-- 
2.16.2


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

[Xen-devel] [PATCH v1.1 for-4.11 1/3] vpci/bars: fix error message

[Roger Pau Monne]

Error message is incorrectly using map when it should be using
map->map instead.

Reported-by: Coverity
Signed-off-by: Roger Pau Monné 
---
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 
---
 xen/drivers/vpci/header.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 91a71ca66e..0ec4c082a6 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -63,7 +63,7 @@ static int map_range(unsigned long s, unsigned long e, void 
*data,
 {
 printk(XENLOG_G_WARNING
"Failed to identity %smap [%lx, %lx] for d%d: %d\n",
-   map ? "" : "un", s, e, map->d->domain_id, rc);
+   map->map ? "" : "un", s, e, map->d->domain_id, rc);
 break;
 }
 ASSERT(rc < size);
-- 
2.16.2


___
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] vpci bugfixes

[Roger Pau Monné]

On Mon, Mar 26, 2018 at 12:21:01PM +0100, Roger Pau Monne wrote:
> Hello,
> 
> This tree patches are bugfixes for the vPCI code merged last week. They
> where spotted by Coverity.

Sorry I've failed to CC the maintainers, will resend.

Roger.

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

[Xen-devel] [PATCH for-4.11 1/3] vpci/bars: fix error message

[Roger Pau Monne]

Error message is incorrectly using map when it should be using
map->map instead.

Reported-by: Coverity
Signed-off-by: Roger Pau Monné 
---
 xen/drivers/vpci/header.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 91a71ca66e..0ec4c082a6 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -63,7 +63,7 @@ static int map_range(unsigned long s, unsigned long e, void 
*data,
 {
 printk(XENLOG_G_WARNING
"Failed to identity %smap [%lx, %lx] for d%d: %d\n",
-   map ? "" : "un", s, e, map->d->domain_id, rc);
+   map->map ? "" : "un", s, e, map->d->domain_id, rc);
 break;
 }
 ASSERT(rc < size);
-- 
2.16.2


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

[Xen-devel] [PATCH for-4.11 2/3] vpci/msix: fix incorrect usage of bitmask

[Roger Pau Monne]

The bitmask to clear the low bits of the address field should be
~0xull, the current mask clears both the low and the high bits
of the address field, which is a bug.

Reported-by: Coverity
Signed-off-by: Roger Pau Monné 
---
 xen/drivers/vpci/msix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 3b378c2e51..bcf63256f6 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -328,7 +328,7 @@ static int msix_write(struct vcpu *v, unsigned long addr, 
unsigned int len,
 entry->addr = data;
 break;
 }
-entry->addr &= ~0x;
+entry->addr &= ~0xull;
 entry->addr |= data;
 break;
 
-- 
2.16.2


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

[Xen-devel] [PATCH for-4.11 3/3] vpci/msi: fix size of the vectors fields

[Roger Pau Monne]

The current size (5bits) is not enough to store the maximum number of
vectors (32), bump it by one bit.

Note that the size of the struct is still the same.

Reported-by: Coverity
Signed-off-by: Roger Pau Monné 
---
 xen/include/xen/vpci.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index cb39e0ebea..fac12a1c42 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -100,7 +100,7 @@ struct vpci {
 /* Data. */
 uint16_t data;
 /* Maximum number of vectors supported by the device. */
-uint8_t max_vectors : 5;
+uint8_t max_vectors : 6;
 /* Enabled? */
 bool enabled: 1;
 /* Supports per-vector masking? */
@@ -108,7 +108,7 @@ struct vpci {
 /* 64-bit address capable? */
 bool address64  : 1;
 /* Number of vectors configured. */
-uint8_t vectors : 5;
+uint8_t vectors : 6;
 /* Arch-specific data. */
 struct vpci_arch_msi arch;
 } *msi;
-- 
2.16.2


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

[Xen-devel] [PATCH for-4.11 0/3] vpci bugfixes

[Roger Pau Monne]

Hello,

This tree patches are bugfixes for the vPCI code merged last week. They
where spotted by Coverity.

Thanks, Roger.

Roger Pau Monne (3):
  vpci/bars: fix error message
  vpci/msix: fix incorrect usage of bitmask
  vpci/msi: fix size of the vectors fields

 xen/drivers/vpci/header.c | 2 +-
 xen/drivers/vpci/msix.c   | 2 +-
 xen/include/xen/vpci.h| 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.16.2


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

Re: [Xen-devel] Sunseting mercurial

[Anthony PERARD]

On Mon, Mar 26, 2018 at 04:35:40AM -0600, Jan Beulich wrote:
> >>> On 25.03.18 at 04:46,  wrote:
> > Its been officially 5+ years since Xen has moved to git so I propose we
> > start thinking about when to retire the mercurial mirrors. At this point
> > the last stable version to be tracked in mercurial is 4.4 which is long
> > out of any form of support. I know some vendors still have support for
> > versions of Xen down to 4.1 but let's be realistic, there's not a flurry
> > of development happening in those old versions. The mercurial mirror is
> > often out of date (I know someone that's tried to use it) and in fact as

Cardoe, if you mean "often out of date", that's true, but only by 10min
(which is the frequency at which a cron run).

> > of this email its several weeks out of date.

I'm guessing it's out of date just because I didn't thought of checking
if it was working after the recent server update. I only check every 2
months. I'm going to fix the script now.

> > So maybe its time we start thinking about sunsetting the mercurial
> > mirrors and use those resources for more practical uses.
> 
> This was brought up before, and I continue to agree _as long_ as
> our web representation of the tree gains something similar to
> hg's "annotate" functionality. Without that I find it quite hard to
> locate commits most recently changing a line or an area of code.
> Of course aiui this can be done from the command line, but only if
> one happens to have a repo on the particular machine (which for
> example I don't have or intend to have at home).
> 
> Speaking of mercurial, Anthony, wasn't it you who looks after the
> git -> hg mirroring? The unstable.hg branch didn't get updated
> after the last push from staging to master. Could you please see
> if this can be rectified (assuming that my memory didn't fail me in
> the first place, and it was someone else)?

Yes, that me that looks after it. I'll fix the mirror.

Thanks,

-- 
Anthony PERARD

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

Re: [Xen-devel] Sunseting mercurial

[Roger Pau Monné]

On Mon, Mar 26, 2018 at 04:35:40AM -0600, Jan Beulich wrote:
> >>> On 25.03.18 at 04:46,  wrote:
> > Its been officially 5+ years since Xen has moved to git so I propose we
> > start thinking about when to retire the mercurial mirrors. At this point
> > the last stable version to be tracked in mercurial is 4.4 which is long
> > out of any form of support. I know some vendors still have support for
> > versions of Xen down to 4.1 but let's be realistic, there's not a flurry
> > of development happening in those old versions. The mercurial mirror is
> > often out of date (I know someone that's tried to use it) and in fact as
> > of this email its several weeks out of date.
> > 
> > So maybe its time we start thinking about sunsetting the mercurial
> > mirrors and use those resources for more practical uses.
> 
> This was brought up before, and I continue to agree _as long_ as
> our web representation of the tree gains something similar to
> hg's "annotate" functionality. Without that I find it quite hard to
> locate commits most recently changing a line or an area of code.
> Of course aiui this can be done from the command line, but only if
> one happens to have a repo on the particular machine (which for
> example I don't have or intend to have at home).

AFAICT this could be enabled by setting:

$feature{'blame'}{'default'} = [1];

In the gitweb configuration file [0][1].

Roger.

[0] https://git-scm.com/docs/gitweb#gitweb-arguments
[1] https://git-scm.com/docs/gitweb#gitweb-blame

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

[Xen-devel] [seabios test] 121238: regressions - FAIL

[osstest service owner]

flight 121238 seabios real [real]
http://logs.test-lab.xenproject.org/osstest/logs/121238/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop   fail REGR. vs. 115539

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 115539
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 115539
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 115539
 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-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 seabios  4922d6cb391b8ea48a35a73c46e484cf5f1a9b1a
baseline version:
 seabios  0ca6d6277dfafc671a5b3718cbeb5c78e2a888ea

Last test of basis   115539  2017-11-03 20:48:58 Z  142 days
Failing since115733  2017-11-10 17:19:59 Z  135 days  154 attempts
Testing same since   121050  2018-03-22 07:01:10 Z4 days3 attempts


People who touched revisions under test:
  Kevin O'Connor 
  Marc-André Lureau 
  Marcel Apfelbaum 
  Michael S. Tsirkin 
  Nikolay Nikolov 
  Paul Menzel 
  Stefan Berger 
  Stephen Douthit 

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-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 test-amd64-amd64-qemuu-nested-amdfail
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-amd64-xl-qemuu-ws16-amd64 fail
 test-amd64-i386-xl-qemuu-ws16-amd64  fail
 test-amd64-amd64-xl-qemuu-win10-i386 fail
 test-amd64-i386-xl-qemuu-win10-i386  fail
 test-amd64-amd64-qemuu-nested-intel  pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass



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

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

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

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


Not pushing.

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

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

Re: [Xen-devel] [PATCH] retire bitkeeper bits

[Wei Liu]

On Sat, Mar 24, 2018 at 09:32:47PM -0500, Doug Goldstein wrote:
> While the project could migrate from git to $nextscm, its unlikely that
> these bits will ever be useful again.
> 
> Signed-off-by: Doug Goldstein 

Acked-by: Wei Liu 

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

[Xen-devel] [C stubdom] printf to console not complete

[Lele Ma]

Dear all,

This is Lele Ma, a graduate student from The College of William and Mary. I
am playing with c-stubdom on Xen-4.9.1 and encountered problems when do
printings using printf.

It seems that if I print many lines of strings consecutively, many lines
would got cut at the tail.
Here is an example code I tested in C stubdom:

// file /stubdom/c/main.c

int main(void) {
  int i = 0;
  for (i=0; i< 1000; i++){
printf("print %d \n", i);
  }
  return 0;
}

After build and run, the output got cut in the middle and only the first
~400 lines got printed, as following:

- output begin -

root@xen-4.9.1: c # xl create -c ../../extras/mini-os/domain_config
Parsing config from ../../extras/mini-os/domain_config
...
print 427*<---looks good*
print 428
print 429
print 430
print 431
print 432
print 433
print 434   *<--- prints got cut here (should print to 999), and different
executions got cut in different lines. *
root@xen-4.9.1: c #

 output end ---

However, if I use sleep(1) after every printf, it could print all lines.

Could anyone give any hints about what is happening here? And where need to
be fixed to get all printings properly with printf?

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

Re: [Xen-devel] [PATCH v3 7/7] xen/x86: use PCID feature

[Jan Beulich]

>>> On 26.03.18 at 12:29,  wrote:
> On 26/03/18 12:13, Jan Beulich wrote:
> On 26.03.18 at 10:55,  wrote:
>>> I can change the scheme to use different values for guest PCIDs
>>> with XPTI on, of course. Are you fine with:
>>>
>>> - XPTI off: PCID 0 = kernel, PCID 1 = user
>>> - XPTI on:  PCID 0 = kernel/Xen, PCID 1 = user/Xen,
>>> PCID 2 = kernel/guest, PCID 3 = user/guest
>> 
>> Yes, that would fit the first variant I've described. I take it you
>> prefer not to avoid PCID 0 altogether when PCIDs are enabled -
>> is there a particular reason?
> 
> Yes. As written in the comment in flush_area_local() I can't be sure
> whether the current address space is that of a domain with XPTI
> enabled (the idle domain could be "current"). So I need to always
> flush with PCID 0 and with the possible PCID values for a XPTI domain.
> When using PCID 0 for XPTI as well I'll need 4 INVPCIDs, while when
> avoiding it I'd need 5 (at least when current == idle).

I see. Which makes me wonder whether a suitable combination
of INVLPG (to get rid of global entries) and INVPCID couldn't be
used instead. For example, you may be able to replace the
INVPCID for the active PCID by INVLPG (without needing to
know who "current" is).

Jan


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

Re: [Xen-devel] Sunseting mercurial

[Jan Beulich]

>>> On 25.03.18 at 04:46,  wrote:
> Its been officially 5+ years since Xen has moved to git so I propose we
> start thinking about when to retire the mercurial mirrors. At this point
> the last stable version to be tracked in mercurial is 4.4 which is long
> out of any form of support. I know some vendors still have support for
> versions of Xen down to 4.1 but let's be realistic, there's not a flurry
> of development happening in those old versions. The mercurial mirror is
> often out of date (I know someone that's tried to use it) and in fact as
> of this email its several weeks out of date.
> 
> So maybe its time we start thinking about sunsetting the mercurial
> mirrors and use those resources for more practical uses.

This was brought up before, and I continue to agree _as long_ as
our web representation of the tree gains something similar to
hg's "annotate" functionality. Without that I find it quite hard to
locate commits most recently changing a line or an area of code.
Of course aiui this can be done from the command line, but only if
one happens to have a repo on the particular machine (which for
example I don't have or intend to have at home).

Speaking of mercurial, Anthony, wasn't it you who looks after the
git -> hg mirroring? The unstable.hg branch didn't get updated
after the last push from staging to master. Could you please see
if this can be rectified (assuming that my memory didn't fail me in
the first place, and it was someone else)?

Thanks, Jan


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

Re: [Xen-devel] [PATCH v3 7/7] xen/x86: use PCID feature

[Juergen Gross]

On 26/03/18 12:13, Jan Beulich wrote:
 On 26.03.18 at 10:55,  wrote:
>> On 26/03/18 10:28, Jan Beulich wrote:
>> On 26.03.18 at 08:49,  wrote:
 On 23/03/18 16:58, Jan Beulich wrote:
 On 23.03.18 at 15:11,  wrote:
>> On 23/03/18 14:46, Jan Beulich wrote:
>>> So in the end the question is: Why not use just two PCIDs, and
>>> allow global pages just like we do now, with the added benefit
>>> that we no longer need to flush Xen's global TLB entries just
>>> because we want to get rid of PV guest user ones.
>>
>> I can't see how that would work without either needing some more TLB
>> flushes in order to prevent stale TLB entries or loosing the Meltdown
>> mitigation.
>>
>> Which %cr3/PCID combination should be used in hypervisor, guest kernel
>> and guest user mode?
>
> Xen would run with PCID 0 (and full Xen mappings) at all times
> (except early entry and late exit code of course). The guest would
> run with PCID 1 (and minimal Xen mappings) at all times. The switch
> of PCID eliminates the need for flushes on the way out and back in.

 You still need the kernel page tables flushed when switching to user
 mode, right?
>>>
>>> Of course.
>>>
>> Which pages would be global?
>
> Use of global pages would continue to be as today: Xen has some,
> and guest user mode has some. Of course it is quite possible that
> the use of global pages with a single guest PCID is still worse than
> no global pages with two guest PCIDs, but that's a separate step
> to take (and measure) imo.

 But global pages of Xen would either make it vulnerable with regard to
 Meltdown or you need a TLB flush again when switching between Xen and
 guest making all the PCID stuff moot.
>>>
>>> No - the guest would run with PCID 1 active, and global Xen TLB
>>> entries would exist for PCID 0 only.
>>
>> Uuh, global pages are accessible via all PCIDs. That's why they are
>> called global...
> 
> Okay, in that case all of what I've said in this regard was rubbish.
> (I don't, btw, think that this is the only sensible interpretation of
> "global" - it could as well mean protected from ordinary flushes
> within the given PCID realm.)

That's the reason I gave the reference to the SDM. It clearly states
that TLB entries with the global bit set don't have to match the current
PCID for being regarded to match.

> 
 - 2 PCIDs
 - no TLB flushes needed when switching between Xen and guest
 - when switching from guest kernel to guest user the kernel pages must
   be flushed from TLB
 - flushing of single guest user pages needs 2 changes of %cr3 and 2
   INVLPGs, switch code must be mapped to guest page tables
 - flushing of complete TLB via 1 INVPCID

 So the advantage of the 2 PCID solution are the single TLB entries for
 guest user pages compared to 2 entries for guest user pages accessed by
 the guest kernel or Xen.

 The disadvantage are the flushed guest kernel pages when executing user
 code, the more complicated single user page flushing and the dynamical
 Xen global bit handling.
>>>
>>> Right. In order to make forward progress here I think we should
>>> shelve the discussion on the 2-PCID alternative for now. What I'd
>>> like to ask for as a change to your current approach is to use
>>> PCID 0 for Xen rather than running Xen with PCIDs 2 or 3 when
>>> PCIDs are enabled, and (implicitly) with PCID 0 when they're
>>> disabled. Or alternatively don't use PCID 0 at all when PCIDs are
>>> enabled. I'm simply worried of us overlooking a case where PCID
>>> 0 TLB entries may be left in place (when switching between PCIDs
>>> enabled and PCIDs disabled) when they should have been flushed,
>>> opening back up a Meltdown-like attack window.
>> The reason I didn't use PCID 0 for running Xen was to use a few
>> INVPCID calls as possible for single page invalidation and still
>> covering the cases for PCID on while XPTI off and including PCID 0.
> 
> How would the number of INVPCIDs needed differ depending on
> the actual PCID values used?

See answer below.

>> I can change the scheme to use different values for guest PCIDs
>> with XPTI on, of course. Are you fine with:
>>
>> - XPTI off: PCID 0 = kernel, PCID 1 = user
>> - XPTI on:  PCID 0 = kernel/Xen, PCID 1 = user/Xen,
>> PCID 2 = kernel/guest, PCID 3 = user/guest
> 
> Yes, that would fit the first variant I've described. I take it you
> prefer not to avoid PCID 0 altogether when PCIDs are enabled -
> is there a particular reason?

Yes. As written in the comment in flush_area_local() I can't be sure
whether the current address space is that of a domain with XPTI
enabled (the idle domain could be "current"). So I need to always
flush with PCID 0 and with the possible PCID values for a XPTI domain.
When using PCID 0 for XPTI as well I'll need 4 

Re: [Xen-devel] [PATCH 4/3] README: document that qemu-trad wants libpci

[Jan Beulich]

>>> On 25.03.18 at 16:26,  wrote:
> qemu-traditional wants libpci from pciutils for supporting PCI
> passthrough.

Iirc it builds fine without, disabling respective code. Hence I don't
think this fully fits the other (strict) requirements.

Jan


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

[Xen-devel] [xen-4.8-testing test] 121210: regressions - FAIL

[osstest service owner]

flight 121210 xen-4.8-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/121210/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-pairbroken  in 121083
 test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 120116

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-pair 5 host-install/dst_host(5) broken in 121083 pass in 
121210
 test-xtf-amd64-amd64-3 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 121083 pass 
in 121210
 test-xtf-amd64-amd64-1 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 121083 pass 
in 121210
 test-xtf-amd64-amd64-4   50 xtf/test-hvm64-lbr-tsx-vmentry fail pass in 121083
 test-xtf-amd64-amd64-2   50 xtf/test-hvm64-lbr-tsx-vmentry fail pass in 121083

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 120116

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 120116
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 120116
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 120116
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 120116
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 120116
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 120116
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 120116
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 120116
 build-i386-prev   7 xen-build/dist-test  fail   never pass
 test-xtf-amd64-amd64-3   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-4   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-5   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-2   52 xtf/test-hvm64-memop-seg fail   never pass
 test-xtf-amd64-amd64-1   52 xtf/test-hvm64-memop-seg fail   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
 build-amd64-prev  7 xen-build/dist-test  fail   never pass
 test-amd64-amd64-libvirt 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-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-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-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail 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-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 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-xsm  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  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-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 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
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 

Re: [Xen-devel] [PATCH v3 7/7] xen/x86: use PCID feature

[Jan Beulich]

>>> On 26.03.18 at 10:55,  wrote:
> On 26/03/18 10:28, Jan Beulich wrote:
> On 26.03.18 at 08:49,  wrote:
>>> On 23/03/18 16:58, Jan Beulich wrote:
>>> On 23.03.18 at 15:11,  wrote:
> On 23/03/18 14:46, Jan Beulich wrote:
>> So in the end the question is: Why not use just two PCIDs, and
>> allow global pages just like we do now, with the added benefit
>> that we no longer need to flush Xen's global TLB entries just
>> because we want to get rid of PV guest user ones.
>
> I can't see how that would work without either needing some more TLB
> flushes in order to prevent stale TLB entries or loosing the Meltdown
> mitigation.
>
> Which %cr3/PCID combination should be used in hypervisor, guest kernel
> and guest user mode?

 Xen would run with PCID 0 (and full Xen mappings) at all times
 (except early entry and late exit code of course). The guest would
 run with PCID 1 (and minimal Xen mappings) at all times. The switch
 of PCID eliminates the need for flushes on the way out and back in.
>>>
>>> You still need the kernel page tables flushed when switching to user
>>> mode, right?
>> 
>> Of course.
>> 
> Which pages would be global?

 Use of global pages would continue to be as today: Xen has some,
 and guest user mode has some. Of course it is quite possible that
 the use of global pages with a single guest PCID is still worse than
 no global pages with two guest PCIDs, but that's a separate step
 to take (and measure) imo.
>>>
>>> But global pages of Xen would either make it vulnerable with regard to
>>> Meltdown or you need a TLB flush again when switching between Xen and
>>> guest making all the PCID stuff moot.
>> 
>> No - the guest would run with PCID 1 active, and global Xen TLB
>> entries would exist for PCID 0 only.
> 
> Uuh, global pages are accessible via all PCIDs. That's why they are
> called global...

Okay, in that case all of what I've said in this regard was rubbish.
(I don't, btw, think that this is the only sensible interpretation of
"global" - it could as well mean protected from ordinary flushes
within the given PCID realm.)

>>> - 2 PCIDs
>>> - no TLB flushes needed when switching between Xen and guest
>>> - when switching from guest kernel to guest user the kernel pages must
>>>   be flushed from TLB
>>> - flushing of single guest user pages needs 2 changes of %cr3 and 2
>>>   INVLPGs, switch code must be mapped to guest page tables
>>> - flushing of complete TLB via 1 INVPCID
>>>
>>> So the advantage of the 2 PCID solution are the single TLB entries for
>>> guest user pages compared to 2 entries for guest user pages accessed by
>>> the guest kernel or Xen.
>>>
>>> The disadvantage are the flushed guest kernel pages when executing user
>>> code, the more complicated single user page flushing and the dynamical
>>> Xen global bit handling.
>> 
>> Right. In order to make forward progress here I think we should
>> shelve the discussion on the 2-PCID alternative for now. What I'd
>> like to ask for as a change to your current approach is to use
>> PCID 0 for Xen rather than running Xen with PCIDs 2 or 3 when
>> PCIDs are enabled, and (implicitly) with PCID 0 when they're
>> disabled. Or alternatively don't use PCID 0 at all when PCIDs are
>> enabled. I'm simply worried of us overlooking a case where PCID
>> 0 TLB entries may be left in place (when switching between PCIDs
>> enabled and PCIDs disabled) when they should have been flushed,
>> opening back up a Meltdown-like attack window.
> The reason I didn't use PCID 0 for running Xen was to use a few
> INVPCID calls as possible for single page invalidation and still
> covering the cases for PCID on while XPTI off and including PCID 0.

How would the number of INVPCIDs needed differ depending on
the actual PCID values used?

> I can change the scheme to use different values for guest PCIDs
> with XPTI on, of course. Are you fine with:
> 
> - XPTI off: PCID 0 = kernel, PCID 1 = user
> - XPTI on:  PCID 0 = kernel/Xen, PCID 1 = user/Xen,
> PCID 2 = kernel/guest, PCID 3 = user/guest

Yes, that would fit the first variant I've described. I take it you
prefer not to avoid PCID 0 altogether when PCIDs are enabled -
is there a particular reason?

Jan

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